fix(refusals): name validated identifiers the system already shows (#1873) #1920

Merged
jason.woltje merged 2 commits from org-names into refactor 2026-11-11 05:40:01 +00:00
15 changed files with 250 additions and 81 deletions

No files matched your search

+15 -4
View File
@@ -58,18 +58,20 @@ test('project id mismatch omits the file value in config-command', t => {
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 => {
]) test(`config ${field} repeated/canonical value ${field.endsWith(".id") ? "names the checked id" : "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/);
assert.equal(r.status, 2); assert.ok(r.stderr.includes(field), r.stderr);
if (field.endsWith(".id")) assert.ok(r.stderr.includes(secret.toLowerCase()), r.stderr);
else 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 => {
for (const mission of [false, true]) test(`duplicate ${mission ? 'mission' : 'task'} tool names 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/);
assert.equal(r.status, 2); assert.match(r.stderr, /duplicate tool in/); assert.match(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');
@@ -89,3 +91,12 @@ test('CLI boot project id mismatch omits the configured value and keeps exit 3',
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()));
});
for (const mission of [false, true]) test(`duplicate ${mission ? 'mission' : 'task'} tool rejects an unknown value before naming it`, 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: [secret, secret] } })); }
else d.capabilities = { tools: [secret, secret] };
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, /unsupported.*tool/); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/);
});
+5 -5
View File
@@ -72,8 +72,8 @@ 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 a bus principal`);
if (humans.has(user.id)) refuse(`${at}.id must not be a business's human`);
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 (user.home !== LOCAL_HOME && !ctx.peers.has(user.home)) {
refuse(`${at}.home must be a configured peer or ${JSON.stringify(LOCAL_HOME)}`);
}
@@ -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}`);
if (seen.has(entry)) refuse(`duplicate entry in ${where}: ${JSON.stringify(entry)}`);
seen.add(entry);
});
}
@@ -122,7 +122,7 @@ 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`);
if (seenGrants.has(grant.id)) refuse(`${at}.id is a duplicate grant id: ${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`);
if (typeof grant.business !== "string" || !ctx.businesses.has(grant.business)) {
@@ -153,7 +153,7 @@ export function checkAccess(document, context) {
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`);
if (users.has(user.id)) refuse(`users[${i}].id is a duplicate user id: ${user.id}`);
users.add(user.id);
});
const seenGrants = new Set();
+8 -8
View File
@@ -40,11 +40,11 @@ function checkAuthority(authority) {
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}; expected an action from the role vocabulary`);
if (GATED_ONLY.includes(action)) refuse(`authority.${key} lists an action which is always gated`);
if (GATED_ONLY.includes(action)) refuse(`authority.${key} lists ${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`);
if (both.length > 0) refuse(`action listed as both withinRole and crossRole: ${both.join(", ")}`);
return lists;
}
@@ -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 a creative definition only the human adds`);
if (CREATIVE_DEFINITIONS.includes(name)) refuse(`hireable names ${name}, a creative definition only the human adds`);
const path = join(dir, `${name}.json`);
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 a definition which is role version 1`);
const outside = unhireableAction(named);
if (outside !== null) refuse(`hireable names a definition which holds an action outside the hireable ceiling`);
if (outside !== null) refuse(`hireable names ${name}, which holds ${outside} outside the hireable ceiling`);
});
}
@@ -106,7 +106,7 @@ function checkGiteaScopes(scopes) {
}, { 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 a category twice; a token holds one level per category`);
if (twice.length > 0) refuse(`gitea scopes name ${twice[0]} twice; a token holds one level per category`);
return list;
}
@@ -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 a service twice`);
if (seen.has(entry.service)) refuse(`role credentials name ${entry.service} twice`);
seen.add(entry.service);
if (host) return { service: HOST_SERVICE };
const scopes = entry.service === "gitea" ? checkGiteaScopes(entry.scopes) : checkVikunjaScopes(entry.scopes);
@@ -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 outside the allowed set (allowed: ${CREATIVE_TOOLS.join(", ")})`);
if (wider.length > 0) refuse(`creative role ${role.name} may not name tools ${wider.join(", ")} (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`);
if (services.length > 0) refuse(`creative role ${role.name} may hold only the ${CREATIVE_SERVICE} credential (got ${services.join(", ")})`);
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")) {
+2 -2
View File
@@ -108,8 +108,8 @@ test("users: unknown keys and the id rule", () => {
});
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 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 = "human"; d.grants = []; }), context()), /users\[0\]\.id must not be "human", a bus principal/);
refuses(() => checkAccess(mutated((d) => { d.users[1].id = "tester"; d.grants = []; }), context()), /users\[1\]\.id must not be "tester", 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/);
@@ -53,22 +53,29 @@ for (const [field, edit] of [
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 => {
// is refused earlier. A known service is already displayed in the role's rails.
test('creative service refusal names 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'));
assert.throws(() => validateRoleDocument(d, join(root, 'companion.json')), named('credential', '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`, () => {
for (const at of ['user', 'grant', 'scope', 'human']) test(`access ${at} duplicate/reserved refusal names 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));
assert.throws(() => checkAccess(d, ctx), named(at === 'scope' ? 'instances' : '.id', value));
});
function named(label, value) { return error => {
assert.ok(error instanceof BusinessError); assert.equal(error.exitCode, 2);
assert.ok(error.message.includes(label), error.message);
assert.ok(error.message.includes(value), error.message);
return true;
}; }
+7 -7
View File
@@ -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 a category twice/);
refuses(() => gitea(["read:issue", "write:issue"]), /name issue 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] }), /^creative role ideas may not name tools outside the allowed set /);
refuses(creative("ideas", { tools: ["read", tool] }), /^creative role ideas may not name tools (?:write|edit|bash) \(allowed: /);
}
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$/);
/^creative role ideas may hold only the gitea credential \(got vikunja\)$/);
refuses(creative("ideas", { authority: { withinRole: ["message.send", "library.write"], crossRole: [] }, credentials: [{ service: "host" }] }),
/^creative role ideas may hold only the gitea credential$/);
/^creative role ideas may hold only the gitea credential \(got host\)$/);
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"), /hireable names a definition which holds an action outside the hireable ceiling$/);
refuses(() => loadRole(dir, "chief"), /hireable names worker, which holds [a-z.]+ 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 a definition which holds an action outside the hireable ceiling/);
refuses(() => loadRole(mixed, "chief"), /hireable names worker, which holds [a-z.]+ outside the hireable ceiling/);
const clean = hireDir({
chief: { hireable: ["worker", "helper"] },
worker: { withinRole: [...HIRE_ALLOWED] },
@@ -331,7 +331,7 @@ test("hireable: a missing, version 1, duplicate or badly named entry refuses, an
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"), /hireable names a creative definition only the human adds$/);
refuses(() => loadRole(hireDir({ chief: { hireable: [name] } }), "chief"), /hireable names [a-z-]+, 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/);
@@ -0,0 +1,89 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { validateRoleDocument } from '../src/role.mjs';
import { checkAccess } from '../src/access.mjs';
import { BusinessError } from '../src/errors.mjs';
const bad = 'sk-test-NOT-A-SECRET-1234';
const badId = `${bad.toLowerCase()} "`;
function refusal(fn, names = [], omitted = bad, validation = null) {
assert.throws(fn, error => {
assert.ok(error instanceof BusinessError);
assert.equal(error.exitCode, 2);
for (const name of names) assert.ok(error.message.includes(name), error.message);
assert.doesNotMatch(error.message + error.stack, /sk-test|NOT-A-SECRET/i);
assert.ok(!error.message.includes(omitted), error.message);
if (validation !== null) assert.match(error.message, validation);
return true;
});
}
function role(t, edit = () => {}) {
const root = mkdtempSync(join(tmpdir(), 'names-'));
t.after(() => rmSync(root, { recursive: true, force: true }));
writeFileSync(join(root, 'r.md'), '# R\n\n## Authority\n');
writeFileSync(join(root, 'companion.md'), '# R\n\n## Authority\n');
const doc = { roleVersion: 2, name: 'r', title: 'R', contract: 'r.md', tools: ['read'], network: 'none',
authority: { withinRole: [], crossRole: [] }, credentials: [] };
edit(doc, root);
return () => validateRoleDocument(doc, join(root, `${doc.name}.json`));
}
for (const key of ['withinRole', 'crossRole']) test(`gated ${key} names the known action; unknown action stays unnamed`, t => {
refusal(role(t, d => { d.authority[key] = ['deploy']; }), ['authority.' + key, 'deploy']);
refusal(role(t, d => { d.authority[key] = [bad]; }));
});
test('overlapping authority lists name every known action; invalid entries stop first', t => {
refusal(role(t, d => { d.authority = { withinRole: ['message.send', 'record.write'], crossRole: ['message.send', 'record.write'] }; }), ['message.send', 'record.write']);
refusal(role(t, d => { d.authority = { withinRole: [bad], crossRole: [bad] }; }));
});
for (const kind of ['creative', 'outside']) test(`hireable ${kind} names a known definition; invalid name stops first`, t => {
const edit = (d, root) => {
const name = kind === 'creative' ? 'companion' : 'helper';
d.hireable = [name];
if (kind === 'outside') writeFileSync(join(root, 'helper.json'), JSON.stringify({ ...d, name, hireable: [], authority: { withinRole: ['role.hire'], crossRole: [] } }));
};
refusal(role(t, edit), kind === 'creative' ? ['companion'] : ['helper', 'role.hire']);
refusal(role(t, d => { d.hireable = [badId]; }), [], badId, /role hireable entry must match/);
});
test('hireable wider action rejects unknown action before naming it', t => {
refusal(role(t, (d, root) => {
d.hireable = ['helper'];
writeFileSync(join(root, 'helper.json'), JSON.stringify({ ...d, name: 'helper', hireable: [], authority: { withinRole: [bad], crossRole: [] } }));
}));
});
test('gitea category conflict names the checked category; invalid scope stops first', t => {
refusal(role(t, d => { d.credentials = [{ service: 'gitea', scopes: ['read:issue', 'write:issue'] }]; }), ['issue']);
refusal(role(t, d => { d.credentials = [{ service: 'gitea', scopes: [bad] }]; }));
});
test('duplicate credential names the checked service; invalid service stops first', t => {
refusal(role(t, d => { d.credentials = [{ service: 'gitea', scopes: ['read:issue'] }, { service: 'gitea', scopes: ['read:issue'] }]; }), ['gitea']);
refusal(role(t, d => { d.credentials = [{ service: bad }, { service: bad }]; }));
});
for (const field of ['tools', 'credentials']) test(`creative ${field} names the checked wider grant; invalid grant stops first`, t => {
const edit = d => { d.name = 'companion'; d.contract = 'companion.md'; d.messaging = { sendTo: ['human'], receiveFrom: ['human'] }; };
refusal(role(t, d => { edit(d); if (field === 'tools') d.tools.push('bash'); else d.credentials = [{ service: 'vikunja', scopes: { tasks: ['read_one'] } }]; }), [field === 'tools' ? 'bash' : 'vikunja']);
refusal(role(t, d => { edit(d); if (field === 'tools') d.tools = [bad]; else d.credentials = [{ service: bad }]; }));
});
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: 'one' }] }],
grants: [{ id: 'grant', user: 'bean', business: 'acme', projects: ['*'], instances: ['coder'], level: 'viewer' }] });
for (const kind of ['reserved', 'human', 'user', 'grant', 'project', 'instance']) test(`access ${kind} conflict names checked id; malformed replacement stops first`, () => {
function run(value) {
const d = access(), ctx = context();
if (kind === 'reserved' || kind === 'human') d.users[0].id = value;
if (kind === 'user') d.users.push({ ...structuredClone(d.users[0]), id: value, subjects: [{ provider: 'test', sub: 'two' }] });
if (kind === 'grant') { d.grants[0].id = value; d.grants.push(structuredClone(d.grants[0])); }
if (kind === 'project') d.grants[0].projects = [value, value];
if (kind === 'instance') d.grants[0].instances = [value, value];
return () => checkAccess(d, ctx);
}
const value = { reserved: 'human', human: 'jason', user: 'bean', grant: 'g-known-1', project: '11111111-2222-4333-8444-555555555555', instance: 'coder' }[kind];
refusal(run(value), [value]);
const validation = kind === 'project' ? /lowercase UUID/ : kind === 'instance' ? /declared instance/ : /\.id must match/;
refusal(run(badId), [], badId, validation);
});
+21 -21
View File
@@ -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 is not the owner`);
if (row.claim.seat !== row.owner) throw refuse(`${w} claim seat is not the owner ${row.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}`);
@@ -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; ` : "";
throw refuse(`${claimed}only the owner or a privileged actor may ${doing} row ${row.id}`);
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}`);
}
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; resolve or abandon it before ${doing}`);
if (open.length) throw refuse(`row ${row.id} has an unresolved review request: ${open.join(", ")}; 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 every listed reviewer`);
if (missing.length) throw refuse(`row ${row.id} round ${cur.n} has no approval recorded by ${missing.join(", ")}`);
}
// The request round an attempt belongs to, and the attempt.
@@ -645,7 +645,7 @@ function applyReview(rows, row, entry) {
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`);
if (live.length) throw refuse(`row ${row.id} round ${cur.n} already has a posted request (${live[0].op}, comment ${live[0].comment})`);
const rounds = row.review.rounds.map((r) => (r.n === cur.n ? { ...r, attempts: [...r.attempts, newAttempt(entry)] } : r));
return {
row: { ...row, review: { rounds } },
@@ -657,8 +657,8 @@ function applyReview(rows, row, entry) {
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 the actor who made the request records its outcome`);
if (att.transport !== null) throw refuse(`request already has its outcome`);
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`);
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 is ${att.state}; only a requesting, uncertain or conflict request can be ${resolve ? "resolved" : "abandoned"}`);
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"}`);
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}`);
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.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 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}`);
if (cur.receipts.some((r) => r.reviewer === by)) throw refuse(`${by} 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 {
@@ -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 may start row ${row.id}`);
if (by !== row.owner) throw refuse(`only the owner (${row.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 may request review of row ${row.id}`);
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 (candidate === null) throw refuse("in-progress→in-review needs --candidate <commit|manifest>");
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 or a privileged actor may close row ${row.id}`);
if (by !== row.gateOwner && !isPriv(by)) throw refuse(`only the gate owner (${row.gateOwner}) or a privileged actor may close row ${row.id}`);
refuseUnresolved(row, "closing it");
const cur = row.review?.rounds.at(-1);
if (cur?.request === "comment") {
@@ -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 can't be a listed reviewer`);
if (field === "reviewers" && value.includes(row.owner)) throw refuse(`row ${row.id} owner ${row.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.
@@ -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 can't be a listed reviewer`);
if (reviewers.includes(owner)) throw refuse(`row ${id} owner ${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 or a privileged actor may release row ${row.id}`);
if (!(row.claim && by === row.claim.seat) && !isPriv(by)) throw refuse(`only the claimant (${row.claim?.seat ?? "nobody"}) 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 that seat`);
if (row.reviewers.includes(a.seat)) throw refuse(`can't assign row ${row.id}: seat is one of its reviewers`);
if (row.owner === a.seat) throw refuse(`row ${row.id} is already owned by ${row.owner}`);
if (row.reviewers.includes(a.seat)) throw refuse(`can't assign row ${row.id}: ${a.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, 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 (${row.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";
@@ -1135,7 +1135,7 @@ export function loadDoc(bytes) {
doc.log.forEach(checkEntryShape);
const ops = new Set();
for (const e of doc.log) {
if (ops.has(e.op)) throw refuse(`op id appears twice in the log`);
if (ops.has(e.op)) throw refuse(`op id ${e.op} appears twice in the log`);
ops.add(e.op);
}
if (doc.revision !== doc.log.length - 1) throw refuse(`revision is not the last rev ${doc.log.length - 1}`);
+6 -6
View File
@@ -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 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/);
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/);
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 may start/);
refused(() => step(d, "move", mv(12, "in-progress"), "sage"), /only the owner \(dewey\) 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/);
@@ -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 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/);
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/);
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");
@@ -586,7 +586,7 @@ 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: seat is one of its reviewers/);
refused(() => step(d, "assign", { id: 9, seat: "filbert" }, "sage"), /can't assign row 9: filbert 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$/);
+63 -1
View File
@@ -72,6 +72,7 @@ function operation(verb, args, edit = () => {}, options = {}) {
return applyEntry(state, entry, options.resolved ?? {});
}
const ownerValue = row => { row.owner = seatValue; if (row.claim) row.claim.seat = seatValue; };
const restored = new Set(['claimed owner authority', 'start owner', 'review claimant', 'close gate owner', 'set owner reviewer', 'release claimant', 'assign existing owner', 'assign reviewer', 'note owner', 'add owner reviewer', 'unresolved request', 'missing approval', 'posted request', 'outcome actor', 'duplicate outcome', 'settled resolution', 'reviewer list', 'duplicate verdict']);
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],
@@ -112,7 +113,7 @@ for (const [name, field, fn, omitted = seatValue] of [
// 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(`queue replay refusal ${name} ${restored.has(name) ? 'names validated identifiers' : 'omits stored values'}`, () => assert.throws(fn, restored.has(name) ? named(field, omitted, name === 'posted request' ? 'comment 1' : null) : 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', () => {
@@ -127,3 +128,64 @@ 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));
});
function named(label, value, additional = null) { return error => {
assert.ok(error instanceof QueueError); assert.equal(error.code, 2);
assert.ok(error.message.includes(label), error.message);
assert.ok(error.message.includes(value), error.message);
if (additional !== null) assert.ok(error.message.includes(additional), error.message);
return true;
}; }
// Each restored replay refusal is behind row validation or canonical arguments
// on the store's write path. Replace the relevant identifier at that boundary.
for (const [site, edit] of [
['claimed owner authority', r => { r.owner = secret; }],
['start owner', r => { r.owner = secret; }],
['review claimant', r => { r.claim.seat = secret; }],
['close gate owner', r => { r.gateOwner = secret; }],
['set owner reviewer', r => { r.owner = secret; }],
['release claimant', r => { r.claim.seat = secret; }],
['assign existing owner', r => { r.owner = secret; }],
['assign reviewer', r => { r.reviewers = [secret]; }],
['note owner', r => { r.owner = secret; }],
['unresolved request', r => { reviewRow(r); r.review.rounds[0].attempts[0].op = secret + '/'; }],
['missing approval', r => { reviewRow(r, 'posted'); r.reviewers = [secret]; }],
['posted request', r => { reviewRow(r, 'posted'); r.review.rounds[0].attempts[0].op = secret + '/'; }],
['posted request comment', r => { reviewRow(r, 'posted'); r.review.rounds[0].attempts[0].comment = secret; }],
['outcome actor', r => { reviewRow(r); r.review.rounds[0].attempts[0].by = secret; }],
['duplicate outcome', r => { reviewRow(r, 'posted'); r.review.rounds[0].attempts[0].op = secret + '/'; }],
['settled resolution', r => { reviewRow(r, 'posted'); r.review.rounds[0].attempts[0].op = secret + '/'; }],
['reviewer list', r => { r.reviewers = [secret]; }],
['duplicate verdict', r => { reviewRow(r, 'posted'); r.review.rounds[0].receipts = [{ reviewer: secret, op: 'receipt-op-0001', at, verdict: 'approve', comment: 1, candidate: digestValue }]; }],
]) test(`queue ${site} rejects malformed identifier before replay refusal`, () => {
const row = doc().rows.find(r => r.id === 6); edit(row);
assert.throws(() => validateRow(row), safe('', QueueError, { code: 2 }));
});
test('add owner reviewer rejects invalid owner before the conflict refusal', () => {
assert.throws(() => canonArgs('add', { piece: 'piece', gate: 'gate', brief: 'docs/brief.md#Brief', owner: secret, reviewers: [secret] }), safe('owner', QueueError, { code: 2 }));
});
test('claim mismatch names only the validated owner', () => {
const row = doc().rows.find(r => r.id === 6); row.claim.seat = secret;
assert.throws(() => validateRow(row), error => {
assert.ok(safe('claim seat', QueueError, { code: 2 })(error));
assert.ok(error.message.includes('darkwing'), error.message); return true;
});
});
test('duplicate logged op names the validated op; malformed op stops first', () => {
for (const op of ['genesis-op-0001', secret + '/']) {
const d = doc(); d.log[0].op = op; d.log.push({ ...structuredClone(d.log[0]), rev: 1 }); d.revision = 1;
assert.throws(() => loadDoc(Buffer.from(serialize(d))), op === 'genesis-op-0001' ? named('op id', op) : safe('op', QueueError, { code: 2 }));
}
});
test('unresolved request refusal names every open request', () => {
assert.throws(() => operation('review-request', {}, row => {
reviewRow(row);
row.review.rounds[0].attempts.push({ ...structuredClone(row.review.rounds[0].attempts[0]), op: 'second-request-01' });
}), named('unresolved review request', seatValue, 'second-request-01'));
});
test('approval refusal names every missing approver', () => {
assert.throws(() => operation('move', { to: 'done' }, row => {
reviewRow(row, 'posted'); row.gateOwner = 'sage'; row.reviewers = ['filbert', 'rocko'];
}), named('approval', 'filbert', 'rocko'));
});
+15 -15
View File
@@ -240,8 +240,8 @@ test("an unresolved request blocks a new request, a new round, waiting-on-jason
const head = s.head();
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; 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/);
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; 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; 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);
});
@@ -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; only the owner 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 by darkwing; only the owner \(darkwing\) 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, /reviewer 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, /filbert 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 every listed reviewer/);
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "filbert"), 2, /round 1 has no approval recorded by (?:filbert|rocko)(?:, rocko)?/);
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");
@@ -477,7 +477,7 @@ test("request, changes, a new candidate, approval: every round pinned; no review
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 or a privileged actor/);
no(s.run(["move", "9", "done", "--op", "done-9-000001"], "rocko"), 2, /only the gate owner \(filbert\) 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])]), [
@@ -569,7 +569,7 @@ test("semantics: v1 entries replay as before; review entries need v2", async (t)
// 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 already has its outcome/);
assert.throws(() => queue.applyEntry(full, again, {}), /request review-9-00001 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 the actor who made the request 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 darkwing, who made request review-9-00001, 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 can't be a listed reviewer/);
no(s.run(["set", "9", "reviewers", "filbert,darkwing", "--op", "reviewers-9-self"], "sage"), 2, /row 9 owner darkwing 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 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);
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)(?:, rocko)?$/m);
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "sage"), 2, /no approval recorded by (?:filbert|rocko)(?:, rocko)?$/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 every listed reviewer$/m);
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /no approval recorded by (?:filbert|rocko)(?:, rocko)?$/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 every listed reviewer$/m);
no(s.run(["move", "9", "waiting-on-jason", "--op", "wait-9-000001"], "darkwing"), 2, /row 9 round 2 has no approval recorded by (?:filbert|rocko)(?:, rocko)?$/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);
+3 -3
View File
@@ -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 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 rocko 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 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 darkwing 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: seat is one of its reviewers/);
no(cli(repo, ["assign", "9", "filbert", "--op", "assign-9-filbert"], { by: "sage" }), 2, /can't assign row 9: filbert 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" }));
+1 -1
View File
@@ -278,7 +278,7 @@ function validateFederation(value) {
fail(2, `${where}.id must match ${FEDERATION_ID_PATTERN}`);
}
if (ids.has(peer.id)) {
fail(2, `${where}.id is already named (by federation.self or an earlier peer)`);
fail(2, `${where}.id ${JSON.stringify(peer.id)} is already named (by federation.self or an earlier peer)`);
}
ids.add(peer.id);
validateFederationOrigin(peer.origin, `${where}.origin`);
+2 -2
View File
@@ -125,7 +125,7 @@ function validateMission(document, file) {
if (!SUPPORTED_TOOLS.includes(tool)) {
fail(2, `unsupported mission tool: (supported: ${SUPPORTED_TOOLS.join(", ")})`);
}
if (seen.has(tool)) fail(2, `duplicate tool in mission capabilities.tools`);
if (seen.has(tool)) fail(2, `duplicate tool in mission capabilities.tools: ${tool}`);
seen.add(tool);
}
capabilities = { tools: [...seen] };
@@ -230,7 +230,7 @@ function validateTask(document, file) {
if (!SUPPORTED_TOOLS.includes(tool)) {
fail(2, `unsupported tool: (supported: ${SUPPORTED_TOOLS.join(", ")})`);
}
if (seen.has(tool)) fail(2, `duplicate tool in capabilities.tools`);
if (seen.has(tool)) fail(2, `duplicate tool in capabilities.tools: ${tool}`);
seen.add(tool);
}
tools = [...seen];
+1 -1
View File
@@ -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 is already named' \
expect_fed_refusal "federation duplicate peer id exits 2 naming the key" 'federation.peers[1].id "factory" 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"}]}'