diff --git a/packages/bus/README.md b/packages/bus/README.md index 18f3039dd..7293f98db 100644 --- a/packages/bus/README.md +++ b/packages/bus/README.md @@ -208,6 +208,17 @@ requires a strict task ref. `role.launch` is gated unless the business launch block's `by` names the acting role and refused when launch state is revoked. S6 still owns launch allowlists, capacity and actual spawning. +`decision.raise` checks its choices in this order, and each of these codes names one field: + +| Code | Field | Refused when | +|---|---|---| +| `invalid-options` | `options` | not an array, fewer than 2 or more than 9 options, an option whose `key` isn't 1 to 160 letters, digits or `_.:/-` starting with a letter or digit, or whose `text` isn't a string of 1 to 1024 characters with no NUL, or duplicate keys | +| `invalid-recommendation` | `recommendation` | the value isn't exactly one option key; `"yes: a reason"` refuses, so the reason goes in `question` | +| `invalid-approval-choice` | `approvalChoice` | the value, or the default `yes` when it is omitted, isn't an option key | + +The options come first, so a bad option list refuses `invalid-options` even when the recommendation also misses. +An option that isn't an object with exactly `key` and `text` refuses `invalid-request`, like any other malformed argument. + Authorization requires the resolved option identified by `approvalChoice` (default `yes`, which must exist), the same raiser role/run, action and target. Inbox projections expose `{action,target,approvalChoice}` as `authorization`: diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 64a544d4b..f4a39194f 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -781,8 +781,8 @@ export class Broker { if (!id(o.key) || !text(o.text, 1024)) fail('invalid-options'); } const choices = a.options.map((o) => o.key); - if (new Set(choices).size !== choices.length || !choices.includes(a.recommendation)) - fail('invalid-options'); + if (new Set(choices).size !== choices.length) fail('invalid-options'); + if (!choices.includes(a.recommendation)) fail('invalid-recommendation'); const approvalChoice = a.approvalChoice ?? 'yes'; if (!choices.includes(approvalChoice)) fail('invalid-approval-choice'); const domain = a.domain ?? 'delivery'; diff --git a/packages/bus/tests/broker.test.mjs b/packages/bus/tests/broker.test.mjs index 5dcf091c7..cf2e269bd 100644 --- a/packages/bus/tests/broker.test.mjs +++ b/packages/bus/tests/broker.test.mjs @@ -155,6 +155,37 @@ test('within-role decisions close atomically and invalid options or blocking omi assert.throws(() => raise(b, c, 'made.up'), /unknown-action/); assert.equal(call(b, c, 'inbox').length, 0); }); +test('a recommendation that is not exactly one option key refuses invalid-recommendation after the options', (t) => { + const { b, agent, store } = setup(t), + c = agent('coder'); + call(b, c, 'role.claim'); + const count = () => [ + store.get('SELECT count(*) AS n FROM decisions').n, + store.get('SELECT count(*) AS n FROM messages').n, + ]; + const before = count(); + for (const recommendation of ['yes: it matches the direction', 'maybe', undefined]) + assert.throws(() => raise(b, c, 'prd.approve', { recommendation }), /invalid-recommendation/); + const dup = [ + { key: 'yes', text: 'a' }, + { key: 'yes', text: 'b' }, + ]; + assert.throws(() => raise(b, c, 'prd.approve', { options: dup, recommendation: 'yes' }), /invalid-options/); + assert.throws(() => raise(b, c, 'prd.approve', { options: dup, recommendation: 'yes: a reason' }), /invalid-options/); + assert.throws( + () => raise(b, c, 'prd.approve', { options: [{ key: 'yes', text: 'a' }], recommendation: 'no' }), + /invalid-options/, + ); + const shaped = (o) => raise(b, c, 'prd.approve', { options: [{ key: 'yes', text: 'a' }, o], recommendation: 'no' }); + assert.throws(() => shaped({ key: 'no bad', text: 'b' }), /invalid-options/); + assert.throws(() => shaped({ key: 'no', text: '' }), /invalid-options/); + for (const o of [{ key: 'no' }, { key: 'no', text: 'b', why: 'c' }, 'no']) + assert.throws(() => shaped(o), /invalid-request/, 'a malformed option object is a malformed argument'); + assert.deepEqual(count(), before, 'a refused raise writes no decision and no message'); + assert.equal(call(b, c, 'inbox').length, 0); + const d = raise(b, c, 'prd.approve', { recommendation: 'yes' }); + assert.equal(store.get('SELECT recommendation FROM decisions WHERE id=?', d.id).recommendation, 'yes'); +}); test('observer capabilities read human inbox but cannot mutate or forge launch identity', (t) => { const { b, agent } = setup(t), c = agent('coder'); diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs index 6435140ad..d544befb1 100644 --- a/packages/harness/src/tools.mjs +++ b/packages/harness/src/tools.mjs @@ -134,12 +134,12 @@ export function typedTools({ resolved, arbiter = false, tracker = false, project } add("raise_decision", "decision.raise", null, "Ask for a decision before a gated or cross-role action, or when you need a choice made. The answer arrives as a message.", obj({ action: str("The action the decision is about", { enum: [...ACTIONS] }), - question: str("The question, with enough context to answer it"), + question: str("The question, with enough context to answer it and the reason for your recommendation"), options: { type: "array", minItems: 2, maxItems: 9, items: obj({ key: str("Short key"), text: str("What this option means") }, ["key", "text"]) }, - recommendation: str("Your recommended option key, with a reason"), + recommendation: str("Exactly one option key, as written in options, with nothing added. The reason goes in question"), blocking: { type: "boolean", description: "True only for a decision that holds a task: it needs task_ref, and the task waits for the answer. A request with no task is false" }, target: str("What the action applies to, such as a role instance or task reference"), - approvalChoice: str("The option key that approves the action"), + approvalChoice: str("The option key that approves the action. It defaults to yes, so pass it whenever no option key is yes"), task_ref: TASK_REF, requirement_ref: str("Requirement id"), }, ["action", "question", "options", "recommendation", "blocking"])); diff --git a/packages/harness/tests/tools.test.mjs b/packages/harness/tests/tools.test.mjs index a2bb20556..c70976301 100644 --- a/packages/harness/tests/tools.test.mjs +++ b/packages/harness/tests/tools.test.mjs @@ -50,6 +50,11 @@ test("list_requirements reads the requirement ids, for a task.create holder with const raise = tools.find((x) => x.name === "raise_decision").parameters.properties; assert.match(raise.blocking.description, /needs task_ref/); assert.match(raise.blocking.description, /A request with no task is false/); + assert.match(raise.recommendation.description, /^Exactly one option key, as written in options, with nothing added/); + assert.doesNotMatch(raise.recommendation.description, /with a reason/); + assert.match(raise.recommendation.description, /The reason goes in question/); + assert.match(raise.question.description, /the reason for your recommendation/); + assert.match(raise.approvalChoice.description, /defaults to yes, so pass it whenever no option key is yes/); }); test("a coder gets no launch, no resolve_decision, and no task tools without a tracker", (t) => {