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 @@ -10,6 +10,13 @@
* A remote host describes the pending decision (run a command, read a file, …)
* but does not stamp the `_meta.toolKind` rendering hint local agent adapters
* provide, so the kind is recovered from here instead.
*
* Compatibility bridge, not the durable contract: `promptRequest` and
* `permissionRequest` are raw Copilot runtime payloads, not protocol fields, so
* their shape can change without a version bump and every client has to learn
* Copilot internals to render an approval. Delete this file, and its use in
* `getToolKind`, once the minimum supported host describes confirmations
* natively.
*/

interface IHasPermissionRequestMeta {
Expand Down
8 changes: 6 additions & 2 deletions src/vs/platform/agentHost/common/state/sessionReducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ const PERMISSION_REQUEST_TOOL_KINDS: Readonly<Partial<Record<AgentPermissionRequ
*
* Normally the `_meta.toolKind` flag an agent adapter injects (e.g.
* `copilotEventMapper`); it is not part of the protocol. A remote agent host
* does not stamp that key, so for a call awaiting approval the kind comes from
* the permission request it echoes instead.
* does not stamp that key, so for a call awaiting approval the kind falls back
* to the permission request it echoes — the compatibility bridge documented in
* {@link readAgentPermissionRequestMeta}.
*
* A stamped kind always wins, so a host that starts describing its
* confirmations natively is never overridden by the fallback.
*/
export function getToolKind(tc: ToolCallState | ICompletedToolCall): ToolKind | undefined {
const kind = readToolCallMeta(tc).toolKind;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { localize } from '../../../../../nls.js';
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
import { ILabelService } from '../../../../../platform/label/common/label.js';
import { ILogService } from '../../../../../platform/log/common/log.js';
import { INotificationService } from '../../../../../platform/notification/common/notification.js';
import { SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
import { IGitHubService } from '../../../github/browser/githubService.js';
Expand Down Expand Up @@ -1404,7 +1405,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
* How long the first sandbox turn waits for the session's model catalog to arrive before
* dispatching without the user's model. Long enough to cover the gap between the relay
* connecting and the host publishing its models, short enough not to strand a send behind a
* catalog that is never coming.
* catalog that is never coming. Exceeding it is reported to the user, not only logged.
*/
private static readonly SANDBOX_MODEL_WAIT_MS = 5_000;

Expand Down Expand Up @@ -1507,6 +1508,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
@IConfigurationService private readonly configurationService: IConfigurationService,
@IAgentHostEnablementService private readonly agentHostEnablementService: IAgentHostEnablementService,
@ILogService private readonly logService: ILogService,
@INotificationService private readonly notificationService: INotificationService,
@IGitHubService private readonly gitHubService: IGitHubService,
@IPullRequestIconCache private readonly pullRequestIconCache: IPullRequestIconCache,
@ILabelService private readonly labelService: ILabelService,
Expand Down Expand Up @@ -2139,6 +2141,11 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
return getWorkbenchContribution<CloudSandboxAgentHostContribution>(CloudSandboxAgentHostContribution.ID);
}

/** Test seam: overridden so a test can reach the timeout without waiting out the real budget. */
protected get _sandboxModelWaitMs(): number {
return CopilotChatSessionsProvider.SANDBOX_MODEL_WAIT_MS;
}

/**
* Commit a cloud new-session into a GitHub-managed sandbox instead of the server-run cloud
* agent: provision the sandbox, then hand the session over to the remote-agent-host provider
Expand All @@ -2160,7 +2167,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
let provisioned: ICloudSandboxProvisionedSession | undefined;
// Read before provisioning: the composer session is retired below, and its selection is the
// only record of what the user picked for this turn.
const selectedRawModelId = this._rawCloudModelId(session);
const selectedModel = this._selectedCloudModel(session);
try {
provisioned = await this._getCloudSandboxContribution().provisionSession({
repoNwo,
Expand All @@ -2171,7 +2178,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
// Send into the session's main chat rather than `createNewChat`, which would mint an
// *additional* peer chat inside a session that already has one.
const chat = provisioned.session.mainChat.get();
await this._carryModelToSandbox(provisioned, chat.resource, selectedRawModelId);
await this._carryModelToSandbox(provisioned, chat.resource, selectedModel);
const committed = await provisioned.provider.sendRequest(provisioned.session.sessionId, chat.resource, options);

// Retire only once the turn is dispatched; swapping earlier bounces the view home.
Expand Down Expand Up @@ -2200,40 +2207,46 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
}

/**
* The backend model id behind this composer's selection, as the sandbox knows it.
* The composer's model selection as the sandbox knows it, plus the label to name it by.
*
* Cloud sessions pick from the extension host's `models` option group, whose ids are the
* group's own item ids, while a sandbox registers its models from what the agent host
* advertises. The two are different id spaces, so only the underlying model id crosses over.
* advertises. Different id spaces, so only the underlying model id crosses over.
*
* Only the model, because only the model exists: an option item's `modelMetadata` is hover and
* pricing detail with no configuration schema, so a cloud composer never offers a thinking
* level or context tier to carry alongside it.
*/
private _rawCloudModelId(session: RemoteNewSession): string | undefined {
private _selectedCloudModel(session: RemoteNewSession): { readonly rawModelId: string; readonly label: string } | undefined {
const selectedModelId = session.selectedModelId;
if (!selectedModelId) {
return undefined;
}
const { modelOption } = session.getModelOptionsSnapshot();
const item = modelOption?.group.items.find(i => i.id === selectedModelId);
return item?.modelMetadata?.id ?? item?.id ?? selectedModelId;
const rawModelId = item?.modelMetadata?.id ?? item?.id ?? selectedModelId;
return { rawModelId, label: item?.modelMetadata?.name ?? item?.name ?? rawModelId };
}

/**
* Apply the model the user picked in the composer to the sandbox session before its first turn.
*
* Mission Control starts no run, so this client sends that turn — and a session that has never
* run has no model of its own to restore. Without this the turn carries no model at all and
* silently runs on whatever the agent host defaults to, discarding the user's pick along with
* the thinking level and context tier configured against it.
* runs on whatever the agent host defaults to.
*
* A freshly connected sandbox publishes its models asynchronously, so an empty catalog here is
* "not yet" rather than "no": the model resolution is awaited while it reports `pending`, which
* is the wait {@link ISessionsProvider.getModelsSnapshot} documents. Bounded, because the turn
* cannot be held indefinitely — on timeout, or a model the sandbox genuinely does not offer,
* the host chooses, which is the behavior this had before.
* A freshly connected sandbox publishes its models asynchronously, so an empty catalog is "not
* yet" rather than "no": resolution is awaited while it reports `pending`, bounded because the
* turn cannot be held indefinitely.
*
* Every path that gives up tells the user: an absent `Message.model` means "host decides", so
* nothing downstream would report running at a capability and price they did not choose.
*/
private async _carryModelToSandbox(provisioned: ICloudSandboxProvisionedSession, chatResource: URI, rawModelId: string | undefined): Promise<void> {
if (!rawModelId) {
private async _carryModelToSandbox(provisioned: ICloudSandboxProvisionedSession, chatResource: URI, selected: { readonly rawModelId: string; readonly label: string } | undefined): Promise<void> {
if (!selected) {
return;
}
const { rawModelId, label } = selected;
const sessionId = provisioned.session.sessionId;
const provider = provisioned.provider;

Expand All @@ -2242,13 +2255,14 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
const modelTarget = provider.getModelsSnapshot(sessionId).modelTarget;
if (!modelTarget) {
this.logService.info(`[CopilotChatSessionsProvider] Sandbox session ${sessionId} reported no model target; letting the agent host choose.`);
this._notifySandboxModelNotApplied(label);
return;
}
const desiredModelId = `${modelTarget}:${rawModelId}`;

const store = new DisposableStore();
try {
const deadline = Date.now() + CopilotChatSessionsProvider.SANDBOX_MODEL_WAIT_MS;
const deadline = Date.now() + this._sandboxModelWaitMs;
for (; ;) {
const resolution = provider.getModelsSnapshot(sessionId, desiredModelId).desiredModelResolution;
if (resolution.kind === 'available') {
Expand All @@ -2257,6 +2271,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
}
if (resolution.kind !== 'pending') {
this.logService.info(`[CopilotChatSessionsProvider] Sandbox session ${sessionId} does not advertise model '${rawModelId}'; letting the agent host choose.`);
this._notifySandboxModelNotApplied(label);
return;
}
const remaining = deadline - Date.now();
Expand All @@ -2267,6 +2282,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
: undefined;
if (!published) {
this.logService.warn(`[CopilotChatSessionsProvider] Sandbox session ${sessionId} had not published model '${rawModelId}' in time; letting the agent host choose.`);
this._notifySandboxModelNotApplied(label);
return;
}
}
Expand All @@ -2275,6 +2291,11 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
}
}

/** Name the model so the substitution is attributable. A warning: the turn still runs. */
private _notifySandboxModelNotApplied(label: string): void {
this.notificationService.warn(localize('sandboxModelNotApplied', "Couldn't use {0} for this session. The agent's default model was used instead.", label));
Comment thread
osortega marked this conversation as resolved.
Outdated
}

/** Retire the optimistic placeholder in favour of the session that now exists. */
private _retirePlaceholder(session: RemoteNewSession, placeholder: ISession, committed: ISession): void {
this._sessionCache.delete(session.resource.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { CloudSandboxSessionsProvider } from '../../../remoteAgentHost/browser/c
import { ChatConfiguration, ChatPermissionLevel } from '../../../../../../workbench/contrib/chat/common/constants.js';
import { CopilotChatSessionsProvider, COPILOT_PROVIDER_ID, CopilotCloudSessionType, ICopilotChatSession } from '../../browser/copilotChatSessionsProvider.js';
import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js';
import { INotificationService } from '../../../../../../platform/notification/common/notification.js';
import { ILabelService } from '../../../../../../platform/label/common/label.js';
import { IPathService } from '../../../../../../workbench/services/path/common/pathService.js';
import { MockLabelService } from '../../../../../../workbench/services/label/test/common/mockLabelService.js';
Expand Down Expand Up @@ -314,6 +315,9 @@ function createProviderWithConfig(
onDidChangeFocusedSession: Event.None,
});
instantiationService.stub(ILanguageModelsService, opts?.languageModelsService ?? { lookupLanguageModel: () => undefined });
instantiationService.stub(INotificationService, new class extends mock<INotificationService>() {
override warn(): void { }
}());
instantiationService.stub(ILanguageModelToolsService, {
toToolReferences: () => [],
});
Expand Down Expand Up @@ -348,19 +352,26 @@ function createProviderWithConfig(
class TestSandboxCopilotProvider extends CopilotChatSessionsProvider {
sandboxContribution: Pick<CloudSandboxAgentHostContribution, 'provisionSession'> | undefined;

/** Only the timeout test lowers this; the rest keep the real budget so they cannot race it. */
sandboxModelWaitMs: number | undefined;

protected override _getCloudSandboxContribution(): Pick<CloudSandboxAgentHostContribution, 'provisionSession'> {
if (!this.sandboxContribution) {
throw new Error('No cloud sandbox contribution was registered');
}
return this.sandboxContribution;
}

protected override get _sandboxModelWaitMs(): number {
return this.sandboxModelWaitMs ?? super._sandboxModelWaitMs;
}
}

function createProviderForSendTests(
disposables: DisposableStore,
model: MockAgentSessionsModel,
sendRequest: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise<ChatSendResult>,
opts?: { onDidCommitSession?: Event<{ original: URI; committed: URI }>; configurationService?: TestConfigurationService; agentHostEnabled?: boolean; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined },
opts?: { onDidCommitSession?: Event<{ original: URI; committed: URI }>; configurationService?: TestConfigurationService; agentHostEnabled?: boolean; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; notifications?: string[] },
): TestSandboxCopilotProvider {
const instantiationService = disposables.add(new TestInstantiationService());

Expand Down Expand Up @@ -406,6 +417,9 @@ function createProviderForSendTests(
onDidChangeFocusedSession: Event.None,
});
instantiationService.stub(ILanguageModelsService, { lookupLanguageModel: () => undefined });
instantiationService.stub(INotificationService, new class extends mock<INotificationService>() {
override warn(message: unknown): void { opts?.notifications?.push(String(message)); }
}());
instantiationService.stub(ILanguageModelToolsService, { toToolReferences: () => [] });
instantiationService.stub(IGitService, { openRepository: async () => undefined });
instantiationService.stub(IInstantiationService, instantiationService);
Expand Down Expand Up @@ -1964,11 +1978,12 @@ suite('CopilotChatSessionsProvider', () => {
configurationService.setUserConfiguration(RemoteAgentHostsEnabledSettingId, true);

const cloudSends: string[] = [];
const notifications: string[] = [];
const provider = createProviderForSendTests(disposables, model, async (_resource, message) => {
cloudSends.push(message);
// Never settles: these tests only assert which path the send took.
return new Promise<ChatSendResult>(() => { });
}, { configurationService, getOptionGroups: opts.getOptionGroups });
}, { configurationService, getOptionGroups: opts.getOptionGroups, notifications });

const provisionRequests: ICloudSandboxCreateSessionRequest[] = [];
provider.sandboxContribution = {
Expand All @@ -1980,7 +1995,7 @@ suite('CopilotChatSessionsProvider', () => {
throw new Error('provisioning failed');
},
};
return { provider, provisionRequests, cloudSends };
return { provider, provisionRequests, cloudSends, notifications };
}

/**
Expand Down Expand Up @@ -2058,7 +2073,7 @@ suite('CopilotChatSessionsProvider', () => {
// Mission Control starts no run, so a session that has never run has no model to
// restore: without this the first turn would silently take the agent host default.
const provisioned = provisionedSession(undefined, () => [sandboxModel('claude-sonnet-4.6')]);
const { provider } = createSandboxProvider({
const { provider, notifications } = createSandboxProvider({
provision: async () => provisioned,
getOptionGroups: () => cloudModelOptionGroup('synthetic-cloud-model', 'claude-sonnet-4.6'),
});
Expand All @@ -2070,8 +2085,12 @@ suite('CopilotChatSessionsProvider', () => {
await provider.sendRequest(sessionInfo.sessionId, session.mainChat.get().resource, { query: 'fix it' });

// The id crosses id spaces by backend model id, and arrives as carried over: the user
// picked it for the composer, not for the session that replaced it.
assert.deepStrictEqual(provisioned.modelSelections, [{ modelId: 'agent-host-copilot:claude-sonnet-4.6', source: ChatModelSource.CarriedOver }]);
// picked it for the composer, not for the session that replaced it. Applying the pick
// is the silent case — nothing to tell the user about.
assert.deepStrictEqual(
{ selections: provisioned.modelSelections, notifications },
{ selections: [{ modelId: 'agent-host-copilot:claude-sonnet-4.6', source: ChatModelSource.CarriedOver }], notifications: [] }
);
});

test('waits for a sandbox catalog that is still arriving rather than sending without the model', async () => {
Expand Down Expand Up @@ -2102,11 +2121,12 @@ suite('CopilotChatSessionsProvider', () => {
);
});

test('leaves the model to the agent host when the sandbox does not advertise it', async () => {
// Sending an unroutable id would fail the turn outright, so an unmatched pick keeps
// the previous behavior of letting the host choose.
test('tells the user when the sandbox does not advertise the model they picked', async () => {
// Sending an unroutable id would fail the turn outright, so an unmatched pick still
// lets the host choose — but an absent `Message.model` means "host decides", so
// nothing else would report the substitution.
const provisioned = provisionedSession(undefined, () => [sandboxModel('gpt-5')]);
const { provider } = createSandboxProvider({
const { provider, notifications } = createSandboxProvider({
provision: async () => provisioned,
getOptionGroups: () => cloudModelOptionGroup('synthetic-cloud-model', 'claude-sonnet-4.6'),
});
Expand All @@ -2117,7 +2137,32 @@ suite('CopilotChatSessionsProvider', () => {

await provider.sendRequest(sessionInfo.sessionId, session.mainChat.get().resource, { query: 'fix it' });

assert.deepStrictEqual(provisioned.modelSelections, []);
assert.deepStrictEqual(
{ selections: provisioned.modelSelections, notified: notifications.length, namesModel: notifications[0]?.includes('claude-sonnet-4.6') },
{ selections: [], notified: 1, namesModel: true }
);
});

test('tells the user when the catalog never arrives before the turn is dispatched', async () => {
// The likeliest fallback in practice is a slow sandbox rather than a missing model, so
// the timeout has to be as visible as a conclusive miss.
const provisioned = provisionedSession(undefined, () => []);
const { provider, notifications } = createSandboxProvider({
provision: async () => provisioned,
getOptionGroups: () => cloudModelOptionGroup('synthetic-cloud-model', 'claude-sonnet-4.6'),
});
provider.sandboxModelWaitMs = 1;
const sessionInfo = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id);
const session = provider.getSession(sessionInfo.sessionId)!;
session.setUseSandbox(true);
provider.setModel(sessionInfo.sessionId, session.mainChat.get().resource, 'synthetic-cloud-model', ChatModelSource.Chosen);

await provider.sendRequest(sessionInfo.sessionId, session.mainChat.get().resource, { query: 'fix it' });

assert.deepStrictEqual(
{ selections: provisioned.modelSelections, notified: notifications.length, namesModel: notifications[0]?.includes('claude-sonnet-4.6') },
{ selections: [], notified: 1, namesModel: true }
);
});

test('provisions a sandbox and replaces the draft with the committed session', async () => {
Expand Down
Loading
Loading