diff --git a/changelog.d/202-cancel-returned-result.fixed.md b/changelog.d/202-cancel-returned-result.fixed.md new file mode 100644 index 00000000..94ab7aa5 --- /dev/null +++ b/changelog.d/202-cancel-returned-result.fixed.md @@ -0,0 +1 @@ +Requesters can cancel a returned task result before accepting it. The result and decision history are preserved, and a cancelled task cannot later be accepted. ([#202](https://github.com/ThinkFlowLab/FlyBot/issues/202)) diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index 7a7af572..d6bd49ec 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -10,9 +10,9 @@ // its authority. Outgoing holds stay the owner's (#52 card). // // States: open → claimed → working → blocked ⇄ working → result-ready → accepted; -// cancelled and failed are terminal from any active state, and accepted is reachable -// only from result-ready — so a failed or cancelled task can never display as -// accepted. The record stores pointers and a summary, never private transcripts. +// A returned result may also be cancelled before acceptance. Cancelled and failed +// are terminal, and accepted is reachable only from result-ready. A cancelled or +// failed task never becomes accepted. Records keep pointers and a summary, not transcripts. import { randomBytes } from 'node:crypto' import { currentParticipants } from './room-membership.js' @@ -24,7 +24,7 @@ const TRANSITIONS = { claimed: ['working', 'cancelled', 'failed'], working: ['blocked', 'result-ready', 'cancelled', 'failed'], blocked: ['working', 'cancelled', 'failed'], - 'result-ready': ['accepted'], + 'result-ready': ['accepted', 'cancelled'], accepted: [], cancelled: [], failed: [], diff --git a/core/test/tasks.test.js b/core/test/tasks.test.js index ace45e73..9163e75f 100644 --- a/core/test/tasks.test.js +++ b/core/test/tasks.test.js @@ -95,6 +95,113 @@ const statusOf = async (call, token, id) => { return list.value.find(task => task.id === id) } +async function returnTask(call, createTask, token) { + const created = await createTask(token) + assert.equal(created.ok, true, JSON.stringify(created)) + const id = created.value.id + for (const endpoint of ['task-accept', 'task-start']) { + const moved = await call(endpoint, { id }, token) + assert.equal(moved.ok, true, JSON.stringify(moved)) + } + const returned = await call('task-return', { id, summary: 'Returned evidence awaiting a decision' }, token) + assert.equal(returned.ok, true, JSON.stringify(returned)) + return returned.value +} + +test('the requester can cancel a returned result after restart without losing its evidence', async (t) => { + const first = await start(t) + let app = first + t.after(() => app.stop()) + const alice = await first.signIn('Alice') + const returned = await returnTask(first.call, first.createTask, alice.sessionToken) + await first.stop({ keepHome: true }) + const restored = await start(t, { home: first.home }) + app = restored + const cancelled = await restored.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(cancelled.ok, true, JSON.stringify(cancelled)) + assert.equal(cancelled.value.status, 'cancelled') + assert.deepEqual(cancelled.value.result, returned.result) + assert.deepEqual(cancelled.value.actors.slice(0, -1), returned.actors) + assert.equal(cancelled.value.actors.at(-1).status, 'cancelled') + assert.equal(cancelled.value.actors.at(-1).memberId, alice.memberId) + await restored.stop({ keepHome: true }) + const again = await start(t, { home: first.home }) + app = again + const saved = await statusOf(again.call, alice.sessionToken, returned.id) + assert.equal(saved.status, 'cancelled') + assert.deepEqual(saved.result, returned.result) + for (const endpoint of ['task-accept-result', 'task-accept', 'task-start', 'task-return', 'task-cancel']) { + const late = await again.call(endpoint, { id: returned.id, summary: 'late result' }, alice.sessionToken) + assert.equal(late.ok, false, `${endpoint}: ${JSON.stringify(late)}`) + assert.equal(late.error.code, 'bot/stale') + } +}) + +test('returned-result cancellation keeps requester authority despite forged body claims', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const bob = await app.signIn('Bob', alice.sessionToken) + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + for (const token of [bob.sessionToken, undefined]) { + const refused = await app.call('task-cancel', { id: returned.id, actorId: alice.memberId, requesterId: alice.memberId }, token) + assert.equal(refused.ok, false) + assert.equal(refused.error.code, token ? 'bot/forbidden' : 'bot/unauthorized') + assert.equal((await statusOf(app.call, alice.sessionToken, returned.id)).status, 'result-ready') + } + const accepted = await app.call('task-accept-result', { id: returned.id }, alice.sessionToken) + assert.equal(accepted.ok, true, JSON.stringify(accepted)) + const afterAcceptance = await app.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(afterAcceptance.ok, false) + assert.equal(afterAcceptance.error.code, 'bot/stale') +}) + +test('the requester, rather than the assigned Bot owner, decides whether to cancel a returned result', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const bob = await app.signIn('Bob', alice.sessionToken) + await app.fb.grants.set('s', [{ grantee: bob.memberId }]) + const returned = await returnTask(app.call, token => app.createTask(token, { botId: 's' }), bob.sessionToken) + assert.equal(returned.requesterId, bob.memberId) + const foreign = await app.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(foreign.ok, false) + assert.equal(foreign.error.code, 'bot/forbidden') + const cancelled = await app.call('task-cancel', { id: returned.id }, bob.sessionToken) + assert.equal(cancelled.ok, true, JSON.stringify(cancelled)) + assert.equal(cancelled.value.status, 'cancelled') + assert.equal(cancelled.value.actors.at(-1).memberId, bob.memberId) +}) + +test('a revoked requester session cannot cancel its returned result', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + const revoked = await app.call('auth-revoke', {}, alice.sessionToken) + assert.equal(revoked.ok, true, JSON.stringify(revoked)) + const refused = await app.call('task-cancel', { id: returned.id, actorId: alice.memberId }, alice.sessionToken) + assert.equal(refused.ok, false) + assert.equal(refused.error.code, 'bot/unauthorized') + const saved = JSON.parse(readFileSync(app.statePath, 'utf8')).tasks[returned.id] + assert.equal(saved.status, 'result-ready') + assert.deepEqual(saved.result, returned.result) +}) + +test('acceptance racing returned-result cancellation commits only one terminal outcome', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + const outcomes = await Promise.all(['task-accept-result', 'task-cancel'].map(endpoint => app.call(endpoint, { id: returned.id }, alice.sessionToken))) + assert.equal(outcomes.filter(outcome => outcome.ok).length, 1) + assert.equal(outcomes.find(outcome => !outcome.ok).error.code, 'bot/stale') + const task = await statusOf(app.call, alice.sessionToken, returned.id) + assert.ok(['accepted', 'cancelled'].includes(task.status)) + assert.equal(task.actors.length, returned.actors.length + 1) + assert.deepEqual(task.result, returned.result) +}) + test('a task walks the happy path: created, claimed, worked, returned, accepted by the requester', async (t) => { const { call, signIn, createTask } = await start(t) const alice = await signIn('Alice') diff --git a/docs/interop/opencode-pilot.md b/docs/interop/opencode-pilot.md index 53872c79..f143707a 100644 --- a/docs/interop/opencode-pilot.md +++ b/docs/interop/opencode-pilot.md @@ -149,10 +149,11 @@ facets alongside it, not invented values written into `task.status`. | Abort observed | Show `cancel: observed` only after a version-specific runtime observation confirms that execution has stopped; an HTTP success alone is insufficient. No late event may resume/complete the task. | | Lost connection or ambiguous write | Show `recovery: uncertain`, retain raw status and suppress automatic dispatch/acceptance until reconciled. Do not map uncertainty to success, stopped or zero cost. | -Current `tasks.js` allows `result-ready → accepted` only; it cannot cancel or -reject a returned result. That limitation must be addressed in the host lifecycle -work, or remain an explicit live-pilot blocker. A revision cannot silently reopen -an accepted/cancelled/failed task; it requires a new task with an explicit link. +The requester can dispose of a returned result through `result-ready → cancelled` +using the existing `task-cancel` endpoint (#202), preserving its summary and actor +history. This does not confirm that an external runtime stopped; execution-stop +evidence remains a separate connector obligation. A revision cannot silently +reopen an accepted/cancelled/failed task; it requires a new task with an explicit link. ## Journal, recovery and budget diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 14252a81..23a0efb8 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -54,6 +54,13 @@ Add a new invariant under the heading it belongs to (or a new heading), not at t ### Tasks +The requester may also cancel a `result-ready` task before acceptance (#202). +`task-cancel` keeps the returned summary and actor history; the cancelled task +survives restart and cannot later be accepted, restarted or returned again. +Accepted/failed/cancelled tasks stay terminal. This records the requester's task +disposition, not proof that an external runtime stopped; no runtime abort or +side-effect rollback is implied. + A tracked request with a stable id (`task-`) and a small state machine, owned by `src/host/tasks.js` with `state.tasks` (additive; corrupt values reset, no version bump). States: `open → claimed → working → blocked ⇄ working → result-ready → accepted`, with `cancelled`/`failed` terminal and `accepted` reachable only from `result-ready` — a failed or cancelled task can never display as accepted (#54). Transitions after create are the requester's (server-derived `requesterId`, the signed-in actor or the single owner; `bot/forbidden` otherwise, `bot/stale` on invalid moves), recorded with per-status actor entries. Create does not require owning the Bot: a matching per-Bot grant, or both the requester and the Bot as current members of the named room, is enough; outgoing holds stay the owner's (#52 card). A working task's held send blocks **that** task (`meta.taskId` stamped on the send, matching origin when the Bot has several working tasks) with the approval id; the owner's decide resumes (approve) or cancels (reject) it; deleting the responsible Bot fails its open tasks. `result-ready` is protected from prune while it waits for acceptance. Endpoints `tasks` / `task-create` / `task-accept` / `task-start` / `task-return` / `task-accept-result` / `task-cancel` are an explicit authz allow-list and are record-checked in tasks.js. Records store pointers and a copied summary, never private transcripts. Clarification and `workBotId` wait on #55's exchange wiring. Team mode stays disabled. ### Inbound shared rooms diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index 7a7af572..d6bd49ec 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -10,9 +10,9 @@ // its authority. Outgoing holds stay the owner's (#52 card). // // States: open → claimed → working → blocked ⇄ working → result-ready → accepted; -// cancelled and failed are terminal from any active state, and accepted is reachable -// only from result-ready — so a failed or cancelled task can never display as -// accepted. The record stores pointers and a summary, never private transcripts. +// A returned result may also be cancelled before acceptance. Cancelled and failed +// are terminal, and accepted is reachable only from result-ready. A cancelled or +// failed task never becomes accepted. Records keep pointers and a summary, not transcripts. import { randomBytes } from 'node:crypto' import { currentParticipants } from './room-membership.js' @@ -24,7 +24,7 @@ const TRANSITIONS = { claimed: ['working', 'cancelled', 'failed'], working: ['blocked', 'result-ready', 'cancelled', 'failed'], blocked: ['working', 'cancelled', 'failed'], - 'result-ready': ['accepted'], + 'result-ready': ['accepted', 'cancelled'], accepted: [], cancelled: [], failed: [], diff --git a/patches/ds-bot/files/test/tasks.test.js b/patches/ds-bot/files/test/tasks.test.js index ace45e73..9163e75f 100644 --- a/patches/ds-bot/files/test/tasks.test.js +++ b/patches/ds-bot/files/test/tasks.test.js @@ -95,6 +95,113 @@ const statusOf = async (call, token, id) => { return list.value.find(task => task.id === id) } +async function returnTask(call, createTask, token) { + const created = await createTask(token) + assert.equal(created.ok, true, JSON.stringify(created)) + const id = created.value.id + for (const endpoint of ['task-accept', 'task-start']) { + const moved = await call(endpoint, { id }, token) + assert.equal(moved.ok, true, JSON.stringify(moved)) + } + const returned = await call('task-return', { id, summary: 'Returned evidence awaiting a decision' }, token) + assert.equal(returned.ok, true, JSON.stringify(returned)) + return returned.value +} + +test('the requester can cancel a returned result after restart without losing its evidence', async (t) => { + const first = await start(t) + let app = first + t.after(() => app.stop()) + const alice = await first.signIn('Alice') + const returned = await returnTask(first.call, first.createTask, alice.sessionToken) + await first.stop({ keepHome: true }) + const restored = await start(t, { home: first.home }) + app = restored + const cancelled = await restored.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(cancelled.ok, true, JSON.stringify(cancelled)) + assert.equal(cancelled.value.status, 'cancelled') + assert.deepEqual(cancelled.value.result, returned.result) + assert.deepEqual(cancelled.value.actors.slice(0, -1), returned.actors) + assert.equal(cancelled.value.actors.at(-1).status, 'cancelled') + assert.equal(cancelled.value.actors.at(-1).memberId, alice.memberId) + await restored.stop({ keepHome: true }) + const again = await start(t, { home: first.home }) + app = again + const saved = await statusOf(again.call, alice.sessionToken, returned.id) + assert.equal(saved.status, 'cancelled') + assert.deepEqual(saved.result, returned.result) + for (const endpoint of ['task-accept-result', 'task-accept', 'task-start', 'task-return', 'task-cancel']) { + const late = await again.call(endpoint, { id: returned.id, summary: 'late result' }, alice.sessionToken) + assert.equal(late.ok, false, `${endpoint}: ${JSON.stringify(late)}`) + assert.equal(late.error.code, 'bot/stale') + } +}) + +test('returned-result cancellation keeps requester authority despite forged body claims', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const bob = await app.signIn('Bob', alice.sessionToken) + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + for (const token of [bob.sessionToken, undefined]) { + const refused = await app.call('task-cancel', { id: returned.id, actorId: alice.memberId, requesterId: alice.memberId }, token) + assert.equal(refused.ok, false) + assert.equal(refused.error.code, token ? 'bot/forbidden' : 'bot/unauthorized') + assert.equal((await statusOf(app.call, alice.sessionToken, returned.id)).status, 'result-ready') + } + const accepted = await app.call('task-accept-result', { id: returned.id }, alice.sessionToken) + assert.equal(accepted.ok, true, JSON.stringify(accepted)) + const afterAcceptance = await app.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(afterAcceptance.ok, false) + assert.equal(afterAcceptance.error.code, 'bot/stale') +}) + +test('the requester, rather than the assigned Bot owner, decides whether to cancel a returned result', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const bob = await app.signIn('Bob', alice.sessionToken) + await app.fb.grants.set('s', [{ grantee: bob.memberId }]) + const returned = await returnTask(app.call, token => app.createTask(token, { botId: 's' }), bob.sessionToken) + assert.equal(returned.requesterId, bob.memberId) + const foreign = await app.call('task-cancel', { id: returned.id }, alice.sessionToken) + assert.equal(foreign.ok, false) + assert.equal(foreign.error.code, 'bot/forbidden') + const cancelled = await app.call('task-cancel', { id: returned.id }, bob.sessionToken) + assert.equal(cancelled.ok, true, JSON.stringify(cancelled)) + assert.equal(cancelled.value.status, 'cancelled') + assert.equal(cancelled.value.actors.at(-1).memberId, bob.memberId) +}) + +test('a revoked requester session cannot cancel its returned result', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + const revoked = await app.call('auth-revoke', {}, alice.sessionToken) + assert.equal(revoked.ok, true, JSON.stringify(revoked)) + const refused = await app.call('task-cancel', { id: returned.id, actorId: alice.memberId }, alice.sessionToken) + assert.equal(refused.ok, false) + assert.equal(refused.error.code, 'bot/unauthorized') + const saved = JSON.parse(readFileSync(app.statePath, 'utf8')).tasks[returned.id] + assert.equal(saved.status, 'result-ready') + assert.deepEqual(saved.result, returned.result) +}) + +test('acceptance racing returned-result cancellation commits only one terminal outcome', async (t) => { + const app = await start(t) + t.after(() => app.stop()) + const alice = await app.signIn('Alice') + const returned = await returnTask(app.call, app.createTask, alice.sessionToken) + const outcomes = await Promise.all(['task-accept-result', 'task-cancel'].map(endpoint => app.call(endpoint, { id: returned.id }, alice.sessionToken))) + assert.equal(outcomes.filter(outcome => outcome.ok).length, 1) + assert.equal(outcomes.find(outcome => !outcome.ok).error.code, 'bot/stale') + const task = await statusOf(app.call, alice.sessionToken, returned.id) + assert.ok(['accepted', 'cancelled'].includes(task.status)) + assert.equal(task.actors.length, returned.actors.length + 1) + assert.deepEqual(task.result, returned.result) +}) + test('a task walks the happy path: created, claimed, worked, returned, accepted by the requester', async (t) => { const { call, signIn, createTask } = await start(t) const alice = await signIn('Alice')