Repository navigation
fix: prevent orphaned processes in timeout fallbacks and test fixtures - #47
Merged
Merged
Conversation
…rphans A fake no-mistakes that loops until killed was left spinning as an orphan (parent pid 1, own process group) whenever its test failed or was interrupted; many suites in parallel turned that into a host-wide CPU storm. Root cause in production: the perl hard bound made the child's process group only on the child side. A child slow to be scheduled on a loaded host had not yet created its group when a short bound fired, so TERM and KILL reached nothing and the command ran on, orphaned. Reproduced deterministically by delaying only the child's setpgrp(0, 0) past the bound. - bin/fm-timeout-lib.sh: one shared fm_timeout_perl_bound. Parent and child both call setpgrp, and TERM, INT, or HUP aimed at the bounding process now stops the whole group (exit 128+n) instead of stranding it. - bin/fm-nm-run-lib.sh: fm_nm_bounded's perl arm calls the shared bound instead of its own drifted copy (which also reported a signal death as 0). - bin/fm-watch.sh: same parent-side setpgrp in the check runner's perl arm. - tests/lib.sh: fm_test_track_process / fm_test_process_alive and a reap that kills a tracked stub (pid plus needle match, so a recycled pid is safe); fm_test_cleanup also takes down the test shell's own background jobs and continues a SIGSTOPped watcher before stopping it. - tests/fm-crew-state.test.sh: the fake no-mistakes is bounded by the suite's stub ceiling, tracked for reaping, and the test fails if the bound leaves it running. - tests/fm-timeout-lib.test.sh: regression tests for the slow-to-start child on both perl arms and for TERM forwarding; blocking stubs capped at 25s. - Sweep of other tests: unbounded wait loops and long sleeps in fixtures bounded by FM_TEST_STUB_MAX_BLOCK_SECONDS, and cleanup added where a file replaces the shared trap or its stub is not a job of the test shell.
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.
Intent
when agents time out we need to understand why they timed out
and fix the root cause
When looking at the timeouts and harness issues, assess individual fixes, but also assess as a system with dependencies. We often fix one thing to break another. I want to fix without breaking. I want to fix without needing to fix again.
firstmate, why have you routed around problems vs commissioning their fix? when you route around problems, they become bigger problems that hurt the project
Context (verified 2026-10-07 ~20:25Z): host load average 251-482 on 18 cores; the top CPU users were 120 orphaned processes
/bin/sh -c while :; do :; done(parent pid 1, one process group, ~6 cores). Their source is tests/fm-crew-state.test.sh around lines 3101-3121, the no-timeout case: a fake no-mistakes binary that loops forever, run with FM_CREW_STATE_NM_TIMEOUT=1 and a toolbin with notimeoutcommand. Each run of that test leaves the looping process behind; many workers and pipeline test steps run the suite in parallel. The overload is a leading suspect for today's agent, daemon and drain timeouts. Main killed the 120 orphans by hand.What Changed
Risk Assessment
✅ Low: The changes address the process-group startup race and nested timeout cleanup, bound blocking fixtures, and preserve identity-scoped teardown without a substantiated correctness, security, or intent-conformance defect.
Testing
Targeted timeout, watcher-check, and fixture-cleanup checks passed after resolving the Bash 3.2 prerequisite and disposable-driver setup issues. Live CLI/process scenarios demonstrated bounded execution, no surviving commands, preserved identity guards, and graceful watcher subtree cleanup. Evidence includes crew-state output, a failing-before/passing-after orphan reproduction, process-tree observations, and a final no-survivor audit. All disposable workspace material was removed; no full suite, static checks, other pipeline phases, or live fleet sessions were exercised.
Evidence: Consolidated live validation observations and cleanup
Source: Consolidated live validation observations and cleanup
Evidence: Crew-state CLI output after the dependency deadline
Source: Crew-state CLI output after the dependency deadline
Evidence: Delayed-launch orphan reproduction before and after
Source: Delayed-launch orphan reproduction before and after
Evidence: Real watcher and check-subtree teardown
Source: Real watcher and check-subtree teardown
Evidence: Adversarial process-identity guard observations
Source: Adversarial process-identity guard observations
Evidence: Final scenario-owned process audit
Source: Final scenario-owned process audit
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
tests/lib.sh:309- Cleanup now kills process owners before their existing subtree cleanup can run. For example, if fm-watcher-lock's slow-check case fails its freshness assertion while the check is running, fm_test_reap_jobs KILLs the watcher's direct child (the isolated check controller) and then the watcher. The check script is a grandchild in the controller's separate process group, so neither signal reaches it; killing the watcher also bypasses watcher_cleanup's group reap. The later fm_test_reap_watchers cannot stop an already-dead owner, leaving the check polling until its 120-second ceiling despite fixture removal. Relevant changed sites: tests/lib.sh:310 (owner KILL), tests/lib.sh:316-318 (forced job cleanup precedes watcher cleanup), tests/fm-watcher-lock.test.sh:286 (surviving polling fixture), and tests/fm-watch-triage.test.sh:881 (equivalent fixture on interruption). Stop registered watchers through their existing graceful cleanup before forced job reaping, preserving their opportunity to terminate owned check groups.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-timeout-lib.test.shinitially stopped at the existing BASHPID probe under macOS Bash 3.2.Downloaded and built Bash 5.3 exclusively inside the disposable worktree using./configure --without-bash-malloc --disable-nls --prefix=<workspace-local-prefix> && make -j4; rerantests/fm-timeout-lib.test.shsuccessfully.Selectedtest_no_timeout_uses_perl_boundfromtests/fm-crew-state.test.sh, captured the real crew-state CLI output, and checked that the stalled dependency process no longer existed.FM_TEST_ONLY=test_check_timeout_configuration_and_launch_allowance <workspace-Bash-5.3> tests/fm-watcher-lock.test.sh.<workspace-Bash-5.3> tests/fm-test-fixture-cleanup.test.sh, with TMPDIR confined to the worktree and the orphan-sweep suppression removed for this check.python3 .live-timeout-validation/live_process_scenarios.py <workspace-Bash-5.3>exercised stream/status preservation, twelve concurrent deadlines, watchdog TERM, and direct-owner SIGKILL. Corrected disposable-driver signal targeting and stale PID-file setup before the successful run.python3 .live-timeout-validation/live_watcher_cleanup.py <workspace-Bash-5.3>exercised normal exit, SIGTERM, and stopped-watcher SIGTERM against real watchers in marked disposable homes on a worktree-private tmux socket.Executed base and target timeout libraries with a 1.5-second process-group creation delay against a 1-second deadline; observed and explicitly reaped the base revision's orphan.Executed a real nested 1-second outer/30-second inner bound with a TERM-resistant command; observed captured stdout closing promptly and the child disappearing.Sent SIGINT and SIGHUP directly to real watchdog processes; observed statuses 130 and 129 and no surviving commands.Executed cleanup with stale birth identities, mismatched command needles, a matching owned process group, and an unrelated stopped process behind a stale watcher lock.Audited published scenario PIDs, stopped the private tmux servers, and removed all disposable lab homes, drivers, downloads, build outputs, and test data.✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.
Root cause
The leak had two layers.
The test layer: the no-timeout case in
tests/fm-crew-state.test.shran a fakeno-mistakesthat spun forever, and nothing guaranteed the child died when the test ended.The production layer: the shared Perl time bound made the command's process group only on the child side.
A child scheduled late escaped the bound, so the deadline killed a group that did not exist yet and the command lived on under pid 1.
The live fleet uses GNU
timeout, so the live leak was the test layer; the Perl layer is the same orphan class on any host withouttimeout.System check
What calls the bound:
fm_nm_bounded(bin/fm-nm-run-lib.sh) is called frombin/fm-crew-state.sh,bin/fm-teardown.shandbin/fm-dod-lib.sh.fm_run_timed(bin/fm-timeout-lib.sh) is called from manybin/scripts, for examplefm-fleet-snapshot.sh,fm-session-start.sh,fm-send.sh,fm-startup-network.shandfm-tool-update-check.sh.fm-watch.shrun_check_processhad the same single-sided group race and gets the same parent-side line.What changed in the shared code:
fm_timeout_perl_boundinbin/fm-timeout-lib.sh.fm_run_timedandfm_nm_boundedboth use it, so the two drifted Perl copies are gone.fm_exec_timedalready did.fm_nm_boundedkeeps reporting signal death as128 + signal, which its private copy had lost.Blast radius:
timeoutorgtimeoutnever reach the Perl arm, so their behavior is unchanged.bin/fm-nm-run-lib.shnow sourcesfm-timeout-lib.shwhen the function is missing. Callers that already source it are unaffected.Tests that prove nothing broke:
tests/fm-crew-state.test.shpasses in full (283 ok), including the rewritten no-timeout case that now asserts the fake is dead.tests/fm-timeout-lib.test.shgains tests for a child slow to start in both entry points, TERM forwarding, and a nested bound where the outer deadline fires first. The delayed-group test fails on the old code and passes on the new.bin/fm-lint.shis clean, and all 21 CI checks pass.fm-timeout-libtests and onefm-watcher-lockcheck fail on the base commit as well. They are not caused by this change.Test sweep
The same pattern (fakes or loops that outlive the test) was swept across
tests/.Fixed in this PR, mostly by bounding wait loops, shortening sleeps and registering cleanup:
fm-afk-contract,fm-afk-launch,fm-backend-herdr-focus-flash-e2e,fm-backend-herdr-presentation-e2e,fm-backlog-atomicity,fm-backlog-handoff,fm-backlog-read-bound,fm-bearings-snapshot,fm-bootstrap,fm-branch-supervision,fm-captain-hold-lifecycle,fm-claude-stop-autoarm,fm-control-relaunch,fm-cursor-primary,fm-remote-secondmate-lifecycle-e2e,fm-remote-secondmate-parent-binding,fm-secondmate-reconcile,fm-secondmate-safety,fm-send-remote-delivery,fm-session-lock-ancestry,fm-session-start,fm-sessionstart-nudge,fm-startup-network,fm-stow-cascade,fm-teardown-endpoint-safety,fm-teardown,fm-test-run,fm-turnend-foreign-owner-repro.py,fm-turnend-guard,fm-wake-queue,fm-watch-triage,fm-watcher-lock, plustests/lib.shandtests/fm-timeout-lib.test.sh.tests/lib.shnow tracks and reaps fixture processes with an identity check, and stops watchers gracefully before it reaps jobs.Not fixed here, as follow-up:
fm-extension-bindingonward) produced a raw list of background jobs and loops that was not triaged or changed in this PR.bin/fm-startup-network.shcan leave an orphan worker when its parent dies; that is production code and is left for its own change.