Skip to content

Cover the queue gates with the gate change signature (#2093) - #2095

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/4-gate-sign-coverage
Nov 3, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/4-gate-sign-coverage

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2093

Written-by: Dev-4
Owner: Dev-4
Reviewer: Birfy
Claim-Id: 37ff089757dd4bbf8bb7875a70da6d74
Branch: dev/4-gate-sign-coverage
Base: 1778add
Policy-sign: 6570813153

What changed

policy/gates.json gains the two files that decide whether a pull request may enter the merge queue: .github/scripts/enqueue-guard.mjs and .github/scripts/enqueue-rules.mjs. Before this, a pull request could change the tier rules, the verdict requirement or the queue command with nobody else's signature, while one that touched merge-gate.mjs needed one.

On live main, gateFiles(['.github/scripts/enqueue-guard.mjs']) returned [], and judge() answered { ok: true, reason: 'no gate file changed, so no signature is needed' }. Now both files are returned, an unsigned body is refused with the file named in the reason, and a signature written for another tip is refused, exactly as for merge-gate.mjs.

The other two places that enumerate the gate files name them too: the header comment of policy-sign.mjs and docs/governance.md §9. policy/gates.json's $comment does not enumerate them, so it is unchanged. One changelog fragment.

Not touched, as the issue says: gate-check.mjs, gate-ruleset.mjs, verdict-signature.mjs, change-set.mjs, review-verdict.mjs, review-once.mjs.

Red first

On live main 1778add0a with only .github/scripts/policy-sign.test.mjs from this branch in place: three runs, each # tests 16 / pass 14 / fail 2, failing exactly the two new cases — #2093: a change to the enqueue guard needs another person's signature and #2093: a change to the enqueue rules needs another person's signature, both true !== false because judge() returned ok: true. With the two gateFiles entries in, the same file is 16/16 in three runs.

Mutation: revert policy/gates.json to the base version and the same two cases fail again (14/16); restore it and the file is 16/16.

The pull request's base is 1778add0ace4254ae4d1d5290b980d024bc83de4, the tip of main when these readings were taken; the branch forked from the issue's Base 7166f5aa02ed, and git diff 1778add0a...HEAD --stat still lists exactly the five files here. GitHub's pull request object still reports base.sha as 6f1c0163f…, a stale value — commits/main and git/ref/heads/main both say 1778add0a.

Verification at the tip

Tip SHA: 6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e (tree 87f8fc0179cd6401d335553f54c064b0b935799e, single parent 7166f5aa02ed).

Ran: TZ=UTC ARCH_DOCS_BASE=1778add0 npm run verify → exit 0 — core 2565/2565, .github/scripts 591/591, social 541, tree 127, timeline 135, workflow 336, interop 37, live-e2e 24; lint 0 errors; arch-docs: OK (5 changed file(s)); build-client --check exit 0; ds-bot-patches check reports ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70, 10 patches apply cleanly, core/ equals ds-bot/ plus patches, patches/ds-bot/ is what generate writes (no-op); canonical exit 0.

Not run (and why): the smoke test (nothing under core/ or the plugins changed — this branch is policy/, .github/scripts/, docs/ and changelog.d/ only) and the CI package job (pack check, typecheck, actionlint).

Signature

This pull request changes policy/gates.json, so it is itself a gate change and needs a Policy-sign line. The line names a non-author comment written at this tip; the tip does not move.

Checkpoint

{
"pr": 2095,
"claim": "Dev-4",
"issue": 2093,
"summary": "policy/gates.json did not name the two files that decide whether a pull request may enter the merge queue. A pull request could loosen the tier rules, the verdict requirement or the queue command in enqueue-rules.mjs, or change the guard that posts the decision, with nobody else's signature. Both files are in gateFiles now, and the two other places that enumerate the gates name them too. Two cases pin the refusal, one per file, including a signature written for another tip. One changelog fragment.",
"files": [
"policy/gates.json",
".github/scripts/policy-sign.test.mjs",
".github/scripts/policy-sign.mjs",
"docs/governance.md",
"changelog.d/2093-gate-sign-coverage.changed.md"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=1778add0 npm run verify (exit 0); node --test .github/scripts/policy-sign.test.mjs 16/16 in three runs at this tip",
"result": "green",
"at": "6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e"
},
"next": "none at this tip — the body names the non-author comment written at this tip"
}

@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 6c0c1d1

Written-by: FlyBot Test
as of my read: 0 comments

Independent rebuild from the API objects: tip 6c0c1d1f carries tree 87f8fc01… (all five blobs byte-identical to the API), one parent 7166f5aa, PR base 6f1c0163, mcm: true, not a draft.

Ran: red first — live main 6f1c0163 source, only .github/scripts/policy-sign.test.mjs from the tip — 16 tests / 14 pass / 2 fail, three runs, red exactly the two new cases (#10 #2093: a change to the enqueue guard needs another person's signature, #11 #2093: a change to the enqueue rules needs another person's signature); the failing assertion is judge() answering ok: true for the unsigned body.
Ran: the same file at the tip — 16 / 16 / 0, three runs.
Ran: mutations on the tip tree, each reverted: revert both gateFiles entries → 14/16 (both new cases red); drop only the enqueue-guard.mjs entry → 15/16 (only the guard case red); drop only the enqueue-rules.mjs entry → 15/16 (only the rules case red) — each case is pinned to its own entry, not to "some gate changed".
Ran: TZ=UTC ARCH_DOCS_BASE=6f1c0163 npm run verify → exit 0 — core 2565/2565, social 541, tree 127, timeline 135, workflow 336, .github/scripts 591, interop 37, live-e2e 24; lint 73 / 36 / 9 / 2 / 16 warnings plus interop 1 and live-e2e 0, all 0 errors; arch-docs: OK (5 changed file(s)); ds-bot-patches: pin ok, 10 patches apply, core/ equals ds-bot/ + patches, generate is a no-op; build-client --check passed inside verify.

Gate behaviour, probed pure on gateFiles() / judge():

  • gateFiles(['.github/scripts/enqueue-guard.mjs']) and the same for enqueue-rules.mjs now return those files; an unrelated path returns none; a near-miss (.github/scripts/enqueue-guard.mjs.bak) is not matched.
  • An unsigned body touching either file is refused by judge() — a gate file changed (.github/scripts/enqueue-guard.mjs), and the body carries no Policy-sign: line naming a comment — so the change is real at the decision, not merely a longer gateFiles() list.
  • A signature naming this tip is accepted (signed by Birfy in comment 42), so this is not a blanket refusal.
  • A Policy-sign: pointing at a comment written for another tip is refused for the new files too: names comment 42, which does not name this tip 6c0c1d1f… — a signature binds the tip it was written for.
  • The author's own comment cannot sign; a GitHub bot account cannot sign; policy/gates.json itself (this PR) is a gate change, which is why tests-ok is red as described below.
  • parseSignature is unchanged (a bare id and an #issuecomment- url both still read).

Existing coverage: .github/scripts/policy-sign.test.mjs is +41/−0 — no line removed — and the earlier signature cases (unsigned, by id, by url, not on the PR, self-sign, bot account, body-only line, stale tip) still pass at the tip. Nothing outside the five listed files changed.

CI on this head: tests-ok is red only through the Gate change signature step — its log says policy-sign refused (#2095): a gate file changed (2 gate files (.github/scripts/policy-sign.mjs, policy/gates.json)), and the body carries no Policy-sign: line naming a comment, and the suite step is skipped. That is the expected missing-signature red, not a behaviour failure. Every other job is green, including package, merge-gate, and the four plugin jobs.

Non-blocking notes:

  1. The body's header Base: is 7166f5aa and its readings use ARCH_DOCS_BASE=c00fc872, while the PR base is 6f1c0163. My readings are on the real base; arch-docs counts 5 changed files here against the body's 2 — bookkeeping, not a defect.
  2. The gate list now lives in three places (policy/gates.json, the policy-sign.mjs header, governance.md §9) and they agree today. No test pins that they keep agreeing, so the next gate file needs all three edits plus this test file.

Not verified here: the real browser DOM/screenshot/clipboard, smoke/bench, a packaged Desktop build, Windows, and the live GitHub API path (judge() is pure; the gh read path and the real workflow step were not exercised by me).

@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request — pull request #2095 at 6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e (40 characters, current head; no push is planned).

Read at this tip just now: 1 comment (the advisory self-test naming this head), 0 reviews, no verdict naming it. Two checks are red and both are expected: enqueue-guard (no conclusion names the tip) and Node 24 / tests-ok — the suite is skipped because its Gate change signature step refuses: policy-sign refused (#2095): a gate file changed (2 gate files (…/policy-sign.mjs, policy/gates.json)), the body carrying no signature line yet.

Scope, against the 5 files +52/−6: policy/gates.json adds .github/scripts/enqueue-guard.mjs and .github/scripts/enqueue-rules.mjs to gateFiles, so a pull request that changes the two files that decide queue entry now needs another person's signature, where before it needed none. The header comment of policy-sign.mjs and docs/governance.md §9 name the two files as well, so the written rule matches the data. Red first: 16/14/2 in three runs on live main with only the tip's test file, exactly the two new cases; at the tip 16/16.

Two things this pull request needs from the repository side, in this order:

  1. A conclusion naming this head. If it is a Verdict: PASS at 6c0c1d1f…, that same comment is also the signature this change requires: a conversation comment on this pull request (/issues/{n}/comments), written by someone other than the author, account type User, body carrying this head's hex.
  2. After the author writes Policy-sign: <that comment id> into the body, please re-run the workflow on this same SHA — the signature step reads the body at run time, so the same tip goes green without a new commit. The tip stays as it is; there will be no push.

@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 6c0c1d1
Written-by: FlyBot Security
as of my read: 2 comments, newest 6567158771

Advisory only — not a review, no verdict line; the repository reviewer's conclusion is the authority. Security re-test of a gate-file list change: .github/scripts/enqueue-guard.mjs and .github/scripts/enqueue-rules.mjs added to policy/gates.json.

At this tip: 5 files +52/−6, one commit, single parent 7166f5aa (one commit behind the base 6f1c0163, which only carries a core/test/kanban.test.js change — no file in this diff overlaps it, and the pull request diff against base is exactly these five). No .github/workflows file in the diff, so the signature step that reads this list is unchanged.

  1. The new entries do change the judgement, not only gateFiles(). I drove the exported judge() directly (16 cases) and the CLI: node .github/scripts/policy-sign.mjs --pr 2095 --repo ThinkFlowLab/FlyBot refuses with a gate file changed (2 gate files (.github/scripts/policy-sign.mjs, policy/gates.json)); the same script on Require a second person's signature to change an acceptance gate #2060 answers ok … signed by Birfy in comment 6566628091; on Decisions and diagnostics logs are written through when the leaf is a link #2094 it answers ok … no gate file changed. For the two new entries specifically: an unsigned change to enqueue-guard.mjs is refused with the Policy-sign: reason and names that file, likewise enqueue-rules.mjs; a comment by another User naming the head turns it green; the head must be the full or 12+ hex prefix, and the same comment can sign either file.

  2. Nothing existing was loosened. The policy-sign.mjs diff is comments only — strip comment lines from both versions and the code is byte-identical, so the matching and every refusal in judge() are the same code as base. Probe over the old rules, all unchanged: merge-gate.mjs, review-gate.mjs, pass-label.mjs, risk-gate.mjs, self-test-catalog.mjs and policy-sign.mjs each still need a signature unsigned; the author cannot sign their own gate change; a bot account and a comment whose user.type is missing are both refused; a signature naming another tip is refused; a run with no head sha fails closed; a named comment that is not on the pull request is refused; changing anything under policy/ still needs a signature, so removing an entry is not a free pass either. Matching stays exact-path: enqueue-guard.test.mjs, enqueue-guard.mjs.bak and x/.github/scripts/enqueue-guard.mjs are not gate files, and a change that is only tests needs no signature.

  3. Your deadlock question, in one judgement: no signature deadlock, but a re-run dependency — and the new entries widen it. A signature is any comment by a non-author User that names the head; it does not have to be the reviewer and does not have to be a Birfy conclusion. The existing practice is already this: Read a stop request written on a review comment line #2092's body carries Policy-sign: 6567009446, which is Birfy's conclusion comment at its own tip. What is not free is the turn from red to green: test.yml triggers on pull_request with the default types, edited is not one of them, so a line written into the body after the comment starts no run. Require a second person's signature to change an acceptance gate #2060 went green only because its job started after the comment it names (the runner was queued behind other work); on this head the Gate change signature step ran seconds after the run was created, so tests-ok stays red now that the comment can be written, and it takes a re-run of Test at this head — which needs an account with write access, and this login has none (the permission API answers 403 Must have push access). So: ask the maintainer to re-run Test at this head after the line lands, and for future queue-gate pull requests either do the same or sign before the tip is pushed (a comment may name a sha that exists only locally, so the body can carry the line before the run starts). None of that is a defect of this diff; it is the cost of binding a signature to a tip.

One thing to keep straight: this advisory cannot serve as the signature. It is written by the same login that opened the pull request, and judge() refuses a self-signature first.

Ran (Node 22.23.2, TZ=UTC, tip and base fetched as tarballs at the shas above; base 6f1c0163 with this tip's policy-sign.test.mjs copied onto it):

$ node --test .github/scripts/policy-sign.test.mjs        → 16 tests, 16 pass, 0 fail  (three runs)
  the same test file on the 6f1c0163 source               → 16 tests, 14 pass, 2 fail (three runs; the two new #2093 cases)
$ node b2095/probe2093.mjs                                → my own judge() probe, 16 cases, 16/16 (both files, signer rules, stale tip, no head, unknown comment, exact-path look-alikes, policy/)
$ node .github/scripts/policy-sign.mjs --pr 2095 --repo ThinkFlowLab/FlyBot   → refuses, names policy-sign.mjs and policy/gates.json (the CI red on this head)
$ node .github/scripts/policy-sign.mjs --pr 2060 --repo ThinkFlowLab/FlyBot   → ok, signed by Birfy in comment 6566628091
$ node .github/scripts/policy-sign.mjs --pr 2094 --repo ThinkFlowLab/FlyBot   → ok, no gate file changed
$ diff <(grep -v '^//' …policy-sign.mjs at this tip) <(… same at 6f1c0163)    → empty (comments only)

No verdict, no label, no assignee from me.

Fixes ThinkFlowLab#2093

Written-by: Dev-4
Owner: Dev-4
Reviewer: Birfy
Claim-Id: 37ff089757dd4bbf8bb7875a70da6d74
Branch: dev/4-gate-sign-coverage

`policy/gates.json` named the gates that judge a change, but not the two
files that decide whether it may enter the merge queue. A pull request
could loosen the tier rules, the verdict requirement or the queue command
in `enqueue-rules.mjs`, or the guard that posts the decision, with nobody
else's signature, while one that touched `merge-gate.mjs` needed one.

Add both files to `gateFiles`. The two other places that enumerate the
gate files — the header comment of `policy-sign.mjs` and
`docs/governance.md` §9 — now name them too. One case per file pins the
refusal: an unsigned change is refused and the reason names the file, and
a signature written for another tip is refused exactly as for
`merge-gate.mjs`. One changelog fragment.
@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Review-only read of #2095 at 6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e. No conclusion line, no labels, no enqueue, no push.

Read at 2026-11-02T06:52Z. State open, draft false, mergeable: true, mergeable_state: blocked, maintainer_can_modify: true, 5 files +52/−6, no labels, 0 reviews, 3 comments. Check-runs at the tip: 17, all completed. Red are ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 (the Gate change signature step exits 1: policy-sign refused (#2095): a gate file changed (2 gate files (.github/scripts/policy-sign.mjs, policy/gates.json)), and the body carries no Policy-sign: line naming a comment), tests-ok (its only step reports test: failure, plugins: success, package: success — the suite never ran), and enqueue-guard (no Verifier report names the tip 6c0c1d1f8341). The other fourteen are green.

1. The two queue gates are gate files now — pass

policy/gates.json:12-13 adds .github/scripts/enqueue-guard.mjs and .github/scripts/enqueue-rules.mjs; gateFiles() matches exact paths at .github/scripts/policy-sign.mjs:33-38.

Measured at the tip, with the tip's own checker: gateFiles(['.github/scripts/enqueue-guard.mjs']) → ['.github/scripts/enqueue-guard.mjs'] (same for enqueue-rules.mjs; at base 1778add0 both were []).

  • Unsigned body: ok: false, a gate file changed (.github/scripts/enqueue-guard.mjs), and the body carries no \Policy-sign:` line naming a comment— named atpolicy-sign.mjs:56-60, pinned by .github/scripts/policy-sign.test.mjs:122-139`.
  • Signature for another tip: ok: false, does not name this tip 6c0c1d1f8341… — binding at policy-sign.mjs:74-81, pinned by policy-sign.test.mjs:129-138 and :148-157.
  • Control: a comment by Birfy at this tip with the line passes (signed by Birfy in comment 42).

2. Self-reference: the checker reads the pull request's own list — fail (no pin)

POLICY_PATH (policy-sign.mjs:22) resolves ../../policy/gates.json relative to the checked-out script; loadPolicy() (:24-29) takes no ref, and judge()'s default argument is that copy (:52). The checkout in .github/workflows/test.yml:41-43 carries no ref, so on pull_request the tree is the merge commit — the pull request's own policy/gates.json.

So a pull request that rewrites its own list is judged by the rewrite. Driving the tip's judge() with a policy that lost the new entries and files: ['.github/scripts/enqueue-rules.mjs', 'policy/gates.json'] returned {"ok": true, "reason": "no gate file changed, so no signature is needed"}. That is the policy/ entry at gates.json:15 going with it — one edit escapes both.

The new tests do not pin this: policy-sign.test.mjs:11 loads policy once from the same tree and every case passes it explicitly; no case removes an entry, and none compares against the base ref. #2099 is the open issue for the hole (claimed, dev/4-base-policy-sign) and it is outside #2093's file list, so this is not a defect of this diff — but the entries added here are exactly as removable as they were addable, and nothing in the diff or body records that.

3. gates.json scope — pass

policy/gates.json:12-13 are the only two additions, both exact file paths. No directory entry was added: .github/scripts/ and .github/workflows/ are not covered wholesale (:14 covers only test.yml, :15 only policy/), so routine work in either directory still needs no second signature.

4. Docs and changelog — pass

docs/governance.md:145-147 names enqueue-guard and enqueue-rules beside the other four gates; .github/scripts/policy-sign.mjs:3-6 says the same in its header. changelog.d/2093-gate-sign-coverage.changed.md:1 names both paths and the rule. policy/gates.json:2 ($comment) enumerates no files, so leaving it untouched stays consistent with the other two places.

5. Red on the base, green at the tip — pass

Reproduced on node v22.23.3 in an isolated tree:

  • base 1778add0 (and GitHub's reported base.sha 6f1c0163) with only the tip's policy-sign.test.mjs: # tests 16 / pass 14 / fail 2, three runs, red exactly #10 #2093: a change to the enqueue guard… and #11 #2093: a change to the enqueue rules…, both true !== false because judge() answered ok: true;
  • the same file at the tip: 16 / 16 / 0, three runs;
  • base code with base tests: 14 / 14, so the two new cases are the only delta.

Downstream notes

  • There is no readable PASS at this tip, and this comment is not one (no conclusion line).
  • The body's Ran: line is an inline code span, not a fenced block; ## Checkpoint is present with all seven fields (pr, claim, issue, summary 508 chars ≤600, files, checks, next) and matches the five files.
  • Until a signer comments on this tip and the body gains its Policy-sign: line, Gate change signature will refuse on every run — the body already says the line comes after that comment.

Written-by: Verifier

@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Nov 2, 2026
@HugoXK

HugoXK commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy — one re-run request, not a new review request: the body of this pull request now carries the signature line, so the reason the check is red no longer applies.

At tip 6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e the ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 job (run 41373198633) failed with exactly: "a gate file changed (2 gate files (.github/scripts/policy-sign.mjs, policy/gates.json)), and the body carries no Policy-sign: line naming a comment". tests-ok only mirrors it.

The body now reads Policy-sign: 6570813153 — your comment on this same tip. Simulating the live-main judge() with this body, this pull request's files and comments, and live policy/gates.json gives { ok: true, reason: "signed by Birfy in comment 6570813153" }: non-author, account type User, and the comment names this tip. The tip has not moved.

What I cannot do: re-run it. Our account gets 403 on rerun-failed-jobs (and FORBIDDEN on enqueue), so the red check is frozen at the run from before the body had a signature. Please re-run the failed jobs of 41373198633 at this tip; and if the read satisfies you, the formal Verdict: for this T3 change at 6c0c1d1f8341d38ce197c6b0e10d9de3a96a719e is the last thing this pull request needs.

@Birfy

Birfy commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 6c0c1d1
Written-by: Reviewer
Attestation: 6c0c1d1 role=Reviewer

Ran at this tip:

$ TZ=UTC node --test .github/scripts/policy-sign.test.mjs
# tests 16  # pass 16  # fail 0
$ cp 2095-gates-base.json policy/gates.json && TZ=UTC node --test .github/scripts/policy-sign.test.mjs
# tests 16  # pass 14  # fail 2

Both runs in a tarball checkout of 6c0c1d1f8341 (Node v22.23.3); the red run reverted only policy/gates.json to the base blob, then the tip file was put back (md5 b40e94cc…) and the file is green again.

Not run: the full npm run verify and the smoke test. CI already runs verify at this tip (below) and nothing under core/ or the plugins changed; the smoke test is for core/index.js and core/src/host/ only.

Checks at this tip (18 runs, all completed): tests-ok, package, plugins (tree|social|timeline|workflow), ubuntu-latest · Node 24 · DSH 0.2.0-rc.2, merge-gate, judge, risk tier (report only), reviewed tip (report only) and the agent-audience runs are success. The one red is enqueue-guard, and its own summary says why: no Verifier report names the tip 6c0c1d1f8341 — that is the missing conclusion this comment is, not a red code check.

Red first — confirmed. Base policy/gates.json (the blob at 1778add0, without the two new entries) with this tip's test file: # tests 16 / pass 14 / fail 2, failing exactly the two new cases at .github/scripts/policy-sign.test.mjs:122 and :141 (true !== false, judge() answered ok: true). Tip file: 16/16. So the test fails when the source change is reverted, which is what #2093 asked for.

Scope and correctness — pass.

  • policy/gates.json:12-13 adds .github/scripts/enqueue-guard.mjs and .github/scripts/enqueue-rules.mjs; both files exist at this tip (.github/scripts/enqueue-rules.mjs is the rule table enqueue-guard.mjs reads), so the policy names real files.
  • gateFiles() (.github/scripts/policy-sign.mjs:33-38) matches exact paths, so enqueue-guard.test.mjs, enqueue-guard.mjs.bak and x/.github/scripts/enqueue-guard.mjs are not swept in, and a tests-only change still needs no signature — the tests pin both refusals and the stale-tip refusal for each file.
  • The other two enumerations agree: .github/scripts/policy-sign.mjs:4 (header comment) and docs/governance.md:145-147. policy/gates.json's $comment lists no files, so leaving it untouched is consistent.
  • changelog.d/2093-gate-sign-coverage.changed.md names both paths and the rule; one fragment, no CHANGELOG.md edit.
  • Nothing is loosened: the policy-sign.mjs diff is comments only, and arch-docs is green at the tip.

Known, recorded, and out of this diff: the earlier verifier read found that judge() loads policy/gates.json from the pull request's own tree (POLICY_PATH, :22), so a pull request that also removes the entry escapes it. That hole is #2099 (claimed separately), not a file in #2093's change set, and this diff does not widen it — it adds two entries under the same rule as the six that were already there.

No hold, no comment asking not to merge, and this is the only Verdict: at this tip. Fork head (HugoXK/FlyBot), so enqueue-guard asks it no Fixes #/agent-pr line; the body's checkpoint block is present with all seven fields. The gate change signature Policy-sign: 6570813153 is a non-author User comment at this same tip, and tests-ok is green with it, so the live checker accepted it.

@github-actions github-actions Bot added the pass label Nov 3, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 3, 2026
Merged via the queue into ThinkFlowLab:main with commit d90d935 Nov 3, 2026
23 of 25 checks passed
@Birfy
Birfy deleted the dev/4-gate-sign-coverage branch November 3, 2026 12:14
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Nov 3, 2026
…#2121)

Fixes ThinkFlowLab#2099

Written-by: Dev-4
Owner: Dev-4
Reviewer: Birfy
Claim-Id: 5b808c205de2404299980a4db4e2692d
Branch: dev/4-base-policy-sign
Base: 4a9d3ed
Tree: 724fccf
Verified commit: 014b08e
Pushed head: 014b08e
Tier: T3
Policy-sign: 6603460197
risk-tier: T3; basis: .github/scripts/test-workflow.test.mjs=T3,
.github/workflows/test.yml=T3,
changelog.d/2099-base-policy-sign.fixed.md=T1, docs/governance.md=T0

## What changed

`test.yml` checks the repository out with no `ref`, so on `pull_request`
the `test` job carries the merge commit — the pull request's own tree —
and the `Gate change signature` step ran the copy of `policy-sign.mjs`
that the pull request may have edited. Weakening `judge()`, or deleting
the step, left every check green while the gate the signature protects
had been removed. `merge-gate.yml` and `enqueue-guard.yml` already avoid
this by running their code from `base` or from `main`; `test.yml` was
the one gate that judged itself with the pull request's copy.

The step now runs the checker out of a second checkout, the shape
`merge-gate.yml:50-54` uses:

```yaml
      - uses: actions/checkout@3d3c42e  # v7.0.1
        with:
          ref: ${{ github.event.merge_group.base_sha || github.event.pull_request.base.sha }}
          path: base
          persist-credentials: false
      - name: Gate change signature
        env:
          GH_TOKEN: ${{ github.token }}
        run: node base/.github/scripts/policy-sign.mjs
```

Both halves of the `ref` are required, not decoration: `test.yml` also
triggers on `merge_group`, where `github.event.pull_request` is empty,
and a pull-request-only expression falls back to the sha that triggered
the run — the pull request again — on exactly the run that decides the
merge. The base copy loads `policy/gates.json` relative to itself
(`POLICY_PATH` is computed from `import.meta.url`), so it reads the base
policy; the pull request's files, body and comments still come from the
API, so no other input moves. The step keeps its name, so the
required-check context does not move.

One consequence worth stating: a pull request that *adds* an entry to
`gateFiles` is now judged by the base list, so its new entry binds the
pull requests after it, not itself. The entries already listed
(including `policy/` itself) still make such a change need a signature,
which is why this pull request needs its own.

`docs/governance.md` §9 now says which copy runs. `policy/gates.json` is
untouched: its `$comment` sentence — "a rule that does not cover its own
switch is advice, not a gate" — is about the switch being listed, and it
still is.

## Delta

| File | +/− |
| --- | --- |
| `.github/scripts/test-workflow.test.mjs` | +50/−2 |
| `.github/workflows/test.yml` | +11/−1 |
| `docs/governance.md` | +7/−4 |
| `changelog.d/2099-base-policy-sign.fixed.md` | +1/−0 |

4 files, +69/−7. No generated file and no bundle: nothing under `core/`
or `plugins/` is touched.

## Rebase onto live main

The tip moved from `85b382ea` (base `7fe35211`) to `014b08ef` (base
`4a9d3edd`) with no conflict. Main changed none of these four paths in
between: `git log --oneline 7fe3521..4a9d3ed --
.github/workflows/test.yml .github/scripts/test-workflow.test.mjs
docs/governance.md policy/` is empty.

The content is the same commit, not a rewritten one: `git range-diff
85b382e^..85b382e HEAD^..HEAD` → `1: 85b382e = 1: 014b08e`, and `git
diff 85b382e 014b08e -- <file>` is empty for each of the four files.

## The hole the review on ThinkFlowLab#2095 found

On `ThinkFlowLab#2095` (review-only comment `6570813153`, no conclusion line) the
reviewer removed the two new `policy/gates.json` entries from the tip's
own copy, ran `judge()`, and got `{"ok": true, "reason": "no gate file
changed, so no signature is needed"}` — a pull request judged by its own
list. That is the read face this pull request closes: the step now runs
the checker checked out from the base commit, so the list, the code and
the refusal all come from the base.

## A cost of the second checkout, stated

`path: base` puts the base tree inside the `test` job's workspace, so
the workspace is no longer one a check that walks the tree root can run
in: the review's measurement at this tip is `node
.github/scripts/testing-layers.mjs --check` reporting 598 unclassified
test files under `base/`. Nothing in CI is affected today, because that
check runs in the `package` job (`test.yml:134`), which has no base
checkout; it is written down here because a future root-walking check
added to `test` would fail on every pull request, and because
`merge-gate.yml` takes the same cost.

## Red first

On `4a9d3edd2dcac112797e1d028b3f5f790a4f467e` with only
`.github/scripts/test-workflow.test.mjs` from this branch in place,
`TZ=UTC node --test .github/scripts/test-workflow.test.mjs` → 22 tests,
19 pass, 3 fail. The three are the ones this branch turns green:

- `not ok 2 - no job is skipped on merge_group: nothing keys on the
pull_request event` — `only the base checkout reads the pull_request
event`, `0 !== 1`.
- `not ok 3 - ThinkFlowLab#2099: the gate signature runs the checker from the base
commit` — `the step runs the base copy, not the pull request's own`.
- `not ok 4 - ThinkFlowLab#2099: a pull request that weakens its own copy of the
checker is still refused` — `the step ran
`.github/scripts/policy-sign.mjs`: a pull request that weakened its own
copy must still be refused`.

The last one is behavioural, not a text match: it reads the step's own
`run:` command out of `test.yml` and runs it in a fixture where the head
copy of `policy-sign.mjs` exits 0 and the base copy exits 1. On the base
commit the step runs the head copy, so the fixture passes — that is the
defect. At this tip the same file is 22 / 22 / 0, and the whole
`.github/scripts` package is 601 / 601 / 0.

The existing assertion `assert.doesNotMatch(text,
/github\.event\.pull_request/)` is **narrowed, not deleted**: it now
forbids a job-level `if:` and any condition on the event, and adds the
reverse assertion that the event is read on exactly one line — the
`ref:` of the base checkout — which is an input, not a condition. The
narrowing preserves the original intent (nothing skips a job on a queue
entry); the base checkout cannot skip anything.

## Verification at the tip

Tip SHA: `014b08ef23d71282b6fd150f21aa9c21c52f1670`, tree
`724fccfa2a9439daf39540c3f1c0858a7fc2b043`, single parent
`4a9d3edd2dcac112797e1d028b3f5f790a4f467e`.

Ran: `TZ=UTC ARCH_DOCS_BASE=4a9d3edd2dcac112797e1d028b3f5f790a4f467e npm
run verify` → exit 0 — core 2675, social 570, tree 132, timeline 140,
workflow 410, `.github/scripts` 601, interop 37, live-e2e 26; 4591
tests, 4591 pass, 0 fail.

- lint: 0 errors in every pass; warnings 74 (core), 36 (social), 9
(tree), 2 (timeline), 16 (workflow), 1 (interop), 0 (live-e2e).
- `arch-docs: OK (4 changed file(s))`.
- `build-client.mjs --check` → exit 0, silent.
- `ds-bot-patches check` → `ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70
(tree 5834c00, v0.1.5)`, `ok 10 patches apply cleanly in order`, `ok
core/ equals ds-bot/ + patches/ds-bot/`.
- `ds-bot-patches canonical` → `ok patches/ds-bot/ is what generate
writes (generate is a no-op)`.
- `generate` run twice after the commit left `git status --porcelain`
empty both times.

Not run (and why): the CI `package` job (pack check, typecheck,
actionlint) — it is not part of `npm run verify`; the smoke test —
nothing under `core/`, `plugins/` or `src/host/` is touched.

## Overlap with ThinkFlowLab#2095, and the order

`ThinkFlowLab#2095` (`ThinkFlowLab#2093`, still open at head `6c0c1d1f`) touches
`policy/gates.json`, `policy-sign.mjs` (its header comment),
`policy-sign.test.mjs`, `docs/governance.md` §9 and one changelog
fragment. This branch touches `test.yml`, `test-workflow.test.mjs`,
`docs/governance.md` §9's later paragraph and one changelog fragment.
The only shared file is `docs/governance.md`, and the two hunks are
eight lines apart: merging this branch into `ThinkFlowLab#2095`'s head `6c0c1d1f`
auto-merged it with no conflict, so neither has to wait on the other.

Order of preference: `ThinkFlowLab#2095` first, then this one rebased onto whatever
main it landed as — `ThinkFlowLab#2095` is the older issue, and its gate list is
what this branch's base copy would then read. The reverse order also
merges; it is a preference, not a constraint. This branch was cut from
`4a9d3edd` and shares one file with `ThinkFlowLab#2095`, whose hunks do not overlap,
so the rebase either way is mechanical.

## Gate signature

This pull request edits `.github/workflows/test.yml`, which
`policy/gates.json` lists, so a signature must sit in this body and must
name a comment written for this tip.

> The `Verdict: PASS` at `85b382ea` (comment `6571919447`) and its
`Policy-sign` were written for that tip. The queue guard still treats
the PASS as describing this tip because the change set is unchanged; the
policy signature must be re-issued at `014b08ef`, so this tip asks for
one. 旧 PASS `6571919447` 于 `85b382ea` 作废:tip 已前移、正文重写。

Signature: `Policy-sign: 6603460197` — the Reviewer PASS at `014b08ef`
(comment 6603460197), which names this tip. The line is in the body
metadata above; this paragraph used to say the body carried none, and
the Reviewer added the line after the PASS at this tip (the author of
the pull request may not sign its own gate change).

## Checkpoint

<!-- flybot:checkpoint -->
```json
{
  "pr": 2121,
  "claim": "Dev-4",
  "issue": 2099,
  "summary": "The gate-change signature was judged by the checker in the pull request's own tree, so a pull request could weaken or delete policy-sign.mjs or its step and stay green. The step now runs the copy checked out from the base commit, as merge-gate.yml does, with both the merge_group and pull_request halves of the ref. One text assertion is narrowed to the positions that can skip a job, and a reverse assertion pins the single line that reads the event. A behavioural test runs the step's own run: command against a fixture where the head copy says ok and the base copy refuses.",
  "files": [
    ".github/workflows/test.yml",
    ".github/scripts/test-workflow.test.mjs",
    "docs/governance.md",
    "changelog.d/2099-base-policy-sign.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=4a9d3edd npm run verify (exit 0); node --test .github/scripts/test-workflow.test.mjs red-first on 4a9d3ed 22/19/3 and 22/22 at this tip", "result": "green", "at": "014b08ef23d71282b6fd150f21aa9c21c52f1670" },
  "next": "none — ready for review; a Policy-sign comment at this tip is still needed"
}
```
<!-- /flybot:checkpoint -->

Co-authored-by: FlyBot Dev-4 <dev4@flybot.local>
HugoXK added a commit to HugoXK/FlyBot that referenced this pull request Nov 3, 2026
…owLab#2120)

Fixes ThinkFlowLab#2119

Written-by: Dev3
Owner: Dev3
Reviewer: Reviewer
Policy-sign: 6603688549
Tier: T3
Claim-Id: 09f51b71c0ae414d892960db0a237d86
Branch: dev/2-review-state-stop
Base: 55503e1
Tree: 0755f1c
Verified commit: 2a5f647
Pushed head: 2a5f647
Rebased-by: Dev-2 — this rebase only; the change below is Dev-3's work
risk-tier: T3; basis: .github/scripts/enqueue-guard.mjs=T3,
.github/scripts/enqueue-guard.test.mjs=T3,
.github/scripts/enqueue-rules.mjs=T3, .github/scripts/merge-gate.mjs=T3,
.github/scripts/merge-gate.test.mjs=T3,
changelog.d/2119-review-state-stop.fixed.md=T1

## This tip replaces `3b35c545`

The same six-file change, rebased by Dev-2 onto the live `main`
`55503e16` (the previous base `4a9d3edd` was 11 commits behind it; that
is what made this pull request `dirty`). One commit, six files,
`+162/−15`, single parent `55503e16`.

One conflict, in `stopRequestAt` (`enqueue-rules.mjs`), and it is the
only line where the rebase decides anything. While this change sat,
`main` replaced `LIFT_REQUEST.test(text)` with `unconditionalLift(text)`
(ThinkFlowLab#2246/ThinkFlowLab#2287: a lift that names a condition, or that a preceding clause
withholds, answers nothing). This tip is a rewrite of the same
sentence-shaped answers, so it takes `main`'s reading: the lift that
answers a stop runs through `unconditionalLift`. Everything else in the
six files is byte-identical to the change reviewed at `3b35c545`; `git
range-diff 4a9d3ed..3b35c54 55503e1..2a5f647` shows the two columns
apart only by `main`'s own `responseOnly` filter (ThinkFlowLab#2246) and that one
predicate.

No policy signature is claimed at this tip. The one written for
`6cbb5cad` (comment `6571885896`) binds that tip — a signature binds the
tip it was written for, so a new tip needs a new signature — and it is
not carried here. A fresh `Policy-sign:` is to be issued against
`2a5f647ea` once a reviewer has signed this tip off; until then the
signature check is expected to refuse this body, and that refusal is
this paragraph's point, not a defect in the change.

## What changed

A review that requests changes is the one refusal a person writes
without typing anything, and neither gate could see it: both readers
dropped every review whose body was empty *before* mapping the rows,
`stopRequestAt` matched text only, and the review's own state never
travelled. A pull request a person had rejected answered `ok: true` at
the guard and at the queue entry.

- **`enqueue-guard.mjs` / `merge-gate.mjs`** — the review row carries
`state`, and the `.filter((v) => (v.body ?? '').trim())` in front of the
map is gone, so an empty body no longer drops the review before the stop
reader sees it. The `review-comment` rows (the diff-line face, ThinkFlowLab#2091)
keep their own filter unchanged.
- **`enqueue-rules.mjs` `stopRequestAt`** — a row with `state ===
'CHANGES_REQUESTED'` is a stop, read before any text and with or without
a body. It carries `{ id, login, at, kind: 'review', because: 'state'
}`, so the refusal reads `review <id> by <login> asked for changes at
<at>`; a state has no sentence to quote, and the text path keeps its own
wording.
- **Clearing answers the request it was written on.** The written
request and the state request are tracked apart, and the later one still
standing is what the refusal names and the reader has to answer. A later
`APPROVED` or `DISMISSED` from the same login answers the review and
clears only that; a hold written in a comment stays standing, because
the review UI is not where it was written. A later comment that lets the
tip merge clears both through the existing lift path, and an approval
from another login clears nothing.
- **`merge-gate.mjs`'s note** that the state is "deliberately not read"
now states the new rule and keeps the fact it rested on: the account
cannot approve its own pull requests, so its reviews stay `COMMENTED`
even when the body says INC (the INC on ThinkFlowLab#610 was a comment), and a
`COMMENTED` review with no body is not a stop.
- The "a body carrying a conclusion is not a request" filter and the
`after` anchor are unchanged and run before the loop: a review body that
carries a conclusion is still a conclusion, and a request older than the
conclusion the decision used still does not apply.

6 files, `+162/−15`. No existing assertion was changed.

## Red first, on the new base

Clean `55503e16`, the two test files only, `TZ=UTC node --test
.github/scripts/enqueue-guard.test.mjs
.github/scripts/merge-gate.test.mjs` — **196 tests / 191 pass / 5
fail**:

```
not ok 99 - \ThinkFlowLab#2119: a review requesting changes stops the enqueue with no body, and names the review
not ok 101 - \ThinkFlowLab#2119: a later APPROVED review by the same login clears the request, and the record says which review cleared it
not ok 102 - \ThinkFlowLab#2119: another login's approval does not answer the request, and a comment that lets the tip merge does
not ok 104 - \ThinkFlowLab#2119: a review request and its approval do not answer a request written before them
not ok 167 - \ThinkFlowLab#2119: a review requesting changes with no body refuses the queue entry
```

After the fix, the same two files on the same tree: **196 / 196 / 0**.

**The filter is its own half of the defect** (found at the previous
base, still true here): with `state` carried on both maps and the
empty-body filter left in place, the same guard/merge-gate cases fail
the same way, because the row never reaches `stopRequestAt`. That
contrast is why the fix removes the filter instead of only widening the
map.

## Second round — the overwrite the review found

Review `6570945524` (REQUEST_CHANGES at `34f72771`) and its full read
`6570870283` found one defect: a `CHANGES_REQUESTED` row assigned the
stop unconditionally, so a written hold read earlier became a state
stop, and the same login's later `APPROVED` (or a `DISMISSED`) then
cleared that state stop and took the written hold with it. Reproduced
end to end through `merge-gate`'s `main()` in queue mode: PASS, hold
comment, empty-body `CHANGES_REQUESTED`, `APPROVED` → `ok: true`,
`stopRequest: null`. Case `ThinkFlowLab#2119: a review request and its approval do
not answer a request written before them` (the `not ok 104` above) pins
it.

The fix keeps the two requests apart and returns the later one still
standing. The test starts from a text stop, which no earlier case did:
the later review is named while it stands; its approval answers only the
review and the written hold is still the request that stands; a third
person's hold is untouched by that approval; a dismissal behaves the
same way. The comment above the clearing rule says what the code does: a
written request is answered in writing, a state request by the review it
was submitted on, and a lift comment answers both.

## Verification at the tip

`2a5f647eac5dfdf785e86295dbce389b86f67c56`, tree
`0755f1ca7389124beb8900aeb6b7613379e7bbf1`, single parent
`55503e16b359f034d71328f920aae6bbf674e0d1`. Ran on a clean, committed
tree (`git status --porcelain` empty):

`TZ=UTC ARCH_DOCS_BASE=55503e16b359f034d71328f920aae6bbf674e0d1 npm run
verify` — **exit 0**:

- core **2700**, `plugins/social` **570**, `plugins/tree` **132**,
`plugins/timeline` **140**, `plugins/workflow` **411**,
`.github/scripts` **640**, `interop` **37**, `live-e2e` **26** — **4656
/ 4656 / 0**
- lint: core 74 warnings / 0 errors; plugins 36 / 9 / 2 / 16 warnings /
0 errors; interop 1 warning / 0 errors; live-e2e 0 / 0
- `arch-docs: OK (6 changed file(s))`
- ds-bot: `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00,
v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ +
patches/ds-bot/`, `patches/ds-bot/ is what generate writes (generate is
a no-op)`; `generate` run twice after the commit, `git status
--porcelain` empty both times
- `build-client --check` exits 0

The `.github/scripts` suite alone, at this tip: 640 tests, 640 pass, 0
fail.

Not run (and why): no live GitHub API call and no real merge queue —
`decide`/`guard` are driven through the fixtures both suites already
use, so the refusal is proven against the shapes the API returns, not
against a live queue entry.

## Scope

- Not touched: `policy/gates.json` and `policy-sign.mjs` (ThinkFlowLab#2095/ThinkFlowLab#2099
hold that pair), the diff-line `review-comment` row, the text stop
vocabulary, and the conclusion path.
- Residual, stated rather than solved: an approval from a different
login is not read as clearing another reviewer's request, matching
GitHub's own rule; the test pins it.

## Checkpoint

<!-- flybot:checkpoint -->
```json
{
  "pr": 2120,
  "claim": "Dev3",
  "issue": 2119,
  "summary": "A review requesting changes stopped nothing: both readers dropped every review with an empty body, the stop reader matched text only, and the state never travelled, so a rejected pull request answered ok. The state now travels from enqueue-guard and merge-gate, and a CHANGES_REQUESTED review stops the merge and names the review. Written stops and state stops are read apart, so a later approval answers the review and leaves a hold written in a comment standing. Tests pin the stop, the COMMENTED shape, the clearings, the surviving hold, and the queue entry.",
  "files": [
    ".github/scripts/enqueue-rules.mjs",
    ".github/scripts/enqueue-guard.mjs",
    ".github/scripts/merge-gate.mjs",
    ".github/scripts/enqueue-guard.test.mjs",
    ".github/scripts/merge-gate.test.mjs",
    "changelog.d/2119-review-state-stop.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=55503e16 npm run verify (clean tree)", "result": "green", "at": "2a5f647eac5dfdf785e86295dbce389b86f67c56" },
  "next": "issues resolved — ready for review at this tip"
}
```
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The signature gate does not cover the queue gates it enforces (enqueue-guard.mjs, enqueue-rules.mjs)

2 participants