mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-23 07:32:20 +03:00
fix(compression): CCR must not strand prompts for callers without the retrieve tool (#11084)
Validated on the combined batch board + this branch: ccr-non-mcp-full-prompt-loss + ccr-retrieval-ramp 20/20, file-size gate green with the ccr/index listing (1024, dated annotation — owner-authorized). The callerSupportsCcrRetrieve gate now skips the whole engine for callers whose tools[] cannot reach omniroute_ccr_retrieve — no more 15KB prompt arriving upstream as 112 tokens. Production-measured root cause, textbook TDD. Thank you @HouMinXi!
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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<string, unknown>, { 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<string, unknown>, { 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<string, unknown>, { 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<string, unknown>, { 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<string, unknown>, { 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"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 }
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user