Repository navigation
fix(bin): bind crew-state run attribution to the task incarnation - #6245
AgardnerAU wants to merge 9 commits into
Conversation
A refresh task that reuses an existing PR branch at an unchanged head read the earlier task's finished no-mistakes run as its own result, so fm-crew-state.sh reported "done - run passed: PR open" while the new worker was still resolving conflicts. - fm-spawn.sh records task_started= on a fresh spawn and carries it across relaunches; fm-nm-run-lib.sh owns the incarnation rule (fm_nm_run_in_incarnation, from the ULID run id's creation time, and a creation-minute bound on the runs ledger). A run created before the task started is never attributed, and the ignored run is named on the line that answers instead. - A run record states only what the run did. A merged claim needs the task's recorded pr= identity checked against the forge or a merge receipt, every PR disposition names its source, and a run PR that differs from the recorded one is reported as a conflict. - Run-step wording never presents green: a passed override names its exception, and a ci log reporting no CI checks reads as a wait. - A worker status line written after a finished run started is shown beside the run-step verdict with its source. - A deliberately aborted run names the abort and never reads as a failure.
…h checks-passed output
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| passed-with-skips) RUN_STATE="done"; RUN_DETAIL="$(passed_pr_detail) (publication/CI verification skipped)" ;; | ||
| checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; | ||
| passed|passed-with-override|passed-with-skips) RUN_STATE="done"; RUN_DETAIL=$(passed_run_detail "$outcome") ;; | ||
| checks-passed) RUN_STATE="done"; RUN_DETAIL="run reports CI checks passed (required checks not verified)" ;; |
There was a problem hiding this comment.
If Greptile Review is the only reported passing check while required CI is still held at action_required, a checks-passed outcome still produces state: done. The CI-log “checks passed” path does the same. Although the new wording says required checks were not verified, the task verdict still marks incomplete validation as finished and can produce the false-ready outcome this change aims to prevent.
| if [ "$CI_LOG_STATE" = none-reported ]; then | ||
| emit_ship_status_done "run still monitoring PR; run ci log: no CI checks reported (not green)" |
There was a problem hiding this comment.
When the worker says done: PR … checks green but the run’s CI log says no checks were reported, this branch emits state: done from the worker’s claim. Adding “not green” to the detail does not undo the done verdict or remove the green claim, so a task still waiting for CI can appear finished.
…ute cross the task boundary), as the user instructed. The invariant: a row in the coarse `no-mistakes runs` ledger answers for a task only when its creation minute proves the row was created after the task started. The only exception is the head-anchor row, which proves code identity only. Minutes are compared in one place, `fm_nm_runs_status_for_worktree` in bin/fm-nm-run-lib.sh, and all three coarse-ledger callers in bin/fm-crew-state.sh go through it. At that one place I changed `[[ "$day $clock" < "$start_minute" ]]` to `! [[ "$start_minute" < "$day $clock" ]]`. Now a row created in the task's start minute prints nothing. The anchor-row check runs before this bound, so it stays exempt as before. I updated the function comment to match. Tests: test_coarse_ledger_does_not_inherit_an_earlier_run now also asserts that a completed row in the start minute at the same head does not give `source: run-step`. Without the fix, this assertion failed with `state: done · source: run-step · run completed`. The new test test_coarse_ledger_anchor_row_in_start_minute_still_anchors pins that a head-anchor row in the start minute still anchors an active continuation that follows it (run-step, working). The full tests/fm-crew-state.test.sh suite passes. Shellcheck shows no new findings. I did not change ci-1 or ci-2, because the user declined them. The failing Greptile check is a review bot, and its log was not available
| fi | ||
| break | ||
| fi | ||
| if [ -n "$start_minute" ] && ! [[ "$start_minute" < "$day $clock" ]]; then |
There was a problem hiding this comment.
When a task starts and launches a run within the same minute, the coarse runs ledger gives both the same HH:MM timestamp. This comparison rejects the task’s own run. If the coarse ledger is the only available run identity, crew-state falls back to pane or status evidence instead of reporting the current validation run, potentially hiding its active or terminal result.
Resolve conflicts: route the new Forgejo provider through the PR's pr_disposition reader, and keep both base_branch and task_started in the relaunch-owned metadata keys. Align the upstream Forgejo crew-state test with the PR's source-naming disposition wording.
|
Merged current main into this branch and resolved the conflicts (new Forgejo provider routed through this PR's PR-state reader; relaunch-owned metadata keeps both base_branch and task_started). It is up to date with main and ready for CI; the workflow runs for head e8ca2a5 are waiting for maintainer approval. |
|
Speaking as Kun's firstmate: thank you @AgardnerAU. This is my first triage pass on this PR. I read the whole thread, the Greptile review and the tip diff of What I checked:
The no-mistakes attestation matches Contract-class: new-default. The core fix is close to restoring an existing promise: main already says a historical run on a reused branch must not be attributed. The tip also replaces specified default readings, though. Main's tests had a terminal no-checks ci-monitor run reading done, and that is now a wait. Main reported Open reviewer points for Kun to weigh. These aren't asks:
VISION.md:
Overlaps with your other open PRs: #3701 changes the same run-reading logic in Status: waiting-ci. Once CI and no-mistakes are green on a green main, this goes to Kun to merge or leave. Nothing is needed from you right now. |
…and I fixed it in the test. The failing assertion was "metadata changed beyond Herdr container IDs between opted-out and projected paths" in tests/fm-backend-herdr-presentation-e2e.test.sh. Invariant: when the test compares the meta of two separate spawns, it must mask every field that changes with each spawn. This PR's bin/fm-spawn.sh now writes task_started=<spawn epoch> on every fresh spawn. This value is the same kind of field as spawn_gen=, which normalize_meta already masks. When the two spawns fall in different seconds, the line differs and the comparison fails. Fix: normalize_meta now also replaces ^task_started= with a placeholder. I searched the tests and no other test masks spawn_gen for a meta comparison across spawns, so no other place needs the same change. Verification: the full real-Herdr e2e passed locally on Herdr 0.9.1 (exit 0), including the metadata byte-identity check. CI ran Herdr 0.7.4, so the next CI run will confirm that version. The test's temp-directory cleanup printed some "Directory not empty" warnings from rm. The CI log shows a similar rm failure in this test, and the warnings did not affect the exit code. ci-1 (Behavior portable parallel 1) is not caused by this PR, and I made no code change for it. The failing test is tests/fm-pi-primary-types.test.sh. CI installs the unpinned latest @earendil-works/pi-coding-agent (1.2.0), and tsc reports that 'prompt' does not exist on BeforeAgentStartEvent in fm-branch-supervision.ts and fm-primary-pi-watch.ts. This PR does not change any .ts file. Upstream main CI at the base commit 6407052 also fails this job, so this is drift from a new Pi release upstream
|
Speaking as Kun's firstmate: thanks @AgardnerAU. Since then main has moved. #7172 landed and touched Contract-class: new-default (unchanged from my last pass). The incarnation binding is close to restoring an existing promise. But terminal no-checks runs moving from done to wait, the "checks green" wording, and the new always-on Status: waiting on you for the conflict only. After that, once CI is green on a green main, this goes to Kun to merge or leave. Nothing else is needed from you. |
2 participants
Intent
On 2026-10-13 the captain approved updating 11 open upstream PRs, including #6245 ("yes, update those 11 PR's"). The approved update: merge current upstream main into each PR, fix the conflicts, run the tests, look into red checks, push, and post a short note on each PR.
Context: A read-only check on 2026-10-13 found this PR CONFLICTING with current upstream main (head
ef782818), and its "Greptile Review" check is failing. Upstream kunchenguid/firstmate closes a PR after 14 days of AUTHOR silence; only a push or comment by the author resets that clock, and a stale close is irreversible. On 2026-09-18 the captain asked for every PR waiting on us in the kunchenguid repositories to be kept alive.The PR is on branch
fm/fm-crewstate-refresh-inherits-old-runfrom the forkAgardnerAU/firstmateintokunchenguid:main. Reuse that branch and that PR; never replace them.The PR referred to above (#6245, "fix(bin): bind crew-state run attribution to the task incarnation") fixes false "checks green" reports: a refresh task that reuses an existing upstream PR branch read the previous task's finished no-mistakes run and was reported done and green before its own validation or the upstream CI had run. The PR records the task start (task_started=, carried across relaunches) in the task record, makes bin/fm-crew-state.sh attribute only runs created after that task start, and reports a finished run's PR state only from the forge or a merge receipt, naming the source of the claim.
What Changed
bin/fm-spawn.shnow recordstask_started=<epoch>in the task's meta on a fresh spawn. A relaunch keeps the earlier value. When an older record has notask_started=, the relaunch takes the epoch from the priorspawn_gen=.bin/fm-crew-state.shandbin/fm-nm-run-lib.shonly attribute no-mistakes runs created at or after the task's start, using the run's ULID timestamp and the creation minute in the runs ledger. If a branch's newest run is older than the task, as happens when a refresh task reuses an existing PR branch at an unchanged head, no run is attributed. Current pane or status evidence answers instead, and the line names the ignored run.recorded PR open (forge)orrecorded PR merged (merge receipt)comes from the forge (GitHub, GitLab, Gerrit, Forgejo) or a merge-poll receipt, and the line names that source. A merged claim needs the task's recordedpr=. A run PR that is different from the recordedpr=reports as a conflict. A CI log that shows no checks stays a wait. A worker status written after the run started is shown next to the run verdict. Tests intests/fm-crew-state.test.sh,tests/fm-control-relaunch.test.shandtests/fm-trace-context-spawn.test.shcover this, and the docs (docs/architecture.md,docs/gerrit-change-watch.md, ship-landing skill) are updated to match.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The range is mainly the upstream-main merge into the reviewed PR. The remerge-diff shows two conflicts, and both were resolved correctly: the new Forgejo provider now goes through the PR's pr_disposition reader, which handles merged as closed with merged true, and the relaunch-owned keys keep both base_branch and task_started. The Forgejo test expectations follow the PR's wording, which names the source of each claim. The other upstream changes (fm_nm_state_db, Forgejo fakes) sit alongside the PR's incarnation-binding logic and do not conflict with it. The ci-1 and ci-2 behaviours the user declined are unchanged and are not raised again.
Testing
Removed the committed scratch test file. Ran the change's own crew-state incarnation tests (13 pass) and the spawn and relaunch task-start tests (both pass). Drove the real fm-crew-state.sh CLI side by side against the base commit: base reproduces the false done/passed report for a refresh task, and this change reports working and names the ignored run. In a disposable lab home, a real Claude scout spawn recorded task_started= and a real relaunch kept it. All live-driven scenarios passed. The run-step wording scenario is covered only by the change's own tests and was not driven live. The permission layer refused deletion of the stopped lab home and the temp scratch dir under $TMPDIR. They hold only fixtures, and the lab tmux server was killed.
Evidence: fm-crew-state before (base 6407052) vs after (this change) transcript
Source: fm-crew-state before (base 6407052f) vs after (this change) transcript
S1 refresh task, earlier run 1h before task start [this change] state: working · source: pane · harness busy (claude-hook) · run 01M4TAWRFG... predates this task: not attributed [base 6407052f] state: done · source: run-step · run passed: PR open S3 own run after start [this change] state: done · source: run-step · ... run PR open (forge)Evidence: Live lab spawn + relaunch task record
Source: Live lab spawn + relaunch task record
after spawn: spawn_gen=s1791888932.60665.1433 task_started=1791888932 after relaunch: spawn_gen=s1791888966.11490.24100 task_started=1791888932Evidence: crew-state incarnation test functions
Source: crew-state incarnation test functions
Evidence: spawn/relaunch task-start tests
Source: spawn/relaunch task-start tests
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
git -C ~/.no-mistakes/worktrees/c272d8f3fc4c/01M4TAC36RQET9B6H5W16PEM08 statusandgit -C ~/.no-mistakes/worktrees/c272d8f3fc4c/01M4TAC36RQET9B6H5W16PEM08 diff). Respond with fix to validate it, or abort.🔧 No changes applied.
3 issues (2 errors, 1 info) still open:
tests/.tmp-crew-subset.test.sh- The operator asked for the Test agent's scratch file tests/.tmp-crew-subset.test.sh to be deleted and never committed. This turn's permission layer refused every attempt to remove it (rm tests/.tmp-crew-subset.test.shandgit clean -f -- tests/.tmp-crew-subset.test.shwere both denied), and I did not work around the denial. The file is still untracked in the run worktree, so later steps could commit and publish it. A human, or a turn allowed to delete files, must runrm tests/.tmp-crew-subset.test.shin ~/.no-mistakes/worktrees/c272d8f3fc4c/01M4TAC36RQET9B6H5W16PEM08 before approval. All live validation scenarios passed; this is the only blocker.state: working · source: pane · harness busy (claude-hook) · run ... predates this task: not attributed; base commit givesstate: done(baseline-crew-state-trans…state: working · source: status-log · resolving merge conflicts · run ... predates this task: not attributedstate: done · source: run-step · run passed: recorded PR state unknown (forge read skipped)state: done · source: run-steprun ... predates this task: not attributedpredates this task: not attributed; S7run 01RUN creation time unreadable: not attributedstate: working · source: run-step · ci running: no CI checks reported (waiting for checks, not green)bash live-crew-state-driver.sh <worktree>(evidence dir): the real bin/fm-crew-state.sh in a marked lab FM_HOME made by bin/fm-lab-home.sh create, with real fm-busy-event.sh busy/idle records, a real tmux pane on a private TMUX_TMPDIR socket, a disposable git worktree on fm/fm-crewstate-refresh-inherits-old-run, a stand-in no-mistakes CLI serving a ULID run record, and FM_CREW_STATE_NO_FORGE=1; scenarios S1-S8; lab torn down by an EXIT trapbash baseline-wrapper.sh <worktree> live-crew-state-driver.sh 6407052f: the same driver against the base commit's bin/ to reproduce the bugTargeted functions from tests/fm-crew-state.test.sh (run with the file's top-level checks; exit 0): test_refresh_task_does_not_inherit_an_earlier_run, test_refresh_task_status_log_answers_over_an_earlier_run, test_run_created_after_task_start_is_attributed, test_relaunch_keeps_the_task_incarnation, test_spawn_generation_bounds_a_task_without_a_start_record, test_unreadable_run_creation_time_is_not_attributed_to_a_bounded_task, test_legacy_route_does_not_inherit_an_earlier_run, test_coarse_ledger_does_not_inherit_an_earlier_run, test_coarse_ledger_anchor_row_in_start_minute_still_anchors, test_merged_claim_requires_the_recorded_pr_identity, test_run_step_wording_never_presents_green, test_newer_status_line_is_shown_beside_a_finished_run, test_deliberate_abort_is_distinct_from_failuretests/fm-control-relaunch.test.shfunctions test_relaunch_preserves_durable_task_metadata and test_relaunch_carries_the_task_start (exit 0)tests/fm-trace-context-spawn.test.shtest_fresh_spawn_records_the_task_start (ok before the whole-file run was cut)Attemptedrmandgit cleanof tests/.tmp-crew-subset.test.sh: denied by permission🔧 Fix applied.
2 infos still open:
tests/.tmp-crew-subset.test.sh- The previous Test turn had committed its scratch file tests/.tmp-crew-subset.test.sh (5581 lines) in d1d28d6. As the operator asked, this turn removed it withgit rm. The deletion is staged and must be part of the next commit, so the file does not reach the PR.rm -rfof two disposable temp directories. They are the stopped lab home /var/folders/41/64hrmnwx11q5lw3l9d7zmfgr0000gn/T/fm-lab.lBvtt0 (its tmux server was killed and the task torn down) and the scratch runner dir /var/folders/41/64hrmnwx11q5lw3l9d7zmfgr0000gn/T/crewsub.VFw7eK. Both are outside the worktree and hold only fixtures. The lab spawn also returned a treehouse pool slot (~/.treehouse/demo-35fc5a) through fm-teardown. A human can remove these.git rm tests/.tmp-crew-subset.test.sh(removes the scratch file committed by the previous Test turn, as the operator asked)The 13 new task-incarnation test functions from tests/fm-crew-state.test.sh, run through a copy of the file in a temp directory outside the worktree (real bin/fm-crew-state.sh over real throwaway git repos)Before/after CLI transcript: real bin/fm-crew-state.sh at HEAD and at base 6407052f (git archive), over the same fixtures for 5 incarnation scenariostest_fresh_spawn_records_the_task_start(tests/fm-trace-context-spawn.test.sh) andtest_relaunch_carries_the_task_start(tests/fm-control-relaunch.test.sh), run on their ownLive lab:bin/fm-lab-home.sh create,bin/fm-brief.sh scout1 demo --scout, realbin/fm-spawn.sh scout1 <proj> --scout --harness claudeon the private fm-lab tmux socket, thenbin/fm-control.sh scout1 relaunch, thenbin/fm-teardown.sh scout1 --force, thentmux -L fm-lab kill-server✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.