feat: add safe secondmate lane restart helper#235
Open
mielyemitchell wants to merge 13 commits into
Open
Conversation
4 tasks
Owner
|
Thanks for the PR! It looks like this branch has a merge conflict with the base branch right now. When you get a chance, could you rebase onto (or merge in) the latest base branch, resolve the conflict, and push? Once GitHub shows the PR as mergeable again, it'll be picked back up for review. Noted for firstmate#235 at |
4 tasks
809d5a7 to
137a228
Compare
Author
|
Rebased onto current main — mergeable now at 137a228 (workflows just need maintainer approval to run). Thanks for the nudge! |
4 tasks
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
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
Add a one-command secondmate lane context-refresh helper at bin/fm-restart.sh . It must only accept existing kind=secondmate state records, refuse disposable crewmates, ask the lane to stow durable context through the normal marked fm-send secondmate path, then require a bounded lane status-line signal confirming stow completed before any restart. If that stow completion signal does not arrive, abort clearly without sending raw exit or respawning. Provide --skip-stow as the explicit escape hatch for lanes already stowed or dead. After stow completion or --skip-stow, send the harness exit command itself directly and unmarked to the resolved raw backend endpoint so /exit or /quit is handled by the harness rather than converted into chat. Verify the harness process/pane is actually gone using backend process/current-state primitives, never chat acknowledgement. If the lane is already dead it should skip nudge/wait and respawn cleanly. On herdr it must verify the recorded pane still belongs to the expected fm- tab before control actions, avoid last-tab refusal by renaming the old tab, respawn through bin/fm-spawn.sh --secondmate, then close the old tab with cleanup verification. On tmux it should kill the old window after exit and respawn through fm-spawn. --force should interrupt/exit with the harness-appropriate sequence after stow completion if the lane does not exit in time. Keep bash 3.2 compatibility, use fm-backend.sh primitives instead of raw tmux/herdr when primitives exist, update AGENTS.md briefly, and add focused bin tests for live, dead, force, refusal, required stow completion, --skip-stow, raw-exit routing, and backend cleanup behavior.
What Changed
bin/fm-restart.shto safely restart registered secondmate lanes, with stow gating,--skip-stow,--force, dead-lane handling, and raw harness exit routing.fm-spawn.sh, and clean up old backend surfaces.Risk Assessment
Testing
Baseline full tests had already passed; I then ran the focused restart, herdr backend, and session-start tests, plus a CLI-level fake tmux lane transcript showing stow confirmation before raw
/quit, abort before exit/respawn when stow is missing, and clean respawn when the lane is already dead. No UI screenshot was applicable because this is a shell/backend lane-control change.Evidence: Targeted shell test log
Evidence: fm-restart end-to-end behavior transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
/Users/m_mit/.no-mistakes/worktrees/9a08938f86b4/01KWNJ25XZATCMCWW2A40FSHK8/bin/fm-restart.sh:251-fm-restart.shtreats a live endpoint with an unreadable or unrecognized foreground process as safe to restart without stow.harness_runningcollapses process-read failures, label mismatches, and unknown harness values into the same false result, and the main branch then skips the stow request and goes straight to cleanup/respawn. That violates the restart safety contract for any live secondmate whose process primitive temporarily fails or whose harness value is missing/unrecognized: it can kill a running lane without the requiredstowed: restart-readystatus. Split this into explicit states, e.g.running,not-running, andunknown, and abort onunknownunless--skip-stowis provided.🔧 Fix: Captain, guard uncertain restart process state
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline already completed successfully before this run:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-restart.test.shbash tests/fm-backend-herdr.test.shbash tests/fm-session-start.test.shCreated and ran/var/folders/ql/vwkrq94j4mq90s39twg74z7c0000gn/T/no-mistakes-evidence/01KWNJ25XZATCMCWW2A40FSHK8/fm-restart-e2e.sh, a fake tmux-backed secondmate lane exercise that recorded successful stow-gated restart, missing-stow abort, and dead-lane respawn behavior.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.