fix(bin): guard Stop-hook path commands

[gizm0duck]

Intent

Fix the basename: illegal option -- / noise prefixing Claude Stop-hook messages in the firstmate repo.

Reported symptom: every Stop-hook wake/error message the primary session saw was prefixed with basename: illegal option -- / plus BSD basename usage text. Wakes still delivered (cosmetic), but it polluted every Stop-hook message. The task was to find the actual call site by reading the Stop-hook chain (candidates named: bin/fm-turnend-guard.sh, bin/fm-claude-stop-autoarm.sh, bin/fm-watch-arm.sh, or a library they source) rather than guessing, and to reproduce the message shape on macOS BSD userland before and after.

Accepted requirements: fix the offending call robustly (basename -- "$arg" or a safe parameter-expansion substitute); fix sibling call sites with the same defect class in the touched chain in the same pass; demonstrate before/after that the reproduced BSD-basename error is gone from the Stop-hook message path while wakes still deliver; keep shellcheck clean on touched scripts; update or add colocated tests where the repo's existing test layout covers the touched files. The repo's own contributor rules apply (firstmate-coding-guidelines): one sentence per line in tracked prose, plain dash rather than em dash, shellcheck-clean bin scripts via bin/fm-lint.sh, colocated tests named <subject>.test.sh extending an existing script rather than a new runner, tests exercising behavior through an executable interface and never asserting implementation-source bytes, and no agent name as a commit co-author.

What the investigation established, which a reviewer reading only the diff would not know:

The offending call was basename "$comm" in the harness ancestry walk in bin/fm-session-lock-lib.sh. A login shell reports argv[0] as its own executable path with a leading dash - on macOS login that is literally -/bin/zsh, confirmed live with ps -o comm= on the primary session's shell ancestor - and BSD basename parses a leading dash as an option bundle, producing exactly basename: illegal option -- / plus its usage. It surfaced in the two places that merge that stderr into operator-visible output: the Stop-owned auto-arm's stderr (which Claude Code renders as "Stop hook blocking error") and the session-start LOCK section, which runs fm-lock.sh 2>&1. bin/fm-turnend-guard.sh never showed it, because it is the one script in the chain that does not source that lib - which is what pinned the location.

That specific call already takes -- at HEAD; commit 6ec5e08 (2026-07-29) fixed it. Evidence gathered that the symptom is no longer live: the current primary session's LOCK section reads lock acquired: harness pid 7930 with no diagnostic and all of its Stop-hook messages are clean, while every polluted message still visible in that home's history uses auto-arm banner wording that was removed from bin/ on 2026-08-01, i.e. it is pre-update scrollback in a long-running session. The guard, auto-arm (run end-to-end beneath a synthetic dash-prefixed-argv0 ancestor), arm wrapper, watcher, wake drain and lock acquisition were each exercised at HEAD against an instrumented basename/dirname shim and none produced the message.

Deliberate decisions made, so they are not read as accidents:

1. The remaining path-tool calls in that same chain carried the same latent defect, so they were hardened in this pass as the requirement asks: -- separators for every basename, dirname, readlink and cd in bin/fm-turnend-guard.sh, bin/fm-claude-stop-autoarm.sh, bin/fm-watch-arm.sh, bin/fm-watch.sh, bin/fm-lock.sh and bin/fm-wake-lib.sh. Those arguments are absolute paths today, so the change is behavior-preserving hardening of the defect class, not a live bug fix. bin/fm-session-lock-lib.sh is deliberately unmodified: its call is already correct.
2. Scope was deliberately limited to the Stop-hook chain rather than the roughly 99 unprotected basename calls elsewhere in bin/, because the requirement is "sibling call sites in the touched chain".
3. Two regressions were added to the existing tests/fm-claude-stop-autoarm.test.sh rather than a new test script, per the repo's colocation rule. They run the real hook and the real lock acquisition beneath a live process whose argv[0] is a dash-prefixed path, built with exec -a. The shim deliberately avoids exec after that first re-exec and ends each level with exit $?, because bash's last-command exec optimization would otherwise collapse the whole chain into one pid and leave no login shell for the ancestry walk to pass. The test records what ps -o comm= and ps -o args= actually expose on the running platform and accepts either field, because macOS ps reports argv[0] in comm= while procps reports the kernel exec name there and exposes argv[0] only through args=; it asserts that one of them really carried the dash-prefixed path, so the case cannot pass vacuously.
4. Verification performed: with the original defect restored in bin/fm-session-lock-lib.sh the new test fails with not ok - the Stop-owned auto-arm printed a path-tool diagnostic: basename: illegal option -- /, reproducing the reported message exactly; at HEAD it passes with the hook still exiting 2 and the wake banner intact, stable across three consecutive suite runs. bin/fm-lint.sh exits 0.
5. Three other suites - fm-turnend-guard, fm-watcher-lock and fm-wake-queue - fail in this sandbox, and they fail identically on a pristine HEAD worktree with none of these changes applied, so they are pre-existing and unrelated to this work.

Follow-up round, answering the upstream review on PR 2727 (2026-08-21):

6. The review raised two points. The first claimed readlink -- is not portable to macOS BSD readlink and that the -- should be removed. That premise does not reproduce: measured on macOS 26.4.1 build 25E253, readlink "-weird" fails with readlink: illegal option -- w while readlink -- "-weird" prints the target, so on this platform -- is required to read a dash-prefixed link rather than being unsupported. bin/fm-cursor-lib.sh on main already ships readlink -- today, so the repo already depends on this. The readlink -- calls are therefore deliberately unchanged, and the evidence is being posted to the PR for the maintainer to decide, since only one macOS version was measured.
7. The review's second point is correct and is what this round fixes: the regressions that shipped exercised only the basename ancestry walk under exec -a, so both readlink guards in bin/fm-wake-lib.sh were unpinned. One regression is added to tests/fm-watcher-lock.test.sh, which that suite's own header designates as the owner of watcher and lock-primitive behavior. It drives the lock helpers' own interface in the established style of the surrounding lock tests: a lock link whose path begins with a dash must resolve to its owner directory through fm_lock_link_owner and compare equal through fm_lock_points_to_owner. Verified non-vacuous by removing -- from each helper independently - fm_lock_link_owner alone fails the test with "unreadable" and fm_lock_points_to_owner alone fails it with "mismatch". The measured readlink evidence is recorded in the test's comment so the guard cannot be removed quietly later. bin/fm-lint.sh exits 0.
8. Deliberately out of scope this round: while constructing the test it became visible that mkdir -p "$STATE" in bin/fm-lock.sh and bin/fm-wake-lib.sh, and rm -f "$lockdir" in the lock helpers, carry the same missing-separator class for a dash-prefixed relative state directory. Those were left unchanged because the accepted criterion for this branch names basename, dirname, readlink and cd, and widening it again mid-review would churn the diff the maintainer is already reviewing.

Second follow-up round, conceding the maintainer's re-review of PR 2727 (2026-08-21):

9. The maintainer re-reviewed head 72ddcf6 and held again on a different and better argument: production lock paths are absolute today, so -- is not required for the original basename defect, and adding it to readlink is a regression risk on any BSD readlink that rejects --. They acknowledged the macOS 26.4.1 measurement and the existing readlink -- on main and said neither closes the hold, because the question moved from portability to necessity. That argument holds: lock paths are built under state/ from FM_HOME and are absolute in practice, so no lock path arrives dash-prefixed through any real call path, and the lock helpers were never part of the reported bug.
10. Accordingly the three readlink -- calls in bin/fm-wake-lib.sh (fm_lock_link_owner, fm_lock_points_to_owner, fm_lock_remove_stray_owner_link) are reverted to plain readlink, byte-identical to main. Every basename --, dirname -- and cd -- separator is deliberately KEPT: those are the actual reported defect and no reviewer has questioned them. No portable readlink wrapper is added; the maintainer offered that and the smaller change is to not need one.
11. The tests/fm-watcher-lock.test.sh regression is re-aimed rather than deleted or left pinning a separator that no longer exists. Deleting it would leave fm_lock_link_owner and fm_lock_points_to_owner with no direct coverage at all, since nothing else in tests/ drives them. It now pins what is in contract for an absolute lock path: the acquired lock resolves to its owner directory, the resolved path is absolute and exists, fm_lock_points_to_owner agrees with that directory and rejects a different one, and the link stops resolving after release. The comment records that the dash-prefixed case is deliberately out of contract now that -- is gone. Verified non-vacuous against three independent mutations: an unreadable link fails with "unreadable", an always-agreeing comparison fails with "false-match", and a release that leaves the link fails with "still-linked". bin/fm-lint.sh exits 0.
12. Re-run on updated tooling: the previous round's "PR must be raised via no-mistakes" failure was an installed-tooling version issue, not a defect in this diff. That check requires no-mistakes >= 1.46.0 for the structured pipeline step attestation HTML comment in the PR body, and the earlier runs used v1.41.2, which emits only the signature line. The fleet operator has since updated the shared install to v1.53.0, and this run exists to republish the PR body with that attestation so the check can pass. No code change accompanies it; the branch content is unchanged from the conceded state described above.

What Changed

• Hardened Stop-hook, watcher, lock, and wake path handling by passing option separators to basename, dirname, and cd calls.
• Documented why lock helper readlink calls retain their existing absolute-path contract.
• Added regressions for clean Stop-hook and lock behavior beneath a dash-prefixed login-shell ancestor, plus lock owner-link lifecycle coverage.

Risk Assessment

✅ Low: The changes consistently protect the Stop-hook chain’s basename, dirname, and cd path operands with --, preserve the existing session-lock fix, and add behavior-level regressions for the reported dash-prefixed ancestry case and lock-owner contract.

Testing

The focused Stop-hook suite passed, including end-to-end dash-prefixed login-shell ancestry checks that preserve the Claude wake banner while emitting no path-tool noise; a BSD basename invocation reproduced the reported error shape, and direct lock-owner lifecycle evidence passed. The watcher-lock suite remains blocked by an unchanged, pre-existing X-mode guard-copy assertion before it reaches the newly added owner-link regression.

Evidence: BSD reproduction and clean Stop-hook/lock behavior

Source: BSD reproduction and clean Stop-hook/lock behavior

BSD basename reproduction (the reported unsafe argument): /usr/bin/basename: illegal option -- / usage: basename string [suffix] basename [-a] [-s suffix] string [...] Current lock-owner helper behavior: owner=/Users/shanewolf/.no-mistakes/evidence/01M0KGZZ8WM6XQ482WK36J6WSM/fm-lock-owner-evidence.fRirsc/state/.owner-link.lock.owner.ip0N0p owner-link-cleared=yes Live dash-prefixed Stop-hook ancestry regression: ok - auto-arm: rewake beneath a login-shell ancestor carries no path-tool diagnostic ok - fm-lock: acquisition beneath a login-shell ancestor carries no path-tool diagnostic

BSD basename reproduction (the reported unsafe argument):
/usr/bin/basename: illegal option -- /
usage: basename string [suffix]
       basename [-a] [-s suffix] string [...]

Current lock-owner helper behavior:
owner=/Users/shanewolf/.no-mistakes/evidence/01M0KGZZ8WM6XQ482WK36J6WSM/fm-lock-owner-evidence.fRirsc/state/.owner-link.lock.owner.ip0N0p
owner-link-cleared=yes

Live dash-prefixed Stop-hook ancestry regression:
ok - auto-arm: rewake beneath a login-shell ancestor carries no path-tool diagnostic
ok - fm-lock: acquisition beneath a login-shell ancestor carries no path-tool diagnostic

Evidence: Focused Claude Stop-hook behavioral test log

Source: Focused Claude Stop-hook behavioral test log

ok - auto-arm: inert in a linked child worktree even when in-flight
ok - auto-arm: inert with no session lock
ok - auto-arm: a demonstrably dead recorded session owner is reclaimed through fm-lock.sh before arming
ok - auto-arm: inert without arm, rewake, or lock replacement when another live harness owns the home
ok - auto-arm: inert while AFK owns supervision
ok - auto-arm: stale-owner recovery leaves the AFK and supervision-need gates unchanged
ok - auto-arm: resolves the outermost pid of a nested contiguous claude ancestry (bg-spare chain)
ok - auto-arm: inert with nothing in flight and no X-mode need
ok - auto-arm: actionable close translates to exactly one exit-2 rewake with reason
ok - auto-arm: actionable close survives a healthy successor without duplicate delivery
ok - auto-arm: bounded failure verification emits one automatic-mechanism alarm
ok - auto-arm: consecutive failures keep Stop-owned retry without repeating notice
ok - auto-arm: unverified clean close exhausts retries and fails closed
ok - auto-arm: post-alarm actionable outcomes cannot continue or reset failure state
ok - auto-arm: benign cycle end with a live watcher and fresh beacon stays silent across the next cycle
ok - auto-arm: budget contention preserves the episode and forces a reset retry
ok - auto-arm: X-mode poll need arms the cycle even with no tasks in flight
ok - auto-arm: concurrent firings admit one owner and one rewake translation
ok - auto-arm: an abandoned owner claim is reclaimed so a lapsed cycle re-arms
ok - auto-arm: an owner still arming is never reclaimed, however long the cycle runs
ok - auto-arm: a live claim the ledger does not name is never reclaimed
ok - auto-arm: a claim whose pid was reused is reclaimed even while its ledger entry still reads arming
ok - auto-arm: a reused-pid claim is reclaimed even with no ledger entry to prove it
ok - auto-arm: an identity-matched owner still arming is never reclaimed
ok - auto-arm: the guard's terminal-check claim is never reclaimed
ok - auto-arm: need vanishing mid-cycle closes without a rewake
ok - auto-arm: mid-cycle AFK hands triage to the daemon with no rewake
ok - auto-arm: active in a marked secondmate home
ok - fm-lock: shared session-lock lib preserves the status path
ok - auto-arm: rewake beneath a login-shell ancestor carries no path-tool diagnostic
ok - fm-lock: acquisition beneath a login-shell ancestor carries no path-tool diagnostic

- Outcome: ⚠️ 1 warning across 1 run (2m41s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 warning

• ⚠️ tests/fm-watcher-lock.test.sh:168 - tests/fm-watcher-lock.test.sh stops at its existing X-mode guard-copy assertion (guard repair line did not source the X-mode cadence config) before the newly added owner-link case. Neither bin/fm-guard.sh nor that assertion changed in this diff, so this is outside the Stop-hook hardening change.
• bash tests/fm-claude-stop-autoarm.test.sh
• bash tests/fm-watcher-lock.test.sh (stopped at pre-existing X-mode guard-copy assertion before the newly added owner-link case)
• /usr/bin/basename '-/bin/zsh' plus direct fm_lock_try_acquire / fm_lock_link_owner / fm_lock_points_to_owner / fm_lock_release lifecycle check

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

[kunchenguid]

Speaking as Kun's firstmate:

VISION verdict: align as refusal and canonicalization hardening of the Stop-hook chain.

Class: corrective in intent. basename --, dirname --, and cd -- are the right portable fix for a dash-prefixed login-shell argv0 on BSD.

Hold on stamp: readlink -- is not portable to macOS BSD readlink, which is Herdr's home turf. Main currently uses unguarded readlink "$lockdir". This PR changes those to readlink --, and stock macOS readlink treats that as illegal option -- -, which can fail lock-link resolution. The new tests exercise basename ancestry under exec -a, not the readlink helpers.

Not stamping until readlink is handled without GNU --, or through a portable wrapper. No security issue; approving first-time fork workflow runs. Not merge-eligible. Not a captain-flag.

[gizm0duck]

Thanks for holding on this - a portability worry about readlink on macOS is exactly the right thing to stop a merge for. I went and measured it, and the specific premise does not reproduce on the macOS I have. Evidence first, then the test gap, which you are right about.

Point 1: readlink -- on stock macOS

Measured on macOS 26.4.1 (build 25E253), stock /usr/bin/readlink, with PATH=/usr/bin:/bin so no third-party readlink can be in play:

$ ln -s "$T/owner.d" "$T/-dash.lock"

$ cd "$T" && readlink "-dash.lock"
readlink: illegal option -- d
usage: readlink [-fn] [file ...]
rc=1

$ cd "$T" && readlink -- "-dash.lock"
/var/folders/.../owner.d
rc=0

And the ordinary production shape, an absolute lock path, is unaffected either way:

$ readlink -- "$T/normal.lock"
/var/folders/.../owner.d
rc=0

So on this platform -- is not rejected by BSD readlink; it is what makes a dash-prefixed link readable at all. Removing it would reintroduce the same bug class this PR exists to close, on the platform the hold is about.

Second, independent data point: bin/fm-cursor-lib.sh:59 on main already ships readlink -- "$dir/$base" today, unguarded. The repo is already relying on readlink -- being portable, so this PR is not introducing a new dependency - it is making the wake-lock helpers consistent with a call main already makes.

What I did not prove. I measured one macOS version and build, with stock /usr/bin/readlink. I have not tested older macOS releases, and I have not tested a non-stock readlink earlier in PATH. If you are seeing illegal option -- - somewhere concrete, tell me the OS version, the build, and command -v readlink on that box and I will handle it. I would rather write the portable wrapper on request than build machinery for a failure that has not reproduced. Your call on the merge either way.

Point 2: the test gap was real, and is now closed

You are right that the regressions shipped here exercise the basename ancestry walk under exec -a and never touch the readlink helpers, which left both changed calls unpinned. That is fixed by the regression added in this branch.

The new regression lives in tests/fm-watcher-lock.test.sh, whose own header designates it the owner of watcher and lock-primitive behavior, and it drives the helpers through their own interface in the style of the surrounding lock tests: a lock link whose path begins with a dash must resolve to its owner directory through fm_lock_link_owner and compare equal through fm_lock_points_to_owner.

It is verified non-vacuous against each guard independently - removing -- from fm_lock_link_owner alone fails it with unreadable, and removing it from fm_lock_points_to_owner alone fails it with mismatch. The measured readlink evidence above is recorded in the test's comment so the guard cannot be quietly removed later.

bin/fm-lint.sh exits 0 on the branch.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

_{Reviews (3): Last reviewed commit: "revert(bin): drop the readlink separator..." | Re-trigger Greptile}

[kunchenguid]

Speaking as Kun's firstmate:

VISION verdict: align as corrective hardening of the Stop-hook chain. Scripts own the mechanics; a dash-prefixed login-shell argv0 must not pollute operator-visible messages. New capability is not being assumed.

Class: corrective.

Re-reviewed NEW HEAD 72ddcf67f073. The prior portability hole is still present: fm_lock_link_owner and fm_lock_points_to_owner still call readlink --. The new watcher-lock regression pins that GNU-style separator; it does not replace it with a portable wrapper, and it does not keep unguarded readlink for absolute lock paths.

The author's macOS 26.4.1 measurement and the existing readlink -- in bin/fm-cursor-lib.sh on main are noted. They do not close this hold. Production lock paths are absolute today, so -- is not required for the original basename bug, and adding it remains the regression risk on a BSD readlink that rejects --.

Security: no.
Fork CI: approved on this HEAD (CI + Require no-mistakes). Checks are not yet green.
Ahead 3 / behind 0. mergeable MERGEABLE, mergeStateStatus UNSTABLE.
Overlap: many open PRs touch bin/fm-watch.sh / bin/fm-wake-lib.sh; this diff is -- separators only. No functional overlap with the held pair #2637/#2692 or spawn-freshen #2622/#2693/#2154.

This is waiting on the author, not the captain: handle readlink without GNU --, or through a portable wrapper. Not merge-eligible. Not a captain-flag.

[gizm0duck]

Conceded - you're right, and it's the better argument. The lock helpers were never on the path of the reported defect: lock paths are built under state/ from FM_HOME and are absolute, so none can arrive dash-prefixed, and guarding readlink there was hardening beyond the bug.

Reverted: the three readlink -- calls in bin/fm-wake-lib.sh (fm_lock_link_owner, fm_lock_points_to_owner, fm_lock_remove_stray_owner_link) are back to plain readlink, byte-identical to main. No portable wrapper added - the smaller change is not to need one.

Kept: every basename --, dirname -- and cd -- separator. That is the actual defect, a login shell's dash-prefixed argv[0] reaching the ancestry walk.

The test: re-aimed rather than deleted. Deleting it would have left fm_lock_link_owner and fm_lock_points_to_owner with no direct coverage at all, since nothing else in tests/ drives them, and leaving it as-is would have pinned a separator that is no longer there. It now covers what is in contract for an absolute lock path - resolves to its owner directory, agrees only with that directory, stops resolving after release - and its comment records that the dash-prefixed case is deliberately out of contract now. Checked non-vacuous against three separate mutations.