mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
3c4050e3ad
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Managed connections (Gmail, Google Workspace, GitHub, Asana, Slack manager) get and refresh their OAuth tokens through the Paperclip Cloud connector broker. > - When a provider refuses a refresh grant (the user revoked it, it expired, it was already rotated, or it no longer matches the profile), the broker sent `502 PROVIDER_OPERATION_FAILED`. That is the same answer it sends for a provider outage. > - Paperclip then kept the grant `active`. Each health check, catalog refresh, or catalog read failed again with a 502, and the server reported each failure as a crash. The user did not get a reconnect prompt. > - The broker now sends `409 PROVIDER_REAUTHORIZATION_REQUIRED` for a dead grant. The connector already maps any 409 to `REAUTHORIZATION_REQUIRED`. Thus the grant becomes `needs_reauthorization` and the caller gets a 422 reconnect response. > - This pull request adds the new reason to the diagnostic allowlist, pins the producer contract, and tests the reconnect path from end to end. > - The benefit is that a dead managed grant asks the user to reconnect. It is not reported as a server crash. Real provider outages keep their 502 response. ## Linked Issues or Issue Description **What happened?** A managed Paperclip Cloud connection refresh failed with `Paperclip Cloud connector rejected the request (operation=refresh, status=502, reason=PROVIDER_OPERATION_FAILED)`. The connection health-check and catalog routes wrap that error as an HTTP 502. The error handler reports every status of 500 or more, so the failure went to error monitoring. The grant stayed `active`, so the same failure can occur again on each use. **Expected behavior** If the provider refuses the refresh grant, Paperclip marks the grant `needs_reauthorization` and shows the reconnect path. Only a real provider or Cloud outage gives a 502. **Steps to reproduce** 1. Connect a managed Gmail connection through Paperclip Cloud. 2. Revoke the app access in the Google account, or let the refresh grant expire. 3. Wait until the access token needs a refresh. Then run a connection health check. 4. Before the broker change, the health check returns 502 and the server reports it. The grant stays `active`. **Additional context** The broker-side classification ships separately in Paperclip Cloud. This pull request is safe before or after that change, because the connector already maps any broker 409 to `REAUTHORIZATION_REQUIRED`. Without this pull request, the 409 still works, but diagnostics show `reason=UNKNOWN_BROKER_ERROR`. ## What Changed - `server/src/services/paperclip-cloud-connector.ts`: Add `PROVIDER_REAUTHORIZATION_REQUIRED` to the allowlist of broker reasons that errors can keep. - `server/src/services/fixtures/cloud-connector-refresh-reauthorization.json`: Add the producer HTTP contract (status 409 and the exact JSON error body). - `server/src/services/paperclip-cloud-connector.test.ts`: Test that a refresh with this contract gives `REAUTHORIZATION_REQUIRED`, status 409, the retained reason, and no refresh-token text. - `server/src/__tests__/tool-access-service.test.ts`: Test that `refreshOAuthGrantCredentials` changes a reauthorization refresh failure into a 422 `oauth_reauthorization_required` response and sets the grant to `needs_reauthorization`. - `doc/connections/GMAIL.md`: Add a troubleshooting row that tells the two refresh reasons apart. ## Verification - `cd server && npx vitest run src/services/paperclip-cloud-connector.test.ts`: 48 passed. Without the allowlist change, the new test fails (`reason=UNKNOWN_BROKER_ERROR`). - `cd server && npx vitest run src/__tests__/tool-access-service.test.ts -t "managed grant for reconnection|orders managed reconnect"`: 2 passed, against the embedded test database. - I did not run the full `pnpm -r typecheck`, `pnpm test:run`, and `pnpm build` locally. The sandbox has a 4 GB memory limit, and the server `tsc` process was stopped because it used too much memory. CI runs these checks. ## Risks - Low risk. The status mapping does not change: a 409 already meant `REAUTHORIZATION_REQUIRED`. This change only keeps one more fixed reason code in the error message. - The reason allowlist accepts fixed protocol codes only. The broker message, request data, and tokens stay out of errors and logs. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code agent mode with tool use (shell, file edit, code search, and a sub-agent for the code-path trace). ## 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 - [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: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>