diff --git a/.github/scripts/enqueue-guard.mjs b/.github/scripts/enqueue-guard.mjs index c53e1a089..4c6f7af40 100644 --- a/.github/scripts/enqueue-guard.mjs +++ b/.github/scripts/enqueue-guard.mjs @@ -47,50 +47,6 @@ function arg(name) { // runs); `gate-check.mjs` turns the same object into the `enqueue-guard` check run the ruleset requires // (#812). One function with two readers, so the check and the printed command cannot disagree — #812's // kill condition is a second copy of these rules, and there is none. -// #1344 (owner decision 2026-10-27): a branch an external agent owns is frozen while its write access is -// unresolved, and the list of what that means is a file on the base branch, not code: the owner peels the -// seal off by editing or deleting one JSON document, with no release of this script. The list is read at the -// pull request's BASE commit, never its head — a branch that carries its own copy could empty the list and -// un-freeze itself (#1346's shape) — and a pull request that edits the document is refused outright, so -// "edit the seal from inside" is closed too. This is a queue-entry seal, not a permission boundary: a direct -// push or an admin merge is unaffected, and the shared `Birfy` token can edit the file on `main`. -const FROZEN_PATH = '.github/agent/frozen-branches.json' -const FROZEN_REASON = 'frozen: external agent branch; owner decision pending' -const FROZEN_EDIT_REASON = 'frozen: .github/agent/frozen-branches.json is not editable by a pull request' - -// Missing file, unreadable file, empty arrays: nothing is frozen. A broken read must not stop the whole -// queue, and the seal is meant to be removable — both point the same way here. -function frozenList({ gh, repo, base }) { - const out = { branches: [], markers: [] } - if (!base) return out - try { - const body = JSON.parse(gh('api', `repos/${repo}/contents/${FROZEN_PATH}?ref=${base}`)) - const config = JSON.parse(Buffer.from(String(body?.content ?? ''), 'base64').toString('utf8')) - for (const key of ['branches', 'markers']) { - if (Array.isArray(config?.[key])) out[key] = config[key].map((v) => String(v).toLowerCase()).filter(Boolean) - } - } catch { - // no list, no seal - } - return out -} - -// The refusal this file exists for, or `null`: the head branch starts with a listed prefix, the body carries -// a listed marker, or the diff edits the list itself. `merged` is history and is never re-judged (#943). -function frozenRefusal({ gh, repo, pull, files }) { - const touched = (files ?? []).find((f) => f && f.filename === FROZEN_PATH) - // An ADD is how the seal first arrives (this very pull request). A modify or a delete is the self-unlock - // this rule closes, whatever the branch is. - if (touched && touched.status !== 'added') return FROZEN_EDIT_REASON - if (pull?.merged) return null - const list = frozenList({ gh, repo, base: pull?.base?.sha }) - const branch = String(pull?.head?.ref ?? '').toLowerCase() - const body = String(pull?.body ?? '').toLowerCase() - if (list.branches.some((prefix) => branch.startsWith(prefix))) return FROZEN_REASON - if (list.markers.some((marker) => body.includes(marker))) return FROZEN_REASON - return null -} - export function decidePull({ gh = realGh, env = process.env, repo: repoOption, pr: prOption, expectedHead, sameChangeSet } = {}) { const list = (path) => gh('api', '--paginate', '--jq', '.[]', path).split('\n').filter(Boolean).map((line) => JSON.parse(line)) const repo = repoOption || env.GH_REPO || arg('--repo') @@ -136,17 +92,6 @@ export function decidePull({ gh = realGh, env = process.env, repo: repoOption, p // case-insensitive: `--repo thinkflowlab/flybot` used to read our own branch as a fork and print ALLOW. const fork = String(pull.head?.repo?.full_name ?? '').toLowerCase() !== String(repo).toLowerCase() const decision = guard({ pr, head, labels, body: pull.body, files: files.map((f) => f.filename), mechanicalRelease, fork, draft: pull.draft, merged: pull.merged, result, reports, policy }) - // #1344: the seal answers before anything else can allow the entry — a well-formed PASS at the tip, an - // `agent-pr` label and a clean body change nothing here. - const frozen = frozenRefusal({ gh, repo, pull, files }) - if (frozen) { - return { - decision: { ...decision, ok: false, reasons: [frozen], reason: frozen, message: `do not enqueue #${pr}: ${frozen}` }, - pull, - repo, - pr, - } - } return { decision, pull, repo, pr } } diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index 7bd03c462..ea882844c 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -76,11 +76,7 @@ const withCheckpoint = (text, files) => { return `${text}\n${checkpoint({ issue, claim, files: names.length ? names : [HOST_FILE] })}` } -// #1344: the fixture also answers the contents read for the frozen list, at the pull request's BASE commit. -const BASE = 'c'.repeat(40) -const FROZEN_PATH = '.github/agent/frozen-branches.json' -const DEFAULT_FROZEN = { branches: ['cursor/'], markers: ['CURSOR_AGENT_PR_BODY'] } -const fakeGh = ({ body = null, raw = false, comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, heads = null, author = 'opener', headRepo = 'ThinkFlowLab/FlyBot', permission = undefined, draft = false, merged = false, branch = 'dev/fixture', baseSha = BASE, baseFrozen = DEFAULT_FROZEN } = {}) => { +const fakeGh = ({ body = null, raw = false, comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, heads = null, author = 'opener', headRepo = 'ThinkFlowLab/FlyBot', permission = undefined, draft = false, merged = false, branch = 'dev/fixture' } = {}) => { // `raw` is for the tests about the checkpoint itself: they must be able to hand over a body without one. const given = body ?? `${HEADERS}Fixes #602\n` const text = raw ? given : withCheckpoint(given, files) @@ -93,17 +89,13 @@ const fakeGh = ({ body = null, raw = false, comments = [], reviews = [], labels if (permission == null || permission === 'throw') throw new Error('fake gh: permission ' + path) return JSON.stringify({ permission, role_name: permission, user: { login: 'reviewed' } }) } - if (path.includes('/contents/')) { - if (baseFrozen === null) throw new Error('fake gh: 404 ' + path) - return JSON.stringify({ content: Buffer.from(JSON.stringify(baseFrozen)).toString('base64'), encoding: 'base64' }) - } if (path.includes('/files')) return files.map((f) => JSON.stringify(typeof f === 'string' ? { filename: f } : f)).join('\n') if (path.includes('/comments')) return comments.map((c) => JSON.stringify(c)).join('\n') if (path.includes('/reviews')) return reviews.map((r) => JSON.stringify(r)).join('\n') if (/\/pulls\/\d+$/.test(path)) { // #1112 (c): a run reads the head twice, so the fixture can answer differently the second time. const sha = heads ? heads[Math.min(pullReads++, heads.length - 1)] : head - return JSON.stringify({ head: { sha, ref: branch, repo: { full_name: headRepo } }, base: { sha: baseSha }, user: { login: author }, body: text, labels: labels.map((name) => ({ name })), draft, merged }) + return JSON.stringify({ head: { sha, ref: branch, repo: { full_name: headRepo } }, user: { login: author }, body: text, labels: labels.map((name) => ({ name })), draft, merged }) } // Anything else — `gh pr merge` above all — is the guard doing more than checking. throw new Error('fake gh: unexpected ' + path) @@ -954,39 +946,3 @@ test('#1335: a withdrawal naming another tip leaves this tip enqueueable', () => const r = run({ comments: [comment(), correction({ tip: OLD })] }) assert.equal(r.ok, true, r.reasons.join('; ')) }) - -// ---- #1344: the frozen seal. An external agent owns `cursor/` until its write access is settled, and the -// list lives in a JSON document on the BASE branch so the owner can delete it without a code change. It is -// a queue-entry seal, not a permission boundary: a direct push or an admin merge never reads it. -test('#1344: a cursor/ branch is refused at the queue entry even with a well-formed PASS at its tip', () => { - const r = run({ branch: 'cursor/workflow-1318-c5f8', comments: [comment()] }) - assert.equal(r.ok, false) - assert.ok(r.reasons.includes('frozen: external agent branch; owner decision pending'), r.reasons.join('; ')) - assert.match(r.message, /^do not enqueue #566: frozen: external agent branch; owner decision pending/) -}) - -test('#1344: emptying the list on the base branch un-freezes the branch — the seal is removable', () => { - const frozen = run({ branch: 'cursor/workflow-1318-c5f8', comments: [comment()] }) - assert.equal(frozen.ok, false) - const released = run({ branch: 'cursor/workflow-1318-c5f8', comments: [comment()], baseFrozen: { branches: [], markers: [] } }) - assert.equal(released.ok, true, 'with nothing listed, the branch is judged like any other: ' + released.reasons.join('; ')) - const absent = run({ branch: 'cursor/workflow-1318-c5f8', comments: [comment()], baseFrozen: null }) - assert.equal(absent.ok, true, 'a base without the file freezes nobody: ' + absent.reasons.join('; ')) -}) - -test('#1344: a body carrying the agent marker is refused on a branch that matches no prefix', () => { - const r = run({ branch: 'dev/friendly-name', body: '\n\nFixes #602\n', comments: [comment()] }) - assert.equal(r.ok, false) - assert.ok(r.reasons.includes('frozen: external agent branch; owner decision pending'), r.reasons.join('; ')) -}) - -test('#1344: a pull request that edits the frozen list is refused, so it cannot un-freeze itself', () => { - const r = run({ branch: 'cursor/workflow-1318-c5f8', comments: [comment()], baseFrozen: { branches: [], markers: [] }, files: [HOST_FILE, { filename: FROZEN_PATH, status: 'modified' }] }) - assert.equal(r.ok, false, 'an emptied list inside the pull request is not the list that is read') - assert.ok(r.reasons.includes('frozen: .github/agent/frozen-branches.json is not editable by a pull request'), r.reasons.join('; ')) -}) - -test('#1344: adding the list is how the seal arrives — an add is not an edit', () => { - const r = run({ branch: 'dev/1344-user-only-conclusions', comments: [comment()], baseFrozen: null, files: [HOST_FILE, { filename: FROZEN_PATH, status: 'added' }] }) - assert.equal(r.ok, true, r.reasons.join('; ')) -}) diff --git a/AGENTS.md b/AGENTS.md index 4c01b0357..b2e9e1657 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,26 +200,12 @@ Four rules decide what a queue entry must pass. The mechanical judge runs where 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. -**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA — the whole 40 characters, because a prefix names every head that starts that way and the newest of them is the tip being judged (#1112). **A tip that moved without changing the reviewed change set keeps its conclusion** (#1317): when no conclusion names the head itself, the gate compares `(path → blob sha)` at the reviewed tip and at the head over the pull request's changed paths, and accepts the PASS only when every one matches — naming both shas and the comparison it made — while a changed blob, a path missing on either side, a truncated tree or an unreadable one refuses exactly as before. A conclusion that names the head always decides, so an INC written at the head still vetoes a carried PASS. Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from : …` and carries no `Verdict:` line, so it is not a second verdict; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A withdrawal is the one correction the gates read** (#1335): `Correction from : the PASS at is withdrawn`, in the first line the way a `Verdict:` header is, joins the same newest-wins scan at the tip it names, and the gate refuses with `the PASS at was withdrawn by `. It counts only from the Bot that wrote that PASS or from a `reviewers` login — one sentence from any other writer is prose, so a healthy pull request cannot be parked by whoever can comment. A later PASS at the tip supersedes it, exactly as a later FAIL vetoes an earlier PASS, and a correction quoted inside a body is a quote, not a withdrawal. **A GitHub bot account's `Verdict:` is never a conclusion** — `user.type` is `Bot`, or the login ends `[bot]` — whatever its body says (#943, `cursor[bot]` at #940's tip); the rule is the account type and not a login list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline, so the real Reviewer voids such a report at that tip with `Correction from Reviewer:`. +**One conclusion per tip.** A tip carries at most one conclusion: one `Verdict:` block naming one SHA — the whole 40 characters, because a prefix names every head that starts that way and the newest of them is the tip being judged (#1112). **A tip that moved without changing the reviewed change set keeps its conclusion** (#1317): when no conclusion names the head itself, the gate compares `(path → blob sha)` at the reviewed tip and at the head over the pull request's changed paths, and accepts the PASS only when every one matches — naming both shas and the comparison it made — while a changed blob, a path missing on either side, a truncated tree or an unreadable one refuses exactly as before. A conclusion that names the head always decides, so an INC written at the head still vetoes a carried PASS. Two conclusions at the same tip is an incident, not a tie to be broken — nobody re-verdicts to win, the Reviewer that wrote the later one retracts it in a comment, and the pull request waits. A correction reads `Correction from : …` and carries no `Verdict:` line, so it is not a second verdict; the replacement PASS for a missing `Ran:` ([above](#gates)) is the only other body allowed at one tip. **A withdrawal is the one correction the gates read** (#1335): `Correction from : the PASS at is withdrawn`, in the first line the way a `Verdict:` header is, joins the same newest-wins scan at the tip it names, and the gate refuses with `the PASS at was withdrawn by `. It counts only from the Bot that wrote that PASS or from a `reviewers` login — one sentence from any other writer is prose, so a healthy pull request cannot be parked by whoever can comment. A later PASS at the tip supersedes it, exactly as a later FAIL vetoes an earlier PASS, and a correction quoted inside a body is a quote, not a withdrawal. **A GitHub bot account's `Verdict:` is never a conclusion** — whatever its body says (#943, `cursor[bot]` at #940's tip). The rule is the account type, read the other way round from a login list: only `user.type === 'User'` earns the name of a conclusion, so a bot account (`Bot`, or a login ending `[bot]`) **and a report whose `user` carries no `type` at all** are both refused — the missing field is the shape the impersonation takes, so it is read fail-closed rather than skipped (#1344). The type and not a list, because every Bot writes as the shared `Birfy` account and a list containing it would void the whole pipeline. Such a report never enters the refusal's own sentence: the reason stays `no Verifier report names the tip `, and the account that wrote it, with the text it wrote, is kept as a note in the decision's `judged` field, so the sentence names the tip rather than the bot (#1344, #1350). The real Reviewer voids such a report at that tip with `Correction from Reviewer:`. **Judged on the merge tree, never on the pull request page.** The page answers "does this commit pass alone"; the queue answers "does this commit pass merged onto the current `main`", and the second one is the merge decision. #759 was green on every check, was kicked out of the queue twice, and went red on `main` at `core/test/follow-through.test.js:264`. A red queue entry that names files you did not touch is the stale base of [step 2](#pipeline-contract), not bad luck: fix the base, do not re-enqueue blindly. **A merged pull request's body is not a control.** GitHub evaluates `Fixes #N` once, when the merge lands. Editing the body of a merged pull request afterwards closes nothing and triggers no guard; it only makes the record disagree with what merged. If the issue is still open, close it with a comment naming the pull request — and if the merge itself was wrong, say so on the issue. -#### Untrusted external agent branches - -An external agent can open a pull request here under its own branch, and its write access is not settled (owner decision 2026-10-27). What counts as one of those branches is listed in [`.github/agent/frozen-branches.json`](.github/agent/frozen-branches.json) and read at the pull request's **base** commit — today the prefix `cursor/` and any body marker starting `CURSOR_AGENT_PR_BODY` (`` above all). `enqueue-guard` refuses such a pull request at the queue entry with `frozen: external agent branch; owner decision pending`, whatever conclusion its tip carries, and it refuses one whose diff edits that file — so the list cannot be emptied from inside the branch it judges. Deleting the file (or emptying both arrays) on `main` releases everything. This is a queue-entry seal, not a permission boundary: a direct push or an admin merge never reads it, and the shared `Birfy` token can edit the file. - -Until it is lifted, on such a branch: - -- **Do not adopt its `patches/ds-bot/*.patch`.** Those are an external agent's output, not this pipeline's generated files. -- **Do not rebase onto it**, and do not use it as a base for any branch or any review. -- **Do not write a conclusion, a comment or a label on it.** No `Verdict:`, no `Correction`, no `agent-*`: the record stays as the external agent left it. - -It is also never the baseline for `generate`. Anyone who has to touch `patches/` starts from `git checkout 41decf09 -- patches/ds-bot` and regenerates from there, never from a branch an external agent produced. - -本条自 `cursor` 云端 agent 的写权限被撤销之日起失效，由 Chief 删除。 - #### The checkpoint block: a hand-off the next Bot can read (#1202) A pull request hands itself over in one block in the body, under `## Checkpoint`. The body is the only store: it becomes the squash commit message, so the block cannot fall out of step with the pull request the way a comment or a label can.