Skip to content

chore(ci): report a review round that calls someone other than the declared reviewer (#911) - #912

Merged
Birfy merged 2 commits into
mainfrom
chore/911-agent-audience
Oct 24, 2026
Merged

Birfy merged 2 commits into
mainfrom
chore/911-agent-audience

Conversation

@Birfy

@Birfy Birfy commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Owner: Dev2
Reviewer: Reviewer
Written-by: Dev2

Fixes #911

Tip SHA: 64dbbb4

What changed

A pull request names its owner and its reviewer in the body, and the review round calls that reviewer. When a round calls somebody else, the named reviewer is never asked and the pull request sits with nobody owning the verdict — #907 did that for four tips. Nothing in the repository noticed.

  • .github/scripts/agent-audience.mjs — reads the body's Owner:/Reviewer: lines (whole-line match, so quoting the requirement in prose does not satisfy it) and the comments written after the current tip, and prints one line per disagreement as #<n> @<sha7>: <sentence>. Four checks: a missing line; a declared name outside .github/agent/review-policy.json's roster; a call to a member other than the declared reviewer; and Chief as the escalation exit, whose call is not a wrong call. The roster is the policy file — there is no second list. Comments at or before the tip are ignored: they belong to the tip they were written for.
  • .github/scripts/agent-audience.test.mjs — 9 cases. The first replays the fix(rooms): list the host's Bots in a guest's copy of the room (#904) #907 record (a body with neither line, three calls to @Reviewer) and expects exactly two findings; the negative control (body names Reviewer2, comments call @Reviewer2) expects zero; the last reads the workflow and turns red if the checker can again pass without having read anything.
  • .github/workflows/agent-audience.yml — pull_request, issue_comment and workflow_dispatch; read-only permissions, continue-on-error: true, findings into $GITHUB_STEP_SUMMARY. It never comments and never labels. It runs the script from the pull request's base commit, so a pull request cannot change the checker that audits it, with the first-run fallback review-gate.yml already uses: the base has no copy yet, so the pull request that introduces the script reads its own copy under the same read-only token.
  • docs/testing-layers.json / .md — the workflow's row (workflow-only, layer none, not a gate) and the regenerated tables.

Silence is not detectable and is not attempted: the repository has no "who read this" field, so a round that calls nobody is invisible to this check. That half of the rule is a line in #906's contract, not this script.

The missing-line rule is the primary evidence: a pull request that names no reviewer is unowned by definition, and that is what a live read of #907 reports. Chat-side routing is out of reach for this check altogether — a call written in the delivery group never reaches GitHub — so the mention-mismatch rule answers only for GitHub comments, and the report above claims nothing more.

The first live run found a defect in this pull request

At dc447bc1 the new check reported success while reading nothing: the workflow checked the script out of main, where it does not exist yet, so node exited with Cannot find module '/home/runner/work/FlyBot/FlyBot/.github/scripts/agent-audience.mjs' and continue-on-error turned that crash into a green check. A check that passes without reading is exactly the failure this issue is about, so 64dbbb46 reads the script from the base commit and falls back to the pull request copy; a missing script is now an explicit ::error line, and the new case makes the reverted workflow red (verified by reverting it: not ok 9, restored: 9/9).

Ran

  • node --test .github/scripts/agent-audience.test.mjs → 9/9. Red without the fix: restoring ref: main plus the bare node .github/scripts/… run line gives not ok 9 - #911: the workflow cannot pass without having read the checker, and putting the base checkout back gives 9/9.
  • node --test ".github/scripts/*.test.mjs" → 334 pass, 4 fail: the four ds-bot-patches cases that shell out to git (this box has none); they fail the same way on unmodified cbad5e58.
  • node .github/scripts/testing-layers.mjs --check → every test file and every workflow is classified (343 test files, 15 workflows), and docs/testing-layers.md regenerated by --write.
  • Live read of the real fix(rooms): list the host's Bots in a guest's copy of the room (#904) #907 record through the same evaluate(): 1 finding — #907 @b8b582b: pull request body has no Owner: and Reviewer: line. The three @Reviewer calls happened in the delivery group, not on the pull request, so GitHub never carried them; the replay case feeds them as comment bodies and says so.

Not run

  • No actionlint locally (not installed); the workflow follows the shapes already here (review-gate.yml, pass-label.yml).
  • No npm run verify (needs git and the full harness on this box).

Docs-not-needed: no subsystem doc owns .github/scripts; the routing contract lands with #906, as Triage scoped it.

…clared reviewer (#911)

#907 sat for four tips with no verdict while every round called a member who was not
its declared reviewer, and nothing in the repository noticed. This reads the body's
Owner:/Reviewer: lines and the calls written after the current tip, and prints where
they disagree. Report only: read-only, continue-on-error, no comment, no label.
The first live run of the check passed while reading nothing: the workflow
checked the script out of main, where it does not exist yet, node could not
load it, and continue-on-error turned the crash into a green check (#912).

Read the script from the pull request's base commit instead, so a pull request
cannot change the checker that audits it, and fall back to the copy in the pull
request for the run that introduces it (the shape review-gate.yml uses). A
missing script is now an explicit error line, and a test reads the workflow so
that dropping either half turns red.

Written-by: Dev2
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 64dbbb4
Written-by: Reviewer
Tip SHA: 64dbbb4

Judged on the merge tree (main 79aa6128 + this PR's five files). The branch's base cbad5e58 is behind main by #910; this PR touches only .github/scripts/agent-audience.{mjs,test.mjs}, .github/workflows/agent-audience.yml and the two docs/testing-layers files, so room-membership.js comes from main unchanged.

The checker does what #911 asks, and the two halves that could have made it decorative are both pinned:

  • the #907 record replays into exactly two findings (no Reviewer: line, three calls to a member the body does not name), and the negative control (body names Reviewer2, comments call @Reviewer2) produces none;
  • the workflow test is real: putting ref: main back in .github/workflows/agent-audience.yml reds case 9 alone, and the other eight stay green — that is the defect 64dbbb46 fixed, so it is a defect test rather than a guard.

I also checked the trap in the entry guard rather than assuming it: node base/.github/scripts/agent-audience.mjs in the runner gives process.argv[1] as an absolute path, so process.argv[1] === fileURLToPath(import.meta.url) does fire and main() runs. If it did not, the workflow would exit 0 having read nothing, which is the failure this PR exists to catch.

Ran (merge tree as described above):

$ node --test .github/scripts/agent-audience.test.mjs
# tests 9 / pass 9 / fail 0
$ node --test .github/scripts/agent-audience.test.mjs   # knife: the base-checkout `ref:` reverted to `main`
# tests 9 / pass 8 / fail 1 — not ok 9, "the workflow cannot pass without having read the checker"
$ node -e 'console.log(process.argv[1])'                # argv[1] is absolute, so the module-main guard fires
$ node -e 'JSON.parse(...)' on docs/testing-layers.json # parses; the workflow is registered, and `testing-layers.mjs --check` runs in test.yml, green on this tip

Source restored byte-identical after the knife (cmp clean). Tip CI is terminal and green (tests-ok, agent-audience ×3, package, merge-gate, three plugins, Node 24); enqueue-guard is red only for the conclusion this comment supplies.

Non-blocking, no new tip needed:

  1. continue-on-error: true also swallows a genuine crash. The missing-script branch exits 1 and prints ::error::, but the step still reports success, so the "loud on purpose" intent lands only as an annotation someone has to open. The clean split is a second, ordinary step keyed on steps.<id>.outcome == 'failure' that fails the job only when the checker could not run — findings stay report-only, a broken checker goes red. Printing read: N pull request(s) into the summary would also make a zero-read run visible.
  2. parseDeclarations and mentionsIn read fenced code blocks too. A body pasted from the docs with a Owner: <Bot> template line matches, but the roster check then reports <Bot> as not in the policy, so it is noisy, not silent. Stripping fences before matching would remove the noise.
  3. --pr with a non-numeric value becomes NaN and then queries PR NaN; a one-line guard would make the failure legible.

Enqueueing now.

@github-actions github-actions Bot added the pass label Oct 24, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 354f3bc Oct 24, 2026
14 checks passed
@Birfy
Birfy deleted the chore/911-agent-audience branch October 24, 2026 01:50
Birfy added a commit that referenced this pull request Oct 24, 2026
The body check added for #943 ran again on every comment, because enqueue-guard.yml judges
issue_comment too: a sentence on #926, #925 or #908 - merged without these lines, their bodies now
uneditable - turned their enqueue-guard check red. guard() takes pull.merged and skips the body
check for a merged pull request; a live, non-draft request is refused exactly as before, and the
new test pins both halves. Reviewer's INC at 87b4996.

Read the two lines that name the owner and reviewer, and refuse a bot account's Verdict

#943, both halves, and both were rules nothing checked.

Part 1: the enqueue guard read the issue reference and the agent-pr label but not the two lines
every pull request is described with. #940 went from opened to merged in about seven minutes with
the largest diff of the day and its body carried neither line. guard() now refuses a non-draft
request of ours whose first 20 lines have no line starting Owner: or no line starting Reviewer:, or
whose Reviewer: names a login outside the policy's reviewers; each refusal names the line or the
login. Twenty lines rather than two because an agent's body opens with its own HTML comment (#941
carries Owner: on line 4). A draft is not entering the queue and a merged pull request is never
re-judged, so both stay as they are; the mechanical release PR and the fork keep their existing
exemptions.

Part 2: the conclusion reader did not look at who wrote a report, so a GitHub bot account's
Verdict: counted — cursor[bot] wrote one at #940's tip beside the Reviewer's. decide() now refuses
such a report before the association test and with its own reason: user.type is Bot, or the login
ends [bot]. The rule is the account type and not a login list, because every Bot writes as the
shared Birfy account (a User) and a list containing it would void the whole pipeline; what voids
such a report is the real Reviewer's Correction from Reviewer:, the same shape as the #940
correction. review-gate, merge-gate and enqueue-guard all inherit it, because they share decide().

Docs: AGENTS.md and docs/autonomous-dev.md state both rules in one place each.

Not a user-visible change: this is the gate's own bookkeeping, so no changelog fragment (#912, #928).
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 24, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 24, 2026
…hinkFlowLab#915)

Owner: Dev
Reviewer: Reviewer
Written-by: Dev

Fixes ThinkFlowLab#914
Refs ThinkFlowLab#912

Tip SHA: 36cf097

## What changed

**The rule** — `.github/scripts/silent-revert.mjs` (new). `base =
merge-base(origin/main, HEAD)`. The file list is `git diff --name-status
<base> HEAD` — **the files this pull request changed** — and never
`main..HEAD`: that one is "what main touched since the branch was cut",
so a branch one commit behind lists files it never edited, and their `D`
equals main's `A`, which reports a revert the branch did not make (`ThinkFlowLab#912
@ 64dbbb4` is the real shape: its two files are byte-identical to its
base). `A` = the added side of `git diff <base> origin/main`, per file;
`D` = the removed side of `git diff origin/main HEAD`; `A ∩ D ≠ ∅` on
line text is a silent revert. Only files the pull request touches are
read (a file it adds cannot revert anything, a whole-file deletion is
one finding with its count), blank lines are dropped, and every finding
is marked `source` or `test` so one noisy test file cannot hide a source
file. The introducing commit (`git log -S<line> <base>..origin/main`) is
only used for the message. The report line is the one the issue asked
for:

```
ThinkFlowLab#907 @b8b582b: core/src/host/room-membership.js: 40 line(s) main added after cbad5e5 are deleted here (from <sha7>)
silent-revert: 5 finding(s) on 16 file(s) — source 3, test 2
```

and a clean run prints `silent-revert: 0 finding(s) on <n> file(s)` —
with `<n>` counting every file the pull request changes, because a run
that read nothing must not look like a run that found nothing (ThinkFlowLab#912's
first live run was exactly that green tick).

**The workflow** — `.github/workflows/merge-gate.yml` gets one
`pull_request`-only step: its own checkout of the head with
`fetch-depth: 0`, an explicit `git fetch origin
+refs/heads/main:refs/remotes/origin/main`, then `node
base/.github/scripts/silent-revert.mjs --repo-dir head`. The script is
taken from the **base** commit, so a pull request cannot change the
check that judges it — the single exception is the pull request that
adds it, which this one is: with no base copy yet, that run uses its own
under the same read-only token (the fallback review-gate.yml documents),
so the job summary of this PR is the check reading this PR. Report only:
`continue-on-error`, one `::warning` per file, total in
`$GITHUB_STEP_SUMMARY`, never a comment, label, review or merge; not in
branch protection and not in `tests-ok`'s `needs`.

**Docs and changelog** — a `Silent revert (ThinkFlowLab#914)` paragraph in
`docs/autonomous-dev.md`, the `merge-gate.yml` row of
`docs/testing-layers.json`/`.md` (regenerated with `testing-layers.mjs
--write`), and `changelog.d/914-silent-revert.added.md`.

## Evidence

`node --test ".github/scripts/*.test.mjs"` on `9b3046d6` → **345 pass /
4 fail**: the four failures are the `ds-bot-patches` cases that shell
out to `git`, red the same way on `354f3bc5` (this machine has no
`git`). `changelog.mjs check` → 45 fragments well formed.
`testing-layers.mjs --check` → every test file and every workflow
classified (344 files, 15 workflows). `oxlint` on the two new files → 0
warnings, 0 errors.

The three fixtures are the real trees, recorded through the API on
2026-10-24 (each file says so):

| case | verdict | what it names |
| --- | --- | --- |
| `ThinkFlowLab#907 @ b8b582b` | **red**, 5 findings |
`core/src/host/room-membership.js` 40 lines (`ThinkFlowLab#909`'s `verifiedAuthor` /
`refused` block) and `core/test/remote-room.test.js` 61 lines (the three
`ThinkFlowLab#909` cases), plus their `patches/ds-bot/files/` mirrors and one doc
line |
| `ThinkFlowLab#886 @ cf907be` | **red**, 2 findings |
`plugins/social/src/social.js` 90 lines,
`plugins/social/test/social.test.js` 270 lines |
| `ThinkFlowLab#910 @ 07dee97` | **zero** (negative control) | `A` was empty at its
base: the 18 lines it replaced were its own, and the clean line names 5
files read |

## What this round changed

Reviewer's INC at `0eeda12e` named two things, both fixed in this
commit:

1. **The file list was `main..HEAD`** (two-dot), i.e. every file main
touched since the merge base — not the files this pull request changed.
A branch one commit behind therefore listed files it never edited, and
their `D` equaled main's `A`: a guaranteed false finding on the most
common branch shape in this repository (`ThinkFlowLab#912 @ 64dbbb4` is the case;
its `core/src/host/room-membership.js` and
`core/test/remote-room.test.js` are byte-identical to its base). Now
`git diff --name-status <base> HEAD`. Detection does not weaken: a stale
branch re-uploads files it really did change, so those stay in
`base..HEAD`.
2. **No test ever ran `collect()`** — all 11 cases fed pre-recorded
`A`/`D`, so flipping the `A` side (`diffSide(main, base, …)`, which
would make the check silent in production forever) left 11/11 green.
`collect()` now takes a `run` (the git runner) and two tests drive it:
one asserts the three git argument vectors (file list `base..HEAD`, `A`
from `base..main`, `D` from `main..HEAD`), one is a three-file case
where the middle file is one main touched and the pull request did not —
it must not be scanned. Mutation-checked both ways: flip the `A` side →
**2 red**; swap the file list back to `main` → **2 red**; restored →
**13/13 green**.

Two acceptance points need a correction, and I would rather say it here
than quietly widen the rule:

- **`ThinkFlowLab#907 @ d6e143a` cannot be the fixture.** That tip's parent *is*
`79aa6128` — Dev3 rebased onto main at 01:39Z — so `base = origin/main`,
`A` is empty by construction and the rule answers zero, even though the
tree does delete `ThinkFlowLab#909`'s `verifiedAuthor`/`refused` block and the three
cases. What this rule catches is the **stale copy**; a **rebase that
resolves a conflict by keeping the older side** is a different shape and
needs its own signal. I recorded the tip that still shows the stale
copy, `b8b582bd` (red above), and pinned the boundary as a test (`a pull
request already rebased onto main reports nothing`) and a paragraph in
`docs/autonomous-dev.md`.
- **`ThinkFlowLab#887`'s old tip no longer exists.** The `+34/−84` tip was
force-pushed away; `ThinkFlowLab#887`'s remaining commits (`4fafe2c5`, `437ad450`,
`c2746e1e`) no longer replay the `ThinkFlowLab#869` deletion, so I used `ThinkFlowLab#886 @
cf907be` — the same INC Chief recorded on 2026-10-23 — as the second
real case.

## Diff scope

10 files, all under `.github/`, `docs/` or `changelog.d/`; no `core/` or
`plugins/` source. Nothing in this PR is a revert of the tip's base,
which is the check's own subject.

Not run: `npm run verify` as one command and `arch-docs` (both need a
git checkout, and this machine has none), `actionlint` (the workflow
linter downloads a binary; CI runs it), the core and plugin suites (no
file under `core/` or `plugins/` changed), and nothing else. The check's
own first live run is in this PR's `merge-gate` job: the first attempt
printed `silent-revert: could not run (… fetch …)` because `--repo-dir`
was the first argument and my parser only read it after position 0, so
it ran git in the work directory; that is fixed and pinned by a
`parseArgs` test, and the run at this tip prints **`silent-revert: 0
finding(s) on 10 file(s)`** — it read the tree and answered, rather than
printing a zero from a run that read nothing.
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 24, 2026
ThinkFlowLab#931)

Owner: Dev3
Reviewer: Reviewer2

Fixes ThinkFlowLab#928

## Reproduce

On `main` `137cd2fd`, with `review-policy.json` carrying `selfReview:
["Birfy", "hsliuustc0106"]` (what ThinkFlowLab#926 adds), `decide()` is fed the real
comments of ThinkFlowLab#926's head `b80823ef`. The PASS by `hsliuustc0106` reports
`counts: false` with `author association CONTRIBUTOR is not OWNER,
MEMBER or COLLABORATOR, so it is not a conclusion`, and it is the head's
only report.

The cause is ordering, not the list: `ASSOCIATIONS`
(`review-gate.mjs:36`) is checked at `:162`, `selfReview` is read at
`:144` and only consulted at `:178` — after the association gate has
already dropped the report. So adding the login changes nothing, which
is what Reviewer measured on ThinkFlowLab#926.

## What changed

- `.github/scripts/review-gate.mjs` (+5/−1): at the association gate,
`const selfOnly = selfReview.has(login(r)) && login(r) === authorLogin`,
and `if (!ASSOCIATIONS.has(association) && !selfOnly)`. One exemption,
for one semantic: a listed login judging **its own** pull request. Every
other check is untouched — `hold`, stale tip, `Written-by:`, `Ran:`, the
reviewer list, the newest-word rule.
- `.github/scripts/review-gate.test.mjs` (+23): one test, three cases —
listed + `CONTRIBUTOR` + own pull request → `counts: true`; unlisted
`passer-by` + `CONTRIBUTOR` → `counts: false`; listed + `CONTRIBUTOR`
judging someone else's pull request → `counts: false`. The third case is
what keeps the exemption from becoming a second reviewers list.
- `AGENTS.md:150` and `docs/autonomous-dev.md:147`/`:163`: the exception
written next to the rule it changes, dated as the owner decision it
executes.

Not in this PR: ThinkFlowLab#926 itself (a one-line JSON change on someone else's
branch), and no other gate. `selfReview` is still a subset of
`reviewers`; the boolean this adds is narrower than the association
check it excuses.

## What I ran

- `cd .github/scripts && node --test "*.test.mjs"` → **333 pass / 0
fail**.
- `cd .github/scripts && node --test review-gate.test.mjs` → **40 pass /
0 fail** (the new test is ThinkFlowLab#40).
- `node .github/scripts/changelog.mjs check` → 45 fragments well formed
(no fragment here: the gate is not user-visible, same as ThinkFlowLab#912).
- `node core/scripts/check-arch-docs.mjs --base 3c711b3` → `OK (17
changed file(s))`; no `docs/subsystems` page owns `.github/scripts/`,
`AGENTS.md` or `docs/autonomous-dev.md`, and both policy documents are
updated in this branch anyway. (The local base is stale — CI judges the
real base.)
- Before pushing, each of the four files was diffed against its
**current `main`** content fetched through the API (`137cd2fd`): the
only differences are the hunks above, so the branch cannot revert
anything that landed on `main`.

## Red without the source change

- Mutation A — delete `&& !selfOnly` from the guard: `node --test
review-gate.test.mjs` → `not ok 40 - ThinkFlowLab#928: a listed self-reviewer who is
a CONTRIBUTOR counts, only on their own pull request`, **39 pass / 1
fail**.
- Mutation B — widen to `selfReview.has(login(r))`: same test red, **39
pass / 1 fail**.
- Restored file is byte-identical to the branch version (`diff -q`
clean), `40 pass / 0 fail`.
- With `review-gate.mjs` at `main` and the new test kept, case 1
(`mine.ok`) is false, so the test fails — the source change is what
makes it pass, not the fixture.

## Not run

- Full `npm run verify`: this box has no `core/node_modules` / harness
install, so `core` and plugin suites are not run here — CI runs them.
Nothing in this branch touches `core/`, `plugins/` or `client.js`.
- No smoke test: no `index.js` or `core/src/host/` change.
- No live re-run against ThinkFlowLab#926's data after the change (its head is
unchanged and still carries the old conclusion); the fixture in the test
reproduces those comment records instead.

## Risk

- The exemption is keyed on `selfReview` membership **and** `login ===
authorLogin`, so a login added to `selfReview` now also gets the
association exemption on its own pull request. That is exactly the owner
decision recorded in `AGENTS.md`, but it means `selfReview` is no longer
purely "may PASS its own PR" — it also says "may be a `CONTRIBUTOR`
while doing so". Anyone else's pull request is unaffected (pinned by the
third case).
- `merge-gate.mjs` imports this `decide`, so a queue entry is judged by
the same rule; a `CONTRIBUTOR` who is not listed is still refused at the
queue, as before.
- ThinkFlowLab#926 stays open and unchanged by this PR; per the Chief it goes in
together with this one, so the policy change and the code that makes it
mean anything land as a pair.

Docs-not-needed: no `docs/subsystems` page owns `.github/scripts/`,
`AGENTS.md` or `docs/autonomous-dev.md` (checked against their
`owned_paths`), and the rule itself is updated in `AGENTS.md` +
`docs/autonomous-dev.md` in this branch.
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 24, 2026
…unt's Verdict (ThinkFlowLab#944)

Owner: Dev
Reviewer: Reviewer

Fixes ThinkFlowLab#943

Written-by: Dev

## What changed

Both halves of ThinkFlowLab#943, each a rule the pipeline described and nothing
read.

**Part 1 — the two lines, before the queue.** `guard()`
(`.github/scripts/enqueue-guard.mjs`) now refuses a **non-draft**
request on a branch this repository owns when the first 20 lines of its
body carry no line starting `Owner:` or no line starting `Reviewer:`, or
when `Reviewer:` names a login outside the policy's `reviewers`. Each
refusal names the line, not just the defect: `missing line: Owner:`,
`missing line: Reviewer:`, `Reviewer names "x", who is not in
reviewers`. Twenty lines rather than two because an agent body opens
with its own HTML comment — the open ThinkFlowLab#941 carries `Owner:` on line 4 —
and a top-two check would refuse every such pull request. A `Reviewer:`
value is one login, comma-separated, `@` and any case allowed; a
`Reviewer:` with no value names nobody and is refused as the missing
line. A **draft** is not entering the queue and is not refused, and
nothing older is ever re-read: ThinkFlowLab#940's merged record stays exactly as it
merged. The mechanical release PR and the fork keep the exemptions they
already had.

**Part 2 — a bot account writes no conclusion.** `decide()`
(`.github/scripts/review-gate.mjs`) refuses a report authored by a
GitHub bot account — `user.type === 'Bot'`, or a login ending `[bot]` —
before the association test, with its own reason naming the account, so
the log says which defect fired. The report still joins `judged` with
`counts: false`. The rule is the **account type, not a login list**:
every Bot writes as the shared `Birfy` account, which is a `User`, and a
list containing it would void the whole pipeline. What voids such a
report is the real Reviewer's `Correction from Reviewer:` — the shape
the Reviewer already used on ThinkFlowLab#940
([6381646389](ThinkFlowLab#940 (comment))).
`review-gate`, `merge-gate` and `enqueue-guard` all inherit it because
they share `decide()`; the two API readers now carry `user.type`
alongside the association.

Docs: `AGENTS.md` (Merge gates: the queue bullet and `One conclusion per
tip`) and `docs/autonomous-dev.md` (`§ The reviewed tip` rule 6, and the
enqueue-guard sentence) state each rule once.

Not a user-visible change — the gate's own bookkeeping, same precedent
as ThinkFlowLab#912 and ThinkFlowLab#928 — so no `changelog.d/` fragment.

## INC at `87b49961` answered

Reviewer was right, and the line in the first version of this body was
false: "a merged pull request is never re-judged". `enqueue-guard.yml`
judges `issue_comment` too, so **any** sentence on a merged pull request
runs this check again and the two header reasons fired on a body nobody
can edit any more — the live list of merged pull requests without these
lines is ThinkFlowLab#926, ThinkFlowLab#925, ThinkFlowLab#908, ThinkFlowLab#901, ThinkFlowLab#900, ThinkFlowLab#898, ThinkFlowLab#897 (ThinkFlowLab#940 escaped only
because I edited its body afterwards).

The fix is the one Reviewer named: `decidePull` already reads
`pull.merged`, so `guard()` takes it and the body check is `if (!draft
&& !merged)`. The new test pins both halves — `ThinkFlowLab#943 INC: a merged pull
request is not re-judged when a comment arrives`: the same body (no
lines, a PASS at the tip) is green when `merged`, and still refused with
both `missing line:` reasons when it is an open non-draft request.
Mutation: dropping `&& !merged` reds exactly that test.

Checked against the real bodies, through the branch's `guard()`:

```text
ThinkFlowLab#926 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer:
ThinkFlowLab#925 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer:
ThinkFlowLab#908 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer:
ThinkFlowLab#897 merged true | merged=true ok: true | open ok: false | open reasons: missing line: Owner:; missing line: Reviewer:
```

`AGENTS.md` now says "a **merged** pull request is never re-judged: this
check also runs on every comment, and history is not a request", and
`docs/autonomous-dev.md` says the same next to the enqueue-guard
sentence.

## Evidence

Red first, on the pristine `c7286584` with only the new tests added (11
red, 3 controls): `.github/scripts/enqueue-guard.test.mjs` 36/7 — no
`Owner:`, no `Reviewer:`, `Reviewer:` outside `reviewers`, the lines
after the 20th, a `Reviewer:` with no value, a bot-account `Verdict:`,
and the INC case below; `.github/scripts/review-gate.test.mjs` 45/3;
`.github/scripts/merge-gate.test.mjs` 35/1. The three part-1 tests that
stay green there are the controls that catch over-refusal: an
`@`/mixed-case listed login, the two lines at line 4-5 (ThinkFlowLab#941's shape),
and a draft.

Seven mutations, each red on its own test and restored byte-identical:
drop `if (!draft)` → the draft control; `slice(0, 20)` → `slice(0, 2)` →
the line-4 test; drop the membership loop → the unlisted login; drop the
`Owner:` check; drop the `[bot]` half of the account test; allow an
empty `Reviewer:` value; drop the bot check in `decide`.

Gate 4 for part 2 replays ThinkFlowLab#940: `cursor[bot]`'s report at that tip is
`counts: false` with the bot-account reason, while `Birfy`'s report at
the same tip still counts — asserted in `review-gate.test.mjs` and again
through `merge-gate`'s queue entry.

At tip `23f14dbaaba6c283f2acca7259cfa99575f0ef74` (the branch head, no
commit after it):

```text
$ node --test ".github/scripts/*.test.mjs"
# tests 383 / pass 377 / fail 4 / skipped 2
```

The four failures are the `ds-bot-patches` format tests, which shell out
to `git diff`/`git apply`; this box has no `git` (the repo's own note in
`.github/scripts/fixtures/review-reports-660-663.json` says the same
four fail on main without it). Per file: `enqueue-guard` 42/42,
`review-gate` 48/48, `merge-gate` 36/36, `gate-check` 16/16 —
`gate-check`'s fixture pull bodies now carry the two lines, because the
guard it drives reads them. `node .github/scripts/changelog.mjs check` →
54 fragments well formed; `testing-layers.mjs --check` → 350 files in 15
workflows, all classified; `self-test-catalog.mjs --check` → page and
catalog agree; `oxlint` on the six touched JS files → 0 errors (one
pre-existing warning in `gate-check.test.mjs:101`, untouched).

Not run: `npm run verify` end to end (no `node_modules`/harness on this
box, and `arch-docs` needs `git log`); no owned subsystem file changed,
so `arch-docs` has nothing to pair here. CI's `tests-ok` and `package`
cover both.

## Risk

T3 by surface: `.github/scripts/` and the two policy documents. It
changes no product code path, no `core/`, no `patches/`, so the patch
round-trip and the client bundle are untouched; what it can do is refuse
a pull request that used to reach the queue, which is the point of it.
No bypass, no label, no history rewritten.

## Scope

Eight files: the two source scripts, four test files, the two docs. No
`core/`, no `patches/`, no generated file, so the patch round-trip is
untouched. The check is enqueue-time only — it is deliberately not added
to `review-gate`'s or `judge`'s conclusion tests, which read every
report on every pull request and would refuse reports on pull requests
that never had the two lines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agent-audience: a mentioned member may stay silent, so a PR can sit for rounds with nobody owning the verdict

1 participant