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