Fix: a failed launch was reported as success, and three defects non-ASCII found - #68
Merged
Conversation
Three defects that surfaced during the last triage but were out of that PR's scope. Each verified before being planned. watch had no unknown-flag guard: anything flag-shaped it did not recognise stayed in the argv and became the command. My first probe of this was worthless — a non-tty run refuses on "stdin is not a TTY" before parsing anything, so the refusal was the TTY check and proved nothing. Driven through a real PTY.spawn, `rune watch --timeout 5 -- echo hi` exited 127 with the child never running. run has guarded this since it grew flags; watch never did, which made it the worse of the two, because run at least says something. The guard now lives in Command.flag_error and is shared rather than copied: the two had already drifted once, with run growing the inline-value branch and watch having no guard to grow it in. --grep ignored --since. `filter` was handed the sliced text and then called transcript.grep, which searched the whole transcript, so a read from a cursor recorded after the first line still returned that line and grep_matches counted it. A caller paging with --since=<last cursor> got the whole history on every page under a count that looked like it had filtered. --max-output did not bound clean_stdout/clean_stderr at all: a 200-byte budget returned 10,506 bytes across four fields. Now 1,012. An adversarial pass found two merge-blockers in this work and was right about both. First, the watch guard shipped with NO test — reverting all three guard files left the suite fully green, so CI could not tell the fix from its absence, including its own headline case. Six tests now cover it, including a drift guard and the case it must not lose, a child's own flags surviving. Second, moving unknown_flag_error and INLINE_VALUE_ERROR out of RunCommand left the pty_runner spec documenting them and added six exports nothing documented; specsync reported two hard errors, both now gone. It also caught a false claim in my own comment: watch's VALUE_FLAGS said it "cannot drift from the parser" while --log is appended by hand, because its pattern is inline in scan_head. Corrected to what is true. One reported loss was checked and deliberately left: the guard covers the leading position only, so `rune watch echo hi --log=/tmp/x` still writes rune's own log to the child's path. Measured byte-identical before and after, so it is pre-existing and recorded in invariant 20 rather than fixed here. Note an API change: RunCommand::INLINE_VALUE_ERROR still resolves through inheritance but now carries a second placeholder, so format(..., name:) alone raises KeyError. It has never appeared in a tagged release. 584 examples, 0 failures. Controls: watch guard reverted fails 4 of 33, grep reverted fails 1 of 2, stream bounds reverted fails 1 of 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
…wn in The last of the five renderer gaps, and a correction of my own conclusion about it. A cell model was built once before and reverted, and the spec recorded that it had been "measured worse than the gap". That was wrong. The A/B compared two working trees and misattributed which output came from which side. Re-measured against three explicit revisions on the same 56,928-byte grok capture that had emitted a CJK table: cc8bb3c (one column) "東h京 Tokyo" "大 阪 Osaka" ad76e22 (one column) "東h京 Tokyo" "大 阪 Osaka" cell model "東京 Tokyo" "大阪 Osaka" The one-column model corrupts real agent output and always did: an agent positions its columns assuming two per CJK glyph, and a renderer counting one puts every later write in the wrong place. I had it backwards, reverted a correct fix, and wrote the mistake into the contract. I also ruled out the four renderer fixes shipped since — alt screen, DECAWM, IRM, charsets — by disabling each in turn on top of main. The corrupted rows are byte-identical in all four cases. It is the column arithmetic. Rows are now Arrays of cells: nil, a String of one graphic plus its combining marks, or CONTINUATION for the right half of a wide glyph. That makes two things true the String could not. A cell holds any number of characters without moving the cells after it, which is what makes combining marks work at all — appending a mark to a String row put every later index off by one and the next graphic overwrote it. And the pair invariant can be restored after the fact, in one heal pass, instead of being taught to twelve operations that each slice the row. The first attempt tried the latter and lost. pad/padded_line are gone: assigning past the end of an array fills with nil and a nil cell renders blank. That padding was the mechanism by which a column index became an index into text. 591 examples, 0 failures. Controls: forcing the width table to one column fails 4 of 136 parser examples; making heal a no-op fails 2. Stacked on leif/watch-flag-guard so the change records do not collide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
…SCII found Four defects from two sources, each verified before being planned. A launch that never happened reported success. `start` with a command not on PATH returned status "ok", state "exited", exit_code 127 — so a caller checking the field whose entire job is to say whether the call worked saw success. It was documented as "check state instead", which is the wrong shape of answer: an envelope should not need a footnote to be read correctly. Reported from a 22-minute real drive where it cost an hour. Only 127 fails, deliberately. `start -- true` exits 0 immediately and is a successful launch of a program with nothing to do; treating any prompt exit as failure would break every short-lived child. The session record is kept rather than deleted — start failing loudly is the fix, and removing the transcript that shows why would trade one quiet failure for another. An error that was confidently wrong. A session started in one directory and read from another got "No such session", and that error told the reader to run `rune session list`, which is scoped to their own project and returns nothing, actively confirming the wrong conclusion. rune knew where it was the whole time. The message now names the project. This has caught three readers, two of whom had read the guide's warning first, which is when a documented gotcha stops being a documentation problem. Two defects the native-language translation round found, by conducting each rune session entirely in the language being translated rather than sending English: CharacterWidth::ZERO covered Latin, Greek, Cyrillic, Hebrew, Arabic and Thai and omitted every Indic script, so हिन्दी was charged six columns for six codepoints. My own table, written hours earlier. Fixed as the Mn/Me subset, not every Indic mark: U+093F is a spacing mark and takes a column in wcwidth and xterm, so zeroing all of them would be wrong in the other direction. हिन्दी is now five columns, matching xterm rather than the three a shaping engine draws. ScreenRenderer.resync searched with String#index (characters) and sliced with byteslice (bytes). On "日本語テキスト\e[1mAFTER" the ESC is at character 7 and byte 21; it cut at 7, returning "\xAA\x9Eテキスト\e[1mAFTER" — both splitting a character and failing to drop the remainder it exists to drop. One claim from the same report did NOT reproduce and is not fixed: that a send over ~1024 bytes jams the session while rune answers settled:true. Measured at 600/1100/4096/20000 bytes — no jam below 20k, and at 20k the follow-up is refused with status:error and recovers by itself in ~10s, which is what ROADMAP already records. The reported shape looks like a probe reading clean_output without checking status. Also lands the six in-language guide translations and their report. 604 examples, 0 failures. Controls: launch_failure reverted fails 1 of 3, resync byte index 2, Indic ranges 2, project lookup 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
0xLeif
force-pushed
the
leif/screen-cell-model
branch
2 times, most recently
from
August 18, 2026 18:23
ba810bb to
8893c35
Compare
# Conflicts: # .specsync/change-sequence.json # .specsync/changes/CHG-0069-guard-the-flags-watch-was-executing-and-bound-the-two-fields-max-output-was-not/approvals.json # .specsync/changes/CHG-0069-guard-the-flags-watch-was-executing-and-bound-the-two-fields-max-output-was-not/state.json # .specsync/changes/CHG-0069-guard-the-flags-watch-was-executing-and-bound-the-two-fields-max-output-was-not/verification-attempts.json # .specsync/changes/CHG-0069-guard-the-flags-watch-was-executing-and-bound-the-two-fields-max-output-was-not/verification.json # .specsync/changes/CHG-0070-give-the-screen-a-cell-model-so-a-wide-glyph-occupies-the-two-columns-it-is-draw/approvals.json # .specsync/changes/CHG-0070-give-the-screen-a-cell-model-so-a-wide-glyph-occupies-the-two-columns-it-is-draw/state.json # .specsync/changes/CHG-0070-give-the-screen-a-cell-model-so-a-wide-glyph-occupies-the-two-columns-it-is-draw/verification-attempts.json # .specsync/changes/CHG-0070-give-the-screen-a-cell-model-so-a-wide-glyph-occupies-the-two-columns-it-is-draw/verification.json # lib/rune/parsers/character_width.rb # lib/rune/parsers/screen.rb # spec/rune/parsers/screen_renderer_spec.rb # specs/parsers/parsers.spec.md # specs/session/session.spec.md
Carries three things: the merge needed to keep this stacked PR's specsync evidence reachable after upstream squash-merges, a re-anchor I forgot to commit in the previous push, and a real race CI found on the resulting tree. The merge itself needed real conflict resolution, not just a repeat of the mechanical squash-SHA trap. screen.rb, its spec and both spec docs had genuinely diverged: my branch predates the CodeQL regex fix (#66), so origin's side won on to_s and the resync/Indic tests were additive on both sides. One correction caught before it landed: I initially took origin's copy of character_width.rb wholesale, which would have silently dropped this branch's own Indic-marks fix — caught by grep and the full suite before committing, not after. CI then found what local runs did not. Ruby 3.4's job failed a test this PR added: `abandon` sends SIGKILL and immediately writes state: 'failed', but SIGKILL is asynchronous, so `list` right after could still see the supervisor as alive and report 'running' — describe deliberately recomputes state from real process liveness rather than trusting the record, precisely so a supervisor killed without its cooperation is never reported as-is, which is exactly the case a not-yet-dead abandon target is. stop hit this identical shape once already, and the fix is already written down at its own await_death call: "SIGKILL is asynchronous... the very next command saw the session as running." abandon had the same fire-and-kill shape without the wait. It now calls await_death before recording the failure, matching stop exactly. Passed 3/3 full local runs before this was understood as a real race rather than the flake noted in #68's own PR body — worth correcting: that was this bug, present since abandon was written, and CI's slower runner is what surfaced it. 607 examples, 0 failures. specsync 32/32 files, 7431/7431 LOC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY
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
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.
Four defects from two sources — a real 22-minute drive and a translation round where every rune session was conducted in the language being translated rather than in English.
A launch that never happened reported success
A caller checking
status— the field whose entire job is to say whether the call worked — saw success. It was documented as "checkstateinstead", which is the wrong shape of answer: an envelope shouldn't need a footnote to be read correctly.Only 127 fails, deliberately.
start -- trueexits 0 immediately and is a successful launch of a program with nothing to do; treating any prompt exit as failure would break every short-lived child. And the session record is kept —startfailing loudly is the fix, but deleting the transcript that shows why would trade one quiet failure for another.An error that was confidently wrong
A session started in one directory and read from another got
No such session, and that error told you to runrune session list— which is scoped to your project and returns nothing, actively confirming the wrong conclusion. rune knew where it was the whole time.This has now caught three readers, two of whom had read the guide's warning about directory scoping first. That's the point at which a documented gotcha stops being a documentation problem.
Two defects that only non-ASCII input could find
The previous translation round sent English prompts, so it only ever exercised the output path. Conducting the session in-language exercised the input path, and found:
CharacterWidth::ZEROomitted every Indic script — Latin, Greek, Cyrillic, Hebrew, Arabic and Thai were covered; Devanagari, Bengali, Tamil and the rest were not.हिन्दीwas charged six columns for six codepoints. This is my own table, written hours earlier in Add: a cell model, so a wide glyph occupies the two columns it is drawn in #66.resyncfed a character index tobyteslice. On日本語テキスト\e[1mAFTERthe ESC is at character 7 and byte 21; it cut at 7, returning"\xAA\x9Eテキスト\e[1mAFTER"— both splitting a character and failing to drop the remainder it exists to drop.I narrowed the first one against its report. The finding said "one column per codepoint", which overstates it:
U+093Fis a spacing mark and legitimately takes a column inwcwidthand xterm, so zeroing every Indic mark would be as wrong in the other direction. The fix is the Mn/Me subset —हिन्दीis now five columns, matching xterm, not the three a shaping engine draws. This followswcwidthand doesn't try to settle shaping.One claim that did not reproduce, and is not fixed
The same report said a send over ~1024 bytes jams every later send while rune answers
settled: true, state: running. Measured:That's what
ROADMAP.mdalready records. The reported shape looks like a probe readingclean_outputwithout checkingstatus— the same mistake this session made once already.Also lands
The six in-language guide translations (
docs/i18n/getting_started.*) and the round's report.Test Plan
specsync check32/32 files, 7414/7414 LOClaunch_failurereverted fails 1 of 3; resync byte index 2; Indic ranges 2; project lookup 2🤖 Generated with Claude Code
https://claude.ai/code/session_018rf59AtQmJcodUJ6vXDZNY