bus: a decision raise checks the route may message the raiser; arbiters can't declare messaging lists (row 133, #1627)

Dewey's row 133 round 2 candidate (agents/dewey/work/queue-133/r2,
manifest 1c0e6f8f...0ef0), approved by Filbert in comment 28600.
Landing gate on 4a34fb0d6 with rows 133 and 70 applied: all 24 suites
passed, 0 failed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

packages/bus/README.md

@@ -209,6 +209,13 @@ requires a strict task ref. `role.launch` is gated unless the business launch
 block's `by` names the acting role and refused when launch state is revoked. S6 still owns launch allowlists,
 capacity and actual spawning.
 
+Whoever closes a decision messages the raising role, so a cross-role or gated raise also needs the
+route to be allowed to message the raiser under the role definitions' messaging lists (row 133,
+#1627). In practice the raiser's `receiveFrom` must list the arbiter's definition, or `human` for a
+gated route. Otherwise the raise refuses `decision-route-refused`, logs `action.refused` and stores
+nothing. A within-role raise is exempt. The close itself isn't checked again; the business validator
+refuses an arbiter whose role definition declares messaging lists.
+
 `decision.raise` checks its choices in this order, and each of these codes names one field:
 
 | Code | Field | Refused when |

packages/bus/src/broker.mjs

@@ -336,15 +336,20 @@ export class Broker {
   // without the lists is unlimited, as before. The sender's lists are checked first, then the
   // target's, so a tester or the human sending to a limited role is refused unless listed. Every
   // path that stores a message from a role checks it: message.send, review.request, review.verdict.
-  #checkMessaging(s, to) {
-    const b = this.#business(s.business);
-    const definition = (role) => b.definitions?.[b.roles[role]?.definition];
-    const sender = s.human ? 'human' : s.tester ? 'tester' : b.roles[s.role]?.definition;
-    const target = to === 'human' || to === 'tester' ? to : b.roles[to]?.definition;
-    const out = s.human || s.tester ? undefined : definition(s.role)?.messaging;
-    if (out && !out.sendTo.includes(target)) fail('message-recipient-refused');
-    const inbound = to === 'human' || to === 'tester' ? undefined : definition(to)?.messaging;
-    if (inbound && !inbound.receiveFrom.includes(sender)) fail('message-sender-refused');
+  // Row 133 (#1627): from and to are 'human', 'tester' or a role instance, so decision.raise can
+  // check its route as the sender of the close message; `refused` replaces both codes when given.
+  #checkMessaging(business, from, to, refused) {
+    const b = this.#business(business);
+    const person = (who) => who === 'human' || who === 'tester';
+    const name = (who) => (person(who) ? who : b.roles[who]?.definition);
+    const lists = (who) => (person(who) ? undefined : b.definitions?.[name(who)]?.messaging);
+    const out = lists(from);
+    if (out && !out.sendTo.includes(name(to))) fail(refused ?? 'message-recipient-refused');
+    const inbound = lists(to);
+    if (inbound && !inbound.receiveFrom.includes(name(from))) fail(refused ?? 'message-sender-refused');
+  }
+  #sender(s) {
+    return s.human ? 'human' : s.tester ? 'tester' : s.role;
   }
   #decision(s, id) {
     const d = this.#store.get('SELECT * FROM decisions WHERE id=? AND business=?', id, s.business);
@@ -887,6 +892,10 @@ export class Broker {
         if (!['technical', 'delivery'].includes(domain)) fail('invalid-domain');
         const proposedRoute = cls === 'gated' ? 'human' : cls === 'cross-role' ? b.arbiters[domain] : s.role;
         const route = cls === 'cross-role' && proposedRoute === s.role ? 'human' : proposedRoute;
+        // Row 133 (#1627): whoever closes the decision messages the raiser, so the route must be
+        // allowed to message the raising role before anything is stored. A within-role decision is
+        // routed to the raiser's own role and closes in this call.
+        if (cls !== 'within-role') this.#checkMessaging(s.business, route, s.role, 'decision-route-refused');
         if (a.supersedes) {
           const old = this.#decision(s, a.supersedes);
           if (old.raised_by_run !== s.run || this.#closed(old.id)) fail('supersede-refused');
@@ -988,6 +997,10 @@ export class Broker {
             d.id,
           );
         // Row 115 (#1609): a close by anyone but the raising run tells the raising role, routed like message.send's.
+        // Row 133 (#1627): no messaging check here. decision.raise refused a route that can't message the
+        // raiser, definitions change only with a pinned copy and arbiters only through the business
+        // validator, so the route can still message the raiser when it closes the decision. The human's
+        // withdrawal of an arbiter-routed decision is unchecked too; no shipped role can reach it.
         if (op !== 'seen' && (s.human || s.run !== d.raised_by_run)) {
           const outcome =
             op === 'resolved'
@@ -1038,7 +1051,7 @@ export class Broker {
           if (a.to === 'tester') fail('unknown-role');
         }
         if (a.to !== 'human' && a.to !== 'tester' && !Object.hasOwn(b.roles, a.to)) fail('unknown-role');
-        this.#checkMessaging(s, a.to);
+        this.#checkMessaging(s.business, this.#sender(s), a.to);
         string(a.body, 32768);
         if (
           a.class !== undefined &&
@@ -1143,7 +1156,7 @@ export class Broker {
           fail(older ? 'stale-commit' : 'unknown-commit');
         }
         // REQ-ASSIST-2 has no approval exception: a review request is a message to the reviewer.
-        this.#checkMessaging(s, a.to);
+        this.#checkMessaging(s.business, this.#sender(s), a.to);
         const mid = randomUUID();
         // Row 105 (#1599): the run holding the reviewer role, or null when nobody holds it and the
         // request waits for a claim. #reviewMessage routes on the same holder.
@@ -1186,7 +1199,7 @@ export class Broker {
         )
           fail('verdict-exists');
         // The verdict is a message to the role that asked, so it meets that role's receive list.
-        this.#checkMessaging(s, r.from);
+        this.#checkMessaging(s.business, this.#sender(s), r.from);
         const mid = randomUUID();
         const eid = this.#event(
           s,

packages/bus/tests/decision-route.test.mjs

@@ -0,0 +1,145 @@
+// Row 133 (#1627, REQ-ASSIST-2): whoever closes a decision messages the raising role, so
+// decision.raise refuses a route that can't message the raiser before it stores anything. The route
+// is the sender and the raiser the target, under row 120's messaging rules. A within-role decision,
+// routed to the raiser's own role, is exempt.
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { mkdtempSync, readFileSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { Store } from '../src/store.mjs';
+import { Broker } from '../src/broker.mjs';
+
+// The assistant's lists come from the shipped role file, as in messaging.test.mjs.
+const assistantLists = JSON.parse(
+  readFileSync(fileURLToPath(new URL('../../../roles/assistant.json', import.meta.url)), 'utf8'),
+).messaging;
+
+const options = [
+  { key: 'yes', text: 'Allow' },
+  { key: 'no', text: 'Decline' },
+];
+const pusher = { authority: { withinRole: ['message.send'], crossRole: ['git.push.working'] } };
+const roles = {
+  pm: { definition: 'pm', authority: { withinRole: ['message.send'], crossRole: [] } },
+  chief: { definition: 'chief-of-staff', authority: { withinRole: ['message.send'], crossRole: [] } },
+  assistant: { definition: 'assistant', authority: { withinRole: ['message.send'], crossRole: [] } },
+  // Hears from pm, the technical arbiter, but not from the chief of staff, the delivery arbiter.
+  coder: { definition: 'coder', ...pusher },
+  // Hears from neither arbiter nor the human, but may send to the human: a reversed check passes it.
+  warden: { definition: 'warden', ...pusher },
+  // Hears from the human and both arbiters but sends only to pm: a reversed check refuses it.
+  scribe: { definition: 'scribe', ...pusher },
+};
+const definitions = {
+  pm: {},
+  'chief-of-staff': {},
+  assistant: { messaging: assistantLists },
+  coder: { messaging: { sendTo: ['human', 'pm', 'chief-of-staff'], receiveFrom: ['human', 'pm'] } },
+  warden: { messaging: { sendTo: ['human', 'pm', 'chief-of-staff'], receiveFrom: ['coder'] } },
+  scribe: { messaging: { sendTo: ['pm'], receiveFrom: ['human', 'pm', 'chief-of-staff'] } },
+};
+const businesses = {
+  demo: { id: 'demo', human: 'jason', arbiters: { technical: 'pm', delivery: 'chief' }, roles, definitions },
+};
+const ROUTE = 'decision-route-refused';
+
+function setup(t) {
+  const root = mkdtempSync(join(tmpdir(), 'bus-decision-route-'));
+  const store = new Store(root);
+  t.after(() => {
+    store.close();
+    rmSync(root, { recursive: true, force: true });
+  });
+  const b = new Broker({ store, businesses });
+  const caps = {};
+  for (const role of Object.keys(roles)) {
+    caps[role] = b.bindLaunch({ business: 'demo', role, run: `${role}-run`, harness: 'pi' });
+    b.request(caps[role], { verb: 'role.claim' });
+  }
+  caps.human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true });
+  const call = (who, verb, args = {}) => b.request(caps[who], { verb, args });
+  const raise = (who, action, extra = {}) =>
+    call(who, 'decision.raise', { action, question: `${action}?`, options, recommendation: 'no', blocking: false, ...extra });
+  const count = (sql) => store.get(sql).n;
+  const stored = () => ({
+    decisions: count('SELECT count(*) n FROM decisions'),
+    decisionEvents: count('SELECT count(*) n FROM decision_events'),
+    messages: count('SELECT count(*) n FROM messages'),
+    allowed: count("SELECT count(*) n FROM events WHERE kind='action.allowed'"),
+  });
+  const refusals = () =>
+    store
+      .all("SELECT actor_role, body FROM events WHERE kind='action.refused' ORDER BY seq")
+      .map((e) => [e.actor_role, JSON.parse(e.body).code]);
+  return { store, call, raise, stored, refusals };
+}
+
+test('a cross-role raise whose arbiter the raiser does not hear from is refused and stores nothing', (t) => {
+  const { raise, stored, refusals } = setup(t);
+  const before = stored();
+  // Delivery goes to chief (chief-of-staff), which the coder's receiveFrom doesn't list.
+  assert.throws(() => raise('coder', 'git.push.working', { domain: 'delivery' }), { code: ROUTE });
+  assert.throws(() => raise('warden', 'git.push.working', { domain: 'technical' }), { code: ROUTE });
+  assert.deepEqual(refusals(), [['coder', ROUTE], ['warden', ROUTE]]);
+  assert.deepEqual(stored(), before, 'no decision, decision event, message or allowed event');
+});
+
+test('a gated raise by a role that does not hear from the human is refused and stores nothing', (t) => {
+  const { raise, stored, refusals } = setup(t);
+  const before = stored();
+  // deploy is gated, so the route is the human; warden's receiveFrom lacks human.
+  assert.throws(() => raise('warden', 'deploy', { target: 'release-1' }), { code: ROUTE });
+  // A cross-role action outside the raiser's crossRole is gated too.
+  assert.throws(() => raise('warden', 'task.create'), { code: ROUTE });
+  assert.deepEqual(refusals(), [['warden', ROUTE], ['warden', ROUTE]]);
+  assert.deepEqual(stored(), before);
+});
+
+test('a route the raiser hears from is allowed, checked from the route to the raiser', (t) => {
+  const { raise, call, store } = setup(t);
+  const technical = raise('coder', 'git.push.working', { domain: 'technical' });
+  const gated = raise('coder', 'deploy', { target: 'release-1' });
+  assert.deepEqual([technical.route_to, gated.route_to], ['pm', 'human']);
+  // scribe sends only to pm, but every route here may message scribe.
+  const routes = [
+    raise('scribe', 'git.push.working', { domain: 'delivery' }),
+    raise('scribe', 'git.push.working', { domain: 'technical' }),
+    raise('scribe', 'deploy', { target: 'release-1' }),
+  ].map((d) => d.route_to);
+  assert.deepEqual(routes, ['chief', 'pm', 'human']);
+  // The close message the check stands for reaches the raiser.
+  call('pm', 'decision.resolve', { id: technical.id, choice: 'yes' });
+  const close = store.get("SELECT from_role, to_role FROM messages WHERE class='DECISION' AND decision=?", technical.id);
+  assert.deepEqual({ ...close }, { from_role: 'pm', to_role: 'coder' });
+});
+
+test('a refused supersede withdraws nothing', (t) => {
+  const { raise, store } = setup(t);
+  const old = raise('coder', 'git.push.working', { domain: 'technical' });
+  assert.throws(
+    () => raise('coder', 'git.push.working', { domain: 'delivery', supersedes: old.id }),
+    { code: ROUTE },
+  );
+  assert.equal(store.get('SELECT count(*) n FROM decision_events WHERE decision=?', old.id).n, 0);
+  assert.equal(store.get('SELECT count(*) n FROM decisions').n, 1);
+});
+
+test("the assistant's within-role and gated raises still work", (t) => {
+  const { raise, call, store, refusals } = setup(t);
+  // message.send is within role: routed to the assistant itself, which its own lists don't name.
+  const within = raise('assistant', 'message.send', { choice: 'yes' });
+  assert.equal(within.route_to, 'assistant');
+  assert.equal(
+    store.get("SELECT op FROM decision_events WHERE decision=?", within.id).op,
+    'resolved',
+  );
+  // Anything else is gated: routed to the human, whom the assistant hears from.
+  const gated = raise('assistant', 'task.create');
+  assert.equal(gated.route_to, 'human');
+  call('human', 'decision.resolve', { id: gated.id, choice: 'no' });
+  const close = store.get("SELECT from_role, to_role FROM messages WHERE class='DECISION' AND decision=?", gated.id);
+  assert.deepEqual({ ...close }, { from_role: 'human', to_role: 'assistant' });
+  assert.deepEqual(refusals(), []);
+});

packages/business/src/business.mjs

@@ -156,6 +156,12 @@ export function validateBusinessDocument(document, file, { rolesDir }) {
     const name = document.arbiters[kind];
     requireId(name, `${file} arbiters.${kind}`);
     if (!Object.hasOwn(roles, name)) refuse(`${file} arbiters.${kind} names ${name}, which the business file doesn't declare`);
+    // Row 133 (#1627): an arbiter messages the raiser when it closes a decision, so it can't be a
+    // role whose definition limits who it sends to and hears from.
+    const definition = roles[name].definition;
+    if (definitions[definition].messaging) {
+      refuse(`${file} arbiters.${kind} names ${name}, whose role definition ${definition} declares messaging lists`);
+    }
     arbiters[kind] = name;
   }
 

packages/business/tests/assistant.test.mjs

@@ -122,6 +122,31 @@ test("the instance takes no credential references and no tracker", () => {
   );
 });
 
+// Row 133 (#1627): an arbiter messages the raiser when it closes a decision, so a role limited by
+// messaging lists can't be one. The message names the kind, the instance and the definition.
+test("an arbiter whose role definition declares messaging lists is refused, for each kind", () => {
+  for (const kind of ["delivery", "technical"]) {
+    refuses(
+      () => loadFixture((doc) => { doc.arbiters[kind] = INSTANCE; }),
+      new RegExp(`arbiters\\.${kind} names assistant, whose role definition assistant declares messaging lists`),
+    );
+    // An instance name that isn't the definition's: both are named.
+    refuses(
+      () => loadFixture((doc) => {
+        doc.roles.aide = { definition: "assistant" };
+        doc.arbiters[kind] = "aide";
+      }),
+      new RegExp(`arbiters\\.${kind} names aide, whose role definition assistant declares messaging lists`),
+    );
+  }
+});
+
+test("an arbiter without messaging lists is accepted beside the limited assistant", () => {
+  assert.deepEqual(loadFixture().business.arbiters, { delivery: "pm", technical: "cto" });
+  const { business } = loadFixture((doc) => { doc.arbiters.delivery = "chief"; });
+  assert.deepEqual(business.arbiters, { delivery: "chief", technical: "cto" });
+});
+
 test("the contract passes work to the chief of staff and directs no one", () => {
   const text = readFileSync(join(REPO_ROLES, "assistant.md"), "utf8");
   assert.match(text, /chief of staff/);

packages/business/tests/business.test.mjs

@@ -215,6 +215,8 @@ test("the shipped example refuses as written and validates once filled in", () =
     .replaceAll("/home/you", root));
   const business = validateBusinessDocument(filled, file, { rolesDir: REPO_ROLES });
   assert.deepEqual(Object.keys(business.roles), ["pm", "cto", "coder", "reviewer", "chief", "assistant"]);
+  // Row 133 (#1627): the arbiters declare no messaging lists, so the limited assistant beside them passes.
+  assert.deepEqual(business.arbiters, { delivery: "pm", technical: "cto" });
   assert.equal(business.tracker.sync.credentials.vikunja.file, join(root, ".config/mosaic-dev/secrets/mosaic-stack/sync-vikunja.token"));
   assert.equal(id, 15);
 });