diff --git a/changelog.d/2637.fixed.md b/changelog.d/2637.fixed.md new file mode 100644 index 000000000..d3ec3a0dd --- /dev/null +++ b/changelog.d/2637.fixed.md @@ -0,0 +1 @@ +The group board (`track_tasks`) now matches a Bot name and a group name the way every other door does — trimmed, case-insensitive, a leading `@` dropped from a Bot name and surrounding quotes from a group name — so `assignee: "dev"` and `group: "delivery group"` resolve the Bot and the room they name instead of being refused while the same state resolves them everywhere else. A name that matches no member or more than one still gets the same refusal ([#2637](https://github.com/ThinkFlowLab/FlyBot/issues/2637)). diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index e1dea3545..77e13fe54 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -21,7 +21,7 @@ import { retiredBot } from './store.js' import { currentParticipants, deriveParticipants } from './room-membership.js' import { cardNamesIssue } from '../shared/issue-ref.js' import { ownEntry } from './store.js' -import { codePoints } from './text.js' +import { codePoints, normalize, unquote } from './text.js' import { cut } from '../shared/text-boundary.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) @@ -763,7 +763,8 @@ export function install(rt) { } const roomByName = (botId, name) => { - const rooms = Object.values(state.rooms ?? {}).filter(room => Array.isArray(room.members) && room.members.includes(botId) && room.name === name) + const wanted = normalize(unquote(name)) + const rooms = Object.values(state.rooms ?? {}).filter(room => Array.isArray(room.members) && room.members.includes(botId) && normalize(room.name) === wanted) if (rooms.length !== 1) throw invalid(`No single group named "${name.slice(0, 80)}" that you are in`) return rooms[0].id } @@ -808,10 +809,11 @@ export function install(rt) { throw forbidden(`Only the ${picked ? "room's task lead" : 'group admin'} can ${action}`) } const memberBot = (room, name) => { - const wanted = String(name).trim() + const given = String(name).trim() + const wanted = normalize(given).replace(/^@/, '') const ids = liveParticipants(room).filter(person => person.kind === 'bot' && !person.remote).map(person => person.id) - const matches = [...new Set(ids)].map(id => botRecord(id)).filter(candidate => candidate && !retiredBot(candidate) && candidate.name === wanted) - if (matches.length !== 1) throw invalid(`No single Bot named "${wanted.slice(0, 80)}" in this group`) + const matches = [...new Set(ids)].map(id => botRecord(id)).filter(candidate => candidate && !retiredBot(candidate) && normalize(candidate.name) === wanted) + if (matches.length !== 1) throw invalid(`No single Bot named "${given.slice(0, 80)}" in this group`) return matches[0] } const isOverall = (task, roomId) => task?.kind === 'group' && task.origin?.roomId === roomId && openTask(task) diff --git a/core/test/track-tasks-name-normalize.test.js b/core/test/track-tasks-name-normalize.test.js new file mode 100644 index 000000000..99d696149 --- /dev/null +++ b/core/test/track-tasks-name-normalize.test.js @@ -0,0 +1,127 @@ +// #2637: the group board compared a Bot name and a group name raw, while every other door to +// the same objects normalises them (trim + lowercase, and `unquote` for a group). One name the +// rest of the host resolved was refused here with "No single Bot named …" / "No single group +// named …", so a lead recording a group plan lost the card. The refusal for a genuinely +// missing or ambiguous name stays as it was. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const bot = (id, name, role, createdAt) => ({ id, name, role, sessionId: id, createdAt, ownerId: 'owner-1', createdBy: 'a' }) +const person = (id, role) => ({ kind: 'bot', id, role, displayName: id, joinedSeq: 0, joinedAt: 1 }) + +// One room, "Delivery Group", whose two members are the Chief and a Bot named "Dev". `extra` +// lets a test seat another Bot, or one outside the room, to keep the refusal paths alive. +const team = (extra = {}) => { + const bots = { + a: bot('a', 'Chief', 'Lead', 1), + d1: bot('d1', 'Dev', 'Dev', 2), + ...(extra.bots ?? {}), + } + const members = ['a', 'd1', ...(extra.members ?? [])] + return { + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots, + rooms: { + 'room-1': { + id: 'room-1', name: 'Delivery Group', members, admin: 'a', mode: 'everyone', named: true, + sessions: {}, log: [], seq: 0, seen: {}, + participants: [ + { kind: 'human', id: 'owner-1', role: 'admin', displayName: 'Alice', joinedSeq: 0, joinedAt: 1 }, + ...members.filter(id => bots[id]).map(id => person(id, id === 'a' ? 'admin' : 'member')), + ], + }, + }, + grants: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + prefs: {}, + } +} + +async function start(t, state) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-name-normalize-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(state)) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', + followup() {}, steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + inbox: { nextStep: [], nextTurn: [], remove() {} }, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: 'made-1' }), + resolveAgent: async id => ({ agent: agentOf(id) }), + selectModel: async () => {}, + page: async () => ({ records: [], hasMore: false }), + }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home }) + await rt.load() + t.after(async () => { + try { await ctx.dispose() } catch {} + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + const track = (sessionId, args) => ctx.registeredTools.get('track_tasks').execute(args, { agent: { id: sessionId } }) + const idOf = (body) => { + const id = String(body).match(/task-[0-9a-f]+/)?.[0] + assert.ok(id, String(body)) + return id + } + const overall = async (group) => idOf(await track('a', { tasks: [{ group, title: 'Ship 18', overall: true }] })) + return { rt, ctx, track, idOf, overall } +} + +test('a Bot name resolves on the board however it is typed (#2637)', async (t) => { + const { rt, track, idOf, overall } = await start(t, team()) + const parent = await overall('Delivery Group') + + for (const typed of ['Dev', 'dev', ' DEV ']) { + const created = await track('a', { tasks: [{ group: 'Delivery Group', title: `part ${typed}`, parent, assignee: typed }] }) + assert.doesNotMatch(String(created), /No single Bot named/, `assignee ${JSON.stringify(typed)} resolved`) + const id = idOf(created) + assert.equal(rt.state.tasks[id].ownerBotId, 'd1', `assignee ${JSON.stringify(typed)} lands on the one Bot named "Dev"`) + } +}) + +test('a group name resolves on the board however it is typed (#2637)', async (t) => { + const { rt, track, idOf } = await start(t, team()) + const parent = idOf(await track('a', { tasks: [{ group: 'delivery group', title: 'Ship 19', overall: true }] })) + assert.equal(rt.state.tasks[parent].origin.roomId, 'room-1', 'the lowercase group name found "Delivery Group"') + + const created = await track('a', { tasks: [{ group: '"Delivery Group"', title: 'part quoted', parent, assignee: 'Dev' }] }) + assert.doesNotMatch(String(created), /No single group named/, 'a quoted group name resolved too') +}) + +test('a name that is not in the room still refuses (#2637)', async (t) => { + const { track, overall } = await start(t, team({ bots: { d2: bot('d2', 'Tester', 'QA', 3) }, members: [] })) + const parent = await overall('Delivery Group') + const refused = await track('a', { tasks: [{ group: 'Delivery Group', title: 'part', parent, assignee: 'Tester' }] }) + assert.match(String(refused), /No single Bot named "Tester" in this group/) +}) + +test('a name that matches two Bots still refuses (#2637)', async (t) => { + const { track, overall } = await start(t, team({ bots: { d3: bot('d3', 'DEV', 'Dev', 4) }, members: ['d3'] })) + const parent = await overall('Delivery Group') + const refused = await track('a', { tasks: [{ group: 'Delivery Group', title: 'part', parent, assignee: 'dev' }] }) + assert.match(String(refused), /No single Bot named "dev" in this group/) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 5cb041891..d0bc97b57 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -305,7 +305,7 @@ Each Bot's work sits on the same `state.tasks` records. A column is read from th `track_tasks` is the conversation path. The calling Bot creates or updates only its own tasks (`source: 'conversation'`, `requesterId` the Bot's owner, `origin.sessionId` the turn, `origin.roomId` the group when the turn is in one). The same title in the same group updates the open task instead of adding another. The prompt tells the Bot to record work the user or a group actually handed it, to continue the unfinished task the tool names next, and to pass `ask` when it cannot continue without an answer. -A group job is one overall task plus a subtask per Bot (#1099). The overall task is `kind: 'group'`, owned by the Bot that records it, and tagged with `origin.roomId`. A group keeps one open overall task; recording another updates that one. A subtask sets `parentId` to that task. One Bot divides the board (#1182): `room.taskLead` when that Bot is still a member, otherwise the group admin. A face-to-face room has no board, so it has no task lead. Another Main Bot does not plan. The user, a Main Bot, or the group's admin sets the lead with `update_group` (`task_lead`, `none` clears it) or `update-room` (`taskLead`); group settings shows **Make task lead** / **Use the admin** on a group that lives here. Assigning a subtask to another member claims it (`open` → `claimed`) so the assignee starts without a confirmation, and wakes that assignee in its own chat (#1205); the assignment does not open a group round. The assignee then sets the column to in-progress with `track_tasks`; a chat line does not assign, claim, or record progress. The group board shows that above the columns. The requester stays that Bot's owner, so the lead still cannot move the card through `task-update`. A member updates the card assigned to it, records its own room task, or links its own subtask to the overall task. Only the lead records the overall task or assigns a subtask to someone else. When every child has ended, the overall card completes through the same transition a person's move uses: `accepted` with `progress` 100 when each child ended accepted or cancelled, `failed` when any child ended failed, so a failed part is never hidden behind a green overall card; an overall card already blocked or waiting on a result is left to its owner (#1252). A child also ends when its Bot is deleted (`failed`) or when the owner rejects the send it was waiting on (`cancelled`); both doors settle the overall card the same way, so the card a person sees is closed rather than left working. +A group job is one overall task plus a subtask per Bot (#1099). The overall task is `kind: 'group'`, owned by the Bot that records it, and tagged with `origin.roomId`. A group keeps one open overall task; recording another updates that one. A subtask sets `parentId` to that task. One Bot divides the board (#1182): `room.taskLead` when that Bot is still a member, otherwise the group admin. A face-to-face room has no board, so it has no task lead. Another Main Bot does not plan. The user, a Main Bot, or the group's admin sets the lead with `update_group` (`task_lead`, `none` clears it) or `update-room` (`taskLead`); group settings shows **Make task lead** / **Use the admin** on a group that lives here. Assigning a subtask to another member claims it (`open` → `claimed`) so the assignee starts without a confirmation, and wakes that assignee in its own chat (#1205); the assignment does not open a group round. The assignee then sets the column to in-progress with `track_tasks`; a chat line does not assign, claim, or record progress. The group board shows that above the columns. The requester stays that Bot's owner, so the lead still cannot move the card through `task-update`. A member updates the card assigned to it, records its own room task, or links its own subtask to the overall task. Only the lead records the overall task or assigns a subtask to someone else. When every child has ended, the overall card completes through the same transition a person's move uses: `accepted` with `progress` 100 when each child ended accepted or cancelled, `failed` when any child ended failed, so a failed part is never hidden behind a green overall card; an overall card already blocked or waiting on a result is left to its owner (#1252). A child also ends when its Bot is deleted (`failed`) or when the owner rejects the send it was waiting on (`cancelled`); both doors settle the overall card the same way, so the card a person sees is closed rather than left working. The Bot name an `assignee` carries and the group name a `group` carries are compared the way `findBot` / `findRooms` compare them — trimmed and lowercased, a leading `@` dropped from the Bot name and surrounding quotes from the group name — so a name another door resolves is not refused here; the refusal stays for a name that matches none or two (#2637). A lead can put work in a pull queue instead of naming an assignee (#1247). `track_tasks { tasks: [{ queue: true, role: 'Dev', priority, title }] }` records one item for that role: `queueRole` names it, `queueBy` is the lead, and `ownerBotId` stays the enqueuer until a claim, so an unclaimed item wakes nobody — `task-drive` skips it. Only the room's lead enqueues, and an item whose role no live member of that room carries is refused (`no-member-with-role`) instead of sitting for ever for a role nobody has. `track_tasks { claim: true }` takes the highest-priority item of the caller's own role, oldest first on a tie (`compareForQueue`, not `compareForWork`, whose tiebreak is newest first), and `id` claims a named item; a member of another role is refused by name (`not-in-role`), never silently. One member holds one card in progress, so a member already holding one gets nothing — a blocked card waits on a person and a `result-ready` card waits on its requester, so neither counts (`occupiesSlot` in `src/shared/task-board.js` counts only `claimed` and `working`, #1744). The claim is compare-and-set: the item is read again after the pick yields, and only one still open is taken, so two claims leave exactly one owner. `track_tasks { release: { task, reason, retryInSec } }` returns a claimed item (`blocked → open` included) with `retryAt`, and it is not offered again before then. A claim sets `ownerBotId` and `claimedBy` to the member and keeps `queueRole`, so the card the board and room presence read is the same card: claiming is a board write, not a second row. Tests: `core/test/task-queue.test.js`. Reading the list orders unfinished work by priority (urgent, high, normal, low) and prefers an in-progress task when the priority is the same. `ask` walks the task to working and then to the existing input wait (`waitingFor: 'input'`); the reply tells the Bot to call `ask_user` for the user, and to post one short question in the group only when a person there has to decide. `answer` resumes that wait. The question is the same record the inbox already shows for a blocked task. diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index e1dea3545..77e13fe54 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -21,7 +21,7 @@ import { retiredBot } from './store.js' import { currentParticipants, deriveParticipants } from './room-membership.js' import { cardNamesIssue } from '../shared/issue-ref.js' import { ownEntry } from './store.js' -import { codePoints } from './text.js' +import { codePoints, normalize, unquote } from './text.js' import { cut } from '../shared/text-boundary.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) @@ -763,7 +763,8 @@ export function install(rt) { } const roomByName = (botId, name) => { - const rooms = Object.values(state.rooms ?? {}).filter(room => Array.isArray(room.members) && room.members.includes(botId) && room.name === name) + const wanted = normalize(unquote(name)) + const rooms = Object.values(state.rooms ?? {}).filter(room => Array.isArray(room.members) && room.members.includes(botId) && normalize(room.name) === wanted) if (rooms.length !== 1) throw invalid(`No single group named "${name.slice(0, 80)}" that you are in`) return rooms[0].id } @@ -808,10 +809,11 @@ export function install(rt) { throw forbidden(`Only the ${picked ? "room's task lead" : 'group admin'} can ${action}`) } const memberBot = (room, name) => { - const wanted = String(name).trim() + const given = String(name).trim() + const wanted = normalize(given).replace(/^@/, '') const ids = liveParticipants(room).filter(person => person.kind === 'bot' && !person.remote).map(person => person.id) - const matches = [...new Set(ids)].map(id => botRecord(id)).filter(candidate => candidate && !retiredBot(candidate) && candidate.name === wanted) - if (matches.length !== 1) throw invalid(`No single Bot named "${wanted.slice(0, 80)}" in this group`) + const matches = [...new Set(ids)].map(id => botRecord(id)).filter(candidate => candidate && !retiredBot(candidate) && normalize(candidate.name) === wanted) + if (matches.length !== 1) throw invalid(`No single Bot named "${given.slice(0, 80)}" in this group`) return matches[0] } const isOverall = (task, roomId) => task?.kind === 'group' && task.origin?.roomId === roomId && openTask(task) diff --git a/patches/ds-bot/files/test/track-tasks-name-normalize.test.js b/patches/ds-bot/files/test/track-tasks-name-normalize.test.js new file mode 100644 index 000000000..99d696149 --- /dev/null +++ b/patches/ds-bot/files/test/track-tasks-name-normalize.test.js @@ -0,0 +1,127 @@ +// #2637: the group board compared a Bot name and a group name raw, while every other door to +// the same objects normalises them (trim + lowercase, and `unquote` for a group). One name the +// rest of the host resolved was refused here with "No single Bot named …" / "No single group +// named …", so a lead recording a group plan lost the card. The refusal for a genuinely +// missing or ambiguous name stays as it was. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createRuntime } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const bot = (id, name, role, createdAt) => ({ id, name, role, sessionId: id, createdAt, ownerId: 'owner-1', createdBy: 'a' }) +const person = (id, role) => ({ kind: 'bot', id, role, displayName: id, joinedSeq: 0, joinedAt: 1 }) + +// One room, "Delivery Group", whose two members are the Chief and a Bot named "Dev". `extra` +// lets a test seat another Bot, or one outside the room, to keep the refusal paths alive. +const team = (extra = {}) => { + const bots = { + a: bot('a', 'Chief', 'Lead', 1), + d1: bot('d1', 'Dev', 'Dev', 2), + ...(extra.bots ?? {}), + } + const members = ['a', 'd1', ...(extra.members ?? [])] + return { + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots, + rooms: { + 'room-1': { + id: 'room-1', name: 'Delivery Group', members, admin: 'a', mode: 'everyone', named: true, + sessions: {}, log: [], seq: 0, seen: {}, + participants: [ + { kind: 'human', id: 'owner-1', role: 'admin', displayName: 'Alice', joinedSeq: 0, joinedAt: 1 }, + ...members.filter(id => bots[id]).map(id => person(id, id === 'a' ? 'admin' : 'member')), + ], + }, + }, + grants: {}, + agents: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, + prefs: {}, + } +} + +async function start(t, state) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-name-normalize-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(state)) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', + followup() {}, steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + inbox: { nextStep: [], nextTurn: [], remove() {} }, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: 'made-1' }), + resolveAgent: async id => ({ agent: agentOf(id) }), + selectModel: async () => {}, + page: async () => ({ records: [], hasMore: false }), + }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = createRuntime(ctx, { home }) + await rt.load() + t.after(async () => { + try { await ctx.dispose() } catch {} + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + const track = (sessionId, args) => ctx.registeredTools.get('track_tasks').execute(args, { agent: { id: sessionId } }) + const idOf = (body) => { + const id = String(body).match(/task-[0-9a-f]+/)?.[0] + assert.ok(id, String(body)) + return id + } + const overall = async (group) => idOf(await track('a', { tasks: [{ group, title: 'Ship 18', overall: true }] })) + return { rt, ctx, track, idOf, overall } +} + +test('a Bot name resolves on the board however it is typed (#2637)', async (t) => { + const { rt, track, idOf, overall } = await start(t, team()) + const parent = await overall('Delivery Group') + + for (const typed of ['Dev', 'dev', ' DEV ']) { + const created = await track('a', { tasks: [{ group: 'Delivery Group', title: `part ${typed}`, parent, assignee: typed }] }) + assert.doesNotMatch(String(created), /No single Bot named/, `assignee ${JSON.stringify(typed)} resolved`) + const id = idOf(created) + assert.equal(rt.state.tasks[id].ownerBotId, 'd1', `assignee ${JSON.stringify(typed)} lands on the one Bot named "Dev"`) + } +}) + +test('a group name resolves on the board however it is typed (#2637)', async (t) => { + const { rt, track, idOf } = await start(t, team()) + const parent = idOf(await track('a', { tasks: [{ group: 'delivery group', title: 'Ship 19', overall: true }] })) + assert.equal(rt.state.tasks[parent].origin.roomId, 'room-1', 'the lowercase group name found "Delivery Group"') + + const created = await track('a', { tasks: [{ group: '"Delivery Group"', title: 'part quoted', parent, assignee: 'Dev' }] }) + assert.doesNotMatch(String(created), /No single group named/, 'a quoted group name resolved too') +}) + +test('a name that is not in the room still refuses (#2637)', async (t) => { + const { track, overall } = await start(t, team({ bots: { d2: bot('d2', 'Tester', 'QA', 3) }, members: [] })) + const parent = await overall('Delivery Group') + const refused = await track('a', { tasks: [{ group: 'Delivery Group', title: 'part', parent, assignee: 'Tester' }] }) + assert.match(String(refused), /No single Bot named "Tester" in this group/) +}) + +test('a name that matches two Bots still refuses (#2637)', async (t) => { + const { track, overall } = await start(t, team({ bots: { d3: bot('d3', 'DEV', 'Dev', 4) }, members: ['d3'] })) + const parent = await overall('Delivery Group') + const refused = await track('a', { tasks: [{ group: 'Delivery Group', title: 'part', parent, assignee: 'dev' }] }) + assert.match(String(refused), /No single Bot named "dev" in this group/) +})