Show OAuth callback failures in the UI instead of dropping them

[henry-dowling]

Problem

When an OAuth token exchange failed, the backend redirected to /settings?integration_error=<provider>&reason=connection_failed — but no frontend code consumed that param. Because a re-authorization on an already-approved app auto-redirects instantly, the whole failed round-trip (settings → provider → callback → settings) took under a second and looked exactly like a dead Connect button. Worse, the two most common real failures never emitted the param at all: cancelling on the consent screen returned a raw FastAPI 422 JSON page on the API host (missing required code param), and an expired state (consent tab left open past the 10-minute TTL) returned bare 400 JSON.

What changed

Frontend (SourceConnectorList):

• Reads integration_error + reason on mount and shows an error banner above the connector cards (the old error slot rendered below ~10 cards, under the fold).
• Scrolls the banner into view — the Sources section is the fourth card on /settings, below the fold on typical viewports.
• Distinct message for reason=access_denied ("authorization was cancelled") vs exchange failures.
• Strips the params from the URL so refresh/copied links don't resurrect a stale error.
• Never echoes the URL-controlled provider slug: unknown slugs get a generic "Connection failed" (content-spoofing guard).

Backend (integrations/router.py):

• code/state are now optional; a consent denial (?error=access_denied, no code — RFC 6749 §4.1.2.1) or missing params redirects with integration_error instead of 422 JSON.
• Invalid/expired state redirects instead of raising bare 400 JSON — the callback is always a top-window navigation, so every failure must end in a redirect the UI can render.

Screenshot

Banner on /settings after a failed GitHub connect (captured live in dev): https://app.joinstash.ai/f/7a11943b-2ed0-4b0e-83fe-64fefad3e150

Tests

• Frontend: 9/9 vitest (banner shown + params stripped, unrelated params preserved, access-denied wording, unknown-slug no-echo, scroll-into-view, plus existing disconnect/visibility tests).
• Backend: test_integration_callback_security.py 8/8 — new: consent denial → 302 reason=access_denied, missing code → 302, invalid state → 302.
• Full backend suite: 397 passed; 1 failure (test_github_index_success_logs_internal_source_id_only) reproduces on pristine main with this change stashed — pre-existing, unrelated.
• ruff check/format clean; eslint 0 errors; tsc clean in changed files.

🤖 Generated with Claude Code

[assert-app]

Review on Assert →

2 clusters identified

Merge candidate is ready!

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
stash-web	Ready	Preview, Comment	Jul 20, 2026 9:04pm

[ischindl]

Review: PR #864 — Show OAuth callback failures in the UI instead of dropping them

Author: Henry Dowling
Files: 4 files (+250 / -15)
Reviewed against project standards (AGENTS.md + 12 rules)

Verdict: APPROVED

Standards compliance

1. No backwards compatibility / fallbacks — OAuth callback params (code, state) went from required to optional; the new error param is additive. The old required-param behavior would have 422'd — the PR intentionally converts that to a UI-friendly redirect instead. No compatibility shims, no dual code paths, no fallback defaults.

2. No excessive try/catch — Zero new try/except blocks added. The old except HTTPException: raise becomes except HTTPException: redirect, repurposing an existing handler — no new catch surface.

3. Simple, readable code — The _error_redirect() helper deduplicates redirect-URL construction. The frontend useEffect parses params, sets state, and strips from URL in a single pass. The callbackError split from error is justified by lifecycle differences (refresh() clears error but not callbackError).

4. Tests verify intent — Backend: 3 tests assert correct 302 + location for each failure mode (consent denial, missing code, invalid state). Frontend: 5 tests cover banner visibility, access_denied wording, content-spoofing guard, param preservation, and no-param-no-banner.

5. Match codebase conventions — Uses existing connectorForProvider(), existing error-banner styling, existing pytest/AsyncClient patterns.

Specific observations

• Content-spoofing guard is correct: unrecognized slugs never echo into DOM text.
• URL stripping uses replaceState with [] deps, so refresh doesn't resurrect.
• Old bottom banner correctly removed — consolidated above cards, fixing the below-the-fold invisibility.
• scrollIntoView dependency is justified: the banner renders in the 4th settings section.

Concerns

None. Minimal, correct, well-tested.

Structured outcome

files=[backend/integrations/router.py, backend/tests/test_integration_callback_security.py, frontend/src/components/integrations/SourceConnectorList.tsx, frontend/src/components/integrations/SourceConnectorList.test.tsx], violations={fallbacks:0, trycatch:0, readability:0, test_intent:0, conventions:0}, localized_only=yes