Skip to content

fix(crew-state): stop passed runs from implying PR landing #2778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Mr-Neutr0n wants to merge 2 commits into from
Closed

fix(crew-state): stop passed runs from implying PR landing #2778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91e211a
fix(crew-state): outcome=passed must not claim PR merged/closed
Mr-Neutr0n Aug 22, 2026
068c5ce
fix(crew-state): make the passed-run detail neutral about the forge
Mr-Neutr0n Aug 22, 2026
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
6 changes: 5 additions & 1 deletion bin/fm-crew-state.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,11 @@ if [ "$HAVE_RUN" = 1 ]; then

if [ -n "$outcome" ]; then
case "$outcome" in
passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;;
# `passed` is a claim about the pipeline's own nine steps completing.
# It establishes nothing about the forge: the PR may still be open,
# already merged, or not exist. Only teardown's own pr_is_merged
# check establishes landing, so this detail stays neutral about it.
passed) RUN_STATE="done"; RUN_DETAIL="run passed: pipeline complete" ;;
checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;;
failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;;
cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;;
Expand Down
25 changes: 25 additions & 0 deletions tests/fm-crew-state.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,30 @@ test_terminal_passed() {
pass "terminal passed run is authoritative"
}

test_terminal_passed_does_not_claim_landing() {
# Regression for issue #2421: a pipeline `passed` outcome is a claim about
# the run's own steps, not about the forge. The PR may be open, already
# merged, or absent; rendering "PR merged/closed" (the old wording) or any
# other forge state invents landing the helper never established.
reset_fakes
local d; d=$(new_case passed-open-pr)
make_repo_on_branch "$d/wt" fm/feat-landing
make_fakebin "$d" >/dev/null
fm_write_meta "$d/state/feat-landing.meta" "window=fm:fm-feat-landing" "worktree=$d/wt" "kind=ship"
FM_FAKE_AXI_STATUS="$(run_passed fm/feat-landing)"
local out; out=$(run_crew_state "$d" feat-landing)
assert_contains "$out" "run passed" "passed run keeps its run claim"
assert_contains "$out" "pipeline complete" "detail stays neutral about the forge"
case "$out" in
*merged/closed*|*awaiting*review*)
fail "passed run must not render any forge claim about the PR"
;;
*)
pass "no forge claim for an outcome=passed run"
;;
esac
}

test_terminal_failed() {
reset_fakes
local d; d=$(new_case failed)
Expand Down Expand Up @@ -1427,6 +1451,7 @@ test_ci_fixing_after_green_stays_working
test_top_level_fixing_ci_running_after_green_stays_working
test_top_level_fixing_done_log_stays_working
test_terminal_passed
test_terminal_passed_does_not_claim_landing
test_terminal_failed
test_cross_branch_attribution_via_runs_list
test_cross_branch_attribution_picks_most_recent_row
Expand Down