Skip to content

fix(client): classify 401 as terminal auth failure, never retryable (RBR-1036) - #9

Merged
PraeSynBH merged 1 commit into
masterfrom
rbr-1036-401-fail-fast
Aug 9, 2026
Merged

fix(client): classify 401 as terminal auth failure, never retryable (RBR-1036)#9
PraeSynBH merged 1 commit into
masterfrom
rbr-1036-401-fail-fast

Conversation

@PraeSynBH

Copy link
Copy Markdown
Owner

Summary

Split off from RBR-1015. The API client's own retry/timeout handling must treat 401 as terminal, not retryable.

  • cli/src/client/http.ts: new ApiAuthError (extends ApiRequestError, fixed status: 401, distinct name/instanceof identity) thrown immediately on any 401 response, before the generic error path. The existing single-shot interactive recoverAuth board-login exchange is unaffected — it still gets exactly one bounded recovery attempt.
  • packages/mcp-server/src/client.ts: matching PaperclipApiAuthError extends PaperclipApiError.
  • packages/mcp-server/src/format.ts: tags the MCP tool error payload with errorClass: "auth_failed" so a future incident doesn't require decoding the JWT by hand.
  • skills/paperclip/SKILL.md: documents "never retry a 401" next to the existing "never retry a 409" rule.
  • Regression tests in both packages simulate a 401 mid a bounded retry loop and assert the wrapper stops after one attempt with a distinguishable error class, not the generic timeout/5xx classification.

Acceptance criteria

  1. ✅ 401 classified as a distinct, unmistakable auth-failure error class (ApiAuthError / PaperclipApiAuthError), separate from timeout/5xx/network-error paths.
  2. ✅ Retrying a 401 is a no-op: the client throws the distinct class before any retry/backoff wrapper sees a generic/timeout-shaped error, so callers can special-case instanceof ApiAuthError and fail fast.
  3. ✅ The distinct error is visible via error class name (ApiAuthError/PaperclipApiAuthError) and errorClass: "auth_failed" in the MCP tool error response.
  4. ✅ Regression tests: cli/src/__tests__/http.test.ts and packages/mcp-server/src/client.test.ts simulate a 401 mid-retry-loop and assert one-attempt fail-fast with a distinguishable error.

Verification

  • cli/src/__tests__/http.test.ts — 8/8 passing (via ./node_modules/.bin/vitest run cli/src/__tests__/http.test.ts).
  • packages/mcp-server/src/client.test.ts (new) — 3/3 passing.
  • packages/mcp-server/src/tools.test.ts — 12/13 passing; the 1 failure (allowDuplicate field mismatch) is pre-existing on this branch before this change (confirmed via git stash), unrelated to this fix.
  • tsc --noEmit in packages/mcp-server — clean.
  • tsc --noEmit in cli — pre-existing unrelated noise only (@paperclipai/plugin-sdk module resolution in server/src/services/*, not built in this sandbox); no errors in client/http.ts or its test.
  • Full cli vitest project run — 9 pre-existing failures (embedded-postgres timeouts, a board-profile-persona test needing different env config), confirmed identical on git stash (unmodified branch); none touch client/http.ts.

Coordination

  • Companion to RBR-1020 AC3 (credential_expired terminal reason server-side).
  • Per issue guidance, verification here is fast unit/vitest-level (no live server needed) — did not attempt to spin up a full server against RBR-974 host-oversubscription conditions.

PraeSynBH pushed a commit that referenced this pull request Aug 7, 2026
…RBR-1036)

Rebase of eefe907 onto latest master to resolve the PR #9 merge
conflict (unrelated UI files diverged after the fix branch was cut).
Content is byte-identical to the original commit for the 6 files this
fix touches; no unrelated UI changes carried forward.

- cli/src/client/http.ts: ApiAuthError extends ApiRequestError with a
  fixed status of 401 and a distinct name/instanceof identity, so
  callers writing retry/backoff wrappers can special-case it and fail
  fast on the first occurrence instead of folding it into the
  timeout/5xx/network-error retry path. The existing single-shot
  interactive recoverAuth board-login exchange (CLI-only, human in the
  loop) is unaffected.
- packages/mcp-server/src/client.ts: PaperclipApiAuthError extends
  PaperclipApiError the same way, and format.ts tags its MCP tool
  error response with errorClass: "auth_failed" so a future incident
  doesn't require decoding the JWT by hand.
- Regression tests (both packages) simulate a 401 response mid a
  bounded retry loop and assert the client/wrapper stops after one
  attempt with a distinguishable error, not the generic timeout/5xx
  classification.
- skills/paperclip/SKILL.md: documents the never-retry-a-401 rule next
  to the existing never-retry-a-409 rule.

AC1: 401 classified as a distinct auth-failure error class immediately.
AC2: retrying a 401 is a no-op.
AC3: distinct error class name + errorClass: "auth_failed" in the MCP
     error payload surfaces the real cause without JWT decoding.
AC4: regression tests for both clients.
@PraeSynBH
PraeSynBH force-pushed the rbr-1036-401-fail-fast branch from eefe907 to 398028a Compare August 7, 2026 00:44
PraeSynBH pushed a commit that referenced this pull request Aug 7, 2026
…RBR-1036)

Rebase of eefe907 onto latest master to resolve the PR #9 merge
conflict (unrelated UI files diverged after the fix branch was cut).
Content is byte-identical to the original commit for the 6 files this
fix touches; no unrelated UI changes carried forward.

- cli/src/client/http.ts: ApiAuthError extends ApiRequestError with a
  fixed status of 401 and a distinct name/instanceof identity, so
  callers writing retry/backoff wrappers can special-case it and fail
  fast on the first occurrence instead of folding it into the
  timeout/5xx/network-error retry path. The existing single-shot
  interactive recoverAuth board-login exchange (CLI-only, human in the
  loop) is unaffected.
- packages/mcp-server/src/client.ts: PaperclipApiAuthError extends
  PaperclipApiError the same way, and format.ts tags its MCP tool
  error response with errorClass: "auth_failed" so a future incident
  doesn't require decoding the JWT by hand.
- Regression tests (both packages) simulate a 401 response mid a
  bounded retry loop and assert the client/wrapper stops after one
  attempt with a distinguishable error, not the generic timeout/5xx
  classification.
- skills/paperclip/SKILL.md: documents the never-retry-a-401 rule next
  to the existing never-retry-a-409 rule.

AC1: 401 classified as a distinct auth-failure error class immediately.
AC2: retrying a 401 is a no-op.
AC3: distinct error class name + errorClass: "auth_failed" in the MCP
     error payload surfaces the real cause without JWT decoding.
AC4: regression tests for both clients.
@PraeSynBH
PraeSynBH force-pushed the rbr-1036-401-fail-fast branch from 398028a to 73da26c Compare August 7, 2026 00:46
…RBR-1036)

Both first-party API clients now throw a distinct, unmistakable error
class for a 401 response instead of the generic request-failure error:

- cli/src/client/http.ts: ApiAuthError extends ApiRequestError with a
  fixed status of 401 and a distinct name/instanceof identity, so
  callers writing retry/backoff wrappers can special-case it and fail
  fast on the first occurrence instead of folding it into the
  timeout/5xx/network-error retry path. The existing single-shot
  interactive `recoverAuth` board-login exchange (CLI-only, human in
  the loop) is unaffected — it still gets exactly one bounded recovery
  attempt before giving up.
- packages/mcp-server/src/client.ts: PaperclipApiAuthError extends
  PaperclipApiError the same way, and format.ts tags its MCP tool
  error response with `errorClass: "auth_failed"` so a future incident
  doesn't require decoding the JWT by hand to find the real cause.

Regression tests (both packages) simulate a 401 response mid a bounded
retry loop and assert the client/wrapper stops after one attempt with
a distinguishable error, not the generic timeout/5xx classification.

Also documents the "never retry a 401" rule in skills/paperclip/SKILL.md
next to the existing "never retry a 409" rule, so agents writing their
own retry wrappers around Paperclip API calls know to special-case it.

AC1: 401 classified as a distinct auth-failure error class immediately.
AC2: retrying a 401 is a no-op — the client throws before any retry
     loop sees a generic/timeout-shaped error.
AC3: the distinct error class name and `errorClass: "auth_failed"` in
     the MCP error payload surface the real cause without JWT decoding.
AC4: regression tests for both clients.
@PraeSynBH
PraeSynBH force-pushed the rbr-1036-401-fail-fast branch from 73da26c to 80bbbef Compare August 7, 2026 00:54
@PraeSynBH

Copy link
Copy Markdown
Owner Author

Rebased onto latest master (was 2 commits behind, causing a merge conflict purely in unrelated UI files that had diverged since this branch was cut — IssueFiltersPopover.tsx, ui-flags.ts, etc. from PAP-411). The 6 files this fix actually touches (cli/src/client/http.ts, cli/src/__tests__/http.test.ts, packages/mcp-server/src/client.ts, packages/mcp-server/src/client.test.ts, packages/mcp-server/src/format.ts, skills/paperclip/SKILL.md) are byte-identical to the original commit.

Re-verified after rebase:

  • cli/src/__tests__/http.test.ts — 8/8 passing
  • packages/mcp-server/src/client.test.ts — 3/3 passing

mergeable is now MERGEABLE (was CONFLICTING). The remaining review check failure (ERROR: COMMITPERCLIP_KEY env var not set) is a pre-existing CI secrets-config issue unrelated to this change — PR #8 (also open, unrelated content) fails the identical Generate commitperclip token step for the same reason. This PR is otherwise ready for human review/merge.

@PraeSynBH
PraeSynBH merged commit 6f0a5be into master Aug 9, 2026
25 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants