Skip to content

fix(command-code): preserve conversation cache affinity - #3581

Closed
yansigit wants to merge 1 commit into
lidge-jun:devfrom
yansigit:codex/upstream-command-code-cache-affinity
Closed

fix(command-code): preserve conversation cache affinity#3581
yansigit wants to merge 1 commit into
lidge-jun:devfrom
yansigit:codex/upstream-command-code-cache-affinity

Conversation

@yansigit

@yansigit yansigit commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Derive Command Code x-session-id from opaque, provider-scoped trusted conversation identity so retries and related calls retain cache affinity.
  • Prefer the client thread, then replay scope, then an explicitly non-shared prompt cache key; shared or unclassified cache keys and prompt text never create cross-conversation affinity.
  • Preserve the internal cohort marker through request reparsing and enable prompt-cache-key forwarding only for the Command Code API-key registry entry.
  • Fall back to a fresh random UUID when no trusted identity is available. No raw identity is sent or logged.

Verification

  • Exact head f60397d34, rebased onto current dev be81013fa.
  • bun test tests/providers/command-code-provider.test.ts tests/providers/commandcode-provider.test.ts tests/claude-integration/claude-code-thought-signature-scope.test.ts — 49 pass, 0 fail.
  • bun run typecheck — pass.
  • bun run privacy:scan — pass.
  • git diff --check — pass.
  • bun run test — 18,244 pass, 14 skip, 0 fail across 1,077 files.
  • Correctness and security reviews completed; shared/unclassified prompt-cache cohorts fail closed to random IDs.
  • No GUI change; screenshot not applicable.

Checklist

Scope is limited to Command Code affinity and its request plumbing; no unrelated cleanup is included. Documentation is unchanged because this corrects internal transport affinity without adding user configuration. Security-sensitive identity/cohort behavior received an independent review.

Review readiness checklist

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

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • New Features

    • Command Code requests now use stable, opaque session identifiers when trusted conversation context is available.
    • Command Code now supports forwarding prompt cache keys in chat completion requests.
  • Bug Fixes

    • Improved session continuity across compatible conversation and replay scenarios.
    • Shared or unavailable cache identities no longer produce misleading stable session identifiers.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 5, 2026
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 3bda86d2-306c-4dc6-bfb5-10f3670ec495

📥 Commits

Reviewing files that changed from the base of the PR and between 0d396f0 and f60397d.

📒 Files selected for processing (3)
  • src/adapters/command-code.ts
  • src/providers/registry.ts
  • src/server/responses/core.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Command Code now derives stable session IDs from trusted conversation identity, preserves prompt-cache cohort metadata during request parsing, and forwards configured prompt cache keys to Chat Completions requests.

Changes

Command Code session affinity

Layer / File(s) Summary
Request identity propagation
src/types/request.ts, src/server/responses/core.ts, tests/claude-integration/claude-code-thought-signature-scope.test.ts
OcxParsedRequest records whether prompt_cache_key belongs to a shared cohort. The marker persists through encrypted-task recovery. Tests cover per-session, shared-cohort, and absent keys.
Deterministic session header
src/adapters/command-code.ts, tests/providers/command-code-provider.test.ts
commandCodeSessionId hashes trimmed client-thread, replay-scope, or eligible prompt-cache identity into a UUID-shaped value. buildRequest sends it as x-session-id. Requests without trusted identity use randomUUID().
Prompt cache request forwarding
src/providers/registry.ts, tests/providers/commandcode-provider.test.ts
The Command Code provider enables promptCacheKey. Tests verify registry configuration and serialization as prompt_cache_key.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f6039

Command Code now maintains cache affinity only for trusted conversation identities while keeping shared or unclassified cache keys isolated. The covered behavior is ready to merge with no active current-head risk identified.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RequestParser
  participant CommandCodeAdapter
  participant CommandCodeAPI
  Client->>RequestParser: send thread and prompt-cache identity
  RequestParser->>CommandCodeAdapter: provide OcxParsedRequest with cohort marker
  CommandCodeAdapter->>CommandCodeAdapter: derive commandCodeSessionId
  CommandCodeAdapter->>CommandCodeAPI: send x-session-id and prompt_cache_key
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Command Code fix and its primary purpose: preserving conversation cache affinity. It is concise, specific, and matches the session ID and prompt cache changes.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft September 5, 2026 02:21
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

이 PR은 Command Code 어댑터가 매 요청마다 x-session-id를 새 UUID로 찍어 버리던 문제를 고칩니다. 지금 devsrc/adapters/command-code.ts는 헤더에 randomUUID()만 넣고 있어서, 같은 대화의 재시도·이어붙이기가 업스트림 워커 친화(캐시 affinity)를 잃습니다. Cursor 쪽 수송(live-transport.ts)은 이미 세션을 안정적으로 유지하는데, Command Code만 매번 새로 뽑고 있었습니다.

고치는 방식은 쉽습니다. 믿을 수 있는 대화 정체성만 골라 SHA-256으로 불투명 UUID처럼 보이게 만든 뒤 x-session-id로 보냅니다. 우선순위는 (1) _clientThreadId, (2) 리플레이 스코프의 thread id, (3) “공유 코호트가 아님”이 확실한 promptCacheKey입니다. 공유/미분류 캐시 키나 프롬프트 본문으로 affinity를 만들지 않고, 믿을 키가 없으면 예전처럼 랜덤 UUID로 떨어집니다. 원문 정체성은 헤더·로그에 안 나갑니다.

함께 손본 배선도 중요합니다. src/server/responses/core.ts에서 options.promptCacheKeyIsSharedCohort를 파싱 결과에 붙이고, 암호화 복구 후 재파싱 때 그 플래그를 보존 목록에 넣습니다. src/types/request.ts에 필드가 생기고, API-key 레지스트리 항목 commandcode에만 promptCacheKey: true를 켜서 chat completions로 키를 넘기게 합니다. OAuth 쪽 command-code 어댑터 세션과 API-key 쪽 캐시 키 포워딩을 나눈 설계입니다.

지금 dev 방향(콤보/OAuth 페일오버, Responses 스플릿 열차 S07)과는 파일 겹침이 거의 없습니다. 다만 Responses 코어의 kept-keys 목록과 types/request.ts는 다른 기능 PR도 자주 만지므로, 머지 직전에 dev 리베이스만 한 번 보면 됩니다. 작성자 로컬에서는 포커스 테스트 49개와 전체 게이트가 초록이었고, GitHub에는 아직 draft이며 readiness 체크리스트가 비어 있습니다.

경로 src/adapters/command-code.ts commandCodeSessionId - 공유 코호트(_promptCacheKeyIsSharedCohort === true)와 미설정(undefined)을 둘 다 affinity에서 제외한 것은 맞습니다. 실수로 === false만 허용하지 않으면 예전처럼 대화가 섞입니다.
경로 src/server/responses/core.ts - 재파싱 kept 목록에 _promptCacheKeyIsSharedCohort를 넣은 이유는 복구 경로에서 플래그가 사라지면 cache-key affinity가 다시 열릴 수 있기 때문입니다. 이 한 줄이 보안 경계입니다.
경로 src/providers/registry.ts commandcode - promptCacheKey: true는 API-key 항목에만 있습니다. OAuth command-code에는 안 켠 것이 본문 의과 일치하는지 머지 전 한 번 더 확인하세요.
경로 테스트 tests/providers/command-code-provider.test.ts - “같은 identity면 같은 id / 공유·미분류면 매 호출 다른 id / raw 문자열이 id에 안 박힘”이 핵심 회귀입니다. CI에서 이 두 테스트가 깨지면 머지하지 마세요.
경로 PR 상태 - draft이고 readiness 체크리스트 네 칸이 비어 있습니다. 기능은 좋아 보이지만, 레포 규칙상 draft를 바로 admin 머지하면 안 됩니다.

메인테이너의 판단이 필요한 지점

  • draft를 Ready로 올린 뒤 exact-head CI를 기다릴지, 아니면 체크리스트만 채우고 바로 리뷰 큐에 넣을지
  • OAuth command-code에도 캐시 키 포워딩이 필요한지(지금은 API-key만)
  • 해시로 만든 UUID 형태가 Command Code 업스트림 형식 검증을 깨지 않는지(로컬/실계정 한 번 스모크)

너의 추천
내용은 합쳐도 될 수준입니다. 다만 지금은 draft이니 Ready로 바꾸고 CodeRabbit/Codex 지적을 정리한 뒤, exact-head CI 초록이면 squash 합치세요. types/config 분리에 무효화되는 PR이 아닙니다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

지금 dev HEAD는 a594a7f21이고, 방금 올라온 방향은 콤보 failover blast-radius(#3565), OAuth 시작 시 설정 재조정(#3564), Claude 라우팅 끄면 native 실행(#3563) 쪽이다. 이 PR은 그 열차와 겹치지 않는다. Command Code만 손본다.

지금 devsrc/adapters/command-code.ts를 보면 buildRequest가 요청마다 x-session-idrandomUUID()를 넣는다. 같은 대화를 다시 보내도 업스트림 입장에서는 매번 새 세션이다. 캐시 친화(affinity)가 끊기고, 재시도·이어서 말하기가 비싸지거나 느려질 수 있다. 이 PR은 그걸 고친다.

고치는 방법은 세 갈래다. 첫째, 믿을 수 있는 대화 정체성에서 안개처럼 가린(해시) 세션 id를 만든다. 우선순위는 _clientThreadId(클라이언트 스레드) → _reasoningReplayScope.clientThreadId(리플레이 범위) → 공유 코호트가 아니라고 표시된 promptCacheKey 순이다. 공유 코호트이거나 분류가 없으면 일부러 다시 randomUUID()로 떨어뜨린다. 서로 다른 대화를 한 워커에 묶지 않으려는 안전 장치다. 둘째, src/types/request.ts_promptCacheKeyIsSharedCohort를 두고 src/server/responses/core.ts에서 파싱 직후 복사한 뒤, encrypted-task 재파싱 때 보존 목록에도 넣는다. 셋째, API 키 레지스트리 항목 commandcode에만 promptCacheKey: true를 켠다. OAuth 항목 command-code는 proprietary adapter의 x-session-id로 가고, Chat Completions의 prompt_cache_key 전달은 API 키 경로(openai-chat)에만 연다. 설명과 테스트가 그 경계를 맞춘다.

테스트도 핵심을 찌른다. 같은 스레드면 id가 같고, 원문 비밀값은 UUID 문자열에 안 나오고, 공유/미분류 캐시 키와 프롬프트 본문만으로는 affinity가 생기지 않으며, 레지스트리 플래그가 켜지면 바디에 prompt_cache_key가 실리는지까지 본다. Claude thought-signature 쪽에도 코호트 마커 전달 단언이 추가됐다. 로컬에서 관련 스위트·typecheck·privacy·전체 게이트를 돌렸다고 본문에 적혀 있고, CodeRabbit도 액션 가능한 코멘트 없이 통과했다.

types.ts/config.ts 대형 분할 열차와는 무관하다. 닫고 리베이스할 대상이 아니다. 중복 PR로 보이는 열린 짝도 지금 검색에서는 없다.

라인 - 코드 자체보다 상태: 이 PR은 아직 draft이고, readiness checklist의 “Codex/CodeRabbit 정리 끝”, “리뷰 준비됨” 칸이 비어 있다. 머지 후보로 올리기 전에 undraft가 먼저다.

src/adapters/command-code.ts commandCodeSessionId - UUID 모양을 맞추려고 해시 글자 중 인덱스 12·16을 건너뛰고 버전/variant 니블을 4/8로 고정한다. 동작은 맞고 테스트도 UUID 정규식을 통과하지만, 왜 건너뛰는지 한 줄 주석이 없으면 나중에 “버그로 hex를 빠뜨렸다”고 오해하기 쉽다.

src/providers/registry.ts commandcode 항목 - promptCacheKey: true는 API 키 preset에만 들어간다. OAuth command-code 항목에는 없다. PR 본문 의도(API 키 Chat 경로만)와 일치하는지 메인테이너가 한 번만 확인하면 된다. OAuth 쪽 affinity는 x-session-id 해시로 충분하다.

경로 promptCacheKey 미분류 경로 - Responses 등에서 prompt_cache_key만 오고 _promptCacheKeyIsSharedCohort === false가 명시되지 않으면 fail-closed로 랜덤 id가 된다. 보안상 맞는 기본값이다. 다만 “대화별 키인데 플래그만 없는” 클라이언트가 Command Code API 경로에서 affinity를 못 받을 수 있다. _clientThreadId가 있으면 문제없고, 캐시 키만 믿는 경로가 얼마나 필요한지가 판단 지점이다.

메인테이너의 판단이 필요한 지점

  • draft를 언제 undraft할지, checklist를 기여자에게 다시 채우게 할지
  • 미분류 prompt_cache_key를 계속 fail-closed로 둘지, Responses 전용으로 “키가 있으면 conversation으로 본다” 예외를 둘지
  • commandcode에만 promptCacheKey: true인 범위가 제품 의도와 일치하는지(OAuth는 x-session-id만)

너의 추천
CI(hygiene/label/enforce-target)와 CodeRabbit은 이미 통과했다. 기여자에게 draft 해제·checklist 완료를 요청하고, undraft 뒤 full gate가 초록이면 dev에 독립 버그픽스로 머지해도 된다. S0x 분할·Windows 스위트 열차와 충돌하지 않으니 닫을 이유는 없다. UUID 니블 주석 한 줄은 있으면 좋고, 없어도 머지를 막을 정도는 아니다.

이 댓글은 grok-bot이 작성했습니다

@yansigit
yansigit force-pushed the codex/upstream-command-code-cache-affinity branch from 0d396f0 to 8ef7cde Compare September 5, 2026 03:30
@yansigit

yansigit commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Update after the maintainer and automated reviews:

  • Added the requested inline explanation for the intentionally replaced UUID version/variant nibbles.
  • Kept promptCacheKey: true scoped to the API-key commandcode preset. The OAuth command-code adapter uses the hashed x-session-id path, so enabling Chat Completions cache-key forwarding there would mix two different transports without a demonstrated need.
  • Kept unclassified prompt_cache_key values fail-closed. Only keys explicitly classified as non-shared may create affinity; broadening this would risk cross-conversation worker affinity.
  • Rebased onto current dev 3191fe1aa; exact PR head is 8ef7cde62.
  • Exact-head focused tests (49), typecheck, privacy scan, and diff check pass.
  • The current full suite reports 18,038 pass / 14 skip / 4 fail. I reproduced all four failures unchanged on the exact dev base: three quota-reset route-registry assertions and one quota-reset notification activation assertion. They are upstream-base regressions introduced outside this PR, so I left the local-CI and ready-for-review boxes open rather than making an inaccurate readiness attestation.

CodeRabbit produced no actionable comments. The docstring-coverage warning is non-blocking here: the only new exported helper already has a descriptive name and focused behavioral tests, while the requested UUID-shaping clarification is now directly beside the non-obvious expression.

@yansigit
yansigit force-pushed the codex/upstream-command-code-cache-affinity branch from 8ef7cde to f60397d Compare September 5, 2026 05:02
@yansigit
yansigit marked this pull request as ready for review September 5, 2026 05:02
@yansigit

yansigit commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased cleanly onto current upstream dev (be81013fa), producing exact head f60397d34. The previously observed quota-reset base failures are now fixed upstream: focused tests pass 49/49, typecheck and privacy scan pass, and the full repository suite passes with 18,244 tests, 14 skipped, and 0 failures across 1,077 files. The readiness checklist now reflects this exact head; fresh maintainer review remains the merge gate.

@github-actions
github-actions Bot marked this pull request as draft September 5, 2026 05:32
@yansigit
yansigit marked this pull request as ready for review September 5, 2026 05:37
lidge-jun added a commit that referenced this pull request Sep 5, 2026
Owner-authorized admin integration of the independently reviewed A stack through #3716.

Exact head: 5097e66. All 24 actual full cross-platform producers succeeded in https://github.com/lidge-jun/opencodex/actions/runs/33991642514. Any queued aggregation-only job is accepted only after directly checking its exact allowlist. No pending or failed functional producer of this integration candidate is waived.

Carries #3708 bounded Unix probe cleanup and deterministic verification, #3692 Command Code conversation affinity (source #3581), and #3694 effective provider capability evaluation (source #3671). Earlier failing foundation jobs remain recorded and are superseded by this fully tested descendant. All lower-layer and contributor commits remain ancestors; original author identities are preserved. After dev ancestry verification, originals and superseded review PRs are closed with the actual landing evidence. Related #3661 remains open.

Co-authored-by: SB Yoon <44089734+yansigit@users.noreply.github.com>
Co-authored-by: Hako <25837994+devswha@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Carried through #3692 and integrated by #3716 into dev as a2f69c8aa60976345740ae6f3d2301f89297328e. Original contributor commits and Co-authored-by attribution are preserved. All 24 exact-head functional CI producers passed: https://github.com/lidge-jun/opencodex/actions/runs/33991642514. The carried head and landing commit are verified ancestors of dev.

@lidge-jun lidge-jun closed this Sep 5, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Landed via #3692 at 059de1b (final on dev via #3716 at a2f69c8).

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 5, 2026
lidge-jun added a commit that referenced this pull request Sep 5, 2026
## Summary

- Archive the five completed A runtime/routing work units and record the disposition of original PRs #3672, #3679, #3568, #3581 and #3671. All five originals are closed and their credited changes are on dev.
- Record the actual final stack integration through #3716, dev merge `a2f69c8aa60976345740ae6f3d2301f89297328e`, including GitHub's automatic recognition of folded parents. Related #3661 remains open for the residual recovery scope.
- [Outcome and evidence](https://github.com/lidge-jun/opencodex/blob/5a9476e522c085b3e2308935d89401d07d5ae015/devlog/_fin/260906_a_runtime_stack/090_outcome.md).

## Verification

- Final candidate full CI [33991642514](https://github.com/lidge-jun/opencodex/actions/runs/33991642514): all 24 actual cross-platform producers and aggregate passed.
- Integrated dev CI [33993960826](https://github.com/lidge-jun/opencodex/actions/runs/33993960826): all 17 applicable producers and aggregate passed; two dispatch-only jobs correctly skipped.
- Diff against verified integration is confined to the five A documentation units. Runtime, tests and dependencies are byte-identical. Independent factual, attribution and privacy-content review passed; remote privacy scan passed at exact head `5a9476e522c085b3e2308935d89401d07d5ae015`.
- No local product tests, typechecks or builds were run, as requested by the owner.

## Checklist

- [x] Scope stays focused and avoids unrelated cleanup.
- [x] Docs or release notes were updated when needed.
- [x] Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants