diff --git a/changelog.d/2069-room-rail-double-count.fixed.md b/changelog.d/2069-room-rail-double-count.fixed.md new file mode 100644 index 000000000..1eb7e1a72 --- /dev/null +++ b/changelog.d/2069-room-rail-double-count.fixed.md @@ -0,0 +1 @@ +A room board opened from a Bot board now counts each active card once in its rail: the bot board's read stayed in the rail's own list after the switch, so a room that draws two cards read `1/3` instead of `1/2` ([#2069](https://github.com/ThinkFlowLab/FlyBot/issues/2069)). diff --git a/core/client.js b/core/client.js index c481cec0a..22842a8a5 100644 --- a/core/client.js +++ b/core/client.js @@ -9122,7 +9122,7 @@ function KanbanDialog({ roster, actions, leaving = false }) { }, []); const archiveFull = Array.isArray(archive) && archive.length >= ARCHIVE_SHOWN; const counts2 = {}; - for (const task of [...own, ...scope?.kind === "group" ? groupTasks : [], ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { + for (const task of [...scope?.kind === "group" ? groupTasks : own, ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { const id = task.ownerBotId; const bucket = counts2[id] ?? (counts2[id] = { done: 0, total: 0 }); if (task.status === "cancelled" || task.status === "failed") continue; diff --git a/core/src/client/kanban.js b/core/src/client/kanban.js index c1c8c4857..a49039e03 100644 --- a/core/src/client/kanban.js +++ b/core/src/client/kanban.js @@ -398,9 +398,10 @@ export function KanbanDialog({ roster, actions, leaving = false }) { const counts = {} // #2045: the rail counts what the board draws, so the archive contributes at most its one page. // The list can hold a row more than the page after an optimistic move, and counting that row would - // read as a total the board never showed. P-364: a room board's active cards live in `groupTasks`; - // `own` is only ever filled on a Bot board, so adding up `own` alone counted the archive page. - for (const task of [...own, ...(scope?.kind === 'group' ? groupTasks : []), ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { + // read as a total the board never showed. P-364: a room board's active cards live in `groupTasks`, + // so a Bot board counts `own` and a room board counts that list, never both — `own` still holds the + // Bot board's read after the rail switches, and adding it in counted the room's cards twice (#2069). + for (const task of [...(scope?.kind === 'group' ? groupTasks : own), ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { const id = task.ownerBotId const bucket = counts[id] ?? (counts[id] = { done: 0, total: 0 }) if (task.status === 'cancelled' || task.status === 'failed') continue diff --git a/core/test/kanban.test.js b/core/test/kanban.test.js index c8b5ffc7b..44bfcaf57 100644 --- a/core/test/kanban.test.js +++ b/core/test/kanban.test.js @@ -325,7 +325,11 @@ const boardWith = (scope, own, archive, groupActive = []) => { states.length = 0 // A room board's active list lives in the second of the three board-task slots (P-364); a Bot // board's lives in the first. Both are preset here rather than loaded, like the archive page. - states.push(scope, null, '', '', own, groupActive, archive) + // #2069: a room is only ever reached from a Bot board — the dialog opens on `rail.bots[0]` — so + // `own` is never empty when a room is drawn; presetting it to `[]` here made the rail's double + // count invisible. Seed it from the Bot board's read of the same room cards. + const botRead = scope?.kind === 'group' ? groupActive : own + states.push(scope, null, '', '', botRead, groupActive, archive) const view = expand(KanbanDialog({ roster, actions: { closeBoard: () => {}, boardClose: () => {} } })) const column = name => byClass(view, 'bt-board-column').find(node => node.props['data-column'] === name) const railMeta = id => { @@ -542,8 +546,20 @@ function mountBoard(actions) { assert.ok(button, `the rail names Bot ${botId}`) button.props.onClick() } + // #2069: a room board is reached by clicking a room in the same rail, so the harness drives both + // kinds of row, not only the Bot rows P-360 needed. + const room = (roomId) => { + const button = all(tree).find(node => node.type === 'button' && node.props?.['data-kind'] === 'group' && node.props?.['data-id'] === roomId) + assert.ok(button, `the rail names room ${roomId}`) + button.props.onClick() + } + const railMeta = (id) => { + const row = all(tree).find(node => node.type === 'button' && node.props?.['data-id'] === id) + const meta = byClass(row, 'bt-board-rail-meta')[0] + return meta ? textOf(meta) : '' + } const column = (name) => byClass(tree, 'bt-board-column').find(node => node.props['data-column'] === name) - return { draw, settle, rail, column, get tree() { return tree } } + return { draw, settle, rail, room, railMeta, column, get tree() { return tree } } } // A Bot board's Done column is that Bot's own archive page. Switching the rail therefore has to @@ -592,3 +608,40 @@ test('P-360: switching the rail re-reads the archive, and the new Bot draws its assert.equal(byClass(board.column('done'), 'bt-board-card').length, ARCHIVE_SHOWN, 'Chief draws its own finished cards again') assert.match(textOf(board.column('done')), /A finished 19/) }) + +// #2069: a room board reached from a Bot board counted the room's active cards twice. The dialog +// always opens on the first Bot of the rail, and that Bot's default read is the unfiltered active +// list — a card in a room the Bot may act on lands in `own` as well as in the room's own read — so +// the rail added the same card from both lists. The card the room board draws once was counted +// twice; the room's drawn columns never showed the second copy, only the rail did. +test('#2069: a room board reached from a Bot board counts each active card once in the rail', async () => { + const calls = [] + const roomCard = { id: 'task-open-s', ownerBotId: 's', title: 'Open for s', status: 'open', progress: 0, createdAt: 3, updatedAt: 4 } + const finishedCard = { id: 'task-done-s', ownerBotId: 's', title: 'Finished for s', status: 'accepted', progress: 100, createdAt: 1, updatedAt: 2 } + const actions = { + closeBoard: () => {}, + boardClose: () => {}, + tasks: (query) => { + calls.push({ ...query }) + if (query?.archive) return Promise.resolve([finishedCard]) + if (query?.roomId) return Promise.resolve([roomCard]) + // The host's default read carries every card the Bot may act on, room cards included. + return Promise.resolve([roomCard]) + }, + } + const board = mountBoard(actions) + board.draw() + await board.settle() + assert.deepEqual(calls[0], {}, 'the board opens on the default active read') + // The Bot board's read really did fill `own`: the rail already counts the room card there. + assert.equal(board.railMeta('s'), '0/1', 'the Bot board rail counted the room card its read returned') + + board.room('war') + await board.settle() + assert.deepEqual(calls.filter(query => query?.roomId && !query.archive).at(-1), { roomId: 'war' }, 'the room board reads the room\'s active cards') + // The room board draws the card once, so the rail counts it once: one finished card in the archive + // page plus the one active card is `1/2`, not the `1/3` the doubled list produced. + assert.equal(byClass(board.column('not-started'), 'bt-board-card').length, 1, 'the room\'s active card is drawn once') + assert.equal(byClass(board.column('done'), 'bt-board-card').length, 1, 'the room\'s finished card is drawn once') + assert.equal(board.railMeta('s'), '1/2', 'the rail counts the drawn active card once, plus the archive page') +}) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index ecbd608f4..7c59981d0 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -45,7 +45,7 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl | `core/src/shared/preview.js` | What a preview may show of a stored line (#130, #1347): `visibleText(text)` unwraps complete `` wraps and then removes leftover marker fragments until neither changes the text — a `from="User">` tail, a `untrusted-message>` whose brackets an older pass already stripped, a nested fragment that would otherwise rebuild a marker — and folds the result to the one line a sidebar row is. `stripFence` is the marker half on its own. It is used where a row is built (`previewOf` in `src/client/text.js`, the group row in `src/client/sidebar.js`, the host's `preview` in `src/host/channel.js`), so no surface has to remember it, and by the details drawer's copy export: `transcriptOf` (`src/client/text.js`) reads each stored turn through the same helper, so a copied conversation carries the words, not an untrusted fence, and `copyBodyOf` drops a leading delegation trace only when its first hop is a Bot's name taken from the roster (`copyNames`; longest first, and no names strips nothing), which keeps a line like `Settings → Models 页面` as written; a copied reply keeps its own line breaks, and only a trace-only line or a fence-marker line is dropped. Tests: `core/test/ui-clickthrough.test.js`, `core/test/details-copy.test.js`. | | `core/src/client/board-card.js` | The task board's card frame (#1363): `BoardCard` draws the identity line, the title, the note and the actions the board passes in, with `TASK_SKIN`. A card carries `data-source` (`task`) and `data-key` (`task:`). The caller is `kanban.js`. Tests: `core/test/board-card.test.js`, `core/test/kanban.test.js`. | | `core/src/shared/board-card.js` | Task-card identity (#1363): `taskCardKey(id)` is `task:` and `taskCard(task)` builds the model `BoardCard` draws. Tests: `core/test/board-card.test.js`. | -| `core/src/client/kanban.js` | Per-Bot task board (#1058, #1099): the **Tasks** row under Inbox (`KanbanRow`, `ui.board`, `actions.openBoard` / `closeBoard`) and `KanbanDialog`. `boardRail` lists the viewer's Bots nested by `createdBy` and the groups they administer; `boardColumns` maps task status onto **Not started** / **In progress** / **Done**. Cards show id, priority, title, note, assignee and progress, and are drawn by the shared `board-card.js` component: a task card says `source: 'task'`, keys `task:` and keeps `data-task` / `data-column` for this board's own selectors (#1265 step 1). The dialog body is `BoardView` (#1363): the rail stays per-Bot and the columns are the task columns only. The GitHub metrics panel (Today / 7 days, Retry, Diagnose, the package-default repository header) is not mounted. A task card is the shared `BoardCard` with `TASK_SKIN`, and dragging it or dropping it on a column only reports the request to the dialog's `onMove` (`actions.taskUpdate`), so the view still never writes. In a group scope, `columnTree` nests subtasks under the overall task (`kind: 'group'`) when they share a column, and a subtask in another column says which overall task it is part of. That scope also says assigning, claiming, and progress live on the board (overall task, subtask, assignee, column, progress) and that @ only wakes someone. Drag or **Start** / **Complete** calls `actions.taskUpdate`; only the Bot's owner gets those actions. A group scope loads `tasks { roomId }`. Since #1252 the default read is active work only and the Done column merges a second read of the finished rows — `tasks { ...query, archive: true, limit: ARCHIVE_SHOWN }`, which is room-scoped for a group and names the Bot for a Bot board, because the host cuts the page before the board can filter by Bot — newest-settled first by `updatedAt` (tiebreak `createdAt`), finished rows only, and cut to that limit, so a room with hundreds of finished cards still renders a twenty-card column while the poll carries the slice and not the history. Since #2045 that page is also counted for what it is: when the archive holds a full page (`ARCHIVE_SHOWN`), the Done heading reads `20 newest` and a Bot's rail row reads `20+ done` rather than a fraction that would present the page as the history, and in a group the page is cut across the room, so every Bot's share of it is a lower bound and reads the same way rather than presenting its slice as its own total; a page under the limit keeps `{done}/{total}` exactly as before. P-364: the rail adds up the list the scope actually draws from, and a room board keeps its active cards in a second list of its own (`groupTasks`) — `own` is only ever filled on a Bot board — so a room rail counts the room's active cards as well as the archive page, reading `1/4` beside four drawn cards and `0/1` for a Bot whose cards are all still active, while a Bot board's numbers are unchanged. P-360: a Bot board's load effect is keyed by the Bot it shows, so switching the rail re-reads the new Bot's active list and archive page — the page it kept was that of the Bot it left, and filtering it by the new Bot emptied its Done column while the poll kept refilling the old one. That read without `limit` stays uncapped on purpose: it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row), not what the board polls. Finishing a card moves it off the active list at once, before the next poll. Tests: `core/test/kanban.test.js`. | +| `core/src/client/kanban.js` | Per-Bot task board (#1058, #1099): the **Tasks** row under Inbox (`KanbanRow`, `ui.board`, `actions.openBoard` / `closeBoard`) and `KanbanDialog`. `boardRail` lists the viewer's Bots nested by `createdBy` and the groups they administer; `boardColumns` maps task status onto **Not started** / **In progress** / **Done**. Cards show id, priority, title, note, assignee and progress, and are drawn by the shared `board-card.js` component: a task card says `source: 'task'`, keys `task:` and keeps `data-task` / `data-column` for this board's own selectors (#1265 step 1). The dialog body is `BoardView` (#1363): the rail stays per-Bot and the columns are the task columns only. The GitHub metrics panel (Today / 7 days, Retry, Diagnose, the package-default repository header) is not mounted. A task card is the shared `BoardCard` with `TASK_SKIN`, and dragging it or dropping it on a column only reports the request to the dialog's `onMove` (`actions.taskUpdate`), so the view still never writes. In a group scope, `columnTree` nests subtasks under the overall task (`kind: 'group'`) when they share a column, and a subtask in another column says which overall task it is part of. That scope also says assigning, claiming, and progress live on the board (overall task, subtask, assignee, column, progress) and that @ only wakes someone. Drag or **Start** / **Complete** calls `actions.taskUpdate`; only the Bot's owner gets those actions. A group scope loads `tasks { roomId }`. Since #1252 the default read is active work only and the Done column merges a second read of the finished rows — `tasks { ...query, archive: true, limit: ARCHIVE_SHOWN }`, which is room-scoped for a group and names the Bot for a Bot board, because the host cuts the page before the board can filter by Bot — newest-settled first by `updatedAt` (tiebreak `createdAt`), finished rows only, and cut to that limit, so a room with hundreds of finished cards still renders a twenty-card column while the poll carries the slice and not the history. Since #2045 that page is also counted for what it is: when the archive holds a full page (`ARCHIVE_SHOWN`), the Done heading reads `20 newest` and a Bot's rail row reads `20+ done` rather than a fraction that would present the page as the history, and in a group the page is cut across the room, so every Bot's share of it is a lower bound and reads the same way rather than presenting its slice as its own total; a page under the limit keeps `{done}/{total}` exactly as before. P-364: the rail adds up the list the scope actually draws from, and a room board keeps its active cards in a second list of its own (`groupTasks`), so a room rail counts the room's active cards as well as the archive page, reading `1/4` beside four drawn cards and `0/1` for a Bot whose cards are all still active, while a Bot board's numbers are unchanged. #2069: it counts one list and never both — `own` still holds the Bot board's read after the rail switches to a room, and adding it counted the room's active cards twice, so a room board that draws two cards read `1/3` where it now reads `1/2`. P-360: a Bot board's load effect is keyed by the Bot it shows, so switching the rail re-reads the new Bot's active list and archive page — the page it kept was that of the Bot it left, and filtering it by the new Bot emptied its Done column while the poll kept refilling the old one. That read without `limit` stays uncapped on purpose: it is the id-lookup and tooling path (`statusOf`, a Bot reading a settled row), not what the board polls. Finishing a card moves it off the active list at once, before the next poll. Tests: `core/test/kanban.test.js`. | | `core/src/client/standing-approvals.js` | Standing tool approvals (#540): `StandingApprovalsCard`, the **Always allowed** card in Bot settings (the Bot's grants) and Group settings (the grants this person gave in the group), one row per kind with **Remove** (`actions.removeStandingApproval`). The approval cards (`member-status.js` `ApprovalCard`, the inbox card) add **Always allow for this Bot** and, for an ask from a group, **Always allow for this group** (`alwaysChoices`; never on a friend's Bot). Host side in [core-host](core-host.md#standing-tool-approvals-540). Tests: `core/test/member-status.test.js`, `core/test/standing-approvals.test.js`. | | `core/src/client/mobile.js` | Phone-width layout, step 1 (#362): `MOBILE_CSS` under `@media (max-width:640px)` for held-reply and floating approval buttons, the chat pointers and Agent inbox cards. Tests: `core/test/mobile-layout.test.js`. | diff --git a/patches/ds-bot/0008-client-bundle.patch b/patches/ds-bot/0008-client-bundle.patch index 6316f2440..377e4208e 100644 --- a/patches/ds-bot/0008-client-bundle.patch +++ b/patches/ds-bot/0008-client-bundle.patch @@ -10538,7 +10538,7 @@ diff --git a/client.js b/client.js + }, []); + const archiveFull = Array.isArray(archive) && archive.length >= ARCHIVE_SHOWN; + const counts2 = {}; -+ for (const task of [...own, ...scope?.kind === "group" ? groupTasks : [], ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { ++ for (const task of [...scope?.kind === "group" ? groupTasks : own, ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { + const id = task.ownerBotId; + const bucket = counts2[id] ?? (counts2[id] = { done: 0, total: 0 }); + if (task.status === "cancelled" || task.status === "failed") continue; diff --git a/patches/ds-bot/files/src/client/kanban.js b/patches/ds-bot/files/src/client/kanban.js index c1c8c4857..a49039e03 100644 --- a/patches/ds-bot/files/src/client/kanban.js +++ b/patches/ds-bot/files/src/client/kanban.js @@ -398,9 +398,10 @@ export function KanbanDialog({ roster, actions, leaving = false }) { const counts = {} // #2045: the rail counts what the board draws, so the archive contributes at most its one page. // The list can hold a row more than the page after an optimistic move, and counting that row would - // read as a total the board never showed. P-364: a room board's active cards live in `groupTasks`; - // `own` is only ever filled on a Bot board, so adding up `own` alone counted the archive page. - for (const task of [...own, ...(scope?.kind === 'group' ? groupTasks : []), ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { + // read as a total the board never showed. P-364: a room board's active cards live in `groupTasks`, + // so a Bot board counts `own` and a room board counts that list, never both — `own` still holds the + // Bot board's read after the rail switches, and adding it in counted the room's cards twice (#2069). + for (const task of [...(scope?.kind === 'group' ? groupTasks : own), ...(archive ?? []).slice(0, ARCHIVE_SHOWN)]) { const id = task.ownerBotId const bucket = counts[id] ?? (counts[id] = { done: 0, total: 0 }) if (task.status === 'cancelled' || task.status === 'failed') continue diff --git a/patches/ds-bot/files/test/kanban.test.js b/patches/ds-bot/files/test/kanban.test.js index c8b5ffc7b..44bfcaf57 100644 --- a/patches/ds-bot/files/test/kanban.test.js +++ b/patches/ds-bot/files/test/kanban.test.js @@ -325,7 +325,11 @@ const boardWith = (scope, own, archive, groupActive = []) => { states.length = 0 // A room board's active list lives in the second of the three board-task slots (P-364); a Bot // board's lives in the first. Both are preset here rather than loaded, like the archive page. - states.push(scope, null, '', '', own, groupActive, archive) + // #2069: a room is only ever reached from a Bot board — the dialog opens on `rail.bots[0]` — so + // `own` is never empty when a room is drawn; presetting it to `[]` here made the rail's double + // count invisible. Seed it from the Bot board's read of the same room cards. + const botRead = scope?.kind === 'group' ? groupActive : own + states.push(scope, null, '', '', botRead, groupActive, archive) const view = expand(KanbanDialog({ roster, actions: { closeBoard: () => {}, boardClose: () => {} } })) const column = name => byClass(view, 'bt-board-column').find(node => node.props['data-column'] === name) const railMeta = id => { @@ -542,8 +546,20 @@ function mountBoard(actions) { assert.ok(button, `the rail names Bot ${botId}`) button.props.onClick() } + // #2069: a room board is reached by clicking a room in the same rail, so the harness drives both + // kinds of row, not only the Bot rows P-360 needed. + const room = (roomId) => { + const button = all(tree).find(node => node.type === 'button' && node.props?.['data-kind'] === 'group' && node.props?.['data-id'] === roomId) + assert.ok(button, `the rail names room ${roomId}`) + button.props.onClick() + } + const railMeta = (id) => { + const row = all(tree).find(node => node.type === 'button' && node.props?.['data-id'] === id) + const meta = byClass(row, 'bt-board-rail-meta')[0] + return meta ? textOf(meta) : '' + } const column = (name) => byClass(tree, 'bt-board-column').find(node => node.props['data-column'] === name) - return { draw, settle, rail, column, get tree() { return tree } } + return { draw, settle, rail, room, railMeta, column, get tree() { return tree } } } // A Bot board's Done column is that Bot's own archive page. Switching the rail therefore has to @@ -592,3 +608,40 @@ test('P-360: switching the rail re-reads the archive, and the new Bot draws its assert.equal(byClass(board.column('done'), 'bt-board-card').length, ARCHIVE_SHOWN, 'Chief draws its own finished cards again') assert.match(textOf(board.column('done')), /A finished 19/) }) + +// #2069: a room board reached from a Bot board counted the room's active cards twice. The dialog +// always opens on the first Bot of the rail, and that Bot's default read is the unfiltered active +// list — a card in a room the Bot may act on lands in `own` as well as in the room's own read — so +// the rail added the same card from both lists. The card the room board draws once was counted +// twice; the room's drawn columns never showed the second copy, only the rail did. +test('#2069: a room board reached from a Bot board counts each active card once in the rail', async () => { + const calls = [] + const roomCard = { id: 'task-open-s', ownerBotId: 's', title: 'Open for s', status: 'open', progress: 0, createdAt: 3, updatedAt: 4 } + const finishedCard = { id: 'task-done-s', ownerBotId: 's', title: 'Finished for s', status: 'accepted', progress: 100, createdAt: 1, updatedAt: 2 } + const actions = { + closeBoard: () => {}, + boardClose: () => {}, + tasks: (query) => { + calls.push({ ...query }) + if (query?.archive) return Promise.resolve([finishedCard]) + if (query?.roomId) return Promise.resolve([roomCard]) + // The host's default read carries every card the Bot may act on, room cards included. + return Promise.resolve([roomCard]) + }, + } + const board = mountBoard(actions) + board.draw() + await board.settle() + assert.deepEqual(calls[0], {}, 'the board opens on the default active read') + // The Bot board's read really did fill `own`: the rail already counts the room card there. + assert.equal(board.railMeta('s'), '0/1', 'the Bot board rail counted the room card its read returned') + + board.room('war') + await board.settle() + assert.deepEqual(calls.filter(query => query?.roomId && !query.archive).at(-1), { roomId: 'war' }, 'the room board reads the room\'s active cards') + // The room board draws the card once, so the rail counts it once: one finished card in the archive + // page plus the one active card is `1/2`, not the `1/3` the doubled list produced. + assert.equal(byClass(board.column('not-started'), 'bt-board-card').length, 1, 'the room\'s active card is drawn once') + assert.equal(byClass(board.column('done'), 'bt-board-card').length, 1, 'the room\'s finished card is drawn once') + assert.equal(board.railMeta('s'), '1/2', 'the rail counts the drawn active card once, plus the archive page') +})