SkyFi MCP Authentication #753

Activity

1. ngoiyaeric commented on Jul 24, 2026

   

   ngoiyaeric

   on Jul 24, 2026

   CollaboratorAuthor

   More actions

   @coderabbitai plan
2. coderabbitai commented on Jul 24, 2026

   

   coderabbitaibot

   on Jul 24, 2026 – with coderabbitai

   Contributor

   More actions

   Coding Plan

   Summary

   • Treat the ticket as a bug fix on the existing hand-rolled SkyFi OAuth/PKCE flow rather than a Composio migration, keeping changes minimal and aligned with current architecture.
   • Fix the primary "button not routing" issue by making the redirect URI deterministic/proxy-safe, reusing (not force-recreating) the dynamically registered client, validating the registration response, and surfacing real errors instead of silently bouncing users to sign-in.
   • Guarantee the redirect works end-to-end by keeping redirect_uri consistent through token exchange and normalizing MCP auth headers plus token-refresh handling so a "connected" account can actually query imagery.
   • Improve settings status feedback so users can confirm the connection and recover from failures.

   Design Choices

   Design Choice 1: Whether to keep the existing hand-rolled OAuth/PKCE flow or migrate SkyFi to the Composio pattern used by the mapbox integration.

   Options Considered:

   1. Keep and fix the existing hand-rolled SkyFi OAuth/PKCE flow, addressing the specific routing/registration/redirect bugs.
   2. Re-implement SkyFi on top of Composio (connectedAccounts.initiate + executeAction), mirroring the mapbox integration.

   Chosen Option: 1

   Rationale: The ticket is a bug fix ("button not routing to login"), and a full Composio rewrite would discard a working callback/token pipeline and introduce unnecessary scope and risk; fixing the existing flow is the natural, minimal-change path.

   Design Choice 2: How to determine the correct public-facing redirect URI in deployed environments.

   Options Considered:

   1. Prefer an explicit configured base URL (NEXT_PUBLIC_APP_URL) and fall back to forwarded/host headers only when it is absent.
   2. Continue trusting the request Host header first, keeping env var as fallback.

   Chosen Option: 1

   Rationale: An explicit configured public URL is deterministic and avoids proxy/host-header mismatches that break the OAuth redirect_uri contract, while still allowing local development to work.

   💡 User Tips

   Regenerate the plan with different choices with @coderabbitai <feedback>.

   Implementation Steps

   Phase 1: Fix and Harden the SkyFi Connection Initiation (Primary Bug)

   This phase resolves the core ticket issue — the button not routing users to SkyFi login. The objective is to make startSkyfiConnection reliably produce a valid, correctly-formed authorize URL, to stop silently swallowing failures, and to ensure the redirect URI is consistent and correct across environments so SkyFi accepts the request.

   Task 1: Make redirect URI derivation deterministic and proxy-safe

   Ensure the redirect_uri used for client registration and the authorize URL matches the app's real public domain in all environments.

   • Update getRedirectUri() in lib/actions/skyfi.ts to prefer an explicitly configured public base URL (NEXT_PUBLIC_APP_URL) when present, and only fall back to request-header derivation.
   • When deriving from headers, account for proxy/forwarded headers (e.g., x-forwarded-host/x-forwarded-proto) so the value reflects the public domain rather than an internal host or port.
   • Add NEXT_PUBLIC_APP_URL to .env / document it in .env.local.example as required for non-local deployments so the fallback never silently resolves to localhost in production.

   Task 2: Reuse dynamic client registration instead of forcing re-registration

   Stop overwriting per-user client credentials/tokens on every connect click, and validate the registration response.

   • Change startSkyfiConnection in lib/actions/skyfi.ts to call ensureClientRegistered(provider) without hard-coding forceRegister = true, so an existing valid client_id is reused when the stored redirect_uri still matches.
   • In ensureClientRegistered, validate that the DCR response actually contains a truthy client_id before saving/returning; throw a descriptive error otherwise so a malformed response cannot produce a broken authorize URL.
   • Ensure re-registration only occurs when there is no stored client or the redirect URI has changed, preventing the previously-working token/client pairing from being clobbered.

   Task 3: Surface connection failures instead of failing silently

   Distinguish real errors from "not signed in" so the button no longer appears to do nothing.

   • In lib/auth/get-current-user.ts, differentiate a genuine "no Clerk session" from an internal DB-resolution error in resolveClerkUserToDbUser so startSkyfiConnection does not misreport authenticated users as authRequired (which currently bounces them to /sign-in).
   • In startSkyfiConnection, return clear, distinct error messages for registration/network/timeout failures versus authentication failures.
   • Update the connect handler in components/settings/components/tool-selection-form.tsx so that when a real error is returned (not authRequired), it presents the specific error to the user and resets the connecting state, rather than a generic message that looks like the button "did nothing."

   🤖 Prompt for AI agents

   Implement fixes to resolve the core bug where the "Connect SkyFi Account" button
   fails to route users to SkyFi's login/authorize page.
   
   1. Redirect URI derivation: Update `getRedirectUri()` in `lib/actions/skyfi.ts`
   to prefer an explicitly configured public base URL (`NEXT_PUBLIC_APP_URL`) when
   present, falling back to request-header derivation only when absent. When
   deriving from headers, account for proxy/forwarded headers (e.g.,
   `x-forwarded-host`/`x-forwarded-proto`) so the value reflects the public domain
   rather than an internal host or port. Add `NEXT_PUBLIC_APP_URL` to `.env` and
   document it in `.env.local.example` as required for non-local deployments so the
   fallback never silently resolves to `localhost` in production.
   
   2. Client registration reuse: In `lib/actions/skyfi.ts`, change
   `startSkyfiConnection` to call `ensureClientRegistered(provider)` without
   hard-coding `forceRegister = true`, so an existing valid `client_id` is reused
   when the stored `redirect_uri` still matches. In `ensureClientRegistered`,
   validate that the DCR response actually contains a truthy `client_id` before
   saving/returning; throw a descriptive error otherwise. Ensure re-registration
   only occurs when there is no stored client or the redirect URI has changed.
   
   3. Error surfacing: In `lib/auth/get-current-user.ts`, differentiate a genuine
   "no Clerk session" from an internal DB-resolution error in
   `resolveClerkUserToDbUser` so `startSkyfiConnection` does not misreport
   authenticated users as `authRequired`. In `startSkyfiConnection`, return clear,
   distinct error messages for registration/network/timeout failures versus
   authentication failures. Update the connect handler in
   `components/settings/components/tool-selection-form.tsx` so that when a real
   error is returned (not `authRequired`), it presents the specific error to the
   user and resets the connecting state.
   

   Phase 2: Ensure Post-Redirect Token Exchange and Tool Usage Succeed

   Once the redirect works, this phase guarantees the returned code exchanges into usable tokens and that the MCP tool authenticates consistently, so a "connected" state is real and functional.

   Task 1: Guarantee redirect_uri consistency between authorize and token exchange

   Prevent OAuth server rejection due to mismatched redirect URIs across the two requests.

   • Ensure app/api/skyfi/callback/route.ts derives the redirect_uri for the /oauth/token exchange using the same logic/source as startSkyfiConnection (via the shared getRedirectUri() updated in Task 1.1), so the value sent at token exchange exactly matches the one used at authorize time.

   Task 2: Normalize MCP authentication headers and token refresh handling

   Make token usage consistent and resilient so a connected account can actually query imagery.

   • Reconcile the authentication headers in lib/agents/tools/skyfi.tsx and the status check in lib/actions/skyfi.ts so both call sites use a consistent scheme for the OAuth access token (align on the header(s) SkyFi's MCP server actually expects, per the MCP Authorization: Bearer convention), removing ambiguity from sending the same token under conflicting header names.
   • Ensure SkyfiOAuthProvider.tokens() refresh logic is invoked before MCP connection and that expiry handling works even when expires_in was absent on initial exchange (treat missing expiry conservatively so tokens are validated/refreshed rather than assumed valid indefinitely).
   • On a 401/expired-token failure during tool execution in lib/agents/tools/skyfi.tsx, surface a clear "reconnect in Settings" signal distinct from generic connection errors.

   🤖 Prompt for AI agents

   Ensure that once the redirect works, the returned authorization code exchanges
   into usable tokens and the MCP tool authenticates consistently.
   
   1. Redirect URI consistency: Ensure `app/api/skyfi/callback/route.ts` derives
   the `redirect_uri` for the `/oauth/token` exchange using the same logic/source
   as `startSkyfiConnection` (via the shared, updated `getRedirectUri()`), so the
   value sent at token exchange exactly matches the one used at authorize time.
   
   2. Header and refresh normalization: Reconcile the authentication headers in
   `lib/agents/tools/skyfi.tsx` and the status check in `lib/actions/skyfi.ts` so
   both call sites use a consistent scheme for the OAuth access token, aligning on
   the header(s) SkyFi's MCP server actually expects (per the MCP `Authorization:
   Bearer` convention), removing ambiguity from sending the same token under
   conflicting header names. Ensure `SkyfiOAuthProvider.tokens()` refresh logic is
   invoked before MCP connection and that expiry handling works even when
   `expires_in` was absent on initial exchange (treat missing expiry conservatively
   so tokens are validated/refreshed rather than assumed valid indefinitely). On a
   401/expired-token failure during tool execution in `lib/agents/tools/skyfi.tsx`,
   surface a clear "reconnect in Settings" signal distinct from generic connection
   errors.
   

   Phase 3: Settings Connection-Status Feedback

   Ensure the settings UI accurately reflects connection state so users can confirm the fix and re-initiate if needed.

   Improve connection-status accuracy and reconnect affordance

   Give users reliable feedback on whether their SkyFi account is truly connected.

   • Ensure getSkyfiConnectionStatus in lib/actions/skyfi.ts reports connected only when a valid (refreshable) token exists, and returns actionable status when tokens are missing/expired.
   • Update components/settings/components/tool-selection-form.tsx to reflect error/expired states (e.g., offer reconnect) rather than only "linked" vs "connect," so a failed prior attempt is visible and recoverable.

   🤖 Prompt for AI agents

   Improve the settings UI so connection status is accurate and recoverable.
   
   1. Update `getSkyfiConnectionStatus` in `lib/actions/skyfi.ts` so it reports
   connected only when a valid (refreshable) token exists, and returns actionable
   status when tokens are missing/expired.
   
   2. Update `components/settings/components/tool-selection-form.tsx` to reflect
   error/expired states (e.g., offer reconnect) rather than only "linked" vs
   "connect," so a failed prior attempt is visible and recoverable to the user.
   

   Research

   This is a Next.js app that gates a SkyFi satellite-imagery MCP tool behind a per-user OAuth 2.1 + PKCE connection. The flow spans: a settings radio/card UI (components/settings/components/tool-selection-form.tsx) that calls server actions in lib/actions/skyfi.ts; a persistence-only OAuth provider (lib/skyfi/provider.ts) backed by the skyfi_oauth_tokens table; an OAuth callback route (app/api/skyfi/callback/route.ts) that exchanges the code for tokens; and the MCP tool executor (lib/agents/tools/skyfi.tsx) that connects to https://mcp.skyfi.com/mcp using stored tokens. AOI selection is already implemented via Mapbox draw polygons shared through useMapData and converted to WKT before tool execution. The connection flow is hand-rolled (not using Composio like the mapbox integration), and several silent-failure points in the connect path explain the button bug.

   ---

   🚀 Next Steps

   🤖 All AI agent prompts combined

   Task: 1
   
   Implement fixes to resolve the core bug where the "Connect SkyFi Account" button
   fails to route users to SkyFi's login/authorize page.
   
   1. Redirect URI derivation: Update `getRedirectUri()` in `lib/actions/skyfi.ts`
   to prefer an explicitly configured public base URL (`NEXT_PUBLIC_APP_URL`) when
   present, falling back to request-header derivation only when absent. When
   deriving from headers, account for proxy/forwarded headers (e.g.,
   `x-forwarded-host`/`x-forwarded-proto`) so the value reflects the public domain
   rather than an internal host or port. Add `NEXT_PUBLIC_APP_URL` to `.env` and
   document it in `.env.local.example` as required for non-local deployments so the
   fallback never silently resolves to `localhost` in production.
   
   2. Client registration reuse: In `lib/actions/skyfi.ts`, change
   `startSkyfiConnection` to call `ensureClientRegistered(provider)` without
   hard-coding `forceRegister = true`, so an existing valid `client_id` is reused
   when the stored `redirect_uri` still matches. In `ensureClientRegistered`,
   validate that the DCR response actually contains a truthy `client_id` before
   saving/returning; throw a descriptive error otherwise. Ensure re-registration
   only occurs when there is no stored client or the redirect URI has changed.
   
   3. Error surfacing: In `lib/auth/get-current-user.ts`, differentiate a genuine
   "no Clerk session" from an internal DB-resolution error in
   `resolveClerkUserToDbUser` so `startSkyfiConnection` does not misreport
   authenticated users as `authRequired`. In `startSkyfiConnection`, return clear,
   distinct error messages for registration/network/timeout failures versus
   authentication failures. Update the connect handler in
   `components/settings/components/tool-selection-form.tsx` so that when a real
   error is returned (not `authRequired`), it presents the specific error to the
   user and resets the connecting state.
   ===============================================================================
   
   Task: 2
   
   Ensure that once the redirect works, the returned authorization code exchanges
   into usable tokens and the MCP tool authenticates consistently.
   
   1. Redirect URI consistency: Ensure `app/api/skyfi/callback/route.ts` derives
   the `redirect_uri` for the `/oauth/token` exchange using the same logic/source
   as `startSkyfiConnection` (via the shared, updated `getRedirectUri()`), so the
   value sent at token exchange exactly matches the one used at authorize time.
   
   2. Header and refresh normalization: Reconcile the authentication headers in
   `lib/agents/tools/skyfi.tsx` and the status check in `lib/actions/skyfi.ts` so
   both call sites use a consistent scheme for the OAuth access token, aligning on
   the header(s) SkyFi's MCP server actually expects (per the MCP `Authorization:
   Bearer` convention), removing ambiguity from sending the same token under
   conflicting header names. Ensure `SkyfiOAuthProvider.tokens()` refresh logic is
   invoked before MCP connection and that expiry handling works even when
   `expires_in` was absent on initial exchange (treat missing expiry conservatively
   so tokens are validated/refreshed rather than assumed valid indefinitely). On a
   401/expired-token failure during tool execution in `lib/agents/tools/skyfi.tsx`,
   surface a clear "reconnect in Settings" signal distinct from generic connection
   errors.
   ===============================================================================
   
   Task: 3
   
   Improve the settings UI so connection status is accurate and recoverable.
   
   1. Update `getSkyfiConnectionStatus` in `lib/actions/skyfi.ts` so it reports
   connected only when a valid (refreshable) token exists, and returns actionable
   status when tokens are missing/expired.
   
   2. Update `components/settings/components/tool-selection-form.tsx` to reflect
   error/expired states (e.g., offer reconnect) rather than only "linked" vs
   "connect," so a failed prior attempt is visible and recoverable to the user.
   

   💡 Iterate on the plan with: `@coderabbitai` <feedback>

   Example Feedback
   - `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
   - `@coderabbitai` For design choice 1 go ahead with option 3 and replan.
   

   ---

   💬 Have feedback or questions? Drop into our discord!