Skip to content

fix(bin): attest existing Captain's Call references - #1987

Open
moneyteamameen wants to merge 4 commits into
kunchenguid:mainfrom
moneyteamameen:fm/decision-hold-existing-reference
Open

fix(bin): attest existing Captain's Call references#1987
moneyteamameen wants to merge 4 commits into
kunchenguid:mainfrom
moneyteamameen:fm/decision-hold-existing-reference

Conversation

@moneyteamameen

Copy link
Copy Markdown

Intent

Fix the decision-hold completion lifecycle so a completed investigation or visual review can explicitly attest unresolved captain decisions already registered under earlier formal or legacy structured task identities, without duplicating, closing, or mutating those Captain's Call rows. Extend bin/fm-decision-hold.sh complete narrowly and fail closed with repeated explicit existing task identities; require live reviewing-origin metadata for durable, sorted, idempotent attestation; verify every newly referenced identity exists in the active authoritative backlog and is queued, unblocked, actively captain-held kind captain work; reject mixed invalid sets, missing or malformed identities, completed work, non-captain work, ordinary parked work, external holds, and blocked items with actionable errors before any attestation. Preserve referenced identity, title, body, dependencies, and open state, create no task, and preserve complete --none plus deterministic current-origin-key behavior. Keep verify and scout teardown compatible, retain the default tasks-axi and manual-backend contracts, and never weaken the rule that decisions must already be durable before completion. Cover a pre-existing formal hold, an arbitrary legacy structured hold, idempotent retry, multiple holds, every rejection class, absence of duplicate creation or mutation, unchanged existing flows, successful verify, and teardown gating. Keep exact syntax in the script header and help, policy only in the decision-hold skill, mechanism and regression evidence in the decision-hold document, do not change AGENTS.md, use one sentence per Markdown line and plain dashes, and add no agent co-author. The local focused, documentation, lint, coverage, and lifecycle suites are green; an unrelated Muse detection test fails only in the outer Codex process environment, so classify or correct it within pipeline isolation. Use branch fm/decision-hold-existing-reference, push and open a PR, wait for green checks, and do not merge.

What Changed

  • Extend complete to accept repeated --existing <task-id> references alongside current-origin decision keys, recording sorted identities without creating or mutating referenced backlog items.
  • Require live origin metadata and fail closed unless each new reference is queued, unblocked, actively captain-held work, then publish the completion attestation atomically and verify references during teardown.
  • Update lifecycle policy, mechanism documentation, and regression coverage for formal and legacy references, retries, invalid states, verification, and teardown gating.

Risk Assessment

✅ Low: The follow-up changes close all three previously identified source defects: blocker eligibility now follows raw dependency semantics, completion metadata is published by one same-directory atomic replacement, and the missing rejection and interrupted-publication regressions are covered without broadening behavior.

Testing

The focused lifecycle E2E suite and an independent manual-backend CLI walkthrough passed, covering help syntax, formal and legacy attestation, sorted idempotent metadata, preservation without duplication or mutation, all rejection classes, successful verify, and teardown gating; this is CLI-only behavior, so no visual artifact applies.

Evidence: Decision-hold existing-reference CLI walkthrough

Shows sorted formal/legacy attestation, unchanged referenced tasks and backlog, successful verification, idempotent retry, and atomic rejection of an external hold.

=== Complete with two earlier Captain Calls (reverse input order) ===
complete: demo-followup-review decision inventory reviewed (existing=demo-earlier-review-decision-route,legacy-demo-captain-call)
=== Persisted reviewing-origin attestation ===
decision_keys=
decision_refs=demo-earlier-review-decision-route,legacy-demo-captain-call
decisions_reviewed=1
=== Referenced formal task after completion ===
task:
  id: demo-earlier-review-decision-route
  title: Choose the earlier sample route
  state: queued
  blocked: no
  blocked_by: none
  held: yes
  hold_reason: captain earlier route pending
  hold_kind: captain
  hold_until: "-"
  kind: captain
  repo: sample
  priority: "-"
  created: 2026-08-09
=== Referenced legacy task after completion ===
task:
  id: legacy-demo-captain-call
  title: Choose the legacy sample route
  state: queued
  blocked: no
  blocked_by: none
  held: yes
  hold_reason: captain legacy route pending
  hold_kind: captain
  hold_until: "-"
  kind: captain
  repo: sample
  priority: "-"
  created: 2026-08-09
preservation: backlog_sha256_unchanged=yes formal_projection_unchanged=yes legacy_projection_unchanged=yes formal_row_count=1 legacy_row_count=1
=== Verify attestation ===
verified: demo-followup-review unresolved-decision inventory
=== Idempotent retry with duplicate/reordered identities ===
complete: demo-followup-review decision inventory reviewed (existing=demo-earlier-review-decision-route,legacy-demo-captain-call)
retry: metadata_sha256_unchanged=yes
=== Mixed valid + external-hold rejection ===
exit=1
fm-decision-hold: existing reference demo-external-hold is an external hold, not a Captain's Call item
rejection_atomicity: metadata_sha256_unchanged=yes backlog_sha256_unchanged=yes
artifact_path=/var/folders/s8/0_mzgfjj34v2wr0gvxxh64n80000gn/T/no-mistakes-evidence/01KZFGD9NNPNX0SR6T3SJQH7JA/decision-hold-existing-reference.XXXXXX.txt
fixture_path=/var/folders/s8/0_mzgfjj34v2wr0gvxxh64n80000gn/T/no-mistakes-evidence/01KZFGD9NNPNX0SR6T3SJQH7JA/manual-existing-reference.V8hiI5
=== Full preserved formal task projection (identity/title/state/dependencies/body) ===
task:
  id: demo-earlier-review-decision-route
  title: Choose the earlier sample route
  state: queued
  blocked: no
  blocked_by: none
  held: yes
  hold_reason: captain earlier route pending
  hold_kind: captain
  hold_until: "-"
  kind: captain
  repo: sample
  priority: "-"
  created: 2026-08-09
  closed: "-"
  deps: none
  links: none
  body: "Origin: demo-earlier-review\nDecision key: route\nState: awaiting captain decision."
=== Full preserved legacy task projection (identity/title/state/dependencies/body) ===
task:
  id: legacy-demo-captain-call
  title: Choose the legacy sample route
  state: queued
  blocked: no
  blocked_by: none
  held: yes
  hold_reason: captain legacy route pending
  hold_kind: captain
  hold_until: "-"
  kind: captain
  repo: sample
  priority: "-"
  created: 2026-08-09
  closed: "-"
  deps: none
  links: none
  body: Legacy structured Captain's Call body.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed ✅
  • 🚨 bin/fm-decision-hold.sh:202 - The required criterion says every new reference must be “queued, unblocked, actively captain-held” and blocked items must be rejected, but this trusts tasks-axi’s derived blocked field. That field treats a dangling blocked-by identity as resolved, while Firstmate’s canonical fleet projection deliberately keeps missing blockers unresolved. A captain-held row with blocked-by: missing is therefore attested and teardown can proceed even though Bearings excludes it from Captain’s Call. Validate blocker readiness using the canonical Firstmate semantics and add the dangling-blocker regression.
  • 🚨 bin/fm-decision-hold.sh:390 - The durable attestation is published in two appends: decisions_reviewed=1 and keys first, then decision_refs. If the second append fails or the process stops between them, verify sees a completed inventory with no references and teardown may remove the source. This contradicts the required “durable, sorted, idempotent attestation.” Publish keys, references, and the reviewed marker as one failure-atomic metadata update and cover an interrupted-write failure.
  • 🚨 tests/fm-decision-hold-lifecycle.test.sh:359 - The required “every rejection class” coverage is incomplete: this fixture always creates live origin metadata and never exercises a kind-captain item that is inactive (held=no) or non-queued. Thus the new live-metadata, active-hold, and queued-state rejection branches have no regression evidence. Add those exact cases and assert both metadata and backlog remain unchanged.

🔧 Fix: Fix atomic decision attestation and blocker validation
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-decision-hold-lifecycle.test.sh
  • bin/fm-decision-hold.sh --help
  • Manual-backend CLI walkthrough using complete demo-followup-review --existing demo-earlier-review-decision-route --existing legacy-demo-captain-call, followed by verify, duplicate/reordered retry, and mixed external-hold rejection
  • Compared backlog hashes and full referenced-task projections before and after completion; compared metadata hashes across retry and rejected completion
  • git status --short after testing
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Quote completed blocker state in lint-safe comparison
✅ Re-checked - no issues remain.

✅ **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