Skip to content

fix(tests): drop gexec Compiles spec — segfaults on GH Actions#12

Merged
bborbe merged 2 commits into
masterfrom
fix/test-segfault-gexec
May 31, 2026
Merged

fix(tests): drop gexec Compiles spec — segfaults on GH Actions#12
bborbe merged 2 commits into
masterfrom
fix/test-segfault-gexec

Conversation

@bborbe
Copy link
Copy Markdown
Owner

@bborbe bborbe commented May 31, 2026

Summary

  • The Compiles ginkgo spec spawned a child go build via gexec.Build from the race-instrumented test binary. On the Linux GH Actions runner this segfaults randomly (works locally; only repros on CI under -race).
  • The spec was redundant: each test binary is itself package main, so if main.go does not compile, go test fails immediately.
  • Removes the spec from all 6 affected main_test.go files (agent/claude, agent/code, agent/gemini, agent/pi + their cmd/run-task subdirs).

Test plan

  • CI green on precommit (agent/code|pi|gemini|claude) matrix jobs

The 'Compiles' ginkgo spec called gexec.Build to spawn a child go
build from the race-instrumented test binary, which segfaults
randomly on the Linux GH Actions runner (works locally). The spec
was redundant: each test binary IS package main, so if main.go does
not compile, 'go test' fails immediately.

Removes the spec from all 6 affected main_test.go files.
Copy link
Copy Markdown

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary

Verdict: approve

This is a straightforward, well-reasoned change. The PR removes the gexec.Build Compiles specs from 6 test files across 4 agent services (claude, code, gemini, pi). The rationale is sound:

  1. Redundancy: go test itself fails if main.go doesn't compile — the explicit assertion is unnecessary
  2. CI flakiness: Spawning a child process from a race-instrumented binary segfaults on GH Actions Linux runners (works locally)
  3. Well-documented: Each removal has a detailed NOTE explaining the rationale and referencing the vault document for full diagnosis

No issues found.

Must Fix: None

Should Fix: None

Nice to Have: None


{
  "verdict": "approve",
  "summary": "Clean removal of redundant gexec.Build Compiles specs across 6 test files. The specs were unnecessary since go test already fails if main.go doesn't compile, and their removal eliminates a known GH Actions segfault issue with race-instrumented child processes. Well-documented with clear rationale referencing the vault note.",
  "comments": [],
  "concerns_addressed": []
}

Same root cause as the agent/* fix in this PR: the Compiles spec
segfaults flakily on the GH Actions runner. task/controller hit the
segfault on this PR's first CI run.
Copy link
Copy Markdown

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by ben-s-pull-request-reviewer[bot] — no concerns flagged.

@bborbe bborbe merged commit 44ef1ea into master May 31, 2026
9 checks passed
@bborbe bborbe deleted the fix/test-segfault-gexec branch May 31, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant