diff --git a/CHANGELOG.md b/CHANGELOG.md index cfabfa7..f8c9e7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +- Scope agent temporary files and default Go caches to owned invocation directories, + reclaim them after completion, and add `jury cache clean` for stopped runs (#122). +- Retain Windows agent caches when descendant termination cannot be confirmed, + including during startup cleanup and `jury cache clean`. +- Preserve cache ownership metadata until cache data is deleted, so interrupted + cleanup remains retryable. +- Record temporary-cache initialization failures as agent errors, allowing other + reviewers to finish and their reports and round status to be saved. - Recognize quoted dispute examples in JSON replies with CRLF line endings, avoiding false reopened findings and preserving the correct rebuttal evidence. - Keep agreed findings settled in mixed replies with explicit dispute IDs, and diff --git a/bin/jury.js b/bin/jury.js index f068a5d..b75ef4f 100755 --- a/bin/jury.js +++ b/bin/jury.js @@ -6,6 +6,7 @@ // deciding whether a finding reproduces is the calling agent's job, and roughly // a third of suggestions do not survive that step. import { commandHelp } from "../lib/help.js"; +import { agentTempRoot, agentTemps } from "../lib/agent-temp.js"; import { parseArgs } from "node:util"; import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; @@ -78,6 +79,7 @@ Other commands jury agents judge set the global default judge jury agents jury set the default reviewers jury runs every PR under review + jury cache [clean] list or reclaim stopped agents' temporary caches jury version jury help --all every command and flag @@ -89,6 +91,7 @@ const USAGE_FULL = `jury — review a pull request with multiple AI reviewers un jury finding list | reproduce | resolve | settled — appends events, enforces the gate jury reply [flags] send each reviewer your verdicts on ITS findings, one conversation each jury runs list every PR under review, with its slug for --run + jury cache [clean] list or reclaim stopped agents' temporary caches 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) @@ -143,6 +146,11 @@ reply flags state commands jury runs [--dir ] jury reply [--dir ] [--run ] + +cache commands + jury cache [list] list managed temporary directories and ownership status + jury cache clean remove only known, stopped invocations +Uses ~/.jury/tmp/agents, or JURY_AGENT_TEMP_ROOT. Active and unrecognized entries stay. `; let [, , cmd, ...rest] = process.argv; @@ -181,6 +189,7 @@ try { case "finding": case "findings": await cmdFinding(rest); break; case "reply": await cmdReply(rest); break; case "runs": await cmdRuns(rest); break; + case "cache": await cmdCache(rest); break; case "agents": await cmdAgents(rest); break; case "version": case "-v": case "--version": console.log(`jury ${VERSION}`); break; case "help": case "-h": case "--help": case undefined: @@ -207,6 +216,17 @@ try { process.exit(1); } +async function cmdCache(args) { + if (args.length > 1 || args.length && !["list", "clean"].includes(args[0])) { + throw new Error("Usage: jury cache [list|clean]"); + } + const root = agentTempRoot(); + const entries = await agentTemps({ root, clean: args[0] === "clean" }); + console.log(`Agent temporary caches: ${root}`); + for (const entry of entries) console.log(`${entry.status}\t${entry.path}`); + if (!entries.length) console.log("No managed temporary directories."); +} + /** Resolve an explicit reviewer list or explain each ineligible name. */ function selectReviewers(cfg, requested, judge = null) { if (!requested) return defaultReviewers(cfg, judge); diff --git a/docs/usage.md b/docs/usage.md index a6d84d5..3ba0644 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -241,3 +241,43 @@ event leaves it unavailable. A forced kill can preserve only bytes already recei Protocol references: [Codex events](https://github.com/openai/codex/blob/main/sdk/typescript/src/events.ts), [Droid result JSON](https://docs.factory.com/droid-exec/overview#json), [OpenCode token normalization](https://github.com/anomalyco/opencode/blob/dev/packages/opencode/src/session/session.ts). + + +## Temporary files and Go caches + +Each reviewer, judge and reply invocation gets an owned directory under +`~/.jury/tmp/agents`. Jury points `TMPDIR`, `TMP` and `TEMP` there and supplies +`GOCACHE` and `GOMODCACHE` defaults inside it. Tools using those variables or their +normal temporary-directory API stay within that invocation's storage. File-based +prompts use the same directory. Jury removes it after success, failure or timeout, +once the agent's process group has stopped. On Windows, a reviewer's exit does not +confirm that its descendants stopped, so launched invocations retain their caches. +Read-only Go module directories are also removed when cleanup is safe. + +An explicit `GOCACHE` or `GOMODCACHE` in the parent environment or agent `env` +configuration takes precedence and remains your responsibility. Set these when +persistent Go caches are preferred over a fresh cache for each invocation. +`JURY_AGENT_TEMP_ROOT` selects another managed root; an agent-specific value is +supported, and the same value must be used when inspecting it from the CLI. +Jury overrides inherited temporary-directory variables for each invocation. +If temporary-cache initialization fails (for example, the root is unwritable), +Jury records an error for that agent and lets other reviewers finish. Their reports +and the failed round status are saved before the review stops. + +Cleanup keeps ownership metadata until cache content is deleted, so interrupted +deletions can be retried. Interrupted invocations are reclaimed when the next +agent starts, or explicitly: + +```sh +jury cache # list directories and ownership status +jury cache clean # reclaim known stopped invocations +``` + +Cleanup preserves live owners and process groups, unknown directories, symlinks, +and launches whose process identity could not be recorded. Windows invocations +remain `uncertain` after their owner exits, including after timeout termination; +startup cleanup and `jury cache clean` preserve them. An uncertain entry requires +manual inspection before removal. Processes that deliberately detach from their +agent's process group are outside this lifecycle. Explicit external caches and arbitrary +`/tmp` or `/private/tmp` directories are never claimed or removed. This does not +retroactively identify old, unmarked caches created before managed storage. diff --git a/lib/agent-temp.js b/lib/agent-temp.js new file mode 100644 index 0000000..138c94c --- /dev/null +++ b/lib/agent-temp.js @@ -0,0 +1,153 @@ +// Only directories created here, with a stopped owner and process group, may +// be reclaimed. Never infer ownership from a Go/cache-like name in /tmp. +import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, rmdir } from "node:fs/promises"; +import { renameSync, writeFileSync } from "node:fs"; +import { homedir } from "node:os"; +import path from "node:path"; + +export function agentTempRoot(env = process.env) { + return path.resolve(env.JURY_AGENT_TEMP_ROOT || path.join(homedir(), ".jury", "tmp", "agents")); +} + +function alive(pid) { + try { process.kill(pid, 0); return true; } + catch (error) { return error.code !== "ESRCH"; } +} + +function childAlive(owner) { + return owner.childPid && alive(owner.processGroup ? -owner.childPid : owner.childPid); +} + +function treeUncertain(owner) { + // Without a process group, an exited reviewer PID cannot establish that its + // descendants stopped. Keep the cache even after the owning Jury exits. + return owner.phase === "running" && !owner.processGroup; +} + +function validOwner(owner) { + return owner?.version === 1 && owner.kind === "codejury-agent-temp" + && Number.isSafeInteger(owner.pid) && owner.pid > 0 + && ["prepared", "launching", "running"].includes(owner.phase) + && typeof owner.processGroup === "boolean" + && (owner.phase === "running" + ? Number.isSafeInteger(owner.childPid) && owner.childPid > 0 + : owner.childPid === null); +} + +async function checkRoot(root, create = false) { + if (create) await mkdir(root, { recursive: true, mode: 0o700 }); + let stat; + try { stat = await lstat(root); } + catch (error) { if (error.code === "ENOENT" && !create) return false; throw error; } + if (!stat.isDirectory() || stat.isSymbolicLink() + || process.getuid && stat.uid !== process.getuid()) { + throw new Error(`Agent temporary root must be an owned directory, not a symlink: ${root}`); + } + return true; +} + +// Go module caches contain read-only directories. Make owned directories +// removable, without following symlinks or changing external cache permissions. +async function removeOwned(dir) { + async function writable(entry) { + let stat; + try { stat = await lstat(entry); } + catch (error) { if (error.code === "ENOENT") return; throw error; } + if (!stat.isDirectory() || stat.isSymbolicLink()) return; + await chmod(entry, stat.mode | 0o700); + for (const name of await readdir(entry)) await writable(path.join(entry, name)); + } + await writable(dir); + let entries; + try { entries = await readdir(dir); } + catch (error) { if (error.code === "ENOENT") return; throw error; } + // Keep the ownership marker until all cache data is gone, so an interrupted + // deletion can still be recognized and retried by startup or cache clean. + for (const name of entries) { + if (name !== "owner.json") await rm(path.join(dir, name), { recursive: true, force: true }); + } + await rm(path.join(dir, "owner.json"), { force: true }); + try { await rmdir(dir); } + catch (error) { if (error.code !== "ENOENT") throw error; } +} + +/** Inventory or reclaim interrupted invocations; unknown and active entries stay. */ +export async function agentTemps({ root = agentTempRoot(), clean = false } = {}) { + root = path.resolve(root); + if (!await checkRoot(root)) return []; + const entries = []; + for (const name of await readdir(root)) { + const dir = path.join(root, name); + let status = "unrecognized"; + try { + const stat = await lstat(dir); + if (/^agent-/.test(name) && stat.isDirectory() && !stat.isSymbolicLink() + && (!process.getuid || stat.uid === process.getuid())) { + const marker = path.join(dir, "owner.json"); + const markerStat = await lstat(marker); + const owner = markerStat.isFile() && !markerStat.isSymbolicLink() + ? JSON.parse(await readFile(marker, "utf8")) : null; + if (validOwner(owner)) { + status = alive(owner.pid) || childAlive(owner) ? "active" + : owner.phase === "launching" || treeUncertain(owner) ? "uncertain" : "stale"; + if (clean && status === "stale") { + await removeOwned(dir); + status = "removed"; + } + } + } + } catch (error) { + // A missing marker is an unknown entry; a missing directory was already + // reclaimed by another cleanup. Do not hide unmarked cache directories. + if (error.code === "ENOENT") { + if (!await lstat(dir).catch(() => null)) continue; + } else if (!(error instanceof SyntaxError)) status = `retained: ${error.message}`; + } + entries.push({ path: dir, status }); + } + return entries; +} + +export async function createAgentTemp(env = process.env) { + const root = agentTempRoot(env); + await checkRoot(root, true); + await agentTemps({ root, clean: true }); + const dir = await mkdtemp(path.join(root, "agent-")); + const temporary = path.join(dir, "tmp"); + const owner = { version: 1, kind: "codejury-agent-temp", pid: process.pid, + childPid: null, processGroup: process.platform !== "win32", phase: "prepared" }; + const save = () => { + writeFileSync(path.join(dir, "owner.next"), JSON.stringify(owner), { mode: 0o600 }); + renameSync(path.join(dir, "owner.next"), path.join(dir, "owner.json")); + }; + try { + save(); + await mkdir(temporary, { mode: 0o700 }); + for (const name of ["go-build", "go-mod"]) await mkdir(path.join(temporary, name)); + } catch (error) { + await removeOwned(dir); + throw error; + } + return { + directory: temporary, + env: { ...env, TMPDIR: temporary, TMP: temporary, TEMP: temporary, + GOCACHE: env.GOCACHE || path.join(temporary, "go-build"), + GOMODCACHE: env.GOMODCACHE || path.join(temporary, "go-mod") }, + launching() { owner.phase = "launching"; save(); }, + spawned(pid) { + owner.childPid = pid || null; + owner.phase = pid ? "running" : "prepared"; + save(); + }, + async dispose() { + if (treeUncertain(owner)) return false; + // Group termination happens in agents.js. OS reaping may lag its result. + for (let n = 0; childAlive(owner) && n < 40; n++) { + await new Promise(resolve => setTimeout(resolve, 25)); + } + if (childAlive(owner)) return false; + await removeOwned(dir); + return true; + }, + }; +} diff --git a/lib/agents.js b/lib/agents.js index ab3537e..b0ac16b 100644 --- a/lib/agents.js +++ b/lib/agents.js @@ -1,11 +1,11 @@ // Spawning reviewers and reading what they said back. import { spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; -import { mkdtemp, writeFile, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; +import { writeFile } from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { trackUsage, interruptUsage } from "./usage.js"; +import { createAgentTemp } from "./agent-temp.js"; /** * Where Code Jury itself is installed, for files that ship with it. @@ -96,7 +96,34 @@ function watchGroup(pid) { * dead reviewer is a result, not a crash, and the round should still report the * others. */ -export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onLog, onChunk, onTimeout, timeoutSeconds, sessionId = null, resumed = false, usageContext }) { +export async function runAgent(agent, options) { + if (options.dryRun) return runAgentWithTemp(agent, options, null); + const started = Date.now(); + let temporary; + try { temporary = await createAgentTemp({ ...process.env, ...agent.env }); } + catch (error) { + return { + agent: agent.name, + ok: false, + seconds: +((Date.now() - started) / 1000).toFixed(1), + verdict: "error", + findings: [], + report: `Could not initialize temporary cache: ${error.message}`, + sessionId: options.sessionId ?? null, + raw: "", + }; + } + try { return await runAgentWithTemp(agent, options, temporary); } + finally { + try { + if (!await temporary.dispose()) options.onLog?.(`Temporary cache retained because agent processes may still be active: ${temporary.directory}`); + } catch (error) { + options.onLog?.(`Could not remove temporary cache ${temporary.directory}: ${error.message}. Run jury cache clean after processes stop.`); + } + } +} + +async function runAgentWithTemp(agent, { worktree, prompt, stopToken, dryRun, onLog, onChunk, onTimeout, timeoutSeconds, sessionId = null, resumed = false, usageContext }, temporary) { const started = Date.now(); if (dryRun) { @@ -113,10 +140,8 @@ export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onL } let promptFile = null; - let tmp = null; if (agent.promptDelivery === "file") { - tmp = await mkdtemp(path.join(tmpdir(), "jury-")); - promptFile = path.join(tmp, "prompt.md"); + promptFile = path.join(temporary.directory, "prompt.md"); await writeFile(promptFile, prompt, "utf8"); } @@ -162,8 +187,9 @@ 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. + temporary.launching(); child = spawn(cmd, args, { - cwd, env: { ...process.env, PWD: cwd, ...agent.env, ...modelEnv(agent) }, + cwd, env: { ...temporary.env, PWD: cwd, ...modelEnv(agent) }, stdio: ["ignore", "pipe", "pipe"], detached: processGroup, }); } catch (err) { @@ -171,6 +197,10 @@ export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onL return; } const unwatch = processGroup ? watchGroup(child.pid) : () => {}; + // A failed metadata write leaves the launch marked uncertain for future + // cleanup. Keep tracking and terminating the child already spawned. + try { temporary.spawned(child.pid); } + catch (error) { onLog?.(`Could not record temporary cache owner: ${error.message}`); } // Pipe reads can split a UTF-8 character. Decode each stream incrementally // so the live callback, saved raw text, and final report retain its bytes. child.stdout.setEncoding("utf8"); @@ -246,8 +276,6 @@ export async function runAgent(agent, { worktree, prompt, stopToken, dryRun, onL }); - if (tmp) await rm(tmp, { recursive: true, force: true }); - const seconds = +((Date.now() - started) / 1000).toFixed(1); // A resumable agent that prints a session id gets it captured here. Without diff --git a/lib/help.js b/lib/help.js index f89541e..7be8bc9 100644 --- a/lib/help.js +++ b/lib/help.js @@ -14,6 +14,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")}\n${language}`, runs: `jury runs [--dir ]\n\nList recorded review runs and their slugs.\n${dir}`, + cache: section("cache commands"), 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/scripts/check-package.mjs b/scripts/check-package.mjs index 9e97f5a..0479748 100644 --- a/scripts/check-package.mjs +++ b/scripts/check-package.mjs @@ -24,7 +24,7 @@ const exec = (label, command, args, options = {}) => { try { const [packed] = JSON.parse(exec('npm pack', npm, ['pack', '--json', '--pack-destination', temporary])); const files = packed.files.map(f => f.path); - for (const required of ['bin/jury.js', 'lib/review-group.js', 'lib/language.js', 'lib/agents/kimi-reviewer.md', 'web/index.html', 'jury.config.example.json', 'LICENSE', 'README.md', 'docs/usage.md', 'docs/console-demo.html']) assert.ok(files.includes(required), `missing ${required}`); + for (const required of ['bin/jury.js', 'lib/review-group.js', 'lib/language.js', 'lib/agent-temp.js', 'lib/agents/kimi-reviewer.md', 'web/index.html', 'jury.config.example.json', 'LICENSE', 'README.md', 'docs/usage.md', 'docs/console-demo.html']) assert.ok(files.includes(required), `missing ${required}`); assert.ok(!files.some(f => /(^|\/)(runs|node_modules|test|\.git)(\/|$)|(^|\/)\.(env|npmrc)$|(^|\/)jury\.config\.json$/.test(f)), 'private/development files in package'); exec('npm install tarball', npm, ['install', '--prefix', temporary, '--ignore-scripts', '--no-audit', '--no-fund', path.join(temporary, packed.filename)]); // Exercise npm-created executable links through PATH, as users invoke them. @@ -42,7 +42,7 @@ try { assert.match(help, /--push=false/); assert.match(exec('installed full help', process.execPath, [installed, 'help', '--all'], { cwd: temporary }), /jury review /); // Full related-PR path: subprocess agents, real local remotes, fixes, resume, failures. - exec('installed CLI regression tests', process.execPath, ['--test', 'test/repository-url.test.js', 'test/route-marker-targets.test.js', 'test/prompt-context.test.js', 'test/language.test.js', 'test/language-replies.test.js', 'test/contested-resume.test.js', 'test/usage.test.js', 'test/models.test.js', 'test/run-shape.test.js', 'test/run-attempt.test.js', 'test/review-head.test.js', 'test/commit-fixes.test.js', 'test/reply-template.test.js', 'test/codex-output.test.js', 'test/cli-commands.test.js', 'test/run-publication.test.js', 'test/global-judge.test.js', 'test/help.test.js', 'test/web-launch.test.js', 'test/web-launch-cleanup.test.js', 'test/review-group.test.js', 'test/reviewer-selection.test.js', 'test/automatic-roles.test.js', 'test/same-agent-roles.test.js', 'test/agy.test.js', 'test/opencode.test.js', 'test/new-agents.test.js', 'test/kimi.test.js', 'test/timeout.test.js', 'test/trunk.test.js'], { + exec('installed CLI regression tests', process.execPath, ['--test', 'test/agent-temp.test.js', 'test/repository-url.test.js', 'test/route-marker-targets.test.js', 'test/prompt-context.test.js', 'test/language.test.js', 'test/language-replies.test.js', 'test/contested-resume.test.js', 'test/usage.test.js', 'test/models.test.js', 'test/run-shape.test.js', 'test/run-attempt.test.js', 'test/review-head.test.js', 'test/commit-fixes.test.js', 'test/reply-template.test.js', 'test/codex-output.test.js', 'test/cli-commands.test.js', 'test/run-publication.test.js', 'test/global-judge.test.js', 'test/help.test.js', 'test/web-launch.test.js', 'test/web-launch-cleanup.test.js', 'test/review-group.test.js', 'test/reviewer-selection.test.js', 'test/automatic-roles.test.js', 'test/same-agent-roles.test.js', 'test/agy.test.js', 'test/opencode.test.js', 'test/new-agents.test.js', 'test/kimi.test.js', 'test/timeout.test.js', 'test/trunk.test.js'], { cwd: root, env: { ...isolated.env, JURY_TEST_CLI: installed }, maxBuffer: 8e6, }); console.log(`Verified installed @agentsdance/codejury@${version}: ${files.length} files, CLI commands/subcommands, executable links, saved console, and related-PR fixes/push/resume/failure tests.`); diff --git a/test/agent-temp.test.js b/test/agent-temp.test.js new file mode 100644 index 0000000..13f6325 --- /dev/null +++ b/test/agent-temp.test.js @@ -0,0 +1,468 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {mkdtemp, mkdir, readFile, readdir, rm, writeFile, chmod, symlink, stat} from 'node:fs/promises'; +import {existsSync} from 'node:fs'; +import {spawn, execFile} from 'node:child_process'; +import {once} from 'node:events'; +import {promisify} from 'node:util'; +import {setTimeout as delay} from 'node:timers/promises'; +import {tmpdir} from 'node:os'; +import path from 'node:path'; +import {fileURLToPath, pathToFileURL} from 'node:url'; +import {testEnvironment} from '../scripts/test-environment.mjs'; + +const cli = process.env.JURY_TEST_CLI ?? new URL('../bin/jury.js', import.meta.url).pathname; +const packageRoot = path.dirname(path.dirname(cli)); +const {runAgent} = await import(pathToFileURL(path.join(packageRoot, 'lib/agents.js'))); +const {agentTemps, createAgentTemp} = await import(pathToFileURL(path.join(packageRoot, 'lib/agent-temp.js'))); +const exec = promisify(execFile); +const options = {worktree: process.cwd(), prompt: 'test', stopToken: 'NO NEW FINDINGS'}; +const agent = (script, env = {}) => ({name: 'fixture', cwd: 'worktree', env, + argv: [process.execPath, '-e', script], report: 'whole'}); + +async function fixture(t) { + const dir = await mkdtemp(path.join(tmpdir(), 'jury-agent-temp-test-')); + t.after(() => rm(dir, {recursive: true, force: true})); + return {dir, root: path.join(dir, 'managed')}; +} + +for (const code of [0, 1]) test(`agent-created temporary caches are removed after exit ${code} only when tree termination is confirmed`, async t => { + const {root} = await fixture(t); + for (let iteration = 0; iteration < 2; iteration++) { + const result = await runAgent(agent(` + const fs = require('node:fs'), os = require('node:os'), path = require('node:path'); + const cache = fs.mkdtempSync(path.join(os.tmpdir(), 'jury-owned-go-cache-')); + fs.writeFileSync(path.join(cache, 'compiled'), 'cache content'); + console.log(JSON.stringify({cache, tmp: os.tmpdir(), build: process.env.GOCACHE, + modules: process.env.GOMODCACHE})); + console.log('NO NEW FINDINGS'); + process.exitCode = ${code}; + `, {JURY_AGENT_TEMP_ROOT: root, GOCACHE: '', GOMODCACHE: ''}), options); + const env = JSON.parse(result.raw.split('\n')[0]); + t.after(() => rm(env.cache, {recursive: true, force: true})); + assert.equal(result.ok, code === 0); + assert.equal(existsSync(env.cache), process.platform === 'win32', 'retain caches when descendant termination cannot be confirmed'); + assert.ok(env.tmp.startsWith(root + path.sep)); + assert.ok(env.build.startsWith(root + path.sep)); + assert.ok(env.modules.startsWith(root + path.sep)); + assert.equal((await readdir(root)).length, process.platform === 'win32' ? iteration + 1 : 0); + } +}); + +for (const inherited of ['GOCACHE', 'GOMODCACHE', 'both']) test(`default-cache fixtures ignore inherited ${inherited}`, async t => { + const {dir} = await fixture(t); + const build = path.join(dir, 'external-build'), modules = path.join(dir, 'external-modules'); + await mkdir(build); await mkdir(modules); + await writeFile(path.join(build, 'keep'), 'build'); + await writeFile(path.join(modules, 'keep'), 'modules'); + const env = {...process.env, + GOCACHE: inherited === 'GOMODCACHE' ? '' : build, + GOMODCACHE: inherited === 'GOCACHE' ? '' : modules}; + delete env.NODE_TEST_CONTEXT; // Run the selected fixtures in a fresh test coordinator. + const {stdout} = await exec(process.execPath, ['--test', '--test-reporter=tap', + '--test-name-pattern=^agent-created temporary caches', fileURLToPath(import.meta.url)], {env}) + .catch(error => assert.fail(error.stdout || error.message)); + assert.match(stdout, /# pass 2\b/); + assert.equal(await readFile(path.join(build, 'keep'), 'utf8'), 'build'); + assert.equal(await readFile(path.join(modules, 'keep'), 'utf8'), 'modules'); +}); + +test('explicit Go caches survive cleanup, including read-only module directories', async t => { + const {dir, root} = await fixture(t); + const build = path.join(dir, 'shared-build'), modules = path.join(dir, 'shared-modules'); + await mkdir(build); await mkdir(modules); + const result = await runAgent(agent(` + const fs = require('node:fs'), path = require('node:path'); + for (const key of ['GOCACHE', 'GOMODCACHE']) fs.writeFileSync(path.join(process.env[key], 'keep'), key); + const local = path.join(process.env.TMPDIR, 'readonly'); + fs.mkdirSync(local); fs.writeFileSync(path.join(local, 'module'), 'data'); fs.chmodSync(local, 0o555); + fs.symlinkSync(process.env.GOMODCACHE, path.join(process.env.TMPDIR, 'outside'), 'dir'); + console.log('NO NEW FINDINGS'); + `, {JURY_AGENT_TEMP_ROOT: root, GOCACHE: build, GOMODCACHE: modules}), options); + assert.equal(result.ok, true); + assert.equal(await readFile(path.join(build, 'keep'), 'utf8'), 'GOCACHE'); + assert.equal(await readFile(path.join(modules, 'keep'), 'utf8'), 'GOMODCACHE'); + assert.equal((await readdir(root)).length, process.platform === 'win32' ? 1 : 0); +}); + +test('file-delivered prompts follow cache cleanup and failed launches add no temporary directories', async t => { + const {root} = await fixture(t); + const result = await runAgent({name: 'file-fixture', cwd: 'worktree', report: 'whole', + promptDelivery: 'file', env: {JURY_AGENT_TEMP_ROOT: root}, + argv: [process.execPath, '-e', `console.log(require('node:fs').readFileSync(process.argv[1], 'utf8')); console.log('NO NEW FINDINGS')`, '{{promptFile}}'], + }, options); + assert.equal(result.ok, true); assert.match(result.raw, /^test\n/); + assert.equal((await readdir(root)).length, process.platform === 'win32' ? 1 : 0); + const failed = await runAgent({...agent('', {JURY_AGENT_TEMP_ROOT: root}), argv: ['/jury-missing-agent']}, options); + assert.equal(failed.ok, false); + assert.equal((await readdir(root)).length, process.platform === 'win32' ? 1 : 0); +}); + +test('cache initialization failures return agent errors and preserve known sessions', async t => { + const {dir, root} = await fixture(t); + await writeFile(root, 'not a directory'); + const launched = path.join(dir, 'launched'); + for (const sessionId of [null, 'existing-session']) { + const result = await runAgent(agent(`require('node:fs').writeFileSync(${JSON.stringify(launched)}, 'unexpected')`, + {JURY_AGENT_TEMP_ROOT: root}), {...options, sessionId}); + assert.equal(result.agent, 'fixture'); + assert.equal(result.ok, false); + assert.equal(result.verdict, 'error'); + assert.match(result.report, /Could not initialize temporary cache:/); + assert.ok(result.report.includes(root)); + assert.equal(result.sessionId, sessionId); + assert.deepEqual(result.findings, []); + assert.equal(result.raw, ''); + assert.ok(Number.isFinite(result.seconds) && result.seconds >= 0); + assert.equal(existsSync(launched), false); + } +}); + +test('cache initialization failure records an error while a running peer finishes', { + skip: process.platform === 'win32' || process.getuid?.() === 0, timeout: 20000, +}, async t => { + const {dir, root} = await fixture(t); + const repo = path.join(dir, 'repo'), denied = path.join(dir, 'denied'); + const ready = path.join(dir, 'peer-ready'), finished = path.join(dir, 'peer-finished'); + const launched = path.join(dir, 'blocked-launched'); + await mkdir(repo); await mkdir(denied); await chmod(denied, 0o555); + const isolated = testEnvironment(); + t.after(() => isolated.dispose()); + try { + const git = (...args) => exec('git', args, {cwd: repo, env: isolated.env}); + await git('init', '-q', '-b', 'master'); + await git('commit', '--allow-empty', '-qm', 'base'); + await git('update-ref', 'refs/remotes/origin/master', 'HEAD'); + await writeFile(path.join(repo, 'jury.config.json'), JSON.stringify({agents: [ + {...agent(`require('node:fs').writeFileSync(${JSON.stringify(launched)}, 'unexpected')`, + {JURY_AGENT_TEMP_ROOT: denied}), name: 'blocked', role: 'reviewer'}, + {...agent(` + const fs = require('node:fs'); + fs.writeFileSync(${JSON.stringify(ready)}, 'ready'); + setTimeout(() => { + fs.writeFileSync(${JSON.stringify(finished)}, 'complete'); + console.log('NO NEW FINDINGS'); + }, 300); + `, {JURY_AGENT_TEMP_ROOT: root}), name: 'peer', role: 'reviewer'}, + {...agent(`throw Error('judge must not run')`), name: 'fixture-judge', role: 'main'}, + ]})); + const gate = path.join(dir, 'wait-for-peer.mjs'); + await writeFile(gate, ` + import fs from 'node:fs/promises'; + import {existsSync} from 'node:fs'; + import {syncBuiltinESMExports} from 'node:module'; + import {setTimeout as delay} from 'node:timers/promises'; + const mkdtemp = fs.mkdtemp; + fs.mkdtemp = async (...args) => { + if (String(args[0]) === ${JSON.stringify(path.join(denied, 'agent-'))}) { + const deadline = Date.now() + 10000; + while (!existsSync(${JSON.stringify(ready)}) && Date.now() < deadline) await delay(10); + } + return mkdtemp(...args); // The real filesystem rejects the unwritable root. + }; + syncBuiltinESMExports(); + `); + const result = await exec(process.execPath, ['--import', pathToFileURL(gate).href, cli, 'review', + '--dir', repo, '--trunk', 'master', '--agents', 'blocked,peer', '--judge', 'fixture-judge', + '--rounds', '1', '--web=false', '--push=false'], {cwd: repo, env: isolated.env, timeout: 15000}) + .catch(error => { if (error.killed) throw error; return error; }); + assert.equal(result.code, 1, 'an incomplete review must still exit unsuccessfully'); + assert.equal(existsSync(ready), true, `the peer must be running before initialization fails\n${result.stdout}\n${result.stderr}`); + const [slug] = await readdir(path.join(repo, 'runs')); + const runDir = path.join(repo, 'runs', slug); + const saved = JSON.parse(await readFile(path.join(runDir, 'run.json'), 'utf8')); + assert.equal(saved.target.state, 'human', result.stderr); + assert.match(saved.target.stateNote, /reviewers failed: blocked/); + assert.equal(existsSync(finished), true, 'the healthy peer must finish its work'); + assert.equal(existsSync(launched), false, 'the reviewer with no cache must not launch'); + const events = (await readFile(path.join(runDir, 'events.ndjson'), 'utf8')).trim().split('\n').map(JSON.parse); + const reports = events.filter(event => event.t === 'agent.report'); + assert.equal(reports.length, 2); + assert.equal(reports.find(report => report.agent === 'peer').verdict, 'clean'); + const failed = reports.find(report => report.agent === 'blocked'); + assert.equal(failed.verdict, 'error'); + assert.match(failed.report, /EACCES/); + assert.ok(events.some(event => event.t === 'round.end')); + } finally { await chmod(denied, 0o755); } +}); + +test('timed-out agents release their caches after process termination', {skip: process.platform === 'win32'}, async t => { + const {root} = await fixture(t); + const result = await runAgent(agent(`console.log(process.env.TMPDIR); setInterval(() => {}, 1000)`, + {JURY_AGENT_TEMP_ROOT: root}), {...options, timeoutSeconds: 0.3}); + assert.equal(result.timedOut, true); + assert.deepEqual(await readdir(root), []); +}); + +async function waitFor(predicate) { + for (let i = 0; i < 500; i++) { if (await predicate()) return; await delay(10); } + assert.fail('fixture did not reach expected process state'); +} + +function running(pid) { + try { process.kill(pid, 0); return true; } catch { return false; } +} + +for (const recovery of ['cache clean', 'startup']) test(`interrupted cache deletion remains reclaimable by ${recovery}`, async t => { + const {dir, root} = await fixture(t); + const paused = path.join(dir, 'paused.json'); + const child = spawn(process.execPath, ['--input-type=module', '-e', ` + import fs from 'node:fs'; + import path from 'node:path'; + import {createAgentTemp} from ${JSON.stringify(pathToFileURL(path.join(packageRoot, 'lib/agent-temp.js')).href)}; + const temp = await createAgentTemp({JURY_AGENT_TEMP_ROOT: ${JSON.stringify(root)}}); + const owned = path.dirname(temp.directory); + const marker = path.join(owned, 'owner.json'); + const cache = path.join(temp.directory, 'go-build', 'compiled'); + fs.writeFileSync(cache, 'unfinished cache deletion'); + + // Hold one real unlink while the other recursive-rm operations complete. + // If rm includes the container, wait for its real marker unlink before + // reporting the interruption point; content-only deletion keeps the marker. + let removingContainer = false, markerRemoved = false, blocked = false; + const report = () => { + if (blocked && (!removingContainer || markerRemoved)) { + fs.writeFileSync(${JSON.stringify(paused + '.next')}, JSON.stringify({owned, cache})); + fs.renameSync(${JSON.stringify(paused + '.next')}, ${JSON.stringify(paused)}); + } + }; + const rmdir = fs.rmdir, unlink = fs.unlink; + fs.rmdir = (target, ...args) => { + if (String(target) === owned) removingContainer = true; + return rmdir(target, ...args); + }; + fs.unlink = (target, callback) => { + if (String(target) === cache) { blocked = true; report(); return; } + return unlink(target, error => { + if (!error && String(target) === marker) { markerRemoved = true; report(); } + callback(error); + }); + }; + setInterval(() => {}, 1000); + await temp.dispose(); + `], {stdio: ['ignore', 'ignore', 'pipe']}); + const exited = once(child, 'exit'); + let stderr = ''; + child.stderr.on('data', chunk => { stderr += chunk; }); + try { + await waitFor(() => existsSync(paused) || child.exitCode !== null); + assert.equal(existsSync(paused), true, stderr || 'cleanup must reach the interrupted unlink'); + child.kill('SIGKILL'); await exited; + const {owned, cache} = JSON.parse(await readFile(paused, 'utf8')); + assert.equal(await readFile(cache, 'utf8'), 'unfinished cache deletion'); + assert.equal((await agentTemps({root}))[0].status, 'stale', 'interruption must not orphan the remaining cache as unrecognized'); + assert.equal(existsSync(path.join(owned, 'owner.json')), true, 'ownership must survive until cache deletion completes'); + if (recovery === 'cache clean') { + const result = await exec(process.execPath, [cli, 'cache', 'clean'], {env: {...process.env, JURY_AGENT_TEMP_ROOT: root}}); + assert.match(result.stdout, /removed/); + } else { + const next = await createAgentTemp({JURY_AGENT_TEMP_ROOT: root}); + await next.dispose(); + } + assert.deepEqual(await readdir(root), []); + } finally { + child.kill('SIGKILL'); await exited; + } +}); + +for (const code of [0, 1]) test(`Windows cleanup retains caches used by descendants after reviewer exit ${code}`, async t => { + const {dir, root} = await fixture(t); + const ready = path.join(dir, 'ready.json'), release = path.join(dir, 'release'); + const outcome = path.join(dir, 'outcome'); + const background = ` + const fs = require('node:fs'), path = require('node:path'); + fs.writeFileSync(${JSON.stringify(ready)}, JSON.stringify({pid: process.pid, cache: process.env.GOCACHE})); + process.send('ready'); + const poll = setInterval(() => { + if (!fs.existsSync(${JSON.stringify(release)})) return; + clearInterval(poll); + let result = 'compiled'; + try { fs.writeFileSync(path.join(process.env.GOCACHE, 'compiled'), 'cache content'); } + catch (error) { result = error.code; } + fs.writeFileSync(${JSON.stringify(outcome + '.next')}, result); + fs.renameSync(${JSON.stringify(outcome + '.next')}, ${JSON.stringify(outcome)}); + }, 10); + setTimeout(() => process.exit(), 30000); + `; + const definition = agent(` + const child = require('node:child_process').spawn(process.execPath, ['-e', ${JSON.stringify(background)}], + {stdio: ['ignore', 'ignore', 'ignore', 'ipc']}); + child.once('message', () => { + console.log('pid=' + process.pid); + console.log('NO NEW FINDINGS'); + child.disconnect(); child.unref(); process.exitCode = ${code}; + }); + `, {JURY_AGENT_TEMP_ROOT: root, GOCACHE: '', GOMODCACHE: ''}); + try { + const {stdout} = await exec(process.execPath, ['--input-type=module', '-e', ` + // Select the Windows lifecycle on POSIX hosts, keeping real processes and IO. + Object.defineProperty(process, 'platform', {value: 'win32'}); + const {runAgent} = await import(${JSON.stringify(pathToFileURL(path.join(packageRoot, 'lib/agents.js')).href)}); + const logs = []; + const result = await runAgent(${JSON.stringify(definition)}, + {...${JSON.stringify(options)}, onLog: message => logs.push(message)}); + console.log(JSON.stringify({result, logs})); + `], {timeout: 10000}); + const {result, logs} = JSON.parse(stdout); + const {pid, cache} = JSON.parse(await readFile(ready, 'utf8')); + assert.equal(result.ok, code === 0); + assert.equal(result.timedOut, undefined); + assert.equal(running(Number(result.raw.match(/pid=(\d+)/)[1])), false, 'the reviewer has exited'); + assert.equal(running(pid), true, 'its ordinary background descendant is still running'); + await writeFile(release, ''); + await waitFor(() => existsSync(outcome)); + assert.equal(await readFile(outcome, 'utf8'), 'compiled', 'cleanup must not break a live descendant\'s cache writes'); + assert.equal(await readFile(path.join(cache, 'compiled'), 'utf8'), 'cache content'); + assert.ok(logs.some(message => message.includes('Temporary cache retained'))); + + const clean = await exec(process.execPath, [cli, 'cache', 'clean'], {env: {...process.env, JURY_AGENT_TEMP_ROOT: root}}); + assert.match(clean.stdout, /uncertain/); + const next = await createAgentTemp({JURY_AGENT_TEMP_ROOT: root}); + await next.dispose(); + assert.equal(existsSync(cache), true, 'explicit and startup cleanup must also retain an unverified Windows tree'); + } finally { + if (existsSync(ready)) { + const {pid} = JSON.parse(await readFile(ready, 'utf8')); + try { process.kill(pid, 'SIGKILL'); } catch {} + await waitFor(() => !running(pid)); + } + } +}); + +for (const signal of ['SIGINT', 'SIGKILL']) test(`cleanup reclaims interrupted ${signal} runs only after their process groups stop`, + {skip: process.platform === 'win32'}, async t => { + const {root} = await fixture(t); + const definition = agent(`console.log('ready=' + process.pid); setInterval(() => {}, 1000)`, {JURY_AGENT_TEMP_ROOT: root}); + const child = spawn(process.execPath, ['--input-type=module', '-e', ` + import {runAgent} from ${JSON.stringify(pathToFileURL(path.join(packageRoot, 'lib/agents.js')).href)}; + await runAgent(${JSON.stringify(definition)}, {...${JSON.stringify(options)}, onChunk: s => process.stdout.write(s)}); + `], {stdio: ['ignore', 'pipe', 'pipe']}); + let text = '', pid; + child.stdout.on('data', chunk => { text += chunk; pid = Number(text.match(/ready=(\d+)/)?.[1]); }); + t.after(() => { child.kill('SIGKILL'); if (pid) { try { process.kill(-pid, 'SIGKILL'); } catch {} } }); + await waitFor(() => pid); + assert.equal((await agentTemps({root, clean: true}))[0].status, 'active'); + const exited = once(child, 'exit'); + child.kill(signal); await exited; + if (signal === 'SIGKILL') { + assert.equal((await agentTemps({root, clean: true}))[0].status, 'active', 'orphaned running group must retain its cache'); + process.kill(-pid, 'SIGKILL'); + } + await waitFor(() => !running(-pid)); + const listed = await exec(process.execPath, [cli, 'cache'], {env: {...process.env, JURY_AGENT_TEMP_ROOT: root}}); + assert.match(listed.stdout, /stale/); + assert.equal((await readdir(root)).length, 1, 'listing never deletes'); + if (signal === 'SIGINT') { + const clean = await exec(process.execPath, [cli, 'cache', 'clean'], {env: {...process.env, JURY_AGENT_TEMP_ROOT: root}}); + assert.match(clean.stdout, /removed/); + } else { + await runAgent(agent(`console.log('NO NEW FINDINGS')`, {JURY_AGENT_TEMP_ROOT: root}), options); + } + assert.deepEqual(await readdir(root), []); + }); + +test('concurrent agents keep their own caches while a peer completes', async t => { + const {dir, root} = await fixture(t); + const ready = path.join(dir, 'ready'), release = path.join(dir, 'release'); + const pending = runAgent(agent(` + const fs = require('node:fs'); + fs.writeFileSync(${JSON.stringify(ready + '.next')}, process.env.TMPDIR); + fs.renameSync(${JSON.stringify(ready + '.next')}, ${JSON.stringify(ready)}); + const timer = setInterval(() => { if (fs.existsSync(${JSON.stringify(release)})) { clearInterval(timer); console.log('NO NEW FINDINGS'); } }, 10); + `, {JURY_AGENT_TEMP_ROOT: root}), {...options, timeoutSeconds: 10}); + try { + await waitFor(() => existsSync(ready)); + const active = await readFile(ready, 'utf8'); + await runAgent(agent(`console.log('NO NEW FINDINGS')`, {JURY_AGENT_TEMP_ROOT: root}), options); + assert.equal(existsSync(active), true); + assert.equal((await agentTemps({root, clean: true}))[0].status, 'active'); + } finally { await writeFile(release, ''); await pending; } + assert.equal((await readdir(root)).length, process.platform === 'win32' ? 2 : 0); +}); + +for (const [signal, pattern, count] of [ + ['ready', '^concurrent agents keep their own caches', 1], + ['outcome', '^Windows cleanup retains caches used by descendants', 2], +]) test(`cache fixtures publish ${signal} only after writing completes`, async t => { + const {dir} = await fixture(t); + const preload = path.join(dir, 'pause-signal-write.cjs'), observed = path.join(dir, 'observed'); + await writeFile(preload, ` + const fs = require('node:fs'), fsp = require('node:fs/promises'), path = require('node:path'); + const write = fs.writeFileSync, exists = fs.existsSync, read = fsp.readFile; + const signal = file => typeof file === 'string' && path.basename(file) === ${JSON.stringify(signal)}; + const wait = file => { + const deadline = Date.now() + 5000; + while (!exists(file)) { + if (Date.now() >= deadline) throw Error('Signal handshake timed out: ' + file); + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10); + } + }; + const release = file => { + if (!exists(file + '.observed')) { + fs.appendFileSync(${JSON.stringify(observed)}, 'observed\\n'); + write(file + '.observed', ''); + } + }; + // Pause after the real open creates an empty file, before any data is written. + // Exercise both direct writes and writes staged for an atomic rename. + fs.writeFileSync = (file, ...args) => { + const published = typeof file === 'string' ? file.replace(/\\.next$/, '') : file; + if (!signal(published)) return write(file, ...args); + const fd = fs.openSync(file, 'w'); + try { + write(published + '.opened', ''); + wait(published + '.observed'); + return write(fd, ...args); + } finally { fs.closeSync(fd); } + }; + fs.existsSync = file => { + const writing = signal(file) && exists(file + '.opened'); + const found = exists(file); + if (writing && !found) release(file); + return found; + }; + fsp.readFile = async (file, ...args) => { + const contents = await read(file, ...args); + if (signal(file)) { wait(file + '.opened'); release(file); } + return contents; + }; + require('node:module').syncBuiltinESMExports(); + `); + const env = {...process.env, NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --require=${JSON.stringify(preload)}`}; + delete env.NODE_TEST_CONTEXT; + const {stdout} = await exec(process.execPath, ['--test', '--test-reporter=tap', + '--test-name-pattern=' + pattern, fileURLToPath(import.meta.url)], {env, timeout: 20000}) + .catch(error => assert.fail(error.stdout || error.message)); + assert.match(stdout, new RegExp('# pass ' + count + '\\b')); + assert.equal((await readFile(observed, 'utf8')).trim().split('\n').length, count, + 'each fixture must probe its signal while the writer is paused'); +}); + +test('cleanup retains unrecognized entries and uncertain launches and never follows symlinks', async t => { + const {dir, root} = await fixture(t); + const temp = await createAgentTemp({JURY_AGENT_TEMP_ROOT: root}); + const owned = path.dirname(temp.directory); + const marker = JSON.parse(await readFile(path.join(owned, 'owner.json'), 'utf8')); + const child = spawn(process.execPath, ['-e', '']); const pid = child.pid; await once(child, 'exit'); + await writeFile(path.join(owned, 'owner.json'), JSON.stringify({...marker, pid, phase: 'launching'})); + const outside = path.join(dir, 'outside'); await mkdir(outside); + await writeFile(path.join(outside, 'keep'), 'untouched'); await chmod(outside, 0o555); + await symlink(outside, path.join(root, 'agent-link'), 'dir'); + await mkdir(path.join(root, 'agent-unknown')); + await mkdir(path.join(root, 'agent-bad-marker')); + await writeFile(path.join(root, 'agent-bad-marker', 'owner.json'), '{broken'); + const entries = await agentTemps({root, clean: true}); + assert.equal(entries.find(e => e.path === owned).status, 'uncertain'); + assert.equal(entries.filter(e => e.status === 'unrecognized').length, 3); + assert.equal(await readFile(path.join(outside, 'keep'), 'utf8'), 'untouched'); + if (process.platform !== 'win32') assert.equal((await stat(outside)).mode & 0o777, 0o555); + await chmod(outside, 0o755); + await temp.dispose(); +}); + +test('temporary roots cannot be symlinks', async t => { + const {dir, root} = await fixture(t); + const outside = path.join(dir, 'outside'); await mkdir(outside); await symlink(outside, root, 'dir'); + await assert.rejects(() => createAgentTemp({JURY_AGENT_TEMP_ROOT: root}), /not a symlink/); + await assert.rejects(() => agentTemps({root, clean: true}), /not a symlink/); +});