Files
stack/packages/bus/tests/review.test.mjs
T
jason.woltjeandClaude Opus 5.5 4da6815293 review loop: working repositories, git socket, push and review verdicts, row 93 (#1587)
Commits move as v2 bundles over bus-host/git.sock (decision 135). The bus
host keeps one bare working repository per business and project, seeds main
from the project root under a per-repository lock, fetches a pushed bundle
with fsck into a temp ref, requires history shared with main, and moves the
task's work ref only as a fast-forward. Host and runner git run with no
system or global config, hooks at /dev/null and local transports only. The
socket binds only for roles holding git.push.working or review.verdict.

The broker records a push as the action.allowed for git.push.working with
its commit, takes review.request for the latest push and one review.verdict
per request, and refuses task.close on a pushed task without an approve
verdict for the latest push. recordEvent reserves both shapes, so agents
can't forge either.

Reviewed by Dewey (28196) and Sage (28197), both approve. Gate on 7a729aef:
14 node suites and 9 scripts green.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-13 10:48:34 -05:00

214 lines
12 KiB
JavaScript

// Row 93 (#1587): the review loop's broker side. The host's git socket
// checks a push (checkPush) and records it once the ref moved (recordPush), the
// coder asks for a review (review.request, a review.requested event),
// the reviewer's fetch is checked against that request (reviewFetch), the
// reviewer answers (review.verdict), and task.close on a pushed task needs an
// approve verdict for the latest push.
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';
const TASK = 'vikunja:1/2';
const A = 'a'.repeat(40);
const B = 'b'.repeat(40);
const businesses = {
demo: {
id: 'demo',
human: 'jason',
arbiters: { technical: 'cto', delivery: 'pm' },
projects: { stack: { root: '/nonexistent' } },
roles: {
pm: { authority: { withinRole: ['message.send', 'task.close'], crossRole: [] } },
cto: { authority: { withinRole: ['message.send', 'review.request'], crossRole: [] } },
coder: {
authority: { withinRole: ['task.update.assigned', 'git.push.working', 'review.request', 'message.send'], crossRole: [] },
},
reviewer: { authority: { withinRole: ['review.verdict', 'message.send', 'task.update.assigned'], crossRole: [] } },
second: { authority: { withinRole: ['review.verdict'], crossRole: [] } },
},
},
};
function setup(t) {
const root = mkdtempSync(join(tmpdir(), 'bus-review-'));
const store = new Store(root);
const b = new Broker({ store, businesses });
t.after(() => {
store.close();
rmSync(root, { recursive: true, force: true });
});
const caps = {};
for (const role of Object.keys(businesses.demo.roles)) {
caps[role] = b.bindLaunch({ business: 'demo', role, run: `${role}-run`, harness: 'pi' });
b.request(caps[role], { verb: 'role.claim', args: {} });
}
const call = (role, verb, args) => b.request(caps[role], { verb, args });
// What the git socket does for a push: check, then record once the ref moved.
const push = (commit, previous = null, task_ref = TASK) => {
b.checkPush(caps.coder, task_ref);
return b.recordPush(caps.coder, { task_ref, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit, previous });
};
const events = (kind) => store.all('SELECT * FROM events WHERE kind=? ORDER BY seq', kind);
const pushes = () => events('action.allowed').filter((e) => JSON.parse(e.body).action === 'git.push.working');
return { b, store, caps, call, push, events, pushes };
}
test('a push is recorded as the action.allowed for git.push.working on the task, and only the host records one', (t) => {
const { b, caps, push, events, pushes } = setup(t);
// The check before the host takes the bundle records nothing unless it refuses.
b.checkPush(caps.coder, TASK);
assert.equal(events('action.allowed').length, 0);
assert.throws(() => b.checkPush(caps.reviewer, TASK), /decision-required/);
assert.throws(() => b.checkPush(caps.coder, 'gitea:1'), /invalid-request/);
assert.deepEqual(push(A), { task_ref: TASK, commit: A });
const [e] = pushes();
assert.equal(e.subject, TASK);
assert.equal(e.actor_role, 'coder');
assert.deepEqual(JSON.parse(e.body), {
project: 'stack',
branch: 'feat',
ref: 'work/vikunja-1-2',
commit: A,
previous: null,
action: 'git.push.working',
class: 'within-role',
target: TASK,
});
// A role without the action can't have a push recorded either.
assert.throws(
() => b.recordPush(caps.reviewer, { task_ref: TASK, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit: B, previous: A }),
/decision-required/,
);
const good = { task_ref: TASK, project: 'stack', branch: 'feat', ref: 'work/vikunja-1-2', commit: B, previous: A };
for (const bad of [{ ...good, project: 'other' }, { ...good, commit: 'x' }, { ...good, previous: 'x' }, { ...good, task_ref: 'gitea:1' }]) assert.throws(() => b.recordPush(caps.coder, bad), /invalid-request/);
const { previous, ...short } = good;
assert.throws(() => b.recordPush(caps.coder, short), /invalid-request/);
for (const kind of ['review.requested', 'review.verdict'])
assert.throws(() => b.recordEvent(caps.coder, { kind, body: { task_ref: TASK }, subject: TASK }), /reserved-event/);
assert.throws(
() => b.recordEvent(caps.coder, { kind: 'action.allowed', body: { action: 'git.push.working', target: TASK, commit: B }, subject: TASK }),
/reserved-event/,
);
assert.equal(pushes().length, 1);
});
test('review.request names the latest pushed commit and sends the reviewer a REVIEW-REQUEST', (t) => {
const { store, call, push, events } = setup(t);
push(A);
const r = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer', note: 'please' });
assert.equal(r.commit, A);
assert.equal(r.ref, 'work/vikunja-1-2');
const [e] = events('review.requested');
assert.equal(e.id, r.request);
assert.equal(e.subject, TASK);
assert.deepEqual(JSON.parse(e.body), { task_ref: TASK, project: 'stack', ref: 'work/vikunja-1-2', commit: A, to: 'reviewer', message: r.message });
const m = store.get('SELECT * FROM messages WHERE id=?', r.message);
assert.equal(m.to_role, 'reviewer');
assert.equal(m.from_role, 'coder');
assert.equal(m.class, 'REVIEW-REQUEST');
assert.match(m.body, new RegExp(`Review request ${r.request} for vikunja:1/2: commit ${A}`));
assert.match(m.body, /please$/);
});
test('review.request refuses a commit that isn\'t the latest push, a non-reviewer and a role without the action', (t) => {
const { call, push, events } = setup(t);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }), /unknown-commit/);
push(A);
push(B, A);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }), /stale-commit/);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: 'c'.repeat(40), to: 'reviewer' }), /unknown-commit/);
assert.throws(() => call('coder', 'review.request', { task_ref: 'vikunja:1/3', commit: B, to: 'reviewer' }), /unknown-commit/);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'nobody' }), /unknown-role/);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'pm' }), /not-a-reviewer/);
assert.throws(() => call('cto', 'review.request', { task_ref: TASK, commit: B, to: 'cto' }), /self-review/);
assert.throws(() => call('reviewer', 'review.request', { task_ref: TASK, commit: B, to: 'second' }), /decision-required/);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: 'B', to: 'reviewer' }), /invalid-request/);
assert.throws(() => call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer', extra: 1 }), /invalid-request/);
assert.equal(events('review.requested').length, 0);
// A refusal rolls its action.allowed back, and is recorded as action.refused.
assert.equal(events('action.allowed').filter((e) => JSON.parse(e.body).action === 'review.request').length, 0);
assert.ok(events('action.refused').length >= 10);
// Another role that holds review.request asks too.
assert.equal(call('cto', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer' }).commit, B);
});
test('reviewFetch answers only the addressed reviewer, for the request it cites', (t) => {
const { b, caps, call, push } = setup(t);
push(A);
const { request } = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
assert.deepEqual(b.reviewFetch(caps.reviewer, { task_ref: TASK, request }), { project: 'stack', commit: A, ref: 'work/vikunja-1-2' });
assert.throws(() => b.reviewFetch(caps.coder, { task_ref: TASK, request }), /not-authorized/);
assert.throws(() => b.reviewFetch(caps.pm, { task_ref: TASK, request }), /not-authorized/);
assert.throws(() => b.reviewFetch(caps.second, { task_ref: TASK, request }), /request-not-found/);
assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: 'vikunja:1/3', request }), /request-not-found/);
assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: TASK, request: 'no-such-request' }), /request-not-found/);
assert.throws(() => b.reviewFetch(caps.reviewer, { task_ref: TASK }), /invalid-request/);
});
test('review.verdict answers one request once, to the role that asked', (t) => {
const { store, call, push, events } = setup(t);
push(A);
const req = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
assert.throws(() => call('coder', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'mine' }), /decision-required/);
assert.throws(() => call('second', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'not mine' }), /request-not-found/);
assert.throws(() => call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'maybe', body: 'x' }), /invalid-request/);
const v = call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'changes', body: 'fix the test' });
assert.equal(v.commit, A);
const [e] = events('review.verdict');
assert.equal(e.id, v.verdict);
assert.deepEqual(JSON.parse(e.body), { task_ref: TASK, request: req.request, commit: A, verdict: 'changes', message: v.message });
const m = store.get('SELECT * FROM messages WHERE id=?', v.message);
assert.equal(m.to_role, 'coder');
assert.equal(m.class, 'REVIEW-RESULT');
assert.equal(m.in_reply_to, req.message);
assert.match(m.body, /: changes\. A task_close cites this verdict id\.\n\nfix the test$/);
assert.throws(() => call('reviewer', 'review.verdict', { task_ref: TASK, request: req.request, verdict: 'approve', body: 'changed my mind' }), /verdict-exists/);
assert.equal(events('review.verdict').length, 1);
});
test('task.close on a pushed task needs an approve verdict for the latest push', (t) => {
const { b, caps, call, push, events } = setup(t);
const close = (verdict, target = TASK) => b.authorize(caps.pm, 'task.close', { target, verdict });
// No push: the verdict is a citation the broker doesn't check.
assert.equal(close('anything').class, 'within-role');
push(A);
const allowed = () => events('action.allowed').filter((e) => JSON.parse(e.body).action === 'task.close').length;
const before = allowed();
assert.throws(() => close(undefined), /verdict-not-found/);
assert.throws(() => close('no-such-verdict'), /verdict-not-found/);
const r1 = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
const changes = call('reviewer', 'review.verdict', { task_ref: TASK, request: r1.request, verdict: 'changes', body: 'no' });
assert.throws(() => close(changes.verdict), /verdict-not-approve/);
// A verdict on another task doesn't count for this one.
push(A, null, 'vikunja:1/3');
const other = call('coder', 'review.request', { task_ref: 'vikunja:1/3', commit: A, to: 'reviewer' });
const otherOk = call('reviewer', 'review.verdict', { task_ref: 'vikunja:1/3', request: other.request, verdict: 'approve', body: 'ok' });
assert.throws(() => close(otherOk.verdict), /verdict-not-found/);
const r2 = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' });
const approve = call('reviewer', 'review.verdict', { task_ref: TASK, request: r2.request, verdict: 'approve', body: 'ok' });
// A push after the approval makes it stale.
push(B, A);
assert.throws(() => close(approve.verdict), /verdict-stale/);
assert.equal(allowed(), before, 'each refused close rolled its action.allowed back');
const r3 = call('coder', 'review.request', { task_ref: TASK, commit: B, to: 'reviewer' });
const fresh = call('reviewer', 'review.verdict', { task_ref: TASK, request: r3.request, verdict: 'approve', body: 'ok' });
assert.equal(close(fresh.verdict).class, 'within-role');
assert.equal(allowed(), before + 1);
});
test('a refusal the host decided is filed on the task it names, and only a task ref counts', (t) => {
const { b, caps, events } = setup(t);
assert.equal(b.refuse(caps.coder, 'non-fast-forward', TASK), 'non-fast-forward');
assert.equal(b.refuse(caps.coder, 'invalid-request', 'gitea:1'), 'invalid-request');
assert.equal(b.refuse(caps.coder, 'invalid-request'), 'invalid-request');
assert.deepEqual(
events('action.refused').map((e) => [e.subject, JSON.parse(e.body).code]),
[[TASK, 'non-fast-forward'], [null, 'invalid-request'], [null, 'invalid-request']],
);
});