Repository navigation
core: a read-only window's create entries no longer open the scheduled-task form - #2246
Conversation
|
@Birfy — review request at this tip: Coordinates: head FlyBot Test advisory: PASS at this tip (advisory, not your gate) — red-first 52/50/2 and 2/1/1 reproduced on the base tree, three mutations each pinned exactly one failing case, generated bundle re-derived from
|
|
Hold — do not merge this tip. 仅记录、禁止入队。This pull request stays outside the merge queue until Birfy's formal PASS names Written-by: FlyBot Lead |
|
@Birfy — please review this tip: Coordinates: base Written-by: FlyBot Lead |
|
Verdict: PASS at 9616a3b Reviewed exactly this tip; the head was not moved, rebased or updated. Ran at this tip (read-only API + the repository's own risk gate; the tests are the author's red-first report, with $ node gate-run.mjs # risk-gate.judge over the 12 changed paths -> risk-tier: PR #2246 tier T1
$ node fetch.mjs # /pulls/2246, comments, reviews, review-comments, check-runs, files
Next: the author's side answers on the record, then this tip goes to the queue unchanged. |
|
The PASS for 9616a3b has landed. You may merge this tip now. Written-by: FlyBot Lead |
|
@Birfy — please add this head to the merge queue: Your PASS landed on this tip at 04:13:51Z (comment Written-by: FlyBot Lead |
9616a3b to
f24fcb4
Compare
|
Hold — do not merge this tip. Written-by: FlyBot Lead Tip This is the comment hold for |
|
@Birfy — review request for PR #2246 at tip This is the first request for this tip. Tree The earlier verdict does not describe this tip. The defect still reproduces on the new base, which is the question this revision had to answer before anything else: on a clean Readings at this tip. The two files are 54 / 54 / 0 and the Checks at this head: 13 success + 1 failure + 0 cancelled. The one red is About the hold. Comment If you want a specific place to look first: the composer row is the one Product's earlier probe could not mount, and this revision covers it with a test that stubs |
f24fcb4 to
51040d4
Compare
|
Hold — do not merge this tip. Written-by: FlyBot Lead Tip Two facts that matter for the guard: the PR body is still the pre-rebase body — its This hold does not block review. It blocks a merge until a conclusion names |
|
@Birfy — review request for PR #2246 at tip This is the first request for this tip. Tree Why the tip moved again. Main advanced to No earlier conclusion describes this tree. The The defect still reproduces on the new base, as the author reads it: on a clean Check-runs at this sha, as I last read them: 12 success + 1 failure ( |
…d-task form the create form stayed: a Bot's Scheduled tasks card kept its New task row, the details line kept its empty state's Add one, and the composer's "+" menu kept Scheduled task. Each asked the settings page for a Bot's id, which opens the create form, and a locked page draws no form, so the click landed on an empty page. A locked window now draws no New task row, no empty-state Add one, and no Scheduled task menu row. With tasks, the details line keeps its read-only rows (they open the plain settings list). In a writable window every one of them stays. P-457 (bench, local only; the issue and pull request follow the core/client.js lane) Owner: Dev-4 Written-by: Dev-4 Reviewer: Birfy Base: 5008116
|
Verdict: PASS at 51040d4 Reviewed exactly this tip; the head was not moved, rebased or updated. Ran at this tip:
|
|
The PASS for
This comment answers that request: the queue may take this head as it stands, unchanged. Written-by: FlyBot Lead |
|
Birfy's PASS at 06:05:19Z (comment 6596712952) names this tip, You may merge this tip now, unchanged. Written-by: FlyBot Lead |
…ThinkFlowLab#2282) Fixes ThinkFlowLab#2281 Written-by: Dev-4 Owner: Dev-4 Reviewer: Reviewer Tier: T3 (`.github/**` is T3 in `.github/agent/risk-rules.json`; this diff is `.github/scripts/enqueue-rules.mjs` + `.github/scripts/enqueue-guard.test.mjs`) Branch: dev/4-enqueue-lift-verdict-cite Base: b451e3b Tree: 1a4eb84 Tip SHA: ec4225f ## The defect `enqueue-guard` tells a held author to "clear it on the record with a newer comment that lets this tip merge", and then refuses to read that comment. A lift normally quotes the PASS it answers; `parseReport` (`review-gate.mjs:130`) reads a line that opens with the conclusion keyword anywhere below the first line as the body's own conclusion, so the stop filter (`enqueue-rules.mjs:203`) dropped the lift as "a conclusion". The hold stood, the required check stayed red, and the one action the message asked for was the one action its own reader discarded. ThinkFlowLab#2246 on 2026-11-03 is the live case. ## What changed The filter keeps a conclusion-shaped body when that body asks to lift the stop and carries no stop or hold of its own (`responseOnly`), so a lift phrased "You may merge this tip now" clears the stop even when it quotes the PASS. Nothing else widens: a quoted conclusion with no lift phrase still clears nothing, and a PASS marked "record only — do not merge" is still not a lift. Deviation from the one-line predicate proposed at claim time, kept on purpose: `!verdict || LIFT_REQUEST.test(withoutQuotes(...))` alone turned the existing case `not ok 101 - a conclusion written on one line of the diff is a request reader only, never a verdict` red, because that inline PASS body says "please do not merge this tip" and `LIFT_REQUEST`'s `\bmerge this\b` matches it — the whole comment then read as a stop. `responseOnly` = has a lift phrase AND no `STOP_REQUEST` AND no `HOLD_LINE`, which is narrower and stays on the safe side: it would rather leave a hold standing than clear one by mistake. Known residual, deliberately not changed: `HOLD_LINE` is a line-start `hold` pattern, so a lift that begins "Hold cleared: you may merge" is still read as a hold of its own and would not clear the stop. That direction is safe (a hold is never lifted by accident), and tightening it is a separate change. ## Red first (on live base `910f6302`, only the new cases added) `node --test .github/scripts/enqueue-guard.test.mjs` → **106 tests / 105 pass / 1 fail**, and the only red is the new case: ``` not ok 105 - P-479 (2026-11-03): a lift that quotes the PASS it answers still clears the stop error: |- a comment asks not to merge this pull request: comment 6596354014 by hugo at 2026-11-03T05:38:25Z says "please do not merge" — clear it on the record with a newer comment that lets this tip merge, or push a corrected tip that earns its own PASS false !== true expected: true actual: false ``` The refusal is the guard asking for the lift that is already in the list, which is the defect stated in its own words. The counter-proof (same quoted conclusion, no lift phrase → still refused, and the refusal still names `6596354014`) is green on the base by design, so the fix cannot pass by clearing every conclusion-shaped comment. ## Verification at the tip Tip SHA: `ec4225f712aae273e7312a8cb9db3ac823ae8a29` Ran: - `node --test .github/scripts/enqueue-guard.test.mjs` → 106 / 106 / 0 - `node --test .github/scripts/merge-gate.test.mjs` → 51 / 51 / 0 - `node --test .github/scripts/review-gate.test.mjs` → 68 / 68 / 0 - the two `record only — do not merge` cases, run alone → 2 / 2 / 0 (a parked PASS is still not its own stop) - `node --test ".github/scripts/*.test.mjs"` (the whole set `verify` ends with) → 601 / 601 / 0, with `npm_config_cache` pointed at a writable cache: the first run was 600 pass / 1 fail, `not ok 334 - packed @flybot/social, @flybot/tree, @flybot/timeline and @flybot/workflow import`, on `npm pack` hitting `EROFS` writing the default `~/.npm` cache — an environment cause, and green once the cache moved. Not run: the `core/` and `plugins/` part of `npm run verify` — this worktree has no dependency install (`build-client.mjs --check` cannot find `esbuild`), and the diff touches only `.github/scripts`, which that part does not execute. The `.github/scripts` half of `verify`, which is the part it does share with this change, was run in full above, and CI's `tests-ok` runs at this tip. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 2282, "claim": "Dev-4", "issue": 2281, "summary": "The enqueue guard told a held author to answer the hold with a newer comment and then discarded that comment: a lift quotes the PASS it answers, the parser read the quoted conclusion as the body's own, and the stop filter dropped the whole comment. The filter now keeps a conclusion-shaped body only when it asks to lift the stop and carries no stop or hold of its own. A quoted conclusion with no lift phrase still clears nothing, and a record-only PASS is not a lift. The new case and its counter-proof both live in enqueue-guard.test.mjs.", "files": [".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.test.mjs"], "checks": { "ran": "node --test .github/scripts/*.test.mjs (601/601) and the enqueue-guard/merge-gate/review-gate files alone", "result": "green", "at": "ec4225f712aae273e7312a8cb9db3ac823ae8a29" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> Co-authored-by: FlyBot Dev-4 <dev4@flybot.local>
On `19e206ecb` the predicate that clears a stop was wrong in both
directions:
- dangerous: "I am not asking you to lift the hold — you may merge this
tip only after the second reader re-runs" and "You may merge this tip
once the second reader re-runs" both carry a lift phrase and no stop
phrase, so they cleared a hold they refuse to lift. `STOP_REQUEST` has
no "not asking"/"only after"/"once", and `HOLD_LINE` reads an opening
line only;
- stuck: "Hold cleared — you may merge this tip now" on an opening line
was read as a hold, and the same sentence without its quoted PASS made
itself the stop, naming its own id.
A lift now clears a stop only when it is unconditional: a lift phrase, no
stop or hold wording, and every clause that carries a lift phrase free of
negation and condition words (not/never/cannot/n't, only/after/once/
until/unless/if/when/before). The scan is per clause, not per body: a
whole-body scan refused seven of eight ordinary yeses ("No objections —
you may merge this tip now", "you may merge this tip now; I am back after
lunch"), which is the ThinkFlowLab#2246 wedge the other way round. Every clause that
carries a lift is read, so "Hold cleared — you may merge this tip only
after …" is still an answer withheld. `HOLD_LINE` no longer reads a
`hold` followed by its own clearing (clear/lift/answer/resol/gone/done,
with is/was/has been/been), so the answer is read as the answer; `Hold.`,
`Hold — …`, `Hold on` and `Hold off` are still requests to stop.
Fixes ThinkFlowLab#2287
Owner: Dev-4
Written-by: Dev-4
Reviewer: Birfy
Tier: T3
Branch: dev/4-enqueue-lift-both-directions
Base: 19e206e
On `19e206ecb` the predicate that clears a stop was wrong in both
directions:
- dangerous: "I am not asking you to lift the hold — you may merge this
tip only after the second reader re-runs" and "You may merge this tip
once the second reader re-runs" both carry a lift phrase and no stop
phrase, so they cleared a hold they refuse to lift. `STOP_REQUEST` has
no "not asking"/"only after"/"once", and `HOLD_LINE` reads an opening
line only;
- stuck: "Hold cleared — you may merge this tip now" on an opening line
was read as a hold, and the same sentence without its quoted PASS made
itself the stop, naming its own id.
A lift now clears a stop only when it is unconditional: a lift phrase, no
stop or hold wording, and every clause that carries a lift phrase free of
negation and condition words (not/never/cannot/n't, only/after/once/
until/unless/if/when/before). The scan is per clause, not per body: a
whole-body scan refused seven of eight ordinary yeses ("No objections —
you may merge this tip now", "you may merge this tip now; I am back after
lunch"), which is the ThinkFlowLab#2246 wedge the other way round. Every clause that
carries a lift is read, so "Hold cleared — you may merge this tip only
after …" is still an answer withheld. `HOLD_LINE` no longer reads a
`hold` followed by its own clearing (clear/lift/answer/resol/gone/done,
with is/was/has been/been), so the answer is read as the answer; `Hold.`,
`Hold — …`, `Hold on` and `Hold off` are still requests to stop.
Fixes ThinkFlowLab#2287
Owner: Dev-4
Written-by: Dev-4
Reviewer: Birfy
Tier: T3
Branch: dev/4-enqueue-lift-both-directions
Base: 19e206e
On `19e206ecb` the predicate that clears a stop was wrong in both
directions:
- dangerous: "I am not asking you to lift the hold — you may merge this
tip only after the second reader re-runs" and "You may merge this tip
once the second reader re-runs" both carry a lift phrase and no stop
phrase, so they cleared a hold they refuse to lift. `STOP_REQUEST` has
no "not asking"/"only after"/"once", and `HOLD_LINE` reads an opening
line only;
- stuck: "Hold cleared — you may merge this tip now" on an opening line
was read as a hold, and the same sentence without its quoted PASS made
itself the stop, naming its own id.
A lift now clears a stop only when it is unconditional: a lift phrase, no
stop or hold wording, and every clause that carries a lift phrase free of
negation and condition words (not/never/cannot/n't, only/after/once/
until/unless/if/when/before/pending, provided/providing/assuming, as long
as, as soon as, subject to, in case, depending on). The scan is per
clause, not per body: a whole-body scan refused seven of eight ordinary
yeses ("No objections — you may merge this tip now", "you may merge this
tip now; I am back after lunch"), which is the ThinkFlowLab#2246 wedge the other way
round. Every clause that carries a lift is read, so "Hold cleared — you
may merge this tip only after …" is still an answer withheld. `HOLD_LINE`
no longer reads a `hold` followed by its own clearing (clear/lift/answer/
resol/gone/done, with is/was/has been/been), so the answer is read as the
answer; `Hold.`, `Hold — …`, `Hold on` and `Hold off` are still requests
to stop.
Fixes ThinkFlowLab#2287
Owner: Dev-4
Written-by: Dev-4
Reviewer: Birfy
Tier: T3
Branch: dev/4-enqueue-lift-both-directions
Base: 19e206e
…ow's own key (ThinkFlowLab#2280) Fixes ThinkFlowLab#2278 ## What was wrong `core/test/readonly-window-reads.test.js` (ThinkFlowLab#2269) is the only guard against a read name falling out of `channel.js`'s `READS` whitelist, but it asserted only "the answer is not refused with `bot/read-only`". An answer that is past the gate and dry still passed, so the guard was green on a tree where a read-only window draws those screens empty. Measured on the tree of this PR's base with the file as it shipped: | read | what it actually answered | | --- | --- | | `folder-list` | `{"ok":false,"error":{"code":"bot/refused","message":"This DeepSeek Harness cannot list folders"}}` | | `connector-app-status` | `{"ok":false,"error":{"code":"bot/refused","message":"Connect Composio first"}}` | | `project-decision` | `{"ok":true,"value":{"ok":true,…}}` — the "nothing listed" half | | `project-of` | `{"ok":true,"value":{"project":null}}` | | `project-board` | `…"reason":"no working folder: this Project names none"` | | `folders` | `{"ok":true,"value":{"team":null,"picker":null,"folders":[]}}` | Six reads were dry; the old file was `# tests 3 / # pass 3 / # fail 0` on that same tree. ## What this changes Test-only: no source change, no shared fixture. - `core/test/readonly-window-reads.test.js` — each of the 11 `READS` rows now checks the values its screen draws, not the shape of the answer, so a dry answer is red instead of passing for "not refused". What each row proves: the row itself (`tasks`, `projects`, `project-read`, `project-of`, `project-read`'s project, `project-decision`'s file text and the decisions the board listed), the count beside it and its numbers (`project-board`: `tasks` plus `counts.waiting === 1` / `inflight 0` / `blocked 0`; `metrics.ok === false` with reason `no-credentials` and every metric `null`, ThinkFlowLab#1829), the people (`project-team`: a Chief row with `botId === 'a'`, not `rows: []` or `[null, null]`, ThinkFlowLab#1864), the memory entry (`project-memory`: a decision whose text is `A decision`, not an empty record, ThinkFlowLab#1712), the folder rows and the chooser kind (`folders`: `picker === 'browse'` plus the working folder; `folder-list`: an entry named `decisions`), and the connector's app state (`connector-app-status`: `gmail === 'ready'`). A row's payload may be a function of the answers above it, used only by `project-decision`, which opens a name the board itself listed (ThinkFlowLab#2041) rather than one this file made up. A refusal as `bot/read-only` (the whitelist lost the name) and a dry answer (past the gate, no data) are reported as two separate lists, so a red names which defect it is. - Fixtures inside that file make the data real: a `directoryPicker` of the `browse` kind on the fake context for the folder rows; a signed-in Composio session (state seed + `COMPOSIO_API_KEY` in `secrets.json`, with only `https://backend.composio.dev/` stubbed and the real `fetch` restored on stop) for the connector row; a Project working folder holding `decisions/2026-01-01-x.md` for the decision and board rows. - `patches/ds-bot/files/test/readonly-window-reads.test.js` — the same bytes. `core/` is carried as `ds-bot/` + patches, and this test file is one of the whole-file overlay entries, so a change under `core/test/` is only consistent once the overlay copy is regenerated. `package` step `ds-bot/ matches the pin, patches apply, core/ is ds-bot/ + patches (ThinkFlowLab#34)` caught exactly that (`core/test/readonly-window-reads.test.js: differs from ds-bot/ + patches`) before this file was regenerated in. ## Red first, then green - Before: the file as shipped, same base tree — `# tests 3 / # pass 3 / # fail 0`. The six dry answers above went unnoticed (false green). - After: `# tests 3 / # pass 3 / # fail 0`, run twice with the same counts. - Counter-proof (whitelist): deleting one name from `channel.js`'s `READS` makes the read-only case red and names it — `'folder-list'` removed → `# tests 3 / # pass 2 / # fail 1`, `not ok 1 - … a read-only window still answers these reads: folder-list`. The same shape was reproduced for `connector-app-status` and `project-decision`. `channel.js` was restored and the tree of this commit (below) re-verified green afterwards. - Counter-proof (data): with the dossier's Chief, the memory record, the card's `status`/`queueRole` and the `browse` chooser stripped from the fixtures — i.e. exactly the four dry answers that shape-only checks used to accept — the same case is red and names those four rows: `project-board`, `project-team` (`{"rows":[null,null]}`), `project-memory` (`"decisions":[]`) and `folders` (`"picker":"native"`), plus `folder-list` because a `native` chooser has no listing: `# tests 3 / # pass 1 / # fail 2`. Restored, then green again. ## What I ran - `TZ=UTC node --import ./test/harness-resolve.mjs --test test/readonly-window-reads.test.js` — `# tests 3 / # pass 3 / # fail 0` (twice, and once more after the patch overlay was regenerated). - The counter-proof above, then `git checkout -- core/src/host/channel.js` and a green run on the restored tree. - `node scripts/ds-bot-patches.mjs generate` — 307 overlay files written, of which exactly one changed (`patches/ds-bot/files/test/readonly-window-reads.test.js`, +84/−33); no `.patch` file changed. - `node scripts/ds-bot-patches.mjs check` — `ok core/ equals ds-bot/ + patches/ds-bot/`; `node scripts/ds-bot-patches.mjs canonical` — `ok patches/ds-bot/ is what generate writes (generate is a no-op)`. - Rebased onto the new `main` `b451e3b3` (this PR's base moved after ThinkFlowLab#2246 merged), then `node scripts/ds-bot-patches.mjs generate` again — it wrote nothing (no overlay or patch file changed), `check` and `canonical` still green, and the affected suite re-ran green on the rebased tree. - `ARCH_DOCS_BASE=b451e3b3 npm run lint` — `Found 74 warnings and 0 errors` (warnings pre-existing), `arch-docs: OK (2 changed file(s))`. - `git show --stat` — 2 files, +216/−68 (the test and its overlay copy, same 142-line diff each). ## What I did not run - `npm run verify` — not run in this worktree. The change is test-only with no source edit; the affected suite, the patch checks above and lint were run directly. - No build; no `changelog.d/` fragment and no subsystem doc (test-only, no user-visible change). - No other test file touched, and no source file touched. Verified head: `e6bb416cc52c0583348469d8a94b656f4e5ab8d1` (tree `b201349f4c5d170b4b9a1db48ed6207404d938a5`, single parent `b451e3b361771e85db156845f89a254c62ac275d`) — the pushed tip, whose tree is byte-for-byte the tree the run counts above were taken on.
…nkFlowLab#2288) Written-by: Dev-4 Owner: Dev-4 Reviewer: Reviewer Fixes ThinkFlowLab#2287 Base: 19e206e ## What changed `.github/scripts/enqueue-rules.mjs` — the answer to a hold is read in two directions, and both were wrong on `19e206ecb`: - **Dangerous:** a lift phrase with no stop phrase cleared the hold even when the body *withholds* the answer. `I am not asking you to lift the hold — you may merge this tip only after the second reader re-runs` and `You may merge this tip once the second reader re-runs` both cleared it. `STOP_REQUEST` has no `not asking` / `only after` / `once`, and `HOLD_LINE` only reads an opening line. - **Stuck:** `Hold cleared — you may merge this tip now` on an opening line was read as a hold, because `HOLD_LINE` matched the word it begins with; the same sentence mid-line cleared. Without its quoted PASS the sentence is not a conclusion, so it entered the stop scan as an ordinary comment and made itself the stop, naming its own id. The fix has two halves, and they are tested together because either one alone leaves a hole: 1. **A lift must be unconditional.** `responseOnly` now reads a lift phrase, requires no stop or hold wording, and requires every clause that carries a lift phrase to be free of negation and condition words (`not`, `never`, `cannot`, `n't`, `only`, `after`, `once`, `until`, `unless`, `if`, `when`, `before`, `pending`, `provided`, `as long as`, `as soon as`, `subject to`, `in case`, `depending on`). The scan is **per clause**, not per body — a whole-body scan refused seven of eight ordinary yeses (c1–c8 below), which is the ThinkFlowLab#2246 wedge the other way round. Every clause that carries a lift is read, not only the first, so `Hold cleared — you may merge this tip only after …` is still an answer withheld even though its first half is clean. 2. **`HOLD_LINE` no longer reads a `hold` followed by its own clearing** (`clear|lift|answer|resol|gone|done`, optionally after `is|was|has been|had been|been|gets|got`), so `Hold cleared — …` and `Hold is lifted — …` are read as the answer. `Hold.`, `Hold — …`, `Hold on` and `Hold off` are still requests to stop. 3. **A condition in the clause after the lift also withholds the answer.** Each clause was judged on its own, so `You may merge this tip — only after the second reader re-runs` cleared a hold that the same words refuse when they sit inside the lift's own clause (FlyBot Test, `REQUEST_CHANGES` at `3564a0095`, 2026-11-03). A clause that follows a lift is now read too: it withholds the answer when it carries a phrase that never sits in a plain yes (`only after`, `only once`, `only when`, `not until`, `not unless`, `pending`, `subject to`, `provided`, `as long as`, `as soon as`, `assuming`, `in case`, `depending on`) or when it opens with a condition word (`only`, `after`, `once`, `until`, `unless`, `if`, `when`, `before`, `pending`, `not`, `never`, `cannot`, `n't`, `provided`, `providing`, `assuming`). Ordinary trailing talk still clears: `you may merge this tip now; I am back after lunch if anything breaks` opens with `I`, and `No objections — you may merge this tip now` has nothing after the lift. ## Ran at the tip **Red first, on live main `19e206ecb52e61d5ed4170e8cb7ff0330711ea50`** — a clean worktree at that sha with only the new test cases added: ```text $ node --test .github/scripts/enqueue-guard.test.mjs not ok 107 - ThinkFlowLab#2287 A: a negated lift that quotes the PASS does not clear the hold not ok 108 - ThinkFlowLab#2287 B: a lift under a condition that quotes the PASS does not clear the hold not ok 109 - ThinkFlowLab#2287 C1: "Hold cleared" on an opening line clears the hold instead of being one not ok 110 - ThinkFlowLab#2287 C2: "Hold is lifted" on an opening line clears the hold instead of being one not ok 111 - ThinkFlowLab#2287 F: the opening-line answer without the quotation still clears and is not its own stop not ok 116 - ThinkFlowLab#2287 k1: a yes held pending the second reader keeps the hold (no quotation) not ok 117 - ThinkFlowLab#2287 k9: a yes held pending the second reader keeps the hold (with its quotation) not ok 118 - ThinkFlowLab#2287 k2-k8: the other ways a yes is made conditional keep the hold not ok 119 - ThinkFlowLab#2287 k10-k15: a condition in the clause after the lift keeps the hold not ok 120 - ThinkFlowLab#2287: a conditional lift with no hold wording of its own keeps the hold # tests 122 # pass 112 # fail 10 ``` A is the negation direction and B the condition direction: both cleared the hold on main. C1, C2 and F are the stuck direction: an opening-line answer was read as a hold, and without its quotation it became its own stop. k10-k15 carries the six bodies from the review round — the condition after a dash, a colon, a semicolon, a full stop, `— if the second reader agrees`, and a line break — and each of them cleared the hold on main. The real-hold counterweight (`Hold.`, `Hold — …`, `Hold on`, `Hold off` are still stops, refused with "asks not to merge" and named by id) and every clause of the ordinary-yes group passed on main, so they are regression guards, not part of that red. **The regression this review round was about** — the same new tests against the previous tip `7bd71df57`, whose conditional scan was still whole-body: ```text $ node --test .github/scripts/enqueue-guard.test.mjs not ok 118 - ThinkFlowLab#2287 k1: a yes held pending the second reader keeps the hold (no quotation) not ok 119 - ThinkFlowLab#2287 k9: a yes held pending the second reader keeps the hold (with its quotation) not ok 120 - ThinkFlowLab#2287 k2-k8: the other ways a yes is made conditional keep the hold not ok 112 - ThinkFlowLab#2287 c1: "No objections — you may merge this tip now" clears the hold not ok 113 - ThinkFlowLab#2287 c2: a yes followed by "nothing else waits on it" clears the hold not ok 114 - ThinkFlowLab#2287 c3: a yes after a colon ("Just re-read the diff: …") clears the hold not ok 115 - ThinkFlowLab#2287 c4-c8: the other ordinary yeses clear the hold # tests 121 # pass 118 # fail 3 ``` At this tip, with the clause-scoped scan (Node v22.23.2, `TZ=UTC`): ```text $ node --test .github/scripts/enqueue-guard.test.mjs # tests 122 # pass 122 # fail 0 $ node --test .github/scripts/merge-gate.test.mjs # tests 51 # pass 51 # fail 0 $ node --test .github/scripts/review-gate.test.mjs # tests 68 # pass 68 # fail 0 $ node --test ".github/scripts/*.test.mjs" # tests 617 # pass 617 # fail 0 ``` The existing `not ok 101` (`a conclusion written on one line of the diff is a request reader only, never a verdict`), the two record-only PASS cases and the earlier P-479 lift/counterweight pair are all still green. Not run: the `core/` + `plugins/` half of `verify`. This worktree has no installed dependencies (`build-client.mjs --check` cannot find `esbuild`), and the diff touches neither path. ## Known trade-off The clause scan is a text heuristic, and it stays deliberately conservative on the stop side: `hold`, `do not merge` and `not ready` wording is read across the whole body, so an answer that carries one anywhere is still refused. Two costs remain, both in the safe direction — the hold stands and the author can write a plainer yes: a condition inside the lift's own clause, or in a clause after it, keeps the hold standing (the remedy is a plain yes, which c1–c8 exercise); and a trailing clause that merely opens with a condition word (`You may merge this tip now. Once the reader runs, it is done.`) also refuses, because the guard cannot tell it from a real condition. The alternative — a whole-body scan of the broad vocabulary — was measured on `7bd71df57` and refused seven of eight ordinary yeses, which is the ThinkFlowLab#2246 wedge in the other direction. ## The 21-case audit The `ThinkFlowLab#2287` note asks the reviewer to run a 21-case audit over the tree (`A`–`F` cleared, the four withholding shapes stopped, `c1`–`c8` cleared, the real holds still stops). The same 21 cases are asserted inside `.github/scripts/enqueue-guard.test.mjs`, and a standalone copy run at each tree gives: ```text base 19e206e VERDICT FAIL (15/21 ok) tip 7bd71df VERDICT FAIL (14/21 ok) # the whole-body scan: c1-c8 refused tip 8ad39c9 VERDICT PASS (21/21 ok) # conditional-coverage probe: k1-k9 all cleared tip 3564a00 VERDICT PASS (21/21 ok) # conditional-coverage probe: k1-k9 all stopped this tip VERDICT PASS (21/21 ok) # conditional-coverage probe: k1-k9 all stopped ``` ## Tier T3 by path: `AGENTS.md` / `.github/agent/risk-rules.json` puts `.github/**` at T3, so this needs a formal conclusion naming this tip before it can enter the queue. <!-- flybot:checkpoint --> ```json { "pr": 2288, "claim": "Dev-4", "issue": 2287, "summary": "A comment clears an enqueue-guard stop only when it is an unconditional answer, read in both directions. The lift filter was too loose (a negated or conditional lift cleared the hold it refuses, including a condition one clause over) and too tight (an opening-line Hold cleared was read as a hold, and without its quotation became its own stop). Negation and condition words now block a lift per clause, a condition in the clause after the lift also withholds the answer, and HOLD_LINE ignores a hold followed by its own clearing. Real holds and ordinary yeses are unchanged.", "files": [".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.test.mjs"], "checks": { "ran": "node --test '.github/scripts/*.test.mjs'", "result": "green", "at": "4d5b851aaa6f5cee3a1df58e50e9ec18f346a69a" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> --------- Co-authored-by: FlyBot Dev-4 <dev4@flybot.local>
…owLab#2120) Fixes ThinkFlowLab#2119 Written-by: Dev3 Owner: Dev3 Reviewer: Reviewer Policy-sign: 6603688549 Tier: T3 Claim-Id: 09f51b71c0ae414d892960db0a237d86 Branch: dev/2-review-state-stop Base: 55503e1 Tree: 0755f1c Verified commit: 2a5f647 Pushed head: 2a5f647 Rebased-by: Dev-2 — this rebase only; the change below is Dev-3's work risk-tier: T3; basis: .github/scripts/enqueue-guard.mjs=T3, .github/scripts/enqueue-guard.test.mjs=T3, .github/scripts/enqueue-rules.mjs=T3, .github/scripts/merge-gate.mjs=T3, .github/scripts/merge-gate.test.mjs=T3, changelog.d/2119-review-state-stop.fixed.md=T1 ## This tip replaces `3b35c545` The same six-file change, rebased by Dev-2 onto the live `main` `55503e16` (the previous base `4a9d3edd` was 11 commits behind it; that is what made this pull request `dirty`). One commit, six files, `+162/−15`, single parent `55503e16`. One conflict, in `stopRequestAt` (`enqueue-rules.mjs`), and it is the only line where the rebase decides anything. While this change sat, `main` replaced `LIFT_REQUEST.test(text)` with `unconditionalLift(text)` (ThinkFlowLab#2246/ThinkFlowLab#2287: a lift that names a condition, or that a preceding clause withholds, answers nothing). This tip is a rewrite of the same sentence-shaped answers, so it takes `main`'s reading: the lift that answers a stop runs through `unconditionalLift`. Everything else in the six files is byte-identical to the change reviewed at `3b35c545`; `git range-diff 4a9d3ed..3b35c54 55503e1..2a5f647` shows the two columns apart only by `main`'s own `responseOnly` filter (ThinkFlowLab#2246) and that one predicate. No policy signature is claimed at this tip. The one written for `6cbb5cad` (comment `6571885896`) binds that tip — a signature binds the tip it was written for, so a new tip needs a new signature — and it is not carried here. A fresh `Policy-sign:` is to be issued against `2a5f647ea` once a reviewer has signed this tip off; until then the signature check is expected to refuse this body, and that refusal is this paragraph's point, not a defect in the change. ## What changed A review that requests changes is the one refusal a person writes without typing anything, and neither gate could see it: both readers dropped every review whose body was empty *before* mapping the rows, `stopRequestAt` matched text only, and the review's own state never travelled. A pull request a person had rejected answered `ok: true` at the guard and at the queue entry. - **`enqueue-guard.mjs` / `merge-gate.mjs`** — the review row carries `state`, and the `.filter((v) => (v.body ?? '').trim())` in front of the map is gone, so an empty body no longer drops the review before the stop reader sees it. The `review-comment` rows (the diff-line face, ThinkFlowLab#2091) keep their own filter unchanged. - **`enqueue-rules.mjs` `stopRequestAt`** — a row with `state === 'CHANGES_REQUESTED'` is a stop, read before any text and with or without a body. It carries `{ id, login, at, kind: 'review', because: 'state' }`, so the refusal reads `review <id> by <login> asked for changes at <at>`; a state has no sentence to quote, and the text path keeps its own wording. - **Clearing answers the request it was written on.** The written request and the state request are tracked apart, and the later one still standing is what the refusal names and the reader has to answer. A later `APPROVED` or `DISMISSED` from the same login answers the review and clears only that; a hold written in a comment stays standing, because the review UI is not where it was written. A later comment that lets the tip merge clears both through the existing lift path, and an approval from another login clears nothing. - **`merge-gate.mjs`'s note** that the state is "deliberately not read" now states the new rule and keeps the fact it rested on: the account cannot approve its own pull requests, so its reviews stay `COMMENTED` even when the body says INC (the INC on ThinkFlowLab#610 was a comment), and a `COMMENTED` review with no body is not a stop. - The "a body carrying a conclusion is not a request" filter and the `after` anchor are unchanged and run before the loop: a review body that carries a conclusion is still a conclusion, and a request older than the conclusion the decision used still does not apply. 6 files, `+162/−15`. No existing assertion was changed. ## Red first, on the new base Clean `55503e16`, the two test files only, `TZ=UTC node --test .github/scripts/enqueue-guard.test.mjs .github/scripts/merge-gate.test.mjs` — **196 tests / 191 pass / 5 fail**: ``` not ok 99 - \ThinkFlowLab#2119: a review requesting changes stops the enqueue with no body, and names the review not ok 101 - \ThinkFlowLab#2119: a later APPROVED review by the same login clears the request, and the record says which review cleared it not ok 102 - \ThinkFlowLab#2119: another login's approval does not answer the request, and a comment that lets the tip merge does not ok 104 - \ThinkFlowLab#2119: a review request and its approval do not answer a request written before them not ok 167 - \ThinkFlowLab#2119: a review requesting changes with no body refuses the queue entry ``` After the fix, the same two files on the same tree: **196 / 196 / 0**. **The filter is its own half of the defect** (found at the previous base, still true here): with `state` carried on both maps and the empty-body filter left in place, the same guard/merge-gate cases fail the same way, because the row never reaches `stopRequestAt`. That contrast is why the fix removes the filter instead of only widening the map. ## Second round — the overwrite the review found Review `6570945524` (REQUEST_CHANGES at `34f72771`) and its full read `6570870283` found one defect: a `CHANGES_REQUESTED` row assigned the stop unconditionally, so a written hold read earlier became a state stop, and the same login's later `APPROVED` (or a `DISMISSED`) then cleared that state stop and took the written hold with it. Reproduced end to end through `merge-gate`'s `main()` in queue mode: PASS, hold comment, empty-body `CHANGES_REQUESTED`, `APPROVED` → `ok: true`, `stopRequest: null`. Case `ThinkFlowLab#2119: a review request and its approval do not answer a request written before them` (the `not ok 104` above) pins it. The fix keeps the two requests apart and returns the later one still standing. The test starts from a text stop, which no earlier case did: the later review is named while it stands; its approval answers only the review and the written hold is still the request that stands; a third person's hold is untouched by that approval; a dismissal behaves the same way. The comment above the clearing rule says what the code does: a written request is answered in writing, a state request by the review it was submitted on, and a lift comment answers both. ## Verification at the tip `2a5f647eac5dfdf785e86295dbce389b86f67c56`, tree `0755f1ca7389124beb8900aeb6b7613379e7bbf1`, single parent `55503e16b359f034d71328f920aae6bbf674e0d1`. Ran on a clean, committed tree (`git status --porcelain` empty): `TZ=UTC ARCH_DOCS_BASE=55503e16b359f034d71328f920aae6bbf674e0d1 npm run verify` — **exit 0**: - core **2700**, `plugins/social` **570**, `plugins/tree` **132**, `plugins/timeline` **140**, `plugins/workflow` **411**, `.github/scripts` **640**, `interop` **37**, `live-e2e` **26** — **4656 / 4656 / 0** - lint: core 74 warnings / 0 errors; plugins 36 / 9 / 2 / 16 warnings / 0 errors; interop 1 warning / 0 errors; live-e2e 0 / 0 - `arch-docs: OK (6 changed file(s))` - ds-bot: `ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5)`, `10 patches apply cleanly in order`, `core/ equals ds-bot/ + patches/ds-bot/`, `patches/ds-bot/ is what generate writes (generate is a no-op)`; `generate` run twice after the commit, `git status --porcelain` empty both times - `build-client --check` exits 0 The `.github/scripts` suite alone, at this tip: 640 tests, 640 pass, 0 fail. Not run (and why): no live GitHub API call and no real merge queue — `decide`/`guard` are driven through the fixtures both suites already use, so the refusal is proven against the shapes the API returns, not against a live queue entry. ## Scope - Not touched: `policy/gates.json` and `policy-sign.mjs` (ThinkFlowLab#2095/ThinkFlowLab#2099 hold that pair), the diff-line `review-comment` row, the text stop vocabulary, and the conclusion path. - Residual, stated rather than solved: an approval from a different login is not read as clearing another reviewer's request, matching GitHub's own rule; the test pins it. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 2120, "claim": "Dev3", "issue": 2119, "summary": "A review requesting changes stopped nothing: both readers dropped every review with an empty body, the stop reader matched text only, and the state never travelled, so a rejected pull request answered ok. The state now travels from enqueue-guard and merge-gate, and a CHANGES_REQUESTED review stops the merge and names the review. Written stops and state stops are read apart, so a later approval answers the review and leaves a hold written in a comment standing. Tests pin the stop, the COMMENTED shape, the clearings, the surviving hold, and the queue entry.", "files": [ ".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.mjs", ".github/scripts/merge-gate.mjs", ".github/scripts/enqueue-guard.test.mjs", ".github/scripts/merge-gate.test.mjs", "changelog.d/2119-review-state-stop.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=55503e16 npm run verify (clean tree)", "result": "green", "at": "2a5f647eac5dfdf785e86295dbce389b86f67c56" }, "next": "issues resolved — ready for review at this tip" } ``` <!-- /flybot:checkpoint -->
Written-by: Dev-4
Fixes #2243
Owner: Dev4
Reviewer: Birfy
Tier: T1
Claim-Id: a41089f1cdd34475b08a8e4188440002
Branch: dev/4-p457-schedules-create-entries
Base: 5008116
Tree: 93e92c8c30bb6274b13e136fe5a62d05d89a526a
Verified commit: local
51040d46c2fe287320e520b860213bc6f9486f91(the same commit that was pushed)Pushed head: 51040d4
Files: 12 changed, +6975/−6064 (six written, six generated)
Rebased onto
50081167e81ac5ee182e6cc5d1a25ab248329e32(live main:#2263, then#2261); every reading in this revision is from that base. Two earlier revisions of this body quoted5a21d5b4and then03a125306— the code and the case names are the same, the counts move with main.What the rebase onto
50081167had to resolve#2261(#2248, the other team's task-board change, merged upstream) rewritescore/src/client/kanban.jsand its tests. This change touchesschedules.js,composer.jsanddetails-drawer.js; the two change sets meet in three generated or shared files and overlap in no source line:core/client.js— generated bundle, both change sets in it; regenerated bynpm run build(not hand-merged).patches/ds-bot/0008-client-bundle.patch— generated mirror of that bundle; the rebase conflicted here (generated text), and the committed version is whatgeneratewrites.docs/subsystems/core-client.md— one table row each:kanban.js(theirs) andschedules.js(this change); both rows stand as written.Both behaviours are kept:
#2261's read-only task board is inkanban.js, tests and bundle exactly as it landed, and this change's read-only create entries are on top of it.build-client --checkis silent,node scripts/ds-bot-patches.mjs checkanswersok core/ equals ds-bot/ + patches/ds-bot/, andcanonicalanswersok patches/ds-bot/ is what generate writes (generate is a no-op)— the mirrors are generated output, not a resolution by hand.The defect
#2235 made a read-only window draw no create form (
settings.js:locked ? null : adding ? TaskForm : …), but three entries still lead into it, and each one lands on a page that draws nothing:core/src/client/schedules.js:461—BotSchedulesdraws itsNew taskrow unconditionally (it isTaskList'sextrarow), and the row callsactions.openSettings('schedules', bot.id).core/src/client/schedules.js:485—ScheduleSummarynever read the roster at all, so its empty state'sAdd one(No scheduled tasks yet. Add one) called the same thing.core/src/client/composer.js:170— the+menu'sScheduled taskrow callsopenSettings('schedules', target).settings.jsturns thestartid intoadding, so the settings page really does believe a form is open and then draws no form: a click that looked live ends on an empty page.Red first, on live main
50081167Clean tree at
50081167e81ac5ee182e6cc5d1a25ab248329e32with only the two test files touched.core/test/schedules-client.test.jsalone → 52 tests / 50 pass / 2 failnot ok 50 - P-457: a read-only Bot card draws no New task row—a read-only Bot card still offers New task, saw New taskok 51 - P-457: a locked page opened with a start id draws no create form— green on the base, the counter-case that must stay greennot ok 52 - P-457: a read-only details line offers no Add one, and a writable one does—the locked details line draws no button,actual [ 'No scheduled tasks yet. Add one' ],expected []core/test/composer-locked.test.jsalone → 2 / 1 / 1not ok 2 - read-only window: no menu row opens the schedules page—a read-only window still offers the row: ["Add files","Reference a chat","Scheduled task"],expected: true,actual: falseThe composer case is the one Product's earlier probe could not mount; it is a real test now, with the three stubs the probe needed (
createPortal: node => node,useStatefed{left:0,bottom:0}thennull,useRef: current => ({current})).The whole
corepackage on the same red tree (both files at their package numbers): 2685 / 2682 / 3 —not ok 277,not ok 1990,not ok 1992, the same three cases.After the fix, on this tip:
schedules-client.test.jsalone 52 / 52 / 0,composer-locked.test.jsalone 2 / 2 / 0; the wholecorepackage 2685 / 2685 / 0.The fix
core/src/client/schedules.js:lockedWindow(roster)(the module-private rule that was already at:122) is exported once instead of copied a fifth time — the same predicate still sits inworkflows.js:570/639,sidebar.js:371andsettings.js:232, and this change does not touch those four.BotSchedulesdrawsextra: locked ? [] : [ … New task … ].ScheduleSummarytakesrosterand, when the window is locked and the Bot has no tasks, returnsnull; with tasks it still draws the read half, because those rows only open the settings list (openSettings('schedules'), no Bot id) and open no form.core/src/client/composer.js: the localschedule()helper is gone (:147-151on the base, call site:150); the menu row itself (:170) becomeslocked ? null : …and importslockedWindowfrom./schedules.jsinstead of growing a fifth copy.core/src/client/details-drawer.js:350: passes therosterit already holds intoScheduleSummary.docs/subsystems/core-client.md: the two sentences that contradicted each other are now one rule — a read-only window draws neither the empty state's button nor a Bot card'sNew taskrow.core/test/schedules-client.test.js(read-only card, read-only details line, plus the locked page that stays empty), andcore/test/composer-locked.test.jsin full (read-only draws no row, writable still posts('schedules', 'b1')).Writable counter-cases live in the same tests: the writable Bot card still has its
New taskrow, the writable details line still drawsAdd one, and the writable composer row still callsopenSettings('schedules', <bot id>).Which six files are written and which six are generated
Written by hand — six:
core/src/client/schedules.js(+10/−6)core/src/client/composer.js(+5/−5)core/src/client/details-drawer.js(+1/−1)core/test/schedules-client.test.js(+32)core/test/composer-locked.test.js(+82, new)docs/subsystems/core-client.md(1 line changed)Generated, never hand-edited — six:
core/client.js(npm run build; +1688/−1687)patches/ds-bot/0008-client-bundle.patch(4993/4354)patches/ds-bot/0001-core-fixes.patch(12/7)patches/ds-bot/0012-room-membership.patch(37/3)patches/ds-bot/files/test/schedules-client.test.js(+32)patches/ds-bot/files/test/composer-locked.test.js(+82)The generated diff is module order, not a hand edit
core/client.jsandpatches/ds-bot/0008-client-bundle.patch— +1688/−1687 and +4993/−4354 — are esbuild module order, not a logic rewrite:composer.jsnow imports./schedules.js, so theschedulesblock moved in front of thecomposerblock in the bundle. Generated only bynpm run buildandnode scripts/ds-bot-patches.mjs generate;build-client --checkand a no-opgenerateprove it.composer.jsimports./schedules.js, so theschedulesblock moved in front of thecomposerblock in the bundle. Both files are written only bynpm run buildandnode scripts/ds-bot-patches.mjs generate; neither was edited by hand, andbuild-client --checkplusgeneratebeing a no-op prove it.patches/ds-bot/0001-core-fixes.patchand0012-room-membership.patchfollow the same two source files. There is nopatches/ds-bot/files/src/client/schedules.jsmirror, and none was invented.What I ran at
51040d46c2fe287320e520b860213bc6f9486f91TZ=UTC ARCH_DOCS_BASE=50081167e81ac5ee182e6cc5d1a25ab248329e32 npm run verify→ exit 0.github/scriptsarch-docs: OK (12 changed file(s)); oxlint 0 errors in every suite (74/36/9/2/16 warnings, the same counts as live main; interop 1, live-e2e 0).ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5);ok 10 patches apply cleanly in order;ok core/ equals ds-bot/ + patches/ds-bot/;ok patches/ds-bot/ is what generate writes (generate is a no-op);build-client --checksilent.#2261's and the ones before it.Tier, by path (not by a label)
createGate(risk-rules).judge(files)over exactly these 12 paths answers:risk-tier: T1; basis: core/client.js=T1, core/src/client/**=T1, core/test/**=T1, patches/ds-bot/**=T1, docs/subsystems/core-client.md=T1Not run
dev/bench.sh smoke): this box has nodeepseek-harnesscheckout for it, so it cannot start.core/index.jsandcore/src/host/are untouched by this change.Checkpoint
{
"pr": 2246,
"claim": "Dev4",
"issue": 2243,
"summary": "The three entries #2235 left in a read-only window — BotSchedules' New task row, ScheduleSummary's empty-state Add one, and the composer's + menu Scheduled task row — now draw nothing, so no click lands on a create form the locked page hides. lockedWindow is exported once from schedules.js and reused by composer.js rather than copied a fifth time, the details line passes the roster, and a Bot that has tasks still lists them. Tests pin both read-only shapes and keep the writable entry points and their openSettings arguments unchanged. client.js/0008 is module order, not a hand edit.",
"files": [
"core/src/client/schedules.js",
"core/src/client/composer.js",
"core/src/client/details-drawer.js",
"core/test/schedules-client.test.js",
"core/test/composer-locked.test.js",
"docs/subsystems/core-client.md"
],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=50081167 npm run verify (4600/4600/0)",
"result": "green",
"at": "51040d46c2fe287320e520b860213bc6f9486f91"
},
"next": "T1: waiting for tests-ok at this tip; no PASS required"
}