Repository navigation
Conversation
|
@Birfy review request — tip Base is live main What changed (
Red first: base Checks at this tip: |
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.
1c8467b to
9a2b25b
Compare
|
@Birfy — new tip for review: What moved since Checks at this tip: 11 success + 1 skipped (
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. |
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
ca07de02from 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 astree ok).What was wrong
stopRequestAtreads 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#2520the 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.(comment6860674068, 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:until,till,after,once,when,unless,pending,before,only,as long as,subject to);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 requestis the offer#2520wrote, andotherwiseis not a condition on the plan. Clauses are the existingCLAUSEsplit (.;:!?—–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 readyand the Chinese forms are requests whatever the grammar around them.withoutQuotes()andunconditionalLift()are untouched, nothing is retroactive — a stop already on the record keeps its meaning — andstopPhraseIn()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, twiceThe 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:red-1.logsha256cb90f5a5db5cc47c,red-2.logsha256e19442996750245b. 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
1c8467b8on base3891b4f0with the six original cases: red 6 tests, 5 pass, 1 fail (#2520: an offer to hold on request is not a stop,actualnamingphrase: 'hold it'), logs7c1b69cf…/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.logsha25619e44d334db06ff1,green-2.logsha25652e05b1392fbe014).The nine cases say what the issue asks for, and nothing wider:
#2520offer is not a stop;Please hold it.), a plaindo 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;I held it until CI went green, then released) is not a request at all;I will hold it now; do not merge this tip— is still a stop.stopPhraseIn()scans everySTOP_REQUESTmatch; 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 to3891b4f0(#3086,Release v0.2.33) and later toca07de02(#3084merged). Neither touched a file this branch changes, so it was rebased ontoca07de02— the patch is 3 files+127/−1and every number above was taken on that tree — and the pushed head the branch now carries is9a2b25b0d2.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(tree416f670c, 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 (sha256of the log929680c875b1de47).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 --checkandnode scripts/ds-bot-patches.mjs check/canonicalran green insideverify;cd core && ARCH_DOCS_BASE=ca07de02… node scripts/check-arch-docs.mjs—arch-docs: OK (3 changed file(s)).enqueue-guardwill be red on this row forno 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 instopRequestAt.github/scripts/enqueue-rules.test.mjs(new,+65) — nine cases quoted from the record and the advisorychangelog.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" }