diff --git a/.changeset/external-capability-loading.md b/.changeset/external-capability-loading.md new file mode 100644 index 000000000..8564320b6 --- /dev/null +++ b/.changeset/external-capability-loading.md @@ -0,0 +1,9 @@ +--- +'@bradygaster/squad-cli': minor +--- + +feat(watch): load external WatchCapabilities from .squad/capabilities/ + +Users can now define custom watch capabilities as .js files in `.squad/capabilities/`. +Each file default-exports a WatchCapability object (name, phase, preflight, execute). +Capabilities are loaded at watch startup and participate in the normal phase-based round cycle. diff --git a/.changeset/pid-tracker-cleanup.md b/.changeset/pid-tracker-cleanup.md new file mode 100644 index 000000000..eaf5c8dad --- /dev/null +++ b/.changeset/pid-tracker-cleanup.md @@ -0,0 +1,9 @@ +--- +'@bradygaster/squad-cli': patch +--- + +fix(watch): track child process PIDs and cleanup orphans on exit + +Prevents MCP server process leaks during long-running watch sessions. +Tracks PIDs of spawned copilot sessions, kills them on exit/crash, +and cleans up stale orphans from previous crashed runs on startup. diff --git a/.gitignore b/.gitignore index 971f14978..0eeb162c3 100644 --- a/.gitignore +++ b/.gitignore @@ -22,7 +22,8 @@ docs/src/content/docs/reference/api/ docs/tests/screenshots/ # Squad: SubSquad activation file (local to this machine) .squad-workstream -.squad/.first-run +.squad/.first-run +.squad/.watch-pids # Images folder (root only — don't ignore docs/public/images/) /images/ diff --git a/packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts b/packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts index 8954c6480..0c4171a0b 100644 --- a/packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts +++ b/packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts @@ -152,7 +152,7 @@ async function executeAll( const { cmd, args } = buildAgentCommand(prompt, context); return new Promise<{ success: boolean; error?: string }>((resolve) => { - const _cp: ChildProcess = execFile( + const cp: ChildProcess = execFile( cmd, args, { cwd: context.teamRoot, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024 }, @@ -166,6 +166,18 @@ async function executeAll( } }, ); + + // Track child PID for cleanup on exit/crash + if (context.pidTracker && cp.pid) { + const issueNums = issues.map(i => `#${i.number}`).join(','); + context.pidTracker.track(cp.pid, `copilot-session-${issueNums}`); + } + + cp.on('exit', () => { + if (context.pidTracker && cp.pid) { + context.pidTracker.untrack(cp.pid); + } + }); }); } diff --git a/packages/squad-cli/src/cli/commands/watch/external-loader.ts b/packages/squad-cli/src/cli/commands/watch/external-loader.ts new file mode 100644 index 000000000..8f1fb63fd --- /dev/null +++ b/packages/squad-cli/src/cli/commands/watch/external-loader.ts @@ -0,0 +1,121 @@ +/** + * External capability loader — scans .squad/capabilities/ for user-defined + * WatchCapability modules and registers them alongside built-in capabilities. + */ + +import { existsSync } from 'node:fs'; +import { readdir } from 'node:fs/promises'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import type { CapabilityRegistry } from './registry.js'; +import type { WatchCapability } from './types.js'; +import { GREEN, YELLOW, RED, BOLD, DIM, RESET } from '../../core/output.js'; + +/** Required fields on a valid WatchCapability. */ +const REQUIRED_FIELDS: ReadonlyArray = [ + 'name', + 'description', + 'configShape', + 'requires', + 'phase', + 'preflight', + 'execute', +]; + +/** + * Load external WatchCapability modules from `{teamRoot}/.squad/capabilities/`. + * + * - Skips silently when the directory does not exist. + * - Logs a warning and continues when a file fails to load. + * - Returns the count of successfully loaded capabilities. + */ +export async function loadExternalCapabilities( + teamRoot: string, + registry: CapabilityRegistry, +): Promise { + const capDir = join(teamRoot, '.squad', 'capabilities'); + + if (!existsSync(capDir)) { + return 0; + } + + const entries = await readdir(capDir); + const jsFiles = entries.filter(f => f.endsWith('.js')).sort(); + + if (jsFiles.length === 0) { + return 0; + } + + // Security: warn user before executing external code + console.log( + `\n${YELLOW}${BOLD}⚠️ SECURITY: Loading ${jsFiles.length} external capability file(s) from .squad/capabilities/${RESET}`, + ); + for (const f of jsFiles) { + console.log(`${DIM} • ${f}${RESET}`); + } + console.log( + `${YELLOW} External capabilities execute with FULL process permissions (file system, network, credentials).${RESET}`, + ); + console.log( + `${YELLOW} Review these files if you did not author them.${RESET}\n`, + ); + + let loaded = 0; + + for (const filename of jsFiles) { + const filePath = join(capDir, filename); + try { + const mod = await import(pathToFileURL(filePath).href); + let cap: WatchCapability = mod.default ?? mod; + + // Support class-based capabilities with a no-arg constructor + if (typeof cap === 'function') { + cap = new (cap as new () => WatchCapability)(); + } + + // Validate required fields + const missing = REQUIRED_FIELDS.filter(f => !(f in cap)); + if (missing.length > 0) { + console.log( + `${YELLOW}⚠️ Failed to load capability from ${filename}: missing fields: ${missing.join(', ')}${RESET}`, + ); + continue; + } + + // Validate field types + if (typeof cap.preflight !== 'function' || typeof cap.execute !== 'function') { + console.log( + `${YELLOW}⚠️ Failed to load capability from ${filename}: preflight and execute must be functions${RESET}`, + ); + continue; + } + const validPhases = ['pre-scan', 'post-triage', 'post-execute', 'housekeeping']; + if (!validPhases.includes(cap.phase)) { + console.log( + `${YELLOW}⚠️ Failed to load capability from ${filename}: invalid phase '${cap.phase}' (must be one of: ${validPhases.join(', ')})${RESET}`, + ); + continue; + } + + // Security: block hijacking of built-in capabilities + if (registry.get(cap.name)) { + console.log( + `${YELLOW}⚠️ External capability "${cap.name}" conflicts with built-in — skipped (potential hijack)${RESET}`, + ); + continue; + } + + registry.register(cap); + console.log( + `${GREEN}✅ Loaded external capability: ${cap.name} (phase: ${cap.phase})${RESET}`, + ); + loaded++; + } catch (err) { + console.log( + `${YELLOW}⚠️ Failed to load capability from ${filename}: ${err instanceof Error ? err.message : String(err)}${RESET}`, + ); + } + } + + return loaded; +} diff --git a/packages/squad-cli/src/cli/commands/watch/index.ts b/packages/squad-cli/src/cli/commands/watch/index.ts index 2f38b478f..97e412062 100644 --- a/packages/squad-cli/src/cli/commands/watch/index.ts +++ b/packages/squad-cli/src/cli/commands/watch/index.ts @@ -50,6 +50,8 @@ export type { WatchCapability, WatchContext, WatchPhase, PreflightResult, Capabi export { CapabilityRegistry } from './registry.js'; export { createDefaultRegistry } from './capabilities/index.js'; export { createVerboseLogger, type VerboseLogger } from './verbose.js'; +export { loadExternalCapabilities } from './external-loader.js'; +export { PidTracker, type TrackedProcess } from './pid-tracker.js'; export { getWatchHealth, writePidFile, removePidFile, getPidPath, isProcessAlive, type WatchPidInfo } from './health.js'; // ── Watch Platform Abstraction ─────────────────────────────────── @@ -751,6 +753,24 @@ export async function runWatch(dest: string, options: WatchOptions | WatchConfig // ── Capability system setup ──────────────────────────────────── const registry = createDefaultRegistry(); + + // Load external capabilities from .squad/capabilities/ + const { loadExternalCapabilities } = await import('./external-loader.js'); + await loadExternalCapabilities(teamRoot, registry); + + // PID tracking for child process cleanup + const { PidTracker } = await import('./pid-tracker.js'); + const pidTracker = new PidTracker(teamRoot); + + // Clean up orphans from previous crashed run + const staleKilled = pidTracker.cleanupStale(); + if (staleKilled > 0) { + console.log(`${YELLOW}⚠️ Cleaned up ${staleKilled} orphaned process(es) from previous run${RESET}`); + } + + // Register exit handlers for graceful cleanup + pidTracker.registerExitHandlers(); + const baseContext: WatchContext = { teamRoot, adapter, @@ -760,6 +780,7 @@ export async function runWatch(dest: string, options: WatchOptions | WatchConfig agentCmd: config.agentCmd, copilotFlags: config.copilotFlags, verbose: config.verbose, + pidTracker, }; const enabledCapabilities = await preflightCapabilities(registry, config, baseContext); diff --git a/packages/squad-cli/src/cli/commands/watch/pid-tracker.ts b/packages/squad-cli/src/cli/commands/watch/pid-tracker.ts new file mode 100644 index 000000000..98b189215 --- /dev/null +++ b/packages/squad-cli/src/cli/commands/watch/pid-tracker.ts @@ -0,0 +1,141 @@ +/** + * Tracks child process PIDs spawned during watch rounds. + * Enables cleanup of orphaned processes on exit or restart. + */ + +import { existsSync, readFileSync, writeFileSync, unlinkSync } from 'node:fs'; +import { join } from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { YELLOW, DIM, RESET } from '../../core/output.js'; + +export interface TrackedProcess { + pid: number; + name: string; + spawnedAt: string; // ISO 8601 +} + +export class PidTracker { + private readonly pidFile: string; + private tracked: TrackedProcess[] = []; + + constructor(teamRoot: string) { + this.pidFile = join(teamRoot, '.squad', '.watch-pids'); + } + + /** Add a child PID to tracking. */ + track(pid: number, name: string): void { + this.tracked.push({ pid, name, spawnedAt: new Date().toISOString() }); + this.save(); + } + + /** Remove a PID from tracking (process exited normally). */ + untrack(pid: number): void { + this.tracked = this.tracked.filter(p => p.pid !== pid); + this.save(); + } + + /** Check if a process is still alive. */ + private isAlive(pid: number): boolean { + try { + process.kill(pid, 0); // signal 0 = existence check, doesn't kill + return true; + } catch { + return false; + } + } + + /** Kill a process tree. Cross-platform. */ + private killTree(pid: number): boolean { + try { + if (process.platform === 'win32') { + execFileSync('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore', timeout: 5000 }); + } else { + // Note: We only kill the direct PID. Grandchild cleanup requires + // spawning with detached:true, which is controlled by the Execute + // capability. For now, direct PID kill is the honest approach. + process.kill(pid, 'SIGKILL'); + } + return true; + } catch { + return false; + } + } + + /** Kill all tracked processes and clean up the PID file. */ + cleanup(): { killed: number; total: number } { + let killed = 0; + const total = this.tracked.length; + + for (const proc of this.tracked) { + if (this.isAlive(proc.pid)) { + if (this.killTree(proc.pid)) { + killed++; + } + } + } + + this.tracked = []; + this.deletePidFile(); + return { killed, total }; + } + + /** On startup: check for stale PID file from a previous crashed run. + * Kill any orphans that are still alive. */ + cleanupStale(): number { + if (!existsSync(this.pidFile)) return 0; + + let staleKilled = 0; + try { + const content = readFileSync(this.pidFile, 'utf-8'); + const stale: TrackedProcess[] = JSON.parse(content); + + for (const proc of stale) { + if (this.isAlive(proc.pid)) { + if (this.killTree(proc.pid)) { + staleKilled++; + console.log(`${YELLOW}⚠️ Killed orphaned process: ${proc.name} (PID ${proc.pid}, spawned ${proc.spawnedAt})${RESET}`); + } + } + } + } catch { + // Corrupted PID file — just delete it + } + + this.deletePidFile(); + return staleKilled; + } + + /** Register process exit handlers for cleanup. */ + registerExitHandlers(): void { + const doCleanup = () => { + const result = this.cleanup(); + if (result.killed > 0) { + // Can't use console.log in exit handler reliably, but try + try { + console.log(`${DIM}Cleaned up ${result.killed} child process(es)${RESET}`); + } catch { /* ignore */ } + } + }; + + process.on('exit', doCleanup); + process.on('SIGINT', () => { doCleanup(); process.exit(130); }); + process.on('SIGTERM', () => { doCleanup(); process.exit(143); }); + process.on('uncaughtException', (err) => { + console.error('Uncaught exception:', err); + doCleanup(); + process.exit(1); + }); + } + + private save(): void { + try { + writeFileSync(this.pidFile, JSON.stringify(this.tracked, null, 2), 'utf-8'); + } catch { /* ignore — best effort */ } + } + + private deletePidFile(): void { + try { + if (existsSync(this.pidFile)) unlinkSync(this.pidFile); + } catch { /* ignore */ } + } +} diff --git a/packages/squad-cli/src/cli/commands/watch/types.ts b/packages/squad-cli/src/cli/commands/watch/types.ts index b41e76bc2..05ee68411 100644 --- a/packages/squad-cli/src/cli/commands/watch/types.ts +++ b/packages/squad-cli/src/cli/commands/watch/types.ts @@ -39,6 +39,8 @@ export interface WatchContext { copilotFlags?: string; /** Verbose diagnostic output enabled. */ verbose?: boolean; + /** PID tracker for child process cleanup (optional — only set when watch is running). */ + pidTracker?: { track(pid: number, name: string): void; untrack(pid: number): void }; } /** Contract that every watch capability must implement. */ diff --git a/packages/squad-cli/src/cli/templates/capabilities/issue-pipeline.sample.js b/packages/squad-cli/src/cli/templates/capabilities/issue-pipeline.sample.js new file mode 100644 index 000000000..4435a605b --- /dev/null +++ b/packages/squad-cli/src/cli/templates/capabilities/issue-pipeline.sample.js @@ -0,0 +1,40 @@ +/** + * Sample: Issue Pipeline capability + * + * Copy to .squad/capabilities/issue-pipeline.js to use. + * Enforces: issue → branch → implement → PR → review → merge + * + * Each LLM invocation is a fresh copilot session — no drift. + */ + +export default { + name: 'issue-pipeline', + description: 'Deterministic issue→branch→PR→review→merge pipeline', + configShape: 'object', + requires: ['gh'], + phase: 'post-triage', + + async preflight(context) { + // Check gh CLI is available + try { + const { execFileSync } = await import('node:child_process'); + execFileSync('gh', ['--version'], { stdio: 'ignore' }); + return { ok: true }; + } catch { + return { ok: false, reason: 'gh CLI not available' }; + } + }, + + async execute(context) { + // This is a reference implementation — customize for your workflow + const { execFileSync } = await import('node:child_process'); + + // Find issues that are triaged but don't have a PR yet + // (Real implementation would check for squad labels, existing PRs, etc.) + + return { + success: true, + summary: 'Issue pipeline: checked for actionable issues (sample — customize for your workflow)', + }; + }, +}; diff --git a/test-fixtures/init-test/.gitignore b/test-fixtures/init-test/.gitignore new file mode 100644 index 000000000..bede03212 --- /dev/null +++ b/test-fixtures/init-test/.gitignore @@ -0,0 +1,2 @@ +# Squad: SubSquad activation file (local to this machine) +.squad-workstream diff --git a/test/cli/watch-external-loader.test.ts b/test/cli/watch-external-loader.test.ts new file mode 100644 index 000000000..b6cc8d655 --- /dev/null +++ b/test/cli/watch-external-loader.test.ts @@ -0,0 +1,240 @@ +/** + * External capability loader tests — issue #918 + * + * Verifies loadExternalCapabilities handles: + * - missing directory + * - empty directory + * - valid capability files + * - invalid files (missing fields) + * - files with syntax errors + * - multiple files (mix of valid/invalid) + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtemp, writeFile, rm } from 'node:fs/promises'; +import { existsSync, mkdirSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { CapabilityRegistry } from '../../packages/squad-cli/src/cli/commands/watch/registry.js'; + +// Dynamic import to avoid hoisting issues — each test imports fresh +async function getLoader() { + const mod = await import( + '../../packages/squad-cli/src/cli/commands/watch/external-loader.js' + ); + return mod.loadExternalCapabilities; +} + +describe('loadExternalCapabilities', () => { + let tmpDir: string; + let registry: CapabilityRegistry; + let consoleSpy: ReturnType; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'squad-ext-cap-')); + registry = new CapabilityRegistry(); + consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + consoleSpy.mockRestore(); + if (existsSync(tmpDir)) { + await rm(tmpDir, { recursive: true, force: true }); + } + }); + + it('returns 0 when .squad/capabilities/ directory does not exist', async () => { + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(0); + expect(registry.all()).toHaveLength(0); + }); + + it('returns 0 for an empty capabilities directory', async () => { + mkdirSync(join(tmpDir, '.squad', 'capabilities'), { recursive: true }); + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(0); + expect(registry.all()).toHaveLength(0); + }); + + it('loads and registers a valid capability file', async () => { + const capDir = join(tmpDir, '.squad', 'capabilities'); + mkdirSync(capDir, { recursive: true }); + + const capCode = ` + export default { + name: 'test-cap', + description: 'A test capability', + configShape: 'boolean', + requires: [], + phase: 'housekeeping', + async preflight() { return { ok: true }; }, + async execute() { return { success: true, summary: 'done' }; }, + }; + `; + await writeFile(join(capDir, 'test-cap.js'), capCode, 'utf-8'); + + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(1); + expect(registry.names()).toContain('test-cap'); + + const cap = registry.get('test-cap'); + expect(cap).toBeDefined(); + expect(cap!.phase).toBe('housekeeping'); + expect(cap!.description).toBe('A test capability'); + + // Check success log + const successLog = consoleSpy.mock.calls.find( + call => typeof call[0] === 'string' && call[0].includes('Loaded external capability'), + ); + expect(successLog).toBeDefined(); + }); + + it('warns and returns 0 for a file missing required fields', async () => { + const capDir = join(tmpDir, '.squad', 'capabilities'); + mkdirSync(capDir, { recursive: true }); + + const badCode = ` + export default { + name: 'incomplete', + description: 'Missing phase, preflight, execute', + }; + `; + await writeFile(join(capDir, 'bad-cap.js'), badCode, 'utf-8'); + + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(0); + expect(registry.all()).toHaveLength(0); + + const warnLog = consoleSpy.mock.calls.find( + call => typeof call[0] === 'string' && call[0].includes('Failed to load capability'), + ); + expect(warnLog).toBeDefined(); + expect(warnLog![0]).toContain('missing fields'); + }); + + it('warns and continues for a file with a syntax error', async () => { + const capDir = join(tmpDir, '.squad', 'capabilities'); + mkdirSync(capDir, { recursive: true }); + + await writeFile(join(capDir, 'broken.js'), 'export default {{{', 'utf-8'); + + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(0); + expect(registry.all()).toHaveLength(0); + + const warnLog = consoleSpy.mock.calls.find( + call => typeof call[0] === 'string' && call[0].includes('Failed to load capability'), + ); + expect(warnLog).toBeDefined(); + }); + + it('loads all valid files and skips invalid ones', async () => { + const capDir = join(tmpDir, '.squad', 'capabilities'); + mkdirSync(capDir, { recursive: true }); + + // Valid capability #1 + const cap1 = ` + export default { + name: 'valid-one', + description: 'First valid capability', + configShape: 'boolean', + requires: [], + phase: 'pre-scan', + async preflight() { return { ok: true }; }, + async execute() { return { success: true, summary: 'ok' }; }, + }; + `; + await writeFile(join(capDir, 'valid-one.js'), cap1, 'utf-8'); + + // Invalid — missing fields + const bad = ` + export default { name: 'bad-one' }; + `; + await writeFile(join(capDir, 'bad-one.js'), bad, 'utf-8'); + + // Valid capability #2 + const cap2 = ` + export default { + name: 'valid-two', + description: 'Second valid capability', + configShape: 'object', + requires: ['gh'], + phase: 'post-execute', + async preflight() { return { ok: true }; }, + async execute() { return { success: true, summary: 'ok' }; }, + }; + `; + await writeFile(join(capDir, 'valid-two.js'), cap2, 'utf-8'); + + // Syntax error + await writeFile(join(capDir, 'syntax-err.js'), 'export default !!!', 'utf-8'); + + // Non-JS file — should be ignored + await writeFile(join(capDir, 'readme.md'), '# ignore me', 'utf-8'); + + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + expect(count).toBe(2); + expect(registry.names()).toContain('valid-one'); + expect(registry.names()).toContain('valid-two'); + expect(registry.names()).not.toContain('bad-one'); + expect(registry.all()).toHaveLength(2); + }); + + it('rejects external capability that conflicts with a built-in name', async () => { + const capDir = join(tmpDir, '.squad', 'capabilities'); + mkdirSync(capDir, { recursive: true }); + + // Pre-register a "built-in" capability + const builtIn = { + name: 'execute', + description: 'Built-in execute capability', + configShape: 'boolean' as const, + requires: [] as string[], + phase: 'post-execute' as const, + async preflight() { return { ok: true }; }, + async execute() { return { success: true, summary: 'built-in' }; }, + }; + registry.register(builtIn); + + // External file tries to hijack the 'execute' name + const hijack = ` + export default { + name: 'execute', + description: 'Malicious hijack', + configShape: 'boolean', + requires: [], + phase: 'post-execute', + async preflight() { return { ok: true }; }, + async execute() { return { success: true, summary: 'pwned' }; }, + }; + `; + await writeFile(join(capDir, 'hijack.js'), hijack, 'utf-8'); + + const loadExternalCapabilities = await getLoader(); + const count = await loadExternalCapabilities(tmpDir, registry); + + // Should be rejected — count is 0 + expect(count).toBe(0); + // Built-in should still be there, not replaced + const cap = registry.get('execute'); + expect(cap!.description).toBe('Built-in execute capability'); + + // Should have logged a warning + const warnLog = consoleSpy.mock.calls.find( + call => typeof call[0] === 'string' && call[0].includes('conflicts with built-in'), + ); + expect(warnLog).toBeDefined(); + }); +}); diff --git a/test/cli/watch-pid-tracker.test.ts b/test/cli/watch-pid-tracker.test.ts new file mode 100644 index 000000000..e883ef44c --- /dev/null +++ b/test/cli/watch-pid-tracker.test.ts @@ -0,0 +1,242 @@ +/** + * PID Tracker Tests (#921) + * + * Tests child process PID tracking, cleanup, stale PID file handling, + * and the PID file lifecycle. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; + +// Mock execFileSync so taskkill is never actually called +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + execFileSync: vi.fn(() => Buffer.from('')), + }; +}); + +import { execFileSync } from 'node:child_process'; +import { PidTracker, type TrackedProcess } from '../../packages/squad-cli/src/cli/commands/watch/pid-tracker.js'; + +/** Create a temp directory with a .squad subdirectory for testing. */ +function makeTempTeamRoot(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'squad-pid-test-')); + fs.mkdirSync(path.join(dir, '.squad'), { recursive: true }); + return dir; +} + +/** Read the PID file directly. */ +function readPidFile(teamRoot: string): TrackedProcess[] { + const pidPath = path.join(teamRoot, '.squad', '.watch-pids'); + if (!fs.existsSync(pidPath)) return []; + return JSON.parse(fs.readFileSync(pidPath, 'utf-8')); +} + +/** Write a PID file directly. */ +function writePidFile(teamRoot: string, entries: TrackedProcess[]): void { + const pidPath = path.join(teamRoot, '.squad', '.watch-pids'); + fs.writeFileSync(pidPath, JSON.stringify(entries, null, 2), 'utf-8'); +} + +/** Check if PID file exists. */ +function pidFileExists(teamRoot: string): boolean { + return fs.existsSync(path.join(teamRoot, '.squad', '.watch-pids')); +} + +describe('PidTracker', () => { + let teamRoot: string; + + beforeEach(() => { + teamRoot = makeTempTeamRoot(); + }); + + afterEach(() => { + fs.rmSync(teamRoot, { recursive: true, force: true }); + }); + + describe('track / untrack', () => { + it('tracks a PID and persists it to the PID file', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(12345, 'copilot-session-#1'); + + const entries = readPidFile(teamRoot); + expect(entries).toHaveLength(1); + expect(entries[0]!.pid).toBe(12345); + expect(entries[0]!.name).toBe('copilot-session-#1'); + expect(entries[0]!.spawnedAt).toBeTruthy(); + }); + + it('tracks multiple PIDs', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(100, 'proc-a'); + tracker.track(200, 'proc-b'); + tracker.track(300, 'proc-c'); + + const entries = readPidFile(teamRoot); + expect(entries).toHaveLength(3); + expect(entries.map(e => e.pid)).toEqual([100, 200, 300]); + }); + + it('untracks a PID and updates the PID file', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(100, 'proc-a'); + tracker.track(200, 'proc-b'); + tracker.untrack(100); + + const entries = readPidFile(teamRoot); + expect(entries).toHaveLength(1); + expect(entries[0]!.pid).toBe(200); + }); + + it('untrack is a no-op for unknown PIDs', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(100, 'proc-a'); + tracker.untrack(999); // doesn't exist + + const entries = readPidFile(teamRoot); + expect(entries).toHaveLength(1); + }); + }); + + describe('cleanup', () => { + beforeEach(() => { + vi.mocked(execFileSync).mockClear(); + }); + + it('kills alive processes and reports count', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(111, 'alive-proc'); + tracker.track(222, 'dead-proc'); + + // Mock process.kill: 111 is alive, 222 is dead + const killSpy = vi.spyOn(process, 'kill').mockImplementation(((pid: number, signal?: string | number) => { + if (signal === 0) { + // Existence check — 111 is alive, 222 is dead + if (Math.abs(pid) === 111) return true; + throw new Error('ESRCH'); + } + // Actual kill — succeed for alive process + return true; + }) as typeof process.kill); + + const result = tracker.cleanup(); + + expect(result.total).toBe(2); + // On non-Windows, process.kill is used for killing, so 111 should be killed + if (process.platform !== 'win32') { + expect(result.killed).toBe(1); + } + expect(pidFileExists(teamRoot)).toBe(false); + + killSpy.mockRestore(); + }); + + it('skips dead processes', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(333, 'dead-proc'); + + // Mock: all processes are dead + const killSpy = vi.spyOn(process, 'kill').mockImplementation((() => { + throw new Error('ESRCH'); + }) as typeof process.kill); + + const result = tracker.cleanup(); + expect(result.killed).toBe(0); + expect(result.total).toBe(1); + expect(pidFileExists(teamRoot)).toBe(false); + + killSpy.mockRestore(); + }); + + it('deletes PID file after cleanup', () => { + const tracker = new PidTracker(teamRoot); + tracker.track(444, 'some-proc'); + expect(pidFileExists(teamRoot)).toBe(true); + + // Mock all dead + const killSpy = vi.spyOn(process, 'kill').mockImplementation((() => { + throw new Error('ESRCH'); + }) as typeof process.kill); + + tracker.cleanup(); + expect(pidFileExists(teamRoot)).toBe(false); + + killSpy.mockRestore(); + }); + }); + + describe('cleanupStale', () => { + beforeEach(() => { + vi.mocked(execFileSync).mockClear(); + }); + + it('reads stale PID file and kills alive orphans', () => { + // Write a stale PID file as if from a previous crashed run + writePidFile(teamRoot, [ + { pid: 555, name: 'stale-proc', spawnedAt: '2025-01-01T00:00:00.000Z' }, + ]); + + const tracker = new PidTracker(teamRoot); + + // Mock: process 555 is alive + const killSpy = vi.spyOn(process, 'kill').mockImplementation(((pid: number, signal?: string | number) => { + if (signal === 0 && pid === 555) return true; // alive + if (signal === 0) throw new Error('ESRCH'); + return true; // kill succeeds + }) as typeof process.kill); + + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const staleKilled = tracker.cleanupStale(); + + // On Unix: process.kill(-pid, SIGKILL) or process.kill(pid, SIGKILL) + // On Windows: execSync taskkill + // Either way, it should attempt cleanup + expect(staleKilled).toBeGreaterThanOrEqual(0); // platform-dependent + expect(pidFileExists(teamRoot)).toBe(false); + + killSpy.mockRestore(); + consoleSpy.mockRestore(); + }); + + it('returns 0 when no PID file exists', () => { + const tracker = new PidTracker(teamRoot); + const result = tracker.cleanupStale(); + expect(result).toBe(0); + }); + + it('handles corrupted PID file gracefully', () => { + // Write garbage to the PID file + const pidPath = path.join(teamRoot, '.squad', '.watch-pids'); + fs.writeFileSync(pidPath, '<<>>', 'utf-8'); + + const tracker = new PidTracker(teamRoot); + const result = tracker.cleanupStale(); + + expect(result).toBe(0); + expect(pidFileExists(teamRoot)).toBe(false); + }); + + it('deletes PID file after processing stale entries', () => { + writePidFile(teamRoot, [ + { pid: 666, name: 'dead-orphan', spawnedAt: '2025-01-01T00:00:00.000Z' }, + ]); + + // Mock: process is dead + const killSpy = vi.spyOn(process, 'kill').mockImplementation((() => { + throw new Error('ESRCH'); + }) as typeof process.kill); + + const tracker = new PidTracker(teamRoot); + tracker.cleanupStale(); + + expect(pidFileExists(teamRoot)).toBe(false); + + killSpy.mockRestore(); + }); + }); +});