test(tests): guard fixture cleanup from deleting the working directory

[zeeshaanahmad]

Intent

Make the firstmate test suite's temp-directory cleanup incapable of deleting the working directory.

THE INCIDENT THIS FIXES (real, already happened): a crewmate copied a test out of tests/ to instrument it. Outside tests/, the . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" source failed; bash treats a failed . as an ordinary non-zero return, so the file kept running with every helper undefined. fm_test_tmproot was undefined, so TMP_ROOT became the empty string; the widespread TMP_ROOT=$(cd "$TMP_ROOT" && pwd -P) idiom then turned that into $PWD because cd "" succeeds as a no-op; and the file's own EXIT trap ran rm -rf on it, deleting a live task worktree. 112 test files use fm_test_tmproot. The bug was reproduced end to end in a throwaway sandbox before any fix, and the same action was re-run afterwards to prove it now aborts and deletes nothing.

THREE REQUIREMENTS, all required because they fail independently:

1. An unset or empty TMP_ROOT must abort loudly and never reach rm -rf.
2. A TMP_ROOT that is not under the expected temp root must abort. Non-empty is not the same as safe: $PWD is non-empty.
3. lib.sh failing to load must be an immediate loud failure, not a script that carries on with undefined helpers. That silent continuation is the root cause; every later symptom follows from it.

SHAPE CHOSEN AND WHY (deliberate, not accidental): one shared guarded helper rather than 112 edited call sites. tests/lib.sh gains fm_test_rmtree as the single removal path for a fixture root, so the guard is written once. The 112 files that merely call fm_test_tmproot were left untouched; their fixtures are removed by fm_test_cleanup, which now routes through the guard. Only the 16 files that own their teardown trap and therefore bypass fm_test_cleanup needed a call-site edit, each a one-token substitution.

DELIBERATE DECISIONS A REVIEWER WOULD NOT SEE FROM THE DIFF:

• || exit 1 is appended to every relative source of a tests/-local helper (130 lines across 107+ files, one token each). This is requirement 3 and is load-bearing, not style. The uniform mechanical shape was chosen precisely so the diff stays reviewable.
• fm_test_own_fixture is an intentional, guarded escape hatch. Two fixtures legitimately live outside the temp root: tests/fm-lint.test.sh's source-boundary parity fixture must sit under $ROOT because the check feeds fm-lint.sh a repo-relative path, and tests/fm-session-start.test.sh registers roots at the literal /tmp/fm- because that is what the code under test creates. Declaring is guarded: it refuses the repo root, the working directory, and any ancestor of it.
• Widening the containment rule to trust /tmp or $ROOT was considered and deliberately REJECTED. The working directory can itself be under /tmp - that is exactly how the incident reproduced - and the destroyed worktree was itself under a repo root. Widening either way would re-allow the deletion the guard exists to prevent.
• fm_test_cleanup now exits 1 when any removal was refused, so a refusal swallowed by a teardown still fails the file rather than passing with a fixture left behind.
• The 2>/dev/null on tests/fm-remote-backlog-handoff.test.sh's retry loop was deliberately preserved; it mitigates a documented pre-existing CI flake and the guard still prevents the deletion regardless of diagnostic visibility.
• Two removals were deliberately NOT routed through the guard, under the task's scope boundary: tests/fm-herdr-session-cleanup.test.sh's multi-arg glob cleanup and tests/fm-backend-orca.test.sh's raw rm -rf "/tmp/fm-$id". Neither is a fixture-root teardown and neither can resolve to the working directory.
• No source-text assertion was added to catch a future revert of a guarded call site. This repo forbids tests that assert implementation bytes; the library is the enforcement point and the call sites are convention.

A COMMENT OR CONVENTION NOTE WAS EXPLICITLY NOT ACCEPTABLE as a fix, because the defect is that this happens silently to someone doing an ordinary thing.

REGRESSION AND MUTATION-FALSIFICATION (required): tests/fm-test-fixture-cleanup.test.sh gains cases that drive fm_test_rmtree directly (empty path, path outside the temp root, and the incident's exact unset-root-resolves-to-$PWD chain), cases for the declaration path in both directions, and a sweep that copies all 125 library-dependent files out of tests/ and asserts each refuses and leaves its working directory byte-identical. Every guard was mutation-falsified individually: removing it fails the suite, restoring it passes. The first mutation of the load guard did NOT fail, because the second layer absorbed it; the sweep was sharpened to assert the working directory is unchanged in both directions so it detects the silent continuation itself rather than only its worst outcome.

TWO REGRESSIONS THIS WORK ITSELF CAUSED, FOUND AND FIXED: the guard's first shape refused legitimate teardowns in tests/fm-lint.test.sh and tests/fm-session-start.test.sh. Both were found only by running the full CI portable lanes, not by targeted verification, and both are fixed by explicit guarded declaration rather than by weakening the guard. After the second, every removal target in the suite was enumerated to confirm no third exists.

SCOPE BOUNDARY: fix the deletion hazard only. Do not restructure test fixtures, rename helpers, or tidy the 112 call sites beyond what the guard needs. A large diff is harder to trust and this is the change where trust matters most.

REBASE CONTEXT: this branch was rebased onto current origin/main 99c1d7e, which is PR 28 ("bound remote-job worker waits to fix CI hangs and leaks") and edits tests/lib.sh too. tests/lib.sh did not conflict; both sets of helpers are present. Two conflicts were resolved keeping BOTH changes: tests/fm-remote-job.test.sh keeps PR 28's worker teardown in full and ends on fm_test_rmtree instead of raw rm -rf, and tests/fm-send-remote-delivery.test.sh keeps the upstream-added pending-reply-lib source plus the load guard.

FIRSTMATE REPO CONSTRAINTS: this repo has real working GitHub Actions (13 checks) and --skip=ci must NOT be passed; the checks must actually run and go green, which is the whole point for a change to a library every test loads. Push to origin only, never upstream. Never add an agent name as a commit co-author. One full sentence per line in tracked Markdown, plain dash never an em dash. bin/.sh and bin/backends/.sh must pass bin/fm-lint.sh.

What Changed

• Added fm_test_rmtree to tests/lib.sh as the single guarded removal path for a fixture root: it aborts loudly instead of running rm -rf when the target path is unset/empty or falls outside the expected temp root, closing the TMP_ROOT="" → cd "" && pwd -P → $PWD chain that previously let a teardown trap delete a live working directory. Added fm_test_own_fixture as a guarded escape hatch for declaring fixture roots that legitimately live outside the temp root (refusing the repo root, the working directory, or any of its ancestors), and routed fm_test_cleanup through fm_test_rmtree so it now exits 1 if any removal was refused.
• Appended || exit 1 to the relative . "$(dirname "${BASH_SOURCE[0]}")/lib.sh"-style sources across 107+ test files, so a failed source now aborts the script immediately instead of silently continuing with every helper undefined.
• Updated the 16 test files that own their teardown trap (bypassing fm_test_cleanup) to call fm_test_rmtree at their removal call site instead of raw rm -rf, including tests/fm-remote-job.test.sh (keeping its worker teardown) and tests/fm-remote-backlog-handoff.test.sh (keeping its existing 2>/dev/null flake mitigation).
• Declared the two legitimate out-of-temp-root fixtures — tests/fm-lint.test.sh's source-boundary parity fixture and tests/fm-session-start.test.sh's literal /tmp/fm-<id> roots — through fm_test_own_fixture, and expanded tests/fm-test-fixture-cleanup.test.sh with regression and mutation-falsification coverage for fm_test_rmtree, the declaration path, and a sweep that copies all library-dependent test files out of tests/ and asserts each refuses cleanup while leaving its working directory unchanged.

Risk Assessment

✅ Low: The change is tightly scoped to tests/, implements a symlink-safe, containment-correct guard (fm_test_rmtree/fm_test_own_fixture) that verifiably satisfies all three required invariants (abort on empty root, abort on out-of-root path, abort on failed lib.sh load), and is mutation-falsified with real regression coverage. I traced every remaining un-migrated raw rm -rf in tests/ (LAB/SCRATCH e2e fixtures, /tmp/fm-id literals, sub-path removals) and confirmed none can reach the reported incident's collapse mechanism (a cd ""-driven resolution to $PWD), matching the user intent's explicit scope boundary.

Testing

Ran the targeted regression suite (tests/fm-test-fixture-cleanup.test.sh) plus the two dependent files that exercise the new fm_test_own_fixture escape hatch (fm-lint.test.sh, fm-session-start.test.sh); all pass, directly proving the guard blocks the empty-TMP_ROOT, outside-temp-root, and incident-exact unset-root-resolves-to-$PWD deletion paths while still permitting the two legitimate out-of-root fixtures. Cross-checked the full 127-file diff against every specific claim in the stated intent (guard implementation, mechanical edit scope, intentionally unrouted call sites, rebase-merge resolution) and found no discrepancies. Working tree left clean.

Evidence: fm-test-fixture-cleanup.test.sh output — guard refusing each unsafe path and the 125-file outside-tests/ sweep

ok - fm_test_rmtree refuses an empty fixture path
ok - fm_test_rmtree refuses a non-empty path outside the fixture temp root
ok - fm_test_rmtree refuses the working directory an unset TMP_ROOT resolves to
ok - every tests/lib.sh-dependent file refuses to run outside tests/ (125 files)
ok - fm_test_own_fixture permits teardown of a declared root outside the temp root
ok - fm_test_own_fixture refuses the working directory, its ancestors, and the repo root
ok - a fixture rooted outside $TMPDIR is removable only once declared
FM_TEST_END ... tests/fm-test-fixture-cleanup.test.sh exit=0 duration_ms=11448

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.

• bash bin/fm-test-run.sh tests/fm-test-fixture-cleanup.test.sh — 12/12 pass, including new cases that drive fm_test_rmtree directly (empty path, path outside the temp root, and the incident's exact unset-TMP_ROOT-resolves-to-$PWD chain), the fm_test_own_fixture declaration path in both directions, and the 125-file sweep that copies every lib.sh-dependent test file outside tests/ and asserts each refuses with its working directory byte-identical
• bash bin/fm-test-run.sh tests/fm-lint.test.sh tests/fm-session-start.test.sh — both suites fully green (4/4 and 49/49 respectively); these are the two files whose own teardown traps needed the fm_test_own_fixture escape hatch (fm-lint.test.sh's repo-relative $ROOT-based parity fixture, fm-session-start.test.sh's two /tmp/fm-<id> secondmate fixtures) and were the two regressions this branch itself found and fixed during CI
• manual diff review of all 127 changed test files against the stated intent: confirmed the || exit 1 mechanical edit on every relative tests/-local source line, confirmed the 15-16 files with an owned teardown trap were switched from raw rm -rf to fm_test_rmtree, confirmed the two intentionally-unrouted removals (tests/fm-herdr-session-cleanup.test.sh's multi-arg glob cleanup and tests/fm-backend-orca.test.sh's raw rm -rf "/tmp/fm-$id") remain raw as scoped, confirmed the rebase-conflict resolutions in tests/fm-remote-job.test.sh and tests/fm-send-remote-delivery.test.sh, and confirmed the 2>/dev/null preservation in tests/fm-remote-backlog-handoff.test.sh's retry loop
• git status --short before and after the test runs — clean, no stray fixtures or artifacts left in the working tree

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.