From 492f73e55f66af3a057c0e0b381c6fffd6708299 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Tue, 27 Oct 2026 12:38:16 +0800 Subject: [PATCH] fix(tasks): a dispatch reuses a card only when its own fields name the issue (#1331) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rt.assignFromDispatch` built one match string from `title`, `note` and `target`, so `namesIssue` matched a number the note merely mentioned: a card `#999 fix the other thing` carrying `blocked on #1234 first` was rewritten to the dispatch's note and restarted, waking that member for `#1234` on the `#999` card, and `#1234` itself got no card at all. The `held` lookup now reads `title`/`target` first and falls back to `note` only when neither of them names any issue, so a mention is not ownership while one issue still stays one card. `namesIssue`/`issueRefs` and the dispatch shape rules in `work-dispatch.js` are untouched. Tests: `core/test/work-dispatch.test.js` gets the probe above (red on the base) and the other half — a card whose own fields name no issue is still reused. Mirrors come from `node scripts/ds-bot-patches.mjs generate`; the subsystem doc sentence that said "title, note or target" is updated rather than excused. --- changelog.d/1331-work-dispatch.fixed.md | 1 + core/src/host/tasks.js | 14 ++++++- core/test/work-dispatch.test.js | 40 +++++++++++++++++++ docs/subsystems/core-host.md | 2 +- patches/ds-bot/files/src/host/tasks.js | 14 ++++++- .../ds-bot/files/test/work-dispatch.test.js | 40 +++++++++++++++++++ 6 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 changelog.d/1331-work-dispatch.fixed.md diff --git a/changelog.d/1331-work-dispatch.fixed.md b/changelog.d/1331-work-dispatch.fixed.md new file mode 100644 index 000000000..8d8836d1f --- /dev/null +++ b/changelog.d/1331-work-dispatch.fixed.md @@ -0,0 +1 @@ +A group work dispatch now reuses a member's card only when that card names the issue itself. `title` and `target` are read first, and a card's `note` counts only when neither of them names any issue — so a card for `#999` whose note says "blocked on #1234 first" is no longer rewritten and restarted for `#1234`, and the dispatch opens `#1234` its own card. One issue still gets one card: a card that names the issue only in its note is still reused. ([#1331](https://github.com/ThinkFlowLab/FlyBot/issues/1331)) diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index 32fa5ee0f..d3034c769 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -18,7 +18,7 @@ import { randomBytes } from 'node:crypto' import { columnOf, compareForQueue, compareForWork, sameRole } from '../shared/task-board.js' import { currentParticipants, deriveParticipants } from './room-membership.js' -import { namesIssue } from './work-dispatch.js' +import { issueRefs, namesIssue } from './work-dispatch.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) // result-ready still waits for the requester: prune must not evict it with terminals (#111). @@ -500,6 +500,16 @@ export function install(rt) { } } + // #1331: a card belongs to the issue its own fields name. `title`/`target` name it; a note that + // merely mentions the id ("blocked on #1234 first") does not, or a dispatch for #1234 rewrites the + // #999 card and leaves #1234 with no card at all. The note is still read for a card that names no + // issue anywhere else, so #1254's "one issue is one card" holds for cards written from a note. + const cardNamesIssue = (task, wanted) => { + const own = `${task.title ?? ''} ${task.target?.href ?? ''}` + if (namesIssue(own, wanted)) return true + return issueRefs(own).length === 0 && namesIssue(task.note ?? '', wanted) + } + // #1254: the room's task lead naming one member with an issue id and an imperative is an assign, // not a round (`groups.js` `dispatchOf`). The card that member already holds for that issue is // updated; otherwise one is opened, owned by that member and still `open` — the host assigns, and @@ -519,7 +529,7 @@ export function install(rt) { && openTask(task)) const held = issue === undefined || issue === null || String(issue).trim() === '' ? undefined - : open.filter(task => namesIssue(`${task.title ?? ''} ${task.note ?? ''} ${task.target?.href ?? ''}`, issue)).sort(newestFirst)[0] + : open.filter(task => cardNamesIssue(task, issue)).sort(newestFirst)[0] if (held) { held.note = text // Both paths below append to `actors`, and `record()` pushes without a guard, so a card diff --git a/core/test/work-dispatch.test.js b/core/test/work-dispatch.test.js index b202e43e7..24c1683a0 100644 --- a/core/test/work-dispatch.test.js +++ b/core/test/work-dispatch.test.js @@ -268,6 +268,46 @@ test('a dispatch reuses a card blocked on an input and restarts it (#1254)', asy assert.ok(app.wakeOf('b')) }) +// #1331: a card is that issue's card only when its own fields name the issue. The card below is +// titled `#999` and mentions `#1234` in its note as a blocker; reusing it for the dispatch rewrote +// the #999 note, restarted it (waking Dev1 for #1234 on the wrong card) and left #1234 cardless. +// `title`/`target` decide first; the note is read only for a card that names no issue of its own, +// which is what keeps the `:161` case — its card names the issue in the title — resetting one card. +test('a dispatch does not reclaim a card that only mentions the issue in its note (#1331)', async (t) => { + const app = await start(t) + app.rt.state.tasks['task-999'] = parked('blocked', { + id: 'task-999', title: '#999 fix the other thing', note: 'blocked on #1234 first', + waitingFor: 'input', inputRequest: { id: 'q9', question: 'which file?' }, + }) + const before = { ...app.rt.state.tasks['task-999'] } + await app.relay([app.post('@Dev1 #1234 rebase and push')]).done + const other = app.rt.state.tasks['task-999'] + assert.equal(other.note, 'blocked on #1234 first', "a mention in the note is not the card's issue") + assert.equal(other.status, 'blocked', 'the #999 card is not restarted for #1234') + assert.equal(other.updatedAt, before.updatedAt, 'and nothing about it moved') + assert.equal(other.actors.length, before.actors.length, 'no status entry was appended for #1234') + const fresh = app.cards().find(task => task.id !== 'task-999') + assert.ok(fresh, 'the dispatch wrote a card that names #1234') + assert.equal(fresh.title, '#1234 rebase and push') + assert.equal(app.cards().length, 2, 'two issues, two cards') +}) + +// The other half of #1331's narrowing: a card whose own fields name no issue is still that +// issue's card when the note names it. Without this fallback the title check would strand the cards +// written before a dispatch put the id in the title, and one issue would grow a second card. +test('a card that names the issue only in its note is still that issue\'s card (#1331)', async (t) => { + const app = await start(t) + app.rt.state.tasks['task-note-only'] = parked('working', { + id: 'task-note-only', title: 'rebase and push', note: 'from #1234 please', + }) + await app.relay([app.post('@Dev1 #1234 then push the branch')]).done + assert.equal(app.cards().length, 1, 'the note-only card is reused, so no second card appears') + const card = app.rt.state.tasks['task-note-only'] + assert.equal(card.note, '#1234 then push the branch', 'the dispatch still rewrites the note it read') + assert.equal(card.status, 'working') + assert.ok(app.wakeOf('b'), 'the update moves the card, so the drive wakes the member') +}) + test('a stored card with no actors is updated, never half-written (#1254)', async (t) => { const app = await start(t) const { actors, ...bare } = parked('working') diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 92a7a67ed..65877d78b 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -206,7 +206,7 @@ A member's own line calls the members it names in two shapes where the call used ### A dispatch is a board assign (#1254) -An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and push` opened a group round for that member, so dispatch stayed chat and the board only recorded what chat had already said. Now the line is the assignment. `work-dispatch.js` `workDispatch(text, { callees })` answers only the shape — exactly one named member, an issue/PR id (`#1234` or a `github.com///(issues|pull)/` link), an imperative within 40 characters of that id (English verbs, or Chinese ones such as 修 / 改 / 推 / 补), and no `?` — and `groups.js` `dispatchOf` adds the room's rules: a Bot's own post, no `@all`, not quiet or an event, a room with a board, and the poster is `rt.taskLeadOf(room)` (the picked lead, else the admin). `rt.assignFromDispatch` in `tasks.js` then updates the open card that member already holds for that issue (matched on title, note or `target`) or opens one owned by it, still `open` — the host assigns, and the member claims on its own turn, where #1253's claim line is posted in its own voice. The save runs the drive, so the member is woken in its own chat (#1205). A card that is not driving — `result-ready` or `blocked` — is restarted through an edge the state machine already has (`result-ready → working`, `blocked → working`), because `task-drive` wakes only `open`/`claimed`/`working` and rewriting the note alone would leave the member unwoken; a card blocked on an input is reused rather than duplicated, so one issue stays one card, and a stored card with no `actors` list has one started instead of throwing after the note moved. The relay's `speakers` is empty for that opener and the round records no part or pass; the same dispatch posted while a round runs is written from `showInRound` and adds no `round.calls`, so the member is not called twice — but a card write that answers `undefined` is never a silent drop: the opener's `speakers` fall back to the ordinary ones and a mid-round dispatch falls back to `round.calls`, so the member is called in the round instead of not at all. A question, a remark with no imperative, two named members, `@all`, a poster that does not divide the board, and a face-to-face room (no board) keep the group turn they had. The line is still in the room log, and the card shows on the board. Tests: `core/test/work-dispatch.test.js`. +An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and push` opened a group round for that member, so dispatch stayed chat and the board only recorded what chat had already said. Now the line is the assignment. `work-dispatch.js` `workDispatch(text, { callees })` answers only the shape — exactly one named member, an issue/PR id (`#1234` or a `github.com///(issues|pull)/` link), an imperative within 40 characters of that id (English verbs, or Chinese ones such as 修 / 改 / 推 / 补), and no `?` — and `groups.js` `dispatchOf` adds the room's rules: a Bot's own post, no `@all`, not quiet or an event, a room with a board, and the poster is `rt.taskLeadOf(room)` (the picked lead, else the admin). `rt.assignFromDispatch` in `tasks.js` then updates the open card that member already holds for that issue (matched on `title` or `target`; a card whose own fields name no issue is matched on its `note`, so a note that merely mentions another issue no longer reclaims it — #1331) or opens one owned by it, still `open` — the host assigns, and the member claims on its own turn, where #1253's claim line is posted in its own voice. The save runs the drive, so the member is woken in its own chat (#1205). A card that is not driving — `result-ready` or `blocked` — is restarted through an edge the state machine already has (`result-ready → working`, `blocked → working`), because `task-drive` wakes only `open`/`claimed`/`working` and rewriting the note alone would leave the member unwoken; a card blocked on an input is reused rather than duplicated, so one issue stays one card, and a stored card with no `actors` list has one started instead of throwing after the note moved. The relay's `speakers` is empty for that opener and the round records no part or pass; the same dispatch posted while a round runs is written from `showInRound` and adds no `round.calls`, so the member is not called twice — but a card write that answers `undefined` is never a silent drop: the opener's `speakers` fall back to the ordinary ones and a mid-round dispatch falls back to `round.calls`, so the member is called in the round instead of not at all. A question, a remark with no imperative, two named members, `@all`, a poster that does not divide the board, and a face-to-face room (no board) keep the group turn they had. The line is still in the room log, and the card shows on the board. Tests: `core/test/work-dispatch.test.js`. ### Team state and access diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index 32fa5ee0f..d3034c769 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -18,7 +18,7 @@ import { randomBytes } from 'node:crypto' import { columnOf, compareForQueue, compareForWork, sameRole } from '../shared/task-board.js' import { currentParticipants, deriveParticipants } from './room-membership.js' -import { namesIssue } from './work-dispatch.js' +import { issueRefs, namesIssue } from './work-dispatch.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) // result-ready still waits for the requester: prune must not evict it with terminals (#111). @@ -500,6 +500,16 @@ export function install(rt) { } } + // #1331: a card belongs to the issue its own fields name. `title`/`target` name it; a note that + // merely mentions the id ("blocked on #1234 first") does not, or a dispatch for #1234 rewrites the + // #999 card and leaves #1234 with no card at all. The note is still read for a card that names no + // issue anywhere else, so #1254's "one issue is one card" holds for cards written from a note. + const cardNamesIssue = (task, wanted) => { + const own = `${task.title ?? ''} ${task.target?.href ?? ''}` + if (namesIssue(own, wanted)) return true + return issueRefs(own).length === 0 && namesIssue(task.note ?? '', wanted) + } + // #1254: the room's task lead naming one member with an issue id and an imperative is an assign, // not a round (`groups.js` `dispatchOf`). The card that member already holds for that issue is // updated; otherwise one is opened, owned by that member and still `open` — the host assigns, and @@ -519,7 +529,7 @@ export function install(rt) { && openTask(task)) const held = issue === undefined || issue === null || String(issue).trim() === '' ? undefined - : open.filter(task => namesIssue(`${task.title ?? ''} ${task.note ?? ''} ${task.target?.href ?? ''}`, issue)).sort(newestFirst)[0] + : open.filter(task => cardNamesIssue(task, issue)).sort(newestFirst)[0] if (held) { held.note = text // Both paths below append to `actors`, and `record()` pushes without a guard, so a card diff --git a/patches/ds-bot/files/test/work-dispatch.test.js b/patches/ds-bot/files/test/work-dispatch.test.js index b202e43e7..24c1683a0 100644 --- a/patches/ds-bot/files/test/work-dispatch.test.js +++ b/patches/ds-bot/files/test/work-dispatch.test.js @@ -268,6 +268,46 @@ test('a dispatch reuses a card blocked on an input and restarts it (#1254)', asy assert.ok(app.wakeOf('b')) }) +// #1331: a card is that issue's card only when its own fields name the issue. The card below is +// titled `#999` and mentions `#1234` in its note as a blocker; reusing it for the dispatch rewrote +// the #999 note, restarted it (waking Dev1 for #1234 on the wrong card) and left #1234 cardless. +// `title`/`target` decide first; the note is read only for a card that names no issue of its own, +// which is what keeps the `:161` case — its card names the issue in the title — resetting one card. +test('a dispatch does not reclaim a card that only mentions the issue in its note (#1331)', async (t) => { + const app = await start(t) + app.rt.state.tasks['task-999'] = parked('blocked', { + id: 'task-999', title: '#999 fix the other thing', note: 'blocked on #1234 first', + waitingFor: 'input', inputRequest: { id: 'q9', question: 'which file?' }, + }) + const before = { ...app.rt.state.tasks['task-999'] } + await app.relay([app.post('@Dev1 #1234 rebase and push')]).done + const other = app.rt.state.tasks['task-999'] + assert.equal(other.note, 'blocked on #1234 first', "a mention in the note is not the card's issue") + assert.equal(other.status, 'blocked', 'the #999 card is not restarted for #1234') + assert.equal(other.updatedAt, before.updatedAt, 'and nothing about it moved') + assert.equal(other.actors.length, before.actors.length, 'no status entry was appended for #1234') + const fresh = app.cards().find(task => task.id !== 'task-999') + assert.ok(fresh, 'the dispatch wrote a card that names #1234') + assert.equal(fresh.title, '#1234 rebase and push') + assert.equal(app.cards().length, 2, 'two issues, two cards') +}) + +// The other half of #1331's narrowing: a card whose own fields name no issue is still that +// issue's card when the note names it. Without this fallback the title check would strand the cards +// written before a dispatch put the id in the title, and one issue would grow a second card. +test('a card that names the issue only in its note is still that issue\'s card (#1331)', async (t) => { + const app = await start(t) + app.rt.state.tasks['task-note-only'] = parked('working', { + id: 'task-note-only', title: 'rebase and push', note: 'from #1234 please', + }) + await app.relay([app.post('@Dev1 #1234 then push the branch')]).done + assert.equal(app.cards().length, 1, 'the note-only card is reused, so no second card appears') + const card = app.rt.state.tasks['task-note-only'] + assert.equal(card.note, '#1234 then push the branch', 'the dispatch still rewrites the note it read') + assert.equal(card.status, 'working') + assert.ok(app.wakeOf('b'), 'the update moves the card, so the drive wakes the member') +}) + test('a stored card with no actors is updated, never half-written (#1254)', async (t) => { const app = await start(t) const { actors, ...bare } = parked('working')