test(ci): cover portable release toolchain wrappers in quality gate - #86
Open
navarrocorbi-prog wants to merge 1 commit into
Open
test(ci): cover portable release toolchain wrappers in quality gate#86navarrocorbi-prog wants to merge 1 commit into
navarrocorbi-prog wants to merge 1 commit into
Conversation
This was referenced Jul 30, 2026
Add maintenance coverage for the Linux aarch64-musl Zig wrappers and wire the module into both just maintenance and the CI unittest list. Scope extraction to the Install build tools on Linux step so decoy wrappers elsewhere cannot poison assertions. refs #81 Co-authored-by: navarrocorbi-prog <navarrocorbi-prog@users.noreply.github.com>
WaySoul (SoulWayy)
force-pushed
the
cursor/release-assets-workflow-tests-6b9d
branch
from
August 1, 2026 21:55
86844b1 to
037dba0
Compare
OnlineChef
marked this pull request as ready for review
August 1, 2026 21:55
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
github-actions
Bot
added
the
quality-remediation
Contributor
Quality remediation neededCI run PR: #86 Failed jobs
Top extracted errors
Agent commands
Files touched
|
📝 WalkthroughWalkthroughChangesPortable release workflow tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/test_release_portable_assets_workflow.py (1)
96-130: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding timeouts and reducing duplicated subprocess setup.
None of the
subprocess.runcalls in_run_wrapper(Line 119-124) or the two inline failure tests (Line 226-231, Line 245-250) set atimeout. If a future change to the workflow's wrapper script introduces a hang (for example, a script that waits on stdin), the test would block until the CI job's 15-minute timeout instead of failing quickly with a clear error.The two failure tests (Line 213-232, Line 234-251) also duplicate most of
_run_wrapper's setup (temp dir, wrapper write, fakezig,subprocess.runcall) without reusing it, since_run_wrapperassumes a log file always exists after a successful run.♻️ Proposed direction: add a bounded timeout
result = subprocess.run( [str(wrapper_path), *args], env=env, capture_output=True, text=True, + timeout=10, )Also applies to: 213-232, 234-251
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test_release_portable_assets_workflow.py` around lines 96 - 130, Update _run_wrapper and both inline failure tests to pass a bounded timeout to subprocess.run, so hung wrapper scripts fail promptly. Reduce duplicated setup in the failure tests by reusing _run_wrapper where its recorded-argv assumption permits, or extract shared temporary-directory, fake-zig, and subprocess setup while preserving each test’s failure assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test_release_portable_assets_workflow.py`:
- Around line 234-251: Update test_cc_wrapper_fails_closed_when_zig_is_missing
to set PATH to existing directories such as /usr/bin:/bin that do not contain
zig, keeping bash resolvable so the wrapper body reaches the missing-zig failure
path.
---
Nitpick comments:
In `@scripts/test_release_portable_assets_workflow.py`:
- Around line 96-130: Update _run_wrapper and both inline failure tests to pass
a bounded timeout to subprocess.run, so hung wrapper scripts fail promptly.
Reduce duplicated setup in the failure tests by reusing _run_wrapper where its
recorded-argv assumption permits, or extract shared temporary-directory,
fake-zig, and subprocess setup while preserving each test’s failure assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af50c713-8d73-49cb-a5a1-d5560594a070
📒 Files selected for processing (3)
.github/workflows/ci.ymljustfilescripts/test_release_portable_assets_workflow.py
Comment on lines
+234
to
+251
| def test_cc_wrapper_fails_closed_when_zig_is_missing(self) -> None: | ||
| # `set -euo pipefail` plus the unresolved `exec zig` must produce a | ||
| # non-zero exit rather than silently doing nothing. | ||
| with tempfile.TemporaryDirectory() as tmp: | ||
| tmp_path = Path(tmp) | ||
|
|
||
| wrapper_path = tmp_path / "cc" | ||
| self._write_executable(wrapper_path, self.wrappers["cc"]) | ||
|
|
||
| env = dict(os.environ) | ||
| env["PATH"] = "/nonexistent" | ||
| result = subprocess.run( | ||
| [str(wrapper_path), "-c", "foo.c"], | ||
| env=env, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| self.assertNotEqual(result.returncode, 0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)scripts/test_release_portable_assets_workflow\.py$|(^|/)\.github/workflows/release-portable-assets\.yml$|(^|/)CONTRIBUTING\.md$' || true
echo
echo "== target test section =="
if [ -f scripts/test_release_portable_assets_workflow.py ]; then
sed -n '1,120p' scripts/test_release_portable_assets_workflow.py
echo "---"
sed -n '200,260p' scripts/test_release_portable_assets_workflow.py
fi
echo
echo "== find wrapper contents =="
rg -n 'wrappers|cc\"|zig|exec zig|set -euo pipefail' scripts/test_release_portable_assets_workflow.py
echo
echo "== workflow mentions =="
if [ -f .github/workflows/release-portable-assets.yml ]; then
rg -n 'Install build tools on Linux|zig|CC|cc' .github/workflows/release-portable-assets.yml
fi
echo
echo "== deterministic subprocess/env probe =="
python3 - <<'PY'
import os
import subprocess
import tempfile
from pathlib import Path
script = b'#!/usr/bin/env bash\nset -euo pipefail\nexec zig "$@"\n'
with tempfile.TemporaryDirectory() as tmp:
path = Path(tmp) / "cc"
path.write_bytes(script)
os.chmod(path, 0o755)
for val in ["/nonexistent", "/usr/bin", "/"]:
env = dict(os.environ)
env["PATH"] = val
r = subprocess.run([str(path), "-c", "foo.c"], env=env, capture_output=True, text=True)
print(f"PATH={val!r}")
print(f" returncode={r.returncode}")
print(f" stdout={r.stdout!r}")
print(f" stderr={r.stderr.strip()!r[:200]}")
PYRepository: OnlineChefGroep/herdr
Length of output: 10719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== wrapper cc body =="
sed -n '125,200p' .github/workflows/release-portable-assets.yml
echo
echo "== deterministic subprocess/env probe =="
python3 - <<'PY'
import os
import subprocess
import stat
import tempfile
from pathlib import Path
script = "#!/usr/bin/env bash\nset -euo pipefail\nexec zig \"$@\"\n"
with tempfile.TemporaryDirectory() as tmp:
path = Path(tmp) / "cc"
path.write_text(script, encoding="utf-8")
st = path.stat()
path.chmod(st.st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
for val in ["/nonexistent", "/usr/bin", "/"]:
env = dict(os.environ)
env["PATH"] = val
r = subprocess.run([str(path), "-c", "foo.c"], env=env, capture_output=True, text=True)
print(f"PATH={val!r}")
print(f" returncode={r.returncode}")
print(f" stdout={r.stdout!r}")
stderr = next(iter(r.stderr.splitlines(), ""))
print(f" stderr_first_line={stderr!r}")
PYRepository: OnlineChefGroep/herdr
Length of output: 3629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== wrapper output probe =="
python3 - <<'PY'
import os
import subprocess
import stat
import tempfile
from pathlib import Path
script = "#!/usr/bin/env bash\nset -euo pipefail\nexec zig \"$@\"\n"
with tempfile.TemporaryDirectory() as tmp:
path = Path(tmp) / "cc"
path.write_text(script, encoding="utf-8")
st = path.stat()
path.chmod(st.st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
for val in ["/nonexistent", "/usr/bin", "/"]:
env = dict(os.environ)
env["PATH"] = val
r = subprocess.run([str(path), "-c", "foo.c"], env=env, capture_output=True, text=True)
lines = r.stderr.splitlines()
print(f"PATH={val!r}")
print(f" returncode={r.returncode}")
print(f" stdout={r.stdout!r}")
print(f" stderr_line_count={len(lines)}")
print(f" stderr_first={lines[0]!r if lines else ''}")
print(f" stderr_second={lines[1]!r if len(lines) > 1 else ''}")
PYRepository: OnlineChefGroep/herdr
Length of output: 367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os
import subprocess
from pathlib import Path
script = "#!/usr/bin/env bash\nset -euo pipefail\nexec zig \"$@\"\n"
with tempfile.TemporaryDirectory() as tmp:
toolchain = Path(tmp)
wrapper = toolchain / "cc"
wrapper.write_text(script)
wrapper.chmod(0o755)
for path in ["/nonexistent", "/usr/bin", "/"]:
env = dict(os.environ)
env["PATH"] = path
r = subprocess.run([str(wrapper), "-c", "foo.c"], env=env, capture_output=True, text=True)
lines = r.stderr.splitlines()
print(f"PATH={path!r}")
print(f"returncode={r.returncode}")
print("stderr:")
for line in lines[:3]:
print(" ", line)
PYRepository: OnlineChefGroep/herdr
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import os
import subprocess
import tempfile
from pathlib import Path
script = "#!/usr/bin/env bash\nset -euo pipefail\nexec zig \"$@\"\n"
with tempfile.TemporaryDirectory() as tmp:
toolchain = Path(tmp)
wrapper = toolchain / "cc"
wrapper.write_text(script)
wrapper.chmod(0o755)
for path in ["/nonexistent", "/usr/bin", "/"]:
env = dict(os.environ)
env["PATH"] = path
r = subprocess.run([str(wrapper), "-c", "foo.c"], env=env, capture_output=True, text=True)
lines = r.stderr.splitlines()
print("PATH=", repr(path))
print("returncode=", r.returncode)
print("stdout=", repr(r.stdout))
print("stderr:")
for line in lines[:3]:
print(" ", line)
PYRepository: OnlineChefGroep/herdr
Length of output: 464
Keep bash resolvable in the missing-zig test.
The wrapper uses #!/usr/bin/env bash, while this test sets PATH to /nonexistent. That makes /usr/bin/env fail with No such file or directory, so the wrapper body never reaches exec zig. Set PATH to existing directories that do not contain zig, such as /usr/bin:/bin, so the test exercises the wrapper’s failure path instead of interpreter resolution.
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 244-249: Command coming from incoming request
Context: subprocess.run(
[str(wrapper_path), "-c", "foo.c"],
env=env,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.0)
[error] 245-245: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/test_release_portable_assets_workflow.py` around lines 234 - 251,
Update test_cc_wrapper_fails_closed_when_zig_is_missing to set PATH to existing
directories such as /usr/bin:/bin that do not contain zig, keeping bash
resolvable so the wrapper body reaches the missing-zig failure path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses Greptile findings on #81 and lands the useful CodeRabbit-generated coverage properly:
scripts/test_release_portable_assets_workflow.pyfor the Linux aarch64-musl Zig cc/cxx wrappers + RUSTFLAGSjust maintenanceand the explicit CI unittest list in.github/workflows/ci.yml(so Quality gate actually runs it)Install build tools on Linuxstep so same-named wrappers elsewhere cannot poison assertions#81 is closed as superseded. CI: Quality gate + Maintenance green (16 tests covered via Maintenance).
Test plan
python3 -m unittest scripts.test_release_portable_assets_workflow -v(16 OK)Greptile Summary
Adds regression coverage for the portable release workflow’s aarch64-musl compiler wrappers and Rust flags, and registers the checks in both maintenance CI and
just maintenance.The new tests execute wrapper scripts extracted from the real workflow. All 16 tests passed. In a disposable copy, changing the
ccandcxxwrappers to forward the duplicate Rust target flag caused three wrapper assertions to fail, confirming that the coverage detects the intended target-filtering regression.Confidence Score: 5/5
What T-Rex did
Reviews (1): Last reviewed commit: "test(ci): cover portable release toolcha..." | Re-trigger Greptile