Skip to content

fix(streaming): bound SSE payloads, sockets, streams and subprocesses (salvage #759, #762, #765) - #789

Merged
unohee merged 1 commit into
mainfrom
salvage/streaming-bounds
Sep 28, 2026
Merged

unohee merged 1 commit into
mainfrom
salvage/streaming-bounds

Conversation

@unohee

@unohee unohee commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Salvages the streaming/execution-bounds work from three abandoned draft PRs into one reviewable change on current main:

Draft Kept Dropped
#759 rollback.ts exact stash match, gitStatus.ts maxBuffer + reject-on-error, httpBody.ts streaming decoder, workSessionRoutes.ts canonical-worktree I/O, tui/inputDebug.ts sandbox containment, knowledge/gitInfo.ts churn parser (+ tests) knowledge/graphqlExporter.ts (imports nonexistent ./repoSchema.js, calls graph.getNodes()/getEdges() that KnowledgeGraph does not define — dead code), junk (tmp-*, run-tests-workaround.sh, cursor-*.json, hooks.json)
#762 core/eventHub.ts payload/backpressure bounds, adapters/chatStream.ts retention caps, support/chatBackend.ts output caps, tui/components/LogLine.tsx render bound (+ tests) discord/discordPair.ts (net −226 lines; calls agentPair.getPairStats(), an export that does not exist, and 6 nonexistent locale keys — dead code), junk (.agt3429-*, .cliRunner-from-main.ts, tmp-hook-test.txt)
#765 processRegistry.ts escalation-timer clearing, ciWorker.ts gh run rerun timeout + maxBuffer, fixCommand.ts check timeout, support/dev.ts cancel-until-close adapters/base.ts and cli/workCommand.ts rewrites (destructively revert main's 6 intervening commits and do not compile against their own tree: settle() passes stdoutTruncated/stderrTruncated/signal/timedOut that CliRunResult does not define; broadcastEvent used but never imported)

Nothing was taken from main's own hardening: every hunk that conflicted with work main already shipped keeps main's version.

What was fixed during the salvage

The drafts' code was not taken verbatim where it was broken; each of these was reproduced and fixed:

  • Stash resolution (rollback.ts) was non-functional as drafted. fix(repository): harden filesystem, Git, and verification boundaries — prevent race conditions, ambiguous rollback, and unsafe execution #759 used git stash list --pretty=format:%gd: %gs and required msg === message, but %gs is the reflog subject and actually reads On <branch>: openswarm-checkpoint-abc, so the equality could never hold. Running fix(repository): harden filesystem, Git, and verification boundaries — prevent race conditions, ambiguous rollback, and unsafe execution #759's own tree (bf7ccae) gives 4 failed | 2 passed in rollbackStashIdentity.test.ts. The port matches the subject exactly (whole-subject, else exact : <message> tail), which also stops abc from matching abcd.
  • The churn parser (gitInfo.ts) misread real git output. The draft's state machine expected an empty NUL token between commits; real git log -z output has none, so the next commit's timestamp was parsed as a filename (inventing a file and dropping the real one). The port prefixes timestamps with an ASCII record separator (--format=%x1e%ct) so a timestamp token is self-identifying, and a numeric filename is never read as a date. Verified against a real repo: exact match on per-file counts, last-commit dates, and the set of paths.
  • SSE backpressure disconnected healthy clients. The drafts counted consecutive writes that returned false and disconnected at 64, but broadcastEvent is synchronous — one 23 KB stdout chunk fans out ~400 log events with no chance to drain in between, so an actively-reading dashboard client was destroyed mid-burst (verified over a real socket). Replaced the write count with a 4 MiB queued-byte cap plus a 30 s stall window; a healthy reader now survives a 400-frame burst and a stalled one is still dropped.
  • The chat stream's chunk retention corrupted tool calls and replies. Evicting the oldest of 1024 parsed chunks cut the head off streamed function.arguments (the tool layer then got unparseable JSON) and returned long replies as a suffix that re-entered the conversation as assistant content. Now the reduced shape is accumulated incrementally — bounded content, intact tool calls — and exceeding the tool-call cap throws instead of silently corrupting.
  • SSE backpressure added one drain listener per write, tripping MaxListenersExceededWarning on a slow client; now one listener per socket.
  • chatBackend head-truncated its partial line buffer, so a single >64 KB CLI event line removed the newlines flushLines needs and live streaming stopped for the rest of the turn; both the line buffer and stdout now keep the tail (which is also what extractChatResponse reads).
  • The partial-frame cap truncated before splitting, which silently dropped complete frames whenever a single read delivered more than the cap; now the split happens first and only the unterminated tail is bounded.
  • The sandbox check in inputDebug.ts was lexical only, so a symlinked directory inside ~/.openswarm could still redirect the write outside it. It now canonicalizes the nearest existing ancestor with realpath — and tolerates a first run, where ~/.openswarm does not exist yet and a bare realpathSync would silently drop the diagnostic line.

Verification

  • npx tsc --noEmit — clean.
  • New regression tests fail against the un-fixed versions and pass after (e.g. the symlink-escape test fails on the lexical-only check; the churn-parser test fails on the draft's state machine).
  • Affected suites: src/core/**, src/support/**, src/adapters/chatStream, src/tui/** → 132 files, 1455 tests passed.
  • Focused suites: eventHub, chatStream, gitStatus, inputDebug, rollback*, httpBody, gitInfo, LogLine, ciWorker → 182 tests passed.
  • End-to-end over a real HTTP server: a healthy client reading every frame survives a 400-event same-tick burst with 400/400 frames and no false disconnect, while a stalled client is dropped after the stall window (1 → 0); a truncated log line arrives at exactly MAX_LOG_LINE_CHARS with the ellipsis, and an event over MAX_EVENT_PAYLOAD_BYTES after field bounding is dropped rather than sent.
  • Four findings from an independent review pass (src/core/eventHub.ts burst disconnect, src/adapters/chatStream.ts chunk eviction, src/tui/inputDebug.ts first-run realpath, src/support/chatBackend.ts tail truncation) were each reproduced, fixed, and covered by a regression test that fails on the un-fixed code.

Deliberately not included

Files owned by sibling salvage groups were dropped rather than duplicated here, with the exact hunks handed to their owners: src/runners/cliRunner.ts and src/adapters/codexResponses.ts (output-sanitize group — frame limits into the reducer, truncation into the runner output path), src/adapters/processRegistry.ts and src/support/dev.ts (concurrency-locking group — the #766 version of killProcess subsumes #765's timer fix, and that group ships the union cancelTask/finalize form).

Salvages drafts #759, #762 and #765 onto current main:

- eventHub (#762): MAX_EVENT_PAYLOAD_BYTES frame cap, MAX_LOG_LINE_CHARS /
  MAX_CHAT_TEXT_CHARS field bounds, per-client SSE backpressure counters
  (WeakMap, one drain listener per socket) with disconnectClient.
- chatStream / chatBackend / LogLine (#762): retention caps for partial SSE
  frames, parsed chunks, assembled content, CLI stdout/stderr and rendered
  log lines.
- rollback (#759): resolve the checkpoint stash by EXACT subject match
  (stash@{N} is a position, and 'abc' must not match 'abcd').
- gitStatus (#759): 10 MiB maxBuffer and reject-on-error so a failed git
  call can no longer masquerade as a clean tree.
- httpBody (#759): streaming TextDecoder so a multi-byte char split across
  chunks is decoded correctly.
- workSessionRoutes (#759): diff I/O through the containment-validated
  canonical worktree.
- inputDebug (#759): contain diagnostic writes to ~/.openswarm.
- gitInfo (#759): sentinel-prefixed churn parser that cannot read a
  numeric filename as a timestamp nor the next commit's timestamp as a path.
- ciWorker / fixCommand (#765): subprocess timeouts and buffer bounds.
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