Repository navigation
fix: retire task-private no-mistakes agents during teardown - #60
Merged
Merged
Conversation
A worker whose NO_MISTAKES_HOME sits inside its task copy makes no-mistakes install a KeepAlive launchd agent. Its daemon holds a directory inside the copy, so teardown refused it as unproven custody, and a plain kill is undone by launchd. Teardown now boots out and archives any no-mistakes daemon launch agent whose --root is the task copy or inside it, and leaves every other agent alone. The ship and scout briefs now tell workers to stop private services they start.
…ervice lifecycle contracts
…py teardown recovery
…n and owner pointers
…hold fixtures now initialize their synthetic project registry at the shared boundary, covering interrupted cleanup and missing-copy siblings; all five interruption cases assert the intended worktree-return failure. The Herdr offline cleanup harness now loads the local-state collector and wake-library dependencies. Updated fixture documentation; production behavior is unchanged. Verification: all eight focused lifecycle cases passed, the real-process offline inventory/reaper smoke passed, and shell syntax checks passed. Broader verification was limited: the full captain suite hit a tasks-axi read timeout, and the remaining live Herdr scenarios could not satisfy macOS socket-path constraints within this worktree. Disposable services were stopped and scratch files removed
…ry-lock test’s shared dependencies once in the parent shell and removing the duplicate wake import. Offline reaping remains isolated; production behavior and PID-capture paths are unchanged. Updated fixture documentation. Verification passed: exact full source-aware lint with pinned ShellCheck 0.11.0, shell syntax check, and executable smoke covering owned-process removal, unrelated-process survival, later PID capture, background exit statuses, and parent lock ownership. Full Herdr E2E was not run: its lab-home helper rejects in-worktree homes, and this phase forbids intentional writes outside the worktree. Throwaway verification files were removed
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
I would like all bugs to be fixed so tomorrow can be focused entirely to Vernant and not problems preventning Vernant from getting built.
A recommended architecture should come with proof.
Context (the backlog record for this bug, as filed):
Cleanup refuses: task-private no-mistakes daemon (daemon run --root /.no-mistakes/h) left running 10h (repo: firstmate) (kind: ship) (priority: 1) (since 2026-10-06)
Seen 2026-10-06/07 on fm-nomistakes-push-fails-vernant-prepush-hooks. Root cause: a worker's private no-mistakes home (NO_MISTAKES_HOME inside its task copy, authorized by Main 2026-10-01 for an isolated fixture) auto-installed a launchd agent com.kunchenguid.no-mistakes.daemon. with KeepAlive=true. It ran 6 days, survived a reboot, and blocked teardown (process holds a cwd in the copy, custody unproven). kill alone fails (launchd respawns); NO_MISTAKES_HOME= daemon stop targets the SHARED daemon (must never be used). Main stopped it by hand 2026-10-07: verify plist ProgramArguments --root is under the task copy, launchctl bootout gui//, move the plist to data//launchagent-backup/.
Fix (ship): 1) fm-teardown recognizes a no-mistakes LaunchAgent whose --root resolves under the task's own copy, boots it out, archives the plist to data//, then proceeds; never touches an agent whose root is elsewhere (shared ~/.no-mistakes). 2) Worker brief rule: a worker that starts any private service (private no-mistakes home included) stops it before parking or finishing and names it in its status line. 3) Tests with a fake launchctl.
What Changed
data/<task>/launchagent-backup/. Resolve roots and check nested-worktree registries, including missing-copy recovery, to preserve shared and foreign agents; refuse teardown when safe retirement cannot be completed.no-mistakes daemon stopfor private-agent cleanup.launchctlteardown coverage for private, foreign, nested, unloaded, and failed-unload agents, plus brief-rule tests and script documentation updates.Risk Assessment
✅ Low: The change is bounded to task-private agent retirement and worker lifecycle instructions, with source-backed ownership exclusions and fail-closed unload checks; no material defects or intent contradictions were substantiated.
Testing
Seven live CLI scenarios and the focused fake-launchctl regressions passed. Disposable setup issues and cleanup interference were corrected before final validation. Evidence includes generated briefs and real CLI/launchd transcripts; launchd checks used authorized sleep agents with restricted label mapping, and bootout unavailability was fault-injected. All authorized agents and transient fixtures were removed.
Evidence: Completed live teardown and launchd transcript, including mapping, fault injection, and cleanup proof
Source: Completed live teardown and launchd transcript, including mapping, fault injection, and cleanup proof
Evidence: Generated scout brief using the configured awaiting parking state
Source: Generated scout brief using the configured awaiting parking state
Evidence: Completed scenario outcomes and successful cleanup record
Source: Completed scenario outcomes and successful cleanup record
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 2 issues found → auto-fixed ✅
bin/fm-brief.sh- merge conflict rebasing onto origin/mainbin/fm-teardown.sh- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 4 issues found → auto-fixed (3) ✅
bin/fm-teardown.sh:2519- The prefix check authorizes retiring another lane's daemon when its linked worktree is nested beneath this task's copy. For example, with a registered lane at <WT>/other-lane and its daemon rooted at <WT>/other-lane/.no-mistakes/h, this check passes, bootout kills its service, and the plist is archived before the existing process-custody gate can protect it. Nested lanes are explicitly protected by task_nested_lane_for_path and the existing nested-lane fixtures. Reuse that registry/discovery boundary before authorizing retirement, including registered lanes with missing directories or .git files and lanes registered by sibling clones. Related changed sites: bin/fm-teardown.sh:2536 performs the unauthorized bootout; bin/fm-teardown.sh:2548 moves the foreign plist; bin/fm-teardown.sh:3812 runs both before the custody-aware reap.bin/fm-teardown.sh:2517- The required criterion is to recognize an agent whose '--root resolves under the task's own copy', but the added hunkcase "$root/" in */../*|*/./*|*//*) continue ;; esacrejects valid equivalent paths before resolving them. A plist rooted at <WT>/./.no-mistakes/h or <WT>//.no-mistakes/h is silently skipped even though it resolves inside the copy; a running daemon then reaches the existing descendant-cwd refusal and the reported leak remains. The same exclusion is documented at bin/fm-teardown.sh:304. Replace this blanket rejection with safe canonicalization followed by containment checking, still leaving paths that resolve outside the copy untouched. Do not merely remove the guard: task_canonical_path preserves missing tail components verbatim.bin/fm-brief.sh:615- The new rule hardcodespausedinstead of using the supported parking vocabulary. With FM_CLASSIFY_PAUSED_VERB=awaiting, the emitted brief instructs workers to park usingawaiting, but its private-service cleanup prerequisite listspaused, not that actual state. Render the configured PAUSED_VERB in this shared rule while preserving the literal launchctl command. Both changed consumers need the corrected contract: bin/fm-brief.sh:662 emits it for scouts and bin/fm-brief.sh:743 emits it for ships.tests/fm-teardown.test.sh:5652- The assertion claiming that teardown never stops the shared daemon checks only the launchctl call log. A forbiddenno-mistakes daemon stopinvocation cannot appear in that log, and the existing fake no-mistakes executable silently succeeds for unrecognized commands. Consequently, this safety assertion would still pass if the forbidden operation were introduced. Related changed sites: tests/fm-teardown.test.sh:5565 records only launchctl calls, and tests/fm-teardown.test.sh:5643 reads that log. Make the fake no-mistakes stop command change an observable simulated shared-daemon state, then assert that state remains unchanged.🔧 Fix applied.
3 issues (1 error, 2 warnings) still open:
bin/fm-teardown.sh:2519- The prefix check authorizes retiring another lane's daemon when its linked worktree is nested beneath this task's copy. For example, with a registered lane at <WT>/other-lane and its daemon rooted at <WT>/other-lane/.no-mistakes/h, this check passes, bootout kills its service, and the plist is archived before the existing process-custody gate can protect it. Nested lanes are explicitly protected by task_nested_lane_for_path and the existing nested-lane fixtures. Reuse that registry/discovery boundary before authorizing retirement, including registered lanes with missing directories or .git files and lanes registered by sibling clones. Related changed sites: bin/fm-teardown.sh:2536 performs the unauthorized bootout; bin/fm-teardown.sh:2548 moves the foreign plist; bin/fm-teardown.sh:3812 runs both before the custody-aware reap.bin/fm-brief.sh:615- The new rule hardcodespausedinstead of using the supported parking vocabulary. With FM_CLASSIFY_PAUSED_VERB=awaiting, the emitted brief instructs workers to park usingawaiting, but its private-service cleanup prerequisite listspaused, not that actual state. Render the configured PAUSED_VERB in this shared rule while preserving the literal launchctl command. Both changed consumers need the corrected contract: bin/fm-brief.sh:662 emits it for scouts and bin/fm-brief.sh:743 emits it for ships.tests/fm-teardown.test.sh:5652- The assertion claiming that teardown never stops the shared daemon checks only the launchctl call log. A forbiddenno-mistakes daemon stopinvocation cannot appear in that log, and the existing fake no-mistakes executable silently succeeds for unrecognized commands. Consequently, this safety assertion would still pass if the forbidden operation were introduced. Related changed sites: tests/fm-teardown.test.sh:5565 records only launchctl calls, and tests/fm-teardown.test.sh:5643 reads that log. Make the fake no-mistakes stop command change an observable simulated shared-daemon state, then assert that state remains unchanged.🔧 Fix applied.
2 errors still open:
bin/fm-teardown.sh:2519- The prefix check authorizes retiring another lane's daemon when its linked worktree is nested beneath this task's copy. For example, with a registered lane at <WT>/other-lane and its daemon rooted at <WT>/other-lane/.no-mistakes/h, this check passes, bootout kills its service, and the plist is archived before the existing process-custody gate can protect it. Nested lanes are explicitly protected by task_nested_lane_for_path and the existing nested-lane fixtures. Reuse that registry/discovery boundary before authorizing retirement, including registered lanes with missing directories or .git files and lanes registered by sibling clones. Related changed sites: bin/fm-teardown.sh:2536 performs the unauthorized bootout; bin/fm-teardown.sh:2548 moves the foreign plist; bin/fm-teardown.sh:3812 runs both before the custody-aware reap.bin/fm-teardown.sh:2550- Round 2's nested-lane fix introduces a refusal when the task copy is already absent. Concrete sequence: a private agent's plist remains installed, the copy is removed, and teardown resumes with a confirmed merged PR or an authorized forced discard. Both --root=<WT> and --root=<WT>/.no-mistakes/h pass canonical containment, but task_nested_lane_for_path walks to the nonexistent <WT> and returns failure at line 2215. Teardown consequently refuses before unloading or archiving the agent, even with --force. This regresses the supported absent-copy recovery path demonstrated by test_ship_without_git_copy_completes_with_recorded_merged_pr. Related changed sites: bin/fm-teardown.sh:2513 invokes registry discovery, whose line 2248 skips project registries when the copy is absent; bin/fm-teardown.sh:3847 invokes retirement for absent copies too. Make the shared ownership-classification boundary support missing scan roots using the recorded project and available task-project registries, preserving nested-lane exclusions rather than bypassing the check.🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Test** - 2 issues found → no changes applied ✅
bin/fm-teardown.sh- This step cannot demonstrate the central loaded-KeepAlive guarantee against real launchd. Isolated live CLI scenarios and the requested fake-launchctl regressions passed, but actual bootout, respawn prevention, and loaded foreign-service preservation remain unverified. Creating the required disposable agent would change the GUI launchd domain, which the workspace boundary forbids. Authorize narrowly scoped disposable launchd services, or rerun in a dedicated macOS user/VM where bootstrap and bootout are permitted; production services and credentials need not be touched.git diff fd9b8b020201a029880b3980b545f352ae357a39 529eeee1c887216b5e7ecd930e8d584598543226 --statand the implementation diff forbin/fm-teardown.shandbin/fm-brief.sh.launchctl print gui/$(id -u)/com.kunchenguid.no-mistakes.daemon.fm-validation-uninstalled-01M4EY8MVGAEEDGBGYD6HZR4HGreturned service-not-found exit 113./bin/bash tests/fm-teardown.test.sh test_private_nm_launch_agent_is_retired_before_teardown test_private_nm_launch_agent_equivalent_roots_are_retired test_foreign_nm_launch_agents_are_left_alone test_nested_nm_launch_agents_are_left_alone test_absent_copy_nm_launch_agents_preserve_nested_ownership test_private_nm_launch_agent_bootout_failure_refuses test_private_nm_launch_agent_not_loaded_is_archivedcompleted with exit 0.An initial regression invocation additionally selected absent-copy and custody baseline cases; its capture deadline expired, so no passing result is claimed for that attempt. Its orphaned driver was terminated and cleaned up.python3 .fm-validation/live-scenarios.pydrove realfm-lab-home.sh, Treehouse, private-socket tmux, launchctl absence checks, andfm-teardown.sh: equivalent-root archival, outside-root preservation, absent-copy recovery with sibling-clone nested ownership, and forced archive-failure refusal.Corrected two disposable setup failures before successful live validation: an overlong tmux socket path usingfm-lab-home.sh tmux-dir, and missing scout completion inventory usingfm-captain-hold.sh complete <id> --none.Realfm-brief.shgeneration covered all three ship modes and scouts withFM_CLASSIFY_PAUSED_VERB=pausedandawaiting, plus a secondmate charter; generated public outputs were saved as evidence.Stopped each private tmux server, removed its socket directory and lab, terminated the interrupted test driver, and removed.fm-validation; final checks found no remaining scratch directory or evidence lab.🔧 No changes applied.
✅ Re-checked - no issues remain.
env TMPDIR="$PWD/.live-validation/tmp" bash tests/fm-teardown.test.sh test_private_nm_launch_agent_is_retired_before_teardown test_private_nm_launch_agent_equivalent_roots_are_retired test_foreign_nm_launch_agents_are_left_alone test_nested_nm_launch_agents_are_left_alone test_absent_copy_nm_launch_agents_preserve_nested_ownership test_private_nm_launch_agent_bootout_failure_refuses test_private_nm_launch_agent_not_loaded_is_archivedenv TMPDIR="$PWD/.live-validation/tmp" bash tests/.live-teardown-diagnostic.test.sh test_foreign_nm_launch_agents_are_left_alone— temporary diagnostic copy preserved observable fixture state; passed.env TMPDIR="$PWD/.live-validation/tmp" bash tests/fm-teardown.test.sh test_nested_nm_launch_agents_are_left_alone test_absent_copy_nm_launch_agents_preserve_nested_ownership test_private_nm_launch_agent_bootout_failure_refuses test_private_nm_launch_agent_not_loaded_is_archivedenv TMPDIR="$PWD/.live-validation/tmp" bash tests/fm-teardown.test.sh test_absent_copy_nm_launch_agents_preserve_nested_ownership test_private_nm_launch_agent_bootout_failure_refuses test_private_nm_launch_agent_not_loaded_is_archived— passed after the live driver stopped interfering with concurrent fixture cleanup.python3 .live-validation/drive.py— drove the real teardown CLI with disposable Treehouse pools, marked lab homes, private tmux sockets, and authorized real launchd sleep agents; corrected setup attempts and the completed run are recorded in evidence.bin/fm-brief.sh <id> alpha --mode <no-mistakes|direct-PR|local-only>andbin/fm-brief.sh <id> alpha --scout, with bothFM_CLASSIFY_PAUSED_VERB=pausedandawaiting; inspected the generated public briefs.launchctl bootstrap gui/$(id -u) <scratch-plist>, authorized-labellaunchctl printandbootout, andlaunchctl list | grep com.firstmate.lab; verified KeepAlive respawn, service retirement, preservation, and final absence.Removed temporary validation scripts, copied runtime, pools, fixtures, lab homes, agent plists, and private tmux servers; confirmed temporary worktree paths were absent.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.