Files
stack/agents/darkwing/work/queue-58/BUILD.md
T
jason.woltjeandClaude Opus 5.5 48b0b64869 review(58): Darkwing round 2 candidate, rebased on row 74's tree (#1547)
Round 1's build.patch applies cleanly on 424ada49 (row 74 landed as 351464ce).
Only packages/webui/README.md's digest changes; the bullet text is the same.
Manifest eb2b26c1. Gate 23:41:10Z-23:44:25Z: bus 83, tasks 71, webui 61, every
scripts/test-*.sh green. 16 of 17 mutants fail, output identical to round 1.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-11 18:49:52 -05:00

261 lines
12 KiB
Markdown

# Row 58 (#1547): bus read, task priority and requirement
Brief: `docs/plans/2026-10-10_design-implementation.md`, section "Bus read:
task priority and requirement" (blob `a360554c`). Owner Darkwing; reviewers
Dewey and Filbert.
The candidate is `build.patch` against the base in `base.txt`, 8 files in
`files.txt`. `candidate-manifest.sha256` holds the sha256 of each file after
the patch.
I built it on `72b16a5b`. Row 57 (#1546) then landed as `f78ba57d` and
changed four of these files: `broker.mjs`, both READMEs and the webui
`bus.test.mjs`. `git apply --3way` put the patch on `ae428a54` with no
conflict. Row 57 adds the decisions reads and this row changes the
`tasks` rows, so the hunks sit in different places; I read the merged
`broker.mjs` and both changes are intact. `build.patch`, the manifest and
the mutants are on `ae428a54`. The gate ran on `f2f2861b`, after row 62
landed; no candidate file changed between the two.
## Round 2: rebased on row 74's tree
Row 74 (#1568) landed as `351464ce` and changed `packages/webui/README.md`,
one of this row's 8 files. Sage's landing order is 74, then 58, then 60, so
this round puts the same change on row 74's tree. Round 1 had Filbert's
approval (comment 27426) and Dewey's (comment 27461) on manifest
`e63e4c4f`.
Round 1's `build.patch` applies cleanly with `git apply --3way` on
`424ada49` (`out/r2/apply.txt`; the "direct application" line is the new
test file, which has no base blob). Row 74 added its preview section at
lines 29 to 91 of the README, so this row's one bullet in the S5 section
moved down 66 lines; the two changes share no lines. The other 7 files hash
exactly as in round 1, and the bullet's text is unchanged. Nothing else in
the code, tests or docs changed.
`base.txt`, `build.patch` and `candidate-manifest.sha256` now hold round 2.
Round 1's are in `7f92a103`.
The three notes from round 1's reviews (Dewey's N1, Filbert's N1 and N2)
are not in this round. They go to a test-only follow-up row Sage opens
after this row lands.
### Round 2 checks
On the round 2 tree, one at a time (`out/r2/gate.sh`, `out/r2/mutants.sh`):
The gate ran from 23:41:10Z to 23:44:25Z on 2026-10-11, in a detached
worktree at `424ada49` with `build.patch` applied and `node_modules` linked
to the main checkout's (`out/r2/gate-summary.txt`).
| Suite | Result |
|---|---|
| packages/bus | 83 pass, 0 fail |
| packages/tasks | 71 pass, 0 fail |
| packages/webui | 61 pass, 0 fail, the browser tests and row 74's preview tests included |
| test-auth | 15 passed |
| test-conductor | 17 passed |
| test-config | 24 passed |
| test-discord | 66 passed |
| test-extension-package | 18 passed |
| test-foundation | 44 passed |
| test-queue | 27 passed |
| test-release | 14 passed |
| test-task | 26 passed |
As in round 1, `test-task` ran with `DOCKER_HOST` set to a missing socket,
so it skipped its Docker cases (adapter seam, workspace and capability,
live task, live recall pair), and `test-queue` skipped its canonical-root
check. Sage's gate rerun covers both. Afterwards I removed the
`wt_default` Docker network, which had no containers.
The same 17 mutants ran on the round 2 tree from 23:48:40Z to 23:49:07Z
(`out/r2/mutants.txt`). The output matches round 1's line for line, with
the timings left out: 16 fail, and `ignore-row-priority` survives for the
reason given under Mutants.
Candidate manifest sha256, round 2:
`eb2b26c1b84866868ff6ed0fc5410b10d0debbd0bb299be66201150377474786`.
## The problem
The brief says priority isn't in `task_current.fields`. It is:
`taskFields()` in `packages/tasks/src/digest.mjs` puts `priority` in every
live snapshot, `checkTask()` refuses a task without an integer priority, and
the Tasks table already showed it from the fields. What the read model
lacked was a named priority field, and the requirement, which sat only in
the `task.created` body (`packages/tasks/src/verbs.mjs`). The list never
reads that body, since the list has no trail. So for priority this row
lifts an existing value to a top-level key; the requirement is new data in
the list. The brief asks for both fields in the task read model, a
projection rebuild test, and the two columns in the Tasks view.
## What changed
### The bus `tasks` rows
`Broker.#taskRows()` in `packages/bus/src/broker.mjs` builds the rows the
`tasks` verb returns. Each is `task_current`'s row as before, with two keys
added:
- `priority` is the current row's `fields.priority`. It is null for a
tombstone, whose fields are only `gone` (and `project` on a move), and for
a read with no priority. Vikunja's 0 stays 0.
- `requirement` is the requirement in the first `task.created` for the same
Vikunja task id in the business. A move to another project changes the
project in the ref but not the task id, so the moved task keeps the
requirement it was created with. It is null for a task the stack didn't
create.
Both are computed from append-only rows at each read; nothing new is stored.
I chose that over a schema change. `store.mjs` checks the DDL against
`SCHEMA_SHA256` and an open bus against the reference digest, so a new
column or a changed `task_current` view needs a schema version and a
migration of every existing `bus.sqlite`. A read-time projection needs
neither, and a reopened bus gives the same rows by construction.
`taskView()`, the trusted S3 read, is unchanged and still returns
`task_current`'s rows only. Its comment now names `#taskRows` as the verb's
read.
The requirement read is one query per `tasks` call over the business's
`task.created` events. It isn't indexed by kind; a business with many
thousands of created tasks would make each Tasks refresh scan them. That is
fine at slice 1 sizes, and a later row can add an index if a real business
needs one.
### The Tasks view
`packages/webui/src/public/bus.js`:
- `task()` takes `requirement` from the bus row first, then from the trail's
`task.created`, so the list, which reads no trail, now has it. Priority is
the row's, then the last snapshot that read the task (a tombstone's page),
then null. Before, a missing priority became 0 and showed "unset". A
poller read always has one, so "none" shows only for a row written without
it, like the browser test's #9.
- The Tasks table gains a Requirement column after Task. Requirement and
Priority show a muted "none" when the row has none, in the list and on the
task page.
- `bus.css`: the tasks table's minimum width goes from 900px to 1020px for
the extra column. The browser test's overflow checks still pass.
For a live row the bus `priority` and the fields' `priority` are the same
value, so preferring the row's changes nothing visible today. I kept it so
the view reads the field the brief puts in the read model; the mutant that
drops it survives for that reason (below).
### Docs
`packages/bus/README.md` describes the two keys under "Shared reads and
adapter boundary". `packages/webui/README.md` adds one bullet on the
columns to the S5 section.
## Scope
The brief owns `packages/bus/`, `packages/tasks/`, the webui Tasks endpoint
and tests. The webui `bus.js`, `bus.css` and README edits are the Tasks view
the brief asks for. Nothing in `packages/tasks/` changed: the poller already
writes `priority` in each snapshot's fields, and `task.create` writes
`requirement` in `task.created`. Field writers are row 61.
## Tests
New `packages/bus/tests/task-read.test.mjs`, 3 tests. The seed has six demo
tasks: created by the stack, changed outside it since, never seen by the
stack, created then gone, created then moved to another project, and a
second `task.created` for the moved task's new ref. A second business reuses
one task id with its own requirement.
1. "tasks rows carry priority and requirement for every session that reads
tasks": the human, a reader and a role session get the same rows. A later
poll's priority wins over the stack's write, 0 stays 0, tombstones have
none, the moved task keeps its first requirement, and the other business
sees only its own. Each row minus the two keys equals `taskView(...,
'current')`'s row, and `taskView` has neither key.
2. "the read model rebuilds from the ledger": the rows after a close and
reopen equal the rows before. Each row's priority equals its current
snapshot's in `trail`, and its requirement equals the first `task.created`
for its task id in `events`. A read writes nothing.
3. "an empty business and one with snapshots but no task.created read
cleanly".
All three fail on the base broker (`out/task-read-on-base.log`).
`packages/webui/tests/bus.test.mjs`: the endpoint test asserts the row's
`priority` and `requirement` reach `/api/bus/tasks` as null, not missing.
`packages/webui/tests/bus-browser.test.mjs`:
- The done task (#9) is now written with no priority.
- The Tasks list check reads each table's header and each row's Requirement
and Priority cells: #8 `none`/`high`, #7 `REQ-SLICE-1`/`high`, #9
`none`/`none`. It replaces the `/high/` match.
- #7's page shows `Priorityhigh`. The moved task's page (#10, a tombstone)
shows `Priorityhigh` from its last snapshot.
- #9's page shows `Requirementnone` and `Prioritynone`. This visit comes
after the "never read" failure check, which needs #9's trail unread; an
earlier visit made that check fail.
## Mutants
`out/mutants.sh` edits one source file per mutant, runs the one test file
meant to fail it, and restores the file from the build worktree's index,
which holds the candidate. Output: `out/mutants.txt`, from the run on `ae428a54` at 22:22 UTC on
2026-10-11. 16 of 17 fail, as they did on `72b16a5b`; each webui kill
failed on the intended assertion.
| Mutant | Fails |
|---|---|
| `tasks-verb-unchanged`: the verb returns `task_current` as before | task-read 1, 2, 3 |
| `no-priority`: priority always null | task-read 1, 2 |
| `zero-priority-null`: `\|\| null`, so 0 becomes null | task-read 1, 2 |
| `tombstone-priority-zero`: `?? 0` | task-read 1, 2, 3 |
| `requirement-by-ref`: keyed by the whole ref | task-read 1, 2 |
| `requirement-last-wins`: the last `task.created` wins | task-read 1, 2 |
| `requirement-any-business`: events from every business | task-read 1 |
| `requirement-undefined`: no `?? null` | task-read 1, 2, 3 |
| `task-view-adds-keys`: `taskView` grows the keys too | task-read 1 |
| `endpoint-drops-keys`: the rows lose both keys | webui bus endpoint test |
| `list-requirement-from-trail`: requirement only from the trail | browser |
| `no-requirement-column` | browser |
| `priority-default-zero`: `?? 0` in the view | browser |
| `tombstone-no-fallback`: no snapshot fallback | browser |
| `detail-priority-raw`: the task page prints the raw value | browser |
| `list-priority-raw`: the list prints the raw value | browser |
| `ignore-row-priority`: the view reads only the fields' priority | survives |
`ignore-row-priority` is equivalent today, as said above.
## Gate
`out/gate/gate.sh` ran on 2026-10-11 from 21:57 to 22:02 UTC. Its worktree
was at `f2f2861b` with `build.patch` applied (`out/gate/apply.txt`) and
`node_modules` linked to the main checkout's. It ran each suite in turn and
wrote each log to `out/gate/`. The brief names bus, tasks and webui plus
every `scripts/test-*.sh`.
| Suite | Result |
|---|---|
| packages/bus | 83 pass, 0 fail |
| packages/tasks | 71 pass, 0 fail |
| packages/webui | 40 pass, 0 fail, the browser tests included |
| test-auth | 15 passed |
| test-conductor | 17 passed |
| test-config | 24 passed |
| test-discord | 66 passed |
| test-extension-package | 18 passed |
| test-foundation | 44 passed |
| test-queue | 27 passed |
| test-release | 14 passed |
| test-task | 26 passed |
`test-task` ran with `DOCKER_HOST` set to a missing socket and skipped its
live recall pair. `test-queue` skipped its canonical-root check, since the
gate worktree isn't the queue's root. Sage's gate rerun covers both.
After the gate a `gate_default` Docker network was left with no
containers, and I removed it.
Candidate manifest sha256:
`e63e4c4f756229a9e5bba276c908e272fae0a71ae4f0a1c47e7e47b630bc63e6`.