From c9547dd1f609991d77a56e329c8605ca8b365f9c Mon Sep 17 00:00:00 2001 From: FlyBot Date: Tue, 3 Nov 2026 09:05:11 +0000 Subject: [PATCH] enqueue-guard: read the clause before the lift too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lift comment clears an enqueue-guard stop only when it is unconditional, and a condition can sit on either side of the yes. The clause after the lift is read with the full withheld vocabulary and a bare negation at any position (FlyBot Test on #2288); the clause before it, which was read only when it carried a lift phrase of its own, now gets the same vocabulary plus a narrow opening set, so "If the second reader signs off — you may merge this tip now" keeps the hold. not/only stay out of that opening set ("Not sure — you may merge" is a yes) and once more / once again are excluded. Eight before-the-lift bodies and four counterexamples join the seven after-the-lift ones, each its own test. Fixes #2287 Written-by: Dev-4 --- .github/scripts/enqueue-guard.test.mjs | 56 ++++++++++++++++++++++++++ .github/scripts/enqueue-rules.mjs | 22 +++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index ec3eb531d..9ea24395b 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -1340,6 +1340,62 @@ test('#2287 k10-k15: a condition in the clause after the lift keeps the hold', ( } }) +// FlyBot Test on #2288, 2026-11-03: the clause-after vocabulary still missed seven ways a yes is made +// conditional, and a bare negation inside that clause ("has not re-run it") was read only at its start. +// Each is its own test so a hole in the vocabulary fails by name. +for (const [id, body, at] of [ + [16, "You may merge this tip — upon the second reader's approval.", '2026-11-03T08:20:00Z'], + [17, 'You may merge this tip — whenever the second reader agrees.', '2026-11-03T08:20:01Z'], + [18, 'You may merge this tip — should the second reader agree.', '2026-11-03T08:20:02Z'], + [19, 'You may merge this tip — contingent on the second reader.', '2026-11-03T08:20:03Z'], + [20, "You may merge this tip — following the second reader's re-run.", '2026-11-03T08:20:04Z'], + [21, 'You may merge this tip — on condition that the second reader agrees.', '2026-11-03T08:20:05Z'], + [22, 'You may merge this tip — the second reader has not re-run it.', '2026-11-03T08:20:06Z'], +]) { + test(`#2287 k${id}: a yes conditional in the clause after the lift keeps the hold`, () => { + const r = run({ comments: [comment(report(), '2026-11-03T05:00:00Z', { id: 6596712952 }), note({ id: 6596354014, at: '2026-11-03T05:38:25Z' }), note({ id: 6597499990, body, at })] }) + assert.equal(r.ok, false, `k${id} withholds the answer: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest?.id, 6596354014, `k${id} keeps the original hold`) + }) +} + +// P-489 (Product, 2026-11-03): the clause BEFORE the lift was read only when it carried a lift phrase of +// its own, so a condition split off with `—`, `:`, `.` or a line break sat one clause away and the yes +// cleared the hold it withholds — the mirror of k10-k15. Each case is its own test so a hole in the opening +// set fails by name. +for (const [id, body, at] of [ + [23, 'If the second reader signs off — you may merge this tip now.', '2026-11-03T09:10:00Z'], + [24, 'Only after the second reader re-runs. You may merge this tip.', '2026-11-03T09:10:01Z'], + [25, 'Once the second reader re-runs: you may merge this tip.', '2026-11-03T09:10:02Z'], + [26, 'Pending the second reader — you may merge this tip now.', '2026-11-03T09:10:03Z'], + [27, 'Provided the second reader lands. You may merge this tip.', '2026-11-03T09:10:04Z'], + [28, 'Unless the second reader objects — you may merge this tip now.', '2026-11-03T09:10:05Z'], + [29, 'Not until the second reader re-runs — you may merge this tip now.', '2026-11-03T09:10:06Z'], + [30, 'Only after the second reader re-runs\nyou may merge this tip.', '2026-11-03T09:10:07Z'], +]) { + test(`#2287 k${id}: a condition in the clause before the lift keeps the hold`, () => { + const r = run({ comments: [comment(report(), '2026-11-03T05:00:00Z', { id: 6596712952 }), note({ id: 6596354014, at: '2026-11-03T05:38:25Z' }), note({ id: 6597499990, body, at })] }) + assert.equal(r.ok, false, `k${id} withholds the answer: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest?.id, 6596354014, `k${id} keeps the original hold`) + }) +} + +// The other direction, so the new read cannot swallow an ordinary yes: a clause before the lift that opens +// with an ordinary word, or with a negation that is the answer's own voice ("Not sure"), is not a condition +// on it. `once more` / `once again` are timing, not a clause of their own. +test('#2287 c9-c12: a preceding clause that is not a condition still clears the hold', () => { + for (const [i, body, at] of [ + [9, 'Not sure — you may merge this tip now.', '2026-11-03T09:10:10Z'], + [10, 'Once more: you may merge this tip.', '2026-11-03T09:10:11Z'], + [11, 'Re-read once more — you may merge this tip.', '2026-11-03T09:10:12Z'], + [12, 'All green after the fix — you may merge this tip.', '2026-11-03T09:10:13Z'], + ]) { + const r = ordinaryYes(body, at) + assert.equal(r.ok, true, `c${i} is an ordinary yes: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest, null, `c${i} clears the hold`) + } +}) + test('#2287: a conditional lift with no hold wording of its own keeps the hold', () => { const conditional = note({ id: 6597499999, diff --git a/.github/scripts/enqueue-rules.mjs b/.github/scripts/enqueue-rules.mjs index b43f0c266..01aab243c 100644 --- a/.github/scripts/enqueue-rules.mjs +++ b/.github/scripts/enqueue-rules.mjs @@ -218,12 +218,32 @@ const CLAUSE = /[.;:!?—–\n]/ // answer when it carries a phrase that never sits in a plain yes, or when it opens with a condition word. // Ordinary trailing talk still clears: "you may merge this tip now; I am back after lunch if anything // breaks" opens with "I", which is why the broad vocabulary above is not used here. -const WITHHELD = /\b(?:only (?:after|once|when)|not until|not unless|pending|subject to|provided|as (?:long|soon) as|assuming|in case|depending on)\b/i +// FlyBot Test on #2288 (2026-11-03) found seven more of those phrases clearing the hold — "upon the second +// reader's approval", "whenever …", "should …", "contingent on …", "following …", "on condition that …" — +// plus a bare negation anywhere in the clause ("the second reader has not re-run it"), which the +// opening-word rule read only at its start. `no objections` and `nothing else waits on it` stay clear: a +// word boundary keeps `not` inside them from matching. +const WITHHELD = /\b(?:only (?:after|once|when)|not until|not unless|pending|subject to|provided|as (?:long|soon) as|assuming|in case|depending on|upon|whenever|should|contingent (?:on|upon)|dependent (?:on|upon)|prior to|following|in the event|on condition that|not|never)\b|n['’]t\b/i const WITHHELD_LEADING = /^[\s>*+\-–—(\[]*(?:only|after|once|until|unless|if|when|before|pending|not|never|cannot|n['’]t|provided|providing|assuming)\b/i +// P-489 (Product, 2026-11-03): the clause *before* the lift is read too. A condition split off with `—`, `:`, +// `.` or a line break sat one clause away from the yes, and the answer cleared a hold it withholds: "If the +// second reader signs off — you may merge this tip now". This opening set is narrower than `WITHHELD` because +// `not` and `only` open ordinary prose here ("Not sure — you may merge"), while a negation anywhere else in +// that clause still withholds it — the reading `WITHHELD` gives a clause after the lift. `once more` and +// `once again` are timing, not a condition, so the lookahead keeps them out. +const WITHHELD_OPENING = /^[\s>*+\-–—(\[]*(?:if|unless|provided|providing|assuming|pending|when|after|before|until|once(?!\s+(?:more|again)\b))\b/i +// A leading negation there is the answer's own voice ("Not sure", "Never mind"), not a condition on it; the +// same word anywhere else in the clause is one. So a negation first in the clause falls through to the +// opening-word test with it removed, which still catches `not until` / `not unless` there. +const LEADING_NEGATION = /^[\s>*+\-–—(\[]*(?:not|never|cannot|n['’]t)\b/i +const withholdsBefore = (clause) => (WITHHELD.test(clause) && !LEADING_NEGATION.test(clause)) + || WITHHELD_OPENING.test(clause.replace(LEADING_NEGATION, '')) const unconditionalLift = (text) => { if (!LIFT_REQUEST.test(text) || STOP_REQUEST.test(text) || HOLD_LINE.test(text)) return false const clauses = text.split(CLAUSE) if (clauses.some((clause) => LIFT_REQUEST.test(clause) && CONDITIONAL.test(clause))) return false + const liftAt = clauses.findIndex((clause) => LIFT_REQUEST.test(clause)) + if (liftAt > 0 && withholdsBefore(clauses[liftAt - 1])) return false let afterLift = false for (const clause of clauses) { if (afterLift && (WITHHELD.test(clause) || WITHHELD_LEADING.test(clause))) return false