Skip to content

fix(ccr): recognize client-owned MCP retrieval aliases - #4165

Open
oxura wants to merge 3 commits into
headroomlabs-ai:mainfrom
oxura:fix/issue-4159-client-retrieval-alias-20261011
Open

oxura wants to merge 3 commits into
headroomlabs-ai:mainfrom
oxura:fix/issue-4159-client-retrieval-alias-20261011

Conversation

@oxura

@oxura oxura commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4159.

OpenCode exposes headroom_headroom_retrieve, but the proxy only checks the bare headroom_retrieve when 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

  • Bug fix
  • Documentation update

Changes Made

  • Share one ownership selector between session-sticky injection and the per-request CCRToolInjector fallback. Accept the bare canonical name and the three supported default-Headroom-server client spellings.
  • Do not treat another MCP server's headroom_retrieve suffix as ownership of Headroom's cache. Existing broader compression-exclusion matching is unchanged.
  • Optional system instructions use the executable registered retrieval name in the legacy path and both Anthropic/OpenAI handlers.
  • Preserve client-owned aliases and bare-only proxy interception; do not alter mixed-tool response handling or add execution of client tools.
  • Add namespace/provider regressions across cold, compressed and later turns, plus real-store instruction and foreign-server boundaries.
  • Delete the incidental tool-description-size assertion and error-message wording pin, retaining real eager/lazy/cache-stability validation.
  • Document default-server ownership, alias-only forwarding and mixed-call behavior explicitly.

Testing

  • Relevant CCR/API tests pass: 161 cases across the seven suites below
  • Changed Python source/test Ruff format and check pass
  • Failing-before/passing-after behavioral regressions added
  • Actual CLI/HTTP/native before-and-after smoke performed

Test Output

All 6 initial alias/provider regressions fail against exact upstream main 976aa714ef3e19174dc0cb586cee7b20d064e4ef because unavailable bare retrieval is injected. All 10 first review regressions and 2 additional actual-API regressions fail against immutable first-PR commit 7601252be7e8b02297e342e2775bb153927e2fe9: 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:

tests/test_ccr_tool_injection_cache_stability.py
tests/test_ccr_tool_always_on.py
tests/test_ccr_tool_injection.py
tests/test_issue_728_empty_tools_injection.py
tests/test_corrupt_golden_bytes_recovery.py
tests/test_proxy_anthropic_cache_stability.py
tests/test_proxy_openai_cache_stability.py

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 is claude-sonnet-4-20250514, not a live model run.

  • Exact command / steps: start headroom proxy --port 18858 for baseline and --port 18859 for fixed source, each with --anthropic-api-url http://127.0.0.1:45111 and:

--stateless --no-cache --no-rate-limit --no-subscription-tracking
--disable-kompress --disable-kompress-fallback --no-read-lifecycle --no-telemetry

Optimization and CCR remain enabled. Both genuine CLI startups report event=rust_core_loaded. POST the same /v1/messages request to each with model above, max_tokens: 128, system: "Use the available tools.", user text Retrieve 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 mixed headroom_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 mixed headroom_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 CompressionStore and real HTTP to a controlled upstream at 127.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 ProxyConfig with ccr_inject_system_instructions=True, stateless=True, no caching/rate limiting/subscription tracking/read lifecycle/Kompress, Anthropic URL http://127.0.0.1:45112, and OpenAI URL http://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 only headroom_headroom_retrieve registered and once with only mcp__other__headroom_retrieve registered. Anthropic uses tool-use/tool-result blocks and model claude-sonnet-4-20250514; OpenAI uses function tool-calls/tool messages and model gpt-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_retrieve while 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 TestClient fixtures 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

  • Rollout-managed feature(s): existing CCR tool-injection paths and opt-in retrieval instructions; tool ownership and instruction naming only.
  • Minimum rollout channel: unchanged; no feature is enabled by this patch.
  • Stable/default behavior changed: stop injecting a duplicate bare schema when the client already owns a supported retrieval alias.
  • Kill switch / disable path: existing controls unchanged; no new switch.
  • Unsafe override required: none.
  • Qualification impact: alias/provider/cold-to-compressed regressions, foreign-server and actual-store boundaries, CLI/native proof and both-provider ASGI/native/HTTP proof above; no widened rollout.
  • Rollback path: revert this Python bug fix.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • One shared ownership selector, broader compression exclusions unchanged, no new dependency
  • Documentation updated
  • Consumer-visible failing-before/passing-after regressions included
  • Relevant new and existing tests pass locally
  • Did not edit generated CHANGELOG.md

Mandatory Local Push Gates

Normal git push fork fix/issue-4159-client-retrieval-alias-20261011
executed the installed pre-push hook and make ci-precheck passed:

  • cargo fmt --all -- --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace
  • Genuine editable Rust extension build, install and import verification
  • Critical Python/native parity suite: 177 passed, 4 skipped, 1 warning
  • Conventional commit validation through commitlint

The 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 440fd96aaa11c0ea9263d9628ccc2b5f8b3865d4 also
passed every normal pre-push gate above, including 177 critical Python
tests. This run used RUST_TEST_THREADS=2: the preceding default-parallel
attempt failed the unchanged native
expired_get_does_not_wipe_concurrent_refresh test's final-live assertion
with 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 c5c36ba0e7074cbd815498d00ee8f394bd7287bc
again 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.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@github-actions github-actions Bot added the status: needs author action Pull request body or readiness checklist still needs author updates label Oct 11, 2026
@oxura

oxura commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

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.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates labels Oct 11, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@codecov-commenter

codecov-commenter commented Oct 11, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@oxura
oxura marked this pull request as draft October 11, 2026 17:53
@github-actions github-actions Bot removed the status: ready for review Pull request body is complete and the author marked it ready for human review label Oct 11, 2026
@oxura

oxura commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor Author

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 (admin, maintain, push false), so cannot install the organization/repository app. No coverage gates or settings were weakened.

@oxura
oxura marked this pull request as ready for review October 11, 2026 18:20
@github-actions github-actions Bot added the status: ready for review Pull request body is complete and the author marked it ready for human review label Oct 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready for review Pull request body is complete and the author marked it ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenCode: injected headroom_retrieve is passed to the client on mixed-tool turns, but the client only has headroom_headroom_retrieve

2 participants