pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-07-26 09:52:11 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(compression): skip CCR on tool outputs to preserve agent loop (#7869)

Browse Source 
* fix(compression): skip CCR on tool outputs to preserve agent loop

When OmniRoute is used as a chat-completion PROVIDER (not as an MCP server),
the upstream LLM cannot call `omniroute_ccr_retrieve` to expand CCR markers
on demand. Replacing tool outputs with `[CCR retrieve hash=… chars=…]`
placeholders therefore makes the LLM stall — it sees an opaque marker
where the actual tool result should be and has no way to recover the
verbatim content.

Scope:
- OpenAI format: `{ role: "tool", tool_call_id, content }`
- Anthropic format: `{ role: "user", content: [{ type: "tool_result", … }] }`

Fix: extend `processMessages` in the CCR engine to skip both shapes
verbatim. The engine still applies to plain user / assistant text blocks,
which is where compression yields token savings AND the LLM can reason
about the marker.

Tests:
- New `tests/unit/compression/ccr-skip-tool-outputs.test.ts` covers both
  formats (4 cases) plus a regression guard that plain user text is still
  compressed.
- All 57 pre-existing CCR tests still pass.

Reported-by: herjarsa
Refs: AGENTS.md agent feedback — agent loop stalled on bash/read/grep
       outputs after CCR collapsed them to markers.

* fix(compression): guard CCR against null / non-object parts

Apply gemini-code-assist review feedback on PR #7869:
- Use optional chaining when reading `part["type"]` so malformed
  client payloads (null entries, non-object entries in the parts
  array) cannot throw `TypeError: Cannot read properties of null`.
- The skip rule (this branch) and the existing text-part compression
  path (the next branch) both get the guard, since both dereference
  `part["type"]` directly.

Test:
- New defensive case: a user message whose content array contains a
  `null` entry alongside a `tool_result` must not crash the engine.

Refs: gemini-code-assist review on #7869 (PRR_kwDORPf6ys8AAAABGj5G7Q)

---------

Co-authored-by: herjarsa <herjarsa@users.noreply.github.com>
This commit is contained in:
Hernan Javier Ardila Sanchez
2026-07-20 20:56:27 +02:00
committed by GitHub
parent 286628a8c4
commit fca82af737
2 changed files with 230 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
open-sse/services/compression/engines/ccr/index.ts
View File

@@ -592,6 +592,20 @@ function processMessages(
const result = messages.map((msg) => {
if (msg.role === "system") return { ...msg };
// H-fix1: skip tool outputs (OpenAI `role:"tool"` and Anthropic user
// messages whose content is exclusively `tool_result` parts). When
// OmniRoute is used as a chat-completion PROVIDER, the upstream LLM has no
// way to call `omniroute_ccr_retrieve` and expand markers — replacing tool
// outputs with `[CCR retrieve hash=…]` placeholders therefore breaks the agent
// loop. Preserve tool outputs verbatim so the LLM can keep reasoning.
if (msg.role === "tool") return { ...msg };
if (msg.role === "user" && Array.isArray(msg.content)) {
const parts = msg.content as Array<Record<string, unknown>>;
if (parts.length > 0 && parts.every((p) => p?.["type"] === "tool_result")) {
return { ...msg };
}
}
if (typeof msg.content === "string") {
const { text, replaced } = maybeCcrReplace(msg.content, minChars, principalId, rampFactor);
if (replaced) {
@@ -604,7 +618,7 @@ function processMessages(
if (Array.isArray(msg.content)) {
let changed = false;
const newContent = msg.content.map((part) => {
if (part["type"] !== "text" || typeof part["text"] !== "string") return part;
if (part?.["type"] !== "text" || typeof part?.["text"] !== "string") return part;
const { text, replaced } = maybeCcrReplace(
part["text"] as string,
minChars,

215
tests/unit/compression/ccr-skip-tool-outputs.test.ts Normal file
View File

@@ -0,0 +1,215 @@
/**
* TDD tests for CCR skip-on-tool-outputs behavior (Fix 1).
*
* Background: when OmniRoute is used as a chat-completion PROVIDER (not as an
* MCP server), the upstream LLM cannot call `omniroute_ccr_retrieve` to
* expand CCR markers. Replacing tool outputs with `[CCR retrieve hash=X
* chars=Y]` markers therefore breaks the agent loop — the LLM sees an
* opaque placeholder where the actual tool result should be and stalls.
*
* Fix: CCR must preserve tool-role messages and Anthropic-style tool_result
* parts verbatim. The compression engine still applies to plain user /
* assistant text blocks (where the LLM can reason about the marker).
*
* Run: node --import tsx/esm --test tests/unit/compression/ccr-skip-tool-outputs.test.ts
*/
import { describe, it, before } from "node:test";
import assert from "node:assert/strict";
import {
ccrEngine,
resetCcrStore,
} from "../../../open-sse/services/compression/engines/ccr/index.ts";
import { registerBuiltinCompressionEngines } from "../../../open-sse/services/compression/index.ts";
const LARGE_TOOL_OUTPUT = `Total 247 lines in current working directory.
drwxr-xr-x 18 herjarsa herjarsa 4096 Jul 20 13:12 .
drwxr-xr-x 14 root root 4096 Jul 17 17:59 ..
drwxr-xr-x 4 herjarsa herjarsa 4096 Jul 18 14:41 .codegraph
-rw-r--r-- 1 herjarsa herjarsa 8927 Jul 18 02:10 ARCHITECTURE.md
-rw-r--r-- 1 herjarsa herjarsa 341669 Jul 20 13:12 meta-governor.log
-rw-r--r-- 1 herjarsa herjarsa 81784 Jul 16 13:55 package-lock.json
-rw-r--r-- 1 herjarsa herjarsa 4798 Jul 17 17:59 opencode.json
-rw-r--r-- 1 herjarsa herjarsa 8045 Jul 12 02:07 STRUCTURE.md
drwxr-xr-x 4 herjarsa herjarsa 4096 Jul 20 13:12 .opencode
-rw-r--r-- 1 herjarsa herjarsa 94 Jul 20 13:12 omo-meta-governor-upgrade-check.json
-rw-r--r-- 1 herjarsa herjarsa 8437 Jul 14 18:42 oh-my-openagent.jsonc
-rw------- 1 herjarsa herjarsa 19542 Jul 18 02:09 session.json
drwxr-xr-x 2 herjarsa herjarsa 4096 Jun 14 18:42 snippet
drwxr-xr-x 22 herjarsa herjarsa 4096 Jun 13 12:33 skills
drwxr-xr-x 101 herjarsa herjarsa 4096 Jul 16 03:30 skill-libraries
-rw-r--r-- 1 herjarsa herjarsa 233 Jun 19 22:39 tui.json
-rw-r--r-- 1 herjarsa herjarsa 80415 Jun 14 18:42 structure-summary.json
The CCR engine must NOT replace this content with a marker because the LLM
needs the verbatim output to continue reasoning. Adding more lines to push
the character count well past the default minChars of 600 so the test is
unambiguous about why the skip rule is necessary. Even if minChars were 100
the rule must still hold because tool outputs are never safe to compress.`;
describe("ccr engine — skip tool outputs", () => {
before(() => {
resetCcrStore();
registerBuiltinCompressionEngines();
});
it("does NOT compress role:tool messages (OpenAI format)", () => {
const body = {
model: "gpt-4",
messages: [
{
role: "tool",
tool_call_id: "call_abc123",
content: LARGE_TOOL_OUTPUT,
},
],
};
const result = ccrEngine.apply(body as Record<string, unknown>);
assert.equal(
result.compressed,
false,
"role:tool messages must NEVER trigger CCR compression — tool output is needed verbatim"
);
const messages = result.body.messages as Array<{ role: string; content: string }>;
assert.equal(
messages[0].content,
LARGE_TOOL_OUTPUT,
"role:tool content must be byte-identical to input (no marker substitution)"
);
assert.ok(
!messages[0].content.includes("[CCR retrieve"),
"role:tool content must not contain any CCR marker"
);
});
it("does NOT compress Anthropic-style user message containing only tool_result parts", () => {
const body = {
model: "claude-sonnet-4-5",
messages: [
{
role: "user",
content: [
{
type: "tool_result",
tool_use_id: "toolu_abc123",
content: LARGE_TOOL_OUTPUT,
},
{
type: "tool_result",
tool_use_id: "toolu_def456",
content: LARGE_TOOL_OUTPUT + "\n--- second tool result ---",
},
],
},
],
};
const result = ccrEngine.apply(body as Record<string, unknown>);
assert.equal(
result.compressed,
false,
"user message containing only tool_result parts must NOT trigger CCR compression"
);
const messages = result.body.messages as Array<{
role: string;
content: Array<{ type: string; content?: string; text?: string }>;
}>;
assert.equal(messages[0].content[0].type, "tool_result");
assert.equal(
messages[0].content[0].content,
LARGE_TOOL_OUTPUT,
"first tool_result content must be byte-identical to input"
);
assert.equal(
messages[0].content[1].content,
LARGE_TOOL_OUTPUT + "\n--- second tool result ---",
"second tool_result content must be byte-identical to input"
);
});
it("still compresses plain user text — the skip rule is scoped to tool outputs", () => {
// Sanity check: the fix must NOT regress the existing compression path.
// A plain user-role message with large text content must still be compressed.
const LARGE_USER_TEXT = LARGE_TOOL_OUTPUT; // same length, same trigger
const body = {
model: "gpt-4",
messages: [{ role: "user", content: LARGE_USER_TEXT }],
};
const result = ccrEngine.apply(body as Record<string, unknown>);
assert.equal(
result.compressed,
true,
"plain role:user text block above minChars MUST still be compressed (regression guard)"
);
const messages = result.body.messages as Array<{ role: string; content: string }>;
assert.ok(
messages[0].content.match(/\[CCR retrieve hash=[0-9a-f]{24} chars=\d+\]/),
"plain user text must still be replaced with a CCR marker"
);
});
it("does NOT compress when the user message is purely a tool_result (even one part)", () => {
// Single-element edge case: a user message with a single tool_result part
// must still be skipped, not compressed.
const body = {
model: "claude-sonnet-4-5",
messages: [
{
role: "user",
content: [
{
type: "tool_result",
tool_use_id: "toolu_single",
content: LARGE_TOOL_OUTPUT,
},
],
},
],
};
const result = ccrEngine.apply(body as Record<string, unknown>);
assert.equal(result.compressed, false, "single tool_result part must also be skipped");
const messages = result.body.messages as Array<{
role: string;
content: Array<{ type: string; content?: string }>;
}>;
assert.equal(messages[0].content[0].content, LARGE_TOOL_OUTPUT);
});
});
it("does NOT crash on malformed user content with null / non-object parts (defensive guard)", () => {
// Regression guard for gemini-code-assist review on PR #7869:
// msg.content is parsed from external client input, so a malformed
// payload could deliver `null` or non-object entries in the parts
// array. The optional-chaining check must NOT throw a TypeError.
const body = {
model: "claude-sonnet-4-5",
messages: [
{
role: "user",
content: [
null,
{ type: "tool_result", tool_use_id: "toolu_x", content: LARGE_TOOL_OUTPUT },
],
},
],
};
// Must not throw.
const result = ccrEngine.apply(body as Record<string, unknown>);
// Null is not a tool_result, so the message must NOT be skipped as
// a tool-only message — it falls through to the normal array path
// where the existing text/non-text branch handles each part.
// We just need the engine to not crash on the malformed payload.
assert.ok(result.body, "engine must return a body even with malformed content");
assert.equal(typeof result.compressed, "boolean");
});