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 a400fe1f..adc7a627 100644 --- a/src/commands/bar/bar-server-probe.ts +++ b/src/commands/bar/bar-server-probe.ts @@ -25,6 +25,10 @@ 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, falling back to the --port * in launch.json's args. `ccs bar stop` deletes bar.json but leaves @@ -38,7 +42,7 @@ export function resolveBarPort(ccsDir: string): number | null { try { const raw = fs.readFileSync(barJsonPath, 'utf8'); const parsed = JSON.parse(raw) as Partial<{ port: number }>; - if (typeof parsed.port === 'number') return parsed.port; + if (isValidPort(parsed.port)) return parsed.port; } catch { /* fall through to launch.json */ } @@ -50,8 +54,11 @@ export function resolveBarPort(ccsDir: string): number | null { const args = Array.isArray(parsed.args) ? parsed.args : []; const idx = args.indexOf('--port'); if (idx !== -1 && idx + 1 < args.length) { - const n = parseInt(String(args[idx + 1]), 10); - if (Number.isFinite(n) && n > 0 && n < 65536) return n; + 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 */ @@ -102,18 +109,18 @@ export async function defaultFindRunningServer(ccsDir: string): Promise { const subcommand = args[0]; @@ -59,7 +60,15 @@ export async function handleBarCommand(args: string[]): Promise { // 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('-')) { - await commandHandlers.launch(subcommand === 'launch' ? args.slice(1) : args); + 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-subcommand.ts b/src/commands/bar/launch-subcommand.ts index 63e9686b..4952cd39 100644 --- a/src/commands/bar/launch-subcommand.ts +++ b/src/commands/bar/launch-subcommand.ts @@ -32,21 +32,16 @@ import { isMatchingBarAuthProof, getOrCreateBarAuthToken, } from '../../utils/bar-auth-token'; -import { - getBarDir, - getBarJsonPath, - getLaunchJsonPath, - getServeLogPath, - getServerPidPath, -} from './bar-paths'; +import { getBarDir, getBarJsonPath, getLaunchJsonPath, getServeLogPath } from './bar-paths'; import type { LaunchJson } from './bar-paths'; import { createBarLaunchDescriptor } from './launch-descriptor'; -import { parsePortFlag } from './port-arg'; +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; @@ -191,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) }, @@ -221,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; } @@ -243,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,44 +249,6 @@ function defaultWriteLaunchDescriptor(jsonPath: string, descriptor: LaunchJson): fs.writeFileSync(jsonPath, JSON.stringify(descriptor, null, 2)); } -/** - * SIGTERM the detached server from server.pid, then poll until the process is - * gone (up to ~3 s) so the port is free before we bind the replacement. - * Silently no-ops when no pid file exists or the process is already gone. - */ -async function defaultStopDetachedServer(ccsDir: string): Promise { - const pidPath = getServerPidPath(ccsDir); - let pid: number; - try { - pid = parseInt(fs.readFileSync(pidPath, 'utf8').trim(), 10); - } catch { - return; - } - if (!Number.isFinite(pid) || pid <= 0) return; - - try { - process.kill(pid, 'SIGTERM'); - } catch { - /* already gone */ - } - - const deadline = Date.now() + 3_000; - while (Date.now() < deadline) { - try { - process.kill(pid, 0); // still alive - } catch { - break; // exited - } - await new Promise((resolve) => setTimeout(resolve, 100)); - } - - try { - fs.unlinkSync(pidPath); - } catch { - /* may already be gone */ - } -} - async function defaultOpenApp(appPath: string): Promise { const { execFile } = await import('child_process'); const { promisify } = await import('util'); @@ -318,6 +271,12 @@ 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; @@ -326,7 +285,7 @@ export async function handleBarLaunch( const waitForServerLive = deps.waitForServerLive ?? defaultWaitForServerLive; const createLaunchDescriptor = deps.createLaunchDescriptor ?? createBarLaunchDescriptor; const writeLaunchDescriptor = deps.writeLaunchDescriptor ?? defaultWriteLaunchDescriptor; - const stopDetachedServer = deps.stopDetachedServer ?? defaultStopDetachedServer; + const stopDetachedServer = deps.stopDetachedServer ?? stopDetachedBarServer; // Wire findRunningServer after ccsDir is resolved. const findRunningServer = deps.findRunningServer ?? (() => _defaultFindRunningServer(ccsDir)); @@ -352,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( @@ -384,17 +346,38 @@ export async function handleBarLaunch( return; } - // Explicit --port that differs from the running server: move the server. + // 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(`[!] Could not stop the running server: ${msg}`); - console.error('[i] Run `ccs bar stop` manually, then retry.'); + 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. } @@ -404,9 +387,10 @@ export async function handleBarLaunch( // 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). - let port: number; try { - if (requestedPort !== null) { + 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.`); @@ -428,28 +412,40 @@ export async function handleBarLaunch( return; } - // 2b. Write/refresh launch.json so the Swift app can self-start next time — - // on the same port this launch chose. - try { - const launchDescriptor = createLaunchDescriptor({ port }); - 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; } @@ -467,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 index 9cc1640d..f52f6443 100644 --- a/src/commands/bar/port-arg.ts +++ b/src/commands/bar/port-arg.ts @@ -13,11 +13,28 @@ export interface PortFlag { 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]; - const n = raw === undefined ? NaN : parseInt(raw, 10); - const valid = Number.isFinite(n) && n > 0 && n < 65536; + 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 a7ed3d40..ee47f681 100644 --- a/src/commands/bar/serve-subcommand.ts +++ b/src/commands/bar/serve-subcommand.ts @@ -17,9 +17,15 @@ import * as path from 'path'; import { getCcsDir } from '../../config/config-loader-facade'; import { getBarJsonPath, getServerPidPath } from './bar-paths'; import { defaultFindRunningServer, resolveBarPort } from './bar-server-probe'; -import { parsePortFlag } from './port-arg'; +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 @@ -45,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; } // --------------------------------------------------------------------------- @@ -74,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); } @@ -99,14 +101,22 @@ function defaultGetCcsDir(): string { // --------------------------------------------------------------------------- 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); @@ -141,7 +151,13 @@ export async function handleBarServe(args: string[], deps: Partial = // Honor --port N from the launcher (it pre-selected via getPort to avoid races). // 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 requestedPort = parsePortFlag(args).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; @@ -165,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}`); @@ -192,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/tests/unit/commands/bar-command.test.ts b/tests/unit/commands/bar-command.test.ts index 5d12cab0..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 }); @@ -3798,6 +3837,104 @@ describe('launch: --port selects the server port', () => { 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 }); 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',