fix(daemon,serve): handle log WriteStream and HTTP body stream errors

[SebTardif]

What Problem This Solves

Two long-lived Node stream paths in mcporter can take down or leak work under ordinary I/O failures:

1. Daemon log file (createLogContext): the append WriteStream had no error listener until dispose. Disk-full / EIO on that stream becomes an uncaughtException and kills the daemon process.
2. HTTP serve body (handleNodeRequest): only body.on('error') destroyed the response. Client abort / response-side errors did not destroy the body stream, so partial writes and open readers could leak until GC.

Summary

• Attach an error listener immediately after opening the daemon log WriteStream; warn, drop the writer, and keep logging to console.
• Route serve response bodies through pipeline() (exported as pipeHttpResponseBody) so body and HTTP response clean each other up on either side error.

Evidence

Live node against a build of this branch (not a test runner):

$ pnpm build
$ node -e '/* createLogContext + emit ENOSPC; pipeHttpResponseBody + response error */'
writer error listeners: 1
uncaught after ENOSPC emit: 0
writer cleared: true
[daemon] Log file write error (.../daemon.log): ENOSPC: no space left on device
body destroyed after response error: true

Without the log fix, listenerCount('error') is 0 and emitting ENOSPC throws as an uncaught exception. Without the serve fix, client-side response errors leave the body stream alive.

Real behavior proof

• Behavior or issue addressed: Daemon log WriteStream errors no longer crash the process; HTTP serve body/response errors clean up both sides of the pipe.

• Real environment tested: macOS, Node v26.5.1, mcporter built from this branch under /tmp/oc-mcporter-pr.

• Exact steps or command run after this patch: Built with pnpm build, then ran a short node script that opens a daemon log context, emits ENOSPC on the writer, and pipes a Readable through pipeHttpResponseBody while the destination emits socket hang up.

• Evidence after fix: terminal output from the patched build:

  writer error listeners: 1
  uncaught after ENOSPC emit: 0
  writer cleared: true
  [daemon] Log file write error (.../daemon.log): ENOSPC: no space left on device
  body destroyed after response error: true

• Observed result after fix: ENOSPC is logged and the writer is cleared with zero uncaught exceptions; response-side errors destroy the request body stream.

• What was not tested: Full multi-hour daemon under real disk pressure, and live MCP clients aborting mid-tool-response on a production host.

Test plan

• Red-green: tests/daemon-log-context.test.ts failed on main behavior (0 error listeners / uncaught ENOSPC), passes with the fix
• Red-green: tests/serve-stream-errors.test.ts failed with one-way pipe (hang / uncaught), passes with pipeline
• pnpm check (format, oxlint, typecheck)
• pnpm exec vitest run tests/daemon-log-context.test.ts tests/serve-stream-errors.test.ts tests/serve.test.ts tests/serve-edge.test.ts

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 6, 2026, 11:54 PM ET / August 7, 2026, 03:54 UTC.

ClawSweeper review

What this changes

The PR adds immediate daemon log-stream error handling and replaces one-way HTTP body piping with a bidirectional Node stream pipeline so either endpoint is cleaned up after I/O errors.

Merge readiness

⛔ Blocked until stronger real behavior proof is added - 3 items remain

Keep open: current main still has both stream-error gaps, and no patch defect was found. The HTTP half still needs real serving-boundary proof before merge. Likely related people: steipete (medium confidence).

Priority: P1
Reviewed head: 722f57ccdcf3db5616a38c8fdf42bdf242766bd1

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused implementation and tests are sound on review, but the required real HTTP abort proof is not yet present.
Proof confidence	🦐 gold shrimp (3/6)	Needs stronger real behavior proof before merge: Built-branch terminal output directly supports the log-stream case, but the HTTP case remains helper-level rather than a real serve endpoint with an aborting client; add redacted production-boundary output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Built-branch terminal output directly supports the log-stream case, but the HTTP case remains helper-level rather than a real serve endpoint with an aborting client; add redacted production-boundary output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	5 items	Current main retains the daemon error gap: The current log context assigns the WriteStream directly and has no persistent error listener before later writes. Current main retains one-way response piping: The current serve path listens only for body errors and then calls body.pipe(response), so it does not explicitly coordinate destination-side failure cleanup. PR head provides a focused implementation: The branch adds a WriteStream listener that drops the failed writer and routes the response body through pipeline with cleanup for either endpoint.
Findings	None	None.
Security	None	None.

How this fits together

Daemon logging mirrors daemon activity to an optional local file, while the HTTP serve path forwards MCP handler response bodies to connecting clients. Both are long-lived I/O boundaries where stream failures must not crash the daemon or leave an upstream body alive.

flowchart LR
  A[Daemon events] --> B[Log writer]
  B --> C[Local log file]
  D[MCP handler response] --> E[Response body stream]
  E --> F[HTTP response]
  F --> G[Client connection]
  B --> H[Error containment]
  E --> H

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Built-branch terminal output directly supports the log-stream case, but the HTTP case remains helper-level rather than a real serve endpoint with an aborting client; add redacted production-boundary output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve merge risk (P1) - A mocked Writable and an injected helper error do not prove that the actual Node HTTP response and an aborting client trigger the intended cleanup without affecting normal serving.
• Complete next step (P2) - The remaining merge blocker is contributor-supplied real behavior proof, not a mechanical repair suitable for an automated fix lane.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch size	production +30/-3, tests +120 across 4 files	The change stays focused on two stream-failure boundaries and adds dedicated regression coverage.

Merge-risk options

Maintainer options:

1. Add production-boundary abort proof (recommended)
   Run the patched serve endpoint with a real HTTP client that aborts mid-response and provide redacted output showing the response body is destroyed.

Technical review

Best possible solution:

Keep the focused listener and pipeline approach, supported by a redacted real serveHttp/client-abort trace that demonstrates body destruction after the client disconnects.

Do we have a high-confidence way to reproduce the issue?

No for the complete reported behavior: current source establishes the missing listener and one-way pipe, but the supplied HTTP evidence does not exercise a real serve endpoint and aborting client.

Is this the best way to solve the issue?

Yes in design: Node pipeline is the appropriate narrow primitive for bidirectional stream teardown, pending proof at the actual HTTP serving boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5f30f68d044b.

Labels

Label justifications:

• P1: An unhandled daemon log-stream error can terminate an active daemon process.
• merge-risk: 🚨 availability: The patch changes failure handling for daemon logging and active HTTP response streams.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Built-branch terminal output directly supports the log-stream case, but the HTTP case remains helper-level rather than a real serve endpoint with an aborting client; add redacted production-boundary output before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current main retains the daemon error gap: The current log context assigns the WriteStream directly and has no persistent error listener before later writes. (src/daemon/log-context.ts:26, 49dcd3e7fffd)
• Current main retains one-way response piping: The current serve path listens only for body errors and then calls body.pipe(response), so it does not explicitly coordinate destination-side failure cleanup. (src/serve.ts:235, 49dcd3e7fffd)
• PR head provides a focused implementation: The branch adds a WriteStream listener that drops the failed writer and routes the response body through pipeline with cleanup for either endpoint. (src/serve.ts:236, 722f57ccdcf3)
• Current main does not include the PR head: The proposed commit is not an ancestor of current main, so neither fix is already implemented or released from this branch. (722f57ccdcf3)
• Proof remains helper-level for HTTP serving: The PR body records built-branch terminal output, but its response-side failure is injected into the exported helper rather than observed through serveHttp with a real aborting client; this matches the prior review's unresolved proof requirement. (722f57ccdcf3)

Likely related people:

• steipete: Current-main blame attributes both the log context and HTTP forwarding code to the v0.13.0 snapshot authored by Peter Steinberger; available local history begins at that release graft. (role: current implementation origin and recent area contributor; confidence: medium; commits: 49dcd3e7fffd; files: src/daemon/log-context.ts, src/serve.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Capture a redacted serveHttp-to-aborting-client trace that shows the response body is destroyed after disconnect.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (6 earlier review cycles)

• reviewed 2026-08-06T02:30:04.578Z sha 722f57c :: needs maintainer review before merge. :: none
• reviewed 2026-08-06T05:20:46.389Z sha 722f57c :: needs maintainer review before merge. :: none
• reviewed 2026-08-06T07:17:30.549Z sha 722f57c :: needs maintainer review before merge. :: none
• reviewed 2026-08-06T09:16:02.868Z sha 722f57c :: needs maintainer review before merge. :: none
• reviewed 2026-08-06T11:09:16.376Z sha 722f57c :: needs maintainer review before merge. :: none
• reviewed 2026-08-07T01:53:44.805Z sha 722f57c :: needs real behavior proof before merge. :: none