fix(bin): make teardown exit non-zero after a fatal source failure - #3568
fix(bin): make teardown exit non-zero after a fatal source failure#3568kuan0808 wants to merge 2 commits into
Conversation
…k sibling tests/fm-gotmp.test.sh has failed since cf95112 (kunchenguid#1577), which made bin/backends/tmux.sh source fm-session-lock-lib.sh at load time. The test builds a fake FM_HOME by symlinking only the bin/ siblings teardown needs, and that hand-curated list never gained the new sibling. The teardown kill step then dies on the failed source; the message is hidden behind `2>/dev/null || true`, the EXIT trap returns Bash's post-fatal `$?` of 0, and the test's "did not remove the tasktmp dir" assertion is the first one that notices. Production teardown is unaffected: a real bin/ always has the sibling, and tasktmp removal works. This is a fixture fix only. - Symlink fm-session-lock-lib.sh into the fixture next to fm-tmux-lib.sh. - Stub fm-remote-job-reap-orphans.sh like fm-guard.sh and fm-fleet-sync.sh; teardown calls it best-effort, and the real sweep signals processes, so it must never be symlinked into a fixture. - Let make_fake_root omit the tasktmp= line when called with one argument, and drop the second, duplicated copy of the fixture so the sibling list lives in one place. bin/fm-teardown.sh and bin/fm-spawn.sh are untouched.
bin/fm-teardown.sh ends through an EXIT trap that returns `$status`, and Bash reports `$?` as 0 inside that trap after a fatal shell error such as a failed `source` of a missing sibling. A missing sibling in bin/ therefore made teardown exit 0 silently while the task's meta, tasktmp, endpoint, and state were all left in place, and every caller read that as success. - In the EXIT trap, treat exit 0 with the task record still present as an aborted teardown: report it on the stderr saved before the trap (the abort may happen inside a call whose own stderr is discarded) and exit 1. Every legitimate exit-0 path has already removed the record, so no refusal path or message changes. - In fm_backend_source, check the adapter file is readable before sourcing it, so a missing adapter is the ordinary failed return its callers already handle instead of a Bash 3.2 fatal error that skips their own refusal. This restores teardown's designed "nothing was changed" herdr preflight refusal on stock macOS Bash. - Regression in tests/fm-gotmp.test.sh: remove one sourced sibling from the fixture and assert non-zero exit, the stderr report, and that the task record and tasktmp survive.
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness, security, or repository-rule violations identified. The new trap condition distinguishes successful teardown from fatal aborts using metadata that all successful paths remove, while the adapter precheck routes missing files through existing failure handling. Reviews (1): Last reviewed commit: "fix(bin): make teardown exit non-zero af..." | Re-trigger Greptile |
|
Speaking as Kun's firstmate: HEAD Attestation: MISSING — no head-bound no-mistakes Contract-class: restore — teardown already promised a loud non-zero abort that leaves durable records in place ( VISION (per rule, evidence =
Overlap with #3556: this PR's first commit is the gotmp fixture sibling list ( This is waiting on you (the author), not a captain decision. Please workflow-approvals this pass: 33687571873, 33687571845 |
2 participants
bin/fm-teardown.sh ends through an EXIT trap that returns
$status, and Bash reports$?as 0 inside that trap after a fatal shell error such as a failedsourceof a missing sibling. A missing sibling in bin/ therefore made teardown exit 0 silently while the task's meta, tasktmp, endpoint, and state were all left in place, and every caller read that as success.The first commit is the fixture this fix's regression needs: tests/fm-gotmp.test.sh builds a fake FM_HOME by symlinking only the bin/ siblings teardown needs, and that hand-curated list never gained fm-session-lock-lib.sh, which bin/backends/tmux.sh has sourced since cf95112. Without it the regression cannot reach the failure it asserts. It is carried here rather than in the test-only pull request so both stand alone.