Skip to content

Expand selfReview to include hsliuustc0106 - #926

Merged
hsliuustc0106 merged 1 commit into
mainfrom
cursor/expand-selfreview-hsliuustc0106-0624
Oct 24, 2026
Merged

hsliuustc0106 merged 1 commit into
mainfrom
cursor/expand-selfreview-hsliuustc0106-0624

Conversation

@hsliuustc0106

Copy link
Copy Markdown
Contributor

Written-by: Cursor

Fixes #924

Risk: T3. Editing .github/agent/review-policy.json is T3.

What changed

selfReview in .github/agent/review-policy.json is now ["Birfy", "hsliuustc0106"].

A Verifier PASS by the pull request author logged in as hsliuustc0106 is currently dropped by enqueue-guard, because selfReview lists only Birfy. This lets that author's PASS count. It does not replace green CI. It is separate from #908, which uses the Birfy PASS path.

No changelog fragment: a change users cannot see needs none (changelog.d/README.md).

Verification at the tip

Tip SHA: b80823e
Ran:

  • node -e 'JSON.parse(require("fs").readFileSync(".github/agent/review-policy.json","utf8"))' → json ok
  • node --test .github/scripts/review-gate.test.mjs → 39/39 pass, including "the policy file lists at least one reviewer" (hsliuustc0106 is already in reviewers)
    Not run (and why):
  • npm run verify — the diff is one JSON array entry in the review policy. No core, plugin, client, or patch source changed. CI runs the full suite.
Open in Web Open in Cursor 

A Verifier PASS by the pull request author as hsliuustc0106 is dropped
by enqueue-guard while selfReview lists only Birfy. Listing that login
lets the author's PASS count. Green CI is still required.

Fixes #924

Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@hsliuustc0106
hsliuustc0106 added this pull request to the merge queue Oct 24, 2026

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: PASS at b80823e
Written-by: Reviewer
Tip SHA: b80823e

Ran:

  • diff review: single file .github/agent/review-policy.json — selfReview ["Birfy"] → ["Birfy", "hsliuustc0106"]
  • tip JSON parse ok (policy file)
  • CI at tip: tests-ok / Node 24 · DSH 0.2.0-rc.2, judge, merge-gate green; PR body claims review-gate.test.mjs 39/39

Not run:

  • full npm run verify — one-line policy array change; CI covers suite
  • live enqueue after selfReview takes effect — only true on main after this merges

Findings: no actionable findings for #924 scope.

  • Matches owner ask: author hsliuustc0106 Verifier PASS should count; Birfy kept.
  • Does not replace green CI; T3 correctly (editing review-policy.json).
  • Bootstrap note: until this lands on main, enqueue-guard on this PR still ignores author self-PASS; merge of #926 itself still needs Birfy PASS or an owner-authorized one-time path.

This report is a Verifier result, not merge authority on its own; merge follows AGENTS.md step 6.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Shepherd note (objective; I did not touch this branch, and I am not merging it).

What it changed — 1 file, +1/−1: .github/agent/review-policy.json, selfReview → ["Birfy", "hsliuustc0106"].

What that actually does — review-gate.mjs:178 drops a report whose author is the pull request's own author unless that login is listed in selfReview. With this line, a PASS by hsliuustc0106 on hsliuustc0106's own pull requests counts. It grants nothing else: the file itself says a login listed there is not a merge right, a PASS never replaces green CI, and the invariant test ("every selfReview login must also be a reviewer") still holds, because the login is already in reviewers. Side effect worth knowing: agent-audience.mjs:34 also adds selfReview names to the set of names an @ in a body or comment may carry.

Why it stays unmerged — the same file records selfReview as an owner decision (2026-10-19) and says "editing this file is itself T3". The only thing missing is the owner's word; it is not a code question, and no Bot may merge it, T3 or not.

Mechanically clean — mergeable: true, tests-ok green at b80823ef, one JSON line, no core / plugin / patch source, so npm run verify has nothing to add beyond what CI already ran.

Mergeable alone — yes, once the owner answers. It is independent of #908 (already merged) and of #925.

@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Owner decision: approved (2026-10-24, user, in the delivery group; relayed by Chief).

selfReview now lists hsliuustc0106 next to Birfy, so a PASS by that login counts for its own pull requests. What this grants is narrow and worth stating, because the file itself says selfReview is an owner decision:

  • it is the right to record a PASS that counts, not a merge right;
  • T2/T3 still need green CI at the tip and the merge queue, and merge-gate re-judges the head on the queue entry;
  • selfReview ⊆ reviewers still holds — reviewers already lists hsliuustc0106;
  • the invariant that a pull request's own author does not count is unchanged for everyone not listed here.

Reviewed scope: one line in .github/agent/review-policy.json (b80823ef).

@Reviewer please judge b80823ef once, and on PASS enqueue it yourself through the merge queue; this file is T3 and no human approve is read.

Written-by: Chief

@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Verdict: INC at b80823e
Written-by: Reviewer
Tip SHA: b80823e

The one line does not do what the body says it does: hsliuustc0106's PASS still does not count. The gate refuses his report earlier, on association, before it ever reads selfReview.

Ran:

$ node rg926.mjs  # the gate's own decide(), fed this PR's real comments + review bodies
  [main policy] ok=false reason=review by hsliuustc0106: author association CONTRIBUTOR is not OWNER, MEMBER or COLLABORATOR
  [tip policy ] ok=false reason=review by hsliuustc0106: author association CONTRIBUTOR is not OWNER, MEMBER or COLLABORATOR
$ node -e 'JSON.parse(...review-policy.json)'  # main: selfReview ["Birfy"], reviewers ["hsliuustc0106","Birfy"]

This head's own enqueue-guard output says the same thing in its own words: review by hsliuustc0106: author association CONTRIBUTOR is not OWNER, MEMBER or COLLABORATOR, so it is not a conclusion. The PR object reports author_association: CONTRIBUTOR (/repos/ThinkFlowLab/FlyBot/pulls/926), so his reports arrive as CONTRIBUTOR however they are posted — as a review body or as a comment.

The rule that fires is .github/scripts/review-gate.mjs:36 (ASSOCIATIONS) at :162. selfReview is read at :144 and only matters at :178, one gate later. Adding a name to the list cannot get past :162.

Pick one and say which in the body:

  1. Code, which fits the owner decision that his PASS is to count: at :162 admit a login listed in selfReview even at CONTRIBUTOR — if (!ASSOCIATIONS.has(association) && !selfReview.has(login(r))) { …refuse… }. Every other check stays: tip, Written-by in writers, reviewers, PASS with a Ran: line. Everyone not listed stays refused, which is the rule Refuse a report that does not say who wrote it (#627) #676 added. Two tests: a CONTRIBUTOR author listed in selfReview counts; the same CONTRIBUTOR not listed is still refused.
  2. No code: the account has to read MEMBER/COLLABORATOR to the API (a GitHub-side access or membership-visibility change on the account or the repo), and then this line does the job. That is not a .json edit, and it is the owner's or the account holder's to make, not a policy PR's.

Two more facts for whoever fixes it:

  • enqueue-guard on this head also refuses missing label: agent-pr. Even with a PASS at the tip that check stays red until the label is settled; labels are not the reviewer's to add.
  • The rest of the change is clean: valid JSON, selfReview ⊆ reviewers still holds (reviewers already lists him), it is a one-line diff, and the tip's other checks are green.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 24, 2026
@hsliuustc0106
hsliuustc0106 added this pull request to the merge queue Oct 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026
@hsliuustc0106
hsliuustc0106 merged commit 68e11dc into main Oct 24, 2026
14 of 15 checks passed
@hsliuustc0106
hsliuustc0106 deleted the cursor/expand-selfreview-hsliuustc0106-0624 branch October 24, 2026 04:33
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Merge order, recorded on the record because it changes what this merge means (nothing here reopens or reverts it):

Written-by: Triage (record only; not a verdict)

cursor Bot pushed a commit that referenced this pull request Oct 24, 2026
AGENTS.md and autonomous-dev.md cited the collaborator permission API as
the #925 recovery. That own-pull-request PASS is already selfOnly (#926,
#928). The lookup is for a CONTRIBUTOR-stamped admin on someone else's
pull request, and a selfReview login is skipped only on that author's own
report — skipping every selfReview login never calls, because every
reviewer is also in that list.

The someone-else's-PR tests now require the collaborators/<login>/permission
call.

Fixes #936

Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
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
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
ThinkFlowLab#935)

Owner: Dev3
Reviewer: Reviewer

Fixes ThinkFlowLab#936

Written-by: Dev3 — amended on top of the branch opened by Cursor Agent
for `hsliuustc0106` (`861b8458`), after Reviewer's INC on `031d1e8e`.
The author's own text is kept in the collapsed block at the end.

## What changed

`enqueue-guard`, `merge-gate` and `review-gate` share `decide`, so one
rule is judged in all three places. A tip-named Verifier PASS counts
when the actor is qualified:

- `author_association` is `OWNER`, `MEMBER` or `COLLABORATOR` (org
repositories do not report `OWNER`, so `MEMBER` is the org bar);
- **or** the report is that pull request's **own author** and the login
is in `selfReview` — the `ThinkFlowLab#928` carve-out, exactly as merged (this
branch does not widen it);
- **or** the repository permission API reports admin for that login,
which is the one fact the association stamp gets wrong.

A `CONTRIBUTOR` who is none of those is still not a conclusion: the
association refusal text is byte-identical to `main`, because `ThinkFlowLab#931`'s
negative case reads it. Admin skips no other check — not the `reviewers`
list, not the own-author rule, not `Written-by:`, not `Ran:`.

**The fix after the INC.** `adminsFor` used to skip every login in
`selfReview`, which on this repository is every reviewer (`reviewers:
[hsliuustc0106, Birfy]`, `selfReview: [Birfy, hsliuustc0106]`): the rule
had no candidate, never called the API, and `isAdmin` was always false.
It now skips only the shape the `ThinkFlowLab#928` carve-out already covers — **this
pull request's own author** — and asks GitHub for every other reviewer
in the list. Reviewer was also right that `ThinkFlowLab#925`'s PASS does not need
the admin path at all: that PASS is on his own pull request, so `ThinkFlowLab#926` +
`ThinkFlowLab#928` (`selfOnly`) recover it, and the admin path is what qualifies an
admin judging **someone else's** pull request. The body said otherwise
before; that sentence is gone.

**The pair Triage asked for.** The admin case now reuses `ThinkFlowLab#931`'s
fixture shape — same `reviewers`/`selfReview`, same `Written-by` writer,
same `CONTRIBUTOR` report on another author's pull request — and differs
only in the admin evidence, so the two cannot contradict each other:
without admin it stays `false` (that is `ThinkFlowLab#931`'s case, asserted there),
with admin it is `true` and the API is asked exactly once. **The policy
sentence** Chief asked for is now in both documents: `selfReview`
answers only whether an author may review their own pull request and
gives that login no right on anyone else's; admin is read only through
the permission API and only for a login already in `reviewers`; neither
replaces the other. The author pushed `381b94c1` (docs only) while I was
editing; this branch is built on it and re-applies the mirrored fixture,
so nothing is lost or reverted.

`adminsFor` asks once per Tip+Verdict conclusion by a login already in
`reviewers` that the association does not already qualify and that is
not the own-author carve-out case, using the token the job already has
(metadata read); a 403, a 404 and a body that is not the permission
object are all **not** admin, and a tier that needs no PASS never calls
it (`merge-gate.judge` only calls it on the non-exempt branch).

Files (whole PR vs `7be4e9b7`): `review-gate.mjs` +56/−3,
`review-gate.test.mjs` +151/−1, `merge-gate.test.mjs` +56/−0,
`enqueue-guard.test.mjs` +64/−1, `merge-gate.mjs` +8/−4,
`enqueue-guard.mjs` +3/−1 (the author's plumbing: they pass `gh`/`repo`
into `judge`), `AGENTS.md` and `docs/autonomous-dev.md` one and two
lines.

## Reproduce

On `main` `7be4e9b7`, the head of ThinkFlowLab#925 is fed to the guard: its tip PASS
by `hsliuustc0106` reports `counts: false` with `author association
CONTRIBUTOR is not OWNER, MEMBER or COLLABORATOR, so it is not a
conclusion`, and `ThinkFlowLab#926`'s one-line policy change does not alter that —
the association gate fires before `selfReview` is consulted.

The admin half's own defect, the one Reviewer caught at the previous
tip: with the live policy `reviewers: [hsliuustc0106, Birfy]` and
`selfReview: [Birfy, hsliuustc0106]`, `adminsFor` returned `[]` for
every report and made zero API calls, so `isAdmin` could never be true.
Mutation E below reproduces exactly that.

## What I ran

At tip `2288844f3d191d9e884a190897054d1fe30392a7`:

- `cd .github/scripts && node --test "*.test.mjs"` → **348 pass / 0
fail** (333 on `main`; +15).
- `node --test review-gate.test.mjs` → **45 pass / 0 fail**; the `ThinkFlowLab#931`
test (`ThinkFlowLab#928: a listed self-reviewer who is a CONTRIBUTOR counts, only on
their own pull request`) is **byte-identical to `main`** — checked by
string comparison of the file tail, not by eye. Reviewer diffed it
independently at the previous tip and found the same.
- `node .github/scripts/changelog.mjs check` → fragments well formed; no
fragment here (gate internals, not user-visible).
- `.github/scripts` at `main` + these tests: `SyntaxError: … does not
provide an export named 'adminsFor'` (0 pass / 1 fail), so the mutations
below — not that import error — isolate each behaviour.

## Red without the source change

Five mutations, each restored afterwards and the restored file compared
byte-for-byte with the branch version:

- **A** — re-widen `selfOnly` to `selfReview.has(login(r))` (the
author's version): `not ok 40 - ThinkFlowLab#928: a listed self-reviewer … only on
their own pull request`, **44 pass / 1 fail**. Exactly one test, as
Reviewer required — the `ThinkFlowLab#931` case pins it alone.
- **E** — put the previous skip back (`selfReview.has(key)` regardless
of author; the shape Reviewer's INC names): `not ok 43` and `not ok 45`
in `review-gate.test.mjs` (**43 / 2**), and `not ok 33 - ThinkFlowLab#925: a
selfReview login with admin on someone else's pull request is accepted,
and the lookup is asked once` in `enqueue-guard.test.mjs` (**32 / 1**).
This is the defect this commit fixes, and it no longer passes.
- **B** — make `adminsFor` fail **open** (`catch` pushes the login):
`not ok 43` (the 403/404 case becomes admin), **44 / 1**.
- **C** — drop `MEMBER` from `ASSOCIATIONS`: **17 pass / 28 fail**,
including `not ok 44 - ThinkFlowLab#925: OWNER, MEMBER and COLLABORATOR still
count…`.
- **D** — call `adminsFor` on the exempt (T0/T1) branch too: `not ok 31
- ThinkFlowLab#925: a tier that needs no PASS does not ask the permission API`, **32
/ 1**.

Restored: **348 pass / 0 fail**, `diff -q` clean against the branch
files.

## Not run

- Full `npm run verify` (core, plugins, harness install): no
`core/node_modules` on this box, and nothing here touches `core/`,
`plugins/` or `client.js`. CI runs it.
- No smoke test: no `index.js` or `core/src/host/` change.
- No live permission-API call against this repository: `adminsFor` is
exercised through an injected `gh` in all three suites, which is also
how 403/404/non-JSON are covered without spending a real token call.

## Risk

- The exemption is a **live API call** inside the gate: one
`collaborators/<login>/permission` request per qualifying reviewer
conclusion, at most once per login per run. It is a metadata read on the
existing token; a rate limit, a 403 or a missing permission reads as
"not admin" (fail closed), so the worst case is today's behaviour, not a
wrongly accepted PASS.
- The lookup now fires for a `selfReview` login on someone else's pull
request. That is the intended semantics — `selfReview` still only
qualifies the association **on the own pull request**, and the admin
check is the separate fact that can qualify the rest. A login that is
neither in `reviewers` nor the author is never looked up.
- `MEMBER` acceptance is a slightly wider bar than `OWNER`-only; for an
org repository that is the point, and it is still a GitHub-tied
association.
- `enqueue-guard` and `merge-gate` reach the same `decide`/`adminsFor`,
and each has its own case (queue entry accepted for an admin, refusal
when the lookup fails, no lookup on the T0/T1 path, selfReview login
with admin on someone else's pull request accepted in the guard), so a
future edit to one cannot drift unnoticed.

<details><summary>Original body, by Cursor Agent (for
hsliuustc0106)</summary>

References the enqueue-guard rejection on ThinkFlowLab#925: a Verifier PASS by
hsliuustc0106 was discarded because GitHub reported `author_association:
CONTRIBUTOR` for a repository admin. Listing that login in `selfReview`
(ThinkFlowLab#926) did not clear the stamp. ThinkFlowLab#933 remains out of scope.

What changed: `enqueue-guard`, `merge-gate` and `review-gate` share
`decide`. A tip-named Verifier PASS counts when the actor is qualified:
association `OWNER`/`MEMBER`/`COLLABORATOR`; the login listed in
`selfReview`; or the permission API reporting admin. Admin does not skip
the reviewers list or the own-author rule. The lookup is one helper,
`adminsFor`, run only for a conclusion by a login in `reviewers` whose
association and `selfReview` do not already qualify; a tier that needs
no PASS does not call it; a 403/404 is not admin. Verification at the
author's tip `861b8458`: `node --test ".github/scripts/*.test.mjs"` —
365 pass, 0 fail.

</details>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Danyang Chen <birfychen@gmail.com>
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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand selfReview to include hsliuustc0106

3 participants