From dbd4d5058d724b5541f71b6ce5b13f3c95f65ba3 Mon Sep 17 00:00:00 2001 From: Hessabi Date: Fri, 23 Oct 2026 11:28:32 -0500 Subject: [PATCH 1/2] Stop cloud sessions on cancel and read v3 pr_url Cancelling a handoff job only posted a "please stop" chat message, so the remote session kept running. Add stopCloudSession, which DELETEs the session (v1: /v1/sessions/{session_id}; v3: /v3/organizations/{org_id}/sessions/{devin_id}) through the same validated apiRequest path, and use it in cancel. The report says whether the remote stop was confirmed; API failures are tolerated. getCloudSession read the PR url from pull_requests[0].url, but v3 returns {pr_url, pr_state}. Check pr_url first, keeping v1's pull_request.url and the old accessor as fallbacks. Adds tests/cloud.test.mjs with mocked fetch (no network). --- AGENTS.md | 2 +- README.md | 2 +- plugins/devin/scripts/devin-companion.mjs | 12 ++- plugins/devin/scripts/lib/cloud.mjs | 15 +++- plugins/devin/scripts/lib/render.mjs | 8 +- tests/cloud.test.mjs | 99 +++++++++++++++++++++++ 6 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 tests/cloud.test.mjs diff --git a/AGENTS.md b/AGENTS.md index a40122b..9297cae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ passed through verbatim as prompt text. | `handoff` | Create a cloud session at app.devin.ai with repo+branch+diff context | `--wait`, `--background`, `--context`, `--tag`, `--poll-interval-ms` (both poll to terminal — detach in your own agent loop instead) | | `status` | List jobs; `status ` for one; `--wait` polls to terminal | `--json`, `--timeout-ms` | | `result` | Print a finished job's stored output verbatim | `--json` | -| `cancel` | Kill a running job's process tree (or stop a cloud session) | — | +| `cancel` | Kill a running job's process tree (or terminate a cloud session) | — | Every command accepts `--json` for machine-readable output. diff --git a/README.md b/README.md index 5a3c151..d427a2d 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ devin plugins install TheOnlyFusionCube/devin-plugin-cc#plugins/devin | `/devin:handoff ` | Create a cloud Devin session carrying repo, branch, context, and a bounded uncommitted diff. `--wait` polls until terminal. | | `/devin:status [job-id]` | List local jobs and cloud handoffs, with live phase/elapsed info. | | `/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. | +| `/devin:cancel ` | Kill a running local job's process tree; for cloud handoffs also terminates the remote session (reports if the stop could not be confirmed). | 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. diff --git a/plugins/devin/scripts/devin-companion.mjs b/plugins/devin/scripts/devin-companion.mjs index 11cdba0..20decad 100755 --- a/plugins/devin/scripts/devin-companion.mjs +++ b/plugins/devin/scripts/devin-companion.mjs @@ -13,7 +13,7 @@ import { getCloudSession, isTerminalCloudStatus, pollCloudSession, - sendCloudMessage + stopCloudSession } from "./lib/cloud.mjs"; import { buildDevinPrintArgs, @@ -676,13 +676,17 @@ async function cmdCancel(cwd, argv) { delivered = outcome.delivered; } + // null: no remote stop attempted; false: attempted but not confirmed. + let remoteStopped = null; if (job.kind === "handoff" && job.cloudSessionId && getCloudConfig().available) { - await sendCloudMessage(job.cloudSessionId, "Please stop working on this task — it was cancelled by the user.").catch(() => null); + remoteStopped = await stopCloudSession(job.cloudSessionId, getCloudConfig()) + .then(() => true) + .catch(() => false); } outputResult( - { jobId: job.id, status: "cancelled", delivered }, - renderCancelReport({ job, delivered }), + { jobId: job.id, status: "cancelled", delivered, remoteStopped }, + renderCancelReport({ job, delivered, remoteStopped }), options.json ); } diff --git a/plugins/devin/scripts/lib/cloud.mjs b/plugins/devin/scripts/lib/cloud.mjs index bc65353..89c82ea 100644 --- a/plugins/devin/scripts/lib/cloud.mjs +++ b/plugins/devin/scripts/lib/cloud.mjs @@ -175,7 +175,8 @@ export async function getCloudSession(sessionId, config = getCloudConfig()) { statusDetail: body?.status_detail ?? null, title: body?.title ?? null, url: body?.url ?? null, - pullRequestUrl: body?.pull_request?.url ?? body?.pull_requests?.[0]?.url ?? null, + // v1 returns pull_request: {url}; v3 returns pull_requests: [{pr_url, pr_state}]. + pullRequestUrl: body?.pull_request?.url ?? body?.pull_requests?.[0]?.pr_url ?? body?.pull_requests?.[0]?.url ?? null, structuredOutput: body?.structured_output ?? null, raw: body }; @@ -189,6 +190,18 @@ export async function sendCloudMessage(sessionId, message, config = getCloudConf return apiRequest(config, "POST", `${base}/sessions/${sessionId}/messages`, { message }); } +// Terminates the remote session instead of only asking it to stop. Both API +// versions use the same relative path: DELETE /v1/sessions/{session_id} for +// personal keys, DELETE /v3/organizations/{org_id}/sessions/{devin_id} for +// service keys, so resolveApiBase picks the right base for either. +export async function stopCloudSession(sessionId, config = getCloudConfig()) { + if (!config.available) { + throw new Error("DEVIN_API_KEY is not set."); + } + const base = resolveApiBase(config); + return apiRequest(config, "DELETE", `${base}/sessions/${sessionId}`); +} + export function isTerminalCloudStatus({ status, statusDetail }) { if (status === "error" || status === "suspended") { return { terminal: true, ok: false }; diff --git a/plugins/devin/scripts/lib/render.mjs b/plugins/devin/scripts/lib/render.mjs index 4103586..740817e 100644 --- a/plugins/devin/scripts/lib/render.mjs +++ b/plugins/devin/scripts/lib/render.mjs @@ -128,7 +128,7 @@ export function renderStoredJobResult(job) { return `Job ${job?.id ?? "?"} has no stored output yet (status: ${job?.status ?? "unknown"}).\n`; } -export function renderCancelReport({ job, delivered }) { +export function renderCancelReport({ job, delivered, remoteStopped = null }) { const lines = []; if (delivered) { lines.push(`Cancellation requested for job ${job.id} (${job.kind}).`); @@ -136,7 +136,11 @@ export function renderCancelReport({ job, delivered }) { lines.push(`Job ${job.id} (${job.kind}) was already finished or its process is gone; marked cancelled.`); } if (job.sessionUrl) { - lines.push(`Cloud session: ${job.sessionUrl} — stop or message it from the Devin web app if it is still running.`); + lines.push( + remoteStopped + ? `Cloud session: ${job.sessionUrl} (stopped remotely).` + : `Cloud session: ${job.sessionUrl} (could not confirm the remote stop; check the Devin web app).` + ); } return `${lines.join("\n")}\n`; } diff --git a/tests/cloud.test.mjs b/tests/cloud.test.mjs new file mode 100644 index 0000000..74080bd --- /dev/null +++ b/tests/cloud.test.mjs @@ -0,0 +1,99 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { getCloudConfig, getCloudSession, stopCloudSession } from "../plugins/devin/scripts/lib/cloud.mjs"; +import { renderCancelReport } from "../plugins/devin/scripts/lib/render.mjs"; + +const PERSONAL = getCloudConfig({ DEVIN_API_KEY: "apk_test_key" }); +const SERVICE = getCloudConfig({ DEVIN_API_KEY: "cog_test_key", DEVIN_ORG_ID: "org-123" }); + +function withMockedFetch(handler, fn) { + const original = globalThis.fetch; + const calls = []; + globalThis.fetch = async (url, init) => { + calls.push({ url: String(url), init }); + return handler(url, init); + }; + return Promise.resolve(fn(calls)).finally(() => { + globalThis.fetch = original; + }); +} + +function jsonResponse(body, status = 200) { + return new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); +} + +test("stopCloudSession DELETEs /v1/sessions/{id} for a personal key", async () => { + await withMockedFetch(() => jsonResponse({ detail: "terminated" }), async (calls) => { + await stopCloudSession("devin-abc", PERSONAL); + assert.equal(calls.length, 1); + assert.equal(calls[0].init.method, "DELETE"); + assert.equal(calls[0].url, "https://api.devin.ai/v1/sessions/devin-abc"); + assert.equal(calls[0].init.redirect, "error"); + assert.equal(calls[0].init.headers.Authorization, "Bearer apk_test_key"); + }); +}); + +test("stopCloudSession DELETEs /v3/organizations/{org}/sessions/{id} for a service key", async () => { + await withMockedFetch(() => jsonResponse({ session_id: "devin-abc", status: "exit" }), async (calls) => { + await stopCloudSession("devin-abc", SERVICE); + assert.equal(calls.length, 1); + assert.equal(calls[0].init.method, "DELETE"); + assert.equal(calls[0].url, "https://api.devin.ai/v3/organizations/org-123/sessions/devin-abc"); + }); +}); + +test("stopCloudSession rejects without a key and never calls fetch", async () => { + await withMockedFetch(() => jsonResponse({}), async (calls) => { + await assert.rejects(stopCloudSession("devin-abc", getCloudConfig({})), /DEVIN_API_KEY/); + assert.equal(calls.length, 0); + }); +}); + +test("stopCloudSession goes through DEVIN_API_URL validation: no request is made", async () => { + await withMockedFetch(() => jsonResponse({}), async (calls) => { + const config = getCloudConfig({ DEVIN_API_KEY: "apk_test_key", DEVIN_API_URL: "https://evil.example" }); + await assert.rejects(stopCloudSession("devin-abc", config), /DEVIN_API_URL/); + assert.equal(calls.length, 0); + }); +}); + +test("stopCloudSession surfaces API errors and refuses redirects", async () => { + await withMockedFetch(() => jsonResponse({ detail: "Session already exited" }, 400), async () => { + await assert.rejects(stopCloudSession("devin-abc", PERSONAL), /HTTP 400.*already exited/); + }); + await withMockedFetch(() => new Response(null, { status: 302 }), async () => { + await assert.rejects(stopCloudSession("devin-abc", PERSONAL), /redirect/i); + }); +}); + +test("getCloudSession reads the PR url from v1 pull_request.url", async () => { + const body = { status: "exit", pull_request: { url: "https://github.com/acme/repo/pull/1" } }; + await withMockedFetch(() => jsonResponse(body), async () => { + const snapshot = await getCloudSession("devin-abc", PERSONAL); + assert.equal(snapshot.pullRequestUrl, "https://github.com/acme/repo/pull/1"); + }); +}); + +test("getCloudSession reads the PR url from v3 pull_requests[0].pr_url", async () => { + const body = { status: "exit", pull_requests: [{ pr_url: "https://github.com/acme/repo/pull/2", pr_state: "open" }] }; + await withMockedFetch(() => jsonResponse(body), async () => { + const snapshot = await getCloudSession("devin-abc", SERVICE); + assert.equal(snapshot.pullRequestUrl, "https://github.com/acme/repo/pull/2"); + }); +}); + +test("getCloudSession returns a null PR url when none is present", async () => { + await withMockedFetch(() => jsonResponse({ status: "running", pull_requests: [] }), async () => { + const snapshot = await getCloudSession("devin-abc", SERVICE); + assert.equal(snapshot.pullRequestUrl, null); + }); +}); + +test("renderCancelReport states whether the remote stop was confirmed", () => { + const job = { id: "job-1", kind: "handoff", sessionUrl: "https://app.devin.ai/sessions/abc" }; + assert.match(renderCancelReport({ job, delivered: false, remoteStopped: true }), /stopped remotely/); + assert.match(renderCancelReport({ job, delivered: false, remoteStopped: false }), /could not confirm the remote stop/); + assert.match(renderCancelReport({ job, delivered: false }), /could not confirm the remote stop/); + assert.ok(!renderCancelReport({ job: { id: "job-2", kind: "task" }, delivered: true }).includes("Cloud session")); +}); From 9d2fc152821eb0c2709dfb7378424ec6ef754ca1 Mon Sep 17 00:00:00 2001 From: Hessabi Date: Sat, 24 Oct 2026 16:06:17 -0500 Subject: [PATCH 2/2] Keep cloud cancel pending until the remote stop is confirmed cancel no longer marks a cloud handoff cancelled before the DELETE and no longer swallows its failure. The job is set to cancel_pending first (active, so status keeps refreshing it), and becomes cancelled only after a follow-up GET shows the session exit, error, or suspended. The DELETE response alone is never trusted. Failures keep a short diagnostic in errorMessage (no key or API URL) and the report says how to retry. status settles a cancel_pending job once the session is observed stopped. --json keeps remoteStopped (true only when confirmed) and adds the resulting status and remoteStatus. Adds command-level tests against a local fake Devin API (success, API failure and retry, missing key, status settling, service key v3 path) and renames fake keys in tests/cloud.test.mjs to placeholders. --- AGENTS.md | 2 +- README.md | 2 +- plugins/devin/scripts/devin-companion.mjs | 81 ++++-- plugins/devin/scripts/lib/cloud.mjs | 53 ++++ plugins/devin/scripts/lib/jobs.mjs | 9 +- plugins/devin/scripts/lib/render.mjs | 18 +- tests/cloud-cancel.test.mjs | 299 ++++++++++++++++++++++ tests/cloud.test.mjs | 83 +++++- tests/jobs.test.mjs | 3 +- 9 files changed, 520 insertions(+), 30 deletions(-) create mode 100644 tests/cloud-cancel.test.mjs diff --git a/AGENTS.md b/AGENTS.md index 9297cae..611c6a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ passed through verbatim as prompt text. | `handoff` | Create a cloud session at app.devin.ai with repo+branch+diff context | `--wait`, `--background`, `--context`, `--tag`, `--poll-interval-ms` (both poll to terminal — detach in your own agent loop instead) | | `status` | List jobs; `status ` for one; `--wait` polls to terminal | `--json`, `--timeout-ms` | | `result` | Print a finished job's stored output verbatim | `--json` | -| `cancel` | Kill a running job's process tree (or terminate a cloud session) | — | +| `cancel` | Kill a running job's process tree (or terminate a cloud session; the job is `cancelled` only once the stop is confirmed, otherwise `cancel_pending`, which stays active and can be retried) | none | Every command accepts `--json` for machine-readable output. diff --git a/README.md b/README.md index d427a2d..c4f3aad 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,7 @@ devin plugins install TheOnlyFusionCube/devin-plugin-cc#plugins/devin | `/devin:handoff ` | Create a cloud Devin session carrying repo, branch, context, and a bounded uncommitted diff. `--wait` polls until terminal. | | `/devin:status [job-id]` | List local jobs and cloud handoffs, with live phase/elapsed info. | | `/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 terminates the remote session (reports if the stop could not be confirmed). | +| `/devin:cancel ` | Kill a running local job's process tree; for cloud handoffs also terminates the remote session and confirms it with a follow-up read. If the stop cannot be confirmed (missing key, API error) the job stays `cancel_pending`, still shows as active in `/devin:status`, and can be retried. | 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. diff --git a/plugins/devin/scripts/devin-companion.mjs b/plugins/devin/scripts/devin-companion.mjs index 20decad..5fa0c4d 100755 --- a/plugins/devin/scripts/devin-companion.mjs +++ b/plugins/devin/scripts/devin-companion.mjs @@ -11,9 +11,10 @@ import { createCloudSession, getCloudConfig, getCloudSession, + isCloudSessionStopped, isTerminalCloudStatus, pollCloudSession, - stopCloudSession + terminateCloudSession } from "./lib/cloud.mjs"; import { buildDevinPrintArgs, @@ -525,7 +526,13 @@ async function refreshCloudJob(cwd, job) { pullRequestUrl: snapshot.pullRequestUrl, sessionUrl: snapshot.url ?? job.sessionUrl }; - if (verdict.terminal) { + if (job.status === "cancel_pending") { + // Only an observed-stopped session settles a pending cancel; a session that + // is merely waiting_for_user or finished may still be alive. + if (isCloudSessionStopped(snapshot)) { + Object.assign(patch, { status: "cancelled", phase: "cancelled", completedAt: nowIso(), errorMessage: null }); + } + } else if (verdict.terminal) { patch.status = verdict.ok ? "completed" : "failed"; patch.completedAt = nowIso(); } @@ -647,6 +654,51 @@ async function cmdResult(cwd, argv) { outputResult({ jobId: job.id, kind: job.kind, status: job.status, rendered, result: job.result ?? null }, rendered, options.json); } +function persistJobPatch(cwd, patch) { + upsertJob(cwd, patch); + const jobFile = resolveJobFile(cwd, patch.id); + if (fs.existsSync(jobFile)) { + writeJobFile(cwd, patch.id, { ...readJobFile(jobFile), ...patch }); + } +} + +// Cloud handoffs only become "cancelled" once the remote session is confirmed +// stopped. cancel_pending is written first, so a crash or failure anywhere below +// leaves the job active and refreshable instead of hiding a billing session. +async function cancelCloudHandoff(cwd, job, asJson) { + persistJobPatch(cwd, { + id: job.id, + status: "cancel_pending", + phase: "cancel_pending", + pid: null, + errorMessage: "Cancel requested; the remote stop is not confirmed yet." + }); + + let delivered = false; + if (job.pid) { + delivered = terminateProcessTree(job.pid).delivered; + } + + const outcome = await terminateCloudSession(job.cloudSessionId, getCloudConfig()); + const patch = outcome.confirmed + ? { id: job.id, status: "cancelled", phase: "cancelled", errorMessage: null, completedAt: nowIso() } + : { id: job.id, status: "cancel_pending", phase: "cancel_pending", errorMessage: outcome.diagnostic }; + persistJobPatch(cwd, patch); + + outputResult( + { + jobId: job.id, + status: patch.status, + delivered, + remoteStopped: outcome.confirmed, + remoteStatus: outcome.status, + errorMessage: patch.errorMessage + }, + renderCancelReport({ job, delivered, remoteStopped: outcome.confirmed, status: patch.status, diagnostic: outcome.diagnostic }), + asJson + ); +} + async function cmdCancel(cwd, argv) { const { options, positionals } = parseCommandInput(argv, { booleanOptions: ["json"] }); const jobId = positionals[0] ?? null; @@ -659,16 +711,15 @@ async function cmdCancel(cwd, argv) { return; } + if (job.kind === "handoff" && job.cloudSessionId) { + await cancelCloudHandoff(cwd, job, options.json); + return; + } + // Mark cancelled BEFORE killing: the running companion's close handler // re-reads the job file after its child dies and must see "cancelled" so it // doesn't overwrite the outcome with "failed". - const completedAt = nowIso(); - const patch = { id: job.id, status: "cancelled", phase: "cancelled", pid: null, completedAt }; - upsertJob(cwd, patch); - const jobFile = resolveJobFile(cwd, job.id); - if (fs.existsSync(jobFile)) { - writeJobFile(cwd, job.id, { ...readJobFile(jobFile), ...patch }); - } + persistJobPatch(cwd, { id: job.id, status: "cancelled", phase: "cancelled", pid: null, completedAt: nowIso() }); let delivered = false; if (job.pid) { @@ -676,17 +727,9 @@ async function cmdCancel(cwd, argv) { delivered = outcome.delivered; } - // null: no remote stop attempted; false: attempted but not confirmed. - let remoteStopped = null; - if (job.kind === "handoff" && job.cloudSessionId && getCloudConfig().available) { - remoteStopped = await stopCloudSession(job.cloudSessionId, getCloudConfig()) - .then(() => true) - .catch(() => false); - } - outputResult( - { jobId: job.id, status: "cancelled", delivered, remoteStopped }, - renderCancelReport({ job, delivered, remoteStopped }), + { jobId: job.id, status: "cancelled", delivered, remoteStopped: null }, + renderCancelReport({ job, delivered, remoteStopped: null }), options.json ); } diff --git a/plugins/devin/scripts/lib/cloud.mjs b/plugins/devin/scripts/lib/cloud.mjs index 89c82ea..e39b155 100644 --- a/plugins/devin/scripts/lib/cloud.mjs +++ b/plugins/devin/scripts/lib/cloud.mjs @@ -202,6 +202,59 @@ export async function stopCloudSession(sessionId, config = getCloudConfig()) { return apiRequest(config, "DELETE", `${base}/sessions/${sessionId}`); } +// The session itself no longer runs. Stricter than isTerminalCloudStatus, which +// also counts waiting_for_user and finished: those sessions are still alive. +export function isCloudSessionStopped({ status }) { + return status === "exit" || status === "error" || status === "suspended"; +} + +function sanitizeDiagnostic(error, config) { + let text = (error instanceof Error ? error.message : String(error)).replace(/\s+/g, " ").trim(); + if (config.apiKey) { + text = text.split(config.apiKey).join("[redacted]"); + } + return text.length > 300 ? `${text.slice(0, 299)}…` : text; +} + +// Stops the remote session and reports whether the stop is CONFIRMED. The DELETE +// response is never trusted on its own: v1 returns only {detail}, and v3 returns a +// SessionResponse whose status is not documented as the post-termination state +// (status_detail is only populated on GET). So after the DELETE, even a failed one +// (v1 rejects DELETE on an already-exited session), we GET the session and confirm +// only when its status is exit, error, or suspended. Never throws; the diagnostic +// never contains the API key or the DEVIN_API_URL value. +export async function terminateCloudSession(sessionId, config = getCloudConfig()) { + if (!config.available) { + return { confirmed: false, status: null, diagnostic: "DEVIN_API_KEY is not set; it is needed to stop the remote session." }; + } + let deleteError = null; + try { + await stopCloudSession(sessionId, config); + } catch (error) { + deleteError = sanitizeDiagnostic(error, config); + } + let status = null; + let readError = null; + try { + const snapshot = await getCloudSession(sessionId, config); + status = snapshot.status; + if (isCloudSessionStopped(snapshot)) { + return { confirmed: true, status, diagnostic: null }; + } + } catch (error) { + readError = sanitizeDiagnostic(error, config); + } + const parts = [deleteError ? `Stop request failed: ${deleteError}` : "Stop request was accepted."]; + if (readError) { + if (readError !== deleteError) { + parts.push(`Could not read the session to confirm: ${readError}`); + } + } else { + parts.push(`The session is still reported as "${status}".`); + } + return { confirmed: false, status, diagnostic: parts.join(" ") }; +} + export function isTerminalCloudStatus({ status, statusDetail }) { if (status === "error" || status === "suspended") { return { terminal: true, ok: false }; diff --git a/plugins/devin/scripts/lib/jobs.mjs b/plugins/devin/scripts/lib/jobs.mjs index 8138c4e..9f7eb76 100644 --- a/plugins/devin/scripts/lib/jobs.mjs +++ b/plugins/devin/scripts/lib/jobs.mjs @@ -160,8 +160,15 @@ export function sortJobsNewestFirst(jobs) { }); } +// cancel_pending: a cancel was requested but the remote cloud session is not +// confirmed stopped, so it stays active and refreshable until it is. export function isJobActive(job) { - return job?.status === "queued" || job?.status === "running" || job?.status === "detached"; + return ( + job?.status === "queued" || + job?.status === "running" || + job?.status === "detached" || + job?.status === "cancel_pending" + ); } export function elapsedMs(job) { diff --git a/plugins/devin/scripts/lib/render.mjs b/plugins/devin/scripts/lib/render.mjs index 740817e..1404c67 100644 --- a/plugins/devin/scripts/lib/render.mjs +++ b/plugins/devin/scripts/lib/render.mjs @@ -128,8 +128,22 @@ export function renderStoredJobResult(job) { return `Job ${job?.id ?? "?"} has no stored output yet (status: ${job?.status ?? "unknown"}).\n`; } -export function renderCancelReport({ job, delivered, remoteStopped = null }) { +export function renderCancelReport({ job, delivered, remoteStopped = null, status = "cancelled", diagnostic = null }) { const lines = []; + if (status === "cancel_pending") { + lines.push(`Job ${job.id} (${job.kind}) is NOT cancelled: the cloud session was not confirmed stopped and may still be running.`); + if (delivered) { + lines.push("The local poller was stopped."); + } + if (job.sessionUrl) { + lines.push(`Cloud session: ${job.sessionUrl}`); + } + if (diagnostic) { + lines.push(`Reason: ${diagnostic}`); + } + lines.push(`Retry with /devin:cancel ${job.id}, re-check with /devin:status ${job.id}, or stop it in the Devin web app.`); + return `${lines.join("\n")}\n`; + } if (delivered) { lines.push(`Cancellation requested for job ${job.id} (${job.kind}).`); } else { @@ -138,7 +152,7 @@ export function renderCancelReport({ job, delivered, remoteStopped = null }) { if (job.sessionUrl) { lines.push( remoteStopped - ? `Cloud session: ${job.sessionUrl} (stopped remotely).` + ? `Cloud session: ${job.sessionUrl} (stopped remotely, confirmed).` : `Cloud session: ${job.sessionUrl} (could not confirm the remote stop; check the Devin web app).` ); } diff --git a/tests/cloud-cancel.test.mjs b/tests/cloud-cancel.test.mjs new file mode 100644 index 0000000..86b2cf3 --- /dev/null +++ b/tests/cloud-cancel.test.mjs @@ -0,0 +1,299 @@ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import fs from "node:fs"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { generateJobId, upsertJob } from "../plugins/devin/scripts/lib/state.mjs"; + +const COMPANION = fileURLToPath(new URL("../plugins/devin/scripts/devin-companion.mjs", import.meta.url)); +const SESSION_ID = "devin-example1"; +const PERSONAL_KEY = "apk_example"; +const SERVICE_KEY = "cog_service_example"; +const ORG_ID = "org-example"; + +// Local stand-in for the Devin API. The companion runs as a child process, so the +// server lives in this process and the child is spawned asynchronously (spawnSync +// would block this event loop and the server could never answer). +async function startFakeApi() { + const api = { + requests: [], + deleteStatus: 200, + deleteBody: { detail: "Session terminated" }, + deleteStopsSession: true, + session: { status: "running", status_detail: "working" }, + url: "" + }; + const server = http.createServer((req, res) => { + api.requests.push({ method: req.method, path: req.url, authorization: req.headers.authorization }); + const send = (status, body) => { + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); + }; + const resource = req.url.replace(/^\/v1|^\/v3\/organizations\/[^/]+/, ""); + if (req.method === "POST" && resource === "/sessions") { + send(200, { session_id: SESSION_ID, url: "https://app.devin.ai/sessions/example1" }); + } else if (req.method === "DELETE" && resource === `/sessions/${SESSION_ID}`) { + if (api.deleteStatus < 300 && api.deleteStopsSession) { + api.session = { status: "exit", status_detail: null }; + } + send(api.deleteStatus, api.deleteStatus < 300 ? api.deleteBody : { detail: "internal fault" }); + } else if (req.method === "GET" && resource === `/sessions/${SESSION_ID}`) { + send(200, { session_id: SESSION_ID, url: "https://app.devin.ai/sessions/example1", ...api.session }); + } else { + send(404, { detail: "not found" }); + } + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + api.url = `http://127.0.0.1:${server.address().port}`; + api.close = () => new Promise((resolve) => server.close(resolve)); + api.count = (method) => api.requests.filter((request) => request.method === method).length; + return api; +} + +function runCompanion(args, { cwd, env }) { + return new Promise((resolve, reject) => { + const child = spawn(process.execPath, [COMPANION, ...args], { cwd, env: { ...process.env, ...env } }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("error", reject); + child.on("close", (status) => resolve({ status, stdout, stderr })); + }); +} + +async function createHarness({ serviceKey = false } = {}) { + const api = await startFakeApi(); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "devin-cancel-test-")); + const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "devin-cancel-data-")); + const env = { + CLAUDE_PLUGIN_DATA: dataDir, + DEVIN_API_KEY: serviceKey ? SERVICE_KEY : PERSONAL_KEY, + DEVIN_ORG_ID: serviceKey ? ORG_ID : "", + DEVIN_USER_ID: "", + DEVIN_API_URL: api.url, + DEVIN_COMPANION_ALLOW_LOOPBACK_API: "1" + }; + const run = (args, overrides = {}) => runCompanion(args, { cwd: dir, env: { ...env, ...overrides } }); + const json = async (args, overrides) => { + const result = await run([...args, "--json"], overrides); + assert.equal(result.status, 0, result.stderr); + return JSON.parse(result.stdout); + }; + const handoff = async () => (await json(["handoff", "--task", "fix the flaky test"])).jobId; + const job = async (jobId, overrides) => (await json(["status", jobId], overrides)); + // Every byte the plugin persisted or printed must be free of the key and the API URL. + const assertNoSecrets = (...outputs) => { + const persisted = []; + const walk = (current) => { + for (const entry of fs.readdirSync(current, { withFileTypes: true })) { + const full = path.join(current, entry.name); + entry.isDirectory() ? walk(full) : persisted.push(fs.readFileSync(full, "utf8")); + } + }; + walk(dataDir); + for (const text of [...persisted, ...outputs]) { + assert.ok(!text.includes(PERSONAL_KEY) && !text.includes(SERVICE_KEY), "API key leaked"); + assert.ok(!text.includes(api.url), "DEVIN_API_URL value leaked"); + } + }; + return { + api, + dir, + dataDir, + run, + json, + handoff, + job, + assertNoSecrets, + async cleanup() { + await api.close(); + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(dataDir, { recursive: true, force: true }); + } + }; +} + +test("cancel stops the cloud session, confirms it with a GET, and only then marks the job cancelled", async () => { + const h = await createHarness(); + try { + const jobId = await h.handoff(); + const cancel = await h.json(["cancel", jobId]); + assert.equal(cancel.status, "cancelled"); + assert.equal(cancel.remoteStopped, true); + assert.equal(cancel.remoteStatus, "exit"); + assert.equal(cancel.errorMessage, null); + assert.deepEqual( + h.api.requests.map((request) => `${request.method} ${request.path}`), + ["POST /v1/sessions", `DELETE /v1/sessions/${SESSION_ID}`, `GET /v1/sessions/${SESSION_ID}`] + ); + assert.equal(h.api.requests[1].authorization, `Bearer ${PERSONAL_KEY}`); + + const stored = await h.job(jobId); + assert.equal(stored.status, "cancelled"); + assert.ok(stored.completedAt); + assert.equal(stored.errorMessage, null); + + const again = await h.json(["cancel", jobId]); + assert.equal(again.cancelled, false); + assert.equal(h.api.count("DELETE"), 1); + } finally { + await h.cleanup(); + } +}); + +test("cancel reports a failed DELETE, keeps the job cancel_pending and refreshable, and a retry succeeds", async () => { + const h = await createHarness(); + try { + const jobId = await h.handoff(); + h.api.deleteStatus = 500; + + const text = await h.run(["cancel", jobId]); + assert.equal(text.status, 0, text.stderr); + assert.match(text.stdout, /NOT cancelled/); + assert.match(text.stdout, /HTTP 500/); + assert.match(text.stdout, new RegExp(`/devin:cancel ${jobId}`)); + assert.match(text.stdout, /Devin web app/); + + // The first run already left the job pending; query the structured form of a second attempt. + const failed = await h.json(["cancel", jobId]); + assert.equal(failed.status, "cancel_pending"); + assert.equal(failed.remoteStopped, false); + assert.match(failed.errorMessage, /Stop request failed: Devin API returned HTTP 500/); + assert.equal(h.api.count("DELETE"), 2); + + // status still treats it as active and keeps refreshing it from the API. + const getsBefore = h.api.count("GET"); + const pending = await h.job(jobId); + assert.equal(pending.status, "cancel_pending"); + assert.match(pending.errorMessage, /HTTP 500/); + assert.equal(pending.completedAt, null); + assert.ok(h.api.count("GET") > getsBefore, "status must refresh a cancel_pending job"); + const listed = await h.json(["status"]); + assert.equal(listed.jobs.find((entry) => entry.id === jobId).status, "cancel_pending"); + + // Retry once the API recovers. + h.api.deleteStatus = 200; + const retried = await h.json(["cancel", jobId]); + assert.equal(retried.status, "cancelled"); + assert.equal(retried.remoteStopped, true); + const stored = await h.job(jobId); + assert.equal(stored.status, "cancelled"); + assert.equal(stored.errorMessage, null); + assert.ok(stored.completedAt); + + h.assertNoSecrets(text.stdout, text.stderr); + } finally { + await h.cleanup(); + } +}); + +test("cancel without DEVIN_API_KEY keeps the job cancel_pending and makes no request", async () => { + const h = await createHarness(); + try { + const jobId = await h.handoff(); + const requestsBefore = h.api.requests.length; + + const cancel = await h.json(["cancel", jobId], { DEVIN_API_KEY: "" }); + assert.equal(cancel.status, "cancel_pending"); + assert.equal(cancel.remoteStopped, false); + assert.match(cancel.errorMessage, /DEVIN_API_KEY is not set; it is needed to stop the remote session/); + assert.equal(h.api.requests.length, requestsBefore); + + // Still active and listed without the key; nothing can be refreshed, nothing is requested. + const pending = await h.job(jobId, { DEVIN_API_KEY: "" }); + assert.equal(pending.status, "cancel_pending"); + assert.equal(h.api.requests.length, requestsBefore); + + // With the key back, retry stops the session. + const retried = await h.json(["cancel", jobId]); + assert.equal(retried.status, "cancelled"); + assert.equal(retried.remoteStopped, true); + } finally { + await h.cleanup(); + } +}); + +test("status settles a cancel_pending job once the remote session is observed stopped", async () => { + const h = await createHarness(); + try { + const jobId = await h.handoff(); + h.api.deleteStatus = 500; + const failed = await h.json(["cancel", jobId]); + assert.equal(failed.status, "cancel_pending"); + assert.equal(h.api.count("DELETE"), 1); + + // A session that is merely waiting for the user is still alive: stay pending. + h.api.session = { status: "running", status_detail: "waiting_for_user" }; + const waiting = await h.job(jobId); + assert.equal(waiting.status, "cancel_pending"); + + // The session ends on its own (or the stop lands late): status marks it cancelled. + h.api.session = { status: "exit", status_detail: null }; + const settled = await h.job(jobId); + assert.equal(settled.status, "cancelled"); + assert.ok(settled.completedAt); + assert.equal(settled.errorMessage, null); + assert.equal(h.api.count("DELETE"), 1, "status must not issue DELETE"); + + // Terminal now: later status calls no longer hit the API. + const requestsBefore = h.api.requests.length; + await h.job(jobId); + assert.equal(h.api.requests.length, requestsBefore); + } finally { + await h.cleanup(); + } +}); + +test("a service key goes through /v3 and a DELETE body claiming exit is not trusted", async () => { + const h = await createHarness({ serviceKey: true }); + try { + const jobId = await h.handoff(); + h.api.deleteBody = { session_id: SESSION_ID, status: "exit" }; + h.api.deleteStopsSession = false; + + const cancel = await h.json(["cancel", jobId]); + assert.equal(cancel.status, "cancel_pending"); + assert.equal(cancel.remoteStopped, false); + assert.equal(cancel.remoteStatus, "running"); + assert.match(cancel.errorMessage, /still reported as "running"/); + const prefix = `/v3/organizations/${ORG_ID}/sessions/${SESSION_ID}`; + assert.deepEqual( + h.api.requests.slice(1).map((request) => `${request.method} ${request.path}`), + [`DELETE ${prefix}`, `GET ${prefix}`] + ); + + h.api.deleteStopsSession = true; + const retried = await h.json(["cancel", jobId]); + assert.equal(retried.status, "cancelled"); + assert.equal(retried.remoteStopped, true); + } finally { + await h.cleanup(); + } +}); + +test("cancel of a local job without a cloud session is unchanged and never touches the API", async () => { + const h = await createHarness(); + const previous = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = h.dataDir; + try { + const id = generateJobId("devin"); + upsertJob(h.dir, { id, kind: "task", workspaceRoot: h.dir, status: "running", title: "local task", pid: null }); + const cancel = await h.json(["cancel", id]); + assert.equal(cancel.status, "cancelled"); + assert.equal(cancel.remoteStopped, null); + assert.equal((await h.job(id)).status, "cancelled"); + assert.equal(h.api.requests.length, 0); + } finally { + if (previous === undefined) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previous; + } + await h.cleanup(); + } +}); diff --git a/tests/cloud.test.mjs b/tests/cloud.test.mjs index 74080bd..2e5c678 100644 --- a/tests/cloud.test.mjs +++ b/tests/cloud.test.mjs @@ -1,11 +1,17 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { getCloudConfig, getCloudSession, stopCloudSession } from "../plugins/devin/scripts/lib/cloud.mjs"; +import { + getCloudConfig, + getCloudSession, + isCloudSessionStopped, + stopCloudSession, + terminateCloudSession +} from "../plugins/devin/scripts/lib/cloud.mjs"; import { renderCancelReport } from "../plugins/devin/scripts/lib/render.mjs"; -const PERSONAL = getCloudConfig({ DEVIN_API_KEY: "apk_test_key" }); -const SERVICE = getCloudConfig({ DEVIN_API_KEY: "cog_test_key", DEVIN_ORG_ID: "org-123" }); +const PERSONAL = getCloudConfig({ DEVIN_API_KEY: "apk_example" }); +const SERVICE = getCloudConfig({ DEVIN_API_KEY: "cog_service_example", DEVIN_ORG_ID: "org-123" }); function withMockedFetch(handler, fn) { const original = globalThis.fetch; @@ -30,7 +36,7 @@ test("stopCloudSession DELETEs /v1/sessions/{id} for a personal key", async () = assert.equal(calls[0].init.method, "DELETE"); assert.equal(calls[0].url, "https://api.devin.ai/v1/sessions/devin-abc"); assert.equal(calls[0].init.redirect, "error"); - assert.equal(calls[0].init.headers.Authorization, "Bearer apk_test_key"); + assert.equal(calls[0].init.headers.Authorization, "Bearer apk_example"); }); }); @@ -52,7 +58,7 @@ test("stopCloudSession rejects without a key and never calls fetch", async () => test("stopCloudSession goes through DEVIN_API_URL validation: no request is made", async () => { await withMockedFetch(() => jsonResponse({}), async (calls) => { - const config = getCloudConfig({ DEVIN_API_KEY: "apk_test_key", DEVIN_API_URL: "https://evil.example" }); + const config = getCloudConfig({ DEVIN_API_KEY: "apk_example", DEVIN_API_URL: "https://evil.example" }); await assert.rejects(stopCloudSession("devin-abc", config), /DEVIN_API_URL/); assert.equal(calls.length, 0); }); @@ -97,3 +103,70 @@ test("renderCancelReport states whether the remote stop was confirmed", () => { assert.match(renderCancelReport({ job, delivered: false }), /could not confirm the remote stop/); assert.ok(!renderCancelReport({ job: { id: "job-2", kind: "task" }, delivered: true }).includes("Cloud session")); }); + +test("renderCancelReport says NOT cancelled and how to retry when the stop is unconfirmed", () => { + const job = { id: "job-3", kind: "handoff", sessionUrl: "https://app.devin.ai/sessions/abc" }; + const text = renderCancelReport({ job, delivered: true, remoteStopped: false, status: "cancel_pending", diagnostic: "Stop request failed: HTTP 500" }); + assert.match(text, /NOT cancelled/); + assert.match(text, /Reason: Stop request failed: HTTP 500/); + assert.match(text, /\/devin:cancel job-3/); + assert.match(text, /Devin web app/); +}); + +test("isCloudSessionStopped accepts exit, error, suspended only", () => { + for (const status of ["exit", "error", "suspended"]) { + assert.equal(isCloudSessionStopped({ status }), true, status); + } + for (const status of ["new", "claimed", "running", "resuming", "unknown"]) { + assert.equal(isCloudSessionStopped({ status }), false, status); + } +}); + +test("terminateCloudSession needs a GET showing a stopped session, not just a 2xx DELETE", async () => { + await withMockedFetch( + (url, init) => (init.method === "DELETE" ? jsonResponse({ session_id: "devin-abc", status: "exit" }) : jsonResponse({ status: "running" })), + async (calls) => { + const outcome = await terminateCloudSession("devin-abc", SERVICE); + assert.equal(outcome.confirmed, false); + assert.equal(outcome.status, "running"); + assert.match(outcome.diagnostic, /still reported as "running"/); + assert.deepEqual(calls.map((call) => call.init.method), ["DELETE", "GET"]); + } + ); + await withMockedFetch( + (url, init) => (init.method === "DELETE" ? jsonResponse({ detail: "terminated" }) : jsonResponse({ status: "exit" })), + async () => { + assert.deepEqual(await terminateCloudSession("devin-abc", PERSONAL), { confirmed: true, status: "exit", diagnostic: null }); + } + ); +}); + +test("terminateCloudSession confirms an already-exited session even when DELETE is rejected", async () => { + await withMockedFetch( + (url, init) => (init.method === "DELETE" ? jsonResponse({ detail: "Session already exited" }, 400) : jsonResponse({ status: "exit" })), + async () => { + const outcome = await terminateCloudSession("devin-abc", PERSONAL); + assert.equal(outcome.confirmed, true); + assert.equal(outcome.diagnostic, null); + } + ); +}); + +test("terminateCloudSession reports failures without throwing, never echoing the key", async () => { + await withMockedFetch(() => jsonResponse({ detail: "boom apk_example leaked" }, 500), async () => { + const outcome = await terminateCloudSession("devin-abc", PERSONAL); + assert.equal(outcome.confirmed, false); + assert.match(outcome.diagnostic, /Stop request failed: Devin API returned HTTP 500/); + assert.ok(!outcome.diagnostic.includes("apk_example")); + }); + await withMockedFetch(() => jsonResponse({}), async (calls) => { + const missingKey = await terminateCloudSession("devin-abc", getCloudConfig({})); + assert.equal(missingKey.confirmed, false); + assert.match(missingKey.diagnostic, /DEVIN_API_KEY/); + const badUrl = await terminateCloudSession("devin-abc", getCloudConfig({ DEVIN_API_KEY: "apk_example", DEVIN_API_URL: "https://evil.example" })); + assert.equal(badUrl.confirmed, false); + assert.match(badUrl.diagnostic, /DEVIN_API_URL/); + assert.ok(!badUrl.diagnostic.includes("evil.example")); + assert.equal(calls.length, 0); + }); +}); diff --git a/tests/jobs.test.mjs b/tests/jobs.test.mjs index b978320..816e201 100644 --- a/tests/jobs.test.mjs +++ b/tests/jobs.test.mjs @@ -10,10 +10,11 @@ import { } from "../plugins/devin/scripts/lib/jobs.mjs"; import { isTerminalCloudStatus } from "../plugins/devin/scripts/lib/cloud.mjs"; -test("isJobActive covers running, queued, detached", () => { +test("isJobActive covers running, queued, detached, cancel_pending", () => { assert.equal(isJobActive({ status: "running" }), true); assert.equal(isJobActive({ status: "queued" }), true); assert.equal(isJobActive({ status: "detached" }), true); + assert.equal(isJobActive({ status: "cancel_pending" }), true); assert.equal(isJobActive({ status: "completed" }), false); assert.equal(isJobActive({ status: "cancelled" }), false); });