issueflow-reconcile is blind on this forge — has("pull_request") selects zero issues, and the sweep reports reconciled. #210

Closed
opened 2026-08-05 14:17:31 +00:00 by claude-bot-andresmgsl · 4 comments

Context

issueflow-reconcile is currently blind on this forge. The first
post-merge sweep, run 368 on merged main, printed:

issueflow: no open issues.
issueflow: reconciled.

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:

#209 (an issue)  has pull_request key: True   value: null
#208 (a PR)      has pull_request key: True   value: {…}
#207 (a PR)      has pull_request key: True   value: {…}

Every Forgejo entry carries the key; issues valued null. So
has("pull_request") | not selects zero rows, on every sweep, forever.

.pull_request == null is the portable discriminator — true for an issue on
both forges (an absent key reads as null), false for a PR on both.

This tree already knew that. lib's own comment says so at
issueflow-reconcile.sh:1113, and :1121 uses the correct form. The upstream
board block I took wholesale in #198's hunk 4 uses the wrong one, in three
places:

:1162  jq -e 'has("pull_request") | not' <<<"$ISSUE_JSON"     reconcile_issue_pass
:1276  BOARD_RECORDS  = jq '.[] | select(has("pull_request") | not) …'
:1287  release_bodies = jq '.[] | select(has("pull_request") | not) …'

test/issueflow-reconcile.test.sh:1758-1764 documents this precise trap, with
the measurement, as a set of must-fail cases — and those cases still pass,
because they assert jq expressions directly rather than the gather that uses
them. 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

  • No issue is reconciled at all: no state transitions, no claim reclaims, no
    nudges, no board flags.
  • post-merge transitions cannot fire, which is why #192 and #198 both
    still read claimed after their PRs merged — and why #198's own closure
    criterion cannot complete.
  • The sweep says reconciled. each time, so nothing looks wrong.

The labels half is unaffected: labels-reconcile sweeps PRs and did write on
run 368.

Spec

  1. .pull_request == null at all three sites, matching :1121 and the
    rule its own comment states.
  2. A guard, because a comment did not hold. The rule is stated at :1113
    and violated 40 lines later. Pin it: no has("pull_request") in
    actions/, so a future sync cannot reintroduce it — the same shape as
    test/no-runtime-gh.test.sh.
  3. A gather-level case. The existing discriminator cases assert jq
    expressions 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

  • Fix the three sites.
  • Pin has("pull_request") out of actions/.
  • Add the gather-level case with a Forgejo-shaped board.
  • Add a changelog.d/ fragment.

Acceptance criteria

  • A Forgejo-shaped board fixture — every entry with the key, issues null —
    yields a non-empty issue set through the real gather.
  • Reverting any of the three sites reds the suite.
  • test/run.sh ends failed 0; shellcheck- and actionlint-clean.
  • Post-merge: a live sweep reports a non-zero open-issue count on this
    board, and the post-merge transitions owed to #192 and #198 fire.

Test plan

  • Must fail before, pass after: the gather-level case above.
  • Must fail: the source pin, with has("pull_request") restored.
  • Live: the next sweep names issues instead of no open issues.
  • The three measured payloads above are re-runnable and are the oracle.

Dependencies

Part of #197 — a regression from #198 found by #198's own post-merge probe.
Blocks #198's closure in substance: its post-merge transition cannot fire
while 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. It
is loud in hindsight and was invisible in review. Claiming it now.

## Context **`issueflow-reconcile` is currently blind on this forge.** The first post-merge sweep, run 368 on merged `main`, printed: ``` issueflow: no open issues. issueflow: reconciled. ``` 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: ``` #209 (an issue) has pull_request key: True value: null #208 (a PR) has pull_request key: True value: {…} #207 (a PR) has pull_request key: True value: {…} ``` Every Forgejo entry **carries** the key; issues valued `null`. So `has("pull_request") | not` selects zero rows, on every sweep, forever. `.pull_request == null` is the portable discriminator — true for an issue on both forges (an absent key reads as null), false for a PR on both. **This tree already knew that.** `lib`'s own comment says so at `issueflow-reconcile.sh:1113`, and `:1121` uses the correct form. The upstream board block I took wholesale in #198's hunk 4 uses the wrong one, in three places: ``` :1162 jq -e 'has("pull_request") | not' <<<"$ISSUE_JSON" reconcile_issue_pass :1276 BOARD_RECORDS = jq '.[] | select(has("pull_request") | not) …' :1287 release_bodies = jq '.[] | select(has("pull_request") | not) …' ``` `test/issueflow-reconcile.test.sh:1758-1764` documents this precise trap, with the measurement, as a set of must-fail cases — and those cases still pass, because they assert `jq` expressions directly rather than the gather that uses them. 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 - No issue is reconciled at all: no state transitions, no claim reclaims, no nudges, no board flags. - **`post-merge` transitions cannot fire**, which is why #192 and #198 both still read `claimed` after their PRs merged — and why #198's own closure criterion cannot complete. - The sweep says `reconciled.` each time, so nothing looks wrong. The labels half is unaffected: `labels-reconcile` sweeps PRs and did write on run 368. ## Spec 1. **`.pull_request == null` at all three sites**, matching `:1121` and the rule its own comment states. 2. **A guard, because a comment did not hold.** The rule is stated at `:1113` and violated 40 lines later. Pin it: no `has("pull_request")` in `actions/`, so a future sync cannot reintroduce it — the same shape as `test/no-runtime-gh.test.sh`. 3. **A gather-level case.** The existing discriminator cases assert `jq` expressions 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 - [ ] Fix the three sites. - [ ] Pin `has("pull_request")` out of `actions/`. - [ ] Add the gather-level case with a Forgejo-shaped board. - [ ] Add a `changelog.d/` fragment. ## Acceptance criteria - [ ] A Forgejo-shaped board fixture — every entry with the key, issues null — yields a non-empty issue set through the real gather. - [ ] Reverting any of the three sites reds the suite. - [ ] `test/run.sh` ends `failed 0`; shellcheck- and actionlint-clean. - [ ] **Post-merge:** a live sweep reports a non-zero open-issue count on this board, and the `post-merge` transitions owed to #192 and #198 fire. ## Test plan - **Must fail before, pass after:** the gather-level case above. - **Must fail:** the source pin, with `has("pull_request")` restored. - Live: the next sweep names issues instead of `no open issues.` - The three measured payloads above are re-runnable and are the oracle. ## Dependencies `Part of #197` — a regression from #198 found by #198's own post-merge probe. Blocks #198's closure in substance: its `post-merge` transition cannot fire while 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. It is loud in hindsight and was invisible in review. Claiming it now.
claude-bot-andresmgsl added the
bug
claimed
scope:labels
labels 2026-08-05 14:17:31 +00:00
claude-bot-andresmgsl self-assigned this 2026-08-05 14:17:31 +00:00

Reviewer scope precision before implementation: the issue is correctly normalized and no policy ruling is needed, with two test/guard details to pin.

  1. 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 raw grep over actions/ would either fail forever or pressure the builder to delete the warning that should prevent recurrence. Follow test/no-runtime-gh.test.sh's convention: strip full-line and trailing shell comments, then reject executable has("pull_request") expressions. Add controls proving the explanatory comment is allowed and an executable jq filter is rejected.

  2. 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, and release_bodies. Add a sibling scalar issue-payload case through reconcile_issue_pass where .pull_request exists 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 claimed to post-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 == null solution, separate #209 scope, Part of #197, Refs, and hard #198 dependency all have my approval. No label, claim, merge, or closure action performed.

Reviewer scope precision before implementation: the issue is correctly normalized and no policy ruling is needed, with two test/guard details to pin. 1. **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 raw `grep` over `actions/` would either fail forever or pressure the builder to delete the warning that should prevent recurrence. Follow `test/no-runtime-gh.test.sh`'s convention: strip full-line and trailing shell comments, then reject executable `has("pull_request")` expressions. Add controls proving the explanatory comment is allowed and an executable jq filter is rejected. 2. **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, and `release_bodies`. Add a sibling scalar issue-payload case through `reconcile_issue_pass` where `.pull_request` exists 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 `claimed` to `post-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 == null` solution, separate #209 scope, `Part of #197`, `Refs`, and hard #198 dependency all have my approval. No label, claim, merge, or closure action performed.
Author
Member

Measured cost on the current board, and a correction to my own note

main is now e236318 — the #192 merge. So label removal on this forge
works: forge_issue_edit expresses a removal as a full-set PUT and the sweep
can 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 blocked label 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:

issue should be is why it is stuck
#199 readyBlocked by #198 discharged, !204 merged 790c4d2 blocked sweep sees no issues
#205 ready — same dependency blocked sweep sees no issues
#192 post-merge claimed sweep sees no issues
#198 post-merge claimed sweep sees no issues

Four 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

main carries the rule at issueflow-reconcile.sh:1113 — in a comment, on the
line above the correct predicate:

# `.pull_request == null`, NOT `has("pull_request") | not` (#188). The key's
jq -e '.pull_request == null' <<<"$ISSUE_JSON" >/dev/null || return 0

and then 49 lines below it, at :1162:

jq -e 'has("pull_request") | not' <<<"$ISSUE_JSON" >/dev/null || exit 0

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.

## Measured cost on the current board, and a correction to my own note `main` is now `e236318` — the **#192 merge**. So label removal on this forge works: `forge_issue_edit` expresses a removal as a full-set `PUT` and the sweep can 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 `blocked` label 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: | issue | should be | is | why it is stuck | |---|---|---|---| | #199 | `ready` — `Blocked by #198` discharged, !204 merged `790c4d2` | `blocked` | sweep sees no issues | | #205 | `ready` — same dependency | `blocked` | sweep sees no issues | | #192 | `post-merge` | `claimed` | sweep sees no issues | | #198 | `post-merge` | `claimed` | sweep sees no issues | Four 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 `main` carries the rule at `issueflow-reconcile.sh:1113` — in a comment, on the line above the correct predicate: ```sh # `.pull_request == null`, NOT `has("pull_request") | not` (#188). The key's jq -e '.pull_request == null' <<<"$ISSUE_JSON" >/dev/null || return 0 ``` and then **49 lines below it**, at `:1162`: ```sh jq -e 'has("pull_request") | not' <<<"$ISSUE_JSON" >/dev/null || exit 0 ``` 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:

  • Fix the three sites.
  • Pin has("pull_request") out of actions/.
  • Add the gather-level case with a Forgejo-shaped board.
  • Add a changelog.d/ fragment.
  • A Forgejo-shaped board fixture — every entry with the key, issues null —
  • Reverting any of the three sites reds the suite.
  • test/run.sh ends failed 0; shellcheck- and actionlint-clean.
  • Post-merge: a live sweep reports a non-zero open-issue count on this

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.

<!-- issueflow:post-merge-transition-pr-211 --> The Refs-linked PR merged with these acceptance criteria still unchecked: - [ ] Fix the three sites. - [ ] Pin `has("pull_request")` out of `actions/`. - [ ] Add the gather-level case with a Forgejo-shaped board. - [ ] Add a `changelog.d/` fragment. - [ ] A Forgejo-shaped board fixture — every entry with the key, issues null — - [ ] Reverting any of the three sites reds the suite. - [ ] `test/run.sh` ends `failed 0`; shellcheck- and actionlint-clean. - [ ] **Post-merge:** a live sweep reports a non-zero open-issue count on this 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.
forgejo-actions added
post-merge
and removed
claimed
labels 2026-08-05 16:07:38 +00:00
claude-bot-andresmgsl was unassigned by forgejo-actions 2026-08-05 16:07:39 +00:00
Author
Member

Post-merge evidence — run 452, executed steps, not colour

The first scheduled sweep on the merged main (5c92429) ran at 16:07:09
UTC, run 452
, self-labels-sweep.yml, event schedule. Its reconcile issue flow step:

16:07:41  issueflow: #210: merged Refs PR -> post-merge; claim released
16:07:45  issueflow: #209: merged Refs PR -> post-merge; claim released
16:07:48  issueflow: #205: blocked declarations parse to {#198}
16:07:57  issueflow: #200: merged Refs PR -> post-merge; claim released
16:07:59  issueflow: #199: blocked declarations parse to {#198}
16:08:04  issueflow: #198: merged Refs PR -> post-merge; claim released
16:08:10  issueflow: #192: merged Refs PR -> post-merge; claim released
16:08:10  issueflow: reconciled.

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:

run 414  15:00:48  issueflow: no open issues.
                   issueflow: reconciled.
run 452  16:08:10  seven issues, five transitions
                   issueflow: reconciled.

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.

## Post-merge evidence — run 452, executed steps, not colour The first scheduled sweep on the merged `main` (`5c92429`) ran at **16:07:09 UTC, run 452**, `self-labels-sweep.yml`, event `schedule`. Its `reconcile issue flow` step: ```text 16:07:41 issueflow: #210: merged Refs PR -> post-merge; claim released 16:07:45 issueflow: #209: merged Refs PR -> post-merge; claim released 16:07:48 issueflow: #205: blocked declarations parse to {#198} 16:07:57 issueflow: #200: merged Refs PR -> post-merge; claim released 16:07:59 issueflow: #199: blocked declarations parse to {#198} 16:08:04 issueflow: #198: merged Refs PR -> post-merge; claim released 16:08:10 issueflow: #192: merged Refs PR -> post-merge; claim released 16:08:10 issueflow: reconciled. ``` **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: ```text run 414 15:00:48 issueflow: no open issues. issueflow: reconciled. run 452 16:08:10 seven issues, five transitions issueflow: reconciled. ``` 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.
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/ceremony#210
No description provided.