diff --git a/.github/workflows/sentry-contract.yml b/.github/workflows/sentry-contract.yml index f4bb2bcebbb..87515150a0e 100644 --- a/.github/workflows/sentry-contract.yml +++ b/.github/workflows/sentry-contract.yml @@ -47,7 +47,14 @@ jobs: version: 9.15.4 - name: Install workspace dependencies - run: pnpm install --frozen-lockfile + # Master owns the lockfile; PR verification can start before its refresh. + # Resolve like the normal PR jobs, but this narrow contract needs no + # dependency or workspace lifecycle scripts, including on the fast path. + run: | + if ! pnpm install --frozen-lockfile --ignore-scripts; then + pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile + pnpm install --frozen-lockfile --ignore-scripts + fi - name: Install the audited optional SDK outside the workspace shell: bash diff --git a/doc/observability.md b/doc/observability.md index 491e8e35c85..8a379342a1d 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -406,6 +406,15 @@ They do not change the ambient Sentry scope, whose isolation is unavailable without an OpenTelemetry context manager. Later, unrelated exceptions must not inherit a previous run's identity or fingerprint. +The `run_failure` context also includes the recorded process `exitCode` and +`signal`, so a generic adapter error can still distinguish a nonzero exit from +a signal termination. Exit codes must fit the database's signed 32-bit integer; +missing or malformed values become `null`. Signals must match the reporting +host's Node signal constants; missing values become `null` and unrecognized +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. + ### Browser data The browser sends no page URL, no referrer, no user agent, and no 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 524b52445c0..5cfe758e2bb 100644 --- a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts +++ b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts @@ -62,6 +62,8 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", errorCode: "adapter_failed", agentAdapter: "fixture-adapter", runStatus: "failed" as const, + exitCode: 1, + signal: null, }; const second = { ...first, @@ -71,15 +73,23 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", errorCode: "timeout", agentAdapter: "other-adapter", runStatus: "timed_out" as const, + exitCode: null, + signal: "SIGKILL", }; captureRunFailure(first); await Promise.resolve(); captureException(new Error("unrelated database error")); captureRunFailure(second); + captureRunFailure({ + ...first, + errorMessage: "malformed process metadata", + exitCode: NaN, + signal: "private-signal-payload", + }); captureException(new Error("unrelated filesystem error")); await Sentry.flush(2000); - expect(events).toHaveLength(4); + expect(events).toHaveLength(5); const captured = (message: string) => events.find((event) => (event.exception as { values: Array<{ value: string }> }).values[0]?.value === message, ); @@ -88,10 +98,16 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", tags: { run_id: run.runId, task_id: run.taskId, error_code: run.errorCode, agent_adapter: run.agentAdapter, run_status: run.runStatus }, contexts: { run_failure: { runId: run.runId, taskId: run.taskId, - errorMessage: run.errorMessage, errorCode: run.errorCode, agentAdapter: run.agentAdapter } }, + errorMessage: run.errorMessage, errorCode: run.errorCode, agentAdapter: run.agentAdapter, + exitCode: run.exitCode, signal: run.signal } }, fingerprint: [run.errorCode, run.agentAdapter], }); } + expect(captured("malformed process metadata")).toMatchObject({ + contexts: { run_failure: { exitCode: null, signal: "unknown" } }, + fingerprint: [first.errorCode, first.agentAdapter], + }); + expect(JSON.stringify(events)).not.toContain("private-signal-payload"); for (const message of ["unrelated database error", "unrelated filesystem error"]) { const event = captured(message); expect(event).toBeDefined(); diff --git a/server/src/__tests__/run-failure-sentry.test.ts b/server/src/__tests__/run-failure-sentry.test.ts index 855b6b66853..d5285698a43 100644 --- a/server/src/__tests__/run-failure-sentry.test.ts +++ b/server/src/__tests__/run-failure-sentry.test.ts @@ -119,7 +119,7 @@ describe("captureRunFailure", () => { expect(captureException.mock.calls[0]![1]?.fingerprint).toEqual(["unknown", "claude-code"]); }); - it("sets the five diagnostic values on the run_failure context", async () => { + it("keeps missing process exit evidence null on the run_failure context", async () => { const { sentryModule, captureException } = await importFreshSentryWithGateOpen(); const event = baseEvent(); @@ -131,9 +131,54 @@ describe("captureRunFailure", () => { errorMessage: event.errorMessage, errorCode: event.errorCode, agentAdapter: event.agentAdapter, + exitCode: null, + signal: null, }); }); + it.each([ + { exitCode: 1, signal: null }, + { exitCode: 0, signal: null }, + { exitCode: -1, signal: null }, + { exitCode: -2147483648, signal: null }, + { exitCode: 2147483647, signal: null }, + { exitCode: null, signal: "SIGTERM" }, + { exitCode: null, signal: "SIGKILL" }, + ])("retains process exit evidence without changing grouping: %j", async (processExit) => { + const { sentryModule, captureException } = await importFreshSentryWithGateOpen(); + const event = baseEvent({ ...processExit, errorMessage: "Adapter failed", errorCode: "adapter_failed" }); + + sentryModule.captureRunFailure(event); + + const [error, context] = captureException.mock.calls[0]!; + expect((error as Error).message).toBe("Adapter failed"); + expect(context?.contexts.run_failure).toMatchObject(processExit); + expect(context?.fingerprint).toEqual(["adapter_failed", event.agentAdapter]); + expect(context?.tags).not.toHaveProperty("exitCode"); + expect(context?.tags).not.toHaveProperty("signal"); + }); + + it.each([ + ["private-exit-payload", "SIGTERM private-signal-payload"], + ["1", "constructor"], + [true, "__proto__"], + [1.5, "SIGCUSTOM"], + [NaN, ""], + [Infinity, 9], + [2147483648, { private: "signal-payload" }], + [-2147483649, ["SIGTERM"]], + ])("rejects malformed process exit fields (%j, %j)", async (exitCode, signal) => { + const { sentryModule, captureException } = await importFreshSentryWithGateOpen(); + + sentryModule.captureRunFailure({ ...baseEvent(), exitCode, signal } as RunFailureEvent); + + expect(captureException.mock.calls[0]![1]?.contexts.run_failure).toMatchObject({ + exitCode: null, + signal: "unknown", + }); + expect(JSON.stringify(captureException.mock.calls)).not.toContain("private-"); + }); + it("does not set an instance key on the run_failure context", async () => { const { sentryModule, captureException } = await importFreshSentryWithGateOpen(); diff --git a/server/src/sentry.ts b/server/src/sentry.ts index 424ba16f2e3..1a9c30f4642 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -132,6 +132,28 @@ export interface RunFailureEvent { agentAdapter: string; /** The run status that triggered this report. */ runStatus: RunFailureStatus; + /** Recorded process exit evidence, when available. Validated before capture. */ + exitCode?: number | null; + signal?: string | null; +} + +function normalizeRunExitCode(value: unknown): number | null { + // Match the persisted PostgreSQL integer. Do not coerce adapter-supplied text. + return typeof value === "number" && + Number.isInteger(value) && + value >= -2147483648 && + value <= 2147483647 + ? value + : null; +} + +function normalizeRunSignal(value: unknown): string | null { + if (value == null) return null; + // Only host signal constants may leave the process; arbitrary adapter text + // can contain output or credentials even when stored in the signal column. + return typeof value === "string" && Object.hasOwn(os.constants.signals, value) + ? value + : "unknown"; } /** @@ -167,6 +189,8 @@ export function captureRunFailure(event: RunFailureEvent): void { errorMessage: event.errorMessage, errorCode, agentAdapter: event.agentAdapter, + exitCode: normalizeRunExitCode(event.exitCode), + signal: normalizeRunSignal(event.signal), }, }, fingerprint: [errorCode, event.agentAdapter], diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index 75ba13de1fe..9b32245e763 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -112,6 +112,35 @@ describeEmbeddedPostgres("reportRunFailure", () => { ); }); + it("forwards stored exit evidence even when the adapter error is generic", async () => { + await seedCompanyAndAgent(); + for (const processExit of [ + { exitCode: 1, signal: null }, + { exitCode: null, signal: "SIGTERM" }, + { exitCode: null, signal: null }, + ]) { + mockCaptureRunFailure.mockClear(); + await reportRunFailure(db, buildRun({ + error: "Adapter failed", + errorCode: "adapter_failed", + ...processExit, + stdoutExcerpt: "private-output", + stderrExcerpt: "private-error-output", + resultJson: { private: "adapter-result" }, + })); + + expect(mockCaptureRunFailure).toHaveBeenCalledWith(expect.objectContaining({ + errorMessage: "Adapter failed", + errorCode: "adapter_failed", + ...processExit, + })); + const captured = mockCaptureRunFailure.mock.calls[0][0]; + expect(captured).not.toHaveProperty("stdoutExcerpt"); + expect(captured).not.toHaveProperty("stderrExcerpt"); + expect(captured).not.toHaveProperty("resultJson"); + } + }); + it("captures nothing for succeeded, cancelled, and interrupted", async () => { await seedCompanyAndAgent(); for (const status of ["succeeded", "cancelled", "interrupted"] as const) { diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index 98aa9193111..7e403455138 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -90,6 +90,8 @@ async function captureTerminalRunFailure( : sanitizeAdapterText(run.errorCode, MAX_ERROR_CODE_LENGTH), agentAdapter: agent?.adapterType ?? UNKNOWN_ADAPTER, runStatus, + exitCode: run.exitCode, + signal: run.signal, }); } catch (err) { logger.warn({ err, runId: run.id }, "failed to report run failure to Sentry");