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: 2 additions & 2 deletions .github/skills/sessions/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ Then read the relevant spec for the area you are changing (see table below). If
- **`/btw` must bypass queue/steer, may anchor to `activeTurn`, and should activate the new peer chat through the normal sessions API**: mark the silent slash command `executeDuringRequest` so the chat widget invokes it independently, validate its anchor against completed turns or the current active turn, and after creating the side chat activate it via `ISessionsService.openChat(...)` before sending on that chat. Provider side-chat creation must lock on the new chat, not the source send key. Wrap the first provider prompt with a succinct instruction to prefer explanation over action and avoid work unless explicitly requested; include bounded user-visible active-turn markdown when native forks omit it, then strip the private wrapper from reconstructed visible history. Never inject reasoning or tool payloads.
- **A responsive-layout autorun must re-baseline (not react) to controller-driven restores, holding the flag across the async reveal**: the desktop [D7] responsive sidebar hides the sessions sidebar when small + editor + aux-bar are all open. Switching sessions restores layout via two async paths — the desktop aux-bar restore (`openView`/`openViewContainer`) **and** the base controller's editor working-set apply (`_applyWorkingSet`, which reveals the editor part *after* an `await` and runs on a `Sequencer` microtask). Both reveal parts in a *later* autorun run, so an inline "same-run session changed" check only absorbs the synchronous transition and the async reveal still auto-hid the sidebar on navigation. Fix: a shared base-controller `_withSessionLayoutRestore(work)` epoch wraps **both** restore paths (the working-set wrap is the critical one for non-modal editors); the D7 autorun re-baselines `_previousSpaceConstrained` while `_isRestoringSessionLayout` is true. Also gate the constrained derivation on `!multipleSessionsVisibleObs` so the feature is disabled with multiple sessions visible. Never use a `setTimeout` to bridge the async reveal — tie the flag to the actual promise.
- **A promise-tied "epoch" helper must decrement synchronously for void/sync work, only deferring for real Promises**: `_withSessionLayoutRestore` increments a depth counter, runs `work()`, and decrements when done. If it *always* schedules the decrement on a microtask (`Promise.resolve(result).finally(...)`) — even when `work()` returns `undefined` (the common no-op restore, e.g. a session with no workspace) — the depth stays elevated for the entire synchronous caller/test body, so `_isRestoringSessionLayout` reads `true` forever and the consumer (D7) silently stops acting. Only defer the decrement when `work()` returns a thenable; for void/sync (or throwing) work, decrement in the `finally`.
- **A quick-chat's workspace-less kind is seeded at adapter construction and only ever promoted — every path that can carry `_meta` must carry it**: `AgentHostSessionAdapter` seeds its session-kind (`QuickChatSessionKind` vs `WorkspaceSessionKind`) from `readSessionWorkspaceless(metadata._meta)` in the constructor, and `_promoteToQuickChatIfWorkspaceless` (from `update()`/`setMeta()`) later flips it to a quick chat the first time an authoritative `_meta` says workspace-less — never back, since an absent marker means "not included", not "cleared". So the `_meta.workspaceless` tag must ride on **every** metadata path: `_refreshSessions()`/`listSessions`, the live `_handleSessionAdded(summary)` notification, **and both ends of the AHP `root/listSessions` round-trip** (`protocolServerHandler.ts` and `remoteAgentHostProtocolClient.ts` build their wire items field-by-field, and `satisfies SessionSummary` does **not** catch a dropped *optional* field — this is exactly how the bug shipped twice). Dropping it makes a committed quick chat render under a section header labelled with its raw session UUID, leaking `<userHome>/.copilot/chats/<id>` as a `workspace` (breaking the archive-on-delete fallback, list badges, changes/files) until a later `_meta` heals it. `_persistCache` must overlay the adapter's **live** `isQuickChat` rather than the `_metaByRawId` snapshot, or the next `updateAdapter` silently strips the marker from the startup cache. On the host, `AgentService.listSessions()` overlays `_meta.workspaceless` onto the provider listing from the persisted `copilot.workspaceless` session metadata (the providers themselves, e.g. `CopilotAgent.listSessions()`, do not emit it) so restored sessions carry the tag once the state-manager live summary is gone. The host still keeps the tag on both the summary `_meta` and `SessionState._meta` (`createSessionState(summary)` copies it) so the channels stay consistent. **Note the provider tests cannot catch a wire-level drop** — `MockAgentHostService.listSessions` returns stored `IAgentSessionMetadata` verbatim, bypassing both mappers — so wire regressions need tests at the protocol layer.
- **A quick-chat's workspace-less kind is seeded at adapter construction and only ever promoted — every path that can carry `_meta` must carry it**: `AgentHostSessionAdapter` seeds its session-kind (`QuickChatSessionKind` vs `WorkspaceSessionKind`) from `readSessionWorkspaceless(metadata._meta)` in the constructor, and `_promoteToQuickChatIfWorkspaceless` (from `update()`/`setMeta()`) later flips it to a quick chat the first time an authoritative `_meta` says workspace-less — never back, since an absent marker means "not included", not "cleared". So the `_meta.workspaceless` tag must ride on **every** metadata path: `_refreshSessions()`/`listSessions`, the live `_handleSessionAdded(summary)` notification, **and both ends of the AHP `root/listSessions` round-trip** (`protocolServerHandler.ts` and `remoteAgentHostProtocolClient.ts` build their wire items field-by-field, and `satisfies SessionSummary` does **not** catch a dropped *optional* field — this is exactly how the bug shipped twice). Dropping it makes a committed quick chat render under a section header labelled with its raw session UUID, leaking `<userHome>/.copilot/chats/<id>` as a `workspace` (breaking the archive-on-delete fallback, list badges, changes/files) until a later `_meta` heals it. `_persistCache` must overlay the adapter's **live** `isQuickChat` rather than the `_metaByRawId` snapshot, or the next `updateAdapter` silently strips the marker from the startup cache. On the host, `AgentService.listSessions()` overlays `_meta.workspaceless` onto the provider listing from the persisted `agentHost.workspaceless` session-database key (`AH_META_WORKSPACELESS_DB_KEY`) (the providers themselves, e.g. `CopilotAgent.listSessions()`, do not emit it) so restored sessions carry the tag once the state-manager live summary is gone. The host still keeps the tag on both the summary `_meta` and `SessionState._meta` (`createSessionState(summary)` copies it) so the channels stay consistent. **Note the provider tests cannot catch a wire-level drop** — `MockAgentHostService.listSessions` returns stored `IAgentSessionMetadata` verbatim, bypassing both mappers — so wire regressions need tests at the protocol layer.
- **Don't infer "quick chat" from `workspace === undefined`**: a session's workspace observable is `undefined` for genuine quick chats but also transiently/edge-case for workspace-bound sessions, so keying quick-chat UI (context keys, list grouping) on it is imprecise. Expose the intent explicitly via the optional `ISession.isQuickChat: IObservable<boolean>` (only quick-chat-capable providers set it; absent ⇒ `false`). The agent-host adapter exposes it as a monotonic observable seeded from `readSessionWorkspaceless(metadata._meta)` at construction and promoted by later authoritative `_meta`; non-quick-chat providers omit it. Consume it through `isQuickChatSession(session)` / `session.isQuickChat?.read(reader) ?? false`.
- **Workspace-less is inferred from absent `workingDirectory` — exclude forks from that inference**: in `CopilotAgent.createSession`, `isWorkspaceless` must be `!sessionConfig.fork && !sessionConfig.workingDirectory`. A fork that arrives without an explicit `workingDirectory` should inherit the source session's context, not be tagged `copilot.workspaceless` and dropped into a scratch dir + quick-chat system prompt.
- **Workspace-less is inferred from absent `workingDirectory` — exclude forks from that inference**: in `CopilotAgent.createSession`, `isWorkspaceless` must be `!sessionConfig.fork && !sessionConfig.workingDirectory`. A fork that arrives without an explicit `workingDirectory` should inherit the source session's context, not be tagged `agentHost.workspaceless` and dropped into a scratch dir + quick-chat system prompt.
- **On a failed quick-chat create, don't activate an unrelated draft**: `SessionsService.openQuickChat` must, on `createQuickChat` throwing, log and return `undefined` without falling back to `_activate(newSession.get())` — that observable can hold a workspace-bound draft from a different call site, so activating it is surprising. Return the activated `IActiveSession` from `_activate`/`openQuickChat` (setActive is synchronous and yields the wrapper) and have the caller focus that exact value, rather than re-reading `activeSession.get()` afterwards.
- **Hide an aux-bar view CONTAINER via `hideIfEmpty: true` + a view `when`, not a container `when`**: `IViewContainerDescriptor` has **no `when`** property (only `IViewDescriptor` does). To conditionally hide a whole container (its tab/title) — e.g. the Agents-window Changes/Files containers for workspace-less sessions — put the context-key `when` on the inner view(s) and set `hideIfEmpty: true` on the container. Container visibility is `hideIfEmpty && activeViewDescriptors.length === 0` (`viewsService.updateViewContainerEnablementContextKey`), and `activeViewDescriptors` already respects each view's `when`, so the container hides reactively when all its views' `when` go false.
- **Don't gate "is this a quick chat?" routing on `isCreated && isQuickChat`**: a quick-chat **draft** is `Untitled`, so `isCreated` (`= status !== Untitled`, `visibleSessions.ts`) is `false` — yet it is still a quick chat. Gating quick-chat routing on `isCreated && isQuickChat` (e.g. the "New" action) makes a draft fall through to the workspace path (scratch-dir composer, no session-type picker, "No models available"). Route on **`isQuickChat` alone** so drafts and committed quick chats behave identically; use `isCreated` only when you genuinely need to distinguish a committed session from an in-composer draft.
Expand Down
23 changes: 23 additions & 0 deletions src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,29 @@ suite('ProtocolServerHandler', () => {
assert.deepStrictEqual(result.items.map(item => readSessionWorkspaceless(item._meta)), [true]);
});

test('listSessions omits _meta when the agent provides none', async () => {
// The wire item is built field by field and `satisfies SessionSummary`
// cannot catch a dropped optional, so pin the absent case too: a
// listing must not start manufacturing an empty `_meta` bag that later
// overwrites a richer one on the client.
agentService.listedSessions.push({
session: URI.parse(sessionUri),
startTime: 1000,
modifiedTime: 2000,
summary: 'Session Summary',
});

const transport = connectClient('client-list-no-meta');
transport.sent.length = 0;
const responsePromise = waitForResponse(transport, 2);

transport.simulateMessage(request(2, 'listSessions'));
const resp = await responsePromise;

const result = (resp as unknown as { result: ListSessionsResult }).result;
assert.deepStrictEqual(result.items.map(item => item._meta), [undefined]);
});

test('createSession returns null and broadcasts project in sessionAdded summary', async () => {
const transport = connectClient('client-create');
transport.sent.length = 0;
Expand Down
Loading
Loading