Skip to content

Commit d83f4e1

Browse files
ulugbeknaCopilot
andcommitted
automations: fix: backport catalogue readiness for badge
The badge cherry-pick calls catalogueState, but release/1.137 does not yet contain the catalogue API from #334836. Backport the required contract, aggregate readiness, legacy readability, and Agent Host lifecycle support without importing the templates UI. Include focused catalogue and migration regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e75f732-f18f-42a7-b1e2-a034c03596eb
1 parent b592d2f commit d83f4e1

9 files changed

Lines changed: 556 additions & 48 deletions

File tree

src/vs/sessions/contrib/automations/browser/automationService.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
isAutomationModelConfiguration,
2323
} from '../../../../workbench/contrib/chat/common/automations/automation.js';
2424
import {
25+
AutomationCatalogueState,
2526
type AutomationMutationGuard,
2627
assertAutomationSessionTemplateAuthority,
2728
IAutomationRunClaim,
@@ -122,14 +123,15 @@ export class AutomationStore extends Disposable implements IAutomationStore {
122123

123124
private readonly _automations: ISettableObservable<readonly IAutomationDescriptor[]>;
124125
private readonly _runs: ISettableObservable<readonly IAutomationRun[]>;
126+
private readonly _catalogueState: ISettableObservable<AutomationCatalogueState>;
125127
private _now: () => Date;
126128
private readonly _runsForCache = new Map<string, IObservable<readonly IAutomationRun[]>>();
127129

128130
private _lastSeenRevision = 0;
129-
private _canCompleteMigration = true;
130131

131132
readonly automations: IObservable<readonly IAutomationDescriptor[]>;
132133
readonly runs: IObservable<readonly IAutomationRun[]>;
134+
readonly catalogueState: IObservable<AutomationCatalogueState>;
133135

134136
constructor(
135137
private readonly storageKey: string,
@@ -144,14 +146,15 @@ export class AutomationStore extends Disposable implements IAutomationStore {
144146

145147
const result = this.readLedger(this.storageService.get(this.storageKey, StorageScope.APPLICATION));
146148
const initial = result.kind === 'unsupportedSchema' ? EMPTY_LEDGER : result.ledger;
147-
this._canCompleteMigration = result.kind === 'ledger';
148149
if (result.kind !== 'unsupportedSchema') {
149150
this._lastSeenRevision = result.revision;
150151
}
151152
this._automations = observableValue<readonly IAutomationDescriptor[]>(this, initial.automations);
152153
this._runs = observableValue<readonly IAutomationRun[]>(this, initial.runs);
154+
this._catalogueState = observableValue(this, result.kind === 'ledger' ? 'ready' : 'error');
153155
this.automations = this._automations;
154156
this.runs = this._runs;
157+
this.catalogueState = this._catalogueState;
155158

156159
this._register(this.storageService.onDidChangeValue(StorageScope.APPLICATION, this.storageKey, this._store)(() => {
157160
this.refreshFromStorage();
@@ -168,7 +171,7 @@ export class AutomationStore extends Disposable implements IAutomationStore {
168171
}
169172

170173
canCompleteMigration(): boolean {
171-
return this._canCompleteMigration;
174+
return this._catalogueState.get() === 'ready';
172175
}
173176

174177
runsFor(automationId: string): IObservable<readonly IAutomationRun[]> {
@@ -475,9 +478,11 @@ export class AutomationStore extends Disposable implements IAutomationStore {
475478
while (true) {
476479
const readResult = this.readLedger(raw);
477480
if (readResult.kind === 'unsupportedSchema') {
481+
this._catalogueState.set('error', undefined);
478482
throw new Error('Cannot modify automations: storage was written by a newer version');
479483
}
480484
if (readResult.kind === 'invalid') {
485+
this._catalogueState.set('error', undefined);
481486
throw new Error('Cannot modify automations: persisted storage contains data this version cannot safely interpret');
482487
}
483488

@@ -512,30 +517,33 @@ export class AutomationStore extends Disposable implements IAutomationStore {
512517
}
513518
}
514519

515-
private acceptLedger(ledger: ILedger, revision: number): void {
520+
private acceptLedger(ledger: ILedger, revision: number, catalogueState: AutomationCatalogueState = 'ready'): void {
516521
if (revision < this._lastSeenRevision) {
522+
if (catalogueState === 'error') {
523+
this._catalogueState.set(catalogueState, undefined);
524+
}
517525
return;
518526
}
519-
this.setLedger(ledger, revision);
527+
this.setLedger(ledger, revision, catalogueState);
520528
}
521529

522-
private setLedger(ledger: ILedger, revision: number): void {
530+
private setLedger(ledger: ILedger, revision: number, catalogueState: AutomationCatalogueState = 'ready'): void {
523531
this._lastSeenRevision = revision;
524532
transaction(tx => {
525533
this._automations.set(ledger.automations, tx);
526534
this._runs.set(ledger.runs, tx);
535+
this._catalogueState.set(catalogueState, tx);
527536
});
528537
}
529538

530539
private refreshFromStorage(): void {
531540
const result = this.readLedger(this.storageService.get(this.storageKey, StorageScope.APPLICATION));
532541
if (result.kind === 'unsupportedSchema') {
533-
this._canCompleteMigration = false;
542+
this._catalogueState.set('error', undefined);
534543
return;
535544
}
536545

537-
this._canCompleteMigration = result.kind === 'ledger';
538-
this.acceptLedger(result.ledger, result.revision);
546+
this.acceptLedger(result.ledger, result.revision, result.kind === 'ledger' ? 'ready' : 'error');
539547
}
540548

541549
private readLedger(raw: string | undefined): ReadLedgerResult {

src/vs/sessions/contrib/automations/browser/automations.contribution.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Disposable } from '../../../../base/common/lifecycle.js';
7+
import { observableFromPromise } from '../../../../base/common/observable.js';
78
import { localize } from '../../../../nls.js';
89
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
910
import { ConfigurationScope, Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../../platform/configuration/common/configurationRegistry.js';
1011
import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
12+
import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
1113
import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js';
1214
import product from '../../../../platform/product/common/product.js';
1315
import { Registry } from '../../../../platform/registry/common/platform.js';
14-
import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js';
16+
import { Extensions as WorkbenchExtensions, IWorkbenchContribution, IWorkbenchContributionsRegistry, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js';
1517
import { IAutomationDialogService } from '../../../../workbench/contrib/chat/common/automations/automationDialogService.js';
1618
import { IAutomationRunner } from '../../../../workbench/contrib/chat/common/automations/automationRunner.js';
1719
import { IAutomationService } from '../../../../workbench/contrib/chat/common/automations/automationService.js';
@@ -25,8 +27,12 @@ import { AutomationToolsContribution } from './automationTools.js';
2527
import { IAutomationStorageService } from '../common/automationStorageService.js';
2628
import { AGENT_HOST_AUTOMATIONS_ENABLED_CONFIG_KEY, AGENT_HOST_AUTOMATION_RUN_TIMEOUT_MINUTES_CONFIG_KEY } from '../../../../platform/agentHost/common/automationMigration.js';
2729

30+
const initialProvidersSettled = observableFromPromise(
31+
Registry.as<IWorkbenchContributionsRegistry>(WorkbenchExtensions.Workbench).whenRestored.then(() => true)
32+
).map(result => result.value === true);
33+
2834
registerSingleton(IAutomationStorageService, BrowserAutomationStorageService, InstantiationType.Delayed);
29-
registerSingleton(IAutomationService, ProviderAutomationService, InstantiationType.Delayed);
35+
registerSingleton(IAutomationService, new SyncDescriptor(ProviderAutomationService, [initialProvidersSettled], true));
3036
registerSingleton(IAutomationRunner, AutomationRunner, InstantiationType.Delayed);
3137
registerSingleton(IAutomationDialogService, AutomationDialogService, InstantiationType.Delayed);
3238

src/vs/sessions/contrib/automations/browser/providerAutomationService.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { localize } from '../../../../nls.js';
1111
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
1212
import { ILogService } from '../../../../platform/log/common/log.js';
1313
import { IAutomationDescriptor, IAutomationRun, AutomationRunTrigger } from '../../../../workbench/contrib/chat/common/automations/automation.js';
14-
import { AutomationMutationGuard, IAutomationRunClaim, IAutomationService, ICreateAutomationOptions, IGuardedAutomationUpdateResult, isAutomationActiveRunError, serializeAutomationEditableState, IUpdateAutomationOptions, IUpdateAutomationRunOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js';
14+
import { AutomationCatalogueState, AutomationMutationGuard, combineAutomationCatalogueStates, IAutomationRunClaim, IAutomationService, ICreateAutomationOptions, IGuardedAutomationUpdateResult, isAutomationActiveRunError, serializeAutomationEditableState, IUpdateAutomationOptions, IUpdateAutomationRunOptions } from '../../../../workbench/contrib/chat/common/automations/automationService.js';
1515
import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js';
1616
import { IAutomation, ISessionsProviderAutomations } from '../../../services/sessions/common/sessionsProvider.js';
1717
import { AutomationService } from './automationService.js';
@@ -40,15 +40,25 @@ export class ProviderAutomationService extends Disposable implements IAutomation
4040

4141
readonly automations: IObservable<readonly IAutomationDescriptor[]>;
4242
readonly runs: IObservable<readonly IAutomationRun[]>;
43+
readonly catalogueState: IObservable<AutomationCatalogueState>;
4344

4445
constructor(
46+
initialProvidersSettled: IObservable<boolean>,
4547
@ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService,
4648
@IInstantiationService instantiationService: IInstantiationService,
4749
@ILogService private readonly logService: ILogService,
4850
) {
4951
super();
5052
this.legacyStore = this._register(instantiationService.createInstance(AutomationService));
5153
this.providersChanged = observableSignalFromEvent(this, sessionsProvidersService.onDidChangeProviders);
54+
this.catalogueState = derived(this, reader => {
55+
this.providersChanged.read(reader);
56+
const states = this.getStores().map(entry => entry.store.catalogueState.read(reader));
57+
if (!initialProvidersSettled.read(reader)) {
58+
states.push('loading');
59+
}
60+
return combineAutomationCatalogueStates(states);
61+
});
5262
this.automations = derived(this, reader => {
5363
this.providersChanged.read(reader);
5464
return distinctById(

src/vs/sessions/contrib/automations/test/browser/automationService.test.ts

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@
55

66
import assert from 'assert';
77
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
8+
import { autorun } from '../../../../../base/common/observable.js';
89
import { URI } from '../../../../../base/common/uri.js';
910
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
1011
import { NullLogService } from '../../../../../platform/log/common/log.js';
1112
import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
1213
import { NullTelemetryService } from '../../../../../platform/telemetry/common/telemetryUtils.js';
1314
import { AutomationService, AutomationStore } from '../../browser/automationService.js';
1415
import { AutomationRunTrigger, AutomationTarget, AutomationWorkspaceIsolation, IAutomationRun, IAutomationSchedule } from '../../../../../workbench/contrib/chat/common/automations/automation.js';
15-
import { AutomationActiveRunError, isAutomationActiveRunError } from '../../../../../workbench/contrib/chat/common/automations/automationService.js';
16+
import { AutomationActiveRunError, type AutomationCatalogueState, isAutomationActiveRunError } from '../../../../../workbench/contrib/chat/common/automations/automationService.js';
1617
import { createAutomationService, TestAutomationStorageService } from './automationTestUtils.js';
1718

1819
const FOLDER = URI.parse('file:///workspace');
@@ -78,8 +79,15 @@ suite('AutomationService', () => {
7879

7980
test('starts with an empty ledger when nothing is persisted', () => {
8081
const { service } = createService();
81-
assert.deepStrictEqual(service.automations.get(), []);
82-
assert.deepStrictEqual(service.runs.get(), []);
82+
assert.deepStrictEqual({
83+
automations: service.automations.get(),
84+
runs: service.runs.get(),
85+
catalogueState: service.catalogueState.get(),
86+
}, {
87+
automations: [],
88+
runs: [],
89+
catalogueState: 'ready',
90+
});
8391
});
8492

8593
test('provider stores isolate ledgers by storage key', async () => {
@@ -811,6 +819,7 @@ suite('AutomationService', () => {
811819
// but the service is now in read-only mode.
812820
assert.deepStrictEqual(service.automations.get(), []);
813821
assert.deepStrictEqual(service.runs.get(), []);
822+
assert.strictEqual(service.catalogueState.get(), 'error');
814823

815824
// A subsequent mutation must be rejected (read-only mode) and must not
816825
// destroy the on-disk newer ledger.
@@ -836,7 +845,68 @@ suite('AutomationService', () => {
836845

837846
// The onDidChangeValue refresh must NOT clear our observables to
838847
// empty. We keep displaying what we last knew about.
839-
assert.strictEqual(service.automations.get().length, 1);
848+
assert.deepStrictEqual({
849+
automationCount: service.automations.get().length,
850+
catalogueState: service.catalogueState.get(),
851+
}, {
852+
automationCount: 1,
853+
catalogueState: 'error',
854+
});
855+
});
856+
857+
test('refreshFromStorage reports malformed storage after a newer valid revision', async () => {
858+
const storage = teardown.add(new InMemoryStorageService());
859+
const service = teardown.add(createAutomationService(storage, new NullLogService(), NullTelemetryService));
860+
await service.createAutomation({ name: 'Local', prompt: 'p', schedule: dailySchedule(), target: workspaceTarget() });
861+
const emissions: Array<{ automationCount: number; catalogueState: AutomationCatalogueState }> = [];
862+
teardown.add(autorun(reader => emissions.push({
863+
automationCount: service.automations.read(reader).length,
864+
catalogueState: service.catalogueState.read(reader),
865+
})));
866+
867+
storage.store('chat.automations.ledger', '{', StorageScope.APPLICATION, StorageTarget.MACHINE);
868+
869+
assert.deepStrictEqual({
870+
automationCount: service.automations.get().length,
871+
catalogueState: service.catalogueState.get(),
872+
emissions,
873+
}, {
874+
automationCount: 1,
875+
catalogueState: 'error',
876+
emissions: [
877+
{ automationCount: 1, catalogueState: 'ready' },
878+
{ automationCount: 1, catalogueState: 'error' },
879+
],
880+
});
881+
});
882+
883+
test('publishes catalogue contents and readability atomically on refresh and recovery', async () => {
884+
const { service, storage } = createService();
885+
const initialLedger = JSON.stringify({
886+
schemaVersion: 4, revision: 5,
887+
automations: [serializeLedgerAutomation('saved', 'Saved')],
888+
runs: [],
889+
});
890+
storage.store('chat.automations.ledger', initialLedger, StorageScope.APPLICATION, StorageTarget.MACHINE);
891+
const emissions: Array<{ ids: string[]; catalogueState: AutomationCatalogueState; readable: boolean }> = [];
892+
teardown.add(autorun(reader => emissions.push({
893+
ids: service.automations.read(reader).map(automation => automation.id),
894+
catalogueState: service.catalogueState.read(reader),
895+
readable: service.canCompleteMigration(),
896+
})));
897+
898+
storage.store('chat.automations.ledger', JSON.stringify({
899+
schemaVersion: 4, revision: 6, automations: [], runs: null,
900+
}), StorageScope.APPLICATION, StorageTarget.MACHINE);
901+
storage.store('chat.automations.ledger', initialLedger, StorageScope.APPLICATION, StorageTarget.MACHINE);
902+
await service.updateAutomation('saved', { name: 'Recovered' });
903+
904+
assert.deepStrictEqual(emissions, [
905+
{ ids: ['saved'], catalogueState: 'ready', readable: true },
906+
{ ids: [], catalogueState: 'error', readable: false },
907+
{ ids: ['saved'], catalogueState: 'ready', readable: true },
908+
{ ids: ['saved'], catalogueState: 'ready', readable: true },
909+
]);
840910
});
841911

842912
test('persist bumps the revision counter on every write', async () => {

0 commit comments

Comments
 (0)