Commit graph

27 commits

Author SHA1 Message Date
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
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
ff17d1ea3f fix(forge): term-5 GitHub pins for timeline/activity + keep activity stderr (#188)
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).
2026-08-03 15:26:08 +00:00
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
f2d5fcd565 feat(forge): derive outstanding review requests from the head, not the field
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
2026-08-02 19:48:09 +00:00
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
7d52b2cd4a feat(forge): refuse loudly when the client cannot speak the forge
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
2026-08-02 18:29:08 +00:00
Daniel Marin
4debf53872
Merge pull request #181 from claude-bot-andresmgsl/build/180-blocked-excludes-nh
fix: `blocked` excludes `state:needs-human` in decide_state()
2026-07-25 11:31:23 +01:00
claude-bot-andresmgsl
1c9a82aaf0 fix: blocked excludes state:needs-human in decide_state (#180)
During the ceremony#111 freeze, rig#126/#128 carried blocked beside
state:needs-human — the round had finished, but the hold said the merge
must not happen, and rig#126 was merged seven minutes after the
reconciler wrote the green label. decide_state() only joined the two
axes through blockers(), which emits branch facts; the hand-set blocked
label was never consulted.

blocked becomes the second exclusion on state:needs-human, exactly
parallel to needs-ruling: round says needs-human + has_label blocked ->
state:addressing. Deliberately not a blockers() emission — BLOCKERS is
machine-owned and the converge loop would strip the live hold on the
next tick, the same trap #51 names for needs-ruling.

Ruling record: discussion 122, armed default A fired 2026-07-25T09:00Z.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-25 09:08:54 +00:00
codex-bot-andresmgsl
8e6423a07c docs: define post-merge queue state 2026-07-25 00:09:14 +00:00
codex-bot-andresmgsl
eae000bd62 docs: add actions read to private caller guidance 2026-07-24 22:57:22 +00:00
Daniel Marin
089f2dba29
Merge pull request #140 from claude-bot-andresmgsl/build/139-cancelled-not-verdict
fix: a queue-cancelled duplicate check is not a verdict — checks_state discards it when a real one stands
2026-07-24 14:14:36 +01:00
claude-bot-andresmgsl
d8f54aab04 fix: a queue-cancelled duplicate check is not a verdict
checks_state discards a CANCELLED entry only when its context group holds
at least one non-cancelled sibling — before the sort, so the duplicate the
repo-global reconcile queue evicted (after it had already attached a check
to the head) cannot outvote the success that did its work (#136 a17e497,
#133 4002924). An all-cancelled context never reported at all and still
classifies FAILURE; {FAILURE older, CANCELLED newest} keeps its red.

The fixture that pinned the opposite rule imagined a cancelled run
replacing a success; it never saw one that replaced nothing. Rewritten
with its reason, plus the recorded a17e497 shape, the all-cancelled
groups, and the cancelled-over-FAILURE case.

Closes #139

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 12:34:51 +00:00
claude-bot-andresmgsl
ce24a1a3ba polish: unreadable base version reads 'unreadable' in the release-shape warning
Part of #130.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 12:07:57 +00:00
claude-bot-andresmgsl
16dfdb9a4f feat: reconciler warns on a release-shaped PR missing its release label
Bare X.Y.Z at the head where the base says otherwise, no release label,
not a draft: the sweep emits one :⚠️: per pass naming both
versions. A warning only — release is declared intent and the reconciler
never guesses intent (LABELS.md). Version read via the API, both
backends, jq not node; unreadable reads nag nobody. Plus the yq test
contract in CI and fixture tests for the guard matrix.

Part of #130.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 11:54:49 +00:00
codex-bot-andresmgsl
edbf30867e feat: warn when core taxonomy labels are missing 2026-07-24 09:22:41 +00:00
claude-bot-andresmgsl
fbfe7dd1c1 feat: report why the degraded read degraded
The reconciler's mergeability/checks read kept its correct degrade but
threw the reason away: 2>/dev/null dropped gh's stderr, leaving a
permanent denial and a network hiccup byte-identical in the log (#95 had
to infer a cause from a control case, and the inference did not survive
incubator#48/#49). Capture stderr into a variable via a temp file (D2),
emit it as its own '#N: read failed: …' line beside the byte-identical
counted line (D1), collapsed and bounded by a pure helper (D3/D4), and
lead blind_sweep_warning with the sampled observed reason, demoting the
permissions hint from stated cause to named candidate (D5).

Part of #101 groundwork; tests and changelog follow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 07:34:33 +00:00
codex-bot-andresmgsl
1d3e98497d Merge remote-tracking branch 'origin/main' into build/95-labels-blind-sweep-warning
# Conflicts:
#	CHANGELOG.md
#	test/labels-reconcile.test.sh
2026-07-23 23:15:08 +00:00
codex-bot-andresmgsl
2c57216a56 fix: surface wholly blind label sweeps 2026-07-23 23:12:30 +00:00
claude-bot-andresmgsl
27aa03464c feat: retire the six GitHub default labels at bootstrap
LABELS.md publishes them as deleted at bootstrap; nothing deleted them —
incubator's first dispatch left `good first issue` standing. One registry
(retired_label_names) beside core_label_rows, dispatch-only, through run,
tolerant of absence and refusal (#93).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-23 21:09:54 +00:00
codex-bot-andresmgsl
f15cb8c0c9 feat: add attention core label row 2026-07-23 17:27:24 +00:00
codex-bot-andresmgsl
6a9b501e05 feat: exempt offsite claims from reclaim clock 2026-07-23 12:48:04 +00:00
claude-bot-andresmgsl
a40e42544e feat(labels): wire the ruling pass into both reconcilers + lib test suite
Issue side: the claim-reclaim clock stops under a pending ruling (the
decision still sees an unassigned claim), an already-applied stale heals
off, and reconcile_ruling runs for any flagged issue on any queue state.
PR side: reconcile_ruling rides the (#51) stale section's real-activity
computation. test/ruling.test.sh pins the window boundaries, newest-event
anchoring, per-event marker scoping, the markerless nudge reset, the
unreadable-timeline rule, and that no scenario writes a label.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-23 12:25:05 +00:00
codex-bot-andresmgsl
8174080c9d feat: reconcile the issue work queue 2026-07-23 10:31:15 +00:00
claude-bot-andresmgsl
bd215b06f2 feat(labels): needs-ruling — bootstrap row, decide_state exclusion, staleness skip (#51)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-23 00:44:16 +00:00
codex-bot-andresmgsl
1ff660f539 docs: preserve labels workflow safety rationale 2026-07-22 18:21:11 +00:00
codex-bot-andresmgsl
60e417a13c feat: centralize labels machinery 2026-07-22 18:18:18 +00:00