diff --git a/core/src/host/task-drive.js b/core/src/host/task-drive.js index 5bcb4a5de..b40bc8555 100644 --- a/core/src/host/task-drive.js +++ b/core/src/host/task-drive.js @@ -302,18 +302,26 @@ export function install(rt) { if (!task?.queueRole || task.claimedBy || task.status !== 'open') continue const roomId = task.origin?.roomId const room = roomId ? rt.roomOf(roomId) : undefined - if (!room || room.origin === 'face-to-face') continue + if (roomId && (!room || room.origin === 'face-to-face')) continue if (Number.isFinite(task.retryAt) && task.retryAt > clock.now()) continue - const key = `${roomId}\u0000${String(task.queueRole).toLowerCase()}` + // A handoff card (#1775) waits in its role's pool, room or not: the key is the role, so a + // card handed from one role to the next is offered in the same pass with no dispatch. + const key = `${roomId ?? ''}\u0000${String(task.queueRole).toLowerCase()}` const list = byRoomRole.get(key) ?? [] list.push(task) byRoomRole.set(key, list) } for (const list of byRoomRole.values()) { const item = list.slice().sort(compareForQueue)[0] - const room = rt.roomOf(item.origin.roomId) - const carriers = (room?.members ?? []) - .filter(id => { const bot = rt.botOf(id); return Boolean(bot) && bot.remote === undefined && sameRole(bot.role, item.queueRole) }) + const room = item.origin?.roomId ? rt.roomOf(item.origin.roomId) : undefined + const carriers = room + ? (room.members ?? []) + .filter(id => { const bot = rt.botOf(id); return Boolean(bot) && bot.remote === undefined && sameRole(bot.role, item.queueRole) }) + // No room: the pool is every local Bot carrying the role, so a handoff chain runs between + // members of the team that never share a group. + : Object.values(rt.state.bots ?? {}) + .filter(bot => Boolean(bot) && bot.remote === undefined && bot.role && !retiredBot(bot) && sameRole(bot.role, item.queueRole)) + .map(bot => bot.id) for (const botId of carriers) { const sessionId = rt.chatOf(botId) if (!sessionId || rt.roomOf(sessionId)) continue diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index 749180e37..2459d183b 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -485,11 +485,23 @@ export function install(rt) { if (taskClass !== undefined) task.taskClass = taskClass if (reworkOf !== undefined) task.reworkOf = reworkOf if (handoff !== undefined) task.handoff = handoff + if (handoff !== undefined) routeToPool(task, handoff) if (evidence !== undefined) task.evidence = evidence if (clearingLease) delete task.lease if (lease !== undefined) task.lease = lease return { taskClass, reworkOf, handoff, evidence, lease } } + // The route on the card (#1775): `handoff.to` names the role that owns the next step, so it is + // the card's pool. Recording the handoff starts the card in that pool; recording it on a card a + // member is holding hands the card on in the same write, so the producer lets go of it and the + // target role can take it with no dispatch from a supervisor. + function routeToPool(task, handoff) { + const held = task.claimedBy !== undefined || task.status === 'claimed' || task.status === 'working' || task.status === 'blocked' + task.queueRole = handoff.to + if (held) requeue(task, `handoff to ${handoff.to}`, 0, 'handed on') + else if (task.queueBy === undefined) task.queueBy = task.ownerBotId + return task.queueRole + } // The typed contract on a card that is created here rather than written by a caller. const defaultContract = (task) => { task.taskClass = 'work' @@ -642,15 +654,20 @@ export function install(rt) { // roles, and the rework is one child card linked by `reworkOf` — never `parentId`, which // means "part of an overall card" and must never make `settleOverall` fire. A replay of the // same rejection reuses the child that is still open instead of creating a second one. - function openRework(original, note) { + function openRework(original, note, by = 'requester') { const reason = boundedLine(note, HANDOFF_REASON_MAX, 'handoff.reason') - const to = oneLine(botRecord(original.ownerBotId)?.role ?? '', ROLE_MAX) || 'requester' - const from = 'requester' + // A worker-side return names the role that produced the card (#1775); the requester's own + // send-back stays the owner's role, exactly as before. + const to = by === 'requester' + ? oneLine(botRecord(original.ownerBotId)?.role ?? '', ROLE_MAX) || 'requester' + : oneLine(original.handoff?.from ?? '', ROLE_MAX) + const from = by const at = stamp() original.handoff = { from, to, at, reason } const existing = Object.values(store()).find(task => task.reworkOf === original.id && openTask(task)) if (existing !== undefined) { existing.handoff = { from, to, at, reason } + if (by !== 'requester') existing.queueRole = to existing.actors = [...(existing.actors ?? []), { status: existing.status, at, by: 'rework replayed' }] existing.updatedAt = at return existing @@ -668,6 +685,7 @@ export function install(rt) { note: reason, source: 'rework', handoff: { from, to, at, reason }, + ...(by === 'requester' ? {} : { queueRole: to, queueBy: original.queueBy ?? original.ownerBotId }), origin: { ...(original.origin ?? {}) }, actors: [{ status: 'open', at, by: 'rework opened' }], } @@ -775,7 +793,7 @@ export function install(rt) { .filter(bot => Boolean(bot) && sameRole(bot.role, role)) // blocked → open is the queue's own edge: a member that cannot finish returns the item, and // no other path sends a card back to the queue, so the edge is not on TRANSITIONS. - function requeue(task, reason, retryInSec) { + function requeue(task, reason, retryInSec, by = 'returned to the queue') { dropWorker(task.worker) delete task.worker delete task.waitingFor @@ -789,7 +807,7 @@ export function install(rt) { task.progress = 0 task.retryAt = Date.now() + retryInSec * 1000 task.queueReason = reason - record(task, null, { by: 'returned to the queue', retryInSec }) + record(task, null, { by, retryInSec }) } const resolveParent = (roomId, ref, batch) => { const text = String(ref).trim() @@ -922,7 +940,9 @@ export function install(rt) { async function claimQueued(botId, body, roomId) { const room = resolveRoom(botId, body.group, roomId) const entry = room ? state.rooms?.[room] : undefined - if (!entry) throw invalid('A claim needs a group') + // A handoff card waits in its role's pool, not in a room (#1775): a member of the role takes it + // from its own chat, so a chain handed from one role to the next needs no group and no dispatch. + if (!entry) return claimHandoff(botId, body) if (expireLeases() > 0) await rt.save() const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) if (myRole === '') throw refuse('not-in-role', 'this Bot has no role to claim with') @@ -972,6 +992,66 @@ export function install(rt) { return { item: view(live) } } + // The role's own pool (#1775): a card whose `handoff.to` names this member's role is offered to + // it and to nobody else, and the same compare-and-set gives one member one card. A card with no + // room is not offered to a supervisor: the target role takes it, and an idle member wakes for it. + async function claimHandoff(botId, body) { + if (expireLeases() > 0) await rt.save() + const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) + if (myRole === '') throw refuse('not-in-role', 'this Bot has no role to claim with') + const holding = Object.values(store()).filter(task => task.ownerBotId === botId && occupiesSlot(task)) + if (holding.length > 0) return { item: null, reason: 'wip', held: holding.sort(compareForWork)[0].id } + const now = Date.now() + const inPool = (task) => { + if (task?.status !== 'open' || task.claimedBy !== undefined) return false + if (!sameRole(task.handoff?.to, myRole)) return false + return !(Number.isFinite(task.retryAt) && task.retryAt > now) + } + const wantedId = body.id !== undefined && body.id !== null && String(body.id) !== '' ? String(body.id) : undefined + let pick + if (wantedId !== undefined) { + pick = find(wantedId) + if (pick === undefined) throw stale(`no task ${wantedId}`) + if (!inPool(pick)) throw stale(`no card for ${myRole} in the handoff pool`) + } else { + pick = Object.values(store()).filter(inPool).sort(compareForQueue)[0] + } + if (pick === undefined) return { item: null, reason: 'empty' } + await rt.load() + const live = find(pick.id) + if (live === undefined || live.status !== 'open' || live.claimedBy !== undefined) return { item: null, reason: 'taken' } + live.claimedBy = botId + live.ownerBotId = botId + delete live.retryAt + delete live.queueReason + const claimedAt = Date.now() + live.lease = { holder: botId, takenAt: claimedAt, expiresAt: claimedAt + LEASE_TTL_SEC * 1000 } + live.worker = newWorker() + transition(live, 'claimed', null, { by: 'claimed from the handoff pool' }) + announceClaim(live) + await save() + return { item: view(live) } + } + + // The target role's admission comes back as a rework card (#1775): the member holding the card + // returns it to the role that produced it (`handoff.from`), and the producer takes it from its + // own pool. The child is linked by `reworkOf`, never `parentId`, and the original stays `working` + // until the rework is accepted (`closeRework`). + async function returnToProducer(botId, spec) { + const task = find(spec?.task ?? spec?.id) + if (task === undefined) throw stale(`no task ${String(spec?.task ?? spec?.id ?? '')}`) + if (task.claimedBy !== botId) throw forbidden('Only the member that claimed this card may return it to its producer') + const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) + if (!sameRole(task.handoff?.to, myRole)) throw refuse('not-in-role', `card ${task.id} is in the ${task.handoff?.to ?? 'unknown'} pool, not ${myRole}`) + const reason = readOptionalLine(spec?.reason, 'A reason') + if (reason === undefined) throw invalid('Returning a card to its producer needs a one-line reason') + const producer = oneLine(task.handoff?.from ?? '', ROLE_MAX) + if (producer === '' || producer === 'requester') throw refuse('no-producer', `card ${task.id} carries no producer role to return it to`) + const child = openRework(task, reason, myRole) + await save() + return view(child) + } + // A member that cannot finish returns the item with one line of why; it is not offered // again before `retryAt` (#1247). async function releaseQueued(botId, spec) { @@ -1002,6 +1082,7 @@ export function install(rt) { // Both come before the read path: a claim arrives with `tasks` omitted, so reading first // would return the list instead of the item (#1247). if (body?.release !== undefined) return releaseQueued(botId, body.release) + if (body?.rework !== undefined) return returnToProducer(botId, body.rework) if (body?.claim === true) return claimQueued(botId, body, origin.roomId) const items = body?.tasks if (items === undefined) return listed(botId) diff --git a/core/src/host/tools.js b/core/src/host/tools.js index 4713acd52..252e2dda2 100644 --- a/core/src/host/tools.js +++ b/core/src/host/tools.js @@ -912,6 +912,15 @@ ${task.writeBack.body}`) }, additionalProperties: false, }, + rework: { + type: 'object', + description: 'Return a card you claimed to the role that produced it, with a one-line reason. This is the target role\'s own refusal: a rework card linked to the returned one opens in the producer\'s pool, and the producer takes it without a dispatch.', + properties: { + task: { type: 'string', required: true, description: 'The card you claimed and are returning.' }, + reason: { type: 'string', required: true, description: 'One line on what is missing.' }, + }, + additionalProperties: false, + }, tasks: { type: 'array', description: 'Tasks to create or update. Omit to read your list.', @@ -1004,6 +1013,10 @@ ${task.writeBack.body}`) const why = result?.reason === 'wip' ? ` You already hold an in-progress card (${result.held}).` : '' return `No queued item for ${role === '' ? 'your role' : role} right now.${why}` } + if (args?.rework !== undefined) { + const child = await rt.trackBotTasks(self, args ?? {}, { sessionId: exec.agent?.id, roomId }) + return `Rework ${child.id} on ${child.reworkOf} for the ${child.handoff?.to ?? 'producer'} pool: ${String(child.note ?? '').slice(0, 200)}\nThe producer takes it from its own pool. Set that card's column to in-progress when you pick it up. Do not post a status in the group.` + } if (args?.release !== undefined) { const returned = await rt.trackBotTasks(self, args ?? {}, { sessionId: exec.agent?.id, roomId }) const seconds = Math.max(0, Math.round(((returned.retryAt ?? 0) - Date.now()) / 1000)) diff --git a/core/test/task-handoff-route.test.js b/core/test/task-handoff-route.test.js new file mode 100644 index 000000000..dff98491d --- /dev/null +++ b/core/test/task-handoff-route.test.js @@ -0,0 +1,178 @@ +// The route is the card (#1775): `handoff.to` names the role that owns the next step, so the card +// waits in that role's pool and an idle member of the role wakes for it. Nothing here is a +// dispatch: the producer records the handoff, the target role takes the card, and a card with no +// room is still offered, because the pool is the role and not the group. +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 team = () => ({ + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots: { + a: { id: 'a', name: 'Chief', role: 'Lead', sessionId: 'a', createdAt: 1, ownerId: 'owner-1' }, + d1: { id: 'd1', name: 'Dev1', role: 'Dev', sessionId: 'd1', createdAt: 2, ownerId: 'owner-1', createdBy: 'a' }, + r1: { id: 'r1', name: 'Rev1', role: 'Reviewer', sessionId: 'r1', createdAt: 3, ownerId: 'owner-1', createdBy: 'a' }, + }, + rooms: {}, + grants: {}, + agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, +}) + +async function start(t) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-handoff-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + const inbox = { + nextStep: [], + nextTurn: [], + remove(id) { + const keep = message => message.id !== id + const before = this.nextTurn.length + this.nextStep.length + this.nextTurn = this.nextTurn.filter(keep) + this.nextStep = this.nextStep.filter(keep) + return this.nextTurn.length + this.nextStep.length !== before + }, + } + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', inbox, + followup(message) { this.calls.push(message); this.inbox.nextTurn.push(message) }, + steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async () => ({ id: 'w1' }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: 'made' }), + 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 () => { + await ctx.dispose() + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + const track = (sessionId, args) => ctx.registeredTools.get('track_tasks').execute(args ?? {}, { agent: { id: sessionId } }) + const calls = (sessionId) => agents.get(sessionId)?.calls ?? [] + const text = (sessionId) => calls(sessionId).map(message => message.content.map(block => block.text ?? '').join('\n')).join('\n') + const drive = async () => { await rt.considerTaskDrive(); await ctx.taskDrive.settled() } + const idOf = (body) => { + const id = String(body).match(/task-[0-9a-f]+/)?.[0] + assert.ok(id, body) + return id + } + return { ctx, rt, track, calls, text, drive, idOf, home } +} + +test('a card handed to a role wakes that role and one member claims it, with no dispatch', async (t) => { + const app = await start(t) + // The Chief writes the opening handoff once: the card is in the Dev pool from that write. + const created = await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev', reason: 'first slice' } }] }) + const id = app.idOf(created) + assert.equal(app.rt.state.tasks[id].queueRole, 'Dev') + assert.equal(app.rt.state.tasks[id].handoff.to, 'Dev') + + await app.drive() + assert.match(app.text('d1'), new RegExp(`Take ${id} `), 'the Dev is offered the card by the route alone') + assert.equal(app.text('r1'), '', 'another role is not woken') + + assert.match(await app.track('d1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) + assert.equal(app.rt.state.tasks[id].claimedBy, 'd1') + assert.equal(app.rt.state.tasks[id].status, 'claimed') +}) + +test('a producer hands on its own card and the target role takes it; from and evidence survive', async (t) => { + const app = await start(t) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + + // The producer's own write is the whole handover: it names the role, records what proves the work. + const handed = await app.track('d1', { + tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' }, evidence: [{ sha: 'abcdef1234', check: 'node --test' }] }], + }) + assert.ok(handed) + const card = app.rt.state.tasks[id] + assert.equal(card.status, 'open', 'the producer let go of the card in the same write') + assert.equal(card.claimedBy, undefined) + assert.equal(card.queueRole, 'Reviewer') + assert.equal(card.handoff.from, 'Dev') + assert.equal(card.evidence.length, 1) + assert.equal(app.calls('a').length, 0, 'the Chief ran nothing while the card moved on') + + await app.drive() + assert.match(app.text('r1'), new RegExp(`Take ${id} `)) + assert.match(await app.track('r1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) + assert.equal(app.rt.state.tasks[id].claimedBy, 'r1') + assert.equal(app.rt.state.tasks[id].handoff.from, 'Dev', 'the card still says who produced it') + assert.deepEqual(app.rt.state.tasks[id].evidence.map(entry => entry.sha), ['abcdef1234']) + assert.equal(app.calls('a').length, 0, 'the Chief ran nothing') +}) + +test('with no idle member of the role the card waits in the pool and is taken once one appears', async (t) => { + const app = await start(t) + // The Reviewer already holds a card, so it cannot take another. + const busy = app.idOf(await app.track('r1', { tasks: [{ title: 'Review the last diff', column: 'in-progress' }] })) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + await app.track('d1', { tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' } }] }) + + await app.drive() + const waiting = app.rt.state.tasks[id] + assert.equal(waiting.status, 'open', 'the card is not lost when nobody can take it') + assert.equal(waiting.claimedBy, undefined) + assert.equal(waiting.queueRole, 'Reviewer', 'it stays in the Reviewer pool') + assert.doesNotMatch(app.text('r1'), new RegExp(`Take ${id} `)) + + // The Reviewer delivers the card it held; the pool is offered again on the next pass. + await app.rt.taskEndpoint('task-return', { id: busy, summary: 'the last diff is fine' }) + await app.drive() + assert.match(app.text('r1'), new RegExp(`Take ${id} `)) + assert.match(await app.track('r1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) +}) + +test('a role returns the card to its producer as a rework card the producer can take', async (t) => { + const app = await start(t) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + await app.track('d1', { tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' }, evidence: [{ sha: 'abcdef1234', check: 'node --test' }] }] }) + await app.drive() + await app.track('r1', { claim: true, id }) + await app.track('r1', { tasks: [{ id, column: 'in-progress' }] }) + + const returned = await app.track('r1', { rework: { task: id, reason: 'no test for the return path' } }) + const childId = app.idOf(returned) + const child = app.rt.state.tasks[childId] + assert.equal(child.taskClass, 'rework') + assert.equal(child.reworkOf, id, 'the rework points back at the card it fixes') + assert.equal(child.handoff.to, 'Dev', 'and at the role that produced it') + assert.equal(child.handoff.from, 'Reviewer') + assert.equal(child.queueRole, 'Dev') + assert.equal(app.rt.state.tasks[id].status, 'working', 'the original waits for the rework to be accepted') + + await app.drive() + assert.doesNotMatch(app.text('d1').split('Continue ').at(-1) ?? '', /never/, 'sanity') + const wake = app.calls('d1').map(message => message.content.map(block => block.text ?? '').join('\n')).find(line => line.includes(`Take ${childId} `)) + assert.ok(wake, `the producer is offered the rework card, got:\n${app.text('d1')}`) + assert.match(await app.track('d1', { claim: true, id: childId }), new RegExp(`^Claimed ${childId} `)) + assert.equal(app.rt.state.tasks[childId].claimedBy, 'd1') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 6f085524e..2586d5bbf 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -553,6 +553,15 @@ DSH's approval outcomes are one-shot (`ApprovalOutcome`: `allowed-once` / `rejec A discarded `.catch(() => {})` in the host sources either starts with `void` (fire and forget) or carries a one-line reason comment (an awaited wait); a `.catch(() => undefined)` whose value is used stays. `check-empty-catch` enforces it (see `docs/code-standards.md`). +## Handoff routing (#1775) + +A card carries its own route. `handoff.to` names the role that owns the next step, and that role is the card's pool: `writeContract` calls `routeToPool` (`src/host/tasks.js`) on every handoff it records, so `queueRole` becomes `handoff.to` in the same write. A card created with a handoff starts in that pool (the Chief's one dispatch), and a handoff recorded on a card a member is holding (`claimed`, `working` or `blocked`) hands the card on through `requeue` in the same write — the producer lets go of it, `claimedBy` and the lease are cleared, and the card is `open` in the target pool. Recording a handoff is therefore the whole handover: no supervisor step moves the card afterwards. + +`task-drive`'s queue pass offers the pool to one idle member of the role. A card with a room is offered to that room's members of the role, as the room queue always was; a card with no room is offered to every local Bot carrying the role, so a chain between roles needs no shared group and no graph. The offer is the existing `Take …` wake, `inflight` and the per-Session hold are unchanged, and a role with no idle member (`holdsCard`, `busy`, or a wake already waiting) leaves the card in the pool: the next pass offers it again, so a card is never lost for want of a worker. + +A claim works the same way (#1247's compare-and-set and WIP = 1, unchanged): `track_tasks { claim: true }` without a group takes the highest-priority card of the caller's role whose `handoff.to` matches, and `{ claim: true, id }` takes a named one; a card in another role's pool is refused by name. The target role's own admission is either the claim (accept) or `track_tasks { rework: { task, reason } }` (return): only the member that claimed the card may return it, and the write opens one child card with `taskClass: 'rework'`, `reworkOf` naming the card it fixes and `handoff.to` the producing role — the same `openRework` `task-send-back` uses, now also reachable from the role that received the card. The child is queued for the producer's role, so the producer takes it from its own pool, and the original stays `working` until the rework child is accepted (`closeRework`). A return with no producer role (`handoff.from` missing or `requester`) is refused rather than handed nowhere. Tests: `core/test/task-handoff-route.test.js`. + + ## Tests `npm test` in `core/`; smoke test for `index.js` / `src/host/` changes. diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 3a3e55ac1..ccf2e3e59 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -1409,7 +1409,7 @@ diff --git a/src/host/store.js b/src/host/store.js diff --git a/src/host/tools.js b/src/host/tools.js --- a/src/host/tools.js +++ b/src/host/tools.js -@@ -1,9 +1,47 @@ +@@ -1,10 +1,48 @@ // Team tools: the roster, bot-to-bot messages, group chat settings and posts, and the // question card. Group rounds and reading chats are in groups.js. import { randomUUID } from 'node:crypto' @@ -1425,7 +1425,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js +import { OTHER, settled } from './turn-origin.js' +import { cleanName } from './delivery.js' +import { retiredBot } from './store.js' -+ + +// A run Bot (ephemeral) is not a team member: every tool below has an effect that reaches past +// the run Bot's own Session — it messages, stops or edits another Bot, edits a group, writes or +// attaches a capability, asks the owner for a secret, arms or changes a schedule, reads the team @@ -1456,10 +1456,11 @@ diff --git a/src/host/tools.js b/src/host/tools.js +// One sentence that names the reason and no substitute route: the run Bot cannot become a team +// member by finding another tool that does the same thing. +export const EPHEMERAL_REFUSAL = 'A running Bot is not a team member.' - ++ export const output = { schema: { type: 'string' }, render: (_args, value) => [{ type: 'text', text: String(value) }] } export const GROUP_REF = { type: 'string', required: true, description: 'Exact group chat name from list_bots' } -@@ -18,6 +18,21 @@ const WORK_DIR_PARAM = { type: 'string', description: 'Full path of the folder t + const BLOCKING_QUESTION_TOOL = 'ask_user_question' +@@ -18,6 +56,21 @@ export function install(rt) { const { ctx, state } = rt @@ -1481,7 +1482,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js const callerBot = (exec) => rt.botOf(rt.selfOf(exec.agent?.id)) const callerRoom = (exec) => { const roomId = rt.groupOwners.get(exec.agent?.id)?.roomId -@@ -27,39 +27,112 @@ export function install(rt) { +@@ -27,39 +80,112 @@ const roomLine = (room, self) => { const members = room.members.map(rt.botOf).filter(Boolean) .map(bot => `${bot.name}${bot.id === room.admin ? ' (admin)' : ''}${bot.id === self ? ' (you)' : ''}`) @@ -1601,7 +1602,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js ].join('\n') }, })) -@@ -81,12 +81,18 @@ export function install(rt) { +@@ -81,12 +207,18 @@ // A tool that throws shows as failed in the chat, instead of as sent or stopped. const refuse = (message) => { throw new Error(message) } @@ -1621,7 +1622,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js message: { type: 'string', required: true, description: 'Self-contained message; the other Bot cannot see this conversation' }, reply_to: { type: 'string', -@@ -104,8 +104,18 @@ export function install(rt) { +@@ -104,8 +236,18 @@ await rt.load() const sender = callerBot(exec) if (sender === undefined) return 'Only Bots on the team can use message_bot.' @@ -1642,7 +1643,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (target.id === sender.id) return 'You cannot message yourself.' const turn = rt.live.get(exec.agent?.id) const hop = Math.max(turn?.info?.hop ?? 0, ...(turn?.joined ?? []).map(joined => joined.info?.hop ?? 0)) -@@ -118,7 +118,11 @@ export function install(rt) { +@@ -118,7 +260,11 @@ const toUser = args.reply_to === 'user' const working = rt.live.get(chat)?.turn !== undefined const steer = args.when !== 'queue' && !toUser @@ -1655,7 +1656,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js const sent = toUser ? `Delivered to ${target.name}. It will answer the user in its own chat; nothing comes back to you, so point the user to ${target.name}'s chat instead of relaying.` : working && steer -@@ -152,8 +152,8 @@ export function install(rt) { +@@ -152,8 +298,8 @@ await rt.load() const sender = callerBot(exec) if (sender === undefined || !rt.isMain(sender.id)) refuse('Only a Main Bot can stop other Bots.') @@ -1666,7 +1667,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (target.id === sender.id) refuse('You cannot stop yourself.') const { running } = activeSessions(target.id) if (running.length === 0) refuse(`${target.name} is not running right now, so there is nothing to stop. Use message_bot to reach it.`) -@@ -162,21 +162,32 @@ export function install(rt) { +@@ -162,21 +308,32 @@ agent.cancel({ kind: 'hook', reason: 'ds-bot/stopped' }) } const why = String(args.reason ?? '').trim() || 'Stop what you are doing; the previous turn was ended on purpose. Acknowledge briefly.' @@ -1702,7 +1703,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js }, output, async execute(args, exec) { -@@ -186,8 +186,8 @@ export function install(rt) { +@@ -186,8 +343,8 @@ const workDir = args.work_dir ? await rt.cleanWorkDir(args.work_dir) : undefined const name = String(args.name ?? '').trim() await rt.approveFolder(exec, { path: workDir, who: { en: `the new Bot ${name}`, zh: `新 Bot ${name} ` } }) @@ -1713,7 +1714,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js } catch (error) { return `Could not create the Bot: ${error.message}` } -@@ -212,7 +212,7 @@ export function install(rt) { +@@ -212,7 +369,7 @@ const self = rt.selfOf(exec.agent?.id) const caller = rt.botOf(self) if (caller === undefined) return 'Only Bots on the team can use update_bot.' @@ -1722,11 +1723,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (bot === undefined) return `No Bot named ${args.bot}.` if (bot.id !== caller.id && !rt.isMain(self)) return 'Only a Main Bot can change another Bot. Leave out bot to change yourself, or ask a Main Bot with message_bot.' try { -@@ -230,6 +230,34 @@ export function install(rt) { - }, +@@ -231,6 +388,34 @@ })) -+ ctx.tools.register(defineTool({ + ctx.tools.register(defineTool({ + name: 'archive_bot', + description: 'Main Bot only. Retire another Bot: it leaves the tree (freeing a maxChildren slot) and the default Bot list, while its chats, history and memory stay readable. It is never deleted and no parent moves; deleting a Bot stays the owner\'s own click in the UI.', + parameters: { @@ -1754,10 +1754,11 @@ diff --git a/src/host/tools.js b/src/host/tools.js + }, + })) + - ctx.tools.register(defineTool({ ++ ctx.tools.register(defineTool({ name: 'list_models', description: 'List the models a Bot can run on, as provider/model refs for create_bot and update_bot.', -@@ -251,13 +251,13 @@ export function install(rt) { + parameters: {}, +@@ -251,13 +436,13 @@ const bots = [] const unknown = [] for (const ref of nameList(refs)) { @@ -1773,7 +1774,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js const groupNames = self => visibleRooms(self).map(room => `"${room.name}"`).join(', ') || 'none' const pickRoom = (ref, self) => { const matches = rt.findRooms(ref).filter(room => visibleRooms(self).includes(room)) -@@ -283,7 +283,7 @@ export function install(rt) { +@@ -283,7 +468,7 @@ if (!rt.isMain(self)) return 'Only a Main Bot can create group chats. Ask a Main Bot instead.' const { bots, unknown } = resolveBots(args.members) if (unknown.length > 0) return `No Bot named ${unknown.join(', ')}. Team: ${teamNames()}` @@ -1782,7 +1783,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js if (admin === undefined) return `No Bot named ${args.admin}. Team: ${teamNames()}` const members = [...new Set([self, ...bots.map(bot => bot.id), admin.id])] try { -@@ -297,7 +297,7 @@ export function install(rt) { +@@ -297,7 +482,7 @@ ctx.tools.register(defineTool({ name: 'update_group', @@ -1791,7 +1792,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js parameters: { group: GROUP_REF, name: { type: 'string', description: 'New group name; an empty string names it after its members again' }, -@@ -305,6 +305,7 @@ export function install(rt) { +@@ -305,6 +490,7 @@ add_members: { type: 'array', items: { type: 'string' }, description: 'Exact names of Bots to add' }, remove_members: { type: 'array', items: { type: 'string' }, description: 'Exact names of Bots to remove' }, admin: { type: 'string', description: 'Member to make the admin; "none" leaves the group without one (Main Bots only)' }, @@ -1799,7 +1800,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js mode: MODE_PARAM, work_dir: { ...WORK_DIR_PARAM, description: `${WORK_DIR_PARAM.description}. Every member works there in this group; an empty string lets each member work in its own folder again (Main Bots only)` }, }, -@@ -325,10 +325,18 @@ export function install(rt) { +@@ -325,10 +511,18 @@ if (args.admin !== undefined && args.admin !== '') { if (/^(none|nobody|null|无|没有)$/i.test(String(args.admin).trim())) admin = null else { @@ -1819,7 +1820,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js try { const workDir = moving ? await rt.cleanWorkDir(args.work_dir) : undefined // A Bot that joins a group with a folder works there from its first turn. -@@ -345,6 +345,7 @@ export function install(rt) { +@@ -345,6 +539,7 @@ notice: args.notice, mode: args.mode, admin, @@ -1827,7 +1828,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js add: add.bots.map(bot => bot.id), remove: remove.bots.map(bot => bot.id), ...(moving ? { workDir: workDir ?? '' } : {}), -@@ -395,12 +395,11 @@ export function install(rt) { +@@ -395,12 +590,11 @@ if ((info?.hop ?? 0) >= MAX_HOPS) return 'This chain of Bot messages is already long. Stop here and report to the user instead.' const text = String(args.message ?? '').trim() if (text === '') return 'Write a message to post.' @@ -1843,7 +1844,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js } catch (failure) { return `Could not post in "${room.name}": ${failure.message}` } -@@ -445,6 +445,10 @@ export function install(rt) { +@@ -445,6 +639,10 @@ .filter(option => option.label !== '') .slice(0, 6) if (options.length === 0) return 'Give at least one option.' @@ -1854,7 +1855,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js // A hidden Bot-to-Bot turn shows none of its calls, so its card goes to the // composer dock, which takes questions without a call. const turn = rt.live.get(id)?.turn -@@ -465,6 +465,77 @@ export function install(rt) { +@@ -465,6 +663,77 @@ }, })) @@ -1932,7 +1933,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js // dsh's own question tool holds the turn open until the user answers, and every // message from another Bot or a group round queues behind it. Bot Sessions ask // through ask_user instead, so the tool is taken out of their tool list. -@@ -525,7 +525,7 @@ export function install(rt) { +@@ -525,7 +794,7 @@ name: 'save_capability', description: 'Create or update a shared MCP server, skill, or pack. Changes apply from your next message.', parameters: { @@ -1941,7 +1942,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js kind: { type: 'string', required: true, enum: ['mcp', 'skill', 'pack'] }, description: { type: 'string', description: 'What this capability provides' }, transport: { type: 'string', enum: ['stdio', 'streamable-http'], description: 'How to reach an MCP server: a local command or an HTTP URL' }, -@@ -545,7 +545,7 @@ export function install(rt) { +@@ -545,7 +814,7 @@ try { const item = await rt.capabilitySave(args, { botId: self }) const approval = item.kind === 'mcp' && item.transport === 'stdio' && !item.approved @@ -1950,7 +1951,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js : '' return `Saved ${item.name}. Bots with it attached use the change from their next message.${approval}` } catch (error) { return error.message } -@@ -587,7 +587,7 @@ export function install(rt) { +@@ -587,7 +856,7 @@ : attached ? [attached] : [] const approvals = members .filter(item => item.kind === 'mcp' && item.transport === 'stdio' && item.approved !== true) @@ -1959,7 +1960,7 @@ diff --git a/src/host/tools.js b/src/host/tools.js .join('') return `${verb} ${args.name} ${args.bot ? `to ${result.bot}` : 'to yourself'}. Changes apply from ${result.bot}'s next message.${endpoint === 'attach' ? approvals : ''}` } catch (error) { return error.message } -@@ -595,5 +595,159 @@ export function install(rt) { +@@ -595,5 +864,172 @@ })) } @@ -2011,6 +2012,15 @@ diff --git a/src/host/tools.js b/src/host/tools.js + }, + additionalProperties: false, + }, ++ rework: { ++ type: 'object', ++ description: 'Return a card you claimed to the role that produced it, with a one-line reason. This is the target role\'s own refusal: a rework card linked to the returned one opens in the producer\'s pool, and the producer takes it without a dispatch.', ++ properties: { ++ task: { type: 'string', required: true, description: 'The card you claimed and are returning.' }, ++ reason: { type: 'string', required: true, description: 'One line on what is missing.' }, ++ }, ++ additionalProperties: false, ++ }, + tasks: { + type: 'array', + description: 'Tasks to create or update. Omit to read your list.', @@ -2103,6 +2113,10 @@ diff --git a/src/host/tools.js b/src/host/tools.js + const why = result?.reason === 'wip' ? ` You already hold an in-progress card (${result.held}).` : '' + return `No queued item for ${role === '' ? 'your role' : role} right now.${why}` + } ++ if (args?.rework !== undefined) { ++ const child = await rt.trackBotTasks(self, args ?? {}, { sessionId: exec.agent?.id, roomId }) ++ return `Rework ${child.id} on ${child.reworkOf} for the ${child.handoff?.to ?? 'producer'} pool: ${String(child.note ?? '').slice(0, 200)}\nThe producer takes it from its own pool. Set that card's column to in-progress when you pick it up. Do not post a status in the group.` ++ } + if (args?.release !== undefined) { + const returned = await rt.trackBotTasks(self, args ?? {}, { sessionId: exec.agent?.id, roomId }) + const seconds = Math.max(0, Math.round(((returned.retryAt ?? 0) - Date.now()) / 1000)) diff --git a/patches/ds-bot/files/src/host/task-drive.js b/patches/ds-bot/files/src/host/task-drive.js index 5bcb4a5de..b40bc8555 100644 --- a/patches/ds-bot/files/src/host/task-drive.js +++ b/patches/ds-bot/files/src/host/task-drive.js @@ -302,18 +302,26 @@ export function install(rt) { if (!task?.queueRole || task.claimedBy || task.status !== 'open') continue const roomId = task.origin?.roomId const room = roomId ? rt.roomOf(roomId) : undefined - if (!room || room.origin === 'face-to-face') continue + if (roomId && (!room || room.origin === 'face-to-face')) continue if (Number.isFinite(task.retryAt) && task.retryAt > clock.now()) continue - const key = `${roomId}\u0000${String(task.queueRole).toLowerCase()}` + // A handoff card (#1775) waits in its role's pool, room or not: the key is the role, so a + // card handed from one role to the next is offered in the same pass with no dispatch. + const key = `${roomId ?? ''}\u0000${String(task.queueRole).toLowerCase()}` const list = byRoomRole.get(key) ?? [] list.push(task) byRoomRole.set(key, list) } for (const list of byRoomRole.values()) { const item = list.slice().sort(compareForQueue)[0] - const room = rt.roomOf(item.origin.roomId) - const carriers = (room?.members ?? []) - .filter(id => { const bot = rt.botOf(id); return Boolean(bot) && bot.remote === undefined && sameRole(bot.role, item.queueRole) }) + const room = item.origin?.roomId ? rt.roomOf(item.origin.roomId) : undefined + const carriers = room + ? (room.members ?? []) + .filter(id => { const bot = rt.botOf(id); return Boolean(bot) && bot.remote === undefined && sameRole(bot.role, item.queueRole) }) + // No room: the pool is every local Bot carrying the role, so a handoff chain runs between + // members of the team that never share a group. + : Object.values(rt.state.bots ?? {}) + .filter(bot => Boolean(bot) && bot.remote === undefined && bot.role && !retiredBot(bot) && sameRole(bot.role, item.queueRole)) + .map(bot => bot.id) for (const botId of carriers) { const sessionId = rt.chatOf(botId) if (!sessionId || rt.roomOf(sessionId)) continue diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index 749180e37..2459d183b 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -485,11 +485,23 @@ export function install(rt) { if (taskClass !== undefined) task.taskClass = taskClass if (reworkOf !== undefined) task.reworkOf = reworkOf if (handoff !== undefined) task.handoff = handoff + if (handoff !== undefined) routeToPool(task, handoff) if (evidence !== undefined) task.evidence = evidence if (clearingLease) delete task.lease if (lease !== undefined) task.lease = lease return { taskClass, reworkOf, handoff, evidence, lease } } + // The route on the card (#1775): `handoff.to` names the role that owns the next step, so it is + // the card's pool. Recording the handoff starts the card in that pool; recording it on a card a + // member is holding hands the card on in the same write, so the producer lets go of it and the + // target role can take it with no dispatch from a supervisor. + function routeToPool(task, handoff) { + const held = task.claimedBy !== undefined || task.status === 'claimed' || task.status === 'working' || task.status === 'blocked' + task.queueRole = handoff.to + if (held) requeue(task, `handoff to ${handoff.to}`, 0, 'handed on') + else if (task.queueBy === undefined) task.queueBy = task.ownerBotId + return task.queueRole + } // The typed contract on a card that is created here rather than written by a caller. const defaultContract = (task) => { task.taskClass = 'work' @@ -642,15 +654,20 @@ export function install(rt) { // roles, and the rework is one child card linked by `reworkOf` — never `parentId`, which // means "part of an overall card" and must never make `settleOverall` fire. A replay of the // same rejection reuses the child that is still open instead of creating a second one. - function openRework(original, note) { + function openRework(original, note, by = 'requester') { const reason = boundedLine(note, HANDOFF_REASON_MAX, 'handoff.reason') - const to = oneLine(botRecord(original.ownerBotId)?.role ?? '', ROLE_MAX) || 'requester' - const from = 'requester' + // A worker-side return names the role that produced the card (#1775); the requester's own + // send-back stays the owner's role, exactly as before. + const to = by === 'requester' + ? oneLine(botRecord(original.ownerBotId)?.role ?? '', ROLE_MAX) || 'requester' + : oneLine(original.handoff?.from ?? '', ROLE_MAX) + const from = by const at = stamp() original.handoff = { from, to, at, reason } const existing = Object.values(store()).find(task => task.reworkOf === original.id && openTask(task)) if (existing !== undefined) { existing.handoff = { from, to, at, reason } + if (by !== 'requester') existing.queueRole = to existing.actors = [...(existing.actors ?? []), { status: existing.status, at, by: 'rework replayed' }] existing.updatedAt = at return existing @@ -668,6 +685,7 @@ export function install(rt) { note: reason, source: 'rework', handoff: { from, to, at, reason }, + ...(by === 'requester' ? {} : { queueRole: to, queueBy: original.queueBy ?? original.ownerBotId }), origin: { ...(original.origin ?? {}) }, actors: [{ status: 'open', at, by: 'rework opened' }], } @@ -775,7 +793,7 @@ export function install(rt) { .filter(bot => Boolean(bot) && sameRole(bot.role, role)) // blocked → open is the queue's own edge: a member that cannot finish returns the item, and // no other path sends a card back to the queue, so the edge is not on TRANSITIONS. - function requeue(task, reason, retryInSec) { + function requeue(task, reason, retryInSec, by = 'returned to the queue') { dropWorker(task.worker) delete task.worker delete task.waitingFor @@ -789,7 +807,7 @@ export function install(rt) { task.progress = 0 task.retryAt = Date.now() + retryInSec * 1000 task.queueReason = reason - record(task, null, { by: 'returned to the queue', retryInSec }) + record(task, null, { by, retryInSec }) } const resolveParent = (roomId, ref, batch) => { const text = String(ref).trim() @@ -922,7 +940,9 @@ export function install(rt) { async function claimQueued(botId, body, roomId) { const room = resolveRoom(botId, body.group, roomId) const entry = room ? state.rooms?.[room] : undefined - if (!entry) throw invalid('A claim needs a group') + // A handoff card waits in its role's pool, not in a room (#1775): a member of the role takes it + // from its own chat, so a chain handed from one role to the next needs no group and no dispatch. + if (!entry) return claimHandoff(botId, body) if (expireLeases() > 0) await rt.save() const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) if (myRole === '') throw refuse('not-in-role', 'this Bot has no role to claim with') @@ -972,6 +992,66 @@ export function install(rt) { return { item: view(live) } } + // The role's own pool (#1775): a card whose `handoff.to` names this member's role is offered to + // it and to nobody else, and the same compare-and-set gives one member one card. A card with no + // room is not offered to a supervisor: the target role takes it, and an idle member wakes for it. + async function claimHandoff(botId, body) { + if (expireLeases() > 0) await rt.save() + const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) + if (myRole === '') throw refuse('not-in-role', 'this Bot has no role to claim with') + const holding = Object.values(store()).filter(task => task.ownerBotId === botId && occupiesSlot(task)) + if (holding.length > 0) return { item: null, reason: 'wip', held: holding.sort(compareForWork)[0].id } + const now = Date.now() + const inPool = (task) => { + if (task?.status !== 'open' || task.claimedBy !== undefined) return false + if (!sameRole(task.handoff?.to, myRole)) return false + return !(Number.isFinite(task.retryAt) && task.retryAt > now) + } + const wantedId = body.id !== undefined && body.id !== null && String(body.id) !== '' ? String(body.id) : undefined + let pick + if (wantedId !== undefined) { + pick = find(wantedId) + if (pick === undefined) throw stale(`no task ${wantedId}`) + if (!inPool(pick)) throw stale(`no card for ${myRole} in the handoff pool`) + } else { + pick = Object.values(store()).filter(inPool).sort(compareForQueue)[0] + } + if (pick === undefined) return { item: null, reason: 'empty' } + await rt.load() + const live = find(pick.id) + if (live === undefined || live.status !== 'open' || live.claimedBy !== undefined) return { item: null, reason: 'taken' } + live.claimedBy = botId + live.ownerBotId = botId + delete live.retryAt + delete live.queueReason + const claimedAt = Date.now() + live.lease = { holder: botId, takenAt: claimedAt, expiresAt: claimedAt + LEASE_TTL_SEC * 1000 } + live.worker = newWorker() + transition(live, 'claimed', null, { by: 'claimed from the handoff pool' }) + announceClaim(live) + await save() + return { item: view(live) } + } + + // The target role's admission comes back as a rework card (#1775): the member holding the card + // returns it to the role that produced it (`handoff.from`), and the producer takes it from its + // own pool. The child is linked by `reworkOf`, never `parentId`, and the original stays `working` + // until the rework is accepted (`closeRework`). + async function returnToProducer(botId, spec) { + const task = find(spec?.task ?? spec?.id) + if (task === undefined) throw stale(`no task ${String(spec?.task ?? spec?.id ?? '')}`) + if (task.claimedBy !== botId) throw forbidden('Only the member that claimed this card may return it to its producer') + const myRole = oneLine(botRecord(botId)?.role ?? '', ROLE_MAX) + if (!sameRole(task.handoff?.to, myRole)) throw refuse('not-in-role', `card ${task.id} is in the ${task.handoff?.to ?? 'unknown'} pool, not ${myRole}`) + const reason = readOptionalLine(spec?.reason, 'A reason') + if (reason === undefined) throw invalid('Returning a card to its producer needs a one-line reason') + const producer = oneLine(task.handoff?.from ?? '', ROLE_MAX) + if (producer === '' || producer === 'requester') throw refuse('no-producer', `card ${task.id} carries no producer role to return it to`) + const child = openRework(task, reason, myRole) + await save() + return view(child) + } + // A member that cannot finish returns the item with one line of why; it is not offered // again before `retryAt` (#1247). async function releaseQueued(botId, spec) { @@ -1002,6 +1082,7 @@ export function install(rt) { // Both come before the read path: a claim arrives with `tasks` omitted, so reading first // would return the list instead of the item (#1247). if (body?.release !== undefined) return releaseQueued(botId, body.release) + if (body?.rework !== undefined) return returnToProducer(botId, body.rework) if (body?.claim === true) return claimQueued(botId, body, origin.roomId) const items = body?.tasks if (items === undefined) return listed(botId) diff --git a/patches/ds-bot/files/test/task-handoff-route.test.js b/patches/ds-bot/files/test/task-handoff-route.test.js new file mode 100644 index 000000000..dff98491d --- /dev/null +++ b/patches/ds-bot/files/test/task-handoff-route.test.js @@ -0,0 +1,178 @@ +// The route is the card (#1775): `handoff.to` names the role that owns the next step, so the card +// waits in that role's pool and an idle member of the role wakes for it. Nothing here is a +// dispatch: the producer records the handoff, the target role takes the card, and a card with no +// room is still offered, because the pool is the role and not the group. +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 team = () => ({ + version: 2, + revision: 1, + mainBotIds: ['a'], + members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }, + bots: { + a: { id: 'a', name: 'Chief', role: 'Lead', sessionId: 'a', createdAt: 1, ownerId: 'owner-1' }, + d1: { id: 'd1', name: 'Dev1', role: 'Dev', sessionId: 'd1', createdAt: 2, ownerId: 'owner-1', createdBy: 'a' }, + r1: { id: 'r1', name: 'Rev1', role: 'Reviewer', sessionId: 'r1', createdAt: 3, ownerId: 'owner-1', createdBy: 'a' }, + }, + rooms: {}, + grants: {}, + agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, +}) + +async function start(t) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-handoff-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(team())) + const ctx = fakeContext() + const agents = new Map() + const agentOf = (sessionId) => { + if (!agents.has(sessionId)) { + const inbox = { + nextStep: [], + nextTurn: [], + remove(id) { + const keep = message => message.id !== id + const before = this.nextTurn.length + this.nextStep.length + this.nextTurn = this.nextTurn.filter(keep) + this.nextStep = this.nextStep.filter(keep) + return this.nextTurn.length + this.nextStep.length !== before + }, + } + agents.set(sessionId, { + session: { id: sessionId }, id: sessionId, calls: [], status: 'idle', inbox, + followup(message) { this.calls.push(message); this.inbox.nextTurn.push(message) }, + steer() {}, inject() {}, cancel() {}, async whenIdle() {}, + }) + } + return agents.get(sessionId) + } + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async () => ({ id: 'w1' }), archiveSession: async () => {} }, + sessionController: { + create: async () => ({ sessionId: 'made' }), + 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 () => { + await ctx.dispose() + rmSync(home, { recursive: true, force: true, maxRetries: 5 }) + }) + const track = (sessionId, args) => ctx.registeredTools.get('track_tasks').execute(args ?? {}, { agent: { id: sessionId } }) + const calls = (sessionId) => agents.get(sessionId)?.calls ?? [] + const text = (sessionId) => calls(sessionId).map(message => message.content.map(block => block.text ?? '').join('\n')).join('\n') + const drive = async () => { await rt.considerTaskDrive(); await ctx.taskDrive.settled() } + const idOf = (body) => { + const id = String(body).match(/task-[0-9a-f]+/)?.[0] + assert.ok(id, body) + return id + } + return { ctx, rt, track, calls, text, drive, idOf, home } +} + +test('a card handed to a role wakes that role and one member claims it, with no dispatch', async (t) => { + const app = await start(t) + // The Chief writes the opening handoff once: the card is in the Dev pool from that write. + const created = await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev', reason: 'first slice' } }] }) + const id = app.idOf(created) + assert.equal(app.rt.state.tasks[id].queueRole, 'Dev') + assert.equal(app.rt.state.tasks[id].handoff.to, 'Dev') + + await app.drive() + assert.match(app.text('d1'), new RegExp(`Take ${id} `), 'the Dev is offered the card by the route alone') + assert.equal(app.text('r1'), '', 'another role is not woken') + + assert.match(await app.track('d1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) + assert.equal(app.rt.state.tasks[id].claimedBy, 'd1') + assert.equal(app.rt.state.tasks[id].status, 'claimed') +}) + +test('a producer hands on its own card and the target role takes it; from and evidence survive', async (t) => { + const app = await start(t) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + + // The producer's own write is the whole handover: it names the role, records what proves the work. + const handed = await app.track('d1', { + tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' }, evidence: [{ sha: 'abcdef1234', check: 'node --test' }] }], + }) + assert.ok(handed) + const card = app.rt.state.tasks[id] + assert.equal(card.status, 'open', 'the producer let go of the card in the same write') + assert.equal(card.claimedBy, undefined) + assert.equal(card.queueRole, 'Reviewer') + assert.equal(card.handoff.from, 'Dev') + assert.equal(card.evidence.length, 1) + assert.equal(app.calls('a').length, 0, 'the Chief ran nothing while the card moved on') + + await app.drive() + assert.match(app.text('r1'), new RegExp(`Take ${id} `)) + assert.match(await app.track('r1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) + assert.equal(app.rt.state.tasks[id].claimedBy, 'r1') + assert.equal(app.rt.state.tasks[id].handoff.from, 'Dev', 'the card still says who produced it') + assert.deepEqual(app.rt.state.tasks[id].evidence.map(entry => entry.sha), ['abcdef1234']) + assert.equal(app.calls('a').length, 0, 'the Chief ran nothing') +}) + +test('with no idle member of the role the card waits in the pool and is taken once one appears', async (t) => { + const app = await start(t) + // The Reviewer already holds a card, so it cannot take another. + const busy = app.idOf(await app.track('r1', { tasks: [{ title: 'Review the last diff', column: 'in-progress' }] })) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + await app.track('d1', { tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' } }] }) + + await app.drive() + const waiting = app.rt.state.tasks[id] + assert.equal(waiting.status, 'open', 'the card is not lost when nobody can take it') + assert.equal(waiting.claimedBy, undefined) + assert.equal(waiting.queueRole, 'Reviewer', 'it stays in the Reviewer pool') + assert.doesNotMatch(app.text('r1'), new RegExp(`Take ${id} `)) + + // The Reviewer delivers the card it held; the pool is offered again on the next pass. + await app.rt.taskEndpoint('task-return', { id: busy, summary: 'the last diff is fine' }) + await app.drive() + assert.match(app.text('r1'), new RegExp(`Take ${id} `)) + assert.match(await app.track('r1', { claim: true, id }), new RegExp(`^Claimed ${id} `)) +}) + +test('a role returns the card to its producer as a rework card the producer can take', async (t) => { + const app = await start(t) + const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev' } }] })) + await app.drive() + await app.track('d1', { claim: true, id }) + await app.track('d1', { tasks: [{ id, handoff: { from: 'Dev', to: 'Reviewer', reason: 'diff ready' }, evidence: [{ sha: 'abcdef1234', check: 'node --test' }] }] }) + await app.drive() + await app.track('r1', { claim: true, id }) + await app.track('r1', { tasks: [{ id, column: 'in-progress' }] }) + + const returned = await app.track('r1', { rework: { task: id, reason: 'no test for the return path' } }) + const childId = app.idOf(returned) + const child = app.rt.state.tasks[childId] + assert.equal(child.taskClass, 'rework') + assert.equal(child.reworkOf, id, 'the rework points back at the card it fixes') + assert.equal(child.handoff.to, 'Dev', 'and at the role that produced it') + assert.equal(child.handoff.from, 'Reviewer') + assert.equal(child.queueRole, 'Dev') + assert.equal(app.rt.state.tasks[id].status, 'working', 'the original waits for the rework to be accepted') + + await app.drive() + assert.doesNotMatch(app.text('d1').split('Continue ').at(-1) ?? '', /never/, 'sanity') + const wake = app.calls('d1').map(message => message.content.map(block => block.text ?? '').join('\n')).find(line => line.includes(`Take ${childId} `)) + assert.ok(wake, `the producer is offered the rework card, got:\n${app.text('d1')}`) + assert.match(await app.track('d1', { claim: true, id: childId }), new RegExp(`^Claimed ${childId} `)) + assert.equal(app.rt.state.tasks[childId].claimedBy, 'd1') +}) diff --git a/patches/ds-bot/topics.json b/patches/ds-bot/topics.json index 4c4b5938e..5e1875af0 100644 --- a/patches/ds-bot/topics.json +++ b/patches/ds-bot/topics.json @@ -142,5 +142,8 @@ ], "0016-room-collaboration-smoke": [ "dev/mock-llm.mjs" + ], + "0017-handoff-route": [ + "src/host/tools.js" ] }