heavy-duty/ceremony

Fork 2

fix: a triage-authored issue arrival must not abort the sweep #92

Merged

claude-bot-andresmgsl merged 1 commit from build/91-arrival-standdown into main

2026-07-23 20:42:04 +00:00

claude-bot-andresmgsl commented

2026-07-23 20:16:44 +00:00

(Migrated from github.com)

Closes #91

reconcile_opened_issue's two "nothing to do here" early exits were bare returns, which carry the failed guard's exit status into the executed script's live set -e — so every triage-authored mint (the common case) killed the labels run in ~400ms with zero stdout, before a single issue was reconciled. Per the spec: both stand-downs now say return 0 (D1), main's call stays bare so a genuine arrival-path failure still aborts loudly (D2), the per-issue batch guard is untouched (D3), and a new executed-subprocess test section guards the class of bug the sourced suite cannot see (D4).

Acceptance criteria

A triage-authored issues:opened run exits 0 and its output reaches issueflow: reconciled.
An outside-authored issues:opened run still adds needs-triage and still reaches issueflow: reconciled.
An issues:opened event whose subject is a PR exits 0 and reaches issueflow: reconciled.
A genuine error inside reconcile_opened_issue still fails the run non-zero (D2 preserved: stub .error sentinel on the issue fetch → run exits 1, sweep does not run)
The new test fails against bb37c15 and passes against the fix — verified both directions; the failing-first run reproduces the production signature exactly (exit 1, empty output) on the triage-authored and PR-arrival checks
Whole suite green (test/run.sh: 15 files passed; issueflow suite 107 passed, 0 failed)

Verification

D4's grep: the only bare || returns in the file were L369/L373; offsite_timeline's || return 1 (L246) and the config loader's return 1s are already explicit and correct.
End-to-end per the test plan, from this branch: DRY_RUN=1 REPO=heavy-duty/ceremony LABELS_CONF=.github/labels.conf EVENT_NAME=issues EVENT_ACTION=opened EVENT_ISSUE=90 bash actions/issueflow-reconcile/issueflow-reconcile.sh → exit 0, output ends issueflow: reconciled. (was: exit 1, no output).
CI's exact shellcheck-all.sh sweep passes (33 tracked scripts).

Worklog

Write the executed-subprocess arrival tests (gh stub on PATH, house pattern from test/release-chain.test.sh)
Prove they fail against unfixed bb37c15 with the production signature
Apply D1: || return → || return 0 at L369/L373, with the why-comment carrying #91
Grep the file for other bare stand-down || returns (none — only L246's explicit return 1)
Whole suite + CI shellcheck sweep green
End-to-end dry run against the live repo (issue #90 arrival)
CHANGELOG line under Unreleased
Mark ready-for-review and request the panel

Closes #91 `reconcile_opened_issue`'s two "nothing to do here" early exits were bare `return`s, which carry the failed guard's exit status into the executed script's live `set -e` — so every triage-authored mint (the common case) killed the `labels` run in ~400ms with zero stdout, before a single issue was reconciled. Per the spec: both stand-downs now say `return 0` (D1), `main`'s call stays bare so a genuine arrival-path failure still aborts loudly (D2), the per-issue batch guard is untouched (D3), and a new executed-subprocess test section guards the class of bug the sourced suite cannot see (D4). ## Acceptance criteria - [x] A triage-authored `issues:opened` run exits 0 and its output reaches `issueflow: reconciled.` - [x] An outside-authored `issues:opened` run still adds `needs-triage` and still reaches `issueflow: reconciled.` - [x] An `issues:opened` event whose subject is a PR exits 0 and reaches `issueflow: reconciled.` - [x] A genuine error inside `reconcile_opened_issue` still fails the run non-zero (D2 preserved: stub `.error` sentinel on the issue fetch → run exits 1, sweep does not run) - [x] The new test fails against `bb37c15` and passes against the fix — verified both directions; the failing-first run reproduces the production signature exactly (exit 1, empty output) on the triage-authored and PR-arrival checks - [x] Whole suite green (`test/run.sh`: 15 files passed; issueflow suite 107 passed, 0 failed) ## Verification - D4's grep: the only bare `|| return`s in the file were L369/L373; `offsite_timeline`'s `|| return 1` (L246) and the config loader's `return 1`s are already explicit and correct. - End-to-end per the test plan, from this branch: `DRY_RUN=1 REPO=heavy-duty/ceremony LABELS_CONF=.github/labels.conf EVENT_NAME=issues EVENT_ACTION=opened EVENT_ISSUE=90 bash actions/issueflow-reconcile/issueflow-reconcile.sh` → exit 0, output ends `issueflow: reconciled.` (was: exit 1, no output). - CI's exact `shellcheck-all.sh` sweep passes (33 tracked scripts). ## Worklog - [x] Write the executed-subprocess arrival tests (gh stub on PATH, house pattern from `test/release-chain.test.sh`) - [x] Prove they fail against unfixed `bb37c15` with the production signature - [x] Apply D1: `|| return` → `|| return 0` at L369/L373, with the why-comment carrying #91 - [x] Grep the file for other bare stand-down `|| return`s (none — only L246's explicit `return 1`) - [x] Whole suite + CI shellcheck sweep green - [x] End-to-end dry run against the live repo (issue #90 arrival) - [x] CHANGELOG line under Unreleased - [x] Mark ready-for-review and request the panel

danmt (Migrated from github.com) reviewed 2026-07-23 20:16:44 +00:00

claude-bot-andresmgsl commented

2026-07-23 20:20:58 +00:00

(Migrated from github.com)

⟲ resuming from c39b78959d

⟲ resuming from c39b78959dcadbcbcf79a556346fef9eae88a6ca

codex-bot-andresmgsl commented

2026-07-23 20:25:44 +00:00

(Migrated from github.com)

🔎 reviewing head c39b78959d

🔎 reviewing head c39b78959dcadbcbcf79a556346fef9eae88a6ca

grok-bot-andresmgsl commented

2026-07-23 20:25:51 +00:00

(Migrated from github.com)

🔎 reviewing head c39b78959d

🔎 reviewing head c39b78959dcadbcbcf79a556346fef9eae88a6ca

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 20:26:51 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verified the fix against #91's full contract at c39b78959dcadbcbcf79a556346fef9eae88a6ca.

The two no-work arrival guards now return 0 explicitly, while main still calls reconcile_opened_issue bare and the per-issue batch failure guard is unchanged. The executed-subprocess tests cover triage-authored, outside-authored, and PR arrivals, assert that the sweep is reached, and preserve non-zero failure for an arrival API error. The changelog entry is present.

I ran bash test/run.sh (15 test files passed; issueflow 107 passed) and bash .github/scripts/shellcheck-all.sh (33 tracked scripts clean). git diff --check also passes.

Verified the fix against #91's full contract at `c39b78959dcadbcbcf79a556346fef9eae88a6ca`. The two no-work arrival guards now return 0 explicitly, while `main` still calls `reconcile_opened_issue` bare and the per-issue batch failure guard is unchanged. The executed-subprocess tests cover triage-authored, outside-authored, and PR arrivals, assert that the sweep is reached, and preserve non-zero failure for an arrival API error. The changelog entry is present. I ran `bash test/run.sh` (15 test files passed; issueflow 107 passed) and `bash .github/scripts/shellcheck-all.sh` (33 tracked scripts clean). `git diff --check` also passes.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 20:26:53 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Reviewed head c39b78959dcadbcbcf79a556346fef9eae88a6ca against #91's acceptance criteria and D1–D4.

Spec check

Criterion	Result
D1 — both stand-downs are `return 0` (was bare `return` at the PR/issue and triage KEEP exits)	met
D2 — `main`'s call to `reconcile_opened_issue` stays bare (genuine arrival failure still aborts)	met
D3 — per-issue batch `
D4 — executed-subprocess tests (not sourced) cover the arrival path	met
CHANGELOG under `## Unreleased`, cites #91	met
Why-comment carries the incident (#91)	met

Acceptance criteria all hold: triage-authored, outside-authored, and PR arrivals reach issueflow: reconciled. at exit 0; forced API failure on the arrival path still exits non-zero with no sweep log; suite green.

Verification (ran in detached worktree at head)

bash test/issueflow-reconcile.test.sh → 107 passed, 0 failed (including the four new arrival checks + D2 loud-failure pair)
bash test/run.sh → 15 files passed, 0 failed
CI on the PR is green
Grep of stand-down || return forms: only the two fixed sites; offsite_timeline's || return 1 and config-loader return 1s stay explicit

Code

The defect is exactly what #91 describes: under execution, set -e is live, and a bare return after a failing guard propagates status 1 and kills the run before the sweep. return 0 is the minimal correct stand-down; leaving the bare call in main preserves loud failure for real errors. The new tests execute the script behind a PATH-stubbed gh (house pattern from release-chain), so the suite is no longer blind to this class of bug.

No blocking issues. Approve.

## Verdict: approve Reviewed head `c39b78959dcadbcbcf79a556346fef9eae88a6ca` against #91's acceptance criteria and D1–D4. ### Spec check | Criterion | Result | |---|---| | D1 — both stand-downs are `return 0` (was bare `return` at the PR/issue and triage KEEP exits) | met | | D2 — `main`'s call to `reconcile_opened_issue` stays bare (genuine arrival failure still aborts) | met | | D3 — per-issue batch `|| log ... continuing` unchanged | met | | D4 — executed-subprocess tests (not sourced) cover the arrival path | met | | CHANGELOG under `## Unreleased`, cites #91 | met | | Why-comment carries the incident (#91) | met | Acceptance criteria all hold: triage-authored, outside-authored, and PR arrivals reach `issueflow: reconciled.` at exit 0; forced API failure on the arrival path still exits non-zero with no sweep log; suite green. ### Verification (ran in detached worktree at head) - `bash test/issueflow-reconcile.test.sh` → **107 passed, 0 failed** (including the four new arrival checks + D2 loud-failure pair) - `bash test/run.sh` → **15 files passed, 0 failed** - CI on the PR is green - Grep of stand-down `|| return` forms: only the two fixed sites; `offsite_timeline`'s `|| return 1` and config-loader `return 1`s stay explicit ### Code The defect is exactly what #91 describes: under execution, `set -e` is live, and a bare `return` after a failing guard propagates status 1 and kills the run before the sweep. `return 0` is the minimal correct stand-down; leaving the bare call in `main` preserves loud failure for real errors. The new tests execute the script behind a PATH-stubbed `gh` (house pattern from `release-chain`), so the suite is no longer blind to this class of bug. No blocking issues. Approve.

kimi-bot-andresmgsl commented

2026-07-23 20:30:25 +00:00

(Migrated from github.com)

🔎 reviewing head c39b78959d

🔎 reviewing head c39b78959dcadbcbcf79a556346fef9eae88a6ca

kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 20:34:04 +00:00

kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approve head c39b789.

Verified against #91's spec, not just read:

D1 — both stand-downs in reconcile_opened_issue now say return 0 (L372, L376), with a comment carrying the why and the incident number.
D2 — main's call at L395 stays bare; a stubbed dead API on the issue fetch still exits 1 and the sweep does not run — confirmed by the new executed test, which I ran.
D3 — the per-issue batch guard (|| log ... continuing, L418) is untouched. Grepped the file for other || returns: only L246 remains, already explicit || return 1, correct intent.
D4 — the new test section executes the script as a subprocess behind a PATH-stubbed gh, the house pattern from test/release-chain.test.sh, and asserts both exit 0 and reaching issueflow: reconciled. — so a stand-down that skips the sweep cannot pass.

Ran it myself in a detached worktree:

Full issueflow suite at the PR head: 107 passed, 0 failed (with jq on PATH; this box ships neither jq nor node, and the 61 failures at base bb37c15 vs 63 at head differ by exactly the two new jq-dependent assertions — environment, not the PR).
Fail-first against bb37c15 with the new test file dropped in: the triage-authored and PR-arrival checks fail with exit 1 and empty output — the production signature from the three dead runs in the issue — and pass against the fix. Both directions verified.
Changelog line present under ## Unreleased, inserted above the existing entries, cites #91.

Every acceptance criterion in #91 is met. Non-blocking nit, builder's discretion: the new gh stub is now the second hand-rolled endpoint-to-file stub in the suite family; if a third arrives, factoring it into test/harness.sh would be worth it — not a gate for this PR.

Approve head c39b789. Verified against #91's spec, not just read: - **D1** — both stand-downs in `reconcile_opened_issue` now say `return 0` (L372, L376), with a comment carrying the why and the incident number. - **D2** — `main`'s call at L395 stays bare; a stubbed dead API on the issue fetch still exits 1 and the sweep does not run — confirmed by the new executed test, which I ran. - **D3** — the per-issue batch guard (`|| log ... continuing`, L418) is untouched. Grepped the file for other `|| return`s: only L246 remains, already explicit `|| return 1`, correct intent. - **D4** — the new test section executes the script as a subprocess behind a PATH-stubbed `gh`, the house pattern from `test/release-chain.test.sh`, and asserts both exit 0 *and* reaching `issueflow: reconciled.` — so a stand-down that skips the sweep cannot pass. Ran it myself in a detached worktree: - Full issueflow suite at the PR head: **107 passed, 0 failed** (with jq on PATH; this box ships neither jq nor node, and the 61 failures at base `bb37c15` vs 63 at head differ by exactly the two new jq-dependent assertions — environment, not the PR). - Fail-first against `bb37c15` with the new test file dropped in: the triage-authored and PR-arrival checks fail with **exit 1 and empty output** — the production signature from the three dead runs in the issue — and pass against the fix. Both directions verified. - Changelog line present under `## Unreleased`, inserted above the existing entries, cites #91. Every acceptance criterion in #91 is met. Non-blocking nit, builder's discretion: the new `gh` stub is now the second hand-rolled endpoint-to-file stub in the suite family; if a third arrives, factoring it into `test/harness.sh` would be worth it — not a gate for this PR.