From 3e53af3941eb4a4c64ac2d8e6f7109106aae9985 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sat, 14 Nov 2026 16:09:46 -0600 Subject: [PATCH] fix(bus,webui): pd review round, supersede after apply, one pack hire loop per org, hireError, fromPack (#1873, lane pd) Review pd-review.md (changes 6). Fixed: - P2#2: the owner's pack hire withdraws a pack-raised inbox decision only after its replacement hire applied; a replacement that refuses (route missing, prepare refusal) leaves the old decision open in the inbox. - P3#3: an apply whose hire loop throws after the apply committed still answers the apply receipt, with no hires and the code as hireError; the route passes a code through and the card says the hires did not run. - P3#5: the packs read marks an awaiting-owner line this pack's own hire raised fromPack: true; the owner's card counts it in "Hire the N". - P3#6: one pack hire loop per business; a second refuses organization-busy and raises nothing. - P2#1 docs: TOOLS.md and the bus README say no member surface calls pack.hire.console yet (the member surface is a recorded follow-up). Deferred: P3#4 project agents naming their pack (projectUnion is shared with the runtime). --- docs/TOOLS.md | 2 +- packages/bus/README.md | 14 ++++-- packages/bus/src/broker.mjs | 44 +++++++++++++------ packages/bus/tests/pack-hire.test.mjs | 43 ++++++++++++++++++ packages/webui/README.md | 10 +++-- packages/webui/src/packs-http.mjs | 6 ++- packages/webui/src/public/packs.js | 8 +++- .../webui/tests/packs-hire-browser.test.mjs | 26 +++++++++-- packages/webui/tests/packs-http.test.mjs | 40 +++++++++++++++-- 9 files changed, 161 insertions(+), 32 deletions(-) diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 97e11ec92..0d7003d9f 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -346,7 +346,7 @@ For local development on a named instance only, `mosaic bus start BUSINESS --dev The hire form and the autonomy panel from a terminal (E2, #1873, P1-3). `mosaic org role-templates|hire|fire|change ORG` and `mosaic org autonomy ORG [set|review]` send the page's own request to the running Console's org route (`/api/orgs/ORG/decisions/hire/templates|start`, `/api/orgs/ORG/autonomy[/review]`), as `org reload` does, so the Console applies the same owner, `--send`, origin, body and live-session checks and the same bus verbs run (`hire.templates.console`, `hire.start.console`, `autonomy.get.console`, `autonomy.set.console`, `autonomy.review.console`). `hire` and `change` read the templates first and send the chosen template's name and sha256, as the form does. `autonomy set` needs the 64-hex `--revision` from `mosaic org autonomy ORG` and sends every setting the panel sends, with only the named ones changed; a stale revision is refused `revision-conflict`; an empty `--authority-resolver ""` clears the resolver, as emptying the panel's field does. These verbs need a named instance (`MOSAIC_CONFIG` set to `~/.config/mosaic-NAME/config.json`) whose Console runs with `--dev-local-owner`; the default instance is refused with exit 3. A Console refusal exits 3 with the Console's code and message; `outcome-unknown` exits 1; an unknown template exits 2. Every one of these verbs, reads included, refuses inside an agent run. `mosaic tasks create|edit|archive` are the task board's three writes: the arguments pass the board's own check (`workspace-tasks.mjs parseTaskWrite`) and go to the bus as the person (`tasks.create|edit|archive`, human transport), so the bus runs its human, field, digest and archive checks. A stale `--expect` is refused `task-conflict`; read the digest with `mosaic tasks --json`. -Starter packs (#1873): `mosaic org pack list`, `mosaic org pack preview ID PACK [--project P]`, `mosaic org pack apply ID PACK --digest D [--project P]` and `mosaic org pack hire ID PACK [--project P]` call the bus as the organization's human (`packs`, `pack.preview`, `pack.apply`, `pack.hire`). Ruling 2026-11-14 ("If I choose a starter pack and apply it, that is a decision!"): a person's apply is the hire decision. After the pack commits, the same act hires each proposed agent through the hire-decisions path and the receipt's `hires` lists `{name, state, decision | code}` per proposal (`hired`, `exists`, `open`, `awaiting-owner`, `refused`). The owner's apply approves the flagged and credential lines; a credential line is approved but not bound, so the agent's launch refuses `credential-unbound` until its services are bound. A member with `org.hire` (WebUI bus verb `pack.hire.console`) hires only the proposals with no flagged or credential line, and the rest wait for the owner in the inbox. An agent's apply hires nothing. `pack hire` reruns the loop on a pack already applied and hires none twice; it refuses `pack-not-applied` when the pack has no packs entry, and a proposal refuses `pack-hire-route-missing` when the chief of staff has no harness and model. All pack words refuse inside an agent run (exit 3). The WebUI pack card's "Hire the N proposed agents" is `POST /api/orgs//packs/hire`. Details: `packages/cli/README.md`, `packages/bus/README.md` and `packages/webui/README.md`, "Starter pack hires"; design: `docs/plans/2026-11-12_starter-packs-design.md` section 3. +Starter packs (#1873): `mosaic org pack list`, `mosaic org pack preview ID PACK [--project P]`, `mosaic org pack apply ID PACK --digest D [--project P]` and `mosaic org pack hire ID PACK [--project P]` call the bus as the organization's human (`packs`, `pack.preview`, `pack.apply`, `pack.hire`). Ruling 2026-11-14 ("If I choose a starter pack and apply it, that is a decision!"): a person's apply is the hire decision. After the pack commits, the same act hires each proposed agent through the hire-decisions path and the receipt's `hires` lists `{name, state, decision | code}` per proposal (`hired`, `exists`, `open`, `awaiting-owner`, `refused`). The owner's apply approves the flagged and credential lines; a credential line is approved but not bound, so the agent's launch refuses `credential-unbound` until its services are bound. The bus verb `pack.hire.console` also takes a member's console cap with `org.hire`: it hires only the proposals with no flagged or credential line, and the rest wait for the owner in the inbox. No member surface calls it yet: the WebUI packs routes and the CLI `pack` words are the owner's. An owner's pack hire also replaces and approves a hire this pack's own member hire left in the inbox; the old decision is withdrawn only after the new hire applies. Two pack hires on one organization at once: the second refuses `organization-busy`; an apply whose hire loop did not run still answers the apply's receipt with `hireError` and no hires. An agent's apply hires nothing. `pack hire` reruns the loop on a pack already applied and hires none twice; it refuses `pack-not-applied` when the pack has no packs entry, and a proposal refuses `pack-hire-route-missing` when the chief of staff has no harness and model. All pack words refuse inside an agent run (exit 3). The WebUI pack card's "Hire the N proposed agents" is `POST /api/orgs//packs/hire`. Details: `packages/cli/README.md`, `packages/bus/README.md` and `packages/webui/README.md`, "Starter pack hires"; design: `docs/plans/2026-11-12_starter-packs-design.md` section 3. The bus and Console lifecycle verbs refuse inside an agent run (agent-guard, #1873): `mosaic bus start`, `bus stop`, `bus restart` and `bus recover`, and `mosaic console start`, `console stop` and `console restart`, and `mosaic org archive` and `org delete`, which stop the organization's bus, and `mosaic org restore`, which starts it (`org restore --no-bus` starts nothing and is not refused), and `mosaic org role-templates`, `hire`, `fire`, `change` and `autonomy` (E2). Each exits 3 with `refused: this runs only from a human shell, outside any agent run (found MARKER in the environment)` before it reads anything. It checks the environment it is given and its own process environment, so a sanitized environment does not hide a marker. A usage error (a missing or extra argument, `--pm` with `--no-launch`, or a bad `--dev-insecure-bind`/`--dev-allow-host`) still comes first (exit 4). `mosaic bus status` and `mosaic console status` stay open to agents. The Console's Restart bus and Restart Console controls keep working: the Console starts its bus host with its own environment, which carries no marker, and Restart Console relaunches the Console under its supervisor without going through the CLI. The markers are the ones `packages/cli/src/transport.mjs` lists. The check is cooperative: a process that drops the markers from its environment is not stopped by it. diff --git a/packages/bus/README.md b/packages/bus/README.md index deff79d25..4322163e0 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -1154,11 +1154,17 @@ decision}`. The route is the chief of staff's resolved harness and model; with none the proposal refuses `pack-hire-route-missing`. The entry is written with no credential material, so a credential line leaves its services `unbound`. A proposal whose instance exists is `exists`; one an open hire decision names is -`open`, so nothing is hired twice. The owner approves every line, and withdraws -an open decision an earlier hire of the same pack left for the owner before -hiring the agent itself. A member's flagged or credentialed hires are raised after +`open`, so nothing is hired twice. The owner approves every line, and replaces +an open decision an earlier hire of the same pack left for the owner: the new +hire is raised and applied first, and the old decision is withdrawn only once it +has applied, so a replacement that refuses leaves the old one in the inbox. A member's flagged or credentialed hires are raised after the others, left open for the owner's inbox (`awaiting-owner`). A hire that refuses records `action.allowed {operation: 'pack.hire.refused', code}` and the loop goes on. The `packs` read adds `hired` and `open` to each applied entry, and the `instances` read adds `pack: {id, origin, project}` to each instance a -pack hire created. +pack hire created. An `awaiting-owner` line this pack's own hire raised carries +`fromPack: true` (the owner's pack hire would replace it). One pack hire loop +runs per business at a time; a second refuses `organization-busy`. An apply +whose hire loop throws after the apply committed still answers the apply's +receipt, with no hires and the loop's code as `hireError`. No member surface +calls `pack.hire.console` yet; the Console's packs routes are the owner's. diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 9054331ac..4ac5e8185 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -639,6 +639,9 @@ export class Broker { #builtinBinding; #organization; #hires; + // Businesses with a pack hire loop running: a second loop on the same business refuses + // organization-busy rather than raising a second decision for the same agent. + #packHiring = new Set(); #autonomyHost; #pluginService; #credentialVerbs; @@ -6360,9 +6363,13 @@ export class Broker { }; // Ruling 2026-11-14: a person's apply is the hire decision. Once the apply has committed, the // same call hires the proposals (#packHires); an agent's apply never hires. + // The apply has committed by then, so a hire loop that throws (organization-busy, a store error) + // still returns the apply's receipt, with no hires and the loop's code as hireError. if (verb === 'pack.apply' && !agent) { const hired = async (value) => { - const out = await this.#packHires(s, () => this.#packHireCaller(s), { pack: args.pack, project: args.project ?? null }); + let out; + try { out = await this.#packHires(s, () => this.#packHireCaller(s), { pack: args.pack, project: args.project ?? null }); } + catch (e) { out = { hires: [], hireError: e instanceof BusError && typeof e.code === 'string' ? e.code : 'outcome-unknown' }; } return done({ ...value, ...out }); }; return Promise.resolve(result).then(value => value, packFail).then(hired); @@ -6835,7 +6842,8 @@ export class Broker { for (const h of a.proposedHires) { if (Object.hasOwn(level, h.name)) continue; const decision = open.get(JSON.stringify([h.name, a.project ?? null])); - if (decision) { a.open.push({ name: h.name, reason: h.reason, state: 'awaiting-owner', decision }); continue; } + // fromPack: this pack's own hire raised it, so the owner's pack hire replaces and approves it. + if (decision) { a.open.push({ name: h.name, reason: h.reason, state: 'awaiting-owner', decision, ...(raisedBy.has(decision) && mine(raisedBy.get(decision)) ? { fromPack: true } : {}) }); continue; } const last = events.filter(e => mine(e) && e.hire === h.name).at(-1); if (last?.operation === 'pack.hire.refused') a.open.push({ name: h.name, reason: h.reason, state: 'refused', code: last.code }); else a.open.push({ name: h.name, reason: h.reason, state: 'proposed' }); @@ -6856,6 +6864,12 @@ export class Broker { // refuses records action.allowed pack.hire.refused with its code, and the loop goes on. async #packHires(s, callerOf, { pack, project = null }) { if (!this.#organization || !this.#hires) fail('not-configured'); + if (this.#packHiring.has(s.business)) fail('organization-busy'); + this.#packHiring.add(s.business); + try { return await this.#packHireLoop(s, callerOf, { pack, project }); } + finally { this.#packHiring.delete(s.business); } + } + async #packHireLoop(s, callerOf, { pack, project }) { const caller = callerOf(); const read = () => this.#organization(s.business, 'pack.state', { pack, project }, caller.actor, {}); const state = read(); @@ -6874,18 +6888,13 @@ export class Broker { if (state.names.includes(name)) { out.push({ name, state: 'exists' }); continue; } const openId = open.get(JSON.stringify([name, project])); // The owner's pack hire approves every line, so an open decision an earlier pack hire of this - // same pack left for the owner is withdrawn by the owner and replaced by one approved now. Its - // pin may be stale after the hires approved since. Any other open decision is left alone. + // same pack left for the owner is replaced by one approved now (its pin may be stale after the + // hires approved since). The old one is withdrawn only once the new hire has applied: a + // replacement that refuses leaves it open in the owner's inbox. Any other open decision is left alone. let supersedes = null; if (openId) { const by = raisedBy.get(openId); if (caller.owner !== true || !by || by.pack !== entry.id || (by.origin ?? null) !== (entry.origin ?? null) || (by.project ?? null) !== project) { out.push({ name, state: 'open', decision: openId }); continue; } - this.#store.transaction(() => { - if (this.#closed(openId)) return; - const seq = Number(this.#store.run('INSERT INTO decision_events(decision,at,op,by,choice,note,via) VALUES(?,?,?,?,?,?,?)', - openId, this.#now(), 'withdrawn', caller.actor, null, null, caller.via ?? 'console').lastInsertRowid); - this.#event(s, 'human.input', { kind: 'answer', decision: openId, op: 'withdrawn', choice: null, via: caller.via ?? 'console', subject: caller.via === 'cli' ? null : s.console, decisionSeq: seq }, openId); - }); supersedes = openId; } let input; @@ -6894,11 +6903,18 @@ export class Broker { input = { op: 'hire', level: project === null ? 'org' : 'project', ...(project === null ? {} : { project }), name, role: this.#pinRole(state.noHire, hire), route: structuredClone(state.route), reason: typeof hire.reason === 'string' && hire.reason.trim() ? hire.reason : `Proposed by starter pack ${entry.id}.` }; } catch (e) { refused(name, e); continue; } + let r; try { - const r = await this.#raiseHire(s, input, true, { caller: callerOf, pack: provenance, defer: caller.owner !== true }); - if (r?.deferred) { deferred.push(input); continue; } - out.push({ name, state: 'hired', decision: r.id, ...(supersedes ? { supersedes } : {}) }); - } catch (e) { refused(name, e); } + r = await this.#raiseHire(s, input, true, { caller: callerOf, pack: provenance, defer: caller.owner !== true }); + } catch (e) { refused(name, e); continue; } + if (r?.deferred) { deferred.push(input); continue; } + if (supersedes) this.#store.transaction(() => { + if (this.#closed(supersedes)) return; + const seq = Number(this.#store.run('INSERT INTO decision_events(decision,at,op,by,choice,note,via) VALUES(?,?,?,?,?,?,?)', + supersedes, this.#now(), 'withdrawn', caller.actor, null, null, caller.via ?? 'console').lastInsertRowid); + this.#event(s, 'human.input', { kind: 'answer', decision: supersedes, op: 'withdrawn', choice: null, via: caller.via ?? 'console', subject: caller.via === 'cli' ? null : s.console, decisionSeq: seq }, supersedes); + }); + out.push({ name, state: 'hired', decision: r.id, ...(supersedes ? { supersedes } : {}) }); } for (const input of deferred) { try { diff --git a/packages/bus/tests/pack-hire.test.mjs b/packages/bus/tests/pack-hire.test.mjs index 0fa0b72ad..63580b5f5 100644 --- a/packages/bus/tests/pack-hire.test.mjs +++ b/packages/bus/tests/pack-hire.test.mjs @@ -253,3 +253,46 @@ test('PD the new codes are in the refusal table with a rail and a next step that assert.ok(!/(^|[\s("'`])(\/|~\/)/.test(REFUSALS[c].nextStep)); } }); + +test('PD the owner supersede withdraws an inbox decision only after its replacement hires; a refused replacement leaves it open', async t => { + const h = await harness(t, { route: false }); + h.install(mixkit(h.rolesDir)); + await apply(h.call, { pack: 'mixkit' }); + const setRoute = (vars) => { const doc = h.file(); if (vars) doc.roles.chief.vars = { ...vars }; else delete doc.roles.chief.vars; writeFileSync(h.path, JSON.stringify(doc, null, 2) + '\n'); }; + setRoute(ROUTE); + const out = await h.call('pack.hire.console', { pack: 'mixkit' }, h.member); + const waiting = out.hires.filter(x => x.state === 'awaiting-owner'); + assert.equal(waiting.length, 2); + // The packs read marks each member-raised line as raised by a pack hire. + const open = (await h.call('packs.console')).applied.find(a => a.id === 'mixkit').open; + assert.deepEqual(open.map(o => [o.name, o.state, o.fromPack]), [['coder-1', 'awaiting-owner', true], ['reviewer-1', 'awaiting-owner', true]]); + // The chief loses its route; the owner's pack hire cannot build a replacement. + setRoute(null); + const owner = await h.call('pack.hire.console', { pack: 'mixkit' }); + assert.deepEqual(owner.hires.map(x => [x.name, x.state, x.code ?? null]), [['scout-1', 'exists', null], ['coder-1', 'refused', 'pack-hire-route-missing'], ['reviewer-1', 'refused', 'pack-hire-route-missing']]); + // Both inbox decisions are still open: nothing withdrew them. + for (const x of waiting) assert.deepEqual(h.store.all('SELECT op FROM decision_events WHERE decision=?', x.decision), []); + // With the route back, the owner's pack hire replaces them, withdrawing each only once its replacement applied. + setRoute(ROUTE); + const done = await h.call('pack.hire.console', { pack: 'mixkit' }); + assert.deepEqual(done.hires.map(x => [x.name, x.state, x.supersedes ?? null]), [['scout-1', 'exists', null], ['coder-1', 'hired', waiting[0].decision], ['reviewer-1', 'hired', waiting[1].decision]]); + for (const x of waiting) assert.deepEqual(h.store.all('SELECT op, by FROM decision_events WHERE decision=?', x.decision).map(r => [r.op, r.by]), [['withdrawn', 'jason']]); +}); + +test('PD a second pack hire on the same organization while one runs refuses organization-busy and raises nothing', async t => { + const h = await harness(t, { route: false }); + await apply(h.call); + const doc = h.file(); + doc.roles.chief.vars = { ...ROUTE }; + writeFileSync(h.path, JSON.stringify(doc, null, 2) + '\n'); + const first = h.call('pack.hire.console', { pack: 'coding' }); + const second = h.call('pack.hire.console', { pack: 'coding' }); + await assert.rejects(second, /organization-busy/); + const hired = await first; + assert.deepEqual(hired.hires.map(x => [x.name, x.state]), [['pm-1', 'hired'], ['coder-1', 'hired'], ['reviewer-1', 'hired']]); + // One decision per hire: the refused call raised nothing. + assert.equal(h.op('decision.raise').length, 3); + // Once the loop ends the latch is free again: the next pack hire runs and finds nothing to hire. + const later = await h.call('pack.hire.console', { pack: 'coding' }); + assert.ok(later.hires.every(x => x.state === 'exists'), JSON.stringify(later)); +}); diff --git a/packages/webui/README.md b/packages/webui/README.md index bbea41e9f..c874e5663 100644 --- a/packages/webui/README.md +++ b/packages/webui/README.md @@ -76,12 +76,15 @@ peers in dev mode is a recorded follow-up. "Damnit. If I choose a starter pack and apply it, that is a decision!" `POST /api/orgs//packs/apply` (`pack.apply.console`) hires the pack's proposed agents in the same act, and its receipt is `{revision, added, pack, -hires}`. `POST /api/orgs//packs/hire` with `{pack, project?}` +hires}`; when the apply committed but its hire loop did not run, `hires` is +empty and `hireError` names the code, and the card says the hires did not run. +`POST /api/orgs//packs/hire` with `{pack, project?}` (`pack.hire.console`) runs the same loop on a pack already applied and answers `{pack, hires}`; `pack-not-applied` is 404 and `right-required` 403. Each hire line is `{name, state, decision | code}`. `GET /api/orgs//packs` adds, to each applied entry, `hired` (the agents its hires created) and `open` (each -proposal not hired, with its reason, its state and its decision or code). The +proposal not hired, with its reason, its state and its decision or code, and +`fromPack: true` on an `awaiting-owner` line this pack's own hire raised). The packs routes stay the owner's: a member's pack hire is the bus verb's (`pack.hire.console` needs `org.hire`, and the flagged hires wait for the owner), but no member page calls it yet. @@ -89,7 +92,8 @@ owner), but no member page calls it yet. The pack card says what applying does before the click ("applying is the hire decision, and approves the flagged and credential lines below"), shows "N agents hired" and the open proposals with their reasons after it, and offers -"Hire the N proposed agents" for proposals not waiting for the owner. Apply, +"Hire the N proposed agents" for proposals not waiting for the owner, and for +the owner's own `fromPack` lines, which the owner's hire replaces and approves. Apply, hire and fire dispatch `mosaic-org-changed`, so the Agents page and the organization page redraw in the same cycle instead of at the next 10-second poll. Each agent card a pack hire made says "from pack X". The owner's Agents diff --git a/packages/webui/src/packs-http.mjs b/packages/webui/src/packs-http.mjs index 995ea260f..0d3a746bf 100644 --- a/packages/webui/src/packs-http.mjs +++ b/packages/webui/src/packs-http.mjs @@ -130,7 +130,7 @@ export async function packsHTTP(req, res, { path, owner, read, write, workflows if (!Array.isArray(result.packs)) return refuse(503, 'outcome-unknown'); const applied = Array.isArray(result.applied) ? result.applied.filter(plain).map(a => ({ id: a.id, origin: a.origin ?? null, project: a.project ?? null, hired: Array.isArray(a.hired) ? a.hired.filter(n => typeof n === 'string') : [], - open: Array.isArray(a.open) ? a.open.filter(plain).map(o => ({ name: o.name, reason: typeof o.reason === 'string' ? o.reason : '', state: o.state, ...(typeof o.code === 'string' ? { code: o.code } : {}), ...(typeof o.decision === 'string' ? { decision: o.decision } : {}) })) : [] })) : []; + open: Array.isArray(a.open) ? a.open.filter(plain).map(o => ({ name: o.name, reason: typeof o.reason === 'string' ? o.reason : '', state: o.state, ...(typeof o.code === 'string' ? { code: o.code } : {}), ...(typeof o.decision === 'string' ? { decision: o.decision } : {}), ...(o.fromPack === true ? { fromPack: true } : {}) })) : [] })) : []; return json(res, 200, { packs: result.packs, applied, writable: Boolean(write) }); } if (step === 'preview') { @@ -146,7 +146,9 @@ export async function packsHTTP(req, res, { path, owner, read, write, workflows return json(res, 200, { pack: result.pack, hires }); } if (!REVISION.test(result.revision ?? '')) return refuse(503, 'outcome-unknown'); - return json(res, 200, { revision: result.revision, added: Array.isArray(result.added) ? result.added : [], ...(step === 'apply' ? { pack: result.pack, hires: hires ?? [] } : {}) }); + // hireError: the apply committed but its hire loop did not run (organization-busy, outcome-unknown). + const hireError = step === 'apply' && typeof result.hireError === 'string' && /^[a-z0-9-]{1,64}$/.test(result.hireError) ? { hireError: result.hireError } : {}; + return json(res, 200, { revision: result.revision, added: Array.isArray(result.added) ? result.added : [], ...(step === 'apply' ? { pack: result.pack, hires: hires ?? [], ...hireError } : {}) }); } // SP6: the workflow list and editor's read. Only workflows, the names a step role may resolve to and the diff --git a/packages/webui/src/public/packs.js b/packages/webui/src/public/packs.js index 58d74f4c2..1287340f5 100644 --- a/packages/webui/src/public/packs.js +++ b/packages/webui/src/public/packs.js @@ -40,6 +40,8 @@ 'definition-not-hireable': 'its role is on the no-hire list', 'starter-pack-proposal-stale': 'the pack changed since it was applied; apply it again', 'human-required': 'only a person can hire', + 'organization-busy': 'another pack hire on this organization was running', + 'outcome-unknown': 'the bus did not say why', }; const whyText = code => HIRE_WHY[code] ?? code; const OPEN_STATE = { 'awaiting-owner': 'waiting for the owner in the inbox', proposed: 'proposed, not hired yet' }; @@ -120,7 +122,9 @@ const hired = `

${n === 0 ? 'No agents hired yet' : `${n} ${n === 1 ? 'agent' : 'agents'} hired`}${where}${n ? `: ${a.hired.map(x => `${esc(x)}`).join(', ')}` : ''}

`; const open = a.open.filter(plainObj); const lines = open.length ? `
    ${open.map(o => `
  • ${esc(o.name)}: ${esc(o.reason || 'No reason given.')} ${esc(o.state === 'refused' ? `not hired: ${whyText(o.code)}` : OPEN_STATE[o.state] ?? o.state)}
    • `).join('')}
` : ''; - const hireable = open.filter(o => o.state !== 'awaiting-owner').length; + // The page is the owner's, and the owner's pack hire also approves a line this pack's own hire + // left in the inbox (fromPack). A decision raised some other way stays the inbox's. + const hireable = open.filter(o => o.state !== 'awaiting-owner' || o.fromPack === true).length; const button = hireable && s.writable ? `` : ''; return `
${hired}${lines}${button}
`; }).join(''); @@ -174,7 +178,7 @@ const r = await call(`${base(org)}/packs/apply`, { pack: p.pack, digest: p.value.digest, ...(p.project ? { project: p.project } : {}) }); const added = Array.isArray(r.added) && r.added.length ? r.added.join(', ') : 'nothing new'; s.preview = null; - s.status = `Applied. Added ${added}. ${hireSummary(r.hires)}`; + s.status = `Applied. Added ${added}. ${typeof r.hireError === 'string' ? `The hires did not run: ${whyText(r.hireError)}. Use Hire on the pack card to run them.` : hireSummary(r.hires)}`; applied = true; } } catch (error) { s.status = error.message; } diff --git a/packages/webui/tests/packs-hire-browser.test.mjs b/packages/webui/tests/packs-hire-browser.test.mjs index a96086643..69a57e67d 100644 --- a/packages/webui/tests/packs-hire-browser.test.mjs +++ b/packages/webui/tests/packs-hire-browser.test.mjs @@ -29,7 +29,9 @@ const PREVIEW = { skills: { notEnabled: [] }, chief: true, plan: {}, digest: REV('d'), }; -async function fixture(t) { +// busy: the bus's apply committed but its hire loop did not run (hireError), and the packs read lists one line a +// member's pack hire left in the owner's inbox (fromPack), one raised some other way and one refused. +async function fixture(t, { busy = false } = {}) { const verbs = [], decisions = []; const hired = new Map(); // name -> its pack provenance, as the bus's instances read reports it let applied = null, route = false, instanceReads = 0; @@ -42,7 +44,8 @@ async function fixture(t) { return { name: h.name, state: 'hired', decision }; }); const listed = () => applied ? [{ id: 'software-team', origin: 'shipped', project: null, hired: [...hired.keys()], - open: PREVIEW.hires.filter(h => !hired.has(h.name)).map(h => ({ name: h.name, reason: `Proposed by starter pack software-team for ${h.definition} work.`, state: 'refused', code: 'pack-hire-route-missing' })) }] : []; + open: PREVIEW.hires.filter(h => !hired.has(h.name)).map(h => ({ name: h.name, reason: `Proposed by starter pack software-team for ${h.definition} work.`, + ...(!busy || h.name === 'scout-1' ? { state: 'refused', code: 'pack-hire-route-missing' } : { state: 'awaiting-owner', decision: `inbox-${h.name}`, ...(h.name === 'coder-1' ? { fromPack: true } : {}) }) })) }] : []; const read = async (verb, args) => { verbs.push([verb, args]); if (verb === 'packs.console') return { packs: PACKS, applied: listed() }; @@ -52,7 +55,7 @@ async function fixture(t) { }; const write = async (verb, args) => { verbs.push([verb, args]); - if (verb === 'pack.apply.console') { applied = true; return { revision: REV('f'), added: ['board'], pack: { id: args.pack, version: '1.0.0' }, hires: hireLoop() }; } + if (verb === 'pack.apply.console') { applied = true; return { revision: REV('f'), added: ['board'], pack: { id: args.pack, version: '1.0.0' }, ...(busy ? { hires: [], hireError: 'organization-busy' } : { hires: hireLoop() }) }; } if (verb === 'pack.hire.console') return { pack: args.pack, hires: hireLoop() }; throw Object.assign(new Error('not-configured'), { code: 'not-configured' }); }; @@ -164,3 +167,20 @@ test('PD apply hires the proposals, the pack card counts them, Hire the N propos assert.equal(await b.evaluate(`document.querySelectorAll('[data-org-agents] [data-agent-card]').length`), 4); assert.equal(await b.evaluate(`document.querySelector('[data-agents-receipt]').textContent`), 'Fired reviewer-1. Recorded decision fire-1.'); }); + +test('PD an apply whose hire loop did not run says so; the owner card counts a pack-raised inbox line as hireable and leaves the inbox its own', { timeout: 90000 }, async t => { + const f = await fixture(t, { busy: true }); + const { b } = f; + await b.navigate(`http://127.0.0.1:${f.port}/#/orgs/acme/organization`); + await waitFor(b, `document.querySelector('[data-pack="software-team"] [data-pack-preview-button]')`, 'pack list'); + await b.evaluate(`document.querySelector('[data-pack="software-team"] [data-pack-preview-button]').click();true`); + await waitFor(b, `document.querySelector('[data-pack-apply]')`, 'preview'); + await b.evaluate(`document.querySelector('[data-pack-apply]').click();true`); + await waitFor(b, `/^Applied\\./.test(document.querySelector('[data-packs-status]')?.textContent??'')`, 'apply status'); + assert.equal(await b.evaluate(`document.querySelector('[data-packs-status]').textContent`), 'Applied. Added board. The hires did not run: another pack hire on this organization was running. Use Hire on the pack card to run them.'); + await waitFor(b, `document.querySelector('[data-pack="software-team"] [data-pack-open]')`, 'open lines'); + assert.deepEqual(await b.evaluate(`[...document.querySelectorAll('[data-pack-open-hire]')].map(l=>[l.dataset.packOpenHire,l.dataset.packOpenState])`), + [['coder-1', 'awaiting-owner'], ['reviewer-1', 'awaiting-owner'], ['scout-1', 'refused']]); + // coder-1 was left in the inbox by a pack hire, so the owner's pack hire approves it; reviewer-1 is the inbox's. + assert.equal(await b.evaluate(`document.querySelector('[data-pack-hire-button]').textContent`), 'Hire the 2 proposed agents'); +}); diff --git a/packages/webui/tests/packs-http.test.mjs b/packages/webui/tests/packs-http.test.mjs index 551c91621..2c622b929 100644 --- a/packages/webui/tests/packs-http.test.mjs +++ b/packages/webui/tests/packs-http.test.mjs @@ -25,7 +25,7 @@ import { close } from './fixture.mjs'; const FLOW = (title) => ({ description: `${title}.`, steps: [{ title, state: 'todo', roles: ['human'] }] }); const BOARD = { columns: [{ state: 'todo', label: 'Backlog' }, { state: 'in-progress', label: 'Building' }, { state: 'in-review', label: 'Review' }, { state: 'blocked', label: 'Blocked' }, { state: 'done', label: 'Merged' }], layout: 'kanban' }; -async function fixture(t, { send = true, workflows = true, route = false } = {}) { +async function fixture(t, { send = true, workflows = true, route = false, tap = null } = {}) { const undo = []; t.after(async () => { for (const step of undo.reverse()) await step(); }); const root = mkdtempSync(join(tmpdir(), 'pk-')); const rolesDir = rolesCopy(root), system = systemFor(root); @@ -57,7 +57,9 @@ async function fixture(t, { send = true, workflows = true, route = false } = {}) const upstream = `http://127.0.0.1:${board.address().port}`; const consoleCall = kind => async (verb, args, ctx) => { calls.push([kind, verb, Object.keys(args).sort()]); return new Client({ path, console: { subject: ctx.subject } }).call(verb, args); }; const requestRead = consoleCall('read'); - const packsWrite = packsCaller(consoleCall('write')); + // tap: a test's view of each write's bus result before the route shapes it. + const writer = packsCaller(consoleCall('write')); + const packsWrite = tap ? async (verb, args, ctx) => tap(verb, await writer(verb, args, ctx)) : writer; const session = { provider: 'fixture', iss: 'https://issuer.example', sub: 'owner', name: 'jason', rootOwner: true }; let active = true; const sso = { publicOrigin: '', publicHost: '', formTargets: () => [], session: () => active ? session : null, handle: () => {}, health: () => ({}) }; @@ -71,7 +73,7 @@ async function fixture(t, { send = true, workflows = true, route = false } = {}) sso.publicOrigin = base; sso.publicHost = new URL(base).host; undo.push(() => close(web)); const onDisk = () => JSON.parse(readFileSync(file, 'utf8')); - return { base, file, root, calls, onDisk, flowReads, boardHits, failFlows: () => { flowFails = true; }, signOut: () => { active = false; }, revokeOwner: () => { session.rootOwner = false; session.user = 'legacy-user'; } }; + return { base, file, root, calls, onDisk, broker: runtime.broker, flowReads, boardHits, failFlows: () => { flowFails = true; }, signOut: () => { active = false; }, revokeOwner: () => { session.rootOwner = false; session.user = 'legacy-user'; } }; } function call(f, path, method = 'GET', body, headers = {}) { @@ -412,3 +414,35 @@ test('PD with no chief route the apply records the pack and each hire says pack- assert.deepEqual(list.applied[0].hired, []); assert.deepEqual(list.applied[0].open.map(o => [o.name, o.state, o.code, typeof o.reason]), [['pm-1', 'refused', 'pack-hire-route-missing', 'string'], ['coder-1', 'refused', 'pack-hire-route-missing', 'string'], ['reviewer-1', 'refused', 'pack-hire-route-missing', 'string']]); }); + +test('PD a member-raised awaiting-owner line reads fromPack; the owner hire route approves it; an apply whose hire loop did not run says hireError', async t => { + const f = await fixture(t); + const [, preview] = await call(f, `${ORG}/packs/preview`, 'POST', { pack: 'coding' }); + await call(f, `${ORG}/packs/apply`, 'POST', { pack: 'coding', digest: preview.digest }); + const doc = f.onDisk(); + doc.roles.chief.vars = { harness: 'pi', model: 'claude-opus-5-5' }; + writeFileSync(f.file, JSON.stringify(doc, null, 2) + '\n'); + // A member with org.hire runs the pack hire on the bus (no WebUI route serves a member yet). + const member = f.broker.bindHireConsole({ business: 'demo', actor: 'alice', subject: { provider: 'test', iss: 'test', sub: 'alice' }, rights: ['org.hire', 'board.read'] }); + const out = await f.broker.request(member, { verb: 'pack.hire.console', args: { pack: 'coding' } }); + const waiting = out.hires.filter(x => x.state === 'awaiting-owner').map(x => x.name); + assert.ok(waiting.length > 0, JSON.stringify(out)); + const [, list] = await call(f, `${ORG}/packs`); + const open = list.applied[0].open.filter(o => o.state === 'awaiting-owner'); + assert.deepEqual(open.map(o => [o.name, o.fromPack]), waiting.map(n => [n, true])); + const [status, hired] = await call(f, `${ORG}/packs/hire`, 'POST', { pack: 'coding' }); + assert.equal(status, 200, JSON.stringify(hired)); + assert.deepEqual(hired.hires.filter(x => waiting.includes(x.name)).map(x => [x.name, x.state]), waiting.map(n => [n, 'hired'])); + assert.deepEqual((await call(f, `${ORG}/packs`))[1].applied[0].open, []); + + // The bus answers an apply whose hire loop did not run with hireError; the route passes a code through + // and drops anything that is not one. + for (const [given, shown] of [['organization-busy', 'organization-busy'], ['Not A Code!', undefined], [7, undefined]]) { + const g = await fixture(t, { tap: (verb, result) => verb === 'pack.apply.console' ? { ...result, hires: [], hireError: given } : result }); + const [, p] = await call(g, `${ORG}/packs/preview`, 'POST', { pack: 'coding' }); + const [code, receipt] = await call(g, `${ORG}/packs/apply`, 'POST', { pack: 'coding', digest: p.digest }); + assert.equal(code, 200, JSON.stringify(receipt)); + assert.equal(receipt.hireError, shown); + assert.deepEqual(receipt.hires, []); + } +});