fix(codeagent-wrapper): keep logs + surface parsed error output (#152)

Fixes #150
This commit is contained in:
ben
2026-02-27 10:05:58 +08:00
committed by GitHub
parent a39bf72bc2
commit b204ca94e2
3 changed files with 112 additions and 17 deletions

View File

@@ -200,10 +200,9 @@ func runWithLoggerAndCleanup(fn func() int) (exitCode int) {
for _, entry := range entries { for _, entry := range entries {
fmt.Fprintln(os.Stderr, entry) fmt.Fprintln(os.Stderr, entry)
} }
fmt.Fprintf(os.Stderr, "Log file: %s (deleted)\n", logger.Path()) fmt.Fprintf(os.Stderr, "Log file: %s\n", logger.Path())
} }
} }
_ = logger.RemoveLogFile()
}() }()
defer runCleanupHook() defer runCleanupHook()
@@ -733,6 +732,13 @@ func runSingleMode(cfg *Config, name string) int {
} }
if exitCode != 0 { if exitCode != 0 {
// Surface any parsed backend output even on non-zero exit to avoid "(no output)" in tool runners.
if strings.TrimSpace(result.Message) != "" {
fmt.Println(result.Message)
if result.SessionID != "" {
fmt.Printf("\n---\nSESSION_ID: %s\n", result.SessionID)
}
}
return exitCode return exitCode
} }

View File

@@ -4635,9 +4635,9 @@ func TestRun_ExplicitStdinReadError(t *testing.T) {
if !strings.Contains(logOutput, "Failed to read stdin: broken stdin") { if !strings.Contains(logOutput, "Failed to read stdin: broken stdin") {
t.Fatalf("log missing read error entry, got %q", logOutput) t.Fatalf("log missing read error entry, got %q", logOutput)
} }
// Log file is always removed after completion (new behavior) // Log file should remain for inspection; cleanup is handled via `codeagent-wrapper cleanup`.
if _, err := os.Stat(logPath); !os.IsNotExist(err) { if _, err := os.Stat(logPath); err != nil {
t.Fatalf("log file should be removed after completion") t.Fatalf("expected log file to exist after completion: %v", err)
} }
} }
@@ -4653,6 +4653,51 @@ func TestRun_CommandFails(t *testing.T) {
} }
} }
func TestRun_NonZeroExitPrintsParsedMessage(t *testing.T) {
defer resetTestHooks()
tempDir := t.TempDir()
var scriptPath string
if runtime.GOOS == "windows" {
scriptPath = filepath.Join(tempDir, "codex.bat")
script := "@echo off\r\n" +
"echo {\"type\":\"thread.started\",\"thread_id\":\"tid\"}\r\n" +
"echo {\"type\":\"item.completed\",\"item\":{\"type\":\"agent_message\",\"text\":\"parsed-error\"}}\r\n" +
"exit /b 1\r\n"
if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil {
t.Fatalf("failed to write script: %v", err)
}
} else {
scriptPath = filepath.Join(tempDir, "codex.sh")
script := `#!/bin/sh
printf '%s\n' '{"type":"thread.started","thread_id":"tid"}'
printf '%s\n' '{"type":"item.completed","item":{"type":"agent_message","text":"parsed-error"}}'
sleep 0.05
exit 1
`
if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil {
t.Fatalf("failed to write script: %v", err)
}
}
restore := withBackend(scriptPath, func(cfg *Config, targetArg string) []string { return []string{} })
defer restore()
os.Args = []string{"codeagent-wrapper", "task"}
stdinReader = strings.NewReader("")
isTerminalFn = func() bool { return true }
var exitCode int
output := captureOutput(t, func() { exitCode = run() })
if exitCode != 1 {
t.Fatalf("exit=%d, want 1", exitCode)
}
if !strings.Contains(output, "parsed-error") {
t.Fatalf("stdout=%q, want parsed backend message", output)
}
}
func TestRun_InvalidBackend(t *testing.T) { func TestRun_InvalidBackend(t *testing.T) {
defer resetTestHooks() defer resetTestHooks()
os.Args = []string{"codeagent-wrapper", "--backend", "unknown", "task"} os.Args = []string{"codeagent-wrapper", "--backend", "unknown", "task"}
@@ -4732,9 +4777,9 @@ func TestRun_PipedTaskReadError(t *testing.T) {
if !strings.Contains(logOutput, "Failed to read piped stdin: read stdin: pipe failure") { if !strings.Contains(logOutput, "Failed to read piped stdin: read stdin: pipe failure") {
t.Fatalf("log missing piped read error, got %q", logOutput) t.Fatalf("log missing piped read error, got %q", logOutput)
} }
// Log file is always removed after completion (new behavior) // Log file should remain for inspection; cleanup is handled via `codeagent-wrapper cleanup`.
if _, err := os.Stat(logPath); !os.IsNotExist(err) { if _, err := os.Stat(logPath); err != nil {
t.Fatalf("log file should be removed after completion") t.Fatalf("expected log file to exist after completion: %v", err)
} }
} }
@@ -4788,12 +4833,12 @@ func TestRun_LoggerLifecycle(t *testing.T) {
if !fileExisted { if !fileExisted {
t.Fatalf("log file was not present during run") t.Fatalf("log file was not present during run")
} }
if _, err := os.Stat(logPath); !os.IsNotExist(err) { if _, err := os.Stat(logPath); err != nil {
t.Fatalf("log file should be removed on success, but it exists") t.Fatalf("expected log file to exist on success: %v", err)
} }
} }
func TestRun_LoggerRemovedOnSignal(t *testing.T) { func TestRun_LoggerKeptOnSignal(t *testing.T) {
if runtime.GOOS == "windows" { if runtime.GOOS == "windows" {
t.Skip("signal-based test is not supported on Windows") t.Skip("signal-based test is not supported on Windows")
} }
@@ -4807,7 +4852,8 @@ func TestRun_LoggerRemovedOnSignal(t *testing.T) {
defer signal.Reset(syscall.SIGINT, syscall.SIGTERM) defer signal.Reset(syscall.SIGINT, syscall.SIGTERM)
// Set shorter delays for faster test // Set shorter delays for faster test
_ = executor.SetForceKillDelay(1) restoreForceKillDelay := executor.SetForceKillDelay(1)
defer restoreForceKillDelay()
tempDir := setTempDirEnv(t, t.TempDir()) tempDir := setTempDirEnv(t, t.TempDir())
logPath := filepath.Join(tempDir, fmt.Sprintf("codeagent-wrapper-%d.log", os.Getpid())) logPath := filepath.Join(tempDir, fmt.Sprintf("codeagent-wrapper-%d.log", os.Getpid()))
@@ -4830,13 +4876,19 @@ printf '%s\n' '{"type":"item.completed","item":{"type":"agent_message","text":"l
exitCh := make(chan int, 1) exitCh := make(chan int, 1)
go func() { exitCh <- run() }() go func() { exitCh <- run() }()
deadline := time.Now().Add(1 * time.Second) ready := false
deadline := time.Now().Add(2 * time.Second)
for time.Now().Before(deadline) { for time.Now().Before(deadline) {
if _, err := os.Stat(logPath); err == nil { data, err := os.ReadFile(logPath)
if err == nil && strings.Contains(string(data), "Starting ") {
ready = true
break break
} }
time.Sleep(10 * time.Millisecond) time.Sleep(10 * time.Millisecond)
} }
if !ready {
t.Fatalf("logger did not become ready before deadline")
}
if proc, err := os.FindProcess(os.Getpid()); err == nil && proc != nil { if proc, err := os.FindProcess(os.Getpid()); err == nil && proc != nil {
_ = proc.Signal(syscall.SIGINT) _ = proc.Signal(syscall.SIGINT)
@@ -4852,9 +4904,9 @@ printf '%s\n' '{"type":"item.completed","item":{"type":"agent_message","text":"l
if exitCode != 130 { if exitCode != 130 {
t.Fatalf("exit code = %d, want 130", exitCode) t.Fatalf("exit code = %d, want 130", exitCode)
} }
// Log file is always removed after completion (new behavior) // Log file should remain for inspection; cleanup is handled via `codeagent-wrapper cleanup`.
if _, err := os.Stat(logPath); !os.IsNotExist(err) { if _, err := os.Stat(logPath); err != nil {
t.Fatalf("log file should be removed after completion") t.Fatalf("expected log file to exist after completion: %v", err)
} }
} }
@@ -5115,6 +5167,34 @@ func TestParallelLogPathInSerialMode(t *testing.T) {
} }
} }
func TestRun_KeptLogFileOnSuccess(t *testing.T) {
defer resetTestHooks()
tempDir := setTempDirEnv(t, t.TempDir())
os.Args = []string{"codeagent-wrapper", "do-stuff"}
stdinReader = strings.NewReader("")
isTerminalFn = func() bool { return true }
codexCommand = createFakeCodexScript(t, "cli-session", "ok")
buildCodexArgsFn = func(cfg *Config, targetArg string) []string { return []string{} }
cleanupLogsFn = nil
var exitCode int
_ = captureStderr(t, func() {
_ = captureOutput(t, func() {
exitCode = run()
})
})
if exitCode != 0 {
t.Fatalf("run() exit = %d, want 0", exitCode)
}
expectedLog := filepath.Join(tempDir, fmt.Sprintf("codeagent-wrapper-%d.log", os.Getpid()))
if _, err := os.Stat(expectedLog); err != nil {
t.Fatalf("expected log file to exist: %v", err)
}
}
func TestRun_CLI_Success(t *testing.T) { func TestRun_CLI_Success(t *testing.T) {
defer resetTestHooks() defer resetTestHooks()
os.Args = []string{"codeagent-wrapper", "do-things"} os.Args = []string{"codeagent-wrapper", "do-things"}

View File

@@ -1435,6 +1435,15 @@ waitLoop:
logErrorFn(fmt.Sprintf("%s exited with status %d", commandName, code)) logErrorFn(fmt.Sprintf("%s exited with status %d", commandName, code))
result.ExitCode = code result.ExitCode = code
result.Error = attachStderr(fmt.Sprintf("%s exited with status %d", commandName, code)) result.Error = attachStderr(fmt.Sprintf("%s exited with status %d", commandName, code))
// Preserve parsed output when the backend exits non-zero (e.g. API error with stream-json output).
result.Message = parsed.message
result.SessionID = parsed.threadID
if stdoutLogger != nil {
stdoutLogger.Flush()
}
if stderrLogger != nil {
stderrLogger.Flush()
}
return result return result
} }
logErrorFn(commandName + " error: " + waitErr.Error()) logErrorFn(commandName + " error: " + waitErr.Error())