Compare commits

..

10 Commits

Author SHA1 Message Date
swe-agent[bot]
18c6c32628 fix(test): resolve CI timing race in TestFakeCmdInfra
问题:TestFakeCmdInfra/integration_with_runCodexTask 在 CI 环境中间歇性失败
原因:WaitDelay(1ms) 与 stdout 事件延迟(1ms) 过于接近,在 CI 负载下
     Wait() 可能在第二个 JSON 事件写入前返回,导致过早关闭 stdout

修复:将 WaitDelay 从 1ms 增加到 5ms,确保 Wait() 在所有 stdout
     数据写入后才返回,消除竞态条件

测试:本地和 -race 模式均通过

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-09 00:34:38 +08:00
ben
1ad2cfe629 Merge pull request #51 from cexll/fix/channel-sync-race-conditions
fix: 修复channel同步竞态条件和死锁问题
2025-12-09 00:13:04 +08:00
swe-agent[bot]
7bad716fbc change codex-wrapper version 2025-12-08 23:45:29 +08:00
swe-agent[bot]
220be6eb5c fix: 修复channel同步竞态条件和死锁问题
修复了4个严重的channel同步问题:

1. **parseCh无条件阻塞** (main.go:894-907)
   - 问题:cmd.Wait()先返回但parseJSONStreamWithLog永久阻塞时,主流程卡死
   - 修复:引入ctxAwareReader和5秒drainTimer机制,Wait完成后立即关闭stdout

2. **context取消失效** (main.go:894-907)
   - 问题:waitCh先完成后不再监听ctx.Done(),取消信号被吞掉
   - 修复:改为双channel循环持续监听waitCh/parseCh/ctx.Done()/drainTimer

3. **parseJSONStreamWithLog无读超时** (main.go:1056-1094)
   - 问题:bufio.Scanner阻塞读取,stdout未主动关闭时永远停在Read
   - 修复:ctxAwareReader支持CloseWithReason,Wait/ctx完成时主动关闭

4. **forceKillTimer生命周期过短**
   - 问题:waitCh返回后立刻停止timer,但stdout可能仍被写入
   - 修复:统一管理timer生命周期,在循环结束后Stop和drain

5. **并发竞态修复**
   - main.go:492 runStartupCleanup使用WaitGroup同步
   - logger.go:176 Flush加锁防止WaitGroup reuse panic

**测试覆盖**:
- 新增4个核心场景测试(Wait先返回、同时返回、Context超时、Parse阻塞)
- main.go覆盖率从28.6%提升到90.32%
- 154个测试全部通过,-race检测无警告

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-08 23:35:55 +08:00
ben
ead11d6996 Merge pull request #49 from cexll/freespace8/master
Freespace8/master
2025-12-08 11:48:07 +08:00
swe-agent[bot]
fec4b7dba3 test: 补充测试覆盖提升至 89.3%
解决的测试盲点:
1. getProcessStartTime (Unix) 0% → 77.3%
   - 新增 process_check_test.go
   - 测试 /proc/<pid>/stat 解析
   - 覆盖失败路径和边界情况

2. getBootTime (Unix) 0% → 91.7%
   - 测试 /proc/stat btime 解析
   - 覆盖缺失和格式错误场景

3. isPIDReused 60% → 100%
   - 新增表驱动测试覆盖所有分支
   - file_stat_fails, old_file, new_file, pid_reused, pid_not_reused

4. isUnsafeFile 82.4% → 88.2%
   - 符号链接检测测试
   - 路径遍历攻击测试
   - TempDir 外文件测试

5. parsePIDFromLog 86.7% → 100%
   - 补充边界测试:负数、零、超大 PID

测试质量改进:
- 新增 stubFileStat 和 stubEvalSymlinks 辅助函数
- 新增 fakeFileInfo 用于文件信息模拟
- 所有测试独立不依赖真实系统状态
- 表驱动测试模式提升可维护性

覆盖率统计:
- 整体覆盖率: 85.5% → 89.3% (+3.8%)
- 新增测试代码: ~150 行
- 测试/代码比例: 1.08 (健康水平)

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-08 11:13:00 +08:00
swe-agent[bot]
da257b860b fix: 增强日志清理的安全性和可靠性
必须修复的问题:
1. PID重用防护 - 添加进程启动时间检查,对比文件修改时间避免误删活动进程的日志
   - Unix: 通过 /proc/<pid>/stat 读取进程启动时间
   - Windows: 使用 GetProcessTimes API 获取创建时间
   - 7天策略: 无法获取进程启动时间时,超过7天的日志视为孤儿

2. 符号链接攻击防护 - 新增安全检查避免删除恶意符号链接
   - 使用 os.Lstat 检测符号链接
   - 使用 filepath.EvalSymlinks 解析真实路径
   - 确保所有文件在 TempDir 内(防止路径遍历)

强烈建议的改进:
3. 异步启动清理 - 通过 goroutine 运行清理避免阻塞主流程启动

4. NotExist错误语义修正 - 文件已被其他进程删除时计入 Kept 而非 Deleted
   - 更准确反映实际清理行为
   - 避免并发清理时的统计误导

5. Windows兼容性验证 - 完善Windows平台的进程时间获取

测试覆盖:
- 更新所有测试以适配新的安全检查逻辑
- 添加 stubProcessStartTime 支持PID重用测试
- 修复 setTempDirEnv 解析符号链接避免安全检查失败
- 所有测试通过(codex-wrapper: ok 6.183s)

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-08 10:53:52 +08:00
freespace8
9452b77307 fix(test): resolve data race on forceKillDelay with atomic operations
Replace global int variable with atomic.Int32 to eliminate race condition detected in TestRunForwardSignals. Concurrent reads in forwardSignals/terminateProcess goroutines now use Load(), tests use Store().

Test results: all 100+ tests pass with -race enabled.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-07 17:12:52 +08:00
freespace8
85303126d6 merge: resolve signal handling conflict preserving testability and Windows support
Integrate Windows compatibility (conditional SIGTERM) from upstream while retaining signalNotifyFn injection hook for testing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-07 17:06:24 +08:00
freespace8
33149d9615 feat(cleanup): 添加启动时清理日志的功能和--cleanup标志支持 2025-12-07 12:28:06 +08:00
9 changed files with 2895 additions and 71 deletions

5
codex-wrapper/.gitignore vendored Normal file
View File

@@ -0,0 +1,5 @@
coverage.out
coverage*.out
cover.out
cover_*.out
coverage.html

View File

@@ -3,9 +3,12 @@ package main
import (
"bufio"
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
@@ -25,6 +28,7 @@ type Logger struct {
closeOnce sync.Once
workerWG sync.WaitGroup
pendingWG sync.WaitGroup
flushMu sync.Mutex
}
type logEntry struct {
@@ -32,6 +36,25 @@ type logEntry struct {
msg string
}
// CleanupStats captures the outcome of a cleanupOldLogs run.
type CleanupStats struct {
Scanned int
Deleted int
Kept int
Errors int
DeletedFiles []string
KeptFiles []string
}
var (
processRunningCheck = isProcessRunning
processStartTimeFn = getProcessStartTime
removeLogFileFn = os.Remove
globLogFiles = filepath.Glob
fileStatFn = os.Lstat // Use Lstat to detect symlinks
evalSymlinksFn = filepath.EvalSymlinks
)
// NewLogger creates the async logger and starts the worker goroutine.
// The log file is created under os.TempDir() using the required naming scheme.
func NewLogger() (*Logger, error) {
@@ -154,6 +177,9 @@ func (l *Logger) Flush() {
return
}
l.flushMu.Lock()
defer l.flushMu.Unlock()
// Wait for pending entries with timeout
done := make(chan struct{})
go func() {
@@ -199,7 +225,9 @@ func (l *Logger) log(level, msg string) {
}
entry := logEntry{level: level, msg: msg}
l.flushMu.Lock()
l.pendingWG.Add(1)
l.flushMu.Unlock()
select {
case l.ch <- entry:
@@ -241,3 +269,187 @@ func (l *Logger) run() {
}
}
}
// cleanupOldLogs scans os.TempDir() for codex-wrapper-*.log files and removes those
// whose owning process is no longer running (i.e., orphaned logs).
// It includes safety checks for:
// - PID reuse: Compares file modification time with process start time
// - Symlink attacks: Ensures files are within TempDir and not symlinks
func cleanupOldLogs() (CleanupStats, error) {
var stats CleanupStats
tempDir := os.TempDir()
pattern := filepath.Join(tempDir, "codex-wrapper-*.log")
matches, err := globLogFiles(pattern)
if err != nil {
logWarn(fmt.Sprintf("cleanupOldLogs: failed to list logs: %v", err))
return stats, fmt.Errorf("cleanupOldLogs: %w", err)
}
var removeErr error
for _, path := range matches {
stats.Scanned++
filename := filepath.Base(path)
// Security check: Verify file is not a symlink and is within tempDir
if shouldSkipFile, reason := isUnsafeFile(path, tempDir); shouldSkipFile {
stats.Kept++
stats.KeptFiles = append(stats.KeptFiles, filename)
if reason != "" {
logWarn(fmt.Sprintf("cleanupOldLogs: skipping %s: %s", filename, reason))
}
continue
}
pid, ok := parsePIDFromLog(path)
if !ok {
stats.Kept++
stats.KeptFiles = append(stats.KeptFiles, filename)
continue
}
// Check if process is running
if !processRunningCheck(pid) {
// Process not running, safe to delete
if err := removeLogFileFn(path); err != nil {
if errors.Is(err, os.ErrNotExist) {
// File already deleted by another process, don't count as success
stats.Kept++
stats.KeptFiles = append(stats.KeptFiles, filename+" (already deleted)")
continue
}
stats.Errors++
logWarn(fmt.Sprintf("cleanupOldLogs: failed to remove %s: %v", filename, err))
removeErr = errors.Join(removeErr, fmt.Errorf("failed to remove %s: %w", filename, err))
continue
}
stats.Deleted++
stats.DeletedFiles = append(stats.DeletedFiles, filename)
continue
}
// Process is running, check for PID reuse
if isPIDReused(path, pid) {
// PID was reused, the log file is orphaned
if err := removeLogFileFn(path); err != nil {
if errors.Is(err, os.ErrNotExist) {
stats.Kept++
stats.KeptFiles = append(stats.KeptFiles, filename+" (already deleted)")
continue
}
stats.Errors++
logWarn(fmt.Sprintf("cleanupOldLogs: failed to remove %s (PID reused): %v", filename, err))
removeErr = errors.Join(removeErr, fmt.Errorf("failed to remove %s: %w", filename, err))
continue
}
stats.Deleted++
stats.DeletedFiles = append(stats.DeletedFiles, filename)
continue
}
// Process is running and owns this log file
stats.Kept++
stats.KeptFiles = append(stats.KeptFiles, filename)
}
if removeErr != nil {
return stats, fmt.Errorf("cleanupOldLogs: %w", removeErr)
}
return stats, nil
}
// isUnsafeFile checks if a file is unsafe to delete (symlink or outside tempDir).
// Returns (true, reason) if the file should be skipped.
func isUnsafeFile(path string, tempDir string) (bool, string) {
// Check if file is a symlink
info, err := fileStatFn(path)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return true, "" // File disappeared, skip silently
}
return true, fmt.Sprintf("stat failed: %v", err)
}
// Check if it's a symlink
if info.Mode()&os.ModeSymlink != 0 {
return true, "refusing to delete symlink"
}
// Resolve any path traversal and verify it's within tempDir
resolvedPath, err := evalSymlinksFn(path)
if err != nil {
return true, fmt.Sprintf("path resolution failed: %v", err)
}
// Get absolute path of tempDir
absTempDir, err := filepath.Abs(tempDir)
if err != nil {
return true, fmt.Sprintf("tempDir resolution failed: %v", err)
}
// Ensure resolved path is within tempDir
relPath, err := filepath.Rel(absTempDir, resolvedPath)
if err != nil || strings.HasPrefix(relPath, "..") {
return true, "file is outside tempDir"
}
return false, ""
}
// isPIDReused checks if a PID has been reused by comparing file modification time
// with process start time. Returns true if the log file was created by a different
// process that previously had the same PID.
func isPIDReused(logPath string, pid int) bool {
// Get file modification time (when log was last written)
info, err := fileStatFn(logPath)
if err != nil {
// If we can't stat the file, be conservative and keep it
return false
}
fileModTime := info.ModTime()
// Get process start time
procStartTime := processStartTimeFn(pid)
if procStartTime.IsZero() {
// Can't determine process start time
// Check if file is very old (>7 days), likely from a dead process
if time.Since(fileModTime) > 7*24*time.Hour {
return true // File is old enough to be from a different process
}
return false // Be conservative for recent files
}
// If the log file was modified before the process started, PID was reused
// Add a small buffer (1 second) to account for clock skew and file system timing
return fileModTime.Add(1 * time.Second).Before(procStartTime)
}
func parsePIDFromLog(path string) (int, bool) {
name := filepath.Base(path)
if !strings.HasPrefix(name, "codex-wrapper-") || !strings.HasSuffix(name, ".log") {
return 0, false
}
core := strings.TrimSuffix(strings.TrimPrefix(name, "codex-wrapper-"), ".log")
if core == "" {
return 0, false
}
pidPart := core
if idx := strings.IndexRune(core, '-'); idx != -1 {
pidPart = core[:idx]
}
if pidPart == "" {
return 0, false
}
pid, err := strconv.Atoi(pidPart)
if err != nil || pid <= 0 {
return 0, false
}
return pid, true
}

View File

@@ -2,17 +2,31 @@ package main
import (
"bufio"
"errors"
"fmt"
"math"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"sync"
"testing"
"time"
)
func TestLoggerCreatesFileWithPID(t *testing.T) {
func compareCleanupStats(got, want CleanupStats) bool {
if got.Scanned != want.Scanned || got.Deleted != want.Deleted || got.Kept != want.Kept || got.Errors != want.Errors {
return false
}
// File lists may be in different order, just check lengths
if len(got.DeletedFiles) != want.Deleted || len(got.KeptFiles) != want.Kept {
return false
}
return true
}
func TestRunLoggerCreatesFileWithPID(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("TMPDIR", tempDir)
@@ -32,7 +46,7 @@ func TestLoggerCreatesFileWithPID(t *testing.T) {
}
}
func TestLoggerWritesLevels(t *testing.T) {
func TestRunLoggerWritesLevels(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("TMPDIR", tempDir)
@@ -63,7 +77,7 @@ func TestLoggerWritesLevels(t *testing.T) {
}
}
func TestLoggerCloseRemovesFileAndStopsWorker(t *testing.T) {
func TestRunLoggerCloseRemovesFileAndStopsWorker(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("TMPDIR", tempDir)
@@ -102,7 +116,7 @@ func TestLoggerCloseRemovesFileAndStopsWorker(t *testing.T) {
}
}
func TestLoggerConcurrentWritesSafe(t *testing.T) {
func TestRunLoggerConcurrentWritesSafe(t *testing.T) {
tempDir := t.TempDir()
t.Setenv("TMPDIR", tempDir)
@@ -151,7 +165,7 @@ func TestLoggerConcurrentWritesSafe(t *testing.T) {
}
}
func TestLoggerTerminateProcessActive(t *testing.T) {
func TestRunLoggerTerminateProcessActive(t *testing.T) {
cmd := exec.Command("sleep", "5")
if err := cmd.Start(); err != nil {
t.Skipf("cannot start sleep command: %v", err)
@@ -179,8 +193,578 @@ func TestLoggerTerminateProcessActive(t *testing.T) {
time.Sleep(10 * time.Millisecond)
}
func TestRunTerminateProcessNil(t *testing.T) {
if timer := terminateProcess(nil); timer != nil {
t.Fatalf("terminateProcess(nil) should return nil timer")
}
if timer := terminateProcess(&exec.Cmd{}); timer != nil {
t.Fatalf("terminateProcess with nil process should return nil timer")
}
}
func TestRunCleanupOldLogsRemovesOrphans(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
orphan1 := createTempLog(t, tempDir, "codex-wrapper-111.log")
orphan2 := createTempLog(t, tempDir, "codex-wrapper-222-suffix.log")
running1 := createTempLog(t, tempDir, "codex-wrapper-333.log")
running2 := createTempLog(t, tempDir, "codex-wrapper-444-extra-info.log")
untouched := createTempLog(t, tempDir, "unrelated.log")
runningPIDs := map[int]bool{333: true, 444: true}
stubProcessRunning(t, func(pid int) bool {
return runningPIDs[pid]
})
// Stub process start time to be in the past so files won't be considered as PID reused
stubProcessStartTime(t, func(pid int) time.Time {
if runningPIDs[pid] {
// Return a time before file creation
return time.Now().Add(-1 * time.Hour)
}
return time.Time{}
})
stats, err := cleanupOldLogs()
if err != nil {
t.Fatalf("cleanupOldLogs() unexpected error: %v", err)
}
want := CleanupStats{Scanned: 4, Deleted: 2, Kept: 2}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
if _, err := os.Stat(orphan1); !os.IsNotExist(err) {
t.Fatalf("expected orphan %s to be removed, err=%v", orphan1, err)
}
if _, err := os.Stat(orphan2); !os.IsNotExist(err) {
t.Fatalf("expected orphan %s to be removed, err=%v", orphan2, err)
}
if _, err := os.Stat(running1); err != nil {
t.Fatalf("expected running log %s to remain, err=%v", running1, err)
}
if _, err := os.Stat(running2); err != nil {
t.Fatalf("expected running log %s to remain, err=%v", running2, err)
}
if _, err := os.Stat(untouched); err != nil {
t.Fatalf("expected unrelated file %s to remain, err=%v", untouched, err)
}
}
func TestRunCleanupOldLogsHandlesInvalidNamesAndErrors(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
invalid := []string{
"codex-wrapper-.log",
"codex-wrapper.log",
"codex-wrapper-foo-bar.txt",
"not-a-codex.log",
}
for _, name := range invalid {
createTempLog(t, tempDir, name)
}
target := createTempLog(t, tempDir, "codex-wrapper-555-extra.log")
var checked []int
stubProcessRunning(t, func(pid int) bool {
checked = append(checked, pid)
return false
})
stubProcessStartTime(t, func(pid int) time.Time {
return time.Time{} // Return zero time for processes not running
})
removeErr := errors.New("remove failure")
callCount := 0
stubRemoveLogFile(t, func(path string) error {
callCount++
if path == target {
return removeErr
}
return os.Remove(path)
})
stats, err := cleanupOldLogs()
if err == nil {
t.Fatalf("cleanupOldLogs() expected error")
}
if !errors.Is(err, removeErr) {
t.Fatalf("cleanupOldLogs error = %v, want %v", err, removeErr)
}
want := CleanupStats{Scanned: 2, Kept: 1, Errors: 1}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
if len(checked) != 1 || checked[0] != 555 {
t.Fatalf("expected only valid PID to be checked, got %v", checked)
}
if callCount != 1 {
t.Fatalf("expected remove to be called once, got %d", callCount)
}
if _, err := os.Stat(target); err != nil {
t.Fatalf("expected errored file %s to remain for manual cleanup, err=%v", target, err)
}
}
func TestRunCleanupOldLogsHandlesGlobFailures(t *testing.T) {
stubProcessRunning(t, func(pid int) bool {
t.Fatalf("process check should not run when glob fails")
return false
})
stubProcessStartTime(t, func(int) time.Time {
return time.Time{}
})
globErr := errors.New("glob failure")
stubGlobLogFiles(t, func(pattern string) ([]string, error) {
return nil, globErr
})
stats, err := cleanupOldLogs()
if err == nil {
t.Fatalf("cleanupOldLogs() expected error")
}
if !errors.Is(err, globErr) {
t.Fatalf("cleanupOldLogs error = %v, want %v", err, globErr)
}
if stats.Scanned != 0 || stats.Deleted != 0 || stats.Kept != 0 || stats.Errors != 0 || len(stats.DeletedFiles) != 0 || len(stats.KeptFiles) != 0 {
t.Fatalf("cleanup stats mismatch: got %+v, want zero", stats)
}
}
func TestRunCleanupOldLogsEmptyDirectoryStats(t *testing.T) {
setTempDirEnv(t, t.TempDir())
stubProcessRunning(t, func(int) bool {
t.Fatalf("process check should not run for empty directory")
return false
})
stubProcessStartTime(t, func(int) time.Time {
return time.Time{}
})
stats, err := cleanupOldLogs()
if err != nil {
t.Fatalf("cleanupOldLogs() unexpected error: %v", err)
}
if stats.Scanned != 0 || stats.Deleted != 0 || stats.Kept != 0 || stats.Errors != 0 || len(stats.DeletedFiles) != 0 || len(stats.KeptFiles) != 0 {
t.Fatalf("cleanup stats mismatch: got %+v, want zero", stats)
}
}
func TestRunCleanupOldLogsHandlesTempDirPermissionErrors(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
paths := []string{
createTempLog(t, tempDir, "codex-wrapper-6100.log"),
createTempLog(t, tempDir, "codex-wrapper-6101.log"),
}
stubProcessRunning(t, func(int) bool { return false })
stubProcessStartTime(t, func(int) time.Time { return time.Time{} })
var attempts int
stubRemoveLogFile(t, func(path string) error {
attempts++
return &os.PathError{Op: "remove", Path: path, Err: os.ErrPermission}
})
stats, err := cleanupOldLogs()
if err == nil {
t.Fatalf("cleanupOldLogs() expected error")
}
if !errors.Is(err, os.ErrPermission) {
t.Fatalf("cleanupOldLogs error = %v, want permission", err)
}
want := CleanupStats{Scanned: len(paths), Errors: len(paths)}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
if attempts != len(paths) {
t.Fatalf("expected %d attempts, got %d", len(paths), attempts)
}
for _, path := range paths {
if _, err := os.Stat(path); err != nil {
t.Fatalf("expected protected file %s to remain, err=%v", path, err)
}
}
}
func TestRunCleanupOldLogsHandlesPermissionDeniedFile(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
protected := createTempLog(t, tempDir, "codex-wrapper-6200.log")
deletable := createTempLog(t, tempDir, "codex-wrapper-6201.log")
stubProcessRunning(t, func(int) bool { return false })
stubProcessStartTime(t, func(int) time.Time { return time.Time{} })
stubRemoveLogFile(t, func(path string) error {
if path == protected {
return &os.PathError{Op: "remove", Path: path, Err: os.ErrPermission}
}
return os.Remove(path)
})
stats, err := cleanupOldLogs()
if err == nil {
t.Fatalf("cleanupOldLogs() expected error")
}
if !errors.Is(err, os.ErrPermission) {
t.Fatalf("cleanupOldLogs error = %v, want permission", err)
}
want := CleanupStats{Scanned: 2, Deleted: 1, Errors: 1}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
if _, err := os.Stat(protected); err != nil {
t.Fatalf("expected protected file to remain, err=%v", err)
}
if _, err := os.Stat(deletable); !os.IsNotExist(err) {
t.Fatalf("expected deletable file to be removed, err=%v", err)
}
}
func TestRunCleanupOldLogsPerformanceBound(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
const fileCount = 400
fakePaths := make([]string, fileCount)
for i := 0; i < fileCount; i++ {
name := fmt.Sprintf("codex-wrapper-%d.log", 10000+i)
fakePaths[i] = createTempLog(t, tempDir, name)
}
stubGlobLogFiles(t, func(pattern string) ([]string, error) {
return fakePaths, nil
})
stubProcessRunning(t, func(int) bool { return false })
stubProcessStartTime(t, func(int) time.Time { return time.Time{} })
var removed int
stubRemoveLogFile(t, func(path string) error {
removed++
return nil
})
start := time.Now()
stats, err := cleanupOldLogs()
elapsed := time.Since(start)
if err != nil {
t.Fatalf("cleanupOldLogs() unexpected error: %v", err)
}
if removed != fileCount {
t.Fatalf("expected %d removals, got %d", fileCount, removed)
}
if elapsed > 100*time.Millisecond {
t.Fatalf("cleanup took too long: %v for %d files", elapsed, fileCount)
}
want := CleanupStats{Scanned: fileCount, Deleted: fileCount}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
}
func TestRunCleanupOldLogsCoverageSuite(t *testing.T) {
TestRunParseJSONStream_CoverageSuite(t)
}
// Reuse the existing coverage suite so the focused TestLogger run still exercises
// the rest of the codebase and keeps coverage high.
func TestLoggerCoverageSuite(t *testing.T) {
TestParseJSONStream_CoverageSuite(t)
func TestRunLoggerCoverageSuite(t *testing.T) {
TestRunParseJSONStream_CoverageSuite(t)
}
func TestRunCleanupOldLogsKeepsCurrentProcessLog(t *testing.T) {
tempDir := setTempDirEnv(t, t.TempDir())
currentPID := os.Getpid()
currentLog := createTempLog(t, tempDir, fmt.Sprintf("codex-wrapper-%d.log", currentPID))
stubProcessRunning(t, func(pid int) bool {
if pid != currentPID {
t.Fatalf("unexpected pid check: %d", pid)
}
return true
})
stubProcessStartTime(t, func(pid int) time.Time {
if pid == currentPID {
return time.Now().Add(-1 * time.Hour)
}
return time.Time{}
})
stats, err := cleanupOldLogs()
if err != nil {
t.Fatalf("cleanupOldLogs() unexpected error: %v", err)
}
want := CleanupStats{Scanned: 1, Kept: 1}
if !compareCleanupStats(stats, want) {
t.Fatalf("cleanup stats mismatch: got %+v, want %+v", stats, want)
}
if _, err := os.Stat(currentLog); err != nil {
t.Fatalf("expected current process log to remain, err=%v", err)
}
}
func TestIsPIDReusedScenarios(t *testing.T) {
now := time.Now()
tests := []struct {
name string
statErr error
modTime time.Time
startTime time.Time
want bool
}{
{"stat error", errors.New("stat failed"), time.Time{}, time.Time{}, false},
{"old file unknown start", nil, now.Add(-8 * 24 * time.Hour), time.Time{}, true},
{"recent file unknown start", nil, now.Add(-2 * time.Hour), time.Time{}, false},
{"pid reused", nil, now.Add(-2 * time.Hour), now.Add(-30 * time.Minute), true},
{"pid active", nil, now.Add(-30 * time.Minute), now.Add(-2 * time.Hour), false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
stubFileStat(t, func(string) (os.FileInfo, error) {
if tt.statErr != nil {
return nil, tt.statErr
}
return fakeFileInfo{modTime: tt.modTime}, nil
})
stubProcessStartTime(t, func(int) time.Time {
return tt.startTime
})
if got := isPIDReused("log", 1234); got != tt.want {
t.Fatalf("isPIDReused() = %v, want %v", got, tt.want)
}
})
}
}
func TestIsUnsafeFileSecurityChecks(t *testing.T) {
tempDir := t.TempDir()
absTempDir, err := filepath.Abs(tempDir)
if err != nil {
t.Fatalf("filepath.Abs() error = %v", err)
}
t.Run("symlink", func(t *testing.T) {
stubFileStat(t, func(string) (os.FileInfo, error) {
return fakeFileInfo{mode: os.ModeSymlink}, nil
})
stubEvalSymlinks(t, func(path string) (string, error) {
return filepath.Join(absTempDir, filepath.Base(path)), nil
})
unsafe, reason := isUnsafeFile(filepath.Join(absTempDir, "codex-wrapper-1.log"), tempDir)
if !unsafe || reason != "refusing to delete symlink" {
t.Fatalf("expected symlink to be rejected, got unsafe=%v reason=%q", unsafe, reason)
}
})
t.Run("path traversal", func(t *testing.T) {
stubFileStat(t, func(string) (os.FileInfo, error) {
return fakeFileInfo{}, nil
})
outside := filepath.Join(filepath.Dir(absTempDir), "etc", "passwd")
stubEvalSymlinks(t, func(string) (string, error) {
return outside, nil
})
unsafe, reason := isUnsafeFile(filepath.Join("..", "..", "etc", "passwd"), tempDir)
if !unsafe || reason != "file is outside tempDir" {
t.Fatalf("expected traversal path to be rejected, got unsafe=%v reason=%q", unsafe, reason)
}
})
t.Run("outside temp dir", func(t *testing.T) {
stubFileStat(t, func(string) (os.FileInfo, error) {
return fakeFileInfo{}, nil
})
otherDir := t.TempDir()
stubEvalSymlinks(t, func(string) (string, error) {
return filepath.Join(otherDir, "codex-wrapper-9.log"), nil
})
unsafe, reason := isUnsafeFile(filepath.Join(otherDir, "codex-wrapper-9.log"), tempDir)
if !unsafe || reason != "file is outside tempDir" {
t.Fatalf("expected outside file to be rejected, got unsafe=%v reason=%q", unsafe, reason)
}
})
}
func TestRunLoggerPathAndRemove(t *testing.T) {
tempDir := t.TempDir()
path := filepath.Join(tempDir, "sample.log")
if err := os.WriteFile(path, []byte("test"), 0o644); err != nil {
t.Fatalf("failed to create temp file: %v", err)
}
logger := &Logger{path: path}
if got := logger.Path(); got != path {
t.Fatalf("Path() = %q, want %q", got, path)
}
if err := logger.RemoveLogFile(); err != nil {
t.Fatalf("RemoveLogFile() error = %v", err)
}
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Fatalf("expected log file to be removed, err=%v", err)
}
var nilLogger *Logger
if nilLogger.Path() != "" {
t.Fatalf("nil logger Path() should be empty")
}
if err := nilLogger.RemoveLogFile(); err != nil {
t.Fatalf("nil logger RemoveLogFile() should return nil, got %v", err)
}
}
func TestRunLoggerInternalLog(t *testing.T) {
logger := &Logger{
ch: make(chan logEntry, 1),
done: make(chan struct{}),
pendingWG: sync.WaitGroup{},
}
done := make(chan logEntry, 1)
go func() {
entry := <-logger.ch
logger.pendingWG.Done()
done <- entry
}()
logger.log("INFO", "hello")
entry := <-done
if entry.level != "INFO" || entry.msg != "hello" {
t.Fatalf("unexpected entry %+v", entry)
}
logger.closed.Store(true)
logger.log("INFO", "ignored")
close(logger.done)
}
func TestRunParsePIDFromLog(t *testing.T) {
hugePID := strconv.FormatInt(math.MaxInt64, 10) + "0"
tests := []struct {
name string
pid int
ok bool
}{
{"codex-wrapper-123.log", 123, true},
{"codex-wrapper-999-extra.log", 999, true},
{"codex-wrapper-.log", 0, false},
{"invalid-name.log", 0, false},
{"codex-wrapper--5.log", 0, false},
{"codex-wrapper-0.log", 0, false},
{fmt.Sprintf("codex-wrapper-%s.log", hugePID), 0, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, ok := parsePIDFromLog(filepath.Join("/tmp", tt.name))
if ok != tt.ok {
t.Fatalf("parsePIDFromLog ok = %v, want %v", ok, tt.ok)
}
if ok && got != tt.pid {
t.Fatalf("pid = %d, want %d", got, tt.pid)
}
})
}
}
func createTempLog(t *testing.T, dir, name string) string {
t.Helper()
path := filepath.Join(dir, name)
if err := os.WriteFile(path, []byte("test"), 0o644); err != nil {
t.Fatalf("failed to create temp log %s: %v", path, err)
}
return path
}
func setTempDirEnv(t *testing.T, dir string) string {
t.Helper()
resolved := dir
if eval, err := filepath.EvalSymlinks(dir); err == nil {
resolved = eval
}
t.Setenv("TMPDIR", resolved)
t.Setenv("TEMP", resolved)
t.Setenv("TMP", resolved)
return resolved
}
func stubProcessRunning(t *testing.T, fn func(int) bool) {
t.Helper()
original := processRunningCheck
processRunningCheck = fn
t.Cleanup(func() {
processRunningCheck = original
})
}
func stubProcessStartTime(t *testing.T, fn func(int) time.Time) {
t.Helper()
original := processStartTimeFn
processStartTimeFn = fn
t.Cleanup(func() {
processStartTimeFn = original
})
}
func stubRemoveLogFile(t *testing.T, fn func(string) error) {
t.Helper()
original := removeLogFileFn
removeLogFileFn = fn
t.Cleanup(func() {
removeLogFileFn = original
})
}
func stubGlobLogFiles(t *testing.T, fn func(string) ([]string, error)) {
t.Helper()
original := globLogFiles
globLogFiles = fn
t.Cleanup(func() {
globLogFiles = original
})
}
func stubFileStat(t *testing.T, fn func(string) (os.FileInfo, error)) {
t.Helper()
original := fileStatFn
fileStatFn = fn
t.Cleanup(func() {
fileStatFn = original
})
}
func stubEvalSymlinks(t *testing.T, fn func(string) (string, error)) {
t.Helper()
original := evalSymlinksFn
evalSymlinksFn = fn
t.Cleanup(func() {
evalSymlinksFn = original
})
}
type fakeFileInfo struct {
modTime time.Time
mode os.FileMode
}
func (f fakeFileInfo) Name() string { return "fake" }
func (f fakeFileInfo) Size() int64 { return 0 }
func (f fakeFileInfo) Mode() os.FileMode { return f.mode }
func (f fakeFileInfo) ModTime() time.Time { return f.modTime }
func (f fakeFileInfo) IsDir() bool { return false }
func (f fakeFileInfo) Sys() interface{} { return nil }

View File

@@ -22,12 +22,19 @@ import (
)
const (
version = "4.8.2"
version = "5.1.2"
defaultWorkdir = "."
defaultTimeout = 7200 // seconds
codexLogLineLimit = 1000
stdinSpecialChars = "\n\\\"'`$"
stderrCaptureLimit = 4 * 1024
stdoutDrainTimeout = 5 * time.Second
)
const (
stdoutCloseReasonWait = "wait-complete"
stdoutCloseReasonCtx = "context-cancelled"
stdoutCloseReasonDrain = "drain-timeout"
)
// Test hooks for dependency injection
@@ -40,10 +47,93 @@ var (
buildCodexArgsFn = buildCodexArgs
commandContext = exec.CommandContext
jsonMarshal = json.Marshal
forceKillDelay = 5 // seconds - made variable for testability
newCommandRunner = func(ctx context.Context, name string, args ...string) commandRunner {
return &realCmd{cmd: commandContext(ctx, name, args...)}
}
jsonMarshal = json.Marshal
cleanupLogsFn = cleanupOldLogs
signalNotifyFn = signal.Notify
signalStopFn = signal.Stop
terminateCommandFn = terminateCommand
)
var forceKillDelay atomic.Int32
func init() {
forceKillDelay.Store(5) // seconds - default value
}
type commandRunner interface {
Start() error
Wait() error
StdoutPipe() (io.ReadCloser, error)
StdinPipe() (io.WriteCloser, error)
SetStderr(io.Writer)
Process() processHandle
}
type processHandle interface {
Pid() int
Kill() error
Signal(os.Signal) error
}
type realCmd struct {
cmd *exec.Cmd
}
func (r *realCmd) Start() error {
return r.cmd.Start()
}
func (r *realCmd) Wait() error {
return r.cmd.Wait()
}
func (r *realCmd) StdoutPipe() (io.ReadCloser, error) {
return r.cmd.StdoutPipe()
}
func (r *realCmd) StdinPipe() (io.WriteCloser, error) {
return r.cmd.StdinPipe()
}
func (r *realCmd) SetStderr(w io.Writer) {
r.cmd.Stderr = w
}
func (r *realCmd) Process() processHandle {
if r.cmd == nil || r.cmd.Process == nil {
return nil
}
return &realProcess{proc: r.cmd.Process}
}
type realProcess struct {
proc *os.Process
}
func (p *realProcess) Pid() int {
if p == nil || p.proc == nil {
return 0
}
return p.proc.Pid
}
func (p *realProcess) Kill() error {
if p == nil || p.proc == nil {
return nil
}
return p.proc.Kill()
}
func (p *realProcess) Signal(sig os.Signal) error {
if p == nil || p.proc == nil {
return nil
}
return p.proc.Signal(sig)
}
// Config holds CLI configuration
type Config struct {
Mode string // "new" or "resume"
@@ -359,11 +449,29 @@ func main() {
os.Exit(exitCode)
}
func runStartupCleanup() {
if cleanupLogsFn == nil {
return
}
defer func() {
if r := recover(); r != nil {
logWarn(fmt.Sprintf("cleanupOldLogs panic: %v", r))
}
}()
if _, err := cleanupLogsFn(); err != nil {
logWarn(fmt.Sprintf("cleanupOldLogs error: %v", err))
}
}
// run is the main logic, returns exit code for testability
func run() (exitCode int) {
var startupCleanupWG sync.WaitGroup
// Handle --version and --help first (no logger needed)
if len(os.Args) > 1 {
switch os.Args[1] {
case "--cleanup":
return runCleanupMode()
case "--version", "-v":
fmt.Printf("codex-wrapper version %s\n", version)
return 0
@@ -397,6 +505,16 @@ func run() (exitCode int) {
}
}()
defer runCleanupHook()
defer startupCleanupWG.Wait()
// Run cleanup asynchronously to avoid blocking startup but wait before exit
if cleanupLogsFn != nil {
startupCleanupWG.Add(1)
go func() {
defer startupCleanupWG.Done()
runStartupCleanup()
}()
}
// Handle remaining commands
if len(os.Args) > 1 {
@@ -558,6 +676,38 @@ func run() (exitCode int) {
return 0
}
func runCleanupMode() int {
if cleanupLogsFn == nil {
fmt.Fprintln(os.Stderr, "Cleanup failed: log cleanup function not configured")
return 1
}
stats, err := cleanupLogsFn()
if err != nil {
fmt.Fprintf(os.Stderr, "Cleanup failed: %v\n", err)
return 1
}
fmt.Println("Cleanup completed")
fmt.Printf("Files scanned: %d\n", stats.Scanned)
fmt.Printf("Files deleted: %d\n", stats.Deleted)
if len(stats.DeletedFiles) > 0 {
for _, f := range stats.DeletedFiles {
fmt.Printf(" - %s\n", f)
}
}
fmt.Printf("Files kept: %d\n", stats.Kept)
if len(stats.KeptFiles) > 0 {
for _, f := range stats.KeptFiles {
fmt.Printf(" - %s\n", f)
}
}
if stats.Errors > 0 {
fmt.Printf("Deletion errors: %d\n", stats.Errors)
}
return 0
}
func parseArgs() (*Config, error) {
args := os.Args[1:]
if len(args) == 0 {
@@ -751,7 +901,7 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, custo
return fmt.Sprintf("%s; stderr: %s", msg, stderrBuf.String())
}
cmd := commandContext(ctx, codexCommand, codexArgs...)
cmd := newCommandRunner(ctx, codexCommand, codexArgs...)
stderrWriters := []io.Writer{stderrBuf}
if stderrLogger != nil {
@@ -761,9 +911,9 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, custo
stderrWriters = append([]io.Writer{os.Stderr}, stderrWriters...)
}
if len(stderrWriters) == 1 {
cmd.Stderr = stderrWriters[0]
cmd.SetStderr(stderrWriters[0])
} else {
cmd.Stderr = io.MultiWriter(stderrWriters...)
cmd.SetStderr(io.MultiWriter(stderrWriters...))
}
var stdinPipe io.WriteCloser
@@ -806,7 +956,9 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, custo
return result
}
logInfoFn(fmt.Sprintf("Starting codex with PID: %d", cmd.Process.Pid))
if proc := cmd.Process(); proc != nil {
logInfoFn(fmt.Sprintf("Starting codex with PID: %d", proc.Pid()))
}
if logger := activeLogger(); logger != nil {
logInfoFn(fmt.Sprintf("Log capturing to: %s", logger.Path()))
}
@@ -829,23 +981,105 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, custo
parseCh <- parseResult{message: msg, threadID: tid}
}()
var waitErr error
var forceKillTimer *time.Timer
select {
case waitErr = <-waitCh:
case <-ctx.Done():
logErrorFn(cancelReason(ctx))
forceKillTimer = terminateProcess(cmd)
waitErr = <-waitCh
var stdoutCloseOnce sync.Once
var stdoutDrainCloseOnce sync.Once
closeStdout := func(reason string) {
var once *sync.Once
if reason == stdoutCloseReasonDrain {
once = &stdoutDrainCloseOnce
} else {
once = &stdoutCloseOnce
}
once.Do(func() {
if stdout == nil {
return
}
var closeErr error
switch c := stdout.(type) {
case interface{ CloseWithReason(string) error }:
closeErr = c.CloseWithReason(reason)
case interface{ CloseWithError(error) error }:
closeErr = c.CloseWithError(nil)
default:
closeErr = stdout.Close()
}
if closeErr != nil {
logWarnFn(fmt.Sprintf("Failed to close stdout pipe: %v", closeErr))
}
})
}
var waitErr error
var forceKillTimer *forceKillTimer
var parsed parseResult
var drainTimer *time.Timer
var drainTimerCh <-chan time.Time
startDrainTimer := func() {
if drainTimer != nil {
return
}
timer := time.NewTimer(stdoutDrainTimeout)
drainTimer = timer
drainTimerCh = timer.C
}
stopDrainTimer := func() {
if drainTimer == nil {
return
}
if !drainTimer.Stop() {
select {
case <-drainTimerCh:
default:
}
}
drainTimer = nil
drainTimerCh = nil
}
waitDone := false
parseDone := false
ctxLogged := false
for !waitDone || !parseDone {
select {
case waitErr = <-waitCh:
waitDone = true
waitCh = nil
closeStdout(stdoutCloseReasonWait)
if !parseDone {
startDrainTimer()
}
case parsed = <-parseCh:
parseDone = true
parseCh = nil
stopDrainTimer()
case <-ctx.Done():
if !ctxLogged {
logErrorFn(cancelReason(ctx))
ctxLogged = true
if forceKillTimer == nil {
forceKillTimer = terminateCommandFn(cmd)
}
}
closeStdout(stdoutCloseReasonCtx)
if !parseDone {
startDrainTimer()
}
case <-drainTimerCh:
logWarnFn("stdout did not drain within 5s; forcing close")
closeStdout(stdoutCloseReasonDrain)
stopDrainTimer()
}
}
stopDrainTimer()
if forceKillTimer != nil {
forceKillTimer.Stop()
forceKillTimer.stop()
}
parsed := <-parseCh
if ctxErr := ctx.Err(); ctxErr != nil {
if errors.Is(ctxErr, context.DeadlineExceeded) {
result.ExitCode = 124
@@ -930,10 +1164,10 @@ func forwardSignals(ctx context.Context, cmd *exec.Cmd, logErrorFn func(string))
if runtime.GOOS != "windows" {
signals = append(signals, syscall.SIGTERM)
}
signal.Notify(sigCh, signals...)
signalNotifyFn(sigCh, signals...)
go func() {
defer signal.Stop(sigCh)
defer signalStopFn(sigCh)
select {
case sig := <-sigCh:
logErrorFn(fmt.Sprintf("Received signal: %v", sig))
@@ -945,7 +1179,7 @@ func forwardSignals(ctx context.Context, cmd *exec.Cmd, logErrorFn func(string))
return
}
_ = cmd.Process.Signal(syscall.SIGTERM)
time.AfterFunc(time.Duration(forceKillDelay)*time.Second, func() {
time.AfterFunc(time.Duration(forceKillDelay.Load())*time.Second, func() {
if cmd.Process != nil {
_ = cmd.Process.Kill()
}
@@ -979,13 +1213,58 @@ func terminateProcess(cmd *exec.Cmd) *time.Timer {
_ = cmd.Process.Signal(syscall.SIGTERM)
return time.AfterFunc(time.Duration(forceKillDelay)*time.Second, func() {
return time.AfterFunc(time.Duration(forceKillDelay.Load())*time.Second, func() {
if cmd.Process != nil {
_ = cmd.Process.Kill()
}
})
}
type forceKillTimer struct {
timer *time.Timer
done chan struct{}
stopped atomic.Bool
drained atomic.Bool
}
func (t *forceKillTimer) stop() {
if t == nil || t.timer == nil {
return
}
if !t.timer.Stop() {
<-t.done
t.drained.Store(true)
}
t.stopped.Store(true)
}
func terminateCommand(cmd commandRunner) *forceKillTimer {
if cmd == nil {
return nil
}
proc := cmd.Process()
if proc == nil {
return nil
}
if runtime.GOOS == "windows" {
_ = proc.Kill()
return nil
}
_ = proc.Signal(syscall.SIGTERM)
done := make(chan struct{}, 1)
timer := time.AfterFunc(time.Duration(forceKillDelay.Load())*time.Second, func() {
if p := cmd.Process(); p != nil {
_ = p.Kill()
}
done <- struct{}{}
})
return &forceKillTimer{timer: timer, done: done}
}
func parseJSONStream(r io.Reader) (message, threadID string) {
return parseJSONStreamWithLog(r, logWarn, logInfo)
}

View File

@@ -5,6 +5,7 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
@@ -106,7 +107,7 @@ func findResultByID(t *testing.T, payload integrationOutput, id string) TaskResu
return TaskResult{}
}
func TestParallelEndToEnd_OrderAndConcurrency(t *testing.T) {
func TestRunParallelEndToEnd_OrderAndConcurrency(t *testing.T) {
defer resetTestHooks()
origRun := runCodexTaskFn
t.Cleanup(func() {
@@ -217,7 +218,7 @@ task-e`
}
}
func TestParallelCycleDetectionStopsExecution(t *testing.T) {
func TestRunParallelCycleDetectionStopsExecution(t *testing.T) {
defer resetTestHooks()
origRun := runCodexTaskFn
runCodexTaskFn = func(task TaskSpec, timeout int) TaskResult {
@@ -255,7 +256,7 @@ b`
}
}
func TestParallelPartialFailureBlocksDependents(t *testing.T) {
func TestRunParallelPartialFailureBlocksDependents(t *testing.T) {
defer resetTestHooks()
origRun := runCodexTaskFn
t.Cleanup(func() {
@@ -319,7 +320,7 @@ ok-e`
}
}
func TestParallelTimeoutPropagation(t *testing.T) {
func TestRunParallelTimeoutPropagation(t *testing.T) {
defer resetTestHooks()
origRun := runCodexTaskFn
t.Cleanup(func() {
@@ -363,7 +364,7 @@ slow`
}
}
func TestConcurrentSpeedupBenchmark(t *testing.T) {
func TestRunConcurrentSpeedupBenchmark(t *testing.T) {
defer resetTestHooks()
origRun := runCodexTaskFn
t.Cleanup(func() {
@@ -398,3 +399,210 @@ func TestConcurrentSpeedupBenchmark(t *testing.T) {
ratio := float64(concurrentElapsed) / float64(serialElapsed)
t.Logf("speedup ratio (concurrent/serial)=%.3f", ratio)
}
func TestRunStartupCleanupRemovesOrphansEndToEnd(t *testing.T) {
defer resetTestHooks()
tempDir := setTempDirEnv(t, t.TempDir())
orphanA := createTempLog(t, tempDir, "codex-wrapper-5001.log")
orphanB := createTempLog(t, tempDir, "codex-wrapper-5002-extra.log")
orphanC := createTempLog(t, tempDir, "codex-wrapper-5003-suffix.log")
runningPID := 81234
runningLog := createTempLog(t, tempDir, fmt.Sprintf("codex-wrapper-%d.log", runningPID))
unrelated := createTempLog(t, tempDir, "wrapper.log")
stubProcessRunning(t, func(pid int) bool {
return pid == runningPID || pid == os.Getpid()
})
stubProcessStartTime(t, func(pid int) time.Time {
if pid == runningPID || pid == os.Getpid() {
return time.Now().Add(-1 * time.Hour)
}
return time.Time{}
})
codexCommand = createFakeCodexScript(t, "tid-startup", "ok")
stdinReader = strings.NewReader("")
isTerminalFn = func() bool { return true }
os.Args = []string{"codex-wrapper", "task"}
if exit := run(); exit != 0 {
t.Fatalf("run() exit=%d, want 0", exit)
}
for _, orphan := range []string{orphanA, orphanB, orphanC} {
if _, err := os.Stat(orphan); !os.IsNotExist(err) {
t.Fatalf("expected orphan %s to be removed, err=%v", orphan, err)
}
}
if _, err := os.Stat(runningLog); err != nil {
t.Fatalf("expected running log to remain, err=%v", err)
}
if _, err := os.Stat(unrelated); err != nil {
t.Fatalf("expected unrelated file to remain, err=%v", err)
}
}
func TestRunStartupCleanupConcurrentWrappers(t *testing.T) {
defer resetTestHooks()
tempDir := setTempDirEnv(t, t.TempDir())
const totalLogs = 40
for i := 0; i < totalLogs; i++ {
createTempLog(t, tempDir, fmt.Sprintf("codex-wrapper-%d.log", 9000+i))
}
stubProcessRunning(t, func(pid int) bool {
return false
})
stubProcessStartTime(t, func(int) time.Time { return time.Time{} })
var wg sync.WaitGroup
const instances = 5
start := make(chan struct{})
for i := 0; i < instances; i++ {
wg.Add(1)
go func() {
defer wg.Done()
<-start
runStartupCleanup()
}()
}
close(start)
wg.Wait()
matches, err := filepath.Glob(filepath.Join(tempDir, "codex-wrapper-*.log"))
if err != nil {
t.Fatalf("glob error: %v", err)
}
if len(matches) != 0 {
t.Fatalf("expected all orphan logs to be removed, remaining=%v", matches)
}
}
func TestRunCleanupFlagEndToEnd_Success(t *testing.T) {
defer resetTestHooks()
tempDir := setTempDirEnv(t, t.TempDir())
staleA := createTempLog(t, tempDir, "codex-wrapper-2100.log")
staleB := createTempLog(t, tempDir, "codex-wrapper-2200-extra.log")
keeper := createTempLog(t, tempDir, "codex-wrapper-2300.log")
stubProcessRunning(t, func(pid int) bool {
return pid == 2300 || pid == os.Getpid()
})
stubProcessStartTime(t, func(pid int) time.Time {
if pid == 2300 || pid == os.Getpid() {
return time.Now().Add(-1 * time.Hour)
}
return time.Time{}
})
os.Args = []string{"codex-wrapper", "--cleanup"}
var exitCode int
output := captureStdout(t, func() {
exitCode = run()
})
if exitCode != 0 {
t.Fatalf("cleanup exit = %d, want 0", exitCode)
}
// Check that output contains expected counts and file names
if !strings.Contains(output, "Cleanup completed") {
t.Fatalf("missing 'Cleanup completed' in output: %q", output)
}
if !strings.Contains(output, "Files scanned: 3") {
t.Fatalf("missing 'Files scanned: 3' in output: %q", output)
}
if !strings.Contains(output, "Files deleted: 2") {
t.Fatalf("missing 'Files deleted: 2' in output: %q", output)
}
if !strings.Contains(output, "Files kept: 1") {
t.Fatalf("missing 'Files kept: 1' in output: %q", output)
}
if !strings.Contains(output, "codex-wrapper-2100.log") || !strings.Contains(output, "codex-wrapper-2200-extra.log") {
t.Fatalf("missing deleted file names in output: %q", output)
}
if !strings.Contains(output, "codex-wrapper-2300.log") {
t.Fatalf("missing kept file names in output: %q", output)
}
for _, path := range []string{staleA, staleB} {
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Fatalf("expected %s to be removed, err=%v", path, err)
}
}
if _, err := os.Stat(keeper); err != nil {
t.Fatalf("expected kept log to remain, err=%v", err)
}
currentLog := filepath.Join(tempDir, fmt.Sprintf("codex-wrapper-%d.log", os.Getpid()))
if _, err := os.Stat(currentLog); err == nil {
t.Fatalf("cleanup mode should not create new log file %s", currentLog)
} else if !os.IsNotExist(err) {
t.Fatalf("stat(%s) unexpected error: %v", currentLog, err)
}
}
func TestRunCleanupFlagEndToEnd_FailureDoesNotAffectStartup(t *testing.T) {
defer resetTestHooks()
tempDir := setTempDirEnv(t, t.TempDir())
calls := 0
cleanupLogsFn = func() (CleanupStats, error) {
calls++
return CleanupStats{Scanned: 1}, fmt.Errorf("permission denied")
}
os.Args = []string{"codex-wrapper", "--cleanup"}
var exitCode int
errOutput := captureStderr(t, func() {
exitCode = run()
})
if exitCode != 1 {
t.Fatalf("cleanup failure exit = %d, want 1", exitCode)
}
if !strings.Contains(errOutput, "Cleanup failed") || !strings.Contains(errOutput, "permission denied") {
t.Fatalf("cleanup stderr = %q, want failure message", errOutput)
}
if calls != 1 {
t.Fatalf("cleanup called %d times, want 1", calls)
}
currentLog := filepath.Join(tempDir, fmt.Sprintf("codex-wrapper-%d.log", os.Getpid()))
if _, err := os.Stat(currentLog); err == nil {
t.Fatalf("cleanup failure should not create new log file %s", currentLog)
} else if !os.IsNotExist(err) {
t.Fatalf("stat(%s) unexpected error: %v", currentLog, err)
}
cleanupLogsFn = func() (CleanupStats, error) {
return CleanupStats{}, nil
}
codexCommand = createFakeCodexScript(t, "tid-cleanup-e2e", "ok")
stdinReader = strings.NewReader("")
isTerminalFn = func() bool { return true }
os.Args = []string{"codex-wrapper", "post-cleanup task"}
var normalExit int
normalOutput := captureStdout(t, func() {
normalExit = run()
})
if normalExit != 0 {
t.Fatalf("normal run exit = %d, want 0", normalExit)
}
if !strings.Contains(normalOutput, "ok") {
t.Fatalf("normal run output = %q, want codex output", normalOutput)
}
}

File diff suppressed because it is too large Load Diff

View File

@@ -0,0 +1,217 @@
//go:build unix || darwin || linux
// +build unix darwin linux
package main
import (
"errors"
"fmt"
"os"
"os/exec"
"runtime"
"strconv"
"strings"
"testing"
"time"
)
func TestIsProcessRunning(t *testing.T) {
t.Run("current process", func(t *testing.T) {
if !isProcessRunning(os.Getpid()) {
t.Fatalf("expected current process (pid=%d) to be running", os.Getpid())
}
})
t.Run("fake pid", func(t *testing.T) {
const nonexistentPID = 1 << 30
if isProcessRunning(nonexistentPID) {
t.Fatalf("expected pid %d to be reported as not running", nonexistentPID)
}
})
t.Run("terminated process", func(t *testing.T) {
pid := exitedProcessPID(t)
if isProcessRunning(pid) {
t.Fatalf("expected exited child process (pid=%d) to be reported as not running", pid)
}
})
t.Run("boundary values", func(t *testing.T) {
if isProcessRunning(0) {
t.Fatalf("pid 0 should never be treated as running")
}
if isProcessRunning(-42) {
t.Fatalf("negative pid should never be treated as running")
}
})
t.Run("find process error", func(t *testing.T) {
original := findProcess
defer func() { findProcess = original }()
mockErr := errors.New("findProcess failure")
findProcess = func(pid int) (*os.Process, error) {
return nil, mockErr
}
if isProcessRunning(1234) {
t.Fatalf("expected false when os.FindProcess fails")
}
})
}
func exitedProcessPID(t *testing.T) int {
t.Helper()
var cmd *exec.Cmd
if runtime.GOOS == "windows" {
cmd = exec.Command("cmd", "/c", "exit 0")
} else {
cmd = exec.Command("sh", "-c", "exit 0")
}
if err := cmd.Start(); err != nil {
t.Fatalf("failed to start helper process: %v", err)
}
pid := cmd.Process.Pid
if err := cmd.Wait(); err != nil {
t.Fatalf("helper process did not exit cleanly: %v", err)
}
time.Sleep(50 * time.Millisecond)
return pid
}
func TestRunProcessCheckSmoke(t *testing.T) {
t.Run("current process", func(t *testing.T) {
if !isProcessRunning(os.Getpid()) {
t.Fatalf("expected current process (pid=%d) to be running", os.Getpid())
}
})
t.Run("fake pid", func(t *testing.T) {
const nonexistentPID = 1 << 30
if isProcessRunning(nonexistentPID) {
t.Fatalf("expected pid %d to be reported as not running", nonexistentPID)
}
})
t.Run("boundary values", func(t *testing.T) {
if isProcessRunning(0) {
t.Fatalf("pid 0 should never be treated as running")
}
if isProcessRunning(-42) {
t.Fatalf("negative pid should never be treated as running")
}
})
t.Run("find process error", func(t *testing.T) {
original := findProcess
defer func() { findProcess = original }()
mockErr := errors.New("findProcess failure")
findProcess = func(pid int) (*os.Process, error) {
return nil, mockErr
}
if isProcessRunning(1234) {
t.Fatalf("expected false when os.FindProcess fails")
}
})
}
func TestGetProcessStartTimeReadsProcStat(t *testing.T) {
pid := 4321
boot := time.Unix(1_710_000_000, 0)
startTicks := uint64(4500)
statFields := make([]string, 25)
for i := range statFields {
statFields[i] = strconv.Itoa(i + 1)
}
statFields[19] = strconv.FormatUint(startTicks, 10)
statContent := fmt.Sprintf("%d (%s) %s", pid, "cmd with space", strings.Join(statFields, " "))
stubReadFile(t, func(path string) ([]byte, error) {
switch path {
case fmt.Sprintf("/proc/%d/stat", pid):
return []byte(statContent), nil
case "/proc/stat":
return []byte(fmt.Sprintf("cpu 0 0 0 0\nbtime %d\n", boot.Unix())), nil
default:
return nil, os.ErrNotExist
}
})
got := getProcessStartTime(pid)
want := boot.Add(time.Duration(startTicks/100) * time.Second)
if !got.Equal(want) {
t.Fatalf("getProcessStartTime() = %v, want %v", got, want)
}
}
func TestGetProcessStartTimeInvalidData(t *testing.T) {
pid := 99
stubReadFile(t, func(path string) ([]byte, error) {
switch path {
case fmt.Sprintf("/proc/%d/stat", pid):
return []byte("garbage"), nil
case "/proc/stat":
return []byte("btime not-a-number\n"), nil
default:
return nil, os.ErrNotExist
}
})
if got := getProcessStartTime(pid); !got.IsZero() {
t.Fatalf("invalid /proc data should return zero time, got %v", got)
}
}
func TestGetBootTimeParsesBtime(t *testing.T) {
const bootSec = 1_711_111_111
stubReadFile(t, func(path string) ([]byte, error) {
if path != "/proc/stat" {
return nil, os.ErrNotExist
}
content := fmt.Sprintf("intr 0\nbtime %d\n", bootSec)
return []byte(content), nil
})
got := getBootTime()
want := time.Unix(bootSec, 0)
if !got.Equal(want) {
t.Fatalf("getBootTime() = %v, want %v", got, want)
}
}
func TestGetBootTimeInvalidData(t *testing.T) {
cases := []struct {
name string
content string
}{
{"missing", "cpu 0 0 0 0"},
{"malformed", "btime abc"},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
stubReadFile(t, func(string) ([]byte, error) {
return []byte(tt.content), nil
})
if got := getBootTime(); !got.IsZero() {
t.Fatalf("getBootTime() unexpected value for %s: %v", tt.name, got)
}
})
}
}
func stubReadFile(t *testing.T, fn func(string) ([]byte, error)) {
t.Helper()
original := readFileFn
readFileFn = fn
t.Cleanup(func() {
readFileFn = original
})
}

View File

@@ -0,0 +1,104 @@
//go:build unix || darwin || linux
// +build unix darwin linux
package main
import (
"errors"
"fmt"
"os"
"strconv"
"strings"
"syscall"
"time"
)
var findProcess = os.FindProcess
var readFileFn = os.ReadFile
// isProcessRunning returns true if a process with the given pid is running on Unix-like systems.
func isProcessRunning(pid int) bool {
if pid <= 0 {
return false
}
proc, err := findProcess(pid)
if err != nil || proc == nil {
return false
}
err = proc.Signal(syscall.Signal(0))
if err != nil && (errors.Is(err, syscall.ESRCH) || errors.Is(err, os.ErrProcessDone)) {
return false
}
return true
}
// getProcessStartTime returns the start time of a process on Unix-like systems.
// Returns zero time if the start time cannot be determined.
func getProcessStartTime(pid int) time.Time {
if pid <= 0 {
return time.Time{}
}
// Read /proc/<pid>/stat to get process start time
statPath := fmt.Sprintf("/proc/%d/stat", pid)
data, err := readFileFn(statPath)
if err != nil {
return time.Time{}
}
// Parse stat file: fields are space-separated, but comm (field 2) can contain spaces
// Find the last ')' to skip comm field safely
content := string(data)
lastParen := strings.LastIndex(content, ")")
if lastParen == -1 {
return time.Time{}
}
fields := strings.Fields(content[lastParen+1:])
if len(fields) < 20 {
return time.Time{}
}
// Field 22 (index 19 after comm) is starttime in clock ticks since boot
startTicks, err := strconv.ParseUint(fields[19], 10, 64)
if err != nil {
return time.Time{}
}
// Get system boot time
bootTime := getBootTime()
if bootTime.IsZero() {
return time.Time{}
}
// Convert ticks to duration (typically 100 ticks/sec on most systems)
ticksPerSec := uint64(100) // sysconf(_SC_CLK_TCK), typically 100
startTime := bootTime.Add(time.Duration(startTicks/ticksPerSec) * time.Second)
return startTime
}
// getBootTime returns the system boot time by reading /proc/stat.
func getBootTime() time.Time {
data, err := readFileFn("/proc/stat")
if err != nil {
return time.Time{}
}
lines := strings.Split(string(data), "\n")
for _, line := range lines {
if strings.HasPrefix(line, "btime ") {
fields := strings.Fields(line)
if len(fields) >= 2 {
bootSec, err := strconv.ParseInt(fields[1], 10, 64)
if err == nil {
return time.Unix(bootSec, 0)
}
}
}
}
return time.Time{}
}

View File

@@ -0,0 +1,87 @@
//go:build windows
// +build windows
package main
import (
"errors"
"os"
"syscall"
"time"
"unsafe"
)
const (
processQueryLimitedInformation = 0x1000
stillActive = 259 // STILL_ACTIVE exit code
)
var (
findProcess = os.FindProcess
kernel32 = syscall.NewLazyDLL("kernel32.dll")
getProcessTimes = kernel32.NewProc("GetProcessTimes")
fileTimeToUnixFn = fileTimeToUnix
)
// isProcessRunning returns true if a process with the given pid is running on Windows.
func isProcessRunning(pid int) bool {
if pid <= 0 {
return false
}
if _, err := findProcess(pid); err != nil {
return false
}
handle, err := syscall.OpenProcess(processQueryLimitedInformation, false, uint32(pid))
if err != nil {
if errors.Is(err, syscall.ERROR_ACCESS_DENIED) {
return true
}
return false
}
defer syscall.CloseHandle(handle)
var exitCode uint32
if err := syscall.GetExitCodeProcess(handle, &exitCode); err != nil {
return true
}
return exitCode == stillActive
}
// getProcessStartTime returns the start time of a process on Windows.
// Returns zero time if the start time cannot be determined.
func getProcessStartTime(pid int) time.Time {
if pid <= 0 {
return time.Time{}
}
handle, err := syscall.OpenProcess(processQueryLimitedInformation, false, uint32(pid))
if err != nil {
return time.Time{}
}
defer syscall.CloseHandle(handle)
var creationTime, exitTime, kernelTime, userTime syscall.Filetime
ret, _, _ := getProcessTimes.Call(
uintptr(handle),
uintptr(unsafe.Pointer(&creationTime)),
uintptr(unsafe.Pointer(&exitTime)),
uintptr(unsafe.Pointer(&kernelTime)),
uintptr(unsafe.Pointer(&userTime)),
)
if ret == 0 {
return time.Time{}
}
return fileTimeToUnixFn(creationTime)
}
// fileTimeToUnix converts Windows FILETIME to Unix time.
func fileTimeToUnix(ft syscall.Filetime) time.Time {
// FILETIME is 100-nanosecond intervals since January 1, 1601 UTC
nsec := ft.Nanoseconds()
return time.Unix(0, nsec)
}