diff --git a/docs/reports/hardening-inventory.json b/docs/reports/hardening-inventory.json index 8053edfe..fc6bec5d 100644 --- a/docs/reports/hardening-inventory.json +++ b/docs/reports/hardening-inventory.json @@ -1,10 +1,10 @@ { "scope": "src/**/*.{ts,tsx,js,jsx,mjs,cjs}", "syncFs": { - "totalOccurrences": 2445, - "filesAffected": 258, - "hotpathOccurrences": 1160, - "hotpathFilesAffected": 152, + "totalOccurrences": 2452, + "filesAffected": 259, + "hotpathOccurrences": 1167, + "hotpathFilesAffected": 153, "topHotpathFiles": [ { "file": "src/management/shared-manager/diverged-file-adopter.ts", @@ -306,8 +306,8 @@ ] }, "legacyShim": { - "totalMarkers": 458, - "filesAffected": 173, + "totalMarkers": 465, + "filesAffected": 176, "topFiles": [ { "file": "src/auth/profile-detector.ts", @@ -497,11 +497,11 @@ }, "maintainability": { "typedErrors": { - "totalThrows": 452, - "typedThrows": 80, + "totalThrows": 454, + "typedThrows": 82, "plainThrows": 312, "otherThrows": 60, - "adoptionRatio": 0.177, + "adoptionRatio": 0.1806, "topSubdomainsByThrows": [ { "subdomain": "web-server", @@ -509,18 +509,18 @@ "typed": 16, "plain": 45 }, + { + "subdomain": "commands", + "count": 38, + "typed": 6, + "plain": 30 + }, { "subdomain": "proxy", "count": 38, "typed": 0, "plain": 34 }, - { - "subdomain": "commands", - "count": 36, - "typed": 4, - "plain": 30 - }, { "subdomain": "codex-auth", "count": 35, @@ -579,8 +579,8 @@ }, "loggerCoverage": { "filesWithCreateLogger": 65, - "totalSourceFiles": 759, - "coverageRatio": 0.0856, + "totalSourceFiles": 761, + "coverageRatio": 0.0854, "subdomainsWithZeroCreateLogger": [ "api", "bin", @@ -606,7 +606,7 @@ }, { "subdomain": "commands", - "count": 108, + "count": 110, "withLogger": 2 }, { @@ -652,8 +652,8 @@ ] }, "hotpathConsoleErrors": { - "totalOccurrences": 571, - "exemptOccurrences": 305, + "totalOccurrences": 592, + "exemptOccurrences": 326, "hotpathOccurrences": 266, "filesAffected": 82, "topFiles": [ @@ -725,7 +725,7 @@ "topOver400": [ { "file": "src/web-server/usage/native-quota-collector.ts", - "loc": 1662 + "loc": 1758 }, { "file": "src/web-server/routes/cliproxy-auth-routes.ts", diff --git a/docs/reports/hardening-inventory.md b/docs/reports/hardening-inventory.md index cce70897..d20fb4ca 100644 --- a/docs/reports/hardening-inventory.md +++ b/docs/reports/hardening-inventory.md @@ -6,12 +6,12 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}` | Metric | Value | |---|---:| -| Sync fs occurrences (all) | 2445 | -| Sync fs files affected (all) | 258 | -| Sync fs occurrences (runtime hotpaths) | 1160 | -| Sync fs files affected (runtime hotpaths) | 152 | -| Legacy shim markers | 458 | -| Legacy shim files affected | 173 | +| Sync fs occurrences (all) | 2452 | +| Sync fs files affected (all) | 259 | +| Sync fs occurrences (runtime hotpaths) | 1167 | +| Sync fs files affected (runtime hotpaths) | 153 | +| Legacy shim markers | 465 | +| Legacy shim files affected | 176 | ## Top Runtime Hotpath Sync fs Files @@ -56,11 +56,11 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}` | Metric | Value | |---|---:| -| typed-error adoption (typed/total throws) | 17.7% (80/452) | +| typed-error adoption (typed/total throws) | 18.1% (82/454) | | typed-error adoption (P4 locked subdomains) | 93.3% (28/30), target 40% | -| hotpath console.error/warn occurrences | 266 (571 total, 305 CLI-UX exempt) | +| hotpath console.error/warn occurrences | 266 (592 total, 326 CLI-UX exempt) | | hotpath console.error/warn files | 82 | -| files with createLogger | 65/759 | +| files with createLogger | 65/761 | | subdomains with zero createLogger | 15 (api, bin, channels, cliproxy, cliproxy/accounts, cliproxy/ai-providers, cliproxy/binary, cliproxy/config, cliproxy/management, cliproxy/sync, cliproxy/types, config, dispatcher, shared, types) | | files > 400 LOC | 91 | | files > 600 LOC | 42 | @@ -89,7 +89,7 @@ Scope: `src/**/*.{ts,tsx,js,jsx,mjs,cjs}` | File | LOC | |---|---:| -| `src/web-server/usage/native-quota-collector.ts` | 1662 | +| `src/web-server/usage/native-quota-collector.ts` | 1758 | | `src/web-server/routes/cliproxy-auth-routes.ts` | 1531 | | `src/cliproxy/auth/oauth-handler.ts` | 1510 | | `src/cursor/cursor-executor.ts` | 1234 | diff --git a/package.json b/package.json index 4bfbfe9c..069c48ec 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@kaitranntt/ccs", - "version": "8.8.1-dev.16", + "version": "8.8.1-dev.19", "description": "Claude Codex Switch - Instant profile switching between Claude, GLM, Kimi, and more", "keywords": [ "cli", diff --git a/src/commands/bar/bar-process-control.ts b/src/commands/bar/bar-process-control.ts new file mode 100644 index 00000000..2a779dd6 --- /dev/null +++ b/src/commands/bar/bar-process-control.ts @@ -0,0 +1,269 @@ +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import { ConfigError } from '../../errors/error-types'; +import { getServerPidPath } from './bar-paths'; + +export interface BarServerProcessRecord { + pid: number; + birthIdentity: string; +} + +export type BarServerStopResult = + | 'stopped' + | 'stale' + | 'legacy-record' + | 'invalid-record' + | 'identity-mismatch' + | 'permission-denied' + | 'signal-failed' + | 'timeout'; + +export interface BarServerStopDeps { + getProcessBirthIdentity: (pid: number) => string | null; + killProcess: (pid: number, signal: 'SIGTERM') => void; + waitForProcessExit: ( + pid: number, + birthIdentity: string, + timeoutMs: number + ) => Promise<'exited' | 'identity-mismatch' | 'timeout'>; +} + +export function parseBarServerProcessRecord(raw: string): BarServerProcessRecord | null { + try { + const parsed = JSON.parse(raw) as Partial; + const pid = parsed.pid; + if (!Number.isSafeInteger(pid) || (pid ?? 0) <= 0) return null; + if (typeof parsed.birthIdentity !== 'string' || parsed.birthIdentity.trim() === '') return null; + return { pid: pid as number, birthIdentity: parsed.birthIdentity }; + } catch { + return null; + } +} + +export function parseLegacyServerPid(raw: string): number | null { + const trimmed = raw.trim(); + if (!/^[1-9]\d*$/.test(trimmed)) return null; + const pid = Number(trimmed); + return Number.isSafeInteger(pid) ? pid : null; +} + +export function serializeBarServerProcessRecord(record: BarServerProcessRecord): string { + return JSON.stringify(record, null, 2); +} + +/** + * Return the OS process birth marker for PID reuse protection. CCS Bar is a + * macOS app, where `ps lstart` is stable for a process lifetime. Linux uses the + * same portable ps surface in development and CI. + */ +export function getProcessBirthIdentity(pid: number): string | null { + if (!Number.isSafeInteger(pid) || pid <= 0) return null; + try { + const output = execFileSync('ps', ['-p', String(pid), '-o', 'lstart='], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + return output || null; + } catch { + return null; + } +} + +export async function waitForProcessExit( + pid: number, + birthIdentity: string, + timeoutMs: number +): Promise<'exited' | 'identity-mismatch' | 'timeout'> { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const currentIdentity = getProcessBirthIdentity(pid); + if (currentIdentity === null) return 'exited'; + if (currentIdentity !== birthIdentity) return 'identity-mismatch'; + await new Promise((resolve) => setTimeout(resolve, 100)); + } + return 'timeout'; +} + +export async function stopRecordedBarServer( + rawRecord: string, + deps: Partial = {} +): Promise<{ result: BarServerStopResult; record: BarServerProcessRecord | null; error?: Error }> { + const record = parseBarServerProcessRecord(rawRecord); + if (record === null) { + return { + result: parseLegacyServerPid(rawRecord) === null ? 'invalid-record' : 'legacy-record', + record: null, + }; + } + + const getIdentity = deps.getProcessBirthIdentity ?? getProcessBirthIdentity; + const killProcess = deps.killProcess ?? ((pid, signal) => process.kill(pid, signal)); + const waitForExit = deps.waitForProcessExit ?? waitForProcessExit; + + // Revalidate immediately before SIGTERM. A PID alone is unsafe because the OS + // can reuse it after the recorded CCS Bar process exits. + const currentIdentity = getIdentity(record.pid); + if (currentIdentity === null) return { result: 'stale', record }; + if (currentIdentity !== record.birthIdentity) return { result: 'identity-mismatch', record }; + + try { + killProcess(record.pid, 'SIGTERM'); + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ESRCH') return { result: 'stale', record }; + if (code === 'EPERM') return { result: 'permission-denied', record, error }; + return { result: 'signal-failed', record, error }; + } + + const waitResult = await waitForExit(record.pid, record.birthIdentity, 3_000); + if (waitResult === 'exited') return { result: 'stopped', record }; + if (waitResult === 'identity-mismatch') return { result: 'identity-mismatch', record }; + return { result: 'timeout', record }; +} + +export interface ClaimedBarServerStopOutcome { + result: BarServerStopResult | 'no-record'; + record: BarServerProcessRecord | null; + legacyPid?: number; + error?: Error; + recoveryPath?: string; +} + +function isErrno(err: unknown, code: string): boolean { + return (err as NodeJS.ErrnoException).code === code; +} + +function unlinkIfPresent(filePath: string): void { + try { + fs.unlinkSync(filePath); + } catch (err) { + if (!isErrno(err, 'ENOENT')) throw err; + } +} + +function restoreClaimWithoutOverwrite( + claimPath: string, + canonicalPath: string +): string | undefined { + try { + fs.linkSync(claimPath, canonicalPath); + unlinkIfPresent(claimPath); + return undefined; + } catch (err) { + if (isErrno(err, 'EEXIST')) return claimPath; + throw err; + } +} + +/** + * Atomically claim server.pid before signaling. The serve process therefore + * sees ENOENT during its signal handler and cannot unlink a new owner's record. + */ +export async function stopBarServerProcessFile( + pidPath: string, + deps: Partial = {} +): Promise { + const claimPath = `${pidPath}.claim-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + try { + fs.renameSync(pidPath, claimPath); + } catch (err) { + if (isErrno(err, 'ENOENT')) return { result: 'no-record', record: null }; + throw err; + } + + let rawRecord: string; + try { + rawRecord = fs.readFileSync(claimPath, 'utf8').trim(); + } catch (err) { + const recoveryPath = restoreClaimWithoutOverwrite(claimPath, pidPath); + return { + result: 'invalid-record', + record: null, + error: err instanceof Error ? err : new Error(String(err)), + recoveryPath, + }; + } + + const outcome = await stopRecordedBarServer(rawRecord, deps); + if (outcome.result === 'stopped' || outcome.result === 'stale') { + unlinkIfPresent(claimPath); + return outcome; + } + + const recoveryPath = restoreClaimWithoutOverwrite(claimPath, pidPath); + return { + ...outcome, + legacyPid: + outcome.result === 'legacy-record' + ? (parseLegacyServerPid(rawRecord) ?? undefined) + : undefined, + recoveryPath, + }; +} + +/** Remove server.pid only when it still belongs to this exact serve process. */ +export function removeBarServerProcessRecordIfOwned( + pidPath: string, + expectedRecord: BarServerProcessRecord +): void { + const claimPath = `${pidPath}.cleanup-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + try { + fs.renameSync(pidPath, claimPath); + } catch (err) { + if (isErrno(err, 'ENOENT')) return; + throw err; + } + + let claimedRecord: BarServerProcessRecord | null = null; + try { + claimedRecord = parseBarServerProcessRecord(fs.readFileSync(claimPath, 'utf8')); + } catch { + // Preserve unreadable state below. + } + + if ( + claimedRecord?.pid === expectedRecord.pid && + claimedRecord.birthIdentity === expectedRecord.birthIdentity + ) { + unlinkIfPresent(claimPath); + return; + } + restoreClaimWithoutOverwrite(claimPath, pidPath); +} + +/** Claim stale discovery state so a replacement write can never be unlinked. */ +export function removeBarDiscoveryIfNoProcess(barJsonPath: string, pidPath: string): boolean { + const claimPath = `${barJsonPath}.cleanup-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + try { + fs.renameSync(barJsonPath, claimPath); + } catch (err) { + if (isErrno(err, 'ENOENT')) return true; + throw err; + } + if (fs.existsSync(pidPath)) { + restoreClaimWithoutOverwrite(claimPath, barJsonPath); + return false; + } + unlinkIfPresent(claimPath); + return true; +} + +export async function stopDetachedBarServer(ccsDir: string): Promise { + const pidPath = getServerPidPath(ccsDir); + try { + const outcome = await stopBarServerProcessFile(pidPath); + if (outcome.result !== 'stopped' && outcome.result !== 'stale') { + throw new ConfigError( + `Safe CCS Bar stop aborted (${outcome.result}); recovery state was preserved`, + outcome.recoveryPath ?? pidPath + ); + } + } catch (err) { + if (err instanceof ConfigError) throw err; + throw new ConfigError( + `Cannot safely stop CCS Bar process: ${err instanceof Error ? err.message : String(err)}`, + pidPath + ); + } +} diff --git a/src/commands/bar/bar-server-probe.ts b/src/commands/bar/bar-server-probe.ts index 496521e6..adc7a627 100644 --- a/src/commands/bar/bar-server-probe.ts +++ b/src/commands/bar/bar-server-probe.ts @@ -25,9 +25,16 @@ export interface DashboardInfo { authRequired?: boolean; } +function isValidPort(value: unknown): value is number { + return Number.isSafeInteger(value) && (value as number) >= 1 && (value as number) <= 65535; +} + /** - * Read the port recorded in an existing bar.json. - * Returns null when the file is absent or malformed. + * Read the port recorded in an existing bar.json, falling back to the --port + * in launch.json's args. `ccs bar stop` deletes bar.json but leaves + * launch.json, so the fallback is what keeps the sticky port (and the probe's + * ability to find a server on a non-default port) across a stop/start cycle. + * Returns null when neither file records a port. */ export function resolveBarPort(ccsDir: string): number | null { @@ -35,10 +42,28 @@ export function resolveBarPort(ccsDir: string): number | null { try { const raw = fs.readFileSync(barJsonPath, 'utf8'); const parsed = JSON.parse(raw) as Partial<{ port: number }>; - return typeof parsed.port === 'number' ? parsed.port : null; + if (isValidPort(parsed.port)) return parsed.port; } catch { - return null; + /* fall through to launch.json */ } + + const launchJsonPath = path.join(ccsDir, 'bar', 'launch.json'); + try { + const raw = fs.readFileSync(launchJsonPath, 'utf8'); + const parsed = JSON.parse(raw) as Partial<{ args: unknown[] }>; + const args = Array.isArray(parsed.args) ? parsed.args : []; + const idx = args.indexOf('--port'); + if (idx !== -1 && idx + 1 < args.length) { + const rawPort = args[idx + 1]; + if (typeof rawPort === 'string' && /^[1-9]\d{0,4}$/.test(rawPort)) { + const n = Number(rawPort); + if (isValidPort(n)) return n; + } + } + } catch { + /* absent or malformed -> null */ + } + return null; } /** @@ -84,18 +109,18 @@ export async function defaultFindRunningServer(ccsDir: string): Promise { [ 'Options:', [ + ['--port ', 'Run the server on this port (persists; later launches keep the same port)'], ['--help, -h', 'Show this help message'], ['--version', 'Show CLI and installed app versions'], ], @@ -44,6 +45,7 @@ export async function showHelp(): Promise { 'Examples:', [ ['ccs bar', 'Start the server detached and open CCS Bar'], + ['ccs bar --port 3999', 'Start (or move) the server on port 3999 instead of 3000'], ['ccs bar stop', 'Stop the detached CCS Bar server'], ['ccs bar status', 'Show server running state and PID'], ['ccs bar install', 'Download and install CCS Bar, then prompt to launch'], diff --git a/src/commands/bar/index.ts b/src/commands/bar/index.ts index f12d8cd4..35cd6360 100644 --- a/src/commands/bar/index.ts +++ b/src/commands/bar/index.ts @@ -10,6 +10,7 @@ */ import { hasAnyFlag } from '../arg-extractor'; +import { validatePortArgs } from './port-arg'; export async function handleBarCommand(args: string[]): Promise { const subcommand = args[0]; @@ -56,9 +57,18 @@ export async function handleBarCommand(args: string[]): Promise { }, }; - // Bare `ccs bar` → launch - if (!subcommand || subcommand === 'launch') { - await commandHandlers.launch(subcommand ? args.slice(1) : []); + // Bare `ccs bar` → launch. Bare flags (e.g. `ccs bar --port 3999`) also go to + // launch with the full arg list preserved (--help/--version were handled above). + if (!subcommand || subcommand === 'launch' || subcommand.startsWith('-')) { + const launchArgs = subcommand === 'launch' ? args.slice(1) : args; + const argError = validatePortArgs(launchArgs); + if (argError !== null) { + console.error(`[X] ${argError}`); + console.error('[i] Usage: ccs bar [--port N]'); + process.exitCode = 1; + return; + } + await commandHandlers.launch(launchArgs); return; } diff --git a/src/commands/bar/launch-descriptor.ts b/src/commands/bar/launch-descriptor.ts index 71e3753e..64d4eefa 100644 --- a/src/commands/bar/launch-descriptor.ts +++ b/src/commands/bar/launch-descriptor.ts @@ -27,6 +27,8 @@ export interface LaunchDescriptorOptions { runtime?: string; home?: string; ccsHome?: string; + /** Server port; recorded in args so the Swift app self-starts on the same port. */ + port?: number; } export function getLaunchShimPath(home: string = os.homedir()): string { @@ -84,7 +86,12 @@ export function createBarLaunchDescriptor(options: LaunchDescriptorOptions = {}) return { schema: LAUNCH_JSON_SCHEMA, runtime: options.runtime ?? process.execPath, - args: [entrypoint, 'bar', 'serve'], + args: [ + entrypoint, + 'bar', + 'serve', + ...(options.port !== undefined ? ['--port', String(options.port)] : []), + ], home, ...(ccsHome ? { ccsHome } : {}), }; diff --git a/src/commands/bar/launch-subcommand.ts b/src/commands/bar/launch-subcommand.ts index 2d1f1897..4952cd39 100644 --- a/src/commands/bar/launch-subcommand.ts +++ b/src/commands/bar/launch-subcommand.ts @@ -7,7 +7,11 @@ * Detached model (replaces the old in-process model): * 1. Probe candidate ports (bar.json port first, then 3000/3001/3002/8000/8080). * 2. If a live server is found → reuse it, write bar.json, open app, return. - * 3. Else → refresh launch.json, getPort to pick a free port, spawn + * With an explicit --port that differs from the running server's port, + * stop that server first and fall through to a fresh start instead. + * 3. Else → pick a port (--port exactly when given; otherwise bar.json's + * recorded port first, then the default candidates), refresh launch.json + * (including --port so the Swift app self-starts on the same port), spawn * `ccs bar serve --port N` detached with stdio → serve.log, poll * /api/bar/summary until 200 (timeout ~10 s), write bar.json, open app, * return. The CLI process exits; the server continues as a detached child. @@ -31,11 +35,13 @@ import { import { getBarDir, getBarJsonPath, getLaunchJsonPath, getServeLogPath } from './bar-paths'; import type { LaunchJson } from './bar-paths'; import { createBarLaunchDescriptor } from './launch-descriptor'; +import { parsePortFlag, validatePortArgs } from './port-arg'; import { defaultFindRunningServer as _defaultFindRunningServer, resolveBarPort as _resolveBarPort, } from './bar-server-probe'; import type { DashboardInfo as _DashboardInfo } from './bar-server-probe'; +import { stopDetachedBarServer } from './bar-process-control'; const BAR_PROBE_TIMEOUT_MS = 1500; const MAX_BAR_PROBE_RESPONSE_BYTES = 8192; @@ -83,10 +89,21 @@ export interface LaunchDeps { * Returns the live baseUrl on success, throws on timeout. */ waitForServerLive: (baseUrl: string) => Promise; + /** + * Build the launch.json descriptor (includes the chosen --port so the Swift + * app self-starts the server on the same port). + */ + createLaunchDescriptor: (opts?: { port?: number }) => LaunchJson; /** * Write launch.json so the Swift app can spawn the server independently. */ writeLaunchDescriptor: (jsonPath: string, descriptor: LaunchJson) => void; + /** + * Stop the detached CCS Bar server recorded in server.pid and wait briefly + * for the port to free. Used when an explicit --port differs from the port + * the running server occupies. + */ + stopDetachedServer: (ccsDir: string) => Promise; /** Open the installed .app bundle. Throws if the app is not found. */ openApp: (appPath: string) => Promise; /** Returns path to ~/.ccs (respects CCS_HOME for test isolation). */ @@ -169,15 +186,11 @@ export async function defaultWaitForServerLive(baseUrl: string): Promise { settled = true; clearTimeout(absoluteDeadline); socket.destroy(); - if (statusCode === 200) { - const echoMatch = headerSection.match( - new RegExp(`${BAR_AUTH_TOKEN_HEADER}:\\s*([^\\r\\n]+)`, 'i') - ); - const proof = echoMatch ? echoMatch[1].trim() : ''; - resolve({ statusCode, tokenMatched: isMatchingBarAuthProof(token, nonce, proof) }); - return; - } - resolve({ statusCode, tokenMatched: false }); + const echoMatch = headerSection.match( + new RegExp(`${BAR_AUTH_TOKEN_HEADER}:\\s*([^\\r\\n]+)`, 'i') + ); + const proof = echoMatch ? echoMatch[1].trim() : ''; + resolve({ statusCode, tokenMatched: isMatchingBarAuthProof(token, nonce, proof) }); }; const socket = net.connect( { host: url.hostname.replace(/^\[|\]$/g, ''), port: Number(url.port) }, @@ -199,7 +212,7 @@ export async function defaultWaitForServerLive(baseUrl: string): Promise { const statusMatch = rawResponse.match(/^HTTP\/\d(?:\.\d)?\s+(\d{3})/); if (statusMatch) { const code = Number(statusMatch[1]); - if (code !== 200) { + if (code !== 200 && code !== 401 && code !== 403) { finish(code, rawResponse); return; } @@ -221,7 +234,7 @@ export async function defaultWaitForServerLive(baseUrl: string): Promise { const { statusCode, tokenMatched } = await probe(); if (statusCode === 200 && tokenMatched) return; - if (statusCode !== null && isAuthRequiredStatus(statusCode)) { + if (statusCode !== null && isAuthRequiredStatus(statusCode) && tokenMatched) { throw new BarServerAuthRequiredError(baseUrl, statusCode); } @@ -258,13 +271,21 @@ export async function handleBarLaunch( _args: string[], deps: Partial = {} ): Promise { + const argError = validatePortArgs(_args); + if (argError !== null) { + console.error(`[X] ${argError}`); + process.exitCode = 1; + return; + } const ccsDir = (deps.getCcsDir ?? defaultGetCcsDir)(); const openApp = deps.openApp ?? defaultOpenApp; const appInstallPath = deps.appInstallPath ?? DEFAULT_APP_INSTALL_PATH; const getPortFn = deps.getPort ?? defaultGetPort; const spawnDetachedServer = deps.spawnDetachedServer ?? defaultSpawnDetachedServer; const waitForServerLive = deps.waitForServerLive ?? defaultWaitForServerLive; + const createLaunchDescriptor = deps.createLaunchDescriptor ?? createBarLaunchDescriptor; const writeLaunchDescriptor = deps.writeLaunchDescriptor ?? defaultWriteLaunchDescriptor; + const stopDetachedServer = deps.stopDetachedServer ?? stopDetachedBarServer; // Wire findRunningServer after ccsDir is resolved. const findRunningServer = deps.findRunningServer ?? (() => _defaultFindRunningServer(ccsDir)); @@ -272,6 +293,16 @@ export async function handleBarLaunch( const barJsonPath = getBarJsonPath(ccsDir); const launchJsonPath = getLaunchJsonPath(ccsDir); + // 0. Parse --port. A present-but-invalid value is a hard error (silently + // launching on a different port than the user asked for is worse). + const portFlag = parsePortFlag(_args); + if (portFlag.present && portFlag.port === null) { + console.error('[X] Invalid --port value. Use a number between 1 and 65535.'); + process.exitCode = 1; + return; + } + const requestedPort = portFlag.port; + // 1. Probe for an already-running server. let running: DashboardInfo | null = null; try { @@ -280,6 +311,9 @@ export async function handleBarLaunch( /* any probe error counts as null */ } + let movingFrom: DashboardInfo | null = null; + let port: number | null = null; + if (running !== null) { if (running.authRequired) { console.error( @@ -291,59 +325,127 @@ export async function handleBarLaunch( return; } - // Reuse the live server — write bar.json and open the app. - const barJson: BarDiscoveryJson = { - baseUrl: running.baseUrl, - port: running.port, - authMode: 'loopback', - }; - try { - fs.mkdirSync(ccsDir, { recursive: true }); - fs.writeFileSync(barJsonPath, JSON.stringify(barJson, null, 2)); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - console.error(`[X] Failed to write bar.json: ${msg}`); + if (requestedPort === null || running.port === requestedPort) { + // Reuse the live server — write bar.json and open the app. + const barJson: BarDiscoveryJson = { + baseUrl: running.baseUrl, + port: running.port, + authMode: 'loopback', + }; + try { + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync(barJsonPath, JSON.stringify(barJson, null, 2)); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[X] Failed to write bar.json: ${msg}`); + return; + } + console.log(`[OK] Reusing running CCS web-server at ${running.baseUrl}`); + console.log(`[i] Discovery file written: ${barJsonPath}`); + await _openAppWithFallback(appInstallPath, openApp); return; } - console.log(`[OK] Reusing running CCS web-server at ${running.baseUrl}`); - console.log(`[i] Discovery file written: ${barJsonPath}`); - await _openAppWithFallback(appInstallPath, openApp); - return; + + // Explicit --port that differs from the running server: preflight the + // destination before disrupting the healthy current service. + console.log( + `[i] CCS Bar server is running on port ${running.port}; moving to port ${requestedPort}...` + ); + if (requestedPort === null) return; + try { + const availablePort = await getPortFn({ port: [requestedPort], host: '127.0.0.1' }); + if (availablePort !== requestedPort) { + console.error(`[X] Port ${requestedPort} is already in use by another process.`); + console.error('[i] The existing CCS Bar server was left running.'); + process.exitCode = 1; + return; + } + port = requestedPort; + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[X] Could not preflight port ${requestedPort}: ${msg}`); + console.error('[i] The existing CCS Bar server was left running.'); + process.exitCode = 1; + return; + } + try { + await stopDetachedServer(ccsDir); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[X] Could not safely stop the running server: ${msg}`); + console.error('[i] Recovery state was preserved; resolve the stop error, then retry.'); + process.exitCode = 1; + return; + } + movingFrom = running; + // Fall through to the fresh-start path below. } - // 2. No live server — pick a port, write/refresh launch.json, spawn detached. + // 2. No live server (or moving ports) — pick a port, write/refresh + // launch.json, spawn detached. - // 2a. Pick a free port. - let port: number; + // 2a. Pick a free port. An explicit --port must be honored exactly; without + // it, the port recorded in bar.json is preferred so the server keeps + // coming back on the port the user last chose (sticky port). try { - port = await getPortFn({ port: [3000, 3001, 3002, 8000, 8080], host: '127.0.0.1' }); + if (port !== null) { + // Destination was already preflighted before stopping the prior server. + } else if (requestedPort !== null) { + const got = await getPortFn({ port: [requestedPort], host: '127.0.0.1' }); + if (got !== requestedPort) { + console.error(`[X] Port ${requestedPort} is already in use by another process.`); + console.error('[i] Choose a different port or free it, then retry.'); + process.exitCode = 1; + return; + } + port = requestedPort; + } else { + const stickyPort = _resolveBarPort(ccsDir); + const base = [3000, 3001, 3002, 8000, 8080]; + const candidates = + stickyPort !== null ? [stickyPort, ...base.filter((p) => p !== stickyPort)] : base; + port = await getPortFn({ port: candidates, host: '127.0.0.1' }); + } } catch (err) { const msg = err instanceof Error ? err.message : String(err); console.error(`[X] Could not find a free port: ${msg}`); return; } - // 2b. Write/refresh launch.json so the Swift app can self-start next time. - try { - const launchDescriptor = createBarLaunchDescriptor(); - writeLaunchDescriptor(launchJsonPath, launchDescriptor); - } catch (err) { - // Non-fatal — the Swift app falls back to resolving `ccs` via PATH. - const msg = err instanceof Error ? err.message : String(err); - console.error(`[!] Could not write launch.json: ${msg}`); + if (port === null) { + console.error('[X] Could not resolve a valid CCS Bar port.'); + process.exitCode = 1; + return; } + const selectedPort = port; - // 2c. Spawn the detached server. + const rollbackPriorServer = async (): Promise => { + if (movingFrom === null) return; + try { + spawnDetachedServer(movingFrom.port, serveLogPath); + await waitForServerLive(movingFrom.baseUrl); + console.log(`[OK] Restored CCS Bar server at ${movingFrom.baseUrl}.`); + } catch (rollbackErr) { + const message = rollbackErr instanceof Error ? rollbackErr.message : String(rollbackErr); + console.error(`[X] Failed to restore CCS Bar at ${movingFrom.baseUrl}: ${message}`); + console.error('[i] Existing discovery and launch state was preserved for manual recovery.'); + } + }; + + // 2b. Spawn the detached server. launch.json is not replaced until the new + // server is proven live, preserving the prior recovery path during a move. const serveLogPath = getServeLogPath(ccsDir); - const baseUrl = `http://127.0.0.1:${port}`; + const baseUrl = `http://127.0.0.1:${selectedPort}`; let spawnedChild: ChildProcess | void; try { fs.mkdirSync(getBarDir(ccsDir), { recursive: true }); - spawnedChild = spawnDetachedServer(port, serveLogPath); + spawnedChild = spawnDetachedServer(selectedPort, serveLogPath); } catch (err) { const msg = err instanceof Error ? err.message : String(err); console.error(`[X] Could not start CCS web-server: ${msg}`); - console.error('[i] Run `ccs config` to start the dashboard manually.'); + await rollbackPriorServer(); + if (movingFrom === null) console.error('[i] Run `ccs config` to start the dashboard manually.'); + process.exitCode = 1; return; } @@ -361,18 +463,30 @@ export async function handleBarLaunch( console.error( '[i] Disable dashboard authentication for CCS Bar or start the dashboard manually.' ); - return; + } else { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[X] Could not connect to CCS web-server: ${msg}`); + console.error(`[i] Check logs at ${serveLogPath}`); } - const msg = err instanceof Error ? err.message : String(err); - console.error(`[X] Could not connect to CCS web-server: ${msg}`); - console.error(`[i] Check logs at ${serveLogPath}`); + spawnedChild?.kill(); + await rollbackPriorServer(); + process.exitCode = 1; return; } + // 2d. Persist the proven-good recovery descriptor. + try { + const launchDescriptor = createLaunchDescriptor({ port: selectedPort }); + writeLaunchDescriptor(launchJsonPath, launchDescriptor); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[!] Could not write launch.json: ${msg}`); + } + // 2e. Write bar.json. const barJson: BarDiscoveryJson = { baseUrl, - port, + port: selectedPort, authMode: 'loopback', }; try { diff --git a/src/commands/bar/port-arg.ts b/src/commands/bar/port-arg.ts new file mode 100644 index 00000000..f52f6443 --- /dev/null +++ b/src/commands/bar/port-arg.ts @@ -0,0 +1,40 @@ +/** + * Shared `--port N` flag parsing for the `ccs bar` command family. + * + * `present` distinguishes "flag not given" from "flag given with a bad value" + * so launch can reject typos loudly instead of silently falling back to the + * default port list. + */ + +export interface PortFlag { + /** True when `--port` appears in args at all. */ + present: boolean; + /** The parsed port (1-65535), or null when absent or invalid. */ + port: number | null; +} + +export function validatePortArgs(args: string[]): string | null { + let foundPort = false; + for (let index = 0; index < args.length; index += 1) { + const arg = args[index]; + if (arg !== '--port') return `Unknown option: ${arg}`; + if (foundPort) return 'Duplicate option: --port'; + foundPort = true; + const raw = args[index + 1]; + if (raw === undefined) return 'Missing value for --port'; + index += 1; + } + return null; +} + +export function parsePortFlag(args: string[]): PortFlag { + const idx = args.indexOf('--port'); + if (idx === -1) return { present: false, port: null }; + const raw = args[idx + 1]; + if (raw === undefined || !/^[1-9]\d{0,4}$/.test(raw)) { + return { present: true, port: null }; + } + const n = Number(raw); + const valid = Number.isSafeInteger(n) && n <= 65535; + return { present: true, port: valid ? n : null }; +} diff --git a/src/commands/bar/serve-subcommand.ts b/src/commands/bar/serve-subcommand.ts index e0769baa..ee47f681 100644 --- a/src/commands/bar/serve-subcommand.ts +++ b/src/commands/bar/serve-subcommand.ts @@ -16,9 +16,16 @@ import * as fs from 'fs'; import * as path from 'path'; import { getCcsDir } from '../../config/config-loader-facade'; import { getBarJsonPath, getServerPidPath } from './bar-paths'; -import { defaultFindRunningServer } from './bar-server-probe'; +import { defaultFindRunningServer, resolveBarPort } from './bar-server-probe'; +import { parsePortFlag, validatePortArgs } from './port-arg'; import type { DashboardInfo } from './bar-server-probe'; import type { BarDiscoveryJson } from './launch-subcommand'; +import { + getProcessBirthIdentity, + removeBarServerProcessRecordIfOwned, + serializeBarServerProcessRecord, +} from './bar-process-control'; +import type { BarServerProcessRecord } from './bar-process-control'; // --------------------------------------------------------------------------- // Types — injectable deps for testability @@ -44,10 +51,14 @@ export interface ServeDeps { writeFile: (filePath: string, content: string) => void; /** Remove a file if it exists (for cleanup on exit). */ removeFile: (filePath: string) => void; + /** Remove server.pid only if it still identifies this process. */ + removeProcessRecordIfOwned: (filePath: string, record: BarServerProcessRecord) => void; /** Register process signal handlers. */ onSignal: (signal: 'SIGINT' | 'SIGTERM', handler: () => void) => void; /** Exit the process. */ exit: (code: number) => never; + /** Read a stable OS birth marker so later stop commands cannot signal a reused PID. */ + getProcessBirthIdentity: (pid: number) => string | null; } // --------------------------------------------------------------------------- @@ -73,14 +84,6 @@ function defaultWriteFile(filePath: string, content: string): void { fs.writeFileSync(filePath, content); } -function defaultRemoveFile(filePath: string): void { - try { - fs.unlinkSync(filePath); - } catch { - /* ignore — file may already be gone */ - } -} - function defaultOnSignal(signal: 'SIGINT' | 'SIGTERM', handler: () => void): void { process.on(signal, handler); } @@ -93,32 +96,27 @@ function defaultGetCcsDir(): string { return getCcsDir(); } -// --------------------------------------------------------------------------- -// Argument parsing helpers -// --------------------------------------------------------------------------- - -function parsePortArg(args: string[]): number | null { - const idx = args.indexOf('--port'); - if (idx !== -1 && idx + 1 < args.length) { - const n = parseInt(args[idx + 1], 10); - return Number.isFinite(n) && n > 0 && n < 65536 ? n : null; - } - return null; -} - // --------------------------------------------------------------------------- // Implementation // --------------------------------------------------------------------------- export async function handleBarServe(args: string[], deps: Partial = {}): Promise { + const argError = validatePortArgs(args); + if (argError !== null) { + console.error(`[X] ${argError}`); + (deps.exit ?? defaultExit)(1); + return; + } const ccsDir = (deps.getCcsDir ?? defaultGetCcsDir)(); const findRunningServer = deps.findRunningServer ?? (() => defaultFindRunningServer(ccsDir)); const startServerFn = deps.startServer ?? defaultStartServer; const getPortFn = deps.getPort ?? defaultGetPort; const writeFile = deps.writeFile ?? defaultWriteFile; - const removeFile = deps.removeFile ?? defaultRemoveFile; + const removeProcessRecordIfOwned = + deps.removeProcessRecordIfOwned ?? removeBarServerProcessRecordIfOwned; const onSignal = deps.onSignal ?? defaultOnSignal; const exit = deps.exit ?? defaultExit; + const readProcessBirthIdentity = deps.getProcessBirthIdentity ?? getProcessBirthIdentity; const barJsonPath = getBarJsonPath(ccsDir); const serverPidPath = getServerPidPath(ccsDir); @@ -151,12 +149,24 @@ export async function handleBarServe(args: string[], deps: Partial = // 2. No live server found — start one. // Honor --port N from the launcher (it pre-selected via getPort to avoid races). - const requestedPort = parsePortArg(args); + // Without it, prefer the port recorded in bar.json so the server keeps coming + // back on the port the user last chose (sticky port). + const portFlag = parsePortFlag(args); + if (portFlag.present && portFlag.port === null) { + console.error('[X] Invalid --port value. Use an integer between 1 and 65535.'); + exit(1); + return; + } + const requestedPort = portFlag.port; let port: number; if (requestedPort !== null) { port = requestedPort; } else { - port = await getPortFn({ port: [3000, 3001, 3002, 8000, 8080], host: '127.0.0.1' }); + const stickyPort = resolveBarPort(ccsDir); + const base = [3000, 3001, 3002, 8000, 8080]; + const candidates = + stickyPort !== null ? [stickyPort, ...base.filter((p) => p !== stickyPort)] : base; + port = await getPortFn({ port: candidates, host: '127.0.0.1' }); } // TypeScript cannot infer that exit(1) is `never` when it is injected as a dep, @@ -171,26 +181,43 @@ export async function handleBarServe(args: string[], deps: Partial = exit(1); } - // 3. Write bar.json and server.pid. + // 3. Publish the owned process record before discovery. Stop cleanup uses + // server.pid as the ownership barrier, so discovery must never appear first. const barJson: BarDiscoveryJson = { baseUrl: dashboardInfo.baseUrl, port: dashboardInfo.port, authMode: 'loopback', }; + const birthIdentity = readProcessBirthIdentity(process.pid); + if (birthIdentity === null) { + console.error('[X] Failed to record CCS Bar process identity; stopping unmanaged server.'); + exit(1); + return; + } + + const processRecord = { pid: process.pid, birthIdentity }; try { - writeFile(barJsonPath, JSON.stringify(barJson, null, 2)); + writeFile(serverPidPath, serializeBarServerProcessRecord(processRecord)); } catch (err) { const msg = err instanceof Error ? err.message : String(err); - console.error(`[X] Failed to write bar.json: ${msg}`); + console.error(`[X] Failed to write server.pid: ${msg}`); exit(1); + return; } try { - writeFile(serverPidPath, String(process.pid)); + writeFile(barJsonPath, JSON.stringify(barJson, null, 2)); } catch (err) { - // Non-fatal — stop/status will just degrade gracefully. + try { + removeProcessRecordIfOwned(serverPidPath, processRecord); + } catch (cleanupErr) { + const cleanupMessage = cleanupErr instanceof Error ? cleanupErr.message : String(cleanupErr); + console.error(`[!] Failed to roll back server.pid: ${cleanupMessage}`); + } const msg = err instanceof Error ? err.message : String(err); - console.error(`[!] Failed to write server.pid: ${msg}`); + console.error(`[X] Failed to write bar.json: ${msg}`); + exit(1); + return; } console.log(`[OK] CCS Bar server started at ${dashboardInfo.baseUrl}`); @@ -198,7 +225,7 @@ export async function handleBarServe(args: string[], deps: Partial = // 4. Clean shutdown on SIGINT / SIGTERM. const shutdown = (): void => { - removeFile(serverPidPath); + removeProcessRecordIfOwned(serverPidPath, processRecord); // bar.json is intentionally left in place on clean shutdown so // the Swift app self-heal poll can detect the server is gone via // the liveness check, not a stale discovery file. diff --git a/src/commands/bar/status-subcommand.ts b/src/commands/bar/status-subcommand.ts index 24e25602..4c21bfc4 100644 --- a/src/commands/bar/status-subcommand.ts +++ b/src/commands/bar/status-subcommand.ts @@ -8,6 +8,7 @@ import * as fs from 'fs'; import { getCcsDir } from '../../config/config-loader-facade'; import { getBarJsonPath, getServerPidPath } from './bar-paths'; +import { parseBarServerProcessRecord, parseLegacyServerPid } from './bar-process-control'; // --------------------------------------------------------------------------- // Types — injectable deps @@ -115,11 +116,21 @@ export async function handleBarStatus( return; } - const pid = parseInt(pidRaw, 10); - if (!Number.isFinite(pid) || pid <= 0) { + const processRecord = parseBarServerProcessRecord(pidRaw); + if (processRecord === null) { + const legacyPid = parseLegacyServerPid(pidRaw); + if (legacyPid !== null) { + console.log( + `[!] CCS Bar server: legacy server.pid has unverified PID ${legacyPid}; status cannot safely identify it.` + ); + console.log(`[i] Verify manually with: ps -p ${legacyPid} -o command=`); + console.log('[i] If it is CCS Bar, stop it manually, remove server.pid, then restart.'); + return; + } console.log(`[!] CCS Bar server: server.pid is invalid ("${pidRaw}")`); return; } + const { pid } = processRecord; // 2. Check process liveness. const alive = isProcessAlive(pid); diff --git a/src/commands/bar/stop-subcommand.ts b/src/commands/bar/stop-subcommand.ts index edca8033..71f558df 100644 --- a/src/commands/bar/stop-subcommand.ts +++ b/src/commands/bar/stop-subcommand.ts @@ -8,6 +8,15 @@ import * as fs from 'fs'; import { getCcsDir } from '../../config/config-loader-facade'; import { getBarJsonPath, getServerPidPath } from './bar-paths'; +import { + getProcessBirthIdentity, + parseLegacyServerPid, + removeBarDiscoveryIfNoProcess, + stopBarServerProcessFile, + stopRecordedBarServer, + waitForProcessExit, +} from './bar-process-control'; +import type { ClaimedBarServerStopOutcome } from './bar-process-control'; // --------------------------------------------------------------------------- // Types — injectable deps @@ -26,6 +35,9 @@ export interface StopDeps { * Throws if the signal cannot be delivered (e.g. ESRCH — no such process). */ killProcess: (pid: number, signal: 'SIGTERM') => void; + getProcessBirthIdentity: (pid: number) => string | null; + waitForProcessExit: typeof waitForProcessExit; + stopProcessFile: (pidPath: string) => Promise; /** Remove a file, ignoring errors if absent. */ removeFile: (filePath: string) => void; } @@ -67,43 +79,92 @@ export async function handleBarStop(_args: string[], deps: Partial = { const readPidFile = deps.readPidFile ?? defaultReadPidFile; const killProcess = deps.killProcess ?? defaultKillProcess; const removeFile = deps.removeFile ?? defaultRemoveFile; + const readProcessBirthIdentity = deps.getProcessBirthIdentity ?? getProcessBirthIdentity; + const waitForExit = deps.waitForProcessExit ?? waitForProcessExit; const pidPath = getServerPidPath(ccsDir); const barJsonPath = getBarJsonPath(ccsDir); - // 1. Read the PID file. - const pidRaw = readPidFile(pidPath); - if (pidRaw === null) { - console.log('[i] CCS Bar server is not running (no server.pid found).'); - return; - } - - const pid = parseInt(pidRaw, 10); - if (!Number.isFinite(pid) || pid <= 0) { - console.error(`[X] server.pid contains an invalid PID: "${pidRaw}"`); - // Clean up the corrupted file so subsequent runs start fresh. - removeFile(pidPath); - return; - } - - // 2. Send SIGTERM. - try { - killProcess(pid, 'SIGTERM'); - console.log(`[OK] Sent SIGTERM to CCS Bar server (PID ${pid}).`); - } catch (err) { - const code = (err as NodeJS.ErrnoException).code; - if (code === 'ESRCH') { - // Process no longer exists — stale PID file, clean up silently. - console.log(`[i] Server PID ${pid} is no longer running. Cleaning up stale files.`); - } else { - const msg = err instanceof Error ? err.message : String(err); - console.error(`[X] Failed to stop server (PID ${pid}): ${msg}`); - // Still remove the pid file so the user is not blocked. + let outcome: ClaimedBarServerStopOutcome; + if (deps.stopProcessFile !== undefined || deps.readPidFile === undefined) { + const stopProcessFile = + deps.stopProcessFile ?? + ((filePath: string) => + stopBarServerProcessFile(filePath, { + getProcessBirthIdentity: readProcessBirthIdentity, + killProcess, + waitForProcessExit: waitForExit, + })); + outcome = await stopProcessFile(pidPath); + if (outcome.result === 'no-record') { + console.log('[i] CCS Bar server is not running (no server.pid found).'); + return; } + } else { + // Injected record reads remain available for isolated unit tests. Production + // always uses the atomic file claim above. + const pidRaw = readPidFile(pidPath); + if (pidRaw === null) { + console.log('[i] CCS Bar server is not running (no server.pid found).'); + return; + } + const recordedOutcome = await stopRecordedBarServer(pidRaw, { + getProcessBirthIdentity: readProcessBirthIdentity, + killProcess, + waitForProcessExit: waitForExit, + }); + outcome = { + ...recordedOutcome, + legacyPid: + recordedOutcome.result === 'legacy-record' + ? (parseLegacyServerPid(pidRaw) ?? undefined) + : undefined, + }; + } + const pid = outcome.record?.pid; + + if (outcome.result === 'stopped' || outcome.result === 'stale') { + if (outcome.result === 'stopped') { + console.log(`[OK] CCS Bar server stopped (PID ${pid}).`); + } else { + console.log(`[i] Server PID ${pid} is no longer running. Cleaning up stale files.`); + } + if (deps.readPidFile !== undefined) { + removeFile(pidPath); + removeFile(barJsonPath); + console.log('[i] Removed server.pid and bar.json.'); + } else { + const removedDiscovery = removeBarDiscoveryIfNoProcess(barJsonPath, pidPath); + if (removedDiscovery) console.log('[i] Removed server.pid and bar.json.'); + else + console.log('[i] A replacement server record appeared; its recovery state was preserved.'); + } + return; } - // 3. Remove pid + bar.json regardless of kill result. - removeFile(pidPath); - removeFile(barJsonPath); - console.log('[i] Removed server.pid and bar.json.'); + if (outcome.result === 'legacy-record') { + const legacyPid = outcome.legacyPid; + console.error( + `[X] Legacy server.pid records PID ${legacyPid} without process identity; no signal was sent.` + ); + console.error(`[i] Verify manually with: ps -p ${legacyPid} -o command=`); + console.error( + `[i] If it is CCS Bar, stop it manually, then remove ${outcome.recoveryPath ?? pidPath} and ${barJsonPath}.` + ); + process.exitCode = 1; + return; + } + + const reason = + outcome.result === 'invalid-record' + ? 'server.pid does not contain a verified process record' + : outcome.result === 'identity-mismatch' + ? `PID ${pid} belongs to a different process` + : outcome.result === 'permission-denied' + ? `permission denied while signaling PID ${pid}` + : outcome.result === 'timeout' + ? `PID ${pid} did not exit within 3 seconds` + : `failed to signal PID ${pid}: ${outcome.error?.message ?? 'unknown error'}`; + console.error(`[X] Refusing to remove CCS Bar recovery state: ${reason}.`); + process.exitCode = 1; } diff --git a/src/proxy/transformers/request-transformer.ts b/src/proxy/transformers/request-transformer.ts index 9083c1da..bde13890 100644 --- a/src/proxy/transformers/request-transformer.ts +++ b/src/proxy/transformers/request-transformer.ts @@ -671,6 +671,52 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { return translatedMessages; } +/** + * Hoist every accepted `role: "system"` message to one leading system message. + * + * Claude Code sends the system prompt as the top-level `system` field *and*, + * separately, sends skill/plugin listings as `role: "system"` entries inside + * `messages` (see #1459). `ProxyRequestTransformer.transform` prepends the + * top-level `system` field unconditionally, so once both are present the + * resulting array holds two `system` messages that are not adjacent — + * `coalesceMessages` only merges *consecutive* same-role messages, so it + * cannot fix this case even if it did coalesce `system` (which it explicitly + * excludes below). + * + * Strict OpenAI-compatible backends (LiteLLM among them) reject any request + * where a `system` message is not alone at index 0: + * `400 A 'system' message can only appear at index 0 of the messages array.` + * + * Inline system messages may appear between complete turns, including after + * tool results. They may not interrupt a pending assistant tool-call/result + * sequence; `transformMessages` rejects that ambiguous placement before this + * pass. Accepted system messages are extracted in encounter order, joined with + * a blank line, and reinserted as the sole leading message. Everything else + * keeps its relative order before normal same-role coalescing. + */ +function hoistSystemMessages(messages: OpenAIMessage[]): OpenAIMessage[] { + const systemParts: string[] = []; + const rest: OpenAIMessage[] = []; + + for (const message of messages) { + if (message.role !== 'system') { + rest.push(message); + continue; + } + const content = message.content; + const text = typeof content === 'string' ? content : ''; + if (text.trim().length > 0) { + systemParts.push(text); + } + } + + if (systemParts.length === 0) { + return rest; + } + + return [{ role: 'system', content: systemParts.join('\n\n') }, ...rest]; +} + /** * Coalesce consecutive messages of the same role. * OpenAI/vLLM/Ollama/Mistral require strict user<->assistant alternation. @@ -741,7 +787,7 @@ export class ProxyRequestTransformer { // was billed. See: // https://platform.openai.com/docs/api-reference/chat-streaming ...(source.stream === true ? { stream_options: { include_usage: true } } : {}), - messages: coalesceMessages(allMessages), + messages: coalesceMessages(hoistSystemMessages(allMessages)), max_tokens: asNumber(source.max_tokens), temperature: asNumber(source.temperature), top_p: asNumber(source.top_p), diff --git a/src/web-server/routes/bar-routes.ts b/src/web-server/routes/bar-routes.ts index f68328fc..2cc99fb0 100644 --- a/src/web-server/routes/bar-routes.ts +++ b/src/web-server/routes/bar-routes.ts @@ -243,6 +243,11 @@ function withTimeout(p: Promise, ms: number): Promise { }); } +/** Remaining milliseconds before one absolute request deadline. */ +function remainingRequestBudget(deadlineAt: number): number { + return Math.max(0, deadlineAt - Date.now()); +} + /** * Map a row to its wire shape. The native-only additions use snake_case parent * keys ("quota_windows" / "stale_as_of") to match the existing payload's mixed @@ -486,6 +491,7 @@ export function createBarRouter(deps: BarRouterDeps): Router { */ router.get('/summary', async (req: Request, res: Response): Promise => { try { + const deadlineAt = Date.now() + REQUEST_DEADLINE_MS; const wantsRefresh = req.query['refresh'] === 'true'; // Determine effective refresh mode after applying debounce. @@ -503,10 +509,20 @@ export function createBarRouter(deps: BarRouterDeps): Router { // else: debounce active — fall through to cache path } + // Start the native side-load immediately. It shares the same absolute + // response deadline as cost and CLIProxy quota work, so their individual + // fallback waits cannot stack into a multi-second tail. + const getNative = deps.getNativeAccountRows ?? (async () => [] as BarSummaryRow[]); + const getCachedNative = deps.getCachedNativeRows ?? (() => [] as BarSummaryRow[]); + const nativePromise = Promise.resolve().then(() => getNative({ force: doForceRefresh })); + // Cost side-load is bounded so a slow usage-snapshot read can't stall the // glance. (Health is per-account, derived from each quota result below — // no blocking system audit on the request path.) - const details = await withTimeout(deps.loadCliproxyDetails(), SIDELOAD_TIMEOUT_MS); + const details = await withTimeout( + deps.loadCliproxyDetails(), + Math.min(SIDELOAD_TIMEOUT_MS, remainingRequestBudget(deadlineAt)) + ); const costByAccount: Record = details ? deps.getTodayCostByAccount(details) : {}; @@ -565,24 +581,20 @@ export function createBarRouter(deps: BarRouterDeps): Router { return rows; })(); - const deadline = new Promise((resolve) => { - setTimeout(() => resolve(cacheRows()), REQUEST_DEADLINE_MS); - }); + const rows = (await withTimeout(gather, remainingRequestBudget(deadlineAt))) ?? cacheRows(); - const rows = await Promise.race([gather, deadline]); - - // Native subscription rows (Claude Code + Codex) are side-loaded AFTER the + // Native subscription rows (Claude Code + Codex) are joined after the // CLIProxy rows resolve, bounded so a slow/failed native fetch degrades // rather than blocking or erroring the response. Pass force so a // debounce-passing refresh also re-pulls native rows live. On timeout fall // back to the last-known cached native rows (NOT []) so a slow forced // re-pull never momentarily drops the Claude/Codex cards; the in-flight // fetch keeps warming the cache for the next poll. - const getNative = deps.getNativeAccountRows ?? (async () => [] as BarSummaryRow[]); - const getCachedNative = deps.getCachedNativeRows ?? (() => [] as BarSummaryRow[]); const nativeRows = - (await withTimeout(getNative({ force: doForceRefresh }), NATIVE_SIDELOAD_TIMEOUT_MS)) ?? - getCachedNative(); + (await withTimeout( + nativePromise, + Math.min(NATIVE_SIDELOAD_TIMEOUT_MS, remainingRequestBudget(deadlineAt)) + )) ?? getCachedNative(); res.json([...rows, ...nativeRows].map(serializeBarRow)); } catch (err) { diff --git a/src/web-server/usage/claude-native-credentials.ts b/src/web-server/usage/claude-native-credentials.ts index c4da718f..c2e84a1a 100644 --- a/src/web-server/usage/claude-native-credentials.ts +++ b/src/web-server/usage/claude-native-credentials.ts @@ -15,7 +15,8 @@ */ import { existsSync, readFileSync } from 'node:fs'; -import { execSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; +import * as crypto from 'node:crypto'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -36,10 +37,20 @@ export interface CredentialReaderDeps { homedir?: string; existsSyncImpl?: (p: string) => boolean; readFileSyncImpl?: (p: string) => string; - execSyncImpl?: (cmd: string, opts: Record) => string | Buffer; + execFileImpl?: ExecFileImpl; + keychainTimeoutMs?: number; } +type ExecFileCallback = (error: Error | null, stdout: string | Buffer) => void; +type ExecFileImpl = ( + file: string, + args: readonly string[], + options: Record, + callback: ExecFileCallback +) => { kill?: () => void } | void; + const KEYCHAIN_SERVICE = 'Claude Code-credentials'; +const SECURITY_PATH = '/usr/bin/security'; const KEYCHAIN_TIMEOUT_MS = 5000; /** Subscription types that mean "no real subscription" -> skip the fetch. */ @@ -67,14 +78,13 @@ function parseCredentials(raw: string): ClaudeNativeCredentials | null { * Keychain as a fallback. Returns null when neither source yields a parseable * object. */ -export function readClaudeCredentials( +export async function readClaudeCredentials( deps: CredentialReaderDeps = {} -): ClaudeNativeCredentials | null { +): Promise { const platform = deps.platform ?? os.platform(); const homedir = deps.homedir ?? os.homedir(); const existsImpl = deps.existsSyncImpl ?? existsSync; const readImpl = deps.readFileSyncImpl ?? ((p: string) => readFileSync(p, 'utf8')); - const execImpl = deps.execSyncImpl ?? execSync; const credentialsPath = path.join(homedir, '.claude', '.credentials.json'); if (existsImpl(credentialsPath)) { @@ -87,20 +97,97 @@ export function readClaudeCredentials( } if (platform === 'darwin') { + const parsed = await readCredentialsFromKeychainService(KEYCHAIN_SERVICE, deps); + if (parsed) return parsed; + } + + return null; +} + +/** Read + parse one Keychain generic-password item. Returns null on any failure. */ +async function readCredentialsFromKeychainService( + service: string, + deps: CredentialReaderDeps +): Promise { + const timeoutMs = deps.keychainTimeoutMs ?? KEYCHAIN_TIMEOUT_MS; + const execImpl: ExecFileImpl = + deps.execFileImpl ?? + ((file, args, options, callback) => + execFile(file, [...args], options, (error, stdout) => callback(error, stdout))); + + return new Promise((resolve) => { + let settled = false; + let child: { kill?: () => void } | void; + const finish = (credentials: ClaudeNativeCredentials | null): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve(credentials); + }; + const timer = setTimeout(() => { + child?.kill?.(); + finish(null); + }, timeoutMs); try { - const out = execImpl(`security find-generic-password -s "${KEYCHAIN_SERVICE}" -w`, { - timeout: KEYCHAIN_TIMEOUT_MS, - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'ignore'], - }); - const raw = (typeof out === 'string' ? out : out.toString('utf8')).trim(); - if (raw) { - const parsed = parseCredentials(raw); - if (parsed) return parsed; - } + child = execImpl( + SECURITY_PATH, + ['find-generic-password', '-s', service, '-w'], + { + timeout: timeoutMs, + encoding: 'utf8', + windowsHide: true, + maxBuffer: 1024 * 1024, + }, + (error, stdout) => { + if (error) return finish(null); + const raw = (typeof stdout === 'string' ? stdout : stdout.toString('utf8')).trim(); + finish(raw ? parseCredentials(raw) : null); + } + ); } catch { - // no Keychain entry / access denied -> null + finish(null); } + }); +} + +/** + * Keychain service name Claude Code uses for a non-default CLAUDE_CONFIG_DIR: + * "Claude Code-credentials-". + */ +export function claudeKeychainServiceForConfigDir(configDir: string): string { + const hash = crypto.createHash('sha256').update(configDir).digest('hex').slice(0, 8); + return `${KEYCHAIN_SERVICE}-${hash}`; +} + +/** + * Read the Claude Code credentials for a specific CLAUDE_CONFIG_DIR (e.g. an + * isolated `ccs auth` instance directory). + * + * File-first (/.credentials.json, no prompt), then the macOS + * Keychain item derived from the config dir path. On macOS Claude Code stores + * OAuth tokens in the Keychain by default, so without the Keychain fallback + * every isolated profile looks permanently logged-out to the bar. + */ +export async function readClaudeCredentialsForConfigDir( + configDir: string, + deps: CredentialReaderDeps = {} +): Promise { + const platform = deps.platform ?? os.platform(); + const existsImpl = deps.existsSyncImpl ?? existsSync; + const readImpl = deps.readFileSyncImpl ?? ((p: string) => readFileSync(p, 'utf8')); + + const credentialsPath = path.join(configDir, '.credentials.json'); + if (existsImpl(credentialsPath)) { + try { + const parsed = parseCredentials(readImpl(credentialsPath)); + if (parsed) return parsed; + } catch { + // fall through to Keychain + } + } + + if (platform === 'darwin') { + return readCredentialsFromKeychainService(claudeKeychainServiceForConfigDir(configDir), deps); } return null; diff --git a/src/web-server/usage/native-quota-collector.ts b/src/web-server/usage/native-quota-collector.ts index ecf942c3..a5b87fb2 100644 --- a/src/web-server/usage/native-quota-collector.ts +++ b/src/web-server/usage/native-quota-collector.ts @@ -14,9 +14,9 @@ * - circuit breaker stops calling after repeated 429s for a cooldown * - serve-stale-on-failure; only omit a row when there is genuinely no data * - * Claude path: reads per-profile .credentials.json (file-only, NO keychain) - * and polls api.anthropic.com/api/oauth/usage. If the file is absent the - * profile is emitted as a parked row (paused:true) — never a keychain call. + * Claude path: reads per-profile .credentials.json, then the per-config-dir + * Keychain item, and polls api.anthropic.com/api/oauth/usage. If neither source + * yields credentials the profile is emitted as a parked row (paused:true). * * Codex path: PRIMARY = live network (chatgpt.com/backend-api/wham/usage, via * fetchCodexQuota), FALLBACK = local session logs (getCodexLocalQuota), mirroring @@ -29,13 +29,15 @@ * is maintained — at most 2 live upstream calls per /summary regardless of * profile count. * - * NO macOS Keychain access anywhere in this module. The old global-default - * Claude reader (readClaudeCredentials) is kept for back-compat but is no longer - * used by the multi-profile path. + * Claude per-profile reads are file-first with a per-config-dir macOS Keychain + * fallback (Claude Code stores OAuth tokens in the Keychain by default on + * macOS). The old global-default Claude reader (readClaudeCredentials) is kept + * for back-compat but is no longer used by the multi-profile path. */ import { readClaudeCredentials, + readClaudeCredentialsForConfigDir, getAccessToken, getSubscriptionTier, hasSupportedSubscription, @@ -108,13 +110,15 @@ const CODEX_PROVIDER = CODEX_NATIVE_PROVIDER; export interface NativeQuotaDeps { /** Read the native Claude Code credentials (global default path). */ - readCredentials?: () => ClaudeNativeCredentials | null; + readCredentials?: () => ClaudeNativeCredentials | null | Promise; /** - * Read credentials for a specific Claude profile (file-only, no keychain). + * Read credentials for a specific Claude profile (file-first, Keychain fallback). * Injected so tests never touch real fs or Keychain. * profile: the profile name (e.g. "work"); returns null when absent/unparseable. */ - readClaudeCredentialsForProfile?: (profile: string) => ClaudeNativeCredentials | null; + readClaudeCredentialsForProfile?: ( + profile: string + ) => ClaudeNativeCredentials | null | Promise; /** Fetch Claude quota with a directly-supplied native token. */ fetchClaudeQuota?: (accessToken: string, accountId?: string) => Promise; /** @@ -487,43 +491,39 @@ function serveCached(state: ProviderState): BarSummaryRow | null { } // ============================================================================ -// File-only Claude credentials reader for per-profile paths (NO keychain) +// Per-profile Claude credentials reader (file-first, Keychain fallback) // ============================================================================ /** - * Read credentials for a specific Claude Code profile (file-only, no keychain). + * Read credentials for a specific Claude Code profile (file-first, Keychain fallback). * - * Looks for .credentials.json in the profile's instance directory. If the file - * is absent or unparseable, returns null — the caller emits a parked row. - * Never calls security/Keychain — zero new keychain access from this feature. + * Looks for .credentials.json in the profile's instance directory, then uses + * the bounded per-config-dir macOS Keychain fallback. If neither yields a + * parseable credential object, the caller emits a parked row. */ -function readClaudeCredentialsForProfileFromDisk( +async function readClaudeCredentialsForProfileFromDisk( profile: string, - readDefaultCredentials: () => ClaudeNativeCredentials | null = readClaudeCredentials -): ClaudeNativeCredentials | null { + readDefaultCredentials: () => + | ClaudeNativeCredentials + | null + | Promise = readClaudeCredentials +): Promise { try { const instanceDir = path.join(getCcsDir(), 'instances', profile); - const credFile = path.join(instanceDir, '.credentials.json'); - if (fs.existsSync(credFile)) { - const raw = fs.readFileSync(credFile, 'utf8'); - const parsed = JSON.parse(raw) as unknown; - if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - return parsed as ClaudeNativeCredentials; - } - return null; - } // The bare `ccs` default login uses the standard global credential lookup: // ~/.claude/.credentials.json, falling back to the single global // "Claude Code-credentials" Keychain item that Claude Code itself maintains. - // This is the ONE pre-existing global read the shipped Bar already performs -- - // NOT a per-profile Keychain scan. Isolated `ccs auth` profiles stay - // file-only and never touch the Keychain; a real instance directory named - // "default" is therefore parked when its file is absent. if (profile === DEFAULT_PROFILE && !fs.existsSync(instanceDir)) { - return readDefaultCredentials(); + return await readDefaultCredentials(); } - return null; + + // Isolated `ccs auth` instance: /.credentials.json first, then + // the per-config-dir Keychain item Claude Code maintains for this + // CLAUDE_CONFIG_DIR ("Claude Code-credentials-"). On + // macOS Claude Code stores tokens in the Keychain by default, so without + // the Keychain read every isolated profile is permanently parked. + return await readClaudeCredentialsForConfigDir(instanceDir); } catch { return null; } @@ -787,6 +787,72 @@ function markDefault(row: BarSummaryRow, isDefault: boolean): BarSummaryRow { return { ...row, is_default: isDefault }; } +/** + * Force paused:true on a non-default row AND its cached copy. Non-default rows + * always render dimmed (only the default profile is "active"), including the + * pass where the rotating live slot refreshed them — otherwise the row would + * flicker active for one poll and dim again on the next. + */ +function markPausedAndSyncCache( + map: Map, + profile: string, + row: BarSummaryRow +): BarSummaryRow { + const state = map.get(profile); + if (state?.cachedRow) { + state.cachedRow = { ...state.cachedRow, paused: true }; + } + return { ...row, paused: true }; +} + +/** + * True when the cached row was fetched BEFORE its own next_reset boundary and + * that boundary has now passed — the row describes the previous quota window, + * so its values (and the reset time itself) are visibly wrong in the bar. + * The cachedAt guard means a post-reset payload that still reports a past + * reset cannot cause a refetch loop: once re-fetched, normal TTL applies. + */ +function isCachedRowStaleByReset(state: ProviderState, now: number): boolean { + const nextReset = state.cachedRow?.next_reset; + if (!nextReset) return false; + const resetMs = Date.parse(nextReset); + if (!Number.isFinite(resetMs)) return false; + return resetMs <= now && state.cachedAt < resetMs; +} + +/** + * Pick the non-default profile the rotating live slot should refresh this pass: + * the stalest profile whose cached row is missing, past its TTL, or past its + * own quota reset, skipping profiles inside a breaker/cooldown window (their + * collector would refuse the fetch anyway, wasting the slot). Returns null + * when every profile is fresh. + */ +function pickRotatingLiveProfile( + map: Map, + profiles: string[], + defaultProfile: string | null, + now: number +): string | null { + let picked: string | null = null; + let pickedAt = Number.POSITIVE_INFINITY; + for (const p of profiles) { + if (p === defaultProfile) continue; + const state = map.get(p); + if (state && (now < state.breakerOpenUntil || now < state.cooldownUntil)) continue; + const cachedRow = state?.cachedRow ?? null; + const cachedAt = state?.cachedAt ?? 0; + if (cachedRow && state) { + const ttl = cachedRow.quotaStatus === 'unsupported' ? PARKED_TTL_MS : NATIVE_QUOTA_TTL_MS; + if (now - cachedAt < ttl && !isCachedRowStaleByReset(state, now)) continue; + } + if (cachedAt < pickedAt) { + pickedAt = cachedAt; + picked = p; + } + } + return picked; +} + /** * Tag the row with is_default AND write the flag back onto the cached copy. The * collector caches a row before the default profile is known (the default is @@ -819,9 +885,13 @@ async function collectClaudeRowForProfile( // Serve from cache while within TTL — force bypasses the short-circuit. Parked // rows (no creds -> quotaStatus 'unsupported') use a short TTL so a fresh login // is picked up within seconds instead of staying dimmed for the full quota TTL. + // A row whose own next_reset has passed is stale regardless of TTL — the + // quota snapped back at the boundary and the cached values are visibly wrong. if (!force && state.cachedRow) { const ttl = state.cachedRow.quotaStatus === 'unsupported' ? PARKED_TTL_MS : NATIVE_QUOTA_TTL_MS; - if (now - state.cachedAt < ttl) return serveCached(state); + if (now - state.cachedAt < ttl && !isCachedRowStaleByReset(state, now)) { + return serveCached(state); + } } // Breaker open or cooldown active -> zero network, serve stale (may be null). @@ -835,7 +905,7 @@ async function collectClaudeRowForProfile( return state.pending; } - // For per-profile reads: use the injected seam (file-only, no keychain). + // For per-profile reads: use the injected seam (file-first, Keychain fallback). const readDefaultCredentials = deps.readCredentials ?? readClaudeCredentials; const readCreds = deps.readClaudeCredentialsForProfile ?? @@ -851,12 +921,12 @@ async function collectClaudeRowForProfile( // pending DURING assignment, leaving a stale resolved promise that the // next call's coalescing check would return instead of re-evaluating. await Promise.resolve(); - const creds = readCreds(profile); + const creds = await readCreds(profile); // No credentials file found -> emit parked row (needs auth, file absent). // This is the expected case when the profile exists in the registry but the - // user has not logged in via 'ccs auth' for this machine or the credentials - // are stored only in keychain (which we deliberately do not access here). + // user has not logged in via 'ccs auth' for this machine or neither the + // profile file nor its bounded macOS Keychain fallback yielded credentials. if (!creds) { const parkedRow = buildParkedClaudeProfileRow(profile, now); // Cache the parked row so repeated calls don't re-stat the fs. @@ -967,9 +1037,12 @@ async function collectCodexRowForProfile( // Serve from cache while within TTL — force bypasses the short-circuit. Parked // rows (no auth -> quotaStatus 'unsupported') use a short TTL so a fresh login // is picked up within seconds instead of staying dimmed for the full quota TTL. + // A row whose own next_reset has passed is stale regardless of TTL. if (!force && state.cachedRow) { const ttl = state.cachedRow.quotaStatus === 'unsupported' ? PARKED_TTL_MS : NATIVE_QUOTA_TTL_MS; - if (now - state.cachedAt < ttl) return serveCached(state); + if (now - state.cachedAt < ttl && !isCachedRowStaleByReset(state, now)) { + return serveCached(state); + } } // Breaker open or cooldown active -> skip network, go to LOCAL fallback. @@ -1170,7 +1243,7 @@ async function collectClaudeRow( state.pending = (async (): Promise => { try { - const creds = readCredentialsFn(); + const creds = await readCredentialsFn(); // No token / unsupported subscription -> never spend a call, omit the row. if (!creds || !hasSupportedSubscription(creds)) { return serveCached(state); @@ -1579,13 +1652,28 @@ async function getNativeAccountRowsMultiProfile( const results: (BarSummaryRow | null)[] = []; const now = (deps.now ?? Date.now)(); - // Preserve the safety budget: only the active/default profile for each surface - // may perform a live refresh. Non-default profiles are cache-only (or parked) - // so one /summary request can trigger at most one Claude and one Codex live - // upstream call regardless of configured profile count. + // Preserve the safety budget: the active/default profile for each surface is + // live-polled every pass, plus ONE rotating live slot for the stalest + // non-default profile. All other profiles are cache-only (or parked), so one + // /summary request triggers at most two Claude and two Codex live upstream + // calls regardless of configured profile count — every account converges to + // real quota within a few polls without per-profile fan-out. + const claudeRotating = pickRotatingLiveProfile( + claudeProfileStates, + claudeProfiles, + claudeDefault, + now + ); + const codexRotating = pickRotatingLiveProfile( + codexProfileStates, + codexProfiles, + codexDefault, + now + ); + for (const p of claudeProfiles) { const isDefault = p === claudeDefault; - if (!isDefault) { + if (!isDefault && p !== claudeRotating) { results.push( markDefaultAndSyncCache( claudeProfileStates, @@ -1602,14 +1690,18 @@ async function getNativeAccountRowsMultiProfile( deps, force || claudeProfileStates.get(p)?.cachedRow?.quotaStatus === 'unsupported' ) - .then((r) => (r ? markDefaultAndSyncCache(claudeProfileStates, p, r, true) : null)) + .then((r) => { + if (!r) return null; + const marked = markDefaultAndSyncCache(claudeProfileStates, p, r, isDefault); + return isDefault ? marked : markPausedAndSyncCache(claudeProfileStates, p, marked); + }) .catch(() => null) ); } for (const p of codexProfiles) { const isDefault = p === codexDefault; - if (!isDefault) { + if (!isDefault && p !== codexRotating) { results.push( markDefaultAndSyncCache( codexProfileStates, @@ -1626,7 +1718,11 @@ async function getNativeAccountRowsMultiProfile( deps, force || codexProfileStates.get(p)?.cachedRow?.quotaStatus === 'unsupported' ) - .then((r) => (r ? markDefaultAndSyncCache(codexProfileStates, p, r, true) : null)) + .then((r) => { + if (!r) return null; + const marked = markDefaultAndSyncCache(codexProfileStates, p, r, isDefault); + return isDefault ? marked : markPausedAndSyncCache(codexProfileStates, p, marked); + }) .catch(() => null) ); } diff --git a/tests/unit/commands/bar-command.test.ts b/tests/unit/commands/bar-command.test.ts index 5f649ef1..79c3f043 100644 --- a/tests/unit/commands/bar-command.test.ts +++ b/tests/unit/commands/bar-command.test.ts @@ -169,6 +169,13 @@ describe('bar command dispatcher (index.ts)', () => { expect(calls).toEqual(['launch:']); }); + it('rejects an unknown bare launch flag without dispatching launch', async () => { + const handleBarCommand = await loadHandleBarCommand(); + await handleBarCommand(['--porrt', '3999']); + expect(calls).toEqual([]); + expect(process.exitCode).toBe(1); + }); + it('dispatches `ccs bar install` to install subcommand', async () => { const handleBarCommand = await loadHandleBarCommand(); await handleBarCommand(['install']); @@ -3040,7 +3047,7 @@ describe('defaultFindRunningServer: socket-level 401/403 classifies authRequired // parser correctly sets authRequired without relying on the higher-level dep // injection that existing tests use (which bypasses real status-line parsing). - function makeNetMock(statusLine: string) { + function makeNetMock(statusLine: string, token: string) { return { connect: (opts: { host: string; port: number }, onConnect: () => void): unknown => { const listeners: Record void>> = {}; @@ -3054,19 +3061,25 @@ describe('defaultFindRunningServer: socket-level 401/403 classifies authRequired void cb; return socket; }, - write() { + write(request: string) { + const nonce = + request.match(new RegExp(`${BAR_AUTH_NONCE_HEADER}:\\s*([^\\r\\n]+)`, 'i'))?.[1] ?? + ''; + const proof = createBarAuthProof(token, nonce); + setImmediate(() => { + for (const cb of listeners.data ?? []) { + cb( + Buffer.from(`${statusLine}\r\n${BAR_AUTH_TOKEN_HEADER}: ${proof}\r\n\r\n`, 'utf8') + ); + } + }); return true; }, destroy() { return socket; }, }; - setImmediate(() => { - onConnect(); - for (const cb of listeners.data ?? []) { - cb(Buffer.from(`${statusLine}\r\n\r\n`, 'utf8')); - } - }); + setImmediate(onConnect); return socket; }, }; @@ -3081,7 +3094,8 @@ describe('defaultFindRunningServer: socket-level 401/403 classifies authRequired JSON.stringify({ port: 41401, baseUrl: 'http://127.0.0.1:41401', authMode: 'loopback' }) ); - mock.module('net', () => makeNetMock('HTTP/1.1 401 Unauthorized')); + const token = getOrCreateBarAuthToken(ccsDir); + mock.module('net', () => makeNetMock('HTTP/1.1 401 Unauthorized', token)); moduleSeq++; const { defaultFindRunningServer } = (await import( @@ -3111,7 +3125,8 @@ describe('defaultFindRunningServer: socket-level 401/403 classifies authRequired JSON.stringify({ port: 41403, baseUrl: 'http://127.0.0.1:41403', authMode: 'loopback' }) ); - mock.module('net', () => makeNetMock('HTTP/1.1 403 Forbidden')); + const token = getOrCreateBarAuthToken(ccsDir); + mock.module('net', () => makeNetMock('HTTP/1.1 403 Forbidden', token)); moduleSeq++; const { defaultFindRunningServer } = (await import( @@ -3302,6 +3317,30 @@ describe('defaultWaitForServerLive: rogue 200 without matching token is rejected expect(result === 'timeout' || result === 'rejected').toBe(true); }); + it('unrelated 401/403 without a CCS proof does not trigger auth-required identity', async () => { + for (const status of [401, 403]) { + mock.module('net', () => + buildNetMock(`HTTP/1.1 ${status} Unrelated Service\r\nConnection: close\r\n\r\n`) + ); + moduleSeq++; + const { defaultWaitForServerLive } = (await import( + `../../../src/commands/bar/launch-subcommand?test=${Date.now()}-${moduleSeq}` + )) as { + defaultWaitForServerLive: (baseUrl: string) => Promise; + }; + + const result = await Promise.race([ + defaultWaitForServerLive(`http://127.0.0.1:${9900 + status}`).then( + () => 'resolved' as const, + () => 'rejected' as const + ), + new Promise<'still-probing'>((resolve) => setTimeout(() => resolve('still-probing'), 300)), + ]); + expect(result).toBe('still-probing'); + mock.restore(); + } + }); + it('correct-token 200 resolves defaultWaitForServerLive immediately', async () => { const ccsDir = path.join(tempHome, '.ccs'); fs.mkdirSync(ccsDir, { recursive: true }); @@ -3643,3 +3682,332 @@ describe('bar install: --await-quit waits for the running app to quit (GH-1588)' expect(probes).toBeGreaterThanOrEqual(2); }); }); + +// --------------------------------------------------------------------------- +// --port support: `ccs bar [launch] --port N` (user-selectable dashboard port) +// --------------------------------------------------------------------------- + +describe('bar dispatcher: bare flags route to launch', () => { + beforeEach(() => { + mock.module('../../../src/commands/bar/launch-subcommand', () => ({ + handleBarLaunch: async (args: string[]) => { + calls.push(`launch:${args.join(' ')}`); + }, + })); + }); + + it('dispatches `ccs bar --port 3999` to launch with the flag preserved', async () => { + const handleBarCommand = await loadHandleBarCommand(); + await handleBarCommand(['--port', '3999']); + expect(calls).toEqual(['launch:--port 3999']); + }); + + it('dispatches `ccs bar launch --port 3999` to launch with the flag preserved', async () => { + const handleBarCommand = await loadHandleBarCommand(); + await handleBarCommand(['launch', '--port', '3999']); + expect(calls).toEqual(['launch:--port 3999']); + }); +}); + +describe('launch: --port selects the server port', () => { + function makePortDeps(ccsDir: string) { + const seen: { + spawnPort: number | null; + getPortCandidates: number[] | null; + stopped: boolean; + descriptorPort: number | null; + } = { spawnPort: null, getPortCandidates: null, stopped: false, descriptorPort: null }; + + const deps = { + findRunningServer: async () => null, + getPort: async (opts: { port: number[]; host: string }) => { + seen.getPortCandidates = opts.port; + return opts.port[0]; + }, + spawnDetachedServer: (p: number) => { + seen.spawnPort = p; + }, + waitForServerLive: async () => {}, + createLaunchDescriptor: (opts?: { port?: number }) => { + seen.descriptorPort = opts?.port ?? null; + return { + schema: 1 as const, + runtime: '/usr/bin/node', + args: ['/x/ccs.js', 'bar', 'serve'], + home: '/h', + }; + }, + writeLaunchDescriptor: () => {}, + stopDetachedServer: () => { + seen.stopped = true; + }, + openApp: async () => {}, + getCcsDir: () => ccsDir, + appInstallPath: path.join(tempHome, 'Applications', 'CCS Bar.app'), + }; + return { deps, seen }; + } + + it('spawns the detached server on the requested port and records it in bar.json', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + + await handleBarLaunch(['--port', '3999'], deps); + + expect(seen.spawnPort).toBe(3999); + const barJson = JSON.parse(fs.readFileSync(path.join(ccsDir, 'bar.json'), 'utf8')) as { + port: number; + baseUrl: string; + }; + expect(barJson.port).toBe(3999); + expect(barJson.baseUrl).toBe('http://127.0.0.1:3999'); + }); + + it('persists the chosen port into the launch descriptor for app self-start', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + + await handleBarLaunch(['--port', '3999'], deps); + + expect(seen.descriptorPort).toBe(3999); + }); + + it('errors without spawning when the requested port is busy', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + deps.getPort = async () => 4001; // get-port fell back: 3999 not free + + await handleBarLaunch(['--port', '3999'], deps); + + expect(seen.spawnPort).toBeNull(); + const allOutput = consoleOutput.join('\n'); + expect(allOutput).toMatch(/3999/); + expect(allOutput.toLowerCase()).toMatch(/in use|busy|not free|unavailable/); + }); + + it('errors on an invalid --port value', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + + await handleBarLaunch(['--port', 'banana'], deps); + + expect(seen.spawnPort).toBeNull(); + expect(consoleOutput.join('\n').toLowerCase()).toMatch(/invalid.*port|port.*invalid/); + }); + + it('reuses a running server already on the requested port', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + deps.findRunningServer = async () => ({ port: 3999, baseUrl: 'http://127.0.0.1:3999' }); + + await handleBarLaunch(['--port', '3999'], deps); + + expect(seen.spawnPort).toBeNull(); // reuse, no new spawn + expect(seen.stopped).toBe(false); + const barJson = JSON.parse(fs.readFileSync(path.join(ccsDir, 'bar.json'), 'utf8')) as { + port: number; + }; + expect(barJson.port).toBe(3999); + }); + + it('stops a running server on a different port, then starts on the requested one', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + + await handleBarLaunch(['--port', '3999'], deps); + + expect(seen.stopped).toBe(true); + expect(seen.spawnPort).toBe(3999); + const barJson = JSON.parse(fs.readFileSync(path.join(ccsDir, 'bar.json'), 'utf8')) as { + port: number; + }; + expect(barJson.port).toBe(3999); + }); + + it('preflights the destination before stopping a healthy running server', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps } = makePortDeps(ccsDir); + const events: string[] = []; + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.getPort = async () => { + events.push('preflight'); + return 3999; + }; + deps.stopDetachedServer = () => { + events.push('stop'); + }; + + await handleBarLaunch(['--port', '3999'], deps); + expect(events).toEqual(['preflight', 'stop']); + }); + + it('leaves the healthy server running when destination preflight is busy', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.getPort = async () => 4000; + + await handleBarLaunch(['--port', '3999'], deps); + expect(seen.stopped).toBe(false); + expect(seen.spawnPort).toBeNull(); + }); + + it('aborts the move when the verified stop fails', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.stopDetachedServer = () => { + throw new Error('identity mismatch'); + }; + + await handleBarLaunch(['--port', '3999'], deps); + expect(seen.spawnPort).toBeNull(); + }); + + it('restores the prior server when a check-to-bind race breaks the replacement', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps } = makePortDeps(ccsDir); + const spawnedPorts: number[] = []; + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.spawnDetachedServer = (port: number) => { + spawnedPorts.push(port); + }; + deps.waitForServerLive = async (baseUrl: string) => { + if (baseUrl.endsWith(':3999')) throw new Error('EADDRINUSE after preflight'); + }; + + await handleBarLaunch(['--port', '3999'], deps); + expect(spawnedPorts).toEqual([3999, 3000]); + }); + + it('restores the prior server when replacement spawn fails', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps } = makePortDeps(ccsDir); + const spawnedPorts: number[] = []; + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.spawnDetachedServer = (port: number) => { + spawnedPorts.push(port); + if (port === 3999) throw new Error('spawn failed'); + }; + + await handleBarLaunch(['--port', '3999'], deps); + expect(spawnedPorts).toEqual([3999, 3000]); + }); + + it('preserves prior discovery state when replacement and rollback both fail', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(path.join(ccsDir, 'bar'), { recursive: true }); + const oldBarJson = JSON.stringify({ + baseUrl: 'http://127.0.0.1:3000', + port: 3000, + authMode: 'loopback', + }); + const oldLaunchJson = JSON.stringify({ args: ['ccs.js', 'bar', 'serve', '--port', '3000'] }); + fs.writeFileSync(path.join(ccsDir, 'bar.json'), oldBarJson); + fs.writeFileSync(path.join(ccsDir, 'bar', 'launch.json'), oldLaunchJson); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps } = makePortDeps(ccsDir); + deps.findRunningServer = async () => ({ port: 3000, baseUrl: 'http://127.0.0.1:3000' }); + deps.spawnDetachedServer = () => { + throw new Error('spawn failed'); + }; + + await handleBarLaunch(['--port', '3999'], deps); + expect(fs.readFileSync(path.join(ccsDir, 'bar.json'), 'utf8')).toBe(oldBarJson); + expect(fs.readFileSync(path.join(ccsDir, 'bar', 'launch.json'), 'utf8')).toBe(oldLaunchJson); + }); + + it('without --port, prefers the port recorded in bar.json (sticky port)', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'bar.json'), + JSON.stringify({ baseUrl: 'http://127.0.0.1:3777', port: 3777, authMode: 'loopback' }) + ); + const { handleBarLaunch } = await loadLaunchSubcommand(); + const { deps, seen } = makePortDeps(ccsDir); + + await handleBarLaunch([], deps); + + expect(seen.getPortCandidates?.[0]).toBe(3777); + expect(seen.spawnPort).toBe(3777); + }); +}); + +describe('resolveBarPort: launch.json fallback survives `ccs bar stop`', () => { + // `ccs bar stop` deletes bar.json, so bar.json alone cannot carry the sticky + // port across a stop/start cycle. launch.json (refreshed by launch, NOT + // deleted by stop) records the port in its args and acts as the fallback. + it('falls back to the --port recorded in launch.json when bar.json is absent', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(path.join(ccsDir, 'bar'), { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'bar', 'launch.json'), + JSON.stringify({ + schema: 1, + runtime: '/usr/bin/node', + args: ['/x/ccs.js', 'bar', 'serve', '--port', '3456'], + home: tempHome, + }) + ); + + const { resolveBarPort } = await loadLaunchSubcommand(); + expect(resolveBarPort(ccsDir)).toBe(3456); + }); + + it('bar.json port wins over launch.json when both exist', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(path.join(ccsDir, 'bar'), { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'bar.json'), + JSON.stringify({ baseUrl: 'http://127.0.0.1:4000', port: 4000, authMode: 'loopback' }) + ); + fs.writeFileSync( + path.join(ccsDir, 'bar', 'launch.json'), + JSON.stringify({ + schema: 1, + runtime: '/usr/bin/node', + args: ['/x/ccs.js', 'bar', 'serve', '--port', '3456'], + home: tempHome, + }) + ); + + const { resolveBarPort } = await loadLaunchSubcommand(); + expect(resolveBarPort(ccsDir)).toBe(4000); + }); + + it('returns null when launch.json has no --port and bar.json is absent', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + fs.mkdirSync(path.join(ccsDir, 'bar'), { recursive: true }); + fs.writeFileSync( + path.join(ccsDir, 'bar', 'launch.json'), + JSON.stringify({ + schema: 1, + runtime: '/usr/bin/node', + args: ['/x/ccs.js', 'bar', 'serve'], + home: tempHome, + }) + ); + + const { resolveBarPort } = await loadLaunchSubcommand(); + expect(resolveBarPort(ccsDir)).toBeNull(); + }); +}); diff --git a/tests/unit/commands/bar-lifecycle-hardening.test.ts b/tests/unit/commands/bar-lifecycle-hardening.test.ts new file mode 100644 index 00000000..899131c4 --- /dev/null +++ b/tests/unit/commands/bar-lifecycle-hardening.test.ts @@ -0,0 +1,307 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import * as fs from 'fs'; +import * as http from 'http'; +import * as os from 'os'; +import * as path from 'path'; +import { spawn } from 'child_process'; +import { + defaultFindRunningServer, + resolveBarPort, +} from '../../../src/commands/bar/bar-server-probe'; +import { + serializeBarServerProcessRecord, + getProcessBirthIdentity, + removeBarDiscoveryIfNoProcess, + removeBarServerProcessRecordIfOwned, + stopDetachedBarServer, + stopBarServerProcessFile, + stopRecordedBarServer, +} from '../../../src/commands/bar/bar-process-control'; +import { parsePortFlag, validatePortArgs } from '../../../src/commands/bar/port-arg'; +import { handleBarServe } from '../../../src/commands/bar/serve-subcommand'; +import { handleBarStop } from '../../../src/commands/bar/stop-subcommand'; + +let tempHome: string; +let originalHome: string | undefined; +let originalCcsHome: string | undefined; +const liveChildren = new Set>(); + +beforeEach(() => { + tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-bar-lifecycle-')); + originalHome = process.env.HOME; + originalCcsHome = process.env.CCS_HOME; + process.env.HOME = tempHome; + process.env.CCS_HOME = path.join(tempHome, '.ccs'); + process.exitCode = 0; +}); + +afterEach(() => { + for (const child of liveChildren) { + try { + child.kill('SIGKILL'); + } catch { + // Already exited. + } + } + liveChildren.clear(); + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalCcsHome === undefined) delete process.env.CCS_HOME; + else process.env.CCS_HOME = originalCcsHome; + process.exitCode = 0; + fs.rmSync(tempHome, { recursive: true, force: true }); +}); + +describe('strict Bar port parsing', () => { + it('rejects numeric prefixes, fractions, signs, whitespace, and out-of-range values', () => { + for (const raw of ['3999junk', '3.5', '+3999', ' 3999', '0', '65536']) { + expect(parsePortFlag(['--port', raw])).toEqual({ present: true, port: null }); + } + expect(parsePortFlag(['--port', '3999'])).toEqual({ present: true, port: 3999 }); + }); + + it('rejects unknown and duplicate launch options', () => { + expect(validatePortArgs(['--porrt', '3999'])).toBe('Unknown option: --porrt'); + expect(validatePortArgs(['--port', '3999', '--port', '4000'])).toBe('Duplicate option: --port'); + }); + + it('rejects malformed persisted ports in both discovery files', () => { + const ccsDir = process.env.CCS_HOME!; + fs.mkdirSync(path.join(ccsDir, 'bar'), { recursive: true }); + fs.writeFileSync(path.join(ccsDir, 'bar.json'), JSON.stringify({ port: 3999.5 })); + fs.writeFileSync( + path.join(ccsDir, 'bar', 'launch.json'), + JSON.stringify({ args: ['ccs.js', 'bar', 'serve', '--port', '4555junk'] }) + ); + expect(resolveBarPort(ccsDir)).toBeNull(); + }); +}); + +describe('verified Bar process stopping', () => { + const rawRecord = serializeBarServerProcessRecord({ pid: 4321, birthIdentity: 'birth-a' }); + + it('does not signal when the PID birth identity changed', async () => { + let signaled = false; + const outcome = await stopRecordedBarServer(rawRecord, { + getProcessBirthIdentity: () => 'birth-b', + killProcess: () => { + signaled = true; + }, + }); + expect(outcome.result).toBe('identity-mismatch'); + expect(signaled).toBe(false); + }); + + it('preserves server.pid and bar.json on mismatch, EPERM, and timeout', async () => { + for (const failure of [ + 'identity-mismatch', + 'permission-denied', + 'signal-failed', + 'timeout', + ] as const) { + const ccsDir = path.join(tempHome, failure); + const pidPath = path.join(ccsDir, 'bar', 'server.pid'); + const barJsonPath = path.join(ccsDir, 'bar.json'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + fs.writeFileSync(pidPath, rawRecord); + fs.writeFileSync(barJsonPath, '{}'); + + await handleBarStop([], { + getCcsDir: () => ccsDir, + getProcessBirthIdentity: () => (failure === 'identity-mismatch' ? 'birth-b' : 'birth-a'), + killProcess: () => { + if (failure === 'permission-denied') { + const err = new Error('not permitted') as NodeJS.ErrnoException; + err.code = 'EPERM'; + throw err; + } + if (failure === 'signal-failed') throw new Error('signal transport failed'); + }, + waitForProcessExit: async () => (failure === 'timeout' ? 'timeout' : 'exited'), + }); + + expect(fs.existsSync(pidPath)).toBe(true); + expect(fs.existsSync(barJsonPath)).toBe(true); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + } + }); + + it('never signals a legacy integer PID and gives manual recovery guidance', async () => { + const ccsDir = path.join(tempHome, 'legacy'); + const pidPath = path.join(ccsDir, 'bar', 'server.pid'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + fs.writeFileSync(pidPath, '4321'); + let signaled = false; + + await handleBarStop([], { + getCcsDir: () => ccsDir, + killProcess: () => { + signaled = true; + }, + }); + + expect(signaled).toBe(false); + expect(fs.existsSync(pidPath)).toBe(true); + expect(process.exitCode).toBe(1); + }); + + it('atomically preserves a replacement record written while the old process stops', async () => { + const pidPath = path.join(tempHome, 'race', 'server.pid'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + fs.writeFileSync(pidPath, rawRecord); + const replacement = serializeBarServerProcessRecord({ + pid: 9876, + birthIdentity: 'replacement-birth', + }); + + const outcome = await stopBarServerProcessFile(pidPath, { + getProcessBirthIdentity: () => 'birth-a', + killProcess: () => fs.writeFileSync(pidPath, replacement), + waitForProcessExit: async () => 'exited', + }); + + expect(outcome.result).toBe('stopped'); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(replacement); + }); + + it('does not let an old serve cleanup unlink a replacement record', () => { + const pidPath = path.join(tempHome, 'serve-race', 'server.pid'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + const replacement = serializeBarServerProcessRecord({ + pid: 9876, + birthIdentity: 'replacement-birth', + }); + fs.writeFileSync(pidPath, replacement); + + removeBarServerProcessRecordIfOwned(pidPath, { pid: 4321, birthIdentity: 'birth-a' }); + + expect(fs.readFileSync(pidPath, 'utf8')).toBe(replacement); + }); + + it('stops a real recorded process through the claimed process file', async () => { + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + stdio: 'ignore', + }); + liveChildren.add(child); + const birthIdentity = await waitForBirthIdentity(child.pid!); + const pidPath = path.join(tempHome, 'real-stop', 'server.pid'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + fs.writeFileSync(pidPath, serializeBarServerProcessRecord({ pid: child.pid!, birthIdentity })); + + const outcome = await stopBarServerProcessFile(pidPath); + + expect(outcome.result).toBe('stopped'); + expect(fs.existsSync(pidPath)).toBe(false); + liveChildren.delete(child); + }); + + it('stops a real recorded process through the launch-move stop path', async () => { + const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { + stdio: 'ignore', + }); + liveChildren.add(child); + const birthIdentity = await waitForBirthIdentity(child.pid!); + const ccsDir = path.join(tempHome, 'real-move'); + const pidPath = path.join(ccsDir, 'bar', 'server.pid'); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + fs.writeFileSync(pidPath, serializeBarServerProcessRecord({ pid: child.pid!, birthIdentity })); + await stopDetachedBarServer(ccsDir); + + expect(fs.existsSync(pidPath)).toBe(false); + liveChildren.delete(child); + }); +}); + +describe('Bar serve publication ownership', () => { + it('publishes server.pid before bar.json so old-stop cleanup preserves replacement discovery', async () => { + const ccsDir = path.join(tempHome, 'serve-publication'); + const pidPath = path.join(ccsDir, 'bar', 'server.pid'); + const barJsonPath = path.join(ccsDir, 'bar.json'); + const publicationOrder: string[] = []; + let oldStopRemovedDiscovery: boolean | null = null; + + await handleBarServe(['--port', '4555'], { + getCcsDir: () => ccsDir, + findRunningServer: async () => null, + getPort: async () => 4555, + startServer: async () => ({ port: 4555, baseUrl: 'http://127.0.0.1:4555' }), + getProcessBirthIdentity: () => 'replacement-birth', + writeFile: (filePath, content) => { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + publicationOrder.push(filePath); + if (filePath === barJsonPath) { + oldStopRemovedDiscovery = removeBarDiscoveryIfNoProcess(barJsonPath, pidPath); + } + }, + onSignal: () => {}, + exit: (code) => { + throw new Error(`unexpected exit ${code}`); + }, + }); + + expect(publicationOrder).toEqual([pidPath, barJsonPath]); + expect(oldStopRemovedDiscovery).toBe(false); + expect(fs.existsSync(pidPath)).toBe(true); + expect(JSON.parse(fs.readFileSync(barJsonPath, 'utf8')).port).toBe(4555); + }); + + it('conditionally rolls back its owned process record when discovery publication fails', async () => { + const ccsDir = path.join(tempHome, 'serve-rollback'); + const pidPath = path.join(ccsDir, 'bar', 'server.pid'); + const barJsonPath = path.join(ccsDir, 'bar.json'); + + await expect( + handleBarServe(['--port', '4555'], { + getCcsDir: () => ccsDir, + findRunningServer: async () => null, + getPort: async () => 4555, + startServer: async () => ({ port: 4555, baseUrl: 'http://127.0.0.1:4555' }), + getProcessBirthIdentity: () => 'replacement-birth', + writeFile: (filePath, content) => { + if (filePath === barJsonPath) throw new Error('discovery write failed'); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + }, + onSignal: () => {}, + exit: (code) => { + throw new Error(`exit ${code}`); + }, + }) + ).rejects.toThrow('exit 1'); + + expect(fs.existsSync(pidPath)).toBe(false); + }); +}); + +async function waitForBirthIdentity(pid: number): Promise { + for (let attempt = 0; attempt < 50; attempt += 1) { + const identity = getProcessBirthIdentity(pid); + if (identity !== null) return identity; + await new Promise((resolve) => setTimeout(resolve, 20)); + } + throw new Error(`Process ${pid} never became observable`); +} + +describe('Bar server identity probe', () => { + it('ignores unrelated services returning 401 or 403 without a CCS proof', async () => { + for (const status of [401, 403]) { + const server = http.createServer((_req, res) => { + res.writeHead(status); + res.end(); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const port = (server.address() as { port: number }).port; + const ccsDir = path.join(tempHome, `status-${status}`); + fs.mkdirSync(ccsDir, { recursive: true }); + fs.writeFileSync(path.join(ccsDir, 'bar.json'), JSON.stringify({ port })); + try { + const result = await defaultFindRunningServer(ccsDir); + expect(result?.port).not.toBe(port); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + } + }); +}); diff --git a/tests/unit/commands/bar-lifecycle-subcommands.test.ts b/tests/unit/commands/bar-lifecycle-subcommands.test.ts index fdebbc2b..60c0ed04 100644 --- a/tests/unit/commands/bar-lifecycle-subcommands.test.ts +++ b/tests/unit/commands/bar-lifecycle-subcommands.test.ts @@ -15,6 +15,10 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +function processRecord(pid: number, birthIdentity = 'test-birth'): string { + return JSON.stringify({ pid, birthIdentity }, null, 2); +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -24,6 +28,7 @@ let tempHome: string; let originalCcsHome: string | undefined; let originalConsoleLog: typeof console.log; let originalConsoleError: typeof console.error; +let originalExitCode: number | undefined; function captureConsole(): void { originalConsoleLog = console.log; @@ -113,6 +118,8 @@ beforeEach(() => { tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-bar-lifecycle-test-')); originalCcsHome = process.env.CCS_HOME; + originalExitCode = process.exitCode; + process.exitCode = 0; process.env.CCS_HOME = tempHome; }); @@ -124,6 +131,7 @@ afterEach(() => { } else { process.env.CCS_HOME = originalCcsHome; } + process.exitCode = originalExitCode ?? 0; try { fs.rmSync(tempHome, { recursive: true, force: true }); @@ -214,6 +222,7 @@ describe('serve: start new server', () => { exit: (code: number) => { throw new Error(`__EXIT_${code}__`); }, + getProcessBirthIdentity: () => 'test-birth', }); // bar.json must be written @@ -226,7 +235,10 @@ describe('serve: start new server', () => { // server.pid must be written const pidPath = path.join(ccsDir, 'bar', 'server.pid'); expect(writtenFiles[pidPath]).toBeDefined(); - expect(writtenFiles[pidPath]).toBe(String(process.pid)); + expect(JSON.parse(writtenFiles[pidPath])).toEqual({ + pid: process.pid, + birthIdentity: 'test-birth', + }); // Both SIGINT and SIGTERM handlers registered expect(signals).toContain('SIGINT'); @@ -309,7 +321,7 @@ describe('stop: SIGTERM and cleanup', () => { const ccsDir = path.join(tempHome, '.ccs'); const barDir = path.join(ccsDir, 'bar'); fs.mkdirSync(barDir, { recursive: true }); - fs.writeFileSync(path.join(barDir, 'server.pid'), '12345'); + fs.writeFileSync(path.join(barDir, 'server.pid'), processRecord(12345)); fs.writeFileSync(path.join(ccsDir, 'bar.json'), '{"baseUrl":"http://127.0.0.1:3000"}'); const killed: Array<{ pid: number; signal: string }> = []; @@ -329,6 +341,8 @@ describe('stop: SIGTERM and cleanup', () => { killProcess: (pid: number, signal: string) => { killed.push({ pid, signal }); }, + getProcessBirthIdentity: () => 'test-birth', + waitForProcessExit: async () => 'exited', removeFile: (filePath: string) => { removed.push(filePath); }, @@ -338,7 +352,7 @@ describe('stop: SIGTERM and cleanup', () => { // Both pid and bar.json must be removed expect(removed.some((p) => p.includes('server.pid'))).toBe(true); expect(removed.some((p) => p.includes('bar.json'))).toBe(true); - expect(allOutput()).toMatch(/\[OK\].*SIGTERM/i); + expect(allOutput()).toMatch(/\[OK\].*stopped/i); }); it('prints guidance and returns cleanly when no server.pid exists', async () => { @@ -370,7 +384,8 @@ describe('stop: SIGTERM and cleanup', () => { await handleBarStop([], { getCcsDir: () => ccsDir, - readPidFile: () => '99999', + readPidFile: () => processRecord(99999), + getProcessBirthIdentity: () => null, killProcess: () => { const err = new Error('no such process') as NodeJS.ErrnoException; err.code = 'ESRCH'; @@ -402,9 +417,9 @@ describe('stop: SIGTERM and cleanup', () => { }, }); - expect(allOutput()).toMatch(/\[X\].*invalid/i); - // Corrupted pid file must be cleaned up - expect(removed.some((p) => p.includes('server.pid'))).toBe(true); + expect(allOutput()).toMatch(/\[X\].*verified process record/i); + // Unverified state is preserved rather than risking removal of recovery evidence. + expect(removed.some((p) => p.includes('server.pid'))).toBe(false); }); }); @@ -419,7 +434,7 @@ describe('status: running state reporting', () => { await handleBarStatus([], { getCcsDir: () => ccsDir, - readPidFile: () => '12345', + readPidFile: () => processRecord(12345), isProcessAlive: () => true, probeServer: async () => true, readBarJsonBaseUrl: () => 'http://127.0.0.1:3000', @@ -451,7 +466,7 @@ describe('status: running state reporting', () => { await handleBarStatus([], { getCcsDir: () => ccsDir, - readPidFile: () => '99999', + readPidFile: () => processRecord(99999), isProcessAlive: () => false, probeServer: async () => false, readBarJsonBaseUrl: () => null, @@ -461,13 +476,34 @@ describe('status: running state reporting', () => { expect(allOutput()).toMatch(/99999/); }); + it('reports legacy integer PIDs as unverified and does not inspect their liveness', async () => { + const ccsDir = path.join(tempHome, '.ccs'); + const { handleBarStatus } = await loadStatusSubcommand(); + let inspected = false; + + await handleBarStatus([], { + getCcsDir: () => ccsDir, + readPidFile: () => '12345', + isProcessAlive: () => { + inspected = true; + return true; + }, + probeServer: async () => true, + readBarJsonBaseUrl: () => null, + }); + + expect(inspected).toBe(false); + expect(allOutput()).toMatch(/legacy.*unverified PID 12345/i); + expect(allOutput()).toMatch(/ps -p 12345 -o command=/); + }); + it('reports alive-but-unreachable when PID is alive but HTTP probe fails', async () => { const ccsDir = path.join(tempHome, '.ccs'); const { handleBarStatus } = await loadStatusSubcommand(); await handleBarStatus([], { getCcsDir: () => ccsDir, - readPidFile: () => '12345', + readPidFile: () => processRecord(12345), isProcessAlive: () => true, probeServer: async () => false, readBarJsonBaseUrl: () => 'http://127.0.0.1:3000', diff --git a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts index 656c4e7a..2497b2f4 100644 --- a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts +++ b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts @@ -321,4 +321,100 @@ describe('ProxyRequestTransformer regressions', () => { expect(result.tool_choice).toBe('auto'); }); + + it('merges the top-level system field with a mid-array system message into one leading system message', () => { + // Claude Code sends the main system prompt via the top-level `system` + // field AND a skill/plugin listing as a `role: "system"` message inside + // `messages` (see #1459). Prepending the top-level field unconditionally + // used to leave two non-adjacent `system` messages in the payload, which + // strict OpenAI-compatible backends (LiteLLM among them) reject with: + // `400 A 'system' message can only appear at index 0 of the messages array.` + const result = new ProxyRequestTransformer().transform({ + system: [{ type: 'text', text: 'You are Claude Code, a CLI tool.' }], + messages: [ + { role: 'user', content: 'ping' }, + { + role: 'system', + content: 'The following skills are available for use with the Skill tool:\n- foo', + }, + { role: 'user', content: 'pong' }, + ], + }); + + const systemMessages = result.messages.filter((message) => message.role === 'system'); + expect(systemMessages).toHaveLength(1); + expect(result.messages[0]).toEqual({ + role: 'system', + content: + 'You are Claude Code, a CLI tool.\n\nThe following skills are available for use with the Skill tool:\n- foo', + }); + expect(result.messages[1]).toEqual({ role: 'user', content: 'ping\npong' }); + }); + + it('hoists a late system message after complete parallel tool results without disturbing tool order', () => { + const result = new ProxyRequestTransformer().transform({ + system: 'base instructions', + messages: [ + { role: 'user', content: 'inspect both files' }, + { + role: 'assistant', + content: [ + { type: 'tool_use', id: 'toolu_1', name: 'read', input: { path: 'a.ts' } }, + { type: 'tool_use', id: 'toolu_2', name: 'read', input: { path: 'b.ts' } }, + ], + }, + { + role: 'user', + content: [ + { type: 'tool_result', tool_use_id: 'toolu_1', content: 'a contents' }, + { type: 'tool_result', tool_use_id: 'toolu_2', content: 'b contents' }, + ], + }, + { role: 'system', content: 'late instructions' }, + { role: 'user', content: 'compare them' }, + ], + }); + + expect(result.messages).toEqual([ + { role: 'system', content: 'base instructions\n\nlate instructions' }, + { role: 'user', content: 'inspect both files' }, + { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'toolu_1', + type: 'function', + function: { name: 'read', arguments: '{"path":"a.ts"}' }, + }, + { + id: 'toolu_2', + type: 'function', + function: { name: 'read', arguments: '{"path":"b.ts"}' }, + }, + ], + }, + { role: 'tool', tool_call_id: 'toolu_1', content: 'a contents' }, + { role: 'tool', tool_call_id: 'toolu_2', content: 'b contents' }, + { role: 'user', content: 'compare them' }, + ]); + }); + + it('rejects a system message inserted before pending tool results', () => { + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'tool_use', id: 'toolu_1', name: 'read', input: { path: 'a.ts' } }], + }, + { role: 'system', content: 'interrupting instructions' }, + { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'a contents' }], + }, + ], + }) + ).toThrow('role must be "user" with tool_result blocks after assistant tool_use'); + }); }); diff --git a/tests/unit/proxy/transformers/request-transformer.test.ts b/tests/unit/proxy/transformers/request-transformer.test.ts index f3258f07..847f58ba 100644 --- a/tests/unit/proxy/transformers/request-transformer.test.ts +++ b/tests/unit/proxy/transformers/request-transformer.test.ts @@ -43,7 +43,7 @@ describe('ProxyRequestTransformer', () => { }); }); - it('accepts Claude Code system messages in the messages array', () => { + it('accepts Claude Code system messages in the messages array and hoists them to a single leading system message', () => { const transformer = new ProxyRequestTransformer(); const result = transformer.transform({ messages: [ @@ -53,10 +53,14 @@ describe('ProxyRequestTransformer', () => { ], }); + // Strict OpenAI-compatible backends (e.g. LiteLLM) reject any payload + // where `system` is not alone at index 0, so a mid-array `system` + // message must be hoisted rather than left in place. See #1459 for why + // the message must be accepted at all, and the coalesce-duplicate-system + // fix for why it can't simply stay where it landed. expect(result.messages).toEqual([ - { role: 'user', content: 'hello' }, { role: 'system', content: 'answer tersely' }, - { role: 'user', content: 'which model is this?' }, + { role: 'user', content: 'hello\nwhich model is this?' }, ]); }); diff --git a/tests/unit/web-server/bar-routes.test.ts b/tests/unit/web-server/bar-routes.test.ts index 4890820f..10fbc1d6 100644 --- a/tests/unit/web-server/bar-routes.test.ts +++ b/tests/unit/web-server/bar-routes.test.ts @@ -1116,6 +1116,49 @@ describe('/summary force flag passed to getNativeAccountRows', () => { expect(status).toBe(200); expect(body.some((r) => r.provider === 'codex')).toBe(true); }); + + it('enforces one absolute deadline across cost, quota, and blocked native work', async () => { + const { createBarRouter, resetForceFreshDebounce: resetDebounce } = await import( + '../../../src/web-server/routes/bar-routes' + ); + const app = express(); + app.use(express.json()); + const router = createBarRouter({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getAllAccountsSummary: () => ({ agy: [makeAccountInfo()] }) as any, + getCachedQuota: () => makeQuotaResult(), + setCachedQuota: () => {}, + invalidateQuotaCache: () => {}, + fetchAccountQuota: () => new Promise(() => {}), + getTodayCostByAccount: () => ({}), + loadCliproxyDetails: () => new Promise((resolve) => setTimeout(() => resolve([]), 1_400)), + loadDailyUsage: async () => [], + loadHourlyUsage: async () => [], + getNativeAccountRows: () => new Promise(() => {}), + getCachedNativeRows: () => [], + }); + app.use('/api/bar', router); + const srv = await new Promise((resolve, reject) => { + const instance = app.listen(0, '127.0.0.1'); + instance.once('error', reject); + instance.once('listening', () => resolve(instance)); + }); + const addr = srv.address(); + if (!addr || typeof addr === 'string') throw new Error('No server address'); + resetDebounce(); + + const startedAt = Date.now(); + const { status } = await getJson( + `http://127.0.0.1:${(addr as { port: number }).port}`, + '/api/bar/summary?refresh=true' + ); + const elapsed = Date.now() - startedAt; + await new Promise((resolve) => srv.close(() => resolve())); + + expect(status).toBe(200); + expect(elapsed).toBeGreaterThanOrEqual(2_200); + expect(elapsed).toBeLessThan(3_200); + }); }); // ============================================================================ diff --git a/tests/unit/web-server/claude-native-credentials.test.ts b/tests/unit/web-server/claude-native-credentials.test.ts index 4a4643eb..1de415b6 100644 --- a/tests/unit/web-server/claude-native-credentials.test.ts +++ b/tests/unit/web-server/claude-native-credentials.test.ts @@ -8,6 +8,8 @@ import { describe, expect, it } from 'bun:test'; import { readClaudeCredentials, + readClaudeCredentialsForConfigDir, + claudeKeychainServiceForConfigDir, getAccessToken, getSubscriptionTier, hasSupportedSubscription, @@ -25,14 +27,14 @@ function makeCreds(overrides: Record = {}): ClaudeNativeCredent } describe('readClaudeCredentials', () => { - it('parses the on-disk credentials file when present (file-first, no Keychain)', () => { + it('parses the on-disk credentials file when present (file-first, no Keychain)', async () => { let keychainCalled = false; - const creds = readClaudeCredentials({ + const creds = await readClaudeCredentials({ platform: 'darwin', homedir: '/home/test', existsSyncImpl: () => true, readFileSyncImpl: () => JSON.stringify(makeCreds()), - execSyncImpl: () => { + execFileImpl: () => { keychainCalled = true; return ''; }, @@ -42,44 +44,52 @@ describe('readClaudeCredentials', () => { expect(keychainCalled).toBe(false); }); - it('falls back to the macOS Keychain when the file is absent', () => { - const creds = readClaudeCredentials({ + it('falls back to the macOS Keychain when the file is absent', async () => { + let executable = ''; + let args: readonly string[] = []; + const creds = await readClaudeCredentials({ platform: 'darwin', homedir: '/home/test', existsSyncImpl: () => false, readFileSyncImpl: () => { throw new Error('should not read file'); }, - execSyncImpl: () => JSON.stringify(makeCreds({ subscriptionType: 'pro' })), + execFileImpl: (file, receivedArgs, _options, callback) => { + executable = file; + args = receivedArgs; + callback(null, JSON.stringify(makeCreds({ subscriptionType: 'pro' }))); + }, }); expect(creds?.claudeAiOauth?.subscriptionType).toBe('pro'); + expect(executable).toBe('/usr/bin/security'); + expect(args).toEqual(['find-generic-password', '-s', 'Claude Code-credentials', '-w']); }); - it('returns null when both file and Keychain are absent', () => { - const creds = readClaudeCredentials({ + it('returns null when both file and Keychain are absent', async () => { + const creds = await readClaudeCredentials({ platform: 'darwin', homedir: '/home/test', existsSyncImpl: () => false, readFileSyncImpl: () => { throw new Error('no file'); }, - execSyncImpl: () => { - throw new Error('no keychain entry'); + execFileImpl: (_file, _args, _options, callback) => { + callback(new Error('no keychain entry'), ''); }, }); expect(creds).toBeNull(); }); - it('does not consult the Keychain on non-darwin platforms', () => { + it('does not consult the Keychain on non-darwin platforms', async () => { let keychainCalled = false; - const creds = readClaudeCredentials({ + const creds = await readClaudeCredentials({ platform: 'linux', homedir: '/home/test', existsSyncImpl: () => false, readFileSyncImpl: () => { throw new Error('no file'); }, - execSyncImpl: () => { + execFileImpl: () => { keychainCalled = true; return ''; }, @@ -124,3 +134,107 @@ describe('token + tier extraction', () => { expect(getSubscriptionTier(null)).toBeNull(); }); }); + +// --------------------------------------------------------------------------- +// Per-config-dir credential reading (isolated `ccs auth` profiles on macOS) +// +// Claude Code stores OAuth credentials for a non-default CLAUDE_CONFIG_DIR in +// a per-directory Keychain item: service "Claude Code-credentials-", +// where is the first 8 hex chars of sha256(configDir). These tests pin +// that derivation and the file-first / Keychain-fallback read order. +// --------------------------------------------------------------------------- + +describe('claudeKeychainServiceForConfigDir', () => { + it('derives the service name from sha256 of the config dir path (first 8 hex chars)', () => { + // sha256("/home/test/.ccs/instances/work") = ffeb4b45... + expect(claudeKeychainServiceForConfigDir('/home/test/.ccs/instances/work')).toBe( + 'Claude Code-credentials-ffeb4b45' + ); + }); +}); + +describe('readClaudeCredentialsForConfigDir', () => { + const configDir = '/home/test/.ccs/instances/work'; + const credFile = `${configDir}/.credentials.json`; + + it('reads /.credentials.json when present (file-first, no Keychain)', async () => { + let keychainCalled = false; + const creds = await readClaudeCredentialsForConfigDir(configDir, { + platform: 'darwin', + existsSyncImpl: (p: string) => p === credFile, + readFileSyncImpl: (p: string) => { + expect(p).toBe(credFile); + return JSON.stringify(makeCreds()); + }, + execFileImpl: () => { + keychainCalled = true; + return ''; + }, + }); + expect(creds?.claudeAiOauth?.accessToken).toBe('tok-abc'); + expect(keychainCalled).toBe(false); + }); + + it('uses absolute security path and argument array for the per-config-dir Keychain item', async () => { + let executable = ''; + let args: readonly string[] = []; + const creds = await readClaudeCredentialsForConfigDir(configDir, { + platform: 'darwin', + existsSyncImpl: () => false, + readFileSyncImpl: () => { + throw new Error('should not read file'); + }, + execFileImpl: (file, receivedArgs, _options, callback) => { + executable = file; + args = receivedArgs; + callback(null, JSON.stringify(makeCreds({ subscriptionType: 'team' }))); + }, + }); + expect(creds?.claudeAiOauth?.subscriptionType).toBe('team'); + expect(executable).toBe('/usr/bin/security'); + expect(args).toEqual(['find-generic-password', '-s', 'Claude Code-credentials-ffeb4b45', '-w']); + }); + + it('returns null when both file and Keychain are absent', async () => { + const creds = await readClaudeCredentialsForConfigDir(configDir, { + platform: 'darwin', + existsSyncImpl: () => false, + readFileSyncImpl: () => { + throw new Error('no file'); + }, + execFileImpl: (_file, _args, _options, callback) => { + callback(new Error('no keychain entry'), ''); + }, + }); + expect(creds).toBeNull(); + }); + + it('bounds a blocked Keychain lookup without exposing a credential payload', async () => { + const startedAt = Date.now(); + const creds = await readClaudeCredentialsForConfigDir(configDir, { + platform: 'darwin', + existsSyncImpl: () => false, + keychainTimeoutMs: 20, + execFileImpl: () => ({ kill: () => {} }), + }); + expect(creds).toBeNull(); + expect(Date.now() - startedAt).toBeLessThan(250); + }); + + it('does not consult the Keychain on non-darwin platforms', async () => { + let keychainCalled = false; + const creds = await readClaudeCredentialsForConfigDir(configDir, { + platform: 'linux', + existsSyncImpl: () => false, + readFileSyncImpl: () => { + throw new Error('no file'); + }, + execFileImpl: () => { + keychainCalled = true; + return ''; + }, + }); + expect(creds).toBeNull(); + expect(keychainCalled).toBe(false); + }); +}); diff --git a/tests/unit/web-server/native-quota-collector.test.ts b/tests/unit/web-server/native-quota-collector.test.ts index 9c24fa0f..add60779 100644 --- a/tests/unit/web-server/native-quota-collector.test.ts +++ b/tests/unit/web-server/native-quota-collector.test.ts @@ -1032,7 +1032,7 @@ function makeMultiProfileDeps(opts: { listCodexProfiles: () => codexProfiles, defaultClaudeProfile: () => claudeDefault, defaultCodexProfile: () => codexDefault, - // Credential seams (file-only, no keychain) + // Credential seams (fully injected; no real filesystem or Keychain access) readClaudeCredentialsForProfile: credsForProfile, readCodexNativeAuth: codexNativeAuth, // Fetch seams @@ -1170,8 +1170,10 @@ describe('multi-profile: account_id and wire fields', () => { const rows = await getNativeAccountRows(deps); expect(rows.length).toBe(claudeProfiles.length + codexProfiles.length); - expect(deps.claudeFetchCount()).toBe(1); - expect(deps.codexNetworkCount()).toBe(1); + // Budget per pass: the default + one rotating non-default live slot per + // surface — constant regardless of profile count. + expect(deps.claudeFetchCount()).toBe(2); + expect(deps.codexNetworkCount()).toBe(2); }); it('rows are sorted by (surface, profile)', async () => { @@ -1193,7 +1195,7 @@ describe('multi-profile: account_id and wire fields', () => { }); }); -describe('multi-profile: Claude file-only reader', () => { +describe('multi-profile: Claude credential reader', () => { it('profile with .credentials.json present -> live fetch row (paused:false when default)', async () => { const clock = { now: 1_000_000 }; const deps = makeMultiProfileDeps({ @@ -1396,8 +1398,9 @@ describe('multi-profile: per-profile circuit breaker isolation', () => { const ckRow = rows.find((r) => r.profile === 'ck'); const workRow = rows.find((r) => r.profile === 'work'); - // 'ck' stays cache-only, independent of work's breaker. - expect(ckRow?.quotaStatus).toBe('unsupported'); + // 'ck' gets its own live row via the rotating slot, independent of work's + // breaker history. + expect(ckRow?.quotaStatus).toBe('ok'); // 'work' is also fine after reset (no breaker state). expect(workRow?.quotaStatus).toBe('ok'); }); @@ -1431,10 +1434,11 @@ describe('multi-profile: per-profile circuit breaker isolation', () => { }, }); - // First call: 'work' gets a 429, 'ck' remains cache-only. + // First call: 'work' gets a 429; 'ck' is refreshed by the rotating slot and + // succeeds — work's failures do not leak into ck's state. const rows1 = await getNativeAccountRows(deps); const ck1 = rows1.find((r) => r.profile === 'ck'); - expect(ck1?.quotaStatus).toBe('unsupported'); + expect(ck1?.quotaStatus).toBe('ok'); expect(workCall429Count).toBeGreaterThanOrEqual(1); // Skip past cooldown for 'work' only; 'ck' is within TTL. @@ -1443,8 +1447,8 @@ describe('multi-profile: per-profile circuit breaker isolation', () => { // Second call past 'work' cooldown: work tries again (429 again); ck cached. const rows2 = await getNativeAccountRows(deps); const ck2 = rows2.find((r) => r.profile === 'ck'); - // 'ck' remains cache-only and is not affected by work's breaker. - expect(ck2?.quotaStatus).toBe('unsupported'); + // 'ck' keeps its healthy cached row and is not affected by work's breaker. + expect(ck2?.quotaStatus).toBe('ok'); }); }); @@ -1537,15 +1541,18 @@ describe('review focus areas: reauth caching + codex local fallback', () => { codexNetworkFetch: async () => ({ success: false, needsReauth: true }) as CodexQuotaResult, }); + // The rotating slot polls 'ck' once; the 401 parks it into the reauth + // cooldown. const first = await getNativeAccountRows(deps); const r1 = first.find((r) => r.profile === 'ck'); expect(r1?.needsReauth).toBe(true); expect(r1?.paused).toBe(true); - expect(deps.codexNetworkCount()).toBe(0); + expect(deps.codexNetworkCount()).toBe(1); + // Within the cooldown it is NOT re-polled (no repeated 401s), even forced. const second = await getNativeAccountRows(deps, { force: true }); expect(second.find((r) => r.profile === 'ck')?.cached).toBe(true); - expect(deps.codexNetworkCount()).toBe(0); + expect(deps.codexNetworkCount()).toBe(1); }); it('Codex named profile without on-disk auth is parked, never filled from global local data', async () => { @@ -1655,7 +1662,7 @@ describe('review focus areas: reauth caching + codex local fallback', () => { expect(deps.claudeFetchCount()).toBe(1); // re-checked -> fetched }); - it('Codex named profile with valid auth but sparse payload stays parked when not default', async () => { + it('Codex named profile with valid auth but sparse payload yields an active quota-less row', async () => { resetNativeQuotaState(); const clock = { now: 7_000_000 }; const deps = makeMultiProfileDeps({ @@ -1668,13 +1675,15 @@ describe('review focus areas: reauth caching + codex local fallback', () => { codexNetworkFetch: async () => ({ success: true }) as CodexQuotaResult, }); + // The rotating slot polls 'ck'; the token authenticated, so it is a valid + // active subscription with a sparse payload — an active quota-less row, + // still dimmed because it is not the default profile. const rows = await getNativeAccountRows(deps); const ck = rows.find((r) => r.profile === 'ck'); expect(ck).toBeDefined(); - expect(ck?.paused).toBe(true); // cache-only, parked until selected as default - expect(ck?.needsReauth).toBe(true); - expect(ck?.quotaStatus).toBe('unsupported'); - expect(ck?.quota_percentage).toBeNull(); // no windows while parked + expect(ck?.paused).toBe(true); // non-default rows always render dimmed + expect(ck?.needsReauth).toBe(false); + expect(ck?.quota_percentage).toBeNull(); // no windows in the payload }); it('non-default cache-only rows do not overwrite the canonical live cache', async () => { @@ -1691,9 +1700,11 @@ describe('review focus areas: reauth caching + codex local fallback', () => { }); deps.defaultCodexProfile = () => codexDefault; + // First pass: 'ck' (default) is live-polled and the rotating slot also + // refreshes 'default' — two upstream calls. const first = await getNativeAccountRows(deps); expect(first.find((r) => r.profile === 'ck')?.paused).toBe(false); - expect(deps.codexNetworkCount()).toBe(1); + expect(deps.codexNetworkCount()).toBe(2); codexDefault = 'default'; const second = await getNativeAccountRows(deps); @@ -1710,3 +1721,234 @@ describe('review focus areas: reauth caching + codex local fallback', () => { expect(deps.codexNetworkCount()).toBe(2); }); }); + +// ============================================================================ +// Multi-profile: rotating live slot for non-default profiles +// +// Non-default profiles used to be cache-only forever, so accounts other than +// the default never showed real quota — they sat parked ("needs re-auth") for +// the lifetime of the server. One rotating live slot per surface refreshes the +// stalest eligible non-default profile per pass, so every account converges to +// real data within a few polls while the per-pass upstream budget stays +// constant (<= 2 calls per surface) regardless of profile count. +// ============================================================================ + +describe('multi-profile: rotating live slot for non-default profiles', () => { + it('live-polls the default plus one stale non-default Claude profile per pass', async () => { + const clock = { now: 1_000_000 }; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work', 'personal', 'fc'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async () => successQuota(), + }); + + // Pass 1: default (work) + one stale non-default get live data. + let rows = await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(2); + expect(rows.filter((r) => r.quotaStatus === 'ok').length).toBe(2); + + // Pass 2 after the parked TTL: the remaining profile gets its live row. + clock.now += 31_000; + rows = await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(3); + expect(rows.filter((r) => r.quotaStatus === 'ok').length).toBe(3); + + // Pass 3 while everything is within TTL: fully cached, zero upstream calls. + clock.now += 1_000; + rows = await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(3); + expect(rows.filter((r) => r.quotaStatus === 'ok').length).toBe(3); + }); + + it('rotated non-default rows keep paused:true (only the default renders active)', async () => { + const clock = { now: 1_000_000 }; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work', 'personal'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async () => successQuota(), + }); + + const rows = await getNativeAccountRows(deps); + const personal = rows.find((r) => r.profile === 'personal'); + expect(personal?.quotaStatus).toBe('ok'); + expect(personal?.needsReauth).toBe(false); + expect(personal?.paused).toBe(true); + expect(personal?.is_default).toBe(false); + }); + + it('codex non-default profiles also get one rotating live slot per pass', async () => { + const clock = { now: 1_000_000 }; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: [], + codexProfiles: ['personal', 'ck'], + codexDefault: 'personal', + codexNativeAuth: (p) => ({ accessToken: `tok-${p}`, accountId: `id-${p}` }), + codexNetworkFetch: async () => codexSuccessQuota(), + }); + + const rows = await getNativeAccountRows(deps); + expect(deps.codexNetworkCount()).toBe(2); + const ck = rows.find((r) => r.profile === 'ck'); + expect(ck?.paused).toBe(true); + expect(ck?.needsReauth).toBe(false); + }); + + it('profiles in reauth cooldown are skipped by the rotating slot', async () => { + const clock = { now: 1_000_000 }; + let failProfile: string | null = 'personal'; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work', 'personal'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async (_token: string, accountId?: string) => + accountId === `ccs:${failProfile}` + ? ({ success: false, needsReauth: true, retryable: false } as ClaudeQuotaResult) + : successQuota(), + }); + + // Pass 1: personal is rotated in, 401s, and enters the reauth cooldown. + let rows = await getNativeAccountRows(deps); + expect(rows.find((r) => r.profile === 'personal')?.needsReauth).toBe(true); + expect(deps.claudeFetchCount()).toBe(2); + + // Pass 2 inside the cooldown: the slot must NOT re-poll (and re-401) it. + clock.now += 31_000; + rows = await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(2); + expect(rows.find((r) => r.profile === 'personal')?.needsReauth).toBe(true); + }); +}); + +// ============================================================================ +// Quota reset invalidates cached rows +// +// A cached row whose next_reset has passed no longer describes the current +// window — the quota snapped back at the reset boundary. Serving it for the +// rest of the 10-min TTL makes the bar visibly wrong right after a reset, so +// a passed reset marks the row stale (guarded: only when the row was fetched +// BEFORE the reset, so a post-reset payload that still reports a past reset +// cannot cause a refetch loop). +// ============================================================================ + +describe('multi-profile: quota reset invalidates cached rows', () => { + function quotaResettingAt(resetIso: string): ClaudeQuotaResult { + const base = successQuota(); + return { + ...base, + coreUsage: { + fiveHour: { ...base.coreUsage!.fiveHour!, resetAt: resetIso }, + weekly: base.coreUsage!.weekly, + }, + }; + } + + it('re-fetches the default Claude profile once its next_reset passes, before TTL expiry', async () => { + const clock = { now: Date.parse('2026-06-09T10:00:00.000Z') }; + const resetIso = '2026-06-09T10:01:00.000Z'; // 60s ahead + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async () => quotaResettingAt(resetIso), + }); + + await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(1); + + // 90s later: past the reset but far inside the 10-min TTL. + clock.now += 90_000; + await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(2); + }); + + it('does not refetch-loop when a post-reset payload still reports a past reset', async () => { + const clock = { now: Date.parse('2026-06-09T10:00:00.000Z') }; + const resetIso = '2026-06-09T10:01:00.000Z'; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async () => quotaResettingAt(resetIso), + }); + + await getNativeAccountRows(deps); + clock.now += 90_000; + await getNativeAccountRows(deps); // refetch fires; payload STILL says 10:01 + expect(deps.claudeFetchCount()).toBe(2); + + // Another pass within TTL: the row was fetched after the reset passed, so + // the stale-by-reset rule must not apply again. + clock.now += 30_000; + await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(2); + }); + + it('rotating slot treats a non-default profile with a passed reset as stale', async () => { + const clock = { now: Date.parse('2026-06-09T10:00:00.000Z') }; + const resetIso = '2026-06-09T10:01:00.000Z'; + const farFuture = '2026-06-16T00:00:00.000Z'; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: ['work', 'personal'], + codexProfiles: [], + claudeDefault: 'work', + credsForProfile: () => maxCreds(), + claudeFetch: async (_t, accountId) => + quotaResettingAt(accountId === 'ccs:personal' ? resetIso : farFuture), + }); + + // Pass 1: work (default) + personal (rotating slot, never fetched). + await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(2); + + // 90s later: personal's reset passed; work is fresh. The rotating slot + // must pick personal again even though its row is inside the 10-min TTL. + clock.now += 90_000; + await getNativeAccountRows(deps); + expect(deps.claudeFetchCount()).toBe(3); + }); + + it('Codex rows honour the same reset-staleness rule', async () => { + const clock = { now: Date.parse('2026-06-09T10:00:00.000Z') }; + const codexQuota: CodexQuotaResult = { + ...codexSuccessQuota(), + coreUsage: { + fiveHour: { + label: 'Primary', + remainingPercent: 60, + resetAfterSeconds: 60, + resetAt: '2026-06-09T10:01:00.000Z', + }, + weekly: codexSuccessQuota().coreUsage!.weekly, + }, + }; + const deps = makeMultiProfileDeps({ + clock, + claudeProfiles: [], + codexProfiles: ['personal'], + codexDefault: 'personal', + codexNativeAuth: (p) => ({ accessToken: `tok-${p}`, accountId: `id-${p}` }), + codexNetworkFetch: async () => codexQuota, + }); + + await getNativeAccountRows(deps); + expect(deps.codexNetworkCount()).toBe(1); + + clock.now += 90_000; + await getNativeAccountRows(deps); + expect(deps.codexNetworkCount()).toBe(2); + }); +});