Skip to content

fix(auth): correct Codespaces redirect using asExternalUri #7678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix(auth): correct Codespaces redirect using asExternalUri #7678

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion packages/cloud/src/WebAuthService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,30 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A
// Generate a cryptographically random state parameter.
const state = crypto.randomBytes(16).toString("hex")
await this.context.globalState.update(AUTH_STATE_KEY, state)

// Resolve the deep link target for the CURRENT VS Code instance.
Copy link
Contributor Author

@roomote roomote bot Sep 4, 2025

Choose a reason for hiding this comment

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

Would it be worth adding a JSDoc comment to the login method documenting this new Codespaces-specific behavior? The inline comments are helpful, but a method-level documentation might help future maintainers understand the overall auth flow better.

// In remote/web (e.g. GitHub Codespaces), asExternalUri produces an HTTPS URL
// that routes back to the running instance instead of launching a local VS Code.
const packageJSON = this.context.extension?.packageJSON
const publisher = packageJSON?.publisher ?? "RooVeterinaryInc"
const name = packageJSON?.name ?? "roo-cline"

const deepLink = vscode.Uri.parse(`${vscode.env.uriScheme}://${publisher}.${name}`)
Copy link
Contributor Author

@roomote roomote bot Sep 4, 2025

Choose a reason for hiding this comment

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

Consider extracting this deep link construction and resolution logic (lines 271-280) into a separate private method like resolveAuthRedirectUri(). This would improve readability and make it easier to unit test this specific functionality.


let authRedirect = deepLink.toString()
try {
const external = await vscode.env.asExternalUri(deepLink)
authRedirect = external.toString()
Copy link
Contributor Author

@roomote roomote bot Sep 4, 2025

Choose a reason for hiding this comment

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

Missing test coverage for this new asExternalUri logic. Should we add tests to verify both the success path and the fallback behavior when asExternalUri fails? The existing test file doesn't mock or test this new functionality.

} catch (e) {
// Fallback to the raw deep link if resolution fails (desktop will still work).
this.log(`[auth] asExternalUri failed, falling back to deep link: ${e}`)
Copy link
Contributor Author

@roomote roomote bot Sep 4, 2025

Choose a reason for hiding this comment

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

Could we make the error logging more specific here? Instead of just ${e}, perhaps ${e.message} or even the full error object would help with debugging, especially since this is a new code path that might fail in unexpected ways.

}

const params = new URLSearchParams({
state,
auth_redirect: `${vscode.env.uriScheme}://${publisher}.${name}`,
auth_redirect: authRedirect,
})

const url = `${getRooCodeApiUrl()}/extension/sign-in?${params.toString()}`
await vscode.env.openExternal(vscode.Uri.parse(url))
} catch (error) {
Expand Down
Loading