fix(errors): return tool gateway rejections with their own status (#19158)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents call connected tools through the tool gateway. Some tool
calls need a human review before they run.
> - A user approves a tool review from a task card. The approve route
calls the gateway.
> - The gateway can reject the approval with `ToolGatewayHttpError(409,
"approved_tool_target_changed")`. This is an expected rejection: the
tool or connection changed after the review.
> - `ToolGatewayHttpError` does not extend `HttpError`. The shared error
handler only keeps the status of an `HttpError`. Thus the route answers
500 "Internal server error" and sends the rejection to Sentry as a
crash.
> - The user does not see the reason. The user clicks Approve again, and
each click makes one more crash report. One user made 69 reports on
staging this way.
> - This pull request makes the shared error handler keep the status and
reason code of client (4xx) gateway errors.
> - The benefit is a clear 409 with a reason for the user, and no false
crash reports.

## Linked Issues or Issue Description

Refs #12775 (related community PR. It changes `ToolGatewayHttpError` to
extend `HttpError`, which affects every `instanceof HttpError` check.
This PR is smaller and keeps gateway server failures on the crash path.)

**What happened?**
A user approves a tool review on a task card. The tool definition or
connection changed after the review. The API answers 500 "Internal
server error". Sentry records the expected rejection "Tool definition or
connection changed; request a new review" as a server crash. Each retry
records one more event.

**Expected behavior**
The API answers 409 with `code: "approved_tool_target_changed"` and the
message "Tool definition or connection changed; request a new review".
Sentry does not record a crash.

**Steps to reproduce**
1. Make a connected tool that needs approval, and start a tool call so
that a review card appears.
2. Change the tool connection (for example, its config) before you
approve.
3. Click Approve on the task card (`POST
/api/issues/:id/interactions/:interactionId/accept`).
4. See 500 "Internal server error" and a new Sentry event from
`approveActionRequest`.

**Paperclip version or commit**
`master` at `482ec0aea9ab6f2684baaa1487f2c7a2eedf009e`.

## What Changed

- `server/src/middleware/error-handler.ts`: the handler converts a
`ToolGatewayHttpError` with a 4xx status to an `HttpError`. The new
error keeps the status, the message, the gateway details, and the reason
code as `code`. The original stack stays.
- Gateway errors with a 5xx status do not change. They still go on the
crash path, answer 500 "Internal server error", and go to Sentry. Thus
provider text in those messages stays hidden.
- `server/src/__tests__/error-handler-tool-gateway.test.ts`: new tests
for the 409 mapping, for the unchanged 5xx path, and for an Express
route that lets the gateway error escape.

## Verification

- `cd server && npx vitest run
src/__tests__/error-handler-tool-gateway.test.ts
src/__tests__/error-handler.test.ts` passes: 21 tests.
- Without the fix, the new tests fail with `expected 500 to be 409`.
With the fix, they pass.
- `src/__tests__/issue-thread-interaction-routes.test.ts` and
`src/__tests__/json-parse-error-handler.test.ts` also pass: 175 tests in
4 suites with the two error handler suites.
- I checked the runtime import graph of `services/tool-gateway.ts` (685
modules). It does not import `middleware/error-handler.ts`, so the new
import makes no cycle.
- I did not run the full server typecheck locally, because it ran out of
memory in an 8 GB sandbox. CI must do this check.

## Risks

- Low risk. Only gateway errors with a 4xx status change. Before, they
became 500. Now they keep their own status.
- Some routes outside the gateway router can now return gateway 4xx
details in the `details` field. The gateway routes already return the
same details through `sendGatewayError`.
- `middleware/error-handler.ts` now imports `services/tool-gateway.ts`.
Unit tests that import the error handler load more modules. The app
already loads this module.
- This PR does not fix the cause of some of these rejections. A routine
OAuth token refresh changes values in the signed approval snapshot
(token secret versions, the order of grant secret refs, and token expiry
times). This can make a pending review fail. That fix changes
`services/tool-gateway.ts` and comes in a separate PR.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used as a coding agent
with tool use (shell, file edits, test runs, and GitHub API).

## Checklist

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

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

server/src/__tests__/error-handler-tool-gateway.test.ts

@@ -0,0 +1,91 @@
+import express from "express";
+import type { Request, Response } from "express";
+import request from "supertest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { errorHandler } from "../middleware/error-handler.js";
+import { ToolGatewayHttpError } from "../services/tool-gateway.js";
+
+const captureExceptionMock = vi.hoisted(() => vi.fn());
+const trackErrorHandlerCrashMock = vi.hoisted(() => vi.fn());
+
+vi.mock("../services/responsible-user-denial-run-outcomes.js", () => ({
+  recordResponsibleUserDenialOnActiveRun: vi.fn().mockResolvedValue(null),
+}));
+vi.mock("../sentry.js", () => ({ captureException: captureExceptionMock }));
+vi.mock("../telemetry.js", () => ({ getTelemetryClient: () => ({}) }));
+vi.mock("@paperclipai/shared/telemetry", () => ({
+  trackErrorHandlerCrash: trackErrorHandlerCrashMock,
+}));
+
+function makeReq(): Request {
+  return {
+    method: "POST",
+    originalUrl: "/api/issues/issue-1/interactions/interaction-1/accept",
+    body: {},
+    params: {},
+    query: {},
+  } as unknown as Request;
+}
+
+function makeRes(): Response {
+  const res = { status: vi.fn(), json: vi.fn() } as unknown as Response;
+  (res.status as unknown as ReturnType<typeof vi.fn>).mockReturnValue(res);
+  return res;
+}
+
+function changedTargetError() {
+  return new ToolGatewayHttpError(
+    409,
+    "Tool definition or connection changed; request a new review",
+    "approved_tool_target_changed",
+  );
+}
+
+describe("errorHandler with tool gateway errors", () => {
+  beforeEach(() => {
+    captureExceptionMock.mockReset();
+    trackErrorHandlerCrashMock.mockReset();
+  });
+
+  it("answers an expected gateway rejection with its own status and reason, without a crash report", () => {
+    const res = makeRes();
+
+    errorHandler(changedTargetError(), makeReq(), res, vi.fn());
+
+    expect(res.status).toHaveBeenCalledWith(409);
+    expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+      error: "Tool definition or connection changed; request a new review",
+      code: "approved_tool_target_changed",
+    }));
+    expect(captureExceptionMock).not.toHaveBeenCalled();
+    expect(trackErrorHandlerCrashMock).not.toHaveBeenCalled();
+  });
+
+  it("keeps gateway server failures on the crash path without exposing provider text", () => {
+    const error = new ToolGatewayHttpError(502, "provider said: secret detail", "mcp_remote_invalid_json");
+    const res = makeRes();
+
+    errorHandler(error, makeReq(), res, vi.fn());
+
+    expect(res.status).toHaveBeenCalledWith(500);
+    expect(res.json).toHaveBeenCalledWith({ error: "Internal server error" });
+    expect(captureExceptionMock).toHaveBeenCalledWith(error);
+  });
+
+  it("returns the 409 to a client whose route lets the gateway error escape", async () => {
+    const app = express();
+    app.post("/api/issues/:id/interactions/:interactionId/accept", async () => {
+      throw changedTargetError();
+    });
+    app.use(errorHandler);
+
+    const res = await request(app).post("/api/issues/issue-1/interactions/interaction-1/accept").send({});
+
+    expect(res.status).toBe(409);
+    expect(res.body).toMatchObject({
+      error: "Tool definition or connection changed; request a new review",
+      code: "approved_tool_target_changed",
+    });
+    expect(captureExceptionMock).not.toHaveBeenCalled();
+  });
+});

server/src/middleware/error-handler.ts

@@ -16,6 +16,7 @@ import {
 import { recordResponsibleUserDenialOnActiveRun } from "../services/responsible-user-denial-run-outcomes.js";
 import { isExpectedMcpConnectionFailure } from "../services/mcp-connection-failure.js";
 import { isEndedDbPoolError } from "../database-retry.js";
+import { ToolGatewayHttpError } from "../services/tool-gateway.js";
 
 export interface ErrorContext {
   error: {
@@ -135,12 +136,27 @@ function recordResponsibleUserDenialFromHttpError(
   });
 }
 
+/**
+ * The tool gateway rejects requests with its own error type. A route that lets
+ * one reach this handler (such as approving a tool review from a task card)
+ * would otherwise turn an expected rejection, for example a 409 asking for a
+ * new review, into a 500 crash report. Gateway server failures stay on the
+ * crash path so their provider text is not exposed.
+ */
+function clientToolGatewayErrorAsHttpError(err: unknown): unknown {
+  if (!(err instanceof ToolGatewayHttpError) || err.status < 400 || err.status >= 500) return err;
+  const mapped = new HttpError(err.status, err.message, { ...err.details, code: err.reasonCode });
+  mapped.stack = err.stack;
+  return mapped;
+}
+
 export function errorHandler(
-  err: unknown,
+  thrown: unknown,
   req: Request,
   res: Response,
   _next: NextFunction,
 ) {
+  const err = clientToolGatewayErrorAsHttpError(thrown);
   if (err instanceof AgentLifecycleConflict) { res.status(409).json({ error: err.message }); return; }
   if (err instanceof HttpError) {
     const details =