From bd1f11f6874bbd122943fc7f94ba897fc7f55bc2 Mon Sep 17 00:00:00 2001 From: Tom Owers Date: Wed, 2 Sep 2026 12:53:35 +0100 Subject: [PATCH] fix(orchestrator): name the model allow-list where enqueue_task picks it The `model` parameter on `enqueue_task` was declared as a bare optional string on both harness schemas, while `checkEnqueueGuards` rejects anything outside `VALID_MODELS`. The allow-list only appeared in the rejection message, so an agent had to guess a gateway model id, trip the `invalid-model` guard, and spend a turn reading the list back. Move the list into a shared `ENQUEUE_MODEL_DESCRIPTION` and describe the field with it on both the zod schema and the pi typebox mirror, so the guard and the description cannot disagree. Generated-By: PostHog Desktop Task-Id: 93b396ed-9a30-4d56-bd97-6bd31c232436 --- .../runner/harness/pi/orchestrator-tools.ts | 5 ++- .../__tests__/queue-tools.test.ts | 39 +++++++++++++++++++ .../sequence/orchestrator/queue-tools.ts | 16 +++++++- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/lib/agent/runner/harness/pi/orchestrator-tools.ts b/src/lib/agent/runner/harness/pi/orchestrator-tools.ts index 50b229dd5..0b200fb82 100644 --- a/src/lib/agent/runner/harness/pi/orchestrator-tools.ts +++ b/src/lib/agent/runner/harness/pi/orchestrator-tools.ts @@ -18,6 +18,7 @@ import { applyComplete, applyEnqueue, applyReadHandoffs, + ENQUEUE_MODEL_DESCRIPTION, HANDOFF_FIELDS, REMARK_ASK, type EnqueueArgs, @@ -86,7 +87,9 @@ export function createPiOrchestratorTools( description: 'Task ids that must be done before this task runs.', }), ), - model: Type.Optional(Type.String()), + model: Type.Optional( + Type.String({ description: ENQUEUE_MODEL_DESCRIPTION }), + ), reason: Type.String({ description: 'One line on why this task is needed.', }), diff --git a/src/lib/agent/runner/sequence/orchestrator/__tests__/queue-tools.test.ts b/src/lib/agent/runner/sequence/orchestrator/__tests__/queue-tools.test.ts index 5be5cf466..a33b32f6c 100644 --- a/src/lib/agent/runner/sequence/orchestrator/__tests__/queue-tools.test.ts +++ b/src/lib/agent/runner/sequence/orchestrator/__tests__/queue-tools.test.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { z } from 'zod'; import { analytics } from '@utils/analytics'; import { QueueStore } from '@lib/agent/runner/sequence/orchestrator/queue'; @@ -11,9 +12,15 @@ import { applyComplete, applyEnqueue, applyReadHandoffs, + buildOrchestratorTools, checkEnqueueGuards, + ENQUEUE_MODEL_DESCRIPTION, type OrchestratorToolsContext, } from '@lib/agent/runner/sequence/orchestrator/queue-tools'; +import { + isValidModel, + VALID_MODELS, +} from '@lib/agent/runner/switchboard/models'; function tmpDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), 'queue-tools-test-')); @@ -222,3 +229,35 @@ describe('apply functions', () => { expect(handoffs[0].did).toBe('installed'); }); }); + +/** + * The `invalid-model` guard rejects any model outside the allow-list, so an + * agent that cannot see the list has to trip the guard to learn it. The + * description is the only place it can read the list before it picks. + */ +describe('enqueue_task model description', () => { + it('lists exactly the models the guard accepts', () => { + const listed = ENQUEUE_MODEL_DESCRIPTION.split('one of: ')[1] + .replace(/\.$/, '') + .split(', '); + expect(listed.every(isValidModel)).toBe(true); + expect(listed.sort()).toEqual([...VALID_MODELS].sort()); + }); + + it('is carried by the model field of the MCP schema', () => { + const dir = tmpDir(); + try { + const schemas: Record[] = []; + buildOrchestratorTools( + (_name, _description, schema) => { + schemas.push(schema); + return null; + }, + { store: new QueueStore(dir, 'run-1'), validTypes: VALID }, + ); + expect(schemas[0].model.description).toBe(ENQUEUE_MODEL_DESCRIPTION); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/lib/agent/runner/sequence/orchestrator/queue-tools.ts b/src/lib/agent/runner/sequence/orchestrator/queue-tools.ts index 76fdc6a4e..24b3d9990 100644 --- a/src/lib/agent/runner/sequence/orchestrator/queue-tools.ts +++ b/src/lib/agent/runner/sequence/orchestrator/queue-tools.ts @@ -20,6 +20,20 @@ import { type TaskHandoff, } from './queue'; +/** + * The `enqueue_task` `model` description, shared by both harnesses' schemas. + * + * The field was declared as a bare optional string while + * {@link checkEnqueueGuards} rejects anything outside {@link VALID_MODELS}, so + * the allow-list existed only in the rejection message — an agent had to guess + * a gateway model id, trip the `invalid-model` guard, and spend a turn reading + * the list back before it could enqueue. Naming the list where the agent picks + * the value costs nothing and makes the guess unnecessary. + */ +export const ENQUEUE_MODEL_DESCRIPTION = `Optional model override for this task. Omit it to use the task's default, which is almost always right. If you do set it, it must be one of: ${[ + ...VALID_MODELS, +].join(', ')}.`; + /** The per-task remark ask, shared by both harnesses' complete_task schemas. */ export const REMARK_ASK = 'What information or guidance would have been useful to have in the integration prompt or documentation for this task — specifically anything that would have prevented tool failures, erroneous edits, or other wasted turns.'; @@ -432,7 +446,7 @@ export function buildOrchestratorTools( .array(z.string()) .optional() .describe('Task ids that must be done before this task runs.'), - model: z.string().optional(), + model: z.string().optional().describe(ENQUEUE_MODEL_DESCRIPTION), reason: z.string().describe('One line on why this task is needed.'), }, ((args: EnqueueArgs) => {