From 0ed760ce9a9fd1babd74a29f68c2352ccebc340a Mon Sep 17 00:00:00 2001 From: Naeem Akhtar <32741588+naeemakhtar110@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:57:18 +0300 Subject: [PATCH] fix(server): keep local authorization failures from disabling chat endpoints (#16138) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat endpoints publish agent replies to providers such as GitHub, and a durable outbox classifies each send failure > - A GitHub review run that fails posts a "review failed" reply. When the run has no request binding, Paperclip throws its own `forbidden()` before any GitHub request > - The classifier reads that local 403 as a provider credential failure. The endpoint goes to `attention`, its tool connection is disabled, and new `pull_request` webhooks are dropped > - One failed review can stop all reviews on that endpoint until an operator reconnects it > - This pull request classifies Paperclip's own 401/403 `HttpError` without a provider status as a failure of that publication only > - The benefit is that only real provider credential failures lock an endpoint ## Linked Issues or Issue Description Fixes: #16137 ## What Changed - `server/src/services/chat-publication-errors.ts`: an `HttpError` (Paperclip's own class) with status 401 or 403 and no numeric `details.providerStatus` returns `{ kind: "failed" }`. The check runs before the credential branch. - Provider errors do not change. Adapter `AuthenticationError`/`PermissionError`, Octokit errors (named `HttpError`, but not this class) and Paperclip-wrapped provider rejections that carry `providerStatus` still return `endpoint_attention`. - `server/src/services/chat-publication-errors.test.ts`: a new test covers a local `forbidden()`, a local `unauthorized()` and a wrapped provider 403. ## Verification - `pnpm --filter @paperclipai/server vitest run src/services/chat-publication-errors.test.ts` - I ran the classifier source with type stripping on Node 22: local `forbidden("GitHub failure has no request binding")` → `failed`; local `unauthorized()` → `failed`; `new HttpError(403, …, { providerStatus: 403 })` → `endpoint_attention`; an Octokit-shaped 403 → `endpoint_attention`. - I applied the same change to the built `dist` of a self-hosted install and called its classifier with the same four errors. The results were the same. ## Risks - Low risk. The change applies only to Paperclip's own `HttpError` with status 401/403 and no provider status. Such an error now fails one publication and does not disable the endpoint. Code that throws a local 401/403 to report a provider credential problem must attach `providerStatus`. I found no such caller in `server/src/services`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), in Claude Code with tool use (shell, file edits, test runs). It helped to find the cause, write the change and the test, and run the checks. ## 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 - [ ] I have run tests locally and they pass (classifier checked with type stripping; the full vitest suite runs in CI) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] 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 --- .../services/chat-publication-errors.test.ts | 22 +++++++++++++++++++ .../src/services/chat-publication-errors.ts | 11 ++++++++++ 2 files changed, 33 insertions(+) diff --git a/server/src/services/chat-publication-errors.test.ts b/server/src/services/chat-publication-errors.test.ts index 0635f69c9fc..5bb72024f3f 100644 --- a/server/src/services/chat-publication-errors.test.ts +++ b/server/src/services/chat-publication-errors.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { forbidden, HttpError, unauthorized } from "../errors.js"; import { classifyChatPublicationError } from "./chat-publication-errors.js"; function providerError( @@ -155,6 +156,27 @@ describe("chat publication error classification", () => { ).toMatchObject({ kind: "endpoint_attention" }); }); + it("keeps Paperclip's own authorization failures scoped to the publication", () => { + expect( + classifyChatPublicationError( + forbidden("GitHub failure has no request binding"), + 1, + ), + ).toMatchObject({ kind: "failed" }); + expect(classifyChatPublicationError(unauthorized(), 1)).toMatchObject({ + kind: "failed", + }); + expect( + classifyChatPublicationError( + new HttpError(403, "GitHub rejected this operation", { + code: "github_bot_operation_failed", + providerStatus: 403, + }), + 1, + ), + ).toMatchObject({ kind: "endpoint_attention" }); + }); + it("does not call explicit GitHub 4xx rejections ambiguous delivery", () => { for (const status of [400, 409, 422]) { expect( diff --git a/server/src/services/chat-publication-errors.ts b/server/src/services/chat-publication-errors.ts index 3d020bbc49c..c9666264063 100644 --- a/server/src/services/chat-publication-errors.ts +++ b/server/src/services/chat-publication-errors.ts @@ -1,3 +1,5 @@ +import { HttpError } from "../errors.js"; + export type ChatPublicationErrorDisposition = | { kind: "retry"; @@ -317,6 +319,15 @@ export function classifyChatPublicationError( return { kind: "resource_unavailable", reason }; } + if ( + error instanceof HttpError && + (error.status === 401 || error.status === 403) && + typeof (error.details as { providerStatus?: unknown } | undefined) + ?.providerStatus !== "number" + ) { + return { kind: "failed", reason }; + } + if ( names.includes("AuthenticationError") || names.includes("PermissionError") ||