diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index 9ea24395..2d440087 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -1396,6 +1396,73 @@ test('#2287 c9-c12: a preceding clause that is not a condition still clears the } }) +// P-491 (FlyBot Lead, 2026-11-03): the clause before the lift is read at exactly one position, so a +// condition two clauses back is dropped — "If the second reader signs off. CI is green. You may merge this +// tip now." cleared the hold. Each case is its own test so the position fails by name. +for (const [id, body, at] of [ + [1, 'If the second reader signs off. CI is green. You may merge this tip now.', '2026-11-03T09:20:00Z'], + [2, 'Only after the second reader re-runs — looks good. You may merge this tip.', '2026-11-03T09:20:01Z'], + [3, 'Once the second reader re-runs. Thanks. You may merge this tip.', '2026-11-03T09:20:02Z'], + [4, 'Pending the second reader. Nothing else is open. You may merge this tip now.', '2026-11-03T09:20:03Z'], + [5, 'Unless the second reader objects we wait. Looks fine. You may merge this tip now.', '2026-11-03T09:20:04Z'], + [6, 'Provided the second reader lands — I am around. You may merge this tip now.', '2026-11-03T09:20:05Z'], + [7, 'Not until the second reader re-runs. We wait. You may merge this tip now.', '2026-11-03T09:20:06Z'], + [8, 'If the second reader signs off.\nCI is green.\nyou may merge this tip.', '2026-11-03T09:20:07Z'], +]) { + test(`#2291 g${id}: a condition two clauses 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, `g${id} withholds the answer: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest?.id, 6596354014, `g${id} keeps the original hold`) + }) +} + +// The other direction: a negation further back than the lift is the answer's own voice, not a condition on +// it. Reading `WITHHELD`, bare negation included, over every earlier clause refused all four — the #2287 +// shape, seven of eight ordinary answers refused on `7bd71df5`. +test('#2291 n1-n4: a negation further back than the lift still clears the hold', () => { + for (const [i, body, at] of [ + [1, "I don't see any objections. Tests are green. You may merge this tip now.", '2026-11-03T09:20:10Z'], + [2, 'I was not able to break it. All green. You may merge this tip now.', '2026-11-03T09:20:11Z'], + [3, 'I could not find a hole in it. Ship it. You may merge this tip now.', '2026-11-03T09:20:12Z'], + [4, 'Nothing blocking from me. CI is green. You may merge this tip now.', '2026-11-03T09:20:13Z'], + ]) { + const r = ordinaryYes(body, at) + assert.equal(r.ok, true, `n${i} is an ordinary yes: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest, null, `n${i} clears the hold`) + } +}) + +test('#2291 n5-n7: a time or sequencing phrase further back than the lift still clears', () => { + for (const [i, body, at] of [ + [5, 'After the fix, all green. CI is green. You may merge this tip now.', '2026-11-03T09:45:00Z'], + [6, 'It should be fine. Tests are green. You may merge this tip now.', '2026-11-03T09:45:01Z'], + [7, 'Following the merge, all green. CI is green. You may merge this tip now.', '2026-11-03T09:45:02Z'], + ]) { + const r = ordinaryYes(body, at) + assert.equal(r.ok, true, `n${i} is an ordinary yes: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest, null, `n${i} clears the hold`) + } +}) + +// The narrowed vocabulary is for the clauses further back only. Read next to the lift (the aa424db tip) it +// dropped the bare negation and the temporal openings from a clause merged main still reads them in, and these +// five — every one STOP on 6e5f79ade5 — cleared. They are the floor this fix may not dig under. +test('#2291 k1-k5: a condition next to the lift keeps the reading merged main gives it', () => { + for (const [i, body, at] of [ + [1, 'The second reader has not looked at it yet — you may merge this tip now.', '2026-11-03T10:00:00Z'], + [2, "The second reader hasn't signed off — you may merge this tip now.", '2026-11-03T10:00:01Z'], + [3, 'The second reader never re-ran it — you may merge this tip now.', '2026-11-03T10:00:02Z'], + [4, 'When the second reader signs off — you may merge this tip now.', '2026-11-03T10:00:03Z'], + [5, 'After the second reader re-runs — you may merge this tip now.', '2026-11-03T10:00:04Z'], + ]) { + const r = run({ + comments: [comment(report(), '2026-11-03T05:00:00Z', { id: 6596712952 }), note({ id: 6596354014, at: '2026-11-03T05:38:25Z' }), note({ id: 6597500100 + i, body, at })], + }) + assert.equal(r.ok, false, `k${i} withholds the answer: ${r.reasons.join('; ')}`) + assert.equal(r.stopRequest?.id, 6596354014, `k${i} keeps the original 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 01aab243..087ba6e7 100644 --- a/.github/scripts/enqueue-rules.mjs +++ b/.github/scripts/enqueue-rules.mjs @@ -238,12 +238,23 @@ const WITHHELD_OPENING = /^[\s>*+\-–—(\[]*(?:if|unless|provided|providing|as 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, '')) +// P-491 (FlyBot Lead, 2026-11-03): `liftAt - 1` was the only clause read, so a condition with one neutral +// clause in between ("If the second reader signs off. CI is green. You may merge this tip now.") cleared the +// hold. The clause at `liftAt - 1` keeps the reading above; the ones further back get a narrower one, because +// there a bare negation is the answer's own voice ("I don't see any objections. Tests are green. You may merge +// this tip now.") and a time or sequencing phrase is ordinary prose ("After the fix, all green.", "It should +// be fine.") — refusing those is the #2287 shape. So these read the waiting and condition words only, with +// `if`, `unless`, `until` and `once` as openings, and `only after` / `not until` still count as words. +const WITHHELD_FURTHER = /\b(?:only (?:after|once|when)|not until|not unless|pending|subject to|provided|providing|as (?:long|soon) as|assuming|in case|depending on|contingent (?:on|upon)|dependent (?:on|upon)|in the event|on condition that)\b/i +const FURTHER_OPENING = /^[\s>*+\-–—(\[]*(?:if|unless|until|once(?!\s+(?:more|again)\b))\b/i +const withholdsFurtherBack = (clause) => WITHHELD_FURTHER.test(clause) || FURTHER_OPENING.test(clause) 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 + if (liftAt > 1 && clauses.slice(0, liftAt - 1).some(withholdsFurtherBack)) return false let afterLift = false for (const clause of clauses) { if (afterLift && (WITHHELD.test(clause) || WITHHELD_LEADING.test(clause))) return false