From 69bc0da50a4624feb6f813c83cb898e13ff809f5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 17:41:38 +0000 Subject: [PATCH 1/6] feat(orchestrate): allow env overrides for model role defaults Support ORCHESTRATE_MODEL_{WORKER,SUBPLANNER,VERIFIER,ROOT} so cost-efficient pairings can be set without editing MODEL_CATALOG. Values accept a catalog slug, bare model id, or JSON ModelSelection for out-of-catalog models. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 23 +++ orchestrate/skills/orchestrate/SKILL.md | 1 + .../orchestrate/references/dispatcher.md | 2 + .../scripts/__tests__/models-catalog.test.ts | 28 ++- .../__tests__/models-env-overrides.test.ts | 160 +++++++++++++++++ .../skills/orchestrate/scripts/cli/task.ts | 8 +- .../orchestrate/scripts/core/agent-manager.ts | 13 +- .../skills/orchestrate/scripts/models.ts | 166 +++++++++++++++++- 8 files changed, 384 insertions(+), 17 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts diff --git a/orchestrate/README.md b/orchestrate/README.md index 2cdece73..95a53cf9 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -10,6 +10,29 @@ The skill itself lives in [`skills/orchestrate/SKILL.md`](./skills/orchestrate/S - A Cursor API key in `CURSOR_API_KEY`. - Optional Slack app and bot token if you want a Slack thread mirroring the run. +## Model defaults (optional) + +When a task omits `tasks[].model`, orchestrate picks a catalog default per role. Override those with env vars (useful for cost-efficient pairings without editing the plugin): + +| Env var | Role | Example | +| --- | --- | --- | +| `ORCHESTRATE_MODEL_WORKER` | worker fallback | `composer-2-fast` | +| `ORCHESTRATE_MODEL_SUBPLANNER` | subplanner fallback | `claude-opus-4-8` | +| `ORCHESTRATE_MODEL_VERIFIER` | verifier fallback | `claude-opus-4-8` | +| `ORCHESTRATE_MODEL_ROOT` | kickoff `--model` default | `claude-opus-4-8` | + +Each value may be: + +1. A catalog slug (e.g. `composer-2-fast`) — resolved with the catalog's SDK params. +2. A bare model id (e.g. `composer-2.5`) — sent as `{ id }` (may fail if the backend needs params). +3. A JSON `ModelSelection` for models not in the catalog yet: + +```bash +export ORCHESTRATE_MODEL_WORKER='{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' +``` + +Explicit `tasks[].model` in the plan always wins over these env defaults. Run `bun cli.ts models` to see the catalog with env overrides applied. + ## Cursor API key 1. Open [https://cursor.com/dashboard/integrations](https://cursor.com/dashboard/integrations). diff --git a/orchestrate/skills/orchestrate/SKILL.md b/orchestrate/skills/orchestrate/SKILL.md index ff7fc8e3..59ea02eb 100644 --- a/orchestrate/skills/orchestrate/SKILL.md +++ b/orchestrate/skills/orchestrate/SKILL.md @@ -14,6 +14,7 @@ An explicit `/orchestrate ` fans out a large task across parallel Cursor c - `CURSOR_API_KEY` must be a personal/user key. Create it from [Cursor Dashboard > Integrations](https://cursor.com/dashboard/integrations), then read `cursor-sdk` Auth before using it. - `SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. +- Optional model defaults: `ORCHESTRATE_MODEL_WORKER`, `ORCHESTRATE_MODEL_SUBPLANNER`, `ORCHESTRATE_MODEL_VERIFIER`, and `ORCHESTRATE_MODEL_ROOT` override catalog fallbacks when `tasks[].model` / kickoff `--model` is omitted. Each value may be a catalog slug, a bare model id, or a JSON `ModelSelection` (e.g. `{"id":"composer-2.5"}`) for models not yet in the catalog. See the plugin README. ## Core principles diff --git a/orchestrate/skills/orchestrate/references/dispatcher.md b/orchestrate/skills/orchestrate/references/dispatcher.md index 54d18a64..d9e8ab4a 100644 --- a/orchestrate/skills/orchestrate/references/dispatcher.md +++ b/orchestrate/skills/orchestrate/references/dispatcher.md @@ -16,6 +16,8 @@ One-time setup: run `bun install` inside this skill's `scripts/` directory if `n bun cli.ts kickoff "" [--repo ] [--ref main] [--model claude-opus-4-8] [--slack-channel C123] [--dispatcher-name "Alex"] ``` +`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses `ORCHESTRATE_MODEL_ROOT`, else `claude-opus-4-8`. Role fallbacks for workers/subplanners/verifiers are similarly overridable via `ORCHESTRATE_MODEL_WORKER` / `_SUBPLANNER` / `_VERIFIER` (see the plugin README). + The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. ## Dispatcher identity diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts index 54b05338..f38fff63 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts @@ -1,12 +1,38 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { defaultModelForType, isKnownModel, MODEL_CATALOG, + MODEL_ENV_BY_TYPE, + MODEL_ENV_ROOT, resolveModelSelection, } from "../models.ts"; +const ENV_KEYS = [ + MODEL_ENV_BY_TYPE.worker, + MODEL_ENV_BY_TYPE.subplanner, + MODEL_ENV_BY_TYPE.verifier, + MODEL_ENV_ROOT, +] as const; + +const savedEnv: Record = {}; + +beforeEach(() => { + for (const key of ENV_KEYS) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } +}); + +afterEach(() => { + for (const key of ENV_KEYS) { + const prior = savedEnv[key]; + if (prior === undefined) delete process.env[key]; + else process.env[key] = prior; + } +}); + describe("MODEL_CATALOG", () => { test("every catalog entry passes isKnownModel", () => { for (const profile of MODEL_CATALOG) { diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts new file mode 100644 index 00000000..94cf2eb6 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts @@ -0,0 +1,160 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { + DEFAULT_ROOT_MODEL, + defaultModelForType, + defaultRootModel, + effectiveDefaultLabelForType, + formatModelSelectionLabel, + isKnownModel, + looksLikeSelectionJson, + MODEL_ENV_BY_TYPE, + MODEL_ENV_ROOT, + parseModelSelectionJson, + renderModelCatalog, + resolveModelSelection, +} from "../models.ts"; + +const ENV_KEYS = [ + MODEL_ENV_BY_TYPE.worker, + MODEL_ENV_BY_TYPE.subplanner, + MODEL_ENV_BY_TYPE.verifier, + MODEL_ENV_ROOT, +] as const; + +const savedEnv: Record = {}; + +function clearModelEnv(): void { + for (const key of ENV_KEYS) { + if (!(key in savedEnv)) savedEnv[key] = process.env[key]; + delete process.env[key]; + } +} + +afterEach(() => { + for (const key of ENV_KEYS) { + const prior = savedEnv[key]; + if (prior === undefined) delete process.env[key]; + else process.env[key] = prior; + delete savedEnv[key]; + } +}); + +describe("env model overrides", () => { + test("defaultModelForType uses catalog when env unset", () => { + clearModelEnv(); + expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); + expect(defaultModelForType("subplanner")).toBe( + "claude-opus-4-8-thinking-xhigh" + ); + expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); + }); + + test("catalog slug in env overrides defaultFor", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; + expect(defaultModelForType("worker")).toBe("composer-2-fast"); + expect(resolveModelSelection(defaultModelForType("worker"))).toEqual({ + id: "composer-2", + params: [{ id: "fast", value: "true" }], + }); + // Other roles untouched. + expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); + }); + + test("whitespace-only env is treated as unset", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = " "; + expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); + }); + + test("bare unknown id in env passes through to { id }", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; + const spec = defaultModelForType("worker"); + expect(spec).toBe("composer-2.5"); + expect(isKnownModel(spec)).toBe(false); + expect(resolveModelSelection(spec)).toEqual({ id: "composer-2.5" }); + }); + + test("JSON ModelSelection in env resolves with params", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = + '{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}'; + const spec = defaultModelForType("worker"); + expect(looksLikeSelectionJson(spec)).toBe(true); + expect(resolveModelSelection(spec)).toEqual({ + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + }); + }); + + test("JSON ModelSelection without params is valid", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_SUBPLANNER = '{"id":"grok-4-5"}'; + expect(resolveModelSelection(defaultModelForType("subplanner"))).toEqual({ + id: "grok-4-5", + }); + }); + + test("invalid JSON ModelSelection throws a clear error", () => { + expect(() => parseModelSelectionJson("{not-json")).toThrow( + /invalid model selection JSON/ + ); + expect(() => parseModelSelectionJson('{"params":[]}')).toThrow( + /expected \{"id"/ + ); + expect(() => + parseModelSelectionJson('{"id":"x","params":[{"id":1,"value":"a"}]}') + ).toThrow(/each params entry/); + }); + + test("defaultRootModel reads ORCHESTRATE_MODEL_ROOT", () => { + clearModelEnv(); + expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); + process.env.ORCHESTRATE_MODEL_ROOT = "composer-2-fast"; + expect(defaultRootModel()).toBe("composer-2-fast"); + process.env.ORCHESTRATE_MODEL_ROOT = '{"id":"composer-2.5"}'; + expect(resolveModelSelection(defaultRootModel())).toEqual({ + id: "composer-2.5", + }); + }); + + test("renderModelCatalog reflects catalog-slug env defaults", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; + const text = renderModelCatalog(); + expect(text).toContain("`composer-2-fast` —"); + expect(text).toContain("(default for worker)"); + // Former worker default should no longer claim worker. + const gptLine = text + .split("\n") + .find(l => l.includes("`gpt-5.5-high-fast`")); + expect(gptLine).toBeDefined(); + expect(gptLine).not.toContain("default for worker"); + }); + + test("renderModelCatalog lists non-catalog env overrides separately", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_WORKER = '{"id":"composer-2.5"}'; + const text = renderModelCatalog(); + expect(text).toContain("Env default overrides"); + expect(text).toContain("worker: `composer-2.5`"); + expect(text).toContain("ORCHESTRATE_MODEL_WORKER"); + }); + + test("effectiveDefaultLabelForType formats JSON selections", () => { + clearModelEnv(); + process.env.ORCHESTRATE_MODEL_VERIFIER = + '{"id":"claude-opus-4-8","params":[{"id":"effort","value":"high"}]}'; + expect(effectiveDefaultLabelForType("verifier")).toBe( + "claude-opus-4-8 (effort=high)" + ); + expect( + formatModelSelectionLabel({ + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + }) + ).toBe("composer-2.5 (fast=true)"); + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 3093c913..47232f04 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -4,7 +4,7 @@ import type { CursorAgentError as CursorAgentErrorValue } from "@cursor/sdk"; import type { Command } from "commander"; import { createSlackAdapter } from "../adapters/index.ts"; import { DEFAULT_MAX_RUNTIME_SEC, runOrchestrateLoop } from "../core/loop.ts"; -import { resolveModelSelection } from "../models.ts"; +import { defaultRootModel, resolveModelSelection } from "../models.ts"; import { parsePlanTaskJson, parsePlanTaskValue, @@ -108,7 +108,11 @@ export function registerTaskCommands(program: Command): void { .argument("", "Root orchestration goal") .option("--repo ", "Repository URL to orchestrate") .option("--ref ", "Starting git ref for the cloud workspace", "main") - .option("--model ", "Model id for the root planner", "claude-opus-4-8") + .option( + "--model ", + "Model id for the root planner (catalog slug, bare id, or JSON ModelSelection). Defaults to ORCHESTRATE_MODEL_ROOT, else claude-opus-4-8.", + defaultRootModel() + ) .option( "--force", "Spawn a new root planner even when a recent matching run exists." diff --git a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts index 32ce7199..dafcf3d2 100644 --- a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts +++ b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts @@ -140,6 +140,7 @@ import { import { defaultModelForType, isKnownModel, + looksLikeSelectionJson, resolveModelSelection, } from "../models.ts"; import { AndonPoller, SlackReactionAndonSource } from "./andon.ts"; @@ -618,9 +619,13 @@ export class AgentManager { const attemptNumber = attemptsBefore + 1; this.touch(s, { attempts: attemptNumber }); - if (def.model && !isKnownModel(def.model)) { + const modelSpec = def.model ?? defaultModelForType(def.type); + // JSON ModelSelection overrides intentionally bypass the catalog. Bare + // unknown slugs still warn — they fall through to `{ id }` and may + // `invalid_model` if the backend needs params. + if (!looksLikeSelectionJson(modelSpec) && !isKnownModel(modelSpec)) { this.logAttention( - `${def.name}: model "${def.model}" not in MODEL_CATALOG (spawning anyway). Run \`bun cli.ts models\` to see known slugs.` + `${def.name}: model "${modelSpec}" not in MODEL_CATALOG (spawning anyway). Run \`bun cli.ts models\` to see known slugs, or set a JSON ModelSelection via ORCHESTRATE_MODEL_* / tasks[].model.` ); } @@ -634,9 +639,7 @@ export class AgentManager { const agent = await Agent.create({ apiKey: this.apiKey, name: `${this.plan.rootSlug}/${def.name}`, - model: resolveModelSelection( - def.model ?? defaultModelForType(def.type) - ), + model: resolveModelSelection(modelSpec), cloud: { repos: [{ url: this.plan.repoUrl, startingRef }], autoCreatePR: def.openPR ?? false, diff --git a/orchestrate/skills/orchestrate/scripts/models.ts b/orchestrate/skills/orchestrate/scripts/models.ts index de71c926..82f141df 100644 --- a/orchestrate/skills/orchestrate/scripts/models.ts +++ b/orchestrate/skills/orchestrate/scripts/models.ts @@ -3,7 +3,8 @@ import type { ModelSelection } from "@cursor/sdk"; import type { TaskType } from "./adapters/types.ts"; // Model catalog. Source of truth for `tasks[].model` choices; `defaultFor` -// entries supply the fallback when `tasks[].model` is omitted. +// entries supply the fallback when `tasks[].model` is omitted. Per-role env +// vars (see MODEL_ENV_BY_TYPE) override those catalog defaults at runtime. export interface ModelProfile { /** User-facing slug for `tasks[].model` and `--model` flags. */ @@ -18,6 +19,18 @@ export interface ModelProfile { defaultFor?: TaskType[]; } +/** Env vars that override catalog `defaultFor` when set (non-empty). */ +export const MODEL_ENV_BY_TYPE: Record = { + worker: "ORCHESTRATE_MODEL_WORKER", + subplanner: "ORCHESTRATE_MODEL_SUBPLANNER", + verifier: "ORCHESTRATE_MODEL_VERIFIER", +}; + +/** Env var that overrides the kickoff `--model` default for the root planner. */ +export const MODEL_ENV_ROOT = "ORCHESTRATE_MODEL_ROOT"; + +export const DEFAULT_ROOT_MODEL = "claude-opus-4-8"; + // `slug` is the stable authoring name; `selection` is the canonical SDK form. // Run `bun cli.ts models --check` after SDK or backend model-schema drift. export const MODEL_CATALOG: ModelProfile[] = [ @@ -164,31 +177,166 @@ export const MODEL_CATALOG: ModelProfile[] = [ }, ]; +function readEnvOverride(name: string): string | undefined { + const raw = process.env[name]?.trim(); + return raw || undefined; +} + +/** True when `spec` looks like a JSON ModelSelection object literal. */ +export function looksLikeSelectionJson(spec: string): boolean { + return spec.trimStart().startsWith("{"); +} + +/** + * Parse a dual-format model override: catalog slug, bare model id, or JSON + * `ModelSelection` (`{"id":"…","params":[…]}`). + * + * JSON form is for models not yet in MODEL_CATALOG (e.g. composer-2.5) where + * a bare `{ id: slug }` would lose required params or fail `invalid_model`. + */ +export function parseModelSelectionJson(raw: string): ModelSelection { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new Error( + `invalid model selection JSON: ${err instanceof Error ? err.message : String(err)}` + ); + } + if ( + parsed === null || + typeof parsed !== "object" || + Array.isArray(parsed) || + typeof (parsed as { id?: unknown }).id !== "string" || + !(parsed as { id: string }).id.trim() + ) { + throw new Error( + 'invalid model selection JSON: expected {"id":"", "params"?: [{"id","value"}, ...]}' + ); + } + const obj = parsed as { id: string; params?: unknown }; + const selection: ModelSelection = { id: obj.id.trim() }; + if (obj.params !== undefined) { + if (!Array.isArray(obj.params)) { + throw new Error( + 'invalid model selection JSON: "params" must be an array of {id, value}' + ); + } + const params: { id: string; value: string }[] = []; + for (const p of obj.params) { + if ( + p === null || + typeof p !== "object" || + typeof (p as { id?: unknown }).id !== "string" || + typeof (p as { value?: unknown }).value !== "string" + ) { + throw new Error( + "invalid model selection JSON: each params entry must be {id: string, value: string}" + ); + } + params.push({ + id: (p as { id: string }).id, + value: (p as { value: string }).value, + }); + } + selection.params = params; + } + return selection; +} + +/** Compact label for catalog / attention logs. */ +export function formatModelSelectionLabel(selection: ModelSelection): string { + if (!selection.params?.length) return selection.id; + const params = selection.params.map(p => `${p.id}=${p.value}`).join(", "); + return `${selection.id} (${params})`; +} + +/** + * Fallback model spec when `tasks[].model` is omitted. + * Returns a catalog slug, bare id, or JSON ModelSelection string from env. + */ export function defaultModelForType(type: TaskType): string { + const fromEnv = readEnvOverride(MODEL_ENV_BY_TYPE[type]); + if (fromEnv) return fromEnv; + const match = MODEL_CATALOG.find(m => m.defaultFor?.includes(type)); if (!match) throw new Error(`MODEL_CATALOG missing default for TaskType "${type}"`); return match.slug; } +/** Kickoff `--model` default: ORCHESTRATE_MODEL_ROOT, else catalog root default. */ +export function defaultRootModel(): string { + return readEnvOverride(MODEL_ENV_ROOT) ?? DEFAULT_ROOT_MODEL; +} + export function isKnownModel(slug: string): boolean { + if (looksLikeSelectionJson(slug)) return false; return MODEL_CATALOG.some(m => m.slug === slug); } -/** Unknown slugs pass through as a bare `{ id }` so planners can reach - * server-side models that aren't in our prescriptive catalog. */ -export function resolveModelSelection(slug: string): ModelSelection { - const profile = MODEL_CATALOG.find(m => m.slug === slug); - return profile ? profile.selection : { id: slug }; +/** + * Resolve an authoring slug, bare model id, or JSON ModelSelection into the + * canonical SDK form passed to `Agent.create({ model })`. + */ +export function resolveModelSelection(spec: string): ModelSelection { + const trimmed = spec.trim(); + if (looksLikeSelectionJson(trimmed)) { + return parseModelSelectionJson(trimmed); + } + const profile = MODEL_CATALOG.find(m => m.slug === trimmed); + return profile ? profile.selection : { id: trimmed }; +} + +/** Effective default label per task type (catalog or env override). */ +export function effectiveDefaultLabelForType(type: TaskType): string { + const spec = defaultModelForType(type); + if (looksLikeSelectionJson(spec)) { + return formatModelSelectionLabel(parseModelSelectionJson(spec)); + } + return spec; } export function renderModelCatalog(): string { + const types: TaskType[] = ["worker", "subplanner", "verifier"]; + const effectiveByType = new Map(); + for (const type of types) { + effectiveByType.set(type, effectiveDefaultLabelForType(type)); + } + + // Map catalog slug → roles that currently default to it (after env overrides). + const defaultsBySlug = new Map(); + const nonCatalogDefaults: { type: TaskType; label: string }[] = []; + for (const type of types) { + const label = effectiveByType.get(type); + if (label === undefined) continue; + if (isKnownModel(label)) { + const list = defaultsBySlug.get(label) ?? []; + list.push(type); + defaultsBySlug.set(label, list); + } else { + nonCatalogDefaults.push({ type, label }); + } + } + const lines: string[] = []; + if (nonCatalogDefaults.length) { + lines.push( + "Env default overrides (not in MODEL_CATALOG; used when `tasks[].model` is omitted):" + ); + for (const { type, label } of nonCatalogDefaults) { + const envName = MODEL_ENV_BY_TYPE[type]; + lines.push(`- ${type}: \`${label}\` (from ${envName})`); + } + lines.push(""); + } + for (const m of MODEL_CATALOG) { - const defaults = m.defaultFor?.length - ? ` (default for ${m.defaultFor.join(", ")})` + const defaults = defaultsBySlug.get(m.slug); + const defaultsNote = defaults?.length + ? ` (default for ${defaults.join(", ")})` : ""; - lines.push(`- \`${m.slug}\` — ${m.summary}${defaults}`); + lines.push(`- \`${m.slug}\` — ${m.summary}${defaultsNote}`); lines.push(` speed: ${m.speed}; strengths: ${m.strengths.join(", ")}`); lines.push(` use: ${m.use}`); } From bff5119e743b150d2ae2aa0911f2444324c8670b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 17:54:45 +0000 Subject: [PATCH 2/6] feat(orchestrate): build effective model catalog from env config Merge MODEL_CATALOG with ORCHESTRATE_MODEL_* env config so planners select from the operator's list rather than the hardcoded constant. Env-named models join the catalog by slug and round-trip through resolveModelSelection. Adds ORCHESTRATE_MODEL_CATALOG for extra entries and ORCHESTRATE_MODEL_CATALOG_MODE=env-only to drop the built-ins entirely, so a team can publish an exact menu per task type. Config errors fail fast at CLI startup instead of surfacing mid-run as a spawn failure. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 65 ++- orchestrate/skills/orchestrate/SKILL.md | 2 +- .../skills/orchestrate/prompts/subplanner.md | 2 +- .../orchestrate/references/dispatcher.md | 2 +- .../__tests__/models-env-overrides.test.ts | 244 +++++++-- .../skills/orchestrate/scripts/cli/index.ts | 11 + .../skills/orchestrate/scripts/models.ts | 487 ++++++++++++++---- .../orchestrate/scripts/tools/probe-models.ts | 4 +- 8 files changed, 648 insertions(+), 169 deletions(-) diff --git a/orchestrate/README.md b/orchestrate/README.md index 95a53cf9..dc5dafeb 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -10,28 +10,65 @@ The skill itself lives in [`skills/orchestrate/SKILL.md`](./skills/orchestrate/S - A Cursor API key in `CURSOR_API_KEY`. - Optional Slack app and bot token if you want a Slack thread mirroring the run. -## Model defaults (optional) +## Model configuration (optional) -When a task omits `tasks[].model`, orchestrate picks a catalog default per role. Override those with env vars (useful for cost-efficient pairings without editing the plugin): +The built-in `MODEL_CATALOG` is merged with environment config into an **effective catalog**. That merged catalog is what planners see when they set `tasks[].model`, what `bun cli.ts models` prints, and what supplies role defaults when `tasks[].model` is omitted. Use it to steer cost without editing the plugin. -| Env var | Role | Example | -| --- | --- | --- | -| `ORCHESTRATE_MODEL_WORKER` | worker fallback | `composer-2-fast` | -| `ORCHESTRATE_MODEL_SUBPLANNER` | subplanner fallback | `claude-opus-4-8` | -| `ORCHESTRATE_MODEL_VERIFIER` | verifier fallback | `claude-opus-4-8` | -| `ORCHESTRATE_MODEL_ROOT` | kickoff `--model` default | `claude-opus-4-8` | +### Role defaults -Each value may be: +| Env var | Role | +| --- | --- | +| `ORCHESTRATE_MODEL_WORKER` | worker default | +| `ORCHESTRATE_MODEL_SUBPLANNER` | subplanner default | +| `ORCHESTRATE_MODEL_VERIFIER` | verifier default | +| `ORCHESTRATE_MODEL_ROOT` | kickoff `--model` default | -1. A catalog slug (e.g. `composer-2-fast`) — resolved with the catalog's SDK params. -2. A bare model id (e.g. `composer-2.5`) — sent as `{ id }` (may fail if the backend needs params). -3. A JSON `ModelSelection` for models not in the catalog yet: +Each value may be a catalog slug (`composer-2-fast`), a bare model id (`composer-2.5`), or a JSON entry: ```bash -export ORCHESTRATE_MODEL_WORKER='{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' +export ORCHESTRATE_MODEL_WORKER=composer-2-fast +export ORCHESTRATE_MODEL_SUBPLANNER='{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' ``` -Explicit `tasks[].model` in the plan always wins over these env defaults. Run `bun cli.ts models` to see the catalog with env overrides applied. +Anything named this way joins the effective catalog, so planners can select it by slug and it resolves back to the full selection (params included). A JSON entry may also carry `slug`, `summary`, `use`, `speed`, and `strengths` — worth setting, since planners choose by the capability prose: + +```bash +export ORCHESTRATE_MODEL_WORKER='{"slug":"house-worker","id":"composer-2.5","summary":"House worker model.","use":"Use for all bounded implementation work.","speed":"fast","strengths":["throughput"]}' +``` + +### Adding models + +`ORCHESTRATE_MODEL_CATALOG` takes a JSON array of entries. Entries with an `id` define a model; entries with only a `slug` pull in a built-in by reference. `defaultFor` claims a role. + +```bash +export ORCHESTRATE_MODEL_CATALOG='[ + {"id":"composer-2.5","summary":"Cheap, fast worker.","defaultFor":["worker"]}, + {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} +]' +``` + +### Restricting the menu + +`ORCHESTRATE_MODEL_CATALOG_MODE=env-only` drops the built-in catalog, so planners see exactly the models you list (built-ins can be pulled back in by slug). Every role needs a default in this mode; the CLI exits with a config error if one is missing. + +```bash +export ORCHESTRATE_MODEL_CATALOG_MODE=env-only +export ORCHESTRATE_MODEL_CATALOG='[ + {"slug":"composer-2-fast","defaultFor":["worker"]}, + {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} +]' +``` + +### Precedence and limits + +1. Explicit `tasks[].model` in the plan +2. Role env var (`ORCHESTRATE_MODEL_`) +3. `defaultFor` on an `ORCHESTRATE_MODEL_CATALOG` entry +4. `defaultFor` in the built-in catalog + +Run `bun cli.ts models` to see the effective catalog, and `bun cli.ts models --check` to probe every entry (including your env-defined ones) against `/v1/agents`. + +Two caveats. This shapes what planners choose from, but a planner can still write any model id into `tasks[].model`, so it is guidance rather than a spend ceiling. And each spawned agent reads its own environment: set these as Cursor Cloud secrets for the repo so subplanners and workers inherit them, not just in the dispatcher's local shell. ## Cursor API key diff --git a/orchestrate/skills/orchestrate/SKILL.md b/orchestrate/skills/orchestrate/SKILL.md index 59ea02eb..ab9cd188 100644 --- a/orchestrate/skills/orchestrate/SKILL.md +++ b/orchestrate/skills/orchestrate/SKILL.md @@ -14,7 +14,7 @@ An explicit `/orchestrate ` fans out a large task across parallel Cursor c - `CURSOR_API_KEY` must be a personal/user key. Create it from [Cursor Dashboard > Integrations](https://cursor.com/dashboard/integrations), then read `cursor-sdk` Auth before using it. - `SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. -- Optional model defaults: `ORCHESTRATE_MODEL_WORKER`, `ORCHESTRATE_MODEL_SUBPLANNER`, `ORCHESTRATE_MODEL_VERIFIER`, and `ORCHESTRATE_MODEL_ROOT` override catalog fallbacks when `tasks[].model` / kickoff `--model` is omitted. Each value may be a catalog slug, a bare model id, or a JSON `ModelSelection` (e.g. `{"id":"composer-2.5"}`) for models not yet in the catalog. See the plugin README. +- Model config is optional. `ORCHESTRATE_MODEL_WORKER`, `_SUBPLANNER`, `_VERIFIER`, and `_ROOT` set role defaults; `ORCHESTRATE_MODEL_CATALOG` adds entries; `ORCHESTRATE_MODEL_CATALOG_MODE=env-only` restricts planners to exactly the listed models. All of it merges into the catalog `bun cli.ts models` prints, which is the list planners pick `tasks[].model` from. See the plugin README. ## Core principles diff --git a/orchestrate/skills/orchestrate/prompts/subplanner.md b/orchestrate/skills/orchestrate/prompts/subplanner.md index ea4814b7..6df220f3 100644 --- a/orchestrate/skills/orchestrate/prompts/subplanner.md +++ b/orchestrate/skills/orchestrate/prompts/subplanner.md @@ -30,7 +30,7 @@ Paths you must NOT modify (owned by siblings): Acceptance criteria for your subtree: {{accept}}{{verifyPlan}}{{upstream}} -Model selection: pick `tasks[].model` per task by capability. Available models: +Model selection: pick `tasks[].model` per task by capability, choosing only from the list below. That list is this repo's effective catalog, not a generic menu. Omit `tasks[].model` to accept the marked default for that task type; set it explicitly when the task needs a different capability. {{modelCatalog}} diff --git a/orchestrate/skills/orchestrate/references/dispatcher.md b/orchestrate/skills/orchestrate/references/dispatcher.md index d9e8ab4a..c2a54d27 100644 --- a/orchestrate/skills/orchestrate/references/dispatcher.md +++ b/orchestrate/skills/orchestrate/references/dispatcher.md @@ -16,7 +16,7 @@ One-time setup: run `bun install` inside this skill's `scripts/` directory if `n bun cli.ts kickoff "" [--repo ] [--ref main] [--model claude-opus-4-8] [--slack-channel C123] [--dispatcher-name "Alex"] ``` -`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses `ORCHESTRATE_MODEL_ROOT`, else `claude-opus-4-8`. Role fallbacks for workers/subplanners/verifiers are similarly overridable via `ORCHESTRATE_MODEL_WORKER` / `_SUBPLANNER` / `_VERIFIER` (see the plugin README). +`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses `ORCHESTRATE_MODEL_ROOT`, else `claude-opus-4-8`. Role defaults and the catalog planners choose from are configurable via `ORCHESTRATE_MODEL_WORKER` / `_SUBPLANNER` / `_VERIFIER`, `ORCHESTRATE_MODEL_CATALOG`, and `ORCHESTRATE_MODEL_CATALOG_MODE` (see the plugin README). Run `bun cli.ts models` to print the effective catalog; a bad config exits 2 with the offending env var named. The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts index 94cf2eb6..962cd97a 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts @@ -1,15 +1,19 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { + assertModelEnvConfig, + buildEffectiveCatalog, DEFAULT_ROOT_MODEL, defaultModelForType, defaultRootModel, - effectiveDefaultLabelForType, + effectiveModelCatalog, formatModelSelectionLabel, isKnownModel, - looksLikeSelectionJson, MODEL_ENV_BY_TYPE, + MODEL_ENV_CATALOG, + MODEL_ENV_CATALOG_MODE, MODEL_ENV_ROOT, + ModelConfigError, parseModelSelectionJson, renderModelCatalog, resolveModelSelection, @@ -20,29 +24,29 @@ const ENV_KEYS = [ MODEL_ENV_BY_TYPE.subplanner, MODEL_ENV_BY_TYPE.verifier, MODEL_ENV_ROOT, + MODEL_ENV_CATALOG, + MODEL_ENV_CATALOG_MODE, ] as const; const savedEnv: Record = {}; -function clearModelEnv(): void { +beforeEach(() => { for (const key of ENV_KEYS) { - if (!(key in savedEnv)) savedEnv[key] = process.env[key]; + savedEnv[key] = process.env[key]; delete process.env[key]; } -} +}); afterEach(() => { for (const key of ENV_KEYS) { const prior = savedEnv[key]; if (prior === undefined) delete process.env[key]; else process.env[key] = prior; - delete savedEnv[key]; } }); -describe("env model overrides", () => { - test("defaultModelForType uses catalog when env unset", () => { - clearModelEnv(); +describe("per-role env overrides", () => { + test("catalog defaults apply when env is unset", () => { expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); expect(defaultModelForType("subplanner")).toBe( "claude-opus-4-8-thinking-xhigh" @@ -51,53 +55,63 @@ describe("env model overrides", () => { }); test("catalog slug in env overrides defaultFor", () => { - clearModelEnv(); process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; expect(defaultModelForType("worker")).toBe("composer-2-fast"); - expect(resolveModelSelection(defaultModelForType("worker"))).toEqual({ + expect(resolveModelSelection("composer-2-fast")).toEqual({ id: "composer-2", params: [{ id: "fast", value: "true" }], }); - // Other roles untouched. expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); }); test("whitespace-only env is treated as unset", () => { - clearModelEnv(); process.env.ORCHESTRATE_MODEL_WORKER = " "; expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); }); - test("bare unknown id in env passes through to { id }", () => { - clearModelEnv(); + test("bare unknown id joins the catalog and round-trips", () => { process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; - const spec = defaultModelForType("worker"); - expect(spec).toBe("composer-2.5"); - expect(isKnownModel(spec)).toBe(false); - expect(resolveModelSelection(spec)).toEqual({ id: "composer-2.5" }); + expect(defaultModelForType("worker")).toBe("composer-2.5"); + // Merged in, so a planner copying the slug resolves back to the same model. + expect(isKnownModel("composer-2.5")).toBe(true); + expect(resolveModelSelection("composer-2.5")).toEqual({ + id: "composer-2.5", + }); }); - test("JSON ModelSelection in env resolves with params", () => { - clearModelEnv(); + test("JSON entry keeps params and gets a slug", () => { process.env.ORCHESTRATE_MODEL_WORKER = '{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}'; - const spec = defaultModelForType("worker"); - expect(looksLikeSelectionJson(spec)).toBe(true); - expect(resolveModelSelection(spec)).toEqual({ + const slug = defaultModelForType("worker"); + expect(slug).toBe("composer-2.5"); + expect(resolveModelSelection(slug)).toEqual({ id: "composer-2.5", params: [{ id: "fast", value: "true" }], }); }); - test("JSON ModelSelection without params is valid", () => { - clearModelEnv(); - process.env.ORCHESTRATE_MODEL_SUBPLANNER = '{"id":"grok-4-5"}'; - expect(resolveModelSelection(defaultModelForType("subplanner"))).toEqual({ - id: "grok-4-5", + test("JSON entry can name its own slug and prose", () => { + process.env.ORCHESTRATE_MODEL_WORKER = JSON.stringify({ + slug: "composer-cheap", + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + summary: "House worker model.", + use: "Use for all bounded implementation work.", + speed: "fast", + strengths: ["throughput"], + }); + expect(defaultModelForType("worker")).toBe("composer-cheap"); + expect(resolveModelSelection("composer-cheap")).toEqual({ + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], }); + const text = renderModelCatalog(); + expect(text).toContain("`composer-cheap` — House worker model."); + expect(text).toContain("(default for worker)"); + expect(text).toContain("speed: fast; strengths: throughput"); }); - test("invalid JSON ModelSelection throws a clear error", () => { + test("invalid JSON selection throws a clear error", () => { expect(() => parseModelSelectionJson("{not-json")).toThrow( /invalid model selection JSON/ ); @@ -110,7 +124,6 @@ describe("env model overrides", () => { }); test("defaultRootModel reads ORCHESTRATE_MODEL_ROOT", () => { - clearModelEnv(); expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); process.env.ORCHESTRATE_MODEL_ROOT = "composer-2-fast"; expect(defaultRootModel()).toBe("composer-2-fast"); @@ -120,36 +133,20 @@ describe("env model overrides", () => { }); }); - test("renderModelCatalog reflects catalog-slug env defaults", () => { - clearModelEnv(); + test("rendered catalog moves the default label to the override", () => { process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; const text = renderModelCatalog(); - expect(text).toContain("`composer-2-fast` —"); - expect(text).toContain("(default for worker)"); - // Former worker default should no longer claim worker. + const composerLine = text + .split("\n") + .find(l => l.includes("`composer-2-fast`")); const gptLine = text .split("\n") .find(l => l.includes("`gpt-5.5-high-fast`")); - expect(gptLine).toBeDefined(); + expect(composerLine).toContain("(default for worker)"); expect(gptLine).not.toContain("default for worker"); }); - test("renderModelCatalog lists non-catalog env overrides separately", () => { - clearModelEnv(); - process.env.ORCHESTRATE_MODEL_WORKER = '{"id":"composer-2.5"}'; - const text = renderModelCatalog(); - expect(text).toContain("Env default overrides"); - expect(text).toContain("worker: `composer-2.5`"); - expect(text).toContain("ORCHESTRATE_MODEL_WORKER"); - }); - - test("effectiveDefaultLabelForType formats JSON selections", () => { - clearModelEnv(); - process.env.ORCHESTRATE_MODEL_VERIFIER = - '{"id":"claude-opus-4-8","params":[{"id":"effort","value":"high"}]}'; - expect(effectiveDefaultLabelForType("verifier")).toBe( - "claude-opus-4-8 (effort=high)" - ); + test("formatModelSelectionLabel renders params", () => { expect( formatModelSelectionLabel({ id: "composer-2.5", @@ -158,3 +155,142 @@ describe("env model overrides", () => { ).toBe("composer-2.5 (fast=true)"); }); }); + +describe("ORCHESTRATE_MODEL_CATALOG", () => { + test("adds entries alongside the built-in catalog", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", summary: "Cheap worker." }, + ]); + const slugs = effectiveModelCatalog().map(m => m.slug); + expect(slugs).toContain("composer-2.5"); + expect(slugs).toContain("gpt-5.5-high-fast"); + }); + + test("entry defaultFor claims a role", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", defaultFor: ["worker"] }, + ]); + expect(defaultModelForType("worker")).toBe("composer-2.5"); + }); + + test("role env var wins over an entry defaultFor", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", defaultFor: ["worker"] }, + ]); + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; + expect(defaultModelForType("worker")).toBe("composer-2-fast"); + }); + + test("two entries claiming one role is a config error", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", defaultFor: ["worker"] }, + { id: "grok-4-5", defaultFor: ["worker"] }, + ]); + expect(() => buildEffectiveCatalog()).toThrow(/claim the worker default/); + }); + + test("entry can override a built-in slug's selection", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { slug: "composer-2-fast", id: "composer-2.5" }, + ]); + expect(resolveModelSelection("composer-2-fast")).toEqual({ + id: "composer-2.5", + }); + }); + + test("slug-only entry referencing an unknown model is rejected", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { slug: "not-a-builtin" }, + ]); + expect(() => buildEffectiveCatalog()).toThrow( + /not a built-in MODEL_CATALOG slug/ + ); + }); + + test("malformed config surfaces as ModelConfigError", () => { + process.env.ORCHESTRATE_MODEL_CATALOG = '{"id":"x"}'; + expect(() => buildEffectiveCatalog()).toThrow(ModelConfigError); + expect(() => buildEffectiveCatalog()).toThrow(/expected a JSON array/); + + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "x", speed: "blistering" }, + ]); + expect(() => buildEffectiveCatalog()).toThrow(/"speed" must be/); + + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "x", defaultFor: ["planner"] }, + ]); + expect(() => buildEffectiveCatalog()).toThrow(/"defaultFor" entries/); + }); +}); + +describe("ORCHESTRATE_MODEL_CATALOG_MODE=env-only", () => { + test("drops built-ins so only listed models are published", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", defaultFor: ["worker"] }, + { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, + ]); + const slugs = effectiveModelCatalog().map(m => m.slug); + expect(slugs).toEqual(["composer-2.5", "claude-opus-4-8"]); + expect(slugs).not.toContain("gpt-5.5-high-fast"); + expect(isKnownModel("gpt-5.5-high-fast")).toBe(false); + }); + + test("slug reference pulls a built-in back in with its params", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { slug: "composer-2-fast", defaultFor: ["worker"] }, + ]); + process.env.ORCHESTRATE_MODEL_SUBPLANNER = "claude-opus-4-8"; + process.env.ORCHESTRATE_MODEL_VERIFIER = "claude-opus-4-8"; + expect(resolveModelSelection("composer-2-fast")).toEqual({ + id: "composer-2", + params: [{ id: "fast", value: "true" }], + }); + expect(defaultModelForType("worker")).toBe("composer-2-fast"); + expect(defaultModelForType("subplanner")).toBe("claude-opus-4-8"); + }); + + test("rendered catalog tells planners the menu is exact", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; + process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + { id: "composer-2.5", defaultFor: ["worker", "subplanner", "verifier"] }, + ]); + const text = renderModelCatalog(); + expect(text).toContain("exact model menu"); + expect(text).not.toContain("gpt-5.5-high-fast"); + }); + + test("missing role default fails fast with an actionable message", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; + expect(() => assertModelEnvConfig()).toThrow(ModelConfigError); + expect(() => assertModelEnvConfig()).toThrow( + /leaves no default for subplanner, verifier/ + ); + expect(() => defaultModelForType("verifier")).toThrow( + /ORCHESTRATE_MODEL_VERIFIER/ + ); + }); + + test("assertModelEnvConfig passes when every role resolves", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; + process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; + process.env.ORCHESTRATE_MODEL_SUBPLANNER = "claude-opus-4-8"; + process.env.ORCHESTRATE_MODEL_VERIFIER = "claude-opus-4-8"; + expect(() => assertModelEnvConfig()).not.toThrow(); + }); + + test("unknown mode value is rejected", () => { + process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "exclusive"; + expect(() => buildEffectiveCatalog()).toThrow(/is not valid/); + }); + + test("merge mode is the default and keeps built-ins", () => { + expect(buildEffectiveCatalog().mode).toBe("merge"); + expect(effectiveModelCatalog().map(m => m.slug)).toContain( + "gpt-5.5-high-fast" + ); + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/cli/index.ts b/orchestrate/skills/orchestrate/scripts/cli/index.ts index eca2f1af..93df3fa1 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/index.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/index.ts @@ -1,6 +1,7 @@ #!/usr/bin/env bun import { Command } from "commander"; +import { assertModelEnvConfig, ModelConfigError } from "../models.ts"; import { registerAndonCommands } from "./andon.ts"; import { registerCommentCommands } from "./comments.ts"; import { registerForensicsCommands } from "./forensics.ts"; @@ -8,6 +9,16 @@ import { registerInspectCommands } from "./inspect.ts"; import { registerTaskCommands } from "./task.ts"; export async function main(argv: string[] = process.argv): Promise { + try { + assertModelEnvConfig(); + } catch (err) { + if (err instanceof ModelConfigError) { + console.error(`orchestrate model config: ${err.message}`); + process.exit(2); + } + throw err; + } + const program = new Command(); program diff --git a/orchestrate/skills/orchestrate/scripts/models.ts b/orchestrate/skills/orchestrate/scripts/models.ts index 82f141df..a4cb261b 100644 --- a/orchestrate/skills/orchestrate/scripts/models.ts +++ b/orchestrate/skills/orchestrate/scripts/models.ts @@ -2,9 +2,9 @@ import type { ModelSelection } from "@cursor/sdk"; import type { TaskType } from "./adapters/types.ts"; -// Model catalog. Source of truth for `tasks[].model` choices; `defaultFor` -// entries supply the fallback when `tasks[].model` is omitted. Per-role env -// vars (see MODEL_ENV_BY_TYPE) override those catalog defaults at runtime. +// Built-in model catalog. `buildEffectiveCatalog` merges this with the +// ORCHESTRATE_MODEL_* env config to produce what planners actually see; +// `defaultFor` supplies the fallback when `tasks[].model` is omitted. export interface ModelProfile { /** User-facing slug for `tasks[].model` and `--model` flags. */ @@ -29,6 +29,12 @@ export const MODEL_ENV_BY_TYPE: Record = { /** Env var that overrides the kickoff `--model` default for the root planner. */ export const MODEL_ENV_ROOT = "ORCHESTRATE_MODEL_ROOT"; +/** Env var holding a JSON array of extra catalog entries. */ +export const MODEL_ENV_CATALOG = "ORCHESTRATE_MODEL_CATALOG"; + +/** Env var selecting `merge` (default) or `env-only` catalog construction. */ +export const MODEL_ENV_CATALOG_MODE = "ORCHESTRATE_MODEL_CATALOG_MODE"; + export const DEFAULT_ROOT_MODEL = "claude-opus-4-8"; // `slug` is the stable authoring name; `selection` is the canonical SDK form. @@ -177,73 +183,89 @@ export const MODEL_CATALOG: ModelProfile[] = [ }, ]; +/** Raised when the ORCHESTRATE_MODEL_* environment config is not usable. */ +export class ModelConfigError extends Error {} + +const ALL_TASK_TYPES: TaskType[] = ["worker", "subplanner", "verifier"]; + +const SPEEDS = new Set(["fast", "medium", "slow"]); + function readEnvOverride(name: string): string | undefined { const raw = process.env[name]?.trim(); return raw || undefined; } -/** True when `spec` looks like a JSON ModelSelection object literal. */ +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function errText(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** True when `spec` looks like a JSON object literal rather than a slug. */ export function looksLikeSelectionJson(spec: string): boolean { return spec.trimStart().startsWith("{"); } -/** - * Parse a dual-format model override: catalog slug, bare model id, or JSON - * `ModelSelection` (`{"id":"…","params":[…]}`). - * - * JSON form is for models not yet in MODEL_CATALOG (e.g. composer-2.5) where - * a bare `{ id: slug }` would lose required params or fail `invalid_model`. - */ -export function parseModelSelectionJson(raw: string): ModelSelection { - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (err) { - throw new Error( - `invalid model selection JSON: ${err instanceof Error ? err.message : String(err)}` - ); - } +/** Validate an already-parsed `{ id, params? }` into a canonical selection. */ +export function normalizeModelSelection( + value: unknown, + ctx = "invalid model selection JSON" +): ModelSelection { if ( - parsed === null || - typeof parsed !== "object" || - Array.isArray(parsed) || - typeof (parsed as { id?: unknown }).id !== "string" || - !(parsed as { id: string }).id.trim() + !isPlainObject(value) || + typeof value.id !== "string" || + !value.id.trim() ) { - throw new Error( - 'invalid model selection JSON: expected {"id":"", "params"?: [{"id","value"}, ...]}' + throw new ModelConfigError( + `${ctx}: expected {"id":"", "params"?: [{"id","value"}, ...]}` ); } - const obj = parsed as { id: string; params?: unknown }; - const selection: ModelSelection = { id: obj.id.trim() }; - if (obj.params !== undefined) { - if (!Array.isArray(obj.params)) { - throw new Error( - 'invalid model selection JSON: "params" must be an array of {id, value}' + const selection: ModelSelection = { id: value.id.trim() }; + if (value.params !== undefined) { + if (!Array.isArray(value.params)) { + throw new ModelConfigError( + `${ctx}: "params" must be an array of {id, value}` ); } const params: { id: string; value: string }[] = []; - for (const p of obj.params) { + for (const p of value.params) { if ( - p === null || - typeof p !== "object" || - typeof (p as { id?: unknown }).id !== "string" || - typeof (p as { value?: unknown }).value !== "string" + !isPlainObject(p) || + typeof p.id !== "string" || + typeof p.value !== "string" ) { - throw new Error( - "invalid model selection JSON: each params entry must be {id: string, value: string}" + throw new ModelConfigError( + `${ctx}: each params entry must be {id: string, value: string}` ); } - params.push({ - id: (p as { id: string }).id, - value: (p as { value: string }).value, - }); + params.push({ id: p.id, value: p.value }); } selection.params = params; } return selection; } +/** + * Parse a JSON `ModelSelection` (`{"id":"…","params":[…]}`). + * + * JSON form is for models not yet in MODEL_CATALOG (e.g. composer-2.5) where + * a bare `{ id: slug }` would lose required params or fail `invalid_model`. + */ +export function parseModelSelectionJson( + raw: string, + ctx = "invalid model selection JSON" +): ModelSelection { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new ModelConfigError(`${ctx}: ${errText(err)}`); + } + return normalizeModelSelection(parsed, ctx); +} + /** Compact label for catalog / attention logs. */ export function formatModelSelectionLabel(selection: ModelSelection): string { if (!selection.params?.length) return selection.id; @@ -251,93 +273,366 @@ export function formatModelSelectionLabel(selection: ModelSelection): string { return `${selection.id} (${params})`; } +export type CatalogMode = "merge" | "env-only"; + /** - * Fallback model spec when `tasks[].model` is omitted. - * Returns a catalog slug, bare id, or JSON ModelSelection string from env. + * `env-only` drops MODEL_CATALOG from the effective catalog so a team can + * publish an exact menu of models. Built-in entries can still be pulled back + * in by slug reference from ORCHESTRATE_MODEL_CATALOG or a role env var. */ -export function defaultModelForType(type: TaskType): string { - const fromEnv = readEnvOverride(MODEL_ENV_BY_TYPE[type]); - if (fromEnv) return fromEnv; +export function readCatalogMode(): CatalogMode { + const raw = readEnvOverride(MODEL_ENV_CATALOG_MODE); + if (!raw) return "merge"; + const normalized = raw.toLowerCase(); + if (normalized === "merge" || normalized === "env-only") return normalized; + throw new ModelConfigError( + `${MODEL_ENV_CATALOG_MODE}="${raw}" is not valid. Use "merge" (default) or "env-only".` + ); +} + +function parseSpeed(value: unknown, ctx: string): ModelProfile["speed"] { + if (value === undefined) return "medium"; + if (typeof value === "string" && SPEEDS.has(value as ModelProfile["speed"])) { + return value as ModelProfile["speed"]; + } + throw new ModelConfigError(`${ctx}: "speed" must be fast, medium, or slow`); +} + +function parseStrengths(value: unknown, ctx: string): string[] { + if (value === undefined) return []; + if (!Array.isArray(value) || value.some(v => typeof v !== "string")) { + throw new ModelConfigError( + `${ctx}: "strengths" must be an array of strings` + ); + } + return value as string[]; +} + +function parseDefaultFor(value: unknown, ctx: string): TaskType[] | undefined { + if (value === undefined) return undefined; + if (!Array.isArray(value)) { + throw new ModelConfigError( + `${ctx}: "defaultFor" must be an array of ${ALL_TASK_TYPES.join(" | ")}` + ); + } + const types: TaskType[] = []; + for (const item of value) { + if ( + typeof item !== "string" || + !ALL_TASK_TYPES.includes(item as TaskType) + ) { + throw new ModelConfigError( + `${ctx}: "defaultFor" entries must be one of ${ALL_TASK_TYPES.join(", ")}` + ); + } + types.push(item as TaskType); + } + return types; +} - const match = MODEL_CATALOG.find(m => m.defaultFor?.includes(type)); - if (!match) - throw new Error(`MODEL_CATALOG missing default for TaskType "${type}"`); - return match.slug; +function optionalString( + value: unknown, + field: string, + ctx: string +): string | undefined { + if (value === undefined) return undefined; + if (typeof value !== "string" || !value.trim()) { + throw new ModelConfigError(`${ctx}: "${field}" must be a non-empty string`); + } + return value.trim(); } -/** Kickoff `--model` default: ORCHESTRATE_MODEL_ROOT, else catalog root default. */ +/** A parsed env entry: either a reference to an existing slug, or a profile. */ +type CatalogEntryInput = + | { kind: "ref"; slug: string; defaultFor?: TaskType[] } + | { kind: "profile"; profile: ModelProfile }; + +function parseEntryObject( + raw: Record, + ctx: string +): CatalogEntryInput { + const slug = optionalString(raw.slug, "slug", ctx); + const defaultFor = parseDefaultFor(raw.defaultFor, ctx); + const hasSelection = raw.selection !== undefined || raw.id !== undefined; + if (!hasSelection) { + if (!slug) { + throw new ModelConfigError( + `${ctx}: entry needs "id" (or "selection"), or a "slug" that references a built-in model` + ); + } + return { kind: "ref", slug, defaultFor }; + } + const selection = normalizeModelSelection( + raw.selection ?? { id: raw.id, params: raw.params }, + ctx + ); + const label = formatModelSelectionLabel(selection); + return { + kind: "profile", + profile: { + slug: slug ?? selection.id, + selection, + summary: + optionalString(raw.summary, "summary", ctx) ?? + `Operator-configured model (${label}).`, + strengths: parseStrengths(raw.strengths, ctx), + speed: parseSpeed(raw.speed, ctx), + use: + optionalString(raw.use, "use", ctx) ?? + "Configured for this repo via orchestrate model env config. Prefer it unless the task needs a listed specialist.", + defaultFor, + }, + }; +} + +/** Role env vars accept a slug, a bare model id, or a JSON entry. */ +function parseRoleEnvValue(raw: string, envName: string): CatalogEntryInput { + if (looksLikeSelectionJson(raw)) { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new ModelConfigError(`${envName}: ${errText(err)}`); + } + if (!isPlainObject(parsed)) { + throw new ModelConfigError(`${envName}: expected a JSON object`); + } + return parseEntryObject(parsed, envName); + } + return { kind: "ref", slug: raw }; +} + +function builtinBySlug(slug: string): ModelProfile | undefined { + return MODEL_CATALOG.find(m => m.slug === slug); +} + +/** + * Resolve a slug reference. Role env vars may name a model that isn't in the + * built-in catalog (`composer-2.5`); it becomes a bare `{ id }` entry. + * ORCHESTRATE_MODEL_CATALOG refs must name a built-in, since a typo there + * would silently publish a non-existent model to planners. + */ +function resolveRef( + slug: string, + ctx: string, + synthesizeIfUnknown: boolean +): ModelProfile { + const builtin = builtinBySlug(slug); + if (builtin) return { ...builtin }; + if (!synthesizeIfUnknown) { + throw new ModelConfigError( + `${ctx}: "${slug}" is not a built-in MODEL_CATALOG slug. Give the entry an "id" to define a new model, or use a known slug.` + ); + } + return { + slug, + selection: { id: slug }, + summary: `Operator-configured model (${slug}).`, + strengths: [], + speed: "medium", + use: "Configured for this repo via orchestrate model env config. Prefer it unless the task needs a listed specialist.", + }; +} + +export interface EffectiveCatalog { + mode: CatalogMode; + catalog: ModelProfile[]; + /** Effective role defaults, by slug. Missing when nothing claims the role. */ + defaults: Partial>; +} + +/** + * Merge MODEL_CATALOG with the ORCHESTRATE_MODEL_* env config into the catalog + * planners actually see. Built per call so env changes apply without reload. + */ +export function buildEffectiveCatalog(): EffectiveCatalog { + const mode = readCatalogMode(); + const bySlug = new Map(); + const order: string[] = []; + const upsert = (profile: ModelProfile): void => { + if (!bySlug.has(profile.slug)) order.push(profile.slug); + bySlug.set(profile.slug, profile); + }; + + if (mode === "merge") { + for (const profile of MODEL_CATALOG) upsert({ ...profile }); + } + + const declared: Partial> = {}; + + const rawCatalog = readEnvOverride(MODEL_ENV_CATALOG); + if (rawCatalog) { + let parsed: unknown; + try { + parsed = JSON.parse(rawCatalog); + } catch (err) { + throw new ModelConfigError(`${MODEL_ENV_CATALOG}: ${errText(err)}`); + } + if (!Array.isArray(parsed)) { + throw new ModelConfigError( + `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` + ); + } + parsed.forEach((item, i) => { + const ctx = `${MODEL_ENV_CATALOG}[${i}]`; + if (!isPlainObject(item)) { + throw new ModelConfigError(`${ctx}: expected a JSON object`); + } + const entry = parseEntryObject(item, ctx); + const profile = + entry.kind === "ref" + ? { + ...resolveRef(entry.slug, ctx, false), + defaultFor: entry.defaultFor, + } + : entry.profile; + upsert(profile); + for (const type of profile.defaultFor ?? []) { + const prior = declared[type]; + if (prior && prior !== profile.slug) { + throw new ModelConfigError( + `${MODEL_ENV_CATALOG}: two entries claim the ${type} default ("${prior}" and "${profile.slug}")` + ); + } + declared[type] = profile.slug; + } + }); + } + + // Role env vars are the most specific signal, so they win over any + // defaultFor declared inside ORCHESTRATE_MODEL_CATALOG. + for (const type of ALL_TASK_TYPES) { + const envName = MODEL_ENV_BY_TYPE[type]; + const raw = readEnvOverride(envName); + if (!raw) continue; + const entry = parseRoleEnvValue(raw, envName); + const profile = + entry.kind === "ref" + ? resolveRef(entry.slug, envName, true) + : entry.profile; + const existing = bySlug.get(profile.slug); + upsert(entry.kind === "ref" && existing ? existing : profile); + declared[type] = profile.slug; + } + + // Inherit unclaimed roles from whatever remains in the catalog. + for (const type of ALL_TASK_TYPES) { + if (declared[type]) continue; + const inherited = order + .map(slug => bySlug.get(slug)) + .find(profile => profile?.defaultFor?.includes(type)); + if (inherited) declared[type] = inherited.slug; + } + + // Re-stamp defaultFor so rendering reflects the resolved defaults. + const rolesBySlug = new Map(); + for (const type of ALL_TASK_TYPES) { + const slug = declared[type]; + if (!slug) continue; + const roles = rolesBySlug.get(slug) ?? []; + roles.push(type); + rolesBySlug.set(slug, roles); + } + + const catalog: ModelProfile[] = []; + for (const slug of order) { + const profile = bySlug.get(slug); + if (!profile) continue; + const roles = rolesBySlug.get(slug); + catalog.push({ ...profile, defaultFor: roles?.length ? roles : undefined }); + } + + return { mode, catalog, defaults: declared }; +} + +/** The catalog planners see: MODEL_CATALOG plus/minus the env config. */ +export function effectiveModelCatalog(): ModelProfile[] { + return buildEffectiveCatalog().catalog; +} + +/** + * Fallback model slug when `tasks[].model` is omitted. Resolves against the + * effective catalog, so env config participates. + */ +export function defaultModelForType(type: TaskType): string { + const { defaults, mode } = buildEffectiveCatalog(); + const slug = defaults[type]; + if (slug) return slug; + if (mode === "env-only") { + throw new ModelConfigError( + `no ${type} default: ${MODEL_ENV_CATALOG_MODE}=env-only drops MODEL_CATALOG, so set ${MODEL_ENV_BY_TYPE[type]} or give an ${MODEL_ENV_CATALOG} entry "defaultFor": ["${type}"]` + ); + } + throw new ModelConfigError( + `MODEL_CATALOG missing default for TaskType "${type}"` + ); +} + +/** Kickoff `--model` default: ORCHESTRATE_MODEL_ROOT, else the built-in root model. */ export function defaultRootModel(): string { return readEnvOverride(MODEL_ENV_ROOT) ?? DEFAULT_ROOT_MODEL; } export function isKnownModel(slug: string): boolean { if (looksLikeSelectionJson(slug)) return false; - return MODEL_CATALOG.some(m => m.slug === slug); + return effectiveModelCatalog().some(m => m.slug === slug); } /** * Resolve an authoring slug, bare model id, or JSON ModelSelection into the * canonical SDK form passed to `Agent.create({ model })`. + * + * Unknown slugs pass through as a bare `{ id }` so planners can reach + * server-side models that aren't in the catalog. */ export function resolveModelSelection(spec: string): ModelSelection { const trimmed = spec.trim(); if (looksLikeSelectionJson(trimmed)) { return parseModelSelectionJson(trimmed); } - const profile = MODEL_CATALOG.find(m => m.slug === trimmed); + const profile = effectiveModelCatalog().find(m => m.slug === trimmed); return profile ? profile.selection : { id: trimmed }; } -/** Effective default label per task type (catalog or env override). */ +/** Effective default label per task type (catalog entry or env override). */ export function effectiveDefaultLabelForType(type: TaskType): string { - const spec = defaultModelForType(type); - if (looksLikeSelectionJson(spec)) { - return formatModelSelectionLabel(parseModelSelectionJson(spec)); - } - return spec; + return defaultModelForType(type); } -export function renderModelCatalog(): string { - const types: TaskType[] = ["worker", "subplanner", "verifier"]; - const effectiveByType = new Map(); - for (const type of types) { - effectiveByType.set(type, effectiveDefaultLabelForType(type)); - } - - // Map catalog slug → roles that currently default to it (after env overrides). - const defaultsBySlug = new Map(); - const nonCatalogDefaults: { type: TaskType; label: string }[] = []; - for (const type of types) { - const label = effectiveByType.get(type); - if (label === undefined) continue; - if (isKnownModel(label)) { - const list = defaultsBySlug.get(label) ?? []; - list.push(type); - defaultsBySlug.set(label, list); - } else { - nonCatalogDefaults.push({ type, label }); - } - } +/** + * Fail fast on unusable ORCHESTRATE_MODEL_* config instead of surfacing it as + * a spawn error halfway through a run. + */ +export function assertModelEnvConfig(): void { + const { defaults, mode } = buildEffectiveCatalog(); + if (mode !== "env-only") return; + const missing = ALL_TASK_TYPES.filter(type => !defaults[type]); + if (!missing.length) return; + throw new ModelConfigError( + `${MODEL_ENV_CATALOG_MODE}=env-only leaves no default for ${missing.join(", ")}. Set ${missing + .map(type => MODEL_ENV_BY_TYPE[type]) + .join(", ")}, or add "defaultFor" to an ${MODEL_ENV_CATALOG} entry.` + ); +} +export function renderModelCatalog(): string { + const { catalog, mode } = buildEffectiveCatalog(); const lines: string[] = []; - if (nonCatalogDefaults.length) { + if (mode === "env-only") { lines.push( - "Env default overrides (not in MODEL_CATALOG; used when `tasks[].model` is omitted):" + "This repo publishes an exact model menu. Use only the slugs listed below; do not reach for models outside this list." ); - for (const { type, label } of nonCatalogDefaults) { - const envName = MODEL_ENV_BY_TYPE[type]; - lines.push(`- ${type}: \`${label}\` (from ${envName})`); - } lines.push(""); } - - for (const m of MODEL_CATALOG) { - const defaults = defaultsBySlug.get(m.slug); - const defaultsNote = defaults?.length - ? ` (default for ${defaults.join(", ")})` + for (const m of catalog) { + const defaults = m.defaultFor?.length + ? ` (default for ${m.defaultFor.join(", ")})` + : ""; + lines.push(`- \`${m.slug}\` — ${m.summary}${defaults}`); + const strengths = m.strengths.length + ? `; strengths: ${m.strengths.join(", ")}` : ""; - lines.push(`- \`${m.slug}\` — ${m.summary}${defaultsNote}`); - lines.push(` speed: ${m.speed}; strengths: ${m.strengths.join(", ")}`); + lines.push(` speed: ${m.speed}${strengths}`); lines.push(` use: ${m.use}`); } return lines.join("\n"); diff --git a/orchestrate/skills/orchestrate/scripts/tools/probe-models.ts b/orchestrate/skills/orchestrate/scripts/tools/probe-models.ts index 7b172690..2e0decd2 100644 --- a/orchestrate/skills/orchestrate/scripts/tools/probe-models.ts +++ b/orchestrate/skills/orchestrate/scripts/tools/probe-models.ts @@ -1,6 +1,6 @@ #!/usr/bin/env bun import { Agent } from "@cursor/sdk"; -import { MODEL_CATALOG, type ModelProfile } from "../models.ts"; +import { effectiveModelCatalog, type ModelProfile } from "../models.ts"; const PROBE_REPO = "https://github.com/example-org/example-repo"; @@ -33,7 +33,7 @@ export async function probeModelCatalog( agentApi?: ProbeAgentApi; } = {} ): Promise { - const catalog = opts.catalog ?? MODEL_CATALOG; + const catalog = opts.catalog ?? effectiveModelCatalog(); const agentApi = opts.agentApi ?? Agent; const results: ProbeResult[] = []; for (const profile of catalog) { From 958f953b986eff38ff4158c8d3586c6afdecd397 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 18:00:44 +0000 Subject: [PATCH 3/6] refactor(orchestrate): collapse model config to ORCHESTRATE_MODEL_CATALOG Drop the per-role env vars and the catalog mode flag. A single JSON array replaces the built-in catalog outright when set, which removes merge precedence entirely: the configured list is the complete menu and its defaultFor entries supply every role default, including the root planner. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 65 ++- orchestrate/skills/orchestrate/SKILL.md | 2 +- .../orchestrate/references/dispatcher.md | 2 +- .../scripts/__tests__/models-catalog.test.ts | 27 +- .../__tests__/models-env-overrides.test.ts | 352 +++++++--------- .../skills/orchestrate/scripts/cli/task.ts | 2 +- .../skills/orchestrate/scripts/models.ts | 391 +++++++----------- 7 files changed, 321 insertions(+), 520 deletions(-) diff --git a/orchestrate/README.md b/orchestrate/README.md index dc5dafeb..4c19fb74 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -10,65 +10,52 @@ The skill itself lives in [`skills/orchestrate/SKILL.md`](./skills/orchestrate/S - A Cursor API key in `CURSOR_API_KEY`. - Optional Slack app and bot token if you want a Slack thread mirroring the run. -## Model configuration (optional) +## Model catalog (optional) -The built-in `MODEL_CATALOG` is merged with environment config into an **effective catalog**. That merged catalog is what planners see when they set `tasks[].model`, what `bun cli.ts models` prints, and what supplies role defaults when `tasks[].model` is omitted. Use it to steer cost without editing the plugin. - -### Role defaults - -| Env var | Role | -| --- | --- | -| `ORCHESTRATE_MODEL_WORKER` | worker default | -| `ORCHESTRATE_MODEL_SUBPLANNER` | subplanner default | -| `ORCHESTRATE_MODEL_VERIFIER` | verifier default | -| `ORCHESTRATE_MODEL_ROOT` | kickoff `--model` default | - -Each value may be a catalog slug (`composer-2-fast`), a bare model id (`composer-2.5`), or a JSON entry: - -```bash -export ORCHESTRATE_MODEL_WORKER=composer-2-fast -export ORCHESTRATE_MODEL_SUBPLANNER='{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' -``` - -Anything named this way joins the effective catalog, so planners can select it by slug and it resolves back to the full selection (params included). A JSON entry may also carry `slug`, `summary`, `use`, `speed`, and `strengths` — worth setting, since planners choose by the capability prose: - -```bash -export ORCHESTRATE_MODEL_WORKER='{"slug":"house-worker","id":"composer-2.5","summary":"House worker model.","use":"Use for all bounded implementation work.","speed":"fast","strengths":["throughput"]}' -``` - -### Adding models - -`ORCHESTRATE_MODEL_CATALOG` takes a JSON array of entries. Entries with an `id` define a model; entries with only a `slug` pull in a built-in by reference. `defaultFor` claims a role. +`ORCHESTRATE_MODEL_CATALOG` replaces the built-in model catalog with your own. When it is set, that list is the complete menu: it is what planners choose `tasks[].model` from, what `bun cli.ts models` prints, and where role defaults come from. Nothing is merged with the built-in catalog, so what you write is exactly what runs. Use it to steer cost without editing the plugin. ```bash export ORCHESTRATE_MODEL_CATALOG='[ {"id":"composer-2.5","summary":"Cheap, fast worker.","defaultFor":["worker"]}, - {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} + {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier","root"]} ]' ``` -### Restricting the menu +### Entries + +An entry either **defines** a model or **references** a built-in one: + +- `"id"` (plus optional `"params"`) defines a model. `"slug"` names it for `tasks[].model`, defaulting to the id. +- `"slug"` alone pulls in the built-in profile of that name, so you can curate a subset without retyping SDK params. Run `bun cli.ts models` with the variable unset to see the built-in slugs. + +`"defaultFor"` claims roles: `worker`, `subplanner`, `verifier`, and `root` (the kickoff planner, i.e. the `--model` default). Every role except `root` needs a default somewhere in the list. -`ORCHESTRATE_MODEL_CATALOG_MODE=env-only` drops the built-in catalog, so planners see exactly the models you list (built-ins can be pulled back in by slug). Every role needs a default in this mode; the CLI exits with a config error if one is missing. +Optional prose fields are worth filling in, because planners choose by capability, not by name: ```bash -export ORCHESTRATE_MODEL_CATALOG_MODE=env-only export ORCHESTRATE_MODEL_CATALOG='[ - {"slug":"composer-2-fast","defaultFor":["worker"]}, + { + "slug":"house-worker", + "id":"composer-2.5", + "params":[{"id":"fast","value":"true"}], + "summary":"House worker model.", + "use":"Use for all bounded implementation work.", + "speed":"fast", + "strengths":["throughput"], + "defaultFor":["worker"] + }, {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} ]' ``` -### Precedence and limits +### Precedence 1. Explicit `tasks[].model` in the plan -2. Role env var (`ORCHESTRATE_MODEL_`) -3. `defaultFor` on an `ORCHESTRATE_MODEL_CATALOG` entry -4. `defaultFor` in the built-in catalog +2. The `defaultFor` entry for that task's role -Run `bun cli.ts models` to see the effective catalog, and `bun cli.ts models --check` to probe every entry (including your env-defined ones) against `/v1/agents`. +Run `bun cli.ts models` to print the effective catalog, and `bun cli.ts models --check` to probe every entry against `/v1/agents`. Malformed config (bad JSON, unknown slug reference, duplicate slug, two entries claiming one role, missing role default) exits 2 at startup with the offending index named, rather than failing mid-run. -Two caveats. This shapes what planners choose from, but a planner can still write any model id into `tasks[].model`, so it is guidance rather than a spend ceiling. And each spawned agent reads its own environment: set these as Cursor Cloud secrets for the repo so subplanners and workers inherit them, not just in the dispatcher's local shell. +Two caveats. This shapes what planners choose from, but a planner can still write any model id into `tasks[].model`, so it is guidance rather than a spend ceiling. And each spawned agent reads its own environment: set the variable as a Cursor Cloud secret for the repo so subplanners and workers inherit it, not just in the dispatcher's local shell. ## Cursor API key diff --git a/orchestrate/skills/orchestrate/SKILL.md b/orchestrate/skills/orchestrate/SKILL.md index ab9cd188..29283c8c 100644 --- a/orchestrate/skills/orchestrate/SKILL.md +++ b/orchestrate/skills/orchestrate/SKILL.md @@ -14,7 +14,7 @@ An explicit `/orchestrate ` fans out a large task across parallel Cursor c - `CURSOR_API_KEY` must be a personal/user key. Create it from [Cursor Dashboard > Integrations](https://cursor.com/dashboard/integrations), then read `cursor-sdk` Auth before using it. - `SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. -- Model config is optional. `ORCHESTRATE_MODEL_WORKER`, `_SUBPLANNER`, `_VERIFIER`, and `_ROOT` set role defaults; `ORCHESTRATE_MODEL_CATALOG` adds entries; `ORCHESTRATE_MODEL_CATALOG_MODE=env-only` restricts planners to exactly the listed models. All of it merges into the catalog `bun cli.ts models` prints, which is the list planners pick `tasks[].model` from. See the plugin README. +- `ORCHESTRATE_MODEL_CATALOG` is optional. When set, its JSON array replaces the built-in model catalog outright: it becomes the list planners pick `tasks[].model` from, and its `defaultFor` entries supply each role's default. `bun cli.ts models` prints whichever catalog is in effect. See the plugin README. ## Core principles diff --git a/orchestrate/skills/orchestrate/references/dispatcher.md b/orchestrate/skills/orchestrate/references/dispatcher.md index c2a54d27..aeec581d 100644 --- a/orchestrate/skills/orchestrate/references/dispatcher.md +++ b/orchestrate/skills/orchestrate/references/dispatcher.md @@ -16,7 +16,7 @@ One-time setup: run `bun install` inside this skill's `scripts/` directory if `n bun cli.ts kickoff "" [--repo ] [--ref main] [--model claude-opus-4-8] [--slack-channel C123] [--dispatcher-name "Alex"] ``` -`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses `ORCHESTRATE_MODEL_ROOT`, else `claude-opus-4-8`. Role defaults and the catalog planners choose from are configurable via `ORCHESTRATE_MODEL_WORKER` / `_SUBPLANNER` / `_VERIFIER`, `ORCHESTRATE_MODEL_CATALOG`, and `ORCHESTRATE_MODEL_CATALOG_MODE` (see the plugin README). Run `bun cli.ts models` to print the effective catalog; a bad config exits 2 with the offending env var named. +`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses the catalog entry marked `defaultFor: ["root"]`, else `claude-opus-4-8`. A repo can replace the whole catalog, including every role default, with `ORCHESTRATE_MODEL_CATALOG` (see the plugin README). Run `bun cli.ts models` to print the effective catalog; a bad config exits 2 with the offending entry named. The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts index f38fff63..de23c07b 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-catalog.test.ts @@ -4,33 +4,22 @@ import { defaultModelForType, isKnownModel, MODEL_CATALOG, - MODEL_ENV_BY_TYPE, - MODEL_ENV_ROOT, + MODEL_ENV_CATALOG, resolveModelSelection, } from "../models.ts"; -const ENV_KEYS = [ - MODEL_ENV_BY_TYPE.worker, - MODEL_ENV_BY_TYPE.subplanner, - MODEL_ENV_BY_TYPE.verifier, - MODEL_ENV_ROOT, -] as const; - -const savedEnv: Record = {}; +let savedCatalogEnv: string | undefined; +// These assertions describe the built-in catalog, so an env-provided catalog +// from the surrounding shell must not leak in. beforeEach(() => { - for (const key of ENV_KEYS) { - savedEnv[key] = process.env[key]; - delete process.env[key]; - } + savedCatalogEnv = process.env[MODEL_ENV_CATALOG]; + delete process.env[MODEL_ENV_CATALOG]; }); afterEach(() => { - for (const key of ENV_KEYS) { - const prior = savedEnv[key]; - if (prior === undefined) delete process.env[key]; - else process.env[key] = prior; - } + if (savedCatalogEnv === undefined) delete process.env[MODEL_ENV_CATALOG]; + else process.env[MODEL_ENV_CATALOG] = savedCatalogEnv; }); describe("MODEL_CATALOG", () => { diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts index 962cd97a..ac55b7c9 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts @@ -2,295 +2,231 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { assertModelEnvConfig, - buildEffectiveCatalog, DEFAULT_ROOT_MODEL, defaultModelForType, defaultRootModel, effectiveModelCatalog, formatModelSelectionLabel, isKnownModel, - MODEL_ENV_BY_TYPE, MODEL_ENV_CATALOG, - MODEL_ENV_CATALOG_MODE, - MODEL_ENV_ROOT, ModelConfigError, parseModelSelectionJson, renderModelCatalog, resolveModelSelection, + usingEnvCatalog, } from "../models.ts"; -const ENV_KEYS = [ - MODEL_ENV_BY_TYPE.worker, - MODEL_ENV_BY_TYPE.subplanner, - MODEL_ENV_BY_TYPE.verifier, - MODEL_ENV_ROOT, - MODEL_ENV_CATALOG, - MODEL_ENV_CATALOG_MODE, -] as const; +let saved: string | undefined; -const savedEnv: Record = {}; +function setCatalog(entries: unknown): void { + process.env[MODEL_ENV_CATALOG] = JSON.stringify(entries); +} beforeEach(() => { - for (const key of ENV_KEYS) { - savedEnv[key] = process.env[key]; - delete process.env[key]; - } + saved = process.env[MODEL_ENV_CATALOG]; + delete process.env[MODEL_ENV_CATALOG]; }); afterEach(() => { - for (const key of ENV_KEYS) { - const prior = savedEnv[key]; - if (prior === undefined) delete process.env[key]; - else process.env[key] = prior; - } + if (saved === undefined) delete process.env[MODEL_ENV_CATALOG]; + else process.env[MODEL_ENV_CATALOG] = saved; }); -describe("per-role env overrides", () => { - test("catalog defaults apply when env is unset", () => { +describe("built-in catalog (ORCHESTRATE_MODEL_CATALOG unset)", () => { + test("role defaults come from MODEL_CATALOG", () => { + expect(usingEnvCatalog()).toBe(false); expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); expect(defaultModelForType("subplanner")).toBe( "claude-opus-4-8-thinking-xhigh" ); expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); + expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); }); - test("catalog slug in env overrides defaultFor", () => { - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; - expect(defaultModelForType("worker")).toBe("composer-2-fast"); - expect(resolveModelSelection("composer-2-fast")).toEqual({ - id: "composer-2", - params: [{ id: "fast", value: "true" }], - }); - expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); + test("whitespace-only value is treated as unset", () => { + process.env[MODEL_ENV_CATALOG] = " "; + expect(usingEnvCatalog()).toBe(false); + expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); }); - test("whitespace-only env is treated as unset", () => { - process.env.ORCHESTRATE_MODEL_WORKER = " "; - expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); + test("rendered catalog has no exact-menu preamble", () => { + expect(renderModelCatalog()).not.toContain("exact model menu"); }); +}); - test("bare unknown id joins the catalog and round-trips", () => { - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; - expect(defaultModelForType("worker")).toBe("composer-2.5"); - // Merged in, so a planner copying the slug resolves back to the same model. +describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { + test("only the listed models are published", () => { + setCatalog([ + { id: "composer-2.5", summary: "Cheap worker.", defaultFor: ["worker"] }, + { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, + ]); + expect(usingEnvCatalog()).toBe(true); + expect(effectiveModelCatalog().map(m => m.slug)).toEqual([ + "composer-2.5", + "claude-opus-4-8", + ]); + expect(isKnownModel("gpt-5.5-high-fast")).toBe(false); expect(isKnownModel("composer-2.5")).toBe(true); - expect(resolveModelSelection("composer-2.5")).toEqual({ - id: "composer-2.5", - }); }); - test("JSON entry keeps params and gets a slug", () => { - process.env.ORCHESTRATE_MODEL_WORKER = - '{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}'; - const slug = defaultModelForType("worker"); - expect(slug).toBe("composer-2.5"); - expect(resolveModelSelection(slug)).toEqual({ - id: "composer-2.5", - params: [{ id: "fast", value: "true" }], - }); + test("entries define role defaults", () => { + setCatalog([ + { id: "composer-2.5", defaultFor: ["worker"] }, + { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, + ]); + expect(defaultModelForType("worker")).toBe("composer-2.5"); + expect(defaultModelForType("subplanner")).toBe("claude-opus-4-8"); + expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); }); - test("JSON entry can name its own slug and prose", () => { - process.env.ORCHESTRATE_MODEL_WORKER = JSON.stringify({ - slug: "composer-cheap", - id: "composer-2.5", + test("slug-only entry pulls in a built-in with its params", () => { + setCatalog([{ slug: "composer-2-fast", defaultFor: ["worker"] }]); + expect(resolveModelSelection("composer-2-fast")).toEqual({ + id: "composer-2", params: [{ id: "fast", value: "true" }], - summary: "House worker model.", - use: "Use for all bounded implementation work.", - speed: "fast", - strengths: ["throughput"], }); - expect(defaultModelForType("worker")).toBe("composer-cheap"); - expect(resolveModelSelection("composer-cheap")).toEqual({ + expect(defaultModelForType("worker")).toBe("composer-2-fast"); + }); + + test("entry keeps params and round-trips by slug", () => { + setCatalog([ + { + slug: "house-worker", + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + summary: "House worker model.", + use: "Use for all bounded implementation work.", + speed: "fast", + strengths: ["throughput"], + defaultFor: ["worker"], + }, + ]); + expect(resolveModelSelection("house-worker")).toEqual({ id: "composer-2.5", params: [{ id: "fast", value: "true" }], }); const text = renderModelCatalog(); - expect(text).toContain("`composer-cheap` — House worker model."); + expect(text).toContain("exact model menu"); + expect(text).toContain("`house-worker` — House worker model."); expect(text).toContain("(default for worker)"); expect(text).toContain("speed: fast; strengths: throughput"); }); - test("invalid JSON selection throws a clear error", () => { - expect(() => parseModelSelectionJson("{not-json")).toThrow( - /invalid model selection JSON/ - ); - expect(() => parseModelSelectionJson('{"params":[]}')).toThrow( - /expected \{"id"/ - ); - expect(() => - parseModelSelectionJson('{"id":"x","params":[{"id":1,"value":"a"}]}') - ).toThrow(/each params entry/); + test("slug defaults to the model id and prose is synthesized", () => { + setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); + const [entry] = effectiveModelCatalog(); + expect(entry.slug).toBe("composer-2.5"); + expect(entry.summary).toContain("composer-2.5"); + expect(entry.speed).toBe("medium"); }); - test("defaultRootModel reads ORCHESTRATE_MODEL_ROOT", () => { + test("root default is configurable, else falls back", () => { + setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); - process.env.ORCHESTRATE_MODEL_ROOT = "composer-2-fast"; - expect(defaultRootModel()).toBe("composer-2-fast"); - process.env.ORCHESTRATE_MODEL_ROOT = '{"id":"composer-2.5"}'; - expect(resolveModelSelection(defaultRootModel())).toEqual({ - id: "composer-2.5", - }); - }); - - test("rendered catalog moves the default label to the override", () => { - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; - const text = renderModelCatalog(); - const composerLine = text - .split("\n") - .find(l => l.includes("`composer-2-fast`")); - const gptLine = text - .split("\n") - .find(l => l.includes("`gpt-5.5-high-fast`")); - expect(composerLine).toContain("(default for worker)"); - expect(gptLine).not.toContain("default for worker"); - }); - - test("formatModelSelectionLabel renders params", () => { - expect( - formatModelSelectionLabel({ - id: "composer-2.5", - params: [{ id: "fast", value: "true" }], - }) - ).toBe("composer-2.5 (fast=true)"); + setCatalog([ + { id: "composer-2.5", defaultFor: ["worker"] }, + { slug: "claude-opus-4-8", defaultFor: ["root"] }, + ]); + expect(defaultRootModel()).toBe("claude-opus-4-8"); }); }); -describe("ORCHESTRATE_MODEL_CATALOG", () => { - test("adds entries alongside the built-in catalog", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "composer-2.5", summary: "Cheap worker." }, - ]); - const slugs = effectiveModelCatalog().map(m => m.slug); - expect(slugs).toContain("composer-2.5"); - expect(slugs).toContain("gpt-5.5-high-fast"); +describe("catalog config errors", () => { + test("missing required role fails fast with an actionable message", () => { + setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); + expect(() => assertModelEnvConfig()).toThrow(ModelConfigError); + expect(() => assertModelEnvConfig()).toThrow( + /no default for subplanner, verifier/ + ); + expect(() => defaultModelForType("verifier")).toThrow( + /"defaultFor": \["verifier"\]/ + ); }); - test("entry defaultFor claims a role", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + test("assertModelEnvConfig passes when every role resolves", () => { + setCatalog([ { id: "composer-2.5", defaultFor: ["worker"] }, + { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, ]); - expect(defaultModelForType("worker")).toBe("composer-2.5"); + expect(() => assertModelEnvConfig()).not.toThrow(); }); - test("role env var wins over an entry defaultFor", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "composer-2.5", defaultFor: ["worker"] }, - ]); - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2-fast"; - expect(defaultModelForType("worker")).toBe("composer-2-fast"); - }); + test("non-array, empty, and malformed JSON are rejected", () => { + process.env[MODEL_ENV_CATALOG] = '{"id":"x"}'; + expect(() => effectiveModelCatalog()).toThrow(/expected a JSON array/); - test("two entries claiming one role is a config error", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "composer-2.5", defaultFor: ["worker"] }, - { id: "grok-4-5", defaultFor: ["worker"] }, - ]); - expect(() => buildEffectiveCatalog()).toThrow(/claim the worker default/); - }); + setCatalog([]); + expect(() => effectiveModelCatalog()).toThrow(/at least one entry/); - test("entry can override a built-in slug's selection", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { slug: "composer-2-fast", id: "composer-2.5" }, - ]); - expect(resolveModelSelection("composer-2-fast")).toEqual({ - id: "composer-2.5", - }); + process.env[MODEL_ENV_CATALOG] = "[{id:}]"; + expect(() => effectiveModelCatalog()).toThrow(ModelConfigError); }); - test("slug-only entry referencing an unknown model is rejected", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { slug: "not-a-builtin" }, - ]); - expect(() => buildEffectiveCatalog()).toThrow( + test("unknown slug reference is rejected", () => { + setCatalog([{ slug: "not-a-builtin" }]); + expect(() => effectiveModelCatalog()).toThrow( /not a built-in MODEL_CATALOG slug/ ); }); - test("malformed config surfaces as ModelConfigError", () => { - process.env.ORCHESTRATE_MODEL_CATALOG = '{"id":"x"}'; - expect(() => buildEffectiveCatalog()).toThrow(ModelConfigError); - expect(() => buildEffectiveCatalog()).toThrow(/expected a JSON array/); - - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "x", speed: "blistering" }, - ]); - expect(() => buildEffectiveCatalog()).toThrow(/"speed" must be/); - - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "x", defaultFor: ["planner"] }, - ]); - expect(() => buildEffectiveCatalog()).toThrow(/"defaultFor" entries/); + test("duplicate slugs are rejected", () => { + setCatalog([{ id: "composer-2.5" }, { id: "composer-2.5" }]); + expect(() => effectiveModelCatalog()).toThrow(/duplicate slug/); }); -}); -describe("ORCHESTRATE_MODEL_CATALOG_MODE=env-only", () => { - test("drops built-ins so only listed models are published", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ + test("two entries claiming one role is rejected", () => { + setCatalog([ { id: "composer-2.5", defaultFor: ["worker"] }, - { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, + { id: "grok-4-5", defaultFor: ["worker"] }, ]); - const slugs = effectiveModelCatalog().map(m => m.slug); - expect(slugs).toEqual(["composer-2.5", "claude-opus-4-8"]); - expect(slugs).not.toContain("gpt-5.5-high-fast"); - expect(isKnownModel("gpt-5.5-high-fast")).toBe(false); + expect(() => effectiveModelCatalog()).toThrow(/claim the worker default/); }); - test("slug reference pulls a built-in back in with its params", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { slug: "composer-2-fast", defaultFor: ["worker"] }, - ]); - process.env.ORCHESTRATE_MODEL_SUBPLANNER = "claude-opus-4-8"; - process.env.ORCHESTRATE_MODEL_VERIFIER = "claude-opus-4-8"; - expect(resolveModelSelection("composer-2-fast")).toEqual({ - id: "composer-2", - params: [{ id: "fast", value: "true" }], - }); - expect(defaultModelForType("worker")).toBe("composer-2-fast"); - expect(defaultModelForType("subplanner")).toBe("claude-opus-4-8"); - }); + test("bad field values are rejected", () => { + setCatalog([{ id: "x", speed: "blistering" }]); + expect(() => effectiveModelCatalog()).toThrow(/"speed" must be/); - test("rendered catalog tells planners the menu is exact", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; - process.env.ORCHESTRATE_MODEL_CATALOG = JSON.stringify([ - { id: "composer-2.5", defaultFor: ["worker", "subplanner", "verifier"] }, - ]); - const text = renderModelCatalog(); - expect(text).toContain("exact model menu"); - expect(text).not.toContain("gpt-5.5-high-fast"); + setCatalog([{ id: "x", defaultFor: ["planner"] }]); + expect(() => effectiveModelCatalog()).toThrow(/"defaultFor" entries/); + + setCatalog([{ id: "x", strengths: "fast" }]); + expect(() => effectiveModelCatalog()).toThrow(/"strengths" must be/); + + setCatalog([{ summary: "no id or slug" }]); + expect(() => effectiveModelCatalog()).toThrow(/entry needs "id"/); }); +}); - test("missing role default fails fast with an actionable message", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; - expect(() => assertModelEnvConfig()).toThrow(ModelConfigError); - expect(() => assertModelEnvConfig()).toThrow( - /leaves no default for subplanner, verifier/ +describe("selection parsing", () => { + test("invalid JSON selection throws a clear error", () => { + expect(() => parseModelSelectionJson("{not-json")).toThrow( + /invalid model selection JSON/ ); - expect(() => defaultModelForType("verifier")).toThrow( - /ORCHESTRATE_MODEL_VERIFIER/ + expect(() => parseModelSelectionJson('{"params":[]}')).toThrow( + /expected \{"id"/ ); + expect(() => + parseModelSelectionJson('{"id":"x","params":[{"id":1,"value":"a"}]}') + ).toThrow(/each params entry/); }); - test("assertModelEnvConfig passes when every role resolves", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "env-only"; - process.env.ORCHESTRATE_MODEL_WORKER = "composer-2.5"; - process.env.ORCHESTRATE_MODEL_SUBPLANNER = "claude-opus-4-8"; - process.env.ORCHESTRATE_MODEL_VERIFIER = "claude-opus-4-8"; - expect(() => assertModelEnvConfig()).not.toThrow(); - }); - - test("unknown mode value is rejected", () => { - process.env.ORCHESTRATE_MODEL_CATALOG_MODE = "exclusive"; - expect(() => buildEffectiveCatalog()).toThrow(/is not valid/); + test("JSON selection passes through resolveModelSelection", () => { + expect( + resolveModelSelection( + '{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' + ) + ).toEqual({ + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + }); }); - test("merge mode is the default and keeps built-ins", () => { - expect(buildEffectiveCatalog().mode).toBe("merge"); - expect(effectiveModelCatalog().map(m => m.slug)).toContain( - "gpt-5.5-high-fast" - ); + test("formatModelSelectionLabel renders params", () => { + expect( + formatModelSelectionLabel({ + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + }) + ).toBe("composer-2.5 (fast=true)"); }); }); diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 47232f04..939a0c46 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -110,7 +110,7 @@ export function registerTaskCommands(program: Command): void { .option("--ref ", "Starting git ref for the cloud workspace", "main") .option( "--model ", - "Model id for the root planner (catalog slug, bare id, or JSON ModelSelection). Defaults to ORCHESTRATE_MODEL_ROOT, else claude-opus-4-8.", + 'Model id for the root planner (catalog slug, bare id, or JSON ModelSelection). Defaults to the catalog entry marked defaultFor ["root"], else claude-opus-4-8.', defaultRootModel() ) .option( diff --git a/orchestrate/skills/orchestrate/scripts/models.ts b/orchestrate/skills/orchestrate/scripts/models.ts index a4cb261b..ed17bfbc 100644 --- a/orchestrate/skills/orchestrate/scripts/models.ts +++ b/orchestrate/skills/orchestrate/scripts/models.ts @@ -2,9 +2,21 @@ import type { ModelSelection } from "@cursor/sdk"; import type { TaskType } from "./adapters/types.ts"; -// Built-in model catalog. `buildEffectiveCatalog` merges this with the -// ORCHESTRATE_MODEL_* env config to produce what planners actually see; -// `defaultFor` supplies the fallback when `tasks[].model` is omitted. +// Built-in model catalog, used when ORCHESTRATE_MODEL_CATALOG is unset. +// `defaultFor` supplies the model for a role when `tasks[].model` is omitted. + +/** Roles a catalog entry can be the default for. `root` is the kickoff planner. */ +export type CatalogRole = TaskType | "root"; + +export const CATALOG_ROLES: CatalogRole[] = [ + "worker", + "subplanner", + "verifier", + "root", +]; + +/** Roles that must resolve for a run to be spawnable. */ +const REQUIRED_ROLES: TaskType[] = ["worker", "subplanner", "verifier"]; export interface ModelProfile { /** User-facing slug for `tasks[].model` and `--model` flags. */ @@ -15,26 +27,16 @@ export interface ModelProfile { strengths: string[]; speed: "fast" | "medium" | "slow"; use: string; - /** Task types this profile is the default for. */ - defaultFor?: TaskType[]; + /** Roles this profile is the default for. */ + defaultFor?: CatalogRole[]; } -/** Env vars that override catalog `defaultFor` when set (non-empty). */ -export const MODEL_ENV_BY_TYPE: Record = { - worker: "ORCHESTRATE_MODEL_WORKER", - subplanner: "ORCHESTRATE_MODEL_SUBPLANNER", - verifier: "ORCHESTRATE_MODEL_VERIFIER", -}; - -/** Env var that overrides the kickoff `--model` default for the root planner. */ -export const MODEL_ENV_ROOT = "ORCHESTRATE_MODEL_ROOT"; - -/** Env var holding a JSON array of extra catalog entries. */ +/** + * Env var holding the whole catalog as JSON. When set it replaces + * MODEL_CATALOG outright; entries may still reference a built-in by slug. + */ export const MODEL_ENV_CATALOG = "ORCHESTRATE_MODEL_CATALOG"; -/** Env var selecting `merge` (default) or `env-only` catalog construction. */ -export const MODEL_ENV_CATALOG_MODE = "ORCHESTRATE_MODEL_CATALOG_MODE"; - export const DEFAULT_ROOT_MODEL = "claude-opus-4-8"; // `slug` is the stable authoring name; `selection` is the canonical SDK form. @@ -183,18 +185,11 @@ export const MODEL_CATALOG: ModelProfile[] = [ }, ]; -/** Raised when the ORCHESTRATE_MODEL_* environment config is not usable. */ +/** Raised when ORCHESTRATE_MODEL_CATALOG is not usable. */ export class ModelConfigError extends Error {} -const ALL_TASK_TYPES: TaskType[] = ["worker", "subplanner", "verifier"]; - const SPEEDS = new Set(["fast", "medium", "slow"]); -function readEnvOverride(name: string): string | undefined { - const raw = process.env[name]?.trim(); - return raw || undefined; -} - function isPlainObject(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } @@ -247,12 +242,7 @@ export function normalizeModelSelection( return selection; } -/** - * Parse a JSON `ModelSelection` (`{"id":"…","params":[…]}`). - * - * JSON form is for models not yet in MODEL_CATALOG (e.g. composer-2.5) where - * a bare `{ id: slug }` would lose required params or fail `invalid_model`. - */ +/** Parse a JSON `ModelSelection` (`{"id":"…","params":[…]}`). */ export function parseModelSelectionJson( raw: string, ctx = "invalid model selection JSON" @@ -273,23 +263,6 @@ export function formatModelSelectionLabel(selection: ModelSelection): string { return `${selection.id} (${params})`; } -export type CatalogMode = "merge" | "env-only"; - -/** - * `env-only` drops MODEL_CATALOG from the effective catalog so a team can - * publish an exact menu of models. Built-in entries can still be pulled back - * in by slug reference from ORCHESTRATE_MODEL_CATALOG or a role env var. - */ -export function readCatalogMode(): CatalogMode { - const raw = readEnvOverride(MODEL_ENV_CATALOG_MODE); - if (!raw) return "merge"; - const normalized = raw.toLowerCase(); - if (normalized === "merge" || normalized === "env-only") return normalized; - throw new ModelConfigError( - `${MODEL_ENV_CATALOG_MODE}="${raw}" is not valid. Use "merge" (default) or "env-only".` - ); -} - function parseSpeed(value: unknown, ctx: string): ModelProfile["speed"] { if (value === undefined) return "medium"; if (typeof value === "string" && SPEEDS.has(value as ModelProfile["speed"])) { @@ -308,26 +281,29 @@ function parseStrengths(value: unknown, ctx: string): string[] { return value as string[]; } -function parseDefaultFor(value: unknown, ctx: string): TaskType[] | undefined { +function parseDefaultFor( + value: unknown, + ctx: string +): CatalogRole[] | undefined { if (value === undefined) return undefined; if (!Array.isArray(value)) { throw new ModelConfigError( - `${ctx}: "defaultFor" must be an array of ${ALL_TASK_TYPES.join(" | ")}` + `${ctx}: "defaultFor" must be an array of ${CATALOG_ROLES.join(" | ")}` ); } - const types: TaskType[] = []; + const roles: CatalogRole[] = []; for (const item of value) { if ( typeof item !== "string" || - !ALL_TASK_TYPES.includes(item as TaskType) + !CATALOG_ROLES.includes(item as CatalogRole) ) { throw new ModelConfigError( - `${ctx}: "defaultFor" entries must be one of ${ALL_TASK_TYPES.join(", ")}` + `${ctx}: "defaultFor" entries must be one of ${CATALOG_ROLES.join(", ")}` ); } - types.push(item as TaskType); + roles.push(item as CatalogRole); } - return types; + return roles; } function optionalString( @@ -342,225 +318,138 @@ function optionalString( return value.trim(); } -/** A parsed env entry: either a reference to an existing slug, or a profile. */ -type CatalogEntryInput = - | { kind: "ref"; slug: string; defaultFor?: TaskType[] } - | { kind: "profile"; profile: ModelProfile }; +function builtinBySlug(slug: string): ModelProfile | undefined { + return MODEL_CATALOG.find(m => m.slug === slug); +} -function parseEntryObject( +/** + * Parse one ORCHESTRATE_MODEL_CATALOG entry. An entry with `id` (or + * `selection`) defines a model; an entry with only `slug` pulls in the + * built-in profile of that name, so operators can list a curated subset + * without retyping SDK params. + */ +function parseCatalogEntry( raw: Record, ctx: string -): CatalogEntryInput { +): ModelProfile { const slug = optionalString(raw.slug, "slug", ctx); const defaultFor = parseDefaultFor(raw.defaultFor, ctx); - const hasSelection = raw.selection !== undefined || raw.id !== undefined; - if (!hasSelection) { + + if (raw.selection === undefined && raw.id === undefined) { if (!slug) { throw new ModelConfigError( `${ctx}: entry needs "id" (or "selection"), or a "slug" that references a built-in model` ); } - return { kind: "ref", slug, defaultFor }; + const builtin = builtinBySlug(slug); + if (!builtin) { + throw new ModelConfigError( + `${ctx}: "${slug}" is not a built-in MODEL_CATALOG slug. Give the entry an "id" to define a new model, or use a known slug.` + ); + } + return { ...builtin, defaultFor: defaultFor ?? builtin.defaultFor }; } + const selection = normalizeModelSelection( raw.selection ?? { id: raw.id, params: raw.params }, ctx ); - const label = formatModelSelectionLabel(selection); return { - kind: "profile", - profile: { - slug: slug ?? selection.id, - selection, - summary: - optionalString(raw.summary, "summary", ctx) ?? - `Operator-configured model (${label}).`, - strengths: parseStrengths(raw.strengths, ctx), - speed: parseSpeed(raw.speed, ctx), - use: - optionalString(raw.use, "use", ctx) ?? - "Configured for this repo via orchestrate model env config. Prefer it unless the task needs a listed specialist.", - defaultFor, - }, + slug: slug ?? selection.id, + selection, + summary: + optionalString(raw.summary, "summary", ctx) ?? + `Operator-configured model (${formatModelSelectionLabel(selection)}).`, + strengths: parseStrengths(raw.strengths, ctx), + speed: parseSpeed(raw.speed, ctx), + use: + optionalString(raw.use, "use", ctx) ?? + "Configured for this repo via ORCHESTRATE_MODEL_CATALOG. Prefer it unless the task needs a listed specialist.", + defaultFor, }; } -/** Role env vars accept a slug, a bare model id, or a JSON entry. */ -function parseRoleEnvValue(raw: string, envName: string): CatalogEntryInput { - if (looksLikeSelectionJson(raw)) { - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (err) { - throw new ModelConfigError(`${envName}: ${errText(err)}`); - } - if (!isPlainObject(parsed)) { - throw new ModelConfigError(`${envName}: expected a JSON object`); - } - return parseEntryObject(parsed, envName); - } - return { kind: "ref", slug: raw }; +function readCatalogEnv(): string | undefined { + const raw = process.env[MODEL_ENV_CATALOG]?.trim(); + return raw || undefined; } -function builtinBySlug(slug: string): ModelProfile | undefined { - return MODEL_CATALOG.find(m => m.slug === slug); +/** True when this repo publishes its own catalog instead of the built-in one. */ +export function usingEnvCatalog(): boolean { + return readCatalogEnv() !== undefined; } /** - * Resolve a slug reference. Role env vars may name a model that isn't in the - * built-in catalog (`composer-2.5`); it becomes a bare `{ id }` entry. - * ORCHESTRATE_MODEL_CATALOG refs must name a built-in, since a typo there - * would silently publish a non-existent model to planners. + * The catalog planners choose from. ORCHESTRATE_MODEL_CATALOG replaces + * MODEL_CATALOG outright when set; there is no merging, so the env value is + * the complete menu. Built per call so env changes apply without reload. */ -function resolveRef( - slug: string, - ctx: string, - synthesizeIfUnknown: boolean -): ModelProfile { - const builtin = builtinBySlug(slug); - if (builtin) return { ...builtin }; - if (!synthesizeIfUnknown) { +export function effectiveModelCatalog(): ModelProfile[] { + const raw = readCatalogEnv(); + if (!raw) return MODEL_CATALOG; + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new ModelConfigError(`${MODEL_ENV_CATALOG}: ${errText(err)}`); + } + if (!Array.isArray(parsed)) { throw new ModelConfigError( - `${ctx}: "${slug}" is not a built-in MODEL_CATALOG slug. Give the entry an "id" to define a new model, or use a known slug.` + `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` ); } - return { - slug, - selection: { id: slug }, - summary: `Operator-configured model (${slug}).`, - strengths: [], - speed: "medium", - use: "Configured for this repo via orchestrate model env config. Prefer it unless the task needs a listed specialist.", - }; -} - -export interface EffectiveCatalog { - mode: CatalogMode; - catalog: ModelProfile[]; - /** Effective role defaults, by slug. Missing when nothing claims the role. */ - defaults: Partial>; -} - -/** - * Merge MODEL_CATALOG with the ORCHESTRATE_MODEL_* env config into the catalog - * planners actually see. Built per call so env changes apply without reload. - */ -export function buildEffectiveCatalog(): EffectiveCatalog { - const mode = readCatalogMode(); - const bySlug = new Map(); - const order: string[] = []; - const upsert = (profile: ModelProfile): void => { - if (!bySlug.has(profile.slug)) order.push(profile.slug); - bySlug.set(profile.slug, profile); - }; - - if (mode === "merge") { - for (const profile of MODEL_CATALOG) upsert({ ...profile }); + if (!parsed.length) { + throw new ModelConfigError( + `${MODEL_ENV_CATALOG}: needs at least one entry. Unset it to use the built-in catalog.` + ); } - const declared: Partial> = {}; + const catalog: ModelProfile[] = []; + const bySlug = new Map(); + const claimedBy = new Map(); - const rawCatalog = readEnvOverride(MODEL_ENV_CATALOG); - if (rawCatalog) { - let parsed: unknown; - try { - parsed = JSON.parse(rawCatalog); - } catch (err) { - throw new ModelConfigError(`${MODEL_ENV_CATALOG}: ${errText(err)}`); + parsed.forEach((item, i) => { + const ctx = `${MODEL_ENV_CATALOG}[${i}]`; + if (!isPlainObject(item)) { + throw new ModelConfigError(`${ctx}: expected a JSON object`); } - if (!Array.isArray(parsed)) { + const profile = parseCatalogEntry(item, ctx); + + const priorIndex = bySlug.get(profile.slug); + if (priorIndex !== undefined) { throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` + `${ctx}: duplicate slug "${profile.slug}" (also at index ${priorIndex})` ); } - parsed.forEach((item, i) => { - const ctx = `${MODEL_ENV_CATALOG}[${i}]`; - if (!isPlainObject(item)) { - throw new ModelConfigError(`${ctx}: expected a JSON object`); - } - const entry = parseEntryObject(item, ctx); - const profile = - entry.kind === "ref" - ? { - ...resolveRef(entry.slug, ctx, false), - defaultFor: entry.defaultFor, - } - : entry.profile; - upsert(profile); - for (const type of profile.defaultFor ?? []) { - const prior = declared[type]; - if (prior && prior !== profile.slug) { - throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: two entries claim the ${type} default ("${prior}" and "${profile.slug}")` - ); - } - declared[type] = profile.slug; - } - }); - } - - // Role env vars are the most specific signal, so they win over any - // defaultFor declared inside ORCHESTRATE_MODEL_CATALOG. - for (const type of ALL_TASK_TYPES) { - const envName = MODEL_ENV_BY_TYPE[type]; - const raw = readEnvOverride(envName); - if (!raw) continue; - const entry = parseRoleEnvValue(raw, envName); - const profile = - entry.kind === "ref" - ? resolveRef(entry.slug, envName, true) - : entry.profile; - const existing = bySlug.get(profile.slug); - upsert(entry.kind === "ref" && existing ? existing : profile); - declared[type] = profile.slug; - } - - // Inherit unclaimed roles from whatever remains in the catalog. - for (const type of ALL_TASK_TYPES) { - if (declared[type]) continue; - const inherited = order - .map(slug => bySlug.get(slug)) - .find(profile => profile?.defaultFor?.includes(type)); - if (inherited) declared[type] = inherited.slug; - } + bySlug.set(profile.slug, i); - // Re-stamp defaultFor so rendering reflects the resolved defaults. - const rolesBySlug = new Map(); - for (const type of ALL_TASK_TYPES) { - const slug = declared[type]; - if (!slug) continue; - const roles = rolesBySlug.get(slug) ?? []; - roles.push(type); - rolesBySlug.set(slug, roles); - } - - const catalog: ModelProfile[] = []; - for (const slug of order) { - const profile = bySlug.get(slug); - if (!profile) continue; - const roles = rolesBySlug.get(slug); - catalog.push({ ...profile, defaultFor: roles?.length ? roles : undefined }); - } + for (const role of profile.defaultFor ?? []) { + const prior = claimedBy.get(role); + if (prior) { + throw new ModelConfigError( + `${MODEL_ENV_CATALOG}: two entries claim the ${role} default ("${prior}" and "${profile.slug}")` + ); + } + claimedBy.set(role, profile.slug); + } + catalog.push(profile); + }); - return { mode, catalog, defaults: declared }; + return catalog; } -/** The catalog planners see: MODEL_CATALOG plus/minus the env config. */ -export function effectiveModelCatalog(): ModelProfile[] { - return buildEffectiveCatalog().catalog; +function defaultSlugForRole(role: CatalogRole): string | undefined { + return effectiveModelCatalog().find(m => m.defaultFor?.includes(role))?.slug; } -/** - * Fallback model slug when `tasks[].model` is omitted. Resolves against the - * effective catalog, so env config participates. - */ +/** Model slug for a task type when `tasks[].model` is omitted. */ export function defaultModelForType(type: TaskType): string { - const { defaults, mode } = buildEffectiveCatalog(); - const slug = defaults[type]; + const slug = defaultSlugForRole(type); if (slug) return slug; - if (mode === "env-only") { + if (usingEnvCatalog()) { throw new ModelConfigError( - `no ${type} default: ${MODEL_ENV_CATALOG_MODE}=env-only drops MODEL_CATALOG, so set ${MODEL_ENV_BY_TYPE[type]} or give an ${MODEL_ENV_CATALOG} entry "defaultFor": ["${type}"]` + `${MODEL_ENV_CATALOG} has no ${type} default. Add "defaultFor": ["${type}"] to one entry.` ); } throw new ModelConfigError( @@ -568,9 +457,12 @@ export function defaultModelForType(type: TaskType): string { ); } -/** Kickoff `--model` default: ORCHESTRATE_MODEL_ROOT, else the built-in root model. */ +/** + * Kickoff `--model` default. Honors a `defaultFor: ["root"]` catalog entry, + * else falls back to the built-in root model. + */ export function defaultRootModel(): string { - return readEnvOverride(MODEL_ENV_ROOT) ?? DEFAULT_ROOT_MODEL; + return defaultSlugForRole("root") ?? DEFAULT_ROOT_MODEL; } export function isKnownModel(slug: string): boolean { @@ -594,41 +486,38 @@ export function resolveModelSelection(spec: string): ModelSelection { return profile ? profile.selection : { id: trimmed }; } -/** Effective default label per task type (catalog entry or env override). */ -export function effectiveDefaultLabelForType(type: TaskType): string { - return defaultModelForType(type); -} - /** - * Fail fast on unusable ORCHESTRATE_MODEL_* config instead of surfacing it as - * a spawn error halfway through a run. + * Fail fast on unusable catalog config instead of surfacing it as a spawn + * error halfway through a run. */ export function assertModelEnvConfig(): void { - const { defaults, mode } = buildEffectiveCatalog(); - if (mode !== "env-only") return; - const missing = ALL_TASK_TYPES.filter(type => !defaults[type]); + if (!usingEnvCatalog()) return; + const catalog = effectiveModelCatalog(); + const missing = REQUIRED_ROLES.filter( + role => !catalog.some(m => m.defaultFor?.includes(role)) + ); if (!missing.length) return; throw new ModelConfigError( - `${MODEL_ENV_CATALOG_MODE}=env-only leaves no default for ${missing.join(", ")}. Set ${missing - .map(type => MODEL_ENV_BY_TYPE[type]) - .join(", ")}, or add "defaultFor" to an ${MODEL_ENV_CATALOG} entry.` + `${MODEL_ENV_CATALOG} has no default for ${missing.join(", ")}. Add "defaultFor": [${missing + .map(role => `"${role}"`) + .join(", ")}] across your entries.` ); } export function renderModelCatalog(): string { - const { catalog, mode } = buildEffectiveCatalog(); + const catalog = effectiveModelCatalog(); const lines: string[] = []; - if (mode === "env-only") { + if (usingEnvCatalog()) { lines.push( "This repo publishes an exact model menu. Use only the slugs listed below; do not reach for models outside this list." ); lines.push(""); } for (const m of catalog) { - const defaults = m.defaultFor?.length + const roles = m.defaultFor?.length ? ` (default for ${m.defaultFor.join(", ")})` : ""; - lines.push(`- \`${m.slug}\` — ${m.summary}${defaults}`); + lines.push(`- \`${m.slug}\` — ${m.summary}${roles}`); const strengths = m.strengths.length ? `; strengths: ${m.strengths.join(", ")}` : ""; From 5977e3eb11d8bffdc0e00ae1d5dd3b1eb2c44cf9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 18:18:36 +0000 Subject: [PATCH 4/6] refactor(orchestrate): trim model catalog config to essentials Reuse TaskType instead of a parallel role union: all three task types require a default, and the root planner keeps its hardcoded kickoff default rather than becoming a catalog role. Stop validating descriptive fields. Speed, strengths, and defaultFor values are passed through as written, so new model vocabulary doesn't need a plugin release; only config the CLI genuinely can't read is rejected. Also drops the JSON-in-model-field parsing that the removed per-role env vars needed, which lets agent-manager and the kickoff CLI go back to their original code. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 8 +- .../orchestrate/references/dispatcher.md | 2 +- ...des.test.ts => models-env-catalog.test.ts} | 142 ++----- .../skills/orchestrate/scripts/cli/task.ts | 8 +- .../orchestrate/scripts/core/agent-manager.ts | 13 +- .../skills/orchestrate/scripts/models.ts | 349 ++++-------------- 6 files changed, 123 insertions(+), 399 deletions(-) rename orchestrate/skills/orchestrate/scripts/__tests__/{models-env-overrides.test.ts => models-env-catalog.test.ts} (53%) diff --git a/orchestrate/README.md b/orchestrate/README.md index 4c19fb74..a98521e9 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -17,7 +17,7 @@ The skill itself lives in [`skills/orchestrate/SKILL.md`](./skills/orchestrate/S ```bash export ORCHESTRATE_MODEL_CATALOG='[ {"id":"composer-2.5","summary":"Cheap, fast worker.","defaultFor":["worker"]}, - {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier","root"]} + {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} ]' ``` @@ -28,7 +28,7 @@ An entry either **defines** a model or **references** a built-in one: - `"id"` (plus optional `"params"`) defines a model. `"slug"` names it for `tasks[].model`, defaulting to the id. - `"slug"` alone pulls in the built-in profile of that name, so you can curate a subset without retyping SDK params. Run `bun cli.ts models` with the variable unset to see the built-in slugs. -`"defaultFor"` claims roles: `worker`, `subplanner`, `verifier`, and `root` (the kickoff planner, i.e. the `--model` default). Every role except `root` needs a default somewhere in the list. +`"defaultFor"` claims task types, and each of `worker`, `subplanner`, and `verifier` needs a default somewhere in the list. Root planners are not part of the catalog; they take their model from kickoff `--model`, which defaults to `claude-opus-4-8`. Optional prose fields are worth filling in, because planners choose by capability, not by name: @@ -51,9 +51,9 @@ export ORCHESTRATE_MODEL_CATALOG='[ ### Precedence 1. Explicit `tasks[].model` in the plan -2. The `defaultFor` entry for that task's role +2. The `defaultFor` entry for that task's type -Run `bun cli.ts models` to print the effective catalog, and `bun cli.ts models --check` to probe every entry against `/v1/agents`. Malformed config (bad JSON, unknown slug reference, duplicate slug, two entries claiming one role, missing role default) exits 2 at startup with the offending index named, rather than failing mid-run. +Run `bun cli.ts models` to print the effective catalog, and `bun cli.ts models --check` to probe every entry against `/v1/agents`. Config the CLI can't read as a catalog (bad JSON, not an array, an entry with neither `id` nor a known built-in `slug`, no default for a task type) exits 2 at startup with the offending index named, rather than failing mid-run. Descriptive fields like `speed` and `strengths` are passed through as written rather than checked against a fixed vocabulary, so a wrong value shows up in the rendered catalog instead of blocking the run. Two caveats. This shapes what planners choose from, but a planner can still write any model id into `tasks[].model`, so it is guidance rather than a spend ceiling. And each spawned agent reads its own environment: set the variable as a Cursor Cloud secret for the repo so subplanners and workers inherit it, not just in the dispatcher's local shell. diff --git a/orchestrate/skills/orchestrate/references/dispatcher.md b/orchestrate/skills/orchestrate/references/dispatcher.md index aeec581d..270330b1 100644 --- a/orchestrate/skills/orchestrate/references/dispatcher.md +++ b/orchestrate/skills/orchestrate/references/dispatcher.md @@ -16,7 +16,7 @@ One-time setup: run `bun install` inside this skill's `scripts/` directory if `n bun cli.ts kickoff "" [--repo ] [--ref main] [--model claude-opus-4-8] [--slack-channel C123] [--dispatcher-name "Alex"] ``` -`--model` accepts a catalog slug, bare model id, or JSON `ModelSelection`. When omitted, it uses the catalog entry marked `defaultFor: ["root"]`, else `claude-opus-4-8`. A repo can replace the whole catalog, including every role default, with `ORCHESTRATE_MODEL_CATALOG` (see the plugin README). Run `bun cli.ts models` to print the effective catalog; a bad config exits 2 with the offending entry named. +A repo can replace the model catalog planners choose from, including each task type's default, with `ORCHESTRATE_MODEL_CATALOG` (see the plugin README). That does not cover the root planner: pass `--model` to set it, otherwise it stays `claude-opus-4-8`. Run `bun cli.ts models` to print the catalog in effect; config the CLI can't read exits 2 with the offending entry named. The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts similarity index 53% rename from orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts rename to orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts index ac55b7c9..438c92b5 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-overrides.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts @@ -2,18 +2,13 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { assertModelEnvConfig, - DEFAULT_ROOT_MODEL, defaultModelForType, - defaultRootModel, effectiveModelCatalog, - formatModelSelectionLabel, isKnownModel, MODEL_ENV_CATALOG, ModelConfigError, - parseModelSelectionJson, renderModelCatalog, resolveModelSelection, - usingEnvCatalog, } from "../models.ts"; let saved: string | undefined; @@ -32,26 +27,20 @@ afterEach(() => { else process.env[MODEL_ENV_CATALOG] = saved; }); -describe("built-in catalog (ORCHESTRATE_MODEL_CATALOG unset)", () => { - test("role defaults come from MODEL_CATALOG", () => { - expect(usingEnvCatalog()).toBe(false); - expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); - expect(defaultModelForType("subplanner")).toBe( - "claude-opus-4-8-thinking-xhigh" +describe("ORCHESTRATE_MODEL_CATALOG unset", () => { + test("the built-in catalog is in effect", () => { + expect(effectiveModelCatalog()).toBe( + // Same array identity: no copying or merging when env is unset. + effectiveModelCatalog() ); - expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); - expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); + expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); + expect(renderModelCatalog()).not.toContain("exact model menu"); }); test("whitespace-only value is treated as unset", () => { process.env[MODEL_ENV_CATALOG] = " "; - expect(usingEnvCatalog()).toBe(false); expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); }); - - test("rendered catalog has no exact-menu preamble", () => { - expect(renderModelCatalog()).not.toContain("exact model menu"); - }); }); describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { @@ -60,7 +49,6 @@ describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { { id: "composer-2.5", summary: "Cheap worker.", defaultFor: ["worker"] }, { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, ]); - expect(usingEnvCatalog()).toBe(true); expect(effectiveModelCatalog().map(m => m.slug)).toEqual([ "composer-2.5", "claude-opus-4-8", @@ -69,7 +57,7 @@ describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { expect(isKnownModel("composer-2.5")).toBe(true); }); - test("entries define role defaults", () => { + test("entries supply every task type's default", () => { setCatalog([ { id: "composer-2.5", defaultFor: ["worker"] }, { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, @@ -88,7 +76,7 @@ describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { expect(defaultModelForType("worker")).toBe("composer-2-fast"); }); - test("entry keeps params and round-trips by slug", () => { + test("a defined entry round-trips by slug with its params", () => { setCatalog([ { slug: "house-worker", @@ -112,38 +100,47 @@ describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { expect(text).toContain("speed: fast; strengths: throughput"); }); - test("slug defaults to the model id and prose is synthesized", () => { + test("slug defaults to the id and prose is filled in", () => { setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); const [entry] = effectiveModelCatalog(); expect(entry.slug).toBe("composer-2.5"); - expect(entry.summary).toContain("composer-2.5"); + expect(entry.summary).toContain("ORCHESTRATE_MODEL_CATALOG"); expect(entry.speed).toBe("medium"); }); - test("root default is configurable, else falls back", () => { + test("a model outside the catalog still passes through as a bare id", () => { setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); - expect(defaultRootModel()).toBe(DEFAULT_ROOT_MODEL); + expect(resolveModelSelection("gpt-5.5")).toEqual({ id: "gpt-5.5" }); + }); + + // Descriptive fields are passed through rather than validated, so new model + // vocabulary doesn't require a plugin release. + test("unrecognized descriptive values are passed through", () => { setCatalog([ - { id: "composer-2.5", defaultFor: ["worker"] }, - { slug: "claude-opus-4-8", defaultFor: ["root"] }, + { + id: "composer-2.5", + speed: "blistering", + strengths: ["novel-capability"], + defaultFor: ["worker", "subplanner", "verifier"], + }, ]); - expect(defaultRootModel()).toBe("claude-opus-4-8"); + expect(renderModelCatalog()).toContain( + "speed: blistering; strengths: novel-capability" + ); + expect(() => assertModelEnvConfig()).not.toThrow(); }); }); describe("catalog config errors", () => { - test("missing required role fails fast with an actionable message", () => { + test("a missing task-type default fails fast at startup", () => { setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); expect(() => assertModelEnvConfig()).toThrow(ModelConfigError); expect(() => assertModelEnvConfig()).toThrow( - /no default for subplanner, verifier/ - ); - expect(() => defaultModelForType("verifier")).toThrow( - /"defaultFor": \["verifier"\]/ + /no subplanner default.*"defaultFor": \["subplanner"\]/s ); }); - test("assertModelEnvConfig passes when every role resolves", () => { + test("assertModelEnvConfig passes when every task type resolves", () => { setCatalog([ { id: "composer-2.5", defaultFor: ["worker"] }, { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, @@ -151,82 +148,19 @@ describe("catalog config errors", () => { expect(() => assertModelEnvConfig()).not.toThrow(); }); - test("non-array, empty, and malformed JSON are rejected", () => { - process.env[MODEL_ENV_CATALOG] = '{"id":"x"}'; - expect(() => effectiveModelCatalog()).toThrow(/expected a JSON array/); - - setCatalog([]); - expect(() => effectiveModelCatalog()).toThrow(/at least one entry/); - + test("malformed JSON and non-arrays are rejected", () => { process.env[MODEL_ENV_CATALOG] = "[{id:}]"; expect(() => effectiveModelCatalog()).toThrow(ModelConfigError); - }); - - test("unknown slug reference is rejected", () => { - setCatalog([{ slug: "not-a-builtin" }]); - expect(() => effectiveModelCatalog()).toThrow( - /not a built-in MODEL_CATALOG slug/ - ); - }); - test("duplicate slugs are rejected", () => { - setCatalog([{ id: "composer-2.5" }, { id: "composer-2.5" }]); - expect(() => effectiveModelCatalog()).toThrow(/duplicate slug/); - }); - - test("two entries claiming one role is rejected", () => { - setCatalog([ - { id: "composer-2.5", defaultFor: ["worker"] }, - { id: "grok-4-5", defaultFor: ["worker"] }, - ]); - expect(() => effectiveModelCatalog()).toThrow(/claim the worker default/); + process.env[MODEL_ENV_CATALOG] = '{"id":"x"}'; + expect(() => effectiveModelCatalog()).toThrow(/expected a JSON array/); }); - test("bad field values are rejected", () => { - setCatalog([{ id: "x", speed: "blistering" }]); - expect(() => effectiveModelCatalog()).toThrow(/"speed" must be/); - - setCatalog([{ id: "x", defaultFor: ["planner"] }]); - expect(() => effectiveModelCatalog()).toThrow(/"defaultFor" entries/); - - setCatalog([{ id: "x", strengths: "fast" }]); - expect(() => effectiveModelCatalog()).toThrow(/"strengths" must be/); + test("an entry with neither id nor a known slug is rejected", () => { + setCatalog([{ slug: "not-a-builtin" }]); + expect(() => effectiveModelCatalog()).toThrow(/needs an "id"/); setCatalog([{ summary: "no id or slug" }]); - expect(() => effectiveModelCatalog()).toThrow(/entry needs "id"/); - }); -}); - -describe("selection parsing", () => { - test("invalid JSON selection throws a clear error", () => { - expect(() => parseModelSelectionJson("{not-json")).toThrow( - /invalid model selection JSON/ - ); - expect(() => parseModelSelectionJson('{"params":[]}')).toThrow( - /expected \{"id"/ - ); - expect(() => - parseModelSelectionJson('{"id":"x","params":[{"id":1,"value":"a"}]}') - ).toThrow(/each params entry/); - }); - - test("JSON selection passes through resolveModelSelection", () => { - expect( - resolveModelSelection( - '{"id":"composer-2.5","params":[{"id":"fast","value":"true"}]}' - ) - ).toEqual({ - id: "composer-2.5", - params: [{ id: "fast", value: "true" }], - }); - }); - - test("formatModelSelectionLabel renders params", () => { - expect( - formatModelSelectionLabel({ - id: "composer-2.5", - params: [{ id: "fast", value: "true" }], - }) - ).toBe("composer-2.5 (fast=true)"); + expect(() => effectiveModelCatalog()).toThrow(/needs an "id"/); }); }); diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 939a0c46..3093c913 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -4,7 +4,7 @@ import type { CursorAgentError as CursorAgentErrorValue } from "@cursor/sdk"; import type { Command } from "commander"; import { createSlackAdapter } from "../adapters/index.ts"; import { DEFAULT_MAX_RUNTIME_SEC, runOrchestrateLoop } from "../core/loop.ts"; -import { defaultRootModel, resolveModelSelection } from "../models.ts"; +import { resolveModelSelection } from "../models.ts"; import { parsePlanTaskJson, parsePlanTaskValue, @@ -108,11 +108,7 @@ export function registerTaskCommands(program: Command): void { .argument("", "Root orchestration goal") .option("--repo ", "Repository URL to orchestrate") .option("--ref ", "Starting git ref for the cloud workspace", "main") - .option( - "--model ", - 'Model id for the root planner (catalog slug, bare id, or JSON ModelSelection). Defaults to the catalog entry marked defaultFor ["root"], else claude-opus-4-8.', - defaultRootModel() - ) + .option("--model ", "Model id for the root planner", "claude-opus-4-8") .option( "--force", "Spawn a new root planner even when a recent matching run exists." diff --git a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts index dafcf3d2..32ce7199 100644 --- a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts +++ b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts @@ -140,7 +140,6 @@ import { import { defaultModelForType, isKnownModel, - looksLikeSelectionJson, resolveModelSelection, } from "../models.ts"; import { AndonPoller, SlackReactionAndonSource } from "./andon.ts"; @@ -619,13 +618,9 @@ export class AgentManager { const attemptNumber = attemptsBefore + 1; this.touch(s, { attempts: attemptNumber }); - const modelSpec = def.model ?? defaultModelForType(def.type); - // JSON ModelSelection overrides intentionally bypass the catalog. Bare - // unknown slugs still warn — they fall through to `{ id }` and may - // `invalid_model` if the backend needs params. - if (!looksLikeSelectionJson(modelSpec) && !isKnownModel(modelSpec)) { + if (def.model && !isKnownModel(def.model)) { this.logAttention( - `${def.name}: model "${modelSpec}" not in MODEL_CATALOG (spawning anyway). Run \`bun cli.ts models\` to see known slugs, or set a JSON ModelSelection via ORCHESTRATE_MODEL_* / tasks[].model.` + `${def.name}: model "${def.model}" not in MODEL_CATALOG (spawning anyway). Run \`bun cli.ts models\` to see known slugs.` ); } @@ -639,7 +634,9 @@ export class AgentManager { const agent = await Agent.create({ apiKey: this.apiKey, name: `${this.plan.rootSlug}/${def.name}`, - model: resolveModelSelection(modelSpec), + model: resolveModelSelection( + def.model ?? defaultModelForType(def.type) + ), cloud: { repos: [{ url: this.plan.repoUrl, startingRef }], autoCreatePR: def.openPR ?? false, diff --git a/orchestrate/skills/orchestrate/scripts/models.ts b/orchestrate/skills/orchestrate/scripts/models.ts index ed17bfbc..72f89ea0 100644 --- a/orchestrate/skills/orchestrate/scripts/models.ts +++ b/orchestrate/skills/orchestrate/scripts/models.ts @@ -3,20 +3,8 @@ import type { ModelSelection } from "@cursor/sdk"; import type { TaskType } from "./adapters/types.ts"; // Built-in model catalog, used when ORCHESTRATE_MODEL_CATALOG is unset. -// `defaultFor` supplies the model for a role when `tasks[].model` is omitted. - -/** Roles a catalog entry can be the default for. `root` is the kickoff planner. */ -export type CatalogRole = TaskType | "root"; - -export const CATALOG_ROLES: CatalogRole[] = [ - "worker", - "subplanner", - "verifier", - "root", -]; - -/** Roles that must resolve for a run to be spawnable. */ -const REQUIRED_ROLES: TaskType[] = ["worker", "subplanner", "verifier"]; +// `defaultFor` supplies the model for a task type when `tasks[].model` is +// omitted. Root planners take their model from kickoff `--model`, not here. export interface ModelProfile { /** User-facing slug for `tasks[].model` and `--model` flags. */ @@ -27,8 +15,8 @@ export interface ModelProfile { strengths: string[]; speed: "fast" | "medium" | "slow"; use: string; - /** Roles this profile is the default for. */ - defaultFor?: CatalogRole[]; + /** Task types this profile is the default for. */ + defaultFor?: TaskType[]; } /** @@ -37,7 +25,7 @@ export interface ModelProfile { */ export const MODEL_ENV_CATALOG = "ORCHESTRATE_MODEL_CATALOG"; -export const DEFAULT_ROOT_MODEL = "claude-opus-4-8"; +const TASK_TYPES: TaskType[] = ["worker", "subplanner", "verifier"]; // `slug` is the stable authoring name; `selection` is the canonical SDK form. // Run `bun cli.ts models --check` after SDK or backend model-schema drift. @@ -185,339 +173,148 @@ export const MODEL_CATALOG: ModelProfile[] = [ }, ]; -/** Raised when ORCHESTRATE_MODEL_CATALOG is not usable. */ +/** Raised when ORCHESTRATE_MODEL_CATALOG can't be read as a catalog. */ export class ModelConfigError extends Error {} -const SPEEDS = new Set(["fast", "medium", "slow"]); - function isPlainObject(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } -function errText(err: unknown): string { - return err instanceof Error ? err.message : String(err); -} - -/** True when `spec` looks like a JSON object literal rather than a slug. */ -export function looksLikeSelectionJson(spec: string): boolean { - return spec.trimStart().startsWith("{"); -} - -/** Validate an already-parsed `{ id, params? }` into a canonical selection. */ -export function normalizeModelSelection( - value: unknown, - ctx = "invalid model selection JSON" -): ModelSelection { - if ( - !isPlainObject(value) || - typeof value.id !== "string" || - !value.id.trim() - ) { - throw new ModelConfigError( - `${ctx}: expected {"id":"", "params"?: [{"id","value"}, ...]}` - ); - } - const selection: ModelSelection = { id: value.id.trim() }; - if (value.params !== undefined) { - if (!Array.isArray(value.params)) { - throw new ModelConfigError( - `${ctx}: "params" must be an array of {id, value}` - ); - } - const params: { id: string; value: string }[] = []; - for (const p of value.params) { - if ( - !isPlainObject(p) || - typeof p.id !== "string" || - typeof p.value !== "string" - ) { - throw new ModelConfigError( - `${ctx}: each params entry must be {id: string, value: string}` - ); - } - params.push({ id: p.id, value: p.value }); - } - selection.params = params; - } - return selection; -} - -/** Parse a JSON `ModelSelection` (`{"id":"…","params":[…]}`). */ -export function parseModelSelectionJson( - raw: string, - ctx = "invalid model selection JSON" -): ModelSelection { - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (err) { - throw new ModelConfigError(`${ctx}: ${errText(err)}`); - } - return normalizeModelSelection(parsed, ctx); -} - -/** Compact label for catalog / attention logs. */ -export function formatModelSelectionLabel(selection: ModelSelection): string { - if (!selection.params?.length) return selection.id; - const params = selection.params.map(p => `${p.id}=${p.value}`).join(", "); - return `${selection.id} (${params})`; -} - -function parseSpeed(value: unknown, ctx: string): ModelProfile["speed"] { - if (value === undefined) return "medium"; - if (typeof value === "string" && SPEEDS.has(value as ModelProfile["speed"])) { - return value as ModelProfile["speed"]; - } - throw new ModelConfigError(`${ctx}: "speed" must be fast, medium, or slow`); -} - -function parseStrengths(value: unknown, ctx: string): string[] { - if (value === undefined) return []; - if (!Array.isArray(value) || value.some(v => typeof v !== "string")) { - throw new ModelConfigError( - `${ctx}: "strengths" must be an array of strings` - ); - } - return value as string[]; -} - -function parseDefaultFor( - value: unknown, - ctx: string -): CatalogRole[] | undefined { - if (value === undefined) return undefined; - if (!Array.isArray(value)) { - throw new ModelConfigError( - `${ctx}: "defaultFor" must be an array of ${CATALOG_ROLES.join(" | ")}` - ); - } - const roles: CatalogRole[] = []; - for (const item of value) { - if ( - typeof item !== "string" || - !CATALOG_ROLES.includes(item as CatalogRole) - ) { - throw new ModelConfigError( - `${ctx}: "defaultFor" entries must be one of ${CATALOG_ROLES.join(", ")}` - ); - } - roles.push(item as CatalogRole); - } - return roles; -} - -function optionalString( - value: unknown, - field: string, - ctx: string -): string | undefined { - if (value === undefined) return undefined; - if (typeof value !== "string" || !value.trim()) { - throw new ModelConfigError(`${ctx}: "${field}" must be a non-empty string`); - } - return value.trim(); -} - -function builtinBySlug(slug: string): ModelProfile | undefined { - return MODEL_CATALOG.find(m => m.slug === slug); -} - /** - * Parse one ORCHESTRATE_MODEL_CATALOG entry. An entry with `id` (or - * `selection`) defines a model; an entry with only `slug` pulls in the - * built-in profile of that name, so operators can list a curated subset - * without retyping SDK params. + * Build one catalog entry. An entry with `id` (plus optional `params`) + * defines a model; an entry with only `slug` pulls in the built-in profile of + * that name, so operators can list a curated subset without retyping SDK + * params. Descriptive fields are taken as given: a wrong value shows up in the + * next spawn, which beats validating shapes that drift as models change. */ -function parseCatalogEntry( +function toModelProfile( raw: Record, ctx: string ): ModelProfile { - const slug = optionalString(raw.slug, "slug", ctx); - const defaultFor = parseDefaultFor(raw.defaultFor, ctx); + const slug = typeof raw.slug === "string" ? raw.slug.trim() : undefined; + const defaultFor = Array.isArray(raw.defaultFor) + ? (raw.defaultFor as TaskType[]) + : undefined; - if (raw.selection === undefined && raw.id === undefined) { - if (!slug) { - throw new ModelConfigError( - `${ctx}: entry needs "id" (or "selection"), or a "slug" that references a built-in model` - ); - } - const builtin = builtinBySlug(slug); + if (raw.id === undefined) { + const builtin = MODEL_CATALOG.find(m => m.slug === slug); if (!builtin) { throw new ModelConfigError( - `${ctx}: "${slug}" is not a built-in MODEL_CATALOG slug. Give the entry an "id" to define a new model, or use a known slug.` + `${ctx}: needs an "id" to define a model, or a "slug" naming a built-in one (got ${JSON.stringify(slug ?? null)})` ); } return { ...builtin, defaultFor: defaultFor ?? builtin.defaultFor }; } - const selection = normalizeModelSelection( - raw.selection ?? { id: raw.id, params: raw.params }, - ctx - ); + if (typeof raw.id !== "string") { + throw new ModelConfigError(`${ctx}: "id" must be a string`); + } + const selection: ModelSelection = { id: raw.id }; + if (Array.isArray(raw.params)) { + selection.params = raw.params as ModelSelection["params"]; + } return { - slug: slug ?? selection.id, + slug: slug ?? raw.id, selection, summary: - optionalString(raw.summary, "summary", ctx) ?? - `Operator-configured model (${formatModelSelectionLabel(selection)}).`, - strengths: parseStrengths(raw.strengths, ctx), - speed: parseSpeed(raw.speed, ctx), + typeof raw.summary === "string" + ? raw.summary + : `Configured for this repo via ${MODEL_ENV_CATALOG}.`, + strengths: Array.isArray(raw.strengths) ? (raw.strengths as string[]) : [], + speed: + typeof raw.speed === "string" + ? (raw.speed as ModelProfile["speed"]) + : "medium", use: - optionalString(raw.use, "use", ctx) ?? - "Configured for this repo via ORCHESTRATE_MODEL_CATALOG. Prefer it unless the task needs a listed specialist.", + typeof raw.use === "string" + ? raw.use + : "Prefer this unless the task needs a listed specialist.", defaultFor, }; } -function readCatalogEnv(): string | undefined { +function readEnvCatalog(): ModelProfile[] | undefined { const raw = process.env[MODEL_ENV_CATALOG]?.trim(); - return raw || undefined; -} - -/** True when this repo publishes its own catalog instead of the built-in one. */ -export function usingEnvCatalog(): boolean { - return readCatalogEnv() !== undefined; -} - -/** - * The catalog planners choose from. ORCHESTRATE_MODEL_CATALOG replaces - * MODEL_CATALOG outright when set; there is no merging, so the env value is - * the complete menu. Built per call so env changes apply without reload. - */ -export function effectiveModelCatalog(): ModelProfile[] { - const raw = readCatalogEnv(); - if (!raw) return MODEL_CATALOG; + if (!raw) return undefined; let parsed: unknown; try { parsed = JSON.parse(raw); } catch (err) { - throw new ModelConfigError(`${MODEL_ENV_CATALOG}: ${errText(err)}`); - } - if (!Array.isArray(parsed)) { throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` + `${MODEL_ENV_CATALOG}: ${err instanceof Error ? err.message : String(err)}` ); } - if (!parsed.length) { + if (!Array.isArray(parsed)) { throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: needs at least one entry. Unset it to use the built-in catalog.` + `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` ); } - - const catalog: ModelProfile[] = []; - const bySlug = new Map(); - const claimedBy = new Map(); - - parsed.forEach((item, i) => { + return parsed.map((item, i) => { const ctx = `${MODEL_ENV_CATALOG}[${i}]`; if (!isPlainObject(item)) { throw new ModelConfigError(`${ctx}: expected a JSON object`); } - const profile = parseCatalogEntry(item, ctx); - - const priorIndex = bySlug.get(profile.slug); - if (priorIndex !== undefined) { - throw new ModelConfigError( - `${ctx}: duplicate slug "${profile.slug}" (also at index ${priorIndex})` - ); - } - bySlug.set(profile.slug, i); - - for (const role of profile.defaultFor ?? []) { - const prior = claimedBy.get(role); - if (prior) { - throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: two entries claim the ${role} default ("${prior}" and "${profile.slug}")` - ); - } - claimedBy.set(role, profile.slug); - } - catalog.push(profile); + return toModelProfile(item, ctx); }); - - return catalog; } -function defaultSlugForRole(role: CatalogRole): string | undefined { - return effectiveModelCatalog().find(m => m.defaultFor?.includes(role))?.slug; +/** + * The catalog planners choose from. ORCHESTRATE_MODEL_CATALOG replaces + * MODEL_CATALOG outright when set; there is no merging, so the configured + * list is the complete menu. Built per call so env changes apply on the spot. + */ +export function effectiveModelCatalog(): ModelProfile[] { + return readEnvCatalog() ?? MODEL_CATALOG; } /** Model slug for a task type when `tasks[].model` is omitted. */ export function defaultModelForType(type: TaskType): string { - const slug = defaultSlugForRole(type); - if (slug) return slug; - if (usingEnvCatalog()) { - throw new ModelConfigError( - `${MODEL_ENV_CATALOG} has no ${type} default. Add "defaultFor": ["${type}"] to one entry.` - ); - } + const catalog = readEnvCatalog(); + const match = (catalog ?? MODEL_CATALOG).find(m => + m.defaultFor?.includes(type) + ); + if (match) return match.slug; throw new ModelConfigError( - `MODEL_CATALOG missing default for TaskType "${type}"` + catalog + ? `${MODEL_ENV_CATALOG} has no ${type} default. Add "defaultFor": ["${type}"] to one entry.` + : `MODEL_CATALOG missing default for TaskType "${type}"` ); } -/** - * Kickoff `--model` default. Honors a `defaultFor: ["root"]` catalog entry, - * else falls back to the built-in root model. - */ -export function defaultRootModel(): string { - return defaultSlugForRole("root") ?? DEFAULT_ROOT_MODEL; -} - export function isKnownModel(slug: string): boolean { - if (looksLikeSelectionJson(slug)) return false; return effectiveModelCatalog().some(m => m.slug === slug); } -/** - * Resolve an authoring slug, bare model id, or JSON ModelSelection into the - * canonical SDK form passed to `Agent.create({ model })`. - * - * Unknown slugs pass through as a bare `{ id }` so planners can reach - * server-side models that aren't in the catalog. - */ -export function resolveModelSelection(spec: string): ModelSelection { - const trimmed = spec.trim(); - if (looksLikeSelectionJson(trimmed)) { - return parseModelSelectionJson(trimmed); - } - const profile = effectiveModelCatalog().find(m => m.slug === trimmed); - return profile ? profile.selection : { id: trimmed }; +/** Unknown slugs pass through as a bare `{ id }` so planners can reach + * server-side models that aren't in our prescriptive catalog. */ +export function resolveModelSelection(slug: string): ModelSelection { + const profile = effectiveModelCatalog().find(m => m.slug === slug); + return profile ? profile.selection : { id: slug }; } /** - * Fail fast on unusable catalog config instead of surfacing it as a spawn - * error halfway through a run. + * Surface a broken catalog at CLI startup rather than as a spawn failure + * partway through a run. */ export function assertModelEnvConfig(): void { - if (!usingEnvCatalog()) return; - const catalog = effectiveModelCatalog(); - const missing = REQUIRED_ROLES.filter( - role => !catalog.some(m => m.defaultFor?.includes(role)) - ); - if (!missing.length) return; - throw new ModelConfigError( - `${MODEL_ENV_CATALOG} has no default for ${missing.join(", ")}. Add "defaultFor": [${missing - .map(role => `"${role}"`) - .join(", ")}] across your entries.` - ); + for (const type of TASK_TYPES) defaultModelForType(type); } export function renderModelCatalog(): string { - const catalog = effectiveModelCatalog(); + const envCatalog = readEnvCatalog(); const lines: string[] = []; - if (usingEnvCatalog()) { + if (envCatalog) { lines.push( "This repo publishes an exact model menu. Use only the slugs listed below; do not reach for models outside this list." ); lines.push(""); } - for (const m of catalog) { - const roles = m.defaultFor?.length + for (const m of envCatalog ?? MODEL_CATALOG) { + const defaults = m.defaultFor?.length ? ` (default for ${m.defaultFor.join(", ")})` : ""; - lines.push(`- \`${m.slug}\` — ${m.summary}${roles}`); + lines.push(`- \`${m.slug}\` — ${m.summary}${defaults}`); const strengths = m.strengths.length ? `; strengths: ${m.strengths.join(", ")}` : ""; From 8d36b32c50e0902e1e9f233aadcf62e2b3d5fb0b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 18:28:48 +0000 Subject: [PATCH 5/6] refactor(orchestrate): validate model catalog against a zod schema Require ORCHESTRATE_MODEL_CATALOG to hold entries in the same shape as MODEL_CATALOG, so the hand-rolled entry builder goes away: no id-only shorthand, no built-in slug references, no synthesized prose. Parsing is now one call to the shared parseJsonWithSchema helper, which also means field-level errors and the existing PlanValidationError exit path instead of a bespoke error class. Publishes schemas/model-catalog.schema.json for editor validation, and adds `models --json` to emit the catalog in that shape as a starting point. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 64 ++++--- orchestrate/skills/orchestrate/SKILL.md | 2 +- .../schemas/model-catalog.schema.json | 92 ++++++++++ .../__tests__/models-env-catalog.test.ts | 164 +++++++++--------- .../skills/orchestrate/scripts/cli/index.ts | 7 +- .../skills/orchestrate/scripts/cli/inspect.ts | 12 +- .../skills/orchestrate/scripts/models.ts | 126 ++------------ .../skills/orchestrate/scripts/schemas.ts | 46 +++++ .../scripts/tools/generate-json-schemas.ts | 11 +- 9 files changed, 300 insertions(+), 224 deletions(-) create mode 100644 orchestrate/skills/orchestrate/schemas/model-catalog.schema.json diff --git a/orchestrate/README.md b/orchestrate/README.md index a98521e9..b58eb2e5 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -12,48 +12,56 @@ The skill itself lives in [`skills/orchestrate/SKILL.md`](./skills/orchestrate/S ## Model catalog (optional) -`ORCHESTRATE_MODEL_CATALOG` replaces the built-in model catalog with your own. When it is set, that list is the complete menu: it is what planners choose `tasks[].model` from, what `bun cli.ts models` prints, and where role defaults come from. Nothing is merged with the built-in catalog, so what you write is exactly what runs. Use it to steer cost without editing the plugin. +`ORCHESTRATE_MODEL_CATALOG` replaces the built-in model catalog with your own. When it is set, that list is the complete menu: it is what planners choose `tasks[].model` from, what `bun cli.ts models` prints, and where each task type's default comes from. Nothing is merged with the built-in catalog, so what you write is exactly what runs. Use it to steer cost without editing the plugin. + +The value is a JSON array in the same shape as the built-in catalog, validated against [`skills/orchestrate/schemas/model-catalog.schema.json`](./skills/orchestrate/schemas/model-catalog.schema.json). Start from the built-in list rather than writing entries by hand: ```bash -export ORCHESTRATE_MODEL_CATALOG='[ - {"id":"composer-2.5","summary":"Cheap, fast worker.","defaultFor":["worker"]}, - {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} -]' +bun skills/orchestrate/scripts/cli.ts models --json > catalog.json +# edit catalog.json: drop what you don't want, move defaultFor where you want it +export ORCHESTRATE_MODEL_CATALOG="$(cat catalog.json)" ``` -### Entries - -An entry either **defines** a model or **references** a built-in one: - -- `"id"` (plus optional `"params"`) defines a model. `"slug"` names it for `tasks[].model`, defaulting to the id. -- `"slug"` alone pulls in the built-in profile of that name, so you can curate a subset without retyping SDK params. Run `bun cli.ts models` with the variable unset to see the built-in slugs. +Every entry needs `slug`, `selection`, `summary`, `strengths`, `speed`, and `use`. `defaultFor` and `selection.params` are optional: -`"defaultFor"` claims task types, and each of `worker`, `subplanner`, and `verifier` needs a default somewhere in the list. Root planners are not part of the catalog; they take their model from kickoff `--model`, which defaults to `claude-opus-4-8`. - -Optional prose fields are worth filling in, because planners choose by capability, not by name: - -```bash -export ORCHESTRATE_MODEL_CATALOG='[ +```json +[ { - "slug":"house-worker", - "id":"composer-2.5", - "params":[{"id":"fast","value":"true"}], - "summary":"House worker model.", - "use":"Use for all bounded implementation work.", - "speed":"fast", - "strengths":["throughput"], - "defaultFor":["worker"] + "slug": "house-worker", + "selection": { "id": "composer-2.5", "params": [{ "id": "fast", "value": "true" }] }, + "summary": "Cheap, fast worker.", + "strengths": ["throughput", "well-scoped implementation"], + "speed": "fast", + "use": "Use for all bounded implementation work.", + "defaultFor": ["worker"] }, - {"slug":"claude-opus-4-8","defaultFor":["subplanner","verifier"]} -]' + { + "slug": "house-planner", + "selection": { "id": "claude-opus-4-8" }, + "summary": "Frontier judgment for decomposition and acceptance checks.", + "strengths": ["judgment", "ambiguity resolution"], + "speed": "slow", + "use": "Use when the work needs design decisions rather than execution.", + "defaultFor": ["subplanner", "verifier"] + } +] ``` +`summary`, `strengths`, and `use` are required because planners select by capability, not by model name. An entry with thin prose tends to get passed over. `speed` is a free-form string, so new model vocabulary doesn't need a plugin release. + +Each of `worker`, `subplanner`, and `verifier` needs a `defaultFor` somewhere in the list. Root planners are not part of the catalog; they take their model from kickoff `--model`, which defaults to `claude-opus-4-8`. + ### Precedence 1. Explicit `tasks[].model` in the plan 2. The `defaultFor` entry for that task's type -Run `bun cli.ts models` to print the effective catalog, and `bun cli.ts models --check` to probe every entry against `/v1/agents`. Config the CLI can't read as a catalog (bad JSON, not an array, an entry with neither `id` nor a known built-in `slug`, no default for a task type) exits 2 at startup with the offending index named, rather than failing mid-run. Descriptive fields like `speed` and `strengths` are passed through as written rather than checked against a fixed vocabulary, so a wrong value shows up in the rendered catalog instead of blocking the run. +Run `bun cli.ts models` to print the catalog in effect, and `bun cli.ts models --check` to probe every entry against `/v1/agents`. Invalid config exits 2 at startup, naming the offending entry and field, rather than failing mid-run: + +``` +ORCHESTRATE_MODEL_CATALOG failed zod validation: + [0].summary: Required +``` Two caveats. This shapes what planners choose from, but a planner can still write any model id into `tasks[].model`, so it is guidance rather than a spend ceiling. And each spawned agent reads its own environment: set the variable as a Cursor Cloud secret for the repo so subplanners and workers inherit it, not just in the dispatcher's local shell. diff --git a/orchestrate/skills/orchestrate/SKILL.md b/orchestrate/skills/orchestrate/SKILL.md index 29283c8c..15a532e5 100644 --- a/orchestrate/skills/orchestrate/SKILL.md +++ b/orchestrate/skills/orchestrate/SKILL.md @@ -14,7 +14,7 @@ An explicit `/orchestrate ` fans out a large task across parallel Cursor c - `CURSOR_API_KEY` must be a personal/user key. Create it from [Cursor Dashboard > Integrations](https://cursor.com/dashboard/integrations), then read `cursor-sdk` Auth before using it. - `SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. -- `ORCHESTRATE_MODEL_CATALOG` is optional. When set, its JSON array replaces the built-in model catalog outright: it becomes the list planners pick `tasks[].model` from, and its `defaultFor` entries supply each role's default. `bun cli.ts models` prints whichever catalog is in effect. See the plugin README. +- `ORCHESTRATE_MODEL_CATALOG` is optional. When set, its JSON array replaces the built-in model catalog outright: it becomes the list planners pick `tasks[].model` from, and its `defaultFor` entries supply each task type's default. It is validated against `schemas/model-catalog.schema.json`; `bun cli.ts models` prints whichever catalog is in effect and `--json` emits it in that shape. See the plugin README. ## Core principles diff --git a/orchestrate/skills/orchestrate/schemas/model-catalog.schema.json b/orchestrate/skills/orchestrate/schemas/model-catalog.schema.json new file mode 100644 index 00000000..a4d9abcd --- /dev/null +++ b/orchestrate/skills/orchestrate/schemas/model-catalog.schema.json @@ -0,0 +1,92 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://cursor/orchestrate/model-catalog.schema.json", + "title": "orchestrate ORCHESTRATE_MODEL_CATALOG", + "description": "Optional operator-authored model catalog. When ORCHESTRATE_MODEL_CATALOG is set, it replaces the built-in catalog planners choose `tasks[].model` from.", + "type": "array", + "items": { + "type": "object", + "properties": { + "slug": { + "type": "string", + "minLength": 1, + "description": "Authoring name planners write into `tasks[].model`." + }, + "selection": { + "type": "object", + "properties": { + "id": { + "type": "string", + "minLength": 1, + "description": "Model id as accepted by the Cursor API." + }, + "params": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "value": { + "type": "string" + } + }, + "required": [ + "id", + "value" + ], + "additionalProperties": false + }, + "description": "Model parameters, e.g. reasoning, effort, thinking, fast." + } + }, + "required": [ + "id" + ], + "additionalProperties": false, + "description": "Canonical SDK selection passed to `Agent.create({ model })`." + }, + "summary": { + "type": "string", + "description": "One-line description planners read." + }, + "strengths": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Capability keywords planners match a task against." + }, + "speed": { + "type": "string", + "description": "Relative latency, e.g. fast, medium, slow." + }, + "use": { + "type": "string", + "description": "When a planner should pick this model." + }, + "defaultFor": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "worker", + "subplanner", + "verifier" + ] + }, + "description": "Task types that use this model when `tasks[].model` is omitted." + } + }, + "required": [ + "slug", + "selection", + "summary", + "strengths", + "speed", + "use" + ], + "additionalProperties": false + } +} diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts index 438c92b5..463a4e5a 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/models-env-catalog.test.ts @@ -1,18 +1,34 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { PlanValidationError } from "../errors.ts"; import { assertModelEnvConfig, defaultModelForType, effectiveModelCatalog, isKnownModel, + MODEL_CATALOG, MODEL_ENV_CATALOG, - ModelConfigError, renderModelCatalog, resolveModelSelection, } from "../models.ts"; let saved: string | undefined; +/** A minimally complete entry; every field the schema requires. */ +function entry( + overrides: Record = {} +): Record { + return { + slug: "house-worker", + selection: { id: "composer-2.5" }, + summary: "House worker model.", + strengths: ["throughput"], + speed: "fast", + use: "Use for all bounded implementation work.", + ...overrides, + }; +} + function setCatalog(entries: unknown): void { process.env[MODEL_ENV_CATALOG] = JSON.stringify(entries); } @@ -29,70 +45,62 @@ afterEach(() => { describe("ORCHESTRATE_MODEL_CATALOG unset", () => { test("the built-in catalog is in effect", () => { - expect(effectiveModelCatalog()).toBe( - // Same array identity: no copying or merging when env is unset. - effectiveModelCatalog() - ); + expect(effectiveModelCatalog()).toBe(MODEL_CATALOG); expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); expect(renderModelCatalog()).not.toContain("exact model menu"); }); test("whitespace-only value is treated as unset", () => { process.env[MODEL_ENV_CATALOG] = " "; - expect(defaultModelForType("worker")).toBe("gpt-5.5-high-fast"); + expect(effectiveModelCatalog()).toBe(MODEL_CATALOG); }); }); describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { test("only the listed models are published", () => { - setCatalog([ - { id: "composer-2.5", summary: "Cheap worker.", defaultFor: ["worker"] }, - { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, - ]); - expect(effectiveModelCatalog().map(m => m.slug)).toEqual([ - "composer-2.5", - "claude-opus-4-8", - ]); + setCatalog([entry({ defaultFor: ["worker", "subplanner", "verifier"] })]); + expect(effectiveModelCatalog().map(m => m.slug)).toEqual(["house-worker"]); expect(isKnownModel("gpt-5.5-high-fast")).toBe(false); - expect(isKnownModel("composer-2.5")).toBe(true); + expect(isKnownModel("house-worker")).toBe(true); }); test("entries supply every task type's default", () => { setCatalog([ - { id: "composer-2.5", defaultFor: ["worker"] }, - { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, + entry({ defaultFor: ["worker"] }), + entry({ + slug: "house-planner", + selection: { id: "claude-opus-4-8" }, + defaultFor: ["subplanner", "verifier"], + }), ]); - expect(defaultModelForType("worker")).toBe("composer-2.5"); - expect(defaultModelForType("subplanner")).toBe("claude-opus-4-8"); - expect(defaultModelForType("verifier")).toBe("claude-opus-4-8"); + expect(defaultModelForType("worker")).toBe("house-worker"); + expect(defaultModelForType("subplanner")).toBe("house-planner"); + expect(defaultModelForType("verifier")).toBe("house-planner"); }); - test("slug-only entry pulls in a built-in with its params", () => { - setCatalog([{ slug: "composer-2-fast", defaultFor: ["worker"] }]); - expect(resolveModelSelection("composer-2-fast")).toEqual({ - id: "composer-2", - params: [{ id: "fast", value: "true" }], - }); - expect(defaultModelForType("worker")).toBe("composer-2-fast"); - }); - - test("a defined entry round-trips by slug with its params", () => { + test("a slug resolves to its full selection, params included", () => { setCatalog([ - { - slug: "house-worker", - id: "composer-2.5", - params: [{ id: "fast", value: "true" }], - summary: "House worker model.", - use: "Use for all bounded implementation work.", - speed: "fast", - strengths: ["throughput"], - defaultFor: ["worker"], - }, + entry({ + selection: { + id: "composer-2.5", + params: [{ id: "fast", value: "true" }], + }, + defaultFor: ["worker", "subplanner", "verifier"], + }), ]); expect(resolveModelSelection("house-worker")).toEqual({ id: "composer-2.5", params: [{ id: "fast", value: "true" }], }); + }); + + test("a model outside the catalog still passes through as a bare id", () => { + setCatalog([entry({ defaultFor: ["worker", "subplanner", "verifier"] })]); + expect(resolveModelSelection("gpt-5.5")).toEqual({ id: "gpt-5.5" }); + }); + + test("the rendered catalog is what planners see", () => { + setCatalog([entry({ defaultFor: ["worker"] })]); const text = renderModelCatalog(); expect(text).toContain("exact model menu"); expect(text).toContain("`house-worker` — House worker model."); @@ -100,67 +108,65 @@ describe("ORCHESTRATE_MODEL_CATALOG replaces the built-in catalog", () => { expect(text).toContain("speed: fast; strengths: throughput"); }); - test("slug defaults to the id and prose is filled in", () => { - setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); - const [entry] = effectiveModelCatalog(); - expect(entry.slug).toBe("composer-2.5"); - expect(entry.summary).toContain("ORCHESTRATE_MODEL_CATALOG"); - expect(entry.speed).toBe("medium"); - }); - - test("a model outside the catalog still passes through as a bare id", () => { - setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); - expect(resolveModelSelection("gpt-5.5")).toEqual({ id: "gpt-5.5" }); - }); - - // Descriptive fields are passed through rather than validated, so new model - // vocabulary doesn't require a plugin release. - test("unrecognized descriptive values are passed through", () => { + // `speed` is a free-form string so new model vocabulary doesn't require a + // plugin release. + test("unrecognized speed values are passed through", () => { setCatalog([ - { - id: "composer-2.5", + entry({ speed: "blistering", - strengths: ["novel-capability"], defaultFor: ["worker", "subplanner", "verifier"], - }, + }), ]); - expect(renderModelCatalog()).toContain( - "speed: blistering; strengths: novel-capability" - ); + expect(renderModelCatalog()).toContain("speed: blistering"); + expect(() => assertModelEnvConfig()).not.toThrow(); + }); + + test("the built-in catalog round-trips through the schema", () => { + // `bun cli.ts models --json` is documented as a starting point, so its + // output has to be valid input. + setCatalog(MODEL_CATALOG); + expect(effectiveModelCatalog()).toEqual(MODEL_CATALOG); expect(() => assertModelEnvConfig()).not.toThrow(); }); }); describe("catalog config errors", () => { test("a missing task-type default fails fast at startup", () => { - setCatalog([{ id: "composer-2.5", defaultFor: ["worker"] }]); - expect(() => assertModelEnvConfig()).toThrow(ModelConfigError); + setCatalog([entry({ defaultFor: ["worker"] })]); + expect(() => assertModelEnvConfig()).toThrow(PlanValidationError); expect(() => assertModelEnvConfig()).toThrow( /no subplanner default.*"defaultFor": \["subplanner"\]/s ); }); test("assertModelEnvConfig passes when every task type resolves", () => { - setCatalog([ - { id: "composer-2.5", defaultFor: ["worker"] }, - { slug: "claude-opus-4-8", defaultFor: ["subplanner", "verifier"] }, - ]); + setCatalog([entry({ defaultFor: ["worker", "subplanner", "verifier"] })]); expect(() => assertModelEnvConfig()).not.toThrow(); }); - test("malformed JSON and non-arrays are rejected", () => { - process.env[MODEL_ENV_CATALOG] = "[{id:}]"; - expect(() => effectiveModelCatalog()).toThrow(ModelConfigError); + test("malformed JSON is rejected", () => { + process.env[MODEL_ENV_CATALOG] = "[{slug:}]"; + expect(() => effectiveModelCatalog()).toThrow(/is not valid JSON/); + }); - process.env[MODEL_ENV_CATALOG] = '{"id":"x"}'; - expect(() => effectiveModelCatalog()).toThrow(/expected a JSON array/); + test("an incomplete entry is rejected with the offending field", () => { + const { summary, ...withoutSummary } = entry(); + expect(summary).toBeDefined(); + setCatalog([withoutSummary]); + expect(() => effectiveModelCatalog()).toThrow(PlanValidationError); + expect(() => effectiveModelCatalog()).toThrow(/\[0\]\.summary/); }); - test("an entry with neither id nor a known slug is rejected", () => { - setCatalog([{ slug: "not-a-builtin" }]); - expect(() => effectiveModelCatalog()).toThrow(/needs an "id"/); + test("a bad selection or defaultFor is rejected", () => { + setCatalog([entry({ selection: { id: "" } })]); + expect(() => effectiveModelCatalog()).toThrow(/\[0\]\.selection\.id/); + + setCatalog([entry({ defaultFor: ["planner"] })]); + expect(() => effectiveModelCatalog()).toThrow(/\[0\]\.defaultFor/); + }); - setCatalog([{ summary: "no id or slug" }]); - expect(() => effectiveModelCatalog()).toThrow(/needs an "id"/); + test("a non-array value is rejected", () => { + process.env[MODEL_ENV_CATALOG] = '{"slug":"x"}'; + expect(() => effectiveModelCatalog()).toThrow(PlanValidationError); }); }); diff --git a/orchestrate/skills/orchestrate/scripts/cli/index.ts b/orchestrate/skills/orchestrate/scripts/cli/index.ts index 93df3fa1..c915c8f2 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/index.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/index.ts @@ -1,7 +1,8 @@ #!/usr/bin/env bun import { Command } from "commander"; -import { assertModelEnvConfig, ModelConfigError } from "../models.ts"; +import { PlanValidationError } from "../errors.ts"; +import { assertModelEnvConfig } from "../models.ts"; import { registerAndonCommands } from "./andon.ts"; import { registerCommentCommands } from "./comments.ts"; import { registerForensicsCommands } from "./forensics.ts"; @@ -12,8 +13,8 @@ export async function main(argv: string[] = process.argv): Promise { try { assertModelEnvConfig(); } catch (err) { - if (err instanceof ModelConfigError) { - console.error(`orchestrate model config: ${err.message}`); + if (err instanceof PlanValidationError) { + console.error(err.message); process.exit(2); } throw err; diff --git a/orchestrate/skills/orchestrate/scripts/cli/inspect.ts b/orchestrate/skills/orchestrate/scripts/cli/inspect.ts index 0e2db70a..4cda15f4 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/inspect.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/inspect.ts @@ -1,7 +1,7 @@ import type { Command } from "commander"; import { isAndonActive } from "../core/andon.ts"; import { renderPrompt } from "../core/prompts.ts"; -import { renderModelCatalog } from "../models.ts"; +import { effectiveModelCatalog, renderModelCatalog } from "../models.ts"; import type { TaskState } from "../schemas.ts"; import { firstChars, loadOrBail, parsePositiveIntegerOrBail } from "./util.ts"; @@ -156,10 +156,18 @@ export function registerInspectCommands(program: Command): void { "--check", "Validate each catalog entry against /v1/agents. Run after SDK or backend model-schema changes, or when kickoff/spawn returns invalid_model." ) + .option( + "--json", + "Print the catalog as JSON in ORCHESTRATE_MODEL_CATALOG's shape. Copy this to start a repo-specific catalog." + ) .description( "Print the model catalog. Planners consult this when setting `tasks[].model`." ) - .action(async (opts: { check?: boolean }) => { + .action(async (opts: { check?: boolean; json?: boolean }) => { + if (opts.json) { + console.log(JSON.stringify(effectiveModelCatalog(), null, 2)); + return; + } if (!opts.check) { console.log(renderModelCatalog()); return; diff --git a/orchestrate/skills/orchestrate/scripts/models.ts b/orchestrate/skills/orchestrate/scripts/models.ts index 72f89ea0..9db98506 100644 --- a/orchestrate/skills/orchestrate/scripts/models.ts +++ b/orchestrate/skills/orchestrate/scripts/models.ts @@ -1,27 +1,18 @@ import type { ModelSelection } from "@cursor/sdk"; import type { TaskType } from "./adapters/types.ts"; +import { PlanValidationError } from "./errors.ts"; +import { type ModelProfile, parseModelCatalogJson } from "./schemas.ts"; + +export type { ModelProfile }; // Built-in model catalog, used when ORCHESTRATE_MODEL_CATALOG is unset. // `defaultFor` supplies the model for a task type when `tasks[].model` is // omitted. Root planners take their model from kickoff `--model`, not here. -export interface ModelProfile { - /** User-facing slug for `tasks[].model` and `--model` flags. */ - slug: string; - /** Canonical SDK selection passed to `Agent.create({ model })`. */ - selection: ModelSelection; - summary: string; - strengths: string[]; - speed: "fast" | "medium" | "slow"; - use: string; - /** Task types this profile is the default for. */ - defaultFor?: TaskType[]; -} - /** - * Env var holding the whole catalog as JSON. When set it replaces - * MODEL_CATALOG outright; entries may still reference a built-in by slug. + * Env var holding the whole catalog as JSON, in the same shape as + * MODEL_CATALOG below. When set it replaces MODEL_CATALOG outright. */ export const MODEL_ENV_CATALOG = "ORCHESTRATE_MODEL_CATALOG"; @@ -173,110 +164,26 @@ export const MODEL_CATALOG: ModelProfile[] = [ }, ]; -/** Raised when ORCHESTRATE_MODEL_CATALOG can't be read as a catalog. */ -export class ModelConfigError extends Error {} - -function isPlainObject(value: unknown): value is Record { - return value !== null && typeof value === "object" && !Array.isArray(value); -} - -/** - * Build one catalog entry. An entry with `id` (plus optional `params`) - * defines a model; an entry with only `slug` pulls in the built-in profile of - * that name, so operators can list a curated subset without retyping SDK - * params. Descriptive fields are taken as given: a wrong value shows up in the - * next spawn, which beats validating shapes that drift as models change. - */ -function toModelProfile( - raw: Record, - ctx: string -): ModelProfile { - const slug = typeof raw.slug === "string" ? raw.slug.trim() : undefined; - const defaultFor = Array.isArray(raw.defaultFor) - ? (raw.defaultFor as TaskType[]) - : undefined; - - if (raw.id === undefined) { - const builtin = MODEL_CATALOG.find(m => m.slug === slug); - if (!builtin) { - throw new ModelConfigError( - `${ctx}: needs an "id" to define a model, or a "slug" naming a built-in one (got ${JSON.stringify(slug ?? null)})` - ); - } - return { ...builtin, defaultFor: defaultFor ?? builtin.defaultFor }; - } - - if (typeof raw.id !== "string") { - throw new ModelConfigError(`${ctx}: "id" must be a string`); - } - const selection: ModelSelection = { id: raw.id }; - if (Array.isArray(raw.params)) { - selection.params = raw.params as ModelSelection["params"]; - } - return { - slug: slug ?? raw.id, - selection, - summary: - typeof raw.summary === "string" - ? raw.summary - : `Configured for this repo via ${MODEL_ENV_CATALOG}.`, - strengths: Array.isArray(raw.strengths) ? (raw.strengths as string[]) : [], - speed: - typeof raw.speed === "string" - ? (raw.speed as ModelProfile["speed"]) - : "medium", - use: - typeof raw.use === "string" - ? raw.use - : "Prefer this unless the task needs a listed specialist.", - defaultFor, - }; -} - -function readEnvCatalog(): ModelProfile[] | undefined { - const raw = process.env[MODEL_ENV_CATALOG]?.trim(); - if (!raw) return undefined; - - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (err) { - throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: ${err instanceof Error ? err.message : String(err)}` - ); - } - if (!Array.isArray(parsed)) { - throw new ModelConfigError( - `${MODEL_ENV_CATALOG}: expected a JSON array of model entries` - ); - } - return parsed.map((item, i) => { - const ctx = `${MODEL_ENV_CATALOG}[${i}]`; - if (!isPlainObject(item)) { - throw new ModelConfigError(`${ctx}: expected a JSON object`); - } - return toModelProfile(item, ctx); - }); +function envCatalogJson(): string | undefined { + return process.env[MODEL_ENV_CATALOG]?.trim() || undefined; } /** * The catalog planners choose from. ORCHESTRATE_MODEL_CATALOG replaces * MODEL_CATALOG outright when set; there is no merging, so the configured - * list is the complete menu. Built per call so env changes apply on the spot. + * list is the complete menu. Read per call so env changes apply on the spot. */ export function effectiveModelCatalog(): ModelProfile[] { - return readEnvCatalog() ?? MODEL_CATALOG; + const raw = envCatalogJson(); + return raw ? parseModelCatalogJson(raw, MODEL_ENV_CATALOG) : MODEL_CATALOG; } /** Model slug for a task type when `tasks[].model` is omitted. */ export function defaultModelForType(type: TaskType): string { - const catalog = readEnvCatalog(); - const match = (catalog ?? MODEL_CATALOG).find(m => - m.defaultFor?.includes(type) - ); + const match = effectiveModelCatalog().find(m => m.defaultFor?.includes(type)); if (match) return match.slug; - throw new ModelConfigError( - catalog + throw new PlanValidationError( + envCatalogJson() ? `${MODEL_ENV_CATALOG} has no ${type} default. Add "defaultFor": ["${type}"] to one entry.` : `MODEL_CATALOG missing default for TaskType "${type}"` ); @@ -302,15 +209,14 @@ export function assertModelEnvConfig(): void { } export function renderModelCatalog(): string { - const envCatalog = readEnvCatalog(); const lines: string[] = []; - if (envCatalog) { + if (envCatalogJson()) { lines.push( "This repo publishes an exact model menu. Use only the slugs listed below; do not reach for models outside this list." ); lines.push(""); } - for (const m of envCatalog ?? MODEL_CATALOG) { + for (const m of effectiveModelCatalog()) { const defaults = m.defaultFor?.length ? ` (default for ${m.defaultFor.join(", ")})` : ""; diff --git a/orchestrate/skills/orchestrate/scripts/schemas.ts b/orchestrate/skills/orchestrate/scripts/schemas.ts index dc77a59d..ebf020b2 100644 --- a/orchestrate/skills/orchestrate/scripts/schemas.ts +++ b/orchestrate/skills/orchestrate/scripts/schemas.ts @@ -500,6 +500,38 @@ export const StopResultSchema = z.discriminatedUnion("action", [ NoopStopResultSchema, ]); +const ModelSelectionSchema = z + .object({ + id: z.string().min(1).describe("Model id as accepted by the Cursor API."), + params: z + .array(z.object({ id: z.string(), value: z.string() })) + .optional() + .describe("Model parameters, e.g. reasoning, effort, thinking, fast."), + }) + .describe("Canonical SDK selection passed to `Agent.create({ model })`."); + +const ModelProfileSchema = z.object({ + slug: z + .string() + .min(1) + .describe("Authoring name planners write into `tasks[].model`."), + selection: ModelSelectionSchema, + summary: z.string().describe("One-line description planners read."), + strengths: z + .array(z.string()) + .describe("Capability keywords planners match a task against."), + speed: z.string().describe("Relative latency, e.g. fast, medium, slow."), + use: z.string().describe("When a planner should pick this model."), + defaultFor: z + .array(TaskTypeSchema) + .optional() + .describe( + "Task types that use this model when `tasks[].model` is omitted." + ), +}); + +export const ModelCatalogSchema = z.array(ModelProfileSchema); + const TreeTaskSchema = z .object({ name: taskNameSchema, @@ -534,6 +566,7 @@ export type SpawnResult = z.infer; export type RecoverResult = z.infer; export type StopResult = z.infer; export type TreeTask = z.infer; +export type ModelProfile = z.infer; export interface TreeState { rootSlug: string | null; tasks: TreeTask[]; @@ -577,6 +610,19 @@ export function parsePlanTaskValue(value: unknown, source: string): PlanTask { }); } +export function parseModelCatalogJson( + text: string, + source: string +): ModelProfile[] { + return parseJsonWithSchema({ + schema: ModelCatalogSchema, + text, + source, + recoveryHint: + "Every entry needs slug, selection, summary, strengths, speed, and use. Unset the variable and run `bun cli.ts models --json` to copy the built-in catalog as a starting point.", + }); +} + export function parseStateJson(text: string, source: string): State { return parseJsonWithSchema({ schema: StateSchema, diff --git a/orchestrate/skills/orchestrate/scripts/tools/generate-json-schemas.ts b/orchestrate/skills/orchestrate/scripts/tools/generate-json-schemas.ts index 35c5fd84..cc728a7c 100644 --- a/orchestrate/skills/orchestrate/scripts/tools/generate-json-schemas.ts +++ b/orchestrate/skills/orchestrate/scripts/tools/generate-json-schemas.ts @@ -6,7 +6,7 @@ import { fileURLToPath } from "node:url"; import type { z } from "zod/v3"; import { zodToJsonSchema } from "zod-to-json-schema"; -import { PlanSchema, StateSchema } from "../schemas.ts"; +import { ModelCatalogSchema, PlanSchema, StateSchema } from "../schemas.ts"; const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)); const SCHEMA_DIR = resolve(SCRIPT_DIR, "../../schemas"); @@ -29,6 +29,15 @@ writeSchema({ "Written by scripts/orchestrate.ts. Live task rows; read-only unless you must edit by hand to unstick state.", }); +writeSchema({ + path: "model-catalog.schema.json", + schema: ModelCatalogSchema, + id: "https://cursor/orchestrate/model-catalog.schema.json", + title: "orchestrate ORCHESTRATE_MODEL_CATALOG", + description: + "Optional operator-authored model catalog. When ORCHESTRATE_MODEL_CATALOG is set, it replaces the built-in catalog planners choose `tasks[].model` from.", +}); + function writeSchema(args: { path: string; schema: z.ZodTypeAny; From bb8a368575ae5696240554ffb8aeeaebdc0cd720 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 27 Jul 2026 19:37:21 +0000 Subject: [PATCH 6/6] refactor(orchestrate): prefix Slack env vars with ORCHESTRATE_ SLACK_BOT_TOKEN and SLACK_CHANNEL_ID are generic enough that orchestrate could pick up an unrelated tool's Slack app from a shared environment. They are now ORCHESTRATE_SLACK_BOT_TOKEN and ORCHESTRATE_SLACK_CHANNEL_ID, matching the ORCHESTRATE_ prefix the skill's other settings already use. Both env names now live in constants rather than scattered string literals. The old names are not read as a fallback, since honoring them would defeat the isolation; instead, a token found only under the old name prints a rename notice, so the change can't present as a silent Slack outage mid-run. Co-authored-by: Jonny Alexander Power --- orchestrate/README.md | 6 +- orchestrate/skills/orchestrate/SKILL.md | 2 +- .../orchestrate/references/dispatcher.md | 4 +- .../skills/orchestrate/references/planner.md | 4 +- .../skills/orchestrate/references/spawning.md | 2 +- .../orchestrate/schemas/plan.schema.json | 2 +- .../agent-manager-slack-mirror.test.ts | 20 +++--- .../scripts/__tests__/comment-cli.test.ts | 8 +-- .../scripts/__tests__/slack-adapter.test.ts | 8 +-- .../__tests__/slack-channel-boundary.test.ts | 32 +++++---- .../__tests__/slack-env-rename.test.ts | 71 +++++++++++++++++++ .../scripts/adapters/slack/client.ts | 17 ++++- .../orchestrate/scripts/cli/comments.ts | 3 +- .../skills/orchestrate/scripts/cli/task.ts | 4 +- .../skills/orchestrate/scripts/cli/util.ts | 16 +++-- .../orchestrate/scripts/core/agent-manager.ts | 9 ++- .../scripts/core/comment-retry-queue.ts | 3 +- .../skills/orchestrate/scripts/schemas.ts | 2 +- 18 files changed, 157 insertions(+), 56 deletions(-) create mode 100644 orchestrate/skills/orchestrate/scripts/__tests__/slack-env-rename.test.ts diff --git a/orchestrate/README.md b/orchestrate/README.md index b58eb2e5..cb5db523 100644 --- a/orchestrate/README.md +++ b/orchestrate/README.md @@ -77,6 +77,8 @@ Team service-account keys (Team Settings → Service accounts) also work for bot Slack visibility is opt-in. When the token is unset, the script logs once and runs without Slack; correctness does not change. To enable it: +> **Renamed:** these were `SLACK_BOT_TOKEN` and `SLACK_CHANNEL_ID`. They are now `ORCHESTRATE_SLACK_BOT_TOKEN` and `ORCHESTRATE_SLACK_CHANNEL_ID` so orchestrate can't pick up another tool's Slack app from a shared environment. The unprefixed names are no longer read; if only the old token is set, the CLI says so instead of going quiet. + 1. Create a Slack app at [https://api.slack.com/apps](https://api.slack.com/apps) → **From scratch**. Pick a name and a workspace. 2. Under **OAuth & Permissions** → **Bot Token Scopes**, add: @@ -97,9 +99,9 @@ Slack visibility is opt-in. When the token is unset, the script logs once and ru | `users:read.email` | Resolve the dispatcher's first name from `git config user.email`. Without it, pass `--dispatcher-name` explicitly. | 3. **Install to Workspace** and copy the **Bot User OAuth Token** (`xoxb-...`). -4. Export it: `export SLACK_BOT_TOKEN="xoxb-..."`. +4. Export it: `export ORCHESTRATE_SLACK_BOT_TOKEN="xoxb-..."`. 5. Invite the bot to the channel where you want runs to thread (`/invite @your-bot`). Public channels with `chat:write.public` skip this; private channels require the invite. -6. Grab the channel ID. In Slack: right-click the channel → **View channel details** → bottom of the dialog. Pass it via `--slack-channel ` on `kickoff` (or set `SLACK_CHANNEL_ID`). The first kickoff persists the id on the plan; subplanners and later `run` invocations inherit it. +6. Grab the channel ID. In Slack: right-click the channel → **View channel details** → bottom of the dialog. Pass it via `--slack-channel ` on `kickoff` (or set `ORCHESTRATE_SLACK_CHANNEL_ID`). The first kickoff persists the id on the plan; subplanners and later `run` invocations inherit it. ## Install diff --git a/orchestrate/skills/orchestrate/SKILL.md b/orchestrate/skills/orchestrate/SKILL.md index 15a532e5..13380163 100644 --- a/orchestrate/skills/orchestrate/SKILL.md +++ b/orchestrate/skills/orchestrate/SKILL.md @@ -13,7 +13,7 @@ An explicit `/orchestrate ` fans out a large task across parallel Cursor c ## Setup - `CURSOR_API_KEY` must be a personal/user key. Create it from [Cursor Dashboard > Integrations](https://cursor.com/dashboard/integrations), then read `cursor-sdk` Auth before using it. -- `SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. +- `ORCHESTRATE_SLACK_BOT_TOKEN` is optional. When set, pass `--slack-channel ` to `kickoff` or the first `run --root`, or set `ORCHESTRATE_SLACK_CHANNEL_ID`. The script stores the channel in `plan.slackChannel`, posts the kickoff thread there, mirrors task status, and reads Andon reactions. When the token is unset, the script logs once and runs without Slack visibility; correctness does not change. - `ORCHESTRATE_MODEL_CATALOG` is optional. When set, its JSON array replaces the built-in model catalog outright: it becomes the list planners pick `tasks[].model` from, and its `defaultFor` entries supply each task type's default. It is validated against `schemas/model-catalog.schema.json`; `bun cli.ts models` prints whichever catalog is in effect and `--json` emits it in that shape. See the plugin README. ## Core principles diff --git a/orchestrate/skills/orchestrate/references/dispatcher.md b/orchestrate/skills/orchestrate/references/dispatcher.md index 270330b1..342a14fe 100644 --- a/orchestrate/skills/orchestrate/references/dispatcher.md +++ b/orchestrate/skills/orchestrate/references/dispatcher.md @@ -18,7 +18,7 @@ bun cli.ts kickoff "" [--repo ] [--ref main] [--model claude-opus-4-8 A repo can replace the model catalog planners choose from, including each task type's default, with `ORCHESTRATE_MODEL_CATALOG` (see the plugin README). That does not cover the root planner: pass `--model` to set it, otherwise it stays `claude-opus-4-8`. Run `bun cli.ts models` to print the catalog in effect; config the CLI can't read exits 2 with the offending entry named. -The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. +The CLI reads `CURSOR_API_KEY`, auto-detects the repo from `git config --get remote.origin.url`, builds the spawn prompt, spawns via `cursor-sdk`, and prints `{ agentId, runId, status, url, dispatcherFirstName }` JSON. Slack is optional. If `ORCHESTRATE_SLACK_BOT_TOKEN` is set, also pass `--slack-channel ` or set `ORCHESTRATE_SLACK_CHANNEL_ID`; otherwise kickoff fails before spawning. If the token is unset, Slack stays disabled. ## Dispatcher identity @@ -47,6 +47,6 @@ Progress is observable after dispatch: - `bun cli.ts crawl ` for a deep tree view. - `bun cli.ts status` for top-level state. -- The Slack kickoff thread in `plan.slackChannel`, when `SLACK_BOT_TOKEN` is set. +- The Slack kickoff thread in `plan.slackChannel`, when `ORCHESTRATE_SLACK_BOT_TOKEN` is set. `syncStateToGit` defaults to true. Set `syncStateToGit: false` on the root plan when goals or handoffs should not be committed. diff --git a/orchestrate/skills/orchestrate/references/planner.md b/orchestrate/skills/orchestrate/references/planner.md index d8ea8fe6..0c85a89d 100644 --- a/orchestrate/skills/orchestrate/references/planner.md +++ b/orchestrate/skills/orchestrate/references/planner.md @@ -14,7 +14,7 @@ Scripts expect `bun` on PATH. Install dependencies with `bun install` inside thi Regenerate `schemas/*.json` from `scripts/schemas.ts` with `bun run generate-schemas` in `scripts/` after plan or state shape changes. -Slack visibility uses `SLACK_BOT_TOKEN`. Required scopes: +Slack visibility uses `ORCHESTRATE_SLACK_BOT_TOKEN`. Required scopes: - `chat:write` — post and edit messages. - `chat:write.customize` — set custom username and icon on bot messages. @@ -69,7 +69,7 @@ Write `plan.json` at ``; the default workspace is `.orchestrate/ { if (ORIGINAL_API_KEY === undefined) delete process.env.CURSOR_API_KEY; else process.env.CURSOR_API_KEY = ORIGINAL_API_KEY; if (ORIGINAL_SLACK_TOKEN === undefined) { - delete process.env.SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; } else { - process.env.SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; + process.env.ORCHESTRATE_SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; } }); @@ -391,8 +391,8 @@ describe("AgentManager Slack status mirror", () => { }); test("No token: load works, slackAdapter undefined, attention log + console.error once", async () => { - const original = process.env.SLACK_BOT_TOKEN; - delete process.env.SLACK_BOT_TOKEN; + const original = process.env.ORCHESTRATE_SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; const errors: string[] = []; const originalConsoleError = console.error; console.error = ((...args: unknown[]) => { @@ -428,7 +428,9 @@ describe("AgentManager Slack status mirror", () => { expect(mgr.slackAdapter).toBeUndefined(); expect(slackWebApiCalls()).toHaveLength(0); expect( - errors.filter(line => line.includes("SLACK_BOT_TOKEN not set")) + errors.filter(line => + line.includes("ORCHESTRATE_SLACK_BOT_TOKEN not set") + ) ).toHaveLength(1); expect(readState(workspace).attention).toEqual([]); @@ -443,9 +445,9 @@ describe("AgentManager Slack status mirror", () => { rmSync(workspace, { recursive: true, force: true }); console.error = originalConsoleError; if (original === undefined) { - delete process.env.SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; } else { - process.env.SLACK_BOT_TOKEN = original; + process.env.ORCHESTRATE_SLACK_BOT_TOKEN = original; } } }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/comment-cli.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/comment-cli.test.ts index e3c6954d..eb285b7a 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/comment-cli.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/comment-cli.test.ts @@ -18,7 +18,7 @@ describe("comment CLI", () => { encoding: "utf8", env: { ...process.env, - SLACK_BOT_TOKEN: "xoxb-test", + ORCHESTRATE_SLACK_BOT_TOKEN: "xoxb-test", }, }); @@ -73,7 +73,7 @@ describe("comment CLI", () => { encoding: "utf8", env: { ...process.env, - SLACK_BOT_TOKEN: "xoxb-test", + ORCHESTRATE_SLACK_BOT_TOKEN: "xoxb-test", }, } ); @@ -130,7 +130,7 @@ describe("comment CLI", () => { encoding: "utf8", env: { ...process.env, - SLACK_BOT_TOKEN: "xoxb-test", + ORCHESTRATE_SLACK_BOT_TOKEN: "xoxb-test", }, } ); @@ -187,7 +187,7 @@ describe("comment CLI", () => { encoding: "utf8", env: { ...process.env, - SLACK_BOT_TOKEN: "xoxb-test", + ORCHESTRATE_SLACK_BOT_TOKEN: "xoxb-test", }, } ); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/slack-adapter.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/slack-adapter.test.ts index bb2a5e28..1a8b5ad2 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/slack-adapter.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/slack-adapter.test.ts @@ -12,14 +12,14 @@ installSlackWebApiMock(); const { createSlackAdapter } = await import("../adapters/index.ts"); const TEST_SLACK_CHANNEL = "C123TEST"; -const ORIGINAL_SLACK_TOKEN = process.env.SLACK_BOT_TOKEN; -process.env.SLACK_BOT_TOKEN = "xoxb-test"; +const ORIGINAL_SLACK_TOKEN = process.env.ORCHESTRATE_SLACK_BOT_TOKEN; +process.env.ORCHESTRATE_SLACK_BOT_TOKEN = "xoxb-test"; afterAll(() => { if (ORIGINAL_SLACK_TOKEN === undefined) { - delete process.env.SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; } else { - process.env.SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; + process.env.ORCHESTRATE_SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; } }); diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/slack-channel-boundary.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/slack-channel-boundary.test.ts index 8c3303ce..59956f3f 100644 --- a/orchestrate/skills/orchestrate/scripts/__tests__/slack-channel-boundary.test.ts +++ b/orchestrate/skills/orchestrate/scripts/__tests__/slack-channel-boundary.test.ts @@ -8,43 +8,45 @@ import { resolveWorkspaceSlackChannelOrBail, } from "../cli/util.ts"; -const ORIGINAL_SLACK_TOKEN = process.env.SLACK_BOT_TOKEN; -const ORIGINAL_SLACK_CHANNEL = process.env.SLACK_CHANNEL_ID; +const ORIGINAL_SLACK_TOKEN = process.env.ORCHESTRATE_SLACK_BOT_TOKEN; +const ORIGINAL_SLACK_CHANNEL = process.env.ORCHESTRATE_SLACK_CHANNEL_ID; afterEach(() => { - if (ORIGINAL_SLACK_TOKEN === undefined) delete process.env.SLACK_BOT_TOKEN; - else process.env.SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; - if (ORIGINAL_SLACK_CHANNEL === undefined) delete process.env.SLACK_CHANNEL_ID; - else process.env.SLACK_CHANNEL_ID = ORIGINAL_SLACK_CHANNEL; + if (ORIGINAL_SLACK_TOKEN === undefined) + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; + else process.env.ORCHESTRATE_SLACK_BOT_TOKEN = ORIGINAL_SLACK_TOKEN; + if (ORIGINAL_SLACK_CHANNEL === undefined) + delete process.env.ORCHESTRATE_SLACK_CHANNEL_ID; + else process.env.ORCHESTRATE_SLACK_CHANNEL_ID = ORIGINAL_SLACK_CHANNEL; }); describe("Slack channel boundary", () => { test("Token set without a channel fails before Slack initialization", () => { - process.env.SLACK_BOT_TOKEN = "xoxb-test"; - delete process.env.SLACK_CHANNEL_ID; + process.env.ORCHESTRATE_SLACK_BOT_TOKEN = "xoxb-test"; + delete process.env.ORCHESTRATE_SLACK_CHANNEL_ID; expect(() => resolveKickoffSlackChannelOrBail(undefined)).toThrow( - "set --slack-channel or SLACK_CHANNEL_ID, or unset SLACK_BOT_TOKEN to disable Slack" + "set --slack-channel or ORCHESTRATE_SLACK_CHANNEL_ID, or unset ORCHESTRATE_SLACK_BOT_TOKEN to disable Slack" ); }); test("No token and no channel leaves Slack disabled", () => { - delete process.env.SLACK_BOT_TOKEN; - delete process.env.SLACK_CHANNEL_ID; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_CHANNEL_ID; expect(resolveKickoffSlackChannelOrBail(undefined)).toBeUndefined(); }); test("No token with a channel accepts the channel for later plan persistence", () => { - delete process.env.SLACK_BOT_TOKEN; - delete process.env.SLACK_CHANNEL_ID; + delete process.env.ORCHESTRATE_SLACK_BOT_TOKEN; + delete process.env.ORCHESTRATE_SLACK_CHANNEL_ID; expect(resolveKickoffSlackChannelOrBail("C123TEST")).toBe("C123TEST"); }); test("Workspace run inherits plan.slackChannel", () => { - process.env.SLACK_BOT_TOKEN = "xoxb-test"; - delete process.env.SLACK_CHANNEL_ID; + process.env.ORCHESTRATE_SLACK_BOT_TOKEN = "xoxb-test"; + delete process.env.ORCHESTRATE_SLACK_CHANNEL_ID; const workspace = mkdtempSync(join(tmpdir(), "orch-slack-channel-")); try { writeFileSync( diff --git a/orchestrate/skills/orchestrate/scripts/__tests__/slack-env-rename.test.ts b/orchestrate/skills/orchestrate/scripts/__tests__/slack-env-rename.test.ts new file mode 100644 index 00000000..7c3ffb08 --- /dev/null +++ b/orchestrate/skills/orchestrate/scripts/__tests__/slack-env-rename.test.ts @@ -0,0 +1,71 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; + +import { + createSlackWebClient, + SLACK_TOKEN_ENV, + slackTokenConfigured, +} from "../adapters/slack/client.ts"; + +const LEGACY_TOKEN_ENV = "SLACK_BOT_TOKEN"; +const KEYS = [SLACK_TOKEN_ENV, LEGACY_TOKEN_ENV] as const; + +const saved: Record = {}; +let errors: string[] = []; +let originalConsoleError: typeof console.error; + +beforeEach(() => { + for (const key of KEYS) { + saved[key] = process.env[key]; + delete process.env[key]; + } + errors = []; + originalConsoleError = console.error; + console.error = (...args: unknown[]) => { + errors.push(args.map(String).join(" ")); + }; +}); + +afterEach(() => { + console.error = originalConsoleError; + for (const key of KEYS) { + const prior = saved[key]; + if (prior === undefined) delete process.env[key]; + else process.env[key] = prior; + } +}); + +describe("Slack token env var", () => { + test("the prefixed variable enables Slack", () => { + process.env[SLACK_TOKEN_ENV] = "xoxb-test"; + expect(slackTokenConfigured()).toBe(true); + expect(createSlackWebClient()).toBeDefined(); + expect(errors).toEqual([]); + }); + + test("neither variable set disables Slack with one line", () => { + expect(slackTokenConfigured()).toBe(false); + expect(createSlackWebClient()).toBeUndefined(); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain(`${SLACK_TOKEN_ENV} not set`); + }); + + // The rename would otherwise look like a Slack outage: the token is present, + // but under the old name, so the run goes quiet with no explanation. + test("the unprefixed variable is ignored and named in the notice", () => { + process.env[LEGACY_TOKEN_ENV] = "xoxb-test"; + expect(slackTokenConfigured()).toBe(false); + expect(createSlackWebClient()).toBeUndefined(); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain( + `${LEGACY_TOKEN_ENV} is no longer read; rename it to ${SLACK_TOKEN_ENV}` + ); + }); + + test("the prefixed variable wins when both are set", () => { + process.env[LEGACY_TOKEN_ENV] = "xoxb-legacy"; + process.env[SLACK_TOKEN_ENV] = "xoxb-test"; + expect(slackTokenConfigured()).toBe(true); + expect(createSlackWebClient()).toBeDefined(); + expect(errors).toEqual([]); + }); +}); diff --git a/orchestrate/skills/orchestrate/scripts/adapters/slack/client.ts b/orchestrate/skills/orchestrate/scripts/adapters/slack/client.ts index 918ac531..7e169e8d 100644 --- a/orchestrate/skills/orchestrate/scripts/adapters/slack/client.ts +++ b/orchestrate/skills/orchestrate/scripts/adapters/slack/client.ts @@ -1,10 +1,23 @@ import { WebClient } from "@slack/web-api"; +/** Bot token env var. Prefixed so it can't collide with another tool's Slack app. */ +export const SLACK_TOKEN_ENV = "ORCHESTRATE_SLACK_BOT_TOKEN"; + +const LEGACY_SLACK_TOKEN_ENV = "SLACK_BOT_TOKEN"; + +export function slackTokenConfigured(): boolean { + return Boolean(process.env[SLACK_TOKEN_ENV]?.trim()); +} + export function createSlackWebClient(): WebClient | undefined { - const token = process.env.SLACK_BOT_TOKEN; + const token = process.env[SLACK_TOKEN_ENV]; if (!token) { + // Losing Slack silently after the rename would look like an outage, so + // name the old variable when it's the only one set. console.error( - "[orchestrate] SLACK_BOT_TOKEN not set; Slack visibility disabled" + process.env[LEGACY_SLACK_TOKEN_ENV] + ? `[orchestrate] ${LEGACY_SLACK_TOKEN_ENV} is no longer read; rename it to ${SLACK_TOKEN_ENV}. Slack visibility disabled` + : `[orchestrate] ${SLACK_TOKEN_ENV} not set; Slack visibility disabled` ); return undefined; } diff --git a/orchestrate/skills/orchestrate/scripts/cli/comments.ts b/orchestrate/skills/orchestrate/scripts/cli/comments.ts index da8ac9ef..ed03c2ae 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/comments.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/comments.ts @@ -3,6 +3,7 @@ import { basename, isAbsolute, join, relative, resolve } from "node:path"; import type { Command } from "commander"; import { createSlackAdapter } from "../adapters/index.ts"; +import { SLACK_TOKEN_ENV } from "../adapters/slack/client.ts"; import { appendAgentFooter } from "../core/agent-manager.ts"; import { postOrQueueComment } from "../core/comment-retry-queue.ts"; import { redactBody } from "../core/redact-body.ts"; @@ -147,7 +148,7 @@ async function uploadFileToThread(args: { } const slack = createSlackAdapter(args.channel); if (!slack) { - throw new Error("SLACK_BOT_TOKEN not set; cannot upload Slack file"); + throw new Error(`${SLACK_TOKEN_ENV} not set; cannot upload Slack file`); } const content = readFileSync(args.filePath); const initial = diff --git a/orchestrate/skills/orchestrate/scripts/cli/task.ts b/orchestrate/skills/orchestrate/scripts/cli/task.ts index 3093c913..655160f9 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/task.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/task.ts @@ -59,7 +59,7 @@ export function registerTaskCommands(program: Command): void { ) .option( "--slack-channel ", - "Slack channel id for the root run. Falls back to SLACK_CHANNEL_ID." + "Slack channel id for the root run. Falls back to ORCHESTRATE_SLACK_CHANNEL_ID." ) .option( "--max-runtime-sec ", @@ -115,7 +115,7 @@ export function registerTaskCommands(program: Command): void { ) .option( "--slack-channel ", - "Slack channel id for run visibility. Falls back to SLACK_CHANNEL_ID." + "Slack channel id for run visibility. Falls back to ORCHESTRATE_SLACK_CHANNEL_ID." ) .option( "--dispatcher-name ", diff --git a/orchestrate/skills/orchestrate/scripts/cli/util.ts b/orchestrate/skills/orchestrate/scripts/cli/util.ts index bffd2407..d9d7c2f8 100644 --- a/orchestrate/skills/orchestrate/scripts/cli/util.ts +++ b/orchestrate/skills/orchestrate/scripts/cli/util.ts @@ -4,6 +4,10 @@ import { userInfo } from "node:os"; import { join, resolve } from "node:path"; import { createSlackAdapter } from "../adapters/index.ts"; +import { + SLACK_TOKEN_ENV, + slackTokenConfigured, +} from "../adapters/slack/client.ts"; import type { CommentCriticality, SlackAdapter } from "../adapters/types.ts"; import { AgentManager, type RespawnSource } from "../core/agent-manager.ts"; import type { CommentDestinations } from "../core/comment-retry-queue.ts"; @@ -37,8 +41,10 @@ export interface SpawnOptions { wait?: boolean; } -const SLACK_CHANNEL_REQUIRED_MESSAGE = - "set --slack-channel or SLACK_CHANNEL_ID, or unset SLACK_BOT_TOKEN to disable Slack"; +/** Channel env var. Prefixed so it can't collide with another tool's Slack app. */ +export const SLACK_CHANNEL_ENV = "ORCHESTRATE_SLACK_CHANNEL_ID"; + +const SLACK_CHANNEL_REQUIRED_MESSAGE = `set --slack-channel or ${SLACK_CHANNEL_ENV}, or unset ${SLACK_TOKEN_ENV} to disable Slack`; export interface TreeVictim { taskName: string; @@ -439,7 +445,7 @@ export function resolveSlackChannelOption( ): string | undefined { const flag = explicit?.trim(); if (flag) return flag; - const env = process.env.SLACK_CHANNEL_ID?.trim(); + const env = process.env[SLACK_CHANNEL_ENV]?.trim(); return env || undefined; } @@ -469,7 +475,7 @@ export function resolveWorkspaceSlackChannelOrBail(args: { } function requireSlackChannelIfTokenSet(channel: string | undefined): void { - if (process.env.SLACK_BOT_TOKEN && !channel) { + if (slackTokenConfigured() && !channel) { throw new PlanValidationError(SLACK_CHANNEL_REQUIRED_MESSAGE); } } @@ -565,7 +571,7 @@ export function loadAndonTargetOrBail(opts: { workspace?: string }): { const slack = createSlackAdapter(plan.slackKickoffRef.channel); if (!slack) { throw new PlanValidationError( - "set SLACK_BOT_TOKEN before running andon commands" + `set ${SLACK_TOKEN_ENV} before running andon commands` ); } return { slack, ref: plan.slackKickoffRef }; diff --git a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts index 32ce7199..f19c578c 100644 --- a/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts +++ b/orchestrate/skills/orchestrate/scripts/core/agent-manager.ts @@ -18,7 +18,10 @@ import type { SDKMessage, } from "@cursor/sdk"; import { createSlackAdapter } from "../adapters/index.ts"; -import { createSlackWebClient } from "../adapters/slack/client.ts"; +import { + createSlackWebClient, + slackTokenConfigured, +} from "../adapters/slack/client.ts"; import type { SlackAdapter, TaskStatus } from "../adapters/types.ts"; import { PlanValidationError } from "../errors.ts"; import type { @@ -1648,7 +1651,7 @@ function planSlackChannel(plan: Plan): string | undefined { } function slackAdapterForPlan(plan: Plan): SlackAdapter | undefined { - // SLACK_BOT_TOKEN missing is signalled once via console.error inside + // A missing token is signalled once via console.error inside // createSlackWebClient. Don't double-log to attention.log: env config // belongs to the operator surface, not workspace-visible state. const channel = planSlackChannel(plan); @@ -1656,7 +1659,7 @@ function slackAdapterForPlan(plan: Plan): SlackAdapter | undefined { // Surface the missing-token signal even when the channel is also unset, // so an operator who expected Slack visibility sees a single console line // explaining why it's off. - if (!process.env.SLACK_BOT_TOKEN) createSlackWebClient(); + if (!slackTokenConfigured()) createSlackWebClient(); return undefined; } return createSlackAdapter(channel); diff --git a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts index f9a3b3bf..7aa6692f 100644 --- a/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts +++ b/orchestrate/skills/orchestrate/scripts/core/comment-retry-queue.ts @@ -8,6 +8,7 @@ import { } from "node:fs"; import { dirname, join, resolve } from "node:path"; +import { SLACK_TOKEN_ENV } from "../adapters/slack/client.ts"; import type { CommentCriticality, SlackAdapter } from "../adapters/types.ts"; const COMMENT_RETRY_BACKOFF_MS = [1_000, 5_000, 30_000, 300_000, 1_800_000]; @@ -266,7 +267,7 @@ async function postComment(args: { } if (!args.destinations.slack) { throw new Error( - "Slack destination requested but SLACK_BOT_TOKEN is not set" + `Slack destination requested but ${SLACK_TOKEN_ENV} is not set` ); } const target = parseSlackDestination(args.destination); diff --git a/orchestrate/skills/orchestrate/scripts/schemas.ts b/orchestrate/skills/orchestrate/scripts/schemas.ts index ebf020b2..fa125c30 100644 --- a/orchestrate/skills/orchestrate/scripts/schemas.ts +++ b/orchestrate/skills/orchestrate/scripts/schemas.ts @@ -242,7 +242,7 @@ const PlanObjectSchema = z slackChannel: nonEmptyStringSchema .optional() .describe( - "Slack channel id for run visibility. Set from --slack-channel or SLACK_CHANNEL_ID by kickoff or the first root run." + "Slack channel id for run visibility. Set from --slack-channel or ORCHESTRATE_SLACK_CHANNEL_ID by kickoff or the first root run." ), slackKickoffRef: z .object({