From cc48b124f17caf712eab1a68a25ca52fa3727d55 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Wed, 14 Oct 2026 11:07:12 -0500 Subject: [PATCH] bus, harness: decision.raise takes one recommendation key and says which field failed (row 112, #1606) A recommendation that isn't exactly one option key now refuses invalid-recommendation, checked after the options, instead of sharing invalid-options with bad option lists. The raise_decision tool text says recommendation is one key with the reason in question, and that approvalChoice defaults to yes. The bus README lists the three codes, the field each names and the check order; a malformed option object refuses invalid-request. No schema change. Candidate manifest 013c82c2fd99d23d92f6bed543a788aa04ff97b164019ccb15252ae808b4138e (Darkwing round 2, packet fee89d52f), BASE 96656dd21. Approved by Filbert (comment 28383) and Sage (comment 28384). Sage gate rerun at f63fc20e4 plus the candidate passed. Co-Authored-By: Claude Opus 5.5 --- packages/bus/README.md | 11 ++++++++++ packages/bus/src/broker.mjs | 4 ++-- packages/bus/tests/broker.test.mjs | 31 +++++++++++++++++++++++++++ packages/harness/src/tools.mjs | 6 +++--- packages/harness/tests/tools.test.mjs | 5 +++++ 5 files changed, 52 insertions(+), 5 deletions(-) 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) => {