diff --git a/AGENTS.md b/AGENTS.md index 24ebe670..4e3c5c3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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|#@@@@`. 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 --verdict PASS --workstation --head `; 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 @@ -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=`. 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=`. 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 #` / `Closes #`) 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. @@ -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 `` in `gh-readonly-queue/main/pr--` 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 ` / `Verdict: INC at ` / `Verdict: REQUEST_CHANGES at `, 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 `` in `gh-readonly-queue/main/pr--` 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 ` / `Verdict: INC at ` / `Verdict: REQUEST_CHANGES at `, 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. diff --git a/docs/autonomous-dev.md b/docs/autonomous-dev.md index 1b1ece6f..a0f7f051 100644 --- a/docs/autonomous-dev.md +++ b/docs/autonomous-dev.md @@ -149,7 +149,7 @@ What counts as a reviewed tip, all of it: `Attestation: <40-character sha> role=`. The sha is the whole tip. `role` is the `Written-by` name and is not an `Owner:` of the pull request. A `Verdict:` with no such line, a line after the blank, a short sha, or a role that is the Owner is the author's own GitHub identity and does not count. A conclusion from a different - GitHub login does not carry the line. `decide` parses it; a signature is the later slice of #644. The + GitHub login does not carry the line. `decide` parses it; a signature was the later slice of #644 and is shipped but not enabled (#1110, #1111). The reviewed-tip job, `enqueue-guard` and `merge-gate` call that one function. 3. The conclusion format is fixed (#637): the **first line** is `Verdict: PASS at ` (or `INC` / `REQUEST_CHANGES`), the second is the attribution (`Written-by: Reviewer`; a relay adds `Posted-by: Chief (proxy…)` on the third line, and the older prose `Written by: …` reads the same way), then the evidence — `Ran:` (or a `Ran …:` heading over a fenced block of `$ ` commands, #660–#663) with at least one command; `Not run:` naming any gap is recommended but not checked (owner decision 2026-10-22 09:57 Asia/Shanghai). A body whose first line is not the verdict is no conclusion. The comment's `author_association` must be `OWNER`, `MEMBER` or `COLLABORATOR`, or the report must be that pull request's own author listed in `selfReview` (#928), or the login must be shown as a repo admin by the permission API. Org repositories do not report `OWNER`. A `CONTRIBUTOR` who is none of those counts for nothing, so they can neither pass a tip nor hold one. An association the API did not send is not assumed, and a failed permission lookup is not admin. The permission API is for an admin GitHub did not tie to the repository: a `CONTRIBUTOR` stamp on someone else's pull request, asked only for a Tip+Verdict conclusion by a login in `reviewers`. A `selfReview` login is skipped only on that author's own report, which is the #928 carve-out and needs no lookup — skipping every `selfReview` login would never call, because the shipped policy lists every reviewer there. That lookup does not recover the own-pull-request PASS; #926 and #928 already do. 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`. Every other check is unchanged. 4. The sha on that first line **is the head being judged**. A later commit moves the tip and the old PASS stops @@ -162,19 +162,33 @@ What counts as a reviewed tip, all of it: list: every Bot writes as the shared `Birfy` account, so a list containing it would void the whole pipeline. `cursor[bot]`'s report at #940's tip is the case (`Correction from Reviewer`, [comment `6381646389`](https://github.com/ThinkFlowLab/FlyBot/pull/940#issuecomment-6381646389)). -7. **The report is signed by the workstation it names** (#1110): `Signed-by:` + `Nonce:` + `Signature:`, - verified against `.github/agent/signers.json` for the full head being judged. A `Verdict:` that does not - verify is not a conclusion and stays out of the newest-wins scan — see [Signed conclusions](#signed-conclusions-1110) - below for the canonical string, the configuration errors and the cost. +7. **The report can be signed by the workstation it names, and today it is not** (#1110, #1111): + `Signed-by:` + `Nonce:` + `Signature:` against `.github/agent/signers.json` for the full head being judged + is built and switched **off** — the policy has no `signers` roster and the signers file does not exist, so no + conclusion is asked for those lines and none is dropped as `unverified:`. See [Signed conclusions](#signed-conclusions-1110) + below: it documents dormant code, not what the gates do today. #### Signed conclusions (#1110) -One shared GitHub identity makes `Written-by:` a claim, so since #1110 a conclusion also has to be -**signed by the workstation it names**. Each reviewing workstation holds one -Ed25519 private key in its own secret (`DSH_SECRET_VERDICT_KEY_`); the public keys are in -[`.github/agent/signers.json`](../.github/agent/signers.json), and the roster of workstations that must -have one is the `signers` list in [`review-policy.json`](../.github/agent/review-policy.json). A report -adds three lines under its attribution: +One shared GitHub identity makes `Written-by:` a claim, and #1110 built the check that would prove it: a +conclusion **signed by the workstation it names**. That check is in `main` and **not enabled** — no +conclusion is signed today. Everything below describes the mechanism as written, so that the day the switch +is turned on the behaviour is already documented; it is not what the gates do now. + +The switch is a pair: the roster of workstations that must sign is the `signers` list in +[`review-policy.json`](../.github/agent/review-policy.json), and the public keys live in +[`.github/agent/signers.json`](../.github/agent/signers.json). Today `review-policy.json` has no `signers` +key at all — the code reads a missing or non-array value as an empty roster — and the signers file does not +exist, so the verifier is never asked for a key 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 +pending step. Three cheap gates stand in its place (a conclusion must name the **current** tip, one +conclusion per tip, and `Attestation:` as a self-report), with one accepted residual risk: one shared +`Birfy` identity still allows a forged `Verdict: PASS` from inside, and those gates catch stale-sha and +replay accidents, not deliberate forgery. + +When it is switched on, each reviewing workstation holds one Ed25519 private key in its own secret +(`DSH_SECRET_VERDICT_KEY_`) and a report adds three lines under its attribution: ```text Signed-by: Reviewer @@ -193,16 +207,17 @@ lifted onto a tip its author did not see. It is fail-closed and one cut. When the policy lists signing workstations, a `Verdict:` whose signature does not verify **is not a conclusion at all**: it does not pass, it does not join the newest-wins scan, and -it does not refuse an older PASS — the summary lists it as `unverified:`. There is no unsigned fallback, so -the honest cost is that an unsigned INC no longer holds a merge either; the keys therefore land before the -gate does ([#1111](https://github.com/ThinkFlowLab/FlyBot/issues/1111)). A roster with a missing or unreadable +it does not refuse an older PASS — the summary lists it as `unverified:`. There is no unsigned fallback in +that mode, so the honest cost is that an unsigned INC no longer holds a merge either — which is exactly why +the roster is still absent and was declined on 2026-10-25 ([#1111](https://github.com/ThinkFlowLab/FlyBot/issues/1111), +`not planned`), and why the gate is inert rather than half-on. A roster with a missing or unreadable signers file, or a workstation whose key is missing or does not parse, is a **configuration error** and refuses every pull request: half of that pair is never read as "unsigned is fine". The signature is not a permission — the association, `writers`, `reviewers` and bot-account checks run exactly as before, and the conclusion never reached the reviewed-tip job at all: the signed lines sit in the header, where the author attestation (#644) also lives, so a body can carry both and the two refusals stay separate. -A workstation signs with its own key, which never leaves its secret card: +When it is enabled, a workstation signs with its own key, which never leaves its secret card: ```sh node .github/scripts/sign-verdict.mjs --repo ThinkFlowLab/FlyBot --pr 1132 --verdict PASS \ @@ -213,13 +228,13 @@ node .github/scripts/sign-verdict.mjs --repo ThinkFlowLab/FlyBot --pr 1132 --ver line, and a conclusion puts it on the second, under the fixed verdict line. `decide` refuses a report with no such line and, separately, one whose name is not in the policy's `writers`; Triage reports a write without one instead of editing it. The rule itself lives in AGENTS.md — one shared GitHub identity -makes the line a self-report, so the gate guarantees presence, not truth (#633). Since #1110 the signature -is what closes that gap: the line still claims, the key proves. +makes the line a self-report, so the gate guarantees presence, not truth (#633). The signature above would +close that gap — the line still claims, the key would prove — and it is not enabled (#1111 `not planned`). | File | Purpose | | --- | --- | -| `.github/agent/review-policy.json` | the logins whose PASS counts, `selfReview`, the authors who may PASS their own PR, and `writers`, the Bot names a `Written-by:` line may carry (#627) — a roster, not a permission: `reviewers`/`selfReview` are the permission lists (#962); editing it is T3. `signers` lists the workstations that must have a public key (#1110) | -| `.github/agent/signers.json` | one Ed25519 public key per signing workstation, base64url DER SPKI from `openssl pkey -pubout -outform DER` (#1110); private keys never enter the repository | +| `.github/agent/review-policy.json` | the logins whose PASS counts, `selfReview`, the authors who may PASS their own PR, and `writers`, the Bot names a `Written-by:` line may carry (#627) — a roster, not a permission: `reviewers`/`selfReview` are the permission lists (#962); editing it is T3. `signers` lists the workstations that must have a public key (#1110); there is no such key in the file, which is what keeps signing off (#1111) | +| `.github/agent/signers.json` | one Ed25519 public key per signing workstation, base64url DER SPKI from `openssl pkey -pubout -outform DER` (#1110); private keys never enter the repository. The file does not exist today: creating it together with a non-empty `signers` roster is what turns signing on (#1111) | | `.github/scripts/review-gate.mjs` | the rule above, for one pull request at a time | | `.github/scripts/verdict-signature.mjs` | the canonical string, the signature lines, `loadSigners` and `verifyReport` — one implementation for the gates, the signer and the tests (#1110) | | `.github/scripts/sign-verdict.mjs` | what a workstation runs to sign its own conclusion; reads its own secret, never prints a key (#1110) |