Candidate db463a7b…ab89 (5 files) on 277d1fa21. Adds the note-2 test for a thread
decision raised before row 121; mutants 6 of 6 killed after fixing the test to
supersede an open decision (first run left M6 alive, recorded). Owner's gate exit 0;
gate.sh now sets COMPOSE_PROJECT_NAME=dewey-r136 and removes its network at exit.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
363 lines
17 KiB
Diff
363 lines
17 KiB
Diff
diff --git a/packages/bus/README.md b/packages/bus/README.md
|
|
index f31c699a4..1b1d0dd0e 100644
|
|
--- a/packages/bus/README.md
|
|
+++ b/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
|
|
diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs
|
|
index 61febf532..a9c0a44d3 100644
|
|
--- a/packages/bus/src/broker.mjs
|
|
+++ b/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,
|
|
diff --git a/packages/bus/tests/approval-binding.test.mjs b/packages/bus/tests/approval-binding.test.mjs
|
|
new file mode 100644
|
|
index 000000000..0d10fb619
|
|
--- /dev/null
|
|
+++ b/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), []);
|
|
+});
|
|
diff --git a/packages/bus/tests/broker.test.mjs b/packages/bus/tests/broker.test.mjs
|
|
index cf2e269bd..6e5beea08 100644
|
|
--- a/packages/bus/tests/broker.test.mjs
|
|
+++ b/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');
|
|
diff --git a/packages/bus/tests/single-use.test.mjs b/packages/bus/tests/single-use.test.mjs
|
|
index 65e36292d..1ca595a79 100644
|
|
--- a/packages/bus/tests/single-use.test.mjs
|
|
+++ b/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) => {
|