Skip to content

Return client errors for known OAuth reconnect states - #13794

Merged
devinfoley merged 1 commit into
masterfrom
fix/oauth-reconnect-health-errors
Sep 22, 2026
Merged

devinfoley merged 1 commit into
masterfrom
fix/oauth-reconnect-health-errors

Conversation

@devinfoley

@devinfoley devinfoley commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work.
  • Connected apps use OAuth credentials to discover and call tools.
  • Expired credentials can require a user to sign in again.
  • The service recognizes these states and provides reconnect instructions.
  • Health and discovery requests still convert two of these states to HTTP 502.
  • This pull request returns HTTP 422 for those known reconnect states.
  • Users keep the instructions, while unexpected provider failures remain reportable.

Linked Issues or Issue Description

Refs #13786 and #13788, which handle related connection setup failures.

What happened?

An expired OAuth credential without a refresh token returns HTTP 502 from catalog discovery and health checks. A terminal refresh rejection also becomes HTTP 502. The shared error handler reports both expected reconnect states as server failures.

Expected behavior

Return HTTP 422 with the existing error code and reconnect instructions. Continue reporting unexpected provider and refresh failures.

Steps to reproduce

  1. Connect an OAuth app, then let its access token expire without a refresh token.
  2. Request its catalog, refresh its catalog, or check its health.
  3. Observe HTTP 502 with the existing reconnect message.

Paperclip version or commit

a7d3b17.

Deployment mode

Server with OAuth app connections.

What Changed

  • Map oauth_refresh_missing and oauth_reauthorization_required to HTTP 422.
  • Test catalog discovery, catalog refresh, and health-check routes with expired credentials and no refresh token. Assert reconnect links and no Sentry capture.
  • Assert HTTP 422 for a terminal refresh rejection while preserving the existing single-use refresh-token test.
  • Document the expected reconnect failures.

Verification

  • All four added or changed regression cases failed with HTTP 502 before the mapping change.
  • Tool-access and error-handler suites: 344 tests pass with no skips, using a disposable local PostgreSQL instance through the existing test constructor override.
  • Slack error classifier suite: 19 tests pass. All 363 focused tests pass without skips.
  • Server tsc --noEmit passes.
  • The most recent full-root checks in this session had 8,211 passing tests and 14 existing macOS embedded-PostgreSQL/runtime-cache failures. Root typecheck and build were blocked by missing cargo. These full-root checks predate this small mapping change; Linux CI must pass before merge.

Risks

Low risk. Two known reconnect error codes now return HTTP 422 instead of HTTP 502. Credential refresh, rotation, storage, connection permissions, and retry behavior are unchanged. Unknown refresh failures still use the server-error path. No schema changes.

Model Used

OpenAI GPT-6 via Codex, with reasoning, shell tools, code editing, and test execution. The exact model snapshot and context-window size are not exposed by this session.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change (e.g. docs/..., fix/...) and contains no internal Paperclip ticket id or instance-derived details
  • I have run focused tests locally and they pass (full-root limitations described above)
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • 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
  • I will address all Greptile and reviewer comments before requesting merge

Expired credentials without a refresh token and terminal refresh rejections
already provide reconnect instructions, but health and discovery mapped them
to HTTP 502. Preserve their known codes and return HTTP 422 so expected user
remediation does not appear as an unexpected server failure.

Cover all three discovery/health routes and terminal token rejection.
Tool-access and error-handler tests (344) and server typecheck pass.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@greptile-apps

greptile-apps Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with the changed status classification narrowly limited to established reconnect-required OAuth states.

Summary

This PR classifies two established OAuth reconnect states as client-actionable failures while preserving server-error handling for unexpected provider failures.

  • Maps oauth_refresh_missing and oauth_reauthorization_required to HTTP 422.
  • Adds route-level regression coverage for catalog discovery, catalog refresh, and health checks when an expired credential has no refresh token.
  • Confirms terminal refresh rejection uses 422 and documents the expected observability behavior.

Reviews (1) · Last reviewed commit: "Return client errors for known OAuth rec..."

@devinfoley
devinfoley merged commit c496acb into master Sep 22, 2026
55 checks passed
@devinfoley
devinfoley deleted the fix/oauth-reconnect-health-errors branch September 22, 2026 03:50
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.

1 participant