// 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, held: 'reviewer-run' }); 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$/); }); // Row 105 (#1599): the reply and the event say which run holds the reviewer role when the request is // routed, or null when nobody holds it. A null request waits for the next claim, as before. test('review.request says which run holds the reviewer role, or null, and a null request waits for a claim', (t) => { const { b, store, call, push, events } = setup(t); const routed = (mid) => store.all("SELECT op, holder_run FROM deliveries WHERE message=? AND op='routed' ORDER BY seq", mid).map((r) => ({ ...r })); push(A); const held = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }); assert.equal(held.held, 'reviewer-run'); assert.equal(JSON.parse(events('review.requested')[0].body).held, 'reviewer-run'); assert.deepEqual(routed(held.message), [{ op: 'routed', holder_run: 'reviewer-run' }]); // The reviewer releases the role. The latest role_claims row is a release, so nobody holds it. call('reviewer', 'role.release'); const waiting = call('coder', 'review.request', { task_ref: TASK, commit: A, to: 'reviewer' }); assert.equal(waiting.held, null); assert.equal(JSON.parse(events('review.requested')[1].body).held, null); assert.deepEqual(routed(waiting.message), [{ op: 'routed', holder_run: null }]); // A run that claims the role is routed the waiting request, as #routeWaiting did before row 105. const next = b.bindLaunch({ business: 'demo', role: 'reviewer', run: 'reviewer-2', harness: 'pi' }); b.request(next, { verb: 'role.claim', args: {} }); assert.deepEqual(routed(waiting.message), [ { op: 'routed', holder_run: null }, { op: 'routed', holder_run: 'reviewer-2' }, ]); }); 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']], ); });