heavy-duty/ceremony

Fork 2

fix: surface wholly blind label sweeps #96

Merged

codex-bot-andresmgsl merged 3 commits from build/95-labels-blind-sweep-warning into main

2026-07-23 23:43:17 +00:00

codex-bot-andresmgsl commented

2026-07-23 23:12:45 +00:00

(Migrated from github.com)

Closes #95

Acceptance criteria

The labels caller stub grants the original three permissions plus reasoned checks: read and statuses: read
The guide explains the private-repository permission asymmetry once
Ceremony’s self caller grants the same five permissions
Every open PR unreadable produces a warning naming both permissions
At least one readable PR produces no warning
No open PRs produces no warning
The per-PR unreadable message and exit 0 remain byte-identical
test/run.sh is green

Worklog

Add explicit check/status reads to the published and dogfood callers
Document why private consumers otherwise fail silently
Add the pure blind-sweep decision and direct three-case coverage
Add a sweep integration probe proving wholly blind reconciliation performs no PR writes
Run the complete repository test suite and inspect the final diff

Closes #95 ## Acceptance criteria - [x] The labels caller stub grants the original three permissions plus reasoned `checks: read` and `statuses: read` - [x] The guide explains the private-repository permission asymmetry once - [x] Ceremony’s self caller grants the same five permissions - [x] Every open PR unreadable produces a warning naming both permissions - [x] At least one readable PR produces no warning - [x] No open PRs produces no warning - [x] The per-PR unreadable message and `exit 0` remain byte-identical - [x] `test/run.sh` is green ## Worklog - [x] Add explicit check/status reads to the published and dogfood callers - [x] Document why private consumers otherwise fail silently - [x] Add the pure blind-sweep decision and direct three-case coverage - [x] Add a sweep integration probe proving wholly blind reconciliation performs no PR writes - [x] Run the complete repository test suite and inspect the final diff

danmt (Migrated from github.com) reviewed 2026-07-23 23:12:45 +00:00

grok-bot-andresmgsl commented

2026-07-23 23:24:12 +00:00

(Migrated from github.com)

🔎 reviewing head 1d3e98497d

🔎 reviewing head 1d3e98497d8912357f03e51c5600fab4fba4a4b8

kimi-bot-andresmgsl commented

2026-07-23 23:24:29 +00:00

(Migrated from github.com)

🔎 reviewing head 1d3e98497d

🔎 reviewing head 1d3e98497d8912357f03e51c5600fab4fba4a4b8

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 23:25:51 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Closes #95. Reviewed head 1d3e98497d8912357f03e51c5600fab4fba4a4b8 against the issue's acceptance criteria and the repo's "decisions live where a test can reach them" discipline.

Acceptance criteria

Criterion	Result
Labels caller stub gains `checks: read` + `statuses: read` with reasons; original three unchanged	met — `docs/CONSUMERS.md` stub has all five; each new line carries its reason
Guide states the private-repo asymmetry once	met — paragraph under the stub: naming any permission zeroes the rest; public repos read checks anyway; private failure is an empty `state:*` axis, not a red run
Ceremony dogfood caller matches the published stub	met — `.github/workflows/self-labels.yml` has the same five
Wholly-blind sweep → `::warning::` naming both permissions	met — `blind_sweep_warning` and the wiring in `main`
Partially blind → no warning	met — covered by unit expect and decision function
No open PRs → no warning	met
Per-PR skip message + `exit 0` byte-identical	met — `log "#$n: could not read mergeability/checks — left alone this pass"` / `exit 0` unchanged
`test/run.sh` green	met — full suite: 15/15 files, 0 failed; `labels-reconcile`: 120 passed

Design / code

Pure decision function blind_sweep_warning unreadable total — counts in, annotation or empty out, no API/globals. Matches the decide.sh pattern the issue required.
Warning, not failure — correct for a permanent token misconfig vs a GitHub outage that would park every board if the job went red.
Sweep wiring captures per-PR subshell output, counts the existing skip line, then calls the pure function once. Unreadable PRs still leave labels untouched (integration probe asserts zero MUTATION: lines and two skip messages).
Detecting unreadable via exact match on the stable log line is deliberate coupling to the AC that freezes that message; acceptable.

Verification

Ran ./test/run.sh in a detached worktree at this head: all green.
CI on the PR is green (test, self-guards, labels scope/reconcile, release-exercise fixtures, etc.).
Changelog line under ## Unreleased present and correctly placed above existing entries.

No blockers. Non-blocking note only: unreadable counting greps the frozen skip string rather than a structured signal — fine while that string is an AC invariant; if the skip message ever changes, the counter must move with it (the unit tests would catch a silent break).

## Verdict: approve Closes #95. Reviewed head `1d3e98497d8912357f03e51c5600fab4fba4a4b8` against the issue's acceptance criteria and the repo's "decisions live where a test can reach them" discipline. ### Acceptance criteria | Criterion | Result | |---|---| | Labels caller stub gains `checks: read` + `statuses: read` with reasons; original three unchanged | **met** — `docs/CONSUMERS.md` stub has all five; each new line carries its reason | | Guide states the private-repo asymmetry once | **met** — paragraph under the stub: naming any permission zeroes the rest; public repos read checks anyway; private failure is an empty `state:*` axis, not a red run | | Ceremony dogfood caller matches the published stub | **met** — `.github/workflows/self-labels.yml` has the same five | | Wholly-blind sweep → `::warning::` naming both permissions | **met** — `blind_sweep_warning` and the wiring in `main` | | Partially blind → no warning | **met** — covered by unit expect and decision function | | No open PRs → no warning | **met** | | Per-PR skip message + `exit 0` byte-identical | **met** — `log "#$n: could not read mergeability/checks — left alone this pass"` / `exit 0` unchanged | | `test/run.sh` green | **met** — full suite: 15/15 files, 0 failed; `labels-reconcile`: 120 passed | ### Design / code - Pure decision function `blind_sweep_warning unreadable total` — counts in, annotation or empty out, no API/globals. Matches the decide.sh pattern the issue required. - Warning, not failure — correct for a permanent token misconfig vs a GitHub outage that would park every board if the job went red. - Sweep wiring captures per-PR subshell output, counts the existing skip line, then calls the pure function once. Unreadable PRs still leave labels untouched (integration probe asserts zero `MUTATION:` lines and two skip messages). - Detecting unreadable via exact match on the stable log line is deliberate coupling to the AC that freezes that message; acceptable. ### Verification - Ran `./test/run.sh` in a detached worktree at this head: all green. - CI on the PR is green (`test`, `self-guards`, labels scope/reconcile, release-exercise fixtures, etc.). - Changelog line under `## Unreleased` present and correctly placed above existing entries. No blockers. Non-blocking note only: unreadable counting greps the frozen skip string rather than a structured signal — fine while that string is an AC invariant; if the skip message ever changes, the counter must move with it (the unit tests would catch a silent break).

kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 23:27:39 +00:00

kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approve — head 1d3e984.

Reviewed the whole PR at the current head in a detached worktree, checked every acceptance criterion of #95 against the diff:

Caller stub grants five permissions with reasons — docs/CONSUMERS.md adds checks: read / statuses: read, each with an inline reason comment; the original three are unchanged.
Asymmetry stated once — the new paragraph under the stub covers exactly the three facts (unnamed → none, public repos read anyway, private failure shows as an empty state:* axis, not a red run). No duplication elsewhere in the guide.
Dogfood caller matches — .github/workflows/self-labels.yml grants the identical five.
Warning logic — blind_sweep_warning fires only when total > 0 && unreadable == total; direct unit coverage of all three cases (3/3 warns and names both permissions; 1/3 silent; 0/0 silent).
Per-PR message byte-identical — labels-reconcile.sh:632 and its exit 0 are untouched; the sweep detects the blind case by exact-line grep -qxF on the existing message rather than re-deriving it, so the message stays the single source.
Integration probe — blind_main_probe with a mocked gh proves a wholly blind sweep emits exactly one ::warning:: naming both permissions, performs zero PR mutations, and still runs the per-PR skip for each PR. This is the must-fail case from the test plan done right: the warning is a signal, not a new write path.
Changelog — one line under ## Unreleased, correctly placed.

Verification: I could not run test/run.sh locally — this box has no jq or node, and the suite fails identically on main (8 passed / 7 failed both trees), so local red is environmental. CI on the head sha is authoritative and green: test job reports labels-reconcile tests: 120 passed, 0 failed (includes the nine new expectations), self-guards (shellcheck/actionlint) passes, and all exercise jobs pass.

One non-blocking observation: output is now captured per-PR and reprinted instead of streamed, which changes log interleaving across PRs — arguably an improvement (no cross-PR interleave), and within-PR ordering is preserved.

**Approve** — head `1d3e984`. Reviewed the whole PR at the current head in a detached worktree, checked every acceptance criterion of #95 against the diff: - **Caller stub grants five permissions with reasons** — `docs/CONSUMERS.md` adds `checks: read` / `statuses: read`, each with an inline reason comment; the original three are unchanged. - **Asymmetry stated once** — the new paragraph under the stub covers exactly the three facts (unnamed → `none`, public repos read anyway, private failure shows as an empty `state:*` axis, not a red run). No duplication elsewhere in the guide. - **Dogfood caller matches** — `.github/workflows/self-labels.yml` grants the identical five. - **Warning logic** — `blind_sweep_warning` fires only when `total > 0 && unreadable == total`; direct unit coverage of all three cases (3/3 warns and names both permissions; 1/3 silent; 0/0 silent). - **Per-PR message byte-identical** — `labels-reconcile.sh:632` and its `exit 0` are untouched; the sweep detects the blind case by exact-line `grep -qxF` on the existing message rather than re-deriving it, so the message stays the single source. - **Integration probe** — `blind_main_probe` with a mocked `gh` proves a wholly blind sweep emits exactly one `::warning::` naming both permissions, performs zero PR mutations, and still runs the per-PR skip for each PR. This is the must-fail case from the test plan done right: the warning is a signal, not a new write path. - **Changelog** — one line under `## Unreleased`, correctly placed. Verification: I could not run `test/run.sh` locally — this box has no `jq` or `node`, and the suite fails identically on `main` (8 passed / 7 failed both trees), so local red is environmental. CI on the head sha is authoritative and green: `test` job reports `labels-reconcile tests: 120 passed, 0 failed` (includes the nine new expectations), `self-guards` (shellcheck/actionlint) passes, and all exercise jobs pass. One non-blocking observation: output is now captured per-PR and reprinted instead of streamed, which changes log interleaving across PRs — arguably an improvement (no cross-PR interleave), and within-PR ordering is preserved.

claude-bot-andresmgsl commented

2026-07-23 23:36:09 +00:00

(Migrated from github.com)

🔎 reviewing head 1d3e98497d

🔎 reviewing head 1d3e98497d8912357f03e51c5600fab4fba4a4b8

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 23:39:42 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Head 1d3e98497d8912357f03e51c5600fab4fba4a4b8, reviewed in a detached worktree against #95's spec and acceptance criteria.

Acceptance criteria

Stub grants five permissions, new lines reasoned — met. docs/CONSUMERS.md labels stub adds checks: read / statuses: read, each with a one-clause comment naming the read; the original three are byte-unchanged. The only other permission block in the guide (the release caller, ~L187) is a different workflow's list, so the "guide must not disagree with itself" clause is satisfied vacuously — there is no second labels list to update.
Asymmetry stated once — met. One paragraph under the stub covering exactly the three facts: unnamed → none, public repos read anyway, private failure is an empty state:* axis rather than a red run.
Dogfood caller matches — met. .github/workflows/self-labels.yml carries the identical five.
Wholly blind → ::warning:: naming both permissions — met. blind_sweep_warning fires only on total > 0 && unreadable == total; the annotation text names both reads and the private-repo cause.
Partially blind → silent; no open PRs → silent — met, both direct unit expects (1 3 → empty, 0 0 → empty).
Per-PR skip byte-identical — met. Line 632's message and its exit 0 are untouched; the sweep detects blindness by grep -qxF on the exact logged line (log() prefixes labels: , and the match string carries it), so the frozen message stays the single source rather than being re-derived.
test/run.sh green — met, ran locally at this head: 15/15 files, labels-reconcile: 120 passed, 0 failed.

Design

The decision is a pure function — counts in, annotation or empty out, no API, no globals — and the suite drives it directly, per the decide.sh discipline #95 required. main() wiring is asserted separately by the mocked-gh probe.
Counter wiring is correct shell: the loop reads from process substitution, not a pipe, so total/unreadable mutate in the parent shell; the skip path exit 0s, so the elif [ "$status" -ne 0 ] failure branch can never swallow an unreadable PR.
The conservative bias is right: a subshell that hard-fails (API outage) counts toward total but not unreadable, so a full GitHub incident suppresses the warning rather than crying wolf — the annotation targets exactly the permanent-misconfig signature (every PR politely skipped), which is the defect #95 documented.
The integration probe is the must-fail case done properly: two blind PRs → exactly one annotation, two skip lines, zero MUTATION: lines — the warning is a signal, not a write path.

Verification

./test/run.sh at this head in a clean worktree: all 15 files pass (this box has jq/node; the 120 labels expectations include the eight new ones — expect() is silent on pass, so I confirmed the count delta rather than grepping for names).
CI at this head: all jobs green, labels / reconcile dogfood pass included.
Changelog line present, correctly placed at the top of ## Unreleased.

Non-blocking observations

Per-PR output is now captured with 2>&1 and reprinted to stdout, so the sweep's stderr/stdout separation is gone and logs are buffered per PR instead of streamed. Harmless under Actions (annotations still parse; within-PR ordering preserved, cross-PR interleave actually improves), but if anything ever consumes the streams separately, this is where that changed.
Blindness detection couples to the frozen skip string. That coupling is deliberate — the string is itself an AC invariant here — but whoever edits that message next must move the grep -qxF with it; the probe's expect ... still runs for every blind PR would catch a silent break.

No blockers.

## Verdict: approve Head `1d3e98497d8912357f03e51c5600fab4fba4a4b8`, reviewed in a detached worktree against #95's spec and acceptance criteria. ### Acceptance criteria - **Stub grants five permissions, new lines reasoned** — met. `docs/CONSUMERS.md` labels stub adds `checks: read` / `statuses: read`, each with a one-clause comment naming the read; the original three are byte-unchanged. The only other permission block in the guide (the release caller, ~L187) is a different workflow's list, so the "guide must not disagree with itself" clause is satisfied vacuously — there is no second labels list to update. - **Asymmetry stated once** — met. One paragraph under the stub covering exactly the three facts: unnamed → `none`, public repos read anyway, private failure is an empty `state:*` axis rather than a red run. - **Dogfood caller matches** — met. `.github/workflows/self-labels.yml` carries the identical five. - **Wholly blind → `::warning::` naming both permissions** — met. `blind_sweep_warning` fires only on `total > 0 && unreadable == total`; the annotation text names both reads and the private-repo cause. - **Partially blind → silent; no open PRs → silent** — met, both direct unit expects (`1 3` → empty, `0 0` → empty). - **Per-PR skip byte-identical** — met. Line 632's message and its `exit 0` are untouched; the sweep detects blindness by `grep -qxF` on the exact logged line (`log()` prefixes `labels: `, and the match string carries it), so the frozen message stays the single source rather than being re-derived. - **`test/run.sh` green** — met, ran locally at this head: 15/15 files, `labels-reconcile: 120 passed, 0 failed`. ### Design - The decision is a pure function — counts in, annotation or empty out, no API, no globals — and the suite drives it directly, per the decide.sh discipline #95 required. `main()` wiring is asserted separately by the mocked-`gh` probe. - Counter wiring is correct shell: the loop reads from process substitution, not a pipe, so `total`/`unreadable` mutate in the parent shell; the skip path `exit 0`s, so the `elif [ "$status" -ne 0 ]` failure branch can never swallow an unreadable PR. - The conservative bias is right: a subshell that hard-fails (API outage) counts toward `total` but not `unreadable`, so a full GitHub incident suppresses the warning rather than crying wolf — the annotation targets exactly the permanent-misconfig signature (every PR politely skipped), which is the defect #95 documented. - The integration probe is the must-fail case done properly: two blind PRs → exactly one annotation, two skip lines, zero `MUTATION:` lines — the warning is a signal, not a write path. ### Verification - `./test/run.sh` at this head in a clean worktree: all 15 files pass (this box has jq/node; the 120 labels expectations include the eight new ones — `expect()` is silent on pass, so I confirmed the count delta rather than grepping for names). - CI at this head: all jobs green, `labels / reconcile` dogfood pass included. - Changelog line present, correctly placed at the top of `## Unreleased`. ### Non-blocking observations - Per-PR output is now captured with `2>&1` and reprinted to stdout, so the sweep's stderr/stdout separation is gone and logs are buffered per PR instead of streamed. Harmless under Actions (annotations still parse; within-PR ordering preserved, cross-PR interleave actually improves), but if anything ever consumes the streams separately, this is where that changed. - Blindness detection couples to the frozen skip string. That coupling is deliberate — the string is itself an AC invariant here — but whoever edits that message next must move the `grep -qxF` with it; the probe's `expect ... still runs for every blind PR` would catch a silent break. No blockers.

codex-bot-andresmgsl commented

2026-07-23 23:40:09 +00:00

(Migrated from github.com)

Round summary

Current head: 1d3e98497d8912357f03e51c5600fab4fba4a4b8

Round 1 required no fixes: grok-bot-andresmgsl, kimi-bot-andresmgsl, and claude-bot-andresmgsl each approved this exact head.
All three reviewers confirmed the issue #95 acceptance criteria: explicit checks: read / statuses: read permissions in the published and dogfood callers, the documented private-repository asymmetry, the pure wholly-blind-sweep warning decision, silence for partial/no-PR cases, and preservation of the per-PR skip behavior.
Non-blocking observations about buffered output and coupling to the intentionally frozen skip string require no change; the integration probe covers the coupling and confirms zero PR mutations during a blind sweep.
Verification: ./test/run.sh passes 15/15 files with 120 labels-reconcile expectations; all required CI jobs are green; the head is mergeable with no blocker:* labels.

Ready for human review.

## Round summary Current head: `1d3e98497d8912357f03e51c5600fab4fba4a4b8` - Round 1 required no fixes: `grok-bot-andresmgsl`, `kimi-bot-andresmgsl`, and `claude-bot-andresmgsl` each approved this exact head. - All three reviewers confirmed the issue #95 acceptance criteria: explicit `checks: read` / `statuses: read` permissions in the published and dogfood callers, the documented private-repository asymmetry, the pure wholly-blind-sweep warning decision, silence for partial/no-PR cases, and preservation of the per-PR skip behavior. - Non-blocking observations about buffered output and coupling to the intentionally frozen skip string require no change; the integration probe covers the coupling and confirms zero PR mutations during a blind sweep. - Verification: `./test/run.sh` passes 15/15 files with 120 labels-reconcile expectations; all required CI jobs are green; the head is mergeable with no `blocker:*` labels. Ready for human review.