diff --git a/AGENTS.md b/AGENTS.md index 75f29e9..a45eab7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -42,8 +42,10 @@ Fusion surfaces are agent prompts, not a second runtime command. Claude Code use - **Model**: pinned to `swe-2-max` (SWE-2 Max). Override per call with `--model `; see `devin models list`. - **Reviews are read-only**: `review` and `adversarial-review` never modify - files. They run `devin -p` in `autonomous` mode with `--sandbox`, falling - back to `normal` mode where sandboxing is unavailable. + files. They run `devin -p` in `autonomous` mode with `--sandbox` and fail + closed: if the sandbox is unavailable (native Windows, or Linux without + `bwrap` and `socat`) the job is marked failed with install guidance and is + never retried without the sandbox. - **`task` is write-capable** (`accept-edits`) unless `--read-only` is passed. `--permission-mode dangerous`/`yolo`/`bypass` exist but should only be used when the caller explicitly asks for full auto-approval. diff --git a/README.md b/README.md index 2a44c1d..93d51aa 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ devin plugins install TheOnlyFusionCube/devin-plugin-cc#plugins/devin | `/devin:result [job-id]` | Show the complete stored output of a finished job. | | `/devin:cancel ` | Kill a running local job's process tree; for cloud handoffs also asks the session to stop. | -Review commands are strictly read-only: they never apply fixes. Arguments are parsed POSIX-style — flags first, then free text passed through verbatim. +Review commands are strictly read-only: they never apply fixes. They run inside Devin's OS-level sandbox and fail closed: if the sandbox cannot start, the job fails with platform-specific guidance and is never retried without it. macOS (Seatbelt) works out of the box, Linux needs `bubblewrap` (`bwrap`) and `socat`, and native Windows is not supported (use WSL2). See the [Devin sandbox docs](https://docs.devin.ai/cli/sandbox). Arguments are parsed POSIX-style: flags first, then free text passed through verbatim. The default model is pinned to **`swe-2-max`** (SWE-2 Max); pass `--model ` to any review/task command to override — `devin models list` shows what your account can use. @@ -130,7 +130,7 @@ In Codex, install the fusion skill (via `codex plugin add fusion@devin-plugin-cc - **SessionStart** — exports session metadata for job attribution. - **SessionEnd** — kills local jobs still running from this session. Detached cloud handoffs are left alone and reconciled on the next `status`/`result` call. -- **Stop (opt-in)** — when enabled via `/devin:setup --enable-review-gate`, reviews the previous turn's diff and blocks session stop only on a `BLOCK:` verdict. Infrastructure failures fail open with a warning. +- **Stop (opt-in)**: when enabled via `/devin:setup --enable-review-gate`, reviews the previous turn's diff and blocks session stop only on a `BLOCK:` verdict. Infrastructure failures, including an unavailable sandbox, fail open with a warning; the gate never reruns the review without the sandbox. ## State diff --git a/plugins/devin/scripts/devin-companion.mjs b/plugins/devin/scripts/devin-companion.mjs index 17a21e6..c41d589 100755 --- a/plugins/devin/scripts/devin-companion.mjs +++ b/plugins/devin/scripts/devin-companion.mjs @@ -25,7 +25,8 @@ import { getDevinAuthStatus, getDevinAvailability, normalizePermissionMode, - runDevinPrint + runDevinPrint, + sandboxFailureHint } from "./lib/devin.mjs"; import { readStdinIfPiped } from "./lib/fs.mjs"; import { @@ -76,7 +77,6 @@ const STATUS_POLL_INTERVAL_MS = 2000; const HANDOFF_POLL_INTERVAL_MS = 30000; const MAX_INLINE_REVIEW_FILES = 20; const MAX_INLINE_REVIEW_DIFF_BYTES = 512 * 1024; -const SANDBOX_FAILURE_PATTERN = /sandbox|seatbelt|bubblewrap|bwrap|seccomp/i; function printUsage() { console.log( @@ -169,6 +169,27 @@ async function cmdSetup(cwd, argv) { outputResult(report, renderSetupReport(report), options.json); } +function renderDevinOutput({ rawOutput, result, sandboxHint }) { + const base = rawOutput ? `${rawOutput}\n` : `Devin produced no output (exit ${result.exitStatus}).\n${devinFailureMessage(result)}\n`; + if (!sandboxHint) { + return base; + } + // Keep Devin's own stderr lines next to the hint even when stdout had content. + return `${base}${rawOutput ? `${devinFailureMessage(result)}\n` : ""}\n${sandboxHint}\n`; +} + +// A sandbox failure is an error, not a normal completion: exit non-zero. With --json +// stdout is machine output, so the hint also goes to stderr for hooks and callers. +function reportSandboxFailure(execution, asJson) { + if (!execution.sandboxHint) { + return; + } + process.exitCode = 1; + if (asJson) { + process.stderr.write(`${execution.sandboxHint}\n`); + } +} + async function runDevinJob({ cwd, kind, prompt, promptIsFile = false, model = null, permissionMode = null, sandbox = false, continueLast = false, resumeSessionId = null, title = null, meta = {} }) { const jobsDir = resolveJobsDir(cwd); fs.mkdirSync(jobsDir, { recursive: true }); @@ -209,29 +230,15 @@ async function runDevinJob({ cwd, kind, prompt, promptIsFile = false, model = nu progress?.(`Running devin (${permissionMode ?? "default"} mode${sandbox ? ", sandbox" : ""})…`); // job.pid is the devin child's pid (a process-group leader), so cancel can kill the tree. - let result = await runDevinPrint({ + const result = await runDevinPrint({ cwd, args, onSpawn: (pid) => patchJob?.({ pid }), onStderr: (text) => progress?.(text.trim().split("\n").slice(-1)[0]) }); - // Sandbox may be unavailable on this platform; fall back to a plain read-only-ish run. - if (sandbox && result.exitStatus !== 0 && SANDBOX_FAILURE_PATTERN.test(`${result.stderr}\n${result.stdout}`)) { - progress?.("Sandbox unavailable — retrying without it."); - const retryArgs = buildDevinPrintArgs({ - promptFile, - model: effectiveModel, - permissionMode: "normal", - continueLast, - resumeSessionId - }); - result = await runDevinPrint({ - cwd, - args: retryArgs, - onSpawn: (pid) => patchJob?.({ pid }) - }); - } + // A sandboxed run that fails stays failed: never retry without the sandbox. + const sandboxHint = sandbox && result.exitStatus !== 0 ? sandboxFailureHint({ stderr: result.stderr }) : null; const rawOutput = result.stdout.trim(); const devinSession = findLatestDevinSession(cwd, { createdAfterMs: startedMs - 60_000 }); @@ -242,9 +249,10 @@ async function runDevinJob({ cwd, kind, prompt, promptIsFile = false, model = nu stderr: result.stderr.trim() || null, exitStatus: result.exitStatus }, - rendered: rawOutput ? `${rawOutput}\n` : `Devin produced no output (exit ${result.exitStatus}).\n${devinFailureMessage(result)}\n`, + rendered: renderDevinOutput({ rawOutput, result, sandboxHint }), summary: rawOutput.split("\n").find((line) => line.trim())?.slice(0, 140) ?? null, - devinSessionId: devinSession?.id ?? devinSession?.short_id ?? null + devinSessionId: devinSession?.id ?? devinSession?.short_id ?? null, + sandboxHint }; }, { logFile }); @@ -294,6 +302,7 @@ async function cmdReview(cwd, argv, kind) { rendered: execution.rendered }; outputResult(payload, execution.rendered, options.json); + reportSandboxFailure(execution, options.json); } async function cmdTask(cwd, argv) { @@ -341,6 +350,7 @@ async function cmdTask(cwd, argv) { rendered: execution.rendered }; outputResult(payload, execution.rendered, options.json); + reportSandboxFailure(execution, options.json); } async function cmdResumeCandidate(cwd, argv) { diff --git a/plugins/devin/scripts/lib/devin.mjs b/plugins/devin/scripts/lib/devin.mjs index d913b06..3454a6f 100644 --- a/plugins/devin/scripts/lib/devin.mjs +++ b/plugins/devin/scripts/lib/devin.mjs @@ -7,6 +7,9 @@ import { binaryAvailable, formatCommandFailure, runCommand } from "./process.mjs export const DEVIN_BINARY = process.env.DEVIN_COMPANION_DEVIN_BINARY ?? "devin"; export const SESSION_ID_ENV = "DEVIN_COMPANION_SESSION_ID"; export const DEFAULT_MODEL = "swe-2-max"; +export const SANDBOX_DOCS_URL = "https://docs.devin.ai/cli/sandbox"; + +const SANDBOX_FAILURE_PATTERN = /sandbox|seatbelt|bubblewrap|bwrap|socat|seccomp/i; const VALID_PERMISSION_MODES = new Set([ "normal", @@ -112,6 +115,26 @@ export function buildDevinPrintArgs({ return args; } +const SANDBOX_PLATFORM_HINTS = { + win32: "Native Windows does not support Devin's OS-level sandbox. Run the Devin CLI and this plugin inside WSL2.", + linux: "On Linux the sandbox needs bubblewrap (bwrap) and socat. Install them (for example `sudo apt install bubblewrap socat`) and retry.", + darwin: "On macOS the sandbox uses Seatbelt and needs no extra install. Check the error above, and that Devin is not already running inside another sandbox." +}; + +// Returns an actionable message when a failed sandboxed run looks sandbox-related, +// otherwise null. Devin fails closed when its sandbox cannot start, and so does this +// plugin: the caller must not retry without --sandbox. +export function sandboxFailureHint({ platform = process.platform, stderr = "" } = {}) { + if (!SANDBOX_FAILURE_PATTERN.test(String(stderr ?? ""))) { + return null; + } + return [ + "The sandbox could not start, so the run failed closed. It was not retried without the sandbox.", + SANDBOX_PLATFORM_HINTS[platform] ?? "Make sure the OS-level sandbox prerequisites for this platform are installed.", + `See ${SANDBOX_DOCS_URL}` + ].join("\n"); +} + export function listDevinSessions(cwd) { const result = runCommand(DEVIN_BINARY, ["list", "--format", "json"], { cwd }); if (result.error || result.status !== 0) { diff --git a/tests/fake-devin-fixture.mjs b/tests/fake-devin-fixture.mjs index 435a581..6510670 100644 --- a/tests/fake-devin-fixture.mjs +++ b/tests/fake-devin-fixture.mjs @@ -4,9 +4,11 @@ import path from "node:path"; // Creates a fake `devin` executable that answers the subcommands the companion // uses. The marker file records each -p invocation's prompt file for assertions. -export function createFakeDevin({ responseText = "FAKE_DEVIN_OUTPUT", exitCode = 0, failAuth = false } = {}) { +export function createFakeDevin({ responseText = "FAKE_DEVIN_OUTPUT", exitCode = 0, failAuth = false, stderrText = "" } = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fake-devin-")); const marker = path.join(dir, "invocations.jsonl"); + const stderrFile = path.join(dir, "stderr.txt"); + fs.writeFileSync(stderrFile, stderrText ? `${stderrText}\n` : ""); const script = `#!/usr/bin/env bash set -euo pipefail marker="${marker}" @@ -41,6 +43,7 @@ JSON if [ -n "$prompt_file" ] && [ -f "$prompt_file" ]; then cp "$prompt_file" "\${marker}.lastprompt" fi + cat "${stderrFile}" >&2 echo "${responseText}" exit ${exitCode} ;; @@ -62,6 +65,14 @@ esac } return fs.readFileSync(marker, "utf8").trim().split("\n").map((line) => JSON.parse(line)); }, + // One entry per `devin -p` run, with the full argument string. + printRuns() { + const file = `${marker}.args`; + if (!fs.existsSync(file)) { + return []; + } + return fs.readFileSync(file, "utf8").trim().split("\n").filter((line) => line.startsWith("-p")); + }, lastPrompt() { return fs.existsSync(`${marker}.lastprompt`) ? fs.readFileSync(`${marker}.lastprompt`, "utf8") : null; }, diff --git a/tests/sandbox.test.mjs b/tests/sandbox.test.mjs new file mode 100644 index 0000000..c8ebf4f --- /dev/null +++ b/tests/sandbox.test.mjs @@ -0,0 +1,162 @@ +import assert from "node:assert/strict"; +import { execFileSync, spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { createFakeDevin, createTempRepo } from "./fake-devin-fixture.mjs"; +import { SANDBOX_DOCS_URL, sandboxFailureHint } from "../plugins/devin/scripts/lib/devin.mjs"; + +const COMPANION = fileURLToPath(new URL("../plugins/devin/scripts/devin-companion.mjs", import.meta.url)); + +function runCompanion(args, { cwd, env = {} } = {}) { + return spawnSync(process.execPath, [COMPANION, ...args], { + cwd, + encoding: "utf8", + env: { ...process.env, ...env }, + timeout: 30000 + }); +} + +function initGitRepo(dir) { + execFileSync("git", ["init", "-q"], { cwd: dir }); + execFileSync("git", ["config", "user.email", "test@example.com"], { cwd: dir }); + execFileSync("git", ["config", "user.name", "Test"], { cwd: dir }); + fs.writeFileSync(path.join(dir, "hello.txt"), "hello\n"); + execFileSync("git", ["add", "-A"], { cwd: dir }); + execFileSync("git", ["commit", "-qm", "init"], { cwd: dir }); + fs.writeFileSync(path.join(dir, "hello.txt"), "hello changed\n"); +} + +function withFailingDevin(stderrText, fn) { + const repo = createTempRepo(); + const fake = createFakeDevin({ responseText: "", exitCode: 1, stderrText }); + initGitRepo(repo.dir); + const dataDir = fs.mkdtempSync("/tmp/devin-plugin-data-"); + try { + const env = { ...fake.env, CLAUDE_PLUGIN_DATA: dataDir }; + fn({ repo, fake, env }); + } finally { + repo.cleanup(); + fake.cleanup(); + fs.rmSync(dataDir, { recursive: true, force: true }); + } +} + +function assertRanOnceSandboxed(fake) { + const runs = fake.printRuns(); + assert.equal(runs.length, 1, `expected exactly one devin run, got: ${runs.join(" | ")}`); + assert.match(runs[0], /--sandbox/); +} + +test("sandboxFailureHint: win32 points to WSL2", () => { + const hint = sandboxFailureHint({ platform: "win32", stderr: "error: sandbox is not supported on this platform" }); + assert.match(hint, /Windows/); + assert.match(hint, /WSL2/); + assert.ok(hint.includes(SANDBOX_DOCS_URL)); +}); + +test("sandboxFailureHint: linux names bubblewrap and socat", () => { + const hint = sandboxFailureHint({ platform: "linux", stderr: "sandbox requires bwrap and socat" }); + assert.match(hint, /bubblewrap/); + assert.match(hint, /bwrap/); + assert.match(hint, /socat/); + assert.ok(hint.includes(SANDBOX_DOCS_URL)); +}); + +test("sandboxFailureHint: darwin names Seatbelt", () => { + const hint = sandboxFailureHint({ platform: "darwin", stderr: "sandbox-exec: sandbox_apply: Operation not permitted" }); + assert.match(hint, /Seatbelt/); + assert.ok(hint.includes(SANDBOX_DOCS_URL)); +}); + +test("sandboxFailureHint: other platforms get a generic hint with the docs link", () => { + const hint = sandboxFailureHint({ platform: "freebsd", stderr: "sandbox unavailable" }); + assert.ok(hint.includes(SANDBOX_DOCS_URL)); +}); + +test("sandboxFailureHint: non-sandbox errors get no hint", () => { + assert.equal(sandboxFailureHint({ platform: "linux", stderr: "network timeout talking to api" }), null); + assert.equal(sandboxFailureHint({ platform: "linux", stderr: "" }), null); + assert.equal(sandboxFailureHint({ platform: "linux" }), null); +}); + +test("review with a missing bwrap fails closed: one sandboxed run, failed job, hint shown", () => { + withFailingDevin("Error: sandbox unavailable: bwrap not found in PATH. Install bubblewrap and socat.", ({ repo, fake, env }) => { + const result = runCompanion(["review", "--wait"], { cwd: repo.dir, env }); + assert.equal(result.status, 1); + assert.match(result.stdout, /bwrap not found/); + assert.match(result.stdout, /failed closed/); + assert.match(result.stdout, /not retried without the sandbox/); + assert.ok(result.stdout.includes(SANDBOX_DOCS_URL)); + assertRanOnceSandboxed(fake); + + const status = JSON.parse(runCompanion(["status", "--json"], { cwd: repo.dir, env }).stdout); + assert.equal(status.jobs.length, 1); + assert.equal(status.jobs[0].status, "failed"); + }); +}); + +test("review with a Seatbelt error fails closed without a second run", () => { + withFailingDevin("sandbox-exec: sandbox_apply: Operation not permitted", ({ repo, fake, env }) => { + const result = runCompanion(["review", "--wait", "--json"], { cwd: repo.dir, env }); + assert.equal(result.status, 1); + const payload = JSON.parse(result.stdout); + assert.equal(payload.status, "failed"); + assert.match(payload.rendered, /Operation not permitted/); + assert.match(payload.rendered, /failed closed/); + // With --json the hint also reaches stderr for hooks and wrappers. + assert.match(result.stderr, /failed closed/); + assertRanOnceSandboxed(fake); + }); +}); + +test("task --read-only --sandbox fails closed and never runs without --sandbox", () => { + withFailingDevin("fatal: seccomp setup failed", ({ repo, fake, env }) => { + const result = runCompanion(["task", "--read-only", "--sandbox", "look", "around"], { cwd: repo.dir, env }); + assert.equal(result.status, 1); + assert.match(result.stdout, /failed closed/); + assertRanOnceSandboxed(fake); + assert.doesNotMatch(fake.printRuns()[0], /--permission-mode normal/); + }); +}); + +test("a non-sandbox failure of a sandboxed run gets no sandbox hint and keeps old exit behavior", () => { + withFailingDevin("error: model quota exceeded", ({ repo, fake, env }) => { + const result = runCompanion(["review", "--wait"], { cwd: repo.dir, env }); + assert.equal(result.status, 0); + assert.match(result.stdout, /quota exceeded/); + assert.doesNotMatch(result.stdout, /failed closed/); + assert.ok(!result.stdout.includes(SANDBOX_DOCS_URL)); + assertRanOnceSandboxed(fake); + }); +}); + +test("a sandbox-style failure without --sandbox gets no sandbox hint", () => { + withFailingDevin("sandbox: something unrelated mentioned", ({ repo, fake, env }) => { + const result = runCompanion(["task", "do", "it"], { cwd: repo.dir, env }); + assert.equal(result.status, 0); + assert.doesNotMatch(result.stdout, /failed closed/); + assert.equal(fake.printRuns().length, 1); + }); +}); + +test("stop review gate fails open with a warning when the sandbox is unavailable", () => { + withFailingDevin("Error: sandbox unavailable: bwrap not found in PATH", ({ repo, fake, env }) => { + runCompanion(["setup", "--enable-review-gate"], { cwd: repo.dir, env }); + const hook = fileURLToPath(new URL("../plugins/devin/scripts/stop-review-gate-hook.mjs", import.meta.url)); + const result = spawnSync(process.execPath, [hook], { + cwd: repo.dir, + encoding: "utf8", + input: JSON.stringify({ cwd: repo.dir, last_assistant_message: "done" }), + env: { ...process.env, ...env }, + timeout: 30000 + }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout.trim(), "", "gate must not block"); + assert.match(result.stderr, /failing open/); + assert.match(result.stderr, /failed closed/); + assertRanOnceSandboxed(fake); + }); +});