From 189410ed09d73937e9f0a72a465aae213cfd2a1f Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sun, 11 Oct 2026 21:03:37 -0500 Subject: [PATCH] bus: decisions read follow-up tests, seen-then-resolve, list-before-503 and full rows (#1560) Row 67 round 2 (rebase only), owner Filbert, candidate manifest 4387c8d5 on f38b7f46. Approved by Dewey (27648) and Darkwing (27649); round 1 by Dewey (27452) and Darkwing (27627). Sage's gate green 01:58-02:03Z. Co-Authored-By: Claude Opus 5.5 --- packages/bus/README.md | 4 +- packages/bus/src/broker.mjs | 5 +- packages/bus/tests/decisions.test.mjs | 92 +++++++++++++++++++++++---- packages/webui/tests/bus.test.mjs | 5 ++ 4 files changed, 91 insertions(+), 15 deletions(-) diff --git a/packages/bus/README.md b/packages/bus/README.md index 08be63070..114893605 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -235,7 +235,9 @@ decisions. It takes `{list}`, nothing else. `roles` lists the business's open decisions routed to a role, never to the human, oldest first. `recent` lists its last `RECENT` (50) closed decisions of every route, newest close first, each with `closed`, the closing `decision_events` row. Every row is the -decision as `trail` gives the same session for its id, so the lists add no +`inbox` view of the decision: the `decisions` row `trail` gives the same +session for its id, with `blocking` as a boolean and `authorization` from the +raise's `action.allowed` event, which `trail` also gives. So the lists add no data a reader couldn't already read one id at a time; they add the listing. `credentials` (row 60, #1549) is also for human and reader sessions only; a diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 5e86c60c7..5acc83292 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -861,7 +861,10 @@ export class Broker { return this.#trail(s, identifier(a.subject)); } // Row 57 (#1546): the Inbox's "Routed to roles" and "Recent" lists, for human and - // reader sessions only. Each row is what trail gives the same session for that id. + // reader sessions only. Each row is inbox's view of the decision: the `decisions` row trail + // gives the same session for that id, with blocking as a boolean, plus `authorization` from + // the raise's `action.allowed` event, which trail also gives. A `recent` row adds `closed`, + // the decision's closing `decision_events` row, as trail gives it. case 'decisions': { if (!s.human && !s.reader) fail('human-required'); keys(a, ['list'], ['list']); diff --git a/packages/bus/tests/decisions.test.mjs b/packages/bus/tests/decisions.test.mjs index 6626460b1..069e2ef4c 100644 --- a/packages/bus/tests/decisions.test.mjs +++ b/packages/bus/tests/decisions.test.mjs @@ -109,6 +109,39 @@ test('recent lists closed decisions of every route, newest close first, with how } }); +// Row 67 (#1560): seen, then closed, the usual order. `closed` is the closing event, not the first. +test('recent gives the closing event as closed when the decision was seen first', (t) => { + const { b, store, human, reader, coder, cto, pm, toCto, toPm, toHuman, within } = seed(t); + call(b, human, 'decision.seen', { id: toHuman.id }); + call(b, human, 'decision.resolve', { id: toHuman.id, choice: 'yes' }); + call(b, cto, 'decision.seen', { id: toCto.id }); + call(b, cto, 'decision.resolve', { id: toCto.id, choice: 'no' }); + call(b, pm, 'decision.seen', { id: toPm.id }); + call(b, coder, 'decision.withdraw', { id: toPm.id }); + const events = (id) => store.all('SELECT seq, op FROM decision_events WHERE decision=? ORDER BY seq', id); + for (const cap of [human, reader]) { + const rows = list(b, cap, 'recent'); + assert.deepEqual( + rows.map((r) => r.id), + [toPm.id, toCto.id, toHuman.id, within.id], + ); + for (const [d, op, by] of [ + [toHuman, 'resolved', 'jason'], + [toCto, 'resolved', 'demo-cto-run'], + [toPm, 'withdrawn', 'demo-coder-run'], + ]) { + const ev = events(d.id); + assert.deepEqual( + ev.map((e) => e.op), + ['seen', op], + 'seen, then closed', + ); + const { closed } = rows.find((r) => r.id === d.id); + assert.deepEqual([closed.op, closed.seq, closed.by], [op, ev[1].seq, by]); + } + } +}); + test('recent stops at RECENT rows', (t) => { const { b, reader, coder } = seed(t); const ids = []; @@ -122,22 +155,55 @@ test('recent stops at RECENT rows', (t) => { }); test('every row is what trail already gives the same session for that id', (t) => { - const { b, human, reader, toHuman } = seed(t); + const { b, human, reader, coder, toHuman } = seed(t); call(b, human, 'decision.resolve', { id: toHuman.id, choice: 'yes' }); + // Row 67 (#1560): fill the columns the seed leaves empty: project, requirement_ref, supersedes, + // and an approval choice other than the default. + const first = raise(b, coder, 'task.scope.change', { + domain: 'technical', + target: 'vikunja:3/5', + project: 'demo-web', + requirement_ref: 'REQ-DEC-1', + }); + raise(b, coder, 'task.scope.change', { + domain: 'technical', + target: 'vikunja:3/5', + task_ref: 'vikunja:3/5', + blocking: true, + supersedes: first.id, + approvalChoice: 'no', + }); for (const cap of [human, reader]) - for (const row of [...list(b, cap, 'roles'), ...list(b, cap, 'recent')]) { - const trail = call(b, cap, 'trail', { subject: row.id }); - const d = trail.find((r) => r.table === 'decisions' && r.id === row.id); - assert.ok(d, row.id); - for (const k of ['route_to', 'class', 'action', 'question', 'recommendation', 'raised_by_role', 'raised_by_run']) - assert.equal(d[k], row[k], k); - assert.deepEqual(d.options, row.options); - if (row.closed) { - const { table, ...closed } = trail.find((r) => r.table === 'decision_events' && r.seq === row.closed.seq); - assert.equal(table, 'decision_events'); - assert.deepEqual(closed, { ...row.closed }); + for (const which of ['roles', 'recent']) + for (const row of list(b, cap, which)) { + const trail = call(b, cap, 'trail', { subject: row.id }); + const found = trail.find((r) => r.table === 'decisions' && r.id === row.id); + assert.ok(found, row.id); + // The whole row: the view adds authorization (and closed on recent) and makes blocking + // a boolean. Every other column is trail's. + const { table, ...d } = found; + const { authorization, closed, ...rest } = row; + assert.deepEqual(rest, { ...d, blocking: !!d.blocking }, row.id); + const raised = trail.filter( + (r) => + r.table === 'events' && + r.kind === 'action.allowed' && + r.body.operation === 'decision.raise' && + r.body.decision === row.id, + ); + assert.equal(raised.length, 1, row.id); + assert.deepEqual(authorization, { + action: d.action, + target: raised[0].body.target, + approvalChoice: raised[0].body.approvalChoice, + }); + assert.equal(closed !== undefined, which === 'recent', row.id); + if (closed) { + const { table, ...event } = trail.find((r) => r.table === 'decision_events' && r.seq === closed.seq); + assert.equal(table, 'decision_events'); + assert.deepEqual(event, { ...closed }); + } } - } }); test("a role session can't list decisions, so it can't read another role's open decisions", (t) => { diff --git a/packages/webui/tests/bus.test.mjs b/packages/webui/tests/bus.test.mjs index 46d8016c3..a243ae7e1 100644 --- a/packages/webui/tests/bus.test.mjs +++ b/packages/webui/tests/bus.test.mjs @@ -191,6 +191,11 @@ test('bus refusals map to statuses and never carry a path or a stack', async t = assert.deepEqual([shape.status, (await shape.json()).error], [502, 'invalid-response']); const unset = await fetch(`http://127.0.0.1:${none.address().port}/api/bus/agents`); assert.deepEqual([unset.status, (await unset.json()).error], [503, 'not-configured']); + // Row 67 (#1560): with no bus, a bad argument is still a 400; the request is checked first. + for (const [query, status, error] of [['decisions?list=bad', 400, 'invalid-request'], ['decisions?list=roles', 503, 'not-configured'], ['trail?subject=%3C', 400, 'invalid-request']]) { + const r = await fetch(`http://127.0.0.1:${none.address().port}/api/bus/${query}`); + assert.deepEqual([query, r.status, (await r.json()).error], [query, status, error]); + } }); // A fake human CLI: it reads the request, then acts on args.mode.