Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions bin/fm-decision-hold.sh
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,16 @@ task_show() { # <id>
}

show_field() { # <show-output> <field>
local output=$1 field=$2
printf '%s\n' "$output" | sed -n "s/^ $field: //p" | head -1
# tasks-axi quotes a rendered value only when it needs to (a comma-joined
# blocked_by list, an escaped body, a placeholder "-"), so an unstripped value
# matches for a single dependency and never matches for two. Strip one
# surrounding pair here, once, rather than at each call site.
local output=$1 field=$2 value
value=$(printf '%s\n' "$output" | sed -n "s/^ $field: //p" | head -1)
case "$value" in
'"'*'"') value=${value#\"}; value=${value%\"} ;;
esac
printf '%s\n' "$value"
}

origin_exists_here() { # <origin-id>
Expand Down Expand Up @@ -205,7 +213,9 @@ verify_hold_durable() { # <hold-id>

verify_resolution_identity() {
local id=$1 hold_body=$2 decision_digest=$3 routed_csv=$4 resolution_prefix resolution_fields recorded_digest recorded_routes
resolution_prefix='"Resolution recorded by fm-decision-hold.\nDecision digest: '
# show_field strips tasks-axi's surrounding quotes, so this anchors on the body
# text itself; the escaped newlines are still literal in the rendered value.
resolution_prefix='Resolution recorded by fm-decision-hold.\nDecision digest: '
case "$hold_body" in
"$resolution_prefix"*) resolution_fields=${hold_body#"$resolution_prefix"} ;;
*) fail "captain hold $id has no retry identity record" ;;
Expand Down
3 changes: 2 additions & 1 deletion docs/decision-hold-lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ Scout teardown calls the script's read-only `verify` subcommand after checking f
The `--force` path remains the explicit captain-approved discard escape hatch.

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.
It records the decision digest and routed task identities as a retry identity in the hold body, clears each routed task's edge to the hold through tasks-axi, and marks the hold Done only after those writes succeed.
A routed task may carry other blockers; only its edge to the hold is cleared, so such a task stays blocked by whatever else it depends on.
An exact retry can finish a partial routing operation, while a changed decision or routed-task set is rejected.
A failed intermediate step leaves the hold open.

Expand Down
84 changes: 84 additions & 0 deletions tests/fm-decision-hold-lifecycle.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,89 @@ test_terminal_single_owner_status_decision_does_not_block_empty_inventory() {
pass "terminal single-owner stale status decisions do not block empty inventory"
}

test_multi_dependency_hold_resolves_and_unblocks() {
local home id hold show single_dep multi_dep
home=$(make_home multi-dep-resolve)
id=sample-multi-review
mkdir -p "$home/data/$id"
tasks_in "$home" add "$id" "Investigate multi-dep sample" --kind scout --repo sample --start >/dev/null \
|| fail "could not create investigation fixture"
write_origin_meta "$home" "$id"
cat > "$home/state/$id.status" <<'EOF'
needs-decision [key=route]: choose route north or route south
done: report and visual review complete
EOF
printf '# Multi-dep sample review\n\nOne captain choice remains.\n' > "$home/data/$id/report.md"

hold=$(run_decisions "$home" hold "$id" route \
--title "Choose the sample route" --reason "captain route choice pending" --repo sample) \
|| fail "could not register the route hold"
run_decisions "$home" complete "$id" route >/dev/null \
|| fail "shared completion gate failed"

# Dependent work: one task blocked ONLY by the hold (tasks-axi renders that
# blocked_by unquoted), one task blocked by the hold AND an unrelated blocker
# (a comma-joined list, which tasks-axi renders quoted). The quoted rendering is
# the case the resolve/unblock membership tests could never match.
tasks_in "$home" add sample-unrelated-blocker "Unrelated sample blocker" \
--kind ship --repo sample >/dev/null \
|| fail "could not create unrelated blocker fixture"
tasks_in "$home" add sample-single-dep "Single-dependency dependent" \
--kind ship --repo sample --blocked-by "$hold" >/dev/null \
|| fail "could not create single-dependency fixture"
tasks_in "$home" add sample-multi-dep "Multi-dependency dependent" \
--kind ship --repo sample --blocked-by "$hold" >/dev/null \
|| fail "could not create multi-dependency fixture"
tasks_in "$home" block sample-multi-dep --by sample-unrelated-blocker >/dev/null \
|| fail "could not add the unrelated blocking edge"

# Pin the rendered shapes the bug depends on: single-dep unquoted, multi-dep quoted.
single_dep=$(tasks_in "$home" show sample-single-dep --full | sed -n 's/^ blocked_by: //p' | head -1)
multi_dep=$(tasks_in "$home" show sample-multi-dep --full | sed -n 's/^ blocked_by: //p' | head -1)
[ "$single_dep" = "$hold" ] \
|| fail "single-dependency blocked_by did not render unquoted: $single_dep"
[ "$multi_dep" = "\"$hold,sample-unrelated-blocker\"" ] \
|| fail "multi-dependency blocked_by did not render quoted: $multi_dep"

printf 'Use route north for the sample system.\n' > "$home/route-decision.txt"

# Load-bearing case: the routed set includes the quoted multi-dep task. On unfixed
# code show_field keeps the surrounding quotes, so the membership test tests
# ,"a,b", and matches no id, and resolve exits non-zero ("not durably blocked").
# The single-dep task passes by accident, so it is never the only routed task.
run_decisions "$home" resolve "$id" route --decision-file "$home/route-decision.txt" \
--routed-to sample-single-dep --routed-to sample-multi-dep >/dev/null \
|| fail "resolve could not close a hold with a multi-dependency routed task"

# The hold is durably resolved.
show=$(tasks_in "$home" show "$hold" --full)
assert_contains "$show" "state: done" "multi-dep resolution did not close the hold"
assert_contains "$show" "Resolution recorded by fm-decision-hold" \
"resolved hold lost its durable decision record"

# Unblock path: the hold edge is cleared from both dependents; the single-dep task
# is fully released, the multi-dep task retains ONLY the unrelated edge. This
# exercises the second membership test (the unblock loop) against the quoted render.
show=$(tasks_in "$home" show sample-single-dep --full)
assert_contains "$show" "blocked: no" "single-dependency task was not released"
show=$(tasks_in "$home" show sample-multi-dep --full)
assert_contains "$show" "blocked: yes" "multi-dependency task lost its unrelated blocker"
multi_dep=$(printf '%s\n' "$show" | sed -n 's/^ blocked_by: //p' | head -1)
[ "$multi_dep" = "sample-unrelated-blocker" ] \
|| fail "multi-dependency task did not retain ONLY the unrelated edge: $multi_dep"

# Idempotent re-resolve exercises the coupled identity check: show_field now strips
# tasks-axi's surrounding quotes from the body, so verify_resolution_identity must
# anchor without the leading quote. A quote-strip that omits that coupled change
# would leave the body carrying a leading quote the prefix no longer expects and
# break this re-resolve path.
run_decisions "$home" resolve "$id" route --decision-file "$home/route-decision.txt" \
--routed-to sample-single-dep --routed-to sample-multi-dep >/dev/null \
|| fail "identical multi-dep resolution retry was not idempotent"

pass "multi-dependency captain holds resolve, unblock only the hold edge, and re-resolve idempotently"
}

test_secondmate_hold_stays_in_authoritative_home() {
local parent mate origin hold json
parent=$(make_home main-routing)
Expand Down Expand Up @@ -462,4 +545,5 @@ test_origin_slug_validation_precedes_path_construction
test_visual_review_uses_shared_completion_owner
test_none_inventory_and_resolved_prose_do_not_create_holds
test_terminal_single_owner_status_decision_does_not_block_empty_inventory
test_multi_dependency_hold_resolves_and_unblocks
test_secondmate_hold_stays_in_authoritative_home