Skip to content

Preserve terminal stream disconnect classification - #1563

Open
brsbl wants to merge 2 commits into
mainfrom
bb/retry-terminal-stream-disconnect-thr_957f3xid9y
Open

Preserve terminal stream disconnect classification#1563
brsbl wants to merge 2 commits into
mainfrom
bb/retry-terminal-stream-disconnect-thr_957f3xid9y

Conversation

@brsbl

@brsbl brsbl commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • Preserve stream-disconnected classification when Codex exhausts reconnect attempts and reports the terminal failure as codexErrorInfo: "other".
  • Keep the workaround narrow to the exact stream disconnected before completion message prefix; retry semantics and unrelated generic errors are unchanged.

Local reproduction

The deterministic replay below uses the exact terminal Codex event from the incident. It does not require intentionally breaking your network.

1. Reproduce on main

From a fresh clone:

git clone https://github.com/get-bb/bb.git bb-pr-1563
cd bb-pr-1563
pnpm install --frozen-lockfile
git switch main

node --conditions=source --import tsx --input-type=module <<'NODE'
import { createCodexProviderAdapter } from "./packages/agent-runtime/src/codex/adapter.ts";

const [event] = createCodexProviderAdapter().translateEvent({
  jsonrpc: "2.0",
  method: "error",
  params: {
    threadId: "repro-thread",
    turnId: "repro-turn",
    error: {
      message:
        "stream disconnected before completion: error sending request for url (https://chatgpt.com/backend-api/codex/responses)",
      codexErrorInfo: "other",
      additionalDetails: null,
    },
    willRetry: false,
  },
});

if (event?.type !== "provider/error") throw new Error("Expected provider/error");
console.log(event.errorInfo);
NODE

Observed on main (e1ec66e8c when this PR was opened):

{ category: 'unknown', providerCode: 'other', httpStatusCode: null }

This is the bug: the terminal event loses the structured disconnect category, so BB renders the final timeline row as Provider error.

2. Verify this PR

In the same clone:

git fetch origin pull/1563/head:pr-1563
git switch pr-1563

Rerun the same node ... <<'NODE' block above. Expected output:

{
  category: 'stream-disconnected',
  providerCode: 'responseStreamDisconnected',
  httpStatusCode: null
}

BB can now render the terminal row consistently as Provider stream disconnected.

Optional real-network reproduction

  1. Start a Codex turn that runs long enough to keep the response stream open.
  2. After the turn is accepted, temporarily disable the machine's network or DNS until Codex exhausts its reconnect attempts.
  3. Restore connectivity and inspect the thread events:
bb thread log <thread-id> --format json --limit 5000 \
  | jq '[.[] | select(.type == "provider/error") | {
      seq,
      willRetry: .data.willRetry,
      errorInfo: .data.errorInfo,
      detail: .data.detail
    }] | .[-10:]'

Before this fix, reconnect attempts carry responseStreamDisconnected, but the final willRetry: false event can carry providerCode: "other" and category: "unknown" even though its detail still starts with stream disconnected before completion:. Network behavior and retry timing make this path less deterministic than the event replay above.

Root cause

Codex preserves responseStreamDisconnected during reconnect attempts, then can downgrade the same terminal failure to codexErrorInfo: "other" after its retry budget is exhausted. BB trusted that degraded terminal value and discarded the more specific classification encoded in the unchanged message.

Verification

  • Deterministic replay on main: reproduced category: "unknown".
  • The same replay on this PR: returns category: "stream-disconnected".
  • Regression test failed before the production change and passed afterward.
  • pnpm exec turbo run test --filter=@bb/agent-runtime: 46 files, 943 tests passed.
  • pnpm exec turbo run typecheck --filter=@bb/agent-runtime: passed.
  • Review loop: CORRECT, no P0–P2 findings.
  • GitHub CI: green.

BB-Thread-ID: thr_957f3xid9y

AGENT GENERATED: by GPT-5.6 Sol

@bb-slop-cop

bb-slop-cop Bot commented Aug 13, 2026

Copy link
Copy Markdown

🚨 SLOP COP 🚨 · review

I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and practical behavior.

I will post the complete review after the parallel checks finish.

error: {
message:
"stream disconnected before completion: error sending request for url (https://chatgpt.com/backend-api/codex/responses)",
codexErrorInfo: "other",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — Please add one negative case for an unrelated other error.

The PR states that unrelated generic errors stay unchanged. This test proves only the positive match.

Add a second case with message: "request failed". Assert category: "unknown" and providerCode: "other". This case will protect the narrow guard.

@bb-slop-cop bb-slop-cop Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 SLOP COP 🚨 · review

ELI5: Codex sometimes puts the wrong name on a lost connection. This change adds the correct label, like a label on a toy box.

Review result

I found no security, correctness, performance, or architecture defect. The Codex adapter is the correct layer, and no duplicate classifier exists.

One minor test gap remains. The new test covers the matching other error, but it does not protect unrelated other errors. I left an inline suggestion for a negative case.

The anchored regular expression has a bounded cost. The category change does not affect retries, process restarts, credentials, or rate-limit recovery.

I ran these checks:

  • pnpm exec turbo run test --filter=@bb/agent-runtime --force: 46 files and 943 tests passed.
  • pnpm exec turbo run typecheck --filter=@bb/agent-runtime: passed.
  • A direct adapter replay returned stream-disconnected with responseStreamDisconnected.
  • All required GitHub CI checks pass.

I did not run a browser test because this change has no web route. The direct adapter replay tested the complete changed path.

This is a comment-only review. I did not approve the PR or request changes.

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.

1 participant