fix(mitm): strip trailing assistant prefill to prevent upstream Anthropic 400 errors - #7520
Merged
diegosouzapw merged 10 commits intoJul 19, 2026
Conversation
…the default branch (#7168)
… (queue_conditions alone are eligibility-only) (#7179)
…uto_merge_conditions (rules-based path is EOL 2026-07-16) (#7216)
…; free plan queue is serial) (#7220)
…H-hosted build hang dequeued every attempt) (#7225)
…ils every PR (#7341) main's copy of this test still does git I/O inside a unit test: const baseSrc = git(['show', 'origin/main:' + FILE]); Runners check out a shallow single ref, so origin/main does not resolve and the test dies with 'fatal: invalid object name origin/main'. Every PR into main fails Unit Tests (7/8) on it — today that is #7313, #7315, #7316, #7334, #7336 and #7337, six PRs red on a defect none of them introduced. #7313 has no other red at all. release/v3.8.49 already carries a fix (2e42b8e, #7174: try/catch, fetch origin/main on demand, t.skip() when unreachable), but it only reaches main at release time — so main stays broken for the whole cycle. Cherry-picking it would also import a new problem: PR Test Policy classifies t.skip() as a silenced assertion, which we watched it correctly catch on #7300 today. This is the hermetic version instead (ported from #7327, which does the same for the release branch): read the file straight off disk, compare against an empty base so baseTaut/baseExtTaut are 0 — the strictest possible comparison point — and call evaluateMasking() directly. No git ref, no fetch, no skip, nothing the runner's checkout depth can break. The #6634 regression stays covered: the guard's logic lives in SELF_TEST_FIXTURE_RE (check-test-masking.mjs:337), not in the test. Proven both ways on main before committing — neutralise SELF_TEST_FIXTURE_RE to /$^/ and the test FAILS; restore it and it passes 2/2, with check-test-masking.mjs left byte-identical. Co-authored-by: growab <nekron@icloud.com>
…bers (#7347) main's ratchet had been failing --require-tighten on every PR: 11 metrics improved but the baseline was never tightened. Same class as the #6634 selfref guard — an infra fix that lands only on the release branch leaves main red for the whole cycle, and every PR into main pays for it. Values are the merged-coverage numbers from a run on main itself (a local run measures ~68% vs CI's ~80%; the baseline's own note warns about that gap). Only the 11 coverage values change — gitleaks and semgrepFindings keep main's own state. No changelog fragment: #7326 carries it on release/v3.8.49, and a second one here would double the entry at release time.
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b4515472b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+32
to
+33
| if (lastMsg && lastMsg.role === "assistant") { | ||
| payload.messages.pop(); |
There was a problem hiding this comment.
When a Claude Code payload contains more than one trailing assistant turn, this removes only the final element and still forwards a body whose last message is assistant, so the strict upstreams this change targets can return the same “conversation must end with a user message” 400. Use a loop that stops before emptying the array so every consecutive trailing prefill is removed while preserving at least one message.
Useful? React with 👍 / 👎.
Owner
|
Thanks for chasing this down — the trailing-prefill 400 is a real bug class, and it matches three fixes we already carry for the same upstream error:
One more thing worth discussing: this fires in the MITM ingress handler, before OmniRoute has picked a downstream provider/model, so it strips prefill unconditionally even for models that do support it. The three existing fixes are all scoped to the specific executor/provider that actually rejects prefill for that reason. Could you scope this the same way, or add a short comment explaining why blanket-stripping at ingress is the intended trade-off for Claude Code traffic? Finally, please add a test to |
The trailing-assistant-prefill strip only popped a single message, so a
conversation ending in 2+ consecutive assistant turns still hit the
upstream "This model does not support assistant message prefill" 400.
It also had no floor, so a payload whose entire history was trailing
assistant turns collapsed to messages: [], trading one 400 for another
("messages: at least 1 item required").
Loop over all consecutive trailing assistant messages and never strip
below 1 remaining message, mirroring the same pop-loop bound already used
by dropTrailingAssistantPrefill (open-sse/executors/github.ts) and
stripTrailingAntigravityAssistantTurn (open-sse/executors/antigravity.ts).
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Resolve add/add conflict in .mergify.yml by taking release's current canonical version — the file is functionally inert on release branches (Mergify only reads config from the default branch, per #7168), and this branch's copy predates several Mergify-config iterations already on the release tip (auto-enqueue migration, dast-smoke tolerance, batch_size removal). config/quality/quality-baseline.json auto-merged identical to release's own copy (this branch's own baseline-tightening commit was already superseded). The PR's own while-loop trailing-assistant-prefill fix in src/mitm/handlers/claudeCode.ts auto-merged cleanly with zero conflicts, unchanged. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Owner
|
Merged into |
Merged
HouMinXi
pushed a commit
to HouMinXi/OmniRoute
that referenced
this pull request
Aug 2, 2026
…opic 400 errors (diegosouzapw#7520) * chore(ci): add .mergify.yml to main — Mergify only reads config from the default branch (diegosouzapw#7168) * fix(ci): add the auto-enqueue pull_request_rule to the Mergify config (queue_conditions alone are eligibility-only) (diegosouzapw#7179) * fix(ci): migrate Mergify auto-enqueue to merge_protections_settings.auto_merge_conditions (rules-based path is EOL 2026-07-16) (diegosouzapw#7216) * fix(ci): drop Mergify batch settings (batching is a paid-tier feature; free plan queue is serial) (diegosouzapw#7220) * fix(ci): merge queue tolerates the advisory dast-smoke failure (its GH-hosted build hang dequeued every attempt) (diegosouzapw#7225) * test(ci): make the diegosouzapw#6634 selfref guard hermetic — main's copy hard-fails every PR (diegosouzapw#7341) main's copy of this test still does git I/O inside a unit test: const baseSrc = git(['show', 'origin/main:' + FILE]); Runners check out a shallow single ref, so origin/main does not resolve and the test dies with 'fatal: invalid object name origin/main'. Every PR into main fails Unit Tests (7/8) on it — today that is diegosouzapw#7313, diegosouzapw#7315, diegosouzapw#7316, diegosouzapw#7334, diegosouzapw#7336 and diegosouzapw#7337, six PRs red on a defect none of them introduced. diegosouzapw#7313 has no other red at all. release/v3.8.49 already carries a fix (8bbd411, diegosouzapw#7174: try/catch, fetch origin/main on demand, t.skip() when unreachable), but it only reaches main at release time — so main stays broken for the whole cycle. Cherry-picking it would also import a new problem: PR Test Policy classifies t.skip() as a silenced assertion, which we watched it correctly catch on diegosouzapw#7300 today. This is the hermetic version instead (ported from diegosouzapw#7327, which does the same for the release branch): read the file straight off disk, compare against an empty base so baseTaut/baseExtTaut are 0 — the strictest possible comparison point — and call evaluateMasking() directly. No git ref, no fetch, no skip, nothing the runner's checkout depth can break. The diegosouzapw#6634 regression stays covered: the guard's logic lives in SELF_TEST_FIXTURE_RE (check-test-masking.mjs:337), not in the test. Proven both ways on main before committing — neutralise SELF_TEST_FIXTURE_RE to /$^/ and the test FAILS; restore it and it passes 2/2, with check-test-masking.mjs left byte-identical. Co-authored-by: growab <nekron@icloud.com> * chore(quality): tighten main's coverage baseline to the CI's real numbers (diegosouzapw#7347) main's ratchet had been failing --require-tighten on every PR: 11 metrics improved but the baseline was never tightened. Same class as the diegosouzapw#6634 selfref guard — an infra fix that lands only on the release branch leaves main red for the whole cycle, and every PR into main pays for it. Values are the merged-coverage numbers from a run on main itself (a local run measures ~68% vs CI's ~80%; the baseline's own note warns about that gap). Only the 11 coverage values change — gitleaks and semgrepFindings keep main's own state. No changelog fragment: diegosouzapw#7326 carries it on release/v3.8.49, and a second one here would double the entry at release time. * fix(mitm): strip trailing assistant prefill to prevent upstream Anthropic 400 errors * fix(mitm): loop over all trailing assistant turns + never-empty guard The trailing-assistant-prefill strip only popped a single message, so a conversation ending in 2+ consecutive assistant turns still hit the upstream "This model does not support assistant message prefill" 400. It also had no floor, so a payload whose entire history was trailing assistant turns collapsed to messages: [], trading one 400 for another ("messages: at least 1 item required"). Loop over all consecutive trailing assistant messages and never strip below 1 remaining message, mirroring the same pop-loop bound already used by dropTrailingAssistantPrefill (open-sse/executors/github.ts) and stripTrailingAntigravityAssistantTurn (open-sse/executors/antigravity.ts). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: growab <nekron@icloud.com>
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…opic 400 errors (diegosouzapw#7520) * chore(ci): add .mergify.yml to main — Mergify only reads config from the default branch (diegosouzapw#7168) * fix(ci): add the auto-enqueue pull_request_rule to the Mergify config (queue_conditions alone are eligibility-only) (diegosouzapw#7179) * fix(ci): migrate Mergify auto-enqueue to merge_protections_settings.auto_merge_conditions (rules-based path is EOL 2026-07-16) (diegosouzapw#7216) * fix(ci): drop Mergify batch settings (batching is a paid-tier feature; free plan queue is serial) (diegosouzapw#7220) * fix(ci): merge queue tolerates the advisory dast-smoke failure (its GH-hosted build hang dequeued every attempt) (diegosouzapw#7225) * test(ci): make the diegosouzapw#6634 selfref guard hermetic — main's copy hard-fails every PR (diegosouzapw#7341) main's copy of this test still does git I/O inside a unit test: const baseSrc = git(['show', 'origin/main:' + FILE]); Runners check out a shallow single ref, so origin/main does not resolve and the test dies with 'fatal: invalid object name origin/main'. Every PR into main fails Unit Tests (7/8) on it — today that is diegosouzapw#7313, diegosouzapw#7315, diegosouzapw#7316, diegosouzapw#7334, diegosouzapw#7336 and diegosouzapw#7337, six PRs red on a defect none of them introduced. diegosouzapw#7313 has no other red at all. release/v3.8.49 already carries a fix (83a7551, diegosouzapw#7174: try/catch, fetch origin/main on demand, t.skip() when unreachable), but it only reaches main at release time — so main stays broken for the whole cycle. Cherry-picking it would also import a new problem: PR Test Policy classifies t.skip() as a silenced assertion, which we watched it correctly catch on diegosouzapw#7300 today. This is the hermetic version instead (ported from diegosouzapw#7327, which does the same for the release branch): read the file straight off disk, compare against an empty base so baseTaut/baseExtTaut are 0 — the strictest possible comparison point — and call evaluateMasking() directly. No git ref, no fetch, no skip, nothing the runner's checkout depth can break. The diegosouzapw#6634 regression stays covered: the guard's logic lives in SELF_TEST_FIXTURE_RE (check-test-masking.mjs:337), not in the test. Proven both ways on main before committing — neutralise SELF_TEST_FIXTURE_RE to /$^/ and the test FAILS; restore it and it passes 2/2, with check-test-masking.mjs left byte-identical. Co-authored-by: growab <nekron@icloud.com> * chore(quality): tighten main's coverage baseline to the CI's real numbers (diegosouzapw#7347) main's ratchet had been failing --require-tighten on every PR: 11 metrics improved but the baseline was never tightened. Same class as the diegosouzapw#6634 selfref guard — an infra fix that lands only on the release branch leaves main red for the whole cycle, and every PR into main pays for it. Values are the merged-coverage numbers from a run on main itself (a local run measures ~68% vs CI's ~80%; the baseline's own note warns about that gap). Only the 11 coverage values change — gitleaks and semgrepFindings keep main's own state. No changelog fragment: diegosouzapw#7326 carries it on release/v3.8.49, and a second one here would double the entry at release time. * fix(mitm): strip trailing assistant prefill to prevent upstream Anthropic 400 errors * fix(mitm): loop over all trailing assistant turns + never-empty guard The trailing-assistant-prefill strip only popped a single message, so a conversation ending in 2+ consecutive assistant turns still hit the upstream "This model does not support assistant message prefill" 400. It also had no floor, so a payload whose entire history was trailing assistant turns collapsed to messages: [], trading one 400 for another ("messages: at least 1 item required"). Loop over all consecutive trailing assistant messages and never strip below 1 remaining message, mirroring the same pop-loop bound already used by dropTrailingAssistantPrefill (open-sse/executors/github.ts) and stripTrailingAntigravityAssistantTurn (open-sse/executors/antigravity.ts). Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: growab <nekron@icloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When using newer Anthropic models (e.g. Claude 4.6 Sonnet) or specific configurations where the upstream provider enforces strict formatting constraints, assistant message prefill is not supported. Claude Code automatically sends trailing assistant messages in its payloads, triggering a 400 Bad Request error:
This model does not support assistant message prefill. The conversation must end with a user message.
Solution
Strip trailing assistant prefill messages inside the Claude Code MITM handler before forwarding the request to the router. This ensures the messages array always ends with a user message.
Acceptance Criteria