Skip to content

feat(usage): extend opt-in cache continuation diagnostics - #7479

Draft
yuanyuanlove wants to merge 2 commits into
lidge-jun:devfrom
yuanyuanlove:feature-20261021-cache-diagnostics
Draft

yuanyuanlove wants to merge 2 commits into
lidge-jun:devfrom
yuanyuanlove:feature-20261021-cache-diagnostics

Conversation

@yuanyuanlove

@yuanyuanlove yuanyuanlove commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Extend the existing default-off OPENCODEX_CACHE_DEBUG diagnostics with bounded continuation/prefix comparisons, independent session-header observations, and explicitly derived (not proven) affinity-key comparisons. No routing, cache-behavior or performance change is claimed.
  • Address the caller-isolation P2 in the source review: diagnostics now receive the existing authenticated admission principal through the private request-log hook. They no longer derive caller identity from Authorization headers that Chat/Messages bridges may strip or replace.
  • Different admitted keys remain isolated; one key remains the same principal across its supported authentication headers. No authentication decision, accepted-header policy or upstream credential handling changes. The principal is re-tagged into a private WeakMap, never added to a request header, body, log context or diagnostic record.
  • Without a trusted admission principal, including unauthenticated loopback, cross-request comparisons are uncomparable; per-request inbound/outbound comparisons still work. Raw credentials are never used as diagnostic history keys.
  • Preserve the original PR history while incorporating current dev at ceef4889e4666dc0492a375d272eafa5b785361e. The resulting PR diff is limited to the diagnostics enhancement and this identity fix (11 files); no cache-evidence changes from fix(usage): preserve source-aware cache evidence across attempts #7485 are included.

Verification

Validated head: bffa47bed782a5ca7454a3301f776e68cae61e82 (tree 27a118e083171b916aa108706abcf5a7fee22599). All work ran in an isolated Linux cloud workspace. Tests use synthetic callers and provider responses; the focused integration fixtures assert zero external network calls.

  • RED/GREEN reproduction against the exact original head aa3e7fdb0620cec0fa1b72161b005719a0acc781: the new 17-case integration suite had 2 passing authentication-refusal controls and 15 failing history-isolation regressions. All 17 pass after the fix, including after incorporating current dev.
  • Final focused tests run individually with the pinned Bun 1.4.3 test runner: tests/usage/cache-diagnostic.test.ts (34), cache-diagnostic-caller-isolation.test.ts (17), cache-diagnostic-hook-compat.test.ts (10), and tests/responses/protocol-antigravity-direct-eligibility.test.ts (19): 80 passed.
  • Integration coverage uses real admission resolvers, Responses/Chat/Messages handlers and bridges, and synthetic native Responses transport. It covers accepted header aliases, distinct callers, forged identity headers, stripped/replaced bridge Authorization, missing principals, unchanged authentication refusals, and absence of keys/principals/tags from persisted records and forwarded requests.
  • bun run typecheck, bun run privacy:scan, bun run structure:check, and git diff --check passed. Independent read-only review found no remaining actionable issue.
  • bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts tests/lab/core-lab-boundary.test.ts: 61 passed, no caps raised.
  • Backend bundling and native CLI compilation with Bun 1.4.4 succeeded. The compiled executable's isolated --version smoke returned opencodex 2.93.0. This is not a GUI/release-package build.
  • Offline docs build passed with ASTRO_TELEMETRY_DISABLED=1 OPENCODEX_DOCS_OFFLINE=1 bun run build: 593 pages and 84,861 internal links checked.

Remaining coverage: bun run test:changed was blocked by the validation environment's network policy when it attempted an external Google Cloud endpoint; it was not retried or treated as a passing suite. Full-suite, packaged GUI and native macOS/Windows validation were not run. This uses the repository's documented focused-validation scope exception. Keep this Draft pending exact-head CI approval/execution and maintainer privacy/diagnostic-contract review.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.
  • Current-head CI and maintainer acceptance completed.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

✅ Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

Independent source review (gpt-6.1-sol, dispatched by the triage manager, 2026-10-24) at head aa3e7fdb06:

VERDICT: MUST-FIX
LANE_ALL_REQUIRED: no

Reviewed head: aa3e7fdb0620cec0fa1b72161b005719a0acc781.

  • [P2] Caller isolation uses the wrong identity — src/usage/cache-diagnostic.ts:272. codexLineageScopeKey reads only Authorization. Distinct callers authenticated through x-opencodex-api-key or x-api-key (src/server/auth-cors.ts:622–628) therefore share its local scope. Chat/Messages bridges also strip or replace caller Authorization (chat-completions.ts:427, claude-messages.ts:1046). With identical session/model/protocol, unrelated callers consequently share continuation history, producing false comparisons across principals. Fix: capture an opaque authenticated admission identity before bridging and carry it privately into diagnostics. Add regressions for both alternative authentication headers and bridged callers; the direct-helper test at tests/usage/cache-diagnostic.test.ts:373 misses these cases.

Readiness: exact-head CI is action_required; no submitted reviews or unresolved review threads. The branch is 72 commits behind live dev; GitHub reports MERGEABLE. Maintainer privacy/diagnostic-contract review remains outstanding.

Static inspection: template complete; test registration present in both maps; layout 1,999 lines, structure doc 600; no changed-file ratchet violation. No introduced Windows-specific sensitivity warrants mandatory lane=all.

NOT RUN: tests, typecheck, builds, or validation gates, as requested.

This is a read-only source review; merging still requires the gates named above (exact-head CI, lane=all where marked, and maintainer acceptance where required).

Carry the existing admission principal through the private diagnostic hook instead of deriving caller scope from rewritten Authorization headers. Preserve the original PR history and incorporate current dev for validation.

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

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants