diff --git a/build/.cachesalt b/build/.cachesalt index d382d477ac6c7..c57aa9baf4ff9 100644 --- a/build/.cachesalt +++ b/build/.cachesalt @@ -1 +1 @@ -2026-06-22T15:10:10.546Z +2026-07-07T20:00:56.412Z \ No newline at end of file diff --git a/build/lib/i18n.resources.json b/build/lib/i18n.resources.json index 05de99a03a982..b1dbfbbdc8ac9 100644 --- a/build/lib/i18n.resources.json +++ b/build/lib/i18n.resources.json @@ -680,6 +680,10 @@ "name": "vs/sessions/contrib/chat", "project": "vscode-sessions" }, + { + "name": "vs/sessions/contrib/promptTimeline", + "project": "vscode-sessions" + }, { "name": "vs/sessions/contrib/providers/copilotChatSessions", "project": "vscode-sessions" diff --git a/build/lib/stylelint/validateDesignTokens.ts b/build/lib/stylelint/validateDesignTokens.ts index 91fd26d361edf..01c1a51ff0ddd 100644 --- a/build/lib/stylelint/validateDesignTokens.ts +++ b/build/lib/stylelint/validateDesignTokens.ts @@ -121,12 +121,12 @@ export function validateCodiconFontSizes(text: string): IDesignTokenViolation[] /** Exact px value -> suggested token var(s). Ambiguous values list alternatives. */ const FONT_SIZE_RAMP: ReadonlyMap = new Map([ - [26, 'var(--vscode-agents-fontSize-heading1)'], - [18, 'var(--vscode-agents-fontSize-heading2)'], - [13, 'var(--vscode-bodyFontSize) or var(--vscode-agents-fontSize-heading3) or var(--vscode-agents-fontSize-body1)'], - [12, 'var(--vscode-bodyFontSize-small) or var(--vscode-agents-fontSize-label1)'], - [11, 'var(--vscode-bodyFontSize-xSmall) or var(--vscode-agents-fontSize-body2) or var(--vscode-agents-fontSize-label2)'], - [10, 'var(--vscode-agents-fontSize-label3)'], + [26, 'var(--vscode-fontSize-heading1)'], + [18, 'var(--vscode-fontSize-heading2)'], + [13, 'var(--vscode-fontSize-body1) or var(--vscode-fontSize-heading3)'], + [12, 'var(--vscode-fontSize-label1)'], + [11, 'var(--vscode-fontSize-body2) or var(--vscode-fontSize-label2)'], + [10, 'var(--vscode-fontSize-label3)'], ]); /** @@ -255,7 +255,7 @@ export function validateCornerRadiusTokens(text: string): IDesignTokenViolation[ // Font-weight token suggestions (sessions design-system area only) // --------------------------------------------------------------------------- // -// The agents font ramp defines exactly two weights: regular (400) and +// The font ramp defines exactly two weights: regular (400) and // semiBold (600). Any other numeric weight (e.g. 500, 700) is off the ramp and // snaps to the nearer of the two. The CSS keywords `normal` (400) and `bold` // (700) are normalised before snapping. `inherit`, `lighter`, `bolder` and @@ -268,7 +268,7 @@ interface IFontWeightToken { readonly name: string; } -/** The only two weights in the agents ramp. */ +/** The only two weights in the font ramp. */ const FONT_WEIGHT_TOKENS: readonly IFontWeightToken[] = [ { weight: 400, name: 'regular' }, { weight: 600, name: 'semiBold' }, @@ -303,7 +303,7 @@ function snapFontWeight(weight: number): IFontWeightToken { } /** - * Finds hardcoded `font-weight` values and suggests the agents weight-ramp var. + * Finds hardcoded `font-weight` values and suggests the weight-ramp var. * Exact ramp values (400 / 600, plus `normal` = 400) are flagged as drop-in * replacements; off-ramp values (e.g. 500, 700, `bold`) snap to the nearer * token and call out that the value is off the two-weight ramp. `inherit`, @@ -332,7 +332,7 @@ export function validateFontWeightTokens(text: string): IDesignTokenViolation[] const note = exact ? '' : ' (off-ramp, 400/600 only)'; violations.push({ line, - message: `${shown} -> var(--vscode-agents-fontWeight-${token.name})${note}` + message: `${shown} -> var(--vscode-fontWeight-${token.name})${note}` }); }); @@ -475,3 +475,55 @@ export function validateStrokeTokens(text: string): IDesignTokenViolation[] { return violations; } + +// --------------------------------------------------------------------------- +// Deprecated token usage +// --------------------------------------------------------------------------- +// +// Some `--vscode-*` size tokens have been superseded by more generic ones and +// are marked `@deprecated` in the size registry. Any remaining usage of a +// deprecated token var is reported with its drop-in replacement, regardless of +// which property it appears in. + +interface IDeprecatedToken { + readonly deprecated: string; + readonly replacement: string; +} + +/** Deprecated token var -> its replacement. */ +const DEPRECATED_TOKENS: readonly IDeprecatedToken[] = [ + { deprecated: '--vscode-bodyFontSize', replacement: '--vscode-fontSize-body1' }, + { deprecated: '--vscode-bodyFontSize-small', replacement: '--vscode-fontSize-label1' }, + { deprecated: '--vscode-bodyFontSize-xSmall', replacement: '--vscode-fontSize-body2' }, +]; + +// Longest-first so a suffixed token (e.g. `-small`) matches before the base +// token that is its prefix (`--vscode-bodyFontSize`). +const DEPRECATED_TOKENS_SORTED = [...DEPRECATED_TOKENS].sort((a, b) => b.deprecated.length - a.deprecated.length); + +/** + * Finds usages of deprecated token vars and suggests the replacement token. + * A match is only reported when the token is not the prefix of a longer token + * (the next character does not continue the identifier). Returns one finding + * per occurrence so the terminal can linkify each `file(line,col)`. + */ +export function validateDeprecatedTokens(text: string): IDesignTokenViolation[] { + const violations: IDesignTokenViolation[] = []; + + forEachDeclaration(text, (line, _selector, declaration) => { + for (const { deprecated, replacement } of DEPRECATED_TOKENS_SORTED) { + for (let idx = declaration.indexOf(deprecated); idx !== -1; idx = declaration.indexOf(deprecated, idx + deprecated.length)) { + const nextChar = declaration[idx + deprecated.length] ?? ''; + if (/[\w-]/.test(nextChar)) { + continue; // prefix of a longer token + } + violations.push({ + line, + message: `${deprecated} -> ${replacement} (deprecated)` + }); + } + } + }); + + return violations; +} diff --git a/build/lib/stylelint/vscode-known-variables.json b/build/lib/stylelint/vscode-known-variables.json index 4ef43f8ce6c5b..4346e1d8d47fe 100644 --- a/build/lib/stylelint/vscode-known-variables.json +++ b/build/lib/stylelint/vscode-known-variables.json @@ -955,6 +955,7 @@ "--session-view-background", "--session-view-foreground", "--session-view-centered-content-max-width", + "--prompt-timeline-bottom", "--chat-editing-last-edit-shift", "--chat-voice-icon-glow-color", "--chat-current-response-min-height", diff --git a/build/stylelint.ts b/build/stylelint.ts index 31313a3d2b84d..38c263963f487 100644 --- a/build/stylelint.ts +++ b/build/stylelint.ts @@ -7,7 +7,7 @@ import es from 'event-stream'; import vfs from 'vinyl-fs'; import { stylelintFilter } from './filters.ts'; import { getVariableNameValidator } from './lib/stylelint/validateVariableNames.ts'; -import { validateCodiconFontSizes, validateFontSizeTokens, validateFontWeightTokens, validateCornerRadiusTokens, validateSpacingTokens, validateStrokeTokens } from './lib/stylelint/validateDesignTokens.ts'; +import { validateCodiconFontSizes, validateFontSizeTokens, validateFontWeightTokens, validateCornerRadiusTokens, validateSpacingTokens, validateStrokeTokens, validateDeprecatedTokens } from './lib/stylelint/validateDesignTokens.ts'; interface FileWithLines { __lines?: string[]; @@ -32,7 +32,7 @@ export default function gulpstylelint(reporter: Reporter, designTokensEverywhere const layerCheckerDisablePattern = /\/\*\s*stylelint-disable\s+layer-checker\s*\*\//; // Per-category tally of design-token suggestions for the summary footer. - const designTokenCounts: Record = { codicon: 0, 'font-size': 0, weight: 0, radius: 0, spacing: 0, stroke: 0 }; + const designTokenCounts: Record = { codicon: 0, 'font-size': 0, weight: 0, radius: 0, spacing: 0, stroke: 0, deprecated: 0 }; let designTokenFileCount = 0; return es.through(function (this, file: FileWithLines) { @@ -73,6 +73,7 @@ export default function gulpstylelint(reporter: Reporter, designTokensEverywhere for (const v of validateCornerRadiusTokens(contents)) { findings.push({ line: v.line, category: 'radius', message: v.message }); } for (const v of validateSpacingTokens(contents)) { findings.push({ line: v.line, category: 'spacing', message: v.message }); } for (const v of validateStrokeTokens(contents)) { findings.push({ line: v.line, category: 'stroke', message: v.message }); } + for (const v of validateDeprecatedTokens(contents)) { findings.push({ line: v.line, category: 'deprecated', message: v.message }); } if (findings.length > 0) { findings.sort((a, b) => a.line - b.line); @@ -91,7 +92,7 @@ export default function gulpstylelint(reporter: Reporter, designTokensEverywhere if (errorCount > 0) { reporter('All valid variable names are in `build/lib/stylelint/vscode-known-variables.json`\nTo update that file, run `./scripts/test-documentation.sh|bat.`', false); } - const designTokenTotal = designTokenCounts.codicon + designTokenCounts['font-size'] + designTokenCounts.weight + designTokenCounts.radius + designTokenCounts.spacing + designTokenCounts.stroke; + const designTokenTotal = designTokenCounts.codicon + designTokenCounts['font-size'] + designTokenCounts.weight + designTokenCounts.radius + designTokenCounts.spacing + designTokenCounts.stroke + designTokenCounts.deprecated; if (designTokenTotal > 0) { reporter('', false); reporter( @@ -101,7 +102,8 @@ export default function gulpstylelint(reporter: Reporter, designTokensEverywhere ', weight ' + designTokenCounts.weight + ', radius ' + designTokenCounts.radius + ', spacing ' + designTokenCounts.spacing + - ', stroke ' + designTokenCounts.stroke + ')', + ', stroke ' + designTokenCounts.stroke + + ', deprecated ' + designTokenCounts.deprecated + ')', false ); } diff --git a/extensions/copilot/package.json b/extensions/copilot/package.json index e76a4a2035b1f..4e9731490d961 100644 --- a/extensions/copilot/package.json +++ b/extensions/copilot/package.json @@ -1918,12 +1918,16 @@ }, "maxInputTokens": { "type": "number", - "description": "Maximum number of input tokens supported by the model" + "markdownDescription": "Maximum number of input (prompt) tokens supported by the model. Optional when `contextWindow` is set, in which case it is derived as `contextWindow - maxOutputTokens`." }, "maxOutputTokens": { "type": "number", "description": "Maximum number of output tokens supported by the model" }, + "contextWindow": { + "type": "number", + "markdownDescription": "The model's full context window (input + output) in tokens, e.g. `1000000` for a 1M model. When set it is the source of truth for the context window and `maxInputTokens` can be omitted. Otherwise the window is derived as `maxInputTokens + maxOutputTokens`." + }, "editTools": { "type": "array", "description": "List of edit tools supported by the model. If this is not configured, the editor will try multiple edit tools and pick the best one.\n\n- 'find-replace': Find and replace text in a document.\n- 'multi-find-replace': Find and replace text in a document.\n- 'apply-patch': A file-oriented diff format used by some OpenAI models\n- 'code-rewrite': A general but slower editing tool that allows the model to rewrite and code snippet and provide only the replacement to the editor.", @@ -1981,8 +1985,19 @@ "url", "toolCalling", "vision", - "maxInputTokens", "maxOutputTokens" + ], + "anyOf": [ + { + "required": [ + "maxInputTokens" + ] + }, + { + "required": [ + "contextWindow" + ] + } ] } } @@ -2089,12 +2104,16 @@ }, "maxInputTokens": { "type": "number", - "description": "Maximum number of input tokens supported by the model" + "markdownDescription": "Maximum number of input (prompt) tokens supported by the model. Optional when `contextWindow` is set, in which case it is derived as `contextWindow - maxOutputTokens`." }, "maxOutputTokens": { "type": "number", "description": "Maximum number of output tokens supported by the model" }, + "contextWindow": { + "type": "number", + "markdownDescription": "The model's full context window (input + output) in tokens, e.g. `1000000` for a 1M model. When set it is the source of truth for the context window and `maxInputTokens` can be omitted. Otherwise the window is derived as `maxInputTokens + maxOutputTokens`." + }, "editTools": { "type": "array", "description": "List of edit tools supported by the model. If this is not configured, the editor will try multiple edit tools and pick the best one.\n\n- 'find-replace': Find and replace text in a document.\n- 'multi-find-replace': Find and replace text in a document.\n- 'apply-patch': A file-oriented diff format used by some OpenAI models\n- 'code-rewrite': A general but slower editing tool that allows the model to rewrite and code snippet and provide only the replacement to the editor.", @@ -2176,8 +2195,19 @@ "url", "toolCalling", "vision", - "maxInputTokens", "maxOutputTokens" + ], + "anyOf": [ + { + "required": [ + "maxInputTokens" + ] + }, + { + "required": [ + "contextWindow" + ] + } ] } } @@ -2240,12 +2270,16 @@ }, "maxInputTokens": { "type": "number", - "description": "Maximum number of input tokens supported by the model" + "markdownDescription": "Maximum number of input (prompt) tokens supported by the model. Optional when `contextWindow` is set, in which case it is derived as `contextWindow - maxOutputTokens`." }, "maxOutputTokens": { "type": "number", "description": "Maximum number of output tokens supported by the model" }, + "contextWindow": { + "type": "number", + "markdownDescription": "The model's full context window (input + output) in tokens, e.g. `1000000` for a 1M model. When set it is the source of truth for the context window and `maxInputTokens` can be omitted. Otherwise the window is derived as `maxInputTokens + maxOutputTokens`." + }, "thinking": { "type": "boolean", "default": false, @@ -2290,8 +2324,19 @@ "url", "toolCalling", "vision", - "maxInputTokens", "maxOutputTokens" + ], + "anyOf": [ + { + "required": [ + "maxInputTokens" + ] + }, + { + "required": [ + "contextWindow" + ] + } ] } } diff --git a/extensions/copilot/src/extension/byok/common/byokProvider.ts b/extensions/copilot/src/extension/byok/common/byokProvider.ts index 5427c6904d764..a47b6d00637b6 100644 --- a/extensions/copilot/src/extension/byok/common/byokProvider.ts +++ b/extensions/copilot/src/extension/byok/common/byokProvider.ts @@ -47,8 +47,19 @@ export type BYOKModelConfig = BYOKGlobalKeyModelConfig | BYOKPerModelConfig | BY export interface BYOKModelCapabilities { name: string; url?: string; - maxInputTokens: number; + /** + * The maximum number of prompt (input) tokens. Optional when {@link contextWindow} + * is supplied, in which case it is derived as `contextWindow - maxOutputTokens`. + */ + maxInputTokens?: number; maxOutputTokens: number; + /** + * The model's full context window (input + output) in tokens. Many providers + * publish this directly (e.g. "Context Length: 1M"). When set it is used as the + * source of truth for the context window; otherwise the window is derived as + * `maxInputTokens + maxOutputTokens` for backward compatibility. + */ + contextWindow?: number; toolCalling: boolean; vision: boolean; thinking?: boolean; @@ -93,6 +104,33 @@ export function isNoAuthConfig(config: BYOKModelConfig): config is BYOKNoAuthMod return !('apiKey' in config) && !('deploymentUrl' in config); } +/** + * Resolves a model's token limits from its BYOK capabilities, honoring an explicit + * {@link BYOKModelCapabilities.contextWindow} when provided. + * + * - When `contextWindow` is set it is the source of truth for the full window and, if + * `maxInputTokens` is omitted, the prompt budget is derived as + * `contextWindow - maxOutputTokens`. + * - Otherwise the window falls back to `maxInputTokens + maxOutputTokens` for backward + * compatibility. + * + * The returned limits are always internally consistent: `maxOutputTokens` is clamped so + * it never exceeds the context window, and `maxInputTokens` is clamped to the remaining + * budget (`contextWindow - maxOutputTokens`). This prevents invalid combinations such as + * `maxOutputTokens > contextWindow`, or a `maxInputTokens` supplied alongside a smaller + * `contextWindow` overflowing the window. + */ +export function resolveModelTokenLimits(capabilities: Pick): { contextWindow: number; maxInputTokens: number; maxOutputTokens: number } { + const contextWindow = capabilities.contextWindow ?? ((capabilities.maxInputTokens ?? 0) + capabilities.maxOutputTokens); + // The output budget can never exceed the full window. + const maxOutputTokens = Math.min(capabilities.maxOutputTokens, contextWindow); + // The prompt budget is whatever remains after the output reservation; an explicitly + // provided maxInputTokens is clamped to that remaining budget. + const remainingInputBudget = Math.max(0, contextWindow - maxOutputTokens); + const maxInputTokens = Math.min(capabilities.maxInputTokens ?? remainingInputBudget, remainingInputBudget); + return { contextWindow, maxInputTokens, maxOutputTokens }; +} + export function resolveModelInfo(modelId: string, providerName: string, knownModels: BYOKKnownModels | undefined, modelCapabilities?: BYOKModelCapabilities): IChatModelInformation { // Model Capabilities are something the user has decided on so those take precedence, then we rely on known model info, then defaults. let knownModelInfo = modelCapabilities; @@ -100,7 +138,9 @@ export function resolveModelInfo(modelId: string, providerName: string, knownMod knownModelInfo = knownModels[modelId]; } const modelName = knownModelInfo?.name || modelId; - const contextWinow = knownModelInfo ? (knownModelInfo.maxInputTokens + knownModelInfo.maxOutputTokens) : 128000; + const limits = knownModelInfo + ? resolveModelTokenLimits(knownModelInfo) + : { contextWindow: 128000, maxInputTokens: 100000, maxOutputTokens: 8192 }; const modelInfo: IChatModelInformation = { id: modelId, name: modelName, @@ -119,9 +159,9 @@ export function resolveModelInfo(modelId: string, providerName: string, knownMod }, tokenizer: TokenizerType.O200K, limits: { - max_context_window_tokens: contextWinow, - max_prompt_tokens: knownModelInfo?.maxInputTokens || 100000, - max_output_tokens: knownModelInfo?.maxOutputTokens || 8192 + max_context_window_tokens: limits.contextWindow, + max_prompt_tokens: limits.maxInputTokens, + max_output_tokens: limits.maxOutputTokens } }, is_chat_default: false, @@ -146,12 +186,13 @@ export function byokKnownModelsToAPIInfo(providerName: string, knownModels: BYOK } export function byokKnownModelToAPIInfo(providerName: string, id: string, capabilities: BYOKModelCapabilities): LanguageModelChatInformation { + const limits = resolveModelTokenLimits(capabilities); return { id, name: capabilities.name, version: '1.0.0', - maxOutputTokens: capabilities.maxOutputTokens, - maxInputTokens: capabilities.maxInputTokens, + maxOutputTokens: limits.maxOutputTokens, + maxInputTokens: limits.maxInputTokens, // `detail` is intentionally omitted: when this model is resolved // via a configured provider group, `LanguageModelsService` will // fall back to the group name so multiple instances of the same diff --git a/extensions/copilot/src/extension/byok/common/test/byokProvider.spec.ts b/extensions/copilot/src/extension/byok/common/test/byokProvider.spec.ts index caaaf0dbfad25..cd34a9381644d 100644 --- a/extensions/copilot/src/extension/byok/common/test/byokProvider.spec.ts +++ b/extensions/copilot/src/extension/byok/common/test/byokProvider.spec.ts @@ -5,7 +5,7 @@ import { describe, expect, it } from 'vitest'; import { CopilotToken } from '../../../../platform/authentication/common/copilotToken'; -import { byokKnownModelToAPIInfo, BYOKModelCapabilities, isClientBYOKAllowed, resolveModelInfo } from '../byokProvider'; +import { byokKnownModelToAPIInfo, BYOKModelCapabilities, isClientBYOKAllowed, resolveModelInfo, resolveModelTokenLimits } from '../byokProvider'; describe('byokKnownModelToAPIInfo', () => { const baseCapabilities: BYOKModelCapabilities = { @@ -43,6 +43,21 @@ describe('byokKnownModelToAPIInfo', () => { expect(info.capabilities.editTools).toBeUndefined(); }); + + it('derives maxInputTokens from contextWindow when maxInputTokens is omitted', () => { + // The value surfaced here becomes `model.maxInputTokens`, which the custom + // endpoint/OAI/azure providers read when building the endpoint. + const info = byokKnownModelToAPIInfo('TestProvider', 'm1', { + name: 'BigContextModel', + contextWindow: 1000000, + maxOutputTokens: 384000, + toolCalling: true, + vision: false, + }); + + expect(info.maxInputTokens).toBe(1000000 - 384000); + expect(info.maxOutputTokens).toBe(384000); + }); }); describe('resolveModelInfo', () => { @@ -86,6 +101,73 @@ describe('resolveModelInfo', () => { top_p: 0.95, }); }); + + it('honors an explicit contextWindow as the source of truth for the context window', () => { + // A model documented as: Context Length 1M, Max Output 384K. The user can now + // declare the real capability directly instead of back-computing maxInputTokens. + const info = resolveModelInfo('m1', 'TestProvider', undefined, { + ...baseCapabilities, + contextWindow: 1000000, + maxOutputTokens: 384000, + maxInputTokens: undefined, + }); + + expect(info.capabilities.limits?.max_context_window_tokens).toBe(1000000); + // The prompt budget is derived as contextWindow - maxOutputTokens. + expect(info.capabilities.limits?.max_prompt_tokens).toBe(1000000 - 384000); + expect(info.capabilities.limits?.max_output_tokens).toBe(384000); + }); + + it('derives the context window as maxInputTokens + maxOutputTokens when contextWindow is absent', () => { + const info = resolveModelInfo('m1', 'TestProvider', undefined, { + ...baseCapabilities, + maxInputTokens: 616000, + maxOutputTokens: 384000, + }); + + expect(info.capabilities.limits?.max_context_window_tokens).toBe(616000 + 384000); + expect(info.capabilities.limits?.max_prompt_tokens).toBe(616000); + }); + + it('falls back to a 128000 context window when no capabilities are known', () => { + const info = resolveModelInfo('m1', 'TestProvider', undefined, undefined); + + expect(info.capabilities.limits?.max_context_window_tokens).toBe(128000); + }); +}); + +describe('resolveModelTokenLimits', () => { + it('derives the window from maxInputTokens + maxOutputTokens when contextWindow is absent', () => { + expect(resolveModelTokenLimits({ maxInputTokens: 616000, maxOutputTokens: 384000 })).toEqual({ + contextWindow: 1000000, + maxInputTokens: 616000, + maxOutputTokens: 384000, + }); + }); + + it('derives maxInputTokens from contextWindow when maxInputTokens is omitted', () => { + expect(resolveModelTokenLimits({ contextWindow: 1000000, maxOutputTokens: 384000 })).toEqual({ + contextWindow: 1000000, + maxInputTokens: 616000, + maxOutputTokens: 384000, + }); + }); + + it('clamps maxOutputTokens so it never exceeds the context window', () => { + expect(resolveModelTokenLimits({ contextWindow: 1000, maxOutputTokens: 8192 })).toEqual({ + contextWindow: 1000, + maxInputTokens: 0, + maxOutputTokens: 1000, + }); + }); + + it('clamps an explicit maxInputTokens to the remaining budget when it overflows the window', () => { + expect(resolveModelTokenLimits({ contextWindow: 1000000, maxInputTokens: 900000, maxOutputTokens: 384000 })).toEqual({ + contextWindow: 1000000, + maxInputTokens: 616000, + maxOutputTokens: 384000, + }); + }); }); describe('isClientBYOKAllowed', () => { diff --git a/extensions/copilot/src/extension/byok/vscode-node/azureProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/azureProvider.ts index c160d03e02574..0d42778c40c40 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/azureProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/azureProvider.ts @@ -131,6 +131,7 @@ export class AzureBYOKModelProvider extends AbstractCustomOAIBYOKModelProvider { const modelCapabilities = { maxInputTokens: model.maxInputTokens, maxOutputTokens: model.maxOutputTokens, + contextWindow: modelConfiguration?.contextWindow, toolCalling: !!model.capabilities?.toolCalling || false, vision: !!model.capabilities?.imageInput || false, name: model.name, diff --git a/extensions/copilot/src/extension/byok/vscode-node/customEndpointProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/customEndpointProvider.ts index ea5896d727974..565e6c05a8a0f 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/customEndpointProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/customEndpointProvider.ts @@ -90,8 +90,11 @@ interface _CustomEndpointModelConfig { name: string; url: string; apiType?: CustomEndpointApiType; - maxInputTokens: number; + /** Optional when {@link contextWindow} is set; then derived as `contextWindow - maxOutputTokens`. */ + maxInputTokens?: number; maxOutputTokens: number; + /** The model's full context window (input + output) in tokens, e.g. 1000000 for a 1M model. */ + contextWindow?: number; toolCalling: boolean; vision: boolean; thinking?: boolean; @@ -153,6 +156,7 @@ export class CustomEndpointBYOKModelProvider extends AbstractOpenAICompatibleLMP const modelCapabilities = { maxInputTokens: model.maxInputTokens, maxOutputTokens: model.maxOutputTokens, + contextWindow: modelConfiguration?.contextWindow, toolCalling: !!model.capabilities?.toolCalling || false, vision: !!model.capabilities?.imageInput || false, name: model.name, diff --git a/extensions/copilot/src/extension/byok/vscode-node/customOAIProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/customOAIProvider.ts index 0237f910396e5..318f14dedbae4 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/customOAIProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/customOAIProvider.ts @@ -53,8 +53,11 @@ export interface CustomOAIModelProviderConfig extends LanguageModelChatConfigura interface _CustomOAIModelConfig { name: string; url: string; - maxInputTokens: number; + /** Optional when {@link contextWindow} is set; then derived as `contextWindow - maxOutputTokens`. */ + maxInputTokens?: number; maxOutputTokens: number; + /** The model's full context window (input + output) in tokens, e.g. 1000000 for a 1M model. */ + contextWindow?: number; toolCalling: boolean; vision: boolean; thinking?: boolean; @@ -138,6 +141,7 @@ export abstract class AbstractCustomOAIBYOKModelProvider extends AbstractOpenAIC const modelCapabilities = { maxInputTokens: model.maxInputTokens, maxOutputTokens: model.maxOutputTokens, + contextWindow: modelConfiguration?.contextWindow, toolCalling: !!model.capabilities?.toolCalling || false, vision: !!model.capabilities?.imageInput || false, name: model.name, diff --git a/extensions/copilot/src/extension/byok/vscode-node/openRouterProvider.ts b/extensions/copilot/src/extension/byok/vscode-node/openRouterProvider.ts index fe0e4c02716df..02b9302b453c6 100644 --- a/extensions/copilot/src/extension/byok/vscode-node/openRouterProvider.ts +++ b/extensions/copilot/src/extension/byok/vscode-node/openRouterProvider.ts @@ -25,11 +25,28 @@ interface OpenRouterModelData { architecture?: { input_modalities?: string[]; }; + /** + * The model's actual maximum context window, independent of which provider + * OpenRouter ranks highest. Prefer this over `top_provider.context_length`, + * which only reflects the primary provider and can be far smaller for + * multi-provider models. + * @see https://openrouter.ai/docs/guides/overview/models + */ + context_length?: number; top_provider: { context_length: number; + /** Maximum tokens the primary provider will produce in a response. */ + max_completion_tokens?: number; }; } +/** + * Fallback output-token budget used only when OpenRouter does not report + * `top_provider.max_completion_tokens` for a model. The value is heuristic — most + * tool-capable models do report an explicit budget, in which case this is unused. + */ +const DEFAULT_MAX_OUTPUT_TOKENS = 16_000; + export class OpenRouterLMProvider extends AbstractOpenAICompatibleLMProvider { public static readonly providerName = 'OpenRouter'; @@ -73,12 +90,21 @@ export class OpenRouterLMProvider extends AbstractOpenAICompatibleLMProvider { const supportsReasoningEffort = supportedParameters.includes('reasoning') || supportedParameters.includes('reasoning_effort') ? ['low', 'medium', 'high'] : undefined; + // Prefer the model-level `context_length` (the real capability) over + // `top_provider.context_length`, which only reflects OpenRouter's + // highest-ranked provider and can be much smaller for multi-provider models. + const contextWindow = openRouterModelData.context_length ?? openRouterModelData.top_provider.context_length; + // Reserve output tokens from the window. Clamp the reserve so a small-context + // model (or a missing/oversized `max_completion_tokens`) never yields a + // non-positive prompt budget. + const requestedMaxOutputTokens = openRouterModelData.top_provider.max_completion_tokens ?? DEFAULT_MAX_OUTPUT_TOKENS; + const maxOutputTokens = Math.min(requestedMaxOutputTokens, Math.floor(contextWindow / 2)); return { name: openRouterModelData.name, toolCalling: supportedParameters.includes('tools'), vision: openRouterModelData.architecture?.input_modalities?.includes('image') ?? false, - maxInputTokens: openRouterModelData.top_provider.context_length - 16000, - maxOutputTokens: 16000, + maxInputTokens: contextWindow - maxOutputTokens, + maxOutputTokens, supportsReasoningEffort }; } diff --git a/extensions/copilot/src/extension/byok/vscode-node/test/openRouterProvider.spec.ts b/extensions/copilot/src/extension/byok/vscode-node/test/openRouterProvider.spec.ts new file mode 100644 index 0000000000000..702a51d68e776 --- /dev/null +++ b/extensions/copilot/src/extension/byok/vscode-node/test/openRouterProvider.spec.ts @@ -0,0 +1,109 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { describe, expect, it, vi } from 'vitest'; +import { BYOKModelCapabilities } from '../../common/byokProvider'; +import { OpenRouterLMProvider } from '../openRouterProvider'; + +/** + * Tests for issue #324671: + * OpenRouter BYOK previously derived the context window from `top_provider.context_length`, + * which is the window of the highest-ranked provider — NOT the model's real capability + * (`context_length`). For multi-provider models this could be off by 32×. These tests + * verify the fix: the model-level `context_length` is preferred, with `top_provider` + * used only as a fallback. + */ + +/** Exposes the protected `resolveModelCapabilities` for focused testing. */ +class TestableOpenRouterLMProvider extends OpenRouterLMProvider { + public resolveCapabilities(modelData: unknown): BYOKModelCapabilities | undefined { + return this.resolveModelCapabilities(modelData); + } +} + +function createProvider(): TestableOpenRouterLMProvider { + const logService = { + trace: vi.fn(), debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), + show: vi.fn(), createSubLogger: vi.fn(), withExtraTarget: vi.fn(), + }; + logService.createSubLogger.mockReturnValue(logService); + logService.withExtraTarget.mockReturnValue(logService); + + return new TestableOpenRouterLMProvider( + { getAPIKey: vi.fn().mockResolvedValue(undefined), storeAPIKey: vi.fn(), deleteAPIKey: vi.fn() } as any, + { fetch: vi.fn() } as any, + logService as any, + { createInstance: vi.fn().mockReturnValue({}) } as any, + { isConfigured: vi.fn().mockReturnValue(false), getConfig: vi.fn(), setConfig: vi.fn() } as any, + {} as any, + ); +} + +describe('OpenRouterLMProvider context window (issue #324671)', () => { + it('derives maxInputTokens from the model-level context_length, not top_provider', () => { + const provider = createProvider(); + + // `xiaomi/mimo-v2.5`: the model supports 1M tokens, but the highest-ranked + // provider only serves 32K. The Xiaomi provider (1M) is not the top provider. + const caps = provider.resolveCapabilities({ + id: 'xiaomi/mimo-v2.5', + name: 'MiMo v2.5', + supported_parameters: ['tools'], + architecture: { input_modalities: ['text'] }, + context_length: 1048576, // actual model capability (1M) + top_provider: { context_length: 32000 }, // highest-ranked provider (32K) + }); + + // Uses the real 1M window minus the default 16K output reserve. + expect(caps?.maxInputTokens).toBe(1048576 - 16000); + expect(caps?.maxOutputTokens).toBe(16000); + }); + + it('honors top_provider.max_completion_tokens as the output budget', () => { + const provider = createProvider(); + + const caps = provider.resolveCapabilities({ + id: 'some/reasoning-model', + name: 'Reasoning Model', + supported_parameters: ['tools'], + context_length: 200000, + top_provider: { context_length: 200000, max_completion_tokens: 64000 }, + }); + + expect(caps?.maxOutputTokens).toBe(64000); + expect(caps?.maxInputTokens).toBe(200000 - 64000); + }); + + it('falls back to top_provider.context_length when the model omits context_length', () => { + const provider = createProvider(); + + const caps = provider.resolveCapabilities({ + id: 'legacy/model', + name: 'Legacy Model', + supported_parameters: ['tools'], + top_provider: { context_length: 128000 }, + }); + + expect(caps?.maxInputTokens).toBe(128000 - 16000); + }); + + it('clamps the output reserve so a small-context model keeps a positive prompt budget', () => { + const provider = createProvider(); + + // An 8K model whose provider reports a 16K completion budget larger than the + // window. Without clamping, maxInputTokens would go negative (8000 - 16000). + const caps = provider.resolveCapabilities({ + id: 'tiny/model', + name: 'Tiny Model', + supported_parameters: ['tools'], + context_length: 8000, + top_provider: { context_length: 8000, max_completion_tokens: 16000 }, + }); + + // Reserve is capped at half the window, so the prompt budget stays positive. + expect(caps?.maxOutputTokens).toBe(4000); + expect(caps?.maxInputTokens).toBe(4000); + }); +}); diff --git a/extensions/copilot/src/extension/conversation/vscode-node/terminalFixGenerator.ts b/extensions/copilot/src/extension/conversation/vscode-node/terminalFixGenerator.ts index e1bd3a65f0b7a..89d26fb1dc8dc 100644 --- a/extensions/copilot/src/extension/conversation/vscode-node/terminalFixGenerator.ts +++ b/extensions/copilot/src/extension/conversation/vscode-node/terminalFixGenerator.ts @@ -80,6 +80,12 @@ export async function generateTerminalFixes(instantiationService: IInstantiation } } r(picks); + }, err => { + // Generating terminal fixes is best-effort: a failed model request rejects here. + // Surface it as "No fixes found" instead of leaving an unhandled rejection that + // also strands the quick pick on "Generating…". + instantiationService.invokeFunction(accessor => accessor.get(ILogService)).error(err); + r([]); }); }); picksPromise.then(picks => { diff --git a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts index c633084a0daff..2acee2a253f21 100644 --- a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts +++ b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts @@ -155,17 +155,8 @@ export function isGpt55(model: LanguageModelChat | IChatEndpoint | string) { } export function isGpt56(model: LanguageModelChat | IChatEndpoint | string) { - return isGpt56SolOrTerra(model) || isGpt56Luna(model); -} - -export function isGpt56SolOrTerra(model: LanguageModelChat | IChatEndpoint | string) { - const family = typeof model === 'string' ? model : model.family; - return family === 'ember-alpha'; -} - -export function isGpt56Luna(model: LanguageModelChat | IChatEndpoint | string) { const family = typeof model === 'string' ? model : model.family; - return family === 'opal-alpha'; + return family === 'gpt-5.6-sol' || family === 'gpt-5.6-terra' || family === 'gpt-5.6-luna'; } export function isGpt53Codex(model: LanguageModelChat | IChatEndpoint | string) { @@ -176,7 +167,7 @@ export function isGpt53Codex(model: LanguageModelChat | IChatEndpoint | string) export function isKimiFamily(model: LanguageModelChat | IChatEndpoint | string): boolean { const matches = (value: string): boolean => { const normalized = value.toLowerCase(); - return normalized.startsWith('kimi-k2.6') || normalized.startsWith('kimi-k2.7-code'); + return normalized.includes('kimi-k2.6') || normalized.includes('kimi-k2.7-code'); }; if (typeof model === 'string') { diff --git a/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts b/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts index 9d9f1984a5e9c..24a28604289f6 100644 --- a/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts +++ b/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts @@ -933,7 +933,7 @@ describe('createResponsesRequestBody', () => { describe('createResponsesRequestBody prompt_cache_breakpoint markers', () => { const expectedPromptCacheBreakpoint = { mode: 'explicit' }; - const cacheBreakpointEndpoint: IChatEndpoint = { ...testEndpoint, family: 'ember-alpha' }; + const cacheBreakpointEndpoint: IChatEndpoint = { ...testEndpoint, family: 'gpt-5.6-sol' }; const cacheBreakpoint = (): Raw.ChatCompletionContentPart => ({ type: Raw.ChatCompletionContentPartKind.CacheBreakpoint, diff --git a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts index d8322ee0180b0..8b5ed39ee5ba7 100644 --- a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts +++ b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts @@ -46,6 +46,8 @@ describe('Kimi edit tool capabilities', () => { const models = { 'kimi-k2.6': fakeModel('kimi-k2.6'), 'kimi-k2.7-code': fakeModel('kimi-k2.7-code'), + 'moonshot/kimi-k2.7-code': fakeModel('moonshot/kimi-k2.7-code'), + 'moonshot/kimi-k2.6': fakeModel('moonshot/kimi-k2.6'), 'unknown-family + kimi model id': fakeModel('unknown-family', 'kimi-k2.7-code-preview'), }; const actual = Object.fromEntries(Object.entries(models).map(([name, model]) => [name, { @@ -74,6 +76,22 @@ describe('Kimi edit tool capabilities', () => { supportsApplyPatch: false, canUseApplyPatchExclusively: false, }, + 'moonshot/kimi-k2.7-code': { + isKimiFamily: true, + supportsReplaceString: true, + supportsMultiReplaceString: true, + canUseReplaceStringExclusively: true, + supportsApplyPatch: false, + canUseApplyPatchExclusively: false, + }, + 'moonshot/kimi-k2.6': { + isKimiFamily: true, + supportsReplaceString: true, + supportsMultiReplaceString: true, + canUseReplaceStringExclusively: true, + supportsApplyPatch: false, + canUseApplyPatchExclusively: false, + }, 'unknown-family + kimi model id': { isKimiFamily: true, supportsReplaceString: true, @@ -118,7 +136,7 @@ describe('modelSupportsToolSearch', () => { }); test('rejects Haiku and legacy Claude families', () => { - // Haiku is current-gen but has no tool search support — denied explicitly. + // Haiku is current-gen but has no tool search support — denied explicitly. expect(modelSupportsToolSearch('claude-haiku-4-5')).toBe(false); expect(modelSupportsToolSearch('claude-haiku-4.5')).toBe(false); expect(modelSupportsToolSearch('claude-3-5-sonnet-20241022')).toBe(false); diff --git a/extensions/markdown-language-features/package.json b/extensions/markdown-language-features/package.json index 526d60ba03234..b0b089196ca9d 100644 --- a/extensions/markdown-language-features/package.json +++ b/extensions/markdown-language-features/package.json @@ -12,7 +12,8 @@ "customEditorDiffs", "documentDiff", "documentSyntaxHighlighting", - "textEditorDiffInformation" + "textEditorDiffInformation", + "customEditorPriority" ], "engines": { "vscode": "^1.70.0" @@ -861,7 +862,11 @@ { "viewType": "vscode.markdown.preview.editor", "displayName": "Markdown Preview", - "priority": "option", + "priority": { + "diffEditor": "option", + "textEditor": "option", + "mergeEditor": "never" + }, "selector": [ { "filenamePattern": "*.md" @@ -871,7 +876,11 @@ { "viewType": "vscode.markdown.editor", "displayName": "Markdown Editor (Experimental)", - "priority": "option", + "priority": { + "diffEditor": "never", + "textEditor": "option", + "mergeEditor": "never" + }, "selector": [ { "filenamePattern": "*.md" diff --git a/src/vs/base/browser/ui/tree/abstractTree.ts b/src/vs/base/browser/ui/tree/abstractTree.ts index 0baaf62849795..5111f737d12d7 100644 --- a/src/vs/base/browser/ui/tree/abstractTree.ts +++ b/src/vs/base/browser/ui/tree/abstractTree.ts @@ -3110,6 +3110,22 @@ export abstract class AbstractTree implements IDisposable return this.view.getRelativeTop(index, stickyScrollNode?.position ?? this.stickyScrollController?.height); } + /** + * Returns the absolute top offset of an element in the tree's scroll/content + * space, or `undefined` when the element is not in the tree. Unlike + * {@link getRelativeTop}, this reads the layout height model, so it also + * resolves elements outside the rendered viewport. + */ + getElementTop(location: TRef): number | undefined { + const index = this.model.getListIndex(location); + + if (index === -1) { + return undefined; + } + + return this.view.getElementTop(index); + } + getViewState(identityProvider = this.options.identityProvider): AbstractTreeViewState { if (!identityProvider) { throw new TreeError(this._user, 'Can\'t get tree view state without an identity provider'); diff --git a/src/vs/platform/agentHost/common/meta/browserViewAttachments.ts b/src/vs/platform/agentHost/common/meta/browserViewAttachments.ts new file mode 100644 index 0000000000000..ef8d7637608b8 --- /dev/null +++ b/src/vs/platform/agentHost/common/meta/browserViewAttachments.ts @@ -0,0 +1,35 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { isString } from '../../../../base/common/types.js'; +import { MessageAttachmentKind, type MessageAttachment, type SimpleMessageAttachment } from '../state/protocol/state.js'; + +export const BrowserViewAttachmentDisplayKind = 'browser'; +export const BrowserViewAttachmentMetadataKey = 'browserView'; + +export interface IBrowserViewAttachmentMetadata { + readonly browserId: string; + readonly browserUri: string; +} + +export function isBrowserViewAttachment(attachment: MessageAttachment): attachment is SimpleMessageAttachment { + return attachment.type === MessageAttachmentKind.Simple && attachment.displayKind === BrowserViewAttachmentDisplayKind; +} + +export function getBrowserViewAttachmentMetadata(attachment: MessageAttachment): IBrowserViewAttachmentMetadata | undefined { + if (!isBrowserViewAttachment(attachment)) { + return undefined; + } + // eslint-disable-next-line local/code-no-untyped-meta-access -- sanctioned first hop into the namespaced browser view slot; validated below. + const metadata = attachment._meta?.[BrowserViewAttachmentMetadataKey]; + if (!isRecord(metadata) || !isString(metadata.browserId) || !isString(metadata.browserUri)) { + return undefined; + } + return { browserId: metadata.browserId, browserUri: metadata.browserUri }; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} diff --git a/src/vs/platform/agentHost/node/agentSdkDownloader.ts b/src/vs/platform/agentHost/node/agentSdkDownloader.ts index 55ce50b79f651..29d0686d35d7d 100644 --- a/src/vs/platform/agentHost/node/agentSdkDownloader.ts +++ b/src/vs/platform/agentHost/node/agentSdkDownloader.ts @@ -53,7 +53,7 @@ export interface IAgentSdkPackage { /** * Brand display name for user-facing progress, e.g. `'Claude'`, `'Codex'`. * The downloader puts this on {@link IAgentSdkDownloadProgress.displayName} - * so clients can build a localized "Downloading {displayName} agent…" label. + * so clients can build a localized "Downloading {displayName} agent" label. */ readonly displayName: string; /** Env var that, when set, becomes the SDK root and short-circuits the download. */ diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index c373990951ae0..d3f89f8cec58a 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -1179,8 +1179,10 @@ export class AgentService extends Disposable implements IAgentService { * * `displayName` is the provider's brand noun (e.g. `Claude`). It is woven * into the notification's localized, human-readable `message` (e.g. - * "Downloading Claude agent…") so a generic client can render the indicator - * verbatim without knowing the resource is an agent SDK. + * "Downloading Claude agent") so a generic client can render the indicator + * verbatim without knowing the resource is an agent SDK. No trailing + * ellipsis: clients render progress as ": <percent>", so an ellipsis + * would read as an unusual "…:" (see #324455). */ emitDownloadProgress(packageId: string, displayName: string, receivedBytes: number, totalBytes: number | undefined, terminal: boolean): void { const sessions = this._downloadProgressInterest.get(packageId); @@ -1192,7 +1194,7 @@ export class AgentService extends Disposable implements IAgentService { // indeterminate one where `totalBytes` was never known, plus failures — // the real error surfaces via the session-failure path). const total = terminal ? receivedBytes : totalBytes; - const message = localize('agentHost.download.agentSdkTitle', "Downloading {0} agent…", displayName); + const message = localize('agentHost.download.agentSdkTitle', "Downloading {0} agent", displayName); // `progressToken` is the download's own stable identity (the package id), // shared by every session of the provider, so the client coalesces all // frames into one indicator and dismisses it on the terminal frame. diff --git a/src/vs/platform/agentHost/node/claude/claudeAgent.ts b/src/vs/platform/agentHost/node/claude/claudeAgent.ts index 1ae1cc9302316..d44332773b7f8 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgent.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgent.ts @@ -1482,6 +1482,16 @@ export class ClaudeAgent extends Disposable implements IAgent { * and returns `[]` rather than propagating — mirrors `listSessions`. */ async getSessionMessages(session: URI): Promise<readonly Turn[]> { + // Don't trigger a cold SDK download just to reconstruct a transcript + // during restore (the renderer subscribes to the last-active session + // on startup). Mirrors `listSessions` / `getSessionMetadata`: when the + // SDK isn't local yet, defer with an empty transcript. The download + // fires (with host-level progress) once the user sends the first + // message, after which the transcript re-hydrates on the next restore. + if (!(await this._sdkService.canLoadWithoutDownload())) { + this._logService.info('[Claude] SDK not downloaded yet; deferring session messages until a session triggers the download'); + return []; + } // Additional peer chat: reconstruct its own SDK chat (resolved // from the catalog/in-memory), routed to the chat channel URI. Shares // the same fetch+map path as the default chat via `_reconstructTurns`. @@ -1623,6 +1633,16 @@ export class ClaudeAgent extends Disposable implements IAgent { * fetch and should learn that the SDK module is broken). */ async getSessionMetadata(session: URI): Promise<IAgentSessionMetadata | undefined> { + // Don't trigger a cold SDK download just to hydrate session metadata + // during restore (the renderer subscribes to the last-active session + // on startup). Mirrors `listSessions` / `getSessionMessages`: when the + // SDK isn't local yet, defer. The download fires (with host-level + // progress) once the user sends the first message, after which the + // session re-hydrates on the next restore. + if (!(await this._sdkService.canLoadWithoutDownload())) { + this._logService.info('[Claude] SDK not downloaded yet; deferring session metadata until a session triggers the download'); + return undefined; + } const sessionId = AgentSession.id(session); const sdkInfo = await this._sdkService.getSessionInfo(sessionId); if (!sdkInfo) { diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 1d3abb008824f..61a80468b7064 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -5,6 +5,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'child_process'; import * as fs from 'fs'; +import * as os from 'os'; import { CancellationError } from '../../../../base/common/errors.js'; import { Emitter } from '../../../../base/common/event.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; @@ -324,7 +325,21 @@ interface ICodexSession { */ threadId: string | undefined; readonly sessionUri: URI; - readonly workingDirectory: URI | undefined; + /** + * The directory the codex thread runs in. Usually supplied by the client + * on `createSession`, but Codex requires a cwd, so when none is provided + * (e.g. an editor window with no workspace folder open) one is lazily + * created as a managed temp folder at materialize time (tracked by + * {@link managedWorkingDirectory} for cleanup). Mutable so that lazy + * assignment can happen after the provisional `createSession`. + */ + workingDirectory: URI | undefined; + /** + * Set to the temp folder created for this session when no working + * directory was supplied, so {@link CodexAgent.disposeSession} can remove + * it. `undefined` when the client supplied a working directory. + */ + managedWorkingDirectory: URI | undefined; readonly mapState: ICodexSessionMapState; /** * Phase 4: parked deferreds for `item/commandExecution/requestApproval`, @@ -1644,9 +1659,12 @@ export class CodexAgent extends Disposable implements IAgent { if (config.fork) { throw new Error('Codex agent does not support session forking'); } - if (!config.workingDirectory) { - throw new Error('Codex requires a working directory; pass `workingDirectory` to createSession'); - } + // Codex requires a working directory to start a thread, but the client + // may not have one to give (e.g. an editor window with no workspace + // folder open). Rather than reject session creation — which would break + // both the session and the first-use SDK download progress notification + // that keys off a successful `createSession` — defer: a managed temp + // folder is created lazily at materialize time (see `_materialize`). // Provisional / lazy materialize. We DON'T call `thread/start` here // because the workbench may rebind this URI to a fresh one when the @@ -1676,6 +1694,7 @@ export class CodexAgent extends Disposable implements IAgent { threadId: undefined, sessionUri, workingDirectory: config.workingDirectory, + managedWorkingDirectory: undefined, mapState: createCodexSessionMapState(new Set(this._serverToolHost?.toolNames ?? []), clientToolSet), pendingCommandApprovals: new PendingRequestRegistry<CommandExecutionApprovalDecision>(), acceptedForSession: new Set<string>(), @@ -1745,7 +1764,14 @@ export class CodexAgent extends Disposable implements IAgent { return; } if (!session.workingDirectory) { - throw new Error(`Cannot materialize codex session ${session.sessionId}: no working directory`); + // No working directory was supplied (e.g. an editor window with no + // workspace folder open). Codex requires one, so create a managed + // per-session temp folder and remember it for cleanup on dispose. + const dir = join(os.tmpdir(), 'vscode-agent-codex', session.sessionId); + await fs.promises.mkdir(dir, { recursive: true }); + session.workingDirectory = URI.file(dir); + session.managedWorkingDirectory = session.workingDirectory; + this._logService.info(`[Codex] no working directory supplied for session=${session.sessionUri.toString()}; using managed temp folder ${dir}`); } const conn = await this._ensureConnection(); const config = this._readSessionConfig(session); @@ -2089,6 +2115,15 @@ export class CodexAgent extends Disposable implements IAgent { this._claimPrewarm(session); this._sessions.delete(sessionId); session.mcpController?.dispose(); + // Remove the managed temp folder created for a session that had no + // client-supplied working directory. Best-effort; the OS temp dir is + // reclaimed anyway, but clean up proactively so it doesn't accumulate. + if (session.managedWorkingDirectory) { + const dir = session.managedWorkingDirectory.fsPath; + fs.promises.rm(dir, { recursive: true, force: true }).catch(err => { + this._logService.info(`[Codex] failed to remove managed temp folder ${dir}: ${err instanceof Error ? err.message : String(err)}`); + }); + } if (session.threadId !== undefined) { this._sessionIdByThreadId.delete(session.threadId); } @@ -2245,6 +2280,7 @@ export class CodexAgent extends Disposable implements IAgent { threadId, sessionUri: session, workingDirectory, + managedWorkingDirectory: undefined, mapState: createCodexSessionMapState(new Set(this._serverToolHost?.toolNames ?? []), clientToolSet), pendingCommandApprovals: new PendingRequestRegistry<CommandExecutionApprovalDecision>(), acceptedForSession: new Set<string>(), diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index 50017d1a82bbd..f1cce3421d9cb 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -264,9 +264,18 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { } async canLoadWithoutDownload(): Promise<boolean> { - return true; + return this.canLoadWithoutDownloadResult; } + /** + * Programmable result for {@link canLoadWithoutDownload}. Defaults to + * `true` (SDK already local). Set to `false` to simulate the cold-start + * case where the SDK isn't downloaded yet — restore-reachable reads + * ({@link listSessions}, {@link getSessionInfo} via `getSessionMetadata`, + * {@link getSessionMessages}) MUST defer rather than trigger a download. + */ + canLoadWithoutDownloadResult = true; + /** * Fake for {@link IClaudeAgentSdkService.getSessionInfo}. Tests stage * `sessionList` and the fake searches it by id; setting @@ -3783,6 +3792,58 @@ suite('ClaudeAgent', () => { }); }); + test('restore-reachable SDK reads defer (no download) when the SDK is not yet local (preselection premature-download fix)', async () => { + // Regression: when a materialized Claude session is restored on + // startup (the renderer subscribes to the last-active session), the + // host's restore path calls `getSessionMetadata` -> `getSessionInfo` + // and `getSessionMessages`, both of which dynamically import the SDK. + // Before the fix that eagerly triggered a cold SDK download (with no + // progress interest registered, so no notification) purely from + // preselecting/restoring Claude — the download must only start on the + // first user message. `listSessions` was already guarded; this locks + // in the matching guard on the two other restore-reachable reads. + const sdk = new FakeClaudeAgentSdkService(); + sdk.canLoadWithoutDownloadResult = false; + sdk.sessionList = [ + { sessionId: 'materialized', summary: 'Materialized Session', lastModified: 5000, createdAt: 4900, cwd: '/work' }, + ]; + sdk.sessionMessagesById.set('materialized', forkSourceMessages('materialized')); + + const services = new ServiceCollection( + [ILogService, new NullLogService()], + [IAgentConfigurationService, createTestAgentConfigService(disposables)], + [ICopilotApiService, new FakeCopilotApiService()], + [IClaudeProxyService, new FakeClaudeProxyService()], + [ISessionDataService, createNullSessionDataService()], + [IClaudeAgentSdkService, sdk], + [IAgentPluginManager, new FakeAgentPluginManager()], + [IProductService, FakeProductService], + ); + const instantiationService = disposables.add(new InstantiationService(services)); + const agent = disposables.add(instantiationService.createInstance(ClaudeAgent)); + + const sessionUri = AgentSession.uri('claude', 'materialized'); + const metadata = await agent.getSessionMetadata!(sessionUri); + const messages = await agent.getSessionMessages(sessionUri); + const sessions = await agent.listSessions(); + + assert.deepStrictEqual({ + metadata, + messages, + sessions, + // The SDK must never be touched — no `getSessionInfo` / + // `getSessionMessages` calls => no dynamic import => no download. + getSessionInfoCalls: sdk.getSessionInfoCalls, + getSessionMessagesCalls: sdk.getSessionMessagesCalls, + }, { + metadata: undefined, + messages: [], + sessions: [], + getSessionInfoCalls: [], + getSessionMessagesCalls: [], + }); + }); + test('shutdown is idempotent and returns the same memoized promise on concurrent calls', async () => { // Phase 6+ INVARIANT: the SDK Query subprocess for each live // session is aborted inside `shutdown()`. If two callers race diff --git a/src/vs/sessions/AI_CUSTOMIZATIONS.md b/src/vs/sessions/AI_CUSTOMIZATIONS.md index 50c2a45a8a297..3d48e84fc7930 100644 --- a/src/vs/sessions/AI_CUSTOMIZATIONS.md +++ b/src/vs/sessions/AI_CUSTOMIZATIONS.md @@ -69,6 +69,8 @@ The management editor opens as a compact modal editor. The modal title and welco The first sidebar entry is a static `Overview` navigation item. It is styled like the other sidebar labels and does not mirror the active harness label; harness identity is represented by the modal title and welcome heading instead. +The Tools section can browse the Marketplace in the core workbench, where extension gallery browsing and installation are available. The Sessions window hides Tools Marketplace browsing and only shows the tool enablement list. + ### IAICustomizationWorkspaceService The `IAICustomizationWorkspaceService` interface controls per-window behavior: diff --git a/src/vs/sessions/contrib/promptTimeline/browser/media/promptTimeline.css b/src/vs/sessions/contrib/promptTimeline/browser/media/promptTimeline.css new file mode 100644 index 0000000000000..fdd5ab06431b7 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/media/promptTimeline.css @@ -0,0 +1,264 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +.prompt-timeline-rail { + position: absolute; + top: 0; + right: 0; + bottom: var(--prompt-timeline-bottom, 0); + width: 36px; + pointer-events: none; + z-index: 10; +} + +/* Anchors the rail overlay to the chat widget container (added/removed with the rail's lifecycle). */ +.prompt-timeline-host { + position: relative; +} + +.prompt-timeline-rail.hidden { + display: none; +} + +/* Not enough horizontal room next to the transcript: hide the marks but keep the rail laid out so its ResizeObserver can detect it fits again. */ +.prompt-timeline-rail.overflowing .prompt-timeline-ruler-marks { + display: none; +} + +/* Interactive hover/focus card: prompt text, clickable diff stat, and files. */ +.prompt-timeline-card { + position: absolute; + right: 44px; + width: 280px; + background-color: var(--vscode-editorHoverWidget-background); + color: var(--vscode-editorHoverWidget-foreground); + border: 1px solid var(--vscode-editorHoverWidget-border); + border-radius: 6px; + box-shadow: 0 2px 8px var(--vscode-widget-shadow); + pointer-events: auto; + overflow: hidden; + z-index: 20; +} + +.prompt-timeline-card.hidden { + display: none; +} + +.prompt-timeline-card-head { + padding: 8px 10px; +} + +.prompt-timeline-card-text { + font-size: 12px; + margin-bottom: 4px; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; +} + +.prompt-timeline-card-meta { + display: flex; + flex-wrap: wrap; + gap: 8px; + color: var(--vscode-descriptionForeground); + font-size: 11px; +} + +.prompt-timeline-card-diff-action { + display: inline-flex; + align-items: center; + gap: 6px; + margin-top: 6px; + margin-left: -7px; + padding: 3px 7px; + border: 1px solid transparent; + border-radius: 5px; + background: transparent; + color: var(--vscode-descriptionForeground); + font-size: 11px; + cursor: pointer; +} + +.prompt-timeline-card-diff-action:hover { + background-color: var(--vscode-list-hoverBackground); + border-color: var(--vscode-focusBorder); +} + +.prompt-timeline-card-diff-action:focus-visible { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: 1px; +} + +.prompt-timeline-card-diff-action-chevron { + color: var(--vscode-descriptionForeground); + font-size: 11px; +} + +.prompt-timeline-card-diff-action:hover .prompt-timeline-card-diff-action-chevron { + color: var(--vscode-focusBorder); +} + +.prompt-timeline-card-no-edits { + margin-top: 6px; + color: var(--vscode-descriptionForeground); + font-size: 11px; +} + +.prompt-timeline-card-stat .added, +.prompt-timeline-card-fstat .added { + color: var(--vscode-gitDecoration-addedResourceForeground, var(--vscode-charts-green)); +} + +.prompt-timeline-card-stat .removed, +.prompt-timeline-card-fstat .removed { + color: var(--vscode-gitDecoration-deletedResourceForeground, var(--vscode-charts-red)); + margin-left: 4px; +} + +.prompt-timeline-card-files { + border-top: 1px solid var(--vscode-editorHoverWidget-border); + margin-top: 8px; + max-height: 140px; + overflow-y: auto; +} + +.prompt-timeline-card-file { + display: flex; + align-items: center; + gap: 8px; + width: 100%; + padding: 4px 10px; + border: none; + background: transparent; + color: inherit; + cursor: pointer; + font-size: 11px; + text-align: left; +} + +.prompt-timeline-card-file:hover { + background-color: var(--vscode-list-hoverBackground); +} + +.prompt-timeline-card-file:focus-visible { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: -1px; +} + +.prompt-timeline-card-fname { + flex: 1; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} + +.prompt-timeline-card-fstat { + font-variant-numeric: tabular-nums; +} + +/* Overview-ruler style: the session compressed into the rail height like the editor overview ruler (proportional marks + the scrollbar slider as a you-are-here thumb). */ +.prompt-timeline-ruler-marks { + position: absolute; + inset: 0; + pointer-events: none; +} + +/* Each mark is a >=24px hit target positioned at its proportional scroll offset. */ +.prompt-timeline-ruler-mark { + position: absolute; + right: 0; + width: 22px; + height: 24px; + display: flex; + align-items: center; + justify-content: flex-end; + padding-right: 4px; + border: none; + background: transparent; + cursor: pointer; + pointer-events: auto; +} + +.prompt-timeline-ruler-mark.hidden { + display: none; +} + +/* The visible mark: a short bar, neutral unless the turn changed code. */ +.prompt-timeline-ruler-bar { + display: flex; + overflow: hidden; + width: 12px; + height: 2px; + border-radius: 1px; + background-color: var(--vscode-scrollbarSlider-background); + transition: width 80ms ease, height 80ms ease, background-color 80ms ease; +} + +/* Two-tone edited signal: a green added segment and a red removed segment, sized by the turn's split. */ +.prompt-timeline-ruler-bar .seg-add, +.prompt-timeline-ruler-bar .seg-del { + height: 100%; + min-width: 1px; +} + +.prompt-timeline-ruler-bar.edited .seg-add { + background-color: var(--vscode-editorOverviewRuler-addedForeground); +} + +.prompt-timeline-ruler-bar.edited .seg-del { + background-color: var(--vscode-editorOverviewRuler-deletedForeground); +} + +.prompt-timeline-ruler-mark:hover .prompt-timeline-ruler-bar { + width: 16px; + height: 3px; + background-color: var(--vscode-scrollbarSlider-hoverBackground); +} + +/* Active = the prompt currently in view. */ +.prompt-timeline-ruler-mark.active .prompt-timeline-ruler-bar { + width: 18px; + height: 3px; + background-color: var(--vscode-focusBorder); +} + +.prompt-timeline-ruler-mark.active .prompt-timeline-ruler-bar .seg-add, +.prompt-timeline-ruler-mark.active .prompt-timeline-ruler-bar .seg-del { + background-color: transparent; +} + +.prompt-timeline-rail .prompt-timeline-ruler-mark:focus { + outline: none; +} + +.prompt-timeline-rail .prompt-timeline-ruler-mark:focus-visible .prompt-timeline-ruler-bar { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: 2px; +} + +/* The you-are-here thumb reuses the scrollbar slider. */ +.prompt-timeline-ruler-thumb { + position: absolute; + right: 2px; + width: 6px; + border-radius: 3px; + background-color: var(--vscode-scrollbarSlider-background); + pointer-events: none; +} + +.prompt-timeline-ruler-thumb.hidden { + display: none; +} + +.prompt-timeline-rail-ruler:hover .prompt-timeline-ruler-thumb { + background-color: var(--vscode-scrollbarSlider-hoverBackground); +} + +@media (prefers-reduced-motion: reduce) { + .prompt-timeline-ruler-bar { + transition: none; + } +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptBucketing.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptBucketing.ts new file mode 100644 index 0000000000000..2813ba2986cd9 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptBucketing.ts @@ -0,0 +1,169 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +/** A user prompt that can be represented on the prompt timeline. */ +export interface PromptItem { + /** Stable id of the user request (chat request view model id). */ + readonly requestId: string; + /** Preview text of the prompt (already single-line/plain is fine). */ + readonly text: string; + /** Creation time in ms since epoch. */ + readonly timestamp: number; +} + +/** A timeline tick representing one or more chronological prompts. */ +export interface PromptBucket { + /** Representative prompt (the FIRST prompt in the bucket) — the jump target. */ + readonly prompt: PromptItem; + /** All prompts grouped into this bucket, in chronological order. */ + readonly prompts: readonly PromptItem[]; + /** How many prompts this tick represents (== prompts.length). */ + readonly count: number; +} + +/** Hard cap on the number of ticks produced. */ +export const MAX_TICKS = 24; + +const oneDayMs = 86400000; + +function bucketKey(date: Date, now: Date): string { + const startOfToday = new Date(now.getFullYear(), now.getMonth(), now.getDate()); + + if (date >= startOfToday) { + return `today-${date.getTime()}`; + } + + const daysAgo = (startOfToday.getTime() - date.getTime()) / oneDayMs; + if (daysAgo < 1) { + return `yesterday-h${date.getHours()}`; + } + + if (daysAgo < 30) { + return `day-${date.getFullYear()}-${date.getMonth()}-${date.getDate()}`; + } + + return `month-${date.getFullYear()}-${date.getMonth()}`; +} + +function toBucket(prompt: PromptItem, prompts: readonly PromptItem[] = [prompt]): PromptBucket { + return { + prompt, + prompts, + count: prompts.length + }; +} + +function bucketPrompts(prompts: readonly PromptItem[], now: Date): PromptBucket[] { + const buckets: PromptBucket[] = []; + let currentKey: string | undefined; + + for (const prompt of prompts) { + const key = bucketKey(new Date(prompt.timestamp), now); + const current = buckets[buckets.length - 1]; + + if (!current || key !== currentKey) { + buckets.push(toBucket(prompt)); + currentKey = key; + } else { + const groupedPrompts = [...current.prompts, prompt]; + buckets[buckets.length - 1] = toBucket(current.prompt, groupedPrompts); + } + } + + return buckets; +} + +function uniformSample(buckets: PromptBucket[], maxTicks: number): PromptBucket[] { + if (buckets.length <= maxTicks) { + return buckets; + } + + const first = buckets[0]; + const last = buckets[buckets.length - 1]; + + if (maxTicks <= 2) { + return [first, last]; + } + + const sampled: PromptBucket[] = [first]; + const step = (buckets.length - 1) / (maxTicks - 1); + for (let i = 1; i <= maxTicks - 2; i++) { + const index = Math.round(i * step); + if (index > 0 && index < buckets.length - 1) { + sampled.push(buckets[index]); + } + } + sampled.push(last); + + return sampled; +} + +function expandBucket(bucket: PromptBucket, budget: number): PromptBucket[] { + if (bucket.count <= budget + 1) { + return bucket.prompts.map(prompt => toBucket(prompt)); + } + + const expandedCount = budget; + const remainder = bucket.prompts.slice(0, bucket.count - expandedCount); + const expandedPrompts = bucket.prompts.slice(bucket.count - expandedCount); + + return [ + toBucket(remainder[0], remainder), + ...expandedPrompts.map(prompt => toBucket(prompt)) + ]; +} + +function expandRecentBuckets(buckets: PromptBucket[], maxTicks: number): PromptBucket[] { + if (buckets.length >= maxTicks) { + return buckets; + } + + let expanded = buckets; + let total = buckets.length; + + for (let i = expanded.length - 1; i >= 0 && total < maxTicks; i--) { + const bucket = expanded[i]; + if (bucket.count <= 1) { + continue; + } + + const replacement = expandBucket(bucket, maxTicks - total); + total += replacement.length - 1; + expanded = [ + ...expanded.slice(0, i), + ...replacement, + ...expanded.slice(i + 1) + ]; + } + + return expanded; +} + +/** + * Compresses chronological prompts into a bounded set of recency-aware timeline ticks. + * Pass `now` in tests to make time-based grouping deterministic. `maxTicks` caps the + * result (defaults to {@link MAX_TICKS}); callers lower it to fit the available height. + */ +export function budgetBucketPrompts(prompts: readonly PromptItem[], now = Date.now(), maxTicks = MAX_TICKS): PromptBucket[] { + const cap = Math.min(MAX_TICKS, Math.max(1, maxTicks)); + const buckets = bucketPrompts(prompts, new Date(now)); + if (buckets.length > cap) { + return uniformSample(buckets, cap); + } + + if (buckets.length < cap) { + return expandRecentBuckets(buckets, cap); + } + + return buckets; +} + +/** Internal helpers exposed only for focused unit tests. */ +export const _testing = { + bucketKey, + bucketPrompts, + uniformSample, + expandRecentBuckets +} as const; diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimeline.contribution.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimeline.contribution.ts new file mode 100644 index 0000000000000..7dcefcfa206f6 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimeline.contribution.ts @@ -0,0 +1,28 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { localize } from '../../../../nls.js'; +import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from '../../../../platform/configuration/common/configurationRegistry.js'; +import { Registry } from '../../../../platform/registry/common/platform.js'; +import { ChatWidget } from '../../../../workbench/contrib/chat/browser/widget/chatWidget.js'; +import { PROMPT_TIMELINE_ENABLED_SETTING } from '../common/promptTimeline.js'; +import { registerPromptTimelineActions } from './promptTimelineActions.js'; +import { PromptTimelineWidgetContrib } from './promptTimelineWidgetContrib.js'; + +Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration).registerConfiguration({ + id: 'sessions', + properties: { + [PROMPT_TIMELINE_ENABLED_SETTING]: { + type: 'boolean', + default: false, + description: localize('sessions.promptTimeline.enabled', "Controls whether the prompt timeline rail is shown alongside the chat transcript in the Agents window. The rail lets you scan and jump between the prompts you have sent."), + tags: ['experimental'], + experiment: { mode: 'startup' }, + }, + }, +}); + +ChatWidget.CONTRIBS.push(PromptTimelineWidgetContrib); +registerPromptTimelineActions(); diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineActions.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineActions.ts new file mode 100644 index 0000000000000..c0a5c6727ed9d --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineActions.ts @@ -0,0 +1,84 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { localize, localize2 } from '../../../../nls.js'; +import { Action2, registerAction2 } from '../../../../platform/actions/common/actions.js'; +import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; +import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; +import { IQuickInputService, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; +import { IChatWidgetService } from '../../../../workbench/contrib/chat/browser/chat.js'; +import { ChatContextKeys } from '../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; +import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; +import { PromptTimelineCommandId, PROMPT_TIMELINE_ENABLED_SETTING } from '../common/promptTimeline.js'; +import { PromptTimelineWidgetContrib } from './promptTimelineWidgetContrib.js'; + +const CATEGORY = localize2('promptTimeline.category', "Chat"); + +/** True unless the prompt timeline setting is explicitly disabled. */ +const TIMELINE_ENABLED = ContextKeyExpr.notEquals(`config.${PROMPT_TIMELINE_ENABLED_SETTING}`, false); + +/** Commands require AI features to be on and the prompt timeline setting to be enabled. */ +const TIMELINE_PRECONDITION = ContextKeyExpr.and(ChatContextKeys.enabled, TIMELINE_ENABLED); + +/** Resolves the prompt timeline contribution for the active session's chat widget. */ +function getPromptTimeline(accessor: ServicesAccessor): PromptTimelineWidgetContrib | undefined { + const widgetService = accessor.get(IChatWidgetService); + const sessionsService = accessor.get(ISessionsService); + const resource = sessionsService.activeSession.get()?.activeChat.get().resource; + const widget = (resource && widgetService.getWidgetBySessionResource(resource)) ?? widgetService.lastFocusedWidget; + return widget?.getContrib<PromptTimelineWidgetContrib>(PromptTimelineWidgetContrib.ID); +} + +interface IPromptPickItem extends IQuickPickItem { + readonly requestId: string; +} + +function formatStat(tick: { stat?: { added: number; removed: number; fileCount: number } }): string | undefined { + if (!tick.stat) { + return undefined; + } + const files = tick.stat.fileCount === 1 + ? localize('promptTimeline.oneFile', "1 file") + : localize('promptTimeline.nFiles', "{0} files", tick.stat.fileCount); + return localize('promptTimeline.statDetail', "+{0} \u2212{1} · {2}", tick.stat.added, tick.stat.removed, files); +} + +class GoToPromptAction extends Action2 { + constructor() { + super({ + id: PromptTimelineCommandId.GoToPrompt, + title: localize2('promptTimeline.goToPrompt', "Go to Prompt..."), + category: CATEGORY, + f1: true, + precondition: TIMELINE_PRECONDITION, + }); + } + override async run(accessor: ServicesAccessor): Promise<void> { + const contrib = getPromptTimeline(accessor); + const prompts = contrib?.getAllPrompts() ?? []; + if (!contrib || prompts.length === 0) { + return; + } + + const quickInputService = accessor.get(IQuickInputService); + const items: IPromptPickItem[] = prompts.map(prompt => ({ + label: prompt.text || localize('promptTimeline.emptyPrompt', "(empty prompt)"), + description: formatStat(prompt), + requestId: prompt.requestId, + })); + + const picked = await quickInputService.pick(items, { + placeHolder: localize('promptTimeline.pickPlaceholder', "Go to a prompt in this chat"), + matchOnDescription: true, + }); + if (picked) { + contrib.reveal(picked.requestId); + } + } +} + +export function registerPromptTimelineActions(): void { + registerAction2(GoToPromptAction); +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineCard.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineCard.ts new file mode 100644 index 0000000000000..e789038c7d79c --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineCard.ts @@ -0,0 +1,132 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { $, addDisposableListener, append, clearNode, EventType } from '../../../../base/browser/dom.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; +import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; +import { localize } from '../../../../nls.js'; +import { IPromptReviewFileEvent } from './promptTimelineRail.js'; +import { PromptFileDiff, PromptTick } from './promptTimelineModel.js'; + +/** + * The interactive preview shown when a prompt mark is hovered or focused, shared + * by every rail style. Renders the prompt text, its diff summary (a shortcut to + * review the whole prompt) and per-file rows, and stays open while hovered. + */ +export class PromptTimelineCard extends Disposable { + + private readonly _element: HTMLElement; + private readonly _contentDisposables = this._register(new DisposableStore()); + private _hovered = false; + private _hideTimer: ReturnType<typeof setTimeout> | undefined; + private _filesProvider: (tick: PromptTick) => readonly PromptFileDiff[] = () => []; + + private readonly _onDidReview = this._register(new Emitter<PromptTick>()); + readonly onDidReview: Event<PromptTick> = this._onDidReview.event; + + private readonly _onDidReviewFile = this._register(new Emitter<IPromptReviewFileEvent>()); + readonly onDidReviewFile: Event<IPromptReviewFileEvent> = this._onDidReviewFile.event; + + constructor(private readonly _container: HTMLElement) { + super(); + this._element = append(this._container, $('.prompt-timeline-card')); + this._element.classList.add('hidden'); + this._register(addDisposableListener(this._element, EventType.MOUSE_ENTER, () => { this._hovered = true; })); + this._register(addDisposableListener(this._element, EventType.MOUSE_LEAVE, () => { this._hovered = false; this.scheduleHide(); })); + } + + setFilesProvider(provider: (tick: PromptTick) => readonly PromptFileDiff[]): void { + this._filesProvider = provider; + } + + /** Builds the card for a tick and positions it centered on `anchorCenterY` (relative to the container). */ + show(tick: PromptTick, anchorCenterY: number): void { + if (this._hideTimer !== undefined) { + clearTimeout(this._hideTimer); + this._hideTimer = undefined; + } + this._contentDisposables.clear(); + clearNode(this._element); + + const head = append(this._element, $('.prompt-timeline-card-head')); + append(head, $('.prompt-timeline-card-text')).textContent = tick.text; + // Grouped ticks show how many prompts they cover. No absolute time: agent-host + // sessions don't record per-turn timestamps, so it would be misleading. + if (tick.count > 1) { + append(head, $('.prompt-timeline-card-meta')).textContent = localize('promptTimeline.groupedCount', "{0} prompts", tick.count); + } + + const files = tick.stat ? this._filesProvider(tick) : []; + if (tick.stat) { + const diffAction = append(head, $<HTMLButtonElement>('button.prompt-timeline-card-diff-action')); + diffAction.setAttribute('aria-label', localize( + 'promptTimeline.reviewChangesForPrompt', + "Review Changes for Prompt: {0}", + tick.text, + )); + this._renderStat(append(diffAction, $('span.prompt-timeline-card-stat')), tick.stat.added, tick.stat.removed); + append(diffAction, $('span')).textContent = tick.stat.fileCount === 1 + ? localize('promptTimeline.oneFile', "1 file") + : localize('promptTimeline.nFiles', "{0} files", tick.stat.fileCount); + append(diffAction, $('span.prompt-timeline-card-diff-action-chevron')).textContent = '\u203A'; + this._contentDisposables.add(addDisposableListener(diffAction, EventType.CLICK, () => { + this._onDidReview.fire(tick); + this.hide(); + })); + } else { + append(head, $('div.prompt-timeline-card-no-edits')).textContent = localize('promptTimeline.noEdits', "no edits"); + } + + if (files.length > 0) { + const list = append(this._element, $('.prompt-timeline-card-files')); + for (const file of files) { + const row = append(list, $<HTMLButtonElement>('button.prompt-timeline-card-file')); + row.title = file.name; + append(row, $('.prompt-timeline-card-fname')).textContent = file.name; + this._renderStat(append(row, $('.prompt-timeline-card-fstat')), file.added, file.removed); + this._contentDisposables.add(addDisposableListener(row, EventType.CLICK, () => { + this._onDidReviewFile.fire({ tick, file: file.modifiedURI }); + this.hide(); + })); + } + } + + this._element.classList.remove('hidden'); + const top = anchorCenterY - this._element.offsetHeight / 2; + const clampedTop = Math.max(4, Math.min(top, this._container.clientHeight - this._element.offsetHeight - 4)); + this._element.style.top = `${clampedTop}px`; + } + + private _renderStat(container: HTMLElement, added: number, removed: number): void { + append(container, $('span.added')).textContent = `+${added}`; + append(container, $('span.removed')).textContent = `\u2212${removed}`; + } + + /** Hides the card shortly, unless it (or a mark) is re-hovered first. */ + scheduleHide(): void { + if (this._hideTimer !== undefined) { + clearTimeout(this._hideTimer); + } + this._hideTimer = setTimeout(() => { + this._hideTimer = undefined; + if (!this._hovered) { + this.hide(); + } + }, 200); + } + + hide(): void { + this._hovered = false; + this._contentDisposables.clear(); + this._element.classList.add('hidden'); + } + + override dispose(): void { + if (this._hideTimer !== undefined) { + clearTimeout(this._hideTimer); + } + super.dispose(); + } +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineModel.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineModel.ts new file mode 100644 index 0000000000000..c5dae37168be3 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineModel.ts @@ -0,0 +1,449 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { localize } from '../../../../nls.js'; +import { Disposable, MutableDisposable } from '../../../../base/common/lifecycle.js'; +import { autorun, derived, IObservable, IObservableSignal, IReader, ISettableObservable, observableFromEvent, observableSignal, observableValue, transaction } from '../../../../base/common/observable.js'; +import { URI } from '../../../../base/common/uri.js'; +import { basename, isEqual } from '../../../../base/common/resources.js'; +import { generateUuid } from '../../../../base/common/uuid.js'; +import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; +import { IFileService } from '../../../../platform/files/common/files.js'; +import { IEditorService } from '../../../../workbench/services/editor/common/editorService.js'; +import { MultiDiffEditorInput } from '../../../../workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.js'; +import { MultiDiffEditorItem } from '../../../../workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.js'; +import { IMultiDiffEditorOptions } from '../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.js'; +import { ChatWidget } from '../../../../workbench/contrib/chat/browser/widget/chatWidget.js'; +import { IChatResponseFileChangesService } from '../../../../workbench/contrib/chat/browser/chatResponseFileChangesService.js'; +import { IChatEditingService, IEditSessionEntryDiff } from '../../../../workbench/contrib/chat/common/editing/chatEditingService.js'; +import { isRequestVM } from '../../../../workbench/contrib/chat/common/model/chatViewModel.js'; +import { budgetBucketPrompts, MAX_TICKS, PromptItem } from './promptBucketing.js'; + +/** Aggregated diff stats for the edits a prompt (or bucket) produced. */ +export interface PromptDiffStat { + readonly added: number; + readonly removed: number; + readonly fileCount: number; +} + +/** A single file changed by a prompt, used by the hover card / diff drill-down. */ +export interface PromptFileDiff { + readonly name: string; + readonly originalURI: URI; + /** File identity / go-to-file target (may be the live working file). */ + readonly modifiedURI: URI; + /** RHS content the diff should render; the frozen after-turn snapshot when available. */ + readonly diffModifiedURI: URI; + readonly added: number; + readonly removed: number; +} + +/** Content-space layout used by the overview-ruler rail to place marks + the viewport thumb. */ +export interface IPromptScrollLayout { + /** Each prompt's top offset in transcript content pixels. */ + readonly marks: readonly { readonly requestId: string; readonly top: number }[]; + /** Total transcript content height in pixels. */ + readonly total: number; + /** Current scroll offset in content pixels. */ + readonly scrollTop: number; +} + +/** A single tick shown on the prompt timeline rail. */ +export interface PromptTick { + /** Jump target: the request id of the first prompt in the bucket. */ + readonly requestId: string; + /** Request ids of every prompt this tick represents (for active tracking). */ + readonly allRequestIds: readonly string[]; + /** Preview text (first prompt in the bucket). */ + readonly text: string; + /** Creation time (ms since epoch) of the first prompt in the bucket. */ + readonly timestamp: number; + /** How many prompts this tick represents. */ + readonly count: number; + /** Accessible label announced for the tick. */ + readonly ariaLabel: string; + /** Diff summary of the edits this tick produced, if any. */ + readonly stat?: PromptDiffStat; +} + +const MAX_PREVIEW_LENGTH = 80; + +/** First non-empty line of a prompt, trimmed and length-capped for previews. */ +function getPromptPreview(text: string): string { + const firstLine = text.split('\n').map(l => l.trim()).find(l => l.length > 0) ?? ''; + return firstLine.length <= MAX_PREVIEW_LENGTH ? firstLine : `${firstLine.slice(0, MAX_PREVIEW_LENGTH)}…`; +} + +/** Whether two derived prompt lists are equivalent (order, id, text and time). */ +function promptsEqual(a: readonly PromptItem[], b: readonly PromptItem[]): boolean { + return a.length === b.length && a.every((p, i) => + p.requestId === b[i].requestId && p.text === b[i].text && p.timestamp === b[i].timestamp); +} + +/** A user prompt entry (used by the keyboard "Go to Prompt" picker, independent of rail density). */ +export interface PromptEntry { + readonly requestId: string; + readonly text: string; + readonly timestamp: number; + readonly stat?: PromptDiffStat; +} + +/** + * Derives the prompt timeline (bucketed ticks + the active tick) from a chat + * widget's view model, and reveals prompts on request. + */ +export class PromptTimelineModel extends Disposable { + + /** All user prompts in the chat, updated as the transcript changes. */ + private readonly _prompts: ISettableObservable<readonly PromptItem[]> = observableValue<readonly PromptItem[]>(this, []); + + /** The chat session resource, tracked reactively so the editing session can be resolved. */ + private readonly _sessionResource: IObservable<URI | undefined>; + + /** The chat editing session for this chat, if one exists (local or agent-host). */ + private readonly _editingSession = derived(this, reader => { + const resource = this._sessionResource.read(reader); + if (!resource) { + return undefined; + } + return this.chatEditingService.editingSessionsObs.read(reader).find(s => isEqual(s.chatSessionResource, resource)); + }); + + /** Recency-bucketed ticks, capped to a fixed maximum so each keeps a >=24px slot. */ + private readonly _baseTicks = derived<readonly PromptTick[]>(this, reader => { + const prompts = this._prompts.read(reader); + return budgetBucketPrompts(prompts, Date.now(), MAX_TICKS).map((bucket): PromptTick => ({ + requestId: bucket.prompt.requestId, + allRequestIds: bucket.prompts.map(p => p.requestId), + text: bucket.prompt.text, + timestamp: bucket.prompt.timestamp, + count: bucket.count, + ariaLabel: bucket.count === 1 + ? localize('promptTimeline.tick', "Prompt: {0}", bucket.prompt.text) + : localize('promptTimeline.tickGrouped', "{0} prompts starting with: {1}", bucket.count, bucket.prompt.text), + })); + }); + + /** Ticks decorated with per-prompt diff stats (server per-turn changeset, else editing session). */ + private readonly _ticks = derived<readonly PromptTick[]>(this, reader => { + const base = this._baseTicks.read(reader); + return base.map(tick => { + const stat = this._statForRequests(tick.allRequestIds, reader); + return stat ? { ...tick, stat } : tick; + }); + }); + get ticks(): IObservable<readonly PromptTick[]> { return this._ticks; } + + private readonly _activeRequestId: ISettableObservable<string | undefined> = observableValue<string | undefined>(this, undefined); + get activeRequestId(): IObservable<string | undefined> { return this._activeRequestId; } + + /** Fires when the transcript scroll offset or content height changes (drives the ruler rail). */ + private readonly _scrollLayoutSignal: IObservableSignal<void> = observableSignal<void>(this); + get onDidChangeScrollLayout(): IObservable<void> { return this._scrollLayoutSignal; } + + private readonly _viewModelListener = this._register(new MutableDisposable()); + + constructor( + private readonly widget: ChatWidget, + @IChatEditingService private readonly chatEditingService: IChatEditingService, + @IChatResponseFileChangesService private readonly chatResponseFileChangesService: IChatResponseFileChangesService, + @IEditorService private readonly editorService: IEditorService, + @IInstantiationService private readonly instantiationService: IInstantiationService, + @IFileService private readonly fileService: IFileService, + ) { + super(); + // Assigned here (not as a field initializer) because it reads `this.widget`, + // which parameter properties only assign once the constructor body runs. + this._sessionResource = observableFromEvent(this, this.widget.onDidChangeViewModel, () => this.widget.viewModel?.sessionResource); + this._register(this.widget.onDidChangeViewModel(() => this._bindViewModel())); + this._register(this.widget.onDidScroll(() => { this._updateActive(); this._triggerScrollLayout(); })); + this._register(this.widget.onDidChangeContentHeight(() => this._triggerScrollLayout())); + // Re-evaluate the active tick whenever the ticks change. + this._register(autorun(reader => { + this._baseTicks.read(reader); + this._updateActive(); + this._triggerScrollLayout(); + })); + this._bindViewModel(); + } + + private _triggerScrollLayout(): void { + transaction(tx => this._scrollLayoutSignal.trigger(tx)); + } + + /** + * The prompts' positions in transcript content space, for the overview-ruler + * rail. Each prompt's top comes from the chat list's layout height model + * (`ChatWidget.getElementTop`), the same virtualization-safe source the + * scrollbar uses, so off-screen prompts get correct positions without waiting + * for their rows to render. Marks, `total`, and `scrollTop` all share the + * list's content-pixel space, so the viewport thumb stays aligned. + */ + getScrollLayout(): IPromptScrollLayout | undefined { + const items = this.widget.viewModel?.getItems(); + if (!items) { + return undefined; + } + const marks: { requestId: string; top: number }[] = []; + for (const item of items) { + if (isRequestVM(item)) { + const top = this.widget.getElementTop(item); + if (top !== undefined) { + marks.push({ requestId: item.id, top }); + } + } + } + return { marks, total: this.widget.scrollHeight, scrollTop: this.widget.scrollTop }; + } + + private _bindViewModel(): void { + this._viewModelListener.value = this.widget.viewModel?.onDidChange(() => this._recompute()); + this._recompute(); + } + + private _recompute(): void { + const prompts: PromptItem[] = []; + for (const item of this.widget.viewModel?.getItems() ?? []) { + if (isRequestVM(item)) { + prompts.push({ requestId: item.id, text: getPromptPreview(item.messageText), timestamp: item.timestamp }); + } + } + + // Streaming fires onDidChange for every token; only rebuild ticks when the + // set of prompts actually changed. Rendered heights still shift, so refresh + // the active tick either way. + if (promptsEqual(prompts, this._prompts.get())) { + this._updateActive(); + return; + } + this._prompts.set(prompts, undefined); + } + + /** Recomputes which tick maps to the prompt currently scrolled into view. */ + private _updateActive(): void { + const ticks = this._baseTicks.get(); + const items = this.widget.viewModel?.getItems(); + if (!items || ticks.length === 0) { + this._activeRequestId.set(undefined, undefined); + return; + } + + // The active prompt is the last request whose top edge is at or above the + // viewport top. Positions come from the list's layout height model, so + // off-screen prompts resolve correctly (not just rendered ones). + const scrollTop = this.widget.scrollTop; + const threshold = 24; + let activeRequestId: string | undefined; + let activeTimestamp = 0; + for (const item of items) { + if (isRequestVM(item)) { + const top = this.widget.getElementTop(item); + if (top !== undefined && top <= scrollTop + threshold) { + activeRequestId = item.id; + activeTimestamp = item.timestamp; + } + } + } + + if (activeRequestId === undefined) { + // Scrolled above the oldest prompt: the oldest tick is the active one + // (the loop advances oldest -> newest as you scroll down). + this._activeRequestId.set(ticks.at(0)?.requestId, undefined); + return; + } + + let activeTick = ticks.find(t => t.allRequestIds.includes(activeRequestId!)); + if (!activeTick) { + // The active prompt's bucket may have been sampled away; fall back to the + // nearest surviving tick at or before it (ticks are chronological). + for (const tick of ticks) { + if (tick.timestamp <= activeTimestamp) { + activeTick = tick; + } else { + break; + } + } + } + this._activeRequestId.set((activeTick ?? ticks[ticks.length - 1]).requestId, undefined); + } + + /** Reveals the request with the given id near the top of the transcript. */ + reveal(requestId: string): void { + const item = this.widget.viewModel?.getItems().find(i => isRequestVM(i) && i.id === requestId); + if (item) { + this.widget.reveal(item, 0); + } + // Normalize to the owning tick's representative id so the active highlight + // works even when the id is a mid-bucket prompt (picker). + const owningTick = this._baseTicks.get().find(t => t.allRequestIds.includes(requestId)); + this._activeRequestId.set(owningTick?.requestId ?? requestId, undefined); + } + + /** The changed files for a tick's prompts, aggregated per file (for the hover card / drill-down). */ + getRequestFiles(tick: PromptTick): readonly PromptFileDiff[] { + const byPath = new Map<string, PromptFileDiff>(); + for (const requestId of tick.allRequestIds) { + for (const diff of this._diffsForRequest(requestId)) { + if (diff.identical) { + continue; + } + const key = diff.modifiedURI.toString(); + const existing = byPath.get(key); + if (existing) { + // Grouped tick, same file across prompts: the prompts are + // chronological, so keep the earliest `originalURI` (before) but + // advance `diffModifiedURI` to this later prompt's after-snapshot + // so the opened diff spans the whole tick, not just the first edit. + byPath.set(key, { + ...existing, + diffModifiedURI: diff.modifiedSnapshotURI ?? diff.modifiedURI, + added: existing.added + diff.added, + removed: existing.removed + diff.removed, + }); + } else { + byPath.set(key, { + name: basename(diff.modifiedURI), + originalURI: diff.originalURI, + modifiedURI: diff.modifiedURI, + diffModifiedURI: diff.modifiedSnapshotURI ?? diff.modifiedURI, + added: diff.added, + removed: diff.removed, + }); + } + } + } + return [...byPath.values()]; + } + + /** + * Opens the per-prompt changes as a multi-file diff. When a specific file is + * given (a file row in the card), the same multi-diff is opened but revealed + * at that file, so per-file and whole-prompt review share one experience. + */ + async reviewChanges(tick: PromptTick, file?: URI): Promise<void> { + const files = this.getRequestFiles(tick); + if (files.length === 0) { + return; + } + const items: MultiDiffEditorItem[] = []; + let revealResource: { original: URI | undefined; modified: URI | undefined } | undefined; + for (const f of files) { + const [originalURI, modifiedURI] = await this._readableSides(f); + if (!originalURI && !modifiedURI) { + continue; + } + // Diff the best-available before/after content, but let "go to file" open the live file. + items.push(new MultiDiffEditorItem(originalURI, modifiedURI, f.modifiedURI)); + if (file && isEqual(f.modifiedURI, file)) { + revealResource = { original: originalURI, modified: modifiedURI }; + } + } + if (items.length === 0) { + return; + } + const source = URI.parse(`multi-diff-editor:prompt-timeline/${generateUuid()}`); + const input = this.instantiationService.createInstance( + MultiDiffEditorInput, + source, + localize('promptTimeline.reviewTitle', "Changes · {0}", tick.text), + items, + false, + ); + const options: IMultiDiffEditorOptions | undefined = revealResource + ? { viewState: { revealData: { resource: revealResource } } } + : undefined; + await this.editorService.openEditor(input, options); + } + + /** + * Resolves which sides of a file diff can actually be read. Prefers the frozen + * before/after snapshots so only this turn's changes show, but the agent-host + * checkpoint blobs backing them can be missing (an added file's original, or a + * pruned/restored session where whole checkpoints are gone). The modified side + * then falls back to the live working file so review still opens with the best + * available fidelity; an unreadable side is dropped so the file still renders + * as a pure add/delete instead of crashing the diff editor. + */ + private async _readableSides(file: PromptFileDiff): Promise<[URI | undefined, URI | undefined]> { + // The provider sets originalURI === modifiedURI when there is no "before" + // (a created file); treat that as no frozen original. + const hasFrozenOriginal = !isEqual(file.originalURI, file.modifiedURI); + const hasFrozenModified = !isEqual(file.diffModifiedURI, file.modifiedURI); + const [frozenOriginalReadable, frozenModifiedReadable, liveModifiedReadable] = await Promise.all([ + hasFrozenOriginal ? this._canRead(file.originalURI) : Promise.resolve(false), + hasFrozenModified ? this._canRead(file.diffModifiedURI) : Promise.resolve(false), + this._canRead(file.modifiedURI), + ]); + const modified = frozenModifiedReadable ? file.diffModifiedURI + : liveModifiedReadable ? file.modifiedURI + : undefined; + return [frozenOriginalReadable ? file.originalURI : undefined, modified]; + } + + private async _canRead(resource: URI): Promise<boolean> { + // Agent-host git-blob URIs always `stat` successfully even when the blob + // is missing, so probe with an actual read to detect unreadable sides. + // Read a single byte: enough to surface a not-found error without pulling + // whole (potentially large) file contents just to test availability. + try { + await this.fileService.readFile(resource, { length: 1 }); + return true; + } catch { + return false; + } + } + + /** + * All user prompts (with diff stats where available) for the picker, + * independent of the rail's bucketing. Stats are resolved one-shot, so + * agent-host prompts not currently observed by the rail fall back to their + * timestamp in the picker rather than holding a subscription per prompt. + */ + getAllPrompts(): readonly PromptEntry[] { + return this._prompts.get().map(prompt => { + const stat = this._statForRequests([prompt.requestId]); + return stat ? { ...prompt, stat } : { ...prompt }; + }); + } + + /** + * Per-request file diffs, preferring the session type's authoritative + * provider (agent-host sessions expose a server-computed per-turn changeset + * that survives reload), and falling back to the chat editing session. + */ + private _diffsForRequest(requestId: string, reader?: IReader): readonly IEditSessionEntryDiff[] { + const resource = reader ? this._sessionResource.read(reader) : this._sessionResource.get(); + if (resource) { + const provided = this.chatResponseFileChangesService.getChangesForRequest(resource, requestId); + if (provided) { + return reader ? provided.read(reader) : provided.get(); + } + } + const session = reader ? this._editingSession.read(reader) : this._editingSession.get(); + if (session) { + const obs = session.getDiffsForFilesInRequest(requestId); + return reader ? obs.read(reader) : obs.get(); + } + return []; + } + + /** Sums the diff stats across the given requests, or undefined when nothing changed. */ + private _statForRequests(requestIds: readonly string[], reader?: IReader): PromptDiffStat | undefined { + let added = 0; + let removed = 0; + const files = new Set<string>(); + for (const requestId of requestIds) { + for (const diff of this._diffsForRequest(requestId, reader)) { + if (diff.identical) { + continue; + } + added += diff.added; + removed += diff.removed; + files.add(diff.modifiedURI.toString()); + } + } + return files.size > 0 ? { added, removed, fileCount: files.size } : undefined; + } +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRail.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRail.ts new file mode 100644 index 0000000000000..303f433c8c4c2 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRail.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from '../../../../base/common/event.js'; +import { IDisposable } from '../../../../base/common/lifecycle.js'; +import { URI } from '../../../../base/common/uri.js'; +import { IPromptScrollLayout, PromptFileDiff, PromptTick } from './promptTimelineModel.js'; + +export interface IPromptReviewFileEvent { + readonly tick: PromptTick; + readonly file: URI; +} + +/** + * A prompt timeline rail: a presentation-only vertical strip pinned to the right + * edge of the chat transcript that renders one mark per prompt and lets the user + * preview, jump to, and review each prompt. Rendered as a proportional overview + * ruler with a viewport thumb, like the editor overview ruler. + */ +export interface IPromptTimelineRail extends IDisposable { + readonly domNode: HTMLElement; + + /** Fired when a mark is chosen (click / keyboard), with its request id. */ + readonly onDidSelect: Event<string>; + /** Fired to review all of a prompt's changes. */ + readonly onDidReview: Event<PromptTick>; + /** Fired to review a single changed file of a prompt. */ + readonly onDidReviewFile: Event<IPromptReviewFileEvent>; + + setFilesProvider(provider: (tick: PromptTick) => readonly PromptFileDiff[]): void; + setTicks(ticks: readonly PromptTick[]): void; + setActive(requestId: string | undefined): void; + focusTick(requestId: string): void; + setHostWidth(width: number): void; + + /** Supplies proportional scroll positions for the marks and viewport thumb. */ + setScrollLayout(layout: IPromptScrollLayout | undefined): void; +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRulerRail.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRulerRail.ts new file mode 100644 index 0000000000000..993245a80d985 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineRulerRail.ts @@ -0,0 +1,254 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { $, addDisposableListener, append, clearNode, EventType, getWindow } from '../../../../base/browser/dom.js'; +import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; +import { KeyCode } from '../../../../base/common/keyCodes.js'; +import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; +import { localize } from '../../../../nls.js'; +import { PromptTimelineCard } from './promptTimelineCard.js'; +import { IPromptScrollLayout, PromptFileDiff, PromptTick } from './promptTimelineModel.js'; +import { IPromptReviewFileEvent, IPromptTimelineRail } from './promptTimelineRail.js'; +import './media/promptTimeline.css'; + +/** Minimum clickable target size (WCAG 2.5.8) for each mark's hit area. */ +const MIN_TARGET = 24; +/** Below this transcript width the rail hides so it does not crowd the content. */ +const MIN_HOST_WIDTH = 320; + +interface IMarkEntry { + tick: PromptTick; + readonly button: HTMLButtonElement; + readonly bar: HTMLElement; +} + +/** + * The overview-ruler rail. The whole session is compressed into the rail height + * like the editor's overview ruler: each prompt is a mark at its proportional + * scroll position, coloured only to signal whether it changed code, with the + * real scrollbar slider as a you-are-here thumb. Detail lives in the hover card. + */ +export class PromptTimelineRulerRail extends Disposable implements IPromptTimelineRail { + + private readonly _domNode: HTMLElement; + private readonly _marksContainer: HTMLElement; + private readonly _thumb: HTMLElement; + private readonly _card: PromptTimelineCard; + private readonly _markDisposables = this._register(new DisposableStore()); + private readonly _marks: IMarkEntry[] = []; + + private _activeRequestId: string | undefined; + private _layout: IPromptScrollLayout | undefined; + private _resizeObserverReady = false; + private _hostWidth = Number.POSITIVE_INFINITY; + + private readonly _onDidSelect = this._register(new Emitter<string>()); + readonly onDidSelect: Event<string> = this._onDidSelect.event; + + private readonly _onDidReview = this._register(new Emitter<PromptTick>()); + readonly onDidReview: Event<PromptTick> = this._onDidReview.event; + + private readonly _onDidReviewFile = this._register(new Emitter<IPromptReviewFileEvent>()); + readonly onDidReviewFile: Event<IPromptReviewFileEvent> = this._onDidReviewFile.event; + + get domNode(): HTMLElement { return this._domNode; } + + constructor() { + super(); + this._domNode = $('nav.prompt-timeline-rail.prompt-timeline-rail-ruler'); + this._domNode.setAttribute('aria-label', localize('promptTimeline.railLabel', "Prompt timeline")); + this._domNode.setAttribute('role', 'toolbar'); + this._domNode.setAttribute('aria-orientation', 'vertical'); + this._marksContainer = append(this._domNode, $('.prompt-timeline-ruler-marks')); + this._thumb = append(this._domNode, $('.prompt-timeline-ruler-thumb')); + this._thumb.classList.add('hidden'); + this._card = this._register(new PromptTimelineCard(this._domNode)); + this._register(this._card.onDidReview(tick => this._onDidReview.fire(tick))); + this._register(this._card.onDidReviewFile(e => this._onDidReviewFile.fire(e))); + + // Toolbar keyboard model: one Tab stop, Arrow/Home/End move between marks. + this._register(addDisposableListener(this._marksContainer, EventType.KEY_DOWN, e => this._onMarksKeyDown(e))); + + this._register(addDisposableListener(this._domNode, EventType.FOCUS_OUT, () => { + if (!this._domNode.contains(getWindow(this._domNode).document.activeElement)) { + this._card.scheduleHide(); + } + })); + } + + setFilesProvider(provider: (tick: PromptTick) => readonly PromptFileDiff[]): void { + this._card.setFilesProvider(provider); + } + + setTicks(ticks: readonly PromptTick[]): void { + const sameStructure = ticks.length === this._marks.length + && ticks.every((t, i) => this._marks[i]?.tick.requestId === t.requestId); + if (sameStructure) { + for (let i = 0; i < ticks.length; i++) { + this._renderMark(this._marks[i], ticks[i]); + } + this._updateActiveClasses(); + this._relayout(); + return; + } + + this._markDisposables.clear(); + this._marks.length = 0; + clearNode(this._marksContainer); + this._card.hide(); + + for (const tick of ticks) { + const button = append(this._marksContainer, $<HTMLButtonElement>('button.prompt-timeline-ruler-mark')); + button.tabIndex = -1; + const bar = append(button, $('span.prompt-timeline-ruler-bar')); + const entry: IMarkEntry = { tick, button, bar }; + this._renderMark(entry, tick); + const requestId = tick.requestId; + this._markDisposables.add(addDisposableListener(button, EventType.CLICK, () => this._onDidSelect.fire(requestId))); + this._markDisposables.add(addDisposableListener(button, EventType.MOUSE_ENTER, () => this._showCard(entry))); + this._markDisposables.add(addDisposableListener(button, EventType.FOCUS, () => { this._showCard(entry); this._updateTabStops(this._marks.indexOf(entry)); })); + this._markDisposables.add(addDisposableListener(button, EventType.MOUSE_LEAVE, () => this._card.scheduleHide())); + this._marks.push(entry); + } + + this._ensureResizeObserver(); + // Make the active mark (else the first) the single Tab stop into the toolbar. + const activeIndex = this._marks.findIndex(m => m.tick.requestId === this._activeRequestId); + this._updateTabStops(activeIndex >= 0 ? activeIndex : 0); + this._updateActiveClasses(); + this._relayout(); + } + + /** Roving tabindex: exactly one mark is tabbable so the toolbar is a single Tab stop. */ + private _updateTabStops(focusIndex: number): void { + for (let i = 0; i < this._marks.length; i++) { + this._marks[i].button.tabIndex = i === focusIndex ? 0 : -1; + } + } + + private _onMarksKeyDown(e: KeyboardEvent): void { + if (this._marks.length === 0) { + return; + } + const event = new StandardKeyboardEvent(e); + const currentIndex = this._marks.findIndex(m => m.button === getWindow(this._domNode).document.activeElement); + let nextIndex: number; + switch (event.keyCode) { + case KeyCode.DownArrow: nextIndex = Math.min(this._marks.length - 1, currentIndex + 1); break; + case KeyCode.UpArrow: nextIndex = Math.max(0, currentIndex - 1); break; + case KeyCode.Home: nextIndex = 0; break; + case KeyCode.End: nextIndex = this._marks.length - 1; break; + default: return; + } + event.preventDefault(); + event.stopPropagation(); + this._updateTabStops(nextIndex); + this._marks[nextIndex]?.button.focus(); + } + + private _renderMark(entry: IMarkEntry, tick: PromptTick): void { + entry.tick = tick; + entry.button.setAttribute('aria-label', tick.ariaLabel); + // Two-tone bar: a green added segment and a red removed segment, sized by + // the turn's diff split. Gray when the turn made no edits. + clearNode(entry.bar); + const stat = tick.stat; + const edited = !!stat && stat.added + stat.removed > 0; + entry.bar.classList.toggle('edited', edited); + if (edited) { + // Only append the sides that exist so a pure-add turn is fully green and a + // pure-delete turn fully red; the min-width floor keeps a lopsided split visible. + if (stat!.added > 0) { + append(entry.bar, $('span.seg-add')).style.flexGrow = String(stat!.added); + } + if (stat!.removed > 0) { + append(entry.bar, $('span.seg-del')).style.flexGrow = String(stat!.removed); + } + } + } + + setActive(requestId: string | undefined): void { + this._activeRequestId = requestId; + this._updateActiveClasses(); + } + + focusTick(requestId: string): void { + this._marks.find(m => m.tick.requestId === requestId || m.tick.allRequestIds.includes(requestId))?.button.focus(); + } + + setHostWidth(width: number): void { + if (width > 0 && width !== this._hostWidth) { + this._hostWidth = width; + this._relayout(); + } + } + + setScrollLayout(layout: IPromptScrollLayout | undefined): void { + this._layout = layout; + this._relayout(); + } + + /** Places each mark at its proportional scroll position and sizes the viewport thumb. */ + private _relayout(): void { + const height = this._domNode.clientHeight; + const layout = this._layout; + const overflowing = this._hostWidth < MIN_HOST_WIDTH; + this._domNode.classList.toggle('overflowing', overflowing); + if (overflowing || height <= 0 || !layout || layout.total <= 0) { + this._thumb.classList.add('hidden'); + return; + } + const scale = height / layout.total; + const tops = new Map(layout.marks.map(m => [m.requestId, m.top])); + for (const entry of this._marks) { + const top = tops.get(entry.tick.requestId); + if (top === undefined) { + entry.button.classList.add('hidden'); + continue; + } + entry.button.classList.remove('hidden'); + // The button is a >=24px hit target centered on the mark's compressed position. + entry.button.style.top = `${top * scale - MIN_TARGET / 2}px`; + } + // The thumb reuses the scrollbar slider: its span is the visible viewport, + // approximated by the rail height (the rail overlays the transcript viewport). + this._thumb.classList.remove('hidden'); + this._thumb.style.top = `${(layout.scrollTop / layout.total) * height}px`; + this._thumb.style.height = `${Math.max(20, (height / layout.total) * height)}px`; + } + + private _updateActiveClasses(): void { + for (const entry of this._marks) { + const isActive = entry.tick.requestId === this._activeRequestId; + entry.button.classList.toggle('active', isActive); + if (isActive) { + entry.button.setAttribute('aria-current', 'location'); + } else { + entry.button.removeAttribute('aria-current'); + } + } + } + + private _showCard(entry: IMarkEntry): void { + const markRect = entry.button.getBoundingClientRect(); + const domRect = this._domNode.getBoundingClientRect(); + this._card.show(entry.tick, markRect.top - domRect.top + markRect.height / 2); + } + + private _ensureResizeObserver(): void { + if (this._resizeObserverReady) { + return; + } + const ResizeObserverCtor = getWindow(this._domNode).ResizeObserver; + if (!ResizeObserverCtor) { + return; + } + this._resizeObserverReady = true; + const observer = new ResizeObserverCtor(() => this._relayout()); + observer.observe(this._domNode); + this._register(toDisposable(() => observer.disconnect())); + } +} diff --git a/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineWidgetContrib.ts b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineWidgetContrib.ts new file mode 100644 index 0000000000000..77cba5b5cf97b --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/browser/promptTimelineWidgetContrib.ts @@ -0,0 +1,142 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { getWindow } from '../../../../base/browser/dom.js'; +import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; +import { autorun } from '../../../../base/common/observable.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; +import { IChatWidget } from '../../../../workbench/contrib/chat/browser/chat.js'; +import { IChatWidgetContrib, ChatWidget } from '../../../../workbench/contrib/chat/browser/widget/chatWidget.js'; +import { ChatAgentLocation } from '../../../../workbench/contrib/chat/common/constants.js'; +import { MIN_PROMPTS, PROMPT_TIMELINE_CONTRIB_ID, PROMPT_TIMELINE_ENABLED_SETTING } from '../common/promptTimeline.js'; +import { PromptTimelineModel, PromptEntry } from './promptTimelineModel.js'; +import { IPromptTimelineRail } from './promptTimelineRail.js'; +import { PromptTimelineRulerRail } from './promptTimelineRulerRail.js'; + +/** + * Per-widget contribution that overlays a prompt timeline rail on the chat + * transcript and exposes a navigation API for keyboard-driven commands. The rail + * exists only while `sessions.promptTimeline.enabled` is set, and is torn down + * and re-created when the enablement changes. + */ +export class PromptTimelineWidgetContrib extends Disposable implements IChatWidgetContrib { + + static readonly ID = PROMPT_TIMELINE_CONTRIB_ID; + readonly id = PromptTimelineWidgetContrib.ID; + + private _model: PromptTimelineModel | undefined; + private _rail: IPromptTimelineRail | undefined; + + /** Holds the model, rail and all their wiring while the feature is enabled. */ + private readonly _enablement = this._register(new DisposableStore()); + private _enabled = false; + + constructor( + private readonly widget: IChatWidget, + @IInstantiationService private readonly instantiationService: IInstantiationService, + @IConfigurationService private readonly configurationService: IConfigurationService, + ) { + super(); + + // The rail only makes sense for the main chat transcript location. + if (widget.location !== ChatAgentLocation.Chat) { + return; + } + + this._register(this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration(PROMPT_TIMELINE_ENABLED_SETTING)) { + this._updateRail(); + } + })); + this._updateRail(); + } + + /** Creates or disposes the rail to match the enablement setting. */ + private _updateRail(): void { + const enabled = this.configurationService.getValue<boolean>(PROMPT_TIMELINE_ENABLED_SETTING) !== false; + if (enabled === this._enabled) { + return; + } + this._enabled = enabled; + this._enablement.clear(); + this._model = undefined; + this._rail = undefined; + if (enabled) { + this._createRail(); + } + } + + private _createRail(): void { + // CONTRIBS always constructs contribs with the concrete widget. + const model = this._enablement.add(this.instantiationService.createInstance(PromptTimelineModel, this.widget as ChatWidget)); + const rail: IPromptTimelineRail = this._enablement.add(new PromptTimelineRulerRail()); + this._model = model; + this._rail = rail; + + this._mountRail(rail); + + rail.setFilesProvider(tick => model.getRequestFiles(tick)); + this._enablement.add(rail.onDidSelect(requestId => model.reveal(requestId))); + this._enablement.add(rail.onDidReview(tick => { void model.reviewChanges(tick); })); + this._enablement.add(rail.onDidReviewFile(e => { void model.reviewChanges(e.tick, e.file); })); + + this._enablement.add(autorun(reader => { + const ticks = model.ticks.read(reader); + // Toggle visibility before rendering so the rail's fit measurement in + // setTicks runs against the displayed (non-zero height) element. + rail.domNode.classList.toggle('hidden', ticks.length < MIN_PROMPTS); + rail.setTicks(ticks); + })); + + this._enablement.add(autorun(reader => { + rail.setActive(model.activeRequestId.read(reader)); + })); + + // Supply proportional scroll positions for the marks and viewport thumb. + this._enablement.add(autorun(reader => { + model.onDidChangeScrollLayout.read(reader); + rail.setScrollLayout(model.getScrollLayout()); + })); + } + + private _mountRail(rail: IPromptTimelineRail): void { + const railNode = rail.domNode; + const host = this.widget.domNode; + // Anchor the absolutely-positioned overlay to the chat widget via a class + // we own, removed on teardown so we never leave the foreign container mutated. + host.classList.add('prompt-timeline-host'); + this._enablement.add(toDisposable(() => host.classList.remove('prompt-timeline-host'))); + host.appendChild(railNode); + this._enablement.add(toDisposable(() => railNode.remove())); + + // Keep the rail above the input part so it only spans the transcript. + const inputPart = this.widget.inputPart; + this._enablement.add(autorun(reader => { + railNode.style.setProperty('--prompt-timeline-bottom', `${inputPart.height.read(reader)}px`); + })); + + // Report the host width so the rail can hide on very narrow transcripts. + const ResizeObserverCtor = getWindow(host).ResizeObserver; + if (ResizeObserverCtor) { + const observer = new ResizeObserverCtor(() => rail.setHostWidth(host.clientWidth)); + observer.observe(host); + this._enablement.add(toDisposable(() => observer.disconnect())); + } + rail.setHostWidth(host.clientWidth); + } + + // -- Navigation API (used by promptTimelineActions) -- + + /** All user prompts for the picker (every prompt, not just the bucketed ticks). */ + getAllPrompts(): readonly PromptEntry[] { + return this._model?.getAllPrompts() ?? []; + } + + reveal(requestId: string): void { + this._model?.reveal(requestId); + this._rail?.focusTick(requestId); + } +} diff --git a/src/vs/sessions/contrib/promptTimeline/common/promptTimeline.ts b/src/vs/sessions/contrib/promptTimeline/common/promptTimeline.ts new file mode 100644 index 0000000000000..c7bbcfb401296 --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/common/promptTimeline.ts @@ -0,0 +1,17 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +/** Id of the per-widget contribution that owns the prompt timeline rail. */ +export const PROMPT_TIMELINE_CONTRIB_ID = 'sessions.promptTimeline'; + +/** Setting that controls whether the prompt timeline rail is shown in the Agents window. */ +export const PROMPT_TIMELINE_ENABLED_SETTING = 'sessions.promptTimeline.enabled'; + +/** Minimum number of user prompts before the rail is shown. */ +export const MIN_PROMPTS = 2; + +export const enum PromptTimelineCommandId { + GoToPrompt = 'sessions.promptTimeline.goToPrompt', +} diff --git a/src/vs/sessions/contrib/promptTimeline/test/browser/promptBucketing.test.ts b/src/vs/sessions/contrib/promptTimeline/test/browser/promptBucketing.test.ts new file mode 100644 index 0000000000000..2ec5930f5716f --- /dev/null +++ b/src/vs/sessions/contrib/promptTimeline/test/browser/promptBucketing.test.ts @@ -0,0 +1,140 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { _testing, budgetBucketPrompts, MAX_TICKS, type PromptItem } from '../../browser/promptBucketing.js'; + +suite('PromptBucketing', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + const now = new Date(2026, 6, 1, 12, 0, 0).getTime(); + + function prompt(requestId: string, date: Date): PromptItem { + return { + requestId, + text: `Prompt ${requestId}`, + timestamp: date.getTime() + }; + } + + test('returns no buckets for empty input', () => { + assert.deepStrictEqual(budgetBucketPrompts([], now), []); + }); + + test('keeps a single prompt as one tick', () => { + const item = prompt('p1', new Date(2026, 6, 1, 11, 30)); + + assert.deepStrictEqual(budgetBucketPrompts([item], now), [{ + prompt: item, + prompts: [item], + count: 1 + }]); + }); + + test('keeps today prompts at per-prompt granularity', () => { + const prompts = [ + prompt('p1', new Date(2026, 6, 1, 9)), + prompt('p2', new Date(2026, 6, 1, 9, 30)), + prompt('p3', new Date(2026, 6, 1, 10)) + ]; + + assert.deepStrictEqual(_testing.bucketPrompts(prompts, new Date(now)).map(bucket => ({ + requestId: bucket.prompt.requestId, + ids: bucket.prompts.map(prompt => prompt.requestId), + count: bucket.count + })), [ + { requestId: 'p1', ids: ['p1'], count: 1 }, + { requestId: 'p2', ids: ['p2'], count: 1 }, + { requestId: 'p3', ids: ['p3'], count: 1 } + ]); + }); + + test('groups older consecutive prompts by day and month', () => { + const prompts = [ + prompt('day-1', new Date(2026, 5, 20, 9)), + prompt('day-2', new Date(2026, 5, 20, 18)), + prompt('day-3', new Date(2026, 5, 19, 9)), + prompt('month-1', new Date(2026, 4, 8, 9)), + prompt('month-2', new Date(2026, 4, 25, 9)) + ]; + + assert.deepStrictEqual(_testing.bucketPrompts(prompts, new Date(now)).map(bucket => ({ + requestId: bucket.prompt.requestId, + ids: bucket.prompts.map(prompt => prompt.requestId), + count: bucket.count + })), [ + { requestId: 'day-1', ids: ['day-1', 'day-2'], count: 2 }, + { requestId: 'day-3', ids: ['day-3'], count: 1 }, + { requestId: 'month-1', ids: ['month-1', 'month-2'], count: 2 } + ]); + }); + + test('caps with uniform sampling while keeping first and last', () => { + const buckets = Array.from({ length: 10 }, (_, index) => { + const item = prompt(`p${index}`, new Date(2026, 6, 1, index)); + return { prompt: item, prompts: [item], count: 1 }; + }); + + assert.deepStrictEqual(_testing.uniformSample(buckets, 5).map(bucket => bucket.prompt.requestId), [ + 'p0', + 'p2', + 'p5', + 'p7', + 'p9' + ]); + }); + + test('expands the most recent coarse bucket when under budget', () => { + const prompts = [ + prompt('older-1', new Date(2026, 5, 20, 9)), + prompt('older-2', new Date(2026, 5, 20, 10)), + prompt('recent-1', new Date(2026, 5, 30, 9)), + prompt('recent-2', new Date(2026, 5, 30, 10)), + prompt('recent-3', new Date(2026, 5, 30, 11)) + ]; + const buckets = _testing.bucketPrompts(prompts, new Date(now)); + + assert.deepStrictEqual(_testing.expandRecentBuckets(buckets, 4).map(bucket => ({ + requestId: bucket.prompt.requestId, + ids: bucket.prompts.map(prompt => prompt.requestId), + count: bucket.count + })), [ + { requestId: 'older-1', ids: ['older-1', 'older-2'], count: 2 }, + { requestId: 'recent-1', ids: ['recent-1'], count: 1 }, + { requestId: 'recent-2', ids: ['recent-2'], count: 1 }, + { requestId: 'recent-3', ids: ['recent-3'], count: 1 } + ]); + }); + + test('never returns more than MAX_TICKS', () => { + const prompts = Array.from({ length: 80 }, (_, index) => prompt(`p${index}`, new Date(2026, 6, 1, 0, index))); + + assert.deepStrictEqual({ + length: budgetBucketPrompts(prompts, now).length, + first: budgetBucketPrompts(prompts, now)[0].prompt.requestId, + last: budgetBucketPrompts(prompts, now)[MAX_TICKS - 1].prompt.requestId + }, { + length: MAX_TICKS, + first: 'p0', + last: 'p79' + }); + }); + + test('honors an explicit maxTicks cap smaller than MAX_TICKS', () => { + const prompts = Array.from({ length: 40 }, (_, index) => prompt(`p${index}`, new Date(2026, 6, 1, 0, index))); + const result = budgetBucketPrompts(prompts, now, 6); + + assert.deepStrictEqual({ + length: result.length, + first: result[0].prompt.requestId, + last: result[result.length - 1].prompt.requestId + }, { + length: 6, + first: 'p0', + last: 'p39' + }); + }); +}); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 161fe4b81f1a0..16dbc94ce45bb 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { disposableTimeout, DeferredPromise, raceTimeout } from '../../../../../base/common/async.js'; +import { disposableTimeout, raceTimeout } from '../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { arrayEquals, structuralEquals } from '../../../../../base/common/equals.js'; @@ -25,15 +25,15 @@ import { migrateLegacyAutopilotConfig } from '../../../../../platform/agentHost/ import type { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; import { ResolveSessionConfigResult } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; import { AgentCustomization, ChangesSummary, ChatInteractivity as ProtocolChatInteractivity, ChatOriginKind as ProtocolChatOriginKind, type ClientPluginCustomization, Customization, CustomizationType, ModelSelection, SessionStatus as ProtocolSessionStatus, RootConfigState, RootState, SessionActiveClient, SessionState, SessionSummary, type Changeset } from '../../../../../platform/agentHost/common/state/protocol/state.js'; -import { ActionType, isChatAction, isSessionAction, NotificationType, type ProgressParams } from '../../../../../platform/agentHost/common/state/sessionActions.js'; +import { ActionType, isChatAction, isSessionAction, NotificationType } from '../../../../../platform/agentHost/common/state/sessionActions.js'; import { AgentCapabilities, AgentInfo, buildChatUri, buildDefaultChatUri, isDefaultChatUri, parseChatUri, readSessionGitHubState, readSessionGitState, readSessionWorkspaceless, ROOT_STATE_URI, SessionMeta, StateComponents, withSessionWorkspaceless, type ChatSummary, type ISessionGitState } from '../../../../../platform/agentHost/common/state/sessionState.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'; import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; -import { IProgressService, IProgressStep, ProgressLocation } from '../../../../../platform/progress/common/progress.js'; import { IWorkspaceTrustManagementService } from '../../../../../platform/workspace/common/workspaceTrust.js'; +import { AgentHostDownloadProgress } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostDownloadProgress.js'; import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; import { ChatMode } from '../../../../../workbench/contrib/chat/common/chatModes.js'; @@ -1708,19 +1708,6 @@ class NewSession extends Disposable { * URI-scheme mapping for session metadata, the agent-provider lookup, and * the browse UI. */ -/** - * One in-flight download, tracked by - * {@link BaseAgentHostSessionsProvider._activeDownloads}. Owns the lifecycle - * of a single notification progress: `report` pushes a step, `complete` - * resolves the backing deferred so the notification is dismissed. - */ -interface IActiveDownload { - /** Last reported determinate percentage, used to compute progress increments. */ - lastPercent: number; - report(step: IProgressStep): void; - complete(): void; -} - export abstract class BaseAgentHostSessionsProvider extends Disposable implements IAgentHostSessionsProvider { abstract readonly id: string; @@ -1797,16 +1784,12 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement private _cacheDirty = false; /** - * Active progress indicators keyed by `progressToken`. Today's only - * producer is the agent host's lazy, first-use SDK download, which is - * provider-global: the host emits a single stream per download keyed by the - * download's own stable identity (so distinct sessions of a provider share - * one indicator). Each entry owns one long-running notification progress - * (opened on the first frame), driven via {@link IActiveDownload.report} and - * dismissed via {@link IActiveDownload.complete} once `progress >= total`. - * See {@link _handleProgress}. + * Renders the agent host's lazy, first-use SDK download as a notification + * progress bar. Shared with the editor window so both surfaces render + * download progress identically. Fed by the `NotificationType.Progress` + * frames received in {@link _attachConnectionListeners}. */ - private readonly _activeDownloads = new Map<string, IActiveDownload>(); + private readonly _downloadProgress: AgentHostDownloadProgress; /** * Temporary session that has been sent (first turn dispatched) but not yet @@ -1816,6 +1799,27 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement */ protected _pendingSession: ISession | undefined; + /** + * Raw ids of backend sessions that an in-flight {@link _waitForNewSession} + * has already matched to its send, so a *concurrent* new-session send of + * the same scheme does not resolve to the same committed session. Each + * matched id is released by the owning send in its `finally`. + */ + private readonly _committingSessionRawIds = new Set<string>(); + + /** + * Own raw ids ({@link chatResource} path) of currently in-flight + * new-session sends. A send's committed backend session keeps the eager + * id it was created with, so {@link _waitForNewSession} matches a send to + * its OWN id first. The novelty fallback (for flows where the backend + * assigns a different id) must then never latch onto *another* in-flight + * send's own session — otherwise two concurrent same-scheme sends racing + * in a shared download/materialize window would swap sessions (each + * graduating onto the other's committed session). Populated at send start, + * cleared in the send's `finally`. + */ + private readonly _inFlightNewSessionOwnIds = new Set<string>(); + /** * In-flight new sessions — sessions being composed in the new-chat view * before their first message is sent, keyed by `sessionId`. See @@ -1924,21 +1928,15 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement @IStorageService protected readonly _storageService: IStorageService, @IDialogService protected readonly _dialogService: IDialogService, @IWorkspaceTrustManagementService protected readonly _workspaceTrustManagementService: IWorkspaceTrustManagementService, - @IProgressService protected readonly _progressService: IProgressService, ) { super(); + this._downloadProgress = this._register(this._instantiationService.createInstance(AgentHostDownloadProgress)); this._register(toDisposable(() => { for (const cached of this._sessionCache.values()) { cached.dispose(); } this._sessionCache.clear(); })); - this._register(toDisposable(() => { - for (const download of this._activeDownloads.values()) { - download.complete(); - } - this._activeDownloads.clear(); - })); // Keep the state subscription of every on-screen session pinned so // host-spawned catalog changes (e.g. subagents) reach `cached.chats` @@ -3242,6 +3240,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // Treat that raw id as the session we are waiting for, not old state. const newSessionRawId = chatResource.path.replace(/^\//, ''); existingKeys.delete(newSessionRawId); + // Publish this send's own id so concurrent same-scheme sends don't + // latch onto it via their novelty fallback (which would swap sessions). + this._inFlightNewSessionOwnIds.add(newSessionRawId); const result = await this._chatService.sendRequest(chatResource, query, sendOptions); if (result.kind === 'rejected') { @@ -3259,9 +3260,12 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement this._pendingSession = skeleton; this._onDidChangeSessions.fire({ added: [skeleton], removed: [], changed: [] }); + // Raw id claimed by _waitForNewSession for this send (released in finally). + let committedRawId: string | undefined; try { - const committedSession = await this._waitForNewSession(existingKeys, chatResource.scheme); + const committedSession = await this._waitForNewSession(existingKeys, chatResource.scheme, newSessionRawId); if (committedSession) { + committedRawId = committedSession.resource.path.substring(1); this._preserveNewSessionConfig(newSession, committedSession.sessionId); // Carry the picked custom agent onto the committed session before // the replace event so the agent picker doesn't reset to the @@ -3271,8 +3275,8 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // first turn (see `sendOptions.modeInfo`), so update only the local // mode observable here rather than re-notifying it via `setAgent`. if (selectedAgent) { - const committedRawId = this._rawIdFromChatId(committedSession.sessionId); - const committedAdapter = committedRawId ? this._sessionCache.get(committedRawId) : undefined; + const committedRawIdForAgent = this._rawIdFromChatId(committedSession.sessionId); + const committedAdapter = committedRawIdForAgent ? this._sessionCache.get(committedRawIdForAgent) : undefined; committedAdapter?.setChatAgent(committedAdapter.resource, selectedAgent); } // Session graduated: release the eager subscription without @@ -3293,6 +3297,13 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } catch { // Connection lost or timeout — fall through to the failure cleanup. } finally { + // Release the claim so unrelated future sends can match this + // session if needed; concurrent in-flight sends already captured + // their `existingKeys` and won't retroactively match it. + if (committedRawId !== undefined) { + this._committingSessionRawIds.delete(committedRawId); + } + this._inFlightNewSessionOwnIds.delete(newSessionRawId); // Defensive clear: covers the failure path where the try block // never reached the explicit clear above. this._pendingSession = undefined; @@ -3909,23 +3920,55 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement * type that happens to appear mid-send — a slow codex send racing against a * restored claude session, say — is never mistaken for this send's commit. */ - private async _waitForNewSession(existingKeys: Set<string>, expectedScheme: string): Promise<ISession | undefined> { + private async _waitForNewSession(existingKeys: Set<string>, expectedScheme: string, ownRawId: string): Promise<ISession | undefined> { + // A candidate backend session commits THIS send when it is unclaimed, + // of the expected type, and either (a) carries this send's own id — the + // eager/committed id is preserved, so this is the exact match — or + // (b) is a novel session that is not another in-flight send's own + // session (the novelty fallback covers backends that assign a fresh + // id, without letting two concurrent same-scheme sends swap sessions). + const matches = (rawId: string, scheme: string): boolean => { + if (scheme !== expectedScheme || this._committingSessionRawIds.has(rawId)) { + return false; + } + if (rawId === ownRawId) { + return true; + } + return !existingKeys.has(rawId) && !this._inFlightNewSessionOwnIds.has(rawId); + }; + await this._refreshSessions(); - for (const [key, cached] of this._sessionCache) { - if (!existingKeys.has(key) && cached.resource.scheme === expectedScheme) { - return cached; + // Prefer this send's own id; fall back to any acceptable novel session. + const scan = (): ISession | undefined => { + let fallback: ISession | undefined; + for (const cached of this._sessionCache.values()) { + const rawId = cached.resource.path.substring(1); + if (!matches(rawId, cached.resource.scheme)) { + continue; + } + if (rawId === ownRawId) { + return cached; + } + fallback ??= cached; } + return fallback; + }; + const immediate = scan(); + if (immediate) { + this._committingSessionRawIds.add(immediate.resource.path.substring(1)); + return immediate; } const waitDisposables = new DisposableStore(); try { const sessionPromise = new Promise<ISession | undefined>((resolve) => { waitDisposables.add(this._onDidChangeSessions.event(e => { - const newSession = e.added.find(s => { - const rawId = s.resource.path.substring(1); - return !existingKeys.has(rawId) && s.resource.scheme === expectedScheme; - }); + // Prefer this send's own id within the batch before falling + // back to an acceptable novel session. + const exact = e.added.find(s => s.resource.path.substring(1) === ownRawId && matches(ownRawId, s.resource.scheme)); + const newSession = exact ?? e.added.find(s => matches(s.resource.path.substring(1), s.resource.scheme)); if (newSession) { + this._committingSessionRawIds.add(newSession.resource.path.substring(1)); resolve(newSession); } })); @@ -3953,7 +3996,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } else if (n.type === NotificationType.SessionSummaryChanged) { this._handleSessionSummaryChanged(n.session, n.changes); } else if (n.type === NotificationType.Progress) { - this._handleProgress(n); + this._downloadProgress.handleProgress(n); } })); @@ -4099,79 +4142,6 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement }); } - /** - * Render a generic `progress` notification as a notification progress bar. - * Progress is correlated by {@link ProgressParams.progressToken}; today's - * only producer is the agent host's lazy, first-use SDK download, which the - * host surfaces as a single stream per provider keyed by the download's own - * stable identity — so one indicator per download regardless of how many - * sessions await it. Determinate when the host knows the `total` - * (`Content-Length`), or a byte-count spinner otherwise. The operation is - * complete — and the notification dismissed — once `progress >= total`. The - * human-readable brand noun rides on {@link ProgressParams.message}. - */ - private _handleProgress(progress: ProgressParams): void { - // New AI UI must stay hidden when the user has turned AI features off. - if (this._baseConfigurationService.getValue<boolean>(ChatConfiguration.AIDisabled)) { - return; - } - - // Complete when we reach the (possibly server-synthesized) total. The - // host emits a terminal frame with `progress === total` for success, - // indeterminate completion, and failure alike; real errors surface via - // the session-failure path, not here. - const isComplete = progress.total !== undefined && progress.progress >= progress.total; - if (isComplete) { - this._activeDownloads.get(progress.progressToken)?.complete(); - this._activeDownloads.delete(progress.progressToken); - return; - } - - let entry = this._activeDownloads.get(progress.progressToken); - if (!entry) { - // First frame for this download: open one long-running notification - // progress and drive it via `report` until a terminal frame resolves - // `deferred`. `message` is the host-supplied, already-localized title - // (e.g. "Downloading Claude agent…"); render it verbatim so this stays - // a generic indicator that makes no assumption about what's downloading. - const deferred = new DeferredPromise<void>(); - let report: ((step: IProgressStep) => void) | undefined; - const title = progress.message ?? localize('agentHost.download.titleFallback', "Downloading…"); - this._progressService.withProgress( - { - location: ProgressLocation.Notification, - title, - }, - p => { - report = step => p.report(step); - return deferred.p; - }, - ); - entry = { - lastPercent: 0, - report: step => report?.(step), - complete: () => deferred.complete(), - }; - this._activeDownloads.set(progress.progressToken, entry); - } - - if (progress.total && progress.total > 0) { - const percent = Math.max(0, Math.min(100, Math.round((progress.progress / progress.total) * 100))); - const increment = percent - entry.lastPercent; - entry.lastPercent = percent; - entry.report({ - message: localize('agentHost.download.percent', "{0}%", percent), - increment: increment > 0 ? increment : 0, - total: 100, - }); - } else { - // No total: indeterminate. Show megabytes received so the user - // still sees the download making progress. - const megabytes = (progress.progress / (1024 * 1024)).toFixed(1); - entry.report({ message: localize('agentHost.download.megabytes', "{0} MB", megabytes) }); - } - } - private _handleConfigChanged(session: string, config: Record<string, unknown>, replace: boolean): void { const rawId = AgentSession.id(session); const cached = this._sessionCache.get(rawId); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts index 2f89ea4a922ca..db7bd7424cce1 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts @@ -18,7 +18,6 @@ import { IInstantiationService } from '../../../../../platform/instantiation/com import { ILabelService } from '../../../../../platform/label/common/label.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; import { IStorageService } from '../../../../../platform/storage/common/storage.js'; -import { IProgressService } from '../../../../../platform/progress/common/progress.js'; import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IWorkspaceTrustManagementService } from '../../../../../platform/workspace/common/workspaceTrust.js'; import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; @@ -102,9 +101,8 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide @IDialogService dialogService: IDialogService, @IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService, @IWorkspaceTrustManagementService workspaceTrustManagementService: IWorkspaceTrustManagementService, - @IProgressService progressService: IProgressService, ) { - super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService, workspaceTrustManagementService, progressService); + super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService, workspaceTrustManagementService); this._isSessionsWindow = environmentService.isSessionsWindow; 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 607def92a2da6..deb0fc921659b 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 @@ -3062,6 +3062,50 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(committed.resource.scheme, 'agent-host-codex', `expected the committed session to be the codex session, got ${committed.resource.toString()}`); }); + test('two concurrent same-type new-session sends each commit to their own session (no swap during a shared download window)', async () => { + // Regression: when the first send of a session type triggers a lengthy + // bring-up (e.g. the Claude SDK download) and a SECOND session of the + // same type is started and sent before it finishes, both sends park in + // `_waitForNewSession`. A committed backend session keeps the eager id + // its send created it with, so each send must graduate onto its OWN id. + // Matching purely by novelty + scheme would let the two waiters SWAP + // sessions — whichever materializes first is grabbed by the send that + // parked first, regardless of ownership — leaving the user on the wrong + // session. Here the SECOND session (B) materializes BEFORE the first + // (A), which is exactly the ordering that triggered the swap. + const provider = createProvider(disposables, agentHost, undefined, { + openSession: true, + sendRequest: async (): Promise<ChatSendResult> => ({ kind: 'sent' as const, data: {} as ChatSendResult extends { kind: 'sent'; data: infer D } ? D : never }), + }); + const sessionTypeId = provider.sessionTypes[0].id; + + const sessionA = provider.createNewSession(URI.parse('file:///home/user/a'), sessionTypeId); + const chatA = await provider.createNewChat(sessionA.sessionId); + const ownA = AgentSession.id(chatA.resource.toString()); + const sessionB = provider.createNewSession(URI.parse('file:///home/user/b'), sessionTypeId); + const chatB = await provider.createNewChat(sessionB.sessionId); + const ownB = AgentSession.id(chatB.resource.toString()); + + // Start both sends; each parks in `_waitForNewSession` (listSessions is + // empty because neither session has materialized yet). + const sendA = provider.sendRequest(sessionA.sessionId, chatA.resource, { query: 'A' }); + const sendB = provider.sendRequest(sessionB.sessionId, chatB.resource, { query: 'B' }); + await new Promise<void>(resolve => setTimeout(resolve, 10)); + + // The committed session keeps each send's own (eager) id. Materialize B + // FIRST, then A — the ordering that made A grab B's session. + fireSessionAdded(agentHost, ownB, { title: 'B' }); + await new Promise<void>(resolve => setTimeout(resolve, 10)); + fireSessionAdded(agentHost, ownA, { title: 'A' }); + + const [committedA, committedB] = await Promise.all([sendA, sendB]); + + assert.deepStrictEqual( + { a: AgentSession.id(committedA.resource.toString()), b: AgentSession.id(committedB.resource.toString()) }, + { a: ownA, b: ownB }, + ); + }); + test('sendRequest forwards resolved session config to chat service', async () => { const sendOptions: IChatSendRequestOptions[] = []; const provider = createProvider(disposables, agentHost, undefined, { diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostCustomizationHarness.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostCustomizationHarness.ts index 661e19af02a8e..5c4c08d860909 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostCustomizationHarness.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostCustomizationHarness.ts @@ -18,9 +18,8 @@ import { ActionType } from '../../../../../platform/agentHost/common/state/sessi import { ROOT_STATE_URI, customizationId, type Customization } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { IFileDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { INotificationService } from '../../../../../platform/notification/common/notification.js'; -import { AICustomizationManagementSection, AICustomizationSources, IAICustomizationWorkspaceService, type IStorageSourceFilter } from '../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection, IAICustomizationWorkspaceService } from '../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; import { ICustomizationSyncProvider, type IHarnessDescriptor, type ICustomizationItemAction } from '../../../../../workbench/contrib/chat/common/customizationHarnessService.js'; -import { PromptsType } from '../../../../../workbench/contrib/chat/common/promptSyntax/promptTypes.js'; import { AgentCustomizationItemProvider } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentCustomizationItemProvider.js'; import { CustomizationType } from '../../../../../platform/agentHost/common/state/protocol/state.js'; @@ -141,9 +140,6 @@ export function createRemoteAgentHarnessDescriptor( itemProvider: AgentCustomizationItemProvider, syncProvider: ICustomizationSyncProvider, ): IHarnessDescriptor { - const allSources = [AICustomizationSources.local, AICustomizationSources.user, AICustomizationSources.plugin, AICustomizationSources.extension, AICustomizationSources.builtin]; - const filter: IStorageSourceFilter = { sources: allSources }; - return { id: harnessId, label: displayName, @@ -153,9 +149,6 @@ export function createRemoteAgentHarnessDescriptor( AICustomizationManagementSection.McpServers, ], hideGenerateButton: true, - getStorageSourceFilter(_type: PromptsType): IStorageSourceFilter { - return filter; - }, itemProvider, syncProvider, pluginActions: controller.pluginActions, diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 1e42397af5a3b..24489718b06b7 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -27,7 +27,6 @@ import { ILabelService } from '../../../../../platform/label/common/label.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; import { INotificationService } from '../../../../../platform/notification/common/notification.js'; import { IStorageService } from '../../../../../platform/storage/common/storage.js'; -import { IProgressService } from '../../../../../platform/progress/common/progress.js'; import { IAgentHostActiveClientService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; import { IChatService } from '../../../../../workbench/contrib/chat/common/chatService/chatService.js'; @@ -151,9 +150,8 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid @IAgentHostActiveClientService activeClientService: IAgentHostActiveClientService, @IDialogService dialogService: IDialogService, @IWorkspaceTrustManagementService workspaceTrustManagementService: IWorkspaceTrustManagementService, - @IProgressService progressService: IProgressService, ) { - super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService, workspaceTrustManagementService, progressService); + super(chatSessionsService, chatService, chatWidgetService, languageModelsService, _configurationService, logService, gitHubService, instantiationService, sessionsService, activeClientService, storageService, dialogService, workspaceTrustManagementService); this._connectionAuthority = agentHostAuthority(config.address); this._connectOnDemand = config.connectOnDemand; diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts index 1c33236063356..2ac5fb94b0ae8 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostCustomizationHarness.test.ts @@ -21,7 +21,7 @@ import { PromptsType } from '../../../../../../workbench/contrib/chat/common/pro import { NullLogService } from '../../../../../../platform/log/common/log.js'; import { INotificationService } from '../../../../../../platform/notification/common/notification.js'; import { URI } from '../../../../../../base/common/uri.js'; -import { AICustomizationSources, IAICustomizationWorkspaceService } from '../../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; +import { IAICustomizationWorkspaceService } from '../../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; import { SYNCED_CUSTOMIZATION_SCHEME } from '../../../../../../workbench/services/agentHost/common/agentHostFileSystemService.js'; import { RemoteAgentPluginController } from '../../browser/remoteAgentHostCustomizationHarness.js'; import { CustomizationHarnessServiceBase, IHarnessDescriptor } from '../../../../../../workbench/contrib/chat/common/customizationHarnessService.js'; @@ -725,7 +725,6 @@ suite('RemoteAgentHostCustomizationHarness', () => { id: harnessId, label: 'Remote Agent Host (test)', icon: ThemeIcon.fromId(Codicon.remote.id), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.plugin] }), itemProvider: provider, }; const harnessService = disposables.add(new CustomizationHarnessServiceBase([descriptor], harnessId, new MockPromptsService())); diff --git a/src/vs/sessions/contrib/sessions/test/browser/aiCustomizationShortcutsWidget.fixture.ts b/src/vs/sessions/contrib/sessions/test/browser/aiCustomizationShortcutsWidget.fixture.ts index 20e49a2ccf163..b6e504b58317b 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/aiCustomizationShortcutsWidget.fixture.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/aiCustomizationShortcutsWidget.fixture.ts @@ -19,7 +19,7 @@ import { IAgentHostToolSetEnablementService, IToolEnablementState } from '../../ import { IAICustomizationItemsModel, ItemsModelSection } from '../../../../../workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemsModel.js'; import { ICustomizationHarnessService, IHarnessDescriptor } from '../../../../../workbench/contrib/chat/common/customizationHarnessService.js'; import { getChatSessionType } from '../../../../../workbench/contrib/chat/common/model/chatUri.js'; -import { AICustomizationManagementSection, AICustomizationSources } from '../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection } from '../../../../../workbench/contrib/chat/common/aiCustomizationWorkspaceService.js'; import { IAICustomizationListItem } from '../../../../../workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemSource.js'; import { AICustomizationShortcutsWidget } from '../../browser/aiCustomizationShortcutsWidget.js'; import { CUSTOMIZATION_ITEMS, CustomizationLinkViewItem, ICustomizationItemConfig } from '../../browser/customizationsToolbar.contribution.js'; @@ -163,7 +163,6 @@ function createMockHarnessService(hiddenSections: readonly string[] = []): ICust label: 'Fixture', icon: ThemeIcon.fromId('vm'), hiddenSections, - getStorageSourceFilter: () => ({ sources: AICustomizationSources.all }), }; return new class extends mock<ICustomizationHarnessService>() { override readonly activeSessionResource = observableValue('mockActiveSessionResource', URI.parse(`${descriptor.id}:///session`)); diff --git a/src/vs/sessions/sessions.common.main.ts b/src/vs/sessions/sessions.common.main.ts index 7a8decbf03469..c8313ff3a3bf8 100644 --- a/src/vs/sessions/sessions.common.main.ts +++ b/src/vs/sessions/sessions.common.main.ts @@ -457,6 +457,7 @@ import './browser/layoutActions.js'; import './contrib/accountMenu/browser/account.contribution.js'; import './contrib/aiCustomizationTreeView/browser/aiCustomizationTreeView.contribution.js'; import './contrib/chat/browser/chat.contribution.js'; +import './contrib/promptTimeline/browser/promptTimeline.contribution.js'; import './contrib/providers/agentHost/browser/exportDebugLogsAction.js'; import './contrib/providers/agentHost/browser/agentHostSessionConfigPicker.js'; import './contrib/chat/browser/customizationsDebugLog.contribution.js'; diff --git a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts index a42e2afdce8de..2ea8dcb67d92d 100644 --- a/src/vs/workbench/api/browser/mainThreadChatAgents2.ts +++ b/src/vs/workbench/api/browser/mainThreadChatAgents2.ts @@ -47,7 +47,7 @@ import { ExtHostChatAgentsShape2, ExtHostContext, IChatAgentInvokeResult, IChatS import { NotebookDto } from './mainThreadNotebookDto.js'; import { getChatSessionType, isUntitledChatSession } from '../../contrib/chat/common/model/chatUri.js'; import { ICustomizationHarnessService, ICustomizationItem, ICustomizationItemProvider, IHarnessDescriptor } from '../../contrib/chat/common/customizationHarnessService.js'; -import { AICustomizationManagementSection, AICustomizationSources } from '../../contrib/chat/common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection } from '../../contrib/chat/common/aiCustomizationWorkspaceService.js'; import { IAgentPlugin, IAgentPluginService } from '../../contrib/chat/common/plugins/agentPluginService.js'; import { IWorkbenchEnvironmentService } from '../../services/environment/common/environmentService.js'; @@ -841,11 +841,6 @@ export class MainThreadChatAgents2 extends Disposable implements MainThreadChatA label: metadata.label, icon: metadata.iconId ? ThemeIcon.fromId(metadata.iconId) : ThemeIcon.fromId(Codicon.extensions.id), hiddenSections, - getStorageSourceFilter: () => ({ - // Extension-provided harnesses manage their own items via the provider, - // so we show all sources for storage-filter-based flows. - sources: AICustomizationSources.all - }), itemProvider, }; diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 34056393bbef4..15c3d0787a75c 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -9,7 +9,7 @@ import { isObject, assertReturnsDefined } from '../../../../base/common/types.js import { ICodeEditor, IDiffEditor } from '../../../../editor/browser/editorBrowser.js'; import { IDiffEditorOptions, IEditorOptions as ICodeEditorOptions } from '../../../../editor/common/config/editorOptions.js'; import { AbstractTextEditor, IEditorConfiguration } from './textEditor.js'; -import { TEXT_DIFF_EDITOR_ID, IEditorFactoryRegistry, EditorExtensions, ITextDiffEditorPane, IEditorOpenContext, isEditorInput, isTextEditorViewState, createTooLargeFileError } from '../../../common/editor.js'; +import { TEXT_DIFF_EDITOR_ID, IEditorFactoryRegistry, EditorExtensions, ITextDiffEditorPane, IEditorOpenContext, isEditorInput, isEditorInputWithOptionsAndGroup, isTextEditorViewState, createTooLargeFileError } from '../../../common/editor.js'; import { EditorInput } from '../../../common/editor/editorInput.js'; import { applyTextEditorOptions } from '../../../common/editor/editorOptions.js'; import { DiffEditorInput } from '../../../common/editor/diffEditorInput.js'; @@ -25,6 +25,7 @@ import { Registry } from '../../../../platform/registry/common/platform.js'; import { URI } from '../../../../base/common/uri.js'; import { IEditorGroup, IEditorGroupsService } from '../../../services/editor/common/editorGroupsService.js'; import { IEditorService } from '../../../services/editor/common/editorService.js'; +import { IEditorResolverService } from '../../../services/editor/common/editorResolverService.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; import { EditorActivation, ITextEditorOptions } from '../../../../platform/editor/common/editor.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; @@ -68,7 +69,8 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp @IThemeService themeService: IThemeService, @IEditorGroupsService editorGroupService: IEditorGroupsService, @IFileService fileService: IFileService, - @IPreferencesService private readonly preferencesService: IPreferencesService + @IPreferencesService private readonly preferencesService: IPreferencesService, + @IEditorResolverService private readonly editorResolverService: IEditorResolverService ) { super(TextDiffEditor.ID, group, telemetryService, instantiationService, storageService, configurationService, themeService, editorService, editorGroupService, fileService); } @@ -117,7 +119,7 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp // Fallback to open as binary if not text if (!(resolvedModel instanceof TextDiffEditorModel)) { - this.openAsBinary(input, options); + await this.openAsBinary(input, options); return undefined; } @@ -207,10 +209,43 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp return false; } - private openAsBinary(input: DiffEditorInput, options: ITextEditorOptions | undefined): void { + private async openAsBinary(input: DiffEditorInput, options: ITextEditorOptions | undefined): Promise<void> { const original = input.original; const modified = input.modified; + // The text diff editor cannot render binary content. Before falling back to the generic binary + // "cannot display" panel, check whether a custom editor can render a diff for this resource and + // use it instead. This intentionally includes editors that opted out of diffs via a `never` + // priority: they opt out for text files, but a custom diff editor is strictly better than the + // binary fallback when the content is binary (e.g. an image or hex diff editor). + const modifiedResource = modified.resource; + if (modifiedResource) { + const fallbackEditorId = this.editorResolverService.getBinaryDiffFallbackEditor(modifiedResource); + const originalResource = original.resource; + if (fallbackEditorId && originalResource) { + const resolved = await this.editorResolverService.resolveEditor({ + original: { resource: originalResource }, + modified: { resource: modifiedResource }, + // Passing an explicit `override` bypasses the automatic `never` filtering and the diff + // special-casing, so the resolver returns the custom diff editor directly. + options: { ...options, override: fallbackEditorId } + }, this.group); + if (isEditorInputWithOptionsAndGroup(resolved)) { + this.group.replaceEditors([{ + editor: input, + replacement: resolved.editor, + options: { + ...resolved.options, + activation: EditorActivation.PRESERVE, + pinned: this.group.isPinned(input), + sticky: this.group.isSticky(input) + } + }]); + return; + } + } + } + const binaryDiffInput = this.instantiationService.createInstance(DiffEditorInput, input.getName(), input.getDescription(), original, modified, true); // Forward binary flag to input if supported diff --git a/src/vs/workbench/browser/workbench.contribution.ts b/src/vs/workbench/browser/workbench.contribution.ts index f2a3b35e6eac4..f8a45a27e05ca 100644 --- a/src/vs/workbench/browser/workbench.contribution.ts +++ b/src/vs/workbench/browser/workbench.contribution.ts @@ -364,7 +364,9 @@ const registry = Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Con ], 'description': localize('useModal', "Controls whether editors open in a modal overlay."), 'default': 'some', - agentsWindow: { default: 'all' }, + experiment: { + mode: 'startup' + } }, 'workbench.editor.swipeToNavigate': { 'type': 'boolean', diff --git a/src/vs/workbench/contrib/browserView/common/browserView.ts b/src/vs/workbench/contrib/browserView/common/browserView.ts index 58c2ac79c775b..cf6f1a0559bf6 100644 --- a/src/vs/workbench/contrib/browserView/common/browserView.ts +++ b/src/vs/workbench/contrib/browserView/common/browserView.ts @@ -261,8 +261,8 @@ export interface IBrowserViewWorkbenchService { * editor. Honors the `workbench.browser.newTabPlacement` setting, routing new * tabs into a dedicated (locked) side group or auxiliary window when * configured. When the workbench forces editors into a modal part - * (`workbench.editor.useModal: 'all'`, the default in the Agents window), - * browser opens that target the active group (or leave it unspecified) are + * (`workbench.editor.useModal: 'all'`), browser opens that target the active + * group (or leave it unspecified) are * redirected to the main editor area so the browser docks instead of opening * as a modal overlay. Explicit placements (side group, auxiliary window, a * specific group) are left untouched. diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts index 7e5b2724f74a6..1efe07f5eb1cb 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts @@ -243,10 +243,9 @@ export class BrowserViewWorkbenchService extends Disposable implements IBrowserV return this._getOrCreateDedicatedGroup(placement); } - // When editors are forced modal via `workbench.editor.useModal: 'all'` - // (the default in the Agents window), redirect active/unspecified browser - // opens to the main editor area so the browser docks instead of opening as - // a modal overlay. + // When editors are forced modal via `workbench.editor.useModal: 'all'`, + // redirect active/unspecified browser opens to the main editor area so the + // browser docks instead of opening as a modal overlay. if (this.configurationService.getValue<UseModalEditorMode>(USE_MODAL_EDITOR_SETTING) === 'all') { return this.editorGroupsService.mainPart.activeGroup; } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index c3fb424b42c40..4cacdbb268144 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -10,6 +10,7 @@ import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { localize } from '../../../../../../nls.js'; import { AgentHostEnabledSettingId, claudePreferAgentHostSettingId, IAgentHostService, shouldSurfaceLocalAgentHostProvider, type AgentProvider } from '../../../../../../platform/agentHost/common/agentService.js'; import { type ProtectedResourceMetadata } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { NotificationType } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import { type AgentInfo, type RootState } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; @@ -25,11 +26,12 @@ import { ICustomizationHarnessService } from '../../../common/customizationHarne import { ILanguageModelsService } from '../../../common/languageModels.js'; import { Target } from '../../../common/promptSyntax/promptTypes.js'; import { AgentCustomizationItemProvider } from './agentCustomizationItemProvider.js'; +import { AgentHostDownloadProgress } from './agentHostDownloadProgress.js'; import { authenticateProtectedResources, AgentHostAuthTokenCache, resolveAuthenticationInteractively } from './agentHostAuth.js'; import { AgentHostLanguageModelProvider, agentHostProviderSupportsAutoModel } from './agentHostLanguageModelProvider.js'; import { AgentHostSessionHandler } from './agentHostSessionHandler.js'; import { IAgentHostActiveClientService } from './agentHostActiveClientService.js'; -import { AICustomizationManagementSection, AICustomizationSources } from '../../../common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection } from '../../../common/aiCustomizationWorkspaceService.js'; const LOCAL_AGENT_HOST_SESSION_TYPE_PREFIX = 'agent-host-'; @@ -135,6 +137,21 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr this._authTokenCache.clear(); })); + // Surface the agent host's lazy, first-use SDK download as a progress + // notification. The Agents window renders this via its own sessions + // provider (`BaseAgentHostSessionsProvider`), so only wire it up here + // for regular editor windows to avoid duplicate notifications (this + // contribution runs in both windows). The matching `createSession` + // opt-in (`progressToken`) lives in the editor-window session handlers. + if (!this._isSessionsWindow) { + const downloadProgress = this._register(this._instantiationService.createInstance(AgentHostDownloadProgress)); + this._register(this._agentHostService.onDidNotification(n => { + if (n.type === NotificationType.Progress) { + downloadProgress.handleProgress(n); + } + })); + } + // Process initial root state if already available const initialRootState = this._agentHostService.rootState.value; if (initialRootState && !(initialRootState instanceof Error)) { @@ -251,7 +268,6 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr // The Tools section is surfaced for the Copilot CLI agent host only. hiddenSections: agent.provider === 'copilotcli' ? [AICustomizationManagementSection.Prompts] : [AICustomizationManagementSection.Tools, AICustomizationManagementSection.Prompts], hideGenerateButton: true, - getStorageSourceFilter: () => ({ sources: AICustomizationSources.all }), syncProvider, itemProvider, })); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostDownloadProgress.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostDownloadProgress.ts new file mode 100644 index 0000000000000..016c1ebf75bf3 --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostDownloadProgress.ts @@ -0,0 +1,128 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DeferredPromise } from '../../../../../../base/common/async.js'; +import { Disposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { localize } from '../../../../../../nls.js'; +import { type ProgressParams } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { IProgressService, IProgressStep, ProgressLocation } from '../../../../../../platform/progress/common/progress.js'; +import { ChatConfiguration } from '../../../common/constants.js'; + +/** + * One in-flight download tracked by {@link AgentHostDownloadProgress}. Owns the + * lifecycle of a single notification progress: `report` pushes a step, + * `complete` resolves the backing deferred so the notification is dismissed. + */ +interface IActiveDownload { + /** Last reported determinate percentage, used to compute progress increments. */ + lastPercent: number; + report(step: IProgressStep): void; + complete(): void; +} + +/** + * Renders agent-host `progress` notifications as notification progress bars. + * + * Shared by the Agents window (via `BaseAgentHostSessionsProvider`) and the + * editor window (via `AgentHostContribution`) so both surfaces render the + * agent host's lazy, first-use SDK download identically. + * + * Progress is correlated by {@link ProgressParams.progressToken}; today's only + * producer is the SDK download, which the host surfaces as a single stream per + * provider keyed by the download's own stable identity — so one indicator per + * download regardless of how many sessions await it. Determinate when the host + * knows the `total` (`Content-Length`), or a byte-count spinner otherwise. The + * operation is complete — and the notification dismissed — once + * `progress >= total`. The human-readable brand noun rides on + * {@link ProgressParams.message}. + */ +export class AgentHostDownloadProgress extends Disposable { + + /** + * Active progress indicators keyed by `progressToken`. The host emits a + * single stream per download keyed by the download's own stable identity + * (so distinct sessions of a provider share one indicator). Each entry owns + * one long-running notification progress (opened on the first frame), driven + * via {@link IActiveDownload.report} and dismissed via + * {@link IActiveDownload.complete} once `progress >= total`. + */ + private readonly _activeDownloads = new Map<string, IActiveDownload>(); + + constructor( + @IProgressService private readonly _progressService: IProgressService, + @IConfigurationService private readonly _configurationService: IConfigurationService, + ) { + super(); + this._register(toDisposable(() => { + for (const download of this._activeDownloads.values()) { + download.complete(); + } + this._activeDownloads.clear(); + })); + } + + handleProgress(progress: ProgressParams): void { + // New AI UI must stay hidden when the user has turned AI features off. + if (this._configurationService.getValue<boolean>(ChatConfiguration.AIDisabled)) { + return; + } + + // Complete when we reach the (possibly server-synthesized) total. The + // host emits a terminal frame with `progress === total` for success, + // indeterminate completion, and failure alike; real errors surface via + // the session-failure path, not here. + const isComplete = progress.total !== undefined && progress.progress >= progress.total; + if (isComplete) { + this._activeDownloads.get(progress.progressToken)?.complete(); + this._activeDownloads.delete(progress.progressToken); + return; + } + + let entry = this._activeDownloads.get(progress.progressToken); + if (!entry) { + // First frame for this download: open one long-running notification + // progress and drive it via `report` until a terminal frame resolves + // `deferred`. `message` is the host-supplied, already-localized title + // (e.g. "Downloading Claude agent"); render it verbatim so this stays + // a generic indicator that makes no assumption about what's downloading. + const deferred = new DeferredPromise<void>(); + let report: ((step: IProgressStep) => void) | undefined; + const title = progress.message ?? localize('agentHost.download.titleFallback', "Downloading"); + this._progressService.withProgress( + { + location: ProgressLocation.Notification, + title, + }, + p => { + report = step => p.report(step); + return deferred.p; + }, + ); + entry = { + lastPercent: 0, + report: step => report?.(step), + complete: () => deferred.complete(), + }; + this._activeDownloads.set(progress.progressToken, entry); + } + + if (progress.total && progress.total > 0) { + const percent = Math.max(0, Math.min(100, Math.round((progress.progress / progress.total) * 100))); + const increment = percent - entry.lastPercent; + entry.lastPercent = percent; + entry.report({ + message: localize('agentHost.download.percent', "{0}%", percent), + increment: increment > 0 ? increment : 0, + total: 100, + }); + } else { + // No total: indeterminate. Show megabytes received so the user + // still sees the download making progress. + const megabytes = (progress.progress / (1024 * 1024)).toFixed(1); + entry.report({ message: localize('agentHost.download.megabytes', "{0} MB", megabytes) }); + } + } +} diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostResponseFileChanges.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostResponseFileChanges.ts index 7e9e3cf30b431..eddde2c231272 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostResponseFileChanges.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostResponseFileChanges.ts @@ -124,9 +124,19 @@ export class AgentHostResponseFileChangesProvider extends Disposable implements ? toAgentHostUri(normalized.beforeContentUri, this._connectionAuthority) : modifiedURI; + // The frozen after-turn snapshot, when the changeset provides one. Lets + // consumers show this turn's diff (before-snapshot -> after-snapshot) + // rather than before-snapshot -> live file (which includes later turns). + // Distinct from the checkpoint-ref readability fix (#323932): that made + // these blobs readable; this line decides *which* snapshot to diff against. + const modifiedSnapshotURI = normalized.afterContentUri + ? toAgentHostUri(normalized.afterContentUri, this._connectionAuthority) + : undefined; + return { originalURI, modifiedURI, + modifiedSnapshotURI, added: file.edit.diff?.added ?? 0, removed: file.edit.diff?.removed ?? 0, quitEarly: false, 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 4bb7a240f3f20..dfa219b5b566f 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -19,6 +19,7 @@ import { extUriBiasedIgnorePathCase, isEqual } from '../../../../../../base/comm import { StopWatch } from '../../../../../../base/common/stopwatch.js'; import { Mutable } from '../../../../../../base/common/types.js'; import { URI } from '../../../../../../base/common/uri.js'; +import { generateUuid } from '../../../../../../base/common/uuid.js'; import { IPosition } from '../../../../../../editor/common/core/position.js'; import type { IRange } from '../../../../../../editor/common/core/range.js'; import { isLocation, type Location } from '../../../../../../editor/common/languages.js'; @@ -28,6 +29,7 @@ import { localize } from '../../../../../../nls.js'; import { AgentProvider, AgentSession, type IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; import { agentHostAuthority } from '../../../../../../platform/agentHost/common/agentHostUri.js'; import { AgentFeedbackAttachmentDisplayKind, AgentFeedbackAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/agentFeedbackAttachments.js'; +import { BrowserViewAttachmentDisplayKind, BrowserViewAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/browserViewAttachments.js'; import { readToolCallMeta } from '../../../../../../platform/agentHost/common/meta/agentToolCallMeta.js'; import { readCompletionAttachmentMeta } from '../../../../../../platform/agentHost/common/meta/agentCompletionAttachmentMeta.js'; import { IRemoteAgentHostService } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; @@ -54,6 +56,7 @@ import { AgentHostCompletionReferenceKind, getAgentHostCompletionReferenceKind, isAgentFeedbackVariableEntry, + isBrowserViewVariableEntry, isImageVariableEntry, type IAgentFeedbackVariableEntry, type IChatRequestVariableEntry, @@ -3474,6 +3477,12 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC const activeClient = this._getCurrentActiveClient(); + // Opt in to bring-up progress (chiefly the lazy first-use SDK download) + // so the editor window surfaces the same download notification the + // Agents window does. The host echoes the download's own identity on + // each frame; this token only records interest. + const progressToken = generateUuid(); + let session: URI; try { session = await this._config.connection.createSession({ @@ -3484,6 +3493,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC fork, config, activeClient, + progressToken, }); } catch (err) { // If authentication is required (e.g. token expired), try interactive auth and retry once @@ -3499,6 +3509,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC fork, config, activeClient, + progressToken, }); } else { throw new Error(localize('agentHost.authRequired', "Authentication is required to start a session. Please sign in and try again.")); @@ -4042,6 +4053,21 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC } return this._toSessionReferenceAttachments(v, v.value, trajectoryPath, referenceRange); } + // Browser views are live pages rather than filesystem resources. Preserve + // the page ID as model-readable context so the agent can address the page + // with browser tools without trying to read the vscode-browser URI. + if (isBrowserViewVariableEntry(v)) { + return this._toSimpleAttachment( + v.name, + v.modelDescription ?? `Browser page: ${v.name}. The pageId is "${v.browserId}".`, + { + ...v._meta, + [BrowserViewAttachmentMetadataKey]: { browserId: v.browserId, browserUri: v.value.toString() }, + }, + BrowserViewAttachmentDisplayKind, + referenceRange, + ); + } // Pasted code, prompt text, workspace context, and free-form string entries: surface their // textual representation as an opaque attachment. if (v.kind === 'paste') { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts index c95d5d0bbeea1..f53fd6706bfc6 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostUntitledProvisionalSessionService.ts @@ -54,6 +54,7 @@ import { Disposable } from '../../../../../../base/common/lifecycle.js'; import { ResourceMap, ResourceSet } from '../../../../../../base/common/map.js'; import { equals } from '../../../../../../base/common/objects.js'; import { URI } from '../../../../../../base/common/uri.js'; +import { generateUuid } from '../../../../../../base/common/uuid.js'; import { IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; import { KNOWN_AUTO_APPROVE_VALUES, KNOWN_MODE_VALUES, SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { migrateLegacyAutopilotConfig } from '../../../../../../platform/agentHost/common/agentHostSchema.js'; @@ -306,6 +307,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple session: backendSession, workingDirectory, config: initialConfig, + progressToken: generateUuid(), }); this._entries.set(sessionResource, { backendSession: created, config: { ...(initialConfig ?? {}) }, workingDirectory }); this._onDidChange.fire(sessionResource); @@ -376,6 +378,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple session: newBackendSession, workingDirectory, config, + progressToken: generateUuid(), }); } catch (err) { this._logService.warn(`[AgentHostProvisional] Failed to create rebound provisional: ${err instanceof Error ? err.message : String(err)}`); @@ -438,6 +441,7 @@ export class AgentHostUntitledProvisionalSessionService extends Disposable imple session: entry.backendSession, workingDirectory: newWorkingDirectory, config, + progressToken: generateUuid(), }); } catch (err) { this._logService.warn(`[AgentHostProvisional] Failed to recreate provisional at new cwd: ${err instanceof Error ? err.message : String(err)}`); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index 5993ec09aba72..b3b3ecd6aabbc 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -17,6 +17,7 @@ import { readToolCallMeta } from '../../../../../../platform/agentHost/common/me import { getChatErrorDetailsFromMeta, IChatErrorContext } from '../../../common/chatErrorMessages.js'; import { AGENT_HOST_SCHEME, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js'; import { getAgentFeedbackAttachmentMetadata, isAgentFeedbackAnnotationsAttachment, isAgentFeedbackAttachment } from '../../../../../../platform/agentHost/common/meta/agentFeedbackAttachments.js'; +import { getBrowserViewAttachmentMetadata, isBrowserViewAttachment } from '../../../../../../platform/agentHost/common/meta/browserViewAttachments.js'; import { isViewUnreviewedCommentsTool, isAddCommentTool } from '../../../../../../platform/agentHost/common/meta/agentFeedbackAnnotations.js'; import { MessageAttachmentKind, type FileEdit, type MessageAttachment, type StringOrMarkdown, type TextRange } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; import { normalizeFileEdit } from '../../../../../../platform/agentHost/common/fileEditDiff.js'; @@ -592,6 +593,20 @@ function messageAttachmentToVariableEntry(attachment: MessageAttachment, connect } const modelRepresentation = attachment.type === MessageAttachmentKind.Simple ? attachment.modelRepresentation : undefined; + if (isBrowserViewAttachment(attachment) && modelRepresentation !== undefined) { + const metadata = getBrowserViewAttachmentMetadata(attachment); + if (metadata) { + return { + kind: 'browserView', + id: metadata.browserUri, + name: attachment.label, + value: URI.parse(metadata.browserUri), + browserId: metadata.browserId, + modelDescription: modelRepresentation, + _meta: attachment._meta, + }; + } + } if (attachment.displayKind === 'workspace' && modelRepresentation !== undefined) { return { kind: 'workspace', diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemsModel.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemsModel.ts index df56233b72390..18e7a2ed0f790 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemsModel.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationItemsModel.ts @@ -249,7 +249,7 @@ export class AICustomizationItemsModel extends Disposable implements IAICustomiz } return new PureItemProviderItemSource(sessionResource, descriptor.itemProvider, this.itemNormalizer); } else { - const itemProvider = descriptor.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider, () => descriptor); + const itemProvider = descriptor.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider); return new ItemProviderItemSource( sessionResource, itemProvider, diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts index dc14fb48cf001..9e4b14ab829c2 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationManagementEditor.ts @@ -1265,7 +1265,7 @@ export class AICustomizationManagementEditor extends EditorPane { private async resolveTargetDirectoryWithPicker(type: PromptsType, target: 'workspace' | 'user'): Promise<URI | undefined | null> { const sessionResource = this.harnessService.activeSessionResource.get(); const activeDescriptor = this.harnessService.getActiveDescriptor(); - const provider = activeDescriptor.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider, () => activeDescriptor); + const provider = activeDescriptor.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider); if (!provider.provideSourceFolders) { return undefined; } diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationCreatorService.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationCreatorService.ts index 58396bbd09f5d..cdfbdc3ecbbcc 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationCreatorService.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/customizationCreatorService.ts @@ -151,7 +151,7 @@ export class CustomizationLocationPicker { public async resolveTargetDirectoryWithPicker(sessionResource: URI, type: PromptsType, target: 'local' | 'user'): Promise<URI | undefined | null> { const sessionType = getChatSessionType(sessionResource); const descriptor = this.harnessService.findHarnessById(sessionType); - const provider = descriptor?.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider, () => descriptor ?? this.harnessService.getActiveDescriptor()); + const provider = descriptor?.itemProvider ?? this.instantiationService.createInstance(PromptsServiceCustomizationItemProvider); if (!provider.provideSourceFolders) { return undefined; } diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts index a58862c2dccb6..6352284cb1316 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/promptsServiceCustomizationItemProvider.ts @@ -12,12 +12,12 @@ import { basename, dirname } from '../../../../../base/common/resources.js'; import { localize } from '../../../../../nls.js'; import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js'; import { IProductService } from '../../../../../platform/product/common/productService.js'; -import { IAICustomizationWorkspaceService, applySourceFilter, AICustomizationSources } from '../../common/aiCustomizationWorkspaceService.js'; +import { IAICustomizationWorkspaceService, AICustomizationSources } from '../../common/aiCustomizationWorkspaceService.js'; import { HookType, HOOK_METADATA } from '../../common/promptSyntax/hookTypes.js'; import { formatHookCommandLabel } from '../../common/promptSyntax/hookSchema.js'; import { PromptsType } from '../../common/promptSyntax/promptTypes.js'; import { ICustomAgent, IPromptsService, matchesSessionType, PromptsStorage } from '../../common/promptSyntax/service/promptsService.js'; -import { ICustomizationItem, ICustomizationItemProvider, ICustomizationSourceFolder, IHarnessDescriptor } from '../../common/customizationHarnessService.js'; +import { ICustomizationItem, ICustomizationItemProvider, ICustomizationSourceFolder } from '../../common/customizationHarnessService.js'; import { BUILTIN_STORAGE } from './aiCustomizationManagement.js'; import { getFriendlyName, isChatExtensionItem } from './aiCustomizationItemSource.js'; import { getChatSessionType } from '../../common/model/chatUri.js'; @@ -31,7 +31,6 @@ export class PromptsServiceCustomizationItemProvider implements ICustomizationIt readonly onDidChange: Event<void>; constructor( - private readonly getActiveDescriptor: () => IHarnessDescriptor, @IPromptsService private readonly promptsService: IPromptsService, @IAICustomizationWorkspaceService private readonly workspaceService: IAICustomizationWorkspaceService, @IProductService private readonly productService: IProductService, @@ -181,7 +180,7 @@ export class PromptsServiceCustomizationItemProvider implements ICustomizationIt await this.fetchPromptServiceInstructions(items, extensionInfoByUri, disabledUris, promptType); } - return this.applyLocalFilters(this.applyBuiltinGroupKeys(items, extensionInfoByUri), promptType); + return this.applyBuiltinGroupKeys(items, extensionInfoByUri); } private async fetchPromptServiceHooks(items: ICustomizationItem[], disabledUris: ResourceSet, promptType: PromptsType): Promise<void> { @@ -330,12 +329,4 @@ export class PromptsServiceCustomizationItemProvider implements ICustomizationIt }); } - private applyLocalFilters(groupedItems: ICustomizationItem[], promptType: PromptsType): readonly ICustomizationItem[] { - const descriptor = this.getActiveDescriptor(); - const filter = descriptor.getStorageSourceFilter(promptType); - const items = applySourceFilter(groupedItems, filter); - - return items; - } - } diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/toolsListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/toolsListWidget.ts index 6484b138e44d8..4ff7b9fb229c0 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/toolsListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/toolsListWidget.ts @@ -146,7 +146,7 @@ export class ToolsListWidget extends Disposable { private _searchRow!: HTMLElement; private _treeContainer!: HTMLElement; private _treeScrollable!: DomScrollableElement; - private _browseButtonContainer!: HTMLElement; + private _browseButtonContainer: HTMLElement | undefined; private _backButtonContainer!: HTMLElement; private _galleryContainer!: HTMLElement; private _galleryEmpty!: HTMLElement; @@ -262,11 +262,13 @@ export class ToolsListWidget extends Disposable { }).catch(() => { /* delayer disposed */ }); })); - const browseLabel = localize('toolsBrowseMarketplace', "Browse Marketplace"); - this._browseButtonContainer = DOM.append(this._searchRow, $('.tools-list-browse-button-container')); - const browseButton = this._register(new Button(this._browseButtonContainer, { ...defaultButtonStyles, secondary: true, supportIcons: true, title: browseLabel, ariaLabel: browseLabel })); - browseButton.label = `$(${Codicon.library.id}) ${browseLabel}`; - this._register(browseButton.onDidClick(() => this._setBrowseMode(true))); + if (!this._environmentService.isSessionsWindow) { + const browseLabel = localize('toolsBrowseMarketplace', "Browse Marketplace"); + this._browseButtonContainer = DOM.append(this._searchRow, $('.tools-list-browse-button-container')); + const browseButton = this._register(new Button(this._browseButtonContainer, { ...defaultButtonStyles, secondary: true, supportIcons: true, title: browseLabel, ariaLabel: browseLabel })); + browseButton.label = `$(${Codicon.library.id}) ${browseLabel}`; + this._register(browseButton.onDidClick(() => this._setBrowseMode(true))); + } const backLabel = localize('toolsBrowseBack', "Back"); this._backButtonContainer = DOM.append(this._searchRow, $('.tools-list-browse-button-container')); @@ -411,6 +413,9 @@ export class ToolsListWidget extends Disposable { /** Enters/leaves marketplace browse mode, swapping the tree for the gallery list. */ private _setBrowseMode(browse: boolean): void { + if (browse && this._environmentService.isSessionsWindow) { + return; + } if (this._browseMode === browse) { return; } @@ -418,7 +423,9 @@ export class ToolsListWidget extends Disposable { this._treeScrollable.getDomNode().style.display = browse ? 'none' : ''; this._galleryContainer.style.display = browse ? '' : 'none'; - this._browseButtonContainer.style.display = browse ? 'none' : ''; + if (this._browseButtonContainer) { + this._browseButtonContainer.style.display = browse ? 'none' : ''; + } this._backButtonContainer.style.display = browse ? '' : 'none'; this._searchInput.setPlaceHolder(browse diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatTerminalToolProgressPart.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatTerminalToolProgressPart.css index c6d93f1569b1a..e39285f05ccc5 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatTerminalToolProgressPart.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatTerminalToolProgressPart.css @@ -14,10 +14,14 @@ * single non-wrapping line. Each cell ellipsis-truncates and they split the * available width equally when the content overflows the row. */ -.chat-terminal-thinking-collapsible.chat-terminal-has-intention .chat-used-context-label, -.chat-terminal-thinking-collapsible.chat-terminal-has-intention .monaco-button.monaco-icon-button { +.chat-terminal-thinking-collapsible.chat-terminal-has-intention .chat-used-context-label { width: 100%; max-width: 100%; +} + +.chat-terminal-thinking-collapsible.chat-terminal-has-intention .monaco-button.monaco-icon-button { + width: fit-content; + max-width: 100%; justify-content: flex-start; } @@ -47,6 +51,10 @@ white-space: nowrap; } +.chat-terminal-label-flex > .chat-terminal-command > code { + white-space: nowrap; +} + .chat-terminal-label-flex > .chat-terminal-label-suffix { flex: 0 0 auto; white-space: nowrap; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts index 957e53bbbd3b0..3e9356f3943b7 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts @@ -1745,10 +1745,12 @@ export class ChatTerminalThinkingCollapsibleWrapper extends ChatCollapsibleConte const row = dom.$('span.chat-terminal-label-flex'); const intentionElement = dom.$('span.chat-terminal-intention'); intentionElement.textContent = this._intention; - const codeElement = dom.$('code.chat-terminal-command'); + const commandElement = dom.$('span.chat-terminal-command'); + const codeElement = document.createElement('code'); codeElement.textContent = this._commandText; + commandElement.appendChild(codeElement); row.appendChild(intentionElement); - row.appendChild(codeElement); + row.appendChild(commandElement); if (suffixText) { const suffixElement = dom.$('span.chat-terminal-label-suffix'); suffixElement.textContent = suffixText; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index c66f346f89621..fb9b68242922f 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -997,10 +997,18 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch const rowRoot = templateData.rowContainer.parentElement?.parentElement?.parentElement; rowRoot?.classList.toggle('request', isRequestVM(element)); rowRoot?.classList.toggle('response', isResponseVM(element)); - const isMostRecentChatTreeItem = getStickyScrollTargetItem(this.viewModel?.getItems() ?? []) === element; - templateData.rowContainer.classList.toggle(mostRecentResponseClassName, isMostRecentChatTreeItem); + // `.chat-most-recent-response` marks the actual last row. The reserved-space filler + // (`--chat-current-response-min-height`) is only applied to response rows via CSS, so + // when a queued/steering request row is last it gets no filler and the pending rows + // simply hug the streaming response above them. + templateData.rowContainer.classList.toggle(mostRecentResponseClassName, index === this.delegate.getListLength() - 1); templateData.rowContainer.classList.toggle('confirmation-message', isRequestVM(element) && !!element.confirmation); + // The streaming/progressive-rendering target is the last non-pending item, so the active + // response keeps rendering (and the view keeps following it) even when queued or steering + // rows are shown below it. + const isStickyScrollTargetItem = getStickyScrollTargetItem(this.viewModel?.getItems() ?? []) === element; + // TODO: @justschen decide if we want to hide the header for requests or not const shouldShowHeader = (isResponseVM(element) && !this.rendererOptions.noHeader) && !isSystemInitiatedRequest; templateData.header?.classList.toggle('header-disabled', !shouldShowHeader); @@ -1010,12 +1018,12 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch } // Do a progressive render if - // - This the last response in the list + // - This is the last non-pending response in the list // - And it has some content // - And the response is not complete // - Or, we previously started a progressive rendering of this element (if the element is complete, we will finish progressive rendering with a very fast rate) const incrementalRendering = this.configService.getValue<boolean>(ChatConfiguration.IncrementalRendering); - if (isResponseVM(element) && isMostRecentChatTreeItem && (!element.isComplete || element.renderData)) { + if (isResponseVM(element) && isStickyScrollTargetItem && (!element.isComplete || element.renderData)) { this.traceLayout('renderElement', `start progressive render, index=${index}`); if (incrementalRendering && !element.renderData) { diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts index bc7d2ae8f0ad9..e8f008abb8fc5 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts @@ -27,7 +27,7 @@ import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { IChatFollowup, IChatSendRequestOptions, IChatService } from '../../common/chatService/chatService.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../common/constants.js'; import { IChatRequestModeInfo } from '../../common/model/chatModel.js'; -import { getStickyScrollTargetItem, IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; +import { IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, isRequestVM, isResponseVM } from '../../common/model/chatViewModel.js'; import { ChatAccessibilityProvider } from '../accessibility/chatAccessibilityProvider.js'; import { ChatTreeItem, IChatAccessibilityService, IChatCodeBlockInfo, IChatFileTreeInfo, IChatListItemRendererOptions } from '../chat.js'; import { CodeBlockPart } from './chatContentParts/codeBlockPart.js'; @@ -182,7 +182,6 @@ export class ChatListWidget extends Disposable { private _viewModel: IChatViewModel | undefined; private _visible = true; private _lastItem: ChatTreeItem | undefined; - private _stickyScrollTargetItem: ChatTreeItem | undefined; private _mostRecentlyFocusedItemIndex: number = -1; private _scrollLock: boolean = true; private _suppressAutoScroll: boolean = false; @@ -240,10 +239,6 @@ export class ChatListWidget extends Disposable { return this._lastItem; } - get stickyScrollTargetItem(): ChatTreeItem | undefined { - return this._stickyScrollTargetItem; - } - //#endregion @@ -334,7 +329,8 @@ export class ChatListWidget extends Disposable { this._register(this._renderer.onDidChangeItemHeight(e => { this._updateElementHeight(e.element, e.height); - const secondToLastItem = this.getItemBeforeStickyScrollTarget(); + // If the second-to-last item's height changed, update the last item's min height + const secondToLastItem = this._viewModel?.getItems().at(-2); if (e.element.id === secondToLastItem?.id) { this.updateLastItemMinHeight(); } @@ -529,16 +525,14 @@ export class ChatListWidget extends Disposable { if (!this._viewModel) { this._tree.setChildren(null, []); this._lastItem = undefined; - this._stickyScrollTargetItem = undefined; this._lastItemIdContextKey.set([]); return; } const items = this._viewModel.getItems(); this._lastItem = items.at(-1); - this._stickyScrollTargetItem = getStickyScrollTargetItem(items); this._lastItemIdContextKey.set(this._lastItem ? [this._lastItem.id] : []); - const previousItem = this.getItemBeforeStickyScrollTarget(); + const previousItem = items.at(-2); const needsInitialPreviousItemHeight = (isRequestVM(previousItem) || isResponseVM(previousItem)) && previousItem.currentRenderedHeight === undefined; const treeItems: ITreeElement<ChatTreeItem>[] = items.map(item => ({ @@ -680,6 +674,18 @@ export class ChatListWidget extends Disposable { this._tree.reveal(element, relativeTop); } + /** + * The top offset of an element in transcript content space (same space as + * `scrollTop`/`scrollHeight`), or `undefined` if it is not in the list. Reads + * the layout height model, so it also resolves off-screen elements. + */ + getElementTop(element: ChatTreeItem): number | undefined { + if (!this._tree.hasElement(element)) { + return undefined; + } + return this._tree.getElementTop(element); + } + /** * Get the focused elements. */ @@ -728,11 +734,12 @@ export class ChatListWidget extends Disposable { * Scroll the list to reveal the last item. */ scrollToEnd(): void { - if (this._lastItem) { - const offset = Math.max(this._lastItem.currentRenderedHeight ?? 0, 1e6); - if (this._tree.hasElement(this._lastItem)) { - this._tree.reveal(this._lastItem, offset); - } + // Reveal the tree's actual last node rather than the held `_lastItem`. `reveal` reliably + // scrolls all the way down even while item heights are still settling (see #234089) + const lastElement = this._tree.getNode(null).children.at(-1)?.element; + if (lastElement) { + const offset = Math.max(lastElement.currentRenderedHeight ?? 0, 1e6); + this._tree.reveal(lastElement, offset); } } @@ -885,7 +892,7 @@ export class ChatListWidget extends Disposable { if (this._renderStyle === 'compact' || this._renderStyle === 'minimal') { this._container.style.removeProperty('--chat-current-response-min-height'); } else { - const secondToLastItem = this.getItemBeforeStickyScrollTarget(); + const secondToLastItem = this._viewModel?.getItems().at(-2); const maxRequestShownHeight = 200; const secondToLastItemHeight = Math.min( (isRequestVM(secondToLastItem) || isResponseVM(secondToLastItem)) ? @@ -895,7 +902,7 @@ export class ChatListWidget extends Disposable { this._container.style.setProperty('--chat-current-response-min-height', lastItemMinHeight + 'px'); if (lastItemMinHeight !== this._previousLastItemMinHeight) { this._previousLastItemMinHeight = lastItemMinHeight; - const lastItem = this._stickyScrollTargetItem; + const lastItem = this._viewModel?.getItems().at(-1); if (lastItem && this._visible && this._tree.hasElement(lastItem)) { this._updateElementHeight(lastItem, undefined); } @@ -903,15 +910,6 @@ export class ChatListWidget extends Disposable { } } - private getItemBeforeStickyScrollTarget(): ChatTreeItem | undefined { - const items = this._viewModel?.getItems(); - if (!items || !this._stickyScrollTargetItem) { - return undefined; - } - const targetIndex = items.indexOf(this._stickyScrollTargetItem); - return targetIndex > 0 ? items[targetIndex - 1] : undefined; - } - //#endregion } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index 05fcfa609fc5d..bb5199375c554 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -737,6 +737,10 @@ export class ChatWidget extends Disposable implements IChatWidget { this.listWidget.scrollTop = value; } + get scrollHeight(): number { + return this.listWidget.scrollHeight; + } + get attachmentModel(): ChatAttachmentModel { return this.input.attachmentModel; } @@ -2329,6 +2333,15 @@ export class ChatWidget extends Disposable implements IChatWidget { this.listWidget.reveal(item, relativeTop); } + /** + * The top offset of an item in transcript content space (same space as + * `scrollTop`/`scrollHeight`), or `undefined` if it is not in the list. + * Virtualization-safe for off-screen items (reads the layout height model). + */ + getElementTop(item: ChatTreeItem): number | undefined { + return this.listWidget.getElementTop(item); + } + focus(item: ChatTreeItem): void { if (!this.listWidget.hasElement(item)) { return; @@ -2961,7 +2974,7 @@ export class ChatWidget extends Disposable implements IChatWidget { const inputHeight = this._inputVisible ? this.inputPart.height.get() : 0; const lastElementVisible = this.listWidget.isScrolledToBottom; - const lastItem = this.listWidget.stickyScrollTargetItem; + const lastItem = this.listWidget.lastItem; const contentHeight = Math.max(0, height - inputHeight - chatSuggestNextWidgetHeight); this.listWidget.layout(contentHeight, width); diff --git a/src/vs/workbench/contrib/chat/common/aiCustomizationWorkspaceService.ts b/src/vs/workbench/contrib/chat/common/aiCustomizationWorkspaceService.ts index bc625c89990f7..7b013b579790e 100644 --- a/src/vs/workbench/contrib/chat/common/aiCustomizationWorkspaceService.ts +++ b/src/vs/workbench/contrib/chat/common/aiCustomizationWorkspaceService.ts @@ -8,7 +8,7 @@ import { IObservable } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js'; import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; import { PromptsType } from './promptSyntax/promptTypes.js'; -import { IChatPromptSlashCommand, PromptsStorage } from './promptSyntax/service/promptsService.js'; +import { IChatPromptSlashCommand } from './promptSyntax/service/promptsService.js'; export const IAICustomizationWorkspaceService = createDecorator<IAICustomizationWorkspaceService>('aiCustomizationWorkspaceService'); @@ -54,40 +54,11 @@ export type AICustomizationManagementSection = typeof AICustomizationManagementS * Per-type filter policy controlling which storage sources are visible * for a given customization type. */ -export interface IStorageSourceFilter { - /** - * Which storage groups to display (e.g. workspace, user, extension, builtin). - */ - readonly sources: readonly AICustomizationSource[]; -} - -/** - * Controls which features are shown on the welcome page of the - * AI Customization Management Editor. - */ export interface IWelcomePageFeatures { /** Show the "Configure Your AI" getting-started banner. */ readonly showGettingStartedBanner: boolean; } -/** - * Applies a source filter to an array of items that have uri and source. - * Removes items whose source is not in the filter's source list. - */ -export function applySourceFilter<T extends { readonly uri: URI; readonly source: AICustomizationSource }>(items: readonly T[], filter: IStorageSourceFilter): readonly T[] { - const sourceSet = new Set(filter.sources); - return items.filter(item => sourceSet.has(item.source)); -} - -/** - * Applies a storage filter to an array of items that have uri and storage. - * Removes items whose storage is not in the filter's source list. - */ -export function applyStorageSourceFilter<T extends { readonly uri: URI; readonly storage: PromptsStorage }>(items: readonly T[], filter: IStorageSourceFilter): readonly T[] { - const sourceSet = new Set(filter.sources); - return items.filter(item => sourceSet.has(item.storage)); -} - /** * Provides workspace context for AI Customization views. */ diff --git a/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts b/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts index c29b6a2ed2fc6..c29ef8ad2df53 100644 --- a/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts +++ b/src/vs/workbench/contrib/chat/common/customizationHarnessService.ts @@ -11,7 +11,7 @@ import { ThemeIcon } from '../../../../base/common/themables.js'; import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; -import { AICustomizationManagementSection, AICustomizationSource, AICustomizationSources, BUILTIN_STORAGE, IStorageSourceFilter } from './aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection, AICustomizationSource, BUILTIN_STORAGE } from './aiCustomizationWorkspaceService.js'; import { PromptsType } from './promptSyntax/promptTypes.js'; import { AGENT_MD_FILENAME } from './promptSyntax/config/promptFileLocations.js'; import { IAgentSource, IChatPromptSlashCommand, ICustomAgent, IPromptsService, IResolvedChatPromptSlashCommand, matchesSessionType, PromptsStorage } from './promptSyntax/service/promptsService.js'; @@ -107,11 +107,6 @@ export interface IHarnessDescriptor { * When `undefined`, the harness is always available (e.g. Local). */ readonly requiredAgentId?: string; - /** - * Returns the storage source filter that should be applied to customization - * items of the given type when this harness is active. - */ - getStorageSourceFilter(type: PromptsType): IStorageSourceFilter; /** * When set, this harness is backed by an extension-contributed provider * that can supply customization items directly (bypassing promptsService @@ -368,14 +363,7 @@ export interface ICustomizationSlashCommand { readonly sessionTypes?: readonly string[]; } -// #region Shared filter constants - -/** - * Empty filter returned when no harness is registered yet. - */ -const EMPTY_FILTER: IStorageSourceFilter = { - sources: [], -}; +// #region Shared descriptor constants /** * Empty descriptor returned when no harness is registered yet. @@ -384,7 +372,6 @@ const EMPTY_DESCRIPTOR: IHarnessDescriptor = { id: '', label: '', icon: Codicon.sparkle, - getStorageSourceFilter: () => EMPTY_FILTER, }; @@ -405,7 +392,6 @@ const EMPTY_DESCRIPTOR: IHarnessDescriptor = { * with no user-root restrictions. */ export function createVSCodeHarnessDescriptor(): IHarnessDescriptor { - const filter: IStorageSourceFilter = { sources: AICustomizationSources.all }; return { id: SessionType.Local, label: localize('harness.local', "Local"), @@ -417,7 +403,6 @@ export function createVSCodeHarnessDescriptor(): IHarnessDescriptor { rootFileShortcuts: [AGENT_MD_FILENAME], }], ]), - getStorageSourceFilter: () => filter, }; } diff --git a/src/vs/workbench/contrib/chat/common/editing/chatEditingService.ts b/src/vs/workbench/contrib/chat/common/editing/chatEditingService.ts index 2fec427255dd4..034d702c5969f 100644 --- a/src/vs/workbench/contrib/chat/common/editing/chatEditingService.ts +++ b/src/vs/workbench/contrib/chat/common/editing/chatEditingService.ts @@ -291,6 +291,20 @@ export interface IEditSessionEntryDiff extends IEditSessionDiffStats { originalURI: URI; modifiedURI: URI; + /** + * Optional frozen "after" content for the RHS. When set, this is the exact + * modified-side snapshot the diff represents (e.g. an agent-host per-turn + * checkpoint), as opposed to {@link modifiedURI} which may be the live + * working file and therefore include later changes. Consumers that want the + * changeset's own diff should prefer this when present; {@link modifiedURI} + * remains the file's identity for labels and go-to-file. + * + * Note: distinct from the agent-host checkpoint-ref readability fix (#323932). + * That made the frozen snapshot blobs *readable*; this field carries *which* + * snapshot to diff against so a per-turn review shows only that turn's changes. + */ + modifiedSnapshotURI?: URI; + /** Diff state information: */ quitEarly: boolean; identical: boolean; diff --git a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts index f48b40c050079..9ca8b73e07cf9 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatViewModel.ts @@ -42,6 +42,11 @@ function isPendingChatViewModelItem(item: IChatViewModelItemWithPendingState): b return item.kind === 'pendingDivider' || item.pendingKind !== undefined; } +/** + * The active response that content streams into: the last non-pending item, ignoring + * trailing queued/steering rows (and their dividers). Falls back to the last item when + * everything is pending. + */ export function getStickyScrollTargetItem<T extends IChatViewModelItemWithPendingState>(items: readonly T[]): T | undefined { for (let i = items.length - 1; i >= 0; i--) { const item = items[i]; @@ -668,7 +673,12 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi } get isLast(): boolean { - return getStickyScrollTargetItem(this.session.getItems()) === this; + // NOTE: this is used in `dataId` to force a re-render when the response transitions + // between being the last row and not, e.g. when a queued/steering row is added below + // it. It must reflect the actual last row so the row re-renders and drops the + // reserved-space filler class. Progressive rendering targets the streaming response + // separately (see `getStickyScrollTargetItem`). + return this.session.getItems().at(-1) === this; } renderData: IChatResponseRenderData | undefined = undefined; diff --git a/src/vs/workbench/contrib/chat/test/browser/agentHost/agentHostResponseFileChanges.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentHost/agentHostResponseFileChanges.test.ts index 7929a699f0749..7719494db4864 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentHost/agentHostResponseFileChanges.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentHost/agentHostResponseFileChanges.test.ts @@ -12,6 +12,7 @@ import { mock } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; import { buildTurnChangesetUri } from '../../../../../../platform/agentHost/common/changesetUri.js'; +import { fromAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js'; import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import { ChangesetStatus, StateComponents, type ChangesetState, type SessionState } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { IEditSessionEntryDiff } from '../../../common/editing/chatEditingService.js'; @@ -93,9 +94,15 @@ suite('AgentHostResponseFileChangesProvider', () => { } satisfies ChangesetState); const { latest } = observe(provider, ds); - assert.deepStrictEqual(latest().map(d => ({ added: d.added, removed: d.removed, modified: d.modifiedURI.path })), [ - { added: 3, removed: 1, modified: '/repo/a.ts' }, - { added: 5, removed: 0, modified: '/repo/b.ts' }, + assert.deepStrictEqual(latest().map(d => ({ + added: d.added, + removed: d.removed, + modified: d.modifiedURI.path, + // The RHS diff content is the frozen after-turn snapshot, not the live file. + after: d.modifiedSnapshotURI && fromAgentHostUri(d.modifiedSnapshotURI).authority, + })), [ + { added: 3, removed: 1, modified: '/repo/a.ts', after: 'a-after' }, + { added: 5, removed: 0, modified: '/repo/b.ts', after: 'b-after' }, ]); }); 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 6110eb91ea704..248c9047c5c8f 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 @@ -23,6 +23,7 @@ import { IConfigurationService } from '../../../../../../platform/configuration/ import { IAgentCreateSessionConfig, IAgentHostService, IAgentSessionMetadata, AgentSession } from '../../../../../../platform/agentHost/common/agentService.js'; import type { ChatInputRequestWithPlanReview } from '../../../../../../platform/agentHost/common/agentHostPlanReview.js'; import { AgentFeedbackAttachmentDisplayKind, AgentFeedbackAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/agentFeedbackAttachments.js'; +import { BrowserViewAttachmentDisplayKind, BrowserViewAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/browserViewAttachments.js'; import { ActionType, isSessionAction, isChatAction, type ActionEnvelope, type IRootConfigChangedAction, type SessionAction, type ChatAction, type TerminalAction, type INotification, type IToolCallConfirmedAction, type ITurnStartedAction, type ClientAnnotationsAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import type { IStateSnapshot } from '../../../../../../platform/agentHost/common/state/sessionProtocol.js'; import { CustomizationType, type ClientPluginCustomization, type ToolDefinition } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; @@ -30,6 +31,7 @@ import { ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, import { CompletionItemKind as AhpCompletionItemKind, type CompletionsParams, type CompletionsResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { sessionReducer, chatReducer } from '../../../../../../platform/agentHost/common/state/sessionReducers.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; +import { IProgress, IProgressNotificationOptions, IProgressService, IProgressStep } from '../../../../../../platform/progress/common/progress.js'; import { IAuthenticationService } from '../../../../../services/authentication/common/authentication.js'; import { ChatEntitlement, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; import { IChatAgentData, IChatAgentImplementation, IChatAgentRequest, IChatAgentService } from '../../../common/participants/chatAgents.js'; @@ -642,6 +644,7 @@ function createTestServices(disposables: DisposableStore, workingDirectoryResolv ...languageModelToolsServiceOverride, }); instantiationService.stub(IOutputService, { getChannel: () => undefined }); + instantiationService.stub(IProgressService, { withProgress: <R,>(_options: IProgressNotificationOptions, task: (progress: IProgress<IProgressStep>) => Promise<R>) => task({ report: () => { } }) }); instantiationService.stub(IWorkspaceContextService, { getWorkspace: () => ({ id: '', folders: [] }), getWorkspaceFolder: () => null, onDidChangeWorkspaceFolders: Event.None }); const trustController: { result: boolean | undefined; workspaceTrustCalls: number; resourcesTrustCalls: number } = { result: true, workspaceTrustCalls: 0, resourcesTrustCalls: 0 }; instantiationService.stub(IWorkspaceTrustRequestService, new class extends mock<IWorkspaceTrustRequestService>() { @@ -971,6 +974,43 @@ suite('AgentHostChatContribution', () => { }); }); + // ---- Download progress notification (editor window) ----------------- + + suite('download progress', () => { + + function createWithProgressRecorder(isSessionsWindow: boolean) { + // AI features must be enabled (AIDisabled === false) or the download + // progress handler suppresses the notification; the default config + // stub returns `true` for every key, so override just this one. + const services = createTestServices(disposables, undefined, undefined, undefined, undefined, isSessionsWindow, undefined, { [ChatConfiguration.AIDisabled]: false }); + const openedTitles: (string | undefined)[] = []; + services.instantiationService.stub(IProgressService, { + withProgress: <R,>(options: IProgressNotificationOptions, task: (progress: IProgress<IProgressStep>) => Promise<R>) => { + openedTitles.push(options.title); + return task({ report: () => { } }); + }, + }); + disposables.add(services.instantiationService.createInstance(AgentHostContribution)); + return { agentHostService: services.agentHostService, openedTitles }; + } + + test('editor window renders SDK download progress from root/progress notifications', () => { + const { agentHostService, openedTitles } = createWithProgressRecorder(false); + + agentHostService.fireNotification({ type: 'root/progress', channel: 'ahp-root://root', progressToken: 'claude', progress: 0, total: 1000, message: 'Downloading Claude agent' }); + + assert.deepStrictEqual(openedTitles, ['Downloading Claude agent']); + }); + + test('sessions window does not render download progress via the chat contribution', () => { + const { agentHostService, openedTitles } = createWithProgressRecorder(true); + + agentHostService.fireNotification({ type: 'root/progress', channel: 'ahp-root://root', progressToken: 'claude', progress: 0, total: 1000, message: 'Downloading Claude agent' }); + + assert.strictEqual(openedTitles.length, 0); + }); + }); + // ---- Request attachments -------------------------------------------- suite('request attachments', () => { @@ -4885,6 +4925,52 @@ suite('AgentHostChatContribution', () => { ]); })); + test('browser view variable becomes a model-readable browser attachment', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); + const browserUri = URI.from({ scheme: 'vscode-browser', path: '/page-1' }); + + const { turnPromise, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables, { + message: 'inspect this page', + variables: { + variables: [ + upcastPartial({ + kind: 'browserView', + id: browserUri.toString(), + name: 'Example page', + value: browserUri, + browserId: 'page-1', + modelDescription: 'Browser page: Example. The pageId is "page-1".', + }), + ], + }, + }); + fire({ type: 'chat/turnComplete', session, turnId } as ChatAction); + await turnPromise; + + assert.strictEqual(agentHostService.turnActions.length, 1); + const turnAction = agentHostService.turnActions[0].action as ITurnStartedAction; + assert.deepStrictEqual(turnAction.message.attachments, [{ + type: MessageAttachmentKind.Simple, + label: 'Example page', + modelRepresentation: 'Browser page: Example. The pageId is "page-1".', + displayKind: BrowserViewAttachmentDisplayKind, + _meta: { [BrowserViewAttachmentMetadataKey]: { browserId: 'page-1', browserUri: browserUri.toString() } }, + }]); + + const replayedVariables = messageAttachmentsToVariableData(turnAction.message.attachments, 'test')?.variables; + assert.strictEqual(replayedVariables?.length, 1); + assert.strictEqual(replayedVariables?.[0].value?.toString(), browserUri.toString()); + assert.deepStrictEqual({ ...replayedVariables?.[0], value: undefined }, { + kind: 'browserView', + id: browserUri.toString(), + name: 'Example page', + value: undefined, + browserId: 'page-1', + modelDescription: 'Browser page: Example. The pageId is "page-1".', + _meta: { [BrowserViewAttachmentMetadataKey]: { browserId: 'page-1', browserUri: browserUri.toString() } }, + }); + })); + test('preserves _meta from variable entry on outgoing attachment', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostDownloadProgress.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostDownloadProgress.test.ts new file mode 100644 index 0000000000000..460bcd0539214 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostDownloadProgress.test.ts @@ -0,0 +1,94 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { type ProgressParams } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; +import { IProgress, IProgressNotificationOptions, IProgressService, IProgressStep } from '../../../../../../platform/progress/common/progress.js'; +import { ChatConfiguration } from '../../../common/constants.js'; +import { AgentHostDownloadProgress } from '../../../browser/agentSessions/agentHost/agentHostDownloadProgress.js'; + +interface IRecordedProgress { + title: string | undefined; + readonly steps: IProgressStep[]; + dismissed: boolean; + /** Resolves once the backing notification promise settles (i.e. is dismissed). */ + settled: Promise<void>; +} + +/** Records every `withProgress` invocation, the steps reported into it, and whether it resolved. */ +class RecordingProgressService { + readonly opened: IRecordedProgress[] = []; + + withProgress(options: IProgressNotificationOptions, task: (progress: IProgress<IProgressStep>) => Promise<unknown>): Promise<unknown> { + const record: IRecordedProgress = { title: options.title, steps: [], dismissed: false, settled: Promise.resolve() }; + this.opened.push(record); + const result = task({ report: step => { record.steps.push(step); } }); + record.settled = result.then(() => { record.dismissed = true; }, () => { record.dismissed = true; }); + return result; + } +} + +class FakeConfigurationService { + constructor(private readonly _aiDisabled: boolean) { } + getValue(key: string): unknown { + return key === ChatConfiguration.AIDisabled ? this._aiDisabled : undefined; + } +} + +function frame(partial: Partial<ProgressParams> & Pick<ProgressParams, 'progressToken' | 'progress'>): ProgressParams { + return { channel: 'ahp-root://root', ...partial }; +} + +suite('AgentHostDownloadProgress', () => { + + const store = ensureNoDisposablesAreLeakedInTestSuite(); + + function create(aiDisabled = false) { + const progressService = new RecordingProgressService(); + const configurationService = new FakeConfigurationService(aiDisabled); + const controller = store.add(new AgentHostDownloadProgress( + progressService as unknown as IProgressService, + configurationService as unknown as IConfigurationService, + )); + return { controller, progressService }; + } + + test('determinate download opens one notification, reports percent, dismisses on terminal frame', async () => { + const { controller, progressService } = create(); + + controller.handleProgress(frame({ progressToken: 'claude', progress: 0, total: 1000, message: 'Downloading Claude agent' })); + controller.handleProgress(frame({ progressToken: 'claude', progress: 500, total: 1000, message: 'Downloading Claude agent' })); + controller.handleProgress(frame({ progressToken: 'claude', progress: 1000, total: 1000, message: 'Downloading Claude agent' })); + + // The terminal frame resolves the notification promise asynchronously. + await progressService.opened[0].settled; + + assert.deepStrictEqual( + progressService.opened.map(o => ({ title: o.title, steps: o.steps.map(s => s.message), dismissed: o.dismissed })), + [{ title: 'Downloading Claude agent', steps: ['0%', '50%'], dismissed: true }], + ); + }); + + test('indeterminate download (no total) reports megabytes received', () => { + const { controller, progressService } = create(); + + controller.handleProgress(frame({ progressToken: 'codex', progress: 5 * 1024 * 1024, message: 'Downloading Codex agent' })); + + assert.deepStrictEqual( + progressService.opened.map(o => ({ title: o.title, steps: o.steps.map(s => s.message) })), + [{ title: 'Downloading Codex agent', steps: ['5.0 MB'] }], + ); + }); + + test('no notification when AI features are disabled', () => { + const { controller, progressService } = create(true); + + controller.handleProgress(frame({ progressToken: 'claude', progress: 0, total: 1000, message: 'Downloading Claude agent' })); + + assert.strictEqual(progressService.opened.length, 0); + }); +}); diff --git a/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationItemsModel.test.ts b/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationItemsModel.test.ts index 490e47f41f4c1..3f780412e220d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationItemsModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationItemsModel.test.ts @@ -16,7 +16,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/ import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { workbenchInstantiationService } from '../../../../../test/browser/workbenchTestServices.js'; import { AICustomizationItemsModel } from '../../../browser/aiCustomization/aiCustomizationItemsModel.js'; -import { AICustomizationManagementSection, AICustomizationSources, BUILTIN_STORAGE, IAICustomizationWorkspaceService, IStorageSourceFilter } from '../../../common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection, AICustomizationSources, BUILTIN_STORAGE, IAICustomizationWorkspaceService } from '../../../common/aiCustomizationWorkspaceService.js'; import { ICustomizationHarnessService, ICustomizationItem, ICustomizationItemProvider, ICustomizationSyncProvider, IHarnessDescriptor } from '../../../common/customizationHarnessService.js'; import { ContributionEnablementState } from '../../../common/enablement.js'; import { IAgentPluginService, type IAgentPlugin } from '../../../common/plugins/agentPluginService.js'; @@ -49,7 +49,6 @@ suite('AICustomizationItemsModel', () => { id, label: id, icon: Codicon.settingsGear, - getStorageSourceFilter: (): IStorageSourceFilter => ({ sources: [PromptsStorage.local, PromptsStorage.user] }), itemProvider: provider, syncProvider, }; @@ -539,7 +538,6 @@ suite('AICustomizationItemsModel', () => { id: 'A', label: 'A', icon: Codicon.settingsGear, - getStorageSourceFilter: (): IStorageSourceFilter => ({ sources: [PromptsStorage.local, PromptsStorage.user] }), itemProvider: provider, }; const sessionResource = URI.parse('A:///active-session'); @@ -780,7 +778,6 @@ suite('AICustomizationItemsModel', () => { id: sessionType, label: 'Agent Host Test', icon: Codicon.settingsGear, - getStorageSourceFilter: (): IStorageSourceFilter => ({ sources: [] }), itemProvider: provider, }; const sessionResource = URI.parse(`${sessionType}:///active-session`); diff --git a/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationListWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationListWidget.test.ts index a863b1026ef19..4f04bb2c641b0 100644 --- a/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationListWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/aiCustomization/aiCustomizationListWidget.test.ts @@ -16,12 +16,12 @@ import { workbenchInstantiationService } from '../../../../../test/browser/workb import { AICustomizationListWidget } from '../../../browser/aiCustomization/aiCustomizationListWidget.js'; import { IAICustomizationItemsModel } from '../../../browser/aiCustomization/aiCustomizationItemsModel.js'; import { extractExtensionIdFromPath, getCustomizationSecondaryText, truncateToFirstLine } from '../../../browser/aiCustomization/aiCustomizationListWidgetUtils.js'; -import { AICustomizationManagementSection, IAICustomizationWorkspaceService, IStorageSourceFilter } from '../../../common/aiCustomizationWorkspaceService.js'; +import { AICustomizationManagementSection, IAICustomizationWorkspaceService } from '../../../common/aiCustomizationWorkspaceService.js'; import { ICustomizationHarnessService, IHarnessDescriptor } from '../../../common/customizationHarnessService.js'; import { ContributionEnablementState } from '../../../common/enablement.js'; import { getChatSessionType } from '../../../common/model/chatUri.js'; import { IAgentPluginService } from '../../../common/plugins/agentPluginService.js'; -import { IPromptsService, PromptsStorage } from '../../../common/promptSyntax/service/promptsService.js'; +import { IPromptsService } from '../../../common/promptSyntax/service/promptsService.js'; import { PromptsType } from '../../../common/promptSyntax/promptTypes.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { ResourceSet } from '../../../../../../base/common/map.js'; @@ -164,7 +164,6 @@ suite('aiCustomizationListWidget', () => { id: 'test', label: 'Test', icon: Codicon.settingsGear, - getStorageSourceFilter: (): IStorageSourceFilter => ({ sources: [PromptsStorage.local, PromptsStorage.user] }), itemProvider: { onDidChange: Event.None, provideChatSessionCustomizations: (sessionResource: URI, token: CancellationToken) => Promise.resolve(undefined), diff --git a/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts b/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts index 600215bcfc63c..59b59d2928c00 100644 --- a/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/customizationHarnessService.test.ts @@ -14,7 +14,6 @@ import { ICustomAgent, IPromptsService, PromptsStorage } from '../../common/prom import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { SessionType } from '../../common/chatSessionsService.js'; import { MockPromptsService } from './promptSyntax/service/mockPromptsService.js'; -import { AICustomizationSources } from '../../common/aiCustomizationWorkspaceService.js'; suite('CustomizationHarnessService', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -47,7 +46,6 @@ suite('CustomizationHarnessService', () => { id: harnessId, label: 'Test Harness', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -73,7 +71,6 @@ suite('CustomizationHarnessService', () => { id: harnessId, label: 'Test Harness', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -100,7 +97,6 @@ suite('CustomizationHarnessService', () => { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -122,7 +118,6 @@ suite('CustomizationHarnessService', () => { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -144,7 +139,6 @@ suite('CustomizationHarnessService', () => { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -168,7 +162,6 @@ suite('CustomizationHarnessService', () => { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -190,12 +183,10 @@ suite('CustomizationHarnessService', () => { const service = createService(); const emitter = new Emitter<void>(); store.add(emitter); - const customFilter = { sources: [PromptsStorage.local, PromptsStorage.user] }; const externalDescriptor: IHarnessDescriptor = { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => customFilter, itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -205,7 +196,6 @@ suite('CustomizationHarnessService', () => { store.add(service.registerExternalHarness(externalDescriptor)); service.setActiveSession(activeSessionResource); - assert.deepStrictEqual(service.getActiveDescriptor().getStorageSourceFilter(PromptsType.agent), customFilter); }); test('external harness item provider returns items', async () => { @@ -225,7 +215,6 @@ suite('CustomizationHarnessService', () => { id: 'test-ext', label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider, }; const activeSessionResource = URI.parse('test-ext://session'); @@ -246,7 +235,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), }; const service = createService( createVSCodeHarnessDescriptor(), @@ -260,7 +248,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (from API)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -281,7 +268,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), }; const service = createService( createVSCodeHarnessDescriptor(), @@ -294,7 +280,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (from API)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -315,7 +300,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (static)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), }; const service = createService( createVSCodeHarnessDescriptor(), @@ -328,7 +312,6 @@ suite('CustomizationHarnessService', () => { id: 'cli', label: 'Copilot CLI (from API)', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [], @@ -360,7 +343,6 @@ suite('CustomizationHarnessService', () => { id: testSessionType, label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [ @@ -390,7 +372,6 @@ suite('CustomizationHarnessService', () => { id: testSessionType, label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.plugin] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [ @@ -481,7 +462,6 @@ suite('CustomizationHarnessService', () => { id: testSessionType1, label: 'Test Extension', icon: ThemeIcon.fromId('extensions'), - getStorageSourceFilter: () => ({ sources: [AICustomizationSources.local] }), itemProvider: { onDidChange: emitter.event, provideChatSessionCustomizations: async (_sessionResource: URI, _token: CancellationToken) => [ diff --git a/src/vs/workbench/contrib/customEditor/common/contributedCustomEditors.ts b/src/vs/workbench/contrib/customEditor/common/contributedCustomEditors.ts index 4d49c3a3ba5e7..7a8458964de61 100644 --- a/src/vs/workbench/contrib/customEditor/common/contributedCustomEditors.ts +++ b/src/vs/workbench/contrib/customEditor/common/contributedCustomEditors.ts @@ -108,12 +108,12 @@ function normalizeStoredCustomEditorDescriptor(descriptor: StoredCustomEditorDes selector: descriptor.selector, priority: typeof descriptor.priority === 'string' ? { editor: descriptor.priority, - diff: descriptor.priority, - merge: descriptor.priority, + diff: RegisteredEditorPriority.never, + merge: RegisteredEditorPriority.never, } : { editor: descriptor.priority.editor, - diff: descriptor.priority.diff ?? descriptor.priority.editor, - merge: descriptor.priority.merge ?? descriptor.priority.editor, + diff: descriptor.priority.diff ?? RegisteredEditorPriority.never, + merge: descriptor.priority.merge ?? RegisteredEditorPriority.never, }, }; } @@ -123,11 +123,16 @@ function getPriorityFromContribution( extension: IExtensionDescription, includeDiffAndMergePriority: boolean, ): CustomEditorDescriptor['priority'] { + // The `textEditor` value drives the normal editor and keeps its historical `default` fallback when + // omitted. `diffEditor` and `mergeEditor` do not inherit from `textEditor`: they default to + // `never`, so a custom editor is not used for diffs or merges unless it explicitly opts in (which + // requires the `customEditorPriority` proposal). const editorPriority = getSinglePriorityFromContribution(typeof contribution === 'string' ? contribution : contribution?.textEditor, extension) ?? RegisteredEditorPriority.default; + const readObjectField = includeDiffAndMergePriority && typeof contribution !== 'string'; return { editor: editorPriority, - diff: includeDiffAndMergePriority && typeof contribution !== 'string' ? getSinglePriorityFromContribution(contribution?.diffEditor, extension) ?? editorPriority : editorPriority, - merge: includeDiffAndMergePriority && typeof contribution !== 'string' ? getSinglePriorityFromContribution(contribution?.mergeEditor, extension) ?? editorPriority : editorPriority, + diff: (readObjectField ? getSinglePriorityFromContribution(contribution?.diffEditor, extension) : undefined) ?? RegisteredEditorPriority.never, + merge: (readObjectField ? getSinglePriorityFromContribution(contribution?.mergeEditor, extension) : undefined) ?? RegisteredEditorPriority.never, }; } @@ -139,6 +144,9 @@ function getSinglePriorityFromContribution(value: CustomEditorPriority | undefin case CustomEditorPriority.option: return RegisteredEditorPriority.option; + case CustomEditorPriority.never: + return RegisteredEditorPriority.never; + case CustomEditorPriority.builtin: // Builtin is only valid for builtin extensions return extension.isBuiltin ? RegisteredEditorPriority.builtin : RegisteredEditorPriority.default; diff --git a/src/vs/workbench/contrib/customEditor/common/customEditor.ts b/src/vs/workbench/contrib/customEditor/common/customEditor.ts index bc39cb2309fcf..0f5eacdd03903 100644 --- a/src/vs/workbench/contrib/customEditor/common/customEditor.ts +++ b/src/vs/workbench/contrib/customEditor/common/customEditor.ts @@ -81,6 +81,7 @@ export const enum CustomEditorPriority { default = 'default', builtin = 'builtin', option = 'option', + never = 'never', } export const enum CustomEditorDiffEditorLayout { diff --git a/src/vs/workbench/contrib/customEditor/common/extensionPoint.ts b/src/vs/workbench/contrib/customEditor/common/extensionPoint.ts index 47bf896dceb9f..1f6e0cf391a4e 100644 --- a/src/vs/workbench/contrib/customEditor/common/extensionPoint.ts +++ b/src/vs/workbench/contrib/customEditor/common/extensionPoint.ts @@ -33,10 +33,12 @@ const customEditorPrioritySchema = { enum: [ CustomEditorPriority.default, CustomEditorPriority.option, + CustomEditorPriority.never, ], markdownEnumDescriptions: [ nls.localize('contributes.priority.default', 'The editor is automatically used when the user opens a resource, provided that no other default custom editors are registered for that resource.'), nls.localize('contributes.priority.option', 'The editor is not automatically used when the user opens a resource, but a user can switch to the editor using the `Reopen With` command.'), + nls.localize('contributes.priority.never', 'The editor is never automatically used, and it is also skipped when the user points a `workbench.editorAssociations` entry at it. It can still be opened via the `Reopen With` command or the specialized `workbench.diffEditorAssociations` setting.'), ], } as const satisfies IJSONSchema; @@ -77,7 +79,7 @@ const customEditorsContributionSchema = { } }, [Fields.priority]: { - markdownDescription: nls.localize('contributes.priority', 'Controls if the custom editor is enabled automatically when the user opens a file, diff, or merge editor. This may be overridden by users using the `workbench.editorAssociations` or `workbench.diffEditorAssociations` setting.'), + markdownDescription: nls.localize('contributes.priority', 'Controls if the custom editor is enabled automatically when the user opens a file, diff, or merge editor. This may be overridden by users using the `workbench.editorAssociations` or `workbench.diffEditorAssociations` setting. When omitted, the custom editor defaults to `default` for the normal editor and `never` for diff and merge editors, so it is not used for diffs or merges unless it opts in.'), anyOf: [ customEditorPrioritySchema, { @@ -87,15 +89,15 @@ const customEditorsContributionSchema = { properties: { [PriorityFields.textEditor]: { ...customEditorPrioritySchema, - markdownDescription: nls.localize('contributes.priority.textEditor', 'Controls if the custom editor is enabled automatically when the user opens a file. This is the base value: when `diffEditor` or `mergeEditor` are not specified, they fall back to this value.'), + markdownDescription: nls.localize('contributes.priority.textEditor', 'Controls if the custom editor is enabled automatically when the user opens a file. `diffEditor` and `mergeEditor` do not inherit this value; when they are not specified they default to `never`.'), }, [PriorityFields.diffEditor]: { ...customEditorPrioritySchema, - markdownDescription: nls.localize('contributes.priority.diffEditor', 'Controls if the custom editor is enabled automatically when the user opens a diff. When not specified, the value of `textEditor` is used.'), + markdownDescription: nls.localize('contributes.priority.diffEditor', 'Controls if the custom editor is enabled automatically when the user opens a diff. When not specified this defaults to `never`, so the custom editor is not used for diffs unless it opts in.'), }, [PriorityFields.mergeEditor]: { ...customEditorPrioritySchema, - markdownDescription: nls.localize('contributes.priority.mergeEditor', 'Controls if the custom editor is enabled automatically when the user opens a merge editor. When not specified, the value of `textEditor` is used.'), + markdownDescription: nls.localize('contributes.priority.mergeEditor', 'Controls if the custom editor is enabled automatically when the user opens a merge editor. When not specified this defaults to `never`, so the custom editor is not used for merges unless it opts in.'), }, } } diff --git a/src/vs/workbench/contrib/issue/electron-browser/nativeIssueFormService.ts b/src/vs/workbench/contrib/issue/electron-browser/nativeIssueFormService.ts index 08ba8050337db..cfea1a4f8d172 100644 --- a/src/vs/workbench/contrib/issue/electron-browser/nativeIssueFormService.ts +++ b/src/vs/workbench/contrib/issue/electron-browser/nativeIssueFormService.ts @@ -75,11 +75,11 @@ export class NativeIssueFormService extends IssueFormService implements IIssueFo // the editor pane, so it does not depend on arch/release/type here. const input = this.instantiationService.createInstance(IssueReporterEditorInput, data); - // In the Agents window, editors open in a floating modal editor part by - // default (`workbench.editor.useModal: 'all'`). The issue reporter needs to - // sit alongside the rest of the app so the user can capture screenshots and - // recordings, so target the main editor part's active group explicitly to - // open it docked in the editor area instead of as a modal overlay. + // When editors are forced modal (`workbench.editor.useModal: 'all'`), the + // issue reporter still needs to sit alongside the rest of the app so the + // user can capture screenshots and recordings. In the Agents window, target + // the main editor part's active group explicitly to open it docked in the + // editor area instead of as a modal overlay. const preferredGroup = data.isSessionsWindow ? this.editorGroupService.mainPart.activeGroup : undefined; await this.editorService.openEditor(input, { pinned: true }, preferredGroup); } diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellToolbarStickyScroll.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellToolbarStickyScroll.ts index aee39b19d5959..91bb9698c13a1 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellToolbarStickyScroll.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellToolbarStickyScroll.ts @@ -12,6 +12,13 @@ export function registerCellToolbarStickyScroll(notebookEditor: INotebookEditor, const min = opts?.min ?? 0; const updateForScroll = () => { + // Re-resolve the captured cell against the editor's current view model. The + // scroll listener can outlive the cell's membership (e.g. the cell was removed + // or the pooled editor widget was reattached to a different notebook), in which + // case `getAbsoluteTopOfElement` below would throw an "Invalid index -1" ListError. + if (notebookEditor.getCellByHandle(cell.handle) !== cell) { + return; + } if (cell.isInputCollapsed) { element.style.top = ''; } else { diff --git a/src/vs/workbench/services/editor/browser/editorResolverService.ts b/src/vs/workbench/services/editor/browser/editorResolverService.ts index 8a8b5caefe18c..2c3610a26b740 100644 --- a/src/vs/workbench/services/editor/browser/editorResolverService.ts +++ b/src/vs/workbench/services/editor/browser/editorResolverService.ts @@ -61,6 +61,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver // Constants private static readonly configureDefaultID = 'promptOpenWith.configureDefault'; + private static readonly configureDefaultDiffID = 'promptOpenWith.configureDefaultDiff'; private static readonly cacheStorageID = 'editorOverrideService.cache'; private static readonly conflictingDefaultsStorageID = 'editorOverrideService.conflictingDefaults'; @@ -283,6 +284,8 @@ export class EditorResolverService extends Disposable implements IEditorResolver } private getAssociationsForResourceByType(resource: URI, associationType: EditorAssociationType): EditorAssociations { + // The specialized diff/merge associations win over the general ones and are allowed to target + // an editor even if that editor opted out of diffs/merges through a `never` priority. if (associationType === EditorAssociationType.DiffEditor || associationType === EditorAssociationType.MergeEditor) { const diffAssociations = this.getAssociationsForResourceFromSetting(resource, diffEditorsAssociationsSettingId); if (diffAssociations.length) { @@ -290,7 +293,21 @@ export class EditorResolverService extends Disposable implements IEditorResolver } } - return this.getAssociationsForResource(resource); + // General `editorAssociations` entries must not select an editor that opted out of this kind of + // input via a `never` priority (e.g. a custom editor that only handles the normal editor, not diffs). + const r = this.getAssociationsForResource(resource); + return r.filter(association => !this.isNeverForAssociationType(association.viewType, associationType)); + } + + /** + * Whether every editor registered under `viewType` opts out of the given kind of input via a + * `never` priority. Such editors can only be selected explicitly (e.g. `Reopen Editor With`) or + * through the specialized `workbench.diffEditorAssociations` setting, never through a general + * `workbench.editorAssociations` entry. + */ + private isNeverForAssociationType(viewType: string, associationType: EditorAssociationType): boolean { + const editor = this._registeredEditors.filter(editor => editor.editorInfo.id === viewType).at(0); + return !!editor && this.getEffectivePriority(editor.editorInfo, associationType) === RegisteredEditorPriority.never; } private getAssociationsForResourceFromSetting(resource: URI, settingId: string): EditorAssociations { @@ -409,6 +426,20 @@ export class EditorResolverService extends Disposable implements IEditorResolver this.configurationService.updateValue(settingId, newSettingObject); } + private removeUserAssociationForSetting(settingId: string, globPattern: string): void { + const currentAssociations = this.getAllUserAssociationsForSetting(settingId); + if (!currentAssociations.some(association => association.filenamePattern === globPattern)) { + return; + } + const newSettingObject = Object.create(null); + for (const association of currentAssociations) { + if (association.filenamePattern && association.filenamePattern !== globPattern) { + newSettingObject[association.filenamePattern] = association.viewType; + } + } + this.configurationService.updateValue(settingId, newSettingObject); + } + private findMatchingEditors(resource: URI, associationType: EditorAssociationType = EditorAssociationType.Editor): RegisteredEditor[] { // The user setting should be respected even if the editor doesn't specify that resource in package.json const userSettings = this.getAssociationsForResourceByType(resource, associationType); @@ -457,6 +488,20 @@ export class EditorResolverService extends Disposable implements IEditorResolver return distinct(this._registeredEditors.map(editor => editor.editorInfo), editor => editor.id); } + getBinaryDiffFallbackEditor(resource: URI): string | undefined { + this._flattenedEditors = this._flattenEditorsMap(); + + // `findMatchingEditors(..., DiffEditor)` only keeps editors that provide a diff editor factory + // and sorts them by their diff priority. It still includes `never` editors (they match by glob), + // which is exactly what we want here: a `never` editor opts out of diffs for text files, but is + // the better choice than the generic binary fallback when the text diff editor cannot render the + // content. We exclude the built-in default text editor since that is the editor that already + // failed to render the binary content. + const editors = this.findMatchingEditors(resource, EditorAssociationType.DiffEditor) + .filter(editor => editor.editorInfo.id !== DEFAULT_EDITOR_ASSOCIATION.id); + return editors[0]?.editorInfo.id; + } + /** * Given a resource and an editorId selects the best possible editor * @returns The editor and whether there was another default which conflicted with it @@ -784,11 +829,20 @@ export class EditorResolverService extends Disposable implements IEditorResolver label: localize('promptOpenWith.configureDefault', "Configure default editor for '{0}'...", `*${extname(resource)}`), }; quickPickEntries.push(configureDefaultEntry); + // For diffs, additionally offer to configure a diff-only default so the choice does not + // affect how the resource opens as a normal editor (writes to `diffEditorAssociations`). + if (associationType === EditorAssociationType.DiffEditor) { + const configureDefaultDiffEntry = { + id: EditorResolverService.configureDefaultDiffID, + label: localize('promptOpenWith.configureDefaultDiff', "Configure default editor (diff only) for '{0}'...", `*${extname(resource)}`), + }; + quickPickEntries.push(configureDefaultDiffEntry); + } } return quickPickEntries; } - private async doPickEditor(editor: IUntypedEditorInput, showDefaultPicker?: boolean): Promise<IEditorOptions | undefined> { + private async doPickEditor(editor: IUntypedEditorInput, showDefaultPicker?: boolean, updateAssociationType?: EditorAssociationType): Promise<IEditorOptions | undefined> { type EditorPick = { readonly item: IQuickPickItem; @@ -802,6 +856,21 @@ export class EditorResolverService extends Disposable implements IEditorResolver resource = URI.from({ scheme: Schemas.untitled }); } const associationType = isResourceDiffEditorInput(editor) ? EditorAssociationType.DiffEditor : EditorAssociationType.Editor; + // Which setting the default picker should write to. Defaults to the resource's association type + // so that the per-item gear button keeps writing to the matching setting, but the "Configure + // default editor" entries can target a specific setting (general vs. diff-only). + const updateSettingType = updateAssociationType ?? associationType; + + // Persists the picked editor as the default for this resource's glob. When the user configures + // the general default from a diff context, any diff-only override for the same glob is cleared + // so that the general default also takes effect for diffs. + const persistDefaultAssociation = (editorID: string) => { + const globPattern = `*${extname(resource)}`; + this.updateUserAssociationsForType(updateSettingType, globPattern, editorID); + if (updateSettingType === EditorAssociationType.Editor && associationType === EditorAssociationType.DiffEditor) { + this.removeUserAssociationForSetting(diffEditorsAssociationsSettingId, globPattern); + } + }; // Get all the editors for the resource as quickpick entries const editorPicks = this.mapEditorsToQuickPickEntry(resource, showDefaultPicker, associationType); @@ -810,7 +879,9 @@ export class EditorResolverService extends Disposable implements IEditorResolver const disposables = new DisposableStore(); const editorPicker = disposables.add(this.quickInputService.createQuickPick<IQuickPickItem>({ useSeparators: true })); const placeHolderMessage = showDefaultPicker ? - localize('promptOpenWith.updateDefaultPlaceHolder', "Select new default editor for '{0}'", `*${extname(resource)}`) : + (updateSettingType === EditorAssociationType.DiffEditor ? + localize('promptOpenWith.updateDefaultDiffPlaceHolder', "Select new default editor (diff only) for '{0}'", `*${extname(resource)}`) : + localize('promptOpenWith.updateDefaultPlaceHolder', "Select new default editor for '{0}'", `*${extname(resource)}`)) : localize('promptOpenWith.placeHolder', "Select editor for '{0}'", basename(resource)); editorPicker.placeholder = placeHolderMessage; editorPicker.canAcceptInBackground = true; @@ -835,7 +906,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver // If asked to always update the setting then update it even if the gear isn't clicked if (resource && showDefaultPicker && result?.item.id) { - this.updateUserAssociationsForType(associationType, `*${extname(resource)}`, result.item.id); + persistDefaultAssociation(result.item.id); } resolve(result); @@ -853,7 +924,7 @@ export class EditorResolverService extends Disposable implements IEditorResolver // Persist setting if (resource && e.item?.id) { - this.updateUserAssociationsForType(associationType, `*${extname(resource)}`, e.item.id); + persistDefaultAssociation(e.item.id); } })); @@ -870,7 +941,12 @@ export class EditorResolverService extends Disposable implements IEditorResolver // If the user selected to configure default we trigger this picker again and tell it to show the default picker if (picked.item.id === EditorResolverService.configureDefaultID) { - return this.doPickEditor(editor, true); + return this.doPickEditor(editor, true, EditorAssociationType.Editor); + } + // The diff-only variant writes to `diffEditorAssociations` so it does not change how the + // resource opens as a normal editor. + if (picked.item.id === EditorResolverService.configureDefaultDiffID) { + return this.doPickEditor(editor, true, EditorAssociationType.DiffEditor); } // Figure out options diff --git a/src/vs/workbench/services/editor/common/editorResolverService.ts b/src/vs/workbench/services/editor/common/editorResolverService.ts index 34e2bbe610a4c..2a6544b70cd2e 100644 --- a/src/vs/workbench/services/editor/common/editorResolverService.ts +++ b/src/vs/workbench/services/editor/common/editorResolverService.ts @@ -105,7 +105,15 @@ export enum RegisteredEditorPriority { builtin = 'builtin', option = 'option', exclusive = 'exclusive', - default = 'default' + default = 'default', + /** + * The editor is never automatically used for this kind of input, and it is + * also skipped when the user points a `workbench.editorAssociations` entry at + * it. Unlike `option`, a `never` editor is only used when it is the target of + * the specialized `workbench.diffEditorAssociations` setting or when the user + * explicitly opens it (for example via `Reopen Editor With`). + */ + never = 'never' } /** @@ -244,6 +252,16 @@ export interface IEditorResolverService { */ getEditors(): RegisteredEditorInfo[]; + /** + * Returns the id of the best editor that can render a *diff* for the resource, excluding the + * built-in default text editor. This intentionally includes editors that opted out of diffs via a + * `never` priority: such editors opt out for text files, but when the default text diff editor + * cannot render the content (e.g. it is binary) a custom diff editor is preferable to the generic + * "cannot display" fallback. Returns `undefined` when no such (diff-capable) editor exists. + * @param resource The resource being diffed + */ + getBinaryDiffFallbackEditor(resource: URI): string | undefined; + /** * Get a complete list of editor associations. */ @@ -263,6 +281,9 @@ export function priorityToRank(priority: RegisteredEditorPriority): number { return 3; // Text editor is priority 2 case RegisteredEditorPriority.option: + return 1; + case RegisteredEditorPriority.never: + return 0; default: return 1; } diff --git a/src/vs/workbench/services/editor/common/editorService.ts b/src/vs/workbench/services/editor/common/editorService.ts index 3ba7a2e65381f..3527e7ed84cad 100644 --- a/src/vs/workbench/services/editor/common/editorService.ts +++ b/src/vs/workbench/services/editor/common/editorService.ts @@ -49,7 +49,7 @@ export const USE_MODAL_EDITOR_SETTING = 'workbench.editor.useModal'; * Possible values for the `workbench.editor.useModal` setting: * - `'off'`: never open editors modal (user opt-out, honored over `RequiresModal`) * - `'some'`: open modal only for editors that request it (e.g. `RequiresModal`) - * - `'all'`: open all editors modal (the default in the Agents window) + * - `'all'`: open all editors modal */ export type UseModalEditorMode = 'off' | 'some' | 'all'; diff --git a/src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts b/src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts index 2055733576b80..0883de52895fd 100644 --- a/src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts +++ b/src/vs/workbench/services/editor/test/browser/editorResolverService.test.ts @@ -348,6 +348,204 @@ suite('EditorResolverService', () => { diffAssociationRegisteredEditor.dispose(); }); + test('Diff editor Resolve - editorAssociations does not force a `never` diff editor', async () => { + const DEFAULT_DIFF_INPUT_ID = 'testDefaultDiffInput'; + const NEVER_DIFF_INPUT_ID = 'testNeverDiffInput'; + const instantiationService = workbenchInstantiationService({ + configurationService: () => new TestConfigurationService({ + [editorsAssociationsSettingId]: { + '*.test-never-diff': 'NEVER_DIFF_EDITOR' + } + }) + }, disposables); + const [part, service, accessor] = await createEditorResolverService(instantiationService); + let defaultDiffCounter = 0; + let neverDiffCounter = 0; + + const defaultRegisteredEditor = service.registerEditor('*', + { + id: 'default', + label: 'Default Editor', + detail: 'Default', + priority: RegisteredEditorPriority.builtin + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, TEST_EDITOR_INPUT_ID, disposables) }), + createDiffEditorInput: ({ modified, original }) => { + defaultDiffCounter++; + return { editor: constructDisposableDiffEditorInput(accessor, original, modified, DEFAULT_DIFF_INPUT_ID) }; + } + } + ); + + // An editor that handles the normal editor but explicitly opts out of diffs via a `never` priority. + const neverDiffRegisteredEditor = service.registerEditor('*.test-never-diff', + { + id: 'NEVER_DIFF_EDITOR', + label: 'Never Diff Editor Label', + detail: 'Never Diff Editor Details', + priority: { + editor: RegisteredEditorPriority.option, + diff: RegisteredEditorPriority.never, + merge: RegisteredEditorPriority.never + } + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, NEVER_DIFF_INPUT_ID, disposables) }), + createDiffEditorInput: ({ modified, original }) => { + neverDiffCounter++; + return { editor: constructDisposableDiffEditorInput(accessor, original, modified, NEVER_DIFF_INPUT_ID) }; + } + } + ); + + // The diff must fall back to the default diff editor, not the `never` editor. + const diffResolution = await service.resolveEditor({ + original: { resource: URI.file('resource-basics.test-never-diff') }, + modified: { resource: URI.file('resource-basics.test-never-diff') } + }, part.activeGroup); + assert.ok(diffResolution); + assert.notStrictEqual(typeof diffResolution, 'number'); + if (diffResolution !== ResolvedStatus.ABORT && diffResolution !== ResolvedStatus.NONE) { + assert.strictEqual(neverDiffCounter, 0); + assert.strictEqual(defaultDiffCounter, 1); + diffResolution.editor.dispose(); + } else { + assert.fail(); + } + + // The normal editor association is still honored (`editor` priority is `option`, not `never`). + const editorResolution = await service.resolveEditor({ resource: URI.file('resource-basics.test-never-diff') }, part.activeGroup); + assert.ok(editorResolution); + assert.notStrictEqual(typeof editorResolution, 'number'); + if (editorResolution !== ResolvedStatus.ABORT && editorResolution !== ResolvedStatus.NONE) { + assert.strictEqual(editorResolution.editor.typeId, NEVER_DIFF_INPUT_ID); + editorResolution.editor.dispose(); + } else { + assert.fail(); + } + + defaultRegisteredEditor.dispose(); + neverDiffRegisteredEditor.dispose(); + }); + + test('Diff editor Resolve - diffEditorAssociations force a `never` diff editor', async () => { + const DEFAULT_DIFF_INPUT_ID = 'testDefaultDiffInput'; + const NEVER_DIFF_INPUT_ID = 'testNeverDiffInput'; + const instantiationService = workbenchInstantiationService({ + configurationService: () => new TestConfigurationService({ + [diffEditorsAssociationsSettingId]: { + '*.test-never-diff': 'NEVER_DIFF_EDITOR' + } + }) + }, disposables); + const [part, service, accessor] = await createEditorResolverService(instantiationService); + let defaultDiffCounter = 0; + let neverDiffCounter = 0; + + const defaultRegisteredEditor = service.registerEditor('*', + { + id: 'default', + label: 'Default Editor', + detail: 'Default', + priority: RegisteredEditorPriority.builtin + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, TEST_EDITOR_INPUT_ID, disposables) }), + createDiffEditorInput: ({ modified, original }) => { + defaultDiffCounter++; + return { editor: constructDisposableDiffEditorInput(accessor, original, modified, DEFAULT_DIFF_INPUT_ID) }; + } + } + ); + + const neverDiffRegisteredEditor = service.registerEditor('*.test-never-diff', + { + id: 'NEVER_DIFF_EDITOR', + label: 'Never Diff Editor Label', + detail: 'Never Diff Editor Details', + priority: { + editor: RegisteredEditorPriority.option, + diff: RegisteredEditorPriority.never, + merge: RegisteredEditorPriority.never + } + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, NEVER_DIFF_INPUT_ID, disposables) }), + createDiffEditorInput: ({ modified, original }) => { + neverDiffCounter++; + return { editor: constructDisposableDiffEditorInput(accessor, original, modified, NEVER_DIFF_INPUT_ID) }; + } + } + ); + + // The specialized diff association forces the `never` editor even though it opted out of diffs. + const diffResolution = await service.resolveEditor({ + original: { resource: URI.file('resource-basics.test-never-diff') }, + modified: { resource: URI.file('resource-basics.test-never-diff') } + }, part.activeGroup); + assert.ok(diffResolution); + assert.notStrictEqual(typeof diffResolution, 'number'); + if (diffResolution !== ResolvedStatus.ABORT && diffResolution !== ResolvedStatus.NONE) { + assert.strictEqual(defaultDiffCounter, 0); + assert.strictEqual(neverDiffCounter, 1); + diffResolution.editor.dispose(); + } else { + assert.fail(); + } + + defaultRegisteredEditor.dispose(); + neverDiffRegisteredEditor.dispose(); + }); + + test('getBinaryDiffFallbackEditor returns a diff-capable `never` editor and ignores non-diff editors', async () => { + const [, service] = await createEditorResolverService(); + + // A custom editor that opts out of diffs (`never`) but *does* provide a diff editor factory. + const neverWithDiff = service.registerEditor('*.bin', + { + id: 'BINARY_EDITOR', + label: 'Binary Editor', + detail: 'Binary Editor Details', + priority: { + editor: RegisteredEditorPriority.default, + diff: RegisteredEditorPriority.never, + merge: RegisteredEditorPriority.never + } + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, 'binaryInput', disposables) }), + createDiffEditorInput: ({ modified, original }) => ({ editor: constructDisposableFileEditorInput(modified.resource ?? original.resource!, 'binaryDiffInput', disposables) }) + } + ); + + // A custom editor that provides no diff factory must never be used as a binary diff fallback. + const noDiff = service.registerEditor('*.noDiff', + { + id: 'NO_DIFF_EDITOR', + label: 'No Diff Editor', + detail: 'No Diff Editor Details', + priority: RegisteredEditorPriority.default + }, + {}, + { + createEditorInput: ({ resource }) => ({ editor: constructDisposableFileEditorInput(resource, 'noDiffInput', disposables) }) + } + ); + + assert.strictEqual(service.getBinaryDiffFallbackEditor(URI.file('file.bin')), 'BINARY_EDITOR'); + assert.strictEqual(service.getBinaryDiffFallbackEditor(URI.file('file.noDiff')), undefined); + assert.strictEqual(service.getBinaryDiffFallbackEditor(URI.file('file.unrelated')), undefined); + + neverWithDiff.dispose(); + noDiff.dispose(); + }); + test('Diff editor Resolve - Different Types', async () => { const [part, service, accessor] = await createEditorResolverService(); let diffOneCounter = 0; diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatTerminalCollapsible.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatTerminalCollapsible.fixture.ts index 4ee3ec58189db..227decea621da 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatTerminalCollapsible.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatTerminalCollapsible.fixture.ts @@ -40,10 +40,10 @@ function renderCollapsible(context: ComponentFixtureContext, commandText: string container.style.width = '500px'; container.style.padding = '8px'; - container.classList.add('monaco-workbench'); + container.classList.add('monaco-workbench', 'interactive-session'); - const session = dom.$('.interactive-session'); - container.appendChild(session); + const itemContainer = dom.$('.interactive-item-container'); + container.appendChild(itemContainer); const contentElement = dom.$('.chat-terminal-output-placeholder'); contentElement.textContent = '(terminal output would appear here)'; @@ -64,7 +64,7 @@ function renderCollapsible(context: ComponentFixtureContext, commandText: string undefined, )); - session.appendChild(wrapper.domNode); + itemContainer.appendChild(wrapper.domNode); } export default defineThemedFixtureGroup({ path: 'chat/terminalCollapsible/' }, { @@ -98,6 +98,12 @@ export default defineThemedFixtureGroup({ path: 'chat/terminalCollapsible/' }, { 'Ran - with intention': defineComponentFixture({ render: ctx => renderCollapsible(ctx, 'ls -lh', false, true, false, false, 'List files in the repo root'), }), + 'Height parity - with and without intention': defineComponentFixture({ + render: ctx => { + renderCollapsible(ctx, 'ls -lh', false, true); + renderCollapsible(ctx, 'ls -lh', false, true, false, false, 'List files in the repo root'); + }, + }), 'Running - with intention': defineComponentFixture({ render: ctx => renderCollapsible(ctx, 'npm test', false, false, false, false, 'Run the test suite'), }),