Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions .github/skills/sessions/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ Then read the relevant spec for the area you are changing (see table below). If
- **Collapsing distinct provider identities in pickers**: Do not collapse extension-backed chat session ids (e.g. `copilotcli`) and agent-host ids (e.g. `agent-host-copilotcli`) based only on friendly names or well-known provider enums. They can coexist in the Agents window and route to different infrastructure; keep the exact session type id through selection/delegation and hide ambiguous legacy targets when an agent-host target supersedes them.
- **Resolving a session's provider via the create-only tracking map**: On the agent host, resolve the owning provider for any per-session operation (createChat, disposeChat, sendMessage, …) through `AgentService._findProviderForSession`, never the raw `_sessionToProvider` map. That map is populated only by `createSession`, so a **restored** session (alive in the state manager after a host restart but never created in this process) is absent from it — a direct lookup throws `no provider for session` and silently breaks the feature (e.g. Add Chat did nothing for restored sessions while messaging worked, because messaging already used the fallback). `_findProviderForSession` falls back to the session URI's scheme provider, which is what makes restored sessions work.
- **Dispatching per-chat side-channel actions (agent/model) to the session URI**: An agent-host session can own multiple peer chats, each with its own backend conversation (`CopilotAgent._chatSessions`). Conversation side-channel actions like `SessionAgentChanged`/`SessionModelChanged` must be dispatched to the per-chat **turn channel** (`_resolveTurnDispatchChannel`, which carries a `chatId` fragment for peer chats), not `session.toString()`. The session URI resolves to the session's *default* chat (`_sessions`), so dispatching there silently applies the change to the wrong conversation and an additional chat never sees the agent/model swap. The host must also forward the `chatChannel` through `agentSideEffects.handleAction` → `changeAgent`/`changeModel`, which apply it to `_chatSessions` when present. The protocol models `summary.agent`/`summary.model` at session level only, so equality guards comparing against session summary are valid for the default chat but must be skipped for peer chats.
- **Do not infer or fall back from a peer chat channel after progress was emitted**: Agent progress signals for chat-scoped actions, especially tool-call readiness and permission requests, must be emitted with the exact `ahp-chat://...` channel that owns the tool. Do not recover by scanning active turns, remapping `ChatToolCallConfirmed`, or using `parseDefaultChatUri(...) ?? sessionUri` in `AgentSideEffects`; malformed/misrouted chat channels should fail loudly so the producer or dispatch path is fixed. `handleToolCallConfirmed` and `_toolCallAgents` must use the chat channel URI containing the tool call; keying by the parent session URI makes confirmations miss the pending SDK request.
- **Do not synthesize default chat URIs in the workbench handler**: `AgentHostSessionHandler` must source the upstream default chat URI from hydrated `SessionState.defaultChat` / `SessionState.chats` and store that mapping in its chat-resource-to-upstream-URI map. Calling `buildDefaultChatUri(session)` in the handler assumes one server URI shape and hides protocol/provider bugs; dispatch turn lifecycle and pending/input actions through the mapped upstream chat URI instead.
- **Model subagents as chats, not sessions**: A subagent spawned from a tool call belongs to the parent session as an additional chat with `origin.kind === "tool"`, hidden from the chat tab strip. Do not call `restoreSession` for subagents; that creates `_sessionStates` without a matching `_chatStates` entry, so later chat actions hit "Action for unknown chat". Add a chat on the parent session and dispatch the subagent turn to that chat URI.
- **Keep case-sensitive ids out of URI authority**: URI authorities are case-insensitive, so do not place tool call ids in the `ahp-chat` authority. Subagent chat URIs use a stable `subagent` authority and put the encoded tool call id in the path; use `buildSubagentChatUri(...)` instead of `buildChatUri(..., \`subagent-${toolCallId}\`)`.
- **Selected custom agent must be in the SDK's `customAgents`, not just `pluginDirectories`**: The Copilot SDK validates the session-start `agent:` option (passed to `createSession`/`resumeSession`) against the `customAgents` list **by name only** — it does NOT consult `pluginDirectories`. `copilotSessionLauncher._buildSessionConfig` deliberately omits agents from file-dir plugins from `customAgents` (relying on the SDK's `pluginDirectories` discovery to avoid duplicates), so selecting a plugin/extension-contributed agent (e.g. "Inbox") otherwise fails with `Custom agent '<name>' not found`. The fix (`toSdkSessionCustomAgents`) force-adds the resolved selected agent into `customAgents` while every other file-dir agent still loads via `pluginDirectories`. Note the agent picker offers VS Code chat modes from `IChatModeService`, but only `plugin`/`extension` storage agents are synced to the host (`SYNCABLE_STORAGE_SOURCES`); `user`/`local` agents are never synced, so `_resolveAgentName` returns `undefined` for them and no `agent:` is sent.
- **Derive SDK custom-agent names exactly like `parseAgentFile`**: `_resolveAgentName` resolves the selected agent through the plugin parser, which trims the frontmatter `name` (`getStringValue('name')?.trim() || nameFromFile`). When building the SDK `customAgents` list (`toSdkCustomAgents`), derive the name the same way (`?.trim() || agent.name`); reading the raw frontmatter `name` without trimming yields a config name that won't match the trimmed `resolvedAgentName`, so the SDK still rejects the session with `Custom agent '<name>' not found`.
- **Peer chats have no server `summary`, so dedup side-channel dispatch against the last value sent for that chat**: equality guards before dispatching `SessionModelChanged`/`SessionAgentChanged` compare against `summary.model`/`summary.agent`, which only exist for the session's default chat. For peer chats, track the last-dispatched model/agent on the `AgentHostChatSession` instance (auto-cleaned on dispose) and diff against that — otherwise every peer-chat turn redundantly re-dispatches (and re-resolves the agent), and an intentional "clear selection" (`undefined`) can't be detected.
Expand Down
74 changes: 66 additions & 8 deletions src/vs/base/common/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -919,21 +919,79 @@ export class NKeyMap<TValue, TKeys extends (string | boolean | number)[]> {
return currentMap.get(keys[keys.length - 1]);
}

public delete(...keys: [...TKeys]): boolean {
const maps: Map<any, any>[] = [this._data];
let currentMap = this._data;
for (let i = 0; i < keys.length - 1; i++) {
const nextMap = currentMap.get(keys[i]);
if (nextMap === undefined) {
return false;
}
currentMap = nextMap;
maps.push(currentMap);
}
const deleted = currentMap.delete(keys[keys.length - 1]);
for (let i = keys.length - 2; deleted && i >= 0; i--) {
if (maps[i + 1].size === 0) {
maps[i].delete(keys[i]);
}
}
return deleted;
}

public deleteAll(...keys: Partial<TKeys>): boolean {
if (keys.length === 0) {
const hadData = this._data.size > 0;
this._data.clear();
return hadData;
}
const maps: Map<any, any>[] = [this._data];
let currentMap = this._data;
for (let i = 0; i < keys.length - 1; i++) {
const nextMap = currentMap.get(keys[i]);
if (nextMap === undefined) {
return false;
}
currentMap = nextMap;
maps.push(currentMap);
}
const deleted = currentMap.delete(keys[keys.length - 1]);
for (let i = keys.length - 2; deleted && i >= 0; i--) {
if (maps[i + 1].size === 0) {
maps[i].delete(keys[i]);
}
}
return deleted;
}

public clear(): void {
this._data.clear();
}

public *getAll(...keys: Partial<TKeys>): IterableIterator<TValue> {
let currentMap = this._data;
for (const key of keys) {
const nextMap = currentMap.get(key);
if (nextMap === undefined) {
return;
}
currentMap = nextMap;
}
yield* this._values(currentMap);
}

public *values(): IterableIterator<TValue> {
function* iterate(map: Map<any, any>): IterableIterator<TValue> {
for (const value of map.values()) {
if (value instanceof Map) {
yield* iterate(value);
} else {
yield value;
}
yield* this._values(this._data);
}

private *_values(map: Map<any, any>): IterableIterator<TValue> {
for (const value of map.values()) {
if (value instanceof Map) {
yield* this._values(value);
} else {
yield value;
}
}
yield* iterate(this._data);
}

/**
Expand Down
43 changes: 43 additions & 0 deletions src/vs/base/test/common/map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,49 @@ suite('NKeyMap', () => {
assert.deepStrictEqual(Array.from(map.values()), [1, 2, 3]);
});

test('getAll', () => {
const map = new NKeyMap<number, [string, string, string]>();
map.set(1, 'a', 'b', 'c');
map.set(2, 'a', 'b', 'd');
map.set(3, 'a', 'e', 'f');
map.set(4, 'g', 'h', 'i');
assert.deepStrictEqual(Array.from(map.getAll('a', 'b')), [1, 2]);
assert.deepStrictEqual(Array.from(map.getAll('a')), [1, 2, 3]);
assert.deepStrictEqual(Array.from(map.getAll('missing')), []);
});

test('delete', () => {
const map = new NKeyMap<number, [string, string, string]>();
map.set(1, 'a', 'b', 'c');
map.set(2, 'a', 'b', 'd');
map.set(3, 'x', 'y', 'z');
assert.strictEqual(map.delete('a', 'b', 'c'), true);
assert.strictEqual(map.delete('a', 'b', 'c'), false);
assert.deepStrictEqual(Array.from(map.values()), [2, 3]);
});

test('deleteAll', () => {
const map = new NKeyMap<number, [string, string, string]>();
map.set(1, 'a', 'b', 'c');
map.set(2, 'a', 'b', 'd');
map.set(3, 'a', 'e', 'f');
map.set(4, 'g', 'h', 'i');
assert.strictEqual(map.deleteAll('a', 'b'), true);
assert.deepStrictEqual(Array.from(map.values()), [3, 4]);
assert.strictEqual(map.deleteAll('missing'), false);
assert.strictEqual(map.deleteAll(), true);
assert.deepStrictEqual(Array.from(map.values()), []);
});

test('deleteAll cleans empty parent maps', () => {
const map = new NKeyMap<number, [string, string, string]>();
map.set(1, 'a', 'b', 'c');
map.set(2, 'x', 'y', 'z');
assert.strictEqual(map.deleteAll('a', 'b'), true);
assert.strictEqual(map.deleteAll('a'), false);
assert.deepStrictEqual(Array.from(map.values()), [2]);
});

test('toString', () => {
const map = new NKeyMap<number, [string, string, string]>();
map.set(1, 'f', 'o', 'o');
Expand Down
21 changes: 10 additions & 11 deletions src/vs/platform/agentHost/common/agentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,13 +720,13 @@ export type AgentSignal =
* dispatches the action through the state manager after routing via
* {@link IAgentActionSignal.parentToolCallId} (if set).
*
* Agents are responsible for populating `session` and any `turnId` /
* Agents are responsible for populating the target channel and any `turnId` /
* `partId` fields on the action.
*/
export interface IAgentActionSignal {
readonly kind: 'action';
/** Top-level session URI. For inner subagent events this is the parent session — see {@link parentToolCallId}. */
readonly session: URI;
/** Target session or chat channel URI. For inner subagent events this is the parent session — see {@link parentToolCallId}. */
readonly resource: URI;
/** Protocol action to dispatch. */
readonly action: SessionAction | ChatAction;
/** If set, route the action to the subagent session belonging to this tool call. */
Expand All @@ -749,7 +749,8 @@ export interface IAgentActionSignal {
*/
export interface IAgentToolPendingConfirmationSignal {
readonly kind: 'pending_confirmation';
readonly session: URI;
/** Target chat channel URI containing the tool call. */
readonly chat: URI;
/** Protocol-shaped pending-confirmation state, dispatched verbatim into `ChatToolCallReady`. */
readonly state: ToolCallPendingConfirmationState;
/** Host-only auto-approval kind (not part of the dispatched action). */
Expand Down Expand Up @@ -781,7 +782,7 @@ export interface IAgentToolPendingConfirmationSignal {
*/
export interface IAgentSubagentStartedSignal {
readonly kind: 'subagent_started';
readonly session: URI;
readonly chat: URI;
readonly toolCallId: string;
readonly agentName: string;
readonly agentDisplayName: string;
Expand All @@ -797,14 +798,14 @@ export interface IAgentSubagentStartedSignal {
*/
export interface IAgentSubagentCompletedSignal {
readonly kind: 'subagent_completed';
readonly session: URI;
readonly chat: URI;
readonly toolCallId: string;
}

/** A steering message was consumed (sent to the model). */
export interface IAgentSteeringConsumedSignal {
readonly kind: 'steering_consumed';
readonly session: URI;
readonly chat: URI;
readonly id: string;
}

Expand Down Expand Up @@ -936,10 +937,8 @@ export interface IAgent {
/** Return dynamic completions for a session configuration property. */
sessionConfigCompletions(params: IAgentSessionConfigCompletionsParams): Promise<SessionConfigCompletionsResult>;

/** Send a user message into an existing session. When `chat` is provided
* (and differs from the default chat), the harness routes the message to
* that specific chat within a multi-chat session. */
sendMessage(session: URI, prompt: string, attachments?: readonly MessageAttachment[], turnId?: string, chat?: URI): Promise<void>;
/** Send a user message into a chat within an existing session. */
sendMessage(session: URI, chat: URI, prompt: string, attachments?: readonly MessageAttachment[], turnId?: string): Promise<void>;

/**
* Create an additional chat within an existing session, backed by a new
Expand Down
7 changes: 7 additions & 0 deletions src/vs/platform/agentHost/common/state/agentSubscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export interface IAgentSubscription<T> {
/** Fires when {@link value} changes (optimistic or confirmed). */
readonly onDidChange: Event<T>;

/** Fires when the subscription enters an error state. */
readonly onDidError?: Event<Error>;

/** Fires before a server-originated action is applied to this subscription's state. */
readonly onWillApplyAction: Event<ActionEnvelope>;

Expand Down Expand Up @@ -103,6 +106,9 @@ abstract class BaseAgentSubscription<T> extends Disposable implements IAgentSubs
protected readonly _onDidChange = this._register(new Emitter<T>());
readonly onDidChange: Event<T> = this._onDidChange.event;

protected readonly _onDidError = this._register(new Emitter<Error>());
readonly onDidError: Event<Error> = this._onDidError.event;

protected readonly _onWillApplyAction = this._register(new Emitter<ActionEnvelope>());
readonly onWillApplyAction: Event<ActionEnvelope> = this._onWillApplyAction.event;

Expand Down Expand Up @@ -144,6 +150,7 @@ abstract class BaseAgentSubscription<T> extends Disposable implements IAgentSubs
*/
setError(error: Error): void {
this._error = error;
this._onDidError.fire(error);
}

/**
Expand Down
24 changes: 24 additions & 0 deletions src/vs/platform/agentHost/common/state/sessionState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,14 @@ export function buildDefaultChatUri(sessionUri: ProtocolURI | ResourceURI): stri
return buildChatUri(sessionUri, DEFAULT_CHAT_ID);
}

const SUBAGENT_CHAT_ID = 'subagent';

export function buildSubagentChatUri(sessionUri: ProtocolURI | ResourceURI, toolCallId: string): string {
const session = typeof sessionUri === 'string' ? sessionUri : sessionUri.toString();
const encoded = encodeBase64(VSBuffer.fromString(session), false, true);
return `${AHP_CHAT_SCHEME}://${SUBAGENT_CHAT_ID}/${encoded}/${encodeURIComponent(toolCallId)}`;
}

/**
* Inverse of {@link buildChatUri}: recovers the owning session URI and chat id
* from any chat channel URI. Returns `undefined` when `uri` is not a well-formed
Expand All @@ -637,6 +645,14 @@ export function parseChatUri(uri: ProtocolURI | ResourceURI): { session: string;
return undefined;
}
try {
if (parsed.authority === SUBAGENT_CHAT_ID) {
const [sessionPart, ...toolCallIdParts] = encoded.split('/');
const toolCallId = toolCallIdParts.join('/');
if (!sessionPart || !toolCallId) {
return undefined;
}
return { session: decodeBase64(sessionPart).toString(), chatId: `${SUBAGENT_CHAT_ID}/${decodeURIComponent(toolCallId)}` };
}
return { session: decodeBase64(encoded).toString(), chatId: parsed.authority };
} catch {
return undefined;
Expand All @@ -653,6 +669,14 @@ export function parseDefaultChatUri(uri: ProtocolURI | ResourceURI): string | un
return parseChatUri(uri)?.session;
}

export function parseRequiredSessionUriFromChatUri(uri: ProtocolURI | ResourceURI): string {
const session = parseDefaultChatUri(uri);
if (session === undefined) {
throw new Error(`Malformed AHP chat URI: ${typeof uri === 'string' ? uri : uri.toString()}`);
}
return session;
}

/** Returns `true` when `uri` is the default chat of its session. */
export function isDefaultChatUri(uri: ProtocolURI | ResourceURI): boolean {
return parseChatUri(uri)?.chatId === DEFAULT_CHAT_ID;
Expand Down
Loading
Loading