Skip to content

fix(responses): recall completed combo routes during compaction - #3971

Merged
lidge-jun merged 9 commits into
devfrom
codex/248-a-combo-8c62
Sep 8, 2026
Merged

fix(responses): recall completed combo routes during compaction#3971
lidge-jun merged 9 commits into
devfrom
codex/248-a-combo-8c62

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

After switching combos, Codex can compact with the last emitted bare model instead of the combo selector. Recall the same lane's last successfully completed combo so v1 and v2 compaction re-enter normal combo selection and failover.

Carry #3891 from 52e66f672d9d30e5dd1188590e669c2bac119540, preserving x3M3x. Correct the unlanded patch's acceptance boundaries: record only accepted completed responses and their client-visible model; preserve explicit configured aliases; reject stale/missing combo targets and late writers after configuration replacement. Failed/incomplete/cancelled attempts retain the previous successful selection. Recall remains process-local, bounded to 256 lanes/30 minutes and stores no credentials.

Reuse existing parsed completion hooks, child commit/discard gating and state-store reconciliation. No additional stream reader, timer or generic subsystem. Add activated compaction, terminal, failover and generation regressions plus English/Korean guidance. This PR contains one coherent bug repair; its size includes tests for the original patch's four correctness gaps.

Verification

  • Local product tests, typecheck, builds and dependency installs: NOT RUN, prohibited by the owner.
  • Main inspected runtime/source scope and target-file equality with the coordinator; Git diff checks passed. Original author/trailer preserved.
  • Fresh independent completion and ownership audits passed after fixing normalized lane fixtures and permanently binding native recall eligibility to the first terminal (including missing/null response envelopes). Follow-up source-oracle and fixture-lifecycle corrections were independently re-reviewed.
  • Current candidate 797a74c646b554a1716905c3721dd13dcef0d579 passed Cross-platform CI34173074703:19successful jobs,2explicit skips (Windows runtime shards/macOS unsharded control). Linux and macOS shards passed. Explicit log evidence includes v1/v2 current-account re-selection and admission refusal, subsequent 429/402 alternates, alias precedence, TTL/257-entry eviction, virtual Pro identity, successful/failed/cancelled completion and hidden duplicate-terminal cases. Linux ignores the eager-mode option, actual macOS output also passed all16 hidden-completion variants, including explicit eager relay with terminal recording on/off.
  • Exact documentation source tree built 425 pages on remote macmini-cf with Bun 1.4.0 and frozen dependencies. Subsequent changes touched only runtime/tests; documentation and its build inputs are unchanged. The owned remote scratch was removed; no deployment ran.
  • Prior candidates were not green: an existing source oracle pinned the old abort callback spelling; a fixture expected configured m2 after deliberately emitting final-b; v1 native refusal left a one-second combo cooldown and an unbounded callback wait leaked test state into v2. Each was corrected from actual CI evidence without changing runtime timeouts, skipping tests or weakening scanner/production guards. Superseded runs were cancelled and are not acceptance evidence.
  • Owner-authorized maintainer integration into dev requires the final current-head CI result, current review checks, actual merge-tree equality, dev ancestry and x3M3x credit in the squash commit. No historical candidate success is substituted.
  • No live account requests, release, deployment or workflow changes.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed; English/Korean combo guides and transport architecture are included.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults; isolated fixtures and unchanged admission/credential resolution. Final independent audit remains pending as stated above.

Co-authored-by: x3M3x amroeid1999@gmail.com

Pre-merge destination dc5ee2f49dde307f1c7ece31829915072aa9bde5 adds C documentation, the independent Santiago fixture correction and B's opt-in reset-credit journal serialization after candidate base bbea77a48. No combo recall/terminal/compaction source, its tests, or build manifests/workflows changed. Existing account/admission/config-reconciliation owners remain unchanged. Candidate CI is exact-head evidence for all four A fixes; the final combination with later B/C changes is not claimed as a pre-merge execution. We verify the full expected integration tree and leave the automatic dev push CI as separate evidence.

Summary by CodeRabbit

  • New Features

    • Compaction can recall the most recently successful combo after switching combos when using a matching bare model name.
    • Explicit provider, alias, and combo selections continue to take precedence.
    • Recall is isolated by conversation lane, expires after 30 minutes, and is cleared on restart or invalid configuration changes.
  • Documentation

    • Added English and Korean guidance describing combo recall behavior and limitations.
  • Tests

    • Added coverage for routing, expiration, lane isolation, configuration changes, cancellation, and failover scenarios.

x3M3x and others added 3 commits September 8, 2026 08:35
…ombo switch

When Codex compacts a conversation that was switched to a different combo
mid-session, it sends the bare native model of the new combo target (e.g.
gpt-5.6-terra) rather than the combo/<id> selector. Without recall, the
bare model hits routeCompactionModel and lands on the configured default
provider (or 404s) instead of routing through the combo failover path.

Added a bounded session-lane recall map that records the combo target on
every successful combo turn. Both compaction entry points (v2
compaction_trigger in handleResponsesInner and v1 /responses/compact in
handleResponsesCompact) rewrite a bare model back to the remembered
combo selector when it exactly matches the last-served combo target on
that session lane.

Review round 1 (#3891): the v1 compact endpoint now also syncs the routed
identity (compactModel) with the recall rewrite - previously only
raw.model was rewritten, so a bare model with no canonical openai row
could still 404, and a bare model whose target lives on a native-compact
provider (openai / openai-apikey) resolved straight onto the native
/responses/compact endpoint, bypassing combo dispatch. Two regression
tests cover both routing holes.

Safety properties:
- Only fires for compaction requests (compaction_trigger present)
- Only fires for bare models (no provider/ prefix)
- Only fires when the bare model exactly matches the combo target
- Different session lanes never borrow each others recall
- Explicit combo/provider selectors are never touched

The rewrite in handleResponsesInner runs BEFORE comboIdFromRawBody so
the combo dispatch path engages. The compact endpoint rewrites raw.model
before the non-native dispatch falls through to handleResponses.

(cherry picked from commit 52e66f6)
Use existing parsed completion callbacks and an independent child gate slot. Preserve explicit combo aliases, current target ownership, config-generation rejection, and final client-visible model identity across failover and compaction.

Verification: source review and git diff --check only. Local product tests, typecheck, builds, installs, scripts, servers, and live API calls NOT RUN by user instruction.

Co-authored-by: x3M3x <amroeid1999@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 7037816c-0092-4f62-9a45-15996fd44cb2

📥 Commits

Reviewing files that changed from the base of the PR and between bbea77a and 797a74c.

📒 Files selected for processing (11)
  • docs-site/src/content/docs/guides/combos.md
  • docs-site/src/content/docs/ko/guides/combos.md
  • src/lib/state-store-registrations.ts
  • src/server/responses/combo-session-recall.ts
  • src/server/responses/compact.ts
  • src/server/responses/core.ts
  • structure/04_transports-and-sidecars.md
  • tests/oauth/state-store-sweeper.test.ts
  • tests/responses/passthrough-abort.test.ts
  • tests/responses/responses-compaction-routing.test.ts
  • tests/server/server-combo-failover-e2e.test.ts

📝 Walkthrough

Walkthrough

Changes

The PR adds process-local combo recall per conversation lane. Completed combo responses record their final client-visible model. Later bare-model compaction requests can route through the remembered combo when the lane, model, configuration, and ownership remain valid.

Combo session recall

Layer / File(s) Summary
Recall state and reconciliation
src/server/responses/combo-session-recall.ts, src/lib/state-store-registrations.ts, tests/oauth/state-store-sweeper.test.ts
Adds a 256-entry, 30-minute recall store. Entries require valid lanes, models, generations, and live combo ownership. Reconciliation removes expired or obsolete entries and blocks stale-generation writes.
Validated completion capture
src/server/responses/core.ts, tests/server/server-combo-failover-e2e.test.ts, tests/responses/passthrough-abort.test.ts
Adds onResponseComplete handling across combo, passthrough, streaming, buffered, media, web-search, and runTurn paths. Completion is recorded only for accepted, completed responses with a non-empty model. Cancellation, failure, incomplete terminals, discarded callbacks, and invalid terminal sequences do not replace recall state.
Compaction recall routing
src/server/responses/compact.ts, src/server/responses/core.ts, tests/responses/responses-compaction-routing.test.ts, docs-site/src/content/docs/guides/combos.md, docs-site/src/content/docs/ko/guides/combos.md, structure/04-transports-and-sidecars.md
Eligible bare models are rewritten to combo/<id> before compaction routing. Explicit provider, alias, and combo selectors remain higher priority. Tests cover both compaction APIs, lane isolation, expiry, eviction, configuration changes, provider resolution, account failover, and native compact-target behavior. Documentation describes the recall rules and limits.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant CompactHandler
  participant ComboRecallStore
  participant ComboRouter
  participant Provider
  Client->>CompactHandler: compact with bare model
  CompactHandler->>ComboRecallStore: lookup session lane and model
  ComboRecallStore-->>CompactHandler: remembered combo id or no match
  CompactHandler->>ComboRouter: route as combo/<id> when matched
  ComboRouter->>Provider: send compact request through combo path
Loading

Suggested reviewers: invalid-email-address

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/248-a-combo-8c62

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 added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 58 / 80

이 PR은 대화 중간에 콤보를 바꾼 뒤, 클라이언트가 공급자 접두사 없는 맨모델 이름으로 compact를 요청하면 방금 성공한 콤보 대신 그 맨모델로만 들어가 페일오버·선택이 깨지는 문제를 고칩니다. 새로 생기는 src/server/responses/combo-session-recall.ts가 세션 레인마다 “마지막으로 성공적으로 끝난 콤보”를 프로세스 메모리에 짧게 기억하고, compact.ts의 v1 compact와 core.ts의 compaction_trigger / 콤보 디스패치 앞에서 그 기억을 combo/<id>로 되살립니다. 기여 원본은 #3891(x3M3x)이고, 이 carry는 원본이 빠뜨리기 쉬운 네 경계를 문서·테스트로 못 박습니다. (1) 수락된 completed 응답만 기록, (2) 클라이언트가 본 response model과 일치할 때만 회수, (3) 설정 generation이 바뀌면 늦은 writer 거절, (4) 실패·미완료·취소는 이전 성공을 덮지 않음.

지금 dev HEAD에는 이미 compactHandoffRoutes라는 비슷한 “이전 모델 핸드오프” 맵이 compact 경로에 있습니다. 이번 recall은 그와 겹쳐 보이지만 목적이 다릅니다. handoff는 compact가 고른 라우트를 이어 주는 쪽이고, recall은 “맨모델 compact 요청을 다시 콤보 선택기로 넣기”입니다. recallComboForLane은 모델에 /가 있으면 즉시 포기하고, 설정된 콤보 alias·명시 선택자보다 후순위입니다. 용량 256·TTL 30분·자격증명 미저장·재시작 시 소멸을 영/한 가이드와 structure/04_transports-and-sidecars.md에 적어 두었습니다. state-store-registrations.tscombo-session-recall reconcile을 등록해, 콤보/타깃/프로바이더가 설정에서 사라지면 기억을 지웁니다. 테스트 분량이 큰 이유(compact 라우팅·failover e2e·sweeper)는 위 네 경계를 회귀로 고정하려는 것으로 읽힙니다. types/config 분할에 직접 걸리지 않습니다.

우선순위는 콤보+Codex compact를 쓰는 사용자 체감이 커서 중상입니다. 다만 기억이 프로세스 로컬이라 여러 서버 인스턴스·재시작 뒤에는 예전처럼 맨모델 compact로 떨어질 수 있고, 본문도 그 한계를 숨기지 않습니다. 로컬 product 테스트는 금지되어 hosted CI와 독립 감사가 머지 전 증거입니다. #3891 leftover는 머지 후 landed-via-maintainer로 닫는 패턴이 맞습니다.

라인 src/server/responses/combo-session-recall.ts · rememberComboForLane - writerGeneration < max(lastReconciledGeneration, captureConfigGeneration())이면 기록을 거절합니다. 설정이 바뀐 뒤 늦게 도착한 완료가 옛 콤보를 다시 심지 않게 하는 핵심입니다.

라인 src/server/responses/combo-session-recall.ts · recallComboForLane - TTL·disabled provider·타깃 부재·responseModel === model을 모두 검사합니다. 모델만 같다고 콤보를 되살리지 않습니다.

라인 src/server/responses/compact.ts · handleResponsesCompact - bare model이고 fast-row/alias가 아닐 때만 recall합니다. 명시 combo/·provider/model 선택자를 가로채지 않는지 머지 전 CI에서 확인하세요.

라인 src/server/responses/core.ts · compaction_trigger / completed 콜백 - 수락된 completed만 remember하고, model-less terminal은 recall을 영구 거절합니다. 실패·취소가 성공 기록을 덮지 않는 계약이 여기 있습니다.

경로 #3891 - 같은 버그의 contributor 원본입니다. 이 PR 머지 후 Landed via #3971 at <sha> + landed-via-maintainer로 닫으세요.

경로 compactHandoffRoutes (기존 compact.ts) - 이름이 비슷한 두 맵이 공존합니다. 문서에 “핸드오프 vs 콤보 recall” 한 줄이 더 있으면 이후 기여자가 덜 헷갈립니다. 필수는 아닙니다.

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

  • hosted CI와 본문이 말한 두 번째 독립 감사가 이 head에서 통과했는지
  • 프로세스 로컬 한계를 문서만으로 sufficent하다고 볼지, 혹은 multi-instance에서 더 강한 저장이 필요한지(이 PR 범위 밖으로 두는 게 맞아 보임)
  • 기존 compact handoff와 recall의 역할 분리를 코드 주석/구조 문서에 한 줄 더 남길지

너의 추천
CI와 감사가 초록이면 머지하세요. 콤보 전환 후 bare-model compact가 빗나가는 실사용 버그를, 자격증명 없는 짧은 프로세스 기억으로 고치는 범위가 명확합니다. 머지 후 #3891은 landed-via-maintainer로 정리하면 됩니다. multi-instance sticky가 필요해지면 별도 이슈로 여는 편이 이 PR을 비대하게 만들지 않습니다.

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

@lidge-jun
lidge-jun marked this pull request as ready for review September 8, 2026 00:41
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 8, 2026 00:41
@lidge-jun
lidge-jun merged commit 900567a into dev Sep 8, 2026
32 of 34 checks passed
@lidge-jun
lidge-jun deleted the codex/248-a-combo-8c62 branch September 8, 2026 00:41
@lidge-jun

Copy link
Copy Markdown
Owner Author

Final evidence correction: CI34173074703 actually checked out synthetic merge b12a0bb. Its parents are destination dc5ee2f and candidate797a74c646b554a1716905c3721dd13dcef0d579, and its tree e5a6d802123cb463d7f7ecd172504ff85dff8ff6 exactly equals landed900567af3f38ad6e07e4723c2eedeeffb95c57ab. Thus the final integration including the later B/C changes was tested successfully; the earlier conservative statement that it was untested is superseded. The separate dev-push run34174143615 remains separate evidence. Windows runtime shards and macOS unsharded control were skipped in candidate CI and are not counted as passes.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants