diff --git a/packages/bus/tests/workflow-roles.test.mjs b/packages/bus/tests/workflow-roles.test.mjs new file mode 100644 index 000000000..26a278f1d --- /dev/null +++ b/packages/bus/tests/workflow-roles.test.mjs @@ -0,0 +1,99 @@ +// Row 149 (#1643, REQ-ROLE-5, REQ-DEC-5): the shipped merge-gate and security-reviewer roles on the +// broker. merge-gate raises the merge itself in the technical domain, the technical arbiter resolves +// it, and only merge-gate's own instance and thread can use the approval. security-reviewer answers +// a review with no message.send grant: the verdict is its own path to the role that asked. +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 { busBusiness } from '../src/business.mjs'; +import { resolveInstance, validateBusinessDocument } from '../../business/src/index.mjs'; +import { businessDoc, REPO_ROLES, systemFor } from '../../business/tests/helpers.mjs'; + +const TASK = 'vikunja:3/7'; +const A = 'a'.repeat(40); +const options = [ + { key: 'yes', text: 'Land it' }, + { key: 'no', text: 'Hold it' }, +]; + +// businessDoc's arbiters are pm (delivery) and cto (technical); the two new instances are added by +// definition name, with no credentials or tracker bot. +function setup(t, mutate = () => {}) { + const root = mkdtempSync(join(tmpdir(), 'bus-workflow-roles-')); + const doc = businessDoc(root, 'demo'); + doc.roles['merge-gate'] = { definition: 'merge-gate' }; + doc.roles['security-reviewer'] = { definition: 'security-reviewer' }; + const business = validateBusinessDocument(doc, join(root, 'businesses', 'demo.json'), { rolesDir: REPO_ROLES }); + const resolved = {}; + for (const instance of Object.keys(business.roles)) + resolved[instance] = resolveInstance({ system: systemFor(root), business, instance }); + const config = busBusiness(business, resolved); + mutate(config); + const store = new Store(root); + const b = new Broker({ store, businesses: { demo: config } }); + t.after(() => { + store.close(); + rmSync(root, { recursive: true, force: true }); + }); + const caps = {}; + for (const role of ['merge-gate', 'security-reviewer', 'coder', 'cto', 'pm']) { + caps[role] = b.bindLaunch({ business: 'demo', role, run: `${role}-run`, harness: 'pi' }); + b.request(caps[role], { verb: 'role.claim', args: {} }); + } + const call = (role, verb, args = {}) => b.request(caps[role], { verb, args }); + const raise = (role, action, extra = {}) => + call(role, 'decision.raise', { action, question: `${action}?`, options, recommendation: 'yes', blocking: false, ...extra }); + return { b, store, caps, call, raise }; +} + +test("merge-gate's technical raise is cross-role, goes to the cto, and only merge-gate can use the approval", (t) => { + const { b, store, caps, call, raise } = setup(t); + const d = raise('merge-gate', 'git.merge.protected', { domain: 'technical', target: TASK }); + assert.equal(d.class, 'cross-role'); + assert.equal(d.route_to, 'cto'); + assert.throws(() => b.authorize(caps['merge-gate'], 'git.merge.protected', { decision: d.id, target: TASK }), /decision-not-approved/); + call('cto', 'decision.resolve', { id: d.id, choice: 'yes' }); + const close = store.get("SELECT from_role, to_role FROM messages WHERE class='DECISION' AND decision=?", d.id); + assert.deepEqual({ ...close }, { from_role: 'cto', to_role: 'merge-gate' }); + // The cto resolved it but didn't raise it, so the approval isn't the cto's to use. + assert.throws(() => b.authorize(caps.cto, 'git.merge.protected', { decision: d.id, target: TASK }), /decision-mismatch/); + assert.equal(b.authorize(caps['merge-gate'], 'git.merge.protected', { decision: d.id, target: TASK }).class, 'cross-role'); + assert.throws(() => b.authorize(caps['merge-gate'], 'git.merge.protected', { decision: d.id, target: TASK }), /decision-consumed/); +}); + +test("merge-gate has no message.send, and without a domain its raise goes to the delivery arbiter", (t) => { + const { call, raise } = setup(t); + assert.throws(() => call('merge-gate', 'message.send', { to: 'cto', body: 'landed' }), /decision-required/); + assert.equal(raise('merge-gate', 'git.merge.protected', { target: TASK }).route_to, 'pm'); + // Every other action is gated for merge-gate, so an uncited raise goes to the human. + const push = raise('merge-gate', 'git.push.working', { domain: 'technical', target: TASK }); + assert.deepEqual([push.class, push.route_to], ['gated', 'human']); +}); + +test("the cto must be able to message merge-gate, or the technical raise is refused (row 133)", (t) => { + const { raise, store } = setup(t, (config) => { + const lists = config.definitions['merge-gate'].messaging; + config.definitions['merge-gate'].messaging = { ...lists, receiveFrom: lists.receiveFrom.filter((r) => r !== 'cto') }; + }); + assert.throws(() => raise('merge-gate', 'git.merge.protected', { domain: 'technical', target: TASK }), /decision-route-refused/); + assert.equal(store.get('SELECT count(*) n FROM decisions').n, 0); +}); + +test('security-reviewer answers a review with its verdict alone; it holds no message.send', (t) => { + const { b, store, caps, call } = setup(t); + b.checkPush(caps.coder, TASK); + b.recordPush(caps.coder, { task_ref: TASK, project: 'stack', branch: 'feat', ref: 'work/vikunja-3-7', commit: A, previous: null }); + const r = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'security-reviewer' }); + assert.equal(store.get('SELECT to_role FROM messages WHERE id=?', r.message).to_role, 'security-reviewer'); + assert.throws(() => call('security-reviewer', 'message.send', { to: 'coder', body: 'looks fine' }), /decision-required/); + const v = call('security-reviewer', 'review.verdict', { task_ref: TASK, request: r.request, verdict: 'approve', body: 'No security findings.' }); + const m = store.get('SELECT from_role, to_role, class FROM messages WHERE id=?', v.message); + assert.deepEqual({ ...m }, { from_role: 'security-reviewer', to_role: 'coder', class: 'REVIEW-RESULT' }); + // It can't push or merge on its own. + assert.throws(() => b.checkPush(caps['security-reviewer'], TASK), /decision-required/); + assert.throws(() => b.authorize(caps['security-reviewer'], 'git.merge.protected', { target: TASK }), /decision-required/); +}); diff --git a/packages/business/README.md b/packages/business/README.md index 504fbd14d..23b34bf1c 100644 --- a/packages/business/README.md +++ b/packages/business/README.md @@ -175,6 +175,51 @@ these roles goes live. `packages/business/tests/chief-of-staff.test.mjs` and through `loadBusiness`; `role.test.mjs` loads every file in `roles/` through the validator. +### Security-reviewer and merge-gate instances + +Two roles finish the coding workflow next to `coder` and `reviewer` +(REQ-ROLE-5; row 149, #1643). + +The `security-reviewer` role (`roles/security-reviewer.json`, contract +`roles/security-reviewer.md`) reviews a candidate for security only. It +holds `review.verdict` within role and nothing else: read-only tools +(read, grep, find, ls), no shell, no write tools, no credentials. The +verdict reaches the role that asked through `review.verdict` itself, so the +role needs no `message.send`. Without it the runner's automatic reply to +a message is refused; the verdict still arrives. + +The `merge-gate` role (`roles/merge-gate.json`, contract +`roles/merge-gate.md`) lands approved work. Its one grant is +`git.merge.protected` as a cross-role action, the grant row 137's land op +checks. It raises the merge decision itself with domain `technical`, +because only the raising instance and thread can use an approval +(REQ-DEC-5), and lands once the technical arbiter resolves it. It has the +same read-only tools and no credentials. + +Both declare messaging lists. The technical arbiter must be able to +message merge-gate, or the raise is refused (row 133): the shipped lists +include `cto`, so a business whose technical arbiter is another +definition has to change merge-gate's lists by a reviewed commit. Because +of those lists, neither role can be named in `arbiters`; the business +file is refused. + +No verb adds a role instance. `organization.set`'s `instances` only sets +the model of an instance the business file already declares +(`launch-settings.mjs`), so the human adds both entries by editing the +business file, then runs `validate`. No shipped business declares either +role yet; adding them to a live business is a separate step with its own +go. The entries are: + +```json +"merge-gate": { "definition": "merge-gate" }, +"security-reviewer": { "definition": "security-reviewer" } +``` + +`packages/business/tests/workflow-roles.test.mjs` checks both against +their approved lists, the arbiter refusal and the proactive line in every +contract; `packages/bus/tests/workflow-roles.test.mjs` checks the merge +route and the verdict path on the broker. + ### Project file ```json diff --git a/packages/business/tests/workflow-roles.test.mjs b/packages/business/tests/workflow-roles.test.mjs new file mode 100644 index 000000000..87420e256 --- /dev/null +++ b/packages/business/tests/workflow-roles.test.mjs @@ -0,0 +1,150 @@ +// Row 149 (#1643, REQ-ROLE-5): the security-reviewer and merge-gate roles, and the proactive line in +// every role contract. security-reviewer holds read-only tools and the verdict; merge-gate holds +// git.merge.protected cross-role and nothing else, so every landing needs a decision it raised. +// A widened file fails here. Fixtures live in a temp directory and nothing reads the real config. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { ACTIONS, BusinessError, GATED_ONLY, classify, loadBusiness, loadRole, resolveInstance } from "../src/index.mjs"; +import { businessDoc, REPO_ROLES, systemFor, tmp, writeJson } from "./helpers.mjs"; + +const APPROVED = Object.freeze({ + "security-reviewer": { + tools: ["read", "grep", "find", "ls"], + network: "api-only", + authority: { withinRole: ["review.verdict"], crossRole: [] }, + credentials: [], + // The verdict goes back to the role that asked; coder and cto hold review.request. + messaging: { + sendTo: ["human", "chief-of-staff", "pm", "cto", "coder"], + receiveFrom: ["human", "chief-of-staff", "pm", "cto", "coder"], + }, + }, + "merge-gate": { + tools: ["read", "grep", "find", "ls"], + network: "api-only", + authority: { withinRole: [], crossRole: ["git.merge.protected"] }, + credentials: [], + // cto, the technical arbiter in businessDoc, closes merge-gate's raise and so must reach it (row 133). + messaging: { + sendTo: ["human", "chief-of-staff", "pm", "cto"], + receiveFrom: ["human", "chief-of-staff", "pm", "cto"], + }, + }, +}); +const NAMES = Object.keys(APPROVED); +const WRITES = ["write", "edit", "bash", "web"]; +const PROACTIVE = /Within\s+your\s+grants,\s+act\s+on\s+what\s+you\s+see\s+without\s+waiting\s+to\s+be\s+asked,\s+and\s+raise\s+a\s+decision\s+or\s+send\s+a\s+message\s+for\s+anything\s+outside\s+them\./; + +function fixture(mutate = () => {}) { + const root = tmp(); + const doc = businessDoc(root, "acme"); + for (const name of NAMES) doc.roles[name] = { definition: name }; + mutate(doc); + writeJson(join(root, "businesses", "acme.json"), doc); + return { root, business: loadBusiness("acme", { dir: root, rolesDir: REPO_ROLES }) }; +} +const resolved = ({ root, business }, instance) => resolveInstance({ system: systemFor(root), business, instance }); + +test("both definitions load, with a contract file, and are exactly the approved lists", () => { + for (const name of NAMES) { + const role = loadRole(REPO_ROLES, name); + assert.equal(role.roleVersion, 2); + assert.equal(role.contract, `${name}.md`); + assert.ok(existsSync(join(REPO_ROLES, `${name}.md`)), name); + const { title, ...rest } = JSON.parse(readFileSync(join(REPO_ROLES, `${name}.json`), "utf8")); + assert.equal(typeof title, "string"); + assert.deepEqual(rest, { roleVersion: 2, name, contract: `${name}.md`, ...APPROVED[name] }, name); + assert.match(readFileSync(join(REPO_ROLES, `${name}.md`), "utf8"), new RegExp(`^# Role contract: ${name}\\n`), name); + } +}); + +test("security-reviewer has no write or shell tool and classifies only the verdict as its own", () => { + const role = loadRole(REPO_ROLES, "security-reviewer"); + for (const tool of WRITES) assert.ok(!role.tools.includes(tool), tool); + const record = resolved(fixture(), "security-reviewer"); + for (const tool of WRITES) assert.ok(!record.limits.tools.includes(tool), tool); + assert.deepEqual(record.credentials, {}); + for (const action of ACTIONS) assert.equal(classify(record, action), action === "review.verdict" ? "within" : "gated", action); +}); + +test("merge-gate's only grant is git.merge.protected cross-role, and it has no write tool", () => { + const role = loadRole(REPO_ROLES, "merge-gate"); + assert.deepEqual(role.authority, { withinRole: [], crossRole: ["git.merge.protected"] }); + for (const tool of WRITES) assert.ok(!role.tools.includes(tool), tool); + const record = resolved(fixture(), "merge-gate"); + assert.deepEqual(record.credentials, {}); + for (const action of ACTIONS) assert.equal(classify(record, action), action === "git.merge.protected" ? "cross" : "gated", action); +}); + +test("neither role can be an arbiter: its messaging lists refuse the business file (row 133)", () => { + for (const name of NAMES) { + for (const kind of ["technical", "delivery"]) { + assert.throws(() => fixture((doc) => (doc.arbiters[kind] = name)), + (e) => e instanceof BusinessError && e.message.includes(`arbiters.${kind} names ${name}, whose role definition ${name} declares messaging lists`), + `${kind} ${name}`); + } + } +}); + +test("every role contract carries the proactive line in its Authority section", () => { + const contracts = readdirSync(REPO_ROLES).filter((f) => f.endsWith(".md")); + for (const required of ["assistant.md", "coder.md", "reviewer.md", "pm.md", "cto.md", "chief-of-staff.md", "security-reviewer.md", "merge-gate.md"]) { + assert.ok(contracts.includes(required), required); + } + for (const file of contracts) { + const text = readFileSync(join(REPO_ROLES, file), "utf8"); + const start = text.indexOf("\n## Authority\n"); + assert.ok(start >= 0, file); + const end = text.indexOf("\n## ", start + 1); + assert.match(text.slice(start, end === -1 ? undefined : end), PROACTIVE, file); + } +}); + +test("pm and chief-of-staff say where a raise goes under REQ-DEC-2", () => { + for (const name of ["pm", "chief-of-staff"]) { + const text = readFileSync(join(REPO_ROLES, `${name}.md`), "utf8").replace(/\s+/g, " "); + assert.match(text, /a cross-role raise, or a gated raise that cites the project's current spec commit, goes to the business's arbiter for its domain/, name); + assert.match(text, /Pass `domain` to `raise_decision`, `technical` or `delivery`; leaving it out means `delivery`/, name); + assert.match(text, /An uncited gated raise, and every hard wait, goes to the human through the inbox/, name); + assert.doesNotMatch(text, /raise a decision for the human/, name); + // The hard waits named are exactly GATED_ONLY, so a vocabulary change shows up here. + const listed = text.match(/The hard waits are (.*?); a citation never moves them/)[1].match(/`[a-z.]+`/g).map((w) => w.slice(1, -1)); + assert.deepEqual(listed.sort(), [...GATED_ONLY].sort(), name); + // Q169 (broker.mjs END_PROTECTED, #hardWait): revoking the assistant is a hard wait as well. + assert.match(text, /A `role\.revoke` raise whose target is the assistant is a hard wait too: it goes to the human, cited or not\./, name); + } +}); + +// Sage's ruling sage-0684 on Filbert's 28878 item 1: the launcher binds land_work once, at launch, when +// the broker's landHeld is true, so the run that raises the merge decision can't land it. A later run +// of the same instance, launched after the approval, lands. The cto asks for that run and the chief +// of staff launches it. The broker and launcher states are row 137's tests (BUILD.md, round 3). +// The raise cites the spec commit for acceptance 314 steps 5 and 8 (sage-0691). +test("the merge landing takes two runs: merge-gate, cto and chief-of-staff each say their part", () => { + const read = (name) => readFileSync(join(REPO_ROLES, `${name}.md`), "utf8").replace(/\s+/g, " "); + const gate = read("merge-gate"); + assert.match(gate, /Cite the project's current spec commit on that raise: read it with `read_spec`, then pass `project`, `spec_path` `spec\/north-star\.md` and `spec_commit`\./); + assert.match(gate, /If the raise is refused `stale-spec`, read the spec again and raise once more with the current commit\./); + assert.match(gate, /If the project has no spec the human saved \(`read_spec` is refused, or `confirmed` is false\), raise uncited and say so in the question\./); + assert.match(gate, /The run that raises the decision ends its turn there\./); + assert.match(gate, /If the approval arrives in a run whose tools have no `land_work`, don't try to land and don't raise again\./); + assert.match(gate, /A run launched after the approval has `land_work`, and its launch record has a "Landing" line\./); + // Filbert 28913 (sage-0707): the later run's turn comes on the chief of staff's message, since the raising run read the DECISION. + assert.match(gate, /Its turn comes on the chief of staff's message naming the task reference and the decision id: call `land_work` with them\./); + // sage-0709: only recordLand spends a decision (broker.mjs #spent). checkLand refuses decision-consumed after + // it; a land started while another is in flight is refused at recordLand, in the later RESULT. + assert.match(gate, /A `decision-consumed` refusal, in the `land_work` reply or in the later RESULT, means the landing already happened: another landing of the decision got there first\./); + assert.match(gate, /Report the task landed in your answer, and don't land or raise again\./); + assert.doesNotMatch(gate, /Once the decision is resolved with its approval option, call `land_work`/); + assert.match(read("cto"), /When you resolve a `git\.merge\.protected` decision with its approval option, send the chief of staff one message with `send_message`, naming the instance that raised it \(`raised_by_role`\), the decision id and the task reference \(the decision's `authorization\.target`\)\./); + const chief = read("chief-of-staff"); + assert.match(chief, /When the cto's message names an instance whose `git\.merge\.protected` decision it approved, the decision id and the task reference, launch that instance with `launch`\./); + assert.match(chief, /Once `launch` for that instance succeeds, the first time or after an end, send the instance one message with `send_message` naming the task reference and the decision id\./); + assert.match(chief, /A run takes a turn only on a message it receives, and the approval went to the run that raised it, so the new run lands on your message\./); + // sage-0691: the chief of staff ends a run only after launch refused instance-running, and only that instance's. + assert.match(chief, /End a run only when all of these hold: the cto's message named the instance and the decision, `launch` for that instance was refused `instance-running`, and the run belongs to that instance\./); + assert.match(chief, /Then end it with `end_run`, with a reason naming the decision, and launch once it has exited\. End no other run\./); + assert.match(chief, /Only the human stops a run, with `mosaic stop`, apart from the merge relaunch under Duties\./); +}); diff --git a/packages/harness/README.md b/packages/harness/README.md index 020f7db56..8e1af17f7 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -339,3 +339,8 @@ raise_decision accepts optional project, spec_path and spec_commit. Cite both spec fields together, using the current confirmed file commit returned by read_spec, not the repository HEAD. Uncited raises remain valid. Later human spec commits do not reopen decisions already settled. + +raise_decision also accepts optional domain, technical or delivery (row 149, #1643). +A routed raise, cross-role or gated with a current spec citation, goes to the +business arbiter for that domain. Without domain the broker uses delivery, and +the tool sends no domain key. diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs index d90504742..b2d085d56 100644 --- a/packages/harness/src/tools.mjs +++ b/packages/harness/src/tools.mjs @@ -200,12 +200,13 @@ export function typedTools({ resolved, arbiter = false, tracker = false, project decision: COMMON.decision, }, ["project", "path", "record"])); } - add("raise_decision", "decision.raise", null, "Ask for a decision before a gated or cross-role action, or when you need a choice made. The answer arrives as a message.", obj({ + add("raise_decision", "decision.raise", null, "Ask for a decision before a gated or cross-role action, or when you need a choice made. A cross-role raise, or a gated raise citing the project's current spec commit, goes to the business's arbiter for its domain; an uncited gated raise or a hard wait goes to the human. The answer arrives as a message.", obj({ action: str("The action the decision is about", { enum: [...ACTIONS] }), question: str("The question, with enough context to answer it and the reason for your recommendation"), options: { type: "array", minItems: 2, maxItems: 9, items: obj({ key: str("Short key"), text: str("What this option means") }, ["key", "text"]) }, recommendation: str("Exactly one option key, as written in options, with nothing added. The reason goes in question"), 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" }, + domain: str("The arbiter domain for a routed raise: technical or delivery. Leaving it out means delivery", { enum: ["technical", "delivery"] }), target: str("What the action applies to, such as a role instance or task reference"), approvalChoice: str("The option key that approves the action. It defaults to yes, so pass it whenever no option key is yes"), task_ref: TASK_REF, diff --git a/packages/harness/tests/raise-domain.test.mjs b/packages/harness/tests/raise-domain.test.mjs new file mode 100644 index 000000000..a3903e50c --- /dev/null +++ b/packages/harness/tests/raise-domain.test.mjs @@ -0,0 +1,53 @@ +// Row 149 (#1643): raise_decision takes an optional domain, technical or delivery, so a raise can +// reach arbiters.technical. The tool sends what it is given; the broker defaults to delivery. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { typedTools, callTool } from '../src/tools.mjs'; +import { resolvedFor, fakeToolSocket } from './helpers.mjs'; +import { fixture } from '../../bus/tests/spec-fixtures.mjs'; + +const options = [{ key: 'yes', text: 'Yes' }, { key: 'no', text: 'No' }]; + +test('raise_decision offers domain as technical or delivery only, optional, and says where a raise goes', t => { + for (const role of ['coder', 'reviewer', 'pm', 'chief']) { + const raise = typedTools({ resolved: resolvedFor(t, role) }).find(x => x.name === 'raise_decision'); + const { domain } = raise.parameters.properties; + assert.equal(domain.type, 'string'); + assert.deepEqual(domain.enum, ['technical', 'delivery']); + for (const other of ['Technical', 'DELIVERY', 'human', 'security', '']) assert.ok(!domain.enum.includes(other), other); + assert.match(domain.description, /Leaving it out means delivery/); + assert.ok(!raise.parameters.required.includes('domain')); + assert.equal(raise.parameters.additionalProperties, false); + assert.match(raise.description, /A cross-role raise, or a gated raise citing the project's current spec commit, goes to the business's arbiter for its domain/); + assert.match(raise.description, /an uncited gated raise or a hard wait goes to the human/); + } +}); + +test('raise_decision forwards domain to decision.raise, and a call without it sends none', async t => { + const f = await fixture(t, { + arbiters: { technical: 'cto', delivery: 'pm' }, + roles: { + coder: { definition: 'coder', authority: { withinRole: ['message.send'], crossRole: ['task.reassign'] } }, + cto: { definition: 'cto', authority: { withinRole: ['message.send'], crossRole: [] } }, + pm: { definition: 'pm', authority: { withinRole: ['message.send'], crossRole: [] } }, + }, + }); + const byName = new Map(typedTools({ resolved: { definition: 'coder', limits: { tools: [], authority: { + withinRole: ['message.send'], crossRole: ['task.reassign'], + } } } }).map(tool => [tool.name, tool])); + const server = await fakeToolSocket(t, f.root, (name, args) => + f.broker.request(f.coder, { verb: byName.get(name).verb, args })); + const base = { action: 'task.reassign', question: 'Hand T-1 to another coder?', options, recommendation: 'yes', blocking: false, target: 'T-1' }; + + const technical = await callTool(server.path, 'raise_decision', { ...base, domain: 'technical' }); + const delivery = await callTool(server.path, 'raise_decision', { ...base, domain: 'delivery' }); + const omitted = await callTool(server.path, 'raise_decision', base); + assert.deepEqual([technical.route_to, delivery.route_to, omitted.route_to], ['cto', 'pm', 'pm']); + assert.equal(server.calls[0].args.domain, 'technical'); + assert.equal(server.calls[1].args.domain, 'delivery'); + assert.equal(Object.hasOwn(server.calls[2].args, 'domain'), false); + + // The harness doesn't check its own schema, so a value outside it reaches the broker, which refuses it. + await assert.rejects(callTool(server.path, 'raise_decision', { ...base, domain: 'security' }), /invalid-domain/); + assert.equal(f.store.get('SELECT count(*) n FROM decisions').n, 3); +}); diff --git a/roles/assistant.md b/roles/assistant.md index f230aeae7..541291edd 100644 --- a/roles/assistant.md +++ b/roles/assistant.md @@ -27,6 +27,9 @@ sessions. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, directing another role, launching a role session, pushing to a diff --git a/roles/chief-executive-officer.md b/roles/chief-executive-officer.md index 3cbec50bd..787b83916 100644 --- a/roles/chief-executive-officer.md +++ b/roles/chief-executive-officer.md @@ -11,6 +11,9 @@ You hold the `chief-executive-officer` role for one business. You set the organi ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, diff --git a/roles/chief-financial-officer.md b/roles/chief-financial-officer.md index 6fb381142..c1ee384b5 100644 --- a/roles/chief-financial-officer.md +++ b/roles/chief-financial-officer.md @@ -11,6 +11,9 @@ You hold the `chief-financial-officer` role for one business. You advise on cost ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, diff --git a/roles/chief-information-officer.md b/roles/chief-information-officer.md index 89ce6b687..3624b096d 100644 --- a/roles/chief-information-officer.md +++ b/roles/chief-information-officer.md @@ -11,6 +11,9 @@ You hold the `chief-information-officer` role for one business. You advise on th ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, diff --git a/roles/chief-information-security-officer.md b/roles/chief-information-security-officer.md index 239d2798d..dfe11f7c4 100644 --- a/roles/chief-information-security-officer.md +++ b/roles/chief-information-security-officer.md @@ -11,6 +11,9 @@ You hold the `chief-information-security-officer` role for one business. You adv ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, diff --git a/roles/chief-marketing-officer.md b/roles/chief-marketing-officer.md index 422210653..942ce19d0 100644 --- a/roles/chief-marketing-officer.md +++ b/roles/chief-marketing-officer.md @@ -11,6 +11,9 @@ You hold the `chief-marketing-officer` role for one business. You advise on how ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own: sending messages. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, diff --git a/roles/chief-of-staff.md b/roles/chief-of-staff.md index 902868f3f..1a8d3eacf 100644 --- a/roles/chief-of-staff.md +++ b/roles/chief-of-staff.md @@ -39,24 +39,51 @@ product's code, you don't review it and you don't create tasks yourself. BLOCK or an accept, relay it to the assistant with `send_message` `in_reply_to` the message that carried it. The bus refuses a relay without that link. A BLOCK doesn't revert the commit. +- When the cto's message names an instance whose `git.merge.protected` + decision it approved, the decision id and the task reference, launch + that instance with `launch`. End a run only when all of these hold: the + cto's message named the instance and the decision, `launch` for that + instance was refused `instance-running`, and the run belongs to that + instance. Then end it with `end_run`, with a reason naming the + decision, and launch once it has exited. End no other run. +- Once `launch` for that instance succeeds, the first time or after an + end, send the instance one message with `send_message` naming the task + reference and the decision id. A run takes a turn only on a message it + receives, and the approval went to the run that raised it, so the new + run lands on your message. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + +Where a raise goes (PRD 1.5 REQ-DEC-2): a cross-role raise, or a gated +raise that cites the project's current spec commit, goes to the +business's arbiter for its domain. Pass `domain` to `raise_decision`, +`technical` or `delivery`; leaving it out means `delivery`. An uncited +gated raise, and every hard wait, goes to the human through the inbox. +The hard waits are `spend`, `data.delete`, `network.expose`, +`account.create`, `credential.mint`, `message.external`, +`git.push.protected` and `deploy`; a citation never moves them. +A `role.revoke` raise whose target is the assistant is a hard wait too: +it goes to the human, cited or not. + Your role file lists what you do on your own: sending messages and launching role sessions. Anything the list doesn't name is gated. That includes creating or assigning tasks, pushing to a repository, sending messages outside the business, minting credentials, and any change to policy or to a PRD. When an action is gated, raise a decision. It -reaches the human through the inbox, not through you. Never route around +reaches its arbiter or the human, never you. Never route around a refusal. You hold no human authority. Never resolve a decision for the human, never approve a request in their name, and never write words as if they had said them. -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. +Only the human stops a run, with `mosaic stop`, apart from the merge +relaunch under Duties. 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. ## Protocol diff --git a/roles/coder.md b/roles/coder.md index 9163046c0..10d93e923 100644 --- a/roles/coder.md +++ b/roles/coder.md @@ -22,6 +22,9 @@ assigned to you and hand finished work to review. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own, what needs another role's arbiter, and what goes to the human. The launch record you received shows the resolved list for this business and project, which may be narrower. diff --git a/roles/cto.md b/roles/cto.md index 0e09e0e3d..442242408 100644 --- a/roles/cto.md +++ b/roles/cto.md @@ -11,6 +11,11 @@ assigned engineering work yourself. session can find them. - You arbitrate technical conflicts. When roles disagree on a technical question, the decision comes to you, and you resolve it with a reason. +- When you resolve a `git.merge.protected` decision with its approval + option, send the chief of staff one message with `send_message`, + naming the instance that raised it (`raised_by_role`), the decision id + and the task reference (the decision's `authorization.target`). That + instance can land only in a run launched after your approval. - Request reviews for the work you produce, and do the tasks assigned to you. - When the bus tells you the human saved `spec/north-star.md`, read the @@ -23,6 +28,9 @@ assigned engineering work yourself. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own, what needs another role's arbiter, and what goes to the human. The launch record you received shows the resolved list for this business and project, which may be narrower. diff --git a/roles/merge-gate.json b/roles/merge-gate.json new file mode 100644 index 000000000..448919abc --- /dev/null +++ b/roles/merge-gate.json @@ -0,0 +1,17 @@ +{ + "roleVersion": 2, + "name": "merge-gate", + "title": "Merge gate", + "contract": "merge-gate.md", + "tools": ["read", "grep", "find", "ls"], + "network": "api-only", + "authority": { + "withinRole": [], + "crossRole": ["git.merge.protected"] + }, + "credentials": [], + "messaging": { + "sendTo": ["human", "chief-of-staff", "pm", "cto"], + "receiveFrom": ["human", "chief-of-staff", "pm", "cto"] + } +} diff --git a/roles/merge-gate.md b/roles/merge-gate.md new file mode 100644 index 000000000..5a96628e8 --- /dev/null +++ b/roles/merge-gate.md @@ -0,0 +1,61 @@ +# Role contract: merge-gate + +You hold a `merge-gate` role instance for one business. You land +reviewed work on main. You don't write or review code, and you have no +write tools. + +## Duties + +- Take a task whose latest push the reviewers approved. Check its trail + with `trail`: the latest push, and an approving verdict for that push + from each review the task needs. +- Raise the merge decision yourself with `raise_decision`: action + `git.merge.protected`, domain `technical`, the task reference as the + target. Only the instance and thread that raised a decision can use its + approval, so never land on a decision another role raised. +- Cite the project's current spec commit on that raise: read it with + `read_spec`, then pass `project`, `spec_path` `spec/north-star.md` and + `spec_commit`. If the raise is refused `stale-spec`, read the spec + again and raise once more with the current commit. If the project has + no spec the human saved (`read_spec` is refused, or `confirmed` is + false), raise uncited and say so in the question. +- Landing takes two runs. The run that raises the decision ends its + turn there. The host gives a run `land_work` only at launch, and only + when this instance holds an approved merge decision it hasn't used, so + the raising run can't land. If the approval arrives in a run whose + tools have no `land_work`, don't try to land and don't raise again. + When the technical arbiter approves, it asks the chief of staff to + launch this instance again. +- A run launched after the approval has `land_work`, and its launch + record has a "Landing" line. Its turn comes on the chief of staff's + message naming the task reference and the decision id: call + `land_work` with them. The host checks the decision and the verdicts + again, and the full test gate runs. The outcome comes to you as a + RESULT message: landed, or refused with the reason. +- A `decision-consumed` refusal, in the `land_work` reply or in the + later RESULT, means the landing already happened: another landing of + the decision got there first. Report the task landed in your answer, + and don't land or raise again. +- If the decision is declined, a verdict is missing or stale, or the + landing is refused, don't land and don't retry around it. The bus keeps + the refusal on the task's trail with its code; name the code and the + reason in your answer. + +## Authority + +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + +Your role file grants one action, `git.merge.protected`, as cross-role: +every landing needs a decision resolved by the technical arbiter. The +launch record you received shows the resolved list for this business and +project, which may be narrower. You can't push, review, send messages or +change a task. For anything else, raise a decision and wait for it. Never +route around a refusal. + +## Protocol + +- You act only through the typed tools the broker gives you. You never + hold a token. +- You hold no `send_message` grant, so the turn's reply to a message is + refused. A refusal and a landing are both on the task's trail. diff --git a/roles/pm.md b/roles/pm.md index f4bece2ce..50a95f951 100644 --- a/roles/pm.md +++ b/roles/pm.md @@ -43,15 +43,29 @@ order. You don't write the product's code and you don't review it. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + +Where a raise goes (PRD 1.5 REQ-DEC-2): a cross-role raise, or a gated +raise that cites the project's current spec commit, goes to the +business's arbiter for its domain. Pass `domain` to `raise_decision`, +`technical` or `delivery`; leaving it out means `delivery`. An uncited +gated raise, and every hard wait, goes to the human through the inbox. +The hard waits are `spend`, `data.delete`, `network.expose`, +`account.create`, `credential.mint`, `message.external`, +`git.push.protected` and `deploy`; a citation never moves them. +A `role.revoke` raise whose target is the assistant is a hard wait too: +it goes to the human, cited or not. + Your role file lists what you do on your own, what needs another role's arbiter, and what goes to the human. The launch record you received shows 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. +Anything the list doesn't name is gated: raise a decision 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. +waits for the answer. A decision with no task is non-blocking, and an +uncited 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` diff --git a/roles/reviewer.md b/roles/reviewer.md index 46211299f..57c485c23 100644 --- a/roles/reviewer.md +++ b/roles/reviewer.md @@ -18,6 +18,9 @@ candidates and record a verdict. You don't change the code you review. ## Authority +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + Your role file lists what you do on your own, what needs another role's arbiter, and what goes to the human. The launch record you received shows the resolved list for this business and project, which may be narrower. diff --git a/roles/security-reviewer.json b/roles/security-reviewer.json new file mode 100644 index 000000000..8cf1d6075 --- /dev/null +++ b/roles/security-reviewer.json @@ -0,0 +1,17 @@ +{ + "roleVersion": 2, + "name": "security-reviewer", + "title": "Security reviewer", + "contract": "security-reviewer.md", + "tools": ["read", "grep", "find", "ls"], + "network": "api-only", + "authority": { + "withinRole": ["review.verdict"], + "crossRole": [] + }, + "credentials": [], + "messaging": { + "sendTo": ["human", "chief-of-staff", "pm", "cto", "coder"], + "receiveFrom": ["human", "chief-of-staff", "pm", "cto", "coder"] + } +} diff --git a/roles/security-reviewer.md b/roles/security-reviewer.md new file mode 100644 index 000000000..b54e3e34e --- /dev/null +++ b/roles/security-reviewer.md @@ -0,0 +1,42 @@ +# Role contract: security-reviewer + +You hold a `security-reviewer` role instance for one business. You review +candidates for security only and record a verdict. You don't change the +code you review, and you have no shell and no write tools. + +## Duties + +- Review the candidate named in the request for security: secrets in the + diff or its history, injection, unchecked input at a trust boundary, + widened authority or credentials, paths or network reach outside the + task's scope, and refusals the change routes around. Correctness, style + and test coverage belong to the reviewer, not to you. +- A REVIEW-REQUEST names a request id: call `fetch_review` with it, then + read what you can with your read tools. Never approve what you haven't + read. If you can't read the candidate, say so in a `changes` verdict. +- Record one verdict per request with `review_verdict`: approve, or + changes with each finding and its reason, stated so the author can act + on it. The verdict goes back to the role that asked. +- Never review your own work, or work you helped write. + +## Authority + +Within your grants, act on what you see without waiting to be asked, and +raise a decision or send a message for anything outside them. + +Your role file grants the verdict and nothing else. The launch record you +received shows the resolved list for this business and project, which +may be narrower. You can't push, merge, send messages or change a task. +For anything else, raise a decision and wait for it. Never route around a +refusal. + +Your verdict gates the work in the queue and on the bus. An approval in +Gitea is advisory. + +## Protocol + +- You act only through the typed tools the broker gives you. You never + hold a token. +- Your verdict is your answer. You hold no `send_message` grant, so the + turn's reply to a message is refused; the verdict still reaches the + role that asked.