From 1acb47e5569f6c0da241c63042fc2196efec12d6 Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Thu, 13 Aug 2026 12:53:47 -0400 Subject: [PATCH 1/6] fix(decision-hold): find durably-resolved decisions in the Done archive The investigation-completion gate could only see decisions in the live backlog, so a session that resolved more decisions than the backlog's done_keep retention permanently locked its own investigations open. task_show ran `tasks-axi show`, which reads only the active backlog file. Once retention rotated a resolved captain decision into the configured archive, verify_hold_durable reported it "absent from .../data/backlog.md", which made complete, verify, and therefore fm-teardown.sh all refuse. The only workaround was forcing past the refusal, which is exactly what the gate exists to prevent. verify_hold_durable now falls back to the configured Done archive when an identity is absent from the live backlog. The guarantees are unchanged: only a resolved record is accepted from the archive, it must carry the same `Resolution recorded by fm-decision-hold.` and `Routed work:` body an active record must carry, and verify_hold_active still reads the live backlog alone so an open hold is never satisfiable from the archive. The resolved-record test is now one shared predicate applied to both sources. The archive path is read from `.tasks.toml`'s [markdown] archive key rather than hardcoded. An absent config, absent key, missing file, or empty file is an ordinary absence and refuses as before; an unreadable, non-regular, non-text, or structurally unrecognizable archive refuses distinctly instead of reading as absence. tasks-axi 0.2.5 exposes no archive query, and `--file` pointed at the configured archive is refused outright, so the lookup queries a private throwaway snapshot whose `## Archived ` headings are normalized to `## Done`. That keeps tasks-axi's own parser as the only record parser instead of hand-parsing markdown. Adds a regression covering all four boundaries, driven through the backlog's own `tasks-axi prune` retention rather than hand-moved records, and records the incident with dated evidence in docs/decision-hold-lifecycle.md. --- bin/fm-decision-hold.sh | 153 +++++++++++++++++--- docs/decision-hold-lifecycle.md | 56 +++++++ tests/fm-decision-hold-lifecycle.test.sh | 177 +++++++++++++++++++++++ 3 files changed, 369 insertions(+), 17 deletions(-) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index aeb140a296a..cee0dbd67ca 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -37,6 +37,27 @@ # It writes the captain decision and routed identities into the hold body, clears # those dependency edges, and only then marks the hold Done. A failure before the # final step leaves the captain hold open. +# +# Durably-resolved lookup and the Done archive +# +# The backlog's own retention (`.tasks.toml` [markdown] done_keep) rotates closed +# items out of the active backlog file into the configured archive, and +# `tasks-axi show` reads only the active file. A durably-resolved captain +# decision is therefore looked up in the active backlog first and in that archive +# second, so a session that resolves more decisions than done_keep can still +# complete, verify, and tear down its own investigations. Only a resolved record +# is accepted from the archive, and it must carry the same resolution body an +# active record must carry. An ACTIVE hold is never satisfiable from the archive: +# verify_hold_active reads the live backlog alone. +# +# The archive path comes from `.tasks.toml`'s [markdown] archive key, resolved the +# way tasks-axi resolves it, relative to FM_HOME. An absent config file or absent +# archive key means there is no archive to consult. Because `tasks-axi show +# --file` refuses the configured archive path itself, the lookup queries a +# private throwaway snapshot whose `## Archived ` headings are normalized +# to `## Done`, which keeps tasks-axi's own parser as the only record parser. A +# missing archive is an ordinary absence; an unreadable, non-regular, non-text, or +# structurally unrecognizable archive refuses instead of reading as absence. set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -110,6 +131,93 @@ task_show() { # tasks_axi show "$1" --full 2>/dev/null } +# Refusals below run through fail, which exits. A subshell cannot exit this +# script, so every archive helper reports through a global instead of stdout: a +# refusal captured by command substitution would collapse into an empty result and +# read as a legitimate absence, which is exactly the confusion this fix removes. + +ARCHIVE_PATH='' +# Sets ARCHIVE_PATH to the configured Done-archive path, or to the empty string +# when this home has no archive. Reads only the [markdown] table so an `archive` +# key in another table cannot be mistaken for it, and refuses a present-but-empty +# or unquoted value rather than treating a malformed config as an absent archive. +load_archive_path() { + local config="$FM_HOME/.tasks.toml" value + ARCHIVE_PATH='' + [ -f "$config" ] || return 0 + value=$(awk ' + /^[[:space:]]*#/ { next } + /^[[:space:]]*\[/ { + section = $0 + sub(/^[[:space:]]*\[[[:space:]]*/, "", section) + sub(/[[:space:]]*\].*$/, "", section) + in_markdown = (section == "markdown") + next + } + in_markdown && /^[[:space:]]*archive[[:space:]]*=/ { + value = $0 + sub(/^[[:space:]]*archive[[:space:]]*=[[:space:]]*/, "", value) + if (match(value, /^["\047][^"\047]*["\047]/)) { + printf "value\t%s\n", substr(value, RSTART + 1, RLENGTH - 2) + } else { + print "malformed" + } + exit + } + ' "$config" 2>/dev/null) || fail "could not read the backlog archive setting in $config" + case "$value" in + '') return 0 ;; + "value "*) + value=${value#value } + [ -n "$value" ] || fail "the backlog archive setting in $config is empty" + case "$value" in + /*) ARCHIVE_PATH=$value ;; + *) ARCHIVE_PATH="$FM_HOME/$value" ;; + esac + ;; + *) fail "the backlog archive setting in $config is not a quoted path" ;; + esac +} + +ARCHIVE_SHOW='' +# Sets ARCHIVE_SHOW to a `tasks-axi show --full` record for read from the Done +# archive and returns 0, or clears it and returns 1 when this home has no archive +# or the archive holds no such record. Refuses rather than reporting absence when +# an archive exists but cannot be read as a backlog file. +load_archive_show() { # + local id=$1 archive snapshot rc + ARCHIVE_SHOW='' + load_archive_path + archive=$ARCHIVE_PATH + [ -n "$archive" ] || return 1 + [ -e "$archive" ] || return 1 + [ -f "$archive" ] || fail "the backlog archive is not a regular file: $archive" + [ -r "$archive" ] || fail "the backlog archive is not readable: $archive" + # An empty archive unambiguously holds no records, so it is an absence like a + # missing file. A non-empty file that is not a text backlog is not an absence. + [ -s "$archive" ] || return 1 + [ "$(LC_ALL=C tr -d -c '\000' < "$archive" | wc -c | tr -d ' ')" = 0 ] \ + || fail "the backlog archive is not a text backlog file: $archive" + grep -q '^##[[:space:]]' "$archive" \ + || fail "the backlog archive has no recognizable backlog sections: $archive" + snapshot=$(mktemp "${TMPDIR:-/tmp}/fm-decision-hold-archive.XXXXXX") \ + || fail "could not stage a read-only snapshot of $archive" + if ! sed 's/^##[[:space:]]*Archived\([[:space:]].*\)*$/## Done/' "$archive" > "$snapshot"; then + rm -f "$snapshot" + fail "could not stage a read-only snapshot of $archive" + fi + set +e + ARCHIVE_SHOW=$(tasks_axi show "$id" --file "$snapshot" --full 2>/dev/null) + rc=$? + set -e + rm -f "$snapshot" + if [ "$rc" -ne 0 ]; then + ARCHIVE_SHOW='' + return 1 + fi + return 0 +} + show_field() { # local output=$1 field=$2 printf '%s\n' "$output" | sed -n "s/^ $field: //p" | head -1 @@ -170,9 +278,10 @@ verify_hold_active() { # [ "$hold_kind" = captain ] || fail "backlog item $id is not held for the captain" } -verify_hold_resolved() { # - local id=$1 show state kind body - show=$(task_show "$id") || return 1 +# The single test for a durably-resolved captain decision, applied identically to +# an active-backlog record and an archived one. +record_is_resolved() { # + local show=$1 state kind body state=$(show_field "$show" state) kind=$(show_field "$show" kind) body=$(show_field "$show" body) @@ -184,23 +293,33 @@ verify_hold_resolved() { # return 1 } +verify_hold_resolved() { # + local id=$1 show + show=$(task_show "$id") || return 1 + record_is_resolved "$show" +} + verify_hold_durable() { # - local id=$1 show state held kind hold_kind body - show=$(task_show "$id") || fail "captain decision $id is absent from $FM_HOME/data/backlog.md" - state=$(show_field "$show" state) - held=$(show_field "$show" held) - kind=$(show_field "$show" kind) - hold_kind=$(show_field "$show" hold_kind) - body=$(show_field "$show" body) - if [ "$state" = queued ] && [ "$held" = yes ] && [ "$kind" = captain ] && [ "$hold_kind" = captain ]; then - return 0 + local id=$1 show state held kind hold_kind + if show=$(task_show "$id"); then + state=$(show_field "$show" state) + held=$(show_field "$show" held) + kind=$(show_field "$show" kind) + hold_kind=$(show_field "$show" hold_kind) + if [ "$state" = queued ] && [ "$held" = yes ] && [ "$kind" = captain ] && [ "$hold_kind" = captain ]; then + return 0 + fi + record_is_resolved "$show" && return 0 + fail "captain decision $id is neither actively held nor durably resolved" fi - if [ "$state" = "done" ] && [ "$kind" = captain ]; then - case "$body" in - *"Resolution recorded by fm-decision-hold."*"Routed work:"*) return 0 ;; - esac + # Absent from the live backlog: retention may have rotated a resolved decision + # into the Done archive. Only a resolved record counts there, and it must carry + # the same resolution body an active record must carry. + if load_archive_show "$id"; then + record_is_resolved "$ARCHIVE_SHOW" && return 0 + fail "archived captain decision $id has no durable resolution record" fi - fail "captain decision $id is neither actively held nor durably resolved" + fail "captain decision $id is absent from $FM_HOME/data/backlog.md" } verify_resolution_identity() { diff --git a/docs/decision-hold-lifecycle.md b/docs/decision-hold-lifecycle.md index 234055aec3f..b6acf30a8aa 100644 --- a/docs/decision-hold-lifecycle.md +++ b/docs/decision-hold-lifecycle.md @@ -23,6 +23,12 @@ For an open keyed status decision, it appends a `captain-held [key=]: ...` Scout teardown calls the script's read-only `verify` subcommand after checking for the report and before removing any source state. The `--force` path remains the explicit captain-approved discard escape hatch. +`complete` and `verify` look a durably-resolved decision up in the active backlog first and in the configured Done archive second. +The backlog's own retention rotates closed items out of the active file, and `tasks-axi show` reads only that active file, so without the archive lookup a session that resolves more decisions than `done_keep` locks its own investigations open. +Only a resolved record is accepted from the archive, and it must carry the same `Resolution recorded by fm-decision-hold.` and `Routed work:` body an active record must carry. +An active hold is never satisfiable from the archive, because the separate active-hold check that guards `resolve` reads the live backlog alone. +`bin/fm-decision-hold.sh`'s header and `--help` own the exact archive-path resolution, snapshot mechanics, and refusal conditions. + The `resolve` subcommand requires a decision file and at least one existing dependent task whose structured `blocked-by` edge points to the hold. It records the decision digest and routed task identities as a retry identity in the hold body, clears each dependency edge through tasks-axi, and marks the hold Done only after those writes succeed. An exact retry can finish a partial routing operation, while a changed decision or routed-task set is rejected. @@ -38,16 +44,58 @@ Its secondmate-home summary classifies an actionable captain hold as `captain_de It excludes completed kind `captain` records from Recently Landed. The projection remains read-only and does not inspect historical prose. +## Incident: retention hid resolved decisions from the completion gate + +Observed 2026-08-13 in the main home, with `.tasks.toml` setting `done_keep = 10` and `archive = "data/done-archive.md"`. +Seventeen `resolve` calls landed successfully, then the completion gate refused. + +```text +$ bin/fm-decision-hold.sh complete emotion-scope-division +fm-decision-hold: captain decision emotion-scope-division-decision-boundary-position is absent from .../data/backlog.md +``` + +The decision was not absent. +It had been resolved correctly, with its full resolution body, digest, and routed identities intact, and retention had rotated it out of `data/backlog.md` into `data/done-archive.md`. +Confirmed by hand: `tasks-axi show emotion-scope-division-decision-boundary-position --full` exited 1, while the same id was present in `data/done-archive.md` with its complete resolution content. + +Because `verify` reads the same inventory, `bin/fm-teardown.sh` could not clean the investigation up either. +The failure was silent until the gate refused, and the natural workaround - forcing past the refusal - is exactly what the gate exists to prevent. +Any session resolving more than `done_keep` decisions at once reproduces it. + +The defect and the fix were reproduced at the reported scale in a synthetic home using the same `.tasks.toml`: seventeen holds registered, routed, and resolved, leaving 10 resolved decisions in the active backlog and 7 in the archive. + +```text +$ tasks-axi show incident-scope-review-decision-choice-1 --full +error: "Task \"incident-scope-review-decision-choice-1\" not found in this backlog" +code: NOT_FOUND + +$ bin/fm-decision-hold.sh complete incident-scope-review choice-1 ... choice-17 # before +fm-decision-hold: captain decision incident-scope-review-decision-choice-1 is absent from /tmp/inc/data/backlog.md + +$ bin/fm-decision-hold.sh complete incident-scope-review choice-1 ... choice-17 # after +complete: incident-scope-review decision inventory reviewed (choice-1,choice-10,...,choice-9) + +$ bin/fm-decision-hold.sh verify incident-scope-review # after +verified: incident-scope-review unresolved-decision inventory +``` + +`tasks-axi` 0.2.5 exposes no archive query: `show` and `list` read one backlog file, and `--file` pointed at the configured archive is refused with `Archive path must not be the active backlog path`. +The fallback therefore queries a private throwaway snapshot of the archive whose `## Archived ` headings are normalized to `## Done`, which keeps tasks-axi's own parser as the only record parser instead of hand-parsing markdown. +Verified against 0.2.5: a normalized snapshot returns the identical `show --full` field set for an archived resolved record, including the full `body`, and an archived still-open hold stays unparseable there, so it cannot masquerade as resolved. + ## Verification record Verification date: 2026-07-14. Additional quoted `blocked_by` regression verification date: 2026-07-17. Plural blocker-readiness and mixed-home projection verification date: 2026-07-22. +Done-archive lookup regression verification date: 2026-08-13, with ShellCheck 0.11.0 and tasks-axi 0.2.5. +Two backend scripts, `tests/fm-backend-orca.test.sh` and `tests/fm-backend.test.sh`, failed on that date both on the change branch and on the unmodified base, so they are pre-existing and unrelated to the archive lookup. The focused end-to-end regression uses only synthetic `sample` identities and decision text. It begins with a completed investigation and visual review whose genuine unresolved choice exists only in the report. The initial Bearings snapshot correctly has no open decision, and the new teardown gate refuses to erase the source. A later regression covers tasks-axi's quoted multi-entry `blocked_by` output so `resolve` matches the first, middle, and last ids and rejects a genuinely absent id. +The Done-archive regression drives the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and asserts all four boundaries: a resolved archived decision passes, an archived record stripped of its resolution markers still refuses, an open hold present only in the archive satisfies neither the active-hold check nor completion, and an absent, missing, or corrupt archive refuses exactly as before instead of passing. The final verification commands and their exact summarized outputs follow. @@ -62,6 +110,8 @@ ok - resolved findings and decision-like prose do not create false holds ok - terminal single-owner stale status decisions do not block empty inventory ok - main-home and secondmate-home captain holds remain correctly routed ok - resolve matches first/middle/last in quoted blocked_by and rejects a genuinely absent id +ok - a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse +ok - an absent, missing, or corrupt archive refuses exactly as before rather than passing $ bash tests/fm-fleet-snapshot-view.test.sh ok - backlog normalization preserves strict roles and resolves every blocker compatibly @@ -86,6 +136,12 @@ fm-lint.sh: ShellCheck 0.11.0 (pinned 0.11.0) $ git diff --check (no output) +$ bash tests/fm-backend-orca.test.sh # fails identically on the unmodified base +not ok - Orca spawn should fail when metadata cannot be written + +$ bash tests/fm-backend.test.sh # fails identically on the unmodified base +not ok - fm-send --key: old vs new exit code: expected exit 1, got 0 + $ for test_script in tests/*.test.sh; do bash "$test_script"; done ALL 71 TEST SCRIPTS PASSED ``` diff --git a/tests/fm-decision-hold-lifecycle.test.sh b/tests/fm-decision-hold-lifecycle.test.sh index 0ef84c4a6f5..d6c95c35494 100755 --- a/tests/fm-decision-hold-lifecycle.test.sh +++ b/tests/fm-decision-hold-lifecycle.test.sh @@ -550,6 +550,181 @@ test_resolve_matches_quoted_blocked_by_edges() { pass "resolve matches first/middle/last in quoted blocked_by and rejects a genuinely absent id" } +# The backlog's own retention rotates closed items into the configured archive, and +# `tasks-axi show` reads only the active backlog file. Before the archive fallback, +# a session that resolved more decisions than done_keep locked its own investigations +# open: `complete` and `verify` reported the resolved decision "absent from +# .../data/backlog.md", and teardown could not clean up either. +# +# The gate must find a durably-resolved decision in the archive while keeping every +# other guarantee: an archived record without the resolution markers still refuses, +# an OPEN hold present only in the archive never satisfies the active-hold check, +# and a home with no configured archive behaves exactly as before. +test_resolved_decision_in_done_archive_satisfies_the_gate() { + local home origin hold archive keep_hold show + home=$(make_home archived-resolution) + origin=sample-archive-review + mkdir -p "$home/data/$origin" + archive="$home/data/done-archive.md" + + tasks_in "$home" add "$origin" "Investigate archived sample decisions" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create archived-decision origin" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# Archive review\n\nOne captain choice was resolved and then archived.\n' \ + > "$home/data/$origin/report.md" + + hold=$(run_decisions "$home" hold "$origin" rotation \ + --title "Choose the sample rotation" --reason "captain rotation choice pending" --repo sample) \ + || fail "could not register the rotation hold" + run_decisions "$home" complete "$origin" rotation >/dev/null \ + || fail "completion failed while the hold was still live" + + tasks_in "$home" add sample-rotation-work "Apply the selected sample rotation" \ + --kind ship --repo sample --blocked-by "$hold" >/dev/null \ + || fail "could not create dependent rotation work" + printf 'Rotate the sample clockwise.\n' > "$home/rotation-decision.txt" + run_decisions "$home" resolve "$origin" rotation \ + --decision-file "$home/rotation-decision.txt" --routed-to sample-rotation-work >/dev/null \ + || fail "could not resolve the rotation decision" + + # Force the exact retention rotation that hid the resolved decision, using the + # backlog's own archiving path rather than hand-moving the record. + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the resolved decision through backlog retention" + if tasks_in "$home" show "$hold" --full >/dev/null 2>&1; then + fail "retention fixture did not remove the resolved decision from the active backlog" + fi + assert_grep "$hold" "$archive" "retention fixture did not archive the resolved decision" + assert_grep "Resolution recorded by fm-decision-hold." "$archive" \ + "archived record lost its resolution body" + + run_decisions "$home" complete "$origin" rotation >/dev/null 2> "$home/archived-complete.err" \ + || fail "completion refused a decision durably resolved in the archive: $(cat "$home/archived-complete.err")" + run_decisions "$home" verify "$origin" >/dev/null 2> "$home/archived-verify.err" \ + || fail "verification refused a decision durably resolved in the archive: $(cat "$home/archived-verify.err")" + run_teardown "$home" "$origin" >/dev/null 2> "$home/archived-teardown.err" \ + || fail "teardown refused an investigation whose decisions are archived: $(cat "$home/archived-teardown.err")" + + # An archived record must clear the same resolution bar an active record clears: + # mere presence of the identity in the archive is not durable resolution. + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + perl -0pi -e 's/^ Resolution recorded by fm-decision-hold\.\n//m' "$archive" \ + || fail "could not strip the archived resolution marker" + assert_no_grep "Resolution recorded by fm-decision-hold." "$archive" \ + "stripped fixture still carries the resolution marker" + if run_decisions "$home" verify "$origin" \ + > "$home/stripped-verify.out" 2> "$home/stripped-verify.err"; then + fail "verification accepted an archived record with no resolution body" + fi + if run_decisions "$home" complete "$origin" rotation \ + > "$home/stripped-complete.out" 2> "$home/stripped-complete.err"; then + fail "completion accepted an archived record with no resolution body" + fi + + # An OPEN hold is a live-backlog fact. Finding one only in the archive must never + # satisfy the active-hold check that guards `resolve`. + keep_hold=$(run_decisions "$home" hold "$origin" retention \ + --title "Choose the sample retention" --reason "captain retention choice pending" --repo sample) \ + || fail "could not register the retention hold" + tasks_in "$home" prune --keep 0 --state queued >/dev/null \ + || fail "could not archive the still-open retention hold" + if tasks_in "$home" show "$keep_hold" --full >/dev/null 2>&1; then + fail "queued-retention fixture did not remove the open hold from the active backlog" + fi + assert_grep "$keep_hold" "$archive" "queued-retention fixture did not archive the open hold" + tasks_in "$home" add sample-retention-work "Apply the selected sample retention" \ + --kind ship --repo sample >/dev/null \ + || fail "could not create dependent retention work" + printf 'Keep the sample for one cycle.\n' > "$home/retention-decision.txt" + if run_decisions "$home" resolve "$origin" retention \ + --decision-file "$home/retention-decision.txt" --routed-to sample-retention-work \ + > "$home/archived-open.out" 2> "$home/archived-open.err"; then + fail "resolve satisfied its active-hold check from the archive" + fi + assert_grep "absent from" "$home/archived-open.err" \ + "an open hold found only in the archive must refuse as absent from the live backlog" + if run_decisions "$home" complete "$origin" retention \ + > "$home/archived-open-complete.out" 2> "$home/archived-open-complete.err"; then + fail "completion accepted an open hold that exists only in the archive" + fi + + pass "a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse" +} + +# An absent or unset archive key must behave exactly as before the fallback existed: +# refuse a genuinely missing decision, without crashing and without silently passing. +test_absent_archive_config_behaves_as_before() { + local home origin + home=$(make_home no-archive-config) + origin=sample-no-archive-review + mkdir -p "$home/data/$origin" + cat > "$home/.tasks.toml" <<'EOF' +backend = "markdown" + +[markdown] +path = "data/backlog.md" +done_keep = 10 +EOF + tasks_in "$home" add "$origin" "Investigate without an archive" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create no-archive origin" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# No-archive review\n\nOne captain choice is claimed but absent.\n' \ + > "$home/data/$origin/report.md" + + if run_decisions "$home" complete "$origin" ghost \ + > "$home/no-archive.out" 2> "$home/no-archive.err"; then + fail "completion passed a genuinely absent decision when no archive is configured" + fi + assert_grep "absent from" "$home/no-archive.err" \ + "an absent archive key must refuse with the ordinary absence error" + assert_no_grep "decisions_reviewed=1" "$home/state/$origin.meta" \ + "refused completion recorded a false attestation with no archive configured" + + # A configured-but-missing archive file is a legitimate absence too. + cat > "$home/.tasks.toml" <<'EOF' +backend = "markdown" + +[markdown] +path = "data/backlog.md" +archive = "data/done-archive.md" +done_keep = 10 +EOF + assert_absent "$home/data/done-archive.md" "fixture must start with no archive file" + if run_decisions "$home" complete "$origin" ghost \ + > "$home/missing-archive.out" 2> "$home/missing-archive.err"; then + fail "completion passed an absent decision when the archive file does not exist" + fi + assert_grep "absent from" "$home/missing-archive.err" \ + "a missing archive file must refuse with the ordinary absence error" + + # An empty archive unambiguously holds no records, so it is an absence too. + : > "$home/data/done-archive.md" + if run_decisions "$home" complete "$origin" ghost \ + > "$home/empty-archive.out" 2> "$home/empty-archive.err"; then + fail "completion passed an absent decision when the archive file is empty" + fi + assert_grep "absent from" "$home/empty-archive.err" \ + "an empty archive file must refuse with the ordinary absence error" + + # A corrupt archive is not the same thing as an absence and must not read as one. + printf 'not a backlog\000at all\n' > "$home/data/done-archive.md" + if run_decisions "$home" complete "$origin" ghost \ + > "$home/corrupt-archive.out" 2> "$home/corrupt-archive.err"; then + fail "completion passed while the configured archive was unreadable as a backlog" + fi + assert_grep "archive" "$home/corrupt-archive.err" \ + "a corrupt archive must name the archive as the refusal reason" + assert_no_grep "decisions_reviewed=1" "$home/state/$origin.meta" \ + "a corrupt archive recorded a false completion attestation" + + pass "an absent, missing, or corrupt archive refuses exactly as before rather than passing" +} + test_uninventoried_report_decision_refuses_completion test_scout_teardown_always_requires_inventory_verification @@ -560,3 +735,5 @@ test_none_inventory_and_resolved_prose_do_not_create_holds test_terminal_single_owner_status_decision_does_not_block_empty_inventory test_secondmate_hold_stays_in_authoritative_home test_resolve_matches_quoted_blocked_by_edges +test_resolved_decision_in_done_archive_satisfies_the_gate +test_absent_archive_config_behaves_as_before From 335aebf2d93ef0c8f87c845605ca159738184268 Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Thu, 13 Aug 2026 15:21:10 -0400 Subject: [PATCH 2/6] no-mistakes(review): make archived decision lookup order-independent, fix vacuous test assertions --- bin/fm-decision-hold.sh | 88 ++++++++++++----- tests/fm-decision-hold-lifecycle.test.sh | 119 ++++++++++++++++++++++- 2 files changed, 179 insertions(+), 28 deletions(-) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index cee0dbd67ca..e304d0a5515 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -53,11 +53,15 @@ # The archive path comes from `.tasks.toml`'s [markdown] archive key, resolved the # way tasks-axi resolves it, relative to FM_HOME. An absent config file or absent # archive key means there is no archive to consult. Because `tasks-axi show -# --file` refuses the configured archive path itself, the lookup queries a -# private throwaway snapshot whose `## Archived ` headings are normalized -# to `## Done`, which keeps tasks-axi's own parser as the only record parser. A -# missing archive is an ordinary absence; an unreadable, non-regular, non-text, or -# structurally unrecognizable archive refuses instead of reading as absence. +# --file` refuses the configured archive path itself, and because `show` returns +# only the first record carrying an id while retention can archive the same +# identity more than once, the lookup stages one private throwaway single-record +# snapshot per archived record and queries each under a `## Done` heading. That +# keeps tasks-axi's own parser as the only record parser, and it makes the answer +# independent of archive ordering: the id is durably resolved when any archived +# record for it clears the resolution bar. A missing archive is an ordinary +# absence; an unreadable, non-regular, non-text, or structurally unrecognizable +# archive refuses instead of reading as absence. set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -180,13 +184,19 @@ load_archive_path() { } ARCHIVE_SHOW='' -# Sets ARCHIVE_SHOW to a `tasks-axi show --full` record for read from the Done -# archive and returns 0, or clears it and returns 1 when this home has no archive -# or the archive holds no such record. Refuses rather than reporting absence when -# an archive exists but cannot be read as a backlog file. +ARCHIVE_RESOLVED=0 +# Returns 0 when the Done archive holds at least one record carrying , and 1 +# when this home has no archive or the archive holds no such record. Retention can +# archive the same identity more than once and `tasks-axi show` reports only the +# first match, so every archived record for the id is examined: ARCHIVE_RESOLVED +# is 1 when any of them clears record_is_resolved, which makes the answer +# independent of the order the records sit in the archive. ARCHIVE_SHOW carries the +# record that decided the answer. Refuses rather than reporting absence when an +# archive exists but cannot be read as a backlog file. load_archive_show() { # - local id=$1 archive snapshot rc + local id=$1 archive snapdir snapshot show rc ARCHIVE_SHOW='' + ARCHIVE_RESOLVED=0 load_archive_path archive=$ARCHIVE_PATH [ -n "$archive" ] || return 1 @@ -200,21 +210,49 @@ load_archive_show() { # || fail "the backlog archive is not a text backlog file: $archive" grep -q '^##[[:space:]]' "$archive" \ || fail "the backlog archive has no recognizable backlog sections: $archive" - snapshot=$(mktemp "${TMPDIR:-/tmp}/fm-decision-hold-archive.XXXXXX") \ + snapdir=$(mktemp -d "${TMPDIR:-/tmp}/fm-decision-hold-archive.XXXXXX") \ || fail "could not stage a read-only snapshot of $archive" - if ! sed 's/^##[[:space:]]*Archived\([[:space:]].*\)*$/## Done/' "$archive" > "$snapshot"; then - rm -f "$snapshot" + # Split on top-level record lines only. A record body is always indented, so an + # unindented `- [` line is a record boundary and never body prose. Each candidate + # block is staged alone under a `## Done` heading and handed to tasks-axi, which + # stays the only parser of the record itself: this decides where records start, + # never what they mean. An archived still-open `- [ ]` hold remains unparseable + # under a Done heading, so it cannot masquerade as resolved. + if ! awk -v id="$id" -v dir="$snapdir" ' + /^##[[:space:]]/ { if (out != "") { close(out); out = "" } ; next } + /^-[[:space:]]*\[/ { + if (out != "") { close(out); out = "" } + head = $0 + sub(/^-[[:space:]]*\[[^\]]*\][[:space:]]*/, "", head) + split(head, parts, /[[:space:]]/) + if (parts[1] == id) { + n++ + out = dir "/record-" n ".md" + print "## Done" > out + print $0 > out + } + next + } + out != "" { print > out } + ' "$archive"; then + rm -rf "$snapdir" fail "could not stage a read-only snapshot of $archive" fi - set +e - ARCHIVE_SHOW=$(tasks_axi show "$id" --file "$snapshot" --full 2>/dev/null) - rc=$? - set -e - rm -f "$snapshot" - if [ "$rc" -ne 0 ]; then - ARCHIVE_SHOW='' - return 1 - fi + for snapshot in "$snapdir"/record-*.md; do + [ -f "$snapshot" ] || continue + set +e + show=$(tasks_axi show "$id" --file "$snapshot" --full 2>/dev/null) + rc=$? + set -e + [ "$rc" -eq 0 ] || continue + ARCHIVE_SHOW=$show + if record_is_resolved "$show"; then + ARCHIVE_RESOLVED=1 + break + fi + done + rm -rf "$snapdir" + [ -n "$ARCHIVE_SHOW" ] || return 1 return 0 } @@ -314,9 +352,11 @@ verify_hold_durable() { # fi # Absent from the live backlog: retention may have rotated a resolved decision # into the Done archive. Only a resolved record counts there, and it must carry - # the same resolution body an active record must carry. + # the same resolution body an active record must carry. Retention can archive one + # identity repeatedly, so any archived record clearing that bar resolves the id + # regardless of where it sits among the others. if load_archive_show "$id"; then - record_is_resolved "$ARCHIVE_SHOW" && return 0 + [ "$ARCHIVE_RESOLVED" = 1 ] && return 0 fail "archived captain decision $id has no durable resolution record" fi fail "captain decision $id is absent from $FM_HOME/data/backlog.md" diff --git a/tests/fm-decision-hold-lifecycle.test.sh b/tests/fm-decision-hold-lifecycle.test.sh index d6c95c35494..ea25522094f 100755 --- a/tests/fm-decision-hold-lifecycle.test.sh +++ b/tests/fm-decision-hold-lifecycle.test.sh @@ -609,7 +609,12 @@ test_resolved_decision_in_done_archive_satisfies_the_gate() { # An archived record must clear the same resolution bar an active record clears: # mere presence of the identity in the archive is not durable resolution. + # + # Teardown deleted the origin metadata, so the completed inventory is restored + # before this `verify`: without it verify refuses at "has no completed + # unresolved-decision inventory" and never reaches the archive lookup at all. write_origin_meta "$home" "$origin" + printf 'decisions_reviewed=1\ndecision_keys=rotation\n' >> "$home/state/$origin.meta" printf 'done: report complete\n' > "$home/state/$origin.status" perl -0pi -e 's/^ Resolution recorded by fm-decision-hold\.\n//m' "$archive" \ || fail "could not strip the archived resolution marker" @@ -619,10 +624,18 @@ test_resolved_decision_in_done_archive_satisfies_the_gate() { > "$home/stripped-verify.out" 2> "$home/stripped-verify.err"; then fail "verification accepted an archived record with no resolution body" fi + # The archive-specific refusal proves the archive lookup actually ran and judged + # the record, rather than an earlier gate refusing for an unrelated reason. + assert_grep "archived captain decision $hold has no durable resolution record" \ + "$home/stripped-verify.err" \ + "verification must refuse the archived record on its missing resolution body" if run_decisions "$home" complete "$origin" rotation \ > "$home/stripped-complete.out" 2> "$home/stripped-complete.err"; then fail "completion accepted an archived record with no resolution body" fi + assert_grep "archived captain decision $hold has no durable resolution record" \ + "$home/stripped-complete.err" \ + "completion must refuse the archived record on its missing resolution body" # An OPEN hold is a live-backlog fact. Finding one only in the archive must never # satisfy the active-hold check that guards `resolve`. @@ -654,12 +667,107 @@ test_resolved_decision_in_done_archive_satisfies_the_gate() { pass "a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse" } +# Archives one durably-resolved copy and one closed-unresolved copy of the SAME +# decision identity into 's Done archive, in the requested order, driving the +# real lifecycle: retention rotates a resolved decision out, the same key can then +# be held again because the live backlog no longer carries it, and that second hold +# can be closed without a resolution and rotated out in turn. +archive_duplicate_identity() { # + local home=$1 origin=$2 key=$3 order=$4 hold work + work="sample-$key-work" + + if [ "$order" = unresolved-first ]; then + hold=$(run_decisions "$home" hold "$origin" "$key" \ + --title "Choose the sample $key" --reason "captain $key choice pending" --repo sample) \ + || fail "could not register the first $key hold" + tasks_in "$home" "done" "$hold" >/dev/null \ + || fail "could not close the $key hold without a resolution" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the unresolved $key copy" + fi + + hold=$(run_decisions "$home" hold "$origin" "$key" \ + --title "Choose the sample $key" --reason "captain $key choice pending" --repo sample) \ + || fail "could not register the resolvable $key hold" + tasks_in "$home" add "$work" "Apply the selected sample $key" \ + --kind ship --repo sample --blocked-by "$hold" >/dev/null \ + || fail "could not create dependent $key work" + printf 'Take the %s branch.\n' "$key" > "$home/$key-decision.txt" + run_decisions "$home" resolve "$origin" "$key" \ + --decision-file "$home/$key-decision.txt" --routed-to "$work" >/dev/null \ + || fail "could not resolve the $key decision" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the resolved $key copy" + + if [ "$order" = resolved-first ]; then + hold=$(run_decisions "$home" hold "$origin" "$key" \ + --title "Choose the sample $key" --reason "captain $key choice pending" --repo sample) \ + || fail "could not re-register the $key hold after its resolution was archived" + tasks_in "$home" "done" "$hold" >/dev/null \ + || fail "could not close the second $key hold without a resolution" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the unresolved $key copy" + fi + + if tasks_in "$home" show "$hold" --full >/dev/null 2>&1; then + fail "duplicate fixture left a $key copy in the active backlog" + fi + printf '%s\n' "$hold" +} + +# `tasks-axi show` reports only the FIRST record carrying an id, so a single show +# call against the archive let record ordering decide the gate's answer: the same +# two archived copies of one identity passed when the resolved copy happened to +# come first and refused when it came second. The lookup must examine every +# archived record for the id, so both orderings answer identically. +test_duplicate_archived_identity_is_order_independent() { + local home origin hold order rc + for order in resolved-first unresolved-first; do + home=$(make_home "duplicate-$order") + origin="sample-dup-$order-review" + mkdir -p "$home/data/$origin" + tasks_in "$home" add "$origin" "Investigate duplicated sample decisions" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create duplicate-identity origin ($order)" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# Duplicate review\n\nOne captain choice was resolved and archived twice.\n' \ + > "$home/data/$origin/report.md" + + hold=$(archive_duplicate_identity "$home" "$origin" branch "$order") + [ "$(grep -c -F -- "- [x] $hold - " "$home/data/done-archive.md")" = 2 ] \ + || fail "duplicate fixture did not archive two copies of $hold ($order)" + + set +e + run_decisions "$home" complete "$origin" branch \ + > "$home/dup-complete.out" 2> "$home/dup-complete.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "completion answered differently for $order ordering: $(cat "$home/dup-complete.err")" + + printf 'decisions_reviewed=1\ndecision_keys=branch\n' >> "$home/state/$origin.meta" + set +e + run_decisions "$home" verify "$origin" \ + > "$home/dup-verify.out" 2> "$home/dup-verify.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "verification answered differently for $order ordering: $(cat "$home/dup-verify.err")" + done + + pass "a duplicated archived identity resolves the same way in either ordering" +} + # An absent or unset archive key must behave exactly as before the fallback existed: # refuse a genuinely missing decision, without crashing and without silently passing. test_absent_archive_config_behaves_as_before() { local home origin - home=$(make_home no-archive-config) - origin=sample-no-archive-review + # The home directory name is embedded in refusal messages, so it deliberately + # avoids the substring "archive": otherwise an assertion on that word would match + # any refusal that merely names a path under this home. + home=$(make_home unconfigured-retention) + origin=sample-retention-only-review mkdir -p "$home/data/$origin" cat > "$home/.tasks.toml" <<'EOF' backend = "markdown" @@ -717,8 +825,10 @@ EOF > "$home/corrupt-archive.out" 2> "$home/corrupt-archive.err"; then fail "completion passed while the configured archive was unreadable as a backlog" fi - assert_grep "archive" "$home/corrupt-archive.err" \ - "a corrupt archive must name the archive as the refusal reason" + assert_grep "is not a text backlog file" "$home/corrupt-archive.err" \ + "a corrupt archive must refuse as unreadable rather than as an ordinary absence" + assert_no_grep "absent from" "$home/corrupt-archive.err" \ + "a corrupt archive must not refuse as if the decision were merely absent" assert_no_grep "decisions_reviewed=1" "$home/state/$origin.meta" \ "a corrupt archive recorded a false completion attestation" @@ -736,4 +846,5 @@ test_terminal_single_owner_status_decision_does_not_block_empty_inventory test_secondmate_hold_stays_in_authoritative_home test_resolve_matches_quoted_blocked_by_edges test_resolved_decision_in_done_archive_satisfies_the_gate +test_duplicate_archived_identity_is_order_independent test_absent_archive_config_behaves_as_before From c0f53250c3bcf17b882426ebe3b7103dea948e27 Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Thu, 13 Aug 2026 16:31:06 -0400 Subject: [PATCH 3/6] no-mistakes(review): consult archive when live decision record is stale --- bin/fm-decision-hold.sh | 50 +++++++------ docs/decision-hold-lifecycle.md | 34 ++++++++- tests/fm-decision-hold-lifecycle.test.sh | 89 ++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 24 deletions(-) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index e304d0a5515..023d15b3116 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -45,10 +45,14 @@ # `tasks-axi show` reads only the active file. A durably-resolved captain # decision is therefore looked up in the active backlog first and in that archive # second, so a session that resolves more decisions than done_keep can still -# complete, verify, and tear down its own investigations. Only a resolved record -# is accepted from the archive, and it must carry the same resolution body an -# active record must carry. An ACTIVE hold is never satisfiable from the archive: -# verify_hold_active reads the live backlog alone. +# complete, verify, and tear down its own investigations. The archive is consulted +# whenever the live backlog does not itself settle the question - both when no live +# record exists and when one exists but is neither an active captain hold nor a +# durable resolution - so the answer never depends on where retention happens to +# have left the records. Only a resolved record is accepted from the archive, and +# it must carry the same resolution body an active record must carry. An ACTIVE +# hold is never satisfiable from the archive: verify_hold_active reads the live +# backlog alone. # # The archive path comes from `.tasks.toml`'s [markdown] archive key, resolved the # way tasks-axi resolves it, relative to FM_HOME. An absent config file or absent @@ -183,19 +187,18 @@ load_archive_path() { esac } -ARCHIVE_SHOW='' ARCHIVE_RESOLVED=0 # Returns 0 when the Done archive holds at least one record carrying , and 1 # when this home has no archive or the archive holds no such record. Retention can # archive the same identity more than once and `tasks-axi show` reports only the -# first match, so every archived record for the id is examined: ARCHIVE_RESOLVED -# is 1 when any of them clears record_is_resolved, which makes the answer -# independent of the order the records sit in the archive. ARCHIVE_SHOW carries the -# record that decided the answer. Refuses rather than reporting absence when an -# archive exists but cannot be read as a backlog file. +# first match, so every archived record for the id is examined: ARCHIVE_RESOLVED, +# the only value a caller reads, is 1 when any of them clears record_is_resolved, +# which makes the answer independent of the order the records sit in the archive. +# Finding a record and finding a resolved one are distinct: the return code reports +# the first, ARCHIVE_RESOLVED the second. Refuses rather than reporting absence +# when an archive exists but cannot be read as a backlog file. load_archive_show() { # - local id=$1 archive snapdir snapshot show rc - ARCHIVE_SHOW='' + local id=$1 archive snapdir snapshot show rc found=0 ARCHIVE_RESOLVED=0 load_archive_path archive=$ARCHIVE_PATH @@ -245,14 +248,14 @@ load_archive_show() { # rc=$? set -e [ "$rc" -eq 0 ] || continue - ARCHIVE_SHOW=$show + found=1 if record_is_resolved "$show"; then ARCHIVE_RESOLVED=1 break fi done rm -rf "$snapdir" - [ -n "$ARCHIVE_SHOW" ] || return 1 + [ "$found" = 1 ] || return 1 return 0 } @@ -338,8 +341,9 @@ verify_hold_resolved() { # } verify_hold_durable() { # - local id=$1 show state held kind hold_kind + local id=$1 show state held kind hold_kind live=0 if show=$(task_show "$id"); then + live=1 state=$(show_field "$show" state) held=$(show_field "$show" held) kind=$(show_field "$show" kind) @@ -348,17 +352,21 @@ verify_hold_durable() { # return 0 fi record_is_resolved "$show" && return 0 - fail "captain decision $id is neither actively held nor durably resolved" fi - # Absent from the live backlog: retention may have rotated a resolved decision - # into the Done archive. Only a resolved record counts there, and it must carry - # the same resolution body an active record must carry. Retention can archive one - # identity repeatedly, so any archived record clearing that bar resolves the id - # regardless of where it sits among the others. + # The live record is absent, or exists but is neither an active captain hold nor a + # durable resolution. Either way retention may hold a resolved copy of this + # identity in the Done archive, so the archive is consulted before refusing: + # otherwise the gate's answer would depend on where retention happens to have left + # the records. Only a resolved record counts there, and it must carry the same + # resolution body an active record must carry. Retention can archive one identity + # repeatedly, so any archived record clearing that bar resolves the id regardless + # of where it sits among the others. if load_archive_show "$id"; then [ "$ARCHIVE_RESOLVED" = 1 ] && return 0 fail "archived captain decision $id has no durable resolution record" fi + [ "$live" = 0 ] \ + || fail "captain decision $id is neither actively held nor durably resolved" fail "captain decision $id is absent from $FM_HOME/data/backlog.md" } diff --git a/docs/decision-hold-lifecycle.md b/docs/decision-hold-lifecycle.md index b6acf30a8aa..bbd678c3450 100644 --- a/docs/decision-hold-lifecycle.md +++ b/docs/decision-hold-lifecycle.md @@ -79,23 +79,49 @@ $ bin/fm-decision-hold.sh verify incident-scope-review verified: incident-scope-review unresolved-decision inventory ``` +The real-world failure shape is confirmed live rather than only synthetic. +On 2026-08-13 two scout closeouts were refused by this exact defect. +The first printed `fm-decision-hold: captain decision ds5-build-vs-buy-decision-codegen-experiment-funding is absent from data/backlog.md`, followed by `REFUSED: scout task ds5-build-vs-buy has not passed the unresolved-decision completion gate.`, and `emotion-scope-division` was refused the same way. +Both holds were durably resolved and sat in `data/done-archive.md` marked `[x]` with `(done 2026-08-13)`, carrying full resolution bodies, digests, and routed identities. +The regression drives that same shape - a hold resolved, rotated into the archive by the backlog's own retention, and its origin scout torn down afterward - so it covers the real failure rather than a fabricated archive fixture. + `tasks-axi` 0.2.5 exposes no archive query: `show` and `list` read one backlog file, and `--file` pointed at the configured archive is refused with `Archive path must not be the active backlog path`. -The fallback therefore queries a private throwaway snapshot of the archive whose `## Archived ` headings are normalized to `## Done`, which keeps tasks-axi's own parser as the only record parser instead of hand-parsing markdown. +The fallback therefore stages one private throwaway single-record snapshot per archived record carrying the id, each under a `## Done` heading, and queries them through `tasks-axi show --file`. +That keeps tasks-axi's own parser as the only parser of a record instead of hand-parsing markdown: the split decides only where records start, using the fact that tasks-axi always indents body lines, so an unindented `- [` line is a boundary and never body prose. +Per-record staging is what makes the answer independent of archive ordering, because `show` reports only the first record carrying an id while retention can archive one identity more than once; the id counts as durably resolved when any archived record for it clears the shared bar. Verified against 0.2.5: a normalized snapshot returns the identical `show --full` field set for an archived resolved record, including the full `body`, and an archived still-open hold stays unparseable there, so it cannot masquerade as resolved. +The archive is consulted whenever the live backlog does not settle the question, not only when the live backlog has no record at all. +A live record that is neither an active captain hold nor a durable resolution therefore falls through to the archive instead of refusing outright. +Without that, a stale live copy of an identity whose resolution had already rotated into the archive made the gate refuse `neither actively held nor durably resolved` for a decision that was in fact durably resolved, and the identical facts passed once retention rotated the stale copy out too. +Retention position must not decide the answer. + +Two consequences are intentional and stated here rather than left for a reader to discover. +First, the fallback removes an implicit fail-closed on an unreadable live backlog: tasks-axi cannot distinguish a corrupt backlog from an empty one, so a corrupt `data/backlog.md` now lets `verify` succeed from the archived record alone. +That is the right answer, because an archived resolution record is genuine evidence that the decision was resolved, and the fail-closed requirement was scoped to the archive rather than to the live backlog. +Second, `command_hold`'s "already durably resolved; use a new decision key" guard still reads only the live backlog, so a resolved key can be re-held once its record has been archived. +Closing that would change when a decision key may be reopened, which is semantic policy owned by `.agents/skills/decision-hold-lifecycle/SKILL.md`, so it is a known related gap outside this change's scope. +It is also what lets the ordering and stale-record regressions build their fixtures through the real script instead of hand-writing archive files. + ## Verification record Verification date: 2026-07-14. Additional quoted `blocked_by` regression verification date: 2026-07-17. Plural blocker-readiness and mixed-home projection verification date: 2026-07-22. Done-archive lookup regression verification date: 2026-08-13, with ShellCheck 0.11.0 and tasks-axi 0.2.5. -Two backend scripts, `tests/fm-backend-orca.test.sh` and `tests/fm-backend.test.sh`, failed on that date both on the change branch and on the unmodified base, so they are pre-existing and unrelated to the archive lookup. +Two backend scripts fail for reasons that have nothing to do with the archive lookup, and they fail on unmodified base code with no part of this change present. +That was re-verified by cloning main at `4bf9c08` into a throwaway checkout and running the two suites there: `tests/fm-backend.test.sh` fails `not ok - fm-send --key: old vs new exit code: expected exit 1, got 0`, and `tests/fm-backend-orca.test.sh` fails `not ok - Orca spawn should fail when metadata cannot be written`. +Both are therefore pre-existing and not caused by this change. The focused end-to-end regression uses only synthetic `sample` identities and decision text. It begins with a completed investigation and visual review whose genuine unresolved choice exists only in the report. The initial Bearings snapshot correctly has no open decision, and the new teardown gate refuses to erase the source. A later regression covers tasks-axi's quoted multi-entry `blocked_by` output so `resolve` matches the first, middle, and last ids and rejects a genuinely absent id. -The Done-archive regression drives the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and asserts all four boundaries: a resolved archived decision passes, an archived record stripped of its resolution markers still refuses, an open hold present only in the archive satisfies neither the active-hold check nor completion, and an absent, missing, or corrupt archive refuses exactly as before instead of passing. +Three Done-archive regressions drive the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and together assert six boundaries. +`test_resolved_decision_in_done_archive_satisfies_the_gate` covers three: a resolved archived decision passes, an archived record stripped of its resolution markers still refuses with the archive-specific refusal, and an open hold present only in the archive satisfies neither the active-hold check nor completion. +`test_duplicate_archived_identity_is_order_independent` covers the ordering boundary, building both archive orderings of one duplicated identity through the real hold, resolve, and prune lifecycle and requiring the same answer from each. +`test_stale_live_record_still_consults_the_archive` covers the stale-live-record boundary, pairing an archived resolution with an unresolved live copy of the same identity and requiring that the answer not change when retention later rotates that live copy out. +`test_absent_archive_config_behaves_as_before` covers the last: an absent, missing, empty, or corrupt archive refuses exactly as before instead of passing, with the corrupt case asserted on its own `is not a text backlog file` refusal so it cannot be satisfied by an ordinary absence. The final verification commands and their exact summarized outputs follow. @@ -111,6 +137,8 @@ ok - terminal single-owner stale status decisions do not block empty inventory ok - main-home and secondmate-home captain holds remain correctly routed ok - resolve matches first/middle/last in quoted blocked_by and rejects a genuinely absent id ok - a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse +ok - a duplicated archived identity resolves the same way in either ordering +ok - a stale unresolved live record does not hide a durable resolution in the archive ok - an absent, missing, or corrupt archive refuses exactly as before rather than passing $ bash tests/fm-fleet-snapshot-view.test.sh diff --git a/tests/fm-decision-hold-lifecycle.test.sh b/tests/fm-decision-hold-lifecycle.test.sh index ea25522094f..33f14d63dd9 100755 --- a/tests/fm-decision-hold-lifecycle.test.sh +++ b/tests/fm-decision-hold-lifecycle.test.sh @@ -759,6 +759,94 @@ test_duplicate_archived_identity_is_order_independent() { pass "a duplicated archived identity resolves the same way in either ordering" } +# The archive fallback originally ran only when the live backlog held NO record for +# the id, so a stale live record short-circuited it: with a durably-resolved copy in +# the archive and a closed-unresolved copy of the SAME identity still in the live +# backlog's Done section, the gate refused "neither actively held nor durably +# resolved" even though the decision WAS durably resolved, and the very same facts +# passed once retention rotated the stale live copy out too. Retention position must +# not decide the answer, so the archive is consulted whenever the live backlog does +# not settle the question. +test_stale_live_record_still_consults_the_archive() { + local home origin hold work rc + home=$(make_home stale-live-record) + origin=sample-stale-live-review + mkdir -p "$home/data/$origin" + tasks_in "$home" add "$origin" "Investigate a stale live decision copy" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create stale-live-record origin" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# Stale live review\n\nOne captain choice was resolved, archived, then re-held.\n' \ + > "$home/data/$origin/report.md" + + hold=$(run_decisions "$home" hold "$origin" placement \ + --title "Choose the sample placement" --reason "captain placement choice pending" --repo sample) \ + || fail "could not register the placement hold" + work=sample-placement-work + tasks_in "$home" add "$work" "Apply the selected sample placement" \ + --kind ship --repo sample --blocked-by "$hold" >/dev/null \ + || fail "could not create dependent placement work" + printf 'Place the sample forward.\n' > "$home/placement-decision.txt" + run_decisions "$home" resolve "$origin" placement \ + --decision-file "$home/placement-decision.txt" --routed-to "$work" >/dev/null \ + || fail "could not resolve the placement decision" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the resolved placement copy" + + # The same key can be re-held once its resolution has left the live backlog, and + # that second hold can be closed with no resolution. This leaves the exact state + # the defect mishandled: a stale unresolved live record over an archived resolution. + run_decisions "$home" hold "$origin" placement \ + --title "Choose the sample placement" --reason "captain placement choice pending" --repo sample \ + >/dev/null || fail "could not re-hold the placement key after its resolution was archived" + tasks_in "$home" "done" "$hold" >/dev/null \ + || fail "could not close the second placement hold without a resolution" + tasks_in "$home" show "$hold" --full >/dev/null 2>&1 \ + || fail "stale-live fixture must leave an unresolved copy in the live backlog" + assert_grep "Resolution recorded by fm-decision-hold." "$home/data/done-archive.md" \ + "stale-live fixture must leave the resolved copy in the archive" + + set +e + run_decisions "$home" complete "$origin" placement \ + > "$home/stale-complete.out" 2> "$home/stale-complete.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "completion refused a decision resolved in the archive because a stale live copy existed: $(cat "$home/stale-complete.err")" + set +e + run_decisions "$home" verify "$origin" \ + > "$home/stale-verify.out" 2> "$home/stale-verify.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "verification refused a decision resolved in the archive because a stale live copy existed: $(cat "$home/stale-verify.err")" + + # Rotating the stale live copy out changes nothing about the decisions, so it must + # not change the answer either. + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not rotate the stale live copy out" + if tasks_in "$home" show "$hold" --full >/dev/null 2>&1; then + fail "retention fixture did not remove the stale live copy" + fi + set +e + run_decisions "$home" complete "$origin" placement \ + > "$home/rotated-complete.out" 2> "$home/rotated-complete.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "completion answered differently once the stale live copy rotated out: $(cat "$home/rotated-complete.err")" + set +e + run_decisions "$home" verify "$origin" \ + > "$home/rotated-verify.out" 2> "$home/rotated-verify.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "verification answered differently once the stale live copy rotated out: $(cat "$home/rotated-verify.err")" + + pass "a stale unresolved live record does not hide a durable resolution in the archive" +} + # An absent or unset archive key must behave exactly as before the fallback existed: # refuse a genuinely missing decision, without crashing and without silently passing. test_absent_archive_config_behaves_as_before() { @@ -847,4 +935,5 @@ test_secondmate_hold_stays_in_authoritative_home test_resolve_matches_quoted_blocked_by_edges test_resolved_decision_in_done_archive_satisfies_the_gate test_duplicate_archived_identity_is_order_independent +test_stale_live_record_still_consults_the_archive test_absent_archive_config_behaves_as_before From 183ea00634b549611b2ea23eb90a806b4791dd91 Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Fri, 14 Aug 2026 11:51:15 -0400 Subject: [PATCH 4/6] no-mistakes(review): narrow archive fall-through to settled live decision records --- bin/fm-decision-hold.sh | 45 +++++--- docs/decision-hold-lifecycle.md | 109 +++++++++++++++++-- tests/fm-decision-hold-lifecycle.test.sh | 127 ++++++++++++++++++++++- 3 files changed, 258 insertions(+), 23 deletions(-) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index 023d15b3116..a44da87b855 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -46,17 +46,26 @@ # decision is therefore looked up in the active backlog first and in that archive # second, so a session that resolves more decisions than done_keep can still # complete, verify, and tear down its own investigations. The archive is consulted -# whenever the live backlog does not itself settle the question - both when no live -# record exists and when one exists but is neither an active captain hold nor a -# durable resolution - so the answer never depends on where retention happens to -# have left the records. Only a resolved record is accepted from the archive, and -# it must carry the same resolution body an active record must carry. An ACTIVE -# hold is never satisfiable from the archive: verify_hold_active reads the live -# backlog alone. +# when the live backlog holds no record for the identity at all, and when it holds +# a SETTLED captain record - a closed one - that does not itself carry a durable +# resolution, because retention may have left the resolution in the archive +# instead. Retention position therefore never decides the answer for a settled +# decision. A live record that is still open - queued, in flight, held again, or +# otherwise unsettled - is an unanswered decision in its own right: it refuses, and +# no archived resolution can settle it, so a decision that was answered, archived, +# and then reopened gates completion again. Only a resolved record is accepted from +# the archive, and it must carry the same resolution body an active record must +# carry. An ACTIVE hold is never satisfiable from the archive: verify_hold_active +# reads the live backlog alone. # -# The archive path comes from `.tasks.toml`'s [markdown] archive key, resolved the -# way tasks-axi resolves it, relative to FM_HOME. An absent config file or absent -# archive key means there is no archive to consult. Because `tasks-axi show +# The archive path comes only from `.tasks.toml`'s [markdown] archive key, resolved +# relative to FM_HOME. When that key is absent this lookup is unavailable and the +# gate refuses exactly as it did before the fallback existed. tasks-axi 0.2.5 still +# archives to a default /done-archive.md when the key is unset, so a +# home that does not pin the key can still reproduce the original defect; that is a +# known and accepted limitation rather than a covered case. This repo's tracked +# `.tasks.toml` pins the key and the regression suite copies it into every +# synthetic home, so the shipped path is covered. Because `tasks-axi show # --file` refuses the configured archive path itself, and because `show` returns # only the first record carrying an id while retention can archive the same # identity more than once, the lookup stages one private throwaway single-record @@ -352,12 +361,18 @@ verify_hold_durable() { # return 0 fi record_is_resolved "$show" && return 0 + # An unsettled live record - queued, in flight, or held again - is an unanswered + # captain decision in its own right, whatever the archive remembers about an + # earlier answer. A decision that was resolved, archived, and then reopened must + # gate completion again, so this refuses without consulting the archive. + [ "$state" = "done" ] \ + || fail "captain decision $id has an open unresolved record in $FM_HOME/data/backlog.md (state=$state held=$held kind=$kind)" fi - # The live record is absent, or exists but is neither an active captain hold nor a - # durable resolution. Either way retention may hold a resolved copy of this - # identity in the Done archive, so the archive is consulted before refusing: - # otherwise the gate's answer would depend on where retention happens to have left - # the records. Only a resolved record counts there, and it must carry the same + # The live record is absent, or is a settled record that carries no durable + # resolution. Either way retention may hold a resolved copy of this identity in the + # Done archive, so the archive is consulted before refusing: otherwise the gate's + # answer for a settled decision would depend on where retention happens to have + # left the records. Only a resolved record counts there, and it must carry the same # resolution body an active record must carry. Retention can archive one identity # repeatedly, so any archived record clearing that bar resolves the id regardless # of where it sits among the others. diff --git a/docs/decision-hold-lifecycle.md b/docs/decision-hold-lifecycle.md index bbd678c3450..731bbdd129d 100644 --- a/docs/decision-hold-lifecycle.md +++ b/docs/decision-hold-lifecycle.md @@ -29,6 +29,9 @@ Only a resolved record is accepted from the archive, and it must carry the same An active hold is never satisfiable from the archive, because the separate active-hold check that guards `resolve` reads the live backlog alone. `bin/fm-decision-hold.sh`'s header and `--help` own the exact archive-path resolution, snapshot mechanics, and refusal conditions. +The lookup reads only the archive path pinned under `.tasks.toml`'s `[markdown] archive` key. +When that key is absent the fallback is unavailable and the gate refuses exactly as it did before, which is deliberate; the accepted limitation that follows from it is recorded below. + The `resolve` subcommand requires a decision file and at least one existing dependent task whose structured `blocked-by` edge points to the hold. It records the decision digest and routed task identities as a retry identity in the hold body, clears each dependency edge through tasks-axi, and marks the hold Done only after those writes succeed. An exact retry can finish a partial routing operation, while a changed decision or routed-task set is rejected. @@ -91,12 +94,100 @@ That keeps tasks-axi's own parser as the only parser of a record instead of hand Per-record staging is what makes the answer independent of archive ordering, because `show` reports only the first record carrying an id while retention can archive one identity more than once; the id counts as durably resolved when any archived record for it clears the shared bar. Verified against 0.2.5: a normalized snapshot returns the identical `show --full` field set for an archived resolved record, including the full `body`, and an archived still-open hold stays unparseable there, so it cannot masquerade as resolved. -The archive is consulted whenever the live backlog does not settle the question, not only when the live backlog has no record at all. -A live record that is neither an active captain hold nor a durable resolution therefore falls through to the archive instead of refusing outright. -Without that, a stale live copy of an identity whose resolution had already rotated into the archive made the gate refuse `neither actively held nor durably resolved` for a decision that was in fact durably resolved, and the identical facts passed once retention rotated the stale copy out too. -Retention position must not decide the answer. +The archive is consulted when the live backlog has no record for the identity at all, and when it has a SETTLED - closed - captain record that carries no durable resolution. +Without the second case, a stale closed live copy of an identity whose resolution had already rotated into the archive made the gate refuse `neither actively held nor durably resolved` for a decision that was in fact durably resolved, and the identical facts passed once retention rotated the stale copy out too. +Retention position must not decide the answer for a settled decision. + +The fall-through stops at settled records. +A live record that is still open - queued, in flight, or held again - is an unanswered captain decision in its own right, so it refuses on its own state and no archived resolution can satisfy it. +That keeps a decision that was answered, archived, and then reopened gating completion, verification, and teardown exactly as it did before the archive fallback existed. + +Observed 2026-08-14 in a synthetic home: before the fall-through was narrowed to settled records, a reopened decision stopped gating completion. +The reproduction resolves a decision, lets the backlog's own retention archive it, re-holds the same key, and then leaves that live copy unsettled. + +```text +$ bin/fm-decision-hold.sh hold sample-reopened-review pick --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample +$ bin/fm-decision-hold.sh resolve sample-reopened-review pick --decision-file pick-decision.txt --routed-to sample-pick-work +$ tasks-axi prune --keep 0 --state done +$ bin/fm-decision-hold.sh hold sample-reopened-review pick --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample +$ tasks-axi unhold sample-reopened-review-decision-pick + +$ tasks-axi show sample-reopened-review-decision-pick --full + state: queued + held: no + kind: captain + body: "Origin: sample-reopened-review\nDecision key: pick\nState: awaiting captain decision." + +$ bin/fm-decision-hold.sh complete sample-reopened-review pick # before narrowing +complete: sample-reopened-review decision inventory reviewed (pick) + # rc=0 + +$ bin/fm-decision-hold.sh complete sample-reopened-review pick # after narrowing +fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=queued held=no kind=captain) + # rc=1 +``` + +`tasks-axi start` in place of `unhold` reproduces it identically through a different live shape. + +```text +$ tasks-axi start sample-reopened-review-decision-pick +$ tasks-axi show sample-reopened-review-decision-pick --full + state: in_flight + held: yes + +$ bin/fm-decision-hold.sh complete sample-reopened-review pick # before narrowing +complete: sample-reopened-review decision inventory reviewed (pick) + # rc=0 + +$ bin/fm-decision-hold.sh complete sample-reopened-review pick # after narrowing +fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=in_flight held=yes kind=captain) + # rc=1 +``` + +Both refusals match what base `4bf9c08` did before any archive lookup existed, so teardown can no longer erase the source of a genuinely pending decision. +Settling that same reopened copy without a resolution restores the stale-record case, and the archive answers it again, so the narrowing does not revert the fix. + +```text +$ tasks-axi done sample-reopened-review-decision-pick +$ tasks-axi show sample-reopened-review-decision-pick --full + state: done + held: no + +$ bin/fm-decision-hold.sh complete sample-reopened-review pick # after narrowing +complete: sample-reopened-review decision inventory reviewed (pick) + # rc=0 +``` + +An accepted limitation, verified 2026-08-14 on tasks-axi 0.2.5: the lookup reads only the archive path pinned under `[markdown] archive`, while tasks-axi archives to a default `/done-archive.md` even when that key - or the whole `.tasks.toml` - is absent. +A home that does not pin the key therefore still reproduces the original defect. + +```text +$ cat .tasks.toml +backend = "markdown" + +[markdown] +path = "data/backlog.md" +done_keep = 10 + +$ tasks-axi prune --keep 0 --state done +ok: prune done -> archived 1 (kept 0) + +$ ls data/ +backlog.md +done-archive.md +sample-noarchivekey-review + +$ grep -c "Resolution recorded by fm-decision-hold." data/done-archive.md +1 + +$ bin/fm-decision-hold.sh complete sample-noarchivekey-review pick +fm-decision-hold: captain decision sample-noarchivekey-review-decision-pick is absent from .../data/backlog.md +``` + +That is accepted rather than fixed: honoring tasks-axi's own default archive location is out of this change's scope, and the absent-key path must keep refusing exactly as it did before. +This repo's tracked `.tasks.toml` pins the key, and the regression suite copies it into every synthetic home, so the shipped path is covered. -Two consequences are intentional and stated here rather than left for a reader to discover. +Two further consequences are intentional and stated here rather than left for a reader to discover. First, the fallback removes an implicit fail-closed on an unreadable live backlog: tasks-axi cannot distinguish a corrupt backlog from an empty one, so a corrupt `data/backlog.md` now lets `verify` succeed from the archived record alone. That is the right answer, because an archived resolution record is genuine evidence that the decision was resolved, and the fail-closed requirement was scoped to the archive rather than to the live backlog. Second, `command_hold`'s "already durably resolved; use a new decision key" guard still reads only the live backlog, so a resolved key can be re-held once its record has been archived. @@ -109,6 +200,7 @@ Verification date: 2026-07-14. Additional quoted `blocked_by` regression verification date: 2026-07-17. Plural blocker-readiness and mixed-home projection verification date: 2026-07-22. Done-archive lookup regression verification date: 2026-08-13, with ShellCheck 0.11.0 and tasks-axi 0.2.5. +Reopened-decision narrowing verification date: 2026-08-14, with the same ShellCheck 0.11.0 and tasks-axi 0.2.5. Two backend scripts fail for reasons that have nothing to do with the archive lookup, and they fail on unmodified base code with no part of this change present. That was re-verified by cloning main at `4bf9c08` into a throwaway checkout and running the two suites there: `tests/fm-backend.test.sh` fails `not ok - fm-send --key: old vs new exit code: expected exit 1, got 0`, and `tests/fm-backend-orca.test.sh` fails `not ok - Orca spawn should fail when metadata cannot be written`. Both are therefore pre-existing and not caused by this change. @@ -117,10 +209,12 @@ The focused end-to-end regression uses only synthetic `sample` identities and de It begins with a completed investigation and visual review whose genuine unresolved choice exists only in the report. The initial Bearings snapshot correctly has no open decision, and the new teardown gate refuses to erase the source. A later regression covers tasks-axi's quoted multi-entry `blocked_by` output so `resolve` matches the first, middle, and last ids and rejects a genuinely absent id. -Three Done-archive regressions drive the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and together assert six boundaries. +Five Done-archive regressions drive the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and together assert seven boundaries. `test_resolved_decision_in_done_archive_satisfies_the_gate` covers three: a resolved archived decision passes, an archived record stripped of its resolution markers still refuses with the archive-specific refusal, and an open hold present only in the archive satisfies neither the active-hold check nor completion. +Each of those three refusals is pinned to the identity under test, and the open-hold case resets the inventory to that key alone so which key refuses cannot depend on inventory sort order. `test_duplicate_archived_identity_is_order_independent` covers the ordering boundary, building both archive orderings of one duplicated identity through the real hold, resolve, and prune lifecycle and requiring the same answer from each. -`test_stale_live_record_still_consults_the_archive` covers the stale-live-record boundary, pairing an archived resolution with an unresolved live copy of the same identity and requiring that the answer not change when retention later rotates that live copy out. +`test_stale_live_record_still_consults_the_archive` covers the stale-live-record boundary, pairing an archived resolution with a closed unresolved live copy of the same identity and requiring that the answer not change when retention later rotates that live copy out. +`test_reopened_decision_is_not_settled_by_the_archive` covers the reopened boundary in both live shapes, `unhold` and `start`: completion, verification, and teardown each refuse on the live record's own open state, no false attestation is written, and settling that same copy without a resolution then passes from the archive so the narrowing is proved not to revert the stale-record fix. `test_absent_archive_config_behaves_as_before` covers the last: an absent, missing, empty, or corrupt archive refuses exactly as before instead of passing, with the corrupt case asserted on its own `is not a text backlog file` refusal so it cannot be satisfied by an ordinary absence. The final verification commands and their exact summarized outputs follow. @@ -139,6 +233,7 @@ ok - resolve matches first/middle/last in quoted blocked_by and rejects a genuin ok - a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse ok - a duplicated archived identity resolves the same way in either ordering ok - a stale unresolved live record does not hide a durable resolution in the archive +ok - a decision reopened after its answer was archived gates completion again ok - an absent, missing, or corrupt archive refuses exactly as before rather than passing $ bash tests/fm-fleet-snapshot-view.test.sh diff --git a/tests/fm-decision-hold-lifecycle.test.sh b/tests/fm-decision-hold-lifecycle.test.sh index 33f14d63dd9..097f6216453 100755 --- a/tests/fm-decision-hold-lifecycle.test.sh +++ b/tests/fm-decision-hold-lifecycle.test.sh @@ -657,12 +657,22 @@ test_resolved_decision_in_done_archive_satisfies_the_gate() { > "$home/archived-open.out" 2> "$home/archived-open.err"; then fail "resolve satisfied its active-hold check from the archive" fi - assert_grep "absent from" "$home/archived-open.err" \ + assert_grep "captain hold $keep_hold is absent from" "$home/archived-open.err" \ "an open hold found only in the archive must refuse as absent from the live backlog" + # The inventory is reset to the retention key alone so this refusal cannot be + # satisfied by the stripped rotation record examined above: which key refuses must + # not depend on the order the inventory happens to be sorted in. + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" if run_decisions "$home" complete "$origin" retention \ > "$home/archived-open-complete.out" 2> "$home/archived-open-complete.err"; then fail "completion accepted an open hold that exists only in the archive" fi + assert_grep "captain decision $keep_hold is absent from" \ + "$home/archived-open-complete.err" \ + "completion must refuse the open archived hold on its own absence from the live backlog" + assert_no_grep "$hold" "$home/archived-open-complete.err" \ + "the open-hold refusal must name the retention key, not the rotation key" pass "a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse" } @@ -847,6 +857,120 @@ test_stale_live_record_still_consults_the_archive() { pass "a stale unresolved live record does not hide a durable resolution in the archive" } +# The archive fallback must not let an archived answer settle a decision that is +# OPEN again in the live backlog. Retention rotates a resolved decision out, the same +# key can then be re-held, and an unheld or in-flight copy of it is a genuinely +# unanswered captain decision whatever the archive remembers. Before the fall-through +# was narrowed to settled live records, both of those states passed completion, so +# teardown could erase the source of a pending decision. Each state must refuse, and +# a SETTLED live copy carrying no resolution must still be satisfiable from the +# archive so the narrowing does not revert the fix this fallback exists to deliver. +test_reopened_decision_is_not_settled_by_the_archive() { + local home origin hold work rc state show + for state in unheld in-flight; do + home=$(make_home "reopened-$state") + origin="sample-reopened-$state-review" + mkdir -p "$home/data/$origin" + tasks_in "$home" add "$origin" "Investigate a reopened sample decision" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create reopened-decision origin ($state)" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# Reopened review\n\nOne captain choice was answered, archived, then reopened.\n' \ + > "$home/data/$origin/report.md" + + hold=$(run_decisions "$home" hold "$origin" pick \ + --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample) \ + || fail "could not register the pick hold ($state)" + work=sample-pick-work + tasks_in "$home" add "$work" "Apply the selected sample pick" \ + --kind ship --repo sample --blocked-by "$hold" >/dev/null \ + || fail "could not create dependent pick work ($state)" + printf 'Pick the sample front.\n' > "$home/pick-decision.txt" + run_decisions "$home" resolve "$origin" pick \ + --decision-file "$home/pick-decision.txt" --routed-to "$work" >/dev/null \ + || fail "could not resolve the pick decision ($state)" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the resolved pick copy ($state)" + assert_grep "Resolution recorded by fm-decision-hold." "$home/data/done-archive.md" \ + "reopened fixture must leave the resolved copy in the archive ($state)" + + # Reopening the key is possible once its resolution has left the live backlog. + # Each reopened shape is a live record that is NOT settled. + run_decisions "$home" hold "$origin" pick \ + --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample \ + >/dev/null || fail "could not re-hold the pick key after its resolution was archived ($state)" + if [ "$state" = unheld ]; then + tasks_in "$home" unhold "$hold" >/dev/null \ + || fail "could not drop the hold from the reopened pick record" + else + tasks_in "$home" start "$hold" >/dev/null \ + || fail "could not start the reopened pick record" + fi + show=$(tasks_in "$home" show "$hold" --full) \ + || fail "reopened fixture must leave a live copy in the backlog ($state)" + assert_not_contains "$show" "Resolution recorded by fm-decision-hold." \ + "reopened live copy must carry no resolution body ($state)" + if [ "$state" = unheld ]; then + assert_contains "$show" "state: queued" "unheld fixture must leave a queued record" + assert_contains "$show" "held: no" "unheld fixture must leave an unheld record" + else + assert_contains "$show" "state: in_flight" "in-flight fixture must leave an in_flight record" + fi + + if run_decisions "$home" complete "$origin" pick \ + > "$home/reopened-complete.out" 2> "$home/reopened-complete.err"; then + fail "completion accepted a reopened pending decision from the archive ($state)" + fi + assert_grep "captain decision $hold has an open unresolved record" \ + "$home/reopened-complete.err" \ + "completion must refuse a reopened decision on its own open live record ($state)" + assert_no_grep "decisions_reviewed=1" "$home/state/$origin.meta" \ + "refused completion recorded a false attestation for a reopened decision ($state)" + + printf 'decisions_reviewed=1\ndecision_keys=pick\n' >> "$home/state/$origin.meta" + if run_decisions "$home" verify "$origin" \ + > "$home/reopened-verify.out" 2> "$home/reopened-verify.err"; then + fail "verification accepted a reopened pending decision from the archive ($state)" + fi + assert_grep "captain decision $hold has an open unresolved record" \ + "$home/reopened-verify.err" \ + "verification must refuse a reopened decision on its own open live record ($state)" + if run_teardown "$home" "$origin" \ + > "$home/reopened-teardown.out" 2> "$home/reopened-teardown.err"; then + fail "teardown erased the source of a reopened pending decision ($state)" + fi + assert_present "$home/state/$origin.meta" \ + "refused teardown removed the metadata of a reopened pending decision ($state)" + + # Settling the reopened copy - closed, still with no resolution body - restores + # the stale-record case the fallback exists for, so the archive must answer again. + tasks_in "$home" "done" "$hold" >/dev/null \ + || fail "could not settle the reopened pick record ($state)" + show=$(tasks_in "$home" show "$hold" --full) \ + || fail "settled fixture must leave the live copy in the backlog ($state)" + assert_contains "$show" "state: done" "settled fixture must leave a done record" + assert_not_contains "$show" "Resolution recorded by fm-decision-hold." \ + "settled live copy must still carry no resolution body ($state)" + set +e + run_decisions "$home" complete "$origin" pick \ + > "$home/settled-complete.out" 2> "$home/settled-complete.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "narrowing broke the settled stale-record fall-through ($state): $(cat "$home/settled-complete.err")" + set +e + run_decisions "$home" verify "$origin" \ + > "$home/settled-verify.out" 2> "$home/settled-verify.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "narrowing broke the settled stale-record verification ($state): $(cat "$home/settled-verify.err")" + done + + pass "a decision reopened after its answer was archived gates completion again" +} + # An absent or unset archive key must behave exactly as before the fallback existed: # refuse a genuinely missing decision, without crashing and without silently passing. test_absent_archive_config_behaves_as_before() { @@ -936,4 +1060,5 @@ test_resolve_matches_quoted_blocked_by_edges test_resolved_decision_in_done_archive_satisfies_the_gate test_duplicate_archived_identity_is_order_independent test_stale_live_record_still_consults_the_archive +test_reopened_decision_is_not_settled_by_the_archive test_absent_archive_config_behaves_as_before From 8a21fdf697702b60d68ca9214b6fd4377ac58c1d Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Fri, 14 Aug 2026 13:18:04 -0400 Subject: [PATCH 5/6] no-mistakes(review): correct reopened-decision gating prose, refusal fields, stale counts --- bin/fm-decision-hold.sh | 36 ++++--- docs/decision-hold-lifecycle.md | 84 ++++++++++++--- tests/fm-decision-hold-lifecycle.test.sh | 131 +++++++++++++++++++---- 3 files changed, 202 insertions(+), 49 deletions(-) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index a44da87b855..9d13aa924fe 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -50,13 +50,24 @@ # a SETTLED captain record - a closed one - that does not itself carry a durable # resolution, because retention may have left the resolution in the archive # instead. Retention position therefore never decides the answer for a settled -# decision. A live record that is still open - queued, in flight, held again, or -# otherwise unsettled - is an unanswered decision in its own right: it refuses, and -# no archived resolution can settle it, so a decision that was answered, archived, -# and then reopened gates completion again. Only a resolved record is accepted from -# the archive, and it must carry the same resolution body an active record must -# carry. An ACTIVE hold is never satisfiable from the archive: verify_hold_active -# reads the live backlog alone. +# decision. A live record that is open but is NOT an active captain hold - it is in +# flight, or unheld, or held for something other than the captain - is an unanswered +# decision in its own right: it refuses on its own observed state, and no archived +# resolution can settle it. Only a resolved record is accepted from the archive, and +# it must carry the same resolution body an active record must carry. An ACTIVE hold +# is never satisfiable from the archive: verify_hold_active reads the live backlog +# alone. +# +# Re-holding an archived decision key through this script's own `hold` is a separate +# path with a separate answer, and the gate is not what stops it. Such a record +# presents as an active captain hold (state=queued held=yes kind=captain +# hold_kind=captain), which the active-hold branch accepts before the settled check +# is ever reached, so completion, verification, and teardown all succeed. That is +# base-parity behavior, verified identical on base 4bf9c08, and it is not a gap in +# protection: an active captain hold IS a legitimate durable state, and such a +# decision is gated by that live hold rather than by this check. What lets the key be +# re-held at all is command_hold's live-only resolved-key guard, a known related gap +# whose semantics the decision-hold-lifecycle skill owns. # # The archive path comes only from `.tasks.toml`'s [markdown] archive key, resolved # relative to FM_HOME. When that key is absent this lookup is unavailable and the @@ -361,12 +372,13 @@ verify_hold_durable() { # return 0 fi record_is_resolved "$show" && return 0 - # An unsettled live record - queued, in flight, or held again - is an unanswered - # captain decision in its own right, whatever the archive remembers about an - # earlier answer. A decision that was resolved, archived, and then reopened must - # gate completion again, so this refuses without consulting the archive. + # An open live record that reached here is not an active captain hold and carries + # no resolution, so it is an unanswered captain decision in its own right whatever + # the archive remembers about an earlier answer. It refuses without consulting the + # archive, naming every field the active-hold branch above tests so the refusal can + # explain which one failed. [ "$state" = "done" ] \ - || fail "captain decision $id has an open unresolved record in $FM_HOME/data/backlog.md (state=$state held=$held kind=$kind)" + || fail "captain decision $id has an open unresolved record in $FM_HOME/data/backlog.md (state=$state held=$held kind=$kind hold_kind=$hold_kind)" fi # The live record is absent, or is a settled record that carries no durable # resolution. Either way retention may hold a resolved copy of this identity in the diff --git a/docs/decision-hold-lifecycle.md b/docs/decision-hold-lifecycle.md index 731bbdd129d..d2a35ebe897 100644 --- a/docs/decision-hold-lifecycle.md +++ b/docs/decision-hold-lifecycle.md @@ -99,11 +99,16 @@ Without the second case, a stale closed live copy of an identity whose resolutio Retention position must not decide the answer for a settled decision. The fall-through stops at settled records. -A live record that is still open - queued, in flight, or held again - is an unanswered captain decision in its own right, so it refuses on its own state and no archived resolution can satisfy it. -That keeps a decision that was answered, archived, and then reopened gating completion, verification, and teardown exactly as it did before the archive fallback existed. +A live record that is open but is not an active captain hold - in flight, unheld, or held for something other than the captain - is an unanswered captain decision in its own right, so it refuses on its own observed state and no archived resolution can satisfy it. +The refusal names all four fields the active-hold check tests, `state`, `held`, `kind`, and `hold_kind`, so it can say which one failed. -Observed 2026-08-14 in a synthetic home: before the fall-through was narrowed to settled records, a reopened decision stopped gating completion. -The reproduction resolves a decision, lets the backlog's own retention archive it, re-holds the same key, and then leaves that live copy unsettled. +Re-holding an archived decision key through this script's own `hold` is a different path with a different answer, and this check is not what governs it. +That record presents as an active captain hold - `state=queued held=yes kind=captain hold_kind=captain` - which the active-hold branch accepts before the settled check is reached, so completion, verification, and teardown all succeed. +That is base-parity behavior, verified identical on base `4bf9c08`, and not a gap in protection: an active captain hold is a legitimate durable state, and such a decision is gated by that live hold rather than by this check. +What lets the key be re-held at all is `command_hold`'s live-only resolved-key guard, recorded below as a known related gap. + +Observed 2026-08-14 in a synthetic home: before the fall-through was narrowed to settled records, an archived answer settled a live record that was open and not an active captain hold. +The reproduction resolves a decision, lets the backlog's own retention archive it, re-holds the same key, and then drops that live copy out of an active captain hold. ```text $ bin/fm-decision-hold.sh hold sample-reopened-review pick --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample @@ -115,6 +120,7 @@ $ tasks-axi unhold sample-reopened-review-decision-pick $ tasks-axi show sample-reopened-review-decision-pick --full state: queued held: no + hold_kind: "-" kind: captain body: "Origin: sample-reopened-review\nDecision key: pick\nState: awaiting captain decision." @@ -123,29 +129,31 @@ complete: sample-reopened-review decision inventory reviewed (pick) # rc=0 $ bin/fm-decision-hold.sh complete sample-reopened-review pick # after narrowing -fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=queued held=no kind=captain) +fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=queued held=no kind=captain hold_kind="-") # rc=1 ``` -`tasks-axi start` in place of `unhold` reproduces it identically through a different live shape. +`tasks-axi start` in place of `unhold` reproduces it through a distinct live shape, one that keeps its captain hold but leaves `state=in_flight`. ```text $ tasks-axi start sample-reopened-review-decision-pick $ tasks-axi show sample-reopened-review-decision-pick --full state: in_flight held: yes + hold_kind: captain + kind: captain $ bin/fm-decision-hold.sh complete sample-reopened-review pick # before narrowing complete: sample-reopened-review decision inventory reviewed (pick) # rc=0 $ bin/fm-decision-hold.sh complete sample-reopened-review pick # after narrowing -fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=in_flight held=yes kind=captain) +fm-decision-hold: captain decision sample-reopened-review-decision-pick has an open unresolved record in .../data/backlog.md (state=in_flight held=yes kind=captain hold_kind=captain) # rc=1 ``` -Both refusals match what base `4bf9c08` did before any archive lookup existed, so teardown can no longer erase the source of a genuinely pending decision. -Settling that same reopened copy without a resolution restores the stale-record case, and the archive answers it again, so the narrowing does not revert the fix. +Both refusals match what base `4bf9c08` did before any archive lookup existed, so teardown can no longer erase the source of one of these decisions. +Settling either copy without a resolution restores the stale-record case, and the archive answers it again, so the narrowing does not revert the fix. ```text $ tasks-axi done sample-reopened-review-decision-pick @@ -158,6 +166,48 @@ complete: sample-reopened-review decision inventory reviewed (pick) # rc=0 ``` +Re-holding the same archived key through `bin/fm-decision-hold.sh hold` alone, with no `unhold` or `start` after it, is the base-parity case above rather than a refusal. +Verified 2026-08-14 against both this revision and base `4bf9c08` in the same home: the record is an active captain hold, and both revisions pass it. + +```text +$ bin/fm-decision-hold.sh hold sample-reheld-review pick --title "Pick" --reason "captain pick pending" --repo sample +sample-reheld-review-decision-pick + +$ tasks-axi show sample-reheld-review-decision-pick --full + state: queued + held: yes + hold_kind: captain + kind: captain + +$ bin/fm-decision-hold.sh complete sample-reheld-review pick # this revision +complete: sample-reheld-review decision inventory reviewed (pick) + # rc=0 + +$ bin/fm-decision-hold.sh verify sample-reheld-review # this revision +verified: sample-reheld-review unresolved-decision inventory + # rc=0 + +$ bin/fm-decision-hold.sh complete sample-reheld-review pick # base 4bf9c08 +complete: sample-reheld-review decision inventory reviewed (pick) + # rc=0 +``` + +The refusal names `hold_kind` because that field alone can be what failed. +Verified 2026-08-14: a record held for something other than the captain satisfies `state`, `held`, and `kind`, so without `hold_kind` the message printed only fields that look like a valid active captain hold and could not explain its own refusal. + +```text +$ tasks-axi hold sample-hk-review-decision-pick --reason "external pending" --kind external +$ tasks-axi show sample-hk-review-decision-pick --full + state: queued + held: yes + hold_kind: external + kind: captain + +$ bin/fm-decision-hold.sh complete sample-hk-review pick +fm-decision-hold: captain decision sample-hk-review-decision-pick has an open unresolved record in .../data/backlog.md (state=queued held=yes kind=captain hold_kind=external) + # rc=1 +``` + An accepted limitation, verified 2026-08-14 on tasks-axi 0.2.5: the lookup reads only the archive path pinned under `[markdown] archive`, while tasks-axi archives to a default `/done-archive.md` even when that key - or the whole `.tasks.toml` - is absent. A home that does not pin the key therefore still reproduces the original defect. @@ -209,12 +259,14 @@ The focused end-to-end regression uses only synthetic `sample` identities and de It begins with a completed investigation and visual review whose genuine unresolved choice exists only in the report. The initial Bearings snapshot correctly has no open decision, and the new teardown gate refuses to erase the source. A later regression covers tasks-axi's quoted multi-entry `blocked_by` output so `resolve` matches the first, middle, and last ids and rejects a genuinely absent id. -Five Done-archive regressions drive the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and together assert seven boundaries. +Five Done-archive regressions drive the incident above through the backlog's own `tasks-axi prune` retention rather than hand-moving records, and together assert eight boundaries. `test_resolved_decision_in_done_archive_satisfies_the_gate` covers three: a resolved archived decision passes, an archived record stripped of its resolution markers still refuses with the archive-specific refusal, and an open hold present only in the archive satisfies neither the active-hold check nor completion. Each of those three refusals is pinned to the identity under test, and the open-hold case resets the inventory to that key alone so which key refuses cannot depend on inventory sort order. `test_duplicate_archived_identity_is_order_independent` covers the ordering boundary, building both archive orderings of one duplicated identity through the real hold, resolve, and prune lifecycle and requiring the same answer from each. `test_stale_live_record_still_consults_the_archive` covers the stale-live-record boundary, pairing an archived resolution with a closed unresolved live copy of the same identity and requiring that the answer not change when retention later rotates that live copy out. -`test_reopened_decision_is_not_settled_by_the_archive` covers the reopened boundary in both live shapes, `unhold` and `start`: completion, verification, and teardown each refuse on the live record's own open state, no false attestation is written, and settling that same copy without a resolution then passes from the archive so the narrowing is proved not to revert the stale-record fix. +`test_reopened_decision_is_not_settled_by_the_archive` covers two: the open-without-an-active-captain-hold boundary in all three live shapes that reach it, `unhold`, `start`, and a non-captain `hold`, and the base-parity boundary for the one shape that does not. +In each of the three, completion, verification, and teardown refuse on the live record's own open state, the refusal is required to quote the fields actually observed so it can explain which one failed, no false attestation is written, and settling that same copy without a resolution then passes from the archive so the narrowing is proved not to revert the stale-record fix. +The base-parity case re-holds an archived key through this script's own `hold` and nothing further, leaving an active captain hold, and requires completion and verification to keep succeeding, pinning the behavior recorded above as unchanged from base rather than leaving it implicit. `test_absent_archive_config_behaves_as_before` covers the last: an absent, missing, empty, or corrupt archive refuses exactly as before instead of passing, with the corrupt case asserted on its own `is not a text backlog file` refusal so it cannot be satisfied by an ordinary absence. The final verification commands and their exact summarized outputs follow. @@ -233,7 +285,7 @@ ok - resolve matches first/middle/last in quoted blocked_by and rejects a genuin ok - a resolved decision in the Done archive satisfies the gate while open and unresolved records still refuse ok - a duplicated archived identity resolves the same way in either ordering ok - a stale unresolved live record does not hide a durable resolution in the archive -ok - a decision reopened after its answer was archived gates completion again +ok - an open live record without an active captain hold is not settled by the archive ok - an absent, missing, or corrupt archive refuses exactly as before rather than passing $ bash tests/fm-fleet-snapshot-view.test.sh @@ -265,6 +317,10 @@ not ok - Orca spawn should fail when metadata cannot be written $ bash tests/fm-backend.test.sh # fails identically on the unmodified base not ok - fm-send --key: old vs new exit code: expected exit 1, got 0 -$ for test_script in tests/*.test.sh; do bash "$test_script"; done -ALL 71 TEST SCRIPTS PASSED +$ ls tests/*.test.sh | wc -l + 95 ``` + +No all-suites aggregate is claimed here. +Re-counted 2026-08-14: the repository holds 95 test scripts, not the 71 an earlier revision of this record asserted, and two of them - the backend scripts quoted above - fail on unmodified base code. +An "all scripts passed" line therefore cannot be true as written, so it is dropped rather than restated; the whole-repository run is owned by the dedicated test step, and this record keeps only the suites it verified directly. diff --git a/tests/fm-decision-hold-lifecycle.test.sh b/tests/fm-decision-hold-lifecycle.test.sh index 097f6216453..09f69c24339 100755 --- a/tests/fm-decision-hold-lifecycle.test.sh +++ b/tests/fm-decision-hold-lifecycle.test.sh @@ -857,17 +857,24 @@ test_stale_live_record_still_consults_the_archive() { pass "a stale unresolved live record does not hide a durable resolution in the archive" } -# The archive fallback must not let an archived answer settle a decision that is -# OPEN again in the live backlog. Retention rotates a resolved decision out, the same -# key can then be re-held, and an unheld or in-flight copy of it is a genuinely -# unanswered captain decision whatever the archive remembers. Before the fall-through -# was narrowed to settled live records, both of those states passed completion, so -# teardown could erase the source of a pending decision. Each state must refuse, and -# a SETTLED live copy carrying no resolution must still be satisfiable from the +# The archive fallback must not let an archived answer settle a live record that is +# OPEN and is not an active captain hold. Retention rotates a resolved decision out, +# the same key can then be re-held, and dropping that copy out of its captain hold - +# unheld, in flight, or held for something else - leaves a genuinely unanswered +# captain decision whatever the archive remembers. Before the fall-through was +# narrowed to settled live records, every one of those states passed completion, so +# teardown could erase the source of a pending decision. Each must refuse, naming all +# four fields the active-hold check tests so the refusal explains which one failed, +# and a SETTLED live copy carrying no resolution must still be satisfiable from the # archive so the narrowing does not revert the fix this fallback exists to deliver. +# +# Re-holding through the script's own `hold` alone is deliberately NOT one of these +# states: it presents as an active captain hold, which is a legitimate durable state +# satisfied by the active-hold check before this one, so it passes here exactly as it +# does on base. That base parity is asserted at the end rather than left implicit. test_reopened_decision_is_not_settled_by_the_archive() { - local home origin hold work rc state show - for state in unheld in-flight; do + local home origin hold work rc state show expected + for state in unheld in-flight external-hold; do home=$(make_home "reopened-$state") origin="sample-reopened-$state-review" mkdir -p "$home/data/$origin" @@ -900,23 +907,46 @@ test_reopened_decision_is_not_settled_by_the_archive() { run_decisions "$home" hold "$origin" pick \ --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample \ >/dev/null || fail "could not re-hold the pick key after its resolution was archived ($state)" - if [ "$state" = unheld ]; then - tasks_in "$home" unhold "$hold" >/dev/null \ - || fail "could not drop the hold from the reopened pick record" - else - tasks_in "$home" start "$hold" >/dev/null \ - || fail "could not start the reopened pick record" - fi + case "$state" in + unheld) + tasks_in "$home" unhold "$hold" >/dev/null \ + || fail "could not drop the hold from the reopened pick record" + ;; + in-flight) + tasks_in "$home" start "$hold" >/dev/null \ + || fail "could not start the reopened pick record" + ;; + external-hold) + tasks_in "$home" hold "$hold" --reason "external pick review pending" --kind external \ + >/dev/null || fail "could not re-hold the reopened pick record for a non-captain owner" + ;; + esac show=$(tasks_in "$home" show "$hold" --full) \ || fail "reopened fixture must leave a live copy in the backlog ($state)" assert_not_contains "$show" "Resolution recorded by fm-decision-hold." \ "reopened live copy must carry no resolution body ($state)" - if [ "$state" = unheld ]; then - assert_contains "$show" "state: queued" "unheld fixture must leave a queued record" - assert_contains "$show" "held: no" "unheld fixture must leave an unheld record" - else - assert_contains "$show" "state: in_flight" "in-flight fixture must leave an in_flight record" - fi + # Each shape must fail exactly one of the four active-hold fields, and the refusal + # must name the failing value: otherwise a message listing only satisfying-looking + # fields cannot explain its own refusal. + case "$state" in + unheld) + assert_contains "$show" "state: queued" "unheld fixture must leave a queued record" + assert_contains "$show" "held: no" "unheld fixture must leave an unheld record" + expected="state=queued held=no kind=captain" + ;; + in-flight) + assert_contains "$show" "state: in_flight" "in-flight fixture must leave an in_flight record" + assert_contains "$show" "held: yes" "in-flight fixture must keep its captain hold" + expected="state=in_flight held=yes kind=captain hold_kind=captain" + ;; + external-hold) + assert_contains "$show" "state: queued" "external-hold fixture must leave a queued record" + assert_contains "$show" "held: yes" "external-hold fixture must leave a held record" + assert_contains "$show" "hold_kind: external" \ + "external-hold fixture must leave a non-captain hold_kind" + expected="state=queued held=yes kind=captain hold_kind=external" + ;; + esac if run_decisions "$home" complete "$origin" pick \ > "$home/reopened-complete.out" 2> "$home/reopened-complete.err"; then @@ -925,6 +955,8 @@ test_reopened_decision_is_not_settled_by_the_archive() { assert_grep "captain decision $hold has an open unresolved record" \ "$home/reopened-complete.err" \ "completion must refuse a reopened decision on its own open live record ($state)" + assert_grep "($expected" "$home/reopened-complete.err" \ + "the refusal must report the live fields actually observed ($state)" assert_no_grep "decisions_reviewed=1" "$home/state/$origin.meta" \ "refused completion recorded a false attestation for a reopened decision ($state)" @@ -968,7 +1000,60 @@ test_reopened_decision_is_not_settled_by_the_archive() { || fail "narrowing broke the settled stale-record verification ($state): $(cat "$home/settled-verify.err")" done - pass "a decision reopened after its answer was archived gates completion again" + # Re-holding an archived key through the script's own `hold`, with nothing after it, + # leaves an ACTIVE captain hold. That is a legitimate durable state the active-hold + # check satisfies before the settled check is reached, so it must keep passing: the + # narrowing above governs records that are open WITHOUT such a hold, not this one. + home=$(make_home reheld-active-hold) + origin=sample-reheld-review + mkdir -p "$home/data/$origin" + tasks_in "$home" add "$origin" "Investigate a re-held sample decision" \ + --kind scout --repo sample --start >/dev/null \ + || fail "could not create re-held-decision origin" + write_origin_meta "$home" "$origin" + printf 'done: report complete\n' > "$home/state/$origin.status" + printf '# Re-held review\n\nOne captain choice was answered, archived, then re-held.\n' \ + > "$home/data/$origin/report.md" + + hold=$(run_decisions "$home" hold "$origin" pick \ + --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample) \ + || fail "could not register the re-held pick hold" + work=sample-pick-work + tasks_in "$home" add "$work" "Apply the selected sample pick" \ + --kind ship --repo sample --blocked-by "$hold" >/dev/null \ + || fail "could not create dependent re-held pick work" + printf 'Pick the sample front.\n' > "$home/pick-decision.txt" + run_decisions "$home" resolve "$origin" pick \ + --decision-file "$home/pick-decision.txt" --routed-to "$work" >/dev/null \ + || fail "could not resolve the re-held pick decision" + tasks_in "$home" prune --keep 0 --state "done" >/dev/null \ + || fail "could not archive the resolved re-held pick copy" + run_decisions "$home" hold "$origin" pick \ + --title "Choose the sample pick" --reason "captain pick choice pending" --repo sample \ + >/dev/null || fail "could not re-hold the pick key after its resolution was archived" + + show=$(tasks_in "$home" show "$hold" --full) \ + || fail "re-held fixture must leave a live copy in the backlog" + assert_contains "$show" "state: queued" "re-held fixture must leave a queued record" + assert_contains "$show" "held: yes" "re-held fixture must leave an active hold" + assert_contains "$show" "kind: captain" "re-held fixture must leave a captain record" + assert_contains "$show" "hold_kind: captain" "re-held fixture must leave a captain hold" + set +e + run_decisions "$home" complete "$origin" pick \ + > "$home/reheld-complete.out" 2> "$home/reheld-complete.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "an active captain hold stopped satisfying completion: $(cat "$home/reheld-complete.err")" + set +e + run_decisions "$home" verify "$origin" \ + > "$home/reheld-verify.out" 2> "$home/reheld-verify.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "an active captain hold stopped satisfying verification: $(cat "$home/reheld-verify.err")" + + pass "an open live record without an active captain hold is not settled by the archive" } # An absent or unset archive key must behave exactly as before the fallback existed: From c2be9b51ea1ebfe3bf86a10f77e9f9901f7d8f47 Mon Sep 17 00:00:00 2001 From: Jacy Anderson Date: Fri, 14 Aug 2026 14:46:53 -0400 Subject: [PATCH 6/6] no-mistakes(document): correct hold reopen-guard scope, note archive key consumer --- docs/configuration.md | 1 + docs/decision-hold-lifecycle.md | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index ed4e9d93022..94874b222e8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -26,6 +26,7 @@ Ordinary dead-direct-report recovery is owned by `stuck-crewmate-recovery`, whil ## Backlog backend (.tasks.toml / config/backlog-backend) The tracked `.tasks.toml` pins the default `tasks-axi` markdown backend to `data/backlog.md`, with `done_keep = 10` and an archive at `data/done-archive.md`. +Keep the `[markdown] archive` key pinned: `bin/fm-decision-hold.sh` reads it to find decisions that retention has rotated out of the active backlog, and an unpinned key makes that gate refuse its own resolved decisions ([decision-hold-lifecycle.md](decision-hold-lifecycle.md)). When the default backend is selected and compatible `tasks-axi` is on `PATH`, firstmate uses its verbs for routine backlog mutations. Secondmate handoffs are separate and unconditional: `fm-backlog-handoff.sh` keeps only its own fleet-level validation and always delegates the item move to `tasks-axi mv`, the single owner of the backlog format. It moves in-scope `## Queued` items only and refuses `## In flight` and historical `## Done` records, which stay with their home for pruning or archiving. diff --git a/docs/decision-hold-lifecycle.md b/docs/decision-hold-lifecycle.md index d2a35ebe897..d4e21a5d1c3 100644 --- a/docs/decision-hold-lifecycle.md +++ b/docs/decision-hold-lifecycle.md @@ -11,7 +11,8 @@ It never reads report bodies, review artifacts, terminal output, or chat. The `hold` subcommand maps an originating work id and stable decision key to `-decision-`. It creates a kind `captain` backlog item when absent and invokes `tasks-axi hold --reason --kind captain` on every retry. -It rejects an identity collision, a changed title, and attempts to reopen an already resolved identity. +It rejects an identity collision, a changed title, and an attempt to reopen an identity whose resolved record is still in the active backlog. +That reopen guard reads only the active backlog, so a resolved key can still be re-held once retention has archived its record; the incident record below states that known related gap and its consequences. The `complete` subcommand unions the reviewed keys into `decision_keys=` and appends `decisions_reviewed=1` while originating task metadata is live. A post-teardown visual review can complete against the surviving report and durable holds without recreating volatile task metadata.