diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index d5e6f2bc3..b18965feb 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -1848,6 +1848,11 @@ export class Broker { if (a.class === 'BLOCK') this.#checkBlock(s, to, a.in_reply_to); if (a.decision) this.#decision(s, a.decision); const thread = this.#threadFor(s, to, a.in_reply_to); + // Row 170 (#1667): a tester's message to a role nobody holds is refused before it is + // stored, so a PM test never waits for the role's next run. A run launched but not yet + // claimed counts as no holder, as in #waiting. No lock: a holder that releases once the + // message is routed leaves it like any other message it held. + if (s.tester && !this.#recipient(s.business, to, thread)) fail('tester-target-not-running'); const mid = randomUUID(); this.#store.run( 'INSERT INTO messages(id,at,business,from_role,from_run,to_role,to_name,class,in_reply_to,decision,corrects,body) VALUES(?,?,?,?,?,?,?,?,?,?,?,?)', @@ -2489,7 +2494,8 @@ export class Broker { // told and whether launch.by itself has a holder. A run launched but not yet claimed counts as // no holder; the instance-running refusal stops a second launch. launch.by isn't told of a // message to it, of its own message (this result tells it) or of a tester's (a PM test - // authorizes nothing). Row 152 (#1646): launchable says whether the target is one of + // authorizes nothing); since row 170 (#1667) message.send refuses a tester's message to a role + // nobody holds before it gets here. Row 152 (#1646): launchable says whether the target is one of // launch.instances, so the sender learns at once that no launch can follow. launch.by is // still told: the Console shows the notice as the reply to the human's message. #waiting(s, mid, to, thread) { diff --git a/packages/bus/tests/launch-notice.test.mjs b/packages/bus/tests/launch-notice.test.mjs index a4121bdb8..fba5818cd 100644 --- a/packages/bus/tests/launch-notice.test.mjs +++ b/packages/bus/tests/launch-notice.test.mjs @@ -167,7 +167,7 @@ test('a run launched but not yet claimed counts as no holder', (t) => { assert.equal(ctx.notices().length, 1); }); -test('no notice for a message to launch.by, from it, from a tester, to the human, or in a thread to a thread instance', (t) => { +test('no notice for a message to launch.by, from it, a refused one from a tester, to the human, or in a thread to a thread instance', (t) => { const ctx = setup(t); // To launch.by while it is stopped: the result says so, and nobody is told. assert.deepEqual(ctx.call('human', 'message.send', { to: 'chief', body: 'are you there?' }).waiting, { @@ -187,9 +187,12 @@ test('no notice for a message to launch.by, from it, from a tester, to the human launcherRunning: true, launchable: true, }); - // From a tester: a PM test authorizes nothing, so it wakes no launcher. + // From a tester to a role nobody holds: refused before it is stored (row 170, #1667), so a PM + // test wakes no launcher. ctx.caps.tester = ctx.b.bindTester({ business: 'demo' }); - assert.equal(ctx.call('tester', 'message.send', { to: 'pm', body: '[mos test] ping' }).waiting.told, false); + const stored = ctx.store().get('SELECT count(*) n FROM messages').n; + assert.throws(() => ctx.call('tester', 'message.send', { to: 'pm', body: '[mos test] ping' }), { code: 'tester-target-not-running' }); + assert.equal(ctx.store().get('SELECT count(*) n FROM messages').n, stored); // To the human or the tester, which no run holds: no waiting field at all. ctx.launch('coder', 'coder'); ctx.claim('coder'); diff --git a/packages/bus/tests/tester.test.mjs b/packages/bus/tests/tester.test.mjs index c24376be3..962649b8c 100644 --- a/packages/bus/tests/tester.test.mjs +++ b/packages/bus/tests/tester.test.mjs @@ -134,6 +134,61 @@ test('a tester message is from tester, starts [mos test], and writes no human.in assert.equal(store.get("SELECT count(*) n FROM events WHERE kind='human.input'").n, 0); }); +// Row 170 (#1667): a tester's message to a role nobody holds is refused before it is stored. +const counts = (store) => ({ + messages: store.get('SELECT count(*) n FROM messages').n, + deliveries: store.get('SELECT count(*) n FROM deliveries').n, + notices: store.get("SELECT count(*) n FROM messages WHERE from_role='_bus'").n, +}); + +test('a tester message to a role no run holds refuses tester-target-not-running and stores nothing', (t) => { + const { b, store, call } = setup(t); + const tester = b.bindTester({ business: 'demo' }); + const before = counts(store); + assert.throws(() => call(tester, 'message.send', { to: 'cto', body: say() }), { code: 'tester-target-not-running' }); + assert.deepEqual(counts(store), before); + assert.equal(before.notices, 0); + // The prefix check still comes first. + assert.throws(() => call(tester, 'message.send', { to: 'cto', body: 'no prefix' }), { code: 'tester-prefix' }); +}); + +test('a tester message is refused while the run is only launched, and routes once it claims', (t) => { + const { b, store, call } = setup(t); + const tester = b.bindTester({ business: 'demo' }); + const cto = b.bindLaunch({ business: 'demo', role: 'cto', run: 'cto-run', harness: 'pi' }); + const before = counts(store); + assert.throws(() => call(tester, 'message.send', { to: 'cto', body: say() }), { code: 'tester-target-not-running' }); + assert.deepEqual(counts(store), before); + call(cto, 'role.claim'); + const sent = call(tester, 'message.send', { to: 'cto', body: say() }); + assert.equal(sent.waiting, undefined); + const delivered = store.all('SELECT op, holder_run FROM deliveries WHERE message=? ORDER BY seq', sent.id); + assert.deepEqual(delivered.map((d) => [d.op, d.holder_run]), [['routed', 'cto-run']]); + assert.deepEqual(call(cto, 'message.receive').map((m) => [m.id, m.from_role]), [[sent.id, 'tester']]); +}); + +test("a human message to a role no run holds still waits and tells launch.by", (t) => { + const root = mkdtempSync(join(tmpdir(), 'bus-tester-')); + const store = new Store(root); + t.after(() => { + store.close(); + rmSync(root, { recursive: true, force: true }); + }); + const businesses = business({ 'bus.tester': true }); + businesses.demo.launch = { by: 'pm', instances: ['pm', 'cto'], models: {}, limits: {} }; + const b = new Broker({ store, businesses }); + const pm = b.bindLaunch({ business: 'demo', role: 'pm', run: 'pm-run', harness: 'pi' }); + b.request(pm, { verb: 'role.claim' }); + const human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true }); + const tester = b.bindTester({ business: 'demo' }); + assert.throws(() => b.request(tester, { verb: 'message.send', args: { to: 'cto', body: say() } }), { code: 'tester-target-not-running' }); + const sent = b.request(human, { verb: 'message.send', args: { to: 'cto', body: 'start on it' } }); + assert.deepEqual(sent.waiting, { role: 'cto', launcher: 'pm', told: true, launcherRunning: true, launchable: true }); + assert.equal(store.get('SELECT to_role FROM messages WHERE id=?', sent.id).to_role, 'cto'); + const notices = store.all("SELECT to_role, in_reply_to FROM messages WHERE from_role='_bus'"); + assert.deepEqual(notices.map((n) => [n.to_role, n.in_reply_to]), [['pm', sent.id]]); +}); + test('a role answers a tester only in reply to a tester message, and the answer never reaches the human', (t) => { const { b, store, pm, human, reader, call } = setup(t); const tester = b.bindTester({ business: 'demo' }); @@ -373,8 +428,12 @@ async function runtime(t, vars, launches = []) { } test('tester-cli.mjs binds through the runtime when the switch is on, and refuses off or under a run marker', async (t) => { - const on = await runtime(t, { 'bus.tester': true }); + // A start time no process has, so this test process is not the shim's managed-run ancestor. + const on = await runtime(t, { 'bus.tester': true }, [{ business: 'demo', role: 'pm', run: 'pm-run', harness: 'pi', pid: process.pid, startTime: '1' }]); const reader = new Client({ path: on.path, cap: on.readers[0].cap }); + // Row 170 (#1667): cto has no run, so the tester's message to it is refused. + assert.deepEqual(await run(on.path, { business: 'demo', verb: 'message.send', args: { to: 'cto', body: say() } }), { status: 2, out: '', err: 'tester-target-not-running' }); + await new Client({ path: on.path, cap: on.launches[0].cap }).call('role.claim'); const sent = await run(on.path, { business: 'demo', verb: 'message.send', args: { to: 'pm', body: say() } }); assert.equal(sent.status, 0, sent.err); const id = JSON.parse(sent.out).id; diff --git a/packages/cli/README.md b/packages/cli/README.md index fb3ad8350..3ed8916d0 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -127,8 +127,8 @@ mosaic launches list [--json] when the bus told `launch.by`; `pm isn't running, and the business names no launch.by` with no launch block; `pm isn't running and isn't in the launch block, so chief can't launch it` when the role isn't one of - `launch.instances` (row 152, #1646); `pm isn't running; a tester's message - tells nobody to launch it` for `--tester`. When `launch.by` itself isn't + `launch.instances` (row 152, #1646). A `--tester` message to such a role is + refused instead (see `--tester` below). When `launch.by` itself isn't running, talk prints ` isn't running` (or `isn't running either` after the told line, but not after the launch-block line) and the command that starts it: @@ -168,8 +168,12 @@ mosaic launches list [--json] (`tester-off`), each exit 3. Like the human commands, `--tester` works only from the tree the bus runs from ("One pinned copy" below). `talk --tester` text must start with - `[mos test]`, else exit 2 before any call. The role's reply comes back to - the tester, not to the human's inbox. `decide`, `stop`, `launches` and + `[mos test]`, else exit 2 before any call. When no run holds the role + (row 170, #1667), the bus refuses the message with + `tester-target-not-running` and stores nothing, and talk prints `pm isn't + running, so the tester's message was refused and nothing was stored` and + exits 3. A run launched but not yet claimed counts as not running. The + role's reply comes back to the tester, not to the human's inbox. `decide`, `stop`, `launches` and `inbox --read` or `--read-all` take no `--tester` (exit 4). ## The app diff --git a/packages/cli/src/cli.mjs b/packages/cli/src/cli.mjs index 6a4f68be6..35a70a5d3 100644 --- a/packages/cli/src/cli.mjs +++ b/packages/cli/src/cli.mjs @@ -339,8 +339,6 @@ function waitingLines(w, business) { // Row 152 (#1646): launch.by can launch only launch.instances. An older bus sends no // launchable, so only an explicit false prints this line. if (w.launchable === false) return `${w.role} isn't running and isn't in the launch block, so ${w.launcher} can't launch it\n`; - // A human's message to another role always tells launch.by; a tester's never does. - if (!w.told) return `${w.role} isn't running; a tester's message tells nobody to launch it\n`; return `${w.role} isn't running, ${w.launcher} has been told\n` + (w.launcherRunning ? "" : restart(w.launcher, " either")); } @@ -383,6 +381,11 @@ async function talk(rest, io, deps, receiveOnly = false) { if (e.code === "thread-required-use-thread-send") { throw new CliError(`${to} runs in threads, so a message to it names one: mosaic talk --thread ; mosaic threads lists yours`, 2); } + // Row 170 (#1667): the bus stores no tester message to a role nobody holds. Exit 3 like the + // other tester refusals; transport.mjs's EXIT map stays as row 164 pinned it. + if (e.code === "tester-target-not-running" && parsed.flags.has("--tester")) { + throw new CliError(`${to} isn't running, so the tester's message was refused and nothing was stored`, 3); + } throw e; } io.stdout.write(`sent ${shortId(sent.id)} to ${to}\n`); diff --git a/packages/cli/tests/tester.test.mjs b/packages/cli/tests/tester.test.mjs index 5ccf7705c..0c0bb85a3 100644 --- a/packages/cli/tests/tester.test.mjs +++ b/packages/cli/tests/tester.test.mjs @@ -127,11 +127,43 @@ test("with no stand-in transport, --tester runs tester-cli.mjs on the data root' assert.deepEqual(seen, [{ argv: [TESTER_CLI, join(dataRoot, "bus", "broker.sock")], input: { business: "demo", verb: "agents", args: {} } }]); }); -// Row 146 (#1640): a tester's message to a stopped role tells launch.by nothing, and talk says so. -test("talk --tester to a stopped role says nobody is told to launch it", async (t) => { +// Row 170 (#1667): the bus refuses a tester's message to a stopped role and stores nothing, and +// talk says so and exits 3. +test("talk --tester to a stopped role exits 3, prints no sent line and stores nothing", async (t) => { const s = setup(t, { launch: { by: "pm", instances: ["cto", "coder"], models: {}, limits: {} } }); const x = inAgent(); - await main(["talk", "cto", "[mos test] hello", "--tester", "--wait", "0", "--business", "demo"], x, s.deps); - assert.match(x.out.text, /^sent [0-9a-f-]{8} to cto\ncto isn't running; a tester's message tells nobody to launch it\n$/); - assert.equal(s.store.get("SELECT count(*) n FROM messages WHERE from_role='_bus'").n, 0); + await assert.rejects(main(["talk", "cto", "[mos test] hello", "--tester", "--wait", "0", "--business", "demo"], x, s.deps), (e) => { + assert.equal(e.exitCode, 3); + assert.equal(e.message, "cto isn't running, so the tester's message was refused and nothing was stored"); + return true; + }); + assert.equal(x.out.text, ""); + assert.equal(s.store.get("SELECT count(*) n FROM messages").n, 0); + assert.deepEqual(s.calls.tester.map((c) => c.verb), ["message.send"]); +}); + +test("only a --tester send refused tester-target-not-running is rewritten", async (t) => { + const s = setup(t); + const refusing = (code) => () => async (verb) => { + const error = new Error(code); + error.code = code; + error.exitCode = 3; + throw error; + }; + // Another refusal of a tester send keeps the bus's error. + for (const code of ["tester-off", "tester-required", "tester-refused", "unknown-role"]) { + const x = inAgent(); + await assert.rejects(main(["talk", "cto", "[mos test] hello", "--tester", "--wait", "0", "--business", "demo"], x, { ...s.deps, testerTransport: refusing(code) }), (e) => { + assert.equal(e.code, code); + assert.equal(e.message, code); + return true; + }, code); + assert.equal(x.out.text, ""); + } + // The human's send never gets the tester's line, whatever its transport says. + await assert.rejects(main(["talk", "cto", "hello", "--wait", "0", "--business", "demo"], io(humanEnv()), { ...s.deps, transport: refusing("tester-target-not-running") }), (e) => { + assert.equal(e.code, "tester-target-not-running"); + assert.equal(e.message, "tester-target-not-running"); + return true; + }); });