diff --git a/packages/bus/README.md b/packages/bus/README.md index c5ca68ad7..690eb8e36 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -116,7 +116,7 @@ an operator inspect content and delivery evidence after an uncertain result. | `role.claim` | No args; claims capability's role/run. One current holder per business/instance; a revoked run cannot reclaim that role, including after restart. | | `role.release` | No args; only its current holder releases. | | `role.revoke` | `{role,decision}`; needs human-approved gated decision for this exact target and holder incarnation. | -| `decision.raise` | `{action,question,options:[{key,text}],recommendation,blocking,domain?,target?,project?,task_ref?,requirement_ref?,supersedes?,choice?,approvalChoice?}`. Returns decision with decoded options and authorization context. | +| `decision.raise` | `{action,question,options:[{key,text}],recommendation,blocking,domain?,target?,project?,task_ref?,requirement_ref?,supersedes?,choice?,approvalChoice?}`. Returns decision with decoded options and authorization context. A blocking decision holds a task: `blocking` true with no `task_ref` refuses `task-required`. A non-blocking one needs no task, and a gated action such as `prd.approve` or `role.revoke` still routes to the human's inbox. | | `decision.resolve` | `{id,choice,note?}`; option must exist, resolver must be routed role or outside-agent human CLI. | | `decision.seen` | `{id,note?}`; acknowledgement by resolver, not a resolution. | | `decision.withdraw` | `{id,note?}`; raiser or human. Never rewrites the decision. | @@ -131,6 +131,7 @@ an operator inspect content and delivery evidence after an uncertain result. | `trail` | `{subject}`: task ref, decision UUID or message UUID. Linked records in broker write order. | | `decisions` | `{list}`: `roles` or `recent`; human and reader sessions only. Open decisions routed to a role, or the last 50 closed. | | `credentials` | No args; human and reader sessions only. `[{service,account,role,dateKind,date,state}]` for the capability's business, never a token, file path or variable name. | +| `requirements` | No args; role sessions only, as `task.create` is (row 90, #1584). The approved PRD's requirement ids, `{prd,version,requirements:[{id,title}]}`, from `packages/tasks/requirements.json`, loaded when the broker starts; a bad file refuses the start. A human session refuses `agent-required`, a reader `read-only`. | | `replies` | `{id}`: a message UUID; human and reader sessions only. The messages whose `in_reply_to` is that id, oldest first, at most 50. Writes nothing. | ### Agent state (row 59, #1548) diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 4a3e52a48..5e211eefb 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -1,4 +1,5 @@ import { randomUUID, randomBytes } from 'node:crypto'; +import { loadRequirements } from '../../tasks/src/requirements.mjs'; // Kept closed while S1 lands; its vocabulary is the integration authority. export const ACTIONS = Object.freeze([ @@ -93,6 +94,7 @@ export class Broker { #ended = new Set(); #secretCheck; #credentialStatus; + #requirements; #clock = 0; // credentialStatus is the runtime's metadata-only Credentials.status(); the broker never gets use(). constructor({ store, businesses, secretCheck = () => {}, credentialStatus = () => [] }) { @@ -101,6 +103,8 @@ export class Broker { this.#store = store; this.#secretCheck = secretCheck; this.#credentialStatus = credentialStatus; + // Row 90 (#1584): the approved PRD's requirement ids. A bad requirements.json refuses here. + this.#requirements = loadRequirements(); for (const table of [ 'events', 'role_claims', @@ -894,6 +898,14 @@ export class Broker { ), }; } + // Row 90 (#1584): the approved PRD's requirement ids, which task.create takes. A read for + // every session that can create tasks: task.create is an agent verb, so a human refuses + // agent-required here as there, and a reader read-only. + case 'requirements': { + keys(a, []); + this.#agent(s); + return this.#requirements; + } case 'inbox': case 'agents': case 'tasks': diff --git a/packages/bus/tests/requirements.test.mjs b/packages/bus/tests/requirements.test.mjs new file mode 100644 index 000000000..5aa39cede --- /dev/null +++ b/packages/bus/tests/requirements.test.mjs @@ -0,0 +1,78 @@ +// Row 90 (#1584): `requirements`, the approved PRD's ids that task.create takes, for the role +// sessions that create tasks; and the PM's two no-task decisions, a request no id fits +// (prd.approve) and a run it thinks should stop (role.revoke), which reach the human's Inbox. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { Store } from '../src/store.mjs'; +import { Broker } from '../src/broker.mjs'; +import { loadRequirements } from '../../tasks/src/requirements.mjs'; + +const options = [ + { key: 'yes', text: 'Approve' }, + { key: 'no', text: 'Decline' }, +]; +const roles = { + pm: { authority: { withinRole: ['message.send', 'task.create'], crossRole: [] } }, + coder: { authority: { withinRole: ['message.send'], crossRole: [] } }, +}; +const businesses = { demo: { id: 'demo', human: 'jason', arbiters: { technical: 'pm', delivery: 'pm' }, roles } }; +function setup(t) { + const root = mkdtempSync(join(tmpdir(), 'bus-requirements-')); + const store = new Store(root); + const b = new Broker({ store, businesses }); + t.after(() => { + store.close(); + rmSync(root, { recursive: true, force: true }); + }); + const agent = (role) => { + const cap = b.bindLaunch({ business: 'demo', role, run: `demo-${role}-run`, harness: 'pi', address: role }); + b.request(cap, { verb: 'role.claim' }); + return cap; + }; + const human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true }); + const reader = b.bindReader({ business: 'demo' }); + return { b, agent, human, reader }; +} +const call = (b, cap, verb, args = {}) => b.request(cap, { verb, args }); +const raise = (b, cap, action, extra = {}) => + call(b, cap, 'decision.raise', { action, question: `${action}?`, options, recommendation: 'no', blocking: false, ...extra }); + +test('requirements returns the committed list to a holding role session, and no one else', (t) => { + const { b, agent, human, reader } = setup(t); + const pm = agent('pm'); + const list = call(b, pm, 'requirements'); + assert.deepEqual(list, loadRequirements()); + assert.equal(list.requirements.length, 24); + assert.deepEqual(call(b, agent('coder'), 'requirements'), list); + assert.throws(() => call(b, human, 'requirements'), /agent-required/); + assert.throws(() => call(b, reader, 'requirements'), /read-only/); + // A second launch of the role that never claimed it holds nothing. + const stale = b.bindLaunch({ business: 'demo', role: 'pm', run: 'demo-pm-other', harness: 'pi', address: 'pm2' }); + assert.throws(() => call(b, stale, 'requirements'), /not-holder/); + assert.throws(() => call(b, pm, 'requirements', { prd: 'docs/prd/mosaic-stack.md' }), /invalid-request/); +}); + +test("the PM's prd.approve with no task, when no id fits, reaches the human's Inbox", (t) => { + const { b, agent, human, reader } = setup(t); + const pm = agent('pm'); + const d = raise(b, pm, 'prd.approve', { target: 'mosaic-stack-prd' }); + assert.deepEqual([d.class, d.route_to, d.blocking, d.task_ref], ['gated', 'human', false, null]); + for (const cap of [human, reader]) assert.deepEqual(call(b, cap, 'inbox').map((r) => r.id), [d.id]); + assert.deepEqual(call(b, pm, 'inbox'), []); + // Blocking needs a task to hold. + assert.throws(() => raise(b, pm, 'prd.approve', { blocking: true }), /task-required/); +}); + +test("the PM's non-blocking role.revoke with no task reaches the human's Inbox", (t) => { + const { b, agent, human } = setup(t); + const pm = agent('pm'); + agent('coder'); + const d = raise(b, pm, 'role.revoke', { target: 'coder' }); + assert.deepEqual([d.class, d.route_to, d.blocking, d.task_ref], ['gated', 'human', false, null]); + assert.deepEqual(d.authorization, { action: 'role.revoke', target: 'coder', approvalChoice: 'yes' }); + assert.deepEqual(call(b, human, 'inbox').map((r) => r.id), [d.id]); + assert.throws(() => raise(b, pm, 'role.revoke', { target: 'coder', blocking: true }), /task-required/); +}); diff --git a/packages/harness/src/bundle.mjs b/packages/harness/src/bundle.mjs index 9326be87f..63aa4d049 100644 --- a/packages/harness/src/bundle.mjs +++ b/packages/harness/src/bundle.mjs @@ -69,6 +69,8 @@ function prompt({ contract, resolved, session, tools, untyped }) { `- Bus tools: ${tools.map((t) => t.name).join(", ")}. The broker decides every call; a refusal names its reason.`, "- Each message to you arrives as one turn. Your final answer goes back to the sender as the reply, so end each turn with the answer itself.", "- You never hold a token. You act on the tracker, the forge and other roles only through the bus tools.", + // Row 90 (#1584): PM fault 10 (#228 c91). + "- Only the human stops a run, with `mosaic stop`. If you think a run should stop, raise a role.revoke decision with blocking false and the role instance as target, naming the run and saying why.", ]; if (untyped.length) lines.push(`- Actions you hold that have no bus tool yet: ${untyped.join(", ")}. Say so instead of working around it.`); return lines.join("\n") + "\n"; diff --git a/packages/harness/src/runner.mjs b/packages/harness/src/runner.mjs index ccf6ca43e..c8943244b 100644 --- a/packages/harness/src/runner.mjs +++ b/packages/harness/src/runner.mjs @@ -63,6 +63,8 @@ export function turnRequest(m) { const head = [`Message ${m.id} from ${m.from_role}`, `class ${m.class}`]; if (m.in_reply_to) head.push(`in reply to ${m.in_reply_to}`); if (m.decision) head.push(`citing decision ${m.decision}`); + // Row 90 (#1584): the human input id task_create cites as its request. + if (m.request) head.push(`request ${m.request}`); return `${head.join(", ")}:\n\n${m.body}`; } diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs index eed0818ad..a568a8082 100644 --- a/packages/harness/src/tools.mjs +++ b/packages/harness/src/tools.mjs @@ -44,8 +44,8 @@ const TASK_TOOLS = { priority: PRIORITY, labels: LABEL_IDS, relations: { type: "array", items: RELATION, maxItems: 50 }, - request: str("Id of the message or human input this task answers"), - requirement: str("Requirement id, such as REQ-CRED-2", { pattern: "^REQ-[A-Z]+-[1-9][0-9]*$" }), + request: str("The request id of the human message this task answers, shown as \"request \" in that message's header; not the message's own id"), + requirement: str("Requirement id from list_requirements, such as REQ-CRED-2", { pattern: "^REQ-[A-Z]+-[1-9][0-9]*$" }), decision: COMMON.decision, }, ["title", "request", "requirement"]], "task.assign": ["task_assign", "Assign an unassigned task to a role.", { ...COMMON, role: str("Role instance to assign") }, ["task_ref", "role"]], @@ -103,7 +103,7 @@ export function typedTools({ resolved, arbiter = false, tracker = false }) { question: str("The question, with enough context to answer it"), options: { type: "array", minItems: 2, maxItems: 9, items: obj({ key: str("Short key"), text: str("What this option means") }, ["key", "text"]) }, recommendation: str("Your recommended option key, with a reason"), - blocking: { type: "boolean", description: "True when you can't continue without the answer" }, + blocking: { type: "boolean", description: "True only for a decision that holds a task: it needs task_ref, and the task waits for the answer. A request with no task is false" }, target: str("What the action applies to, such as a role instance or task reference"), approvalChoice: str("The option key that approves the action"), task_ref: TASK_REF, @@ -129,6 +129,9 @@ export function typedTools({ resolved, arbiter = false, tracker = false }) { after: str("The next value of the previous page"), })); add("get_task", "tasks", null, "Read one tracker task in full, description included.", obj({ task_ref: TASK_REF }, ["task_ref"])); + // Row 90 (#1584): the ids task_create takes, for the sessions that hold it. + if (held.has("task.create")) + add("list_requirements", "requirements", null, "List the approved PRD's requirement ids, each with a one-line title. task_create takes only these. When none fits a request, raise a prd.approve decision instead of creating the task.", obj({})); } return tools; } diff --git a/packages/harness/tests/bundle.test.mjs b/packages/harness/tests/bundle.test.mjs index 98cee8ed4..aae81c524 100644 --- a/packages/harness/tests/bundle.test.mjs +++ b/packages/harness/tests/bundle.test.mjs @@ -43,6 +43,8 @@ test("a pi bundle: prompt, policy, tools and manifest, 0600 in a 0700 directory" assert.match(prompt, /^# Contract\n\nDo the work\.\n\n## This session\n/); assert.match(prompt, /Role instance: pm \(definition pm\)\. Run: run-1\./); assert.match(prompt, /Bus tools: send_message, launch, /); + // Row 90 (#1584): PM fault 10 (#228 c91). + assert.match(prompt, /^- Only the human stops a run, with `mosaic stop`\. If you think a run should stop, raise a role\.revoke decision with blocking false/m); assert.deepEqual(JSON.parse(readFileSync(b.files.policy, "utf8")), { harness: "pi", workspace: b.workspace, diff --git a/packages/harness/tests/runner.test.mjs b/packages/harness/tests/runner.test.mjs index 25b4d8a93..777d16148 100644 --- a/packages/harness/tests/runner.test.mjs +++ b/packages/harness/tests/runner.test.mjs @@ -48,6 +48,12 @@ test("turnRequest names the sender, class, reply and decision", () => { turnRequest({ id: "m2", from_role: "cto", class: "RESULT", in_reply_to: "m1", decision: "d1", body: "b" }), "Message m2 from cto, class RESULT, in reply to m1, citing decision d1:\n\nb", ); + // Row 90 (#1584): a human message's request id, which task_create cites; none for a role's. + assert.equal( + turnRequest({ id: "m3", from_role: "jason", class: "REQUEST", request: "e7", body: "b" }), + "Message m3 from jason, class REQUEST, request e7:\n\nb", + ); + assert.equal(turnRequest({ id: "m4", from_role: "cto", class: "RESULT", request: null, body: "b" }), "Message m4 from cto, class RESULT:\n\nb"); }); async function setup(t, { session = {}, policy = {}, tools } = {}) { diff --git a/packages/harness/tests/tools.test.mjs b/packages/harness/tests/tools.test.mjs index f6fca2794..c8fdfe467 100644 --- a/packages/harness/tests/tools.test.mjs +++ b/packages/harness/tests/tools.test.mjs @@ -13,7 +13,7 @@ test("the PM gets launch, its task verbs and the reads", (t) => { "send_message", "launch", "task_create", "task_assign", "task_reassign", "task_schedule", "task_priority_change", "task_scope_change", "task_update_assigned", "task_close", - "raise_decision", "resolve_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task", + "raise_decision", "resolve_decision", "list_agents", "inbox", "trail", "list_tasks", "get_task", "list_requirements", ]); const launch = tools.find((x) => x.name === "launch"); assert.equal(launch.verb, "launch"); @@ -31,6 +31,27 @@ test("the PM gets launch, its task verbs and the reads", (t) => { } }); +// Row 90 (#1584): list_requirements for the sessions that hold task.create, with a tracker. +test("list_requirements reads the requirement ids, for a task.create holder with a tracker", (t) => { + const pm = resolvedFor(t, "pm"); + const tools = typedTools({ resolved: pm, arbiter: true, tracker: true }); + const list = tools.find((x) => x.name === "list_requirements"); + assert.deepEqual([list.verb, list.action], ["requirements", null]); + assert.deepEqual(list.parameters.properties, {}); + assert.match(list.description, /task_create takes only these/); + assert.match(list.description, /raise a prd\.approve decision instead of creating the task/); + assert.ok(!names(typedTools({ resolved: pm, arbiter: true })).includes("list_requirements")); + const noCreate = resolvedFor(t, "pm", (d) => (d.roles.pm.vars["limits.authority"] = ["message.send", "task.assign"])); + const without = names(typedTools({ resolved: noCreate, tracker: true })); + assert.ok(without.includes("task_assign") && !without.includes("task_create") && !without.includes("list_requirements")); + const create = tools.find((x) => x.name === "task_create").parameters.properties; + assert.match(create.requirement.description, /from list_requirements/); + assert.match(create.request.description, /shown as "request " in that message's header; not the message's own id/); + const raise = tools.find((x) => x.name === "raise_decision").parameters.properties; + assert.match(raise.blocking.description, /needs task_ref/); + assert.match(raise.blocking.description, /A request with no task is false/); +}); + test("a coder gets no launch, no resolve_decision, and no task tools without a tracker", (t) => { const coder = resolvedFor(t, "coder"); const tools = typedTools({ resolved: coder }); @@ -117,6 +138,6 @@ test("replyText: compact JSON up to the cap, an error naming the size and the ca return true; }); assert.throws(() => replyText("\u{1F600}".repeat(REPLY_CAP / 2)), /reply too large: 40002 characters/); - // 40000 units fit, though they are 80002 bytes: the cap isn't a byte count. + // 40000 units fit, though they are 79998 bytes: the cap isn't a byte count. assert.equal(replyText("\u{1F600}".repeat(REPLY_CAP / 2 - 1)).length, REPLY_CAP); }); diff --git a/packages/seat/src/sandbox.mjs b/packages/seat/src/sandbox.mjs index 21c938ced..88db8db1e 100644 --- a/packages/seat/src/sandbox.mjs +++ b/packages/seat/src/sandbox.mjs @@ -52,6 +52,7 @@ export const RUNNER_FILES = Object.freeze([ "packages/bus/src/client.mjs", "packages/bus/src/broker.mjs", "packages/business/src/vocabulary.mjs", + "packages/tasks/src/requirements.mjs", ].map((p) => join(REPO, p))); const SYSTEM_LINKS = ["/bin", "/lib", "/lib64", "/sbin"]; diff --git a/packages/tasks/README.md b/packages/tasks/README.md index 5b658bc23..f262ba339 100644 --- a/packages/tasks/README.md +++ b/packages/tasks/README.md @@ -133,6 +133,17 @@ decision isn't in it. That comes from the role's authority in `tests/fields.test.mjs` walks every verb against the fake and checks both. +`requirements.json` holds the approved PRD's requirement ids as data +(row 90, #1584): the PRD's path and version, and each id with a one-line +title, in the PRD's order. The PRD (`docs/prd/mosaic-stack.md`) stays +the reviewed source. `tests/requirements.test.mjs` reads its +`**REQ-...**` bullets and its status line, and fails unless the list +holds the same ids in the same order at the same version. A later PRD +version is a new list in a reviewed commit. `loadRequirements()` +(`src/requirements.mjs`) checks the file's shape, refuses any other and +returns it frozen; the adapter loads it when it starts, and the bus +loads it for its `requirements` read. Neither starts on a bad file. + A write that answers 5xx or doesn't answer is never retried. The adapter reads the task again. If the write took effect, it continues. If not, or the re-read fails too, the verb refuses with `write-uncertain`. A @@ -186,12 +197,15 @@ own write back as a person's edit. Details that aren't obvious from the table: - `task.create` needs `request`, the id of a `human.input` event of this - business (`request-not-found` otherwise), and a `requirement` like - `REQ-TASK-3`. `project` is the tracker project id to create the task - in, the root when left out. Any other project than an installed one of - the tree refuses with `project-not-installed`, and a value that isn't a - positive integer with `invalid-request`. The decision a create may - need is bound to no target, so it doesn't name the project. + business (`request-not-found` otherwise), and a `requirement` from + `requirements.json`, like `REQ-TASK-3`. An id in the right form that + the list doesn't hold refuses `unknown-requirement`. The `request` is + the `request` field `message.receive` gives a human's message, not the + message's own id. `project` is the tracker project id to create the + task in, the root when left out. Any other project than an installed + one of the tree refuses with `project-not-installed`, and a value that + isn't a positive integer with `invalid-request`. The decision a create + may need is bound to no target, so it doesn't name the project. `task.created` is recorded even when a label or relation write after the create fails. The task exists, and the verb then refuses with that write's code. @@ -376,7 +390,7 @@ on a live read with the sync key, and a switch is a follow-up row. | `task-moved`, `task-placement`, `task-project` | the task is in another project, or on no bucket | | `project-not-installed` | the project is watch-only, or not an installed project of the tree | | `task-conflict`, `task-done`, `task-assigned`, `task-unassigned`, `not-assigned` | the task's state doesn't allow the verb | -| `field-writer`, `label-not-allowed`, `unknown-role`, `invalid-state`, `request-not-found`, `invalid-request` | the call itself is wrong | +| `field-writer`, `label-not-allowed`, `unknown-role`, `invalid-state`, `request-not-found`, `unknown-requirement`, `invalid-request` | the call itself is wrong | Startup adds `credential-unavailable`, `credential-expired`, `tracker-version`, `tracker-project`, `tracker-install`, diff --git a/packages/tasks/requirements.json b/packages/tasks/requirements.json new file mode 100644 index 000000000..622d3a687 --- /dev/null +++ b/packages/tasks/requirements.json @@ -0,0 +1,30 @@ +{ + "prd": "docs/prd/mosaic-stack.md", + "version": "1.0", + "requirements": [ + { "id": "REQ-ROLE-1", "title": "Each role is a reviewed roles/.json: contract, authority map, credentials" }, + { "id": "REQ-ROLE-2", "title": "One holder per role instance; a claim locks and logs; revoking a holder is gated" }, + { "id": "REQ-ROLE-3", "title": "A user-written business file declares instances, arbiters and credential references" }, + { "id": "REQ-ROLE-4", "title": "The first business is Mosaic Stack: PM, CTO, coder and reviewer" }, + { "id": "REQ-VAR-1", "title": "Variables in four layers, declared in code; limits intersect; unknown keys refused" }, + { "id": "REQ-VAR-2", "title": "One system config, never auto-written; secrets only as references the broker reads" }, + { "id": "REQ-CRED-1", "title": "Each role instance has its own Gitea and Vikunja token, held by the broker" }, + { "id": "REQ-CRED-2", "title": "A session that finds only founder credentials stops" }, + { "id": "REQ-DEC-1", "title": "Every decision is an append-only record: raiser, class, options, resolver, times" }, + { "id": "REQ-DEC-2", "title": "Decisions route by class: logged, to the domain arbiter, or to the human" }, + { "id": "REQ-DEC-3", "title": "Human resolutions come only from a mosaic CLI session outside any agent run" }, + { "id": "REQ-DEC-4", "title": "A blocking gated decision reaches Jason at once; the rest go in a daily digest" }, + { "id": "REQ-MSG-1", "title": "Messages are addressed to a role, append-only, delivered to the current holder" }, + { "id": "REQ-TASK-1", "title": "Tasks live in Vikunja, each citing a requirement id; the verbs enforce field writers" }, + { "id": "REQ-TASK-2", "title": "Polling is the source of truth: a 30-second board poll and an hourly reconcile" }, + { "id": "REQ-TASK-3", "title": "The installer points at an existing Vikunja or deploys the unmodified upstream image" }, + { "id": "REQ-TASK-4", "title": "The queue keeps the agents' lock, review rounds and audit log; no two-way sync" }, + { "id": "REQ-CLI-1", "title": "mosaic is the front door: talk to the PM, the inbox, tasks, agents and trails" }, + { "id": "REQ-CLI-2", "title": "The PM session mosaic talks to is launched and owned by the stack, not T3" }, + { "id": "REQ-WEB-1", "title": "The WebUI shows the same data as the CLI" }, + { "id": "REQ-LAUNCH-1", "title": "The PM launches role sessions within business-file limits, logged and revocable" }, + { "id": "REQ-HARN-1", "title": "The meta-harness generates each session's launch bundle from role and variables" }, + { "id": "REQ-HARN-2", "title": "Credential scope, verbs, the tool limit and a container enforce the rules" }, + { "id": "REQ-EVT-1", "title": "Every slice 1 step emits an event; measures come from events" } + ] +} diff --git a/packages/tasks/src/requirements.mjs b/packages/tasks/src/requirements.mjs new file mode 100644 index 000000000..620070009 --- /dev/null +++ b/packages/tasks/src/requirements.mjs @@ -0,0 +1,53 @@ +import { readFileSync } from 'node:fs'; +// Row 90 (#1584): the approved PRD's requirement ids as data, in ../requirements.json. The PRD +// (docs/prd/mosaic-stack.md) stays the reviewed source; tests/requirements.test.mjs fails unless +// the list holds its **REQ-...** ids in its order. A later PRD version is a new list in a reviewed +// commit. task.create refuses an id the list doesn't hold, and the broker's `requirements` verb +// returns the list to the sessions that create tasks. This module imports nothing from the bus, +// so the broker can load it. This function is the file's schema: it refuses any other shape. +export const REQUIREMENTS_FILE = new URL('../requirements.json', import.meta.url); +export const REQUIREMENT_ID = /^REQ-[A-Z]+-[1-9][0-9]*$/; +export class RequirementsError extends Error {} +const plain = (x) => x !== null && typeof x === 'object' && !Array.isArray(x); +const str = (x, max) => typeof x === 'string' && x.trim() !== '' && x.length <= max; +function exact(x, where, allowed) { + if (!plain(x)) throw new RequirementsError(`${where}: not an object`); + for (const k of Object.keys(x)) if (!allowed.includes(k)) throw new RequirementsError(`${where}: unknown key ${k}`); + for (const k of allowed) if (!Object.hasOwn(x, k)) throw new RequirementsError(`${where}: missing ${k}`); +} +export function checkRequirements(list) { + exact(list, 'list', ['prd', 'version', 'requirements']); + if (!str(list.prd, 256) || !/^docs\/prd\/[a-z0-9-]+\.md$/.test(list.prd)) + throw new RequirementsError('prd: not a docs/prd/*.md path'); + if (typeof list.version !== 'string' || !/^[1-9][0-9]*\.[0-9]+$/.test(list.version)) + throw new RequirementsError('version: not a version such as 1.0'); + if (!Array.isArray(list.requirements) || !list.requirements.length) + throw new RequirementsError('requirements: not a nonempty list'); + const seen = new Set(); + for (const [i, r] of list.requirements.entries()) { + const at = `requirements[${i}]`; + exact(r, at, ['id', 'title']); + if (typeof r.id !== 'string' || !REQUIREMENT_ID.test(r.id)) throw new RequirementsError(`${at}.id: not a requirement id`); + if (seen.has(r.id)) throw new RequirementsError(`${at}.id: ${r.id} twice`); + seen.add(r.id); + if (!str(r.title, 120) || r.title.includes('\n')) throw new RequirementsError(`${at}.title: not one line of 120 characters or fewer`); + } + return list; +} +function freeze(x) { + if (x && typeof x === 'object') { + for (const v of Object.values(x)) freeze(v); + Object.freeze(x); + } + return x; +} +// The committed list, checked and frozen. A file that fails the check refuses to load. +export function loadRequirements(file = REQUIREMENTS_FILE) { + let list; + try { + list = JSON.parse(readFileSync(file, 'utf8')); + } catch (e) { + throw new RequirementsError(e instanceof SyntaxError ? 'requirements.json: not JSON' : `requirements.json: ${e.code ?? 'unreadable'}`); + } + return freeze(checkRequirements(list)); +} diff --git a/packages/tasks/src/verbs.mjs b/packages/tasks/src/verbs.mjs index 50efa0e72..cef5ae445 100644 --- a/packages/tasks/src/verbs.mjs +++ b/packages/tasks/src/verbs.mjs @@ -2,6 +2,7 @@ import { BusError } from '../../bus/src/broker.mjs'; import { outcome, refusal } from './vikunja.mjs'; import { ref, parseRef, checkTask, taskFields, digest } from './digest.mjs'; import { board, normal } from './sync.mjs'; +import { loadRequirements } from './requirements.mjs'; // The eight task verbs (addendum B section 4). Each one: field-table writer check, compare-read with // the sync token, the caller's expected digest, broker authority, the writes with the acting role's // own token, a final read, then one self snapshot and its event. A write that answers 5xx or not at @@ -64,6 +65,8 @@ const sorted = (a) => [...a].sort((x, y) => x - y); const decision = (x) => (x === undefined ? null : text(x, 128, 1)); export function verbs(ctx) { const fail = (code) => refuse(code); + // Row 90 (#1584): the approved PRD's ids. A bad requirements.json refuses here, at adapter start. + const requirements = new Set(loadRequirements().requirements.map((r) => r.id)); const label = (id) => { // Second guard after the bot-owner rule (probes.md, L): only label ids in the business file. if (!ctx.labels.has(id)) fail('label-not-allowed'); @@ -215,6 +218,7 @@ export function verbs(ctx) { plan.request = text(args.request, 64, 1); plan.requirement = text(args.requirement, 64); if (!/^REQ-[A-Z]+-[1-9][0-9]*$/.test(plan.requirement)) fail('invalid-request'); + if (!requirements.has(plan.requirement)) fail('unknown-requirement'); return plan; } case 'task.assign': diff --git a/packages/tasks/tests/adapter.test.mjs b/packages/tasks/tests/adapter.test.mjs index 3a4903e6c..e394cd704 100644 --- a/packages/tasks/tests/adapter.test.mjs +++ b/packages/tasks/tests/adapter.test.mjs @@ -181,9 +181,9 @@ test('close waits for a running verb and refuses one that has not started', asyn const pm = w.agent('pm'); let release; hold = new Promise((ok) => (release = ok)); - const running = w.call(pm, 'task.create', { title: 'x', request: w.instruction(), requirement: 'REQ-S-1' }); + const running = w.call(pm, 'task.create', { title: 'x', request: w.instruction(), requirement: 'REQ-TASK-1' }); await reached; - const queued = w.call(pm, 'task.create', { title: 'y', request: w.instruction(), requirement: 'REQ-S-1' }); + const queued = w.call(pm, 'task.create', { title: 'y', request: w.instruction(), requirement: 'REQ-TASK-1' }); let done = false; const closing = w.adapter.close().then(() => (done = true)); await sleep(20); @@ -193,7 +193,7 @@ test('close waits for a running verb and refuses one that has not started', asyn assert.ok((await running).task_ref); await assert.rejects(queued, (e) => e.code === 'tracker-closed'); await assert.rejects( - w.call(pm, 'task.create', { title: 'z', request: w.instruction(), requirement: 'REQ-S-1' }), + w.call(pm, 'task.create', { title: 'z', request: w.instruction(), requirement: 'REQ-TASK-1' }), (e) => e.code === 'tracker-closed', ); assert.equal(w.events('task.created').length, 1); diff --git a/packages/tasks/tests/fields.test.mjs b/packages/tasks/tests/fields.test.mjs index 597048bdc..59332d0fe 100644 --- a/packages/tasks/tests/fields.test.mjs +++ b/packages/tasks/tests/fields.test.mjs @@ -187,10 +187,10 @@ test('each verb refuses, asks for a decision and writes as the table and the rol } const others = ROLE_NAMES.filter((r) => r !== 'pm'); const create = { title: 'b', description: 'what', due_date: '2026-11-01T00:00:00.000Z', priority: 2, labels: [w.label] }; - for (const r of others) await attempt(r, 'task.create', { ...create, request: w.instruction(), requirement: 'REQ-S-1' }); - const a = await attempt('pm', 'task.create', { title: 'a', request: w.instruction(), requirement: 'REQ-S-1' }); + for (const r of others) await attempt(r, 'task.create', { ...create, request: w.instruction(), requirement: 'REQ-TASK-1' }); + const a = await attempt('pm', 'task.create', { title: 'a', request: w.instruction(), requirement: 'REQ-TASK-1' }); const relation = { kind: 'related', task_ref: a.task_ref }; - const { task_ref } = await attempt('pm', 'task.create', { ...create, relations: [relation], request: w.instruction(), requirement: 'REQ-S-1' }); + const { task_ref } = await attempt('pm', 'task.create', { ...create, relations: [relation], request: w.instruction(), requirement: 'REQ-TASK-1' }); for (const r of others) { await attempt(r, 'task.assign', { task_ref, role: 'coder' }); await attempt(r, 'task.reassign', { task_ref, role: 'coder' }); diff --git a/packages/tasks/tests/requirements.test.mjs b/packages/tasks/tests/requirements.test.mjs new file mode 100644 index 000000000..5e8f54530 --- /dev/null +++ b/packages/tasks/tests/requirements.test.mjs @@ -0,0 +1,81 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, readFileSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { loadRequirements, checkRequirements, RequirementsError, REQUIREMENTS_FILE } from '../src/requirements.mjs'; +// Row 90 (#1584): requirements.json against the approved PRD it names. The PRD's **REQ-...** +// bullets are the source; the list must hold the same ids in the same order, and the PRD's +// status line must give the list's version. +const list = loadRequirements(); +const raw = () => JSON.parse(readFileSync(REQUIREMENTS_FILE, 'utf8')); +const REPO = fileURLToPath(new URL('../../../', import.meta.url)); +const listError = (message) => (e) => e instanceof RequirementsError && (message === undefined || e.message === message); + +test("the list holds the PRD's requirement ids, in its order, at its version", () => { + const prd = readFileSync(join(REPO, list.prd), 'utf8'); + const ids = [...prd.matchAll(/^- \*\*(REQ-[A-Z]+-[0-9]+)\.\*\*/gm)].map((m) => m[1]); + // Any other mention of a bold id would be a bullet this pattern missed. + assert.equal(ids.length, [...prd.matchAll(/\*\*REQ-/g)].length); + assert.equal(ids.length, 24); + assert.deepEqual(list.requirements.map((r) => r.id), ids); + assert.match(prd, new RegExp(`^- Status: approved, version ${list.version.replace('.', '\\.')}\\.`, 'm')); + assert.deepEqual([list.prd, list.version], ['docs/prd/mosaic-stack.md', '1.0']); +}); + +test('the committed list loads frozen', () => { + assert.ok(Object.isFrozen(list) && Object.isFrozen(list.requirements) && Object.isFrozen(list.requirements[0])); + assert.deepEqual(checkRequirements(raw()), raw()); + assert.deepEqual(loadRequirements(), list); +}); + +test('the schema refuses any other shape', () => { + const bad = { + 'an unknown top key': (l) => (l.source = 'x'), + 'no prd': (l) => delete l.prd, + 'a prd outside docs/prd': (l) => (l.prd = 'docs/plans/x.md'), + 'a prd that is not markdown': (l) => (l.prd = 'docs/prd/mosaic-stack.txt'), + 'a prd with a parent step': (l) => (l.prd = 'docs/prd/../x.md'), + 'no version': (l) => delete l.version, + 'a numeric version': (l) => (l.version = 1.5), + 'a version with no minor': (l) => (l.version = '1'), + 'a version 0': (l) => (l.version = '0.4'), + 'no requirements': (l) => delete l.requirements, + 'an empty list': (l) => (l.requirements = []), + 'requirements not a list': (l) => (l.requirements = {}), + 'an entry not an object': (l) => (l.requirements[0] = 'REQ-ROLE-1'), + 'an unknown entry key': (l) => (l.requirements[0].note = 'x'), + 'no id': (l) => delete l.requirements[0].id, + 'no title': (l) => delete l.requirements[0].title, + 'an id in lower case': (l) => (l.requirements[0].id = 'REQ-role-1'), + 'an id numbered 0': (l) => (l.requirements[0].id = 'REQ-ROLE-0'), + 'an id with a suffix': (l) => (l.requirements[0].id = 'REQ-ROLE-1a'), + 'an id twice': (l) => (l.requirements[1].id = l.requirements[0].id), + 'a blank title': (l) => (l.requirements[0].title = ' '), + 'a title over 120 characters': (l) => (l.requirements[0].title = 'x'.repeat(121)), + 'a title of two lines': (l) => (l.requirements[0].title = 'a\nb'), + 'a title that is not a string': (l) => (l.requirements[0].title = 1), + }; + for (const x of [[], null, 'list']) assert.throws(() => checkRequirements(x), listError('list: not an object')); + for (const [what, edit] of Object.entries(bad)) { + const l = raw(); + edit(l); + assert.throws(() => checkRequirements(l), listError(), what); + } + const ok = raw(); + ok.requirements[0].title = 'x'.repeat(120); + assert.equal(checkRequirements(ok), ok); + const dir = mkdtempSync(join(tmpdir(), 'requirements-')); + try { + writeFileSync(join(dir, 'cut.json'), '{"prd":'); + assert.throws(() => loadRequirements(join(dir, 'cut.json')), listError('requirements.json: not JSON')); + assert.throws(() => loadRequirements(join(dir, 'absent.json')), listError('requirements.json: ENOENT')); + const twice = raw(); + twice.requirements.push(structuredClone(twice.requirements[0])); + writeFileSync(join(dir, 'twice.json'), JSON.stringify(twice)); + assert.throws(() => loadRequirements(join(dir, 'twice.json')), listError('requirements[24].id: REQ-ROLE-1 twice')); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/packages/tasks/tests/shapes.test.mjs b/packages/tasks/tests/shapes.test.mjs index c16f1f443..0a3ce8e36 100644 --- a/packages/tasks/tests/shapes.test.mjs +++ b/packages/tasks/tests/shapes.test.mjs @@ -113,11 +113,11 @@ test('the fake answers each route with the statuses and shapes Vikunja v2.7.0 se await w.adapter.start('demo'); const pm = w.agent('pm'), coder = w.agent('coder'); - const a = await w.call(pm, 'task.create', { title: 'a', request: w.instruction(), requirement: 'REQ-S-1' }); + const a = await w.call(pm, 'task.create', { title: 'a', request: w.instruction(), requirement: 'REQ-TASK-1' }); const b = await w.call(pm, 'task.create', { title: 'b', request: w.instruction(), - requirement: 'REQ-S-1', + requirement: 'REQ-TASK-1', labels: [w.label], relations: [{ kind: 'blocked', task_ref: a.task_ref }], }); @@ -201,7 +201,7 @@ test('the client works against the fake over real HTTP with the platform fetch', const w = await world(t, { fake, fetch: globalThis.fetch, tracker: { baseUrl: server.url } }); await w.adapter.start('demo'); assert.equal(w.adapter.status()[0].state, 'ready', w.adapter.status()[0].refused); - const r = await w.call(w.agent('pm'), 'task.create', { title: 'over http', request: w.instruction(), requirement: 'REQ-S-1' }); + const r = await w.call(w.agent('pm'), 'task.create', { title: 'over http', request: w.instruction(), requirement: 'REQ-TASK-1' }); assert.equal(r.task_ref, `vikunja:${w.project}/1`); await w.ui('PATCH', '/tasks/1', { title: 'edited' }); await sleep(20); diff --git a/packages/tasks/tests/sync.test.mjs b/packages/tasks/tests/sync.test.mjs index 5b7c9ffdd..71a96b184 100644 --- a/packages/tasks/tests/sync.test.mjs +++ b/packages/tasks/tests/sync.test.mjs @@ -8,7 +8,7 @@ const ready = async (t, edit) => { return w; }; const create = (w, title = 'a task') => - w.call(w.caps.pm ?? w.agent('pm'), 'task.create', { title, request: w.instruction(), requirement: 'REQ-S-1' }); + w.call(w.caps.pm ?? w.agent('pm'), 'task.create', { title, request: w.instruction(), requirement: 'REQ-TASK-1' }); const external = (w, subject) => w.events('task.changed.external').filter((e) => subject === undefined || e.subject === subject); // A fake whose clock runs an hour behind, so every task sits outside the cursor window and only the // board or a task read can see it. diff --git a/packages/tasks/tests/verbs.test.mjs b/packages/tasks/tests/verbs.test.mjs index 9824a7fa9..251317d39 100644 --- a/packages/tasks/tests/verbs.test.mjs +++ b/packages/tasks/tests/verbs.test.mjs @@ -2,6 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { world, sleep, SCOPES } from './world.mjs'; import { FakeVikunja } from '../src/fake.mjs'; +import { loadRequirements } from '../src/requirements.mjs'; const ready = async (t, edit) => { const w = await world(t, edit); await w.adapter.start('demo'); @@ -13,12 +14,12 @@ const refusedWith = (code) => (e) => e.code === code; // A pm-created task, returned with its ref and digest. const task = async (w, args = {}) => { const pm = w.caps.pm ?? w.agent('pm'); - return w.call(pm, 'task.create', { title: 'a task', request: w.instruction(), requirement: 'REQ-S-1', ...args }); + return w.call(pm, 'task.create', { title: 'a task', request: w.instruction(), requirement: 'REQ-TASK-1', ...args }); }; test('task.create needs a recorded human request and a requirement id', async (t) => { const w = await ready(t); const pm = w.agent('pm'); - const args = { title: 'x', request: w.instruction(), requirement: 'REQ-S-1' }; + const args = { title: 'x', request: w.instruction(), requirement: 'REQ-TASK-1' }; await assert.rejects(w.call(pm, 'task.create', { ...args, request: 'not-an-event' }), refusedWith('request-not-found')); await assert.rejects(w.call(pm, 'task.create', { ...args, requirement: 'S-1' }), refusedWith('invalid-request')); await assert.rejects(w.call(pm, 'task.create', { title: 'x', request: args.request }), refusedWith('invalid-request')); @@ -28,7 +29,7 @@ test('task.create needs a recorded human request and a requirement id', async (t assert.equal(r.task_ref, `vikunja:${w.project}/1`); const created = w.events('task.created'); assert.equal(created.length, 1); - assert.deepEqual(created[0].body, { request: args.request, requirement: 'REQ-S-1' }); + assert.deepEqual(created[0].body, { request: args.request, requirement: 'REQ-TASK-1' }); assert.equal(created[0].actor_role, 'pm'); const [s] = w.snapshots(r.task_ref); assert.equal(s.source, 'self'); @@ -50,6 +51,35 @@ test('task.create needs a recorded human request and a requirement id', async (t ); assert.equal(allowed(w, 'task.create').length, 1); }); +// Row 90 (#1584): the ids are the approved PRD's, from requirements.json. +test('task.create takes only an approved requirement id', async (t) => { + const w = await ready(t); + const pm = w.agent('pm'); + const { requirements } = loadRequirements(); + const args = { title: 'x', request: w.instruction() }; + // REQ-QA-1 is the id P19 #229 and #230 were created under: the right form, not in the PRD. + for (const requirement of ['REQ-QA-1', 'REQ-TASK-5', 'REQ-S-1']) + await assert.rejects(w.call(pm, 'task.create', { ...args, requirement }), refusedWith('unknown-requirement'), requirement); + // The form is checked first, as before. + await assert.rejects(w.call(pm, 'task.create', { ...args, requirement: 'REQ-task-1' }), refusedWith('invalid-request')); + assert.equal(allowed(w, 'task.create').length, 0); + assert.equal(w.events('task.created').length, 0); + for (const { id } of [requirements[0], requirements.at(-1)]) await w.call(pm, 'task.create', { ...args, requirement: id }); + assert.deepEqual(w.events('task.created').map((e) => e.body.requirement), [requirements[0].id, requirements.at(-1).id]); +}); +test("task.create cites the request a received message carries, not the message's id", async (t) => { + const w = await ready(t); + const pm = w.agent('pm'); + const sent = w.broker.request(w.human, { verb: 'message.send', args: { to: 'pm', body: 'build the thing' } }); + const [m] = w.broker.request(pm, { verb: 'message.receive' }); + assert.equal(m.id, sent.id); + assert.equal(m.request, sent.request); + assert.notEqual(m.request, m.id); + const args = { title: 'the thing', requirement: 'REQ-TASK-1' }; + await assert.rejects(w.call(pm, 'task.create', { ...args, request: m.id }), refusedWith('request-not-found')); + const r = await w.call(pm, 'task.create', { ...args, request: m.request }); + assert.deepEqual(w.events('task.created').map((e) => [e.subject, e.body]), [[r.task_ref, { request: m.request, requirement: 'REQ-TASK-1' }]]); +}); test('only labels named in the business file can be written', async (t) => { const w = await ready(t); // The pm can see this label in Vikunja (its owner created it), but the business file doesn't list it. diff --git a/roles/pm.md b/roles/pm.md index 4cbd3d8ee..534220823 100644 --- a/roles/pm.md +++ b/roles/pm.md @@ -10,6 +10,14 @@ order. You don't write the product's code and you don't review it. you create cites the requirement id it serves, such as `REQ-TASK-1`, and the human request that asked for it. A task without both is refused. +- Before creating a task, pick its requirement id from + `list_requirements`, the approved PRD's list. `task_create` refuses + any other id. The request is the `request ` in the header of the + human message you're answering, not the message's own id. +- When no id fits the request, don't create the task, and don't ask in + your answer. Raise a decision with action `prd.approve`, `blocking` + false and no `task_ref`, naming the request and the nearest ids. It + goes to the human's Inbox. - Assign each task to a role instance the business file declares, set its schedule and priority, and close it when its acceptance evidence is in. @@ -26,6 +34,14 @@ the resolved list for this business and project, which may be narrower. Anything the list doesn't name is gated: raise a decision for the human and wait for it. Never route around a refusal. +A blocking decision holds a task: it needs `task_ref`, and the task +waits for the answer. A decision with no task is non-blocking, and a +gated one still reaches the human's Inbox. + +Only the human stops a run, with `mosaic stop`. If you think a coder or +reviewer run should stop, raise a `role.revoke` decision with `blocking` +false and the role instance as target, naming the run and saying why. + Changing a task's priority or scope against the CTO's technical call is cross-role. Raise it as a decision for the technical arbiter.