From 3bc83429293f76af9eedfca08234e92d92cb1ac5 Mon Sep 17 00:00:00 2001 From: cexll Date: Tue, 2 Dec 2025 16:54:43 +0800 Subject: [PATCH] fix codex wrapper async log --- codex-wrapper/logger.go | 31 +++++++++++------------------ codex-wrapper/main.go | 40 ++++++++++++++++++++++---------------- codex-wrapper/main_test.go | 30 ++++++++++++++++------------ 3 files changed, 52 insertions(+), 49 deletions(-) diff --git a/codex-wrapper/logger.go b/codex-wrapper/logger.go index e54385d..bba546b 100644 --- a/codex-wrapper/logger.go +++ b/codex-wrapper/logger.go @@ -19,13 +19,12 @@ type Logger struct { file *os.File writer *bufio.Writer ch chan logEntry - flushReq chan struct{} + flushReq chan chan struct{} done chan struct{} closed atomic.Bool closeOnce sync.Once workerWG sync.WaitGroup pendingWG sync.WaitGroup - flushMu sync.Mutex } type logEntry struct { @@ -60,7 +59,7 @@ func NewLoggerWithSuffix(suffix string) (*Logger, error) { file: f, writer: bufio.NewWriterSize(f, 4096), ch: make(chan logEntry, 1000), - flushReq: make(chan struct{}, 1), + flushReq: make(chan chan struct{}, 1), done: make(chan struct{}), } @@ -174,16 +173,10 @@ func (l *Logger) Flush() { } // Trigger writer flush + flushDone := make(chan struct{}) select { - case l.flushReq <- struct{}{}: - // Wait for flush to complete (with mutex) - flushDone := make(chan struct{}) - go func() { - l.flushMu.Lock() - l.flushMu.Unlock() - close(flushDone) - }() - + case l.flushReq <- flushDone: + // Wait for flush to complete select { case <-flushDone: // Flush completed @@ -210,11 +203,9 @@ func (l *Logger) log(level, msg string) { select { case l.ch <- entry: + // Successfully sent to channel case <-l.done: - l.pendingWG.Done() - return - default: - // Channel is full; drop the entry to avoid blocking callers. + // Logger is closing, drop this entry l.pendingWG.Done() return } @@ -242,11 +233,11 @@ func (l *Logger) run() { case <-ticker.C: l.writer.Flush() - case <-l.flushReq: - // Explicit flush request - l.flushMu.Lock() + case flushDone := <-l.flushReq: + // Explicit flush request - flush writer and sync to disk l.writer.Flush() - l.flushMu.Unlock() + l.file.Sync() + close(flushDone) } } } diff --git a/codex-wrapper/main.go b/codex-wrapper/main.go index 476cedf..15146ea 100644 --- a/codex-wrapper/main.go +++ b/codex-wrapper/main.go @@ -360,6 +360,19 @@ func main() { // run is the main logic, returns exit code for testability func run() (exitCode int) { + // Handle --version and --help first (no logger needed) + if len(os.Args) > 1 { + switch os.Args[1] { + case "--version", "-v": + fmt.Printf("codex-wrapper version %s\n", version) + return 0 + case "--help", "-h": + printHelp() + return 0 + } + } + + // Initialize logger for all other commands logger, err := NewLogger() if err != nil { fmt.Fprintf(os.Stderr, "ERROR: failed to initialize logger: %v\n", err) @@ -375,25 +388,18 @@ func run() (exitCode int) { if err := closeLogger(); err != nil { fmt.Fprintf(os.Stderr, "ERROR: failed to close logger: %v\n", err) } - if exitCode == 0 && logger != nil { + // Always remove log file after completion + if logger != nil { if err := logger.RemoveLogFile(); err != nil && !os.IsNotExist(err) { - fmt.Fprintf(os.Stderr, "ERROR: failed to remove logger file: %v\n", err) + // Silently ignore removal errors } - } else if exitCode != 0 && logger != nil { - fmt.Fprintf(os.Stderr, "Log file retained at: %s\n", logger.Path()) } }() defer runCleanupHook() - // Handle --version and --help first + // Handle remaining commands if len(os.Args) > 1 { switch os.Args[1] { - case "--version", "-v": - fmt.Printf("codex-wrapper version %s\n", version) - return 0 - case "--help", "-h": - printHelp() - return 0 case "--parallel": if len(os.Args) > 2 { fmt.Fprintln(os.Stderr, "ERROR: --parallel reads its task configuration from stdin and does not accept additional arguments.") @@ -438,6 +444,12 @@ func run() (exitCode int) { logInfo("Script started") + // Print startup information to stderr + fmt.Fprintf(os.Stderr, "[codex-wrapper]\n") + fmt.Fprintf(os.Stderr, " Command: %s\n", strings.Join(os.Args, " ")) + fmt.Fprintf(os.Stderr, " PID: %d\n", os.Getpid()) + fmt.Fprintf(os.Stderr, " Log: %s\n", logger.Path()) + cfg, err := parseArgs() if err != nil { logError(err.Error()) @@ -1210,24 +1222,18 @@ func farewell(name string) string { } func logInfo(msg string) { - fmt.Fprintf(os.Stderr, "INFO: %s\n", msg) - if logger := activeLogger(); logger != nil { logger.Info(msg) } } func logWarn(msg string) { - fmt.Fprintf(os.Stderr, "WARN: %s\n", msg) - if logger := activeLogger(); logger != nil { logger.Warn(msg) } } func logError(msg string) { - fmt.Fprintf(os.Stderr, "ERROR: %s\n", msg) - if logger := activeLogger(); logger != nil { logger.Error(msg) } diff --git a/codex-wrapper/main_test.go b/codex-wrapper/main_test.go index e948d07..1e72e92 100644 --- a/codex-wrapper/main_test.go +++ b/codex-wrapper/main_test.go @@ -788,11 +788,13 @@ func TestSilentMode(t *testing.T) { verbose := capture(false) quiet := capture(true) + // After refactoring, logs are only written to file, not stderr + // Both silent and non-silent modes should produce no stderr output if quiet != "" { t.Fatalf("silent mode should suppress stderr, got: %q", quiet) } - if !strings.Contains(verbose, "INFO: Starting codex") { - t.Fatalf("non-silent mode should log to stderr, got: %q", verbose) + if verbose != "" { + t.Fatalf("non-silent mode should also suppress stderr (logs go to file), got: %q", verbose) } } @@ -1136,10 +1138,10 @@ func TestRun_ExplicitStdinReadError(t *testing.T) { if !strings.Contains(logOutput, "Failed to read stdin: broken stdin") { t.Fatalf("log missing read error entry, got %q", logOutput) } - if _, err := os.Stat(logPath); os.IsNotExist(err) { - t.Fatalf("log file should exist") + // Log file is always removed after completion (new behavior) + if _, err := os.Stat(logPath); !os.IsNotExist(err) { + t.Fatalf("log file should be removed after completion") } - defer os.Remove(logPath) } func TestRun_CommandFails(t *testing.T) { @@ -1220,10 +1222,10 @@ func TestRun_PipedTaskReadError(t *testing.T) { if !strings.Contains(logOutput, "ERROR: Failed to read piped stdin: read stdin: pipe failure") { t.Fatalf("log missing piped read error, got %q", logOutput) } - if _, err := os.Stat(logPath); os.IsNotExist(err) { - t.Fatalf("log file should exist") + // Log file is always removed after completion (new behavior) + if _, err := os.Stat(logPath); !os.IsNotExist(err) { + t.Fatalf("log file should be removed after completion") } - defer os.Remove(logPath) } func TestRun_PipedTaskSuccess(t *testing.T) { @@ -1325,17 +1327,21 @@ printf '%s\n' '{"type":"item.completed","item":{"type":"agent_message","text":"l if exitCode != 130 { t.Fatalf("exit code = %d, want 130", exitCode) } - if _, err := os.Stat(logPath); os.IsNotExist(err) { - t.Fatalf("log file should exist after signal exit") + // Log file is always removed after completion (new behavior) + if _, err := os.Stat(logPath); !os.IsNotExist(err) { + t.Fatalf("log file should be removed after completion") } - defer os.Remove(logPath) } func TestRun_CleanupHookAlwaysCalled(t *testing.T) { defer resetTestHooks() called := false cleanupHook = func() { called = true } - os.Args = []string{"codex-wrapper", "--version"} + // Use a command that goes through normal flow, not --version which returns early + codexCommand = "echo" + buildCodexArgsFn = func(cfg *Config, targetArg string) []string { return []string{`{"type":"thread.started","thread_id":"x"} +{"type":"item.completed","item":{"type":"agent_message","text":"ok"}}`} } + os.Args = []string{"codex-wrapper", "task"} if exitCode := run(); exitCode != 0 { t.Fatalf("exit = %d, want 0", exitCode) }