diff --git a/open-sse/services/compression/engines/ccr/index.ts b/open-sse/services/compression/engines/ccr/index.ts index d2136fc8f1..92869bfbc9 100644 --- a/open-sse/services/compression/engines/ccr/index.ts +++ b/open-sse/services/compression/engines/ccr/index.ts @@ -45,7 +45,7 @@ import { } from "../../../../../src/lib/db/ccrBlocks.ts"; import { createCompressionStats } from "../../stats.ts"; import { queryBlock, type CcrQuery } from "./ccrQuery.ts"; -import { injectCcrProtocolInstruction } from "./protocolInstruction.ts"; +import { callerSupportsCcrRetrieve, injectCcrProtocolInstruction } from "./protocolInstruction.ts"; import type { CompressionEngine, CompressionEngineApplyOptions, @@ -939,6 +939,30 @@ export const ccrEngine: CompressionEngine = { return { body, compressed: false, stats: null }; } + // #7746 follow-up: only callers whose tools[] proves they can reach + // omniroute_ccr_retrieve may have content replaced at all. For everyone + // else (plain OpenAI-compatible clients — the marker is an MCP-only + // contract) replacement would strand the original text behind a hash the + // model has no way to resolve. Skip the whole engine for them. The check + // is wrapped defensively: a malformed body must fail OPEN (no + // compression), never throw into the request pipeline. + let callerCanRetrieve = false; + try { + callerCanRetrieve = callerSupportsCcrRetrieve(body); + } catch (err) { + // Defensive: the helper is total, but if it ever throws we must fail + // OPEN (no compression) — and surface it so a future regression in the + // helper is visible instead of silently bypassing compression forever. + console.warn( + "[compression/ccr] callerSupportsCcrRetrieve threw; skipping compression:", + err instanceof Error ? err.message : err + ); + callerCanRetrieve = false; + } + if (!callerCanRetrieve) { + return { body, compressed: false, stats: null }; + } + const minChars = typeof stepConfig["minChars"] === "number" ? (stepConfig["minChars"] as number) diff --git a/tests/unit/compression/ccr-non-mcp-full-prompt-loss-7746.test.ts b/tests/unit/compression/ccr-non-mcp-full-prompt-loss-7746.test.ts index 0f4abd534e..a646744871 100644 --- a/tests/unit/compression/ccr-non-mcp-full-prompt-loss-7746.test.ts +++ b/tests/unit/compression/ccr-non-mcp-full-prompt-loss-7746.test.ts @@ -15,6 +15,7 @@ import { describe, it, before } from "node:test"; import assert from "node:assert/strict"; import { ccrEngine, + getCcrStoreStats, resetCcrStore, retrieveBlock, } from "../../../open-sse/services/compression/engines/ccr/index.ts"; @@ -54,39 +55,117 @@ describe("issue #7746 — CCR must not reduce the sole user prompt to a bare, un }); it("prompt fixture is realistically sized (>= default 600-char minChars)", () => { - assert.ok(REPORTER_PROMPT.length >= 600, `fixture must be >= 600 chars, got ${REPORTER_PROMPT.length}`); - }); - - it("does not leave the model with only the bare CCR marker when no retrieve tool is available", () => { - resetCcrStore(); - const body = makeOpenCodeStyleRequestBody(); - const result = ccrEngine.apply(body as Record, { stepConfig: {} }); - - assert.equal(result.compressed, true, "CCR compressed the sole user message (reproducing the report)"); - - const messages = result.body.messages as Array<{ role: string; content: string }>; - const compressedContent = messages[0].content; - const isBareMarkerOnly = /^\[CCR retrieve hash=[0-9a-f]{24} chars=\d+\]$/.test(compressedContent); - - assert.equal( - isBareMarkerOnly, - false, - "BUG #7746: CCR replaced the ENTIRE sole user message with nothing but the bare " + - `[CCR retrieve hash=...] marker, permanently losing the original prompt for any ` + - `non-MCP caller that cannot resolve the marker. Got: ${JSON.stringify(compressedContent)}` + assert.ok( + REPORTER_PROMPT.length >= 600, + `fixture must be >= 600 chars, got ${REPORTER_PROMPT.length}` ); }); - it("the original prompt remains fully retrievable by hash even after the guard applies", () => { + it("non-MCP caller: CCR skips entirely — the sole user prompt passes through verbatim", () => { resetCcrStore(); const body = makeOpenCodeStyleRequestBody(); const result = ccrEngine.apply(body as Record, { stepConfig: {} }); + // #7746 follow-up (forge review outage, 2026-08-22): the preamble guard was + // not enough — a non-MCP caller received "[CCR retrieve hash=...] markers" + // it had no tool to resolve (upstream saw 112 of ~3.6K tokens). The engine + // now refuses to replace content at all when tools[] lacks + // omniroute_ccr_retrieve: compressed=false, message content untouched. + assert.equal( + result.compressed, + false, + "CCR must not compress for a caller without the retrieve tool" + ); + assert.equal(result.stats, null, "no stats when the engine is skipped"); const messages = result.body.messages as Array<{ role: string; content: string }>; - const compressedContent = messages[0].content; + assert.equal(messages[0].role, "user", "message role must stay user"); + assert.equal( + messages[0].content, + REPORTER_PROMPT, + "sole user prompt must pass through verbatim" + ); + assert.equal(messages.length, 1, "no protocol instruction may be injected for non-MCP callers"); + // Guard regression check: if callerSupportsCcrRetrieve ever returned true + // here, the store would silently accumulate blocks no non-MCP caller can + // retrieve. After a skip the store must hold nothing for this principal. + assert.equal(getCcrStoreStats().entries, 0, "store must stay empty after a non-MCP skip"); + }); + + // tools:[] and unrelated tools are distinct caller shapes that must all be + // treated as non-MCP: an empty array and a foreign tool list both mean the + // retrieve tool is unreachable. + for (const label of ["empty tools array", "unrelated tools"] as const) { + it(`non-MCP caller with ${label}: CCR skips entirely`, () => { + resetCcrStore(); + const tools = + label === "empty tools array" + ? [] + : [ + { type: "function", function: { name: "get_weather" } }, + { type: "function", function: { name: "web_search" } }, + ]; + const body = { ...makeOpenCodeStyleRequestBody(), tools }; + const result = ccrEngine.apply(body as Record, { stepConfig: {} }); + + assert.equal(result.compressed, false, `${label} must not compress`); + const messages = result.body.messages as Array<{ role: string; content: string }>; + assert.equal(messages[0].content, REPORTER_PROMPT, "prompt passes through verbatim"); + assert.equal(messages.length, 1, "no protocol instruction injected"); + }); + } + + // A malformed body (tools as a non-array, or entries of unexpected shape) + // must fail OPEN — no compression, never a throw into the request pipeline. + for (const malformed of [ + { tools: "not-an-array" }, + { tools: [null, 42, "x"] }, + { tools: [{}, { type: "function" }] }, + ]) { + it(`malformed tools payload (${JSON.stringify(malformed.tools)}): engine skips without throwing`, () => { + resetCcrStore(); + const body = { ...makeOpenCodeStyleRequestBody(), ...malformed }; + const result = ccrEngine.apply(body as Record, { stepConfig: {} }); + + assert.equal(result.compressed, false, "malformed tools must fail open (skip)"); + const messages = result.body.messages as Array<{ role: string; content: string }>; + assert.equal(messages[0].content, REPORTER_PROMPT, "prompt passes through verbatim"); + assert.equal( + getCcrStoreStats().entries, + 0, + "store must stay empty after a malformed-tools skip" + ); + }); + } + + it("MCP-capable caller (tools[] advertises omniroute_ccr_retrieve): replacement still runs and stays retrievable", () => { + resetCcrStore(); + const body = { + ...makeOpenCodeStyleRequestBody(), + tools: [{ type: "function", function: { name: "omniroute_ccr_retrieve" } }], + }; + const result = ccrEngine.apply(body as Record, { stepConfig: {} }); + + assert.equal(result.compressed, true, "CCR still compresses for MCP-capable callers"); + const messages = result.body.messages as Array<{ role: string; content: string }>; + // The protocol instruction is injected as a leading system message, so the + // compressed conversation is exactly: [instruction, original user message]. + assert.equal(messages.length, 2, "instruction + user message"); + assert.equal(messages[0].role, "system", "instruction is a leading system message"); + assert.ok( + typeof messages[0].content === "string" && messages[0].content.length > 0, + "instruction content must be non-empty" + ); + assert.ok( + messages[0].content.includes("omniroute_ccr_retrieve"), + "instruction must teach the retrieve tool contract" + ); + const compressedContent = messages[1].content; const match = compressedContent.match(/\[CCR retrieve hash=([0-9a-f]{24}) chars=\d+\]/); - assert.ok(match, "compressed content must still contain a resolvable CCR marker"); - const hash = match![1]; - assert.equal(retrieveBlock(hash), REPORTER_PROMPT, "original prompt must be stored verbatim and retrievable"); + assert.ok(match, "compressed content must contain a resolvable CCR marker"); + assert.equal( + retrieveBlock(match![1]), + REPORTER_PROMPT, + "original prompt must be stored verbatim and retrievable" + ); }); }); diff --git a/tests/unit/compression/ccr-retrieval-ramp.test.ts b/tests/unit/compression/ccr-retrieval-ramp.test.ts index 2b475e69bf..6e87f7b787 100644 --- a/tests/unit/compression/ccr-retrieval-ramp.test.ts +++ b/tests/unit/compression/ccr-retrieval-ramp.test.ts @@ -78,7 +78,13 @@ describe("ccrEngine.apply — retrieval-aware compression (H8)", () => { const block = (len: number) => "x".repeat(len); const run = (content: string, retrievalRampFactor = 2) => ccrEngine.apply( - { messages: [{ role: "user", content }] }, + // The retrieve tool is advertised — this suite exercises the compression + // path itself (H8 ramp); without the tool declaration the #7746 guard + // skips the engine entirely. + { + messages: [{ role: "user", content }], + tools: [{ type: "function", function: { name: "omniroute_ccr_retrieve" } }], + }, { stepConfig: { minChars: BASE, retrievalRampFactor }, principalId: P } );