Skip to content

Commit 10c4714

Browse files
ulugbeknaCopilot
andcommitted
automations: fix: use aggregate discovery for New badge
Include legacy ledger readability and live Agent Host catalogue health in badge eligibility. Propagate host disconnects, retain simple prior-window tracking, and add regression coverage without changing shared storage APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e75f732-f18f-42a7-b1e2-a034c03596eb
1 parent a3f35ed commit 10c4714

14 files changed

Lines changed: 106 additions & 149 deletions

File tree

‎src/vs/sessions/AUTOMATIONS.md‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -216,16 +216,6 @@ Every non-supported state exposes the provider-scoped legacy fallback when it co
216216

217217
After legacy data has migrated and its source is drained, disconnecting or disabling the provider does not create a second authority. The host retains durable definitions; its projection becomes available again after reconnect or re-enable.
218218

219-
### Initial discovery
220-
221-
Provider Automation stores may expose an initial-discovery state through `ISessionsProviderAutomations`:
222-
223-
- `pending` means the provider may still reveal existing definitions;
224-
- `ready` means its initial authoritative catalogue is definitive;
225-
- `unavailable` means it cannot currently establish a definitive catalogue.
226-
227-
Stores without this state are synchronously initialized. Agent Host stores become ready only after their initial catalogue and migration state are authoritative. A host without Automation capability is ready because its synchronous legacy store remains authoritative; a disconnected host is unavailable because host-owned definitions cannot be projected.
228-
229219
## Migration
230220

231221
Migration has two ownership boundaries:

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,13 +1257,16 @@ suite('ProviderAutomationService', () => {
12571257
runs: [],
12581258
});
12591259
const { service, providerStore, storage } = createService(futureLedger);
1260+
const initialDiscoveryState = service.initialDiscoveryState.get();
12601261

12611262
await assert.rejects(service.waitForMigrationForTesting(), /cannot be migrated safely/);
12621263

12631264
assert.deepStrictEqual({
1265+
initialDiscoveryState,
12641266
providerAutomations: providerStore.automations.get(),
12651267
persisted: storage.get(AUTOMATION_STORAGE_KEY, StorageScope.APPLICATION),
12661268
}, {
1269+
initialDiscoveryState: 'unavailable',
12671270
providerAutomations: [],
12681271
persisted: futureLedger,
12691272
});

‎src/vs/sessions/contrib/providers/agentHost/browser/agentHostAutomationStore.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { ITelemetryService } from '../../../../../platform/telemetry/common/tele
2626
import { assertAutomationSessionTemplate, type AutomationRunTrigger, type AutomationTarget, type IAutomationDescriptor, type IAutomationRun, type IAutomationSchedule, type IAutomationSessionTemplate } from '../../../../../workbench/contrib/chat/common/automations/automation.js';
2727
import { AutomationActiveRunError, type AutomationCatalogueState, assertAutomationSessionTemplateAuthority, combineAutomationCatalogueStates, type AutomationMutationGuard, type IAutomationRunClaim, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, isAutomationActiveRunError, serializeAutomationEditableState, type IUpdateAutomationOptions, type IUpdateAutomationRunOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js';
2828
import { publishAutomationMigration } from '../../../../../workbench/contrib/chat/common/automations/automationTelemetry.js';
29-
import type { AutomationInitialDiscoveryState, IAutomation, IAutomationSnapshotImportResult, IGuardedAutomationSnapshotRemovalResult, ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js';
29+
import type { IAutomation, IAutomationSnapshotImportResult, IGuardedAutomationSnapshotRemovalResult, ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js';
3030
import { IAutomationStorageService } from '../../../automations/common/automationStorageService.js';
3131

3232
const MUTATION_TIMEOUT_MS = 30_000;
@@ -87,7 +87,6 @@ export class AgentHostAutomationStore extends Disposable implements ISessionsPro
8787
private _migrationPromise: Promise<void> | undefined;
8888
private _lastPreflightDeferralKey: string | undefined;
8989

90-
readonly initialDiscoveryState = derived<AutomationInitialDiscoveryState>(this, reader => this._ready.read(reader) ? 'ready' : 'pending');
9190
readonly automations: IObservable<readonly IAutomationDescriptor[]>;
9291
readonly runs: IObservable<readonly IAutomationRun[]>;
9392
readonly catalogueState: IObservable<AutomationCatalogueState>;

‎src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,10 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide
264264
};
265265
bindConnection();
266266
this._register(this._agentHostService.onAgentHostStart(bindConnection));
267+
this._register(this._agentHostService.onAgentHostExit(() => {
268+
connectionListeners.clear();
269+
automations.clearConnection();
270+
}));
267271

268272
// Eagerly populate the session cache once authentication has settled.
269273
// Without this, the sidebar would only call `getSessions()` after some

‎src/vs/sessions/contrib/providers/agentHost/browser/reconnectableAgentHostAutomationStore.ts‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { IConfigurationService } from '../../../../../platform/configuration/com
1313
import { ILogService } from '../../../../../platform/log/common/log.js';
1414
import type { AutomationRunTrigger, IAutomationDescriptor, IAutomationRun } from '../../../../../workbench/contrib/chat/common/automations/automation.js';
1515
import { type AutomationCatalogueState, isAutomationActiveRunError, type AutomationMutationGuard, type IAutomationRunClaim, type ICreateAutomationOptions, type IGuardedAutomationUpdateResult, type IUpdateAutomationOptions, type IUpdateAutomationRunOptions } from '../../../../../workbench/contrib/chat/common/automations/automationService.js';
16-
import type { AutomationInitialDiscoveryState, IAutomation, IAutomationSnapshotImportResult, IGuardedAutomationSnapshotRemovalResult, ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js';
16+
import type { IAutomation, IAutomationSnapshotImportResult, IGuardedAutomationSnapshotRemovalResult, ISessionsProviderAutomations } from '../../../../services/sessions/common/sessionsProvider.js';
1717
import { AgentHostAutomationStore, type IAgentHostAutomationBoundaryMapper, type IAgentHostAutomationConnection } from './agentHostAutomationStore.js';
1818
import { CHAT_AUTOMATIONS_ENABLED_SETTING } from '../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js';
1919

@@ -35,20 +35,6 @@ export class ReconnectableAgentHostAutomationStore extends Disposable implements
3535
private readonly _authorityState = observableValue<AutomationAuthorityState>(this, { kind: 'disconnected' });
3636
private readonly _disposeCancellation = new CancellationTokenSource();
3737

38-
readonly initialDiscoveryState = derived<AutomationInitialDiscoveryState>(this, reader => {
39-
const state = this._authorityState.read(reader);
40-
switch (state.kind) {
41-
case 'supported':
42-
return state.store.initialDiscoveryState.read(reader);
43-
case 'unsupported':
44-
return 'ready';
45-
case 'initializing':
46-
return 'pending';
47-
case 'disconnected':
48-
case 'disabled':
49-
return 'unavailable';
50-
}
51-
});
5238
readonly automations = derived(this, reader => this._currentStore.read(reader)?.automations.read(reader) ?? this._legacySource?.automations.read(reader) ?? []);
5339
readonly runs = derived(this, reader => this._currentStore.read(reader)?.runs.read(reader) ?? this._legacySource?.runs.read(reader) ?? []);
5440
readonly catalogueState: IObservable<AutomationCatalogueState> = derived(this, reader => {

‎src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostAutomationStore.test.ts‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,19 +2214,16 @@ suite('AgentHostAutomationStore', () => {
22142214
configurationService,
22152215
));
22162216
store.setConnection(connection);
2217-
assert.strictEqual(store.initialDiscoveryState.get(), 'pending');
22182217
await assert.rejects(store.completeMigration(), /cannot be migrated safely/);
22192218

22202219
legacy.migrationAllowed = true;
22212220
store.setConnection(connection);
22222221
await store.completeMigration();
22232222

22242223
assert.deepStrictEqual({
2225-
initialDiscoveryState: store.initialDiscoveryState.get(),
22262224
subscriptions: connection.subscribedChannel,
22272225
completionRequests: connection.dispatched.filter(entry => entry.channel === ROOT_STATE_URI).length,
22282226
}, {
2229-
initialDiscoveryState: 'ready',
22302227
subscriptions: URI.parse(AUTOMATION_CATALOG_URI).toString(),
22312228
completionRequests: 1,
22322229
});
@@ -2416,7 +2413,6 @@ suite('AgentHostAutomationStore', () => {
24162413
configurationService,
24172414
));
24182415
store.setConnection(connection);
2419-
const pendingState = store.initialDiscoveryState.get();
24202416

24212417
let settled = false;
24222418
const migration = store.completeMigration().finally(() => settled = true);
@@ -2430,15 +2426,7 @@ suite('AgentHostAutomationStore', () => {
24302426
}, undefined);
24312427
await migration;
24322428

2433-
assert.deepStrictEqual({
2434-
pendingState,
2435-
readyState: store.initialDiscoveryState.get(),
2436-
completionRequests: connection.dispatched.filter(entry => entry.channel === ROOT_STATE_URI).length,
2437-
}, {
2438-
pendingState: 'pending',
2439-
readyState: 'ready',
2440-
completionRequests: 1,
2441-
});
2429+
assert.strictEqual(connection.dispatched.filter(entry => entry.channel === ROOT_STATE_URI).length, 1);
24422430
});
24432431

24442432
test('migration resolves without subscribing after an older host finishes initializing', async () => {
@@ -2463,7 +2451,6 @@ suite('AgentHostAutomationStore', () => {
24632451
));
24642452
store.setConnection(connection);
24652453
const migration = store.completeMigration();
2466-
const pendingState = store.initialDiscoveryState.get();
24672454

24682455
connection.initializeResult.set({
24692456
protocolVersion: '1',
@@ -2473,13 +2460,9 @@ suite('AgentHostAutomationStore', () => {
24732460
await migration;
24742461

24752462
assert.deepStrictEqual({
2476-
pendingState,
2477-
readyState: store.initialDiscoveryState.get(),
24782463
subscribedChannel: connection.subscribedChannel,
24792464
completionRequests: connection.dispatched.filter(entry => entry.channel === ROOT_STATE_URI).length,
24802465
}, {
2481-
pendingState: 'pending',
2482-
readyState: 'ready',
24832466
subscribedChannel: undefined,
24842467
completionRequests: 0,
24852468
});
@@ -2509,13 +2492,7 @@ suite('AgentHostAutomationStore', () => {
25092492

25102493
await store.completeMigration();
25112494

2512-
assert.deepStrictEqual({
2513-
initialDiscoveryState: store.initialDiscoveryState.get(),
2514-
dispatched: connection.dispatched.length,
2515-
}, {
2516-
initialDiscoveryState: 'pending',
2517-
dispatched: 0,
2518-
});
2495+
assert.strictEqual(connection.dispatched.length, 0);
25192496
}));
25202497

25212498
test('disposing while capabilities initialize settles migration', async () => {

‎src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { IWorkspaceTrustManagementService, IWorkspaceTrustRequestService, Resour
3838
import { IChatWidget, IChatWidgetService } from '../../../../../../workbench/contrib/chat/browser/chat.js';
3939
import { IChatService, type ChatSendResult, type IChatModelReference, type IChatSendRequestOptions } from '../../../../../../workbench/contrib/chat/common/chatService/chatService.js';
4040
import { IChatSessionsService, isIChatSessionFileChange2 } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js';
41+
import { CHAT_AUTOMATIONS_ENABLED_SETTING } from '../../../../../../workbench/contrib/chat/common/automations/automationsEnabled.js';
4142
import { ChatModeKind } from '../../../../../../workbench/contrib/chat/common/constants.js';
4243
import { ILanguageModelsService, type ILanguageModelChatMetadata } from '../../../../../../workbench/contrib/chat/common/languageModels.js';
4344
import type { IChatModel, IChatModelInputState, IInputModel } from '../../../../../../workbench/contrib/chat/common/model/chatModel.js';
@@ -88,6 +89,8 @@ class MockAgentHostService extends mock<IAgentHostService>() {
8889
override get rootState(): IAgentSubscription<RootState> { return this._rootStateSubscription; }
8990
private readonly _onAgentHostStart = new Emitter<void>();
9091
override readonly onAgentHostStart = this._onAgentHostStart.event;
92+
private readonly _onAgentHostExit = new Emitter<number>();
93+
override readonly onAgentHostExit = this._onAgentHostExit.event;
9194
override readonly initializeResult = constObservable({
9295
protocolVersion: '1',
9396
serverSeq: 0,
@@ -381,6 +384,10 @@ class MockAgentHostService extends mock<IAgentHostService>() {
381384
this._onAgentHostStart.fire();
382385
}
383386

387+
fireAgentHostExit(): void {
388+
this._onAgentHostExit.fire(0);
389+
}
390+
384391
setRootStateError(): void {
385392
const error = new Error('root state failed');
386393
this._rootStateValue = error;
@@ -401,6 +408,7 @@ class MockAgentHostService extends mock<IAgentHostService>() {
401408
this._onDidRootStateChange.dispose();
402409
this._onDidRootStateError.dispose();
403410
this._onAgentHostStart.dispose();
411+
this._onAgentHostExit.dispose();
404412
for (const emitter of this._sessionStateEmitters.values()) {
405413
emitter.dispose();
406414
}
@@ -672,6 +680,27 @@ suite('LocalAgentHostSessionsProvider', () => {
672680

673681
// ---- Provider identity -------
674682

683+
test('Automation discovery follows local Agent Host connection lifetime', () => {
684+
const provider = createProvider(disposables, agentHost, undefined, {
685+
configurationService: new TestConfigurationService({ [CHAT_AUTOMATIONS_ENABLED_SETTING]: true }),
686+
});
687+
const initial = provider.automations.initialDiscoveryState?.get();
688+
689+
agentHost.fireAgentHostExit();
690+
const disconnected = provider.automations.initialDiscoveryState?.get();
691+
agentHost.fireAgentHostStart();
692+
693+
assert.deepStrictEqual({
694+
initial,
695+
disconnected,
696+
reconnected: provider.automations.initialDiscoveryState?.get(),
697+
}, {
698+
initial: 'ready',
699+
disconnected: 'unavailable',
700+
reconnected: 'ready',
701+
});
702+
});
703+
675704
test('has correct id, label, and sessionType from rootState agents', () => {
676705
const provider = createProvider(disposables, agentHost);
677706

‎src/vs/sessions/contrib/sessions/browser/automationsNewBadge.ts‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Disposable, MutableDisposable } from '../../../../base/common/lifecycle.js';
7-
import { autorun, derived, IObservable, observableSignalFromEvent, observableValue } from '../../../../base/common/observable.js';
7+
import { autorun, derived, observableValue } from '../../../../base/common/observable.js';
88
import { onUnexpectedError } from '../../../../base/common/errors.js';
99
import { IConfigurationService, isConfigured } from '../../../../platform/configuration/common/configuration.js';
1010
import { ILogService } from '../../../../platform/log/common/log.js';
@@ -14,7 +14,6 @@ import { IAutomationService } from '../../../../workbench/contrib/chat/common/au
1414
import { IWorkbenchAssignmentService } from '../../../../workbench/services/assignment/common/assignmentService.js';
1515
import { ILifecycleService, LifecyclePhase } from '../../../../workbench/services/lifecycle/common/lifecycle.js';
1616
import { ICustomViewService } from '../../../services/customView/browser/customViewService.js';
17-
import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js';
1817
import { ISessionsWindowUsageService } from '../../../services/sessions/browser/sessionsWindowUsageService.js';
1918
import { AUTOMATIONS_CUSTOM_VIEW_ID } from './automationsConstants.js';
2019

@@ -43,7 +42,6 @@ export class AutomationsNewBadgeState extends Disposable {
4342
private readonly forcePreview = observableValue(this, false);
4443
private readonly startupDecision = observableValue<AutomationsNewBadgeStartupDecision>(this, 'pending');
4544
private readonly automationEvidenceObserver = this._register(new MutableDisposable());
46-
private readonly providersChanged: IObservable<void>;
4745
private observersRegistered = false;
4846
private initializationPromise: Promise<void> | undefined;
4947
private styleRequest = 0;
@@ -66,12 +64,10 @@ export class AutomationsNewBadgeState extends Disposable {
6664
@IConfigurationService private readonly configurationService: IConfigurationService,
6765
@ILogService private readonly logService: ILogService,
6866
@ISessionsWindowUsageService private readonly sessionsWindowUsageService: ISessionsWindowUsageService,
69-
@ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService,
7067
@ILifecycleService private readonly lifecycleService: ILifecycleService,
7168
) {
7269
super();
7370
this.seen = this._register(automationsNewBadgeSeenMemento(StorageScope.APPLICATION, StorageTarget.MACHINE, storageService));
74-
this.providersChanged = observableSignalFromEvent(this, sessionsProvidersService.onDidChangeProviders);
7571
}
7672

7773
initialize(): Promise<void> {
@@ -100,13 +96,8 @@ export class AutomationsNewBadgeState extends Disposable {
10096
if (this.forcePreview.read(reader) || this.seen.read(reader) || this.startupDecision.read(reader) !== 'eligible') {
10197
return;
10298
}
103-
this.providersChanged.read(reader);
104-
for (const provider of this.sessionsProvidersService.getProviders()) {
105-
if (provider.automations
106-
&& (provider.automations.initialDiscoveryState?.read(reader) ?? 'ready') !== 'ready') {
107-
this.startupDecision.set('suppressed', undefined);
108-
return;
109-
}
99+
if (this.automationService.initialDiscoveryState.read(reader) !== 'ready') {
100+
this.startupDecision.set('suppressed', undefined);
110101
}
111102
}));
112103
}
@@ -133,7 +124,7 @@ export class AutomationsNewBadgeState extends Disposable {
133124
}
134125

135126
private async doInitialize(): Promise<void> {
136-
if (this.seen.get() || this.forcePreview.get()) {
127+
if (this._store.isDisposed || this.seen.get() || this.forcePreview.get()) {
137128
return;
138129
}
139130

@@ -143,14 +134,11 @@ export class AutomationsNewBadgeState extends Disposable {
143134
}
144135

145136
await this.lifecycleService.when(LifecyclePhase.Eventually);
146-
if (this.seen.get() || this.forcePreview.get()) {
137+
if (this._store.isDisposed || this.seen.get() || this.forcePreview.get()) {
147138
return;
148139
}
149140

150-
const providersReady = this.sessionsProvidersService.getProviders()
151-
.every(provider => !provider.automations
152-
|| (provider.automations.initialDiscoveryState?.get() ?? 'ready') === 'ready');
153-
if (!providersReady) {
141+
if (this.automationService.initialDiscoveryState.get() !== 'ready') {
154142
this.startupDecision.set('suppressed', undefined);
155143
return;
156144
}
@@ -187,7 +175,7 @@ export class AutomationsNewBadgeState extends Disposable {
187175
}
188176

189177
private canResolveStyle(): boolean {
190-
return !this.seen.get() && (this.forcePreview.get() || this.startupDecision.get() === 'eligible');
178+
return !this._store.isDisposed && !this.seen.get() && (this.forcePreview.get() || this.startupDecision.get() === 'eligible');
191179
}
192180

193181
private normalizeStyle(value: string | undefined): AutomationsNewBadgeStyle {

0 commit comments

Comments
 (0)