fix(packs): SP5 review round, broker import closure, no-op spend, role.hire before the read, template-named steps (#1873)

The broker no longer imports organization util or definitions; the proposal
pin digest is digestPrd and the inline composition is built in place, so the
sandbox client import closure project-runtime.test.mjs pins is unchanged.
An agent apply the runtime skips (a repeat that adds nothing) never reaches
beforeWrite and spends no grant. A proposal raise checks role.hire before it
reads the packs list. An agent apply keeps a workflow step only for a pack
role the organization defines or one the pack pins to the template of that
name; an inline definition sharing a template's name is left out. Tests: a
repeat apply spends nothing, apply then raise end to end with the broker
booted from the file, a project pack, the template-name shadow, and a chief
without role.hire; the revision refusal is matched exactly.

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 };

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 });

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') };