Candidate on 96656dd21: broker split refuses invalid-recommendation after the
option checks, raise_decision text asks for one key and names the yes default,
bus README codes table, broker and tools tests. Mutants m1 (old shared line)
and m2 (order swapped) killed; full gate passed 15:42:03-15:47:36Z.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
110 lines
6.7 KiB
Diff
110 lines
6.7 KiB
Diff
diff --git a/packages/bus/README.md b/packages/bus/README.md
|
|
index 18f3039dd..88076348f 100644
|
|
--- a/packages/bus/README.md
|
|
+++ b/packages/bus/README.md
|
|
@@ -208,6 +208,16 @@ 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 refusal names one field:
|
|
+
|
|
+| Code | Field | Refused when |
|
|
+|---|---|---|
|
|
+| `invalid-options` | `options` | fewer than 2 or more than 9 options, an option that isn't `{key,text}` with a valid key and text, 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.
|
|
+
|
|
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..a3bb753b4 100644
|
|
--- a/packages/bus/tests/broker.test.mjs
|
|
+++ b/packages/bus/tests/broker.test.mjs
|
|
@@ -155,6 +155,32 @@ 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/,
|
|
+ );
|
|
+ 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) => {
|