From a6ab203be57ff2ff72be2d3c599dd854d0f35f67 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Sat, 24 Oct 2026 13:24:21 -0400 Subject: [PATCH 1/4] test(harness): compare fixture exit codes - nine were never checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: a kaibo review of AGENTS.md (#49) found that checkProcess never compared Expect.ExitCode. Nine fixtures set it; none was tested. Amy: "yes, go ahead on a test PR if we found some gaps." Approach: checkProcess compares the exit code when the process exited on its own (a killed process reports -1). Fixture fixes: #360 now uses ExitCode: 42 in place of a CheckFunc; "protocol: bad config" with --fail expects 1. The three exec signal fixtures expect the shell convention, 128 + signal: 130 for SIGINT, 137 for SIGKILL. Their old values (2 and 0) were never checked and match no rule. Learned: otel-cli exits 255 when the exec child dies from a signal. ProcessState.ExitCode() is -1, and os.Exit(-1) is 255. Next: this commit fails on purpose (3 signal fixtures); the fix follows. 🤖 Claude Co-Authored-By: Claude Opus 5.5 --- data_for_test.go | 17 ++++++----------- main_test.go | 5 +++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index abfe8d1..df46c29 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -583,14 +583,8 @@ var suites = []FixtureSuite{ IsLongTest: true, }, Expect: Results{ - Config: otelcli.DefaultConfig(), - }, - CheckFuncs: []CheckFunc{ - func(t *testing.T, f Fixture, r Results) { - if r.ExitCode != 42 { - t.Errorf("expected exit code 42 from child process, got %d", r.ExitCode) - } - }, + Config: otelcli.DefaultConfig(), + ExitCode: 42, }, }, }, @@ -1155,6 +1149,7 @@ var suites = []FixtureSuite{ CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), CliOutput: "invalid protocol setting \"xxx\"\n", Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), + ExitCode: 1, // --fail Diagnostics: otelcli.Diagnostics{ IsRecording: false, NumArgs: 7, @@ -1403,7 +1398,7 @@ var suites = []FixtureSuite{ Expect: Results{ SpanCount: 1, Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), - ExitCode: 2, + ExitCode: 130, // 128 + SIGINT, as shells report it SpanData: map[string]string{ "status_code": "2", "status_description": "exec command failed: signal: interrupt", @@ -1423,7 +1418,7 @@ var suites = []FixtureSuite{ Expect: Results{ SpanCount: 1, Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), - ExitCode: 2, + ExitCode: 137, // 128 + SIGKILL SpanData: map[string]string{ "status_code": "2", "status_description": "exec command failed: signal: killed", @@ -1444,7 +1439,7 @@ var suites = []FixtureSuite{ Expect: Results{ SpanCount: 1, Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), - ExitCode: 0, + ExitCode: 137, // --command-timeout kills sleep at 100ms }, }, }, diff --git a/main_test.go b/main_test.go index 563c13e..ac0a1c0 100644 --- a/main_test.go +++ b/main_test.go @@ -207,6 +207,11 @@ func checkProcess(t *testing.T, fixture Fixture, results Results) bool { t.Errorf("[%s] command failed is %t but expected %t", fixture.Name, results.CommandFailed, fixture.Expect.CommandFailed) return false } + // a killed process has no exit code of its own, so only compare clean exits + if !results.TimedOut && !results.CommandFailed && results.ExitCode != fixture.Expect.ExitCode { + t.Errorf("[%s] exit code is %d but expected %d", fixture.Name, results.ExitCode, fixture.Expect.ExitCode) + return false + } return true } From 4442efd7d56a566a4ef79a1e673ea474fdefea5d Mon Sep 17 00:00:00 2001 From: A Tobey Date: Sat, 24 Oct 2026 13:25:14 -0400 Subject: [PATCH 2/4] fix(exec): exit 128 + signal when the child is killed - not 255 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: the harness now checks exit codes, and three exec fixtures showed otel-cli exits 255 when its child dies from a signal (Ctrl-C forwarded, or --command-timeout). ProcessState.ExitCode() is -1 for a signaled child, and os.Exit(-1) is 255, so scripts lose the signal. Approach: when ProcessState.Sys() is a syscall.WaitStatus and Signaled(), exit 128 + signal, as bash does: SIGINT 130, SIGKILL 137. WaitStatus exists on Windows too (Signaled is always false); windows, darwin and freebsd cross-build. Learned: --command-timeout kills with SIGKILL (exec.CommandContext), so a timeout exits 137, not timeout(1)'s 124. Next: Amy decides whether 128 + signal is the behavior to keep. Verified: go vet, go test ./... green; `otel-cli exec --command-timeout 50ms -- sleep 1` exits 137; `exec -- sh -c 'exit 3'` exits 3. 🤖 Claude Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ otelcli/exec.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4c627b..cc442bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ Begin agent-assisted development era. - README: improved Contributing section for both agents and humans - LICENSE: updated copyright to include Equinix Metal and 2025 A. Tobey +### Fixed + +- `exec` exits 128 + signal when the child is killed by a signal, not 255 + ## [0.5.0] - 2025-04-21 Fork otel-cli to @tobert's personal GitHub. diff --git a/otelcli/exec.go b/otelcli/exec.go index c8f7945..c15b9a6 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -8,6 +8,7 @@ import ( "os/signal" "os/user" "strings" + "syscall" "time" "github.com/spf13/cobra" @@ -156,6 +157,11 @@ func doExec(cmd *cobra.Command, args []string) { // capture the child's exit code before OTLP export so SoftFail can use it (#360) if child.ProcessState != nil { Diag.ExecExitCode = child.ProcessState.ExitCode() + // a child killed by a signal reports -1, which os.Exit turns into 255; + // exit 128 + signal instead, as shells do + if status, ok := child.ProcessState.Sys().(syscall.WaitStatus); ok && status.Signaled() { + Diag.ExecExitCode = 128 + int(status.Signal()) + } } else { Diag.ExecExitCode = 127 } From 662e30ce7ed7658856a81c5e9ba091c331c3de10 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Sat, 24 Oct 2026 13:33:12 -0400 Subject: [PATCH 3/4] test(harness): skip exit codes only for killed processes - kaibo review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: kaibo (deepseek) review of #50. The server timer sets TimedOut even when otel-cli exits on its own (main_test.go:446-456), so the TimedOut guard could skip a real exit code. CommandFailed alone means "no exit code". Approach: guard on !CommandFailed only. Removed the "protocol: bad config" ExecExitCode: 1 expectation: Diagnostics.ToStringMap omits exec_exit_code and only exec writes it, so it checked nothing. CHANGELOG notes the fix is Unix-only (WaitStatus.Signaled is always false on Windows). Learned: exec.go closes the signals channel while signal.Notify is still registered; a second SIGINT during export can panic. Pre-existing, not fixed here. Next: Amy's call on the signal.Stop fix. Verified: go vet, go test (3 consecutive runs green). 🔍 Reviewed-by: DeepSeek (kaibo, deepseek cast) 🤖 Claude Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- data_for_test.go | 1 - main_test.go | 5 +++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc442bc..605c139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ Begin agent-assisted development era. ### Fixed -- `exec` exits 128 + signal when the child is killed by a signal, not 255 +- `exec` exits 128 + signal when the child is killed by a signal, not 255 (Unix) ## [0.5.0] - 2025-04-21 diff --git a/data_for_test.go b/data_for_test.go index df46c29..74b728b 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -1157,7 +1157,6 @@ var suites = []FixtureSuite{ ParsedTimeoutMs: 1000, Endpoint: "*", EndpointSource: "*", - ExecExitCode: 1, }, SpanCount: 0, }, diff --git a/main_test.go b/main_test.go index ac0a1c0..84e3f3e 100644 --- a/main_test.go +++ b/main_test.go @@ -207,8 +207,9 @@ func checkProcess(t *testing.T, fixture Fixture, results Results) bool { t.Errorf("[%s] command failed is %t but expected %t", fixture.Name, results.CommandFailed, fixture.Expect.CommandFailed) return false } - // a killed process has no exit code of its own, so only compare clean exits - if !results.TimedOut && !results.CommandFailed && results.ExitCode != fixture.Expect.ExitCode { + // a killed process has no exit code of its own; TimedOut alone is not enough + // to skip, because the server timer sets it even when otel-cli exits normally + if !results.CommandFailed && results.ExitCode != fixture.Expect.ExitCode { t.Errorf("[%s] exit code is %d but expected %d", fixture.Name, results.ExitCode, fixture.Expect.ExitCode) return false } From c5831d4f66a2339c1b6ad46fc6dfa1ba3b3c5dde Mon Sep 17 00:00:00 2001 From: A Tobey Date: Sat, 24 Oct 2026 13:36:43 -0400 Subject: [PATCH 4/4] fix(exec): stop signal delivery before close - ctrl-c during export panicked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: the kaibo review of #50 found that exec.go closes the signals channel while signal.Notify is still registered. Amy: "it can go in pr 50". Approach: test first. New fixture "exec ctrl-c during export does not panic": /bin/true exits, the export to a stopped server retries for up to --timeout 1s, and the harness sends SIGINT at 200ms. Before the fix it exits 2 with "panic: send on closed channel" (reproduced by hand too). Fix: signal.Stop(signals) before close(signals). SIGINT during export now has its default action, so otel-cli stops and the span is not sent. Learned: a hand repro from a bash background job exits 0, not 130. Background jobs start with SIGINT ignored, and after signal.Stop Go returns to the inherited ignore. Next: review. Verified: go vet, go test ./... green 3 runs in a row; fixture red before the fix. 🤖 Claude Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + data_for_test.go | 19 +++++++++++++++++++ otelcli/exec.go | 2 ++ 3 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 605c139..614a300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Begin agent-assisted development era. ### Fixed - `exec` exits 128 + signal when the child is killed by a signal, not 255 (Unix) +- `exec` no longer panics on Ctrl-C during the span export ## [0.5.0] - 2025-04-21 diff --git a/data_for_test.go b/data_for_test.go index 74b728b..5027311 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -587,6 +587,25 @@ var suites = []FixtureSuite{ ExitCode: 42, }, }, + { + Name: "exec ctrl-c during export does not panic", + Config: FixtureConfig{ + CliArgs: []string{"exec", + "--endpoint", "{{endpoint}}", + "--timeout", "1s", + "--", "/bin/true", + }, + StopServerBeforeExec: true, + KillAfter: time.Millisecond * 200, // child is done, export is retrying + KillSignal: syscall.SIGINT, + TestTimeoutMs: 3000, + IsLongTest: true, + }, + Expect: Results{ + Config: otelcli.DefaultConfig(), + CommandFailed: true, // ended by the signal, not a panic (exit 2) + }, + }, }, // otel-cli span with no OTLP config should do and print nothing { diff --git a/otelcli/exec.go b/otelcli/exec.go index c15b9a6..47edbef 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -167,6 +167,8 @@ func doExec(cmd *cobra.Command, args []string) { } cancelCtxDeadline() + // stop delivery before close, or a ctrl-c during export panics on a closed channel + signal.Stop(signals) close(signals) <-signalsDone