diff --git a/packages/tasks/src/fields.mjs b/packages/tasks/src/fields.mjs index 15d8e4f47..1c1d61fbc 100644 --- a/packages/tasks/src/fields.mjs +++ b/packages/tasks/src/fields.mjs @@ -1,8 +1,8 @@ import { readFileSync } from 'node:fs'; import { TASK_VERBS } from '../../bus/src/broker.mjs'; -// Row 61 (#1550): the slice 1 field table as data, in ../fields.json. The prose in -// docs/plans/2026-10-04_slice-1.md stays the reviewed source; tests/fields.test.mjs checks both -// against what the verbs do. Two layers, as lead decision 113 rules: +// Row 61 (#1550): the slice 1 field table as data, in ../fields.json. The Field table in +// docs/plans/2026-10-04_slice-1.md is the reviewed prose it must match, and tests/fields.test.mjs +// checks that prose and this table against what the verbs do. Two layers, as lead decision 113 rules: // - `verbs`: the writer verbs.mjs checks before anything else. `pm` is the pm definition // (field-writer otherwise), `assigned` a role whose bot is an assignee (not-assigned // otherwise), `any` no writer check. diff --git a/packages/tasks/tests/fields.test.mjs b/packages/tasks/tests/fields.test.mjs index 59332d0fe..ef81f3385 100644 --- a/packages/tasks/tests/fields.test.mjs +++ b/packages/tasks/tests/fields.test.mjs @@ -9,11 +9,11 @@ import { FakeVikunja } from '../src/fake.mjs'; import { loadFields, checkFields, FieldTableError, FIELDS_FILE } from '../src/fields.mjs'; import { TASK_VERBS } from '../../bus/src/broker.mjs'; import { loadRole } from '../../business/src/index.mjs'; -// Row 61 (#1550): fields.json against what the verbs do. The writer layer is read from the table; -// whether a write needs a decision, and who resolves it, from roles/*.json as the business loader -// reads them (lead decision 113). A version 1 file, the researcher's, grants nothing. -// Neither expectation is written out here, so a change to the table, a role file or the verbs -// that leaves them disagreeing fails this file. +// Row 61 (#1550): fields.json, and the Field table in docs/plans/2026-10-04_slice-1.md, against what +// the verbs in src/verbs.mjs do, with the role files as given. The writer layer is read from the table; whether a write needs a decision, and who +// resolves it, from roles/*.json as the business loader reads them (lead decision 113). A version 1 +// file, the researcher's, grants nothing. Neither expectation is written out here, so a change to +// the table, the prose, a role file or the verbs that leaves them disagreeing fails this file. const table = loadFields(); const ROLES = fileURLToPath(new URL('../../../roles/', import.meta.url)); const authority = (r) => loadRole(ROLES, r).authority; @@ -26,6 +26,14 @@ test('the committed table loads frozen; its verbs are the broker task verbs', () assert.deepEqual(checkFields(raw()), raw()); }); +test('a write names a verb the table has as its own key, not one it inherits', () => { + for (const verb of ['constructor', 'toString']) { + const t = raw(); + t.fields[0].writes[0].verb = verb; + assert.throws(() => checkFields(t), tableError('fields[0].writes[0].verb: not a task verb'), verb); + } +}); + test('the schema refuses any other shape', () => { const bad = { 'an unknown top key': (t) => (t.writers = {}), @@ -80,6 +88,34 @@ test('the schema refuses any other shape', () => { } }); +test('the Field table in the slice 1 plan names the verbs each field is written by', () => { + const doc = readFileSync(new URL('../../../docs/plans/2026-10-04_slice-1.md', import.meta.url), 'utf8'); + const start = doc.indexOf('### Field table'); + const section = doc.slice(start, doc.indexOf('### Rows and order', start)); + // Header and separator aside, a row is Field, Writer, Action, Vikunja call. + const rows = section + .split('\n') + .filter((l) => l.startsWith('| ')) + .slice(1) + .map((l) => l.split('|').slice(1, -1).map((c) => c.trim())); + // Names compare without case, backticks or a plural s: "Assignees" is "assignee" in the prose. + const norm = (s) => s.toLowerCase().replace(/`/g, '').replace(/s$/, ''); + // Every name the prose gives must be a label in fields.json. A name with no label fails here and + // the message names it, so a renamed or invented field cannot pass by matching nothing. + const labels = new Set(table.fields.map((f) => norm(f.label))); + for (const [field] of rows) { + for (const name of field.split(', ')) { + assert.ok(labels.has(norm(name)), `the Field table names "${name}", which fields.json does not have`); + } + } + for (const f of table.fields) { + const hit = rows.filter(([field]) => field.split(', ').map(norm).includes(norm(f.label))); + assert.equal(hit.length, 1, `${f.field}: one row of the Field table`); + const named = [...hit[0][2].matchAll(/`(task\.[a-z.]+)`/g)].map((m) => m[1]); + assert.deepEqual([...new Set(named)].sort(), [...new Set(f.writes.map((w) => w.verb))].sort(), f.field); + } +}); + // A Vikunja write, under /api/v2, as the table names it, and the fields it writes. A bucket move // writes `done` when it moves into the done bucket and the bucket otherwise. const CALLS = [ @@ -114,12 +150,23 @@ test('each verb refuses, asks for a decision and writes as the table and the rol }; const dir = mkdtempSync(join(tmpdir(), 'fields-')); t.after(() => rmSync(dir, { recursive: true, force: true })); - // The role files' authority, and a researcher seat. + // The role files' authority, a researcher seat, and the cto as a tracker seat: the cto is the + // technical arbiter, and its own writes are walked like the others'. const w = await world(t, { fake, fetch, business: (b, { owner, project }) => { for (const r of ['pm', 'coder', 'reviewer', 'cto']) b.roles[r].authority = authority(r); + const ctoBot = fake.user('bot-demo-cto', { owner }); + fake.share(project, ctoBot, 1); + const ctoFile = join(dir, 'cto.token'); + writeFileSync(ctoFile, fake.token(ctoBot, SCOPES.worker) + '\n'); + chmodSync(ctoFile, 0o600); + b.roles.cto = { + ...b.roles.cto, + tracker: { bot: 'bot-demo-cto', botId: ctoBot }, + credentials: { vikunja: { file: ctoFile, expires: '2099-01-01' } }, + }; const bot = fake.user('bot-demo-researcher', { owner }); fake.share(project, bot, 1); const file = join(dir, 'researcher.token'); @@ -136,9 +183,8 @@ test('each verb refuses, asks for a decision and writes as the table and the rol assert.deepEqual(w.businesses.demo.roles.researcher.authority, { withinRole: [], crossRole: [] }); await w.adapter.start('demo'); assert.equal(w.adapter.status()[0].state, 'ready'); - const ROLE_NAMES = ['pm', 'coder', 'reviewer', 'researcher']; + const ROLE_NAMES = ['pm', 'coder', 'reviewer', 'researcher', 'cto']; const caps = Object.fromEntries(ROLE_NAMES.map((r) => [r, w.agent(r)])); - const cto = w.agent('cto'); const botId = (r) => w.businesses.demo.roles[r].tracker.botId; const done = w.installed.buckets.done; const seen = Object.fromEntries(TASK_VERBS.map((v) => [v, new Set()])); @@ -175,9 +221,12 @@ test('each verb refuses, asks for a decision and writes as the table and the rol }, }); // Cross-role goes to the technical arbiter; anything the role file doesn't list, to the human. - const route = a.crossRole.includes(verb) ? w.businesses.demo.arbiters.technical : 'human'; + // The arbiter's own cross-role decision goes to the human: the broker never routes a role's + // decision back to that role (broker.mjs, decision.raise). + const arbiter = w.businesses.demo.arbiters.technical; + const route = a.crossRole.includes(verb) && arbiter !== role ? arbiter : 'human'; assert.equal(w.store.get('SELECT route_to FROM decisions WHERE id=?', d.id).route_to, route, `${role} ${verb} route`); - w.broker.request(route === 'human' ? w.human : cto, { verb: 'decision.resolve', args: { id: d.id, choice: 'yes' } }); + w.broker.request(route === 'human' ? w.human : caps[route], { verb: 'decision.resolve', args: { id: d.id, choice: 'yes' } }); decision = d.id; } const r = await w.call(caps[role], verb, decision ? { ...args, decision } : args); @@ -207,11 +256,12 @@ test('each verb refuses, asks for a decision and writes as the table and the rol for (const [i, r] of ROLE_NAMES.entries()) await attempt(r, 'task.scope.change', { task_ref, title: `b${i}`, description: `what ${i}` }); for (const r of ['pm', 'coder', 'researcher']) await attempt(r, 'task.update.assigned', { task_ref, state: 'in-progress' }); await attempt('reviewer', 'task.update.assigned', { task_ref, percent_done: 0.5, state: 'in-progress', comment: 'started' }); - // People add the coder and the researcher in Vikunja; each may then write as the assigned role. + // People add the coder, the researcher and the cto in Vikunja; each may then write as the assigned role. const id = Number(task_ref.split('/')[1]); - for (const r of ['coder', 'researcher']) assert.equal((await w.ui('POST', `/tasks/${id}/assignees`, { user_id: botId(r) })).status, 201); + for (const r of ['coder', 'researcher', 'cto']) assert.equal((await w.ui('POST', `/tasks/${id}/assignees`, { user_id: botId(r) })).status, 201); await attempt('coder', 'task.update.assigned', { task_ref, percent_done: 0.7 }); await attempt('researcher', 'task.update.assigned', { task_ref, state: 'in-review' }); + await attempt('cto', 'task.update.assigned', { task_ref, percent_done: 0.8 }); await attempt('pm', 'task.close', { task_ref, verdict: 'row 61 r1' }); assert.equal(fake.task(id).done, true); for (const v of TASK_VERBS) assert.deepEqual([...seen[v]].sort(), expected(v), v); diff --git a/packages/webui/README.md b/packages/webui/README.md index 07caedf3a..de34660cf 100644 --- a/packages/webui/README.md +++ b/packages/webui/README.md @@ -519,7 +519,7 @@ GET `/api/task-fields` is row 61's read (#1550): the task field table, request. It is repository data, so it answers without `--business` or a system config. The answer is the table itself, with no `at`. A table that fails its check is refused whole with 503 `read-failed` and the loader's -message, which names the file and never a path. Any other method is 405. +message, which names the file or the key at fault, never a path. Any other method is 405. Console's shared CSS, Console CSS, brand.js and local Manrope fonts were copied unchanged from `agents/dewey/work/wui/`. Font license and source URLs accompany diff --git a/packages/webui/src/public/bus.js b/packages/webui/src/public/bus.js index b9f9b2e21..108df3b98 100644 --- a/packages/webui/src/public/bus.js +++ b/packages/webui/src/public/bus.js @@ -397,7 +397,7 @@ if (!fields.length) return `

${why(Object.assign(new ReadError('invalid-response', ''), { module: true }))}

`; const by = f => (Array.isArray(f.writes) ? f.writes : []).map(w => `${txt(WRITER[verbs[w.verb]] ?? 'unknown')} ${txt(w.verb)}`).join('
'); const rows = fields.map(f => `${txt(f.label)}${f.note ? `${txt(f.note)}` : ''}${by(f)}`).join(''); - return `
${rows}
FieldWriter and verb
`; + return `
${rows}
FieldWriter and verb
`; } async function taskView(get, ref) { const row = (await get('tasks')).find(r => r.task_ref === ref); diff --git a/packages/webui/tests/bus-browser.test.mjs b/packages/webui/tests/bus-browser.test.mjs index e9b2a9c8e..106a8dd26 100644 --- a/packages/webui/tests/bus-browser.test.mjs +++ b/packages/webui/tests/bus-browser.test.mjs @@ -4,7 +4,7 @@ // CLI or reaches a real broker; the seeding writes go straight to the broker. import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { copyFileSync, mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { copyFileSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { Store } from '../../bus/src/store.mjs'; @@ -71,12 +71,33 @@ function seed(root) { return { store, bus, calls, second, fail: code => { fail = code; }, ids: { old: old.id, open: open.id, routed: routed.id, resolved: resolved.id } }; } +// Row 61 (#1550): the README's account of a refused field table, pinned so the wording and the +// behavior it describes move together. reads.test.mjs checks the messages themselves. +test('the README says a refused field table names the file or the key at fault, never a path', () => { + const readme = readFileSync(new URL('../README.md', import.meta.url), 'utf8'); + assert.match(readme, /the loader's\s+message, which names the file or the key at fault, never a path\./); +}); + test('S5 pages in a browser: real broker rows, inert text, Copy, routes, failures, layout and no writes', { timeout: 180000 }, async () => { const root = mkdtempSync(join(tmpdir(), 'webui-bus-browser-')); const s = seed(root), f = await fixture(), b = await browser(); // Row 61 (#1550): a copy of the field table, so the test can break it and read it again. const fieldsFile = join(root, 'fields.json'); copyFileSync(FIELDS_FILE, fieldsFile); + // Hostile text in the comments row, as the table's own label and note: the label is at most + // 80 characters, so it carries an excerpt of HOSTILE; the note carries all of it. + const hostile = JSON.parse(readFileSync(fieldsFile, 'utf8')); + const comments = hostile.fields.find(x => x.field === 'comments'); + comments.label = `Comments ${HOSTILE.slice(34)}`; + comments.note = `Append only. ${HOSTILE}`; + // The label excerpt must keep each hostile class, so a later change to the slice cannot drop one + // without failing here. These assert on the data copied in, not on the rendered page. + assert.ok(comments.label.length <= 80, 'the label fits the 80-character cap'); + assert.ok(comments.label.includes(''), 'the label excerpt is shown'); + assert.ok(t7.includes(`Append only. ${HOSTILE.slice(0, 34)}`), 'the note is shown'); await inertCheck('task'); await noOverflow('task'); // A table that fails its check: the section says why, and the rest of the task page stays. writeFileSync(fieldsFile, '{"source":');