fix(labels): grade Forgejo review states #244

Merged
andres merged 4 commits from codex-bot-andresmgsl/ceremony:build/235-forgejo-review-vocabulary into main 2026-08-24 00:16:46 +00:00

Closes #235

Summary

  • keep Forgejo review-request rows out of submitted review facts
  • grade GitHub and Forgejo verdict spellings explicitly
  • treat unknown review states as missing and log the unrecognized state

Acceptance criteria

  • Three panelists whose only rows are REQUEST_REVIEW at the current head and whose requests are live keep round_state at state:bots-reviewing; RED was recorded first.
  • outstanding_requests returns a login whose only row is REQUEST_REVIEW.
  • bot_verdict returns BLOCK for both REQUEST_CHANGES and CHANGES_REQUESTED; existing GitHub fixtures remain unedited.
  • An unnamed review state returns MISSING, not FEEDBACK, and logs the login plus state.
  • bot_verdict returns FEEDBACK for both COMMENT and COMMENTED, and a COMMENT row survives the REVIEWS_JSON allow-list.
  • REQUEST_REVIEW does not survive the REVIEWS_JSON ingestion filter, independently of grading.
  • All bots approving while the human carries REQUEST_CHANGES yields state:addressing; human approval yields state:needs-human.
  • No existing assertion is deleted or weakened; the assertion count rises.
  • Full suite and sanctioned shellcheck are green at the PR head.

Worklog

  • Establish clean baseline: test/labels-reconcile.test.sh 248 passed, 0 failed.
  • Add test-only Forgejo vocabulary fixtures and push RED checkpoint e5ebbf5.
  • Record RED: 251 passed, 7 expected failures on the unfixed tree.
  • Make review ingestion an allow-list of submitted, gradeable states.
  • Grade both forge vocabularies and log unknown states conservatively.
  • Add changelog.d/235.md.
  • Prove the ingestion filter is independently load-bearing with mutation evidence.
  • Run focused tests (258/258), full 31-file suite, and sanctioned chunked shellcheck (64 tracked scripts).
  • Prepare the complete build-round reply and exact-head draft handoff; the signal and ready flip follow this Worklog.
  • Add and mutation-prove the amended COMMENT ingestion guard: dropping only COMMENT from the allow-list yielded 257 passed / 1 failure; restoring it yielded 258/258. Re-run: 31 test files, 64 chunked shellchecks, 9 actionlint files, repository guards, git diff --check, and clean status all green on 1cd4602.
Closes #235 ## Summary - keep Forgejo review-request rows out of submitted review facts - grade GitHub and Forgejo verdict spellings explicitly - treat unknown review states as missing and log the unrecognized state ## Acceptance criteria - [x] Three panelists whose only rows are `REQUEST_REVIEW` at the current head and whose requests are live keep `round_state` at `state:bots-reviewing`; RED was recorded first. - [x] `outstanding_requests` returns a login whose only row is `REQUEST_REVIEW`. - [x] `bot_verdict` returns `BLOCK` for both `REQUEST_CHANGES` and `CHANGES_REQUESTED`; existing GitHub fixtures remain unedited. - [x] An unnamed review state returns `MISSING`, not `FEEDBACK`, and logs the login plus state. - [x] `bot_verdict` returns `FEEDBACK` for both `COMMENT` and `COMMENTED`, and a `COMMENT` row survives the `REVIEWS_JSON` allow-list. - [x] `REQUEST_REVIEW` does not survive the `REVIEWS_JSON` ingestion filter, independently of grading. - [x] All bots approving while the human carries `REQUEST_CHANGES` yields `state:addressing`; human approval yields `state:needs-human`. - [x] No existing assertion is deleted or weakened; the assertion count rises. - [x] Full suite and sanctioned shellcheck are green at the PR head. ## Worklog - [x] Establish clean baseline: `test/labels-reconcile.test.sh` 248 passed, 0 failed. - [x] Add test-only Forgejo vocabulary fixtures and push RED checkpoint `e5ebbf5`. - [x] Record RED: 251 passed, 7 expected failures on the unfixed tree. - [x] Make review ingestion an allow-list of submitted, gradeable states. - [x] Grade both forge vocabularies and log unknown states conservatively. - [x] Add `changelog.d/235.md`. - [x] Prove the ingestion filter is independently load-bearing with mutation evidence. - [x] Run focused tests (258/258), full 31-file suite, and sanctioned chunked shellcheck (64 tracked scripts). - [x] Prepare the complete build-round reply and exact-head draft handoff; the signal and ready flip follow this Worklog. - [x] Add and mutation-prove the amended `COMMENT` ingestion guard: dropping only `COMMENT` from the allow-list yielded 257 passed / 1 failure; restoring it yielded 258/258. Re-run: 31 test files, 64 chunked shellchecks, 9 actionlint files, repository guards, `git diff --check`, and clean status all green on `1cd4602`.
codex-bot-andresmgsl added 1 commit 2026-08-23 23:11:59 +00:00
test(labels): reproduce Forgejo review vocabulary gaps
Some checks failed
labels / labels (pull_request) Failing after 21s
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
Refs guard / refs-not-closing (pull_request) Has been cancelled
e5ebbf57fb
codex-bot-andresmgsl added 1 commit 2026-08-23 23:13:16 +00:00
fix(labels): grade Forgejo review states
Some checks failed
labels / labels (pull_request) Failing after 21s
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
Refs guard / refs-not-closing (pull_request) Has been cancelled
58e58f2ada
codex-bot-andresmgsl added 1 commit 2026-08-23 23:17:25 +00:00
test(labels): mark indirect filter probe call
Some checks failed
labels / labels (pull_request) Failing after 21s
Refs guard / refs-not-closing (pull_request) Has been cancelled
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
c2cca7c1be
codex-bot-andresmgsl changed title from fix(labels): grade Forgejo review states to WIP: fix(labels): grade Forgejo review states 2026-08-23 23:20:18 +00:00
Author
Member

Build round complete on head c2cca7c1be.

  • Review ingestion now allow-lists only APPROVED, both blocking spellings, and both comment spellings. PENDING, REQUEST_REVIEW, and unknown rows do not enter REVIEWS_JSON; the main-driven boundary probe proves REQUEST_REVIEW is removed while an APPROVED control survives.
  • bot_verdict now maps CHANGES_REQUESTED and REQUEST_CHANGES to BLOCK, COMMENTED and COMMENT to FEEDBACK, preserves head-sensitive APPROVED, and maps every unknown spelling to MISSING while logging the login and state.
  • The Forgejo opening-round fixture keeps all three request-only reviewers outstanding and returns state:bots-reviewing. The human REQUEST_CHANGES fixture returns state:addressing, while its human-approval control returns state:needs-human.
  • Existing assertions were preserved; the focused suite rose from 248 to 258 assertions. Red-first evidence on the unfixed tree was 251 passed / 7 expected failures. With only the old ingestion predicate restored after the grader fix, mutation evidence was 257 passed / 1 failure, solely the independent filter assertion. Restored GREEN is 258/258.
  • Added changelog.d/235.md with the grouped Fixed entry.

Verification on this exact pushed tree: full suite 31 test files passed / 0 failed; sanctioned memory-capped shellcheck covered 64 tracked scripts with no findings; actionlint covered 9 workflows; self-ref, marker, vendored, changelog-armed, git diff --check, and the worktree status are clean. The Forgejo create call initially ignored its draft boolean, so I corrected the PR through the sanctioned draft conversion before this handoff; it is now draft and no review request was made.

Build round complete on head c2cca7c1be491d12abbb1281b9f4dfe8d2476fe9. - Review ingestion now allow-lists only `APPROVED`, both blocking spellings, and both comment spellings. `PENDING`, `REQUEST_REVIEW`, and unknown rows do not enter `REVIEWS_JSON`; the main-driven boundary probe proves `REQUEST_REVIEW` is removed while an `APPROVED` control survives. - `bot_verdict` now maps `CHANGES_REQUESTED` and `REQUEST_CHANGES` to `BLOCK`, `COMMENTED` and `COMMENT` to `FEEDBACK`, preserves head-sensitive `APPROVED`, and maps every unknown spelling to `MISSING` while logging the login and state. - The Forgejo opening-round fixture keeps all three request-only reviewers outstanding and returns `state:bots-reviewing`. The human `REQUEST_CHANGES` fixture returns `state:addressing`, while its human-approval control returns `state:needs-human`. - Existing assertions were preserved; the focused suite rose from 248 to 258 assertions. Red-first evidence on the unfixed tree was 251 passed / 7 expected failures. With only the old ingestion predicate restored after the grader fix, mutation evidence was 257 passed / 1 failure, solely the independent filter assertion. Restored GREEN is 258/258. - Added `changelog.d/235.md` with the grouped Fixed entry. Verification on this exact pushed tree: full suite 31 test files passed / 0 failed; sanctioned memory-capped shellcheck covered 64 tracked scripts with no findings; actionlint covered 9 workflows; self-ref, marker, vendored, changelog-armed, `git diff --check`, and the worktree status are clean. The Forgejo create call initially ignored its draft boolean, so I corrected the PR through the sanctioned draft conversion before this handoff; it is now draft and no review request was made.
Author
Member

📣 round answered at head c2cca7c1be

📣 round answered at head c2cca7c1be491d12abbb1281b9f4dfe8d2476fe9
codex-bot-andresmgsl changed title from WIP: fix(labels): grade Forgejo review states to fix(labels): grade Forgejo review states 2026-08-23 23:21:06 +00:00
forgejo-actions added the
state:addressing
label 2026-08-23 23:27:12 +00:00
Author
Member

🔧 addressing round on head c2cca7c1be

  • Agree — blocking contract amendment: the tree already proves COMMENT grades as FEEDBACK, but the ingestion boundary probe does not include a COMMENT row. I will add a regression assertion proving both COMMENT and the existing APPROVED control survive while REQUEST_REVIEW is excluded.
  • Agree — non-blocking checklist note: I will sync the newly explicit acceptance criterion into the PR body and carry this fix as a Worklog checkbox.
  • Agree — non-blocking closure note: Closes #235 remains correct; no body change is needed there. No reviewer verdicts exist on this head, so this response covers the full demand currently outstanding.
🔧 addressing round on head c2cca7c1be491d12abbb1281b9f4dfe8d2476fe9 - **Agree — blocking contract amendment:** the tree already proves `COMMENT` grades as `FEEDBACK`, but the ingestion boundary probe does not include a `COMMENT` row. I will add a regression assertion proving both `COMMENT` and the existing `APPROVED` control survive while `REQUEST_REVIEW` is excluded. - **Agree — non-blocking checklist note:** I will sync the newly explicit acceptance criterion into the PR body and carry this fix as a Worklog checkbox. - **Agree — non-blocking closure note:** `Closes #235` remains correct; no body change is needed there. No reviewer verdicts exist on this head, so this response covers the full demand currently outstanding.
codex-bot-andresmgsl changed title from fix(labels): grade Forgejo review states to WIP: fix(labels): grade Forgejo review states 2026-08-23 23:28:01 +00:00
codex-bot-andresmgsl added 1 commit 2026-08-23 23:29:31 +00:00
test(labels): guard Forgejo comment ingestion
Some checks failed
labels / labels (pull_request) Failing after 20s
CI / test (pull_request) Successful in 3m41s
CI / release-exercise (pull_request) Successful in 25s
CI / self-guards (pull_request) Successful in 20s
CI / action-exercise (pull_request) Successful in 18s
CI / docs-sync-exercise (pull_request) Successful in 19s
Refs guard / refs-not-closing (pull_request) Has been cancelled
1cd46028ed
Author
Member

Amendment round complete on head 1cd46028ed.

  • The blocking contract point is addressed: the main-driven ingestion probe now supplies a Forgejo COMMENT row and asserts that COMMENT,APPROVED survive while REQUEST_REVIEW is removed. The existing direct grader assertion continues to prove COMMENT maps to FEEDBACK.
  • Mutation evidence proves the new filter half is load-bearing: removing only the production COMMENT allow-list arm produced 257 passed / 1 failure, exactly at the amended assertion; restoring it produced 258/258.
  • The non-blocking PR-body note is addressed: the copied acceptance checklist and Worklog now carry the amended criterion and evidence. The closure note is accepted; Closes #235 remains correct.
  • Fresh verification on this pushed head: 31 test files passed / 0 failed; 64 tracked scripts passed the per-file 4 GiB-capped shellcheck; actionlint covered 9 workflows; self-ref, marker, vendored, and changelog-armed guards passed; git diff --check and worktree status are clean. No reviewer verdicts exist, so this reply covers the full outstanding demand.
Amendment round complete on head 1cd46028ede9891ec34dc7af2f5b208aec1515ac. - The blocking contract point is addressed: the main-driven ingestion probe now supplies a Forgejo `COMMENT` row and asserts that `COMMENT,APPROVED` survive while `REQUEST_REVIEW` is removed. The existing direct grader assertion continues to prove `COMMENT` maps to `FEEDBACK`. - Mutation evidence proves the new filter half is load-bearing: removing only the production `COMMENT` allow-list arm produced 257 passed / 1 failure, exactly at the amended assertion; restoring it produced 258/258. - The non-blocking PR-body note is addressed: the copied acceptance checklist and Worklog now carry the amended criterion and evidence. The closure note is accepted; `Closes #235` remains correct. - Fresh verification on this pushed head: 31 test files passed / 0 failed; 64 tracked scripts passed the per-file 4 GiB-capped shellcheck; actionlint covered 9 workflows; self-ref, marker, vendored, and changelog-armed guards passed; `git diff --check` and worktree status are clean. No reviewer verdicts exist, so this reply covers the full outstanding demand.
Author
Member

📣 round answered at head 1cd46028ed

📣 round answered at head 1cd46028ede9891ec34dc7af2f5b208aec1515ac
codex-bot-andresmgsl changed title from WIP: fix(labels): grade Forgejo review states to fix(labels): grade Forgejo review states 2026-08-23 23:33:12 +00:00
forgejo-actions added the
blocker:ci-red
label 2026-08-23 23:49:29 +00:00
Author
Member

INFRASTRUCTURE — labels / labels (pull_request) at head 1cd46028ede9891ec34dc7af2f5b208aec1515ac failed in run 1517, job 0 because the Forgejo workflow token could not write to the upstream repository.

I read the job log rather than grading from the check name. The pull_request_target run explicitly fetched and checked out base main at 17a13685a72c60f1dc1dab8b49cf34dfcac6e959, not this PR's head. It then received HTTP 403 twice: first dispatching self-labels-sweep.yml, then posting scope labels to PR !244. The branch's changed code was never checked out or executed. This is therefore outside the PR tree (event-token/repository permission context), not a branch code, test, or config failure.

I attempted the permitted same-head rerun exactly once. POST /heavy-duty/ceremony/actions/runs/1517/jobs/0/rerun with the available API credential was redirected without starting a run, and this Forgejo 8 instance returns 404 for both /api/v1/repos/heavy-duty/ceremony/actions/runs/1517/rerun and the job-scoped API variant. Forgejo 8 exposes rerun only through an authenticated web session; the available credential is API-only. No rerun started, and I made no code change.

Because the log proves the job ran the base tree and failed only on repository write permission, I am using the documented green-head exception: the current-head round was already answered and signalled at this exact SHA, and I will request the configured panel by hand with this evidence rather than leave the red head silent.

INFRASTRUCTURE — `labels / labels (pull_request)` at head `1cd46028ede9891ec34dc7af2f5b208aec1515ac` failed in run 1517, job 0 because the Forgejo workflow token could not write to the upstream repository. I read the job log rather than grading from the check name. The `pull_request_target` run explicitly fetched and checked out base `main` at `17a13685a72c60f1dc1dab8b49cf34dfcac6e959`, not this PR's head. It then received HTTP 403 twice: first dispatching `self-labels-sweep.yml`, then posting scope labels to PR !244. The branch's changed code was never checked out or executed. This is therefore outside the PR tree (event-token/repository permission context), not a branch code, test, or config failure. I attempted the permitted same-head rerun exactly once. `POST /heavy-duty/ceremony/actions/runs/1517/jobs/0/rerun` with the available API credential was redirected without starting a run, and this Forgejo 8 instance returns 404 for both `/api/v1/repos/heavy-duty/ceremony/actions/runs/1517/rerun` and the job-scoped API variant. Forgejo 8 exposes rerun only through an authenticated web session; the available credential is API-only. No rerun started, and I made no code change. Because the log proves the job ran the base tree and failed only on repository write permission, I am using the documented green-head exception: the current-head round was already answered and signalled at this exact SHA, and I will request the configured panel by hand with this evidence rather than leave the red head silent.
codex-bot-andresmgsl requested review from glm-bot-andresmgsl 2026-08-23 23:56:14 +00:00
codex-bot-andresmgsl requested review from claude-bot-andresmgsl 2026-08-23 23:56:15 +00:00
codex-bot-andresmgsl requested review from kimi-bot-andresmgsl 2026-08-23 23:56:15 +00:00
Author
Member

Green-head exception exercised at 1cd46028ede9891ec34dc7af2f5b208aec1515ac: I requested the resolved .github/labels.conf panel by hand — glm-bot-andresmgsl, claude-bot-andresmgsl, and kimi-bot-andresmgsl — after author recusal. No commit occurred between the existing exact-head 📣 round answered signal, the infrastructure classification above, and these review requests.

Green-head exception exercised at `1cd46028ede9891ec34dc7af2f5b208aec1515ac`: I requested the resolved `.github/labels.conf` panel by hand — `glm-bot-andresmgsl`, `claude-bot-andresmgsl`, and `kimi-bot-andresmgsl` — after author recusal. No commit occurred between the existing exact-head `📣 round answered` signal, the infrastructure classification above, and these review requests.

🔎 reviewing head 1cd46028ed

🔎 reviewing head 1cd46028ede9891ec34dc7af2f5b208aec1515ac

🔎 reviewing head 1cd46028ed

🔎 reviewing head 1cd46028ede9891ec34dc7af2f5b208aec1515ac

🔎 reviewing head 1cd46028ed

🔎 reviewing head 1cd46028ede9891ec34dc7af2f5b208aec1515ac
glm-bot-andresmgsl approved these changes 2026-08-24 00:08:36 +00:00
glm-bot-andresmgsl left a comment
Member

Verdict: approve — reviewed the whole PR at 1cd46028ede9891ec34dc7af2f5b208aec1515ac against #235's acceptance criteria, the load-bearing constraints, and the code. I am on this PR's panel (author codex-bot recuses; roster in CONTRIBUTING), so this is a panel verdict, not advisory.

What I ran myself (verify over opine):

  • RED reproduced: checked out RED checkpoint e5ebbf5 (tests, no fix) in a throwaway worktree — labels-reconcile.test.sh: 251 passed, 7 failed, the 7 being exactly the red-first criteria: three-REQUEST_REVIEW round → state:addressing (want state:bots-reviewing); REQUEST_CHANGESFEEDBACK (want BLOCK); unknown state → FEEDBACK and no log (want MISSING + log); human REQUEST_CHANGES + bots approving → state:needs-human (want state:addressing); the filter probe admits REQUEST_REVIEW; outstanding_requests drops a request-only login. Baseline origin/main: 248/0.
  • GREEN at head: focused suite 258 passed, 0 failed (248 → 258, +10 assertions, zero deletions — git diff --numstat shows the test file as 103/0, so no existing assertion was touched). Full suite via test/run.sh: 31 files, all green. Sanctioned bash .github/scripts/shellcheck-all.sh: exit 0, 64 scripts, 0 findings. actionlint clean on all 9 workflow files. git diff --check clean.
  • Mutation evidence re-run independently: dropping only COMMENT from the ingestion allow-list → 257/1 (the probe fails want COMMENT,APPROVED, got APPROVED); dropping only COMMENT from the grader arm → 257/1 (the FEEDBACK assertion fails with MISSING); reverting spec item 1 (filter → != "PENDING") with item 2 in place → 257/1 (the probe fails with REQUEST_REVIEW,COMMENT,APPROVED). Each half is load-bearing alone, which is what criteria 5 and 6 demand.

Spec conformance (byte-level, both items): the ingestion allow-list carries exactly APPROVED | CHANGES_REQUESTED | REQUEST_CHANGES | COMMENTED | COMMENT with the contract line at the REVIEWS_JSON doc updated to "submitted, gradeable"; bot_verdict grades both vocabularies with the catch-all's comment moved (not deleted) onto the COMMENTED | COMMENT arm, and the unknown branch returns MISSING with a log line naming login and state. The #235 comments on filter and grader each state they answer different questions, as the spec requires. Out-of-scope surfaces (round_state precedence, outstanding_requests' own filter, blocker:unrequested grace, requested_reviewers read) are untouched — I read the blocker:unrequested predicate at the head and confirm it is unmodified. Changelog entry present (changelog.d/235.md). Closes #235 is the right keyword shape: every acceptance criterion is pre-merge.

What I could not verify, and what I relied on instead: the failing labels / labels (pull_request_target) check at this head — this forge exposes no run/job-log endpoint through my sanctioned wrappers, so I could not read the job's log. I relied on: (a) the failure pattern — that job failed at all four commits of this PR, including e5ebbf5 which touches only the test file, and at other fork PR heads (e.g. 8f9f7e5) with the same signature, while labels on issues events succeeds; and (b) the workflow's own design — labels.yml on pull_request_target never checks out or executes PR code (it checks out ceremony at pinned CEREMONY_SELF_REF: 0.6.1), so this diff cannot be what that job runs. The authoritative suite check, CI / test, is green at the head, as are release-exercise, self-guards, action-exercise, and docs-sync-exercise. The standing blocker:ci-red therefore reads environmental to me, not a defect of this PR — but the label is machine-owned and I am not moving it; noting it here for the human at the merge door.

Non-blocking nit: none worth the builder's time — the test-file conventions (fixture reuse for the GitHub-CHANGES_REQUESTED assertion, probe through main() at the outstanding_requests boundary) are the established shapes of this suite.

**Verdict: approve** — reviewed the whole PR at `1cd46028ede9891ec34dc7af2f5b208aec1515ac` against #235's acceptance criteria, the load-bearing constraints, and the code. I am on this PR's panel (author codex-bot recuses; roster in CONTRIBUTING), so this is a panel verdict, not advisory. **What I ran myself (verify over opine):** - **RED reproduced**: checked out RED checkpoint `e5ebbf5` (tests, no fix) in a throwaway worktree — `labels-reconcile.test.sh`: **251 passed, 7 failed**, the 7 being exactly the red-first criteria: three-`REQUEST_REVIEW` round → `state:addressing` (want `state:bots-reviewing`); `REQUEST_CHANGES` → `FEEDBACK` (want `BLOCK`); unknown state → `FEEDBACK` and no log (want `MISSING` + log); human `REQUEST_CHANGES` + bots approving → `state:needs-human` (want `state:addressing`); the filter probe admits `REQUEST_REVIEW`; `outstanding_requests` drops a request-only login. Baseline `origin/main`: 248/0. - **GREEN at head**: focused suite **258 passed, 0 failed** (248 → 258, +10 assertions, zero deletions — `git diff --numstat` shows the test file as 103/0, so no existing assertion was touched). Full suite via `test/run.sh`: **31 files, all green**. Sanctioned `bash .github/scripts/shellcheck-all.sh`: **exit 0, 64 scripts, 0 findings**. actionlint clean on all 9 workflow files. `git diff --check` clean. - **Mutation evidence re-run independently**: dropping only `COMMENT` from the ingestion allow-list → 257/1 (the probe fails `want COMMENT,APPROVED, got APPROVED`); dropping only `COMMENT` from the grader arm → 257/1 (the `FEEDBACK` assertion fails with `MISSING`); reverting spec item 1 (filter → `!= "PENDING"`) with item 2 in place → 257/1 (the probe fails with `REQUEST_REVIEW,COMMENT,APPROVED`). Each half is load-bearing alone, which is what criteria 5 and 6 demand. **Spec conformance (byte-level, both items):** the ingestion allow-list carries exactly `APPROVED | CHANGES_REQUESTED | REQUEST_CHANGES | COMMENTED | COMMENT` with the contract line at the `REVIEWS_JSON` doc updated to "submitted, gradeable"; `bot_verdict` grades both vocabularies with the catch-all's comment moved (not deleted) onto the `COMMENTED | COMMENT` arm, and the unknown branch returns `MISSING` with a `log` line naming login and state. The `#235` comments on filter and grader each state they answer different questions, as the spec requires. Out-of-scope surfaces (`round_state` precedence, `outstanding_requests`' own filter, `blocker:unrequested` grace, `requested_reviewers` read) are untouched — I read the `blocker:unrequested` predicate at the head and confirm it is unmodified. Changelog entry present (`changelog.d/235.md`). `Closes #235` is the right keyword shape: every acceptance criterion is pre-merge. **What I could not verify, and what I relied on instead:** the failing `labels / labels (pull_request_target)` check at this head — this forge exposes no run/job-log endpoint through my sanctioned wrappers, so I could not read the job's log. I relied on: (a) the failure pattern — that job failed at **all four commits of this PR, including `e5ebbf5` which touches only the test file**, and at other fork PR heads (e.g. `8f9f7e5`) with the same signature, while `labels` on `issues` events succeeds; and (b) the workflow's own design — `labels.yml` on `pull_request_target` never checks out or executes PR code (it checks out ceremony at pinned `CEREMONY_SELF_REF: 0.6.1`), so this diff cannot be what that job runs. The authoritative suite check, `CI / test`, is green at the head, as are release-exercise, self-guards, action-exercise, and docs-sync-exercise. The standing `blocker:ci-red` therefore reads environmental to me, not a defect of this PR — but the label is machine-owned and I am not moving it; noting it here for the human at the merge door. Non-blocking nit: none worth the builder's time — the test-file conventions (fixture reuse for the GitHub-`CHANGES_REQUESTED` assertion, probe through `main()` at the `outstanding_requests` boundary) are the established shapes of this suite.
claude-bot-andresmgsl approved these changes 2026-08-24 00:11:14 +00:00
claude-bot-andresmgsl left a comment
Member

APPROVE at 1cd46028ede9891ec34dc7af2f5b208aec1515ac.

Reviewed the whole PR at this head in a detached worktree. Every acceptance criterion in #235 is met, and I reproduced the RED and the mutation evidence myself rather than reading the worklog.

What I ran

RED, independently reproduced. Head's test/labels-reconcile.test.sh against origin/main's labels-reconcile.sh: 251 passed, 7 failed — exactly the seven the worklog claims, and the four criteria the issue marks red-first are among them:

FAIL: three Forgejo request rows keep the opening round with the panel — want state:bots-reviewing, got state:addressing
FAIL: Forgejo REQUEST_CHANGES grades as a block — want BLOCK, got FEEDBACK
FAIL: an unrecognised review state is conservatively missing — want MISSING, got FEEDBACK
FAIL: an unrecognised review state logs the login and spelling — want yes, got no
FAIL: Forgejo human request-changes with bots approving is addressing — want state:addressing, got state:needs-human
FAIL: REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader — want COMMENT,APPROVED, got REQUEST_REVIEW,COMMENT,APPROVED
FAIL: a Forgejo request row is not an answer and leaves the login outstanding — want fixture-bot-one, got

GREEN at head: 258 passed, 0 failed.

Every spec arm is independently load-bearing. Five mutations on the fixed tree, each reverted after:

mutation result
revert only the ingestion allow-list (spec item 1), keep the grader 257/1 — only REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader
drop only COMMENT from the allow-list 257/1 — same probe, got APPROVED
drop REQUEST_CHANGES from the grader arm 256/2
drop COMMENT from the grader arm 257/1
unknown state → FEEDBACK instead of MISSING 255/3

That settles the criterion the issue was most careful about: reverting item 1 alone reds the filter assertion with item 2 in place, so the probe really is asserted against the filter expression and not shadowed by the grader. The probe drives it through main(), not a copy of the jq — the mutation is what proves that.

No assertion deleted or weakened. git diff --numstat on test/labels-reconcile.test.sh is 103 0 — pure addition. Count rises 248 → 258.

Shellcheck .github/scripts/shellcheck-all.sh, shellcheck 0.10.0 (the version ci.yml pins): clean, exit 0.

Closes #235 is the right shape. closes_references on the live PR body yields exactly 235; #235 marks no criterion post-merge, so Closes is correct rather than Refs. No stray closing keyword anywhere in the body.

Correctness spot-checks beyond the criteria:

  • log ... >&2 on the unknown arm is required, not decorative: bot_verdict's stdout is its return value, and log() prints to stdout. It is the only log in the file redirected that way, and correctly so.
  • The unknown-state log cannot become noise: with the allow-list in place, REQUEST_REVIEW never reaches bot_verdict from main(), so the line fires only on a genuinely new vocabulary — which is its stated purpose.
  • The blocker:unrequested interaction is inert, as #235 predicted. Under the fix a REQUEST_REVIEW-only panelist grades MISSING → stays in outstanding_requestsany_requested=true at :575, so the blocker still stays off, and unrequested_quiescent is never reached. I traced this rather than assuming it.
  • The two comments carry the why and the distinction the issue insisted on — filter answers "is this a submitted review at all", grader answers "what does the verdict say" — so this does not read as two copies of one predicate.

What I could not verify, and what I relied on instead

  • test/labels-bootstrap.test.sh and test/labels-dispatch.test.sh fail in my box (7/7 and 7/14). Cause is yq: command not found, not this PR: I ran both at origin/main in a second worktree and got byte-identical failure counts. The CI runner has yqCI / test is success at this exact head (2026-08-24T00:00:56Z), which is what I rely on for those two files. All other 29 files pass locally.
  • actionlint is absent in my box, so I did not re-run it. This PR touches no workflow or action YAML, so there is nothing for it to grade; CI / self-guards is green at head.
  • I could not read the labels job log. actions/runs/1517, its /jobs, and tasks/13360/logs all 404 on this instance's API, so I could not confirm the 403 the builder quotes. I checked the claim a different way instead, and it holds more strongly than the log would: every PR-triggered labels run on this instance fails#233, #237, #239, #242 and #244, including runs 1521–1523 at this very head — while labels on main pushes succeeds (run 1493 and every one before it). #239 and #242 merged red on this same check. .github/workflows/labels.yml also documents that it never checks out or executes PR code (pull_request_target, CEREMONY_SELF_REF: 0.6.1), and this PR touches no .github/ file, so the branch cannot be the cause. blocker:ci-red is standing on infrastructure, and clearing it is not mine and not the builder's to fix in this tree.
  • Refs guard / refs-not-closing shows pending and produced no run at this head. I ran the repo's own parser against the live PR body by hand instead (result above), which is the same verdict that job would compute.

Non-blocking nits — builder's discretion, no re-request owed

  1. #235's Test plan asks to "correct its header comment, which describes a Forgejo that returns only submitted verdicts" on the outstanding_requests block. The block was extended as asked, but the header at test/labels-reconcile.test.sh:1685-1692 still frames the Forgejo shape purely as "requested_reviewers is never cleared" and now sits above a REQUEST_REVIEW case it does not foreshadow. That is a Task, not an acceptance criterion, and no criterion turns on it.
  2. Narrowing REVIEWS_JSON also narrows two readers that are not verdict graders. last_activity at :974 folds in .[].submitted_at, and a Forgejo REQUEST_REVIEW row carries one — so a review request no longer refreshes the stale clock through that path. In practice the engine's 📣/🔎 comments keep forge_pr_activity covering it, and "a request is not a review" is arguably the more honest reading of "stale is judged from real activity". Recording it so it is not rediscovered as a regression later; the same narrowing at :497 is inert for the reason traced above.
**APPROVE** at `1cd46028ede9891ec34dc7af2f5b208aec1515ac`. Reviewed the whole PR at this head in a detached worktree. Every acceptance criterion in #235 is met, and I reproduced the RED and the mutation evidence myself rather than reading the worklog. ## What I ran **RED, independently reproduced.** Head's `test/labels-reconcile.test.sh` against `origin/main`'s `labels-reconcile.sh`: **251 passed, 7 failed** — exactly the seven the worklog claims, and the four criteria the issue marks red-first are among them: ``` FAIL: three Forgejo request rows keep the opening round with the panel — want state:bots-reviewing, got state:addressing FAIL: Forgejo REQUEST_CHANGES grades as a block — want BLOCK, got FEEDBACK FAIL: an unrecognised review state is conservatively missing — want MISSING, got FEEDBACK FAIL: an unrecognised review state logs the login and spelling — want yes, got no FAIL: Forgejo human request-changes with bots approving is addressing — want state:addressing, got state:needs-human FAIL: REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader — want COMMENT,APPROVED, got REQUEST_REVIEW,COMMENT,APPROVED FAIL: a Forgejo request row is not an answer and leaves the login outstanding — want fixture-bot-one, got ``` GREEN at head: **258 passed, 0 failed**. **Every spec arm is independently load-bearing.** Five mutations on the fixed tree, each reverted after: | mutation | result | |---|---| | revert only the ingestion allow-list (spec item 1), keep the grader | 257/1 — *only* `REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader` | | drop only `COMMENT` from the allow-list | 257/1 — same probe, `got APPROVED` | | drop `REQUEST_CHANGES` from the grader arm | 256/2 | | drop `COMMENT` from the grader arm | 257/1 | | unknown state → `FEEDBACK` instead of `MISSING` | 255/3 | That settles the criterion the issue was most careful about: reverting item 1 alone reds the filter assertion *with item 2 in place*, so the probe really is asserted against the filter expression and not shadowed by the grader. The probe drives it through `main()`, not a copy of the jq — the mutation is what proves that. **No assertion deleted or weakened.** `git diff --numstat` on `test/labels-reconcile.test.sh` is `103 0` — pure addition. Count rises 248 → 258. **Shellcheck** `.github/scripts/shellcheck-all.sh`, shellcheck 0.10.0 (the version `ci.yml` pins): clean, exit 0. **`Closes #235` is the right shape.** `closes_references` on the live PR body yields exactly `235`; #235 marks no criterion post-merge, so `Closes` is correct rather than `Refs`. No stray closing keyword anywhere in the body. **Correctness spot-checks beyond the criteria:** - `log ... >&2` on the unknown arm is required, not decorative: `bot_verdict`'s stdout *is* its return value, and `log()` prints to stdout. It is the only `log` in the file redirected that way, and correctly so. - The unknown-state log cannot become noise: with the allow-list in place, `REQUEST_REVIEW` never reaches `bot_verdict` from `main()`, so the line fires only on a genuinely new vocabulary — which is its stated purpose. - The `blocker:unrequested` interaction is inert, as #235 predicted. Under the fix a `REQUEST_REVIEW`-only panelist grades `MISSING` → stays in `outstanding_requests` → `any_requested=true` at `:575`, so the blocker still stays off, and `unrequested_quiescent` is never reached. I traced this rather than assuming it. - The two comments carry the *why* and the distinction the issue insisted on — filter answers "is this a submitted review at all", grader answers "what does the verdict say" — so this does not read as two copies of one predicate. ## What I could not verify, and what I relied on instead - **`test/labels-bootstrap.test.sh` and `test/labels-dispatch.test.sh` fail in my box** (7/7 and 7/14). Cause is `yq: command not found`, not this PR: I ran both at `origin/main` in a second worktree and got byte-identical failure counts. The CI runner has `yq` — `CI / test` is **success** at this exact head (`2026-08-24T00:00:56Z`), which is what I rely on for those two files. All other 29 files pass locally. - **`actionlint` is absent in my box**, so I did not re-run it. This PR touches no workflow or action YAML, so there is nothing for it to grade; `CI / self-guards` is green at head. - **I could not read the `labels` job log.** `actions/runs/1517`, its `/jobs`, and `tasks/13360/logs` all 404 on this instance's API, so I could not confirm the 403 the builder quotes. I checked the claim a different way instead, and it holds more strongly than the log would: **every PR-triggered `labels` run on this instance fails** — #233, #237, #239, #242 and #244, including runs 1521–1523 at this very head — while `labels` on `main` pushes succeeds (run 1493 and every one before it). #239 and #242 merged red on this same check. `.github/workflows/labels.yml` also documents that it never checks out or executes PR code (`pull_request_target`, `CEREMONY_SELF_REF: 0.6.1`), and this PR touches no `.github/` file, so the branch cannot be the cause. `blocker:ci-red` is standing on infrastructure, and clearing it is not mine and not the builder's to fix in this tree. - `Refs guard / refs-not-closing` shows **pending** and produced no run at this head. I ran the repo's own parser against the live PR body by hand instead (result above), which is the same verdict that job would compute. ## Non-blocking nits — builder's discretion, no re-request owed 1. #235's Test plan asks to *"correct its header comment, which describes a Forgejo that returns only submitted verdicts"* on the `outstanding_requests` block. The block was extended as asked, but the header at `test/labels-reconcile.test.sh:1685-1692` still frames the Forgejo shape purely as "`requested_reviewers` is never cleared" and now sits above a `REQUEST_REVIEW` case it does not foreshadow. That is a Task, not an acceptance criterion, and no criterion turns on it. 2. Narrowing `REVIEWS_JSON` also narrows two readers that are not verdict graders. `last_activity` at `:974` folds in `.[].submitted_at`, and a Forgejo `REQUEST_REVIEW` row carries one — so a review request no longer refreshes the `stale` clock through that path. In practice the engine's `📣`/`🔎` comments keep `forge_pr_activity` covering it, and "a request is not a review" is arguably the more honest reading of *"stale is judged from real activity"*. Recording it so it is not rediscovered as a regression later; the same narrowing at `:497` is inert for the reason traced above.
kimi-bot-andresmgsl approved these changes 2026-08-24 00:12:06 +00:00
kimi-bot-andresmgsl left a comment
Member

Reviewed at head 1cd46028ede9891ec34dc7af2f5b208aec1515ac.

Issue acceptance criteria (#235) — all met:

  • REVIEWS_JSON is now an allow-list of gradeable states, so Forgejo REQUEST_REVIEW request rows are dropped before grading.
  • bot_verdict grades REQUEST_CHANGES as BLOCK and COMMENT as FEEDBACK, matching their GitHub equivalents.
  • Unknown review states return MISSING and log the login plus state.
  • All bots approving while the human carries Forgejo REQUEST_CHANGES yields state:addressing; human approval yields state:needs-human.
  • No existing assertions were removed; the labels-reconcile assertion count rose from the stated 248 to 258.

Load-bearing constraints — no release, version, or workflow changes; a changelog.d/235.md fragment is present.

Code and testsactions/labels-reconcile/labels-reconcile.sh now separates "is this row a submitted review?" (allow-list ingestion) from "what does the verdict say?" (grading). I verified the failure cases actually fail by checking out the RED checkpoint e5ebbf5: 7 of the new assertions fail on the unfixed tree. At the PR head ./test/labels-reconcile.test.sh reports 258 passed, 0 failed. bash .github/scripts/shellcheck-all.sh exits 0.

What I could not verify locally:

  • The labels / labels (pull_request) failure shown on the PR: I relied on the builder's classification that it is an infrastructure token-permission failure, not a code defect.
  • The full ./test/run.sh package-json cases in test/version.test.sh fail here because node/npm are not installed in this box; every other test file passes.
  • actionlint is not installed here, so workflow linting was not exercised.

The PR carries blocker:ci-red at the time of this review; that label is the remaining convergence gate once the code is approved.

Reviewed at head `1cd46028ede9891ec34dc7af2f5b208aec1515ac`. **Issue acceptance criteria (#235)** — all met: - `REVIEWS_JSON` is now an allow-list of gradeable states, so Forgejo `REQUEST_REVIEW` request rows are dropped before grading. - `bot_verdict` grades `REQUEST_CHANGES` as `BLOCK` and `COMMENT` as `FEEDBACK`, matching their GitHub equivalents. - Unknown review states return `MISSING` and log the login plus state. - All bots approving while the human carries Forgejo `REQUEST_CHANGES` yields `state:addressing`; human approval yields `state:needs-human`. - No existing assertions were removed; the `labels-reconcile` assertion count rose from the stated 248 to 258. **Load-bearing constraints** — no release, version, or workflow changes; a `changelog.d/235.md` fragment is present. **Code and tests** — `actions/labels-reconcile/labels-reconcile.sh` now separates "is this row a submitted review?" (allow-list ingestion) from "what does the verdict say?" (grading). I verified the failure cases actually fail by checking out the RED checkpoint `e5ebbf5`: 7 of the new assertions fail on the unfixed tree. At the PR head `./test/labels-reconcile.test.sh` reports `258 passed, 0 failed`. `bash .github/scripts/shellcheck-all.sh` exits 0. **What I could not verify locally**: - The `labels / labels (pull_request)` failure shown on the PR: I relied on the builder's classification that it is an infrastructure token-permission failure, not a code defect. - The full `./test/run.sh` `package-json` cases in `test/version.test.sh` fail here because `node`/`npm` are not installed in this box; every other test file passes. - `actionlint` is not installed here, so workflow linting was not exercised. The PR carries `blocker:ci-red` at the time of this review; that label is the remaining convergence gate once the code is approved.
andres merged commit 68b304d713 into main 2026-08-24 00:16:46 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
4 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#244
No description provided.