fix(issueflow): a failed read never reaches a decision function
`gh api` prints a 5xx response body to stdout AND exits non-zero, and
GitHub's 5xx body is a JSON object. Inside the per-issue subshell that
payload passed `has("pull_request") | not`, emptied `.labels[]`, and
`queue_decision` — correct on the input it was handed — wrote
`needs-triage` onto a healthy epic. The run then logged `reconciled.`
and exited 0 (crew#329, #247).
errexit could not have caught it: a command whose status is tested by
`||` runs with errexit suppressed, and the suppression extends through
the whole subshell body, so the `|| log` handler is what disables the
errexit that would have aborted at the failed read. Removing the handler
revives errexit and loses #91's resilience, and an inline `set -e` does
not re-arm it. Explicit per-read checks are the mechanism.
Every read inside that subshell is now checked — the issue read on its
status AND on its payload shape (an HTTP 200 whose body is `null` exits
0 and empties the label set just the same), both reads in
`last_issue_activity`, and the comments read in
`issue_comment_has_marker`. On failure the issue is left exactly as it
is, the reason rides its own `#$n:` line, and the subshell exits with a
distinguished status the sweep counts, so a deliberate skip is not
reported as a crash and a genuine crash is still named byte-identically.
`read_failure_reason` moves to lib/read.sh beside a new `guarded_read`,
sourced by both reconcilers: labels-reconcile's copy was the only one,
and the issue surface needs the identical rule.
Refs #247
2026-08-03 18:01:58 +00:00
|
|
|
### Fixed
|
|
|
|
|
|
|
|
|
|
- The issue sweep no longer derives label writes from a read that failed. An
|
|
|
|
|
HTTP 504 whose body is GitHub's JSON error object passed every guard and
|
|
|
|
|
emptied the label set, so a healthy epic was written `needs-triage` and the
|
|
|
|
|
pass reported success (#247).
|
|
|
|
|
- A failed comments read no longer reclaims a live claim. Swallowed, it dated
|
|
|
|
|
the issue by `created_at` and unassigned the builder under a comment
|
|
|
|
|
asserting 48 hours of silence about an issue commented on seconds earlier
|
|
|
|
|
(#247).
|
|
|
|
|
- A failed comments read no longer reads as "no marker", which re-posted the
|
|
|
|
|
comment the marker exists to suppress (#247).
|
|
|
|
|
- Every read inside the per-issue subshell is checked explicitly, on its
|
|
|
|
|
status and on its payload shape; the issue is left exactly as it is and the
|
|
|
|
|
sweep continues. A partial pass names its skipped issues after
|
|
|
|
|
`reconciled.` (#247).
|
fix(issueflow): a per-issue pass commits its whole effect, or none of it
The per-read guards closed the reported class — a failed read never reaches
a decision function — and left one layer standing. A pass could mutate and
only THEN reach a guarded read, fail it, and report the issue as skipped:
`stale` removed, or `needs-triage` minted, under a log line saying the
sweep had touched nothing. That is the same false report #247 exists to
close, told from the other end, and the panel reproduced it on four
separate compositions.
Fixed as the ordering invariant rather than per site. Inside
reconcile_issue_pass's subshell, run() and log() stage their effects, and
commit_staged_effects replays them in order once the pass has completed.
skip_issue emits its own line directly and exits, so the buffer dies with
the subshell. A skip therefore implies zero `gh issue edit`, zero
`gh issue comment`, and no log line about a mutation that never landed —
for compositions nobody has written yet, because reconcile_issue has no way
to mutate directly. Reads stay where they are: they may happen anywhere,
since nothing lands until the end.
Stated per site it would hold until the next composition. Two consequences
worth naming: reconcile_ruling is covered without touching lib/ruling.sh,
because it posts through the sourcing script's run()/log() — the PR surface
keeps its own and is unaffected; and a genuine crash mid-pass now also
lands nothing, where before it left the earlier mutations applied. D4's
handler string, D6's tail and D7's exit 0 are all unchanged, and the
healthy path is byte-identical: every staged write commits under the same
`>/dev/null` its call site already applied.
Refs #247
2026-08-03 18:55:52 +00:00
|
|
|
- A per-issue pass is now atomic: its writes and its log lines commit only
|
|
|
|
|
once the pass completes. A skip could previously land after an earlier
|
|
|
|
|
mutation, reporting an issue as untouched when a label had already been
|
|
|
|
|
written or removed (#247).
|