From e480ee4e756b7045ad313e3f20c8e1dc1f09c5c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 10:21:48 +0000 Subject: [PATCH 1/2] feat: save default reviewers globally with jury agents jury Without --jury, a review used every enabled reviewer, so one missing CLI (e.g. grok) aborted every run unless --jury was passed each time or each repository's jury.config.json was edited. `jury agents jury [,]` saves a default reviewer list in ~/.jury/config.json beside the global judge; `--reset` removes it, and with no arguments in a terminal a readline checkbox picker shows install status and marks the judge. Precedence: --reviewer/--jury, repository reviewer roles, saved defaults, built-in pool. A saved list disables automatic role assignment, excludes the judge, and a saved reviewer that is uninstalled or disabled stops the run with an error naming the setting. Closes #98 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YWwuLax3VBtTJuujgK748c --- CHANGELOG.md | 4 + README.md | 15 +++ bin/jury.js | 101 +++++++++++++++++-- docs/configuration.md | 5 + lib/config.js | 57 ++++++++++- lib/help.js | 2 +- lib/picker.js | 75 ++++++++++++++ test/default-reviewers.test.js | 174 +++++++++++++++++++++++++++++++++ 8 files changed, 421 insertions(+), 12 deletions(-) create mode 100644 lib/picker.js create mode 100644 test/default-reviewers.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 282044e..dfe5f6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +- 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 + `--reviewer`/`--jury` still take precedence. `jury agents` marks the saved defaults (#98). - Add TRAE CLI (`traecli`) as an opt-in built-in agent named `trae`, usable as a reviewer (`--jury trae`) or the main agent (`--judge trae`). Reviews run in its read-only sandbox (#96). ## 0.7.0 — 2026-09-18 diff --git a/README.md b/README.md index 28b0b9d..a16de56 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,21 @@ is excluded from the reviewer pool. Without an explicit selection, enabled agents with the reviewer role are used; disable unavailable ones or select installed reviewers explicitly. +Or save default reviewers once for all repositories: + +```bash +jury agents jury claude # only claude reviews by default +jury agents jury claude,droid,amp # several; opt-in agents allowed +jury agents jury # show them, or pick with a checkbox list in a terminal +jury agents jury --reset # back to the built-in reviewer pool +``` + +Saved in `~/.jury/config.json`, next to the global judge. Precedence: `--reviewer`/`--jury`, +repository `reviewer` roles, saved default reviewers, then the built-in pool. A saved list turns +off automatic role assignment, as `--jury` does. The judge is left out of its own review. If a saved +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. + ### Built-in agents | name | product | install | default | diff --git a/bin/jury.js b/bin/jury.js index 5f61e73..316ddb4 100755 --- a/bin/jury.js +++ b/bin/jury.js @@ -13,7 +13,7 @@ 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, globalConfigPath } from "../lib/config.js"; +import { loadConfig, reviewers, judgeAgent, knownAgents, readGlobalConfig, saveGlobalJudge, saveGlobalReviewers, defaultReviewers, savedReviewersNote, globalConfigPath } from "../lib/config.js"; import { runAgent, probe } from "../lib/agents.js"; import { threadFor, buildReply, replyArgv } from "../lib/reply.js"; import { buildPrompt } from "../lib/prompt.js"; @@ -26,6 +26,7 @@ import { assertPrCheckout, repositoryFromPrUrl, resolvePrCheckout } from "../lib import { resolveJuryDirectory } from "../lib/directories.js"; import * as st from "../lib/style.js"; import { parseReviewArgs, reviewerOptions, requestedReviewers } from "../lib/cli-options.js"; +import { pick } from "../lib/picker.js"; import { startReviewConsole } from "../lib/review-console.js"; const run = promisify(execFile); @@ -60,7 +61,7 @@ Common flags --dir working/state root (default: Git cwd or ~/.jury) --rounds stop after n rounds (default: 10) - --reviewer only these reviewers, repeatable or comma-separated (default: configured reviewers) + --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) --push commit and push fixes (default: true) @@ -70,6 +71,7 @@ Other commands jury agents which reviewers are installed jury agents judge set the global default judge + jury agents jury set the default reviewers jury runs every PR under review jury version @@ -84,6 +86,7 @@ const USAGE_FULL = `jury — review a pull request with multiple AI reviewers un jury runs list every PR under review, with its slug for --run 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 version Related PRs: jury review @@ -100,7 +103,7 @@ review (triages, fixes, commits, and pushes automatica --title what the change does (default: read from the PR) --summary intent, passed to reviewers (default: the PR description) --rounds maximum rounds (default: 10) - --reviewer only these reviewers, repeatable or comma-separated (default: configured reviewers) + --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) --resume continue an existing run instead of starting a new one @@ -198,8 +201,8 @@ try { /** Resolve an explicit reviewer list or explain each ineligible name. */ function selectReviewers(cfg, requested, judge = null) { + if (!requested) return defaultReviewers(cfg, judge); const eligible = reviewers(cfg).filter((a) => a.name !== judge); - if (!requested) return eligible; const names = requested; if (!names.length) throw new Error("--reviewer needs at least one reviewer name"); @@ -608,7 +611,10 @@ async function cmdAgent(argv) { if (missing.length) { const reason = `reviewers not installed: ${missing.map(a => `${a.name} (${a.bin})`).join(", ")}`; await publishRun(dir, { ...target, state: "human", stateNote: reason }); - throw new Error(`${reason}. Install them, disable them in jury.config.json, or select installed reviewers with --reviewer . No agents were started.`); + const fromSaved = !roles && !requestedReviewers(values) && cfg.savedReviewers; + throw new Error(fromSaved + ? `${reason}, from the ${savedReviewersNote}. Install them, save installed reviewers, or select reviewers with --reviewer . No agents were started.` + : `${reason}. Install them, disable them in jury.config.json, or select installed reviewers with --reviewer . No agents were started.`); } } const first = Math.max(0, ...prior.filter((e) => e.t === "round.start").map((e) => e.n)) + 1; @@ -1191,8 +1197,9 @@ async function cmdRuns(argv) { } async function cmdAgents(args = []) { + if (["jury", "reviewer", "reviewers"].includes(args[0])) return cmdDefaultReviewers(args.slice(1)); if (args.length) { - if (args[0] !== "judge" || args.length > 2) throw new Error("Usage: jury agents judge [|--reset]"); + if (args[0] !== "judge" || args.length > 2) throw new Error("Usage: jury agents judge [|--reset] | jury agents jury [,...|--reset]"); const name = args[1]; if (name === "--help" || name === "-h") { process.stdout.write(commandHelp("agents", USAGE_FULL)); @@ -1219,6 +1226,7 @@ async function cmdAgents(args = []) { return; } const cfg = await loadConfig(); + const saved = new Set(cfg.savedReviewers ?? []); // Every known agent is listed, not just the default pool: an opt-in built-in // that is never shown is an agent nobody discovers. They are marked so the // listing still says which ones actually run without being asked for. @@ -1230,9 +1238,15 @@ async function cmdAgents(args = []) { const role = roleOf.get(p.name); 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)" : ""), + + (byName.get(p.name)?.enabled === false ? " (opt-in)" : "") + + (saved.has(p.name) ? " (default reviewer)" : ""), ); } + if (cfg.savedReviewers) { + console.log(`\nDefault reviewers: ${cfg.savedReviewers.join(", ")} (saved in ${globalConfigPath()}; change with jury agents jury)`); + } else if (cfg.globalReviewers) { + console.log(`\nSaved default reviewers (${cfg.globalReviewers.join(", ")}) are overridden by reviewer roles in ${path.basename(cfg.configFile)}.`); + } // An agent with no read-only mode still reviews, but only the prompt is // keeping it from editing the worktree. That is a real difference in what a @@ -1252,7 +1266,11 @@ async function cmdAgents(args = []) { // Only the default pool decides the exit status. An opt-in agent nobody asked // for is not a broken install, and failing on it would make `jury agents` // red on every machine that has not installed every supported CLI. - const missing = found.filter((p) => !p.ok && byName.get(p.name)?.enabled !== false); + // With saved default reviewers, those are the pool; the judge still counts. + const inPool = (p) => cfg.savedReviewers + ? saved.has(p.name) || roleOf.get(p.name) === "main" + : byName.get(p.name)?.enabled !== false; + const missing = found.filter((p) => !p.ok && inPool(p)); if (missing.length) { console.log(`\n${missing.length} agent(s) not installed. Install them, or disable in jury.config.json.`); for (const p of missing) { @@ -1263,6 +1281,73 @@ async function cmdAgents(args = []) { } } +/** + * `jury agents jury`: the saved default reviewers, used when a run names none. + * + * Mirrors `jury agents judge`. With no arguments on a TTY it opens a checkbox + * picker; anywhere else it prints the current setting, so a script piping + * `jury agents jury` never blocks waiting for keys. + */ +async function cmdDefaultReviewers(args) { + const usage = "Usage: jury agents jury [[,...]|--reset]"; + if (args.includes("--help") || args.includes("-h")) { + process.stdout.write(commandHelp("agents", USAGE_FULL)); + return; + } + if (args.includes("--reset")) { + if (args.length > 1) throw new Error(usage); + await saveGlobalReviewers(null); + console.log("Default reviewers reset; repository roles or the built-in reviewer pool apply."); + return; + } + if (args.some(a => a.startsWith("-"))) throw new Error(usage); + const cfg = await loadConfig(); + const known = knownAgents(cfg); + const names = [...new Set(args.flatMap(a => a.split(",")).map(n => n.trim()).filter(Boolean))]; + if (args.length && !names.length) throw new Error(usage); + + if (!names.length && !(process.stdin.isTTY && process.stdout.isTTY)) { + const settings = await readGlobalConfig(); + console.log(`Default reviewers: ${settings.reviewers?.join(", ") ?? "not set (built-in reviewer pool)"}`); + if (settings.reviewers && !cfg.savedReviewers) { + console.log(`Overridden here by reviewer roles in ${path.basename(cfg.configFile)}.`); + } + return; + } + + let chosen = names; + if (!chosen.length) { + const judge = judgeAgent(cfg)?.name; + const found = await Promise.all(known.map(probe)); + const items = known.map((a, i) => ({ + name: a.name, + status: found[i].ok ? "ok" : "MISSING", + notes: [ + ...(a.enabled === false ? ["opt-in"] : []), + ...(a.name === judge ? ["judge — excluded"] : []), + ], + })); + const current = cfg.globalReviewers ?? defaultReviewers(cfg, judge).map(a => a.name); + chosen = await pick(items, current, { title: "Default reviewers" }); + if (!chosen) { + console.log("Cancelled; default reviewers unchanged."); + return; + } + } + + // The whole known set, so an opt-in built-in can be a default reviewer + // without first being enabled in whichever repository is the cwd right now. + const unknown = chosen.filter(n => !known.some(a => a.name === n)); + if (unknown.length) { + throw new Error(`Unknown or disabled reviewer ${unknown.map(n => `"${n}"`).join(", ")}. Choose: ${known.map(a => a.name).join(", ")}`); + } + await saveGlobalReviewers(chosen); + console.log(`Default reviewers: ${chosen.join(", ")} (${globalConfigPath()})`); + const judge = judgeAgent(cfg)?.name; + if (chosen.includes(judge)) console.log(`${judge} is the current judge and is left out of runs it judges.`); + console.log("Repository reviewer roles and --reviewer/--jury override this default."); +} + /** * The repository's default branch, asked of the remote rather than assumed. * diff --git a/docs/configuration.md b/docs/configuration.md index 04ba9d0..370647c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -17,6 +17,11 @@ The example configuration keeps Codex as judge and Claude as reviewer: } ``` +Reviewers used when a run names none resolve in this order: `--reviewer`/`--jury`, any enabled +agent this file gives `"role": "reviewer"`, the default reviewers saved with `jury agents jury` +(`"reviewers"` in `~/.jury/config.json`), then every enabled built-in reviewer. A saved name that +this file disables, or that is no longer an agent, stops the run with an error naming the setting. + The CLI checks executable availability before starting selected reviewers. It cannot preflight provider login, credits, or quota without invoking the provider. Runtime failures stop the loop and remain visible in the saved reports. diff --git a/lib/config.js b/lib/config.js index 8112037..26dcae9 100644 --- a/lib/config.js +++ b/lib/config.js @@ -38,6 +38,10 @@ export async function readGlobalConfig(file = globalConfigPath()) { || (settings.judge !== undefined && (typeof settings.judge !== "string" || !settings.judge.trim()))) { throw new Error("expected an object with a nonempty judge name"); } + if (settings.reviewers !== undefined && (!Array.isArray(settings.reviewers) || !settings.reviewers.length + || settings.reviewers.some(n => typeof n !== "string" || !n.trim()))) { + throw new Error("expected reviewers to be a nonempty list of agent names"); + } return settings; } catch (err) { if (err.code === "ENOENT") return {}; @@ -46,9 +50,19 @@ export async function readGlobalConfig(file = globalConfigPath()) { } export async function saveGlobalJudge(name, file = globalConfigPath()) { + await saveGlobalSetting("judge", name, file); +} + +/** Save the default reviewer list, or remove it with null. */ +export async function saveGlobalReviewers(names, file = globalConfigPath()) { + if (names !== null && !names.length) throw new Error("at least one default reviewer is required"); + await saveGlobalSetting("reviewers", names, file); +} + +async function saveGlobalSetting(key, value, file) { const settings = await readGlobalConfig(file); - if (name === null) delete settings.judge; - else settings.judge = name; + if (value === null) delete settings[key]; + else settings[key] = value; await mkdir(path.dirname(file), { recursive: true }); const temp = `${file}.${randomUUID()}.tmp`; try { @@ -131,10 +145,19 @@ export async function loadConfig(dir = process.cwd(), { globalFile = globalConfi } const main = globalJudge ? selectable.find(a => a.name === globalJudge) ?? null : mains[0] ?? null; + // Saved default reviewers sit below the repository's own roles: a repository + // that names a reviewer explicitly has chosen its jury, and a global setting + // made for some other project must not silently replace it. + const repoReviewers = (user.agents ?? []).some(a => a.role === "reviewer" && a.enabled !== false); + const savedReviewers = !repoReviewers && global.reviewers ? [...global.reviewers] : null; + return { - explicitRoles: Boolean(global.judge || (user.agents ?? []).some(a => + explicitRoles: Boolean(global.judge || global.reviewers || (user.agents ?? []).some(a => Object.hasOwn(a, "role") || Object.hasOwn(a, "enabled"))), globalJudge: global.judge ?? null, + globalReviewers: global.reviewers ?? null, + savedReviewers, + disabledByRepo: [...offByUser], stopToken: user.stopToken ?? DEFAULTS.stopToken, agents, available: selectable, @@ -165,3 +188,31 @@ export function knownAgents(cfg) { export function reviewers(cfg) { return cfg.agents.filter((a) => (a.role ?? "reviewer") === "reviewer"); } + +const SAVED = "saved default reviewers (~/.jury/config.json; change with `jury agents jury`)"; + +/** + * The reviewers a run uses when none are named on the command line. + * + * Saved defaults reach the whole known set, like --jury, so an opt-in built-in + * can be a default. A saved name that no longer resolves is an error naming the + * setting rather than a silent drop: a jury that quietly shrank is a review + * with fewer eyes than the user believes it has. The judge is left out, as it + * never reviews its own work. + */ +export function defaultReviewers(cfg, judge = null) { + if (!cfg.savedReviewers) return reviewers(cfg).filter(a => a.name !== judge); + const byName = new Map(knownAgents(cfg).map(a => [a.name, a])); + const problems = cfg.savedReviewers.filter(name => !byName.has(name)).map(name => + (cfg.disabledByRepo ?? []).includes(name) + ? `"${name}" is disabled in ${path.basename(cfg.configFile)}` + : `"${name}" is not a configured agent`); + if (problems.length) throw new Error(`${SAVED}: ${problems.join("; ")}`); + const pool = cfg.savedReviewers.filter(name => name !== judge).map(name => byName.get(name)); + if (!pool.length) { + throw new Error(`${SAVED} list only "${judge}", which is the selected judge; save another reviewer or pass --reviewer `); + } + return pool; +} + +export const savedReviewersNote = SAVED; diff --git a/lib/help.js b/lib/help.js index 820929d..5591647 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.", + 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.", 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/picker.js b/lib/picker.js new file mode 100644 index 0000000..7ab2f28 --- /dev/null +++ b/lib/picker.js @@ -0,0 +1,75 @@ +// A checkbox picker for `jury agents jury`, on Node's readline alone. +// +// The key handling is a pure function of (state, key) so it can be tested +// without a terminal; `pick` only wires it to raw-mode stdin and redraws. +import readline from "node:readline"; + +/** + * Apply one keypress. Returns the next state; `done` is "save" or "cancel" + * once the picker should close. Saving nothing is refused rather than + * accepted, because an empty default jury is a run that cannot start. + */ +export function pickerKey(state, key = {}) { + const { items, cursor, selected } = state; + const next = { ...state, error: "" }; + if ((key.ctrl && key.name === "c") || key.name === "escape" || key.name === "q") return { ...next, done: "cancel" }; + if (key.name === "up" || key.name === "k") return { ...next, cursor: (cursor - 1 + items.length) % items.length }; + if (key.name === "down" || key.name === "j") return { ...next, cursor: (cursor + 1) % items.length }; + if (key.name === "space") { + const set = new Set(selected); + const name = items[cursor].name; + if (set.has(name)) set.delete(name); else set.add(name); + return { ...next, selected: [...set] }; + } + if (key.name === "return" || key.name === "enter") { + if (!selected.length) return { ...next, error: "Select at least one reviewer." }; + return { ...next, done: "save" }; + } + return next; +} + +export function pickerState(items, selected) { + return { items, cursor: 0, selected: items.map(i => i.name).filter(n => selected.includes(n)), error: "", done: null }; +} + +export function renderPicker(state, title) { + const width = Math.max(...state.items.map(i => i.name.length), 4) + 2; + const lines = [`${title} (space: toggle, enter: save, esc: cancel)`]; + state.items.forEach((item, i) => { + const mark = state.selected.includes(item.name) ? "[x]" : "[ ]"; + const pointer = i === state.cursor ? ">" : " "; + const notes = item.notes?.length ? ` ${item.notes.join(", ")}` : ""; + lines.push(`${pointer} ${mark} ${item.name.padEnd(width)}${item.status.padEnd(8)}${notes}`.trimEnd()); + }); + if (state.error) lines.push(state.error); + return lines; +} + +/** Run the picker on a TTY. Resolves with the chosen names, or null on cancel. */ +export function pick(items, selected, { title, input = process.stdin, output = process.stdout } = {}) { + let state = pickerState(items, selected); + let drawn = 0; + const draw = () => { + if (drawn) output.write(`\x1b[${drawn}A\x1b[0J`); + const lines = renderPicker(state, title); + output.write(lines.join("\n") + "\n"); + drawn = lines.length; + }; + return new Promise(resolve => { + readline.emitKeypressEvents(input); + const wasRaw = input.isRaw; + input.setRawMode?.(true); + input.resume(); + const onKey = (_str, key) => { + state = pickerKey(state, key); + draw(); + if (!state.done) return; + input.off("keypress", onKey); + input.setRawMode?.(wasRaw ?? false); + input.pause(); + resolve(state.done === "save" ? state.selected : null); + }; + input.on("keypress", onKey); + draw(); + }); +} diff --git a/test/default-reviewers.test.js b/test/default-reviewers.test.js new file mode 100644 index 0000000..31fcad2 --- /dev/null +++ b/test/default-reviewers.test.js @@ -0,0 +1,174 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, readFile, writeFile, rm } 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 { + loadConfig, defaultReviewers, readGlobalConfig, saveGlobalJudge, saveGlobalReviewers, +} from "../lib/config.js"; +import { automaticRoles } from "../lib/roles.js"; +import { pickerKey, pickerState, renderPicker } from "../lib/picker.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; +} + +test("saved default reviewers round-trip beside the judge and reject empty or malformed lists", async t => { + const dir = await scratch(t, "jury-reviewers-save-"); + const file = path.join(dir, "config.json"); + await saveGlobalJudge("claude", file); + await saveGlobalReviewers(["droid", "amp"], file); + assert.deepEqual(JSON.parse(await readFile(file, "utf8")), { judge: "claude", reviewers: ["droid", "amp"] }); + await assert.rejects(saveGlobalReviewers([], file), /at least one/); + await saveGlobalReviewers(null, file); + assert.deepEqual(JSON.parse(await readFile(file, "utf8")), { judge: "claude" }); + for (const reviewers of [[], "claude", [""], [3]]) { + await writeFile(file, JSON.stringify({ reviewers })); + await assert.rejects(readGlobalConfig(file), /reviewers to be a nonempty list/); + } +}); + +test("precedence: repository reviewer roles, then saved reviewers, then the built-in pool", async t => { + const dir = await scratch(t, "jury-reviewers-precedence-"); + const globalFile = path.join(dir, "global.json"); + let cfg = await loadConfig(dir, { globalFile }); + const builtin = defaultReviewers(cfg, "codex").map(a => a.name); + assert.ok(builtin.length > 1); + + // Saved: exactly these, opt-in agents included, in the saved order. + await writeFile(globalFile, JSON.stringify({ reviewers: ["amp", "claude"] })); + cfg = await loadConfig(dir, { globalFile }); + assert.deepEqual(defaultReviewers(cfg, "codex").map(a => a.name), ["amp", "claude"]); + // A saved list is an explicit role choice, so automatic assignment stays off. + assert.equal(await automaticRoles(cfg, { check: async () => ({ ok: true }) }), null); + + // The judge is left out; a list of only the judge is an error, not an empty jury. + assert.deepEqual(defaultReviewers(cfg, "claude").map(a => a.name), ["amp"]); + await writeFile(globalFile, JSON.stringify({ reviewers: ["claude"] })); + cfg = await loadConfig(dir, { globalFile }); + assert.throws(() => defaultReviewers(cfg, "claude"), /saved default reviewers.*only "claude", which is the selected judge/); + + // A repository that disables a saved reviewer fails loudly, naming both. + await writeFile(globalFile, JSON.stringify({ reviewers: ["claude", "droid"] })); + await writeFile(path.join(dir, "jury.config.json"), JSON.stringify({ agents: [{ name: "droid", enabled: false }] })); + cfg = await loadConfig(dir, { globalFile }); + assert.throws(() => defaultReviewers(cfg, "codex"), /saved default reviewers.*"droid" is disabled in jury\.config\.json/); + + // A saved name that is no longer an agent at all. + await writeFile(globalFile, JSON.stringify({ reviewers: ["claude", "gone"] })); + await rm(path.join(dir, "jury.config.json")); + cfg = await loadConfig(dir, { globalFile }); + assert.throws(() => defaultReviewers(cfg, "codex"), /"gone" is not a configured agent/); + + // Repository reviewer roles win over the saved list. + await writeFile(globalFile, JSON.stringify({ reviewers: ["amp"] })); + await writeFile(path.join(dir, "jury.config.json"), JSON.stringify({ agents: [{ name: "grok", role: "reviewer" }] })); + cfg = await loadConfig(dir, { globalFile }); + assert.equal(cfg.savedReviewers, null); + assert.deepEqual(cfg.globalReviewers, ["amp"]); + assert.ok(!defaultReviewers(cfg, "codex").some(a => a.name === "amp")); + assert.ok(defaultReviewers(cfg, "codex").some(a => a.name === "grok")); +}); + +test("jury agents jury saves, shows, validates and resets the global default", async t => { + const home = await scratch(t, "jury-reviewers-cli-"); + const cwd = path.join(home, "work"); + await mkdir(cwd); + const env = { ...process.env, HOME: home, USERPROFILE: home }; + const run = (args) => execFileSync(process.execPath, [cli, ...args], { cwd, env, encoding: "utf8" }); + const file = path.join(home, ".jury", "config.json"); + + assert.match(run(["agents", "jury"]), /Default reviewers: not set/); + assert.match(run(["agents", "jury", "claude,amp", "droid"]), /Default reviewers: claude, amp, droid/); + assert.deepEqual(JSON.parse(await readFile(file, "utf8")).reviewers, ["claude", "amp", "droid"]); + assert.match(run(["agents", "jury"]), /Default reviewers: claude, amp, droid/); + + const before = await readFile(file, "utf8"); + for (const args of [["typo"], ["claude,typo"], [","], ["--bogus"], ["--reset", "claude"]]) { + const bad = spawnSync(process.execPath, [cli, "agents", "jury", ...args], { cwd, env, encoding: "utf8" }); + assert.notEqual(bad.status, 0, args.join(" ")); + assert.equal(await readFile(file, "utf8"), before); + } + assert.match(run(["agents", "jury", "--help"]), /jury agents jury/); + + const listing = spawnSync(process.execPath, [cli, "agents"], { cwd, env, encoding: "utf8" }); + assert.match(listing.stdout, /claude .*\(default reviewer\)/); + assert.match(listing.stdout, /Default reviewers: claude, amp, droid/); + assert.doesNotMatch(listing.stdout, /grok .*\(default reviewer\)/); + + assert.match(run(["agents", "jury", "--reset"]), /reset/); + assert.match(run(["agents", "jury"]), /not set/); +}); + +test("a review without --jury uses the saved default reviewers", async t => { + const home = await scratch(t, "jury-reviewers-run-"); + const repo = path.join(home, "repo"); + await mkdir(repo); + const env = { ...process.env, HOME: home, USERPROFILE: home }; + const g = (...a) => execFileSync("git", ["-C", repo, ...a], { env }); + g("init", "-q", "-b", "master"); + g("config", "user.email", "t@example.com"); + g("config", "user.name", "t"); + await writeFile(path.join(repo, "a.txt"), "one\n"); + g("add", "-A"); + g("commit", "-qm", "base"); + g("checkout", "-q", "-b", "feature"); + await writeFile(path.join(repo, "a.txt"), "two\n"); + g("commit", "-qam", "change"); + await mkdir(path.join(home, ".jury")); + await writeFile(path.join(home, ".jury", "config.json"), JSON.stringify({ reviewers: ["droid", "codex"] })); + + const review = (...extra) => spawnSync(process.execPath, [cli, "review", "--dir", repo, "--trunk", "master", + "--rounds", "1", "--web=false", "--dry-run", ...extra], { cwd: repo, env, encoding: "utf8" }); + let result = review(); + assert.equal(result.stderr, ""); + assert.match(result.stdout, /judge\s+codex/); + assert.match(result.stdout, /juries\s+droid(?!,)/); + + result = review("--jury", "claude"); + assert.match(result.stdout, /juries\s+claude/); + + result = review("--judge", "claude"); + assert.match(result.stdout, /juries\s+droid, codex/); +}); + +test("picker keys move, toggle, refuse an empty save, and cancel", () => { + const items = [ + { name: "claude", status: "ok", notes: [] }, + { name: "grok", status: "MISSING", notes: [] }, + { name: "codex", status: "ok", notes: ["judge — excluded"] }, + ]; + let s = pickerState(items, ["claude"]); + assert.deepEqual(s.selected, ["claude"]); + s = pickerKey(s, { name: "up" }); + assert.equal(s.cursor, 2); + s = pickerKey(s, { name: "down" }); + s = pickerKey(s, { name: "j" }); + assert.equal(s.cursor, 1); + s = pickerKey(s, { name: "space" }); + assert.deepEqual(s.selected, ["claude", "grok"]); + s = pickerKey(pickerKey(s, { name: "space" }), { name: "k" }); + s = pickerKey(s, { name: "space" }); + assert.deepEqual(s.selected, []); + s = pickerKey(s, { name: "return" }); + assert.equal(s.done, null); + assert.match(s.error, /at least one/); + assert.ok(renderPicker(s, "Default reviewers").includes("Select at least one reviewer.")); + s = pickerKey(pickerKey(s, { name: "space" }), { name: "return" }); + assert.equal(s.done, "save"); + assert.deepEqual(s.selected, ["claude"]); + + const lines = renderPicker(pickerState(items, ["claude"]), "Default reviewers"); + assert.match(lines[1], /^> \[x\] claude\s+ok$/); + assert.match(lines[2], /\[ \] grok\s+MISSING/); + assert.match(lines[3], /codex\s+ok\s+judge — excluded/); + + for (const key of [{ name: "escape" }, { name: "q" }, { name: "c", ctrl: true }]) { + assert.equal(pickerKey(pickerState(items, []), key).done, "cancel"); + } +}); From 672c9ce37e622633db758cf1299b26f552de13aa Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 15:50:23 +0000 Subject: [PATCH 2/2] fix: write run.json atomically so the console never loses the live run writeRun truncated run.json and rewrote it in place. The loop republishes it throughout a review while the console lists runs, so a read landing between truncate and write saw empty or partial JSON, skipped the run, and the live review vanished from /api/run. That race failed web-launch.test.js on macOS CI (#112, Node 24 / macos-latest): the opened URL named a run missing from the API response. Write to a temp file in the run directory and rename it into place. A stress reproduction went from ~40% missed reads to none; the new runs.test.js case fails without the fix and passes with it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YWwuLax3VBtTJuujgK748c --- lib/store.js | 15 +++++++++++++-- test/runs.test.js | 23 ++++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/store.js b/lib/store.js index a39f2d5..9c3d4f0 100644 --- a/lib/store.js +++ b/lib/store.js @@ -3,7 +3,8 @@ // runs//events.ndjson append-only; two writers are safe (the CLI appends // agent events, the main agent appends verdicts) // runs//run.json the folded view the console reads -import { mkdir, readFile, writeFile, appendFile, readdir, open } from "node:fs/promises"; +import { mkdir, readFile, writeFile, appendFile, readdir, open, rename, rm } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; import path from "node:path"; /** @@ -115,7 +116,17 @@ export async function readArtifact(dir, name) { export async function writeRun(dir, run) { await mkdir(dir, { recursive: true }); - await writeFile(path.join(dir, "run.json"), JSON.stringify(run, null, 2) + "\n", "utf8"); + // Replace, never rewrite in place: run.json is republished while the console + // is reading it, and a reader that lands between truncate and write sees an + // empty file and drops the live run from the list. + const file = path.join(dir, "run.json"); + const temp = `${file}.${randomUUID()}.tmp`; + try { + await writeFile(temp, JSON.stringify(run, null, 2) + "\n", "utf8"); + await rename(temp, file); + } finally { + await rm(temp, { force: true }); + } } export async function readRun(dir) { diff --git a/test/runs.test.js b/test/runs.test.js index 5bf108c..fd7d014 100644 --- a/test/runs.test.js +++ b/test/runs.test.js @@ -9,7 +9,7 @@ import assert from "node:assert/strict"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; -import { appendEvent, slugFor, listRuns, foldEvents, readEvents } from "../lib/store.js"; +import { appendEvent, slugFor, listRuns, foldEvents, readEvents, writeRun } from "../lib/store.js"; import { findingsIn, settledList } from "../lib/findings.js"; import { buildPrompt } from "../lib/prompt.js"; @@ -147,3 +147,24 @@ test("listRuns still ranks by state before recency", async () => { await cleanup(); } }); + +test("a run being republished never drops out of the listing", async (t) => { + // The console lists runs while the loop rewrites run.json; a reader landing + // between truncate and write used to see an empty file and skip the live run. + const base = await mkdtemp(path.join(tmpdir(), "jury-republish-")); + t.after(() => rm(base, { recursive: true, force: true })); + const dir = path.join(base, "live"); + const run = { target: { state: "review", id: "#1" }, + rounds: Array.from({ length: 200 }, (_, n) => ({ n, text: "x".repeat(200) })) }; + await writeRun(dir, run); + let writing = true; + const writer = (async () => { while (writing) await writeRun(dir, run); })(); + let misses = 0; + for (const end = Date.now() + 1000; Date.now() < end;) { + const { runs, skipped } = await listRuns(base); + if (!runs.some((r) => r.slug === "live") || skipped.length) misses++; + } + writing = false; + await writer; + assert.equal(misses, 0); +});