Skip to content

fix(host): flush queued output before closing clients on shutdown - #49

Open
aksOps wants to merge 1 commit into
mainfrom
fix/attach-shutdown-drain
Open

fix(host): flush queued output before closing clients on shutdown#49
aksOps wants to merge 1 commit into
mainfrom
fix/attach-shutdown-drain

Conversation

@aksOps

@aksOps aksOps commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

First of the three findings #48 verified but deferred.

The defect

pumpPTY runs to EOF on the host's main goroutine, so by the time shutdown runs, everything the agent ever wrote is already queued on each client. shutdown then called client.drop(), which closes the connection immediately — the writer never got to put the queue on the wire.

Two symptoms, both on the viewer:

  • the agent's last screen is truncated (you miss whatever it printed on the way out);
  • a connection closed mid-frame yields io.ErrUnexpectedEOF, which is not io.EOF, so copyAttachOutputConfigured returns it and the attach client exits non-zero with attach output: unexpected EOF instead of printing [uam: session ended].

The fix

attachClient gains a second signal. done still means stop now; flush means finish, then stop. attachWriter selects on both — on flush it drains whatever is queued, then closes the connection itself.

shutdown asks every client to flush and waits up to shutdownFlushWindow (250ms) for each to close. The same window is the socket write deadline, so a viewer that stopped reading cannot hold teardown open, and a client registered without a running writer is force-closed when the window expires.

shutdown is split so the client half (shutdownClients) is testable without the record/file side effects.

Verification

  • go test ./... — 1160 pass, 8 skip. The one failure, providerstate.TestResolvedAncestryWarnsForWritableComponent, reproduces unchanged on main (environment-dependent; it's the next item on the list).
  • go test -race ./internal/session/... — clean.
  • go vet, gofmt, golangci-lint — clean.
  • New tests: writer drains its queue on the flush signal; writer discards on done (the contrast that makes the distinction meaningful); shutdown bounded against a stalled viewer; shutdown closes a client that has no writer. Removing the flush arm from attachWriter makes the first test fail — checked.
  • End to end against the built binary over a real PTY: an agent printing 60 lines and exiting while a client is
    attached delivers every line, prints [uam: session ended], no unexpected EOF, attach exits 0. Note this case
    passes against a pre-fix binary too — on a fast local socket the writer keeps up, so it is a smoke test of the
    clean-exit path, not a reproduction. The discriminating evidence is the unit test above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X3PrstXFvYdemQrz8Vb1b1

pumpPTY has already returned by the time shutdown runs, so everything the
agent ever wrote is sitting in each client's queue. Closing the connection
there truncated the agent's last screen, and cutting a frame in half reached
the viewer as "attach output: unexpected EOF" with a non-zero exit instead of
a clean "[uam: session ended]".

Clients now carry a second signal: done still means stop now, flush means
finish first. Shutdown asks every client to flush, then waits up to
shutdownFlushWindow for each writer to drain its queue and close the
connection itself. A viewer that has stopped reading cannot hold teardown
open — the same window bounds both the socket write deadline and the wait —
and a client with no writer running is force-closed when the window expires.

Verified end to end against the built binary: an agent that prints 60 lines
and exits while a client is attached now delivers every line, prints the
session-ended note, and the attach client exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01X3PrstXFvYdemQrz8Vb1b1
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