Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ function createTestCustomAgentsService(connection: MockAgentConnection, rootCust
return [...rootCustomizations, ...(sessionState.customizations ?? [])];
},
getFolderPickerDecision: () => undefined,
whenCustomizationsReady: () => Promise.resolve(),
getWorkingDirectory(sessionResource: URI): string | undefined {
return undefined;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,11 @@ export class AgentCustomizationItemProvider extends Disposable implements ICusto
};
}

async provideSourceFolders(sessionResource: URI, type: PromptsType, _token: CancellationToken): Promise<readonly ICustomizationSourceFolder[]> {
async provideSourceFolders(sessionResource: URI, type: PromptsType, token: CancellationToken): Promise<readonly ICustomizationSourceFolder[]> {
// One-shot callers (the migration hint) must not read the empty
// placeholder a still-loading session reports, or they conclude there is
// nothing to migrate.
await this._customAgentsService.whenCustomizationsReady(sessionResource, token);
Comment thread
aeschli marked this conversation as resolved.
const workingDirectories = this._customAgentsService.getWorkingDirectories(sessionResource);

const folders: ICustomizationSourceFolder[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
*--------------------------------------------------------------------------------------------*/

import { URI } from '../../../../../../base/common/uri.js';
import { raceCancellation, raceTimeout } from '../../../../../../base/common/async.js';
import { CancellationToken } from '../../../../../../base/common/cancellation.js';
import { Emitter, Event } from '../../../../../../base/common/event.js';
import { StringSHA1 } from '../../../../../../base/common/hash.js';
import { Disposable, DisposableResourceMap, IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js';
import { Disposable, DisposableResourceMap, DisposableStore, IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js';
import { ResourceSet } from '../../../../../../base/common/map.js';
import { AgentHostMcpServers, AgentHostMcpServersConfigKey } from '../../../../../../platform/agentHost/common/agentHostSchema.js';
import { IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js';
Expand Down Expand Up @@ -42,6 +44,18 @@ export interface IAgentHostCustomizationService {

getCustomizations(sessionResource: URI): readonly Customization[];

/**
* Resolves once {@link getCustomizations} reflects a real snapshot for
* `sessionResource` rather than the empty placeholder returned while the
* session state is still loading. Resolves immediately when the session is
* not backed by an agent host, or when a snapshot already arrived.
*
* Reactive callers should keep reading synchronously and re-render on
* {@link onDidChangeCustomizations}; this exists for one-shot callers that
* would otherwise mistake "not loaded yet" for "no customizations".
*/
whenCustomizationsReady(sessionResource: URI, token?: CancellationToken): Promise<void>;
Comment thread
Copilot marked this conversation as resolved.

/**
* The harness-owned decision about the multi-root Folder picker for a
* session (or `undefined` when the provider expressed no opinion). Read from
Expand Down Expand Up @@ -108,6 +122,9 @@ export class NullAgentHostCustomizationService implements IAgentHostCustomizatio
getCustomizations(_sessionResource: URI): readonly Customization[] {
return [];
}
whenCustomizationsReady(_sessionResource: URI, _token?: CancellationToken): Promise<void> {
return Promise.resolve();
}
getFolderPickerDecision(_sessionResource: URI): ISessionFolderPickerDecision | undefined {
return undefined;
}
Expand Down Expand Up @@ -185,6 +202,14 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i
return this._resolveTarget(sessionResource)?.customizations ?? [];
}

/**
* Targets resolved by this base are backed by already-materialized provider
* state, so a snapshot is available as soon as the target resolves.
*/
whenCustomizationsReady(_sessionResource: URI, _token?: CancellationToken): Promise<void> {
return Promise.resolve();
}

getFolderPickerDecision(sessionResource: URI): ISessionFolderPickerDecision | undefined {
return this._resolveTarget(sessionResource)?.folderPickerDecision;
}
Expand Down Expand Up @@ -434,6 +459,12 @@ export function getPresentableMcpServerCustomizations(customizations: readonly C
return entries.filter(entry => entry.isTopLevel || !topLevelNames.has(entry.server.name));
}

/**
* Upper bound on how long {@link WorkbenchAgentHostCustomizationService.whenCustomizationsReady}
* waits for a session's first state snapshot.
*/
const SESSION_STATE_SNAPSHOT_TIMEOUT_MS = 2000;

export class WorkbenchAgentHostCustomizationService extends AbstractAgentHostCustomizationService {

private readonly _sessionStateSubscriptions = this._register(new DisposableResourceMap<IDisposable & { readonly connection: IAgentConnection; readonly backendSession: URI; readonly sub: IAgentSubscription<SessionState> }>());
Expand Down Expand Up @@ -528,6 +559,39 @@ export class WorkbenchAgentHostCustomizationService extends AbstractAgentHostCus
};
}

/**
* Session state arrives asynchronously over the protocol, so a freshly
* created subscription reports `undefined` until its first snapshot lands.
* Waiting is bounded because the chat request path blocks on this before
* sending the user's message: on timeout the caller falls back to the
* current (possibly empty) snapshot rather than stalling the send.
*/
override async whenCustomizationsReady(sessionResource: URI, token: CancellationToken = CancellationToken.None): Promise<void> {
const target = this._resolveSessionTarget(sessionResource);
if (!target) {
return;
}
const subscription = this._ensureSessionStateSubscription(sessionResource, target)?.sub;
// An `Error` value counts as resolved: the subscription settled, just not with a snapshot.
if (!subscription || subscription.value !== undefined) {
return;
}

const store = new DisposableStore();
try {
const firstSnapshot = new Promise<void>(resolve => {
store.add(subscription.onDidChange(() => resolve()));
const onDidError = subscription.onDidError;
if (onDidError) {
store.add(onDidError(() => resolve()));
}
});
await raceTimeout(raceCancellation(firstSnapshot, token), SESSION_STATE_SNAPSHOT_TIMEOUT_MS);
} finally {
store.dispose();
}
}

private _readSessionState(sessionResource: URI): SessionState | undefined {
const target = this._resolveSessionTarget(sessionResource);
const subscription = target ? this._ensureSessionStateSubscription(sessionResource, target)?.sub : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import assert from 'assert';
import { Event } from '../../../../../../base/common/event.js';
import { timeout } from '../../../../../../base/common/async.js';
import { Emitter, Event } from '../../../../../../base/common/event.js';
import { IReference } from '../../../../../../base/common/lifecycle.js';
import { ResourceMap } from '../../../../../../base/common/map.js';
import { URI } from '../../../../../../base/common/uri.js';
Expand Down Expand Up @@ -390,4 +391,147 @@ suite('WorkbenchAgentHostCustomizationService', () => {
afterError: [retainedRoot.toString()],
});
});

/**
* A subscription whose snapshot arrives after the fact, so tests can observe
* the window in which `value` is still `undefined`.
*/
class LiveSessionSubscription extends mock<IAgentSubscription<SessionState>>() {
private readonly _onDidChange = new Emitter<SessionState>();
override readonly onDidChange = this._onDidChange.event;
private readonly _onDidError = new Emitter<Error>();
override readonly onDidError = this._onDidError.event;
private current: SessionState | Error | undefined;
private confirmed: SessionState | undefined;

override get value(): SessionState | Error | undefined {
return this.current;
}

override get verifiedValue(): SessionState | undefined {
return this.confirmed;
}

setSnapshot(state: SessionState): void {
this.current = state;
this.confirmed = state;
this._onDidChange.fire(state);
}

setError(error: Error): void {
this.current = error;
this._onDidError.fire(error);
}

dispose(): void {
this._onDidChange.dispose();
this._onDidError.dispose();
}
}

function createReadinessSut() {
const sessionResource = URI.parse('untitled:chat');
const backendSession = URI.parse('copilot:/session');
const subscription = store.add(new LiveSessionSubscription());
const connection = new class extends mock<IAgentConnection>() {
override readonly resourceUris = identityAgentHostResourceUriMapper;
override readonly onDidAction = Event.None;
override readonly rootState = {
value: undefined,
verifiedValue: undefined,
onDidChange: Event.None,
onWillApplyAction: Event.None,
onDidApplyAction: Event.None,
} satisfies IAgentSubscription<RootState>;

override getSubscription<T>(_kind: StateComponents): IReference<IAgentSubscription<T>> {
return {
object: subscription as unknown as IAgentSubscription<T>,
dispose: () => { },
};
}
}();
const instantiationService = store.add(new TestInstantiationService());
instantiationService.stub(ILoggerService, store.add(new NullLoggerService()));
instantiationService.stub(IOutputService, {
getChannel: () => undefined,
getChannelDescriptor: () => undefined,
showChannel: async () => { },
});
const service = store.add(new WorkbenchAgentHostCustomizationService(
new class extends mock<IAgentHostConnectionsService>() {
override readonly ambientConnection = connection;
}(),
new class extends mock<IAgentHostUntitledProvisionalSessionService>() {
override readonly onDidChange = Event.None;
override get(): URI {
return backendSession;
}
override getProvisionalWorkingDirectories(): readonly URI[] {
return [];
}
}(),
instantiationService,
new NullLogService(),
new class extends mock<IChatService>() {
override readonly onDidDisposeSession = Event.None;
}(),
new class extends mock<IAgentHostActiveClientService>() { }(),
));
const directory: Customization = {
type: CustomizationType.Directory,
id: 'dir-1',
uri: 'file:///workspace/.github/skills',
name: 'skills',
contents: CustomizationType.Skill,
writable: true,
children: [],
} as unknown as Customization;
const stateWithDirectory: SessionState = {
...createSessionState({
resource: backendSession.toString(),
provider: 'copilot',
title: 'Session',
status: SessionStatus.Idle,
createdAt: new Date(0).toISOString(),
modifiedAt: new Date(0).toISOString(),
}),
customizations: [directory],
};
return { service, subscription, sessionResource, stateWithDirectory };
}

test('whenCustomizationsReady defers until the first snapshot rather than reporting no customizations', async () => {
const { service, subscription, sessionResource, stateWithDirectory } = createReadinessSut();

let resolved = false;
const ready = service.whenCustomizationsReady(sessionResource).then(() => { resolved = true; });
await timeout(0);
const whileLoading = { resolved, customizations: service.getCustomizations(sessionResource).map(c => c.id) };

subscription.setSnapshot(stateWithDirectory);
await ready;
const afterSnapshot = { resolved, customizations: service.getCustomizations(sessionResource).map(c => c.id) };

let resolvedAgain = false;
service.whenCustomizationsReady(sessionResource).then(() => { resolvedAgain = true; });
await timeout(0);

assert.deepStrictEqual({ whileLoading, afterSnapshot, resolvedAgain }, {
whileLoading: { resolved: false, customizations: [] },
afterSnapshot: { resolved: true, customizations: ['dir-1'] },
resolvedAgain: true,
});
});

test('whenCustomizationsReady stops waiting when the subscription fails', async () => {
const { service, subscription, sessionResource } = createReadinessSut();

let resolved = false;
const ready = service.whenCustomizationsReady(sessionResource).then(() => { resolved = true; });
subscription.setError(new Error('subscription failed'));
await ready;

assert.strictEqual(resolved, true);
});
});
Loading