ci(sanitizers): pin Run tests step to bash so pipefail + PIPESTATUS work#3155
Conversation
There was a problem hiding this comment.
Diagnosis matches what's on main: line 75 uses set -o pipefail and line 83 uses ${PIPESTATUS[0]}, both of which Dash rejects (Ubuntu-based container images symlink /bin/sh → dash). continue-on-error: true plus the downstream steps.ctest.outputs.exit != '0' gates would silently mask the early exit exactly as described — that's a nasty failure mode for a workflow whose entire purpose is finding latent bugs.
shell: bash is the right one-line fix:
- Bash is present in the container (it's a standard Ubuntu/Debian base).
- Scoped to just the step that uses bashisms, so it doesn't perturb the other steps that work fine under Dash.
- No protocol/behavior change for the sanitizer logic itself — just makes it actually run.
The inline comment block is a nice touch — anyone touching this step in the future will see why the pin matters.
Thanks for the careful root-cause writeup and the workflow_dispatch verification. LGTM pending the verification run reaching ctest.
🤖 aethersdr-agent · cost: $4.3549 · model: claude-opus-4-7
There was a problem hiding this comment.
Thanks @ten9876 — this is a clean, well-targeted fix and the root-cause writeup is excellent.
Verified against the workflow:
- Lines 75 (
set -o pipefail) and 83 (${PIPESTATUS[0]}) are the only bashisms in the file, and both live in the "Run tests under sanitizer" step. Pinning only this step to bash is the right minimum. - The other steps (Configure, Build, Extract failure report) only use POSIX features and can stay on the default shell, so the patch correctly resists the temptation to set
shell: bashat the job level. continue-on-error: true+ downstream gating onsteps.ctest.outputs.exit != '0'plausibly explains the false-green: when the step dies at line 75 before reaching theecho "exit=...", the output is never written and the gated steps get skipped, so the run finishes apparently clean with zero sanitizer coverage.
Nothing else in scope to flag. The inline comment is a good defensive note — future readers won't be tempted to "simplify" the explicit shell: bash away.
LGTM pending the workflow_dispatch verification run reaching ctest.
🤖 aethersdr-agent · cost: $3.8451 · model: claude-opus-4-7
The Sanitizers workflow runs inside ghcr.io/ten9876/aethersdr-ci:latest,
where /bin/sh is Dash. Dash doesn't understand `set -o pipefail` or
${PIPESTATUS[0]}. The "Run tests under sanitizer" step exited at code
2 on its very first line — before ctest ran — and `continue-on-error:
true` silenced the failure at the job level. Every downstream gate
on `steps.ctest.outputs.exit != '0'` then matched against an unset
output, so the workflow walked through to completion without
executing a single test under ASan/UBSan or TSan. Net result: every
scheduled run since the workflow landed reported green but had no
sanitizer coverage.
Add `shell: bash` to the step so the bashisms run as intended. Bash
is present in the container image; only this single step uses
non-POSIX features, so the other steps are left on the default
shell.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gates fire
The bash-shell fix in this PR's first commit got ctest actually running
under the sanitizers — and the verification dispatch run immediately
surfaced a real SEGV in theme_manager_test under both ASan and TSan.
But the sticky-issue / artifact-upload / fail-job steps all skipped
because outputs.exit was never set.
Root cause: GitHub Actions' default `shell: bash` runs with `-eo
pipefail`. When ctest fails, the pipeline `ctest | tee` exits non-zero
via pipefail, and `-e` immediately terminates the script — before the
`echo "exit=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT"` line runs. Every
downstream `if: steps.ctest.outputs.exit != '0'` then evaluates against
null and (apparently) skips, so the workflow walks through to
completion looking green.
Capture into `ec=0; ctest ... || ec=$?` — the OR branch absorbs the
non-zero exit so `-e` doesn't trigger, ec gets ctest's real exit code,
and the echo always runs. Downstream gates now fire on actual failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f52db0d to
0877c6c
Compare
|
Thanks for digging into this @ten9876 — and the diagnosis in the PR comment is spot on. A note on the failing CI here: What failed
That sequence is the smoking gun: ctest ran, a sanitizer report was extracted, the tracking issue was opened/updated, and then the gate fired. In other words, the sanitizers are now correctly reporting real findings — which is precisely what your patch was designed to make happen. Why this is expected, not brokenBefore your fix, the step was silently exiting at code 2 inside Dash before What to doThis PR shouldn't be blocked by the CI red; it's doing its job. The right follow-up is the tracking issue that the "Open or update tracking issue" step just filed (or updated) — that's where the underlying ASan/UBSan/TSan findings live, and they should be triaged on their own merits against If you want, in a follow-up commit on this branch you could also add a brief note to the workflow comment block (or a CHANGELOG entry) flagging that "this PR is expected to flip currently-green-but-broken sanitizer jobs to red on first run; that's the desired effect" — saves the next reviewer the same investigation. Really nice catch on the Dash-vs-bash interaction; that's a subtle one and the kind of CI bug that hides for a long time. 🎯 🤖 aethersdr-agent · cost: $5.8465 · model: claude-opus-4-7 |
…ork (aethersdr#3155) ## Summary The Sanitizers workflow ([first run #26397650182](https://github.com/aethersdr/AetherSDR/actions/runs/26397650182), 2026-05-25 07:00 UTC) reported green — but it didn't actually run anything under the sanitizers. The "Run tests under sanitizer" step exited at code 2 on its very first line, before `ctest` ran. \`continue-on-error: true\` silenced the failure at the job level, and every downstream gate on \`steps.ctest.outputs.exit != '0'\` then matched against an unset output, so the workflow walked through to completion. Every scheduled run since the workflow landed (PR aethersdr#2866, 2026-05-21) has produced a false-clean result with zero sanitizer coverage. ## Root cause The container image \`ghcr.io/ten9876/aethersdr-ci:latest\` defaults \`/bin/sh\` to **Dash**, which doesn't understand \`set -o pipefail\` or \`\${PIPESTATUS[0]}\`. Two bashisms, both at line 75/83 of [`.github/workflows/sanitizers.yml`](.github/workflows/sanitizers.yml). Run log signature: ``` /__w/_temp/...sh: 1: set: Illegal option -o pipefail ##[error]Process completed with exit code 2. ``` ## Fix One-line: add \`shell: bash\` to the "Run tests under sanitizer" step. Bash is present in the container; only this step uses non-POSIX features, so leaving the other steps on the default shell keeps the patch minimal. ## Verification Triggered \`workflow_dispatch\` on this branch: [run #26415106480](https://github.com/aethersdr/AetherSDR/actions/runs/26415106480). With the fix in place, the step should now reach \`ctest\` and either pass cleanly or surface real sanitizer findings (and file/update the sticky tracking issue). Result will appear there before this PR merges. ## Test plan - [ ] Workflow dispatch run on this branch executes \`ctest\` (verifiable in the step log) - [ ] If tests pass under sanitizers: no behavior change vs. \"green\" appearance, but now meaningful - [ ] If tests fail: sticky issue gets opened or appended, artifacts uploaded - [ ] CI green on the PR itself 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The Sanitizers workflow (first run #26397650182, 2026-05-25 07:00 UTC) reported green — but it didn't actually run anything under the sanitizers. The "Run tests under sanitizer" step exited at code 2 on its very first line, before
ctestran. `continue-on-error: true` silenced the failure at the job level, and every downstream gate on `steps.ctest.outputs.exit != '0'` then matched against an unset output, so the workflow walked through to completion. Every scheduled run since the workflow landed (PR #2866, 2026-05-21) has produced a false-clean result with zero sanitizer coverage.Root cause
The container image `ghcr.io/ten9876/aethersdr-ci:latest` defaults `/bin/sh` to Dash, which doesn't understand `set -o pipefail` or `${PIPESTATUS[0]}`. Two bashisms, both at line 75/83 of
.github/workflows/sanitizers.yml.Run log signature:
Fix
One-line: add `shell: bash` to the "Run tests under sanitizer" step. Bash is present in the container; only this step uses non-POSIX features, so leaving the other steps on the default shell keeps the patch minimal.
Verification
Triggered `workflow_dispatch` on this branch: run #26415106480. With the fix in place, the step should now reach `ctest` and either pass cleanly or surface real sanitizer findings (and file/update the sticky tracking issue). Result will appear there before this PR merges.
Test plan
🤖 Generated with Claude Code