Fix: retained-renderer escape carry, and report what a session is actually doing - #74
Merged
Conversation
A user reported one broken link in a translated document. A repo-wide scan found 139. Every translated document lives in docs/i18n/, two directories below the English source it was translated from, and the relative paths were carried over verbatim, so each resolved two levels too shallow: 54 ](docs/X.md) in all 9 README.<lang>.md 81 ](../X) in all 9 getting_started.<lang>.md 4 ](i18n/X.pt-BR.md) stray prefix Same defect class as the License badge repaired earlier, which was fixed as a single symptom rather than generalised. Links now follow the reader: a document that has a translation points at the translated sibling, code/spec/example paths point at the repo root via ../../, and releasing.md keeps pointing at English because it is deliberately untranslated. Independent per-language review then caught three things a path scan cannot see, one of them introduced by this repair: - Retargeting left the visible text reading "docs/sessions.md" while the target became sessions.<lang>.md -- a working link with a lying label, in all 9 files. Text agrees with target again, as in English. - README.ru.md had a CommonMark bug unique to Russian: --- sat directly under a paragraph, making it a setext H2 underline rather than a rule. - Four stale one-language index blocks sent readers to the Portuguese document and asserted the translation itself was authoritative, contradicting line 1 of the same file. Removed. Measured: 139 broken before, 0 broken across 296 links after. All 8 external URLs return 200; all 9 translated READMEs retain their 3 badges. Left open deliberately: 34 of 36 translated files carry no language index, so a reader on a translated page cannot reach a sibling language. That is a convention decision across 36 files, not a broken link. CHG-0075, accepted, documentation kind, no spec change.
…ode is not exempt Three documents described this limit as losing ONE LINE. It stops the session accepting input entirely. The ROADMAP entry is the instructive one. The 0.8.0 claim -- "every later send is silently discarded while settled: true" -- was retracted on the strength of a 20,000-character send to python3 -q, which does receive its input in full and does refuse an overlapping send. Every fact in that retraction reproduces (len() and sha256 both verified). It refuted nothing: python3 -q is in RAW mode when the bytes land, and the original claim was about COOKED mode. A true finding was retracted from the wrong regime. Measured with the child writing byte counts to a file rather than echoing, so the tty echo cannot be mistaken for delivery, plus a post-recovery read proving the input was never merely delayed: - The limit is 1024 bytes per canonical line INCLUDING the terminator rune appends, so a 1023-byte payload arrives and 1024 does not (20/20 deterministic). At exactly 1024 the payload is accepted whole and the TERMINATOR is the rejected byte, so the line can never be submitted. - Every later send is discarded, indefinitely (measured to +120s), while each reply says status ok, settled true, state running, exit_code null. After ^U recovery the intervening sends are permanently absent. - It is not rune: one 1025-byte write() to a bare PTY.spawn wedges identically, so the split payload/terminator write is not the cause. - The discriminator is the tty ICANON state when the bytes land, proven by holding the child binary constant and flipping one termios flag. Not the child's identity, not whether it drains. - "Raw-mode children are unaffected" is FALSE, and this is what matters for rune's targets: bash and python3 are raw at their prompt and cooked while a foreground command runs. 1200 bytes to a busy bash loses the line and silently corrupts the next command; to a busy python3 the follow-up statement never executes. - "Over-long line" is wrong at the caller's level: four 256-byte writes with no terminator wedge the queue too. - BEL is not a usable detector -- absent when an unterminated 1024-byte send wedges, present on a child that legitimately rings the bell, and suppressed entirely when IMAXBEL is cleared. - Complete lines are never silently lost; they get backpressure and a loud refusal. Only unterminated input disappears. Also reopened and re-verified CHG-0073 and CHG-0074, whose accepted evidence lists ROADMAP.md and went stale when this change edited it. CHG-0076, accepted, session spec 37 -> 38.
…s a one-shot render ROADMAP names a retained per-session Screen as the thing 1.0 hangs off, and names one obstacle: the parser is stateless between calls. The grid was always retained across render calls; the StringScanner was rebuilt each time, so an escape split across a chunk boundary failed to match CSI, fell through to PRINTABLE, and was written onto the grid as literal text. Measured on a 425-byte stream of ordinary TUI output: chunk_size retained oneshot identical? 1 384 209 NO 7 284 209 NO 64 218 209 NO 512 209 209 yes At one byte per chunk the screen was 84% longer than the truth because every escape byte had been printed: oneshot : "row 0 some text here" retained: "[2J[H[1;1H[1;31mrow 0 some text here[0m[K[2;1H..." An unterminated sequence is now held for the next chunk, which is what a real terminal does. The renderer already detected this case (INCOMPLETE) and discarded the bytes -- right for a one-shot render of a truncated transcript, wrong for a retained one. Bounded by MAX_CARRY_BYTES so an unclosed OSC cannot buffer without limit; past the ceiling the carry is dropped, which is what a one-shot render already did. Testing every split point rather than a few tidy ones surfaced a SECOND, PRE-EXISTING defect this fix did not cause: INCOMPLETE matched an OSC body as \][^\a\e]*, which stops at \e. ST is a two-byte terminator, so a buffer ending between them left a body that pattern could not cover -- the sequence read as complete-but-unrecognised and its body was printed. A one-shot `read --screen` of a transcript ending mid-ST has always rendered ]0;title as visible text. Validated across 2571 split configurations over ten escape families -- every single-point split, byte-at-a-time, and 200 random multi-point splits per stream -- 0 divergent. Both arms verified against deliberately reverted code: reverting either produces failures. Measured payoff over 20 frames of a 40x120 TUI repaint: growing-prefix one-shot : 1033.9 ms/tick retained : 99.9 ms/tick 10.4x Explicitly not claimed: that this makes per-tick screen matching affordable. 100ms/tick is still above POLL_INTERVAL and Screen#heal runs per glyph. This makes the retained path correct; the cost question stays open. CHG-0077, accepted, parsers spec 15 -> 16.
Two defects, the same mistake wearing different faces: rune reported a state it had not established. 1. A real 127 was reported as "not on PATH". launch_failure tested exit_code == 127, reasoning that 127 is the shell's "command not found" and therefore the one case where the child never ran. True of a shell, false of a child -- 127 is an ordinary status any program may choose. Measured with the child appending to a file BEFORE exiting 127, so its own file rather than the reply proves execution: misreported as "not on PATH": 7/12 reported ok: 5/12 children that actually executed: 12 Racy, turning on whether the child died before the supervisor recorded it as running, so the same script passed or failed by timing alone -- and the failure told the user to check whether a program they had just run was installed. An exit status cannot carry the distinction, so the supervisor records it: reaching the ENOENT/EACCES rescue around PTY.spawn IS the fact that exec failed, because that raise happens before the child exists. launch_failed is absent rather than false otherwise, so older meta reads as a child that ran. This also made a non-executable target a loud failure; it previously returned status "ok" and failed only on the next send. 2. Thirty-six hours of silence rendered as routine. A session sat on an agent's approval prompt -- "Run this command? 1. Approve once..." -- for 36.2 hours while state said running, the same word a healthy child gets. rune was not blind to it: idle_ms was 130,293,976 and child_busy was false. The rendering failed. idle_suffix printed minutes at every scale, so the line read `idle 2172m` in the same dim grey as every healthy session. Minutes stop being a unit anyone converts around an hour, and grey is the colour of "ignore me". Hours and days past 90 minutes, and no dimming for a running session past STALE_IDLE_SECONDS. A stopped session stays dim -- its idle time is only how long ago it stopped. kimi-rev running idle 36.3h <- yellow grok-rev running idle 21s <- grey, genuinely working Deliberately NOT a stuck-detector. rune cannot tell a child waiting on a human from one thinking hard, and four heuristics of that shape have been measured and rejected on the adjacent settle problem. The threshold decides a colour and claims nothing. It does nothing for the harder half either: a child repainting a spinner while blocked holds idle_ms near zero and stays indistinguishable from one that is working. Every new test verified against deliberately reverted code, with the revert confirmed to have applied -- an earlier regex revert silently matched nothing and "passed", proving the opposite of what it appeared to. CHG-0078, accepted, session spec 39 -> 40.
Merge rather than rebase: this branch carries accepted spec-sync verification commits, and rebasing onto the squash-merged main would mint new SHAs and orphan the recorded evidence. # Conflicts: # .specsync/change-sequence.json
Merge rather than rebase: both branches carry accepted spec-sync verification commits, and rebasing would mint new SHAs and orphan the recorded evidence.
Merge rather than rebase: this branch carries accepted spec-sync verification commits for CHG-0077 and CHG-0078, and rebasing onto the squash-merged main would mint new SHAs and orphan the recorded evidence. # Conflicts: # .specsync/change-sequence.json # .specsync/changes/CHG-0073-record-that-wait-for-regex-can-match-a-prior-turn-redraw-and-bring-the-1-0-ro/approvals.json # .specsync/changes/CHG-0073-record-that-wait-for-regex-can-match-a-prior-turn-redraw-and-bring-the-1-0-ro/state.json # .specsync/changes/CHG-0073-record-that-wait-for-regex-can-match-a-prior-turn-redraw-and-bring-the-1-0-ro/verification-attempts.json # .specsync/changes/CHG-0073-record-that-wait-for-regex-can-match-a-prior-turn-redraw-and-bring-the-1-0-ro/verification.json # .specsync/changes/CHG-0074-correct-the-wait-for-regex-reprint-advice-qualify-reprint-not-visible-histor/approvals.json # .specsync/changes/CHG-0074-correct-the-wait-for-regex-reprint-advice-qualify-reprint-not-visible-histor/state.json # .specsync/changes/CHG-0074-correct-the-wait-for-regex-reprint-advice-qualify-reprint-not-visible-histor/verification-attempts.json # .specsync/changes/CHG-0074-correct-the-wait-for-regex-reprint-advice-qualify-reprint-not-visible-histor/verification.json # .specsync/changes/CHG-0076-correct-the-max-canon-claims-an-unterminated-1024-byte-line-wedges-the-session/approvals.json # .specsync/changes/CHG-0076-correct-the-max-canon-claims-an-unterminated-1024-byte-line-wedges-the-session/state.json # .specsync/changes/CHG-0076-correct-the-max-canon-claims-an-unterminated-1024-byte-line-wedges-the-session/verification-attempts.json # .specsync/changes/CHG-0076-correct-the-max-canon-claims-an-unterminated-1024-byte-line-wedges-the-session/verification.json # specs/session/session.spec.md
…icate changelog rows Merging main (which now carries CHG-0076 via #73) conflicted on the session spec in two places, and resolving it surfaced a defect that had already shipped. - version: main is 39; this branch adds exactly one change, so 40. It read 41 because the version was bumped by hand AND by `specsync change accept` applying the delta. - Change Log: CHG-0076 appeared twice on main and CHG-0078 twice here, from the same cause -- `accept` appends the row itself, so the manual `cat >>` that preceded it duplicated every one. This is the same defect CHG-0074 was raised to fix once already, reintroduced by hand. The four accepted changes that list this spec were reopened with a recorded reason, re-verified against the full gate, and re-accepted.
Five reviewers were asked whether rune should cut 1.0 -- two agents driven
through rune session, three subagents attacking specific surfaces. Both
agents answered do not ship. This is the code half of what they found. Two
of the five are corrections to work accepted earlier the same day.
1. A control-socket request could kill a child.
JSON.parse accepts any JSON value, not only an object, so null / 123 /
true / "x" / [1,2,3] all parsed and then raised on request[:op] past every
rescue -- unwinding into the supervisor's crash path and SIGKILLing a
healthy child. Measured with a heartbeat child, its death read from its own
file: 5/5 killed both, while {"op":123} and unparseable text were answered
correctly. Invariant 28 says a control client can never take the session
down with it; this was the hole in it. Reachable only from the non-Ruby
socket client the protocol exists to serve.
2. read reported a dead session as running, indefinitely.
liveness returned the recorded state; describe recomputes it and carries a
comment saying a recorded "running" must never be reported as-is. With the
supervisor SIGKILLed: list said dead, read said running with a real cursor
and status ok, and send said "is not running (state running, exit code
nil)" -- contradicting itself and printing Ruby's nil at a user. The guide
recommends polling read for a readiness marker, so that loop waited on a
session already gone.
3. The idle rendering shipped that morning had a boundary bug.
idle_suffix thresholded on raw seconds while humanize_idle rounded, so
`idle 15m` rendered dim at 870s and yellow at 900s -- identical label,
different colour, nothing to explain it. Flooring makes label and colour
change together, and fixes an hour printing 60m and 90s printing 2m.
4. The carry ceiling was set to exactly the producer's read size.
MAX_CARRY_BYTES was 4096, exactly READ_CHUNK, so nothing spanning more than
one pty read could ever be carried -- an 8205-byte sequence survived 0 of
14 offsets. And the invariant published with it claimed that past the
ceiling bytes "are dropped, which is what a one-shot render already did".
False both ways: a terminated 12KB OSC renders 11 bytes one-shot and 1935
bytes of base64 retained. Raised to 64KB; the invariant now states the
residual divergence instead of denying it.
5. Two parsers in one module disagreed about what an escape is.
ScreenRenderer::CSI was widened to the full ECMA-48 grammar after a capture
of one agent contained 80 sequences it printed instead of obeying.
TextSanitizer kept the narrow pattern, so colon-form truecolour SGR and
DECSCUSR still survived into clean_output, --grep and last_line while
--screen rendered them correctly.
Every new test verified against deliberately reverted code, with each
revert confirmed to have applied. One harness error of my own caught and
fixed: the straddle test used Array#one?, which counts truthy elements, so
[false].one? is false and every all-dim label read as a straddle.
CHG-0079, accepted. session spec 40 -> 41, parsers 17 -> 18.
…hange it, give failures a code
After CHG-0079 fixed the code the reviewers found, what remained was three
places where rune's documents said something other than what rune does, and
one contract decision that is free now and impossible after a freeze.
1. The guide taught a start contract that no longer exists.
It said a missing command still returns status ok with exit 0 and
exit_code 127, and to "check state, not the process exit status". Measured
now, three trials each:
missing / not executable / a directory -> error, exit 1, no data
exists, exits 3 at once -> ok, exit 0, state "running"
true -> ok, exit 0, state "exited"
The warning was right; its example and its remedy were both wrong. A
command that cannot be executed is a hard failure. One that starts and dies
is still a success, because it is one. And state in a START reply is a
snapshot that can already be false -- the script exiting 3 reported running
3 of 3. list recomputes; the next send tells you too.
2. The roadmap listed solved and unsolved items as one blocker set.
A reader could not tell which were gates and which would ship documented,
and that ambiguity is what kept the list growing: every unsolved item
looked like a blocker whether or not anyone meant to solve it. Now split by
what would actually change the answer -- one gate left, three technical
items moved to Known and documented, each saying why it moved. Nothing
deleted: the record of what was tried is what stops it being tried again.
3. Failures could only be distinguished by reading English.
Every failure was {status, error}. The same missing-session condition
produces two entirely different sentences depending on whether the session
exists in another project, one carrying remediation advice wrong for the
other case -- and at 1.0 those sentences become a frozen API. Failures now
carry data.code and data.name. Additive: Result#to_h already emitted data,
and the human renderer reads only error, so nothing visible moves. The code
set is open and callers are told to treat an unknown code as generic.
Left open deliberately, and recorded rather than deferred silently: the
translated copies of the start paragraph, which need per-language work
rather than a mechanical edit; `list --archived` returning a name no other
verb accepts; and --home/--project being parsed everywhere and inert.
CHG-0080, accepted, session spec 41 -> 42.
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.
Two changes, both bugs where rune reported something it had not established.
CHG-0077 — the retained renderer printed escapes onto the grid
ROADMAP.mdnames a retained per-sessionScreenas the thing 1.0 hangs off, and names one obstacle: the parser is stateless between calls. The grid was always retained acrossrendercalls; theStringScannerwas rebuilt each time, so an escape split across a chunk boundary failed to match CSI, fell through toPRINTABLE, and was written onto the grid as literal text.Measured on a 425-byte stream of ordinary TUI output:
At one byte per chunk the screen was 84% longer than the truth:
An unterminated sequence is now held for the next chunk, as a real terminal holds it in its parser. The renderer already detected this (
INCOMPLETE) and discarded the bytes — right for a one-shot render of a truncated transcript, wrong for a retained one. Bounded byMAX_CARRY_BYTES, past which the carry is dropped, which is what one-shot already did.A second, pre-existing defect surfaced from testing every split point rather than a few tidy ones.
INCOMPLETEmatched an OSC body as\][^\a\e]*, which stops at\e. ST is a two-byte terminator, so a buffer ending between them left a body that pattern could not cover — the sequence read as complete-but-unrecognised and its body was printed. A one-shotread --screenof a transcript ending mid-ST has always rendered]0;titleas visible text.2571 split configurations across ten escape families, 0 divergent. Measured payoff: 10.4× (1033.9 → 99.9 ms/tick over 20 frames of a 40×120 repaint).
Explicitly not claimed: that this makes per-tick screen matching affordable. 100ms/tick is still above
POLL_INTERVALandScreen#healruns per glyph. This makes the retained path correct; the cost question stays open.CHG-0078 — a real 127, and 36 hours of silence
launch_failuretestedexit_code == 127, reasoning that 127 is the shell's "command not found" and so the one case where the child never ran. True of a shell, false of a child — 127 is an ordinary status any program may choose. Measured with the child appending to a file before exiting 127, so its own file proves execution:Racy, turning on whether the child died before the supervisor recorded it as running — so the same script passed or failed by timing alone, and the failure told the user to check whether a program they had just run was installed.
An exit status cannot carry the distinction, so the supervisor records it: reaching the
ENOENT/EACCESrescue aroundPTY.spawnis the fact that exec failed, since that raise happens before the child exists.launch_failedis absent rather thanfalseotherwise, so older meta reads as a child that ran.status: "ok"Second: a session sat on an agent's approval prompt — "Run this command? 1. Approve once…" — for 36.2 hours while
statesaidrunning, the same word a healthy child gets. rune was not blind to it:idle_mswas 130,293,976 andchild_busywasfalse. The rendering failed.idle_suffixprinted minutes at every scale, so it readidle 2172m, in the same dim grey as every healthy line.Deliberately not a stuck-detector. rune cannot tell a child waiting on a human from one thinking hard, and four heuristics of that shape have been measured and rejected on the adjacent settle problem. The threshold decides a colour and claims nothing. It also does nothing for the harder half: a child repainting a spinner while blocked holds
idle_msnear zero and stays indistinguishable from working.Test Plan
specsync check: 2 specs passed, 32/32 files, 7529/7529 LOCOne pre-existing flake observed once in 14 runs (
session_spec.rb:2580, an absurd-resize test that waits on a repaint with a 15s deadline). Recorded rather than papered over with a bigger timeout.CHG-0077 (parsers 15 → 16) and CHG-0078 (session 39 → 40), both accepted.
🤖 Generated with Claude Code
https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY