Skip to content

Enqueue guard reads a first-person hold announcement as a stop request (hold it in our own verdict request) - #3088

Open
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-stop-request-announced-hold
Open

HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-stop-request-announced-hold

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3083

Owner: Dev-1
Reviewer: Birfy
Written-by: Dev-1
Bot: FlyBot Dev-1
Claim-Id: 2cc7472be3d942248c74b706a84fb3e2
Tier: T3 (.github/**)
Base: ca07de0
Verified commit: a1c260601a7677704ade8f3fc349d233a75f9ec3
Tree: 416f670c7a450f1ba5d302d5c88d45f439568bf0
Pushed head: 9a2b25b

The push path rebuilds the commit with parent ca07de02 from this tree, so the pushed head is not byte-identical to the verified commit; every check below ran on the tree above, and the tree the push built is the same one (416f670c7a, printed as tree ok).

What was wrong

stopRequestAt reads a person's sentence as a request not to merge a pull request. It read one shape that is the opposite: a first-person offer to hold, offered to whoever is being addressed. On #2520 the author wrote it inside a request of his own — If this PR should wait behind other work, say so on the PR and I will hold it; otherwise it is the oldest unanswered T3 request in the queue. (comment 6860674068, 2026-11-13T17:51:54Z) — and the guard took the hold phrase in the second half of that sentence as the author stopping his own pull request. The author's answer then had to be written as a second, separate sentence to be read as a lift.

The distinction is grammatical, not lexical. Announcing a plan to hold, in the first person, is the writer offering to hold if asked; it is not the request that this tip must not merge.

The rule

A matched hold-family phrase is not read as a stop when the same clause opens it with a first-person future or intent auxiliary — I will, I'll, we'll, I shall, I would, I'd, I'm going to — and no condition qualifies the plan. The condition is looked for on both sides of the clause bound, because an em dash, a semicolon and a full stop end the clause without ending the sentence:

  • in the rest of the phrase's own clause (until, till, after, once, when, unless, pending, before, only, as long as, subject to);
  • or in a following clause that opens with one of those words — I will hold it — until the second reader signs off, I will hold it; pending the second reader, I will hold it. Until the second reader signs off, this waits.

Only an opener counts in the next clause: … I will hold it; otherwise it is the oldest unanswered request is the offer #2520 wrote, and otherwise is not a condition on the plan. Clauses are the existing CLAUSE split (. ; : ! ? — – line break), so commas do not chop the offer in two.

Only the hold family (HOLD_FAMILY) is read this way; do not merge, not ready and the Chinese forms are requests whatever the grammar around them. withoutQuotes() and unconditionalLift() are untouched, nothing is retroactive — a stop already on the record keeps its meaning — and stopPhraseIn() scans every hold-family match, not just the first, so an announcement can never hide a request written behind it.

The cost is stated in the source next to the rule: a bare I will hold it. with no condition reads as a statement rather than a request, which is the shape the exclusion is for.

Red first, at ca07de02, twice

The three cases for the condition after the bound were added with the source at the pre-rework state: node --test .github/scripts/enqueue-rules.test.mjs → 9 tests, 6 pass, 3 fail, exit 1, both runs:

not ok 7 - #3088: a plan whose condition follows the phrase is still a stop — em dash
  error: 'Expected "actual" to be strictly unequal to: null'
not ok 8 - #3088: a plan whose condition follows the phrase is still a stop — semicolon
not ok 9 - #3088: a plan whose condition follows the phrase is still a stop — full stop

red-1.log sha256 cb90f5a5db5cc47c, red-2.log sha256 e19442996750245b. The reading was a real regression against the base: a plan that says when it ends is an undertaking, and it was going quiet.

The rows opened at 1c8467b8 on base 3891b4f0 with the six original cases: red 6 tests, 5 pass, 1 fail (#2520: an offer to hold on request is not a stop, actual naming phrase: 'hold it'), logs 7c1b69cf…/90ffe904…; green after the first version 6/6/0 (d8a9f34a…/54e0e848…).

Green after the fix, twice

Same command on the tip → 9 tests, 9 pass, 0 fail, exit 0, both runs (green-1.log sha256 19e44d334db06ff1, green-2.log sha256 52e05b1392fbe014).

The nine cases say what the issue asks for, and nothing wider:

  • the #2520 offer is not a stop;
  • an imperative (Please hold it.), a plain do not merge this tip, and a plan whose condition is in its own clause (I will hold it until the second reader signs off) are stops;
  • a plan whose condition follows the bound is a stop — em dash, semicolon, full stop;
  • a past hold (I held it until CI went green, then released) is not a request at all;
  • the case that must stay — I will hold it now; do not merge this tip — is still a stop. stopPhraseIn() scans every STOP_REQUEST match; a reader that stopped at the first match would let a real request written behind an announcement through, which is the one brake a person has. Removing that case would pin only half of the behaviour.

Base moved, then a rework

This branch was READY on b0ffd54f; main then moved to 3891b4f0 (#3086, Release v0.2.33) and later to ca07de02 (#3084 merged). Neither touched a file this branch changes, so it was rebased onto ca07de02 — the patch is 3 files +127/−1 and every number above was taken on that tree — and the pushed head the branch now carries is 9a2b25b0d2.

The rework is the advisory's finding: the first version looked for the condition only inside the phrase's own clause, so a plan whose condition followed an em dash, a semicolon or a full stop was suppressed and a real stop was swallowed. The next-clause opener closes that gap; the three red rows above are the three shapes.

Gates, at a1c26060 (tree 416f670c, the pushed tree)

TZ=UTC npm run verify → exit 0: core 3248, social 684, tree 149, timeline 172, workflow 535, project 147, gate/policy 687, interop 37, live-e2e 34 — 0 fail everywhere (sha256 of the log 929680c875b1de47).

Also run on this tree: node .github/scripts/changelog.mjs check — 2 fragments well formed; node .github/scripts/testing-layers.mjs --check — every test file and every workflow classified; node .github/scripts/check-mojibake.mjs --check — 0 occurrences of U+FFFD in 1707 text files; node .github/scripts/check-pack.mjs — exit 0; node core/scripts/build-client.mjs --check and node scripts/ds-bot-patches.mjs check/canonical ran green inside verify; cd core && ARCH_DOCS_BASE=ca07de02… node scripts/check-arch-docs.mjs — arch-docs: OK (3 changed file(s)).

enqueue-guard will be red on this row for no Verifier report names the tip, the expected state of a T3 pull request before the verdict lands. Policy-sign: is not in this body: the row changes gate files, so the queue needs a signature comment by an account that is neither the author nor this team's bot, and that comment is not mine to write.

Files

3 files, +127/−1:

  • .github/scripts/enqueue-rules.mjs (+62/−1) — HOLD_FAMILY, ANNOUNCED_HOLD, HOLD_CONDITION, HOLD_CONDITION_OPENER, clauseBounds(), nextClause(), announcedHoldAt(), stopPhraseIn(), and the one call site in stopRequestAt
  • .github/scripts/enqueue-rules.test.mjs (new, +65) — nine cases quoted from the record and the advisory
  • changelog.d/3083-stop-request-announced-hold.fixed.md (new)

Checkpoint

{
  "pr": 3088,
  "claim": "Dev-1",
  "issue": 3083,
  "summary": "stopRequestAt read a first-person offer to hold as the author's own stop, so the #2520 sentence that offered to hold parked its own pull request. The hold family now reads as an announcement when a first-person future or intent auxiliary opens the phrase and no condition qualifies the plan. The condition is looked for in the same clause and in a following clause that opens with one, because an em dash, a semicolon or a full stop ends the clause but not the sentence. Imperatives, do not merge, not ready and the Chinese forms are untouched, nothing is retroactive, and every match is scanned.",
  "files": [
    ".github/scripts/enqueue-rules.mjs",
    ".github/scripts/enqueue-rules.test.mjs",
    "changelog.d/3083-stop-request-announced-hold.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC npm run verify; node --test .github/scripts/enqueue-rules.test.mjs (red x2 before, green x2 after, then red/green again for the advisory rework); build-client --check; ds-bot-patches check/canonical; changelog, testing-layers, check-mojibake, check-pack; arch-docs", "result": "green", "at": "9a2b25b0d20cf603db8de18b77a855d6daee59b9" },
  "next": "verdict at the pushed head; the signature line is not mine to write"
}

@HugoXK

HugoXK commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor Author

@Birfy review request — tip 1c8467b8683b37f71082d36d6bab1559ee001c45 (T3, Fixes #3083).

Base is live main 3891b4f07701; the branch was rebased onto it before the push, and every number below was re-run on the rebased tree.

What changed (.github/scripts/enqueue-rules.mjs, +47/−1, plus enqueue-rules.test.mjs +50 and one changelog fragment):

  • a first-person announcement of the very thing the queue wants ("I will it now") is no longer read as a stop request: new ANNOUNCED_HOLD / HOLD_CONDITION handling around clauseBounds() / announcedHoldAt(), consumed inside stopPhraseIn();
  • a genuine instruction still counts as a stop, and because stopPhraseIn() now scans every match, an announcement earlier in a comment cannot mask a real instruction later in the same comment — the sixth case in .github/scripts/enqueue-rules.test.mjs pins exactly that pairing;
  • trigger in the wild: #2520 issue comment 6860674068 (2026-11-13T17:51:54Z, HugoXK) — its closing sentence is such an announcement, and it froze that pull request's queue slot.

Red first: base enqueue-rules.mjs + the new test file, twice # tests 6 / # pass 5 / # fail 1 (the failing case is the #2520 one). At this tip, twice 6/6/0. TZ=UTC npm run verify exits 0 on the rebased tree (core 3248, social 679, tree 149, timeline 172, workflow 535, project 147, gate/policy 684, interop 37, live-e2e 34); build-client --check 0; ds-bot-patches check ok / canonical no-op.

Checks at this tip: enqueue-guard red because no Verifier report names 1c8467b8683b; policy-sign red because the body carries no Policy-sign: line yet. Per AGENTS.md a Verdict is not a signature: that line has to name a comment written by a real human account that is neither this pull request's author nor a bot, and a reviewer's verdict does not satisfy it — a separate owner-side step, not something this branch can produce. plugins (tree|social|workflow|timeline) were still running when this was written; agent-audience cancelled as usual.

A first-person plan to hold is an offer to be asked, not a request that this tip must not merge, so stopRequestAt no longer reads it as one. The condition that turns the offer back into an undertaking is looked for on both sides of the clause bound: in the same clause, and in a clause that opens with one (until, pending, only after, subject to), because an em dash, a semicolon or a full stop ends the clause but not the sentence. Every hold-family match is scanned, so an announcement cannot hide a request behind it.
@HugoXK

HugoXK commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy — new tip for review: 9a2b25b0d20cf603db8de18b77a855d6daee59b9 (single commit on live main ca07de0289).

What moved since 1c8467b8: the advisory finding was that a first-person plan whose condition sits in the following clause — after an em dash, a semicolon, or a full stop — went quiet instead of stopping. That is fixed here: when the matched phrase is an announced plan, the scan also reads the clause after it, and an until|pending|only after|subject to opener counts as a condition. Three cases pin those shapes, and the every-match scan still returns a real request that follows an announcement in a later line.

Checks at this tip: 11 success + 1 skipped (ubuntu-latest · Node 24 · DSH 0.2.0-rc.2 skips on a .github-only change) + 3 red, all expected rather than defects:

  • policy-sign — a gate file changed (.github/scripts/enqueue-rules.mjs) and the body carries no Policy-sign: line; that line is a human signature, not mine to write.
  • tests-ok — aggregates exactly that same policy-sign failure ("Every job passed" red); every other job is green.
  • enqueue-guard — "no Verifier report names the tip 9a2b25b", which a Verdict at this sha clears.

Tier T3, so a Verdict naming this sha is what the queue needs; FlyBot Test's independent re-check of the three shapes is running in parallel.

@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 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Enqueue guard reads a first-person hold announcement as a stop request (hold it in our own verdict request)

1 participant