Repository navigation
fix(bin): stop the wake drain forking a process per status line, stat, and cursor row - #79
Merged
Merged
Conversation
…ne status line A drain over a 25-task fleet entered about 6,500 subshells per run, because every presentation-cursor row read paid IFS=$(printf '\t'), every identity, size, and mtime read paid its own stat plus a command substitution, and every unread resolved line paid two more substitutions for its key and note. - Use $'\t' for the field separator in the drain-path loops. - Give the key, note, cursor-path, identity, size, offset, and unread-line readers an optional out-variable form and read the manifest without cat. - Read identity, size, and mtime from one stat per file. On a scratch copy of the main home the drain falls from 6,534 to 466 subshell entries with byte-identical output. A new test counts subshell entries and fails when they grow with unread history or grow faster than linearly with the fleet.
…ne status line A drain over a 25-task fleet entered about 6,500 subshells per run, because every presentation-cursor row read paid IFS=$(printf '\t'), every identity, size, and mtime read paid its own stat plus a command substitution, and every unread resolved line paid two more substitutions for its key and note. - Use $'\t' for the field separator in the drain-path loops. - Give the key, note, cursor-path, identity, size, offset, and unread-line readers an optional out-variable form and read the manifest without cat. - Read identity, size, and mtime from one stat per file. On a scratch copy of the main home the drain falls from 6,534 to 466 subshell entries with byte-identical output. A new test counts subshell entries and fails when they grow with unread history or grow faster than linearly with the fleet.
…exceeds latency target
…ree cursor updates
…ne status line A drain over a 25-task fleet entered about 6,500 subshells per run, because every presentation-cursor row read paid IFS=$(printf '\t'), every identity, size, and mtime read paid its own stat plus a command substitution, and every unread resolved line paid two more substitutions for its key and note. - Use $'\t' for the field separator in the drain-path loops. - Give the key, note, cursor-path, identity, size, offset, and unread-line readers an optional out-variable form and read the manifest without cat. - Read identity, size, and mtime from one stat per file. On a scratch copy of the main home the drain falls from 6,534 to 466 subshell entries with byte-identical output. A new test counts subshell entries and fails when they grow with unread history or grow faster than linearly with the fleet.
…exceeds latency target
…ree cursor updates
…thout per-row forks
…o branch Ordinary merge of refs/no-mistakes/recover/01M4F9B87M5A8ZB6B0YDWF5A7V. Conflicts in six wake-drain files resolved to the pipeline side, which carries the same fork-free drain fix rebased onto newer main plus the review, test and document fix commits. Resulting tree equals 9bdbde2.
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.
Intent
I would like all bugs to be fixed so tomorrow can be focused entirely to Vernant and not problems preventning Vernant from getting built.
A recommended architecture should come with proof.
Context (the backlog record for this bug, as filed):
Wake drain spawns about 4,500 processes per run; cut it so a locked session start meets the under-20-second target (repo: firstmate) (kind: ship) (priority: 1) (since 2026-10-07)
Found by fm-session-start-fast (2026-10-08): after that fix, a full locked session start fell from 107s to 51s on a scratch copy of the main home, but the wake drain alone stays about 22-29s because it creates about 4,500 processes per run. The under-20-second target is not met until the drain is fixed. Measure the drain's process count and time first, then remove the per-line subprocess fan-out. Mode no-mistakes, yolo per firstmate registry.
What Changed
status_presentation_cursor_offset,status_outcome_backstop_cursor_offset,status_new_lines_since_cursor,_fm_decision_key_into,_fm_decision_fold_line_into,status_line_note, and_fm_decision_drop. The stdout forms still exist and keep their old bytes. The incremental decision fold folds each line in-process. Routine closes against an empty open set skip the key and note parse.basenameand$(printf '\t')are replaced with parameter expansion and$'\t'.bin/fm-status-io-lib.shadds one batchedstatcall per scan (_fm_status_stat_batch_into). It also adds a combined identity/size/mtime reader (_fm_status_stat_raw/_fm_status_stat_into) that falls back to one per-file stat. The identity and size reader seams still bypass the batch, and post-read validation instatus_snapshot_latest_eventstill uses a fresh stat. Presentation-manifest reads use_fm_read_file_into, which checks the read length against the file size. A short or failed read is an error, not empty data.status_acknowledge_presented_snapshotnow reads and classifies each task's unread span one time. It returns the acknowledgement rows and the UNREAD STATUS rows together, soprint_unread_status_sectionprints the same bytes the receipt acknowledges and does not re-scan the logs. The changes are documented indocs/architecture.md. New tests cover subshell growth as history and fleet size grow, exact fold/checkpoint bytes, manifest read failures, and an append during the latest-event read.Risk Assessment
✅ Low: The fork-free fold, the batched stat facts and the checked manifest read keep the old byte and failure contracts on every production path I traced: fresh validation before commit, drain work under the presentation lock, and exact fold semantics. Behavioral drain tests cover them, and the one open latency concern was declined by the user.
Testing
I ran the real drain from base and target code roots against the same disposable lab homes. Each home had 25 cold status logs with 1,500 keyed resolutions. Target drain time fell from about 36 s to about 4 s. Subshell entries fell from 13,571 to 202. Stdout and cursor bytes did not change. A real Claude primary on a private fm-lab tmux socket did a locked session start three times, in 7.4 s, 9.7 s and 11.3 s. Each run had a matching lock and completion PID and a complete digest. Load was 32–50 during these runs. The HEAD change's snapshot-race guarantee and the retained helpers passed live. The two focused test suites passed. The manifest-read-failure and flat-fork-count scenario was not driven live in a lab; the unread-status suite (19/19 ok) is its only cover, so it is marked untested. I removed all labs, private tmux servers and the extracted base tree. There is no UI surface, so the evidence is CLI transcripts and JSON measurements.
Evidence: Validation summary (all numbers)
Source: Validation summary (all numbers)
Evidence: Base vs target drain comparison transcript
Source: Base vs target drain comparison transcript
Evidence: Drain comparison driver
Source: Drain comparison driver
Evidence: Locked start run 1 measurement
Source: Locked start run 1 measurement
Evidence: Locked start run 2 measurement
Source: Locked start run 2 measurement
Evidence: Locked start run 3 measurement
Source: Locked start run 3 measurement
Evidence: Locked start run 1 digest
Source: Locked start run 1 digest
Evidence: Locked start driver
Source: Locked start driver
Evidence: Snapshot race live transcript
Source: Snapshot race live transcript
Evidence: Headline numbers
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-classify-lib.sh:703- Simplification: after this change no production code calls scan_unread_surface_lines (fm-classify-lib.sh:700) or scan_unread_surface_snapshot (fm-classify-lib.sh:722). print_unread_status_section (bin/fm-wake-drain.sh:434) now prints rows that status_acknowledge_presented_snapshot prepares. Only tests call the helpers: tests/fm-wake-drain-unread-status.test.sh:268/272, tests/fm-parent-channel-scan-exclusion.test.sh:153/191 and tests/fm-spawn-dispatch-profile.test.sh:1247. The change still adds _FM_STATUS_STAT_BATCH scoping and _fm_status_stat_batch_into calls to both helpers (fm-classify-lib.sh:703-704 and 724-725). The intent (cut the drain's per-line or per-row process fan-out) does not need those lines. The helpers also become a parallel copy of the unread-surface row rule that now lives inline at fm-classify-lib.sh:604-609. As a result, test_snapshot_does_not_ack_a_later_append (tests/fm-wake-drain-unread-status.test.sh:268) checks the endpoint-bound invariant through the unused helper, not the production acknowledgement path. Recommended remedy: remove the added batching from these two helpers. Then decide whether to retire scan_unread_surface_snapshot, or point that test at the drain path.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
drain-compare-driver.sh <base 924f479c archive> <worktree> <evidence>: real bin/fm-wake-drain.sh on base and target, 25 cold logs x 60 keyed pairs, 3 cold reps each, DEBUG-trap subshell count, PATH-shim exec count, warm second drain, stdout and cursor-byte difflocked-start-driver.sh locked-start-run{1,2,3}: realclaude -pprimary on privatetmux -L fm-labsocket with marked lab home (bin/fm-lab-home.sh create), SessionStart hook runsbin/fm-session-start.sh --source startupover 25 cold logs; lab torn down each runsnapshot-race-driver.sh <worktree>: real drain in a lab home with a note appended after the presentation snapshot; then two plain drains; then direct calls to scan_unread_surface_lines and scan_unread_surface_snapshot/bin/bash tests/fm-wake-drain-unread-status.test.sh(19/19 ok)/bin/bash tests/fm-parent-channel-scan-exclusion.test.sh(ok)✅ **Document** - passed
✅ No issues found.
🔧 Fix applied.
1 warning still open:
✅ **Push** - passed
✅ No issues found.