diff --git a/docs/TOOLS.md b/docs/TOOLS.md index f7de3dc77..499278dfc 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -407,7 +407,10 @@ spec/north-star.md or docs/NAME.md of a project: one local commit authored by th business's human, refused `stale-base` if the file changed since the editor opened it. It is the human bus verb `project.file.save` through the human transport; an agent refuses it. Nothing is pushed. Without `--send` the page reads and previews only. See -`packages/webui/README.md` ("Spec and docs editor"). +`packages/webui/README.md` ("Spec and docs editor"). A save of spec/north-star.md also +sends the business's pm and cto one notice naming the commit (row 143); a BLOCK from +either goes to the chief of staff and on to the assistant. See `packages/bus/README.md` +("Spec review notices"). The unit runs a pinned copy, not the working checkout (row 77, #1571). `copy COMMIT` resolves COMMIT to its full id, refuses one not reachable diff --git a/packages/bus/README.md b/packages/bus/README.md index cf299049f..f18c527a0 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -314,9 +314,19 @@ message stranded for a role nobody holds shows once a run claims the role. Unknown keys, roles, references, actions and verbs refuse. If writing refusal evidence fails, the caller receives the static `storage-unavailable` code. `human` is the reserved recipient. Message classes are REQUEST, ASSIGNMENT, REVIEW-REQUEST, REVIEW-RESULT, -RESULT, INFO, DECISION and REACTION. `staff.request` also writes STAFF-REQUEST (row 140); +RESULT, INFO, DECISION, REACTION and BLOCK (row 143). `staff.request` also writes STAFF-REQUEST (row 140); `message.send` and `thread.send` refuse that class. References cannot cross businesses. +A BLOCK answers a spec notice (Spec review notices, below). `message.send` accepts it only from a +pm or cto instance (`block-sender`), only to the chief-of-staff instance (`block-recipient`; the +assistant may hear only from the user and the chief of staff, REQ-ASSIST-2), and only with +`in_reply_to` naming a notice the bus sent that sender (`block-reply-required`). The BLOCK sits in no +thread, because a message may join only a thread whose instance sent or received it. When the chief of +staff relays to the assistant in reply to the BLOCK (or to anything on a chain back to the notice), the +reply chain reaches the notice, and the relay is filed in the user's most recently opened thread of the +notice's project whose instance is the assistant. With no such thread the relay is refused +(`thread-required-reply-to-thread-message`). `thread.send` refuses the class. + Routing is immutable at raise: within-role closes atomically with `choice`, cross-role routes to the configured delivery arbiter (or technical arbiter with `domain:'technical'`). A gated raise that cites the project's current spec commit routes the same way, @@ -760,6 +770,22 @@ Every refusal is recorded as the others are. The save verb alone may carry a req up to 1 MiB + 64 KiB (the client and the server both check it); every other verb keeps the 64 KiB envelope. Tests: `packages/bus/tests/project-files.test.mjs`. +### Spec review notices + +Row 143 (#1637, REQ-PROJ-4): a successful `project.file.save` of `spec/north-star.md` sends one +notice to every instance whose definition is `pm` or `cto`. It comes from `_bus` (run `bus`), class +RESULT, in no thread. The body begins `The human saved spec/north-star.md in project : commit +, from the editor's base .` and asks the role to read the change with `read_spec`, check it +against settled decisions and work in flight, and answer the chief of staff once, in reply to the +notice: an INFO beginning "Accept", or a BLOCK naming each conflict. A BLOCK does not revert the commit. + +Only that verb sends a notice. A save of a docs file, a refused or stale save, an agent's +`record.write` commit and a git commit made outside the Console send none. The notice is stored after +the commit; if storing it fails the save answers `outcome-unknown` and the commit stands. A notice to a +role no run holds waits and is delivered at the next claim. The broker launches nothing and does not +tell `launch.by`; the chief of staff's `launch` is how a pm or cto session starts. A business with +no pm or cto instance sends nothing. Tests: `packages/bus/tests/spec-review.test.mjs`. + The 3e→3f offline step adds nullable spec_path/spec_commit columns to decisions, with nulls on existing rows. It changes no event or resolution and creates no spec/confirmation tables. The 3e fixture is pinned from 26a0b0fab's tree; no live diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index a72f95885..3f760abe3 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -156,6 +156,13 @@ const THREAD_DEFINITION = 'assistant'; // business ID_PATTERN, which starts with [a-z0-9], so no instance can be named this. const NOTICE_ROLE = '_bus'; const NOTICE_RUN = 'bus'; +// Row 143 (#1637, REQ-PROJ-4): a human save of this file through project.file.save sends one notice to each +// instance of these definitions. Their answer goes to the instance of SPEC_ANSWER_TO, the chief of staff, +// because only the user and the chief of staff may send to the assistant (REQ-ASSIST-2). +const SPEC_PATH = 'spec/north-star.md'; +const SPEC_REVIEWERS = ['pm', 'cto']; +const SPEC_ANSWER_TO = 'chief-of-staff'; +const SPEC_NOTICE = `The human saved ${SPEC_PATH} `; // How many messages `thread.messages` returns by default and at most, oldest first. export const THREAD_PAGE = 50; export const THREAD_PAGE_MAX = 200; @@ -283,7 +290,7 @@ export class Broker { for (let at = inReplyTo ?? null; at !== null && !seen.has(at); ) { seen.add(at); const m = this.#store.get( - 'SELECT m.in_reply_to, t.thread FROM messages m LEFT JOIN message_threads t ON t.message=m.id WHERE m.id=? AND m.business=?', + 'SELECT m.in_reply_to, m.from_role, m.body, t.thread FROM messages m LEFT JOIN message_threads t ON t.message=m.id WHERE m.id=? AND m.business=?', at, s.business, ); @@ -291,6 +298,11 @@ export class Broker { if (this.#store.get('SELECT instance FROM threads WHERE id=?', m.thread).instance !== to) fail('thread-instance-mismatch'); return m.thread; } + // Row 143: a chain that reaches a spec notice belongs to the user's thread in that notice's project. + if (m?.from_role === NOTICE_ROLE && m.body.startsWith(SPEC_NOTICE)) { + const spec = this.#specThread(s, to, m.body); + if (spec) return spec; + } at = m?.in_reply_to ?? null; } // The code is the whole message a run sees (runner.mjs forwards codes only), so it says what to do. @@ -1235,6 +1247,12 @@ export class Broker { keys(args, ['project', 'path', 'base', 'content'], ['project', 'path', 'base', 'content']); const saved = await writeFile({ ...context, path: args.path, base: args.base, content: args.content }); result = { path: saved.path, commit: saved.commit }; + // The commit stands whatever happens next, so a notice that fails to store answers outcome-unknown + // and the Console reloads to the new commit, as record.write does after its commit. + if (saved.path === SPEC_PATH) { + try { this.#store.transaction(() => this.#specNotice(s.business, args.project, saved.commit, args.base)); } + catch { fail('outcome-unknown'); } + } } this.#secretCheck(result); return result; @@ -1642,6 +1660,7 @@ export class Broker { 'INFO', 'DECISION', 'REACTION', + 'BLOCK', ].includes(a.class) ) fail('invalid-class'); @@ -1652,6 +1671,7 @@ export class Broker { fail('message-not-found'); } } + if (a.class === 'BLOCK') this.#checkBlock(s, to, a.in_reply_to); if (a.decision) this.#decision(s, a.decision); const thread = this.#threadFor(s, to, a.in_reply_to); const mid = randomUUID(); @@ -2306,6 +2326,67 @@ export class Broker { ); this.#route(business, nid, by, null); } + // Row 143 (#1637, REQ-PROJ-4): one notice per spec commit to each pm and cto instance, from the bus, + // class RESULT like #notice. The runner takes a turn on it and sends no automatic reply, so the body + // names where the answer goes. Written once, in the transaction that follows the save's commit; no + // other path calls it, so a record commit by an agent and a git commit outside the Console send none. + // The notice sits in no thread, so the user's thread view shows none of this traffic. Nothing is + // launched here: a notice to a role no run holds waits for a claim and does not tell launch.by. + #specNotice(business, project, commit, base) { + const b = this.#business(business); + const roles = Object.keys(b.roles).filter((r) => SPEC_REVIEWERS.includes(b.roles[r].definition)); + if (!roles.length) return; + const chief = Object.keys(b.roles).find((r) => b.roles[r].definition === SPEC_ANSWER_TO) ?? SPEC_ANSWER_TO; + for (const to of roles) { + const nid = randomUUID(); + this.#store.run( + 'INSERT INTO messages(id,at,business,from_role,from_run,to_role,class,in_reply_to,decision,corrects,body) VALUES(?,?,?,?,?,?,?,?,?,?,?)', + nid, + this.#now(), + business, + NOTICE_ROLE, + NOTICE_RUN, + to, + 'RESULT', + null, + null, + null, + `${SPEC_NOTICE}in project ${project}: commit ${commit}, from the editor's base ${base}. ` + + `Read the change with read_spec (commit ${commit}, and commit ${base} for the text before it) and check it against ` + + `settled decisions and work in flight. Answer once, to ${chief} with send_message in_reply_to ${nid}: ` + + `class BLOCK naming the conflict, or class INFO that begins "Accept". A BLOCK does not revert the commit. ` + + `Don't reply to _bus; it receives nothing.`, + ); + this.#route(business, nid, to, null); + } + } + // Row 143: a BLOCK answers a spec notice. Only a pm or cto instance sends one, only to the chief of + // staff, and only in reply to a notice the bus sent that sender. The BLOCK itself sits in no thread: the + // schema lets a message join only a thread whose instance sent or received it. The chief's relay to the + // assistant replies to the BLOCK, and #specThread finds the user's thread from the notice at the end of + // that chain, which is the only way a result reaches the assistant (REQ-ASSIST-2). + #checkBlock(s, to, inReplyTo) { + const b = this.#business(s.business); + if (s.human || s.tester || !SPEC_REVIEWERS.includes(b.roles[s.role]?.definition)) fail('block-sender'); + if (b.roles[to]?.definition !== SPEC_ANSWER_TO) fail('block-recipient'); + const n = inReplyTo === undefined ? null : this.#store.get('SELECT from_role, to_role, body FROM messages WHERE id=? AND business=?', inReplyTo, s.business); + if (!n || n.from_role !== NOTICE_ROLE || n.to_role !== s.role || !n.body.startsWith(SPEC_NOTICE)) fail('block-reply-required'); + } + // The thread a reply chain reaches through a spec notice: the user's most recently opened thread in the + // notice's project whose instance is the recipient, or null when there is none. + #specThread(s, to, body) { + const project = /^in project ([^ :]+):/.exec(body.slice(SPEC_NOTICE.length))?.[1]; + return ( + this.#store.get( + `SELECT t.id FROM threads t JOIN projects p ON p.id=t.project WHERE t.business=? AND t.owner=? AND t.instance=? AND p.repo=? + ORDER BY coalesce((SELECT max(o.seq) FROM thread_opens o WHERE o.thread=t.id), 0) DESC, t.seq DESC LIMIT 1`, + s.business, + this.#business(s.business).human, + to, + project, + )?.id ?? null + ); + } #delivery(mid, op, h, transport = null) { this.#store.run( 'INSERT INTO deliveries(message,at,op,holder_run,transport,address) VALUES(?,?,?,?,?,?)', diff --git a/packages/bus/tests/spec-review.test.mjs b/packages/bus/tests/spec-review.test.mjs new file mode 100644 index 000000000..2c5c16350 --- /dev/null +++ b/packages/bus/tests/spec-review.test.mjs @@ -0,0 +1,190 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { fixture, git } from './spec-fixtures.mjs'; +// Row 143 (#1637, REQ-PROJ-4): a human save of spec/north-star.md through project.file.save sends one notice +// to each pm and cto instance. A BLOCK goes to the chief of staff and, through its relay, to the assistant. +// The assistant's lists are the shipped roles/assistant.json's, so the chain runs under the lists the stack runs. +const shipped = name => JSON.parse(readFileSync(fileURLToPath(new URL(`../../../roles/${name}.json`, import.meta.url)), 'utf8')); +const within = { authority: { withinRole: ['message.send'], crossRole: [] } }; +const SPEC = 'spec/north-star.md'; +const reject = (promise, code) => assert.rejects(Promise.resolve(promise), { code }); +// message.send refuses synchronously, so its calls come in as thunks. +const refuse = (thunk, code) => assert.rejects(async () => thunk(), { code }); + +async function setup(t, { running = ['chief', 'pm', 'cto'], thread = true } = {}) { + const f = await fixture(t, { roles: { + chief: { definition: 'chief-of-staff', ...within }, pm: { definition: 'pm', ...within }, + cto: { definition: 'cto', ...within }, assistant: { definition: 'assistant', ...within }, + } }); + f.business.definitions = { assistant: { messaging: shipped('assistant').messaging } }; + f.cap = Object.fromEntries(running.map(role => [role, f.agent(role)])); + if (thread) { + const project = f.call(f.human, 'project.create', { name: 'stack work', repo: 'stack' }).id; + f.threadId = f.call(f.human, 'thread.create', { project, name: 'alpha' }).id; + f.cap.assistant = f.broker.bindLaunch({ business: 'demo', role: 'assistant', run: 'assistant-run', harness: 'pi', thread: f.threadId }); + f.broker.request(f.cap.assistant, { verb: 'role.claim' }); + } + f.notices = () => f.store.all("SELECT * FROM messages WHERE from_role='_bus' ORDER BY seq"); + f.noticeTo = role => f.notices().find(n => n.to_role === role); + f.threadOf = id => f.store.get('SELECT thread FROM message_threads WHERE message=?', id)?.thread ?? null; + f.head = async () => (await f.call(f.human, 'project.file', { project: 'stack', path: SPEC })).commit; + f.saveSpec = async content => f.call(f.human, 'project.file.save', { project: 'stack', path: SPEC, base: await f.head(), content }); + f.send = (role, to, args) => f.call(f.cap[role], 'message.send', { to, body: 'text', ...args }); + return f; +} + +test('a human spec save sends one notice to each pm and cto, naming the commit, and each commit sends its own', async t => { + const f = await setup(t); + const saved = await f.saveSpec('# Edited\n'); + const sent = f.notices(); + assert.deepEqual(sent.map(n => n.to_role).sort(), ['cto', 'pm']); + for (const n of sent) { + assert.equal(n.from_run, 'bus'); assert.equal(n.class, 'RESULT'); assert.equal(n.in_reply_to, null); + assert.ok(n.body.startsWith(`The human saved ${SPEC} in project stack: commit ${saved.commit}`)); + assert.ok(n.body.includes(`from the editor's base ${f.commit}`) && n.body.includes('read_spec') && n.body.includes(`in_reply_to ${n.id}`)); + assert.equal(f.threadOf(n.id), null); + // Routed to the run that holds the role, as any message is, so the runner takes a turn on it. + assert.ok(f.store.get('SELECT holder_run FROM deliveries WHERE message=?', n.id)?.holder_run); + } + for (const role of ['pm', 'cto']) + assert.deepEqual((await f.call(f.cap[role], 'message.receive')).map(m => m.id), [f.noticeTo(role).id]); + assert.deepEqual(await f.call(f.cap.chief, 'message.receive'), []); + const second = await f.saveSpec('# Again\n'); + assert.equal(f.notices().length, 4); + assert.equal(f.notices().filter(n => n.body.includes(`commit ${second.commit}`)).length, 2); +}); + +test('no notice for a docs save, an agent record commit, a git commit outside the Console, or a refused save', async t => { + const f = await setup(t); + const docs = await f.call(f.human, 'project.file', { project: 'stack', path: 'docs/README.md' }); + await f.call(f.human, 'project.file.save', { project: 'stack', path: 'docs/README.md', base: docs.commit, content: '# Docs\n' }); + await f.call(f.coder, 'record.write', { project: 'stack', path: 'docs/result.md', record: '# Result\n' }); + const outside = f.save('# By git\n'); + await reject(f.call(f.human, 'project.file.save', { project: 'stack', path: SPEC, base: f.commit, content: '# Stale\n' }), 'stale-base'); + await reject(f.call(f.coder, 'project.file.save', { project: 'stack', path: SPEC, base: outside, content: '# Agent\n' }), 'human-required'); + await reject(f.call(f.human, 'project.file.save', { project: 'stack', path: SPEC, base: 'abc', content: '# Bad\n' }), 'project-record-invalid'); + assert.deepEqual(f.notices(), []); + await f.saveSpec('# Mine\n'); + assert.equal(f.notices().length, 2); +}); + +test('a notice to a role no run holds waits for a claim, and launches nobody and tells nobody', async t => { + const f = await setup(t, { running: ['chief', 'cto'] }); + await f.saveSpec('# Edited\n'); + const waiting = f.noticeTo('pm'); + assert.ok(waiting); + assert.equal(f.notices().length, 2); + assert.deepEqual(await f.call(f.cap.chief, 'message.receive'), []); + const pm = f.agent('pm'); + assert.deepEqual((await f.call(pm, 'message.receive')).map(m => m.id), [waiting.id]); +}); + +test('a business without a pm or a cto saves the spec and sends nothing', async t => { + const f = await fixture(t); + const open = await f.call(f.human, 'project.file', { project: 'stack', path: SPEC }); + const saved = await f.call(f.human, 'project.file.save', { project: 'stack', path: SPEC, base: open.commit, content: '# Edited\n' }); + assert.equal(git(f.repo, 'rev-parse', 'HEAD'), saved.commit); + assert.deepEqual(f.store.all("SELECT 1 FROM messages WHERE from_role='_bus'"), []); +}); + +test('a notice that cannot be stored answers outcome-unknown, and the commit stands', async t => { + const f = await setup(t); + f.store.run("CREATE TRIGGER no_notice BEFORE INSERT ON messages WHEN NEW.from_role='_bus' BEGIN SELECT RAISE(ABORT, 'no'); END"); + const before = await f.head(); + await reject(f.call(f.human, 'project.file.save', { project: 'stack', path: SPEC, base: before, content: '# Edited\n' }), 'outcome-unknown'); + assert.notEqual(git(f.repo, 'rev-parse', 'HEAD'), before); + assert.equal(f.notices().length, 0); +}); + +test('a BLOCK reaches the chief of staff, the chief relays it to the assistant in the user\'s thread, and the commit stands', async t => { + const f = await setup(t); + const saved = await f.saveSpec('# Edited\n'); + const block = f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: f.noticeTo('pm').id, body: 'Conflicts with decision 41.' }); + assert.equal(f.threadOf(block.id), null); + const [got] = await f.call(f.cap.chief, 'message.receive'); + assert.equal(got.id, block.id); assert.equal(got.class, 'BLOCK'); assert.equal(got.from_role, 'pm'); + const relay = f.send('chief', 'assistant', { class: 'INFO', in_reply_to: block.id, body: 'pm blocks the spec change.' }); + assert.equal(f.threadOf(relay.id), f.threadId); + assert.deepEqual((await f.call(f.cap.assistant, 'message.receive')).map(m => m.id), [relay.id]); + assert.equal(git(f.repo, 'rev-parse', 'HEAD'), saved.commit); + assert.equal(git(f.repo, 'show', `HEAD:${SPEC}`), '# Edited'); + // The cto's accept is an ordinary INFO to the chief; it needs no BLOCK path. + const accept = f.send('cto', 'chief', { class: 'INFO', in_reply_to: f.noticeTo('cto').id, body: 'Accept.' }); + assert.equal(f.threadOf(accept.id), null); + assert.equal(f.store.get('SELECT class FROM messages WHERE id=?', accept.id).class, 'INFO'); +}); + +test('with no thread a BLOCK still reaches the chief of staff, and the relay to the assistant is refused', async t => { + const f = await setup(t, { thread: false }); + await f.saveSpec('# Edited\n'); + const block = f.send('cto', 'chief', { class: 'BLOCK', in_reply_to: f.noticeTo('cto').id }); + assert.deepEqual((await f.call(f.cap.chief, 'message.receive')).map(m => m.id), [block.id]); + await refuse(() => f.send('chief', 'assistant', { class: 'INFO', in_reply_to: block.id }), 'thread-required-reply-to-thread-message'); +}); + +test('within the notice\'s project the relay lands in the user\'s most recently opened thread', async t => { + const f = await setup(t); + const project = f.store.get('SELECT project FROM threads WHERE id=?', f.threadId).project; + const beta = f.call(f.human, 'thread.create', { project, name: 'beta' }).id; + await f.saveSpec('# Edited\n'); + const block = f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: f.noticeTo('pm').id }); + // No thread has been opened, so the newest created one wins; opening alpha moves the relay back to it. + assert.equal(f.threadOf(f.send('chief', 'assistant', { class: 'INFO', in_reply_to: block.id }).id), beta); + f.call(f.human, 'thread.open', { thread: f.threadId }); + assert.equal(f.threadOf(f.send('chief', 'assistant', { class: 'INFO', in_reply_to: block.id }).id), f.threadId); +}); + +test('a more recently opened thread in another project does not take the relay', async t => { + const f = await setup(t); + // The one assistant instance holds the user's threads in every project. The second project has no repo + // (the broker clones the business at construction, so the fixture cannot add another), and it is the + // thread the user opened last; the relay still belongs to the thread of the project the notice names. + const other = f.call(f.human, 'project.create', { name: 'side notes' }).id; + const gamma = f.call(f.human, 'thread.create', { project: other, name: 'gamma' }).id; + f.call(f.human, 'thread.open', { thread: gamma }); + assert.notEqual(gamma, f.threadId); + await f.saveSpec('# Edited\n'); + const block = f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: f.noticeTo('pm').id }); + assert.equal(f.threadOf(f.send('chief', 'assistant', { class: 'INFO', in_reply_to: block.id }).id), f.threadId); +}); + +test('a BLOCK is refused from anyone but a pm or cto, to anyone but the chief of staff, and outside a reply to its own notice', async t => { + const f = await setup(t); + await f.saveSpec('# Edited\n'); + const mine = f.noticeTo('pm').id, theirs = f.noticeTo('cto').id; + const plain = f.send('pm', 'chief', { class: 'INFO' }).id; + const before = f.store.get('SELECT count(*) n FROM messages').n; + await refuse(() => f.call(f.coder, 'message.send', { to: 'chief', body: 'x', class: 'BLOCK', in_reply_to: mine }), 'block-sender'); + await refuse(() => f.call(f.human, 'message.send', { to: 'chief', body: 'x', class: 'BLOCK', in_reply_to: mine }), 'block-sender'); + await refuse(() => f.call(f.cap.assistant, 'message.send', { to: 'chief', body: 'x', class: 'BLOCK', in_reply_to: mine }), 'block-sender'); + await refuse(() => f.send('pm', 'cto', { class: 'BLOCK', in_reply_to: mine }), 'block-recipient'); + await refuse(() => f.send('pm', 'human', { class: 'BLOCK', in_reply_to: mine }), 'block-recipient'); + await refuse(() => f.send('pm', 'chief', { class: 'BLOCK' }), 'block-reply-required'); + await refuse(() => f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: plain }), 'block-reply-required'); + await refuse(() => f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: theirs }), 'block-reply-required'); + // A bus notice of another kind (row 146's launch notice) is not a spec notice. + const other = 'other-bus-notice'; + f.store.run("INSERT INTO messages(id,at,business,from_role,from_run,to_role,class,body) VALUES(?,?,?,?,?,?,?,?)", + other, new Date().toISOString(), 'demo', '_bus', 'bus', 'pm', 'RESULT', 'A message for pm is waiting.'); + const withOther = f.store.get('SELECT count(*) n FROM messages').n; + await refuse(() => f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: other }), 'block-reply-required'); + assert.equal(f.store.get('SELECT count(*) n FROM messages').n, withOther); + assert.equal(withOther, before + 1); +}); + +test('a role cannot forge a spec notice: its text opens neither a BLOCK nor the assistant\'s thread', async t => { + const f = await setup(t); + const real = (await (async () => { await f.saveSpec('# Edited\n'); return f.noticeTo('pm'); })()).body; + const forged = f.send('chief', 'pm', { class: 'INFO', body: real }); + await refuse(() => f.send('pm', 'chief', { class: 'BLOCK', in_reply_to: forged.id }), 'block-reply-required'); + const answer = f.send('pm', 'chief', { class: 'INFO', in_reply_to: forged.id, body: 'Accept.' }); + await refuse(() => f.send('chief', 'assistant', { class: 'INFO', in_reply_to: answer.id }), 'thread-required-reply-to-thread-message'); +}); + +test('the bus receives nothing: a reply to a notice addressed to _bus is refused', async t => { + const f = await setup(t); + await f.saveSpec('# Edited\n'); + await refuse(() => f.send('pm', '_bus', { in_reply_to: f.noticeTo('pm').id }), 'unknown-role'); +}); diff --git a/packages/business/tests/spec-review-contracts.test.mjs b/packages/business/tests/spec-review-contracts.test.mjs new file mode 100644 index 000000000..aaa66220c --- /dev/null +++ b/packages/business/tests/spec-review-contracts.test.mjs @@ -0,0 +1,43 @@ +// Row 143 (#1637, REQ-PROJ-4, REQ-ASSIST-3): the role contracts for the review of a human spec save. The +// pm and the cto answer the bus's notice to the chief of staff; the assistant tells the user the result in +// its own words. The text tests pin the files the stack runs, as assistant.test.mjs does. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { REPO_ROLES } from "./helpers.mjs"; + +const contract = (name) => readFileSync(join(REPO_ROLES, `${name}.md`), "utf8"); + +test("the pm and the cto check a saved spec against settled decisions and work in flight, and answer with a BLOCK or an accept", () => { + for (const name of ["pm", "cto"]) { + const text = contract(name); + assert.match(text, /the human saved\s+`spec\/north-star\.md`/, name); + assert.match(text, /`read_spec`/, name); + assert.match(text, /decisions\s+already\s+settled/, name); + assert.match(text, /in\s+flight/, name); + assert.match(text, /chief\s+of\s+staff\s+once/, name); + assert.match(text, /class\s+`BLOCK`\s+naming\s+each\s+conflict/, name); + assert.match(text, /class\s+`INFO`\s+that\s+begins\s+"Accept"/, name); + assert.match(text, /A\s+BLOCK\s+doesn't\s+revert\s+the\s+commit/, name); + assert.match(text, /the\s+assistant\s+tells\s+the\s+user/, name); + } +}); + +test("the chief of staff relays a BLOCK or an accept to the assistant as a reply to the message that carried it", () => { + const text = contract("chief-of-staff"); + assert.match(text, /answers\s+the\s+bus's\s+notice\s+of\s+a\s+saved\s+spec\s+with\s+a\s+BLOCK\s+or\s+an\s+accept/); + assert.match(text, /relay\s+it\s+to\s+the\s+assistant\s+with\s+`send_message`\s+`in_reply_to`\s+the\s+message\s+that\s+carried\s+it/); + assert.match(text, /The\s+bus\s+refuses\s+a\s+relay\s+without\s+that\s+link/); + assert.match(text, /A\s+BLOCK\s+doesn't\s+revert\s+the\s+commit/); +}); + +test("the assistant summarizes a spec review in its own words and doesn't repeat the chief of staff's text", () => { + const text = contract("assistant"); + assert.match(text, /review\s+of\s+the\s+user's\s+saved\s+spec\s+change,\s+summarize\s+it\s+for\s+the\s+user\s+in\s+your\s+own\s+words/); + assert.match(text, /Don't\s+repeat\s+the\s+chief\s+of\s+staff's\s+text\s+or\s+quote\s+it\s+back/); + assert.match(text, /A\s+BLOCK\s+doesn't\s+undo\s+the\s+user's\s+commit/); + // The user's own rulings still go to the chief of staff verbatim; only a review result is summarized. + assert.match(text, /Relay each ruling the user gives verbatim to the chief\s+of\s+staff/); +}); diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs index 495b1acfd..d90504742 100644 --- a/packages/harness/src/tools.mjs +++ b/packages/harness/src/tools.mjs @@ -75,7 +75,7 @@ const TASK_TOOLS = { "task.close": ["task_close", "Close a task after its review verdict. A task with pushed work closes only on an approve verdict for its latest push.", { ...COMMON, verdict: str("The review verdict id review_verdict returned, as the REVIEW-RESULT message names it") }, ["task_ref", "verdict"]], }; -const MESSAGE_CLASSES = ["REQUEST", "ASSIGNMENT", "REVIEW-REQUEST", "REVIEW-RESULT", "RESULT", "INFO", "DECISION", "REACTION"]; +const MESSAGE_CLASSES = ["REQUEST", "ASSIGNMENT", "REVIEW-REQUEST", "REVIEW-RESULT", "RESULT", "INFO", "DECISION", "REACTION", "BLOCK"]; // typedTools({ resolved, arbiter, tracker, projects, land }) -> [{ name, verb, action, description, parameters }] // resolved resolveInstance(...) output for the session's instance diff --git a/packages/harness/tests/tools.test.mjs b/packages/harness/tests/tools.test.mjs index fc2882ce7..ffe5d3d17 100644 --- a/packages/harness/tests/tools.test.mjs +++ b/packages/harness/tests/tools.test.mjs @@ -29,6 +29,12 @@ test("the PM gets its task verbs and the reads, and no launch", (t) => { } }); +// Row 143 (#1637): send_message offers the BLOCK class a pm or cto answers a spec notice with. +test("send_message offers the BLOCK class, beside the classes it already had", (t) => { + const send = typedTools({ resolved: resolvedFor(t, "pm"), arbiter: true, tracker: true }).find((x) => x.name === "send_message"); + assert.deepEqual(send.parameters.properties.class.enum, ["REQUEST", "ASSIGNMENT", "REVIEW-REQUEST", "REVIEW-RESULT", "RESULT", "INFO", "DECISION", "REACTION", "BLOCK"]); +}); + // Row 119 (#1613): the chief of staff holds launch and message.send; row 123 adds role.end and organization.change, which row 132 gives tools. test("the chief of staff gets launch, send_message, end_run and change_organization, and no task verbs", (t) => { const chief = resolvedFor(t, "chief"); diff --git a/roles/assistant.md b/roles/assistant.md index 240d4eded..f230aeae7 100644 --- a/roles/assistant.md +++ b/roles/assistant.md @@ -18,6 +18,12 @@ sessions. it, paraphrase it or add to it. Don't send it to that role yourself. - Report to the user what the chief of staff answers, and every refusal with its exact text. +- When the chief of staff passes you the result of a review of the user's + saved spec change, summarize it for the user in your own words. Say + whether the product manager and the CTO accepted, and for each BLOCK + say what conflicts with what and what the user can do about it. Don't + repeat the chief of staff's text or quote it back. A BLOCK doesn't + undo the user's commit, so say that the change stands. ## Authority diff --git a/roles/chief-of-staff.md b/roles/chief-of-staff.md index 432846492..902868f3f 100644 --- a/roles/chief-of-staff.md +++ b/roles/chief-of-staff.md @@ -35,6 +35,10 @@ product's code, you don't review it and you don't create tasks yourself. may do, raise a `policy.change` decision. - Report to the assistant what the roles answer, and every refusal with its exact text. +- When the pm or the cto answers the bus's notice of a saved spec with a + 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. ## Authority diff --git a/roles/cto.md b/roles/cto.md index 2f8b82093..0e09e0e3d 100644 --- a/roles/cto.md +++ b/roles/cto.md @@ -13,6 +13,13 @@ assigned engineering work yourself. question, the decision comes to you, and you resolve it with a reason. - 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 + change with `read_spec` and check it against the decisions already + settled and the work in flight. Answer the chief of staff once, in + reply to the bus's notice: with `send_message` class `BLOCK` naming each + conflict, or class `INFO` that begins "Accept". A BLOCK doesn't revert + the commit. The chief of staff tells the assistant, and the assistant + tells the user. ## Authority diff --git a/roles/pm.md b/roles/pm.md index 6baa8b6f6..f4bece2ce 100644 --- a/roles/pm.md +++ b/roles/pm.md @@ -33,6 +33,13 @@ order. You don't write the product's code and you don't review it. many sessions it needs. The chief of staff answers by message with the instances it launched, or with a refusal and its reason. The limits are the chief of staff's to apply, not yours. +- When the bus tells you the human saved `spec/north-star.md`, read the + change with `read_spec` and check it against the decisions already + settled and the tasks in flight. Answer the chief of staff once, in + reply to the bus's notice: with `send_message` class `BLOCK` naming each + conflict, or class `INFO` that begins "Accept". A BLOCK doesn't revert + the commit. The chief of staff tells the assistant, and the assistant + tells the user. ## Authority