diff --git a/changelog.d/1569.fixed.md b/changelog.d/1569.fixed.md new file mode 100644 index 000000000..a0e6fcefc --- /dev/null +++ b/changelog.d/1569.fixed.md @@ -0,0 +1 @@ +On a read-only host — another `dsh` holds this team — `workflow/advance` and `workflow/reset` no longer move a run or write `workflows.json`: both refuse with `{ ok: false, code: 'read-only', error: 'The host is read-only' }` before they touch the run, the same way `pause` does. A node Bot's reply that would advance a run is reported once through the plugin's warning path instead of being swallowed, and a writable host is unchanged. ([#1569](https://github.com/ThinkFlowLab/FlyBot/issues/1569)) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index 45f7109e3..043b5456a 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -918,6 +918,7 @@ export function createWorkflowService({ return present() }, advance(body = {}) { + if (readOnly()) return { ok: false, code: 'read-only', error: 'The host is read-only' } const workflowId = String(body.workflowId ?? '') const outcome = body.outcome === 'failed' ? 'fail' : 'complete' const nodeId = String(body.nodeId ?? '') @@ -1021,6 +1022,7 @@ export function createWorkflowService({ }) }, reset(body = {}) { + if (readOnly()) return { ok: false, code: 'read-only', error: 'The host is read-only' } const workflowId = String(body.workflowId ?? '') const runId = body.runId ? String(body.runId) : '' if (runId) { diff --git a/plugins/workflow/test/assign.test.js b/plugins/workflow/test/assign.test.js index ab4224609..d07c3eda6 100644 --- a/plugins/workflow/test/assign.test.js +++ b/plugins/workflow/test/assign.test.js @@ -129,6 +129,38 @@ test('a read-only host refuses assign', () => { assert.equal(refused.code, 'read-only') }) +// #1569: advance and reset are writes like pause. A read-only host refuses them +// before they touch the run or the file, so neither the live run nor its queue moves. +test('#1569: a read-only host refuses advance and reset without moving the run', () => { + let readOnly = false + const svc = service({ readOnly: () => readOnly }) + svc.setConcurrency({ workflowId: DELIVERY_ID, maxConcurrent: 1 }) + svc.assign({ workflowId: DELIVERY_ID, taskId: 'task-first', title: 'First' }) + svc.assign({ workflowId: DELIVERY_ID, taskId: 'task-queued', title: 'Queued behind it' }) + const before = delivery(svc.state()) + assert.equal(before.run.nodeStatus.planner, 'running') + assert.equal(before.queueDepth, 1) + + readOnly = true + const refusedAdvance = svc.advance({ workflowId: DELIVERY_ID, nodeId: 'planner', outcome: 'done' }) + assert.deepEqual(refusedAdvance, { ok: false, code: 'read-only', error: 'The host is read-only' }) + const refusedReset = svc.reset({ workflowId: DELIVERY_ID }) + assert.deepEqual(refusedReset, { ok: false, code: 'read-only', error: 'The host is read-only' }) + + const after = delivery(svc.state()) + assert.equal(after.activeCount, 1) + assert.equal(after.run.status, 'running') + assert.equal(after.run.nodeStatus.planner, 'running') + assert.equal(after.queueDepth, 1) + assert.deepEqual(after.queue.map(task => task.id), ['task-queued']) + + // Nothing else changes on a writable host: the same call still advances. + readOnly = false + const moved = svc.advance({ workflowId: DELIVERY_ID, nodeId: 'planner', outcome: 'done' }) + assert.equal(moved.ok, true) + assert.equal(delivery(moved).run.nodeStatus.writer, 'running') +}) + test('the store reloads a workflow file and drops a file that does not parse', () => { const dir = mkdtempSync(join(tmpdir(), 'flybot-wf-')) try { diff --git a/plugins/workflow/test/ephemeral.test.js b/plugins/workflow/test/ephemeral.test.js index 7144a97dc..849d03ea3 100644 --- a/plugins/workflow/test/ephemeral.test.js +++ b/plugins/workflow/test/ephemeral.test.js @@ -413,6 +413,39 @@ test('#1389: a step the host refuses is not marked as handed, and a later settle assert.match(warnings.join('\n'), /no-sender/) }) +// #1569: the Bot reply hook calls service.stepResult directly, so it never passes +// through the host channel's read-only check. advance's own guard has to stop it, +// and the refusal is reported once instead of being swallowed. +test('#1569: a read-only host refuses the Bot reply that would advance a run', async () => { + let readOnly = false + const warnings = [] + const { svc } = harness({ service: { readOnly: () => readOnly, warn: (...args) => warnings.push(args.join(' ')) } }) + const made = svc.define({ + name: 'Pair', + nodes: [ + { id: 'plan', name: 'Planner', role: 'plan', prompt: 'Write the plan.' }, + { id: 'code', name: 'Coder', role: 'code', prompt: 'Implement it.' }, + ], + edges: [], + }).workflows.find(item => item.name === 'Pair') + const started = await svc.runNamed({ workflow: 'Pair', brief: 'Ship the note' }) + const run = started.workflows.find(item => item.id === made.id).run + const planner = run.ephemeral.plan + assert.equal(run.nodeStatus.plan, 'running') + + readOnly = true + await svc.stepResult({ botId: planner, ok: true, text: 'The plan is ready.' }) + const after = svc.state().workflows.find(item => item.id === made.id).run + assert.equal(after.status, 'running') + assert.equal(after.nodeStatus.plan, 'running') + assert.equal(after.doneCount, run.doneCount) + assert.match(warnings.join('\n'), /read-only/) + + // The same step replies again: still refused, and the warning is not repeated. + await svc.stepResult({ botId: planner, ok: true, text: 'The plan is ready.' }) + assert.equal(warnings.filter(line => line.includes('read-only')).length, 1) +}) + // #1388: reap is the only path that cleans up a run whose host went away, and the // plugin calls it exactly once, at load. A run still marked `running` at that // moment is what a restart leaves behind: nothing resumes it, so its ephemeral