diff --git a/docs/superpowers/plans/2026-05-26-deepseek-cache-diagnostics.md b/docs/superpowers/plans/2026-05-26-deepseek-cache-diagnostics.md new file mode 100644 index 00000000000..3f97286c6e1 --- /dev/null +++ b/docs/superpowers/plans/2026-05-26-deepseek-cache-diagnostics.md @@ -0,0 +1,770 @@ +# DeepSeek Cache Diagnostics Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add privacy-preserving OpenAI-compatible tool cache-stability diagnostics so DeepSeek users can identify tool set, order, schema, and serialization drift. + +**Architecture:** Extend the existing disabled-by-default `runtimeDiagnostics` collector with pure hashing helpers and an optional `cacheStability` summary on OpenAI wire request diagnostics. Pass provider context from the OpenAI pipeline using hostname-based DeepSeek detection, then document how to interpret the hashes. + +**Tech Stack:** TypeScript, Vitest, Node `crypto`, Qwen Code core/cli workspaces, Markdown docs. + +--- + +## File Structure + +- Modify `packages/core/src/utils/runtimeDiagnostics.ts` + - Owns privacy-preserving summaries for runtime profiling. + - Add `OpenAICacheStabilityDiagnostics`, `OpenAIWireRequestDiagnosticsOptions`, and pure hashing/canonicalization helpers. + - Keep request bodies unchanged and diagnostics disabled unless `QWEN_CODE_PROFILE_RUNTIME=1`. + +- Modify `packages/core/src/utils/runtimeDiagnostics.test.ts` + - Unit coverage for ordered names, set hash, exact schema hash, canonical manifest hash, and privacy boundaries. + +- Modify `packages/core/src/core/openaiContentGenerator/pipeline.ts` + - Pass provider context into `runtimeDiagnostics.recordOpenAIWireRequest`. + +- Modify `packages/core/src/core/openaiContentGenerator/pipeline.test.ts` + - Verify DeepSeek hostnames are labeled as DeepSeek and non-DeepSeek hostnames are not. + +- Modify `packages/cli/src/config/config.test.ts` + - Add a regression test for `deepseek-v4-pro` ToolSearch auto-deny. + +- Modify `docs/users/configuration/model-providers.md` + - Add a short DeepSeek cache diagnostics subsection. + +--- + +### Task 1: Runtime Diagnostics Failing Tests + +**Files:** + +- Modify: `packages/core/src/utils/runtimeDiagnostics.test.ts` + +- [ ] **Step 1: Add failing tests for OpenAI tool cache-stability summaries** + +Append these tests inside the existing `describe('RuntimeDiagnosticsCollector', () => { ... })` block, before the closing `});`. + +```ts +it('summarizes OpenAI tool cache stability without retaining schema bodies', () => { + const summary = summarizeOpenAIWireRequest( + { + model: 'wire-model', + stream: false, + messages: [{ role: 'user', content: 'secret user prompt' }], + tools: [ + { + type: 'function', + function: { + name: 'read_file', + description: 'secret tool description', + parameters: { + type: 'object', + properties: { + secretPathProperty: { type: 'string' }, + }, + }, + }, + }, + { + type: 'function', + function: { + name: 'run_shell_command', + description: 'another secret description', + parameters: { + type: 'object', + properties: { + secretCommandProperty: { type: 'string' }, + }, + }, + }, + }, + ], + }, + { provider: 'deepseek' }, + ); + + expect(summary.cacheStability).toMatchObject({ + provider: 'deepseek', + toolNames: ['read_file', 'run_shell_command'], + toolNameSequenceHash: expect.stringMatching(/^[a-f0-9]{64}$/), + toolNameSetHash: expect.stringMatching(/^[a-f0-9]{64}$/), + toolSchemaHash: expect.stringMatching(/^[a-f0-9]{64}$/), + canonicalToolManifestHash: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + expect(JSON.stringify(summary)).not.toContain('secret user prompt'); + expect(JSON.stringify(summary)).not.toContain('secret tool description'); + expect(JSON.stringify(summary)).not.toContain('secretPathProperty'); + expect(JSON.stringify(summary)).not.toContain('secretCommandProperty'); +}); + +it('distinguishes OpenAI tool order drift from tool set drift', () => { + const alphaFirst = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + ], + }); + const bravoFirst = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + ], + }); + + expect(alphaFirst.cacheStability?.toolNames).toEqual(['alpha', 'bravo']); + expect(bravoFirst.cacheStability?.toolNames).toEqual(['bravo', 'alpha']); + expect(alphaFirst.cacheStability?.toolNameSetHash).toBe( + bravoFirst.cacheStability?.toolNameSetHash, + ); + expect(alphaFirst.cacheStability?.toolNameSequenceHash).not.toBe( + bravoFirst.cacheStability?.toolNameSequenceHash, + ); +}); + +it('uses canonical manifest hashes for equivalent schema key order', () => { + const first = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { + path: { type: 'string', description: 'Path' }, + limit: { type: 'number', description: 'Limit' }, + }, + }, + }, + }, + ], + }); + const second = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + parameters: { + properties: { + limit: { description: 'Limit', type: 'number' }, + path: { description: 'Path', type: 'string' }, + }, + type: 'object', + }, + description: 'Inspect', + name: 'inspect', + }, + }, + ], + }); + + expect(first.cacheStability?.canonicalToolManifestHash).toBe( + second.cacheStability?.canonicalToolManifestHash, + ); + expect(first.cacheStability?.toolSchemaHash).not.toBe( + second.cacheStability?.toolSchemaHash, + ); +}); + +it('changes canonical manifest hash when schema content changes', () => { + const before = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { type: 'object', properties: {} }, + }, + }, + ], + }); + const after = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { path: { type: 'string' } }, + }, + }, + }, + ], + }); + + expect(before.cacheStability?.canonicalToolManifestHash).not.toBe( + after.cacheStability?.canonicalToolManifestHash, + ); +}); +``` + +- [ ] **Step 2: Run the targeted test and verify failure** + +Run: + +```powershell +npm test --workspace=packages/core -- src/utils/runtimeDiagnostics.test.ts +``` + +Expected: FAIL with TypeScript or assertion errors mentioning missing `cacheStability` / missing second argument support on `summarizeOpenAIWireRequest`. + +--- + +### Task 2: Implement Runtime Tool Cache-Stability Helpers + +**Files:** + +- Modify: `packages/core/src/utils/runtimeDiagnostics.ts` +- Test: `packages/core/src/utils/runtimeDiagnostics.test.ts` + +- [ ] **Step 1: Add the crypto import and public diagnostics types** + +At the top of `packages/core/src/utils/runtimeDiagnostics.ts`, add the import before existing type imports. + +```ts +import { createHash } from 'node:crypto'; +``` + +Add these interfaces near `OpenAIWireRequestDiagnostics`. + +```ts +export type OpenAICacheStabilityProvider = 'deepseek' | 'openai-compatible'; + +export interface OpenAICacheStabilityDiagnostics { + provider?: OpenAICacheStabilityProvider; + toolNames: string[]; + toolNameSequenceHash: string; + toolNameSetHash: string; + toolSchemaHash: string; + canonicalToolManifestHash: string; +} + +export interface OpenAIWireRequestDiagnosticsOptions { + provider?: OpenAICacheStabilityProvider; +} +``` + +Add the optional field to `OpenAIWireRequestDiagnostics`. + +```ts + cacheStability?: OpenAICacheStabilityDiagnostics; +``` + +- [ ] **Step 2: Thread options through collector recording** + +Change `recordOpenAIWireRequest` to accept options and pass them into the summarizer. + +```ts + recordOpenAIWireRequest( + request: OpenAI.Chat.ChatCompletionCreateParams, + options: OpenAIWireRequestDiagnosticsOptions = {}, + ): void { + if (!this.enabled) { + return; + } + + this.openAIWireRequestIndex += 1; + this.openaiWireRequests.push({ + index: this.openAIWireRequestIndex, + timestamp: this.now(), + ...summarizeOpenAIWireRequest(request, options), + }); + } +``` + +- [ ] **Step 3: Add cache stability to OpenAI request summaries** + +Change the function signature and add `cacheStability` to the return object. + +```ts +export function summarizeOpenAIWireRequest( + request: OpenAI.Chat.ChatCompletionCreateParams, + options: OpenAIWireRequestDiagnosticsOptions = {}, +): OpenAIWireRequestDiagnostics { + const requestRecord = asRecord(request); + const messages = Array.isArray(requestRecord['messages']) + ? requestRecord['messages'] + : []; + const tools = Array.isArray(requestRecord['tools']) + ? requestRecord['tools'] + : []; + const messageBytesByRole: Record = {}; + for (const message of messages) { + const messageRecord = asRecord(message); + const role = + typeof messageRecord['role'] === 'string' + ? messageRecord['role'] + : 'unknown'; + messageBytesByRole[role] = + (messageBytesByRole[role] ?? 0) + utf8Bytes(messageRecord['content']); + } + + return { + model: + typeof requestRecord['model'] === 'string' + ? requestRecord['model'] + : 'unknown', + stream: requestRecord['stream'] === true, + bodyBytes: utf8Bytes(request), + messageCount: messages.length, + messageBytesByRole, + toolsCount: tools.length, + toolSchemaBytes: utf8Bytes(tools), + topLevelKeys: Object.keys(requestRecord).sort(), + cacheStability: summarizeOpenAIToolCacheStability(tools, options), + }; +} +``` + +- [ ] **Step 4: Add pure helper functions near `safeStringify`** + +Place these helpers near the existing private JSON/record helpers at the bottom of the file. + +```ts +function summarizeOpenAIToolCacheStability( + tools: unknown[], + options: OpenAIWireRequestDiagnosticsOptions, +): OpenAICacheStabilityDiagnostics { + const toolNames = extractOpenAIToolNames(tools); + return { + provider: options.provider, + toolNames, + toolNameSequenceHash: hashStableJson(toolNames), + toolNameSetHash: hashStableJson([...toolNames].sort()), + toolSchemaHash: hashString(safeStringify(tools)), + canonicalToolManifestHash: hashStableJson( + buildCanonicalOpenAIToolManifest(tools), + ), + }; +} + +function extractOpenAIToolNames(tools: unknown[]): string[] { + const names: string[] = []; + for (const tool of tools) { + const toolRecord = asRecord(tool); + const fn = asOptionalRecord(toolRecord['function']); + const name = fn && typeof fn['name'] === 'string' ? fn['name'] : undefined; + if (name) { + names.push(name); + } + } + return names; +} + +function buildCanonicalOpenAIToolManifest(tools: unknown[]): Array<{ + name: string; + descriptionHash: string; + parametersHash: string; +}> { + const manifest: Array<{ + name: string; + descriptionHash: string; + parametersHash: string; + }> = []; + for (const tool of tools) { + const toolRecord = asRecord(tool); + const fn = asOptionalRecord(toolRecord['function']); + if (!fn || typeof fn['name'] !== 'string') { + continue; + } + manifest.push({ + name: fn['name'], + descriptionHash: hashString( + typeof fn['description'] === 'string' ? fn['description'] : '', + ), + parametersHash: hashStableJson(fn['parameters']), + }); + } + manifest.sort((a, b) => a.name.localeCompare(b.name)); + return manifest; +} + +function hashStableJson(value: unknown): string { + return hashString(stableStringify(value)); +} + +function hashString(value: string): string { + return createHash('sha256').update(value).digest('hex'); +} + +function stableStringify(value: unknown): string { + return JSON.stringify(toStableJsonValue(value)) ?? ''; +} + +function toStableJsonValue( + value: unknown, + seen = new WeakSet(), +): unknown { + if (value === null || typeof value !== 'object') { + return value; + } + if (seen.has(value)) { + return '[Circular]'; + } + seen.add(value); + if (Array.isArray(value)) { + return value.map((item) => toStableJsonValue(item, seen)); + } + const record = value as Record; + const stableRecord: Record = {}; + for (const key of Object.keys(record).sort()) { + stableRecord[key] = toStableJsonValue(record[key], seen); + } + return stableRecord; +} +``` + +- [ ] **Step 5: Run the runtime diagnostics test** + +Run: + +```powershell +npm test --workspace=packages/core -- src/utils/runtimeDiagnostics.test.ts +``` + +Expected: PASS. + +- [ ] **Step 6: Commit runtime diagnostics implementation** + +Run: + +```powershell +git add packages/core/src/utils/runtimeDiagnostics.ts packages/core/src/utils/runtimeDiagnostics.test.ts +git commit -m "feat(core): add OpenAI tool cache diagnostics" +``` + +Expected: commit succeeds. + +--- + +### Task 3: Pass DeepSeek Provider Context From Pipeline + +**Files:** + +- Modify: `packages/core/src/core/openaiContentGenerator/pipeline.test.ts` +- Modify: `packages/core/src/core/openaiContentGenerator/pipeline.ts` + +- [ ] **Step 1: Add failing pipeline tests** + +Add this import near the other imports in `pipeline.test.ts`. + +```ts +import { runtimeDiagnostics } from '../../utils/runtimeDiagnostics.js'; +``` + +Add these tests inside `describe('execute', () => { ... })` after the existing successful execution test. + +```ts +it('labels runtime OpenAI diagnostics as deepseek for DeepSeek hostnames', async () => { + const diagnosticsSpy = vi + .spyOn(runtimeDiagnostics, 'recordOpenAIWireRequest') + .mockImplementation(() => undefined); + mockContentGeneratorConfig.baseUrl = 'https://api.deepseek.com'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + }; + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockOpenAIResponse = { + id: 'response-id', + choices: [ + { message: { content: 'Hello response' }, finish_reason: 'stop' }, + ], + created: Date.now(), + model: 'deepseek-v4-pro', + } as OpenAI.Chat.ChatCompletion; + const mockGeminiResponse = new GenerateContentResponse(); + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + mockGeminiResponse, + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue( + mockOpenAIResponse, + ); + + await pipeline.execute(request, 'test-prompt-id'); + + expect(diagnosticsSpy).toHaveBeenCalledWith( + expect.objectContaining({ model: 'deepseek-v4-pro' }), + { provider: 'deepseek' }, + ); +}); + +it('labels runtime OpenAI diagnostics as openai-compatible for non-DeepSeek hostnames', async () => { + const diagnosticsSpy = vi + .spyOn(runtimeDiagnostics, 'recordOpenAIWireRequest') + .mockImplementation(() => undefined); + mockContentGeneratorConfig.baseUrl = 'https://example.test/v1'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + }; + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockOpenAIResponse = { + id: 'response-id', + choices: [ + { message: { content: 'Hello response' }, finish_reason: 'stop' }, + ], + created: Date.now(), + model: 'deepseek-v4-pro', + } as OpenAI.Chat.ChatCompletion; + const mockGeminiResponse = new GenerateContentResponse(); + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + mockGeminiResponse, + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue( + mockOpenAIResponse, + ); + + await pipeline.execute(request, 'test-prompt-id'); + + expect(diagnosticsSpy).toHaveBeenCalledWith(expect.any(Object), { + provider: 'openai-compatible', + }); +}); +``` + +- [ ] **Step 2: Run the pipeline test and verify failure** + +Run: + +```powershell +npm test --workspace=packages/core -- src/core/openaiContentGenerator/pipeline.test.ts +``` + +Expected: FAIL because `recordOpenAIWireRequest` is still called with one argument. + +- [ ] **Step 3: Update the pipeline diagnostics call** + +In `packages/core/src/core/openaiContentGenerator/pipeline.ts`, replace: + +```ts +runtimeDiagnostics.recordOpenAIWireRequest(openaiRequest); +``` + +with: + +```ts +runtimeDiagnostics.recordOpenAIWireRequest(openaiRequest, { + provider: isDeepSeekHostname(this.contentGeneratorConfig) + ? 'deepseek' + : 'openai-compatible', +}); +``` + +- [ ] **Step 4: Run the pipeline test** + +Run: + +```powershell +npm test --workspace=packages/core -- src/core/openaiContentGenerator/pipeline.test.ts +``` + +Expected: PASS. + +- [ ] **Step 5: Commit pipeline provider context** + +Run: + +```powershell +git add packages/core/src/core/openaiContentGenerator/pipeline.ts packages/core/src/core/openaiContentGenerator/pipeline.test.ts +git commit -m "feat(core): label DeepSeek cache diagnostics" +``` + +Expected: commit succeeds. + +--- + +### Task 4: CLI DeepSeek Model Regression + +**Files:** + +- Modify: `packages/cli/src/config/config.test.ts` + +- [ ] **Step 1: Add the failing or passing regression test first** + +Add this test near the existing ToolSearch DeepSeek auto-disable tests. + +```ts +it('should auto-disable tool_search for deepseek-v4-pro models', async () => { + process.argv = ['node', 'script.js', '--model', 'deepseek-v4-pro']; + const argv = await parseArguments(); + const settings: Settings = {}; + const config = await loadCliConfig(settings, argv, undefined, []); + expect(config.getPermissionsDeny()).toContain('tool_search'); +}); +``` + +- [ ] **Step 2: Run the targeted CLI config test** + +Run: + +```powershell +npm run test --workspace=packages/cli -- src/config/config.test.ts +``` + +Expected: PASS. If it fails because the regex does not match `deepseek-v4-pro`, update the model regex in `packages/cli/src/config/config.ts` so `deepseek-v4-pro` remains denied by default. + +- [ ] **Step 3: Commit the regression test** + +Run: + +```powershell +git add packages/cli/src/config/config.test.ts +git commit -m "test(cli): cover deepseek v4 pro tool search default" +``` + +Expected: commit succeeds. + +--- + +### Task 5: Documentation And Full Validation + +**Files:** + +- Modify: `docs/users/configuration/model-providers.md` + +- [ ] **Step 1: Add DeepSeek cache diagnostics docs** + +In `docs/users/configuration/model-providers.md`, add this subsection after the existing DeepSeek reasoning configuration notes. + +```md +### DeepSeek cache-stability diagnostics + +DeepSeek-hosted OpenAI-compatible requests rely heavily on exact prefix reuse. +When `QWEN_CODE_PROFILE_RUNTIME=1` is enabled, Qwen Code records +privacy-preserving OpenAI wire diagnostics that help identify tool-prefix drift +without storing prompts, tool descriptions, schema bodies, arguments, or tool +outputs. + +For OpenAI-compatible requests, the runtime diagnostics snapshot includes a +`cacheStability` object: + +- `provider`: `deepseek` for `api.deepseek.com` hostnames, otherwise + `openai-compatible`. +- `toolNames`: function names in the exact wire order sent to the provider. +- `toolNameSequenceHash`: changes when the ordered tool sequence changes. +- `toolNameSetHash`: remains stable when the same tools are only reordered. +- `toolSchemaHash`: hashes the exact `tools` JSON shape sent on the wire. +- `canonicalToolManifestHash`: hashes a name-sorted manifest with recursively + sorted schema keys, so it stays stable across equivalent JSON key ordering. + +Useful comparisons: + +- Same `toolNameSetHash`, different `toolNameSequenceHash`: tool order drift. +- Same `toolNameSequenceHash`, different `canonicalToolManifestHash`: schema + content drift. +- Same `canonicalToolManifestHash`, different `toolSchemaHash`: JSON + serialization or key-order drift. +``` + +- [ ] **Step 2: Run focused tests** + +Run: + +```powershell +npm test --workspace=packages/core -- src/utils/runtimeDiagnostics.test.ts +npm test --workspace=packages/core -- src/core/openaiContentGenerator/pipeline.test.ts +npm run test --workspace=packages/cli -- src/config/config.test.ts +``` + +Expected: all PASS. + +- [ ] **Step 3: Run typechecks** + +Run: + +```powershell +npm run typecheck --workspace=packages/core +npm run typecheck --workspace=packages/cli +``` + +Expected: both PASS. + +- [ ] **Step 4: Run formatting/check guards** + +Run: + +```powershell +git diff --check +``` + +Expected: no output and exit code 0. + +- [ ] **Step 5: Commit docs** + +Run: + +```powershell +git add docs/users/configuration/model-providers.md +git commit -m "docs: explain DeepSeek cache diagnostics" +``` + +Expected: commit succeeds. + +- [ ] **Step 6: Final status** + +Run: + +```powershell +git status --short +git log --oneline -5 +``` + +Expected: clean worktree and recent commits for the spec, runtime diagnostics, pipeline provider labeling, CLI regression, and docs. + +--- + +## Self-Review Checklist + +- The plan covers every acceptance criterion from `docs/superpowers/specs/2026-05-26-deepseek-cache-diagnostics-design.md`. +- Each implementation task starts with tests before production code. +- Diagnostics retain tool names and hashes only, not prompts, descriptions, schemas, args, or outputs. +- Provider labeling uses `isDeepSeekHostname`, not model-name fallback. +- The rollout remains behind existing runtime diagnostics gating. +- No task changes ToolSearch behavior or reorders tools. diff --git a/docs/superpowers/specs/2026-05-26-deepseek-cache-diagnostics-design.md b/docs/superpowers/specs/2026-05-26-deepseek-cache-diagnostics-design.md new file mode 100644 index 00000000000..3269a40b6c8 --- /dev/null +++ b/docs/superpowers/specs/2026-05-26-deepseek-cache-diagnostics-design.md @@ -0,0 +1,179 @@ +# DeepSeek Cache Diagnostics Design + +## Goal + +Add a small, mergeable P0 diagnostics layer that helps Qwen Code operators see whether DeepSeek cache-sensitive tool prefixes are stable across turns, without changing request behavior or logging prompt text, tool descriptions, tool parameters, or tool outputs. + +This spec intentionally does not implement a static tool manifest protocol, semantic cache, sidecar cache, or subagent prefix DAG. Those ideas need separate specs after this baseline proves where request drift is happening. + +## Context + +DeepSeek cache hits depend on exact prefix reuse. The user-provided research report highlights a public Qwen Code case where ToolSearch reduced prompt size but lowered DeepSeek cache hit rate because the tool schema prefix changed across requests. + +The current code already has several P0 defenses: + +- `packages/cli/src/config/config.ts` auto-denies `tool_search` for `deepseek-v3`, `deepseek-v4`, and `deepseek-chat` style model names unless the user explicitly enables ToolSearch. +- `packages/core/src/core/client.ts` eagerly reveals deferred tools when ToolSearch is unavailable, which keeps schemas visible in the initial declaration list. +- `packages/core/src/core/openaiContentGenerator/provider/deepseek.ts` adapts DeepSeek request shape for content flattening and `reasoning_effort`. +- `packages/core/src/utils/runtimeDiagnostics.ts` already records OpenAI wire request sizes and tool schema byte counts when `QWEN_CODE_PROFILE_RUNTIME=1`. + +The missing piece is a stable, privacy-preserving fingerprint for the tool declaration prefix. Today diagnostics can say "tools changed size"; they cannot say whether the same tool set reordered, whether schema bytes changed, or whether DeepSeek-related requests are using a stable tool manifest. + +## Proposed Approach + +Add provider-aware cache-stability diagnostics to the existing runtime diagnostics path. + +For each OpenAI-compatible wire request, record a `cacheStability` object with hashes and small metadata derived from `request.tools`. For DeepSeek-hosted requests, mark the diagnostics as cache-sensitive so operators know these values affect cost. The first implementation is observational only: it must not reorder tools, mutate schema objects, or change provider requests. + +The recommended shape is: + +```ts +interface OpenAICacheStabilityDiagnostics { + provider?: 'deepseek' | 'openai-compatible'; + toolNames: string[]; + toolNameSequenceHash: string; + toolNameSetHash: string; + toolSchemaHash: string; + canonicalToolManifestHash: string; +} +``` + +The fields mean: + +- `provider`: `deepseek` when the configured base URL hostname is `api.deepseek.com` or a subdomain; otherwise omitted or `openai-compatible`. +- `toolNames`: function names in the exact wire order sent to the provider. +- `toolNameSequenceHash`: hash of the ordered tool-name sequence. +- `toolNameSetHash`: hash of the sorted tool-name set. +- `toolSchemaHash`: hash of the exact `request.tools` JSON shape as sent on the wire. +- `canonicalToolManifestHash`: hash of a canonical manifest sorted by function name. Each entry should store only the function name plus hashes of the description and parameters after recursive key sorting. +- `toolSchemaBytes`: keep the existing byte count. + +These fields allow three useful comparisons: + +- Same `toolNameSetHash`, different `toolNameSequenceHash`: tool order drift. +- Same `toolNameSequenceHash`, different `canonicalToolManifestHash`: schema content drift. +- Same `canonicalToolManifestHash`, different `toolSchemaHash`: serialization/key-order drift. + +## Architecture + +### Runtime Diagnostics + +Extend `packages/core/src/utils/runtimeDiagnostics.ts`. + +Responsibilities: + +- Keep summarization privacy-preserving. +- Compute stable SHA-256 hashes for tool diagnostics. +- Keep existing public summaries backward-compatible by adding optional fields. +- Export small pure helpers so unit tests can exercise canonicalization directly. + +The diagnostics collector remains disabled unless `QWEN_CODE_PROFILE_RUNTIME=1`, matching current behavior. + +### OpenAI Pipeline + +Extend `packages/core/src/core/openaiContentGenerator/pipeline.ts`. + +Responsibilities: + +- Pass provider context into `runtimeDiagnostics.recordOpenAIWireRequest`. +- Use `isDeepSeekHostname(this.contentGeneratorConfig)` for DeepSeek-hosted detection. +- Avoid model-name-only DeepSeek detection for cache-sensitive provider labeling because self-hosted DeepSeek-compatible models might not share DeepSeek billing/cache behavior. + +### Documentation + +Update `docs/users/configuration/model-providers.md` or the runtime diagnostics section of settings docs. + +Responsibilities: + +- Explain that DeepSeek uses exact-prefix cache behavior and that stable tool schemas matter. +- Show how to enable `QWEN_CODE_PROFILE_RUNTIME=1`. +- Explain how to interpret the new hashes. +- Reiterate that diagnostics store hashes and sizes, not prompt text or schema bodies. + +## Data Flow + +1. Qwen Code builds a Gemini request with tool declarations. +2. The OpenAI pipeline converts it to an OpenAI-compatible request. +3. The provider reshapes the request as needed. +4. Before dispatch, the pipeline records runtime diagnostics. +5. Runtime diagnostics extracts OpenAI tool definitions, computes hashes, and stores the summary in the in-memory snapshot. +6. Existing stats/debug surfaces can read the snapshot without needing provider request bodies. + +## Privacy And Safety + +The diagnostics must not retain: + +- user prompt text, +- system prompt text, +- tool descriptions, +- JSON schema bodies, +- tool arguments, +- tool outputs, +- API keys or headers. + +The hasher may read tool descriptions and schemas transiently in memory, but the stored snapshot must contain only names, counts, byte lengths, and hashes. + +The diagnostics may retain: + +- tool names, +- counts, +- byte lengths, +- SHA-256 hashes of tool manifests, +- provider label. + +Tool names are already surfaced in other diagnostics paths, but this spec still treats them as operational metadata rather than user content. + +## Error Handling + +Diagnostics must never block a model request. If hashing or canonicalization encounters unexpected input, it should fall back to an empty or partial cache-stability summary and continue. + +Circular structures should be handled with the existing safe stringification style. Request tools are normally JSON-compatible, so circular input is a defensive case, not the expected path. + +## Testing + +Use TDD for implementation. + +### Unit Tests + +Add focused tests in `packages/core/src/utils/runtimeDiagnostics.test.ts`: + +- OpenAI summaries include cache-stability hashes and ordered tool names. +- The summary still does not contain prompt text, tool descriptions, schema property names from sensitive fixtures, or user content. +- `toolNameSetHash` remains stable when tools are reordered. +- `toolNameSequenceHash` changes when tools are reordered. +- `canonicalToolManifestHash` remains stable when equivalent schema objects use different key insertion order. +- `toolSchemaHash` changes when the exact wire schema changes. + +### Pipeline Tests + +Add or update tests in `packages/core/src/core/openaiContentGenerator/pipeline.test.ts`: + +- DeepSeek hostname requests pass provider context to runtime diagnostics. +- Non-DeepSeek OpenAI-compatible requests do not get marked as DeepSeek cache-sensitive. + +### Config Regression Test + +Add a regression case in `packages/cli/src/config/config.test.ts` for the user-facing model name `deepseek-v4-pro`, confirming ToolSearch remains auto-denied unless explicitly enabled. + +## Rollout + +This is safe to merge behind the existing `QWEN_CODE_PROFILE_RUNTIME=1` diagnostics gate. No new user-facing feature flag is required because the change is observational and disabled by default. + +Rollback is simple: remove the optional fields or stop passing provider context. Since the request body is not mutated, rollback should not affect model behavior. + +## Non-Goals + +- Do not reorder tools in this change. +- Do not change ToolSearch behavior. +- Do not add a static tool manifest protocol. +- Do not add local semantic cache or object storage. +- Do not add prompt/body hashes that fingerprint full user messages. +- Do not change DeepSeek request generation other than diagnostics metadata. + +## Acceptance Criteria + +- Runtime diagnostics can distinguish tool set drift, tool order drift, schema drift, and serialization drift. +- DeepSeek-hosted OpenAI-compatible requests are labeled as cache-sensitive in diagnostics. +- Tests prove no prompt text, tool descriptions, schema bodies, tool args, or tool outputs are retained. +- The implementation has no behavioral effect when diagnostics are disabled. +- The docs explain how to enable and interpret the new diagnostics. diff --git a/docs/users/configuration/model-providers.md b/docs/users/configuration/model-providers.md index aff8d1eb78c..007f5046af7 100644 --- a/docs/users/configuration/model-providers.md +++ b/docs/users/configuration/model-providers.md @@ -594,6 +594,38 @@ You can pin an exact thinking-token budget by including `budget_tokens` alongsid For Anthropic this becomes `thinking.budget_tokens`. For OpenAI/DeepSeek the field is preserved but currently ignored by the server — `reasoning_effort` is the load-bearing knob. +### DeepSeek cache-stability diagnostics + +DeepSeek-hosted OpenAI-compatible requests rely heavily on exact prefix reuse. +When `QWEN_CODE_PROFILE_RUNTIME=1` is enabled, Qwen Code records +privacy-preserving OpenAI wire diagnostics that help identify tool-prefix drift +without storing prompts, tool descriptions, schema bodies, arguments, or tool +outputs. + +For OpenAI-compatible requests, the runtime diagnostics snapshot includes a +`cacheStability` object: + +- `provider`: `deepseek` for `api.deepseek.com` hostnames, otherwise + `openai-compatible`. +- `toolNames`: function names in the exact wire order sent to the provider. +- `toolNameSequenceHash`: changes when the ordered tool sequence changes. +- `toolNameSetHash`: remains stable when the same tools are only reordered. +- `toolSchemaHash`: hashes the exact `tools` JSON shape sent on the wire. +- `canonicalToolManifestHash`: hashes a name-sorted manifest of function names + plus hashed descriptions and recursively sorted parameter schemas, so it + stays stable across equivalent JSON key ordering. + +Useful comparisons: + +- Same `toolNameSetHash`, different `toolNameSequenceHash`: tool order drift. +- Same `toolNameSequenceHash`, different `canonicalToolManifestHash`: tool + manifest drift, such as description or parameter-schema changes. +- Same `canonicalToolManifestHash`, different `toolSchemaHash`: JSON + serialization or key-order drift. + +These hashes only identify client-side drift clues. Actual cache hit rates still +need to be confirmed from DeepSeek/provider usage metrics or logs. + ## Provider Models vs Runtime Models Qwen Code distinguishes between two types of model configurations: diff --git a/integration-tests/cli/qwen-serve-client-mcp.test.ts b/integration-tests/cli/qwen-serve-client-mcp.test.ts index 0ac716eaf14..690be9b60b1 100644 --- a/integration-tests/cli/qwen-serve-client-mcp.test.ts +++ b/integration-tests/cli/qwen-serve-client-mcp.test.ts @@ -108,7 +108,9 @@ beforeAll(async () => { if (wantsReadPage && hasToolResult) { return { content: FINAL_ASSISTANT_TEXT }; } - return { content: 'unused — this suite only prompts in the tools/call test' }; + return { + content: 'unused — this suite only prompts in the tools/call test', + }; }); homeDir = mkdtempSync(path.join(tmpdir(), 'qwen-serve-client-mcp-home-')); daemon = spawn( @@ -212,7 +214,9 @@ function answerHandshakeFrame( }; }, onReadPageCall?: (args: unknown) => void, -): { type: 'mcp_message'; id: string; server: string; payload: unknown } | undefined { +): + | { type: 'mcp_message'; id: string; server: string; payload: unknown } + | undefined { const { payload } = frame; if (payload.id === undefined || payload.id === null) return undefined; // notification let result: unknown; @@ -251,7 +255,10 @@ function answerHandshakeFrame( payload: { jsonrpc: '2.0', id: payload.id, - error: { code: -32602, message: `unknown tool: ${String(toolName)}` }, + error: { + code: -32602, + message: `unknown tool: ${String(toolName)}`, + }, }, }; } @@ -275,7 +282,10 @@ function answerHandshakeFrame( payload: { jsonrpc: '2.0', id: payload.id, - error: { code: -32601, message: `method not found: ${payload.method}` }, + error: { + code: -32601, + message: `method not found: ${payload.method}`, + }, }, }; } @@ -287,435 +297,466 @@ function answerHandshakeFrame( }; } -describeMaybe('qwen serve — reverse tool channel (client-hosted MCP over WS)', () => { - it('discovers a client-hosted tool end-to-end via the ACP child', async () => { - const ws = new WebSocket(`ws://127.0.0.1:${port}/acp`, { - headers: { Authorization: `Bearer ${TOKEN}` }, - }); - await new Promise((resolve, reject) => { - ws.once('open', () => resolve()); - ws.once('error', reject); - }); +describeMaybe( + 'qwen serve — reverse tool channel (client-hosted MCP over WS)', + () => { + it('discovers a client-hosted tool end-to-end via the ACP child', async () => { + const ws = new WebSocket(`ws://127.0.0.1:${port}/acp`, { + headers: { Authorization: `Bearer ${TOKEN}` }, + }); + await new Promise((resolve, reject) => { + ws.once('open', () => resolve()); + ws.once('error', reject); + }); - // Demux: ACP JSON-RPC replies (by id) and client-MCP frames (by type). - const acpReplies = new Map>(); - let registeredAck: Record | undefined; - ws.on('message', (data) => { - const msg = JSON.parse(data.toString()) as Record; - if (msg['type'] === 'mcp_message') { - const reply = answerHandshakeFrame( - msg as unknown as { - id: string; - server: string; - payload: { id?: number | string; method?: string }; - }, - ); - if (reply) ws.send(JSON.stringify(reply)); - return; - } - if (msg['type'] === 'mcp_registered' || msg['type'] === 'mcp_error') { - registeredAck = msg; - return; - } - if (typeof msg['id'] === 'number') { - acpReplies.set(msg['id'] as number, msg); - } - }); + // Demux: ACP JSON-RPC replies (by id) and client-MCP frames (by type). + const acpReplies = new Map>(); + let registeredAck: Record | undefined; + ws.on('message', (data) => { + const msg = JSON.parse(data.toString()) as Record; + if (msg['type'] === 'mcp_message') { + const reply = answerHandshakeFrame( + msg as unknown as { + id: string; + server: string; + payload: { id?: number | string; method?: string }; + }, + ); + if (reply) ws.send(JSON.stringify(reply)); + return; + } + if (msg['type'] === 'mcp_registered' || msg['type'] === 'mcp_error') { + registeredAck = msg; + return; + } + if (typeof msg['id'] === 'number') { + acpReplies.set(msg['id'] as number, msg); + } + }); - const waitForAcp = (id: number, timeoutMs = 20_000) => - new Promise>((resolve, reject) => { + const waitForAcp = (id: number, timeoutMs = 20_000) => + new Promise>((resolve, reject) => { + const started = Date.now(); + const tick = () => { + const r = acpReplies.get(id); + if (r) return resolve(r); + if (Date.now() - started > timeoutMs) + return reject( + new Error(`timeout waiting for ACP reply id=${id}`), + ); + setTimeout(tick, 25); + }; + tick(); + }); + + // 1. initialize + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: {}, + }), + ); + await waitForAcp(1); + + // 2. session/new — spawns the real ACP child + binds the session manager's + // sendSdkMcpMessage to the client_mcp/message ext-method. + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { cwd: REPO_ROOT }, + }), + ); + const sessionReply = await waitForAcp(2, 30_000); + const sessionId = (sessionReply['result'] as { sessionId?: string }) + ?.sessionId; + expect(typeof sessionId).toBe('string'); + + // 3. mcp_register — provider adds an SDK-type runtime server in the child; + // the child's discovery handshake round-trips back over THIS WS. + ws.send(JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' })); + + // 4. wait for the registration ack (proves the child discovered the tool). + await new Promise((resolve, reject) => { const started = Date.now(); const tick = () => { - const r = acpReplies.get(id); - if (r) return resolve(r); - if (Date.now() - started > timeoutMs) - return reject(new Error(`timeout waiting for ACP reply id=${id}`)); + if (registeredAck) return resolve(); + if (Date.now() - started > 25_000) + return reject(new Error('timeout waiting for mcp_registered')); setTimeout(tick, 25); }; tick(); }); - // 1. initialize - ws.send( - JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} }), - ); - await waitForAcp(1); - - // 2. session/new — spawns the real ACP child + binds the session manager's - // sendSdkMcpMessage to the client_mcp/message ext-method. - ws.send( - JSON.stringify({ - jsonrpc: '2.0', - id: 2, - method: 'session/new', - params: { cwd: REPO_ROOT }, - }), - ); - const sessionReply = await waitForAcp(2, 30_000); - const sessionId = (sessionReply['result'] as { sessionId?: string }) - ?.sessionId; - expect(typeof sessionId).toBe('string'); - - // 3. mcp_register — provider adds an SDK-type runtime server in the child; - // the child's discovery handshake round-trips back over THIS WS. - ws.send(JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' })); - - // 4. wait for the registration ack (proves the child discovered the tool). - await new Promise((resolve, reject) => { - const started = Date.now(); - const tick = () => { - if (registeredAck) return resolve(); - if (Date.now() - started > 25_000) - return reject(new Error('timeout waiting for mcp_registered')); - setTimeout(tick, 25); + // A surprising `mcp_error` here means the round-trip broke somewhere in the + // child → parent → WS chain; surface its code/message for triage. + expect( + registeredAck, + `expected mcp_registered, got ${JSON.stringify(registeredAck)}`, + ).toMatchObject({ type: 'mcp_registered', server: 'chrome-tools' }); + expect(registeredAck?.['toolCount']).toBe(1); + + // 5. Secondary confirm: the child's tool registry surfaces the tool via the + // workspace MCP tools route (REST, separate from the WS). + const toolsRes = await fetch(`${base}/workspace/mcp/chrome-tools/tools`, { + headers: { Authorization: `Bearer ${TOKEN}` }, + }); + expect(toolsRes.status).toBe(200); + const toolsBody = (await toolsRes.json()) as { + tools?: Array<{ name?: string; serverToolName?: string }>; }; - tick(); - }); - - // A surprising `mcp_error` here means the round-trip broke somewhere in the - // child → parent → WS chain; surface its code/message for triage. - expect( - registeredAck, - `expected mcp_registered, got ${JSON.stringify(registeredAck)}`, - ).toMatchObject({ type: 'mcp_registered', server: 'chrome-tools' }); - expect(registeredAck?.['toolCount']).toBe(1); - - // 5. Secondary confirm: the child's tool registry surfaces the tool via the - // workspace MCP tools route (REST, separate from the WS). - const toolsRes = await fetch(`${base}/workspace/mcp/chrome-tools/tools`, { - headers: { Authorization: `Bearer ${TOKEN}` }, - }); - expect(toolsRes.status).toBe(200); - const toolsBody = (await toolsRes.json()) as { - tools?: Array<{ name?: string; serverToolName?: string }>; - }; - // Tool names may be server-prefixed in the registry; match the raw tool id - // against both the registered `name` and the un-prefixed `serverToolName`. - const hasReadPage = (toolsBody.tools ?? []).some( - (t) => - t.serverToolName === 'chrome_read_page' || - (t.name ?? '').includes('chrome_read_page'), - ); - expect(hasReadPage).toBe(true); - - ws.close(); - }, 60_000); - - // FULL reverse-channel loop, end-to-end: this test drives the genuine - // model→agent→tools/call→reverse-WS→ws-client→result path and asserts the - // tool result is consumed by the agent's turn. - // - // The session-scoping fix (#5626) makes the runtime-added client-hosted MCP - // server reach the PER-SESSION tool registry, not just the bootstrap one: - // - // • `mcp_register` → `workspaceMcpRuntimeAdd` adds the server to the - // BOOTSTRAP/workspace Config (so discovery + `GET /workspace/mcp/.../tools` - // see it) AND fans the add out to every active session's manager - // (packages/cli/src/acp-integration/acpAgent.ts), binding THAT session's - // `sendSdkMcpMessage` (the `__clientMcpOverWs` reverse path). - // • A session created LATER also inherits the bootstrap Config's runtime MCP - // servers in `newSessionConfig` before `config.initialize()`. - // - // So a model-driven `tools/call` for `chrome_read_page` now resolves in the - // session registry, crosses the reverse WS channel to this stand-in - // extension, returns a `CallToolResult`, and the agent's turn consumes it. - // - // This test does session/new THEN mcp_register (the "register after a session - // already exists" timing), exercising the fan-out path specifically. - // Under container sandboxing, the ACP child cannot reach the host-loopback - // fake model server used below; keep the discovery-only test running there. - itPromptedModelMaybe('drives a model→agent tools/call of chrome_read_page over the reverse WS channel and consumes the result', async () => { - const ws = new WebSocket(`ws://127.0.0.1:${port}/acp`, { - headers: { Authorization: `Bearer ${TOKEN}` }, - }); - await new Promise((resolve, reject) => { - ws.once('open', () => resolve()); - ws.once('error', reject); - }); + // Tool names may be server-prefixed in the registry; match the raw tool id + // against both the registered `name` and the un-prefixed `serverToolName`. + const hasReadPage = (toolsBody.tools ?? []).some( + (t) => + t.serverToolName === 'chrome_read_page' || + (t.name ?? '').includes('chrome_read_page'), + ); + expect(hasReadPage).toBe(true); + + ws.close(); + }, 60_000); + + // FULL reverse-channel loop, end-to-end: this test drives the genuine + // model→agent→tools/call→reverse-WS→ws-client→result path and asserts the + // tool result is consumed by the agent's turn. + // + // The session-scoping fix (#5626) makes the runtime-added client-hosted MCP + // server reach the PER-SESSION tool registry, not just the bootstrap one: + // + // • `mcp_register` → `workspaceMcpRuntimeAdd` adds the server to the + // BOOTSTRAP/workspace Config (so discovery + `GET /workspace/mcp/.../tools` + // see it) AND fans the add out to every active session's manager + // (packages/cli/src/acp-integration/acpAgent.ts), binding THAT session's + // `sendSdkMcpMessage` (the `__clientMcpOverWs` reverse path). + // • A session created LATER also inherits the bootstrap Config's runtime MCP + // servers in `newSessionConfig` before `config.initialize()`. + // + // So a model-driven `tools/call` for `chrome_read_page` now resolves in the + // session registry, crosses the reverse WS channel to this stand-in + // extension, returns a `CallToolResult`, and the agent's turn consumes it. + // + // This test does session/new THEN mcp_register (the "register after a session + // already exists" timing), exercising the fan-out path specifically. + // Under container sandboxing, the ACP child cannot reach the host-loopback + // fake model server used below; keep the discovery-only test running there. + itPromptedModelMaybe('drives a model→agent tools/call of chrome_read_page over the reverse WS channel and consumes the result', async () => { + const ws = new WebSocket(`ws://127.0.0.1:${port}/acp`, { + headers: { Authorization: `Bearer ${TOKEN}` }, + }); + await new Promise((resolve, reject) => { + ws.once('open', () => resolve()); + ws.once('error', reject); + }); - // Records every reverse-channel `tools/call` frame the stand-in extension - // saw, plus the forwarded arguments — this is the model→agent→child→parent→WS - // path the discovery test never exercises. - const readPageCalls: unknown[] = []; - - const acpReplies = new Map>(); - let registeredAck: Record | undefined; - ws.on('message', (data) => { - const msg = JSON.parse(data.toString()) as Record; - if (msg['type'] === 'mcp_message') { - // Same canned client-hosted MCP server as the discovery test, now also - // answering `tools/call`. Record `chrome_read_page` invocations so the - // assertions below can prove the reverse round-trip fired. - const reply = answerHandshakeFrame( - msg as unknown as { - id: string; - server: string; - payload: { - id?: number | string; - method?: string; - params?: { name?: string; arguments?: unknown }; - }; - }, - (args) => readPageCalls.push(args), - ); - if (reply) ws.send(JSON.stringify(reply)); - return; - } - if (msg['type'] === 'mcp_registered' || msg['type'] === 'mcp_error') { - registeredAck = msg; - return; - } - if (typeof msg['id'] === 'number') { - acpReplies.set(msg['id'] as number, msg); - } - }); + // Records every reverse-channel `tools/call` frame the stand-in extension + // saw, plus the forwarded arguments — this is the model→agent→child→parent→WS + // path the discovery test never exercises. + const readPageCalls: unknown[] = []; + + const acpReplies = new Map>(); + let registeredAck: Record | undefined; + ws.on('message', (data) => { + const msg = JSON.parse(data.toString()) as Record; + if (msg['type'] === 'mcp_message') { + // Same canned client-hosted MCP server as the discovery test, now also + // answering `tools/call`. Record `chrome_read_page` invocations so the + // assertions below can prove the reverse round-trip fired. + const reply = answerHandshakeFrame( + msg as unknown as { + id: string; + server: string; + payload: { + id?: number | string; + method?: string; + params?: { name?: string; arguments?: unknown }; + }; + }, + (args) => readPageCalls.push(args), + ); + if (reply) ws.send(JSON.stringify(reply)); + return; + } + if (msg['type'] === 'mcp_registered' || msg['type'] === 'mcp_error') { + registeredAck = msg; + return; + } + if (typeof msg['id'] === 'number') { + acpReplies.set(msg['id'] as number, msg); + } + }); - const waitForAcp = (id: number, timeoutMs = 20_000) => - new Promise>((resolve, reject) => { + const waitForAcp = (id: number, timeoutMs = 20_000) => + new Promise>((resolve, reject) => { + const started = Date.now(); + const tick = () => { + const r = acpReplies.get(id); + if (r) return resolve(r); + if (Date.now() - started > timeoutMs) + return reject( + new Error(`timeout waiting for ACP reply id=${id}`), + ); + setTimeout(tick, 25); + }; + tick(); + }); + + // 1. initialize + 2. session/new (real ACP child) — identical to discovery. + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: {}, + }), + ); + await waitForAcp(1); + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { cwd: REPO_ROOT }, + }), + ); + const sessionReply = await waitForAcp(2, 30_000); + const sessionId = (sessionReply['result'] as { sessionId?: string }) + ?.sessionId as string; + expect(typeof sessionId).toBe('string'); + + // 3. mcp_register chrome-tools + wait for the ack (tool discovered). + ws.send(JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' })); + await new Promise((resolve, reject) => { const started = Date.now(); const tick = () => { - const r = acpReplies.get(id); - if (r) return resolve(r); - if (Date.now() - started > timeoutMs) - return reject(new Error(`timeout waiting for ACP reply id=${id}`)); + if (registeredAck) return resolve(); + if (Date.now() - started > 25_000) + return reject(new Error('timeout waiting for mcp_registered')); setTimeout(tick, 25); }; tick(); }); - - // 1. initialize + 2. session/new (real ACP child) — identical to discovery. - ws.send( - JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'initialize', params: {} }), - ); - await waitForAcp(1); - ws.send( - JSON.stringify({ - jsonrpc: '2.0', - id: 2, - method: 'session/new', - params: { cwd: REPO_ROOT }, - }), - ); - const sessionReply = await waitForAcp(2, 30_000); - const sessionId = (sessionReply['result'] as { sessionId?: string }) - ?.sessionId as string; - expect(typeof sessionId).toBe('string'); - - // 3. mcp_register chrome-tools + wait for the ack (tool discovered). - ws.send(JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' })); - await new Promise((resolve, reject) => { - const started = Date.now(); - const tick = () => { - if (registeredAck) return resolve(); - if (Date.now() - started > 25_000) - return reject(new Error('timeout waiting for mcp_registered')); - setTimeout(tick, 25); - }; - tick(); - }); - expect( - registeredAck, - `expected mcp_registered, got ${JSON.stringify(registeredAck)}`, - ).toMatchObject({ type: 'mcp_registered', server: 'chrome-tools' }); - expect(registeredAck?.['toolCount']).toBe(1); - - // 4. Pin the session to `yolo` so the model-emitted tool call auto-approves - // (no human in the loop on the WS) — otherwise a `permission_request` would - // stall the turn forever. Matches the daemon's intended "extension drives - // tools unattended" posture. - const modeRes = await fetch(`${base}/session/${sessionId}/approval-mode`, { - method: 'POST', - headers: { - Authorization: `Bearer ${TOKEN}`, - 'content-type': 'application/json', - }, - body: JSON.stringify({ mode: 'yolo' }), - }); - expect(modeRes.status).toBe(200); - - // 5. Drive a real prompt over REST. The fake model returns a - // `chrome_read_page` tool_call on this turn (see beforeAll), so the agent - // must invoke the client-hosted tool through the reverse WS channel. - const promptRes = await fetch(`${base}/session/${sessionId}/prompt`, { - method: 'POST', - headers: { - Authorization: `Bearer ${TOKEN}`, - 'content-type': 'application/json', - }, - body: JSON.stringify({ - prompt: [ - { - type: 'text', - text: `${READ_PAGE_PROMPT_SENTINEL}: read the current browser page and summarize it.`, + expect( + registeredAck, + `expected mcp_registered, got ${JSON.stringify(registeredAck)}`, + ).toMatchObject({ type: 'mcp_registered', server: 'chrome-tools' }); + expect(registeredAck?.['toolCount']).toBe(1); + + // 4. Pin the session to `yolo` so the model-emitted tool call auto-approves + // (no human in the loop on the WS) — otherwise a `permission_request` would + // stall the turn forever. Matches the daemon's intended "extension drives + // tools unattended" posture. + const modeRes = await fetch( + `${base}/session/${sessionId}/approval-mode`, + { + method: 'POST', + headers: { + Authorization: `Bearer ${TOKEN}`, + 'content-type': 'application/json', }, - ], - }), - }); - expect(promptRes.status).toBe(202); - const { promptId, lastEventId } = (await promptRes.json()) as { - promptId: string; - lastEventId: number; - }; - expect(typeof promptId).toBe('string'); - - // 6. Subscribe to the session SSE stream from the cursor BEFORE this turn so - // no tool_call / tool_call_update / turn_complete frame is missed. Collect - // until `turn_complete` for THIS promptId (or timeout). - const sseAbort = new AbortController(); - const events: Array<{ type: string; data: unknown }> = []; - const sseDone = (async () => { - const res = await fetch(`${base}/session/${sessionId}/events`, { + body: JSON.stringify({ mode: 'yolo' }), + }, + ); + expect(modeRes.status).toBe(200); + + // 5. Drive a real prompt over REST. The fake model returns a + // `chrome_read_page` tool_call on this turn (see beforeAll), so the agent + // must invoke the client-hosted tool through the reverse WS channel. + const promptRes = await fetch(`${base}/session/${sessionId}/prompt`, { + method: 'POST', headers: { Authorization: `Bearer ${TOKEN}`, - Accept: 'text/event-stream', - 'Last-Event-ID': String(lastEventId), + 'content-type': 'application/json', }, - signal: sseAbort.signal, + body: JSON.stringify({ + prompt: [ + { + type: 'text', + text: `${READ_PAGE_PROMPT_SENTINEL}: read the current browser page and summarize it.`, + }, + ], + }), }); - if (!res.ok || !res.body) throw new Error(`SSE open failed: ${res.status}`); - const reader = res.body.getReader(); - const decoder = new TextDecoder(); - let buf = ''; - try { - for (;;) { - const { done, value } = await reader.read(); - if (done) break; - buf += decoder.decode(value, { stream: true }); - // Parse complete SSE frames (separated by a blank line). - let sep: number; - while ((sep = buf.indexOf('\n\n')) !== -1) { - const rawFrame = buf.slice(0, sep); - buf = buf.slice(sep + 2); - let evType = 'message'; - const dataLines: string[] = []; - for (const line of rawFrame.split('\n')) { - if (line.startsWith('event:')) evType = line.slice(6).trim(); - else if (line.startsWith('data:')) dataLines.push(line.slice(5).trim()); - } - if (dataLines.length === 0) continue; // heartbeat / comment - let parsed: unknown; - try { - parsed = JSON.parse(dataLines.join('\n')); - } catch { - continue; + expect(promptRes.status).toBe(202); + const { promptId, lastEventId } = (await promptRes.json()) as { + promptId: string; + lastEventId: number; + }; + expect(typeof promptId).toBe('string'); + + // 6. Subscribe to the session SSE stream from the cursor BEFORE this turn so + // no tool_call / tool_call_update / turn_complete frame is missed. Collect + // until `turn_complete` for THIS promptId (or timeout). + const sseAbort = new AbortController(); + const events: Array<{ type: string; data: unknown }> = []; + const sseDone = (async () => { + const res = await fetch(`${base}/session/${sessionId}/events`, { + headers: { + Authorization: `Bearer ${TOKEN}`, + Accept: 'text/event-stream', + 'Last-Event-ID': String(lastEventId), + }, + signal: sseAbort.signal, + }); + if (!res.ok || !res.body) + throw new Error(`SSE open failed: ${res.status}`); + const reader = res.body.getReader(); + const decoder = new TextDecoder(); + let buf = ''; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + buf += decoder.decode(value, { stream: true }); + // Parse complete SSE frames (separated by a blank line). + let sep: number; + while ((sep = buf.indexOf('\n\n')) !== -1) { + const rawFrame = buf.slice(0, sep); + buf = buf.slice(sep + 2); + let evType = 'message'; + const dataLines: string[] = []; + for (const line of rawFrame.split('\n')) { + if (line.startsWith('event:')) evType = line.slice(6).trim(); + else if (line.startsWith('data:')) + dataLines.push(line.slice(5).trim()); + } + if (dataLines.length === 0) continue; // heartbeat / comment + let parsed: unknown; + try { + parsed = JSON.parse(dataLines.join('\n')); + } catch { + continue; + } + const env = parsed as { type?: string; data?: unknown }; + events.push({ type: env.type ?? evType, data: env.data }); + const isTurnComplete = + (env.type ?? evType) === 'turn_complete' && + (env.data as { promptId?: string })?.promptId === promptId; + if (isTurnComplete) return; } - const env = parsed as { type?: string; data?: unknown }; - events.push({ type: env.type ?? evType, data: env.data }); - const isTurnComplete = - (env.type ?? evType) === 'turn_complete' && - (env.data as { promptId?: string })?.promptId === promptId; - if (isTurnComplete) return; } + } finally { + reader.cancel().catch(() => {}); } - } finally { - reader.cancel().catch(() => {}); + })(); + + // 7. Wait for the turn to complete (consuming the tool result) or time out. + let timedOut = false; + await Promise.race([ + sseDone, + new Promise((resolve) => + setTimeout(() => { + timedOut = true; + resolve(); + }, 40_000), + ), + ]); + sseAbort.abort(); + await sseDone.catch(() => {}); + if (timedOut) { + // A timeout (vs. a clean turn_complete) usually means the model call never + // reached the fake server — most often a localhost-bypassing HTTP proxy in + // the dev env. Surface the request count + event trace for triage. + throw new Error( + `timeout waiting for turn_complete; fakeReqs=${fakeServer.requests.length} ` + + `readPageCalls=${readPageCalls.length} ` + + `events=${JSON.stringify( + events.map((e) => ({ + t: e.type, + u: ( + e.data as { + update?: { sessionUpdate?: string; status?: string }; + } + )?.update?.sessionUpdate, + s: (e.data as { update?: { status?: string } })?.update?.status, + })), + )}`, + ); } - })(); - - // 7. Wait for the turn to complete (consuming the tool result) or time out. - let timedOut = false; - await Promise.race([ - sseDone, - new Promise((resolve) => - setTimeout(() => { - timedOut = true; - resolve(); - }, 40_000), - ), - ]); - sseAbort.abort(); - await sseDone.catch(() => {}); - if (timedOut) { - // A timeout (vs. a clean turn_complete) usually means the model call never - // reached the fake server — most often a localhost-bypassing HTTP proxy in - // the dev env. Surface the request count + event trace for triage. - throw new Error( - `timeout waiting for turn_complete; fakeReqs=${fakeServer.requests.length} ` + - `readPageCalls=${readPageCalls.length} ` + - `events=${JSON.stringify( - events.map((e) => ({ - t: e.type, - u: (e.data as { update?: { sessionUpdate?: string; status?: string } }) - ?.update?.sessionUpdate, - s: (e.data as { update?: { status?: string } })?.update?.status, - })), - )}`, - ); - } - // Collect the tool-call lifecycle the agent surfaced for THIS turn. - const toolCallUpdates = events.filter( - (e) => - e.type === 'session_update' && - ((e.data as { update?: { sessionUpdate?: string } })?.update - ?.sessionUpdate === 'tool_call' || - (e.data as { update?: { sessionUpdate?: string } })?.update - ?.sessionUpdate === 'tool_call_update'), - ); - const readPageUpdate = toolCallUpdates.find((e) => { - const u = (e.data as { update?: Record })?.update ?? {}; - const meta = u['_meta'] as { toolName?: string } | undefined; - const contentText = JSON.stringify(u['content'] ?? ''); - return ( - meta?.toolName === 'chrome_read_page' || - contentText.includes('chrome_read_page') || - String(u['title'] ?? '').includes('chrome_read_page') + // Collect the tool-call lifecycle the agent surfaced for THIS turn. + const toolCallUpdates = events.filter( + (e) => + e.type === 'session_update' && + ((e.data as { update?: { sessionUpdate?: string } })?.update + ?.sessionUpdate === 'tool_call' || + (e.data as { update?: { sessionUpdate?: string } })?.update + ?.sessionUpdate === 'tool_call_update'), ); - }); + const readPageUpdate = toolCallUpdates.find((e) => { + const u = + (e.data as { update?: Record })?.update ?? {}; + const meta = u['_meta'] as { toolName?: string } | undefined; + const contentText = JSON.stringify(u['content'] ?? ''); + return ( + meta?.toolName === 'chrome_read_page' || + contentText.includes('chrome_read_page') || + String(u['title'] ?? '').includes('chrome_read_page') + ); + }); - // ── The model→agent dispatch fired ────────────────────────────────────── - // The model emitted a `chrome_read_page` tool call (the fake server saw the - // prompt) and the agent surfaced a tool_call(_update) for it — i.e. the - // prompt is wired through to the agent's tool dispatcher for the - // client-hosted tool name. - expect(fakeServer.requests.length).toBeGreaterThanOrEqual(1); - expect( - readPageUpdate, - `expected a tool_call(_update) naming chrome_read_page; ` + - `events=${JSON.stringify(events.map((e) => e.type))}`, - ).toBeDefined(); - - // ── SUCCESS PATH (session-scoped runtime MCP — #5626) ─────────────────── - // (a) The stand-in extension RECEIVED the reverse `tools/call`: the agent - // resolved `chrome_read_page` in the SESSION registry, bound the session's - // `sendSdkMcpMessage`, and the frame crossed the WS to this client. - expect( - readPageCalls.length, - `expected the reverse tools/call to reach the ws client; ` + - `updates=${JSON.stringify( - toolCallUpdates.map( - (e) => (e.data as { update?: { status?: string } })?.update?.status, - ), - )}`, - ).toBeGreaterThanOrEqual(1); - // The model emitted args `{}`, forwarded verbatim over the reverse channel. - expect(typeof readPageCalls[0]).toBe('object'); - - // (b) The agent CONSUMED the result — the tool call reached `completed`. - const completed = toolCallUpdates.some( - (e) => - (e.data as { update?: { status?: string } })?.update?.status === - 'completed', - ); - expect( - completed, - `expected a completed tool_call_update for chrome_read_page; ` + - `statuses=${JSON.stringify( - toolCallUpdates.map( - (e) => (e.data as { update?: { status?: string } })?.update?.status, - ), - )}`, - ).toBe(true); - - // (c) The turn ended cleanly (the agent fed the tool result back to the - // model, which returned its final assistant message). - const turnComplete = events.find( - (e) => - e.type === 'turn_complete' && - (e.data as { promptId?: string })?.promptId === promptId, - ); - expect(turnComplete, 'expected a turn_complete for this prompt').toBeDefined(); + // ── The model→agent dispatch fired ────────────────────────────────────── + // The model emitted a `chrome_read_page` tool call (the fake server saw the + // prompt) and the agent surfaced a tool_call(_update) for it — i.e. the + // prompt is wired through to the agent's tool dispatcher for the + // client-hosted tool name. + expect(fakeServer.requests.length).toBeGreaterThanOrEqual(1); + expect( + readPageUpdate, + `expected a tool_call(_update) naming chrome_read_page; ` + + `events=${JSON.stringify(events.map((e) => e.type))}`, + ).toBeDefined(); + + // ── SUCCESS PATH (session-scoped runtime MCP — #5626) ─────────────────── + // (a) The stand-in extension RECEIVED the reverse `tools/call`: the agent + // resolved `chrome_read_page` in the SESSION registry, bound the session's + // `sendSdkMcpMessage`, and the frame crossed the WS to this client. + expect( + readPageCalls.length, + `expected the reverse tools/call to reach the ws client; ` + + `updates=${JSON.stringify( + toolCallUpdates.map( + (e) => + (e.data as { update?: { status?: string } })?.update?.status, + ), + )}`, + ).toBeGreaterThanOrEqual(1); + // The model emitted args `{}`, forwarded verbatim over the reverse channel. + expect(typeof readPageCalls[0]).toBe('object'); + + // (b) The agent CONSUMED the result — the tool call reached `completed`. + const completed = toolCallUpdates.some( + (e) => + (e.data as { update?: { status?: string } })?.update?.status === + 'completed', + ); + expect( + completed, + `expected a completed tool_call_update for chrome_read_page; ` + + `statuses=${JSON.stringify( + toolCallUpdates.map( + (e) => + (e.data as { update?: { status?: string } })?.update?.status, + ), + )}`, + ).toBe(true); + + // (c) The turn ended cleanly (the agent fed the tool result back to the + // model, which returned its final assistant message). + const turnComplete = events.find( + (e) => + e.type === 'turn_complete' && + (e.data as { promptId?: string })?.promptId === promptId, + ); + expect( + turnComplete, + 'expected a turn_complete for this prompt', + ).toBeDefined(); - ws.close(); - }, 90_000); -}); + ws.close(); + }, 90_000); + }, +); diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index 0cefa41122a..41cab203e96 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -818,7 +818,9 @@ describe('BridgeClient — reverse tool channel (qwen/control/client_mcp/message * the serve layer). The registrar pushes outbound frames to `onFrame` so the * test can answer them like the extension's WS would. */ - function makeClientWithRegistrar(registrar: ClientMcpRegistrar): BridgeClient { + function makeClientWithRegistrar( + registrar: ClientMcpRegistrar, + ): BridgeClient { const sender: ClientMcpMessageSender = (serverName: string) => registrar.hasServer(serverName) ? (payload: unknown) => diff --git a/packages/acp-bridge/src/bridgeOptions.ts b/packages/acp-bridge/src/bridgeOptions.ts index 9a55e29a8f2..2df680691aa 100644 --- a/packages/acp-bridge/src/bridgeOptions.ts +++ b/packages/acp-bridge/src/bridgeOptions.ts @@ -398,6 +398,4 @@ export interface BridgeOptions { */ export type ClientMcpMessageSender = ( serverName: string, -) => - | ((payload: unknown) => Promise) - | undefined; +) => ((payload: unknown) => Promise) | undefined; diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 6b0d739bf96..2ab608d927d 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -511,6 +511,25 @@ vi.mock('../ui/commands/contextCommand.js', () => ({ })); vi.mock('./session/Session.js', () => ({ Session: vi.fn(), + isHistorySnapshot: (value: unknown) => { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + const snapshot = value as { + history?: unknown; + modelFacingUserTurnCount?: unknown; + }; + const count = snapshot.modelFacingUserTurnCount; + return ( + Array.isArray(snapshot.history) && + snapshot.history.length > 0 && + typeof count === 'number' && + Number.isInteger(count) && + Number.isFinite(count) && + count >= 0 && + count <= Number.MAX_SAFE_INTEGER + ); + }, buildAvailableCommandsSnapshot: vi.fn().mockResolvedValue({ availableCommands: [], availableSkills: [], @@ -1561,9 +1580,10 @@ describe('QwenAgent MCP SSE/HTTP support', () => { startCronScheduler: vi.fn(), dispose: vi.fn(), emitGoalStatus: vi.fn(), - captureHistorySnapshot: vi - .fn() - .mockReturnValue([{ role: 'user', parts: [{ text: 'before' }] }]), + captureHistorySnapshot: vi.fn().mockReturnValue({ + history: [{ role: 'user', parts: [{ text: 'before' }] }], + modelFacingUserTurnCount: 1, + }), restoreHistory: vi.fn(), rewindToTurn: vi .fn() @@ -5415,7 +5435,10 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }); expect(response).toEqual({ success: true, - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'before' }] }], + historyBeforeRewind: { + history: [{ role: 'user', parts: [{ text: 'before' }] }], + modelFacingUserTurnCount: 1, + }, targetTurnIndex: 1, apiTruncateIndex: 2, filesChanged: [], @@ -5576,6 +5599,131 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('restoreSessionHistory extension method restores history snapshots', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + const snapshot = { + history: [{ role: 'user', parts: [{ text: 'restored' }] }], + modelFacingUserTurnCount: 1, + }; + const response = await agent.extMethod('restoreSessionHistory', { + sessionId, + history: snapshot, + cwd: '/tmp', + }); + + expect(lastSessionMock?.restoreHistory).toHaveBeenCalledWith(snapshot); + expect(response).toEqual({ success: true }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('restoreSessionHistory rejects invalid history snapshot turn counts', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + for (const modelFacingUserTurnCount of [ + NaN, + Infinity, + -Infinity, + -1, + 1.5, + Number.MAX_SAFE_INTEGER + 1, + ]) { + await expect( + agent.extMethod('restoreSessionHistory', { + sessionId, + history: { + history: [], + modelFacingUserTurnCount, + }, + }), + ).rejects.toThrow('Invalid or missing history'); + } + + for (const modelFacingUserTurnCount of [NaN, -1, 1.5]) { + await expect( + agent.extMethod('restoreSessionHistory', { + sessionId, + history: { + history: [{ role: 'user', parts: [{ text: 'hello' }] }], + modelFacingUserTurnCount, + }, + }), + ).rejects.toThrow('Invalid or missing history'); + } + + expect(lastSessionMock?.restoreHistory).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('restoreSessionHistory rejects empty history snapshots', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + await setupSessionMocks(sessionId); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + await expect( + agent.extMethod('restoreSessionHistory', { + sessionId, + history: { + history: [], + modelFacingUserTurnCount: 0, + }, + }), + ).rejects.toThrow('Invalid or missing history'); + expect(lastSessionMock?.restoreHistory).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('restoreSessionHistory rejects invalid session ids', async () => { await setupSessionMocks('11111111-1111-1111-1111-111111111111'); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 606c1cecc94..a43bee021bf 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -155,7 +155,12 @@ import { buildDisabledSkillNamesProvider, loadCliConfig, } from '../config/config.js'; -import { Session, buildAvailableCommandsSnapshot } from './session/Session.js'; +import { + Session, + buildAvailableCommandsSnapshot, + isHistorySnapshot, + type HistorySnapshot, +} from './session/Session.js'; import { buildSessionTasksStatus } from './session/tasksSnapshot.js'; import { HistoryReplayer } from './session/HistoryReplayer.js'; import { @@ -6876,7 +6881,7 @@ class QwenAgent implements Agent { 'Invalid or missing sessionId', ); } - if (!Array.isArray(history)) { + if (!Array.isArray(history) && !isHistorySnapshot(history)) { throw RequestError.invalidParams( undefined, 'Invalid or missing history', @@ -6890,7 +6895,7 @@ class QwenAgent implements Agent { ); } - session.restoreHistory(history as Content[]); + session.restoreHistory(history as Content[] | HistorySnapshot); return { success: true }; } case 'getAccountInfo': { diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index a26db79e356..2022899cd21 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -10,7 +10,9 @@ import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import { + computeInitialModelFacingUserTurnCountFromHistory, computeInitialTurnFromHistory, + computeMaxModelFacingUserTurnCountFromHistory, fireSessionPermissionDeniedForAutoMode, Session, } from './Session.js'; @@ -26,6 +28,7 @@ import { AuthType, SYSTEM_REMINDER_OPEN, SYSTEM_REMINDER_CLOSE, + buildApiHistoryFromConversation, } from '@qwen-code/qwen-code-core'; import * as core from '@qwen-code/qwen-code-core'; import { SettingScope } from '../../config/settings.js'; @@ -172,6 +175,145 @@ describe('computeInitialTurnFromHistory', () => { }); }); +describe('computeInitialModelFacingUserTurnCountFromHistory', () => { + it('counts model-facing user text records without slash-only records', () => { + expect( + computeInitialModelFacingUserTurnCountFromHistory( + [ + chatRecord({ + uuid: 'user-1', + message: { parts: [{ text: 'first' }] }, + }), + chatRecord({ + uuid: 'slash-1', + message: { parts: [{ text: '/help' }] }, + }), + chatRecord({ + uuid: 'question-command-1', + message: { parts: [{ text: '?help' }] }, + }), + chatRecord({ + uuid: 'cron-1', + subtype: 'cron', + message: { parts: [{ text: 'cron prompt' }] }, + }), + chatRecord({ + uuid: 'mid-turn-1', + subtype: 'mid_turn_user_message', + message: { parts: [{ text: 'mid-turn prompt text' }] }, + }), + chatRecord({ + uuid: 'notification-1', + subtype: 'notification', + message: { parts: [{ text: 'FYI' }] }, + }), + chatRecord({ + uuid: 'other-session', + sessionId: 'other-session-id', + message: { parts: [{ text: 'other' }] }, + }), + ], + 'test-session-id', + ), + ).toBe(2); + }); + + it('classifies multi-part user records after joining text parts', () => { + expect( + computeInitialModelFacingUserTurnCountFromHistory( + [ + chatRecord({ + uuid: 'multi-part-prompt', + message: { + parts: [{ text: 'please' }, { text: 'continue' }], + }, + }), + chatRecord({ + uuid: 'split-question-command', + message: { + parts: [{ text: '?' }, { text: 'help' }], + }, + }), + chatRecord({ + uuid: 'split-slash-command', + message: { + parts: [{ text: '/' }, { text: 'clear' }], + }, + }), + ], + 'test-session-id', + ), + ).toBe(1); + }); +}); + +describe('computeMaxModelFacingUserTurnCountFromHistory', () => { + it('restores historical model-facing peak from rewind records', () => { + expect( + computeMaxModelFacingUserTurnCountFromHistory( + [ + chatRecord({ + uuid: 'rewind-1', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 3, + maxModelFacingUserTurnCount: 5, + }, + }), + chatRecord({ + uuid: 'rewind-2', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 1, + maxModelFacingUserTurnCount: 4, + }, + }), + chatRecord({ + uuid: 'invalid-rewind', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 1, + maxModelFacingUserTurnCount: Number.POSITIVE_INFINITY, + }, + }), + chatRecord({ + uuid: 'other-session-rewind', + sessionId: 'other-session-id', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 1, + maxModelFacingUserTurnCount: 99, + }, + }), + ], + 'test-session-id', + ), + ).toBe(5); + }); + + it('ignores pre-PR rewind records without a model-facing peak', () => { + expect( + computeMaxModelFacingUserTurnCountFromHistory( + [ + chatRecord({ + uuid: 'legacy-rewind', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 3, + }, + }), + ], + 'test-session-id', + ), + ).toBe(0); + }); +}); + // Helper to create empty async generator (avoids memory leak from inline generators) function createEmptyStream() { return (async function* () {})(); @@ -208,6 +350,31 @@ function createStreamWithChunks( })(); } +function createHistoryCommittingSendMessageStream( + chat: GeminiChat, + streamFactory: ( + message: Part[], + ) => AsyncIterable<{ type: unknown; value: unknown }>, +) { + return vi.fn( + async (_model: string, request: { message: Part[] }, _promptId: string) => { + chat.addHistory({ role: 'user', parts: request.message }); + return streamFactory(request.message) as AsyncGenerator< + { type: unknown; value: unknown }, + void, + unknown + >; + }, + ); +} + +function createFailingDispatchedStream(errorMessage: string) { + return (async function* () { + yield { type: core.StreamEventType.CHUNK, value: {} }; + throw new Error(errorMessage); + })(); +} + function expectCompressBeforeSend( compressMock: ReturnType, sendMock: ReturnType, @@ -222,6 +389,32 @@ function expectCompressBeforeSend( ); } +function setSessionTurnCounters( + targetSession: Session, + counters: { + turn?: number; + modelFacingUserTurnCount?: number; + maxModelFacingUserTurnCount?: number; + }, +) { + const sessionCounters = targetSession as unknown as Record; + Object.assign(sessionCounters, counters); + if ( + counters.modelFacingUserTurnCount !== undefined && + counters.maxModelFacingUserTurnCount === undefined + ) { + sessionCounters['maxModelFacingUserTurnCount'] = Math.max( + sessionCounters['maxModelFacingUserTurnCount'] ?? 0, + counters.modelFacingUserTurnCount, + ); + } +} + +function getSessionModelFacingUserTurnCount(targetSession: Session): number { + return (targetSession as unknown as { modelFacingUserTurnCount: number }) + .modelFacingUserTurnCount; +} + describe('Session', () => { let mockChat: GeminiChat; let mockConfig: Config; @@ -370,6 +563,9 @@ describe('Session', () => { stripThoughtsFromHistory: vi.fn(), stripOrphanedUserEntriesFromHistory: vi.fn().mockReturnValue([]), } as unknown as GeminiChat; + vi.mocked(mockChat.getHistoryShallow).mockImplementation((curated) => + mockChat.getHistory(curated), + ); mockGeminiClient = { getChat: vi.fn().mockReturnValue(mockChat), isInitialized: vi.fn().mockReturnValue(true), @@ -756,15 +952,19 @@ describe('Session', () => { ]; vi.mocked(mockChat.getHistory).mockReturnValue(history); vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + setSessionTurnCounters(session, { modelFacingUserTurnCount: 2 }); const result = session.rewindToTurn(1); expect(result).toEqual({ targetTurnIndex: 1, apiTruncateIndex: 2 }); expect(mockChat.truncateHistory).toHaveBeenCalledWith(2); expect(mockChat.stripThoughtsFromHistory).toHaveBeenCalled(); + expect(mockFileHistoryService.restoreFromSnapshots).toHaveBeenCalledWith( + [], + ); expect(mockChatRecordingService.rewindRecording).toHaveBeenCalledWith( 1, - { truncatedCount: 2 }, + { truncatedCount: 2, maxModelFacingUserTurnCount: 2 }, [], ); }); @@ -795,180 +995,780 @@ describe('Session', () => { ).not.toHaveBeenCalled(); expect(mockChatRecordingService.rewindRecording).toHaveBeenCalledWith( 1, - { truncatedCount: 2 }, + { truncatedCount: 2, maxModelFacingUserTurnCount: 0 }, undefined, ); }); - it('preserves startup context when rewinding to the first user turn', () => { + it('preserves startup context when rewinding to the first user turn', () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { + text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, + }, + ], + }, + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + const result = session.rewindToTurn(0); + + expect(result).toEqual({ targetTurnIndex: 0, apiTruncateIndex: 1 }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(1); + }); + + it('counts only real user prompts as rewindable turns', () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { + text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, + }, + ], + }, + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + { + role: 'user', + parts: [ + { + text: `${SYSTEM_REMINDER_OPEN}\nNew tools available: foo\n${SYSTEM_REMINDER_CLOSE}`, + }, + ], + }, + { role: 'user', parts: [{ text: 'second' }] }, + ]; + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + expect(session.getRewindableUserTurnCount()).toBe(2); + }); + + it('counts only visible tail prompts as rewindable after compression', () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { + role: 'user', + parts: [ + { + text: + 'Recently accessed file (full current content embedded):\n\n' + + '## a.ts\n\n```ts\nexport const a = 1;\n```', + }, + ], + }, + { + role: 'model', + parts: [{ functionCall: { name: 'read_file', args: {} } }], + } as unknown as Content, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + ]; + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + expect(session.getRewindableUserTurnCount()).toBe(2); + }); + + it('does not count a mid-history MCP added-tool reminder as a user turn', () => { + // drainPendingAddedMcpToolsReminder injects a pure + // user entry mid-history. Counting it as a real turn would land the + // rewind one entry early, dropping the reminder plus a turn's context. + const history: Content[] = [ + { + role: 'user', + parts: [ + { + text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, + }, + ], + }, + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + { + role: 'user', + parts: [ + { + text: `${SYSTEM_REMINDER_OPEN}\nNew tools available: foo\n${SYSTEM_REMINDER_CLOSE}`, + }, + ], + }, + { role: 'user', parts: [{ text: 'second' }] }, + { role: 'model', parts: [{ text: 'second reply' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + const result = session.rewindToTurn(1); + + // Keep startup + turn 1 + the MCP reminder (indices 0–3); truncate at + // the second prompt (index 4). Counting the reminder would return 3. + expect(result).toEqual({ targetTurnIndex: 1, apiTruncateIndex: 4 }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(4); + }); + + it('maps ACP rewind to the uncompressed tail after chat compression', () => { + setSessionTurnCounters(session, { + turn: 3, + modelFacingUserTurnCount: 3, + }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + + const result = session.rewindToTurn(2); + + expect(result).toEqual({ targetTurnIndex: 2, apiTruncateIndex: 2 }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(2); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('matches rewind mapping after replayHistory when slash commands were persisted', async () => { + const records = [ + chatRecord({ + uuid: 'user-1', + message: { role: 'user', parts: [{ text: 'first' }] }, + }), + chatRecord({ + uuid: 'assistant-1', + type: 'assistant', + message: { role: 'model', parts: [{ text: 'first reply' }] }, + }), + chatRecord({ + uuid: 'slash-1', + message: { role: 'user', parts: [{ text: '/help' }] }, + }), + chatRecord({ + uuid: 'user-2', + message: { role: 'user', parts: [{ text: 'second' }] }, + }), + chatRecord({ + uuid: 'assistant-2', + type: 'assistant', + message: { role: 'model', parts: [{ text: 'second reply' }] }, + }), + ]; + const resumedHistory = buildApiHistoryFromConversation({ + sessionId: 'test-session-id', + projectHash: 'test-project-hash', + startTime: '2024-01-01T00:00:00Z', + lastUpdated: '2024-01-01T00:00:00Z', + messages: records, + }); + expect(resumedHistory).toEqual([ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + { role: 'user', parts: [{ text: 'second' }] }, + { role: 'model', parts: [{ text: 'second reply' }] }, + ]); + + vi.mocked(mockChat.getHistory).mockReturnValue(resumedHistory); + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(resumedHistory); + await session.replayHistory(records); + + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + expect(session.rewindToTurn(1)).toEqual({ + targetTurnIndex: 1, + apiTruncateIndex: 2, + }); + }); + + it('keeps compressed tail reachable after rewind and resend', async () => { + setSessionTurnCounters(session, { + turn: 3, + modelFacingUserTurnCount: 3, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + expect(session.rewindToTurn(2)).toEqual({ + targetTurnIndex: 2, + apiTruncateIndex: 2, + }); + + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'new third' }], + }); + + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'new third' }] }, + { role: 'model', parts: [{ text: 'new third reply' }] }, + ]); + mockChat.truncateHistory = vi.fn(); + + expect(session.rewindToTurn(2)).toEqual({ + targetTurnIndex: 2, + apiTruncateIndex: 2, + }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(2); + }); + + it('updates model-facing turn count through the real prompt send path', async () => { + setSessionTurnCounters(session, { + turn: 2, + modelFacingUserTurnCount: 2, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'third' }], + }); + + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + expect(() => session.rewindToTurn(1)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('uses model-facing turn count when slash commands advance prompt ids', () => { + setSessionTurnCounters(session, { + turn: 4, + modelFacingUserTurnCount: 3, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + const result = session.rewindToTurn(2); + + expect(result).toEqual({ targetTurnIndex: 2, apiTruncateIndex: 2 }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(2); + }); + + it('rejects compressed rewind targets when the model-facing count is too low', () => { + setSessionTurnCounters(session, { + turn: 3, + modelFacingUserTurnCount: 2, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + expect(() => session.rewindToTurn(2)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('uses model-facing turn count when cron adds user text entries', () => { + setSessionTurnCounters(session, { + turn: 2, + modelFacingUserTurnCount: 3, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'cron prompt' }] }, + { role: 'model', parts: [{ text: 'cron reply' }] }, + ]); + + expect(() => session.rewindToTurn(1)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('allows ACP rewind to the beginning after compression absorbed the first turn', () => { + setSessionTurnCounters(session, { + turn: 3, + modelFacingUserTurnCount: 3, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + expect(session.rewindToTurn(0)).toEqual({ + targetTurnIndex: 0, + apiTruncateIndex: 0, + }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(0); + }); + + it('does not treat post-compact attachment restoration as an ACP rewind target', () => { + setSessionTurnCounters(session, { + turn: 3, + modelFacingUserTurnCount: 3, + }); + vi.mocked(mockChat.getHistory).mockReturnValue([ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { + role: 'user', + parts: [ + { + text: + 'Recently accessed file (full current content embedded):\n\n' + + '## a.ts\n\n```ts\nexport const a = 1;\n```', + }, + ], + }, + { + role: 'model', + parts: [{ functionCall: { name: 'read_file', args: {} } }], + } as unknown as Content, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + ]); + + expect(() => session.rewindToTurn(1)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + + expect(session.rewindToTurn(2)).toEqual({ + targetTurnIndex: 2, + apiTruncateIndex: 4, + }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(4); + }); + + it('rejects unreachable user turns', () => { + const history: Content[] = [{ role: 'user', parts: [{ text: 'first' }] }]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + expect(() => session.rewindToTurn(2)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects rewinds while a cron prompt is mutating history', () => { + (session as unknown as { cronProcessing: boolean }).cronProcessing = true; + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects invalid target turn indexes', () => { + expect(() => session.rewindToTurn(-1)).toThrow( + 'targetTurnIndex must be a non-negative integer', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects rewinds while a prompt is running', () => { + (session as unknown as { pendingPrompt: AbortController }).pendingPrompt = + new AbortController(); + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects rewinds while a cron abort is active', () => { + ( + session as unknown as { cronAbortController: AbortController } + ).cronAbortController = new AbortController(); + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects rewinds while a notification prompt is processing', () => { + ( + session as unknown as { notificationProcessing: boolean } + ).notificationProcessing = true; + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('rejects rewinds while a notification abort controller is active', () => { + ( + session as unknown as { notificationAbortController: AbortController } + ).notificationAbortController = new AbortController(); + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('restores a captured history snapshot', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 1 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + ]; + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + const snapshot = session.captureHistorySnapshot(); + session.restoreHistory(snapshot); + + expect(snapshot).toEqual({ + history, + modelFacingUserTurnCount: 1, + }); + expect(mockChat.setHistory).toHaveBeenCalledWith(history); + const restoredHistory = vi.mocked(mockChat.setHistory).mock.calls[0]![0]; + expect(restoredHistory).not.toBe(snapshot.history); + expect(mockChat.getHistory).not.toHaveBeenCalled(); + }); + + it('captures the chat shallow history copy without deep-cloning payloads', () => { + const inlineData = { data: 'large-image', mimeType: 'image/png' }; + const history: Content[] = [{ role: 'user', parts: [{ inlineData }] }]; + vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); + + const snapshot = session.captureHistorySnapshot(); + + expect(snapshot.history).toBe(history); + expect(snapshot.history[0]!.parts![0]).toBe(history[0]!.parts![0]); + expect(mockChat.getHistory).not.toHaveBeenCalled(); + }); + + it('restores model-facing turn count with the history snapshot', () => { + setSessionTurnCounters(session, { + turn: 4, + modelFacingUserTurnCount: 4, + }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + { role: 'model', parts: [{ text: 'fourth reply' }] }, + ]; + vi.mocked(mockChat.getHistory).mockReturnValue(history); + + const snapshot = session.captureHistorySnapshot(); + expect(session.rewindToTurn(2)).toEqual({ + targetTurnIndex: 2, + apiTruncateIndex: 2, + }); + + session.restoreHistory(snapshot); + expect(getSessionModelFacingUserTurnCount(session)).toBe(4); + vi.mocked(mockChat.getHistory).mockReturnValue(history); + vi.mocked(mockChat.truncateHistory).mockClear(); + + expect(() => session.rewindToTurn(1)).toThrow( + 'Cannot rewind to the requested turn', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + }); + + it('validates history snapshots before mutating chat history', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 2 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: -1, + }), + ).toThrow( + 'modelFacingUserTurnCount must be a non-negative finite integer', + ); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('rejects history snapshots whose counter exceeds non-compressed user entries', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 2 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: 2, + }), + ).toThrow('exceeds visible model-facing user entries'); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('rejects history snapshots whose counter is lower than non-compressed user entries', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 2 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, + { role: 'user', parts: [{ text: 'second' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: 1, + }), + ).toThrow('is less than visible model-facing user entries'); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('rejects compressed history snapshots whose counter is lower than the visible tail', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 2 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of earlier turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: 1, + }), + ).toThrow('is less than visible model-facing user entries'); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('rejects compressed history snapshots whose counter exceeds the known model-facing count', () => { + setSessionTurnCounters(session, { + modelFacingUserTurnCount: 2, + maxModelFacingUserTurnCount: 4, + }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: 5, + }), + ).toThrow('exceeds known model-facing user entries'); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('derives model-facing turn count when restoring legacy history arrays', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); const history: Content[] = [ + { role: 'user', parts: [{ text: 'first' }] }, + { role: 'model', parts: [{ text: 'first reply' }] }, { role: 'user', - parts: [ - { - text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, - }, - ], + parts: [{ functionResponse: { name: 'tool', response: {} } }], }, - { role: 'user', parts: [{ text: 'first' }] }, - { role: 'model', parts: [{ text: 'first reply' }] }, + { role: 'user', parts: [{ text: 'second' }] }, ]; - vi.mocked(mockChat.getHistory).mockReturnValue(history); - vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); - const result = session.rewindToTurn(0); + session.restoreHistory(history); - expect(result).toEqual({ targetTurnIndex: 0, apiTruncateIndex: 1 }); - expect(mockChat.truncateHistory).toHaveBeenCalledWith(1); + expect(mockChat.setHistory).toHaveBeenCalledWith(history); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); - it('counts only real user prompts as rewindable turns', () => { + it('rewinds correctly after restoring legacy history arrays', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); const history: Content[] = [ - { - role: 'user', - parts: [ - { - text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, - }, - ], - }, { role: 'user', parts: [{ text: 'first' }] }, { role: 'model', parts: [{ text: 'first reply' }] }, - { - role: 'user', - parts: [ - { - text: `${SYSTEM_REMINDER_OPEN}\nNew tools available: foo\n${SYSTEM_REMINDER_CLOSE}`, - }, - ], - }, { role: 'user', parts: [{ text: 'second' }] }, + { role: 'model', parts: [{ text: 'second reply' }] }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, ]; + + session.restoreHistory(history); vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); - expect(session.getRewindableUserTurnCount()).toBe(2); + expect(session.rewindToTurn(1)).toEqual({ + targetTurnIndex: 1, + apiTruncateIndex: 2, + }); + expect(mockChat.truncateHistory).toHaveBeenCalledWith(2); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); }); - it('does not count a mid-history MCP added-tool reminder as a user turn', () => { - // drainPendingAddedMcpToolsReminder injects a pure - // user entry mid-history. Counting it as a real turn would land the - // rewind one entry early, dropping the reminder plus a turn's context. + it('does not count background notifications when restoring legacy history arrays', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); const history: Content[] = [ - { - role: 'user', - parts: [ - { - text: `${SYSTEM_REMINDER_OPEN}\nstartup context\n${SYSTEM_REMINDER_CLOSE}`, - }, - ], - }, { role: 'user', parts: [{ text: 'first' }] }, { role: 'model', parts: [{ text: 'first reply' }] }, { role: 'user', parts: [ { - text: `${SYSTEM_REMINDER_OPEN}\nNew tools available: foo\n${SYSTEM_REMINDER_CLOSE}`, + text: 'completed', }, ], }, + { role: 'model', parts: [{ text: 'noted' }] }, { role: 'user', parts: [{ text: 'second' }] }, - { role: 'model', parts: [{ text: 'second reply' }] }, ]; - vi.mocked(mockChat.getHistory).mockReturnValue(history); - vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); - - const result = session.rewindToTurn(1); - - // Keep startup + turn 1 + the MCP reminder (indices 0–3); truncate at - // the second prompt (index 4). Counting the reminder would return 3. - expect(result).toEqual({ targetTurnIndex: 1, apiTruncateIndex: 4 }); - expect(mockChat.truncateHistory).toHaveBeenCalledWith(4); - }); - - it('rejects unreachable user turns', () => { - const history: Content[] = [{ role: 'user', parts: [{ text: 'first' }] }]; - vi.mocked(mockChat.getHistory).mockReturnValue(history); - vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); - - expect(() => session.rewindToTurn(2)).toThrow( - 'Cannot rewind to the requested turn', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); - }); - - it('rejects rewinds while a cron prompt is mutating history', () => { - (session as unknown as { cronProcessing: boolean }).cronProcessing = true; - expect(() => session.rewindToTurn(0)).toThrow( - 'Cannot rewind while a prompt is running', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); - }); + session.restoreHistory(history); - it('rejects invalid target turn indexes', () => { - expect(() => session.rewindToTurn(-1)).toThrow( - 'targetTurnIndex must be a non-negative integer', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + expect(mockChat.setHistory).toHaveBeenCalledWith(history); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); - it('rejects rewinds while a prompt is running', () => { - (session as unknown as { pendingPrompt: AbortController }).pendingPrompt = - new AbortController(); - - expect(() => session.rewindToTurn(0)).toThrow( - 'Cannot rewind while a prompt is running', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); - }); + it('does not count compression summaries when restoring legacy history arrays', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + ]; - it('rejects rewinds while a cron abort is active', () => { - ( - session as unknown as { cronAbortController: AbortController } - ).cronAbortController = new AbortController(); + session.restoreHistory(history); - expect(() => session.rewindToTurn(0)).toThrow( - 'Cannot rewind while a prompt is running', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + expect(mockChat.setHistory).toHaveBeenCalledWith(history); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); - it('rejects rewinds while a notification prompt is processing', () => { - ( - session as unknown as { notificationProcessing: boolean } - ).notificationProcessing = true; - - expect(() => session.rewindToTurn(0)).toThrow( - 'Cannot rewind while a prompt is running', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); - }); + it('does not count post-compact attachment restoration when restoring legacy history arrays', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { + role: 'user', + parts: [ + { + text: + 'Recently accessed file (full current content embedded):\n\n' + + '## a.ts\n\n```ts\nexport const a = 1;\n```', + }, + ], + }, + { + role: 'model', + parts: [{ functionCall: { name: 'read_file', args: {} } }], + } as unknown as Content, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, + ]; - it('rejects rewinds while a notification abort controller is active', () => { - ( - session as unknown as { notificationAbortController: AbortController } - ).notificationAbortController = new AbortController(); + session.restoreHistory(history); - expect(() => session.rewindToTurn(0)).toThrow( - 'Cannot rewind while a prompt is running', - ); - expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + expect(mockChat.setHistory).toHaveBeenCalledWith(history); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); - it('restores a captured history snapshot', () => { + it('derives model-facing turn count with startup context and compression', () => { + setSessionTurnCounters(session, { modelFacingUserTurnCount: 99 }); const history: Content[] = [ - { role: 'user', parts: [{ text: 'first' }] }, - { role: 'model', parts: [{ text: 'first reply' }] }, + { role: 'user', parts: [{ text: 'startup context' }] }, + { + role: 'model', + parts: [{ text: 'Got it. Thanks for the context!' }], + }, + { role: 'user', parts: [{ text: 'summary of first two turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'third' }] }, + { role: 'model', parts: [{ text: 'third reply' }] }, + { role: 'user', parts: [{ text: 'fourth' }] }, ]; - vi.mocked(mockChat.getHistoryShallow).mockReturnValue(history); - const snapshot = session.captureHistorySnapshot(); - session.restoreHistory(snapshot); + session.restoreHistory(history); - expect(snapshot).toEqual(history); expect(mockChat.setHistory).toHaveBeenCalledWith(history); - expect(mockChat.getHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); it('rejects history restore while a prompt is running', () => { @@ -981,6 +1781,16 @@ describe('Session', () => { expect(mockChat.setHistory).not.toHaveBeenCalled(); }); + it('rejects empty history snapshots without mutating chat history', () => { + expect(() => + session.restoreHistory({ + history: [], + modelFacingUserTurnCount: 0, + }), + ).toThrow('Cannot restore an empty history snapshot'); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + }); + it('rejects history restore while a cron prompt is mutating history', () => { (session as unknown as { cronProcessing: boolean }).cronProcessing = true; @@ -1708,14 +2518,108 @@ describe('Session', () => { }, }, }); - expect(mockClient.extNotification).toHaveBeenCalledWith( - '_qwencode/end_turn', - { - sessionId: 'test-session-id', - reason: 'end_turn', - source: 'background_notification', - }, - ); + expect(mockClient.extNotification).toHaveBeenCalledWith( + '_qwencode/end_turn', + { + sessionId: 'test-session-id', + reason: 'end_turn', + source: 'background_notification', + }, + ); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); + }); + + it('does not advance prompt count when a compressed notification is stopped before streaming', async () => { + mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); + mockGeminiClient.tryCompressChat + .mockResolvedValueOnce({ + originalTokenCount: 50, + newTokenCount: 50, + compressionStatus: core.CompressionStatus.NOOP, + }) + .mockResolvedValueOnce({ + originalTokenCount: 1200, + newTokenCount: 101, + compressionStatus: core.CompressionStatus.COMPRESSED, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'start background work' }], + }); + + const callback = mockBackgroundTaskRegistry.setNotificationCallback.mock + .calls[0][0] as ( + displayText: string, + modelText: string, + meta: { agentId: string; status: string; toolUseId?: string }, + ) => void; + callback( + 'Background agent "worker" completed.', + 'completed', + { + agentId: 'agent-1', + status: 'completed', + toolUseId: 'tool-1', + }, + ); + + await vi.waitFor(() => { + expect(mockGeminiClient.tryCompressChat).toHaveBeenCalledTimes(2); + }); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); + }); + + it('does not advance prompt count when a dispatched notification stream throws', async () => { + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockImplementationOnce( + async (_model: string, request: { message: Part[] }) => { + mockChat.addHistory({ role: 'user', parts: request.message }); + return createFailingDispatchedStream('notification stream failed'); + }, + ); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'start background work' }], + }); + + const callback = mockBackgroundTaskRegistry.setNotificationCallback.mock + .calls[0][0] as ( + displayText: string, + modelText: string, + meta: { agentId: string; status: string; toolUseId?: string }, + ) => void; + callback( + 'Background agent "worker" completed.', + 'completed', + { + agentId: 'agent-1', + status: 'completed', + toolUseId: 'tool-1', + }, + ); + + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: expect.objectContaining({ + sessionUpdate: 'agent_message_chunk', + content: expect.objectContaining({ + type: 'text', + text: '[notification error] notification stream failed', + }), + }), + }); + }); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(2); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); }); it('cancels an in-flight background notification prompt', async () => { @@ -2169,6 +3073,47 @@ describe('Session', () => { ); }); + it('restores max model-facing turn count from replayed rewind records', async () => { + await session.replayHistory([ + chatRecord({ + uuid: 'user-1', + message: { parts: [{ text: '1' }] }, + }), + chatRecord({ + uuid: 'user-2', + timestamp: '2026-05-17T07:27:20.446Z', + message: { parts: [{ text: '2' }] }, + }), + chatRecord({ + uuid: 'rewind-1', + timestamp: '2026-05-17T07:27:22.000Z', + type: 'system', + subtype: 'rewind', + systemPayload: { + truncatedCount: 4, + maxModelFacingUserTurnCount: 5, + }, + }), + ]); + + const history: Content[] = [ + { role: 'user', parts: [{ text: 'summary of earlier turns' }] }, + { + role: 'model', + parts: [{ text: core.COMPRESSION_SUMMARY_MODEL_ACK }], + }, + { role: 'user', parts: [{ text: 'tail turn' }] }, + ]; + + expect(() => + session.restoreHistory({ + history, + modelFacingUserTurnCount: 5, + }), + ).not.toThrow(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(5); + }); + it('degrades an oversized inline image to a text placeholder before sending to the model', async () => { const ENV_KEY = 'QWEN_CODE_MAX_INLINE_MEDIA_BYTES'; const original = process.env[ENV_KEY]; @@ -3011,7 +3956,7 @@ describe('Session', () => { expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); }); - it('stops before sending when the compressed prompt exceeds the session token limit', async () => { + it('rolls back model-facing turn count when a compressed send is stopped before streaming', async () => { mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); mockGeminiClient.tryCompressChat.mockResolvedValueOnce({ originalTokenCount: 1200, @@ -3032,6 +3977,7 @@ describe('Session', () => { expect(mockGeminiClient.tryCompressChat).toHaveBeenCalled(); expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); expect(mockChat.addHistory).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(0); expect(mockClient.sessionUpdate).not.toHaveBeenCalledWith({ sessionId: 'test-session-id', update: { @@ -3058,6 +4004,66 @@ describe('Session', () => { }); }); + it('rolls back model-facing turn count when a non-compressed send is stopped before streaming', async () => { + mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); + mockGeminiClient.tryCompressChat.mockResolvedValueOnce({ + originalTokenCount: 101, + newTokenCount: 101, + compressionStatus: core.CompressionStatus.NOOP, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).resolves.toEqual({ stopReason: 'max_tokens' }); + + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + expect(getSessionModelFacingUserTurnCount(session)).toBe(0); + }); + + it('keeps model-facing turn count when a compressed stream throws after compression', async () => { + mockGeminiClient.tryCompressChat.mockResolvedValueOnce({ + originalTokenCount: 1200, + newTokenCount: 450, + compressionStatus: core.CompressionStatus.COMPRESSED, + }); + mockChat.sendMessageStream = createHistoryCommittingSendMessageStream( + mockChat, + () => createFailingDispatchedStream('stream failed'), + ) as unknown as typeof mockChat.sendMessageStream; + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toThrow('stream failed'); + + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); + }); + + it('keeps model-facing turn count when a dispatched non-compressed stream throws', async () => { + mockChat.sendMessageStream = createHistoryCommittingSendMessageStream( + mockChat, + () => createFailingDispatchedStream('stream failed'), + ) as unknown as typeof mockChat.sendMessageStream; + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toThrow('stream failed'); + + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); + expect(mockChat.addHistory).toHaveBeenCalled(); + }); + it('stops without throwing when the token-limit diagnostic fails', async () => { mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); mockGeminiClient.tryCompressChat.mockResolvedValueOnce({ @@ -3145,6 +4151,7 @@ describe('Session', () => { sendMessageStream, 1, ); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); }); it('injects drained mid-turn user messages with tool responses', async () => { @@ -4457,7 +5464,7 @@ describe('Session', () => { .mockResolvedValueOnce({ originalTokenCount: 101, newTokenCount: 101, - compressionStatus: core.CompressionStatus.NOOP, + compressionStatus: core.CompressionStatus.COMPRESSED, }); mockChat.sendMessageStream = vi .fn() @@ -4506,6 +5513,7 @@ describe('Session', () => { }), ], }); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ sessionId: 'test-session-id', update: { @@ -4575,6 +5583,7 @@ describe('Session', () => { sendMessageStream, 1, ); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); }); it('skips automatic compression after the first Stop-hook continuation', async () => { @@ -4677,7 +5686,7 @@ describe('Session', () => { .mockResolvedValueOnce({ originalTokenCount: 101, newTokenCount: 101, - compressionStatus: core.CompressionStatus.NOOP, + compressionStatus: core.CompressionStatus.COMPRESSED, }); mockChat.getHistory = vi .fn() @@ -4706,6 +5715,7 @@ describe('Session', () => { expect.any(AbortSignal), ); expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ sessionId: 'test-session-id', update: { @@ -4720,6 +5730,106 @@ describe('Session', () => { }); }); + it('rolls back Stop-hook continuation count when a non-compressed send is stopped before streaming', async () => { + const messageBus = { + request: vi + .fn() + .mockResolvedValueOnce({ + success: true, + output: { + decision: 'block', + reason: 'Continue after Stop hook', + }, + }) + .mockResolvedValueOnce({ + success: true, + output: {}, + }), + }; + mockConfig.getMessageBus = vi.fn().mockReturnValue(messageBus); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); + mockConfig.hasHooksForEvent = vi + .fn() + .mockImplementation((eventName: string) => eventName === 'Stop'); + mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); + mockGeminiClient.tryCompressChat + .mockResolvedValueOnce({ + originalTokenCount: 50, + newTokenCount: 50, + compressionStatus: core.CompressionStatus.NOOP, + }) + .mockResolvedValueOnce({ + originalTokenCount: 101, + newTokenCount: 101, + compressionStatus: core.CompressionStatus.NOOP, + }); + mockChat.getHistory = vi + .fn() + .mockReturnValue([ + { role: 'model', parts: [{ text: 'response text' }] }, + ]); + mockChat.getLastModelMessageText = vi + .fn() + .mockReturnValue('response text'); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).resolves.toEqual({ stopReason: 'max_tokens' }); + + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); + }); + + it('keeps Stop-hook continuation count when a dispatched non-compressed stream throws', async () => { + const messageBus = { + request: vi.fn().mockResolvedValueOnce({ + success: true, + output: { + decision: 'block', + reason: 'Continue after Stop hook', + }, + }), + }; + mockConfig.getMessageBus = vi.fn().mockReturnValue(messageBus); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); + mockConfig.hasHooksForEvent = vi + .fn() + .mockImplementation((eventName: string) => eventName === 'Stop'); + mockChat.getHistory = vi + .fn() + .mockReturnValue([ + { role: 'model', parts: [{ text: 'response text' }] }, + ]); + mockChat.getLastModelMessageText = vi + .fn() + .mockReturnValue('response text'); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockImplementation(async (_model, request: { message: Part[] }) => { + mockChat.addHistory({ role: 'user', parts: request.message }); + return createFailingDispatchedStream( + 'stop continuation stream failed', + ); + }); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toThrow('stop continuation stream failed'); + + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(2); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + it('runs automatic compression before cron-fired ACP prompt sends', async () => { const scheduler = { size: 1, @@ -6537,7 +7647,7 @@ describe('Session', () => { .mockResolvedValueOnce({ originalTokenCount: 101, newTokenCount: 101, - compressionStatus: core.CompressionStatus.NOOP, + compressionStatus: core.CompressionStatus.COMPRESSED, }); mockChat.sendMessageStream = vi .fn() @@ -6560,6 +7670,7 @@ describe('Session', () => { expect.any(AbortSignal), ); expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(getSessionModelFacingUserTurnCount(session)).toBe(1); expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ sessionId: 'test-session-id', update: { @@ -6616,6 +7727,100 @@ describe('Session', () => { expect(tokenLimitDiagnosticCount()).toBe(diagnosticCountBefore); }); + it('keeps cron prompt count when a dispatched non-compressed stream throws', async () => { + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn((callback: (job: { prompt: string }) => void) => { + callback({ prompt: 'scheduled prompt' }); + }), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockGeminiClient.tryCompressChat + .mockResolvedValueOnce({ + originalTokenCount: 50, + newTokenCount: 50, + compressionStatus: core.CompressionStatus.NOOP, + }) + .mockResolvedValueOnce({ + originalTokenCount: 50, + newTokenCount: 50, + compressionStatus: core.CompressionStatus.NOOP, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(createEmptyStream()) + .mockImplementation(async (_model, request: { message: Part[] }) => { + mockChat.addHistory({ role: 'user', parts: request.message }); + return createFailingDispatchedStream('cron stream failed'); + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(2); + }); + await vi.waitFor(() => { + expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ + sessionId: 'test-session-id', + update: { + sessionUpdate: 'agent_message_chunk', + content: { + type: 'text', + text: '[cron error] cron stream failed', + }, + }, + }); + }); + expect(getSessionModelFacingUserTurnCount(session)).toBe(2); + }); + + it('rolls back cron prompt count when a non-compressed send is stopped before streaming', async () => { + const scheduler = { + size: 1, + hasPendingWork: true, + start: vi.fn((callback: (job: { prompt: string }) => void) => { + callback({ prompt: 'scheduled prompt' }); + }), + stop: vi.fn(), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockConfig.getSessionTokenLimit = vi.fn().mockReturnValue(100); + mockGeminiClient.tryCompressChat + .mockResolvedValueOnce({ + originalTokenCount: 50, + newTokenCount: 50, + compressionStatus: core.CompressionStatus.NOOP, + }) + .mockResolvedValueOnce({ + originalTokenCount: 101, + newTokenCount: 101, + compressionStatus: core.CompressionStatus.NOOP, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(mockGeminiClient.tryCompressChat).toHaveBeenCalledTimes(2); + }); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(1); + expect(getSessionModelFacingUserTurnCount(session)).toBe(0); + }); + it('does not auto-compress slash commands handled without a model send', async () => { vi.mocked( nonInteractiveCliCommands.handleSlashCommand, diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 43a888bf7ee..cfc1d618cbb 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -162,6 +162,12 @@ import { type HistoryItemGoalStatus, } from '../../ui/types.js'; import { parseAcpModelOption } from '../../utils/acpModelUtils.js'; +import { + getApiUserTextIndices, + getCompressionTailStartIndex, + hasCompressionSummaryPair, + isApiUserTextContent, +} from '../../utils/api-history-utils.js'; import { classifyApiError } from '../../ui/hooks/useGeminiStream.js'; import { getPersistScopeForModelSelection } from '../../config/modelProvidersScope.js'; import { @@ -205,8 +211,116 @@ function maskApiKeyForDisplay(apiKey: string | undefined): string { } type AutoCompressionSendResult = - | { responseStream: AsyncGenerator; stopReason?: never } - | { responseStream: null; stopReason: PromptResponse['stopReason'] }; + | { + responseStream: AsyncGenerator; + stopReason?: never; + } + | { + responseStream: null; + stopReason: PromptResponse['stopReason']; + }; + +export interface HistorySnapshot { + history: Content[]; + modelFacingUserTurnCount: number; +} + +export function isHistorySnapshot(value: unknown): value is HistorySnapshot { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false; + } + const snapshot = value as { + history?: unknown; + modelFacingUserTurnCount?: unknown; + }; + const count = snapshot.modelFacingUserTurnCount; + return ( + Array.isArray(snapshot.history) && + snapshot.history.length > 0 && + typeof count === 'number' && + Number.isInteger(count) && + Number.isFinite(count) && + count >= 0 && + count <= Number.MAX_SAFE_INTEGER + ); +} + +function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { + const startIndex = getStartupContextLength(apiHistory); + if (hasCompressionSummaryPair(apiHistory, startIndex)) { + return getApiUserTextIndices( + apiHistory, + getCompressionTailStartIndex(apiHistory, startIndex), + ).length; + } + return getApiUserTextIndices(apiHistory, startIndex).length; +} + +function validateModelFacingUserTurnCount(count: unknown): number { + if (typeof count !== 'number' || !Number.isInteger(count)) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount must be an integer, got ${typeof count}`, + ); + } + if (!Number.isFinite(count) || count < 0) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount must be a non-negative finite integer, got ${count}`, + ); + } + if (count > Number.MAX_SAFE_INTEGER) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount exceeds maximum safe integer, got ${count}`, + ); + } + return count; +} + +function validateModelFacingUserTurnCountForHistory( + history: Content[], + count: unknown, + maxKnownModelFacingUserTurnCount: number, +): number { + const validatedCount = validateModelFacingUserTurnCount(count); + const startIndex = getStartupContextLength(history); + + if (hasCompressionSummaryPair(history, startIndex)) { + const visibleTailTurnCount = getApiUserTextIndices( + history, + getCompressionTailStartIndex(history, startIndex), + ).length; + if (validatedCount < visibleTailTurnCount) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount ${validatedCount} is less than visible model-facing user entries ${visibleTailTurnCount}`, + ); + } + if (validatedCount > maxKnownModelFacingUserTurnCount) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount ${validatedCount} exceeds known model-facing user entries ${maxKnownModelFacingUserTurnCount}`, + ); + } + return validatedCount; + } + + const visibleTurnCount = computeVisibleModelFacingUserTurnCount(history); + if (validatedCount < visibleTurnCount) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount ${validatedCount} is less than visible model-facing user entries ${visibleTurnCount}`, + ); + } + if (validatedCount > visibleTurnCount) { + throw RequestError.invalidParams( + undefined, + `modelFacingUserTurnCount ${validatedCount} exceeds visible model-facing user entries ${visibleTurnCount}`, + ); + } + return validatedCount; +} type RunToolResult = { parts: Part[]; @@ -571,6 +685,51 @@ export function computeInitialTurnFromHistory( return maxPromptTurn > 0 ? maxPromptTurn : userMessageCount; } +export function computeInitialModelFacingUserTurnCountFromHistory( + records: ChatRecord[], + sessionId: string, +): number { + return records.filter( + (record) => + record.sessionId === sessionId && isModelFacingUserPromptRecord(record), + ).length; +} + +export function computeMaxModelFacingUserTurnCountFromHistory( + records: ChatRecord[], + sessionId: string, +): number { + let maxModelFacingUserTurnCount = 0; + + for (const record of records) { + if ( + record.sessionId !== sessionId || + record.type !== 'system' || + record.subtype !== 'rewind' + ) { + continue; + } + + const count = ( + record.systemPayload as { maxModelFacingUserTurnCount?: unknown } + )?.maxModelFacingUserTurnCount; + if ( + typeof count === 'number' && + Number.isInteger(count) && + Number.isFinite(count) && + count >= 0 && + count <= Number.MAX_SAFE_INTEGER + ) { + maxModelFacingUserTurnCount = Math.max( + maxModelFacingUserTurnCount, + count, + ); + } + } + + return maxModelFacingUserTurnCount; +} + export async function fireSessionPermissionDeniedForAutoMode( config: Config, decision: AutoModeDecision, @@ -639,6 +798,35 @@ function isUserPromptRecord(record: ChatRecord): boolean { ); } +function isModelFacingUserPromptRecord(record: ChatRecord): boolean { + if (record.type !== 'user') { + return false; + } + if ( + record.subtype === 'notification' || + record.subtype === 'mid_turn_user_message' + ) { + return false; + } + const textParts = + record.message?.parts + ?.filter( + (part): part is { text: string } & Part => + 'text' in part && + typeof part.text === 'string' && + part.text.trim().length > 0, + ) + .map((part) => part.text.trim()) ?? []; + if (textParts.length === 0) { + return false; + } + if (record.subtype === 'cron') { + return true; + } + const fullText = textParts.join(' '); + return !fullText.startsWith('?') && !isSlashCommand(fullText); +} + const AT_TOKEN_RE = /@([^\s,;!?()[\]{}]+)/g; function collectExtensionMentionRefs( @@ -789,6 +977,8 @@ export class Session implements SessionContext { */ private followupAbort: AbortController | null = null; private turn: number = 0; + private modelFacingUserTurnCount: number = 0; + private maxModelFacingUserTurnCount: number = 0; private readonly createdAt: number = Date.now(); /** * Running cumulative usage for this session, snapshotted onto each todo/plan @@ -1007,6 +1197,23 @@ export class Session implements SessionContext { this.turn, computeInitialTurnFromHistory(records, this.config.getSessionId()), ); + const initialModelFacingUserTurnCount = + computeInitialModelFacingUserTurnCountFromHistory( + records, + this.config.getSessionId(), + ); + this.modelFacingUserTurnCount = Math.max( + this.modelFacingUserTurnCount, + initialModelFacingUserTurnCount, + ); + this.maxModelFacingUserTurnCount = Math.max( + this.maxModelFacingUserTurnCount, + this.modelFacingUserTurnCount, + computeMaxModelFacingUserTurnCountFromHistory( + records, + this.config.getSessionId(), + ), + ); await this.historyReplayer.replay(records); } @@ -1053,6 +1260,9 @@ export class Session implements SessionContext { chat.truncateHistory(apiTruncateIndex); chat.stripThoughtsFromHistory(); + // targetTurnIndex is zero-based; after truncating before that turn, + // exactly targetTurnIndex model-facing user turns remain. + this.modelFacingUserTurnCount = targetTurnIndex; const rewindFiles = opts?.rewindFiles !== false; const fileHistoryService = this.config.getFileHistoryService(); @@ -1064,36 +1274,31 @@ export class Session implements SessionContext { fileHistoryService.restoreFromSnapshots(survivingSnapshots); } - this.config - .getChatRecordingService() - ?.rewindRecording( - targetTurnIndex, - { truncatedCount: Math.max(0, apiHistory.length - apiTruncateIndex) }, - survivingSnapshots, - ); + this.config.getChatRecordingService()?.rewindRecording( + targetTurnIndex, + { + truncatedCount: Math.max(0, apiHistory.length - apiTruncateIndex), + maxModelFacingUserTurnCount: this.maxModelFacingUserTurnCount, + }, + survivingSnapshots, + ); return { targetTurnIndex, apiTruncateIndex }; } - captureHistorySnapshot(): Content[] { - return this.config.getGeminiClient()!.getChat().getHistoryShallow(); + captureHistorySnapshot(): HistorySnapshot { + return { + history: this.config.getGeminiClient()!.getChat().getHistoryShallow(), + modelFacingUserTurnCount: this.modelFacingUserTurnCount, + }; } getRewindableUserTurnCount(): number { - const apiHistory = this.captureHistorySnapshot(); - const startIndex = getStartupContextLength(apiHistory); - let count = 0; - - for (let i = startIndex; i < apiHistory.length; i++) { - if (this.#isUserTextContent(apiHistory[i]!)) { - count += 1; - } - } - - return count; + const { history: apiHistory } = this.captureHistorySnapshot(); + return computeVisibleModelFacingUserTurnCount(apiHistory); } - restoreHistory(history: Content[]): void { + restoreHistory(snapshot: Content[] | HistorySnapshot): void { if ( this.pendingPrompt || this.cronProcessing || @@ -1107,10 +1312,29 @@ export class Session implements SessionContext { ); } + const history = Array.isArray(snapshot) ? snapshot : snapshot.history; + if (history.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Cannot restore an empty history snapshot', + ); + } + const newModelFacingUserTurnCount = Array.isArray(snapshot) + ? computeVisibleModelFacingUserTurnCount(history) + : validateModelFacingUserTurnCountForHistory( + history, + snapshot.modelFacingUserTurnCount, + this.maxModelFacingUserTurnCount, + ); this.config .getGeminiClient()! .getChat() .setHistory(structuredClone(history)); + this.modelFacingUserTurnCount = newModelFacingUserTurnCount; + this.maxModelFacingUserTurnCount = Math.max( + this.maxModelFacingUserTurnCount, + newModelFacingUserTurnCount, + ); } #computeApiTruncationIndexForUserTurn( @@ -1123,40 +1347,59 @@ export class Session implements SessionContext { return startIndex; } - let realUserPromptCount = 0; - for (let i = startIndex; i < apiHistory.length; i++) { - if (!this.#isUserTextContent(apiHistory[i]!)) { - continue; + if (hasCompressionSummaryPair(apiHistory, startIndex)) { + const apiTailUserIndices = getApiUserTextIndices( + apiHistory, + getCompressionTailStartIndex(apiHistory, startIndex), + ); + if (this.modelFacingUserTurnCount < targetTurnIndex + 1) { + debugLogger.warn( + `Cannot rewind to user turn ${targetTurnIndex}; ` + + `modelFacingUserTurnCount=${this.modelFacingUserTurnCount}, ` + + `apiHistoryLength=${apiHistory.length}, startIndex=${startIndex}.`, + ); + return -1; } - - if (realUserPromptCount === targetTurnIndex) { - return i; + const totalUserTurns = this.modelFacingUserTurnCount; + if (totalUserTurns < apiTailUserIndices.length) { + debugLogger.warn( + `Inconsistent compressed rewind state for turn ${targetTurnIndex}: ` + + `modelFacingUserTurnCount=${totalUserTurns}, ` + + `apiTailUserIndices.length=${apiTailUserIndices.length}, ` + + `apiHistoryLength=${apiHistory.length}, startIndex=${startIndex}.`, + ); } + const compressedTurnCount = Math.max( + 0, + totalUserTurns - apiTailUserIndices.length, + ); - realUserPromptCount += 1; - } - - return -1; - } - - #isUserTextContent(content: Content): boolean { - if (content.role !== 'user') return false; - if (!content.parts || content.parts.length === 0) return false; + if (targetTurnIndex < compressedTurnCount) { + debugLogger.warn( + `Rewind to turn ${targetTurnIndex} rejected after compression: ` + + `compressedTurnCount=${compressedTurnCount}, ` + + `modelFacingUserTurnCount=${totalUserTurns}, ` + + `apiTailUserIndices.length=${apiTailUserIndices.length}, ` + + `apiHistoryLength=${apiHistory.length}, startIndex=${startIndex}.`, + ); + return -1; + } - const hasFunctionResponse = content.parts.some( - (part) => 'functionResponse' in part, - ); - if (hasFunctionResponse) return false; + const tailOffset = targetTurnIndex - compressedTurnCount; + if (tailOffset < 0 || tailOffset >= apiTailUserIndices.length) { + debugLogger.warn( + `Compressed rewind index out of bounds: targetTurnIndex=${targetTurnIndex}, ` + + `compressedTurnCount=${compressedTurnCount}, ` + + `apiTailUserIndices.length=${apiTailUserIndices.length}, ` + + `apiHistoryLength=${apiHistory.length}, startIndex=${startIndex}.`, + ); + return -1; + } - // Exclude pure entries (the startup prelude and the - // mid-history MCP added-tool reminders). They are structural, not real - // user prompts; counting them would shift the rewind truncation index and - // silently drop a real turn. A genuine user turn that merely has a - // per-turn reminder prepended still has a non-reminder prompt part, so it - // is NOT excluded. - if (isSystemReminderContent(content)) return false; + return apiTailUserIndices[tailOffset]!; + } - return content.parts.some((part) => 'text' in part && part.text); + return getApiUserTextIndices(apiHistory, startIndex)[targetTurnIndex] ?? -1; } async cancelPendingPrompt(): Promise { @@ -1745,6 +1988,7 @@ export class Session implements SessionContext { turnCount++; if (pendingSend.signal.aborted) { this.#getCurrentChat().addHistory(nextMessage); + this.#recordModelFacingUserTurn(nextMessage); return { stopReason: 'cancelled' }; } @@ -1752,8 +1996,12 @@ export class Session implements SessionContext { let usageMetadata: GenerateContentResponseUsageMetadata | null = null; const streamStartTime = Date.now(); + let recordedModelFacingTurn = false; + let sendDispatched = false; try { + recordedModelFacingTurn = + this.#recordModelFacingUserTurn(nextMessage); const sendResult = await this.#sendMessageStreamWithAutoCompression( promptId, @@ -1761,6 +2009,11 @@ export class Session implements SessionContext { pendingSend.signal, ); if (!sendResult.responseStream) { + if (sendResult.stopReason !== 'cancelled') { + this.#rollbackModelFacingUserTurn( + recordedModelFacingTurn, + ); + } // Preserve the full message (not just functionResponse // parts) for a continuation: its content was stripped from // history before the send, so dropping it here on a @@ -1772,6 +2025,7 @@ export class Session implements SessionContext { ); return { stopReason: sendResult.stopReason }; } + sendDispatched = true; const responseStream = sendResult.responseStream; nextMessage = null; @@ -1814,6 +2068,9 @@ export class Session implements SessionContext { } } } catch (error) { + if (!sendDispatched) { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); + } // Restore the stripped orphan if the send threw before // re-pushing it (the null-stream path above already preserves; // an exception bypasses it). Gate on the push counter — like @@ -2071,8 +2328,12 @@ export class Session implements SessionContext { const functionCalls: FunctionCall[] = []; let usageMetadata: GenerateContentResponseUsageMetadata | null = null; const streamStartTime = Date.now(); + let recordedModelFacingTurn = false; + let sendDispatched = false; try { + recordedModelFacingTurn = + this.#recordModelFacingUserTurn(nextMessage); const continueSendResult = await this.#sendMessageStreamWithAutoCompression( promptId + '_stop_hook_' + stopHookIterationCount, @@ -2081,12 +2342,16 @@ export class Session implements SessionContext { { skipCompression: stopHookIterationCount > 1 }, ); if (!continueSendResult.responseStream) { + if (continueSendResult.stopReason !== 'cancelled') { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); + } this.#preserveUnsentMessageHistory( nextMessage, continueSendResult.stopReason === 'cancelled', ); return { stopReason: continueSendResult.stopReason }; } + sendDispatched = true; const continueResponseStream = continueSendResult.responseStream; nextMessage = null; @@ -2126,6 +2391,10 @@ export class Session implements SessionContext { } } } catch (error) { + if (!sendDispatched) { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); + } + // Fire StopFailure hook (fire-and-forget) const errorStatus = getErrorStatus(error); const errorMessage = @@ -2216,6 +2485,27 @@ export class Session implements SessionContext { return this.config.getGeminiClient()!.getChat(); } + #recordModelFacingUserTurn(message: Content): boolean { + if (isApiUserTextContent(message)) { + this.modelFacingUserTurnCount += 1; + this.maxModelFacingUserTurnCount = Math.max( + this.maxModelFacingUserTurnCount, + this.modelFacingUserTurnCount, + ); + return true; + } + return false; + } + + #rollbackModelFacingUserTurn(recorded: boolean): void { + if (recorded) { + this.modelFacingUserTurnCount = Math.max( + 0, + this.modelFacingUserTurnCount - 1, + ); + } + } + /** * Mirrors the core send path for ACP model sends. * @@ -2260,7 +2550,10 @@ export class Session implements SessionContext { } catch (compressionError) { if (abortSignal.aborted || this.#isAbortError(compressionError)) { debugLogger.debug(`Auto-compression aborted for prompt ${promptId}`); - return { responseStream: null, stopReason: 'cancelled' }; + return { + responseStream: null, + stopReason: 'cancelled', + }; } debugLogger.warn( `Auto-compression failed for prompt ${promptId}; proceeding without compression: ` + @@ -2271,7 +2564,10 @@ export class Session implements SessionContext { if (abortSignal.aborted) { debugLogger.debug(`Auto-compression aborted for prompt ${promptId}`); - return { responseStream: null, stopReason: 'cancelled' }; + return { + responseStream: null, + stopReason: 'cancelled', + }; } if (!compressionInfo) { @@ -2292,7 +2588,10 @@ export class Session implements SessionContext { 'Please start a new session or increase the sessionTokenLimit in your settings.json.', `Failed to emit token limit diagnostic for prompt ${promptId}`, ); - return { responseStream: null, stopReason: 'max_tokens' }; + return { + responseStream: null, + stopReason: 'max_tokens', + }; } } @@ -2307,7 +2606,10 @@ export class Session implements SessionContext { debugLogger.debug( `Send aborted after compression diagnostic for prompt ${promptId}`, ); - return { responseStream: null, stopReason: 'cancelled' }; + return { + responseStream: null, + stopReason: 'cancelled', + }; } const responseStream = await this.#getCurrentChat().sendMessageStream( @@ -2989,78 +3291,95 @@ export class Session implements SessionContext { let usageMetadata: GenerateContentResponseUsageMetadata | null = null; const streamStartTime = Date.now(); + let recordedModelFacingTurn = false; + let sendDispatched = false; - const sendResult = - await this.#sendMessageStreamWithAutoCompression( - promptId, - nextMessage.parts ?? [], - ac.signal, - ); - if (!sendResult.responseStream) { - this.#preserveUnsentMessageHistory( - nextMessage, - sendResult.stopReason === 'cancelled', - ); - if (sendResult.stopReason === 'max_tokens') { - this.#stopCronAfterTokenLimit(); + try { + recordedModelFacingTurn = + this.#recordModelFacingUserTurn(nextMessage); + const sendResult = + await this.#sendMessageStreamWithAutoCompression( + promptId, + nextMessage.parts ?? [], + ac.signal, + ); + if (!sendResult.responseStream) { + if (sendResult.stopReason !== 'cancelled') { + this.#rollbackModelFacingUserTurn( + recordedModelFacingTurn, + ); + } + this.#preserveUnsentMessageHistory( + nextMessage, + sendResult.stopReason === 'cancelled', + ); + if (sendResult.stopReason === 'max_tokens') { + this.#stopCronAfterTokenLimit(); + } + return; } - return; - } - const responseStream = sendResult.responseStream; - if (loopTick && turnCount === 1) { - // The block reached the model (the send started); commit it so - // the next tick can detect "unchanged". Deferring the commit - // to here keeps an abort before delivery from poisoning the - // cache into a dangling short reminder. - this.loopTickResolver?.markDelivered(); - } - nextMessage = null; + sendDispatched = true; + const responseStream = sendResult.responseStream; + if (loopTick && turnCount === 1) { + // The block reached the model (the send started); commit it so + // the next tick can detect "unchanged". Deferring the commit + // to here keeps an abort before delivery from poisoning the + // cache into a dangling short reminder. + this.loopTickResolver?.markDelivered(); + } + nextMessage = null; - for await (const resp of responseStream) { - if (ac.signal.aborted) return; + for await (const resp of responseStream) { + if (ac.signal.aborted) return; - if ( - resp.type === StreamEventType.CHUNK && - resp.value.candidates && - resp.value.candidates.length > 0 - ) { - const candidate = resp.value.candidates[0]; - for (const part of candidate.content?.parts ?? []) { - if (!part.text) continue; - this.messageEmitter.emitMessage( - part.text, - 'assistant', - part.thought, - ); + if ( + resp.type === StreamEventType.CHUNK && + resp.value.candidates && + resp.value.candidates.length > 0 + ) { + const candidate = resp.value.candidates[0]; + for (const part of candidate.content?.parts ?? []) { + if (!part.text) continue; + this.messageEmitter.emitMessage( + part.text, + 'assistant', + part.thought, + ); + } } - } - if ( - resp.type === StreamEventType.CHUNK && - resp.value.usageMetadata - ) { - usageMetadata = resp.value.usageMetadata; - } + if ( + resp.type === StreamEventType.CHUNK && + resp.value.usageMetadata + ) { + usageMetadata = resp.value.usageMetadata; + } - if ( - resp.type === StreamEventType.CHUNK && - resp.value.functionCalls - ) { - functionCalls.push(...resp.value.functionCalls); + if ( + resp.type === StreamEventType.CHUNK && + resp.value.functionCalls + ) { + functionCalls.push(...resp.value.functionCalls); + } } - } - if (usageMetadata) { - this.#recordPromptTokenCount(usageMetadata); - if (this.messageRewriter) { - this.messageRewriter.flushTurn(ac.signal); + if (usageMetadata) { + this.#recordPromptTokenCount(usageMetadata); + if (this.messageRewriter) { + this.messageRewriter.flushTurn(ac.signal); + } + const durationMs = Date.now() - streamStartTime; + await this.messageEmitter.emitUsageMetadata( + usageMetadata, + '', + durationMs, + ); } - const durationMs = Date.now() - streamStartTime; - await this.messageEmitter.emitUsageMetadata( - usageMetadata, - '', - durationMs, - ); + } catch (error) { + if (!sendDispatched) { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); + } + throw error; } if (functionCalls.length > 0) { @@ -3297,65 +3616,78 @@ export class Session implements SessionContext { null; let responseText = ''; const streamStartTime = Date.now(); + const recordedModelFacingTurn = false; + let sendDispatched = false; - const sendResult = await this.#sendMessageStreamWithAutoCompression( - promptId, - nextMessage.parts ?? [], - ac.signal, - ); - if (!sendResult.responseStream) { - this.#preserveUnsentMessageHistory( - nextMessage, - sendResult.stopReason === 'cancelled', - ); - await this.#emitBackgroundNotificationEndTurn( - sendResult.stopReason, - ); - return; - } - - const responseStream = sendResult.responseStream; - nextMessage = null; - - for await (const resp of responseStream) { - if (ac.signal.aborted) { - await this.#emitBackgroundNotificationEndTurn('cancelled'); + try { + const sendResult = + await this.#sendMessageStreamWithAutoCompression( + promptId, + nextMessage.parts ?? [], + ac.signal, + ); + if (!sendResult.responseStream) { + if (sendResult.stopReason !== 'cancelled') { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); + } + this.#preserveUnsentMessageHistory( + nextMessage, + sendResult.stopReason === 'cancelled', + ); + await this.#emitBackgroundNotificationEndTurn( + sendResult.stopReason, + ); return; } + sendDispatched = true; + const responseStream = sendResult.responseStream; + nextMessage = null; - if ( - resp.type === StreamEventType.CHUNK && - resp.value.candidates && - resp.value.candidates.length > 0 - ) { - const candidate = resp.value.candidates[0]; - for (const part of candidate.content?.parts ?? []) { - if (!part.text) continue; - if (part.thought) { - await this.messageEmitter.emitMessage( - part.text, - 'assistant', - true, - ); - } else { - responseText += part.text; + for await (const resp of responseStream) { + if (ac.signal.aborted) { + await this.#emitBackgroundNotificationEndTurn('cancelled'); + return; + } + + if ( + resp.type === StreamEventType.CHUNK && + resp.value.candidates && + resp.value.candidates.length > 0 + ) { + const candidate = resp.value.candidates[0]; + for (const part of candidate.content?.parts ?? []) { + if (!part.text) continue; + if (part.thought) { + await this.messageEmitter.emitMessage( + part.text, + 'assistant', + true, + ); + } else { + responseText += part.text; + } } } - } - if ( - resp.type === StreamEventType.CHUNK && - resp.value.usageMetadata - ) { - usageMetadata = resp.value.usageMetadata; - } + if ( + resp.type === StreamEventType.CHUNK && + resp.value.usageMetadata + ) { + usageMetadata = resp.value.usageMetadata; + } - if ( - resp.type === StreamEventType.CHUNK && - resp.value.functionCalls - ) { - functionCalls.push(...resp.value.functionCalls); + if ( + resp.type === StreamEventType.CHUNK && + resp.value.functionCalls + ) { + functionCalls.push(...resp.value.functionCalls); + } + } + } catch (error) { + if (!sendDispatched) { + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); } + throw error; } if (responseText.length > 0) { diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 19c090eddee..7577d51d0f1 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1769,6 +1769,14 @@ describe('mergeExcludeTools', () => { expect(config.getPermissionsDeny()).toContain('tool_search'); }); + it('should auto-disable tool_search for deepseek-v4-pro models', async () => { + process.argv = ['node', 'script.js', '--model', 'deepseek-v4-pro']; + const argv = await parseArguments(); + const settings: Settings = {}; + const config = await loadCliConfig(settings, argv, undefined, []); + expect(config.getPermissionsDeny()).toContain('tool_search'); + }); + it('should auto-disable tool_search for deepseek-v3 models', async () => { process.argv = ['node', 'script.js', '--model', 'deepseek-v3']; const argv = await parseArguments(); diff --git a/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts b/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts index 7bacebab419..8e8fa3b0155 100644 --- a/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts +++ b/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts @@ -61,9 +61,7 @@ const fakeWorkspace = {} as unknown as DaemonWorkspaceService; */ class AgentSideProvider implements ClientMcpServerProvider { readonly clients = new Map(); - lastToolList: - | Awaited> - | undefined; + lastToolList: Awaited> | undefined; async registerClientMcpServer( serverName: string, @@ -145,7 +143,10 @@ function answerHandshakeFrame(frame: { payload: { jsonrpc: '2.0', id: payload.id, - error: { code: -32601, message: `method not found: ${payload.method}` }, + error: { + code: -32601, + message: `method not found: ${payload.method}`, + }, } as JSONRPCMessage, }; } @@ -177,9 +178,7 @@ describe('client_mcp_over_ws reverse channel (serve layer)', () => { workspace: fakeWorkspace, enabled: true, clientMcpOverWs: opts.clientMcpOverWs ?? true, - ...(opts.withProvider === false - ? {} - : { clientMcpProvider: provider }), + ...(opts.withProvider === false ? {} : { clientMcpProvider: provider }), }); server = app.listen(0, '127.0.0.1', () => { port = (server.address() as AddressInfo).port; @@ -250,9 +249,7 @@ describe('client_mcp_over_ws reverse channel (serve layer)', () => { }); }); - ws.send( - JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' }), - ); + ws.send(JSON.stringify({ type: 'mcp_register', server: 'chrome-tools' })); const ack = await registered; // (a) daemon registered the runtime server, with the discovered catalog. diff --git a/packages/cli/src/ui/utils/historyMapping.test.ts b/packages/cli/src/ui/utils/historyMapping.test.ts index 21dd633a7d7..9cb738a5b85 100644 --- a/packages/cli/src/ui/utils/historyMapping.test.ts +++ b/packages/cli/src/ui/utils/historyMapping.test.ts @@ -9,10 +9,13 @@ import { computeApiTruncationIndex, isRealUserTurn } from './historyMapping.js'; import type { HistoryItem } from '../types.js'; import type { Content, Part } from '@google/genai'; import { + COMPRESSION_SUMMARY_MODEL_ACK, SYSTEM_REMINDER_OPEN, SYSTEM_REMINDER_CLOSE, } from '@qwen-code/qwen-code-core'; +const STARTUP_CONTEXT_MODEL_ACK = 'Got it. Thanks for the context!'; + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -25,6 +28,17 @@ function modelContent(text: string): Content { return { role: 'model', parts: [{ text } as Part] }; } +function functionCallContent(): Content { + return { + role: 'model', + parts: [ + { + functionCall: { name: 'tool', args: {} }, + } as unknown as Part, + ], + }; +} + function functionResponseContent(): Content { return { role: 'user', @@ -36,6 +50,13 @@ function functionResponseContent(): Content { }; } +function startupPair(): [Content, Content] { + return [ + userContent('Environment context...'), + modelContent(STARTUP_CONTEXT_MODEL_ACK), + ]; +} + function startupEntry(): Content { return userContent( `${SYSTEM_REMINDER_OPEN}\nEnvironment context...\n${SYSTEM_REMINDER_CLOSE}`, @@ -70,6 +91,16 @@ describe('computeApiTruncationIndex', () => { expect(computeApiTruncationIndex(ui, 1, api)).toBe(0); }); + it('returns -1 when the target user item is absent', () => { + const ui: HistoryItem[] = [userItem(1), geminiItem(2)]; + const api: Content[] = [ + userContent('prompt 1'), + modelContent('response 1'), + ]; + + expect(computeApiTruncationIndex(ui, 99, api)).toBe(-1); + }); + describe('without startup context', () => { it('rewinds to the first user turn (keep nothing)', () => { const ui: HistoryItem[] = [ @@ -248,6 +279,170 @@ describe('computeApiTruncationIndex', () => { }); describe('compression fallback', () => { + it('maps tail user turns after a compression summary pair', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 5, api)).toBe(2); + }); + + it('keeps compressed turns unreachable after a compression summary pair', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 3, api)).toBe(-1); + }); + + it('keeps the first UI turn unreachable when compression absorbed it', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 1, api)).toBe(-1); + }); + + it('keeps all UI turns unreachable when compression absorbed the full tail', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + ]; + const api: Content[] = [ + userContent('compressed summary of all prior turns'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + ]; + + expect(computeApiTruncationIndex(ui, 1, api)).toBe(-1); + expect(computeApiTruncationIndex(ui, 3, api)).toBe(-1); + }); + + it('does not treat post-compact attachment restoration as a tail turn', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent( + 'Recently accessed file (full current content embedded):\n\n' + + '## a.ts\n\n```ts\nexport const a = 1;\n```', + ), + functionCallContent(), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 3, api)).toBe(-1); + expect(computeApiTruncationIndex(ui, 5, api)).toBe(4); + }); + + it('maps compressed tail turns after startup context', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + ...startupPair(), + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 5, api)).toBe(4); + }); + + it('maps the first middle tail turn after compression', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + userItem(7), + geminiItem(8), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + userContent('prompt 5'), + modelContent('response 5'), + userContent('prompt 7'), + modelContent('response 7'), + ]; + + expect(computeApiTruncationIndex(ui, 5, api)).toBe(2); + expect(computeApiTruncationIndex(ui, 7, api)).toBe(4); + }); + + it('ignores tool-call entries between the bridge and next tail prompt', () => { + const ui: HistoryItem[] = [ + userItem(1), + geminiItem(2), + userItem(3), + geminiItem(4), + userItem(5), + geminiItem(6), + ]; + const api: Content[] = [ + userContent('compressed summary of prompt 1 and prompt 3'), + modelContent(COMPRESSION_SUMMARY_MODEL_ACK), + functionCallContent(), + functionResponseContent(), + modelContent('tool result response'), + userContent('prompt 5'), + modelContent('response 5'), + ]; + + expect(computeApiTruncationIndex(ui, 5, api)).toBe(5); + }); + it('returns -1 when not enough user prompts found', () => { const ui: HistoryItem[] = [ userItem(1), diff --git a/packages/cli/src/ui/utils/historyMapping.ts b/packages/cli/src/ui/utils/historyMapping.ts index c86c72ecd88..ec640d7aca2 100644 --- a/packages/cli/src/ui/utils/historyMapping.ts +++ b/packages/cli/src/ui/utils/historyMapping.ts @@ -7,19 +7,26 @@ import type { HistoryItem, HistoryItemUser } from '../types.js'; import type { Content } from '@google/genai'; import { + createDebugLogger, getStartupContextLength, - isSystemReminderContent, } from '@qwen-code/qwen-code-core'; import { isSlashCommand } from './commandUtils.js'; +import { + getApiUserTextIndices, + getCompressionTailStartIndex, + hasCompressionSummaryPair, +} from '../../utils/api-history-utils.js'; + +const debugLogger = createDebugLogger('HISTORY_MAPPING'); /** * Returns true when the history item represents a real user prompt that was * sent to the model, as opposed to a slash-command invocation (`/help`, - * `/stats`, …) which is stored with `type: 'user'` in the UI but never + * `/stats`, ...) which is stored with `type: 'user'` in the UI but never * reaches the API history or `turnParentUuids`. * * Typed as a type predicate so callers can drop their `as HistoryItemUser` - * casts — a regression that loosened either side of the narrowing would now + * casts - a regression that loosened either side of the narrowing would now * be caught by tsc instead of silently bypassing it. */ export function isRealUserTurn( @@ -34,28 +41,23 @@ export function isRealUserTurn( return !isSlashCommand(item.text) && !item.text.startsWith('?'); } -/** - * Checks if a Content entry is a user-initiated text prompt - * as opposed to a tool result (functionResponse). - */ -function isUserTextContent(content: Content): boolean { - if (content.role !== 'user') return false; - if (!content.parts || content.parts.length === 0) return false; +function getUiTurnOrdinals( + uiHistory: HistoryItem[], + targetUserItemId: number, +): { targetOrdinal: number; totalRealUserTurns: number } { + let targetOrdinal = -1; + let totalRealUserTurns = 0; - const hasFunctionResponse = content.parts.some( - (part) => 'functionResponse' in part, - ); - if (hasFunctionResponse) return false; + for (const item of uiHistory) { + if (!isRealUserTurn(item)) continue; - // Exclude pure entries (the startup prelude and the - // mid-history MCP added-tool reminders). They are structural, not real user - // prompts; counting them here would shift the rewind truncation index and - // silently drop a real turn's context. A genuine user turn that merely has - // a per-turn reminder prepended still has a non-reminder prompt part, so it - // is NOT excluded. - if (isSystemReminderContent(content)) return false; + totalRealUserTurns++; + if (item.id === targetUserItemId) { + targetOrdinal = totalRealUserTurns; + } + } - return content.parts.some((part) => 'text' in part && part.text); + return { targetOrdinal, totalRealUserTurns }; } /** @@ -63,13 +65,13 @@ function isUserTextContent(content: Content): boolean { * to a specific user turn in the UI history. * * The API history may include: - * - A startup context entry at the beginning + * - A startup context entry or startup context pair at the beginning * - User text prompts (corresponding to UI user turns) * - Model responses (with optional functionCall parts) * - Tool result entries: user(functionResponse) + model(response) * * This function counts user text Content entries (skipping tool results - * and the startup context entry) to find the API boundary corresponding + * and the startup context) to find the API boundary corresponding * to the target UI user turn. * * Note: In IDE mode, additional user Content entries may be injected for @@ -88,41 +90,47 @@ export function computeApiTruncationIndex( targetUserItemId: number, apiHistory: Content[], ): number { - // Count how many UI user turns exist before the target - let uiUserTurnCount = 0; - for (const item of uiHistory) { - if (item.id === targetUserItemId) { - break; - } - if (isRealUserTurn(item)) { - uiUserTurnCount++; - } - } + const { targetOrdinal, totalRealUserTurns } = getUiTurnOrdinals( + uiHistory, + targetUserItemId, + ); + + if (targetOrdinal < 0) return -1; - // Determine the starting index in the API history (skip startup context) const startIndex = getStartupContextLength(apiHistory); - if (uiUserTurnCount === 0) { + if (hasCompressionSummaryPair(apiHistory, startIndex)) { + // Compression replaces the oldest N UI turns with one synthetic + // summary/attachment prelude. The remaining API user-text entries are + // the uncompressed tail, so align that tail against the end of the UI + // turn list instead of counting from the front. + const apiTailUserIndices = getApiUserTextIndices( + apiHistory, + getCompressionTailStartIndex(apiHistory, startIndex), + ); + const compressedTurnCount = Math.max( + 0, + totalRealUserTurns - apiTailUserIndices.length, + ); + + if (targetOrdinal <= compressedTurnCount) { + debugLogger.info( + `Rewind target turn ${targetOrdinal} is unreachable: compressed ${compressedTurnCount} of ${totalRealUserTurns} total turns, tail has ${apiTailUserIndices.length} entries`, + ); + return -1; + } + + return apiTailUserIndices[targetOrdinal - compressedTurnCount - 1]!; + } + + if (targetOrdinal === 1) { // Rewinding to the first user turn: keep only startup context (if any) return startIndex; } - // Walk the API history from after the startup context, counting - // user text prompts to find the one corresponding to the target turn. - let realUserPromptCount = 0; - - for (let i = startIndex; i < apiHistory.length; i++) { - if (isUserTextContent(apiHistory[i]!)) { - realUserPromptCount++; - // The target turn is the (uiUserTurnCount + 1)th real user prompt. - // We want to truncate right before it. - if (realUserPromptCount > uiUserTurnCount) { - return i; - } - } - } + const apiUserTextIndices = getApiUserTextIndices(apiHistory, startIndex); + const targetApiIndex = apiUserTextIndices[targetOrdinal - 1]; + if (targetApiIndex !== undefined) return targetApiIndex; - // If we didn't find enough user prompts (e.g., after compression), - // signal that the target turn is unreachable. return -1; } diff --git a/packages/cli/src/utils/api-history-utils.test.ts b/packages/cli/src/utils/api-history-utils.test.ts new file mode 100644 index 00000000000..5da59ae2c99 --- /dev/null +++ b/packages/cli/src/utils/api-history-utils.test.ts @@ -0,0 +1,298 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import type { Content, Part } from '@google/genai'; +import { + COMPRESSION_SUMMARY_MODEL_ACK, + POST_COMPACT_ATTACHMENT_TEXT_PREFIXES, + SYSTEM_REMINDER_CLOSE, + SYSTEM_REMINDER_OPEN, +} from '@qwen-code/qwen-code-core'; +import { + hasTextPart, + hasModelTextPart, + isApiUserTextContent, + hasCompressionSummaryPair, + getCompressionTailStartIndex, + getApiUserTextIndices, + isPostCompactAttachmentContent, +} from './api-history-utils.js'; + +const STARTUP_CONTEXT_MODEL_ACK = 'Got it. Thanks for the context!'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function userTextContent(text: string): Content { + return { role: 'user', parts: [{ text } as Part] }; +} + +function modelTextContent(text: string): Content { + return { role: 'model', parts: [{ text } as Part] }; +} + +function functionResponseContent(): Content { + return { + role: 'user', + parts: [ + { + functionResponse: { name: 'tool', response: { result: 'ok' } }, + } as unknown as Part, + ], + }; +} + +function functionCallContent(): Content { + return { + role: 'model', + parts: [{ functionCall: { name: 'tool', args: {} } } as unknown as Part], + }; +} + +// --------------------------------------------------------------------------- +// hasTextPart +// --------------------------------------------------------------------------- + +describe('hasTextPart', () => { + it('returns true when content has a text part matching exactly', () => { + expect(hasTextPart(userTextContent('hello'), 'hello')).toBe(true); + }); + + it('returns false when text does not match', () => { + expect(hasTextPart(userTextContent('hello'), 'world')).toBe(false); + }); + + it('returns false for undefined content', () => { + expect(hasTextPart(undefined, 'hello')).toBe(false); + }); + + it('returns false when parts is undefined', () => { + expect(hasTextPart({ role: 'user' }, 'hello')).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// hasModelTextPart +// --------------------------------------------------------------------------- + +describe('hasModelTextPart', () => { + it('returns true when model content has matching text', () => { + expect(hasModelTextPart(modelTextContent('ack'), 'ack')).toBe(true); + }); + + it('returns false when role is not model', () => { + expect(hasModelTextPart(userTextContent('ack'), 'ack')).toBe(false); + }); + + it('returns false when text does not match', () => { + expect(hasModelTextPart(modelTextContent('ack'), 'other')).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// isApiUserTextContent +// --------------------------------------------------------------------------- + +describe('isApiUserTextContent', () => { + it('returns true for user text content', () => { + expect(isApiUserTextContent(userTextContent('hello'))).toBe(true); + }); + + it('returns false for model content', () => { + expect(isApiUserTextContent(modelTextContent('hello'))).toBe(false); + }); + + it('returns false for functionResponse content', () => { + expect(isApiUserTextContent(functionResponseContent())).toBe(false); + }); + + it('returns false for empty parts', () => { + expect(isApiUserTextContent({ role: 'user', parts: [] })).toBe(false); + }); + + it('returns false for undefined parts', () => { + expect(isApiUserTextContent({ role: 'user' })).toBe(false); + }); + + it('returns false for functionCall content (model role)', () => { + expect(isApiUserTextContent(functionCallContent())).toBe(false); + }); + + it('returns false for pure system reminder content', () => { + const content = userTextContent( + `${SYSTEM_REMINDER_OPEN}\nNew tools available: foo\n${SYSTEM_REMINDER_CLOSE}`, + ); + + expect(isApiUserTextContent(content)).toBe(false); + }); + + it('rejects user content with no text (only functionResponse)', () => { + const content: Content = { + role: 'user', + parts: [ + { functionResponse: { name: 't', response: {} } } as unknown as Part, + { text: 'some text' } as Part, + ], + }; + expect(isApiUserTextContent(content)).toBe(false); + }); + + it('returns false for question-prefixed side queries', () => { + expect(isApiUserTextContent(userTextContent('?help'))).toBe(false); + expect( + isApiUserTextContent({ + role: 'user', + parts: [{ text: '?' } as Part, { text: 'status' } as Part], + }), + ).toBe(false); + }); + + it('returns false for background task notifications', () => { + expect( + isApiUserTextContent( + userTextContent( + 'completed', + ), + ), + ).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// hasCompressionSummaryPair +// --------------------------------------------------------------------------- + +describe('hasCompressionSummaryPair', () => { + it('detects a compression summary pair', () => { + const history: Content[] = [ + userTextContent('summary text'), + modelTextContent(COMPRESSION_SUMMARY_MODEL_ACK), + ]; + expect(hasCompressionSummaryPair(history, 0)).toBe(true); + }); + + it('returns false when the ack text does not match', () => { + const history: Content[] = [ + userTextContent('summary text'), + modelTextContent('different ack'), + ]; + expect(hasCompressionSummaryPair(history, 0)).toBe(false); + }); + + it('returns false when startIndex is out of bounds', () => { + const history: Content[] = [userTextContent('only one')]; + expect(hasCompressionSummaryPair(history, 1)).toBe(false); + }); + + it('respects startIndex offset', () => { + const history: Content[] = [ + userTextContent('env context'), + modelTextContent(STARTUP_CONTEXT_MODEL_ACK), + userTextContent('summary'), + modelTextContent(COMPRESSION_SUMMARY_MODEL_ACK), + ]; + expect(hasCompressionSummaryPair(history, 0)).toBe(false); + expect(hasCompressionSummaryPair(history, 2)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// post-compact tail detection +// --------------------------------------------------------------------------- + +describe('post-compact tail detection', () => { + it.each(POST_COMPACT_ATTACHMENT_TEXT_PREFIXES)( + 'detects post-compact attachment content for %s', + (prefix) => { + expect( + isPostCompactAttachmentContent(userTextContent(`${prefix}\nbody`)), + ).toBe(true); + }, + ); + + it('does not classify real prompts as post-compact attachment content', () => { + expect(isPostCompactAttachmentContent(userTextContent('real prompt'))).toBe( + false, + ); + }); + + it('returns the summary/ack boundary when there are no attachments', () => { + const history: Content[] = [ + userTextContent('summary'), + modelTextContent(COMPRESSION_SUMMARY_MODEL_ACK), + userTextContent('tail turn'), + ]; + + expect(getCompressionTailStartIndex(history, 0)).toBe(2); + }); + + it('skips the post-compact attachment block and trailing functionCall', () => { + const history: Content[] = [ + userTextContent('summary'), + modelTextContent(COMPRESSION_SUMMARY_MODEL_ACK), + userTextContent( + 'Recently accessed file (full current content embedded):\n\n## a.ts', + ), + functionCallContent(), + userTextContent('tail turn'), + ]; + + expect(getCompressionTailStartIndex(history, 0)).toBe(4); + }); + + it('respects startup context offsets when skipping post-compact output', () => { + const history: Content[] = [ + userTextContent('Environment context...'), + modelTextContent(STARTUP_CONTEXT_MODEL_ACK), + userTextContent('summary'), + modelTextContent(COMPRESSION_SUMMARY_MODEL_ACK), + userTextContent('\nplan reminder\n'), + userTextContent('tail turn'), + ]; + + expect(getCompressionTailStartIndex(history, 2)).toBe(5); + }); +}); + +// --------------------------------------------------------------------------- +// getApiUserTextIndices +// --------------------------------------------------------------------------- + +describe('getApiUserTextIndices', () => { + it('returns indices of all user text entries from startIndex', () => { + const history: Content[] = [ + userTextContent('first'), + modelTextContent('ack'), + userTextContent('second'), + modelTextContent('resp'), + userTextContent('third'), + ]; + expect(getApiUserTextIndices(history, 0)).toEqual([0, 2, 4]); + }); + + it('respects startIndex', () => { + const history: Content[] = [ + userTextContent('first'), + modelTextContent('ack'), + userTextContent('second'), + modelTextContent('resp'), + ]; + expect(getApiUserTextIndices(history, 2)).toEqual([2]); + }); + + it('skips functionResponse entries', () => { + const history: Content[] = [ + userTextContent('first'), + modelTextContent('resp'), + functionResponseContent(), + modelTextContent('resp2'), + userTextContent('second'), + ]; + expect(getApiUserTextIndices(history, 0)).toEqual([0, 4]); + }); +}); diff --git a/packages/cli/src/utils/api-history-utils.ts b/packages/cli/src/utils/api-history-utils.ts new file mode 100644 index 00000000000..01786728bda --- /dev/null +++ b/packages/cli/src/utils/api-history-utils.ts @@ -0,0 +1,139 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Content, Part } from '@google/genai'; +import { + COMPRESSION_SUMMARY_MODEL_ACK, + POST_COMPACT_ATTACHMENT_TEXT_PREFIXES, + isSystemReminderContent, +} from '@qwen-code/qwen-code-core'; + +const BACKGROUND_NOTIFICATION_PREFIX = ' 'text' in part && part.text === text) ?? + false + ); +} + +export function hasModelTextPart( + content: Content | undefined, + text: string, +): boolean { + return content?.role === 'model' && hasTextPart(content, text); +} + +function hasModelFunctionCallPart(content: Content | undefined): boolean { + return ( + content?.role === 'model' && + (content.parts?.some( + (part) => 'functionCall' in part && part.functionCall, + ) ?? + false) + ); +} + +export function isPostCompactAttachmentContent( + content: Content | undefined, +): boolean { + if (!content || content.role !== 'user') return false; + + return ( + content.parts?.some((part) => { + const text = 'text' in part ? part.text : undefined; + return ( + typeof text === 'string' && + POST_COMPACT_ATTACHMENT_TEXT_PREFIXES.some((prefix) => + text.startsWith(prefix), + ) + ); + }) ?? false + ); +} + +/** + * Checks if a Content entry is a user-initiated text prompt + * as opposed to a tool result (functionResponse). + */ +export function isApiUserTextContent(content: Content): boolean { + if (content.role !== 'user') return false; + if (!content.parts || content.parts.length === 0) return false; + + const hasFunctionResponse = content.parts.some( + (part) => 'functionResponse' in part, + ); + if (hasFunctionResponse) return false; + if (isSystemReminderContent(content)) return false; + + const textParts = content.parts + .filter( + (part): part is { text: string } & Part => + 'text' in part && + typeof part.text === 'string' && + part.text.trim().length > 0, + ) + .map((part) => part.text.trim()); + if (textParts.length === 0) return false; + + const fullText = textParts.join(' '); + return ( + !fullText.startsWith('?') && + !fullText.startsWith(BACKGROUND_NOTIFICATION_PREFIX) + ); +} + +export function hasCompressionSummaryPair( + apiHistory: Content[], + startIndex: number, +): boolean { + const summary = apiHistory[startIndex]; + return ( + !!summary && + isApiUserTextContent(summary) && + hasModelTextPart(apiHistory[startIndex + 1], COMPRESSION_SUMMARY_MODEL_ACK) + ); +} + +/** + * Returns the first API history index after the synthetic post-compression + * prelude. Compression always emits summary + ack, and may also emit one + * synthetic user attachment block plus a trailing model functionCall block. + */ +export function getCompressionTailStartIndex( + apiHistory: Content[], + startIndex: number, +): number { + if (!hasCompressionSummaryPair(apiHistory, startIndex)) return startIndex; + + let tailStartIndex = startIndex + 2; + if (isPostCompactAttachmentContent(apiHistory[tailStartIndex])) { + tailStartIndex += 1; + if (hasModelFunctionCallPart(apiHistory[tailStartIndex])) { + tailStartIndex += 1; + } + } + + return tailStartIndex; +} + +export function getApiUserTextIndices( + apiHistory: Content[], + startIndex: number, +): number[] { + const indices: number[] = []; + + for (let i = startIndex; i < apiHistory.length; i++) { + const content = apiHistory[i]!; + if (!isApiUserTextContent(content)) continue; + indices.push(i); + } + + return indices; +} diff --git a/packages/core/src/core/openaiContentGenerator/converter.test.ts b/packages/core/src/core/openaiContentGenerator/converter.test.ts index 91418e07b1f..4c778313b08 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.test.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.test.ts @@ -2721,6 +2721,35 @@ describe('OpenAIContentConverter', () => { (usage?.promptTokenCount ?? 0) + (usage?.candidatesTokenCount ?? 0), ).toBe(5); }); + + it('maps DeepSeek prompt cache hit tokens into cached content token count', () => { + const response = converter.convertOpenAIResponseToGemini( + { + object: 'chat.completion', + id: 'chatcmpl-deepseek-cache', + created: 123, + model: 'deepseek-v4-pro', + choices: [], + usage: { + prompt_tokens: 21225, + completion_tokens: 45, + total_tokens: 21270, + prompt_cache_hit_tokens: 21120, + prompt_cache_miss_tokens: 105, + }, + } as unknown as OpenAI.Chat.ChatCompletion, + requestContext, + ); + + expect(response.usageMetadata).toEqual( + expect.objectContaining({ + promptTokenCount: 21225, + candidatesTokenCount: 45, + totalTokenCount: 21270, + cachedContentTokenCount: 21120, + }), + ); + }); }); describe('OpenAI -> Gemini reasoning content', () => { diff --git a/packages/core/src/core/openaiContentGenerator/converter.ts b/packages/core/src/core/openaiContentGenerator/converter.ts index 33bd88be0ce..b00c02f3c95 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.ts @@ -37,6 +37,8 @@ const SPLIT_TOOL_MEDIA_TEXT = '(attached media from previous tool call)'; */ interface ExtendedCompletionUsage extends OpenAI.CompletionUsage { cached_tokens?: number; + prompt_cache_hit_tokens?: number; + prompt_cache_miss_tokens?: number; } export interface ExtendedChatCompletionAssistantMessageParam @@ -70,6 +72,16 @@ export interface ExtendedCompletionChunkDelta // so it preserves catch-rate without silently suppressing legitimate chunks. const CUMULATIVE_DELTA_EXACT_REPEAT_MIN_LENGTH = 64; +function getCachedPromptTokens(usage: OpenAI.CompletionUsage): number { + const extendedUsage = usage as ExtendedCompletionUsage; + return ( + usage.prompt_tokens_details?.cached_tokens ?? + extendedUsage.cached_tokens ?? + extendedUsage.prompt_cache_hit_tokens ?? + 0 + ); +} + // Once this many bytes have been emitted without entering cumulative mode the // stream is almost certainly a standard incremental provider. Stop growing // emittedText beyond this point to bound per-stream memory and CPU. The true @@ -1167,13 +1179,8 @@ export function convertOpenAIResponseToGemini( const promptTokens = usage.prompt_tokens || 0; const completionTokens = usage.completion_tokens || 0; const totalTokens = usage.total_tokens || 0; - // Support both formats: prompt_tokens_details.cached_tokens (OpenAI standard) - // and cached_tokens (some models return it at top level) - const extendedUsage = usage as ExtendedCompletionUsage; - const cachedTokens = - usage.prompt_tokens_details?.cached_tokens ?? - extendedUsage.cached_tokens ?? - 0; + // Support OpenAI and provider-specific cache usage fields. + const cachedTokens = getCachedPromptTokens(usage); const thinkingTokens = usage.completion_tokens_details?.reasoning_tokens || 0; @@ -1431,13 +1438,8 @@ export function convertOpenAIChunkToGemini( const totalTokens = usage.total_tokens || 0; const thinkingTokens = usage.completion_tokens_details?.reasoning_tokens || 0; - // Support both formats: prompt_tokens_details.cached_tokens (OpenAI standard) - // and cached_tokens (some models return it at top level) - const extendedUsage = usage as ExtendedCompletionUsage; - const cachedTokens = - usage.prompt_tokens_details?.cached_tokens ?? - extendedUsage.cached_tokens ?? - 0; + // Support OpenAI and provider-specific cache usage fields. + const cachedTokens = getCachedPromptTokens(usage); // If we only have total tokens but no breakdown, estimate the split // Typically input is ~70% and output is ~30% for most conversations diff --git a/packages/core/src/core/openaiContentGenerator/pipeline.test.ts b/packages/core/src/core/openaiContentGenerator/pipeline.test.ts index d2c2f3a1c5e..e4026b8d9fe 100644 --- a/packages/core/src/core/openaiContentGenerator/pipeline.test.ts +++ b/packages/core/src/core/openaiContentGenerator/pipeline.test.ts @@ -26,6 +26,7 @@ import { MAX_STREAM_IDLE_TIMEOUT_MS, QWEN_STREAM_IDLE_TIMEOUT_MS_ENV, } from './constants.js'; +import { runtimeDiagnostics } from '../../utils/runtimeDiagnostics.js'; // Mock dependencies vi.mock('./converter.js', () => ({ @@ -176,6 +177,93 @@ describe('ContentGenerationPipeline', () => { ); }); + it('labels runtime OpenAI diagnostics as deepseek for DeepSeek hostnames', async () => { + const diagnosticsSpy = vi + .spyOn(runtimeDiagnostics, 'recordOpenAIWireRequest') + .mockImplementation(() => undefined); + try { + mockContentGeneratorConfig.baseUrl = 'https://api.deepseek.com'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + }; + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockOpenAIResponse = { + id: 'response-id', + choices: [ + { message: { content: 'Hello response' }, finish_reason: 'stop' }, + ], + created: Date.now(), + model: 'deepseek-v4-pro', + } as OpenAI.Chat.ChatCompletion; + const mockGeminiResponse = new GenerateContentResponse(); + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + mockGeminiResponse, + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue( + mockOpenAIResponse, + ); + + await pipeline.execute(request, 'test-prompt-id'); + + expect(diagnosticsSpy).toHaveBeenCalledWith( + expect.objectContaining({ model: 'deepseek-v4-pro' }), + { provider: 'deepseek' }, + ); + } finally { + diagnosticsSpy.mockRestore(); + } + }); + + it('labels runtime OpenAI diagnostics as openai-compatible for non-DeepSeek hostnames', async () => { + const diagnosticsSpy = vi + .spyOn(runtimeDiagnostics, 'recordOpenAIWireRequest') + .mockImplementation(() => undefined); + try { + mockContentGeneratorConfig.baseUrl = 'https://example.test/v1'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + }; + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockOpenAIResponse = { + id: 'response-id', + choices: [ + { message: { content: 'Hello response' }, finish_reason: 'stop' }, + ], + created: Date.now(), + model: 'deepseek-v4-pro', + } as OpenAI.Chat.ChatCompletion; + const mockGeminiResponse = new GenerateContentResponse(); + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + mockGeminiResponse, + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue( + mockOpenAIResponse, + ); + + await pipeline.execute(request, 'test-prompt-id'); + + expect(diagnosticsSpy).toHaveBeenCalledWith(expect.any(Object), { + provider: 'openai-compatible', + }); + } finally { + diagnosticsSpy.mockRestore(); + } + }); + it('should use request.model when provided', async () => { // Arrange const request: GenerateContentParameters = { @@ -600,6 +688,113 @@ describe('ContentGenerationPipeline', () => { ); }); + it('sorts DeepSeek tools by function name before sending the wire request', async () => { + mockContentGeneratorConfig.baseUrl = 'https://api.deepseek.com/v1'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + config: { + tools: [ + { + functionDeclarations: [ + { + name: 'placeholder', + description: 'Placeholder function', + parameters: { type: Type.OBJECT, properties: {} }, + }, + ], + }, + ], + }, + }; + + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockTools = [ + { type: 'function', function: { name: 'zeta' } }, + { type: 'function', function: { name: 'alpha' } }, + { type: 'function', function: { name: 'bravo' } }, + ] as OpenAI.Chat.ChatCompletionTool[]; + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertGeminiToolsToOpenAI as Mock).mockResolvedValue( + mockTools, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + new GenerateContentResponse(), + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue({ + id: 'response-id', + choices: [], + } as unknown as OpenAI.Chat.ChatCompletion); + + await pipeline.execute(request, 'test-prompt-id'); + + const apiCall = (mockClient.chat.completions.create as Mock).mock + .calls[0][0]; + expect( + apiCall.tools.map( + (tool: OpenAI.Chat.ChatCompletionTool) => tool.function.name, + ), + ).toEqual(['alpha', 'bravo', 'zeta']); + }); + + it('preserves tool order for non-DeepSeek hostnames', async () => { + mockContentGeneratorConfig.baseUrl = 'https://example.test/v1'; + const request: GenerateContentParameters = { + model: 'deepseek-v4-pro', + contents: [{ parts: [{ text: 'Hello' }], role: 'user' }], + config: { + tools: [ + { + functionDeclarations: [ + { + name: 'placeholder', + description: 'Placeholder function', + parameters: { type: Type.OBJECT, properties: {} }, + }, + ], + }, + ], + }, + }; + + const mockMessages = [ + { role: 'user', content: 'Hello' }, + ] as OpenAI.Chat.ChatCompletionMessageParam[]; + const mockTools = [ + { type: 'function', function: { name: 'zeta' } }, + { type: 'function', function: { name: 'alpha' } }, + ] as OpenAI.Chat.ChatCompletionTool[]; + + (mockConverter.convertGeminiRequestToOpenAI as Mock).mockReturnValue( + mockMessages, + ); + (mockConverter.convertGeminiToolsToOpenAI as Mock).mockResolvedValue( + mockTools, + ); + (mockConverter.convertOpenAIResponseToGemini as Mock).mockReturnValue( + new GenerateContentResponse(), + ); + (mockClient.chat.completions.create as Mock).mockResolvedValue({ + id: 'response-id', + choices: [], + } as unknown as OpenAI.Chat.ChatCompletion); + + await pipeline.execute(request, 'test-prompt-id'); + + const apiCall = (mockClient.chat.completions.create as Mock).mock + .calls[0][0]; + expect( + apiCall.tools.map( + (tool: OpenAI.Chat.ChatCompletionTool) => tool.function.name, + ), + ).toEqual(['zeta', 'alpha']); + }); + it('should skip empty tools array in request', async () => { // Arrange — tools: [] should NOT be included in the API request const request: GenerateContentParameters = { diff --git a/packages/core/src/core/openaiContentGenerator/pipeline.ts b/packages/core/src/core/openaiContentGenerator/pipeline.ts index 18b9ec6cd0e..5352018882b 100644 --- a/packages/core/src/core/openaiContentGenerator/pipeline.ts +++ b/packages/core/src/core/openaiContentGenerator/pipeline.ts @@ -29,6 +29,26 @@ import { createDebugLogger } from '../../utils/debugLogger.js'; const debugLogger = createDebugLogger('OPENAI_PIPELINE'); +function compareStableStrings(left: string, right: string): number { + if (left < right) return -1; + if (left > right) return 1; + return 0; +} + +function getToolSortKey(tool: OpenAI.Chat.ChatCompletionTool): string { + return [tool.function.name, tool.type, JSON.stringify(tool)].join('\u0000'); +} + +function sortToolsForCacheStableRequest( + request: OpenAI.Chat.ChatCompletionCreateParams, +): void { + if (!request.tools || request.tools.length < 2) return; + + request.tools = [...request.tools].sort((left, right) => + compareStableStrings(getToolSortKey(left), getToolSortKey(right)), + ); +} + /** * Error thrown when the API returns an error embedded as stream content * instead of a proper HTTP error. Some providers (e.g., certain OpenAI-compatible @@ -612,6 +632,14 @@ export class ContentGenerationPipeline { } } + // DeepSeek's KV cache is prefix-exact: a different tool order changes the + // serialized prompt prefix even when the tool set and schemas are identical. + // Sort only for official DeepSeek endpoints to avoid surprising other + // OpenAI-compatible providers with changed tool presentation order. + if (isDeepSeekHostname(this.contentGeneratorConfig)) { + sortToolsForCacheStableRequest(providerRequest); + } + return providerRequest; } @@ -742,7 +770,11 @@ export class ContentGenerationPipeline { // provider enhancement, post disable-reasoning) and before the SDK call // so the logger sees the exact bytes sent on the wire. openaiRequestCaptureContext.getStore()?.(openaiRequest); - runtimeDiagnostics.recordOpenAIWireRequest(openaiRequest); + runtimeDiagnostics.recordOpenAIWireRequest(openaiRequest, { + provider: isDeepSeekHostname(this.contentGeneratorConfig) + ? 'deepseek' + : 'openai-compatible', + }); const result = await executor(openaiRequest, context); return result; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 57db2d099a0..e43362193df 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -231,6 +231,7 @@ export type { TokenUsageTotals, } from './services/tokenUsageService.js'; export * from './services/worktreeSessionService.js'; +export * from './services/chat-compression-constants.js'; export { stripTerminalControlSequences, TERMINAL_OSC_REGEX, diff --git a/packages/core/src/services/chat-compression-constants.ts b/packages/core/src/services/chat-compression-constants.ts new file mode 100644 index 00000000000..691dd8c98d4 --- /dev/null +++ b/packages/core/src/services/chat-compression-constants.ts @@ -0,0 +1,25 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +export const COMPRESSION_SUMMARY_MODEL_ACK = + 'Got it. Thanks for the additional context!'; + +export const POST_COMPACT_FILE_REFERENCES_PREFIX = + 'The following files were recently accessed before context was compacted.'; +export const POST_COMPACT_FILE_EMBED_PREFIX = + 'Recently accessed file (full current content embedded):'; +export const POST_COMPACT_IMAGE_RESTORATION_PREFIX = + 'Recent visual snapshots preserved from before context was compacted'; +export const POST_COMPACT_PLAN_MODE_PREFIX = ''; +export const POST_COMPACT_BACKGROUND_TASKS_PREFIX = ''; + +export const POST_COMPACT_ATTACHMENT_TEXT_PREFIXES = [ + POST_COMPACT_FILE_REFERENCES_PREFIX, + POST_COMPACT_FILE_EMBED_PREFIX, + POST_COMPACT_IMAGE_RESTORATION_PREFIX, + POST_COMPACT_PLAN_MODE_PREFIX, + POST_COMPACT_BACKGROUND_TASKS_PREFIX, +] as const; diff --git a/packages/core/src/services/chatCompressionService.ts b/packages/core/src/services/chatCompressionService.ts index 73c5129596a..dc92d05e147 100644 --- a/packages/core/src/services/chatCompressionService.ts +++ b/packages/core/src/services/chatCompressionService.ts @@ -38,6 +38,7 @@ import { stripAnalysisBlock, type SubagentSnapshot, } from './postCompactAttachments.js'; +import { COMPRESSION_SUMMARY_MODEL_ACK } from './chat-compression-constants.js'; /** * Hard cap on the compression sideQuery output (summary text only, since @@ -686,10 +687,7 @@ export class ChatCompressionService { }, { role: 'model', - parts: [ - { text: 'Got it. Thanks for the additional context!' }, - ...fcParts, - ], + parts: [{ text: COMPRESSION_SUMMARY_MODEL_ACK }, ...fcParts], }, ]; } diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 39c1c8a7910..bb25cc7840c 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -443,6 +443,11 @@ export interface AttributionSnapshotPayload { export interface RewindRecordPayload { /** Number of UI history items truncated. */ truncatedCount: number; + /** + * Highest model-facing user turn count observed before this rewind. Used by + * ACP resume to preserve snapshot validation's historical upper bound. + */ + maxModelFacingUserTurnCount?: number; } /** diff --git a/packages/core/src/services/postCompactAttachments.test.ts b/packages/core/src/services/postCompactAttachments.test.ts index f8758c7749c..3c8b4eacd07 100644 --- a/packages/core/src/services/postCompactAttachments.test.ts +++ b/packages/core/src/services/postCompactAttachments.test.ts @@ -963,26 +963,37 @@ describe('composePostCompactHistory', () => { const secret = join(outsideDir, 'secret.txt'); writeFileSync(secret, 'TOP_SECRET_CONTENT'); const link = join(tmpDir, 'innocent.ts'); - symlinkSync(secret, link); // workspace/innocent.ts -> outsideDir/secret.txt - - const history: Content[] = [ - { - role: 'model', - parts: [ - { functionCall: { name: 'read_file', args: { file_path: link } } }, - ], - }, - ]; - const result = await composePostCompactHistory(history, 'SUM', { - workspaceRoot: tmpDir, - }); - const allText = result - .flatMap((c) => c.parts ?? []) - .map((p) => (p as { text?: string }).text ?? '') - .join('\n'); - // Lexical resolve would embed the secret; realpath rejects the link. - expect(allText).not.toContain('TOP_SECRET_CONTENT'); - rmSync(outsideDir, { recursive: true, force: true }); + try { + try { + symlinkSync(secret, link); // workspace/innocent.ts -> outsideDir/secret.txt + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES' || code === 'ENOTSUP') { + return; + } + throw error; + } + + const history: Content[] = [ + { + role: 'model', + parts: [ + { functionCall: { name: 'read_file', args: { file_path: link } } }, + ], + }, + ]; + const result = await composePostCompactHistory(history, 'SUM', { + workspaceRoot: tmpDir, + }); + const allText = result + .flatMap((c) => c.parts ?? []) + .map((p) => (p as { text?: string }).text ?? '') + .join('\n'); + // Lexical resolve would embed the secret; realpath rejects the link. + expect(allText).not.toContain('TOP_SECRET_CONTENT'); + } finally { + rmSync(outsideDir, { recursive: true, force: true }); + } }); it('honors AbortSignal — does not invoke file reads after abort (Finding 5)', async () => { diff --git a/packages/core/src/services/postCompactAttachments.ts b/packages/core/src/services/postCompactAttachments.ts index e5d70209074..56907601fca 100644 --- a/packages/core/src/services/postCompactAttachments.ts +++ b/packages/core/src/services/postCompactAttachments.ts @@ -23,6 +23,14 @@ import { CHARS_PER_TOKEN } from './tokenEstimation.js'; import { getFunctionResponseParts } from './compactionInputSlimming.js'; import { escapeXml } from '../utils/xml.js'; import { ToolNames } from '../tools/tool-names.js'; +import { + COMPRESSION_SUMMARY_MODEL_ACK, + POST_COMPACT_BACKGROUND_TASKS_PREFIX, + POST_COMPACT_FILE_EMBED_PREFIX, + POST_COMPACT_FILE_REFERENCES_PREFIX, + POST_COMPACT_IMAGE_RESTORATION_PREFIX, + POST_COMPACT_PLAN_MODE_PREFIX, +} from './chat-compression-constants.js'; export const POST_COMPACT_MAX_FILES_TO_RESTORE = 5; @@ -389,7 +397,7 @@ export async function buildFileRestorationBlocks( if (references.length > 0) { const lines = [ - 'The following files were recently accessed before context was compacted. They are listed as reference only because they are large. Use `read_file` to view current content for any file you need:', + `${POST_COMPACT_FILE_REFERENCES_PREFIX} They are listed as reference only because they are large. Use \`read_file\` to view current content for any file you need:`, '', ...references.map((p) => `- ${sanitizePathForDisplay(p)}`), ]; @@ -411,7 +419,7 @@ export async function buildFileRestorationBlocks( parts: [ { text: - `Recently accessed file (full current content embedded):\n\n` + + `${POST_COMPACT_FILE_EMBED_PREFIX}\n\n` + `## ${sanitizePathForDisplay(path)}\n\n` + safeFence + '\n' + @@ -439,7 +447,7 @@ export function buildImageRestorationBlock( if (images.length === 0) return null; const lines = [ - 'Recent visual snapshots preserved from before context was compacted (most recent last). Each image corresponds to a tool result or user-pasted image earlier in the conversation:', + `${POST_COMPACT_IMAGE_RESTORATION_PREFIX} (most recent last). Each image corresponds to a tool result or user-pasted image earlier in the conversation:`, '', ]; for (const img of images) { @@ -667,7 +675,7 @@ function safeRealpath(p: string): string { // leaving stale guidance. Keep the list small (the most common modification // tools) — an exhaustive enumeration would drift faster than it helps. const PLAN_MODE_REMINDER_TEXT = - '\n' + + `${POST_COMPACT_PLAN_MODE_PREFIX}\n` + 'You are currently in PLAN mode. You may research, read files, and ' + 'propose plans, but you may not execute modification tools (' + `${ToolNames.WRITE_FILE}, ${ToolNames.EDIT}, ${ToolNames.SHELL}, etc.) ` + @@ -736,7 +744,7 @@ function buildSubagentSnapshotPart(snaps: SubagentSnapshot[]): Part | null { } return { text: - '\n' + + `${POST_COMPACT_BACKGROUND_TASKS_PREFIX}\n` + 'The following background subagent tasks were active at compaction. ' + 'The summary above does not include their per-task state. Use ' + '`task_stop` / `send_message` to interact; do not assume they ' + @@ -825,9 +833,7 @@ export async function composePostCompactHistory( // model→model adjacency that would otherwise // arise from a separate appended entry. const trailingFc = trailingFunctionCallContent(history); - const ackParts: Part[] = [ - { text: 'Got it. Thanks for the additional context!' }, - ]; + const ackParts: Part[] = [{ text: COMPRESSION_SUMMARY_MODEL_ACK }]; const out: Content[] = [ { role: 'user', parts: [{ text: postProcessSummary(summary) }] }, diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index 6599c7605be..e9e21d98624 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -1376,6 +1376,149 @@ describe('SessionService', () => { expect(history).toEqual([recordA1.message, assistantA1.message]); }); + it('excludes slash and question-prefixed user records from resume API history', () => { + const slashCommand: ChatRecord = { + ...recordA1, + uuid: 'slash-command', + message: { role: 'user', parts: [{ text: '/help' }] }, + }; + const questionCommand: ChatRecord = { + ...recordA1, + uuid: 'question-command', + message: { role: 'user', parts: [{ text: '?status' }] }, + }; + const secondUser: ChatRecord = { + ...recordA1, + uuid: 'user-2', + message: { role: 'user', parts: [{ text: 'second prompt' }] }, + }; + const assistantReply: ChatRecord = { + ...recordB2, + sessionId: sessionIdA, + parentUuid: secondUser.uuid, + message: { role: 'model', parts: [{ text: 'reply' }] }, + }; + + const conversation: ConversationRecord = { + sessionId: sessionIdA, + projectHash: 'test-project-hash', + startTime: '2024-01-01T00:00:00Z', + lastUpdated: '2024-01-01T00:00:00Z', + messages: [ + recordA1, + slashCommand, + questionCommand, + secondUser, + assistantReply, + ], + }; + + const history = buildApiHistoryFromConversation(conversation); + + expect(history).toEqual([ + recordA1.message, + secondUser.message, + assistantReply.message, + ]); + }); + + it('keeps slash-like user records that are not resume slash commands', () => { + const doubleSlashText: ChatRecord = { + ...recordA1, + uuid: 'double-slash-text', + message: { role: 'user', parts: [{ text: '// not a command' }] }, + }; + const blockCommentText: ChatRecord = { + ...recordA1, + uuid: 'block-comment-text', + message: { role: 'user', parts: [{ text: '/* not a command */' }] }, + }; + const pathText: ChatRecord = { + ...recordA1, + uuid: 'path-text', + message: { role: 'user', parts: [{ text: '/tmp/example.txt' }] }, + }; + const conversation: ConversationRecord = { + sessionId: sessionIdA, + projectHash: 'test-project-hash', + startTime: '2024-01-01T00:00:00Z', + lastUpdated: '2024-01-01T00:00:00Z', + messages: [doubleSlashText, blockCommentText, pathText], + }; + + const history = buildApiHistoryFromConversation(conversation); + + expect(history).toEqual([ + doubleSlashText.message, + blockCommentText.message, + pathText.message, + ]); + }); + + it('keeps cron user records in resume API history', () => { + const cronPrompt: ChatRecord = { + ...recordA1, + uuid: 'cron-prompt', + subtype: 'cron', + message: { role: 'user', parts: [{ text: '/scheduled prompt' }] }, + }; + const assistantReply: ChatRecord = { + ...recordB2, + sessionId: sessionIdA, + parentUuid: cronPrompt.uuid, + message: { role: 'model', parts: [{ text: 'cron reply' }] }, + }; + const conversation: ConversationRecord = { + sessionId: sessionIdA, + projectHash: 'test-project-hash', + startTime: '2024-01-01T00:00:00Z', + lastUpdated: '2024-01-01T00:00:00Z', + messages: [cronPrompt, assistantReply], + }; + + const history = buildApiHistoryFromConversation(conversation); + + expect(history).toEqual([cronPrompt.message, assistantReply.message]); + }); + + it('keeps notification records in resume API history', () => { + const notification: ChatRecord = { + ...recordA1, + uuid: 'notification', + subtype: 'notification', + message: { + role: 'user', + parts: [ + { + text: 'completed', + }, + ], + }, + }; + const assistantReply: ChatRecord = { + ...recordB2, + sessionId: sessionIdA, + parentUuid: notification.uuid, + message: { role: 'model', parts: [{ text: 'noted' }] }, + }; + + const conversation: ConversationRecord = { + sessionId: sessionIdA, + projectHash: 'test-project-hash', + startTime: '2024-01-01T00:00:00Z', + lastUpdated: '2024-01-01T00:00:00Z', + messages: [recordA1, notification, assistantReply], + }; + + const history = buildApiHistoryFromConversation(conversation); + + expect(history).toEqual([ + recordA1.message, + notification.message, + assistantReply.message, + ]); + }); + it('does not deep-clone stored messages when rebuilding resume API history', () => { const largePayload = { output: 'x'.repeat(128 * 1024), diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index ab4767ccbb3..53f39e312fe 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -1422,9 +1422,67 @@ function copyContentForApiHistory(content: Content): Content { }; } +function getUserRecordText(record: ChatRecord): string { + const textParts = + record.message?.parts + ?.filter( + (part): part is { text: string } & Part => + 'text' in part && + typeof part.text === 'string' && + part.text.trim().length > 0, + ) + .map((part) => part.text.trim()) ?? []; + return textParts.join(' '); +} + +function isResumeSlashCommand(query: string): boolean { + if (!query.startsWith('/')) { + return false; + } + if (query.startsWith('//') || query.startsWith('/*')) { + return false; + } + const firstToken = query.slice(1).trimStart().split(/\s+/u)[0] ?? ''; + if (/[/\\]/.test(firstToken)) { + return false; + } + return true; +} + +function shouldIncludeUserRecordInApiHistory(record: ChatRecord): boolean { + if (record.type !== 'user') { + return false; + } + if (record.subtype === 'notification') { + return true; + } + if (record.subtype === 'mid_turn_user_message') { + return true; + } + if ( + record.message?.parts?.some( + (part) => 'functionResponse' in part && part.functionResponse, + ) + ) { + return true; + } + const fullText = getUserRecordText(record); + if (fullText.length === 0) { + return false; + } + if (record.subtype === 'cron') { + return true; + } + return !fullText.startsWith('?') && !isResumeSlashCommand(fullText); +} + function appendApiHistoryRecord(history: Content[], record: ChatRecord): void { if (!record.message) return; + if (record.type === 'user' && !shouldIncludeUserRecordInApiHistory(record)) { + return; + } + const message = copyContentForApiHistory(record.message as Content); if (record.subtype === 'mid_turn_user_message') { const previous = history.at(-1); diff --git a/packages/core/src/tools/client-mcp-registrar.test.ts b/packages/core/src/tools/client-mcp-registrar.test.ts index ff623928efd..96a22b6cde5 100644 --- a/packages/core/src/tools/client-mcp-registrar.test.ts +++ b/packages/core/src/tools/client-mcp-registrar.test.ts @@ -53,9 +53,10 @@ class InMemoryServerTransport { } /** Build a canned client-hosted MCP server exposing one echo tool. */ -function buildCannedServer( - sink: (message: JSONRPCMessage) => void, -): { transport: InMemoryServerTransport; ready: Promise } { +function buildCannedServer(sink: (message: JSONRPCMessage) => void): { + transport: InMemoryServerTransport; + ready: Promise; +} { const server = new McpServer({ name: 'chrome-tools', version: '0.0.1', diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index ef05d2aa868..6ce63c81db2 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1725,7 +1725,8 @@ export async function createTransport( ) { const provider = new ServiceAccountImpersonationProvider(mcpServerConfig); const transportOptions: - StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { + | StreamableHTTPClientTransportOptions + | SSEClientTransportOptions = { authProvider: provider, }; @@ -1753,7 +1754,8 @@ export async function createTransport( ) { const provider = new GoogleCredentialProvider(mcpServerConfig); const transportOptions: - StreamableHTTPClientTransportOptions | SSEClientTransportOptions = { + | StreamableHTTPClientTransportOptions + | SSEClientTransportOptions = { authProvider: provider, }; if (mcpServerConfig.httpUrl) { diff --git a/packages/core/src/utils/runtimeDiagnostics.test.ts b/packages/core/src/utils/runtimeDiagnostics.test.ts index cff3de3c33c..fd27de790a2 100644 --- a/packages/core/src/utils/runtimeDiagnostics.test.ts +++ b/packages/core/src/utils/runtimeDiagnostics.test.ts @@ -234,4 +234,284 @@ describe('RuntimeDiagnosticsCollector', () => { }); expect(JSON.stringify(snapshot)).not.toContain('/private/path.txt'); }); + + it('summarizes OpenAI tool cache stability without retaining schema bodies', () => { + const summary = summarizeOpenAIWireRequest( + { + model: 'wire-model', + stream: false, + messages: [{ role: 'user', content: 'secret user prompt' }], + tools: [ + { + type: 'function', + function: { + name: 'read_file', + description: 'secret tool description', + parameters: { + type: 'object', + properties: { + secretPathProperty: { type: 'string' }, + }, + }, + }, + }, + { + type: 'function', + function: { + name: 'run_shell_command', + description: 'another secret description', + parameters: { + type: 'object', + properties: { + secretCommandProperty: { type: 'string' }, + }, + }, + }, + }, + ], + }, + { provider: 'deepseek' }, + ); + + expect(summary.cacheStability).toMatchObject({ + provider: 'deepseek', + toolNames: ['read_file', 'run_shell_command'], + toolNameSequenceHash: expect.stringMatching(/^[a-f0-9]{64}$/), + toolNameSetHash: expect.stringMatching(/^[a-f0-9]{64}$/), + toolSchemaHash: expect.stringMatching(/^[a-f0-9]{64}$/), + canonicalToolManifestHash: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + expect(JSON.stringify(summary)).not.toContain('secret user prompt'); + expect(JSON.stringify(summary)).not.toContain('secret tool description'); + expect(JSON.stringify(summary)).not.toContain('secretPathProperty'); + expect(JSON.stringify(summary)).not.toContain('secretCommandProperty'); + }); + + it('distinguishes OpenAI tool order drift from tool set drift', () => { + const alphaFirst = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + ], + }); + const bravoFirst = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + ], + }); + + expect(alphaFirst.cacheStability?.toolNames).toEqual(['alpha', 'bravo']); + expect(bravoFirst.cacheStability?.toolNames).toEqual(['bravo', 'alpha']); + expect(alphaFirst.cacheStability?.toolNameSetHash).toBe( + bravoFirst.cacheStability?.toolNameSetHash, + ); + expect(alphaFirst.cacheStability?.canonicalToolManifestHash).toBe( + bravoFirst.cacheStability?.canonicalToolManifestHash, + ); + expect(alphaFirst.cacheStability?.toolNameSequenceHash).not.toBe( + bravoFirst.cacheStability?.toolNameSequenceHash, + ); + }); + + it('deduplicates OpenAI tool names for set drift diagnostics', () => { + const uniqueNames = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + ], + }); + const duplicateName = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + { + type: 'function', + function: { name: 'bravo', description: 'B', parameters: {} }, + }, + { + type: 'function', + function: { name: 'alpha', description: 'A', parameters: {} }, + }, + ], + }); + + expect(uniqueNames.cacheStability?.toolNameSetHash).toBe( + duplicateName.cacheStability?.toolNameSetHash, + ); + }); + + it('uses canonical manifest hashes for equivalent schema key order', () => { + const first = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { + path: { type: 'string', description: 'Path' }, + limit: { type: 'number', description: 'Limit' }, + }, + }, + }, + }, + ], + }); + const second = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + parameters: { + properties: { + limit: { description: 'Limit', type: 'number' }, + path: { description: 'Path', type: 'string' }, + }, + type: 'object', + }, + description: 'Inspect', + name: 'inspect', + }, + }, + ], + }); + + expect(first.cacheStability?.canonicalToolManifestHash).toBe( + second.cacheStability?.canonicalToolManifestHash, + ); + expect(first.cacheStability?.toolSchemaHash).not.toBe( + second.cacheStability?.toolSchemaHash, + ); + }); + + it('canonicalizes shared schema references like duplicated schema objects', () => { + const sharedStringSchema = { type: 'string', description: 'Value' }; + const sharedSchema = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { + a: sharedStringSchema, + b: sharedStringSchema, + }, + }, + }, + }, + ], + }); + const duplicatedSchema = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { + a: { description: 'Value', type: 'string' }, + b: { description: 'Value', type: 'string' }, + }, + }, + }, + }, + ], + }); + + expect(sharedSchema.cacheStability?.canonicalToolManifestHash).toBe( + duplicatedSchema.cacheStability?.canonicalToolManifestHash, + ); + }); + + it('changes canonical manifest hash when schema content changes', () => { + const before = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { type: 'object', properties: {} }, + }, + }, + ], + }); + const after = summarizeOpenAIWireRequest({ + model: 'wire-model', + stream: false, + messages: [], + tools: [ + { + type: 'function', + function: { + name: 'inspect', + description: 'Inspect', + parameters: { + type: 'object', + properties: { path: { type: 'string' } }, + }, + }, + }, + ], + }); + + expect(before.cacheStability?.canonicalToolManifestHash).not.toBe( + after.cacheStability?.canonicalToolManifestHash, + ); + }); }); diff --git a/packages/core/src/utils/runtimeDiagnostics.ts b/packages/core/src/utils/runtimeDiagnostics.ts index 74f367bba9c..b8bbbb46b5b 100644 --- a/packages/core/src/utils/runtimeDiagnostics.ts +++ b/packages/core/src/utils/runtimeDiagnostics.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { createHash } from 'node:crypto'; import type { GenerateContentParameters } from '@google/genai'; import type Anthropic from '@anthropic-ai/sdk'; import type OpenAI from 'openai'; @@ -50,6 +51,21 @@ export interface RuntimeToolSchemaDiagnostics { schemaBytes: number; } +export type OpenAICacheStabilityProvider = 'deepseek' | 'openai-compatible'; + +export interface OpenAICacheStabilityDiagnostics { + provider?: OpenAICacheStabilityProvider; + toolNames: string[]; + toolNameSequenceHash: string; + toolNameSetHash: string; + toolSchemaHash: string; + canonicalToolManifestHash: string; +} + +export interface OpenAIWireRequestDiagnosticsOptions { + provider?: OpenAICacheStabilityProvider; +} + export interface OpenAIWireRequestDiagnostics { index?: number; timestamp?: string; @@ -60,6 +76,7 @@ export interface OpenAIWireRequestDiagnostics { messageBytesByRole: Record; toolsCount: number; toolSchemaBytes: number; + cacheStability?: OpenAICacheStabilityDiagnostics; topLevelKeys: string[]; } @@ -182,6 +199,7 @@ export class RuntimeDiagnosticsCollector { recordOpenAIWireRequest( request: OpenAI.Chat.ChatCompletionCreateParams, + options: OpenAIWireRequestDiagnosticsOptions = {}, ): void { if (!this.enabled) { return; @@ -191,7 +209,7 @@ export class RuntimeDiagnosticsCollector { this.openaiWireRequests.push({ index: this.openAIWireRequestIndex, timestamp: this.now(), - ...summarizeOpenAIWireRequest(request), + ...summarizeOpenAIWireRequest(request, options), }); } @@ -266,6 +284,12 @@ export class RuntimeDiagnosticsCollector { openaiWireRequests: this.openaiWireRequests.map((request) => ({ ...request, messageBytesByRole: { ...request.messageBytesByRole }, + cacheStability: request.cacheStability + ? { + ...request.cacheStability, + toolNames: [...request.cacheStability.toolNames], + } + : undefined, topLevelKeys: [...request.topLevelKeys], })), anthropicWireRequests: this.anthropicWireRequests.map((request) => ({ @@ -300,6 +324,7 @@ export const runtimeDiagnostics = new RuntimeDiagnosticsCollector(); export function summarizeOpenAIWireRequest( request: OpenAI.Chat.ChatCompletionCreateParams, + options: OpenAIWireRequestDiagnosticsOptions = {}, ): OpenAIWireRequestDiagnostics { const requestRecord = asRecord(request); const messages = Array.isArray(requestRecord['messages']) @@ -330,6 +355,7 @@ export function summarizeOpenAIWireRequest( messageBytesByRole, toolsCount: tools.length, toolSchemaBytes: utf8Bytes(tools), + cacheStability: summarizeOpenAIToolCacheStability(tools, options), topLevelKeys: Object.keys(requestRecord).sort(), }; } @@ -501,6 +527,106 @@ function summarizeToolSchemas(tools: unknown): RuntimeToolSchemaDiagnostics { }; } +function summarizeOpenAIToolCacheStability( + tools: unknown[], + options: OpenAIWireRequestDiagnosticsOptions, +): OpenAICacheStabilityDiagnostics { + const toolNames = extractOpenAIToolNames(tools); + return { + provider: options.provider, + toolNames, + toolNameSequenceHash: hashStableJson(toolNames), + toolNameSetHash: hashStableJson(Array.from(new Set(toolNames)).sort()), + toolSchemaHash: hashString(safeStringify(tools)), + canonicalToolManifestHash: hashStableJson( + buildCanonicalOpenAIToolManifest(tools), + ), + }; +} + +function extractOpenAIToolNames(tools: unknown[]): string[] { + const names: string[] = []; + for (const tool of tools) { + const toolRecord = asRecord(tool); + const fn = asOptionalRecord(toolRecord['function']); + const name = fn && typeof fn['name'] === 'string' ? fn['name'] : undefined; + if (name) { + names.push(name); + } + } + return names; +} + +function buildCanonicalOpenAIToolManifest(tools: unknown[]): Array<{ + name: string; + descriptionHash: string; + parametersHash: string; +}> { + const manifest: Array<{ + name: string; + descriptionHash: string; + parametersHash: string; + }> = []; + for (const tool of tools) { + const toolRecord = asRecord(tool); + const fn = asOptionalRecord(toolRecord['function']); + if (!fn || typeof fn['name'] !== 'string') { + continue; + } + manifest.push({ + name: fn['name'], + descriptionHash: hashString( + typeof fn['description'] === 'string' ? fn['description'] : '', + ), + parametersHash: hashStableJson(fn['parameters']), + }); + } + manifest.sort((a, b) => a.name.localeCompare(b.name)); + return manifest; +} + +function hashStableJson(value: unknown): string { + return hashString(stableStringify(value)); +} + +function hashString(value: string): string { + return createHash('sha256').update(value).digest('hex'); +} + +function stableStringify(value: unknown): string { + try { + return JSON.stringify(toStableJsonValue(value)) ?? ''; + } catch { + return '[unserializable]'; + } +} + +function toStableJsonValue( + value: unknown, + seen = new WeakSet(), +): unknown { + if (value === null || typeof value !== 'object') { + return value; + } + if (seen.has(value)) { + return '[Circular]'; + } + seen.add(value); + try { + if (Array.isArray(value)) { + return value.map((item) => toStableJsonValue(item, seen)); + } + const record = value as Record; + const stableRecord: Record = {}; + for (const key of Object.keys(record).sort()) { + stableRecord[key] = toStableJsonValue(record[key], seen); + } + return stableRecord; + } finally { + seen.delete(value); + } +} + function toJsonSafeRequest(request: GenerateContentParameters): unknown { return { model: request.model, diff --git a/packages/vscode-ide-companion/src/services/acpConnection.ts b/packages/vscode-ide-companion/src/services/acpConnection.ts index ec59490cd0d..272af4ebb5b 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.ts @@ -42,6 +42,13 @@ import * as fs from 'node:fs'; import { AcpFileHandler } from './acpFileHandler.js'; import { ACP_ERROR_CODES } from '../constants/acpSchema.js'; +export type AcpHistorySnapshot = + | unknown[] + | { + history: unknown[]; + modelFacingUserTurnCount: number; + }; + /** * ACP Connection Handler for VSCode Extension * @@ -493,7 +500,7 @@ export class AcpConnection { async rewindSession( targetTurnIndex: number, - ): Promise<{ historyBeforeRewind?: unknown[] }> { + ): Promise<{ historyBeforeRewind?: AcpHistorySnapshot }> { const conn = this.ensureConnection(); if (!this.sessionId) { throw new Error('No active ACP session'); @@ -503,10 +510,10 @@ export class AcpConnection { sessionId: this.sessionId, targetTurnIndex, cwd: this.workingDir, - })) as { historyBeforeRewind?: unknown[] }; + })) as { historyBeforeRewind?: AcpHistorySnapshot }; } - async restoreSessionHistory(history: unknown[]): Promise { + async restoreSessionHistory(history: AcpHistorySnapshot): Promise { const conn = this.ensureConnection(); if (!this.sessionId) { throw new Error('No active ACP session'); diff --git a/packages/vscode-ide-companion/src/services/qwenAgentManager.ts b/packages/vscode-ide-companion/src/services/qwenAgentManager.ts index 703535abd9a..64280e5c29b 100644 --- a/packages/vscode-ide-companion/src/services/qwenAgentManager.ts +++ b/packages/vscode-ide-companion/src/services/qwenAgentManager.ts @@ -3,7 +3,7 @@ * Copyright 2025 Qwen Team * SPDX-License-Identifier: Apache-2.0 */ -import { AcpConnection } from './acpConnection.js'; +import { AcpConnection, type AcpHistorySnapshot } from './acpConnection.js'; import type { ModelInfo, AvailableCommand, @@ -41,7 +41,7 @@ import { isAuthenticationRequiredError } from '../utils/authErrors.js'; import { getErrorMessage } from '../utils/errorMessage.js'; import { handleAuthenticateUpdate } from '../utils/authNotificationHandler.js'; -export type { ChatMessage, PlanEntry, ToolCallUpdateData }; +export type { AcpHistorySnapshot, ChatMessage, PlanEntry, ToolCallUpdateData }; /** * Extract session list items from ACP response. @@ -398,11 +398,11 @@ export class QwenAgentManager { async rewindSession( targetTurnIndex: number, - ): Promise<{ historyBeforeRewind?: unknown[] }> { + ): Promise<{ historyBeforeRewind?: AcpHistorySnapshot }> { return this.connection.rewindSession(targetTurnIndex); } - async restoreSessionHistory(history: unknown[]): Promise { + async restoreSessionHistory(history: AcpHistorySnapshot): Promise { await this.connection.restoreSessionHistory(history); } diff --git a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts index a8400c7efbc..8f25e788bab 100644 --- a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts +++ b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.test.ts @@ -238,7 +238,10 @@ describe('SessionMessageHandler', () => { isConnected: true, currentSessionId: 'session-1', rewindSession: vi.fn().mockResolvedValue({ - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'first' }] }], + historyBeforeRewind: { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }, }), restoreSessionHistory: vi.fn().mockResolvedValue(undefined), sendMessage: vi.fn().mockResolvedValue(undefined), @@ -445,7 +448,10 @@ describe('SessionMessageHandler', () => { isConnected: true, currentSessionId: 'session-1', rewindSession: vi.fn().mockResolvedValue({ - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'first' }] }], + historyBeforeRewind: { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }, }), restoreSessionHistory: vi.fn().mockResolvedValue(undefined), sendMessage: vi.fn().mockResolvedValue(undefined), @@ -524,12 +530,14 @@ describe('SessionMessageHandler', () => { createdAt: 1, updatedAt: 4, }; + const historyBeforeRewind = { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }; const agentManager = { isConnected: true, currentSessionId: 'session-1', - rewindSession: vi.fn().mockResolvedValue({ - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'first' }] }], - }), + rewindSession: vi.fn().mockResolvedValue({ historyBeforeRewind }), restoreSessionHistory: vi.fn().mockResolvedValue(undefined), sendMessage: vi.fn().mockRejectedValue(new Error('send failed')), }; @@ -557,9 +565,9 @@ describe('SessionMessageHandler', () => { }, }); - expect(agentManager.restoreSessionHistory).toHaveBeenCalledWith([ - { role: 'user', parts: [{ text: 'first' }] }, - ]); + expect(agentManager.restoreSessionHistory).toHaveBeenCalledWith( + historyBeforeRewind, + ); expect(conversationStore.replaceMessages).toHaveBeenCalledWith( 'session-1', originalConversation.messages, @@ -586,7 +594,10 @@ describe('SessionMessageHandler', () => { isConnected: true, currentSessionId: 'session-1', rewindSession: vi.fn().mockResolvedValue({ - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'first' }] }], + historyBeforeRewind: { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }, }), restoreSessionHistory: vi.fn(), sendMessage: vi.fn().mockResolvedValue(undefined), @@ -658,7 +669,10 @@ describe('SessionMessageHandler', () => { isConnected: true, currentSessionId: 'session-1', rewindSession: vi.fn().mockResolvedValue({ - historyBeforeRewind: [{ role: 'user', parts: [{ text: 'first' }] }], + historyBeforeRewind: { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }, }), restoreSessionHistory: vi.fn().mockResolvedValue(undefined), sendMessage: vi.fn().mockResolvedValue(undefined), @@ -781,7 +795,10 @@ describe('SessionMessageHandler', () => { promptImages: [], }); - const historyBeforeRewind = [{ role: 'user', parts: [{ text: 'first' }] }]; + const historyBeforeRewind = { + history: [{ role: 'user', parts: [{ text: 'first' }] }], + modelFacingUserTurnCount: 1, + }; const originalConversation = { id: 'session-1', title: 'Existing session', diff --git a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.ts b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.ts index c60b866abee..d0d30b1c8e8 100644 --- a/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.ts +++ b/packages/vscode-ide-companion/src/webview/handlers/SessionMessageHandler.ts @@ -6,7 +6,10 @@ import * as vscode from 'vscode'; import { BaseMessageHandler } from './BaseMessageHandler.js'; -import type { ChatMessage } from '../../services/qwenAgentManager.js'; +import type { + AcpHistorySnapshot, + ChatMessage, +} from '../../services/qwenAgentManager.js'; import type { Conversation } from '../../services/conversationStore.js'; import type { ImageAttachment } from '../../utils/imageSupport.js'; import type { ApprovalModeValue } from '../../types/approvalModeValueTypes.js'; @@ -594,7 +597,7 @@ export class SessionMessageHandler extends BaseMessageHandler { let editRestoreSnapshot: Conversation | null = null; let editStoreMutationApplied = false; let editAcpMutationApplied = false; - let editAcpHistorySnapshot: unknown[] | null = null; + let editAcpHistorySnapshot: AcpHistorySnapshot | null = null; if (editTargetTurnIndex !== undefined) { if (!Number.isInteger(editTargetTurnIndex) || editTargetTurnIndex < 0) {