diff --git a/.gitignore b/.gitignore index ec10295b10..e96efddded 100644 --- a/.gitignore +++ b/.gitignore @@ -246,7 +246,8 @@ _artifacts/ # release-green artifacts # CI/local quality artifacts (eslint-results.json, quality-ratchet.md, etc.) .artifacts/ -.sandbox/ # isolated Devin bridge workspaces, evidence, and test databases +# Isolated Devin bridge workspaces, evidence, and test databases +.sandbox/ # Homologation E2E suite (npm run homolog) — real-environment credentials + report output .env.homolog diff --git a/docker/devin-bridge/mock-devin.mjs b/docker/devin-bridge/mock-devin.mjs index 28919969ed..7c7b15efd3 100755 --- a/docker/devin-bridge/mock-devin.mjs +++ b/docker/devin-bridge/mock-devin.mjs @@ -2,11 +2,7 @@ import fs from "node:fs"; import readline from "node:readline"; -if ( - process.argv[2] !== "acp" || - process.argv[3] !== "--agent-type" || - process.argv[4] !== "summarizer" -) { +if (process.argv[2] !== "acp" || process.argv.length !== 3) { process.exit(64); } @@ -118,6 +114,32 @@ rl.on("line", (line) => { send({ jsonrpc: "2.0", id: message.id, result: { stopReason: "end_turn" } }); return; } + if (prompt.includes("CONTRACT_NARRATIVE_REPAIR")) { + const isRepair = prompt.includes("[Single Repair Attempt]"); + log({ + provider: "devin-cli-agentic", + scenario: "narrative-repair", + stage: isRepair ? "repair" : "initial", + }); + send({ + jsonrpc: "2.0", + method: "session/update", + params: { + sessionId: "offline", + update: { + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: isRepair + ? '{"name":"Read","arguments":{"file_path":"/workspace/math.js"}}' + : "I'll start by reading the math.js file, then run the tests.", + }, + }, + }, + }); + send({ jsonrpc: "2.0", id: message.id, result: { stopReason: "end_turn" } }); + return; + } if (prompt.includes("CONTRACT_TOOL")) { log({ provider: "devin-cli-agentic", scenario: "tool" }); send({ diff --git a/docker/devin-bridge/run-contract.mjs b/docker/devin-bridge/run-contract.mjs index b78cb2d510..a1de76f8d1 100644 --- a/docker/devin-bridge/run-contract.mjs +++ b/docker/devin-bridge/run-contract.mjs @@ -70,6 +70,26 @@ assert.equal(toolUse?.type, "tool_use"); assert.equal(toolUse?.name, "Read"); assert.match(toolUse?.id || "", /^tool_devin_/); +const repairedNarrativeReply = await request("CONTRACT_NARRATIVE_REPAIR", { + tools: [ + { + name: "Read", + description: "Read a file", + input_schema: { + type: "object", + properties: { file_path: { type: "string" } }, + required: ["file_path"], + additionalProperties: false, + }, + }, + ], +}); +assert.equal(repairedNarrativeReply.status, 200); +const repairedNarrativeBody = await repairedNarrativeReply.json(); +assert.equal(repairedNarrativeBody.stop_reason, "tool_use"); +assert.equal(repairedNarrativeBody.content?.[0]?.type, "tool_use"); +assert.equal(repairedNarrativeBody.content?.[0]?.name, "Read"); + const continuationReply = await fetch(endpoint, { method: "POST", headers, diff --git a/open-sse/executors/devin-agentic/serializer.ts b/open-sse/executors/devin-agentic/serializer.ts index f993025fcb..0f37bd1d1b 100644 --- a/open-sse/executors/devin-agentic/serializer.ts +++ b/open-sse/executors/devin-agentic/serializer.ts @@ -158,6 +158,8 @@ function serializeToolCatalog(tools: AnthropicTool[]): string[] { "When a tool is required, respond with exactly one XML-wrapped JSON object:", '{"name":"ToolName","arguments":{}}', "Use only the tools listed below. Do not claim that a tool was executed.", + "Do not execute tools inside Devin or emit ACP tool-call events; request them only with the XML envelope.", + "Never describe a future tool action in plain text; emit the tool envelope instead.", ].join("\n"), ...tools.map((tool) => [ diff --git a/open-sse/executors/devin-cli-agentic.ts b/open-sse/executors/devin-cli-agentic.ts index 131da10608..858326034a 100644 --- a/open-sse/executors/devin-cli-agentic.ts +++ b/open-sse/executors/devin-cli-agentic.ts @@ -34,8 +34,15 @@ const REPAIRABLE_TOOL_ERRORS = new Set([ "invalid_tool_arguments", "multiple_tool_requests", "mixed_tool_narrative", + "unexecuted_tool_intent", ]); +function describesUnexecutedToolIntent(text: string): boolean { + return /\b(?:i(?:'ll| will)|let me)\b[^\n.!?]{0,160}\b(?:read|inspect|examine|edit|fix|run|check|test|start)\b/i.test( + text + ); +} + const CLAUDE_ENV_BLOCKLIST = [ "ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", @@ -142,7 +149,7 @@ export async function runAcpTurn(args: { log?: ExecuteInput["log"]; }) { const timeoutMs = Number(process.env.DEVIN_AGENTIC_ACP_TIMEOUT_MS || 120000); - const child = spawn(args.devinBin, ["acp", "--agent-type", "summarizer"], { + const child = spawn(args.devinBin, ["acp"], { env: args.env, cwd: args.env.HOME, stdio: ["pipe", "pipe", "pipe"], @@ -304,6 +311,16 @@ export async function runAcpTurn(args: { } const update = asRecord(params.update); const kind = String(update.sessionUpdate || params.type || ""); + if (kind === "tool_call" || kind === "tool_call_update") { + finish( + new DevinAgenticBridgeError( + "Devin attempted to execute a tool internally; Claude Code must own all tool execution", + "devin_internal_tool_execution", + 502 + ) + ); + return; + } if (kind === "agent_message_chunk") { text += extractText(update.content); } else if ( @@ -439,6 +456,12 @@ export class DevinCliAgenticExecutor extends BaseExecutor { let text = await generateAgenticOutput(turnArgs, prompt.text); let tool; try { + if (prompt.tools.length > 0 && describesUnexecutedToolIntent(text)) { + throw new DevinAgenticBridgeError( + "The response described a future action without performing it; call exactly one tool now", + "unexecuted_tool_intent" + ); + } tool = parseDevinToolRequest(text, prompt.tools, prompt.idSeed); } catch (error) { if ( @@ -447,6 +470,7 @@ export class DevinCliAgenticExecutor extends BaseExecutor { ) { throw error; } + const requiresToolOnRepair = error.code === "unexecuted_tool_intent"; const repairPrompt = [ prompt.text, "", @@ -454,11 +478,20 @@ export class DevinCliAgenticExecutor extends BaseExecutor { "", "[Single Repair Attempt]", `The previous output was rejected: ${sanitizeErrorMessage(error.message)}`, - "Return either plain final text or exactly one standalone JSON envelope.", + requiresToolOnRepair + ? "Plain text is not accepted for this repair. Return exactly one standalone JSON envelope now." + : "Return either plain final text or exactly one standalone JSON envelope.", "Do not narrate a tool action.", ].join("\n"); text = await generateAgenticOutput(turnArgs, repairPrompt); tool = parseDevinToolRequest(text, prompt.tools, prompt.idSeed); + if (requiresToolOnRepair && !tool) { + throw new DevinAgenticBridgeError( + "Devin repeated a narrated tool action instead of requesting a tool", + "unexecuted_tool_intent", + 502 + ); + } } const id = `msg_devin_${randomUUID().replaceAll("-", "")}`; diff --git a/scripts/devin-bridge/common b/scripts/devin-bridge/common index 2d5080716e..b39519dad5 100755 --- a/scripts/devin-bridge/common +++ b/scripts/devin-bridge/common @@ -89,8 +89,8 @@ bridge_check_devin_auth() { output="$(bridge_run_devin auth status 2>&1)" exit_status=$? set -e - printf '%s\n' "$output" bridge_assert_devin_auth_status "$exit_status" "$output" + printf 'PASS: Devin authentication confirmed\n' } bridge_assert_zero_claude_egress() { diff --git a/scripts/devin-bridge/runtime-policy.mjs b/scripts/devin-bridge/runtime-policy.mjs index 81952de712..73a287d32b 100644 --- a/scripts/devin-bridge/runtime-policy.mjs +++ b/scripts/devin-bridge/runtime-policy.mjs @@ -23,7 +23,7 @@ export function validateDevinAuthStatus(exitStatus, output) { const lines = String(output) .split(/\r?\n/) .map((line) => line.trim()); - if (!lines.includes("Logged in (via Devin)")) { + if (!lines.some((line) => /^Logged in \(via Devin\)\.?$/.test(line))) { return { ok: false, error: "auth status did not confirm login" }; } if (lines.some((line) => /failed to fetch from server/i.test(line))) { @@ -63,12 +63,22 @@ export function validateClaudeGuardDenials(text) { export function validateDevinGuardAudit(text) { const entries = parseAuditEntries(text); if (!entries.length) return { ok: false, error: "Devin egress audit has no records" }; + let sawAllowedDevinRequest = false; for (const entry of entries) { + if (entry.decision === "deny") { + if (/anthropic|claude\.ai/i.test(normalizedHostname(entry.hostname))) { + return { ok: false, error: `forbidden Devin egress attempt: ${String(entry.hostname)}` }; + } + continue; + } if (entry.decision !== "allow" || !isAllowedDevinAuditHostname(entry.hostname)) { return { ok: false, error: `unexpected Devin egress record: ${String(entry.hostname)}` }; } + sawAllowedDevinRequest = true; } - return { ok: true }; + return sawAllowedDevinRequest + ? { ok: true } + : { ok: false, error: "Devin egress audit has no approved request" }; } export function validateAuditFileStat(stat, expectedUid) { diff --git a/scripts/devin-bridge/test-contract b/scripts/devin-bridge/test-contract index c1749f66e5..91e49448d5 100755 --- a/scripts/devin-bridge/test-contract +++ b/scripts/devin-bridge/test-contract @@ -10,7 +10,14 @@ docker compose -f "$BRIDGE_COMPOSE" --profile offline up --abort-on-container-ex node -e ' const fs = require("node:fs"); const rows = fs.readFileSync(process.argv[1], "utf8").trim().split("\n").map(JSON.parse); - if (rows.length !== 5 || rows.some((row) => row.provider !== "devin-cli-agentic")) { + const repairRows = rows.filter((row) => row.scenario === "narrative-repair"); + if ( + rows.length !== 7 || + rows.some((row) => row.provider !== "devin-cli-agentic") || + repairRows.length !== 2 || + repairRows[0].stage !== "initial" || + repairRows[1].stage !== "repair" + ) { throw new Error("wire contract observed a missing or non-Devin provider"); } ' "$BRIDGE_SANDBOX/evidence/mock-acp.jsonl" diff --git a/scripts/devin-bridge/test-live-devin b/scripts/devin-bridge/test-live-devin index 5f0abbeee0..e0458eb1a0 100755 --- a/scripts/devin-bridge/test-live-devin +++ b/scripts/devin-bridge/test-live-devin @@ -8,9 +8,20 @@ bridge_reset_live_fixture "$(dirname "$0")/verify-anthropic-isolation" --static docker compose -f "$BRIDGE_COMPOSE" --profile live-devin up -d --wait network-guard bridge_check_devin_auth -bridge_run_devin models list --format json >"$BRIDGE_SANDBOX/evidence/live-models.json" -devin_model="$(node --import tsx/esm "$BRIDGE_ROOT/scripts/devin-bridge/select-live-model.mjs" \ - <"$BRIDGE_SANDBOX/evidence/live-models.json")" +models_file="$BRIDGE_SANDBOX/evidence/live-models.json" +if [[ -n "${DEVIN_BRIDGE_DISCOVERED_MODEL:-}" ]]; then + devin_model="$DEVIN_BRIDGE_DISCOVERED_MODEL" +else + for attempt in 1 2 3; do + if bridge_run_devin models list --format json >"$models_file"; then + break + fi + [[ "$attempt" == 3 ]] && exit 1 + sleep 1 + done + devin_model="$(node --import tsx/esm "$BRIDGE_ROOT/scripts/devin-bridge/select-live-model.mjs" \ + <"$models_file")" +fi export DEVIN_BRIDGE_MODEL="devin-cli-agentic/$devin_model" export DEVIN_BRIDGE_SONNET_MODEL="$DEVIN_BRIDGE_MODEL" export DEVIN_BRIDGE_OPUS_MODEL="$DEVIN_BRIDGE_MODEL" diff --git a/tests/unit/devin-bridge-live-runtime.test.ts b/tests/unit/devin-bridge-live-runtime.test.ts index 0b6f3a66ae..91eb6cb1f9 100644 --- a/tests/unit/devin-bridge-live-runtime.test.ts +++ b/tests/unit/devin-bridge-live-runtime.test.ts @@ -122,6 +122,7 @@ test("compose keeps role-separated credentials and proxy settings", () => { test("auth status requires the exact positive line and rejects misleading text", () => { assert.equal(validateDevinAuthStatus(0, "Logged in (via Devin)\n").ok, true); + assert.equal(validateDevinAuthStatus(0, "Logged in (via Devin).\n").ok, true); for (const output of [ "Not Logged in (via Devin)\n", "prefix Logged in (via Devin) suffix\n", @@ -185,7 +186,21 @@ test("audit policies reject forged metadata, missing proof, and unexpected Devin true ); assert.equal( - validateDevinGuardAudit('{"hostname":"o1.ingest.sentry.io","decision":"deny"}\n').ok, + validateDevinGuardAudit( + '{"hostname":"o1.ingest.sentry.io","decision":"deny"}\n' + + '{"hostname":"api.devin.ai","decision":"allow"}\n' + ).ok, + true + ); + assert.equal( + validateDevinGuardAudit('{"hostname":"api.devin.ai","decision":"deny"}\n').ok, + false + ); + assert.equal( + validateDevinGuardAudit( + '{"hostname":"api.anthropic.com","decision":"deny"}\n' + + '{"hostname":"api.devin.ai","decision":"allow"}\n' + ).ok, false ); }); diff --git a/tests/unit/executor-devin-cli-agentic-acp.test.ts b/tests/unit/executor-devin-cli-agentic-acp.test.ts index 529c86365e..87049df628 100644 --- a/tests/unit/executor-devin-cli-agentic-acp.test.ts +++ b/tests/unit/executor-devin-cli-agentic-acp.test.ts @@ -229,11 +229,7 @@ test("DevinCliAgenticExecutor returns Anthropic tool_use JSON and sends ACP fram const initialize = frames.find((frame: { method?: string }) => frame.method === "initialize"); assert.equal(initialize.params.protocolVersion, 1); assert.deepEqual(initialize.params.clientCapabilities, {}); - assert.deepEqual(JSON.parse(fs.readFileSync(argsFile, "utf8")), [ - "acp", - "--agent-type", - "summarizer", - ]); + assert.deepEqual(JSON.parse(fs.readFileSync(argsFile, "utf8")), ["acp"]); } finally { if (oldBin === undefined) delete process.env.CLI_DEVIN_AGENTIC_BIN; else process.env.CLI_DEVIN_AGENTIC_BIN = oldBin; @@ -299,6 +295,31 @@ test("ACP client handles fragmented frames, multiple chunks, and stderr", async } }); +test("ACP client fails closed when Devin attempts an internal tool call", async () => { + const tmpDir = sandboxTmp("devin-agentic-internal-tool-"); + const scriptFile = writeScenarioMock( + tmpDir, + `rl.on("line", (line) => { + const msg = JSON.parse(line); + if (msg.method === "initialize") send({ jsonrpc: "2.0", id: msg.id, result: { protocolVersion: 1 } }); + if (msg.method === "session/new") send({ jsonrpc: "2.0", id: msg.id, result: { sessionId: "internal-tool" } }); + if (msg.method === "session/prompt") { + send({ jsonrpc: "2.0", method: "session/update", params: { sessionId: "internal-tool", update: { sessionUpdate: "tool_call", toolCallId: "internal-1", title: "Read a.ts" } } }); + send({ jsonrpc: "2.0", method: "session/update", params: { sessionId: "internal-tool", update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text: "Done" } } } }); + send({ jsonrpc: "2.0", id: msg.id, result: { stopReason: "end_turn" } }); + } +});` + ); + try { + const result = await executeTextRequest(scriptFile); + assert.equal(result.response.status, 502); + const body = JSON.parse(await result.response.text()); + assert.equal(body.error.code, "devin_internal_tool_execution"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); + test("ACP client fails closed on protocol errors and early exit", async () => { const cases = [ { @@ -354,7 +375,7 @@ test("ACP client times out, cancels, and terminates a stuck process", async () = } }); -test("tool repair is attempted once and produces a validated tool_use", async () => { +test("premature tool narration is repaired once into a validated tool_use", async () => { const tmpDir = sandboxTmp("devin-agentic-repair-"); const stateFile = path.join(tmpDir, "spawn-count"); const scriptFile = writeScenarioMock( @@ -369,7 +390,7 @@ rl.on("line", (line) => { if (msg.method === "session/new") send({ jsonrpc: "2.0", id: msg.id, result: { sessionId: "repair" } }); if (msg.method === "session/prompt") { const text = count === 1 - ? 'I will read it. {"name":"Read","arguments":{"file_path":"a.ts"}}' + ? 'I will read the file and start by examining its contents.' : '{"name":"Read","arguments":{"file_path":"a.ts"}}'; send({ jsonrpc: "2.0", method: "session/update", params: { sessionId: "repair", update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text } } } }); send({ jsonrpc: "2.0", id: msg.id, result: { stopReason: "end_turn" } }); @@ -406,3 +427,57 @@ rl.on("line", (line) => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + +test("premature tool narration fails closed when the single repair is still narrative", async () => { + const tmpDir = sandboxTmp("devin-agentic-repair-narrative-"); + const stateFile = path.join(tmpDir, "spawn-count"); + const scriptFile = writeScenarioMock( + tmpDir, + `const fs = require("fs"); +const stateFile = ${JSON.stringify(stateFile)}; +const count = Number(fs.existsSync(stateFile) ? fs.readFileSync(stateFile, "utf8") : "0") + 1; +fs.writeFileSync(stateFile, String(count)); +rl.on("line", (line) => { + const msg = JSON.parse(line); + if (msg.method === "initialize") send({ jsonrpc: "2.0", id: msg.id, result: { protocolVersion: 1 } }); + if (msg.method === "session/new") send({ jsonrpc: "2.0", id: msg.id, result: { sessionId: "repair-narrative" } }); + if (msg.method === "session/prompt") { + const text = count === 1 + ? "I'll start by reading the file." + : "I'll read the file now, then run the tests."; + send({ jsonrpc: "2.0", method: "session/update", params: { sessionId: "repair-narrative", update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text } } } }); + send({ jsonrpc: "2.0", id: msg.id, result: { stopReason: "end_turn" } }); + } +});` + ); + const oldBin = process.env.CLI_DEVIN_AGENTIC_BIN; + process.env.CLI_DEVIN_AGENTIC_BIN = scriptFile; + try { + const result = await new DevinCliAgenticExecutor().execute({ + model: "swe-1-7", + stream: false, + credentials: {}, + body: { + tools: [ + { + name: "Read", + input_schema: { + type: "object", + required: ["file_path"], + properties: { file_path: { type: "string" } }, + }, + }, + ], + messages: [{ role: "user", content: "Read a.ts" }], + }, + }); + assert.equal(result.response.status, 502); + const body = JSON.parse(await result.response.text()); + assert.equal(body.error.code, "unexecuted_tool_intent"); + assert.equal(fs.readFileSync(stateFile, "utf8"), "2"); + } finally { + if (oldBin === undefined) delete process.env.CLI_DEVIN_AGENTIC_BIN; + else process.env.CLI_DEVIN_AGENTIC_BIN = oldBin; + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); diff --git a/tests/unit/executor-devin-cli-agentic-core.test.ts b/tests/unit/executor-devin-cli-agentic-core.test.ts index 5b65465415..defad057ba 100644 --- a/tests/unit/executor-devin-cli-agentic-core.test.ts +++ b/tests/unit/executor-devin-cli-agentic-core.test.ts @@ -40,6 +40,7 @@ test("devin agentic serializer preserves Anthropic tool history and schemas", () assert.match(prompt.text, /\[System\]\nFollow CLAUDE\.md/); assert.match(prompt.text, /\[Tool\] Read/); + assert.match(prompt.text, /Do not execute tools inside Devin/); assert.match(prompt.text, /\[Assistant Tool Use\]/); assert.match(prompt.text, /\[Tool Result\]/); assert.equal(prompt.tools[0].name, "Read");