Skip to content

Clearing tokens properly #96

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

Merged
merged 1 commit into from
Jun 6, 2025
Merged

Clearing tokens properly #96

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"open": "^10.1.0"
},
"devDependencies": {
"@modelcontextprotocol/sdk": "^1.12.1",
"@modelcontextprotocol/sdk": "https://pkg.pr.new/geelen/typescript-sdk/@modelcontextprotocol/sdk@cdf3508",
"@types/express": "^5.0.0",
"@types/node": "^22.13.10",
"prettier": "^3.5.3",
Expand Down
11 changes: 6 additions & 5 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 30 additions & 41 deletions src/lib/coordination.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ export type AuthCoordinator = {
export async function isPidRunning(pid: number): Promise<boolean> {
try {
process.kill(pid, 0) // Doesn't kill the process, just checks if it exists
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Process ${pid} is running`)
if (DEBUG) debugLog(`Process ${pid} is running`)
return true
} catch (err) {
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Process ${pid} is not running`, err)
if (DEBUG) debugLog(`Process ${pid} is not running`, err)
return false
}
}
Expand All @@ -32,14 +32,14 @@ export async function isPidRunning(pid: number): Promise<boolean> {
* @returns True if the lockfile is valid, false otherwise
*/
export async function isLockValid(lockData: LockfileData): Promise<boolean> {
if (DEBUG) await debugLog(global.currentServerUrlHash!, 'Checking if lockfile is valid', lockData)
if (DEBUG) debugLog('Checking if lockfile is valid', lockData)

// Check if the lockfile is too old (over 30 minutes)
const MAX_LOCK_AGE = 30 * 60 * 1000 // 30 minutes
if (Date.now() - lockData.timestamp > MAX_LOCK_AGE) {
log('Lockfile is too old')
if (DEBUG)
await debugLog(global.currentServerUrlHash!, 'Lockfile is too old', {
debugLog('Lockfile is too old', {
age: Date.now() - lockData.timestamp,
maxAge: MAX_LOCK_AGE,
})
Expand All @@ -49,13 +49,13 @@ export async function isLockValid(lockData: LockfileData): Promise<boolean> {
// Check if the process is still running
if (!(await isPidRunning(lockData.pid))) {
log('Process from lockfile is not running')
if (DEBUG) await debugLog(global.currentServerUrlHash!, 'Process from lockfile is not running', { pid: lockData.pid })
if (DEBUG) debugLog('Process from lockfile is not running', { pid: lockData.pid })
return false
}

// Check if the endpoint is accessible
try {
if (DEBUG) await debugLog(global.currentServerUrlHash!, 'Checking if endpoint is accessible', { port: lockData.port })
if (DEBUG) debugLog('Checking if endpoint is accessible', { port: lockData.port })

const controller = new AbortController()
const timeout = setTimeout(() => controller.abort(), 1000)
Expand All @@ -67,12 +67,11 @@ export async function isLockValid(lockData: LockfileData): Promise<boolean> {
clearTimeout(timeout)

const isValid = response.status === 200 || response.status === 202
if (DEBUG)
await debugLog(global.currentServerUrlHash!, `Endpoint check result: ${isValid ? 'valid' : 'invalid'}`, { status: response.status })
if (DEBUG) debugLog(`Endpoint check result: ${isValid ? 'valid' : 'invalid'}`, { status: response.status })
return isValid
} catch (error) {
log(`Error connecting to auth server: ${(error as Error).message}`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, 'Error connecting to auth server', error)
if (DEBUG) debugLog('Error connecting to auth server', error)
return false
}
}
Expand All @@ -84,44 +83,41 @@ export async function isLockValid(lockData: LockfileData): Promise<boolean> {
*/
export async function waitForAuthentication(port: number): Promise<boolean> {
log(`Waiting for authentication from the server on port ${port}...`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Waiting for authentication from server on port ${port}`)

try {
let attempts = 0
while (true) {
attempts++
const url = `http://127.0.0.1:${port}/wait-for-auth`
log(`Querying: ${url}`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Poll attempt ${attempts}: ${url}`)
if (DEBUG) debugLog(`Poll attempt ${attempts}`)

try {
const response = await fetch(url)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Poll response status: ${response.status}`)
if (DEBUG) debugLog(`Poll response status: ${response.status}`)

if (response.status === 200) {
// Auth completed, but we don't return the code anymore
log(`Authentication completed by other instance`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Authentication completed by other instance`)
return true
} else if (response.status === 202) {
// Continue polling
log(`Authentication still in progress`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Authentication still in progress, will retry in 1s`)
if (DEBUG) debugLog(`Will retry in 1s`)
await new Promise((resolve) => setTimeout(resolve, 1000))
} else {
log(`Unexpected response status: ${response.status}`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Unexpected response status`, { status: response.status })
return false
}
} catch (fetchError) {
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Fetch error during poll`, fetchError)
if (DEBUG) debugLog(`Fetch error during poll`, fetchError)
// If we can't connect, we'll try again after a delay
await new Promise((resolve) => setTimeout(resolve, 2000))
}
}
} catch (error) {
log(`Error waiting for authentication: ${(error as Error).message}`)
if (DEBUG) await debugLog(global.currentServerUrlHash!, `Error waiting for authentication`, error)
if (DEBUG) debugLog(`Error waiting for authentication`, error)
return false
}
}
Expand All @@ -140,16 +136,16 @@ export function createLazyAuthCoordinator(serverUrlHash: string, callbackPort: n
initializeAuth: async () => {
// If auth has already been initialized, return the existing state
if (authState) {
if (DEBUG) await debugLog(serverUrlHash, 'Auth already initialized, reusing existing state')
if (DEBUG) debugLog('Auth already initialized, reusing existing state')
return authState
}

log('Initializing auth coordination on-demand')
if (DEBUG) await debugLog(serverUrlHash, 'Initializing auth coordination on-demand', { serverUrlHash, callbackPort })
if (DEBUG) debugLog('Initializing auth coordination on-demand', { serverUrlHash, callbackPort })

// Initialize auth using the existing coordinateAuth logic
authState = await coordinateAuth(serverUrlHash, callbackPort, events)
if (DEBUG) await debugLog(serverUrlHash, 'Auth coordination completed', { skipBrowserAuth: authState.skipBrowserAuth })
if (DEBUG) debugLog('Auth coordination completed', { skipBrowserAuth: authState.skipBrowserAuth })
return authState
},
}
Expand All @@ -167,42 +163,39 @@ export async function coordinateAuth(
callbackPort: number,
events: EventEmitter,
): Promise<{ server: Server; waitForAuthCode: () => Promise<string>; skipBrowserAuth: boolean }> {
if (DEBUG) await debugLog(serverUrlHash, 'Coordinating authentication', { serverUrlHash, callbackPort })
if (DEBUG) debugLog('Coordinating authentication', { serverUrlHash, callbackPort })

// Check for a lockfile (disabled on Windows for the time being)
const lockData = process.platform === 'win32' ? null : await checkLockfile(serverUrlHash)

if (DEBUG) {
if (process.platform === 'win32') {
await debugLog(serverUrlHash, 'Skipping lockfile check on Windows')
debugLog('Skipping lockfile check on Windows')
} else {
await debugLog(serverUrlHash, 'Lockfile check result', { found: !!lockData, lockData })
debugLog('Lockfile check result', { found: !!lockData, lockData })
}
}

// If there's a valid lockfile, try to use the existing auth process
if (lockData && (await isLockValid(lockData))) {
log(`Another instance is handling authentication on port ${lockData.port}`)
if (DEBUG) await debugLog(serverUrlHash, 'Another instance is handling authentication', { port: lockData.port, pid: lockData.pid })
log(`Another instance is handling authentication on port ${lockData.port} (pid: ${lockData.pid})`)

try {
// Try to wait for the authentication to complete
if (DEBUG) await debugLog(serverUrlHash, 'Waiting for authentication from other instance')
if (DEBUG) debugLog('Waiting for authentication from other instance')
const authCompleted = await waitForAuthentication(lockData.port)

if (authCompleted) {
log('Authentication completed by another instance')
if (DEBUG) await debugLog(serverUrlHash, 'Authentication completed by another instance, will use tokens from disk')
log('Authentication completed by another instance. Using tokens from disk')

// Setup a dummy server - the client will use tokens directly from disk
const dummyServer = express().listen(0) // Listen on any available port
const dummyPort = (dummyServer.address() as AddressInfo).port
if (DEBUG) await debugLog(serverUrlHash, 'Started dummy server', { port: dummyPort })
if (DEBUG) debugLog('Started dummy server', { port: dummyPort })

// This shouldn't actually be called in normal operation, but provide it for API compatibility
const dummyWaitForAuthCode = () => {
log('WARNING: waitForAuthCode called in secondary instance - this is unexpected')
if (DEBUG) debugLog(serverUrlHash, 'WARNING: waitForAuthCode called in secondary instance - this is unexpected').catch(() => {})
// Return a promise that never resolves - the client should use the tokens from disk instead
return new Promise<string>(() => {})
}
Expand All @@ -214,25 +207,23 @@ export async function coordinateAuth(
}
} else {
log('Taking over authentication process...')
if (DEBUG) await debugLog(serverUrlHash, 'Taking over authentication process')
}
} catch (error) {
log(`Error waiting for authentication: ${error}`)
if (DEBUG) await debugLog(serverUrlHash, 'Error waiting for authentication', error)
if (DEBUG) debugLog('Error waiting for authentication', error)
}

// If we get here, the other process didn't complete auth successfully
if (DEBUG) await debugLog(serverUrlHash, 'Other instance did not complete auth successfully, deleting lockfile')
if (DEBUG) debugLog('Other instance did not complete auth successfully, deleting lockfile')
await deleteLockfile(serverUrlHash)
} else if (lockData) {
// Invalid lockfile, delete it
log('Found invalid lockfile, deleting it')
if (DEBUG) await debugLog(serverUrlHash, 'Found invalid lockfile, deleting it')
await deleteLockfile(serverUrlHash)
}

// Create our own lockfile
if (DEBUG) await debugLog(serverUrlHash, 'Setting up OAuth callback server', { port: callbackPort })
if (DEBUG) debugLog('Setting up OAuth callback server', { port: callbackPort })
const { server, waitForAuthCode, authCompletedPromise } = setupOAuthCallbackServerWithLongPoll({
port: callbackPort,
path: '/oauth/callback',
Expand All @@ -242,21 +233,19 @@ export async function coordinateAuth(
// Get the actual port the server is running on
const address = server.address() as AddressInfo
const actualPort = address.port
if (DEBUG) await debugLog(serverUrlHash, 'OAuth callback server running', { port: actualPort })
if (DEBUG) debugLog('OAuth callback server running', { port: actualPort })

log(`Creating lockfile for server ${serverUrlHash} with process ${process.pid} on port ${actualPort}`)
if (DEBUG) await debugLog(serverUrlHash, 'Creating lockfile', { serverUrlHash, pid: process.pid, port: actualPort })
await createLockfile(serverUrlHash, process.pid, actualPort)

// Make sure lockfile is deleted on process exit
const cleanupHandler = async () => {
try {
log(`Cleaning up lockfile for server ${serverUrlHash}`)
if (DEBUG) await debugLog(serverUrlHash, 'Cleaning up lockfile')
await deleteLockfile(serverUrlHash)
} catch (error) {
log(`Error cleaning up lockfile: ${error}`)
if (DEBUG) await debugLog(serverUrlHash, 'Error cleaning up lockfile', error)
if (DEBUG) debugLog('Error cleaning up lockfile', error)
}
}

Expand All @@ -273,11 +262,11 @@ export async function coordinateAuth(

// Also handle SIGINT separately
process.once('SIGINT', async () => {
if (DEBUG) await debugLog(serverUrlHash, 'Received SIGINT signal, cleaning up')
if (DEBUG) debugLog('Received SIGINT signal, cleaning up')
await cleanupHandler()
})

if (DEBUG) await debugLog(serverUrlHash, 'Auth coordination complete, returning primary instance handlers')
if (DEBUG) debugLog('Auth coordination complete, returning primary instance handlers')
return {
server,
waitForAuthCode,
Expand Down
Loading
Loading