Files
PaperClipAI/server/src/sentry.ts
DottaandPaperclip 7a52dcdc74 fix: repair MCP validation and cancelled execution recovery (#14951)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The tool gateway gives agents access to connected services. Recovery
controls what happens when a run stops.
> - Generated tool names can exceed the provider limit after the MCP
client adds its prefix.
> - The same invalid definition can fail each automatic retry. A
cancelled run can also hold saved messages without showing its cause.
> - This pull request bounds tool names, stops configuration retries,
and retains cancellation evidence.
> - It shows the stopped run and admits saved input only after the
existing safety checks pass.
> - The benefit is a clear recovery path that preserves operator Stop
and prevents duplicate message delivery.

## Linked Issues or Issue Description

**What happened?**

A long connected MCP tool name makes the provider reject the entire
request. Automatic recovery repeats the invalid request. Separately,
unexpected legacy cancellations can leave saved input behind a recovery
hold. The notice does not identify the stopped run or its cause.

**Expected behavior**

Complete MCP names fit the provider limit. Tool-definition errors
require configuration repair. Cancelled runs retain their source and
reason. The recovery notice shows the cause and saved-message count.
Verified unexpected cancellations can start a fresh turn through the
existing admission checks.

**Steps to reproduce**

1. Assign an App gallery connection with a long application key and tool
name to a Claude agent.
2. Start a run. The provider rejects a name over 128 characters,
including its MCP prefix.
3. For cancellation recovery, stop a legacy provider turn without an
operator Stop request and send a user message while the recovery hold is
active.
4. Inspect the recovery notice and the deferred message queue.

**Paperclip version or commit**

Rebased onto master at `cf8ad63c806685bfd7c48e3ed4a919d61a7c55f1`.

**Deployment mode**

Hosted or self-hosted server with legacy Claude or Codex execution.

Related public work:

- Refs #14017. That PR caps name segments. This PR preserves existing
short names and uses stable hash aliases for long complete names. It
also covers classification and recovery.
- Refs #4510. That PR adds a cancellation-source column. This PR records
bounded evidence in the existing run result, without a migration.
- Refs #12552 and #4506. Those PRs suppress recovery after operator
cancellation. This PR preserves operator intent and uses the existing
continuation gates.

## What Changed

- Bound gateway names with the full provider prefix in the 128-character
budget. Retain the original upstream tool name for dispatch and
permissions.
- Classify invalid tool definitions as configuration failures before
diagnostic redaction. Stop automatic retries and continuation attempts
for that error code.
- Persist cancellation source, expectedness, initiator, reason, and
time. Preserve recorded Stop intent when adapter results arrive. Report
unexpected started cancellations with closed diagnostic labels.
- Show the run cause, saved-message count, and Inspect run link. Offer
Continue for eligible unexpected cancellations. Require verified
provider stop, empty tool inventory, ownership, and the existing pause,
budget, approval, and dependency gates. Use the existing queue for
single delivery.
- Add regression coverage and update the execution, MCP gateway, and
run-log documentation.

## Verification

- `pnpm -r typecheck` and `pnpm build` passed.
- `pnpm check:token-gates` passed.
- Ran `pnpm test:run` and completed its workspace and serialized groups.
Initial resource and timing failures passed on isolated reruns. All 149
serialized route suites passed.
- Reran the changed server, adapter, and UI suites after the rebase.
Coverage includes long-name upstream dispatch, configuration retry
suppression, cancellation evidence retention, privacy labels, oversized
run projection, and concurrent saved-message delivery.
- `pnpm test:e2e tests/e2e/legacy-failure-continuation.spec.ts` passed
all six browser scenarios. The recovery notice shows the run cause and
inspection link, and each recovery entry point reaches one new response.
- Added database-backed checks for active, removed, paused, unavailable,
and disabled chat connections. The final continuation and
recovery-notice suites passed 167 tests. Externally bound chats hide
board Continue and show a usable next action.
- All 55 GitHub checks passed on
`42afbf1371dcaeb72646e3d8f65c19ff7cddf8de`. Two unrelated Storybook jobs
were skipped by their normal conditions. Greptile reviewed that commit
at 5/5 with no findings and no open review threads.

## Risks

- Long tool names change to aliases. Existing short names stay
compatible. The original connection and upstream name remain the
dispatch authority.
- Invalid tool definitions no longer get automatic retries. An operator
must repair the configuration before a new attempt.
- Continuation changes apply only to positively identified unexpected
legacy cancellations with complete empty tool inventory. Operator Stop,
unknown historical cancellations, outstanding tools, and unverified
provider termination keep their holds.
- No database migration. The added projection fields are optional.
Cancellation reason and initiator IDs remain local run evidence; Sentry
receives only closed source and initiator-type labels and expectedness.

## Model Used

- OpenAI GPT-6 through Codex, with reasoning, repository editing, shell
execution, and GitHub tool use. The runtime does not expose the exact
model variant or context-window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-02 13:47:59 -05:00

371 lines
15 KiB
TypeScript

// Optional Sentry error monitoring for the server process.
//
// Activated only when the backend DSN resolves to a value — see
// `resolveSentryDsns` in `sentry-dsn.ts` for the precedence between
// `SENTRY_DSN_BACKEND` and the legacy `SENTRY_DSN` fallback. When it
// resolves to `null`, no Sentry package is loaded at all.
//
// The import is dynamic and the package is an optional runtime dependency —
// operators who want server-side error monitoring install `@sentry/node`
// themselves. That keeps Sentry off the default dependency graph and avoids
// forcing a lockfile bump for an opt-in feature. This gate mirrors the
// OpenTelemetry gate in `instrumentation.ts`.
//
// OpenTelemetry keeps ownership of trace setup: the initializer passes
// `skipOpenTelemetrySetup: true` and `tracesSampleRate: 0`, so this module
// adds error monitoring only and starts no span or trace behavior of its
// own.
//
// `serverName`: the initializer sets this to the host name of the process,
// with `os.hostname()`. The `@sentry/node` client already falls back to the
// same host name when the caller omits this option, so this line makes an
// existing default explicit instead of changing captured event content. An
// explicit value stays correct if a future SDK version changes its default.
//
// An operator can still send a different value in place of the host name.
// When the environment variable `SENTRY_NAME` holds a non-empty string, the
// initializer uses that value instead. This keeps the same order the
// `@sentry/node` client itself uses when the caller omits `serverName`.
//
// Default-integration privacy note: `sendDefaultPii: false` filters values
// by name, inside the `RequestData` integration only. Three other default
// integrations copy raw values past that filter, so the initializer removes
// or narrows them with built-in Sentry options — no custom filter code:
// - `Console` turns a `console.*` call into a breadcrumb with the raw
// arguments. The initializer drops it.
// - `ContextLines` reads local source lines around each stack frame off
// the host disk. The initializer drops it.
// - `Http` records a breadcrumb for each outbound request, with its URL
// and query string. The initializer keeps the integration (`RequestData`
// and request isolation need it) and turns the breadcrumb off with the
// integration's own `breadcrumbs` option.
//
// `onUnhandledRejectionIntegration` defaults to `mode: "warn"`, which
// registers a `process.on("unhandledRejection")` listener. Node cancels its
// own crash-on-unhandled-rejection behavior when any listener is registered.
// The server relies on that crash today, so the initializer passes
// `mode: "strict"`: Sentry still captures the event, then exits the process,
// so the existing crash-and-restart behavior stays.
//
// Before it imports the package, the bootstrap checks the installed
// `@sentry/node` version against the exact version this manifest's
// `peerDependencies` declares — the same audited version documented in
// `doc/observability.md`. A missing or a mismatched version logs one
// diagnostic and leaves the server running without error monitoring; it
// never throws. This gate mirrors the OpenTelemetry gate in
// `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";
import { resolveSentryDsns } from "./sentry-dsn.js";
const { backend: dsn, legacyFallbackUsed } = resolveSentryDsns();
if (legacyFallbackUsed) {
// eslint-disable-next-line no-console
console.warn(
"[paperclip] SENTRY_DSN_FRONTEND or SENTRY_DSN_BACKEND is not set. " +
"The server uses the legacy SENTRY_DSN value for the affected " +
"component. Set SENTRY_DSN_FRONTEND and SENTRY_DSN_BACKEND to send " +
"each component to its own Sentry project.",
);
}
/** Event-local context accepted by the optional Sentry package. */
interface SentryCaptureContext {
tags: Record<string, string>;
contexts: Record<string, Record<string, unknown>>;
fingerprint: string[];
}
/** The subset of the `@sentry/node` client surface this gate calls. */
interface SentryHandle {
captureException(error: unknown, context?: SentryCaptureContext): string;
close(timeout?: number): Promise<boolean>;
}
let sentryHandle: SentryHandle | null = null;
let shutdownPromise: Promise<void> | null = null;
/**
* Resolves once the Sentry SDK has started, or once bootstrap has failed and
* logged, or at once when the backend DSN resolves to `null`. No caller
* needs to await this before calling `captureException` — it is a no-op
* until ready — but `index.ts` awaits it at startup so the first real error
* has a live client.
*/
export const sentryReady: Promise<void> = dsn ? bootstrapSentry(dsn) : Promise.resolve();
/**
* Report an error to Sentry. A no-op before the gate opens, when the gate
* never opens (the backend DSN resolves to `null`), or when bootstrap
* failed. Never throws — observability must not change control flow.
*/
export function captureException(error: unknown): void {
if (!sentryHandle) return;
try {
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);
}
}
/** The run status values that mark a run as a genuine terminal failure. */
export type RunFailureStatus = "failed" | "timed_out" | "cancelled";
/**
* The diagnostic values `captureRunFailure` sends with a terminal-failure
* event. `errorCode` is `null` when the run holds no error code.
*/
export interface RunFailureEvent {
/** The task UUID the run belongs to. */
taskId: string;
/** The `heartbeat_runs` row id. */
runId: string;
/** The redacted error message. */
errorMessage: string;
/** The run's error code, or `null` when the run holds none. */
errorCode: string | null;
/** The agent's adapter type, or `"unknown"` when the agent row is absent. */
agentAdapter: string;
/** The run status that triggered this report. */
runStatus: RunFailureStatus;
/** Bounded, redacted diagnostics selected by the run failure reporter. */
diagnostics?: RunFailureDiagnostics;
/** 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";
}
/**
* Report one terminal run failure to Sentry. A no-op before the gate opens
* or when the gate never opens. Never throws — observability must not
* change run control flow.
*
* Sets the fingerprint to `[errorCode, agentAdapter]`, in that order, so
* Sentry groups events by error code and adapter. The error message stays
* out of the fingerprint — it still travels as the exception message and as
* a field of the `run_failure` context.
*/
export function captureRunFailure(event: RunFailureEvent): void {
if (!sentryHandle) return;
const handle = sentryHandle;
try {
const errorCode = event.errorCode ?? "unknown";
// Sentry's async scope isolation is absent when OTel setup is skipped.
// A withScope mutation can then persist into unrelated later captures.
// Pass these fields on this event only; do not mutate the ambient scope.
const diagnostics = event.diagnostics;
const contexts: Record<string, Record<string, unknown>> = {};
if (diagnostics) {
contexts.run_execution = { ...diagnostics.execution, truncatedFields: diagnostics.truncatedFields };
if (Object.keys(diagnostics.adapter).length) contexts.adapter_failure = diagnostics.adapter;
if (Object.keys(diagnostics.provider).length) contexts.provider_failure = diagnostics.provider;
diagnostics.exceptions.forEach(({ name, code, status, requestId }, index) => {
contexts[`run_exception_${index}`] = { name, code, status, requestId };
});
}
// Rebuild only sanitized exception fields. Passing a raw SDK Error can
// serialize its request/response, headers, or other enumerable properties.
let cause: Error | undefined;
for (const entry of [...(diagnostics?.exceptions ?? [])].reverse()) {
const error: Error = new Error(entry.message ?? event.errorMessage, cause ? { cause } : undefined);
error.name = entry.name ?? "Error";
error.stack = entry.stack;
cause = error;
}
const exception = cause ?? new Error(event.errorMessage);
if (!cause) {
// A saved adapter result is not a thrown Error. Do not pretend that the
// reporter's own stack is the failure location.
exception.stack = typeof diagnostics?.adapter.stackPreview === "string"
? diagnostics.adapter.stackPreview : undefined;
}
handle.captureException(exception, {
tags: {
run_id: event.runId,
task_id: event.taskId,
error_code: errorCode,
agent_adapter: event.agentAdapter,
run_status: event.runStatus,
},
contexts: {
...contexts,
run_failure: {
taskId: event.taskId,
runId: event.runId,
errorMessage: event.errorMessage,
errorCode,
agentAdapter: event.agentAdapter,
exitCode: normalizeRunExitCode(event.exitCode),
signal: normalizeRunSignal(event.signal),
},
},
fingerprint: [errorCode, event.agentAdapter],
});
} catch (err) {
// eslint-disable-next-line no-console
console.error("[paperclip] Sentry captureRunFailure failed", err);
}
}
/**
* Flush buffered events and close the Sentry client. Idempotent — concurrent
* callers share one shutdown. A no-op when monitoring is off or bootstrap
* failed.
*/
export function shutdownSentry(): Promise<void> {
shutdownPromise ??= (async () => {
await sentryReady;
if (!sentryHandle) return;
try {
// Awaiting matters: the client flushes buffered events to Sentry
// during close; exiting before it settles silently drops them.
await sentryHandle.close(5_000);
} catch (err) {
// eslint-disable-next-line no-console
console.error("[paperclip] Sentry shutdown failed", err);
}
})();
return shutdownPromise;
}
/**
* The subset of the `@sentry/node` module surface the initializer needs to
* build its options object. A structural type, not the real Sentry type —
* the real type is unavailable at compile time because the package is an
* optional runtime dependency (see the module comment above).
*/
interface SentryModuleLike {
httpIntegration(options: { breadcrumbs: boolean }): { name: string };
onUnhandledRejectionIntegration(options: { mode: "strict" }): { name: string };
}
/** The `Sentry.init` options this gate builds. */
export interface SentryInitOptions {
dsn: string;
release?: string;
skipOpenTelemetrySetup: boolean;
tracesSampleRate: number;
sendDefaultPii: boolean;
serverName: string;
integrations: (defaults: Array<{ name: string }>) => Array<{ name: string }>;
}
/**
* Build the `Sentry.init` options object. A pure function, split out from
* `bootstrapSentry` so a test can call it with a real `@sentry/node` module
* and assert the resolved integration list and the captured-event shape
* against the true SDK, not a stand-in.
*/
export function buildSentryInitOptions(
dsn: string,
Sentry: SentryModuleLike,
): SentryInitOptions {
return {
dsn,
release: process.env.SENTRY_RELEASE?.trim() || readBuildCommit() || undefined,
skipOpenTelemetrySetup: true,
tracesSampleRate: 0,
sendDefaultPii: false,
serverName: process.env.SENTRY_NAME || os.hostname(),
integrations: (defaults: Array<{ name: string }>) => {
const kept = defaults.filter(
(integration) =>
integration.name !== "Console" &&
integration.name !== "ContextLines" &&
integration.name !== "Http" &&
integration.name !== "OnUnhandledRejection",
);
return [
...kept,
// Keep the rest of the Http integration — RequestData and request
// isolation need it — but turn the outbound breadcrumb off.
Sentry.httpIntegration({ breadcrumbs: false }),
// Keep today's crash-on-unhandled-rejection behavior. See the
// module comment above for why the default mode cannot stay.
Sentry.onUnhandledRejectionIntegration({ mode: "strict" }),
];
},
};
}
async function bootstrapSentry(dsn: string): Promise<void> {
// Gate on the exact peer version before touching the dynamic import: a
// package installed at the wrong version can still load and start, which
// would silently invalidate the privacy audit `doc/observability.md`
// records against one exact version. Checking first turns that into one
// precise, fail-open diagnostic.
const versionCheck = checkExactPeerVersions(["@sentry/node"]);
if (!versionCheck.ok) {
// eslint-disable-next-line no-console
console.warn(
"[paperclip] The backend Sentry DSN is set, but the @sentry/node " +
"package is not installed, or is installed at an unsupported " +
"version. Install the declared version of @sentry/node to enable " +
"server error monitoring. Continuing without it.",
versionCheck.detail,
);
return;
}
try {
// Dynamic import so type-resolution doesn't require the package to be
// installed unless the operator actually opts in.
// @ts-ignore optional peer dep
const Sentry = await import("@sentry/node");
Sentry.init(buildSentryInitOptions(dsn, Sentry));
sentryHandle = {
captureException: (...args) => Sentry.captureException(...args),
close: (timeout) => Sentry.close(timeout),
};
} catch (err) {
// The exact-version gate above already confirmed @sentry/node is
// installed at the declared version, so only a load or init failure
// after that point reaches this block.
// eslint-disable-next-line no-console
console.warn(
"[paperclip] The backend Sentry DSN is set, and @sentry/node passed " +
"the version check, but it failed to load or initialize. " +
"Continuing without error monitoring.",
err,
);
}
}