Skip to content

fix(executors): rotate on upstream 400 empty-body rejections (opencode) - #11158

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
maxmad64bis:fix/opencode-empty-rejection-rotation
Aug 23, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.50from
maxmad64bis:fix/opencode-empty-rejection-rotation

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • OpencodeExecutor treated a malformed upstream 400 — a completion envelope with no error field, empty content, and finish_reason: null — as a successful response and propagated it fatally. On free-tier models this killed client and subagent sessions (observed on muse-spark-1.2-contributor-free).
  • The executor now detects that signature and rotates (or, on a single direct account, retries once) instead of propagating the empty rejection as success.
  • Body reads are conditioned on status === 400 so the 200/streaming success path is never buffered (anti-bufferisation invariant).
  • 400s that carry a real error field still propagate immediately and untouched — no cooldown, no success, no rotation.

Related Issues

Validation

Focused loop from the Contribution Golden Path:

  • Change type: executors / routing (open-sse executor)
  • Focused tests added: tests/unit/opencode-empty-rejection-rotation.test.ts (11 tests) + predicate suite in tests/unit/account-rotation.test.ts (11 added)
  • node --import tsx/esm --test green — 101/101 across account-rotation + proxy-rotation-4954 + opencode-executor + new suite
  • bun scripts/check/check-provider-consistency.ts OK (266 registry entries, 0 exceptions)
  • ESLint clean on all changed files (no-explicit-any zero-warning policy); the pre-existing any in transformRequest is outside this PR's diff.

Tests Added Or Updated

  • tests/unit/opencode-empty-rejection-rotation.test.ts (new, 11 tests): loop rotation on empty 400, bounded N-account cap + intact-body propagation, single-account retry-once, 429/empty-400/200 coexistence, error-400 passthrough, fast-path retry + intact propagation, anti-bufferisation on 200 (loop + fast path), shared-egress-guard interaction.
  • tests/unit/account-rotation.test.ts (extended, +11 tests): isEmptyUpstreamRejection signature table + extractChatcmplId.

Coverage Notes

  • Touches open-sse/executors/opencode.ts and open-sse/executors/accountRotation.ts; covered by the new unit suites above. CI runs the 60% gate.

Reviewer Notes

  • Predicate deliberately does not reuse detectMalformedNonStream (diagnostics.ts): that classifier also flags {error:{…}} bodies as empty_choices, which would rotate on genuine errors (false-positive class with prior history).
  • Loop budget is +1 attempt only when a single account exists; multi-account fleets rely on rotation through the accounts (no extra loop). This caps a persistently malformed upstream at N attempts.
  • Fast path preserves BaseExecutor's intra-URL 429 retries (no skipUpstreamRetry there).
  • No cooldown/markSuccess is applied on the empty rejection: the failure is upstream's, not this account's.

Why a targeted fix and not a refactor of base.ts

base.ts already has four intra-URL 400 recovery passes (context-editing, thinking-budget clamp, generic field-downgrade, WAF content_blocked) plus isProviderModelUnsupported400 in accountFallback.ts. They all retry the same URL, same account, by mutating the request body — which is precisely what this fix must not do: the observed envelope is empty/unparseable (finish_reason: null, no content, no error field), so there is nothing to mutate and the correct response is rotate the account (or retry once on a single direct account).

The OpencodeExecutor loop deliberately sets skipUpstreamRetry: true so base.ts's intra-account retries do not fire — this loop owns the cross-account fallback. So base.ts performs no rotation on a 400, and there was no existing "rotate-on-empty-400" mechanism to reuse. This PR fills that gap where it belongs: the rotation owner.

This is intentionally not generalized into a shared 400-classifier in base.ts (which would be the more "meta" design). Reasons:

Follow-up (not in this PR): generalize the 400 classification in base.ts into one pluggable table that can also emit a "rotate account" signal, so future noauth executors (e.g. Mimocode) share this logic instead of each re-implementing a body-read-and-classify step.

OpencodeExecutor treated a malformed upstream 400 (completion envelope with no
error field, empty content, null finish_reason) as a successful response and
propagated it fatally — killing client/subagent sessions on free-tier models.

- Add isEmptyUpstreamRejection / extractChatcmplId predicates in accountRotation.ts
  (strict signature: status 400, no error field, no real content/tool_calls,
  null finish_reason; conservative on other content shapes).
- Loop now retries/rotates on an empty 400 (bounded +1 only for a single account;
  multi-account rotation through the fleet is the retry). Body is read only for a
  400 (never a 200/streaming) so the success path is never buffered.
- Fast path (single direct account, no proxies) retries exactly once on an empty 400.
- 400s carrying a real error field still propagate immediately, untouched.
- Clean up the now-obsolete Mimocode mention in the accountRotation header (diegosouzapw#10130).

Tests: accountRotation predicate suite + OpencodeExecutor wiring suite
(rotation, bounded retry, fast path, anti-bufferisation, error-400 passthrough)
against the existing proxy-rotation-4954 fetch-stub harness.

Refs upstream precedent diegosouzapw#10402 (rotate beyond 429), diegosouzapw#10460 (classify 400 by
signature before rotating).
@maxmad64bis
maxmad64bis marked this pull request as ready for review August 22, 2026 20:21
@diegosouzapw
diegosouzapw merged commit 1058426 into diegosouzapw:release/v3.8.50 Aug 23, 2026
17 of 24 checks passed
@maxmad64bis
maxmad64bis deleted the fix/opencode-empty-rejection-rotation branch September 24, 2026 21:14
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e) (diegosouzapw#11158)

Validated on the combined batch board over tip 4813c32: static gates clean (changelog, file-size, complexity 2624<=2774, cognitive 1182<=1223, dead-code 411<=416), typecheck:core clean, focused tests green.

Empty-envelope 400 (no error field, empty content, finish_reason null) now rotates/retries instead of propagating as success; 200/streaming path never buffered; real-error 400s untouched. account-rotation + new rotation suite 34/34 on the board. Thank you @maxmad64bis!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants