diff --git a/docs/TOOLS.md b/docs/TOOLS.md index fc04a5879..9ef3406de 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -348,6 +348,8 @@ The hire form and the autonomy panel from a terminal (E2, #1873, P1-3). `mosaic 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. +Refusal lines (E2E6, #1873). A refusal that has a next step prints it on a second line, `mosaic: next: ...`, after the unchanged message, so callers that match the message keep working. `mosaic bus status`, `start`, `stop` and `restart` on a stale bus host lock exit 3 with `mosaic: stale bus host lock ...` and `mosaic: next: if the bus host died, clear it with mosaic bus recover ORG; recover refuses while the host still runs`. A human-transport verb whose bus is not running (no socket, or the connect refused before anything was sent) exits 3 with `bus-not-running: the bus for ORG is not running, so the request was not sent` and a next line naming `mosaic bus start ORG` and `mosaic bus recover ORG`; `outcome-unknown` (exit 1) stays for a request that left and got no answer. `mosaic org definitions` with an unknown organization, an unknown or malformed name, or a bad id prints one refusal line (exit 3; `unknown-organization` only when the organization file itself is missing, `definitions-unreadable` when `templates` cannot read the roles directory) or, for a bad id, the usage (exit 4), with no stack trace and no host path. Pack, board and workflow refusals carry their next step as `template save` does. `mosaic console start` and `console stop` clear a `console/start.lock` whose named supervisor (pid and start time) is gone, once, with a `cleared a console start lock left by process PID, which is gone` warning; a lock whose holder still runs, or that names no holder, refuses and stays. A start or stop releasing its lock marks it released through its own descriptor and removes the path only while it is still that file, so a lock put back after its release is cleared the same way, with a `cleared a console start lock its holder had released` warning. An `autonomy-precondition` refusal from the bus carries the check that failed (owner notices off, no tested reverse, a resolver without the right, no `review.verdict` holder, no enforced budget, out of the loop only from near the loop, the trail review or near-loop decision missing); the human-transport child writes only the code, so a terminal `org autonomy set` shows the Console's message. `mosaic org template save` writes free text verbatim: it never rewrites prose, and the preview lists every field that mentions the organization id, a project id, an agent name or the owner's account, with the names it mentions, for the owner to edit before sharing. Saving under a name already saved for the account refuses `template-exists` (exit 2, nothing written) with a next line to save under another name or list the saved ones with `mosaic org template list ORG`. + The instance processing switch is separate from per-business launches off/on. `mosaic halt` durably stops managed processing across every business in the selected configuration's data root. The bus stays up for reads and human diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index b0f489190..dae7ff69b 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -233,7 +233,7 @@ const TEMPLATE_KEYS = Object.freeze({ export const TEMPLATE_BUS_KEYS = Object.freeze(Object.fromEntries(Object.entries(TEMPLATE_KEYS).flatMap(([verb, k]) => [[verb, k], [`${verb}.console`, k]]))); // The codes a template refusal keeps, each with its reason: a field path or a checked name, never a // value from the organization file and never a host path. -const TEMPLATE_CODES = Object.freeze(['template-secret-suspected', 'invalid-request', 'human-required', 'not-configured', 'pack-plan-stale', 'starter-pack-invalid', 'starter-pack-template-changed']); +const TEMPLATE_CODES = Object.freeze(['template-secret-suspected', 'template-exists', 'invalid-request', 'human-required', 'not-configured', 'pack-plan-stale', 'starter-pack-invalid', 'starter-pack-template-changed']); // Row 175: the organization verbs a tester may call. Its writes run every check the human's do // and log actor `tester`; row 176 adds routes reads; limits.set, organization.set and organization.revision refuse it. export const TESTER_ORGANIZATION_VERBS = Object.freeze(['limits', 'plugins', 'instances', 'plugins.set', 'instances.set', 'routes.set', 'routes']); @@ -289,9 +289,12 @@ export class BusError extends Error { if (reason !== undefined) this.reason = reason; } } -const fail = (code) => { - throw new BusError(code); +// E2E6 fix 7: a refusal may carry a reason, static text that names which check failed; it never +// quotes an argument or file bytes. +const fail = (code, reason) => { + throw new BusError(code, reason); }; +const BUDGET_UNCOVERED = 'no enforced budget covers new agents; authority away from the loop needs one'; const object = (x) => x !== null && typeof x === 'object' && !Array.isArray(x); // #1873 CB5: a copy binding proposal leaves the bus only as its copy and its change. The copy id, // incarnation, definition pin, services, grant ids and slots stay in the raise event, which the @@ -6262,7 +6265,7 @@ export class Broker { if (CREDENTIAL_VERBS.includes(request.verb)) return this.#requestCredential(s, request.verb, structuredClone(args)); if(['autonomy.get.console','autonomy.set.console','autonomy.review.console','decision.reverse.console'].includes(request.verb)) { this.#autonomyOwner(s); - return this.#autonomyConsole(s,request.verb,structuredClone(args)).catch(e=>{throw this.#refused(s,e instanceof BusError?e:new BusError(e.code??'outcome-unknown'),request);}); + return this.#autonomyConsole(s,request.verb,structuredClone(args)).catch(e=>{throw this.#refused(s,e instanceof BusError?e:new BusError(e.code??'outcome-unknown',e instanceof BusError?e.reason:undefined),request);}); } if (s.hireMembership && !['hire.preview.console','hire.resolve.console','hire.start.console','hire.templates.console'].includes(request.verb)) fail('human-required'); if(request.verb==='decision.resolve' && !s.human) { @@ -6271,7 +6274,7 @@ export class Broker { } if(request.verb==='decision.resolve' && !s.human && ['role.hire','role.fire','role.change'].includes(this.#decision(s,args.id).action)) { const d=this.#decision(s,args.id); - if(d.route_to!=='human')return this.#resolveAuthority(s,args).catch(e=>{throw this.#refused(s,e instanceof BusError?e:new BusError(e.code??'outcome-unknown'),request);}); + if(d.route_to!=='human')return this.#resolveAuthority(s,args).catch(e=>{throw this.#refused(s,e instanceof BusError?e:new BusError(e.code??'outcome-unknown',e instanceof BusError?e.reason:undefined),request);}); } if(request.verb==='hire.templates.console') { keys(args,[]);const caller=this.#hireCaller(s); @@ -6285,12 +6288,12 @@ export class Broker { const input=request.verb==='decision.raise' ? (()=>{ keys(args,['action','hire'],['action','hire']); if(args.action!==`role.${args.hire?.op}`)fail('invalid-request'); return args.hire; })() : args; // #1873 SP5: a raise from a pack's recorded proposal; the pin, not the caller, names the role. if(request.verb==='hire.raise' && object(args) && Object.hasOwn(args,'proposal')) - return Promise.resolve().then(()=>this.#raiseHire(s,this.#proposalHire(s,args),false)).catch(e=>{throw this.#refused(s,new BusError(e.code??'invalid-request'),request);}); - return this.#raiseHire(s,input,request.verb==='hire.start.console').catch(e=>{throw this.#refused(s,new BusError(e.code??'invalid-request'),request);}); + return Promise.resolve().then(()=>this.#raiseHire(s,this.#proposalHire(s,args),false)).catch(e=>{throw this.#refused(s,new BusError(e.code??'invalid-request',e instanceof BusError?e.reason:undefined),request);}); + return this.#raiseHire(s,input,request.verb==='hire.start.console').catch(e=>{throw this.#refused(s,new BusError(e.code??'invalid-request',e instanceof BusError?e.reason:undefined),request);}); } - if(request.verb==='decision.preview.console' && ['role.hire','role.fire','role.change'].includes(this.#decision(s,args.id).action))return this.#hireConsole(s,'hire.preview.console',args).catch(e=>{throw this.#refused(s,new BusError(e.code??'outcome-unknown'),request);}); + if(request.verb==='decision.preview.console' && ['role.hire','role.fire','role.change'].includes(this.#decision(s,args.id).action))return this.#hireConsole(s,'hire.preview.console',args).catch(e=>{throw this.#refused(s,new BusError(e.code??'outcome-unknown',e instanceof BusError?e.reason:undefined),request);}); if (['hire.preview.console','hire.resolve.console'].includes(request.verb)) - return this.#hireConsole(s,request.verb,args).catch(e=>{throw this.#refused(s,new BusError(e.code??'outcome-unknown'),request);}); + return this.#hireConsole(s,request.verb,args).catch(e=>{throw this.#refused(s,new BusError(e.code??'outcome-unknown',e instanceof BusError?e.reason:undefined),request);}); if (request.verb === 'grants.scope' || request.verb === 'grants.scope.set') return this.#grantScopeVerb(s, request.verb, structuredClone(args)); // #1873 SP4: the owner's starter packs, board and workflows. Only the organization's owner calls @@ -6628,7 +6631,7 @@ export class Broker { if(!this.#autonomyHost?.get || !this.#autonomyHost?.set)fail('not-configured'); const old=await this.#autonomyHost.get(s.business),b=this.#business(s.business); if(a.revision!==old.revision)fail('revision-conflict'); - if(AUTONOMY_CLASSES.some(cls=>decisionAutonomy(cls,a.autonomy)==='near the loop') && a.autonomy.notices!==true)fail('autonomy-precondition'); + if(AUTONOMY_CLASSES.some(cls=>decisionAutonomy(cls,a.autonomy)==='near the loop') && a.autonomy.notices!==true)fail('autonomy-precondition','owner notices are off; a class near the loop needs them on'); for(const cls of AUTONOMY_CLASSES) { const before=decisionAutonomy(cls,old.autonomy),after=decisionAutonomy(cls,a.autonomy); // Pulling back is unconditional. Only a move away from the loop has prerequisites. @@ -6637,22 +6640,24 @@ export class Broker { if(cls==='policy')continue; if(cls==='authority' && after==='out of the loop' && before!==after)fail('authority-limits-unavailable'); if(['in the loop','near the loop','out of the loop'].indexOf(after)<=['in the loop','near the loop','out of the loop'].indexOf(before))continue; - if(a.autonomy.notices!==true || !this.#autonomyHost.reversible?.(cls,b))fail('autonomy-precondition'); + if(a.autonomy.notices!==true)fail('autonomy-precondition',`owner notices are off; moving ${cls} away from the loop needs them on`); + if(!this.#autonomyHost.reversible?.(cls,b))fail('autonomy-precondition',`${cls} has no tested reverse in this organization, so it stays in the loop`); const resolvers=cls==='authority'?[a.autonomy.authorityResolver]:cls==='merge'?[b.arbiters.technical]:cls==='policy'?[b.arbiters.delivery]:Object.values(b.arbiters); for(const name of resolvers) { const role=b.roles[name],right=cls==='authority'?'decision.resolve.authority':`decision.resolve.${cls==='merge'?'technical':cls==='policy'?'delivery':name===b.arbiters.technical?'technical':'delivery'}`; - if(!role?.authority?.withinRole.includes(right) || (cls==='authority' && name===b.launch?.by))fail('autonomy-precondition'); + if(!role?.authority?.withinRole.includes(right))fail('autonomy-precondition',cls==='authority'?'the authority resolver is not a role with decision.resolve.authority':`the ${cls} resolver is not a role with ${right}`); + if(cls==='authority' && name===b.launch?.by)fail('autonomy-precondition','the authority resolver is the role that launches agents; name a separate resolver'); } - if(['merge','policy'].includes(cls) && !Object.values(b.roles).some(r=>r.authority?.withinRole.includes('review.verdict')))fail('autonomy-precondition'); - if(cls==='authority' && !this.#budgetCovered(b))fail('autonomy-precondition'); + if(['merge','policy'].includes(cls) && !Object.values(b.roles).some(r=>r.authority?.withinRole.includes('review.verdict')))fail('autonomy-precondition',`no role holds review.verdict, which ${cls} away from the loop needs`); + if(cls==='authority' && !this.#budgetCovered(b))fail('autonomy-precondition',BUDGET_UNCOVERED); if(after==='out of the loop') { - if(before!=='near the loop')fail('autonomy-precondition'); + if(before!=='near the loop')fail('autonomy-precondition',`${cls} moves out of the loop only from near the loop`); const review=this.#store.get("SELECT seq,body FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.operation')='autonomy.review' AND json_extract(body,'$.decisionClass')=? ORDER BY seq DESC LIMIT 1",s.business,cls); const latest=this.#store.get("SELECT seq FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.operation')='autonomy.set' ORDER BY seq DESC LIMIT 1",s.business); - if(!review || JSON.parse(review.body).settingSeq!==latest?.seq)fail('autonomy-precondition'); + if(!review || JSON.parse(review.body).settingSeq!==latest?.seq)fail('autonomy-precondition',`${cls} needs your review of its trail since the last autonomy change`); const notice=this.#store.get("SELECT seq FROM events WHERE business=? AND kind='action.allowed' AND json_extract(body,'$.operation')='autonomy.notice' AND json_extract(body,'$.decisionClass')=? AND seq>? ORDER BY seq DESC LIMIT 1",s.business,cls,latest?.seq??0); const reversal=this.#store.get("SELECT max(e.seq) seq FROM events e WHERE e.business=? AND e.kind='action.allowed' AND json_extract(e.body,'$.operation') LIKE 'autonomy.reverse%' AND EXISTS(SELECT 1 FROM events n WHERE n.business=e.business AND n.kind='action.allowed' AND json_extract(n.body,'$.operation')='autonomy.notice' AND json_extract(n.body,'$.decisionClass')=? AND json_extract(n.body,'$.decision')=json_extract(e.body,'$.decision'))",s.business,cls); - if(!notice || review.seq<=Math.max(notice.seq,reversal?.seq??0))fail('autonomy-precondition'); + if(!notice || review.seq<=Math.max(notice.seq,reversal?.seq??0))fail('autonomy-precondition',`${cls} needs a near-loop decision since the last autonomy change, and your trail review after it`); } } this.#store.transaction(()=>this.#event(s,'action.allowed',{operation:'autonomy.set.requested',autonomy:a.autonomy,subject:s.console},null)); @@ -6785,9 +6790,9 @@ export class Broker { // A binding always goes to the owner; no autonomy level moves it (design section 4). if(!immediate && !p.binding && level!=='in the loop' && p.level!=='root' && !p.flagged.length && !p.credentialLines.length) { if(level==='out of the loop')fail('authority-limits-unavailable'); - if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition'); + if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition',BUDGET_UNCOVERED); route=b.autonomy?.authorityResolver; - if(!b.roles[route]?.authority?.withinRole.includes('decision.resolve.authority'))fail('autonomy-precondition'); + if(!b.roles[route]?.authority?.withinRole.includes('decision.resolve.authority'))fail('autonomy-precondition','the authority resolver is not a role with decision.resolve.authority'); if(route===s.role || resolverSubject(b,route,p.name))route='human'; } this.#store.transaction(()=>{ @@ -6818,7 +6823,7 @@ export class Broker { if(this.#expireHire(s,d,p))fail('decision-expired'); if(this.#autonomyLevel(d)==='in the loop')fail('hire-resolution-required'); if(this.#autonomyLevel(d)==='out of the loop')fail('authority-limits-unavailable'); - if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition'); + if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition',BUDGET_UNCOVERED); if(!this.#business(s.business).roles[s.role]?.authority?.withinRole.includes('decision.resolve.authority'))fail('not-resolver'); }; validate(); @@ -6838,7 +6843,7 @@ export class Broker { const recheck=()=>{ this.#agent(s); if(this.#autonomyLevel(d)==='out of the loop')fail('authority-limits-unavailable'); - if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition'); + if(!this.#budgetCovered(this.#business(s.business)))fail('autonomy-precondition',BUDGET_UNCOVERED); if(this.#autonomyLevel(d)==='in the loop' || !this.#business(s.business).roles[s.role]?.authority?.withinRole.includes('decision.resolve.authority') || resolverSubject(this.#business(s.business),s.role,p.name))fail('not-resolver'); }; recheck(); diff --git a/packages/bus/src/client.mjs b/packages/bus/src/client.mjs index 80b5ce29d..d845b535e 100644 --- a/packages/bus/src/client.mjs +++ b/packages/bus/src/client.mjs @@ -1,5 +1,11 @@ import { connect } from 'node:net'; import { BusError, PROJECT_SAVE_VERB, PROJECT_SAVE_REQUEST_MAX } from './broker.mjs'; +// E2E6 fix 6: a connect that fails with one of these never sent the request. The code stays +// outcome-unknown for every caller that reads it (the harness, the Console), and the error is marked +// notSent so the human and tester shims can exit with NOT_SENT_EXIT and `mosaic` can say the bus is +// not running. +const NOT_SENT = new Set(['ENOENT', 'ECONNREFUSED', 'ENOTDIR', 'ENOTSOCK']); +export const NOT_SENT_EXIT = 5; export class Client { constructor({ path, cap, human, tester, user, console: owner, timeout = 5000 }) { this.path = path; @@ -22,7 +28,8 @@ export class Client { return new Promise((resolve, reject) => { const socket = connect(this.path); let input = Buffer.alloc(0), - settled = false; + settled = false, + connected = false; const end = (error, value) => { if (settled) return; settled = true; @@ -30,8 +37,12 @@ export class Client { error ? reject(error) : resolve(value); }; socket.setTimeout(['processing.halt', 'processing.resume'].includes(verb) ? Math.max(this.timeout, 20000) : this.timeout, () => end(new BusError('outcome-unknown'))); - socket.on('connect', () => socket.write(request)); - socket.on('error', () => end(new BusError('outcome-unknown'))); + socket.on('connect', () => { connected = true; socket.write(request); }); + socket.on('error', (error) => { + const unknown = new BusError('outcome-unknown'); + if (!connected && NOT_SENT.has(error?.code)) unknown.notSent = true; + end(unknown); + }); socket.on('end', () => end(new BusError('outcome-unknown'))); socket.on('data', (b) => { input = Buffer.concat([input, b]); diff --git a/packages/bus/src/human-cli.mjs b/packages/bus/src/human-cli.mjs index 0855456be..00f24a4df 100644 --- a/packages/bus/src/human-cli.mjs +++ b/packages/bus/src/human-cli.mjs @@ -3,7 +3,7 @@ import { randomBytes } from 'node:crypto'; import { spawnSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { readFileSync } from 'node:fs'; -import { Client } from './client.mjs'; +import { Client, NOT_SENT_EXIT } from './client.mjs'; import { readProcess } from './human.mjs'; const file = fileURLToPath(import.meta.url); if (!process.env.MOSAIC_BUS_CLI_NONCE) { @@ -39,5 +39,7 @@ try { process.stdout.write(JSON.stringify(result) + '\n'); } catch (e) { process.stderr.write((/^[a-z-]{1,64}$/.test(e.code ?? '') ? e.code : 'invalid-request') + '\n'); - process.exitCode = 2; + // E2E6 fix 6: a request that never left (no socket, or the connect refused) keeps its code and + // exits NOT_SENT_EXIT, so `mosaic` can say the bus is not running. Every other failure exits 2. + process.exitCode = e?.notSent === true ? NOT_SENT_EXIT : 2; } diff --git a/packages/bus/src/refusals.mjs b/packages/bus/src/refusals.mjs index b0f372e30..47cbe509d 100644 --- a/packages/bus/src/refusals.mjs +++ b/packages/bus/src/refusals.mjs @@ -225,6 +225,12 @@ group( "R1/R10: a saved template carries no credential value the bus holds", "A field the template would keep holds a credential value. Remove it from the named definition, board or workflow text, then preview and save again." ); +// E2E6 fix 8: save never replaces a template already saved under the name. +group( + "template-exists", + "R10: a saved template is never replaced", + "A template of this name is already saved for your account. Save under another name, or list your saved templates to pick a free one." +); group( "not-resolver", "Role resolve right and decision route", diff --git a/packages/bus/src/tester-cli.mjs b/packages/bus/src/tester-cli.mjs index 17a35c658..4b285734f 100644 --- a/packages/bus/src/tester-cli.mjs +++ b/packages/bus/src/tester-cli.mjs @@ -15,7 +15,7 @@ if (!process.env.MOSAIC_BUS_TESTER_NONCE) { // P19 #876: only the re-executed child, whose initial environment holds the nonce, talks to the // bus. Loading the client (and the broker module graph behind it) here instead of at the top // keeps the re-exec parent to a bare node start. -const { Client } = await import('./client.mjs'); +const { Client, NOT_SENT_EXIT } = await import('./client.mjs'); const { readProcess } = await import('./human.mjs'); try { const request = JSON.parse(readFileSync(0, 'utf8')); @@ -39,5 +39,6 @@ try { process.stdout.write(JSON.stringify(result) + '\n'); } catch (e) { process.stderr.write((/^[a-z-]{1,64}$/.test(e.code ?? '') ? e.code : 'invalid-request') + '\n'); - process.exitCode = 2; + // E2E6 fix 6: as human-cli.mjs, a request that never left exits NOT_SENT_EXIT. + process.exitCode = e?.notSent === true ? NOT_SENT_EXIT : 2; } diff --git a/packages/bus/tests/autonomy-authority.test.mjs b/packages/bus/tests/autonomy-authority.test.mjs index e302b5905..d5fca4c99 100644 --- a/packages/bus/tests/autonomy-authority.test.mjs +++ b/packages/bus/tests/autonomy-authority.test.mjs @@ -64,7 +64,7 @@ test('AU authority reverse crossing deadline in prepared IO never publishes',asy }); test('AU authority resolver rechecks budget removal after a proposal was raised',async t=>{ const f=world(t),d=await f.raise(),b=structuredClone(f.loaded());b.plugins['limit-check']=[];f.broker.reloadBusiness('demo',b); - const before=f.writer.read('demo').revision;await assert.rejects(f.call('judge','decision.resolve',{id:d.id,choice:'yes'}),{code:'autonomy-precondition'}); + const before=f.writer.read('demo').revision;await assert.rejects(f.call('judge','decision.resolve',{id:d.id,choice:'yes'}),{code:'autonomy-precondition',reason:'no enforced budget covers new agents; authority away from the loop needs one'}); assert.equal(f.writer.read('demo').revision,before);assert.equal(f.store.get('SELECT count(*) n FROM decision_events WHERE decision=?',d.id).n,0); }); @@ -113,7 +113,7 @@ test('AU an account-quota authority raise routes to the resolver only when the h const covered=world(t,{account:true,countsAccount:name=>name==='pi'}),d=await covered.raise();assert.equal(d.route_to,'judge'); for(const countsAccount of [undefined,()=>false,()=>null]){ const f=world(t,{account:true,countsAccount}); - await assert.rejects(f.raise(),{code:'autonomy-precondition'}); + await assert.rejects(f.raise(),{code:'autonomy-precondition',reason:'no enforced budget covers new agents; authority away from the loop needs one'}); assert.equal(f.store.get("SELECT count(*) n FROM decisions").n,0); } }); diff --git a/packages/bus/tests/autonomy.test.mjs b/packages/bus/tests/autonomy.test.mjs index dd5040df0..3597b8907 100644 --- a/packages/bus/tests/autonomy.test.mjs +++ b/packages/bus/tests/autonomy.test.mjs @@ -137,11 +137,11 @@ test('AU authority near requires an enforced organization budget covering every const f=await settingsFixture(t); for(const tokens of [{},{instances:{coder:tokenCap}},{providers:{unrelated:tokenCap}}]){ f.business.launch.limits.tokens=tokens;f.broker.reloadBusiness('demo',f.business); - await assert.rejects(f.set(authorityNear),{code:'autonomy-precondition'}); + await assert.rejects(f.set(authorityNear),{code:'autonomy-precondition',reason:'no enforced budget covers new agents; authority away from the loop needs one'}); } f.business.launch.limits.tokens={providers:{anthropic:{...tokenCap,limit:0}}}; f.business.plugins={'limit-check':[]};f.broker.reloadBusiness('demo',f.business); - await assert.rejects(f.set(authorityNear),{code:'autonomy-precondition'}); + await assert.rejects(f.set(authorityNear),{code:'autonomy-precondition',reason:'no enforced budget covers new agents; authority away from the loop needs one'}); f.business.plugins={'limit-check':['token-limit']};f.broker.reloadBusiness('demo',f.business); await f.set(authorityNear);assert.equal(f.writes(),1); }); @@ -153,12 +153,20 @@ test('AU authority out refuses until per-definition agent limits exist, even wit await assert.rejects(f.set({...authorityNear,authority:'out of the loop'}),e=>e.code==='authority-limits-unavailable' && /near the loop/.test(e.nextStep)); assert.equal(f.writes(),1); }); +test('AU a class moves out of the loop only from near the loop, and the refusal says so (E2E6 fix 7)',async t=>{ + const f=await settingsFixture(t);await f.set({'cross-role':'in the loop'}); + await assert.rejects(f.set({'cross-role':'out of the loop',notices:true}),{code:'autonomy-precondition',reason:'cross-role moves out of the loop only from near the loop'}); + assert.equal(f.writes(),1); +}); test('AU moving out requires an actual near resolution and a later owner review',async t=>{ const f=await settingsFixture(t);await f.set({merge:'in the loop'});await f.set({merge:'near the loop',notices:true}); + // E2E6 fix 7: each refusal names the check that failed. + await assert.rejects(f.set({merge:'near the loop'}),{code:'autonomy-precondition',reason:'owner notices are off; a class near the loop needs them on'}); + await assert.rejects(f.set({merge:'out of the loop',notices:true}),{code:'autonomy-precondition',reason:'merge needs your review of its trail since the last autonomy change'}); f.call(f.owner,'autonomy.review.console',{decisionClass:'merge'}); - await assert.rejects(f.set({merge:'out of the loop',notices:true}),{code:'autonomy-precondition'}); + await assert.rejects(f.set({merge:'out of the loop',notices:true}),{code:'autonomy-precondition',reason:'merge needs a near-loop decision since the last autonomy change, and your trail review after it'}); const d=f.raise({action:'git.merge.protected'});f.call(f.chief,'decision.resolve',{id:d.id,choice:'yes'}); - await assert.rejects(f.set({merge:'out of the loop',notices:true}),{code:'autonomy-precondition'}); + await assert.rejects(f.set({merge:'out of the loop',notices:true}),{code:'autonomy-precondition',reason:'merge needs a near-loop decision since the last autonomy change, and your trail review after it'}); f.call(f.owner,'autonomy.review.console',{decisionClass:'merge'}); await f.set({merge:'out of the loop',notices:true}); }); diff --git a/packages/bus/tests/channel-proof.test.mjs b/packages/bus/tests/channel-proof.test.mjs index eb59400e4..2f2e8223e 100644 --- a/packages/bus/tests/channel-proof.test.mjs +++ b/packages/bus/tests/channel-proof.test.mjs @@ -11,6 +11,8 @@ // which shares humanCall's spawn as transport(). The human proof and its spawn are unchanged. // Row 225 (C2-10b) re-pins webui bus.mjs for one STATUS entry, library-revert-not-agent: 409; no proof // or transport change. +// E2E6 fix 6 re-pins human-cli.mjs, which exits 5 when its connect failed before the request was +// written, and cli transport.mjs, which reads that exit as bus-not-running. The proof is unchanged. import test from "node:test"; import assert from "node:assert/strict"; import { createHash } from "node:crypto"; @@ -21,9 +23,10 @@ import { Client } from "../src/client.mjs"; const baselines = { "packages/bus/src/human.mjs": "bb13c146717cdc46d75d157177e400edac4d3e10f14fe511027a25fd44cec885", - "packages/bus/src/human-cli.mjs": "271b9781d73cf31cc562a56180e3ebcd725b5fde148a83347a201fdef2fc28af", + "packages/bus/src/human-cli.mjs": "410892787cc70440b0ba4c45d6f24861d12ee5e70cba678cfeb1ed86b796c149", // Row 239 (#1745): EXIT outcome-unknown-revoked 1; no proof or transport change. - "packages/cli/src/transport.mjs": "662642a8ce739fcb6c188ad07125a94e008c8a305eced127c8041d472c56e55b", + // E2E6 fix 6: shim exit 5 with outcome-unknown becomes bus-not-running (exit 3, start and recover next). + "packages/cli/src/transport.mjs": "101870f7ffb47a9cbd54ebd8c1901734cc9f442d6770cb2a0d059afd4d14b160", // Row 165 (#1662): seven STATUS entries for the role instance routes (organization-instance-exists, // organization-unknown-instance, organization-credentials-out-of-scope, organization-launch-not-configured, // organization-no-change, organization-project-mismatch, organization-unresolvable-instance), no proof or diff --git a/packages/bus/tests/templates.test.mjs b/packages/bus/tests/templates.test.mjs index 7d576500d..7e7d65770 100644 --- a/packages/bus/tests/templates.test.mjs +++ b/packages/bus/tests/templates.test.mjs @@ -154,7 +154,7 @@ test('SP8 argument checks: a second save, a .. name, an unknown key, preview wit const { call, saved } = await harness(t); const { preview } = await save(call, 'demo-shape'); const first = readFileSync(saved('demo-shape')); - await assert.rejects(call('template.save.console', { name: 'demo-shape', digest: preview.digest }), (e) => e.code === 'invalid-request' && e.reason === 'a template of this name is already saved'); + await assert.rejects(call('template.save.console', { name: 'demo-shape', digest: preview.digest }), (e) => e.code === 'template-exists' && e.reason === 'a template of this name is already saved; save under another name'); assert.deepEqual(readFileSync(saved('demo-shape')), first); for (const name of ['..', '../x', 'a/b']) await assert.rejects(call('template.save.console', { name, preview: true }), { code: 'invalid-request' }); for (const args of [{ name: 'x', preview: true, account: 'alex' }, { name: 'x', preview: true, digest: preview.digest }, { name: 'x' }, { preview: true }, { name: 'x', preview: false }]) diff --git a/packages/bus/tests/transport.test.mjs b/packages/bus/tests/transport.test.mjs index 2b8168228..defec661d 100644 --- a/packages/bus/tests/transport.test.mjs +++ b/packages/bus/tests/transport.test.mjs @@ -121,3 +121,24 @@ test('committed mutation followed by dropped reply reports unknown and is never assert.equal(calls, 1); assert.equal(store.get('SELECT count(*) n FROM messages').n, 1); }); + +// E2E6 fix 6: a connect that fails before the request is written is marked notSent; the code stays +// outcome-unknown for every caller. A request that left and got no answer is not marked. +test('E2E6 a failed connect is outcome-unknown marked notSent; a dropped answer is not marked', async (t) => { + const { createServer } = await import('node:net'); + const { writeFileSync } = await import('node:fs'); + const { NOT_SENT_EXIT } = await import('../src/client.mjs'); + assert.equal(NOT_SENT_EXIT, 5); + const dir = mkdtempSync(join(tmpdir(), 'e2e6-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const plain = join(dir, 'plain'); + writeFileSync(plain, ''); + for (const path of [join(dir, 'missing.sock'), plain, join(plain, 'below')]) { + await assert.rejects(new Client({ path, cap: 'x' }).call('inbox', {}), (e) => e.code === 'outcome-unknown' && e.notSent === true, path); + } + const sock = join(dir, 'drop.sock'); + const server = createServer((c) => c.once('data', () => c.destroy())); + await new Promise((r) => server.listen(sock, r)); + t.after(() => server.close()); + await assert.rejects(new Client({ path: sock, cap: 'x' }).call('inbox', {}), (e) => e.code === 'outcome-unknown' && e.notSent === undefined); +}); diff --git a/packages/cli/src/cli.mjs b/packages/cli/src/cli.mjs index b46260f13..f589107a9 100644 --- a/packages/cli/src/cli.mjs +++ b/packages/cli/src/cli.mjs @@ -845,6 +845,7 @@ export async function run(argv, io = { env: process.env, stdin: process.stdin, s if (!(error instanceof CliError)) throw error; io.stderr.write(`mosaic: ${error.message}\n`); if (OTHER_TREE_CODES.has(error.code)) io.stderr.write(`mosaic: ${OTHER_TREE}\n`); + if (typeof error.next === "string" && error.next !== "") io.stderr.write(`mosaic: next: ${error.next}\n`); return error.exitCode; } } diff --git a/packages/cli/src/errors.mjs b/packages/cli/src/errors.mjs index 8ac27f4fe..511d10257 100644 --- a/packages/cli/src/errors.mjs +++ b/packages/cli/src/errors.mjs @@ -3,11 +3,15 @@ // input, a decision or option that doesn't exist); 3 refused (system or // business config problem, the broker or notifier refused, the human proof // refused, a host already running); 4 usage. +// +// E2E6: `next` is an optional next-step line. run() prints it after the message as +// `mosaic: next: ...`; the message itself stays as callers (the Console among them) match it. export class CliError extends Error { - constructor(message, exitCode = 2) { + constructor(message, exitCode = 2, { next } = {}) { super(message); this.name = "CliError"; this.exitCode = exitCode; + if (typeof next === "string" && next !== "") this.next = next; } } diff --git a/packages/cli/src/host.mjs b/packages/cli/src/host.mjs index 2c64fa958..10d215a22 100644 --- a/packages/cli/src/host.mjs +++ b/packages/cli/src/host.mjs @@ -76,6 +76,11 @@ const same = (a, b) => a?.pid === b?.pid && a?.startTime === b?.startTime && a?. // tried again, like one that goes before the check. `found` runs once the // lock is found and before it is read, `between` after the stale lock is // read; both are for the tests. Exported for the tests. +// E2E6 fix 1: every stale-lock refusal names the verb that clears a host that died. +export const recoverNext = (business) => + typeof business === "string" && /^[a-z0-9][a-z0-9._-]{0,63}$/.test(business) + ? `if the bus host died, clear it with mosaic bus recover ${business}; recover refuses while the host still runs` + : "if the bus host died, clear it with mosaic bus recover ; recover refuses while the host still runs"; export function claimHostLock(dataRoot, state, { found = () => {}, between = () => {}, refuseStale = false } = {}) { mkdirSync(hostDir(dataRoot), { recursive: true, mode: 0o700 }); const file = hostLock(dataRoot); @@ -94,7 +99,8 @@ export function claimHostLock(dataRoot, state, { found = () => {}, between = () const prior = readState(file, found); if (!prior) continue; if (prior.live) throw new CliError(`a bus host already runs for ${prior.business} (pid ${prior.pid})`, 3); - if (refuseStale) throw new CliError("stale bus host lock", 3); + // E2E6 fix 1: the message stays (the Console matches it); the next line names the clearing verb. + if (refuseStale) throw new CliError("stale bus host lock", 3, { next: recoverNext(prior.business) }); between(); try { renameSync(file, aside); diff --git a/packages/cli/src/org-bus.mjs b/packages/cli/src/org-bus.mjs index be6eec6dc..bda9f991d 100644 --- a/packages/cli/src/org-bus.mjs +++ b/packages/cli/src/org-bus.mjs @@ -8,7 +8,7 @@ import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { resolveOrg, withOrgLifecycle } from '../../organization/src/org-registry.mjs'; import { orgPaths } from './config.mjs'; -import { cmdlineOf, hostFile, hostLock, hostStatus, readHostState, startTimeOf, stopHost } from './host.mjs'; +import { cmdlineOf, hostFile, hostLock, hostStatus, readHostState, recoverNext, startTimeOf, stopHost } from './host.mjs'; import { CliError } from './errors.mjs'; import { readBootStatus } from './boot-status.mjs'; @@ -27,20 +27,22 @@ async function target(ctx, id, allowDeleting = false) { return { ...org, paths: orgPaths(org.root) }; } function checkedState(org) { + // E2E6 fix 1: each refusal keeps its message and adds the recover line as its next step. + const stale = message => { throw new CliError(message, 3, { next: recoverNext(org.id) }); }; const s = hostStatus(org.root); let lock = null; try { lock = JSON.parse(readFileSync(hostLock(org.root), 'utf8')); } - catch (e) { if (e.code !== 'ENOENT') fail('bus host lock is unreadable'); } - if (lock && (!Number.isSafeInteger(lock.pid) || typeof lock.startTime !== 'string' || startTimeOf(lock.pid) !== lock.startTime)) fail('stale bus host lock'); - if (s.host && (!s.host.live || s.host.business !== org.id)) fail('stale or mismatched bus host state'); + catch (e) { if (e.code !== 'ENOENT') stale('bus host lock is unreadable'); } + if (lock && (!Number.isSafeInteger(lock.pid) || typeof lock.startTime !== 'string' || startTimeOf(lock.pid) !== lock.startTime)) stale('stale bus host lock'); + if (s.host && (!s.host.live || s.host.business !== org.id)) stale('stale or mismatched bus host state'); if (s.host) { const args = cmdlineOf(s.host.pid) ?? [], i = args.indexOf(CLI); - if (i < 0 || args[i + 1] !== 'bus' || args[i + 2] !== 'start' || args[i + 3] !== org.id) fail('bus host identity mismatch'); + if (i < 0 || args[i + 1] !== 'bus' || args[i + 2] !== 'start' || args[i + 3] !== org.id) stale('bus host identity mismatch'); } - if (lock && (!s.host || lock.pid !== s.host.pid || lock.business !== org.id)) fail('bus host lock does not match host state'); - if (s.writerLock && !s.writerLock.live) fail('stale bus writer lock'); - if (!s.host && s.writerLock) fail('bus writer lock has no matching host'); - if (s.host && !lock) fail('bus host lock is missing'); + if (lock && (!s.host || lock.pid !== s.host.pid || lock.business !== org.id)) stale('bus host lock does not match host state'); + if (s.writerLock && !s.writerLock.live) stale('stale bus writer lock'); + if (!s.host && s.writerLock) stale('bus writer lock has no matching host'); + if (s.host && !lock) stale('bus host lock is missing'); return s; } export function probeBusSocket(path, { signal, deadline } = {}) { diff --git a/packages/cli/src/org-console.mjs b/packages/cli/src/org-console.mjs index a5fc39fc2..4a5eefab9 100644 --- a/packages/cli/src/org-console.mjs +++ b/packages/cli/src/org-console.mjs @@ -3,7 +3,7 @@ import { randomBytes } from 'node:crypto'; import { processState } from '../../seat/src/process-state.mjs'; import { spawn } from 'node:child_process'; import { request as httpRequest } from 'node:http'; -import { constants, closeSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, rmSync, renameSync, writeFileSync, writeSync } from 'node:fs'; +import { constants, closeSync, fstatSync, ftruncateSync, linkSync, lstatSync, mkdirSync, openSync, readFileSync, rmSync, renameSync, writeFileSync, writeSync } from 'node:fs'; import { basename, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { listOrgs, resolveOrg } from '../../organization/src/org-registry.mjs'; @@ -60,14 +60,79 @@ function removeStale(ctx, s, warn) { warn(`mosaic: stale console state moved to ${aside}\n`); return true; } -function takeLock(ctx) { +// E2E6 fix 4: what a start.lock that already exists names. Null when it names no holder: unreadable, +// empty (a holder between its open and its write), not a regular file of this user, or a bad pid or +// start time. Such a lock is never cleared. A lock its holder marked released ({"released":true}, +// written through the holder's own descriptor) names no live holder and is cleared (f6c review P3-1). +function lockHolder(path) { + let fd; + try { fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); } + catch { return null; } + try { + const st = fstatSync(fd); + if (!st.isFile() || st.uid !== process.getuid() || st.nlink !== 1 || st.size > 4096) return null; + const data = JSON.parse(readFileSync(fd, 'utf8')); + if (data?.released === true && Object.keys(data).length === 1) return { released: true, dev: st.dev, ino: st.ino }; + if (!Number.isSafeInteger(data?.pid) || data.pid <= 0 || typeof data.startTime !== 'string' || !/^\d+$/.test(data.startTime)) return null; + return { pid: data.pid, startTime: data.startTime, dev: st.dev, ino: st.ino }; + } catch { return null; } finally { closeSync(fd); } +} +// True when the process the lock names (pid and start time) is gone. A /proc read that cannot settle +// it counts as live, so an uncertain holder is never cleared. +function holderGone(holder) { + if (holder.released) return true; + let p; + try { p = processState(holder.pid); } catch { return false; } + return !p.alive || p.startTime !== holder.startTime; +} +// Moves aside and removes the lock the holder check read, and only that file: a lock another start +// took in between is put back and the clear refuses. +function clearLock(path, holder) { + const aside = `${path}.dead-${randomBytes(4).toString('hex')}`; + try { renameSync(path, aside); } catch { fail('console start lock changed while it was being cleared; run the command again'); } + const st = lstatSync(aside); + if (st.dev !== holder.dev || st.ino !== holder.ino) { + // Put back without replacing: if a third start took the path meanwhile, the aside stays for a human. + let restored = false; + try { linkSync(aside, path); restored = true; } catch { /* left aside */ } + if (restored) rmSync(aside, { force: true }); + fail('console start lock changed while it was being cleared; run the command again'); + } + rmSync(aside, { force: true }); +} +function takeLock(ctx, warn = () => {}) { checkDir(ctx); const path = join(directory(ctx), 'start.lock'); - let fd; try { fd = openSync(path, 'wx', 0o600); } catch { fail('console start lock exists'); } + let fd; + for (let cleared = false; fd === undefined;) { + try { fd = openSync(path, 'wx', 0o600); } + catch (e) { + if (e?.code !== 'EEXIST' || cleared) fail('console start lock exists'); + // E2E6 fix 4: a lock whose named holder (pid and start time) is gone is cleared once, and the + // take tried again. A live holder, or a lock that names none, refuses and stays. + const holder = lockHolder(path); + if (!holder) fail('console start lock exists and names no holder; remove console/start.lock under the data root once no console start or stop runs'); + if (!holderGone(holder)) fail(`console start lock is held by process ${holder.pid}, which still runs; wait for its start or stop to finish`); + clearLock(path, holder); + warn(holder.released ? 'mosaic: cleared a console start lock its holder had released\n' : `mosaic: cleared a console start lock left by process ${holder.pid}, which is gone\n`); + cleared = true; + } + } // D1: the lock names its holder, so a detached start that timed out can clear the lock of the - // supervisor it killed (clearDeadLock) and no other. + // supervisor it killed (clearDeadLock), and a later start or stop can clear it once that holder is gone. try { writeSync(fd, JSON.stringify({ pid: process.pid, startTime: startTimeOf(process.pid) })); } catch { /* an unnamed lock is only never cleared */ } - return () => { closeSync(fd); rmSync(path, { force: true }); }; + // f6c review P3-1: the release marks its own file released, through its own descriptor, and removes + // the path only while the path is still that file. A clear that moved the lock aside and puts it back + // after this release puts back a released lock, which the next start or stop clears; and the release + // never removes a lock another start took meanwhile. + // A failed marker write (disk full) still removes the path: an empty lock would refuse forever. + return () => { + try { + try { ftruncateSync(fd, 0); writeSync(fd, JSON.stringify({ released: true }), 0); } catch { /* the matched unlink below still runs */ } + const own = fstatSync(fd), now = lstatSync(path); + if (own.dev === now.dev && own.ino === now.ino) rmSync(path, { force: true }); + } catch { /* gone already, or moved aside by a clear */ } finally { closeSync(fd); } + }; } // Removes start.lock only when it names this exact process (pid and start time) and that process // is gone. A lock that is unreadable, unnamed or another holder's stays. @@ -242,7 +307,7 @@ export async function consoleStart(ctx, { devLocalOwner = false, detach = false, // re-check P1). A Restart Console relaunch goes through relaunch(), not here. refuseInsideAgent(process.env); if (ctx.env) refuseInsideAgent(ctx.env); if (detach && devLocalOwner) return detachedSupervisor(ctx, { port, bind, allowHosts, warn }); - const release = takeLock(ctx); + const release = takeLock(ctx, warn); let run, args; try { const prior = state(ctx); @@ -266,7 +331,7 @@ export async function consoleStart(ctx, { devLocalOwner = false, detach = false, return result; } export async function consoleStop(ctx, { warn = s => process.stderr.write(s) } = {}) { - const release = takeLock(ctx); + const release = takeLock(ctx, warn); try { const s = state(ctx); if (!s || removeStale(ctx, s, warn)) return { stopped: false }; diff --git a/packages/cli/src/org-definitions.mjs b/packages/cli/src/org-definitions.mjs index bb009c08f..547d4c71f 100644 --- a/packages/cli/src/org-definitions.mjs +++ b/packages/cli/src/org-definitions.mjs @@ -4,10 +4,12 @@ // credential store, whose held-secret check every definition write needs (R1), and no other process // holds the tokens it looks for. Each write prints its receipt. import { readFile } from 'node:fs/promises'; +import { basename } from 'node:path'; import { configDir } from '../../organization/src/organization-config.mjs'; import { organizationWriter } from '../../organization/src/organization.mjs'; import { definitionVerbs } from '../../organization/src/definition-verbs.mjs'; import { checkDefinition } from '../../organization/src/definitions.mjs'; +import { ID_PATTERN } from '../../organization/src/vocabulary.mjs'; import { rolesDir } from './config.mjs'; import { CliError } from './errors.mjs'; import { busExit, refuseInsideAgent } from './transport.mjs'; @@ -59,6 +61,33 @@ function refusal(code, verb, name, rev, read) { return error; } +// E2E6 fix 2: a read or a write's file check that fails says so in one line: an unknown +// organization, a missing definition or a bad file, by code. No stack trace, and no host path: a raw +// filesystem error is named by its operation and code, and any absolute path left in a message is cut. +const NAME_SHOWN = /^[a-z0-9][a-z0-9._-]{0,63}$/; +function missing(id, name) { return `${NAME_SHOWN.test(name ?? '') ? `definitions declares no ${name}` : 'definitions declares no definition by that name'}; list them with mosaic org definitions ${id} list`; } +// f6c review P3-3: a path after ':' or a backtick is cut too. +function pathFree(message) { return String(message).replace(/(^|[\s'"(=:`])\/[^\s'"),`]*/g, '$1'); } +export function definitionsFailure(e, id, name) { + if (e instanceof CliError) return e; + const coded = (code, text, exit = 3) => Object.assign(new CliError(`${code}: ${text}`, exit), { code }); + if (typeof e?.syscall === 'string' && typeof e.code === 'string' && /^E[A-Z0-9]+$/.test(e.code)) { + // f6c review P3-2: only a missing organization file means no organization; another missing file + // (the roles directory for `templates`, a role template) says which operation failed. + if (e.code === 'ENOENT' && (typeof e.path !== 'string' || basename(e.path) === `${id}.json`)) return coded('unknown-organization', `this instance has no organization ${id}; list them with mosaic org list`); + if (id === 'templates' || id === undefined) return coded('definitions-unreadable', `cannot ${e.syscall} a file the definitions command reads (${e.code})`); + return coded('organization-unreadable', `cannot ${e.syscall} the file of organization ${id} (${e.code})`); + } + const message = typeof e?.message === 'string' ? e.message : ''; + if (/^definitions declares no /.test(message)) return coded('definition-missing', missing(id, name)); + if (typeof e?.code === 'string' && /^[a-z][a-z-]{0,63}$/.test(e.code)) { + const text = e.code === 'definition-missing' ? missing(id, name) : pathFree(message); + return coded(e.code, text); + } + if (e?.name === 'OrganizationConfigError' || e?.name === 'BusinessError') return coded('organization-invalid', pathFree(message)); + return coded('definitions-failed', 'the definitions command failed', 1); +} + // deps.callFor(parsed) returns the human's call(verb, args) for the organization parsed names (cli.mjs // passes its callFor). Without one, a write refuses: it never runs without the bus's secret check. export async function definitionsCommand(args, io, deps = {}) { @@ -68,6 +97,10 @@ export async function definitionsCommand(args, io, deps = {}) { const ifRev = () => { const r = need('--if-rev'); if (!/^[1-9][0-9]*$/.test(r)) throw new CliError('invalid --if-rev', 4); return Number(r); }; const [id, verb, name] = pos; if (WRITES.includes(verb)) refuseInsideAgent(io.env); + if (id !== 'templates' && id !== undefined && !ID_PATTERN.test(id)) throw new CliError(`invalid organization id: an id is 1 to 64 of a-z, 0-9, '.', '_' and '-', starting with a letter or digit\n${DEFINITIONS_USAGE}`, 4); + try { await run(); } + catch (e) { throw definitionsFailure(e, id, name); } + async function run() { const writer = deps.writer ?? organizationWriter({ dir: configDir(io.env), rolesDir: rolesDir(io.env) }); const verbs = definitionVerbs({ writer, rolesDir: rolesDir(io.env) }); let result; @@ -93,4 +126,5 @@ export async function definitionsCommand(args, io, deps = {}) { } } else throw new CliError(DEFINITIONS_USAGE, 4); io.stdout.write(JSON.stringify(result, null, 2) + '\n'); + } } diff --git a/packages/cli/src/org-owner.mjs b/packages/cli/src/org-owner.mjs index 6fb0a5b90..be61075ba 100644 --- a/packages/cli/src/org-owner.mjs +++ b/packages/cli/src/org-owner.mjs @@ -100,6 +100,15 @@ export function autonomyDocument(current, changes) { return out; } +// E2E6 fix 7: the autonomy-precondition check the CLI can see from the read and the document it sent: +// owner notices off while a class sits near the loop, or moves away from the loop. +const ORDER = ['in the loop', 'near the loop', 'out of the loop']; +export function preconditionSeen(current, doc) { + if (doc?.notices === true) return undefined; + const moved = CLASSES.filter(c => ORDER.indexOf(doc?.[c]) > ORDER.indexOf(current?.levels?.[c] ?? 'in the loop')); + if (CLASSES.some(c => doc?.[c] === 'near the loop') || moved.length) return 'owner notices are off; a class near the loop, or one moving away from it, needs them on (--notices on)'; + return undefined; +} // Autonomy codes carry no message from the route; these are the panel's own words (autonomy.js). const AUTONOMY_GUIDANCE = { 'revision-conflict': 'the organization changed; read it again with mosaic org autonomy ID', @@ -117,6 +126,10 @@ function refused(answer, what, unknown, guidance = {}) { const code = typeof raw === 'string' && /^[a-z-]{1,64}$/.test(raw) ? raw : `http ${answer.status}`; if (code === 'outcome-unknown' || (answer.status >= 500 && !raw)) return new CliError(`outcome unknown: ${unknown}`, 1); const said = typeof answer.body?.message === 'string' ? answer.body.message.slice(0, 500) : guidance[code]; + // E2E6 fix 7: a route that forwards the bus's reason (which check failed) has it said first; the + // general guidance becomes the next line. A reason is one printable line, never a control byte. + const reason = typeof answer.body?.reason === 'string' && /^[\x20-\x7e]{1,300}$/.test(answer.body.reason) ? answer.body.reason : undefined; + if (reason) return new CliError(`${what} refused (${code}): ${reason}`, 3, { next: said }); return new CliError(`${what} refused (${code})${said ? `: ${said}` : ''}`, 3); } @@ -173,8 +186,14 @@ export async function orgOwnerCommand(args, io, deps = {}) { const read = await consoleOrgRequest(ctx, id, { path: 'autonomy' }); if (!ok(read)) throw refused(read, 'autonomy set', 'the autonomy read did not finish; nothing was changed', AUTONOMY_GUIDANCE); const unknown = `the setting may have been saved; check mosaic org autonomy ${id} before trying again`; - const answer = await consoleOrgRequest(ctx, id, { method: 'POST', path: 'autonomy', body: { revision, autonomy: autonomyDocument(read.body, changes) }, unknown }); - if (!ok(answer)) throw refused(answer, 'autonomy set', unknown, AUTONOMY_GUIDANCE); + const doc = autonomyDocument(read.body, changes); + const answer = await consoleOrgRequest(ctx, id, { method: 'POST', path: 'autonomy', body: { revision, autonomy: doc }, unknown }); + if (!ok(answer)) { + // E2E6 fix 7: a route that forwards no reason still gets the one check this side can see. + const local = answer.body?.error === 'autonomy-precondition' && answer.body?.reason === undefined ? preconditionSeen(read.body, doc) : undefined; + if (local) throw new CliError(`autonomy set refused (autonomy-precondition): ${local}`, 3, { next: AUTONOMY_GUIDANCE['autonomy-precondition'] }); + throw refused(answer, 'autonomy set', unknown, AUTONOMY_GUIDANCE); + } return { value: answer.body, text: b => `saved autonomy for ${id}: revision ${b.revision ?? 'unknown'}${b.transaction ? `, receipt ${b.transaction}` : ''}\n` }; }; } else if (step === 'review') { diff --git a/packages/cli/src/org.mjs b/packages/cli/src/org.mjs index 4b5d22728..d6c1521bd 100644 --- a/packages/cli/src/org.mjs +++ b/packages/cli/src/org.mjs @@ -63,7 +63,22 @@ const PACK_REFUSALS = { 'invalid-request': 'the bus refused the request: a bad argument, a project that is not a code project, or a pack that fails the composition check', 'human-required': 'only the organization\'s owner, from a human shell, may do this', 'starter-pack-not-found': 'no installed, shipped or saved pack has that name', + 'starter-pack-invalid': 'the pack fails its own checks or the composition check; nothing was written', + 'starter-pack-template-changed': 'a template the pack names changed since it was loaded; preview again', + 'not-configured': 'this organization\'s bus has no organization writer configured', }; +// E2E6 fix 3: humanTransport throws a CliError whose message and .code are the bus code alone. A code +// a table knows gains its hint, as template save's do; a next line it carried is kept. Any other +// CliError (bus-not-running with its own next line, a usage error) passes unchanged, and a bare coded +// error (a test's call) becomes a CliError named by its code. +function hinted(e, ...tables) { + const code = typeof e?.code === 'string' ? e.code : ''; + const table = tables.find(t => Object.hasOwn(t, code)); + if (!table && (code === '' || e instanceof CliError)) return e; + const error = new CliError(table ? `${code}: ${table[code]}` : code, busExit(code), { next: e.next }); + error.code = code; + return error; +} async function packsCommand(args, io, deps) { const pos = [], values = {}, as = []; for (let i = 0; i < args.length; i++) { @@ -105,13 +120,7 @@ async function packsCommand(args, io, deps) { } let result; try { result = await call(verb, request); } - catch (e) { - if (typeof e?.code !== 'string' || e.code === '' || e instanceof CliError) throw e; - const why = PACK_REFUSALS[e.code]; - const error = new CliError(why ? `${e.code}: ${why}` : e.code, busExit(e.code)); - error.code = e.code; - throw error; - } + catch (e) { throw hinted(e, PACK_REFUSALS); } io.stdout.write(JSON.stringify(result, null, 2) + '\n'); } // #1873 SP8: template save previews (--preview) or writes the previewed JSON (--digest); template @@ -120,7 +129,8 @@ async function packsCommand(args, io, deps) { const TEMPLATE_REFUSALS = { 'template-secret-suspected': 'a field the template would keep holds a credential the organization\'s bus holds; remove it and preview again (nothing was written)', 'pack-plan-stale': 'the organization or a template changed since the preview; preview again and save its digest', - 'invalid-request': 'the bus refused the request: a bad name, a name already saved, an unknown project, or an organization the template cannot hold', + 'template-exists': 'a template of this name is already saved for your account; nothing was written', + 'invalid-request': 'the bus refused the request: a bad name, an unknown project, or an organization the template cannot hold', 'human-required': 'only the organization\'s owner, from a human shell, may do this', 'not-configured': 'the templates directory is missing or not private to this instance\'s user', 'starter-pack-invalid': 'the template would not be a valid pack: more than 8 hires, or a definition wider than its template', @@ -147,12 +157,9 @@ async function templateCommand(args, io, deps) { let result; try { result = await packCall(id, io, deps)(verb, request); } catch (e) { - // 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; + const error = hinted(e, TEMPLATE_REFUSALS, PACK_REFUSALS); + // E2E6 fix 8: a taken name says what to do next. + if (error.code === 'template-exists' && error.next === undefined) error.next = `preview and save under another name, or see the saved names with mosaic org template list ${id}`; throw error; } io.stdout.write(JSON.stringify(result, null, 2) + '\n'); diff --git a/packages/cli/src/transport.mjs b/packages/cli/src/transport.mjs index 52829a26c..0a8be6117 100644 --- a/packages/cli/src/transport.mjs +++ b/packages/cli/src/transport.mjs @@ -41,6 +41,7 @@ const EXIT = { "tester-required": 3, "tester-off": 3, "tester-refused": 3, + "bus-not-running": 3, "outcome-unknown": 1, "outcome-unknown-revoked": 1, "response-too-large": 1, @@ -51,6 +52,17 @@ export function busExit(code) { return EXIT[code] ?? 2; } +// E2E6 fix 6: the shim exits 5 (packages/bus/src/client.mjs NOT_SENT_EXIT) when its connect failed +// before the request was written: no socket, or the connect refused. That is a stopped bus, and +// nothing happened; outcome-unknown stays for a request that left and got no answer. +const NOT_SENT_EXIT = 5; +export function busNotRunning(business) { + const error = new CliError(`bus-not-running: the bus for ${business} is not running, so the request was not sent`, 3); + error.code = "bus-not-running"; + error.next = `start it with mosaic bus start ${business}, or clear a host that died with mosaic bus recover ${business}`; + return error; +} + // Returns a call(verb, args) for one business. `cli` and `spawn` exist for // the tests; the command line never sets them. export function humanTransport({ socket, business, env = process.env, cli = HUMAN_CLI, spawn = spawnSync, timeoutMs = 30000 }) { @@ -65,6 +77,7 @@ export function humanTransport({ socket, business, env = process.env, cli = HUMA if (proc.error || proc.status === null) throw new CliError("outcome-unknown: the bus transport did not finish", 1); if (proc.status !== 0) { const code = String(proc.stderr ?? "").trim().split("\n").reverse().find((l) => /^[a-z-]{1,64}$/.test(l)) ?? "invalid-response"; + if (proc.status === NOT_SENT_EXIT && code === "outcome-unknown") throw busNotRunning(business); const error = new CliError(code, busExit(code)); error.code = code; throw error; diff --git a/packages/cli/tests/cli-parity.test.mjs b/packages/cli/tests/cli-parity.test.mjs index 753996217..0bc4cd762 100644 --- a/packages/cli/tests/cli-parity.test.mjs +++ b/packages/cli/tests/cli-parity.test.mjs @@ -89,6 +89,8 @@ function organizations({ send = true } = {}) { calls.push({ verb, args, opts }); if (verb === 'autonomy.review.console') return { reviewed: args.decisionClass }; if (args.revision !== state.revision) throw new BusReadError('revision-conflict'); + // E2E6 fix 7: the bus refuses policy out of the loop here; the route forwards the code alone. + if (args.autonomy.policy === 'out of the loop') throw new BusReadError('autonomy-precondition'); const { notices, reverseWindowHours, authorityResolver, ...levels } = args.autonomy; state = { revision: REV(parseInt(state.revision, 16) + 1), levels, autonomy: { notices, reverseWindowHours, ...(authorityResolver ? { authorityResolver } : {}) } }; return { revision: state.revision, transaction: `tx-${parseInt(state.revision, 16)}` }; @@ -259,6 +261,21 @@ test('autonomy reads, saves with the panel\'s revision check and records a revie assert.equal((await page(c.port, 'GET', '/api/orgs/alpha/autonomy')).body.revision, REV(5)); c.calls.splice(0); + // E2E6 fix 7: an autonomy-precondition the route sends without a reason still names the one check the + // terminal can see from its read and the document it sent (notices off while a class moves away), + // and the general guidance becomes the next line. + r = await mosaic(env, ['org', 'autonomy', 'alpha', 'set', '--revision', REV(5), '--policy', 'out']); + assert.equal(r.status, 3, r.stderr); + assert.match(r.stderr, /^mosaic: autonomy set refused \(autonomy-precondition\): owner notices are off; a class near the loop, or one moving away from it, needs them on \(--notices on\)$/m); + assert.match(r.stderr, /^mosaic: next: this move needs a separate resolver/m); + const { preconditionSeen } = await import('../src/org-owner.mjs'); + const shown = { levels: { ...LOOP, merge: 'near the loop' } }; + assert.equal(preconditionSeen(shown, { ...LOOP, merge: 'near the loop', notices: true }), undefined); + assert.equal(preconditionSeen(shown, { ...LOOP, notices: false }), undefined, 'moving back toward the loop needs no notices'); + assert.match(preconditionSeen(shown, { ...LOOP, merge: 'near the loop', notices: false }), /^owner notices are off/); + assert.match(preconditionSeen({ levels: LOOP }, { ...LOOP, authority: 'out of the loop', notices: false }), /^owner notices are off/); + c.calls.splice(0); + r = await mosaic(env, ['org', 'autonomy', 'alpha', 'review', 'merge']); assert.equal(r.status, 0, r.stderr); assert.equal(r.stdout, 'recorded your trail review for merge on alpha\n'); diff --git a/packages/cli/tests/org-bus.test.mjs b/packages/cli/tests/org-bus.test.mjs index 48db54e78..a9adcea46 100644 --- a/packages/cli/tests/org-bus.test.mjs +++ b/packages/cli/tests/org-bus.test.mjs @@ -67,6 +67,15 @@ test('two organization buses are isolated and refusals never select a sibling', writeFileSync(join(alpha.root, 'bus-host/host.json'), JSON.stringify(staleHost), { mode: 0o600 }); writeFileSync(join(alpha.root, 'bus-host/host.lock'), JSON.stringify(staleHost), { mode: 0o600 }); writeFileSync(join(alpha.root, 'bus/writer.lock'), JSON.stringify({ pid: 2147483647, at: '2026-01-01T00:00:00Z' }), { mode: 0o600 }); + // E2E6 fix 1: status, start, stop and restart refuse a dead host's lock with the message unchanged and a + // next line naming the verb that clears it. + for (const args of [['status', 'alpha', '--json'], ['start', 'alpha', '--no-launch', '--detach'], ['stop', 'alpha'], ['restart', 'alpha']]) { + const r = spawnSync(join(REPO, 'scripts/mosaic'), ['bus', ...args], { env, encoding: 'utf8', timeout: 30000 }); + assert.equal(r.status, 3, `${args[0]}: ${r.stderr}`); + assert.match(r.stderr, /^mosaic: stale bus host lock$/m, args[0]); + assert.match(r.stderr, /^mosaic: next: if the bus host died, clear it with mosaic bus recover alpha; recover refuses while the host still runs$/m, args[0]); + } + await assert.rejects(busStatus(ctx, 'alpha'), e => e.message === 'stale bus host lock' && e.exitCode === 3 && /mosaic bus recover alpha/.test(e.next)); const recovery = spawnSync(join(REPO, 'scripts/mosaic'), ['bus', 'recover', 'alpha'], { env, encoding: 'utf8' }); assert.equal(recovery.status, 0, recovery.stderr); const restarted = await busStart(ctx, 'alpha', { detach: true, noLaunch: true }); diff --git a/packages/cli/tests/org-console.test.mjs b/packages/cli/tests/org-console.test.mjs index f0e37b2c2..3521e44d0 100644 --- a/packages/cli/tests/org-console.test.mjs +++ b/packages/cli/tests/org-console.test.mjs @@ -417,3 +417,60 @@ test('a detached console without the dev owner has no supervisor, so it refuses assert.equal(start.stderr, ''); assert.equal((await consoleStop(ctx)).stopped, true); }); + +// E2E6 fix 4: console start and stop clear a start.lock whose named supervisor (pid and start time) is +// gone, and never one whose holder still runs or that names no holder. +test('E2E6 a start.lock whose holder is gone is cleared by stop and by start; a live or unnamed one refuses and stays', { timeout: 60000 }, async t => { + const { startTimeOf } = await import('../src/host.mjs'); + const { ctx } = await instance(t); + const dir = join(ctx.dataRoot, 'console'), lock = join(dir, 'start.lock'); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + // A process this test starts and lets end: its pid and start time name a supervisor that is gone. + const child = spawn(process.execPath, ['-e', 'setTimeout(() => {}, 30000)'], { stdio: 'ignore' }); + await new Promise(r => child.once('spawn', r)); + const deadHolder = { pid: child.pid, startTime: startTimeOf(child.pid) }; + assert.match(deadHolder.startTime, /^\d+$/); + child.kill('SIGKILL'); + await new Promise(r => child.once('exit', r)); + + const warnings = []; + writeFileSync(lock, JSON.stringify(deadHolder), { mode: 0o600 }); + assert.deepEqual(await consoleStop(ctx, { warn: s => warnings.push(s) }), { stopped: false }); + assert.deepEqual(warnings, [`mosaic: cleared a console start lock left by process ${child.pid}, which is gone\n`]); + assert.equal(existsSync(lock), false); + assert.deepEqual(readdirSync(dir).filter(x => x.startsWith('start.lock')), []); + + // A live pid with another start time is a reused pid: its holder is gone too. + writeFileSync(lock, JSON.stringify({ pid: process.pid, startTime: '1' }), { mode: 0o600 }); + warnings.length = 0; + const result = await consoleStart(ctx, { detach: true, port: 'auto', warn: s => warnings.push(s) }); + assert.deepEqual(warnings, [`mosaic: cleared a console start lock left by process ${process.pid}, which is gone\n`]); + assert.equal(existsSync(lock), false, 'the start released its own lock'); + assert.equal((await consoleStop(ctx)).stopped, true); + assert.ok(Number.isSafeInteger(result.pid)); + + // A live holder refuses on start and stop, and the lock stays as it was. + const live = JSON.stringify({ pid: process.pid, startTime: startTimeOf(process.pid) }); + writeFileSync(lock, live, { mode: 0o600 }); + await assert.rejects(consoleStop(ctx), e => e.message === `console start lock is held by process ${process.pid}, which still runs; wait for its start or stop to finish`); + await assert.rejects(consoleStart(ctx, { detach: true, port: 'auto' }), /which still runs/); + assert.equal(readFileSync(lock, 'utf8'), live); + // A lock that names no holder (empty: a holder between its open and its write; or bad JSON) stays too. + for (const body of ['', '{"pid":', JSON.stringify({ pid: 0, startTime: '1' }), JSON.stringify({ pid: child.pid, startTime: 7 })]) { + writeFileSync(lock, body, { mode: 0o600 }); + await assert.rejects(consoleStop(ctx), /console start lock exists and names no holder/); + assert.equal(readFileSync(lock, 'utf8'), body); + } + // f6c review P3-1: a lock its holder marked released (a clear put it back after the release) is + // cleared; a released marker with anything else in it names no holder and stays. + writeFileSync(lock, JSON.stringify({ released: true }), { mode: 0o600 }); + warnings.length = 0; + assert.deepEqual(await consoleStop(ctx, { warn: s => warnings.push(s) }), { stopped: false }); + assert.deepEqual(warnings, ['mosaic: cleared a console start lock its holder had released\n']); + assert.equal(existsSync(lock), false); + const odd = JSON.stringify({ released: true, pid: 1 }); + writeFileSync(lock, odd, { mode: 0o600 }); + await assert.rejects(consoleStop(ctx), /console start lock exists and names no holder/); + assert.equal(readFileSync(lock, 'utf8'), odd); + rmSync(lock); +}); diff --git a/packages/cli/tests/org-definitions.test.mjs b/packages/cli/tests/org-definitions.test.mjs index d29c96739..a83008a8d 100644 --- a/packages/cli/tests/org-definitions.test.mjs +++ b/packages/cli/tests/org-definitions.test.mjs @@ -136,3 +136,55 @@ test('T1 mosaic org definitions: writes go through the bus and its secret check, await assert.rejects(orgCommand(['definitions', 'acme', 'remove', 'copy', '--if-rev', '1'], x), (e) => e.exitCode === 3 && /organization bus/.test(e.message)); assert.deepEqual(readFileSync(file), before); }); + +// E2E6 fix 2: an unknown definition, an unknown organization, a bad id and a bad name each end in +// one usage or refusal line: no stack trace and no host path. +test('E2E6 mosaic org definitions: unknown names, an unknown org and a bad id give one line, no stack and no path', async (t) => { + const root = tmp(t); + const f = fixture(root, 'acme'); + const { run } = await import('../src/cli.mjs'); + const cases = [ + [['acme', 'show', 'ghost'], 3, 'mosaic: definition-missing: definitions declares no ghost; list them with mosaic org definitions acme list'], + [['acme', 'diff', 'ghost'], 3, 'mosaic: definition-missing: definitions declares no ghost; list them with mosaic org definitions acme list'], + [['acme', 'show', 'Bad Name'], 3, 'mosaic: definition-missing: definitions declares no definition by that name; list them with mosaic org definitions acme list'], + [['nosuch', 'list'], 3, 'mosaic: unknown-organization: this instance has no organization nosuch; list them with mosaic org list'], + [['nosuch', 'show', 'x'], 3, 'mosaic: unknown-organization: this instance has no organization nosuch; list them with mosaic org list'], + ]; + for (const [args, exit, line] of cases) { + const x = io(f.env); + assert.equal(await run(['org', 'definitions', ...args], x, {}), exit, args.join(' ')); + assert.equal(x.err.text, `${line}\n`, args.join(' ')); + assert.ok(!x.err.text.includes(root) && !/\n\s+at /.test(x.err.text), args.join(' ')); + } + for (const args of [['Bad/Id', 'list'], ['../x', 'show', 'y']]) { + const x = io(f.env); + assert.equal(await run(['org', 'definitions', ...args], x, {}), 4, args.join(' ')); + assert.match(x.err.text, /^mosaic: invalid organization id: an id is 1 to 64 of a-z/); + assert.match(x.err.text, /\nmosaic org definitions templates \| definitions ID list/); + assert.ok(!x.err.text.includes(root) && !/\n\s+at /.test(x.err.text)); + } +}); + +test('E2E6 definitionsFailure: a raw filesystem error is named by its operation and code, a message path is cut, an unknown error says only that it failed', async () => { + const { definitionsFailure } = await import('../src/org-definitions.mjs'); + const fsError = (code, syscall) => Object.assign(new Error(`${code}: no such file, open '/home/someone/.config/x/acme.json'`), { code, syscall, path: '/home/someone/.config/x/acme.json' }); + let e = definitionsFailure(fsError('EACCES', 'open'), 'acme'); + assert.deepEqual([e.code, e.exitCode, e.message], ['organization-unreadable', 3, 'organization-unreadable: cannot open the file of organization acme (EACCES)']); + e = definitionsFailure(Object.assign(new Error("organization file /srv/x/acme.json is not valid"), { code: 'organization-invalid' }), 'acme'); + assert.equal(e.message, 'organization-invalid: organization file is not valid'); + e = definitionsFailure(new TypeError('Cannot read properties of undefined'), 'acme'); + assert.deepEqual([e.code, e.exitCode, e.message], ['definitions-failed', 1, 'definitions-failed: the definitions command failed']); + // f6c review P3-2: only the organization file missing means no organization. + e = definitionsFailure(fsError('ENOENT', 'open'), 'acme'); + assert.deepEqual([e.code, e.message], ['unknown-organization', 'unknown-organization: this instance has no organization acme; list them with mosaic org list']); + const rolesGone = Object.assign(new Error("ENOENT: no such file or directory, scandir '/home/someone/roles'"), { code: 'ENOENT', syscall: 'scandir', path: '/home/someone/roles' }); + e = definitionsFailure(rolesGone, 'templates'); + assert.deepEqual([e.code, e.exitCode, e.message], ['definitions-unreadable', 3, 'definitions-unreadable: cannot scandir a file the definitions command reads (ENOENT)']); + e = definitionsFailure(rolesGone, 'acme'); + assert.equal(e.code, 'organization-unreadable'); + // f6c review P3-3: a path after ':' or inside backticks is cut. + e = definitionsFailure(Object.assign(new Error('organization file:/srv/x/acme.json is not valid'), { code: 'organization-invalid' }), 'acme'); + assert.equal(e.message, 'organization-invalid: organization file: is not valid'); + e = definitionsFailure(Object.assign(new Error('bad role at `/srv/roles/x.json`'), { code: 'organization-invalid' }), 'acme'); + assert.equal(e.message, 'organization-invalid: bad role at ``'); +}); diff --git a/packages/cli/tests/org-packs.test.mjs b/packages/cli/tests/org-packs.test.mjs index bd309fd14..5c2b58905 100644 --- a/packages/cli/tests/org-packs.test.mjs +++ b/packages/cli/tests/org-packs.test.mjs @@ -97,3 +97,36 @@ test('SP4 an agent shell refuses every pack, board and workflow word, pack list assert.deepEqual(s.calls, []); assert.deepEqual(readFileSync(s.file), before); }); + +// E2E6 fix 3: the real transport shape. humanTransport throws a CliError whose message and .code are +// the bus code alone (packages/cli/src/transport.mjs); pack, board and workflow refusals must still +// gain their hint, as template save's do, and a CliError with its own next line passes unchanged. +test('E2E6 pack, board and workflow refusals from a CliError-shaped transport print the hint', async (t) => { + const { CliError } = await import('../src/errors.mjs'); + const { busExit, busNotRunning } = await import('../src/transport.mjs'); + const root = tmp(t); + const f = fixture(root, 'acme'); + const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: 'acme', env: f.env }); + const b = await startBroker({ dataRoot: f.dataRoot, businesses: boot.businesses, organization: boot.organization }); + t.after(() => b.close()); + const human = b.broker.bindHuman({ business: 'acme', human: f.doc.human, via: 'cli', outsideAgent: true }); + let down = false; + const deps = { system: { dataRoot: f.dataRoot }, transport: () => async (verb, args = {}) => { + if (down) throw busNotRunning('acme'); + try { return structuredClone(await b.broker.request(human, { verb, args })); } + 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) => { const x = io(f.env); await main(['org', ...args], x, deps); return JSON.parse(x.out.text); }; + const hinted = (e) => typeof e.code === 'string' && e instanceof CliError && e.message.startsWith(`${e.code}: `) && e.message.length > e.code.length + 10; + + await assert.rejects(run(['pack', 'preview', 'acme', 'no-such-pack']), (e) => e.code === 'starter-pack-not-found' && e.message === 'starter-pack-not-found: no installed, shipped or saved pack has that name'); + const spec = join(root, 'board.json'); + writeFileSync(spec, JSON.stringify({ columns: [{ state: 'todo', label: 'Backlog' }], layout: 'kanban' })); + await assert.rejects(run(['board', 'set', 'acme', spec]), (e) => hinted(e) && e.exitCode === busExit(e.code)); + await assert.rejects(run(['workflow', 'remove', 'acme', 'ghost']), (e) => hinted(e) && e.exitCode === busExit(e.code)); + + // A stopped bus keeps its own message and next line. + down = true; + await assert.rejects(run(['pack', 'preview', 'acme', 'no-such-pack']), (e) => e.code === 'bus-not-running' && e.exitCode === 3 + && e.message === 'bus-not-running: the bus for acme is not running, so the request was not sent' && /mosaic bus start acme/.test(e.next) && /mosaic bus recover acme/.test(e.next)); +}); diff --git a/packages/cli/tests/org-paths.test.mjs b/packages/cli/tests/org-paths.test.mjs index db3195974..69b623321 100644 --- a/packages/cli/tests/org-paths.test.mjs +++ b/packages/cli/tests/org-paths.test.mjs @@ -37,8 +37,13 @@ test('organization host claim refuses a stale lock without removing it', t => { const stale = { pid: 99999999, startTime: '0', business: 'alpha', startedAt: 'old' }; writeFileSync(hostLock(root), JSON.stringify(stale), { mode: 0o600 }); const mine = { pid: process.pid, startTime: startTimeOf(process.pid), business: 'alpha', startedAt: 'now' }; - assert.throws(() => claimHostLock(root, mine, { refuseStale: true }), /stale.*lock/i); + // E2E6 fix 1: the message stays; the next line names the verb that clears the dead host. + assert.throws(() => claimHostLock(root, mine, { refuseStale: true }), e => e.message === 'stale bus host lock' && e.exitCode === 3 + && e.next === 'if the bus host died, clear it with mosaic bus recover alpha; recover refuses while the host still runs'); assert.deepEqual(JSON.parse(readFileSync(hostLock(root), 'utf8')), stale); + // A lock naming no valid organization id still names the verb, with a placeholder. + writeFileSync(hostLock(root), JSON.stringify({ ...stale, business: '../x\nsecret' }), { mode: 0o600 }); + assert.throws(() => claimHostLock(root, mine, { refuseStale: true }), e => e.next === 'if the bus host died, clear it with mosaic bus recover ; recover refuses while the host still runs'); }); import { busStatus } from '../src/org-bus.mjs'; diff --git a/packages/cli/tests/org-templates.test.mjs b/packages/cli/tests/org-templates.test.mjs index 8d438eba3..8b59900e5 100644 --- a/packages/cli/tests/org-templates.test.mjs +++ b/packages/cli/tests/org-templates.test.mjs @@ -40,7 +40,7 @@ test('SP8 mosaic org template save: --preview writes nothing, --digest writes th const before = readFileSync(s.file); const preview = await s.run(['template', 'save', 'acme', 'acme-shape', '--preview']); assert.match(preview.digest, /^[0-9a-f]{64}$/); - assert.ok(Array.isArray(preview.freeText) && Array.isArray(preview.replacements)); + assert.ok(Array.isArray(preview.freeText) && Array.isArray(preview.mentions) && !('replacements' in preview)); assert.ok(!existsSync(s.saved('acme-shape')), 'preview writes no file'); const out = await s.run(['template', 'save', 'acme', 'acme-shape', '--digest', preview.digest]); @@ -49,7 +49,9 @@ test('SP8 mosaic org template save: --preview writes nothing, --digest writes th assert.equal(lstatSync(s.saved('acme-shape')).mode & 0o777, 0o600); assert.deepEqual(readFileSync(s.file), before, 'save writes nothing to the organization file'); - await assert.rejects(s.run(['template', 'save', 'acme', 'acme-shape', '--digest', preview.digest]), (e) => e.code === 'invalid-request' && /already saved/.test(e.message)); + await assert.rejects(s.run(['template', 'save', 'acme', 'acme-shape', '--digest', preview.digest]), (e) => e.code === 'template-exists' && e.exitCode === 2 + && e.message === 'template-exists: a template of this name is already saved for your account; nothing was written' + && /under another name/.test(e.next) && /mosaic org template list acme/.test(e.next)); await assert.rejects(s.run(['template', 'save', 'acme', 'other', '--digest', preview.digest]), (e) => e.code === 'pack-plan-stale' && /preview again/.test(e.message)); const listed = await s.run(['template', 'list', 'acme']); assert.deepEqual(listed.templates.map((x) => x.name), ['acme-shape']); diff --git a/packages/cli/tests/transport.test.mjs b/packages/cli/tests/transport.test.mjs index 08a4ad9d7..3175ca4d0 100644 --- a/packages/cli/tests/transport.test.mjs +++ b/packages/cli/tests/transport.test.mjs @@ -1,8 +1,8 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { busExit, humanTransport, refuseInsideAgent } from "../src/transport.mjs"; +import { busExit, humanTransport, refuseInsideAgent, HUMAN_CLI } from "../src/transport.mjs"; import { fakeCli, humanEnv, tmp } from "./helpers.mjs"; // The fake records argv and stdin, then answers per verb. The real @@ -15,6 +15,8 @@ const { verb } = JSON.parse(input); if (verb === "inbox") process.stdout.write(JSON.stringify([{ id: "d1" }]) + "\\n"); else if (verb === "garbage") process.stdout.write("not json"); else if (verb === "hang") setTimeout(() => {}, 60000); +else if (verb === "not-sent") { process.stderr.write("outcome-unknown\\n"); process.exit(5); } +else if (verb === "not-sent-other") { process.stderr.write("human-required\\n"); process.exit(5); } else { process.stderr.write("some noise\\n" + verb + "\\n"); process.exit(2); } `; @@ -50,3 +52,28 @@ test("busExit and refuseInsideAgent", () => { assert.doesNotThrow(() => refuseInsideAgent(humanEnv())); assert.throws(() => refuseInsideAgent({ PI_AGENT_DIR: "/x" }), (e) => e.exitCode === 3 && /PI_AGENT_DIR/.test(e.message)); }); + +// E2E6 fix 6: the shim exits 5 when its connect failed before the request was written. That is a stopped +// bus: mosaic says so with the start and recover line. outcome-unknown stays for a request that left. +test("E2E6 exit 5 with outcome-unknown is bus-not-running; exit 2 with outcome-unknown is unchanged", async (t) => { + const s = setup(t); + await assert.rejects(s.call("not-sent"), (e) => e.code === "bus-not-running" && e.exitCode === 3 + && e.message === "bus-not-running: the bus for acme is not running, so the request was not sent" + && e.next === "start it with mosaic bus start acme, or clear a host that died with mosaic bus recover acme"); + await assert.rejects(s.call("outcome-unknown"), (e) => e.code === "outcome-unknown" && e.exitCode === 1 && e.message === "outcome-unknown" && e.next === undefined); + // Exit 5 says only that nothing was sent; any other code it carries keeps its own meaning. + await assert.rejects(s.call("not-sent-other"), (e) => e.code === "human-required" && e.exitCode === 3); + assert.equal(busExit("bus-not-running"), 3); +}); + +// The real shim, with no bus behind the path: nothing reaches a broker, so no proof runs. A missing socket +// and a path that is not a listening socket are both a bus that is not running. +test("E2E6 the real human shim against a missing socket or a plain file says the bus is not running", async (t) => { + const root = tmp(t); + const plain = join(root, "plain.sock"); + writeFileSync(plain, ""); + for (const socket of [join(root, "missing.sock"), plain]) { + const call = humanTransport({ socket, business: "acme", env: humanEnv(), cli: HUMAN_CLI, timeoutMs: 20000 }); + await assert.rejects(call("inbox"), (e) => e.code === "bus-not-running" && e.exitCode === 3, socket); + } +}); diff --git a/packages/organization/src/template-save.mjs b/packages/organization/src/template-save.mjs index a252b774d..c37e0995b 100644 --- a/packages/organization/src/template-save.mjs +++ b/packages/organization/src/template-save.mjs @@ -10,10 +10,13 @@ // (instance credentials, copyBindings, copyId, orgIncarnation, generation), launch, plugins, vars, // project roots and git state, definition revs and hire decisions, and everything on the bus. // -// Free text is kept, with the organization id, its project ids, its agent names, the owner's -// account and the project roots replaced by {org}, {project}, {agent}, {owner} and {root}. The -// preview lists each replacement and every kept free-text field by path, and the exact JSON save -// writes; save takes that JSON's digest. Every string and key is passed to the credential store's +// Free text is kept verbatim: save never rewrites prose (E2E6 fix 5, Lead ruling). A placeholder +// could stand only where a field's whole value is an organization identifier, and a pack carries no +// such field (hire names are -, step roles name definitions or templates), so save writes +// none and apply has none to resolve. The preview lists every kept free-text field by path, each +// one that mentions the organization id, a project id, an agent name, the owner's account or a +// project root (so the owner can edit it before saving), and the exact JSON save writes; save takes +// that JSON's digest. Every string and key is passed to the credential store's // assertClean; a hit refuses template-secret-suspected naming the path, never the text. // // A refusal is a TemplateError: a bus code and a reason that names a path or a checked name only, @@ -49,8 +52,8 @@ export const DEFAULT_BOARD = Object.freeze({ layout: "kanban", }); const HIRE_REASON = "The organization this template was saved from ran this agent."; -// A word boundary for placeholder replacement: an identifier character on either side means the -// literal is part of a longer name, which stays as it is. +// A word boundary for a mention: an identifier character on either side means the literal is part of +// a longer name, which is not a mention. const WORD = "A-Za-z0-9_-"; export class TemplateError extends Error { @@ -100,63 +103,63 @@ function templateOrNull(rolesDir, name, cache) { return cache.get(name); } -// ---- Placeholders ---- +// ---- Mentions ---- -// The literals section 5 replaces, longest first so a root path is replaced before the ids inside -// it. A name that is also a template or a definition name is a role's name, not a person's or a -// client's, and stays: replacing "coder" would rewrite every role text that says coder. -function literals(document, { rolesDir, cache }) { +// The organization-specific names the preview looks for in kept text, longest first so a root path +// is found before the ids inside it. A name that is also a template or a definition name is a role's +// name, not a person's or a client's, and is not a mention: role text says "coder" on purpose. A +// mention is labelled by kind and name; a project root is labelled by kind alone, so the preview +// names no host path beyond the text it already shows. +function orgNames(document, { rolesDir, cache }) { const out = []; - const add = (text, placeholder) => { - if (typeof text === "string" && text.length > 0 && !out.some((l) => l.text === text)) out.push({ text, placeholder }); + const add = (text, label) => { + if (typeof text === "string" && text.length > 0 && !out.some((l) => l.text === text)) out.push({ text, label }); }; for (const project of Object.values(document.projects ?? {})) { - add(project?.root, "{root}"); - add(project?.repo, "{root}"); + add(project?.root, "project root"); + add(project?.repo, "project root"); } - add(document.id, "{org}"); - for (const id of Object.keys(document.projects ?? {})) add(id, "{project}"); - add(document.human, "{owner}"); + if (typeof document.id === "string") add(document.id, `organization ${document.id}`); + for (const id of Object.keys(document.projects ?? {})) add(id, `project ${id}`); + if (typeof document.human === "string") add(document.human, `owner ${document.human}`); const role = (name) => Object.hasOwn(document.definitions ?? {}, name) || templateOrNull(rolesDir, name, cache) !== null; const agents = new Set([...Object.keys(document.roles ?? {}), ...(Array.isArray(document.retired) ? document.retired : [])]); 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}"); + for (const name of agents) if (!role(name)) add(name, `agent ${name}`); out.sort((a, b) => b.text.length - a.text.length); return out; } -// 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}. +// The labels of every name the text mentions, in first-mention order, each once. One pass with one +// alternation, longest name first, so a root path is one mention and not the ids inside it. const combined = new WeakMap(); -function replace(text, list, counts) { - if (list.length === 0) return text; +function mentions(text, list) { + if (list.length === 0) return []; let entry = combined.get(list); if (!entry) { - const by = new Map(list.map((l) => [l.text, l.placeholder])); + const by = new Map(list.map((l) => [l.text, l.label])); entry = { by, pattern: new RegExp(`(? escape(l.text)).join("|")})(?![${WORD}])`, "g") }; combined.set(list, entry); } - return text.replace(entry.pattern, (match) => { - const placeholder = entry.by.get(match); - counts[placeholder] = (counts[placeholder] ?? 0) + 1; - return placeholder; - }); + const found = []; + for (const match of text.matchAll(entry.pattern)) { + const label = entry.by.get(match[0]); + if (!found.includes(label)) found.push(label); + } + return found; } // ---- Capture ---- -// Builds the template document. With list null, free text is kept as it is (the scan's second -// copy); otherwise each replacement and each kept free-text field is recorded by path. +// Builds the template document. Free text is copied verbatim; each kept free-text field, and each +// one that mentions an organization-specific name, is recorded by path. function capture(document, { name, project, rolesDir, list, record }) { const cache = new Map(); const text = (value, path) => { - if (list === null) return value; - const counts = {}; - const out = replace(value, list, counts); - for (const [placeholder, count] of Object.entries(counts)) record.replacements.push({ path, placeholder, count }); - record.freeText.push({ path, text: out }); - return out; + record.freeText.push({ path, text: value }); + const names = mentions(value, list); + if (names.length > 0) record.mentions.push({ path, names }); + return value; }; const orgDefinitions = plain(document.definitions) ? document.definitions : {}; const scope = project === undefined ? "org" : "project"; @@ -233,7 +236,7 @@ function capture(document, { name, project, rolesDir, list, record }) { if (!template) refuse("starter-pack-invalid", `${at}.from names a template that is not a version 2 role`); // A template that changed since the definition copied it: the saved from names its current // hash, and the role must still narrow the current template. The preview lists the re-pin. - if (template.sha256 !== entry.from.sha256 && list !== null) record.repinned.push(`${at}.from`); + if (template.sha256 !== entry.from.sha256) record.repinned.push(`${at}.from`); out.from = { template: template.name, sha256: template.sha256 }; let composition; try { @@ -323,11 +326,12 @@ function capture(document, { name, project, rolesDir, list, record }) { seen.add(k); packs.push({ id: entry.id, version: entry.version }); } - if (list !== null) record.packsDropped = Math.max(0, packs.length - PACK_BOUNDS.packs); + record.packsDropped = Math.max(0, packs.length - PACK_BOUNDS.packs); const keptPacks = packs.slice(Math.max(0, packs.length - PACK_BOUNDS.packs)); const label = name.slice(0, PACK_BOUNDS.label); - const descriptionText = scope === "org" ? "Saved from {org}." : "Saved from {project} in {org}."; + // No placeholder: apply copies the description as it stands, so a "{org}" here would stay literal. + const descriptionText = scope === "org" ? "Saved from an organization." : "Saved from one project of an organization."; return { packVersion: PACK_VERSION, id: name, version: "1.0.0", label, description: descriptionText, scope, definitions, hires, board, workflows, skills, packs: keptPacks, @@ -356,20 +360,16 @@ function scan(value, path, assertClean) { } } -// The preview: the exact JSON save writes, its digest, each replacement and each kept free-text -// field by path. Refuses before returning when any string or key holds a held credential value. +// The preview: the exact JSON save writes, its digest, each kept free-text field by path, each such +// field that mentions an organization-specific name, with the names it mentions, and each re-pin. +// Refuses before returning when any string or key holds a held credential value. export function templatePreview({ document, name, project, rolesDir, assertClean }) { checkTemplateName(name); if (project !== undefined && (typeof project !== "string" || !ID_PATTERN.test(project))) refuse("invalid-request", "the project id is invalid"); if (typeof assertClean !== "function") throw new TypeError("templatePreview needs the credential store's assertClean"); if (!plain(document)) refuse("invalid-request", "the organization file is not readable"); - const scope = { name, project, rolesDir }; - // The scan reads both copies: the kept text before replacement (a held value a placeholder would - // break up still refuses) and the document save writes. - const raw = capture(document, { ...scope, list: null, record: null }); - scan(raw, "", assertClean); - const record = { replacements: [], freeText: [], repinned: [], packsDropped: 0 }; - const template = capture(document, { ...scope, list: literals(document, { rolesDir, cache: new Map() }), record }); + const record = { freeText: [], mentions: [], repinned: [], packsDropped: 0 }; + const template = capture(document, { name, project, rolesDir, list: orgNames(document, { rolesDir, cache: new Map() }), record }); scan(template, "", assertClean); let checked; try { @@ -386,13 +386,27 @@ export function templatePreview({ document, name, project, rolesDir, assertClean return { name, scope: checked.scope, ...(project === undefined ? {} : { project }), template: checked, json, digest: sha256(json), - replacements: record.replacements.sort(byPath), freeText: record.freeText.sort(byPath), + freeText: record.freeText.sort(byPath), mentions: record.mentions.sort(byPath), repinned: record.repinned.sort(), packsDropped: record.packsDropped, }; } // ---- The file ---- +const TEMPLATE_EXISTS = "a template of this name is already saved; save under another name"; +// Whether a file (or anything, a symlink included) already holds the name. It looks only through a +// templates and an account directory that are real directories, so a link is never followed to say +// what lies elsewhere; any other case is left to the directory checks and the exclusive create. +function taken(templates, home, file) { + try { + if (!lstatSync(templates).isDirectory() || !lstatSync(home).isDirectory()) return false; + lstatSync(file); + return true; + } catch { + return false; + } +} + // A directory save writes under: real, the uid's own, writable by nobody else. The account's own // directory is private (0700 or narrower), as readSavedTemplate requires. function directory(path, uid, { private: closed }) { @@ -413,23 +427,25 @@ function directory(path, uid, { private: closed }) { } // Save: the preview again, its digest compared, then the file created exclusively (wx, 0600). A -// name already saved refuses; nothing is replaced. +// name already saved refuses template-exists; nothing is replaced. The name is checked before the +// digest, so a save under a taken name says so even when the preview is stale. export function templateSave({ document, name, project, digest, rolesDir, assertClean, configDir, account, uid = process.getuid() }) { checkAccount(account); checkTemplateName(name); if (typeof digest !== "string" || !/^[0-9a-f]{64}$/.test(digest)) refuse("invalid-request", "save takes the preview's digest"); - const preview = templatePreview({ document, name, project, rolesDir, assertClean }); - if (preview.digest !== digest) refuse("pack-plan-stale", "the organization or a template changed since the preview"); const templates = join(configDir, "templates"); const home = join(templates, account); + const file = join(home, `${name}.json`); + if (taken(templates, home, file)) refuse("template-exists", TEMPLATE_EXISTS); + const preview = templatePreview({ document, name, project, rolesDir, assertClean }); + if (preview.digest !== digest) refuse("pack-plan-stale", "the organization or a template changed since the preview"); directory(templates, uid, { private: false }); directory(home, uid, { private: true }); - const file = join(home, `${name}.json`); let fd; try { fd = openSync(file, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, 0o600); } catch (error) { - if (error?.code === "EEXIST") refuse("invalid-request", "a template of this name is already saved"); + if (error?.code === "EEXIST") refuse("template-exists", TEMPLATE_EXISTS); refuse("not-configured", "the template file cannot be created"); } try { diff --git a/packages/organization/tests/template-save.test.mjs b/packages/organization/tests/template-save.test.mjs index 9348c6efa..a2af7a031 100644 --- a/packages/organization/tests/template-save.test.mjs +++ b/packages/organization/tests/template-save.test.mjs @@ -1,6 +1,7 @@ // Starter packs SP8 (#1873): saving an organization or one of its projects as a template. The saved // file carries no credential material and no member, keeps each role's credential request lines, -// replaces the organization's own names with placeholders, and reads back as an origin-saved pack. +// keeps free text verbatim (E2E6 fix 5) while the preview lists the fields that mention the +// organization's own names, and reads back as an origin-saved pack. import test from "node:test"; import assert from "node:assert/strict"; import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; @@ -94,34 +95,56 @@ test("SP8 the saved file keeps each role's credential request lines", (t) => { assert.ok(template.definitions.writer.role.credentials.length > 0); }); -test("SP8 placeholders replace the org and project ids, the owner, agent names and project roots, in free text only", (t) => { +// E2E6 fix 5, red before: the old save rewrote prose word by word, so a project named "work" turned +// "hand finished work to review" into "hand finished {project} to review", and apply never resolves +// a placeholder. Save now keeps every free-text field as it stands. +test("E2E6 red before: save never rewrites prose; a project named work leaves \"hand finished work to review\" as it is", (t) => { + const { doc, rolesDir } = fixture(t); + doc.projects.work = { ...structuredClone(doc.projects.stack), root: join(doc.projects.stack.root, "..", "work-root") }; + doc.workflows.change.steps[1].title = "hand finished work to review"; + const preview = templatePreview({ document: doc, name: "team-shape", rolesDir, assertClean }); + assert.equal(preview.template.workflows.change.steps[1].title, "hand finished work to review"); + assert.ok(!preview.json.includes("{project}")); + // The owner sees the mention and may edit the text before saving. + assert.deepEqual(preview.mentions.find((m) => m.path === "workflows.change.steps[1].title"), { path: "workflows.change.steps[1].title", names: ["project work"] }); +}); + +test("E2E6 free text is kept verbatim, no placeholder is written, and the preview lists each field that mentions an org-specific name", (t) => { const { doc, rolesDir } = fixture(t); const preview = templatePreview({ document: doc, name: "team-shape", rolesDir, assertClean }); const { template } = preview; - assert.equal(template.definitions.writer.instructions, "Write code for {org} in {root}/src. Ask {owner}; pair with {agent} on {project}."); - assert.equal(template.definitions.writer.personality.summary, "Careful {org} engineer."); - assert.deepEqual(template.definitions.writer.personality.traits, ["works with {agent}", "plain"]); - assert.equal(template.board.columns[0].label, "{org} backlog"); - assert.equal(template.workflows.change.description, "Change for {project}"); - assert.equal(template.workflows.change.steps[0].title, "{agent} writes it"); - // Agent names that are role names (pm, coder) stay; role text says "coder" on purpose. - const all = JSON.stringify(template); - for (const literal of ["acme", "jason", "ana", doc.projects.stack.root]) assert.ok(!all.includes(literal), literal); - assert.equal(template.description, "Saved from {org}."); - // Each replacement is listed by path. - const at = (path) => preview.replacements.filter((r) => r.path === path).map((r) => r.placeholder).sort(); - assert.deepEqual(at("definitions.writer.instructions"), ["{agent}", "{org}", "{owner}", "{project}", "{root}"]); + assert.equal(template.definitions.writer.instructions, doc.definitions.writer.instructions); + assert.equal(template.definitions.writer.personality.summary, "Careful acme engineer."); + assert.deepEqual(template.definitions.writer.personality.traits, ["works with ana", "plain"]); + assert.equal(template.board.columns[0].label, "acme backlog"); + assert.equal(template.workflows.change.description, "Change for stack"); + assert.equal(template.workflows.change.steps[0].title, "ana writes it"); + for (const placeholder of ["{org}", "{project}", "{agent}", "{owner}", "{root}"]) assert.ok(!preview.json.includes(placeholder), placeholder); + assert.equal(template.description, "Saved from an organization."); + assert.ok(!Object.hasOwn(preview, "replacements")); + const at = (path) => preview.mentions.find((m) => m.path === path)?.names; + // A root path is one mention, labelled by kind alone, not the ids inside it. + assert.deepEqual(at("definitions.writer.instructions"), ["organization acme", "project root", "owner jason", "agent ana", "project stack"]); + assert.deepEqual(at("definitions.writer.personality.traits[1]"), undefined); + assert.deepEqual(at("board.columns[0].label"), ["organization acme"]); + // Role names (writer, coder) are not mentions; a longer word holding a name is not one either. + assert.equal(at("board.columns[1].label"), undefined); + doc.board.columns[1].label = "acmes and anas"; + assert.equal(templatePreview({ document: doc, name: "team-shape", rolesDir, assertClean }).mentions.find((m) => m.path === "board.columns[1].label"), undefined); + // Every mention is a kept free-text field, in path order. + const paths = preview.mentions.map((m) => m.path); + assert.deepEqual(paths, [...paths].sort()); + for (const p of paths) assert.ok(preview.freeText.some((f) => f.path === p), p); }); -test("SP8 an instance named root, org or agent never rewrites a placeholder already written in", (t) => { +test("E2E6 text that looks like a placeholder, or an instance named root, org or agent, stays verbatim", (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."; + doc.definitions.writer.instructions = "Work for acme with root on stack. Ask org, then jason. Keep {org} and {agent} literal."; 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}"]); + assert.equal(preview.template.definitions.writer.instructions, doc.definitions.writer.instructions); + assert.deepEqual(preview.mentions.find((m) => m.path === "definitions.writer.instructions").names, ["organization acme", "agent root", "project stack", "agent org", "owner jason"]); }); test("SP8 the preview lists every kept free-text field with its saved text", (t) => { @@ -151,7 +174,7 @@ test("SP8 a held credential in kept text refuses template-secret-suspected namin doc.definitions.writer.personality.voice = `short, key ${TOKEN}`; refused(() => templatePreview({ document: doc, name: "acme-shape", rolesDir, assertClean }), "template-secret-suspected", "definitions.writer.personality.voice"); try { templatePreview({ document: doc, name: "acme-shape", rolesDir, assertClean }); } catch (e) { assert.ok(!e.message.includes(TOKEN)); } - // A held value a placeholder would split still refuses: the scan reads the text before replacement. + // A held value inside other text still refuses: the scan reads the text save keeps. const second = fixture(t); second.doc.workflows.change.steps[1].title = `acme${TOKEN}`; refused(() => templatePreview({ document: second.doc, name: "acme-shape", rolesDir: second.rolesDir, assertClean }), "template-secret-suspected", "workflows.change.steps[1].title"); @@ -170,7 +193,9 @@ test("SP8 a second save to the same name refuses and leaves the first file", (t) const first = readFileSync(file); doc.definitions.writer.instructions = "Changed."; const next = templatePreview({ document: doc, name: "acme-shape", rolesDir, assertClean }); - refused(() => templateSave({ document: doc, name: "acme-shape", digest: next.digest, rolesDir, assertClean, configDir, account: "jason" }), "invalid-request", "a template of this name is already saved"); + refused(() => templateSave({ document: doc, name: "acme-shape", digest: next.digest, rolesDir, assertClean, configDir, account: "jason" }), "template-exists", "a template of this name is already saved; save under another name"); + // E2E6 fix 8: the name is checked first, so a stale or malformed-but-valid digest still says the name is taken. + refused(() => templateSave({ document: doc, name: "acme-shape", digest: "0".repeat(64), rolesDir, assertClean, configDir, account: "jason" }), "template-exists"); assert.deepEqual(readFileSync(file), first); assert.equal(lstatSync(file).mode & 0o777, 0o600); assert.equal(lstatSync(join(configDir, "templates", "jason")).mode & 0o777, 0o700); @@ -246,7 +271,7 @@ test("SP8 a project template carries the project's agents, the definitions it na assert.deepEqual(Object.keys(template.definitions).sort(), ["reviewer", "writer"]); assert.deepEqual(template.board, JSON.parse(JSON.stringify(DEFAULT_BOARD))); assert.deepEqual(template.workflows, {}); - assert.equal(template.description, "Saved from {project} in {org}."); + assert.equal(template.description, "Saved from one project of an organization."); refused(() => templatePreview({ document: doc, name: "x", project: "nope", rolesDir, assertClean }), "invalid-request"); }); diff --git a/packages/webui/src/template-http.mjs b/packages/webui/src/template-http.mjs index d0cca1232..591b3fc70 100644 --- a/packages/webui/src/template-http.mjs +++ b/packages/webui/src/template-http.mjs @@ -28,7 +28,7 @@ const LIMIT = 4096; const codes = { 'invalid-request': 400, 'template-secret-suspected': 400, 'starter-pack-invalid': 400, 'project-not-found': 404, - 'pack-plan-stale': 409, 'starter-pack-template-changed': 409, 'organization-busy': 409, + 'pack-plan-stale': 409, 'starter-pack-template-changed': 409, 'organization-busy': 409, 'template-exists': 409, 'human-required': 403, 'read-only': 403, 'not-configured': 503, 'no-bus-host': 503, }; diff --git a/packages/webui/tests/template-http.test.mjs b/packages/webui/tests/template-http.test.mjs index f67a1a7bb..2ed0fa693 100644 --- a/packages/webui/tests/template-http.test.mjs +++ b/packages/webui/tests/template-http.test.mjs @@ -85,7 +85,7 @@ test('SP8 the owner previews a template (nothing written), saves it with the dig assert.equal(readFileSync(f.saved('demo-shape'), 'utf8'), preview.json); assert.deepEqual(readFileSync(f.file), before, 'the organization file is unchanged'); const [again, againBody] = await call(f, `${ORG}/templates`, 'POST', { name: 'demo-shape', digest: preview.digest }); - assert.deepEqual([again, againBody.error], [400, 'invalid-request']); + assert.deepEqual([again, againBody.error], [409, 'template-exists']); const [, list] = await call(f, `${ORG}/templates`); assert.deepEqual(list.templates.map(x => [x.name, x.scope]), [['demo-shape', 'org']]); // The preview and the list went over the read; only the saves used the fenced writer.