Commit graph

6 commits

Author SHA1 Message Date
2168e4ef9a test(forge): hermetic cases for the two forgejo edit asymmetries
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
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
baf4a20571 feat(forge): port every reconciler call site onto the shim
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
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
a968e13ca4 fix(forge): parity gaps in the forgejo verbs — upsert, timestamps, typos
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
@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
714a2e0413 feat(forge): the reconciler verb surface on both backends
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
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
66e20f12f0 fix(forge): validate the completeness bound itself, on every page
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
@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
ab23a3b1b6 feat(forge): two backends behind one call surface, and the shim owns paging
Some checks failed
CI / test (pull_request) Has been cancelled
CI / release-exercise (pull_request) Has been cancelled
CI / self-guards (pull_request) Has been cancelled
CI / action-exercise (pull_request) Has been cancelled
CI / docs-sync-exercise (pull_request) Has been cancelled
labels / labels (pull_request) Has been cancelled
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