diff --git a/.github/workflows/label-pr-review-state.yml b/.github/workflows/label-pr-review-state.yml index 98f7428c7a..08ecf2e3fe 100644 --- a/.github/workflows/label-pr-review-state.yml +++ b/.github/workflows/label-pr-review-state.yml @@ -14,7 +14,7 @@ on: # This workflow only reads PR metadata and never checks out or executes PR code. # pull_request_target gives fork PRs a token that can update labels and comments. pull_request_target: - types: [opened, reopened, ready_for_review, synchronize, review_requested, review_request_removed, labeled, unlabeled] + types: [opened, reopened, ready_for_review, converted_to_draft, synchronize, review_requested, review_request_removed, labeled, unlabeled] pull_request_review: types: [submitted, dismissed] # Fork review events have a read-only token. CodeRabbit's status-comment update @@ -81,6 +81,11 @@ jobs: color: '5319e7', description: 'Required CI passed; CodeRabbit review is active', }, + { + name: 'community-approved', + color: 'c2e0c6', + description: 'Fresh community approval on the current head; maintainer review still required', + }, ]; const guideMarker = ''; const codeRabbitLabelMarkerPrefix = '`); const match = comment?.body?.match(markerPattern); @@ -419,7 +469,10 @@ jobs: return `${guideMarker}\n### Review status\n\n${authorNote}\n\n` + `**Current step:** ${phaseMessage(phase)}${phaseHelp}\n\n` + - 'Review-state labels are managed by this workflow; do not edit them manually.' + + 'Review-state labels are managed by this workflow; do not edit them manually. ' + + '`community-approved` is managed the same way — do not add or remove it manually. ' + + 'It signals a fresh community code approval for the current head as an advisory ' + + 'priority only; maintainer review is still required.' + labelMarker; } @@ -497,6 +550,7 @@ jobs: const existingGuide = await findReviewGuide(pr); await setCodeRabbitReviewActive(pr, false); await reconcileLabels(pr, null); + await reconcileCommunityApproval(pr, 'remove'); await updateReviewGuide(pr, 'configuration-error', existingGuide); continue; } @@ -518,6 +572,7 @@ jobs: await updateReviewGate(pr, 'conflict', false); await setCodeRabbitReviewActive(pr, false); await reconcileLabels(pr, 'has-conflicts'); + await reconcileCommunityApproval(pr, 'remove'); await updateReviewGuide(pr, 'conflict', existingGuide); continue; } @@ -623,6 +678,7 @@ jobs: await updateReviewGate(pr, ciPending ? 'ci-pending' : 'ci-failed', false); await setCodeRabbitReviewActive(pr, false); await reconcileLabels(pr, null); + await reconcileCommunityApproval(pr, 'remove'); await updateReviewGuide( pr, ciPending ? 'ci-pending' : 'ci-failed', @@ -642,7 +698,10 @@ jobs: // block the PR or indicate the author needs to act. const latest = new Map(); for (const r of reviews) { - const reviewer = r.user.login.toLowerCase(); + // Ghost accounts have no usable identity; their reviews carry + // no actionable state for either review loop. + const reviewer = r.user?.login?.toLowerCase(); + if (!reviewer) continue; const reviewerKey = codeRabbitLogins.has(reviewer) ? codeRabbitLogin : reviewer; if (r.state === 'DISMISSED') { latest.delete(reviewerKey); @@ -742,6 +801,43 @@ jobs: freshMaintainerReviews.push(review); } } + // Community approval eligibility reuses the decisive per-reviewer + // reduction above: the reviewer's latest decisive review (selected + // by review id, not array order) must be an approval on the current + // head. Ghost accounts, bots, CodeRabbit, and the PR author are + // already excluded there; comments are neutral. Permission results + // are shared with maintainer classification through permissionFor: + // a 404 qualifies an otherwise valid reviewer, while a lookup + // failure is unknown permission and never counts as community + // eligibility. One eligible approval is enough; bot-authored PRs + // follow the same rule. A known invalidation — an approval that + // was dismissed, replaced, or left behind by a head change — + // takes priority over any lookup uncertainty. + const approvedByReviewer = new Set(); + for (const r of reviews) { + const reviewer = r.user?.login?.toLowerCase(); + if (reviewer && r.state === 'APPROVED') approvedByReviewer.add(reviewer); + } + let communityApprovalFresh = false; + let communityPermissionUncertain = false; + let communityApprovalInvalidated = false; + for (const [reviewer, review] of latestHumanReview) { + if (review.state === 'APPROVED' && review.commit_id === pr.head.sha) { + let permission; + try { + permission = await permissionFor(review.user.login); + } catch (error) { + communityPermissionUncertain = true; + continue; + } + if (!['admin', 'maintain', 'write'].includes(permission)) { + communityApprovalFresh = true; + break; + } + } else if (approvedByReviewer.has(reviewer)) { + communityApprovalInvalidated = true; + } + } const automatedAuthor = pr.user?.type === 'Bot'; const codeRabbitEligibleAuthor = !automatedAuthor || codeRabbitEligibleBotLogins.has(pr.user?.login.toLowerCase()); @@ -793,15 +889,24 @@ jobs: phase = 'approved'; } + // Advisory community label decision for the branches below. + // Non-maintainer phases always remove it; the maintainer queue + // refines the decision after the head/mergeability refetch. + let communityDecision = 'remove'; if (desiredLabel === 'awaiting-maintainer') { const { data: latestPrDetail } = await github.rest.pulls.get({ owner, repo, pull_number: pr.number, }); + // A head change since the event fetch invalidates every + // approval, even if another check is uncertain. + const headChanged = Boolean(latestPrDetail.head?.sha) && + latestPrDetail.head.sha !== pr.head.sha; if (latestPrDetail.mergeable === false && latestPrDetail.mergeable_state === 'dirty') { core.info(`PR #${pr.number}: has merge conflicts — labeling has-conflicts`); await updateReviewGate(pr, 'conflict', false); await setCodeRabbitReviewActive(pr, false); await reconcileLabels(pr, 'has-conflicts'); + await reconcileCommunityApproval(pr, 'remove'); await updateReviewGuide(pr, 'conflict', existingGuide); continue; } @@ -815,8 +920,23 @@ jobs: error.preserveStateLabels = true; throw error; } + // Unknown mergeability alone, with an unchanged head and no + // known invalidation, preserves an existing label and never adds it. + await reconcileCommunityApproval( + pr, + headChanged || communityApprovalInvalidated ? 'remove' : 'preserve' + ); continue; } + communityDecision = headChanged + ? 'remove' + : communityApprovalFresh + ? 'add' + : communityApprovalInvalidated + ? 'remove' + : communityPermissionUncertain + ? 'preserve' + : 'remove'; } core.info( @@ -842,6 +962,7 @@ jobs: await updateReviewGuide(pr, phase, existingGuide); } await reconcileLabels(pr, desiredLabel); + await reconcileCommunityApproval(pr, communityDecision); if (readyForMaintainer) { await updateReviewGate(pr, phase, true); } @@ -864,6 +985,9 @@ jobs: } catch (cleanupError) { metadataErrors.push(cleanupError); } + // A global failure that clears the maintainer queue clears the + // advisory community signal too. + await reconcileCommunityApproval(pr, 'remove'); } const detail = error.status ? `${error.message} (HTTP ${error.status}${error.response?.data?.message ? `: ${error.response.data.message}` : ''})` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f22cf96e9a..4b40d7e504 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -134,7 +134,7 @@ pnpm install ### Review Process -Ready-for-review PRs must pass required CI checks, address actionable review feedback, and receive fresh maintainer or CODEOWNER approval after automated review completes for the latest commit. Automated review may add a guidance comment and managed state labels; contributors should follow the indicated next step rather than editing those labels directly. New commits reset review state for the updated code. +Ready-for-review PRs must pass required CI checks, address actionable review feedback, and receive fresh maintainer or CODEOWNER approval after automated review completes for the latest commit. Automated review may add a guidance comment and managed state labels; contributors should follow the indicated next step rather than editing those labels directly. New commits reset review state for the updated code. A `community-approved` label marks a fresh community approval on the latest commit while the PR waits for maintainer review. It is an advisory priority signal for maintainers; it does not replace maintainer review or any required check, and it is managed by the review workflow. Automated review supports maintainers but does not replace human approval. Outside contributors cannot direct CodeRabbit through comments; eligible reviews restart automatically after required CI passes, and maintainers handle any exceptional restart. Warnings are advisory unless repository policy says otherwise, and native GitHub required-check, CODEOWNER, and review protections remain authoritative for merging. diff --git a/apps/vscode-e2e/package.json b/apps/vscode-e2e/package.json index 4a2bed8a87..d19b5b765b 100644 --- a/apps/vscode-e2e/package.json +++ b/apps/vscode-e2e/package.json @@ -29,7 +29,7 @@ "@vscode/test-electron": "2.5.2", "dotenv-cli": "11.0.0", "glob": "11.1.0", - "mocha": "11.2.2", + "mocha": "11.8.0", "rimraf": "6.0.1" } } diff --git a/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png b/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png index e8497070ef..3c03cbac03 100644 Binary files a/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png and b/apps/vscode-e2e/src/visual/__screenshots__/electron-chat-dark-sidebar.png differ diff --git a/package.json b/package.json index 1dd8d3d260..1bf0b4e8dd 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ ], "overrides": { "tar-fs": ">=3.1.1", + "mocha>diff": "8.0.4", "esbuild": "0.28.1", "rollup": "4.60.4", "vite": "8.1.0", diff --git a/packages/types/src/__tests__/openai-models.test.ts b/packages/types/src/__tests__/openai-models.test.ts index bc2b41427c..78a3259f09 100644 --- a/packages/types/src/__tests__/openai-models.test.ts +++ b/packages/types/src/__tests__/openai-models.test.ts @@ -1,7 +1,69 @@ -import { openAiCodexModels } from "../providers/openai-codex.js" +import { openAiCodexDefaultModelId, openAiCodexModels } from "../providers/openai-codex.js" import { openAiNativeDefaultModelId, openAiNativeModels } from "../providers/openai.js" describe("OpenAI native models", () => { + it("describes GPT-6.1 Sol pricing and required reasoning without changing defaults", () => { + expect(openAiNativeDefaultModelId).toBe("gpt-5.6-sol") + expect(openAiCodexDefaultModelId).toBe("gpt-5.6-sol") + expect(openAiNativeModels["gpt-6.1-sol"]).toMatchObject({ + maxTokens: 128_000, + contextWindow: 1_050_000, + includedTools: ["apply_patch"], + excludedTools: ["apply_diff", "write_to_file"], + supportsImages: true, + supportsPromptCache: true, + supportsReasoningEffort: ["low", "medium", "high", "xhigh", "max"], + requiredReasoningEffort: true, + reasoningEffort: "medium", + supportsTemperature: false, + supportsVerbosity: true, + inputPrice: 2, + outputPrice: 10, + cacheWritesPrice: 2.5, + cacheReadsPrice: 0.1, + longContextPricing: { + thresholdTokens: 272_000, + inputPriceMultiplier: 2, + outputPriceMultiplier: 1.5, + cacheWritesPriceMultiplier: 2, + cacheReadsPriceMultiplier: 2, + appliesToServiceTiers: ["default", "flex", "priority"], + }, + }) + expect(openAiNativeModels["gpt-6.1-sol"].tiers).toEqual([ + { + name: "flex", + contextWindow: 1_050_000, + inputPrice: 1, + outputPrice: 5, + cacheWritesPrice: 1.25, + cacheReadsPrice: 0.05, + }, + { + name: "priority", + contextWindow: 1_050_000, + inputPrice: 4, + outputPrice: 20, + cacheWritesPrice: 5, + cacheReadsPrice: 0.2, + }, + ]) + expect(openAiCodexModels["gpt-6.1-sol"]).toMatchObject({ + maxTokens: 128_000, + contextWindow: 872_000, + includedTools: ["apply_patch"], + excludedTools: ["apply_diff", "write_to_file"], + supportsImages: true, + supportsPromptCache: true, + supportsReasoningEffort: ["low", "medium", "high", "xhigh", "max"], + requiredReasoningEffort: true, + reasoningEffort: "low", + supportsTemperature: false, + inputPrice: 0, + outputPrice: 0, + }) + }) + it("describes GPT-6 Astra without changing the provider default", () => { expect(openAiNativeDefaultModelId).toBe("gpt-5.6-sol") expect(openAiNativeModels["gpt-6-astra"]).toMatchObject({ diff --git a/packages/types/src/__tests__/opencode-go.test.ts b/packages/types/src/__tests__/opencode-go.test.ts index 2a69eb7c00..7a253d156f 100644 --- a/packages/types/src/__tests__/opencode-go.test.ts +++ b/packages/types/src/__tests__/opencode-go.test.ts @@ -33,6 +33,7 @@ describe("opencode-go registry", () => { "mimo-v2.5-pro", "deepseek-v4-pro", "deepseek-v4-flash", + "deepseek-v4.1-flash", "omen-alpha", "grok-4.5", ] @@ -224,6 +225,19 @@ describe("opencode-go registry", () => { cacheReadsPrice: 0.19, }, }, + { + id: "deepseek-v4.1-flash", + expected: { + maxTokens: 384_000, + contextWindow: 1_000_000, + supportsImages: true, + supportsPromptCache: true, + supportsMaxTokens: true, + inputPrice: 0.3, + outputPrice: 1.2, + cacheReadsPrice: 0.006, + }, + }, { id: "longcat-2.0", expected: { @@ -444,6 +458,7 @@ describe("opencode-go registry", () => { it("DeepSeek entries expose supportsMaxTokens so the max-output slider is available", () => { expect(getOpencodeGoModelInfo("deepseek-v4-pro")?.supportsMaxTokens).toBe(true) expect(getOpencodeGoModelInfo("deepseek-v4-flash")?.supportsMaxTokens).toBe(true) + expect(getOpencodeGoModelInfo("deepseek-v4.1-flash")?.supportsMaxTokens).toBe(true) }) }) diff --git a/packages/types/src/providers/anthropic.ts b/packages/types/src/providers/anthropic.ts index 8103690da0..7584225aa0 100644 --- a/packages/types/src/providers/anthropic.ts +++ b/packages/types/src/providers/anthropic.ts @@ -165,7 +165,7 @@ export const anthropicModels = { description: "Claude Opus 5 is Anthropic's most capable model for complex agentic coding and enterprise work.", }, "claude-opus-5-5": { - maxTokens: 128_000, // Overridden to 8k if `enableReasoningEffort` is false. + maxTokens: 128_000, contextWindow: 1_000_000, // 1M context window native (no beta header required) supportsImages: true, supportsPromptCache: true, @@ -173,10 +173,7 @@ export const anthropicModels = { outputPrice: 20.0, // $20 per million output tokens cacheWritesPrice: 5.0, // $5 per million tokens cacheReadsPrice: 0.2, // $0.20 per million tokens - // Opus 5.5 keeps the Opus 5 adaptive-thinking / binary-toggle convention on - // the direct Anthropic provider path: manual budget_tokens and non-default - // sampling parameters return a 400. - supportsReasoningBudget: true, + // Opus 5.5 always uses adaptive thinking and rejects manual budget_tokens. supportsReasoningBinary: true, supportsTemperature: false, description: diff --git a/packages/types/src/providers/openai-codex.ts b/packages/types/src/providers/openai-codex.ts index dab48ef218..95ba2dfae1 100644 --- a/packages/types/src/providers/openai-codex.ts +++ b/packages/types/src/providers/openai-codex.ts @@ -24,6 +24,25 @@ export const openAiCodexDefaultModelId: OpenAiCodexModelId = "gpt-5.6-sol" * Costs are 0 as they are covered by the subscription. */ export const openAiCodexModels = { + // https://github.com/openai/codex/blob/main/codex-rs/models-manager/models.json + "gpt-6.1-sol": { + maxTokens: 128000, + // Use Codex's supported maximum rather than its 272K default compaction budget. + contextWindow: 872000, + includedTools: ["apply_patch"], + excludedTools: ["apply_diff", "write_to_file"], + supportsImages: true, + supportsPromptCache: true, + supportsReasoningEffort: ["low", "medium", "high", "xhigh", "max"], + requiredReasoningEffort: true, + reasoningEffort: "low", + inputPrice: 0, + outputPrice: 0, + supportsVerbosity: true, + supportsTemperature: false, + description: + "GPT-6.1 Sol: Near-Astra performance for complex coding and professional work via ChatGPT subscription", + }, "gpt-6-astra": { maxTokens: 128000, contextWindow: 872000, diff --git a/packages/types/src/providers/openai.ts b/packages/types/src/providers/openai.ts index 4f56bb740a..b5db12bb70 100644 --- a/packages/types/src/providers/openai.ts +++ b/packages/types/src/providers/openai.ts @@ -17,6 +17,51 @@ export const OPENAI_API_PROTOCOL = "openai" export const openAiNativeDefaultModelId: OpenAiNativeModelId = "gpt-5.6-sol" export const openAiNativeModels = { + // https://developers.openai.com/api/docs/models/gpt-6.1-sol + "gpt-6.1-sol": { + maxTokens: 128000, + contextWindow: 1_050_000, + includedTools: ["apply_patch"], + excludedTools: ["apply_diff", "write_to_file"], + supportsImages: true, + supportsPromptCache: true, + supportsReasoningEffort: ["low", "medium", "high", "xhigh", "max"], + requiredReasoningEffort: true, + reasoningEffort: "medium", + inputPrice: 2.0, + outputPrice: 10.0, + cacheWritesPrice: 2.5, + cacheReadsPrice: 0.1, + longContextPricing: { + thresholdTokens: 272_000, + inputPriceMultiplier: 2, + outputPriceMultiplier: 1.5, + cacheWritesPriceMultiplier: 2, + cacheReadsPriceMultiplier: 2, + appliesToServiceTiers: ["default", "flex", "priority"], + }, + supportsVerbosity: true, + supportsTemperature: false, + tiers: [ + { + name: "flex", + contextWindow: 1_050_000, + inputPrice: 1.0, + outputPrice: 5.0, + cacheWritesPrice: 1.25, + cacheReadsPrice: 0.05, + }, + { + name: "priority", + contextWindow: 1_050_000, + inputPrice: 4.0, + outputPrice: 20.0, + cacheWritesPrice: 5.0, + cacheReadsPrice: 0.2, + }, + ], + description: "GPT-6.1 Sol: Near-Astra performance for complex coding and professional work at a lower cost", + }, "gpt-6-astra": { maxTokens: 128000, contextWindow: 1_050_000, diff --git a/packages/types/src/providers/opencode-go.ts b/packages/types/src/providers/opencode-go.ts index b31db1bd99..17120858bc 100644 --- a/packages/types/src/providers/opencode-go.ts +++ b/packages/types/src/providers/opencode-go.ts @@ -465,6 +465,22 @@ export const opencodeGoModels: Record = { description: "DeepSeek-V4-Flash is DeepSeek's fast, cost-efficient V4 model supporting thinking and non-thinking modes. Available via the Opencode Go plan.", }, + "deepseek-v4.1-flash": { + maxTokens: 384_000, + contextWindow: 1_000_000, + supportsImages: true, + supportsPromptCache: true, + supportsMaxTokens: true, + supportsReasoningEffort: ["disable", "low", "medium", "high", "xhigh"], + preserveReasoning: true, + reasoningEffort: "high", + // Go has weekday peak/off-peak pricing; use peak rates until ModelInfo can express schedules. + inputPrice: 0.3, + outputPrice: 1.2, + cacheReadsPrice: 0.006, + description: + "DeepSeek V4.1 Flash is a fast, cost-efficient reasoning model with a 1M context window. Available via the Opencode Go plan.", + }, "deepseek-v4-flash-vision-exp": { maxTokens: 384_000, contextWindow: 1_000_000, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9b93c1cfe5..044ce1a3e4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,7 @@ settings: overrides: tar-fs: '>=3.1.1' + mocha>diff: 8.0.4 esbuild: 0.28.1 rollup: 4.60.4 vite: 8.1.0 @@ -190,8 +191,8 @@ importers: specifier: 11.1.0 version: 11.1.0 mocha: - specifier: 11.2.2 - version: 11.2.2 + specifier: 11.8.0 + version: 11.8.0 rimraf: specifier: 6.0.1 version: 6.0.1 @@ -799,7 +800,7 @@ importers: version: 0.16.47 lru-cache: specifier: ^11.1.0 - version: 11.2.2 + version: 11.5.3 lucide-react: specifier: ^1.18.0 version: 1.21.0(react@18.3.1) @@ -4789,6 +4790,10 @@ packages: resolution: {integrity: sha512-vtcDfH3TOjP8UekytvnHH1o1P4FcUdt4eQ1Y+Abap1tk/OB2MWQvcwS2ClCd1zuIhc3JKOx6p3kod8Vfys3E+A==} engines: {node: '>=0.3.1'} + diff@8.0.4: + resolution: {integrity: sha512-DPi0FmjiSU5EvQV0++GFDOJ9ASQUVFh5kD+OzOnYdi7n3Wpm9hWWGfB/O2blfHcMVTL5WkQXSnRiK9makhrcnw==} + engines: {node: '>=0.3.1'} + dingbat-to-unicode@1.0.1: resolution: {integrity: sha512-98l0sW87ZT58pU4i61wa2OHwxbiYSbuxsCBozaVnYX2iCnr3bLM3fIes1/ej7h1YdOKuKt/MLs706TVnALA65w==} @@ -5851,6 +5856,10 @@ packages: resolution: {integrity: sha512-41Cifkg6e8TylSpdtTpeLVMqvSBEVzTttHvERD741+pnZ8ANv0004MRL43QKPDlK9cGvNp6NZWZUBlbGXYxxng==} engines: {node: '>=0.12.0'} + is-path-inside@3.0.3: + resolution: {integrity: sha512-Fd4gABb+ycGAmKou8eMftCupSir5lRxqf4aD/vd0cD2qc4HL07OjCeuHMr8Ro4CoMaeCKDB0/ECBOVWjTwUvPQ==} + engines: {node: '>=8'} + is-plain-obj@2.1.0: resolution: {integrity: sha512-YWnfyRwxL/+SsrWYfOpUtz5b3YD+nyfkHvjbcanzk8zgyO4ASD67uVMRt8k5bM4lLMDnXfriRhOpemw+NfT1eA==} engines: {node: '>=8'} @@ -6402,8 +6411,8 @@ packages: lru-cache@10.4.3: resolution: {integrity: sha512-JNAzZcXrCt42VGLuYz0zfAzDfAvJWW6AfYlDBQyDV5DClI2m5sAmK+OIO7s59XfsRsWHp02jAJrRadPRGTt6SQ==} - lru-cache@11.2.2: - resolution: {integrity: sha512-F9ODfyqML2coTIsQpSkRHnLSZMtkU8Q+mSfcaIyKwy58u+8k5nvAYeiNhsyMARvzNcXJ9QfWVrcPsC9e9rAxtg==} + lru-cache@11.5.3: + resolution: {integrity: sha512-U4N8FgzmWxc8k1VH8Kr6lQg18U7Fjvby6wXHVRX/ZZ7IwWbRMgrRbP0Wrb5q5NVinryp4SQampHKdvtecItxUg==} engines: {node: 20 || >=22} lru-cache@5.1.1: @@ -6734,8 +6743,8 @@ packages: mlly@1.7.4: resolution: {integrity: sha512-qmdSIPC4bDJXgZTCR7XosJiNKySV7O215tsPtDN9iEO/7q/76b/ijtgRu/+epFXSJhijtTCCGp3DWS549P3xKw==} - mocha@11.2.2: - resolution: {integrity: sha512-VlSBxrPYHK4YNOEbFdkCxHQbZMoNzBkoPprqtZRW6311EUF/DlSxoycE2e/2NtRk4WKkIXzyrXDTrlikJMWgbw==} + mocha@11.8.0: + resolution: {integrity: sha512-VyCeUdGN3A9lmCTTgG4yuvY9ixxaDk+xt2R/7/+1AP6EqNG+G9OKkzBwhVtVYoNX8YsxNSgAl8mOv3IAeOpFbw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} hasBin: true @@ -8780,9 +8789,6 @@ packages: resolution: {integrity: sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA==} engines: {node: '>=0.10.0'} - workerpool@6.5.1: - resolution: {integrity: sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA==} - workerpool@9.2.0: resolution: {integrity: sha512-PKZqBOCo6CYkVOwAxWxQaSF2Fvb5Iv2fCeTP7buyWI2GiynWr46NcXSgK/idoV6e60dgCBfgYc+Un3HMvmqP8w==} @@ -9418,7 +9424,7 @@ snapshots: '@babel/compat-data': 8.0.0 '@babel/helper-validator-option': 8.0.0 browserslist: 4.24.5 - lru-cache: 11.2.2 + lru-cache: 11.5.3 semver: 7.8.5 '@babel/helper-create-class-features-plugin@8.0.1(@babel/core@8.0.1)': @@ -13147,6 +13153,8 @@ snapshots: diff@5.2.2: {} + diff@8.0.4: {} + dingbat-to-unicode@1.0.1: {} dir-glob@3.0.1: @@ -14440,6 +14448,8 @@ snapshots: is-number@7.0.0: {} + is-path-inside@3.0.3: {} + is-plain-obj@2.1.0: {} is-plain-obj@4.1.0: {} @@ -15001,7 +15011,7 @@ snapshots: lru-cache@10.4.3: {} - lru-cache@11.2.2: {} + lru-cache@11.5.3: {} lru-cache@5.1.1: dependencies: @@ -15577,25 +15587,26 @@ snapshots: pkg-types: 1.3.1 ufo: 1.6.1 - mocha@11.2.2: + mocha@11.8.0: dependencies: browser-stdout: 1.3.1 chokidar: 4.0.3 debug: 4.4.3(supports-color@8.1.1) - diff: 5.2.2 + diff: 8.0.4 escape-string-regexp: 4.0.0 find-up: 5.0.0 glob: 11.1.0 he: 1.2.0 + is-path-inside: 3.0.3 js-yaml: 4.2.0 log-symbols: 4.1.0 - minimatch: 5.1.6 + minimatch: 9.0.5 ms: 2.1.3 picocolors: 1.1.1 serialize-javascript: 6.0.2 strip-json-comments: 3.1.1 supports-color: 8.1.1 - workerpool: 6.5.1 + workerpool: 9.2.0 yargs: 17.7.2 yargs-parser: 21.1.1 yargs-unparser: 2.0.0 @@ -16033,7 +16044,7 @@ snapshots: path-scurry@2.0.0: dependencies: - lru-cache: 11.2.2 + lru-cache: 11.5.3 minipass: 7.1.2 path-to-regexp@8.2.0: {} @@ -17872,8 +17883,6 @@ snapshots: word-wrap@1.2.5: {} - workerpool@6.5.1: {} - workerpool@9.2.0: {} wrap-ansi@7.0.0: diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 56ccd52588..d0a8b274b8 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -12,7 +12,7 @@ vi.mock("vscode", () => ({ tabGroups: { onDidChangeTabs: vi.fn(), }, - onDidChangeActiveTextEditor: vi.fn(), + onDidChangeActiveTextEditor: vi.fn().mockReturnValue({ dispose: vi.fn() }), }, workspace: { registerTextDocumentContentProvider: vi.fn(), @@ -205,6 +205,7 @@ vi.mock("../core/webview/ClineProvider", async () => { { // Static method used by extension.ts getVisibleInstance: vi.fn().mockReturnValue(mockInstance), + getAllInstances: vi.fn().mockReturnValue([]), sideBarId: "zoo-code.SidebarProvider", }, ), @@ -238,6 +239,141 @@ describe("extension.ts", () => { settingsUpdatedHandler = undefined }) + test("initializes the code index scope and registers it for extension cleanup", async () => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const init = vi.spyOn(CodeIndexScope.prototype, "init") + const dispose = vi.spyOn(CodeIndexScope.prototype, "dispose") + try { + const { activate } = await import("../extension") + await activate(mockContext) + + const scopes = mockContext.subscriptions.filter((entry) => entry instanceof CodeIndexScope) + expect(scopes).toHaveLength(1) + expect(init).toHaveBeenCalledExactlyOnceWith() + expect(init.mock.contexts[0]).toBe(scopes[0]) + expect(dispose).not.toHaveBeenCalled() + scopes[0].dispose() + expect(dispose).toHaveBeenCalledExactlyOnceWith() + } finally { + init.mockRestore() + dispose.mockRestore() + } + }) + + test("publishes indexing status to matching providers and logs delivery failures", async () => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + const provider = ClineProvider.getVisibleInstance()! + Object.defineProperty(provider, "workspacePath", { configurable: true, value: "/workspace" }) + const getAll = vi.mocked(ClineProvider.getAllInstances) + const post = vi.mocked(provider.postMessageToWebview) + getAll.mockReturnValue([provider, provider]) + post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(undefined) + try { + const { activate } = await import("../extension") + await activate(mockContext) + const scope = mockContext.subscriptions.find((entry) => entry instanceof CodeIndexScope)! + const status = { + systemStatus: "Standby" as const, + message: "Ready", + processedItems: 0, + totalItems: 0, + currentItemUnit: "blocks", + workspacePath: "/workspace", + workspaceEnabled: true, + autoEnableDefault: true, + } + scope["statusManager"]!["publishStatus"](status) + await Promise.resolve() + expect(post).toHaveBeenCalledTimes(2) + expect(post).toHaveBeenCalledWith({ type: "indexingStatusUpdate", values: status }) + expect(log).toHaveBeenCalledWith( + "[CodeIndexStatusManager] Failed to publish indexing status:", + expect.objectContaining({ message: "delivery failed" }), + ) + scope.dispose() + } finally { + log.mockRestore() + getAll.mockReturnValue([]) + post.mockReset() + Reflect.deleteProperty(provider, "workspacePath") + } + }) + + test.each([ + { workspacePath: "/other-workspace", receivesStatus: false }, + { workspacePath: "/workspace", receivesStatus: true }, + { workspacePath: undefined, receivesStatus: true }, + { workspacePath: "", receivesStatus: true }, + ])( + "routes status for provider workspace=$workspacePath with receivesStatus=$receivesStatus", + async ({ workspacePath, receivesStatus }) => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const provider = ClineProvider.getVisibleInstance()! + Object.defineProperty(provider, "workspacePath", { configurable: true, value: workspacePath }) + const getAll = vi.mocked(ClineProvider.getAllInstances) + getAll.mockReturnValue([provider]) + try { + const { activate } = await import("../extension") + await activate(mockContext) + const scope = mockContext.subscriptions.find((entry) => entry instanceof CodeIndexScope)! + const status = { + systemStatus: "Indexing" as const, + message: "Processing confidential.ts", + processedItems: 1, + totalItems: 2, + currentItemUnit: "files", + workspacePath: "/workspace", + workspaceEnabled: true, + autoEnableDefault: true, + } + scope["statusManager"]!["publishStatus"](status) + await Promise.resolve() + if (receivesStatus) { + expect(provider.postMessageToWebview).toHaveBeenCalledExactlyOnceWith({ + type: "indexingStatusUpdate", + values: status, + }) + } else { + expect(provider.postMessageToWebview).not.toHaveBeenCalled() + } + scope.dispose() + } finally { + getAll.mockReturnValue([]) + Reflect.deleteProperty(provider, "workspacePath") + } + }, + ) + + test.each([new Error("scope initialization failed"), "scope initialization failed"])( + "continues activation and logs code index scope initialization failure: %s", + async (error) => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const init = vi.spyOn(CodeIndexScope.prototype, "init").mockImplementationOnce(() => { + throw error + }) + try { + const { activate } = await import("../extension") + await expect(activate(mockContext)).resolves.toBeDefined() + + const vscode = await import("vscode") + const channel = vi.mocked(vscode.window.createOutputChannel).mock.results.at(-1)?.value + expect(channel?.appendLine).toHaveBeenCalledWith( + "[CodeIndexScope] Failed to initialize: scope initialization failed", + ) + expect(mockContext.subscriptions.filter((entry) => entry instanceof CodeIndexScope)).toHaveLength(1) + } finally { + init.mockRestore() + } + }, + ) + test("does not call dotenv.config when optional .env does not exist", async () => { vi.resetModules() vi.clearAllMocks() diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 6a3192798f..a53fc62ce5 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -486,7 +486,6 @@ describe("AnthropicHandler", () => { apiKey: "test-api-key", apiModelId: "claude-opus-5-5", enableReasoningEffort: true, - modelMaxTokens: 32768, }) const stream = opusHandler.createMessage(systemPrompt, [ @@ -502,7 +501,7 @@ describe("AnthropicHandler", () => { const requestOptions = mockCreate.mock.calls[mockCreate.mock.calls.length - 1]?.[1] expect(requestBody?.thinking).toEqual({ type: "adaptive" }) expect(requestBody?.temperature).toBeUndefined() - expect(requestBody?.max_tokens).toBe(32768) + expect(requestBody?.max_tokens).toBe(128_000) expect(requestOptions?.headers?.["anthropic-beta"]).toContain("prompt-caching-2024-07-31") }) @@ -755,9 +754,9 @@ describe("AnthropicHandler", () => { expect(model.info.outputPrice).toBe(20.0) expect(model.info.cacheWritesPrice).toBe(5.0) expect(model.info.cacheReadsPrice).toBe(0.2) - expect(model.maxTokens).toBe(8192) + expect(model.maxTokens).toBe(128000) expect(model.info.supportsReasoningBinary).toBe(true) - expect(model.info.supportsReasoningBudget).toBe(true) + expect(model.info.supportsReasoningBudget).toBeUndefined() expect(model.info.supportsPromptCache).toBe(true) expect(model.info.supportsTemperature).toBe(false) expect(model.reasoningBudget).toBeUndefined() diff --git a/src/api/providers/__tests__/openai-sol-6.1.spec.ts b/src/api/providers/__tests__/openai-sol-6.1.spec.ts new file mode 100644 index 0000000000..ea9e8e59e8 --- /dev/null +++ b/src/api/providers/__tests__/openai-sol-6.1.spec.ts @@ -0,0 +1,116 @@ +vitest.mock("@roo-code/telemetry", () => ({ + TelemetryService: { instance: { captureException: vitest.fn() } }, +})) + +import type { ApiHandlerOptions } from "../../../shared/api" +import { openAiCodexOAuthManager } from "../../../integrations/openai-codex/oauth" +import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" +import { OpenAiNativeHandler } from "../openai-native" +import { OpenAiCodexHandler } from "../openai-codex" + +const reasoningCases: { options: ApiHandlerOptions; native: string; codex: string }[] = [ + { options: {}, native: "medium", codex: "low" }, + { options: { reasoningEffort: "none" }, native: "medium", codex: "low" }, + { options: { reasoningEffort: "minimal" }, native: "medium", codex: "low" }, + { options: { reasoningEffort: "disable" }, native: "medium", codex: "low" }, + { options: { enableReasoningEffort: false }, native: "medium", codex: "low" }, + ...(["low", "medium", "high", "xhigh", "max"] as const).map((effort) => ({ + options: { reasoningEffort: effort }, + native: effort, + codex: effort, + })), +] + +describe("GPT-6.1 Sol requests", () => { + afterEach(() => vitest.restoreAllMocks()) + + it.each(reasoningCases)("uses supported reasoning for $options", ({ options, native, codex }) => { + const settings = { apiModelId: "gpt-6.1-sol", openAiNativeApiKey: "test-key", ...options } + const nativeHandler = new OpenAiNativeHandler(settings) + const codexHandler = new OpenAiCodexHandler(settings) + expect(nativeHandler.getModel().id).toBe("gpt-6.1-sol") + expect(codexHandler.getModel().id).toBe("gpt-6.1-sol") + expect(nativeHandler["getReasoningEffort"](nativeHandler.getModel())).toBe(native) + expect(codexHandler["getReasoningEffort"](codexHandler.getModel())).toBe(codex) + }) + + it("builds native Responses tool requests with verified pricing and no temperature", () => { + const handler = new OpenAiNativeHandler({ + apiModelId: "gpt-6.1-sol", + openAiNativeApiKey: "test-key", + openAiNativeServiceTier: "priority", + modelTemperature: 0.7, + }) + const model = handler.getModel() + const body = handler["buildRequestBody"](model, [], "System", "low", handler["getReasoningEffort"](model), { + taskId: "sol-task", + tools: [ + { type: "function", function: { name: "read_file", parameters: { type: "object", properties: {} } } }, + ], + }) + expect(body).toMatchObject({ + model: "gpt-6.1-sol", + max_output_tokens: 128_000, + reasoning: { effort: "medium" }, + service_tier: "priority", + tools: [{ type: "function", name: "read_file", strict: true }], + }) + expect(body).not.toHaveProperty("temperature") + expect(handler["applyServiceTierPricing"](model.info, "priority")).toMatchObject({ + inputPrice: 4, + outputPrice: 20, + cacheReadsPrice: 0.2, + cacheWritesPrice: 5, + }) + }) + + it("routes Codex tool requests through Responses Lite with Fast mode and required reasoning", async () => { + const handler = new OpenAiCodexHandler({ + apiModelId: "gpt-6.1-sol", + reasoningEffort: "none", + openAiCodexServiceTier: "priority", + }) + vitest.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") + vitest.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("test-account") + const create = vitest.fn().mockResolvedValue(asyncStreamFrom([])) + Reflect.set(handler, "client", { responses: { create } }) + await collectStream( + handler.createMessage("System", [{ role: "user", content: "Read the file" }], { + taskId: "sol-task", + tools: [ + { + type: "function", + function: { name: "read_file", parameters: { type: "object", properties: {} } }, + }, + ], + }), + ) + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ + model: "gpt-6.1-sol", + service_tier: "priority", + prompt_cache_key: "sol-task", + reasoning: { effort: "low", summary: "auto", context: "all_turns" }, + tool_choice: "auto", + parallel_tool_calls: false, + input: expect.arrayContaining([ + expect.objectContaining({ + type: "additional_tools", + tools: [expect.objectContaining({ type: "function", name: "read_file" })], + }), + ]), + }), + expect.objectContaining({ + headers: expect.objectContaining({ + "x-openai-internal-codex-responses-lite": "true", + "session-id": "sol-task", + }), + }), + ) + const body = create.mock.calls[0][0] + expect(body).not.toHaveProperty("instructions") + expect(body).not.toHaveProperty("tools") + expect(body).not.toHaveProperty("max_output_tokens") + expect(body).not.toHaveProperty("temperature") + }) +}) diff --git a/src/api/providers/__tests__/vscode-lm.spec.ts b/src/api/providers/__tests__/vscode-lm.spec.ts index 8010a27532..86a9a30c38 100644 --- a/src/api/providers/__tests__/vscode-lm.spec.ts +++ b/src/api/providers/__tests__/vscode-lm.spec.ts @@ -69,7 +69,15 @@ vi.mock("vscode", () => { }) import * as vscode from "vscode" -import { VsCodeLmHandler, extractLeakedToolCalls, trailingPartialToolMarkerLength } from "../vscode-lm" +import { + VsCodeLmHandler, + extractLeakedToolCalls, + trailingPartialToolMarkerLength, + middleOutTruncate, + estimateMessagesChars, + truncateToolResultsToFitWindow, +} from "../vscode-lm" +import { checkContextWindowExceededError } from "../../../core/context/context-management/context-error-handling" import type { ApiHandlerOptions } from "../../../shared/api" import type { Anthropic } from "@anthropic-ai/sdk" import { openAiModelInfoSaneDefaults, vscodeLlmDefaultModelId, vscodeLlmModels } from "@roo-code/types" @@ -507,6 +515,174 @@ describe("VsCodeLmHandler", () => { }) }) + it("still trims oversized tool_results when the system prompt consumes most of the budget", async () => { + // A system prompt large enough to drive the raw budget negative; the clamp keeps trimming + // active for the case where the request is most oversized. + const systemPrompt = "S".repeat(handler.getCondenseContextWindow() * 3) + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "hi" }], + }, + { + role: "assistant", + content: [{ type: "tool_use", id: "t1", name: "some_tool", input: { a: 1 } }], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t1", content: "X".repeat(50_000) }], + }, + ] + + // No sendRequest response is queued: the request must be refused before it is sent, and a + // queued-but-unconsumed response would leak into later tests. + // The clamped floor cannot be met once the tool_result bottoms out at its minimum, so the + // request must be refused rather than sent over-window (which orphans the tool_result). + const stream = handler.createMessage(systemPrompt, messages, { taskId: "test-task" }) + let refusal: unknown + try { + for await (const _chunk of stream) { + // drain + } + } catch (error) { + refusal = error + } + const message = (refusal as Error | undefined)?.message ?? "" + expect(message).toMatch(/too large for this model's context window/) + + // The reported figure proves trimming ran: without it the estimate would still carry the + // full 50,000-char tool_result. + const reported = Number(message.match(/estimated ([\d,]+) characters/)?.[1]?.replace(/,/g, "")) + expect(reported).toBeLessThan(10_000) + + expect(mockLanguageModelChat.sendRequest).not.toHaveBeenCalled() + }) + + it("refuses a request that exceeds a small positive raw budget below the trimming floor", async () => { + // The clamp to MIN_TOOL_RESULT_CHARS only keeps trimming productive; admission must still + // respect the raw budget, otherwise a conversation between the raw budget and the floor is + // sent over-window. Sized so the remaining content exceeds the raw budget but stays under + // the floor, and so no tool_result is large enough for trimming to shrink anything. + const targetRawBudgetChars = 1000 + const systemPrompt = "S".repeat( + Math.floor(handler.getCondenseContextWindow() * 0.8 * 3) - targetRawBudgetChars, + ) + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "assistant", + content: [{ type: "tool_use", id: "t1", name: "some_tool", input: { a: 1 } }], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t1", content: "X".repeat(1999) }], + }, + ] + + // No sendRequest response is queued: refusal must happen before the request is sent. + const stream = handler.createMessage(systemPrompt, messages, { taskId: "test-task" }) + await expect( + (async () => { + for await (const _chunk of stream) { + // drain + } + })(), + ).rejects.toThrow(/too large for this model's context window/) + expect(mockLanguageModelChat.sendRequest).not.toHaveBeenCalled() + }) + + it("refuses with an error the task recognises as a context-window failure", async () => { + // Asserted through the real detector: without a recognised shape the task takes its + // generic retry path and re-sends the same over-window history instead of condensing. + const systemPrompt = "S".repeat(handler.getCondenseContextWindow() * 3) + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "assistant", + content: [{ type: "tool_use", id: "t1", name: "some_tool", input: { a: 1 } }], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t1", content: "X".repeat(50_000) }], + }, + ] + + const stream = handler.createMessage(systemPrompt, messages, { taskId: "test-task" }) + const refusal = await (async () => { + try { + for await (const _chunk of stream) { + // drain + } + } catch (error) { + return error + } + throw new Error("expected the request to be refused") + })() + + expect(checkContextWindowExceededError(refusal)).toBe(true) + }) + + it("sends a request that fits within a small positive raw budget", async () => { + const targetRawBudgetChars = 1000 + const systemPrompt = "S".repeat( + Math.floor(handler.getCondenseContextWindow() * 0.8 * 3) - targetRawBudgetChars, + ) + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Y".repeat(500) }], + }, + ] + + mockLanguageModelChat.sendRequest.mockResolvedValueOnce({ + stream: (async function* () { + yield new vscode.LanguageModelTextPart("ok") + return + })(), + text: (async function* () { + yield "ok" + return + })(), + }) + + const stream = handler.createMessage(systemPrompt, messages, { taskId: "test-task" }) + const chunks = await collectStream(stream) + + expect(mockLanguageModelChat.sendRequest).toHaveBeenCalled() + expect(chunks).toContainEqual({ type: "text", text: "ok" }) + }) + + it("sends the request when trimming brings the conversation back under budget", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "assistant", + content: [{ type: "tool_use", id: "t1", name: "some_tool", input: { a: 1 } }], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t1", content: "X".repeat(500_000) }], + }, + ] + + mockLanguageModelChat.sendRequest.mockResolvedValueOnce({ + stream: (async function* () { + yield new vscode.LanguageModelTextPart("ok") + return + })(), + text: (async function* () { + yield "ok" + return + })(), + }) + + const stream = handler.createMessage("system", messages, { taskId: "test-task" }) + for await (const _chunk of stream) { + // drain + } + + const sent = JSON.stringify(mockLanguageModelChat.sendRequest.mock.calls[0][0]) + expect(sent).toContain("characters truncated") + expect(sent).not.toContain("X".repeat(400_000)) + }) + it("should handle native tool calls when tools are provided", async () => { const systemPrompt = "You are a helpful assistant" const messages: Anthropic.Messages.MessageParam[] = [ @@ -2535,3 +2711,373 @@ describe("leaked tool-call parser scaling", () => { expect(extractLeakedToolCalls(cues(500), tools).calls).toHaveLength(0) }) }) + +describe("context-window tool_result truncation", () => { + describe("middleOutTruncate", () => { + it("returns text unchanged when within the limit", () => { + expect(middleOutTruncate("hello world", 100)).toBe("hello world") + }) + + it("returns text unchanged when its length exactly equals the limit", () => { + // Longer than the truncation marker, so `<=` is observable here: the marker-less + // fallback for tiny limits would otherwise return the same text under either operator. + const text = "A".repeat(200) + expect(middleOutTruncate(text, 200)).toBe(text) + }) + + it("stays within a limit too small to hold the truncation marker", () => { + const text = "A".repeat(500) + for (const limit of [1, 5, 50]) { + const result = middleOutTruncate(text, limit) + expect(result).toBe("A".repeat(limit)) + } + }) + + it("does not end a marker-less truncation on a lone high surrogate", () => { + // Index-based so a surrogate pair contributes BOTH of its code units to the assertion. + const codeUnits = (value: string): number[] => + Array.from({ length: value.length }, (_, index) => value.charCodeAt(index)) + + // The limit lands between the halves of the pair at index 3, which would otherwise be + // emitted alone and make the payload un-encodable as UTF-8. + const result = middleOutTruncate(`abc\uD83D\uDE00${"z".repeat(200)}`, 4) + + expect(codeUnits(result)).toEqual(codeUnits("abc")) + }) + + it("keeps the head and tail and inserts a truncation marker", () => { + const text = "A".repeat(500) + "B".repeat(500) + const result = middleOutTruncate(text, 200) + + expect(result.length).toBeLessThanOrEqual(200) + expect(result).toContain("characters truncated to fit the model context window") + expect(result.startsWith("A")).toBe(true) + expect(result.endsWith("B")).toBe(true) + }) + + it("returns an empty string for a non-positive limit", () => { + expect(middleOutTruncate("anything", 0)).toBe("") + }) + + // A lone surrogate cannot be encoded as UTF-8 and 400s the whole request. + it("never splits a surrogate pair across the removed middle", () => { + const pair = "\u{1F600}" // one astral char = high + low surrogate + const text = pair.repeat(400) + const result = middleOutTruncate(text, 200) + + expect(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { + const toolUseMessage = (id: string): Anthropic.Messages.MessageParam => ({ + role: "assistant", + content: [ + { type: "text", text: "Calling a tool." }, + { type: "tool_use", id, name: "some_tool", input: { a: 1 } }, + ], + }) + + const toolResultMessage = (id: string, content: string): Anthropic.Messages.MessageParam => ({ + role: "user", + content: [ + { type: "tool_result", tool_use_id: id, content }, + { type: "text", text: "env" }, + ], + }) + + const findBlock = (message: Anthropic.Messages.MessageParam, type: string) => + (message.content as unknown as Array<{ type: string; [key: string]: unknown }>).find( + (block) => block.type === type, + )! + + it("is a no-op when the conversation already fits the budget", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "small result"), + ] + const before = JSON.parse(JSON.stringify(messages)) + + truncateToolResultsToFitWindow(messages, 100_000) + + expect(messages).toEqual(before) + }) + + it("returns messages untouched when the budget is not a usable number", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "Y".repeat(50_000)), + ] + const before = JSON.parse(JSON.stringify(messages)) + + expect(truncateToolResultsToFitWindow(messages, 0).messages).toBe(messages) + expect(truncateToolResultsToFitWindow(messages, Number.NaN).messages).toBe(messages) + expect(messages).toEqual(before) + }) + + it("returns a remainingChars equal to a fresh scan on every path", () => { + // The caller trusts remainingChars instead of re-scanning, so incremental bookkeeping + // that drifts from estimateMessagesChars would silently admit an over-window request. + const scenarios: Array<{ name: string; messages: Anthropic.Messages.MessageParam[]; budget: number }> = [ + { + name: "unusable budget", + messages: [toolUseMessage("t1"), toolResultMessage("t1", "Y".repeat(50_000))], + budget: 0, + }, + { + name: "already fits", + messages: [toolUseMessage("t1"), toolResultMessage("t1", "small result")], + budget: 100_000, + }, + { + name: "single oversized result", + messages: [toolUseMessage("t1"), toolResultMessage("t1", "X".repeat(50_000))], + budget: 10_000, + }, + { + name: "several results, largest first", + messages: [ + toolUseMessage("t1"), + toolResultMessage("t1", "B".repeat(40_000)), + toolUseMessage("t2"), + toolResultMessage("t2", "C".repeat(20_000)), + toolUseMessage("t3"), + toolResultMessage("t3", "small but real result"), + ], + budget: 12_000, + }, + { + name: "floor reached, budget unreachable", + messages: [ + toolUseMessage("t1"), + toolResultMessage("t1", "D".repeat(30_000)), + toolUseMessage("t2"), + toolResultMessage("t2", "E".repeat(30_000)), + ], + budget: 100, + }, + { + name: "array-form content with an image part", + messages: [ + toolUseMessage("t1"), + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "t1", + content: [ + { type: "text", text: "Z".repeat(50_000) }, + { + type: "image", + source: { type: "base64", media_type: "image/png", data: "abc" }, + }, + ], + }, + ], + } as unknown as Anthropic.Messages.MessageParam, + ], + budget: 9_000, + }, + { + name: "plain string turn and a non-string non-array tool_result", + messages: [ + { role: "user", content: "a plain string turn" }, + toolUseMessage("t1"), + toolResultMessage("t1", "F".repeat(50_000)), + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t2", content: undefined }], + } as unknown as Anthropic.Messages.MessageParam, + ], + budget: 10_000, + }, + { + name: "astral characters that truncation may trim by a surrogate", + messages: [toolUseMessage("t1"), toolResultMessage("t1", "😀".repeat(20_000))], + budget: 7_000, + }, + ] + + for (const scenario of scenarios) { + const { messages, remainingChars } = truncateToolResultsToFitWindow(scenario.messages, scenario.budget) + expect(`${scenario.name}:${remainingChars}`).toBe(`${scenario.name}:${estimateMessagesChars(messages)}`) + } + }) + + it("truncates the largest tool_result first, sparing a smaller one above the floor", () => { + // Both results sit above MIN_TOOL_RESULT_CHARS, so descending order is what spares the + // smaller one; ascending order would shrink it before reaching the larger result. + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "B".repeat(30_000)), + toolUseMessage("t2"), + toolResultMessage("t2", "C".repeat(5_000)), + ] + + truncateToolResultsToFitWindow(messages, 20_000) + + expect(String(findBlock(messages[1], "tool_result").content)).toContain("characters truncated") + expect(findBlock(messages[3], "tool_result").content).toBe("C".repeat(5_000)) + }) + + it("truncates array-form tool_result content and preserves non-text parts", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "t1", + content: [ + { type: "text", text: "Z".repeat(50_000) }, + { type: "image", source: { type: "base64", media_type: "image/png", data: "abc" } }, + ], + }, + ], + } as unknown as Anthropic.Messages.MessageParam, + ] + + truncateToolResultsToFitWindow(messages, 10_000) + + const toolResult = findBlock(messages[1], "tool_result") + const parts = toolResult.content as Array<{ type: string; text?: string }> + expect(parts[0].text).toContain("characters truncated") + // Exact shape: a surviving original text part would leave a third element behind. + expect(parts.map((part) => part.type)).toEqual(["text", "image"]) + }) + + it("ignores string content and skips messages that cannot hold tool_result blocks", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { role: "user", content: "a plain string turn" }, + toolUseMessage("t1"), + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "t1", content: "W".repeat(50_000) }, + { type: "image", source: { type: "base64", media_type: "image/png", data: "abc" } }, + ], + } as unknown as Anthropic.Messages.MessageParam, + ] + + truncateToolResultsToFitWindow(messages, 10_000) + + expect(messages[0].content).toBe("a plain string turn") + expect(String(findBlock(messages[2], "tool_result").content)).toContain("characters truncated") + }) + + it("ignores a tool_result whose content is neither string nor array", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "V".repeat(50_000)), + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "t2", content: undefined }], + } as unknown as Anthropic.Messages.MessageParam, + ] + + truncateToolResultsToFitWindow(messages, 10_000) + + expect(findBlock(messages[2], "tool_result").content).toBeUndefined() + expect(String(findBlock(messages[1], "tool_result").content)).toContain("characters truncated") + }) + + it("leaves every tool_result untouched when the conversation already fits the budget", () => { + // Budget exceeds the estimated total, so the early return fires and nothing is trimmed. + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "U".repeat(3000)), + toolUseMessage("t2"), + toolResultMessage("t2", "T".repeat(2500)), + ] + + truncateToolResultsToFitWindow(messages, 5700) + + expect(findBlock(messages[1], "tool_result").content).toBe("U".repeat(3000)) + expect(findBlock(messages[3], "tool_result").content).toBe("T".repeat(2500)) + }) + + it("leaves a tool_result at or below the minimum size alone", () => { + const shortResult = "S".repeat(1500) + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", shortResult), + toolUseMessage("t2"), + toolResultMessage("t2", shortResult), + ] + + truncateToolResultsToFitWindow(messages, 100) + + expect(findBlock(messages[1], "tool_result").content).toBe(shortResult) + expect(findBlock(messages[3], "tool_result").content).toBe(shortResult) + }) + + it("shrinks an oversized tool_result so the conversation fits the budget", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "X".repeat(50_000)), + ] + + truncateToolResultsToFitWindow(messages, 10_000) + + const toolResult = findBlock(messages[1], "tool_result") + expect(toolResult.tool_use_id).toBe("t1") // pairing preserved + expect(String(toolResult.content).length).toBeLessThanOrEqual(10_000) + expect(String(toolResult.content)).toContain("characters truncated") + }) + + it("truncates the largest tool_result first and leaves small ones intact", () => { + const small = "small but real result" + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "B".repeat(40_000)), + toolUseMessage("t2"), + toolResultMessage("t2", small), + ] + + truncateToolResultsToFitWindow(messages, 12_000) + + expect(String(findBlock(messages[1], "tool_result").content)).toContain("characters truncated") + expect(findBlock(messages[3], "tool_result").content).toBe(small) // untouched + }) + + it("never truncates tool_use blocks, assistant text, or environment details", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + toolResultMessage("t1", "X".repeat(50_000)), + ] + + truncateToolResultsToFitWindow(messages, 8_000) + + expect(findBlock(messages[0], "text").text).toBe("Calling a tool.") + expect(findBlock(messages[0], "tool_use")).toMatchObject({ id: "t1", name: "some_tool" }) + expect(findBlock(messages[1], "text").text).toBe("env") + }) + + it("handles array-form tool_result content and keeps it valid", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + toolUseMessage("t1"), + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "t1", + content: [{ type: "text", text: "Y".repeat(40_000) }], + }, + ], + }, + ] + + truncateToolResultsToFitWindow(messages, 8_000) + + const toolResult = findBlock(messages[1], "tool_result") + expect(toolResult.tool_use_id).toBe("t1") + expect(Array.isArray(toolResult.content)).toBe(true) + const parts = toolResult.content as Array<{ type: string; text?: string }> + expect(parts[0].type).toBe("text") + expect(String(parts[0].text)).toContain("characters truncated") + }) + }) +}) diff --git a/src/api/providers/fetchers/__tests__/opencode-go.spec.ts b/src/api/providers/fetchers/__tests__/opencode-go.spec.ts index 123e912f7f..b39e4e772c 100644 --- a/src/api/providers/fetchers/__tests__/opencode-go.spec.ts +++ b/src/api/providers/fetchers/__tests__/opencode-go.spec.ts @@ -197,6 +197,7 @@ describe("Opencode Go Fetchers", () => { "glm-5", "deepseek-v4-pro", "deepseek-v4-flash", + "deepseek-v4.1-flash", "deepseek-v4-flash-vision-exp", "qwen3.7-max", "qwen3.8-max", @@ -257,6 +258,19 @@ describe("Opencode Go Fetchers", () => { expect(info.supportsReasoningEffort).toEqual(["disable", "low", "medium", "high", "xhigh"]) }) + it("resolves DeepSeek V4.1 Flash to its 1M context when live metadata is absent", () => { + const info = parseOpencodeGoModel({ id: "deepseek-v4.1-flash" }) + expect(info.contextWindow).toBe(1_000_000) + expect(info.maxTokens).toBe(384_000) + expect(info.supportsPromptCache).toBe(true) + expect(info.supportsMaxTokens).toBe(true) + expect(info.supportsReasoningEffort).toEqual(["disable", "low", "medium", "high", "xhigh"]) + expect(info.supportsImages).toBe(true) + expect(info.inputPrice).toBe(0.3) + expect(info.outputPrice).toBe(1.2) + expect(info.cacheReadsPrice).toBe(0.006) + }) + it("resolves GLM-5.2 with its 1M context and High/Max reasoning effort", () => { const info = parseOpencodeGoModel({ id: "glm-5.2" }) expect(info.contextWindow).toBe(1_000_000) diff --git a/src/api/providers/openai-codex.ts b/src/api/providers/openai-codex.ts index 1498aafc66..ff8f4a24b0 100644 --- a/src/api/providers/openai-codex.ts +++ b/src/api/providers/openai-codex.ts @@ -41,7 +41,7 @@ type OpenAiCodexRequestServiceTier = typeof OpenAiCodexServiceTier.Priority const CODEX_API_BASE_URL = "https://chatgpt.com/backend-api/codex" const LUNA_MODEL_ID = "gpt-5.6-luna" const LUNA_CODEX_VERSION = "0.144.0" -const RESPONSES_LITE_MODEL_IDS = new Set([LUNA_MODEL_ID, "gpt-6-astra"]) +const RESPONSES_LITE_MODEL_IDS = new Set([LUNA_MODEL_ID, "gpt-6-astra", "gpt-6.1-sol"]) /** * A refusal is streamed as text so the chat still shows why the model declined, but it is not part diff --git a/src/api/providers/vscode-lm.ts b/src/api/providers/vscode-lm.ts index d1ede2d09e..2771227680 100644 --- a/src/api/providers/vscode-lm.ts +++ b/src/api/providers/vscode-lm.ts @@ -24,6 +24,8 @@ import { sanitizeToolNameSurrogates, } from "../transform/vscode-lm-format" +import { CONTEXT_WINDOW_EXCEEDED_STATUS } from "../../core/context/context-management/context-error-handling" + import { BaseProvider } from "./base-provider" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index" @@ -357,6 +359,210 @@ function declaredParamType(schema: Record | undefined, paramNam return undefined } +/** + * Context-window safety for Copilot's backend + * ------------------------------------------- + * Copilot's backend enforces its own context window and, for third-party `sendRequest` callers, + * trims an over-window request in a way that is NOT tool-pair-aware: it can drop the assistant + * message holding a `tool_use` while keeping the matching `tool_result`, after which Anthropic + * rejects the request with "unexpected tool_use_id". To keep trimming on OUR side — where + * pairing is preserved — we shrink oversized `tool_result` payloads before sending. Only + * `tool_result` text is truncated (never `tool_use`, assistant text, summaries, or environment + * details), and only when the request would otherwise exceed the budget. + */ + +/** + * Conservative characters-per-token ratio used to turn a token window into a character budget. + * + * `client.countTokens` is the model's real tokenizer, but it counts only a string: it cannot price + * the tool schemas, image placeholders, or per-message framing the backend adds, so it cannot give + * the true total for the request we are about to send. It is also an async, per-call RPC, and the + * budget is needed for every message on every turn. We therefore keep a character estimate here + * and stay deliberately conservative — 3 chars/token rather than the ~4 typical of English — + * because the token-dense JSON, logs, and code that dominate oversized tool results tokenize to + * fewer characters per token than prose. Under-counting biases toward trimming too early, which is + * recoverable; over-counting sends an over-window request, which is not. + */ +const VSCODE_LM_BUDGET_CHARS_PER_TOKEN = 3 + +/** + * Fraction of the context window the *entire* input (system prompt + tool schemas + conversation) + * is allowed to occupy. The remaining headroom absorbs char/token estimation variance and any + * output/overhead the backend reserves. + */ +const VSCODE_LM_INPUT_BUDGET_FRACTION = 0.8 + +/** A tool_result is never shrunk below this many characters, so a truncated result stays useful. */ +const MIN_TOOL_RESULT_CHARS = 2000 + +/** + * Length charged for an image block. VS Code LM cannot carry image data, so + * `convertToVsCodeLmMessages` replaces each image with a sentence-long textual placeholder; this + * is that placeholder's approximate length. + */ +const IMAGE_PLACEHOLDER_CHARS = 64 + +function readToolResultText(block: Anthropic.Messages.ContentBlockParam): string | undefined { + if (!block || (block as { type?: string }).type !== "tool_result") { + return undefined + } + const content = (block as Anthropic.Messages.ToolResultBlockParam).content + if (typeof content === "string") { + return content + } + if (Array.isArray(content)) { + return content + .filter((part): part is Anthropic.Messages.TextBlockParam => (part as { type?: string })?.type === "text") + .map((part) => part.text ?? "") + .join("") + } + return undefined +} + +function writeToolResultText(block: Anthropic.Messages.ContentBlockParam, text: string): void { + const toolResult = block as Anthropic.Messages.ToolResultBlockParam + const content = toolResult.content + if (Array.isArray(content)) { + // Preserve any non-text parts (e.g. images) and collapse the text into one truncated part. + const nonText = content.filter((part) => (part as { type?: string })?.type !== "text") + toolResult.content = [{ type: "text", text }, ...nonText] as typeof content + return + } + toolResult.content = text +} + +/** + * Drops a trailing lone high surrogate, whose low half was cut away. A lone surrogate cannot be + * encoded as UTF-8, and the backend 400s the whole request when one is present. + */ +function trimTrailingHighSurrogate(text: string): string { + return text.length > 0 && (text.charCodeAt(text.length - 1) & 0xfc00) === 0xd800 ? text.slice(0, -1) : text +} + +/** + * Middle-out truncate `text` to at most `maxChars`, keeping the head and tail and replacing the + * middle with a marker noting how many characters were removed. Head/tail are preserved because + * logs and file dumps carry the most signal at their start (structure) and end (recent output). + */ +export function middleOutTruncate(text: string, maxChars: number): string { + if (maxChars <= 0) { + return "" + } + if (text.length <= maxChars) { + return text + } + + const buildMarker = (removed: number) => + `\n\n[... ${removed.toLocaleString("en-US")} characters truncated to fit the model context window ...]\n\n` + + // Reserve room for the marker, sized against the original length so the result never grows. + const reservedMarkerLength = buildMarker(text.length).length + // A budget too small to hold the marker cannot describe its own truncation without breaking the + // maxChars promise this function makes to callers, so drop the marker and keep a bare head. + if (maxChars <= reservedMarkerLength) { + return trimTrailingHighSurrogate(text.slice(0, maxChars)) + } + const keep = maxChars - reservedMarkerLength + const headLength = Math.ceil(keep / 2) + const tailLength = keep - headLength + const head = trimTrailingHighSurrogate(text.slice(0, headLength)) + let tail = tailLength > 0 ? text.slice(text.length - tailLength) : "" + // Likewise, don't start the tail on a lone low surrogate (its high half is in the removed middle). + if (tail.length > 0 && (tail.charCodeAt(0) & 0xfc00) === 0xdc00) { + tail = tail.slice(1) + } + const removed = text.length - head.length - tail.length + return `${head}${buildMarker(removed)}${tail}` +} + +/** Estimated character cost of a whole conversation, using the same accounting as truncation. */ +export function estimateMessagesChars(messages: Anthropic.Messages.MessageParam[]): number { + return messages.reduce((sum, message) => sum + estimateContentChars(message.content), 0) +} + +function estimateContentChars(content: Anthropic.Messages.MessageParam["content"]): number { + if (typeof content === "string") { + return content.length + } + if (!Array.isArray(content)) { + return 0 + } + let total = 0 + for (const block of content) { + const type = (block as { type?: string })?.type + if (type === "text") { + total += (block as Anthropic.Messages.TextBlockParam).text?.length ?? 0 + } else if (type === "tool_result") { + total += readToolResultText(block)?.length ?? 0 + } else if (type === "tool_use") { + total += JSON.stringify((block as Anthropic.Messages.ToolUseBlockParam).input ?? {}).length + } else if (type === "image") { + // VS Code LM cannot send image data; convertToVsCodeLmMessages substitutes a textual + // placeholder, so charge that placeholder's real length rather than a token-sized guess. + total += IMAGE_PLACEHOLDER_CHARS + } + } + return total +} + +/** + * Shrinks oversized `tool_result` payloads (largest first, middle-out) until the conversation fits + * `budgetChars`. Mutates the tool_result blocks of the supplied messages in place — callers pass a + * cloned array (see `createMessage`) so stored history is never mutated. A no-op when the + * conversation already fits. `remainingChars` is the post-truncation cost, equal to + * `estimateMessagesChars(messages)`, so callers need not re-scan. + */ +export function truncateToolResultsToFitWindow( + messages: Anthropic.Messages.MessageParam[], + budgetChars: number, +): { messages: Anthropic.Messages.MessageParam[]; remainingChars: number } { + const initialTotal = () => messages.reduce((sum, message) => sum + estimateContentChars(message.content), 0) + if (!Number.isFinite(budgetChars) || budgetChars <= 0) { + return { messages, remainingChars: initialTotal() } + } + + let total = initialTotal() + if (total <= budgetChars) { + return { messages, remainingChars: total } + } + + // Collect every truncatable tool_result block, largest first. The text is derived once per + // block here because the comparator and the truncation loop below would otherwise re-derive it. + const blockText = new Map() + for (const message of messages) { + if (!Array.isArray(message.content)) { + continue + } + for (const block of message.content) { + const text = readToolResultText(block) + if (text !== undefined) { + blockText.set(block, text) + } + } + } + const toolResultBlocks = [...blockText.keys()] + toolResultBlocks.sort((a, b) => (blockText.get(b)?.length ?? 0) - (blockText.get(a)?.length ?? 0)) + + for (const block of toolResultBlocks) { + if (total <= budgetChars) { + break + } + const text = blockText.get(block) + if (text === undefined || text.length <= MIN_TOOL_RESULT_CHARS) { + continue + } + + const overage = total - budgetChars + const target = Math.max(MIN_TOOL_RESULT_CHARS, text.length - overage) + + const truncated = middleOutTruncate(text, target) + total -= text.length - truncated.length + writeToolResultText(block, truncated) + } + + return { messages, remainingChars: total } +} + /** * Converts one leaked parameter's raw text to the type its schema declares. * @@ -843,6 +1049,45 @@ export class VsCodeLmHandler extends BaseProvider implements SingleCompletionHan content: this.cleanMessageContent(msg.content), })) + // Keep context-window trimming on OUR side. Copilot's backend trims an over-window request + // without preserving tool_use/tool_result pairing, which orphans a tool_result and triggers a + // 400 ("unexpected tool_use_id"). See truncateToolResultsToFitWindow. + const contextWindowTokens = this.getCondenseContextWindow() + if (Number.isFinite(contextWindowTokens) && contextWindowTokens > 0) { + const toolSchemaChars = metadata?.tools ? JSON.stringify(metadata.tools).length : 0 + const rawBudgetChars = + contextWindowTokens * VSCODE_LM_INPUT_BUDGET_FRACTION * VSCODE_LM_BUDGET_CHARS_PER_TOKEN - + systemPrompt.length - + toolSchemaChars + // A system prompt or tool schema large enough to consume the whole budget would leave a + // non-positive budget, which disables trimming exactly when the request is most oversized. + const messagesBudgetChars = Math.max(MIN_TOOL_RESULT_CHARS, rawBudgetChars) + const { remainingChars } = truncateToolResultsToFitWindow(cleanedMessages, messagesBudgetChars) + + // Shrinking tool_results cannot always reach the budget: each keeps MIN_TOOL_RESULT_CHARS, + // and the excess may be non-tool content (a huge paste, tool_use inputs, or the system + // prompt) that we must not touch. Dropping messages here would orphan a tool_result from + // its tool_use — the exact 400 this guard exists to prevent — so fail loudly instead of + // sending a request we already know is over the window. + // Admission is judged against the RAW budget, not the clamped one: the clamp exists only + // to keep trimming productive, so accepting up to it would send a request the window + // genuinely cannot hold whenever the raw budget falls below MIN_TOOL_RESULT_CHARS. + if (remainingChars > rawBudgetChars) { + // `status` is what makes checkContextWindowExceededError recognise this as a context + // -window failure; without it the task takes its generic retry path and re-sends the + // same over-window history instead of condensing. + throw Object.assign( + new Error( + "Zoo Code : The request is too large for this model's context window " + + `(estimated ${remainingChars.toLocaleString("en-US")} characters against a budget of ` + + `${Math.max(0, Math.floor(rawBudgetChars)).toLocaleString("en-US")}), and it cannot be reduced further without ` + + "breaking tool-call pairing. Condense the conversation or start a new task.", + ), + { status: CONTEXT_WINDOW_EXCEEDED_STATUS }, + ) + } + } + // Convert Anthropic messages to VS Code LM messages const vsCodeLmMessages: vscode.LanguageModelChatMessage[] = [ vscode.LanguageModelChatMessage.Assistant(sanitizeSurrogates(systemPrompt)), diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 10a4446182..122579c2a0 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -61,7 +61,17 @@ export class NativeToolCallParser { // Raw chunk tracking state (keyed by index from one API stream) private static rawChunkTrackersByScope = new WeakMap< object, - Map + Map< + number, + { + id?: string + name: string + // Track whether the provider sent a name, including an empty name. + nameSeen: boolean + hasStarted: boolean + deltaBuffer: string[] + } + > >() public static createScope(): object { @@ -124,49 +134,54 @@ export class NativeToolCallParser { let tracked = rawChunkTracker.get(index) - // Initialize new tool call tracking when we receive an id - if (id && !tracked) { + // Create the tracker on first sight of this index, independent of whether + // an id has arrived yet. Keying the lifecycle by index (not id) ensures any + // `arguments` that stream before the id is known are buffered rather than dropped. + if (!tracked) { tracked = { id, name: name || "", + nameSeen: name !== undefined, hasStarted: false, deltaBuffer: [], } rawChunkTracker.set(index, tracked) } - if (!tracked) { - return events + // Record id and name as they arrive (they may come in separate chunks). + if (id) { + tracked.id = id } - - // Update name if present in chunk and not yet set - if (name) { + if (name !== undefined) { tracked.name = name + tracked.nameSeen = true } - // Emit start event when we have the name - if (!tracked.hasStarted && tracked.name) { + // Emit start event only once both id and name are known. Using a local + // non-null id keeps emitted events typed as id: string. + if (!tracked.hasStarted && tracked.id && tracked.nameSeen) { + const startedId = tracked.id events.push({ type: "tool_call_start", - id: tracked.id, + id: startedId, name: tracked.name, }) tracked.hasStarted = true - // Flush buffered deltas + // Flush buffered deltas accumulated during the pre-start window. for (const bufferedDelta of tracked.deltaBuffer) { events.push({ type: "tool_call_delta", - id: tracked.id, + id: startedId, delta: bufferedDelta, }) } tracked.deltaBuffer = [] } - // Emit delta event for argument chunks + // Emit delta event for argument chunks, buffering until start is emitted. if (args) { - if (tracked.hasStarted) { + if (tracked.hasStarted && tracked.id) { events.push({ type: "tool_call_delta", id: tracked.id, @@ -190,7 +205,7 @@ export class NativeToolCallParser { if (rawChunkTracker) { for (const [, tracked] of rawChunkTracker.entries()) { - if (tracked.hasStarted) { + if (tracked.hasStarted && tracked.id) { events.push({ type: "tool_call_end", id: tracked.id, diff --git a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts index 47ea2a36cc..ee79d7963f 100644 --- a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts +++ b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts @@ -1,4 +1,4 @@ -import { NativeToolCallParser } from "../NativeToolCallParser" +import { NativeToolCallParser, type ToolCallStreamEvent } from "../NativeToolCallParser" describe("NativeToolCallParser", () => { describe("parseToolCall", () => { @@ -289,6 +289,69 @@ describe("NativeToolCallParser", () => { }) describe("processStreamingChunk", () => { + it("preserves read_file arguments that arrive before the call id and name", () => { + const scope = NativeToolCallParser.createScope() + const argumentsJson = JSON.stringify({ path: "src/leading.ts", mode: "slice", offset: 1, limit: 2000 }) + const split = 24 + + expect( + NativeToolCallParser.processRawChunk({ index: 0, arguments: argumentsJson.slice(0, split) }, scope), + ).toEqual([]) + + const identifiedEvents = NativeToolCallParser.processRawChunk( + { index: 0, id: "call_late_identity", name: "read_file" }, + scope, + ) + expect(identifiedEvents).toEqual([ + { type: "tool_call_start", id: "call_late_identity", name: "read_file" }, + { + type: "tool_call_delta", + id: "call_late_identity", + delta: argumentsJson.slice(0, split), + }, + ]) + + NativeToolCallParser.startStreamingToolCall("call_late_identity", "read_file", scope) + for (const event of identifiedEvents) { + if (event.type === "tool_call_delta") { + NativeToolCallParser.processStreamingChunk(event.id, event.delta, scope) + } + } + + const trailingEvents = NativeToolCallParser.processRawChunk( + { index: 0, arguments: argumentsJson.slice(split) }, + scope, + ) + expect(trailingEvents).toEqual([ + { + type: "tool_call_delta", + id: "call_late_identity", + delta: argumentsJson.slice(split), + }, + ]) + for (const event of trailingEvents) { + if (event.type === "tool_call_delta") { + NativeToolCallParser.processStreamingChunk(event.id, event.delta, scope) + } + } + + expect(NativeToolCallParser.finalizeRawChunks(scope)).toEqual([ + { type: "tool_call_end", id: "call_late_identity" }, + ]) + const result = NativeToolCallParser.finalizeStreamingToolCall("call_late_identity", scope) + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + expect(result.nativeArgs).toEqual({ path: "src/leading.ts", mode: "slice", offset: 1, limit: 2000 }) + } + }) + + it("does not emit an end event for argument chunks that never receive an identity", () => { + const scope = NativeToolCallParser.createScope() + NativeToolCallParser.processRawChunk({ index: 0, arguments: '{"path":"orphan.ts"}' }, scope) + + expect(NativeToolCallParser.finalizeRawChunks(scope)).toEqual([]) + }) + it("retains peer calls until each call in a scope is finalized", () => { const scope = NativeToolCallParser.createScope() NativeToolCallParser.startStreamingToolCall("call_first", "read_file", scope) @@ -391,11 +454,14 @@ describe("NativeToolCallParser", () => { expect(NativeToolCallParser.finalizeRawChunks(firstScope)).toEqual([]) expect(NativeToolCallParser.finalizeStreamingToolCall("call_first", firstScope)).toBeNull() expect( - NativeToolCallParser.processRawChunk({ index: 0, arguments: "ignored-after-cleanup" }, firstScope), + NativeToolCallParser.processRawChunk({ index: 0, arguments: "buffered-after-cleanup" }, firstScope), ).toEqual([]) expect( NativeToolCallParser.processRawChunk({ index: 0, id: "call_reprobe", name: "read_file" }, firstScope), - ).toEqual([{ type: "tool_call_start", id: "call_reprobe", name: "read_file" }]) + ).toEqual([ + { type: "tool_call_start", id: "call_reprobe", name: "read_file" }, + { type: "tool_call_delta", id: "call_reprobe", delta: "buffered-after-cleanup" }, + ]) }) describe("read_file tool", () => { @@ -450,4 +516,255 @@ describe("NativeToolCallParser", () => { }) }) }) + + describe("processRawChunk streaming reassembly", () => { + // Mirror the sequencing Task.ts performs: feed each raw chunk through + // processRawChunk, drive startStreamingToolCall on tool_call_start, feed + // tool_call_delta into processStreamingChunk, and emit ends at stream close + // via finalizeRawChunks() (the same call Task.ts makes after the stream ends). + // Returns the ordered event types/ids plus the finalized tool uses by id. + const drive = (rawChunks: Array<{ index: number; id?: string; name?: string; arguments?: string }>) => { + const events: ToolCallStreamEvent[] = [] + const scope = NativeToolCallParser.createScope() + + const handleEvent = (event: ToolCallStreamEvent) => { + events.push(event) + if (event.type === "tool_call_start") { + NativeToolCallParser.startStreamingToolCall(event.id, event.name, scope) + } else if (event.type === "tool_call_delta") { + NativeToolCallParser.processStreamingChunk(event.id, event.delta, scope) + } + } + + for (const chunk of rawChunks) { + for (const event of NativeToolCallParser.processRawChunk(chunk, scope)) { + handleEvent(event) + } + } + + // Task.ts finalizes any tool calls still open at stream end via + // finalizeRawChunks(), which emits the tool_call_end events. + for (const event of NativeToolCallParser.finalizeRawChunks(scope)) { + handleEvent(event) + } + + const finalized = new Map>() + const startIds = events.filter((e) => e.type === "tool_call_start").map((e) => e.id) + for (const id of startIds) { + finalized.set(id, NativeToolCallParser.finalizeStreamingToolCall(id, scope)) + } + + return { events, finalized } + } + + it("preserves leading argument bytes that arrive before the id", () => { + // First chunk carries arguments but NO id; id+name arrive later, then more args. + const fullArgs = JSON.stringify({ path: "src/leading.ts", mode: "slice" }) + const firstHalf = fullArgs.slice(0, 10) + const secondHalf = fullArgs.slice(10) + + const { events, finalized } = drive([ + { index: 0, arguments: firstHalf }, + { index: 0, id: "call_late_id", name: "read_file" }, + { index: 0, arguments: secondHalf }, + ]) + + // Exactly one start, in the right order, with the late id. + const starts = events.filter((e) => e.type === "tool_call_start") + expect(starts).toHaveLength(1) + expect(starts[0].id).toBe("call_late_id") + + // The finalized arguments must contain the complete, uncorrupted payload. + const result = finalized.get("call_late_id") + expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + const nativeArgs = result.nativeArgs as { path: string; mode?: string } + expect(nativeArgs.path).toBe("src/leading.ts") + expect(nativeArgs.mode).toBe("slice") + } + }) + + it("handles id and name arriving in separate chunks (issue #218)", () => { + const fullArgs = JSON.stringify({ path: "src/split.ts" }) + + const { events, finalized } = drive([ + { index: 0, id: "call_split" }, + { index: 0, name: "read_file" }, + { index: 0, arguments: fullArgs }, + ]) + + const starts = events.filter((e) => e.type === "tool_call_start") + expect(starts).toHaveLength(1) + expect(starts[0].id).toBe("call_split") + + const result = finalized.get("call_split") + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + const nativeArgs = result.nativeArgs as { path: string } + expect(nativeArgs.path).toBe("src/split.ts") + } + }) + + it("handles name arriving before id with buffered args in between (reverse ordering)", () => { + const fullArgs = JSON.stringify({ path: "src/reverse.ts" }) + const firstHalf = fullArgs.slice(0, 9) + const secondHalf = fullArgs.slice(9) + + const { events, finalized } = drive([ + { index: 0, name: "read_file" }, + { index: 0, arguments: firstHalf }, + { index: 0, id: "call_reverse" }, + { index: 0, arguments: secondHalf }, + ]) + + // Start must not fire until the id arrives, so exactly one start with the late id. + const starts = events.filter((e) => e.type === "tool_call_start") + expect(starts).toHaveLength(1) + expect(starts[0].id).toBe("call_reverse") + + // The buffered delta must be flushed only after the start event. + const startIndex = events.findIndex((e) => e.type === "tool_call_start") + const firstDeltaIndex = events.findIndex((e) => e.type === "tool_call_delta") + expect(startIndex).toBeLessThan(firstDeltaIndex) + + const result = finalized.get("call_reverse") + expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + expect((result.nativeArgs as { path: string }).path).toBe("src/reverse.ts") + } + }) + + it("keeps two parallel tool calls on distinct indices isolated", () => { + const argsA = JSON.stringify({ path: "src/a.ts" }) + const argsB = JSON.stringify({ path: "src/b.ts" }) + + const { events, finalized } = drive([ + { index: 0, arguments: argsA.slice(0, 8) }, + { index: 1, arguments: argsB.slice(0, 8) }, + { index: 0, id: "call_a", name: "read_file" }, + { index: 1, id: "call_b", name: "read_file" }, + { index: 0, arguments: argsA.slice(8) }, + { index: 1, arguments: argsB.slice(8) }, + ]) + + const starts = events.filter((e) => e.type === "tool_call_start") + expect(starts).toHaveLength(2) + + const resultA = finalized.get("call_a") + const resultB = finalized.get("call_b") + expect(resultA).not.toBeNull() + expect(resultB).not.toBeNull() + if (resultA?.type === "tool_use") { + expect((resultA.nativeArgs as { path: string }).path).toBe("src/a.ts") + } + if (resultB?.type === "tool_use") { + expect((resultB.nativeArgs as { path: string }).path).toBe("src/b.ts") + } + }) + + it("emits the same event sequence for the single-chunk-with-id flow (regression guard)", () => { + const fullArgs = JSON.stringify({ path: "src/single.ts" }) + + const { events, finalized } = drive([ + { index: 0, id: "call_single", name: "read_file", arguments: fullArgs }, + ]) + + expect(events.map((e) => e.type)).toEqual(["tool_call_start", "tool_call_delta", "tool_call_end"]) + expect(events.every((e) => e.id === "call_single")).toBe(true) + + const result = finalized.get("call_single") + expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + expect((result.nativeArgs as { path: string }).path).toBe("src/single.ts") + } + }) + + it("does not emit a phantom tool_call_end for a tracker that never received an id", () => { + const { events } = drive([{ index: 0, arguments: '{"path":"orphan.ts"}' }]) + + expect(events.filter((e) => e.type === "tool_call_start")).toHaveLength(0) + expect(events.filter((e) => e.type === "tool_call_end")).toHaveLength(0) + }) + + it("finalizeRawChunks() emits end events and guards against missing id", () => { + const scope = NativeToolCallParser.createScope() + // Simulate a started tool call: process chunks to populate state + const chunks = [ + { index: 0, id: "call_finalize", name: "read_file" }, + { index: 0, arguments: '{"path":"file.ts"' }, + { index: 0, arguments: ',"mode":"slice"}' }, + ] + + const events: Array<{ type: string; id?: string }> = [] + for (const chunk of chunks) { + for (const event of NativeToolCallParser.processRawChunk(chunk, scope)) { + events.push(event) + if (event.type === "tool_call_start") { + NativeToolCallParser.startStreamingToolCall(event.id, event.name, scope) + } else if (event.type === "tool_call_delta") { + NativeToolCallParser.processStreamingChunk(event.id, event.delta, scope) + } + } + } + + // Now finalize the raw chunks to emit the end event + const finalizeEvents = NativeToolCallParser.finalizeRawChunks(scope) + for (const event of finalizeEvents) { + events.push(event) + } + + // Verify the end event was produced by finalizeRawChunks + const ends = events.filter((e) => e.type === "tool_call_end") + expect(ends).toHaveLength(1) + expect(ends[0].id).toBe("call_finalize") + + // Finalize the tool call to ensure it contains the complete arguments + const result = NativeToolCallParser.finalizeStreamingToolCall("call_finalize", scope) + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + expect((result.nativeArgs as { path: string }).path).toBe("file.ts") + } + }) + + it("finalizeRawChunks() does not emit end for tracker without id", () => { + const scope = NativeToolCallParser.createScope() + // Start a tracker with arguments but no id, then finalize + const chunks = [{ index: 0, arguments: '{"incomplete":true}' }] + + for (const chunk of chunks) { + NativeToolCallParser.processRawChunk(chunk, scope) + } + + // Finalize should not emit an end event if id was never set + const finalizeEvents = NativeToolCallParser.finalizeRawChunks(scope) + const ends = finalizeEvents.filter((e) => e.type === "tool_call_end") + expect(ends).toHaveLength(0) + + NativeToolCallParser.clearRawChunkState(scope) + }) + + it("does not double-fire end events across repeated finalizeRawChunks calls", () => { + const scope = NativeToolCallParser.createScope() + // Drive a started tool call through the raw chunk path. + const chunks = [ + { index: 0, id: "call_dup", name: "read_file" }, + { index: 0, arguments: '{"path":"file.ts"}' }, + ] + for (const chunk of chunks) { + NativeToolCallParser.processRawChunk(chunk, scope) + } + + const finishEvents = NativeToolCallParser.finalizeRawChunks(scope) + const finalizeEvents = NativeToolCallParser.finalizeRawChunks(scope) + + const allEnds = [...finishEvents, ...finalizeEvents].filter((e) => e.type === "tool_call_end") + expect(allEnds).toHaveLength(1) + expect(allEnds[0].id).toBe("call_dup") + + NativeToolCallParser.clearRawChunkState(scope) + }) + }) }) diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts index 3247ba49d2..7381b2c2d0 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts @@ -348,6 +348,309 @@ describe("presentAssistantMessage - tool usage attribution", () => { expect(mockTask.recordToolUsage).not.toHaveBeenCalled() expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled() }) + + it("rejects at dispatch when use_mcp_tool is in disabledTools, without invoking the server method", async () => { + const mockHub = { + findServerNameBySanitizedName: () => "my_server", + getAllServers: () => [ + { + name: "my_server", + tools: [{ name: "do_thing", enabledForPrompt: true }], + }, + ], + callTool: vi.fn(), + } + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + disabledTools: ["use_mcp_tool"], + }), + getMcpHub: () => mockHub, + }), + } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_native_mcp_disabled", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + // The decision must come from the shared layer, under the canonical name, + // with the disabledTools entry folded into the requirements map. + const calls = vi.mocked(validateToolUse).mock.calls + expect(calls.length).toBeGreaterThan(0) + expect(calls[calls.length - 1][0]).toBe("use_mcp_tool") + expect(calls[calls.length - 1][3]).toMatchObject({ use_mcp_tool: false }) + + // Exactly one error tool_result for the block's id; the handler is never + // reached, so the server method is never invoked. + expect(mockTask.userMessageContent).toHaveLength(1) + expect(mockTask.userMessageContent).toContainEqual( + expect.objectContaining({ + type: "tool_result", + tool_use_id: "call_native_mcp_disabled", + is_error: true, + content: expect.stringContaining("not allowed in code mode"), + }), + ) + expect(mockHub.callTool).not.toHaveBeenCalled() + + // No success attribution on rejection; a safe static failure key instead. + expect(mockTask.recordToolUsage).not.toHaveBeenCalled() + expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled() + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool", expect.any(String)) + expect(mockTask.consecutiveMistakeCount).toBe(1) + // The stream must not be interrupted by a validation rejection. + expect(mockTask.didAlreadyUseTool).toBe(false) + }) + + it("rejects at dispatch when use_mcp_tool is in the model's excludedTools", async () => { + const mockHub = { + findServerNameBySanitizedName: () => "my_server", + getAllServers: () => [ + { + name: "my_server", + tools: [{ name: "do_thing", enabledForPrompt: true }], + }, + ], + callTool: vi.fn(), + } + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + getMcpHub: () => mockHub, + }), + } + // excludedTools reaches the requirements map only through + // buildToolRequirements' modelInfo fold, so this pins that input on the + // native path. + mockTask.api = { + getModel: () => ({ id: "test-model", info: { excludedTools: ["use_mcp_tool"] } }), + } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_native_mcp_excluded", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + const calls = vi.mocked(validateToolUse).mock.calls + expect(calls.length).toBeGreaterThan(0) + expect(calls[calls.length - 1][0]).toBe("use_mcp_tool") + expect(calls[calls.length - 1][3]).toMatchObject({ use_mcp_tool: false }) + + expect(mockTask.userMessageContent).toHaveLength(1) + expect(mockTask.userMessageContent).toContainEqual( + expect.objectContaining({ + type: "tool_result", + tool_use_id: "call_native_mcp_excluded", + is_error: true, + content: expect.stringContaining("not allowed in code mode"), + }), + ) + expect(mockHub.callTool).not.toHaveBeenCalled() + + expect(mockTask.recordToolUsage).not.toHaveBeenCalled() + expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled() + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool", expect.any(String)) + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.didAlreadyUseTool).toBe(false) + }) + + it("skips shared-layer validation for partial blocks so streaming never pushes error results", async () => { + // Validating partial blocks would surface one validation error tool_result + // per streamed chunk for the same tool_use_id. + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + getMcpHub: () => ({ + findServerNameBySanitizedName: () => "my_server", + }), + }), + } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_native_mcp_partial", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: true, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + expect(validateToolUse).not.toHaveBeenCalled() + expect(mockTask.userMessageContent).toHaveLength(0) + expect(mockTask.consecutiveMistakeCount).toBe(0) + }) + + it("rejects through the state ?? {} fallback when the provider is unavailable", async () => { + // With no provider state the validator must still receive the task-local + // mode plus empty policy inputs: customModes [] and an empty requirements + // map, rather than the dispatch crashing. + mockTask.providerRef = { deref: () => undefined } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_native_mcp_no_provider", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + // The validator throws so the handler is never reached — the assertions + // pin the arguments the call site passed under the empty-state fallback. + const calls = vi.mocked(validateToolUse).mock.calls + expect(calls.length).toBeGreaterThan(0) + expect(calls[calls.length - 1][0]).toBe("use_mcp_tool") + expect(calls[calls.length - 1][1]).toBe("code") + expect(calls[calls.length - 1][2]).toEqual([]) + expect(calls[calls.length - 1][3]).toEqual({}) + // The default model info carries no includedTools. + expect(calls[calls.length - 1][6]).toBeUndefined() + + expect(mockTask.userMessageContent).toHaveLength(1) + expect(mockTask.userMessageContent).toContainEqual( + expect.objectContaining({ + type: "tool_result", + tool_use_id: "call_native_mcp_no_provider", + is_error: true, + content: expect.stringContaining("not allowed in code mode"), + }), + ) + + expect(mockTask.recordToolUsage).not.toHaveBeenCalled() + expect(TelemetryService.instance.captureToolUsage).not.toHaveBeenCalled() + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool", expect.any(String)) + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.didAlreadyUseTool).toBe(false) + }) + + it("resolves aliases in the model's includedTools before passing them to the shared layer", async () => { + // A model may advertise tools under alias names; the shared layer compares + // canonical names, so "write_file" must arrive as "write_to_file". + mockTask.api = { + getModel: () => ({ id: "test-model", info: { includedTools: ["write_file"] } }), + } + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + getMcpHub: () => ({ + findServerNameBySanitizedName: () => "my_server", + }), + }), + } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_native_mcp_alias", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + const calls = vi.mocked(validateToolUse).mock.calls + expect(calls.length).toBeGreaterThan(0) + expect(calls[calls.length - 1][0]).toBe("use_mcp_tool") + expect(calls[calls.length - 1][6]).toEqual(["write_to_file"]) + }) + + it("pushes no tool_result when the native block omits its id, but still records the rejection", async () => { + // McpToolUse.id is optional; with no tool_use_id there is nothing to + // answer, so the error tool_result is skipped while the rejection is + // still recorded. + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "code", + customModes: [], + }), + getMcpHub: () => ({ + findServerNameBySanitizedName: () => "my_server", + }), + }), + } + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error('Tool "use_mcp_tool" is not allowed in code mode.') + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + expect(mockTask.pushToolResultToUserContent).not.toHaveBeenCalled() + expect(mockTask.userMessageContent).toHaveLength(0) + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool", expect.any(String)) + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.didAlreadyUseTool).toBe(false) + }) }) describe("undefined provider state", () => { @@ -415,5 +718,49 @@ describe("presentAssistantMessage - tool usage attribution", () => { expect(calls[0][0]).toBe("read_file") expect(calls[0][1]).toBe("architect") }) + + it("passes the task-local mode to validateToolUse for a native mcp_tool_use block", async () => { + // The native mcp arm has its own validateToolUse call site and must honor + // the same task-local mode contract as the tool_use arm: provider state + // deliberately reports a different mode than the task's. + mockTask.providerRef = { + deref: () => ({ + getState: vi.fn().mockResolvedValue({ + mode: "orchestrator", + customModes: [], + }), + getMcpHub: () => ({ + findServerNameBySanitizedName: () => "my_server", + }), + }), + } + mockTask.getTaskMode = vi.fn().mockResolvedValue("architect") + // Throw so the handler is never reached — the assertion is on the + // arguments the call site passed to the shared layer. + vi.mocked(validateToolUse).mockImplementationOnce(() => { + throw new Error("Task-local mode check.") + }) + + mockTask.assistantMessageContent = [ + { + type: "mcp_tool_use", + id: "call_delegation_mcp", + name: "mcp_my_server_do_thing", + serverName: "my_server", + toolName: "do_thing", + arguments: {}, + partial: false, + }, + ] + + await presentAssistantMessage(mockTask as unknown as Task) + + const calls = vi.mocked(validateToolUse).mock.calls + expect(calls.length).toBeGreaterThan(0) + expect(calls[calls.length - 1][0]).toBe("use_mcp_tool") + // The discriminating assertion: task-local "architect", not provider + // "orchestrator" — the exact pre-delegation bug shape. + expect(calls[calls.length - 1][1]).toBe("architect") + }) }) }) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 546c43c06c..610e034ab6 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -290,6 +290,67 @@ export async function presentAssistantMessage(cline: Task) { }, } + // Consult the shared policy layer before executing — ONLY for complete + // (non-partial) blocks. Validating partial blocks would surface validation + // errors repeatedly during streaming, pushing multiple tool_results for the + // same tool_use_id and making the stream appear frozen. + if (!mcpBlock.partial) { + const state = await cline.providerRef.deref()?.getState() + const { customModes, experiments: stateExperiments, disabledTools } = state ?? {} + // Read the task-local mode, not the shared provider mode. + // A delegated child task may run in a different mode than its parent. + const taskMode = await cline.getTaskMode() + + const modelInfo = cline.api.getModel() + // Resolve aliases in includedTools before validation + // e.g., "write_file" should resolve to "write_to_file" + const rawIncludedTools = modelInfo.info.includedTools + const { resolveToolAlias } = await import("../prompts/tools/filter-tools-for-mode") + const includedTools = rawIncludedTools?.map((tool) => resolveToolAlias(tool)) + + try { + // Validate under the canonical "use_mcp_tool" name: the requirements map + // is keyed by canonical/alias names, so the dynamic mcp_* name would never + // key-match a disabledTools or excludedTools entry naming use_mcp_tool. + // Build requirements through the shared policy module so every suppressed + // entry reaches the validator, which checks them before the + // always-available class. See `buildToolRequirements` in + // effective-tool-policy.ts. + const toolRequirements = buildToolRequirements(disabledTools, modelInfo.info) + + validateToolUse( + "use_mcp_tool", + taskMode, + customModes ?? [], + toolRequirements, + syntheticToolUse.params, + stateExperiments, + includedTools, + ) + } catch (error) { + cline.consecutiveMistakeCount++ + // For validation errors, send a tool_result with the error (required for + // native tool calling) but do NOT set didAlreadyUseTool = true — the tool + // was never executed, and interrupting the stream here would make the + // extension appear to hang. + const errorContent = formatResponse.toolError(error.message) + if (toolCallId) { + cline.pushToolResultToUserContent({ + type: "tool_result", + tool_use_id: sanitizeToolUseId(toolCallId), + content: errorContent, + is_error: true, + }) + } + + // The validator's message names the canonical tool, never the + // model-controlled dynamic name, so this key is safe. + cline.recordToolError("use_mcp_tool", error.message) + + break + } + } + await useMcpToolTool.handle(cline, syntheticToolUse, { askApproval, handleError, diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 3fcc0e6e43..19fae0e38c 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -504,9 +504,9 @@ export class ProviderSettingsManager { } /** - * Set the API config for a specific mode. + * Set the API config for a specific mode, or remove its mapping with undefined. */ - public async setModeConfig(mode: Mode, configId: string) { + public async setModeConfig(mode: Mode, configId: string | undefined) { try { return await this.lock(async () => { const providerProfiles = await this.load() @@ -514,8 +514,11 @@ export class ProviderSettingsManager { if (!providerProfiles.modeApiConfigs) { providerProfiles.modeApiConfigs = {} } - // Assign the chosen config ID to this mode - providerProfiles.modeApiConfigs[mode] = configId + if (configId === undefined) { + delete providerProfiles.modeApiConfigs[mode] + } else { + providerProfiles.modeApiConfigs[mode] = configId + } await this.store(providerProfiles) }) } catch (error) { diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index 13e1aeeb2d..8f87573e2f 100644 --- a/src/core/config/__tests__/ProviderSettingsManager.spec.ts +++ b/src/core/config/__tests__/ProviderSettingsManager.spec.ts @@ -350,6 +350,26 @@ describe("ProviderSettingsManager", () => { }) }) + describe("setModeConfig", () => { + it("persists removal of one mapping while preserving other modes", async () => { + let stored = JSON.stringify({ + currentApiConfigName: "default", + apiConfigs: { default: { id: "default" } }, + modeApiConfigs: { code: "new-profile", ask: "default" }, + }) + mockSecrets.get.mockImplementation(async () => stored) + mockSecrets.store.mockImplementation(async (_key: string, value: string) => { + stored = value + }) + await providerSettingsManager.setModeConfig("code", undefined) + expect(JSON.parse(stored).modeApiConfigs).toEqual({ ask: "default" }) + expect(await providerSettingsManager.getModeConfigId("code")).toBeUndefined() + expect(await providerSettingsManager.getModeConfigId("ask")).toBe("default") + await providerSettingsManager.setModeConfig("code", "default") + expect(await providerSettingsManager.getModeConfigId("code")).toBe("default") + }) + }) + describe("ListConfig", () => { it("should list all available configs", async () => { const existingConfig: ProviderProfiles = { diff --git a/src/core/context/context-management/context-error-handling.ts b/src/core/context/context-management/context-error-handling.ts index 6cfe993f95..b7ed0cad16 100644 --- a/src/core/context/context-management/context-error-handling.ts +++ b/src/core/context/context-management/context-error-handling.ts @@ -1,5 +1,11 @@ import { APIError } from "openai" +/** + * Status a provider must attach to an error for it to be routed to the condense path. Exported so + * providers raising a context-window failure locally stay coupled to the predicate below. + */ +export const CONTEXT_WINDOW_EXCEEDED_STATUS = 400 + export function checkContextWindowExceededError(error: unknown): boolean { return ( checkIsOpenAIContextWindowError(error) || @@ -27,7 +33,10 @@ function checkIsOpenRouterContextWindowError(error: unknown): boolean { /\btoo\s*many\s*tokens?\b/i, ] as const - return String(status) === "400" && CONTEXT_ERROR_PATTERNS.some((pattern) => pattern.test(message)) + return ( + String(status) === String(CONTEXT_WINDOW_EXCEEDED_STATUS) && + CONTEXT_ERROR_PATTERNS.some((pattern) => pattern.test(message)) + ) } catch { return false } @@ -46,7 +55,7 @@ function checkIsOpenAIContextWindowError(error: unknown): boolean { return ( Boolean(error) && error instanceof APIError && - error.code?.toString() === "400" && + error.code?.toString() === String(CONTEXT_WINDOW_EXCEEDED_STATUS) && KNOWN_CONTEXT_ERROR_SUBSTRINGS.some((substring) => error.message.includes(substring)) ) } catch { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 88bd8141b8..0362252174 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1292,8 +1292,19 @@ export class Task extends EventEmitter implements TaskLike { this.clineMessages.push(message) const provider = this.providerRef.deref() // Unanswered asks must reach the webview before Message listeners can respond against its state. - const requiresImmediateState = - message.partial === true || (message.type === "ask" && message.isAnswered !== true) + // + // Partial `say` messages deliberately do NOT flush. `flushPostStateToWebviewThrottled()` + // invoked right after a leading-edge debounce call has no pending trailing invocation to + // run, so it only cancels the trailing timer — which makes the *next* call hit the leading + // edge and post immediately. Flushing on every partial therefore defeated the debounce + // entirely (one full-state post per message, the very behaviour #1078 set out to remove). + // They are safe on the throttled path: the trailing/maxWait post carries the message's + // current text, so a `messageUpdated` dropped for a not-yet-known `ts` is superseded + // rather than lost. + // + // Partial *asks* still flush, via the clause below — `Task#ask` adds them without + // `isAnswered`, so they keep the ordering guarantee that unanswered asks depend on. + const requiresImmediateState = message.type === "ask" && message.isAnswered !== true try { await provider?.postStateToWebviewThrottled() } catch (error) { @@ -3236,6 +3247,7 @@ export class Task extends EventEmitter implements TaskLike { this.didRejectTool = false this.didAlreadyUseTool = false this.assistantMessageSavedToHistory = false + this.didFinishAbortingStream = false this.resetAssistantMessagePersistence() // Reset tool failure flag for each new assistant turn - this ensures that tool failures // only prevent attempt_completion within the same assistant message, not across turns @@ -3715,8 +3727,8 @@ export class Task extends EventEmitter implements TaskLike { await abortStream(cancelReason, streamingFailedMessage) if (this.abort) { - // User cancelled - abort the entire task - this.abortReason = cancelReason + // ??= keeps the first reason; a cancel can land during abortStream after cancelReason was already computed. + this.abortReason ??= "user_cancelled" await this.abortTask() } else if (error instanceof OutputTokenLimitError) { // Truncation repeats on an identical request, so never auto-retry it @@ -3751,8 +3763,7 @@ export class Task extends EventEmitter implements TaskLike { console.log( `[Task#${this.taskId}.${this.instanceId}] Task aborted during mid-stream retry backoff`, ) - // Abort the entire task - this.abortReason = "user_cancelled" + this.abortReason ??= "user_cancelled" await this.abortTask() break } diff --git a/src/core/task/__tests__/Task.abort-reason-race.spec.ts b/src/core/task/__tests__/Task.abort-reason-race.spec.ts new file mode 100644 index 0000000000..7b71ee1dd6 --- /dev/null +++ b/src/core/task/__tests__/Task.abort-reason-race.spec.ts @@ -0,0 +1,216 @@ +// npx vitest run core/task/__tests__/Task.abort-reason-race.spec.ts + +import * as os from "os" +import * as path from "path" + +import type { GlobalState, ProviderSettings } from "@roo-code/types" +import { providerIdentifiers } from "@roo-code/types/provider-identifiers" +import { findLast } from "../../../shared/array" +import { TelemetryService } from "@roo-code/telemetry" + +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" + +const { mockSaveTaskMessages, mockSaveApiMessages } = vi.hoisted(() => ({ + mockSaveTaskMessages: vi.fn().mockResolvedValue(undefined), + mockSaveApiMessages: vi.fn().mockResolvedValue(undefined), +})) + +// vscode is globally aliased to __mocks__/vscode.js by vitest.config.ts. +vi.mock("delay", () => ({ __esModule: true, default: vi.fn().mockResolvedValue(undefined) })) +vi.mock("execa", () => ({ execa: vi.fn() })) +vi.mock("p-wait-for", () => ({ default: vi.fn().mockResolvedValue(undefined) })) + +vi.mock("../../task-persistence", async (importOriginal) => { + const mod = await importOriginal() + return { + ...mod, + saveApiMessages: mockSaveApiMessages, + saveTaskMessages: mockSaveTaskMessages, + TaskHistoryStore: vi.fn().mockImplementation(function () { + return { + initialize: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn(), + get: vi.fn(), + getAll: vi.fn().mockReturnValue([]), + upsert: vi.fn().mockResolvedValue([]), + delete: vi.fn().mockResolvedValue(undefined), + deleteMany: vi.fn().mockResolvedValue(undefined), + reconcile: vi.fn().mockResolvedValue(undefined), + initialized: Promise.resolve(), + } + }), + } +}) + +vi.mock("../../mentions", () => ({ + parseMentions: vi + .fn() + .mockImplementation((text) => + Promise.resolve({ text: `processed: ${text}`, mode: undefined, contentBlocks: [] }), + ), + openMention: vi.fn(), + getLatestTerminalOutput: vi.fn(), +})) + +vi.mock("../../mentions/processUserContentMentions", () => ({ + processUserContentMentions: vi.fn().mockImplementation(async ({ userContent }: { userContent: unknown[] }) => ({ + content: userContent, + mode: undefined, + })), +})) + +vi.mock("../../../integrations/misc/extract-text", () => ({ + extractTextFromFile: vi.fn().mockResolvedValue(""), +})) + +vi.mock("../../environment/getEnvironmentDetails", () => ({ + getEnvironmentDetails: vi.fn().mockResolvedValue(""), +})) + +vi.mock("../../ignore/RooIgnoreController") + +vi.mock("../../../utils/storage", () => ({ + getTaskDirectoryPath: vi + .fn() + .mockImplementation((base: string, id: string) => Promise.resolve(`${base}/tasks/${id}`)), + getSettingsDirectoryPath: vi.fn().mockImplementation((base: string) => Promise.resolve(`${base}/settings`)), +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockReturnValue(false), +})) + +vi.mock("../../../i18n", () => ({ + t: (key: string) => key, +})) + +function makeMockProvider() { + return { + log: vi.fn(), + taskHistoryStore: { get: () => undefined }, + updateTaskHistory: vi.fn().mockResolvedValue([]), + getState: vi.fn().mockResolvedValue({}), + getSkillsManager: vi.fn().mockReturnValue(undefined), + postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), + flushPostStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined), + context: { + globalStorageUri: { fsPath: path.join(os.tmpdir(), "test-storage-abort-race") }, + globalState: { + get: vi.fn().mockImplementation((_key: keyof GlobalState) => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + workspaceState: { + get: vi.fn().mockImplementation(() => undefined), + update: vi.fn().mockResolvedValue(undefined), + keys: vi.fn().mockReturnValue([]), + }, + secrets: { + get: vi.fn().mockResolvedValue(undefined), + store: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }, + extensionUri: { fsPath: "/mock/extension" }, + extension: { packageJSON: { version: "1.0.0" } }, + }, + } +} + +describe("Task abort-reason race", () => { + let mockApiConfig: ProviderSettings + + beforeEach(() => { + vi.clearAllMocks() + mockSaveTaskMessages.mockResolvedValue(undefined) + + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + mockApiConfig = { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-api-key", + } + }) + + it("keeps user_cancelled when a cancel lands during abortStream cleanup", async () => { + const mockProvider = makeMockProvider() + + // Double cast: plain object satisfies only the methods called by this code path. + const task = new Task({ + provider: mockProvider as unknown as ClineProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) + const abortTaskSpy = vi.spyOn(task, "abortTask") + + // eslint-disable-next-line require-yield -- intentional error-only async source + task["attemptApiRequest"] = async function* () { + throw new Error("mid-stream API failure") + } + + // abortStream calls updateApiReqMsg("streaming_failed") synchronously before saving, + // so the api_req_started row has cancelReason="streaming_failed" when we are inside it. + mockSaveTaskMessages.mockImplementation(async () => { + const apiReqMsg = findLast(task.clineMessages, (m) => m.say === "api_req_started") + const parsed = apiReqMsg?.text ? (JSON.parse(apiReqMsg.text) as { cancelReason?: string }) : {} + if (parsed.cancelReason === "streaming_failed" && !task.didFinishAbortingStream) { + // cancelTask sets abortReason before abort=true. + task["abortReason"] = "user_cancelled" + task["abort"] = true + } + }) + + // The outer catch in recursivelyMakeClineRequests swallows the abort throw and returns true. + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }]) + expect(result).toBe(true) + expect(task.didFinishAbortingStream).toBe(true) + expect(abortTaskSpy).toHaveBeenCalledOnce() + expect(task.abortReason).toBe("user_cancelled") + }) + + it("keeps pre-existing abortReason when cancel lands during retry backoff", async () => { + const mockProvider = makeMockProvider() + // autoApprovalEnabled causes the else branch to call backoffAndAnnounce. + mockProvider.getState = vi.fn().mockResolvedValue({ autoApprovalEnabled: true }) + + // Double cast: plain object satisfies only the methods called by this code path. + const task = new Task({ + provider: mockProvider as unknown as ClineProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(task, "dispose").mockResolvedValue(undefined) + + // eslint-disable-next-line require-yield -- intentional error-only async source + task["attemptApiRequest"] = async function* () { + throw new Error("mid-stream failure") + } + + // Simulate cancel landing during backoff: cancelTask sets abortReason before abort=true. + // Use a distinct seed value so ??= (keeps it) is distinguishable from = (overwrites). + task["backoffAndAnnounce"] = async () => { + task["abortReason"] = "streaming_failed" + task["abort"] = true + } + + const abortTaskSpy = vi.spyOn(task, "abortTask") + // break after backoff exits the while loop; the outer try returns false. + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }]) + expect(result).toBe(false) + expect(abortTaskSpy).toHaveBeenCalledOnce() + expect(task.abortReason).toBe("streaming_failed") + }) +}) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 72d0641efd..88db6f2f2a 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -86,6 +86,7 @@ vi.mock("delay", () => ({ })) import delay from "delay" +import pWaitFor from "p-wait-for" vi.mock("uuid", async (importOriginal) => { const actual = await importOriginal() @@ -713,6 +714,98 @@ describe("Cline", () => { ]) }) + it("clears didFinishAbortingStream on retry so cancelTask waits for the new stream (#1801)", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "abort flag reset test", + startTask: false, + }) + + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(getTaskTestAccess(task), "safeEnsureModelFetched").mockResolvedValue(stubModelInfo) + vi.spyOn(getTaskTestAccess(task), "presentAssistantMessageSafe").mockImplementation(() => {}) + + const failingStream = async function* (): AsyncGenerator { + yield { type: "text", text: "partial" } + throw new Error("simulated mid-stream failure") + } + + let releaseRetryStream: (() => void) | undefined + let markRetryStreamPaused: (() => void) | undefined + const retryStreamRelease = new Promise((resolve) => { + releaseRetryStream = resolve + }) + const retryStreamPaused = new Promise((resolve) => { + markRetryStreamPaused = resolve + }) + const pausedRetryStream = async function* (): AsyncGenerator { + yield { type: "text", text: "retry" } + markRetryStreamPaused?.() + await retryStreamRelease + } + + const attemptApiRequestSpy = vi + .spyOn(task, "attemptApiRequest") + .mockImplementationOnce(() => failingStream()) + .mockImplementationOnce(() => pausedRetryStream()) + .mockImplementation(() => { + throw new Error("stop after retry response") + }) + + const { default: realPWaitFor } = await vi.importActual("p-wait-for") + vi.mocked(pWaitFor).mockImplementation(realPWaitFor) + let request: Promise | undefined + let cancel: Promise | undefined + try { + await mockProvider.addClineToStack(task) + request = task.recursivelyMakeClineRequests([{ type: "text", text: "abort flag reset test" }]) + await Promise.race([ + retryStreamPaused, + request.then(() => { + throw new Error("request settled before the retry stream paused") + }), + ]) + + const firstApiReq = task.clineMessages.find((message) => message.say === "api_req_started") + expect(JSON.parse(firstApiReq?.text ?? "{}")).toMatchObject({ cancelReason: "streaming_failed" }) + expect(attemptApiRequestSpy).toHaveBeenCalledTimes(2) + + let markCancelWaiting: (() => void) | undefined + const cancelWaiting = new Promise((resolve) => { + markCancelWaiting = resolve + }) + vi.mocked(pWaitFor).mockImplementation((condition, options) => + realPWaitFor(async () => { + const done = await condition() + if (!done) markCancelWaiting?.() + return done + }, options), + ) + + vi.spyOn(mockProvider, "createTaskWithHistoryItem").mockImplementation((historyItem) => + ClineProvider.prototype.createTaskWithHistoryItem.call(mockProvider, historyItem, { + startTask: false, + }), + ) + + let cancelSettled = false + cancel = mockProvider.cancelTask().then(() => { + cancelSettled = true + }) + await Promise.race([cancelWaiting, cancel]) + expect(cancelSettled, "cancelTask must wait while the retry stream is live").toBe(false) + + releaseRetryStream?.() + await cancel + await expect(request).resolves.toBe(true) + } finally { + releaseRetryStream?.() + await Promise.allSettled([request, cancel]) + vi.mocked(pWaitFor).mockImplementation(async () => {}) + } + }) + it("finalizes MCP tool call using the request-scoped parser state", async () => { const task = new Task({ provider: mockProvider, @@ -2955,7 +3048,7 @@ describe("Cline", () => { expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled() }) - it("waits for a new partial message flush before a following message update", async () => { + it("keeps a new partial message on the throttled path without flushing", async () => { const task = new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, @@ -2964,11 +3057,6 @@ describe("Cline", () => { }) const taskAccess = getTaskTestAccess(task) vi.spyOn(taskAccess, "saveClineMessages").mockResolvedValue(true) - let releaseFlush!: () => void - const pendingFlush = new Promise((resolve) => { - releaseFlush = resolve - }) - const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockReturnValueOnce(pendingFlush) const updatePostSpy = vi.mocked(mockProvider.postMessageToWebview) const partialMessage = { ts: 1, @@ -2977,22 +3065,15 @@ describe("Cline", () => { text: "partial message", partial: true, } - let partialAddSettled = false - const addThenUpdate = taskAccess.addToClineMessages(partialMessage).then(async () => { - partialAddSettled = true - await taskAccess.updateClineMessage({ ...partialMessage, text: "updated partial" }) - }) - await Promise.resolve() - expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith() - expect(flushSpy).toHaveBeenCalledWith() - expect(partialAddSettled).toBe(false) - expect(updatePostSpy).not.toHaveBeenCalled() - - releaseFlush() - await addThenUpdate + await taskAccess.addToClineMessages(partialMessage) + await taskAccess.updateClineMessage({ ...partialMessage, text: "updated partial" }) - expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(updatePostSpy.mock.invocationCallOrder[0]) + // Flushing here would cancel the debounce's trailing timer and send the *next* + // message straight through the leading edge, collapsing the throttle back to one + // full-state post per message. + expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith() + expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled() expect(updatePostSpy).toHaveBeenCalledWith({ type: "messageUpdated", clineMessage: { diff --git a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts index 03789f7d40..f2c4cc2aed 100644 --- a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts +++ b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts @@ -6,6 +6,7 @@ import type { ToolCallbacks } from "../BaseTool" import { CodebaseSearchTool } from "../CodebaseSearchTool" import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry" import { CodeIndexManager } from "../../../services/code-index/manager" +import { CodeIndexStateManager } from "../../../services/code-index/state-manager" import { getWorkspacePath } from "../../../utils/path" import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode" @@ -15,6 +16,7 @@ vi.mock("vscode", () => ({ Uri: { file: vi.fn() }, })) vi.mock("../../../utils/path", () => ({ getWorkspacePath: vi.fn() })) +vi.mock("../../../services/code-index/state-manager") vi.mock("../../../services/code-index/manager", () => ({ CodeIndexManager: vi.fn().mockImplementation(function (workspacePath: string) { let initialized = false @@ -196,6 +198,7 @@ describe("CodebaseSearchTool workspace selection", () => { "/external-task", expect.objectContaining({ fsPath: "/external-task" }), provider.context, + expect.any(CodeIndexStateManager), ) expect(vscode.Uri.file).toHaveBeenCalledExactlyOnceWith("/external-task") const manager = CodeIndexManagerRegistry.getOrCreate(provider.context, "/external-task")! diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 2f8c7f8adc..b0d76ecbf8 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -90,9 +90,8 @@ import { McpHub } from "../../services/mcp/McpHub" import { McpServerManager } from "../../services/mcp/McpServerManager" import { MarketplaceManager } from "../../services/marketplace" import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService" -import type { CodeIndexManager } from "../../services/code-index/manager" import { CodeIndexManagerRegistry } from "../../services/code-index/code-index-manager-registry" -import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" +import type { CodeIndexWorkspaceScope } from "../../services/code-index/code-index-workspace-scope" import { MdmService } from "../../services/mdm/MdmService" import { SkillsManager } from "../../services/skills/SkillsManager" @@ -186,6 +185,25 @@ type GetStateOptions = { includeTaskHistory?: boolean } +/** + * Internal options for {@link ClineProvider.upsertProviderProfile}. + */ +type UpsertProviderProfileOptions = { + /** + * Internal-only bypass of the organization model allow-list, used exclusively + * for Zoo Gateway credential synchronization (token refresh) and sign-out + * writes. These are auth writes, not model selections: a restrictive + * allow-list may omit `zoo-gateway` entirely, or list the provider without the + * active `zooGatewayModelId`, and must not reject the credential write and + * leave stale credentials behind in the active profile. + * + * This flag must never be set from a webview-originated code path: the webview + * is not a trusted boundary, so every user-driven profile write keeps the + * allow-list enforcement. + */ + bypassAllowList?: boolean +} + export class ClineProvider extends EventEmitter implements vscode.WebviewViewProvider, TelemetryPropertiesProvider, TaskProviderLike @@ -212,8 +230,6 @@ export class ClineProvider private taskScheduler = new TaskScheduler() private static readonly delegationTransitionLocks = new Map>() private cancelledDelegationChildIds = new Set() - private codeIndexStatusSubscription?: vscode.Disposable - private codeIndexManager?: CodeIndexManager private _workspaceTracker?: WorkspaceTracker // workSpaceTracker read-only for access outside this class protected mcpHub?: McpHub // Change from private to protected protected skillsManager?: SkillsManager @@ -406,35 +422,7 @@ export class ClineProvider } this.emit(RooCodeEventName.TaskCompleted, taskId, tokenUsage, toolUsage) } - const onTaskAborted = async () => { - this.emit(RooCodeEventName.TaskAborted, instance.taskId) - - try { - // Only rehydrate on genuine streaming failures. - // User-initiated cancels are handled by cancelTask(). - if (instance.abortReason === "streaming_failed") { - // Defensive safeguard: if another path already replaced this instance, skip - const current = this.getCurrentTask() - if (current && current.instanceId !== instance.instanceId) { - this.log( - `[onTaskAborted] Skipping rehydrate: current instance ${current.instanceId} != aborted ${instance.instanceId}`, - ) - return - } - - const { historyItem } = await this.getTaskWithId(instance.taskId) - const rootTask = instance.rootTask - const parentTask = instance.parentTask - await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask }) - } - } catch (error) { - this.log( - `[onTaskAborted] Failed to rehydrate after streaming failure: ${ - error instanceof Error ? error.message : String(error) - }`, - ) - } - } + const onTaskAborted = () => this.emit(RooCodeEventName.TaskAborted, instance.taskId) const onTaskFocused = () => this.emit(RooCodeEventName.TaskFocused, instance.taskId) const onTaskUnfocused = () => this.emit(RooCodeEventName.TaskUnfocused, instance.taskId) const onTaskActive = (taskId: string) => this.emit(RooCodeEventName.TaskActive, taskId) @@ -1073,17 +1061,6 @@ export class ClineProvider // and executes code based on the message that is received. this.setWebviewMessageListener(webviewView.webview) - // Initialize code index status subscription for the current workspace. - this.updateCodeIndexStatusSubscription() - - // Listen for active editor changes to update code index status for the - // current workspace. - const activeEditorSubscription = vscode.window.onDidChangeActiveTextEditor(() => { - // Update subscription when workspace might have changed. - this.updateCodeIndexStatusSubscription() - }) - this.webviewDisposables.push(activeEditorSubscription) - // Listen for when the panel becomes visible. // https://github.com/microsoft/vscode-discussions/discussions/840 if ("onDidChangeViewState" in webviewView) { @@ -1121,8 +1098,6 @@ export class ClineProvider } else { this.log("Clearing webview resources for sidebar view") this.clearWebviewResources() - // Reset current workspace manager reference when view is disposed - this.codeIndexManager = undefined } }, null, @@ -1534,7 +1509,7 @@ export class ClineProvider console.error("[ClineProvider:Vite] Failed to read Vite port file:", err) } - const localServerUrl = `localhost:${localPort}` + const localServerUrl = `127.0.0.1:${localPort}` // Check if local dev server is running. try { @@ -1573,7 +1548,7 @@ export class ClineProvider const reactRefresh = /*html*/ `