Repository navigation
fix(host): compare the quiet window with the due instant and list only cards that wait - #1761
Conversation
…t wait Three items in the Project Assistant's digest, prepared together. A quiet window is wall clock in the owner's zone, while the digest is due at 09:00 Asia/Shanghai. Comparing the two strings is only right when both zones are the same, so a UTC owner whose window covers 01:00Z was dropped for a window that never touches their due time. The window is now tested against the due instant read back in the owner's zone, and the held label starts at the instant the digest was held at instead of at another zone's 09:00. The personal block now says how many decisions the five-card cap left out, and it lists only the cards that wait on the reader: blocked and result-ready, read from the card's own status. A row with no status keeps the older answer, so no card disappears silently.
|
@Birfy please review this tip State at this head (base
|
|
Verdict: PASS at f01a17c Ran at this tip: Review points:
Not run: the full core suite could not finish on this box — it died on |
… so the queue stalls after every delivery (ThinkFlowLab#1745) Fixes ThinkFlowLab#1744 Owner: Dev2 Reviewer: Birfy Written-by: Dev-2 Head: `572606461b6fb87cab62873e2215a880a18f0b3d` (tree `f69ff5a600f7ca665fb7006986a76146c562f72b`), base `8252bce955f847b1ddeb49566635c3f8cd961bbc`. ## What changed A delivered card (`result-ready`) still counted as work in progress, so it filled the member's one WIP slot: after every delivery the member could not claim the next queued item, and `task-drive` offered that item to another member — or to nobody in a one-member role. One predicate answers both call sites. `occupiesSlot(task)` sits beside `columnOf` in `core/src/shared/task-board.js` and is true only for `claimed` and `working`. `claimQueued` (`core/src/host/tasks.js`) and `holdsCard` (`core/src/host/task-drive.js`) call it; the expression now exists nowhere else. The `task-drive` comment says a delivered card waits on its requester, and the old comment that named only `blocked` is gone — that comment is where this bug was born. The board reading is unchanged: `columnOf` still maps `result-ready` to `in-progress`, so the card stays **In progress** until it is accepted. `client/kanban.js` (Complete button) and `host/project-assistant.js` (digest) are untouched, as are the send-back rules. ## Rebase onto `8252bce9` The old tip went `dirty` when `ThinkFlowLab#1762` (Settings graph fit) merged: the two pull requests shared `core/client.js`, `patches/ds-bot/0008-client-bundle.patch` and `docs/subsystems/core-client.md`. `ThinkFlowLab#1761` then landed (`core/src/host/project-assistant.js`, `docs/subsystems/core-host.md`), which is also why this branch was rebased onto the live `main` at push time. - The only conflict was `0008`. Nothing in `patches/ds-bot/` was hand-edited: the base copy was taken, then the whole chain was regenerated from the merged sources (`cd core && npm run build`, then `node scripts/ds-bot-patches.mjs generate`). - Both `docs/` files merged as text, keeping `main`'s sentence and this branch's sentence in each. - The source delta is unchanged by the resolution: **112** real lines (98 added / 14 removed), the same as before the rebase and far under 150. ## Red first (on `8252bce9`, the rebased base, sources untouched) Three `not ok` from `core/test/task-queue.test.js` plus the shared-module unit test, which cannot even import the export: ``` not ok 1 - test/bot-tasks.test.js SyntaxError: The requested module '../src/shared/task-board.js' does not provide an export named 'occupiesSlot' ``` ``` not ok 5 - a delivered card does not hold the member's WIP slot (ThinkFlowLab#1744) error: |- The input did not match the regular expression /^Claimed task-4192c7315ecaf26e /. Input: 'No queued item for Dev right now. You already hold an in-progress card (task-03819cdfab971c48).' ``` ``` not ok 6 - the wake pass offers the queue to a member whose card was delivered (ThinkFlowLab#1744) error: |- the member whose card was delivered is offered the queue item 1 !== 2 ``` ``` not ok 7 - a send-back returns the card as work and the next claim is capped (ThinkFlowLab#1744) error: |- The input did not match the regular expression /^Claimed task-235ac0df90fd8adf /. Input: 'No queued item for Dev right now. You already hold an in-progress card (task-e8fc9c4a968ea161).' ``` The existing WIP test ("a member that already holds an in-progress card claims nothing") is unchanged and stays green: WIP is still enforced at the claim, only `result-ready` and `blocked` no longer count. ## Lines Real changes, excluding the generated `core/client.js`, the five `patches/ds-bot/files/` mirrors and `0008`: | file | lines | | --- | --- | | `core/src/host/task-drive.js` | +6/−6 | | `core/src/host/tasks.js` | +4/−4 | | `core/src/shared/task-board.js` | +8/−0 | | `core/test/bot-tasks.test.js` | +15/−1 | | `core/test/task-queue.test.js` | +61/−0 | | `docs/subsystems/core-client.md` | +1/−1 | | `docs/subsystems/core-host.md` | +2/−2 | | `changelog.d/1744-result-ready-slot.fixed.md` | +1/−0 | **112** changed lines (105 code + 7 docs/changelog). Generated at this base, listed separately: `core/client.js` **+3/−0**; `patches/ds-bot/0008-client-bundle.patch` **+4/−1**; mirrors `patches/ds-bot/files/src/host/task-drive.js` +6/−6, `files/src/host/tasks.js` +4/−4, `files/src/shared/task-board.js` +8/−0, `files/test/bot-tasks.test.js` +15/−1, `files/test/task-queue.test.js` +61/−0. `0008` and `client.js` are a **real** bundle change, not drift: the new `occupiesSlot` export is in the module the client bundle carries, so `client.js` gains those three lines and the patch's hunk count moves by one (`+2805,4192` → `+2805,4195`). A no-change `generate` afterwards left `0008`, `0003`, every mirror and the source tree byte-identical. `generate` is still not byte-idempotent on this box (git 2.34.1 writes hunk headers with function context): after the rebuild it rewrote `0001-core-fixes.patch` (44/44) and `0014-flybot-branding.patch` (2/2) for header reasons alone. Both were restored to the base bytes — the application result was unchanged — and a second, no-change `generate` then rewrote **only** those two headers, with `0008` and every other patch unchanged. No hunk was hand-edited. ## Check `check`, twice on this tree, prints the same three lines: ``` 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/ ``` `arch-docs` ran against the rebase base `8252bce955f847b1ddeb49566635c3f8cd961bbc`: **OK (15 changed file(s))**. ## What I ran and what I did not run Ran at the tip: `node scripts/ds-bot-patches.mjs check` (twice), `TZ=UTC ARCH_DOCS_BASE=8252bce9 npm run verify` (exit 0), and the red-first run at the base with only the two test files in place. The full verify at this head: core **2108**, social **539**, tree **125**, timeline **129**, workflow **289**, `.github` **549**, interop **37**, live-e2e **24**, every suite 0 fail; `arch-docs: OK (15 changed file(s))`; `10 patches apply cleanly in order`; `build-client --check` silent. Not run: no browser screenshot (host-side queue rule, no visual surface); no `dev/bench.sh smoke` (no `index.js` / `src/host/` wiring change beyond the predicates, and it would need a bench). No other pull request or branch was touched. ## Gates at this head `enqueue-guard` at this head is red, verbatim: ``` do not enqueue ThinkFlowLab#1745: comment by Birfy: names 89bfbf9, not the tip 5726064 — the change set differs at core/client.js, so that conclusion does not describe this head; comment by Birfy: names 3b0ff87, not the tip 5726064 — the change set differs at core/client.js, so that conclusion does not describe this head ``` That is the guard rejecting both standing conclusions as describing other heads — the two shas inside that sentence are the stale verdicts, not evidence for this tip. Labels it read at this head: `risk:T3`, `pass`; the fork exemption covers the issue reference and `agent-pr`. A PASS naming `572606461b6fb87cab62873e2215a880a18f0b3d` is what clears it. ## Checkpoint <!-- flybot:checkpoint --> {"pr": 1745, "claim": "Dev2", "issue": 1744, "summary": "A delivered card (result-ready) no longer fills the member's WIP slot. One predicate, occupiesSlot in core/src/shared/task-board.js, is true only for claimed and working, and both call sites (claimQueued, holdsCard) use it; the column reading still shows result-ready as In progress. The branch was rebased onto live main after ThinkFlowLab#1762 and ThinkFlowLab#1761 merged, with the generated files regenerated from zero and no hand-edited patch hunk. Tests: three new task-queue cases plus a shared-module test, red first; full verify green at the tip.", "files": ["core/src/shared/task-board.js", "core/src/host/tasks.js", "core/src/host/task-drive.js", "core/test/task-queue.test.js", "core/test/bot-tasks.test.js", "docs/subsystems/core-host.md", "docs/subsystems/core-client.md", "changelog.d/1744-result-ready-slot.fixed.md"], "checks": {"ran": "TZ=UTC ARCH_DOCS_BASE=8252bce9 npm run verify", "result": "green", "at": "572606461b6fb87cab62873e2215a880a18f0b3d"}, "next": "none — ready for a PASS at this tip"} <!-- /flybot:checkpoint -->
…settings (ThinkFlowLab#1743) Fixes ThinkFlowLab#1740 `project-delete`, `project-memory`, `project-memory-write` and `project-assistant-owner` shipped without the ownership check that `project-bind` already made, so any signed-in member could delete another member's Project, read or append to its private assistant memory, and rewrite another owner's digest settings. Base: `8252bce955f847b1ddeb49566635c3f8cd961bbc` (main, the merge of `ThinkFlowLab#1761`). Head: `178bf7583a838f6184c40f4648eab866a0be276c` (tree `3053568544d9d3aa6689807fe4aec823d7f328cd`). This tip is a rebase, not a redraw: the PR was `mergeable_state: dirty` against its old base `4916bd5f`, so it now sits on `8252bce9`. Two files conflicted and both were resolved without touching a code decision — `docs/subsystems/core-host.md` keeps both sections (`ThinkFlowLab#1761`'s quiet-window / due-instant text and this PR's owner-gate paragraph), and `patches/ds-bot/0002-flybot-api-host.patch` was **regenerated from scratch** by `scripts/ds-bot-patches.mjs generate`, with no hunk edited by hand. `core/src/host/store.js`, `core/test/project-owner-gate.test.js` and the changelog applied unchanged, and their line counts are identical to the pre-rebase tip. ## Red first Test `core/test/project-owner-gate.test.js`, run on the PR's original base `4916bd5f`; the file is byte-identical at this tip (`git diff` between the two tips lists no line of it), same runner as CI: ``` node --import ./test/harness-resolve.mjs --test test/project-owner-gate.test.js ``` ``` not ok 1 - \ThinkFlowLab#1740: a member who does not own the Project cannot delete it error: 'Missing expected rejection.' code: 'ERR_ASSERTION' not ok 2 - \ThinkFlowLab#1740: a member who does not own the Project cannot read its memory error: 'Missing expected rejection.' code: 'ERR_ASSERTION' not ok 3 - \ThinkFlowLab#1740: a refused memory write is a refusal, not a silent no-op error: 'Missing expected rejection.' code: 'ERR_ASSERTION' not ok 4 - \ThinkFlowLab#1740: a member cannot change another owner's digest settings error: 'Missing expected rejection.' code: 'ERR_ASSERTION' ok 5 - \ThinkFlowLab#1740: the owner still uses all four, and a missing id still answers not-found # tests 5 # pass 1 # fail 4 ``` Test 5 is the control: it passes on the base too, so the red above is the gate being absent, not the test being unable to pass. It also pins the unknown-id behaviour that must survive the fix (`bot/not-found`, not a refusal and not a 500). Every refusal assertion is on `error.code === 'bot/forbidden'`, not on message text and not on "the result differs". Test 3 additionally asserts the stored memory is byte-identical before and after the refused write, so replacing the refusal with a silent no-op cannot pass it. ## What changed `core/src/host/store.js` (the only source file touched): - `project-delete`, `project-memory`, `project-memory-write`: read the record first, then the existing `requireProjectOwner(project, actor)`. Reading first is what preserves the unknown-id answer: the gate returns early on `null`, so a missing id still reaches the call that owns the not-found answer for that endpoint. - `project-assistant-owner`: `requireProject` + `requireProjectOwner`, then the body's `ownerId` must equal the caller (`projectActorId(actor)`), omitted meaning the caller's own. The record being written belongs to that owner, so ownership of the Project alone is not the whole gate. - The comment on `projectActorId`/`requireProjectOwner` now states the rule as implemented: a non-owner may read the board and nothing else; the record, its memory and the assistant settings are the owner's. `docs/subsystems/core-host.md` says the same thing. `authz.js`, `channel.js` and the endpoint names are untouched; `project-assistant-owner` is still not wired into the channel case table (separate issue). The predicate is the one already in the file — no second copy. ## Control: the assistant's own write path still works The gate lives in `projectEndpoint`, and the assistant never goes through it: - the assistant's digest build reads and writes through `rt.projectAssistant`'s own `read`/`write` (`core/src/host/project-assistant.js:156-173`), which calls `rt.appendProjectMemory` directly (`:171`) and holds no reference to `projectEndpoint`; - so this change cannot reach the assistant's own path — it only adds a check at the four endpoints, and the endpoint-level `requireProjectOwner` is deliberately not inside `appendProjectMemory` (putting it there would gate the assistant's writes too). Evidence at this tip: `core/test/project-assistant.test.js` green, including - `memory is private to one Project and an unknown id is refused` - `two owners of one Project get two digests from one read, with no memory bleed either way` - `a cross-owner memory read is refused and names the boundary` ## Mirror `core/src/host/store.js` is an upstream file, so it travels in the patch chain, and the new test file is a file upstream does not have: - `patches/ds-bot/0002-flybot-api-host.patch` — `+28/−7` against the base, all of it the `store.js` section plus the one hunk header whose count follows the new lines. It is the **only** patch in `patches/ds-bot/` that differs from the base: `git diff --name-only 8252bce HEAD -- 'patches/ds-bot/*.patch'` lists exactly that file. `0002` was recut by `generate`; if another in-flight PR touches `0002` too, the later merge rebases and regenerates it from scratch. - `patches/ds-bot/files/test/project-owner-gate.test.js` — the new test, mirrored whole (byte-identical to `core/test/project-owner-gate.test.js`, `cmp` clean). `generate` is not byte-idempotent in this repo: a run on a clean tree also rewrites `0001-core-fixes.patch` and `0014-flybot-branding.patch` with hunk-header drift only (0 body lines differ). Both were restored to the base bytes, and `check` is green again with them; no applied byte changed. The regenerated `0008-client-bundle.patch` is byte-identical to the base's, which is what "the first merge normalises it" requires. ## What I ran At `ARCH_DOCS_BASE=8252bce955f847b1ddeb49566635c3f8cd961bbc`, in this tree, `TZ=UTC`: - `npm run verify` — exit 0. Includes `npm --prefix core run lint`, `scripts/lint-plugins.mjs`, `core/scripts/build-client.mjs --check` (exit 0; the bundle does not carry `store.js`), `scripts/ds-bot-patches.mjs check`, core tests (2109), all four plugin suites (social 539, tree 125, timeline 129, workflow 289), `.github/scripts/*.test.mjs` (549), interop (37) and live-e2e (24); every suite `# fail 0`. - `scripts/ds-bot-patches.mjs check` on its own, twice, byte-identical output: ``` 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/ ``` - `core/scripts/build-client.mjs --check` on its own — exit 0. - `check-arch-docs` — exit 0, `arch-docs: OK (6 changed file(s))` against the base ref above. - `core/test/project-owner-gate.test.js` after the fix: 5/5 green. What I did not run: the real-model `bench.mjs` (minutes, spends tokens, not part of `verify`). No live instance was touched. ## Gates at this head `enqueue-guard` on `178bf7583a838f6184c40f4648eab866a0be276c`: `do not enqueue ThinkFlowLab#1743: comment by Birfy: names e0745e2, not the tip 178bf75 — the change set differs at docs/subsystems/core-host.md, so that conclusion does not describe this head` — red until a Verifier report names this tip; the issue-reference and `agent-pr` rules are not asked of a fork head. ## Scope No other contribution is touched. `ThinkFlowLab#1761` is merged and its docs section is kept, not replaced. `ThinkFlowLab#1727` is another Dev's draft on `ThinkFlowLab#1709`; `ThinkFlowLab#1509`'s frozen draft is untouched.
Fixes #1758
Three defects in the digest text a Project owner receives, all in
core/src/host/project-assistant.js: a quiet window compared as wall-clock text against a dueinstant, silent truncation past five waiting decisions, and cards that wait on nobody listed as
waiting on the owner.
Base:
7f4aa2a70766a7292cf5474ec8a7bbc364f5d1ea(the tip after #1747 merged; that merge touches nofile this PR touches).
Head:
f01a17c6c9a52c0e3e047447292960f22a12f9de(tree8031b5b95085ead0d199dea0b5d52e93e9b72e18;the locally verified commit
8a67f9a0e80e6c556810dd944c5ec09d3384dc4fhas the same tree — the pushhelper rebuilt the commit).
What changed
quiet window is wall clock in the owner's own zone. Comparing the string
09:00withinQuietHourscompared two different zones' wall clocks, so an owner outside +08:00 was droppedon the wrong day. The due instant is now computed (
Date.parse(day + 'T09:00:00+08:00'), readback with
clockIn(quiet.zone, …)) and compared as a clock in the owner's zone. The held labeluses the instant the digest was actually held at, and the drop reason names the due clock rather
than the literal
09:00.wait, a sixth line says
… and N more waiting on you. Five or fewer print exactly the bytes theyprinted before.
blocked("needs yourcall") and
result-ready("ready for your acceptance") cards, read from the card's own status. Acard still
open,claimedorworkingwaits on nobody and appears only in the sharedin-progress section. A row that carries no status keeps the older answer — absence is not
evidence that the card is moving.
Red first
Red first drives the digest through
rt.projectAssistant.setOwner(...)directly: that endpoint hasno channel case (
core/src/host/store.js:608handlesproject-assistant-owner, butcore/src/host/channel.js:670-677lists only the eightproject-*endpoints, so the request fallsto
defaultand failsUnknown endpoint/bot/not-foundat:716). No tool, hook, CLI orschedule path reaches it either, so no product-level path exists today; the window matters once
P-198 wires it.
What is visible today, in two classes:
quietrecord,quietOf(project-assistant.js:294)yields
{on:false}, so thecoversDue/ label / reason paths cannot fire.has
projects: 0, andproject-createhas no client caller and no UI path, so no Project existsto produce a digest.
On base
7f4aa2a7with only the source reverted (# tests 22 / # pass 16 / # fail 6), the sixfailures are:
With the fix:
22/22atTZ=UTCand22/22atTZ=Asia/Shanghai.The process time zone changes nothing; the owner's zone does. Same probe, base source vs. this
branch, owner quiet
09:00–11:00@Asia/Shanghai— the reason is byte-identical in both processzones:
quiet hours 09:00–11:00 cover 09:00, so nothing is delivered — the expected outcome. Ownerquiet
08:00–10:00@ UTC,01:00Z: base drops the digest withcover 09:00; this branch deliversonce and carries no reason.
Two existing tests changed
held from 08:00instead of09:00. The old string washeld from 09:00 to 08:45(base:325) — it starts after it ends, because09:00is exactlythe
DIGEST_TIMEconstant (project-assistant.js:24) the old code printed in the label(
:375) and compared ininQuietHours(quiet, DIGEST_TIME)(:365): the old expectationasserted the defect itself. The label now starts at the instant the digest was actually held
(
clockIn(quiet.zone, held.at ?? at)), which is08:00in UTC.task-create(defaultstatus: 'open') and still expectedtwo cards in the personal block, which P-195 no longer allows. The new fixture drives both cards
through
task-accept → task-start → task-wait-inputtoblocked, and adds aworkingcard withthe
a card the owner's Bot is still working on waits on nobodynegative assertion. The fouroriginal title assertions are byte-identical; the assertions got stronger, not looser.
Everything else is new tests, red as written.
Generated files
core/src/host/project-assistant.jsand its test are mirrored whole, so this diff carries exactlytwo generated paths:
patches/ds-bot/files/src/host/project-assistant.jsandpatches/ds-bot/files/test/project-assistant.test.js. No change topatches/ds-bot/0002-flybot-api-host.patchand none topatches/ds-bot/files/src/client/*.generateis not byte-idempotent in this repository: on this clean base it also rewrites0001-core-fixes.patch,0003-flybot-api-client.patchand0014-flybot-branding.patchwith44 / 23 / 2 lines of pure hunk-header and count drift, and
0008-client-bundle.patch(192 content linesdiffer in the patch, but the applied bytes are unchanged —
checkis green with the regenerated0008as well as with the base bytes). All four are restored to the base bytes, so the diff carries only the two mirrors; theapplied bytes are unchanged, and
checkstays green either way.What I ran
At this tip,
TZ=UTC,ARCH_DOCS_BASE=7f4aa2a70766a7292cf5474ec8a7bbc364f5d1ea:npm run verify— exit 0. Suites: core 2101, social 539, tree 125, timeline 129,workflow 289,
.github549, interop 37, live-e2e 24 — every suite fail 0.node core/scripts/build-client.mjs --check— silent, exit 0 (core/client.jsuntouched; thebundle does not carry
project-assistant.js).node scripts/ds-bot-patches.mjs check— the three lines above.arch-docs: OK (5 changed file(s))against the base ref above.22/22atTZ=UTCand22/22atTZ=Asia/Shanghai.Not run: the real-model
bench.mjs(minutes, spends tokens, not part ofverify). No live instancewas touched.
Size
Measured as add + delete, generated files and mirrors excluded: 190 lines of change (+164/−26) —
source +48/−17 (65 changed lines), tests +101/−4, docs +15/−5.
Gates at this head
At
f01a17c6c9a52c0e3e047447292960f22a12f9de, 13 of 14 check runs are green:tests-ok,merge-gate,judge,package,agent-audience,reviewed tip (report only),risk tier (report only),plugins (social/tree/timeline/workflow),Node 24.enqueue-guardis red onlyfor the missing Verifier report:
Its summary states the same fork rule the earlier PRs saw:
the head lives in a fork: the issue reference and agent-pr rules are not asked of it. Nothing else refuses this head.