diff --git a/doc/observability.md b/doc/observability.md index 5c1e57c1784..e8d1262a0bb 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -269,9 +269,9 @@ environment variables: `SENTRY_DSN_FRONTEND` for the browser and `SENTRY_DSN_BACKEND` for the server. Each variable is optional. A specific variable always wins for its own component; a legacy variable, `SENTRY_DSN`, supplies a component that has no specific value set. An -empty string counts as absent for all three variables. The feature uses -built-in Sentry options only. It adds no `beforeSend` hook and no custom -filter code. +empty string counts as absent for all three variables. SDK configuration uses +built-in Sentry options only. It adds no `beforeSend` hook. The run-failure +reporter selects reportable outcomes before calling the SDK, as described below. The server is inactive when the backend DSN resolves to `null`; then it imports no Sentry package. The browser is inactive when the front-end DSN @@ -434,6 +434,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. +Known missing-secret configuration blockers are kept in the task's run log, +blocked state, and owner recovery action, without a Sentry run-failure event. +This requires a failed `configuration_incomplete` run in the preparing stage, +a setup-phase report, explicit proof that provider work did not start, and a +nonempty list of recognized missing or inactive secret bindings. Process exit +evidence, unknown binding reasons, secret-provider failures, ambiguous missing +secret-definition lookups, and workspace failures remain reportable. This filter +does not change task recovery, credentials, or execution policy. + 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; diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index cc60102766a..86b3867ae50 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -5,6 +5,7 @@ import { legacyDispositionFingerprint, LEGACY_DISPOSITION_REPAIR_INSTRUCTION } f import * as controllerLeases from "../services/legacy-controller-lease.js"; import * as instructionWorkingCopies from "../services/agent-instruction-working-copies.js"; import * as runEvents from "../services/heartbeat-run-events.js"; +import { waitForPendingRunFailureReports } from "../services/run-failure-report.js"; import { instanceSettingsService } from "../services/instance-settings.js"; import { createHash, randomUUID } from "node:crypto"; import { terminalizeLegacyExecution } from "../services/legacy-execution-recovery.js"; @@ -5688,6 +5689,9 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { ); }); expect(configurationComment).toBeTruthy(); + await heartbeat.waitForRunExecutionDrain(runId); + await waitForPendingRunFailureReports(); + expect(mockCaptureRunFailure.mock.calls.filter(([event]) => event.runId === runId)).toEqual([]); }); it("queues one finish-handoff wake when a successful run leaves in-progress work without a next action", async () => { diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index a717dfd3479..9176bdb659b 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -110,6 +110,99 @@ describeEmbeddedPostgres("reportRunFailure", () => { } as unknown as typeof heartbeatRuns.$inferSelect; } + function missingSecretRun(overrides: Partial = {}) { + return buildRun({ + errorCode: "configuration_incomplete", + executionStage: "preparing", + resultJson: { + configurationIncomplete: { + reason: "secret_binding_missing", + missingBindings: [{ bindingType: "user_secret_ref", errorCode: "user_secret_missing" }], + }, + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + }, + ...overrides, + }); + } + + it.each([ + { bindingType: "secret_ref" }, + { bindingType: "secret_ref", errorCode: "binding_missing" }, + ...["binding_missing", "responsible_user_missing", "user_secret_missing", "secret_inactive", "user_secret_definition_inactive"] + .map((errorCode) => ({ bindingType: "user_secret_ref", errorCode })), + ])("keeps the known pre-dispatch secret blocker local: %j", async (binding) => { + await seedCompanyAndAgent(); + const run = missingSecretRun(); + (run.resultJson!.configurationIncomplete as Record).missingBindings = [binding]; + const saved = structuredClone(run); + + await reportRunFailure(db, run, { phase: "setup" }); + + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + expect(run).toEqual(saved); + }); + + it.each([ + {}, + { reason: "secret_binding_missing", missingBindings: [] }, + { reason: "secret_binding_missing", missingBindings: [null] }, + { reason: "secret_binding_missing", missingBindings: [{ errorCode: "user_secret_missing" }] }, + { reason: "secret_binding_missing", missingBindings: [{ bindingType: "user_secret_ref", errorCode: "provider_error" }] }, + { reason: "secret_binding_missing", missingBindings: [{ bindingType: "user_secret_ref", errorCode: "user_secret_definition_missing" }] }, + { reason: "secret_binding_missing", missingBindings: [{ bindingType: "user_secret_ref", errorCode: "new_unknown_reason" }] }, + { reason: "secret_binding_missing", missingBindings: [ + { bindingType: "user_secret_ref", errorCode: "user_secret_missing" }, + { bindingType: "secret_ref", errorCode: "provider_error" }, + ] }, + { reason: "ai_connection_unavailable" }, + ])("reports ambiguous, unknown, and provider failures: %j", async (configurationIncomplete) => { + await seedCompanyAndAgent(); + const run = missingSecretRun(); + run.resultJson!.configurationIncomplete = configurationIncomplete; + + await reportRunFailure(db, run, { phase: "setup" }); + + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); + + it.each([ + { status: "timed_out" }, + { errorCode: "setup_failed" }, + { errorCode: "workspace_validation_failed" }, + { errorCode: "adapter_failed" }, + { executionStage: "executing" }, + { executionStage: null }, + { exitCode: 1 }, + { exitCode: 0 }, + { signal: "SIGTERM" }, + ])("reports missing-secret text without a proven pre-dispatch outcome: %j", async (overrides) => { + await seedCompanyAndAgent(); + + await reportRunFailure(db, missingSecretRun(overrides), { phase: "setup" }); + + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); + + it.each([undefined, { kind: "bootstrap" }, { kind: "bootstrap", providerWorkStarted: true }])( + "reports missing-secret failures without explicit bootstrap proof: %j", async (executionRecovery) => { + await seedCompanyAndAgent(); + const run = missingSecretRun(); + run.resultJson!.executionRecovery = executionRecovery; + + await reportRunFailure(db, run, { phase: "setup" }); + + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }, + ); + + it.each([undefined, "execute"] as const)("reports configuration failures outside setup: %s", async (phase) => { + await seedCompanyAndAgent(); + + await reportRunFailure(db, missingSecretRun(), { phase }); + + expect(mockCaptureRunFailure).toHaveBeenCalledTimes(1); + }); + it("captures once for the status failed", async () => { await seedCompanyAndAgent(); const run = buildRun({ status: "failed" }); diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index 2ed65286119..9c341eeca6a 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -40,6 +40,50 @@ function readTaskId(run: HeartbeatRun): string | null { return typeof contextIssueId === "string" && contextIssueId.length > 0 ? contextIssueId : null; } +function asRecord(value: unknown): Record | null { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? value as Record + : null; +} + +/** A known owner action before dispatch, not a secret-provider or runtime failure. */ +function isMissingSecretConfigurationBlocker(run: HeartbeatRun, options: RunFailureReportOptions): boolean { + if ( + run.status !== "failed" || + run.errorCode !== "configuration_incomplete" || + run.executionStage !== "preparing" || + options.phase !== "setup" || + run.exitCode != null || + run.signal != null + ) return false; + + const configuration = asRecord(run.resultJson?.configurationIncomplete); + const recovery = asRecord(run.resultJson?.executionRecovery); + if ( + configuration?.reason !== "secret_binding_missing" || + recovery?.kind !== "bootstrap" || + recovery.providerWorkStarted !== false || + !Array.isArray(configuration.missingBindings) || + configuration.missingBindings.length === 0 + ) return false; + + return configuration.missingBindings.every((value: unknown) => { + const binding = asRecord(value); + if (!binding) return false; + if (binding.bindingType === "secret_ref") { + return binding.errorCode == null || binding.errorCode === "binding_missing"; + } + if (binding.bindingType !== "user_secret_ref") return false; + // Missing-definition resolution can also catch a database error. Keep that + // ambiguous case, unknown codes, and provider errors visible in Sentry. + return binding.errorCode === "binding_missing" || + binding.errorCode === "responsible_user_missing" || + binding.errorCode === "user_secret_missing" || + binding.errorCode === "secret_inactive" || + binding.errorCode === "user_secret_definition_inactive"; + }); +} + /** * Report a terminal run failure to Sentry. Returns at once for any status * other than failures and unexpected started cancellations. Never throws — a Sentry failure or a @@ -54,6 +98,7 @@ function readTaskId(run: HeartbeatRun): string | null { export function reportRunFailure(db: Db, run: HeartbeatRun, options: RunFailureReportOptions = {}): Promise { if (!isRunFailureStatus(run.status)) return Promise.resolve(); if (run.status === "cancelled" && !isUnexpectedRunCancellation(run)) return Promise.resolve(); + if (isMissingSecretConfigurationBlocker(run, options)) return Promise.resolve(); const runStatus = run.status; const report = captureTerminalRunFailure(db, run, runStatus, options); pendingRunFailureReports.add(report);