diff --git a/.github/workflows/cli-release-build.yml b/.github/workflows/cli-release-build.yml index 741b32bbd7..ed0c7a5be7 100644 --- a/.github/workflows/cli-release-build.yml +++ b/.github/workflows/cli-release-build.yml @@ -184,6 +184,7 @@ jobs: # Fast path: --version exits synchronously through commander, so it # only catches early sync failures. Run it for parity with old CI. "$BIN" --version + "$BIN" --smoke-login-primitives # Slow path: keep the binary alive long enough for *async* startup # failures (e.g. the Parser.init rejection that crashed the @@ -338,6 +339,7 @@ jobs: # Sync check — exits via commander before async tasks fire. "$BIN" --version + "$BIN" --smoke-login-primitives # Long-running check — gives async startup failures time to surface. # This is the step that would have caught the post-OpenTUI-upgrade diff --git a/.github/workflows/freebuff-e2e.yml b/.github/workflows/freebuff-e2e.yml index f1fc8afbba..d238263bd6 100644 --- a/.github/workflows/freebuff-e2e.yml +++ b/.github/workflows/freebuff-e2e.yml @@ -44,6 +44,7 @@ jobs: # startup failures (e.g. the Parser.init rejection from a broken # tree-sitter wasm load). cli/bin/freebuff --version + cli/bin/freebuff --smoke-login-primitives # Run for a few seconds so unhandled rejections during module init # have a chance to fire and trip earlyFatalHandler. bun cli/scripts/smoke-binary.ts cli/bin/freebuff @@ -178,6 +179,7 @@ jobs: # startup failures (e.g. the Parser.init rejection from a broken # tree-sitter wasm load). ./cli/bin/freebuff.exe --version + ./cli/bin/freebuff.exe --smoke-login-primitives # Run for several seconds so unhandled rejections during module # init have time to fire — the freebuff 0.0.62 wasm regression # surfaced through the *late* renderer-cleanup handler, after the diff --git a/cli/scripts/smoke-binary.ts b/cli/scripts/smoke-binary.ts index 2553c87ef2..268e3d3c77 100644 --- a/cli/scripts/smoke-binary.ts +++ b/cli/scripts/smoke-binary.ts @@ -8,8 +8,13 @@ * binary, lets it run for a few seconds, then kills it and asserts the TUI * actually rendered a known boot screen. * - * The positive check matters more than the negative one: a "did the boot - * screen appear" assertion catches *any* startup failure — known fatals, + * Before the long-running UI check, this also invokes explicit compiled + * runtime smoke flags in the binary. Those cover startup-sensitive assets + * (tree-sitter) and non-UI runtime integrations (network, subprocesses, + * vendored native tools, filesystem IO). + * + * The positive boot check matters more than the negative one: a "did the + * boot screen appear" assertion catches *any* startup failure — known fatals, * novel error messages, silent crashes, hangs, segfaults that produce no * output. Negative pattern matches are kept only for clearer diagnostics * when a known regression recurs. @@ -81,9 +86,21 @@ const FATAL_PATTERNS = [ // the renderer is up). const DEFAULT_RUN_SECONDS = 10 -function runTreeSitterSmoke(binary: string): Promise { +function runFlagSmoke({ + binary, + flag, + label, + okPattern, + timeoutMs, +}: { + binary: string + flag: string + label: string + okPattern: RegExp + timeoutMs: number +}): Promise { return new Promise((resolve, reject) => { - const proc = spawn(binary, ['--smoke-tree-sitter'], { + const proc = spawn(binary, [flag], { stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, NO_COLOR: '1', TERM: 'dumb' }, }) @@ -95,16 +112,36 @@ function runTreeSitterSmoke(binary: string): Promise { proc.stdout?.on('data', append) proc.stderr?.on('data', append) + let timedOut = false + const timeout = setTimeout(() => { + timedOut = true + proc.kill('SIGKILL') + }, timeoutMs) + proc.once('error', reject) proc.once('exit', (code) => { - if (code === 0 && /tree-sitter smoke ok/.test(captured)) { + clearTimeout(timeout) + + if (timedOut) { + reject( + new Error( + `${label} smoke timed out after ${timeoutMs}ms\n${captured.slice( + 0, + 8 * 1024, + )}`, + ), + ) + return + } + + if (code === 0 && okPattern.test(captured)) { resolve() return } reject( new Error( - `tree-sitter smoke failed with exit code ${code}\n${captured.slice( + `${label} smoke failed with exit code ${code}\n${captured.slice( 0, 8 * 1024, )}`, @@ -133,9 +170,24 @@ async function main(): Promise { console.log(`smoke-binary: spawning ${binary} for ${runSeconds}s…`) - await runTreeSitterSmoke(binary) + await runFlagSmoke({ + binary, + flag: '--smoke-tree-sitter', + label: 'tree-sitter', + okPattern: /tree-sitter smoke ok/, + timeoutMs: 30_000, + }) console.log('smoke-binary: tree-sitter init OK.') + await runFlagSmoke({ + binary, + flag: '--smoke-runtime-primitives', + label: 'runtime primitives', + okPattern: /runtime primitives smoke ok/, + timeoutMs: 90_000, + }) + console.log('smoke-binary: runtime primitives OK.') + const proc = spawn(binary, [], { stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, NO_COLOR: '1', TERM: 'dumb' }, diff --git a/cli/src/index.tsx b/cli/src/index.tsx index 302ccaeac6..d98bf01fac 100644 --- a/cli/src/index.tsx +++ b/cli/src/index.tsx @@ -262,6 +262,82 @@ async function main(): Promise { } } + // CI gate: ` --smoke-login-primitives` checks the local pieces that + // must work before browser OAuth can complete. This is intentionally not a + // full OAuth flow: CI should not depend on a real GitHub account/browser + // round trip just to validate the compiled Windows executable. + if (process.argv.includes('--smoke-login-primitives')) { + try { + const [{ withTimeout }, fingerprint, { getWindowsOpenUrlCommand }] = + await Promise.all([ + import('@codebuff/common/util/promise'), + import('./utils/fingerprint'), + import('./utils/open-url'), + ]) + + let timeoutRejected = false + try { + await withTimeout( + new Promise(() => {}), + 50, + 'login smoke expected timeout', + ) + } catch (err) { + timeoutRejected = + err instanceof Error && + err.message.includes('login smoke expected timeout') + } + if (!timeoutRejected) { + throw new Error('withTimeout did not reject a hanging promise') + } + + const fingerprintId = await withTimeout( + fingerprint.calculateFingerprint(), + 5_000, + 'calculateFingerprint exceeded login smoke timeout', + ) + const fingerprintType = fingerprint.getFingerprintType(fingerprintId) + if (fingerprintType === 'unknown') { + throw new Error(`Unexpected fingerprint type for ${fingerprintId}`) + } + + if (process.platform === 'win32') { + const opener = getWindowsOpenUrlCommand('https://example.com') + if ( + opener.command !== 'rundll32.exe' || + opener.args[0] !== 'url.dll,FileProtocolHandler' || + opener.args[1] !== 'https://example.com' + ) { + throw new Error( + `Unexpected Windows URL opener: ${opener.command} ${opener.args.join(' ')}`, + ) + } + } + + console.log(`login primitives smoke ok (${fingerprintType})`) + process.exit(0) + } catch (err) { + console.error('login primitives smoke FAIL:', err) + process.exit(1) + } + } + + // CI gate: ` --smoke-runtime-primitives` exercises the highest-risk + // compiled runtime integrations without entering the TUI: disk IO, vendored + // native tools, subprocesses, and the real login-code HTTP endpoint. + if (process.argv.includes('--smoke-runtime-primitives')) { + try { + const { runRuntimePrimitivesSmoke } = await import( + './smoke/runtime-primitives' + ) + await runRuntimePrimitivesSmoke() + process.exit(0) + } catch (err) { + console.error('runtime primitives smoke FAIL:', err) + process.exit(1) + } + } + // Run OSC theme detection BEFORE anything else. // This MUST happen before OpenTUI starts because OSC responses come through stdin, // and OpenTUI also listens to stdin. Running detection here ensures stdin is clean. diff --git a/cli/src/smoke/runtime-primitives.ts b/cli/src/smoke/runtime-primitives.ts new file mode 100644 index 0000000000..238f2ab1ab --- /dev/null +++ b/cli/src/smoke/runtime-primitives.ts @@ -0,0 +1,288 @@ +import { spawnSync } from 'child_process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'fs' +import { tmpdir } from 'os' +import path from 'path' + +import { withTimeout } from '@codebuff/common/util/promise' + +import { LOGIN_WEBSITE_URL } from '../login/constants' +import { getRgPath } from '../native/ripgrep' +import { createCodebuffApiClient } from '../utils/codebuff-api' +import { + calculateFingerprint, + getFingerprintType, +} from '../utils/fingerprint' +import { logger } from '../utils/logger' + +import { runTerminalCommand } from '../../../sdk/src/tools/run-terminal-command' + +const STEP_TIMEOUT_MS = 30_000 +const NETWORK_TIMEOUT_MS = 45_000 + +type SmokeResult = { + name: string + ms: number +} + +async function runStep( + name: string, + timeoutMs: number, + fn: () => Promise | void, +): Promise { + const started = Date.now() + try { + await withTimeout( + Promise.resolve().then(fn), + timeoutMs, + `${name} timed out after ${timeoutMs}ms`, + ) + } catch (err) { + throw new Error( + `${name} failed: ${err instanceof Error ? err.message : String(err)}`, + { cause: err }, + ) + } + return { name, ms: Date.now() - started } +} + +function assertCondition(condition: unknown, message: string): asserts condition { + if (!condition) { + throw new Error(message) + } +} + +function assertOutputContains(output: string, needle: string, label: string) { + assertCondition( + output.includes(needle), + `${label} output did not contain ${JSON.stringify(needle)}. Output: ${ + output.slice(0, 2048) + }`, + ) +} + +async function smokeFilesystemAndRuntime(): Promise { + const dir = mkdtempSync(path.join(tmpdir(), 'codebuff-runtime-smoke-')) + try { + const nestedFile = path.join(dir, 'folder with spaces', 'payload.json') + const payload = { + marker: 'codebuff-runtime-smoke', + platform: process.platform, + arch: process.arch, + execPath: process.execPath, + argv0: process.argv[0], + } + + mkdirSync(path.dirname(nestedFile), { recursive: true }) + await Bun.write(nestedFile, JSON.stringify(payload, null, 2)) + assertCondition(existsSync(nestedFile), `Expected ${nestedFile} to exist`) + + const parsed = JSON.parse( + readFileSync(nestedFile, 'utf8'), + ) as typeof payload + assertCondition( + parsed.marker === payload.marker, + 'Round-trip JSON marker changed', + ) + assertCondition( + parsed.platform === process.platform, + 'Round-trip platform changed', + ) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +} + +async function smokeLoginNetwork(): Promise { + const fingerprintId = await calculateFingerprint() + const fingerprintType = getFingerprintType(fingerprintId) + assertCondition( + fingerprintType !== 'unknown', + `Unexpected fingerprint type for ${fingerprintId}`, + ) + + const smokeFingerprintId = [ + fingerprintId, + 'runtime-smoke', + process.platform, + process.arch, + Date.now().toString(36), + ].join('-') + + const apiClient = createCodebuffApiClient({ + baseUrl: LOGIN_WEBSITE_URL, + defaultTimeoutMs: 15_000, + retry: { + maxRetries: 1, + initialDelayMs: 500, + maxDelayMs: 1_500, + }, + }) + + const loginCode = await apiClient.loginCode({ + fingerprintId: smokeFingerprintId, + }) + if (!loginCode.ok) { + throw new Error( + `loginCode returned ${loginCode.status}: ${loginCode.error ?? ''}`, + ) + } + assertCondition(loginCode.data, 'loginCode returned no data') + + const { loginUrl, fingerprintHash, expiresAt } = loginCode.data + assertCondition(typeof loginUrl === 'string', 'loginUrl was not a string') + assertCondition( + typeof fingerprintHash === 'string', + 'fingerprintHash was not a string', + ) + + const parsedUrl = new URL(loginUrl) + assertCondition( + parsedUrl.protocol === 'https:' || parsedUrl.hostname === 'localhost', + `Unexpected loginUrl protocol/host: ${loginUrl}`, + ) + assertCondition( + parsedUrl.searchParams.has('auth_code'), + `loginUrl missing auth_code query param: ${loginUrl}`, + ) + + const expiresAtMs = Number(expiresAt) + assertCondition( + Number.isFinite(expiresAtMs), + `expiresAt was not numeric: ${expiresAt}`, + ) + assertCondition( + expiresAtMs > Date.now(), + `expiresAt was not in the future: ${expiresAt}`, + ) + + const status = await apiClient.loginStatus({ + fingerprintId: smokeFingerprintId, + fingerprintHash, + expiresAt: String(expiresAt), + }) + assertCondition( + status.status === 401 || status.ok, + `loginStatus returned unexpected ${status.status}: ${ + status.ok ? '' : status.error ?? '' + }`, + ) +} + +async function smokeRipgrep(): Promise { + const dir = mkdtempSync(path.join(tmpdir(), 'codebuff-rg-smoke-')) + try { + const marker = `CODEBUFF_RG_SMOKE_${Date.now().toString(36)}` + const fixturePath = path.join(dir, 'fixture.txt') + writeFileSync(fixturePath, `before\n${marker}\nafter\n`) + + const rgPath = await getRgPath() + assertCondition(existsSync(rgPath), `rg path does not exist: ${rgPath}`) + + const version = spawnSync(rgPath, ['--version'], { + encoding: 'utf8', + timeout: 10_000, + }) + assertCondition( + version.status === 0, + `rg --version failed (${version.status}): ${version.stderr || version.stdout}`, + ) + assertOutputContains(version.stdout, 'ripgrep', 'rg --version') + + const search = spawnSync( + rgPath, + ['--no-config', '-n', '--json', '--', marker, dir], + { + encoding: 'utf8', + timeout: 10_000, + }, + ) + assertCondition( + search.status === 0, + `rg marker search failed (${search.status}): ${search.stderr || search.stdout}`, + ) + assertOutputContains(search.stdout, marker, 'rg marker search') + } finally { + rmSync(dir, { recursive: true, force: true }) + } +} + +async function smokeSubprocesses(): Promise { + const result = await runTerminalCommand({ + command: 'printf codebuff-smoke-bash', + process_type: 'SYNC', + cwd: process.cwd(), + timeout_seconds: 10, + }) + const payload = result[0]?.type === 'json' ? result[0].value : null + assertCondition( + payload && typeof payload === 'object', + 'runTerminalCommand returned no JSON payload', + ) + + const stdout = String((payload as { stdout?: unknown }).stdout ?? '') + const exitCode = (payload as { exitCode?: unknown }).exitCode + assertCondition( + exitCode === 0, + `runTerminalCommand exit code was ${String(exitCode)}`, + ) + assertOutputContains(stdout, 'codebuff-smoke-bash', 'runTerminalCommand') + + if (process.platform === 'win32') { + const powershell = spawnSync( + 'powershell', + ['-NoProfile', '-Command', 'Write-Output codebuff-smoke-powershell'], + { + encoding: 'utf8', + timeout: 10_000, + windowsHide: true, + }, + ) + assertCondition( + powershell.status === 0, + `powershell smoke failed (${powershell.status}): ${ + powershell.stderr || powershell.stdout + }`, + ) + assertOutputContains( + powershell.stdout, + 'codebuff-smoke-powershell', + 'powershell', + ) + } +} + +export async function runRuntimePrimitivesSmoke(): Promise { + const results: SmokeResult[] = [] + + results.push( + await runStep( + 'filesystem/runtime', + STEP_TIMEOUT_MS, + smokeFilesystemAndRuntime, + ), + ) + results.push( + await runStep('ripgrep extraction/search', STEP_TIMEOUT_MS, smokeRipgrep), + ) + results.push(await runStep('subprocesses', STEP_TIMEOUT_MS, smokeSubprocesses)) + results.push( + await runStep('login network', NETWORK_TIMEOUT_MS, smokeLoginNetwork), + ) + + logger.info( + { results, baseUrl: LOGIN_WEBSITE_URL }, + 'Runtime primitives smoke completed', + ) + console.log( + `runtime primitives smoke ok (${results + .map((result) => `${result.name}: ${result.ms}ms`) + .join(', ')})`, + ) +} diff --git a/cli/src/utils/__tests__/fingerprint.test.ts b/cli/src/utils/__tests__/fingerprint.test.ts index 12d71ddfda..3d8a0ed20e 100644 --- a/cli/src/utils/__tests__/fingerprint.test.ts +++ b/cli/src/utils/__tests__/fingerprint.test.ts @@ -1,6 +1,10 @@ -import { describe, test, expect } from 'bun:test' +import { describe, test, expect, spyOn } from 'bun:test' -import { getFingerprintType, generateFingerprintIdSync } from '../fingerprint' +import { + getFingerprintType, + generateFingerprintIdSync, + generateLegacyFingerprintSuffix, +} from '../fingerprint' describe('fingerprint utilities', () => { describe('getFingerprintType', () => { @@ -141,4 +145,43 @@ describe('fingerprint utilities', () => { }) }) }) + + describe('generateLegacyFingerprintSuffix', () => { + test('falls back to Web Crypto when node randomBytes fails', () => { + const suffix = generateLegacyFingerprintSuffix( + () => { + throw new Error('randomBytes unavailable') + }, + { + getRandomValues: (bytes) => { + bytes.set([0, 1, 2, 3, 4, 5]) + return bytes + }, + }, + ) + + expect(suffix).toBe('AAECAwQF') + }) + + test('falls back to Math.random when node and Web Crypto both fail', () => { + const mathRandomSpy = spyOn(Math, 'random').mockReturnValue(0) + + try { + const suffix = generateLegacyFingerprintSuffix( + () => { + throw new Error('randomBytes unavailable') + }, + { + getRandomValues: () => { + throw new Error('getRandomValues unavailable') + }, + }, + ) + + expect(suffix).toBe('AAAAAAAA') + } finally { + mathRandomSpy.mockRestore() + } + }) + }) }) diff --git a/cli/src/utils/__tests__/open-url.test.ts b/cli/src/utils/__tests__/open-url.test.ts new file mode 100644 index 0000000000..e58e4d8151 --- /dev/null +++ b/cli/src/utils/__tests__/open-url.test.ts @@ -0,0 +1,52 @@ +import { EventEmitter } from 'events' +import type { ChildProcess, spawn } from 'child_process' + +import { describe, expect, mock, test } from 'bun:test' + +import { + getWindowsOpenUrlCommand, + openUrlWithWindowsHandler, +} from '../open-url' + +function createMockChildProcess(): ChildProcess { + const child = new EventEmitter() as ChildProcess + child.unref = mock(() => {}) as unknown as ChildProcess['unref'] + return child +} + +describe('Windows URL opener', () => { + test('builds the rundll32 URL handler command', () => { + expect(getWindowsOpenUrlCommand('https://example.com')).toEqual({ + command: 'rundll32.exe', + args: ['url.dll,FileProtocolHandler', 'https://example.com'], + }) + }) + + test('returns false when spawn emits an async error', async () => { + const child = createMockChildProcess() + const spawnUrlHandler = mock(() => child) as unknown as typeof spawn + + const result = openUrlWithWindowsHandler( + 'https://example.com', + spawnUrlHandler, + ) + child.emit('error', new Error('ENOENT')) + + await expect(result).resolves.toBe(false) + expect(child.unref).not.toHaveBeenCalled() + }) + + test('returns true and unrefs the process after spawn succeeds', async () => { + const child = createMockChildProcess() + const spawnUrlHandler = mock(() => child) as unknown as typeof spawn + + const result = openUrlWithWindowsHandler( + 'https://example.com', + spawnUrlHandler, + ) + child.emit('spawn') + + await expect(result).resolves.toBe(true) + expect(child.unref).toHaveBeenCalledTimes(1) + }) +}) diff --git a/cli/src/utils/fingerprint.ts b/cli/src/utils/fingerprint.ts index 22e974fdda..a1ff3799af 100644 --- a/cli/src/utils/fingerprint.ts +++ b/cli/src/utils/fingerprint.ts @@ -11,6 +11,7 @@ import { createHash, randomBytes } from 'node:crypto' import { cpus, networkInterfaces } from 'node:os' import { AnalyticsEvent } from '@codebuff/common/constants/analytics-events' +import { withTimeout } from '@codebuff/common/util/promise' import { trackEvent } from './analytics' import { detectShell } from './detect-shell' @@ -20,6 +21,37 @@ import { logger } from './logger' let machineIdModule: typeof import('node-machine-id') | null = null let systeminformationModule: typeof import('systeminformation') | null = null +const ENHANCED_FINGERPRINT_TIMEOUT_MS = 3000 +const LEGACY_FINGERPRINT_SUFFIX_LENGTH = 8 +const LEGACY_FINGERPRINT_RANDOM_BYTES = 6 +const BASE64URL_ALPHABET = + 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_' + +type RandomValuesProvider = { + getRandomValues: (bytes: Uint8Array) => Uint8Array +} +type FingerprintLogLevel = 'debug' | 'info' | 'warn' + +function logFingerprint( + level: FingerprintLogLevel, + data: Record, + message: string, +): void { + try { + logger[level](data, message) + } catch { + // Fingerprinting is part of login; logging must not block it. + } +} + +function trackFingerprintGenerated(properties: Record): void { + try { + trackEvent(AnalyticsEvent.FINGERPRINT_GENERATED, properties) + } catch { + // Fingerprinting is part of login; telemetry must not block it. + } +} + async function getMachineId(): Promise { if (!machineIdModule) { machineIdModule = await import('node-machine-id') @@ -133,10 +165,69 @@ async function calculateEnhancedFingerprint(): Promise { * Used as a fallback when enhanced fingerprinting fails. */ function calculateLegacyFingerprint(): string { - const randomSuffix = randomBytes(6).toString('base64url').substring(0, 8) + const randomSuffix = generateLegacyFingerprintSuffix() return `codebuff-cli-${randomSuffix}` } +function encodeLegacyFingerprintSuffix(bytes: Uint8Array): string { + return Buffer.from(bytes) + .toString('base64url') + .substring(0, LEGACY_FINGERPRINT_SUFFIX_LENGTH) +} + +function tryGenerateLegacyFingerprintSuffix( + readBytes: () => Uint8Array, + warning: string, +): string | null { + try { + return encodeLegacyFingerprintSuffix(readBytes()) + } catch (err) { + logFingerprint( + 'warn', + { + errorMessage: err instanceof Error ? err.message : String(err), + }, + warning, + ) + return null + } +} + +function generateMathRandomSuffix(): string { + return Array.from({ length: LEGACY_FINGERPRINT_SUFFIX_LENGTH }, () => { + return BASE64URL_ALPHABET[Math.floor(Math.random() * BASE64URL_ALPHABET.length)] + }).join('') +} + +export function generateLegacyFingerprintSuffix( + randomByteSource: (byteCount: number) => Uint8Array = randomBytes, + randomValuesProvider: RandomValuesProvider | undefined = globalThis.crypto, +): string { + const nodeSuffix = tryGenerateLegacyFingerprintSuffix( + () => randomByteSource(LEGACY_FINGERPRINT_RANDOM_BYTES), + 'Node crypto randomBytes failed for legacy fingerprint suffix', + ) + if (nodeSuffix) { + return nodeSuffix + } + + if (randomValuesProvider) { + const webCryptoSuffix = tryGenerateLegacyFingerprintSuffix( + () => { + const bytes = new Uint8Array(LEGACY_FINGERPRINT_RANDOM_BYTES) + randomValuesProvider.getRandomValues(bytes) + return bytes + }, + 'Web Crypto getRandomValues failed for legacy fingerprint suffix', + ) + if (webCryptoSuffix) { + return webCryptoSuffix + } + } + + return generateMathRandomSuffix() +} + /** * Cached fingerprint promise. Populated on first call and reused for the * process lifetime so every auth step in a session ships the same fingerprint @@ -162,21 +253,27 @@ export function getFingerprintId(): Promise { */ export async function calculateFingerprint(): Promise { try { - const fingerprint = await calculateEnhancedFingerprint() - logger.debug( + const fingerprint = await withTimeout( + calculateEnhancedFingerprint(), + ENHANCED_FINGERPRINT_TIMEOUT_MS, + `Enhanced CLI fingerprinting timed out after ${ENHANCED_FINGERPRINT_TIMEOUT_MS}ms`, + ) + logFingerprint( + 'debug', { fingerprintType: 'enhanced_cli', fingerprintId: fingerprint.substring(0, 20) + '...', }, 'Enhanced CLI fingerprint generated successfully', ) - trackEvent(AnalyticsEvent.FINGERPRINT_GENERATED, { + trackFingerprintGenerated({ fingerprintType: 'enhanced_cli', success: true, }) return fingerprint } catch (enhancedError) { - logger.info( + logFingerprint( + 'info', { errorMessage: enhancedError instanceof Error ? enhancedError.message : String(enhancedError), @@ -185,33 +282,22 @@ export async function calculateFingerprint(): Promise { 'Enhanced CLI fingerprinting failed, using legacy fallback', ) - try { - const fingerprint = calculateLegacyFingerprint() - logger.debug( - { - fingerprintType: 'legacy_fallback', - fingerprintId: fingerprint, - }, - 'Legacy fingerprint generated successfully as fallback', - ) - trackEvent(AnalyticsEvent.FINGERPRINT_GENERATED, { - fingerprintType: 'legacy', - success: true, - fallbackReason: - enhancedError instanceof Error ? enhancedError.message : 'unknown', - }) - return fingerprint - } catch (legacyError) { - logger.error( - { - errorMessage: - legacyError instanceof Error ? legacyError.message : String(legacyError), - fingerprintType: 'failed', - }, - 'Both enhanced and legacy fingerprint generation failed', - ) - throw new Error('Fingerprint generation failed') - } + const fingerprint = calculateLegacyFingerprint() + logFingerprint( + 'debug', + { + fingerprintType: 'legacy_fallback', + fingerprintId: fingerprint, + }, + 'Legacy fingerprint generated successfully as fallback', + ) + trackFingerprintGenerated({ + fingerprintType: 'legacy', + success: true, + fallbackReason: + enhancedError instanceof Error ? enhancedError.message : 'unknown', + }) + return fingerprint } } diff --git a/cli/src/utils/open-url.ts b/cli/src/utils/open-url.ts index 1dffeaac06..708c6098d4 100644 --- a/cli/src/utils/open-url.ts +++ b/cli/src/utils/open-url.ts @@ -1,10 +1,59 @@ import os from 'os' +import { spawn, type ChildProcess } from 'child_process' import open from 'open' import { getCliEnv } from './env' import { logger } from './logger' +export function getWindowsOpenUrlCommand(url: string): { + command: string + args: string[] +} { + return { + command: 'rundll32.exe', + args: ['url.dll,FileProtocolHandler', url], + } +} + +export async function openUrlWithWindowsHandler( + url: string, + spawnUrlHandler: typeof spawn = spawn, +): Promise { + const { command, args } = getWindowsOpenUrlCommand(url) + + return new Promise((resolve) => { + let subprocess: ChildProcess + try { + subprocess = spawnUrlHandler(command, args, { + detached: true, + stdio: 'ignore', + windowsHide: true, + }) + } catch (err) { + logger.error(err, 'Failed to spawn Windows URL handler') + resolve(false) + return + } + + let settled = false + const finish = (success: boolean) => { + if (settled) return + settled = true + resolve(success) + } + + subprocess.once('error', (err) => { + logger.error(err, 'Failed to open browser with Windows URL handler') + finish(false) + }) + subprocess.once('spawn', () => { + subprocess.unref() + finish(true) + }) + }) +} + /** * Safely open a URL in the user's default browser. * @@ -16,6 +65,10 @@ import { logger } from './logger' * @returns `true` if the browser was (likely) opened, `false` if skipped. */ export async function safeOpen(url: string): Promise { + if (os.platform() === 'win32') { + return openUrlWithWindowsHandler(url) + } + if (os.platform() === 'linux') { const env = getCliEnv() const hasDisplay = Boolean(env.DISPLAY || env.WAYLAND_DISPLAY) diff --git a/common/src/util/__tests__/promise.test.ts b/common/src/util/__tests__/promise.test.ts index fac801c7ae..5f45716839 100644 --- a/common/src/util/__tests__/promise.test.ts +++ b/common/src/util/__tests__/promise.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test' -import { INITIAL_RETRY_DELAY, withRetry } from '../promise' +import { INITIAL_RETRY_DELAY, withRetry, withTimeout } from '../promise' describe('withRetry', () => { describe('basic functionality', () => { @@ -323,3 +323,40 @@ describe('withRetry', () => { }) }) }) + +describe('withTimeout', () => { + it('should resolve when the promise finishes before the timeout', async () => { + await expect( + withTimeout(Promise.resolve('ok'), 100, 'too slow'), + ).resolves.toBe('ok') + }) + + it('should reject when the promise exceeds the timeout', async () => { + const neverResolves = new Promise(() => {}) + + await expect(withTimeout(neverResolves, 1, 'too slow')).rejects.toThrow( + 'too slow', + ) + }) + + it('should clear the timeout when the wrapped promise rejects first', async () => { + let clearedTimeout: ReturnType | null = null + const originalClearTimeout = globalThis.clearTimeout + const clearTimeoutSpy = spyOn(globalThis, 'clearTimeout').mockImplementation( + ((timeout: ReturnType) => { + clearedTimeout = timeout + return originalClearTimeout(timeout) + }) as typeof clearTimeout, + ) + + try { + await expect( + withTimeout(Promise.reject(new Error('failed first')), 100, 'too slow'), + ).rejects.toThrow('failed first') + + expect(clearedTimeout).not.toBeNull() + } finally { + clearTimeoutSpy.mockRestore() + } + }) +}) diff --git a/common/src/util/promise.ts b/common/src/util/promise.ts index 5511176b0c..36a5523b72 100644 --- a/common/src/util/promise.ts +++ b/common/src/util/promise.ts @@ -55,7 +55,7 @@ export async function withTimeout( timeoutMs: number, timeoutMessage: string = `Operation timed out after ${timeoutMs}ms`, ): Promise { - let timeoutId: NodeJS.Timeout + let timeoutId: ReturnType | null = null const timeoutPromise = new Promise((_, reject) => { timeoutId = setTimeout(() => { @@ -63,11 +63,9 @@ export async function withTimeout( }, timeoutMs) }) - return Promise.race([ - promise.then((result) => { + return Promise.race([promise, timeoutPromise]).finally(() => { + if (timeoutId) { clearTimeout(timeoutId) - return result - }), - timeoutPromise, - ]) + } + }) }