14 commits

Author	SHA1	Message	Date
cluade-reviewer-andresmgsl	a55fbaef15	fix(forge): forge_commit_at — Forgejo serves a single commit at /git/commits/{sha} (#209) ... Found by the first post-merge sweep after the 0.6.0 merge — #198's own acceptance probe — not by review. Three PRs in one run: labels: #208: could not read the head commit's date: forge_api: HTTP 404 from 'GET repos/heavy-duty/ceremony/commits/f3a1336…' — blocker:unrequested not judged this pass Measured against this instance: forgejo repos/{o}/{r}/commits/{sha} -> 404 forgejo repos/{o}/{r}/git/commits/{sha} -> 200, date under `.created` github repos/{o}/{r}/commits/{sha} -> 200, date nested A fourth asymmetry, alongside the three lib/forge-forgejo.sh's header already records. #198 ported this call site onto the shim with GitHub's path unchanged — correct against GitHub, and the block it lives in (#236 D2) arrived WITH the merge, so nothing here had ever executed it. So it becomes a verb rather than a path at the call site: the caller wants one timestamp and should not have to know either shape. Cost while it stood was bounded and loud rather than silent — guarded_read refused and the sweep said so — but blocker:unrequested could never be judged on this forge. The tests pin each backend's PATH and FIELD, because a stubbed forge_api cannot catch a wrong path; that is exactly how this shipped and why it took a live sweep to find. Swapping the paths reds the forgejo pair; swapping the fields reds the github one. test/run.sh 28/28 under jq 1.7 and jq 1.6; forge-backends 124/124; shellcheck 0.10.0 and actionlint clean. Refs #209	2026-08-05 15:01:32 +00:00
cluade-reviewer-andresmgsl	a35a77f752	fix(forgejo): a read failure names its verb too, and the tests assert the whole diagnostic (#192) ... @codex-reviewer-andresmgsl caught a test that describes evidence it does not collect — mine, and it is the class this PR is about. Two cases were titled "naming the verb, path and status" and asserted only the substring "500". The PUT boundary happened to satisfy the contract because forgejo_write already passes "PUT $endpoint" to forgejo_http_ok. The GET boundary did not: the diagnostic was `HTTP 500 from 'repos/o/r/issues/5'`, with no verb at all — so a caller could not tell a failed READ from a failed WRITE of the same path, and #192's acceptance criterion asks for exactly that distinction. Reads now pass "GET $endpoint" on both non-paginated and paginated paths, and the two tests assert the complete expected diagnostic as one substring rather than a status code that any failure would contain. Reverting the verb reds the GET case. Also, per the same review: the failed GET is asserted to write nothing, and the failed PUT to have attempted exactly one write. forge-backends 117/117 (was 115), test/run.sh 22/22 under jq 1.7 and jq 1.6, shellcheck 0.10.0 and actionlint clean. Refs #192	2026-08-05 13:11:33 +00:00
cluade-reviewer-andresmgsl	062e016a42	fix(labels): all four review gaps — preserved ids, zero-write no-op, every mutation counted, no success token (#192) ... @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	2026-08-05 13:03:25 +00:00
cluade-reviewer-andresmgsl	0f20f4b6ef	fix(labels): a label removal that cannot happen fails the sweep, and removal itself now works (#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	2026-08-05 12:48:09 +00:00
cluade-reviewer-andresmgsl	ca99182e80	fix(forge): percent-encode asset names, and stop the docs naming a client ... Both findings are @codex's on !193 (#1583), and both are real. The asset name travels as a QUERY VALUE, and the artifact-hook contract permits any file the consumer drops in RELEASE_ASSETS_DIR. Raw interpolation meant `release asset.tgz` made curl reject the URL outright (exit 3), and '&', '#', '+', '%' silently changed the name or the query's shape. `gh release create` handled all of those, so a 1:1 port had to. Encoded through one boundary — jq's @uri, since jq is already a hard dependency of this backend and a hand-rolled sed class is how the next unescaped character gets through. Six backend cases cover it: the encoder on a space and on the delimiters, uploads under both names, the created release id in the path, and the multipart attachment. Mutation-checked: dropping the encoder fails exactly the two name assertions. docs/CONSUMERS.md's artifact-hook recovery still told operators to "run `gh release create` by hand" and described the hook as running "before `gh release create`" — on a Forgejo runner that is precisely the failure this PR fixes. It now names the forge-neutral tag-door recovery first and shows both clients for the manual path, without regressing the GitHub guidance. 1035 assertions, 22 suites, shellcheck-all and actionlint clean. Refs #191 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 12:11:06 +00:00
cluade-reviewer-andresmgsl	21c70e06a4	fix(forge): an empty REPO cannot become a fact, and the backend verbs are tested ... Both panel blockers on c63a550. @kimi found the one that mattered: facts.sh got the REPO fix, release.yml's own four call sites did not. A workflow `run:` shell carries no `set -u`, so an unset REPO expands empty and the verb addresses `repos//…` — which 404s, and the 404 is then read as an ANSWER. Reproduced read-only against this instance before fixing: forge_release_exists 0.4.1 -> "no", rc 0 forge_commit_pulls 7fc9afe4 -> "[]", rc 0 (the !189 merge, which HAS a merged PR behind it) The first would have let the nothing-exists assert proceed to CREATE; the second is the drill's original fabricated `labeled=no`, one step after the fix meant to kill it. Fixed once rather than at four call sites, as kimi suggested: forge_select defaults REPO from GITHUB_REPOSITORY, and forgejo_api_base — which every verb reaches the network through — refuses an empty REPO outright. No fifth call site can forget it. @grok and @kimi both blocked on the same AC gap: the backend suite did not cover the five new verbs, so the two measured asymmetries had no offline coverage. test/forge-backends.test.sh now has 15 cases for them — singular /pull wrapped to an array, 404 as an empty array, 500 refusing, release present/absent/unreadable, POST /tags vs /git/refs, the publish body, and the REPO-empty must-fail. Mutation-checked: reading the plural path fails one case, dropping the REPO guard fails the two must-fails. 1029 assertions, 22 suites, shellcheck-all and actionlint clean. Refs #191 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 11:53:28 +00:00
cluade-reviewer-andresmgsl	957f72739d	fix(forge): the release doors speak the shim, and an unread fact refuses (#191) ... The 0.4.1 drill measured both doors dead on Forgejo. lib/facts.sh gathered `released` with `gh release view` and `labeled` with `gh api .../pulls`, and release.yml tagged and published with `gh` — none of which exist on the runner image. The merge door therefore read labeled=no for a correctly labeled, correctly merged ceremony PR and refused it as "a bare push"; the tag door cleared every gate and died at `gh release create`. Both are ported onto lib/forge.sh. Two asymmetries were measured against the live instance and its swagger rather than assumed: * GitHub serves an ARRAY of PRs at /commits/{sha}/pulls; Forgejo serves a single OBJECT at /commits/{sha}/pull and 404s on the plural. Both verbs emit the array shape, so facts.sh carries one jq expression. * GitHub creates a tag by POSTing to /git/refs; Forgejo serves that path GET-only and creates tags at /tags. A 1:1 port of the gh call would have 404'd forever. The behaviour change is the second half of the bug. Any failure used to become a definite `no`, which is safe for row 4 and catastrophic for row 5: it is how a missing binary became "this was not a release ceremony". Now a completed read that finds nothing is still `no` and still fail-closed, and a read that did not complete refuses and emits no fact at all. Four new cases in test/facts.test.sh cover exactly that, and a mutation back to the old fail-closed-on-error behaviour kills all four and nothing else. 1014 assertions, 22 suites, shellcheck and actionlint clean. Refs #191 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 11:31:06 +00:00
grok-reviewer-andresmgsl	5c8e4f5b84	feat(forge): timeline normalizer, portable PR activity, shellcheck install (#188) ... 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.	2026-08-03 15:13:30 +00:00
cluade-reviewer-andresmgsl	2168e4ef9a	test(forge): hermetic cases for the two forgejo edit asymmetries ... The coverage owed with the call-site port (@grok-reviewer-andresmgsl #4741 note 2, #4751 item 2). Live scratch-repo evidence proved these work; these pin the request SHAPE so they keep working. - a removal resolves name -> numeric id, and never sends the name as the path segment (measured: DELETE .../labels/probe:one -> 422, DELETE .../labels/149 -> 204); - a removal of a label the repo does not have writes nothing, matching gh: the reconcilers call --remove-label unconditionally to converge state; - adds take names directly, one request, comma-separated values split as gh splits them; - an assignee removal PATCHes the SURVIVING list, because Forgejo sets assignees rather than adding and removing them — a naive translation would have cleared every other assignee as a side effect of removing one, which is what the mutation test proves is caught. Payloads are now compact JSON. They were pretty-printed, which spread a single write across several lines — harder to read in a log, and it hid the shape from any assertion matching a line. Refs #188	2026-08-02 19:50:37 +00:00
cluade-reviewer-andresmgsl	baf4a20571	feat(forge): port every reconciler call site onto the shim ... 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	2026-08-02 19:43:58 +00:00
cluade-reviewer-andresmgsl	a968e13ca4	fix(forge): parity gaps in the forgejo verbs — upsert, timestamps, typos ... @codex-reviewer-andresmgsl's three findings (#4743), all real. 1. forge_label_create is now an UPSERT, matching gh label create --force. bootstrap_labels creates every declared label on EVERY workflow_dispatch, so a plain POST onto an existing name aborted the bootstrap under set -e from the second dispatch onward. Resolves name -> id and PATCHes when it exists. 2. forge_pr_view carries createdAt/completedAt. checks_state groups repeated contexts and selects the newest by [.startedAt, .createdAt, .completedAt]; mapping only {context,state} left the winner to incidental array order, so a stale re-run could outrank the live verdict. The combined status carries created_at and updated_at — measured. 3. forge_issue_edit refuses unknown flags and missing values. The github backend hands them to gh, which fails; dropping them here turned a mis-typed port site into a mutation that silently did not happen — this issue's own failure class, inside the fix for it. Also settles @grok-reviewer-andresmgsl's note 3 (#4741): Forgejo Actions DO land as commit statuses on this instance, so the rollup is not empty. rig main carries four — "ci / check (push)" and siblings, state success, each with created_at. statusCheckRollup therefore populates, and NONE is not silently substituted for SUCCESS. Each fix mutation-verified: dropping the timestamps, forcing POST-always, and restoring the silent flag skip each red exactly their own cases. The newest-verdict case drives the real checks_state, not a copy. Refs #188	2026-08-02 19:22:28 +00:00
cluade-reviewer-andresmgsl	714a2e0413	feat(forge): the reconciler verb surface on both backends ... github is the existing gh invocation extracted 1:1 (term 5). forgejo is /api/v1, and encodes three asymmetries measured against this instance on a scratch repo — never a live board: 1. Adding labels takes NAMES; removing one takes a numeric ID. POST /issues/1/labels {"labels":["probe:one"]} -> 200 DELETE /issues/1/labels/probe:one -> 422 DELETE /issues/1/labels/149 -> 204 So a removal resolves name -> id first. gh hides this; the shim cannot. 2. Assignees are SET, not added and removed: PATCH /issues/{n} takes the whole list and {"assignees":[]} clears it. --remove-assignee is therefore a read-modify-write, not a delete. 3. There is no statusCheckRollup. The portable equivalent is the combined commit status, GET /commits/{sha}/status, mapped into the node shape checks_state already parses so the decision code is untouched. gh pr list --limit 100 moves behind forge_pr_list: that page size lives in gh's own flag namespace, so no URL-parameter strip could have caught it (@grok-reviewer-andresmgsl's note 3). Every verb driven live against a real Forgejo instance: label list/create/ delete, add and remove labels by name, a removal of a label the repo does not have (no-op, as gh behaves), comment, assignee add and remove, pr_list. Call sites are still unported, so this is not yet reachable on either forge. Refs #188	2026-08-02 19:13:14 +00:00
cluade-reviewer-andresmgsl	66e20f12f0	fix(forge): validate the completeness bound itself, on every page ... @codex-reviewer-andresmgsl's three findings (#4712), each a route by which an unprovable read could still be reported as a whole one — the guard leaking the failure class it exists to stop. 1. x-total-count was never validated. `X-Total-Count: not-a-number` returned rc=0 with that string as the bound the walk compared against, reproduced on ab23a3b. Now required to be a canonical non-negative integer. 2. The total was read once. A collection changing size under the walk was invisible: page 1 declaring 4 and page 2 declaring 9 stopped at 4 believing itself whole. Now re-read per page; a moving total means the read was not atomic and is refused. 3. A 200 whose body is not an array counted as zero items, so an error object or scalar arriving where a list belongs read as a complete EMPTY collection whenever the declared total was 0. Now refused, quoting the body. A genuinely empty array is still fine — covered. Each guard is mutation-verified: removing it reds exactly its own cases and no others. Refs #188	2026-08-02 19:07:32 +00:00
cluade-reviewer-andresmgsl	ab23a3b1b6	feat(forge): two backends behind one call surface, and the shim owns paging ... Term 1's foundation. lib/forge.sh gains forge_select, which sources exactly one of lib/forge-github.sh or lib/forge-forgejo.sh; both define the same verbs, so no branching reaches the 61 call sites. The github backend is the current gh invocation extracted 1:1 — term 5 is kept by making that path boring. The page size moves OUT of the call sites and into the backend, because it is not portable and fails silently. Measured 2026-08-02: ?per_page=100 GitHub 100 items Forgejo 30 items (ignored) ?limit=100 GitHub 30 items Forgejo 50 items (capped) Both answer HTTP 200 with valid JSON. Every call site here is GitHub-shaped, so a verbatim port would have swept 30 of rig's 137 issues and printed "reconciled." — criterion 2 failing green, the same failure class as the blind sweep. Both page_url helpers strip a stray page-size parameter in either dialect, so a call site cannot reintroduce it by accident. Forgejo caps a page at 50 whatever is asked, so pagination is mandatory, not an optimisation. The gather is then PROVEN complete against x-total-count rather than assumed complete because a loop ended. @kimi-reviewer-andresmgsl's hardening (#4699): a missing x-total-count is itself a loud refusal. Header exposure is a server setting, and an assert that cannot run must not silently pass — that is the failure class re-entering through the guard built to stop it. Call sites are not ported yet; that is the next commit. Refs #188	2026-08-02 19:00:58 +00:00