diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 0272092f2..ba10609b8 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -12,8 +12,6 @@ import { NAME as HARNESS_NAME } from '../../plugins/src/kinds.mjs'; import { loadRequirements } from '../../tasks/src/requirements.mjs'; import { MERGE, LandingRefusal, readLanding } from './landing.mjs'; import { pluginName, ID_PATTERN, THREAD_DEFINITIONS, END_PROTECTED, forkProblem, forkUrl, credentialKind, validArgv, commandAllowed, HOST_ID, CREATIVE_DEFINITIONS, giteaName, parseGiteaUrl, covers, grantScopeOf, grantInScope } from '../../organization/src/vocabulary.mjs'; -import { canonicalJson } from '../../organization/src/util.mjs'; -import { compositionOf } from '../../organization/src/definitions.mjs'; // Kept closed while S1 lands; its vocabulary is the integration authority. export const ACTIONS = Object.freeze([ @@ -6310,9 +6308,10 @@ export class Broker { // organization.change, the test organization.set uses; any other role gets decision-required. // The packs read checks the grant and never spends it, as organization.revision; an agent // without it gets human-required. A write checks before the writer, again inside it, and - // spends the grant once the writer has committed. The actor is the definition and instance. + // spends the grant once the writer has committed. A write the runtime skips (a repeat apply that + // adds nothing) never reaches beforeWrite and spends nothing. The actor is the definition and instance. const agent = !request.verb.endsWith('.console') && !s.human && !s.console; - let actor = s.human, authorityError = null; + let actor = s.human, authorityError = null, wrote = false; if (agent) { keys(args, ...PACK_BUS_KEYS[request.verb]); if (request.verb === 'packs') { @@ -6329,7 +6328,7 @@ export class Broker { if (!this.#organization) fail('not-configured'); const verb = request.verb.replace(/\.console$/, ''); const spends = agent && verb !== 'packs' && verb !== 'pack.preview'; - const agentContext = agent ? { agent: true, beforeWrite: () => { try { this.#checkAuthority(s, 'organization.change'); } catch (e) { authorityError = e; throw e; } } } : {}; + const agentContext = agent ? { agent: true, beforeWrite: () => { try { this.#checkAuthority(s, 'organization.change'); } catch (e) { authorityError = e; throw e; } wrote = true; } } : {}; const packFail = e => { if (authorityError && e === authorityError) throw e; this.#packFail(e); }; const openHires = verb === 'pack.preview' || verb === 'pack.apply' ? this.#store.all(`SELECT d.id FROM decisions d WHERE d.business=? AND d.action='role.hire' AND NOT EXISTS(SELECT 1 FROM decision_events x WHERE x.decision=d.id AND x.op IN ${CLOSED}) ORDER BY d.at, d.id`, s.business) @@ -6339,7 +6338,7 @@ export class Broker { try { result = this.#organization(s.business, verb, structuredClone(args), actor, { openHires, ...agentContext }); } catch (e) { packFail(e); } const done = (value) => { - if (spends) this.#store.transaction(() => this.#consumeAuthority(s, 'organization.change')); + if (spends && wrote) this.#store.transaction(() => this.#consumeAuthority(s, 'organization.change')); this.#secretCheck(value); return value; }; return result?.then ? result.then(done, packFail) : done(result); @@ -6701,11 +6700,14 @@ export class Broker { // stored entry must still hash to the pin (starter-pack-proposal-stale), and its from template // must not be in launch.noHire (definition-not-hireable), since prepare's own noHire check // reads only a template or definition ref. The returned input carries no proposal key, so - // prepare's request keys are unchanged. reason defaults to the one the pack recorded. + // prepare's request keys are unchanged. reason defaults to the one the pack recorded. The chief + // check and the role.hire check run before the packs list is read, so neither a non-chief nor a + // chief without role.hire learns whether a proposal exists. #proposalHire(s, args) { this.#agent(s); const b = this.#business(s.business); if (s.role !== b.launch?.by) fail('role-not-chief'); + if (this.#classification(s, 'role.hire') !== 'within-role') fail('not-authorized'); keys(args, ['proposal', 'route', 'reason'], ['proposal', 'route']); const proposal = args.proposal; keys(proposal, ['pack', 'project', 'hire'], ['pack', 'hire']); @@ -6722,10 +6724,14 @@ export class Broker { let role; if (Object.hasOwn(pin, 'inline')) { if (!object(hire.entry) || typeof pin.sha256 !== 'string') fail('invalid-request'); - if (sha256(canonicalJson(hire.entry)) !== pin.sha256) fail('starter-pack-proposal-stale'); + // The broker's sandbox import closure is fixed, so the digest and the composition are the pure + // local forms: digestPrd is sha256 over sorted-key JSON, the canonicalJson digest SP4 records + // for the parsed file; the composition is definitions.mjs compositionOf. + if (digestPrd(hire.entry) !== pin.sha256) fail('starter-pack-proposal-stale'); const from = hire.entry.from?.template; if (from !== undefined && (b.launch?.noHire ?? []).includes(from)) fail('definition-not-hireable'); - role = { inline: compositionOf(hire.entry) }; + if (!object(hire.entry.role) || typeof hire.entry.instructions !== 'string') fail('invalid-request'); + role = { inline: { ...hire.entry.role, instructions: hire.entry.instructions, ...(hire.entry.personality !== undefined ? { personality: hire.entry.personality } : {}) } }; } else role = structuredClone(pin); const reason = Object.hasOwn(args, 'reason') ? args.reason : hire.reason; return { op: 'hire', level: project === null ? 'org' : 'project', ...(project === null ? {} : { project }), name: hire.name, role, route: structuredClone(args.route), reason }; diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs index e92e20bb0..e3e0d7938 100644 --- a/packages/bus/src/runtime.mjs +++ b/packages/bus/src/runtime.mjs @@ -447,7 +447,10 @@ function packOps({ writer, rolesDir, dataRoot, system, registryFor, assertClean, const path = `${prefix}workflows.${target}`; const leftOut = []; if (agent) { - const unwritable = r => r !== 'human' && Object.hasOwn(pack.definitions, r) && !Object.hasOwn(doc.definitions ?? {}, role(r)) && !isTemplate(role(r)); + // A step keeps a pack role the organization already defines, or one the pack itself pins to the + // template of that name. An inline definition that only shares a template's name is left out. + const unwritable = r => r !== 'human' && Object.hasOwn(pack.definitions, r) && !Object.hasOwn(doc.definitions ?? {}, role(r)) + && !(checked[r].ref?.template === role(r) && isTemplate(role(r))); workflow.steps = workflow.steps.filter((s, i) => { const missing = value.steps[i].roles.filter(unwritable).map(role); if (missing.length) leftOut.push({ step: i + 1, title: s.title, roles: missing }); diff --git a/packages/bus/tests/packs-agent.test.mjs b/packages/bus/tests/packs-agent.test.mjs index 3cb8e20d7..5ea77c4a8 100644 --- a/packages/bus/tests/packs-agent.test.mjs +++ b/packages/bus/tests/packs-agent.test.mjs @@ -28,8 +28,10 @@ const chiefAuthority = shipped('chief-of-staff').authority; const sha = (text) => createHash('sha256').update(text).digest('hex'); const ROUTE = { harness: 'pi', model: 'claude-opus-5-5' }; -// The SP4 harness (packs.test.mjs) with the shipped chief grant and a coder without it. -async function harness(t, { mutate = () => {} } = {}) { +// The SP4 harness (packs.test.mjs) with the shipped chief grant and a coder without it. With +// fromFile, the broker boots from the loaded organization file, as autonomy-authority.test.mjs does, +// so the reload after a write reaches the broker and a later verb reads what was written. +async function harness(t, { mutate = () => {}, fromFile = false } = {}) { const { startBroker } = await import('../src/runtime.mjs'); const root = mkdtempSync(join(tmpdir(), 'packs5-')); const rolesDir = rolesCopy(root); @@ -43,7 +45,8 @@ async function harness(t, { mutate = () => {} } = {}) { chief: { definition: 'chief-of-staff', authority: chiefAuthority }, coder: { definition: 'coder', authority: { withinRole: ['message.send'], crossRole: [] } }, }; - const businesses = { demo: { id: 'demo', human: 'jason', arbiters: { technical: 'chief', delivery: 'chief' }, roles } }; + const loaded = () => { const l = loadBusiness('demo', { dir: root, rolesDir }); return { ...l, roles: Object.fromEntries(Object.entries(l.roles).map(([n, e]) => [n, { ...l.definitions[e.definition], ...e }])) }; }; + const businesses = { demo: fromFile ? loaded() : { id: 'demo', human: 'jason', arbiters: { technical: 'chief', delivery: 'chief' }, roles } }; const bus = await startBroker({ dataRoot: data, businesses, organization: { dir: root, rolesDir, system: systemFor(root) } }); const thaw = (p) => { const st = lstatSync(p); if (st.isSymbolicLink()) return; if (st.isDirectory()) { chmodSync(p, 0o700); for (const e of readdirSync(p)) thaw(join(p, e)); } }; t.after(async () => { await bus.close(); thaw(root); rmSync(root, { recursive: true, force: true }); }); @@ -107,6 +110,71 @@ test('SP5 the chief applies the coding pack: board and workflows written, no def assert.equal(decisions(), 0); }); +test('SP5 a repeat apply that writes nothing spends no grant', async t => { + const { call, chief, file, spent } = await harness(t); + const first = await call(chief, 'pack.preview', { pack: 'coding' }); + await call(chief, 'pack.apply', { pack: 'coding', digest: first.digest }); + assert.equal(spent(), 1); + const before = file(); + // The same pack again: the board and workflow are the same and the proposals unchanged, so the + // runtime skips the write and the broker spends nothing. + const again = await call(chief, 'pack.preview', { pack: 'coding' }); + const out = await call(chief, 'pack.apply', { pack: 'coding', digest: again.digest }); + assert.deepEqual(out.added, []); + assert.deepEqual(file(), before); + assert.equal(spent(), 1); +}); + +test('SP5 end to end: the chief applies a pack, the organization reloads, and the chief raises a hire from the recorded proposal', async t => { + const { call, chief, file, rolesDir, decisions } = await harness(t, { fromFile: true }); + const preview = await call(chief, 'pack.preview', { pack: 'coding' }); + await call(chief, 'pack.apply', { pack: 'coding', digest: preview.digest }); + assert.equal(decisions(), 0); + // The raise reads the packs list the broker reloaded after the write, not a fixture's. + const d = await call(chief, 'hire.raise', { proposal: { pack: 'coding', hire: 'coder-1' }, route: ROUTE }); + assert.equal(d.hire.name, 'coder-1'); + assert.equal(d.hire.level, 'org'); + assert.equal(decisions(), 1); + const direct = await call(chief, 'hire.raise', { op: 'hire', level: 'org', name: 'coder-b', role: { template: 'coder', sha256: templateHash(rolesDir, 'coder') }, route: ROUTE, reason: 'Code.' }); + assert.deepEqual(d.hire.role, direct.hire.role); + assert.deepEqual(d.hire.flagged, direct.hire.flagged); + // A second preview leaves out the hire the open decision names; the file is unchanged by the raise. + assert.equal((await call(chief, 'pack.preview', { pack: 'coding' })).hires.find(h => h.name === 'coder-1').mark, 'dropped'); + assert.equal(file().packs.length, 1); +}); + +test('SP5 a project pack: the chief applies it to a code project and raises the project hire from it', async t => { + const { call, chief, file, install, decisions } = await harness(t, { fromFile: true }); + install(variant('projkit', p => { p.scope = 'project'; })); + const preview = await call(chief, 'pack.preview', { pack: 'projkit', project: 'stack' }); + assert.equal(marks(preview)['projects.stack.board'], 'add'); + assert.equal(marks(preview)['definitions.coder'], 'not-written'); + await call(chief, 'pack.apply', { pack: 'projkit', project: 'stack', digest: preview.digest }); + assert.deepEqual(file().projects.stack.board, codingPack().board); + assert.equal(file().packs.at(-1).project, 'stack'); + // The proposal names its project; without it, no org-level entry matches. + await assert.rejects(call(chief, 'hire.raise', { proposal: { pack: 'projkit', hire: 'coder-1' }, route: ROUTE }), { code: 'invalid-request' }); + const d = await call(chief, 'hire.raise', { proposal: { pack: 'projkit', project: 'stack', hire: 'coder-1' }, route: ROUTE }); + assert.equal(d.hire.level, 'project'); + assert.equal(d.hire.project, 'stack'); + assert.equal(decisions(), 1); +}); + +test('SP5 an inline pack definition that shares a template name does not keep its workflow step', async t => { + const { call, chief, install, rolesDir } = await harness(t); + const { rev: _rev, ...entry } = entryFromTemplate(readTemplate(rolesDir, 'coder')); + install(variant('shadowkit', p => { + p.definitions = { reviewer: { ...entry, instructions: entry.instructions + '\nShadow.' } }; + p.hires = []; + p.workflows = { check: { description: 'Check.', steps: [{ id: 'look', title: 'Look', state: 'todo', roles: ['human'] }, { id: 'review', title: 'Review it', state: 'in-review', roles: ['reviewer'] }] } }; + })); + const preview = await call(chief, 'pack.preview', { pack: 'shadowkit' }); + const item = preview.items.find(i => i.path === 'workflows.check'); + assert.equal(item.mark, 'add'); + assert.deepEqual(item.leftOut, [{ step: 2, title: 'Review it', roles: ['reviewer'] }]); + assert.deepEqual(preview.plan.workflows.check.steps.map(s => s.roles), [['human']]); +}); + test('SP5 a role without organization.change gets decision-required for the writes and human-required for the packs read', async t => { const { call, coder, chief, path, revision, install, spent } = await harness(t); const before = readFileSync(path); @@ -142,7 +210,7 @@ test('SP5 the chief writes a board and a workflow, each spending the grant once, assert.equal(file().workflows, undefined); assert.equal(spent(), 3); // A refused write spends nothing. - await assert.rejects(call(chief, 'board.set', { revision: 'b'.repeat(64), board: null }), /organization-|invalid-request|stale|conflict/); + await assert.rejects(call(chief, 'board.set', { revision: 'b'.repeat(64), board: null }), { code: 'revision-conflict' }); await assert.rejects(call(chief, 'workflow.set', { revision: revision(), name: 'x', workflow: { description: 'x', steps: [{ title: 'Do', state: 'todo', roles: ['nobody-at-all'] }] } }), /invalid-request/); assert.equal(spent(), 3); }); @@ -277,6 +345,18 @@ test('SP5 a proposal raise: role-not-chief, decision-stale, definition-rev-confl assert.equal(count(), 2); }); +test('SP5 a chief without role.hire is refused a proposal raise before the packs list is read', async t => { + const { record, pack, raise, b, loaded, rolesDir, count } = fixture(t); + record({ packs: pack([{ name: 'scout', reason: 'Review.', pin: { template: 'reviewer', sha256: templateHash(rolesDir, 'reviewer') } }]) }); + const doc = loaded(); + const authority = doc.roles.chief.authority; + b.reloadBusiness('demo', { ...doc, roles: { ...doc.roles, chief: { ...doc.roles.chief, authority: { ...authority, withinRole: authority.withinRole.filter(a => a !== 'role.hire') } } } }); + await assert.rejects(raise({ pack: 'kit', hire: 'scout' }), { code: 'not-authorized' }); + // A proposal that does not exist gets the same refusal, so the refusal leaks nothing. + await assert.rejects(raise({ pack: 'nosuch', hire: 'nobody' }), { code: 'not-authorized' }); + assert.equal(count(), 0); +}); + test('SP5 a proposal raise names no role, name, level or other hire key; unknown proposals refuse; a plain raise is unchanged', async t => { const { rolesDir, record, pack, raise, call, caps, count } = fixture(t); const pin = { template: 'reviewer', sha256: templateHash(rolesDir, 'reviewer') };