From 3d0e74e7432ff6e8dbf6ac05bd261add181f2004 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 8 Oct 2026 10:19:40 -0700 Subject: [PATCH 01/25] Keep proven external connection failures out of Sentry (#15590) Apply the reviewed change for Keep proven external connection failures out of Sentry. Validation: local typecheck/build and focused regression tests, passing exact-head CI, Greptile 5/5, and independent source review. The PR records full-suite evidence and any local environment limitations. Co-Authored-By: Paperclip --- doc/observability.md | 42 +++- .../src/connection-failure.test.ts | 105 +++++++++ .../adapter-utils/src/connection-failure.ts | 58 +++++ .../hermes/src/gateway/server/execute.test.ts | 187 +++++++++++++++- .../hermes/src/gateway/server/execute.ts | 86 +++++++- .../agent-hire-ai-connections.test.ts | 33 ++- .../__tests__/ai-connection-router.test.ts | 52 ++++- server/src/__tests__/error-handler.test.ts | 30 +++ ...eartbeat-managed-clone-credentials.test.ts | 74 +++++++ .../heartbeat-process-recovery.test.ts | 175 ++++++++++++++- .../heartbeat-workspace-busy.test.ts | 93 +++++++- .../heartbeat-workspace-session.test.ts | 23 ++ .../remote-http-endpoint-guard.test.ts | 22 ++ .../__tests__/remote-http-rebinding.test.ts | 101 ++++++++- .../src/__tests__/tool-access-service.test.ts | 193 ++++++++++++++++- server/src/middleware/error-handler.ts | 7 +- .../__tests__/git-connection-failure.test.ts | 202 ++++++++++++++++++ .../__tests__/mcp-connection-failure.test.ts | 76 +++++++ .../__tests__/run-failure-report.test.ts | 181 +++++++++++++++- server/src/services/git-connection-failure.ts | 90 ++++++++ server/src/services/heartbeat.ts | 37 +++- server/src/services/heartbeat/workspaces.ts | 52 +++-- server/src/services/mcp-connection-failure.ts | 63 ++++++ server/src/services/mcp-http.ts | 15 +- .../src/services/remote-connection-failure.ts | 80 +++++++ .../services/remote-http-endpoint-guard.ts | 12 +- server/src/services/remote-http-fetch.ts | 21 +- server/src/services/run-failure-report.ts | 47 ++++ server/src/services/tool-access.ts | 20 +- 29 files changed, 2097 insertions(+), 80 deletions(-) create mode 100644 packages/adapter-utils/src/connection-failure.test.ts create mode 100644 packages/adapter-utils/src/connection-failure.ts create mode 100644 server/src/services/__tests__/git-connection-failure.test.ts create mode 100644 server/src/services/__tests__/mcp-connection-failure.test.ts create mode 100644 server/src/services/git-connection-failure.ts create mode 100644 server/src/services/mcp-connection-failure.ts create mode 100644 server/src/services/remote-connection-failure.ts diff --git a/doc/observability.md b/doc/observability.md index 2114c16eb79..b62a1263309 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -448,9 +448,45 @@ explicit `local_path` or `non_git_path` project workspace has no repository URL, Git confirms the selected directory is not a repository, and the task requests a Git worktree. The run must fail during setup before provider work starts. Its existing validation error, blocked task and board recovery action remain. -Git command failures, permissions, corrupt repositories, failed materialization, -fallback paths, and generic missing or unrestorable workspaces remain reportable; -the error text or `workspace_validation_failed` code alone never suppresses them. +Local Git command failures, permissions, corrupt repositories, fallback paths, +and generic missing or unrestorable workspaces remain reportable. The error text +or `workspace_validation_failed` code alone never suppresses them. + +Recognized external connection failures also stay in the local run result instead +of producing application-error events in Sentry. Their producer records a closed +`connectionFailure` descriptor: schema version, provider, operation, and reason. +It contains no URL, host, path, credential, or remote response. This classification +does not change the run's failed state, task recovery action, retry policy, or +whether remote work may have been accepted. + +- Managed remote Git clones classify known authentication, repository access, + invalid URL, DNS (including machine-specific SSH aliases), connection, TLS, + rate-limit, and upstream failures from the actual clone subprocess. Setup must + fail before provider work starts. Every failed workspace candidate must have + proven connection evidence; a mixed or unknown failure remains reportable. + Local/file clones, credential-provider exceptions, local checkout or copy + failures, process signals, and repository corruption remain reportable. Remote + fetch failures already return workspace warnings; their classification is not + transferred to a later failure resolving a local base ref. +- Hermes gateway configuration validation and the initial create-run request use + exact config, HTTP, or structured transport evidence. Missing configuration, + rejected credentials, unavailable endpoints, rate limits, and upstream outages + remain visible to the run owner. Unknown protocol/parser errors, missing run + identifiers, untyped terminal failures, and timeouts after accepted work remain + reportable. Stream/poll errors retain their existing retry and logging behavior. + +Expected pre-provider waits for a shared project workspace, a credential rotation +lock, or an exhausted AI connection pool carry a server-owned expected +cancellation receipt. They retain existing retry and lock behavior without a +Sentry cancellation event. A later failed retry still reports its actual error; +the wait code alone is not sufficient to suppress an event. + +MCP tool discovery records recognized outbound HTTP/transport failures through +server-only provenance. These failures preserve the response status, stored +connection health, audit record, local context, and first-party telemetry while +skipping only Sentry. A proven missing OAuth scope returns an actionable 422 +response with its reconnect guidance. Unknown responses, parsing errors, local +callbacks, database errors, and unproven network-looking messages still report. The `run_failure` context also includes the recorded process `exitCode` and `signal`, so a generic adapter error can still distinguish a nonzero exit from diff --git a/packages/adapter-utils/src/connection-failure.test.ts b/packages/adapter-utils/src/connection-failure.test.ts new file mode 100644 index 00000000000..602e01fd9ec --- /dev/null +++ b/packages/adapter-utils/src/connection-failure.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; +import { readConnectionFailure } from "./connection-failure.js"; + +describe("readConnectionFailure", () => { + const git = { schemaVersion: 1, provider: "git", operation: "clone", reason: "authentication_failed" }; + const gateway = { schemaVersion: 1, provider: "hermes_gateway", operation: "create_run", reason: "connection_refused" }; + const mcp = { schemaVersion: 1, provider: "mcp_http", operation: "discover_tools", reason: "connection_refused" }; + + it.each([ + "authentication_failed", "repository_unavailable", "invalid_remote", "dns_failure", "connection_refused", + "network_unreachable", "connection_reset", "connection_timeout", "tls_failure", "rate_limited", "remote_unavailable", + ])("accepts the Git clone reason %s", (reason) => { + const input = { ...git, reason }; + expect(readConnectionFailure(input)).toEqual(input); + }); + + it.each(["endpoint_missing", "endpoint_invalid", "insecure_transport", "credentials_missing"])( + "accepts the gateway configuration reason %s", (reason) => { + const input = { ...gateway, operation: "configuration", reason }; + expect(readConnectionFailure(input)).toEqual(input); + }, + ); + + it.each([ + "authentication_failed", "endpoint_not_found", "rate_limited", "remote_unavailable", "connection_refused", + "dns_failure", "network_unreachable", "connection_reset", "connection_timeout", "tls_failure", + ])("accepts the gateway request reason %s", (reason) => { + const input = { ...gateway, reason }; + expect(readConnectionFailure(input)).toEqual(input); + }); + + it.each([ + "authentication_failed", "endpoint_not_found", "rate_limited", "remote_unavailable", "connection_refused", + "dns_failure", "network_unreachable", "connection_reset", "connection_timeout", "tls_failure", + ])("accepts the MCP tool discovery request reason %s", (reason) => { + const input = { ...mcp, reason }; + expect(readConnectionFailure(input)).toEqual(input); + }); + + it.each([ + { ...git, operation: "fetch" }, + { ...git, operation: "create_run" }, + { ...git, reason: "endpoint_not_found" }, + { ...gateway, operation: "clone" }, + { ...gateway, operation: "poll_status" }, + { ...gateway, operation: "stop_run" }, + { ...gateway, operation: "configuration" }, + { ...gateway, reason: "credentials_missing" }, + { ...gateway, reason: "repository_unavailable" }, + { ...gateway, reason: "timeout" }, + { ...gateway, reason: "unknown" }, + { ...gateway, provider: "other_gateway" }, + { ...gateway, provider: "hermes_local" }, + { ...mcp, operation: "create_run" }, + { ...mcp, operation: "clone" }, + { ...mcp, operation: "configuration" }, + { ...mcp, operation: "call_tool" }, + { ...mcp, reason: "credentials_missing" }, + { ...mcp, reason: "repository_unavailable" }, + { ...gateway, operation: "discover_tools" }, + { ...git, operation: "discover_tools" }, + ])("rejects an unsupported provider/operation/reason combination: %j", (input) => { + expect(readConnectionFailure(input)).toBeNull(); + }); + + it.each([ + undefined, null, false, 1, "connection_refused", [], [gateway], {}, + { ...gateway, schemaVersion: 0 }, { ...gateway, schemaVersion: 2 }, { ...gateway, schemaVersion: "1" }, + { ...gateway, schemaVersion: null }, { ...gateway, schemaVersion: undefined }, + { ...gateway, provider: null }, { ...gateway, provider: ["hermes_gateway"] }, + { ...gateway, operation: null }, { ...gateway, operation: 1 }, + { ...gateway, reason: null }, { ...gateway, reason: 1 }, { ...gateway, reason: ["connection_refused"] }, + { ...gateway, reason: { code: "ECONNREFUSED" } }, + { provider: "hermes_gateway", operation: "create_run", reason: "connection_refused" }, + { ...gateway, reason: "connection_refused " }, { ...gateway, reason: "CONNECTION_REFUSED" }, + ])("rejects unknown versions or malformed values: %j", (input) => { + expect(readConnectionFailure(input)).toBeNull(); + }); + + it.each([ + { url: "https://private.example.test" }, + { account: "private-account" }, + { token: "synthetic-secret" }, + { remoteRunAccepted: false }, + { providerWorkStarted: false }, + { details: { message: "private diagnostics" } }, + { extra: undefined }, + ])("rejects extra metadata instead of broadening the bounded marker: %j", (extra) => { + expect(readConnectionFailure({ ...gateway, ...extra })).toBeNull(); + }); + + it("rejects unknown own fields even when a prototype supplies a valid descriptor", () => { + const input = Object.assign(Object.create(gateway), { a: 1, b: 2, c: 3, d: 4 }); + expect(readConnectionFailure(input)).toBeNull(); + }); + + it("returns an independent bounded value without mutating its producer's input", () => { + const input = Object.freeze({ ...gateway }); + const output = readConnectionFailure(input); + expect(output).toEqual(gateway); + expect(output).not.toBe(input); + expect(input).toEqual(gateway); + expect(Object.keys(output ?? {}).sort()).toEqual(["operation", "provider", "reason", "schemaVersion"]); + }); +}); diff --git a/packages/adapter-utils/src/connection-failure.ts b/packages/adapter-utils/src/connection-failure.ts new file mode 100644 index 00000000000..a5a3e5895d2 --- /dev/null +++ b/packages/adapter-utils/src/connection-failure.ts @@ -0,0 +1,58 @@ +/** + * A bounded producer-owned explanation of an external connection failure. + * This changes error reporting only: it never proves remote acceptance, + * authorizes a retry, or turns a failed run into a success. + */ +const gitReasons = [ + "authentication_failed", "repository_unavailable", "invalid_remote", + "dns_failure", "connection_refused", "network_unreachable", "connection_reset", + "connection_timeout", "tls_failure", "rate_limited", "remote_unavailable", +] as const; +const hermesConfigurationReasons = [ + "endpoint_missing", "endpoint_invalid", "insecure_transport", "credentials_missing", +] as const; +const hermesRequestReasons = [ + "authentication_failed", "endpoint_not_found", "rate_limited", "remote_unavailable", + "connection_refused", "dns_failure", "network_unreachable", "connection_reset", + "connection_timeout", "tls_failure", +] as const; + +export type GitConnectionFailure = { + schemaVersion: 1; + provider: "git"; + operation: "clone"; + reason: typeof gitReasons[number]; +}; + +export type ConnectionFailure = GitConnectionFailure | { + schemaVersion: 1; + provider: "hermes_gateway"; +} & ({ + operation: "configuration"; + reason: typeof hermesConfigurationReasons[number]; +} | { + operation: "create_run"; + reason: typeof hermesRequestReasons[number]; +}) | { + schemaVersion: 1; + provider: "mcp_http"; + operation: "discover_tools"; + reason: typeof hermesRequestReasons[number]; +}; + +/** Unknown versions, combinations and extra payload fields remain reportable. */ +export function readConnectionFailure(value: unknown): ConnectionFailure | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const record = value as Record; + const keys = Object.keys(record); + if (keys.length !== 4 || !keys.every((key) => ["schemaVersion", "provider", "operation", "reason"].includes(key)) || + record.schemaVersion !== 1 || typeof record.reason !== "string") return null; + let reasons: readonly string[]; + if (record.provider === "git" && record.operation === "clone") reasons = gitReasons; + else if (record.provider === "hermes_gateway" && record.operation === "configuration") reasons = hermesConfigurationReasons; + else if (record.provider === "hermes_gateway" && record.operation === "create_run") reasons = hermesRequestReasons; + else if (record.provider === "mcp_http" && record.operation === "discover_tools") reasons = hermesRequestReasons; + else return null; + if (!reasons.includes(record.reason)) return null; + return { schemaVersion: 1, provider: record.provider, operation: record.operation, reason: record.reason } as ConnectionFailure; +} diff --git a/packages/adapters/hermes/src/gateway/server/execute.test.ts b/packages/adapters/hermes/src/gateway/server/execute.test.ts index efe4df66660..39393432a73 100644 --- a/packages/adapters/hermes/src/gateway/server/execute.test.ts +++ b/packages/adapters/hermes/src/gateway/server/execute.test.ts @@ -633,7 +633,9 @@ describe("execute", () => { expect(result).toMatchObject({ exitCode: 1, signal: null, timedOut: false, errorCode: "hermes_gateway_connect_failed", errorFamily: "transient_upstream", retryNotBefore: null }); expect(result.executionRecovery).toBeUndefined(); - expect(result.resultJson).toBeUndefined(); + expect(result.resultJson).toEqual({ connectionFailure: { + schemaVersion: 1, provider: "hermes_gateway", operation: "create_run", reason: "connection_refused", + } }); expect(ctx.onDispatch).toHaveBeenCalledOnce(); expect(fetchMock).toHaveBeenCalledOnce(); expect(fetchMock.mock.calls[0]?.[1]?.redirect).toBeUndefined(); @@ -738,6 +740,189 @@ describe("execute", () => { }); }); +describe("gateway connection failure reporting", () => { + const config = { apiBaseUrl: "https://gateway.example.test", apiKey: "gateway-private-key" }; + const marker = (operation: string, reason: string) => ({ + schemaVersion: 1, provider: "hermes_gateway", operation, reason, + }); + + it.each([ + [{}, "api_base_url_missing", "endpoint_missing"], + [{ apiBaseUrl: "not a URL" }, "api_base_url_invalid", "endpoint_invalid"], + [{ apiBaseUrl: "ftp://gateway.example.test" }, "api_base_url_invalid", "endpoint_invalid"], + [{ apiBaseUrl: "http://gateway.example.test" }, "plain_http_remote_denied", "insecure_transport"], + [{ apiBaseUrl: config.apiBaseUrl }, "api_key_missing", "credentials_missing"], + ])("classifies configuration failure %s without dispatch", async (settings, code, reason) => { + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const ctx = makeCtx(settings); + ctx.onDispatch = vi.fn(); + const result = await execute(ctx); + expect(result).toMatchObject({ exitCode: 1, signal: null, timedOut: false, + errorCode: `hermes_gateway_${code}`, resultJson: { connectionFailure: marker("configuration", reason) } }); + expect(result.errorMessage).toBeTruthy(); + expect(ctx.onDispatch).not.toHaveBeenCalled(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it.each([ + [401, "auth_failed", "authentication_failed", null], + [403, "auth_failed", "authentication_failed", null], + [404, "runs_unsupported", "endpoint_not_found", null], + [429, "rate_limited", "rate_limited", "transient_upstream"], + [500, "upstream_error", "remote_unavailable", "transient_upstream"], + [503, "upstream_error", "remote_unavailable", "transient_upstream"], + [599, "upstream_error", "remote_unavailable", "transient_upstream"], + ])("classifies HTTP %i without changing retry or dispatch behavior", async (status, code, reason, errorFamily) => { + const fetchMock = vi.fn(async () => new Response(JSON.stringify({ detail: "remote failure" }), { + status, headers: { "retry-after": "7" }, + })); + vi.stubGlobal("fetch", fetchMock); + const ctx = makeCtx(config); + ctx.onDispatch = vi.fn(); + const result = await execute(ctx); + expect(result).toMatchObject({ exitCode: 1, signal: null, timedOut: false, + errorCode: `hermes_gateway_${code}`, errorFamily, retryNotBefore: "7", + errorMeta: { status, body: { detail: "remote failure" } }, + resultJson: { connectionFailure: marker("create_run", reason) } }); + expect(result.errorMessage).toContain(`Hermes gateway HTTP ${status}`); + expect(result.executionRecovery).toBeUndefined(); + expect(ctx.onDispatch).toHaveBeenCalledOnce(); + expect(fetchMock).toHaveBeenCalledOnce(); + expect(Object.keys(result.resultJson ?? {})).toEqual(["connectionFailure"]); + }); + + it.each([ + ["ECONNREFUSED", "connection_refused"], ["ENOTFOUND", "dns_failure"], ["EAI_AGAIN", "dns_failure"], + ["EHOSTUNREACH", "network_unreachable"], ["ENETUNREACH", "network_unreachable"], + ["ECONNRESET", "connection_reset"], ["EPIPE", "connection_reset"], ["UND_ERR_SOCKET", "connection_reset"], + ["ETIMEDOUT", "connection_timeout"], ["UND_ERR_CONNECT_TIMEOUT", "connection_timeout"], + ["UND_ERR_HEADERS_TIMEOUT", "connection_timeout"], ["UND_ERR_BODY_TIMEOUT", "connection_timeout"], + ["CERT_HAS_EXPIRED", "tls_failure"], ["CERT_NOT_YET_VALID", "tls_failure"], + ["DEPTH_ZERO_SELF_SIGNED_CERT", "tls_failure"], ["SELF_SIGNED_CERT_IN_CHAIN", "tls_failure"], + ["UNABLE_TO_VERIFY_LEAF_SIGNATURE", "tls_failure"], ["UNABLE_TO_GET_ISSUER_CERT_LOCALLY", "tls_failure"], + ["ERR_TLS_CERT_ALTNAME_INVALID", "tls_failure"], + ])("classifies structured fetch cause %s", async (code, reason) => { + vi.stubGlobal("fetch", vi.fn(async () => { + throw new TypeError("fetch failed", { cause: Object.assign(new Error("external transport failed"), { code }) }); + })); + const result = await execute(makeCtx(config)); + expect(result).toMatchObject({ exitCode: 1, signal: null, timedOut: false, + errorCode: "hermes_gateway_connect_failed", errorFamily: "transient_upstream", retryNotBefore: null, + resultJson: { connectionFailure: marker("create_run", reason) } }); + expect(result.errorMessage).toContain(code); + expect(result.executionRecovery).toBeUndefined(); + }); + + it("classifies a network failure reading the create response without rewriting its error", async () => { + const response = new Response("unread"); + vi.spyOn(response, "text").mockRejectedValue(new TypeError("terminated", { cause: { code: "UND_ERR_SOCKET" } })); + vi.stubGlobal("fetch", vi.fn(async () => response)); + const result = await execute(makeCtx(config)); + expect(result).toMatchObject({ exitCode: 1, signal: null, timedOut: false, + errorCode: "hermes_gateway_protocol_error", errorFamily: null, errorMessage: "terminated", + resultJson: { connectionFailure: marker("create_run", "connection_reset") } }); + expect(result.executionRecovery).toBeUndefined(); + }); + + it("requires every aggregate member to prove the same external failure", async () => { + vi.stubGlobal("fetch", vi.fn(async () => { throw new TypeError("fetch failed", { cause: { + code: "ECONNREFUSED", errors: [{ code: "ECONNREFUSED" }, { code: "ECONNREFUSED" }], + } }); })); + const result = await execute(makeCtx(config)); + expect(result.resultJson).toEqual({ connectionFailure: marker("create_run", "connection_refused") }); + }); + + it.each([ + undefined, { message: "ECONNREFUSED" }, { code: "ERR_INTERNAL_ASSERTION" }, { code: "toString" }, + { code: "ECONNREFUSED", errors: [] }, { code: "ECONNREFUSED", errors: "invalid" }, + { code: "ECONNREFUSED", errors: Array.from({ length: 9 }, () => ({ code: "ECONNREFUSED" })) }, + { code: "ECONNREFUSED", errors: [{ code: "ECONNREFUSED" }, { code: "ETIMEDOUT" }] }, + { code: "ECONNREFUSED", errors: [{ code: "ECONNREFUSED" }, new TypeError("internal bug")] }, + { code: "ERR_INTERNAL_ASSERTION", errors: [{ code: "ECONNREFUSED" }] }, + { code: "ECONNRESET", name: "AbortError" }, + ])("keeps unknown, mixed, or internal transport causes reportable: %j", async (cause) => { + vi.stubGlobal("fetch", vi.fn(async () => { throw new TypeError("ECONNREFUSED", { cause }); })); + const result = await execute(makeCtx(config)); + expect(result.errorCode).toBe("hermes_gateway_connect_failed"); + expect(result.resultJson).toBeUndefined(); + }); + + it("keeps abort failures reportable even if their cause looks like a transport error", async () => { + const aborted = Object.assign(new Error("cancelled", { cause: { code: "ECONNRESET" } }), { name: "AbortError" }); + vi.stubGlobal("fetch", vi.fn(async () => { throw aborted; })); + const result = await execute(makeCtx(config)); + expect(result.resultJson).toBeUndefined(); + expect(result.errorCode).toBe("hermes_gateway_connect_failed"); + }); + + it.each([400, 409, 422])("does not trust HTTP %i body claims about connection failures", async (status) => { + vi.stubGlobal("fetch", vi.fn(async () => new Response(JSON.stringify({ + error: "ECONNREFUSED", connectionFailure: marker("create_run", "connection_refused"), + }), { status }))); + const result = await execute(makeCtx(config)); + expect(result.errorCode).toBe("hermes_gateway_protocol_error"); + expect(result.resultJson).toBeUndefined(); + }); + + it.each(["not json", JSON.stringify({ status: "running", connectionFailure: marker("create_run", "connection_refused") })])( + "keeps malformed successful create responses reportable", async (body) => { + vi.stubGlobal("fetch", vi.fn(async () => new Response(body))); + const result = await execute(makeCtx(config)); + expect(result.errorCode).toBe("hermes_gateway_protocol_error"); + expect(result.resultJson).toBeUndefined(); + }, + ); + + it("does not classify a local response reader TypeError", async () => { + const response = new Response("unread"); + vi.spyOn(response, "text").mockRejectedValue(new TypeError("cannot read property of undefined")); + vi.stubGlobal("fetch", vi.fn(async () => response)); + const result = await execute(makeCtx(config)); + expect(result.errorCode).toBe("hermes_gateway_protocol_error"); + expect(result.resultJson).toBeUndefined(); + }); + + it("does not trust a callback error with forged gateway codes or markers", async () => { + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + const ctx = makeCtx(config); + ctx.onDispatch = () => { throw Object.assign(new Error("callback bug"), { + code: "hermes_gateway_auth_failed", status: 401, cause: { code: "ECONNREFUSED" }, + connectionFailure: marker("create_run", "authentication_failed"), + }); }; + const result = await execute(ctx); + expect(result.resultJson).toBeUndefined(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it.each(["completed", "failed", "cancelled"])("does not classify remote terminal %s from text or a forged marker", (status) => { + const result = mapFinalResultForTest({ + terminal: { runId: "run-remote", status, payload: { + error: "ECONNREFUSED", code: "ECONNREFUSED", + connectionFailure: marker("create_run", "connection_refused"), + } }, outputChunks: [], sessionKey: null, strategy: "none", + }); + expect(result.resultJson?.connectionFailure).toBeUndefined(); + expect(result.errorCode).toBe(status === "completed" ? undefined : status === "failed" ? "hermes_gateway_run_failed" : "hermes_gateway_cancelled"); + }); + + it("retains an ambiguous accepted-run timeout after stream, poll, and stop connection failures", async () => { + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + if (String(input).endsWith("/v1/runs")) return new Response(JSON.stringify({ run_id: "run-accepted" })); + throw new TypeError("fetch failed", { cause: { code: "ECONNRESET" } }); + }); + vi.stubGlobal("fetch", fetchMock); + const ctx = makeCtx({ ...config, timeoutSec: 0.3, pollIntervalMs: 250, eventReconnectMs: 250 }); + const result = await execute(ctx); + expect(result).toMatchObject({ timedOut: true, errorCode: "hermes_gateway_timeout" }); + expect(result.resultJson?.connectionFailure).toBeUndefined(); + expect(result.resultJson?.run_id).toBe("run-accepted"); + expect(fetchMock.mock.calls.some(([input]) => String(input).endsWith("/stop"))).toBe(true); + expect(ctx.onLog).toHaveBeenCalledWith("stderr", expect.stringContaining("status poll failed")); + }); +}); + describe("testEnvironment", () => { it("fails remote plain HTTP before probing health", async () => { const fetchMock = vi.fn(async () => new Response("{}", { status: 200 })); diff --git a/packages/adapters/hermes/src/gateway/server/execute.ts b/packages/adapters/hermes/src/gateway/server/execute.ts index 6942af59fc0..e4551b1cb6d 100644 --- a/packages/adapters/hermes/src/gateway/server/execute.ts +++ b/packages/adapters/hermes/src/gateway/server/execute.ts @@ -3,6 +3,7 @@ import type { AdapterExecutionResult, UsageSummary, } from "@paperclipai/adapter-utils"; +import type { ConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; import { asNumber, asString, @@ -61,6 +62,72 @@ type ExecutionState = { type TextRedactor = (value: string) => string; +type GatewayConfigurationFailure = Extract; +type GatewayRequestFailure = Extract; + +// Only errors observed at our outbound request boundary acquire this provenance. +// A callback or remote payload cannot opt itself out of product error reporting. +const requestConnectionFailures = new WeakMap(); +const NETWORK_FAILURE_REASONS: Readonly> = { + ECONNREFUSED: "connection_refused", + ENOTFOUND: "dns_failure", + EAI_AGAIN: "dns_failure", + EHOSTUNREACH: "network_unreachable", + ENETUNREACH: "network_unreachable", + ECONNRESET: "connection_reset", + EPIPE: "connection_reset", + UND_ERR_SOCKET: "connection_reset", + ETIMEDOUT: "connection_timeout", + UND_ERR_CONNECT_TIMEOUT: "connection_timeout", + UND_ERR_HEADERS_TIMEOUT: "connection_timeout", + UND_ERR_BODY_TIMEOUT: "connection_timeout", + CERT_HAS_EXPIRED: "tls_failure", + CERT_NOT_YET_VALID: "tls_failure", + DEPTH_ZERO_SELF_SIGNED_CERT: "tls_failure", + SELF_SIGNED_CERT_IN_CHAIN: "tls_failure", + UNABLE_TO_VERIFY_LEAF_SIGNATURE: "tls_failure", + UNABLE_TO_GET_ISSUER_CERT_LOCALLY: "tls_failure", + ERR_TLS_CERT_ALTNAME_INVALID: "tls_failure", +}; + +function gatewayConfigurationFailure(reason: GatewayConfigurationFailure["reason"]): GatewayConfigurationFailure { + return { schemaVersion: 1, provider: "hermes_gateway", operation: "configuration", reason }; +} + +function networkFailureReason(value: unknown, depth = 0): GatewayRequestFailure["reason"] | null { + const cause = asRecord(value); + if (!cause || depth > 2 || cause.name === "AbortError") return null; + const reason = typeof cause.code === "string" && Object.hasOwn(NETWORK_FAILURE_REASONS, cause.code) + ? NETWORK_FAILURE_REASONS[cause.code]! + : null; + if (cause.code !== undefined && !reason) return null; + if (cause.errors !== undefined) { + if (!Array.isArray(cause.errors) || cause.errors.length === 0 || cause.errors.length > 8) return null; + const reasons = cause.errors.map((error) => networkFailureReason(error, depth + 1)); + const first = reasons[0]; + return first && reasons.every((item) => item === first) && (!reason || reason === first) ? first : null; + } + return reason; +} + +function rememberNetworkFailure(target: Error, error: unknown): void { + if (!(error instanceof Error) || error.name === "AbortError") return; + // Native fetch wraps transport errors in cause. Do not classify an arbitrary + // TypeError, its text, or remote JSON that happens to name a network error. + const reason = networkFailureReason(error.cause); + if (reason) requestConnectionFailures.set(target, { + schemaVersion: 1, provider: "hermes_gateway", operation: "create_run", reason, + }); +} + +function httpFailureReason(status: number): GatewayRequestFailure["reason"] | null { + if (status === 401 || status === 403) return "authentication_failed"; + if (status === 404) return "endpoint_not_found"; + if (status === 429) return "rate_limited"; + if (status >= 500 && status <= 599) return "remote_unavailable"; + return null; +} + const CRITICAL_HEADERS = new Set([ "authorization", "content-type", @@ -380,9 +447,16 @@ async function fetchJson(input: RequestInfo | URL, init: RequestInit): Promise vi.fn()); +vi.mock("../sentry.js", async (importOriginal) => ({ + ...await importOriginal(), + captureRunFailure, +})); let database: Awaited>; +const externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL?.trim(); let db: ReturnType; let home: string; @@ -31,11 +39,17 @@ beforeAll(async () => { vi.stubEnv("PAPERCLIP_HOME", home); vi.stubEnv("PAPERCLIP_INSTANCE_ID", "hire-ai"); vi.stubEnv("PAPERCLIP_IN_WORKTREE", "false"); - database = await startEmbeddedPostgresTestDatabase("paperclip-hire-ai-db-"); - db = createDb(database.connectionString); + if (externalTestDatabaseUrl) db = createDb(externalTestDatabaseUrl); + else { + database = await startEmbeddedPostgresTestDatabase("paperclip-hire-ai-db-"); + db = createDb(database.connectionString); + } }, 90_000); afterAll(async () => { + await waitForPendingRunFailureReports(); + if (externalTestDatabaseUrl) await closeRegisteredClients(externalTestDatabaseUrl); + await db?.$client.end(); await database?.cleanup(); vi.unstubAllEnvs(); if (home) await rm(home, { recursive: true, force: true }); @@ -488,10 +502,21 @@ describe("hired agents sharing a subscription", () => { expect(run).not.toBeNull(); await expect.poll(async () => (await heartbeat.getRun(run!.id))?.status, { timeout: 20_000 }).toBe("cancelled"); await heartbeat.drainActiveRunExecutions(); - expect(await heartbeat.getRun(run!.id)).toMatchObject({ errorCode: "ai_connection_busy", resultJson: { executionRecovery: { providerWorkStarted: false } } }); + const cancelled = await heartbeat.getRun(run!.id); + expect(cancelled).toMatchObject({ errorCode: "ai_connection_busy", resultJson: { + executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false }, + cancellation: { + source: "control_plane", expected: true, initiator: { type: "system" }, + reason: "Waiting for shared AI credentials", + recordedAt: cancelled!.finishedAt!.toISOString(), + }, + } }); + await waitForPendingRunFailureReports(); + expect(captureRunFailure.mock.calls.filter(([report]) => report.runId === run!.id)).toHaveLength(0); const retries = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, run!.id)); expect(retries).toHaveLength(1); expect(retries[0]).toMatchObject({ status: "scheduled_retry", scheduledRetryReason: "ai_connection_busy" }); + expect(retries[0].resultJson?.cancellation).toBeUndefined(); const [savedIssue] = await db.select().from(issues).where(eq(issues.id, issue.id)); expect(savedIssue.status).not.toBe("blocked"); expect(savedIssue.executionRunId).toBe(retries[0].id); diff --git a/server/src/__tests__/ai-connection-router.test.ts b/server/src/__tests__/ai-connection-router.test.ts index b2e13a92e8a..1a1f3f29bd2 100644 --- a/server/src/__tests__/ai-connection-router.test.ts +++ b/server/src/__tests__/ai-connection-router.test.ts @@ -4,7 +4,7 @@ import { mkdtemp, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { and, eq } from "drizzle-orm"; -import { createDb, companies, agents, companyMemberships, connectionGrants, plugins, aiConnectionRouterCursors, aiConnectionTaskPins, toolConnections, companySecrets, heartbeatRuns, issues } from "@paperclipai/db"; +import { createDb, closeRegisteredClients, companies, agents, companyMemberships, connectionGrants, plugins, aiConnectionRouterCursors, aiConnectionTaskPins, toolConnections, companySecrets, heartbeatRuns, issues } from "@paperclipai/db"; import { startEmbeddedPostgresTestDatabase } from "@paperclipai/db/test-embedded-postgres"; import type { AiConnectionPoolMember, AiConnectionRouterRequest, AiConnectionRouterResult, PaperclipPluginManifestV1 } from "@paperclipai/shared"; import { aiConnectionRouterService, AiConnectionPoolExhausted, poolMemberRuntimeConfig, applyAiConnectionRouterTaskSettings } from "../services/ai-connection-router.js"; @@ -24,8 +24,16 @@ import { heartbeatService, buildEffectiveRunSessionConfigMetadata, resolveTaskSe import { connectionIntentService } from "../services/connection-intents.js"; import { legacyExecutionNeedsReconciliation } from "../services/legacy-execution-recovery.js"; import { getServerAdapter, registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js"; +import { waitForPendingRunFailureReports } from "../services/run-failure-report.js"; + +const captureRunFailure = vi.hoisted(() => vi.fn()); +vi.mock("../sentry.js", async (importOriginal) => ({ + ...await importOriginal(), + captureRunFailure, +})); let database: Awaited>; +const externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL?.trim(); let db: ReturnType; let home: string; const companyId = randomUUID(), otherCompanyId = randomUUID(), agentId = randomUUID(), secondAgentId = randomUUID(), pluginId = randomUUID(); @@ -46,7 +54,8 @@ const makePool = async (config = {}) => service().save(pluginKey, { companyId, c beforeAll(async () => { home = await mkdtemp(path.join(os.tmpdir(), "paperclip-router-tests-")); vi.stubEnv("PAPERCLIP_HOME", home); vi.stubEnv("PAPERCLIP_INSTANCE_ID", "router-fixture"); - database = await startEmbeddedPostgresTestDatabase("paperclip-router-db-"); db = createDb(database.connectionString); + if (externalTestDatabaseUrl) db = createDb(externalTestDatabaseUrl); + else { database = await startEmbeddedPostgresTestDatabase("paperclip-router-db-"); db = createDb(database.connectionString); } await db.insert(companies).values([{ id: companyId, name: "Router fixture", issuePrefix: "RTF" }, { id: otherCompanyId, name: "Other", issuePrefix: "RTO" }]); await db.insert(agents).values([agentId, secondAgentId].map(id => ({ id, companyId, name: "Router agent", adapterType: "paperclip_runner" }))); await db.insert(companyMemberships).values(["alice", "bob"].map(principalId => ({ companyId, principalId, principalType: "user", status: "active", membershipRole: "member" }))); @@ -59,7 +68,12 @@ beforeAll(async () => { { id: randomUUID(), binding: { mode: "delegated", provider: "anthropic", method: "subscription", ...claude }, profile: { provider: "acpx", acpxAgent: "claude", model: "claude-sonnet-5" } }, ]; }, 90000); -afterAll(async () => { await database?.cleanup(); vi.unstubAllEnvs(); if (home) await rm(home, { recursive: true, force: true }); }); +afterAll(async () => { + await waitForPendingRunFailureReports(); + if (externalTestDatabaseUrl) await closeRegisteredClients(externalTestDatabaseUrl); + await db?.$client.end(); + await database?.cleanup(); vi.unstubAllEnvs(); if (home) await rm(home, { recursive: true, force: true }); +}); describe("durable, authorized connection routing", () => { it("defaults off and does not expose selectable pools before opt-in", async () => { @@ -321,16 +335,48 @@ describe("durable, authorized connection routing", () => { expect(run).not.toBeNull(); await expect.poll(async () => (await heartbeat.getRun(run!.id))?.status, { timeout: 20_000 }).toBe("cancelled"); const cancelled = await heartbeat.getRun(run!.id); expect(cancelled?.errorCode).toBe("ai_connection_pool_exhausted"); + expect(cancelled?.resultJson).toMatchObject({ + executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false }, + cancellation: { + source: "control_plane", expected: true, initiator: { type: "system" }, + reason: "Waiting for an AI connection pool account", + recordedAt: cancelled!.finishedAt!.toISOString(), + }, + }); expect(legacyExecutionNeedsReconciliation(cancelled!)).toBe(false); expect(legacyExecutionNeedsReconciliation({ ...cancelled!, resultJson: { executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: true } } })).toBe(true); await expect.poll(async () => (await db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.agentId, agent!.id), eq(heartbeatRuns.scheduledRetryReason, "ai_connection_pool_wait")))).length, { timeout: 5000 }).toBe(1); const [retry] = await db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.agentId, agent!.id), eq(heartbeatRuns.scheduledRetryReason, "ai_connection_pool_wait"))); + await waitForPendingRunFailureReports(); + expect(captureRunFailure.mock.calls.filter(([report]) => report.runId === run!.id)).toHaveLength(0); + expect(retry?.resultJson?.cancellation).toBeUndefined(); expect(retry?.contextSnapshot?.aiRouterTaskKey).toBe(issue!.id); expect(retry?.contextSnapshot?.executionRetryAccounting).toMatchObject({ failureRetries: 0 }); expect(execute).not.toHaveBeenCalled(); const [pin] = await db.select().from(aiConnectionTaskPins).where(and(eq(aiConnectionTaskPins.poolId, pool.id), eq(aiConnectionTaskPins.agentId, agent!.id))); expect(pin?.selection).toEqual(pinned); const [cursor] = await db.select().from(aiConnectionRouterCursors).where(eq(aiConnectionRouterCursors.poolId, pool.id)); expect(cursor?.version).toBe(1); await db.update(heartbeatRuns).set({ status: "cancelled", finishedAt: new Date() }).where(eq(heartbeatRuns.agentId, agent!.id)); } finally { fetch.mockRestore(); proposal.mockReset(); proposal.mockImplementation(defaultProposal); unregisterServerAdapter("claude_local"); await heartbeat.drainActiveRunExecutions(); } }, 30_000); + it("keeps unexpected pool policy failures reportable before provider work", async () => { + await instanceSettingsService(db).updateExperimental({ enableAiConnectionRouters: true, enableWorktreeRunExecution: true }); + const pool = await makePool({ members: [members[1]!] }); + const [agent] = await db.insert(agents).values({ companyId, name: "Unexpected router failure", adapterType: "claude_local", adapterConfig: { cwd: home, engine: "cli" }, runtimeConfig: { aiConnection: { mode: "router", connectionId: pool.id }, heartbeat: { enabled: false } } }).returning(); + const [issue] = await db.insert(issues).values({ companyId, title: "Router failure fixture", status: "todo", assigneeAgentId: agent!.id, responsibleUserId: "alice", createdByUserId: "alice" }).returning(); + const execute = vi.fn(); registerServerAdapter({ ...getServerAdapter("claude_local"), execute }); + proposal.mockRejectedValueOnce(new Error("Synthetic unexpected router policy failure")); + const heartbeat = heartbeatService(db, { pluginWorkerManager: worker, runtimeEnv: {} }); + try { + const run = await heartbeat.wakeup(agent!.id, { source: "assignment", reason: "issue_assigned", payload: { issueId: issue!.id }, contextSnapshot: { issueId: issue!.id, wakeReason: "issue_assigned", responsibleUserId: "alice" }, triggerDetail: "system" }); + expect(run).not.toBeNull(); + await expect.poll(async () => (await heartbeat.getRun(run!.id))?.status, { timeout: 20_000 }).toBe("failed"); + const failed = await heartbeat.getRun(run!.id); + expect(failed?.errorCode).toBe("configuration_incomplete"); + expect(failed?.resultJson).toMatchObject({ configurationIncomplete: { reason: "ai_connection_unavailable" } }); + expect(failed?.resultJson?.cancellation).toBeUndefined(); + expect(execute).not.toHaveBeenCalled(); + await expect.poll(() => captureRunFailure.mock.calls.filter(([report]) => report.runId === run!.id)).toHaveLength(1); + expect(captureRunFailure).toHaveBeenCalledWith(expect.objectContaining({ runId: run!.id, runStatus: "failed", errorCode: "configuration_incomplete" })); + } finally { proposal.mockReset(); proposal.mockImplementation(defaultProposal); unregisterServerAdapter("claude_local"); await heartbeat.drainActiveRunExecutions(); } + }, 30_000); it("repairs the failed pool account without replacing the pool or adopting the configured provider", async () => { const pool = await makePool({ members: [members[1]!] }); const [agent] = await db.insert(agents).values({ companyId, name: "Pool repair", adapterType: "paperclip_runner", adapterConfig: { provider: "codex", model: "gpt-5.6-sol" }, runtimeConfig: { aiConnection: { mode: "router", connectionId: pool.id } } }).returning(); diff --git a/server/src/__tests__/error-handler.test.ts b/server/src/__tests__/error-handler.test.ts index 52bf838559f..934cf7aa7c0 100644 --- a/server/src/__tests__/error-handler.test.ts +++ b/server/src/__tests__/error-handler.test.ts @@ -2,6 +2,7 @@ import type { NextFunction, Request, Response } from "express"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { HttpError } from "../errors.js"; import { errorHandler } from "../middleware/error-handler.js"; +import { mcpDiscoveryHttpFailure, retainMcpConnectionFailure } from "../services/mcp-connection-failure.js"; const recordResponsibleUserDenialOnActiveRunMock = vi.hoisted(() => vi.fn()); const captureExceptionMock = vi.hoisted(() => vi.fn()); @@ -121,6 +122,35 @@ describe("errorHandler", () => { expect(res.__errorContext?.error?.message).toBe("db exploded"); }); + it("keeps HTTP failures and existing telemetry for proven external MCP outages while skipping Sentry", () => { + const req = makeReq(); + const res = makeRes() as any; + const original = mcpDiscoveryHttpFailure(new Response(null, { status: 503 }), "Remote app returned HTTP 503"); + const err = retainMcpConnectionFailure(original, new HttpError(502, original.message)); + + errorHandler(err, req, res, vi.fn()); + + expect(res.status).toHaveBeenCalledWith(502); + expect(res.json).toHaveBeenCalledWith({ error: original.message }); + expect(res.err).toBe(err); + expect(res.__errorContext.error.message).toBe(original.message); + expect(telemetryMocks.trackErrorHandlerCrash).toHaveBeenCalledOnce(); + expect(captureExceptionMock).not.toHaveBeenCalled(); + }); + + it("reports copied connection markers and unrelated database failures", () => { + for (const err of [ + Object.assign(new HttpError(502, "Remote app returned HTTP 503", { status: 503 }), { + connectionFailure: { schemaVersion: 1, provider: "mcp_http", operation: "discover_tools", reason: "remote_unavailable" }, + }), + new HttpError(500, "database unavailable"), + ]) { + errorHandler(err, makeReq(), makeRes(), vi.fn()); + } + expect(captureExceptionMock).toHaveBeenCalledTimes(2); + expect(telemetryMocks.trackErrorHandlerCrash).toHaveBeenCalledTimes(2); + }); + it("sanitizes chat setup errors before logs and crash reporting", () => { const req = { ...makeReq(), diff --git a/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts b/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts index 279fb75cc4f..74c1b602e79 100644 --- a/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts +++ b/server/src/__tests__/heartbeat-managed-clone-credentials.test.ts @@ -8,6 +8,7 @@ import { ensureManagedProjectWorkspace, prepareProjectRepositoryWorkspaces } fro import { buildGitAuthInvocation, GIT_CREDENTIAL_TOKEN_ENV_KEY } from "../services/git-credentials.ts"; import { sanitizeRuntimeServiceBaseEnv } from "../services/workspace-runtime.ts"; import { resolveManagedProjectWorkspaceDir } from "../home-paths.ts"; +import { readGitConnectionFailure } from "../services/git-connection-failure.js"; const execFile = promisify(execFileCallback); @@ -38,6 +39,79 @@ async function createLocalSourceRepo() { } describe("ensureManagedProjectWorkspace clone credentials", () => { + it.each([ + ["authentication_failed", "fatal: Authentication failed for 'https://example.test/team/repo.git/'"], + ["dns_failure", "ssh: Could not resolve hostname repository-alias.example.test: Name or service not known\nfatal: Could not read from remote repository."], + [null, "fatal: Authentication failed for 'https://example.test/team/repo.git/'\nfatal: bad config line 1 in file /tmp/config"], + [null, "fatal: corrupt loose object"], + ] as const)("preserves only proven outbound clone classification through the actionable wrapper: %s", async (reason, stderr) => { + const bin = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-fake-clone-")); + await fs.writeFile(path.join(bin, "git"), `#!/bin/sh +printf '%s\\n' "$FIXTURE_CLONE_STDERR" >&2 +exit 128 +`, { mode: 0o755 }); + try { + const failure = await ensureManagedProjectWorkspace({ + companyId: "remote-clone-fixture", projectId: path.basename(bin), + repoUrl: "https://fixture-user:fixture-password@example.test/team/repo.git", + resolveGitAuth: async () => ({ + configArgs: [], env: { PATH: bin, FIXTURE_CLONE_STDERR: stderr }, + source: "company_secret", secretName: "GITHUB_TOKEN", + }), + }).catch((error: unknown) => error); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain("Failed to prepare managed checkout"); + expect((failure as Error).message).not.toContain("fixture-password"); + expect(readGitConnectionFailure(failure)).toEqual(reason ? { + schemaVersion: 1, provider: "git", operation: "clone", reason, + } : null); + const homeEntries = await fs.readdir(path.dirname(resolveManagedProjectWorkspaceDir({ + companyId: "remote-clone-fixture", projectId: path.basename(bin), repoName: "repo", + }))); + expect(homeEntries.some((name) => name.includes(".clone-"))).toBe(false); + } finally { + await fs.rm(bin, { recursive: true, force: true }); + } + }); + + it("does not classify a credential-provider exception that resembles a Git failure", async () => { + const failure = Object.assign(new Error("fatal: Authentication failed for 'https://example.test/repo.git'"), { + code: 128, stderr: "fatal: Authentication failed for 'https://example.test/repo.git'", + }); + const caught = await ensureManagedProjectWorkspace({ + companyId: "credential-provider-fixture", projectId: "project", repoUrl: "https://example.test/repo.git", + resolveGitAuth: async () => { throw failure; }, + }).catch((error: unknown) => error); + expect(caught).toBe(failure); + expect(readGitConnectionFailure(caught)).toBeNull(); + }); + + it("does not transfer clone classification to a later local checkout failure", async () => { + const bin = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-fake-checkout-")); + const previousPath = process.env.PATH; + await fs.writeFile(path.join(bin, "git"), `#!/bin/sh +if [ "$1" = clone ]; then + for argument in "$@"; do destination="$argument"; done + /bin/mkdir -p "$destination/.git" + exit 0 +fi +printf '%s\\n' "fatal: Authentication failed for 'https://example.test/repo.git'" >&2 +exit 128 +`, { mode: 0o755 }); + try { + process.env.PATH = bin; + const failure = await ensureManagedProjectWorkspace({ + companyId: "checkout-failure-fixture", projectId: "project", repoUrl: "https://example.test/repo.git", repoRef: "missing-branch", + }).catch((error: unknown) => error); + expect(failure).toBeInstanceOf(Error); + expect((failure as Error).message).toContain("Authentication failed"); + expect(readGitConnectionFailure(failure)).toBeNull(); + } finally { + if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; + await fs.rm(bin, { recursive: true, force: true }); + } + }); + it("materializes every repository-only project row inside the task workspace and reuses local edits", async () => { const first = await createLocalSourceRepo(); const second = await createLocalSourceRepo(); diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 386f2a30ac3..87c14e17b66 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -12,7 +12,7 @@ import { recordLegacyWorkspaceRestoreFailure, terminalizeLegacyExecution } from import { issueService } from "../services/issues.js"; import { getExecutionBlocker } from "../services/execution-blocker.js"; import { adapterExecutionControls, createAdapterExecutionControl } from "../services/adapter-execution-control.js"; -import { spawn, type ChildProcess } from "node:child_process"; +import { execFileSync, spawn, type ChildProcess } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -5591,6 +5591,179 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { } }); + it.each([ + { name: "HTTPS authentication", failures: ["auth"], report: false }, + { name: "SSH alias DNS", failures: ["dns"], report: false }, + { name: "mixed recognized and unknown candidates", failures: ["auth", "unknown"], report: true }, + { name: "mixed local and authentication diagnostics", failures: ["mixed"], report: true }, + { name: "unknown clone failure", failures: ["unknown"], report: true }, + ])("preserves failed workspace recovery while classifying managed Git connections: $name", async ({ failures, report }) => { + const { companyId, agentId, runId, issueId } = await seedQueuedIssueRunFixture(); + const home = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-git-connection-lifecycle-")); + const bin = path.join(home, "bin"); + const callsPath = path.join(home, "git-calls.jsonl"); + const projectId = randomUUID(); + const workspaceIds = failures.map(() => randomUUID()); + const remotes = failures.map((failure, index) => failure === "dns" + ? `git@clone-alias.example.test:team/repository-${index}.git` + : `https://clone.example.test/team/repository-${index}.git`); + const diagnostics = failures.map((failure) => failure === "dns" + ? "ssh: Could not resolve hostname clone-alias.example.test: Name or service not known\nfatal: Could not read from remote repository." + : failure === "unknown" ? "fatal: unexpected internal clone failure" + : "fatal: Authentication failed for 'https://clone.example.test/'" + + (failure === "mixed" ? "\nfatal: No space left on device" : "")); + await fs.mkdir(bin); + // A real child process supplies execFile's numeric exit/stderr evidence. + // Every command is intercepted, so no fixture can reach a remote Git host. + await fs.writeFile(path.join(bin, "git"), `#!${process.execPath}\n` + + `const fs = require('node:fs');\n` + + `const args = process.argv.slice(2);\n` + + `fs.appendFileSync(${JSON.stringify(callsPath)}, JSON.stringify(args) + '\\n');\n` + + `const remotes = ${JSON.stringify(remotes)};\n` + + `const diagnostics = ${JSON.stringify(diagnostics)};\n` + + `const index = remotes.indexOf(args.at(-2));\n` + + `if (!args.includes('clone') || index < 0) { process.stderr.write('Unexpected fixture Git command\\n'); process.exit(2); }\n` + + `process.stderr.write(diagnostics[index] + '\\n'); process.exitCode = 128;\n`, { mode: 0o755 }); + const heartbeat = heartbeatService(db); + const previousSettings = await instanceSettingsService(db).getExperimental(); + const previousPath = process.env.PATH; + const previousHome = process.env.PAPERCLIP_HOME; + try { + process.env.PATH = `${bin}${path.delimiter}${previousPath ?? ""}`; + process.env.PAPERCLIP_HOME = home; + await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: true }); + await db.insert(projects).values({ id: projectId, companyId, name: "Git connection fixture", status: "in_progress" }); + await db.insert(projectWorkspaces).values(workspaceIds.map((id, index) => ({ + id, companyId, projectId, name: `Repository ${index}`, sourceType: "git_repo", + repoUrl: remotes[index], cwd: null, isPrimary: index === 0, + }))); + await db.update(issues).set({ + projectId, projectWorkspaceId: workspaceIds[0], executionWorkspaceSettings: { mode: "isolated_workspace" }, + }).where(eq(issues.id, issueId)); + await db.update(agents).set({ adapterConfig: { workspaceStrategy: { type: "git_worktree" } } }).where(eq(agents.id, agentId)); + + await heartbeat.resumeQueuedRuns(); + await waitForRunToSettle(heartbeat, runId, 5_000); + await heartbeat.waitForRunExecutionDrain(runId); + await waitForPendingRunFailureReports(); + + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + expect(run).toMatchObject({ + status: "failed", errorCode: "workspace_validation_failed", processStartedAt: null, + resultJson: { + workspaceValidation: { reason: "git_worktree_base_materialization_failed" }, + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + }, + }); + expect(run.error).toContain("checkout could not be prepared"); + const validation = run.resultJson!.workspaceValidation as Record; + const materializations = validation.materializationFailures as Array>; + expect(materializations).toHaveLength(failures.length); + expect(materializations.map((failure) => failure.connectionFailure)).toEqual(failures.map((failure) => + failure === "auth" || failure === "dns" ? { + schemaVersion: 1, provider: "git", operation: "clone", + reason: failure === "auth" ? "authentication_failed" : "dns_failure", + } : undefined)); + expect(run.resultJson!.connectionFailure).toEqual(report ? undefined : materializations[0].connectionFailure); + expect(mockAdapterExecute).not.toHaveBeenCalled(); + const [issue] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(issue).toMatchObject({ status: "blocked", executionRunId: null, projectId, projectWorkspaceId: workspaceIds[0] }); + const [action] = await db.select().from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, issueId), + )); + expect(action).toMatchObject({ status: "active", kind: "workspace_validation", ownerType: "board" }); + expect(action.nextAction).toContain("workspace"); + const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId)); + expect(comments.some((comment) => comment.body.includes("workspace failed validation"))).toBe(true); + expect(mockCaptureRunFailure.mock.calls.filter(([event]) => event.runId === runId)).toHaveLength(report ? 1 : 0); + const calls = (await fs.readFile(callsPath, "utf8")).trim().split("\n").map((line) => JSON.parse(line) as string[]); + expect(calls).toHaveLength(failures.length); + expect(calls.every((args) => args.includes("clone"))).toBe(true); + } finally { + await heartbeat.waitForRunExecutionDrain(runId); + if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome; + await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: previousSettings.enableIsolatedWorkspaces }); + await fs.rm(home, { recursive: true, force: true }); + } + }); + + it("keeps a direct additional-repository connection failure visible without reporting it to Sentry", async () => { + const { companyId, agentId, runId, issueId } = await seedQueuedIssueRunFixture(); + const home = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-additional-git-connection-")); + const cwd = path.join(home, "primary"); + const bin = path.join(home, "bin"); + const cloneCalls = path.join(home, "clone-calls.jsonl"); + const projectId = randomUUID(); + const workspaceId = randomUUID(); + const repoUrl = "git@clone-alias.example.test:team/additional.git"; + const previousPath = process.env.PATH; + const previousHome = process.env.PAPERCLIP_HOME; + const heartbeat = heartbeatService(db); + await fs.mkdir(cwd); + await fs.mkdir(bin); + execFileSync("git", ["init", "-b", "main"], { cwd, stdio: "ignore" }); + await fs.writeFile(path.join(cwd, "README.md"), "Keep this local repository intact.\n"); + execFileSync("git", ["add", "README.md"], { cwd, stdio: "ignore" }); + execFileSync("git", ["-c", "user.name=Test", "-c", "user.email=test@example.test", "commit", "-m", "Fixture"], { cwd, stdio: "ignore" }); + await fs.writeFile(path.join(bin, "git"), `#!${process.execPath}\n` + + `const fs = require('node:fs'); const { spawnSync } = require('node:child_process');\n` + + `const args = process.argv.slice(2);\n` + + `if (args.includes('clone')) {\n` + + ` fs.appendFileSync(${JSON.stringify(cloneCalls)}, JSON.stringify(args) + '\\n');\n` + + ` if (args.at(-2) !== ${JSON.stringify(repoUrl)}) process.exit(2);\n` + + ` process.stderr.write('ssh: Could not resolve hostname clone-alias.example.test: Name or service not known\\nfatal: Could not read from remote repository.\\n'); process.exit(128);\n` + + `}\n` + + `if (args.some(arg => ['fetch', 'pull', 'push', 'ls-remote', 'submodule'].includes(arg))) process.exit(2);\n` + + `const result = spawnSync('git', args, { stdio: 'inherit', env: { ...process.env, PATH: ${JSON.stringify(previousPath)} } });\n` + + `process.exit(result.status ?? 2);\n`, { mode: 0o755 }); + try { + process.env.PATH = `${bin}${path.delimiter}${previousPath ?? ""}`; + process.env.PAPERCLIP_HOME = home; + await db.insert(projects).values({ id: projectId, companyId, name: "Additional repository fixture", status: "in_progress" }); + await db.insert(projectWorkspaces).values([ + { id: workspaceId, companyId, projectId, name: "Primary", sourceType: "local_path", cwd, isPrimary: true }, + { id: randomUUID(), companyId, projectId, name: "Additional", sourceType: "git_repo", repoUrl, cwd: null, isPrimary: false }, + ]); + await db.update(issues).set({ projectId, projectWorkspaceId: workspaceId, + executionWorkspaceSettings: { mode: "shared_workspace" }, + }).where(eq(issues.id, issueId)); + await db.update(agents).set({ adapterConfig: { cwd } }).where(eq(agents.id, agentId)); + + await heartbeat.resumeQueuedRuns(); + await waitForRunToSettle(heartbeat, runId, 5_000); + await heartbeat.waitForRunExecutionDrain(runId); + await waitForPendingRunFailureReports(); + + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + expect(run).toMatchObject({ status: "failed", errorCode: "setup_failed", executionStage: "preparing", processStartedAt: null, + resultJson: { + connectionFailure: { schemaVersion: 1, provider: "git", operation: "clone", reason: "dns_failure" }, + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + }, + }); + expect(run.resultJson?.workspaceValidation).toBeUndefined(); + expect(run.error).toContain("Could not resolve hostname"); + expect(mockAdapterExecute).not.toHaveBeenCalled(); + const [issue] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(issue).toMatchObject({ status: "blocked", executionRunId: null, projectId, projectWorkspaceId: workspaceId }); + const [action] = await db.select().from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, issueId), + )); + expect(action).toMatchObject({ status: "active", ownerType: "board" }); + expect(mockCaptureRunFailure.mock.calls.filter(([event]) => event.runId === runId)).toHaveLength(0); + const calls = (await fs.readFile(cloneCalls, "utf8")).trim().split("\n").map((line) => JSON.parse(line) as string[]); + expect(calls).toHaveLength(1); + expect(calls[0].at(-2)).toBe(repoUrl); + expect(await fs.readFile(path.join(cwd, "README.md"), "utf8")).toBe("Keep this local repository intact.\n"); + } finally { + await heartbeat.waitForRunExecutionDrain(runId); + if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = previousHome; + await fs.rm(home, { recursive: true, force: true }); + } + }); + it.each(["throw", "result"])("redacts opaque environment-bound credentials from Sentry diagnostics: %s", async (mode) => { const { companyId, agentId, runId } = await seedQueuedIssueRunFixture(); const svc = secretService(db); diff --git a/server/src/__tests__/heartbeat-workspace-busy.test.ts b/server/src/__tests__/heartbeat-workspace-busy.test.ts index 0d2fc3f1be1..af0d1f63c94 100644 --- a/server/src/__tests__/heartbeat-workspace-busy.test.ts +++ b/server/src/__tests__/heartbeat-workspace-busy.test.ts @@ -14,6 +14,7 @@ import { companies, companySkills, createDb, + closeRegisteredClients, environments, environmentLeases, executionWorkspaces, @@ -46,6 +47,13 @@ import { heartbeatService, } from "../services/heartbeat.ts"; import { instanceSettingsService } from "../services/instance-settings.ts"; +import { reportRunFailure, waitForPendingRunFailureReports } from "../services/run-failure-report.js"; + +const captureRunFailure = vi.hoisted(() => vi.fn()); +vi.mock("../sentry.js", async (importOriginal) => ({ + ...await importOriginal(), + captureRunFailure, +})); // Exercise the real SSH lease and heartbeat paths without connecting to a host. vi.mock("@paperclipai/adapter-utils/ssh", async (importOriginal) => ({ @@ -55,7 +63,8 @@ vi.mock("@paperclipai/adapter-utils/ssh", async (importOriginal) => ({ }), })); -const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL?.trim(); +const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; const WORKSPACE_BUSY_TEST_ADAPTER = "workspace_busy_test"; @@ -91,10 +100,14 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { let workspaceCwd!: string; const executedRunIds: string[] = []; const executedInputs = new Map(); + const failExecutionRunIds = new Set(); beforeAll(async () => { - tempDb = await startEmbeddedPostgresTestDatabase("paperclip-heartbeat-workspace-busy-"); - db = createDb(tempDb.connectionString); + if (externalTestDatabaseUrl) db = createDb(externalTestDatabaseUrl); + else { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-heartbeat-workspace-busy-"); + db = createDb(tempDb.connectionString); + } heartbeat = heartbeatService(db); workspaceCwd = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-workspace-busy-")); registerServerAdapter({ @@ -102,6 +115,15 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { execute: async (input) => { executedRunIds.push(input.runId); executedInputs.set(input.runId, input); + if (failExecutionRunIds.has(input.runId)) { + return { + exitCode: 1, + signal: null, + timedOut: false, + errorMessage: "Synthetic unexpected adapter failure after workspace wait", + resultJson: {}, + }; + } return { exitCode: 0, signal: null, @@ -127,15 +149,20 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { .set({ status: "cancelled", finishedAt: new Date() }) .where(eq(heartbeatRuns.status, "running")); await drainHeartbeatRunsToQuiescence(db, heartbeat); + await waitForPendingRunFailureReports(); await cleanupFixture(); executedRunIds.length = 0; executedInputs.clear(); + failExecutionRunIds.clear(); + captureRunFailure.mockClear(); await instanceSettingsService(db).updateGeneral({ executionMode: "any" }); }); afterAll(async () => { unregisterServerAdapter(WORKSPACE_BUSY_TEST_ADAPTER); if (workspaceCwd) await fs.rm(workspaceCwd, { recursive: true, force: true }); + if (externalTestDatabaseUrl) await closeRegisteredClients(externalTestDatabaseUrl); + await db.$client.end(); await tempDb?.cleanup(); }); @@ -547,6 +574,16 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { holderIssueId: fixture.holderIssueId, deferralAttempt: 0, }); + expect(finishedRun?.resultJson).toMatchObject({ + executionRecovery: { kind: "workspace_wait", providerWorkStarted: false }, + cancellation: { + source: "control_plane", + expected: true, + initiator: { type: "system" }, + reason: "Waiting for the shared project workspace", + recordedAt: finishedRun!.finishedAt!.toISOString(), + }, + }); // The deferred run's adapter never executed — the whole point of the gate. expect(executedRunIds).not.toContain(run!.id); @@ -557,6 +594,8 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { scheduledRetryAttempt: 1, scheduledRetryReason: WORKSPACE_BUSY_RETRY_REASON, }); + await waitForPendingRunFailureReports(); + expect(captureRunFailure.mock.calls.filter(([report]) => report.runId === run!.id)).toHaveLength(0); expect((retryRun?.contextSnapshot as Record | null)?.wakeReason).toBe( WORKSPACE_BUSY_RETRY_WAKE_REASON, ); @@ -603,7 +642,7 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { } }); - it("executes the scheduled retry once the holder has finished", async () => { + it.each([false, true])("executes the scheduled retry without inheriting expected cancellation (adapter fails: %s)", async (adapterFails) => { const fixture = await seedWorkspaceFixture(); const run = await heartbeat.invoke( @@ -618,6 +657,8 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { const retryRun = await waitForRetryRun(run!.id); expect(retryRun?.status).toBe("scheduled_retry"); + expect(retryRun?.resultJson?.cancellation).toBeUndefined(); + if (adapterFails) failExecutionRunIds.add(retryRun!.id); // Holder finishes; the due retry promotes, queues, and executes. await db @@ -631,8 +672,50 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { await heartbeat.resumeQueuedRuns(); const finishedRetry = await waitForRunToLeaveActiveStates(retryRun!.id); - expect(finishedRetry?.status).toBe("succeeded"); + expect(finishedRetry?.status).toBe(adapterFails ? "failed" : "succeeded"); expect(executedRunIds).toContain(retryRun!.id); + expect(finishedRetry?.resultJson?.cancellation).toBeUndefined(); + if (adapterFails) { + await expect.poll(() => captureRunFailure.mock.calls.filter(([report]) => report.runId === retryRun!.id)).toHaveLength(1); + expect(captureRunFailure).toHaveBeenCalledWith(expect.objectContaining({ + runId: retryRun!.id, + runStatus: "failed", + exitCode: 1, + })); + } + await waitForPendingRunFailureReports(); + expect(captureRunFailure.mock.calls.filter(([report]) => report.runId === run!.id)).toHaveLength(0); + }); + + it("does not hide ambiguous, malformed, provider, or transport cancellations behind workspace_busy", async () => { + const fixture = await seedWorkspaceFixture(); + const run = await heartbeat.invoke(fixture.agentId, "assignment", { + issueId: fixture.issueId, wakeReason: "issue_assigned", + }, "system"); + expect(run).not.toBeNull(); + await waitForRunToLeaveActiveStates(run!.id); + await waitForRetryRun(run!.id); + const [deferred] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run!.id)); + expect(deferred?.status).toBe("cancelled"); + const receipt = deferred!.resultJson!.cancellation as Record; + for (const cancellation of [ + undefined, + { ...receipt, expected: "true" }, + { ...receipt, expected: false, source: "provider", initiator: { type: "provider" } }, + { ...receipt, expected: false, source: "transport" }, + ]) { + captureRunFailure.mockClear(); + const unexpected = { + ...deferred!, id: randomUUID(), + resultJson: { ...deferred!.resultJson, cancellation }, + }; + await reportRunFailure(db, unexpected); + expect(captureRunFailure).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ + runId: unexpected.id, + runStatus: "cancelled", + errorCode: WORKSPACE_BUSY_ERROR_CODE, + })); + } }); it("preserves a legacy accepted mention run through workspace retry", async () => { diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index e57c34ff195..906a7d23c6e 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -462,6 +462,29 @@ describe("assertGitSensitiveAdapterWorkspaceValid", () => { }); describe("assertGitWorktreeBaseWorkspaceReady", () => { + it.each(["all_external", "mixed", "malformed"])("keeps materialization aggregation fail-closed: %s", async (kind) => { + const first = { schemaVersion: 1, provider: "git", operation: "clone", reason: "authentication_failed" } as const; + const second = kind === "all_external" ? { ...first, reason: "dns_failure" as const } : + kind === "malformed" ? { ...first, reason: "unknown" } : undefined; + const failures = [first, second].map((connectionFailure, index) => ({ + projectWorkspaceId: `workspace-${index}`, repoUrl: "https://example.test/team/repo.git", + error: "Managed checkout failed", ...(connectionFailure ? { connectionFailure } : {}), + })); + const error = await assertGitWorktreeBaseWorkspaceReady({ + requestedExecutionWorkspaceMode: "isolated_workspace", + config: { workspaceStrategy: { type: "git_worktree" } }, + issue: { id: "issue-1", identifier: "TEST-1", projectId: "project-1", projectWorkspaceId: "workspace-0" }, + base: { baseCwd: "/tmp/unused-fallback", source: "project_primary", projectId: "project-1", workspaceId: "workspace-0", repoUrl: "https://example.test/team/repo.git", repoRef: null }, + anchor: { baseCwdFallback: true, materializationFailures: failures as never }, + }).catch((error: unknown) => error); + expect(error).toMatchObject({ + code: "workspace_validation_failed", + resultJson: { workspaceValidation: { reason: "git_worktree_base_materialization_failed", materializationFailures: failures } }, + }); + expect((error as { resultJson: Record }).resultJson.connectionFailure) + .toEqual(kind === "all_external" ? first : undefined); + }); + it("rejects projectless isolated git worktrees that resolved to agent_home", async () => { const fallbackCwd = resolveDefaultAgentWorkspaceDir("agent-1"); diff --git a/server/src/__tests__/remote-http-endpoint-guard.test.ts b/server/src/__tests__/remote-http-endpoint-guard.test.ts index 3602be470f4..2e23eaeec45 100644 --- a/server/src/__tests__/remote-http-endpoint-guard.test.ts +++ b/server/src/__tests__/remote-http-endpoint-guard.test.ts @@ -1,11 +1,33 @@ import { describe, expect, it } from "vitest"; import { assertPublicRemoteHttpEndpoint } from "../services/remote-http-endpoint-guard.js"; +import { readRemoteConnectionFailure } from "../services/remote-connection-failure.js"; function guardError(message: string, code: string) { return Object.assign(new Error(message), { code }); } describe("remote HTTP endpoint guard", () => { + it.each(["ENOTFOUND", "ENODATA", "EAI_AGAIN"])("retains proven DNS failure %s across the public wrapper", async (code) => { + const failure = await assertPublicRemoteHttpEndpoint(new URL("https://missing.example.test/mcp"), { + lookup: async () => { throw Object.assign(new Error("lookup failed"), { code }); }, + }, guardError).catch((error) => error); + expect(failure).toMatchObject({ code: "remote_http_dns_failed" }); + expect(readRemoteConnectionFailure(failure)).toBe("dns_failure"); + }); + + it("marks its exact DNS deadline without trusting unknown lookup exceptions", async () => { + const timedOut = await assertPublicRemoteHttpEndpoint(new URL("https://silent.example.test/mcp"), { + lookup: async () => new Promise(() => {}), dnsTimeoutMs: 1, + }, guardError).catch((error) => error); + expect(timedOut).toMatchObject({ code: "remote_http_dns_failed" }); + expect(readRemoteConnectionFailure(timedOut)).toBe("connection_timeout"); + const unknown = await assertPublicRemoteHttpEndpoint(new URL("https://broken.example.test/mcp"), { + lookup: async () => { throw new TypeError("internal lookup bug: ENOTFOUND"); }, + }, guardError).catch((error) => error); + expect(unknown).toMatchObject({ code: "remote_http_dns_failed" }); + expect(readRemoteConnectionFailure(unknown)).toBeNull(); + }); + it("blocks hostnames that resolve to private network addresses", async () => { await expect(assertPublicRemoteHttpEndpoint( new URL("https://metadata.example/mcp"), diff --git a/server/src/__tests__/remote-http-rebinding.test.ts b/server/src/__tests__/remote-http-rebinding.test.ts index 6d9b834977a..7730086e184 100644 --- a/server/src/__tests__/remote-http-rebinding.test.ts +++ b/server/src/__tests__/remote-http-rebinding.test.ts @@ -1,9 +1,13 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http"; -import { connect as netConnect, createServer as netCreateServer, type AddressInfo, type Socket } from "node:net"; +import { connect as netConnect, createServer as netCreateServer, Socket, type AddressInfo } from "node:net"; import { gzipSync } from "node:zlib"; import { afterEach, describe, expect, it } from "vitest"; import { guardedRemoteHttpFetch, type RemoteHttpSocketFactory } from "../services/remote-http-fetch.js"; +import { readRemoteConnectionFailure } from "../services/remote-connection-failure.js"; +import { isExpectedMcpConnectionFailure, retainMcpConnectionFailure, withMcpConnectionFailure } from "../services/mcp-connection-failure.js"; +import { HttpError } from "../errors.js"; +import { initializeMcpHttpSession } from "../services/mcp-http.js"; /** * PAP-17098 — DNS-rebinding regression coverage for outbound MCP/OAuth calls. @@ -147,14 +151,16 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => { throw Object.assign(new TypeError("fetch failed"), { cause }); }) as typeof fetch; - await expect(guardedRemoteHttpFetch("https://8.8.8.8/mcp", {}, { + const failure = await guardedRemoteHttpFetch("https://8.8.8.8/mcp", {}, { allowPrivateNetwork: true, unpinnedFetch, error: guardError, - })).rejects.toMatchObject({ + }).catch((error) => error); + expect(failure).toMatchObject({ code: "remote_http_dns_failed", message: "Remote MCP connection hostname could not be resolved", }); + expect(readRemoteConnectionFailure(failure)).toBe("dns_failure"); }); it("pins the connection to the approved address so a rebind never reaches loopback", async () => { @@ -352,13 +358,15 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => { }); const network = routingSocketFactory({ [PUBLIC_ADDRESS]: upstream.port }); - await expect(guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, { + const failure = await guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, { allowPrivateNetwork: false, lookup: async () => [{ address: PUBLIC_ADDRESS, family: 4 }], socketFactory: network.factory, responseTimeoutMs: 150, error: guardError, - })).rejects.toMatchObject({ code: "remote_http_response_timeout" }); + }).catch((error) => error); + expect(failure).toMatchObject({ code: "remote_http_response_timeout" }); + expect(readRemoteConnectionFailure(failure)).toBe("connection_timeout"); // The deadline has to hand back the socket, not just the request handler: // a bounded call that still leaks a descriptor per silent peer is the same @@ -416,11 +424,69 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => { }); expect(response.status).toBe(200); - await expect(response.text()).rejects.toMatchObject({ code: "remote_http_response_timeout" }); + const failure = await response.text().catch((error) => error); + expect(failure).toMatchObject({ code: "remote_http_response_timeout" }); + expect(readRemoteConnectionFailure(failure)).toBe("connection_timeout"); await flush(); expect(network.sockets.map((socket) => socket.destroyed)).toEqual([true]); }); + it("preserves the owned body timeout through the MCP initialization wrapper", async () => { + const upstream = await startServer((_req, res) => { + res.writeHead(200, { "content-type": "application/json" }); + res.write('{"jsonrpc":"2.0",'); + }); + const network = routingSocketFactory({ [PUBLIC_ADDRESS]: upstream.port }); + const failure = await initializeMcpHttpSession({ + requestId: "owned-deadline", + send: (init) => withMcpConnectionFailure(() => guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, init, { + allowPrivateNetwork: false, + lookup: async () => [{ address: PUBLIC_ADDRESS, family: 4 }], + socketFactory: network.factory, + responseTimeoutMs: 150, + error: guardError, + })), + }).catch((error) => error); + expect(failure).toMatchObject({ name: "McpHttpInitializationError", stage: "initialize", status: null, + message: "Remote MCP initialization returned an invalid response" }); + expect(failure.response).toBeUndefined(); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(failure, new HttpError(502, failure.message)))).toBe(true); + await flush(); + expect(network.sockets.map((socket) => socket.destroyed)).toEqual([true]); + }); + + it.each([ + ["EMFILE", "ECONNREFUSED", false], + ["ERR_SSL_UNKNOWN_PROTOCOL", "deadline", false], + ["ECONNRESET", "ECONNREFUSED", true], + ["ETIMEDOUT", "deadline", true], + ] as const)("keeps every failed address relevant to reporting: %s then %s", async (firstCode, lastCode, expected) => { + const first = Object.assign(new Error("first address failed"), { code: firstCode }); + const last = Object.assign(new Error("last address failed"), { code: lastCode }); + const addresses = ["93.184.216.35", PUBLIC_ADDRESS]; + const dialled: string[] = []; + const failure = await withMcpConnectionFailure(() => guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, { + allowPrivateNetwork: false, + lookup: async () => addresses.map((address) => ({ address, family: 4 })), + socketFactory: ({ address }) => { + dialled.push(address); + const socket = new Socket(); + openSockets.push(socket); + if (dialled.length === 1 || lastCode !== "deadline") { + queueMicrotask(() => socket.emit("error", dialled.length === 1 ? first : last)); + } + return socket; + }, + connectTimeoutMs: 5, + error: guardError, + })).catch((error) => error); + expect(dialled).toEqual(addresses); + if (lastCode === "deadline") expect(failure.message).toBe("Timed out trying to connect to the remote MCP endpoint"); + else expect(failure).toBe(last); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(failure, new HttpError(502, failure.message)))).toBe(expected); + expect(openSockets.every((socket) => socket.destroyed)).toBe(true); + }); + it("falls over to the next approved address when the first is unreachable", async () => { // Pinning must not cost the failover `fetch` gave a multi-homed host: one // dead A record should not take the connection down with it. @@ -447,6 +513,29 @@ describe("guarded remote HTTP fetch (PAP-17098 DNS rebinding)", () => { expect(network.dialled).toEqual(["93.184.216.35", PUBLIC_ADDRESS]); }); + it("keeps a successful fallback unchanged even after an unknown first-address failure", async () => { + const upstream = await startServer(); + const network = routingSocketFactory({ [PUBLIC_ADDRESS]: upstream.port }); + const dialled: string[] = []; + const response = await withMcpConnectionFailure(() => guardedRemoteHttpFetch(`http://${REBIND_HOST}/mcp`, {}, { + allowPrivateNetwork: false, + lookup: async () => [{ address: "93.184.216.35", family: 4 }, { address: PUBLIC_ADDRESS, family: 4 }], + socketFactory: (target) => { + dialled.push(target.address); + if (target.address === PUBLIC_ADDRESS) return network.factory(target); + const socket = new Socket(); + openSockets.push(socket); + queueMicrotask(() => socket.emit("error", Object.assign(new Error("local limit"), { code: "EMFILE" }))); + return socket; + }, + error: guardError, + })); + expect(dialled).toEqual(["93.184.216.35", PUBLIC_ADDRESS]); + await expect(response.json()).resolves.toEqual({ ok: true }); + expect(readRemoteConnectionFailure(response)).toBeNull(); + expect(upstream.requests).toHaveLength(1); + }); + it("fails closed instead of falling over when a peer fails the address check", async () => { // Failover covers unreachable addresses only. A peer that answers from an // unapproved address is the rebinding defence firing, and must not be diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 27b8c6d8b4f..ba944f45d45 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -87,6 +87,7 @@ import { import { toolAccessRoutes } from "../routes/tool-access.js"; import { errorHandler } from "../middleware/index.js"; import * as sentry from "../sentry.js"; +import { HttpError } from "../errors.js"; import type { VercelConnectClient } from "../services/vercel-connect.js"; import { invalidatePaperclipCloudConnectorCapabilities, type PaperclipCloudConnector } from "../services/paperclip-cloud-connector.js"; @@ -2965,6 +2966,35 @@ describeEmbeddedPostgres("tool access service", () => { ); }); + it("reports persistence failure after a recognized MCP outage instead of suppressing the database error", async () => { + const company = await createCompany(db); + const [application] = await db.insert(toolApplications).values({ + companyId: company.id, applicationKey: `audit-failure-${randomUUID()}`, + name: "MCP audit fixture", type: "mcp_http", status: "active", + }).returning(); + const [connection] = await db.insert(toolConnections).values({ + companyId: company.id, applicationId: application!.id, name: "MCP audit fixture", + uid: `test/${randomUUID()}`, transport: "mcp_remote", status: "draft", enabled: false, + config: { url: "https://audit-failure.example.test/mcp" }, + transportConfig: { url: "https://audit-failure.example.test/mcp" }, + }).returning(); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response(null, { status: 503 })); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + await db.execute(sql`CREATE FUNCTION test_mcp_audit_failure() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN RAISE EXCEPTION 'synthetic MCP audit write failure'; END $$`); + await db.execute(sql`CREATE TRIGGER test_mcp_audit_failure BEFORE INSERT ON tool_access_audit_events + FOR EACH ROW EXECUTE FUNCTION test_mcp_audit_failure()`); + try { + const response = await request(createRouteApp(db)).post(`/api/tool-connections/${connection!.id}/catalog/refresh`); + expect(response.status).toBe(500); + expect(capture).toHaveBeenCalledOnce(); + expect(capture.mock.calls[0]?.[0]).toMatchObject({ cause: expect.objectContaining({ message: "synthetic MCP audit write failure" }) }); + } finally { + await db.execute(sql`DROP TRIGGER test_mcp_audit_failure ON tool_access_audit_events`); + await db.execute(sql`DROP FUNCTION test_mcp_audit_failure()`); + } + }); + it.each([ [ "local_trusted", @@ -15297,15 +15327,81 @@ describeEmbeddedPostgres("tool access service", () => { }, ); + it.each( + (["catalog", "catalog/refresh", "health-check"] as const).flatMap((path) => + ([401, 403] as const).map((upstreamStatus) => ({ path, upstreamStatus })), + ), + )( + "preserves reconnect guidance and degraded health for explicit OAuth scope failure on $path (HTTP $upstreamStatus)", + async ({ path, upstreamStatus }) => { + const company = await createCompany(db); + const [application] = await db.insert(toolApplications).values({ + companyId: company.id, + applicationKey: `scope-status-${randomUUID()}`, + name: "OAuth scope fixture", + type: "mcp_http", + status: "active", + }).returning(); + const [connection] = await db.insert(toolConnections).values({ + companyId: company.id, + applicationId: application!.id, + name: "OAuth scope fixture", + uid: `test/${randomUUID()}`, + transport: "mcp_remote", + status: "draft", + enabled: false, + config: { url: "https://scope-status.example.test/mcp" }, + transportConfig: { url: "https://scope-status.example.test/mcp" }, + }).returning(); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response( + JSON.stringify({ error: "private provider details" }), + { status: upstreamStatus, headers: { "www-authenticate": 'Bearer error="insufficient_scope", scope="tools:read"' } }, + )); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const app = createRouteApp(db); + const url = `/api/tool-connections/${connection!.id}/${path}`; + const response = await (path === "catalog" ? request(app).get(url) : request(app).post(url)); + + expect(response.status).toBe(422); + expect(response.body).toMatchObject({ + code: "oauth_insufficient_scope", + error: expect.stringContaining("Reconnect this connection"), + details: { + code: "oauth_insufficient_scope", + setupUrl: expect.any(String), + reconnectUrl: expect.any(String), + }, + }); + expect(JSON.stringify(response.body)).not.toContain("private provider details"); + expect(capture).not.toHaveBeenCalled(); + const [updated] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection!.id)); + expect(updated?.healthStatus).toBe("degraded"); + expect(updated?.healthMessage).toBe(response.body.error); + await expect(db.select().from(toolAccessAuditEvents).where(eq(toolAccessAuditEvents.connectionId, connection!.id))) + .resolves.toEqual(expect.arrayContaining([expect.objectContaining({ + action: path === "health-check" ? "tool_connection.health_check" : "tool_connection.catalog_refresh", + outcome: "failure", + reasonCode: "oauth_insufficient_scope", + })])); + await expect(db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connection!.id))) + .resolves.toHaveLength(0); + }, + ); + it.each([ ["catalog", 401, 'Bearer realm="app"', 422, false], ["catalog/refresh", 401, 'Bearer realm="app"', 422, false], ["catalog", 400, null, 502, true], ["catalog/refresh", 400, null, 502, true], - ["catalog", 503, null, 502, true], - ["catalog/refresh", 503, null, 502, true], + ["catalog", 503, null, 502, false], + ["catalog/refresh", 503, null, 502, false], + ["catalog/refresh", 403, null, 502, false], + ["health-check", 503, 'Bearer error="insufficient_scope"', 502, false], + ["catalog/refresh", 404, null, 502, false], + ["catalog/refresh", 429, null, 502, false], + ["health-check", 500, null, 502, false], ] as const)( - "classifies %s upstream HTTP %i without hiding provider failures", + "classifies %s upstream HTTP %i while retaining the failed response", async (path, upstreamStatus, challenge, expectedStatus, reportable) => { const company = await createCompany(db); const [application] = await db @@ -15349,7 +15445,7 @@ describeEmbeddedPostgres("tool access service", () => { : request(app).post(url)); expect(res.status).toBe(expectedStatus); - if (challenge) { + if (challenge && upstreamStatus === 401) { expect(res.body).toMatchObject({ error: "This app needs you to sign in.", code: "oauth_challenge", @@ -15361,6 +15457,9 @@ describeEmbeddedPostgres("tool access service", () => { }); } expect(capture).toHaveBeenCalledTimes(reportable ? 1 : 0); + const [updated] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection!.id)); + expect(updated?.healthStatus).toBe("error"); + expect(updated?.healthMessage).toBe(res.body.error); await expect( db.select().from(toolCatalogEntries) .where(eq(toolCatalogEntries.connectionId, connection!.id)), @@ -15368,6 +15467,92 @@ describeEmbeddedPostgres("tool access service", () => { }, ); + it.each([ + ["initialize reset", false], + ["initialize parser", true], + ["initialize internal reader", true], + ["notification 503", false], + ["notification 400", true], + ] as const)("preserves session-required MCP failure reporting: %s", async (scenario, reportable) => { + const company = await createCompany(db); + const [application] = await db.insert(toolApplications).values({ + companyId: company.id, applicationKey: `session-status-${randomUUID()}`, + name: "MCP session fixture", type: "mcp_http", status: "active", + }).returning(); + const config = { url: "https://session-status.example.test/mcp", mcpSessionRequired: true }; + const [connection] = await db.insert(toolConnections).values({ + companyId: company.id, applicationId: application!.id, name: "MCP session fixture", + uid: `test/${randomUUID()}`, transport: "mcp_remote", status: "draft", enabled: false, + config, transportConfig: config, + }).returning(); + const methods: string[] = []; + vi.spyOn(globalThis, "fetch").mockImplementation(async (_url, init) => { + const request = JSON.parse(String(init?.body)); + methods.push(request.method); + if (request.method === "notifications/initialized") return new Response(null, { status: scenario === "notification 503" ? 503 : 400 }); + expect(request.method).toBe("initialize"); + if (scenario === "initialize parser") return new Response("not JSON"); + if (scenario === "initialize reset" || scenario === "initialize internal reader") { + const error = scenario === "initialize reset" + ? new TypeError("body terminated", { cause: { code: "ECONNRESET" } }) + : new TypeError("internal reader failed"); + return new Response(new ReadableStream({ start(controller) { controller.error(error); } })); + } + return new Response(JSON.stringify({ jsonrpc: "2.0", id: request.id, result: { protocolVersion: "2025-06-18" } })); + }); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const response = await request(createRouteApp(db)).post(`/api/tool-connections/${connection!.id}/catalog/refresh`); + expect(response.status).toBe(502); + expect(response.body.error).toBeTruthy(); + expect(capture).toHaveBeenCalledTimes(reportable ? 1 : 0); + expect(methods).toEqual(scenario.startsWith("notification") ? ["initialize", "notifications/initialized"] : ["initialize"]); + const [updated] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection!.id)); + expect(updated?.healthStatus).toBe("error"); + expect(updated?.healthMessage).toBe(response.body.error); + await expect(db.select().from(toolAccessAuditEvents).where(eq(toolAccessAuditEvents.connectionId, connection!.id))) + .resolves.toEqual(expect.arrayContaining([expect.objectContaining({ outcome: "failure", action: "tool_connection.catalog_refresh" })])); + }); + + it.each([ + ["known transport", () => { throw new TypeError("fetch failed", { cause: { code: "ECONNREFUSED" } }); }, false], + ["direct transport", () => { throw Object.assign(new Error("connection reset"), { code: "ECONNRESET" }); }, false], + ["unknown transport", () => { throw new TypeError("ECONNREFUSED"); }, true], + ["database code", () => { throw Object.assign(new Error("query failed"), { code: "42P01" }); }, true], + ["mixed transport", () => { throw new TypeError("fetch failed", { cause: { errors: [{ code: "ECONNREFUSED" }, { code: "ERR_INTERNAL_ASSERTION" }] } }); }, true], + ["forged classification", () => { throw new HttpError(502, "Remote app returned HTTP 503", { status: 503, connectionFailure: { schemaVersion: 1, provider: "mcp_http", operation: "discover_tools", reason: "remote_unavailable" } }); }, true], + ["malformed response", () => new Response("not JSON"), true], + ["invalid catalog", () => new Response(JSON.stringify({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: {} })), true], + ] as const)( + "preserves MCP diagnostics and reports only unclassified failure: %s", + async (_name, reply, reportable) => { + const company = await createCompany(db); + const [application] = await db.insert(toolApplications).values({ + companyId: company.id, applicationKey: `transport-status-${randomUUID()}`, + name: "MCP transport fixture", type: "mcp_http", status: "active", + }).returning(); + const [connection] = await db.insert(toolConnections).values({ + companyId: company.id, applicationId: application!.id, name: "MCP transport fixture", + uid: `test/${randomUUID()}`, transport: "mcp_remote", status: "draft", enabled: false, + config: { url: "https://transport-status.example.test/mcp" }, + transportConfig: { url: "https://transport-status.example.test/mcp" }, + }).returning(); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => reply()); + const capture = vi.spyOn(sentry, "captureException").mockImplementation(() => {}); + const response = await request(createRouteApp(db)).post(`/api/tool-connections/${connection!.id}/catalog/refresh`); + + expect(response.status).toBe(502); + expect(response.body.error).toBeTruthy(); + expect(capture).toHaveBeenCalledTimes(reportable ? 1 : 0); + const [updated] = await db.select().from(toolConnections).where(eq(toolConnections.id, connection!.id)); + expect(updated?.healthStatus).toBe("error"); + expect(updated?.healthMessage).toBe(response.body.error); + await expect(db.select().from(toolAccessAuditEvents).where(eq(toolAccessAuditEvents.connectionId, connection!.id))) + .resolves.toEqual(expect.arrayContaining([expect.objectContaining({ outcome: "failure", action: "tool_connection.catalog_refresh" })])); + await expect(db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connection!.id))) + .resolves.toHaveLength(0); + }, + ); + it.each([ [ "local_trusted", diff --git a/server/src/middleware/error-handler.ts b/server/src/middleware/error-handler.ts index 31dd8535e62..23103185c79 100644 --- a/server/src/middleware/error-handler.ts +++ b/server/src/middleware/error-handler.ts @@ -13,6 +13,7 @@ import { redactSensitiveValueOccurrences, } from "./redact-sensitive.js"; import { recordResponsibleUserDenialOnActiveRun } from "../services/responsible-user-denial-run-outcomes.js"; +import { isExpectedMcpConnectionFailure } from "../services/mcp-connection-failure.js"; export interface ErrorContext { error: { @@ -85,10 +86,10 @@ function sanitizeSecretSensitiveResponse( } /** Report a server-side crash to every error sink. */ -function reportCrash(error: Error): void { +function reportCrash(error: Error, reportToSentry = true): void { const tc = getTelemetryClient(); if (tc) trackErrorHandlerCrash(tc, { errorCode: error.name }); - captureException(error); + if (reportToSentry) captureException(error); } function getPaperclipDb(req: Request): Db | null { @@ -176,7 +177,7 @@ export function errorHandler( }, reportableError, ); - reportCrash(reportableError); + reportCrash(reportableError, !isExpectedMcpConnectionFailure(err)); } const secretSensitiveServerError = err.status >= 500 && diff --git a/server/src/services/__tests__/git-connection-failure.test.ts b/server/src/services/__tests__/git-connection-failure.test.ts new file mode 100644 index 00000000000..17692dfe407 --- /dev/null +++ b/server/src/services/__tests__/git-connection-failure.test.ts @@ -0,0 +1,202 @@ +import { describe, expect, it } from "vitest"; +import type { GitConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; +import { + classifyGitCloneFailure, + GitConnectionFailureError, + readGitConnectionFailure, +} from "../git-connection-failure.js"; + +const httpsRemote = "https://example.test/team/repository.git"; +const sshRemote = "git@example.test:team/repository.git"; +const authenticationStderr = `fatal: Authentication failed for '${httpsRemote}/'`; + +function childFailure(stderr: string, overrides: Record = {}) { + return Object.assign(new Error("Git clone subprocess failed"), { + code: 128, + stderr, + signal: null, + killed: false, + ...overrides, + }); +} + +function classification(reason: GitConnectionFailure["reason"]): GitConnectionFailure { + return { schemaVersion: 1, provider: "git", operation: "clone", reason }; +} + +describe("Git clone connection failure classification", () => { + it.each([ + ["HTTPS authentication", httpsRemote, authenticationStderr, "authentication_failed"], + ["disabled username prompt", httpsRemote, + "fatal: could not read Username for 'https://example.test': terminal prompts disabled", "authentication_failed"], + ["disabled password prompt", httpsRemote, + "fatal: could not read Password for 'https://git@example.test': terminal prompts disabled", "authentication_failed"], + ["SSH public key", sshRemote, + "git@example.test: Permission denied (publickey).\nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.", "authentication_failed"], + ["private repository", httpsRemote, + `remote: Repository not found.\nfatal: repository '${httpsRemote}/' not found`, "repository_unavailable"], + ["HTTP unauthorized", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 401`, "authentication_failed"], + ["HTTP forbidden", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 403`, "authentication_failed"], + ["HTTP missing repository", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 404`, "repository_unavailable"], + ["invalid HTTP port", "https://example.test:invalid/team/repository.git", + "fatal: unable to access 'https://example.test:invalid/team/repository.git/': Port number was not a decimal number between 0 and 65535", "invalid_remote"], + ["invalid HTTP URL", "https://example.test:invalid/team/repository.git", + "fatal: unable to access 'https://example.test:invalid/team/repository.git/': URL rejected: Port number was not a decimal number between 0 and 65535", "invalid_remote"], + ["SSH alias DNS", "git@repo-alias.example.test:team/repository.git", + "ssh: Could not resolve hostname repo-alias.example.test: Name or service not known\nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.", "dns_failure"], + ["HTTPS DNS", httpsRemote, + `fatal: unable to access '${httpsRemote}/': Could not resolve host: example.test`, "dns_failure"], + ["SSH refusal", sshRemote, + "ssh: connect to host example.test port 22: Connection refused\nfatal: Could not read from remote repository.", "connection_refused"], + ["HTTPS refusal", httpsRemote, + `fatal: unable to access '${httpsRemote}/': Failed to connect to example.test: Connection refused`, "connection_refused"], + ["SSH unreachable network", "ssh://git@example.test/team/repository.git", + "ssh: connect to host example.test port 22: Network is unreachable\nfatal: Could not read from remote repository.", "network_unreachable"], + ["SSH missing route", sshRemote, + "ssh: connect to host example.test port 22: No route to host\nfatal: Could not read from remote repository.", "network_unreachable"], + ["HTTPS reset", httpsRemote, + `fatal: unable to access '${httpsRemote}/': Recv failure: Connection was reset`, "connection_reset"], + ["SSH reset", sshRemote, + "Connection reset by example.test port 22\nfatal: Could not read from remote repository.", "connection_reset"], + ["SSH timeout", sshRemote, + "ssh: connect to host example.test port 22: Connection timed out\nfatal: Could not read from remote repository.", "connection_timeout"], + ["HTTPS timeout", httpsRemote, + `fatal: unable to access '${httpsRemote}/': Operation timed out after 30000 milliseconds with 0 bytes received`, "connection_timeout"], + ["HTTPS certificate", httpsRemote, + `fatal: unable to access '${httpsRemote}/': SSL certificate problem: unable to get local issuer certificate`, "tls_failure"], + ["HTTPS rate limit", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 429`, "rate_limited"], + ["HTTPS service unavailable", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 503`, "remote_unavailable"], + ["HTTPS bad gateway", httpsRemote, + `fatal: unable to access '${httpsRemote}/': The requested URL returned error: 502`, "remote_unavailable"], + ] as const)("classifies the actual remote clone diagnostic: %s", (_name, remote, stderr, reason) => { + expect(classifyGitCloneFailure(remote, childFailure(`Cloning into 'repository'...\n${stderr}\n`))) + .toEqual(classification(reason)); + }); + + it.each([ + "/tmp/repository", "./repository", "../repository", "repository", "~/repository", + "C:\\repository", "C:/repository", "file:///tmp/repository", "file://example.test/repository", + "ftp://example.test/repository", "gopher://example.test/repository", "ext::example.test/repository", + "https::example.test/repository", "", "example.test/repository", + ])("keeps local paths and unsupported transports reportable: %s", (remote) => { + expect(classifyGitCloneFailure(remote, childFailure(authenticationStderr))).toBeNull(); + }); + + it.each([ + ["missing Git executable", { code: "ENOENT" }], + ["string exit code", { code: "128" }], + ["successful exit", { code: 0 }], + ["non-fatal exit", { code: 1 }], + ["command error exit", { code: 2 }], + ["usage exit", { code: 129 }], + ["negative exit", { code: -1 }], + ["fractional exit", { code: 128.5 }], + ["unknown exit", { code: undefined }], + ["nonfinite exit", { code: Infinity }], + ["killed child", { killed: true }], + ["terminated child", { signal: "SIGTERM" }], + ["missing stderr", { stderr: undefined }], + ["buffer stderr", { stderr: Buffer.from(authenticationStderr) }], + ["object stderr", { stderr: { message: authenticationStderr } }], + ] as const)("requires unambiguous child-process failure evidence: %s", (_name, overrides) => { + expect(classifyGitCloneFailure(httpsRemote, childFailure(authenticationStderr, overrides))).toBeNull(); + }); + + it.each([ + "fatal: No space left on device", + "fatal: Read-only file system", + "fatal: Input/output error", + "fatal: Too many open files", + "fatal: Cannot allocate memory", + "fatal: could not create work tree dir 'repository': Permission denied", + "fatal: cannot open file 'repository/.git/config': Permission denied", + "fatal: bad object HEAD", + "fatal: corrupt loose object", + "fatal: invalid index", + "fatal: not a git repository", + "fatal: detected dubious ownership in repository", + "fatal: unexpected internal clone failure", + "error: unrelated local helper failed", + "warning: unrelated local configuration failure", + ])("does not hide a mixed local or unknown diagnostic: %s", (otherDiagnostic) => { + for (const stderr of [ + `${authenticationStderr}\n${otherDiagnostic}`, + `${otherDiagnostic}\n${authenticationStderr}`, + ]) { + expect(classifyGitCloneFailure(httpsRemote, childFailure(stderr))).toBeNull(); + } + }); + + it.each([ + "", "fatal: Could not read from remote repository.", + "fatal: destination path 'repository' already exists and is not an empty directory.", + "fatal: unable to access 'https://example.test/team/repository.git/': The requested URL returned error: 418", + "fatal: invalid object name 'The requested URL returned error: 503'", + "fatal: cannot open file 'Permission denied (publickey)': Permission denied", + "error: local helper reported fatal: Authentication failed before crashing", + ])("does not infer a remote cause from ambiguous stderr: %s", (stderr) => { + const error = childFailure(stderr); + error.message = authenticationStderr; + expect(classifyGitCloneFailure(httpsRemote, error)).toBeNull(); + }); + + it("does not classify messages or serialized child-error lookalikes", () => { + for (const error of [ + authenticationStderr, + new Error(authenticationStderr), + { message: authenticationStderr, code: 128, stderr: authenticationStderr }, + null, + ]) { + expect(classifyGitCloneFailure(httpsRemote, error)).toBeNull(); + } + }); +}); + +describe("producer-owned Git connection failure errors", () => { + it("preserves the existing error message and returns only the bounded classification", () => { + const diagnostic = classifyGitCloneFailure(httpsRemote, childFailure(authenticationStderr)); + expect(diagnostic).toEqual(classification("authentication_failed")); + const error = new GitConnectionFailureError("Unable to prepare project repository", diagnostic!); + + expect(error).toBeInstanceOf(Error); + expect(error.name).toBe("GitConnectionFailureError"); + expect(error.message).toBe("Unable to prepare project repository"); + expect(readGitConnectionFailure(error)).toEqual(classification("authentication_failed")); + expect(JSON.stringify(readGitConnectionFailure(error))).not.toContain("example.test"); + expect(error).not.toHaveProperty("connectionFailure"); + expect(error).not.toHaveProperty("stderr"); + }); + + it("does not let caller mutations change retained failure evidence", () => { + const input = classification("authentication_failed"); + const error = new GitConnectionFailureError("Clone failed", input); + input.reason = "remote_unavailable"; + const firstRead = readGitConnectionFailure(error)!; + expect(firstRead).toEqual(classification("authentication_failed")); + firstRead.reason = "dns_failure"; + expect(readGitConnectionFailure(error)).toEqual(classification("authentication_failed")); + expect(readGitConnectionFailure(error)).not.toBe(firstRead); + }); + + it("rejects spoofed markers, prototype lookalikes, and serialized errors", () => { + const diagnostic = classification("authentication_failed"); + const real = new GitConnectionFailureError("Clone failed", diagnostic); + const spoofed = Object.assign(new Error("Clone failed"), { + name: "GitConnectionFailureError", connectionFailure: diagnostic, + }); + for (const value of [ + null, "Clone failed", diagnostic, { connectionFailure: diagnostic }, spoofed, + Object.assign(Object.create(GitConnectionFailureError.prototype), spoofed), + JSON.parse(JSON.stringify(real)), + new Error("Wrapper", { cause: real }), + ]) { + expect(readGitConnectionFailure(value)).toBeNull(); + } + expect(readGitConnectionFailure(real)).toEqual(diagnostic); + }); +}); diff --git a/server/src/services/__tests__/mcp-connection-failure.test.ts b/server/src/services/__tests__/mcp-connection-failure.test.ts new file mode 100644 index 00000000000..6a422a1af31 --- /dev/null +++ b/server/src/services/__tests__/mcp-connection-failure.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; +import { HttpError } from "../../errors.js"; +import { isExpectedMcpConnectionFailure, retainMcpConnectionFailure, withMcpConnectionFailure } from "../mcp-connection-failure.js"; +import { assertPublicRemoteHttpEndpoint } from "../remote-http-endpoint-guard.js"; +import { classifyRemoteConnectionError, markRemoteConnectionFailure, readRemoteConnectionFailure, recordRemoteConnectionAttempts } from "../remote-connection-failure.js"; + +describe("MCP outbound failure provenance", () => { + it("lets an unknown address override existing MCP and owned-timeout receipts on the final error", async () => { + const last = markRemoteConnectionFailure(Object.assign(new HttpError(502, "last address timed out"), { code: "ETIMEDOUT" }), "connection_timeout"); + await expect(withMcpConnectionFailure(async () => { throw last; })).rejects.toBe(last); + expect(isExpectedMcpConnectionFailure(last)).toBe(true); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(last, new HttpError(502, last.message)))).toBe(true); + const unknown = Object.assign(new Error("first address exhausted file descriptors"), { code: "EMFILE" }); + expect(recordRemoteConnectionAttempts(last, [unknown, last])).toBe(last); + expect(readRemoteConnectionFailure(last)).toBeNull(); + expect(classifyRemoteConnectionError(last)).toBeNull(); + expect(classifyRemoteConnectionError(new TypeError("wrapped", { cause: last }))).toBeNull(); + expect(isExpectedMcpConnectionFailure(last)).toBe(false); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(last, new HttpError(502, last.message)))).toBe(false); + await expect(withMcpConnectionFailure(async () => { throw last; })).rejects.toBe(last); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(last, new HttpError(502, last.message)))).toBe(false); + }); + + it.each([ + "ECONNREFUSED", "ENOTFOUND", "ENODATA", "EAI_AGAIN", "EHOSTUNREACH", "ENETUNREACH", + "ECONNRESET", "EPIPE", "UND_ERR_SOCKET", "ETIMEDOUT", "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", "UND_ERR_BODY_TIMEOUT", "CERT_HAS_EXPIRED", "CERT_NOT_YET_VALID", + "DEPTH_ZERO_SELF_SIGNED_CERT", "SELF_SIGNED_CERT_IN_CHAIN", "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "UNABLE_TO_GET_ISSUER_CERT_LOCALLY", "ERR_TLS_CERT_ALTNAME_INVALID", + ])("recognizes the structured transport failure %s without changing the thrown error", async (code) => { + const original = new TypeError("request failed", { cause: { code } }); + await expect(withMcpConnectionFailure(async () => { throw original; })).rejects.toBe(original); + const wrapped = retainMcpConnectionFailure(original, new HttpError(502, original.message)); + expect(isExpectedMcpConnectionFailure(wrapped)).toBe(true); + expect(Object.keys(original)).not.toContain("connectionFailure"); + }); + + it.each([ + new TypeError("ECONNREFUSED"), + new DOMException("request expired", "TimeoutError"), + Object.assign(new Error("cancelled", { cause: { code: "ECONNRESET" } }), { name: "AbortError" }), + Object.assign(new Error("too many files"), { code: "EMFILE" }), + new TypeError("request failed", { cause: { code: "toString" } }), + new TypeError("request failed", { cause: { code: "ECONNRESET", errors: [] } }), + new TypeError("request failed", { cause: { errors: [{ code: "ECONNRESET" }, { code: "ERR_INTERNAL_ASSERTION" }] } }), + ])("keeps unknown, local, cancellation, and mixed failures visible: %s", async (original) => { + await expect(withMcpConnectionFailure(async () => { throw original; })).rejects.toBe(original); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(original, new HttpError(502, original.message)))) + .toBe(false); + }); + + it("does not carry provenance onto a subsequent independent error", async () => { + const transport = Object.assign(new Error("connection reset"), { code: "ECONNRESET" }); + await expect(withMcpConnectionFailure(async () => { throw transport; })).rejects.toBe(transport); + const persistence = new Error("database write failed", { cause: transport }); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(persistence, new HttpError(502, persistence.message)))) + .toBe(false); + }); + + it.each(["known DNS", "DNS deadline", "unknown lookup"])("retains only producer-proven %s across discovery wrapping", async (kind) => { + const original = await assertPublicRemoteHttpEndpoint(new URL("https://missing.example.test/mcp"), { + dnsTimeoutMs: 1, + lookup: async () => { + if (kind === "known DNS") throw Object.assign(new Error("lookup failed"), { code: "ENOTFOUND" }); + if (kind === "unknown lookup") throw new TypeError("lookup implementation failed"); + return new Promise(() => {}); + }, + }, (message, code) => new HttpError(400, message, { code })).catch((error) => error); + expect(original).toBeInstanceOf(HttpError); + const wrapped = retainMcpConnectionFailure(original, new HttpError(502, original.message)); + expect(isExpectedMcpConnectionFailure(wrapped)).toBe(kind !== "unknown lookup"); + const unrelated = new Error("database failed", { cause: original }); + expect(isExpectedMcpConnectionFailure(retainMcpConnectionFailure(unrelated, new HttpError(502, unrelated.message)))) + .toBe(false); + }); +}); diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index 6271b6f520c..be38e96b93b 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import os from "node:os"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { agents, companies, createDb, heartbeatRuns, type Db } from "@paperclipai/db"; +import { agents, applyPendingMigrations, closeRegisteredClients, companies, createDb, heartbeatRuns, type Db } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, @@ -30,7 +30,8 @@ vi.mock("../../log-redaction.js", async (importOriginal) => { import { reportRunFailure, waitForPendingRunFailureReports } from "../run-failure-report.js"; import { REDACTED_EVENT_VALUE } from "../../redaction.js"; -const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const testDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL; +const embeddedPostgresSupport = testDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; describeEmbeddedPostgres("reportRunFailure", () => { @@ -41,8 +42,13 @@ describeEmbeddedPostgres("reportRunFailure", () => { let inheritedEnv: NodeJS.ProcessEnv; beforeAll(async () => { - tempDb = await startEmbeddedPostgresTestDatabase("paperclip-run-failure-report-"); - db = createDb(tempDb.connectionString); + if (testDatabaseUrl) { + await applyPendingMigrations(testDatabaseUrl); + db = createDb(testDatabaseUrl); + } else { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-run-failure-report-"); + db = createDb(tempDb.connectionString); + } }, 20_000); beforeEach(() => { @@ -66,6 +72,7 @@ describeEmbeddedPostgres("reportRunFailure", () => { }); afterAll(async () => { + if (testDatabaseUrl) await closeRegisteredClients(testDatabaseUrl); await tempDb?.cleanup(); }); @@ -110,6 +117,172 @@ describeEmbeddedPostgres("reportRunFailure", () => { } as unknown as typeof heartbeatRuns.$inferSelect; } + const gitConnection = { schemaVersion: 1, provider: "git", operation: "clone", reason: "authentication_failed" }; + const hermesConnection = { schemaVersion: 1, provider: "hermes_gateway", operation: "create_run", reason: "connection_refused" }; + + function gitConnectionRun(overrides: Partial = {}) { + return buildRun({ + errorCode: "setup_failed", executionStage: "preparing", exitCode: null, signal: null, + resultJson: { connectionFailure: gitConnection, + executionRecovery: { kind: "bootstrap", providerWorkStarted: false } }, + ...overrides, + }); + } + + function hermesConnectionRun(overrides: Partial = {}) { + return buildRun({ + errorCode: "hermes_gateway_connect_failed", executionStage: "executing", exitCode: 1, signal: null, + resultJson: { connectionFailure: hermesConnection }, ...overrides, + }); + } + + it("keeps a proven Git clone failure and its persisted task error unchanged", async () => { + await seedCompanyAndAgent(); + const input = gitConnectionRun(); + const [run] = await db.insert(heartbeatRuns).values(input).returning(); + const before = structuredClone(run); + await reportRunFailure(db, run!, { phase: "setup" }); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + expect(run).toEqual(before); + expect(await db.select().from(heartbeatRuns)).toEqual([before]); + }); + + it("keeps an all-external Git materialization aggregate local even across different external causes", async () => { + await seedCompanyAndAgent(); + const run = gitConnectionRun({ errorCode: "workspace_validation_failed" }); + run.resultJson!.workspaceValidation = { reason: "git_worktree_base_materialization_failed", materializationFailures: [ + { connectionFailure: gitConnection }, + { connectionFailure: { ...gitConnection, reason: "repository_unavailable" } }, + ] }; + const before = structuredClone(run); + await reportRunFailure(db, run, { phase: "setup" }); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + expect(run).toEqual(before); + }); + + it.each([ + { status: "timed_out" }, { executionStage: "executing" }, { executionStage: null }, + { errorCode: "adapter_failed" }, { exitCode: 1 }, { exitCode: 0 }, { signal: "SIGTERM" }, + { resultJson: {} }, + { resultJson: { connectionFailure: { ...gitConnection, schemaVersion: 2 }, executionRecovery: { kind: "bootstrap", providerWorkStarted: false } } }, + { resultJson: { connectionFailure: hermesConnection, executionRecovery: { kind: "bootstrap", providerWorkStarted: false } } }, + ])("reports a Git marker without its matching setup outcome: %j", async (overrides) => { + await seedCompanyAndAgent(); + await reportRunFailure(db, gitConnectionRun(overrides), { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each([undefined, "execute"] as const)("reports Git failures outside setup: %s", async (phase) => { + await seedCompanyAndAgent(); + await reportRunFailure(db, gitConnectionRun(), { phase }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each([undefined, { kind: "bootstrap" }, { kind: "bootstrap", providerWorkStarted: true }, + { kind: "runtime", providerWorkStarted: false }])("reports Git failures without unstarted bootstrap proof: %j", async (recovery) => { + await seedCompanyAndAgent(); + const run = gitConnectionRun(); + run.resultJson!.executionRecovery = recovery; + await reportRunFailure(db, run, { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each([ + [], [null], [{ connectionFailure: gitConnection }, {}], + [{ connectionFailure: gitConnection }, { connectionFailure: hermesConnection }], + [{ connectionFailure: gitConnection }, { connectionFailure: { ...gitConnection, reason: "unknown" } }], + [{ connectionFailure: { ...gitConnection, reason: "repository_unavailable" } }], + ].map((failures) => ({ failures })))("reports mixed, malformed, or inconsistent Git materialization evidence: %j", async ({ failures }) => { + await seedCompanyAndAgent(); + const run = gitConnectionRun({ errorCode: "workspace_validation_failed" }); + run.resultJson!.workspaceValidation = { reason: "git_worktree_base_materialization_failed", materializationFailures: failures }; + await reportRunFailure(db, run, { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each([ + { reason: "git_worktree_base_materialization_failed", materializationFailures: [{ connectionFailure: gitConnection }] }, + { reason: "git_worktree_not_reusable" }, + ])("does not use setup_failed to bypass separate workspace validation evidence: %j", async (validation) => { + await seedCompanyAndAgent(); + const run = gitConnectionRun(); + run.resultJson!.workspaceValidation = validation; + await reportRunFailure(db, run, { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each([ + ["configuration", "endpoint_missing", "hermes_gateway_api_base_url_missing"], + ["configuration", "endpoint_invalid", "hermes_gateway_api_base_url_invalid"], + ["configuration", "insecure_transport", "hermes_gateway_plain_http_remote_denied"], + ["configuration", "credentials_missing", "hermes_gateway_api_key_missing"], + ["create_run", "authentication_failed", "hermes_gateway_auth_failed"], + ["create_run", "endpoint_not_found", "hermes_gateway_runs_unsupported"], + ["create_run", "rate_limited", "hermes_gateway_rate_limited"], + ["create_run", "remote_unavailable", "hermes_gateway_upstream_error"], + ...["connection_refused", "dns_failure", "network_unreachable", "connection_reset", "connection_timeout", "tls_failure"] + .flatMap((reason) => ["hermes_gateway_connect_failed", "hermes_gateway_protocol_error"].map((code) => ["create_run", reason, code])), + ])("keeps the matching Hermes %s/%s task error local", async (operation, reason, errorCode) => { + await seedCompanyAndAgent({ adapterType: "hermes_gateway" }); + const run = hermesConnectionRun({ errorCode, resultJson: { connectionFailure: { ...hermesConnection, operation, reason } } }); + const before = structuredClone(run); + await reportRunFailure(db, run, { phase: "execute" }); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + expect(run).toEqual(before); + }); + + it.each([undefined, { kind: "bootstrap", providerWorkStarted: true }, { kind: "runtime", providerWorkStarted: true }])( + "does not infer remote nonacceptance from a classified Hermes connection failure: %j", async (recovery) => { + await seedCompanyAndAgent({ adapterType: "hermes_gateway" }); + const input = hermesConnectionRun(); + input.resultJson!.executionRecovery = recovery; + const [run] = await db.insert(heartbeatRuns).values(input).returning(); + const before = structuredClone(run); + await reportRunFailure(db, run!); + expect(mockCaptureRunFailure).not.toHaveBeenCalled(); + expect(await db.select().from(heartbeatRuns)).toEqual([before]); + }, + ); + + it.each([ + { status: "timed_out" }, { status: "cancelled", startedAt: new Date(0) }, + { exitCode: 0 }, { exitCode: null }, { signal: "SIGTERM" }, + { errorCode: "hermes_gateway_timeout" }, { errorCode: "hermes_gateway_run_failed" }, + { errorCode: "hermes_gateway_auth_failed" }, + { resultJson: {} }, + { resultJson: { connectionFailure: { ...hermesConnection, reason: "unknown" } } }, + { resultJson: { connectionFailure: { ...hermesConnection, privateUrl: "https://private.example.test" } } }, + { resultJson: { connectionFailure: gitConnection } }, + { resultJson: { connectionFailure: { ...hermesConnection, operation: "configuration", reason: "credentials_missing" } } }, + ])("reports an unproven or mismatched Hermes outcome: %j", async (overrides) => { + await seedCompanyAndAgent({ adapterType: "hermes_gateway" }); + const run = hermesConnectionRun(overrides); + const before = structuredClone(run); + await reportRunFailure(db, run, { phase: "execute" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + expect(run).toEqual(before); + }); + + it("keeps setup errors reportable even when they carry a matching Hermes marker", async () => { + await seedCompanyAndAgent({ adapterType: "hermes_gateway" }); + await reportRunFailure(db, hermesConnectionRun(), { phase: "setup" }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + }); + + it.each(["codex_local", "hermes_local"])("requires the actual database adapter, not a Hermes marker on %s", async (adapterType) => { + await seedCompanyAndAgent({ adapterType }); + const run = hermesConnectionRun({ contextSnapshot: { adapterType: "hermes_gateway" } }); + await reportRunFailure(db, run); + expect(mockCaptureRunFailure).toHaveBeenCalledWith(expect.objectContaining({ agentAdapter: adapterType })); + }); + + it.each(["missing", "other_company"])("does not suppress Hermes when its database agent is %s", async (kind) => { + await seedCompanyAndAgent({ adapterType: "hermes_gateway" }); + const run = hermesConnectionRun(kind === "missing" ? { agentId: randomUUID() } : { companyId: randomUUID() }); + await reportRunFailure(db, run); + expect(mockCaptureRunFailure).toHaveBeenCalledWith(expect.objectContaining({ agentAdapter: "unknown" })); + }); + function missingSecretRun(overrides: Partial = {}) { return buildRun({ errorCode: "configuration_incomplete", diff --git a/server/src/services/git-connection-failure.ts b/server/src/services/git-connection-failure.ts new file mode 100644 index 00000000000..f8b7cac02de --- /dev/null +++ b/server/src/services/git-connection-failure.ts @@ -0,0 +1,90 @@ +import type { GitConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; + +const classifiedErrors = new WeakMap(); + +/** Only the outbound Git call may create this reporting classification. */ +export class GitConnectionFailureError extends Error { + constructor(message: string, failure: GitConnectionFailure) { + super(message); + this.name = "GitConnectionFailureError"; + classifiedErrors.set(this, { ...failure }); + } +} + +export function readGitConnectionFailure(error: unknown): GitConnectionFailure | null { + const failure = error instanceof Error ? classifiedErrors.get(error) : undefined; + return failure ? { ...failure } : null; +} + +function isNetworkRemote(remote: string): boolean { + // Local paths/file remotes and arbitrary remote helpers are not connections. + // Keep malformed HTTP/SSH URLs eligible only for Git's explicit URL error. + if (/^(?:https?|ssh|git):\/\//i.test(remote)) return true; + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(remote) || remote.includes("::")) return false; + return /^(?:[^\s/@:]+@)?[a-z0-9][a-z0-9.-]+:[^:\s][^\s]*$/i.test(remote) && !/^[a-z]:[\\/]/i.test(remote); +} + +/** + * Read the actual failed clone subprocess, not a wrapped error message. These + * diagnostics belong to the remote transport; filesystem/repository damage + * and mixed failures remain application errors even if auth text is present. + */ +export function classifyGitCloneFailure(remote: string, error: unknown): GitConnectionFailure | null { + if (!isNetworkRemote(remote) || !(error instanceof Error)) return null; + const failure = error as Error & { code?: unknown; stderr?: unknown; signal?: unknown; killed?: unknown }; + if (failure.code !== 128 || failure.signal != null || failure.killed === true || typeof failure.stderr !== "string") return null; + const lines = failure.stderr.split(/\r?\n/).map((line) => line.trim()).filter(Boolean); + let reason: GitConnectionFailure["reason"] | undefined; + for (const line of lines) { + const classified = classifyTransportLine(line); + if (classified) { + reason ??= classified; + continue; + } + // Git and SSH append these fixed progress/help lines to recognized + // transport errors. Unknown diagnostics, including a second local failure, + // keep the whole operation reportable. The boilerplate alone proves nothing. + if (/^Cloning into ['"].+['"]\.\.\.$/.test(line) || + /^(?:remote: )?(?:Enumerating|Counting|Compressing|Receiving|Resolving|Updating) (?:objects|deltas|files):/.test(line) || + /^remote: Total \d+ /.test(line) || + /^fatal: Could not read from remote repository\.$/.test(line) || + /^Please make sure you have the correct access rights$/.test(line) || + /^and the repository exists\.$/.test(line) || + /^remote: Please see https:\/\/[^\s]+ for information/.test(line)) continue; + return null; + } + return reason ? { schemaVersion: 1, provider: "git", operation: "clone", reason } : null; +} + +function classifyTransportLine(line: string): GitConnectionFailure["reason"] | undefined { + // Parse the transport's own diagnostic prefix before examining its detail. + // A local error may quote exactly the same words in a path or object name. + const http = /^fatal: unable to access ['"][^'"\r\n]+['"]: (.+)$/i.exec(line)?.[1]; + if (http) { + const status = /^The requested URL returned error: (\d{3})$/i.exec(http)?.[1]; + if (status === "401" || status === "403") return "authentication_failed"; + if (status === "404") return "repository_unavailable"; + if (status === "429") return "rate_limited"; + if (status && /^5\d\d$/.test(status)) return "remote_unavailable"; + if (/^(?:URL (?:using bad\/illegal format|rejected)|Port number was not a decimal number)/i.test(http)) return "invalid_remote"; + if (/^Could not resolve host: [^\s]+$/i.test(http)) return "dns_failure"; + if (/^Failed to connect to .+: Connection refused$/i.test(http)) return "connection_refused"; + if (/^Failed to connect to .+: (?:Network is unreachable|No route to host)$/i.test(http)) return "network_unreachable"; + if (/^Recv failure: Connection was reset$/i.test(http)) return "connection_reset"; + if (/^Failed to connect to .+: Connection timed out$|^Operation timed out(?: after \d+ milliseconds with \d+ bytes received)?$/i.test(http)) return "connection_timeout"; + if (/^(?:SSL certificate problem|server certificate verification failed|SSL peer certificate|gnutls_handshake\(\) failed)/i.test(http)) return "tls_failure"; + return undefined; + } + if (/^fatal: Authentication failed for ['"][^'"\r\n]+['"]$/i.test(line) || + /^fatal: could not read (?:Username|Password) for ['"][^'"\r\n]+['"]: terminal prompts disabled$/i.test(line) || + /^remote: (?:Invalid username or (?:password|token)|Permission [^\r\n]+ denied|Support for password authentication was removed|Password authentication is not supported)/i.test(line) || + /^(?:[^\s:@]+@)?[a-z0-9][a-z0-9.-]*: Permission denied \((?:publickey|password|keyboard-interactive)(?:,[a-z-]+)*\)\.?$/i.test(line)) return "authentication_failed"; + if (/^remote: Repository not found\.?$|^fatal: repository ['"][^'"\r\n]+['"] not found$/i.test(line)) return "repository_unavailable"; + if (/^ssh: Could not resolve hostname [^\s]+: (?:Name or service not known|nodename nor servname provided, or not known|Temporary failure in name resolution|No such host is known\.?)$/i.test(line)) return "dns_failure"; + const ssh = /^ssh: connect to host [^\s]+ port \d+: (.+)$/i.exec(line)?.[1]; + if (ssh === "Connection refused") return "connection_refused"; + if (ssh === "Network is unreachable" || ssh === "No route to host") return "network_unreachable"; + if (ssh === "Connection timed out") return "connection_timeout"; + if (/^Connection reset by (?:peer|[^\s]+ port \d+)$/i.test(line)) return "connection_reset"; + return undefined; +} diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index f4199bba621..fb84e328c19 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -1,3 +1,4 @@ +import { readGitConnectionFailure } from "./git-connection-failure.js"; import { WORKSPACE_VALIDATION_FAILURE_CODE, isWorkspaceValidationFailedRun, @@ -12660,7 +12661,16 @@ export function heartbeatService( const now = new Date(); const cancelled = await setRunStatusIfRunning(run.id, "cancelled", { error: error.message, errorCode: AI_CONNECTION_BUSY_RETRY_REASON, finishedAt: now, - resultJson: { executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false } }, + resultJson: { + executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false }, + cancellation: { + source: "control_plane", + expected: true, + initiator: { type: "system" }, + reason: "Waiting for shared AI credentials", + recordedAt: now.toISOString(), + }, + }, contextSnapshot: { ...parseObject(run.contextSnapshot), aiConnectionBusyDeferredWhileAssignee: wasIssueAssignee, @@ -12718,6 +12728,13 @@ export function heartbeatService( kind: "workspace_wait", providerWorkStarted: false, }, + cancellation: { + source: "control_plane", + expected: true, + initiator: { type: "system" }, + reason: "Waiting for the shared project workspace", + recordedAt: now.toISOString(), + }, workspaceBusy: { projectWorkspaceId: deferral.projectWorkspaceId, holderRunId: deferral.holder.runId, @@ -17268,7 +17285,21 @@ export function heartbeatService( if (error instanceof AiConnectionPoolExhausted && !routerHasPersistedInput) { const now = new Date(); context.aiConnectionBusyDeferredWhileAssignee = issueContext?.assigneeAgentId === agent.id; - const cancelled = await setRunStatusIfRunning(run.id, "cancelled", { error: error.message, errorCode: error.code, finishedAt: now, resultJson: { executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false }, retryAt: error.retryAt }, contextSnapshot: context }); + const cancelled = await setRunStatusIfRunning(run.id, "cancelled", { + error: error.message, errorCode: error.code, finishedAt: now, + resultJson: { + executionRecovery: { kind: "ai_connection_wait", providerWorkStarted: false }, + cancellation: { + source: "control_plane", + expected: true, + initiator: { type: "system" }, + reason: "Waiting for an AI connection pool account", + recordedAt: now.toISOString(), + }, + retryAt: error.retryAt, + }, + contextSnapshot: context, + }); if (cancelled.updated) { await setWakeupStatus(run.wakeupRequestId, "cancelled", { finishedAt: now, error: error.message }); const retry = await scheduleBoundedRetryForRun(cancelled.run ?? run, agent, { now, retryReason: AI_CONNECTION_POOL_WAIT_RETRY_REASON, wakeReason: "ai_connection_pool_retry", maxAttempts: (run.scheduledRetryAttempt ?? 0) + 1, delayMs: Math.max(1000, Date.parse(error.retryAt) - Date.now()) }); @@ -22952,8 +22983,10 @@ export function heartbeatService( sandboxProviderPluginNotReadySetupFailure, ) : null); + const connectionFailure = readGitConnectionFailure(outerErr); const setupFailureResultJson = { ...setupFailureDetails, + ...(connectionFailure ? { connectionFailure } : {}), executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, }; const setupFailureWrite = await setRunStatusIfRunning(runId, "failed", { diff --git a/server/src/services/heartbeat/workspaces.ts b/server/src/services/heartbeat/workspaces.ts index 5893ca1e7c6..69b879f3c28 100644 --- a/server/src/services/heartbeat/workspaces.ts +++ b/server/src/services/heartbeat/workspaces.ts @@ -1,3 +1,5 @@ +import { readConnectionFailure, type GitConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; +import { classifyGitCloneFailure, GitConnectionFailureError, readGitConnectionFailure } from "../git-connection-failure.js"; import { managedAiSessionFingerprintConfig } from "../ai-connection-runtime.js"; import { PROJECT_REPOSITORIES_DIR, @@ -549,22 +551,30 @@ async function materializeManagedProjectWorkspace( : null; const cloneTmpDir = await fs.mkdtemp(`${cwd}.clone-`); try { - await execFile( - "git", - [...(auth?.configArgs ?? []), "clone", "--no-hardlinks", "--", input.localSource ?? input.repoUrl, cloneTmpDir], - { - env: { - // Spread order matters: the sanitizer strips PAPERCLIP_*, which would remove the - // credential-helper token env if it came first. GIT_TERMINAL_PROMPT=0 fails a - // credential-less private clone immediately instead of hanging on a prompt until - // the clone timeout. - ...sanitizeRuntimeServiceBaseEnv(process.env), - GIT_TERMINAL_PROMPT: "0", - ...(auth?.env ?? {}), + try { + await execFile( + "git", + [...(auth?.configArgs ?? []), "clone", "--no-hardlinks", "--", input.localSource ?? input.repoUrl, cloneTmpDir], + { + env: { + // Spread order matters: the sanitizer strips PAPERCLIP_*, which would remove the + // credential-helper token env if it came first. GIT_TERMINAL_PROMPT=0 fails a + // credential-less private clone immediately instead of hanging on a prompt until + // the clone timeout. + ...sanitizeRuntimeServiceBaseEnv(process.env), + GIT_TERMINAL_PROMPT: "0", + ...(auth?.env ?? {}), + }, + timeout: MANAGED_WORKSPACE_GIT_CLONE_TIMEOUT_MS, }, - timeout: MANAGED_WORKSPACE_GIT_CLONE_TIMEOUT_MS, - }, - ); + ); + } catch (error) { + const connectionFailure = !input.localSource ? classifyGitCloneFailure(input.repoUrl, error) : null; + if (connectionFailure) throw new GitConnectionFailureError( + error instanceof Error ? error.message : "Git clone failed", connectionFailure, + ); + throw error; + } if (input.localSource) { const snapshot = await readGitWorkspaceSnapshot(input.localSource, false); if (!snapshot) throw new Error("Configured repository folder is not a Git checkout"); @@ -595,6 +605,8 @@ async function materializeManagedProjectWorkspace( // Preserve the closed failure code without copying subprocess output or // credentials into the durable run. Setup recovery needs the actual cause. if (isWorkspaceGitScanError(error)) throw new WorkspaceGitScanError(error.code, message); + const connectionFailure = readGitConnectionFailure(error); + if (connectionFailure) throw new GitConnectionFailureError(message, connectionFailure); throw new Error(message); } @@ -1060,8 +1072,10 @@ export async function assertGitWorktreeBaseWorkspaceReady(input: { reason: string, message: string, extra: Record = {}, + connectionFailure?: GitConnectionFailure, ) => { throw new WorkspaceValidationFailure(message, { + ...(connectionFailure ? { connectionFailure } : {}), workspaceValidation: { reason, issueId: input.issue!.id, @@ -1098,10 +1112,15 @@ export async function assertGitWorktreeBaseWorkspaceReady(input: { const materializationFailures = input.anchor?.materializationFailures ?? []; if (input.anchor?.baseCwdFallback && materializationFailures.length > 0) { const failureDetail = `: ${materializationFailures[0].error.replace(/\s+/g, " ")}`; + const connections = materializationFailures.map((failure) => readConnectionFailure(failure.connectionFailure)); + // Mixed connection/local failures must retain their application-error report. + const connectionFailure = connections.every((failure) => failure?.provider === "git") + ? connections[0] as GitConnectionFailure : undefined; fail( "git_worktree_base_materialization_failed", `Issue ${issueLabel} requested ${input.requestedExecutionWorkspaceMode} with git_worktree, but the project workspace checkout could not be prepared${failureDetail}. Repair the project workspace repository URL, clone access, or configured local cwd, then retry.`, { baseCwdFallback: true, materializationFailures }, + connectionFailure, ); } @@ -1410,6 +1429,7 @@ export type WorkspaceMaterializationFailure = { projectWorkspaceId: string | null; repoUrl: string | null; error: string; + connectionFailure?: GitConnectionFailure; }; export type ResolvedWorkspaceForRun = { @@ -3526,12 +3546,14 @@ export function createHeartbeatWorkspaceResolver(db: Db) { error instanceof Error ? error.message : String(error), ); const workspaceRepoUrl = readNonEmptyString(workspace.repoUrl); + const connectionFailure = readGitConnectionFailure(error); materializationFailures.push({ projectWorkspaceId: workspace.id, repoUrl: workspaceRepoUrl ? scrubGitCredentialText(workspaceRepoUrl) : null, error: scrubbedError, + ...(connectionFailure ? { connectionFailure } : {}), }); if (preferredWorkspace?.id === workspace.id) { preferredWorkspaceWarning = scrubbedError; diff --git a/server/src/services/mcp-connection-failure.ts b/server/src/services/mcp-connection-failure.ts new file mode 100644 index 00000000000..b3cbb816e0b --- /dev/null +++ b/server/src/services/mcp-connection-failure.ts @@ -0,0 +1,63 @@ +import type { ConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; +import { HttpError } from "../errors.js"; +import { classifyRemoteConnectionError, hasUnclassifiedRemoteConnectionAttempts, readRemoteConnectionFailure } from "./remote-connection-failure.js"; + +type McpConnectionFailure = Extract; +type Reason = McpConnectionFailure["reason"]; + +// These are local provenance receipts, never properties supplied by a provider, +// request body, or persisted error. They affect Sentry reporting only. +const failures = new WeakMap(); + +function remember(error: Error, reason: Reason): void { + failures.set(error, { schemaVersion: 1, provider: "mcp_http", operation: "discover_tools", reason }); +} + +/** Call only around the outbound MCP request or its response-body read. */ +export async function withMcpConnectionFailure(request: () => Promise): Promise { + try { + return await request(); + } catch (error) { + if (error instanceof Error) { + if (hasUnclassifiedRemoteConnectionAttempts(error)) failures.delete(error); + const reason = readRemoteConnectionFailure(error) ?? classifyRemoteConnectionError(error); + if (reason) remember(error, reason); + } + throw error; + } +} + +/** Preserve the existing HTTP failure; classify only the actual remote status. */ +export function mcpDiscoveryHttpFailure(response: Response, message: string): HttpError { + return markMcpHttpResponseFailure(response, new HttpError(502, message, { status: response.status })); +} + +/** Only an actual MCP response can attach this evidence to a protocol wrapper. */ +export function markMcpHttpResponseFailure(response: Response, error: T): T { + const status = response.status; + const reason: Reason | null = status === 401 || status === 403 ? "authentication_failed" + : status === 404 ? "endpoint_not_found" + : status === 429 ? "rate_limited" + : status >= 500 && status <= 599 ? "remote_unavailable" : null; + if (reason) remember(error, reason); + return error; +} + +/** Rewrapping a known error must not classify unrelated persistence failures. */ +export function retainMcpConnectionFailure(source: unknown, target: T): T { + if (hasUnclassifiedRemoteConnectionAttempts(source)) { + failures.delete(target); + return target; + } + const failure = source instanceof Error ? failures.get(source) : undefined; + if (failure) failures.set(target, failure); + else { + const reason = readRemoteConnectionFailure(source); + if (reason) remember(target, reason); + } + return target; +} + +export function isExpectedMcpConnectionFailure(error: unknown): boolean { + return error instanceof HttpError && error.status === 502 && !hasUnclassifiedRemoteConnectionAttempts(error) && failures.has(error); +} diff --git a/server/src/services/mcp-http.ts b/server/src/services/mcp-http.ts index 445d26b93ea..80db7f9c17e 100644 --- a/server/src/services/mcp-http.ts +++ b/server/src/services/mcp-http.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import { markMcpHttpResponseFailure, retainMcpConnectionFailure, withMcpConnectionFailure } from "./mcp-connection-failure.js"; // Helpers for talking to remote MCP servers over the Streamable HTTP transport. // // The MCP Streamable HTTP spec requires the client to advertise that it accepts @@ -72,18 +73,18 @@ export async function initializeMcpHttpSession(input: { }), }); if (!initializeResponse.ok) { - throw new McpHttpInitializationError( + throw markMcpHttpResponseFailure(initializeResponse, new McpHttpInitializationError( `Remote MCP initialization returned HTTP ${initializeResponse.status}`, "initialize", initializeResponse.status, initializeResponse, - ); + )); } let payload: unknown; try { - payload = await readMcpHttpResponse(initializeResponse, `${input.requestId}-initialize`); - } catch { - throw new McpHttpInitializationError("Remote MCP initialization returned an invalid response", "initialize", null); + payload = await withMcpConnectionFailure(() => readMcpHttpResponse(initializeResponse, `${input.requestId}-initialize`)); + } catch (error) { + throw retainMcpConnectionFailure(error, new McpHttpInitializationError("Remote MCP initialization returned an invalid response", "initialize", null)); } const result = payload && typeof payload === "object" && "result" in payload ? (payload as { result?: unknown }).result @@ -109,11 +110,11 @@ export async function initializeMcpHttpSession(input: { }), }); if (!initializedResponse.ok) { - throw new McpHttpInitializationError( + throw markMcpHttpResponseFailure(initializedResponse, new McpHttpInitializationError( `Remote MCP initialized notification returned HTTP ${initializedResponse.status}`, "initialized_notification", initializedResponse.status, - ); + )); } return sessionHeaders; } diff --git a/server/src/services/remote-connection-failure.ts b/server/src/services/remote-connection-failure.ts new file mode 100644 index 00000000000..25b1e0269b3 --- /dev/null +++ b/server/src/services/remote-connection-failure.ts @@ -0,0 +1,80 @@ +/** Server-local provenance of a failure at a remote connection boundary. */ +export type RemoteConnectionFailureReason = "connection_refused" | "dns_failure" | "network_unreachable" + | "connection_reset" | "connection_timeout" | "tls_failure"; +type Reason = RemoteConnectionFailureReason; +const failures = new WeakMap(); +const failedAttempts = new WeakMap(); + +function attemptFailureReason(reasons: readonly (Reason | null)[]): Reason | null { + return reasons.length > 0 && reasons.every((reason) => reason !== null) ? reasons[reasons.length - 1]! : null; +} + +/** Keep every address's classification without changing the final thrown value. */ +export function recordRemoteConnectionAttempts(error: T, causes: readonly unknown[]): T { + if (error instanceof Error) { + const reasons = causes.map((cause) => readRemoteConnectionFailure(cause) ?? classifyRemoteConnectionError(cause)); + failedAttempts.set(error, Object.freeze(reasons)); + } + return error; +} + +export function hasUnclassifiedRemoteConnectionAttempts(error: unknown): boolean { + const attempts = error instanceof Error ? failedAttempts.get(error) : undefined; + return attempts !== undefined && attemptFailureReason(attempts) === null; +} + +export function markRemoteConnectionFailure(error: T, reason: Reason): T { + failures.set(error, reason); + return error; +} + +export function readRemoteConnectionFailure(error: unknown): Reason | null { + if (!(error instanceof Error)) return null; + const attempts = failedAttempts.get(error); + // An explicit unknown attempt overrides even a known final owned deadline. + return attempts ? attemptFailureReason(attempts) : failures.get(error) ?? null; +} + +const transportReasons: Readonly> = { + ECONNREFUSED: "connection_refused", + ENOTFOUND: "dns_failure", + ENODATA: "dns_failure", + EAI_AGAIN: "dns_failure", + EHOSTUNREACH: "network_unreachable", + ENETUNREACH: "network_unreachable", + ECONNRESET: "connection_reset", + EPIPE: "connection_reset", + UND_ERR_SOCKET: "connection_reset", + ETIMEDOUT: "connection_timeout", + UND_ERR_CONNECT_TIMEOUT: "connection_timeout", + UND_ERR_HEADERS_TIMEOUT: "connection_timeout", + UND_ERR_BODY_TIMEOUT: "connection_timeout", + CERT_HAS_EXPIRED: "tls_failure", + CERT_NOT_YET_VALID: "tls_failure", + DEPTH_ZERO_SELF_SIGNED_CERT: "tls_failure", + SELF_SIGNED_CERT_IN_CHAIN: "tls_failure", + UNABLE_TO_VERIFY_LEAF_SIGNATURE: "tls_failure", + UNABLE_TO_GET_ISSUER_CERT_LOCALLY: "tls_failure", + ERR_TLS_CERT_ALTNAME_INVALID: "tls_failure", +}; + +export function classifyRemoteConnectionError(value: unknown, depth = 0): Reason | null { + if (!value || typeof value !== "object" || Array.isArray(value) || depth > 2) return null; + const attempts = value instanceof Error ? failedAttempts.get(value) : undefined; + // Do not fall back to the last error's code/cause after mixed attempts. + if (attempts) return attemptFailureReason(attempts); + const error = value as Record; + if (error.name === "AbortError") return null; + const reason = typeof error.code === "string" && Object.hasOwn(transportReasons, error.code) + ? transportReasons[error.code]! + : null; + if (error.code !== undefined && !reason) return null; + if (error.errors !== undefined) { + if (!Array.isArray(error.errors) || error.errors.length === 0 || error.errors.length > 8) return null; + const reasons = error.errors.map((child) => classifyRemoteConnectionError(child, depth + 1)); + const first = reasons[0]; + return first && reasons.every((child) => child === first) && (!reason || reason === first) ? first : null; + } + if (reason) return reason; + return error.cause !== undefined ? classifyRemoteConnectionError(error.cause, depth + 1) : null; +} diff --git a/server/src/services/remote-http-endpoint-guard.ts b/server/src/services/remote-http-endpoint-guard.ts index 0c5c0b3ce34..a120718b1ea 100644 --- a/server/src/services/remote-http-endpoint-guard.ts +++ b/server/src/services/remote-http-endpoint-guard.ts @@ -1,5 +1,6 @@ import { lookup as dnsLookup } from "node:dns/promises"; import { isIP } from "node:net"; +import { classifyRemoteConnectionError, markRemoteConnectionFailure, readRemoteConnectionFailure } from "./remote-connection-failure.js"; const DEFAULT_DNS_TIMEOUT_MS = 5_000; @@ -83,11 +84,14 @@ export async function resolveApprovedRemoteHttpAddresses( options.lookup ?? defaultLookup, options.dnsTimeoutMs ?? DEFAULT_DNS_TIMEOUT_MS, ); - } catch { - throw error("Remote MCP connection hostname could not be resolved", "remote_http_dns_failed"); + } catch (cause) { + const failure = error("Remote MCP connection hostname could not be resolved", "remote_http_dns_failed"); + const reason = readRemoteConnectionFailure(cause) ?? classifyRemoteConnectionError(cause); + if (reason === "dns_failure" || reason === "connection_timeout") markRemoteConnectionFailure(failure, reason); + throw failure; } if (results.length === 0) { - throw error("Remote MCP connection hostname did not resolve", "remote_http_dns_failed"); + throw markRemoteConnectionFailure(error("Remote MCP connection hostname did not resolve", "remote_http_dns_failed"), "dns_failure"); } if (results.some((result) => isAlwaysDeniedLinkLocalIp(result.address) @@ -123,7 +127,7 @@ async function lookupWithTimeout(hostname: string, lookup: RemoteHttpEndpointLoo lookup(hostname), new Promise((_, reject) => { timeout = setTimeout(() => { - reject(new Error(`DNS lookup timed out for ${hostname}`)); + reject(markRemoteConnectionFailure(new Error(`DNS lookup timed out for ${hostname}`), "connection_timeout")); }, timeoutMs); timeout.unref?.(); }), diff --git a/server/src/services/remote-http-fetch.ts b/server/src/services/remote-http-fetch.ts index c6efa351344..b02d8769fa3 100644 --- a/server/src/services/remote-http-fetch.ts +++ b/server/src/services/remote-http-fetch.ts @@ -4,6 +4,7 @@ import { connect as netConnect, isIP, type Socket } from "node:net"; import { Readable } from "node:stream"; import { connect as tlsConnect, type TLSSocket } from "node:tls"; import { createBrotliDecompress, createGunzip, createInflate } from "node:zlib"; +import { classifyRemoteConnectionError, markRemoteConnectionFailure, recordRemoteConnectionAttempts } from "./remote-connection-failure.js"; import { isAlwaysDeniedLinkLocalIp, @@ -104,10 +105,12 @@ export async function guardedRemoteHttpFetch( return await platformFetch(endpoint.toString(), { ...init, redirect: "manual" }); } catch (error) { if (isDnsResolutionError(error)) { - throw options.error( + const failure = options.error( "Remote MCP connection hostname could not be resolved", "remote_http_dns_failed", ); + if (classifyRemoteConnectionError(error) === "dns_failure") markRemoteConnectionFailure(failure, "dns_failure"); + throw failure; } throw error; } @@ -188,17 +191,21 @@ async function dialApprovedAddress(input: { options: GuardedRemoteHttpFetchOptions; }): Promise { let lastReason: unknown; + const failedAttempts: unknown[] = []; for (const address of input.approved) { input.signal?.throwIfAborted?.(); try { return await openVerifiedSocket({ ...input, address }); } catch (error) { - if (!(error instanceof UnreachableAddressError)) throw error; + if (!(error instanceof UnreachableAddressError)) { + throw recordRemoteConnectionAttempts(error, [...failedAttempts, error]); + } lastReason = error.reason; + failedAttempts.push(error.reason); } } - throw lastReason - ?? input.options.error("Remote MCP endpoint could not be reached", "remote_http_connect_failed"); + throw recordRemoteConnectionAttempts(lastReason + ?? input.options.error("Remote MCP endpoint could not be reached", "remote_http_connect_failed"), failedAttempts); } /** @@ -318,7 +325,7 @@ async function sendRequest(input: { const headersTimer = setTimeout(() => { // Destroying the request tears the socket down too, so a silent peer costs // neither a pending handler nor a leaked descriptor. - req.destroy(error("Remote MCP endpoint did not respond in time", "remote_http_response_timeout")); + req.destroy(markRemoteConnectionFailure(error("Remote MCP endpoint did not respond in time", "remote_http_response_timeout"), "connection_timeout")); }, responseTimeoutMs); headersTimer.unref?.(); req.on("response", () => clearTimeout(headersTimer)); @@ -330,7 +337,7 @@ async function sendRequest(input: { // Body idle deadline, mirroring undici's `bodyTimeout`: a stalled stream is // destroyed so `response.text()` rejects instead of hanging the caller. message.setTimeout(responseTimeoutMs, () => { - message.destroy(error("Remote MCP endpoint stalled mid-response", "remote_http_response_timeout")); + message.destroy(markRemoteConnectionFailure(error("Remote MCP endpoint stalled mid-response", "remote_http_response_timeout"), "connection_timeout")); }); // A finished response must not leave an armed socket timer behind, or a later // reader of the same socket inherits a deadline it never asked for. @@ -395,7 +402,7 @@ function once( ): Promise { return new Promise((resolve, reject) => { const timer = setTimeout(() => { - settle(new Error(`Timed out trying to ${input.what} the remote MCP endpoint`)); + settle(markRemoteConnectionFailure(new Error(`Timed out trying to ${input.what} the remote MCP endpoint`), "connection_timeout")); }, input.timeoutMs); timer.unref?.(); diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index 33f6fbe33e8..d44a1329280 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -11,6 +11,7 @@ import { } from "./run-failure-diagnostics.js"; import { logger } from "../middleware/logger.js"; import { isUnexpectedRunCancellation } from "./run-cancellation.js"; +import { readConnectionFailure, type ConnectionFailure } from "@paperclipai/adapter-utils/connection-failure"; type HeartbeatRun = typeof heartbeatRuns.$inferSelect; @@ -103,6 +104,49 @@ function isLocalPathWorkspaceConfigurationBlocker(run: HeartbeatRun, options: Ru recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false; } +/** Only the managed-clone producer can attach this bounded pre-dispatch evidence. */ +function isGitConnectionFailure(run: HeartbeatRun, options: RunFailureReportOptions): boolean { + const connection = readConnectionFailure(run.resultJson?.connectionFailure); + if (connection?.provider !== "git" || run.status !== "failed" || run.executionStage !== "preparing" || + options.phase !== "setup" || run.exitCode != null || run.signal != null) return false; + const recovery = asRecord(run.resultJson?.executionRecovery); + if (recovery?.kind !== "bootstrap" || recovery.providerWorkStarted !== false) return false; + const validation = asRecord(run.resultJson?.workspaceValidation); + if (run.errorCode === "setup_failed") return !validation; + if (run.errorCode !== "workspace_validation_failed" || validation?.reason !== "git_worktree_base_materialization_failed") return false; + const failures = validation.materializationFailures; + return Array.isArray(failures) && failures.length > 0 && failures.every((failure) => { + const marker = readConnectionFailure(asRecord(failure)?.connectionFailure); + return marker?.provider === "git"; + }) && readConnectionFailure(asRecord(failures[0])?.connectionFailure)?.reason === connection.reason; +} + +const hermesTransportReasons = new Set([ + "connection_refused", "dns_failure", "network_unreachable", "connection_reset", "connection_timeout", "tls_failure", +]); + +/** An adapter result remains failed; this only excludes its proven external cause. */ +function isHermesConnectionFailure(run: HeartbeatRun, options: RunFailureReportOptions, adapterType: string | undefined): boolean { + if (adapterType !== "hermes_gateway" || run.status !== "failed" || run.exitCode !== 1 || run.signal != null || options.phase === "setup") return false; + const connection = readConnectionFailure(run.resultJson?.connectionFailure); + if (connection?.provider !== "hermes_gateway") return false; + const code = run.errorCode; + if (connection.operation === "configuration") { + const codes: Record["reason"], string> = { + endpoint_missing: "hermes_gateway_api_base_url_missing", + endpoint_invalid: "hermes_gateway_api_base_url_invalid", + insecure_transport: "hermes_gateway_plain_http_remote_denied", + credentials_missing: "hermes_gateway_api_key_missing", + }; + return code === codes[connection.reason]; + } + if (hermesTransportReasons.has(connection.reason)) return code === "hermes_gateway_connect_failed" || code === "hermes_gateway_protocol_error"; + return (connection.reason === "authentication_failed" && code === "hermes_gateway_auth_failed") || + (connection.reason === "endpoint_not_found" && code === "hermes_gateway_runs_unsupported") || + (connection.reason === "rate_limited" && code === "hermes_gateway_rate_limited") || + (connection.reason === "remote_unavailable" && code === "hermes_gateway_upstream_error"); +} + /** * 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 @@ -119,6 +163,7 @@ export function reportRunFailure(db: Db, run: HeartbeatRun, options: RunFailureR if (run.status === "cancelled" && !isUnexpectedRunCancellation(run)) return Promise.resolve(); if (isMissingSecretConfigurationBlocker(run, options)) return Promise.resolve(); if (isLocalPathWorkspaceConfigurationBlocker(run, options)) return Promise.resolve(); + if (isGitConnectionFailure(run, options)) return Promise.resolve(); const runStatus = run.status; const report = captureTerminalRunFailure(db, run, runStatus, options); pendingRunFailureReports.add(report); @@ -147,6 +192,8 @@ async function captureTerminalRunFailure( .where(and(eq(agents.id, run.agentId), eq(agents.companyId, run.companyId))) .then((rows) => rows[0] ?? null); + if (isHermesConnectionFailure(run, options, agent?.adapterType)) return; + const taskId = readTaskId(run); if (!taskId) { logger.warn({ runId: run.id }, "run failure report has no task id, skipping Sentry report"); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index b01550aea82..fb473843888 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -10,6 +10,7 @@ import { composioAppAccounts, composioAppSetupResult } from "./composio-app-setu import { honchoManagedArguments } from "./honcho-connection.js"; import { defaultConnectionAgentInstructions } from "@paperclipai/shared"; import { isInsufficientConnectionScope, INSUFFICIENT_CONNECTION_SCOPE_MESSAGE } from "./connection-permission-errors.js"; +import { mcpDiscoveryHttpFailure, retainMcpConnectionFailure, withMcpConnectionFailure } from "./mcp-connection-failure.js"; import { ASANA_CONNECTOR_SCOPES, isAsanaConnectorProfileId, type AsanaConnectorProfileId } from "@paperclipai/shared"; import { BROWSER_USE_TOOLS } from "@paperclipai/shared"; import { browserUseClient, isBrowserUseConnection } from "./browser-use-client.js"; @@ -2885,6 +2886,7 @@ function healthFailureHttpStatus(failure: { if (failure.code === "oauth_challenge") return 422; if (failure.code === "oauth_refresh_missing") return 422; if (failure.code === "oauth_reauthorization_required") return 422; + if (failure.code === "oauth_insufficient_scope") return 422; if (failure.code === "slack_mcp_access_disabled") return 422; if (failure.code === "user_authorization_required") return 422; if (failure.code === "composio_broker_retired") return 422; @@ -7141,7 +7143,7 @@ export function toolAccessService( // PAP-17098 closed for the OAuth endpoints. let listRequestId = "paperclip-catalog-refresh"; let sessionHeaders = headers; - const sendRemote = (init: RequestInit) => requestRemoteHttpEndpoint(new URL(endpoint), init); + const sendRemote = (init: RequestInit) => withMcpConnectionFailure(() => requestRemoteHttpEndpoint(new URL(endpoint), init)); const sendToolsList = (requestHeaders: Record, cursor?: string) => { sessionHeaders = requestHeaders; return sendRemote({ method: "POST", headers: mcpHttpRequestHeaders(requestHeaders), @@ -7329,14 +7331,12 @@ export function toolAccessService( oauthSupported: Boolean(endpoints), }); } - throw new HttpError(502, `Remote app returned HTTP ${response.status}`, { - status: response.status, - }); + throw mcpDiscoveryHttpFailure(response, `Remote app returned HTTP ${response.status}`); } const descriptors: McpToolDescriptor[] = []; const seenCursors = new Set(); for (let page = 0; ; page += 1) { - const payload = await readMcpHttpResponse(response, listRequestId); + const payload = await withMcpConnectionFailure(() => readMcpHttpResponse(response, listRequestId)); const record = asRecord(payload); if (record.error) throw new HttpError(502, "Remote MCP tool discovery failed", { code: "mcp_catalog_error" }); const result = asRecord(record.result); @@ -7348,7 +7348,7 @@ export function toolAccessService( seenCursors.add(cursor); listRequestId = `paperclip-catalog-refresh-${page + 1}`; response = await sendToolsList(sessionHeaders, cursor); - if (!response.ok) throw new HttpError(502, "Remote MCP catalog page could not be read", { status: response.status }); + if (!response.ok) throw mcpDiscoveryHttpFailure(response, "Remote MCP catalog page could not be read"); } if (!isRailwayConnection(connection)) return descriptors; if (descriptors.some((tool) => normalizeRailwayToolName(tool.name).startsWith(RAILWAY_TOOL_PREFIX))) { @@ -7699,13 +7699,13 @@ export function toolAccessService( actor, details: { status: failure.status, transport: connection.transport }, }); - throw new HttpError(healthFailureHttpStatus(failure), failure.message, { + throw retainMcpConnectionFailure(error, new HttpError(healthFailureHttpStatus(failure), failure.message, { code: failure.code, connection: toConnection(updated), runtimeSlot, setupUrl: connectionSetupUrl(connection), reconnectUrl: connectionReconnectUrl(connection), - }); + })); } } @@ -7752,11 +7752,11 @@ export function toolAccessService( details: { status: failure.status }, actor, }); - throw new HttpError(healthFailureHttpStatus(failure), failure.message, { + throw retainMcpConnectionFailure(error, new HttpError(healthFailureHttpStatus(failure), failure.message, { code: failure.code, setupUrl: connectionSetupUrl(connection), reconnectUrl: connectionReconnectUrl(connection), - }); + })); } const existingRows = await db From 317ed367d4e7ef3221cccab2079018534b5be560 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Thu, 8 Oct 2026 10:20:32 -0700 Subject: [PATCH 02/25] Handle incomplete task policies and preserve review limits (#15593) Apply the reviewed change for Handle incomplete task policies and preserve review limits. Validation: local typecheck/build and focused regression tests, passing exact-head CI, Greptile 5/5, and independent source review. The PR records full-suite evidence and any local environment limitations. Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 10 ++ ...issue-execution-policy-persistence.test.ts | 45 ++++++++ .../__tests__/issue-execution-policy.test.ts | 18 +++ server/src/services/issue-execution-policy.ts | 3 +- .../ExecutionParticipantPicker.test.tsx | 70 ++++++++++++ .../components/ExecutionParticipantPicker.tsx | 7 ++ ui/src/components/IssueProperties.test.tsx | 70 ++++++++++-- .../issue-properties/IssueProperties.tsx | 42 ++++--- ui/src/hooks/useExecutionPolicy.test.tsx | 49 +++++++++ ui/src/hooks/useExecutionPolicy.ts | 23 ++++ ui/src/lib/issue-execution-policy.test.ts | 104 +++++++++++++++++- ui/src/lib/issue-execution-policy.ts | 26 ++++- .../stories/issue-management.stories.tsx | 15 +++ 13 files changed, 452 insertions(+), 30 deletions(-) create mode 100644 server/src/__tests__/issue-execution-policy-persistence.test.ts create mode 100644 ui/src/components/ExecutionParticipantPicker.test.tsx create mode 100644 ui/src/hooks/useExecutionPolicy.test.tsx create mode 100644 ui/src/hooks/useExecutionPolicy.ts diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 53ea15d19ac..50e1034572e 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -174,6 +174,16 @@ checkout, including a non-primary checkout. Changing projects clears the previous worktree choice. Switching from reuse to a new worktree or the project workspace restores the project default checkout. Task creation uses the selected company and has no separate heading or settings control. + +Task execution-policy controls apply the shared schema defaults when stages or +participants are omitted. An invalid policy shows an unavailable notice and +disables policy edits while leaving other task properties usable. Refresh to +retry. Optional browser error monitoring reports only a fixed field category, +once per mounted control; it never includes the policy or task identifiers. +Edits in the reviewer and monitor controls retain an explicitly configured +review-round limit when the last reviewer or monitor is removed, so adding a +reviewer later uses the saved limit. + Fresh tasks start with an empty request and the last task assignee chosen in that company, including a human. If that assignee is unavailable, the CEO is the default, or the first eligible agent when the company has no CEO. Explicit launch diff --git a/server/src/__tests__/issue-execution-policy-persistence.test.ts b/server/src/__tests__/issue-execution-policy-persistence.test.ts new file mode 100644 index 00000000000..743c248efc1 --- /dev/null +++ b/server/src/__tests__/issue-execution-policy-persistence.test.ts @@ -0,0 +1,45 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { companies, createDb, issues } from "@paperclipai/db"; +import { buildExecutionPolicy } from "../../../ui/src/lib/issue-execution-policy.ts"; +import { normalizeIssueExecutionPolicy } from "../services/issue-execution-policy.js"; +import { issueService } from "../services/issues.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("edited execution policy persistence", () => { + let temporary: Awaited>; + let db: ReturnType; + beforeAll(async () => { + temporary = await startEmbeddedPostgresTestDatabase("paperclip-policy-persistence-"); + db = createDb(temporary.connectionString); + }, 30_000); + afterAll(async () => { await temporary?.cleanup(); }); + + it.each(["reviewer", "monitor"] as const)("retains the independent review limit after removing the last %s", async (removed) => { + const companyId = randomUUID(); + const issueId = randomUUID(); + const original = normalizeIssueExecutionPolicy({ + maxReviewRounds: 4, + ...(removed === "monitor" + ? { monitor: { nextCheckAt: "2099-01-01T00:00:00.000Z" } } + : { stages: [{ type: "review", participants: [{ type: "user", userId: "board-reviewer" }] }] }), + }); + await db.insert(companies).values({ id: companyId, name: "Policy fixture", issuePrefix: `P${companyId.slice(0, 6)}` }); + await db.insert(issues).values({ id: issueId, companyId, title: "Review limit fixture", executionPolicy: original }); + const edited = buildExecutionPolicy({ existingPolicy: original, reviewerValues: [], approverValues: [] })!; + // The monitor editor removes this key after building the unchanged stages. + const { monitor: _removedMonitor, ...withoutMonitor } = edited; + const normalized = normalizeIssueExecutionPolicy(removed === "monitor" ? withoutMonitor : edited); + const service = issueService(db); + await service.update(issueId, { executionPolicy: normalized, companyGuard: companyId }); + const saved = await service.getById(issueId); + expect(saved?.executionPolicy).toEqual({ mode: "normal", commentRequired: true, stages: [], maxReviewRounds: 4 }); + + const restored = normalizeIssueExecutionPolicy(buildExecutionPolicy({ + existingPolicy: saved?.executionPolicy, + reviewerValues: ["user:board-reviewer"], approverValues: [], + })); + expect(restored).toMatchObject({ maxReviewRounds: 4, stages: [{ type: "review" }] }); + }); +}); diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 8b9d560191c..c49890ba0b9 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { applyIssueExecutionPolicyTransition, normalizeIssueExecutionPolicy, parseIssueExecutionState } from "../services/issue-execution-policy.ts"; import type { IssueExecutionPolicy, IssueExecutionState } from "@paperclipai/shared"; +import { buildExecutionPolicy } from "../../../ui/src/lib/issue-execution-policy.ts"; const coderAgentId = "11111111-1111-4111-8111-111111111111"; const qaAgentId = "22222222-2222-4222-8222-222222222222"; @@ -43,6 +44,23 @@ describe("normalizeIssueExecutionPolicy", () => { expect(normalizeIssueExecutionPolicy({ stages: [] })).toBeNull(); }); + it("retains an independent review limit through removing and restoring the last reviewer", () => { + const original = normalizeIssueExecutionPolicy({ + maxReviewRounds: 4, + stages: [{ type: "review", participants: [{ type: "agent", agentId: qaAgentId }] }], + }); + const edited = buildExecutionPolicy({ existingPolicy: original, reviewerValues: [], approverValues: [] }); + const saved = normalizeIssueExecutionPolicy(edited); + expect(saved).toEqual({ mode: "normal", commentRequired: true, stages: [], maxReviewRounds: 4 }); + expect(applyIssueExecutionPolicyTransition({ + issue: { status: "in_progress", assigneeAgentId: coderAgentId }, + policy: saved, requestedStatus: "done", requestedAssigneePatch: {}, actor: { agentId: coderAgentId }, + })).toEqual({ patch: {} }); + expect(normalizeIssueExecutionPolicy(buildExecutionPolicy({ + existingPolicy: saved, reviewerValues: [`agent:${qaAgentId}`], approverValues: [], + }))).toMatchObject({ maxReviewRounds: 4, stages: [{ type: "review" }] }); + }); + it("throws when all participants are invalid (missing agentId)", () => { expect(() => normalizeIssueExecutionPolicy({ diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 859dc5fe1e3..ed38f91fb9b 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -401,7 +401,8 @@ export function normalizeIssueExecutionPolicy(input: unknown): IssueExecutionPol const reviewPreset = parsed.data.reviewPreset; const authorizationPolicy = parsed.data.authorizationPolicy; - if (stages.length === 0 && !monitor && !reviewPreset && !authorizationPolicy) return null; + if (stages.length === 0 && !monitor && !reviewPreset && !authorizationPolicy + && parsed.data.maxReviewRounds == null) return null; return { mode: parsed.data.mode ?? "normal", diff --git a/ui/src/components/ExecutionParticipantPicker.test.tsx b/ui/src/components/ExecutionParticipantPicker.test.tsx new file mode 100644 index 00000000000..98fa99a5aab --- /dev/null +++ b/ui/src/components/ExecutionParticipantPicker.test.tsx @@ -0,0 +1,70 @@ +// @vitest-environment jsdom +import { act, type ReactNode } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { issueExecutionPolicySchema, type Issue } from "@paperclipai/shared"; +import { describe, expect, it, vi } from "vitest"; +import { ExecutionParticipantPicker } from "./ExecutionParticipantPicker"; + +vi.mock("../api/access", () => ({ accessApi: { listUserDirectory: async () => ({ users: [] }) } })); +vi.mock("../lib/sentry", () => ({ captureBrowserException: vi.fn() })); +vi.mock("@/components/ui/popover", () => ({ + Popover: ({ children }: { children: ReactNode }) =>
{children}
, + PopoverTrigger: ({ children }: { children: ReactNode }) => <>{children}, + PopoverContent: ({ children }: { children: ReactNode }) =>
{children}
, +})); + +function renderPicker(policy: unknown) { + const container = document.createElement("div"); + const root = createRoot(container); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const onUpdate = vi.fn(); + const issue = { companyId: "company-fixture", executionPolicy: policy } as Issue; + act(() => root.render( + + )); + return { container, onUpdate, cleanup: () => { act(() => root.unmount()); client.clear(); } }; +} + +describe("ExecutionParticipantPicker", () => { + it("adds a reviewer to a valid policy that omits stages without dropping authorization", () => { + const policy = { authorizationPolicy: { trustPreset: "standard" }, maxReviewRounds: 4 }; + const { container, onUpdate, cleanup } = renderPicker(policy); + expect(container.textContent).not.toContain("Execution policy unavailable"); + const assign = Array.from(container.querySelectorAll("button")).find((button) => button.textContent === "Assign to me"); + expect(assign).toBeTruthy(); + act(() => assign!.click()); + expect(onUpdate).toHaveBeenCalledTimes(1); + const updated = onUpdate.mock.calls[0][0].executionPolicy; + expect(issueExecutionPolicySchema.safeParse(updated).success).toBe(true); + expect(updated).toMatchObject({ ...policy, stages: [{ type: "review", participants: [{ type: "user", userId: "board-reviewer" }] }] }); + expect(policy).not.toHaveProperty("stages"); + cleanup(); + }); + + it("shows an existing reviewer and preserves other policy settings when that reviewer is removed", () => { + const policy = { + authorizationPolicy: { trustPreset: "standard" }, maxReviewRounds: 4, + stages: [{ type: "review", participants: [{ type: "user", userId: "board-reviewer" }] }], + }; + const { container, onUpdate, cleanup } = renderPicker(policy); + expect(container.querySelector("button")?.textContent).toBe("You"); + const remove = Array.from(container.querySelectorAll("button")).find((button) => button.textContent === "No reviewers"); + expect(remove).toBeTruthy(); + act(() => remove!.click()); + expect(onUpdate).toHaveBeenCalledWith({ executionPolicy: { + mode: "normal", commentRequired: true, stages: [], + authorizationPolicy: policy.authorizationPolicy, maxReviewRounds: 4, + } }); + expect(policy.stages[0].participants).toHaveLength(1); + cleanup(); + }); + + it("keeps a malformed policy unavailable and exposes no mutation controls", () => { + const { container, onUpdate, cleanup } = renderPicker({ stages: [{ type: "review", participants: {} }] }); + expect(container.textContent).toBe("Execution policy unavailable. Refresh to try again."); + expect(container.querySelector("button")).toBeNull(); + expect(onUpdate).not.toHaveBeenCalled(); + cleanup(); + }); +}); diff --git a/ui/src/components/ExecutionParticipantPicker.tsx b/ui/src/components/ExecutionParticipantPicker.tsx index b13d60748f7..b5aba28216a 100644 --- a/ui/src/components/ExecutionParticipantPicker.tsx +++ b/ui/src/components/ExecutionParticipantPicker.tsx @@ -11,6 +11,7 @@ import { buildExecutionPolicy, stageParticipantValues, } from "../lib/issue-execution-policy"; +import { useExecutionPolicy } from "../hooks/useExecutionPolicy"; import { cn } from "../lib/utils"; import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { User, Eye, ShieldCheck } from "lucide-react"; @@ -34,6 +35,7 @@ export function ExecutionParticipantPicker({ }: ExecutionParticipantPickerProps) { const [open, setOpen] = useState(false); const [search, setSearch] = useState(""); + const policyAvailable = useExecutionPolicy(issue.executionPolicy).success; const reviewerValues = stageParticipantValues(issue.executionPolicy, "review"); const approverValues = stageParticipantValues(issue.executionPolicy, "approval"); @@ -73,6 +75,7 @@ export function ExecutionParticipantPicker({ }; const updatePolicy = (nextValues: string[]) => { + if (!policyAvailable) return; onUpdate({ executionPolicy: buildExecutionPolicy({ existingPolicy: issue.executionPolicy ?? null, @@ -92,6 +95,10 @@ export function ExecutionParticipantPicker({ const label = stageType === "review" ? "Reviewers" : "Approvers"; const Icon = stageType === "review" ? Eye : ShieldCheck; + if (!policyAvailable) { + return Execution policy unavailable. Refresh to try again.; + } + return ( { setOpen(o); if (!o) setSearch(""); }}> diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index 683acb658ec..dd19e8712b6 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -2735,10 +2735,10 @@ describe("IssueProperties", () => { executionPolicy: createExecutionPolicy({ stages: [ { - id: "review-stage", + id: "00000000-0000-4000-8000-000000000010", type: "review", approvalsNeeded: 1, - participants: [{ id: "participant-1", type: "agent", agentId: "agent-1", userId: null }], + participants: [{ id: "00000000-0000-4000-8000-000000000012", type: "agent", agentId: "00000000-0000-4000-8000-000000000013", userId: null }], }, ], }), @@ -2767,10 +2767,10 @@ describe("IssueProperties", () => { executionPolicy: createExecutionPolicy({ stages: [ { - id: "approval-stage", + id: "00000000-0000-4000-8000-000000000011", type: "approval", approvalsNeeded: 1, - participants: [{ id: "participant-2", type: "user", agentId: null, userId: "user-1" }], + participants: [{ id: "00000000-0000-4000-8000-000000000014", type: "user", agentId: null, userId: "user-1" }], }, ], }), @@ -2793,10 +2793,10 @@ describe("IssueProperties", () => { executionPolicy: createExecutionPolicy({ stages: [ { - id: "review-stage", + id: "00000000-0000-4000-8000-000000000010", type: "review", approvalsNeeded: 1, - participants: [{ id: "participant-1", type: "agent", agentId: "agent-1", userId: null }], + participants: [{ id: "00000000-0000-4000-8000-000000000012", type: "agent", agentId: "00000000-0000-4000-8000-000000000013", userId: null }], }, ], }), @@ -2819,10 +2819,10 @@ describe("IssueProperties", () => { executionPolicy: createExecutionPolicy({ stages: [ { - id: "review-stage", + id: "00000000-0000-4000-8000-000000000010", type: "review", approvalsNeeded: 1, - participants: [{ id: "participant-1", type: "agent", agentId: "agent-1", userId: null }], + participants: [{ id: "00000000-0000-4000-8000-000000000012", type: "agent", agentId: "00000000-0000-4000-8000-000000000013", userId: null }], }, ], }), @@ -2924,6 +2924,60 @@ describe("IssueProperties", () => { dateNowSpy.mockRestore(); }); + it.each([true, false])("clears a monitor with omitted stages while preserving independent limits (authorization: %s)", async (withAuthorization) => { + const onUpdate = vi.fn(); + const executionPolicy = { + ...(withAuthorization ? { authorizationPolicy: { trustPreset: "standard" } } : {}), + maxReviewRounds: 4, + monitor: { nextCheckAt: "2099-01-01T12:00:00.000Z", notes: "Check service", scheduledBy: "board" }, + } as IssueExecutionPolicy; + const before = structuredClone(executionPolicy); + const root = renderProperties(container, { + issue: createIssue({ executionPolicy }), childIssues: [], onUpdate, inline: true, + }); + await flush(); + + expect(container.textContent).toContain("Reviewers"); + expect(container.textContent).not.toContain("Execution policy unavailable"); + const monitorTrigger = container.querySelector('[data-testid="monitor-row-trigger"]')?.closest("button"); + expect(monitorTrigger).toBeTruthy(); + await act(async () => { monitorTrigger!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); + const clearButton = Array.from(container.querySelectorAll("button")).find((button) => button.textContent === "Clear"); + expect(clearButton).toBeTruthy(); + await act(async () => { clearButton!.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); + + expect(onUpdate).toHaveBeenCalledWith({ executionPolicy: { + mode: "normal", commentRequired: true, stages: [], + ...(withAuthorization ? { authorizationPolicy: before.authorizationPolicy } : {}), maxReviewRounds: 4, + } }); + expect(executionPolicy).toEqual(before); + act(() => root.unmount()); + }); + + it.each([ + { stages: {} }, + { stages: [null] }, + { stages: [{ type: "review", participants: {} }] }, + { monitor: { nextCheckAt: {}, notes: {} } }, + ])("keeps malformed policies unavailable without exposing destructive edits: %j", async (executionPolicy) => { + const onUpdate = vi.fn(); + const root = renderProperties(container, { + issue: createIssue({ executionPolicy: executionPolicy as unknown as IssueExecutionPolicy }), + childIssues: [], onUpdate, inline: true, + }); + await flush(); + + expect(container.textContent).toContain("Execution policy unavailable. Refresh to try again."); + expect(container.textContent).toContain("Assignee"); + expect(container.textContent).not.toContain("No reviewers"); + expect(container.textContent).not.toContain("No approvers"); + expect(container.textContent).not.toContain("Run review now"); + expect(container.querySelector('[data-testid="monitor-row-trigger"]')).toBeNull(); + expect(container.querySelector('input[type="datetime-local"]')).toBeNull(); + expect(onUpdate).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + it("renders scheduled, retrying, due, overdue, cleared, and empty monitor row states", async () => { // Anchored to a fixed *local* time, not a fixed UTC instant. The row renders // these in the machine's timezone and labels them "Today" only while they diff --git a/ui/src/components/issue-properties/IssueProperties.tsx b/ui/src/components/issue-properties/IssueProperties.tsx index db6de10d240..2b00c76abae 100644 --- a/ui/src/components/issue-properties/IssueProperties.tsx +++ b/ui/src/components/issue-properties/IssueProperties.tsx @@ -47,6 +47,7 @@ import { getRecentProjectIds } from "../../lib/recent-projects"; import { orderItemsBySelectedAndRecent } from "../../lib/recent-selections"; import { formatAssigneeUserLabel, formatUserLabel } from "../../lib/assignees"; import { buildExecutionPolicy, stageParticipantValues } from "../../lib/issue-execution-policy"; +import { useExecutionPolicy } from "../../hooks/useExecutionPolicy"; import { formatMonitorAbsolute, formatMonitorAbsoluteFull, @@ -246,6 +247,9 @@ export function IssueProperties({ documentDeepLink, sidePanelContentOnly = false, }: IssuePropertiesProps) { + const policyResult = useExecutionPolicy(issue.executionPolicy); + const policyAvailable = policyResult.success; + const executionPolicy = policyResult.success ? policyResult.data : null; const { selectedCompanyId } = useCompany(); const { isMobile } = useSidebar(); const queryClient = useQueryClient(); @@ -392,9 +396,9 @@ export function IssueProperties({ const [newLabelName, setNewLabelName] = useState(""); // token-extraction: allowlisted — color-picker seed state, persisted into label-create payload; a var() string would break that payload. const [newLabelColor, setNewLabelColor] = useState("#6366f1"); - const [monitorAtInput, setMonitorAtInput] = useState(() => toDateTimeLocalValue(issue.executionPolicy?.monitor?.nextCheckAt)); - const [monitorNotesInput, setMonitorNotesInput] = useState(issue.executionPolicy?.monitor?.notes ?? ""); - const [monitorServiceInput, setMonitorServiceInput] = useState(issue.executionPolicy?.monitor?.serviceName ?? ""); + const [monitorAtInput, setMonitorAtInput] = useState(() => toDateTimeLocalValue(executionPolicy?.monitor?.nextCheckAt)); + const [monitorNotesInput, setMonitorNotesInput] = useState(executionPolicy?.monitor?.notes ?? ""); + const [monitorServiceInput, setMonitorServiceInput] = useState(executionPolicy?.monitor?.serviceName ?? ""); const [runtimeActionMessage, setRuntimeActionMessage] = useState(null); const [runtimeActionErrorMessage, setRuntimeActionErrorMessage] = useState(null); const [unarchiveErrorMessage, setUnarchiveErrorMessage] = useState(null); @@ -999,6 +1003,7 @@ export function IssueProperties({ applyAssignee(next, track); }; const updateExecutionPolicy = (nextReviewers: string[], nextApprovers: string[]) => { + if (!policyAvailable) return; onUpdate({ executionPolicy: buildExecutionPolicy({ existingPolicy: issue.executionPolicy ?? null, @@ -1073,13 +1078,13 @@ export function IssueProperties({ return `${stageLabel} pending${participantLabel ? ` with ${participantLabel}` : ""}`; })(); useEffect(() => { - setMonitorAtInput(toDateTimeLocalValue(issue.executionPolicy?.monitor?.nextCheckAt)); - setMonitorNotesInput(issue.executionPolicy?.monitor?.notes ?? ""); - setMonitorServiceInput(issue.executionPolicy?.monitor?.serviceName ?? ""); + setMonitorAtInput(toDateTimeLocalValue(executionPolicy?.monitor?.nextCheckAt)); + setMonitorNotesInput(executionPolicy?.monitor?.notes ?? ""); + setMonitorServiceInput(executionPolicy?.monitor?.serviceName ?? ""); }, [ - issue.executionPolicy?.monitor?.nextCheckAt, - issue.executionPolicy?.monitor?.notes, - issue.executionPolicy?.monitor?.serviceName, + executionPolicy?.monitor?.nextCheckAt, + executionPolicy?.monitor?.notes, + executionPolicy?.monitor?.serviceName, ]); // Re-sync watchdog editor inputs when the persisted watchdog changes (and reset on close). useEffect(() => { @@ -1261,6 +1266,7 @@ export function IssueProperties({ ? M | null : never : never) => { + if (!policyAvailable) return; const basePolicy = buildExecutionPolicy({ existingPolicy: issue.executionPolicy ?? null, reviewerValues, @@ -1270,8 +1276,10 @@ export function IssueProperties({ onUpdate({ executionPolicy: null }); return; } + const { monitor: _previousMonitor, ...policyWithoutMonitor } = basePolicy ?? {}; onUpdate({ executionPolicy: { + ...policyWithoutMonitor, mode: basePolicy?.mode ?? issue.executionPolicy?.mode ?? "normal", commentRequired: true, stages: basePolicy?.stages ?? [], @@ -1299,11 +1307,11 @@ export function IssueProperties({ setMonitorOpen(false); }; const monitorState = issue.executionState?.monitor ?? null; - const monitorNextCheckAt = monitorState?.nextCheckAt ?? issue.monitorNextCheckAt ?? issue.executionPolicy?.monitor?.nextCheckAt ?? null; + const monitorNextCheckAt = monitorState?.nextCheckAt ?? issue.monitorNextCheckAt ?? executionPolicy?.monitor?.nextCheckAt ?? null; const monitorAttemptCount = issue.monitorAttemptCount ?? monitorState?.attemptCount ?? 0; const monitorLastTriggeredAt = issue.monitorLastTriggeredAt ?? monitorState?.lastTriggeredAt ?? null; - const monitorServiceName = issue.executionPolicy?.monitor?.serviceName ?? monitorState?.serviceName ?? null; - const monitorNotes = issue.executionPolicy?.monitor?.notes ?? monitorState?.notes ?? null; + const monitorServiceName = executionPolicy?.monitor?.serviceName ?? monitorState?.serviceName ?? null; + const monitorNotes = executionPolicy?.monitor?.notes ?? monitorState?.notes ?? null; const monitorNow = useMonitorCountdown(monitorNextCheckAt); const monitorRelative = monitorNextCheckAt ? formatMonitorEta(monitorNextCheckAt, monitorNow) : null; const monitorIsDueNow = monitorRelative === "due now"; @@ -1585,7 +1593,7 @@ export function IssueProperties({ > Schedule - {issue.executionPolicy?.monitor ? ( + {executionPolicy?.monitor ? (
-
+
{status === "failed" ? (