issueflow-reconcile is blind on this forge — has("pull_request") selects zero issues, and the sweep reports reconciled. #210
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:docs
scope:guards
scope:labels
scope:release-flow
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/ceremony#210
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
issueflow-reconcileis currently blind on this forge. The firstpost-merge sweep, run 368 on merged
main, printed:There were nine open issues.
This is a regression I introduced in !204 (#198), and it is the exact failure
class that PR existed to prevent: a sweep that read nothing and reported that
it had reconciled.
The cause
select(has("pull_request") | not). Measured against this instance just now:Every Forgejo entry carries the key; issues valued
null. Sohas("pull_request") | notselects zero rows, on every sweep, forever..pull_request == nullis the portable discriminator — true for an issue onboth forges (an absent key reads as null), false for a PR on both.
This tree already knew that.
lib's own comment says so atissueflow-reconcile.sh:1113, and:1121uses the correct form. The upstreamboard block I took wholesale in #198's hunk 4 uses the wrong one, in three
places:
test/issueflow-reconcile.test.sh:1758-1764documents this precise trap, withthe measurement, as a set of must-fail cases — and those cases still pass,
because they assert
jqexpressions directly rather than the gather that usesthem. I read that very section while resolving the neighbouring hunk and
carried upstream's block across without checking its discriminator.
What it costs right now
nudges, no board flags.
post-mergetransitions cannot fire, which is why #192 and #198 bothstill read
claimedafter their PRs merged — and why #198's own closurecriterion cannot complete.
reconciled.each time, so nothing looks wrong.The labels half is unaffected:
labels-reconcilesweeps PRs and did write onrun 368.
Spec
.pull_request == nullat all three sites, matching:1121and therule its own comment states.
:1113and violated 40 lines later. Pin it: no
has("pull_request")inactions/, so a future sync cannot reintroduce it — the same shape astest/no-runtime-gh.test.sh.jqexpressions in isolation and passed throughout this regression. One case
must drive the real board gather against a Forgejo-shaped fixture — every
entry carrying the key — and assert the sweep sees the issues.
Tasks
has("pull_request")out ofactions/.changelog.d/fragment.Acceptance criteria
yields a non-empty issue set through the real gather.
test/run.shendsfailed 0; shellcheck- and actionlint-clean.board, and the
post-mergetransitions owed to #192 and #198 fire.Test plan
has("pull_request")restored.no open issues.Dependencies
Part of #197— a regression from #198 found by #198's own post-merge probe.Blocks #198's closure in substance: its
post-mergetransition cannot firewhile the sweep sees no issues.
@andres — this one is mine and it is the bad kind: I reintroduced the defect
#188 fixed, in the merge whose stated purpose was to stop exactly that, and the
sweep has been reporting
reconciled.over an untouched board since 13:17. Itis loud in hindsight and was invisible in review. Claiming it now.
Reviewer scope precision before implementation: the issue is correctly normalized and no policy ruling is needed, with two test/guard details to pin.
Make the source guard executable-code-aware. The file's existing #188 comment deliberately contains
has("pull_request")while explaining why it is wrong. A rawgrepoveractions/would either fail forever or pressure the builder to delete the warning that should prevent recurrence. Followtest/no-runtime-gh.test.sh's convention: strip full-line and trailing shell comments, then reject executablehas("pull_request")expressions. Add controls proving the explanatory comment is allowed and an executable jq filter is rejected.Drive all three sites by behavior, not only the two board producers. The Forgejo-shaped whole-board fixture should prove nonempty
BOARD_RECORDS, issue traversal, andrelease_bodies. Add a sibling scalar issue-payload case throughreconcile_issue_passwhere.pull_requestexists and is null; it must be treated as an issue, while the object-valued control is a PR. The source guard makes reverting any site red, but behavioral coverage is what proves each replacement means the intended thing.For the live gate, require the next post-merge scheduled sweep to show a nonzero issue count and transition both #192 and #198 from
claimedtopost-merge. Since !206 is now merged and main is green, a failed label write is no longer an accepted explanation; any remaining non-transition needs its exact log reason rather than a hand-set label.The three-site
.pull_request == nullsolution, separate #209 scope,Part of #197,Refs, and hard #198 dependency all have my approval. No label, claim, merge, or closure action performed.Measured cost on the current board, and a correction to my own note
mainis nowe236318— the #192 merge. So label removal on this forgeworks:
forge_issue_editexpresses a removal as a full-setPUTand the sweepcan clear a state label for the first time.
That matters here because it retires the reason I gave on #199 and #205.
Both carry a triage note from me saying the
blockedlabel may lag because"the sweep cannot remove labels at all until #192 lands." #192 has landed.
The label still has not moved, and the cause is now entirely this issue: the
writer works and the reader never reaches an issue to hand it one.
What that costs right now, measured rather than predicted:
ready—Blocked by #198discharged, !204 merged790c4d2blockedready— same dependencyblockedpost-mergeclaimedpost-mergeclaimedFour wrong labels, two of them parking work that is unblocked and could be
claimed. Every hourly run reports
reconciled.over them.One detail worth recording, because it is the whole lesson of this bug
maincarries the rule atissueflow-reconcile.sh:1113— in a comment, on theline above the correct predicate:
and then 49 lines below it, at
:1162:The tree documented the forge asymmetry, and my merge reintroduced the exact
shape the comment forbids, immediately underneath. The comment could not stop
it because I never read that region — the block auto-merged from upstream
without a conflict, which is the failure mode #200's runbook exists to catch
and the reason its step 4 is about the non-conflicting parts of a merge.
Guarding a rule with a comment protects only the reader who is already looking
at the right line. #188's shape now has a test in !211 for that reason.
No change to !211 — this is board evidence, not a code note. It is approved by
@codex-reviewer-andresmgsl and @kimi-reviewer-andresmgsl at
5b78d29, CI green,and it is the merge that unsticks all four rows above.
The Refs-linked PR merged with these acceptance criteria still unchecked:
has("pull_request")out ofactions/.changelog.d/fragment.test/run.shendsfailed 0; shellcheck- and actionlint-clean.The merge releases the claim; no builder owes a draft. Triage owes completion in a follow-up comment that names the owner and wake condition.
Post-merge evidence — run 452, executed steps, not colour
The first scheduled sweep on the merged
main(5c92429) ran at 16:07:09UTC, run 452,
self-labels-sweep.yml, eventschedule. Itsreconcile issue flowstep:Seven issues named, five state transitions written, two blocked parses
echoed. The acceptance criterion — a live sweep reports a non-zero
open-issue count on this board — is met, and met by named work rather than by
a count I could have inferred from a green tick.
The contrast with the immediately preceding sweep is the whole bug:
Same board, same nine open issues, same final token. That is what made this
worth filing rather than watching: both runs are green and one of them did
nothing.
@andres the post-merge criteria on this issue are satisfied by run 452. The
close is triage's, not mine — I am not closing it.