fix(packs): SP8 review round, single-pass placeholders, CLI hints, pins (#1873)

- template-save: one alternation pass replaces every literal, so an
  instance named root, org or agent never rewrites a placeholder already
  written in. checkPack's refusal message is passed on as the reason. A
  write or fsync failure removes the partial file it created.
- broker #templateFail drops a reason the credential check refuses.
- cli template words: humanTransport throws a CliError named by the bus
  code; a known code now gains its hint (the test transport throws the
  same CliError).
- template-http: the header notes serve.mjs strips /api/orgs/<id>.
- pins: the two .console verbs in console-decide and request-console;
  the packages/business/src re-export for template-save.mjs.
- tests: placeholder collision, a held-value key masked as [key], and a
  save after a real pack.apply.console keeping {id, version}.

packages/bus/src/broker.mjs

@@ -7046,7 +7046,12 @@ export class Broker {
   // #1873 SP8: a template refusal keeps its code and its reason (a path, never the text); anything
   // else is read as a pack refusal.
   #templateFail(e) {
-    if (e?.name === 'TemplateError' && TEMPLATE_CODES.includes(e.code)) throw new BusError(e.code, typeof e.reason === 'string' ? e.reason.slice(0, 300) : undefined);
+    if (e?.name === 'TemplateError' && TEMPLATE_CODES.includes(e.code)) {
+      // A reason names a field path or a definition name; one that would quote a held value is dropped.
+      let reason = typeof e.reason === 'string' ? e.reason.slice(0, 300) : undefined;
+      if (reason !== undefined) try { this.#secretCheck(reason); } catch { reason = undefined; }
+      throw new BusError(e.code, reason);
+    }
     this.#packFail(e);
   }
   // The bus code for an organization writer's refusal. Anything the writer didn't refuse by name is

packages/bus/tests/console-decide.test.mjs

@@ -92,7 +92,9 @@ test('N4: owner Console verbs have an explicit immutable whitelist and exclude d
   assert.deepEqual([...CONSOLE_VERBS], ['system-types.console', 'project.remote.get.console', 'project.remote.set.console', 'autonomy.get.console', 'autonomy.set.console', 'autonomy.review.console', 'decision.confirm.console', 'decision.reverse.console', 'request.workspace.console', 'request.artifact.console', 'decision.preview.console', 'decision.resolve.console', 'hire.preview.console', 'hire.resolve.console', 'hire.start.console', 'hire.templates.console', 'prd.revoke.console', 'request.mode.console', 'request.draft.console',
     'request.get.console', 'request.projects.console', 'request.answer.console', 'request.attach.console', 'request.cancel.console',
     // Starter packs SP4 (#1873): the owner's pack list and preview, and the three pack writes.
-    'packs.console', 'pack.preview.console', 'pack.apply.console', 'board.set.console', 'workflow.set.console']);
+    'packs.console', 'pack.preview.console', 'pack.apply.console', 'board.set.console', 'workflow.set.console',
+    // Starter packs SP8 (#1873): the owner saves and lists templates.
+    'template.save.console', 'template.list.console']);
   assert.ok(CONSOLE_VERBS.every(verb => verb.endsWith('.console')));
   assert.equal(CONSOLE_VERBS.includes('decision.resolve'), false);
   assert.ok(Object.isFrozen(CONSOLE_VERBS));

packages/bus/tests/request-console.test.mjs

@@ -93,7 +93,9 @@ test('the console verb list includes the artifact read and existing owner contro
   assert.deepEqual([...CONSOLE_VERBS], ['system-types.console', 'project.remote.get.console', 'project.remote.set.console', 'autonomy.get.console', 'autonomy.set.console', 'autonomy.review.console', 'decision.confirm.console', 'decision.reverse.console', 'request.workspace.console', 'request.artifact.console', 'decision.preview.console', 'decision.resolve.console', 'hire.preview.console', 'hire.resolve.console', 'hire.start.console', 'hire.templates.console', 'prd.revoke.console', 'request.mode.console', 'request.draft.console',
     'request.get.console', 'request.projects.console', 'request.answer.console', 'request.attach.console', 'request.cancel.console',
     // Starter packs SP4 (#1873): the owner's pack list and preview, and the three pack writes.
-    'packs.console', 'pack.preview.console', 'pack.apply.console', 'board.set.console', 'workflow.set.console']);
+    'packs.console', 'pack.preview.console', 'pack.apply.console', 'board.set.console', 'workflow.set.console',
+    // Starter packs SP8 (#1873): the owner saves and lists templates.
+    'template.save.console', 'template.list.console']);
 });
 
 test('only the signed-in owner capability reaches the console verbs and it reaches nothing else', async t => {

packages/bus/tests/templates.test.mjs

@@ -124,6 +124,32 @@ test('SP8 the real credential store: a held value in kept text refuses template-
   assert.ok(!existsSync(join(root, 'templates')));
 });
 
+test('SP8 a key that holds a held value: the refusal reason masks it as [key], never the value', async t => {
+  // A definition keyed by one never loads (definition-holds-credential); a workflow keyed by one is
+  // what save sees. The broker also drops any reason its credential check refuses.
+  const workflow = { description: 'Ship it.', steps: [{ title: 'Write', state: 'todo', roles: ['writer'] }] };
+  const { call, root } = await harness(t, { mutate: doc => { doc.workflows = { [TOKEN]: workflow }; } });
+  await assert.rejects(call('template.save.console', { name: 'leaky', preview: true }), (e) => {
+    assert.equal(e.code, 'template-secret-suspected');
+    assert.equal(e.reason, 'workflows[key]');
+    assert.ok(!String(e.message).includes(TOKEN) && !JSON.stringify(e).includes(TOKEN));
+    return true;
+  });
+  assert.ok(!existsSync(join(root, 'templates')));
+});
+
+test('SP8 a save after a real pack.apply.console keeps each packs entry as {id, version}', async t => {
+  const { call, saved } = await harness(t);
+  const packs = await call('packs.console');
+  const shipped = packs.packs.find(p => p.origin === 'shipped');
+  assert.ok(shipped, JSON.stringify(packs.packs.map(p => [p.id, p.origin])));
+  const pack = await call('pack.preview.console', { pack: shipped.id });
+  await call('pack.apply.console', { pack: shipped.id, digest: pack.digest });
+  await save(call, 'after-apply');
+  const kept = JSON.parse(readFileSync(saved('after-apply'), 'utf8')).packs;
+  assert.deepEqual(kept, [{ id: shipped.id, version: pack.pack.version }]);
+});
+
 test('SP8 argument checks: a second save, a .. name, an unknown key, preview with a digest, and a stale digest', async t => {
   const { call, saved } = await harness(t);
   const { preview } = await save(call, 'demo-shape');

packages/business/src/template-save.mjs

@@ -0,0 +1,2 @@
+// Old path of packages/organization/src/template-save.mjs (rename design r2, stage B).
+export * from "../../organization/src/template-save.mjs";

packages/cli/src/org.mjs

@@ -149,9 +149,11 @@ async function templateCommand(args, io, deps) {
   let result;
   try { result = await packCall(id, io, deps)(verb, request); }
   catch (e) {
-    if (typeof e?.code !== 'string' || e.code === '' || e instanceof CliError) throw e;
-    const why = TEMPLATE_REFUSALS[e.code] ?? PACK_REFUSALS[e.code];
-    const error = new CliError(why ? `${e.code}: ${why}` : e.code, busExit(e.code));
+    // humanTransport throws a CliError named by the bus code alone: a known code gains its hint.
+    const why = typeof e?.code !== 'string' ? undefined
+      : Object.hasOwn(TEMPLATE_REFUSALS, e.code) ? TEMPLATE_REFUSALS[e.code] : Object.hasOwn(PACK_REFUSALS, e.code) ? PACK_REFUSALS[e.code] : undefined;
+    if (!why) throw e;
+    const error = new CliError(`${e.code}: ${why}`, busExit(e.code));
     error.code = e.code;
     throw error;
   }

packages/cli/tests/org-templates.test.mjs

@@ -8,6 +8,8 @@ import { join } from 'node:path';
 import { main } from '../src/cli.mjs';
 import { ORG_USAGE, TEMPLATE_USAGE } from '../src/org.mjs';
 import { bootConfig, loadSystem } from '../src/config.mjs';
+import { CliError } from '../src/errors.mjs';
+import { busExit } from '../src/transport.mjs';
 import { BusError } from '../../bus/src/broker.mjs';
 import { startBroker } from '../../bus/src/runtime.mjs';
 import { fixture, io, tmp } from './helpers.mjs';
@@ -24,7 +26,8 @@ async function setup(t) {
   const deps = { system: { dataRoot: f.dataRoot }, transport: () => async (verb, args = {}) => {
     calls.push([verb, Object.keys(args).sort()]);
     try { return structuredClone(await b.broker.request(human, { verb, args })); }
-    catch (e) { if (!(e instanceof BusError)) throw e; throw Object.assign(new Error(e.code), { code: e.code }); }
+    // What humanTransport throws for a bus refusal: a CliError named by the code alone.
+    catch (e) { if (!(e instanceof BusError)) throw e; throw Object.assign(new CliError(e.code, busExit(e.code)), { code: e.code }); }
   } };
   const run = async (args, env = f.env) => { const x = io(env); await main(['org', ...args], x, deps); return JSON.parse(x.out.text); };
   const saved = (name) => join(boot.organization.dir, 'templates', f.doc.human, `${name}.json`);

packages/organization/src/template-save.mjs

@@ -19,7 +19,7 @@
 // A refusal is a TemplateError: a bus code and a reason that names a path or a checked name only,
 // never a value from the document and never a host path.
 
-import { closeSync, constants, fchmodSync, fsyncSync, lstatSync, mkdirSync, openSync, readdirSync, writeSync } from "node:fs";
+import { closeSync, constants, fchmodSync, fsyncSync, lstatSync, mkdirSync, openSync, readdirSync, unlinkSync, writeSync } from "node:fs";
 import { join } from "node:path";
 import { ORG_PREFIX, checkDefinition, readTemplate } from "./definitions.mjs";
 import { sha256 } from "./util.mjs";
@@ -122,19 +122,26 @@ function literals(document, { rolesDir, cache }) {
   for (const project of Object.values(document.projects ?? {})) for (const name of Object.keys(project?.agents ?? {})) agents.add(name);
   for (const name of agents) if (!role(name)) add(name, "{agent}");
   out.sort((a, b) => b.text.length - a.text.length);
-  return out.map((l) => ({ ...l, pattern: new RegExp(`(?<![${WORD}])${escape(l.text)}(?![${WORD}])`, "g") }));
+  return out;
 }
 
-// Returns the text with every literal replaced, and counts each placeholder used.
+// Returns the text with every literal replaced, and counts each placeholder used. One pass over the
+// text with one alternation, longest literal first: a placeholder written in never matches a later
+// literal, so an instance named "root" or "agent" cannot rewrite {root} or {agent}.
+const combined = new WeakMap();
 function replace(text, list, counts) {
-  let out = text;
-  for (const { pattern, placeholder } of list) {
-    out = out.replace(pattern, () => {
-      counts[placeholder] = (counts[placeholder] ?? 0) + 1;
-      return placeholder;
-    });
+  if (list.length === 0) return text;
+  let entry = combined.get(list);
+  if (!entry) {
+    const by = new Map(list.map((l) => [l.text, l.placeholder]));
+    entry = { by, pattern: new RegExp(`(?<![${WORD}])(?:${list.map((l) => escape(l.text)).join("|")})(?![${WORD}])`, "g") };
+    combined.set(list, entry);
   }
-  return out;
+  return text.replace(entry.pattern, (match) => {
+    const placeholder = entry.by.get(match);
+    counts[placeholder] = (counts[placeholder] ?? 0) + 1;
+    return placeholder;
+  });
 }
 
 // ---- Capture ----
@@ -368,7 +375,9 @@ export function templatePreview({ document, name, project, rolesDir, assertClean
   try {
     checked = checkPack(template, { origin: "saved", rolesDir });
   } catch (error) {
-    if (error?.name === "PluginError") refuse(/^starter-pack-[a-z-]+$/.test(error.code ?? "") ? error.code : "starter-pack-invalid");
+    if (error?.name === "PluginError") {
+      refuse(/^starter-pack-[a-z-]+$/.test(error.code ?? "") ? error.code : "starter-pack-invalid", typeof error.message === "string" ? error.message.slice(0, 300) : undefined);
+    }
     throw error;
   }
   const json = `${JSON.stringify(checked, null, 2)}\n`;
@@ -429,8 +438,14 @@ export function templateSave({ document, name, project, digest, rolesDir, assert
     let offset = 0;
     while (offset < bytes.length) offset += writeSync(fd, bytes, offset);
     fsyncSync(fd);
-  } finally {
+  } catch (error) {
+    // A partial file would hold the name: remove the file this call created, then report the failure.
     closeSync(fd);
+    fd = undefined;
+    try { unlinkSync(file); } catch { /* already gone */ }
+    throw error;
+  } finally {
+    if (fd !== undefined) closeSync(fd);
   }
   return { name, scope: preview.scope, ...(project === undefined ? {} : { project }), digest };
 }

packages/organization/tests/template-save.test.mjs

@@ -113,6 +113,17 @@ test("SP8 placeholders replace the org and project ids, the owner, agent names a
   assert.deepEqual(at("definitions.writer.instructions"), ["{agent}", "{org}", "{owner}", "{project}", "{root}"]);
 });
 
+test("SP8 an instance named root, org or agent never rewrites a placeholder already written in", (t) => {
+  const { doc, rolesDir } = fixture(t);
+  doc.roles.root = { definition: "org:writer" };
+  doc.roles.org = { definition: "org:writer" };
+  doc.definitions.writer.instructions = "Work for acme with root on stack. Ask org, then jason.";
+  const preview = templatePreview({ document: doc, name: "team-shape", rolesDir, assertClean });
+  assert.equal(preview.template.definitions.writer.instructions, "Work for {org} with {agent} on {project}. Ask {agent}, then {owner}.");
+  const at = preview.replacements.filter((r) => r.path === "definitions.writer.instructions").map((r) => r.placeholder).sort();
+  assert.deepEqual(at, ["{agent}", "{org}", "{owner}", "{project}"]);
+});
+
 test("SP8 the preview lists every kept free-text field with its saved text", (t) => {
   const { doc, rolesDir } = fixture(t);
   const preview = templatePreview({ document: doc, name: "acme-shape", rolesDir, assertClean });

packages/webui/src/template-http.mjs

@@ -2,6 +2,9 @@
 // template under their own account, and lists the templates they saved. Owner Console transports only;
 // no caller-supplied business, account, subject or origin reaches the bus.
 //
+// serve.mjs strips /api/orgs/<id> before the route is matched, so the paths below reach this module as
+// /api/templates and /api/templates/preview.
+//
 // GET  /api/orgs/<id>/templates           {templates, writable}: template.list.console, the owner's saved templates
 // POST /api/orgs/<id>/templates/preview   {name, project?}: template.save.console with preview true. Writes nothing;
 //                                         answers the template JSON, every free-text field and the digest