Skip to content

fix(coderouter): fail OpenCode uploads cleanly when the client body errors on Bun 1.3 - #16360

Closed
lawrencecchen wants to merge 1 commit into
mainfrom
fix-opencode-upload-error
Closed

lawrencecchen wants to merge 1 commit into
mainfrom
fix-opencode-upload-error

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

web / Web tests (2/4) fails on main since #16165: coderouter OpenCode Go proxy > pinned fetch fails the upload when the client body fails dies with an uncaught error: client went away, and Bun blames the next test (routes around an unavailable OpenCode account) too (main run, job 110212202838).

pinnedFetch streamed the client body to the provider through Readable.fromWeb(body). On Bun 1.3.14, which CI runs and web/scripts/run-tests.sh declares as the minimum, fromWeb throws a source-stream error as an uncaught exception instead of passing it to pipeline; even process.on("uncaughtException") doesn't see it and the process exits. Bun 1.4.2 and Node pass the error to pipeline's callback, so the test passed locally on newer Bun. Production runs on Node, so live traffic wasn't affected, but the test is the spec and the code has to meet it on the supported runtime.

The body is now read with Readable.from(body, { objectMode: false }). A web ReadableStream is an async iterable, so pipeline receives its error, destroys the upstream request and rejects the fetch on Bun 1.3.14, Bun 1.4.2 and Node.

Testing

  • Standalone repro (web stream errors after one chunk, piped to a Writable): Readable.fromWeb crashes Bun 1.3.14; Readable.from delivers the error to the pipeline callback on Bun 1.3.14, Bun 1.4.2 and Node 24. A happy-path POST through Readable.from echoes the exact body on Node and Bun 1.3.14.
  • bun test tests/coderouter-opencode-proxy.test.ts on Bun 1.3.14: 2 fail before, 15 pass / 0 fail after. Bun 1.4.2: 15 pass both before and after.
  • ./scripts/run-tests.sh --shard 2/4 on Bun 1.3.14: 1179 pass, 0 fail.
  • tsc --noEmit and bun run lint:complexity clean.

Changelog

none

🤖 Generated with Claude Code


Summary by cubic

Fixes a crash in the OpenCode Go proxy when a client body fails during upload on Bun 1.3.14, which was failing the web tests 2/4 shard.

Bug Fixes

  • Streaming the client body with Readable.fromWeb made Bun 1.3 throw source-stream errors as uncaught exceptions instead of passing them to pipeline, crashing the test process and blaming the next test.
  • The body is now read with Readable.from as an async iterable, so pipeline receives the error, destroys the upstream request, and rejects the fetch on Bun 1.3, Bun 1.4, and Node alike.
  • Production runs on Node, so live traffic never hit this.

Written for commit 2f675e7. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability for requests with streamed bodies in Bun. Stream errors are now handled without escaping as uncaught exceptions, helping prevent unexpected interruptions during request processing.

… errors on Bun 1.3

pinnedFetch fed the client request body to the provider through
Readable.fromWeb. On Bun 1.3.14, the minimum Bun the web tests support and
the version CI runs, fromWeb throws a source-stream error as an uncaught
exception instead of handing it to pipeline, so "pinned fetch fails the
upload when the client body fails" crashed the test process and blamed the
next test. Read the body as an async iterable with Readable.from instead;
pipeline then sees the error and destroys the upstream request on Bun 1.3,
Bun 1.4 and Node alike.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
.github/review-bot-rules/source-control-artifacts.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: afb9a8b7-818e-44db-a3df-f07a398e9d88

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4c52e and 2f675e7.

📒 Files selected for processing (1)
  • web/services/coderouter/opencodeProxy.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

pinnedFetch now converts the request body with Readable.from in byte mode before piping it to the outgoing request. A comment describes Bun 1.3's behavior with source-stream errors from Readable.fromWeb.

Changes

Proxy request stream

Layer / File(s) Summary
Convert request body stream
web/services/coderouter/opencodeProxy.ts
pinnedFetch uses Readable.from with objectMode: false to convert the web stream before piping it to the outgoing request. A comment notes Bun 1.3's uncaught-exception behavior with Readable.fromWeb.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 2f675

The proxy continues to forward request bodies and reject stream failures; no concrete production regression is established. The change appears ready to merge, though Bun 1.3 validation is author-reported.

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the affected component, failure mode, and Bun 1.3 fix. It accurately summarizes the main change.
Description check ✅ Passed The description explains the problem, implementation, expected behavior, testing performed, and changelog status. The optional demo video and checklist sections are omitted, but the core required info…
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 1 files.
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.
Cmux Cloud Persistent Session And Early Input ✅ Passed The rule applies to Cloud terminal creation, persistent cmux-tui transport, manual panes, and terminal runtime admission. The PR changes only web/services/coderouter/opencodeProxy.ts, replacing `Rea…
Cmux Swift Actor Isolation ✅ Passed PASS. The pull request changes only web/services/coderouter/opencodeProxy.ts and introduces no Swift changes. The Swift actor-isolation check is therefore inapplicable.
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts. It introduces no production Swift changes and no Swift blocking or timing-based synchronization.
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts. Its diff changes request-body stream handling from Readable.fromWeb to Readable.from; it does not change browser soc…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts, a TypeScript file. It adds no production Swift code and does not modify any agent-history load, workspace/panel/tab/wind…
Cmux Cache Substitution Correctness ✅ Passed The diff changes only the request-body stream adapter in pinnedFetch: it replaces Readable.fromWeb(body) with Readable.from(body, { objectMode: false }) before pipeline. It does not replace an…
Cmux No Hacky Sleeps ✅ Passed The production TypeScript diff changes only request-body stream conversion and pipeline handling. It adds no sleep, timer, polling loop, fixed delay, or wall-clock wait. The timing-related terms found…
Cmux Algorithmic Complexity ✅ Passed The pull request changes one stream-conversion operation in pinnedFetch: it replaces Readable.fromWeb(body) with Readable.from(body, { objectMode: false }). The diff adds no nested collection sc…
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes one TypeScript file, web/services/coderouter/opencodeProxy.ts, and changes Readable.fromWeb to Readable.from. The diff contains no Swift code and no Swift concurre…
Cmux Swift @Concurrent ✅ Passed The pull request changes only web/services/coderouter/opencodeProxy.ts, a TypeScript file. The reviewed diff contains no Swift files, Swift functions, or Swift call sites. The Swift @concurrent ch…
Cmux Swift Package Boundaries ✅ Passed PASS: The authoritative PR diff changes only web/services/coderouter/opencodeProxy.ts, a TypeScript web service. It introduces no production Swift changes, so the Swift package-boundary rule does no…
Cmux Swiftpm Lockfiles ✅ Passed The authoritative PR diff contains only web/services/coderouter/opencodeProxy.ts. It changes a TypeScript stream conversion and includes no SwiftPM package, Package.swift, Package.resolved, `.gi…
Cmux Swift Logging ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts, a TypeScript file. The diff adds stream-conversion logic and comments only. It adds no Swift logging or diagnostic outpu…
Cmux User-Facing Error Privacy ✅ Passed PASS: The diff only changes the internal request-body stream adapter and adds a developer-only Bun comment. It adds no user-facing error text or payload. In the production proxy path, body failures ar…
Cmux Full Internationalization ✅ Passed PASS. The PR changes only web/services/coderouter/opencodeProxy.ts, replacing Readable.fromWeb with Readable.from and adding developer-only comments. It introduces or changes no user-facing text…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts. The diff contains TypeScript stream handling and no Swift or SwiftUI code, so it introduces none of the state, layout, r…
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts, a TypeScript file. It introduces no Swift architecture changes, timing repair paths, extra state owners, duplicate UI wi…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts. The diff contains no Swift files and introduces no NSWindow, NSPanel, NSWindowController, SwiftUI Window, or `Wi…
Cmux Source Artifacts ✅ Passed The PR changes only web/services/coderouter/opencodeProxy.ts. The diff is a hand-written source change that replaces Readable.fromWeb with Readable.from and adds an explanatory comment. It does …
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The pull request changes only web/services/coderouter/opencodeProxy.ts. The diff contains no Swift file under a production Sources/ path, so the no-test-or-debug-seam-in-production-source ch…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 1 file

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on 2f675e7ac8 (run 36814886596 attempt 1): 2 unknown.

Job Verdict Why
web / web-db-migrations unknown no known signature; failed step: Apply migrations
web / Web tests (1/4) unknown no known signature; failed step: Run web test shard

Not re-run automatically: web / web-db-migrations, web / Web tests (1/4) are not machine failures.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

@lawrencecchen main already fixes this. #15154 (e96920b) replaced Readable.fromWeb in pinnedFetch with writeWebRequestBody, which reads the web stream directly, and that's why this PR conflicts.

I merged main into this branch locally. The only conflict was that pinnedFetch hunk, and taking main's side leaves this PR with no diff. With main's code, bun test tests/coderouter-opencode-proxy.test.ts on Bun 1.3.14 gives 15 pass, 0 fail. So I didn't push an empty merge to your branch. If you'd rather keep the Readable.from approach over main's, say so and I'll merge it the other way. Otherwise this can be closed.

The same failure still cancels Web tests (1/4) on feat-cmux-next until catch-up brings e96920b over.

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Superseded on main: #15154 (e96920b) replaced the Readable.fromWeb pipeline in pinnedFetch with writeWebRequestBody(body, outgoing, init.signal), which reads the web stream directly and rejects on a body error. That is why this branch now conflicts. If coderouter OpenCode Go proxy > pinned fetch fails the upload when the client body fails passes on Bun 1.3.14 at current main, this PR can close.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Closing: covered on main by #15154 (e96920b), which replaced the Readable.fromWeb pipeline with writeWebRequestBody.

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