From a1844f259ad09fe9285bff306751541a42b5164d Mon Sep 17 00:00:00 2001 From: FlyBot Dev-4 Date: Tue, 3 Nov 2026 09:31:34 +0000 Subject: [PATCH 1/3] enqueue-guard: read every clause before the lift, not just one P-491: the clause immediately before the lift was the only one read, so a condition two clauses back was dropped and an answer that withholds the lift cleared the hold ("If the second reader signs off. CI is green. You may merge this tip now."). The earlier clauses are read with the opening set, without the bare negation this rule carries for the clause right next to the lift, so "I don't see any objections. Tests are green." still clears. Fixes #2291 --- .github/scripts/enqueue-guard.test.mjs | 36 ++++++++++++++++++++++++++ .github/scripts/enqueue-rules.mjs | 11 ++++++++ 2 files changed, 47 insertions(+) diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index 9ea24395..a8a05ab4 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -1396,6 +1396,42 @@ 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('#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..f0d90838 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 before the lift that was read, so a +// condition separated from the yes by one neutral clause was dropped — "If the second reader signs off. CI +// is green. You may merge this tip now." cleared the hold. Every clause further back is read with the same +// opening set, but the bare negation `WITHHELD` carries is left out here: further back it is ordinary voice +// ("I don't see any objections. Tests are green. You may merge this tip now."), and refusing those is the +// #2287 shape the one-clause read was already narrowed against. `not until` / `not unless` stay words of +// their own, and a leading "Not" still falls through to the opening test, so they keep withholding. +const WITHHELD_FAR = /\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)\b/i +const withholdsFurtherBack = (clause) => (WITHHELD_FAR.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 + 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 From aa424db246a8c294a9d78c47ce86baeac60827bf Mon Sep 17 00:00:00 2001 From: FlyBot Dev-4 Date: Tue, 3 Nov 2026 09:43:26 +0000 Subject: [PATCH 2/3] enqueue-guard: narrow the preceding-clause words to conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Product's review found the far read refused three ordinary yeses — "After the fix, …", "It should be fine. …", "Following the merge, …" — because a time or sequencing phrase before the yes was read as a condition on it. The clause next to the lift and the clauses further back now share one vocabulary of waiting and condition words, so the two positions cannot drift apart again; "following the merge" opens one of them and clears. Fixes #2291 --- .github/scripts/enqueue-guard.test.mjs | 12 ++++++++ .github/scripts/enqueue-rules.mjs | 42 ++++++++++++-------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index a8a05ab4..384f0c51 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -1432,6 +1432,18 @@ test('#2291 n1-n4: a negation further back than the lift still 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. 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`) + } +}) + 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 f0d90838..cea88e86 100644 --- a/.github/scripts/enqueue-rules.mjs +++ b/.github/scripts/enqueue-rules.mjs @@ -227,34 +227,32 @@ const WITHHELD = /\b(?:only (?:after|once|when)|not until|not unless|pending|sub 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, '')) -// P-491 (FlyBot Lead, 2026-11-03): `liftAt - 1` was the only clause before the lift that was read, so a -// condition separated from the yes by one neutral clause was dropped — "If the second reader signs off. CI -// is green. You may merge this tip now." cleared the hold. Every clause further back is read with the same -// opening set, but the bare negation `WITHHELD` carries is left out here: further back it is ordinary voice -// ("I don't see any objections. Tests are green. You may merge this tip now."), and refusing those is the -// #2287 shape the one-clause read was already narrowed against. `not until` / `not unless` stay words of -// their own, and a leading "Not" still falls through to the opening test, so they keep withholding. -const WITHHELD_FAR = /\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)\b/i -const withholdsFurtherBack = (clause) => (WITHHELD_FAR.test(clause) && !LEADING_NEGATION.test(clause)) - || WITHHELD_OPENING.test(clause.replace(LEADING_NEGATION, '')) +// second reader signs off — you may merge this tip now". P-491 (FlyBot Lead, 2026-11-03): `liftAt - 1` was +// also the only clause read, so the same body with one neutral clause in between ("If the second reader signs +// off. CI is green. You may merge this tip now.") cleared it. Every clause before the lift is read with the +// same vocabulary, so the two positions cannot drift apart again. +// +// `not` and `only` open ordinary prose before a yes ("Not sure — you may merge"), so this set leaves the bare +// negation out: further back it is the answer's own voice ("I don't see any objections. Tests are green. You +// may merge this tip now."), and refusing that is the #2287 shape. `not until` / `not unless` stay words of +// their own. +// +// A time or sequencing phrase is not a condition on the yes either: "After the fix, all green.", "It should +// be fine." and "Following the merge, all green." are ordinary prose (FlyBot Lead, 2026-11-03, after +// Product's review), while "Once …, Thanks." and "Pending …" wait on something. So this set keeps the waiting +// and condition words — `only after`, `not until`, `pending`, `provided`, `contingent on` … — and drops +// `after`, `before`, `when`, `until`, `upon`, `whenever`, `should`, `prior to`, `following`; the opening test +// is narrowed to `if`, `unless` and `once` for the same reason. +const WITHHELD_FAR = /\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 FAR_OPENING = /^[\s>*+\-–—(\[]*(?:if|unless|once(?!\s+(?:more|again)\b))\b/i +const withholdsBefore = (clause) => WITHHELD_FAR.test(clause) || FAR_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 + if (liftAt > 1 && clauses.slice(0, liftAt - 1).some(withholdsBefore)) return false let afterLift = false for (const clause of clauses) { if (afterLift && (WITHHELD.test(clause) || WITHHELD_LEADING.test(clause))) return false From dce22dce574297f168e792b2ed43fa94137c7377 Mon Sep 17 00:00:00 2001 From: Dev-4 Date: Tue, 3 Nov 2026 10:14:29 +0000 Subject: [PATCH 3/3] enqueue-guard: keep the near clause reading, narrow only the clauses further back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clause next to the lift lost the bare negation and the temporal openings when the far-clause vocabulary was merged into it, so five answers that merged main reads as withheld cleared a standing hold ("The second reader has not looked at it yet — you may merge this tip now."). Read the clause at liftAt - 1 exactly as main does, and give only the clauses before it the narrower set: waiting and condition words plus if/unless/until/ once, no bare negation, no time or sequencing openings. The five cases are tests of their own, red on the previous tip. --- .github/scripts/enqueue-guard.test.mjs | 21 ++++++++++++- .github/scripts/enqueue-rules.mjs | 42 ++++++++++++++------------ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index 384f0c51..2d440087 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -1436,7 +1436,7 @@ test('#2291 n5-n7: a time or sequencing phrase further back than the lift still 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. You may merge this tip now.', '2026-11-03T09:45:02Z'], + [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('; ')}`) @@ -1444,6 +1444,25 @@ test('#2291 n5-n7: a time or sequencing phrase further back than the lift still } }) +// 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 cea88e86..087ba6e7 100644 --- a/.github/scripts/enqueue-rules.mjs +++ b/.github/scripts/enqueue-rules.mjs @@ -227,32 +227,34 @@ const WITHHELD = /\b(?:only (?:after|once|when)|not until|not unless|pending|sub 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". P-491 (FlyBot Lead, 2026-11-03): `liftAt - 1` was -// also the only clause read, so the same body with one neutral clause in between ("If the second reader signs -// off. CI is green. You may merge this tip now.") cleared it. Every clause before the lift is read with the -// same vocabulary, so the two positions cannot drift apart again. -// -// `not` and `only` open ordinary prose before a yes ("Not sure — you may merge"), so this set leaves the bare -// negation out: further back it is the answer's own voice ("I don't see any objections. Tests are green. You -// may merge this tip now."), and refusing that is the #2287 shape. `not until` / `not unless` stay words of -// their own. -// -// A time or sequencing phrase is not a condition on the yes either: "After the fix, all green.", "It should -// be fine." and "Following the merge, all green." are ordinary prose (FlyBot Lead, 2026-11-03, after -// Product's review), while "Once …, Thanks." and "Pending …" wait on something. So this set keeps the waiting -// and condition words — `only after`, `not until`, `pending`, `provided`, `contingent on` … — and drops -// `after`, `before`, `when`, `until`, `upon`, `whenever`, `should`, `prior to`, `following`; the opening test -// is narrowed to `if`, `unless` and `once` for the same reason. -const WITHHELD_FAR = /\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 FAR_OPENING = /^[\s>*+\-–—(\[]*(?:if|unless|once(?!\s+(?:more|again)\b))\b/i -const withholdsBefore = (clause) => WITHHELD_FAR.test(clause) || FAR_OPENING.test(clause) +// 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, '')) +// 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(withholdsBefore)) 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