Conversation
added 9 commits
August 22, 2026 08:03
Thirteen scripts chose between BSD `stat -f <fmt>` and GNU `stat -c <fmt>` by branching on `uname`. That is the wrong discriminator: a Darwin kernel routinely resolves `stat` to GNU coreutils (nix-darwin, or Homebrew coreutils ahead of /usr/bin on PATH), so the kernel's name does not predict the binary's dialect. When it guesses wrong the failure is silent, not loud. GNU's `-f` is not "format" - it is --file-system. `stat -f %m <path>` treats %m as an extra operand, stats the FILESYSTEM of <path>, and prints a multi-line apfs dump. Downstream numeric checks then fail permanently: the remote-job readiness probe never reports ready (robots-xw8p), lock reaping refuses every abandoned lock, and the pending-reply signature pins at 'unreadable' so no missed reply is ever detected. The `stat -f ... || stat -c ...` fallback form does not help and is worse - GNU exits 0 on the dump, so the `||` never fires and the correct call never runs. Add bin/fm-stat-lib.sh as the single owner. It feature-detects the binary once per process and caches the answer, so the hot callers (fm_path_mtime runs inside 0.2s confirm and 0.5s attach polls) stay fork-free after the first probe. The probe order is load-bearing: it tries `stat -c %s /` FIRST, because `-c` on BSD is an unknown option that writes nothing to stdout and exits non-zero, while `-f` on GNU succeeds with junk. Only `-c`-first cannot be fooled by either flavor. Named accessors (fm_stat_mtime, _mode, _device, _inode, _links, _size, _identity, _signature, _fingerprint) keep the letter pairs in one place, and the single-value ones gate on a bare unsigned integer so a third flavor would fail cleanly instead of leaking a stray token into arithmetic. Converted call sites: fm-x-lib.sh (4 fns, plus the :421 ordering trap), fm-watch.sh, fm-pr-lib.sh (4 fns), fm-supervise-daemon.sh, fm-lock-lib.sh, fm-wake-lib.sh, fm-remote-inherit.sh, fm-remote-inherit-push.sh, fm-remote-file.sh, fm-classify-lib.sh, fm-pending-reply-lib.sh, fm-backlog-receive.sh, and fm-remote-job-lib.sh - the last so the fix from robots-xw8p, whose branch never landed, is not left as a fourteenth private copy of the same decision. bin/fm-test-run.sh is deliberately NOT converted. Its one site is already GNU-first, which is the safe ordering, and its own test copies just that file into a fixture repo, so it cannot source a sibling. A comment now pins the ordering so nobody "fixes" it into the broken direction. Eight further uname-keyed sites exist that this ticket's survey missed; they are filed as robots-ldiu rather than folded in here, because two of them need fake-`stat` fixture surgery that does not belong in a mechanical refactor. Test fixtures that hand-list a partial bin/ gain the new sibling: without it the remote-job worker fails to start and every fm-wake-lib.sh consumer aborts on a missing source. Refs: robots-e8x5, robots-xw8p, robots-ldiu
PR #102 made bin/fm-pr-lib.sh and bin/fm-lock-lib.sh source the new leaf bin/fm-stat-lib.sh, each resolving the sibling from its own unresolved BASH_SOURCE directory. fm-gotmp.test.sh builds a fake FM_HOME/bin by symlinking each sibling the real fm-teardown.sh sources, but not the new fm-stat-lib.sh, so the nested source failed against the fake bin and teardown exited non-zero under set -e ("teardown exited non-zero with a valid tasktmp"), failing CI on the #102 run. Add the fm-stat-lib.sh symlink to both fake-bin fixtures in this file (make_fake_root and the inline builder in test_teardown_skips_gracefully_without_tasktmp); the second would have tripped the same failure once the first test stopped aborting the suite.
…sion-lib, fix backend fixture
…dering
Three comments asserted that GNU `stat -f <fmt> <path>` "exits 0 with a
filesystem dump, so the fallback never runs". That is inverted, and it is the
load-bearing justification for GNU-first probe ordering — a maintainer who
tests the claim, finds it false, and concludes the `||` fires cleanly would
flip to BSD-first and reintroduce the exact defect this branch fixes.
Measured (GNU coreutils 9.7 ahead of /usr/bin on Darwin):
stat -f %m README.md -> rc=1, 227 bytes of apfs
dump ALREADY on stdout
stat -f %m F 2>/dev/null || stat -c %Y F ... -> rc=0, len=238
(dump + real mtime)
stat -c %Y F 2>/dev/null || stat -f %m F ... -> rc=0, len=10 (correct)
/usr/bin/stat -c %s / -> rc=1, stdout EMPTY
GNU's `-f` is --file-system and takes no format operand: it writes the dump to
STDOUT and only THEN exits 1. The `||` DOES fire; `2>/dev/null` silences only
stderr, so the fallback's correct integer is APPENDED to the dump already in
the pipe. The hazard is STDOUT POLLUTION AT rc=0, never a suppressed exit
status.
GNU-first is required because only BSD rejects CLEANLY — non-zero with nothing
on stdout — so its discarded probe cannot contribute output. Comments now match
the corrected bin/fm-stat-lib.sh header. Comment-only; no behavior change.
Fixes robots-qyim.
7e7bd9f corrected three of the comments asserting that GNU `stat -f <fmt> <path>` "exits 0 with a filesystem dump" so the `||` "never fires". Two sites carrying the same inverted claim were missed. This finishes the sweep; the three sites 7e7bd9f already fixed are left exactly as it wrote them, because its wording is correct and equivalent and rewording it would be diff noise on a branch with 13 green CI shards. Why they were missed, since it generalises: a line-oriented grep for the phrase cannot match bin/fm-watch.sh, where it wraps across lines 99-100 ("...and the `||` never" / "fires, so arithmetic..."). Any prose sweep over comments has to join lines before matching. bin/fm-watch.sh:99 - comment-only, same correction as the other three: GNU's `-f` is --file-system, so it writes the dump to STDOUT and only THEN exits 1. The `||` DOES fire; that is the trap, not the escape. A single `$(... || ...)` captures both commands, so the fallback's correct integer lands appended to the dump at overall rc=0, and the arithmetic under `set -u` aborts on a stray token and silently kills the watcher mid-cycle. tests/fm-watch-triage.test.sh:68 - NOT comment-only, and not cosmetic. The comment was wrong in a second way ("writes a partial filesystem dump on Linux" - wrong platform; the break is GNU-coreutils-on-Darwin), and the helper underneath it was still live-buggy: if [ "$(uname)" = Darwin ]; then stat -f %m "$1"; else stat -c %Y "$1"; fi That is the original robots-e8x5 defect verbatim. On this host - GNU coreutils ahead of /usr/bin on PATH, Darwin kernel - it takes the Darwin arm, runs GNU `stat -f %m`, and returns an apfs dump as an "mtime" into test arithmetic. Rewritten to the GNU-first probe the other standalone caller already uses (bin/fm-test-run.sh): BSD rejects `-c` with a usage error on stderr and writes NOTHING to stdout, so its discarded probe cannot pollute the pipe. Verified: helper returns a clean integer under a GNU-shadowed PATH and under a BSD-only PATH; tests/fm-watch-triage.test.sh passes 21/21 (exit=0); shellcheck -x clean; the bin/ hunk is provably comment-only (diff with comment lines filtered is empty). Refs: robots-qyim, robots-e8x5, robots-xw8p
PR 102's own review flagged bin/fm-busy-event.sh:105 as a live BSD-first
`stat -f %m || stat -c %Y` chain and then reported "no issues remain"
without the line being touched. It was never fixed, so the PR shipped the
exact anti-pattern fm-stat-lib.sh exists to delete (robots-ivgz).
The failure is not a fallback that picks the wrong flavor. GNU's `-f` is
--file-system, so `stat -f %m "$LOCK"` treats %m as a second FILE operand,
writes a multi-line filesystem dump to STDOUT, and only then exits 1. Both
halves of the chain share one command substitution, so the fallback's
correct mtime is APPENDED to the dump and the whole substitution succeeds
at rc=0 — the trailing `|| echo "$now"` guard never fires either.
`age=$((now - mtime))` then dies on the garbage.
Impact: the stale-lock reap in lock_acquire is unreachable on any host
where `stat` resolves to GNU coreutils (nix-darwin, Homebrew coreutils
ahead of /usr/bin) regardless of what `uname` says. A busy-state lock left
by a holder that died mid-write is never reclaimed — permanently, not for
FM_BUSY_LOCK_STALE_SECS. lock_acquire falls through to "busy-state lock
timeout" and every subsequent write to that task's record fails.
Route it through fm_stat_mtime, which feature-detects the binary and
returns a bare integer or nothing, and keep an explicit digit gate before
the arithmetic.
Regression test: tests/fm-busy-state.test.sh shadows `stat` with a shim
that speaks GNU's dialect on any kernel — `-c` delegates to the host's
real stat, `-f` dumps and exits 1 — stages an aged lock dir, and asserts
the reap. Verified it fails on origin/main's fm-busy-event.sh
("line 107: File: unbound variable") and passes here.
Also restore the `# <path>; prints epoch seconds, or nothing and returns 1`
contract comment on fm_remote_job_path_mtime, dropped while rebasing this
branch over the inline fix main landed there.
Refs: robots-ivgz, robots-6qo0
…, narrow docs claim
Owner
Author
|
Superseded, closing. PR #104 (
Rebasing this branch onto main would therefore be ~90% no-op conflict resolution for no added value. One thing here was genuinely absent from main: the GNU-stat regression test in Tracking ticket: robots-6qo0. |
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.
What Changed
bin/fm-stat-lib.sh, a single shared owner for portablestat: it feature-detects whether the host'sstatbinary speaks GNU (-c) or BSD (-f) by probing the binary itself (probing-cfirst so the discarded probe can never pollute stdout), caches the answer per process, and exposes field helpers (fm_stat_mtime,fm_stat_size,fm_stat_mode,fm_stat_identity,fm_stat_signature,fm_stat_fingerprint, etc.) that validate integer results and support anFM_STAT_DIALECT_OVERRIDEtest hook.uname-keyed branches and unsafestat -f … || stat -c …fallback chains — both of which pick the wrong dialect or capture GNU's filesystem dump when a Darwin kernel resolvesstatto GNU coreutils; infm-busy-event.shthis makes the stale writer-lock reclaim actually reachable on GNU-stat hosts, where the multi-line mtime capture previously broke the age arithmetic.fm-stat-lib.shindocs/scripts.md.Risk Assessment
✅ Low: All three prescribed fixes are verified correct in the current code (source-time cache priming persists under set -eu, both missed fixtures now carry fm-stat-lib.sh, docs claim narrowed), the fix round stayed exactly within scope, and no new hazards were introduced — the primed cache cannot leak into the PATH-shim regression test since the variable is unexported and that test drives a child executable.
Testing
Ran the targeted suites at the target commit: fm-busy-state fully green including the new GNU stale-lock reclaim regression test, the fm-remote-job dialect shim subtests green with round 1's cache-clear fix, and fm-gotmp green; captured a CLI transcript proving end-to-end that fm-stat-lib.sh detects the dialect from the stat binary rather than uname (GNU stat ahead of PATH on a Darwin kernel yields 'gnu' and correct integer mtimes, while both legacy patterns are shown emitting garbage); the only failure is the pre-existing base-commit timing flake in fm-remote-job, and the slow fm-watch-triage watcher suite (test-helper-only change here) is left to remote CI after exceeding the local time budget.
Evidence: End-to-end stat dialect detection transcript (BSD host, GNU-shim-on-Darwin, and legacy-pattern failure demo)
--- Scenario B: same Darwin kernel, GNU-coreutils stat ahead of PATH (the robots-6qo0 failure mode) --- uname: Darwin <-- uname still says Darwin stat is now: .../gnubin/stat (speaks GNU) fm_stat_dialect: gnu <-- probed from the binary, NOT uname fm_stat_mtime .../probe: 1787414270 --- Scenario C: why the old patterns were broken under GNU stat --- old "stat -f ... || stat -c ..." chain output (garbage PREPENDED to real answer, overall rc=0): File: ".../probe" ID: 100000d00000008 Namelen: 255 Type: apfs Block size: 4096 Fundamental block size: 4096 1787414270 rc=0 <-- non-integer multiline value at rc=0: fatal to caller arithmeticEvidence: New GNU stale-lock regression test passing in fm-busy-state suite
Evidence: Remote-job dialect shim subtests passing at target commit
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed ✅
bin/fm-stat-lib.sh:36- The dialect cache (_FM_STAT_DIALECT) never persists: every caller invokes the helpers via command substitution ($(fm_stat_mtime ...)), so the cache assignment happens in a discarded subshell and the probe fork re-runs on every single call. This doubles stat forks in exactly the hot paths the header cites (fm-wake-lib's 0.2s confirm / 0.5s attach polls), and fm-wake-lib.sh:71-73's new comment ('stays fork-free per call') is false as written — a regression versus the old code, which resolved uname once at source time. Fix: prime the cache at source time in fm-stat-lib.sh (e.g.fm_stat_dialect >/dev/null 2>&1 || trueat the bottom of the file — it runs in the sourcing shell, so the cached value is inherited by all later subshells), and update the 'No side effects on source' header line accordingly.tests/fm-remote-job-orphan-reap.test.sh:71- Two test fixtures were missed by the branch's own fm-stat-lib sibling sweep. tests/fm-remote-job-orphan-reap.test.sh:71 copies fm-remote-job-lib.sh + fm-remote-job-worker.sh into the fixture root without fm-stat-lib.sh, and tests/fm-sessionstart-hook-live-e2e.test.sh:120 symlinks fm-wake-lib.sh into the lab without it. Both libs now source fm-stat-lib.sh from their own directory; under the affected scripts' set -u (no set -e) the failed source does not abort but leaves fm_stat_* undefined — verified: bash continues after a missingsource— so worker_lock_recent silently always answers 'recent', mtime-based paths degrade, and 'No such file or directory' / 'command not found' noise lands on stderr. The branch fixed four sibling fixtures for exactly this reason ('omitting it makes the worker fail to start' per its own comment in tests/fm-remote-job.test.sh); add fm-stat-lib.sh to these two copy/symlink lists the same way.docs/scripts.md:107- The claimed durable fix leaves the same authorized failure reachable on the exact host class it targets (Darwin kernel resolvingstatto GNU coreutils). Unmigrated uname-keyed sites: bin/fm-bootstrap.sh beads_sync_stamp_mtime (~line 273) and x_mode_write_if_changed (~lines 918-928, wherestat -f %dunder GNU exits 1 so Relay artifact writes are silently fail-closed refused); bin/fm-config-inherit-lib.sh:97-117 (mode/device/link-count helpers gating inherited-material propagation); bin/fm-fleet-snapshot.sh:956-963 and 1091-1093 (file_mtime_epoch captures a multi-line filesystem dump via|| true, and the bsd stat_style handed to the subprocess makes the size stat exit 3); bin/fm-startup-memory-budget-lib.sh:25-29. docs/scripts.md:107 meanwhile declares fm-stat-lib.sh 'the single owner of portable stat calls across the fleet', which is not yet true. The shared boundary now exists — migrate these stragglers to fm_stat_* (mechanical, same pattern as the 14 sites already converted), or at minimum narrow the docs claim until they are.bin/fm-stat-lib.sh:26- The dialect-detection design itself is solid: probing-cfirst is provably the only clean order (BSD rejects -c with empty stdout; GNU pollutes stdout on -f before failing), the _fm_stat_uint integer guard converts any third-dialect surprise into a clean failure, and the new test_stale_writer_lock_reclaimed_under_gnu_stat exercises the real fm-busy-event executable through a PATH-shimmed GNU stat rather than grepping source — a genuine behavioral regression test for the reported failure.🔧 Fix: prime stat dialect cache at source, fix fixtures, narrow docs claim
✅ Re-checked - no issues remain.
tests/fm-remote-job.test.sh:404- Pre-existing test failure unrelated to this change: tests/fm-remote-job.test.sh case 'queue time consumed the second job's execution timeout' (line 404) fails identically at the base commit (04d3ad7, verified via a clean temp extraction) and at the target commit on this host. It is a timing-sensitive case (1.8s job under a 3s execution timeout queued behind another job) and its failure aborts the suite, so the cases after it could not be verified locally at either commit. Remote CI owns confirming this suite's tail; if it is green there, this is a host-timing flake.tests/fm-remote-job.test.sh:159- The head commit 411a61b added source-time priming of fm-stat-lib.sh's dialect cache but missed reconciling tests/fm-remote-job.test.sh: its GNU/BSD dialect subtests swap a shimmed stat into PATH inside a subshell of the already-primed test shell, so the inherited cache ignored the shim and the GNU case failed ('got &feat(fm-backend): extend legacy-metadata self-repair to zellij and cmux backends #39;&feat(fm-backend): extend legacy-metadata self-repair to zellij and cmux backends #39;'). Fixed by clearing the inherited _FM_STAT_DIALECT in the two shim subshells so the probe under test actually runs against the shimmed binary, matching the lib's documented per-process caching contract (production never swaps stat mid-process; children re-source and re-prime under their own PATH). Suite's dialect cases now pass.bash tests/fm-busy-state.test.sh— all 21 cases pass, including the newtest_stale_writer_lock_reclaimed_under_gnu_statregression testbash tests/fm-gotmp.test.sh— passes; validates the fixture additions that symlink fm-stat-lib.sh next to its sourcing siblingsbash tests/fm-watch-triage.test.sh— passes (exit 0); exercises the migrated GNU-first mtime helperbash tests/fm-remote-job.test.sh— dialect subtests pass after fixing the cache-priming fixture staleness; one pre-existing timing case fails identically at base and targetManual end-to-end demo: GNU-stat shim first on PATH on a real Darwin host — old uname-keyed logic and old BSD-first fallback both produce garbage, new fm-stat-lib.sh detectsgnuand returns the correct mtime; unmodified host detectsbsdand returns the same correct mtime (transcript in evidence dir)Sourced each of the 9 migrated libs (fm-stat-lib,fm-wake-lib,fm-lock-lib,fm-pr-lib,fm-classify-lib,fm-supervision-lib,fm-x-lib,fm-pending-reply-lib,fm-remote-job-lib) underbash -euc— all load cleanlygit archive 04d3ad7into a temp dir and re-rantests/fm-remote-job.test.shat base to prove the timing failure pre-exists this change🔧 Fix: clear primed stat-dialect cache in remote-job shim subtests
1 warning still open:
tests/fm-remote-job.test.sh:404- Pre-existing test failure unrelated to this change (carried from round 1): the timing-sensitive case 'queue time consumed the second job's execution timeout' fails identically at base and target on this host and aborts the suite, so the cases after it remain locally unverified at both commits. All dialect-related cases before it — including the shim subtests this change touches — pass. Remote CI owns confirming the suite's tail; green there means this is a host-timing flake.bash tests/fm-busy-state.test.sh— all 21 cases pass, including the newtest_stale_writer_lock_reclaimed_under_gnu_statregression test (GNU stat shim on Darwin kernel; stale lock reclaimed, no dialect garbage, lock released)bash tests/fm-remote-job.test.sh— the GNU/BSD stat-dialect shim subtests ('the mtime reader survives either stat dialect ahead of /usr/bin') pass with round 1's_FM_STAT_DIALECT=cache-clear fix; the later 'queue time consumed the second job's execution timeout' failure is the pre-existing host-timing flake verified at base in round 1bash tests/fm-gotmp.test.sh— all 3 teardown tasktmp cases pass with fm-stat-lib.sh symlinked into the fake teardown binManual end-to-end verification: sourcedbin/fm-stat-lib.shon this macOS host (uname=Darwin, BSD /usr/bin/stat → dialect 'bsd', correct mtime/size), then with a GNU-coreutils-dialect stat shim ahead of PATH (uname still Darwin → dialect 'gnu', correct integer mtime), plus a demonstration that the legacyuname-branch andstat -f || stat -cpatterns emit a filesystem dump / multi-line non-integer at rc=0 under GNU statbash tests/fm-watch-triage.test.sh— started but did not complete within the local time budget (slow watcher-poll suite); its only change is the test-local file_mtime helper migrating to the GNU-first fallback, whose behavior the manual transcript demonstrates directly; remote CI covers the suite✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.