Skip to content

Commit 57a514a

Browse files
Harshit16gHarshit16g
committed
fix(github): fix self-polling bug in callback-server; use port 0 and redirect_uri
1 parent 4180021 commit 57a514a

2 files changed

Lines changed: 140 additions & 133 deletions

File tree

‎packages/opencode/src/cli/cmd/github.handler.ts‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -283,19 +283,18 @@ export const githubInstall = Effect.fn("Cli.github.install")(function* () {
283283
const installation = await getInstallation()
284284
if (installation) return s.stop("GitHub app already installed")
285285

286-
// Start a local HTTP server to receive the browser callback.
287-
// This ensures the CLI waits for the user to complete the GitHub App
288-
// installation in the browser before polling — preventing the hang where
289-
// polling starts before the user even switches to the browser window.
286+
// Start a local HTTP server on an OS-assigned port to receive the
287+
// GitHub App installation redirect. The server's promise resolves only
288+
// when the real browser hits the callback URL — no polling.
290289
const { startCallbackServer, waitForCallback } = await import("@/util/callback-server")
291-
const server = startCallbackServer()
292-
const callbackPromise = waitForCallback(server, {
293-
path: "/github-install-callback",
294-
timeoutMs: 300_000, // 5 minutes
295-
})
296-
297-
// Open browser
298-
const url = "https://github.com/apps/opencode-agent"
290+
const callbackPath = "/github-install-callback"
291+
const server = startCallbackServer(callbackPath)
292+
const callbackPromise = waitForCallback(server, { timeoutMs: 300_000 })
293+
294+
// Build the install URL with a redirect_uri so GitHub sends the browser
295+
// back to the local callback server after the user installs the app.
296+
const redirectUri = `http://127.0.0.1:${server.port}${callbackPath}`
297+
const url = `https://github.com/apps/opencode-agent/installations/new?redirect_uri=${encodeURIComponent(redirectUri)}`
299298
const command =
300299
process.platform === "darwin"
301300
? `open "${url}"`
@@ -305,31 +304,31 @@ export const githubInstall = Effect.fn("Cli.github.install")(function* () {
305304

306305
exec(command, (error) => {
307306
if (error) {
308-
prompts.log.warn(`Could not open browser. Please visit: ${url}`)
307+
prompts.log.warn(
308+
`Could not open browser. Please visit: https://github.com/apps/opencode-agent`,
309+
)
309310
}
310311
})
311312

312-
// Wait for user to complete GitHub App installation in the browser.
313-
// The server receives the redirect after the user installs the app,
314-
// which unblocks the CLI so it can verify with getInstallation().
315-
s.message("Waiting for GitHub app to be installed - complete it in your browser")
313+
// Block until the browser hits /github-install-callback (user finished
314+
// installing) or the 5-minute timeout fires.
315+
s.message("Waiting for GitHub app to be installed — complete it in your browser")
316316
try {
317317
await callbackPromise
318318
} catch {
319-
server.close()
320319
s.stop("GitHub app authorization timed out. Please try again.")
321320
throw new UI.CancelledError()
322321
}
323322

324-
// Verify installation completed
323+
// Confirm via API that the installation is actually visible server-side.
325324
const installed = await getInstallation()
326325
if (!installed) {
327-
server.close()
328-
s.stop(`Failed to detect GitHub app installation. Make sure to install the app for the \`${app.owner}/${app.repo}\` repository.`)
326+
s.stop(
327+
`Failed to detect GitHub app installation. Make sure to install the app for the \`${app.owner}/${app.repo}\` repository.`,
328+
)
329329
throw new UI.CancelledError()
330330
}
331331

332-
server.close()
333332
s.stop("Installed GitHub app")
334333

335334
async function getInstallation() {

‎packages/opencode/src/util/callback-server.ts‎

Lines changed: 119 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,53 @@
11
/**
2-
* Minimal HTTP server utilities for receiving browser-based OAuth/callbacks.
3-
*
4-
* This module provides a way to wait for a user to complete a browser-based
5-
* flow (e.g. GitHub App installation) by running a short-lived HTTP server
6-
* that resolves a promise when the browser hits the callback URL.
7-
*
8-
* Usage:
9-
* const { startCallbackServer, waitForCallback } = await import("@/util/callback-server")
10-
* const server = startCallbackServer()
11-
* const promise = waitForCallback(server, { path: "/callback", timeoutMs: 300_000 })
12-
* // Open browser, user completes auth in browser
13-
* await promise // resolves when browser hits the callback
14-
* server.close()
2+
* @fileOverview
3+
* @path: packages/opencode/src/util/callback-server.ts
4+
* @layer: SERVICE
5+
* @role: Short-lived HTTP server that waits for a browser OAuth/install callback
6+
* @owner: CLI
7+
*/
8+
9+
/**
10+
* @dependencies
11+
* internal: none
12+
* external:
13+
* - node:http
14+
*/
15+
16+
/**
17+
* @flow
18+
* startCallbackServer(path) -> binds on 127.0.0.1:0 (OS-assigned port)
19+
* -> caller opens browser with redirect_uri=http://127.0.0.1:{port}{path}
20+
* -> browser hits {path} after user completes auth
21+
* -> server serves HTML_SUCCESS, resolves internal promise
22+
* -> waitForCallback resolves (Promise.race with timeout)
23+
* -> caller calls server.close()
24+
*/
25+
26+
/**
27+
* @performance
28+
* - Uses port 0 (OS-assigned) — no hard-coded port collision risk
29+
* - No polling; purely event-driven via Promise
30+
*/
31+
32+
/**
33+
* @security
34+
* - Bound only to 127.0.0.1 — not reachable from outside localhost
35+
* - No CORS headers; no cross-origin access
36+
*/
37+
38+
/**
39+
* @scalability
40+
* - Single-use server; closed immediately after callback fires
41+
*/
42+
43+
/**
44+
* @verdict
45+
* status: CLEAN
46+
* priority: LOW
1547
*/
1648

1749
import http from "node:http"
1850

19-
// Reusable free port range — try a few to avoid conflicts
20-
const PORTS = [29734, 29735, 29736, 29737, 29738]
2151
const HTML_SUCCESS = `<!DOCTYPE html>
2252
<html lang="en">
2353
<head>
@@ -60,135 +90,113 @@ const HTML_SUCCESS = `<!DOCTYPE html>
6090
</body>
6191
</html>`
6292

63-
export interface CallbackOptions {
64-
/** URL path to listen on (e.g. "/github-install-callback") */
65-
path: string
66-
/** Timeout in milliseconds (default: 5 minutes) */
67-
timeoutMs?: number
68-
}
69-
70-
/** A started HTTP server that can be used with waitForCallback() */
7193
export interface CallbackServer {
72-
port: number
73-
close: () => void
94+
/** The OS-assigned port the server is listening on. */
95+
readonly port: number
96+
/** A promise that resolves when the browser hits the callback path. */
97+
readonly promise: Promise<void>
98+
/** Shuts down the server. Safe to call multiple times. */
99+
close(): void
74100
}
75101

76-
type DeferredResolve = () => void
77-
type DeferredReject = (err: Error) => void
78-
79102
/**
80-
* Starts an HTTP server on a port from the reusable range (29734–29738).
81-
* Returns a server that auto-responds with a success page on the configured path.
103+
* Starts a short-lived HTTP server on 127.0.0.1 with an OS-assigned port.
104+
*
105+
* The server exposes a `promise` that resolves when the browser hits `path`.
106+
* The caller is responsible for including the callback URL in whatever link
107+
* it opens in the browser, e.g.:
108+
* `http://127.0.0.1:${server.port}/github-install-callback`
109+
*
110+
* @param path - The URL path to listen on (e.g. "/github-install-callback")
82111
*/
83-
export function startCallbackServer(): CallbackServer {
84-
let closed = false
85-
let resolvePromise: DeferredResolve | undefined
86-
let rejectPromise: DeferredReject | undefined
87-
88-
const promise = new Promise<void>((resolve, reject) => {
89-
resolvePromise = resolve
90-
rejectPromise = reject
112+
export function startCallbackServer(path: string): CallbackServer {
113+
let _resolve!: () => void
114+
let _reject!: (err: Error) => void
115+
// The promise that resolves when the real browser hits `path`.
116+
// No internal polling — this is purely event-driven.
117+
const _promise = new Promise<void>((res, rej) => {
118+
_resolve = res
119+
_reject = rej
91120
})
121+
// Prevent Node from crashing if nobody awaits the promise before the
122+
// server is closed externally (e.g. on SIGINT).
123+
_promise.catch(() => {})
92124

93-
// Prevent unhandled rejection when the server is closed before callback
94-
promise.catch(() => {})
95-
96-
let currentPortIndex = 0
97-
98-
function tryListen(server: http.Server) {
99-
const port = PORTS[currentPortIndex]
100-
server.listen(port, "127.0.0.1", () => {
101-
// resolved on first successful listen
102-
})
103-
}
125+
let resolved = false
104126

105127
const server = http.createServer((req, res) => {
106-
if (closed) return
107-
108-
const url = new URL(req.url ?? "/", `http://127.0.0.1:${server.address()?.port ?? 29734}`)
128+
const url = new URL(req.url ?? "/", `http://127.0.0.1:${currentPort()}`)
109129

110-
if (url.pathname === "/github-install-callback") {
130+
if (url.pathname === path) {
111131
res.writeHead(200, {
112132
"Content-Type": "text/html; charset=utf-8",
113133
"Cache-Control": "no-store",
114134
})
115135
res.end(HTML_SUCCESS)
116136

117-
if (!closed) {
118-
closed = true
119-
resolvePromise?.()
120-
// Give the browser a moment to render before closing
121-
setTimeout(() => server.close(), 500)
137+
if (!resolved) {
138+
resolved = true
139+
// Give the browser 500ms to render before closing the server
140+
setTimeout(() => {
141+
server.close()
142+
_resolve()
143+
}, 500)
122144
}
123145
return
124146
}
125147

126-
// Redirect unknown paths to the GitHub App install page
127-
res.writeHead(302, { Location: "https://github.com/apps/opencode-agent" })
128-
res.end()
148+
// Any other path → 404
149+
res.writeHead(404)
150+
res.end("Not found")
129151
})
130152

131-
// Try ports in sequence
132-
tryListen(server)
153+
// Port 0 lets the OS pick a free port — no collision risk
154+
server.listen(0, "127.0.0.1")
133155

134-
// Expose close on server
135-
server.on("error", (err: NodeJS.ErrnoException) => {
136-
if (err.code === "EADDRINUSE" && currentPortIndex < PORTS.length - 1) {
137-
currentPortIndex++
138-
tryListen(server)
139-
} else {
140-
rejectPromise?.(err)
141-
}
156+
server.on("error", (err) => {
157+
_reject(err)
142158
})
143159

160+
function currentPort(): number {
161+
const addr = server.address()
162+
return typeof addr === "object" && addr !== null ? addr.port : 0
163+
}
164+
144165
return {
145166
get port() {
146-
const addr = server.address()
147-
return typeof addr === "object" && addr !== null ? addr.port : PORTS[currentPortIndex]
167+
return currentPort()
168+
},
169+
get promise() {
170+
return _promise
148171
},
149172
close() {
150-
closed = true
173+
if (!resolved) {
174+
resolved = true
175+
_reject(new Error("Server closed before callback was received"))
176+
}
151177
server.close()
152178
},
153-
} as CallbackServer
179+
}
154180
}
155181

156182
/**
157-
* Returns a promise that resolves when the browser hits the configured path,
158-
* or rejects if the timeout is reached first.
183+
* Returns a promise that resolves when the browser hits the callback server,
184+
* or rejects after `timeoutMs` (default: 5 minutes).
185+
*
186+
* No polling. Races `server.promise` against a timeout.
159187
*/
160-
export function waitForCallback(server: CallbackServer, opts: CallbackOptions): Promise<void> {
161-
const { path, timeoutMs = 300_000 } = opts
162-
let timeout: NodeJS.Timeout
163-
164-
return new Promise<void>((resolve, reject) => {
165-
timeout = setTimeout(() => {
166-
server.close()
167-
reject(new Error(`Callback timeout after ${timeoutMs}ms`))
168-
}, timeoutMs)
169-
170-
// Poll the server port until it closes (meaning callback fired)
171-
const checkInterval = setInterval(() => {
172-
const req = http.get(
173-
`http://127.0.0.1:${server.port}${path}`,
174-
(res) => {
175-
clearInterval(checkInterval)
176-
clearTimeout(timeout)
177-
resolve()
178-
},
179-
)
180-
req.on("error", () => {
181-
// Server not ready yet, keep waiting
182-
})
183-
}, 500)
184-
185-
// Also resolve if server closes on its own (callback fired)
186-
const originalClose = server.close.bind(server)
187-
server.close = () => {
188-
clearInterval(checkInterval)
189-
clearTimeout(timeout)
190-
resolve()
191-
originalClose()
192-
}
193-
})
188+
export function waitForCallback(
189+
server: CallbackServer,
190+
opts: { timeoutMs?: number } = {},
191+
): Promise<void> {
192+
const timeoutMs = opts.timeoutMs ?? 300_000
193+
return Promise.race([
194+
server.promise,
195+
new Promise<void>((_, reject) =>
196+
setTimeout(
197+
() => reject(new Error(`GitHub app authorization timed out after ${timeoutMs / 1000}s`)),
198+
timeoutMs,
199+
),
200+
),
201+
])
194202
}

0 commit comments

Comments
 (0)