fix(packs): SP4 re-check, the board route method split, human role reserved, distinct step roles (#1873)

- serve.mjs: only GET and HEAD /api/board stay the Board read; every
  other method goes to the pack route, so DELETE and PUT answer its 405
  again. The owner-only check moves inside packsHTTP after the method
  check, so OPTIONS on a root Console stays 405 as before SP4.
- runtime.mjs: a --as rename cannot target the definition name human,
  and a human step role never follows a rename. A step whose mapped
  roles collide refuses invalid-request at preview, not only at apply.
- The no-op apply skip also needs the same pack sha256, so a changed
  pack with nothing to add still records its entry.
- Tests: the --as refusals for human, a step role collision, a pack
  defining human, a changed saved pack recording its entry, board by
  PUT, a granted session refused owner-only on every pack route, and a
  Console without sign-in refused owner-only.

packages/bus/src/runtime.mjs

@@ -323,7 +323,8 @@ function packOps({ writer, rolesDir, dataRoot, system, registryFor, assertClean,
     if (!Array.isArray(as) || as.length > 32) bad();
     for (const value of as) {
       const m = typeof value === 'string' ? PACK_AS.exec(value) : null;
-      if (!m || out[m[1]].has(m[2]) || [...out[m[1]].values()].includes(m[3])) bad();
+      // `human` is the reserved step role: no rename may write a definition by that name.
+      if (!m || out[m[1]].has(m[2]) || [...out[m[1]].values()].includes(m[3]) || (m[1] === 'definitions' && m[3] === 'human')) bad();
       out[m[1]].set(m[2], m[3]);
     }
     return out;
@@ -416,9 +417,11 @@ function packOps({ writer, rolesDir, dataRoot, system, registryFor, assertClean,
     for (const [name, value] of Object.entries(pack.workflows)) {
       const target = as.workflows.get(name) ?? name;
       // The step ids are the pack's own; the organization's workflow keeps title, state and roles.
-      // A role naming one of this pack's definitions follows that definition's --as rename.
-      const role = r => Object.hasOwn(pack.definitions, r) ? as.definitions.get(r) ?? r : r;
+      // A role naming one of this pack's definitions follows that definition's --as rename; the
+      // reserved `human` role never moves. A step whose mapped roles collide refuses here, not at apply.
+      const role = r => r !== 'human' && Object.hasOwn(pack.definitions, r) ? as.definitions.get(r) ?? r : r;
       const workflow = { description: value.description, steps: value.steps.map(s => ({ title: s.title, state: s.state, roles: s.roles.map(role) })) };
+      if (workflow.steps.some(s => new Set(s.roles).size !== s.roles.length)) bad();
       const path = `${prefix}workflows.${target}`;
       const current = scope.workflows?.[target];
       let mark;
@@ -468,11 +471,13 @@ function packOps({ writer, rolesDir, dataRoot, system, registryFor, assertClean,
       if (typeof args.digest !== 'string') bad();
       const preview = await build(id, args, actor, context);
       if (preview.digest !== args.digest) throw new BusError('pack-plan-stale');
-      // An apply that adds nothing and proposes what the last apply of this pack proposed writes
-      // nothing, so a retry loop does not fill the packs list to its cap. The digest pins the revision.
+      // An apply of the same pack (same sha256) that adds nothing and proposes what the last apply of
+      // it proposed writes nothing, so a retry loop does not fill the packs list to its cap. A changed
+      // pack still records its entry. The digest pins the revision.
       const { plan } = preview;
       const last = (writer.read(id).document.packs ?? []).filter(e => sameApply(e, plan.packs.id, plan.packs.origin, plan.packs.project)).at(-1);
-      if (Object.keys(plan).every(k => k === 'packs') && last && canonicalJson(last.proposedHires ?? []) === canonicalJson(plan.packs.proposedHires))
+      if (Object.keys(plan).every(k => k === 'packs') && last && last.sha256 === plan.packs.sha256
+        && canonicalJson(last.proposedHires ?? []) === canonicalJson(plan.packs.proposedHires))
         return { revision: preview.revision, added: [], pack: preview.pack.id };
       return { ...write(id, plan, preview.revision, actor, {}), pack: preview.pack.id };
     },

packages/bus/tests/packs.test.mjs

@@ -203,7 +203,7 @@ test('SP4 --as renames a definition or workflow and pins the hire to the new nam
   assert.equal(file().definitions.coder, undefined);
   assert.ok(file().workflows.ship);
   // A rename onto another pack item's own name would show both as add while one replaced the other.
-  for (const as of [['definitions.nobody=x'], ['definitions.coder=a', 'definitions.coder=b'], ['definitions.coder=x', 'definitions.pm=x'], ['roles.coder=x'], 'definitions.coder=x', ['definitions.coder=reviewer']])
+  for (const as of [['definitions.nobody=x'], ['definitions.coder=a', 'definitions.coder=b'], ['definitions.coder=x', 'definitions.pm=x'], ['roles.coder=x'], 'definitions.coder=x', ['definitions.coder=reviewer'], ['definitions.coder=human'], ['definitions.merge-gate=human']])
     await assert.rejects(call('pack.preview.console', { pack: 'coding', as }), /invalid-request/);
 });
 
@@ -226,6 +226,45 @@ test('SP4 --as onto another workflow of the pack refuses; a renamed inline defin
   assert.ok(file().definitions.writer);
   assert.equal(file().definitions.scribe, undefined);
   assert.deepEqual(file().workflows.notes.steps[0].roles, ['writer', 'human']);
+  // A rename onto a role the same step already names refuses at preview, not only at apply.
+  install(variant('pairkit', p => {
+    p.definitions = { scribe };
+    p.hires = [];
+    p.workflows = { notes: { description: 'Notes.', steps: [{ id: 'write', title: 'Write it', state: 'todo', roles: ['scribe', 'pm'] }] } };
+  }));
+  await call('pack.preview.console', { pack: 'pairkit' });
+  await assert.rejects(call('pack.preview.console', { pack: 'pairkit', as: ['definitions.scribe=pm'] }), /invalid-request/);
+  // The reserved human step role never follows a rename, even when the pack defines a definition by that name.
+  install(variant('humankit', p => {
+    p.definitions = { human: scribe };
+    p.hires = [];
+    p.workflows = { signoff: { description: 'Sign-off.', steps: [{ id: 'sign', title: 'Sign it', state: 'todo', roles: ['human'] }] } };
+  }));
+  const renamed = await call('pack.preview.console', { pack: 'humankit', as: ['definitions.human=bot'] });
+  assert.deepEqual(renamed.plan.workflows.signoff.steps[0].roles, ['human']);
+});
+
+test('SP4 a changed pack with nothing to add still records its entry; an identical repeat writes nothing', async t => {
+  const { call, file, save } = await harness(t);
+  save('jason', 'mine', variant('mine'));
+  // The first apply pins the definitions it adds; the second pins the pack's template refs, so it
+  // records an entry too. From the second on, the proposals are settled.
+  for (let i = 0; i < 2; i++) {
+    const p = await call('pack.preview.console', { pack: 'saved:mine' });
+    await call('pack.apply.console', { pack: 'saved:mine', digest: p.digest });
+  }
+  assert.equal(file().packs.length, 2);
+  save('jason', 'mine', variant('mine', p => { p.version = '1.0.1'; p.description = 'A changed description.'; }));
+  const changed = await call('pack.preview.console', { pack: 'saved:mine' });
+  assert.ok(changed.items.every(i => i.mark === 'same'), JSON.stringify(changed.items));
+  await call('pack.apply.console', { pack: 'saved:mine', digest: changed.digest });
+  assert.equal(file().packs.length, 3);
+  assert.equal(file().packs.at(-1).version, '1.0.1');
+  assert.notEqual(file().packs.at(-1).sha256, file().packs[1].sha256);
+  const repeat = await call('pack.preview.console', { pack: 'saved:mine' });
+  const out = await call('pack.apply.console', { pack: 'saved:mine', digest: repeat.digest });
+  assert.deepEqual(out.added, []);
+  assert.equal(file().packs.length, 3);
 });
 
 test('SP4 an instance by the hire name drops the proposal; launch.noHire marks it', async t => {

packages/webui/src/packs-http.mjs

@@ -51,7 +51,7 @@ async function body(req, required, optional) {
   return value;
 }
 
-export async function packsHTTP(req, res, { path, read, write, session, live, origin }) {
+export async function packsHTTP(req, res, { path, owner, read, write, session, live, origin }) {
   const refuse = (status, error) => json(res, status, { error });
   const fail = error => { const code = codes[error?.code] ? error.code : 'outcome-unknown'; return refuse(codes[code] ?? 503, code); };
   const match = PACKS_ROUTE.exec(path);
@@ -64,6 +64,8 @@ export async function packsHTTP(req, res, { path, read, write, session, live, or
   const list = path === '/api/packs';
   const allowed = list ? ['GET'] : name !== null ? ['POST', 'DELETE'] : ['POST'];
   if (!allowed.includes(req.method)) return refuse(405, 'method-not-allowed');
+  // A method the route never serves answers 405 first; every served method is the root owner's.
+  if (!owner) return refuse(403, 'owner-only');
   if (!session) return refuse(401, 'sign-in-required');
   if (!read) return refuse(503, 'not-configured');
   const writes = !list && step !== 'preview';

packages/webui/src/serve.mjs

@@ -934,8 +934,8 @@ export async function startServer({ host = '127.0.0.1', port = 7330, board = DEF
     if (sso !== null && !named && !(path === '/healthz' && req.method === 'GET')) return json(res, 403, { error: 'public Host required' });
     // No CORS. JSON content type and same-origin checks keep browser forms out.
     const decisionPath = DECISION_ROUTE.test(path), requestPath = REQUEST_ROUTE.test(path), remotePath = PROJECT_REMOTE_ROUTE.test(path), systemTypesPath = SYSTEM_TYPES_ROUTE.test(path), autonomyPath = AUTONOMY_ROUTE.test(path),
-      // GET /api/board stays the Board read; only POST /api/board is the pack route's board write.
-      packsPath = PACKS_ROUTE.test(path) && (path !== '/api/board' || req.method === 'POST');
+      // GET and HEAD /api/board stay the Board read; every other method on it is the pack route's (POST writes, the rest 405).
+      packsPath = PACKS_ROUTE.test(path) && !(path === '/api/board' && (req.method === 'GET' || req.method === 'HEAD'));
     if (req.headers.origin && req.headers.origin !== (named ? (sso ? (localOrigin ?? sso.publicOrigin) : `https://${preview.host}`) : `http://${req.headers.host}`))
       return decisionPath ? decisionRefuse(res, 'cross-origin') : requestPath ? requestRefuse(res, 'cross-origin') : json(res, 403, { error: 'cross-origin request refused' });
     let signer = preview, ssoSession = null;
@@ -1077,9 +1077,9 @@ export async function startServer({ host = '127.0.0.1', port = 7330, board = DEF
     }
     // Starter packs SP4 (#1873): the owner's pack, board and workflow routes on the selected organization.
     if (packsPath) {
-      if (tester || granted || !named || sso === null) return json(res, 403, { error: 'owner-only' });
-      return packsHTTP(req, res, { path, read: requestRead, write: send ? packsWrite : null,
-        session: ssoSession, live: () => { const live = sso.session(req); return live?.rootOwner === true ? live : null; }, origin: localOrigin ?? sso.publicOrigin });
+      const owner = !(tester || granted || !named || sso === null);
+      return packsHTTP(req, res, { path, owner, read: requestRead, write: send ? packsWrite : null,
+        session: ssoSession, live: () => { const live = sso.session(req); return live?.rootOwner === true ? live : null; }, origin: localOrigin ?? sso?.publicOrigin });
     }
     if (tester && ['/project-remote.js', '/project-remote.css', '/files.js', '/files.css', '/system-types.js', '/system-types.css'].includes(path)) return json(res, 403, { error: 'owner-only' });
     // Row 194 (#1692, D4.1): the tester also launches and terminates.

packages/webui/tests/packs-http.test.mjs

@@ -143,6 +143,7 @@ test('SP4 the admission checks refuse before any bus call; a read-only Console s
     ['cross site', `${ORG}/board`, 'POST', { revision: revisionNow, board: BOARD }, { 'sec-fetch-site': 'cross-site' }, [403, 'cross-origin']],
     ['preview by GET', `${ORG}/packs/preview`, 'GET', undefined, {}, [405, 'method-not-allowed']],
     ['board by DELETE', `${ORG}/board`, 'DELETE', { revision: revisionNow }, {}, [405, 'method-not-allowed']],
+    ['board by PUT', `${ORG}/board`, 'PUT', { revision: revisionNow }, {}, [405, 'method-not-allowed']],
   ]) {
     const [status, result] = await call(f, path, method, body, headers);
     assert.deepEqual([status, result.error], expected, label);
@@ -181,8 +182,9 @@ test('SP4 a session that is not the root owner reaches no pack route', async t =
   const before = readFileSync(f.file);
   f.revokeOwner();
   for (const [path, method, body] of [[`${ORG}/packs`, 'GET'], [`${ORG}/packs/preview`, 'POST', { pack: 'coding' }], [`${ORG}/packs/apply`, 'POST', { pack: 'coding', digest: preview.digest }], [`${ORG}/workflows/one`, 'POST', { revision: preview.revision, workflow: FLOW('one') }], [`${ORG}/board`, 'POST', { revision: preview.revision, board: BOARD }]]) {
-    const [status] = await call(f, path, method, body);
-    assert.ok([401, 403, 404].includes(status), `${method} ${path} answered ${status}`);
+    // revokeOwner leaves a granted user's session: the route refuses it owner-only, POST /api/board included.
+    const [status, result] = await call(f, path, method, body);
+    assert.deepEqual([status, result.error], [403, 'owner-only'], `${method} ${path}`);
   }
   assert.equal(f.calls.length, asked, 'nothing reached the bus');
   assert.deepEqual(readFileSync(f.file), before);
@@ -207,3 +209,14 @@ test('SP4 a sign-out while the body is still arriving dispatches nothing', async
   assert.equal(f.calls.length, asked);
   assert.deepEqual(readFileSync(f.file), before);
 });
+
+test('SP4 a Console without sign-in serves no pack route; a method the route never serves stays 405', async t => {
+  const web = await startServer({ port: 0, board: 'http://127.0.0.1:9', log: () => {} });
+  t.after(() => close(web));
+  const f = { base: `http://127.0.0.1:${web.address().port}` };
+  for (const [path, method, body] of [['/api/packs', 'GET'], ['/api/packs/preview', 'POST', { pack: 'coding' }], ['/api/board', 'POST', { revision: '0'.repeat(64), board: BOARD }]]) {
+    const [status, result] = await call(f, path, method, body);
+    assert.deepEqual([status, result.error], [403, 'owner-only'], `${method} ${path}`);
+  }
+  for (const method of ['OPTIONS', 'PUT', 'DELETE']) assert.equal((await call(f, '/api/board', method))[0], 405, method);
+});