Skip to content

fix(devin-cli): do not write to a cancelled stream when the client disconnects - #16607

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
pacocartones:fix/devin-cli-client-disconnect
Oct 15, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.52from
pacocartones:fix/devin-cli-client-disconnect

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • When the HTTP client disconnects mid-stream, the devin-cli executor's child close handler calls controller.enqueue() / controller.close() on a cancelled stream. That throws ERR_INVALID_STATE inside a ChildProcess listener, an uncaught exception that can take down the server and every other stream in flight.
  • Root cause: the ReadableStream has no cancel(); the abort listener only kills the child, and the child's close handler then writes to a controller whose consumer is gone.

Related Issues

Target Branch

  • Base is the active release/v3.8.x branch (the highest open release/v*), not main

Proof Of Fix (required)

  • TDD (preferred) — a test that fails before the fix and passes after it.
    • Test file(s): tests/unit/devin-cli-client-disconnect.test.ts (node as the fake acp CLI, the same trick as devin-cli-fallback-spawn-local-only-15159.test.ts)
    • Failing output before the fix:
      TypeError [ERR_INVALID_STATE]: Invalid state: Controller is already closed
      

Tests Added Or Updated

  • tests/unit/devin-cli-client-disconnect.test.ts: the client cancels the body and aborts the signal mid-stream; the test asserts that no uncaughtException fires.

Coverage Notes

  • Covers cancel() and the guarded enqueue / close in devin-cli.ts; the neighbouring devin unit tests keep passing.

Scope

  • Only the files the failing test proves need changing are touched (devin-cli.ts, the test and the changelog fragment)
  • No test was weakened or deleted to make the suite pass

Validation

  • Change type: provider / executor
  • Focused tests and category gates from the golden path: the devin unit tests pass (14 tests in the four neighbouring devin-cli files); check:ai-attribution, check:changelog-integrity and check:file-size pass
  • npm run lint (the repository-wide run is left to CI; eslint reports one no-unused-vars on ProviderCredentials in devin-cli.ts that is already on the base)

Reviewer Notes

  • Only the executor changes; streams that are not cancelled behave as before.

…sconnects

The devin-cli executor streams from a spawned devin acp child. When the client disconnects, the response body is cancelled and the abort signal kills the child, whose close handler then called controller.enqueue() and controller.close() on a cancelled stream. That throws ERR_INVALID_STATE inside a ChildProcess listener, an uncaught exception that takes down the server and every other stream in flight. Define cancel() to mark the consumer gone and stop the child, and guard enqueue/close.
pacocartones and others added 2 commits October 14, 2026 18:49
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 5c60bd8 into cheaper-inference:release/v3.8.52 Oct 15, 2026
46 of 60 checks passed
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