test: enforce brief scaffolding under stock macOS Bash - #1877
Open
moneyteamameen wants to merge 1 commit into
Open
test: enforce brief scaffolding under stock macOS Bash#1877moneyteamameen wants to merge 1 commit into
moneyteamameen wants to merge 1 commit into
Conversation
huynhtandat223
pushed a commit
to huynhtandat223/firstmate
that referenced
this pull request
Aug 7, 2026
…unchenguid#1229) * fix: remove projected workspaces through Herdr's focus-preserving pane-death path Herdr 0.7.5's explicit close of a workspace-emptying last pane moves the attached client's focus to a neighbor workspace, flashing the captain's whole window and routing in-flight keystrokes to the wrong pane until Firstmate's exact-tab restore masks it 56-197 ms later. Teardown and cleanup now plan a workspace-emptying close as a focus-safe removal: verify the close empties the workspace, reposition the doomed workspace behind the focused one through the verified workspace.move transport when it sits before a non-last focused workspace, prove the pane holds one lone idle shell, and end that shell so Herdr removes the emptied workspace through its focus-preserving pane-death path. Any ambiguity or failure falls back to the plain close behind the existing restore backstop, and fm_backend_herdr_kill applies the same plan for non-projected removals. Two conditions proven on real hardware are encoded in the adapter: BSD ps reports a login shell's comm as "-zsh", and an idle shell transiently hosts a prompt helper right after a workspace.move relayout, absorbed by a bounded strict-sample settle window in the idle-shell proof, now the single owner shared with session-start cleanup. An isolated-lab regression reproduces the raw steal on 0.7.5 and proves the plan removes a doomed workspace with zero wrong-focus samples and no corrective focus; unit fixtures cover the position, edge, ambiguity, move and kill failure, escalation, and transient-helper cases. Upstream fixes (kunchenguid#1877 explicit close, kunchenguid#1912 pane death) are merged but unreleased; once released the plan degrades to a harmless reorder-then-remove. * no-mistakes(review): Confirm pane death from structured not-found responses * no-mistakes(review): Serialize Herdr kills and sample focus continuously * no-mistakes(review): Synchronize Herdr focus evidence output * no-mistakes(review): Refuse unlocked Herdr pane closes * no-mistakes(document): Correct Herdr focus-safety documentation * no-mistakes: apply CI fixes * fix: never erase a Herdr task's records while its pane survives a refused close A transient presentation-lock contention could produce a completed teardown while the exact Herdr pane stayed alive as an unowned restored shell: the kill refused the unlocked close (correctly), returned success, the warning was suppressed, and cleanup erased the task's status, turn-end, and metadata records after the isolated copy had already been returned. Teardown now acquires the named-session presentation lock before anything destructive: a contended lock refuses up front while the isolated copy, the task branch, every durable record, and the endpoint are all intact for a plain rerun, and the projected and flat close paths both run under that one held lock instead of acquiring their own. Durable records are erased only once the exact pane is confirmed gone through its structured presence; a refused, skipped, or failed close retains every record with a visible, retryable error, and after a skipped close (unresolvable lock path) only a structured pane_not_found counts as gone - unknown never does. The teardown regression drives a live contending lock holder end to end: the refusal touches nothing (no worktree return, no branch drop, no close attempt), and the retry after release returns the copy, closes the pane under the lock, and removes the records. The unconfirmed projected close now refuses with records retained, and the structured-presence gate has a strict/default unit matrix. * no-mistakes(review): Require structured pane-not-found before Herdr record removal * no-mistakes(document): Correct Herdr record-retention verification date * fix: refuse ambiguity, revalidate SIGKILL ownership, and roll back failed removals Three accepted-contract corrections from the post-CI personal review of the Herdr keep-spaces focus-flash mitigation. Ambiguous endpoint identity no longer counts as a confirmed-gone pane: a missing or malformed target refuses record removal in the structured presence gate, and teardown treats missing confirmation machinery as a refusal instead of skipping the gate, so only an exact structured pane_not_found ever erases durable task records. The pane-death SIGKILL escalation re-reads the exact pane's process information and refuses to signal unless the same shell pid still passes the strict bare-idle ownership proof, so a pid that exited and was reused by an unrelated process is never signaled; the refused escalation falls back to the plain close with the unrelated process untouched. A reposition whose removal is not confirmed no longer outlives the attempt: the emptying-close plan records the verified pre-move order and original index whenever it invokes the mover, and both close owners restore the exact original workspace order through a second verified move, under the same held session lock, before reporting the close as failed. Each defect was reproduced first: the unit matrix documented malformed identity as gone, the PID-reuse regression showed SIGKILL reaching a disowned pid, and the rollback regression showed a single unrestored move. Teardown-level regressions cover unparseable presence retention alongside the strict identity matrix. * no-mistakes(review): Require confirmed Herdr removal and resolvable teardown locks * no-mistakes(review): Enforce structured Herdr closes and teardown preflight * no-mistakes(review): Preflight explicit Herdr close confirmation helper * no-mistakes(document): Document Herdr rollback failure semantics * no-mistakes(review): Captain, harden recursive Herdr teardown safety * no-mistakes(document): Document recursive Herdr teardown evidence * fix: retain nested secondmate home when a recursive child cleanup fails Captain-decided Option A correction for nm-askuser-flash-r6, found during complete-diff rereview of the merged head. cleanup_firstmate_home_children's recursive secondmate branch called itself for a nested child's home without checking the result, then unconditionally removed that home right after. remove_firstmate_home ends in an unconditional recursive delete with no check for leftover records, so a nested secondmate whose own Herdr grandchild failed its confirmed-gone check would have its entire home - retained grandchild records included - erased by the very next line. Guard the recursive call the same way every other fallible call in this function already is: || return 1, skipping remove_firstmate_home and leaving the nested home and its records for a safe rerun. Empirically, fm-teardown.sh's set -eu already halted the script on the prior unguarded call before reaching removal (verified by hand with the guard reverted, under both this session's bash and stock macOS bash 3.2) - the reachable behavior was already correct. The explicit guard is still applied exactly as decided: it matches every sibling call site in the function, and it stops the correctness of this path depending on errexit's well-known fragility under refactors (a wrapping if/&&, or a future subshell) rather than on an explicit check. Adds a teardown-level regression building on the existing direct-child Herdr fixtures: a top-level secondmate contains a nested secondmate, whose own Herdr child's close goes unconfirmed. Proves through the public fm-teardown.sh interface that the nested home, the nested secondmate's own record, and the grandchild's metadata and status all survive, and that the top-level secondmate's record survives too. * no-mistakes(document): Document nested Herdr teardown retention
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
Fix Firstmate execution card FM-BRIEF-001 so bin/fm-brief.sh reliably scaffolds ship and scout briefs under stock macOS Bash 3.2 when generated prose, interpolated repository names, or quoted paths contain apostrophes. Reproduce the historical heredoc-inside-command-substitution parse failure, use the smallest style-consistent solution, preserve generated brief bytes and all no-mistakes, direct-PR, local-only, and scout behavior, and add focused regression coverage that explicitly runs /bin/bash syntax and scaffolding checks for every ordinary-task mode with exact assertions for literal commands, paths, variables, placeholders, quoting, and mode-specific sections. Do not change dispatch policy, task lifecycle semantics, unrelated scaffolding text, or authoritative docs unless the implementation contract changes, and never merge the PR. Investigation established that the structural implementation fix is already present on the current default branch from prior work, so this branch must add the missing explicit Bash 3.2 runtime enforcement without churning generated production bytes.
What Changed
fm-brief.shsyntax and scaffolding checks explicitly through/bin/bash.Risk Assessment
✅ Low: The change is test-only, preserves production brief bytes, and adds explicit stock Bash 3.2 syntax and apostrophe-bearing scaffolding coverage for all ordinary-task modes.
Testing
No prior baseline results were supplied. Stock macOS
/bin/bash3.2.57 reproduced the historical heredoc/apostrophe parse failure, then passed the focused suite and exact CI count contract; all four ordinary-task modes scaffolded correctly with apostrophes while preserving literal placeholders, quoting, commands, paths, and mode sections. One initial CI-wrapper attempt inherited zsh and failed before testing becauseBASH_VERSIONwas unset; rerunning with the configured/bin/bashsucceeded. The worktree remained clean. This is CLI-generated Markdown, so reviewer-visible brief artifacts were captured instead of screenshots.Evidence: Historical Bash 3.2 failure reproduction
GNU Bash 3.2.57 reports “unexpected EOF while looking for matching'” and exits 2, reproducing the historical parser defect.Evidence: No-mistakes brief
Generated with an apostrophe-bearing repository and FM_HOME; SHA-256 d495afdf116593d3109884070989adaad5274e6fe65c234ae721cd74e191b9d1.Evidence: Direct-PR brief
Generated with an apostrophe-bearing repository and FM_HOME; SHA-256 2251d81e7e2e7a2a12fd9c6ec0ac8b0ac769e7df3aba400ad274e2509f825956.Evidence: Local-only brief
Generated with an apostrophe-bearing repository and FM_HOME; SHA-256 2db9b6320f47c11409d1f8836c484aafeb58c534b1dc545a41afb7ce61aac3d7.Evidence: Scout brief
Generated with an apostrophe-bearing repository and FM_HOME; SHA-256 cd092eb930349b2b59e368c96c0645ab524096c1543980d7906a72831ddca91a.Evidence: Generator byte preservation
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
/bin/bash --version | head -1/bin/bash tests/fm-brief.test.shCI-equivalent Bash 3.2 wrapper invoking/bin/bash tests/fm-brief.test.shand asserting exactly 21 successful checks/bin/bash -n .../historical-heredoc-apostrophe.sh(expected historical failure, exit 2)FM_HOME=".../captain's evidence" /bin/bash bin/fm-brief.sh apostrophe-nm "captain's repo" --mode no-mistakesFM_HOME=".../captain's evidence" /bin/bash bin/fm-brief.sh apostrophe-direct "captain's repo" --mode direct-PRFM_HOME=".../captain's evidence" /bin/bash bin/fm-brief.sh apostrophe-local "captain's repo" --mode local-onlyFM_HOME=".../captain's evidence" /bin/bash bin/fm-brief.sh apostrophe-scout "captain's repo" --scoutCompared base and targetbin/fm-brief.shGit blob IDs and generated-brief contents, quoting, placeholders, paths, commands, and mode-specific sections.✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.