bus: approvals bind to the decision and the role instance, not the run (row 136, #1630)

Round 2 candidate db463a7b18962ee4afa393565abac9c4dd694dba49b18db20db064cac21fab89
(Dewey, packet 8b4cad1f2), approved by Filbert in comment 28704. Sage's landing
gate on 05ad2e7d0: 24 steps rc 0, 23:58:57Z to 00:03:24Z.

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

packages/bus/README.md

@@ -298,7 +298,16 @@ The options come first, so a bad option list refuses `invalid-options` even when
 An option that isn't an object with exactly `key` and `text` refuses `invalid-request`, like any other malformed argument.
 
 Authorization requires the resolved option identified by `approvalChoice`
-(default `yes`, which must exist), the same raiser role/run, action and target.
+(default `yes`, which must exist), the same raiser role instance, action and
+target. Row 136 (#1630): the approval binds to the decision, the business and
+the role instance (`raised_by_role`, the key in the business's `roles` map),
+not to the run that raised it, so it outlives a restart and any run of that
+instance holding its claim may consume it. A decision raised by a thread run
+binds to that run's thread too: only a run holding the same thread consumes
+it, and a decision a thread instance raised before row 121 matches no thread
+run. Another instance, or the same instance in another thread, refuses
+`decision-mismatch`. `role.revoke` still requires the target's holder run
+recorded at the raise.
 Inbox projections expose `{action,target,approvalChoice}` as `authorization`:
 **S4 must display this context when offering resolution**, not infer approval
 from recommendation or option position. Any other option is a refusal, even if
@@ -307,8 +316,8 @@ including cross-role approval (lead decision 64). `authorize`, `message.send`
 and `role.revoke` share one authority-and-consumption helper. Each caller keeps
 the consumption check, `action.allowed` event and any broker-owned effect in one
 transaction.
-A later use through any of those paths, including after restart, refuses with
-`decision-consumed`. The `decision.raise` context event does not consume approval.
+A later use through any of those paths, by any run and including after
+restart, refuses with `decision-consumed`. The `decision.raise` context event does not consume approval.
 A failed transaction rolls consumption back. A mismatch between the decision's
 recorded class and current policy refuses with `decision-mismatch` before use,
 in either direction. Within-role `authorize` calls without a decision retain
@@ -326,7 +335,10 @@ authority and the full matching, resolution and single-use checks still apply.
 A consumed approval is not an exactly-once external service operation receipt;
 an uncertain external outcome must not be retried with that approval. S3/S6
 must implement their own external-operation identity and uncertain-outcome
-handling. Raising/superseding and resolving are transactional.
+handling. Raising/superseding and resolving are transactional. `supersedes` names an
+open decision raised by the same role instance, in the same thread when it was
+raised in one, by this run or an earlier one; otherwise the raise refuses
+`supersede-refused` and withdraws nothing.
 
 ### Decision close messages (row 115, #1609)
 
@@ -348,7 +360,7 @@ where `<by>` is the close's `decision_events.by` (the human's name or the
 closing run). A close with a note adds `Note: <note>` on its own line.
 
 No message goes out for `decision.seen`, for the raising run's own withdraw, for
-a supersede (also the raising run's withdraw), or for a within-role decision,
+a supersede (a withdraw by the raising role instance), or for a within-role decision,
 which closes inside `decision.raise`. `message.receive` gives a human close's
 message the close's `human.input` answer event as its `request`, so the role's
 turn cites it as it cites a human instruction; a role close's `request` is

packages/bus/src/broker.mjs

@@ -489,6 +489,18 @@ export class Broker {
     if (!d) fail('decision-not-found');
     return d;
   }
+  // Row 136 (#1630): the thread a decision was raised in, the one its raising run claimed, or
+  // null. A run is bound to one thread for its life and claimed it before it could raise. A
+  // decision a thread instance raised before row 121 has no claim, so no thread run matches it.
+  #raisedThread(d) {
+    return (
+      this.#store.get(
+        "SELECT thread FROM thread_claims WHERE business=? AND holder_run=? AND op='claim' ORDER BY seq DESC LIMIT 1",
+        d.business,
+        d.raised_by_run,
+      )?.thread ?? null
+    );
+  }
   #decisionView(d) {
     const row = this.#store.get(
       "SELECT body FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.operation')='decision.raise' AND json_extract(body,'$.decision')=? ORDER BY seq LIMIT 1",
@@ -602,11 +614,13 @@ export class Broker {
       d.id,
     );
     const context = evidence ? JSON.parse(evidence.body) : null;
+    // Row 136 (#1630): bound to the role instance and its thread, not the run, so an approval
+    // outlives a restart. Single use is #consumeAuthority's.
     if (
       d.class !== cls ||
       d.action !== action ||
       d.raised_by_role !== s.role ||
-      d.raised_by_run !== s.run ||
+      this.#raisedThread(d) !== s.thread ||
       !context ||
       context.target !== target
     )
@@ -1112,7 +1126,9 @@ export class Broker {
         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');
+          // Row 136: the same role instance and thread as #checkAuthority's, so a later run supersedes.
+          if (old.raised_by_role !== s.role || this.#raisedThread(old) !== s.thread || this.#closed(old.id))
+            fail('supersede-refused');
           this.#store.run(
             'INSERT INTO decision_events(decision,at,op,by) VALUES(?,?,?,?)',
             old.id,

packages/bus/tests/approval-binding.test.mjs

@@ -0,0 +1,195 @@
+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';
+
+// Row 136 (#1630): an approval binds to the decision, the business and the role instance, plus
+// the thread when the raising run held one, not to the run that raised it. coder and coder2 are
+// two instances of one definition; assistant is a thread instance.
+const businesses = {
+  demo: {
+    id: 'demo',
+    human: 'jason',
+    arbiters: { technical: 'pm', delivery: 'pm' },
+    roles: {
+      coder: { definition: 'coder', authority: { withinRole: [], crossRole: [] } },
+      coder2: { definition: 'coder', authority: { withinRole: [], crossRole: [] } },
+      pm: { definition: 'pm', authority: { withinRole: [], crossRole: [] } },
+      assistant: { definition: 'assistant', authority: { withinRole: ['message.send'], crossRole: [] } },
+    },
+    definitions: { coder: {}, pm: {}, assistant: {} },
+  },
+};
+
+function setup(t) {
+  const root = mkdtempSync(join(tmpdir(), 'bus-binding-'));
+  let store, b;
+  const caps = {};
+  const ctx = {
+    get b() {
+      return b;
+    },
+    get store() {
+      return store;
+    },
+    caps,
+    call: (who, verb, args = {}) => b.request(caps[who], { verb, args }),
+    open(bs = businesses) {
+      store = new Store(root);
+      b = new Broker({ store, businesses: bs });
+      caps.human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true });
+    },
+    // A run of `role` that holds its claim; `thread` for the assistant.
+    run(name, role, thread) {
+      caps[name] = b.bindLaunch({ business: 'demo', role, run: name, harness: 'pi', ...(thread ? { thread } : {}) });
+      ctx.call(name, 'role.claim');
+      return caps[name];
+    },
+    end: (name) => b.endLaunch({ business: 'demo', run: name, reason: 'exited' }),
+    // A gated decision, so it routes to the human.
+    raise: (who, extra = {}) =>
+      ctx.call(who, 'decision.raise', {
+        action: 'deploy',
+        target: 'release1',
+        question: 'Ship release1?',
+        options: [
+          { key: 'yes', text: 'Yes' },
+          { key: 'no', text: 'No' },
+        ],
+        recommendation: 'yes',
+        blocking: false,
+        ...extra,
+      }),
+    approve: (d) => ctx.call('human', 'decision.resolve', { id: d.id, choice: 'yes' }),
+    use: (who, d) => b.authorize(caps[who], 'deploy', { decision: d.id, target: 'release1' }),
+    used: (d) =>
+      store.get(
+        "SELECT count(*) AS n FROM events WHERE kind='action.allowed' AND json_extract(body,'$.decision')=? AND json_extract(body,'$.operation') IS NOT 'decision.raise'",
+        d.id,
+      ).n,
+    ops: (d) => store.all('SELECT op FROM decision_events WHERE decision=? ORDER BY seq', d.id).map((r) => r.op),
+  };
+  ctx.open();
+  t.after(() => {
+    store.close();
+    rmSync(root, { recursive: true, force: true });
+  });
+  return ctx;
+}
+
+// Two assistant threads in one project.
+function threads(f) {
+  const project = f.call('human', 'project.create', { name: 'stack work' }).id;
+  const A = f.call('human', 'thread.create', { project, name: 'alpha' }).id;
+  const B = f.call('human', 'thread.create', { project, name: 'beta' }).id;
+  return { A, B };
+}
+
+test('REQ-DEC an approval outlives its run and a broker restart; a new run of the instance consumes it once', (t) => {
+  const f = setup(t);
+  f.run('run1', 'coder');
+  const d = f.raise('run1');
+  f.approve(d);
+  f.end('run1');
+  f.store.close();
+  f.open();
+  f.run('run2', 'coder');
+  assert.deepEqual(f.use('run2', d), { class: 'gated', decision: d.id });
+  assert.equal(f.used(d), 1);
+  // Once: neither the consuming run nor a later one uses it again.
+  assert.throws(() => f.use('run2', d), { code: 'decision-consumed' });
+  f.end('run2');
+  f.run('run3', 'coder');
+  assert.throws(() => f.use('run3', d), { code: 'decision-consumed' });
+  assert.equal(f.used(d), 1);
+});
+
+test('REQ-DEC another instance of the same definition cannot consume an approval, and its refusal consumes nothing', (t) => {
+  const f = setup(t);
+  f.run('run1', 'coder');
+  const d = f.raise('run1');
+  f.approve(d);
+  f.run('other', 'coder2');
+  assert.throws(() => f.use('other', d), { code: 'decision-mismatch' });
+  assert.equal(f.used(d), 0);
+  f.use('run1', d);
+  assert.equal(f.used(d), 1);
+});
+
+test('REQ-DEC REQ-THREAD-1 a thread approval is consumed only by a run holding the same thread', (t) => {
+  const f = setup(t);
+  const { A, B } = threads(f);
+  f.run('a1', 'assistant', A);
+  f.run('b1', 'assistant', B);
+  const d = f.raise('a1');
+  f.approve(d);
+  // The same instance in another thread.
+  assert.throws(() => f.use('b1', d), { code: 'decision-mismatch' });
+  assert.equal(f.used(d), 0);
+  f.end('a1');
+  f.run('a2', 'assistant', A);
+  f.use('a2', d);
+  assert.equal(f.used(d), 1);
+  assert.throws(() => f.use('a2', d), { code: 'decision-consumed' });
+});
+
+test('REQ-DEC a later run of the same instance supersedes an open decision; another instance cannot', (t) => {
+  const f = setup(t);
+  f.run('run1', 'coder');
+  const old = f.raise('run1');
+  f.end('run1');
+  f.run('run2', 'coder');
+  const replacement = f.raise('run2', { supersedes: old.id, question: 'Ship release1 tonight?' });
+  assert.equal(replacement.supersedes, old.id);
+  assert.deepEqual(f.ops(old), ['withdrawn']);
+  // The old decision is closed now, so superseding it again refuses.
+  assert.throws(() => f.raise('run2', { supersedes: old.id }), { code: 'supersede-refused' });
+  f.run('other', 'coder2');
+  assert.throws(() => f.raise('other', { supersedes: replacement.id }), { code: 'supersede-refused' });
+  assert.deepEqual(f.ops(replacement), []);
+  assert.equal(f.store.get('SELECT count(*) AS n FROM decisions').n, 2);
+});
+
+test('REQ-DEC REQ-THREAD-1 a thread decision is superseded only from the same thread', (t) => {
+  const f = setup(t);
+  const { A, B } = threads(f);
+  f.run('a1', 'assistant', A);
+  f.run('b1', 'assistant', B);
+  const old = f.raise('a1');
+  assert.throws(() => f.raise('b1', { supersedes: old.id }), { code: 'supersede-refused' });
+  assert.deepEqual(f.ops(old), []);
+  f.end('a1');
+  f.run('a2', 'assistant', A);
+  f.raise('a2', { supersedes: old.id });
+  assert.deepEqual(f.ops(old), ['withdrawn']);
+});
+
+// Before row 121 a thread instance's run held no thread, so a decision it raised has no thread
+// claim. The broker below reads assistant as a plain role to raise one the way such a run did.
+test('REQ-DEC REQ-THREAD-1 a thread instance decision raised before row 121 is not consumed or superseded by any thread run', (t) => {
+  const f = setup(t);
+  const { A } = threads(f);
+  f.store.close();
+  const plain = structuredClone(businesses);
+  plain.demo.roles.assistant.definition = 'assistant-before-121';
+  plain.demo.definitions['assistant-before-121'] = {};
+  f.open(plain);
+  f.run('old', 'assistant');
+  const d = f.raise('old');
+  f.approve(d);
+  // Left open: a resolved decision refuses supersede as closed, whatever its thread.
+  const open = f.raise('old', { question: 'Ship release2 tonight?' });
+  f.end('old');
+  assert.equal(f.store.get('SELECT count(*) AS n FROM thread_claims WHERE holder_run=?', 'old').n, 0);
+  f.store.close();
+  f.open();
+  f.run('a1', 'assistant', A);
+  assert.throws(() => f.use('a1', d), { code: 'decision-mismatch' });
+  assert.equal(f.used(d), 0);
+  assert.deepEqual(f.ops(d), ['resolved']);
+  assert.throws(() => f.raise('a1', { supersedes: open.id }), { code: 'supersede-refused' });
+  assert.deepEqual(f.ops(open), []);
+});

packages/bus/tests/broker.test.mjs

@@ -271,7 +271,7 @@ test('business isolation includes inherited object names and cross-business mess
   );
   assert.deepEqual(call(b, caps[1], 'trail', { subject: d.id }), []);
 });
-test('authority never transfers between action, run, target, unresolved or replaced role holder', (t) => {
+test('authority never transfers between action, target, unresolved or replaced role holder; a later run of the instance keeps it', (t) => {
   const { b, agent, human } = setup(t),
     c = agent('coder'),
     p = agent('pm');
@@ -284,7 +284,8 @@ test('authority never transfers between action, run, target, unresolved or repla
   call(b, c, 'role.release');
   const c2 = agent('coder', 'r2');
   call(b, c2, 'role.claim');
-  assert.throws(() => b.authorize(c2, 'deploy', { target: 'v1', decision: d.id }), /decision-mismatch/);
+  // Row 136 (#1630): the approval binds to the instance, not the run (approval-binding.test.mjs).
+  assert.equal(b.authorize(c2, 'deploy', { target: 'v1', decision: d.id }).decision, d.id);
   const revoke = raise(b, c2, 'role.revoke', { target: 'pm' });
   call(b, human, 'decision.resolve', { id: revoke.id, choice: 'yes' });
   call(b, p, 'role.release');

packages/bus/tests/single-use.test.mjs

@@ -104,21 +104,23 @@ test('gated approval authorizes once, survives store reopen, and fresh approval
   f.use(fresh);
   assert.equal(f.count(fresh), 1);
 });
-test('another run cannot consume an approval; a failed check leaves it usable', (t) => {
+// Row 136 (#1630): an approval binds to the role instance, so another run of it consumes it once.
+test('another run of the same instance consumes an approval once; a failed check leaves it usable', (t) => {
   const f = fixture(t),
     d = f.approve(),
     other = f.bind('run2');
   f.call(f.cap, 'role.release');
   f.call(other, 'role.claim');
-  assert.throws(() => f.use(d, other), /decision-mismatch/);
-  assert.equal(f.count(d), 0);
-  f.call(other, 'role.release');
-  f.call(f.cap, 'role.claim');
   assert.throws(
-    () => f.b.authorize(f.cap, 'deploy', { decision: d.id, target: 'other' }),
+    () => f.b.authorize(other, 'deploy', { decision: d.id, target: 'other' }),
     /decision-mismatch/,
   );
-  f.use(d);
+  assert.equal(f.count(d), 0);
+  f.use(d, other);
+  assert.equal(f.count(d), 1);
+  f.call(other, 'role.release');
+  f.call(f.cap, 'role.claim');
+  assert.throws(() => f.use(d), /decision-consumed/);
   assert.equal(f.count(d), 1);
 });
 test('two scheduled callers have exactly one grant and one consumed refusal', async (t) => {