Keep server-injected bytes out of stall detection - #64
Merged
not-varram merged 3 commits intoSep 4, 2026
Conversation
not-varram
force-pushed
the
varram/sup-63-injection-banner-counts-as-agent-output
branch
from
September 4, 2026 03:58
9604617 to
0e4ca79
Compare
Stall detection parks a run at needs-attention when the agent goes quiet, and reads the PTY's last-output clock to decide when it is talking again. Two lots of bytes on that stream are the server's, not the agent's: the attributed banner an injection renders, and the terminal's echo of the injected line, which the line discipline sends back even when the agent never reads its input. Steering a hung agent therefore cleared its stall, hiding the hang for another full threshold every time someone poked it. The banner no longer stamps the clock, and the session now discounts the echo an injection is still owed before treating output as the agent's. The first byte that diverges from that echo is the agent talking and counts as it did before, so an agent that answers a steer still returns to running. The stall chaos scenario gains its companion case: an agent that never answers stays parked after a steer. The scheduler's PTY fake stamped the clock on inject too, so its stalled-run test asserted the old contract; both now match the real host.
The expectation rewrote only the trailing carriage return as CRLF, but ICRNL and ONLCR rewrite every lone CR or LF, and ECHOCTL renders other control bytes in hat notation. A multi-line steer - what the dashboard's textarea sends - therefore diverged at its first newline, dropped the expectation and stamped the liveness clock: the original bug, reproduced on the chaos scenario against real Docker with "wake\nup". Build the expectation the way the terminal builds the echo instead. Keystrokes typed on an attach echo the same way and registered nothing, so typing at a hung agent still cleared its stall while the docs said steering was not agent output. They now register their echo like an injected line, which is the contract the docs describe. Registration moves inside stdinMu so concurrent writes queue their echoes in the order they reach the PTY, and a failed write drops the expectation rather than discounting bytes the terminal never saw. The expectation also expires after a second and is capped, so a terminal with echo off - every full-screen agent - cannot leave it standing against the agent's own bytes or grow it for as long as anyone keeps steering. The ptyhost tests become a table over the echo shapes: CRLF, an echo split across reads, a terminal without ONLCR, multi-line, a control byte, TAB, queued writes, divergence, echo and answer in one chunk, expiry and the cap. The chaos scenario's unanswered steer is now multi-line.
DEL was modelled as ^?, but the line editor consumes it as VERASE and repaints with backspace-space-backspace, and a leading DEL echoes nothing at all. ECHOCTL only reaches it with ICANON off, where echo is normally off too. It now takes the divergence path with the other editing characters, which costs a stall one more threshold rather than modelling the echo wrongly. The table's "without ONLCR" case fed bytes no terminal sends, so the branch it named went uncovered. It now feeds what a PTY really sends with each translation turned off - a bare LF, and the CR in hat notation - alongside DEL's repaint, all three as divergence. The server.Config comment and the guide follow the same wording as the rest: agent output, not PTY output, and the discount is best effort - an odd terminal, an oversized steer or a late echo falls through to counting the bytes.
not-varram
force-pushed
the
varram/sup-63-injection-banner-counts-as-agent-output
branch
from
September 4, 2026 04:11
0e4ca79 to
86f939a
Compare
not-varram
deleted the
varram/sup-63-injection-banner-counts-as-agent-output
branch
September 4, 2026 04:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes SUP-63. Stall detection parks a run when nothing has come out of its terminal for a threshold. The server's own injection banner went through the same path, so steering a hung agent flipped it back to running and hid the hang for another full threshold. Reproducing it against real Docker showed a second source of fake output: the terminal echoes the injected line back whether or not the agent reads it.
The PTY session now leaves the banner out of the liveness clock and remembers the echo an injection or attach keystroke is owed, modelled the way the line discipline sends it back (every CR or LF as CRLF, control characters in hat notation, editing characters and other divergences counted as agent output). Only that expected echo is discounted; the first byte that differs ends the expectation, and it expires after a second or above 8 KiB, so a live agent is never made to look stalled. Viewers and the transcript still get every byte.
Docs: failure-handling.md says a steer is not itself agent output, and names the limits; testing.md covers the new scenario.
Validation
TestInjectLiveStalledNeedsAttentionnow proves a run stays parked until the agent answers. Each test fails with the corresponding fix reverted.TestIntegrationChaosStallUXgains a non-responding agent steered with a multi-line message; passes on real Docker and on the in-process runtime.make fmt-check vet lint test test-scripts public-auditandmake test-integrationpass.🤖 Generated with Claude Code
https://claude.ai/code/session_019irxg6aWWKmn2w3QsJR5CJ