Skip to content

Fix mock admin @all and in-place compaction in smoke (#917) - #925

Merged
hsliuustc0106 merged 1 commit into
mainfrom
cursor/fix-917-smoke-admin-compact-0356
Oct 24, 2026
Merged

hsliuustc0106 merged 1 commit into
mainfrom
cursor/fix-917-smoke-admin-compact-0356

Conversation

@hsliuustc0106

Copy link
Copy Markdown
Contributor

Fixes #917

These two smoke failures were already red on main (same result at 32f3e7c and on this branch's base). They are not a #903/#908 regression. Harness default and the usage five-field Step 0 work are untouched.

What changed

Admin calls everyone. #894 moved the members line into the group system-prompt section. The mock still looked for Members: … and you (admin). in the turn text, so Chief was never the admin and answered 我觉得可以。 alone. The mock now reads that line from the system prompt (and still from the turn, for older prompts).

Writer condensed in place. The steward did say compact, and the checkpoint was written. DSH's token meter stays anchored on the provider usage from before the span was replaced and only subtracts a 4-characters-per-token estimate of the removed text. The mock reports usage at about 3 characters per token, so after a real checkpoint the anchored total still sat above the check line and the part switched (+3). After a compaction, the part now stays when the heuristic size of the remaining surface plus the tool schemas is under the check line. A surface that is still over it still moves on. Crossing the line in the first place still uses the anchored total.

Verification at the tip

Tip SHA: de7b2e6f0c3a98fd023448a91d6a078eb4db716c

Ran:

  • node --import ./test/harness-resolve.mjs --test test/mock-group-roster.test.js test/session-identity.test.js at the unbroken tip: red. Admin answered 我觉得可以。 instead of @all 大家各说一句。. The compact path returned { kind: 'reject' } while the remaining surface was 8,000 tokens.
  • Same command after the fix: 10 pass.
  • npm test in core/: 1347 pass, 0 fail.
  • npm run lint in core/: oxlint warnings are the pre-existing set; no new warning on the edited lines. arch-docs after the commit, with ARCH_DOCS_BASE=origin/main: OK (11 changed files).
  • node scripts/ds-bot-patches.mjs generate && node scripts/ds-bot-patches.mjs check: patches apply, core/ matches.
  • node .github/scripts/changelog.mjs check: fragments well formed.
  • Mock smoke, no API key: DSH_HARNESS pointed at a checkout-shaped install of @deepseek-ai/dsh@0.2.0-rc.2, SLOT=1, sh core/dev/check.sh --only smoke. check passed in 220 s. admin calls everyone and Writer condensed in place, not switched both ok, and the rest of the smoke was green.

Not run (and why):

  • npm run verify (plugin suites, interop, live-e2e). This change is core host + the mock.
  • Web and desktop shots.
  • A live model / DEEPSEEK_API_KEY path.

Re-run smoke on a Linux host with DSH

From the repo root, with Node 22+ and a DeepSeek Harness checkout at dsh-v0.2.0-rc.2 (or DSH_HARNESS set to one), and pnpm on PATH:

cd core
npm ci && npm run harness
cd ..
CONTEXT_WINDOW=100000 CHECK_RATIO=0.45
# dev/check.sh sets those itself for the smoke step
SLOT=1 sh core/dev/check.sh --only smoke

SLOT picks free ports (smoke uses 31n1 / 88n1). The bench data stays under .dsh-test-check-smoke. No API key: DEEPSEEK_API_KEY=mock-key and DEEPSEEK_BASE_URL point at core/dev/mock-llm.mjs.

DSH follow-up

@deepseek-ai/dsh-token-meter (0.2.0-rc.2) reuses the last provider usage as the pressure baseline and applies a heuristic surface delta (CHARS_PER_TOKEN = 4). Compaction does not report usage of its own, so a denser provider report (this mock, or CJK on a real model) can leave measure().totalTokens above the check line after the span is gone. FlyBot compensates only when it decides whether a checkpoint got the part under the line. The meter's own total, which other readers still see, is unchanged. That anchoring belongs in DSH.

Open in Web Open in Cursor 

The group members line lives in the system prompt (#894), so the mock
no longer saw "you (admin)" on the turn and Chief answered alone. After
a checkpoint, the token meter still prices removed messages at the
provider's density and only subtracts a 4-characters-per-token estimate,
so a part the steward asked to condense still looked past the check line
and switched.

Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@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 did not add Owner: / Reviewer: lines — those are Bot-pipeline fields, and this is a human pull request).

What it changed — 11 files, +317/−10:

  • core/dev/mock-llm.mjs +12/−4: the members/admin line is read from the group's system-prompt section as well as from the turn (Group turns re-send and store the group notice and fixed room rules on every turn #894 moved it there); decide is exported; the server binds only on a direct run;
  • core/src/host/parts.js +26/−3: pressureLeft — after a compaction, the stay/move decision reads the heuristic size of what remains (surface + tool schemas at 4 characters per token) instead of the anchored meter total;
  • new core/test/mock-group-roster.test.js, core/test/session-identity.test.js +44/−2, one line each in docs/subsystems/core-host.md and docs/subsystems/ds-bot-core.md, changelog.d/917.fixed.md;
  • generated: patches/ds-bot/0001-core-fixes.patch +65, 0014-flybot-branding.patch +56, 0016-room-collaboration-smoke.patch +35, files/test/mock-group-roster.test.js.

Scope — the two smoke failures in #917 and nothing else. #903/#908's harness and the usage fields are untouched.

Verified on this machine (no git here, so no npm run verify):

  • at the tip de7b2e6f: node --import ./test/harness-resolve.mjs --test test/mock-group-roster.test.js test/session-identity.test.js → 10 pass / 0 fail;
  • the same two test files on the base b38783df → 2 fail: the compaction case (a checkpoint stays in place when the remaining surface fits and only the usage anchor reads high) is a real behavioural red, and the new mock test cannot load on base at all — importing mock-llm.mjs there starts the server and keeps the process alive, which the tip fixes with the export plus the "only on a direct run" guard. So the admin half's red proof is at module level, not at an assertion; the issue's claim that both were red on main holds, but only the compaction half is red as a value.

Risk to weigh — pressureLeft only lowers the number the post-compaction decision reads, and it prices the remaining surface at 4 characters per token. Real (CJK, or a dense model) text is denser than that, so a part can now stay where today it would move on. The change is gated to baseline.kind === 'usage' && surfaceDeltaTokens < 0, and "a surface still over the line still moves the part on" is covered — but that case is the mock's density, not a dense model's. core/src/host/parts.js is the autonomous-loop part switching, a known area, and this is the only behavioural change in the diff.

Mergeable alone — not as it stands: the base is b38783df, one merge behind (main is 0b22aa53 after #908), and #908 edited patches/ds-bot/0001-core-fixes.patch and docs/subsystems/core-host.md, both of which this pull request also edits. It needs an update-branch or a rebase before anyone judges it; GitHub still answers mergeable: unknown.

@hsliuustc0106
hsliuustc0106 marked this pull request as ready for review October 24, 2026 03:37
@hsliuustc0106 hsliuustc0106 added the agent-pr Pull request opened by an agent label Oct 24, 2026 — with Cursor
@cursor

cursor Bot commented Oct 24, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@hsliuustc0106 hsliuustc0106 added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 24, 2026 — with Cursor

@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 de7b2e6
Written-by: Reviewer
Tip SHA: de7b2e6

Ran:

  • diff review: mock roster from system section (#894/#917); pressureLeft after compaction so dense usage anchors do not force a part switch when remaining surface fits
  • unit coverage: mock-group-roster.test.js, session-identity checkpoint stay/switch cases
  • hk2 (via 开发工程师): SLOT=1 sh core/dev/check.sh --only smoke → 42/42, including admin calls everyone and Writer condensed in place, not switched
  • CI at tip: tests-ok / Node 24 · DSH 0.2.0-rc.2, judge, merge-gate green; labels agent-pr, risk:T3
  • PR body: patches check + core mirrors claimed green

Not run:

  • full npm run verify / live model path — smoke+CI cover the #917 scope; live deferred
  • re-run hk2 personally — accepted 开发工程师 report at tip de7b2e6

Findings: no actionable blockers for #917.

  • Soft: changelog fragment covers compaction stay; admin/@ALL mock fix is only in PR body (optional follow-up fragment).
  • Soft: until #926 lands on main, author self-PASS may still be ignored by enqueue-guard; Birfy or owner path may still be needed to enqueue this tip.

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

@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 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 7457e12 into main Oct 24, 2026
14 of 18 checks passed
@hsliuustc0106
hsliuustc0106 deleted the cursor/fix-917-smoke-admin-compact-0356 branch October 24, 2026 04:34
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
Owner: Dev
Reviewer: Reviewer2

<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
Written-by: Cursor Agent (for hsliuustc0106)

References ThinkFlowLab#874

Step 1 only. This does not close the RFC: Step 2 (the experience
library) is still open, so the body says References rather than Fixes.

## What changed

Lite, Medium, and Heavy load on the Step 0 harness (`resolveHarness`).
Each tier names the same modules as `default` and adds a declared
budget. `applied` is false, so `prompt.js`, `tools.js`, `models.js`, and
`turns.js` still run as they do today. `install` leaves `rt.harness` on
`default`.

Triage selects a tier with exactly one label:

- `harness:lite`
- `harness:medium`
- `harness:heavy`

No such label, or any other label including `risk:T*`, stays on
`default`. Two tier labels, or an unknown `harness:*` label, throw. The
controller does not write these labels. They are in
`.github/labels.json`; creating them on the repository is `node
.github/scripts/sync-labels.mjs --repo ThinkFlowLab/FlyBot --apply`,
which this PR does not run.

Declared budgets (Lite < Medium < Heavy):

- prompt `maxChars`: 8000 / 16000 / 48000
- tools `maxCalls`: 8 / 24 / 64
- tools `allow`: six read tools / those plus work tools, still without
`create_bot`, `delete_group`, and `stop_bot` / `null` (the full set the
host registers)
- model `maxTokens`: 2048 / 8192 / 16384
- step `maxSteps`: 4 / 12 / 32
- step `maxRetries`: 0 / 1 / 1 (one written-tool-call retry is what
`turns.js` does today)

`harnessPro` is the offline check for a tier config: same four modules,
`verifiers.offline` is `ci`, `core/dev/eval`, `rsi-baseline`, and no key
or token field. It does not run those checks and it does not call a
model.

Usage rows already carry issue/PR, tool-call count, and wall time from
Step 0. `rt.noteUsageHarness(sessionId, idOrLabels)` stamps the routed
tier on the `harness` column. The resume cursor stays `{ provider,
model, retry }`.

No changelog fragment: the chat UI is unchanged, and the default path
does not change behavior.

## Verification at the tip

Tip SHA: `ea8dec628ef6da1b555ce9816d9bed41edda157b`

Red on base `68e11dc` (before the implementation), `node --test
core/test/harness.test.js core/test/usage-fields.test.js`: 2 failed.
`harness.test.js` did not load (`HARNESS_PRO_VERIFIERS` missing).
`foldCalls` recorded `harness` `default` when the test passed `lite`.

Ran at this tip:

- `node --import ./test/harness-resolve.mjs --test test/harness.test.js
test/usage-fields.test.js` in `core/` → 16/16 pass
- `npm test` in `core/` → 1364/1364 pass (same tree as this commit, run
immediately before the commit)
- `npm run lint` in `core/` → oxlint warnings only, the same `usage.js`
`mark?.fold ?? {}` warning that already fires on `main`; `TZ=UTC node
scripts/check-arch-docs.mjs` after the commit → `arch-docs: OK (11
changed file(s))`
- `node scripts/ds-bot-patches.mjs check` → pin, 10 patches, `core/`
equals the mirrors
- `node core/dev/eval/run.mjs --list` → `P3-tree-split`,
`P4-invite-approve`, `P6-timeline`, `memory-recall`
- `node --test .github/scripts/rsi-baseline.test.mjs
.github/scripts/sync-labels.test.mjs .github/scripts/controller.test.mjs
.github/scripts/risk-label-docs.test.mjs` → 35/35 pass

Not run:

- `npm run verify` — plugin suites, interop, and live-e2e.
`plugins/*/node_modules` is not installed here.
- Smoke bench. `usage.js` is under `src/host/`, and the bench was left
out with the ThinkFlowLab#925 smoke work.
- Control set of 10, and any live DSH turn. No `DEEPSEEK_API_KEY`.
- `rsi-baseline.mjs` against GitHub. That needs a token. The
deterministic test above does not.
- `sync-labels.mjs --apply`. The labels are defined, not created on the
repository.
- Merge. This pull request stays a draft.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-4ac91855-557d-53da-9d53-32b6df19ca9f?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-4ac91855-557d-53da-9d53-32b6df19ca9f&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>

Fixes ThinkFlowLab#937

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

risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

smoke: mock admin @all + Writer compact-in-place both red on main

3 participants