diff --git a/agents/darkwing/work/slice1-s2c-review/comment.md b/agents/darkwing/work/slice1-s2c-review/comment.md new file mode 100644 index 00000000..2d7e8840 --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/comment.md @@ -0,0 +1,31 @@ +Row 44, S2c, round 1: **approve** (Darkwing) + +Candidate `candidate-manifest.sha256` `aa249ad9…`, three files under +`packages/bus/`. Full record: +`agents/darkwing/work/slice1-s2c-review/review-r1.md`. + +The patch applies at `83cfd18c` and the manifest checks 3/3. Tests pass +58/58 on Node 26 and on Node 24. + +- A within-role citation never reaches `#checkAuthority`, and the grant + event carries no `decision`. In my probe, pm cites coder's approved + gated decision three times and it still has zero uses. coder then uses + it once. +- Policy decides within-role, from the session's role. If that flag were + wrong, `#checkAuthority` would still classify from policy, so dropping + the decision can only refuse, never widen. +- Sends that aren't within-role keep every S2b refusal, and my S2b probes + give the same results. +- The README wording is fixed. + +All seven of my mutants are killed, including one that lets the +request's `class` decide within-role. + +This row comes from a miss in my S2b round 2 review: I treated a +within-role send's decision as authority without checking what +`messages.decision` means to the trail. Dewey caught it. + +Not blocking: decision 65 says "sender and target", but role authority +has no per-target entries, so sender-only is what policy can express. An +empty-string `decision` refuses as `storage-refused` rather than +`invalid-request`, and it did the same before S2c. diff --git a/agents/darkwing/work/slice1-s2c-review/mutations-s2c.py b/agents/darkwing/work/slice1-s2c-review/mutations-s2c.py new file mode 100644 index 00000000..35b2c5e2 --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/mutations-s2c.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# S2c round 1: Darkwing's mutants, repository layout, each compared to a clean baseline. +import pathlib,tempfile,shutil,subprocess,json,re,sys +cases=[ + ('citation-as-authority','broker.mjs',"decision: withinRole ? null : (a.decision ?? null),","decision: a.decision ?? null,"), + ('always-within-role','broker.mjs',"this.#classification(s, 'message.send') === 'within-role'","true"), + ('never-within-role','broker.mjs',"this.#classification(s, 'message.send') === 'within-role'","false"), + ('caller-class-decides','broker.mjs',"this.#classification(s, 'message.send') === 'within-role'","a.class === 'DECISION'"), + ('drop-existence-check','broker.mjs',"if (a.decision) this.#decision(s, a.decision);",""), + ('drop-consumed-check','broker.mjs',"fail('decision-consumed');",";"), + ('drop-class-match','broker.mjs',"d.class !== cls ||",""), +] +source=pathlib.Path(sys.argv[1]) +def run(dest): + r=subprocess.run(['node','--test',*[str(p) for p in sorted((dest/'tests').glob('*.test.mjs'))]],capture_output=True,text=True,timeout=300) + tests=set(l[2:].rsplit(' (',1)[0] for l in r.stdout.splitlines() if l.startswith('✖ ') and not l.startswith('✖ failing tests')) + return r.returncode,tests +with tempfile.TemporaryDirectory(prefix='dw-s2c-mut-') as root: + dest=pathlib.Path(root)/'packages'/'bus';shutil.copytree(source,dest,ignore=shutil.ignore_patterns('dw')) + code,base=run(dest);print(json.dumps({'baseline_exit':code,'baseline_failures':sorted(base)}),flush=True) + code,f=run(dest);print(json.dumps({'mutation':'no-op','exit':code,'killed':bool(f-base)}),flush=True) + out=[] + for name,file,old,new in cases: + p=dest/'src'/file;b=p.read_text();pat=r'\s*'.join(re.escape(c) for c in old if not c.isspace());n=len(re.findall(pat,b)) + if n!=1: print(json.dumps({'mutation':name,'error':f'{n} matches'}),flush=True);continue + p.write_text(re.sub(pat,lambda m:new,b,count=1)) + try: code,f=run(dest) + finally: p.write_text(b) + x=sorted(f-base);out.append({'mutation':name,'exit':code,'killed':bool(x),'new_failures':x}) + print(json.dumps(out[-1]),flush=True) + print('killed',sum(x['killed'] for x in out),'of',len(out)) diff --git a/agents/darkwing/work/slice1-s2c-review/mutations-s2c.txt b/agents/darkwing/work/slice1-s2c-review/mutations-s2c.txt new file mode 100644 index 00000000..c8ff1ccc --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/mutations-s2c.txt @@ -0,0 +1,10 @@ +{"baseline_exit": 0, "baseline_failures": []} +{"mutation": "no-op", "exit": 0, "killed": false} +{"mutation": "citation-as-authority", "exit": 1, "killed": true, "new_failures": ["within-role sends cite an open gated launch decision without spending it or naming it in grants"]} +{"mutation": "always-within-role", "exit": 1, "killed": true, "new_failures": ["cross-role sends still need a matching resolved decision and consume it once", "message.send consumes approval and prevents a later send or authorize"]} +{"mutation": "never-within-role", "exit": 1, "killed": true, "new_failures": ["within-role sends cite an open gated launch decision without spending it or naming it in grants"]} +{"mutation": "caller-class-decides", "exit": 1, "killed": true, "new_failures": ["cross-role sends still need a matching resolved decision and consume it once"]} +{"mutation": "drop-existence-check", "exit": 1, "killed": true, "new_failures": ["missing and foreign-business citations refuse and roll back message and grant"]} +{"mutation": "drop-consumed-check", "exit": 1, "killed": true, "new_failures": ["cross-role sends still need a matching resolved decision and consume it once", "failed commit rolls consumption back; cross-role consumes and within-role stays reusable", "gated approval authorizes once, survives store reopen, and fresh approval works", "message.send consumes approval and prevents a later send or authorize", "role.revoke consumes approval and prevents a later revoke or authorize", "two scheduled callers have exactly one grant and one consumed refusal", "within-role sends cite an open gated launch decision without spending it or naming it in grants"]} +{"mutation": "drop-class-match", "exit": 1, "killed": true, "new_failures": ["class drift cross-role to gated refuses before consumption", "class drift cross-role to within-role refuses before consumption", "class drift gated to cross-role refuses before consumption", "class drift gated to within-role refuses before consumption", "class drift within-role to cross-role refuses before consumption", "class drift within-role to gated refuses before consumption"]} +killed 7 of 7 diff --git a/agents/darkwing/work/slice1-s2c-review/node24.txt b/agents/darkwing/work/slice1-s2c-review/node24.txt new file mode 100644 index 00000000..f3584f0d --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/node24.txt @@ -0,0 +1,67 @@ +v24.21.0 +✔ launch identity is stamped, payload identity is refused and stale holder cannot send (235.92328ms) +✔ decision classes route from policy; gated resolution is human-only, choice and target must match (426.910198ms) +✔ claim exclusion, holder release, gated revoke and rerouting to a new holder are atomic (495.105331ms) +✔ launch events require a human CLI capability; generic emit cannot forge authority events (228.698793ms) +✔ within-role decisions close atomically and invalid options or blocking omissions refuse (213.364447ms) +✔ observer capabilities read human inbox but cannot mutate or forge launch identity (225.385799ms) +✔ task action subjects and linked decision trail are complete and ordered (288.189341ms) +✔ launch binding is durable and reconnecting requires the identical trusted record (115.454413ms) +✔ business isolation includes inherited object names and cross-business message references (237.185536ms) +✔ authority never transfers between action, run, target, unresolved or replaced role holder (339.545483ms) +✔ task projection uses schema current view, skipping earlier and equal-start polls (201.677639ms) +✔ revocation permanently bars the old run from reclaiming first, including after broker restart (224.355313ms) +✔ empty message references refuse before storage; refusal-evidence failure stays a typed error (162.265056ms) +✔ both arbiters require human resolution when their cross-role route is themselves (295.039131ms) +✔ S1 adapter takes resolved limits and refs, rejects mismatched instance, never mutates input (2.787893ms) +✔ only validated broker references load; returned data and exceptions cannot expose a known token (4.70321ms) +✔ bad file modes, symlinks, repository/data paths, malformed tokens and missing dates refuse (4.119351ms) +✔ expiry refuses use and env references never become client data (1.029855ms) +✔ S1 parsed service refs work, service mismatch refuses, Gitea rotation due is a warning state (1.699922ms) +✔ opaque tokens shorter than 16 characters refuse before use (0.358077ms) +✔ human proof binds CLI entry, process start and nonce; agents and incomplete ancestry refuse (3.73813ms) +✔ process reader gets own kernel identity without exposing environment values (0.776608ms) +✔ EACCES ancestor environments skip only markers; commands and registered launches still refuse (2.587563ms) +✔ real pid 1 remains inspectable when its environment is protected (0.494531ms) +✔ within-role sends cite an open gated launch decision without spending it or naming it in grants (337.195777ms) +✔ missing and foreign-business citations refuse and roll back message and grant (289.73417ms) +✔ cross-role sends still need a matching resolved decision and consume it once (454.4974ms) +✔ broker process binds trusted launches, offers reader capabilities, refuses human mutation, closes cleanly (319.212871ms) +✔ startup token refusal returns safe code without value or partial listening broker (33.355174ms) +✔ loaded fixture token is absent from socket replies and SQLite, including refusal evidence (268.632275ms) +✔ killed broker leaves an explicit stale lock; another process cannot silently reclaim it (247.294035ms) +✔ trusted host registers later launches; socket clients never have a registration verb (287.231207ms) +✔ runtime excludes declared project roots even when host supplies no repoRoots (37.381501ms) +✔ a refused launch binding leaves the broker and existing capabilities alive; bad protocol stops it (211.009213ms) +✔ v3b prototype refusals, views and append-only mutations (1621.484807ms) +✔ gated approval authorizes once, survives store reopen, and fresh approval works (408.772144ms) +✔ another run cannot consume an approval; a failed check leaves it usable (384.58817ms) +✔ two scheduled callers have exactly one grant and one consumed refusal (308.166326ms) +✔ failed commit rolls consumption back; cross-role consumes and within-role stays reusable (408.067494ms) +✔ class drift gated to cross-role refuses before consumption (276.232894ms) +✔ class drift cross-role to gated refuses before consumption (325.648398ms) +✔ class drift gated to within-role refuses before consumption (229.333491ms) +✔ class drift cross-role to within-role refuses before consumption (285.037582ms) +✔ class drift within-role to gated refuses before consumption (248.866242ms) +✔ class drift within-role to cross-role refuses before consumption (240.466824ms) +✔ message.send consumes approval and prevents a later send or authorize (236.059843ms) +✔ role.revoke consumes approval and prevents a later revoke or authorize (317.996245ms) +✔ creates private WAL store and excludes a second writer until explicit close (131.553502ms) +✔ rollback is atomic and schema metadata is checked against trusted DDL, not just itself (354.078946ms) +✔ existing empty database and symlink runtime directory refuse, never initialize over damage (315.784909ms) +✔ crash during a transaction recovers no partial event after explicit fixture-only lock removal (316.674129ms) +✔ writer refuses mixed at/read_at forms atomically, even through trusted SQL helpers (147.610319ms) +✔ async transactions refuse before invoking their function (105.656724ms) +✔ socket capability stamps launch identity; shared views use wire, no SQL client (254.245647ms) +✔ two wire claims serialize; a lost reply never automatically retries (270.023535ms) +✔ malformed, oversized and identity-forging envelopes refuse without echoing input (187.584216ms) +✔ client preserves UTF-8 when a response divides a multibyte character (11.863202ms) +✔ committed mutation followed by dropped reply reports unknown and is never retried (279.410147ms) +ℹ tests 58 +ℹ suites 0 +ℹ pass 58 +ℹ fail 0 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms 3767.887114 diff --git a/agents/darkwing/work/slice1-s2c-review/node26.txt b/agents/darkwing/work/slice1-s2c-review/node26.txt new file mode 100644 index 00000000..ace9935b --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/node26.txt @@ -0,0 +1,67 @@ +v26.8.1 +✔ launch identity is stamped, payload identity is refused and stale holder cannot send (309.288448ms) +✔ decision classes route from policy; gated resolution is human-only, choice and target must match (389.505343ms) +✔ claim exclusion, holder release, gated revoke and rerouting to a new holder are atomic (388.378687ms) +✔ launch events require a human CLI capability; generic emit cannot forge authority events (282.319597ms) +✔ within-role decisions close atomically and invalid options or blocking omissions refuse (342.637861ms) +✔ observer capabilities read human inbox but cannot mutate or forge launch identity (286.281563ms) +✔ task action subjects and linked decision trail are complete and ordered (234.634195ms) +✔ launch binding is durable and reconnecting requires the identical trusted record (139.318368ms) +✔ business isolation includes inherited object names and cross-business message references (233.819604ms) +✔ authority never transfers between action, run, target, unresolved or replaced role holder (392.107224ms) +✔ task projection uses schema current view, skipping earlier and equal-start polls (181.476509ms) +✔ revocation permanently bars the old run from reclaiming first, including after broker restart (248.26384ms) +✔ empty message references refuse before storage; refusal-evidence failure stays a typed error (175.355958ms) +✔ both arbiters require human resolution when their cross-role route is themselves (213.325796ms) +✔ S1 adapter takes resolved limits and refs, rejects mismatched instance, never mutates input (1.797616ms) +✔ only validated broker references load; returned data and exceptions cannot expose a known token (5.495212ms) +✔ bad file modes, symlinks, repository/data paths, malformed tokens and missing dates refuse (8.858866ms) +✔ expiry refuses use and env references never become client data (1.204726ms) +✔ S1 parsed service refs work, service mismatch refuses, Gitea rotation due is a warning state (1.723925ms) +✔ opaque tokens shorter than 16 characters refuse before use (0.334733ms) +✔ human proof binds CLI entry, process start and nonce; agents and incomplete ancestry refuse (2.786979ms) +✔ process reader gets own kernel identity without exposing environment values (1.719189ms) +✔ EACCES ancestor environments skip only markers; commands and registered launches still refuse (1.104467ms) +✔ real pid 1 remains inspectable when its environment is protected (0.472363ms) +✔ within-role sends cite an open gated launch decision without spending it or naming it in grants (358.694493ms) +✔ missing and foreign-business citations refuse and roll back message and grant (280.488708ms) +✔ cross-role sends still need a matching resolved decision and consume it once (378.131086ms) +✔ broker process binds trusted launches, offers reader capabilities, refuses human mutation, closes cleanly (306.608846ms) +✔ startup token refusal returns safe code without value or partial listening broker (38.704728ms) +✔ loaded fixture token is absent from socket replies and SQLite, including refusal evidence (253.957547ms) +✔ killed broker leaves an explicit stale lock; another process cannot silently reclaim it (266.396841ms) +✔ trusted host registers later launches; socket clients never have a registration verb (279.147369ms) +✔ runtime excludes declared project roots even when host supplies no repoRoots (42.325635ms) +✔ a refused launch binding leaves the broker and existing capabilities alive; bad protocol stops it (368.437951ms) +✔ v3b prototype refusals, views and append-only mutations (1715.387995ms) +✔ gated approval authorizes once, survives store reopen, and fresh approval works (418.109218ms) +✔ another run cannot consume an approval; a failed check leaves it usable (353.859553ms) +✔ two scheduled callers have exactly one grant and one consumed refusal (241.394104ms) +✔ failed commit rolls consumption back; cross-role consumes and within-role stays reusable (607.068845ms) +✔ class drift gated to cross-role refuses before consumption (316.906561ms) +✔ class drift cross-role to gated refuses before consumption (328.648705ms) +✔ class drift gated to within-role refuses before consumption (279.136459ms) +✔ class drift cross-role to within-role refuses before consumption (279.892283ms) +✔ class drift within-role to gated refuses before consumption (278.753046ms) +✔ class drift within-role to cross-role refuses before consumption (231.006303ms) +✔ message.send consumes approval and prevents a later send or authorize (240.480023ms) +✔ role.revoke consumes approval and prevents a later revoke or authorize (237.166599ms) +✔ creates private WAL store and excludes a second writer until explicit close (231.440902ms) +✔ rollback is atomic and schema metadata is checked against trusted DDL, not just itself (257.362193ms) +✔ existing empty database and symlink runtime directory refuse, never initialize over damage (295.830776ms) +✔ crash during a transaction recovers no partial event after explicit fixture-only lock removal (247.185327ms) +✔ writer refuses mixed at/read_at forms atomically, even through trusted SQL helpers (136.784102ms) +✔ async transactions refuse before invoking their function (172.440818ms) +✔ socket capability stamps launch identity; shared views use wire, no SQL client (287.255309ms) +✔ two wire claims serialize; a lost reply never automatically retries (263.102933ms) +✔ malformed, oversized and identity-forging envelopes refuse without echoing input (166.897513ms) +✔ client preserves UTF-8 when a response divides a multibyte character (11.875821ms) +✔ committed mutation followed by dropped reply reports unknown and is never retried (218.929616ms) +ℹ tests 58 +ℹ suites 0 +ℹ pass 58 +ℹ fail 0 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms 3912.090626 diff --git a/agents/darkwing/work/slice1-s2c-review/probes-s2c.test.mjs b/agents/darkwing/work/slice1-s2c-review/probes-s2c.test.mjs new file mode 100644 index 00000000..d12571c1 --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/probes-s2c.test.mjs @@ -0,0 +1,59 @@ +// Darkwing S2c probes. Not part of the candidate. +import test from 'node:test'; +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'; +const options = [{ key: 'yes', text: 'Allow' }, { key: 'no', text: 'Decline' }]; +const policy = () => ({ demo: { id: 'demo', human: 'jason', arbiters: { technical: 'cto', delivery: 'pm' }, roles: { + pm: { authority: { withinRole: ['message.send'], crossRole: [] } }, + cto: { authority: { withinRole: ['message.send'], crossRole: [] } }, + coder: { authority: { withinRole: [], crossRole: [] } } } } }); +const code = (f) => { try { const r = f(); return JSON.stringify(r?.id ? 'ok' : r); } catch (e) { return e.code ?? String(e.message); } }; +function setup(t) { + const root = mkdtempSync(join(tmpdir(), 'dw-s2c-')); + const store = new Store(root), b = new Broker({ store, businesses: policy() }); + t.after(() => { try { store.close(); } catch {} rmSync(root, { recursive: true, force: true }); }); + const human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true }); + const call = (cap, verb, args = {}) => b.request(cap, { verb, args }); + const bind = (role) => { const c = b.bindLaunch({ business: 'demo', role, run: role + '-run', harness: 'pi' }); call(c, 'role.claim'); return c; }; + const c = bind('coder'), pm = bind('pm'), cto = bind('cto'); + const uses = (id) => store.get("SELECT count(*) n FROM events WHERE kind='action.allowed' AND json_extract(body,'$.decision')=? AND json_extract(body,'$.operation') IS NOT 'decision.raise'", id).n; + return { b, store, human, call, c, pm, cto, uses }; +} +test('I another role cites an approved gated decision; the raiser still uses it once', (t) => { + const o = setup(t); + const d = o.call(o.c, 'decision.raise', { action: 'deploy', target: 'svc', question: 'Deploy?', options, recommendation: 'no', blocking: false }); + o.call(o.human, 'decision.resolve', { id: d.id, choice: 'yes' }); + for (let i = 0; i < 3; i++) console.log('I pm cites', i, code(() => o.call(o.pm, 'message.send', { to: 'cto', class: 'INFO', body: 'fyi', decision: d.id }))); + console.log('I uses after citations', o.uses(d.id)); + console.log('I pm authorize with it', code(() => o.b.authorize(o.pm, 'deploy', { decision: d.id, target: 'svc' }))); + console.log('I coder authorize', code(() => o.b.authorize(o.c, 'deploy', { decision: d.id, target: 'svc' }))); + console.log('I coder again', code(() => o.b.authorize(o.c, 'deploy', { decision: d.id, target: 'svc' }))); + console.log('I uses', o.uses(d.id)); + console.log('I decision trail messages', o.call(o.human, 'trail', { subject: d.id }).filter((r) => r.table === 'messages').length); +}); +test('J odd citation values on a within-role send', (t) => { + const o = setup(t); + for (const v of ['', 5, false, {}, 'x'.repeat(300)]) { + const r = code(() => o.call(o.pm, 'message.send', { to: 'cto', body: 'b', decision: v })); + const stored = o.store.get('SELECT decision FROM messages ORDER BY seq DESC LIMIT 1')?.decision; + console.log('J', JSON.stringify(v).slice(0, 20), r, 'last stored', JSON.stringify(stored)?.slice(0, 20)); + } +}); +test('K a gated sender (coder, no message.send) still needs and spends its approval', (t) => { + const o = setup(t); + console.log('K no decision', code(() => o.call(o.c, 'message.send', { to: 'pm', body: 'x' }))); + const other = o.call(o.c, 'decision.raise', { action: 'deploy', target: 'svc', question: 'Deploy?', options, recommendation: 'no', blocking: false }); + o.call(o.human, 'decision.resolve', { id: other.id, choice: 'yes' }); + console.log('K cites an approved deploy decision', code(() => o.call(o.c, 'message.send', { to: 'pm', body: 'x', decision: other.id }))); + console.log('K deploy uses', o.uses(other.id)); +}); +test('L human send with a citation', (t) => { + const o = setup(t); + const d = o.call(o.pm, 'decision.raise', { action: 'deploy', target: 'svc', question: 'Deploy?', options, recommendation: 'no', blocking: false }); + console.log('L human cites', code(() => o.call(o.human, 'message.send', { to: 'pm', body: 'x', decision: d.id }))); + console.log('L human missing', code(() => o.call(o.human, 'message.send', { to: 'pm', body: 'x', decision: 'nope' }))); + console.log('L uses', o.uses(d.id)); +}); diff --git a/agents/darkwing/work/slice1-s2c-review/probes-s2c.txt b/agents/darkwing/work/slice1-s2c-review/probes-s2c.txt new file mode 100644 index 00000000..57fa5209 --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/probes-s2c.txt @@ -0,0 +1,56 @@ +A raise class gated route_to human +A message.send 0 "string" +A message.send 1 decision-consumed +A message.send 2 decision-consumed +A action.allowed uses recorded 1 +A authorize after 3 sends decision-consumed +B role.revoke verb {"revoked":true} +B action.allowed uses recorded 1 +B authorize role.revoke after the verb decision-consumed +B authorize again decision-consumed +C raise class cross-role route_to cto +C current class is gated; authorize with arbiter approval: +C authorize 0 decision-mismatch +C authorize 1 decision-mismatch +C authorize 2 decision-mismatch +D authorize 0 {"class":"cross-role","decision":"a82fd4cb-b04e-42c3-8ac6-f3c83792d0e4"} +D authorize 1 decision-consumed +D authorize 2 decision-consumed +E second Store writer-locked +✔ A gated message.send approval: reuse through the verb, then authorize (146.708868ms) +✔ B role.revoke verb, then authorize with the same decision (134.530146ms) +✔ C cross-role approval after policy makes the action gated (129.665881ms) +✔ D cross-role approval reuse under unchanged policy (132.864553ms) +✔ E a second Store on the same data root refuses, so writers serialize in one process (61.074761ms) +I pm cites 0 "ok" +I pm cites 1 "ok" +I pm cites 2 "ok" +I uses after citations 0 +I pm authorize with it decision-mismatch +I coder authorize {"class":"gated","decision":"566bbf7a-2ffa-4abf-830f-a26e19f8bec8"} +I coder again decision-consumed +I uses 1 +I decision trail messages 3 +J "" storage-refused last stored undefined +J 5 decision-not-found last stored undefined +J false storage-refused last stored undefined +J {} decision-not-found last stored undefined +J "xxxxxxxxxxxxxxxxxxx decision-not-found last stored undefined +K no decision decision-required +K cites an approved deploy decision decision-mismatch +K deploy uses 0 +L human cites "ok" +L human missing decision-not-found +L uses 0 +✔ I another role cites an approved gated decision; the raiser still uses it once (181.561508ms) +✔ J odd citation values on a within-role send (175.824925ms) +✔ K a gated sender (coder, no message.send) still needs and spends its approval (167.861139ms) +✔ L human send with a citation (125.261302ms) +ℹ tests 9 +ℹ suites 0 +ℹ pass 9 +ℹ fail 0 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms 721.992715 diff --git a/agents/darkwing/work/slice1-s2c-review/review-r1.md b/agents/darkwing/work/slice1-s2c-review/review-r1.md new file mode 100644 index 00000000..16d07e8e --- /dev/null +++ b/agents/darkwing/work/slice1-s2c-review/review-r1.md @@ -0,0 +1,109 @@ +# Row 44, S2c message decision citations, round 1 review (Darkwing) + +Issue #1526. Candidate: Rocko's `candidate-manifest.sha256` (sha256 +`aa249ad9e0cae7dbc733727d664d22c2c0dd4c68469812702bd764e20292fef8`), three +files under `packages/bus/`. Packet `agents/rocko/work/slice1-s2c-r1/`, +`BUILD.md` sha256 `17d4f52d…`, `build.patch` sha256 `2c4f8d9f…`. Brief: +`docs/plans/2026-10-05_s2c-message-decision-citation.md`. Ruling: lead +decision 65 (dd35a0ab). Sage's request is comment 26775, rev 154. + +Verdict: **approve.** A within-role citation can't consume a decision, +can't authorize anything, and never appears as `decision` in an +`action.allowed` event. A send that isn't within-role keeps every S2b +refusal. The README is fixed. Two notes below, neither blocking. + +This row exists because of a miss in my S2b round 2 review. I saw that a +within-role send naming a decision now ran the full check, and I called +that failing closed. I didn't check what `messages.decision` means to the +trail and the inbox. Dewey did. + +## What I checked + +- The three packet hashes match. `build.patch` applies at `83cfd18c`; the + manifest checks 3/3. +- I read the broker and README diff, the new test file, the rest of the + `message.send` case, `#trail`, and the `messages` table in + `schema.sql`. +- Tests: 58/58 on Node 26.8.1 (`node26.txt`). In `node:24` (24.21.0), + with no network, a non-root UID and the package mounted read-only, also + 58/58 (`node24.txt`). + +## Sage's four points + +**A citation can't consume, authorize or show up in a grant.** On a +within-role send, the broker passes `decision: null` to +`#consumeAuthority`, so `#checkAuthority` returns `{class: 'within-role'}` +and the event body is `{action, class, target}`. The consumption query +looks only at `$.decision` in `action.allowed` events, so a citation +can't count as a use. In probe I, pm cites coder's approved gated +`deploy` decision three times, and the decision still has zero uses. pm's +own `authorize` with it refuses `decision-mismatch`, since pm didn't +raise it. coder then uses it once, and the second use refuses +`decision-consumed`. The decision's trail lists all three citing +messages. Rocko's test checks that no grant event has a `decision` key, +and it runs Dewey's case: an open gated `role.launch` cited twice, then +resolved, used once, and cited again after consumption. + +**Policy decides within-role, not the caller.** `withinRole` comes from +`#classification(s, 'message.send')`, which reads the business policy +for the session's role. No request field reaches it. Even a wrong +`withinRole` can only lose authority. `#checkAuthority` classifies again +from policy, so a cross-role or gated sender whose decision is dropped +gets `decision-required`. The `always-within-role` and +`caller-class-decides` mutants below both fail that way and are killed. + +**Sends that aren't within-role keep S2b.** Rocko's cross-role test gets +`decision-required`, `decision-mismatch`, `decision-not-approved`, one +send, then `decision-consumed`, and `authorize` afterwards refuses. My +S2b probes A to E give the same results as against S2b +(`probes-s2c.txt`). In probe K, a coder with no `message.send` authority +still gets `decision-required` without a decision. With an approved +decision for a different action, it gets `decision-mismatch`, and that +decision isn't spent. + +**README.** The "remain unchanged" sentence now covers only `authorize`. +A new paragraph says every agent send writes `action.allowed` with the +recipient role as subject, and it describes citation and authority as +decision 65 does. + +## Mutation evidence + +`mutations-s2c.py` runs seven mutants against the full test glob in the +repository layout, against a clean baseline (`mutations-s2c.txt`). The +no-op isn't killed, and all seven are. + +| Mutant | Killed by | +|---|---| +| within-role citation goes through the authority check | Dewey's case | +| every send treated as within-role | cross-role and S2b `message.send` tests | +| no send treated as within-role | Dewey's case | +| request `class: 'DECISION'` decides within-role | cross-role test | +| drop the existence check | missing and foreign-business test | +| drop `fail('decision-consumed')` | 7 tests | +| drop the class match | 6 tests | + +Without the existence check, the foreign key on `messages.decision` +still refuses a missing id, but a decision from another business would +be stored. The test catches that. + +## Notes, not blocking + +1. Decision 65 says "within-role for the sender and target". The broker + classifies `message.send` by sender only, because role authority has + no per-target entries. That matches what the policy can express. +2. Unchanged from S2b: an empty-string or boolean `decision` refuses as + `storage-refused` (the foreign key or the SQLite binding), not + `invalid-request` (probe J). It fails closed, but the code is vague. + An `identifier()` check would fix it when the file is next touched. + +## Files + +- `review-r1.md`, `comment.md` +- `node26.txt`, `node24.txt` +- `probes-s2c.test.mjs`: probes I to L +- `probes-s2c.txt`: S2b's probes A to E against this candidate, plus I + to L +- `mutations-s2c.py`, `mutations-s2c.txt` + +The probes run from a `dw/` directory beside `packages/bus/src`, with +`slice1-s2b-review/probes-s2b.test.mjs` copied beside them.