Skip to content

fix(bin): enforce crew-state and merge outcome truth - #103

Merged
dnth merged 1 commit into
mainfrom
fm/port-3194-3104-3064-crewstate-merge-truth
Sep 5, 2026
Merged

fix(bin): enforce crew-state and merge outcome truth#103
dnth merged 1 commit into
mainfrom
fm/port-3194-3104-3064-crewstate-merge-truth

Conversation

@dnth

@dnth dnth commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Intent

Port upstream kunchenguid/firstmate pull requests kunchenguid#3194, kunchenguid#3104, and kunchenguid#3064 (crew-state run attribution + merge-outcome truth) into this fork, faithfully but adapted to the fork, preserving the fork's OMP crew/backend support so OMP crew-state attribution and merge/close reporting do not regress.

Requirements, in their current accepted form:

  1. Crew-state run attribution (fix(bin): prioritize active pipeline-owned crew runs kunchenguid/firstmate#3194): bin/fm-crew-state.sh must attribute the current run/step to the live crew incarnation. Precedence: (a) a daemon-attributed ACTIVE no-mistakes run for the task's branch from axi status is authoritative while branch_sync.state is pipeline_owned, without requiring head equality, reporting working with run-step detail; (b) otherwise the newest terminal run for the branch that no later run supersedes surfaces its terminal state, and a genuinely failed run with no later run must still report failed; (c) an unresolvable run head classifies as unknown attribution and must never cause a silent fallback onto an older row; in the coarse no-mistakes runs scan an ACTIVE row for this branch whose head cannot be resolved must not be skipped in favor of an older terminal row. The attribution helpers (fm_nm_head_resolvable, fm_nm_run_is_pipeline_owned_active) live in bin/fm-nm-run-lib.sh as the single owner of the run-attribution rule; the exemption requires an ACTIVE run and branch_sync.state=pipeline_owned specifically; fm-teardown.sh keeps strict head equality. Tests exercise the real bin/fm-crew-state.sh entrypoint with a PATH-stubbed no-mistakes.

  2. Merge-outcome truth (fix(bin): verify the real GitHub merge outcome instead of reporting an unproved merge kunchenguid/firstmate#3064): bin/fm-pr-merge.sh must verify the real GitHub merge outcome instead of reporting an unproved merge. After the gh-axi merge call, read the pull request's live state back and accept exactly two outcomes as success: merged, or in the merge queue. Anything else fails non-zero naming the concrete observed state (state, merged, isInMergeQueue). When the base branch requires a merge queue, the refusal names the queue's configured merge method and the exact -- --auto --<method> retry flags; never silently choose a merge method for the caller. No-rule, unreadable, conflicting, and unrecognised queue rule readings are reported apart. A caller who already passed --auto with the queue's method is told to re-check the queue state rather than handed back the flags. Refusals quote the merge command's own output separately from the script's verdict; a failed merge command keeps its original error first and never claims auto-merge was armed. PR metadata recording (pr=, pr_head=) and merge-poll arming happen once, unconditionally, before the forge call and survive any later failure. gh-axi always performs the merge; gh's GraphQL read is an optional queue-aware enrichment, and when gh is absent or its read fails the gh-axi view still proves a landed merge; a refusal built on the gh-axi view says the merge queue could not be observed. A caller-requested --auto that leaves the PR neither merged nor queued is refused.

  3. Durable merge outcome (fix(bin): durably report merged pull requests kunchenguid/firstmate#3104): a self-performed merge that succeeds emits exactly one durable outcome; a refused, failed, queued, or unconfirmed merge records nothing. In a secondmate home (.fm-secondmate-home present) append-once to the parent reply channel in the shape done [key=merged-<task-id>]: merged <task-id> <pr-url> (remote route: state/parent-replies.status; local route: the parent home's state/.status). In a main home append a durable check wake naming the merged PR. The existing merge poll's merged confirmation in bin/fm-watch.sh feeds the same emitter so self and poll origins produce one consistent durably-recorded outcome, with no second watch path. At-most-once is achieved by reusing the canonical merge-notification marker (fm_pr_poll_merge_already_notified / fm_pr_poll_merge_mark_notified) in bin/fm-pr-lib.sh; the outcome is published before the marker is committed so a failed commit stays eligible for at-least-once retry. The new bin/fm-merge-outcome-lib.sh is a separate small library. A merge that landed but whose record could not be written prints an actionable line to stderr and still exits 0. The mate charter in bin/fm-brief.sh names "work you landed" (a merge performed under standing merge authority and one the captain merged on the forge) as a reporting trigger. Teardown removes the merge-notified marker.

  4. Fork adaptations, all deliberate: bin/fm-pr-merge.sh stays GitHub-only and continues to refuse a GitLab merge request URL before recording state because GitLab merge parity has not landed in this fork, so upstream's GitLab merge tests are not ported; bin/fm-inactive-reconcile.sh does not exist in this fork so its diagnostic hardening is not ported; bin/fm-merge-outcome-lib.sh must not re-source bin/fm-wake-lib.sh inside a process that already loaded it because this fork's watcher carries recovery-marker state across marker transitions; the OMP supervision-branch merge prohibition (fm_lease_forbid_branch) in bin/fm-pr-merge.sh is retained; the main-home merge-outcome row is an ordinary check-kind wake, which the OMP branch dispatcher keeps main-owned; an OMP crew with unknown attribution falls to its ordinary pane fallback rather than a superseded failed row, pinned by a crew-state test.

  5. Tests: port the upstream tests, adapted for the fork, into tests/fm-crew-state.test.sh, tests/fm-pr-merge.test.sh, tests/fm-pr-check-security.test.sh, and tests/fm-brief.test.sh; the ported tests plus the relevant existing crew-state and merge tests and bin/fm-lint.sh (shellcheck) must pass. Tests exercise behavior through executable interfaces with PATH-stubbed forge CLIs and never assert implementation source bytes.

  6. This is firstmate's own shared tracked material, so .agents/skills/firstmate-coding-guidelines/SKILL.md is binding: one sentence per line in tracked Markdown, plain dash never an em dash, no agent co-author, shellcheck-clean bin scripts through bin/fm-lint.sh, colocated tests named .test.sh extending existing runners, one-owner rule for contracts, script headers own mechanics. Documentation updated: AGENTS.md state list and merge/validation sentences, docs/architecture.md, docs/scripts.md, docs/configuration.md.

Firstmate-Validation-Generation: 141a0f50463896b871e5015ed4476fdb

What Changed

  • Centralized no-mistakes run attribution so crew-state reporting respects pipeline-owned active runs, terminal-run supersession, and unknown head resolution without falling back to stale rows.
  • Made GitHub PR merges verify a live merged or queued outcome, report merge-queue requirements precisely, preserve metadata and polling before the forge call, and publish deduplicated durable merge outcomes for self and watcher paths.
  • Updated teardown, watcher, mate charter, documentation, and executable shell tests to cover the new attribution, merge verification, security, durability, and fork-specific OMP behavior.

Risk Assessment

⚠️ Medium: The merge remains conservatively unconfirmed, but the gh-read-failure fallback can misreport a concrete open outcome and omit the required queue-unobservable guidance in a supported degradation path.

Testing

Ran the four relevant executable-interface test scripts with PATH-stubbed dependencies; all passed and logs were captured as evidence artifacts. No UI surface was involved, and linting was not run because this assigned test phase explicitly prohibits linters.

Evidence: Crew-state targeted tests
ok - active run-step is authoritative
ok - stale needs-decision over active run is superseded
ok - stale blocked over active run is superseded
ok - genuine parked run is not flagged superseded
ok - scalar gate parked run is not flagged superseded
ok - gate block parked run is not flagged superseded
ok - ci-ready status log beats monitoring run
ok - ci-monitoring run with checks already green surfaces done
ok - top-level ci status uses ci log green marker
ok - terminal no-checks ci-monitor marker surfaces done
ok - base-advance rearm after green stays working
ok - pending no-checks ci-monitor marker stays working
ok - ci-monitoring run with checks not yet green stays working
ok - a fresh issue after an earlier green reading is not masked
ok - stale checks-green status log does not mask CI relapse
ok - ci fixing is not overridden by an earlier green marker
ok - top-level fixing is not overridden by a stale ci running row
ok - top-level fixing is not overridden by a stale done log
ok - terminal passed run is authoritative
ok - terminal failed run is authoritative
ok - cross-branch run is attributed via the real runs list
ok - cross-branch attribution picks the branch's most recent row
ok - coarse run does not probe another branch's ci log
ok - another branch's run is ignored, falls back
ok - no run + a busy semantic record reads working, attributed to its source
ok - a converted adapter never reads working from rendered footer text
ok - grok still reads working through its isolated rendered-tail fallback
ok - herdr's native busy verdict reads working with no record present
ok - a mid-tool-call crew stays working because its record outranks herdr's generation state
ok - an idle record with idle agent_status stays not-busy (no regression for a human-blocked agent)
ok - no run + idle pane uses the status-log verb
ok - no run + idle pane parses keyed status syntax
ok - no run + idle pane on a paused: status reports state: paused with its reason
ok - no run + idle pane honors the configured paused verb
ok - a trailing resolved: event does not corrupt state render (idle stays idle)
ok - dead window ignores stale status log
ok - closed pane still reports a terminal run-step
ok - closed pane still reports an active run-step
ok - no timeout command uses perl bound
ok - scout skips the run lookup
ok - torn-down worktree is handled gracefully
ok - missing meta is handled gracefully
ok - crew_is_provably_working absorbs a validating crew found only via the runs-list fallback
ok - crew_is_provably_working still surfaces a genuinely stopped crew (safety property preserved)
ok - usage error exits 2
ok - historical same-branch rewritten head is not attributed as current
ok - active run with valid descendant fix head remains current
ok - local work advanced past run head invalidates attribution
ok - pipeline-owned active run binds without head equality and beats the failed row
ok - a genuinely failed run with no later run is not hidden
ok - coarse scan stops on an unresolvable active row instead of binding an older one
ok - coarse scan guard sends an OMP crew to its pane fallback instead of a superseded failure
ok - the exemption requires branch_sync.state=pipeline_owned
ok - the exemption never applies to a terminal run
ok - missing run head falls back instead of matching by branch
ok - ship completion requires evidence and current-head implementation completion
ok - ship completion fails closed when the evidence contract is malformed
ok - run-step done requires current-generation validation completion
ok - status-log done requires existing plan completion
ok - final done requires a clean inspectable worktree
ok - LOW validation remains parked until PR completion
ok - direct-PR and local-only state reads skip No-Mistakes
ok - ship state requires one valid mode before run lookup
all fm-crew-state tests passed
Evidence: PR merge targeted tests
ok - fm-pr-merge reports exact queue retry flags after a zero-exit false success
ok - fm-pr-merge omits merge-queue retry guidance for a closed GitHub PR
ok - fm-pr-merge aggregates agreeing merge-queue rules
ok - fm-pr-merge reports ambiguity for conflicting merge-queue rules
ok - fm-pr-merge records pr= and pr_head= for a verified GitHub merge
ok - fm-pr-merge records pr= before the forge call can land the merge
ok - fm-pr-merge propagates a real merge failure without silently succeeding
ok - fm-pr-merge refuses a GitHub merge call that leaves the PR open and unqueued
ok - fm-pr-merge keeps PR bookkeeping when it cannot read a successful merge call's outcome
ok - fm-pr-merge refuses with the forge's own output quoted apart from its verdict
ok - fm-pr-merge quotes the forge output when it cannot read the outcome either
ok - fm-pr-merge does not echo back queue flags the caller already used
ok - fm-pr-merge still names retry flags when the caller used a different method
ok - fm-pr-merge names the queue requirement even when its method is unrecognised
ok - fm-pr-merge distinguishes unreadable branch rules from a base with no merge queue
ok - fm-pr-merge says nothing about a merge queue when the base branch has no queue rule
ok - fm-pr-merge says the merge queue was unobservable when only the gh-axi view answered
ok - fm-pr-merge explains an armed auto-merge that landed nothing on a queue-less base
ok - fm-pr-merge never reports auto-merge as armed when the merge command failed
ok - fm-pr-merge claims no acceptance for a failed merge command carrying queue flags
ok - fm-pr-merge falls back to the gh-axi view when gh's read fails
ok - fm-pr-merge names a landed state hiding behind a failed GitHub merge command
ok - fm-pr-merge reaches and verifies the gh-axi merge path without gh
ok - fm-pr-merge preserves bookkeeping when gh is absent and the fallback read fails
ok - fm-pr-merge verifies a genuinely merged GitHub pull request
ok - fm-pr-merge refuses to merge when poll recording fails
ok - fm-pr-merge accepts and accurately reports a GitHub merge-queue entry
ok - fm-pr-merge explains how to retry with the required GitHub merge queue method
ok - fm-pr-merge forwards extra flags to gh-axi pr merge after the -- separator
ok - fm-pr-merge refuses before merging when task meta is missing
ok - fm-pr-merge refuses malformed and GitLab PR URLs before calling gh-axi
ok - fm-pr-merge refuses unsafe PR URL segments before recording state
ok - fm-pr-merge refuses repo override args, including bundled short clusters, before recording state
ok - fm-pr-merge does not add default --squash when the caller passes an explicit merge method
ok - fm-pr-merge respects --method=<value> as an explicit merge method
ok - fm-pr-merge parses a GitHub PR URL into gh-axi number and --repo arguments
ok - a merge a secondmate home performs itself is reported upward exactly once
ok - a locally routed secondmate home reports the landed PR into its parent's own channel
ok - a refused or failed merge reports no outcome
ok - a merge call the forge accepted but did not land reports no outcome
ok - a merge a main home performs itself leaves one durable check-kind wake naming the PR
ok - a queued GitHub merge stays silent and leaves confirmation to the armed poll
ok - distinct merged PRs for one task retain distinct captain-facing wakes
ok - an uncommitted marker retry preserves at least one durable outcome
ok - a secondmate home that cannot report upward says so instead of merging in silence
all fm-pr-merge tests passed
Evidence: PR check security targeted tests
ok - raw-byte parser accepts canonical URLs and rejects the complete adversarial matrix
ok - GitLab merge requests are followed on any instance and never wake falsely
ok - validated merged polls notify once and retire before the next watcher cycle
ok - a repeat identical merged poll for an already-notified task is absorbed, never queued as a main-blocking row
ok - a failed upward merge report keeps its poll armed for repair and retry
ok - staged self-merge and poll interleavings are never silent
ok - a merge detected by the poll is reported upward from a secondmate home exactly once
ok - a different merged PR for the same task gets its own first notification
ok - merged poll retirement preserves every persistent secondmate lifecycle artifact
ok - queue, receipt, and every fixed-path removal crash point recover without loss or repeated execution
ok - open/red, closed-unmerged, malformed, and forge errors remain armed until an exact merged transition
ok - replacement, nonterminal, tampered, and custom results receive no deletion authority
ok - queue failure and untrusted receipts preserve canonical poll evidence
ok - GitHub and GitLab exact merged results share one retirement path
ok - PR registration serializes with validation planning
ok - fast PR registration completes and keeps its watcher armed
ok - PR metadata publication rejects post-snapshot redirection
ok - watcher defers valid pre-metadata polls while the validation lock is held
ok - watcher bounds pre-metadata deferral by validation lock freshness
ok - PR and teardown entrypoints reject invalid arguments before every side effect
ok - valid direct and merge flows record exact metadata and reject multiline head metadata
ok - rejected metacharacter bytes remain inert at generation and watcher time
ok - static poll is silent except for one merged line and remains watcher-bounded
ok - interrupted atomic preparation cleans private temporaries and publishes nothing
ok - concurrent watchers observe only complete private poll publications
ok - post-rename poll validation faults revoke both names and allow a clean retry
ok - poll publication paths refuse symlinks and directories
ok - live poll and custom-check boundaries require single-link private files
ok - watcher signals promptly stop custom checks and clean private state
ok - returned custom check descendants are drained on installed and fallback timeout paths
ok - teardown removes live PR-check artifacts
Evidence: Brief targeted tests
ok - fm-brief.sh: bash -n succeeds
/tmp/fm-brief.hrPsZF/heredoc-in-substitution.sh:2
ok - fm-brief.sh: no heredoc is nested inside a command substitution (Bash 3.2 parse-safe)
ok - fm-brief.sh: --help renders the complete header
ok - fm-brief.sh: no-mistakes/direct-PR/local-only briefs generate cleanly
ok - fm-brief.sh: ship --mode is required and closed-set validated
ok - fm-brief.sh: the explicit ship mode wins over the registered posture
ok - fm-brief.sh: --yolo and scout/secondmate --mode are refused, never silently dropped
ok - fm-brief.sh: faster paths use configured authority without stacked review
ok - fm-brief.sh: no-mistakes DOD keeps its apostrophe prose, now parse-safe
ok - fm-brief.sh: ship project-memory wording carries the AGENTS.md authoring bar
ok - fm-brief.sh: --herdr-lab emits the complete hard safety contract
ok - fm-brief.sh: --herdr-lab uses its quoted Firstmate-owned helper path
ok - fm-brief.sh: ship and scout scaffolds make omitted Herdr intent fail-visible
ok - fm-brief.sh: the documented {TASK} fill cannot corrupt the Herdr safety gate
ok - fm-brief.sh: Herdr lab contract covers scouts and rejects secondmate misuse
ok - fm-brief.sh: --no-projects scaffolds a project-less charter and guards misuse
ok - fm-brief.sh: marked requests avoid generic acknowledgements and preserve material reporting
ok - fm-brief.sh: relative directory inputs ignore CDPATH, render stable absolute charter paths, or fail loudly
ok - fm-brief.sh: custom pause verb renders in every scaffold
ok - fm-brief.sh: investigation and visual-review completions load the shared decision policy
ok - fm-brief: scout and secondmate code paths still scaffold well-formed briefs
ok - fm-brief: concurrent ship scaffolds preserve one complete owner
ok - fm-brief: ship evidence publication is atomic and retryable

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 warning
  • ⚠️ bin/fm-pr-merge.sh:259 - When gh is present but its GraphQL read fails, github_read_outcome only accepts the gh-axi fallback if it reports merged=true; if gh-axi pr view reports an open/unqueued PR, the script exits via the generic “could not read” path and never runs github_report_unmerged_outcome. Thus a concrete fallback observation such as state=open is not reported with isInMergeQueue=unknown or the required “merge queue could not be observed” refusal. This contradicts the intent that “when gh is absent or its read fails the gh-axi view still proves a landed merge; a refusal built on the gh-axi view says the merge queue could not be observed.”
✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-crew-state.test.sh
  • bash tests/fm-pr-merge.test.sh
  • bash tests/fm-pr-check-security.test.sh
  • bash tests/fm-brief.test.sh
  • Verified git status --short is clean after testing
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…truth

Port kunchenguid/firstmate kunchenguid#3194, kunchenguid#3104, and kunchenguid#3064 into this fork.

Crew-state run attribution (kunchenguid#3194): bin/fm-nm-run-lib.sh gains
fm_nm_head_resolvable and fm_nm_run_is_pipeline_owned_active, and
bin/fm-crew-state.sh binds an active pipeline-owned run by branch without
requiring its lane head to resolve in the task worktree, stops the coarse
runs scan on an unresolvable newer head instead of falling through to a
superseded terminal row, and still surfaces a genuine unsuperseded failure.

Merge-outcome truth (kunchenguid#3064): bin/fm-pr-merge.sh records pr= and arms the
merge poll once before the forge call, then reads the live GitHub state back
and accepts only merged or in-merge-queue as success. Anything else refuses
non-zero naming the observed state, quotes the forge's own output apart from
the verdict, names exact merge-queue retry flags without choosing a method,
and falls back to the gh-axi view only when gh's read fails.

Durable merge outcome (kunchenguid#3104): new bin/fm-merge-outcome-lib.sh publishes one
role-routed outcome for a merge this home performed or its poll detected,
deduplicated through the merge-notified marker now owned by bin/fm-pr-lib.sh;
the watcher routes merged polls through it before retiring, teardown removes
the marker, and the mate charter names landed work as a report trigger.

Fork adaptations: fm-pr-merge stays GitHub-only (GitLab merge parity has not
landed here), fm-inactive-reconcile does not exist here so its diagnostic
hardening is not ported, the merge-outcome library never re-sources
fm-wake-lib inside a process that already loaded it because this fork's
watcher carries recovery-marker state across marker transitions, the OMP
supervision-branch merge prohibition in fm-pr-merge is retained, and the OMP
crew pane fallback is pinned by a new crew-state case.

Claude-Session: https://claude.ai/code/session_013LDmHMU6VLbFa9jW1J4FQV
@dnth
dnth merged commit bf0c504 into main Sep 5, 2026
28 of 29 checks passed
@dnth
dnth deleted the fm/port-3194-3104-3064-crewstate-merge-truth branch September 5, 2026 00:29
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