Skip to content

Conversation

JonZeolla
Copy link

@JonZeolla JonZeolla commented Sep 9, 2025

Description

This replaces the hardcoded localhost from #6800 with using asExternalUrl to support configuration in more complex environments

AI Code Review

  • Team members only: AI review runs automatically when PR is opened or marked ready for review
  • Team members can also trigger a review by commenting @continue-general-review or @continue-detailed-review

Checklist

  • I've read the contributing guide
  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

Screen recording or screenshot

asExternalUrl.mov

Tests

Updated core/context/mcp/MCPOauth.vitest.ts to ensure use of getExternalUri when available, and fallback to the old localhost:3000 when it isn't, and some (limited) concurrency tests.

npx vitest run context/mcp/MCPOauth.vitest.ts

 RUN  v3.1.4 /Users/jonzeolla/src/jonzeolla/continue/core

(node:73219) [DEP0147] DeprecationWarning: In future versions of Node.js, fs.rmdir(path, { recursive: true }) will be removed. Use fs.rm(path, { recursive: true }) instead
(Use `node --trace-deprecation ...` to show where the warning was created)
stderr | context/mcp/MCPOauth.vitest.ts > MCPOauth > redirect URL handling > should handle getExternalUri errors gracefully
Failed to initialize redirect URL: Error: Network error
    at Object.<anonymous> (/Users/jonzeolla/src/jonzeolla/continue/core/context/mcp/MCPOauth.vitest.ts:218:52)
    at Object.mockCall (file:///Users/jonzeolla/src/jonzeolla/continue/core/node_modules/@vitest/spy/dist/index.js:66:15)
    at Object.spy [as getExternalUri] (file:///Users/jonzeolla/src/jonzeolla/continue/core/node_modules/tinyspy/dist/index.js:45:80)
    at MCPConnectionOauthProvider._initializeRedirectUrl (/Users/jonzeolla/src/jonzeolla/continue/core/context/mcp/MCPOauth.ts:89:44)
    at new MCPConnectionOauthProvider (/Users/jonzeolla/src/jonzeolla/continue/core/context/mcp/MCPOauth.ts:81:41)
    at Module.performAuth (/Users/jonzeolla/src/jonzeolla/continue/core/context/mcp/MCPOauth.ts:241:24)
    at /Users/jonzeolla/src/jonzeolla/continue/core/context/mcp/MCPOauth.vitest.ts:226:13
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at file:///Users/jonzeolla/src/jonzeolla/continue/core/node_modules/@vitest/runner/dist/index.js:596:20

 ✓ context/mcp/MCPOauth.vitest.ts (12 tests) 10ms
   ✓ MCPOauth > getOauthToken > should return undefined when no tokens are stored 1ms
   ✓ MCPOauth > getOauthToken > should return access token when tokens are stored 1ms
   ✓ MCPOauth > getOauthToken > should handle different server URLs independently 0ms
   ✓ MCPOauth > performAuth > should call auth with correct parameters and return auth result 2ms
   ✓ MCPOauth > removeMCPAuth > should clear oauth storage for the server 0ms
   ✓ MCPOauth > removeMCPAuth > should handle non-existent server gracefully 0ms
   ✓ MCPOauth > redirect URL handling > should use getExternalUri when available for VS Code 0ms
   ✓ MCPOauth > redirect URL handling > should fallback to localhost when getExternalUri is not available 0ms
   ✓ MCPOauth > redirect URL handling > should handle getExternalUri errors gracefully 3ms
   ✓ MCPOauth > concurrent authentication > should handle multiple concurrent auth flows 0ms
   ✓ MCPOauth > error handling > should clean up context on auth failure 1ms
   ✓ MCPOauth > error handling > should handle missing server URL 0ms

 Test Files  1 passed (1)
      Tests  12 passed (12)
   Start at  07:25:48
   Duration  224ms (transform 56ms, setup 11ms, collect 80ms, tests 10ms, environment 0ms, prepare 27ms)

Summary by cubic

Switch OAuth redirects to use IDE external URLs (VS Code asExternalUri) instead of hardcoded localhost, so callbacks work in local, remote, and web environments. Adds state-based concurrency and safer server handling.

  • New Features

    • Use ide.getExternalUri for the redirect URL; fallback to http://localhost:3000.
    • Add OAuth state to support multiple concurrent auth flows.
    • Start the local callback server only when using localhost; close it after completion.
    • Extend IDE interface with getExternalUri and implement it in the VS Code extension.
  • Bug Fixes

    • Fix broken OAuth in remote/web VS Code where localhost callbacks failed.
    • Better error handling and toasts; clean up auth context on failure.
    • Add tests for redirect URL behavior, fallbacks, and concurrent auth flows.

@JonZeolla JonZeolla requested a review from a team as a code owner September 9, 2025 11:30
@JonZeolla JonZeolla requested review from RomneyDa and removed request for a team September 9, 2025 11:30
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 9, 2025
Copy link

github-actions bot commented Sep 9, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@JonZeolla
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

}

get redirectUrl() {
return `http://localhost:${PORT}`; // TODO: this has to be a hub url or should we spin up a server?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main thing I'm addressing in this PR. I don't fully understand the question about hub URLs; should I leave a comment behind somewhere like there was before?

@JonZeolla JonZeolla force-pushed the feat/oauth/redirect_uri branch from b54ed5c to ada6cdd Compare September 9, 2025 12:24
@RomneyDa
Copy link
Collaborator

@JonZeolla the files changed appears to be in an odd state where it's showing a bunch of extraneous added spaces, formatting changes, etc. Could you take a look and whittle down to only the relevant changes?

@JonZeolla
Copy link
Author

@RomneyDa that came from prettier, following the contributing guide. I can undo it though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants