Repository navigation
Add Lite, Medium, and Heavy harness tiers - #933
Merged
Merged
Conversation
Step 1 of #874 loads three budgets on the Step 0 harness and routes one triage label (harness:lite, harness:medium, harness:heavy). The process harness stays default. HarnessPro checks a tier offline against CI, core/dev/eval, and rsi-baseline. References #874 Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
hsliuustc0106
marked this pull request as ready for review
October 24, 2026 05:52
This was referenced Oct 24, 2026
Collaborator
|
Verdict: PASS at ea8dec6 Ran:
The four points I was asked to watch:
Non-blocking:
|
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owner: Dev
Reviewer: Reviewer2
Written-by: Cursor Agent (for hsliuustc0106)
References #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 asdefaultand adds a declared budget.appliedis false, soprompt.js,tools.js,models.js, andturns.jsstill run as they do today.installleavesrt.harnessondefault.Triage selects a tier with exactly one label:
harness:liteharness:mediumharness:heavyNo such label, or any other label including
risk:T*, stays ondefault. Two tier labels, or an unknownharness:*label, throw. The controller does not write these labels. They are in.github/labels.json; creating them on the repository isnode .github/scripts/sync-labels.mjs --repo ThinkFlowLab/FlyBot --apply, which this PR does not run.Declared budgets (Lite < Medium < Heavy):
maxChars: 8000 / 16000 / 48000maxCalls: 8 / 24 / 64allow: six read tools / those plus work tools, still withoutcreate_bot,delete_group, andstop_bot/null(the full set the host registers)maxTokens: 2048 / 8192 / 16384maxSteps: 4 / 12 / 32maxRetries: 0 / 1 / 1 (one written-tool-call retry is whatturns.jsdoes today)harnessProis the offline check for a tier config: same four modules,verifiers.offlineisci,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 theharnesscolumn. 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:
ea8dec628ef6da1b555ce9816d9bed41edda157bRed on base
68e11dc(before the implementation),node --test core/test/harness.test.js core/test/usage-fields.test.js: 2 failed.harness.test.jsdid not load (HARNESS_PRO_VERIFIERSmissing).foldCallsrecordedharnessdefaultwhen the test passedlite.Ran at this tip:
node --import ./test/harness-resolve.mjs --test test/harness.test.js test/usage-fields.test.jsincore/→ 16/16 passnpm testincore/→ 1364/1364 pass (same tree as this commit, run immediately before the commit)npm run lintincore/→ oxlint warnings only, the sameusage.jsmark?.fold ?? {}warning that already fires onmain;TZ=UTC node scripts/check-arch-docs.mjsafter the commit →arch-docs: OK (11 changed file(s))node scripts/ds-bot-patches.mjs check→ pin, 10 patches,core/equals the mirrorsnode core/dev/eval/run.mjs --list→P3-tree-split,P4-invite-approve,P6-timeline,memory-recallnode --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 passNot run:
npm run verify— plugin suites, interop, and live-e2e.plugins/*/node_modulesis not installed here.usage.jsis undersrc/host/, and the bench was left out with the Fix mock admin @all and in-place compaction in smoke (#917) #925 smoke work.DEEPSEEK_API_KEY.rsi-baseline.mjsagainst GitHub. That needs a token. The deterministic test above does not.sync-labels.mjs --apply. The labels are defined, not created on the repository.Fixes #937