fix(combo): fail over Astra tool routing mismatch by 87003697 · Pull Request #5142 · lidge-jun/opencodex

87003697 · 2026-09-19T09:14:41Z

Summary

treat the exact null-code Astra function-tool routing mismatch as target-local combo incompatibility
preserve fail-closed behavior for omitted codes, altered models, malformed envelopes, and unrelated 400s
verify the same function result reaches both native Responses targets in streaming and non-streaming failover

Root cause

OpenCodex dispatched through the openai-responses adapter, but one Xhub credential/backend returned a structured 400 claiming the request used /v1/chat/completions; the same Responses tool workflow succeeded through another credential. The combo previously treated that target-local mismatch as terminal.

Validation

bun x tsc --noEmit
bun test tests/routing/router-combo-failover-classification.test.ts tests/server/server-combo-failover-e2e.test.ts (233 pass)
bun scripts/structure-ssot.ts

Summary by CodeRabbit

Bug Fixes
- Improved request failover when function tools are incompatible with the selected model and reasoning effort.
- Requests now automatically try a compatible backup target instead of failing immediately.
- Preserved tool-result inputs and support for both streaming and non-streaming requests during recovery.
- Prevented temporarily rejecting the incompatible target when the failure is recognized as a routing mismatch.

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.

coderabbitai · 2026-09-19T09:15:12Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4ea8b343-39ff-42b0-b148-2e3152a7d20e

📥 Commits

Reviewing files that changed from the base of the PR and between 311bbab and 2bb2678.

📒 Files selected for processing (4)

src/combos/failover.ts
structure/runtime.md
tests/routing/router-combo-failover-classification.test.ts
tests/server/server-combo-failover-e2e.test.ts

 _____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________
< I think a lot of new programmers like to use advanced data structures and advanced language features as a way of demonstrating their ability. I call it the lion-tamer syndrome. Such demonstrations are impressive, but unless they actually translate into real wins for the project, avoid them. - Glyn Williams >
 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a8e7fe4c-1052-4cfe-8f62-cade7a1116a7

📥 Commits

Reviewing files that changed from the base of the PR and between 134c92a and 311bbab.

📒 Files selected for processing (4)

src/combos/failover.ts
structure/runtime.md
tests/routing/router-combo-failover-classification.test.ts
tests/server/server-combo-failover-e2e.test.ts

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

📝 Walkthrough

Walkthrough

The failover classifier now recognizes an exact gpt-6-astra function-tool routing refusal. Tests verify hopping to a backup Responses target for streaming and non-streaming requests without cooling the rejected target.

Changes

Routing failover

Layer / File(s)	Summary
Classify the routing refusal `src/combos/failover.ts`, `tests/routing/router-combo-failover-classification.test.ts`	`isRequestLocalTargetIncompatibility` now matches the null-code `reasoning_effort` refusal with the exact `gpt-6-astra` function-tool message. Unit tests verify that matching refusals hop without cooldowns and that altered parameters, codes, model text, or wording remain terminal.
Verify failover execution `tests/server/server-combo-failover-e2e.test.ts`	End-to-end tests cover streaming and non-streaming Responses requests. The exact tool-result input is sent to the incompatible target and the backup target. The backup response succeeds, the hit order is `["incompatible", "backup"]`, and target `a` is not cooled down.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ComboFailover
  participant FirstResponsesTarget
  participant BackupResponsesTarget
  Client->>ComboFailover: Send function_call_output request
  ComboFailover->>FirstResponsesTarget: Forward request
  FirstResponsesTarget-->>ComboFailover: 400 reasoning_effort routing refusal
  ComboFailover->>ComboFailover: Classify refusal as request-local
  ComboFailover->>BackupResponsesTarget: Forward unchanged request
  BackupResponsesTarget-->>Client: 200 successful response

Merge Risk: ⚪ Minimal · up to 311bb

The documented failover behavior remains internally consistent, and no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: failover handling for the Astra tool-routing mismatch in combo routing. It is concise, specific, and consistent with the described code and test changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 …
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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 · 2026-09-19T09:15:31Z

✅ Deterministic PR hygiene checks passed.

github-actions · 2026-09-19T09:16:25Z

⏳ 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

⬜ 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.

0/4 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

lidge-jun · 2026-09-19T11:02:05Z

리뷰 · 우선순위 48 / 80

콤보의 첫 타깃이 한 가지 400을 주면, 콤보를 끊지 않고 다음 타깃으로 넘겨요. gpt-6-astra에 함수 도구와 reasoning_effort를 같이 실으면, 어떤 Xhub 쪽이 "이 요청은 /v1/chat/completions로 갔다"는 400을 돌려요. OpenCodex는 openai-responses로 보냈는데도 그래요. 다른 자격 증명은 같은 요청이 돼요. 예전에는 이 400이면 콤보가 거기 끝났어요.

이제는 그 400이 정해 둔 모양일 때만 넘겨요. 상태 번호는 400이어야 하고, error.code는 null이어야 하고, param은 reasoning_effort여야 하고, 문장은 한 글자까지 같아야 해요. 모델 이름은 gpt-6-astra이거나, 뒤에 2026-09-03처럼 날짜만 붙은 이름만 받아요. 미리보기 이름, 날짜 칸이 다른 이름, 짧은 문장, 다른 param, 다른 code는 그대로 멈춰요. 넘길 때 그 타깃을 잠시 막지는 않아요. 요청도 고치지 않아요. 테스트는 스트림이든 아니든, 같은 도구 결과가 두 타깃에 가는지 봐요.

베이스는 main이에요. types.ts와 config.ts를 나누는 일과는 관계없어요. 이 문장을 고치는 다른 열린 PR은 없어요.

라인 베이스 - dev가 아니라 main을 봐요. 게이트가 제목에 [WRONG BRANCH]를 붙였고, 초안으로 남겨 뒀어요. 설명의 준비 체크는 0/4예요.

라인 src/combos/failover.ts 402행 - 문장 끝의 마침표까지 같아야 넘어가요. 실제로 돌아온 400에 요청 번호가 붙거나, param이 reasoning.effort이면 안 넘어가요. 단위 테스트의 넘김은 바깥 code를 invalid_request_error로 넣어요. 서버는 이 에러의 code가 null이면 바깥 code를 비워요 (src/server/responses/core-errors.ts 70행). 비어 있어도 이 함수는 넘어가게 되어 있어요. 그 호출은 hop 테스트에 없어요.

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

실제로 받은 Xhub 400이 이 문장과 같은지 봐 주세요. 한 글자만 달라도 테스트는 통과하고, 그 장애는 그대로예요.

다음 타깃도 같은 잘못된 쪽이면 또 400이에요. 콤보는 타깃마다 한 번만 가서 끝나기는 해요. reasoning_effort를 none으로 지우지는 않아요. 문서에 적어 둔 그대로예요. 이대로 둘지 정해 주세요.

너의 추천

일치를 좁혀 둔 것은 맞아요. 닫을 중복은 없어요. main에는 넣지 마세요. 베이스를 dev로 바꾸고, 최신 dev 위에 올린 다음, 받은 400 원문이 정규식과 같은지만 보면 돼요.

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

lidge-jun · 2026-09-19T13:04:26Z

추가 리뷰 · 우선순위 36 / 80

지난 리뷰 뒤에 보는 가지만 바뀌었어요. 12:39에 가지를 다시 밀어 넣었고, 베이스가 main에서 dev로 옮겨졌어요. 400을 넘기는 규칙은 그대로예요. 문장이 한 글자까지 같아야 하고, 그 타깃을 잠시 막지는 않아요.

이 머리는 dev보다 커밋 4개가 뒤예요. 뒤의 네 커밋은 이미지 프롬프트, 웹소켓, 사이드밴드 프레임, 히스토리 잡이에요. 이 PR이 고친 파일과는 안 겹쳐요.

제목에는 아직도 [WRONG BRANCH]가 붙어 있어요. 가지는 이미 dev예요. 초안이고, 설명의 준비 체크는 0/4예요. types.ts와 config.ts를 나누는 일과는 관계없고, 이 문장을 고치는 다른 열린 PR은 없어요.

라인 제목 - 가지를 고친 뒤에도 [WRONG BRANCH]가 남았어요. 타깃을 검사하는 작업은 취소됐어요. 접두어를 빼는 쪽이 다시 안 돌았어요.

라인 src/combos/failover.ts - 문장 끝의 마침표까지 같아야 넘어가요. param이 reasoning.effort이거나, 문장 뒤에 요청 번호가 붙으면 안 넘어가요. 이 부분은 안 고쳤어요.

라인 tests/routing/router-combo-failover-classification.test.ts - 넘김 테스트는 바깥 code를 invalid_request_error로 넣어요. 서버는 안쪽 code가 null이면 바깥 code를 안 넘겨요. 빈 값이어도 이 함수는 넘어가고, 서버 테스트가 그 길을 타요. 단위 테스트의 넘김 호출만 서버가 넣는 값과 달라요.

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

실제로 받은 Xhub 400이 이 문장과 같은지 봐 주세요. 한 글자만 달라도 테스트는 통과하고, 그 장애는 그대로예요.

다음 타깃도 같은 잘못된 쪽이면 또 400이에요. reasoning_effort를 none으로 지우지는 않아요. 이대로 둘지 정해 주세요.

너의 추천

가지는 맞아요. 닫을 중복은 없어요. 제목의 [WRONG BRANCH]는 지워도 돼요. 최신 dev 위에 올린 다음, 받은 400 원문만 보면 돼요. 넘기는 조건을 더 넓히지는 마세요.

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

) * refactor(codex,tests): split two files back under their size caps The file-size ratchet reported two offenders on dev. src/codex/history-provider.ts reached 2009 lines and is not in the baseline, so it tripped NEW_OVERSIZED at the 2000-line threshold. tests/server/server-combo-failover-e2e.test.ts reached 4192 against a committed cap of 4166. Both came from the batch merged just before: #5137 added the paginated-openai verdict to the history preflight, and #5142 added a tool-routing failover case. Raising either cap is not available. updateBaseline only ever lowers a cap and tests/ci-workflows/file-size-ratchet.test.ts asserts that, so a baseline edit would fail the same gate. Trimming the rationale comments those two changes carry would have fit the line budget and is the wrong trade: the comments are why the verdicts are readable at all. src/codex/history-rollout-read.ts takes the reading side of rollout JSONL files: the session_meta fold and the thread fields a quarantine restore reconstructs. The cut follows a seam rather than a line count. Everything moved reads a file and returns plain data; everything left behind writes a database or a backup manifest. history-provider.ts re-exports the public names, so no importer changes. tests/helpers/combo-tool-routing-cases.ts follows the register-cases seam the forced-effort, context-overflow and context-headroom groups already use in that file. The case body is unchanged; only its home moved, and it still runs inside the same describe with the same isolated homes, mocks and cleanup. No local suite, focused test, typecheck or build was run. Exact-head hosted CI is the execution evidence. * test(codex): verify the Reserve cache on disk instead of the write flag Three Reserve catalog lifecycle cases fail on the dev tip's macOS shards with refreshOutcome committed, catalogExists true, catalogWritten false and cacheSynced false. The fixture's oracle required cacheSynced, and #5108 made that flag report whether bytes were written, so a sync that finds the desired bytes already on disk now answers false. The production meaning is the one to keep. src/cli/index.ts and src/cli/dispatch.ts gate restart and reporting on catalogWritten or cacheSynced, and a start that changed nothing should not claim it did. The stale assertion is the fixture's. Deleting the assertion is not available either: a broad catch inside the refresh also yields false, so dropping it would stop distinguishing a no-op from a failure. Instead the child now requires the committed verdict and then reads the cache back, proving the active catalog's slugs appear in the cache file in the same order. That is independent of the write flag and of the cache document shape, and every existing Reserve assertion is untouched. Root cause analysis by the contract-campaign task, which confirmed the genuine changed-source case still passes and only the byte-identical no-op trips the old oracle. No local suite, focused test, typecheck or build was run. Exact-head hosted CI is the execution evidence. --------- Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>

87003697 requested review from Ingwannu and lidge-jun as code owners September 19, 2026 09:14

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

github-actions Bot changed the title ~~fix(combo): fail over Astra tool routing mismatch~~ [WRONG BRANCH] fix(combo): fail over Astra tool routing mismatch Sep 19, 2026

github-actions Bot marked this pull request as draft September 19, 2026 09:16

87003697 force-pushed the fix/astra-tool-routing-failover branch from 311bbab to 18879cf Compare September 19, 2026 09:28

fix(combo): fail over Astra tool routing mismatch

2bb2678

lidge-jun force-pushed the fix/astra-tool-routing-failover branch from 18879cf to 2bb2678 Compare September 19, 2026 12:39

lidge-jun changed the base branch from main to dev September 19, 2026 12:39

github-actions Bot changed the title ~~[WRONG BRANCH] fix(combo): fail over Astra tool routing mismatch~~ fix(combo): fail over Astra tool routing mismatch Sep 19, 2026

lidge-jun marked this pull request as ready for review September 19, 2026 17:26

lidge-jun merged commit 187c959 into lidge-jun:dev Sep 19, 2026
6 of 9 checks passed

lidge-jun mentioned this pull request Sep 19, 2026

refactor(codex,tests): split two files back under their size caps #5201

Merged

3 tasks done

Navigation Menu

Repository navigation

fix(combo): fail over Astra tool routing mismatch - #5142

fix(combo): fail over Astra tool routing mismatch#5142
lidge-jun merged 1 commit into
lidge-jun:devfrom
87003697:fix/astra-tool-routing-failover

87003697 commented Sep 19, 2026 •

edited by github-actions Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Sep 19, 2026

Uh oh!

github-actions Bot commented Sep 19, 2026 •

edited

Loading

Uh oh!

lidge-jun commented Sep 19, 2026

Uh oh!

lidge-jun commented Sep 19, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

87003697 commented Sep 19, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Validation

Summary by CodeRabbit

Review readiness checklist

Uh oh!

coderabbitai Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Sep 19, 2026

Uh oh!

github-actions Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏳ DRAFT

What to do

Review readiness checklist

Uh oh!

lidge-jun commented Sep 19, 2026

리뷰 · 우선순위 48 / 80

Uh oh!

lidge-jun commented Sep 19, 2026

추가 리뷰 · 우선순위 36 / 80

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

87003697 commented Sep 19, 2026 •

edited by github-actions Bot

Loading

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

github-actions Bot commented Sep 19, 2026 •

edited

Loading