From 58cf8d8c327ca06aa1b521723cb0a9595d302579 Mon Sep 17 00:00:00 2001 From: vritant24 Date: Tue, 12 May 2026 09:37:24 -0700 Subject: [PATCH 01/43] bg todo: scope policy to current-turn activity and guard final review against stale turns Background todo passes were triggered by accumulated activity from previous turns when a conversation continued, causing todos to be recreated even for trivial messages like 'hi'. Three interacting problems are fixed: 1. Policy threshold used total substantiveToolCallCount across all unprocessed rounds (including previous turns). Add currentTurnSubstantiveToolCallCount to IBackgroundTodoDeltaMetadata and use it in shouldRun() so only current- turn activity drives threshold checks. 2. requestFinalReview fired on every turn end as long as _hasCreatedTodos was true, using _lastExecutionContext from a previous turn. Track _lastExecutionContextTurnId and skip when the context is stale. 3. _consecutiveInitialNoops persisted across turns, penalizing a new user message with backoff from a previous turn's exploration. Track _lastSeenTurnId and reset per-turn state when the turn changes. Also annotate tool-call rounds with 1-based turn indices (IToolCallRoundWithTurn) so the prompt can render boundary markers, and add renderRoundsGroupedByTurn for token-efficient turn grouping. Fixes stream: false on the copilot-fast request that caused SSE parse failures. --- .../src/extension/intents/node/agentIntent.ts | 6 +- .../prompts/node/agent/backgroundTodoDelta.ts | 31 ++++- .../node/agent/backgroundTodoProcessor.ts | 109 ++++++++++++++---- 3 files changed, 121 insertions(+), 25 deletions(-) diff --git a/extensions/copilot/src/extension/intents/node/agentIntent.ts b/extensions/copilot/src/extension/intents/node/agentIntent.ts index cab0bcacc4c2bc..28b579124f1e5e 100644 --- a/extensions/copilot/src/extension/intents/node/agentIntent.ts +++ b/extensions/copilot/src/extension/intents/node/agentIntent.ts @@ -1273,9 +1273,11 @@ export class AgentIntentInvocation extends EditCodeIntentInvocation implements I promptContext, }; + const turnId = promptContext.conversation?.getLatestTurn()?.id; + if (decision === BackgroundTodoDecision.Wait && reason === 'processorInProgress' && delta) { // Coalesce into the queue so the latest context is not lost. - processor.requestRegularPass(delta, executionContext, token); + processor.requestRegularPass(delta, executionContext, token, turnId); return; } @@ -1283,7 +1285,7 @@ export class AgentIntentInvocation extends EditCodeIntentInvocation implements I return; } - processor.requestRegularPass(delta, executionContext, token); + processor.requestRegularPass(delta, executionContext, token, turnId); } override processResponse = undefined; diff --git a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoDelta.ts b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoDelta.ts index 8cc41f29854782..69a183466f6612 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoDelta.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoDelta.ts @@ -50,10 +50,14 @@ export interface IBackgroundTodoDeltaMetadata { readonly newRoundCount: number; /** Total number of individual tool calls across new rounds. */ readonly newToolCallCount: number; - /** Number of substantive (non-excluded) tool calls in new rounds. - * Substantive = the agent did real work (reads, edits, terminal, - * searches, subagents, etc); excluded = infrastructure noise. */ + /** Number of substantive (non-excluded) tool calls across ALL new rounds + * (current turn + any unprocessed history rounds). */ readonly substantiveToolCallCount: number; + /** Number of substantive tool calls from the current turn only + * (`promptContext.toolCallRounds`). Used by the invocation policy + * so that unprocessed rounds from previous turns don't inflate the + * threshold and trigger spurious passes. */ + readonly currentTurnSubstantiveToolCallCount: number; /** True when this is the very first delta for the session (no rounds processed yet). */ readonly isInitialDelta: boolean; /** True when the delta contains only a user request and zero new rounds. */ @@ -125,6 +129,26 @@ export class BackgroundTodoDeltaTracker { } } + // Count substantive calls from current-turn rounds only so that + // unprocessed history rounds don't inflate the policy threshold. + const currentTurnRoundIds = new Set(); + for (const round of currentRounds) { + if (!this._processedRoundIds.has(round.id)) { + currentTurnRoundIds.add(round.id); + } + } + let currentTurnSubstantiveToolCallCount = 0; + for (const round of newRounds) { + if (!currentTurnRoundIds.has(round.id)) { + continue; + } + for (const call of round.toolCalls) { + if (classifyTool(call.name) === 'substantive') { + currentTurnSubstantiveToolCallCount++; + } + } + } + return { userRequest, newRounds, @@ -134,6 +158,7 @@ export class BackgroundTodoDeltaTracker { newRoundCount: newRounds.length, newToolCallCount, substantiveToolCallCount, + currentTurnSubstantiveToolCallCount, isInitialDelta, isRequestOnly: newRounds.length === 0, }, diff --git a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoProcessor.ts b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoProcessor.ts index 1a6ee0d03ff19a..8b19b648530f51 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoProcessor.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoProcessor.ts @@ -156,6 +156,10 @@ export class BackgroundTodoProcessor { /** Number of consecutive no-op passes that completed while no todos had been * created yet. Used to back off the initial-branch firing threshold. */ private _consecutiveInitialNoops: number = 0; + /** Turn ID from the most recent {@link requestRegularPass} call. When + * the turn changes, per-turn state like `_consecutiveInitialNoops` is + * reset so a new user message starts with a fresh policy evaluation. */ + private _lastSeenTurnId: string | undefined; // ── Two-slot queue ────────────────────────────────────────── // Regular passes coalesce into one slot; final review occupies a @@ -176,6 +180,10 @@ export class BackgroundTodoProcessor { * call. Used by {@link requestFinalReview} to build the synthetic * final-review delta when no explicit context is provided. */ private _lastExecutionContext: IBackgroundTodoExecutionContext | undefined; + /** Turn ID associated with {@link _lastExecutionContext}. Used by + * {@link requestFinalReview} to skip when the execution context is + * stale (from a previous turn with no new activity this turn). */ + private _lastExecutionContextTurnId: string | undefined; readonly deltaTracker = new BackgroundTodoDeltaTracker(); @@ -219,7 +227,7 @@ export class BackgroundTodoProcessor { return { decision: BackgroundTodoDecision.Wait, reason: 'processorInProgress', delta }; } - const { substantiveToolCallCount, isInitialDelta, isRequestOnly } = delta.metadata; + const { currentTurnSubstantiveToolCallCount, isInitialDelta, isRequestOnly } = delta.metadata; // ── Initial request (no tool calls yet) ──────────────────── if (isRequestOnly && isInitialDelta) { @@ -249,22 +257,22 @@ export class BackgroundTodoProcessor { BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD << this._consecutiveInitialNoops, BackgroundTodoProcessor.MAX_INITIAL_BACKOFF_THRESHOLD, ); - if (substantiveToolCallCount >= effectiveThreshold) { - this._logService?.debug(`[BackgroundTodo] policy: Run (initialActivity) — substantive=${substantiveToolCallCount} >= effective threshold=${effectiveThreshold} (noops=${this._consecutiveInitialNoops}), rounds=${delta.metadata.newRoundCount}`); + if (currentTurnSubstantiveToolCallCount >= effectiveThreshold) { + this._logService?.debug(`[BackgroundTodo] policy: Run (initialActivity) — substantive=${currentTurnSubstantiveToolCallCount} >= effective threshold=${effectiveThreshold} (noops=${this._consecutiveInitialNoops}), rounds=${delta.metadata.newRoundCount}`); return { decision: BackgroundTodoDecision.Run, reason: 'initialActivity', delta }; } const reason = this._consecutiveInitialNoops > 0 ? 'initialBackoff' : 'belowThreshold'; - this._logService?.debug(`[BackgroundTodo] policy: Wait (${reason}) — substantive=${substantiveToolCallCount} < effective threshold=${effectiveThreshold} (noops=${this._consecutiveInitialNoops}), rounds=${delta.metadata.newRoundCount}`); + this._logService?.debug(`[BackgroundTodo] policy: Wait (${reason}) — substantive=${currentTurnSubstantiveToolCallCount} < effective threshold=${effectiveThreshold} (noops=${this._consecutiveInitialNoops}), rounds=${delta.metadata.newRoundCount}`); return { decision: BackgroundTodoDecision.Wait, reason, delta }; } // ── Subsequent passes (todos already exist) ───────────────── - if (substantiveToolCallCount >= BackgroundTodoProcessor.SUBSEQUENT_SUBSTANTIVE_THRESHOLD) { - this._logService?.debug(`[BackgroundTodo] policy: Run (substantiveActivity) — substantive=${substantiveToolCallCount} >= threshold=${BackgroundTodoProcessor.SUBSEQUENT_SUBSTANTIVE_THRESHOLD}, rounds=${delta.metadata.newRoundCount}`); + if (currentTurnSubstantiveToolCallCount >= BackgroundTodoProcessor.SUBSEQUENT_SUBSTANTIVE_THRESHOLD) { + this._logService?.debug(`[BackgroundTodo] policy: Run (substantiveActivity) — substantive=${currentTurnSubstantiveToolCallCount} >= threshold=${BackgroundTodoProcessor.SUBSEQUENT_SUBSTANTIVE_THRESHOLD}, rounds=${delta.metadata.newRoundCount}`); return { decision: BackgroundTodoDecision.Run, reason: 'substantiveActivity', delta }; } - this._logService?.debug(`[BackgroundTodo] policy: Wait (belowThreshold) — substantive=${substantiveToolCallCount}, rounds=${delta.metadata.newRoundCount}`); + this._logService?.debug(`[BackgroundTodo] policy: Wait (belowThreshold) — substantive=${currentTurnSubstantiveToolCallCount}, rounds=${delta.metadata.newRoundCount}`); return { decision: BackgroundTodoDecision.Wait, reason: 'belowThreshold', delta }; } @@ -274,14 +282,24 @@ export class BackgroundTodoProcessor { * Enqueue or coalesce a regular background pass. If a pass is already * running, the delta is stashed and will drain when the current pass * completes. Always updates {@link _lastExecutionContext}. + * + * @param turnId The ID of the turn that triggered this pass. Used by + * {@link requestFinalReview} to detect stale contexts. */ requestRegularPass( delta: IBackgroundTodoDelta, context: IBackgroundTodoExecutionContext, parentToken?: CancellationToken, + turnId?: string, ): void { + // Reset per-turn state when a new turn is detected. + if (turnId !== undefined && turnId !== this._lastSeenTurnId) { + this._consecutiveInitialNoops = 0; + this._lastSeenTurnId = turnId; + } this._lastExecutionContext = context; - this._logService?.debug(`[BackgroundTodo] requestRegularPass — newRounds=${delta.metadata.newRoundCount}, substantive=${delta.metadata.substantiveToolCallCount}, state=${this._state}`); + this._lastExecutionContextTurnId = turnId; + this._logService?.debug(`[BackgroundTodo] requestRegularPass — newRounds=${delta.metadata.newRoundCount}, substantive=${delta.metadata.substantiveToolCallCount}, state=${this._state}, turnId=${turnId}`); this._pendingRegularDelta = delta; this._pendingRegularContext = context; this._pendingRegularToken = parentToken; @@ -303,6 +321,12 @@ export class BackgroundTodoProcessor { this._logService?.debug(`[BackgroundTodo] final review skipped — hasCreatedTodos=${this._hasCreatedTodos}, hasExecutionContext=${this._lastExecutionContext !== undefined}`); return; } + // Skip if the execution context is from a different turn — no bg todo + // work happened this turn, so there is nothing new to finalize. + if (this._lastExecutionContextTurnId !== undefined && this._lastExecutionContextTurnId !== turnId) { + this._logService?.debug(`[BackgroundTodo] final review skipped — execution context is from turn ${this._lastExecutionContextTurnId}, not current turn ${turnId}`); + return; + } if (this._finalReviewAttemptedTurnId === turnId) { this._logService?.debug(`[BackgroundTodo] final review skipped — already attempted for turn ${turnId}`); return; @@ -436,13 +460,14 @@ export class BackgroundTodoProcessor { // Build a synthetic delta from the full trajectory so the // finalize prompt sees every round. - const allRounds = collectAllRounds( + const allRoundsWithTurns = collectAllRounds( finalCtx.promptContext.history, finalCtx.promptContext.toolCallRounds ?? [], ); - if (allRounds.length === 0) { + if (allRoundsWithTurns.length === 0) { return; } + const allRounds = allRoundsWithTurns.map(r => r.round); let substantive = 0; for (const round of allRounds) { for (const call of round.toolCalls) { @@ -460,6 +485,7 @@ export class BackgroundTodoProcessor { newRoundCount: allRounds.length, newToolCallCount: substantive, substantiveToolCallCount: substantive, + currentTurnSubstantiveToolCallCount: substantive, isInitialDelta: false, isRequestOnly: false, }, @@ -656,7 +682,6 @@ export class BackgroundTodoProcessor { location: ChatLocation.Other, requestOptions: { temperature: 0, - stream: false, tools: normalizedTools, }, userInitiatedRequest: false, @@ -780,6 +805,8 @@ export class BackgroundTodoProcessor { this._pendingFinalReview = undefined; this._pendingFinalReviewToken = undefined; this._lastExecutionContext = undefined; + this._lastExecutionContextTurnId = undefined; + this._lastSeenTurnId = undefined; this._finalReviewAttemptedTurnId = undefined; } } @@ -956,6 +983,9 @@ export interface IBackgroundTodoHistoryRound { readonly id: string; /** Position in the chronological list, starting at 1. */ readonly index: number; + /** 1-based turn index this round belongs to. Rounds from history turns + * precede the current turn's rounds. Used to render `` boundaries. */ + readonly turnIndex: number; /** Optional model thinking text rendered as a block in the round chunk. */ readonly thinking?: string; /** Tool calls issued during the round; excluded tools are filtered out. */ @@ -981,7 +1011,7 @@ export interface IBackgroundTodoHistory { // ── Builder ───────────────────────────────────────────────────── export interface IBuildBackgroundTodoHistoryOptions { - readonly allRounds: readonly IToolCallRound[]; + readonly allRounds: readonly IToolCallRoundWithTurn[]; readonly newRoundIds: ReadonlySet; } @@ -992,7 +1022,8 @@ export function buildBackgroundTodoHistory(opts: IBuildBackgroundTodoHistoryOpti const newRounds: IBackgroundTodoHistoryRound[] = []; let index = 0; - for (const round of allRounds) { + for (const roundWithTurn of allRounds) { + const round = roundWithTurn.round; const summaries = summarizeToolCalls(round.toolCalls); const thinking = serializeThinking(round.thinking); const response = round.response.trim().length > 0 ? round.response : undefined; @@ -1006,6 +1037,7 @@ export function buildBackgroundTodoHistory(opts: IBuildBackgroundTodoHistoryOpti const historyRound: IBackgroundTodoHistoryRound = { id: round.id, index, + turnIndex: roundWithTurn.turnIndex, thinking, toolCalls: summaries, response, @@ -1118,6 +1150,33 @@ export function renderBackgroundTodoRound(round: IBackgroundTodoHistoryRound): s return lines.join('\n'); } +/** + * Render a list of rounds grouped by `turnIndex`, wrapping consecutive + * same-turn rounds inside `…` tags. This saves + * tokens compared to repeating a `turn` attribute on every ``. + */ +export function renderRoundsGroupedByTurn(rounds: readonly IBackgroundTodoHistoryRound[]): string { + if (rounds.length === 0) { + return ''; + } + const lines: string[] = []; + let currentTurn: number | undefined; + for (const round of rounds) { + if (round.turnIndex !== currentTurn) { + if (currentTurn !== undefined) { + lines.push(''); + } + lines.push(``); + currentTurn = round.turnIndex; + } + lines.push(renderBackgroundTodoRound(round)); + } + if (currentTurn !== undefined) { + lines.push(''); + } + return lines.join('\n'); +} + /** * Compute a prompt-tsx priority for a previous-context round so newer * rounds survive budget pressure ahead of older history. Values are @@ -1132,17 +1191,27 @@ export function computeRoundPriority(round: IBackgroundTodoHistoryRound, totalPr return Math.min(879, 700 + Math.min(round.index, totalPreviousRounds)); } +/** A tool-call round annotated with its 1-based turn index. */ +export interface IToolCallRoundWithTurn { + readonly round: IToolCallRound; + readonly turnIndex: number; +} + /** * Collect all tool-call rounds from history turns and current-turn rounds - * in chronological order. + * in chronological order, annotated with 1-based turn indices. */ -export function collectAllRounds(history: readonly Turn[], currentRounds: readonly IToolCallRound[]): IToolCallRound[] { - const all: IToolCallRound[] = []; - for (const turn of history) { - for (const round of turn.rounds) { - all.push(round); +export function collectAllRounds(history: readonly Turn[], currentRounds: readonly IToolCallRound[]): IToolCallRoundWithTurn[] { + const all: IToolCallRoundWithTurn[] = []; + for (let i = 0; i < history.length; i++) { + const turnIndex = i + 1; + for (const round of history[i].rounds) { + all.push({ round, turnIndex }); } } - all.push(...currentRounds); + const currentTurnIndex = history.length + 1; + for (const round of currentRounds) { + all.push({ round, turnIndex: currentTurnIndex }); + } return all; } From f2d89d642f84788ac1841e88316887e5bdae465c Mon Sep 17 00:00:00 2001 From: vritant24 Date: Tue, 12 May 2026 09:37:39 -0700 Subject: [PATCH 02/43] bg todo: add cross-turn awareness to background todo prompts Update both system messages (regular and final-review) with turn-aware trajectory format documentation and cross-turn rules: - Rounds are now grouped inside wrappers instead of repeating a turn attribute per round, saving tokens. - Cross-turn rules instruct the model to treat previous turns as already- reflected context and only update todos based on current-turn activity. - Final-review prompt focuses completion evidence on the latest turn and skips tool calls when the turn had no substantive activity. PreviousContextRoundChunk emits open/close tags at boundaries via annotateWithTurnBoundaries(). Inline rendering uses renderRoundsGroupedByTurn. --- .../node/agent/backgroundTodoPrompt.tsx | 57 ++++++++++++++++--- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx index 405fb3a555a683..4556cbf7b5bb63 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx +++ b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { BasePromptElementProps, Chunk, PrioritizedList, PromptElement, PromptSizing, SystemMessage, UserMessage } from '@vscode/prompt-tsx'; -import { computeRoundPriority, escapeForPromptTag, IBackgroundTodoHistory, IBackgroundTodoHistoryRound, renderBackgroundTodoRound } from './backgroundTodoProcessor'; +import { computeRoundPriority, escapeForPromptTag, IBackgroundTodoHistory, IBackgroundTodoHistoryRound, renderBackgroundTodoRound, renderRoundsGroupedByTurn } from './backgroundTodoProcessor'; export interface BackgroundTodoPromptProps extends BasePromptElementProps { /** Current todo list state as rendered markdown, or undefined if no todos exist yet. */ @@ -27,8 +27,15 @@ Trajectory format: - The agent trajectory is split into two sections: - contains rounds from before this background pass. They provide continuity context only — do not treat them as new work. - contains the rounds that happened since your previous background pass. Use these to decide whether the todo list should change. +- Each block carries an index attribute. Rounds are grouped inside wrappers. A turn is one user message plus all the agent work that follows it. When the turn number changes, a new user message was sent. Rounds from earlier turns represent completed previous interactions. - Each block may contain the agent's optional , a list (with file path or category target and an optional intent note), and a with the assistant text that followed. +Cross-turn rules: +- Work from previous turns is already finished. Their rounds are context for what was accomplished before, not new activity. +- If a todo list already exists and all rounds in belong to the same turn as the latest user message, compare the new work against the current list. Only call the tool if statuses or items need updating based on the new work in the current turn. +- Never recreate or re-emit a todo list just because previous turns' rounds are visible in . The current todo list already reflects that work. +- If the new turn's activity is trivial (e.g. a greeting, a question, or a simple acknowledgment with no substantive tool calls), do NOT update the todo list. + Do NOT call tools when: - The current todo list already accurately reflects the work: same items, same statuses, same order. This is the most common case — most rounds require no update. - No todo list exists yet and the task does not qualify for one (see below). @@ -109,7 +116,13 @@ Default to silence. Before calling manage_todo_list, ask yourself: "Would the up Trajectory format: - The agent trajectory is presented inside a single block containing a chronological list of blocks. Each round may contain the agent's optional , a list (with file path or category target and an optional intent note), and a with the assistant text that followed. -- This is a final review — reason about the entire trajectory. +- Each carries an index attribute. Rounds are grouped inside wrappers. A turn is one user message plus all the agent work that follows it. When the turn number changes, a new user message was sent. +- This is a final review — reason about the entire trajectory, but focus completion evidence on the current (latest) turn. + +Cross-turn rules: +- Rounds from earlier turns represent work that was already completed in previous interactions. Their outcomes should already be reflected in the current todo list. +- Only use rounds from the current (latest) turn to determine whether new items should be marked completed or in-progress. +- If the current turn had no substantive tool calls (e.g. the user just sent a greeting or asked a question), do NOT call the tool — the existing todo list is already accurate. Do NOT call tools when: - No todo list exists. @@ -134,6 +147,10 @@ Ordering and state rules: interface PreviousContextRoundChunkProps extends BasePromptElementProps { readonly round: IBackgroundTodoHistoryRound; readonly totalPreviousRounds: number; + /** Whether this is the first round in its turn group. */ + readonly isFirstInTurn: boolean; + /** Whether this is the last round in its turn group. */ + readonly isLastInTurn: boolean; } /** @@ -144,14 +161,34 @@ interface PreviousContextRoundChunkProps extends BasePromptElementProps { class PreviousContextRoundChunk extends PromptElement { render() { const priority = computeRoundPriority(this.props.round, this.props.totalPreviousRounds); + const { round, isFirstInTurn, isLastInTurn } = this.props; return ( - {renderBackgroundTodoRound(this.props.round)} + {isFirstInTurn ? `\n` : ''} + {renderBackgroundTodoRound(round)} + {isLastInTurn ? '\n' : ''} ); } } +/** + * Annotate each round with whether it is the first/last in its turn group, + * so that `PreviousContextRoundChunk` can emit `` open/close tags + * only at boundaries. + */ +function annotateWithTurnBoundaries(rounds: readonly IBackgroundTodoHistoryRound[]): { round: IBackgroundTodoHistoryRound; isFirstInTurn: boolean; isLastInTurn: boolean }[] { + return rounds.map((round, i) => { + const prevTurn = i > 0 ? rounds[i - 1].turnIndex : undefined; + const nextTurn = i < rounds.length - 1 ? rounds[i + 1].turnIndex : undefined; + return { + round, + isFirstInTurn: round.turnIndex !== prevTurn, + isLastInTurn: round.turnIndex !== nextTurn, + }; + }); +} + /** * Prompt-tsx element for the background todo processor. * @@ -196,10 +233,12 @@ export class BackgroundTodoPrompt extends PromptElement {'\n'} - {[...history.previousRounds, ...history.newRounds].map(round => ( + {annotateWithTurnBoundaries([...history.previousRounds, ...history.newRounds]).map(item => ( ))} @@ -211,10 +250,12 @@ export class BackgroundTodoPrompt extends PromptElement {'\n'} - {history.previousRounds.map(round => ( + {annotateWithTurnBoundaries(history.previousRounds).map(item => ( ))} @@ -225,7 +266,7 @@ export class BackgroundTodoPrompt extends PromptElement {'\nUse these rounds to decide whether the todo list needs updating:\n'} - {history.newRounds.map(round => renderBackgroundTodoRound(round)).join('\n')} + {renderRoundsGroupedByTurn(history.newRounds)} {'\n'} )} From 422f532d4a2b59eceeceb1d8b4f55f92c14e609f Mon Sep 17 00:00:00 2001 From: vritant24 Date: Tue, 12 May 2026 09:37:52 -0700 Subject: [PATCH 03/43] bg todo: add tests for cross-turn policy scoping and turn-aware history - backgroundTodoDelta: test currentTurnSubstantiveToolCallCount only counts current-turn rounds, is zero for history-only deltas, excludes processed rounds, and matches total when no history is present. - backgroundTodoHistory: update collectAllRounds test for turn-annotated output, wrap buildBackgroundTodoHistory rounds with turnIndex, add turnIndex to all IBackgroundTodoHistoryRound inline objects, add renderRoundsGroupedByTurn tests for turn boundary wrapping. --- .../agent/test/backgroundTodoDelta.spec.ts | 56 +++++++++++++++++ .../agent/test/backgroundTodoHistory.spec.ts | 62 ++++++++++++++++--- 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoDelta.spec.ts b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoDelta.spec.ts index d9866e0971b674..481c6453ab87dc 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoDelta.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoDelta.spec.ts @@ -182,6 +182,62 @@ describe('BackgroundTodoDeltaTracker', () => { expect(delta.metadata.isRequestOnly).toBe(false); }); + // ── currentTurnSubstantiveToolCallCount ────────────────────── + + test('currentTurnSubstantiveToolCallCount counts only current-turn rounds', () => { + const tracker = new BackgroundTodoDeltaTracker(); + const historyRound: IToolCallRound = { + id: 'h1', response: '', toolInputRetry: 0, + toolCalls: [ + { name: 'read_file', arguments: '{}', id: 'tc-h1a' }, + { name: 'edit_file', arguments: '{}', id: 'tc-h1b' }, + ], + }; + const currentRound: IToolCallRound = { + id: 'c1', response: '', toolInputRetry: 0, + toolCalls: [{ name: 'read_file', arguments: '{}', id: 'tc-c1' }], + }; + const ctx = makePromptContext({ historyRounds: [[historyRound]], toolCallRounds: [currentRound] }); + const delta = tracker.peekDelta(ctx)!; + // Total substantive counts all unprocessed rounds (2 from history + 1 current) + expect(delta.metadata.substantiveToolCallCount).toBe(3); + // Current-turn only counts the current round (1) + expect(delta.metadata.currentTurnSubstantiveToolCallCount).toBe(1); + }); + + test('currentTurnSubstantiveToolCallCount is zero when all rounds are from history', () => { + const tracker = new BackgroundTodoDeltaTracker(); + const historyRound = makeRound('h1'); + const ctx = makePromptContext({ historyRounds: [[historyRound]] }); + const delta = tracker.peekDelta(ctx)!; + expect(delta.metadata.substantiveToolCallCount).toBe(1); + expect(delta.metadata.currentTurnSubstantiveToolCallCount).toBe(0); + }); + + test('currentTurnSubstantiveToolCallCount excludes already-processed current-turn rounds', () => { + const tracker = new BackgroundTodoDeltaTracker(); + const r1 = makeRound('r1'); + const ctx1 = makePromptContext({ toolCallRounds: [r1] }); + tracker.markProcessed(tracker.peekDelta(ctx1)!); + + // r1 is processed, r2 is new — only r2 should count + const r2 = makeRound('r2'); + const ctx2 = makePromptContext({ toolCallRounds: [r1, r2] }); + const delta = tracker.peekDelta(ctx2)!; + expect(delta.metadata.currentTurnSubstantiveToolCallCount).toBe(1); + expect(delta.metadata.substantiveToolCallCount).toBe(1); + }); + + test('currentTurnSubstantiveToolCallCount equals substantiveToolCallCount when no history', () => { + const tracker = new BackgroundTodoDeltaTracker(); + const r1 = makeRound('r1'); + const r2 = makeRound('r2'); + const ctx = makePromptContext({ toolCallRounds: [r1, r2] }); + const delta = tracker.peekDelta(ctx)!; + expect(delta.metadata.currentTurnSubstantiveToolCallCount).toBe(2); + expect(delta.metadata.substantiveToolCallCount).toBe(2); + }); + // ── Peek / commit semantics ───────────────────────────────── test('peekDelta does not advance cursor', () => { diff --git a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoHistory.spec.ts b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoHistory.spec.ts index 1e7b3137347287..f5ef547cab6c44 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoHistory.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoHistory.spec.ts @@ -14,7 +14,9 @@ import { extractTarget, extractToolNote, IBackgroundTodoHistoryRound, + IToolCallRoundWithTurn, renderBackgroundTodoRound, + renderRoundsGroupedByTurn, } from '../backgroundTodoProcessor'; function makeCall(name: string, args: Record = {}, id?: string): IToolCall { @@ -29,6 +31,10 @@ function makeRound(id: string, calls: IToolCall[], response = '', thinkingText?: return round; } +function wrapRound(round: IToolCallRound, turnIndex: number = 1): IToolCallRoundWithTurn { + return { round, turnIndex }; +} + describe('classifyTool', () => { test('classifies tool categories consistently', () => { expect({ @@ -116,12 +122,12 @@ describe('extractToolNote', () => { }); describe('collectAllRounds', () => { - test('combines history and current rounds in order', () => { + test('combines history and current rounds in order with turn indices', () => { const historyRound = makeRound('h1', [makeCall(ToolName.ReadFile)]); const currentRound = makeRound('c1', [makeCall(ToolName.CreateFile)]); const history = [{ rounds: [historyRound] }] as any; const result = collectAllRounds(history, [currentRound]); - expect(result.map(r => r.id)).toEqual(['h1', 'c1']); + expect(result.map(r => ({ id: r.round.id, turnIndex: r.turnIndex }))).toEqual([{ id: 'h1', turnIndex: 1 }, { id: 'c1', turnIndex: 2 }]); }); }); @@ -130,13 +136,14 @@ describe('buildBackgroundTodoHistory', () => { const r1 = makeRound('r1', [makeCall(ToolName.ReadFile, { filePath: 'src/a.ts' })], 'Read the file', 'Plan: read the file'); const r2 = makeRound('r2', [makeCall(ToolName.ReplaceString, { filePath: 'src/a.ts', explanation: 'fix typo' })], 'Done'); const result = buildBackgroundTodoHistory({ - allRounds: [r1, r2], + allRounds: [wrapRound(r1, 1), wrapRound(r2, 1)], newRoundIds: new Set(['r2']), }); expect(result.previousRounds.map(round => ({ id: round.id, index: round.index, + turnIndex: round.turnIndex, thinking: round.thinking, toolCalls: round.toolCalls, response: round.response, @@ -144,6 +151,7 @@ describe('buildBackgroundTodoHistory', () => { { id: 'r1', index: 1, + turnIndex: 1, thinking: 'Plan: read the file', toolCalls: [{ name: ToolName.ReadFile, target: 'src/a.ts', category: 'substantive' }], response: 'Read the file', @@ -153,6 +161,7 @@ describe('buildBackgroundTodoHistory', () => { expect(result.newRounds.map(round => ({ id: round.id, index: round.index, + turnIndex: round.turnIndex, thinking: round.thinking, toolCalls: round.toolCalls, response: round.response, @@ -160,6 +169,7 @@ describe('buildBackgroundTodoHistory', () => { { id: 'r2', index: 2, + turnIndex: 1, thinking: undefined, toolCalls: [{ name: ToolName.ReplaceString, target: 'src/a.ts', note: 'fix typo', category: 'substantive' }], response: 'Done', @@ -169,13 +179,13 @@ describe('buildBackgroundTodoHistory', () => { test('thinking with array text is joined and trimmed', () => { const r1 = makeRound('r1', [makeCall(ToolName.ReadFile, { filePath: 'a.ts' })], '', [' step one ', 'step two']); - const result = buildBackgroundTodoHistory({ allRounds: [r1], newRoundIds: new Set() }); + const result = buildBackgroundTodoHistory({ allRounds: [wrapRound(r1)], newRoundIds: new Set() }); expect(result.previousRounds[0].thinking).toBe('step one \nstep two'); }); test('skips entirely empty rounds', () => { const empty = makeRound('r1', [makeCall(ToolName.CoreManageTodoList)]); - const result = buildBackgroundTodoHistory({ allRounds: [empty], newRoundIds: new Set() }); + const result = buildBackgroundTodoHistory({ allRounds: [wrapRound(empty)], newRoundIds: new Set() }); expect(result.previousRounds).toHaveLength(0); expect(result.newRounds).toHaveLength(0); }); @@ -184,7 +194,7 @@ describe('buildBackgroundTodoHistory', () => { const r1 = makeRound('r1', [makeCall(ToolName.ReplaceString, { filePath: 'a.ts' })], 'r1'); const r2 = makeRound('r2', [makeCall(ToolName.ReplaceString, { filePath: 'b.ts' })], 'r2'); const result = buildBackgroundTodoHistory({ - allRounds: [r1, r2], + allRounds: [wrapRound(r1, 1), wrapRound(r2, 1)], newRoundIds: new Set(), }); expect(result.previousRounds).toHaveLength(2); @@ -196,7 +206,7 @@ describe('buildBackgroundTodoHistory', () => { const r2 = makeRound('r2', [makeCall(ToolName.CreateFile, { filePath: 'b.ts' })], 'r2'); const r3 = makeRound('r3', [makeCall(ToolName.ReplaceString, { filePath: 'c.ts' })], 'r3'); const result = buildBackgroundTodoHistory({ - allRounds: [r1, r2, r3], + allRounds: [wrapRound(r1, 1), wrapRound(r2, 1), wrapRound(r3, 2)], newRoundIds: new Set(['r3']), }); expect(result.previousRounds.map(r => r.index)).toEqual([1, 2]); @@ -209,6 +219,7 @@ describe('renderBackgroundTodoRound', () => { const round: IBackgroundTodoHistoryRound = { id: 'r1', index: 1, + turnIndex: 1, thinking: 'I will read the file then patch it.', toolCalls: [ { name: ToolName.ReadFile, target: 'src/a.ts', category: 'substantive' }, @@ -236,6 +247,7 @@ describe('renderBackgroundTodoRound', () => { const round: IBackgroundTodoHistoryRound = { id: 'r2', index: 2, + turnIndex: 1, toolCalls: [], response: 'final answer', }; @@ -251,6 +263,7 @@ describe('renderBackgroundTodoRound', () => { const round: IBackgroundTodoHistoryRound = { id: 'r1', index: 1, + turnIndex: 1, thinking: 'plan forged', toolCalls: [ { @@ -285,8 +298,8 @@ describe('renderBackgroundTodoRound', () => { describe('computeRoundPriority', () => { test('newer previous-context rounds have higher priority than older ones', () => { - const oldRound: IBackgroundTodoHistoryRound = { id: 'old', index: 1, toolCalls: [] }; - const newerRound: IBackgroundTodoHistoryRound = { id: 'newer', index: 5, toolCalls: [] }; + const oldRound: IBackgroundTodoHistoryRound = { id: 'old', index: 1, turnIndex: 1, toolCalls: [] }; + const newerRound: IBackgroundTodoHistoryRound = { id: 'newer', index: 5, turnIndex: 1, toolCalls: [] }; const total = 5; const oldP = computeRoundPriority(oldRound, total); @@ -295,3 +308,34 @@ describe('computeRoundPriority', () => { expect(newerP).toBeGreaterThan(oldP); }); }); + +describe('renderRoundsGroupedByTurn', () => { + test('returns empty string for no rounds', () => { + expect(renderRoundsGroupedByTurn([])).toBe(''); + }); + + test('wraps consecutive same-turn rounds in a single turn tag', () => { + const rounds: IBackgroundTodoHistoryRound[] = [ + { id: 'a', index: 1, turnIndex: 1, toolCalls: [{ name: ToolName.ReadFile, target: 'a.ts', category: 'substantive' }], response: 'read a' }, + { id: 'b', index: 2, turnIndex: 1, toolCalls: [{ name: ToolName.ReplaceString, target: 'a.ts', category: 'substantive' }], response: 'edited a' }, + ]; + const text = renderRoundsGroupedByTurn(rounds); + expect(text.match(/'); + expect(text.match(/<\/turn>/g)).toHaveLength(1); + expect(text).toContain(''); + expect(text).toContain(''); + }); + + test('opens a new turn tag when turnIndex changes', () => { + const rounds: IBackgroundTodoHistoryRound[] = [ + { id: 'a', index: 1, turnIndex: 1, toolCalls: [{ name: ToolName.ReadFile, target: 'a.ts', category: 'substantive' }], response: 'r1' }, + { id: 'b', index: 2, turnIndex: 2, toolCalls: [{ name: ToolName.CreateFile, target: 'b.ts', category: 'substantive' }], response: 'r2' }, + ]; + const text = renderRoundsGroupedByTurn(rounds); + expect(text.match(/'); + expect(text).toContain(''); + expect(text.match(/<\/turn>/g)).toHaveLength(2); + }); +}); From d33e8c2ffe250a95f0cd73ce892439f2d9d2a2a4 Mon Sep 17 00:00:00 2001 From: vritant24 Date: Tue, 12 May 2026 09:53:06 -0700 Subject: [PATCH 04/43] bg todo: address PR review comments - Make PreviousContextRoundChunk self-contained by always wrapping each round in tags, preventing unbalanced tags when PrioritizedList prunes boundary rounds. Remove annotateWithTurnBoundaries helper. - Add currentTurnSubstantiveToolCallCount to all dummyMeta object literals in policy and processor tests for type safety. - Add unit tests for requestFinalReview turn-ID guard: verify it skips when execution context is from a different turn and runs when IDs match. --- .../node/agent/backgroundTodoPrompt.tsx | 43 +++++-------------- .../agent/test/backgroundTodoPolicy.spec.ts | 12 +++--- .../test/backgroundTodoProcessor.spec.ts | 34 +++++++++++++++ 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx index 4556cbf7b5bb63..2479ded1279cfd 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx +++ b/extensions/copilot/src/extension/prompts/node/agent/backgroundTodoPrompt.tsx @@ -147,48 +147,29 @@ Ordering and state rules: interface PreviousContextRoundChunkProps extends BasePromptElementProps { readonly round: IBackgroundTodoHistoryRound; readonly totalPreviousRounds: number; - /** Whether this is the first round in its turn group. */ - readonly isFirstInTurn: boolean; - /** Whether this is the last round in its turn group. */ - readonly isLastInTurn: boolean; } /** * Prompt element rendering a single previous-context round as its own * Chunk so that prompt-tsx can drop older rounds independently under - * budget pressure. + * budget pressure. Each chunk is self-contained: it wraps its round + * in `` tags so that pruning any subset of rounds never produces + * unbalanced or mis-nested tags. */ class PreviousContextRoundChunk extends PromptElement { render() { const priority = computeRoundPriority(this.props.round, this.props.totalPreviousRounds); - const { round, isFirstInTurn, isLastInTurn } = this.props; + const { round } = this.props; return ( - {isFirstInTurn ? `\n` : ''} + {`\n`} {renderBackgroundTodoRound(round)} - {isLastInTurn ? '\n' : ''} + {'\n'} ); } } -/** - * Annotate each round with whether it is the first/last in its turn group, - * so that `PreviousContextRoundChunk` can emit `` open/close tags - * only at boundaries. - */ -function annotateWithTurnBoundaries(rounds: readonly IBackgroundTodoHistoryRound[]): { round: IBackgroundTodoHistoryRound; isFirstInTurn: boolean; isLastInTurn: boolean }[] { - return rounds.map((round, i) => { - const prevTurn = i > 0 ? rounds[i - 1].turnIndex : undefined; - const nextTurn = i < rounds.length - 1 ? rounds[i + 1].turnIndex : undefined; - return { - round, - isFirstInTurn: round.turnIndex !== prevTurn, - isLastInTurn: round.turnIndex !== nextTurn, - }; - }); -} - /** * Prompt-tsx element for the background todo processor. * @@ -233,12 +214,10 @@ export class BackgroundTodoPrompt extends PromptElement {'\n'} - {annotateWithTurnBoundaries([...history.previousRounds, ...history.newRounds]).map(item => ( + {[...history.previousRounds, ...history.newRounds].map(round => ( ))} @@ -250,12 +229,10 @@ export class BackgroundTodoPrompt extends PromptElement {'\n'} - {annotateWithTurnBoundaries(history.previousRounds).map(item => ( + {history.previousRounds.map(round => ( ))} diff --git a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts index 959a1f64a4e726..5f7741dd5e4088 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts @@ -83,7 +83,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('returns Wait when processor is already InProgress', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, currentTurnSubstantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; processor.start( { userRequest: 'old', newRounds: [makeMeaningfulRound('r0')], history: [], sessionResource: undefined, metadata: dummyMeta }, async () => { @@ -128,7 +128,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('skips when processor has already created todos and no new activity', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, currentTurnSubstantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; // Simulate a successful pass processor.start( { userRequest: 'old', newRounds: [makeMeaningfulRound('r0')], history: [], sessionResource: undefined, metadata: dummyMeta }, @@ -199,7 +199,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('after first pass, waits until subsequent threshold is met', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, currentTurnSubstantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false }; // Simulate a successful first pass so hasCreatedTodos becomes true. processor.start( { userRequest: 'old', newRounds: Array.from({ length: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD }, (_, i) => makeMeaningfulRound(`r${i}`)), history: [], sessionResource: undefined, metadata: dummyMeta }, @@ -227,7 +227,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('subsequent threshold is met by any mix of substantive calls', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, currentTurnSubstantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false }; processor.start( { userRequest: 'old', newRounds: Array.from({ length: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD }, (_, i) => makeMeaningfulRound(`r${i}`)), history: [], sessionResource: undefined, metadata: dummyMeta }, async () => ({ outcome: 'success' }) @@ -288,7 +288,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('hasCreatedTodos becomes true after successful pass', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, currentTurnSubstantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; processor.start( { userRequest: 'test', newRounds: [makeMeaningfulRound('r1')], history: [], sessionResource: undefined, metadata: dummyMeta }, async () => ({ outcome: 'success' }) @@ -299,7 +299,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { test('hasCreatedTodos stays false after noop pass', async () => { const processor = new BackgroundTodoProcessor(); - const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; + const dummyMeta = { newRoundCount: 1, newToolCallCount: 1, substantiveToolCallCount: 1, currentTurnSubstantiveToolCallCount: 1, isInitialDelta: true, isRequestOnly: false }; processor.start( { userRequest: 'test', newRounds: [makeMeaningfulRound('r1')], history: [], sessionResource: undefined, metadata: dummyMeta }, async () => ({ outcome: 'noop' }) diff --git a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoProcessor.spec.ts b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoProcessor.spec.ts index b2aeeb2273ddbe..185ad6ce3109c6 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoProcessor.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoProcessor.spec.ts @@ -23,6 +23,7 @@ function makeDelta(rounds: string[] = []): IBackgroundTodoDelta { newRoundCount: rounds.length, newToolCallCount: 0, substantiveToolCallCount: 0, + currentTurnSubstantiveToolCallCount: 0, isInitialDelta: true, isRequestOnly: rounds.length === 0, }, @@ -269,6 +270,39 @@ describe('BackgroundTodoProcessor', () => { expect(processor.state).toBe(BackgroundTodoProcessorState.Idle); }); + test('requestFinalReview skips when execution context is from a different turn', async () => { + const processor = new BackgroundTodoProcessor(); + // Simulate a successful pass so hasCreatedTodos becomes true + processor.start(makeDelta(['r1']), async () => ({ outcome: 'success' })); + await processor.waitForCompletion(); + expect(processor.hasCreatedTodos).toBe(true); + + // Record context for turn-1 + processor.requestRegularPass(makeDelta(['r2']), makeExecutionContext(['r2']), undefined, 'turn-1'); + await processor.waitForCompletion(); + + // Request final review for turn-2 — should skip because context is from turn-1 + processor.requestFinalReview('turn-2'); + expect(processor.state).toBe(BackgroundTodoProcessorState.Idle); + }); + + test('requestFinalReview runs when turn IDs match', async () => { + const processor = new BackgroundTodoProcessor(); + // Simulate a successful pass so hasCreatedTodos becomes true + processor.start(makeDelta(['r1']), async () => ({ outcome: 'success' })); + await processor.waitForCompletion(); + expect(processor.hasCreatedTodos).toBe(true); + + // Record context for turn-1 + processor.requestRegularPass(makeDelta(['r2']), makeExecutionContext(['r2']), undefined, 'turn-1'); + await processor.waitForCompletion(); + + // Request final review for turn-1 — should run + processor.requestFinalReview('turn-1'); + expect(processor.state).toBe(BackgroundTodoProcessorState.InProgress); + await processor.waitForCompletion(); + }); + test('requestFinalReview drains after a regular pass completes', async () => { const logMessages: string[] = []; const telemetryEvents: string[] = []; From c34016f3230af0579507239b2369e20558f114b1 Mon Sep 17 00:00:00 2001 From: vritant24 Date: Tue, 12 May 2026 10:07:31 -0700 Subject: [PATCH 05/43] fix type --- .../prompts/node/agent/test/backgroundTodoPolicy.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts index 5f7741dd5e4088..acf8db6b7ba63f 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/agent/test/backgroundTodoPolicy.spec.ts @@ -319,6 +319,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { newRoundCount: firstBatchRounds.length, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, + currentTurnSubstantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false, }; @@ -346,6 +347,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { newRoundCount: firstBatchRounds.length, newToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, substantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, + currentTurnSubstantiveToolCallCount: BackgroundTodoProcessor.INITIAL_SUBSTANTIVE_THRESHOLD, isInitialDelta: true, isRequestOnly: false, }; @@ -376,6 +378,7 @@ describe('BackgroundTodoProcessor.shouldRun (policy)', () => { newRoundCount: rounds.length, newToolCallCount: threshold, substantiveToolCallCount: threshold, + currentTurnSubstantiveToolCallCount: threshold, isInitialDelta: batchIdx === 0, isRequestOnly: false, }; From 9e7a6245c6b3cecbea2a20ca4770e680adb7aae3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 22 May 2026 04:24:35 +1000 Subject: [PATCH 06/43] Enhance session customization handling and simplify sync logic (#317700) * feat: enhance session customization handling and state management across agents * refactor: simplify customization sync logic in CopilotAgent * fix: guard initial customization publish for disposed sessions Co-authored-by: DonJayamanne <1948812+DonJayamanne@users.noreply.github.com> * feat: improve initial customization handling during session creation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- .../platform/agentHost/node/agentService.ts | 27 ++++- .../agentHost/node/copilot/copilotAgent.ts | 28 +++++- .../browser/baseAgentHostSessionsProvider.ts | 90 ++++++++++++++++- .../localAgentHostSessionsProvider.test.ts | 98 +++++++++++++++++++ .../agentHost/agentHostSessionHandler.ts | 7 ++ .../agentHostChatContribution.test.ts | 23 +++-- 6 files changed, 257 insertions(+), 16 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index ebbb8cd52a61d6..abb6f29b6af726 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -415,7 +415,26 @@ export class AgentService extends Disposable implements IAgentService { this._sessionToProvider.set(session.toString(), provider.id); this._logService.trace(`[AgentService] createSession returned: ${session.toString()}`); - const sessionConfig = await this._resolveCreatedSessionConfig(provider, config); + // Resolve config and seed the initial customization set in parallel so + // both are available before we register the session in the state + // manager. Seeding `state.customizations` directly (instead of + // dispatching `SessionCustomizationsChanged` after the fact) means + // the very first snapshot a subscriber sees already contains + // host/global customizations and the custom agents they contribute, + // so the agent picker doesn't have to wait for a follow-up republish + // (`RootConfigChanged`, plugin reload, or the first message's + // `setClientCustomizations`). Subsequent updates flow through the + // existing `SessionCustomizationsChanged` / `SessionCustomizationUpdated` + // actions published by `PluginController`. + const [sessionConfig, initialCustomizations] = await Promise.all([ + this._resolveCreatedSessionConfig(provider, config), + provider.getSessionCustomizations + ? provider.getSessionCustomizations(session).catch(err => { + this._logService.error('[AgentService] createSession: failed to resolve initial customizations', err); + return undefined; + }) + : Promise.resolve(undefined), + ]); // When forking, populate the new session's protocol state with // the source session's turns so the client sees the forked history. @@ -432,6 +451,9 @@ export class AgentService extends Disposable implements IAgentService { state.config = sessionConfig; state.turns = sourceTurns; state.activeClient = config.activeClient; + if (initialCustomizations && initialCustomizations.length > 0) { + state.customizations = [...initialCustomizations]; + } } else { // Provisional sessions defer the `sessionAdded` notification and // the `SessionReady` lifecycle transition until the agent fires @@ -443,6 +465,9 @@ export class AgentService extends Disposable implements IAgentService { const state = this._stateManager.createSession(summary, { emitNotification: !created.provisional }); state.config = sessionConfig; state.activeClient = config?.activeClient; + if (initialCustomizations && initialCustomizations.length > 0) { + state.customizations = [...initialCustomizations]; + } } // Persist initial config values so a subsequent `restoreSession` can // re-hydrate them. We persist the full resolved values (not just the diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index e8546189e922b9..a17d394ef628b2 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -313,7 +313,7 @@ export class CopilotAgent extends Disposable implements IAgent { } async getSessionCustomizations(session: URI): Promise { - return this._plugins.getSessionCustomizations(await this._getSessionCustomizationDirectory(session)); + return this._plugins.getSessionCustomizationsSettled(await this._getSessionCustomizationDirectory(session)); } private async _getSessionCustomizationDirectory(session: URI): Promise { @@ -1949,6 +1949,32 @@ class PluginController extends Disposable { return result; } + /** + * Settled variant of {@link getSessionCustomizations}: awaits the + * in-flight host sync, the in-flight client sync, and (when a directory + * is supplied) the session-discovered entry's initial scan + parse + * before snapshotting the customization list. + * + * Callers that publish customizations into session state at session + * creation time MUST use this — the synchronous variant can return an + * empty list for a brand-new working directory because + * {@link SessionDiscoveredEntry} kicks off its `_refresh()` in its + * constructor without anyone awaiting it. + */ + public async getSessionCustomizationsSettled(directory: URI | undefined): Promise { + const entry = directory ? this._getOrCreateSessionEntry(directory) : undefined; + await Promise.all([ + this._hostSync.catch(err => { + this._logService.warn('[Copilot:PluginController] Host customization update failed', err); + }), + this._clientSync.catch(err => { + this._logService.warn('[Copilot:PluginController] Customization sync failed', err); + }), + entry?.whenSettled(), + ]); + return this.getSessionCustomizations(directory); + } + /** * Returns the current parsed plugins, awaiting any pending sync. */ diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index bc13266a6801c6..9ec79813c0b8ae 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -23,6 +23,7 @@ import { ResolveSessionConfigResult } from '../../../../../platform/agentHost/co import { AgentSelection, CustomizationAgentRef, ModelSelection, SessionStatus as ProtocolSessionStatus, RootConfigState, RootState, SessionState, SessionSummary, type ChangesetSummary } from '../../../../../platform/agentHost/common/state/protocol/state.js'; import { ActionType, isSessionAction, NotificationType } from '../../../../../platform/agentHost/common/state/sessionActions.js'; import { readSessionGitState, ROOT_STATE_URI, SessionMeta, StateComponents, type ISessionGitState } from '../../../../../platform/agentHost/common/state/sessionState.js'; +import type { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; @@ -474,6 +475,16 @@ interface INewSessionConstructionContext { * list as the committed session that replaces it. */ readonly instantiationService: IInstantiationService; + /** + * Forwards `SessionState` snapshots from the eagerly-held wire + * subscription back to the provider. `state === undefined` is a + * cleanup sentinel emitted by {@link NewSession.dispose} on the + * close-without-graduation path so the provider can drop any cached + * entry it accumulated for this session. The graduation path skips + * this sentinel because the running-session subscription pipeline + * takes over ownership of the same `sessionId` key. + */ + readonly onSessionState?: (sessionId: string, state: SessionState | undefined) => void; } /** @@ -531,7 +542,16 @@ class NewSession extends Disposable { /** Connection used to create the backend session, captured for `disposeSession` on tear-down. */ private _connection: IAgentConnection | undefined; /** Held state subscription. Set after the wire `createSession` resolves. */ - private _subscription: IReference | undefined; + private _subscription: IReference> | undefined; + /** + * `onDidChange` listener for {@link _subscription}. Forwards every + * `SessionState` snapshot to the provider via {@link _onSessionState} + * so the new session's customizations (and any other state) reach + * `_lastSessionStates` while the session is still Untitled. Detached + * in {@link graduate} (handoff) and {@link dispose} (close-without-send). + */ + private readonly _stateListener = this._register(new MutableDisposable()); + private readonly _onSessionState: ((sessionId: string, state: SessionState | undefined) => void) | undefined; private readonly _logService: ILogService; private readonly _providerId: string; @@ -546,6 +566,7 @@ class NewSession extends Disposable { this.agentProvider = ctx.sessionType.id; this._providerId = ctx.providerId; this._logService = ctx.logService; + this._onSessionState = ctx.onSessionState; const resource = URI.from({ scheme: ctx.resourceScheme, path: `/${generateUuid()}` }); this._status = observableValue(this, SessionStatus.Untitled); @@ -747,7 +768,23 @@ class NewSession extends Disposable { // handler refcounts the same subscription via `getSubscription` // when chat content opens, so when we release this ref on // graduation the wire-level refcount stays positive. - this._subscription = connection.getSubscription(StateComponents.Session, backendUri); + const ref = connection.getSubscription(StateComponents.Session, backendUri); + this._subscription = ref; + + // Forward `SessionState` updates back to the provider so + // `_lastSessionStates` (and therefore `getCustomAgents`) becomes + // populated for this still-Untitled session. Seed once from the + // cached value, then attach a listener for subsequent deltas. + const onSessionState = this._onSessionState; + if (onSessionState) { + const initial = ref.object.value; + if (initial && !(initial instanceof Error)) { + onSessionState(this.sessionId, initial); + } + this._stateListener.value = ref.object.onDidChange(state => { + onSessionState(this.sessionId, state); + }); + } })(); } @@ -757,6 +794,12 @@ class NewSession extends Disposable { * graduated into a real running session. */ graduate(): void { + // Detach the new-session listener BEFORE releasing the subscription. + // Both code paths (this one and the running-session pipeline) write + // `_lastSessionStates` under the same `sessionId` key, so detaching + // here hands ownership cleanly to `_ensureSessionStateSubscription` + // without a transient empty-read window or a duplicate writer. + this._stateListener.clear(); this._subscription?.dispose(); this._subscription = undefined; this._backendUri = undefined; @@ -768,6 +811,18 @@ class NewSession extends Disposable { // Bump the seq so any in-flight resolveConfig discards itself. this._configRequestSeq++; + // Detach the state listener BEFORE firing the cleanup sentinel so + // a racing `onDidChange` cannot re-populate `_lastSessionStates` + // after we have asked the provider to delete the entry. Then fire + // the sentinel so the provider drops the cached snapshot. Only + // fires when a listener was actually wired (i.e. `eagerCreate` + // reached the post-`createSession` branch). + const hadListener = !!this._stateListener.value; + this._stateListener.clear(); + if (hadListener) { + this._onSessionState?.(this.sessionId, undefined); + } + this._subscription?.dispose(); this._subscription = undefined; @@ -1125,6 +1180,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement logService: this._logService, initialConfigValues: this._initialNewSessionConfig(), instantiationService: this._instantiationService, + onSessionState: (id, state) => state === undefined + ? this._handleNewSessionStateGone(id) + : this._handleNewSessionStateUpdate(id, state), }); this._newSession = newSession; this._onDidChangeSessionConfig.fire(newSession.sessionId); @@ -1757,6 +1815,34 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement this._applySessionMetaFromState(sessionId, state); } + /** + * NewSession variant of {@link _applySessionStateUpdate}: writes the + * customizations subset (the only one the agent picker reads) and + * fires `_onDidChangeCustomAgents` when it changes. Skips + * {@link _seedRunningConfigFromState} (NewSession owns its own config + * via `NewSession._config`) and {@link _applySessionMetaFromState} + * (which only applies to cached running sessions). + */ + private _handleNewSessionStateUpdate(sessionId: string, state: SessionState): void { + const previous = this._lastSessionStates.get(sessionId); + this._lastSessionStates.set(sessionId, state); + if (previous?.customizations !== state.customizations || previous?.activeClient?.customizations !== state.activeClient?.customizations) { + this._onDidChangeCustomAgents.fire(); + } + } + + /** + * Cleanup sentinel from {@link NewSession.dispose}: drops the cached + * `_lastSessionStates` entry the new session contributed. Fires + * `_onDidChangeCustomAgents` so any open picker re-reads and falls + * back to the empty list rather than rendering stale agents. + */ + private _handleNewSessionStateGone(sessionId: string): void { + if (this._lastSessionStates.delete(sessionId)) { + this._onDidChangeCustomAgents.fire(); + } + } + private _applySessionMetaFromState(sessionId: string, state: SessionState): void { const rawId = this._rawIdFromChatId(sessionId); if (!rawId) { diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index e6366378606cfd..df22398e2e77f4 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -852,6 +852,104 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(fired, afterFirstCustomization, 'expected event NOT to fire when customizations are unchanged'); }); + test('NewSession forwards SessionState into _lastSessionStates so the picker sees customizations before first message', async () => { + const provider = createProvider(disposables, agentHost); + const sessionTypeId = provider.sessionTypes[0].id; + const session = provider.createNewSession(URI.parse('file:///home/user/proj'), sessionTypeId); + await timeout(0); // let eagerCreate complete and the subscription seed + + const rawId = session.resource.path.substring(1); + + let fired = 0; + disposables.add(provider.onDidChangeCustomAgents(() => { fired++; })); + + // Push a SessionState carrying customizations as if the host had + // resolved them and dispatched a SessionCustomizationsChanged. + const customizations = [{ + customization: { uri: 'plugin://new-session', displayName: 'p' }, + enabled: true, + status: CustomizationStatus.Loaded, + agents: [ + { uri: 'agent://reviewer', name: 'reviewer' }, + { uri: 'agent://triage', name: 'triage' }, + ], + }]; + const state: SessionState = { + summary: { + resource: AgentSession.uri(sessionTypeId, rawId).toString(), + provider: sessionTypeId, + title: '', + status: ProtocolSessionStatus.Idle, + createdAt: 0, + modifiedAt: 0, + }, + lifecycle: SessionLifecycle.Ready, + turns: [], + customizations, + }; + agentHost.setSessionState(rawId, sessionTypeId, state); + + assert.deepStrictEqual(provider.getCustomAgents(session.sessionId), [ + { uri: 'agent://reviewer', name: 'reviewer' }, + { uri: 'agent://triage', name: 'triage' }, + ]); + assert.ok(fired > 0, 'expected onDidChangeCustomAgents to fire when SessionState arrives'); + + // A second update with a different customizations identity should + // re-fire and update the picker. + const after = fired; + agentHost.setSessionState(rawId, sessionTypeId, { + ...state, + customizations: [{ + ...customizations[0], + agents: [{ uri: 'agent://only', name: 'only' }], + }], + }); + assert.deepStrictEqual(provider.getCustomAgents(session.sessionId), [ + { uri: 'agent://only', name: 'only' }, + ]); + assert.ok(fired > after, 'expected onDidChangeCustomAgents to fire again on a second update'); + }); + + test('NewSession dispose clears _lastSessionStates entry and fires onDidChangeCustomAgents', async () => { + const provider = createProvider(disposables, agentHost); + const sessionTypeId = provider.sessionTypes[0].id; + const first = provider.createNewSession(URI.parse('file:///home/user/a'), sessionTypeId); + await timeout(0); + + const rawId = first.resource.path.substring(1); + agentHost.setSessionState(rawId, sessionTypeId, { + summary: { + resource: AgentSession.uri(sessionTypeId, rawId).toString(), + provider: sessionTypeId, + title: '', + status: ProtocolSessionStatus.Idle, + createdAt: 0, + modifiedAt: 0, + }, + lifecycle: SessionLifecycle.Ready, + turns: [], + customizations: [{ + customization: { uri: 'plugin://x', displayName: 'p' }, + enabled: true, + status: CustomizationStatus.Loaded, + agents: [{ uri: 'agent://x', name: 'x' }], + }], + }); + assert.strictEqual(provider.getCustomAgents(first.sessionId).length, 1); + + let fired = 0; + disposables.add(provider.onDidChangeCustomAgents(() => { fired++; })); + + // Trigger disposal of the first NewSession by creating a second one + // (the MutableDisposable holding `_newSession` disposes the previous). + provider.createNewSession(URI.parse('file:///home/user/b'), sessionTypeId); + await timeout(0); + + assert.deepStrictEqual(provider.getCustomAgents(first.sessionId), []); + assert.ok(fired > 0, 'expected onDidChangeCustomAgents to fire on NewSession dispose'); + }); + // ---- Session lifecycle ------- test('createNewSession returns session with correct fields', () => { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index f0578aeb6c6937..ae18f375698cd6 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -693,6 +693,13 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC if (!isNewSession) { this._ensurePendingMessageSubscription(sessionResource, resolvedSession); + // Claim active client up-front so the host syncs this client's + // customizations into session state immediately. Without this, + // session-scoped customizations (and the agents derived from + // them) wouldn't land until the user sent their first message, + // leaving the agent picker empty on session open. + this._ensureActiveClientForMessage(resolvedSession); + // If there are historical turns with file edits, eagerly create // the editing session once the ChatModel is available so that // edit pills render with diff info on session restore. diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index 706862fcc56f0b..ab5ba9436077ad 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -4426,7 +4426,7 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(ac.activeClient.customizations?.[0].uri, 'file:///plugin-b'); }); - test('does not dispatch activeClientChanged when an existing session is restored', async () => { + test('does not dispatch activeClientChanged when an existing session is restored and this client is already active', async () => { const { instantiationService, agentHostService } = createTestServices(disposables); const sessionResource = AgentSession.uri('copilot', 'existing-session'); const summary: SessionSummary = { @@ -4441,8 +4441,9 @@ suite('AgentHostChatContribution', () => { ...createSessionState(summary), lifecycle: SessionLifecycle.Ready, activeClient: { - clientId: 'other-client', + clientId: agentHostService.clientId, tools: [], + customizations: [], }, }); @@ -4465,8 +4466,8 @@ suite('AgentHostChatContribution', () => { ); }); - test('dispatches activeClientChanged before sending a message when another client is active', async () => { - const { instantiationService, agentHostService, chatAgentService } = createTestServices(disposables); + test('dispatches activeClientChanged when restoring a session where another client is active', async () => { + const { instantiationService, agentHostService } = createTestServices(disposables); const sessionResource = AgentSession.uri('copilot', 'existing-session'); const summary: SessionSummary = { resource: sessionResource.toString(), @@ -4495,17 +4496,16 @@ suite('AgentHostChatContribution', () => { connectionAuthority: 'local', })); - const { turnPromise, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables, { sessionResource }); - fire({ type: 'session/turnComplete', session, turnId } as SessionAction); - await turnPromise; + const chatSession = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None); + disposables.add(toDisposable(() => chatSession.dispose())); const activeClientActions = agentHostService.dispatchedActions.filter(d => d.action.type === 'session/activeClientChanged'); assert.strictEqual(activeClientActions.length, 1); assert.strictEqual(activeClientActions[0].channel, sessionResource.toString()); }); - test('dispatches activeClientChanged before sending a message when current client customizations are stale', async () => { - const { instantiationService, agentHostService, chatAgentService } = createTestServices(disposables); + test('dispatches activeClientChanged when restoring a session where current client customizations are stale', async () => { + const { instantiationService, agentHostService } = createTestServices(disposables); const customizations = observableValue('customizations', [ { uri: 'file:///plugin-new', displayName: 'Plugin New' }, ]); @@ -4539,9 +4539,8 @@ suite('AgentHostChatContribution', () => { customizations, })); - const { turnPromise, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables, { sessionResource }); - fire({ type: 'session/turnComplete', session, turnId } as SessionAction); - await turnPromise; + const chatSession = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None); + disposables.add(toDisposable(() => chatSession.dispose())); const activeClientActions = agentHostService.dispatchedActions.filter(d => d.action.type === 'session/activeClientChanged'); assert.strictEqual(activeClientActions.length, 1); From bed9e7518bc1cb0254f76987fd31128180c470d1 Mon Sep 17 00:00:00 2001 From: Matt Bierner <12821956+mjbvz@users.noreply.github.com> Date: Thu, 21 May 2026 11:08:41 -0700 Subject: [PATCH 07/43] Show a warning when showing the rendered markdown diff Our current implementation has some bugs that can hide changes. However even after fixing these, I'm not 100% confident a unexpected change wont sneak through. Best to make users aware of this while also trying to prevent this --- .../src/extension.shared.ts | 2 +- .../src/preview/documentRenderer.ts | 8 +- .../src/preview/preview.ts | 17 +++- .../src/preview/previewManager.ts | 17 +++- .../src/preview/renderedDiffWarning.ts | 91 +++++++++++++++++++ 5 files changed, 125 insertions(+), 10 deletions(-) create mode 100644 extensions/markdown-language-features/src/preview/renderedDiffWarning.ts diff --git a/extensions/markdown-language-features/src/extension.shared.ts b/extensions/markdown-language-features/src/extension.shared.ts index 6f245bd8ab708e..c9b1184062aca6 100644 --- a/extensions/markdown-language-features/src/extension.shared.ts +++ b/extensions/markdown-language-features/src/extension.shared.ts @@ -39,7 +39,7 @@ export function activateShared( const opener = new MdLinkOpener(client); const contentProvider = new MdDocumentRenderer(engine, context, cspArbiter, contributions, logger); - const previewManager = new MarkdownPreviewManager(contentProvider, logger, contributions, opener); + const previewManager = new MarkdownPreviewManager(contentProvider, logger, contributions, opener, context.workspaceState); context.subscriptions.push(previewManager); context.subscriptions.push(registerMarkdownLanguageFeatures(client, commandManager, engine)); diff --git a/extensions/markdown-language-features/src/preview/documentRenderer.ts b/extensions/markdown-language-features/src/preview/documentRenderer.ts index 85407fb81bf633..e29e37ede74567 100644 --- a/extensions/markdown-language-features/src/preview/documentRenderer.ts +++ b/extensions/markdown-language-features/src/preview/documentRenderer.ts @@ -27,7 +27,7 @@ const previewStrings = { cspAlertMessageTitle: vscode.l10n.t("Potentially unsafe or insecure content has been disabled in the Markdown preview. Change the Markdown preview security setting to allow insecure content or enable scripts"), - cspAlertMessageLabel: vscode.l10n.t("Content Disabled Security Warning") + cspAlertMessageLabel: vscode.l10n.t("Content Disabled Security Warning"), }; export interface MarkdownContentProviderOutput { @@ -44,14 +44,14 @@ export interface ImageInfo { export class MdDocumentRenderer { readonly #engine: MarkdownItEngine; - readonly #context: vscode.ExtensionContext; + readonly #context: Pick; readonly #cspArbiter: ContentSecurityPolicyArbiter; readonly #contributionProvider: MarkdownContributionProvider; readonly #logger: ILogger; constructor( engine: MarkdownItEngine, - context: vscode.ExtensionContext, + context: Pick, cspArbiter: ContentSecurityPolicyArbiter, contributionProvider: MarkdownContributionProvider, logger: ILogger @@ -139,7 +139,7 @@ export class MdDocumentRenderer { ): Promise { const innerChanges = lineChanges?.innerChanges; - // If there are inner changes, inject empty marker spans into the source text + // If there are inner changes, inject invisible marker text into the source text // before rendering. The webview uses the CSS Custom Highlight API to create // highlights between each marker pair, which works across HTML tag boundaries. const input: vscode.TextDocument | string = innerChanges?.length diff --git a/extensions/markdown-language-features/src/preview/preview.ts b/extensions/markdown-language-features/src/preview/preview.ts index 1fa1eede17a378..ebafa050daf453 100644 --- a/extensions/markdown-language-features/src/preview/preview.ts +++ b/extensions/markdown-language-features/src/preview/preview.ts @@ -79,6 +79,7 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider { readonly #resource: vscode.Uri; readonly #webviewPanel: vscode.WebviewPanel; + readonly #isDiffView: boolean; #line: number | undefined; readonly #scrollToFragment: string | undefined; @@ -126,7 +127,8 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider { this.#webviewPanel = webview; this.#resource = resource; - this.#scrollToFirstDiffChange = !startingScroll && !!delegate.getLineChanges; + this.#isDiffView = !!delegate.getLineChanges; + this.#scrollToFirstDiffChange = !startingScroll && this.#isDiffView; switch (startingScroll?.type) { case 'line': @@ -223,6 +225,10 @@ class MarkdownPreview extends Disposable implements WebviewResourceProvider { return this.#resource; } + public get isDiffView(): boolean { + return this.#isDiffView; + } + public get state() { return { resource: this.#resource.toString(), @@ -519,6 +525,7 @@ export interface IManagedMarkdownPreview { readonly resource: vscode.Uri; readonly resourceColumn: vscode.ViewColumn; + readonly isDiffView: boolean; readonly onDispose: vscode.Event; readonly onDidChangeViewState: vscode.Event; @@ -666,6 +673,10 @@ export class StaticMarkdownPreview extends Disposable implements IManagedMarkdow public get resourceColumn() { return this.#webviewPanel.viewColumn || vscode.ViewColumn.One; } + + public get isDiffView(): boolean { + return this.#preview.isDiffView; + } } interface DynamicPreviewInput { @@ -824,6 +835,10 @@ export class DynamicMarkdownPreview extends Disposable implements IManagedMarkdo return this.#resourceColumn; } + public get isDiffView(): boolean { + return this.#preview.isDiffView; + } + public reveal(viewColumn: vscode.ViewColumn) { this.#webviewPanel.reveal(viewColumn); } diff --git a/extensions/markdown-language-features/src/preview/previewManager.ts b/extensions/markdown-language-features/src/preview/previewManager.ts index f5f8bf0da5d016..81f71faf623b83 100644 --- a/extensions/markdown-language-features/src/preview/previewManager.ts +++ b/extensions/markdown-language-features/src/preview/previewManager.ts @@ -14,6 +14,7 @@ import { MdDocumentRenderer } from './documentRenderer'; import { MarkdownPreviewLineDiffProvider } from './lineDiff'; import { DynamicMarkdownPreview, IManagedMarkdownPreview, StaticMarkdownPreview } from './preview'; import { MarkdownPreviewConfigurationManager } from './previewConfig'; +import { RenderedDiffWarningManager } from './renderedDiffWarning'; import { scrollEditorToLine, StartingScrollFragment, StartingScrollLine, StartingScrollLocation } from './scrolling'; import { getVisibleLine, TopmostLineMonitor } from './topmostLineMonitor'; import type { DiffScrollSyncData, MarkdownPreviewLineChanges } from '../../types/previewMessaging'; @@ -86,12 +87,14 @@ export class MarkdownPreviewManager extends Disposable implements vscode.Webview readonly #logger: ILogger; readonly #contributions: MarkdownContributionProvider; readonly #opener: MdLinkOpener; + readonly #renderedDiffWarning: RenderedDiffWarningManager; public constructor( contentProvider: MdDocumentRenderer, logger: ILogger, contributions: MarkdownContributionProvider, opener: MdLinkOpener, + workspaceState: vscode.Memento, ) { super(); @@ -99,6 +102,7 @@ export class MarkdownPreviewManager extends Disposable implements vscode.Webview this.#logger = logger; this.#contributions = contributions; this.#opener = opener; + this.#renderedDiffWarning = this._register(new RenderedDiffWarningManager(workspaceState)); this._register(vscode.window.registerWebviewPanelSerializer(DynamicMarkdownPreview.viewType, this)); @@ -310,7 +314,7 @@ export class MarkdownPreviewManager extends Disposable implements vscode.Webview getDiffScrollSync ); this.#registerStaticPreview(preview); - this.#activePreview = preview; + this.#setActivePreview(preview); return preview; } @@ -343,7 +347,7 @@ export class MarkdownPreviewManager extends Disposable implements vscode.Webview this.#contributions, this.#opener); - this.#activePreview = preview; + this.#setActivePreview(preview); return this.#registerDynamicPreview(preview); } @@ -386,14 +390,19 @@ export class MarkdownPreviewManager extends Disposable implements vscode.Webview #trackActive(preview: IManagedMarkdownPreview): void { preview.onDidChangeViewState(({ webviewPanel }) => { - this.#activePreview = webviewPanel.active ? preview : undefined; + this.#setActivePreview(webviewPanel.active ? preview : undefined); }); preview.onDispose(() => { if (this.#activePreview === preview) { - this.#activePreview = undefined; + this.#setActivePreview(undefined); } }); } + #setActivePreview(preview: IManagedMarkdownPreview | undefined): void { + this.#activePreview = preview; + this.#renderedDiffWarning.setActiveDiffPreview(!!preview?.isDiffView); + } + } diff --git a/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts b/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts new file mode 100644 index 00000000000000..ccdebae1541c9b --- /dev/null +++ b/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts @@ -0,0 +1,91 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { Disposable } from '../util/dispose'; + +const suppressedStorageKey = 'markdown.preview.renderedDiffWarning.suppressed'; +const notificationShownStorageKey = 'markdown.preview.renderedDiffWarning.notificationShown'; + +export class RenderedDiffWarningManager extends Disposable { + + readonly #workspaceState: vscode.Memento; + + #statusBarItem: vscode.StatusBarItem | undefined; + #hasActiveDiffPreview = false; + + readonly #showWarningCommandId = '_markdown.preview.showRenderedDiffWarning'; + + constructor(workspaceState: vscode.Memento) { + super(); + + this.#workspaceState = workspaceState; + + this._register(vscode.commands.registerCommand(this.#showWarningCommandId, () => { + void this.#showWarningNotification(); + })); + } + + override dispose(): void { + this.#statusBarItem?.dispose(); + this.#statusBarItem = undefined; + super.dispose(); + } + + /** + * Set whether a diff preview is currently the active editor. + * + * Drives the visibility of the status bar warning and triggers the one-time + * notification the first time the user focuses a diff preview. + */ + public setActiveDiffPreview(active: boolean): void { + if (this.#isSuppressed() || this.#hasActiveDiffPreview === active) { + return; + } + + this.#hasActiveDiffPreview = active; + this.#updateStatusBar(); + + if (active && !this.#workspaceState.get(notificationShownStorageKey, false)) { + void this.#workspaceState.update(notificationShownStorageKey, true); + void this.#showWarningNotification(); + } + } + + #updateStatusBar(): void { + if (this.#isSuppressed() || !this.#hasActiveDiffPreview) { + this.#statusBarItem?.dispose(); + this.#statusBarItem = undefined; + return; + } + + if (!this.#statusBarItem) { + this.#statusBarItem = vscode.window.createStatusBarItem('markdown.renderedDiffWarning', vscode.StatusBarAlignment.Right, 100); + this.#statusBarItem.name = vscode.l10n.t('Rendered Markdown Diff Warning'); + this.#statusBarItem.text = '$(warning) ' + vscode.l10n.t('Rendered Diff'); + this.#statusBarItem.tooltip = vscode.l10n.t('Rendered Markdown diffs may hide important changes. Click for details.'); + this.#statusBarItem.backgroundColor = new vscode.ThemeColor('statusBarItem.warningBackground'); + this.#statusBarItem.command = this.#showWarningCommandId; + } + this.#statusBarItem.show(); + } + + async #showWarningNotification(): Promise { + const dontShowAgain = vscode.l10n.t("Don't Show Again"); + const selected = await vscode.window.showWarningMessage( + vscode.l10n.t('Rendered Markdown diffs may hide important changes such as formatting, whitespace, links, or HTML. Switch to the text diff if you need to review them.'), + dontShowAgain, + ); + if (selected === dontShowAgain) { + await this.#workspaceState.update(suppressedStorageKey, true); + this.#hasActiveDiffPreview = false; + this.#updateStatusBar(); + } + } + + #isSuppressed(): boolean { + return this.#workspaceState.get(suppressedStorageKey, false); + } +} From b541e17dac658c33c77a7a9613a59e4185d6bf2b Mon Sep 17 00:00:00 2001 From: Taylor Blair Date: Thu, 21 May 2026 12:42:59 -0700 Subject: [PATCH 08/43] Add Model, ConversationId, and RequestId to the gent.tool.responselength telmetry event --- .../prompts/node/panel/chatVariables.tsx | 5 ++++- .../prompts/node/panel/test/toolCalling.spec.ts | 16 ++++++++++++++++ .../prompts/node/panel/toolCalling.tsx | 17 ++++++++++++++--- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/panel/chatVariables.tsx b/extensions/copilot/src/extension/prompts/node/panel/chatVariables.tsx index bb3fb0b4d16897..9d20d6c1f2d2a4 100644 --- a/extensions/copilot/src/extension/prompts/node/panel/chatVariables.tsx +++ b/extensions/copilot/src/extension/prompts/node/panel/chatVariables.tsx @@ -443,7 +443,10 @@ export class ChatToolReferences extends PromptElement { const name = toolReference.range ? this.props.promptContext.query.slice(toolReference.range[0], toolReference.range[1]) : undefined; try { const result = await this.toolsService.invokeToolWithEndpoint(tool.name, { input: { ...toolArgs, ...internalToolArgs }, toolInvocationToken: tools.toolInvocationToken }, this.promptEndpoint, token || CancellationToken.None); - sendInvokedToolTelemetry(this.instantiationService, this.promptEndpoint, this.telemetryService, tool.name, result); + sendInvokedToolTelemetry(this.instantiationService, this.promptEndpoint, this.telemetryService, tool.name, result, { + conversationId: this.props.promptContext.conversation?.sessionId, + requestId: this.props.promptContext.requestId, + }); results.push({ name, value: result }); } catch (err) { const errResult = toolCallErrorToResult(err); diff --git a/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts b/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts index c5833fc5aad9b4..3ca0e304885ca8 100644 --- a/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts @@ -14,6 +14,7 @@ import { Event } from '../../../../../util/vs/base/common/event'; import { constObservable } from '../../../../../util/vs/base/common/observable'; import { IInstantiationService } from '../../../../../util/vs/platform/instantiation/common/instantiation'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry'; +import type { SpyingTelemetryService } from '../../../../../platform/telemetry/node/spyingTelemetryService'; import { LanguageModelDataPart, LanguageModelTextPart, LanguageModelToolResult } from '../../../../../vscodeTypes'; import { ChatVariablesCollection } from '../../../../prompt/common/chatVariablesCollection'; import type { Conversation } from '../../../../prompt/common/conversation'; @@ -600,6 +601,7 @@ describe('ChatToolCalls (toolCalling.tsx)', () => { const endpointProvider = accessor.get(IEndpointProvider); const endpoint = await endpointProvider.getChatEndpoint('copilot-utility'); const telemetryService = accessor.get(ITelemetryService); + const spyingTelemetryService = telemetryService as SpyingTelemetryService; const configService = accessor.get(IConfigurationService); // Disable image uploads in test environment to avoid auth requirement @@ -625,10 +627,24 @@ describe('ChatToolCalls (toolCalling.tsx)', () => { telemetryService, 'testTool', toolResult, + { conversationId: 'conversation-id', requestId: 'request-id' }, ); }).not.toThrow(); // Give async rendering a moment to complete without unhandled rejection await new Promise(resolve => setTimeout(resolve, 100)); + + const telemetryEvent = spyingTelemetryService.getEvents().telemetryServiceEvents.find(event => event.eventName === 'agent.tool.responseLength'); + expect(telemetryEvent).toMatchObject({ + properties: { + conversationId: 'conversation-id', + requestId: 'request-id', + model: endpoint.model, + toolName: 'testTool', + }, + measurements: { + tokenCount: expect.any(Number), + }, + }); }); }); diff --git a/extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx b/extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx index 987d13418f2315..5d41940fd8420f 100644 --- a/extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx +++ b/extensions/copilot/src/extension/prompts/node/panel/toolCalling.tsx @@ -317,7 +317,10 @@ function buildToolResultElement(accessor: ServicesAccessor, props: ToolResultOpt } toolResult = await toolsService.invokeToolWithEndpoint(props.toolCall.name, invocationOptions, promptEndpoint, props.token); - sendInvokedToolTelemetry(instantiationService, promptEndpoint, telemetryService, props.toolCall.name, toolResult); + sendInvokedToolTelemetry(instantiationService, promptEndpoint, telemetryService, props.toolCall.name, toolResult, { + conversationId: promptContext.conversation?.sessionId, + requestId: props.requestId, + }); // Run hook context handling after tool execution appendHookContext(toolResult, hookResult, chatHookService, props, inputObj, promptContext); @@ -489,7 +492,12 @@ class ToolResultElement extends PromptElement Date: Fri, 22 May 2026 01:22:48 +0530 Subject: [PATCH 09/43] Merge pull request #307886 from yogeshwaran-c/feat/watch-copy-all-306116 feat(debug): add 'Copy All' action to Watch view context menu --- .../debug/browser/watchExpressionsView.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts index f290f709ae5e67..9fef1481a40a20 100644 --- a/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts +++ b/src/vs/workbench/contrib/debug/browser/watchExpressionsView.ts @@ -692,3 +692,30 @@ registerAction2(class CopyExpression extends ViewAction { } } }); + +export const COPY_ALL_WATCH_EXPRESSIONS_COMMAND_ID = 'workbench.debug.viewlet.action.copyAllWatchExpressions'; + +registerAction2(class CopyAllWatchExpressions extends ViewAction { + constructor() { + super({ + id: COPY_ALL_WATCH_EXPRESSIONS_COMMAND_ID, + title: localize('copyAllWatchExpressions', "Copy All"), + f1: false, + viewId: WATCH_VIEW_ID, + precondition: CONTEXT_WATCH_EXPRESSIONS_EXIST, + menu: { + id: MenuId.DebugWatchContext, + order: 45, + group: '3_modification' + } + }); + } + + runInView(accessor: ServicesAccessor): void { + const clipboardService = accessor.get(IClipboardService); + const debugService = accessor.get(IDebugService); + const watches = debugService.getModel().getWatchExpressions(); + const lines = watches.map(w => `${w.name}: ${w.value}`); + clipboardService.writeText(lines.join('\n')); + } +}); From b0bad8c45839cce34cb2b86c3c4c36b946f374ee Mon Sep 17 00:00:00 2001 From: Taylor Blair Date: Thu, 21 May 2026 12:55:22 -0700 Subject: [PATCH 10/43] Add turn to the GHCP telmetry event response.success --- .../extension/byok/vscode-node/anthropicProvider.ts | 1 + .../byok/vscode-node/geminiNativeProvider.ts | 1 + .../extension/prompt/node/chatMLFetcherTelemetry.ts | 12 ++++++++++++ 3 files changed, 14 insertions(+) diff --git a/extensions/copilot/src/extension/byok/vscode-node/anthropicProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/anthropicProvider.ts index 548962097c938e..1f3e1ff17e4f43 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/anthropicProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/anthropicProvider.ts @@ -398,6 +398,7 @@ export class AnthropicLMProvider extends AbstractLanguageModelChatProvider { "requestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Id of the current turn request" }, "gitHubRequestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "GitHub request id if available" }, "associatedRequestId": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Another request ID that this request is associated with (eg, the originating request of a summarization request)." }, + "turn": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "How many turns have been made in the conversation.", "isMeasurement": true }, "reasoningEffort": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning effort level" }, "reasoningSummary": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning summary level" }, "fetcher": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "The fetcher used for the request" }, diff --git a/extensions/copilot/src/extension/byok/vscode-node/geminiNativeProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/geminiNativeProvider.ts index 0cb51a2bb7d981..1be258e9a61ac4 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/geminiNativeProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/geminiNativeProvider.ts @@ -269,6 +269,7 @@ export class GeminiNativeBYOKLMProvider extends AbstractLanguageModelChatProvide "requestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Id of the current turn request" }, "gitHubRequestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "GitHub request id if available" }, "associatedRequestId": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Another request ID that this request is associated with (eg, the originating request of a summarization request)." }, + "turn": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "How many turns have been made in the conversation.", "isMeasurement": true }, "reasoningEffort": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning effort level" }, "reasoningSummary": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning summary level" }, "fetcher": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "The fetcher used for the request" }, diff --git a/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts b/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts index d9b6e18ef7762e..ff80b7256ad35b 100644 --- a/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts +++ b/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts @@ -89,6 +89,16 @@ export interface IChatMLFetcherErrorData { resumeEventSeen: boolean | undefined; } +function getTurnFromBaseTelemetry(baseTelemetry: TelemetryData): number | undefined { + const turnIndex = baseTelemetry.properties.turnIndex; + if (typeof turnIndex !== 'string') { + return undefined; + } + + const parsedTurnIndex = Number(turnIndex); + return Number.isFinite(parsedTurnIndex) ? parsedTurnIndex + 1 : undefined; +} + export class ChatMLFetcherTelemetrySender { public static sendSuccessTelemetry( @@ -130,6 +140,7 @@ export class ChatMLFetcherTelemetrySender { "requestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Id of the current turn request" }, "gitHubRequestId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "GitHub request id if available" }, "associatedRequestId": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "Another request ID that this request is associated with (eg, the originating request of a summarization request)." }, + "turn": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "comment": "How many turns have been made in the conversation.", "isMeasurement": true }, "reasoningEffort": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning effort level" }, "reasoningSummary": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reasoning summary level" }, "fetcher": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "The fetcher used for the request" }, @@ -207,6 +218,7 @@ export class ChatMLFetcherTelemetrySender { ...(baseTelemetry?.properties.connectivityTestErrorGitHubRequestId ? { connectivityTestErrorGitHubRequestId: baseTelemetry.properties.connectivityTestErrorGitHubRequestId } : {}), ...(baseTelemetry?.properties.retryAfterFilterCategory ? { retryAfterFilterCategory: baseTelemetry.properties.retryAfterFilterCategory } : {}), }, { + turn: getTurnFromBaseTelemetry(baseTelemetry), totalTokenMax: chatEndpointInfo?.modelMaxPromptTokens ?? -1, tokenCountMax: maxResponseTokens, promptTokenCount: chatCompletion.usage?.prompt_tokens, From f4c17b347cf26b2f6206344eb9da517005d05c27 Mon Sep 17 00:00:00 2001 From: Matt Bierner <12821956+mjbvz@users.noreply.github.com> Date: Thu, 21 May 2026 13:03:53 -0700 Subject: [PATCH 11/43] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/preview/renderedDiffWarning.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts b/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts index ccdebae1541c9b..0d85caad454be0 100644 --- a/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts +++ b/extensions/markdown-language-features/src/preview/renderedDiffWarning.ts @@ -64,7 +64,7 @@ export class RenderedDiffWarningManager extends Disposable { if (!this.#statusBarItem) { this.#statusBarItem = vscode.window.createStatusBarItem('markdown.renderedDiffWarning', vscode.StatusBarAlignment.Right, 100); this.#statusBarItem.name = vscode.l10n.t('Rendered Markdown Diff Warning'); - this.#statusBarItem.text = '$(warning) ' + vscode.l10n.t('Rendered Diff'); + this.#statusBarItem.text = vscode.l10n.t('{0} Rendered Diff', '$(warning)'); this.#statusBarItem.tooltip = vscode.l10n.t('Rendered Markdown diffs may hide important changes. Click for details.'); this.#statusBarItem.backgroundColor = new vscode.ThemeColor('statusBarItem.warningBackground'); this.#statusBarItem.command = this.#showWarningCommandId; From e7137a3065516a0e0132007ffbada5934d1b1ee0 Mon Sep 17 00:00:00 2001 From: dileepyavan <52841896+dileepyavan@users.noreply.github.com> Date: Thu, 21 May 2026 13:17:09 -0700 Subject: [PATCH 12/43] Integrating MXC for windows sandboxing (#317669) * draft version * draft version * adding mxc for windows sandboxing * Potential fix for pull request finding Merging readwritePaths Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * cleaning up PR * cleaning up * Run Windows MXC commands directly * Pin MXC SDK lockfiles * fixing tests and cleaning up env variables * adding a flag for windows sandboxing as its experimental * adding a flag for windows sandboxing as its experimental * adding a flag for windows sandboxing as its experimental --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- package-lock.json | 14 ++ package.json | 1 + remote/package-lock.json | 24 ++- remote/package.json | 1 + .../sandbox/browser/sandboxHelperService.ts | 10 +- .../sandbox/common/sandboxHelperIpc.ts | 14 +- .../sandbox/common/sandboxHelperService.ts | 7 + src/vs/platform/sandbox/common/settings.ts | 2 + .../sandbox/common/terminalSandboxEngine.ts | 132 ++++++++++++-- .../common/terminalSandboxMxcRuntime.ts | 166 ++++++++++++++++++ .../common/terminalSandboxReadAllowList.ts | 4 + ...SandboxRuntimeConfigurationPerOperation.ts | 2 + src/vs/platform/sandbox/node/sandboxHelper.ts | 60 ++++++- .../test/common/terminalSandboxEngine.test.ts | 153 +++++++++++++++- .../terminal/terminalContribExports.ts | 2 + .../terminal.chatAgentTools.contribution.ts | 6 +- .../browser/tools/sandboxOutputAnalyzer.ts | 15 +- .../terminalChatAgentToolsConfiguration.ts | 47 +++++ .../common/terminalSandboxService.ts | 46 ++++- .../browser/terminalSandboxService.test.ts | 90 +++++++++- 20 files changed, 750 insertions(+), 46 deletions(-) create mode 100644 src/vs/platform/sandbox/common/terminalSandboxMxcRuntime.ts diff --git a/package-lock.json b/package-lock.json index f76a83d76b425f..f6503c85b1ca82 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,7 @@ "@microsoft/dev-tunnels-management": "^1.3.41", "@microsoft/dev-tunnels-ssh": "^3.12.22", "@microsoft/dev-tunnels-ssh-tcp": "^3.12.22", + "@microsoft/mxc-sdk": "0.2.0", "@parcel/watcher": "^2.5.6", "@types/semver": "^7.5.8", "@vscode/codicons": "^0.0.46-11", @@ -1939,6 +1940,19 @@ "resolved": "https://registry.npmjs.org/@microsoft/dynamicproto-js/-/dynamicproto-js-1.1.9.tgz", "integrity": "sha512-n1VPsljTSkthsAFYdiWfC+DKzK2WwcRp83Y1YAqdX552BstvsDjft9YXppjUzp11BPsapDoO1LDgrDB0XVsfNQ==" }, + "node_modules/@microsoft/mxc-sdk": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@microsoft/mxc-sdk/-/mxc-sdk-0.2.0.tgz", + "integrity": "sha512-xgWTV0nvIzl+IjlIhLGw++/A1eeZYORDoMLGLlDpSE8tMPWLbQIF627Xsb0pkb04MB9vtZl9P+RRNB7fwS3PXA==", + "license": "MIT", + "dependencies": { + "node-pty": "^1.2.0-beta.12", + "semver": "^7.7.4" + }, + "engines": { + "node": ">=18.0.0" + } + }, "node_modules/@modelcontextprotocol/sdk": { "version": "1.29.0", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.29.0.tgz", diff --git a/package.json b/package.json index 6a56b9569405b2..a5ac5f99cb298b 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,7 @@ "@microsoft/dev-tunnels-management": "^1.3.41", "@microsoft/dev-tunnels-ssh": "^3.12.22", "@microsoft/dev-tunnels-ssh-tcp": "^3.12.22", + "@microsoft/mxc-sdk": "0.2.0", "@parcel/watcher": "^2.5.6", "@types/semver": "^7.5.8", "@vscode/codicons": "^0.0.46-11", diff --git a/remote/package-lock.json b/remote/package-lock.json index 34d559ec1ba37b..81ab1e8adc4ce7 100644 --- a/remote/package-lock.json +++ b/remote/package-lock.json @@ -12,6 +12,7 @@ "@github/copilot-sdk": "^0.3.0", "@microsoft/1ds-core-js": "^3.2.13", "@microsoft/1ds-post-js": "^3.2.13", + "@microsoft/mxc-sdk": "0.2.0", "@parcel/watcher": "^2.5.6", "@vscode/copilot-api": "^0.3.0", "@vscode/deviceid": "^0.1.1", @@ -245,6 +246,19 @@ "resolved": "https://registry.npmjs.org/@microsoft/dynamicproto-js/-/dynamicproto-js-1.1.9.tgz", "integrity": "sha512-n1VPsljTSkthsAFYdiWfC+DKzK2WwcRp83Y1YAqdX552BstvsDjft9YXppjUzp11BPsapDoO1LDgrDB0XVsfNQ==" }, + "node_modules/@microsoft/mxc-sdk": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@microsoft/mxc-sdk/-/mxc-sdk-0.2.0.tgz", + "integrity": "sha512-xgWTV0nvIzl+IjlIhLGw++/A1eeZYORDoMLGLlDpSE8tMPWLbQIF627Xsb0pkb04MB9vtZl9P+RRNB7fwS3PXA==", + "license": "MIT", + "dependencies": { + "node-pty": "^1.2.0-beta.12", + "semver": "^7.7.4" + }, + "engines": { + "node": ">=18.0.0" + } + }, "node_modules/@parcel/watcher": { "version": "2.5.6", "resolved": "https://registry.npmjs.org/@parcel/watcher/-/watcher-2.5.6.tgz", @@ -1418,12 +1432,10 @@ "license": "MIT" }, "node_modules/semver": { - "version": "7.5.4", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.5.4.tgz", - "integrity": "sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==", - "dependencies": { - "lru-cache": "^6.0.0" - }, + "version": "7.8.0", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.8.0.tgz", + "integrity": "sha512-AcM7dV/5ul4EekoQ29Agm5vri8JNqRyj39o0qpX6vDF2GZrtutZl5RwgD1XnZjiTAfncsJhMI48QQH3sN87YNA==", + "license": "ISC", "bin": { "semver": "bin/semver.js" }, diff --git a/remote/package.json b/remote/package.json index 80e06e711be024..5aba80b5c90795 100644 --- a/remote/package.json +++ b/remote/package.json @@ -7,6 +7,7 @@ "@github/copilot-sdk": "^0.3.0", "@microsoft/1ds-core-js": "^3.2.13", "@microsoft/1ds-post-js": "^3.2.13", + "@microsoft/mxc-sdk": "0.2.0", "@parcel/watcher": "^2.5.6", "@vscode/copilot-api": "^0.3.0", "@vscode/deviceid": "^0.1.1", diff --git a/src/vs/platform/sandbox/browser/sandboxHelperService.ts b/src/vs/platform/sandbox/browser/sandboxHelperService.ts index 2506ae2e49fac6..54cfa7895fffc8 100644 --- a/src/vs/platform/sandbox/browser/sandboxHelperService.ts +++ b/src/vs/platform/sandbox/browser/sandboxHelperService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { InstantiationType, registerSingleton } from '../../instantiation/common/extensions.js'; -import { ISandboxDependencyStatus, ISandboxHelperService } from '../common/sandboxHelperService.js'; +import { ISandboxDependencyStatus, ISandboxHelperService, IWindowsMxcFilesystemPolicy } from '../common/sandboxHelperService.js'; class NullSandboxHelperService implements ISandboxHelperService { declare readonly _serviceBrand: undefined; @@ -18,6 +18,14 @@ class NullSandboxHelperService implements ISandboxHelperService { socatInstalled: true, }; } + + async getWindowsMxcFilesystemPolicy(): Promise { + return undefined; + } + + async getWindowsMxcEnvironment(): Promise { + return undefined; + } } registerSingleton(ISandboxHelperService, NullSandboxHelperService, InstantiationType.Delayed); diff --git a/src/vs/platform/sandbox/common/sandboxHelperIpc.ts b/src/vs/platform/sandbox/common/sandboxHelperIpc.ts index 125288dcf6875e..e210254331fe6a 100644 --- a/src/vs/platform/sandbox/common/sandboxHelperIpc.ts +++ b/src/vs/platform/sandbox/common/sandboxHelperIpc.ts @@ -6,7 +6,7 @@ import { Event } from '../../../base/common/event.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; import { IChannel, IServerChannel } from '../../../base/parts/ipc/common/ipc.js'; -import { ISandboxDependencyStatus, ISandboxHelperService } from './sandboxHelperService.js'; +import { ISandboxDependencyStatus, ISandboxHelperService, IWindowsMxcFilesystemPolicy } from './sandboxHelperService.js'; export const SANDBOX_HELPER_CHANNEL_NAME = 'sandboxHelper'; @@ -22,6 +22,10 @@ export class SandboxHelperChannel implements IServerChannel { switch (command) { case 'checkSandboxDependencies': return this.service.checkSandboxDependencies() as Promise; + case 'getWindowsMxcFilesystemPolicy': + return this.service.getWindowsMxcFilesystemPolicy() as Promise; + case 'getWindowsMxcEnvironment': + return this.service.getWindowsMxcEnvironment() as Promise; } throw new Error('Invalid call'); @@ -36,4 +40,12 @@ export class SandboxHelperChannelClient implements ISandboxHelperService { checkSandboxDependencies(): Promise { return this.channel.call('checkSandboxDependencies'); } + + getWindowsMxcFilesystemPolicy(): Promise { + return this.channel.call('getWindowsMxcFilesystemPolicy'); + } + + getWindowsMxcEnvironment(): Promise { + return this.channel.call('getWindowsMxcEnvironment'); + } } diff --git a/src/vs/platform/sandbox/common/sandboxHelperService.ts b/src/vs/platform/sandbox/common/sandboxHelperService.ts index 5660dd21eb63eb..fc754dc666cdef 100644 --- a/src/vs/platform/sandbox/common/sandboxHelperService.ts +++ b/src/vs/platform/sandbox/common/sandboxHelperService.ts @@ -12,7 +12,14 @@ export interface ISandboxDependencyStatus { readonly socatInstalled: boolean; } +export interface IWindowsMxcFilesystemPolicy { + readonly readonlyPaths: string[]; + readonly readwritePaths: string[]; +} + export interface ISandboxHelperService { readonly _serviceBrand: undefined; checkSandboxDependencies(): Promise; + getWindowsMxcFilesystemPolicy(): Promise; + getWindowsMxcEnvironment(): Promise; } diff --git a/src/vs/platform/sandbox/common/settings.ts b/src/vs/platform/sandbox/common/settings.ts index df6b7d92ea067e..b16b21460df15b 100644 --- a/src/vs/platform/sandbox/common/settings.ts +++ b/src/vs/platform/sandbox/common/settings.ts @@ -8,11 +8,13 @@ */ export const enum AgentSandboxSettingId { AgentSandboxEnabled = 'chat.agent.sandbox.enabled', + AgentSandboxWindowsEnabled = 'chat.agent.sandbox.enabled.windows', AgentSandboxAllowUnsandboxedCommands = 'chat.agent.sandbox.allowUnsandboxedCommands', AgentSandboxAutoApproveUnsandboxedCommands = 'chat.agent.sandbox.autoApproveUnsandboxedCommands', AgentSandboxAllowAutoApprove = 'chat.agent.sandbox.allowAutoApprove', AgentSandboxLinuxFileSystem = 'chat.agent.sandbox.fileSystem.linux', AgentSandboxMacFileSystem = 'chat.agent.sandbox.fileSystem.mac', + AgentSandboxWindowsFileSystem = 'chat.agent.sandbox.fileSystem.windows', AgentSandboxAdvancedRuntime = 'chat.agent.sandbox.advanced.runtime', DeprecatedAgentSandboxEnabled = 'chat.agent.sandbox', DeprecatedAgentSandboxLinuxFileSystem = 'chat.agent.sandboxFileSystem.linux', diff --git a/src/vs/platform/sandbox/common/terminalSandboxEngine.ts b/src/vs/platform/sandbox/common/terminalSandboxEngine.ts index 1d588558e47cfd..954ff03a79510f 100644 --- a/src/vs/platform/sandbox/common/terminalSandboxEngine.ts +++ b/src/vs/platform/sandbox/common/terminalSandboxEngine.ts @@ -6,7 +6,7 @@ import { VSBuffer } from '../../../base/common/buffer.js'; import { Event } from '../../../base/common/event.js'; import { Disposable } from '../../../base/common/lifecycle.js'; -import { dirname, posix, win32 } from '../../../base/common/path.js'; +import { posix, win32 } from '../../../base/common/path.js'; import { OperatingSystem, OS } from '../../../base/common/platform.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; @@ -15,8 +15,9 @@ import { IFileService } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; import { matchesDomainPattern, normalizeDomain } from '../../networkFilter/common/domainMatcher.js'; import { AgentNetworkDomainSettingId } from '../../networkFilter/common/settings.js'; -import { ISandboxDependencyStatus } from './sandboxHelperService.js'; +import { ISandboxDependencyStatus, IWindowsMxcFilesystemPolicy } from './sandboxHelperService.js'; import { AgentSandboxEnabledValue, AgentSandboxSettingId } from './settings.js'; +import { IWindowsMxcTerminalSandboxRuntime } from './terminalSandboxMxcRuntime.js'; import { getTerminalSandboxReadAllowListForCommands } from './terminalSandboxReadAllowList.js'; import { getTerminalSandboxRuntimeConfigurationForCommands } from './terminalSandboxRuntimeConfigurationPerOperation.js'; import { ITerminalSandboxCommand, ITerminalSandboxPrerequisiteCheckResult, ITerminalSandboxResolvedNetworkDomains, ITerminalSandboxWrapResult, TerminalSandboxPrerequisiteCheck } from './terminalSandboxService.js'; @@ -41,6 +42,8 @@ export interface ITerminalSandboxRuntimeInfo { * `execPath` already points at a real `node` binary (remote, agent host). */ runAsNode?: boolean; + /** CPU architecture of the environment that runs the sandbox runtime. */ + arch?: string; } /** @@ -71,6 +74,10 @@ export interface ITerminalSandboxEngineHost { readonly onDidChangeRoots: Event; /** Resolves the installed sandbox-dependency status (bubblewrap, socat). */ checkSandboxDependencies(): Promise; + /** Resolves host filesystem policy fragments needed by the Windows MXC process container. */ + getWindowsMxcFilesystemPolicy(): Promise; + /** Resolves host environment variables needed by the Windows MXC process container. */ + getWindowsMxcEnvironment(): Promise; } /** @@ -96,6 +103,9 @@ export class TerminalSandboxEngine extends Disposable { private _userHome: URI | undefined; private _srtPath: string | undefined; private _rgPath: string | undefined; + private _mxcPath: string | undefined; + private _windowsMxcFilesystemPolicy: IWindowsMxcFilesystemPolicy | undefined; + private _windowsMxcEnvironment: string[] | undefined; private _sandboxConfigPath: string | undefined; private _sandboxDependencyStatus: ISandboxDependencyStatus | undefined; private _needsForceUpdateConfigFile = true; @@ -103,6 +113,8 @@ export class TerminalSandboxEngine extends Disposable { private _commandAllowListKeywords: readonly string[] = []; private _commandAllowListCommandDetails: readonly ITerminalSandboxCommand[] = []; private _commandCwd: URI | undefined; + private _commandLine: string | undefined; + private _commandShell: string | undefined; private _os: OperatingSystem = OS; private readonly _defaultWritePaths: string[] = []; @@ -111,6 +123,7 @@ export class TerminalSandboxEngine extends Disposable { @IConfigurationService private readonly _configurationService: IConfigurationService, @IFileService private readonly _fileService: IFileService, @ILogService private readonly _logService: ILogService, + @IWindowsMxcTerminalSandboxRuntime private readonly _windowsMxcRuntime: IWindowsMxcTerminalSandboxRuntime, ) { super(); this._register(Event.runAndSubscribe(this._configurationService.onDidChangeConfiguration, (e: IConfigurationChangeEvent | undefined) => { @@ -163,11 +176,14 @@ export class TerminalSandboxEngine extends Disposable { || !this._areStringArraysEqual(this._commandAllowListKeywords, normalizedCommandKeywords) || !this._areStringArraysEqual(currentReadAllowListPaths, nextReadAllowListPaths) || !this._areObjectsEqual(currentRuntimeConfiguration, nextRuntimeConfiguration) - || this._commandCwd?.toString() !== cwd?.toString(); + || this._commandCwd?.toString() !== cwd?.toString() + || (this._os === OperatingSystem.Windows && (this._commandLine !== command || this._commandShell !== shell)); if (shouldRefreshConfig) { this._commandAllowListKeywords = normalizedCommandKeywords; this._commandAllowListCommandDetails = normalizedCommandDetails; this._commandCwd = cwd; + this._commandLine = command; + this._commandShell = shell; await this.getSandboxConfigPath(true); } @@ -197,6 +213,16 @@ export class TerminalSandboxEngine extends Disposable { }; } + if (this._os === OperatingSystem.Windows) { + if (!this._mxcPath) { + throw new Error('MXC executable path not resolved'); + } + return { + command: this._windowsMxcRuntime.wrapCommand(this._mxcPath, this._sandboxConfigPath), + isSandboxWrapped: true, + }; + } + if (!this._execPath) { throw new Error('Executable path not set to run sandbox commands'); } @@ -210,7 +236,7 @@ export class TerminalSandboxEngine extends Disposable { // TMPDIR must be set as environment variable before the command // Quote shell arguments so the wrapped command cannot break out of the outer shell. const commandToRunInSandbox = this._getSandboxCommandWithPreservedCwd(command, cwd); - const sandboxRuntimeCommand = `PATH="$PATH:${dirname(this._rgPath)}" TMPDIR="${this._tempDir.path}" CLAUDE_TMPDIR="${this._tempDir.path}" "${this._execPath}" "${this._srtPath}" --settings "${this._sandboxConfigPath}" -c ${this._quoteShellArgument(commandToRunInSandbox)}`; + const sandboxRuntimeCommand = `PATH="$PATH:${this._pathDirname(this._rgPath)}" TMPDIR="${this._tempDir.path}" CLAUDE_TMPDIR="${this._tempDir.path}" "${this._execPath}" "${this._srtPath}" --settings "${this._sandboxConfigPath}" -c ${this._quoteShellArgument(commandToRunInSandbox)}`; const wrappedCommand = this._os === OperatingSystem.Linux && cwd?.path && cwd.path !== this._tempDir.path ? `cd ${this._quoteShellArgument(this._tempDir.path)}; ${sandboxRuntimeCommand}` : sandboxRuntimeCommand; @@ -319,6 +345,7 @@ export class TerminalSandboxEngine extends Disposable { return true; // initial run-and-subscribe } return e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxEnabled) + || e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxWindowsEnabled) || e.affectsConfiguration(AgentSandboxSettingId.DeprecatedAgentSandboxEnabled) || e.affectsConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains) || e.affectsConfiguration(AgentNetworkDomainSettingId.DeprecatedSandboxAllowedNetworkDomains) @@ -330,13 +357,14 @@ export class TerminalSandboxEngine extends Disposable { || e.affectsConfiguration(AgentSandboxSettingId.DeprecatedAgentSandboxLinuxFileSystem) || e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxMacFileSystem) || e.affectsConfiguration(AgentSandboxSettingId.DeprecatedAgentSandboxMacFileSystem) + || e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxWindowsFileSystem) || e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxAdvancedRuntime); } private async _checkSandboxDependencies(forceRefresh = false): Promise { const os = await this.getOS(); if (os === OperatingSystem.Windows) { - return false; + return true; } if (!forceRefresh && this._sandboxDependencyStatus) { @@ -368,6 +396,9 @@ export class TerminalSandboxEngine extends Disposable { } private _wrapUnsandboxedCommand(command: string, shell?: string): string { + if (this._os === OperatingSystem.Windows) { + return this._windowsMxcRuntime.wrapUnsandboxedCommand(command); + } if (!this._tempDir?.path) { return command; } @@ -473,9 +504,9 @@ export class TerminalSandboxEngine extends Disposable { } private async _isSandboxConfiguredEnabled(): Promise { - const os = await this.getOS(); - if (os === OperatingSystem.Windows) { - return false; + await this.getOS(); + if (this._os === OperatingSystem.Windows) { + return this._getSandboxConfiguredWindowsEnabledValue() === AgentSandboxEnabledValue.AllowNetwork; } const value = this._getSandboxConfiguredEnabledValue(); return value === true || value === AgentSandboxEnabledValue.On || value === AgentSandboxEnabledValue.AllowNetwork; @@ -493,6 +524,7 @@ export class TerminalSandboxEngine extends Disposable { this._userHome = await this._host.getUserHome(); this._srtPath = this._pathJoin(this._appRoot, 'node_modules', '@vscode', 'sandbox-runtime', 'dist', 'cli.js'); this._rgPath = this._pathJoin(this._appRoot, 'node_modules', '@vscode', 'ripgrep', 'bin', 'rg'); + this._mxcPath = this._windowsMxcRuntime.getExecutablePath(this._appRoot, runtimeInfo.arch); } private async _createSandboxConfig(): Promise { @@ -510,6 +542,9 @@ export class TerminalSandboxEngine extends Disposable { const macFileSystemSetting = this._os === OperatingSystem.Macintosh ? this._getSettingValue(AgentSandboxSettingId.AgentSandboxMacFileSystem, AgentSandboxSettingId.DeprecatedAgentSandboxMacFileSystem) ?? {} : {}; + const windowsFileSystemSetting = this._os === OperatingSystem.Windows + ? this._getSettingValue(AgentSandboxSettingId.AgentSandboxWindowsFileSystem) ?? {} + : {}; const runtimeSetting = this._getSettingValue>(AgentSandboxSettingId.AgentSandboxAdvancedRuntime) ?? {}; const commandRuntimeSetting = getTerminalSandboxRuntimeConfigurationForCommands(this._os, this._commandAllowListCommandDetails); const commandRuntimeAllowReadPaths = this._getCommandRuntimeFileSystemPaths(commandRuntimeSetting, 'allowRead'); @@ -519,7 +554,17 @@ export class TerminalSandboxEngine extends Disposable { let allowReadPaths: string[] = []; let denyReadPaths: string[] = []; let denyWritePaths: string[] | undefined; - if (this._os === OperatingSystem.Macintosh) { + if (this._os === OperatingSystem.Windows) { + const filesystemPolicy = await this._getWindowsMxcFilesystemPolicy(); + const env = await this._getWindowsMxcEnvironment(); + allowWritePaths = await this._resolveFileSystemPaths([ + ...this._updateAllowWritePathsWithWorkspaceFolders(windowsFileSystemSetting.allowWrite, commandRuntimeAllowWritePaths), + ...filesystemPolicy.readwritePaths + ]); + allowReadPaths = await this._resolveFileSystemPaths([...(await this._updateAllowReadPathsWithAllowWrite(windowsFileSystemSetting.allowRead, allowWritePaths, commandRuntimeAllowReadPaths)), ...filesystemPolicy.readonlyPaths]); + denyReadPaths = await this._resolveFileSystemPaths(this._updateDenyReadPathsWithHome(windowsFileSystemSetting.denyRead)); + this._windowsMxcEnvironment = env; + } else if (this._os === OperatingSystem.Macintosh) { allowWritePaths = await this._resolveFileSystemPaths(this._updateAllowWritePathsWithWorkspaceFolders(macFileSystemSetting.allowWrite, commandRuntimeAllowWritePaths)); allowReadPaths = await this._resolveFileSystemPaths(await this._updateAllowReadPathsWithAllowWrite(macFileSystemSetting.allowRead, allowWritePaths, commandRuntimeAllowReadPaths)); denyReadPaths = await this._resolveFileSystemPaths(this._updateDenyReadPathsWithHome(macFileSystemSetting.denyRead)); @@ -530,7 +575,17 @@ export class TerminalSandboxEngine extends Disposable { denyReadPaths = await this._resolveFileSystemPaths(this._updateDenyReadPathsWithHome(linuxFileSystemSetting.denyRead)); denyWritePaths = await this._resolveFileSystemPaths(linuxFileSystemSetting.denyWrite); } - const sandboxSettings = { + const sandboxSettings = this._os === OperatingSystem.Windows ? this._windowsMxcRuntime.createConfig({ + command: this._commandLine ?? '', + cwd: this._commandCwd, + tempDir: this._tempDir, + allowNetwork, + networkDomains: this.getResolvedNetworkDomains(), + allowReadPaths, + allowWritePaths, + denyReadPaths, + env: this._windowsMxcEnvironment ?? [], + }) : { network: allowNetwork ? { allowedDomains: [], deniedDomains: [], enabled: false } : this.getResolvedNetworkDomains(), filesystem: { denyRead: denyReadPaths, @@ -539,9 +594,11 @@ export class TerminalSandboxEngine extends Disposable { denyWrite: denyWritePaths, }, }; - this._mergeAdditionalSandboxConfigProperties(sandboxSettings as Record, allowNetwork ? this._withoutNetworkRuntimeSetting(runtimeSetting) : runtimeSetting); - this._mergeAdditionalSandboxConfigProperties(sandboxSettings as Record, commandRuntimeSetting); - this._sandboxConfigPath = configFileUri.path; + if (this._os !== OperatingSystem.Windows) { + this._mergeAdditionalSandboxConfigProperties(sandboxSettings as Record, allowNetwork ? this._withoutNetworkRuntimeSetting(runtimeSetting) : runtimeSetting); + this._mergeAdditionalSandboxConfigProperties(sandboxSettings as Record, commandRuntimeSetting); + } + this._sandboxConfigPath = this._getUriPath(configFileUri); await this._fileService.createFile(configFileUri, VSBuffer.fromString(JSON.stringify(sandboxSettings, null, '\t')), { overwrite: true }); return this._sandboxConfigPath; } @@ -584,11 +641,33 @@ export class TerminalSandboxEngine extends Disposable { return typeof value === 'object' && value !== null && !Array.isArray(value); } + private async _getWindowsMxcFilesystemPolicy(): Promise { + if (!this._windowsMxcFilesystemPolicy) { + this._windowsMxcFilesystemPolicy = await this._host.getWindowsMxcFilesystemPolicy() ?? { readonlyPaths: [], readwritePaths: [] }; + } + return this._windowsMxcFilesystemPolicy; + } + + private async _getWindowsMxcEnvironment(): Promise { + if (!this._windowsMxcEnvironment) { + this._windowsMxcEnvironment = await this._host.getWindowsMxcEnvironment() ?? []; + } + return this._windowsMxcEnvironment; + } + private _pathJoin = (...segments: string[]) => { const path = this._os === OperatingSystem.Windows ? win32 : posix; return path.join(...segments); }; + private _pathDirname(path: string): string { + return (this._os === OperatingSystem.Windows ? win32 : posix).dirname(path); + } + + private _getUriPath(uri: URI): string { + return this._os === OperatingSystem.Windows ? this._windowsMxcRuntime.toWindowsPath(uri) : uri.path; + } + private async _initTempDir(): Promise { if (!(await this.isEnabled())) { return; @@ -597,19 +676,23 @@ export class TerminalSandboxEngine extends Disposable { this._tempDir = await this._host.getSandboxTempDir(); if (this._tempDir) { await this._fileService.createFolder(this._tempDir); - this._defaultWritePaths.push(this._tempDir.path); + this._defaultWritePaths.push(this._getUriPath(this._tempDir)); } else { this._logService.warn('TerminalSandboxEngine: Cannot create sandbox settings file because no tmpDir is available in this environment'); } } private _updateAllowWritePathsWithWorkspaceFolders(configuredAllowWrite: string[] | undefined, commandRuntimeAllowWrite: string[] = []): string[] { - const writeRootPaths = this._host.getWriteRoots().map(folder => folder.path); + const writeRootPaths = this._host.getWriteRoots().map(folder => this._getUriPath(folder)); return [...new Set([...writeRootPaths, ...this._defaultWritePaths, ...(configuredAllowWrite ?? []), ...commandRuntimeAllowWrite])]; } private _updateDenyReadPathsWithHome(configuredDenyRead: string[] | undefined): string[] { - const userHome = this._userHome?.path; + // TODO: On Windows, deny read on home directory. + if (this._os === OperatingSystem.Windows) { + return [...new Set(configuredDenyRead ?? [])]; + } + const userHome = this._userHome ? this._getUriPath(this._userHome) : undefined; return [...new Set([...(configuredDenyRead ?? []), ...(userHome ? [userHome] : [])])]; } @@ -624,6 +707,9 @@ export class TerminalSandboxEngine extends Disposable { private async _resolveFileSystemPath(path: string): Promise { const expandedPath = this._os === OperatingSystem.Linux ? this._expandHomePath(path) : path; + if (this._os === OperatingSystem.Windows) { + return expandedPath; + } if (!this._isAbsoluteFileSystemPath(expandedPath)) { return expandedPath; } @@ -662,9 +748,12 @@ export class TerminalSandboxEngine extends Disposable { if (!this._appRoot) { return []; } + if (this._os === OperatingSystem.Windows) { + return this._windowsMxcRuntime.getRuntimeReadPaths(this._appRoot, this._mxcPath); + } const paths: string[] = [this._appRoot]; if (this._execPath) { - for (const path of [this._execPath, dirname(this._execPath)]) { + for (const path of [this._execPath, this._pathDirname(this._execPath)]) { if (!this._isPathUnderAppRoot(path)) { paths.push(path); } @@ -682,14 +771,21 @@ export class TerminalSandboxEngine extends Disposable { private async _getWorkspaceStorageReadPaths(): Promise { const root = await this._host.getWorkspaceStorageReadRoot(); - return root ? [root.path] : []; + return root ? [this._getUriPath(root)] : []; } private _getSandboxConfiguredEnabledValue(): AgentSandboxEnabledValue | boolean { return this._getSettingValue(AgentSandboxSettingId.AgentSandboxEnabled, AgentSandboxSettingId.DeprecatedAgentSandboxEnabled) ?? AgentSandboxEnabledValue.Off; } + private _getSandboxConfiguredWindowsEnabledValue(): AgentSandboxEnabledValue { + return this._getSettingValue(AgentSandboxSettingId.AgentSandboxWindowsEnabled) ?? AgentSandboxEnabledValue.Off; + } + private _isSandboxAllowNetworkConfigured(): boolean { + if (this._os === OperatingSystem.Windows) { + return this._getSandboxConfiguredWindowsEnabledValue() === AgentSandboxEnabledValue.AllowNetwork; + } return this._getSandboxConfiguredEnabledValue() === AgentSandboxEnabledValue.AllowNetwork; } diff --git a/src/vs/platform/sandbox/common/terminalSandboxMxcRuntime.ts b/src/vs/platform/sandbox/common/terminalSandboxMxcRuntime.ts new file mode 100644 index 00000000000000..740b82034c09fd --- /dev/null +++ b/src/vs/platform/sandbox/common/terminalSandboxMxcRuntime.ts @@ -0,0 +1,166 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { win32 } from '../../../base/common/path.js'; +import { URI } from '../../../base/common/uri.js'; +import { createDecorator } from '../../instantiation/common/instantiation.js'; +import type { ITerminalSandboxResolvedNetworkDomains } from './terminalSandboxService.js'; + +export interface IWindowsMxcProcessConfig { + commandLine: string; + cwd?: string; + env: string[]; + timeout: number; +} + +export interface IWindowsMxcFilesystemConfig { + readwritePaths: string[]; + readonlyPaths: string[]; + deniedPaths: string[]; +} + +export interface IWindowsMxcNetworkConfig { + defaultPolicy: 'allow' | 'block'; + allowedHosts?: string[]; + blockedHosts?: string[]; +} + +export interface IWindowsMxcConfig { + version: string; + containerId: string; + containment: 'process'; + lifecycle: { + destroyOnExit: boolean; + preservePolicy: boolean; + }; + process: IWindowsMxcProcessConfig; + filesystem: IWindowsMxcFilesystemConfig; + network: IWindowsMxcNetworkConfig; + ui: { + disable: boolean; + clipboard: 'none'; + injection: boolean; + }; +} + +export interface IWindowsMxcConfigOptions { + command: string; + cwd: URI | undefined; + tempDir: URI; + allowNetwork: boolean; + networkDomains: ITerminalSandboxResolvedNetworkDomains; + allowReadPaths: string[]; + allowWritePaths: string[]; + denyReadPaths: string[]; + env: string[]; +} + +export const IWindowsMxcTerminalSandboxRuntime = createDecorator('windowsMxcTerminalSandboxRuntime'); + +export interface IWindowsMxcTerminalSandboxRuntime { + readonly _serviceBrand: undefined; + + getExecutablePath(appRoot: string, arch: string | undefined): string; + getRuntimeReadPaths(appRoot: string | undefined, executablePath: string | undefined): string[]; + createConfig(options: IWindowsMxcConfigOptions): IWindowsMxcConfig; + wrapCommand(executablePath: string, configPath: string): string; + wrapUnsandboxedCommand(command: string): string; + toWindowsPath(uri: URI): string; +} + +/** + * Windows-only MXC integration for terminal sandboxing. + * + * This class is intentionally isolated from the SRT-backed runtime so it can be + * removed once SRT supports Windows sandboxing. + */ +export class WindowsMxcTerminalSandboxRuntime implements IWindowsMxcTerminalSandboxRuntime { + declare readonly _serviceBrand: undefined; + + private readonly _configVersion = '0.4.0-alpha'; + + getExecutablePath(appRoot: string, arch: string | undefined): string { + const binArch = arch === 'arm64' ? 'arm64' : 'x64'; + return win32.join(appRoot, 'node_modules', '@microsoft', 'mxc-sdk', 'bin', binArch, 'wxc-exec.exe'); + } + + getRuntimeReadPaths(appRoot: string | undefined, executablePath: string | undefined): string[] { + const paths: string[] = []; + if (appRoot) { + paths.push(appRoot); + } + if (executablePath) { + paths.push(executablePath, win32.dirname(executablePath)); + } + return [...new Set(paths)]; + } + + createConfig(options: IWindowsMxcConfigOptions): IWindowsMxcConfig { + const tempDirPath = this.toWindowsPath(options.tempDir); + return { + version: this._configVersion, + containerId: 'vscode-terminal-sandbox', + containment: 'process', + lifecycle: { + destroyOnExit: true, + preservePolicy: false, + }, + process: { + commandLine: options.command, + cwd: options.cwd ? this.toWindowsPath(options.cwd) : tempDirPath, + env: [ + ...options.env + ], + timeout: 0, + }, + filesystem: { + readwritePaths: [...new Set([...options.allowWritePaths])], + readonlyPaths: [...new Set([tempDirPath, ...options.allowReadPaths])], + deniedPaths: options.denyReadPaths, + }, + network: this._createNetworkConfig(options.allowNetwork, options.networkDomains), + ui: { + disable: false, + clipboard: 'none', + injection: false, + }, + }; + } + + wrapCommand(executablePath: string, configPath: string): string { + return `& ${this._quotePowerShellArgument(executablePath)} ${this._quotePowerShellArgument(configPath)}`; + } + + wrapUnsandboxedCommand(command: string): string { + return command; + } + + toWindowsPath(uri: URI): string { + let value: string; + if (uri.authority && uri.path.length > 1 && uri.scheme === 'file') { + value = `\\\\${uri.authority}${uri.path}`; + } else if (/^\/[a-zA-Z]:/.test(uri.path)) { + value = uri.path.slice(1); + } else { + value = uri.fsPath; + } + return value.replace(/\//g, '\\'); + } + + private _createNetworkConfig(allowNetwork: boolean, networkDomains: ITerminalSandboxResolvedNetworkDomains): IWindowsMxcNetworkConfig { + if (allowNetwork) { + return { defaultPolicy: 'allow' }; + } + return { + defaultPolicy: 'block', + allowedHosts: networkDomains.allowedDomains, + blockedHosts: networkDomains.deniedDomains + }; + } + + private _quotePowerShellArgument(value: string): string { + return `'${value.replace(/'/g, `''`)}'`; + } +} diff --git a/src/vs/platform/sandbox/common/terminalSandboxReadAllowList.ts b/src/vs/platform/sandbox/common/terminalSandboxReadAllowList.ts index d1ff40e5afd435..bbbeb9d115a922 100644 --- a/src/vs/platform/sandbox/common/terminalSandboxReadAllowList.ts +++ b/src/vs/platform/sandbox/common/terminalSandboxReadAllowList.ts @@ -86,6 +86,10 @@ const terminalSandboxReadAllowListKeywordMap: ReadonlyMap Promise; @@ -31,4 +33,58 @@ export class SandboxHelperService implements ISandboxHelperService { checkSandboxDependencies(): Promise { return SandboxHelperService.checkSandboxDependenciesWith(findExecutable); } + + async getWindowsMxcFilesystemPolicy(): Promise { + if (!isWindows) { + return undefined; + } + + const { getAvailableToolsPolicy, getUserProfilePolicy } = await import('@microsoft/mxc-sdk'); + const availableToolsPolicy = getAvailableToolsPolicy(process.env, { containerType: 'processcontainer' }); + const userProfilePolicy = getUserProfilePolicy(); + const psHome = await this._getPSHome(); + return { + readonlyPaths: [...new Set([...availableToolsPolicy.readonlyPaths, ...userProfilePolicy.readonlyPaths, ...this._getTempReadPaths(), ...(psHome ? [psHome] : [])])], + readwritePaths: [...new Set([...availableToolsPolicy.readwritePaths, ...userProfilePolicy.readwritePaths])], + }; + } + + async getWindowsMxcEnvironment(): Promise { + if (!isWindows) { + return undefined; + } + + const env: string[] = []; + const path = getCaseInsensitive(process.env, 'PATH'); + if (typeof path === 'string' && path) { + env.push(`PATH=${path}`); + } + + const psHome = await this._getPSHome(); + if (psHome) { + env.push(`PSHOME=${psHome}`); + } + return env; + } + + private async _getPSHome(): Promise { + const psHome = getCaseInsensitive(process.env, 'PSHOME'); + if (typeof psHome === 'string' && psHome) { + return psHome; + } + + const powerShellPath = await findExecutable('pwsh') ?? await findExecutable('powershell'); + return powerShellPath ? win32.dirname(powerShellPath) : undefined; + } + + private _getTempReadPaths(): string[] { + const paths: string[] = []; + for (const variable of ['TMP', 'TEMP']) { + const path = getCaseInsensitive(process.env, variable); + if (typeof path === 'string' && path) { + paths.push(path); + } + } + return paths; + } } diff --git a/src/vs/platform/sandbox/test/common/terminalSandboxEngine.test.ts b/src/vs/platform/sandbox/test/common/terminalSandboxEngine.test.ts index a97970e7152266..b77cbc94c5a040 100644 --- a/src/vs/platform/sandbox/test/common/terminalSandboxEngine.test.ts +++ b/src/vs/platform/sandbox/test/common/terminalSandboxEngine.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ok, strictEqual } from 'assert'; +import { deepStrictEqual, ok, strictEqual } from 'assert'; import { VSBuffer } from '../../../../base/common/buffer.js'; import { Emitter } from '../../../../base/common/event.js'; import { OperatingSystem } from '../../../../base/common/platform.js'; @@ -14,9 +14,10 @@ import { TestConfigurationService } from '../../../configuration/test/common/tes import { IFileService } from '../../../files/common/files.js'; import { TestInstantiationService } from '../../../instantiation/test/common/instantiationServiceMock.js'; import { ILogService, NullLogService } from '../../../log/common/log.js'; -import type { ISandboxDependencyStatus } from '../../common/sandboxHelperService.js'; +import type { ISandboxDependencyStatus, IWindowsMxcFilesystemPolicy } from '../../common/sandboxHelperService.js'; import { AgentSandboxEnabledValue, AgentSandboxSettingId } from '../../common/settings.js'; import { ITerminalSandboxEngineHost, ITerminalSandboxRuntimeInfo, TerminalSandboxEngine } from '../../common/terminalSandboxEngine.js'; +import { IWindowsMxcTerminalSandboxRuntime, WindowsMxcTerminalSandboxRuntime } from '../../common/terminalSandboxMxcRuntime.js'; suite('TerminalSandboxEngine', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -42,7 +43,12 @@ suite('TerminalSandboxEngine', () => { async createFile(uri: URI, content: VSBuffer): Promise { createFileCount++; - createdFiles.set(uri.path, content.toString()); + const contentString = content.toString(); + createdFiles.set(uri.path, contentString); + createdFiles.set(uri.fsPath, contentString); + if (/^\/[a-zA-Z]:/.test(uri.path)) { + createdFiles.set(uri.path.slice(1).replace(/\//g, '\\'), contentString); + } return {}; } async createFolder(uri: URI): Promise { @@ -68,11 +74,35 @@ suite('TerminalSandboxEngine', () => { getWriteRoots: () => [URI.file('/workspace')], onDidChangeRoots: rootsEmitter.event, checkSandboxDependencies: (): Promise => Promise.resolve({ bubblewrapInstalled: true, socatInstalled: true }), + getWindowsMxcFilesystemPolicy: (): Promise => Promise.resolve(undefined), + getWindowsMxcEnvironment: (): Promise => Promise.resolve(undefined), ...overrides, }; return Object.assign(host, { rootsEmitter }); } + function createWindowsHost(overrides: Partial = {}): ITerminalSandboxEngineHost & { rootsEmitter: Emitter } { + return createHost({ + getOS: () => Promise.resolve(OperatingSystem.Windows), + getRuntimeInfo: () => Promise.resolve({ appRoot: 'C:\\app', arch: 'x64' }), + getUserHome: () => Promise.resolve(URI.from({ scheme: 'file', path: '/c:/Users/user' })), + getSandboxTempDir: () => Promise.resolve(URI.from({ scheme: 'file', path: '/c:/Users/user/.test-data/tmp' })), + getWorkspaceStorageReadRoot: () => Promise.resolve(URI.from({ scheme: 'file', path: '/c:/Users/user/workspaceStorage/workspace-id' })), + getWriteRoots: () => [URI.from({ scheme: 'file', path: '/c:/workspace' })], + getWindowsMxcFilesystemPolicy: () => Promise.resolve({ readonlyPaths: ['C:\\tools\\node', 'C:\\tools\\python', 'C:\\Users\\user\\AppData\\Local\\Programs\\Git', 'C:\\Users\\user\\AppData\\Local\\Temp'], readwritePaths: [] }), + getWindowsMxcEnvironment: () => Promise.resolve(['PATH=C:\\tools\\node;C:\\Windows\\System32', 'PSHOME=C:\\Program Files\\PowerShell\\7']), + ...overrides, + }); + } + + function normalizeWindowsPathForAssert(path: string): string { + return path.replace(/\\/g, '/').toLowerCase(); + } + + function enableWindowsSandbox(): void { + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxWindowsEnabled, AgentSandboxEnabledValue.AllowNetwork); + } + setup(() => { createdFiles = new Map(); createFileCount = 0; @@ -86,6 +116,7 @@ suite('TerminalSandboxEngine', () => { instantiationService.stub(IConfigurationService, configurationService); instantiationService.stub(IFileService, fileService); instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(IWindowsMxcTerminalSandboxRuntime, instantiationService.createInstance(WindowsMxcTerminalSandboxRuntime)); }); test('runAsNode=true prefixes the wrapped command with ELECTRON_RUN_AS_NODE=1', async () => { @@ -201,12 +232,124 @@ suite('TerminalSandboxEngine', () => { await engine.cleanupTempDir(); // must not throw }); - test('isEnabled returns false on Windows regardless of configuration', async () => { - const host = createHost({ getOS: () => Promise.resolve(OperatingSystem.Windows) }); + test('isEnabled returns false on Windows when Windows sandbox setting is disabled by default', async () => { + const host = createWindowsHost(); const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); strictEqual(await engine.isEnabled(), false); strictEqual(await engine.isSandboxAllowNetworkEnabled(), false); + strictEqual(await engine.getSandboxConfigPath(), undefined); + }); + + test('isEnabled returns true on Windows when Windows sandbox setting allows network even if global sandboxing is off', async () => { + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxEnabled, AgentSandboxEnabledValue.Off); + enableWindowsSandbox(); + const host = createWindowsHost(); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + strictEqual(await engine.isEnabled(), true); + strictEqual(await engine.isSandboxAllowNetworkEnabled(), true); + }); + + test('wrapCommand uses MXC executable and writes MXC config on Windows', async () => { + enableWindowsSandbox(); + const host = createWindowsHost(); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + const wrapped = await engine.wrapCommand('echo hello', false, 'pwsh', URI.from({ scheme: 'file', path: '/c:/workspace' })); + const configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const config = JSON.parse(createdFiles.get(configPath)!); + + strictEqual(wrapped.isSandboxWrapped, true); + ok(wrapped.command.startsWith(`& 'C:\\app\\node_modules\\@microsoft\\mxc-sdk\\bin\\x64\\wxc-exec.exe'`), `Expected MXC executable. Actual: ${wrapped.command}`); + ok(wrapped.command.includes(` '${configPath}'`), `Expected wrapped command to pass the MXC config path. Actual: ${wrapped.command}`); + strictEqual(config.version, '0.4.0-alpha'); + strictEqual(config.containment, 'process'); + strictEqual(config.process.commandLine, 'echo hello'); + strictEqual(normalizeWindowsPathForAssert(config.process.cwd), 'c:/workspace'); + strictEqual(config.ui.disable, false); + ok(config.process.env.includes('PATH=C:\\tools\\node;C:\\Windows\\System32'), 'PATH should be injected into the MXC process env'); + ok(config.process.env.includes('PSHOME=C:\\Program Files\\PowerShell\\7'), 'PSHOME should be injected into the MXC process env'); + deepStrictEqual(config.network, { defaultPolicy: 'allow' }); + ok(config.filesystem.readwritePaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/workspace'), 'Workspace should be writable'); + ok(config.filesystem.readwritePaths.some((path: string) => normalizeWindowsPathForAssert(path).endsWith('/.test-data/tmp')), 'Sandbox temp dir should be writable'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path).endsWith('/.test-data/tmp')), 'Sandbox temp dir should be readable through readonly paths'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/app'), 'App root should be readable for MXC'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/tools/node'), 'MXC available tools policy should add tool paths to readonly paths'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/users/user/appdata/local/programs/git'), 'MXC user profile policy should add user profile paths to readonly paths'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/users/user/appdata/local/temp'), 'MXC actual temp policy should add host temp path to readonly paths'); + ok(!config.filesystem.deniedPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/users/user'), 'User home should not be denied by default on Windows'); + }); + + test('wrapCommand applies Windows filesystem setting to MXC config', async () => { + enableWindowsSandbox(); + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxWindowsFileSystem, { + allowWrite: ['C:\\configured\\write'], + allowRead: ['C:\\configured\\read'], + denyRead: ['C:\\configured\\secret'], + }); + const host = createWindowsHost(); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + await engine.wrapCommand('echo hello', false, 'pwsh'); + const configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const config = JSON.parse(createdFiles.get(configPath)!); + + ok(config.filesystem.readwritePaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/configured/write'), 'Configured Windows allowWrite path should be writable'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/configured/read'), 'Configured Windows allowRead path should be readonly'); + ok(config.filesystem.readonlyPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/users/user/appdata/local/temp'), 'Host temp path from Windows policy should be readonly'); + ok(config.filesystem.deniedPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/configured/secret'), 'Configured Windows denyRead path should be denied'); + ok(!config.filesystem.deniedPaths.some((path: string) => normalizeWindowsPathForAssert(path) === 'c:/users/user'), 'User home should not be denied by default on Windows'); + }); + + test('wrapCommand uses arm64 MXC executable on Windows arm64', async () => { + enableWindowsSandbox(); + const host = createWindowsHost({ + getRuntimeInfo: () => Promise.resolve({ appRoot: 'C:\\app', arch: 'arm64' }), + }); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + const wrapped = await engine.wrapCommand('echo hello', false, 'pwsh'); + const configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const config = JSON.parse(createdFiles.get(configPath)!); + + strictEqual(wrapped.command, `& 'C:\\app\\node_modules\\@microsoft\\mxc-sdk\\bin\\arm64\\wxc-exec.exe' '${configPath}'`); + strictEqual(normalizeWindowsPathForAssert(config.process.cwd), 'c:/users/user/.test-data/tmp'); + }); + + test('wrapCommand rewrites MXC config when Windows command changes', async () => { + enableWindowsSandbox(); + const host = createWindowsHost(); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + await engine.wrapCommand('echo first', false, 'pwsh'); + let configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const firstCommandLine = JSON.parse(createdFiles.get(configPath)!).process.commandLine; + strictEqual(firstCommandLine, 'echo first'); + + await engine.wrapCommand('echo second', false, 'pwsh'); + configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const secondCommandLine = JSON.parse(createdFiles.get(configPath)!).process.commandLine; + strictEqual(secondCommandLine, 'echo second'); + }); + + test('allowNetwork maps to MXC allow network config on Windows', async () => { + enableWindowsSandbox(); + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxEnabled, AgentSandboxEnabledValue.AllowNetwork); + const host = createWindowsHost(); + const engine = store.add(instantiationService.createInstance(TerminalSandboxEngine, host)); + + await engine.wrapCommand('curl https://example.com', false, 'pwsh'); + const configPath = await engine.getSandboxConfigPath(); + ok(configPath, 'Config path should be defined'); + const config = JSON.parse(createdFiles.get(configPath)!); + + deepStrictEqual(config.network, { defaultPolicy: 'allow' }); }); test('uses OS-specific filesystem absolute path detection', async () => { diff --git a/src/vs/workbench/contrib/terminal/terminalContribExports.ts b/src/vs/workbench/contrib/terminal/terminalContribExports.ts index 9dd025191eb87f..bf4eb058cca5cd 100644 --- a/src/vs/workbench/contrib/terminal/terminalContribExports.ts +++ b/src/vs/workbench/contrib/terminal/terminalContribExports.ts @@ -48,6 +48,7 @@ export const enum TerminalContribSettingId { ShellIntegrationTimeout = TerminalChatAgentToolsSettingId.ShellIntegrationTimeout, OutputLocation = TerminalChatAgentToolsSettingId.OutputLocation, AgentSandboxEnabled = AgentSandboxSettingId.AgentSandboxEnabled, + AgentSandboxWindowsEnabled = AgentSandboxSettingId.AgentSandboxWindowsEnabled, AgentSandboxAllowUnsandboxedCommands = AgentSandboxSettingId.AgentSandboxAllowUnsandboxedCommands, AgentSandboxAutoApproveUnsandboxedCommands = AgentSandboxSettingId.AgentSandboxAutoApproveUnsandboxedCommands, AgentSandboxAllowAutoApprove = AgentSandboxSettingId.AgentSandboxAllowAutoApprove, @@ -56,6 +57,7 @@ export const enum TerminalContribSettingId { DeprecatedAgentSandboxMacFileSystem = TerminalChatAgentToolsSettingId.DeprecatedAgentSandboxMacFileSystem, AgentSandboxLinuxFileSystem = TerminalChatAgentToolsSettingId.AgentSandboxLinuxFileSystem, AgentSandboxMacFileSystem = TerminalChatAgentToolsSettingId.AgentSandboxMacFileSystem, + AgentSandboxWindowsFileSystem = TerminalChatAgentToolsSettingId.AgentSandboxWindowsFileSystem, } // HACK: Export some context key strings from `terminalContrib/` that are depended upon elsewhere. diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts index bf5671edec17c5..5b73122b41bd47 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts @@ -35,11 +35,13 @@ import { GetTaskOutputTool, GetTaskOutputToolData } from './tools/task/getTaskOu import { RunTaskTool, RunTaskToolData } from './tools/task/runTaskTool.js'; import { registerTerminalCompressors } from './tools/terminalOutputCompressor.js'; import { InstantiationType, registerSingleton } from '../../../../../platform/instantiation/common/extensions.js'; +import { IWindowsMxcTerminalSandboxRuntime, WindowsMxcTerminalSandboxRuntime } from '../../../../../platform/sandbox/common/terminalSandboxMxcRuntime.js'; import { ITerminalSandboxService, TerminalSandboxService } from '../common/terminalSandboxService.js'; import { isNumber } from '../../../../../base/common/types.js'; // #region Services +registerSingleton(IWindowsMxcTerminalSandboxRuntime, WindowsMxcTerminalSandboxRuntime, InstantiationType.Delayed); registerSingleton(ITerminalSandboxService, TerminalSandboxService, InstantiationType.Delayed); // #endregion Services @@ -147,6 +149,7 @@ export class ChatAgentToolsContribution extends Disposable implements IWorkbench this._register(this._configurationService.onDidChangeConfiguration(e => { if ( e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxEnabled) || + e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxWindowsEnabled) || e.affectsConfiguration(AgentSandboxSettingId.DeprecatedAgentSandboxEnabled) || e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxAllowUnsandboxedCommands) || e.affectsConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains) || @@ -158,7 +161,8 @@ export class ChatAgentToolsContribution extends Disposable implements IWorkbench e.affectsConfiguration(TerminalChatAgentToolsSettingId.AgentSandboxLinuxFileSystem) || e.affectsConfiguration(TerminalChatAgentToolsSettingId.DeprecatedAgentSandboxLinuxFileSystem) || e.affectsConfiguration(TerminalChatAgentToolsSettingId.AgentSandboxMacFileSystem) || - e.affectsConfiguration(TerminalChatAgentToolsSettingId.DeprecatedAgentSandboxMacFileSystem) + e.affectsConfiguration(TerminalChatAgentToolsSettingId.DeprecatedAgentSandboxMacFileSystem) || + e.affectsConfiguration(TerminalChatAgentToolsSettingId.AgentSandboxWindowsFileSystem) ) { this._registerRunInTerminalTool(); } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sandboxOutputAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sandboxOutputAnalyzer.ts index 47207ee8db2775..8ca6f476cc2efd 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sandboxOutputAnalyzer.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sandboxOutputAnalyzer.ts @@ -30,9 +30,18 @@ export class SandboxOutputAnalyzer extends Disposable implements IOutputAnalyzer } const os = await this._sandboxService.getOS(); - const fileSystemSetting = os === OperatingSystem.Linux - ? TerminalChatAgentToolsSettingId.AgentSandboxLinuxFileSystem - : TerminalChatAgentToolsSettingId.AgentSandboxMacFileSystem; + let fileSystemSetting: TerminalChatAgentToolsSettingId; + switch (os) { + case OperatingSystem.Linux: + fileSystemSetting = TerminalChatAgentToolsSettingId.AgentSandboxLinuxFileSystem; + break; + case OperatingSystem.Windows: + fileSystemSetting = TerminalChatAgentToolsSettingId.AgentSandboxWindowsFileSystem; + break; + default: + fileSystemSetting = TerminalChatAgentToolsSettingId.AgentSandboxMacFileSystem; + break; + } const prefix = knownFailure ? 'Command failed while running in sandboxed mode. If the command failed due to sandboxing:' diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts index 559d508002b025..a8097f878ee1b6 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts @@ -22,6 +22,7 @@ export const enum TerminalChatAgentToolsSettingId { OutputLocation = 'chat.tools.terminal.outputLocation', AgentSandboxLinuxFileSystem = 'chat.agent.sandbox.fileSystem.linux', AgentSandboxMacFileSystem = 'chat.agent.sandbox.fileSystem.mac', + AgentSandboxWindowsFileSystem = 'chat.agent.sandbox.fileSystem.windows', AgentSandboxAdvancedRuntime = 'chat.agent.sandbox.advanced.runtime', PreventShellHistory = 'chat.tools.terminal.preventShellHistory', EnforceTimeoutFromModel = 'chat.tools.terminal.enforceTimeoutFromModel', @@ -562,6 +563,21 @@ export const terminalChatAgentToolsConfiguration: IStringDictionary this._workspaceContextService.getWorkspace().folders.map(folder => folder.uri), onDidChangeRoots: this._onDidChangeRoots.event, checkSandboxDependencies: () => this._resolveSandboxDependencyStatus(), + getWindowsMxcFilesystemPolicy: () => this._resolveWindowsMxcFilesystemPolicy(), + getWindowsMxcEnvironment: () => this._resolveWindowsMxcEnvironment(), }; this._engine = this._register(instantiationService.createInstance(TerminalSandboxEngine, host)); @@ -158,13 +161,26 @@ export class TerminalSandboxService extends Disposable implements ITerminalSandb const remoteEnv = await this._resolveRemoteEnv(); if (remoteEnv) { // Remote workbench: server resolves a real `node` binary, no env prefix needed. - return { appRoot: remoteEnv.appRoot.path, execPath: remoteEnv.execPath, runAsNode: false }; + return { appRoot: remoteEnv.os === OperatingSystem.Windows ? this._toWindowsPath(remoteEnv.appRoot) : remoteEnv.appRoot.path, execPath: remoteEnv.execPath, runAsNode: false, arch: remoteEnv.arch }; } // Local workbench: app root is local and exec path points at the Electron binary, // so the engine must prefix `ELECTRON_RUN_AS_NODE=1` when invoking it. - const localAppRoot = dirname(FileAccess.asFileUri('').path); + const localAppRootUri = FileAccess.asFileUri(''); + const localAppRoot = OS === OperatingSystem.Windows ? dirname(localAppRootUri.fsPath) : dirname(localAppRootUri.path); const nativeEnv = this._environmentService as IEnvironmentService & { execPath?: string }; - return { appRoot: localAppRoot, execPath: nativeEnv.execPath, runAsNode: true }; + return { appRoot: localAppRoot, execPath: nativeEnv.execPath, runAsNode: true, arch }; + } + + private _toWindowsPath(uri: URI): string { + let value: string; + if (uri.authority && uri.path.length > 1 && uri.scheme === 'file') { + value = `\\\\${uri.authority}${uri.path}`; + } else if (/^\/[a-zA-Z]:/.test(uri.path)) { + value = uri.path.slice(1); + } else { + value = uri.fsPath; + } + return value.replace(/\//g, '\\'); } private async _resolveUserHome(): Promise { @@ -216,6 +232,28 @@ export class TerminalSandboxService extends Disposable implements ITerminalSandb return this._sandboxHelperService.checkSandboxDependencies(); } + private async _resolveWindowsMxcFilesystemPolicy(): Promise { + const connection = this._remoteAgentService.getConnection(); + if (connection) { + return connection.withChannel(SANDBOX_HELPER_CHANNEL_NAME, channel => { + const sandboxHelper = new SandboxHelperChannelClient(channel); + return sandboxHelper.getWindowsMxcFilesystemPolicy(); + }); + } + return this._sandboxHelperService.getWindowsMxcFilesystemPolicy(); + } + + private async _resolveWindowsMxcEnvironment(): Promise { + const connection = this._remoteAgentService.getConnection(); + if (connection) { + return connection.withChannel(SANDBOX_HELPER_CHANNEL_NAME, channel => { + const sandboxHelper = new SandboxHelperChannelClient(channel); + return sandboxHelper.getWindowsMxcEnvironment(); + }); + } + return this._sandboxHelperService.getWindowsMxcEnvironment(); + } + // ---- workbench-only flows ----------------------------------------------- async installMissingSandboxDependencies(missingDependencies: string[], sessionResource: URI | undefined, token: CancellationToken, options: ISandboxDependencyInstallOptions): Promise { diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts index 90712f2521b5fd..dd43231744e0d3 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts @@ -27,7 +27,8 @@ import { IRemoteAgentEnvironment } from '../../../../../../platform/remote/commo import { IWorkspace, IWorkspaceContextService, IWorkspaceFolder, IWorkspaceFoldersChangeEvent, IWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, WorkbenchState } from '../../../../../../platform/workspace/common/workspace.js'; import { testWorkspace } from '../../../../../../platform/workspace/test/common/testWorkspace.js'; import { ILifecycleService } from '../../../../../services/lifecycle/common/lifecycle.js'; -import { ISandboxDependencyStatus, ISandboxHelperService } from '../../../../../../platform/sandbox/common/sandboxHelperService.js'; +import { ISandboxDependencyStatus, ISandboxHelperService, IWindowsMxcFilesystemPolicy } from '../../../../../../platform/sandbox/common/sandboxHelperService.js'; +import { IWindowsMxcTerminalSandboxRuntime, WindowsMxcTerminalSandboxRuntime } from '../../../../../../platform/sandbox/common/terminalSandboxMxcRuntime.js'; import { getTerminalSandboxRuntimeConfigurationForCommands } from '../../../../../../platform/sandbox/common/terminalSandboxRuntimeConfigurationPerOperation.js'; suite('TerminalSandboxService - network domains', () => { @@ -52,6 +53,10 @@ suite('TerminalSandboxService - network domains', () => { createFileCount++; const contentString = content.toString(); createdFiles.set(uri.path, contentString); + createdFiles.set(uri.fsPath, contentString); + if (/^\/[a-zA-Z]:/.test(uri.path)) { + createdFiles.set(uri.path.slice(1).replace(/\//g, '\\'), contentString); + } return {}; } @@ -96,8 +101,8 @@ suite('TerminalSandboxService - network domains', () => { } async getEnvironment(): Promise { - // Return a Linux environment to ensure tests pass on Windows - // (sandbox is not supported on Windows) + // Return a Linux environment so the default test expectations are + // independent of the local OS. return this.remoteEnvironment; } } @@ -157,11 +162,24 @@ suite('TerminalSandboxService - network domains', () => { bubblewrapInstalled: true, socatInstalled: true, }; + filesystemPolicy: IWindowsMxcFilesystemPolicy = { + readonlyPaths: ['c:\\tools\\node'], + readwritePaths: [], + }; + environment = ['PATH=c:\\tools\\node;c:\\windows\\system32', 'PSHOME=c:\\program files\\powershell\\7']; checkSandboxDependencies(): Promise { this.callCount++; return Promise.resolve(this.status); } + + getWindowsMxcFilesystemPolicy(): Promise { + return Promise.resolve(this.filesystemPolicy); + } + + getWindowsMxcEnvironment(): Promise { + return Promise.resolve(this.environment); + } } setup(() => { @@ -206,6 +224,7 @@ suite('TerminalSandboxService - network domains', () => { instantiationService.stub(IWorkspaceContextService, workspaceContextService); instantiationService.stub(ILifecycleService, lifecycleService); instantiationService.stub(ISandboxHelperService, sandboxHelperService); + instantiationService.stub(IWindowsMxcTerminalSandboxRuntime, instantiationService.createInstance(WindowsMxcTerminalSandboxRuntime)); }); test('dependency checks should not be called for isEnabled', async () => { @@ -1164,7 +1183,7 @@ suite('TerminalSandboxService - network domains', () => { test('should prefix wrapped command with ELECTRON_RUN_AS_NODE=1 when no remote env is available', async function () { if (isWindows) { - // Sandbox is disabled on Windows when no remote env is available. + // Local Windows uses MXC, which launches wxc-exec.exe directly instead of SRT through Electron-as-Node. this.skip(); } remoteAgentService.remoteEnvironment = null; @@ -1185,9 +1204,70 @@ suite('TerminalSandboxService - network domains', () => { ok(!wrapped.command.startsWith('ELECTRON_RUN_AS_NODE='), `Remote workbench should not add the env prefix. Actual: ${wrapped.command}`); }); + test('should route remote Windows sandbox commands through MXC', async () => { + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxEnabled, AgentSandboxEnabledValue.Off); + configurationService.setUserConfiguration(AgentSandboxSettingId.AgentSandboxWindowsEnabled, AgentSandboxEnabledValue.AllowNetwork); + remoteAgentService.remoteEnvironment = { + ...remoteAgentService.remoteEnvironment!, + os: OperatingSystem.Windows, + appRoot: URI.file('/c:/app'), + execPath: 'c:\\app\\Code.exe', + tmpDir: URI.file('/c:/tmp'), + userHome: URI.file('/c:/Users/test'), + workspaceStorageHome: URI.file('/c:/Users/test/AppData/Roaming/Code/User/workspaceStorage'), + arch: 'arm64' + }; + workspaceContextService.setWorkspaceFolders([URI.file('/c:/workspace-one')]); + const sandboxService = store.add(instantiationService.createInstance(TerminalSandboxService)); + + const configPath = await sandboxService.getSandboxConfigPath(); + const wrapped = await sandboxService.wrapCommand('echo test', false, 'pwsh.exe', URI.file('/c:/workspace-one')); + + ok(configPath, 'Config path should be defined for remote Windows'); + const configContent = createdFiles.get(configPath); + ok(configContent, 'Config file should be created for remote Windows'); + const config = JSON.parse(configContent); + + strictEqual(wrapped.isSandboxWrapped, true); + ok(wrapped.command.includes('node_modules\\@microsoft\\mxc-sdk\\bin\\arm64\\wxc-exec.exe'), `Wrapped command should use the MXC Windows executable. Actual: ${wrapped.command}`); + ok(wrapped.command.includes(configPath), `Wrapped command should pass the MXC config path. Actual: ${wrapped.command}`); + strictEqual(config.version, '0.4.0-alpha'); + strictEqual(config.containment, 'process'); + strictEqual(config.process.commandLine, 'echo test'); + strictEqual(config.process.cwd, 'c:\\workspace-one'); + ok(config.process.env.includes('PATH=c:\\tools\\node;c:\\windows\\system32'), 'PATH should be injected into the MXC process env'); + ok(config.process.env.includes('PSHOME=c:\\program files\\powershell\\7'), 'PSHOME should be injected into the MXC process env'); + ok(config.filesystem.readwritePaths.includes('c:\\workspace-one'), 'Workspace folder should be writable in the MXC config'); + ok(config.filesystem.readwritePaths.some((path: string) => path.includes('tmp_vscode_7')), 'Sandbox temp dir should be writable in the MXC config'); + ok(config.filesystem.readonlyPaths.includes('c:\\app'), 'VS Code app root should be readable in the MXC config'); + ok(config.filesystem.readonlyPaths.includes('c:\\tools\\node'), 'MXC available tools policy should add tool paths to readonly paths'); + ok(!config.filesystem.deniedPaths.includes('c:\\Users\\test'), 'User home should not be denied by default in the MXC config on Windows'); + }); + + test('should keep remote Windows sandbox disabled unless Windows sandbox setting allows network', async () => { + remoteAgentService.remoteEnvironment = { + ...remoteAgentService.remoteEnvironment!, + os: OperatingSystem.Windows, + appRoot: URI.file('/c:/app'), + execPath: 'c:\\app\\Code.exe', + tmpDir: URI.file('/c:/tmp'), + userHome: URI.file('/c:/Users/test'), + workspaceStorageHome: URI.file('/c:/Users/test/AppData/Roaming/Code/User/workspaceStorage'), + arch: 'x64' + }; + workspaceContextService.setWorkspaceFolders([URI.file('/c:/workspace-one')]); + const sandboxService = store.add(instantiationService.createInstance(TerminalSandboxService)); + + strictEqual(await sandboxService.isEnabled(), false, 'Windows sandbox should be disabled when the Windows sandbox setting is off'); + strictEqual(await sandboxService.getSandboxConfigPath(), undefined, 'Windows sandbox config should not be created unless the Windows setting is enabled'); + const prereqs = await sandboxService.checkForSandboxingPrereqs(); + strictEqual(prereqs.enabled, false, 'Prereq checks should report Windows sandbox disabled when the Windows setting is disabled'); + strictEqual(prereqs.failedCheck, undefined, 'No prereq check should fail when Windows sandboxing is disabled'); + }); + test('should place sandbox temp dir under the local data folder when no remote env is available', async function () { if (isWindows) { - // Sandbox is disabled on Windows when no remote env is available. + // Local Windows uses MXC, which does not use the POSIX local temp-dir path shape asserted below. this.skip(); } remoteAgentService.remoteEnvironment = null; From db056114a475ecc4ae07aa53bff82fade1e1f2fd Mon Sep 17 00:00:00 2001 From: Taylor Blair Date: Thu, 21 May 2026 13:22:27 -0700 Subject: [PATCH 13/43] Report existing turn in response success telemetry --- .../copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts b/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts index ff80b7256ad35b..f47c6bc89abebf 100644 --- a/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts +++ b/extensions/copilot/src/extension/prompt/node/chatMLFetcherTelemetry.ts @@ -96,7 +96,7 @@ function getTurnFromBaseTelemetry(baseTelemetry: TelemetryData): number | undefi } const parsedTurnIndex = Number(turnIndex); - return Number.isFinite(parsedTurnIndex) ? parsedTurnIndex + 1 : undefined; + return Number.isFinite(parsedTurnIndex) ? parsedTurnIndex : undefined; } export class ChatMLFetcherTelemetrySender { From 05d3f35d12d460b08a2d1a87f95cd75db9ba4644 Mon Sep 17 00:00:00 2001 From: "vs-code-engineering[bot]" <122617954+vs-code-engineering[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 20:34:50 +0000 Subject: [PATCH 14/43] [cherry-pick] Reverting fetch web tool changes for sandboxing (#317199) Co-authored-by: vs-code-engineering[bot] --- .../sharedProcess/sharedProcessMain.ts | 3 +- .../common/networkFilterService.ts | 44 ++---------- .../test/common/networkFilterService.test.ts | 71 ++----------------- .../builtInTools/fetchPageTool.ts | 4 +- 4 files changed, 16 insertions(+), 106 deletions(-) diff --git a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts index 3d52370163fdf7..8a86a911aa423b 100644 --- a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts @@ -141,7 +141,6 @@ import { IMeteredConnectionService } from '../../../platform/meteredConnection/c import { MeteredConnectionChannelClient, METERED_CONNECTION_CHANNEL } from '../../../platform/meteredConnection/common/meteredConnectionIpc.js'; import { PlaywrightChannel } from '../../../platform/browserView/node/playwrightChannel.js'; import { AgentNetworkFilterService } from '../../../platform/networkFilter/common/networkFilterService.js'; -import { NullTerminalSandboxService } from '../../../platform/sandbox/common/terminalSandboxService.js'; import { ILocalGitService } from '../../../platform/git/common/localGitService.js'; import { LocalGitService } from '../../../platform/git/node/localGitService.js'; @@ -494,7 +493,7 @@ class SharedProcessMain extends Disposable implements IClientConnectionFilter { this.server.registerChannel('sharedWebContentExtractor', webContentExtractorChannel); // Playwright - const agentNetworkFilterService = this._register(new AgentNetworkFilterService(accessor.get(IConfigurationService), new NullTerminalSandboxService())); + const agentNetworkFilterService = this._register(new AgentNetworkFilterService(accessor.get(IConfigurationService))); const playwrightChannel = this._register(new PlaywrightChannel(this.server, accessor.get(IMainProcessService), accessor.get(ILogService), agentNetworkFilterService)); this.server.registerChannel('playwright', playwrightChannel); diff --git a/src/vs/platform/networkFilter/common/networkFilterService.ts b/src/vs/platform/networkFilter/common/networkFilterService.ts index 5dc2ebdeb33345..fa147b2b2cb255 100644 --- a/src/vs/platform/networkFilter/common/networkFilterService.ts +++ b/src/vs/platform/networkFilter/common/networkFilterService.ts @@ -10,22 +10,17 @@ import { URI } from '../../../base/common/uri.js'; import { localize } from '../../../nls.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; -import { AgentSandboxSettingId } from '../../sandbox/common/settings.js'; -import { ITerminalSandboxService } from '../../sandbox/common/terminalSandboxService.js'; import { extractDomainFromUri, isDomainAllowed } from './domainMatcher.js'; import { AgentNetworkDomainSettingId } from './settings.js'; export const IAgentNetworkFilterService = createDecorator('agentNetworkFilterService'); -export const AgentNetworkFilterFetchWebToolName = 'fetchWebTool'; - /** * Service that filters network requests made by agent tools (fetch tool, * integrated browser) based on the configured allowed/denied domain lists. * * Filtering is active for all callers when the `chat.agent.networkFilter` setting - * is enabled. When only sandboxing is enabled, filtering is active for fetch web - * page tool requests. This has to be revisited for integrated browser requests. + * is enabled. * When both domain lists are empty, all domains are denied. * When a domain appears on the denied list it is always blocked, even if it * also matches an entry on the allowed list. @@ -37,10 +32,9 @@ export interface IAgentNetworkFilterService { * Extracts the domain from a URI and checks it against the configured * allowed/denied domain filter. * File URIs and URIs without an authority always pass. - * @param toolName Optional tool name for sandbox-only filtering. * @returns `true` if the URI's domain is allowed, `false` if blocked. */ - isUriAllowed(uri: URI, toolName?: string): boolean; + isUriAllowed(uri: URI): boolean; /** * Formats an error message for a blocked URI based on the current filter configuration. @@ -59,7 +53,6 @@ export class AgentNetworkFilterService extends Disposable implements IAgentNetwo readonly _serviceBrand: undefined; private networkFilterEnabled = false; - private terminalSandboxEnabled = false; private allowedPatterns: string[] = []; private deniedPatterns: string[] = []; private readonly domainCache = new LRUCache(100); @@ -69,11 +62,9 @@ export class AgentNetworkFilterService extends Disposable implements IAgentNetwo constructor( @IConfigurationService private readonly configurationService: IConfigurationService, - @ITerminalSandboxService private readonly terminalSandboxService: ITerminalSandboxService, ) { super(); this.readConfiguration(); - void this.updateTerminalSandboxEnabled(); this._register(this.configurationService.onDidChangeConfiguration(e => { if ( @@ -83,11 +74,6 @@ export class AgentNetworkFilterService extends Disposable implements IAgentNetwo ) { this.readConfiguration(); this.onDidChangeEmitter.fire(); - } else if ( - e.affectsConfiguration(AgentSandboxSettingId.AgentSandboxEnabled) || - e.affectsConfiguration(AgentSandboxSettingId.DeprecatedAgentSandboxEnabled) - ) { - void this.updateTerminalSandboxEnabled(); } })); } @@ -101,23 +87,9 @@ export class AgentNetworkFilterService extends Disposable implements IAgentNetwo this.domainCache.clear(); } - private async updateTerminalSandboxEnabled(): Promise { - const [isSandboxEnabled, isSandboxAllowNetworkEnabled] = await Promise.all([ - this.terminalSandboxService.isEnabled(), - this.terminalSandboxService.isSandboxAllowNetworkEnabled(), - ]); - const enabled = isSandboxEnabled && !isSandboxAllowNetworkEnabled; - if (this.terminalSandboxEnabled === enabled) { - return; - } - this.terminalSandboxEnabled = enabled; - this.readConfiguration(); - this.onDidChangeEmitter.fire(); - } - - isUriAllowed(uri: URI, toolName?: string): boolean { + isUriAllowed(uri: URI): boolean { // When domain filtering is inactive, allow all requests. - if (!this.shouldFilter(toolName)) { + if (!this.shouldFilter()) { return true; } @@ -140,11 +112,9 @@ export class AgentNetworkFilterService extends Disposable implements IAgentNetwo return result; } // Determines whether network filtering should be applied for a given request - // based on the global network filter setting, the terminal sandbox state, and the tool making the request. - // For sandbox mode, network filtering is applied only when the global network filter is disabled - // and the request is coming from the fetch web tool. - private shouldFilter(toolName: string | undefined): boolean { - return this.networkFilterEnabled || (this.terminalSandboxEnabled && toolName === AgentNetworkFilterFetchWebToolName); + // based on the global network filter setting. + private shouldFilter(): boolean { + return this.networkFilterEnabled; } formatError(uri: URI): string { diff --git a/src/vs/platform/networkFilter/test/common/networkFilterService.test.ts b/src/vs/platform/networkFilter/test/common/networkFilterService.test.ts index b491b008f49986..d2aad37425fcfd 100644 --- a/src/vs/platform/networkFilter/test/common/networkFilterService.test.ts +++ b/src/vs/platform/networkFilter/test/common/networkFilterService.test.ts @@ -9,28 +9,17 @@ import { DisposableStore } from '../../../../base/common/lifecycle.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { ConfigurationTarget } from '../../../configuration/common/configuration.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; -import { AgentNetworkFilterFetchWebToolName, AgentNetworkFilterService } from '../../common/networkFilterService.js'; +import { AgentNetworkFilterService } from '../../common/networkFilterService.js'; import { AgentNetworkDomainSettingId } from '../../common/settings.js'; -import { AgentSandboxSettingId } from '../../../sandbox/common/settings.js'; -import { ITerminalSandboxService, NullTerminalSandboxService } from '../../../sandbox/common/terminalSandboxService.js'; suite('AgentNetworkFilterService', () => { let disposables: DisposableStore; let configService: TestConfigurationService; - let terminalSandboxEnabled: boolean; - let terminalSandboxAllowNetworkEnabled: boolean; - let terminalSandboxService: ITerminalSandboxService; setup(() => { disposables = new DisposableStore(); configService = new TestConfigurationService(); - terminalSandboxEnabled = false; - terminalSandboxAllowNetworkEnabled = false; - terminalSandboxService = Object.assign(new NullTerminalSandboxService(), { - isEnabled: async () => terminalSandboxEnabled, - isSandboxAllowNetworkEnabled: async () => terminalSandboxAllowNetworkEnabled, - }); configService.setUserConfiguration(AgentNetworkDomainSettingId.NetworkFilter, true); configService.setUserConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains, []); configService.setUserConfiguration(AgentNetworkDomainSettingId.DeniedNetworkDomains, []); @@ -43,9 +32,8 @@ suite('AgentNetworkFilterService', () => { ensureNoDisposablesAreLeakedInTestSuite(); async function createService(): Promise { - const service = new AgentNetworkFilterService(configService, terminalSandboxService); + const service = new AgentNetworkFilterService(configService); disposables.add(service); - await Promise.resolve(); return service; } @@ -58,36 +46,16 @@ suite('AgentNetworkFilterService', () => { }); } - test('allows all domains when filter is disabled', async () => { - configService.setUserConfiguration(AgentNetworkDomainSettingId.NetworkFilter, false); - const service = await createService(); - assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://anything.test')), true); - }); - - test('network filter disabled with sandbox enabled filters fetch web tool only', async () => { + test('allows all domains when filter is disabled, regardless of configured lists', async () => { configService.setUserConfiguration(AgentNetworkDomainSettingId.NetworkFilter, false); - terminalSandboxEnabled = true; configService.setUserConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains, ['example.com']); + configService.setUserConfiguration(AgentNetworkDomainSettingId.DeniedNetworkDomains, ['blocked.com']); const service = await createService(); assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com'), AgentNetworkFilterFetchWebToolName), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com'), AgentNetworkFilterFetchWebToolName), false); - }); - - test('network filter disabled with sandbox network allowed does not activate filtering', async () => { - configService.setUserConfiguration(AgentNetworkDomainSettingId.NetworkFilter, false); - terminalSandboxEnabled = true; - terminalSandboxAllowNetworkEnabled = true; - configService.setUserConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains, ['example.com']); - - const service = await createService(); - - assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com')), true); + assert.strictEqual(service.isUriAllowed(URI.parse('https://anything.test')), true); + assert.strictEqual(service.isUriAllowed(URI.parse('https://blocked.com')), true); }); test('denies all domains when both lists are empty', async () => { @@ -162,31 +130,4 @@ suite('AgentNetworkFilterService', () => { assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com')), false); }); - test('terminal sandbox network mode change fires onDidChange and updates fetch web tool filtering', async () => { - configService.setUserConfiguration(AgentNetworkDomainSettingId.NetworkFilter, false); - configService.setUserConfiguration(AgentNetworkDomainSettingId.AllowedNetworkDomains, ['example.com']); - terminalSandboxEnabled = true; - terminalSandboxAllowNetworkEnabled = true; - const service = await createService(); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com'), AgentNetworkFilterFetchWebToolName), true); - - let fired = false; - const didChange = new Promise(resolve => { - disposables.add(service.onDidChange(() => { - fired = true; - resolve(); - })); - }); - - terminalSandboxAllowNetworkEnabled = false; - fireConfigChange(AgentSandboxSettingId.AgentSandboxEnabled); - await didChange; - - assert.strictEqual(fired, true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com')), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://example.com'), AgentNetworkFilterFetchWebToolName), true); - assert.strictEqual(service.isUriAllowed(URI.parse('https://other.com'), AgentNetworkFilterFetchWebToolName), false); - }); }); diff --git a/src/vs/workbench/contrib/chat/electron-browser/builtInTools/fetchPageTool.ts b/src/vs/workbench/contrib/chat/electron-browser/builtInTools/fetchPageTool.ts index 68643ff74edc94..32963a68d60c5e 100644 --- a/src/vs/workbench/contrib/chat/electron-browser/builtInTools/fetchPageTool.ts +++ b/src/vs/workbench/contrib/chat/electron-browser/builtInTools/fetchPageTool.ts @@ -20,7 +20,7 @@ import { IChatService } from '../../common/chatService/chatService.js'; import { ChatImageMimeType } from '../../common/languageModels.js'; import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, IToolResultDataPart, IToolResultTextPart, ToolDataSource, ToolProgress } from '../../common/tools/languageModelToolsService.js'; import { InternalFetchWebPageToolId } from '../../common/tools/builtinTools/tools.js'; -import { AgentNetworkFilterFetchWebToolName, IAgentNetworkFilterService } from '../../../../../platform/networkFilter/common/networkFilterService.js'; +import { IAgentNetworkFilterService } from '../../../../../platform/networkFilter/common/networkFilterService.js'; import { WorkingDirectory } from '../../common/workingDirectory.js'; export const FetchWebPageToolData: IToolData = { @@ -291,7 +291,7 @@ export class FetchWebPageTool implements IToolImpl { try { const uriObj = URI.parse(url); if (uriObj.scheme === 'http' || uriObj.scheme === 'https') { - if (!this._agentNetworkFilterService.isUriAllowed(uriObj, AgentNetworkFilterFetchWebToolName)) { + if (!this._agentNetworkFilterService.isUriAllowed(uriObj)) { blockedUris.add(url); } else { webUris.set(url, uriObj); From 3b3dd1b58387295201a3e2bd1f49afee50d4ef48 Mon Sep 17 00:00:00 2001 From: cwebster-99 Date: Thu, 21 May 2026 15:48:57 -0500 Subject: [PATCH 15/43] Hide anonymous access setting from settings ui --- .../workbench/contrib/chat/browser/chat.shared.contribution.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index 447c78f56f1fef..ed6ed50940d3eb 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -1641,6 +1641,7 @@ configurationRegistry.registerConfiguration({ type: 'boolean', description: nls.localize('chat.allowAnonymousAccess', "Controls whether anonymous access is allowed in chat."), default: false, + included: false, tags: ['experimental'], experiment: { mode: 'auto' From bc13ab1981f0686670df914732483800dc877064 Mon Sep 17 00:00:00 2001 From: Vijay Upadya <41652029+vijayupadya@users.noreply.github.com> Date: Thu, 21 May 2026 14:11:44 -0700 Subject: [PATCH 16/43] Chronicle: Add cost-tips command (#317809) * Chronicle: Add cost-tips command * feedback updates --- .../prompts/chronicle-cost-tips.prompt.md | 5 ++ .../assets/prompts/skills/chronicle/SKILL.md | 74 +++++++++++++++++++ extensions/copilot/package.json | 7 ++ .../node/test/sessionStoreSqlTool.spec.ts | 18 ++++- 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 extensions/copilot/assets/prompts/chronicle-cost-tips.prompt.md diff --git a/extensions/copilot/assets/prompts/chronicle-cost-tips.prompt.md b/extensions/copilot/assets/prompts/chronicle-cost-tips.prompt.md new file mode 100644 index 00000000000000..816e0fb4029319 --- /dev/null +++ b/extensions/copilot/assets/prompts/chronicle-cost-tips.prompt.md @@ -0,0 +1,5 @@ +--- +name: chronicle:cost-tips +description: Get personalized tips to reduce token usage and Copilot cost +--- +Analyze my recent chat session history and give me personalized, data-grounded tips to reduce token usage and Copilot cost. Use the **chronicle** skill — it documents the `copilot_sessionStoreSql` tool, the session-store schema, and the Cost Tips workflow for finding expensive sessions, token-heavy patterns, and concrete habit changes. diff --git a/extensions/copilot/assets/prompts/skills/chronicle/SKILL.md b/extensions/copilot/assets/prompts/skills/chronicle/SKILL.md index b5fe96a1b032b5..584dff31936d3e 100644 --- a/extensions/copilot/assets/prompts/skills/chronicle/SKILL.md +++ b/extensions/copilot/assets/prompts/skills/chronicle/SKILL.md @@ -96,6 +96,80 @@ Analysis dimensions to explore: If the session store has little data, acknowledge that and suggest features to try based on what configuration you found in the workspace. +### Cost Tips + +When the user asks for cost tips, ways to reduce token usage, or how to lower Copilot spend (e.g. `/chronicle:cost-tips`): + +The goal is **personalized, data-grounded recommendations** for reducing token usage — not a generic checklist. Every tip must point to a specific pattern you observed in their data. + +**Cost-relevant schema (in addition to the Database Schema section below)** + +- **Cloud DuckDB only** — the local SQLite store does **not** record per-event token usage and has no `events` table. If the active backend is local, gate all token queries and tell the user that real token-level analysis requires enabling cloud sync (`chat.sessionSync.enabled`). +- **events** (cloud): per-event billing — rows where `type = 'assistant.usage'` carry `usage_input_tokens`, `usage_output_tokens`, `usage_model`. To break spend down by agent type, JOIN `events e` to `sessions s ON s.id = e.session_id` and group by `s.agent_name`. +- **sessions.agent_name** / **agent_description** (both backends): values like `VS Code agent` (VS Code chat), `Copilot CLI`, `Copilot Coding Agent`, `Copilot Code Review`, or custom agents/subagents. Use to break spend down by agent type. +- Use `LENGTH(user_message)` on `turns` (or `LENGTH(user_content)` on `events` where `type = 'user.message'`) to find oversized pastes. + +**Step 1: Investigate cost and token patterns** + +Use `copilot_sessionStoreSql` with `action: "query"`. What to investigate depends on the active backend. + +*Cloud (DuckDB) — start with agent-type awareness:* + +The session store mixes session types via `sessions.agent_name` (join events to sessions on `session_id` to get the agent for any per-event analysis). Your advice is only useful if you know which agents the user actually runs, so this is the **first** thing to learn. + +- **Enumerate every agent in use.** Run e.g. `SELECT agent_name, agent_description, COUNT(*) AS n FROM sessions WHERE updated_at > now() - INTERVAL '30 days' GROUP BY 1, 2 ORDER BY n DESC` so you see the full inventory — official agents and any custom agents/subagents in `agent_description`. Do not assume. +- **Decide which to advise on.** Include any agent type the user can make cheaper: `VS Code agent` (VS Code chat — usually the dominant agent), `Copilot CLI` (interactive terminal), `Copilot Coding Agent` (autonomous cloud tasks), custom agents and subagents. **Always exclude** `agent_name = 'Copilot Code Review'` and any other agent the user does not drive interactively. +- **Tailor advice per agent.** VS Code agent tips (compaction, model picker, fresh chats, `.github/copilot-instructions.md`, custom skills/agents) look different from CLI tips (compaction, model switching, subagent delegation), Coding Agent tips (prompt scoping, smaller task framing), and custom-agent tips (slimming tool lists, narrowing prompts). + +Then drill into cost patterns (filter `events` rows by `type = 'assistant.usage'` for billable rows): + +- **Token-heavy sessions and turns** — sum `usage_input_tokens` and `usage_output_tokens` per session and per model from `events` where `type = 'assistant.usage'`. Which sessions burned the most tokens? Which models? +- **Input-to-output ratios** — when input tokens dwarf output tokens, the user is paying to re-send a bloated context every turn. Strongest signal that compaction, smaller working sets, or fresh sessions would help. +- **Model mix** — break down spend by `usage_model`. Are premium models being used for routine work (renames, simple edits, status checks) that a cheaper model could handle? +- **Per-turn growth** — within long sessions, does `usage_input_tokens` keep climbing turn-over-turn? Strong signal that compaction wasn't used. +- **Oversized pastes** — `LENGTH(user_content)` on `events` where `type = 'user.message'` to find user messages that should have been file references (also visible in `session_files` as repeated reads of the same path within one session). +- **Group cost breakdowns by `agent_name`** (and `agent_description` where useful) in at least one query so the user sees where their spend actually goes — and so you spot if a single custom agent dominates. + +*Local (SQLite) — no token data; use proxies:* + +- **Long sessions without compaction** — sessions with many turns and no rows in `checkpoints` (each `checkpoints` row is a successful compaction). `LEFT JOIN checkpoints c ON c.session_id = s.id WHERE c.session_id IS NULL` + a turn-count threshold gives prime candidates. +- **Late compaction** — for sessions that *do* have checkpoints, compare `checkpoints.checkpoint_number` and `created_at` against the session's turn count. A first compaction at turn 60 of an 80-turn session is far less helpful than one at turn 25. +- **Repeated large file reads** — in `session_files`, look for the same file read many times within one session, or across sessions. +- **Tool-call thrash** — sessions with many turns and repeated tool calls often indicate the agent rediscovered the same context multiple times. +- **Oversized pastes** — use `LENGTH(user_message)` on `turns` to find very long user messages that should have been file references. + +*Both backends:* + +- **Long-running sessions** — sessions with many turns or that span many hours drag a growing context window across every turn. +- **Repeated work** — the same file/topic showing up in many sessions, or the same agent stumbling block recurring (suggesting a custom skill, agent, or `copilot-instructions.md` entry would let the model do the work in one shot). +- **Subagent usage** — are heavyweight investigations being run in the main session (paying for their tokens to live in main context) when they could be delegated to a subagent that returns only a summary? + +Drill into a few of the most expensive sessions and read the actual conversation turns to understand *why* they were expensive. Don't just report aggregates — explain the cause. + +**Step 2: Map findings to features and habits** + +If the current workspace has a `.github/` folder, check `.github/copilot-instructions.md`, `.github/skills/`, and `.github/agents/` to see what custom configuration already exists. Do NOT look outside the workspace. Cost-relevant capabilities to keep in mind: + +- Mid-session compaction (e.g. `/compact`) to shrink the context window; for users who never compact, this is often the single biggest win. +- Model picker — switch to a cheaper model for routine work; check whether premium models are being used for simple tasks. +- Starting a fresh chat instead of continuing a bloated session. +- Subagents/delegation for offloading heavy research into a sub-context whose tokens don't accrete into the main session. +- Custom skills (`.github/skills/`) and custom agents (`.github/agents/`) so repeated workflows don't re-derive context each time. +- `.github/copilot-instructions.md` to encode project conventions the model otherwise has to be told every session. +- For cloud-enabled users, the Copilot usage view to inspect current premium-request spend. + +**Step 3: Provide tips** + +Give the user 3-5 specific, actionable tips. Each tip should: + +- **Be grounded in their data** — reference a specific session, file, model, or pattern you observed (with rough numbers when you have them: turn counts, token totals, file-read counts, etc.). +- **Be non-obvious** — skip basics any returning user already knows. Assume they know compaction and fresh chats exist; help them notice they're not *using* them where it would matter. +- **Quantify the win when possible** — "compacting around turn 30 of that 80-turn session would have shaved ~X input tokens off every subsequent turn" is far better than "consider compacting". +- **Be concrete** — name the workflow change, command, or config file edit. If the suggestion is a custom skill or agent, sketch what it would cover. +- **Match the agent type** — if a finding is specific to one `agent_name`, say so. Don't propose CLI-only fixes for findings from Coding Agent sessions, and vice versa. + +If the session store has little data (e.g., cloud store is empty, or only a handful of local sessions), say so plainly and offer 2-3 non-obvious cost-saving habits anchored in available features rather than fabricating findings. If the user is on local-only storage, end by noting that enabling `chat.sessionSync.enabled` unlocks per-event token analysis for sharper future tips. + ### Search When the user asks to search, find, or look up past sessions by keyword (e.g. `/chronicle:search `): diff --git a/extensions/copilot/package.json b/extensions/copilot/package.json index 3b48d774a9e798..7d973bf2549fed 100644 --- a/extensions/copilot/package.json +++ b/extensions/copilot/package.json @@ -6619,6 +6619,13 @@ "local" ] }, + { + "path": "./assets/prompts/chronicle-cost-tips.prompt.md", + "when": "github.copilot.sessionSearch.enabled", + "sessionTypes": [ + "local" + ] + }, { "path": "./assets/prompts/chronicle-reindex.prompt.md", "when": "github.copilot.sessionSearch.enabled", diff --git a/extensions/copilot/src/extension/tools/node/test/sessionStoreSqlTool.spec.ts b/extensions/copilot/src/extension/tools/node/test/sessionStoreSqlTool.spec.ts index 3948b299e8dd1c..c72843e7e1e6e6 100644 --- a/extensions/copilot/src/extension/tools/node/test/sessionStoreSqlTool.spec.ts +++ b/extensions/copilot/src/extension/tools/node/test/sessionStoreSqlTool.spec.ts @@ -480,12 +480,28 @@ describe('SessionStoreSqlTool', () => { it('chronicle slash prompts reference the chronicle skill', () => { const promptDir = path.join(copilotRoot, 'assets', 'prompts'); - const prompts = ['chronicle-tips.prompt.md', 'chronicle-standup.prompt.md', 'chronicle-search.prompt.md']; + const prompts = ['chronicle-tips.prompt.md', 'chronicle-cost-tips.prompt.md', 'chronicle-standup.prompt.md', 'chronicle-search.prompt.md']; const missing = prompts.filter(name => { const body = fs.readFileSync(path.join(promptDir, name), 'utf-8'); return !/\*\*chronicle\*\* skill/.test(body); }); expect(missing).toEqual([]); }); + + it('chronicle SKILL.md Cost Tips section carries required anchors', () => { + const skill = fs.readFileSync( + path.join(copilotRoot, 'assets', 'prompts', 'skills', 'chronicle', 'SKILL.md'), + 'utf-8', + ); + const required = [ + '### Cost Tips', + 'usage_input_tokens', 'usage_output_tokens', 'usage_model', + 'agent_name', + 'assistant.usage', + 'local SQLite', + 'chat.sessionSync.enabled', + ]; + expect(required.filter(a => !skill.includes(a))).toEqual([]); + }); }); }); From 8725a47e28394ac639efd7df91fe676a0422a34e Mon Sep 17 00:00:00 2001 From: Taylor Blair Date: Thu, 21 May 2026 14:20:25 -0700 Subject: [PATCH 17/43] Fix tool telemetry test spy setup --- .../extension/prompts/node/panel/test/toolCalling.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts b/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts index 3ca0e304885ca8..658aa81b5ee2e2 100644 --- a/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts +++ b/extensions/copilot/src/extension/prompts/node/panel/test/toolCalling.spec.ts @@ -8,13 +8,14 @@ import type * as vscode from 'vscode'; import { IChatHookService, type IPreToolUseHookResult } from '../../../../../platform/chat/common/chatHookService'; import { ConfigKey, IConfigurationService } from '../../../../../platform/configuration/common/configurationService'; import { IEndpointProvider } from '../../../../../platform/endpoint/common/endpointProvider'; +import type { IChatEndpoint } from '../../../../../platform/networking/common/networking'; import { DeferredPromise } from '../../../../../util/vs/base/common/async'; import { CancellationToken } from '../../../../../util/vs/base/common/cancellation'; import { Event } from '../../../../../util/vs/base/common/event'; import { constObservable } from '../../../../../util/vs/base/common/observable'; import { IInstantiationService } from '../../../../../util/vs/platform/instantiation/common/instantiation'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry'; -import type { SpyingTelemetryService } from '../../../../../platform/telemetry/node/spyingTelemetryService'; +import { SpyingTelemetryService } from '../../../../../platform/telemetry/node/spyingTelemetryService'; import { LanguageModelDataPart, LanguageModelTextPart, LanguageModelToolResult } from '../../../../../vscodeTypes'; import { ChatVariablesCollection } from '../../../../prompt/common/chatVariablesCollection'; import type { Conversation } from '../../../../prompt/common/conversation'; @@ -596,12 +597,13 @@ describe('ChatToolCalls (toolCalling.tsx)', () => { // of undefined (reading "supportsVision")' when a tool result contained an image. const testingServiceCollection = createExtensionUnitTestingServices(); + const spyingTelemetryService = new SpyingTelemetryService(); + testingServiceCollection.define(ITelemetryService, spyingTelemetryService); const accessor = testingServiceCollection.createTestingAccessor(); const instantiationService = accessor.get(IInstantiationService); const endpointProvider = accessor.get(IEndpointProvider); const endpoint = await endpointProvider.getChatEndpoint('copilot-utility'); const telemetryService = accessor.get(ITelemetryService); - const spyingTelemetryService = telemetryService as SpyingTelemetryService; const configService = accessor.get(IConfigurationService); // Disable image uploads in test environment to avoid auth requirement @@ -623,7 +625,7 @@ describe('ChatToolCalls (toolCalling.tsx)', () => { expect(() => { sendInvokedToolTelemetry( instantiationService, - endpoint as any, // endpoint satisfies IChatEndpoint + endpoint as IChatEndpoint, telemetryService, 'testTool', toolResult, From 7680cf4dc4667e7df6eddba00d93d526b654237f Mon Sep 17 00:00:00 2001 From: Kyle Cutler <67761731+kycutler@users.noreply.github.com> Date: Thu, 21 May 2026 14:32:29 -0700 Subject: [PATCH 18/43] Browser: support element selection in subframes (#317405) * Browser: support element selection in subframes * feedback * graceful handling after frame disposal --- .../electron-browser/preload-browserView.ts | 66 ++- .../browserView/electron-main/browserView.ts | 6 +- ...pector.ts => browserViewFrameInspector.ts} | 522 ++++++++---------- .../electron-main/browserViewInspector.ts | 475 ++++++++++++++++ .../electron-main/browserViewMainService.ts | 4 +- 5 files changed, 744 insertions(+), 329 deletions(-) rename src/vs/platform/browserView/electron-main/{browserViewElementInspector.ts => browserViewFrameInspector.ts} (50%) create mode 100644 src/vs/platform/browserView/electron-main/browserViewInspector.ts diff --git a/src/vs/platform/browserView/electron-browser/preload-browserView.ts b/src/vs/platform/browserView/electron-browser/preload-browserView.ts index 27bd96adc253da..80f4d61766227a 100644 --- a/src/vs/platform/browserView/electron-browser/preload-browserView.ts +++ b/src/vs/platform/browserView/electron-browser/preload-browserView.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ /* eslint-disable no-restricted-globals */ +/* eslint-disable no-restricted-syntax */ // Only `import type` is allowed in preload scripts — Electron preloads cannot resolve module imports at runtime. import type { IBrowserViewTheme } from '../common/browserView.js'; @@ -46,7 +47,6 @@ function init() { // Listen for keydown events that the page did not handle and forward them for shortcut handling. window.addEventListener('keydown', (event) => { // Require that the event is trusted -- i.e. user-initiated. - // eslint-disable-next-line no-restricted-syntax if (!(event instanceof KeyboardEvent) || !event.isTrusted) { return; } @@ -155,10 +155,29 @@ function init() { } }, { capture: true }); + // Invoked over IPC to support frames (executeJavaScriptInIsolatedWorld doesn't exist on WebFrameMain). + ipcRenderer.on('vscode:browserView:setTheme', (_event: unknown, theme: IBrowserViewTheme) => { + elementPicker.setTheme(theme); + }); + ipcRenderer.on('vscode:browserView:startElementPicker', (_event: unknown) => { + elementPicker.start(); + }); + ipcRenderer.on('vscode:browserView:stopElementPicker', (_event: unknown) => { + elementPicker.stop(); + }); + ipcRenderer.on('vscode:browserView:highlightElement', (_event: unknown, { elementId }: { elementId: string }) => { + const element = getElement(elementId); + if (element) { + elementPicker.highlight(element); + } + }); + ipcRenderer.on('vscode:browserView:hideHighlight', (_event: unknown) => { + elementPicker.hideHighlight(); + }); + const getElement = (id: string): Element | null => { switch (id) { case 'active': - // eslint-disable-next-line no-restricted-syntax return document.activeElement; case 'context-menu-target': return contextMenuTargetRef?.deref() ?? null; @@ -180,26 +199,18 @@ function init() { } catch { return ''; } - }, - setTheme(theme: IBrowserViewTheme): void { - elementPicker.setTheme(theme); - }, - pickElement: elementPicker.api, - highlightElement(id: string): boolean { - const element = getElement(id); - if (!element) { - return false; - } - elementPicker.highlight(element); - return true; - }, - hideHighlight(): void { - elementPicker.hideHighlight(); } }; + // Generate a unique token for this frame instance. This token is used to + // correlate the Electron WebFrameMain (available via IPC senderFrame) with + // the CDP target session (discoverable via Runtime.evaluate in the main world). + const frameToken = `frame-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const mainWorldHelpers = { - getElement + getElement, + /** Opaque token exposed for CDP-side frame matching. */ + getFrameToken(): string { return frameToken; } }; try { @@ -214,7 +225,7 @@ function init() { console.error(error); } - ipcRenderer.send('vscode:browserView:preloadReady'); + ipcRenderer.send('vscode:browserView:preloadReady', frameToken); } /** @@ -233,9 +244,7 @@ function findCommonVisibleAncestor(candidates: readonly (Node | null | undefined // Find the nearest visible ancestor of a single element. const findVisible = (el: Element): Element => { for (let cur: Element | null = el; cur; cur = cur.parentElement) { - // eslint-disable-next-line no-restricted-syntax const width = cur instanceof HTMLElement ? cur.offsetWidth : cur.clientWidth; - // eslint-disable-next-line no-restricted-syntax const height = cur instanceof HTMLElement ? cur.offsetHeight : cur.clientHeight; if (width > 0 && height > 0) { return cur; @@ -307,12 +316,6 @@ class ElementPicker { private _highlightTarget: Element | undefined; private _cursorStylesheet: HTMLStyleElement | undefined; - readonly api = { - start: (): boolean => this.start(), - stop: (): void => this.stop(), - isActive: (): boolean => this._selectionActive, - }; - constructor( private readonly _onPicked: (element: Element) => void, private readonly _onStopped: () => void @@ -376,7 +379,6 @@ class ElementPicker { return true; } this._continuous = false; // for now - // eslint-disable-next-line no-restricted-syntax document.documentElement.appendChild(this._shadowHost); this._selectionActive = true; @@ -385,13 +387,12 @@ class ElementPicker { // Updated to crosshair in _onPointerDown, reset in _onPointerUp. const cursorStyle = document.createElement('style'); cursorStyle.textContent = ElementPicker._CURSOR_DEFAULT; - // eslint-disable-next-line no-restricted-syntax document.head.appendChild(cursorStyle); this._cursorStylesheet = cursorStyle; // Register high-frequency listeners only while selection is active. window.addEventListener('pointermove', this._onPointerMove, true); - window.addEventListener('pointerleave', this._onPointerLeave, true); + document.addEventListener('pointerleave', this._onPointerLeave, true); window.addEventListener('pointerdown', this._onPointerDown, true); window.addEventListener('pointerup', this._onPointerUp, true); window.addEventListener('click', this._onClick, true); @@ -413,7 +414,7 @@ class ElementPicker { // Remove high-frequency listeners. window.removeEventListener('pointermove', this._onPointerMove, true); - window.removeEventListener('pointerleave', this._onPointerLeave, true); + document.removeEventListener('pointerleave', this._onPointerLeave, true); window.removeEventListener('pointerdown', this._onPointerDown, true); window.removeEventListener('pointerup', this._onPointerUp, true); window.removeEventListener('click', this._onClick, true); @@ -444,7 +445,6 @@ class ElementPicker { */ highlight(element: Element): void { if (!this._shadowHost.parentNode) { - // eslint-disable-next-line no-restricted-syntax document.documentElement.appendChild(this._shadowHost); } this._updateHighlight(element); @@ -584,7 +584,6 @@ class ElementPicker { /** Return the page element under a viewport point, skipping our own overlay host. */ private _pickElementAt(x: number, y: number): Element | undefined { - // eslint-disable-next-line no-restricted-syntax const candidates = document.elementsFromPoint(x, y); for (const el of candidates) { if (el === this._shadowHost || this._shadowHost.contains(el)) { @@ -654,7 +653,6 @@ class ElementPicker { const labelTop = Math.max(0, Math.min(viewportHeight - labelHeight, idealTop)); // Use clientWidth (excludes scrollbar) rather than innerWidth so the // label doesn't extend behind the scrollbar on Windows/Linux. - // eslint-disable-next-line no-restricted-syntax const viewportWidth = document.documentElement.clientWidth; // Position label at the element's left edge, but push it left if it // would overflow the viewport. Clamp to 0 so it never goes off-screen. diff --git a/src/vs/platform/browserView/electron-main/browserView.ts b/src/vs/platform/browserView/electron-main/browserView.ts index e1ae26c8e26263..52b7f923b934ed 100644 --- a/src/vs/platform/browserView/electron-main/browserView.ts +++ b/src/vs/platform/browserView/electron-main/browserView.ts @@ -8,7 +8,7 @@ import { Disposable } from '../../../base/common/lifecycle.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { VSBuffer } from '../../../base/common/buffer.js'; import { IBrowserViewBounds, IBrowserViewDevToolsStateEvent, IBrowserViewFocusEvent, IBrowserViewKeyDownEvent, IBrowserViewState, IBrowserViewNavigationEvent, IBrowserViewLoadingEvent, IBrowserViewLoadError, IBrowserViewTitleChangeEvent, IBrowserViewFaviconChangeEvent, IBrowserViewCaptureScreenshotOptions, IBrowserViewFindInPageOptions, IBrowserViewFindInPageResult, IBrowserViewVisibilityEvent, browserViewIsolatedWorldId, browserZoomFactors, browserZoomDefaultIndex, IBrowserViewOwner, IBrowserViewOpenOptions } from '../common/browserView.js'; -import { BrowserViewElementInspector } from './browserViewElementInspector.js'; +import { BrowserViewInspector } from './browserViewInspector.js'; import { IWindowsMainService } from '../../windows/electron-main/windows.js'; import { ICodeWindow, LoadReason } from '../../window/electron-main/window.js'; import { IAuxiliaryWindowsMainService } from '../../auxiliaryWindow/electron-main/auxiliaryWindows.js'; @@ -41,7 +41,7 @@ export class BrowserView extends Disposable { private _browserZoomIndex: number = browserZoomDefaultIndex; readonly debugger: BrowserViewDebugger; - readonly inspector: BrowserViewElementInspector; + readonly inspector: BrowserViewInspector; private _ownerWindow: ICodeWindow; private _currentWindow: ICodeWindow | IAuxiliaryWindow | undefined; @@ -191,7 +191,7 @@ export class BrowserView extends Disposable { }); this.debugger = new BrowserViewDebugger(this, this.logService); - this.inspector = this._register(new BrowserViewElementInspector(this)); + this.inspector = this._register(new BrowserViewInspector(this)); this.setupEventListeners(); } diff --git a/src/vs/platform/browserView/electron-main/browserViewElementInspector.ts b/src/vs/platform/browserView/electron-main/browserViewFrameInspector.ts similarity index 50% rename from src/vs/platform/browserView/electron-main/browserViewElementInspector.ts rename to src/vs/platform/browserView/electron-main/browserViewFrameInspector.ts index 936547fb462cf7..5742d36c540c38 100644 --- a/src/vs/platform/browserView/electron-main/browserViewElementInspector.ts +++ b/src/vs/platform/browserView/electron-main/browserViewFrameInspector.ts @@ -5,10 +5,15 @@ import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable, DisposableStore, IDisposable, MutableDisposable } from '../../../base/common/lifecycle.js'; -import { browserViewIsolatedWorldId, IElementData, IElementAncestor, IBrowserViewTheme } from '../common/browserView.js'; +import { IElementData, IElementAncestor, IBrowserViewTheme } from '../common/browserView.js'; import { collapseToShorthands, formatMatchedStyles, keyComputedProperties, type IMatchedStyles } from '../common/cssHelpers.js'; import { ICDPConnection } from '../common/cdp/types.js'; -import type { BrowserView } from './browserView.js'; + +export interface IFrameElementHandle extends IDisposable { + addToChat(): Promise; + highlight(): Promise; + hideHighlight(): Promise; +} type Quad = [number, number, number, number, number, number, number, number]; @@ -37,27 +42,46 @@ interface ILayoutMetricsResult { }; } -interface IActiveSelection extends IDisposable { - readonly isCDP: boolean; -} - -export interface IElementHandle extends IDisposable { - addToChat(): Promise; - highlight(): Promise; - hideHighlight(): Promise; -} - -/** - * Well-known ids understood by `__vscode_helpers.getElement(id)` in - * `preload-browserView.ts`. Any other string is treated as the id of a - * dynamically tracked element. - */ -export const enum BrowserViewInspectElementId { - /** The page's `document.activeElement`. */ - Active = 'active', - /** The element targeted by the most recent `contextmenu` event. */ - ContextMenuTarget = 'context-menu-target', -} +/** Slightly customised CDP debugger inspect highlight colours. */ +export const inspectHighlightConfig = { + showInfo: true, + showRulers: false, + showStyles: true, + showAccessibilityInfo: true, + showExtensionLines: false, + contrastAlgorithm: 'aa', + contentColor: { r: 173, g: 216, b: 255, a: 0.8 }, + paddingColor: { r: 150, g: 200, b: 255, a: 0.5 }, + borderColor: { r: 120, g: 180, b: 255, a: 0.7 }, + marginColor: { r: 200, g: 220, b: 255, a: 0.4 }, + eventTargetColor: { r: 130, g: 160, b: 255, a: 0.8 }, + shapeColor: { r: 130, g: 160, b: 255, a: 0.8 }, + shapeMarginColor: { r: 130, g: 160, b: 255, a: 0.5 }, + gridHighlightConfig: { + rowGapColor: { r: 140, g: 190, b: 255, a: 0.3 }, + rowHatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, + columnGapColor: { r: 140, g: 190, b: 255, a: 0.3 }, + columnHatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, + rowLineColor: { r: 120, g: 180, b: 255 }, + columnLineColor: { r: 120, g: 180, b: 255 }, + rowLineDash: true, + columnLineDash: true + }, + flexContainerHighlightConfig: { + containerBorder: { color: { r: 120, g: 180, b: 255 }, pattern: 'solid' }, + itemSeparator: { color: { r: 140, g: 190, b: 255 }, pattern: 'solid' }, + lineSeparator: { color: { r: 140, g: 190, b: 255 }, pattern: 'solid' }, + mainDistributedSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, + crossDistributedSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, + rowGapSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, + columnGapSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, + }, + flexItemHighlightConfig: { + baseSizeBox: { hatchColor: { r: 130, g: 170, b: 255, a: 0.6 } }, + baseSizeBorder: { color: { r: 120, g: 180, b: 255 }, pattern: 'solid' }, + flexibilityArrow: { color: { r: 130, g: 190, b: 255 } } + }, +}; function useScopedDisposal() { const store = new DisposableStore() as DisposableStore & { [Symbol.dispose](): void }; @@ -66,284 +90,218 @@ function useScopedDisposal() { } /** - * Manages element inspection on a browser view. + * Per-frame element inspector backed by a dedicated CDP session. + * + * Owns the full lifecycle of element inspection for a single frame: + * CDP domain initialization, element picking (overlay + CDP modes), + * node data extraction, and highlight management. * - * Attaches a persistent CDP session in the constructor; methods wait for - * it to be ready before issuing commands. + * Fires {@link onDidInspectElement} when an element is selected via + * CDP inspect mode (debugger paused). */ -export class BrowserViewElementInspector extends Disposable { +export class BrowserViewFrameInspector extends Disposable { - private readonly _connectionPromise: Promise; + private _isDisposed = false; + private readonly _onWillDispose = this._register(new Emitter()); + readonly onWillDispose: Event = this._onWillDispose.event; - private readonly _onDidSelectElement = this._register(new Emitter()); - readonly onDidSelectElement: Event = this._onDidSelectElement.event; + private readonly _onDidInspectElement = this._register(new Emitter()); + readonly onDidInspectElement: Event = this._onDidInspectElement.event; - private readonly _onDidChangeElementSelectionActive = this._register(new Emitter()); - readonly onDidChangeElementSelectionActive: Event = this._onDidChangeElementSelectionActive.event; + private readonly _onDidStopPicking = this._register(new Emitter()); + readonly onDidStopPicking: Event = this._onDidStopPicking.event; - private _elementSelectionActive = false; - get isElementSelectionActive(): boolean { return this._elementSelectionActive; } + private _isPaused = false; + private readonly _activeInspection = this._register(new MutableDisposable()); - private readonly _activeSelection = this._register(new MutableDisposable()); - private _theme: IBrowserViewTheme = {}; + /** Whether this frame's JavaScript execution is currently paused by the debugger. */ + get isPaused(): boolean { return this._isPaused; } - constructor(private readonly browser: BrowserView) { - super(); - this._connectionPromise = this._createConnection(); - this._registerListeners().catch(() => { }); - } + /** Whether element inspection is currently active on this frame. */ + get isInspecting(): boolean { return !!this._activeInspection.value; } - private async _createConnection(): Promise { - const conn = await this.browser.debugger.attach(); + /** The CDP frame ID for this frame. */ + get frameId(): string { return this._frameId; } - try { - // Initialize CDP domains up-front rather than during selection: - // some (e.g. CSS.enable) hang if sent while the debugger is paused, - // but succeed when enabled before any pause. - await conn.sendCommand('DOM.enable'); - await conn.sendCommand('Overlay.enable'); - await conn.sendCommand('CSS.enable'); - await conn.sendCommand('Runtime.enable'); - } catch (error) { - conn.dispose(); - throw error; - } + /** + * @param connection The CDP session that owns this frame's target. + * @param frame The Electron WebFrameMain for this frame. + * @param _uniqueContextId The unique execution context ID for Runtime calls in this frame. + * @param _frameId The CDP frame ID for this frame. + */ + constructor( + readonly connection: ICDPConnection, + readonly frame: Electron.WebFrameMain, + private readonly _uniqueContextId: string, + private readonly _frameId: string, + ) { + super(); - if (this._store.isDisposed) { - conn.dispose(); - throw new Error('Inspector disposed before connection was ready'); - } - this._register(conn); + this._register(connection.onClose(() => { + this.dispose(); + })); - return conn; - } + this._register(connection.onEvent(async event => { + switch (event.method) { + case 'Overlay.inspectNodeRequested': { + const params = event.params as { backendNodeId: number }; + if (params?.backendNodeId) { + try { + // Verify the node belongs to this frame (important when + // sharing a session with same-origin siblings). + const { node } = await this.connection.sendCommand('DOM.describeNode', { + backendNodeId: params.backendNodeId, + }) as { node: { frameId?: string } }; + if (node.frameId && node.frameId !== this._frameId) { + break; + } + const nodeData = await this.extractNodeData({ backendNodeId: params.backendNodeId }); + this._onDidInspectElement.fire(nodeData); + } catch { + // Best effort. + } + } + break; + } + case 'Debugger.paused': + this._isPaused = true; + break; + case 'Debugger.resumed': + this._isPaused = false; + break; + } + })); - private async _registerListeners(): Promise { - const webContents = this.browser.webContents; - const onPicked = async (_event: unknown, pickId: string) => { - if (!pickId) { + // Listen for element-picked IPC from this frame's preload + const onPicked = async (event: Electron.IpcMainEvent, pickId: string) => { + if (!pickId || event.senderFrame !== this.frame) { return; } - - this._activeSelection.clear(); - try { - const handle = await this.getElementHandle(pickId); - await handle?.addToChat(); + const nodeData = await this.extractNodeDataById(pickId); + this._onDidInspectElement.fire(nodeData); } catch { // Best effort; user can re-pick. } }; - webContents.ipc.on('vscode:browserView:elementPicked', onPicked); - this._register({ - dispose: () => webContents.ipc.removeListener('vscode:browserView:elementPicked', onPicked) - }); - const onPickStopped = () => { - if (this._activeSelection.value) { - this._elementSelectionActive = false; - this._onDidChangeElementSelectionActive.fire(false); - this._activeSelection.clearAndLeak(); - } - }; - webContents.ipc.on('vscode:browserView:elementPickStopped', onPickStopped); - this._register({ - dispose: () => webContents.ipc.removeListener('vscode:browserView:elementPickStopped', onPickStopped) - }); - - // Navigation to a new document destroys the preload's page-side overlay - // and resets the CDP inspect mode. Clear the active selection so the - // workbench reflects the actual state. - const onNavigated = () => this._activeSelection.clear(); - webContents.on('did-navigate', onNavigated); - this._register({ - dispose: () => webContents.removeListener('did-navigate', onNavigated) - }); - - const connection = await this._connectionPromise; - this._register(connection.onEvent(async (event) => { - if (event.method !== 'Overlay.inspectNodeRequested') { - return; - } - - if (!this._activeSelection.value?.isCDP) { - return; - } + frame.ipc.on('vscode:browserView:elementPicked', onPicked); + this._register({ dispose: () => frame.ipc.removeListener('vscode:browserView:elementPicked', onPicked) }); - const params = event.params as { backendNodeId: number }; - if (!params?.backendNodeId) { + // Listen for pick-stopped IPC from this frame's preload + const onPickStopped = (event: Electron.IpcMainEvent) => { + if (event.senderFrame !== this.frame) { return; } + this._onDidStopPicking.fire(); + }; + frame.ipc.on('vscode:browserView:elementPickStopped', onPickStopped); + this._register({ dispose: () => frame.ipc.removeListener('vscode:browserView:elementPickStopped', onPickStopped) }); - this._activeSelection.clear(); + this._enableDomains().catch(() => { }); + } - try { - const nodeData = await extractNodeData(connection, { backendNodeId: params.backendNodeId }); - this._onDidSelectElement.fire({ - ...nodeData, - url: this.browser.getURL() - }); - } catch (err) { - // Best effort; ignore errors and let the user try again if they want. - } - })); + private async _enableDomains(): Promise { + await this.connection.sendCommand('DOM.enable'); + await this.connection.sendCommand('Overlay.enable'); + await this.connection.sendCommand('CSS.enable'); + await this.connection.sendCommand('Runtime.enable'); + await this.connection.sendCommand('Page.enable'); + } - webContents.on('ipc-message', async (event, channel) => { - if (channel === 'vscode:browserView:preloadReady' && event.senderFrame === webContents.mainFrame) { - this.setTheme(this._theme); - } - }); - this._register({ - dispose: () => webContents.removeAllListeners('ipc-message') - }); + override dispose() { + if (this._isDisposed) { + return; + } + this._isDisposed = true; + this._onWillDispose.fire(); + super.dispose(); } + /** + * Send the theme to this frame's preload. + */ setTheme(theme: IBrowserViewTheme): void { - this._theme = theme; - const webContents = this.browser.webContents; - const themeJson = JSON.stringify(theme); - webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: `window.browserViewAPI?.setTheme?.(${themeJson})` } - ]).catch(() => { /* best effort — page may not be loaded yet */ }); + this.frame.postMessage('vscode:browserView:setTheme', theme); } /** - * Toggle element selection mode on the browser view. - * - * When enabled, mounts a page-side overlay (see `preload-browserView.ts`) that - * lets the user click an element or drag a region (region → deepest common - * ancestor). When the debugger is paused, falls back to Chromium's built-in - * `Overlay.setInspectMode`. - * - * Each pick fires {@link onDidSelectElement}; state changes are delivered via - * {@link onDidChangeElementSelectionActive}. - * - * @param enabled Whether to enable or disable. Omit to toggle. + * Start element inspection on this frame. + * Uses CDP inspect mode if paused, otherwise the preload picker. + * Stores a disposable so stop always tears down the correct mode. */ - async toggleElementSelection(enabled?: boolean): Promise { - const newEnabled = enabled ?? !this._elementSelectionActive; - if (newEnabled === this._elementSelectionActive) { - return; - } - - if (!newEnabled) { - this._activeSelection.clear(); - return; - } - - const useCDP = this.browser.debugger.isPaused; - const start = useCDP ? async () => { - const connection = await this._connectionPromise; - await connection.sendCommand('Overlay.setInspectMode', { + async startInspection(): Promise { + if (this._isPaused) { + await this.connection.sendCommand('Overlay.setInspectMode', { mode: 'searchForNode', highlightConfig: inspectHighlightConfig, }); - } : async () => { - const webContents = this.browser.webContents; - const started = await webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: `window.browserViewAPI?.pickElement?.start?.() ?? false` } - ]); - if (!started) { - throw new Error('Preload element picker not available'); - } - }; - const stop = useCDP ? async () => { - const connection = await this._connectionPromise; - await connection.sendCommand('Overlay.setInspectMode', { - mode: 'none', - highlightConfig: { showInfo: false, showStyles: false } - }); - await connection.sendCommand('Overlay.hideHighlight'); - } : async () => { - const webContents = this.browser.webContents; - await webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: 'window.browserViewAPI?.pickElement?.stop?.()' } - ]); - }; - - const selection: IActiveSelection = { - isCDP: useCDP, - dispose: () => { - if (this._activeSelection.value === selection) { - this._elementSelectionActive = false; - this._onDidChangeElementSelectionActive.fire(false); - this._activeSelection.clearAndLeak(); - void stop().catch(() => { /* best-effort cleanup */ }); + this._activeInspection.value = { + dispose: async () => { + if (this.frame.isDestroyed()) { + return; + } + try { + await this.connection.sendCommand('Overlay.setInspectMode', { + mode: 'none', + highlightConfig: { showInfo: false, showStyles: false } + }); + await this.connection.sendCommand('Overlay.hideHighlight'); + } catch { + // Best effort. + } } - } - }; - this._activeSelection.value = selection; - - try { - await start(); - if (this._activeSelection.value === selection) { - this._elementSelectionActive = true; - this._onDidChangeElementSelectionActive.fire(true); - } - } catch { - this._activeSelection.clear(); + }; + } else { + this.frame.postMessage('vscode:browserView:startElementPicker', {}); + this._activeInspection.value = { + dispose: () => { + if (this.frame.isDestroyed()) { + return; + } + this.frame.postMessage('vscode:browserView:stopElementPicker', {}); + } + }; } } /** - * Resolve a handle to the element identified by `id`. - * - * `id` is interpreted by `__vscode_helpers.getElement(id)` in the page - * preload (see {@link BrowserViewSelectedElementId} for well-known values). - * Returns `undefined` if no element is found. + * Stop element inspection on this frame. */ - async getElementHandle(id: string): Promise { - const connection = await this._connectionPromise; + async stopInspection(): Promise { + this._activeInspection.clear(); + } - const { result } = await connection.sendCommand('Runtime.evaluate', { - expression: `window.__vscode_helpers?.getElement(${JSON.stringify(id)})`, + /** + * Resolve an element by its preload-tracked id and extract full node data. + */ + async extractNodeDataById(elementId: string): Promise { + const { result } = await this.connection.sendCommand('Runtime.evaluate', { + expression: `window.__vscode_helpers?.getElement(${JSON.stringify(elementId)})`, returnByValue: false, + uniqueContextId: this._uniqueContextId, }) as { result: { objectId?: string } }; if (!result?.objectId) { - return undefined; + throw new Error(`Element not found: ${elementId}`); } - const objectId = result.objectId; - const elementId = id; - let disposed = false; + return this.extractNodeData({ objectId: result.objectId }); + } - return { - addToChat: async () => { - const nodeData = await extractNodeData(connection, { objectId }); - this._onDidSelectElement.fire({ - ...nodeData, - url: this.browser.getURL() - }); - }, - highlight: async () => { - const webContents = this.browser.webContents; - await webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: `window.browserViewAPI?.highlightElement?.(${JSON.stringify(elementId)})` } - ]); - }, - hideHighlight: async () => { - const webContents = this.browser.webContents; - await webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: 'window.browserViewAPI?.hideHighlight?.()' } - ]); - }, - dispose: () => { - if (disposed) { - return; - } - disposed = true; - const webContents = this.browser.webContents; - void webContents.executeJavaScriptInIsolatedWorld(browserViewIsolatedWorldId, [ - { code: 'window.browserViewAPI?.hideHighlight?.()' } - ]).catch(() => { /* best effort */ }); - } - }; + /** + * Extract full element data from a CDP node reference. + */ + async extractNodeData(id: { backendNodeId?: number; objectId?: string }): Promise { + const data = await extractNodeData(this.connection, id); + return { ...data, url: this.frame.url }; } + /** + * Get the visual viewport scale for this frame. + */ async getVisualViewportScale(): Promise { try { - const connection = await this._connectionPromise; - const result = await connection.sendCommand('Page.getLayoutMetrics') as ILayoutMetricsResult; + const result = await this.connection.sendCommand('Page.getLayoutMetrics') as ILayoutMetricsResult; if (typeof result.cssVisualViewport?.scale === 'number') { const scale = Number(result.cssVisualViewport.scale); if (Number.isFinite(scale) && scale > 0) { @@ -353,12 +311,37 @@ export class BrowserViewElementInspector extends Disposable { } catch { // Ignore execution errors while loading and use defaults. } - return 1; } + + /** + * Create a handle to an element tracked by the preload script. + */ + getElementHandle(elementId: string): IFrameElementHandle { + let disposed = false; + return { + addToChat: async () => { + const nodeData = await this.extractNodeDataById(elementId); + this._onDidInspectElement.fire(nodeData); + }, + highlight: async () => { + this.frame.postMessage('vscode:browserView:highlightElement', { elementId }); + }, + hideHighlight: async () => { + this.frame.postMessage('vscode:browserView:hideHighlight', {}); + }, + dispose: () => { + if (disposed) { + return; + } + disposed = true; + this.frame.postMessage('vscode:browserView:hideHighlight', {}); + } + }; + } } -async function extractNodeData(connection: ICDPConnection, id: { backendNodeId?: number; objectId?: string }): Promise { +export async function extractNodeData(connection: ICDPConnection, id: { backendNodeId?: number; objectId?: string }): Promise { using store = useScopedDisposal(); const discoveredNodesByNodeId: Record = {}; @@ -504,44 +487,3 @@ function attributeArrayToRecord(attributes: string[]): Record { } return record; } - -/** Slightly customised CDP debugger inspect highlight colours. */ -const inspectHighlightConfig = { - showInfo: true, - showRulers: false, - showStyles: true, - showAccessibilityInfo: true, - showExtensionLines: false, - contrastAlgorithm: 'aa', - contentColor: { r: 173, g: 216, b: 255, a: 0.8 }, - paddingColor: { r: 150, g: 200, b: 255, a: 0.5 }, - borderColor: { r: 120, g: 180, b: 255, a: 0.7 }, - marginColor: { r: 200, g: 220, b: 255, a: 0.4 }, - eventTargetColor: { r: 130, g: 160, b: 255, a: 0.8 }, - shapeColor: { r: 130, g: 160, b: 255, a: 0.8 }, - shapeMarginColor: { r: 130, g: 160, b: 255, a: 0.5 }, - gridHighlightConfig: { - rowGapColor: { r: 140, g: 190, b: 255, a: 0.3 }, - rowHatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, - columnGapColor: { r: 140, g: 190, b: 255, a: 0.3 }, - columnHatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, - rowLineColor: { r: 120, g: 180, b: 255 }, - columnLineColor: { r: 120, g: 180, b: 255 }, - rowLineDash: true, - columnLineDash: true - }, - flexContainerHighlightConfig: { - containerBorder: { color: { r: 120, g: 180, b: 255 }, pattern: 'solid' }, - itemSeparator: { color: { r: 140, g: 190, b: 255 }, pattern: 'solid' }, - lineSeparator: { color: { r: 140, g: 190, b: 255 }, pattern: 'solid' }, - mainDistributedSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, - crossDistributedSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, - rowGapSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, - columnGapSpace: { hatchColor: { r: 140, g: 190, b: 255, a: 0.7 }, fillColor: { r: 140, g: 190, b: 255, a: 0.4 } }, - }, - flexItemHighlightConfig: { - baseSizeBox: { hatchColor: { r: 130, g: 170, b: 255, a: 0.6 } }, - baseSizeBorder: { color: { r: 120, g: 180, b: 255 }, pattern: 'solid' }, - flexibilityArrow: { color: { r: 130, g: 190, b: 255 } } - }, -}; diff --git a/src/vs/platform/browserView/electron-main/browserViewInspector.ts b/src/vs/platform/browserView/electron-main/browserViewInspector.ts new file mode 100644 index 00000000000000..21eeb3c7c75acb --- /dev/null +++ b/src/vs/platform/browserView/electron-main/browserViewInspector.ts @@ -0,0 +1,475 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Emitter, Event } from '../../../base/common/event.js'; +import { Disposable, IDisposable, MutableDisposable } from '../../../base/common/lifecycle.js'; +import { IElementData, IBrowserViewTheme } from '../common/browserView.js'; +import { ICDPConnection } from '../common/cdp/types.js'; +import type { BrowserView } from './browserView.js'; +import { BrowserViewFrameInspector } from './browserViewFrameInspector.js'; + +interface IActiveSelection extends IDisposable { +} + +export interface IElementHandle extends IDisposable { + addToChat(): Promise; + highlight(): Promise; + hideHighlight(): Promise; +} + +/** + * Well-known ids understood by `__vscode_helpers.getElement(id)` in + * `preload-browserView.ts`. Any other string is treated as the id of a + * dynamically tracked element. + */ +export const enum BrowserViewInspectElementId { + /** The page's `document.activeElement`. */ + Active = 'active', + /** The element targeted by the most recent `contextmenu` event. */ + ContextMenuTarget = 'context-menu-target', +} + +/** + * Manages element inspection across all frames in a browser view. + * + * Creates a {@link BrowserViewFrameInspector} for the main frame and + * automatically discovers iframe CDP targets via auto-attach, matching + * them to their corresponding `WebFrameMain` instances using an opaque + * token generated by the preload script. + * + * This class is a thin orchestrator — all per-frame CDP logic (domain + * initialization, element extraction, CDP inspect mode) lives in + * {@link BrowserViewFrameInspector}. + */ +export class BrowserViewInspector extends Disposable { + + private readonly _onDidSelectElement = this._register(new Emitter()); + readonly onDidSelectElement: Event = this._onDidSelectElement.event; + + private readonly _onDidChangeElementSelectionActive = this._register(new Emitter()); + readonly onDidChangeElementSelectionActive: Event = this._onDidChangeElementSelectionActive.event; + + private _elementSelectionActive = false; + get isElementSelectionActive(): boolean { return this._elementSelectionActive; } + + private readonly _activeSelection = this._register(new MutableDisposable()); + private _theme: IBrowserViewTheme = {}; + + private readonly _registry = this._register(new FrameInspectorRegistry()); + + constructor(private readonly browser: BrowserView) { + super(); + + const webContents = this.browser.webContents; + + // Wire up inspector adoption from the registry + this._register(this._registry.onDidAdopt(inspector => this._onInspectorAdopted(inspector))); + + // Navigation destroys preload overlays and CDP state + const onNavigated = () => { + this._activeSelection.clear(); + }; + webContents.on('did-navigate', onNavigated); + this._register({ dispose: () => webContents.removeListener('did-navigate', onNavigated) }); + + // Preload ready — the key correlation point between WebFrameMain and CDP target + const onIpcMessage = (_event: Electron.Event, channel: string, ...args: unknown[]) => { + if (channel !== 'vscode:browserView:preloadReady') { + return; + } + const senderFrame = (_event as { senderFrame?: Electron.WebFrameMain }).senderFrame; + if (!senderFrame) { + return; + } + const frameToken = args[0] as string; + if (!frameToken) { + return; + } + + // Apply theme immediately regardless of inspector state + senderFrame.postMessage('vscode:browserView:setTheme', this._theme); + + this._registry.notifyFrameReady(senderFrame, frameToken); + }; + webContents.on('ipc-message', onIpcMessage); + this._register({ dispose: () => webContents.removeListener('ipc-message', onIpcMessage) }); + + // Cross-origin (OOPIF) targets get their own session — watch it for contexts + this._register(this.browser.debugger.onTargetDiscovered(async ({ targetId, type }) => { + if (type === 'iframe') { + try { + const session = await this.browser.debugger.attachToTarget(targetId); + this._watchSession(session); + } catch { + return; + } + } + })); + + // Attach the main debugger session and watch it for contexts + this.browser.debugger.attach().then(conn => this._watchSession(conn)).catch(() => { }); + } + + /** + * Watch a CDP session for execution contexts. When a default context appears, + * probes for the preload token and correlates with the pending WebFrameMain. + * + * Called for every session: the main page session (sees same-origin frames) + * and each cross-origin target session (sees only its own frame). + */ + private _watchSession(session: ICDPConnection): void { + this._register(session.onEvent(async event => { + if (event.method === 'Runtime.executionContextCreated') { + const context = (event.params as { + context: { + uniqueId: string; + auxData?: { isDefault?: boolean; frameId?: string }; + }; + }).context; + + if (!context?.auxData?.isDefault || !context.auxData.frameId) { + return; + } + + const frameId = context.auxData.frameId; + const uniqueContextId = context.uniqueId; + + // Probe for the preload token in this context + try { + const { result } = await session.sendCommand('Runtime.evaluate', { + expression: 'window.__vscode_helpers?.getFrameToken?.()', + returnByValue: true, + uniqueContextId, + }) as { result: { value?: string } }; + + const token = result.value; + if (!token) { + return; + } + + this._registry.notifyContextDiscovered(session, uniqueContextId, frameId, token); + } catch { + // Context may have been destroyed by now — ignore. + } + } else if (event.method === 'Page.frameDetached') { + const frameId = (event.params as { frameId?: string })?.frameId; + if (frameId) { + this._registry.disposeByFrameId(frameId); + } + } else if (event.method === 'Runtime.executionContextsCleared') { + // Navigation cleared all contexts — dispose inspectors owned by this session + this._registry.disposeBySession(session); + } + })); + + Event.once(session.onClose)(() => { + this._registry.disposeBySession(session); + }); + + // Enable Runtime + Page to start receiving context and frame events + session.sendCommand('Runtime.enable').catch(() => { }); + session.sendCommand('Page.enable').catch(() => { }); + } + + /** + * Called by the registry when a frame inspector is fully adopted. + * Wires its events to this orchestrator. + */ + private _onInspectorAdopted(inspector: BrowserViewFrameInspector): void { + inspector.onDidInspectElement(async nodeData => { + this._activeSelection.clear(); + try { + const offset = await this._getFrameOffsetInPage(inspector.frame); + nodeData = this._offsetElementData(nodeData, offset); + } catch { + // Best effort. + } + this._onDidSelectElement.fire(nodeData); + }); + + // When a frame's preload stops picking, stop all other frames too + inspector.onDidStopPicking(() => { + this._activeSelection.clear(); + }); + + // If element selection is currently active, start it on the new frame + if (this._activeSelection.value) { + inspector.startInspection().catch(() => { }); + } + + inspector.setTheme(this._theme); + } + + setTheme(theme: IBrowserViewTheme): void { + this._theme = theme; + // Broadcast to all known inspectors + for (const inspector of this._registry.inspectors) { + inspector.setTheme(theme); + } + } + + /** + * Toggle element selection mode across all frames. + */ + async toggleElementSelection(enabled?: boolean): Promise { + const newEnabled = enabled ?? !this._elementSelectionActive; + if (newEnabled === this._elementSelectionActive) { + return; + } + + if (!newEnabled) { + this._activeSelection.clear(); + return; + } + + const start = () => Promise.all([...this._registry.inspectors].map(i => i.startInspection())); + const stop = () => Promise.all([...this._registry.inspectors].map(i => i.stopInspection())); + + const selection: IActiveSelection = { + dispose: () => { + if (this._activeSelection.value === selection) { + this._elementSelectionActive = false; + this._onDidChangeElementSelectionActive.fire(false); + this._activeSelection.clearAndLeak(); + void stop().catch(() => { }); + } + } + }; + this._activeSelection.value = selection; + + try { + await start(); + if (this._activeSelection.value === selection) { + this._elementSelectionActive = true; + this._onDidChangeElementSelectionActive.fire(true); + } + } catch { + this._activeSelection.clear(); + } + } + + /** + * Resolve a handle to an element. Routes to the correct frame inspector. + */ + getElementHandle(id: string, frame: Electron.WebFrameMain): IElementHandle | undefined { + return this._registry.getByFrame(frame)?.getElementHandle(id); + } + + async getVisualViewportScale(frame: Electron.WebFrameMain = this.browser.webContents.mainFrame): Promise { + return this._registry.getByFrame(frame)?.getVisualViewportScale() ?? 1; + } + + /** + * Compute the cumulative offset of a frame relative to the top-level page. + * Walks up the frame hierarchy using the parent's CDP session to query the + * iframe element's box model via `DOM.getFrameOwner` + `DOM.getBoxModel`. + * Works for both same-origin and cross-origin frames. + */ + private async _getFrameOffsetInPage(frame: Electron.WebFrameMain): Promise<{ x: number; y: number }> { + const mainFrame = this.browser.webContents.mainFrame; + let x = 0; + let y = 0; + let current = frame; + + while (current !== mainFrame) { + const parent = current.parent; + if (!parent) { + break; + } + + const childInspector = this._registry.getByFrame(current); + const parentInspector = this._registry.getByFrame(parent); + if (!childInspector || !parentInspector) { + break; + } + + try { + const childFrameId = childInspector.frameId; + + // Ask the parent session for the iframe element that owns this frame + const frameOwner = await parentInspector.connection.sendCommand('DOM.getFrameOwner', { + frameId: childFrameId, + }) as { backendNodeId: number }; + + // Get the iframe element's box model in the parent's coordinate space + const boxModel = await parentInspector.connection.sendCommand('DOM.getBoxModel', { + backendNodeId: frameOwner.backendNodeId, + }) as { model: { content: number[] } }; + + // content quad: [x1,y1, x2,y2, x3,y3, x4,y4] — top-left is first pair + const content = boxModel.model.content; + x += content[0]; + y += content[1]; + } catch { + break; + } + + current = parent; + } + + return { x, y }; + } + + /** + * Offset element data bounds by a frame offset. + */ + private _offsetElementData(data: IElementData, offset: { x: number; y: number }): IElementData { + if (offset.x === 0 && offset.y === 0) { + return data; + } + return { + ...data, + bounds: { + x: data.bounds.x + offset.x, + y: data.bounds.y + offset.y, + width: data.bounds.width, + height: data.bounds.height, + } + }; + } +} + + +interface IPendingContext { + readonly session: ICDPConnection; + readonly uniqueContextId: string; + readonly frameId: string; +} + +/** + * Tracks the two-sided correlation between preload tokens (from WebFrameMain IPC) + * and CDP execution contexts, and indexes adopted inspectors for O(1) lookup by + * frame, frameId, or owning session. + */ +class FrameInspectorRegistry extends Disposable { + + private readonly _onDidAdopt = this._register(new Emitter()); + readonly onDidAdopt: Event = this._onDidAdopt.event; + + /** Pending halves waiting for their counterpart. */ + private readonly _pendingFrames = new Map(); + private readonly _pendingSessions = new Map(); + + /** Adopted inspectors indexed multiple ways. */ + private readonly _all = new Set(); + private readonly _byFrame = new WeakMap(); + private readonly _byFrameId = new Map(); + private readonly _bySession = new Map>(); + + get inspectors(): Iterable { return this._all; } + + getByFrame(frame: Electron.WebFrameMain): BrowserViewFrameInspector | undefined { + return this._byFrame.get(frame); + } + + /** + * Called when a preload script signals readiness with a token. + * If a matching CDP context was already discovered, adopts immediately. + */ + notifyFrameReady(frame: Electron.WebFrameMain, token: string): void { + const pending = this._pendingSessions.get(token); + if (pending) { + this._pendingSessions.delete(token); + this._adopt(pending.session, pending.uniqueContextId, pending.frameId, frame); + } else { + this._pendingFrames.set(token, frame); + } + } + + /** + * Called when a CDP execution context is discovered and its preload token probed. + * If a matching WebFrameMain was already registered, adopts immediately. + */ + notifyContextDiscovered(session: ICDPConnection, uniqueContextId: string, frameId: string, token: string): void { + const frame = this._pendingFrames.get(token); + if (frame) { + this._pendingFrames.delete(token); + this._adopt(session, uniqueContextId, frameId, frame); + } else { + this._pendingSessions.set(token, { session, uniqueContextId, frameId }); + } + } + + /** Dispose the inspector owning the given CDP frameId, if any. Also cleans pending entries. */ + disposeByFrameId(frameId: string): void { + this._byFrameId.get(frameId)?.dispose(); + // Remove pending session entries whose frameId matches the detached frame + for (const [token, pending] of this._pendingSessions) { + if (pending.frameId === frameId) { + this._pendingSessions.delete(token); + } + } + // Remove any pending frame entries whose frame is now detached/destroyed + for (const [token, frame] of this._pendingFrames) { + if (frame.detached || frame.isDestroyed()) { + this._pendingFrames.delete(token); + } + } + } + + /** Dispose all inspectors whose connection is the given session and clear related pending state. */ + disposeBySession(session: ICDPConnection): void { + const set = this._bySession.get(session); + if (set) { + for (const inspector of [...set]) { + inspector.dispose(); + } + } + for (const [token, pending] of this._pendingSessions) { + if (pending.session === session) { + this._pendingSessions.delete(token); + } + } + } + + private _adopt( + session: ICDPConnection, + uniqueContextId: string, + frameId: string, + frame: Electron.WebFrameMain, + ): void { + // Guard: frame may have been destroyed between IPC and context match + if (frame.detached || frame.isDestroyed()) { + return; + } + + const inspector = new BrowserViewFrameInspector(session, frame, uniqueContextId, frameId); + + this._all.add(inspector); + this._byFrame.set(frame, inspector); + this._byFrameId.set(frameId, inspector); + + let sessionSet = this._bySession.get(session); + if (!sessionSet) { + sessionSet = new Set(); + this._bySession.set(session, sessionSet); + } + sessionSet.add(inspector); + + inspector.onWillDispose(() => { + this._all.delete(inspector); + this._byFrame.delete(frame); + this._byFrameId.delete(frameId); + const s = this._bySession.get(session); + if (s) { + s.delete(inspector); + if (s.size === 0) { + this._bySession.delete(session); + } + } + }); + + this._onDidAdopt.fire(inspector); + } + + override dispose(): void { + for (const inspector of [...this._all]) { + inspector.dispose(); + } + this._pendingFrames.clear(); + this._pendingSessions.clear(); + super.dispose(); + } +} diff --git a/src/vs/platform/browserView/electron-main/browserViewMainService.ts b/src/vs/platform/browserView/electron-main/browserViewMainService.ts index 62fdb0cb098e84..967053905491e6 100644 --- a/src/vs/platform/browserView/electron-main/browserViewMainService.ts +++ b/src/vs/platform/browserView/electron-main/browserViewMainService.ts @@ -20,7 +20,7 @@ import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { localize } from '../../../nls.js'; import { INativeHostMainService } from '../../native/electron-main/nativeHostMainService.js'; import { htmlAttributeEncodeValue } from '../../../base/common/strings.js'; -import { BrowserViewInspectElementId } from './browserViewElementInspector.js'; +import { BrowserViewInspectElementId } from './browserViewInspector.js'; export const IBrowserViewMainService = createDecorator('browserViewMainService'); @@ -400,7 +400,7 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa const inspectTarget = this._configuration.aiFeaturesDisabled ? undefined - : await view.inspector.getElementHandle(BrowserViewInspectElementId.ContextMenuTarget); + : params.frame && await view.inspector.getElementHandle(BrowserViewInspectElementId.ContextMenuTarget, params.frame); const menu = new Menu(); if (params.linkURL) { From 907ffc426d3e9d33e28fdf79e81fec65fb1c57a3 Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Thu, 21 May 2026 17:53:35 -0400 Subject: [PATCH 19/43] Fix entitlements rendering incorrectly (#317857) --- .../workbench/services/chat/common/chatEntitlementService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/services/chat/common/chatEntitlementService.ts b/src/vs/workbench/services/chat/common/chatEntitlementService.ts index 1fb197cefc1e81..f9110d37144df7 100644 --- a/src/vs/workbench/services/chat/common/chatEntitlementService.ts +++ b/src/vs/workbench/services/chat/common/chatEntitlementService.ts @@ -814,8 +814,8 @@ export function parseQuotas(entitlementsData: IEntitlementsData): IQuotas { hasQuota: rawQuotaSnapshot.has_quota, usageBasedBilling: entitlementsData.token_based_billing, resetAt: rawQuotaSnapshot.quota_reset_at || undefined, - entitlement: parsedEntitlement !== undefined && Number.isSafeInteger(parsedEntitlement) && parsedEntitlement >= 0 ? parsedEntitlement : undefined, - quotaRemaining: parsedQuotaRemaining !== undefined && Number.isSafeInteger(parsedQuotaRemaining) && parsedQuotaRemaining >= 0 ? parsedQuotaRemaining : undefined, + entitlement: parsedEntitlement !== undefined && Number.isFinite(parsedEntitlement) && parsedEntitlement >= 0 ? parsedEntitlement : undefined, + quotaRemaining: parsedQuotaRemaining !== undefined && Number.isFinite(parsedQuotaRemaining) && parsedQuotaRemaining >= 0 ? parsedQuotaRemaining : undefined, }; switch (quotaType) { From 46050be407ed64db2763d0c47d028a4f83b31390 Mon Sep 17 00:00:00 2001 From: Kyle Cutler <67761731+kycutler@users.noreply.github.com> Date: Thu, 21 May 2026 14:57:50 -0700 Subject: [PATCH 20/43] Add browser emulation toolbar (#317831) --- src/vs/platform/actions/common/actions.ts | 1 + .../browserView/common/browserView.ts | 31 + .../browserView/electron-main/browserView.ts | 11 +- .../electron-main/browserViewEmulator.ts | 112 ++ .../electron-main/browserViewMainService.ts | 10 +- .../contrib/browserView/common/browserView.ts | 26 +- .../electron-browser/browserEditor.ts | 156 ++- .../browserView.contribution.ts | 1 + .../electron-browser/browserViewActions.ts | 5 +- .../browserEditorEmulationFeatures.ts | 1004 +++++++++++++++++ .../electron-browser/media/browser.css | 146 ++- 11 files changed, 1459 insertions(+), 44 deletions(-) create mode 100644 src/vs/platform/browserView/electron-main/browserViewEmulator.ts create mode 100644 src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorEmulationFeatures.ts diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index 2bea6c4668e8a0..a850b16a356411 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -303,6 +303,7 @@ export class MenuId { static readonly DiffEditorSelectionToolbar = new MenuId('DiffEditorSelectionToolbar'); static readonly BrowserNavigationToolbar = new MenuId('BrowserNavigationToolbar'); static readonly BrowserActionsToolbar = new MenuId('BrowserActionsToolbar'); + static readonly BrowserEmulationToolbar = new MenuId('BrowserEmulationToolbar'); static readonly AgentSessionsViewerFilterSubMenu = new MenuId('AgentSessionsViewerFilterSubMenu'); static readonly AgentSessionsContext = new MenuId('AgentSessionsContext'); static readonly AgentSessionSectionContext = new MenuId('AgentSessionSectionContext'); diff --git a/src/vs/platform/browserView/common/browserView.ts b/src/vs/platform/browserView/common/browserView.ts index cfff846b44301c..ce51cf96222599 100644 --- a/src/vs/platform/browserView/common/browserView.ts +++ b/src/vs/platform/browserView/common/browserView.ts @@ -85,6 +85,11 @@ export interface IBrowserViewBounds { height: number; zoomFactor: number; cornerRadius: number; + emulation?: { + viewportWidth: number; + viewportHeight: number; + scale: number; + }; } export interface IBrowserViewCaptureScreenshotOptions { @@ -156,6 +161,7 @@ export interface IBrowserViewState { storageScope: BrowserViewStorageScope; browserZoomIndex: number; isElementSelectionActive: boolean; + device: IBrowserDeviceProfile | undefined; } export interface IBrowserViewNavigationEvent { @@ -255,6 +261,27 @@ export function browserZoomAccessibilityLabel(zoomFactor: number): string { return localize('browserZoomAccessibilityLabel', "Page Zoom: {0}%", Math.round(zoomFactor * 100)); } +/** + * The "device" half of browser emulation: characteristics the page sees as + * intrinsic to the device (touch / mobile media features, DPR, UA string). + */ +export interface IBrowserDeviceProfile { + readonly mobile?: boolean; + readonly userAgent?: string; + readonly deviceScaleFactor?: number; +} + +/** + * The "screen" half of browser emulation: the desired viewport size and zoom. + * + * `undefined` values mean the view should be sized to fit the container. + */ +export interface IBrowserScreenProfile { + readonly width?: number; + readonly height?: number; + readonly scale?: number; +} + /** * This should match the isolated world ID defined in `preload-browserView.ts`. */ @@ -281,6 +308,7 @@ export interface IBrowserViewService { onDynamicDidClose(id: string): Event; onDynamicDidSelectElement(id: string): Event; onDynamicDidChangeElementSelectionActive(id: string): Event; + onDynamicDidChangeDeviceEmulation(id: string): Event; /** * Get all known browser views with their ownership and state information. @@ -431,6 +459,9 @@ export interface IBrowserViewService { /** Set the browser zoom index (independent from VS Code zoom). */ setBrowserZoomIndex(id: string, zoomIndex: number): Promise; + /** Set or clear the active device profile for a browser view. */ + setDeviceEmulation(id: string, device: IBrowserDeviceProfile | undefined): Promise; + /** * Trust a certificate for a given host in the browser view's session. * The page will be automatically reloaded after trusting. diff --git a/src/vs/platform/browserView/electron-main/browserView.ts b/src/vs/platform/browserView/electron-main/browserView.ts index 52b7f923b934ed..6ae74ed36edaca 100644 --- a/src/vs/platform/browserView/electron-main/browserView.ts +++ b/src/vs/platform/browserView/electron-main/browserView.ts @@ -8,6 +8,7 @@ import { Disposable } from '../../../base/common/lifecycle.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { VSBuffer } from '../../../base/common/buffer.js'; import { IBrowserViewBounds, IBrowserViewDevToolsStateEvent, IBrowserViewFocusEvent, IBrowserViewKeyDownEvent, IBrowserViewState, IBrowserViewNavigationEvent, IBrowserViewLoadingEvent, IBrowserViewLoadError, IBrowserViewTitleChangeEvent, IBrowserViewFaviconChangeEvent, IBrowserViewCaptureScreenshotOptions, IBrowserViewFindInPageOptions, IBrowserViewFindInPageResult, IBrowserViewVisibilityEvent, browserViewIsolatedWorldId, browserZoomFactors, browserZoomDefaultIndex, IBrowserViewOwner, IBrowserViewOpenOptions } from '../common/browserView.js'; +import { BrowserViewEmulator } from './browserViewEmulator.js'; import { BrowserViewInspector } from './browserViewInspector.js'; import { IWindowsMainService } from '../../windows/electron-main/windows.js'; import { ICodeWindow, LoadReason } from '../../window/electron-main/window.js'; @@ -41,6 +42,7 @@ export class BrowserView extends Disposable { private _browserZoomIndex: number = browserZoomDefaultIndex; readonly debugger: BrowserViewDebugger; + readonly emulator: BrowserViewEmulator; readonly inspector: BrowserViewInspector; private _ownerWindow: ICodeWindow; @@ -191,6 +193,7 @@ export class BrowserView extends Disposable { }); this.debugger = new BrowserViewDebugger(this, this.logService); + this.emulator = this._register(new BrowserViewEmulator(this, this.logService)); this.inspector = this._register(new BrowserViewInspector(this)); this.setupEventListeners(); @@ -472,7 +475,8 @@ export class BrowserView extends Disposable { certificateError: this.session.trust.getCertificateError(url), storageScope: this.session.storageScope, browserZoomIndex: this._browserZoomIndex, - isElementSelectionActive: this.inspector.isElementSelectionActive + isElementSelectionActive: this.inspector.isElementSelectionActive, + device: this.emulator.device }; } @@ -497,6 +501,11 @@ export class BrowserView extends Disposable { } this._view.setBorderRadius(Math.round(bounds.cornerRadius * bounds.zoomFactor)); + + if (bounds.emulation) { + this.emulator.applyScreenEmulation(bounds.emulation.viewportWidth, bounds.emulation.viewportHeight, bounds.emulation.scale, bounds.zoomFactor); + } + this._view.setBounds({ x: Math.round(bounds.x * bounds.zoomFactor), y: Math.round(bounds.y * bounds.zoomFactor), diff --git a/src/vs/platform/browserView/electron-main/browserViewEmulator.ts b/src/vs/platform/browserView/electron-main/browserViewEmulator.ts new file mode 100644 index 00000000000000..aa699988bc9f3d --- /dev/null +++ b/src/vs/platform/browserView/electron-main/browserViewEmulator.ts @@ -0,0 +1,112 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable, toDisposable } from '../../../base/common/lifecycle.js'; +import { Emitter, Event } from '../../../base/common/event.js'; +import { IBrowserDeviceProfile } from '../common/browserView.js'; +import { ILogService } from '../../log/common/log.js'; +import type { BrowserView } from './browserView.js'; + +/** + * Manages device emulation for a browser view. The renderer is authoritative + * for layout (it computes the on-screen size and emulation scale); this class + * just forwards values to `webContents.enableDeviceEmulation` and manages the + * touch / media / user-agent overrides that have no native Electron equivalent. + */ +export class BrowserViewEmulator extends Disposable { + + private _device: IBrowserDeviceProfile | undefined; + private readonly _defaultUserAgent: string; + private _lastApplied: { viewportWidth: number; viewportHeight: number; scale: number; hostZoom: number } | undefined; + + private readonly _onDidChange = this._register(new Emitter()); + readonly onDidChange: Event = this._onDidChange.event; + + constructor( + private readonly browser: BrowserView, + @ILogService private readonly logService: ILogService, + ) { + super(); + this._defaultUserAgent = this.browser.webContents.getUserAgent(); + + // Chromium may reset emulation on cross-process navigation. + const onNavigate = () => { + if (this._device) { + void this._applyTouchAndMedia(); + this._lastApplied = undefined; + } + }; + this.browser.webContents.on('did-navigate', onNavigate); + this._register(toDisposable(() => this.browser.webContents.removeListener('did-navigate', onNavigate))); + } + + get device(): IBrowserDeviceProfile | undefined { + return this._device; + } + + async setDevice(device: IBrowserDeviceProfile | undefined): Promise { + const prev = this._device; + this._device = device; + + const nextUA = device?.userAgent; + if (prev?.userAgent !== nextUA) { + this.browser.webContents.setUserAgent(nextUA ?? this._defaultUserAgent); + } + + const mobileChanged = !!prev?.mobile !== !!device?.mobile; + const toggled = !!prev !== !!device; + if (mobileChanged || toggled) { + await this._applyTouchAndMedia(); + } + + this._lastApplied = undefined; + if (!device) { + this.browser.webContents.disableDeviceEmulation(); + } + + this._onDidChange.fire(device); + } + + /** + * Apply viewport + scale via Chromium's emulation API. `hostZoom` is the host + * window's CSS-to-screen zoom factor: bounds in main are multiplied by it, + * so the emulation scale must be too or the emulated viewport won't fill + * the WebContentsView when the workbench is zoomed. + */ + applyScreenEmulation(viewportWidth: number, viewportHeight: number, scale: number, hostZoom: number): void { + if (!this._device) { + return; + } + const w = Math.max(1, Math.round(viewportWidth)); + const h = Math.max(1, Math.round(viewportHeight)); + const z = Math.max(0.01, hostZoom); + const s = Math.max(0.01, scale); + const last = this._lastApplied; + if (last && last.viewportWidth === w && last.viewportHeight === h + && Math.abs(last.scale - s) < 0.0001 && Math.abs(last.hostZoom - z) < 0.0001) { + return; + } + this._lastApplied = { viewportWidth: w, viewportHeight: h, scale: s, hostZoom: z }; + this.browser.webContents.enableDeviceEmulation({ + screenPosition: this._device.mobile ? 'mobile' : 'desktop', + screenSize: { width: w, height: h }, + viewSize: { width: w, height: h }, + deviceScaleFactor: this._device.deviceScaleFactor ?? 0, + viewPosition: { x: 0, y: 0 }, + scale: s * z, + }); + } + + private async _applyTouchAndMedia(): Promise { + const mobile = !!this._device?.mobile; + try { + await this.browser.debugger.sendCommand('Emulation.setTouchEmulationEnabled', { enabled: mobile, maxTouchPoints: mobile ? 5 : 1 }); + await this.browser.debugger.sendCommand('Emulation.setEmulatedMedia', { features: this._device ? [{ name: 'pointer', value: mobile ? 'coarse' : 'fine' }] : [] }); + await this.browser.debugger.sendCommand('Emulation.setEmitTouchEventsForMouse', { enabled: mobile }); + } catch (err) { + this.logService.error('[BrowserViewEmulator] _applyTouchAndMedia failed', err); + } + } +} diff --git a/src/vs/platform/browserView/electron-main/browserViewMainService.ts b/src/vs/platform/browserView/electron-main/browserViewMainService.ts index 967053905491e6..25d9a6e909b036 100644 --- a/src/vs/platform/browserView/electron-main/browserViewMainService.ts +++ b/src/vs/platform/browserView/electron-main/browserViewMainService.ts @@ -6,7 +6,7 @@ import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable, DisposableMap } from '../../../base/common/lifecycle.js'; import { VSBuffer } from '../../../base/common/buffer.js'; -import { IBrowserViewBounds, IBrowserViewState, IBrowserViewService, IBrowserViewCaptureScreenshotOptions, IBrowserViewFindInPageOptions, BrowserViewCommandId, IBrowserViewOwner, IBrowserViewInfo, IBrowserViewCreatedEvent, IBrowserViewOpenOptions, IBrowserViewCreateOptions, IBrowserViewTheme, IBrowserViewConfiguration } from '../common/browserView.js'; +import { IBrowserViewBounds, IBrowserViewState, IBrowserViewService, IBrowserViewCaptureScreenshotOptions, IBrowserViewFindInPageOptions, BrowserViewCommandId, IBrowserViewOwner, IBrowserViewInfo, IBrowserViewCreatedEvent, IBrowserViewOpenOptions, IBrowserViewCreateOptions, IBrowserViewTheme, IBrowserViewConfiguration, IBrowserDeviceProfile } from '../common/browserView.js'; import { clipboard, Menu, MenuItem } from 'electron'; import { IEnvironmentMainService } from '../../environment/electron-main/environmentMainService.js'; import { createDecorator, IInstantiationService } from '../../instantiation/common/instantiation.js'; @@ -187,6 +187,10 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa return this._getBrowserView(id).inspector.onDidChangeElementSelectionActive; } + onDynamicDidChangeDeviceEmulation(id: string) { + return this._getBrowserView(id).emulator.onDidChange; + } + async getState(id: string): Promise { return this._getBrowserView(id).getState(); } @@ -263,6 +267,10 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa return this._getBrowserView(id).setBrowserZoomIndex(zoomIndex); } + async setDeviceEmulation(id: string, device: IBrowserDeviceProfile | undefined): Promise { + return this._getBrowserView(id).emulator.setDevice(device); + } + async trustCertificate(id: string, host: string, fingerprint: string): Promise { return this._getBrowserView(id).trustCertificate(host, fingerprint); } diff --git a/src/vs/workbench/contrib/browserView/common/browserView.ts b/src/vs/workbench/contrib/browserView/common/browserView.ts index 14fcb2ddc275e6..8b48b0c1297e9c 100644 --- a/src/vs/workbench/contrib/browserView/common/browserView.ts +++ b/src/vs/workbench/contrib/browserView/common/browserView.ts @@ -35,7 +35,8 @@ import { IBrowserViewOwner, browserZoomDefaultIndex, browserZoomFactors, - IBrowserViewState + IBrowserViewState, + IBrowserDeviceProfile } from '../../../../platform/browserView/common/browserView.js'; import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; import { isLocalhostAuthority } from '../../../../platform/url/common/trustedDomains.js'; @@ -213,6 +214,7 @@ export interface IBrowserViewModel extends IDisposable { readonly canZoomIn: boolean; readonly canZoomOut: boolean; readonly isElementSelectionActive: boolean; + readonly device: IBrowserDeviceProfile | undefined; readonly onDidChangeSharingState: Event; readonly onDidChangeZoom: Event; @@ -229,6 +231,7 @@ export interface IBrowserViewModel extends IDisposable { readonly onWillDispose: Event; readonly onDidSelectElement: Event; readonly onDidChangeElementSelectionActive: Event; + readonly onDidChangeDevice: Event; layout(bounds: IBrowserViewBounds): Promise; setVisible(visible: boolean): Promise; @@ -251,6 +254,7 @@ export interface IBrowserViewModel extends IDisposable { resetZoom(): Promise; getConsoleLogs(): Promise; toggleElementSelection(enabled?: boolean): Promise; + setDevice(device: IBrowserDeviceProfile | undefined): Promise; } export class BrowserViewModel extends Disposable implements IBrowserViewModel { @@ -272,6 +276,7 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { private _sharedWithAgent: boolean = false; private _browserZoomIndex: number = browserZoomDefaultIndex; private _isElementSelectionActive: boolean = false; + private _device: IBrowserDeviceProfile | undefined; private readonly _onDidChangeSharingState = this._register(new Emitter()); readonly onDidChangeSharingState: Event = this._onDidChangeSharingState.event; @@ -314,6 +319,7 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { this._storageScope = initialState.storageScope; this._browserZoomIndex = initialState.browserZoomIndex; this._isElementSelectionActive = initialState.isElementSelectionActive; + this._device = initialState.device; this._isEphemeral = this._storageScope === BrowserViewStorageScope.Ephemeral; this._zoomHost = parseZoomHost(this._url); @@ -387,6 +393,10 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { this._visible = visible; })); + this._register(this.onDidChangeDevice(device => { + this._device = device; + })); + this._register(this.onDidChangeElementSelectionActive(active => { if (active) { this.telemetryService.publicLog2('integratedBrowser.addElementToChat.start', {}); @@ -425,6 +435,8 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { get zoomFactor(): number { return browserZoomFactors[this._browserZoomIndex]; } get canZoomIn(): boolean { return this._browserZoomIndex < browserZoomFactors.length - 1; } get canZoomOut(): boolean { return this._browserZoomIndex > 0; } + get isElementSelectionActive(): boolean { return this._isElementSelectionActive; } + get device(): IBrowserDeviceProfile | undefined { return this._device; } get onDidNavigate(): Event { return this.browserViewService.onDynamicDidNavigate(this.id); @@ -462,6 +474,10 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { return this.browserViewService.onDynamicDidChangeVisibility(this.id); } + get onDidChangeDevice(): Event { + return this.browserViewService.onDynamicDidChangeDeviceEmulation(this.id); + } + get onDidClose(): Event { return this.browserViewService.onDynamicDidClose(this.id); } @@ -583,10 +599,6 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { return this.browserViewService.getConsoleLogs(this.id); } - get isElementSelectionActive(): boolean { - return this._isElementSelectionActive; - } - async toggleElementSelection(enabled?: boolean): Promise { return this.browserViewService.toggleElementSelection(this.id, enabled); } @@ -599,6 +611,10 @@ export class BrowserViewModel extends Disposable implements IBrowserViewModel { return this.browserViewService.onDynamicDidChangeElementSelectionActive(this.id); } + async setDevice(device: IBrowserDeviceProfile | undefined): Promise { + return this.browserViewService.setDeviceEmulation(this.id, device); + } + private static readonly SHARE_DONT_ASK_KEY = 'browserView.shareWithAgent.dontAskAgain'; async setSharedWithAgent(shared: boolean): Promise { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts index 7fbbfad8b4c0eb..e09eb970857b14 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts @@ -103,11 +103,50 @@ export abstract class BrowserEditorContribution extends Disposable { * Called when the editor is laid out with a new dimension. */ layout(_width: number): void { } + + /** + * Called once after the editor's browser container DOM has been created. + * Use to do setup that needs to attach to `editor.browserContainer`. + */ + onContainerReady(_container: HTMLElement): void { } + + /** + * Return an override to customize how the editor sizes the browser + * container. Returning `undefined` falls through to the next contribution + * (and finally to the default: container fills the wrapper's content area). + * The first contribution to return a non-undefined override wins. + */ + getContainerLayoutOverride(): IContainerLayoutOverride | undefined { return undefined; } } -/** - * A widget that can be contributed to the browser editor URL bar. - */ +/** Customization returned by {@link BrowserEditorContribution.getContainerLayoutOverride}. */ +export interface IContainerLayoutOverride { + /** + * Wrapper padding (CSS px) — typically used to reserve space for widgets + * that sit outside the container (e.g. resize sashes). Applied as inline + * style before the pane is measured for {@link compute}. + */ + readonly padding: { + top?: number; + right?: number; + bottom?: number; + left?: number; + }; + /** Compute the container layout given the measured pane size. */ + compute(paneWidth: number, paneHeight: number): IContainerLayout; +} + +export interface IContainerLayout { + readonly width: number; + readonly height: number; + readonly emulation?: { + readonly viewportWidth: number; + readonly viewportHeight: number; + readonly scale: number; + }; +} + +/** A widget that can be contributed to the browser editor URL bar. */ export interface IBrowserEditorWidgetContribution { readonly element: HTMLElement; /** Ordering value — lower numbers appear first (left). */ @@ -366,6 +405,7 @@ export class BrowserEditor extends EditorPane { private overlayManager: BrowserOverlayManager | undefined; private _screenshotTimeout: ReturnType | undefined; private readonly _certActionButton = this._register(new MutableDisposable()); + private _currentPadding: { top: number; right: number; bottom: number; left: number } = { top: 0, right: 3, bottom: 3, left: 3 }; constructor( group: IEditorGroup, @@ -444,6 +484,12 @@ export class BrowserEditor extends EditorPane { this._browserContainer.tabIndex = 0; // make focusable this._browserContainerWrapper.appendChild(this._browserContainer); + // Notify contributions that the container DOM is ready (used e.g. by + // the device feature to attach resize sashes to the container). + for (const contribution of this._contributionInstances.values()) { + contribution.onContainerReady(this._browserContainer); + } + // Create additional wrapper around placeholder contents for applying border radius clipping. const placeholderContents = $('.browser-placeholder-contents'); this._browserContainer.appendChild(placeholderContents); @@ -610,6 +656,10 @@ export class BrowserEditor extends EditorPane { if (targetWindowId === this.window.vscodeWindowId) { // Update CSS variable for size calculations this._browserContainerWrapper.style.setProperty('--zoom-factor', String(getZoomFactor(this.window))); + // Re-push container bounds and emulation: zoom-factor affects + // both the screen-px conversion in main and the Chromium + // emulation scale (so the emulated viewport fills the WCV). + this.layoutBrowserContainer(); } })); @@ -1002,32 +1052,90 @@ export class BrowserEditor extends EditorPane { } /** - * Recompute the layout of the browser container and update the model with the new bounds. - * This should generally only be called via layout() to ensure that the container is ready and all necessary styles are loaded. + * Recompute the layout of the browser container and push the resulting + * bounds + emulation to the WebContentsView. Should generally only be + * called via {@link layout} so the container is fully styled first. */ layoutBrowserContainer(retries = 2): void { - if (this._model) { - this.checkOverlays(); - - const containerRect = this._browserContainer.getBoundingClientRect(); - const cornerRadius = this.window.getComputedStyle(this._browserContainer).borderTopLeftRadius ?? '0'; - - // This can happen under certain conditions. Keep trying for a couple of frames to allow things to stabilize. - if ((containerRect.width === 0 || containerRect.height === 0) && retries > 0) { - this.window.requestAnimationFrame(() => this.layoutBrowserContainer(retries - 1)); - return; + if (!this._model) { + return; + } + this.checkOverlays(); + + // Pick the first contribution that wants to override sizing. + let override: IContainerLayoutOverride | undefined; + for (const c of this._contributionInstances.values()) { + const o = c.getContainerLayoutOverride(); + if (o) { + override = o; + break; } + } - void this._model.layout({ - windowId: this.group.windowId, - x: containerRect.left, - y: containerRect.top, - width: containerRect.width, - height: containerRect.height, - zoomFactor: getZoomFactor(this.window), - cornerRadius: parseFloat(cornerRadius) - }); + // Apply the wrapper padding the editor will assume below. Inline style + // is the single source of truth — the wrapper's CSS has no padding. + // Right/bottom/left are clamped so the container always has breathing + // room (and resize sashes that sit on those edges remain reachable). + const raw = override?.padding; + const padding = { + top: raw?.top ?? 0, + right: Math.max(3, raw?.right ?? 0), + bottom: Math.max(3, raw?.bottom ?? 0), + left: Math.max(3, raw?.left ?? 0), + }; + this._currentPadding = padding; + this._browserContainerWrapper.style.padding = `${padding.top}px ${padding.right}px ${padding.bottom}px ${padding.left}px`; + + const wrapperRect = this._browserContainerWrapper.getBoundingClientRect(); + if ((wrapperRect.width === 0 || wrapperRect.height === 0) && retries > 0) { + // Wrapper not measured yet; retry on the next frame. + this.window.requestAnimationFrame(() => this.layoutBrowserContainer(retries - 1)); + return; } + + const paneWidth = Math.max(0, wrapperRect.width - padding.left - padding.right); + const paneHeight = Math.max(0, wrapperRect.height - padding.top - padding.bottom); + let layout: IContainerLayout; + if (override) { + layout = override.compute(paneWidth, paneHeight); + } else { + const z = getZoomFactor(this.window); + const snap = (v: number) => Math.floor(v * z) / z; + layout = { width: snap(paneWidth), height: snap(paneHeight) }; + } + + // Size the container, then derive its absolute screen rect analytically: + // the wrapper's flex rules center the container within the pane. + this._browserContainer.style.width = `${layout.width}px`; + this._browserContainer.style.height = `${layout.height}px`; + const containerLeft = wrapperRect.left + padding.left + (paneWidth - layout.width) / 2; + const containerTop = wrapperRect.top + padding.top + (paneHeight - layout.height) / 2; + const cornerRadius = parseFloat(this.window.getComputedStyle(this._browserContainer).borderTopLeftRadius ?? '0'); + void this._model.layout({ + windowId: this.group.windowId, + x: containerLeft, + y: containerTop, + width: layout.width, + height: layout.height, + zoomFactor: getZoomFactor(this.window), + cornerRadius, + emulation: layout.emulation, + }); + } + + /** + * Wrapper content-area size in CSS px — the maximum room the container + * can occupy after the active padding is applied. Derived from the last + * padding we wrote to the wrapper, so it stays in sync without re-reading + * the computed style. + */ + get paneSize(): { width: number; height: number } { + const r = this._browserContainerWrapper.getBoundingClientRect(); + const p = this._currentPadding; + return { + width: Math.max(0, r.width - p.left - p.right), + height: Math.max(0, r.height - p.top - p.bottom), + }; } override clearInput(): void { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserView.contribution.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserView.contribution.ts index e8fc1087cc783f..f2eee43e297936 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserView.contribution.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserView.contribution.ts @@ -25,6 +25,7 @@ import './features/browserDataStorageFeatures.js'; import './features/browserDevToolsFeature.js'; import './features/browserEditorChatFeatures.js'; import './features/browserEditorZoomFeature.js'; +import './features/browserEditorEmulationFeatures.js'; import './features/browserEditorFindFeature.js'; import './features/browserTabManagementFeatures.js'; diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserViewActions.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserViewActions.ts index 2bc3313b6ddcda..548235081240de 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserViewActions.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserViewActions.ts @@ -26,8 +26,9 @@ export const BrowserActionCategory = localize2('browserCategory', "Browser"); export enum BrowserActionGroup { Tabs = '1_tabs', Zoom = '2_zoom', - Page = '3_page', - Settings = '4_settings' + Developer = '3_developer', + Page = '4_page', + Settings = '5_settings' } class GoBackAction extends Action2 { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorEmulationFeatures.ts b/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorEmulationFeatures.ts new file mode 100644 index 00000000000000..b71bd9ce3df562 --- /dev/null +++ b/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorEmulationFeatures.ts @@ -0,0 +1,1004 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { $, addDisposableListener, EventType, getWindow } from '../../../../../base/browser/dom.js'; +import { getZoomFactor } from '../../../../../base/browser/browser.js'; +import { ActionBar } from '../../../../../base/browser/ui/actionbar/actionbar.js'; +import { IHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegate.js'; +import { ISashEvent, Orientation, OrthogonalEdge, Sash, SashState } from '../../../../../base/browser/ui/sash/sash.js'; +import { HoverPosition } from '../../../../../base/browser/ui/hover/hoverWidget.js'; +import { InputBox } from '../../../../../base/browser/ui/inputbox/inputBox.js'; +import { SelectBox } from '../../../../../base/browser/ui/selectBox/selectBox.js'; +import { Action } from '../../../../../base/common/actions.js'; +import { Codicon } from '../../../../../base/common/codicons.js'; +import { Emitter } from '../../../../../base/common/event.js'; +import { KeyCode } from '../../../../../base/common/keyCodes.js'; +import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js'; +import { ThemeIcon } from '../../../../../base/common/themables.js'; +import { localize, localize2 } from '../../../../../nls.js'; +import { MenuWorkbenchToolBar } from '../../../../../platform/actions/browser/toolbar.js'; +import { Action2, MenuId, MenuRegistry, registerAction2 } from '../../../../../platform/actions/common/actions.js'; +import { IBrowserDeviceProfile, IBrowserScreenProfile } from '../../../../../platform/browserView/common/browserView.js'; +import { ContextKeyExpr, IContextKey, IContextKeyService, RawContextKey } from '../../../../../platform/contextkey/common/contextkey.js'; +import { IContextViewService } from '../../../../../platform/contextview/browser/contextView.js'; +import { IHoverService, WorkbenchHoverDelegate } from '../../../../../platform/hover/browser/hover.js'; +import { IInstantiationService, ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; +import { KeybindingWeight } from '../../../../../platform/keybinding/common/keybindingsRegistry.js'; +import { IQuickInputService, IQuickPickItem } from '../../../../../platform/quickinput/common/quickInput.js'; +import { defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../../platform/theme/browser/defaultStyles.js'; +import { IEditorService } from '../../../../services/editor/common/editorService.js'; +import { IBrowserViewModel } from '../../common/browserView.js'; +import { BrowserEditor, BrowserEditorContribution, CONTEXT_BROWSER_HAS_ERROR, CONTEXT_BROWSER_HAS_URL, IContainerLayout, IContainerLayoutOverride } from '../browserEditor.js'; +import { BROWSER_EDITOR_ACTIVE, BrowserActionCategory, BrowserActionGroup } from '../browserViewActions.js'; + +const CONTEXT_BROWSER_EMULATION_TOOLBAR_VISIBLE = new RawContextKey( + 'browserEmulationToolbarVisible', + false, + localize('browser.emulationToolbarVisible', "Whether the browser emulation toolbar is visible") +); + +const CONTEXT_BROWSER_EMULATION_IS_MOBILE = new RawContextKey( + 'browserEmulationIsMobile', + false, + localize('browser.emulationIsMobile', "Whether the browser emulation is in mobile mode") +); + +const CONTEXT_BROWSER_EMULATION_HAS_USER_AGENT = new RawContextKey( + 'browserEmulationHasUserAgent', + false, + localize('browser.emulationHasUserAgent', "Whether the browser emulation has a custom user agent") +); + +/** + * A named device preset. Applying a preset stamps its `device` onto the + * active device profile and its `screen` (size only) onto the active screen + * profile, preserving the user's current zoom. + */ +export interface IBrowserDevicePreset { + readonly name: string; + readonly device?: IBrowserDeviceProfile; + readonly screen?: IBrowserScreenProfile; +} + +/** + * Keep track of the last used device and screen settings so we can restore them when the toolbar is opened. + * Note this isn't (currently) persisted in storage. + */ +const lastSettings = { + device: undefined as IBrowserDeviceProfile | undefined, + screen: undefined as IBrowserScreenProfile | undefined, +}; + +/** + * Toolbar shown above the browser viewport with device emulation controls + * (dimensions, DPR, zoom, and an action toolbar for presets / UA / mobile / close). + */ +class BrowserEmulationToolbar extends Disposable { + + readonly element: HTMLElement; + private readonly _groupWrapper: HTMLElement; + + private readonly _widthInput: InputBox; + private readonly _heightInput: InputBox; + private readonly _swapDimensionsAction: Action; + private readonly _dprInput: InputBox; + private readonly _zoom: SelectBox; + + private _suppressChange = false; + private _autoFitScale = 1; + + private static readonly ZOOM_PRESETS = [0.5, 0.75, 1, 1.25, 1.5, 2]; + private static readonly AUTO_INDEX = 0; + + constructor( + private readonly _feature: BrowserEditorEmulationSupport, + actionsContainer: HTMLElement, + hoverDelegate: IHoverDelegate, + @IContextViewService contextViewService: IContextViewService, + @IHoverService hoverService: IHoverService, + ) { + super(); + + this.element = $('.browser-emulation-toolbar'); + this.element.style.display = 'none'; + + this._groupWrapper = $('.browser-emulation-toolbar-groups'); + this.element.appendChild(this._groupWrapper); + + const dimensions = this._appendGroup('dimensions'); + const dimensionsLabel = $('span.browser-emulation-toolbar-label'); + dimensionsLabel.textContent = localize('browser.device.dimensionsLabel', "Dimensions:"); + dimensions.appendChild(dimensionsLabel); + this._widthInput = this._createNumberInput(dimensions, contextViewService, localize('browser.device.widthAriaLabel', "Viewport width"), 1, 9999); + + const swapDimensionsLabel = localize('browser.device.swapDimensionsTitle', "Swap Dimensions"); + this._swapDimensionsAction = this._register(new Action( + 'browser.device.swapDimensions', + swapDimensionsLabel, + ThemeIcon.asClassName(Codicon.arrowSwap), + false, + async () => this._feature.swapDimensions() + )); + const swapDimensionsBar = this._register(new ActionBar(dimensions, { hoverDelegate })); + swapDimensionsBar.push(this._swapDimensionsAction, { icon: true, label: false }); + + this._heightInput = this._createNumberInput(dimensions, contextViewService, localize('browser.device.heightAriaLabel', "Viewport height"), 1, 9999); + + // DPR override. Blank / 0 = system DPR. + const dprGroup = this._appendGroup('dpr'); + const dprLabel = $('span.browser-emulation-toolbar-label'); + dprLabel.textContent = localize('browser.device.dprLabel', "DPR:"); + this._register(hoverService.setupManagedHover(hoverDelegate, dprLabel, localize('browser.device.dprTitle', "Device pixel ratio (blank = system default)"))); + dprGroup.appendChild(dprLabel); + this._dprInput = this._createNumberInput(dprGroup, contextViewService, localize('browser.device.dprAriaLabel', "Device pixel ratio"), 0, 8, 'decimal'); + + const zoomGroup = this._appendGroup('zoom'); + const zoomLabel = $('span.browser-emulation-toolbar-label'); + zoomLabel.textContent = localize('browser.device.scaleLabel', "Scale:"); + zoomGroup.appendChild(zoomLabel); + this._zoom = this._register(new SelectBox( + this._buildZoomOptions(), + BrowserEmulationToolbar.AUTO_INDEX, + contextViewService, + defaultSelectBoxStyles, + { ariaLabel: localize('browser.device.zoomAriaLabel', "Zoom factor") } + )); + this._zoom.render(zoomGroup); + + this.element.appendChild($('.browser-emulation-toolbar-spacer')); + + this.element.appendChild(actionsContainer); + + this._registerEvents(); + } + + private _registerEvents(): void { + const commitDims = () => this._onDimensionInput(); + const onEnterDims = (e: KeyboardEvent) => { + if (e.keyCode === KeyCode.Enter) { + this._onDimensionInput(); + } + }; + this._register(addDisposableListener(this._widthInput.inputElement, EventType.CHANGE, commitDims)); + this._register(addDisposableListener(this._heightInput.inputElement, EventType.CHANGE, commitDims)); + this._register(addDisposableListener(this._widthInput.inputElement, EventType.KEY_DOWN, onEnterDims)); + this._register(addDisposableListener(this._heightInput.inputElement, EventType.KEY_DOWN, onEnterDims)); + + this._register(addDisposableListener(this._dprInput.inputElement, EventType.CHANGE, () => this._onDprInput())); + this._register(addDisposableListener(this._dprInput.inputElement, EventType.KEY_DOWN, (e: KeyboardEvent) => { + if (e.keyCode === KeyCode.Enter) { + this._onDprInput(); + } + })); + + this._register(this._zoom.onDidSelect(e => { + const model = this._feature.model; + if (this._suppressChange || !model?.device) { + return; + } + const screen = this._feature.screen ?? {}; + const scale = e.index === BrowserEmulationToolbar.AUTO_INDEX + ? undefined + : BrowserEmulationToolbar.ZOOM_PRESETS[e.index - 1]; + if (scale === screen.scale) { + return; + } + this._feature.setScreen({ ...screen, scale }); + })); + } + + get isVisible(): boolean { + return this.element.style.display !== 'none'; + } + + show(): void { + this.element.style.display = ''; + } + + hide(): void { + this.element.style.display = 'none'; + } + + setAutoFitScale(scale: number): void { + if (this._autoFitScale === scale) { + return; + } + const oldPercent = Math.round(this._autoFitScale * 100); + this._autoFitScale = scale; + const newPercent = Math.round(scale * 100); + if (oldPercent !== newPercent) { + // setOptions rebuilds