@codex-reviewer-andresmgsl's four gaps, all real, all taken.
1. PRESERVED IDS COME FROM THE ISSUE. The removal path read only .labels[].name
and then re-resolved every preserved label through the repository-wide list
— so preservation depended on a paginated read with nothing to do with this
issue, and an incomplete one would drop a bystander. It now keeps
name<TAB>id from the issue payload, subtracts removals by name, and resolves
ONLY added names. Fixture: a bystander on the issue with id 14 that is
absent from the repo-list fixture entirely must still survive the PUT.
2. AN ABSENT REMOVAL WRITES NOTHING. I had it PUT the unchanged set, arguing
the write proved the sweep reached the forge. The GET already proves that,
and replacing a set with itself opens ceremony#128's window for no state
change — most calls here are exactly this case, since the reconcilers call
--remove-label unconditionally. Short-circuits when the wanted set equals
the current one. This was the policy-shaped choice flagged for @andres; the
reviewer's reasoning is better than mine was.
3. EVERY LABEL MUTATION REACHES THE TALLY. The marker was only on the primary
state edit, so clearing `merge-next` and both `stale` edits could fail into
the generic per-PR branch and still finish `reconciled.` and exit 0. All
four sites go through one `label_write` helper, so a future call site cannot
reopen it by forgetting to mark itself. Probe: a failed NON-primary write
(unstale on a blocked PR) must fail the sweep.
4. NO SUCCESS TOKEN IN A FAILURE TAIL. "NOT reconciled." still contains
"reconciled.", which a log-tail consumer greps for. The line is now
"sweep incomplete", and the test asserts the whole output is free of the
token rather than only of the success prefix.
Also added the two fault boundaries the acceptance plan named and the fixtures
never proved: a failed current-label GET and a failed replacement PUT, each
non-zero with the backend's verb/path/status diagnostic.
Mutation-tested, each gap separately: bypassing the tally reds 3, re-resolving
preserved ids reds 7, writing the unchanged set reds 1.
forge-backends 115/115 (was 110), labels-reconcile 175/175 (was 172),
test/run.sh 22/22 under jq 1.7 and jq 1.6, shellcheck 0.10.0 and actionlint
clean.
Refs #192
Two defects, one cause, and the second is why the first survived a week.
THE WRITE. Removal was a per-label `DELETE .../labels/{id}` loop. On this
instance that call returns HTTP 500 for every removal under the token the
sweep actually holds — measured inside Actions, probe run 701, where the same
`PUT .../labels` with the desired full set returns 200 including the empty set
for a full clear. A PAT gets 204 on the same DELETE, which is exactly why it
went unseen: it fails only for `${{ github.token }}`.
Net effect before this: on Forgejo the state machine could only ever ADD
labels. Every `state:*` transition needing the previous state cleared and every
`blocker:*` that should lift was inert. Both PRs open right now carry stale
`blocker:*` labels that are false and that nothing can remove.
So the removal path is read-current, compute-wanted, one PUT — the same shape
the assignee branch beside it already used. An ADD-ONLY call keeps its additive
POST: ceremony#128 lost a `release` label to a read-modify-write that clobbered
a concurrent set, and forge_labels_add stays pinned against ever doing that.
The window is accepted here and only here, where the caller asked to REMOVE
and no additive verb can say that. An unresolvable --add-label refuses before
any write, so a replacement PUT can never drop a label nobody asked to remove.
THE REPORTING. `labels-reconcile` logged `WARNING: label edit failed`, fell
through, and `main` printed `reconciled.` and exited 0 — while
`issueflow-reconcile` treated the identical 500 as fatal. One cause, two
contradictory policies, and the wrong one hid the write defect.
A failed write is fatal now, and the tally reaches main's exit code. That
second half is load-bearing: making reconcile_pr fatal alone is not enough,
because the loop swallows a per-PR non-zero into a log line and finishes. The
per-PR tolerance is right and stays — one bad PR must not blind the board — but
it now applies to READS. A sweep that could not write exits non-zero and never
prints `reconciled.`
The diagnostic says what was attempted and that it did not happen. The old text
blamed a missing label and told the operator to bootstrap, when the label was
present and the call returned 500 — #101's rule is report, do not diagnose.
Mutation-tested, all three ways: restoring the warn-and-continue reds 5 cases,
removing the tally reds 2, restoring the DELETE loop reds 7.
test/run.sh 22 files 0 failed under jq 1.7 and jq 1.6; shellcheck 0.10.0 and
actionlint clean.
Refs #192
@codex-reviewer-andresmgsl's second review, both points taken.
The refs action goes back to `forge_preflight || exit 1`. 97e63ac had it exit 0
with a notice so the PR check would not be red, and that conflated two
different questions: "this action cannot produce a verdict" is the ACTION's
contract and must stay a refusal, while "this check should not block the
board" is the CALLER's decision. The caller now carries it —
refs-guard.yml skips unless github.server_url is github.com, mirroring
forge_detect positively. A skipped check is a green head; an action that
reports success it did not earn is not. The leaked preflight_err temp file
goes with the revert.
The workflow guard asked the wrong question. `command -v gh` alone passes the
moment a Forgejo runner image happens to ship gh, and then dispatches against
a forge that cannot serve it — the client/forge mismatch forge_preflight
exists to prevent. It decides the FORGE first now, mirroring forge_detect
positively, and the binary second. The source guard splits to match: a
declaration guarded only by binary presence is reported, with a fixture that
fails on exactly that shape.
The warning text was also wrong on the facts, as noted: issue-event sweeps ARE
this caller's event-driven wakes, so they are precisely what is lost. It now
says the hourly scheduled sweep survives and every event-driven wake through
this caller does not, until #205.
Point 1 of that review — jq 1.6 accepting an empty payload — was already fixed
in 728102a, pushed before the review landed.
Verified under the runner's jq 1.6 as well as 1.7: 28 test files, 0 failed
both ways. shellcheck 0.10.0 (CI's pin), actionlint, self-ref, marker,
vendored, changelog-armed all clean with every file tracked.
Refs #198
`CI / test` was red at 97e63ac on a case that passes on this box: upstream's
own "an empty payload is refused". The cause is not the test.
`jq -e` disagrees with itself across versions on EMPTY input. jq 1.7 exits 4 —
no valid result was ever produced. jq 1.6 exits 0. Measured both ways today
against the same filter. This instance's runner image
(ghcr.io/catthehacker/ubuntu:act-22.04) carries jq 1.6.
So on this forge the guard #247 D3 added specifically to refuse an unreadable
read was ACCEPTING one: an empty body read as a valid issue payload, and the
sweep would have reconciled an issue from a payload it never received. The
test is upstream's, it is correct, and it passes on a GitHub runner — which is
why upstream never saw this.
The fix does not depend on jq's exit code for an input it never receives: the
payload is read, emptiness is decided in the shell, and jq judges only a
non-empty body.
Verified under BOTH jq versions, not just the one on this box: empty refused
and healthy accepted on 1.6 and 1.7, and the whole suite green under jq 1.6 —
28 test files, 0 failed — as well as under 1.7.
Refs #198
@codex-reviewer-andresmgsl's blocker 1 is right and the filename exemption was
the wrong shape. It exempted the whole FILE — any later `gh` call anywhere in
labels.yml would have ridden in free — and it let the merge ship a step that
dies with `command not found` on every sweep on this forge, which #197's bar
does not permit.
The declaration mechanism already existed; a workflow simply could not reach
it. It can: `CEREMONY_FORGE_CLIENT: gh` in the step's env is the same
declaration actions/refs-not-closing carries, and the refusal that a script
gets from forge_preflight is inline here because a workflow has no shell to
call it from. The dispatch now warns by name, cites #205, and exits 0 rather
than reddening every sweep for a known gap.
So the guard needs no exemption list at all. It now requires the pair —
declared AND refusing — and reports a declaration that carries no refusal,
which is a permission slip for `command not found`.
That predicate was wrong on its first write, and its mutation test caught it:
`refuses_when_unavailable` matched the word `forge_preflight` inside
labels.yml's own comment explaining that it has NO forge_preflight to call. A
guard reading prose as evidence is the blind sweep again, in the guard written
to forbid it. Comments are stripped now, as gh_calls already stripped them.
Blocker 4: the nudge strips a trailing slash from the server URL. Reverting the
strip reds two cases.
Blockers 2 and 3 were already fixed in 97e63ac, before either review landed.
test/run.sh 28 files 0 failed under CI's env; shellcheck 0.10.0 (CI's pin),
actionlint, self-ref, marker, vendored and changelog-armed all clean, with
every file tracked this time.
Refs #198
The first head's `Refs guard` failed on this PR, correctly: spec 4's
CEREMONY_FORGE_CLIENT=gh declaration made forge_preflight refuse by name on
this forge. But that workflow runs on every pull request here, so the
declaration as first written turns every future PR red until #199 lands —
blocking the board for a gap that already has its own issue.
Refusing and scheduling are different questions. This action must never
produce a verdict from a graph it did not read, and it does not: on a forge it
cannot speak it now says so by name, cites #199, states that no verdict was
produced, and reaches the forge zero times. A preflight failure for any other
reason stays fatal, and on a forge it CAN speak nothing changes.
Also: five SC2016 findings in test/no-runtime-gh.test.sh. They were invisible
locally because shellcheck-all.sh lints TRACKED files and the guard was still
untracked when I ran it — a new file is exactly the case that check cannot
see. Verified this time against CI's pinned shellcheck 0.10.0 with the file
committed.
test/run.sh: 28 test files, 0 failed, under CI's CEREMONY_REQUIRE_* env.
shellcheck, actionlint, self-ref, marker and vendored guards all clean.
Refs #198
`git merge` of upstream `8c3a4d1` onto `dad99dd`, common ancestor `84bb1a4`.
18 hunks in 10 files; `lib/forge.sh`, `lib/forge-github.sh` and
`lib/forge-forgejo.sh` conflict in none and come out byte-identical.
The resolutions the issue decided: VERSION and both CEREMONY_SELF_REF
carriers take upstream's numbers; `.github/labels.conf` and `drills/0.4.1.md`
keep this forge's; CHANGELOG keeps both sides and names the upstream commit
this tree carries.
The part the hunks did not contain. Upstream's 0.5.0/0.6.0 work added whole
functions to files this tree already owned, so `git merge` took its side
without raising a conflict — and with them, EIGHT runtime `gh` call sites
that #188 had removed. Seven are ported onto the shim: two reads and four
comment writes in issueflow-reconcile, and labels-reconcile's HEAD_COMMIT_AT
read. The eighth is `gh workflow run` in labels.yml, which a workflow cannot
declare a client for and whose Forgejo equivalent this instance answers with
500 rather than a 4xx — named with its reason rather than ported on a guess.
test/no-runtime-gh.test.sh makes the rule mechanical, because reviewing the
diff could not: four reviewers reading it each found a different subset, and
the contract suite stubs `gh`, so a reintroduced call site passes it.
Three seams the resolution decides are silent when resolved wrongly, and each
now has a case that fails on the wrong one: the merged record's `merged_at`
third column (without it every sort key ties and the highest PR number comes
back), the open gather's one-BODY-row-per-line feed (a whole decoded body as
one record loses every declaration including the first), and the whole-board
read whose COLLISION_FLAGS/WINDOW_FLAGS consumers auto-merged.
The open gather carries CLOSING rows as well as BODY rows. `Refs` alone would
drop every `Closes #N` link on the open side and reclaim a claim the PR was
holding — the existing base64 round-trip case is red without it.
actions/refs-not-closing declares CEREMONY_FORGE_CLIENT=gh: its only gather
is GraphQL, which Forgejo does not serve at all. #199 ports it.
test/run.sh: 28 test files, 0 failed. shellcheck and actionlint clean.
Refs #198
B1: deliverable_keys answered a multiset, so a `+` title whose segments
normalize to one key made collision_flags find the issue adjacent to
itself and chain it to its own number — the comment asked an issue to
declare `Blocked by` itself, and two such carriers corrupted the chain
between them. The keys are deduped where the set property belongs.
B2: window_in_scope excluded only blocked/epic/post-merge, admitting
`needs-triage` and label-less issues, so the sweep could add
`needs-triage` to an issue and then tell it about a mint-time membership
call in the same pass. Both flags now call one unblocked_claimable
predicate — #293 D2 corrected gives one gloss on `unblocked` and D3b
says D3 uses it.
The window log line says "unblocked", not "ready": D3b corrected exactly
that wording, and the flag fires on `claimed` too.
Both checks ride the existing sweep walk and write nothing but comments
(#293 D1). The board read now answers the whole payload, because both
decisions are over the WHOLE board — every open issue's labels and title,
and every open release issue's body — and a second pagination for the same
rows would be a second board free to disagree with this one mid-sweep.
Fixtures still owed.
Claiming a needs-ruling issue dated it through the assigned timeline
event, silencing the 7-day escalation nudge at exactly the moment a
builder started working through it. The ruling block now reads
last_issue_comment_activity (D1); the reclaim clock keeps the assignment
(D2) because there the assignment IS the claim; post-merge hands its
evidence read to the ruling block instead of reading again (D6, D7); the
claimed branch reads both clocks at its top, before anything it posts.
LABELS.md and lib/ruling.sh now say what each surface's clock reads
(D4, D5). The #257-era order compositions move to the comments read —
the timeline is no longer an input the issue clocks take, and its
unreadability no longer holds unrelated writes hostage; that narrowing
is pinned rather than implied.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"no activity for N days" was true of the old clock and is now imprecise:
an assignment no longer counts, so an item assigned yesterday would read
a nudge claiming nine days of nothing. It says "no comment" instead —
the fact the sweep actually read.
Refs #254
The nudge rode `last_issue_activity`, which counts `assigned` timeline
events because assignment is the claim the 48-hour reclaim protects.
`post-merge` has no claim: an assignee there is the invalid composition
the flag beside it reports, so counting the assignment let a broken board
buy the item another 7 days of silence — this issue's failure direction
taken backwards.
One computation, two clocks over it: `issue_activity_at` is the body,
`last_issue_activity` keeps the reclaim and ruling clocks byte-identical,
and `last_issue_comment_activity` is the evidence clock. Both clocks are
read before this branch posts anything, the ruling one included — read
after, it would date the issue by the evidence nudge's own comment and
silence the ruling nudge, which is the self-silencing the branch already
guarded against in the other direction.
Refs #254
WIP checkpoint: the nudge itself, tests still owed. Reuses
ruling_nudge_decision so the 7-day rule keeps one spelling, addresses the
triage actor (post-merge is triage's completion queue), and carries no
idempotency marker — the comment is itself activity, so it self-rate-limits.
Refs #254
The echo body wrote `no longer blocked by #9` unbackticked, twice in one
sentence. GitHub linkifies both, so every echo posted a pair of "mentioned in"
events onto this repo's issue #9 — an issue with nothing to do with the one
being echoed. On a 15-minute cron that is one pair per `blocked` issue on the
board, and the first sweep after merge would have written the whole board's
worth at once.
The file's own convention two branches down already avoids this: the
`blocked-unparseable` comment code-spans its `Blocked by #N` for exactly this
reason. The prose is unchanged, the rendered set is unchanged, and the marker
digests the parsed set rather than the comment body, so no marker moves.
ensure_comment's any-occurrence grep answers "have I ever said this", which
is right for a flag like blocked-unparseable and wrong for a value that
changes. A -> B -> A found A's own first echo and stayed silent, leaving the
thread's newest echo asserting B while the sweep gated on A: a stale parse
presented as the current one, and the third edit did change the parsed set,
so the criterion says it speaks.
blocked_parse_echo_needed compares this parse's marker against the LAST
blockers-parsed-* marker on the thread. The read stays inside guarded_read /
skip_issue, so an unreadable history still fails closed (#247 D1) rather than
answering "nothing echoed yet" and re-posting. ensure_comment is untouched
for every other caller.
Refs #252
The marker claimed to be scoped to the parsed set's value and was scoped
to a lossy rendering of it: `tr -c '[:alnum:]' '-'` maps `acme/widgets#9`
and `acme-widgets#9` — both parses this reconciler accepts — onto one
marker, so a declaration edited between them found the old echo and said
nothing. Silence in exactly the case the echo exists to speak about.
The identity is now a digest of the exact rendered set. The readable slug
stays in front of it and decides nothing. Distinguishing `/` would have
closed the reported pair and left the class: `-`, `_` and `.` are all
legal in a qualifier and all collapse the same way, so all four are
pinned, and the sweep probe observes the second echo actually landing.
Refs #252
The idempotency contract is the marker's scope, so both directions are
pinned: an unchanged set must reuse its marker (or a 15-minute cron
repeats itself forever) and a changed one must not (or a misparse hides
under a marker the thread already carries). crew#308's negated clause is
replayed through the sweep, and the empty parse is echoed beside the
untouched `blocked-unparseable` flag.
Refs #252
The clause parse is exact and unforgiving, and its output was invisible:
every incident in this class was found by a human running the parser by
hand, hours or days late. The sweep now states what it read — one marker
comment per distinct parsed set, comment-only, no label writes.
Refs #252
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
`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
The predicate's fixtures cannot see the read that sets HEAD_COMMIT_AT, so a
sweep probe drives it both ways: read, and the blocker is written off a dated
head; denied, and the denial is named on its own line while the state still
converges — this read narrows one blocker, it does not skip the PR the way an
unreadable rollup does. Renaming the assignment reds the probe.
The read also moves after the mergeability/checks skip: a PR the sweep walks
away from must not pay for a call whose only consumer is a blocker that pass
will never decide.
Refs #236
`blocker:unrequested` is the one blocker that names an act the author must
perform, and it never knew whether performing it was permitted. BUILDER.md's
review round requires a green check at the head before requesting, so a
builder waiting out a pending run is complying — and the blocker fired on
compliance (crew#318 ~12:44Z, ceremony#235 12:30Z, both 2026-08-03).
Gate the branch on CHECKS ∈ SUCCESS | NONE (D1): PENDING is CI's move, which
state:addressing already says, and FAILURE belongs to blocker:ci-red rather
than to a second label on the same stall. Then require the supporting facts —
the head's own date and the round's newest submitted review — to have stood
for RECONCILE_UNREQUESTED_GRACE (default 300s, D2), measured off those
timestamps because this sweep is stateless per pass. A timestamp that cannot
be read refuses the blocker.
Refs #236
issue_probe's merged-PR argument becomes a spec list — `PR` or `PR@<iso>`
— so a probe can state merge order; the bare form keeps every existing call
site literal.
The direct-drive cases cover crew#176's shape (the lower number merged
later), agreeing orders, interleaved issues, the mergedAt tie broken by
highest PR number under both input orders, and the empty answer. The
end-to-end probe is crew#321's: a marker already standing for the
later-merged, lower-numbered PR must suppress the transition, which
selecting by number could never do.
Two static pins keep the request count honest — the sweep issues exactly two
GraphQL queries, with mergedAt selected on the merged-PR node it already
fetched.
Refs #242
post_merge_pr_for_issue answered "which merged Refs PR is this issue's
deliverable?" with sort -n | tail -n1. Merge order is not number order:
crew#176's two Refs PRs merged #184 at 19:05:16Z and #182 at 19:05:18Z.
MERGED_REF_PR_RECORDS gains mergedAt as a third column — a field on the
merged-PR node set already fetched, so no additional GraphQL request — and
the selection sorts on it, breaking ties by highest PR number so the answer
never depends on input order.
Refs #242
Codex 1566 held APPROVE: only Forgejo stubs covered forge_timeline and
forge_pr_activity. Pin the github twins as 1:1 extractions (timeline
paginate; issue comments + flat pulls comments + commits).
Cluade #4879: drop 2>/dev/null on the labels-reconcile activity call site
so a failed read still degrades last_activity but names the failure in the
job log (keep || true).
Panel-unanimous batch that was staged unpushed on 57abe15 (#4853):
- forge_timeline: project Forgejo label events into the GitHub shape
so the ruling ladder fires on this forge (measured mapping #4849)
- forge_pr_activity: stop calling /pulls/{n}/comments (404 here); use
reviews with comments_count > 0 for inline comments (#4844)
- ci.yml: install shellcheck before lint, mirroring actionlint — the
act-22.04 runner image does not ship it
Status captured before jq so an unreadable timeline cannot report empty.
Found by rehearsing DRY_RUN against heavy-duty/rig's live board, not by any
test. issueflow swept ZERO issues on Forgejo and printed "reconciled." — the
blind sweep again, one layer in, and invisible because the log is identical
to a legitimately empty queue.
Measured on the two list endpoints, 2026-08-02:
GitHub plain issues OMIT pull_request -> 0 of 9 carried the key
Forgejo every entry HAS it, null on issues -> 10 of 10 carried it
So `select(has("pull_request") | not)` matched nothing here. Replaced with
`.pull_request == null`, which is true for an issue on both forges (an absent
key reads as null) and false for a PR on both. Verified against both live
list endpoints: Forgejo 10 open issues, GitHub 9 — each matching its API.
With the fix the sweep produces real decisions against rig rather than
silence: needs-triage on six issues with no queue state, the merged-Refs
post-merge transition on #133, and the conflicting-queue-labels flag on #129.
The regression test keeps the old expression as a must-fail: it disagrees
with the new one on exactly the Forgejo shape.
Refs #188
Found by running ceremony's own CI at 9357f09 on a real Forgejo runner
rather than reasoning about it.
`label` is a reserved word in jq's grammar (`label $out | … | break $out`),
so jq **1.6** rejects `$label` outright:
jq: error: syntax error, unexpected label, expecting IDENT
jq 1.7 parses it, which is why this survived: GitHub's hosted ubuntu-latest
ships 1.7, and ghcr.io/catthehacker/ubuntu:act-22.04 — the image this
instance maps ubuntu-latest to — ships 1.6. So parse_labeler_config died on
a compile error before it read a byte of config, and EVERY scope derivation
on this forge failed. Renamed to $lbl in the jq program only; the bash
locals keep their names.
Also makes test/forge.test.sh hermetic. Its "github + gh passes" case
depended on gh being on the HOST's PATH, so it passed on a developer box and
failed in the runner image, which has no gh. The preflight cases now run
against stub binaries, and the missing-binary refusal gets its own arm on a
PATH carrying the shell and text tools but no clients — the condition under
test, rather than whatever the machine happens to have.
Verified in both environments: local (jq 1.7, gh present) and the runner
image (jq 1.6, no gh) — shellcheck 0, 22 files 0 failed in each.
Refs #188
@codex-reviewer-andresmgsl #4780, concurred by @grok-reviewer-andresmgsl
#4785. All four real.
1. Three issueflow call sites still named per_page=100. The backend
sanitized it so it worked, but the frozen term and the changelog both say
no call site names a page size — and a contract that holds only because
something downstream cleans up is not the contract. Endpoints now carry
their logical query alone.
2. The suite's summary and `[ "$fail" -eq 0 ]` gate sat in the MIDDLE of
test/labels-reconcile.test.sh, and the eight outstanding_requests expects
were appended after them. Proven before fixing: a deliberately broken
term-4 assertion printed FAIL, was excluded from the totals, and the
suite still exited 0. Those assertions were decorative. The gate moves to
the true end, with a note that nothing goes below it; the reported count
goes 157 -> 164, which is the eight that were never being counted.
3. forge_labels_add and forge_request_reviewer arrived with the port and had
no boundary pins. Both backends now have them, and the labels_add cases
pin the property ceremony#128 turns on: an additive POST, never a PUT of
the whole set, exactly one write so nothing is read-modify-written.
Mutation-verified — making it RMW/PUT, or routing github through
`issue edit --add-label`, each red their own cases.
4. The historical comment said the old gathers were `forge_api graphql`. My
own mechanical port rewrote it; before #188 they were `gh api graphql`
and the abstraction did not exist.
Refs #188
Term 4. GitHub clears requested_reviewers when a verdict lands, so the field
answers "who still owes a verdict" by itself. Forgejo never clears it —
measured: rig!140 listed all three panelists with all three verdicts in, and
rig!146 still lists three while MERGED, so the field is stale even on a
closed PR.
Read raw on Forgejo that is not a cosmetic over-count. `requested` drives
three decisions, and a permanently-true field pins a PR at
state:bots-reviewing for life and stops blocker:unrequested from ever being
true: the sweep believes a round is live forever and no staleness can
correct it.
So the requested set is intersected with who has NOT submitted a verdict for
the current head, derived from /pulls/{n}/reviews — the read that is true on
both forges. On GitHub the filter removes nothing, because the field is
already accurate; term 5 holds by construction rather than by care.
A STALE approval — an approval of an older head — still owes a verdict. That
is the case that matters: treating it as answered would let a stale round
read as complete, which is the shape #136 exists to prevent.
Mutation-verified both ways: reading the field raw again reds three cases,
and treating STALE as answered reds two.
Also documents @grok-reviewer-andresmgsl's ask (#4763): every panel= account
must be able to read the repo, or the forge refuses the review request —
422 naming the account on Forgejo. A real failure mode for private
consumers, and it fails loudly rather than sweeping blind.
Refs #188
Term 1 completed. All 52 runtime gh call sites in the three reconcilers and
lib/ruling.sh now go through forge_* verbs; the three remaining matches in
labels-reconcile are prose in comments. lib/facts.sh is deliberately
untouched — it is the release door, and the ruling keeps release.yml out of
this issue.
The CEREMONY_FORGE_CLIENT:-gh wrappers die here, in the same commit as the
sites they described, so the tree is never in a state where the declaration
lies. main() now runs forge_preflight then forge_select "".
Two sites needed judgment rather than substitution:
- labels-scope's write is forge_labels_add, a genuine additive POST on
both backends, NOT forge_issue_edit --add-label. ceremony#128 turns on
that write not being a read-modify-PUT: the labeler action computed
(labels-at-job-start union derived) and PUT the whole set, silently
dropping a label applied while the job ran. Routing it through a generic
edit verb would have quietly reopened that.
- the human-review request is forge_request_reviewer. Contrary to my
earlier reading, POST /pulls/{n}/requested_reviewers DOES exist on
Forgejo — 422 naming the reviewer's access without it, 201 with it. The
earlier 404 was a GET, which the endpoint does not serve, plus a
username that did not exist.
Test churn, all of it the term-5 boundary move:
- the suites select the github backend, so their existing gh() stubs stay
the boundary and keep intercepting;
- stubs strip the paging the shim injects, so fixtures stay keyed on the
logical endpoint (inlined in the PATH stub, which is a standalone
executable and cannot see a shell function);
- fixtures renamed off the per_page suffix for the same reason;
- recorded-mutation assertions now match the verb, not the raw gh line;
- gh() stubs carry SC2317: they are reached through the backend now, so
shellcheck can no longer see the call path.
Refs #188
Term 3 of #188. Forgejo has no GraphQL API, so these two gathers could not
be translated — there is no endpoint to translate them to. A real
forgejo-runner job says so from the other side: GITHUB_GRAPHQL_URL arrives
set to the empty string (probe task 278).
MERGED_REF_PR_RECORDS was already a body parse; GraphQL was buying
pagination, nothing semantic. OPEN_PR_ISSUES used GitHub's own parse of the
closing keywords, so it becomes lib/closes_references.sh — a sibling of
refs_references, sharing its LOCAL/CROSS classifier so rig#112 can still
never be read as local #112 (#61).
Both gathers now read /pulls, which /api/v3 and /api/v1 return in the same
shape (measured on both). merged_at replaces GraphQL's states: MERGED.
Bodies travel base64: jq's @tsv escapes a newline to a literal backslash-n,
which a line parser reads as one line and loses every declaration after the
first.
The accepted delta, written down rather than rediscovered: GitHub also
records closing links attached through the PR development sidebar, which
live in no body. This family declares links in the body, so the delta is
zero here.
Refs #188
The preflight half of #188, landed first so it stands alone: the forge is
decided once, before any sweep, and a client that cannot speak it exits
non-zero with a named reason.
Measured against forgejo.heavyduty.builders at 84bb1a4 — two of the three
actions reported SUCCESS having read nothing:
labels-scope exit 0 "no .github/labeler.yml" (the file is HTTP 200)
labels-reconcile exit 0 "reconciled." (zero PRs enumerated)
issueflow-reconcile exit 1 "unexpected end of JSON input"
labels-reconcile's blind-sweep warning (#96) could not fire: it counts
unreadable PRs against a list `gh pr list` never produced, and a process
substitution's failure does not trip set -e, so total stayed 0. Installing
gh makes it worse, silencing the one loud failure.
Detection is measured, not inferred from docs: a real forgejo-runner v6.3.1
job (probe task 278) shows Forgejo populating the whole GITHUB_* namespace,
so GITHUB_ACTIONS proves nothing. GITHUB_API_URL's shape, GITEA_ACTIONS and
GITHUB_SERVER_URL do. The same probe shows the runner image carries neither
gh nor stoke, which is what makes the forgejo backend REST.
Tests declare CEREMONY_FORGE at the forge boundary rather than stubbing gh
and staying silent about the forge — the boundary move term 5 asks for.
Refs #188
panel[z]]=b parsed at the round-1 head: the case pattern only proves
some ]= occurs, so the stray ] stayed inside the login and the real
author silently fell back to the base panel — the misroute D4 exists to
refuse. The login charset is now enforced with the bracket-specific
diagnostic; codex's probe and an invalid-character row are the new
must-fail fixtures.
Refs #224
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
round_outranks_draft consults the round before draft short-circuits: a
re-drafted PR carrying CHANGES_REQUESTED, an owed round-reply, or
push-staled approvals reads state:addressing; a live panel request on a
draft surfaces as state:bots-reviewing rather than being absorbed
(the must-not-paper-over combination, decided as: visible). Approvals do
not outrank draft, so a draft never reads needs-human, and a virgin
draft is byte-identical to before. LABELS.md's state:building row makes
draft evidence, not the definition.
Refs #205
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
