Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ The gate refuses what it can check, and fails closed. [`review-gate.mjs`](.githu

The gate reads a conclusion only from someone tied to this repository (#676): `author_association` `OWNER`, `MEMBER` or `COLLABORATOR` (org repositories do not report `OWNER`), the pull request's own author when that login is in `selfReview` (#928), or a login the permission API reports as admin. The permission API is for an admin GitHub did not tie to the repository — a `CONTRIBUTOR` stamp on someone else's pull request — not for the own-author carve-out. That own-pull-request PASS is already `selfOnly` (#926, #928); the lookup does not recover it. A `CONTRIBUTOR` who is none of those is not a conclusion, so a passer-by's `Verdict:` can neither pass nor hold a merge. A missing association is not assumed, and a failed permission lookup is not admin. The lookup uses the token the job already has, and only for a Tip+Verdict conclusion by a login in `reviewers` that is not already `OWNER`, `MEMBER` or `COLLABORATOR`. A `selfReview` login is skipped only on that author's own report, because the shipped policy lists every reviewer in `selfReview` and skipping the whole list would never call. Admin does not skip the reviewers list or the own-author rule. The two qualifications do not replace each other: `selfReview` answers only whether an author may review their own pull request and gives that login no right on anyone else’s, while admin is read only through the permission API and only for a login already in `reviewers`.

What this buys is presence and a name from a closed list, not truth: with one shared identity the line is a self-report that can be left out (refused) and can name the wrong Bot. **Since #1110 the signature is what closes that gap:** a conclusion also carries `Signed-by:` / `Nonce:` / `Signature:`, one Ed25519 key per reviewing workstation, the public keys in [`.github/agent/signers.json`](.github/agent/signers.json), verified offline by the gate over `FBV1|<repo>#<pr>@<VERDICT>@<full 40-char head>@<workstation>@<nonce>`. A `Verdict:` whose signature does not verify is **not a conclusion** — it cannot pass, it does not vote in the newest-wins scan, and it does not hold a merge; the summary lists it as `unverified:`. There is no unsigned fallback, and a policy roster without a usable key for every workstation is a configuration error that refuses everything. A workstation signs with its own key, which never leaves its secret card: `node .github/scripts/sign-verdict.mjs --pr <n> --verdict PASS --workstation <name> --head <full sha>`; the rules are in [docs/autonomous-dev.md → Signed conclusions](docs/autonomous-dev.md#signed-conclusions-1110). The write layer knowing the writer is the DSH-side half, #633.
What this buys is presence and a name from a closed list, not truth: with one shared identity the line is a self-report that can be left out (refused) and can name the wrong Bot. **The signature that would close that gap is shipped and not enabled (#1110, #1111).** The verifier, the `Signed-by:` / `Nonce:` / `Signature:` lines and [`sign-verdict.mjs`](.github/scripts/sign-verdict.mjs) are in `main`, but its switch is the `signers` list in [`.github/agent/review-policy.json`](.github/agent/review-policy.json), which holds no workstation at all, and [`.github/agent/signers.json`](.github/agent/signers.json) does not exist — so no conclusion is asked to carry those lines today and an unsigned `Verdict:` is judged exactly as it was before #1110. The owner decided on 2026-10-25 not to introduce per-workstation keys ([#1111](https://github.com/ThinkFlowLab/FlyBot/issues/1111), closed `not planned`): a decision, not a step still to come. Three cheap gates stand in its place — a conclusion must name the **current** tip (a moved tip voids it), one conclusion per tip, and `Attestation:` as a self-report. The residual risk is stated, not solved: one shared `Birfy` identity still allows a forged `Verdict: PASS` from inside, and those three gates catch stale-sha and replay accidents, not deliberate forgery. Turning the signature on later takes two configuration lines and one key per workstation; the rules for that day are in [docs/autonomous-dev.md → Signed conclusions](docs/autonomous-dev.md#signed-conclusions-1110). The write layer knowing the writer is the DSH-side half, #633.

### Gates

Expand All @@ -165,7 +165,7 @@ What this buys is presence and a name from a closed list, not truth: with one sh

- **T2 and T3 need no maintainer approve** (user decision 2026-10-22 07:15Z). A human `APPROVED` review is not one of the gates and nothing reads one; the tier decides how closely a change is reviewed, not who signs it off.
- **No implementer writes the Verdict.** On the API side self-review cannot be told apart mechanically — every pull request is opened by the shared `Birfy` account, and `selfReview: ["Birfy"]` in [`review-policy.json`](.github/agent/review-policy.json) makes a PASS always look like Birfy judging Birfy — so what the team holds is the role split: the implementer Bot does not write the Verdict, the Bot that writes the Verdict neither changes the tip nor re-judges its own verdict, and `selfReview` stays as it is because clearing it would leave every pull request without a conclusion.
- **An author-login comment is not a review (#644, option B, first slice).** When the conclusion's GitHub login is the pull request author, `selfReview` is not enough. The header — the lines before the first blank, and within the first eight non-empty lines — carries `Attestation: <40-character sha> role=<Bot>`. The sha is the whole tip (#1112). `role` is the `Written-by` name. That role is not an `Owner:` of the pull request. A free-form comment, a `Verdict:` with no attestation line, a line after the blank, a short sha, a role that is not the `Written-by` name, or a role that is the Owner refuses. A conclusion from a different GitHub login is unchanged. `decide` in [`review-gate.mjs`](.github/scripts/review-gate.mjs) is the check, and the reviewed-tip job, `enqueue-guard` and `merge-gate` all call it. The line is parsed, not signed; a signature is the later slice of #644. T0/T1 still need no PASS.
- **An author-login comment is not a review (#644, option B, first slice).** When the conclusion's GitHub login is the pull request author, `selfReview` is not enough. The header — the lines before the first blank, and within the first eight non-empty lines — carries `Attestation: <40-character sha> role=<Bot>`. The sha is the whole tip (#1112). `role` is the `Written-by` name. That role is not an `Owner:` of the pull request. A free-form comment, a `Verdict:` with no attestation line, a line after the blank, a short sha, a role that is not the `Written-by` name, or a role that is the Owner refuses. A conclusion from a different GitHub login is unchanged. `decide` in [`review-gate.mjs`](.github/scripts/review-gate.mjs) is the check, and the reviewed-tip job, `enqueue-guard` and `merge-gate` all call it. The line is parsed, not signed. A signature was carved out later (#644) and is shipped but not enabled (#1110, #1111): the `signers` roster is empty, so no conclusion is signed today. T0/T1 still need no PASS.
- **`hold` is human-only.** Only a person sets the label and only a person lifts it; automation refuses a held pull request and never removes the label itself. That is the one human input left on the merge path.
- **The label awaiting-approval is retired.** A PASS at the tip goes straight to the queue through the enqueue guard (#687); no pull request waits for an approval click and the label is not set again.
- **A pull request enters the queue with its issue and its `agent-pr` label.** On a branch this repository owns, the enqueue guard also refuses one whose body names no issue (`Fixes #<n>` / `Closes #<n>`) and one without the `agent-pr` label (#718), so "one issue, one pull request" is checked rather than remembered. It refuses a **live, non-draft** request whose body carries no `Owner:` line or no `Reviewer:` line in its first 20 lines, or a `Reviewer:` that is neither a `reviewers` login nor a `writers` Bot name, or that names nobody who can record a PASS — a Bot name counts only through the one identity every Bot writes as, `Birfy`, so it names nobody while that account is outside `reviewers` (#947); a `Reviewer:` that names the same Bot as the `Owner:` is refused too, compared name by name after commas, a leading `@` and case are normalised, because one Bot owning and reviewing the same pull request is the shape #940 had with no lines at all (#951); both lines also need a value the policy knows — a `Reviewer:` outside both vocabularies and an `Owner:` outside `writers` are refused, and a line with no value is the missing line, so an empty `Owner:` cannot clear the comparison (#951); the refusal names the missing line, or the word and the list it missed (#943, #947, #951; #940 merged in about seven minutes with the largest diff of the day and neither line). A **merged** pull request is never re-judged: this check also runs on every comment, and history is not a request. Two exemptions: a mechanical release PR, which carries neither by design, and a **fork** (`pull.head.repo.full_name` is not this repository), whose head we cannot push to and where the contributor keeps their own workflow.
Expand All @@ -181,7 +181,7 @@ Four rules decide what a queue entry must pass. The mechanical judge runs where
1. **`hold` blocks mechanically.** A pull request labelled `hold` fails `merge-gate` on its queue entry. The label is only a marker; the job is the gate, and it is a required check of the `main merge queue` ruleset (#639, set 2026-10-22), so the refusal holds the entry.
2. **Labelling a pull request `hold` means adding a record.** Add the label **and** post a new comment saying **Record only — do not merge / 仅记录、禁止入队**. The pair is what holds it. Add that comment; never edit or delete an earlier PASS report to change what it said — the history is the record, and rewriting it is the thing that reads as an incident afterwards.
3. **An authorisation is not a queue permit.** The owner or the Chief saying "let it through", "claim it" or "handle it along with the rest" licenses no specific pull request for the queue. Entering the queue is an explicit action on that one pull request, after its own gate passed, while it carries no `hold`. #566 is the case this exists for: "handle it along with the rest" was read as a permit, the tip was PASSed at 09:48:18Z, enqueued at 09:48:28Z and merged at 09:50:40Z as `abaf938b`.
4. **The newest conclusion at the head being merged decides.** Read the pull request's comments and review bodies, live, for the exact head SHA being merged — the pull request's own head, on a queue entry too (the `<sha>` in `gh-readonly-queue/main/pr-<N>-<sha>` is the entry's base, not its head; #654) — and take the newest text that carries `Verdict:` or a `## Review —` heading: INC or REQUEST_CHANGES refuses, PASS allows, no conclusion refuses too, and so does a report with no `Written-by:` line or one that names a writer the policy does not list (#627). A `Verdict:` whose signature does not verify is not a conclusion at all (#1110): it is skipped before that scan rather than counted as a refusal, so an unsigned INC no longer holds a merge — the reason the keys land before the gate does. Since #637 the format is fixed: the **first line** of the conclusion is exactly `Verdict: PASS at <sha>` / `Verdict: INC at <sha>` / `Verdict: REQUEST_CHANGES at <sha>`, with the attribution on the second line (`Written-by: Reviewer`, and a relay adds `Posted-by: Chief (proxy…)`; the older prose `Written by: …` reads the same way) and the evidence below it — a body that carries a verdict anywhere else is no conclusion, and no conclusion refuses. A result from `pull_request` time never stands in — on #610 the tip report ran at 10:31:08Z and the INC landed at 11:24:37Z, 54 minutes later. The gating agent checks this before enqueueing; `merge-gate.yml` checks it again at `merge_group`, and the stricter answer wins. That job runs on two events: at `merge_group` it judges and a refusal fails it, and at `pull_request` it only reports what the queue entry would refuse — a required check that never reports on the pull request head leaves every pull request waiting for a status that never comes and unable to enter the queue at all (#639). The required status check is the job name `merge-gate`; the workflow name `Merge gate` is not a context any run produces.
4. **The newest conclusion at the head being merged decides.** Read the pull request's comments and review bodies, live, for the exact head SHA being merged — the pull request's own head, on a queue entry too (the `<sha>` in `gh-readonly-queue/main/pr-<N>-<sha>` is the entry's base, not its head; #654) — and take the newest text that carries `Verdict:` or a `## Review —` heading: INC or REQUEST_CHANGES refuses, PASS allows, no conclusion refuses too, and so does a report with no `Written-by:` line or one that names a writer the policy does not list (#627). Signing is shipped but not enabled (#1110, #1111): while the `signers` roster is empty nothing asks a conclusion for a signature, and an unsigned INC holds a merge exactly as before. Were the roster filled, a `Verdict:` whose signature does not verify would not be a conclusion at all — skipped before that scan rather than counted as a refusal — so an unsigned INC would no longer hold a merge; that cost is why the roster is empty and why the owner declined keys on 2026-10-25. Since #637 the format is fixed: the **first line** of the conclusion is exactly `Verdict: PASS at <sha>` / `Verdict: INC at <sha>` / `Verdict: REQUEST_CHANGES at <sha>`, with the attribution on the second line (`Written-by: Reviewer`, and a relay adds `Posted-by: Chief (proxy…)`; the older prose `Written by: …` reads the same way) and the evidence below it — a body that carries a verdict anywhere else is no conclusion, and no conclusion refuses. A result from `pull_request` time never stands in — on #610 the tip report ran at 10:31:08Z and the INC landed at 11:24:37Z, 54 minutes later. The gating agent checks this before enqueueing; `merge-gate.yml` checks it again at `merge_group`, and the stricter answer wins. That job runs on two events: at `merge_group` it judges and a refusal fails it, and at `pull_request` it only reports what the queue entry would refuse — a required check that never reports on the pull request head leaves every pull request waiting for a status that never comes and unable to enter the queue at all (#639). The required status check is the job name `merge-gate`; the workflow name `Merge gate` is not a context any run produces.

A review's `state` is not read at all: approvals are not a gate any more (user decision 2026-10-22), and the account cannot approve its own pull requests, so its reviews stay `COMMENTED` even for an INC — the INC on #610 was a comment rather than a review.

Expand Down
Loading
Loading