stack/packages/bus/tests/review.test.mjs

// 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']],
  );
});