fix(bin): classify composer rows by bytes, not locale character classes - #1976
Open
nhwoodward wants to merge 7 commits into
Open
fix(bin): classify composer rows by bytes, not locale character classes#1976nhwoodward wants to merge 7 commits into
nhwoodward wants to merge 7 commits into
Conversation
Author
|
@kunchenguid when you get a chance, this fork PR is waiting on workflow approval before CI can run. It's a small locale-independence fix for the shared composer classifier (idle Claude composer rows misread as pending under C/POSIX and glibc UTF-8 locales), with tests and the verification record updated. Thanks! |
An idle Claude Code composer row is `❯` (U+276F) followed by U+00A0
NO-BREAK SPACE, not an ASCII space. Under C/POSIX two shell primitives
in the shared classifier turned that row into a false `pending`:
- `[[:space:]]` does not cover U+00A0, so the trailing NBSP survived
trimming and the row never looked empty.
- `${content#?}` strips one BYTE rather than one character, so shearing
the leading glyph left undecodable debris that read as typed text.
fm_composer_classify_content is the single fleet-wide owner of the
empty/pending/unknown injection-safety verdict, so the false `pending`
silently deferred away-mode injections and failed steer verification on
any home running under C/POSIX - a cron job, a minimal shell, a remote
host - as a quiet supervision degradation rather than a crash.
Classification now depends only on literal byte sequences: NBSP is
folded to a plain space on entry and re-trimmed, and each multibyte
agent glyph is stripped as its own literal instead of through `?`. The
fold matches the exact two-byte sequence rather than a bracket class,
for the same reason FM_BACKEND_HERDR_BARE_PROMPT_RE stays an
alternation. The verdict vocabulary and every previously tested
behavior are unchanged.
The regression asserts the same verdicts under both C/POSIX and a
probed UTF-8 locale, and refuses to pass when no UTF-8 locale is
installed rather than comparing C against nothing. It covers both
load-bearing halves: the idle row must read `empty` so injections are
not deferred forever, and real typed text after the same NBSP must stay
`pending` so nothing is typed over unsubmitted input. The existing
idle-placeholder case was already failing under LC_ALL=C before this
change, so the suite now runs under both locales.
The portable serial shard 4 failure "the serving child ignored TERM" is a real, racy defect in the remote job worker, not test noise. When a teardown removes a worker's state root, the serving loop's stale-record reap reaches fm_remote_job_prepare_state, which recreates the state root on the very next pass. The heartbeat then keeps succeeding, so the child never notices anything is wrong - but nothing recreates the worker.lock directory inside it. From that half-restored state the child owns nothing (a second worker can take the free lock and serve beside it) and worker_publish_quarantine can never succeed again, so worker_shutdown re-arms its trap and returns on every signal. The child becomes unstoppable by TERM, which is precisely the leak this worker exists to prevent. Whether CI hit it depended on where the removal landed inside a 0.05s serving pass: before the reap the child self-terminated on a failed heartbeat and the test passed, after it the child ran on forever. Treat losing the lock as fatal to the serving child, exactly like a failed heartbeat: it exits non-zero and the supervisor starts a replacement that acquires ownership cleanly. The check runs before the heartbeat so an unowned worker never publishes another round of readiness. Pinned by a new case that removes only worker.lock, with the state root left intact, so the regression is deterministic rather than a race. That case fails on every run without this fix. The existing teardown case is now deterministic too, and the script drops from ~9.5s to ~3.6s.
nhwoodward
force-pushed
the
fm/composer-locale-nbsp
branch
from
August 9, 2026 01:01
223b7b8 to
ea7d594
Compare
4 tasks
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.
Intent
The developer's goal was to fix a fleet-wide locale bug in firstmate's shared composer classifier, where an idle Claude Code composer row (U+276F followed by U+00A0) classified as pending instead of empty under the C/POSIX locale, because bash's [[:space:]] does not match NBSP and ${content#?} strips one byte rather than one character. They required the bug be reproduced first end-to-end with a real tmux capture-pane of an idle claude under LANG=C as baseline evidence, then fixed in bin/fm-composer-lib.sh so character operations are byte-safe, covering the coherent class of sibling ghost/whitespace forms the lib already models without over-engineering, and keeping the verdict vocabulary and every existing tested behavior unchanged. They asked for regression tests colocated with the existing tests/fm-composer-*.test.sh files proving the idle row reads empty under both LANG=C and en_US.UTF-8, shellcheck-clean code, a full repo suite run with pre-existing failures explicitly baselined, and adherence to the firstmate coding guidelines and CONTRIBUTING.md since this ships upstream. Scope was confined to the classifier fix plus its tests and docs with no drive-by refactors, delivered from the nhwoodward/firstmate fork as a PR against upstream referencing PR #1948, and the worker was told to recover a prior interrupted worker's uncommitted patch read-only rather than redo the work. When the validation gate surfaced two ask-user findings about overstated claims, the developer decided to fix only the first: narrow the header comment so it claims just the U+00A0 fold rather than full locale independence, explicitly leaving bin/fm-tmux-lib.sh and the second finding alone as out of scope for a future change, then respond at the gate and drive the pipeline to completion.
What Changed
fm_composer_classify_contentinbin/fm-composer-lib.shnow folds U+00A0 NO-BREAK SPACE to a plain space through an explicit two-byte match (FM_COMPOSER_NBSP) and re-trims bothcontentandplain_content, and strips each multibyte agent glyph (❯,›,⟩) as its own literal byte sequence instead of${content#?}/${content#??}. Previously the trailing NBSP survived[[:space:]]trimming and?sheared a single byte off a 3-byte glyph, so an idle Claude composer row (❯+ U+00A0) classified aspendinginstead ofemptyand silently deferred away-mode injections and failed steer verification. The verdict vocabulary and every previously tested verdict are unchanged.tests/fm-composer-lib.test.shgains six locale cases that re-exec bash under bothCand a probed UTF-8 locale (rather than a hardcodeden_US.UTF-8, and failing loudly when no UTF-8 locale is installed): the idle row through the classifier and through the realfm_tmux_composer_row_statecapture path, all three agent glyphs, typed text after the NBSP stayingpending, an idle placeholder after a multibyte glyph, and a bare shell prompt stayingunknown.docs/verification/runtime-backends.mdgains a "Composer classification locale independence" record holding thecapture-panehexdump of the real idle row, the probed per-platform[[:space:]]matrix for the other fixed-width Unicode spaces, and a two-locale re-verification loop for the composer and tmux-adapter suites that are not self-parameterized. The header comment and doc scope the guarantee to the U+00A0 fold and the glyph strip specifically, not to general locale independence;bin/fm-tmux-lib.shgeometry handling of NBSP is explicitly left out of scope and recorded as a deferred gap. The one Test-phase finding (tests/fm-backend-orca.test.shmetadata case) is pre-existing and reproduces identically on base833a9a2.Risk Assessment
✅ Low: The only change since the previously reviewed commit is comment and documentation prose that corrects the stated blast radius and hardens a manual verification loop, with the classifier logic, verdicts, and test assertions provably untouched, and the two empirical claims I could check on this host verified accurate.
Testing
Beyond running the committed suites, I reproduced the bug the way an end user hits it: a real idle
claudein a live tmux pane, whose composer row genuinely carries❯+ U+00A0. Under LC_ALL=C the base classifier called that live panepending, sofm_pane_input_pendingreported the composer as not proven empty and an away-mode injection would defer forever; the fix commit calls the same live paneemptyunder both C and en_US.UTF-8. Typing real unsubmitted text into the same composer keeps every configuration atpending, so the safety half is intact, and the matrix also shows the fix closing a macOS-UTF-8 hole where a bare$prompt plus NBSP used to readempty. Targeted suites are green: the composer lib suite (with its 6 new locale cases) and the doc-prescribed composer/tmux-adapter set under both locales, plus the herdr, cmux and documentation suites. Visual evidence is three rendered screenshots of the live pane, the byte-level capture and the verdict matrix. The only failure seen is one orca spawn case that reproduces unchanged on the base commit and is unrelated to the classifier./var/folders/22/8q8yl6t93s192drlxs4hkp5r0000gn/T/no-mistakes-evidence/01KZHNKVFPBWPX6K3SQX7PMWHM/screenshots/evidence-1-live-idle-pane.png)/var/folders/22/8q8yl6t93s192drlxs4hkp5r0000gn/T/no-mistakes-evidence/01KZHNKVFPBWPX6K3SQX7PMWHM/screenshots/evidence-2-unsubmitted-text.png)/var/folders/22/8q8yl6t93s192drlxs4hkp5r0000gn/T/no-mistakes-evidence/01KZHNKVFPBWPX6K3SQX7PMWHM/screenshots/evidence-3-regression.png)Evidence: Rendered HTML: live idle pane evidence page
Evidence: Rendered HTML: unsubmitted-text safety page
Evidence: Rendered HTML: regression coverage page
Evidence: Live idle Claude pane: capture-pane bytes and before/after verdicts
# Live idle Claude Code composer, classified through the real tmux adapter # host: macOS, tmux 3.7b, claude 2.1.226 (Claude Code) # pane: tmux -L fm-nbsp-ev session "nbsp", a realclaudesitting idle (nothing typed) $ tmux capture-pane -p -e -t nbsp | sed -n 29p | hexdump -C 00000000 1b 5b 33 39 6d e2 9d af c2 a0 0a |.[39m......| 0000000b ESC [ 3 9 m | e2 9d af = U+276F | c2 a0 = U+00A0 NO-BREAK SPACE BEFORE bin/fm-composer-lib.sh @ 833a9a2 LC_ALL=C fm_tmux_composer_state -> pending away-mode/steer: DEFER - composer not proven empty BEFORE bin/fm-composer-lib.sh @ 833a9a2 LC_ALL=en_US.UTF-8 (macOS-only false green) fm_tmux_composer_state -> empty away-mode/steer: DELIVER - composer proven empty AFTER bin/fm-composer-lib.sh @ 69aa5a3 LC_ALL=C fm_tmux_composer_state -> empty away-mode/steer: DELIVER - composer proven empty AFTER bin/fm-composer-lib.sh @ 69aa5a3 LC_ALL=en_US.UTF-8 fm_tmux_composer_state -> empty away-mode/steer: DELIVER - composer proven emptyEvidence: Live pane with unsubmitted text: verdict stays pending everywhere
Evidence: Classifier verdict matrix over the NBSP row family (base vs fix, both locales)
Evidence: Composer and tmux-adapter suites under C and en_US.UTF-8
Source: Composer and tmux-adapter suites under C and en_US.UTF-8 (local file:
/var/folders/22/8q8yl6t93s192drlxs4hkp5r0000gn/T/no-mistakes-evidence/01KZHNKVFPBWPX6K3SQX7PMWHM/two-locale-suite-loop.txt)/var/folders/22/8q8yl6t93s192drlxs4hkp5r0000gn/T/no-mistakes-evidence/01KZHNKVFPBWPX6K3SQX7PMWHM/idle-claude-pane-raw.ansi) - Outcome:Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
bin/fm-composer-lib.sh:61- The narrowed claim is still Darwin-specific. Lines 60-64 state that U+202F/U+2007/U+3000 "would still be trimmed under UTF-8 and survive under C/POSIX, the same divergence this task fixed for U+00A0". That divergence is macOS locale data: I probed this host and under en_US.UTF-8 both U+00A0 and U+202F are [[:space:]] while under C neither is. glibc excludes the non-breaking spaces (U+00A0, U+202F, U+2007) from the space class in every locale, so on Linux U+202F/U+2007 are not trimmed under UTF-8 either, and, more importantly, the pre-fix falsependingon the idle claude row reproduced under en_US.UTF-8 and C.UTF-8 on Linux too, not only under C/POSIX. The fix is byte-exact so it covers both; only the claim needs narrowing. Same wording is mirrored at docs/verification/runtime-backends.md:172 and :174 ("for any home running under C/POSIX") and tests/fm-composer-lib.test.sh:25-28. Suggest scoping the sentence to the verified macOS observation, or restating it as "[[:space:]] coverage of these characters is locale-data dependent (glibc excludes the non-breaking ones in every locale), which is exactly why the fold is byte-exact".docs/verification/runtime-backends.md:199- The manual re-verification loop hardcodesfor loc in C en_US.UTF-8, eight lines after the doc explains (line 191) that the test probes its UTF-8 locale name because glibc images and macOS ship different names. On an image without en_US.UTF-8 (common on Debian/Ubuntu, which ship C.utf8),LC_ALL=en_US.UTF-8makes bash warn and fall back to C, so the loop runs C twice, prints no FAIL line, and reads as a clean two-locale pass. The suites in that loop (fm-composer-ghost, fm-tmux-*) are not self-parameterized like fm-composer-lib.test.sh, so this loop is the only locale coverage they get. Use the same probe the test uses, or at minimum fall back to C.UTF-8, and have the loop fail loudly when no UTF-8 locale resolves.bin/fm-tmux-lib.sh:175- Recording the deferred adjacent gap so it stays visible, not asking for it in this change. If a harness ever draws NBSP inside a BORDERED composer, fm_tmux_composer_geometry_spaces still fails it: theLC_ALL=C sed 's/[!-~]/ /g'only maps ASCII printables, so the NBSP bytes survive, and the resulting string can never equaltop_spaces(a run of plain spaces). Under C it returns 1 via*[![:space:]]*; under macOS UTF-8 it passes that guard but fails the width-string equality at fm-tmux-lib.sh:283. Either way geometry_ambiguous=1 and fm_tmux_composer_state reportsunknown, which fm_pane_input_pending treats as not-empty, reproducing the same deferred-injection symptom in every locale. This is not reachable for the harness that motivated the fix (claude 2.1.226 draws a bare❯+ NBSP row, per the captured hexdump), and the author explicitly scoped bin/fm-tmux-lib.sh out, with the header and doc claims narrowed to match. No action needed for this merge.🔧 Fix: correct locale blast-radius claims and harden doc locale loop
✅ Re-checked - no issues remain.
tests/fm-backend-orca.test.sh:688- tests/fm-backend-orca.test.sh fails at "Orca spawn should fail when metadata cannot be written" (fm-spawn.sh exits 0 when the metadata path is a directory, instead of failing with "Is a directory"). This is unrelated to the composer classifier and is pre-existing: it reproduces identically on the base commit 833a9a2 (verified by extracting that tree withgit archiveand running the same suite). Baselined, not a regression from this change.tmux -L fm-nbsp-ev new-session -d -s nbsp -x 120 -y 32 "LANG=C LC_ALL=C claude"— launched a real idle Claude Code (2.1.226) in tmux 3.7b, thentmux capture-pane -p -e -t nbsp | sed -n 29p | hexdump -Cto prove the composer row isESC[39m+ U+276F + U+00A0LC_ALL=C bash -c '. bin/fm-tmux-lib.sh; fm_tmux_composer_state nbsp; fm_pane_input_pending nbsp'against the live pane, withbin/fm-composer-lib.shfrom base833a9a2(pending / DEFER) and from fix69aa5a3(empty / DELIVER); repeated underLC_ALL=en_US.UTF-8tmux send-keys -t nbsp -l 'fix findings 1 and 3'then the same livefm_tmux_composer_statecalls — pending under base-C, fix-C and fix-UTF-8 (unsubmitted text is never typed over); composer cleared withC-uafterwardsfm_composer_classify_contentverdict matrix over the whole NBSP row family (idle ❯/›/⟩ rows, idle placeholder, typed text, bare$prompt with and without a command) under base vs fix, LC_ALL=C and en_US.UTF-8LC_ALL=C bin/fm-test-run.sh tests/fm-composer-lib.test.sh(15 cases, includes the 6 new locale cases)Doc-prescribed two-locale loop:LC_ALL=$loc tests/$t.test.shforfm-backend-tmux-smoke,fm-busy-adapter-wiring,fm-busy-state,fm-composer-ghost,fm-composer-lib,fm-tmux-agent-liveness,fm-tmux-submit-busyunder bothCanden_US.UTF-8bin/fm-test-run.sh tests/fm-backend-herdr.test.sh tests/fm-backend-cmux.test.sh tests/fm-backend-orca.test.sh tests/fm-documentation-audiences.test.sh— the other consumers of the shared classifier plus the docs-surface suitePre-fix negative control: rantests/fm-composer-lib.test.shagainst a shadow root carrying the basebin/fm-composer-lib.sh, and re-rantests/fm-backend-orca.test.shinside agit archive 833a9a25extraction to baseline its failuredocs/verification/runtime-backends.md:177- Judgment call, left as is. This maintainer-verification record is headed "Verified on 2026-08-08 with tmux 3.7b and claude 2.1.226 (Claude Code) on macOS", but lines 172 and 177 assert glibc behavior (U+00A0 excluded from the space class in every locale; only U+3000 trimmed of the three fixed-width spaces) that was never probed on a Linux host. I confirmed the macOS half directly on this host (UTF-8 trims U+00A0/U+202F/U+2007/U+3000; C trims none) and left the glibc half alone: line 175 already attributes it to glibc locale data rather than to a probe, and the self-parameterizing regression in tests/fm-composer-lib.test.sh exercises the real glibc behavior wherever Linux CI runs it. Flagging it only so a reviewer with a Linux host can promote it from cited-upstream-behavior to probed evidence, or scope the sentence explicitly.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.