Add Clone Loop OAuth login command#28
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new /clone:api-key login command that enables browser-based OAuth setup for the Clone API key. It implements a local loopback callback server to receive the authorization code, exchanges it for an API key, and saves it. Documentation and tests have been updated accordingly. Feedback on the implementation highlights a potential hanging promise in startCallbackServer if the server address is invalid, a missing timeout for browser authorization, and an unhandled promise rejection if the callback server fails to start.
| async function startCallbackServer(expectedState) { | ||
| let resolveReady | ||
| let rejectReady | ||
| const ready = new Promise((resolve, reject) => { | ||
| resolveReady = resolve | ||
| rejectReady = reject | ||
| }) | ||
| let resolveCode | ||
| let rejectCode | ||
| const codePromise = new Promise((resolve, reject) => { | ||
| resolveCode = resolve | ||
| rejectCode = reject | ||
| }) | ||
| const server = createServer((req, res) => { | ||
| const requestUrl = new URL(req.url || '/', 'http://127.0.0.1') | ||
| if (requestUrl.pathname !== '/callback') { | ||
| res.writeHead(404, { 'Content-Type': 'text/plain; charset=utf-8' }) | ||
| res.end('Not found') | ||
| return | ||
| } | ||
|
|
||
| const code = requestUrl.searchParams.get('code') || '' | ||
| const state = requestUrl.searchParams.get('state') || '' | ||
| if (!code || state !== expectedState) { | ||
| res.writeHead(400, { 'Content-Type': 'text/plain; charset=utf-8' }) | ||
| res.end('Clone Loop login failed. You can close this tab and retry.') | ||
| rejectCode(new Error('Clone Loop login callback state mismatch.')) | ||
| return | ||
| } | ||
|
|
||
| res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) | ||
| res.end('<!doctype html><title>Clone Loop connected</title><p>Clone Loop connected. You can close this tab.</p>') | ||
| resolveCode(code) | ||
| }) | ||
| server.once('error', (err) => { | ||
| rejectReady(err) | ||
| rejectCode(err) | ||
| }) | ||
| server.listen(0, '127.0.0.1', () => { | ||
| const address = server.address() | ||
| if (!address || typeof address === 'string') { | ||
| rejectCode(new Error('Could not start Clone Loop login callback server.')) | ||
| server.close() | ||
| return | ||
| } | ||
| resolveReady({ | ||
| redirectUri: `http://127.0.0.1:${address.port}/callback`, | ||
| codePromise, | ||
| close: () => new Promise((resolveClose) => server.close(resolveClose)), | ||
| }) | ||
| }) | ||
| return ready | ||
| } |
There was a problem hiding this comment.
Hanging Promise & Missing Timeout in Callback Server
There are two issues in startCallbackServer:
- Hanging Promise on Invalid Address: If
addressis null or a string,rejectCodeis called, butrejectReadyis never called. SinceloginWithCloneawaitsstartCallbackServer(which returns thereadypromise), the command will hang indefinitely becausereadyis never resolved or rejected. - Missing Timeout: If the user starts the login flow but never completes it in the browser, the callback server will run indefinitely, hanging the CLI process. Adding a standard 5-minute timeout ensures the process exits gracefully if inactive.
async function startCallbackServer(expectedState) {
let resolveReady
let rejectReady
const ready = new Promise((resolve, reject) => {
resolveReady = resolve
rejectReady = reject
})
let resolveCode
let rejectCode
const codePromise = new Promise((resolve, reject) => {
resolveCode = resolve
rejectCode = reject
})
const timeoutId = setTimeout(() => {
rejectCode(new Error('Clone Loop login timed out after 5 minutes.'))
}, 300000)
const server = createServer((req, res) => {
const requestUrl = new URL(req.url || '/', 'http://127.0.0.1')
if (requestUrl.pathname !== '/callback') {
res.writeHead(404, { 'Content-Type': 'text/plain; charset=utf-8' })
res.end('Not found')
return
}
const code = requestUrl.searchParams.get('code') || ''
const state = requestUrl.searchParams.get('state') || ''
if (!code || state !== expectedState) {
res.writeHead(400, { 'Content-Type': 'text/plain; charset=utf-8' })
res.end('Clone Loop login failed. You can close this tab and retry.')
rejectCode(new Error('Clone Loop login callback state mismatch.'))
return
}
res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' })
res.end('<!doctype html><title>Clone Loop connected</title><p>Clone Loop connected. You can close this tab.</p>')
clearTimeout(timeoutId)
resolveCode(code)
})
server.once('error', (err) => {
clearTimeout(timeoutId)
rejectReady(err)
rejectCode(err)
})
server.listen(0, '127.0.0.1', () => {
const address = server.address()
if (!address || typeof address === 'string') {
clearTimeout(timeoutId)
const err = new Error('Could not start Clone Loop login callback server.')
rejectReady(err)
rejectCode(err)
server.close()
return
}
resolveReady({
redirectUri: `http://127.0.0.1:${address.port}/callback`,
codePromise,
close: () => {
clearTimeout(timeoutId)
return new Promise((resolveClose) => server.close(resolveClose))
},
})
})
return ready
}| const state = randomState() | ||
| const callback = await startCallbackServer(state) | ||
| const authorizeUrl = new URL('/clone-loop/connect', dashboardBaseUrl()) | ||
| authorizeUrl.searchParams.set('redirect_uri', callback.redirectUri) | ||
| authorizeUrl.searchParams.set('state', state) | ||
| authorizeUrl.searchParams.set('agent_id', cloneLoopAgentId()) | ||
|
|
||
| console.log(`Authorize: ${authorizeUrl.toString()}`) | ||
| if (!noOpen) { | ||
| try { | ||
| await openBrowser(authorizeUrl.toString()) | ||
| } catch (err) { | ||
| console.log(`Open this URL in your browser: ${authorizeUrl.toString()}`) | ||
| console.log(`Browser launch failed: ${err instanceof Error ? err.message : String(err)}`) | ||
| } | ||
| } | ||
| console.log('Waiting for Clone browser authorization...') | ||
|
|
||
| try { | ||
| const code = await callback.codePromise | ||
| await callback.close() | ||
| const token = await exchangeCloneLoopCode({ code, state }) | ||
| writePluginConfigToken(token) | ||
| console.log('Stored Clone API key from Clone OAuth login.') | ||
| console.log(`Token: ${maskToken(token)}`) | ||
| console.log(`Plugin config: ${authFilePath()}`) | ||
| await connectResolvedToDashboard({ | ||
| token, | ||
| source: 'plugin config', | ||
| masked: maskToken(token), | ||
| isDemo: false, | ||
| }) | ||
| } catch (err) { | ||
| try { | ||
| await callback.close() | ||
| } catch {} | ||
| fail(err instanceof Error ? err.message : String(err)) | ||
| } |
There was a problem hiding this comment.
Unhandled Promise Rejection on Server Startup or Browser Launch Failure
Currently, startCallbackServer is called outside of the try...catch block. If the callback server fails to start (e.g., due to port binding issues or invalid address info), the promise will reject and throw an unhandled exception, crashing the CLI process with a full stack trace instead of exiting cleanly.
Wrapping the entire initialization and execution flow in a single try...catch block ensures all errors (including server startup and browser launch failures) are caught and handled gracefully via fail().
let callback
try {
const state = randomState()
callback = await startCallbackServer(state)
const authorizeUrl = new URL('/clone-loop/connect', dashboardBaseUrl())
authorizeUrl.searchParams.set('redirect_uri', callback.redirectUri)
authorizeUrl.searchParams.set('state', state)
authorizeUrl.searchParams.set('agent_id', cloneLoopAgentId())
console.log(`Authorize: ${authorizeUrl.toString()}`)
if (!noOpen) {
try {
await openBrowser(authorizeUrl.toString())
} catch (err) {
console.log(`Open this URL in your browser: ${authorizeUrl.toString()}`)
console.log(`Browser launch failed: ${err instanceof Error ? err.message : String(err)}`)
}
}
console.log('Waiting for Clone browser authorization...')
const code = await callback.codePromise
await callback.close()
const token = await exchangeCloneLoopCode({ code, state })
writePluginConfigToken(token)
console.log('Stored Clone API key from Clone OAuth login.')
console.log(`Token: ${maskToken(token)}`)
console.log(`Plugin config: ${authFilePath()}`)
await connectResolvedToDashboard({
token,
source: 'plugin config',
masked: maskToken(token),
isDemo: false,
})
} catch (err) {
if (callback) {
try {
await callback.close()
} catch {}
}
fail(err instanceof Error ? err.message : String(err))
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15622519f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const code = await callback.codePromise | ||
| await callback.close() | ||
| const token = await exchangeCloneLoopCode({ code, state }) | ||
| writePluginConfigToken(token) |
There was a problem hiding this comment.
When the agent process already has a nonblank CLONE_API_TOKEN, this stores the OAuth key in plugin config and then verifies that new key directly, but subsequent Clone Loop commands still resolve the environment token first via resolveCloneToken(). In that environment /clone:api-key login appears to switch the user to the newly authorized private key while later loops continue using the old environment key, so the command should fail/warn or otherwise make the effective-token precedence clear after login.
Useful? React with 👍 / 👎.
1 participant
Summary
Test Plan
Companion