diff --git a/doc/observability.md b/doc/observability.md index 93513a80716..f859149d146 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -415,6 +415,16 @@ values become `unknown`. A signal such as `SIGKILL` does not establish who sent it or prove an out-of-memory kill. These fields do not change error grouping or run outcomes, and do not include process output or adapter result payloads. +An unconfirmed adapter Stop timeout has an event-local `adapter_stop` context: +the run UUID, built-in adapter type, native/legacy runtime mode, configured +wait duration, and whether abort was requested. Invalid identities become +`null`, and unknown adapter/runtime values become `unknown`. The report does +not include stop reasons, prompts, process output, provider responses, or +credentials. It preserves default error grouping and does not acknowledge +termination, remove the live execution control, or change the timeout. The +context is sent only through the existing opt-in Sentry gate and never leaks +into unrelated captures. + The shared reporter also attaches bounded diagnostic contexts for both legacy and native runs: diff --git a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts index 11b3c259f58..a3f26b22edd 100644 --- a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts +++ b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts @@ -35,6 +35,53 @@ afterEach(async () => { }); describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", () => { + it("keeps unconfirmed Stop context off unrelated events and drops arbitrary error fields", async () => { + const Sentry = sentryPackage!; + const events: Array> = []; + vi.stubEnv("SENTRY_DSN_BACKEND", "https://public@example.invalid/1"); + vi.doMock("../peer-version-check.js", () => ({ checkExactPeerVersions: () => ({ ok: true }) })); + vi.doMock("@sentry/node", () => ({ + ...Sentry, + init: (options: Record) => Sentry.init({ + ...options, + transport: () => ({ send: async () => ({}), flush: async () => true }), + beforeSend: (event: Record) => { events.push(event); return event; }, + }), + })); + vi.resetModules(); + const { sentryReady, captureException } = await import("../sentry.js"); + const { AdapterStopTimeoutError } = await import("../services/adapter-stop-timeout.js"); + await sentryReady; + const timeout = Object.assign(new AdapterStopTimeoutError(60_000, { + runId: "11111111-1111-4111-8111-111111111111", adapterType: "cursor", + runtimeMode: "legacy", abortRequested: true, + }), { + cause: new Error("private-provider-cause"), + providerResponse: { headers: "private-provider-headers", body: "private-provider-body" }, + }); + captureException(timeout); + await Promise.resolve(); + captureException(new Error("unrelated stop diagnostic fixture")); + await Sentry.flush(2000); + expect(events).toHaveLength(2); + const captured = (message: string) => events.find((event) => + (event.exception as { values: Array<{ value: string }> }).values.some((entry) => entry.value === message), + ); + expect(captured(timeout.message)).toMatchObject({ + tags: { error_code: "adapter_stop_unconfirmed" }, fingerprint: ["{{ default }}"], + contexts: { adapter_stop: { + runId: "11111111-1111-4111-8111-111111111111", adapterType: "cursor", + runtimeMode: "legacy", abortRequested: true, timeoutMs: 60_000, + } }, + }); + expect(JSON.stringify(events)).not.toContain("private-provider-"); + const unrelated = captured("unrelated stop diagnostic fixture"); + expect(unrelated).toBeDefined(); + expect(unrelated).not.toHaveProperty("contexts.adapter_stop"); + expect(unrelated).not.toHaveProperty("tags.error_code"); + expect(unrelated).not.toHaveProperty("fingerprint"); + }); + it("keeps each run's identity and fingerprint off unrelated errors", async () => { const Sentry = sentryPackage!; const events: Array> = []; diff --git a/server/src/__tests__/sentry.test.ts b/server/src/__tests__/sentry.test.ts index 030029c9b02..08659e40a17 100644 --- a/server/src/__tests__/sentry.test.ts +++ b/server/src/__tests__/sentry.test.ts @@ -127,6 +127,45 @@ describe("sentryReady", () => { }); describe("captureException", () => { + it("adds bounded Stop timeout context only to that event", async () => { + process.env[BACKEND_DSN_ENV] = "https://fixture@example.com/1"; + const sdk = mockSentryPackage(); + const { captureException, sentryReady } = await importFreshSentry(); + const { AdapterStopTimeoutError } = await import("../services/adapter-stop-timeout.js"); + await sentryReady; + const error = new AdapterStopTimeoutError(60_000, { + runId: "11111111-1111-4111-8111-111111111111", + adapterType: "cursor", runtimeMode: "legacy", abortRequested: true, + }); + Object.assign(error, { providerResponse: "private fixture payload" }); + captureException(error); + const unrelated = new Error("unrelated"); + captureException(unrelated); + expect(sdk.captureException.mock.calls[0]).toEqual([ + expect.objectContaining({ message: error.message, stack: error.stack }), + { tags: { error_code: "adapter_stop_unconfirmed" }, fingerprint: ["{{ default }}"], contexts: { + adapter_stop: { runId: "11111111-1111-4111-8111-111111111111", adapterType: "cursor", runtimeMode: "legacy", abortRequested: true, timeoutMs: 60_000 }, + } }, + ]); + expect(JSON.stringify(sdk.captureException.mock.calls[0])).not.toContain("private fixture payload"); + expect(sdk.captureException.mock.calls[1]).toEqual([unrelated]); + }); + + it("does not send arbitrary Stop diagnostic values", async () => { + process.env[BACKEND_DSN_ENV] = "https://fixture@example.com/1"; + const sdk = mockSentryPackage(); + const { captureException, sentryReady } = await importFreshSentry(); + const { AdapterStopTimeoutError } = await import("../services/adapter-stop-timeout.js"); + await sentryReady; + captureException(new AdapterStopTimeoutError(NaN, { + runId: "private fixture payload", adapterType: "private fixture payload", runtimeMode: "private fixture payload", + })); + expect(JSON.stringify(sdk.captureException.mock.calls)).not.toContain("private fixture payload"); + expect(sdk.captureException.mock.calls[0]).toEqual([expect.any(Error), expect.objectContaining({ contexts: { + adapter_stop: { runId: null, adapterType: "unknown", runtimeMode: "unknown", abortRequested: null, timeoutMs: null }, + } })]); + }); + it("is a no-op and does not throw when the gate is closed", async () => { const { captureException, sentryReady } = await importFreshSentry(); await sentryReady; diff --git a/server/src/sentry.ts b/server/src/sentry.ts index ee7f28f49d5..0a4ad5d678b 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -56,6 +56,7 @@ // `instrumentation.ts`. import os from "node:os"; +import { AdapterStopTimeoutError } from "./services/adapter-stop-timeout.js"; import type { RunFailureDiagnostics } from "./services/run-failure-diagnostics.js"; import { readBuildCommit } from "./build-commit.js"; import { checkExactPeerVersions } from "./peer-version-check.js"; @@ -106,7 +107,18 @@ export const sentryReady: Promise = dsn ? bootstrapSentry(dsn) : Promise.r export function captureException(error: unknown): void { if (!sentryHandle) return; try { - sentryHandle.captureException(error); + if (error instanceof AdapterStopTimeoutError) { + // Event-local, fixed-shape context: no ambient scope or raw error fields. + const exception = new Error(error.message); + exception.stack = error.stack; + sentryHandle.captureException(exception, { + tags: { error_code: "adapter_stop_unconfirmed" }, + contexts: { adapter_stop: { ...error.diagnostics } }, + fingerprint: ["{{ default }}"], + }); + } else { + sentryHandle.captureException(error); + } } catch (err) { // eslint-disable-next-line no-console console.error("[paperclip] Sentry captureException failed", err); diff --git a/server/src/services/adapter-execution-control.test.ts b/server/src/services/adapter-execution-control.test.ts index 1a93d91a021..93c7143d39f 100644 --- a/server/src/services/adapter-execution-control.test.ts +++ b/server/src/services/adapter-execution-control.test.ts @@ -1,4 +1,5 @@ import { afterEach, expect, it, vi } from "vitest"; +import { AdapterStopTimeoutError } from "./adapter-stop-timeout.js"; import { adapterExecutionControls, captureAdapterStopOwnership, @@ -120,3 +121,33 @@ it("bounds Stop when an adapter does not settle", async () => { await assertion; expect(vi.getTimerCount()).toBe(0); }); + +it("records the exact unconfirmed run without finishing or removing its control", async () => { + vi.useFakeTimers(); + const runId = "11111111-1111-4111-8111-111111111111"; + const control = createAdapterExecutionControl(); + await registerAdapterExecutionControl(runId, control); + control.controller.abort(); + let finished = false; + void control.settled.then(() => { finished = true; }); + const result = waitForAdapterStop(control.settled, 1000, { + runId, adapterType: "claude_local", runtimeMode: "legacy", abortRequested: control.controller.signal.aborted, + }).catch((error: unknown) => error); + try { + await vi.advanceTimersByTimeAsync(1000); + const error = await result; + expect(error).toBeInstanceOf(AdapterStopTimeoutError); + expect((error as AdapterStopTimeoutError).diagnostics).toEqual({ + runId, adapterType: "claude_local", runtimeMode: "legacy", abortRequested: true, timeoutMs: 1000, + }); + expect(adapterExecutionControls.get(runId)).toBe(control); + expect(finished).toBe(false); + expect(vi.getTimerCount()).toBe(0); + control.finish(); + await control.settled; + expect(finished).toBe(true); + } finally { + control.finish(); + adapterExecutionControls.delete(runId); + } +}); diff --git a/server/src/services/adapter-execution-control.ts b/server/src/services/adapter-execution-control.ts index b648e080117..a2964c7e84d 100644 --- a/server/src/services/adapter-execution-control.ts +++ b/server/src/services/adapter-execution-control.ts @@ -1,3 +1,5 @@ +import { AdapterStopTimeoutError, type AdapterStopContext } from "./adapter-stop-timeout.js"; + /** Live adapter ownership shared by routes and scheduler service instances. */ export function createAdapterExecutionControl() { const controller = new AbortController(); @@ -67,6 +69,7 @@ export async function registerAdapterExecutionControl( export async function waitForAdapterStop( settled: Promise, timeoutMs = 60_000, + diagnostics?: AdapterStopContext, ) { let timer: ReturnType | undefined; try { @@ -76,9 +79,7 @@ export async function waitForAdapterStop( timer = setTimeout( () => reject( - new Error( - "Execution is still stopping; termination has not been verified.", - ), + new AdapterStopTimeoutError(timeoutMs, diagnostics), ), timeoutMs, ); diff --git a/server/src/services/adapter-stop-timeout.ts b/server/src/services/adapter-stop-timeout.ts new file mode 100644 index 00000000000..d90ed90bfce --- /dev/null +++ b/server/src/services/adapter-stop-timeout.ts @@ -0,0 +1,27 @@ +import { AGENT_ADAPTER_TYPES } from "@paperclipai/shared"; + +export interface AdapterStopContext { + runId?: string; + adapterType?: string; + runtimeMode?: string; + abortRequested?: boolean; +} + +/** Diagnostic identity only. This error never acknowledges termination. */ +export class AdapterStopTimeoutError extends Error { + readonly diagnostics: Readonly>; + + constructor(timeoutMs: number, context?: AdapterStopContext) { + super("Execution is still stopping; termination has not been verified."); + // Keep the existing Error name and timer stack for default error grouping. + Error.captureStackTrace?.(this, AdapterStopTimeoutError); + this.diagnostics = Object.freeze({ + timeoutMs: Number.isSafeInteger(timeoutMs) && timeoutMs >= 0 ? timeoutMs : null, + runId: typeof context?.runId === "string" && /^[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}$/i.test(context.runId) + ? context.runId : null, + adapterType: AGENT_ADAPTER_TYPES.some((type) => type === context?.adapterType) ? context!.adapterType! : "unknown", + runtimeMode: context?.runtimeMode === "native" || context?.runtimeMode === "legacy" ? context.runtimeMode : "unknown", + abortRequested: typeof context?.abortRequested === "boolean" ? context.abortRequested : null, + }); + } +} diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 490101f2fd0..9b2d4fde920 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -29266,7 +29266,12 @@ export function heartbeatService( } if (control) { - await waitForAdapterStop(control.settled); + await waitForAdapterStop(control.settled, undefined, { + runId: run.id, + adapterType: agent?.adapterType, + runtimeMode: run.runtimeMode, + abortRequested: control.controller.signal.aborted, + }); const stopped = await getRun(run.id); if (stopped && isHeartbeatRunTerminalStatus(stopped.status)) { if (