Repository navigation
feat(board): one card model and one card component for both boards (#1265) - #1276
Merged
Merged
Conversation
…1265) The delivery board and the task board draw cards that look alike and are told apart only by which view rendered them. Step 1 of #1265 gives them one card: `source: 'github' | 'task'` plus a key that is unique across both — `owner/name#number` for a repository card, `task:<id>` for a task record — and one component that draws either. The two boards keep their own class names through a `skin`, so nothing in their stylesheets moves until step 2 collapses the columns. `owner/name#number` is deliberately not keyed by kind: an issue and the pull request that closes it are one card, which is the identity step 3 hangs a FlyBot record under. A row with no repository falls back to `github:<kind>:<number>` instead of implying an identity it cannot have. Part of #1265 — columns (step 2), records linked under issue cards (step 3) and multiple repositories (step 4) stay out of this change.
This was referenced Oct 26, 2026
Collaborator
Author
|
Verdict: PASS at fa19ecb 19 files, The identity is what the body says. Both boards really draw through the one component. Knives re-run by me (not taken from the body), each on the three files that should catch it: Both restored: Ran: Body and checkpoint are consistent with this head: Two notes, neither blocking: when |
HugoXK
added a commit
to HugoXK/FlyBot
that referenced
this pull request
Oct 27, 2026
…hinkFlowLab#1252) (ThinkFlowLab#1289) fix(board): archive finished task cards and complete the overall card (ThinkFlowLab#1252) Written-by: Dev-1 Owner: Dev-1 Reviewer: Reviewer Fixes ThinkFlowLab#1252 Tip SHA: `5b832b91f8ebdb1b7de782e87d40f35b24f6549d` ## What changed - `core/src/host/tasks.js`: the `tasks` read lists active work only — a card that is `accepted`, `cancelled` or `failed` leaves the list a board polls. `{ archive: true }` is the same read including those rows; `{ archive: true, limit: N }` returns the newest N **finished** rows by `updatedAt` (tiebreak `createdAt`) and no active rows, so the board's 15 s poll carries the slice and not the history while the default read keeps carrying the active work. `limit` is a number or nothing: a string, boolean or object is refused (`bot/invalid`) instead of coerced. A read **without** `limit` still returns every settled row on purpose, because it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row) — an id outside the newest page must stay readable. - The same file: new `settleOverall(child)` completes an overall group card when every child has ended, through the same `transition`/`applyColumn` write a person's move uses. It completes `accepted` (with `progress: 100`) when each child ended `accepted` or `cancelled`, and `failed` when any child ended `failed` — a failed part is not hidden behind a green overall card. It is called where a move lands on `accepted` (`applyColumn`), from `task-accept-result`, from `task-cancel`, and on the two other doors a child ends through — `taskBotDeleted` (deleting a Bot fails its open cards) and `taskDecided` (an owner's reject cancels the card that waited on the approval) — so neither path can leave the overall card `working` behind a child that already ended. An overall card already `blocked` or `result-ready` is left to its owner. - `core/src/client/kanban.js`: the board keeps an `archive` list from its second read and sorts it newest first by `updatedAt` (tiebreak `createdAt`) **before** cutting, renders `boardTasksFor(tasks, archive)` — active work plus the twenty newest finished cards — and moves a finished card off the active list at once in `replace`, before the next poll. A **Bot** board names its Bot in that read (`tasks { botId, archive: true, limit: ARCHIVE_SHOWN }`): the host spends the page's budget before the board can filter by `ownerBotId`, so an unfiltered page lets another Bot's newer cards empty this Done column. The default read stays unfiltered on purpose — it is active work, a superset the board filters locally without losing a card. Ordering by `createdAt` would drop exactly the card a member just finished, which has an old `createdAt` and a fresh `updatedAt`. - `core/src/client/inbox.js`: ThinkFlowLab#1252 made the default read active-only, but the Inbox is a second consumer that classifies finished and failed cards (`inboxModel`: `failed` → Needs you, `accepted`/`cancelled` → Done) and counts them in `needsCount`. The one shared `useTasks` hook behind the Inbox row and the dialog now asks `tasks { archive: true }` — the uncapped list — and publishes that, so those cards reach the badge and both groups again. The board's bounded second read is untouched. - `docs/subsystems/core-client.md`: the `kanban.js` row said the cap twice in one line (once for the read, once for the column); that is now one sentence that also says a Bot board names its Bot, and the archive read without `limit` staying uncapped is said once. ## Red first All of them were written before the fix and run red on the unfixed tree (`node --import ./test/harness-resolve.mjs --test --test-name-pattern="1252" test/bot-tasks.test.js`). First round, the original ThinkFlowLab#1252 behaviour: ``` a done card leaves the room board by default true !== false expected: false actual: true ``` ``` the overall card completes with its last child 'open' !== 'accepted' expected: 'accepted' actual: 'open' ``` Second round, after Security's INC on the previous tip: the failed child, the unbounded payload, and the oldest-first slice. ``` a failed child makes the overall card fail, not pass + 'accepted' - 'failed' ``` ``` the requested limit bounds the payload 200 !== 20 expected: 20 actual: 200 ``` ``` the column starts at the newest finished card + 'task-done-19' - 'task-done-36' ``` Third round, Security's second INC: a busy room spent the whole budget on active rows. ``` the bounded archive carries finished rows only false !== true expected: true actual: false ``` Fourth round, Reviewer2's INC on the previous tip: the two other doors a child ends through, and the Bot-scoped archive page. Each knife was cut on the `5385491f` tree, the file restored from a copy, and the pair re-run green. Knife 1 — delete `settleOverall(task)` from `taskBotDeleted` (`core/src/host/tasks.js`), which reddens `ThinkFlowLab#1252: deleting a child's Bot fails the overall card, read back from the archive`: ``` a failed child closes the overall card as failed 'open' !== 'failed' expected: 'failed' actual: 'open' ``` Knife 2 — put the Bot board's archive read back to the unfiltered `query` (`core/src/client/kanban.js`), which reddens `ThinkFlowLab#1252: a Bot board asks the host for that Bot's archive, not the whole scope`: ``` the archive read names the Bot + actual - expected { archive: true, - botId: 'a', limit: 20 } ``` Restored: `bot-tasks.test.js` + `kanban.test.js` `20/20` green again. Fifth round, Reviewer2's INC at `5385491f`: the Inbox is a second consumer of `tasks`, and the active-only default cut its finished and failed cards off. The drive posts `{ archive: true }` and asserts the settled rows reach the badge and the two groups; on the unfixed tree the read posted **no body at all**, so the fake host answered the active-only default and every assertion fell over: ``` the Inbox read asks for the settled rows expected: 1 actual: 0 ``` The same run, measured before the assertions: `posted null`, badge `"4"` (the base count, unchanged), Done card `null`, Needs-you card `null`. ## Reviewer finding `6415648677` (`Verdict: INC at 86e9795`) — answered at this tip - **Item 1 — two terminal paths ended a child without settling the overall card.** `settleOverall(task)` now runs directly after the terminal `transition` in both `rt.taskDecided` branches (the waiting-input branch and the blocked branch) and in `rt.taskBotDeleted`, with the `continue` / `return save()` kept and no second gate added. Proven by `ThinkFlowLab#1252: deleting a child's Bot fails the overall card, read back from the archive` — child `failed`, parent `failed`, `columnOf` done, and the progress the card already carried (the failed branch only calls `transition`; `progress: 100` is set on the accepted branch alone) — and by `ThinkFlowLab#1252: a rejected approval cancels the child and completes the overall card, read back` — child `cancelled`, parent `accepted`, `progress 100`, `columnOf` done. Both read the parent back through `tasks { archive: true }` / `{ roomId, archive: true }`, not through a return value. - **Item 2 — the Bot-scoped page was cut before the `botId` filter.** A Bot board now sends `archiveQuery = { botId: scope.botId }` with the archive read: `{ botId, archive: true, limit: ARCHIVE_SHOWN }`; the group board keeps `roomId`, and the default read stays unfiltered. Proven by `ThinkFlowLab#1252: a Bot-scoped archive page is cut after the Bot filter` (the Bot-scoped host page carries its own card while the unfiltered page does not) and by the client drive that captures the posted body as exactly `{ botId: 'a', archive: true, limit: ARCHIVE_SHOWN }`. - **Non-blocking, docs.** The duplicated cap sentence in `docs/subsystems/core-client.md` is collapsed to the sentence that states the behaviour; the literal call syntax is gone. ## Reviewer finding `6416140014` (`Verdict: INC at 5385491`) — answered at this tip - **Blocking — the Inbox stopped receiving finished and failed cards.** The chain at `5385491f`: `core/src/client/inbox.js` `useTasks` called `actions?.tasks?.()` with no body, `core/src/client/index.js` sent it as `{}`, and `core/src/host/tasks.js` answered the active-only default (`archived ? rows : rows.filter(openTask)`), while `inboxModel` classifies `failed` → Needs you and `accepted`/`cancelled` → Done and `needsCount` counts them. Fixed in the one shared hook: `useTasks` now sends `{ archive: true }` and publishes the list it gets; the board's bounded second read is unchanged. Red at `5385491f` is quoted under *Red first*, fifth round (`expected: 1, actual: 0` — no body posted; badge unchanged at the snapshot count; both card lookups `null`). Green at this tip: the same test passes, `core/test/inbox.test.js` **27/27**, and the assertion is the behaviour — `failed` lands in Needs you, `accepted` in Done, and the row badge counts the failed card. - **Non-blocking — the bounded page is `settled`, the Done column keeps only `accepted`.** Recorded as a decided residual under *Decisions recorded, not omissions* below: newer `cancelled`/`failed` rows can spend a scope's page budget and push an older Done card out of the twenty. Not changed at this tip, and the reason is written down there. - **Non-blocking — the Bot board's default read is still unfiltered client-side.** That read is the **active** list: unfiltered is a superset of the Bot's own rows and the board filters it in memory, so no card can be lost — a host-side cut is what made the archive read need `botId`, and that one sends it. - **Non-blocking — the stale base line.** The old sentence naming a `main` base that was only true at claim time is gone; the base history below is the real path to this tip. Advisory rounds: none at this tip yet — Chief freezes before a PASS request. Superseded as evidence and kept as record: `6415907830` (Test) and `6415965572` (Security) at `5385491f`; `6415574602` and `6415627981` at `86e9795`; `6415419164` at `bdf867b2`; `6415205741` and `6415207539` at `150aeaa`. ## Why these files, and the base `main` moved several times while this was built: the branch was cut at `99b1fab`, hand-re-applied onto ThinkFlowLab#1276's new card model (`taskCard` / `BoardCardFrame`), then rebased onto `b4e1faa` (ThinkFlowLab#1284, which rewrote `docs/subsystems/core-host.md`, a file in this diff), `63f81f0` (ThinkFlowLab#1278), `ef5dae42` (ThinkFlowLab#1291) and now **`e37f1f83`** (ThinkFlowLab#1296 and ThinkFlowLab#1297, both landed after `ef5dae42`). The tip is therefore the merged tree: current `main` + this diff. `ThinkFlowLab#1297`'s delivery-board test in `core/test/kanban.test.js` was kept alongside this board's tests — the conflict was resolved by keeping both sides — and the board text still sits in `### Bot task board (ThinkFlowLab#1058)`, with no contradiction of ThinkFlowLab#1267's mid-turn-line subsection. At this tip the pull request is **19 files, +1007/−104, one commit** on base `e37f1f83`. - `core/src/host/tasks.js` — the read contract, the bounded archive read, the overall-card completion, and the settle on every door a child ends through. - `core/src/client/kanban.js` — the archive read (Bot-scoped on a Bot board), newest-first ordering, the twenty-card column. - `core/src/client/inbox.js` — the one shared task load asks for the archive, so the Inbox keeps its Done and Needs-you cards. - `core/client.js` — generated by `npm run build` in `core/`, never edited by hand. - `core/test/bot-tasks.test.js` — the eight ThinkFlowLab#1252 host cases (active-only board and explicit archive; last child completes the overall card; a failed child fails it; the requested archive bound with the unbounded read still seeing every row; a busy room's bounded read carrying finished rows only, with the `limit` type refusals; a deleted Bot failing the overall card; a rejected approval cancelling the child and completing the overall card; a Bot-scoped page cut after the Bot filter) plus one compat read: the post-restart read of a finished card now asks for the archive. This is where the active-only/archive case lives, not `core/test/tasks.test.js`. - `core/test/inbox.test.js` — the Inbox drive: the read posts `{ archive: true }`, a `failed` card lands in Needs you, an `accepted` card lands in Done, and the row badge counts the failed card. The shared harness keeps its no-op-effect stub for every other case; only this drive runs its effect. - `core/test/tasks.test.js` — only the `statusOf` helper changes: it reads `{ archive: true }` (no limit) because it looks tasks up by id and some are finished. - `core/test/kanban.test.js` — the client fixture: 37 finished cards give a literal twenty-card column starting at the newest, the oldest is outside the slice, a card with an old `createdAt` and a fresh `updatedAt` leads the column, and a driven load asserts the posted body (Bot board `botId`, group `roomId`, default read unfiltered). - `docs/subsystems/core-host.md` — the read contract with and without `limit`, the bot/room filter before the cut, and when an overall card completes accepted or failed (owned page for `tasks.js`). - `docs/subsystems/core-client.md` — the board's two reads, the newest-first cut, a Bot board naming its Bot, and the uncapped id-lookup path; the duplicated cap sentence is merged (owned page for `kanban.js`). - `changelog.d/1252-board-archive.changed.md` — user-visible board and Inbox change. - `patches/ds-bot/0008-client-bundle.patch` and the `patches/ds-bot/files/` mirrors — written by `node scripts/ds-bot-patches.mjs generate` from that same `core/` tree; the generator's context-only rewrite of `0005`, `0012` and `0014` was reverted, and `check` re-run green. ## Decisions recorded, not omissions - The board's archive read is capped at the same limit as the column (`{ ...query, archive: true, limit: ARCHIVE_SHOWN }`, room-scoped for a group, `{ botId }` for a Bot board, newest-settled first, finished rows only — the default read carries the active ones), so the poll carries the slice and not the history. The archive read **without** `limit` stays uncapped on purpose: it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row, and now the Inbox), not what the board polls. - The `counts` loop still runs over `[...own, ...archive]` unchanged, and no counts assertion moved: the finished-only page is what stops a card appearing in both lists and being counted twice. - A child's requester can complete the room's overall card: the settle path runs the same transition with no actor, so the card's status moves and the requester gate is not consulted. It is status-only — nothing is deleted, and the card's actors and history are kept. - The bounded page is `settled`, while the Done column keeps only `accepted`: twenty newer `cancelled`/`failed` rows in one scope can spend the page's budget and push an older Done card out of the column. Left as it is at this tip, deliberately: the page is the recent-activity slice ordered by `updatedAt`, and narrowing it to `accepted` would make a "newest finished" read lie about cancelled/failed work other callers read. An older card is still readable without `limit`, which stays uncapped on purpose. ## What I ran At `5b832b91` (tree `f14bf9fd`, base `e37f1f83`). - `TZ=UTC npm run verify` — exit 0: core **1784 tests, 1784 pass, 0 fail**, plugins/social 525, plugins/tree 125, plugins/timeline 125, `.github/scripts` 509, interop 37, live-e2e 24; every suite green. - the ThinkFlowLab#1252 suites on their own at the same tree: `core/test/bot-tasks.test.js` 14/14, `core/test/kanban.test.js` 7/7, `core/test/inbox.test.js` 27/27. - `npm run build` in `core/` before `generate`; `node scripts/ds-bot-patches.mjs generate` then `check` — `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/`. The generator's context-only rewrite of `0005`, `0012` and `0014` was reverted and `check` re-run green. - `node core/scripts/build-client.mjs --check` inside `verify` — the committed `core/client.js` matches `core/src/` at this tip. ## Not run - The smoke bench (`core/dev/bench.sh smoke`): it needs a live DSH instance and a free port; this touches `src/host/tasks.js`, `src/client/kanban.js` and `src/client/inbox.js`, not `index.js`, and the host behaviour is covered by `core/test/bot-tasks.test.js`, `core/test/tasks.test.js`, `core/test/inbox.test.js` and `core/test/task-drive.test.js`. - Packaging, deploy and the desktop app — untouched. ## Verification at the tip Everything above ran at `5b832b91f8ebdb1b7de782e87d40f35b24f6549d` — the tip this pull request pushes, tree `f14bf9fd5a0e75746d39c4168f57cb0ad1b719b2`, base `e37f1f838023ce81e673d8fbec66a55d81dfe5e3` (19 files, +1007/−104, one commit). ``` 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/ ``` ## Checkpoint The generated mirrors — `patches/ds-bot/files/src/{client/inbox.js,client/kanban.js,host/tasks.js}` and `patches/ds-bot/files/test/{bot-tasks,inbox,kanban,tasks}.test.js` — are copies written by the generator from the files above, so the block lists the 12 paths a reviewer should open first; the diff itself is the full 19. <!-- flybot:checkpoint --> ``` { "pr": 1289, "claim": "Dev-1", "issue": 1252, "summary": "A finished card no longer ships with every board read: the tasks endpoint lists active work, { archive: true } adds the finished rows, and { archive: true, limit: N } returns the newest N finished rows, so the board's poll carries twenty settled cards and no active ones. A Bot board names its Bot there, and the Inbox asks for the archive so its Done and Needs-you cards return. The overall group card completes when every child has ended — accepted if each ended accepted or cancelled, failed if any failed. Tests were red first; docs, the client bundle and the patches ride in the same commit.", "files": [ "changelog.d/1252-board-archive.changed.md", "core/client.js", "core/src/client/inbox.js", "core/src/client/kanban.js", "core/src/host/tasks.js", "core/test/bot-tasks.test.js", "core/test/inbox.test.js", "core/test/kanban.test.js", "docs/subsystems/core-client.md", "docs/subsystems/core-host.md", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/host/tasks.js" ], "checks": { "ran": "TZ=UTC npm run verify; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "5b832b91f8ebdb1b7de782e87d40f35b24f6549d" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1277, step 1 of #1265 (the triage comment on the umbrella splits it into four pull requests; this one is the card identity). Columns stay as they are, and 3 (records linked under issue cards) and 4 (multiple repositories) are later pull requests.
Owner: Dev4
Reviewer: Reviewer
Written-by: Dev4
Tip SHA:
fa19ecb7101fd60a0c630ed99071233067aac5a1Reproduce
Today the same-looking card is two implementations with no shared identity.
core/src/client/delivery-view.js'sDeliveryCarddraws a GitHub row keyed only bydata-kind/data-number;core/src/client/kanban.js'sBoardCarddraws a task record withdata-task. Nothing on screen or in the DOM says which board a card came from, and a GitHub card has no key that survives the row arriving as an issue in one column and as the pull request that closes it in another — which is exactly what step 3 has to hang a FlyBot record on.What changed
core/src/shared/board-card.js(new):githubCardKey(repo, number)isowner/name#number— deliberately not keyed by kind, so an issue and the pull request that closes it are one card — andtaskCardKey(id)istask:<id>.githubCard(row, repo)andtaskCard(task)build the model,sameCard(a, b)is the one comparison. A row with no repository falls back togithub:<kind>:<number>instead of implying a cross-repository identity it does not have.core/src/client/board-card.js(new): oneBoardCardcomponent behind both boards. It draws the identity line, title, labels, owner, note, linked/sha/url and the caller's slots (priority, status, progress, actions). Each board passes its ownskin(DELIVERY_SKIN/TASK_SKIN), so both stylesheets stay untouched until step 2 collapses the columns, and every card now carriesdata-sourceanddata-key.core/src/client/delivery-view.js:DeliveryCardis a thin caller of that component; the board passes the snapshot'srepodown, so the card is keyedowner/name#number.core/src/client/kanban.js: the task card is a thin caller too, keepingdata-task/data-column/data-priorityand the Start / Complete actions for this board's own selectors and drag-and-drop.docs/subsystems/core-client.mdandcore-host.mdrows for the two new files;changelog.d/1265-board-cards.added.md;core/client.jsrebuilt;patches/ds-bot/files/regenerated (the three new files are whole-file entries).What I ran
At
fa19ecb7101fd60a0c630ed99071233067aac5a1(base = livemain4095e135):Red without the source change
Two knives, each reverted in
core/and restored afterwards (git statusclean,build-client --checkexit 0,ds-bot-patches checkgreen again):githubCardKeyreturningowner/name/numberinstead ofowner/name#numbernot ok 1 - a repository card is keyed by repository and number, so an issue and its pull request are one card—expected: 'o/r#12' actual: 'o/r/12'; alsonot ok 4(the component test) andnot ok 10(the delivery board'sdata-keycheck), 3 faildata-sourcenot ok 4—expected: 'github'(undefined);not ok 10(delivery-view) —expected: 'github';not ok 27(kanban) —expected: 'task', 3 failNot run
npm teston this box: the whole suite still hitsspawn … EAGAIN; CI on Node 24 is the reference.npm run verifyas one command, for the same reason — its steps are the individual commands above.core/src/client/only, notindex.jsorsrc/host/.Risk
core/client.jsshows a large diff for a small source delta: adding a module renumbers esbuild's deduplicated identifiers (h47→h48and so on).build-client --checkis the proof it is the generator's output, not a hand edit.skin, so this is a markup/AST change with no visual change; the task card's note now renders before its group/part-of lines, which is the one ordering difference.data-source/data-keyare new attributes on existing elements; nothing reads them yet — step 3 will.Checkpoint
{ "pr": 1276, "claim": "Dev4", "issue": 1277, "summary": "Step 1 of one board: a card now says which side it came from and carries one identity. A repository card is keyed owner/name#number (not by kind, so an issue and the pull request closing it are one card) and a task record task:<id>; a row with no repository falls back to a kind-qualified key instead of implying a cross-repository identity. Both boards draw through one BoardCard component that takes a per-board skin, so the delivery and task stylesheets stay as they are until step 2 collapses the columns.", "files": [ "core/src/shared/board-card.js", "core/src/client/board-card.js", "core/src/client/delivery-view.js", "core/src/client/kanban.js", "core/test/board-card.test.js", "docs/subsystems/core-host.md" ], "checks": { "ran": "on fa19ecb7 (base 4095e135): node scripts/ds-bot-patches.mjs check; node core/scripts/build-client.mjs --check; arch-docs; oxlint; changelog; mojibake; eleven core suites (120 tests); two knives", "result": "green", "at": "fa19ecb7101fd60a0c630ed99071233067aac5a1" }, "next": "none — step 1 ready for review; steps 2 (columns), 3 (records under issue cards) and 4 (multiple repositories) remain" }