feat(bin): recover relaunch through a rebuilt pane for gone Herdr endpoints - #2770
feat(bin): recover relaunch through a rebuilt pane for gone Herdr endpoints#2770khaira777 wants to merge 5 commits into
Conversation
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains. Reviews (2): Last reviewed commit: "no-mistakes(document): Document recover-..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35d351725e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # A previous missing-endpoint recovery can leave its replacement pane | ||
| # structurally present but agent-free after the launch fails. The control | ||
| # plane authorizes this path only from that recorded recovery journal; | ||
| # accepting the recovery-grade dead state here is therefore safe and | ||
| # avoids stranding the same task behind an empty Herdr pane. |
There was a problem hiding this comment.
Captain, when fm-spawn.sh <id> --relaunch --recover-missing is invoked directly for any stopped Herdr task, this branch accepts dead solely because the flag was supplied; it never verifies SPAWN_CONTROL_PARENT, a matching relaunch transaction, or the claimed failed-recovery journal. The caller can therefore create and publish a new pane while bypassing fm-control's checkpoint, required progress note, and transactional rollback, even though the comment says the control plane authorized this path. Refuse --recover-missing unless it comes from the lifecycle-control parent with matching recovery provenance.
AGENTS.md reference: AGENTS.md:L302-L305
Useful? React with 👍 / 👎.
| if [ "$BACKEND" = herdr ] \ | ||
| && { [ "$state" = missing ] \ | ||
| || { [ "$state" = dead ] \ | ||
| && [ -f "$JOURNAL" ] \ | ||
| && grep -Fqx 'phase=failed:launching' "$JOURNAL" 2>/dev/null; }; }; then |
There was a problem hiding this comment.
When an ordinary same-endpoint Herdr relaunch publishes its replacement record but the new agent never becomes alive, rollback also leaves a dead pane and phase=failed:launching. This predicate cannot distinguish that ordinary failure from a failed missing-endpoint recovery, so the next fm-control ... relaunch incorrectly passes --recover-missing, creates another pane, and abandons the still-valid agent-free endpoint instead of retrying it. Persist and check an explicit recovery-attempt marker rather than inferring provenance from backend, state, and phase alone.
AGENTS.md reference: AGENTS.md:L302-L305
Useful? React with 👍 / 👎.
|
Speaking as Kun's firstmate: VISION: aligned as a failure-mode recovery. A restart is a non-event; a gone Herdr endpoint should not strand relaunch. The path is fail-closed on live/ambiguous endpoints, scoped to Herdr, and leaves the ordinary tmux same-endpoint retry alone. Class: corrective (default relaunch recovery for that Herdr-missing shape; no enable flag). Defect, waiting on the author: the recovery cwd wait is: bash Security: none. Overlap / do not land together:
CI: first-time fork workflows approved after diff review. Greptile is green. no-mistakes is pending and blocking. Ahead 1, behind 0, mergeable. Waiting on the author for the wait-loop defect, and on CI — not waiting on the captain. Merge-eligible: NO. Captain-flag NOW: NO. |
|
Speaking as Kun's firstmate: Corrective Herdr-only relaunch recovery for an authoritatively gone endpoint ( Not merge-eligible. Require no-mistakes FAILED (body compliance). CI in progress. Overlap with #2760 on Waiting on the author for no-mistakes — not waiting on the captain. |
…points A recorded Herdr endpoint that is authoritatively gone (app restart) stranded relaunch: the ordinary path requires a positively agent-free endpoint to adopt. fm-control relaunch now detects a missing endpoint, or an agent-free one with a failed:launching journal from a prior attempt (Herdr records only, so an ordinary tmux retry keeps its same-endpoint path), skips the stopping step, and delegates to fm-spawn --relaunch --recover-missing. The launch rebuilds exactly one replacement pane in the recorded session/workspace - or the home's flat workspace when that workspace is gone too - reuses the task's existing local copy and records, adopts the rebuilt pane into the durable record, and refuses live or ambiguous endpoints instead of risking duplicate recovery. Ship/scout kinds only; secondmates keep their own recovery.
…rable attempt marker fm-spawn now refuses --recover-missing unless the caller is a live fm-control relaunch child (control lock naming our parent plus the relaunch transaction id) or fm-control's recovery-attempt marker authorizes finishing a failed attempt, so an endpoint rebuild can no longer be requested outside the control plane. fm-control persists that marker before delegating a recovery launch and removes it once the replacement is confirmed alive; the dead-endpoint recovery predicate now requires the marker instead of inferring from the failed:launching journal shape, so an ordinary failed relaunch retry stays on the ordinary same-endpoint path. Also fixes the recovery cwd wait's left-associative ||/|| chain that broke on an empty first path read; it now breaks only on a positive settled-path conjunction.
…and add marker to teardown cleanup
…ndpoint recovery routing on the recovery-attempt marker
35d3517 to
02d1f3a
Compare
2 participants
What
A recorded Herdr endpoint that is authoritatively gone (app restart) used to strand
bin/fm-control.sh <id> relaunch: the ordinary path requires a positively agent-free endpoint to adopt. Relaunch now recovers it instead.bin/fm-control.sh): detects a missing endpoint, or an agent-free one with afailed:launchingjournal from a prior recovery attempt - scoped to Herdr records only, so an ordinary tmux relaunch failure retry keeps its working same-endpoint path unchanged. It preserves the recovery reason before the journal rewrite, skips the ordinary stopping/exit sequence (nothing left to stop), delegates tofm-spawn --relaunch --recover-missing, refuses endpoints that turn live or ambiguous between checks, and adopts the rebuilt pane from the just-published record before its liveness wait and completion journal.bin/fm-spawn.sh):--recover-missingapplies to--relaunchonly and to ship/scout kinds (refused after record resolution for secondmates); accepts only recovery-grade missing-or-dead endpoint states; rebuilds exactly one replacement pane in the recorded session/workspace - or the home's flat workspace when that workspace is gone too - while reusing the task's existing local copy and records; never a fresh spawn, never a second worktree. The rebuilt shell must prove it sits in the recorded worktree before launch.docs/agent-control.md(verb table row, transactional-relaunch steps 4-5, fail-closed boundary) updated minimally.The implementation adapts an ~80%-complete preserved commit (
903a4econfm/local-recover-missing) across later Cursor-harness upstream changes to both target files, preserving both intents; every referenced helper was verified against currentbin/backends/herdr.sh/bin/fm-backend.shcontracts.Validation
tests/fm-control-relaunch.test.sh: new hermetic coverage without a live Herdr daemon - flag refusals (--recover-missingwithout--relaunch, secondmate kind after record resolution), successful missing-endpoint recovery end-to-end on a stateful fake CLI (label reuse, recorded-worktree cwd, record rotation onto the rebuilt pane), ambiguous-workspace refusal keeping the prior durable record, tmux failed-launching retry staying on the ordinary path, live-endpoint refusal. Full suite green across three consecutive runs.tests/fm-control.test.sh,tests/fm-trace-context-spawn.test.sh,tests/fm-spawn-batch.test.sh: green.bin/fm-lint.sh: ShellCheck 0.11.0, actionlint 1.7.12).real-herdr-gatedsuites require a live Herdr daemon and skip identically at base commit in this environment.Delivery note
Shipped direct-PR per captain approval: the local no-mistakes gate refuses its configured pipeline agents in this checkout because neither has a verified AGENTS.md-neutralization knob (error preserved in the task record); GitHub-hosted CI validates this PR instead.