diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe5f6f..5579887 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- Configure the model each agent runs with: `jury agents model ` saves a default in + `~/.jury/config.json`, a `model` field per agent in `jury.config.json` sets one per repository, + and `--model =` sets one for a run, each overriding the one before. The model is + passed as a flag or environment variable only when Jury runs the agent, on first runs, resumed + rounds, replies and judging alike; the agent's own configuration is never changed. Supported for + codex, claude, grok, opencode, qwen, copilot and kimi; a configured model for any other agent + stops the run with an error. Runs record the models used, and `jury agents` shows them (#99). - Add `jury agents jury` to save default reviewers globally in `~/.jury/config.json`, beside the global judge: `jury agents jury claude,droid`, `--reset`, and a checkbox picker when run with no arguments in a terminal. Runs without `--jury` use them; repository `reviewer` roles and diff --git a/README.md b/README.md index a16de56..0278e9f 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,15 @@ off automatic role assignment, as `--jury` does. The judge is left out of its ow reviewer is later uninstalled or disabled in a repository, the run stops with an error naming the saved setting rather than reviewing with fewer agents. +Pick the model an agent runs with, without touching its own configuration: + +```bash +jury agents model claude opus # saved default +jury review --model codex=gpt-5.5 # this run only +``` + +See [models](docs/configuration.md#models) for each agent's flag and the precedence rules. + ### Built-in agents | name | product | install | default | diff --git a/bin/jury.js b/bin/jury.js index 316ddb4..aebdeb5 100755 --- a/bin/jury.js +++ b/bin/jury.js @@ -13,8 +13,8 @@ import { readFileSync } from "node:fs"; import { promisify } from "node:util"; import path from "node:path"; import { automaticRoles, automaticReviewers } from "../lib/roles.js"; -import { loadConfig, reviewers, judgeAgent, knownAgents, readGlobalConfig, saveGlobalJudge, saveGlobalReviewers, defaultReviewers, savedReviewersNote, globalConfigPath } from "../lib/config.js"; -import { runAgent, probe } from "../lib/agents.js"; +import { loadConfig, reviewers, judgeAgent, knownAgents, readGlobalConfig, saveGlobalJudge, saveGlobalReviewers, defaultReviewers, savedReviewersNote, globalConfigPath, applyModelFlags, assertModelsSupported, modelsUsed, saveGlobalModel, modelProblem } from "../lib/config.js"; +import { runAgent, probe, supportsModel } from "../lib/agents.js"; import { threadFor, buildReply, replyArgv } from "../lib/reply.js"; import { buildPrompt } from "../lib/prompt.js"; import { serve } from "../lib/server.js"; @@ -64,6 +64,7 @@ Common flags --reviewer only these reviewers, repeatable or comma-separated (default: configured or saved reviewers) --jury same as --reviewer --judge codex one agent that triages and fixes (default: configured, auto for 1–2 installed CLIs, then codex) + --model = this run's model for an agent, repeatable (default: jury.config.json, then jury agents model) --push commit and push fixes (default: true) --web open the browser console (default: true) @@ -87,6 +88,7 @@ const USAGE_FULL = `jury — review a pull request with multiple AI reviewers un jury agents check which configured agents are installed jury agents judge set the global default judge jury agents jury set the default reviewers (picker in a terminal) + jury agents model set the model jury starts an agent with jury version Related PRs: jury review @@ -106,6 +108,7 @@ review (triages, fixes, commits, and pushes automatica --reviewer only these reviewers, repeatable or comma-separated (default: configured or saved reviewers) --jury same as --reviewer --judge one agent that triages and fixes (default: configured, auto for 1–2 installed CLIs, then codex) + --model = this run's model for an agent, repeatable (default: jury.config.json, then jury agents model) --resume continue an existing run instead of starting a new one --web open the console; stays up after review (default: true) --web-only view saved reviews without running agents @@ -470,6 +473,7 @@ async function cmdAgent(argv) { rounds: { type: "string", default: "10" }, ...reviewerOptions, judge: { type: "string" }, + model: { type: "string", multiple: true }, push: { type: "boolean", default: true }, resume: { type: "string" }, "dry-run": { type: "boolean", default: false }, @@ -519,6 +523,7 @@ async function cmdAgent(argv) { // A local ignored config belongs to the requested checkout. An automatic // clone intentionally starts from the repository's committed/default config. const cfg = await loadConfig(group ? group.targets[0].worktree : resolved ? worktree : requestedWorktree); + applyModelFlags(cfg, values.model); const git = await describe(group ? group.targets[0].worktree : worktree); // Both asked for rather than assumed: the trunk from the remote's own HEAD, @@ -605,6 +610,8 @@ async function cmdAgent(argv) { const pool = roles ? automaticReviewers(cfg, roles) : selectReviewers(cfg, requestedReviewers(values), judge.name); if (!pool.length) throw new Error("no reviewers configured after excluding the judge"); + assertModelsSupported([judge, ...pool]); + target.models = modelsUsed([judge, ...pool]); if (!values["dry-run"]) { const checks = await Promise.all(pool.map(probe)); const missing = checks.filter(a => !a.ok); @@ -637,6 +644,9 @@ async function cmdAgent(argv) { console.log(st.field("judge", st.agent(judge.name))); console.log(st.field("juries", pool.map((a) => st.agent(a.name)).join(", ") + (values["dry-run"] ? st.warn(" (dry run)") : ""))); + if (target.models) { + console.log(st.field("models", Object.entries(target.models).map(([n, m]) => `${st.agent(n)} ${m}`).join(", "))); + } console.log(st.field("rounds", st.muted(`${first}..${first + maxRounds - 1}, ${MAX_TURNS} turns per finding`))); console.log(st.field("fixes", st.muted(values.push ? `committed and pushed to ${group ? group.targets.map(t => t.branch).join(", ") : pushTarget.branch}` : "committed to the worktree only"))); @@ -1072,12 +1082,14 @@ async function cmdReply(argv) { run: { type: "string" }, dir: { type: "string" }, ...reviewerOptions, + model: { type: "string", multiple: true }, "dry-run": { type: "boolean", default: false }, }, }); const worktree = await commandDirectory(values.dir); const cfg = await loadConfig(worktree); + applyModelFlags(cfg, values.model); const dir = await resolveRun(values.run, worktree); const events = await readEvents(dir); const judge = currentJudge(events); @@ -1094,6 +1106,7 @@ async function cmdReply(argv) { return t.length && t.some((f) => f.status !== "open"); }); if (!pool.length) throw new Error("nothing to reply about — resolve some findings first"); + assertModelsSupported(pool); console.log(`judge ${judge}`); console.log(`replying ${pool.map((a) => a.name).join(", ")} (separate conversations)`); @@ -1198,6 +1211,7 @@ async function cmdRuns(argv) { async function cmdAgents(args = []) { if (["jury", "reviewer", "reviewers"].includes(args[0])) return cmdDefaultReviewers(args.slice(1)); + if (["model", "models"].includes(args[0])) return cmdAgentModels(args.slice(1)); if (args.length) { if (args[0] !== "judge" || args.length > 2) throw new Error("Usage: jury agents judge [|--reset] | jury agents jury [,...|--reset]"); const name = args[1]; @@ -1239,7 +1253,8 @@ async function cmdAgents(args = []) { console.log( `${p.ok ? "ok " : "MISSING"} ${p.name.padEnd(9)} ${role.padEnd(8)} ${p.path ?? p.bin}` + (byName.get(p.name)?.enabled === false ? " (opt-in)" : "") - + (saved.has(p.name) ? " (default reviewer)" : ""), + + (saved.has(p.name) ? " (default reviewer)" : "") + + (byName.get(p.name)?.model ? ` (model ${byName.get(p.name).model})` : ""), ); } if (cfg.savedReviewers) { @@ -1281,6 +1296,50 @@ async function cmdAgents(args = []) { } } +/** + * `jury agents model`: the model each agent is started with when jury runs it. + * + * Saved in ~/.jury/config.json and passed to the agent as a flag or variable + * on each run; the agent's own configuration is never touched, so running the + * CLI outside jury keeps its normal default. + */ +async function cmdAgentModels(args) { + const usage = "Usage: jury agents model [ | --reset]"; + if (args.includes("--help") || args.includes("-h")) { + process.stdout.write(commandHelp("agents", USAGE_FULL)); + return; + } + const cfg = await loadConfig(); + const known = knownAgents(cfg); + if (!args.length) { + for (const a of known) { + const setting = a.model ? `${a.model} (${a.modelSource})` : supportsModel(a) ? "CLI default" : "CLI default (no per-run model)"; + console.log(`${a.name.padEnd(9)} ${setting}`); + } + return; + } + if (args.length !== 2) throw new Error(usage); + const [name, model] = args; + const agent = known.find(a => a.name === name); + if (!agent) throw new Error(`Unknown or disabled agent "${name}". Choose: ${known.map(a => a.name).join(", ")}`); + if (model === "--reset") { + await saveGlobalModel(name, null); + console.log(`${name}: saved model removed; the repository setting or the CLI's own default applies.`); + return; + } + const problem = modelProblem(model); + if (problem) throw new Error(problem); + if (!supportsModel(agent)) { + throw new Error(`${name} cannot select a model per run: ${agent.modelNote ?? "its command has no {{modelArgs}} slot"}`); + } + await saveGlobalModel(name, model); + console.log(`${name}: model ${model} (${globalConfigPath()})`); + if (agent.modelSource && agent.modelSource !== "~/.jury/config.json") { + console.log(`${agent.modelSource} sets ${agent.model} for ${name} here and takes precedence.`); + } + console.log("Used only when jury runs this agent; its own configuration is unchanged. --model = overrides it per run."); +} + /** * `jury agents jury`: the saved default reviewers, used when a run names none. * diff --git a/docs/configuration.md b/docs/configuration.md index 370647c..6199e65 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -57,6 +57,47 @@ A judge may specify separate `judgeArgv`. A custom `argv` overrides the built-in unless you also specify `judgeArgv`. Optional session configuration is illustrated by the built-in definitions in `lib/agents/`. +## Models + +Jury can start an agent with a chosen model. The model is passed only on the command line or in +the environment of the process Jury starts; Jury never edits an agent's own configuration, so +running `claude`, `codex` and the others outside Jury keeps their normal default. + +```bash +jury agents model # the model each agent starts with +jury agents model claude opus # save a default in ~/.jury/config.json +jury agents model claude --reset # back to the CLI's own default +jury review --model claude=sonnet --model codex=gpt-5.5 # this run only +``` + +A repository can set one per agent in `jury.config.json`: + +```json +{ "agents": [{ "name": "claude", "model": "opus" }] } +``` + +Precedence: `--model =`, then `model` in `jury.config.json`, then the saved model, +then the CLI's own default. The same model is used on the first run, on resumed rounds and +replies, and when the agent judges. The run records the models it used (`models` in `run.json`), +and `jury agents` shows each agent's model. An agent with a configured model but no way to take +one stops the run before any agent starts rather than silently using its default. + +| Agent | How the model is passed | Verified against | +| --- | --- | --- | +| `codex` | `--model` after `exec` (and `exec resume`) | codex-cli 0.156.1 | +| `claude` | `--model` | Claude Code 2.1.281 | +| `grok` | `GROK_MODEL` environment variable | grok-cli README | +| `opencode` | `--model provider/model` after `run` | opencode 1.18.32 | +| `qwen` | `--model` | qwen 0.24.4 | +| `copilot` | `--model` | Copilot CLI 1.0.88 | +| `kimi` | `--model`, an alias defined in `~/.kimi-code/config.toml` | Kimi Code 2.1.1 | +| `amp` | not supported: `--mode` selects model, prompt and tools together | — | +| `droid`, `agy`, `cursor`, `trae` | not yet verified; set the model in the CLI's own configuration | — | + +A built-in definition supports models by marking the flag's position with a `"{{modelArgs}}"` +element in every command it runs and giving `modelArgs` (for example `["--model", "{{model}}"]`), +or by giving `modelEnv`. A custom agent can use `{{model}}` directly in its `argv`. + ## Adding a built-in agent Built-in agents are data, not code: each is one JSON file in `lib/agents/`, listed in diff --git a/lib/agent-schema.js b/lib/agent-schema.js index 114d43a..32d8329 100644 --- a/lib/agent-schema.js +++ b/lib/agent-schema.js @@ -69,6 +69,12 @@ const FIELDS = { why: "an object with a boolean `supported`" }, expectSeconds: { required: false, check: (v) => Number.isFinite(v) && v > 0, why: "a positive number of seconds" }, + modelArgs: { required: false, check: (v) => isArgv(v) && v.some((a) => a.includes("{{model}}")), + why: 'the arguments that select a model, naming it as {{model}} (e.g. ["--model", "{{model}}"])' }, + modelEnv: { required: false, check: (v) => isEnv(v) && Object.values(v).some((a) => a.includes("{{model}}")), + why: 'environment variables that select a model, naming it as {{model}} (e.g. {"GROK_MODEL": "{{model}}"})' }, + modelNote: { required: false, check: (v) => typeof v === "string" && v.trim().length > 0, + why: "why this agent cannot select a model per run, shown when a model is configured for it" }, enabled: { required: false, check: (v) => typeof v === "boolean", why: "a boolean" }, install: { required: false, check: (v) => typeof v === "string" && v.trim().length > 0, why: "the command that installs this agent" }, @@ -135,6 +141,22 @@ export function validateAgent(agent, { source = "agent" } = {}) { problems.push(`${source}: argv[0] must be the executable name, not a template`); } + // A model slot in one command but not another would switch models between a + // review and its resumed reply. Every variant carries it, or none does. + const variants = [["argv", agent.argv], ["judgeArgv", agent.judgeArgv], ["replyArgv", agent.replyArgv], + ["resume.argv", agent.resume?.supported ? agent.resume.argv : null]].filter(([, v]) => isArgv(v)); + const slotted = variants.filter(([, v]) => v.includes("{{modelArgs}}")); + if (Object.hasOwn(agent, "modelArgs")) { + for (const [key] of variants.filter((v) => !slotted.includes(v))) { + problems.push(`${source}: modelArgs is set, so ${key} must contain a "{{modelArgs}}" element marking where it goes`); + } + } else if (slotted.length) { + problems.push(`${source}: "{{modelArgs}}" appears in ${slotted.map(([k]) => k).join(", ")} but modelArgs is not defined`); + } + if ((agent.modelArgs || agent.modelEnv) && agent.modelNote) { + problems.push(`${source}: modelNote explains a missing model setting, but this agent has one`); + } + return problems; } diff --git a/lib/agents.js b/lib/agents.js index 04d0e1c..586e942 100644 --- a/lib/agents.js +++ b/lib/agents.js @@ -20,6 +20,41 @@ function subst(argv, vars) { return argv.map((a) => a.replace(/\{\{(\w+)\}\}/g, (_, k) => vars[k] ?? "")); } +/** + * Put the configured model on a command line. + * + * A definition marks where its model flag belongs with a standalone + * "{{modelArgs}}" element and says what goes there in `modelArgs` (codex: + * ["-m", "{{model}}"]). The slot disappears when no model is set, so a run + * without one is exactly the command it always was, and the agent's own + * default applies. Nothing is ever written to the agent's own config. + */ +export function withModel(argv, agent) { + const model = agent.model; + return argv.flatMap((a) => { + if (a !== "{{modelArgs}}") return [a.replaceAll("{{model}}", model ?? "")]; + return model && agent.modelArgs ? agent.modelArgs.map((m) => m.replaceAll("{{model}}", model)) : []; + }); +} + +/** Environment that carries the model, for agents that take it that way. */ +export function modelEnv(agent) { + if (!agent.model || !agent.modelEnv) return {}; + return Object.fromEntries(Object.entries(agent.modelEnv).map(([k, v]) => [k, v.replaceAll("{{model}}", agent.model)])); +} + +/** + * Whether this invocation can carry a model. Every command variant must: a + * model that reached the first round but was dropped from a resumed one would + * switch models halfway through a conversation. + */ +export function supportsModel(agent) { + if (agent.modelEnv) return true; + const variants = [agent.argv, agent.resume?.supported ? agent.resume.argv : null].filter(Boolean); + const carries = (argv) => argv.some((a) => a === "{{modelArgs}}" && agent.modelArgs || a.includes("{{model}}")); + return variants.length > 0 && variants.every(carries); +} + /** * Run one reviewer against a worktree. Never throws for a failing agent — a * dead reviewer is a result, not a crash, and the round should still report the @@ -60,7 +95,7 @@ export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onL worktree, promptFile: promptFile ?? "", promptText: prompt, sessionId: sessionId ?? assigned ?? "", packageDir: PACKAGE_DIR, }; - const [cmd, ...args] = subst(commandArgv, vars); + const [cmd, ...args] = subst(withModel(commandArgv, agent), vars); const cwd = agent.cwd === "worktree" ? worktree : process.cwd(); // The caller already prints the agent's name at the head of this line, so @@ -77,7 +112,7 @@ export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onL // stdin must be closed, not an open pipe. A pipe that never delivers and // never ends leaves an agent waiting on input forever: codex sat at 0% CPU // for over an hour before this was fixed. - child = spawn(cmd, args, { cwd, env: { ...process.env, PWD: cwd, ...agent.env }, stdio: ["ignore", "pipe", "pipe"] }); + child = spawn(cmd, args, { cwd, env: { ...process.env, PWD: cwd, ...agent.env, ...modelEnv(agent) }, stdio: ["ignore", "pipe", "pipe"] }); } catch (err) { resolve({ code: -1, stdout: "", stderr: String(err) }); return; diff --git a/lib/agents/agy.json b/lib/agents/agy.json index d3d3e5f..2045d8d 100644 --- a/lib/agents/agy.json +++ b/lib/agents/agy.json @@ -36,6 +36,7 @@ ], "idFrom": "\"conversation_id\"\\s*:\\s*\"([^\"]+)\"" }, + "modelNote": "no per-run model flag verified for this CLI yet; set its model in the CLI's own configuration", "report": "agy-json", "expectSeconds": 420, "install": "npm install -g @google/antigravity-cli", diff --git a/lib/agents/amp.json b/lib/agents/amp.json index 23155ca..27585c3 100644 --- a/lib/agents/amp.json +++ b/lib/agents/amp.json @@ -19,6 +19,7 @@ "supported": false, "reason": "execute mode prints no thread id to resume by" }, + "modelNote": "Amp has no per-run model flag: --mode (low, medium, high, ultra) selects the model, system prompt and tools together", "report": "whole", "expectSeconds": 400, "install": "npm install -g @sourcegraph/amp", diff --git a/lib/agents/claude.json b/lib/agents/claude.json index 27b8595..1addd57 100644 --- a/lib/agents/claude.json +++ b/lib/agents/claude.json @@ -6,6 +6,7 @@ "cwd": "worktree", "argv": [ "claude", + "{{modelArgs}}", "-p", "{{promptText}}", "--session-id", @@ -19,6 +20,7 @@ ], "judgeArgv": [ "claude", + "{{modelArgs}}", "-p", "{{promptText}}", "--permission-mode", @@ -32,6 +34,7 @@ "supported": true, "argv": [ "claude", + "{{modelArgs}}", "-p", "{{promptText}}", "--resume", @@ -44,6 +47,10 @@ "{{worktree}}" ] }, + "modelArgs": [ + "--model", + "{{model}}" + ], "report": "whole", "expectSeconds": 900, "install": "npm install -g @anthropic-ai/claude-code", diff --git a/lib/agents/codex.json b/lib/agents/codex.json index 0349f3b..830d652 100644 --- a/lib/agents/codex.json +++ b/lib/agents/codex.json @@ -4,15 +4,16 @@ "role": "main", "promptDelivery": "argv", "cwd": "worktree", - "argv": ["codex", "exec", "--sandbox", "read-only", "--skip-git-repo-check", "{{promptText}}"], - "judgeArgv": ["codex", "exec", "--sandbox", "workspace-write", "--skip-git-repo-check", "{{promptText}}"], + "argv": ["codex", "exec", "{{modelArgs}}", "--sandbox", "read-only", "--skip-git-repo-check", "{{promptText}}"], + "judgeArgv": ["codex", "exec", "{{modelArgs}}", "--sandbox", "workspace-write", "--skip-git-repo-check", "{{promptText}}"], "sandbox": "sandbox", "sandboxNote": "reviews run under --sandbox read-only; judging uses workspace-write", "resume": { "supported": true, - "argv": ["codex", "exec", "resume", "{{sessionId}}", "{{promptText}}"], + "argv": ["codex", "exec", "resume", "{{modelArgs}}", "{{sessionId}}", "{{promptText}}"], "idFrom": "session[ _-]?id[:=]?\\s*([0-9a-f-]{8,})" }, + "modelArgs": ["--model", "{{model}}"], "report": "tail", "expectSeconds": 1100, "install": "npm install -g @openai/codex", diff --git a/lib/agents/copilot.json b/lib/agents/copilot.json index c6f8ec8..da03e18 100644 --- a/lib/agents/copilot.json +++ b/lib/agents/copilot.json @@ -7,6 +7,7 @@ "cwd": "worktree", "argv": [ "copilot", + "{{modelArgs}}", "-p", "{{promptText}}", "--silent", @@ -31,6 +32,7 @@ ], "judgeArgv": [ "copilot", + "{{modelArgs}}", "-p", "{{promptText}}", "--silent", @@ -44,6 +46,10 @@ "supported": false, "reason": "fresh conversation with finding context; no session ids inferred from assistant prose" }, + "modelArgs": [ + "--model", + "{{model}}" + ], "report": "whole", "expectSeconds": 400, "install": "npm install -g @github/copilot", diff --git a/lib/agents/cursor.json b/lib/agents/cursor.json index 72def9e..bf91d77 100644 --- a/lib/agents/cursor.json +++ b/lib/agents/cursor.json @@ -28,6 +28,7 @@ "supported": false, "reason": "fresh conversation with finding context; never extract session ids from assistant prose" }, + "modelNote": "no per-run model flag verified for this CLI yet; set its model in the CLI's own configuration", "report": "result-json", "expectSeconds": 400, "install": "curl https://cursor.com/install -fsS | bash", diff --git a/lib/agents/droid.json b/lib/agents/droid.json index 6a11bad..2516861 100644 --- a/lib/agents/droid.json +++ b/lib/agents/droid.json @@ -36,6 +36,7 @@ ], "idFrom": "\"session_id\"\\s*:\\s*\"([^\"]+)\"" }, + "modelNote": "no per-run model flag verified for this CLI yet; set its model in the CLI's own configuration", "report": "result-json", "expectSeconds": 130, "install": "curl -fsSL https://app.factory.ai/cli | sh", diff --git a/lib/agents/grok.json b/lib/agents/grok.json index 9f6a2c7..5ee2b86 100644 --- a/lib/agents/grok.json +++ b/lib/agents/grok.json @@ -15,6 +15,7 @@ "supported": true, "argv": ["grok", "--cwd", "{{worktree}}", "--always-approve", "--resume", "{{sessionId}}", "-p", "{{promptText}}"] }, + "modelEnv": { "GROK_MODEL": "{{model}}" }, "report": "whole", "expectSeconds": 240, "install": "npm install -g @vibe-kit/grok-cli", diff --git a/lib/agents/kimi.json b/lib/agents/kimi.json index 73ccd2e..324d6a9 100644 --- a/lib/agents/kimi.json +++ b/lib/agents/kimi.json @@ -7,6 +7,7 @@ "cwd": "worktree", "argv": [ "kimi", + "{{modelArgs}}", "-p", "{{promptText}}", "--output-format", @@ -16,6 +17,7 @@ ], "judgeArgv": [ "kimi", + "{{modelArgs}}", "-p", "{{promptText}}", "--output-format", @@ -27,6 +29,7 @@ "supported": true, "argv": [ "kimi", + "{{modelArgs}}", "-p", "{{promptText}}", "--output-format", @@ -36,6 +39,10 @@ ], "idFrom": "session_id\\W{0,3}((?:session_)?[0-9a-f]{8}-[0-9a-f-]{27})" }, + "modelArgs": [ + "--model", + "{{model}}" + ], "report": "kimi-json", "expectSeconds": 600, "install": "npm install -g @moonshot-ai/kimi-code", diff --git a/lib/agents/opencode.json b/lib/agents/opencode.json index 925ab6b..25f85ed 100644 --- a/lib/agents/opencode.json +++ b/lib/agents/opencode.json @@ -7,6 +7,7 @@ "argv": [ "opencode", "run", + "{{modelArgs}}", "--dir", "{{worktree}}", "--agent", @@ -19,6 +20,7 @@ "judgeArgv": [ "opencode", "run", + "{{modelArgs}}", "--dir", "{{worktree}}", "--agent", @@ -42,6 +44,7 @@ "argv": [ "opencode", "run", + "{{modelArgs}}", "--dir", "{{worktree}}", "--agent", @@ -55,6 +58,10 @@ ], "idFrom": "\"sessionID\"\\s*:\\s*\"([^\"]+)\"" }, + "modelArgs": [ + "--model", + "{{model}}" + ], "report": "opencode-json", "expectSeconds": 600, "install": "npm install -g opencode-ai", diff --git a/lib/agents/qwen.json b/lib/agents/qwen.json index 35a33c8..05093dd 100644 --- a/lib/agents/qwen.json +++ b/lib/agents/qwen.json @@ -7,6 +7,7 @@ "cwd": "worktree", "argv": [ "qwen", + "{{modelArgs}}", "--approval-mode", "default", "--output-format", @@ -29,6 +30,7 @@ ], "judgeArgv": [ "qwen", + "{{modelArgs}}", "--approval-mode", "yolo", "--output-format", @@ -42,6 +44,7 @@ "supported": true, "argv": [ "qwen", + "{{modelArgs}}", "--approval-mode", "default", "--output-format", @@ -63,6 +66,10 @@ "--prompt={{promptText}}" ] }, + "modelArgs": [ + "--model", + "{{model}}" + ], "report": "result-json", "expectSeconds": 400, "install": "npm install -g @qwen-code/qwen-code", diff --git a/lib/agents/trae.json b/lib/agents/trae.json index 4acd131..ec2ff6b 100644 --- a/lib/agents/trae.json +++ b/lib/agents/trae.json @@ -29,6 +29,7 @@ "supported": false, "reason": "exec session ids are not documented in its output; replies start fresh with finding context" }, + "modelNote": "no per-run model flag verified for this CLI yet; set its model in the CLI's own configuration", "report": "whole", "expectSeconds": 900, "install": "sh -c \"$(curl -fsSL https://trae.cn/trae-cli/install_v2.sh)\"", diff --git a/lib/config.js b/lib/config.js index 26dcae9..5857d77 100644 --- a/lib/config.js +++ b/lib/config.js @@ -13,6 +13,7 @@ import path from "node:path"; import { homedir } from "node:os"; import { randomUUID } from "node:crypto"; import { BUILTIN_AGENTS } from "./agents/registry.js"; +import { supportsModel } from "./agents.js"; export const DEFAULTS = { stopToken: "NO NEW FINDINGS", @@ -42,6 +43,10 @@ export async function readGlobalConfig(file = globalConfigPath()) { || settings.reviewers.some(n => typeof n !== "string" || !n.trim()))) { throw new Error("expected reviewers to be a nonempty list of agent names"); } + if (settings.models !== undefined && (!settings.models || typeof settings.models !== "object" + || Array.isArray(settings.models) || Object.values(settings.models).some(m => modelProblem(m)))) { + throw new Error("expected models to map agent names to model names"); + } return settings; } catch (err) { if (err.code === "ENOENT") return {}; @@ -59,6 +64,26 @@ export async function saveGlobalReviewers(names, file = globalConfigPath()) { await saveGlobalSetting("reviewers", names, file); } +/** Save one agent's default model, or remove it with null. */ +export async function saveGlobalModel(agent, model, file = globalConfigPath()) { + const settings = await readGlobalConfig(file); + const models = { ...(settings.models ?? {}) }; + if (model === null) delete models[agent]; + else models[agent] = model; + await saveGlobalSetting("models", Object.keys(models).length ? models : null, file); +} + +/** + * Why a model name is unusable, or null. A model reaches the agent as its own + * argument, so the one real hazard is a value the CLI would parse as a flag. + */ +export function modelProblem(model) { + if (typeof model !== "string" || !model.trim()) return "a model name must be a nonempty string"; + if (model !== model.trim() || /\s/.test(model)) return `model "${model}" must not contain whitespace`; + if (model.startsWith("-")) return `model "${model}" must not start with "-"`; + return null; +} + async function saveGlobalSetting(key, value, file) { const settings = await readGlobalConfig(file); if (value === null) delete settings[key]; @@ -101,9 +126,24 @@ export async function loadConfig(dir = process.cwd(), { globalFile = globalConfi // A custom executable must also be used for judging unless explicitly overridden. if (a.argv && !Object.hasOwn(a, "judgeArgv")) delete merged.judgeArgv; if (a.role === "main" && a.enabled !== false) merged.enabled = true; + if (Object.hasOwn(a, "model")) { + const problem = modelProblem(a.model); + if (problem) throw new Error(`${found}: ${a.name}: ${problem}`); + merged.modelSource = found; + } byName.set(a.name, merged); } + // A saved default model fills in only where the repository chose none: the + // repository's jury.config.json is the more specific setting. + for (const [name, model] of Object.entries(global.models ?? {})) { + const agent = byName.get(name); + if (agent && !Object.hasOwn(agent, "modelSource")) { + agent.model = model; + agent.modelSource = "~/.jury/config.json"; + } + } + // Two sets, deliberately. `agents` is the default pool — what runs when nobody // names a reviewer. `available` is every agent that is merely *known*, which // includes built-ins that ship opt-in so that a default install does not @@ -216,3 +256,41 @@ export function defaultReviewers(cfg, judge = null) { } export const savedReviewersNote = SAVED; + +/** + * Apply `--model =` for this run only. It outranks both config + * files, and names an agent from the whole known set, like --reviewer. + */ +export function applyModelFlags(cfg, flags = []) { + const byName = new Map(knownAgents(cfg).map(a => [a.name, a])); + for (const flag of flags) { + const at = flag.indexOf("="); + const name = at > 0 ? flag.slice(0, at).trim() : ""; + const model = at > 0 ? flag.slice(at + 1) : ""; + if (!name) throw new Error(`--model expects =, got "${flag}"`); + const agent = byName.get(name); + if (!agent) throw new Error(`--model names unknown or disabled agent "${name}". Choose: ${[...byName.keys()].join(", ")}`); + const problem = modelProblem(model); + if (problem) throw new Error(`--model ${name}: ${problem}`); + agent.model = model; + agent.modelSource = "--model"; + } +} + +/** + * Refuse a run that would ignore a configured model. Starting anyway would + * review with the agent's own default while the run record, and the user, + * believe another model was used. + */ +export function assertModelsSupported(agents) { + const problems = agents.filter(a => a.model && !supportsModel(a)).map(a => + `${a.name} cannot select a model per run (${a.modelNote ?? "its command has no {{modelArgs}} slot"}); ` + + `remove model "${a.model}" from ${a.modelSource ?? "its configuration"}`); + if (problems.length) throw new Error(problems.join("\n")); +} + +/** The model each agent in a run will use, for the run record. */ +export function modelsUsed(agents) { + const used = agents.filter(a => a.model).map(a => [a.name, a.model]); + return used.length ? Object.fromEntries(used) : undefined; +} diff --git a/lib/help.js b/lib/help.js index 5591647..2eeda59 100644 --- a/lib/help.js +++ b/lib/help.js @@ -13,7 +13,7 @@ export function commandHelp(topic, full) { finding: section("finding commands", "With a PR URL"), reply: `jury reply [--dir ] [--run ]\n\nReply to each reviewer about its answered findings.\n${dir}\n${[...new Set(reviewers)].join("\n")}`, runs: `jury runs [--dir ]\n\nList recorded review runs and their slugs.\n${dir}`, - agents: "jury agents\n\nCheck which configured agents are installed and show their roles.\n\njury agents judge show the global default\njury agents judge set the global default\njury agents judge --reset remove the global override\n\nSaved in ~/.jury/config.json. Precedence: --judge, repository main role, global judge, built-in default.\n\njury agents jury show the default reviewers (a picker in a terminal)\njury agents jury [,] set the default reviewers\njury agents jury --reset back to the built-in reviewer pool\n\nSaved in ~/.jury/config.json. Precedence: --reviewer/--jury, repository reviewer roles, saved default reviewers, built-in pool.", + agents: "jury agents\n\nCheck which configured agents are installed and show their roles.\n\njury agents judge show the global default\njury agents judge set the global default\njury agents judge --reset remove the global override\n\nSaved in ~/.jury/config.json. Precedence: --judge, repository main role, global judge, built-in default.\n\njury agents jury show the default reviewers (a picker in a terminal)\njury agents jury [,] set the default reviewers\njury agents jury --reset back to the built-in reviewer pool\n\nSaved in ~/.jury/config.json. Precedence: --reviewer/--jury, repository reviewer roles, saved default reviewers, built-in pool.\n\njury agents model show the model each agent starts with\njury agents model save a default model for an agent\njury agents model --reset back to the CLI's own default\n\nPassed as a flag or variable only when jury runs the agent; the agent's own config is never changed. Precedence: --model =, the agent's \"model\" in jury.config.json, saved model, CLI default.", version: "jury version\n\nPrint the installed package version.", help: "jury help [command]\njury help --all\n\nShow command help or the complete command reference.", }; diff --git a/lib/reply.js b/lib/reply.js index 60b785c..d40936d 100644 --- a/lib/reply.js +++ b/lib/reply.js @@ -11,6 +11,8 @@ // otherwise a fresh process that has never seen the thread, so its own // prior findings must be quoted back or the reply is // addressed to an agent with no idea what it said +import { withModel } from "./agents.js"; + /** What one reviewer said, and what the main agent decided about each item. */ export function threadFor(agent, findings) { @@ -127,7 +129,7 @@ export function replyArgv(agent, { promptText, promptFile, worktree, sha, sessio // it here left a file-delivery reviewer spawned as `-f ""` with no way to // recover the placeholder. An unknown key is left standing rather than // erased, so a downstream substitution can still fill it. - argv: argv.map((a) => + argv: withModel(argv, agent).map((a) => a.replace(/\{\{(\w+)\}\}/g, (m, k) => (vars[k] === undefined ? m : vars[k])), ), resumed: Boolean(resumable), diff --git a/test/models.test.js b/test/models.test.js new file mode 100644 index 0000000..9978f04 --- /dev/null +++ b/test/models.test.js @@ -0,0 +1,196 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, readFile, readdir, writeFile, rm, chmod } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { execFileSync, spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { + DEFAULTS, loadConfig, judgeAgent, knownAgents, applyModelFlags, assertModelsSupported, modelsUsed, + saveGlobalModel, readGlobalConfig, +} from "../lib/config.js"; +import { runAgent, withModel, modelEnv, supportsModel } from "../lib/agents.js"; +import { replyArgv } from "../lib/reply.js"; +import { validateAgent } from "../lib/agent-schema.js"; +const cli = process.env.JURY_TEST_CLI ?? fileURLToPath(new URL("../bin/jury.js", import.meta.url)); + +async function scratch(t, prefix) { + const dir = await mkdtemp(path.join(tmpdir(), prefix)); + t.after(() => rm(dir, { recursive: true, force: true })); + return dir; +} + +// Where each CLI's model flag lands, verified against its own --help: +// codex-cli 0.156.1 (exec and exec resume: -m, --model), Claude Code 2.1.281 +// (--model), opencode 1.18.32 (run: -m, --model), qwen 0.24.4 (-m, --model), +// Copilot CLI 1.0.88 (--model), Kimi Code 2.1.1 (-m, --model). +const FLAG_AT = { + codex: { argv: 2, judgeArgv: 2, resume: 3 }, + claude: { argv: 1, judgeArgv: 1, resume: 1 }, + opencode: { argv: 2, judgeArgv: 2, resume: 2 }, + qwen: { argv: 1, judgeArgv: 1, resume: 1 }, + copilot: { argv: 1, judgeArgv: 1 }, + kimi: { argv: 1, judgeArgv: 1, resume: 1 }, +}; +const byName = new Map(DEFAULTS.agents.map(a => [a.name, a])); + +test("each supported agent gets its model flag in every command, and nothing without a model", () => { + for (const [name, at] of Object.entries(FLAG_AT)) { + const agent = byName.get(name); + assert.ok(supportsModel(agent), name); + const variants = { argv: agent.argv, judgeArgv: agent.judgeArgv, resume: agent.resume?.argv }; + for (const [key, index] of Object.entries(at)) { + const argv = variants[key]; + const withOne = withModel(argv, { ...agent, model: "m-1" }); + assert.deepEqual(withOne.slice(index, index + 2), ["--model", "m-1"], `${name} ${key}`); + assert.deepEqual(withModel(argv, agent), argv.filter(a => a !== "{{modelArgs}}"), `${name} ${key} unchanged without a model`); + } + } + // grok reads GROK_MODEL (superagent-ai/grok-cli README); its argv is untouched. + const grok = byName.get("grok"); + assert.ok(supportsModel(grok)); + assert.deepEqual(modelEnv({ ...grok, model: "grok-4.3" }), { GROK_MODEL: "grok-4.3" }); + assert.deepEqual(modelEnv(grok), {}); + assert.deepEqual(withModel(grok.argv, { ...grok, model: "grok-4.3" }), grok.argv); +}); + +test("agents without a verified per-run model say so and refuse a configured model", () => { + for (const name of ["amp", "droid", "agy", "cursor", "trae"]) { + const agent = byName.get(name); + assert.equal(supportsModel(agent), false, name); + assert.ok(agent.modelNote, `${name} explains why`); + assert.throws(() => assertModelsSupported([{ ...agent, model: "x", modelSource: "--model" }]), + new RegExp(`${name} cannot select a model per run .*remove model "x" from --model`)); + } + assert.doesNotThrow(() => assertModelsSupported([byName.get("amp"), { ...byName.get("codex"), model: "x" }])); + // A custom command without a slot cannot carry the model either. + assert.equal(supportsModel({ name: "mine", argv: ["mine", "{{promptText}}"], resume: { supported: false } }), false); + assert.equal(supportsModel({ name: "mine", argv: ["mine", "--model={{model}}"], resume: { supported: false } }), true); +}); + +test("the schema keeps the model slot and its arguments together", () => { + const codex = byName.get("codex"); + const noSlot = { ...codex, judgeArgv: codex.judgeArgv.filter(a => a !== "{{modelArgs}}") }; + assert.ok(validateAgent(noSlot).some(p => /judgeArgv must contain a "\{\{modelArgs\}\}"/.test(p))); + const { modelArgs, ...slotOnly } = codex; + assert.ok(validateAgent(slotOnly).some(p => /modelArgs is not defined/.test(p))); + assert.ok(validateAgent({ ...codex, modelArgs: ["--model"] }).some(p => /"modelArgs" must be/.test(p))); + assert.ok(validateAgent({ ...codex, modelNote: "none" }).some(p => /modelNote/.test(p))); +}); + +test("the model reaches the process on first runs, resumed rounds, judging and replies", async t => { + const dir = await scratch(t, "jury-model-run-"); + const log = path.join(dir, "calls.jsonl"); + const exe = path.join(dir, "fake-agent"); + await writeFile(exe, `#!${process.execPath}\nrequire("node:fs").appendFileSync(${JSON.stringify(log)}, JSON.stringify({ argv: process.argv.slice(2), grok: process.env.GROK_MODEL ?? null }) + "\\n");\nconsole.log("session id: 0123abcd-0000");\nconsole.log("NO NEW FINDINGS");\n`); + await chmod(exe, 0o755); + const swap = argv => [exe, ...argv.slice(1)]; + const codex = { ...byName.get("codex"), model: "gpt-x" }; + const stand = { ...codex, argv: swap(codex.argv), resume: { ...codex.resume, argv: swap(codex.resume.argv) } }; + const opts = { worktree: dir, prompt: "review", stopToken: "NO NEW FINDINGS", timeoutSeconds: 10 }; + assert.ok((await runAgent(stand, opts)).ok); + assert.ok((await runAgent(stand, { ...opts, sessionId: "0123abcd-0000" })).ok); + assert.ok((await runAgent({ ...stand, argv: swap(codex.judgeArgv) }, opts)).ok); + assert.ok((await runAgent({ ...stand, model: undefined }, opts)).ok); + const grok = byName.get("grok"); + assert.ok((await runAgent({ ...grok, model: "grok-4.3", argv: swap(grok.argv) }, opts)).ok); + const calls = (await readFile(log, "utf8")).trim().split("\n").map(l => JSON.parse(l)); + assert.deepEqual(calls[0].argv.slice(0, 3), ["exec", "--model", "gpt-x"]); + assert.deepEqual(calls[1].argv.slice(0, 5), ["exec", "resume", "--model", "gpt-x", "0123abcd-0000"]); + assert.deepEqual(calls[2].argv.slice(0, 4), ["exec", "--model", "gpt-x", "--sandbox"]); + assert.ok(!calls[3].argv.includes("--model")); + assert.ok(!calls[3].argv.some(a => a.includes("{{"))); + assert.equal(calls[4].grok, "grok-4.3"); + + const reply = replyArgv(codex, { promptText: "verdicts", worktree: dir, sessionId: "abc-123" }); + assert.deepEqual(reply.argv.slice(0, 6), ["codex", "exec", "resume", "--model", "gpt-x", "abc-123"]); + const fresh = replyArgv({ ...codex, model: undefined }, { promptText: "verdicts", worktree: dir }); + assert.ok(!fresh.argv.some(a => a.includes("{{modelArgs}}") || a === "--model")); +}); + +test("precedence: --model, then the repository, then the saved model, then the CLI default", async t => { + const dir = await scratch(t, "jury-model-precedence-"); + const globalFile = path.join(dir, "global.json"); + let cfg = await loadConfig(dir, { globalFile }); + assert.equal(knownAgents(cfg).find(a => a.name === "claude").model, undefined); + + await saveGlobalModel("claude", "sonnet", globalFile); + await saveGlobalModel("codex", "gpt-a", globalFile); + cfg = await loadConfig(dir, { globalFile }); + const claude = () => knownAgents(cfg).find(a => a.name === "claude"); + assert.equal(claude().model, "sonnet"); + assert.equal(claude().modelSource, "~/.jury/config.json"); + assert.equal(judgeAgent(cfg).model, "gpt-a", "the judge carries its model too"); + + await writeFile(path.join(dir, "jury.config.json"), JSON.stringify({ agents: [{ name: "claude", model: "opus" }] })); + cfg = await loadConfig(dir, { globalFile }); + assert.equal(claude().model, "opus"); + assert.equal(claude().modelSource, "jury.config.json"); + + applyModelFlags(cfg, ["claude=haiku", "amp=smart"]); + assert.equal(claude().model, "haiku"); + assert.equal(claude().modelSource, "--model"); + assert.deepEqual(modelsUsed([judgeAgent(cfg), claude()]), { codex: "gpt-a", claude: "haiku" }); + assert.equal(modelsUsed([knownAgents(cfg).find(a => a.name === "grok")]), undefined); + + for (const [flag, why] of [["claude", /=/], ["=x", /=/], ["nobody=x", /unknown or disabled agent "nobody"/], + ["claude=", /nonempty/], ["claude=-x", /must not start with "-"/], ["claude=a b", /whitespace/]]) { + assert.throws(() => applyModelFlags(cfg, [flag]), why, flag); + } + await writeFile(path.join(dir, "jury.config.json"), JSON.stringify({ agents: [{ name: "claude", model: "--danger" }] })); + await assert.rejects(loadConfig(dir, { globalFile }), /jury\.config\.json: claude: model "--danger" must not start with "-"/); + + await saveGlobalModel("claude", null, globalFile); + await saveGlobalModel("codex", null, globalFile); + assert.deepEqual(await readGlobalConfig(globalFile), {}); + await writeFile(globalFile, JSON.stringify({ models: { claude: "" } })); + await assert.rejects(readGlobalConfig(globalFile), /models to map agent names/); +}); + +test("jury agents model saves, lists, refuses and resets, and a review records the models used", async t => { + const home = await scratch(t, "jury-model-cli-"); + const repo = path.join(home, "repo"); + await mkdir(repo); + const env = { ...process.env, HOME: home, USERPROFILE: home }; + const run = (args, cwd = repo) => execFileSync(process.execPath, [cli, ...args], { cwd, env, encoding: "utf8" }); + const fail = (args) => spawnSync(process.execPath, [cli, ...args], { cwd: repo, env, encoding: "utf8" }); + const file = path.join(home, ".jury", "config.json"); + + assert.match(run(["agents", "model", "codex", "gpt-x"]), /codex: model gpt-x/); + const before = await readFile(file, "utf8"); + for (const [args, why] of [[["amp", "x"], /amp cannot select a model per run/], [["codex", "-x"], /must not start/], + [["nobody", "x"], /Unknown or disabled agent/], [["codex"], /Usage/]]) { + const bad = fail(["agents", "model", ...args]); + assert.notEqual(bad.status, 0, args.join(" ")); + assert.match(bad.stderr, why); + assert.equal(await readFile(file, "utf8"), before); + } + const listing = run(["agents", "model"]); + assert.match(listing, /codex\s+gpt-x\s+\(~\/\.jury\/config\.json\)/); + assert.match(listing, /claude\s+CLI default\n/); + assert.match(listing, /amp\s+CLI default\s+\(no per-run model\)/); + assert.match(spawnSync(process.execPath, [cli, "agents"], { cwd: repo, env, encoding: "utf8" }).stdout, /codex .*\(model gpt-x\)/); + assert.match(run(["agents", "model", "--help"]), /jury agents model /); + + const g = (...a) => execFileSync("git", ["-C", repo, ...a], { env }); + g("init", "-q", "-b", "master"); + g("-c", "user.email=t@example.com", "-c", "user.name=t", "commit", "-q", "--allow-empty", "-m", "base"); + g("checkout", "-q", "-b", "feature"); + g("-c", "user.email=t@example.com", "-c", "user.name=t", "commit", "-q", "--allow-empty", "-m", "change"); + const review = (...extra) => spawnSync(process.execPath, [cli, "review", "--dir", repo, "--trunk", "master", + "--rounds", "1", "--web=false", "--dry-run", "--jury", "claude", ...extra], { cwd: repo, env, encoding: "utf8" }); + + let result = review("--model", "claude=opus"); + assert.equal(result.stderr, ""); + assert.match(result.stdout, /models\s+codex gpt-x, claude opus/); + const [slug] = await readdir(path.join(repo, "runs")); + const saved = JSON.parse(await readFile(path.join(repo, "runs", slug, "run.json"), "utf8")); + assert.deepEqual(saved.target.models, { codex: "gpt-x", claude: "opus" }); + + result = review("--jury", "amp", "--model", "amp=x"); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /amp cannot select a model per run.*remove model "x" from --model/); + + run(["agents", "model", "codex", "--reset"]); + assert.match(run(["agents", "model"]), /codex\s+CLI default/); +});