Repository navigation
Conversation
PR governanceThis PR follows the template and is marked ready for human review. |
|
Updated the Real Behavior Proof section to use the four exact template field labels: Environment, Exact command / steps, Observed result, and Not tested. The actual before/after CLI/HTTP/native evidence and successful normal pre-push results are included; the fix and its verification are complete. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Final verification for c5c36ba: all latest enabled CI checks passed (main CI run 38164986293, four test shards and Windows tool-result interceptors; scope-skipped jobs remain skipped). The updated Codecov report now says “All modified and coverable lines are covered by tests.” Both provider instruction calls are covered by actual API regressions: they fail on the initial PR SHA and pass with the fix. All seven CCR/API suites pass locally (161 tests), and all normal unfiltered pre-push gates passed without bypass. The separate Codecov GitHub App installation warning requires upstream owner/admin access. This contributor has pull-only permissions ( |
Description
Fixes #4159.
OpenCode exposes
headroom_headroom_retrieve, but the proxy only checks the bareheadroom_retrievewhen deciding whether the client already owns retrieval. It therefore adds a second, unavailable client tool. Retrieval-only proxy turns can be intercepted internally; mixed retrieval + Read turns are forwarded to the client, which cannot execute the bare spelling.Type of Change
Changes Made
CCRToolInjectorfallback. Accept the bare canonical name and the three supported default-Headroom-server client spellings.headroom_retrievesuffix as ownership of Headroom's cache. Existing broader compression-exclusion matching is unchanged.Testing
Test Output
All 6 initial alias/provider regressions fail against exact upstream main
976aa714ef3e19174dc0cb586cee7b20d064e4efbecause unavailable bare retrieval is injected. All 10 first review regressions and 2 additional actual-API regressions fail against immutable first-PR commit7601252be7e8b02297e342e2775bb153927e2fe9: instructions name an unavailable tool, or a foreign MCP server displaces proxy retrieval. Current fixed result: 161 passed, 1 existing Starlette TestClient deprecation warning across:Ruff format/check passed for the four changed Python sources and three regression files. A genuine-module-preloaded, coverage-instrumented CCR run passed all 110 cases (four warnings); an additional instrumented API run passed both new API regressions and measured both changed handler instruction calls as executed. Import-time coverage is incomplete in those local runs, so they are not whole-package or CI patch-coverage claims. Whole-repository pytest and whole-package mypy are not claimed by these scoped results.
Real Behavior Proof
Environment: Linux x86_64, Python 3.13.14; genuine installed development/proxy extras and editable Rust extension, actual ONNX Runtime 1.26.0 via
ORT_DYLIB_PATH. Independent baseline/fixed worktrees. Beacon off, isolated settings, stateless proxy, no live credentials. Provider is a controlled local Anthropic-shaped HTTP upstream; request model string isclaude-sonnet-4-20250514, not a live model run.Exact command / steps: start
headroom proxy --port 18858for baseline and--port 18859for fixed source, each with--anthropic-api-url http://127.0.0.1:45111and:Optimization and CCR remain enabled. Both genuine CLI startups report
event=rust_core_loaded. POST the same/v1/messagesrequest to each with model above,max_tokens: 128,system: "Use the available tools.", user textRetrieve the original compressed data and read the related file., and these client tool schemas:[ {"name":"headroom_headroom_retrieve","description":"Retrieve original Headroom content through the client MCP server","input_schema":{"type":"object","properties":{"hash":{"type":"string"}},"required":["hash"]}}, {"name":"Read","description":"Read a local file","input_schema":{"type":"object","properties":{"file_path":{"type":"string"}},"required":["file_path"]}} ]The controlled upstream observes the actual advertised tools, selects bare retrieval if advertised (otherwise the available alias), and returns a mixed retrieval + Read response.
Observed before: upstream tools are
Read,headroom_headroom_retrieve,headroom_retrieve. HTTP 200 forwards mixedheadroom_retrieve+Read, leaving one returned tool unavailable in the client's registry.Observed result: after the fix, upstream tools are only
Read,headroom_headroom_retrieve. HTTP 200 forwards mixedheadroom_headroom_retrieve+Read; zero unavailable returned names. No mixed-call suppression or response rewriting hides the problem.Not tested: live Anthropic, installed OpenCode end-to-end, actual CCR hash/payload recovery, Windows or native Gemini HTTP. Per-request fallback is exercised directly by regressions. Controlled-upstream behavior alone is not offered as proof: this ran the real baseline/fixed proxy CLI, HTTP path and native extension.
Additional review behavior proof
The first PR SHA and corrected source each ran the actual proxy app through its full lifespan, with the genuine native extension loaded, actual
CompressionStoreand real HTTP to a controlled upstream at127.0.0.1:45112. Requests entered the genuine Anthropic and OpenAI ASGI routes, not patched handlers; this additional check was not an external CLI socket or live-provider run.Construct
ProxyConfigwithccr_inject_system_instructions=True,stateless=True, no caching/rate limiting/subscription tracking/read lifecycle/Kompress, Anthropic URLhttp://127.0.0.1:45112, and OpenAI URLhttp://127.0.0.1:45112/v1. After startup, store original/compressed content in the actual in-memory store and use its returned hash in<<ccr:HASH 1_rows_offloaded>>. Send a valid assistant tool call and matching tool result to each provider endpoint, once with onlyheadroom_headroom_retrieveregistered and once with onlymcp__other__headroom_retrieveregistered. Anthropic uses tool-use/tool-result blocks and modelclaude-sonnet-4-20250514; OpenAI uses function tool-calls/tool messages and modelgpt-4o. The upstream records advertised names and names invoked by the actual injected system instructions, then returns a normal final text response.All eight before/after requests returned HTTP 200. Before: both providers instructed bare
headroom_retrievewhile neither registry contained it. After: the Headroom alias is retained alone and instructions invoke it; a foreign server's tool is retained alongside proxy-owned bare retrieval and instructions invoke the bare tool. Zero unavailable instruction names in all four corrected requests. The upstream and both apps were stopped afterwards.The permanent API regressions reuse the existing full-lifespan provider
TestClientfixtures and a real isolated in-memory store. Only the upstream response boundary supplies fixed provider-shaped JSON; the CCR injector, instruction generation and ownership selector are real. Both tests fail on the first PR SHA because the instructions name unavailable bare retrieval, then pass after the fix. They assert executable-name and real-hash invariants, not that a helper received forwarded arguments. Scoped coverage measured the previously untested instruction calls in both handlers as executed.Runtime Rollout Safety
Review Readiness
Checklist
CHANGELOG.mdMandatory Local Push Gates
Normal
git push fork fix/issue-4159-client-retrieval-alias-20261011executed the installed pre-push hook and
make ci-precheckpassed:cargo fmt --all -- --checkcargo clippy --workspace -- -D warningscargo test --workspaceThe warning is the existing no-client Anthropic tokenizer approximation
reported by
test_relevance_with_query_context; it was not suppressed.The earlier cold-build push timed out. Retrying with the real shared Cargo
build cache and two build jobs completed all gates and pushed normally;
no bypass flag or skipped hook was used.
The review update
440fd96aaa11c0ea9263d9628ccc2b5f8b3865d4alsopassed every normal pre-push gate above, including 177 critical Python
tests. This run used
RUST_TEST_THREADS=2: the preceding default-parallelattempt failed the unchanged native
expired_get_does_not_wipe_concurrent_refreshtest's final-live assertionwith a 20 ms TTL. Baseline/current native source hashes are identical.
No Rust fix is claimed; no tests or gates were filtered or bypassed.
The API regression update
c5c36ba0e7074cbd815498d00ee8f394bd7287bcagain passed all normal local push gates above with the same bounded
test/build concurrency, genuine native rebuild/import and 177 critical
Python tests. The two additional API regressions and complete seven-suite
161-test run are separate measured verification, not mocked CCR plumbing.