mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
Keep pre-dispatch secret configuration blockers out of Sentry (#15472)
Keep confirmed pre-dispatch missing-secret setup blockers out of Sentry while preserving failed runs and owner recovery actions. Runtime, provider and ambiguous failures remain reportable. Verified full exact-head CI, focused local regressions, workspace typecheck and independent review; Greptile 5/5 with no unresolved comments. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ae6f95ed7a
commit
43b0aff54b
4 files changed
+154
-3
No files matched your search
+12
-3
@@ -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;
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -110,6 +110,99 @@ describeEmbeddedPostgres("reportRunFailure", () => {
|
||||
} as unknown as typeof heartbeatRuns.$inferSelect;
|
||||
}
|
||||
|
||||
function missingSecretRun(overrides: Partial<typeof heartbeatRuns.$inferSelect> = {}) {
|
||||
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<string, unknown>).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" });
|
||||
|
||||
@@ -40,6 +40,50 @@ function readTaskId(run: HeartbeatRun): string | null {
|
||||
return typeof contextIssueId === "string" && contextIssueId.length > 0 ? contextIssueId : null;
|
||||
}
|
||||
|
||||
function asRecord(value: unknown): Record<string, unknown> | null {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value)
|
||||
? value as Record<string, unknown>
|
||||
: 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<void> {
|
||||
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);
|
||||
|
||||
Reference in new issue
Block a user