Skip to content

compat auto-fix: developer→system retry can still 400 on system-at-index-0 backends (#377 class); add second-chance developer→user learn #583

Closed

Description

@ranxianglei

Context

#552 / PR#579 shipped a learn-on-failure auto-fix for role-rejecting upstreams: on a 400 like Invalid role: developer, bili rewrites the role to system, retries once, and on success remembers the mapping in session.metadata.learnedCompatRoles (session-scoped; config file untouched). If the retry fails, the original 400 is passed through verbatim.

The edge

An upstream can enforce both:

  1. unknown roles are rejected (developer → 400), and
  2. system messages are only accepted at index 0 (SGLang-style strict placement — the same class as [Bug] ACP: ≥2 compressed blocks render as multiple system messages → SGLang (vLLM-compatible) upstreams return 400 #377).

If the developer message sits mid-list (codex sends developer role messages in Responses input[]), the auto-retry rewrites it to a mid-list system → the retry 400s again (a #377-class placement error, not a role-name error) → the original error passes through verbatim. No loop, no corruption — but this sub-case stays broken and the auto-learn never engages.

Scope

  • Responses path only. The chat path is unaffected: the rebuild chain systemToUser(coreToOpenai(...)) (src/server.ts:1864, src/loop/adapter-openai.ts:193) already normalizes mid-list system/developer to user before the wire, so the compat rewrite is a no-op there.
  • Existing escape hatch: manual config compat.roles {"developer": "user"} already covers such upstreams.

Suggested fix

Second-chance learning, mirroring the existing one-retry discipline:

  • Teach detectRoleRejection (src/compat-roles.ts) a conservative pattern for system placement errors (e.g. system message only at index 0, multiple system messages).
  • When the first retry (→ system) fails with that error class, do one more controlled retry with {role: "user"}.
  • If that succeeds, learn developer → user for the session (position-class learning wins over the generic one).
  • Hard cap stays at: original attempt + 2 retries, no loops; anything still failing passes the original 400 through verbatim.

Found while answering a maintainer question about the relationship between #377 and #552 after PR#579 merged.

Activity

  1. ranxianglei commented on Sep 6, 2026

    @ranxianglei
    OwnerAuthor

    🤖 ework agent · qwen3.8-27b

    Implemented your suggested fix and opened a PR: #585

    What I verified first (triage)

    I reproduced the edge against a mock placement-strict upstream and traced the Responses pipeline. One finding worth flagging because it refines the issue's mental model:

    A well-formed developer item does NOT survive mid-list. The kernel's responsesToCore pulls every system/developer message into its front system parts, and injectResponsesDeveloperMessage re-injects them as a single front developer message (index 0). So through the normal compression path, a rejected developer becomes a front developer, and the existing #552 primary retry (→system) already lands at index 0 → accepted by SGLang-style backends → the second-chance never fires there.

    A developer reaches the forward boundary mid-list only in two narrow cases:

    1. the item is non-normalized (e.g. missing type) — normalizeResponsesMessageItems deliberately does not stamp type for developer/system roles, so such items are left in place; or
    2. the unchanged body is forwarded (the transform-failure fallback in src/server.ts).

    So this is a defensive safety-net for those cases rather than a common-path fix — but it closes the class you described cleanly. Chat path is unaffected, as you noted: systemToUser(coreToOpenai(...)) already normalizes mid-list system/developer→user before the wire.

    What the PR does

    • detectSystemPlacementError(status, bodyText) in src/compat-roles.ts — conservative detector for system-placement 400s (Only one 'system'…at the beginning, Multiple system messages, system … at index N, …). Requires a 400 plus the word "system" plus a strong placement/multiplicity marker, so quota / overflow / missing-system / auth 400s never trip it. Kept as a separate function from detectRoleRejection on purpose — a placement error names no role, and this keeps the {role} contract stable.
    • src/server.ts: the single retry becomes a fixed ladder — primary developer→system; only if that 400s with a placement error, one more developer→user. On success the session learns the winning mapping (developer→user beats the generic →system). Hard cap unchanged: original + 2 retries, never a loop; anything still failing passes the original 400 through verbatim.

    Tests & pre-flight

    • New unit test for detectSystemPlacementError; e2e G (full ladder + cross-request learn) and H (every hop fails → hard cap, verbatim original 400); the existing billion-context: 400 Bad Request when codex sends role: "developer" messages — request a request-body compat transform hook #552 E/F regression tests still pass.
    • tsc --noEmit clean, npm run build OK. Full suite green except a pre-existing, environment-dependent launcher.test.ts case (codex resolve to themselves) that fails identically on a clean tree whenever a real codex binary is on PATH — unrelated to this change.
    • The gated Responses e2e (tests/e2e/e2e-codex.test.ts) needs a live upstream so wasn't run here; CI manual-dispatch covers it.

    Merging stays with you. If you'd prefer the second-chance be narrower (e.g. only when the offending item was actually mid-list rather than any placement-400), say the word and I'll tighten it.

  2. added a commit that references this issue on Sep 7, 2026
  3. ranxianglei commented on Sep 7, 2026

    @ranxianglei
    OwnerAuthor

    修复已随 PR#585 合并 (ba55fea): dev→system 重试仍 400 时, 再试一次 dev→user 并学习该 provider 的组合偏好; 场景 = 同时强制 no-developer 和 system-at-index-0 的后端。PR 标题里的 (#583) 不是 closing 关键字所以没有自动关, 手动关闭。

  4. added a commit that references this issue on Sep 9, 2026
  5. added a commit that references this issue on Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions