Add the pure handoff pool contract and its tests, not wired

core/test/task-handoff-route.test.js

@@ -7,6 +7,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
+import { validateHandoff } from '../../plugins/workflow/src/pool-contract.js'
 import { createRuntime } from '../index.js'
 import { fakeContext } from './fake-ctx.js'
 
@@ -244,3 +245,41 @@ test('a cancelled rework hands its original back to the lease recovery (#1804)',
   await app.drive()
   assert.match(app.text('r1'), new RegExp(`Take ${id} `), 'the Reviewer is offered the card again')
 })
+
+// #1853: the pool contract's first two checks read the same board facts as the host's route
+// (`core/src/host/tasks.js:441-442`, `:494-501`), so the two policies cannot drift apart in
+// silence. The contract's roles, pools and exec owner are taken from the live board here, not
+// written down a second time.
+test('the pool contract and the host route reach the same target and owner verdict (#1853)', async (t) => {
+  const app = await start(t)
+  const bots = Object.values(app.rt.state.bots)
+  const roles = [...new Set(bots.map(bot => bot.role))]
+  const context = (over = {}) => ({
+    roles,
+    pools: Object.fromEntries(roles.map(role => [role, { workers: 1 }])),
+    execOwner: bots.find(bot => bot.id === 'd1').ownerId,
+    worker: { wip: 0 },
+    pool: { inflight: 0, workers: 1 },
+    seen: new Set(),
+    ...over,
+  })
+
+  // 1. target — the host refuses a role no Bot carries, and the contract refuses the same role.
+  const refused = await app.track('a', { tasks: [{ title: 'Ghost handoff', handoff: { from: 'Lead', to: 'Ghost', reason: 'x' } }] })
+  assert.match(refused, /unknown-role/)
+  assert.equal(validateHandoff({ card: 'task-x', fromRole: 'Lead', toRole: 'Ghost', reason: 'x' }, context()).code, 'unknown-role')
+  assert.equal(validateHandoff({ card: 'task-x', fromRole: 'Ghost', toRole: 'Dev', reason: 'x' }, context()).ok, true, 'the contract checks the target role, as the host does, and not the producer')
+
+  // ... and the same acceptance: the pool the host routes the card to is a pool the contract takes.
+  const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev', reason: 'first slice' } }] }))
+  const routed = app.rt.state.tasks[id].queueRole
+  assert.equal(routed, 'Dev')
+  assert.equal(validateHandoff({ card: id, fromRole: 'Lead', toRole: routed, reason: 'first slice' }, context()).ok, true)
+
+  // 2. owner — the writing Bot's owner is the exec's only owner; another owner is refused.
+  const dev = bots.find(bot => bot.id === 'd1')
+  assert.equal(validateHandoff({ card: id, fromRole: 'Dev', toRole: 'Reviewer', reason: 'diff ready', owner: dev.ownerId }, context()).ok, true)
+  const notOwner = validateHandoff({ card: id, fromRole: 'Dev', toRole: 'Reviewer', reason: 'diff ready', owner: 'owner-2' }, context())
+  assert.equal(notOwner.code, 'not-owner')
+  assert.equal(dev.ownerId, context().execOwner, 'the board itself gives the exec its one owner')
+})

docs/subsystems/workflow.md

@@ -66,6 +66,18 @@ A core board task (`state.tasks`, `core/src/host/tasks.js`) is the only task ide
 
 Core never reads `$DSH_HOME/bot/flybot-workflow`. The plugin registers `flybot.tasks.onProjectProvider(fn)`; the host calls it with `hasTask(taskId)` and the plugin answers `{ taskId, runId, nodeId? }` from `service.projectTasks()` — one entry per live run that holds a card, `nodeId` being the running node (else the last node handed the card). A run whose card is gone is dropped by the plugin before it can become a row. The aggregate writes nothing: no card, claim or run changes when the board is read, so an install with no Projects, and every caller that never asks, behaves exactly as before.
 
+## Pool contract (#1853)
+
+`plugins/workflow/src/pool-contract.js` is the typed handoff contract of #1550 Stage 3 P0: one pure
+`validateHandoff(handoff, ctx)` running the seven checks in order — target, owner, evidence,
+concurrency, idempotency, cycle, quota — plus `escalationTarget(action)`, which names the Chief for
+the two escalating refusals (`cycle-budget`, `quota`) and nobody else. It does no I/O, imports no
+`service.js`, and **no dispatch path calls it**: no endpoint, tool, run, `index.js` or `service.js`
+mentions it, so loading the plugin behaves exactly as before. P1 is the step that will call it;
+until then `plugins/workflow/test/pool-contract.test.js` is its only caller, and
+`core/test/task-handoff-route.test.js` feeds the host's own board facts to its target and owner
+checks so the two policies cannot drift apart silently.
+
 ## Tests
 
 `plugins/workflow/test/`: graph advance and join, graph edit, assign and persistence, the FIFO queue and concurrency cap, Bot-reply advance, a dispatch that fails without being marked handed, a bounded hand-off retry and a step deadline that fails the run and clears its timer (#1530), a per-workflow `stepTimeoutMinutes` that replaces the ten-minute constant and is refused outside 1..120 (#1580), disposing releases the Bots before the record drops their ids and a released runner name is reusable again, assign-to-bot versus assign-to-workflow, inputs and outputs along edges, a loop that breaks and an if/else branch, failure recovery (onFail edges with max attempts, exhaustion behavior, and compatibility with failOnce and loop edges), templates, `define` / `runNamed`, the matcher, ephemeral spawn and cleanup, a stopped run with its settled node map (#1424), the reply middleware, the client canvas, the settings page (including a condition and a loop), rename and delete (including factory-id rows such as `delivery`), the stage chat's scroll rule (a new visible line follows, a filtered node reply moves nothing), the assign dialog's preselected card and its failed and empty board states (#1457, #1458), a sidebar click that leaves the Workflows section in place, and the claim rules — a re-routed card moves with one claim and one route, the same target writes nothing, a trigger never invalidates, the held list is capped and omits a Bot claim, `rematch: true` drops one claim, and the panel's one re-match action goes through `ingest` (#1647). Run `npm test` in `plugins/workflow`. The host seam that applies `ownerBotId` is `core/test/task-route.test.js`, which also pins that a card written through `track_tasks` reaches the listener (#1462). The host omission of run Bots, and the grant of `stepTools` plus the hidden-approval answer, is `core/test/flybot.test.js`. Which connectors a run Bot may call is `core/test/connector-access.test.js`, and the chain from a node's `tools` through `spawn` to that guard — the built-in delivery nodes included — is `core/test/workflow-node-connectors.test.js`.

patches/ds-bot/files/test/task-handoff-route.test.js

@@ -7,6 +7,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
+import { validateHandoff } from '../../plugins/workflow/src/pool-contract.js'
 import { createRuntime } from '../index.js'
 import { fakeContext } from './fake-ctx.js'
 
@@ -244,3 +245,41 @@ test('a cancelled rework hands its original back to the lease recovery (#1804)',
   await app.drive()
   assert.match(app.text('r1'), new RegExp(`Take ${id} `), 'the Reviewer is offered the card again')
 })
+
+// #1853: the pool contract's first two checks read the same board facts as the host's route
+// (`core/src/host/tasks.js:441-442`, `:494-501`), so the two policies cannot drift apart in
+// silence. The contract's roles, pools and exec owner are taken from the live board here, not
+// written down a second time.
+test('the pool contract and the host route reach the same target and owner verdict (#1853)', async (t) => {
+  const app = await start(t)
+  const bots = Object.values(app.rt.state.bots)
+  const roles = [...new Set(bots.map(bot => bot.role))]
+  const context = (over = {}) => ({
+    roles,
+    pools: Object.fromEntries(roles.map(role => [role, { workers: 1 }])),
+    execOwner: bots.find(bot => bot.id === 'd1').ownerId,
+    worker: { wip: 0 },
+    pool: { inflight: 0, workers: 1 },
+    seen: new Set(),
+    ...over,
+  })
+
+  // 1. target — the host refuses a role no Bot carries, and the contract refuses the same role.
+  const refused = await app.track('a', { tasks: [{ title: 'Ghost handoff', handoff: { from: 'Lead', to: 'Ghost', reason: 'x' } }] })
+  assert.match(refused, /unknown-role/)
+  assert.equal(validateHandoff({ card: 'task-x', fromRole: 'Lead', toRole: 'Ghost', reason: 'x' }, context()).code, 'unknown-role')
+  assert.equal(validateHandoff({ card: 'task-x', fromRole: 'Ghost', toRole: 'Dev', reason: 'x' }, context()).ok, true, 'the contract checks the target role, as the host does, and not the producer')
+
+  // ... and the same acceptance: the pool the host routes the card to is a pool the contract takes.
+  const id = app.idOf(await app.track('a', { tasks: [{ title: 'Build the route', handoff: { from: 'Lead', to: 'Dev', reason: 'first slice' } }] }))
+  const routed = app.rt.state.tasks[id].queueRole
+  assert.equal(routed, 'Dev')
+  assert.equal(validateHandoff({ card: id, fromRole: 'Lead', toRole: routed, reason: 'first slice' }, context()).ok, true)
+
+  // 2. owner — the writing Bot's owner is the exec's only owner; another owner is refused.
+  const dev = bots.find(bot => bot.id === 'd1')
+  assert.equal(validateHandoff({ card: id, fromRole: 'Dev', toRole: 'Reviewer', reason: 'diff ready', owner: dev.ownerId }, context()).ok, true)
+  const notOwner = validateHandoff({ card: id, fromRole: 'Dev', toRole: 'Reviewer', reason: 'diff ready', owner: 'owner-2' }, context())
+  assert.equal(notOwner.code, 'not-owner')
+  assert.equal(dev.ownerId, context().execOwner, 'the board itself gives the exec its one owner')
+})

plugins/workflow/src/pool-contract.js

@@ -0,0 +1,140 @@
+// The handoff contract of #1550 Stage 3 P0 (#1853): the seven validations of a handoff, in one
+// order, as a pure decision. Nothing in the plugin imports this file yet — no endpoint, tool, run
+// or dispatch path calls it — so loading the plugin behaves exactly as it did before. P1 wires it
+// into dispatch; this module keeps the policy out of `service.js` until then.
+import { createHash } from 'node:crypto'
+
+const DEFAULT_CYCLE_MAX = 3
+const QUOTA_UNITS = ['steps', 'ms', 'tokens']
+const ESCALATING = ['cycle-budget', 'quota']
+
+const isObject = (value) => value !== null && typeof value === 'object' && !Array.isArray(value)
+const text = (value) => (value === undefined || value === null ? '' : String(value))
+
+// The key is derived from four named facts and never from the object's shape, so re-ordering the
+// handoff's keys cannot mint a second card, while a new `attempt` is a new handoff on purpose.
+export function handoffKey(handoff = {}) {
+  const parts = [handoff.card, handoff.fromRole, handoff.toRole, handoff.attempt].map(text)
+  return createHash('sha256').update(parts.join('|')).digest('hex')
+}
+
+const roleNames = (roles) => {
+  const list = roles instanceof Set ? [...roles] : Array.isArray(roles) ? roles : []
+  return list.map((role) => (typeof role === 'string' ? role : role?.role ?? role?.name ?? role?.id)).filter((name) => typeof name === 'string' && name !== '')
+}
+
+const hasPool = (pools, role) => {
+  if (pools instanceof Map) return pools.has(role)
+  if (Array.isArray(pools)) return pools.some((pool) => pool === role || pool?.role === role)
+  return isObject(pools) && Object.prototype.hasOwnProperty.call(pools, role) && pools[role] !== undefined && pools[role] !== null
+}
+
+const seenBefore = (seen, key) => {
+  if (seen instanceof Set) return seen.has(key)
+  if (seen instanceof Map) return seen.has(key)
+  if (Array.isArray(seen)) return seen.includes(key)
+  return isObject(seen) && Object.prototype.hasOwnProperty.call(seen, key)
+}
+
+const recordKey = (seen, key) => {
+  if (seen instanceof Set) seen.add(key)
+  else if (seen instanceof Map) seen.set(key, true)
+  else if (isObject(seen)) seen[key] = true
+}
+
+const passOf = (entry) => {
+  if (!isObject(entry)) return undefined
+  if (isObject(entry.pass)) return entry.pass
+  if (entry.kind === 'pass' || (entry.tip !== undefined && entry.pr === undefined)) return entry
+  return undefined
+}
+
+const ranLines = (value) => {
+  const list = Array.isArray(value) ? value : [value]
+  return list.map((line) => text(line).trim()).filter((line) => line !== '')
+}
+
+// Check 3: every item `acceptance.requires` names is backed by evidence, and a held PR never
+// enters the merge pool. The refusal names the missing item, so the producer knows what to add.
+function missingEvidence(handoff) {
+  const requires = Array.isArray(handoff.acceptance?.requires) ? handoff.acceptance.requires : []
+  if (requires.length === 0) return undefined
+  const evidence = Array.isArray(handoff.evidence) ? handoff.evidence : []
+  const passes = evidence.map(passOf).filter(Boolean)
+  const prs = evidence.filter((entry) => isObject(entry) && entry.pr !== undefined && entry.pr !== null)
+  for (const requirement of requires) {
+    if (!isObject(requirement)) return `acceptance.requires carries an item the contract cannot read: ${text(requirement)}`
+    if (isObject(requirement.pass)) {
+      const tip = text(requirement.pass.tip)
+      const found = passes.find((entry) => text(entry.tip) === tip)
+      if (found === undefined) return `no PASS names the tip ${tip || '(none)'}`
+      if (ranLines(found.ran).length === 0) return `the PASS at ${tip || '(none)'} carries no Ran: line`
+      continue
+    }
+    if (requirement.pr === undefined || requirement.pr === null) return `acceptance.requires carries an item the contract cannot read: ${JSON.stringify(requirement)}`
+    const wanted = Number(requirement.pr)
+    const found = prs.find((entry) => Number(entry.pr) === wanted)
+    if (found === undefined) return `no PR ${wanted}`
+    if (found.hold === true) return `PR ${wanted} carries hold, so it may not enter the merge pool`
+    if (text(requirement.checks) === 'tests-ok' && text(found.checks) !== 'tests-ok') return `PR ${wanted} carries no green tests-ok`
+  }
+  return undefined
+}
+
+const refusal = (code, reason) => ({ ok: false, code, reason, ...(ESCALATING.includes(code) ? { escalate: true } : {}) })
+
+// The seven checks, in this order, each refusing before the next one reads: target, owner,
+// evidence, concurrency, idempotency, cycle, quota.
+export function validateHandoff(handoff = {}, ctx = {}) {
+  const card = handoff.card
+  const fromRole = text(handoff.fromRole)
+  const toRole = text(handoff.toRole).trim()
+
+  // 1. target — the workflow declares the role and the role has a pool.
+  if (!roleNames(ctx.roles).includes(toRole)) return refusal('unknown-role', `toRole "${toRole}" is not a role the workflow declares`)
+  if (!hasPool(ctx.pools, toRole)) return refusal('no-pool', `the role "${toRole}" has no pool`)
+
+  // 2. owner — the exec writes for exactly one owner.
+  const execOwner = text(ctx.execOwner).trim()
+  if (execOwner === '') return refusal('not-owner', 'the exec names no owner, so no handoff can be written on its behalf')
+  const statedOwner = handoff.owner ?? ctx.owner
+  if (statedOwner !== undefined && statedOwner !== null && text(statedOwner) !== execOwner) return refusal('not-owner', `the handoff is written on behalf of "${text(statedOwner)}", but the exec belongs to "${execOwner}"`)
+
+  // 3. evidence — the acceptance gate and the hold rule.
+  const missing = missingEvidence(handoff)
+  if (missing !== undefined) return refusal('missing-evidence', missing)
+
+  // 4. concurrency — one card per worker, and the pool must have room.
+  const wip = ctx.worker?.wip
+  if (wip !== undefined && wip !== null && Number.isFinite(Number(wip)) && Number(wip) > 1) return refusal('wip-limit', `the worker already holds ${Number(wip)} cards; a handoff may add one only when it holds at most one`)
+  const inflight = ctx.pool?.inflight
+  const workers = ctx.pool?.workers
+  if (Number.isFinite(Number(inflight)) && Number.isFinite(Number(workers)) && Number(inflight) >= Number(workers)) return refusal('pool-full', `the pool for "${toRole}" has ${Number(inflight)} of ${Number(workers)} in flight`)
+
+  // 5. idempotency — one key, accepted once. A duplicate is a success that records nothing.
+  const key = text(handoff.idempotencyKey) !== '' ? text(handoff.idempotencyKey) : handoffKey({ card, fromRole, toRole, attempt: handoff.attempt })
+  if (seenBefore(ctx.seen, key)) return { ok: true, duplicate: true, key }
+
+  // 6. cycle — the per-edge attempt budget, and a used-up budget asks the Chief.
+  const cycleMax = ctx.cycleMax === undefined || ctx.cycleMax === null ? DEFAULT_CYCLE_MAX : Number(ctx.cycleMax)
+  if (Number.isFinite(cycleMax) && Number(handoff.attempt ?? 0) > cycleMax) return refusal('cycle-budget', `attempt ${Number(handoff.attempt ?? 0)} is over the edge budget of ${cycleMax}`)
+
+  // 7. quota — a spent limit is a refusal the Chief decides on, never a silent stop.
+  const quota = ctx.quota ?? {}
+  const used = ctx.used ?? {}
+  for (const unit of QUOTA_UNITS) {
+    const limit = quota[unit]
+    if (limit === undefined || limit === null || !Number.isFinite(Number(limit))) continue
+    if (Number(used[unit] ?? 0) > Number(limit)) return refusal('quota', `${unit} used ${Number(used[unit] ?? 0)} of ${Number(limit)}`)
+  }
+
+  recordKey(ctx.seen, key)
+  return { ok: true, key }
+}
+
+// Only the two escalating refusals ask the Chief, and only the Chief decides whether a meeting
+// opens: nothing auto-escalates to a meeting (owner decision, #1550 Stage 3).
+export function escalationTarget(action) {
+  const code = typeof action === 'string' ? action : action?.code
+  return ESCALATING.includes(code) ? 'chief' : null
+}