Skip to content

Commit 33f3200

Browse files
Fix stdio OAuth startup and sandbox flag handling
1 parent 90be4f7 commit 33f3200

6 files changed

Lines changed: 71 additions & 42 deletions

File tree

benchmarks/core-runtime.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ async function runBenchmark() {
136136

137137
try {
138138
const totalMs = await runBenchmark()
139-
console.log(`METRIC total_ms=${totalMs}`)
139+
globalThis.console.log(`METRIC total_ms=${totalMs}`)
140140
} finally {
141141
await setSandboxDirectory(null)
142142
await fs.rm(fixtureRoot, { recursive: true, force: true })

src/auth/nutrient-oauth.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ type CachedCredentials = z.infer<typeof CachedCredentialsSchema>
4040

4141
const FETCH_TIMEOUT_MS = 15_000
4242
const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000
43+
const pendingTokenRequests = new Map<string, Promise<string>>()
4344

4445
export function getDefaultCredentialsPath(
4546
env: NodeJS.ProcessEnv = process.env,
@@ -367,7 +368,24 @@ export async function invalidateCachedToken(config: NutrientOAuthConfig): Promis
367368
export async function getToken(config: NutrientOAuthConfig): Promise<string> {
368369
const credentialsPath = config.credentialsPath ?? getDefaultCredentialsPath()
369370

370-
logger.debug('getToken called', { credentialsPath })
371+
const pendingRequest = pendingTokenRequests.get(credentialsPath)
372+
if (pendingRequest) {
373+
logger.debug('Awaiting in-flight token acquisition', { credentialsPath })
374+
return pendingRequest
375+
}
376+
377+
const tokenRequest = getTokenUncached(config, credentialsPath).finally(() => {
378+
if (pendingTokenRequests.get(credentialsPath) === tokenRequest) {
379+
pendingTokenRequests.delete(credentialsPath)
380+
}
381+
})
382+
383+
pendingTokenRequests.set(credentialsPath, tokenRequest)
384+
return tokenRequest
385+
}
386+
387+
async function getTokenUncached(config: NutrientOAuthConfig, credentialsPath: string): Promise<string> {
388+
logger.debug('Starting token acquisition', { credentialsPath })
371389

372390
// 1. Check cached token
373391
const cached = await readCachedCredentials(credentialsPath)

src/index.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,6 @@ export async function runServer(environment: Environment): Promise<RunServerResu
240240

241241
const apiClient = createStdioApiClient(environment)
242242

243-
// Authenticate eagerly before accepting tool calls — in stdio mode there's
244-
// no transport-level mechanism to pause while waiting for browser auth
245-
if (!environment.nutrientApiKey) {
246-
logger.info('No API key set, authenticating via OAuth before accepting connections...')
247-
await getToken(buildOAuthConfig(environment))
248-
logger.info('OAuth authentication completed')
249-
}
250-
251243
const server = createMcpServer({
252244
sandboxEnabled,
253245
apiClient,

src/utils/sandbox.ts

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,40 +7,31 @@
77
* @returns The sandbox directory path or undefined if none specified
88
*/
99
export function parseSandboxPath(args: string[], envVar?: string): string | undefined {
10-
const argsLength = args.length
11-
if (argsLength === 2) {
12-
const firstArg = args[0]
13-
if (firstArg === '--sandbox') {
14-
return args[1]
15-
}
16-
if (firstArg === '-s') {
17-
return args[1]
18-
}
19-
} else if (argsLength === 0) {
10+
if (args.length === 0) {
2011
return envVar || undefined
2112
}
2213

23-
const firstArg = args[0]
24-
if (firstArg === '--sandbox' || firstArg === '-s') {
25-
if (argsLength > 1) {
26-
return args[1]
27-
}
28-
29-
throw new Error('--sandbox flag requires a directory path')
30-
}
14+
let sandboxPath: string | undefined
3115

32-
// Check command line arguments first (higher precedence)
33-
for (let i = 1; i < argsLength; i++) {
16+
for (let i = 0; i < args.length; i++) {
3417
const arg = args[i]
18+
3519
if (arg === '--sandbox' || arg === '-s') {
36-
if (i + 1 < argsLength) {
37-
return args[i + 1]
20+
if (i + 1 < args.length) {
21+
sandboxPath = args[i + 1]
22+
i += 1
23+
continue
3824
}
3925

4026
throw new Error('--sandbox flag requires a directory path')
4127
}
28+
29+
if (arg.startsWith('-')) {
30+
throw new Error(`Unknown CLI flag: ${arg}`)
31+
}
32+
33+
throw new Error(`Unexpected argument: ${arg}`)
4234
}
4335

44-
// Fall back to environment variable
45-
return envVar || undefined
36+
return sandboxPath || envVar || undefined
4637
}

tests/nutrient-oauth.test.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,29 @@ describe('getToken integration', () => {
325325
expect(token).toBe('concurrent-new-token')
326326
}
327327

328-
// NOTE: Without dedup, each call makes its own refresh request.
329-
// This test documents the current behavior. If dedup is added,
330-
// change the assertion to: expect(refreshCallCount).toBe(1)
331-
expect(refreshCallCount).toBeGreaterThanOrEqual(1)
328+
expect(refreshCallCount).toBe(1)
329+
})
330+
331+
it('concurrent calls start the browser flow only once when no credentials exist', async () => {
332+
const { getToken } = await import('../src/auth/nutrient-oauth.js')
333+
const config = makeConfig({
334+
tokenUrl: 'http://localhost:1/token',
335+
credentialsPath: join(testDir, 'nonexistent.json'),
336+
clientId: 'fresh-client',
337+
})
338+
339+
const openMock = (await import('open')).default as ReturnType<typeof vi.fn>
340+
openMock.mockClear()
341+
342+
const first = getToken(config)
343+
const second = getToken(config)
344+
345+
await vi.waitFor(() => {
346+
expect(openMock).toHaveBeenCalledOnce()
347+
}, { timeout: 5_000 })
348+
349+
first.catch(() => {})
350+
second.catch(() => {})
332351
})
333352

334353
it('goes straight to browser flow when no refresh token is cached', async () => {

tests/unit.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -766,10 +766,19 @@ describe('API Functions', () => {
766766
expect(() => parseSandboxPath(args, undefined)).toThrow('--sandbox flag requires a directory path')
767767
})
768768

769-
it('should handle multiple arguments and find sandbox flag', () => {
770-
const args = ['--help', '--sandbox', '/path/to/sandbox', '--verbose']
771-
const result = parseSandboxPath(args, undefined)
772-
expect(result).toBe('/path/to/sandbox')
769+
it('should throw on unknown flags', () => {
770+
const args = ['--sandbox-dir', '/path/to/sandbox']
771+
expect(() => parseSandboxPath(args, undefined)).toThrow('Unknown CLI flag: --sandbox-dir')
772+
})
773+
774+
it('should throw on unexpected positional arguments', () => {
775+
const args = ['/path/to/sandbox']
776+
expect(() => parseSandboxPath(args, undefined)).toThrow('Unexpected argument: /path/to/sandbox')
777+
})
778+
779+
it('should throw when known and unknown flags are mixed', () => {
780+
const args = ['--sandbox', '/path/to/sandbox', '--verbose']
781+
expect(() => parseSandboxPath(args, undefined)).toThrow('Unknown CLI flag: --verbose')
773782
})
774783

775784
it('should return undefined when empty env var provided', () => {

0 commit comments

Comments
 (0)