diff --git a/packages/cli/src/config.mjs b/packages/cli/src/config.mjs index f17368306..816fa6447 100644 --- a/packages/cli/src/config.mjs +++ b/packages/cli/src/config.mjs @@ -86,7 +86,7 @@ function trackerFor(system, b, warn, dataRoot) { if (dataRoot && managedState({ dataRoot, business: b.id, project: id, entry }) === 'reserved') continue; if (!existsSync(projectFilePath(entry.root))) continue; const project = loadProject(entry.root); - if (project.id !== id) throw new CliError(`project file ${project.file} has id ${project.id}, but business ${b.id} declares it as ${id}`, 3); + if (project.id !== id) throw new CliError(`project file ${project.file} has id that does not match the declared project key`, 3); const vars = resolveInstance({ system, business: b, project, instance: first }).vars; if (vars["tracker.project"] !== undefined) named.push({ id, vars }); } diff --git a/packages/cli/tests/access.test.mjs b/packages/cli/tests/access.test.mjs index 1e0eba7d9..30b6ccdc2 100644 --- a/packages/cli/tests/access.test.mjs +++ b/packages/cli/tests/access.test.mjs @@ -353,7 +353,7 @@ test("T2 a bad user add refuses before any write", async (t) => { } const checked = [ [USER_ADD("bob", { subject: "github:bob-sub" }), /provider must be a configured provider/], - [USER_ADD("alice", { subject: "authentik:alice-2" }), /duplicate user id: alice/], + [USER_ADD("alice", { subject: "authentik:alice-2" }), /duplicate user id/], [USER_ADD("tester"), /a bus principal/], [USER_ADD("jason"), /a business's human/], [USER_ADD("bob", { subject: "authentik:jason-sub" }), /owner sign-in subject/], diff --git a/packages/cli/tests/last-values.test.mjs b/packages/cli/tests/last-values.test.mjs new file mode 100644 index 000000000..84a294c98 --- /dev/null +++ b/packages/cli/tests/last-values.test.mjs @@ -0,0 +1,91 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { spawnSync } from 'node:child_process'; +import { fixture } from './helpers.mjs'; +import { runConfigCommand } from '../../organization/src/config-command.mjs'; +const repo = join(import.meta.dirname, '../../..'); +const task = () => ({ taskVersion: 1, id: 'valid', prompt: 'hello' }); +for (const [field, edit] of [ + ['task id', d => { d.id = secret; }], + ['taskVersion', d => { d.taskVersion = secret; }], + ['tool', d => { d.capabilities = { tools: [secret] }; }], + ['mission id', (d, root) => { d.mission = 'mission.json'; writeFileSync(join(root, d.mission), JSON.stringify({ missionVersion: 1, id: secret, objective: 'hello' })); }], + ['missionVersion', (d, root) => { d.mission = 'mission.json'; writeFileSync(join(root, d.mission), JSON.stringify({ missionVersion: secret, id: 'valid', objective: 'hello' })); }], + ['mission tool', (d, root) => { d.mission = 'mission.json'; writeFileSync(join(root, d.mission), JSON.stringify({ missionVersion: 1, id: 'valid', objective: 'hello', capabilities: { tools: [secret] } })); }], +]) test(`task validation ${field} omits file value and keeps exit 2`, t => { + const root = scratch(t), d = task(); edit(d, root); const file = join(root, 'task.json'); writeFileSync(file, JSON.stringify(d)); + const r = spawnSync(process.execPath, [join(repo, 'scripts/mosaic-task.mjs'), 'validate', file], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 2, r.stderr); assert.ok(r.stderr.includes(field), r.stderr); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/); +}); +const configCases = [ + ...['configVersion', 'environment', 'dataRoot'].map(k => [k, d => { d[k] = secret; }]), + ...['backend', 'provider', 'model', 'adapter'].map(k => [`execution.${k}`, d => { d.execution[k] = secret + '\n'; }]), + ['federation.self', d => { d.federation = { self: secret, peers: [] }; }], + ['federation.peers[0].id', d => { d.federation = { self: 'self', peers: [{ id: secret, origin: 'https://peer.example', label: 'Peer' }] }; }], + ['federation.peers[0].origin', d => { d.federation = { self: 'self', peers: [{ id: 'peer', origin: secret, label: 'Peer' }] }; }], +]; +for (const [field, edit] of configCases) test(`config ${field} omits the configured value`, t => { + const root = scratch(t), f = fixture(root), d = JSON.parse(readFileSync(f.config, 'utf8')); edit(d); writeFileSync(f.config, JSON.stringify(d)); + const r = spawnSync(process.execPath, [join(repo, 'scripts/mosaic-config.mjs'), 'validate'], { encoding: 'utf8', env: { ...f.env, PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 2, r.stderr); assert.ok(r.stderr.includes(field), r.stderr); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/); +}); +test('config-command refuses system config without relaying stderr file values', t => { + const root = scratch(t), f = fixture(root), d = JSON.parse(readFileSync(f.config, 'utf8')); d.execution.model = secret + '\n'; writeFileSync(f.config, JSON.stringify(d)); + let err = ''; const status = runConfigCommand(['validate', 'acme'], { env: f.env, stdout: { write() {} }, stderr: { write(s) { err += s; } } }); + assert.equal(status, 3); assert.match(err, /system config problem \(mosaic-config exit 2\)/); assert.ok(err.includes(f.config), err); assert.doesNotMatch(err, /sk-test|NOT-A-SECRET/); assert.match(err, /execution.model/); assert.match(err, /expected a 1-200 character name/); +}); +test('project id mismatch omits the file value in config-command', t => { + const root = scratch(t), f = fixture(root), file = join(f.doc.projects.stack.root, '.mosaic', 'project.json'); + mkdirSync(join(f.doc.projects.stack.root, '.mosaic')); + writeFileSync(file, JSON.stringify({ projectVersion: 1, id: secret.toLowerCase() })); + let err = ''; const status = runConfigCommand(['validate', 'acme'], { env: f.env, stdout: { write() {} }, stderr: { write(s) { err += s; } } }); + assert.equal(status, 2); assert.ok(err.includes(file), err); assert.match(err, /id/); assert.doesNotMatch(err, /sk-test|NOT-A-SECRET/); +}); +for (const [field, edit] of [ + ['dataRoot', d => { d.dataRoot = `/x/${secret}/../y`; }], + ['federation.peers[0].id', d => { d.federation = { self: secret.toLowerCase(), peers: [{ id: secret.toLowerCase(), origin: 'https://peer.example', label: 'Peer' }] }; }], +]) test(`config ${field} repeated/canonical value is omitted`, t => { + const root = scratch(t), f = fixture(root), d = JSON.parse(readFileSync(f.config, 'utf8')); edit(d); writeFileSync(f.config, JSON.stringify(d)); + const r = spawnSync(process.execPath, [join(repo, 'scripts/mosaic-config.mjs'), 'validate'], { encoding: 'utf8', env: { ...f.env, PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 2); assert.ok(r.stderr.includes(field), r.stderr); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/); +}); +for (const mission of [false, true]) test(`duplicate ${mission ? 'mission' : 'task'} tool omits the validated value`, t => { + const root = scratch(t), d = task(); + if (mission) { d.mission = 'mission.json'; writeFileSync(join(root, d.mission), JSON.stringify({ missionVersion: 1, id: 'valid', objective: 'hello', capabilities: { tools: ['read', 'read'] } })); } + else d.capabilities = { tools: ['read', 'read'] }; + const file = join(root, 'task.json'); writeFileSync(file, JSON.stringify(d)); + const r = spawnSync(process.execPath, [join(repo, 'scripts/mosaic-task.mjs'), 'validate', file], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 2); assert.match(r.stderr, /duplicate tool in/); assert.doesNotMatch(r.stderr, /read/); +}); +for (const childStderr of [secret, `mosaic-config: execution.model must match ${secret}`]) test(`config-command does not relay arbitrary child stderr${childStderr.startsWith('mosaic-config:') ? ' after a known field' : ''}`, t => { + const root = scratch(t), file = join(root, 'config.json'); + const code = `import cp from 'node:child_process'; import { syncBuiltinESMExports } from 'node:module'; +cp.spawnSync = () => ({ status: 2, stderr: ${JSON.stringify(childStderr)}, stdout: '' }); syncBuiltinESMExports(); +const { runConfigCommand } = await import(${JSON.stringify(new URL('../../organization/src/config-command.mjs', import.meta.url).href)}); +process.exitCode = runConfigCommand(['validate', 'acme'], { env: { MOSAIC_CONFIG: ${JSON.stringify(file)} } });`; + const r = spawnSync(process.execPath, ['--input-type=module', '-e', code], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 3, r.stderr); assert.ok(r.stderr.includes(file), r.stderr); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/); +}); + +import { bootConfig, loadSystem } from '../src/config.mjs'; +import { CliError } from '../src/errors.mjs'; +test('CLI boot project id mismatch omits the configured value and keeps exit 3', t => { + const root = scratch(t), f = fixture(root), file = join(f.doc.projects.stack.root, '.mosaic', 'project.json'); + mkdirSync(join(f.doc.projects.stack.root, '.mosaic')); + writeFileSync(file, JSON.stringify({ projectVersion: 1, id: secret.toLowerCase() })); + assert.throws(() => bootConfig({ system: loadSystem({ env: f.env }), businessId: 'acme', env: f.env }), safe(file, CliError, { exitCode: 3 }, secret.toLowerCase())); +}); diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs index fce460a15..6076f52ee 100644 --- a/packages/discord/src/binding.mjs +++ b/packages/discord/src/binding.mjs @@ -96,7 +96,7 @@ function onlyKeys(obj, allowed, where) { function requireString(obj, key, where, pattern, what) { const v = obj[key]; if (typeof v !== "string" || v.length === 0) throw new DiscordError(`${where}: ${key} must be a non-empty string`); - if (pattern && !pattern.test(v)) throw new DiscordError(`${where}: ${key} is not ${what}: ${JSON.stringify(v)}`); + if (pattern && !pattern.test(v)) throw new DiscordError(`${where}: ${key} is not ${what}`); return v; } @@ -148,7 +148,7 @@ export function validateBinding(raw, where = "binding") { if (!Array.isArray(u.channels) || u.channels.length === 0) throw new DiscordError(`${w}: channels must be a non-empty array of listed channel ids`); allowed = u.channels.map((cid, j) => { if (typeof cid !== "string" || !SNOWFLAKE.test(cid)) throw new DiscordError(`${w}.channels[${j}]: not a Discord snowflake id`); - if (!channels.some((c) => c.id === cid)) throw new DiscordError(`${w}.channels[${j}]: ${cid} is not a listed channel`); + if (!channels.some((c) => c.id === cid)) throw new DiscordError(`${w}.channels[${j}]: is not a listed channel`); return cid; }); if (new Set(allowed).size !== allowed.length) throw new DiscordError(`${w}: duplicate channel id`); @@ -204,7 +204,7 @@ export function validateBinding(raw, where = "binding") { const rname = requireString(r, "name", w, ROOT_NAME, "a root name"); const rpath = requireString(r, "path", w); if (!isAbsolute(rpath) || rpath.includes("\0")) throw new DiscordError(`${w}: path must be an absolute path`); - if (rpath.split(sep).some((seg) => seg.startsWith(".") && seg.length > 0)) throw new DiscordError(`${w}: path must not have a dot-prefixed segment (${rpath})`); + if (rpath.split(sep).some((seg) => seg.startsWith(".") && seg.length > 0)) throw new DiscordError(`${w}: path must not have a dot-prefixed segment`); if (resolve(rpath) === sep || resolve(rpath) === homedir()) throw new DiscordError(`${w}: path must not be the filesystem root or the home directory`); if (r.write !== undefined && r.write !== true && r.write !== false) throw new DiscordError(`${w}: write must be true or false`); let git = null; @@ -343,7 +343,7 @@ function setsparkConfig(raw, where, users) { const approvers = {}; for (const u of users) { const name = u.name.trim().toLowerCase(); - if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`); + if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users share a name; approver names must be distinct (case-insensitive)`); approvers[name] = u.id; } try { diff --git a/packages/discord/src/gateway.mjs b/packages/discord/src/gateway.mjs index 14655e096..67f5a4f86 100644 --- a/packages/discord/src/gateway.mjs +++ b/packages/discord/src/gateway.mjs @@ -46,7 +46,7 @@ export function createGateway({ random = Math.random, properties = { os: process.platform, browser: "mosaic-stack", device: "mosaic-stack" }, } = {}) { - if (typeof url !== "string" || !url.startsWith("wss://")) throw new DiscordError(`gateway: url must be wss://, got ${JSON.stringify(url)}`, 1); + if (typeof url !== "string" || !url.startsWith("wss://")) throw new DiscordError(`gateway: url must be wss://`, 1); if (typeof token !== "string" || token.length === 0) throw new DiscordError("gateway: token required", 1); if (typeof WebSocketImpl !== "function") throw new DiscordError("gateway: WebSocket implementation required", 1); diff --git a/packages/discord/src/journal.mjs b/packages/discord/src/journal.mjs index 61ddea826..98e206cfe 100644 --- a/packages/discord/src/journal.mjs +++ b/packages/discord/src/journal.mjs @@ -131,7 +131,8 @@ export function writeTurn(dir, id, record) { } export function readTurn(dir, id) { - return JSON.parse(readFileSync(turnPath(dir, id), "utf8")); + const path = turnPath(dir, id); + return parseJsonFileText(readFileSync(path, "utf8"), path, "turn record"); } export function listTurns(dir) { @@ -139,7 +140,7 @@ export function listTurns(dir) { if (!existsSync(turns)) return []; return readdirSync(turns) .filter((f) => f.endsWith(".json")) - .map((f) => JSON.parse(readFileSync(join(turns, f), "utf8"))); + .map((f) => parseJsonFileText(readFileSync(join(turns, f), "utf8"), join(turns, f), "turn record")); } // The daily ceiling counts admissions on the current UTC date. An admission diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs index 25c5a6883..41b0c95de 100644 --- a/packages/discord/src/setspark.mjs +++ b/packages/discord/src/setspark.mjs @@ -96,7 +96,7 @@ export function loadSetsparkConfig(raw, where = "setspark") { function loadApprovers(raw, where) { if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`); for (const [name, id] of Object.entries(raw)) { - if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`); + if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: an approver name must be trimmed, lower-case, 1-64 characters, with no control characters`); if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`); } if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`); diff --git a/packages/discord/tests/last-values.test.mjs b/packages/discord/tests/last-values.test.mjs new file mode 100644 index 000000000..1dd183c61 --- /dev/null +++ b/packages/discord/tests/last-values.test.mjs @@ -0,0 +1,43 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { validateBinding } from '../src/binding.mjs'; +import { createGateway } from '../src/gateway.mjs'; +import { readTurn, listTurns } from '../src/journal.mjs'; +import { DiscordError } from '../src/errors.mjs'; +import { rawBinding } from './helpers.mjs'; +for (const [field, edit] of [ + ['guildId', d => { d.guildId = secret; }], + ['name', d => { d.name = secret + '/'; }], + ['path', d => { d.tools = { roots: [{ name: 'root', path: `/x/.${secret}`, write: false }] }; }], +]) test(`binding ${field} refusal omits its configured value`, () => { + const d = rawBinding(); edit(d); assert.throws(() => validateBinding(d), safe(field, DiscordError, { exitCode: 2 })); +}); +test('gateway url refusal omits its value', () => assert.throws(() => createGateway({ url: secret, token: 'fake' }), safe('url', DiscordError, { exitCode: 1 }))); +for (const mode of ['read', 'list']) test(`turn ${mode} parser omits file bytes and keeps SyntaxError`, t => { + const root = scratch(t); mkdirSync(join(root, 'turns')); + const file = join(root, 'turns', 'turn-1.json'); writeFileSync(file, secret); + assert.throws(() => mode === 'read' ? readTurn(root, 'turn-1') : listTurns(root), safe(file, SyntaxError)); +}); + +for (const duplicate of [true, false]) test(`binding approver ${duplicate ? 'duplicate' : 'invalid'} name omits user values`, t => { + const root = scratch(t), keyFile = join(root, 'fixture-key'); writeFileSync(keyFile, 'fixture\n', { mode: 0o600 }); + const name = secret.toLowerCase() + (duplicate ? '' : '\u0001'); + const users = [{ id: '100000000000000100', name }]; + if (duplicate) users.push({ id: '100000000000000101', name }); + const raw = rawBinding({ users, tools: { roots: [{ name: 'docs', path: root }], setspark: { baseUrl: 'https://api.setspark.io', keyFile, principal: 'sage' } } }); + assert.throws(() => validateBinding(raw), safe(duplicate ? 'users' : 'approvers', DiscordError, { exitCode: 2 }, secret.toLowerCase())); +}); diff --git a/packages/discord/tests/setspark.test.mjs b/packages/discord/tests/setspark.test.mjs index 7fe2999bf..0a52949e3 100644 --- a/packages/discord/tests/setspark.test.mjs +++ b/packages/discord/tests/setspark.test.mjs @@ -157,7 +157,7 @@ test("setspark config: approvers come from the binding's users, never from the b const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" }); assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/); - assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/); + assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users share a name/); const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart"); const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })); diff --git a/packages/harness/src/runner.mjs b/packages/harness/src/runner.mjs index 8a0aaba67..aca5e5ac7 100644 --- a/packages/harness/src/runner.mjs +++ b/packages/harness/src/runner.mjs @@ -525,14 +525,20 @@ export async function main(runDir, { stdin = process.stdin, env = process.env, l process.on("SIGINT", stop); let session, cap, policy; + let input = "session.json"; try { session = { ...DEFAULTS, ...JSON.parse(readFileSync(join(runDir, "session.json"), "utf8")), runDir, proxyFirstTurn: existsSync(join(runDir, "proxy-first-turn-enabled")) }; + input = "stdin capability"; cap = JSON.parse(await readLine(stdin)).cap; if (typeof cap !== "string" || !/^[0-9a-f]{64}$/.test(cap)) throw new Error("no capability on stdin"); + input = "policy.json"; policy = JSON.parse(readFileSync(session.bundle.policy, "utf8")); } catch (e) { - log(`runner: ${e.message}`); + const reason = e.message === "stdin too large" ? "stdin too large (expected at most 4096 characters)" + : e.message === "no capability on stdin" ? "no capability on stdin (expected cap with 64 lowercase hex characters)" + : `${input} must be readable JSON with the expected session, capability or policy fields`; + log(`runner: ${e.code ? `${e.code}: ` : ""}${reason}`); return EXIT.usage; } if (!landingOk(session.landing)) { @@ -569,7 +575,13 @@ export async function main(runDir, { stdin = process.stdin, env = process.env, l } log(`runner: ${session.business}/${session.instance} claimed by run ${session.run}`); - const tools = JSON.parse(readFileSync(session.bundle.tools, "utf8")); + let tools; + try { + tools = JSON.parse(readFileSync(session.bundle.tools, "utf8")); + } catch (error) { + if (error instanceof SyntaxError) throw new SyntaxError("tools.json must be valid JSON"); + throw error; + } const server = await toolServer({ path: session.toolSocket, tools, diff --git a/packages/harness/tests/last-values.test.mjs b/packages/harness/tests/last-values.test.mjs new file mode 100644 index 000000000..589aaa3c3 --- /dev/null +++ b/packages/harness/tests/last-values.test.mjs @@ -0,0 +1,48 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { spawnSync } from 'node:child_process'; +import { EXIT } from '../src/runner.mjs'; +const runner = join(import.meta.dirname, '../src/runner.mjs'); +for (const at of ['session.json', 'stdin', 'policy.json']) test(`runner ${at} failure omits parser bytes and keeps usage exit`, t => { + const root = scratch(t), policy = join(root, 'policy.json'); + writeFileSync(join(root, 'session.json'), at === 'session.json' ? secret : JSON.stringify({ bundle: { policy } })); + writeFileSync(policy, at === 'policy.json' ? secret : '{}'); + const r = spawnSync(process.execPath, [runner, root], { encoding: 'utf8', input: at === 'stdin' ? secret + '\n' : JSON.stringify({ cap: 'a'.repeat(64) }) + '\n', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, EXIT.usage, r.stderr); + assert.ok(r.stderr.includes(at), r.stderr); + assert.doesNotMatch(r.stdout + r.stderr, /sk-test|NOT-A-SECRET|Unexpected token/); +}); +test('runner keeps a read errno without relaying its message', t => { + const root = scratch(t); mkdirSync(join(root, 'session.json')); + const r = spawnSync(process.execPath, [runner, root], { encoding: 'utf8', input: '', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, EXIT.usage); assert.match(r.stderr, /session.json/); assert.match(r.stderr, /runner: EISDIR:/); +}); +test('runner tools parser keeps SyntaxError without file bytes', t => { + const root = scratch(t), policy = join(root, 'policy.json'), tools = join(root, 'tools.json'); + writeFileSync(join(root, 'session.json'), JSON.stringify({ bundle: { policy, tools }, turnEnd: 'exit', harnessEnv: {} })); + writeFileSync(policy, '{"needs":[]}'); writeFileSync(tools, secret); + // Isolate the claim boundary: this test exercises the real post-claim file parser, + // and intentionally never starts a broker, adapter or tool server. + const code = `import { Readable } from 'node:stream'; +import { Client } from ${JSON.stringify(new URL('../../bus/src/client.mjs', import.meta.url).href)}; +import { main } from ${JSON.stringify(new URL('../src/runner.mjs', import.meta.url).href)}; +Client.prototype.call = async () => ({}); +try { await main(${JSON.stringify(root)}, { stdin: Readable.from([JSON.stringify({ cap: 'a'.repeat(64) }) + '\\n']), env: {}, log() {} }); } +catch (e) { if (!(e instanceof SyntaxError)) throw e; console.error(e.stack); process.exitCode = 9; }`; + const r = spawnSync(process.execPath, ['--input-type=module', '-e', code], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } }); + assert.equal(r.status, 9, r.stderr); assert.match(r.stderr, /tools.json/); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET|Unexpected token/); +}); diff --git a/packages/organization/src/access.mjs b/packages/organization/src/access.mjs index 7d6a77421..6fa840c46 100644 --- a/packages/organization/src/access.mjs +++ b/packages/organization/src/access.mjs @@ -72,10 +72,10 @@ function checkUser(user, at, ctx, humans, seenSubjects) { requireObject(user, at); rejectUnknownKeys(user, ["id", "home", "subjects"], at); requireId(user.id, `${at}.id`); - if (RESERVED_USERS.includes(user.id)) refuse(`${at}.id must not be ${JSON.stringify(user.id)}, a bus principal`); - if (humans.has(user.id)) refuse(`${at}.id must not be a business's human (${JSON.stringify(user.id)})`); + if (RESERVED_USERS.includes(user.id)) refuse(`${at}.id must not be a bus principal`); + if (humans.has(user.id)) refuse(`${at}.id must not be a business's human`); if (user.home !== LOCAL_HOME && !ctx.peers.has(user.home)) { - refuse(`${at}.home must be a configured peer or ${JSON.stringify(LOCAL_HOME)} (got ${JSON.stringify(user.home)})`); + refuse(`${at}.home must be a configured peer or ${JSON.stringify(LOCAL_HOME)}`); } const subjects = requireArray(user.subjects, `${at}.subjects`); if (subjects.length === 0) refuse(`${at}.subjects must not be empty`); @@ -84,7 +84,7 @@ function checkUser(user, at, ctx, humans, seenSubjects) { requireObject(s, sat); rejectUnknownKeys(s, ["provider", "sub"], sat); if (typeof s.provider !== "string" || !ctx.providers.has(s.provider)) { - refuse(`${sat}.provider must be a configured provider (got ${JSON.stringify(s.provider)})`); + refuse(`${sat}.provider must be a configured provider`); } requireText(s.sub, `${sat}.sub`); const key = subjectKey(s.provider, s.sub); @@ -105,7 +105,7 @@ function checkScope(value, where, check) { const seen = new Set(); list.forEach((entry, i) => { check(entry, `${where}[${i}]`); - if (seen.has(entry)) refuse(`duplicate entry in ${where}: ${JSON.stringify(entry)}`); + if (seen.has(entry)) refuse(`duplicate entry in ${where}`); seen.add(entry); }); } @@ -122,20 +122,20 @@ function checkGrant(grant, at, ctx, users, seenGrants) { requireObject(grant, at); rejectUnknownKeys(grant, ["id", "user", "business", "projects", "instances", "level", "expires"], at); requireId(grant.id, `${at}.id`); - if (seenGrants.has(grant.id)) refuse(`${at}.id is a duplicate grant id: ${grant.id}`); + if (seenGrants.has(grant.id)) refuse(`${at}.id is a duplicate grant id`); seenGrants.add(grant.id); - if (typeof grant.user !== "string" || !users.has(grant.user)) refuse(`${at}.user must name a user in this file (got ${JSON.stringify(grant.user)})`); + if (typeof grant.user !== "string" || !users.has(grant.user)) refuse(`${at}.user must name a user in this file`); if (typeof grant.business !== "string" || !ctx.businesses.has(grant.business)) { - refuse(`${at}.business must name a declared business (got ${JSON.stringify(grant.business)})`); + refuse(`${at}.business must name a declared business`); } const business = ctx.businesses.get(grant.business); checkScope(grant.projects, `${at}.projects`, (p, where) => { - if (typeof p !== "string" || !UUID.test(p)) refuse(`${where} must be "*" or a bus project id, a lowercase UUID (got ${JSON.stringify(p)})`); + if (typeof p !== "string" || !UUID.test(p)) refuse(`${where} must be "*" or a bus project id, a lowercase UUID`); }); checkScope(grant.instances, `${at}.instances`, (x, where) => { if (typeof x !== "string" || !business.instances.has(x)) refuse(`${where} must be "*" or a declared instance of the grant's business`); }); - if (!LEVELS.includes(grant.level)) refuse(`${at}.level must be one of ${LEVELS.join(", ")} (got ${JSON.stringify(grant.level)})`); + if (!LEVELS.includes(grant.level)) refuse(`${at}.level must be one of ${LEVELS.join(", ")}`); if (grant.expires !== undefined) checkExpires(grant.expires, `${at}.expires`); } @@ -147,13 +147,13 @@ export function checkAccess(document, context) { const humans = new Set([...ctx.businesses.values()].map((b) => b.human)); if (!isPlainObject(document)) refuse("access file must be a JSON object"); rejectUnknownKeys(document, ["accessVersion", "revision", "users", "grants"], "access file"); - if (document.accessVersion !== 1) refuse(`accessVersion must be 1 (got ${JSON.stringify(document.accessVersion)})`); + if (document.accessVersion !== 1) refuse(`accessVersion must be 1`); requirePositiveInt(document.revision, "revision"); const users = new Set(); const seenSubjects = new Set(); requireArray(document.users, "users").forEach((user, i) => { checkUser(user, `users[${i}]`, ctx, humans, seenSubjects); - if (users.has(user.id)) refuse(`users[${i}].id is a duplicate user id: ${user.id}`); + if (users.has(user.id)) refuse(`users[${i}].id is a duplicate user id`); users.add(user.id); }); const seenGrants = new Set(); diff --git a/packages/organization/src/appearance.mjs b/packages/organization/src/appearance.mjs index 67219c455..8665d1e30 100644 --- a/packages/organization/src/appearance.mjs +++ b/packages/organization/src/appearance.mjs @@ -18,10 +18,10 @@ export function checkAppearance(value, where) { requireObject(value, where); rejectUnknownKeys(value, ["theme", "mode"], where); if (Object.hasOwn(value, "theme") && (typeof value.theme !== "string" || value.theme.length > 64 || !NAME.test(value.theme))) { - refuse(`${where}.theme must be an installed theme's name, matching ${NAME} (got ${JSON.stringify(value.theme)})`); + refuse(`${where}.theme must be an installed theme's name, matching ${NAME}`); } if (Object.hasOwn(value, "mode") && !APPEARANCE_MODES.includes(value.mode)) { - refuse(`${where}.mode must be one of ${APPEARANCE_MODES.join(", ")} (got ${JSON.stringify(value.mode)})`); + refuse(`${where}.mode must be one of ${APPEARANCE_MODES.join(", ")}`); } return Object.freeze({ ...(Object.hasOwn(value, "theme") ? { theme: value.theme } : {}), diff --git a/packages/organization/src/config-command.mjs b/packages/organization/src/config-command.mjs index 87472af2d..9fc01c902 100644 --- a/packages/organization/src/config-command.mjs +++ b/packages/organization/src/config-command.mjs @@ -67,7 +67,20 @@ export function runConfigCommand(argv, { stdout = process.stdout, stderr = proce maxBuffer: 1024 * 1024, env, }); - if (proc.status !== 0) fail(3, `system config problem (mosaic-config exit ${proc.status}): ${proc.stderr.trim()}`); + // Render known key paths with fixed constraints; never relay child stderr. + if (proc.status !== 0) { + const problem = proc.stderr.trim(); + const field = problem.startsWith('mosaic-config: execution "dataRoot"') ? "dataRoot" + : /^mosaic-config: (?:unsupported )?"?((?:configVersion|environment|dataRoot|execution|console|federation|compactionReadBytes|compactionStreams)(?:\.[A-Za-z][A-Za-z0-9]*|\[\d+\])*)"?(?=[: ])/u.exec(problem)?.[1]; + const name = "a 1-200 character name starting with a letter or digit and containing only letters, digits, . _ : / -"; + const expected = { configVersion: "1", environment: "development or production", dataRoot: "a canonical absolute instance data path", + "execution.backend": "docker", "execution.adapter": "pi or mock", "execution.provider": name, "execution.model": name }; + const detail = problem.startsWith("mosaic-config: configuration is not valid JSON (") + ? "configuration is not valid JSON; expected readable JSON: " + : field ? `${field}: expected ${expected[field] ?? "a value satisfying the system configuration schema"}: ` + : "expected valid system configuration: "; + fail(3, `system config problem (mosaic-config exit ${proc.status}): ${detail}${env.MOSAIC_CONFIG || join(env.HOME || "~", ".config", "mosaic-dev", "config.json")}${proc.error?.code ? ` (${proc.error.code})` : ""}`); + } try { return JSON.parse(proc.stdout); } catch { @@ -106,7 +119,7 @@ export function runConfigCommand(argv, { stdout = process.stdout, stderr = proce continue; } const project = loadProject(entry.root); - if (project.id !== id) fail(2, `project file ${file} has id ${project.id}, but business ${organization.id} declares it as ${id}`); + if (project.id !== id) fail(2, `project file ${file} has id that does not match the declared project key`); for (const instance of Object.keys(organization.roles)) resolveInstance({ system, business: organization, project, instance }); projects[id] = "valid"; } diff --git a/packages/organization/src/project.mjs b/packages/organization/src/project.mjs index 320ec018c..6e6e9aab5 100644 --- a/packages/organization/src/project.mjs +++ b/packages/organization/src/project.mjs @@ -30,7 +30,7 @@ export function validateProjectDocument(document, file) { } export function loadProject(root) { - if (typeof root !== "string" || !isAbsolute(root) || normalize(root) !== root) refuse(`project root must be a normalized absolute path (got ${JSON.stringify(root)})`); + if (typeof root !== "string" || !isAbsolute(root) || normalize(root) !== root) refuse(`project root must be a normalized absolute path`); const file = projectFilePath(root); return validateProjectDocument(readJsonFile(file, "project file"), file); } diff --git a/packages/organization/src/role.mjs b/packages/organization/src/role.mjs index c002ab437..837812ce0 100644 --- a/packages/organization/src/role.mjs +++ b/packages/organization/src/role.mjs @@ -24,7 +24,7 @@ const SKILL_NAME = /^[A-Za-z0-9._-]+$/; function checkTools(tools) { if (!Array.isArray(tools) || tools.length === 0) refuse('role "tools" must be a non-empty array of tool names'); return requireDistinctList(tools, "role tools", (tool) => { - if (!TOOLS.includes(tool)) refuse(`unsupported tool: ${JSON.stringify(tool)} (supported: ${TOOLS.join(", ")})`); + if (!TOOLS.includes(tool)) refuse(`unsupported tool (supported: ${TOOLS.join(", ")})`); }); } @@ -39,12 +39,12 @@ function checkAuthority(authority) { const lists = {}; for (const key of ["withinRole", "crossRole"]) { lists[key] = requireDistinctList(authority[key], `role authority.${key}`, (action) => { - if (!ACTIONS.includes(action)) refuse(`unknown action in authority.${key}: ${JSON.stringify(action)}`); - if (GATED_ONLY.includes(action)) refuse(`authority.${key} lists ${action}, which is always gated`); + if (!ACTIONS.includes(action)) refuse(`unknown action in authority.${key}; expected an action from the role vocabulary`); + if (GATED_ONLY.includes(action)) refuse(`authority.${key} lists an action which is always gated`); }); } const both = lists.withinRole.filter((a) => lists.crossRole.includes(a)); - if (both.length > 0) refuse(`action listed as both withinRole and crossRole: ${both.join(", ")}`); + if (both.length > 0) refuse(`action listed as both withinRole and crossRole`); return lists; } @@ -71,7 +71,7 @@ function checkMessaging(messaging) { function checkSkills(skills) { return requireDistinctList(skills, "role skills", (name) => { if (typeof name !== "string" || !SKILL_NAME.test(name) || name === "." || name === "..") { - refuse(`role skill must be a skill directory name matching ${SKILL_NAME} (got ${JSON.stringify(name)})`); + refuse(`role skill must be a skill directory name matching ${SKILL_NAME}`); } }); } @@ -87,13 +87,13 @@ function checkHireable(hireable, file) { const dir = dirname(file); return requireDistinctList(hireable, "role hireable", (name) => { requireId(name, "role hireable entry"); - if (CREATIVE_DEFINITIONS.includes(name)) refuse(`hireable names ${name}, a creative definition only the human adds`); + if (CREATIVE_DEFINITIONS.includes(name)) refuse(`hireable names a creative definition only the human adds`); const path = join(dir, `${name}.json`); - if (!existsSync(path)) refuse(`hireable names ${name}, which has no role file`); + if (!existsSync(path)) refuse(`hireable names a definition which has no role file`); const named = validateRoleDocument(readJsonFile(path, "role file"), path, { hireable: false }); - if (named.roleVersion !== 2) refuse(`hireable names ${name}, which is role version 1`); + if (named.roleVersion !== 2) refuse(`hireable names a definition which is role version 1`); const outside = unhireableAction(named); - if (outside !== null) refuse(`hireable names ${name}, which holds ${outside}`); + if (outside !== null) refuse(`hireable names a definition which holds an action outside the hireable ceiling`); }); } @@ -101,12 +101,12 @@ function checkGiteaScopes(scopes) { const list = requireDistinctList(scopes, "gitea scopes", (scope) => { const m = typeof scope === "string" ? /^(read|write):([a-z]+)$/.exec(scope) : null; if (!m || !GITEA_SCOPE_CATEGORIES.includes(m[2])) { - refuse(`unsupported gitea scope: ${JSON.stringify(scope)} (expected read: or write:; categories: ${GITEA_SCOPE_CATEGORIES.join(", ")})`); + refuse(`unsupported gitea scope (expected read: or write:; categories: ${GITEA_SCOPE_CATEGORIES.join(", ")})`); } }, { nonEmpty: true }); const categories = list.map((s) => s.split(":")[1]); const twice = categories.filter((c, i) => categories.indexOf(c) !== i); - if (twice.length > 0) refuse(`gitea scopes name ${twice[0]} twice; a token holds one level per category`); + if (twice.length > 0) refuse(`gitea scopes name a category twice; a token holds one level per category`); return list; } @@ -116,9 +116,9 @@ function checkVikunjaScopes(scopes) { const out = {}; for (const [group, verbs] of Object.entries(scopes)) { const allowed = VIKUNJA_GRANTABLE[group]; - if (!allowed) refuse(`vikunja route group not grantable to a role: ${JSON.stringify(group)}`); + if (!allowed) refuse(`vikunja route group not grantable to a role`); out[group] = requireDistinctList(verbs, `vikunja scopes.${group}`, (verb) => { - if (!allowed.includes(verb)) refuse(`vikunja verb not grantable to a role: ${group}.${JSON.stringify(verb)}`); + if (!allowed.includes(verb)) refuse(`vikunja verb not grantable to a role`); }, { nonEmpty: true }); } return out; @@ -135,7 +135,7 @@ function checkCredentials(credentials) { if (!host && !SERVICES.includes(entry.service)) { refuse(`role credentials[${i}].service must be one of: ${[...SERVICES, HOST_SERVICE].join(", ")}`); } - if (seen.has(entry.service)) refuse(`role credentials name ${entry.service} twice`); + if (seen.has(entry.service)) refuse(`role credentials name a service twice`); seen.add(entry.service); if (host) return { service: HOST_SERVICE }; const scopes = entry.service === "gitea" ? checkGiteaScopes(entry.scopes) : checkVikunjaScopes(entry.scopes); @@ -145,7 +145,7 @@ function checkCredentials(credentials) { function checkContract(contract, file) { if (typeof contract !== "string" || !CONTRACT_NAME.test(contract)) { - refuse(`role "contract" must be a Markdown file name in the role's directory, matching ${CONTRACT_NAME} (got ${JSON.stringify(contract)})`); + refuse(`role "contract" must be a Markdown file name in the role's directory, matching ${CONTRACT_NAME}`); } const path = join(dirname(file), contract); let stat; @@ -167,12 +167,12 @@ function checkContract(contract, file) { // docs: its authority is withinRole CREATIVE_ACTIONS at most, and its one credential is gitea. function checkCreative(role) { const wider = role.tools.filter((tool) => !CREATIVE_TOOLS.includes(tool)); - if (wider.length > 0) refuse(`creative role ${role.name} may not name tools ${wider.join(", ")} (allowed: ${CREATIVE_TOOLS.join(", ")})`); + if (wider.length > 0) refuse(`creative role ${role.name} may not name tools outside the allowed set (allowed: ${CREATIVE_TOOLS.join(", ")})`); if (role.authority.crossRole.length > 0 || role.authority.withinRole.some((a) => !CREATIVE_ACTIONS.includes(a))) { refuse(`creative role ${role.name} may hold only withinRole ${CREATIVE_ACTIONS.join(" and ")}`); } const services = role.credentials.map((c) => c.service).filter((service) => service !== CREATIVE_SERVICE); - if (services.length > 0) refuse(`creative role ${role.name} may hold only the ${CREATIVE_SERVICE} credential (got ${services.join(", ")})`); + if (services.length > 0) refuse(`creative role ${role.name} may hold only the ${CREATIVE_SERVICE} credential`); if (role.messaging === null) refuse(`creative role ${role.name} requires "messaging"`); for (const key of ["sendTo", "receiveFrom"]) { if (role.messaging[key].some((entry) => entry !== "human")) { @@ -193,7 +193,7 @@ export function validateRoleDocument(document, file, options = {}) { rejectUnknownKeys(document, document.roleVersion === 1 ? V1_KEYS : V2_KEYS, "role"); requireId(document.name, "role name"); const base = basename(file).replace(/\.json$/, ""); - if (document.name !== base) refuse(`role "name" (${document.name}) must match its filename (${base}.json)`); + if (document.name !== base) refuse(`role "name" must match its filename (${base}.json)`); const tools = checkTools(document.tools); if (document.roleVersion === 1) { if (CREATIVE_DEFINITIONS.includes(document.name)) refuse(`creative role ${document.name} must be version 2`); diff --git a/packages/organization/src/util.mjs b/packages/organization/src/util.mjs index c50867510..326190fe5 100644 --- a/packages/organization/src/util.mjs +++ b/packages/organization/src/util.mjs @@ -44,7 +44,7 @@ export function requireDistinctList(value, where, check, { nonEmpty = false } = const seen = new Set(); for (const item of value) { check(item); - if (seen.has(item)) refuse(`duplicate entry in ${where}: ${JSON.stringify(item)}`); + if (seen.has(item)) refuse(`duplicate entry in ${where}`); seen.add(item); } return [...seen]; diff --git a/packages/organization/src/vocabulary.mjs b/packages/organization/src/vocabulary.mjs index 66f482299..8a7de8e66 100644 --- a/packages/organization/src/vocabulary.mjs +++ b/packages/organization/src/vocabulary.mjs @@ -295,7 +295,7 @@ export const GRANT_ALL = "*"; // and a viewer grant never covers it. An expired grant covers nothing. export function covers(grant, { business, project, instance }, now, { level } = {}) { if (!(now instanceof Date) || Number.isNaN(now.getTime())) throw new TypeError("covers needs a valid Date for now"); - if (!LEVELS.includes(level)) throw new TypeError(`covers needs a level, one of ${LEVELS.join(", ")} (got ${JSON.stringify(level)})`); + if (!LEVELS.includes(level)) throw new TypeError(`covers needs a level, one of ${LEVELS.join(", ")}`); if (grant.expires !== undefined && now.getTime() >= Date.parse(grant.expires)) return false; if (grant.business !== business) return false; if (!(grant.projects[0] === GRANT_ALL || grant.projects.includes(project))) return false; diff --git a/packages/organization/tests/access.test.mjs b/packages/organization/tests/access.test.mjs index b6813bd47..28ade0aa0 100644 --- a/packages/organization/tests/access.test.mjs +++ b/packages/organization/tests/access.test.mjs @@ -104,19 +104,19 @@ test("users: unknown keys and the id rule", () => { refuses(() => checkAccess(mutated((d) => { d.users[1].role = "x"; }), context()), /unsupported users\[1\] key: "role"/); refuses(() => checkAccess(mutated((d) => { d.users[0] = "bean"; }), context()), /users\[0\] must be a JSON object/); refuses(() => checkAccess(mutated((d) => { d.users[0].id = "Bean"; }), context()), /users\[0\]\.id must match/); - refuses(() => checkAccess(mutated((d) => { d.users[1].id = "bean"; }), context()), /users\[1\]\.id is a duplicate user id: bean/); + refuses(() => checkAccess(mutated((d) => { d.users[1].id = "bean"; }), context()), /users\[1\]\.id is a duplicate user id/); }); test("users: human, tester and any business's human are refused", () => { - refuses(() => checkAccess(mutated((d) => { d.users[0].id = "human"; d.grants = []; }), context()), /users\[0\]\.id must not be "human"/); - refuses(() => checkAccess(mutated((d) => { d.users[1].id = "tester"; d.grants = []; }), context()), /users\[1\]\.id must not be "tester"/); - refuses(() => checkAccess(mutated((d) => { d.users[0].id = "melanie"; d.grants = []; }), context()), /users\[0\]\.id must not be a business's human \("melanie"\)/); + refuses(() => checkAccess(mutated((d) => { d.users[0].id = "human"; d.grants = []; }), context()), /users\[0\]\.id must not be a bus principal/); + refuses(() => checkAccess(mutated((d) => { d.users[1].id = "tester"; d.grants = []; }), context()), /users\[1\]\.id must not be a bus principal/); + refuses(() => checkAccess(mutated((d) => { d.users[0].id = "melanie"; d.grants = []; }), context()), /users\[0\]\.id must not be a business's human/); // The human of a business the grant doesn't name is refused too. - refuses(() => checkAccess(mutated((d) => { d.users[1].id = "jason"; d.grants = []; }), context()), /users\[1\]\.id must not be a business's human \("jason"\)/); + refuses(() => checkAccess(mutated((d) => { d.users[1].id = "jason"; d.grants = []; }), context()), /users\[1\]\.id must not be a business's human/); }); test("users: home is a given peer or local", () => { - refuses(() => checkAccess(mutated((d) => { d.users[0].home = "elsewhere"; }), context()), /users\[0\]\.home must be a configured peer or "local" \(got "elsewhere"\)/); + refuses(() => checkAccess(mutated((d) => { d.users[0].home = "elsewhere"; }), context()), /users\[0\]\.home must be a configured peer or "local"/); refuses(() => checkAccess(mutated((d) => { delete d.users[1].home; }), context()), /users\[1\]\.home must be a configured peer/); refuses(() => checkAccess(doc(), context({ peers: [] })), /users\[0\]\.home must be a configured peer/); }); @@ -125,7 +125,7 @@ test("users: subjects, provider, sub, uniqueness and owner subjects", () => { refuses(() => checkAccess(mutated((d) => { d.users[0].subjects = []; }), context()), /users\[0\]\.subjects must not be empty/); refuses(() => checkAccess(mutated((d) => { delete d.users[0].subjects; }), context()), /users\[0\]\.subjects must be an array/); refuses(() => checkAccess(mutated((d) => { d.users[0].subjects[0].extra = 1; }), context()), /unsupported users\[0\]\.subjects\[0\] key: "extra"/); - refuses(() => checkAccess(mutated((d) => { d.users[1].subjects[0].provider = "google"; }), context()), /users\[1\]\.subjects\[0\]\.provider must be a configured provider \(got "google"\)/); + refuses(() => checkAccess(mutated((d) => { d.users[1].subjects[0].provider = "google"; }), context()), /users\[1\]\.subjects\[0\]\.provider must be a configured provider/); refuses(() => checkAccess(mutated((d) => { d.users[0].subjects[0].sub = ""; }), context()), /users\[0\]\.subjects\[0\]\.sub must be a non-empty string/); refuses(() => checkAccess(mutated((d) => { d.users[0].subjects[0].sub = 7; }), context()), /users\[0\]\.subjects\[0\]\.sub must be a non-empty string/); // A pair twice in one user, and across two users. @@ -140,15 +140,15 @@ test("users: subjects, provider, sub, uniqueness and owner subjects", () => { test("grants: unknown keys, id, user and business", () => { refuses(() => checkAccess(mutated((d) => { d.grants[0].role = "x"; }), context()), /unsupported grants\[0\] key: "role"/); - refuses(() => checkAccess(mutated((d) => { d.grants[1].id = "g-1"; }), context()), /grants\[1\]\.id is a duplicate grant id: g-1/); + refuses(() => checkAccess(mutated((d) => { d.grants[1].id = "g-1"; }), context()), /grants\[1\]\.id is a duplicate grant id/); refuses(() => checkAccess(mutated((d) => { d.grants[0].id = "G 1"; }), context()), /grants\[0\]\.id must match/); - refuses(() => checkAccess(mutated((d) => { d.grants[0].user = "nobody"; }), context()), /grants\[0\]\.user must name a user in this file \(got "nobody"\)/); - refuses(() => checkAccess(mutated((d) => { d.grants[1].business = "other"; }), context()), /grants\[1\]\.business must name a declared business \(got "other"\)/); + refuses(() => checkAccess(mutated((d) => { d.grants[0].user = "nobody"; }), context()), /grants\[0\]\.user must name a user in this file/); + refuses(() => checkAccess(mutated((d) => { d.grants[1].business = "other"; }), context()), /grants\[1\]\.business must name a declared business/); refuses(() => checkAccess(mutated((d) => { d.grants[1].business = "constructor"; }), context()), /grants\[1\]\.business must name a declared business/); }); test("grants: projects is [\"*\"] or distinct lowercase UUIDs", () => { - refuses(() => checkAccess(mutated((d) => { d.grants[1].projects = ["stack"]; }), context()), /grants\[1\]\.projects\[0\] must be "\*" or a bus project id, a lowercase UUID \(got "stack"\)/); + refuses(() => checkAccess(mutated((d) => { d.grants[1].projects = ["stack"]; }), context()), /grants\[1\]\.projects\[0\] must be "\*" or a bus project id, a lowercase UUID/); refuses(() => checkAccess(mutated((d) => { d.grants[1].projects = [P1.toUpperCase()]; }), context()), /grants\[1\]\.projects\[0\] must be "\*" or a bus project id/); refuses(() => checkAccess(mutated((d) => { d.grants[1].projects = [P1, 7]; }), context()), /grants\[1\]\.projects\[1\] must be "\*" or a bus project id/); refuses(() => checkAccess(mutated((d) => { d.grants[1].projects = ["*", P1]; }), context()), /grants\[1\]\.projects must be \["\*"\] alone/); @@ -172,7 +172,7 @@ test("grants: any level other than collaborator or viewer refuses, naming grants for (const bad of ["owner", "Collaborator", "COLLABORATOR", "Viewer", "", null, 1]) { refuses(() => checkAccess(mutated((d) => { d.grants[1].level = bad; }), context()), /grants\[1\]\.level must be one of collaborator, viewer/); } - refuses(() => checkAccess(mutated((d) => { delete d.grants[0].level; }), context()), /grants\[0\]\.level must be one of collaborator, viewer \(got undefined\)/); + refuses(() => checkAccess(mutated((d) => { delete d.grants[0].level; }), context()), /grants\[0\]\.level must be one of collaborator, viewer/); }); test("grants: expires is optional ISO time, and a past expiry is not refused", () => { diff --git a/packages/organization/tests/case1-project-key.test.mjs b/packages/organization/tests/case1-project-key.test.mjs index 66173b49b..fdc19ee9f 100644 --- a/packages/organization/tests/case1-project-key.test.mjs +++ b/packages/organization/tests/case1-project-key.test.mjs @@ -59,7 +59,7 @@ test("row 325: the old mosaic-stack project key refuses against those same proje const r = x.validate(); assert.equal(r.status, 2, r.stdout + r.stderr); assert.equal(r.stdout, ""); - assert.match(r.stderr, /has id stack, but business mosaic-stack declares it as mosaic-stack/); + assert.match(r.stderr, /has id that does not match the declared project key/); assert.ok(r.stderr.includes(x.projectFile)); assert.doesNotMatch(r.stderr, /no project file/); }); diff --git a/packages/organization/tests/cli.test.mjs b/packages/organization/tests/cli.test.mjs index a3c399f1e..8371b4625 100644 --- a/packages/organization/tests/cli.test.mjs +++ b/packages/organization/tests/cli.test.mjs @@ -77,7 +77,7 @@ test("validate: project files", () => { assert.deepEqual(JSON.parse(r.out).projects, { stack: "valid" }); writeJson(file, { projectVersion: 1, id: "other" }); - assert.match(run(s, "validate", "acme").err, /has id other, but business acme declares it as stack/); + assert.match(run(s, "validate", "acme").err, /has id that does not match the declared project key/); assert.equal(run(s, "validate", "acme").code, 2); writeJson(file, { projectVersion: 1, id: "stack", roles: { ghost: {} } }); const ghost = run(s, "validate", "acme"); diff --git a/packages/organization/tests/last-values.test.mjs b/packages/organization/tests/last-values.test.mjs new file mode 100644 index 000000000..abef521aa --- /dev/null +++ b/packages/organization/tests/last-values.test.mjs @@ -0,0 +1,74 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { checkAccess } from '../src/access.mjs'; +import { checkAppearance } from '../src/appearance.mjs'; +import { loadProject } from '../src/project.mjs'; +import { validateRoleDocument } from '../src/role.mjs'; +import { covers } from '../src/vocabulary.mjs'; +import { BusinessError } from '../src/errors.mjs'; +const context = { providers: ['test'], peers: [], businesses: { acme: { human: 'jason', instances: ['coder'] } }, ssoSubjects: [] }; +const access = () => ({ accessVersion: 1, revision: 1, users: [{ id: 'bean', home: 'local', subjects: [{ provider: 'test', sub: 'user' }] }], grants: [{ id: 'grant', user: 'bean', business: 'acme', projects: ['*'], instances: ['coder'], level: 'viewer' }] }); +for (const [field, edit] of [ + ['users[0].home', d => { d.users[0].home = secret; }], + ['users[0].subjects[0].provider', d => { d.users[0].subjects[0].provider = secret; }], + ...['user', 'business', 'level'].map(k => [`grants[0].${k}`, d => { d.grants[0][k] = secret; }]), + ['grants[0].projects[0]', d => { d.grants[0].projects = [secret]; }], + ['accessVersion', d => { d.accessVersion = secret; }], +]) test(`access ${field} omits the refused value`, () => { + const d = access(); edit(d); assert.throws(() => checkAccess(d, context), safe(field, BusinessError, { exitCode: 2 })); +}); +for (const k of ['theme', 'mode']) test(`appearance ${k} omits its value`, () => { + assert.throws(() => checkAppearance({ [k]: secret }, 'appearance'), safe(k, BusinessError, { exitCode: 2 })); +}); +test('project root omits its configured value', () => assert.throws(() => loadProject(secret), safe('project root', BusinessError, { exitCode: 2 }))); +test('covers names the level field, not its value', () => assert.throws(() => covers({}, {}, new Date(), { level: secret }), safe('level', TypeError))); +const role = () => ({ roleVersion: 2, name: 'r', title: 'R', contract: 'r.md', tools: ['read'], network: 'none', authority: { withinRole: [], crossRole: [] }, credentials: [] }); +for (const [field, edit] of [ + ['skill', d => { d.skills = [secret + '/']; }], + ['contract', d => { d.contract = secret; }], + ['tool', d => { d.tools = [secret]; }], + ['authority.withinRole', d => { d.authority.withinRole = [secret]; }], + ['hireable', d => { d.hireable = [secret.toLowerCase()]; }], + ['gitea scope', d => { d.credentials = [{ service: 'gitea', scopes: [secret] }]; }], + ['vikunja route group', d => { d.credentials = [{ service: 'vikunja', scopes: { [secret]: ['read'] } }]; }], + ['vikunja verb', d => { d.credentials = [{ service: 'vikunja', scopes: { tasks: [secret] } }]; }], + ['name', d => { d.name = secret.toLowerCase(); }], +]) test(`role ${field} omits the configured value`, t => { + const root = scratch(t); writeFileSync(join(root, 'r.md'), '# R\n\n## Authority\n'); + const d = role(); edit(d); + assert.throws(() => validateRoleDocument(d, join(root, 'r.json')), safe(field, BusinessError, { exitCode: 2 }, secret.toLowerCase())); +}); +// This branch is reachable only with already-validated service names; a sentinel service +// is refused earlier. Pin the same no-value rule with an admissible service. +test('creative service refusal omits the validated service list', t => { + const root = scratch(t); writeFileSync(join(root, 'companion.md'), '# R\n\n## Authority\n'); + const d = { ...role(), name: 'companion', contract: 'companion.md', messaging: { sendTo: ['human'], receiveFrom: ['human'] }, credentials: [{ service: 'vikunja', scopes: { tasks: ['read_one'] } }] }; + assert.throws(() => validateRoleDocument(d, join(root, 'companion.json')), safe('credential', BusinessError, { exitCode: 2 }, 'vikunja')); +}); +test('role duplicate skill omits the accepted list value', t => { + const root = scratch(t); writeFileSync(join(root, 'r.md'), '# R\n\n## Authority\n'); + const d = { ...role(), skills: [secret, secret] }; + assert.throws(() => validateRoleDocument(d, join(root, 'r.json')), safe('role skills', BusinessError, { exitCode: 2 })); +}); +for (const at of ['user', 'grant', 'scope', 'human']) test(`access ${at} duplicate/reserved refusal omits accepted ids`, () => { + const value = secret.toLowerCase(), d = access(), ctx = structuredClone(context); + if (at === 'user') { d.users[0].id = value; d.users.push({ ...structuredClone(d.users[0]), subjects: [{ provider: 'test', sub: 'second' }] }); } + if (at === 'grant') { d.grants[0].id = value; d.grants.push(structuredClone(d.grants[0])); } + if (at === 'scope') { ctx.businesses.acme.instances = [value]; d.grants[0].instances = [value, value]; } + if (at === 'human') { ctx.businesses.acme.human = value; d.users[0].id = value; } + assert.throws(() => checkAccess(d, ctx), safe(at === 'scope' ? 'instances' : '.id', BusinessError, { exitCode: 2 }, value)); +}); diff --git a/packages/organization/tests/role.test.mjs b/packages/organization/tests/role.test.mjs index 5e495e6f9..ffc6d330d 100644 --- a/packages/organization/tests/role.test.mjs +++ b/packages/organization/tests/role.test.mjs @@ -174,7 +174,7 @@ test("row 64 skills: optional, distinct skill directory names, none on version 1 assert.deepEqual(role.skills, ["ms-unslop", "ms_sdlc.v2"]); assert.ok(Object.isFrozen(role.skills)); refuses(() => check({ skills: "ms-unslop" }), /role skills must be an array/); - refuses(() => check({ skills: ["ms-unslop", "ms-unslop"] }), /duplicate entry in role skills: "ms-unslop"/); + refuses(() => check({ skills: ["ms-unslop", "ms-unslop"] }), /duplicate entry in role skills/); for (const bad of [".", "..", "", "a/b", "../ms-unslop", "ms unslop", 7, null]) { refuses(() => check({ skills: [bad] }), /role skill must be a skill directory name/); } @@ -189,7 +189,7 @@ test("credentials: Gitea scopes", () => { refuses(() => gitea(["write:admin"]), /unsupported gitea scope/); refuses(() => gitea(["all"]), /unsupported gitea scope/); refuses(() => gitea(["sudo:repository"]), /unsupported gitea scope/); - refuses(() => gitea(["read:issue", "write:issue"]), /name issue twice/); + refuses(() => gitea(["read:issue", "write:issue"]), /name a category twice/); refuses(() => gitea([]), /must not be empty/); }); @@ -255,7 +255,7 @@ test("M1: a creative role file wider than read and answer is refused", () => { }; assert.doesNotThrow(creative("ideas", { tools: ["read", "grep", "find", "ls", "web"] })); for (const tool of ["write", "edit", "bash"]) { - refuses(creative("ideas", { tools: ["read", tool] }), new RegExp(`^creative role ideas may not name tools ${tool} `)); + refuses(creative("ideas", { tools: ["read", tool] }), /^creative role ideas may not name tools outside the allowed set /); } const gitea = [{ service: "gitea", scopes: ["read:user", "write:repository"] }]; assert.doesNotThrow(creative("ideas", { authority: { withinRole: ["message.send", "library.write"], crossRole: [] }, credentials: gitea })); @@ -268,9 +268,9 @@ test("M1: a creative role file wider than read and answer is refused", () => { refuses(creative("proofreader", { authority: { withinRole: ["message.send"], crossRole: ["library.write"] } }), only); // Row 285: library.write with any credential but gitea refuses. refuses(creative("ideas", { authority: { withinRole: ["message.send", "library.write"], crossRole: [] }, credentials: [...gitea, { service: "vikunja", scopes: { tasks: ["read_one"] } }] }), - /^creative role ideas may hold only the gitea credential \(got vikunja\)$/); + /^creative role ideas may hold only the gitea credential$/); refuses(creative("ideas", { authority: { withinRole: ["message.send", "library.write"], crossRole: [] }, credentials: [{ service: "host" }] }), - /^creative role ideas may hold only the gitea credential \(got host\)$/); + /^creative role ideas may hold only the gitea credential$/); refuses(creative("continuity", { messaging: undefined }), /^creative role continuity requires "messaging"$/); refuses(creative("research", { messaging: { sendTo: ["human", "pm"], receiveFrom: ["human"] } }), /^creative role research messaging.sendTo may name only human$/); refuses(creative("research", { messaging: { sendTo: ["human"], receiveFrom: ["assistant"] } }), /^creative role research messaging.receiveFrom may name only human$/); @@ -307,14 +307,14 @@ test("hireable: a list naming a definition that holds any refused action refuses for (const action of REFUSED_FOR_HIRE) { for (const key of ["withinRole", "crossRole"]) { const dir = hireDir({ chief: { hireable: ["worker"] }, worker: { [key]: [action], ...(key === "crossRole" ? { withinRole: [] } : {}) } }); - refuses(() => loadRole(dir, "chief"), new RegExp(`hireable names worker, which holds ${action.replace(/\./g, "\\.")}$`)); + refuses(() => loadRole(dir, "chief"), /hireable names a definition which holds an action outside the hireable ceiling$/); } } }); test("hireable: a mixed definition refuses, a clean list loads, and the named file's own hireable isn't read", () => { const mixed = hireDir({ chief: { hireable: ["worker"] }, worker: { withinRole: ["message.send", "role.launch"] } }); - refuses(() => loadRole(mixed, "chief"), /hireable names worker, which holds role\.launch/); + refuses(() => loadRole(mixed, "chief"), /hireable names a definition which holds an action outside the hireable ceiling/); const clean = hireDir({ chief: { hireable: ["worker", "helper"] }, worker: { withinRole: [...HIRE_ALLOWED] }, @@ -323,15 +323,15 @@ test("hireable: a mixed definition refuses, a clean list loads, and the named fi }); assert.deepEqual(loadRole(clean, "chief").hireable, ["worker", "helper"]); assert.ok(Object.isFrozen(loadRole(clean, "chief").hireable)); - refuses(() => loadRole(clean, "helper"), /hireable names nobody, which has no role file/); + refuses(() => loadRole(clean, "helper"), /hireable names a definition which has no role file/); }); test("hireable: a missing, version 1, duplicate or badly named entry refuses, and the key is optional", () => { - refuses(() => loadRole(hireDir({ chief: { hireable: ["ghost"] } }), "chief"), /hireable names ghost, which has no role file/); - refuses(() => loadRole(hireDir({ chief: { hireable: ["old"] }, old: 1 }), "chief"), /hireable names old, which is role version 1/); + refuses(() => loadRole(hireDir({ chief: { hireable: ["ghost"] } }), "chief"), /hireable names a definition which has no role file/); + refuses(() => loadRole(hireDir({ chief: { hireable: ["old"] }, old: 1 }), "chief"), /hireable names a definition which is role version 1/); // Row 213 (M10): a creative definition refuses by name, before its file is read. for (const name of CREATIVE_DEFINITIONS) { - refuses(() => loadRole(hireDir({ chief: { hireable: [name] } }), "chief"), new RegExp(`hireable names ${name}, a creative definition only the human adds$`)); + refuses(() => loadRole(hireDir({ chief: { hireable: [name] } }), "chief"), /hireable names a creative definition only the human adds$/); } refuses(() => loadRole(hireDir({ chief: { hireable: ["w", "w"] }, w: {} }), "chief"), /duplicate entry in role hireable/); refuses(() => loadRole(hireDir({ chief: { hireable: ["../w"] } }), "chief"), /role hireable entry/); diff --git a/packages/project/src/onboarding.mjs b/packages/project/src/onboarding.mjs index 6e0837e75..c15e9dd11 100644 --- a/packages/project/src/onboarding.mjs +++ b/packages/project/src/onboarding.mjs @@ -67,7 +67,7 @@ export function validateInterviewTemplate(value) { value.questions.forEach((q, i) => { if (!object(q) || !sameKeys(q, ['key', 'text'])) { problems.push(`questions[${i}] keys must be exactly key, text`); return; } if (typeof q.key !== 'string' || !KEY.test(q.key)) problems.push(`questions[${i}].key must be lower-case letters, digits and dashes`); - else if (seen.has(q.key)) problems.push(`questions[${i}].key repeats ${q.key}`); + else if (seen.has(q.key)) problems.push(`questions[${i}].key repeats an earlier question key`); seen.add(q.key); if (!line(q.text, 500)) problems.push(`questions[${i}].text must be one line of 1-500 characters`); }); @@ -76,7 +76,7 @@ export function validateInterviewTemplate(value) { problems.push('prompt must be 1-200 lines of text, each at most 2000 characters without line breaks'); else { const text = value.prompt.join('\n'), used = new Set([...text.matchAll(/\{([^{}]*)\}/g)].map(m => m[1])); - for (const name of used) if (!PLACEHOLDERS.includes(name)) problems.push(`prompt uses unknown placeholder {${name}}`); + for (const name of used) if (!PLACEHOLDERS.includes(name)) problems.push(`prompt uses unknown placeholder; expected one of ${PLACEHOLDERS.join(", ")}`); for (const name of REQUIRED) if (!used.has(name)) problems.push(`prompt must use {${name}}`); if (!text.trim()) problems.push('prompt must not be empty'); } @@ -87,7 +87,7 @@ export function validateInterviewTemplate(value) { export function loadInterviewTemplate(url = INTERVIEW_TEMPLATE_URL) { let value; try { value = JSON.parse(readFileSync(url, 'utf8')); } - catch (error) { throw new OnboardingError('onboarding-template-invalid', `interview template unreadable: ${error.message}`); } + catch (error) { throw new OnboardingError('onboarding-template-invalid', `interview template unreadable: expected readable JSON in ${url}${error.code ? ` (${error.code})` : ""}`); } const problems = validateInterviewTemplate(value); if (problems.length) throw new OnboardingError('onboarding-template-invalid', problems.join('; ')); return value; diff --git a/packages/project/tests/last-values.test.mjs b/packages/project/tests/last-values.test.mjs new file mode 100644 index 000000000..23b909250 --- /dev/null +++ b/packages/project/tests/last-values.test.mjs @@ -0,0 +1,35 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { loadInterviewTemplate, OnboardingError } from '../src/onboarding.mjs'; +import { initProject, writeRecord, ProjectError } from '../src/index.mjs'; +test('onboarding template parser omits file bytes and keeps its code', t => { + const file = join(scratch(t), 'template.json'); writeFileSync(file, secret); + assert.throws(() => loadInterviewTemplate(file), safe(file, OnboardingError, { code: 'onboarding-template-invalid', stage: 'check' })); +}); +test('project nonobject record already reports only its type (BASE control)', async t => { + const root = scratch(t), context = { business: { id: 'acme', human: 'jason', projects: { stack: { root: join(root, 'code') } } }, project: 'stack', dataRoot: join(root, 'data') }; + await initProject(context); + await assert.rejects(() => writeRecord({ ...context, path: 'requirements/REQ-1.json', record: JSON.stringify(secret), author: { name: 'Tester', email: 'test@example.invalid' } }), safe('object', ProjectError, { code: 'project-record-invalid' })); +}); +import { INTERVIEW_TEMPLATE_URL } from '../src/onboarding.mjs'; +for (const at of ['question', 'placeholder']) test(`template ${at} semantic refusal omits its value`, t => { + const d = JSON.parse(readFileSync(INTERVIEW_TEMPLATE_URL, 'utf8')); + if (at === 'question') { d.questions[0].key = secret.toLowerCase(); d.questions[1].key = secret.toLowerCase(); } + else d.prompt.push(`{${secret}}`); + const file = join(scratch(t), 'template.json'); writeFileSync(file, JSON.stringify(d)); + assert.throws(() => loadInterviewTemplate(file), safe(at === 'question' ? 'questions[1].key' : 'placeholder', OnboardingError, { code: 'onboarding-template-invalid' }, secret.toLowerCase())); +}); diff --git a/packages/project/tests/onboarding.test.mjs b/packages/project/tests/onboarding.test.mjs index d65189b66..2b1eb0411 100644 --- a/packages/project/tests/onboarding.test.mjs +++ b/packages/project/tests/onboarding.test.mjs @@ -46,12 +46,12 @@ test('a replacement template is checked key by key and every problem is named', [t => { t.drafts.requirementPrefix = '-x'; }, 'drafts.requirementPrefix must start a record id'], [t => { delete t.drafts.prd; }, 'drafts keys must be exactly'], [t => { t.questions = []; }, 'questions must list 1-12 questions'], - [t => { t.questions[1].key = t.questions[0].key; }, 'questions[1].key repeats outcome'], + [t => { t.questions[1].key = t.questions[0].key; }, 'questions[1].key repeats an earlier question key'], [t => { t.questions[0].key = 'Bad Key'; }, 'questions[0].key must be lower-case'], [t => { t.questions[0].text = ''; }, 'questions[0].text must be one line'], [t => { t.questions[0].extra = 1; }, 'questions[0] keys must be exactly key, text'], [t => { t.prompt = ['line\nbreak']; }, 'prompt must be 1-200 lines'], - [t => { t.prompt.push('{secret}'); }, 'prompt uses unknown placeholder {secret}'], + [t => { t.prompt.push('{secret}'); }, 'prompt uses unknown placeholder; expected one of'], [t => { t.prompt = t.prompt.map(l => l.replaceAll('{questions}', 'the questions')); }, 'prompt must use {questions}'], [t => { t.prompt = t.prompt.map(l => l.replaceAll('{northStarDraft}', 'a doc')); }, 'prompt must use {northStarDraft}'], ]; diff --git a/packages/queue/src/queue.mjs b/packages/queue/src/queue.mjs index 3d17d1877..36ad2fe00 100644 --- a/packages/queue/src/queue.mjs +++ b/packages/queue/src/queue.mjs @@ -99,18 +99,18 @@ export function checkCellText(v, what, opts) { // An op id a caller chose: 8 to 72 characters, not ending in `.outcome`. export function checkCallerOpId(v, what = "op id") { - if (typeof v !== "string" || !CALLER_OP_RE.test(v)) throw refuse(`${what} ${JSON.stringify(v)} must match ${CALLER_OP_RE.source} (8 to 72 characters)`); - if (v.endsWith(".outcome")) throw refuse(`${what} ${JSON.stringify(v)} ends in .outcome, which is reserved`); + if (typeof v !== "string" || !CALLER_OP_RE.test(v)) throw refuse(`${what} must match ${CALLER_OP_RE.source} (8 to 72 characters)`); + if (v.endsWith(".outcome")) throw refuse(`${what} ends in .outcome, which is reserved`); return v; } export function checkName(v, what) { - if (typeof v !== "string" || !NAME_RE.test(v)) throw refuse(`${what} must be a seat name (lowercase letters, digits, hyphens): ${JSON.stringify(v)}`); + if (typeof v !== "string" || !NAME_RE.test(v)) throw refuse(`${what} must be a seat name (lowercase letters, digits, hyphens)`); return v; } function checkId(v, what = "row id") { - if (!Number.isInteger(v) || v < 1 || v > 1_000_000) throw refuse(`${what} must be a positive integer: ${JSON.stringify(v)}`); + if (!Number.isInteger(v) || v < 1 || v > 1_000_000) throw refuse(`${what} must be a positive integer`); return v; } @@ -148,14 +148,14 @@ function checkTime(v, what, { unknown = false, date = false } = {}) { // as March 2). const t = Date.parse(iso ? v : `${v}T00:00:00.000Z`); if (Number.isFinite(t) && new Date(t).toISOString().slice(0, v.length) === v) return v; - throw refuse(`${what} is not a calendar ${iso ? "time" : "date"}: ${JSON.stringify(v)}`); + throw refuse(`${what} is not a calendar ${iso ? "time" : "date"}`); } - throw refuse(`${what} must be an ISO time${date ? " or date" : ""}${unknown ? " or \"unknown\"" : ""}: ${JSON.stringify(v)}`); + throw refuse(`${what} must be an ISO time${date ? " or date" : ""}${unknown ? " or \"unknown\"" : ""}`); } export function checkRepoPath(v, what) { if (typeof v !== "string" || !PATH_RE.test(v) || v.split("/").some((s) => s === "." || s === "..")) { - throw refuse(`${what} must be a repo-relative path: ${JSON.stringify(v)}`); + throw refuse(`${what} must be a repo-relative path`); } return v; } @@ -163,7 +163,7 @@ export function checkRepoPath(v, what) { export function parseBriefSpec(spec) { if (typeof spec !== "string") throw refuse("brief must be PATH#ANCHOR"); const i = spec.indexOf("#"); - if (i < 0) throw refuse(`brief must be PATH#ANCHOR, naming the heading: ${JSON.stringify(spec)}`); + if (i < 0) throw refuse(`brief must be PATH#ANCHOR, naming the heading`); const path = checkRepoPath(spec.slice(0, i), "brief path"); const anchor = checkCellText(spec.slice(i + 1), "brief anchor", { max: 200 }); return { path, anchor }; @@ -197,9 +197,9 @@ export function parseManifest(text) { const lines = text.slice(0, -1).split("\n"); for (const line of lines) { const m = /^([0-9a-f]{64}) {2}(.+)$/.exec(line); - if (!m) throw refuse(`candidate manifest line is not " ": ${JSON.stringify(line)}`); + if (!m) throw refuse(`candidate manifest line is not " "`); checkRepoPath(m[2], "candidate manifest path"); - if (seen.has(m[2])) throw refuse(`candidate manifest names ${m[2]} twice`); + if (seen.has(m[2])) throw refuse(`candidate manifest names a path twice`); seen.add(m[2]); } return lines.length; @@ -222,7 +222,7 @@ export function attemptOf(row, op) { } function checkCommentId(v, what = "comment id") { - if (!Number.isSafeInteger(v) || v < 1) throw refuse(`${what} must be a positive integer: ${JSON.stringify(v)}`); + if (!Number.isSafeInteger(v) || v < 1) throw refuse(`${what} must be a positive integer`); return v; } @@ -250,7 +250,7 @@ function checkAttempt(a) { checkCallerOpId(a.op, "review attempt op"); checkName(a.by, "review attempt by"); checkTime(a.at, "review attempt at"); - if (!ATTEMPT_STATES.includes(a.state)) throw refuse(`review attempt state ${JSON.stringify(a.state)} is not one of ${ATTEMPT_STATES.join(", ")}`); + if (!ATTEMPT_STATES.includes(a.state)) throw refuse(`review attempt state is not one of ${ATTEMPT_STATES.join(", ")}`); if (a.comment !== null) checkCommentId(a.comment); if ((a.state === "posted") !== (a.comment !== null)) throw refuse("a review attempt carries a comment id exactly when it is posted"); if (a.transport !== null) checkTransport(a.transport); @@ -311,7 +311,7 @@ function checkRound(v, n) { } export function validateRow(row) { - keysExactly(row, ROW_KEYS, `row ${row?.id}`); + keysExactly(row, ROW_KEYS, `row`); const w = `row ${row.id}`; checkId(row.id); checkCellText(row.piece, `${w} piece`, { max: 300 }); @@ -319,7 +319,7 @@ export function validateRow(row) { checkIssues(row.issues, `${w} issues`); checkIssues(row.closes, `${w} closes`); for (const n of row.closes) if (!row.issues.includes(n)) throw refuse(`${w} closes #${n}, which is not in its issues`); - if (!STATES.includes(row.state)) throw refuse(`${w} state ${JSON.stringify(row.state)} is not a state`); + if (!STATES.includes(row.state)) throw refuse(`${w} state is not a state (expected one of ${STATES.join(", ")})`); if (row.state === "blocked") { if (!NON_TERMINAL.has(row.previousState)) throw refuse(`${w} is blocked and needs the non-terminal state it returns to`); checkCellText(row.blockedReason, `${w} blockedReason`); @@ -346,7 +346,7 @@ export function validateRow(row) { } if (row.claim !== null) { keysExactly(row.claim, ["seat", "op"], `${w} claim`); - if (row.claim.seat !== row.owner) throw refuse(`${w} claim seat ${row.claim.seat} is not the owner ${row.owner}`); + if (row.claim.seat !== row.owner) throw refuse(`${w} claim seat is not the owner`); checkCallerOpId(row.claim.op, `${w} claim op`); const s = row.state === "blocked" ? row.previousState : row.state; if (!CLAIM_KEPT.has(s)) throw refuse(`${w} is claimed but ${row.state}`); @@ -398,7 +398,7 @@ function nullable(v, fn) { function sortedUnique(list, cmp) { const out = [...list].sort(cmp); - for (let i = 1; i < out.length; i++) if (cmp(out[i], out[i - 1]) === 0) throw refuse(`repeated value ${JSON.stringify(out[i])}`); + for (let i = 1; i < out.length; i++) if (cmp(out[i], out[i - 1]) === 0) throw refuse(`repeated value in list`); return out; } @@ -463,7 +463,7 @@ export function canonArgs(verb, a) { }; } case "move": - if (!STATES.includes(a.to)) throw refuse(`unknown state ${JSON.stringify(a.to)}; states: ${STATES.join(", ")}`); + if (!STATES.includes(a.to)) throw refuse(`unknown state; states: ${STATES.join(", ")}`); return { id: checkId(a.id), to: a.to, @@ -503,7 +503,7 @@ export function canonArgs(verb, a) { if (a.verdict !== "approve" && a.verdict !== "changes") throw refuse("--verdict must be approve or changes"); return { id: checkId(a.id), verdict: a.verdict, comment: checkCommentId(a.comment), candidate: checkDigest(a.candidate, "candidate") }; default: - throw refuse(`unknown verb ${JSON.stringify(verb)}`); + throw refuse(`unknown verb; expected a queue operation`); } } @@ -517,8 +517,8 @@ const isPriv = (by) => PRIVILEGED.has(by); function ownerOrPriv(row, by, doing) { if (by === row.owner || isPriv(by)) return; - const claimed = row.claim !== null ? `row ${row.id} is claimed by ${row.claim.seat}; ` : ""; - throw refuse(`${claimed}only the owner (${row.owner}) or a privileged actor may ${doing} row ${row.id}`); + const claimed = row.claim !== null ? `row ${row.id} is claimed; ` : ""; + throw refuse(`${claimed}only the owner or a privileged actor may ${doing} row ${row.id}`); } function requirePriv(by, doing) { @@ -580,7 +580,7 @@ function unresolvedAttempts(row) { function refuseUnresolved(row, doing) { const open = unresolvedAttempts(row); - if (open.length) throw refuse(`row ${row.id} has an unresolved review request: ${open.join(", ")}; resolve or abandon it before ${doing}`); + if (open.length) throw refuse(`row ${row.id} has an unresolved review request; resolve or abandon it before ${doing}`); } // A comment round leaves in-review, for done or for Jason's gate, only on @@ -589,7 +589,7 @@ function requireApprovals(row, cur, to) { if (row.reviewers.length === 0) throw refuse(`row ${row.id} lists no reviewers now; a privileged actor sets them before it ${to === "done" ? "closes" : `moves to ${to}`}`); const approved = new Set(cur.receipts.filter((r) => r.verdict === "approve").map((r) => r.reviewer)); const missing = row.reviewers.filter((s) => !approved.has(s)); - if (missing.length) throw refuse(`row ${row.id} round ${cur.n} has no approval recorded by ${missing.join(", ")}`); + if (missing.length) throw refuse(`row ${row.id} round ${cur.n} has no approval recorded by every listed reviewer`); } // The request round an attempt belongs to, and the attempt. @@ -599,7 +599,7 @@ function findAttempt(row, op) { const i = r.attempts.findIndex((x) => x.op === op); if (i >= 0) return { round: r, index: i, attempt: r.attempts[i] }; } - throw refuse(`row ${row.id} has no review request ${op}`); + throw refuse(`row ${row.id} has no review request matching the attempt op`); } // A copy of `row` with one attempt of one round replaced. @@ -632,20 +632,20 @@ function outcomeState(attempt, t) { return t.outcome === "posted" && t.comment === attempt.comment ? "posted" : "conflict"; } if (attempt.state === "abandoned") return t.outcome === "posted" ? "conflict" : "abandoned"; - throw refuse(`attempt ${attempt.op} is ${attempt.state}; it already has its outcome`); + throw refuse(`attempt is ${attempt.state}; it already has its outcome`); } function applyReview(rows, row, entry) { const a = entry.args; const by = entry.by; - if (!row.review && entry.verb !== "review-request" && entry.verb !== "review-record") throw refuse(`row ${row.id} has no review request ${a.attempt}`); + if (!row.review && entry.verb !== "review-request" && entry.verb !== "review-record") throw refuse(`row ${row.id} has no review request matching the attempt op`); switch (entry.verb) { case "review-request": { ownerOrPriv(row, by, "request review of"); const cur = currentCommentRound(row, "review request"); const live = cur.attempts.filter((x) => x.state !== "failed" && x.state !== "abandoned"); refuseUnresolved(row, "a new request"); - if (live.length) throw refuse(`row ${row.id} round ${cur.n} already has a posted request (${live[0].op}, comment ${live[0].comment})`); + if (live.length) throw refuse(`row ${row.id} round ${cur.n} already has a posted request`); const rounds = row.review.rounds.map((r) => (r.n === cur.n ? { ...r, attempts: [...r.attempts, newAttempt(entry)] } : r)); return { row: { ...row, review: { rounds } }, @@ -654,11 +654,11 @@ function applyReview(rows, row, entry) { }; } case "review-outcome": { - if (entry.op !== `${a.attempt}.outcome`) throw refuse(`an outcome's op is its attempt's op plus .outcome (${a.attempt}.outcome)`); + if (entry.op !== `${a.attempt}.outcome`) throw refuse(`an outcome's op is its attempt's op plus .outcome`); const found = findAttempt(row, a.attempt); const att = found.attempt; - if (by !== att.by) throw refuse(`only ${att.by}, who made request ${att.op}, records its outcome`); - if (att.transport !== null) throw refuse(`request ${att.op} already has its outcome`); + if (by !== att.by) throw refuse(`only the actor who made the request records its outcome`); + if (att.transport !== null) throw refuse(`request already has its outcome`); const t = { outcome: a.outcome, status: a.status, comment: a.comment, detail: a.detail }; const state = outcomeState(att, t); const comment = state === "posted" ? (att.comment ?? t.comment) : null; @@ -678,7 +678,7 @@ function applyReview(rows, row, entry) { else requirePriv(by, "abandon a review request"); const found = findAttempt(row, a.attempt); const att = found.attempt; - if (!UNRESOLVED.has(att.state)) throw refuse(`request ${att.op} is ${att.state}; only a requesting, uncertain or conflict request can be ${resolve ? "resolved" : "abandoned"}`); + if (!UNRESOLVED.has(att.state)) throw refuse(`request is ${att.state}; only a requesting, uncertain or conflict request can be ${resolve ? "resolved" : "abandoned"}`); const res = { verb: resolve ? "resolve" : "abandon", op: entry.op, by, at: entry.at, comment: resolve ? a.comment : null, reason: resolve ? null : a.reason }; const state = resolve ? "posted" : "abandoned"; const next = withAttempt(row, found, { ...att, state, comment: resolve ? a.comment : null, resolutions: [...att.resolutions, res] }, resolve ? {} : { duplicateRisk: true }); @@ -689,11 +689,11 @@ function applyReview(rows, row, entry) { }; } case "review-record": { - if (!row.reviewers.includes(by) || by === row.owner) throw refuse(`only a listed reviewer other than the owner may record a verdict on row ${row.id} (reviewers: ${fmt(row.reviewers)})`); + if (!row.reviewers.includes(by) || by === row.owner) throw refuse(`only a listed reviewer other than the owner may record a verdict on row ${row.id}`); if (!row.review) throw refuse(`row ${row.id} has no review round`); const cur = currentCommentRound(row, "review record"); - if (a.candidate !== cur.candidate.digest) throw refuse(`candidate ${a.candidate} is not round ${cur.n}'s candidate ${cur.candidate.digest}`); - if (cur.receipts.some((r) => r.reviewer === by)) throw refuse(`${by} already recorded a verdict for row ${row.id} round ${cur.n}`); + if (a.candidate !== cur.candidate.digest) throw refuse(`candidate is not round ${cur.n}'s candidate`); + if (cur.receipts.some((r) => r.reviewer === by)) throw refuse(`reviewer already recorded a verdict for row ${row.id} round ${cur.n}`); const rec = { reviewer: by, op: entry.op, at: entry.at, verdict: a.verdict, comment: a.comment, candidate: a.candidate }; const rounds = row.review.rounds.map((r) => (r.n === cur.n ? { ...r, receipts: [...r.receipts, rec] } : r)); return { @@ -703,7 +703,7 @@ function applyReview(rows, row, entry) { }; } default: - throw refuse(`unknown verb ${entry.verb}`); + throw refuse(`unknown verb; expected a queue operation`); } } @@ -759,14 +759,14 @@ function applyMove(rows, row, entry, resolved) { } else if (from === "queued" && to === "briefed") { requirePriv(by, "accept a brief (queued→briefed)"); } else if (from === "briefed" && to === "in-progress") { - if (by !== row.owner) throw refuse(`only the owner (${row.owner}) may start row ${row.id}`); + if (by !== row.owner) throw refuse(`only the owner may start row ${row.id}`); const missing = afterSatisfied(rows, row); if (missing.length) throw refuse(`row ${row.id} waits on ${missing.join(", ")}`); next.claim = { seat: row.owner, op: entry.op }; } else if (from === "in-progress" && to === "briefed") { throw refuse(`use \`queue release ${row.id}\` to give up a claim`); } else if (from === "in-progress" && to === "in-review") { - if (row.claim === null || by !== row.claim.seat) throw refuse(`only the claimant (${row.claim?.seat ?? "nobody"}) may request review of row ${row.id}`); + if (row.claim === null || by !== row.claim.seat) throw refuse(`only the claimant may request review of row ${row.id}`); if (candidate === null) throw refuse("in-progress→in-review needs --candidate "); revIssue = reviewIssue(row, issue); cand = checkCandidate(resolved.candidate); @@ -797,7 +797,7 @@ function applyMove(rows, row, entry, resolved) { next.claim = null; } else if (from === "in-review" && to === "done") { if (row.gateOwner === "jason") throw refuse(`row ${row.id}'s gate is Jason's; it goes through waiting-on-jason`); - if (by !== row.gateOwner && !isPriv(by)) throw refuse(`only the gate owner (${row.gateOwner}) or a privileged actor may close row ${row.id}`); + if (by !== row.gateOwner && !isPriv(by)) throw refuse(`only the gate owner or a privileged actor may close row ${row.id}`); refuseUnresolved(row, "closing it"); const cur = row.review?.rounds.at(-1); if (cur?.request === "comment") { @@ -808,7 +808,7 @@ function applyMove(rows, row, entry, resolved) { const ev = parseReviewEvidence(evidence); if (!cur) throw refuse(`row ${row.id} has no review round to cite`); if (ev.round !== cur.n) throw refuse(`evidence names round ${ev.round}; row ${row.id} is in round ${cur.n}`); - if (ev.candidate !== cur.candidate.digest) throw refuse(`evidence candidate ${ev.candidate} is not round ${cur.n}'s candidate ${cur.candidate.digest}`); + if (ev.candidate !== cur.candidate.digest) throw refuse(`evidence candidate is not round ${cur.n}'s candidate`); } round = cur.n; next.claim = null; @@ -834,7 +834,7 @@ function applySet(rows, row, entry, resolved) { switch (field) { case "piece": case "gate": case "reviewers": case "issues": requirePriv(by, `change ${field}`); - if (field === "reviewers" && value.includes(row.owner)) throw refuse(`row ${row.id} owner ${row.owner} can't be a listed reviewer`); + if (field === "reviewers" && value.includes(row.owner)) throw refuse(`row ${row.id} owner can't be a listed reviewer`); next[field] = value; // N10: closes follows the issues only if nobody narrowed it. A logged // narrowing keeps its intersection with the new issues. @@ -860,7 +860,7 @@ function applySet(rows, row, entry, resolved) { requirePriv(by, "re-pin a brief"); next.brief = checkBrief(resolved.brief); if (next.brief.path !== parseBriefSpec(value).path || next.brief.anchor !== parseBriefSpec(value).anchor) throw refuse("resolved brief does not match the brief argument"); - if (row.brief !== null && next.brief.blob === row.brief.blob && sameJson(next.brief, row.brief)) throw refuse(`row ${row.id} brief is already pinned to ${row.brief.blob}`); + if (row.brief !== null && next.brief.blob === row.brief.blob && sameJson(next.brief, row.brief)) throw refuse(`row ${row.id} brief is already pinned to that blob`); break; case "required": if (value) { @@ -877,7 +877,7 @@ function applySet(rows, row, entry, resolved) { throw refuse(`set cannot change ${field}`); } if (sameJson(next[key], row[key]) && (field !== "issues" || sameJson(next.closes, row.closes))) { - throw refuse(`row ${row.id} ${key} is already ${fmt(row[key])}`); + throw refuse(`row ${row.id} ${key} is already set to that value`); } const result = { row: row.id, field: key, from: row[key], to: next[key] }; if (field === "issues") result.closes = { from: row.closes, to: next.closes, narrowed: !sameJson(row.closes, row.issues) }; @@ -908,7 +908,7 @@ export function applyEntry(state, entry, resolved) { const id = highWater + 1; const owner = a.owner ?? by; const reviewers = a.reviewers ?? []; - if (reviewers.includes(owner)) throw refuse(`row ${id} owner ${owner} can't be a listed reviewer`); + if (reviewers.includes(owner)) throw refuse(`row ${id} owner can't be a listed reviewer`); highWater = id; const required = a.required ?? false; const row = { @@ -947,7 +947,7 @@ export function applyEntry(state, entry, resolved) { case "release": { const row = getRow(rows, a.id); if (row.state !== "in-progress") throw refuse(`row ${row.id} is ${row.state}; release applies to in-progress rows`); - if (!(row.claim && by === row.claim.seat) && !isPriv(by)) throw refuse(`only the claimant (${row.claim?.seat}) or a privileged actor may release row ${row.id}`); + if (!(row.claim && by === row.claim.seat) && !isPriv(by)) throw refuse(`only the claimant or a privileged actor may release row ${row.id}`); rows.set(row.id, touch({ ...row, state: "briefed", claim: null }, entry)); result = { row: row.id, from: "in-progress", to: "briefed", receipt: receipt(entry, rev, `row ${row.id} in-progress→briefed`) }; break; @@ -956,8 +956,8 @@ export function applyEntry(state, entry, resolved) { const row = getRow(rows, a.id); requirePriv(by, "assign a row"); if (row.state === "done") throw refuse(`row ${row.id} is done; done rows never change`); - if (row.owner === a.seat) throw refuse(`row ${row.id} is already owned by ${a.seat}`); - if (row.reviewers.includes(a.seat)) throw refuse(`can't assign row ${row.id} to ${a.seat}: ${a.seat} is one of its reviewers`); + if (row.owner === a.seat) throw refuse(`row ${row.id} is already owned by that seat`); + if (row.reviewers.includes(a.seat)) throw refuse(`can't assign row ${row.id}: seat is one of its reviewers`); const claim = row.claim ? { seat: a.seat, op: entry.op } : null; rows.set(row.id, touch({ ...row, owner: a.seat, claim }, entry)); result = { row: row.id, field: "owner", from: row.owner, to: a.seat, receipt: receipt(entry, rev, `row ${row.id} owner: ${row.owner}→${a.seat}`) }; @@ -966,7 +966,7 @@ export function applyEntry(state, entry, resolved) { case "note": { const row = getRow(rows, a.id); if (row.state === "done" || row.state === "parked") throw refuse(`row ${row.id} is ${row.state}; notes are closed`); - if (by !== row.owner && !row.reviewers.includes(by) && !isPriv(by)) throw refuse(`only the owner (${row.owner}), a listed reviewer or a privileged actor may note row ${row.id}`); + if (by !== row.owner && !row.reviewers.includes(by) && !isPriv(by)) throw refuse(`only the owner, a listed reviewer or a privileged actor may note row ${row.id}`); // On a blocked row the note is the reason (8.7: "update the reason // with note"); elsewhere it is the row's note, and "" clears it. const field = row.state === "blocked" ? "blockedReason" : "note"; @@ -1003,7 +1003,7 @@ export function applyEntry(state, entry, resolved) { case "genesis": throw refuse("genesis is only log[0]"); default: - throw refuse(`unknown verb ${entry.verb}`); + throw refuse(`unknown verb; expected a queue operation`); } validateRows([...rows.values()]); return { state: { rows, highWater, retired: state.retired, revision: rev }, result }; @@ -1029,11 +1029,11 @@ export function parseMigrationMap(text) { keysExactly(map, ["rows", "retired", "highWater"], "migration map"); if (!Array.isArray(map.rows)) throw refuse("migration map rows must be a list"); map.rows.forEach((r) => { - keysExactly(r, MAP_ROW_KEYS, `migration map row ${r?.id}`); - if (r.brief !== null) keysExactly(r.brief, ["path", "anchor"], `migration map row ${r.id} brief`); + keysExactly(r, MAP_ROW_KEYS, `migration map row`); + if (r.brief !== null) keysExactly(r.brief, ["path", "anchor"], `migration map row brief`); // The rule add, set and assign apply. Here, not in checkGenesis, so a // log written before the rule still replays. - if (Array.isArray(r.reviewers) && r.reviewers.includes(r.owner)) throw refuse(`migration map row ${r.id} owner ${r.owner} can't be a listed reviewer`); + if (Array.isArray(r.reviewers) && r.reviewers.includes(r.owner)) throw refuse(`migration map row owner can't be a listed reviewer`); }); if (!Array.isArray(map.retired)) throw refuse("migration map retired must be a list"); map.retired.forEach((id) => checkId(id, "retired id")); @@ -1102,8 +1102,8 @@ export function rowsArray(state) { function checkEntryShape(e, i) { keysExactly(e, ENTRY_KEYS, `log entry ${i}`); - if (e.rev !== i) throw refuse(`log entry ${i} has rev ${e.rev}`); - if (!VERBS.includes(e.verb)) throw refuse(`log entry ${i} verb ${JSON.stringify(e.verb)} is unknown`); + if (e.rev !== i) throw refuse(`log entry ${i} has rev that must equal its index`); + if (!VERBS.includes(e.verb)) throw refuse(`log entry ${i} verb is unknown`); if (e.verb === "review-outcome") { if (typeof e.op !== "string" || !LOG_OP_RE.test(e.op) || !isObj(e.args) || e.op !== `${e.args.attempt}.outcome`) throw refuse(`log entry ${i} op must be its attempt's op plus .outcome`); } else { @@ -1111,7 +1111,7 @@ function checkEntryShape(e, i) { } checkName(e.by, `log entry ${i} by`); checkTime(e.at, `log entry ${i} at`); - if (!Number.isInteger(e.semantics) || e.semantics < 1 || e.semantics > SEMANTICS) throw refuse(`log entry ${i} semantics ${e.semantics} is not 1 to ${SEMANTICS}`); + if (!Number.isInteger(e.semantics) || e.semantics < 1 || e.semantics > SEMANTICS) throw refuse(`log entry ${i} semantics is not 1 to ${SEMANTICS}`); if (REVIEW_VERBS.includes(e.verb) && e.semantics < 2) throw refuse(`log entry ${i} ${e.verb} needs semantics 2`); if (typeof e.viewSha !== "string" || !SHA256_RE.test(e.viewSha)) throw refuse(`log entry ${i} viewSha is not a SHA-256`); } @@ -1129,16 +1129,16 @@ export function loadDoc(bytes) { try { doc = JSON.parse(text); } catch { throw refuse(`queue.json is not JSON`); } keysExactly(doc, DOC_KEYS, "queue.json"); if (serialize(doc) !== text) throw refuse("queue.json does not re-serialize byte for byte (a hand edit?)"); - if (doc.version !== VERSION) throw refuse(`queue.json version ${doc.version} is not ${VERSION}`); + if (doc.version !== VERSION) throw refuse(`queue.json version is not ${VERSION}`); if (typeof doc.canonicalRoot !== "string" || !doc.canonicalRoot.startsWith("/")) throw refuse("canonicalRoot must be an absolute path"); if (!Array.isArray(doc.rows) || !Array.isArray(doc.log) || doc.log.length === 0) throw refuse("queue.json needs rows and a log starting at genesis"); doc.log.forEach(checkEntryShape); const ops = new Set(); for (const e of doc.log) { - if (ops.has(e.op)) throw refuse(`op id ${e.op} appears twice in the log`); + if (ops.has(e.op)) throw refuse(`op id appears twice in the log`); ops.add(e.op); } - if (doc.revision !== doc.log.length - 1) throw refuse(`revision ${doc.revision} is not the last rev ${doc.log.length - 1}`); + if (doc.revision !== doc.log.length - 1) throw refuse(`revision is not the last rev ${doc.log.length - 1}`); const state = replay(doc.log); if (!sameJson(rowsArray(state), doc.rows)) throw refuse("rows do not equal the replay of the log (a hand edit?)"); if (doc.canonicalRoot !== doc.log[0].args.root) throw refuse("canonicalRoot differs from the genesis root"); @@ -1158,10 +1158,10 @@ export function replay(log) { try { out = applyEntry(state, e, resolvedFromResult(e.verb, e.args, e.result)); } catch (err) { - if (err instanceof QueueError) throw refuse(`log entry ${i} (${e.op}) does not replay: ${err.message}`); + if (err instanceof QueueError) throw refuse(`log entry ${i} does not replay: ${err.message}`); throw err; } - if (!sameJson(out.result, e.result)) throw refuse(`log entry ${i} (${e.op}) result differs from its replay`); + if (!sameJson(out.result, e.result)) throw refuse(`log entry ${i} result differs from its replay`); state = out.state; if (e.viewSha !== sha256(render(rowsArray(state), i))) throw refuse(`log entry ${i} viewSha is not the render of its rows`); } diff --git a/packages/queue/tests/data.test.mjs b/packages/queue/tests/data.test.mjs index 714da63c4..5669d4330 100644 --- a/packages/queue/tests/data.test.mjs +++ b/packages/queue/tests/data.test.mjs @@ -119,8 +119,8 @@ test("add: defaults for an ordinary seat, privileged extras, refusals", () => { refused(() => step(d, "add", { ...add, owner: "rocko" }, "dewey", { brief: brief() }), /only a privileged actor may set owner/); refused(() => step(d, "add", { ...add, reviewers: ["rocko"] }, "dewey", { brief: brief() }), /reviewers/); refused(() => step(d, "add", { ...add, required: true }, "dewey", { brief: brief() }), /required/); - refused(() => step(d, "add", { ...add, owner: "rocko", reviewers: ["rocko"] }, "sage", { brief: brief() }), /row 12 owner rocko can't be a listed reviewer/); - refused(() => step(d, "add", { ...add, reviewers: ["sage"] }, "sage", { brief: brief() }), /row 12 owner sage can't be a listed reviewer/); + refused(() => step(d, "add", { ...add, owner: "rocko", reviewers: ["rocko"] }, "sage", { brief: brief() }), /row 12 owner can't be a listed reviewer/); + refused(() => step(d, "add", { ...add, reviewers: ["sage"] }, "sage", { brief: brief() }), /row 12 owner can't be a listed reviewer/); const d2 = step(d, "add", { ...add, owner: "rocko", gateOwner: "filbert", after: [{ id: 9, when: "done" }], reviewers: ["dewey"], required: true }, "sage", { brief: brief() }); const r2 = row(d2, 12); assert.deepEqual([r2.owner, r2.gateOwner, r2.required, r2.after], ["rocko", "filbert", true, [{ id: 9, when: "done" }]]); @@ -134,7 +134,7 @@ test("matrix: queued→briefed privileged; briefed→in-progress owner with afte let d = step(genesisDoc(), "add", { piece: "N", gate: "g", brief: "docs/plans/brief-b.md#Queue" }, "dewey", { brief: brief() }); refused(() => step(d, "move", mv(12, "briefed"), "dewey"), /privileged/); d = step(d, "move", mv(12, "briefed"), "sage"); - refused(() => step(d, "move", mv(12, "in-progress"), "sage"), /only the owner \(dewey\) may start/); + refused(() => step(d, "move", mv(12, "in-progress"), "sage"), /only the owner may start/); d = step(d, "move", mv(12, "in-progress"), "dewey"); assert.deepEqual(row(d, 12).claim.seat, "dewey"); refused(() => step(d, "move", mv(12, "in-progress"), "dewey"), /not a transition/); @@ -158,7 +158,7 @@ test("matrix: release, review round, changes requested and waiting-on-jason", () assert.deepEqual(Object.keys(rv), ["rounds"]); assert.deepEqual(rv.rounds.map((r) => [r.n, r.issue, r.request, r.candidate.digest]), [[1, 1508, "none", CAND.digest]]); assert.match(d.log.at(-1).result.receipt, /in-progress→in-review round 1 on #1508$/); - refused(() => step(d, "move", mv(9, "in-progress"), "dewey"), /claimed by darkwing/); + refused(() => step(d, "move", mv(9, "in-progress"), "dewey"), /claimed/); d = step(d, "move", mv(9, "in-progress"), "darkwing"); assert.equal(row(d, 9).claim.seat, "darkwing"); d = step(d, "move", mv(9, "in-review", { candidate: "y" }), "darkwing", { candidate: { kind: "commit", digest: BLOB2, text: null } }); @@ -182,7 +182,7 @@ test("matrix J5: in-review→done by the gate owner with evidence naming the cur refused(() => step(d, "move", mv(9, "done", { evidence: `comment=4242,candidate=${CAND.digest}` }), "filbert"), /round=/); refused(() => step(d, "move", mv(9, "done", { evidence: `comment=1,round=1,candidate=${"d".repeat(64)}` }), "filbert"), /is not round 1's candidate/); refused(() => step(d, "move", mv(9, "done", { evidence: `comment=4242,round=2,candidate=${CAND.digest}` }), "filbert"), /evidence names round 2; row 9 is in round 1/); - refused(() => step(d, "move", mv(9, "done", { evidence: ev }), "rocko"), /only the gate owner \(filbert\)/); + refused(() => step(d, "move", mv(9, "done", { evidence: ev }), "rocko"), /only the gate owner/); const done = step(d, "move", mv(9, "done", { evidence: ev }), "filbert"); assert.equal(row(done, 9).state, "done"); step(d, "move", mv(9, "done", { evidence: ev }), "sage"); @@ -357,7 +357,7 @@ test("matrix R1: every state × target × actor class matches 8.7, gate owner ja test("matrix: blocked keeps the claim and returns only to previousState", () => { let d = row9Started(); refused(() => step(d, "move", mv(9, "blocked"), "darkwing"), /needs --reason/); - refused(() => step(d, "move", mv(9, "blocked", { reason: "x" }), "rocko"), /claimed by darkwing/); + refused(() => step(d, "move", mv(9, "blocked", { reason: "x" }), "rocko"), /claimed/); d = step(d, "move", mv(9, "blocked", { reason: "waiting on git" }), "darkwing"); const r = row(d, 9); assert.deepEqual([r.state, r.previousState, r.blockedReason, r.claim.seat], ["blocked", "in-progress", "waiting on git", "darkwing"]); @@ -391,8 +391,8 @@ test("field edits: who may change what", () => { const set = (id, field, value, reason = null) => ({ id, field, value, reason }); refused(() => step(d, "set", set(11, "piece", "x"), "dewey"), /privileged/); assert.match(step(d, "set", set(11, "piece", "Renamed"), "sage").log.at(-1).result.receipt, /row 11 piece: Brief template→Renamed$/); - refused(() => step(d, "set", set(9, "reviewers", ["darkwing"]), "sage"), /row 9 owner darkwing can't be a listed reviewer/); - refused(() => step(d, "set", set(9, "reviewers", ["filbert", "darkwing"]), "sage"), /row 9 owner darkwing can't be a listed reviewer/); + refused(() => step(d, "set", set(9, "reviewers", ["darkwing"]), "sage"), /row 9 owner can't be a listed reviewer/); + refused(() => step(d, "set", set(9, "reviewers", ["filbert", "darkwing"]), "sage"), /row 9 owner can't be a listed reviewer/); assert.deepEqual(row(step(d, "set", set(9, "reviewers", ["dewey"]), "sage"), 9).reviewers, ["dewey"]); refused(() => step(d, "set", set(9, "after", []), "sage"), /only jason may change after on a required row/); step(d, "set", set(9, "after", []), "jason"); @@ -508,7 +508,7 @@ test("every accepted text renders to nine cells on every row (N8)", () => { test("genesis: the map refuses an owner among its row's reviewers; replay doesn't (2026-10-04)", () => { const self = MAP_ROWS.map((m) => (m.id === 9 ? { ...m, reviewers: ["darkwing", "filbert"] } : m)); - refused(() => parseMigrationMap(mapText(self)), /migration map row 9 owner darkwing can't be a listed reviewer/); + refused(() => parseMigrationMap(mapText(self)), /migration map row owner can't be a listed reviewer/); // A log from before the rule still loads: the check is on the map, not in replay. const old = genesisDoc(self, "genesis-op-0001", { rows: self, retired: [7], highWater: 11 }); assert.deepEqual(row(loadDoc(Buffer.from(serialize(old))).doc, 9).reviewers, ["darkwing", "filbert"]); @@ -522,8 +522,8 @@ test("times and dates must be calendar values, not just the shape (2026-10-04)", validateRow({ ...r, createdAt: v }); } for (const v of ["2026-13-01", "2026-00-10", "2026-01-32", "2026-02-30", "2027-02-29", "2026-04-31"]) { - refused(() => validateRow({ ...r, requiredSince: v }), new RegExp(`row 9 requiredSince is not a calendar date: "${v}"`)); - refused(() => validateRow({ ...r, createdAt: v }), new RegExp(`row 9 createdAt is not a calendar date: "${v}"`)); + refused(() => validateRow({ ...r, requiredSince: v }), /row 9 requiredSince is not a calendar date/); + refused(() => validateRow({ ...r, createdAt: v }), /row 9 createdAt is not a calendar date/); } // ISO times: the same days, and times past the clock. V8 reads 2026-02-30 // as March 2 and 24:00 as the next day; only the round trip catches those. @@ -533,15 +533,15 @@ test("times and dates must be calendar values, not just the shape (2026-10-04)", } const T = "T12:00:00.000Z"; for (const v of [`2026-13-01${T}`, `2026-01-32${T}`, `2026-02-30${T}`, `2027-02-29${T}`, "2026-01-01T24:00:00.000Z", "2026-01-01T23:60:00.000Z", "2026-01-01T23:59:60.000Z"]) { - refused(() => validateRow({ ...r, updatedAt: v }), new RegExp(`row 9 updatedAt is not a calendar time: "${v}"`)); - refused(() => validateRow({ ...r, requiredSince: v }), new RegExp(`row 9 requiredSince is not a calendar time: "${v}"`)); + refused(() => validateRow({ ...r, updatedAt: v }), /row 9 updatedAt is not a calendar time/); + refused(() => validateRow({ ...r, requiredSince: v }), /row 9 requiredSince is not a calendar time/); } // A shape error keeps its own message. - refused(() => validateRow({ ...r, updatedAt: "2026-01-01" }), /row 9 updatedAt must be an ISO time: "2026-01-01"/); + refused(() => validateRow({ ...r, updatedAt: "2026-01-01" }), /row 9 updatedAt must be an ISO time/); // A log entry's time goes through the same check on replay. const d = step(genesisDoc(), "note", { id: 9, text: "x" }, "darkwing"); d.log[1].at = `2026-02-30${T}`; - refused(() => loadDoc(Buffer.from(serialize(d))), /log entry 1 at is not a calendar time: "2026-02-30T12:00:00.000Z"/); + refused(() => loadDoc(Buffer.from(serialize(d))), /log entry 1 at is not a calendar time/); }); test("replay holds every op id to the caller's rule (N11)", () => { @@ -550,10 +550,10 @@ test("replay holds every op id to the caller's rule (N11)", () => { // 72 characters replay; 73 to 80 no longer do. load(step(d, "note", { id: 9, text: "x" }, "darkwing", {}, "a".repeat(72))); for (const n of [73, 80]) { - refused(() => load(step(d, "note", { id: 9, text: "x" }, "darkwing", {}, "a".repeat(n))), /log entry 1 op .* must match/); + refused(() => load(step(d, "note", { id: 9, text: "x" }, "darkwing", {}, "a".repeat(n))), /log entry 1 op must match/); } // .outcome is reserved on every verb, accept-history included. - refused(() => load(step(d, "accept-history", { reason: "r" }, "sage", { oldWitness: null }, "accept-history-1.outcome")), /log entry 1 op .* ends in \.outcome/); + refused(() => load(step(d, "accept-history", { reason: "r" }, "sage", { oldWitness: null }, "accept-history-1.outcome")), /log entry 1 op ends in \.outcome/); refused(() => load(step(d, "note", { id: 9, text: "x" }, "darkwing", {}, "note-row9.outcome")), /\.outcome, which is reserved/); // The genesis op is checked too. load(genesisDoc(MAP_ROWS, "genesis-ok-0001")); @@ -586,11 +586,11 @@ test("note: owner, listed reviewer or privileged; empty clears", () => { test("assign moves the claim with the owner; done clears it", () => { let d = row9Started(); refused(() => step(d, "assign", { id: 9, seat: "dewey" }, "darkwing"), /privileged/); - refused(() => step(d, "assign", { id: 9, seat: "filbert" }, "sage"), /can't assign row 9 to filbert: filbert is one of its reviewers/); + refused(() => step(d, "assign", { id: 9, seat: "filbert" }, "sage"), /can't assign row 9: seat is one of its reviewers/); d = step(d, "assign", { id: 9, seat: "dewey" }, "sage", {}, "assign-row-9-dewey"); assert.deepEqual(row(d, 9).claim, { seat: "dewey", op: "assign-row-9-dewey" }); assert.match(d.log.at(-1).result.receipt, /owner: darkwing→dewey$/); - refused(() => step(d, "move", mv(9, "in-review", { candidate: "x" }), "darkwing", { candidate: CAND }), /only the claimant \(dewey\)/); + refused(() => step(d, "move", mv(9, "in-review", { candidate: "x" }), "darkwing", { candidate: CAND }), /only the claimant/); const unclaimed = step(genesisDoc(), "assign", { id: 11, seat: "rocko" }, "sage"); assert.equal(row(unclaimed, 11).claim, null); }); diff --git a/packages/queue/tests/last-values.test.mjs b/packages/queue/tests/last-values.test.mjs new file mode 100644 index 000000000..0a4c0f378 --- /dev/null +++ b/packages/queue/tests/last-values.test.mjs @@ -0,0 +1,129 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { checkCallerOpId, checkName, checkRepoPath, parseManifest, validateRow, loadDoc, serialize } from '../src/queue.mjs'; +import { QueueError } from '../src/errors.mjs'; +for (const [field, fn] of [ + ['op', () => checkCallerOpId(secret + '/', 'op')], + ['op', () => checkCallerOpId(secret.toLowerCase() + '.outcome', 'op')], + ['owner', () => checkName(secret, 'owner')], + ['path', () => checkRepoPath(secret + '/', 'path')], + ['manifest', () => parseManifest(secret + '\n')], + ['row', () => validateRow({ id: secret })], + ['version', () => loadDoc(Buffer.from(serialize({ version: secret, canonicalRoot: '/x', revision: 0, rows: [], log: [] })))], +]) test(`queue ${field} semantic refusal omits the value`, () => assert.throws(fn, safe(field, QueueError, { code: 2 }, secret.toLowerCase()))); +import { checkTransport, parseBriefSpec, genesisRows, parseMigrationMap, render, sha256, genesisReceipt } from '../src/queue.mjs'; +import { MAP_ROWS, mapText } from './helpers.mjs'; +const at = '2026-11-10T00:00:00.000Z'; +function doc() { + const map = parseMigrationMap(mapText()), rows = genesisRows(map, new Map(map.rows.filter(r => r.brief).map(r => [r.id, 'a'.repeat(40)])), 'genesis-op-0001', at, 'sage'); + const entry = { rev: 0, op: 'genesis-op-0001', verb: 'genesis', args: { root: '/x', branch: 'refactor', map: 'docs/map.md' }, by: 'sage', at, semantics: 1, + result: { mapBlob: 'b'.repeat(40), highWater: map.highWater, retired: map.retired, rows, legacyView: 'legacy\n', receipt: genesisReceipt('genesis-op-0001', rows.length) }, viewSha: sha256(render(rows, 0)) }; + return { version: 1, canonicalRoot: '/x', revision: 0, rows, log: [entry] }; +} +for (const [field, edit, value = secret] of [ + ['id', r => { r.id = secret; }], ['state', r => { r.state = secret; }], + ['createdAt', r => { r.createdAt = secret; }], + ['createdAt', r => { r.createdAt = '2026-02-30'; }, '2026-02-30'], + ['claim seat', r => { r.claim = { seat: secret, op: 'claimed-op-0001' }; }], +]) test(`row ${field} omits semantic value ${value === secret ? 'sentinel' : 'calendar control'}`, () => { + const r = doc().rows[0]; edit(r); assert.throws(() => validateRow(r), safe(field, QueueError, { code: 2 }, value)); +}); +for (const [field, edit] of [ + ['rev', d => { d.log[0].rev = secret; }], ['verb', d => { d.log[0].verb = secret; }], + ['semantics', d => { d.log[0].semantics = secret; }], ['revision', d => { d.revision = secret; }], +]) test(`queue ${field} omits semantic value from the log/document`, () => { + const d = doc(); edit(d); assert.throws(() => loadDoc(Buffer.from(serialize(d))), safe(field, QueueError, { code: 2 })); +}); +test('transport comment omits its value', () => assert.throws(() => checkTransport({ outcome: 'posted', status: 201, comment: secret, detail: 'fixture' }), safe('comment', QueueError, { code: 2 }))); +test('brief omits its unparseable spec', () => assert.throws(() => parseBriefSpec(secret), safe('brief', QueueError, { code: 2 }))); +test('manifest duplicate path omits the value', () => assert.throws(() => parseManifest(`${'a'.repeat(64)} ${secret}\n`.repeat(2)), safe('manifest', QueueError, { code: 2 }))); + +import { canonArgs, applyEntry, replay } from '../src/queue.mjs'; +const seatValue = secret.toLowerCase(), digestValue = 'a'.repeat(40); +function reviewRow(row, state = 'requesting') { + row.state = 'in-review'; + const posted = state === 'posted'; + row.review = { rounds: [{ n: 1, op: seatValue, by: 'darkwing', at, issue: 1511, + candidate: { kind: 'commit', digest: digestValue, text: null }, request: 'comment', + attempts: [{ op: seatValue, by: 'darkwing', at, state, comment: posted ? 1 : null, + transport: posted ? { outcome: 'posted', status: 201, comment: 1, detail: 'fixture' } : null, resolutions: [] }], + duplicateRisk: false, receipts: [] }] }; +} +function operation(verb, args, edit = () => {}, options = {}) { + const state = replay(doc().log), row = state.rows.get(6); + edit(row); + if (!options.unchecked) validateRow(row); + const by = options.by ?? 'sage', op = options.op ?? 'operation-op-0001'; + const entry = { verb, args: options.raw ? args : canonArgs(verb, { id: 6, ...args }), by, op, at, semantics: 2 }; + return applyEntry(state, entry, options.resolved ?? {}); +} +const ownerValue = row => { row.owner = seatValue; if (row.claim) row.claim.seat = seatValue; }; +for (const [name, field, fn, omitted = seatValue] of [ + ['canonical duplicate reviewer', 'repeated value', () => canonArgs('set', { id: 6, field: 'reviewers', value: [seatValue, seatValue] })], + ['canonical state', 'state', () => canonArgs('move', { id: 6, to: secret }), secret], + ['canonical verb', 'verb', () => canonArgs(secret, {}), secret], + ['claimed owner authority', 'owner', () => operation('move', { to: 'blocked', reason: 'pause' }, ownerValue, { by: 'outsider' })], + ['start owner', 'owner', () => operation('move', { to: 'in-progress' }, r => { ownerValue(r); r.state = 'briefed'; r.claim = null; }, { by: 'outsider' })], + ['review claimant', 'claimant', () => operation('move', { to: 'in-review', candidate: 'HEAD' }, ownerValue, { by: 'outsider' })], + ['close gate owner', 'gate owner', () => operation('move', { to: 'done' }, r => { reviewRow(r, 'posted'); r.gateOwner = seatValue; }, { by: 'outsider' })], + ['set owner reviewer', 'owner', () => operation('set', { field: 'reviewers', value: [seatValue] }, ownerValue)], + ['set stored piece', 'piece', () => operation('set', { field: 'piece', value: secret }, r => { r.piece = secret; }), secret], + ['set pinned brief', 'brief', () => { const r = doc().rows.find(r => r.id === 6), brief = { ...r.brief, blob: digestValue }; + return operation('set', { field: 'brief', value: `${brief.path}#${brief.anchor}` }, r => { r.brief = brief; }, { resolved: { brief } }); }, digestValue], + ['release claimant', 'claimant', () => operation('release', {}, ownerValue, { by: 'outsider' })], + ['assign existing owner', 'owned', () => operation('assign', { seat: seatValue }, ownerValue)], + ['assign reviewer', 'reviewers', () => operation('assign', { seat: seatValue }, r => { r.reviewers = [seatValue]; })], + ['note owner', 'owner', () => operation('note', { text: 'note' }, ownerValue, { by: 'outsider' })], + ['add owner reviewer', 'owner', () => operation('add', { piece: 'piece', gate: 'gate', brief: 'docs/brief.md#Brief', owner: seatValue, reviewers: [seatValue] }, () => {}, + { resolved: { brief: { path: 'docs/brief.md', anchor: 'Brief', blob: digestValue } } })], + ['unresolved request', 'unresolved review request', () => operation('review-request', {}, r => reviewRow(r))], + ['missing approval', 'approval', () => operation('move', { to: 'done' }, r => { reviewRow(r, 'posted'); r.gateOwner = 'sage'; r.reviewers = [seatValue]; })], + ['missing attempt without round', 'review request', () => operation('review-resolve', { attempt: seatValue, comment: 1 })], + ['missing attempt in round', 'review request', () => operation('review-resolve', { attempt: seatValue + '-missing', comment: 1 }, r => reviewRow(r))], + ['posted request', 'posted request', () => operation('review-request', {}, r => reviewRow(r, 'posted'))], + ['outcome op', 'outcome', () => operation('review-outcome', { attempt: seatValue, outcome: 'posted', status: 201, comment: 1, detail: 'fixture' }, r => reviewRow(r))], + ['outcome actor', 'outcome', () => operation('review-outcome', { attempt: seatValue, outcome: 'posted', status: 201, comment: 1, detail: 'fixture' }, r => { reviewRow(r); r.review.rounds[0].attempts[0].by = seatValue; }, + { by: 'outsider', op: seatValue + '.outcome' })], + ['duplicate outcome', 'outcome', () => operation('review-outcome', { attempt: seatValue, outcome: 'posted', status: 201, comment: 1, detail: 'fixture' }, r => reviewRow(r, 'posted'), + { by: 'darkwing', op: seatValue + '.outcome' })], + ['settled resolution', 'request', () => operation('review-resolve', { attempt: seatValue, comment: 1 }, r => reviewRow(r, 'posted'))], + ['reviewer list', 'reviewer', () => operation('review-record', { verdict: 'approve', comment: 1, candidate: digestValue }, r => { reviewRow(r, 'posted'); r.reviewers = [seatValue]; }, { by: 'outsider' })], + ['candidate mismatch', 'candidate', () => operation('review-record', { verdict: 'approve', comment: 1, candidate: 'b'.repeat(40) }, r => reviewRow(r, 'posted'), { by: 'filbert' }), digestValue], + ['duplicate verdict', 'verdict', () => operation('review-record', { verdict: 'approve', comment: 1, candidate: digestValue }, r => { reviewRow(r, 'posted'); r.reviewers = [seatValue]; + r.review.rounds[0].receipts = [{ reviewer: seatValue, op: 'receipt-op-0001', at, verdict: 'approve', comment: 1, candidate: digestValue }]; }, { by: seatValue })], + ['legacy candidate evidence', 'candidate', () => operation('move', { to: 'done', evidence: `comment=1,round=1,candidate=${'b'.repeat(40)}` }, r => { + reviewRow(r); r.gateOwner = 'sage'; const round = r.review.rounds[0]; round.request = 'none'; delete round.attempts; delete round.duplicateRisk; delete round.receipts; + }), digestValue], + ['direct unknown operation', 'verb', () => operation(secret, {}, () => {}, { raw: true }), secret], + // outcomeState also defends direct callers with an inconsistent transport record. + ['inconsistent outcome record', 'outcome', () => operation('review-outcome', { attempt: seatValue, outcome: 'posted', status: 201, comment: 1, detail: 'fixture' }, r => reviewRow(r, 'failed'), + { by: 'darkwing', op: seatValue + '.outcome', unchecked: true })], +]) test(`queue replay refusal ${name} omits stored values`, () => assert.throws(fn, safe(field, QueueError, { code: 2 }, omitted))); + +test('migration map shape omits an unchecked row id', () => assert.throws(() => parseMigrationMap(mapText([{ id: secret }])), safe('migration map row', QueueError, { code: 2 }))); +test('migration map brief shape omits an unchecked row id', () => { + const row = structuredClone(MAP_ROWS[1]); row.id = secret; row.brief = {}; + assert.throws(() => parseMigrationMap(mapText([row])), safe('brief', QueueError, { code: 2 })); +}); +test('migration map duplicate owner omits unvalidated values', () => { + const row = structuredClone(MAP_ROWS[1]); row.owner = secret; row.reviewers = [secret]; + assert.throws(() => parseMigrationMap(mapText([row])), safe('owner', QueueError, { code: 2 })); +}); +test('BASE control: genesis expected claim already omits the stored op', () => { + const d = doc(); d.log[0].op = seatValue; d.log[0].result.receipt = genesisReceipt(seatValue, d.rows.length); + assert.throws(() => loadDoc(Buffer.from(serialize(d))), safe('claim', QueueError, { code: 2 }, seatValue)); +}); diff --git a/packages/queue/tests/review.test.mjs b/packages/queue/tests/review.test.mjs index 7a3c46f75..dfc742946 100644 --- a/packages/queue/tests/review.test.mjs +++ b/packages/queue/tests/review.test.mjs @@ -115,7 +115,7 @@ test("a request posts once as the requester; a retry sends nothing", async (t) = const again = ok(s.run(move9(), "darkwing"), /\(already recorded at rev 3\)$/m); assert.match(again.err, /request review-9-00001 is posted \(row 9 round 1 on #1508\), comment 1000; nothing was sent again/); assert.equal(s.calls().length, 2); - no(s.run(request("request-9-0002"), "darkwing"), 2, /round 1 already has a posted request \(review-9-00001, comment 1000\)/); + no(s.run(request("request-9-0002"), "darkwing"), 2, /round 1 already has a posted request/); no(s.run(["review", "outcome", "9"], "darkwing"), 4, /review takes request, resolve, abandon, record or verify-commit/); throwsCode(() => s.mut({ verb: "review-outcome", op: "review-9-00001.outcome", args: { id: 9, attempt: "review-9-00001", outcome: "failed", status: 403, comment: null, detail: "x" }, by: "darkwing" }), 2, /records its own outcome/); }); @@ -238,10 +238,10 @@ test("an unresolved request blocks a new request, a new round, waiting-on-jason no(s.run(move9(), "darkwing"), 3, /^Look on #1508 for a comment carrying \.$/m); s.rules([]); const head = s.head(); - no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\)/); + no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request/); ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "77", "--candidate", head, "--op", "record-9-filbert1"], "filbert")); - no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before closing it/); - no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before moving it to waiting-on-jason/); + no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /unresolved review request; resolve or abandon it before closing it/); + no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /unresolved review request; resolve or abandon it before moving it to waiting-on-jason/); assert.equal(s.posts().length, 1); // A POST that lands after the row reached waiting-on-jason: abandoned, @@ -256,7 +256,7 @@ test("an unresolved request blocks a new request, a new round, waiting-on-jason }; assert.equal(s3.mut({ verb: "move", op: "review-9-00001", args: { id: 9, to: "in-review", candidate: "HEAD" }, by: "darkwing" }, { hook }).code, 3); assert.deepEqual([s3.row(9).state, s3.round(9, 1).attempts[0].state], ["waiting-on-jason", "conflict"]); - no(s3.run(["move", "9", "done", "--op", "done-9-000002"], "jason"), 2, /unresolved review request: review-9-00001 \(round 1, conflict\); resolve or abandon it before closing it/); + no(s3.run(["move", "9", "done", "--op", "done-9-000002"], "jason"), 2, /unresolved review request; resolve or abandon it before closing it/); // Back to in-progress is allowed; the next round is not. const s2 = await ready(t); @@ -264,7 +264,7 @@ test("an unresolved request blocks a new request, a new round, waiting-on-jason no(s2.run(move9(), "darkwing"), 3); s2.rules([]); ok(s2.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing")); - no(s2.run(["move", "9", "in-review", "--candidate", "HEAD", "--op", "review-9-00002"], "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, uncertain\); resolve or abandon it before a new round/); + no(s2.run(["move", "9", "in-review", "--candidate", "HEAD", "--op", "review-9-00002"], "darkwing"), 2, /unresolved review request; resolve or abandon it before a new round/); assert.equal(s2.posts().length, 1); }); @@ -326,7 +326,7 @@ test("late outcomes: after an abandon, and after a resolve with the same or anot let a = s.round(9, 1).attempts[0]; assert.deepEqual([a.state, a.comment, a.transport.comment, a.resolutions.map((x) => x.verb)], ["conflict", null, 1000, ["abandon"]]); assert.equal(s.round(9, 1).duplicateRisk, true); - no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request: review-9-00001 \(round 1, conflict\)/); + no(s.run(request("request-9-0002"), "darkwing"), 2, /unresolved review request/); ok(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "1000", "--op", "resolve-9-00001"], "darkwing"), /conflict→posted comment 1000$/m); a = s.round(9, 1).attempts[0]; assert.deepEqual([a.state, a.comment, a.resolutions.map((x) => x.verb)], ["posted", 1000, ["abandon", "resolve"]]); @@ -407,7 +407,7 @@ test("resolve checks the comment: issue, markers, round, candidate and author", no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing"), 1, /failed or had no answer/); no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "darkwing", { MOSAIC_GITEA_CREDENTIAL_FILE: s.tokens.dewey }), 2, /cannot check comment 7001: .*not darkwing's token file/); const before = s.calls().length; - no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "rocko"), 2, /^queue: row 9 is claimed by darkwing; only the owner \(darkwing\) or a privileged actor may resolve a request on row 9$/m); + no(s.run(["review", "resolve", "9", "review-9-00001", "--comment", "7001", "--op", "resolve-9-00001"], "rocko"), 2, /^queue: row 9 is claimed; only the owner or a privileged actor may resolve a request on row 9$/m); assert.equal(s.calls().length, before, "a refused actor makes no request"); assert.equal(s.doc().revision, 4); s.rules([{ method: "GET", path: "comments/7001$", http: 200, body: good }]); @@ -463,9 +463,9 @@ test("request, changes, a new candidate, approval: every round pinned; no review no(s.run(["review", "record", "9", "--verdict", "maybe", "--comment", "88", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), 2, /approve or changes/); no(s.run(["review", "record", "9", "--verdict", "changes", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), 4, /--comment N is required/); ok(s.run(["review", "record", "9", "--verdict", "changes", "--comment", "88", "--candidate", c1, "--op", "record-9-filb01"], "filbert"), /row 9 round 1 changes by filbert \(comment 88\)$/m); - no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /filbert already recorded a verdict for row 9 round 1/); + no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /reviewer already recorded a verdict for row 9 round 1/); ok(cli(s.repo, ["next", "filbert"]), /^nothing$/m); - no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /round 1 has no approval recorded by filbert/); + no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /round 1 has no approval recorded by every listed reviewer/); ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing")); no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /is in-progress; review record applies to in-review rows/); writeFileSync(join(s.repo.root, "packages/fix.mjs"), "export {};\n"); @@ -474,10 +474,10 @@ test("request, changes, a new candidate, approval: every round pinned; no review const c2 = s.head(); ok(s.run(move9("review-9-00002"), "darkwing"), /round 2 on #1508; request review-9-00002 requesting$/m); ok(cli(s.repo, ["next", "filbert"]), /^review row 9/m); - no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, new RegExp(`candidate ${c1} is not round 2's candidate ${c2}`)); + no(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c1, "--op", "record-9-filb02"], "filbert"), 2, /candidate is not round 2's candidate/); ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "89", "--candidate", c2, "--op", "record-9-filb02"], "filbert")); no(s.run(["move", "9", "done", "--evidence", `comment=89,round=2,candidate=${c2}`, "--op", "done-9-000001"], "filbert"), 2, /closes on its recorded verdicts; drop --evidence/); - no(s.run(["move", "9", "done", "--op", "done-9-000001"], "rocko"), 2, /only the gate owner \(filbert\) or a privileged actor/); + no(s.run(["move", "9", "done", "--op", "done-9-000001"], "rocko"), 2, /only the gate owner or a privileged actor/); ok(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), /in-review→done round 2$/m); const rounds = s.row(9).review.rounds; assert.deepEqual(rounds.map((r) => [r.n, r.candidate.digest, r.attempts.map((a) => [a.op, a.state, a.comment]), r.receipts.map((x) => [x.reviewer, x.verdict, x.comment, x.candidate])]), [ @@ -565,11 +565,11 @@ test("semantics: v1 entries replay as before; review entries need v2", async (t) const outcome = d.log.at(-1); assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: [...d.log.slice(0, -1), { ...outcome, semantics: 1 }] }))), /review-outcome needs semantics 2/); assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: [...d.log.slice(0, -1), { ...outcome, op: "review-9-00002.outcome" }] }))), /op must be its attempt's op plus \.outcome/); - assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: d.log.map((e, i) => (i === 1 ? { ...e, semantics: 3 } : e)) }))), /semantics 3 is not 1 to 2/); + assert.throws(() => queue.loadDoc(Buffer.from(queue.serialize({ ...d, log: d.log.map((e, i) => (i === 1 ? { ...e, semantics: 3 } : e)) }))), /semantics is not 1 to 2/); // An attempt takes one outcome, even one that agrees with it. const full = queue.replay(d.log); const again = { ...outcome, rev: full.revision + 1 }; - assert.throws(() => queue.applyEntry(full, again, {}), /request review-9-00001 already has its outcome/); + assert.throws(() => queue.applyEntry(full, again, {}), /request already has its outcome/); // review-outcome is the one entry a done row takes. const at = "2026-09-27T00:00:00.000Z"; // The request was abandoned and the row closed before the POST's answer came. @@ -585,7 +585,7 @@ test("semantics: v1 entries replay as before; review entries need v2", async (t) assert.deepEqual([late.state.rows.get(9).state, a.state, a.transport.comment], ["done", "conflict", 1000]); const lateOk = (verb, op, args, by) => queue.applyEntry(st, entry(st, verb, op, args, by), {}); assert.throws(() => lateOk("review-resolve", "resolve-9-00001", { id: 9, attempt: "review-9-00001", comment: 5 }), /done rows never change/); - assert.throws(() => queue.applyEntry(mid, entry(mid, "review-outcome", "review-9-00001.outcome", { id: 9, attempt: "review-9-00001", outcome: "failed", status: 403, comment: null, detail: "x" }, "sage"), {}), /only darkwing, who made request review-9-00001, records its outcome/); + assert.throws(() => queue.applyEntry(mid, entry(mid, "review-outcome", "review-9-00001.outcome", { id: 9, attempt: "review-9-00001", outcome: "failed", status: 403, comment: null, detail: "x" }, "sage"), {}), /only the actor who made the request records its outcome/); assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "failed", status: 500, comment: null, detail: "x" }), /a failed outcome's status is one of 400, 401, 403, 404, 422/); assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 201, comment: null, detail: "x" }), /posted exactly when HTTP 201 returned a comment id/); assert.throws(() => queue.canonArgs("review-outcome", { id: 9, attempt: "review-9-00001", outcome: "posted", status: 200, comment: 4, detail: "x" }), /posted exactly when HTTP 201/); @@ -593,7 +593,7 @@ test("semantics: v1 entries replay as before; review entries need v2", async (t) test("set reviewers refuses the row's owner (2026-09-28)", async (t) => { const s = await ready(t); - no(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"), 2, /row 9 owner darkwing can't be a listed reviewer/); + no(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"), 2, /row 9 owner can't be a listed reviewer/); assert.deepEqual(s.row(9).reviewers, ["filbert"]); }); @@ -665,17 +665,17 @@ test("a Jason-gated row reaches waiting-on-jason only on every reviewer's approv ok(s.run(move9(), "darkwing"), /posted comment 1000$/m); assert.deepEqual(s.round(9, 1).receipts, []); no(s.run(["move", "9", "done", "--op", "done-9-000001"], "darkwing"), 2, /gate is Jason's; it goes through waiting-on-jason/); - no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 1 has no approval recorded by filbert, rocko$/m); - no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "sage"), 2, /no approval recorded by filbert, rocko$/m); + no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 1 has no approval recorded by every listed reviewer$/m); + no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "sage"), 2, /no approval recorded by every listed reviewer$/m); ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "88", "--candidate", head, "--op", "record-9-filb01"], "filbert")); ok(s.run(["review", "record", "9", "--verdict", "changes", "--comment", "89", "--candidate", head, "--op", "record-9-rock01"], "rocko")); - no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /no approval recorded by rocko$/m); + no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /no approval recorded by every listed reviewer$/m); // Changes go back to in-progress, as before. ok(s.run(["move", "9", "in-progress", "--op", "changes-9-0001"], "darkwing")); ok(s.run(move9("review-9-00002"), "darkwing"), /posted comment 1001$/m); ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "91", "--candidate", head, "--op", "record-9-rock02"], "rocko")); // Filbert's round 1 approval doesn't carry into round 2. - no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by filbert$/m); + no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by every listed reviewer$/m); ok(s.run(["review", "record", "9", "--verdict", "approve", "--comment", "90", "--candidate", head, "--op", "record-9-filb02"], "filbert")); ok(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), /in-review→waiting-on-jason$/m); ok(s.run(["move", "9", "done", "--op", "done-9-000001"], "jason"), /waiting-on-jason→done$/m); diff --git a/packages/queue/tests/store.test.mjs b/packages/queue/tests/store.test.mjs index 336bce960..c66283970 100644 --- a/packages/queue/tests/store.test.mjs +++ b/packages/queue/tests/store.test.mjs @@ -65,7 +65,7 @@ test("genesis: the map must be committed, well formed, with committed briefs and no(cli(noBrief, genesisArgs(noBrief), { by: "sage" }), 2, /only a done row may lack one/); // The owner-not-reviewer rule of add, set and assign (2026-10-04). const selfReview = scratchRepo(t, { map: mapText(MAP_ROWS.map((r) => (r.id === 9 ? { ...r, reviewers: ["darkwing", "filbert"] } : r))) }); - no(cli(selfReview, genesisArgs(selfReview), { by: "sage" }), 2, /migration map row 9 owner darkwing can't be a listed reviewer/); + no(cli(selfReview, genesisArgs(selfReview), { by: "sage" }), 2, /migration map row owner can't be a listed reviewer/); for (const repo of [ghost, noBrief, selfReview]) { assert.throws(() => readFileSync(repo.queuePath)); assert.throws(() => readFileSync(join(repo.gitDir, "mosaic-queue.head"))); @@ -245,11 +245,11 @@ test("claims and add defaults through the CLI; candidates are manifests or reach test("add, set reviewers and assign refuse the row's owner as a reviewer", (t) => { const repo = ready(t); const before = doc(repo); - no(cli(repo, ["add", "--op", "add-owner-reviewer", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--gate-owner", "filbert", "--reviewer", "rocko"], { by: "sage" }), 2, /row 12 owner rocko can't be a listed reviewer/); + no(cli(repo, ["add", "--op", "add-owner-reviewer", "--piece", "x", "--gate", "g", "--brief", "docs/plans/brief-b.md#Template", "--owner", "rocko", "--gate-owner", "filbert", "--reviewer", "rocko"], { by: "sage" }), 2, /row 12 owner can't be a listed reviewer/); assert.deepEqual(doc(repo), before); - no(cli(repo, ["set", "9", "reviewers", "filbert,darkwing", "--op", "set-9-owner-reviewer"], { by: "sage" }), 2, /row 9 owner darkwing can't be a listed reviewer/); + no(cli(repo, ["set", "9", "reviewers", "filbert,darkwing", "--op", "set-9-owner-reviewer"], { by: "sage" }), 2, /row 9 owner can't be a listed reviewer/); assert.deepEqual(doc(repo), before); - no(cli(repo, ["assign", "9", "filbert", "--op", "assign-9-filbert"], { by: "sage" }), 2, /can't assign row 9 to filbert: filbert is one of its reviewers/); + no(cli(repo, ["assign", "9", "filbert", "--op", "assign-9-filbert"], { by: "sage" }), 2, /can't assign row 9: seat is one of its reviewers/); assert.deepEqual(doc(repo), before); // The unchanged accept paths: a reviewer list or a new owner that does not collide with the other. ok(cli(repo, ["set", "9", "reviewers", "filbert,dewey", "--op", "set-9-ok"], { by: "sage" })); diff --git a/packages/runs/src/state.mjs b/packages/runs/src/state.mjs index e226e0cb8..542634970 100644 --- a/packages/runs/src/state.mjs +++ b/packages/runs/src/state.mjs @@ -67,7 +67,7 @@ function logEntry(line) { // With last, only the newest last well-formed entries are returned. export function readActivationLog(dataRoot, { last } = {}) { if (last !== undefined && (!Number.isInteger(last) || last < 1)) { - throw new RunsError(`last must be a positive integer (got ${JSON.stringify(last)})`); + throw new RunsError(`last must be a positive integer`); } const state = readStateFile(dataRoot, ACTIVATION_LOG_FILE); if (state === null) return { entries: [], malformed: 0 }; diff --git a/packages/runs/tests/last-values.test.mjs b/packages/runs/tests/last-values.test.mjs new file mode 100644 index 000000000..be8de407f --- /dev/null +++ b/packages/runs/tests/last-values.test.mjs @@ -0,0 +1,19 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +const secret = 'sk-test-NOT-A-SECRET-1234'; +function scratch(t) { const root = mkdtempSync(join(tmpdir(), 'r4-')); t.after(() => rmSync(root, { recursive: true, force: true })); return root; } +function safe(label, Type, props = {}, omitted = secret) { return error => { + assert.ok(error instanceof Type, error.stack); + for (const [key, value] of Object.entries(props)) assert.equal(error[key], value); + assert.ok(error.message.includes(label), error.message); + assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET|Unexpected token/); + assert.ok(!error.message.includes(omitted), error.message); + assert.equal(error.cause, undefined); + return true; +}; } +import { readActivationLog } from '../src/state.mjs'; +import { RunsError } from '../src/errors.mjs'; +test('activation last refusal names the field without its value', () => assert.throws(() => readActivationLog('/unused', { last: secret }), safe('last', RunsError, { exitCode: 4 }))); diff --git a/scripts/mosaic-config.mjs b/scripts/mosaic-config.mjs index aae41b94b..369bf5341 100755 --- a/scripts/mosaic-config.mjs +++ b/scripts/mosaic-config.mjs @@ -195,11 +195,11 @@ function validateDataRoot(value, file) { fail(2, 'execution "dataRoot" contains a NUL byte'); } if (!path.isAbsolute(value)) { - fail(2, `execution "dataRoot" must be an absolute path (got "${value}")`); + fail(2, `execution "dataRoot" must be an absolute path`); } const resolved = path.resolve(value); if (resolved !== value) { - fail(2, `execution "dataRoot" must be canonical without ".", "..", trailing slashes, or redundant separators (got "${value}")`); + fail(2, `execution "dataRoot" must be canonical without ".", "..", trailing slashes, or redundant separators`); } if (resolved === path.parse(resolved).root) { fail(2, 'execution "dataRoot" must not be the filesystem root'); @@ -252,7 +252,7 @@ function validateFederationOrigin(value, where) { url = null; } if (url === null || url.protocol !== "https:" || url.origin !== value) { - fail(2, `${where} must be an https origin with no path, query, or userinfo (got ${JSON.stringify(value)})`); + fail(2, `${where} must be an https origin with no path, query, or userinfo`); } } @@ -262,7 +262,7 @@ function validateFederation(value) { } rejectUnknownKeys(value, ["self", "peers"], "federation"); if (typeof value.self !== "string" || !FEDERATION_ID_PATTERN.test(value.self)) { - fail(2, `federation.self must match ${FEDERATION_ID_PATTERN} (got ${JSON.stringify(value.self)})`); + fail(2, `federation.self must match ${FEDERATION_ID_PATTERN}`); } if (!Array.isArray(value.peers)) { fail(2, 'federation.peers must be an array'); @@ -275,10 +275,10 @@ function validateFederation(value) { } rejectUnknownKeys(peer, ["id", "origin", "label"], where); if (typeof peer.id !== "string" || !FEDERATION_ID_PATTERN.test(peer.id)) { - fail(2, `${where}.id must match ${FEDERATION_ID_PATTERN} (got ${JSON.stringify(peer.id)})`); + fail(2, `${where}.id must match ${FEDERATION_ID_PATTERN}`); } if (ids.has(peer.id)) { - fail(2, `${where}.id ${JSON.stringify(peer.id)} is already named (by federation.self or an earlier peer)`); + fail(2, `${where}.id is already named (by federation.self or an earlier peer)`); } ids.add(peer.id); validateFederationOrigin(peer.origin, `${where}.origin`); @@ -306,10 +306,10 @@ async function validate(document, file) { ); if (document.configVersion !== SUPPORTED_CONFIG_VERSION) { - fail(2, `unsupported configVersion: ${JSON.stringify(document.configVersion)} (supported: ${SUPPORTED_CONFIG_VERSION})`); + fail(2, `unsupported configVersion: (supported: ${SUPPORTED_CONFIG_VERSION})`); } if (!SUPPORTED_ENVIRONMENTS.has(document.environment)) { - fail(2, `unsupported environment: ${JSON.stringify(document.environment)} (supported: ${[...SUPPORTED_ENVIRONMENTS].join(", ")})`); + fail(2, `unsupported environment: (supported: ${[...SUPPORTED_ENVIRONMENTS].join(", ")})`); } const dataRoot = validateDataRoot(document.dataRoot, file); @@ -319,12 +319,12 @@ async function validate(document, file) { } rejectUnknownKeys(document.execution, ["backend", "provider", "model", "adapter"], '"execution"'); if (!SUPPORTED_BACKENDS.has(document.execution.backend)) { - fail(2, `unsupported execution.backend: ${JSON.stringify(document.execution.backend)} (supported: ${[...SUPPORTED_BACKENDS].join(", ")})`); + fail(2, `unsupported execution.backend: (supported: ${[...SUPPORTED_BACKENDS].join(", ")})`); } for (const key of ["provider", "model"]) { const value = document.execution[key]; if (typeof value !== "string" || !NAME_PATTERN.test(value)) { - fail(2, `execution.${key} must match ${NAME_PATTERN} (got ${JSON.stringify(value)})`); + fail(2, `execution.${key} must match ${NAME_PATTERN}`); } } @@ -332,7 +332,7 @@ async function validate(document, file) { ? "pi" : document.execution.adapter; if (typeof adapter !== "string" || !SUPPORTED_ADAPTERS.has(adapter)) { - fail(2, `unsupported execution.adapter: ${JSON.stringify(adapter)} (supported: ${[...SUPPORTED_ADAPTERS].join(", ")})`); + fail(2, `unsupported execution.adapter: (supported: ${[...SUPPORTED_ADAPTERS].join(", ")})`); } // Last, so the older checks keep their order. The key is in the output only when the file has diff --git a/scripts/mosaic-task.mjs b/scripts/mosaic-task.mjs index 4d251efe2..d931d8f8c 100755 --- a/scripts/mosaic-task.mjs +++ b/scripts/mosaic-task.mjs @@ -84,7 +84,7 @@ function readJsonFile(file, what) { function validateId(value, what) { if (typeof value !== "string" || !ID_PATTERN.test(value)) { - fail(2, `${what} must match ${ID_PATTERN} (got ${JSON.stringify(value)})`); + fail(2, `${what} must match ${ID_PATTERN}`); } return value; } @@ -93,7 +93,7 @@ function validateMission(document, file) { if (!isPlainObject(document)) fail(2, "mission must be a JSON object"); rejectUnknownKeys(document, ["missionVersion", "id", "objective", "directives", "capabilities"], "mission"); if (document.missionVersion !== 1) { - fail(2, `unsupported missionVersion: ${JSON.stringify(document.missionVersion)} (supported: 1)`); + fail(2, `unsupported missionVersion: (supported: 1)`); } validateId(document.id, "mission id"); if (typeof document.objective !== "string" || document.objective.trim().length === 0 || document.objective.length > 4000) { @@ -123,9 +123,9 @@ function validateMission(document, file) { const seen = new Set(); for (const tool of document.capabilities.tools) { if (!SUPPORTED_TOOLS.includes(tool)) { - fail(2, `unsupported mission tool: ${JSON.stringify(tool)} (supported: ${SUPPORTED_TOOLS.join(", ")})`); + fail(2, `unsupported mission tool: (supported: ${SUPPORTED_TOOLS.join(", ")})`); } - if (seen.has(tool)) fail(2, `duplicate tool in mission capabilities.tools: ${tool}`); + if (seen.has(tool)) fail(2, `duplicate tool in mission capabilities.tools`); seen.add(tool); } capabilities = { tools: [...seen] }; @@ -138,7 +138,7 @@ function validateTask(document, file) { if (!isPlainObject(document)) fail(2, "task must be a JSON object"); rejectUnknownKeys(document, ["taskVersion", "id", "prompt", "mission", "expectExact", "timeoutSeconds", "workspace", "capabilities", "session", "sessionForkFrom"], "task"); if (document.taskVersion !== 1) { - fail(2, `unsupported taskVersion: ${JSON.stringify(document.taskVersion)} (supported: 1)`); + fail(2, `unsupported taskVersion: (supported: 1)`); } validateId(document.id, "task id"); @@ -228,9 +228,9 @@ function validateTask(document, file) { const seen = new Set(); for (const tool of document.capabilities.tools) { if (!SUPPORTED_TOOLS.includes(tool)) { - fail(2, `unsupported tool: ${JSON.stringify(tool)} (supported: ${SUPPORTED_TOOLS.join(", ")})`); + fail(2, `unsupported tool: (supported: ${SUPPORTED_TOOLS.join(", ")})`); } - if (seen.has(tool)) fail(2, `duplicate tool in capabilities.tools: ${tool}`); + if (seen.has(tool)) fail(2, `duplicate tool in capabilities.tools`); seen.add(tool); } tools = [...seen]; diff --git a/scripts/test-config.sh b/scripts/test-config.sh index 5daed4294..aae644e9d 100755 --- a/scripts/test-config.sh +++ b/scripts/test-config.sh @@ -238,7 +238,7 @@ expect_fed_refusal "federation peer id pattern exits 2 naming the key" "federati FED_LONG_LABEL="$(printf 'x%.0s' $(seq 81))" expect_fed_refusal "federation peer label over 80 characters exits 2 naming the key" "federation.peers[0].label" \ '{"self":"hub","peers":[{"id":"factory","origin":"https://mosaic.woltje.com","label":"'"$FED_LONG_LABEL"'"}]}' -expect_fed_refusal "federation duplicate peer id exits 2 naming the key" 'federation.peers[1].id "factory" is already named' \ +expect_fed_refusal "federation duplicate peer id exits 2 naming the key" 'federation.peers[1].id is already named' \ '{"self":"hub","peers":[{"id":"factory","origin":"https://mosaic.woltje.com","label":"F"},{"id":"factory","origin":"https://other.woltje.com","label":"G"}]}' expect_fed_refusal "federation peer origin with a trailing slash exits 2 naming the key" "federation.peers[0].origin" \ '{"self":"hub","peers":[{"id":"factory","origin":"https://mosaic.woltje.com/","label":"F"}]}' diff --git a/scripts/test-task.sh b/scripts/test-task.sh index b61893363..bbc9942fb 100755 --- a/scripts/test-task.sh +++ b/scripts/test-task.sh @@ -317,7 +317,7 @@ EOF && printf '%s\n' "$V2_OUT" | grep -q '^MOSAIC_ROLE_NETWORK=api-only$' || V2_OK=1 done check "shipped version 2 roles resolve with their contracts (pm, cto, coder, reviewer)" "$V2_OK" - printf '# v2\n' > "$SANDBOX/roles/v2.md" + printf '# v2\n\n## Authority\n' > "$SANDBOX/roles/v2.md" v2role() { # v2role NAME AUTHORITY_JSON CREDENTIALS_JSON [CONTRACT] printf '{"roleVersion":2,"name":"%s","title":"T","contract":"%s","tools":["read"],"network":"none","authority":%s,"credentials":%s}' \ "$1" "${4:-v2.md}" "$2" "$3" > "$SANDBOX/roles/$1.json"