You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit d04712b
Browse filesBrowse the repository at this point in the historyBrowse files
fix(gate): refuse the shared write identity on the Owner: and Reviewer: lines (#1228) (#1229)
Owner: Dev2
Reviewer: Reviewer
Written-by: Dev2
Fixes#1228
Tip SHA: eeb3176
## Reproduce
On `main` (`58bd5575`), `headerReasons()` in
`.github/scripts/enqueue-rules.mjs` reads `Owner:` against `writers` and
`Reviewer:` against `reviewers`/`writers`. `Birfy` is in both lists — it
is the one account every Bot writes as — so both lines accepted it and
named no workstation. #1214 merged with `Reviewer: Birfy` in the body
the queue entry read.
Before this change the guard answered `ok: true` for `Owner: Dev3 /
Reviewer: Birfy` and for `Owner: Birfy / Reviewer: Reviewer`.
## What changed
`.github/scripts/enqueue-rules.mjs`: `headerReasons()` refuses the
shared identity on either line **before** either vocabulary is
consulted, with two sentences in the voice of `missing line: Owner:`:
- `Owner names "birfy", who is the shared write identity every Bot
writes as — name the Bot that claims this pull request`
- `Reviewer names "birfy", who is the shared write identity every Bot
writes as — name the Bot that reviews`
Nothing else changes: a `writers` Bot name on `Reviewer:` still passes
while `Birfy` is in `reviewers`, an independent login still passes,
`Reviewer:` naming the same Bot as `Owner:` is still refused (#951), and
a name outside both vocabularies still gets both sentences. One
implementation: `guard()` and `merge-gate` read the same function (#812,
#960). The checkpoint needs no new field — an `Owner:` that names nobody
is already refused, so `claim: "Birfy"` cannot agree with an accepted
owner.
`review-policy.json` is untouched: who may record a PASS is a different
question and editing it is T3.
`AGENTS.md` (enqueue-guard clause) and `docs/autonomous-dev.md` (where
the vocabularies are listed) say the same thing in one clause each.
## What I ran
```
node --test .github/scripts/enqueue-guard.test.mjs
# before the guard change: 1..85, # pass 79, # fail 6 (the six #1228 cases)
# after: 1..85, # pass 85, # fail 0
node --test .github/scripts/enqueue-guard.test.mjs .github/scripts/merge-gate.test.mjs
# 1..129, # pass 129, # fail 0
node --test ".github/scripts/*.test.mjs"
# 1..495, # pass 493, # fail 0 (2 skipped by their own tests)
node .github/scripts/check-mojibake.mjs --check
# ok — 0 occurrences of U+FFFD in 1066 text file(s)
node .github/scripts/changelog.mjs check
# 2 fragment(s) in changelog.d/ are well formed
```
## Red without the source change
The six `#1228:` cases, run against `headerReasons()` without the two
refusals:
```
not ok 61 - #1228: Reviewer: Birfy names no reviewer, and the refusal names the line and the value
not ok 62 - #1228: Owner: Birfy claims no owner, and the refusal names the line and the value
not ok 64 - #1228: @ and case are normalised first, so the shared identity is refused once, not twice
not ok 65 - #1228: one unusable name in a list is enough
not ok 66 - #1228: the shared identity on both lines is refused on both, and the overlap refusal is unchanged
not ok 67 - #1228: the checkpoint cannot become the way in — claim "Birfy" under Owner: Birfy is refused
# tests 85 / # pass 79 / # fail 6
```
The controls stay green in the same run: `#1228: two real Bots on the
two lines are still accepted`, `#951: a name that is both a login and a
writers name`, and the `#943`/`#947` shapes.
## Not run (and why)
- `npm run verify` / `node --test ".github/scripts/*.test.mjs"` is all I
could run here: this machine has no `git`, and `arch-docs` (`git log
<base>..HEAD`) and the `core` suite (which installs DSH under
`core/dev/harness`) need it. The change touches no `core/` file, so the
`core` suite is CI's first run of it at this tip.
- `actionlint` and the pack check of the `package` job: not run here;
they are part of CI.
## Risk
- A body that names `Birfy` on either line is refused until it names a
Bot: #1226 is open with `Reviewer: Birfy` and will be refused at enqueue
entry until its owner corrects that line. #1214 already merged and is
not re-judged (`merged`), so history stays as it is.
- `merge-gate` shares `headerReasons()`, so a queue entry whose body
names the shared identity now fails there too — that is the same rule,
not a second one.
- No loosening: the refusal only adds reasons; nothing that was refused
before is accepted now.
Docs-not-needed: `.github/scripts/` is not in any subsystem page's
`owned_paths`; the two prose places that list the vocabularies
(`AGENTS.md`, `docs/autonomous-dev.md`) are updated in this diff.
## Checkpoint
<!-- flybot:checkpoint -->
```json
{
"pr": 1229,
"claim": "Dev2",
"issue": 1228,
"summary": "Both header lines could name the shared write identity, which is the one value the two policy vocabularies both contain, so a body could name no Bot and still pass the queue entry. The guard now refuses it on either line before either vocabulary is consulted, with one sentence per line that echoes the missing-line refusal. Nothing else about the lines changes: real Bots, independent logins and the owner/reviewer overlap refusal behave as before. The checkpoint gains no field, because an Owner line that names nobody is refused already. The policy file is untouched.",
"files": [".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.test.mjs", ".github/scripts/merge-gate.test.mjs", "AGENTS.md", "docs/autonomous-dev.md"],
"checks": { "ran": "node --test .github/scripts/enqueue-guard.test.mjs .github/scripts/merge-gate.test.mjs (129/129); node --test '.github/scripts/*.test.mjs' (493 pass, 0 fail); mojibake; changelog", "result": "green", "at": "eeb31767ff918d5085587c1e767c3b37f0be59d8" },
"next": "none — ready for review"
}
```
<!-- /flybot:checkpoint -->
if(!bots.has(name))reasons.push(`Owner names "${name}", who is not a writers Bot name`)
82
+
if(name===SHARED_WRITE_NAME)reasons.push(`Owner names "${name}", who is the shared write identity every Bot writes as — name the Bot that claims this pull request`)
83
+
elseif(!bots.has(name))reasons.push(`Owner names "${name}", who is not a writers Bot name`)
Copy file name to clipboardExpand all lines: AGENTS.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -175,7 +175,7 @@ What this buys is presence and a name from a closed list, not truth: with one sh
175
175
- **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.
176
176
-**`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.
177
177
-**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.
178
-
- **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.
178
+
- **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); neither line may name `Birfy`, the one identity every Bot writes as, because it is the single value both vocabularies list and so names no workstation at all (#1228); 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.
179
179
180
180
**`enqueue-guard` writes a check on the head, and the queue's ruleset does not require it** (#812, re-read 2026-10-24). Team pull requests merge upstream in `ThinkFlowLab/FlyBot` through the `main merge queue`, whose ruleset requires three contexts — `Node 24 · DSH 0.2.0-rc.2`, `tests-ok` and `merge-gate`; the paragraph below (#960) is how `merge-gate` reads the body the queue does not read. `enqueue-guard` still runs on that head, and on the queue entry: [`enqueue-guard.yml`](.github/workflows/enqueue-guard.yml) takes its code from `main` and never from the pull request, on every event that can change the answer for a fixed head — a push, a label added or removed, a comment, a review body, or a manual run — and calls [`enqueue-guard.mjs`](.github/scripts/enqueue-guard.mjs)'s own decision, the same function that prints the command, so the check and the command cannot disagree. It refuses a head that carries a `hold`, a pull request with no `agent-pr`, or a body that names no issue, and — **for T2 and T3 only** — a head that has no PASS naming that exact tip or a PASS that lists no `Ran:`; a T0/T1 change and the mechanical release pull request need no PASS at all, because the tier is computed from the diff and never read from a `risk:T*` label (#640). A new commit is a new head and a new check run, and a green check never carries over.
0 commit comments