Skip to content

fix: bind task custody and delivery to immutable revisions - #1901

Open
Vladykart wants to merge 6 commits into
kunchenguid:mainfrom
Vladykart:fm/fm-worktree-revision-binding-p1
Open

fix: bind task custody and delivery to immutable revisions#1901
Vladykart wants to merge 6 commits into
kunchenguid:mainfrom
Vladykart:fm/fm-worktree-revision-binding-p1

Conversation

@Vladykart

Copy link
Copy Markdown

Intent

Harden Firstmate's existing task lifecycle so one authoritative custody check prevents multiple tasks or delivery processes from owning the same isolated worktree while still allowing safe same-task recovery. Bind review, readiness, approval, PR registration, and merge operations to one exact immutable Git revision and refuse moved, missing, malformed, red, or mismatched heads rather than silently rebinding. Cover GitHub and GitLab PR monitoring, guarded local-only delivery, crash recovery, pooled-copy reuse, malformed durable records, and every supported runtime backend. Add executable public-interface regression coverage and authoritative operator and maintainer documentation. Validate and publish committed head 9bee9d0 from branch fm/fm-worktree-revision-binding-p1 as a green pull request, but do not merge it.

What Changed

  • Add authoritative worktree custody validation to prevent cross-task ownership conflicts while permitting recovery of the same task’s recorded copy when its backend endpoint is definitively dead or missing.
  • Bind review, PR readiness, polling, approval, and local or forge merges to one immutable commit, refusing malformed records, moved heads, mismatched revisions, and non-green GitHub checks across GitHub and GitLab workflows.
  • Document the lifecycle identity contract and add regression coverage for custody conflicts, backend recovery, revision movement, guarded local delivery, and PR merge security.

Risk Assessment

✅ Low: The requested fix removes only the redundant forge lookup while preserving the single publication-bound validation and actionable failure diagnostic.

Testing

Startup baseline completed; focused automated regressions passed for authoritative custody, same-task recovery, malformed records, GitHub/GitLab monitoring, crash recovery, guarded local delivery, teardown, documentation, and tmux/Herdr/Zellij/Orca/cmux backends. The captured end-to-end CLI transcript demonstrates duplicate-custody and moved-head refusals followed by an exact freshly bound merge. No visual artifact was applicable because the changed end-user surfaces are shell CLIs, so reviewer-visible evidence is the direct CLI transcript.

Evidence: End-to-end local revision lifecycle CLI transcript
$ fm-merge-local.sh demo --prepare
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no watcher has a fresh beacon (last beat: never, grace 300s).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision with a foreground checkpoint: bin/fm-watch-checkpoint.sh --seconds 180.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
prepared task demo for review at ed1ccf240b5186b4fb698fc5865dbc89d9ab6bca
BOUND_HEAD=ed1ccf240b5186b4fb698fc5865dbc89d9ab6bca

$ fm-review-diff.sh demo --stat
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
diff base: main
diff head: ed1ccf240b5186b4fb698fc5865dbc89d9ab6bca
 file.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

$ fm-merge-local.sh other --prepare  # same worktree, different task
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
REFUSED: isolated copy /tmp/no-mistakes-evidence/01KZEDXBJB3VPSYC6BSXY62SFY/local-revision-lifecycle-fixture-3104447/worktree is still owned by task demo; finish or safely clean up that task before reuse.
exit=1

BRANCH_MOVED_TO=d4f7cdac1104c52136d078b6575dcf40dcf904bc
$ fm-review-diff.sh demo --stat  # stale readiness binding
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
REFUSED: local branch moved after readiness was registered at ed1ccf240b5186b4fb698fc5865dbc89d9ab6bca.
Run bin/fm-merge-local.sh demo --prepare before reviewing or approving the new revision.
exit=1
$ fm-merge-local.sh demo  # stale approval cannot merge
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
REFUSED: fm/demo moved after review from ed1ccf240b5186b4fb698fc5865dbc89d9ab6bca to d4f7cdac1104c52136d078b6575dcf40dcf904bc.
Run bin/fm-merge-local.sh demo --prepare, review the new revision, and obtain fresh merge approval.
exit=1

$ fm-merge-local.sh demo --prepare  # bind the new revision
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
prepared task demo for review at d4f7cdac1104c52136d078b6575dcf40dcf904bc
NEW_BOUND_HEAD=d4f7cdac1104c52136d078b6575dcf40dcf904bc
$ fm-review-diff.sh demo --stat
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
diff base: main
diff head: d4f7cdac1104c52136d078b6575dcf40dcf904bc
 file.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
$ fm-merge-local.sh demo
WARNING: watcher still down (same stale episode; last beat: never, grace 300s) - full banner already printed this episode.
merged approved revision d4f7cdac1104c52136d078b6575dcf40dcf904bc from fm/demo into local main (d41615d -> d4f7cda) in /tmp/no-mistakes-evidence/01KZEDXBJB3VPSYC6BSXY62SFY/local-revision-lifecycle-fixture-3104447/project
LANDED_HEAD=d4f7cdac1104c52136d078b6575dcf40dcf904bc
RESULT: landed HEAD exactly matches the freshly reviewed immutable binding

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • ⚠️ bin/fm-pr-check.sh:69 - The forge head is fetched twice consecutively before publication. This doubles API traffic and creates an unnecessary failure/rate-limit window where the first lookup succeeds but the identical second lookup aborts registration. Remove the first lookup block and retain one lookup at the publication boundary.

🔧 Fix: Remove redundant forge head lookup
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bin/fm-session-start.sh
  • bash tests/fm-backend-orca.test.sh tests/fm-merge-local.test.sh tests/fm-pr-check-security.test.sh tests/fm-pr-merge.test.sh tests/fm-review-diff.test.sh tests/fm-spawn-dispatch-profile.test.sh tests/fm-spawn-worktree-settle.test.sh (executed sequentially)
  • bash tests/fm-documentation-audiences.test.sh tests/fm-task-delivery.test.sh tests/fm-teardown.test.sh tests/fm-teardown-endpoint-safety.test.sh (executed sequentially)
  • bash tests/fm-backend.test.sh tests/fm-backend-herdr.test.sh tests/fm-backend-zellij.test.sh tests/fm-backend-cmux.test.sh tests/fm-backend-tmux-smoke.test.sh (executed sequentially)
  • Manual CLI lifecycle: prepare and review an immutable local revision; attempt conflicting second-task custody; move the branch; verify stale review and merge refusal; re-prepare, review, merge, and compare landed HEAD with ready_head
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant