diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..a43d5ba --- /dev/null +++ b/.gitattributes @@ -0,0 +1,7 @@ +# Normalize all text files to LF in the repo +* text=auto eol=lf + +# Windows scripts use CRLF (required by CMD/BAT; conventional for .ps1) +*.ps1 text eol=crlf +*.cmd text eol=crlf +*.bat text eol=crlf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bdeac4..8769c71 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,11 @@ on: jobs: test: - runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -23,3 +27,32 @@ jobs: - name: Run tests run: bun test + + - name: Install codex CLI + run: npm install -g @openai/codex + + - name: Smoke-test installer (Linux/macOS) + if: runner.os != 'Windows' + run: | + ./install.sh + codex-collab --help + codex-collab health + + - name: Smoke-test installer (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: | + Set-ExecutionPolicy Bypass -Scope Process -Force + .\install.ps1 + codex-collab --help + codex-collab health + + - name: Smoke-test .cmd shim from CMD (Windows) + if: runner.os == 'Windows' + shell: cmd + run: | + REM Simulate new terminal: reload PATH from registry, keep session PATH for bun/node + for /f "delims=" %%i in ('powershell -Command "[Environment]::GetEnvironmentVariable('Path','User') + ';' + [Environment]::GetEnvironmentVariable('Path','Machine')"') do set "PATH=%%i;%PATH%" + codex-collab --help + codex-collab health + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fe4407..2c5facc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,12 @@ bun install ./install.sh --dev # symlink for live iteration ``` +On Windows (PowerShell): + +```powershell +powershell -ExecutionPolicy Bypass -File install.ps1 -Dev +``` + ## Running Tests ```bash diff --git a/README.md b/README.md index ab556ec..1446f6a 100644 --- a/README.md +++ b/README.md @@ -21,29 +21,46 @@ codex-collab is a [Claude Code skill](https://docs.anthropic.com/en/docs/claude- - **Thread reuse** — Resume existing threads to send follow-up prompts, build on previous responses, or steer the work in a new direction. - **Approval control** — Configurable approval policies for tool calls: auto-approve, interactive, or deny. -## Prerequisites - -Tested on Linux (Ubuntu 22.04) and macOS. Both must be installed and on your PATH. - -- [Bun](https://bun.sh/) >= 1.0 — runs the CLI -- [Codex CLI](https://github.com/openai/codex) — must support `codex app-server` (tested with 0.106.0; `npm install -g @openai/codex`) - ## Installation +Requires [Bun](https://bun.sh/) >= 1.0 and [Codex CLI](https://github.com/openai/codex) (`npm install -g @openai/codex`) on your PATH. Tested on Linux (Ubuntu 22.04), macOS, and Windows 10. + ```bash git clone https://github.com/Kevin7Qi/codex-collab.git cd codex-collab +``` + +### Linux / macOS + +```bash ./install.sh ``` -The install script builds a self-contained bundle, copies it to `~/.claude/skills/codex-collab/`, and symlinks the binary. Once installed, Claude discovers the skill automatically and can invoke it without explicit prompting. +### Windows -For development (live-reloading source changes): +```powershell +powershell -ExecutionPolicy Bypass -File install.ps1 +``` + +After installation, **reopen your terminal** so the updated PATH takes effect, then run `codex-collab health` to verify. + +The installer builds a self-contained bundle, deploys it to your home directory (`~/.claude/skills/codex-collab/` on Linux/macOS, `%USERPROFILE%\.claude\skills\codex-collab\` on Windows), and adds a binary shim to your PATH. Once installed, Claude discovers the skill automatically. + +
+Development mode + +Use `--dev` to symlink source files for live-reloading instead of building a bundle: ```bash +# Linux / macOS ./install.sh --dev + +# Windows (may require Developer Mode or an elevated terminal for symlinks) +powershell -ExecutionPolicy Bypass -File install.ps1 -Dev ``` +
+ ## Quick Start ```bash diff --git a/README.zh-CN.md b/README.zh-CN.md index 96a6873..609a0b9 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -21,29 +21,46 @@ codex-collab 是一个 [Claude Code 技能](https://docs.anthropic.com/en/docs/c - **会话复用** — 接续先前的会话继续对话，在既有上下文基础上推进，不必从头开始。 - **审批控制** — 按需配置工具调用的审批策略：自动批准、交互确认或拒绝。 -## 环境要求 - -在 Linux (Ubuntu 22.04) 与 macOS 上测试通过。请确保以下工具已安装并加入 PATH： - -- [Bun](https://bun.sh/) >= 1.0 — 用于运行 CLI -- [Codex CLI](https://github.com/openai/codex) — 须支持 `codex app-server` 子命令（已测试 0.106.0；`npm install -g @openai/codex`） - ## 安装 +需要 [Bun](https://bun.sh/) >= 1.0 和 [Codex CLI](https://github.com/openai/codex)（`npm install -g @openai/codex`）并加入 PATH。已在 Linux (Ubuntu 22.04)、macOS 与 Windows 10 上测试通过。 + ```bash git clone https://github.com/Kevin7Qi/codex-collab.git cd codex-collab +``` + +### Linux / macOS + +```bash ./install.sh ``` -运行后会自动构建独立 bundle 并部署到 `~/.claude/skills/codex-collab/`。完成后 Claude 即可自动发现该技能，无需额外配置。 +### Windows -开发模式（源码变更实时生效）： +```powershell +powershell -ExecutionPolicy Bypass -File install.ps1 +``` + +安装完成后，**重新打开终端**以使 PATH 生效，然后运行 `codex-collab health` 验证安装。 + +安装脚本会自动构建独立 bundle，部署到主目录下（Linux/macOS 为 `~/.claude/skills/codex-collab/`，Windows 为 `%USERPROFILE%\.claude\skills\codex-collab\`），并添加可执行文件到 PATH。完成后 Claude 即可自动发现该技能。 + +
+开发模式 + +使用 `--dev` 以符号链接方式安装，源码变更实时生效： ```bash +# Linux / macOS ./install.sh --dev + +# Windows（可能需要启用开发者模式或使用管理员终端以创建符号链接） +powershell -ExecutionPolicy Bypass -File install.ps1 -Dev ``` +
+ ## 快速开始 ```bash diff --git a/install.ps1 b/install.ps1 new file mode 100644 index 0000000..5525e8d --- /dev/null +++ b/install.ps1 @@ -0,0 +1,154 @@ +#Requires -Version 5.1 +<# +.SYNOPSIS + Install codex-collab on Windows. +.PARAMETER Dev + Symlink source files for live development instead of building. +#> +param( + [switch]$Dev, + [switch]$Help +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" + +$RepoDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$SkillDir = Join-Path $env:USERPROFILE ".claude\skills\codex-collab" +$BinDir = Join-Path $env:USERPROFILE ".local\bin" + +function Show-Usage { + Write-Host "Usage: powershell -File install.ps1 [-Dev]" + Write-Host "" + Write-Host " (default) Build and copy a self-contained skill directory" + Write-Host " -Dev Symlink source files for live development" +} + +if ($Help) { + Show-Usage + exit 0 +} + +# Check prerequisites +$missing = @() +if (-not (Get-Command bun -ErrorAction SilentlyContinue)) { $missing += "bun" } +if (-not (Get-Command codex -ErrorAction SilentlyContinue)) { $missing += "codex" } + +if ($missing.Count -gt 0) { + Write-Host "Missing prerequisites: $($missing -join ', ')" + Write-Host " bun: https://bun.sh/" + Write-Host " codex: npm install -g @openai/codex" + exit 1 +} + +# Install dependencies +Write-Host "Installing dependencies..." +Push-Location $RepoDir +try { + bun install + if ($LASTEXITCODE -ne 0) { throw "'bun install' failed with exit code $LASTEXITCODE" } +} catch { + Write-Host "Error: $_" + exit 1 +} finally { + Pop-Location +} + +if ($Dev) { + Write-Host "Installing in dev mode (symlinks)..." + Write-Host "Note: Symlinks on Windows may require Developer Mode or elevated privileges." + + # Create skill directory + New-Item -ItemType Directory -Path (Join-Path $SkillDir "scripts") -Force | Out-Null + + # Symlink skill files (requires Developer Mode or elevated privileges) + $links = @( + @{ Path = (Join-Path $SkillDir "SKILL.md"); Target = (Join-Path $RepoDir "SKILL.md") } + @{ Path = (Join-Path $SkillDir "scripts\codex-collab"); Target = (Join-Path $RepoDir "src\cli.ts") } + @{ Path = (Join-Path $SkillDir "LICENSE.txt"); Target = (Join-Path $RepoDir "LICENSE") } + ) + + foreach ($link in $links) { + if (Test-Path $link.Path) { Remove-Item $link.Path -Force } + try { + New-Item -ItemType SymbolicLink -Path $link.Path -Target $link.Target -Force | Out-Null + } catch { + Write-Host "" + Write-Host "Error: Cannot create symlinks. Dev mode requires one of:" + Write-Host " 1. Enable Developer Mode: Settings > Update & Security > For developers" + Write-Host " 2. Run this script in an elevated (Administrator) terminal" + Write-Host "" + Write-Host "Alternatively, use build mode (without -Dev) which does not need symlinks." + exit 1 + } + } + Write-Host "Linked skill to $SkillDir" + + $shimTarget = Join-Path $RepoDir "src\cli.ts" + +} else { + Write-Host "Building..." + + # Build bundled JS + $skillBuild = Join-Path $RepoDir "skill\codex-collab" + if (Test-Path $skillBuild) { Remove-Item $skillBuild -Recurse -Force } + New-Item -ItemType Directory -Path (Join-Path $skillBuild "scripts") -Force | Out-Null + + $built = Join-Path $skillBuild "scripts\codex-collab" + try { + bun build (Join-Path $RepoDir "src\cli.ts") --outfile $built --target bun + if ($LASTEXITCODE -ne 0) { throw "'bun build' failed with exit code $LASTEXITCODE" } + } catch { + Write-Host "Error: $_" + exit 1 + } + + # Prepend shebang if missing (needed for Unix execution; harmless on Windows with Bun) + $content = Get-Content $built -Raw + if (-not $content.StartsWith("#!/")) { + [System.IO.File]::WriteAllText($built, "#!/usr/bin/env bun`n" + $content, [System.Text.UTF8Encoding]::new($false)) + } + + # Copy SKILL.md and LICENSE + Copy-Item (Join-Path $RepoDir "SKILL.md") (Join-Path $skillBuild "SKILL.md") + Copy-Item (Join-Path $RepoDir "LICENSE") (Join-Path $skillBuild "LICENSE.txt") + + # Install skill + if (Test-Path $SkillDir) { Remove-Item $SkillDir -Recurse -Force } + New-Item -ItemType Directory -Path (Split-Path $SkillDir) -Force | Out-Null + Copy-Item $skillBuild $SkillDir -Recurse + Write-Host "Installed skill to $SkillDir" + + $shimTarget = Join-Path $SkillDir "scripts\codex-collab" +} + +# Create .cmd shim (CMD/PowerShell) and extensionless bash wrapper (Git Bash/MSYS2) +New-Item -ItemType Directory -Path $BinDir -Force | Out-Null +$cmdShim = Join-Path $BinDir "codex-collab.cmd" +[System.IO.File]::WriteAllText($cmdShim, "@bun `"$shimTarget`" %*`r`n", [System.Text.UTF8Encoding]::new($false)) +$bashShim = Join-Path $BinDir "codex-collab" +[System.IO.File]::WriteAllText($bashShim, "#!/usr/bin/env bash`nexec bun `"$shimTarget`" `"`$@`"", [System.Text.UTF8Encoding]::new($false)) +Write-Host "Created binary shims at $BinDir" + +# Add bin dir to user PATH if not already present +$userPath = [Environment]::GetEnvironmentVariable("Path", "User") +if ($userPath -notlike "*$BinDir*") { + [Environment]::SetEnvironmentVariable("Path", "$BinDir;$userPath", "User") + Write-Host "Added $BinDir to user PATH (permanent)." +} +# Update current session PATH +if ($env:Path -notlike "*$BinDir*") { + $env:Path = "$BinDir;$env:Path" +} + +# Verify and health check +Write-Host "" +codex-collab health +if ($LASTEXITCODE -ne 0) { + Write-Host "Error: 'codex-collab health' failed. The installation may be broken." + exit 1 +} + +$mode = if ($Dev) { "dev" } else { "build" } +Write-Host "" +Write-Host "Done ($mode mode). Open a new terminal, then run 'codex-collab --help' to get started." diff --git a/src/approvals.test.ts b/src/approvals.test.ts index 5506058..27b286c 100644 --- a/src/approvals.test.ts +++ b/src/approvals.test.ts @@ -1,9 +1,11 @@ import { describe, expect, test, beforeEach } from "bun:test"; import { autoApproveHandler, InteractiveApprovalHandler } from "./approvals"; import { mkdirSync, rmSync, writeFileSync, readFileSync, existsSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; import type { CommandApprovalRequest, FileChangeApprovalRequest } from "./types"; -const TEST_APPROVALS_DIR = `${process.env.TMPDIR || "/tmp/claude-1000"}/codex-collab-test-approvals`; +const TEST_APPROVALS_DIR = join(tmpdir(), "codex-collab-test-approvals"); beforeEach(() => { if (existsSync(TEST_APPROVALS_DIR)) rmSync(TEST_APPROVALS_DIR, { recursive: true }); @@ -51,23 +53,23 @@ describe("InteractiveApprovalHandler", () => { // Write decision file after a short delay setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/appr-001.decision`, "accept"); + writeFileSync(join(TEST_APPROVALS_DIR, "appr-001.decision"), "accept"); }, 200); const decision = await handler.handleCommandApproval(mockCommandRequest); expect(decision).toBe("accept"); expect(lines.some((l) => l.includes("APPROVAL NEEDED"))).toBe(true); // Request file should be cleaned up - expect(existsSync(`${TEST_APPROVALS_DIR}/appr-001.json`)).toBe(false); + expect(existsSync(join(TEST_APPROVALS_DIR, "appr-001.json"))).toBe(false); // Decision file should be cleaned up - expect(existsSync(`${TEST_APPROVALS_DIR}/appr-001.decision`)).toBe(false); + expect(existsSync(join(TEST_APPROVALS_DIR, "appr-001.decision"))).toBe(false); }); test("returns decline when decision file says decline", async () => { const handler = new InteractiveApprovalHandler(TEST_APPROVALS_DIR, () => {}, 100); setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/appr-001.decision`, "decline"); + writeFileSync(join(TEST_APPROVALS_DIR, "appr-001.decision"), "decline"); }, 200); const decision = await handler.handleCommandApproval(mockCommandRequest); @@ -83,14 +85,14 @@ describe("InteractiveApprovalHandler", () => { ); setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/item2.decision`, "accept"); + writeFileSync(join(TEST_APPROVALS_DIR, "item2.decision"), "accept"); }, 200); const decision = await handler.handleFileChangeApproval(mockFileChangeRequest); expect(decision).toBe("accept"); expect(lines.some((l) => l.includes("file change"))).toBe(true); // Cleanup happened - expect(existsSync(`${TEST_APPROVALS_DIR}/item2.json`)).toBe(false); + expect(existsSync(join(TEST_APPROVALS_DIR, "item2.json"))).toBe(false); }); test("uses itemId as fallback when approvalId is null", async () => { @@ -102,7 +104,7 @@ describe("InteractiveApprovalHandler", () => { }; setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/item1.decision`, "accept"); + writeFileSync(join(TEST_APPROVALS_DIR, "item1.decision"), "accept"); }, 200); const decision = await handler.handleCommandApproval(reqWithNullApprovalId); @@ -114,7 +116,7 @@ describe("InteractiveApprovalHandler", () => { // Start the approval but write decision after verifying request file setTimeout(() => { - const requestPath = `${TEST_APPROVALS_DIR}/appr-001.json`; + const requestPath = join(TEST_APPROVALS_DIR, "appr-001.json"); expect(existsSync(requestPath)).toBe(true); const content = JSON.parse(readFileSync(requestPath, "utf-8")); expect(content.type).toBe("commandExecution"); @@ -123,14 +125,14 @@ describe("InteractiveApprovalHandler", () => { expect(content.reason).toBe("network access"); expect(content.threadId).toBe("t1"); expect(content.turnId).toBe("turn1"); - writeFileSync(`${TEST_APPROVALS_DIR}/appr-001.decision`, "accept"); + writeFileSync(join(TEST_APPROVALS_DIR, "appr-001.decision"), "accept"); }, 150); await handler.handleCommandApproval(mockCommandRequest); }); test("creates approvalsDir if it does not exist", async () => { - const nestedDir = `${TEST_APPROVALS_DIR}/nested/deep`; + const nestedDir = join(TEST_APPROVALS_DIR, "nested", "deep"); const handler = new InteractiveApprovalHandler(nestedDir, () => {}, 100); expect(existsSync(nestedDir)).toBe(true); @@ -145,7 +147,7 @@ describe("InteractiveApprovalHandler", () => { ); setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/appr-001.decision`, "accept"); + writeFileSync(join(TEST_APPROVALS_DIR, "appr-001.decision"), "accept"); }, 200); await handler.handleCommandApproval(mockCommandRequest); @@ -164,14 +166,14 @@ describe("InteractiveApprovalHandler", () => { handler.handleCommandApproval(mockCommandRequest, controller.signal), ).rejects.toThrow("cancelled"); - expect(existsSync(`${TEST_APPROVALS_DIR}/appr-001.json`)).toBe(false); + expect(existsSync(join(TEST_APPROVALS_DIR, "appr-001.json"))).toBe(false); }); test("treats unknown decision text as decline", async () => { const handler = new InteractiveApprovalHandler(TEST_APPROVALS_DIR, () => {}, 100); setTimeout(() => { - writeFileSync(`${TEST_APPROVALS_DIR}/appr-001.decision`, "garbage"); + writeFileSync(join(TEST_APPROVALS_DIR, "appr-001.decision"), "garbage"); }, 200); const decision = await handler.handleCommandApproval(mockCommandRequest); diff --git a/src/approvals.ts b/src/approvals.ts index 710e1a5..6835234 100644 --- a/src/approvals.ts +++ b/src/approvals.ts @@ -1,6 +1,7 @@ // src/approvals.ts — Approval handler abstraction import { writeFileSync, readFileSync, unlinkSync, existsSync, mkdirSync } from "fs"; +import { join } from "path"; import type { ApprovalDecision, CommandApprovalRequest, @@ -77,7 +78,7 @@ export class InteractiveApprovalHandler implements ApprovalHandler { private writeRequestFile(id: string, data: unknown): void { try { - writeFileSync(`${this.approvalsDir}/${id}.json`, JSON.stringify(data, null, 2), { mode: 0o600 }); + writeFileSync(join(this.approvalsDir, `${id}.json`), JSON.stringify(data, null, 2), { mode: 0o600 }); } catch (e) { console.error(`[codex] Failed to write approval request: ${e instanceof Error ? e.message : e}`); throw e; @@ -85,8 +86,8 @@ export class InteractiveApprovalHandler implements ApprovalHandler { } private async pollForDecision(id: string, timeoutMs: number, signal?: AbortSignal): Promise { - const decisionPath = `${this.approvalsDir}/${id}.decision`; - const requestPath = `${this.approvalsDir}/${id}.json`; + const decisionPath = join(this.approvalsDir, `${id}.decision`); + const requestPath = join(this.approvalsDir, `${id}.json`); const deadline = Date.now() + timeoutMs; const cleanup = () => { diff --git a/src/cli.test.ts b/src/cli.test.ts index c2d3bfd..d9a3738 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -12,6 +12,7 @@ function run(...args: string[]): { stdout: string; stderr: string; exitCode: num encoding: "utf-8", cwd: import.meta.dir + "/..", stdio: ["pipe", "pipe", "pipe"], + timeout: 5000, }); return { stdout: (result.stdout ?? "") as string, @@ -51,15 +52,14 @@ describe("CLI valid commands", () => { describe("CLI flag parsing", () => { it("--all does not error", () => { - // --all should be accepted by the parser (jobs command needs server, so we just check parse doesn't error) - // We can't test jobs without a server, but we can verify --all isn't "Unknown option" - const { stderr, exitCode } = run("run", "test", "--all"); - // Should fail because codex isn't available, not because --all is unknown + // Use 'health' instead of 'run' to avoid starting app server (hangs if codex installed) + const { stderr } = run("health", "--all"); expect(stderr).not.toContain("Unknown option"); }); it("--content-only does not error", () => { - const { stderr } = run("run", "test", "--content-only"); + // Use 'health' instead of 'run' to avoid starting app server (hangs if codex installed) + const { stderr } = run("health", "--content-only"); expect(stderr).not.toContain("Unknown option"); }); }); @@ -87,9 +87,10 @@ describe("CLI invalid inputs", () => { expect(stderr).toContain("Invalid sandbox mode"); }); - it("run without prompt exits 1", () => { - const { exitCode } = run("run"); + it("run without prompt exits with error message", () => { + const { stderr, exitCode } = run("run"); expect(exitCode).toBe(1); + expect(stderr).toContain("No prompt provided"); }); it("unknown option exits 1", () => { diff --git a/src/cli.ts b/src/cli.ts index b34fabb..bf2f246 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -35,7 +35,7 @@ import { unlinkSync, writeFileSync, } from "fs"; -import { resolve } from "path"; +import { resolve, join } from "path"; import type { ReviewTarget, Thread, @@ -635,6 +635,16 @@ async function cmdKill(positional: string[]) { const threadId = resolveThreadId(config.threadsFile, id); + // Check local status — skip server operations if already killed + const mapping = loadThreadMapping(config.threadsFile); + const shortId = findShortId(config.threadsFile, threadId); + const localStatus = shortId ? mapping[shortId]?.lastStatus : undefined; + + if (localStatus === "killed") { + progress(`Thread ${id} is already killed`); + return; + } + const archived = await withClient(async (client) => { // Try to read thread status first and interrupt active turn if any try { @@ -683,7 +693,7 @@ function resolveLogPath(positional: string[], usage: string): string { const threadId = resolveThreadId(config.threadsFile, id); const shortId = findShortId(config.threadsFile, threadId); if (!shortId) die(`Thread not found: ${id}`); - return `${config.logsDir}/${shortId}.log`; + return join(config.logsDir, `${shortId}.log`); } async function cmdOutput(positional: string[], opts: Options) { @@ -750,11 +760,11 @@ async function cmdApproveOrDecline( if (!approvalId) die(`Usage: codex-collab ${verb} `); validateIdOrDie(approvalId); - const requestPath = `${config.approvalsDir}/${approvalId}.json`; + const requestPath = join(config.approvalsDir, `${approvalId}.json`); if (!existsSync(requestPath)) die(`No pending approval: ${approvalId}`); - const decisionPath = `${config.approvalsDir}/${approvalId}.decision`; + const decisionPath = join(config.approvalsDir, `${approvalId}.decision`); try { writeFileSync(decisionPath, decision, { mode: 0o600 }); } catch (e) { @@ -771,7 +781,7 @@ function deleteOldFiles(dir: string, maxAgeMs: number): number { const now = Date.now(); let deleted = 0; for (const file of readdirSync(dir)) { - const path = `${dir}/${file}`; + const path = join(dir, file); try { if (now - Bun.file(path).lastModified > maxAgeMs) { unlinkSync(path); @@ -804,7 +814,7 @@ async function cmdClean() { try { let lastActivity = new Date(entry.createdAt).getTime(); if (Number.isNaN(lastActivity)) lastActivity = 0; - const logPath = `${config.logsDir}/${shortId}.log`; + const logPath = join(config.logsDir, `${shortId}.log`); if (existsSync(logPath)) { lastActivity = Math.max(lastActivity, Bun.file(logPath).lastModified); } @@ -853,7 +863,7 @@ async function cmdDelete(positional: string[]) { } if (shortId) { - const logPath = `${config.logsDir}/${shortId}.log`; + const logPath = join(config.logsDir, `${shortId}.log`); if (existsSync(logPath)) unlinkSync(logPath); removeThread(config.threadsFile, shortId); } @@ -866,13 +876,15 @@ async function cmdDelete(positional: string[]) { } async function cmdHealth() { - const which = Bun.spawnSync(["which", "codex"]); + const findCmd = process.platform === "win32" ? "where" : "which"; + const which = Bun.spawnSync([findCmd, "codex"]); if (which.exitCode !== 0) { die("codex CLI not found. Install: npm install -g @openai/codex"); } console.log(` bun: ${Bun.version}`); - console.log(` codex: ${which.stdout.toString().trim()}`); + // `where` on Windows returns multiple matches; show only the first + console.log(` codex: ${which.stdout.toString().trim().split("\n")[0].trim()}`); try { const userAgent = await withClient(async (client) => client.userAgent); @@ -941,7 +953,7 @@ Examples: // --------------------------------------------------------------------------- /** Ensure data directories exist (called only for commands that need them). - * Config getters throw if HOME is unset, producing a clear error. */ + * Config getters throw if the home directory cannot be determined, producing a clear error. */ function ensureDataDirs(): void { mkdirSync(config.logsDir, { recursive: true }); mkdirSync(config.approvalsDir, { recursive: true }); diff --git a/src/config.ts b/src/config.ts index eda2278..3d22ac0 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,10 +1,12 @@ // src/config.ts — Configuration for codex-collab +import { homedir } from "os"; +import { join } from "path"; import pkg from "../package.json"; function getHome(): string { - const home = process.env.HOME; - if (!home) throw new Error("HOME environment variable is not set"); + const home = homedir(); + if (!home) throw new Error("Cannot determine home directory"); return home; } @@ -28,12 +30,12 @@ export const config = { defaultTimeout: 1200, // seconds — turn completion (20 min) requestTimeout: 30_000, // milliseconds — individual protocol requests (30s) - // Data paths — lazy via getters so HOME is validated at point of use, not import time. + // Data paths — lazy via getters so the home directory is validated at point of use, not import time. // Validated by ensureDataDirs() in cli.ts before any file operations. - get dataDir() { return `${getHome()}/.codex-collab`; }, - get threadsFile() { return `${this.dataDir}/threads.json`; }, - get logsDir() { return `${this.dataDir}/logs`; }, - get approvalsDir() { return `${this.dataDir}/approvals`; }, + get dataDir() { return join(getHome(), ".codex-collab"); }, + get threadsFile() { return join(this.dataDir, "threads.json"); }, + get logsDir() { return join(this.dataDir, "logs"); }, + get approvalsDir() { return join(this.dataDir, "approvals"); }, // Display jobsListLimit: 20, diff --git a/src/events.test.ts b/src/events.test.ts index b7e19a4..c77aa6d 100644 --- a/src/events.test.ts +++ b/src/events.test.ts @@ -1,8 +1,10 @@ import { describe, expect, test, beforeEach } from "bun:test"; import { EventDispatcher } from "./events"; import { mkdirSync, rmSync, readFileSync, existsSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; -const TEST_LOG_DIR = `${process.env.TMPDIR || "/tmp/claude-1000"}/codex-collab-test-logs`; +const TEST_LOG_DIR = join(tmpdir(), "codex-collab-test-logs"); beforeEach(() => { if (existsSync(TEST_LOG_DIR)) rmSync(TEST_LOG_DIR, { recursive: true }); @@ -66,7 +68,7 @@ describe("EventDispatcher", () => { }); dispatcher.flush(); - const logPath = `${TEST_LOG_DIR}/test4.log`; + const logPath = join(TEST_LOG_DIR, "test4.log"); expect(existsSync(logPath)).toBe(true); const content = readFileSync(logPath, "utf-8"); expect(content).toContain("echo hello"); @@ -138,7 +140,7 @@ describe("EventDispatcher", () => { test("progress events auto-flush to log file", () => { const dispatcher = new EventDispatcher("test-autoflush", TEST_LOG_DIR); - const logPath = `${TEST_LOG_DIR}/test-autoflush.log`; + const logPath = join(TEST_LOG_DIR, "test-autoflush.log"); // Trigger a progress event (command started) — should auto-flush without explicit flush() call dispatcher.handleItemStarted({ diff --git a/src/events.ts b/src/events.ts index 2a4c7a8..ed3f62f 100644 --- a/src/events.ts +++ b/src/events.ts @@ -1,6 +1,7 @@ // src/events.ts — Event dispatcher for app server notifications import { appendFileSync, mkdirSync, existsSync } from "fs"; +import { join } from "path"; import type { ItemStartedParams, ItemCompletedParams, DeltaParams, ErrorNotificationParams, @@ -24,7 +25,7 @@ export class EventDispatcher { onProgress?: ProgressCallback, ) { if (!existsSync(logsDir)) mkdirSync(logsDir, { recursive: true }); - this.logPath = `${logsDir}/${shortId}.log`; + this.logPath = join(logsDir, `${shortId}.log`); this.onProgress = onProgress ?? ((line) => process.stderr.write(line + "\n")); } diff --git a/src/integration.test.ts b/src/integration.test.ts index 0db74cb..4d59317 100644 --- a/src/integration.test.ts +++ b/src/integration.test.ts @@ -6,7 +6,7 @@ import { connect } from "./protocol"; const runIntegration = process.env.RUN_INTEGRATION === "1" && - Bun.spawnSync(["which", "codex"]).exitCode === 0; + Bun.spawnSync([process.platform === "win32" ? "where" : "which", "codex"]).exitCode === 0; describe.skipIf(!runIntegration)("integration", () => { test("connect and list models", async () => { diff --git a/src/protocol.test.ts b/src/protocol.test.ts index 0d136e7..2d580fc 100644 --- a/src/protocol.test.ts +++ b/src/protocol.test.ts @@ -1,5 +1,7 @@ import { describe, expect, test, beforeAll, beforeEach, afterEach } from "bun:test"; import { parseMessage, formatNotification, formatResponse, connect, type AppServerClient } from "./protocol"; +import { join } from "path"; +import { tmpdir } from "os"; // Test-local formatRequest helper with its own counter (not exported from protocol.ts // to avoid ID collisions with AppServerClient's internal counter). @@ -11,58 +13,66 @@ function formatRequest(method: string, params?: unknown): { line: string; id: nu return { line: JSON.stringify(msg) + "\n", id }; } -const TEST_DIR = `${process.env.TMPDIR || "/tmp/claude-1000"}/codex-collab-test-protocol`; -const MOCK_SERVER = `${TEST_DIR}/mock-app-server.ts`; +async function captureErrorMessage(promise: Promise): Promise { + // Workaround: bun test on Windows doesn't flush .rejects properly, so we + // capture the rejection message manually instead of using .rejects.toThrow(). + let resolved = false; + try { + await promise; + resolved = true; + } catch (e) { + return e instanceof Error ? e.message : String(e); + } + if (resolved) throw new Error("Expected promise to reject, but it resolved"); + return ""; // unreachable +} + +const TEST_DIR = join(tmpdir(), "codex-collab-test-protocol"); +const MOCK_SERVER = join(TEST_DIR, "mock-app-server.ts"); const MOCK_SERVER_SOURCE = `#!/usr/bin/env bun -const decoder = new TextDecoder(); function respond(obj) { process.stdout.write(JSON.stringify(obj) + "\\n"); } const exitEarly = process.env.MOCK_EXIT_EARLY === "1"; const errorResponse = process.env.MOCK_ERROR_RESPONSE === "1"; -async function main() { - const reader = Bun.stdin.stream().getReader(); - let buffer = ""; - try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - let idx; - while ((idx = buffer.indexOf("\\n")) !== -1) { - const line = buffer.slice(0, idx).trim(); - buffer = buffer.slice(idx + 1); - if (!line) continue; - let msg; - try { msg = JSON.parse(line); } catch { continue; } - if (msg.id !== undefined && msg.method) { - switch (msg.method) { - case "initialize": - respond({ id: msg.id, result: { userAgent: "mock-codex-server/0.1.0" } }); - if (exitEarly) setTimeout(() => process.exit(0), 50); - break; - case "thread/start": - if (errorResponse) { - respond({ id: msg.id, error: { code: -32603, message: "Internal error: model not available" } }); - } else { - respond({ id: msg.id, result: { - thread: { id: "thread-mock-001", preview: "", modelProvider: "openai", - createdAt: Date.now(), updatedAt: Date.now(), status: { type: "idle" }, - path: null, cwd: "/tmp", cliVersion: "0.1.0", source: "mock", name: null, - agentNickname: null, agentRole: null, gitInfo: null, turns: [] }, - model: msg.params?.model || "gpt-5.3-codex", modelProvider: "openai", - cwd: "/tmp", approvalPolicy: "never", sandbox: null, - }}); - } - break; - default: - respond({ id: msg.id, error: { code: -32601, message: "Method not found: " + msg.method } }); +let buffer = ""; +process.stdin.setEncoding("utf-8"); +process.stdin.on("data", (chunk) => { + buffer += chunk; + let idx; + while ((idx = buffer.indexOf("\\n")) !== -1) { + const line = buffer.slice(0, idx).trim(); + buffer = buffer.slice(idx + 1); + if (!line) continue; + let msg; + try { msg = JSON.parse(line); } catch { continue; } + if (msg.id !== undefined && msg.method) { + switch (msg.method) { + case "initialize": + respond({ id: msg.id, result: { userAgent: "mock-codex-server/0.1.0" } }); + if (exitEarly) setTimeout(() => process.exit(0), 50); + break; + case "thread/start": + if (errorResponse) { + respond({ id: msg.id, error: { code: -32603, message: "Internal error: model not available" } }); + } else { + respond({ id: msg.id, result: { + thread: { id: "thread-mock-001", preview: "", modelProvider: "openai", + createdAt: Date.now(), updatedAt: Date.now(), status: { type: "idle" }, + path: null, cwd: "/tmp", cliVersion: "0.1.0", source: "mock", name: null, + agentNickname: null, agentRole: null, gitInfo: null, turns: [] }, + model: msg.params?.model || "gpt-5.3-codex", modelProvider: "openai", + cwd: "/tmp", approvalPolicy: "never", sandbox: null, + }}); } - } + break; + default: + respond({ id: msg.id, error: { code: -32601, message: "Method not found: " + msg.method } }); } } - } catch {} -} -main(); + } +}); +process.stdin.on("end", () => process.exit(0)); +process.stdin.on("error", () => process.exit(1)); `; beforeAll(async () => { @@ -226,260 +236,262 @@ describe("parseMessage", () => { // AppServerClient integration tests (using mock server) // --------------------------------------------------------------------------- +// Each test manages its own client lifecycle to avoid dangling-process races +// when bun runs tests concurrently within a describe block. describe("AppServerClient", () => { - let client: AppServerClient | null = null; - + // On Windows, bun's test runner doesn't fully await async finally blocks before moving + // to the next test. close() takes ~400ms on Windows (dominated by taskkill process tree + // cleanup). This delay ensures close() completes before the next test spawns a mock server. afterEach(async () => { - if (client) { - await client.close(); - client = null; + if (process.platform === "win32") { + await new Promise((r) => setTimeout(r, 1000)); } }); test("connect performs initialize handshake and returns userAgent", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); - expect(client.userAgent).toBe("mock-codex-server/0.1.0"); + try { + expect(c.userAgent).toBe("mock-codex-server/0.1.0"); + } finally { + await c.close(); + } }); test("close shuts down gracefully", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); - await client.close(); - client = null; + await c.close(); // No error means success — process exited cleanly }); test("request sends and receives response", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); - - const result = await client.request<{ thread: { id: string }; model: string }>( - "thread/start", - { model: "gpt-5.3-codex" }, - ); - - expect(result.thread.id).toBe("thread-mock-001"); - expect(result.model).toBe("gpt-5.3-codex"); + try { + const result = await c.request<{ thread: { id: string }; model: string }>( + "thread/start", + { model: "gpt-5.3-codex" }, + ); + expect(result.thread.id).toBe("thread-mock-001"); + expect(result.model).toBe("gpt-5.3-codex"); + } finally { + await c.close(); + } }); test("request rejects with descriptive error on JSON-RPC error response", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, env: { MOCK_ERROR_RESPONSE: "1" }, }); - - await expect( - client.request("thread/start", { model: "bad-model" }), - ).rejects.toThrow("JSON-RPC error -32603: Internal error: model not available"); + try { + const error = await captureErrorMessage( + c.request("thread/start", { model: "bad-model" }), + ); + expect(error).toContain( + "JSON-RPC error -32603: Internal error: model not available", + ); + } finally { + await c.close(); + } }); test("request rejects with error for unknown method", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); - - await expect( - client.request("unknown/method"), - ).rejects.toThrow("Method not found: unknown/method"); + try { + const error = await captureErrorMessage(c.request("unknown/method")); + expect(error).toContain("Method not found: unknown/method"); + } finally { + await c.close(); + } }); test("request rejects when process exits unexpectedly", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, env: { MOCK_EXIT_EARLY: "1" }, }); - - // The mock server exits after initialize, so the next request should fail - // Give a tiny delay for the process to actually exit - await new Promise((r) => setTimeout(r, 100)); - - await expect( - client.request("thread/start"), - ).rejects.toThrow(); + try { + // The mock server exits after initialize, so the next request should fail + await new Promise((r) => setTimeout(r, 100)); + const error = await captureErrorMessage(c.request("thread/start")); + expect(error.length).toBeGreaterThan(0); + } finally { + await c.close(); + } }); test("request rejects after client is closed", async () => { - client = await connect({ - command: ["bun", "run", MOCK_SERVER], - requestTimeout: 5000, - }); - - await client.close(); - client = null; - - // We need a fresh reference but close has been called, so we reconnect then close then try const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); await c.close(); - await expect( - c.request("thread/start"), - ).rejects.toThrow("Client is closed"); + const error = await captureErrorMessage(c.request("thread/start")); + expect(error).toContain("Client is closed"); }); test("notification handlers receive server notifications", async () => { // For this test we use a custom inline mock that sends a notification const notifyServer = ` - const decoder = new TextDecoder(); - async function main() { - const reader = Bun.stdin.stream().getReader(); - let buffer = ""; - try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - let idx; - while ((idx = buffer.indexOf("\\n")) !== -1) { - const line = buffer.slice(0, idx).trim(); - buffer = buffer.slice(idx + 1); - if (!line) continue; - const msg = JSON.parse(line); - if (msg.id !== undefined && msg.method === "initialize") { - process.stdout.write(JSON.stringify({ - id: msg.id, - result: { userAgent: "notify-server/0.1.0" }, - }) + "\\n"); - } - // After receiving "initialized" notification, send a server notification - if (!msg.id && msg.method === "initialized") { - process.stdout.write(JSON.stringify({ - method: "item/started", - params: { item: { type: "agentMessage", id: "item-1", text: "" }, threadId: "t1", turnId: "turn-1" }, - }) + "\\n"); - } - } + let buffer = ""; + process.stdin.setEncoding("utf-8"); + process.stdin.on("data", (chunk) => { + buffer += chunk; + let idx; + while ((idx = buffer.indexOf("\\n")) !== -1) { + const line = buffer.slice(0, idx).trim(); + buffer = buffer.slice(idx + 1); + if (!line) continue; + const msg = JSON.parse(line); + if (msg.id !== undefined && msg.method === "initialize") { + process.stdout.write(JSON.stringify({ + id: msg.id, + result: { userAgent: "notify-server/0.1.0" }, + }) + "\\n"); } - } catch {} - } - main(); + if (!msg.id && msg.method === "initialized") { + process.stdout.write(JSON.stringify({ + method: "item/started", + params: { item: { type: "agentMessage", id: "item-1", text: "" }, threadId: "t1", turnId: "turn-1" }, + }) + "\\n"); + } + } + }); + process.stdin.on("end", () => process.exit(0)); + process.stdin.on("error", () => process.exit(1)); `; - const serverPath = `${TEST_DIR}/mock-notify-server.ts`; + const serverPath = join(TEST_DIR, "mock-notify-server.ts"); await Bun.write(serverPath, notifyServer); const received: unknown[] = []; - client = await connect({ + const c = await connect({ command: ["bun", "run", serverPath], requestTimeout: 5000, }); - client.on("item/started", (params) => { - received.push(params); - }); - - // Give time for the notification to arrive - await new Promise((r) => setTimeout(r, 200)); - - expect(received.length).toBe(1); - expect(received[0]).toEqual({ - item: { type: "agentMessage", id: "item-1", text: "" }, - threadId: "t1", - turnId: "turn-1", - }); + try { + c.on("item/started", (params) => { + received.push(params); + }); + + // Give time for the notification to arrive + await new Promise((r) => setTimeout(r, 200)); + + expect(received.length).toBe(1); + expect(received[0]).toEqual({ + item: { type: "agentMessage", id: "item-1", text: "" }, + threadId: "t1", + turnId: "turn-1", + }); + } finally { + await c.close(); + } }); test("onRequest handler responds to server requests", async () => { // Mock server that sends a server request after initialize const approvalServer = ` - const decoder = new TextDecoder(); let sentApproval = false; - async function main() { - const reader = Bun.stdin.stream().getReader(); - let buffer = ""; - try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - let idx; - while ((idx = buffer.indexOf("\\n")) !== -1) { - const line = buffer.slice(0, idx).trim(); - buffer = buffer.slice(idx + 1); - if (!line) continue; - const msg = JSON.parse(line); - if (msg.id !== undefined && msg.method === "initialize") { - process.stdout.write(JSON.stringify({ - id: msg.id, - result: { userAgent: "approval-server/0.1.0" }, - }) + "\\n"); - } - // After initialized notification, send a server request - if (!msg.id && msg.method === "initialized" && !sentApproval) { - sentApproval = true; - process.stdout.write(JSON.stringify({ - id: "srv-1", - method: "item/commandExecution/requestApproval", - params: { command: "rm -rf /", threadId: "t1", turnId: "turn-1", itemId: "item-1" }, - }) + "\\n"); - } - // When we get back our response, send a verification notification - if (msg.id === "srv-1" && msg.result) { - process.stdout.write(JSON.stringify({ - method: "test/approvalReceived", - params: { decision: msg.result.decision }, - }) + "\\n"); - } - } + let buffer = ""; + process.stdin.setEncoding("utf-8"); + process.stdin.on("data", (chunk) => { + buffer += chunk; + let idx; + while ((idx = buffer.indexOf("\\n")) !== -1) { + const line = buffer.slice(0, idx).trim(); + buffer = buffer.slice(idx + 1); + if (!line) continue; + const msg = JSON.parse(line); + if (msg.id !== undefined && msg.method === "initialize") { + process.stdout.write(JSON.stringify({ + id: msg.id, + result: { userAgent: "approval-server/0.1.0" }, + }) + "\\n"); } - } catch {} - } - main(); + if (!msg.id && msg.method === "initialized" && !sentApproval) { + sentApproval = true; + process.stdout.write(JSON.stringify({ + id: "srv-1", + method: "item/commandExecution/requestApproval", + params: { command: "rm -rf /", threadId: "t1", turnId: "turn-1", itemId: "item-1" }, + }) + "\\n"); + } + if (msg.id === "srv-1" && msg.result) { + process.stdout.write(JSON.stringify({ + method: "test/approvalReceived", + params: { decision: msg.result.decision }, + }) + "\\n"); + } + } + }); + process.stdin.on("end", () => process.exit(0)); + process.stdin.on("error", () => process.exit(1)); `; - const serverPath = `${TEST_DIR}/mock-approval-server.ts`; + const serverPath = join(TEST_DIR, "mock-approval-server.ts"); await Bun.write(serverPath, approvalServer); - client = await connect({ + const c = await connect({ command: ["bun", "run", serverPath], requestTimeout: 5000, }); - // Register handler for approval requests - client.onRequest("item/commandExecution/requestApproval", (params: any) => { - return { decision: "accept" }; - }); + try { + // Register handler for approval requests + c.onRequest("item/commandExecution/requestApproval", (params: any) => { + return { decision: "accept" }; + }); - // Wait for the round-trip - const received: unknown[] = []; - client.on("test/approvalReceived", (params) => { - received.push(params); - }); + // Wait for the round-trip + const received: unknown[] = []; + c.on("test/approvalReceived", (params) => { + received.push(params); + }); - await new Promise((r) => setTimeout(r, 300)); + await new Promise((r) => setTimeout(r, 300)); - expect(received.length).toBe(1); - expect(received[0]).toEqual({ decision: "accept" }); + expect(received.length).toBe(1); + expect(received[0]).toEqual({ decision: "accept" }); + } finally { + await c.close(); + } }); test("on returns unsubscribe function", async () => { - client = await connect({ + const c = await connect({ command: ["bun", "run", MOCK_SERVER], requestTimeout: 5000, }); - const received: unknown[] = []; - const unsub = client.on("test/event", (params) => { - received.push(params); - }); + try { + const received: unknown[] = []; + const unsub = c.on("test/event", (params) => { + received.push(params); + }); - // Unsubscribe immediately - unsub(); + // Unsubscribe immediately + unsub(); - // Even if a notification arrived, handler should not fire - // (no notification is sent by the basic mock, but this verifies the unsub mechanism) - expect(received.length).toBe(0); + // Even if a notification arrived, handler should not fire + // (no notification is sent by the basic mock, but this verifies the unsub mechanism) + expect(received.length).toBe(0); + } finally { + await c.close(); + } }); }); diff --git a/src/protocol.ts b/src/protocol.ts index 7dcaf82..4f9d510 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -1,6 +1,7 @@ // src/protocol.ts — JSON-RPC client for Codex app server import { spawn } from "bun"; +import { spawnSync } from "child_process"; import type { JsonRpcMessage, JsonRpcRequest, @@ -89,7 +90,10 @@ export interface AppServerClient { onRequest(method: string, handler: ServerRequestHandler): () => void; /** Send a response to a server-sent request. */ respond(id: RequestId, result: unknown): void; - /** Gracefully close: close stdin -> wait 5s -> SIGTERM -> wait 3s -> SIGKILL. */ + /** Close the connection and terminate the server process. + * On Unix: close stdin -> wait 5s -> SIGTERM -> wait 3s -> SIGKILL. + * On Windows: close stdin, then immediately terminate the process tree + * (no timed grace period, unlike Unix). */ close(): Promise; /** The user-agent string from the initialize handshake. */ userAgent: string; @@ -364,7 +368,7 @@ export async function connect(opts?: ConnectOptions): Promise { closed = true; rejectAll("Client closed"); - // Step 1: Close stdin to signal the server to exit + // Close stdin to signal the server to exit try { proc.stdin.end(); } catch (e) { @@ -373,14 +377,40 @@ export async function connect(opts?: ConnectOptions): Promise { } } - // Step 2: Wait up to 5s for graceful exit - if (await waitForExit(5000)) { await readLoop; return; } + if (process.platform === "win32") { + // Windows: no SIGTERM equivalent — process termination is immediate. + // Kill the process tree first via taskkill /T /F, then fall back to + // proc.kill(). This order matters: if codex is a .cmd wrapper, killing + // the direct child first removes the PID that taskkill needs to traverse + // the tree, potentially leaving the real app-server alive. + // Note: we intentionally do NOT await readLoop/proc.exited here because + // Bun's test runner on Windows doesn't fully await async finally blocks, + // and the extra latency causes inter-test process races. The process is + // already dead after taskkill+kill, so the dangling promise is benign. + if (proc.pid) { + try { + const r = spawnSync("taskkill", ["/PID", String(proc.pid), "/T", "/F"], { stdio: "pipe", timeout: 5000 }); + // status 128: process already exited; null: spawnSync timed out + if (r.status !== 0 && r.status !== null && r.status !== 128) { + const msg = r.stderr?.toString().trim(); + console.error(`[codex] Warning: taskkill exited ${r.status}${msg ? ": " + msg : ""}`); + } + } catch (e) { + console.error(`[codex] Warning: process tree cleanup failed: ${e instanceof Error ? e.message : String(e)}`); + } + } + try { proc.kill(); } catch (e) { + if (!exited) { + console.error(`[codex] Warning: proc.kill() failed: ${e instanceof Error ? e.message : String(e)}`); + } + } + return; + } - // Step 3: SIGTERM, wait 3s + // Unix: wait for graceful exit, then escalate + if (await waitForExit(5000)) { await readLoop; return; } proc.kill("SIGTERM"); if (await waitForExit(3000)) { await readLoop; return; } - - // Step 4: SIGKILL proc.kill("SIGKILL"); await proc.exited; await readLoop; diff --git a/src/threads.test.ts b/src/threads.test.ts index f94c463..ba39c82 100644 --- a/src/threads.test.ts +++ b/src/threads.test.ts @@ -4,8 +4,10 @@ import { resolveThreadId, registerThread, findShortId, removeThread, } from "./threads"; import { rmSync, existsSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; -const TEST_THREADS_FILE = `${process.env.TMPDIR || "/tmp/claude-1000"}/codex-collab-test-threads.json`; +const TEST_THREADS_FILE = join(tmpdir(), "codex-collab-test-threads.json"); beforeEach(() => { if (existsSync(TEST_THREADS_FILE)) rmSync(TEST_THREADS_FILE); diff --git a/src/turns.test.ts b/src/turns.test.ts index dbd4a5c..17ff893 100644 --- a/src/turns.test.ts +++ b/src/turns.test.ts @@ -9,8 +9,10 @@ import type { ReviewStartResponse, } from "./types"; import { mkdirSync, rmSync, existsSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; -const TEST_LOG_DIR = `${process.env.TMPDIR || "/tmp/claude-1000"}/codex-collab-test-turns`; +const TEST_LOG_DIR = join(tmpdir(), "codex-collab-test-turns"); beforeEach(() => { if (existsSync(TEST_LOG_DIR)) rmSync(TEST_LOG_DIR, { recursive: true });