jason.woltjeandClaude Opus 5.5
fee89d52fe
README invalid-options cell lists only count, key, text and duplicates; a
non-{key,text} option refuses invalid-request (Sage 28378, Filbert C1 28379).
Broker test pins the option shape cases. m1 and m2 killed; full gate passed
15:53:56-15:59:33Z on BASE 96656dd21.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
116 lines
7.4 KiB
Diff
116 lines
7.4 KiB
Diff
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) => {
|