-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(chat): defer optional native-main helper claims (carry #4118) #4338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Cache lane roadmap | ||
|
|
||
| Three independent fixes address optional helper admission, final OpenCode Go conversation affinity, and explicitly enabled Claude instruction stabilization. Hermes cache observations are investigated separately: missing inbound identity is not proof of proxy loss, and a shared prefix is not a conversation. | ||
|
|
||
| Satisfy-spec HOTL, triggered by the authorized cache lane assignment. Scope: PRs #4118/#4050/#4052 and issue #3433. No local tests of any size, build/typecheck/install, service changes, merges, closures, releases, workflow or permission changes. Commits and --no-verify pushes plus ordinary PR creation are authorized. Existing tool/account scope only; no user-set time/token/agent cap. Main implements; inherited-model subagents review. Native architect selection is unavailable; supported independent design review records that limitation. | ||
|
|
||
| Verification: git diff --check for textual integrity; independent source review; GitHub hosted Cross-platform CI at each independent final PR tip. Local product checks are NOT RUN. Source/applicability checks do not prove runtime behavior. Stop after concrete dispositions, final hosted evidence and durable handoff; field evidence or review/access gaps remain explicit, never a false fix. Tool gate denial is reported without bypass. Two failed independent reviewer contexts return work to main; implementation remains main-owned. | ||
|
|
||
| Existing layout: src/server (wire bridges), src/claude (translator), src/providers (Go transport), tests/{responses,providers,claude-integration,codex-integration}, structure (contracts), docs-site (user guidance). Reuse these owners; no new framework or runtime abstraction. | ||
|
|
||
| Work phases, each a full P-A-B-C-D cycle: | ||
| - roadmap: docs only; lock all following plans. | ||
| - claim: 010, independent dev PR for #4118. | ||
| - affinity: 020, independent dev PR for #4050; prerequisite request-lane allocator is already on dev. | ||
| - prefix: 030, independent dev PR for #4052 with an actual default-off configuration boundary. | ||
| - hermes: 040, independent contract evidence for #3433, no invented identity. | ||
| - verify: 050, inspect hosted results, repair confirmed scoped failures in added cycles, hand off exact heads. | ||
|
|
||
| The implementation order is a work ledger, not a false PR dependency. No native stack requested. Each independent PR is its own final tip. A repair that depends on a delivered implementation may be a child layer. | ||
|
|
||
| Source inventory and raw latest GitHub evidence stay in .tmp/cache-handoff/. Public source PRs are the provenance; measurements are author-reported and are not reproduced here. Unpublished security notes stay in scratch. Source ownership updates accompany each actual patch. | ||
|
|
||
| Design disposition: accept CACHE-D01 through D05. D04 uses the existing Claude configuration argument as its single control; no separate conflicting translator option. D05 covers underscore session_id and hyphenated session/thread pair separately. Native architect role not selected; inherited supported subagent performed actual design review. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Claim deferral | ||
|
|
||
| Prerequisite: roadmap; origin/dev baseline. Independent PR. Carry source #4118 at fc8c03833e9ffd0f2bfd30f5ef7de19425c87645, preserving author trailers. | ||
|
|
||
| - MODIFY `docs-site/src/content/docs/fr/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/ja/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/ko/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/ru/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/tr/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/zh-cn/reference/proxy-formats.md` | ||
| - MODIFY `docs-site/src/content/docs/zh-tw/reference/proxy-formats.md` | ||
| - MODIFY `src/server/chat-completions.ts` | ||
| - MODIFY `src/server/responses/core.ts` | ||
| - MODIFY `src/vision/plan.ts` | ||
| - MODIFY `src/web-search/index.ts` | ||
| - MODIFY `structure/providers/openai-tiers.md` | ||
| - MODIFY `tests/codex-integration/bearer-admission-routed-provider.test.ts` | ||
| - MODIFY `tests/vision/vision-cache.test.ts` | ||
| - MODIFY `tests/web-search/web-search.test.ts` | ||
|
|
||
| Before: caller-auth noncanonical Chat eagerly claims stored main; helper admission does not share all terminal/routed/search exclusions. After: only non-caller-auth keeps early enrichment; carry `allowStoredOpenAiSidecarAuth` privately, then claim before reading main only when a canonical Direct helper candidate is actually needed. Snapshot stays separate from primary/retry credentials. Share routed-vision eligibility and tool-choice exclusions. Preserve loopback hostname/listener fields. | ||
|
|
||
| Activation: held keyless Cursor request without helper leaves main request count zero and profile switch succeeds; Direct helper carries main only to helper wire; Pool/exact account and excluded tool choices retain behavior. Auth review required. | ||
|
Comment on lines
+22
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: set -eu
printf '%s\n' '--- target test file ---'
if [ -f tests/codex-integration/bearer-admission-routed-provider.test.ts ]; then
wc -l tests/codex-integration/bearer-admission-routed-provider.test.ts
rg -n -C 5 'allowStoredOpenAiSidecarAuth|stored-main|Authorization|authorization|Cursor|Direct|redirect|failover|combo|route' tests/codex-integration/bearer-admission-routed-provider.test.ts
else
printf '%s\n' 'missing target test file'
fd -i 'bearer.*admission.*routed.*provider|routed.*provider.*bearer' tests || true
fi
printf '%s\n' '--- relevant implementation continuation ---'
sed -n '2230,2315p' src/server/responses/core.ts
printf '%s\n' '--- repository status and nearby test files ---'
git status --short
fd -t f -a tests | rg 'codex-integration|bearer-admission|routed-provider' | head -80Repository: lidge-jun/opencodex Length of output: 47808 🏁 Script executed: set -eu
printf '%s\n' '--- existing canonical and combo coverage ---'
sed -n '800,875p' tests/codex-integration/bearer-admission-routed-provider.test.ts
sed -n '998,1040p' tests/codex-integration/bearer-admission-routed-provider.test.ts
printf '%s\n' '--- redirect coverage in the named test ---'
rg -n -i -C 4 'redirect|https|destination|location' tests/codex-integration/bearer-admission-routed-provider.test.ts || true
printf '%s\n' '--- transport redirect handling near upstream requests ---'
rg -n -i -C 4 'redirect|follow:|location' src/server src/providers src | head -160 || trueRepository: lidge-jun/opencodex Length of output: 26496 🏁 Script executed: set -eu
printf '%s\n' '--- OpenAI Responses transport and redirect policy ---'
rg -n -i -C 6 'openai-responses|providerOutbound|redirect:|fetch\(' src/adapters src/providers src/server | rg -i 'openai|responses|providerOutbound|redirect|fetch' | head -240
printf '%s\n' '--- exact transport helpers ---'
rg -n 'providerOutboundPost|providerRedirectError|redirect: "manual"|redirect: "follow"' src/lib src/server src/adapters src/providersRepository: lidge-jun/opencodex Length of output: 26864 🏁 Script executed: set -eu
printf '%s\n' '--- Responses fetch helper ---'
sed -n '1,230p' src/server/responses/fetch-helpers.ts
printf '%s\n' '--- Responses upstream dispatch ---'
sed -n '4235,4305p' src/server/responses/core.tsRepository: lidge-jun/opencodex Length of output: 12757 Sensitive Data Exposure Reachability: External Add a redirect regression test. Existing tests cover the keyless Cursor, combo/failover, and final Direct paths. Add one test that returns a 3xx response and asserts that the redirect target is not contacted, the response remains manual, and stored-main 🤖 Prompt for AI Agents |
||
|
|
||
| Exact executable delta is the public diff at https://github.com/lidge-jun/opencodex/pull/4118.diff captured locally in .tmp/cache-handoff/pr-4118.diff; git apply --check exited 0 on baseline. Read and adapt source context before application. No source deletion. Add concise current-contract references to all mapped source ownership docs, with canonical details in structure/data-planes/inbound-compat.md and structure/providers/openai-tiers.md (claim) or structure/transports/responses.md (affinity). | ||
|
|
||
| C: git diff --check plus independent review; local tests NOT RUN. Runtime acceptance deferred to final hosted tip CI. D records implementation and pending remote evidence, not test success. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Final Go affinity | ||
|
|
||
| Prerequisite: roadmap; origin/dev baseline. Independent PR. Carry source #4050 at e5c2411f7b35c6265aacce19f66f13eace544579, preserving author trailers. | ||
|
|
||
| - MODIFY `docs-site/src/content/docs/guides/providers.md` | ||
| - MODIFY `src/server/claude-messages.ts` | ||
| - MODIFY `src/server/responses/core.ts` | ||
| - MODIFY `tests/providers/opencode-go-session-header.test.ts` | ||
|
|
||
| Before: preliminary Claude route injects Go identity into replay headers. After: derive validated lane with explicit session > Go header > valid Claude metadata > original request allocation, carry `claudeGoAffinity` in HandleResponsesOptions through combo recursion and consume only at final Go normalization. Never synthesize shared system hash identity or leak Go-only headers to non-Go. | ||
|
|
||
| Activation: existing two-wire/random/failover matrix gains metadata, explicit-header precedence, malformed/shared identity and independent sessionless controls; operator override wins. No public option or serialization: private in-memory options, recursion spreads options, final transport consumes. | ||
|
|
||
| Exact executable delta is the public diff at https://github.com/lidge-jun/opencodex/pull/4050.diff captured locally in .tmp/cache-handoff/pr-4050.diff; git apply --check exited 0 on baseline. Read and adapt source context before application. No source deletion. Add concise current-contract references to all mapped source ownership docs, with canonical details in structure/data-planes/inbound-compat.md and structure/providers/openai-tiers.md (claim) or structure/transports/responses.md (affinity). | ||
|
|
||
| C: git diff --check plus independent review; local tests NOT RUN. Runtime acceptance deferred to final hosted tip CI. D records implementation and pending remote evidence, not test success. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Explicit Claude prefix stabilization | ||
|
|
||
| Prerequisite roadmap; independent dev PR. Reimplement #4052 at 43def4039ba60039df9a2a91fb6352b91ba74d70; preserve Warexpor and Cursor Agent credit. Do not copy binary paper or unverified measurements. | ||
|
|
||
| NEW src/claude/inbound-cache-stabilize.ts: adopt source helper's complete trailing exact total_tokens/two TaskCreate matchers and fenced-range parser, including unclosed fence through EOF. Source full text is in .tmp/cache-handoff/pr-4052.diff. MODIFY src/claude/inbound.ts: read `cc?.stabilizePromptCache === true` from the existing Claude config parameter, defaulting stabilization off, relocate only when true, append latest dynamic notice as user input, use stabilized instructions for opted-in Desktop cache key; preserve original systemParts hashing otherwise. | ||
|
|
||
| MODIFY src/types/config.ts OcxClaudeCodeConfig: add `stabilizePromptCache?: boolean` with default false and role-change warning. Serialization/deserialization: existing config JSON save/load retains the boolean; no new wire option; malformed non-true values do not activate. KEEP src/server/claude-messages.ts existing three-argument translation call, which already passes config.claudeCode. Never use unconditional true or infer opt-in from metadata, endpoint or text. Configuration is operator-owned and opt-in applies to translated Messages traffic; native passthrough stays unchanged. | ||
|
|
||
| NEW tests/claude-integration/claude-inbound-cache-stabilize.test.ts: adopt translator/helper controls; replace source-phrase assertion with real handler outbound capture proving default/unset/false retain exact suffix and original key, true relocates, fences preserve content, both TaskCreate shapes peel, metadata session key stays stable. MODIFY scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json to register new file. Add save/load and malformed-value control where existing config fixture permits. | ||
|
Comment on lines
+7
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🤖 get_repo_knowledge executed:
Length of output: 18908 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate plan ---'
cat -n devlog/_plan/260912_cache_lane/030_prefix.md | sed -n '1,40p'
printf '%s\n' '--- config files ---'
fd -t f -i 'config' src tests scripts | head -80
printf '%s\n' '--- config symbol references ---'
rg -n --glob '!node_modules' --glob '!dist' 'OcxClaudeCodeConfig|claudeCode|serialize|deserialize|save.*config|load.*config|config.*JSON|JSON.*config' src tests scripts | head -240
printf '%s\n' '--- config type outline ---'
if [ -f src/types/config.ts ]; then
ast-grep outline src/types/config.ts
wc -l src/types/config.ts
fiRepository: lidge-jun/opencodex Length of output: 39559 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- OcxClaudeCodeConfig ---'
cat -n src/types/config.ts | sed -n '1,145p'
printf '%s\n' '--- config module outline ---'
ast-grep outline src/config.ts
printf '%s\n' '--- config persistence symbols ---'
rg -n '^(export )?(function|const) (loadConfig|saveConfig|mutatePersistedConfig|getConfigPath)|loadConfig|saveConfig|config\.json|JSON\.parse|JSON\.stringify' src/config.ts tests/config/config-save-boundary.test.ts tests/config/config-load-degrade.test.ts tests/claude-integration/claude-messages-endpoint.test.ts | head -220
printf '%s\n' '--- config module persistence sections ---'
rg -n -A45 -B12 'function loadConfig|function saveConfig|const loadConfig|const saveConfig|mutatePersistedConfig|getConfigPath' src/config.ts
printf '%s\n' '--- focused existing round-trip/raw JSON tests ---'
cat -n tests/config/config-load-degrade.test.ts | sed -n '1,260p'
cat -n tests/config/config-save-boundary.test.ts | sed -n '1,260p'Repository: lidge-jun/opencodex Length of output: 50375 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- Claude-related schema and normalization ---'
sed -n '1170,1215p' src/config.ts
sed -n '2088,2145p' src/config.ts
printf '%s\n' '--- loadConfig ---'
sed -n '2380,2498p' src/config.ts
printf '%s\n' '--- defaults and persistence ---'
sed -n '2638,2660p' src/config.ts
sed -n '3358,3462p' src/config.ts
printf '%s\n' '--- existing config load-degrade tests ---'
cat -n tests/config/config-load-degrade.test.ts | sed -n '1,180p'Repository: lidge-jun/opencodex Length of output: 23366 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- claudeCode schema references ---'
rg -n -A18 -B8 'claudeCode' src/config.ts | head -180
printf '%s\n' '--- getDefaultConfig ---'
sed -n '4010,4055p' src/config.ts
printf '%s\n' '--- direct schema field context near claudeCode ---'
python3 - <<'PY'
from pathlib import Path
lines = Path("src/config.ts").read_text().splitlines()
for i, line in enumerate(lines, 1):
if "claudeCode:" in line:
print(f"{i}: {line}")
for n in range(max(1, i - 8), min(len(lines), i + 20) + 1):
print(f"{n}: {lines[n - 1]}")
PYRepository: lidge-jun/opencodex Length of output: 15869 Make
🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| MODIFY docs-site/src/content/docs/guides/claude-code.md and applicable translated pages: document default-off claudeCode.stabilizePromptCache, changed message role/key scope and no hit-rate guarantee. MODIFY structure/data-planes/inbound-compat.md/config.md and other mapped source-owner docs with canonical contract references. No GUI control, new dependency, automatic user config edit or cache-key-to-session synthesis. | ||
|
|
||
| C: textual diff/source review only locally; all product tests NOT RUN. Hosted tests must observe actual outbound instructions/input/key, not phrase presence. Independent reviewer must confirm that earlier #4052 HTTP activation blocker is resolved. D publishes exact implementation with remote evidence pending until verification phase. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Hermes identity boundary | ||
|
|
||
| Prerequisite roadmap; independent path from Claude changes. Latest issue #3433 comment 5556427205 and controlled sample 5551855276 establish no measured inbound identity, not a dropped value. Preserve issue OPEN disposition. | ||
|
|
||
| MODIFY tests/responses/chat-completions-endpoint.test.ts or a registered adjacent contract file: use the existing real Chat handler + mocked Responses upstream. Send synthetic session A on two growth turns and session B on a fresh turn; cross body prompt_cache_key present/absent with session_id present/absent. Assert captured outbound session_id and body key are exactly caller supplied; absent remains absent; shared key is not converted into session_id. Use fixture identity distinct from raw personal data, and compare at actual adapter fetch boundary. Existing src/chat/inbound.ts copies prompt_cache_key; Chat FORWARD_HEADERS and openai-responses adapter forward session_id. No runtime mutation unless this controlled contract reveals a specific defect. | ||
|
|
||
| MODIFY canonical inbound contract docs to distinguish stable client conversation identity, request-scoped lane and prompt prefix. Durable scratch evidence names public comment URLs, actual test command coverage and limitations. Real Hermes same-conversation/fresh-session identifier and outbound capture from its running client are unavailable unless provided by existing public evidence; synthetic regression proves transport contract only. Do not claim actual client identity was observed, cache hits improved or #3433 solved. | ||
|
|
||
| C hosted final tip executes the contract; local suite NOT RUN. D records exactly what is proven and remaining controlled live-client comparison. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Hosted verification and delivery | ||
|
|
||
| Prerequisites: independent implementation PRs. NEW durable .tmp/cache-handoff/050_handoff.md records actual worktree, branches/PR URLs/full head SHA, source dispositions/credits, cycle receipts, remaining acceptance and reviews. Capture gh pr view/checks and gh run view JSON at each final independent tip; ordinary manual children only for real correction dependencies. No native membership mutation. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
path=".tmp/cache-handoff/050_handoff.md"
test -f "$path"
if git check-ignore -q "$path"; then
echo "$path is ignored" >&2
exit 1
fi
git ls-files --error-unmatch "$path" >/dev/nullRepository: lidge-jun/opencodex Length of output: 157 🏁 Script executed: #!/usr/bin/env bash
set -u
path=".tmp/cache-handoff/050_handoff.md"
printf '%s\n' '--- target ---'
if [ -e "$path" ]; then
ls -l "$path"
sed -n '1,120p' "$path"
else
echo "MISSING: $path"
fi
printf '%s\n' '--- ignore status ---'
if git check-ignore -v "$path"; then
:
else
status=$?
echo "git check-ignore status: $status"
fi
printf '%s\n' '--- tracked status ---'
git ls-files --stage -- "$path" || true
printf '%s\n' '--- nearby tracked files ---'
git ls-files --stage -- '.tmp/cache-handoff' 'devlog/_plan/260912_cache_lane' | sed -n '1,120p'
printf '%s\n' '--- repository status ---'
git status --short --untracked-files=all -- "$path" '.tmp/cache-handoff' 'devlog/_plan/260912_cache_lane'Repository: lidge-jun/opencodex Length of output: 938 🤖 get_repo_knowledge executed:
Length of output: 5503 Move the handoff to a tracked location before using it for delivery decisions.
🤖 Prompt for AI Agents |
||
|
|
||
| No product source changes planned here. If CI exposes a scoped defect, append a numbered repair plan and full PABCD cycle before implementation, then verify new exact head. Hosted workflow definition determines jobs actually executed; skipped/cancelled runs are never passes. No automatic workflow cancellation or protection edit. Local tests/build/typecheck/install remain NOT RUN. Only source/diff checks may be wrapped in cxc receipt and must retain their true label. | ||
|
|
||
| C: final head matches hosted run headSha; successful required jobs and skipped jobs recorded individually. D: finish handoff with source review gaps and Hermes field residual, no merge/issue closure. Parent decides integration. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge lidge-jun/opencodex /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/learnings /tmp/coderabbit-repo-knowledge/lidge-jun-opencodex-7afea732/conventionsLength of output: 18868
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 50378
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 182
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 50378
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 5717
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 40613
Complete the documentation map and record translation follow-up.
At
devlog/_plan/260912_cache_lane/010_claim.md:5-12, retain the eight proxy-format pages and add the canonicaldocs-site/src/content/docs/guides/providers.mdanddocs-site/src/content/docs/reference/adapters.md. These pages define authentication,FORWARD_HEADERS, sidecar authority, and stored-main behavior. Record the correspondingfr,ja,ko,ru,tr,zh-cn, andzh-twpages for translation review.At
devlog/_plan/260912_cache_lane/020_affinity.md:5-8, enumeratedocs-site/src/content/docs/{fr,ja,ko,ru,zh-cn,tr,zh-tw}/guides/providers.md. The Traditional Chinese guide already documents session affinity atzh-tw/guides/providers.md:177-182, so check it against the new precedence rules. For pages that omit the new affinity details without contradicting English, record translation as follow-up instead of requiring a same-PR translation.📍 Affects 2 files
devlog/_plan/260912_cache_lane/010_claim.md#L5-L12(this comment)devlog/_plan/260912_cache_lane/020_affinity.md#L5-L8🤖 Prompt for AI Agents
Source: Path instructions