Repository navigation
feat(OMN-14761): fail-closed shellcheck gate + shared merge-sweep bash lib (F-22) - #2340
Merged
Merged
Conversation
…h lib (F-22) F-22: hand-typed zsh-fragile probe loops (newline-splitting, quoted `repo pr` loops producing invalid gh names) caused merge-sweep probe/rerun noise. This adds the canonical bash surface + a fail-closed shellcheck gate wired as BOTH a CI job and a pre-commit hook (rule #5), in the same PR. - scripts/lib/merge_sweep_common.sh: canonical REPOS[] array, for_each_repo iterator (fail-fast by default; MERGE_SWEEP_CONTINUE_ON_ERROR=1 to visit all), opt-in msc_strict (set -euo pipefail), and a heredoc'd msc_mergequeue_probe — the safe replacement for hand-written probe loops. pr-snapshot.sh now sources it, removing the duplicated inline REPOS list. (.gitignore carves scripts/lib/ out of the generic Python lib/ ignore.) - scripts/ci/check_shell_hygiene.sh: fail-closed shellcheck gate. scripts/lib/** held to --severity=style (strict, new canonical code); the rest of the tree to --severity=error (clean today across all 91 tracked shell scripts -> ZERO baseline). Fails closed when shellcheck is absent. - .github/workflows/shellcheck-gate.yml + .pre-commit-config shell-hygiene hook: both invoke the SAME gate script so local and CI enforcement cannot drift. The workflow runs on every PR (no path filter) so it always reports and can be promoted to a required status check without the path-filtered-wedge trap. Tests (17, green via uv run; git driven in disposable tmp repos, GIT_* stripped): gate rejects a real error-severity defect and a scripts/lib/** style defect, passes a style-only defect on non-lib scripts, scans extensionless shell scripts, fails closed with no shellcheck; lib exposes REPOS/for_each_repo/msc_strict, fails fast vs continue-on-error, refuses direct execution, is style-clean. Not flipping branch protection: promoting shellcheck-gate to a required context is left as an additive-then-subtractive follow-up (needs prove-fires-on-all-PR- classes + rollback), per repo policy.
📝 WalkthroughWalkthroughAdds fail-closed shellcheck enforcement to pre-commit and CI, with architecture-specific installation and severity tiers. Introduces shared merge-sweep shell helpers, updates PR snapshotting to use them, and adds unit coverage for both features. ChangesShell hygiene enforcement
Merge-sweep shell helpers
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ShellHygieneGate
participant Git
participant Shellcheck
GitHubActions->>ShellHygieneGate: run bash scripts/ci/check_shell_hygiene.sh
ShellHygieneGate->>Git: enumerate tracked shell files
ShellHygieneGate->>Shellcheck: scan files at configured severity
Shellcheck-->>ShellHygieneGate: findings or success
ShellHygieneGate-->>GitHubActions: exit code and summary
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
added 2 commits
July 18, 2026 01:45
…no xz binary) CI run 29637794554 failed: the self-hosted omnibase-ci runner has no `xz` binary, so `tar -xJf` could not extract the shellcheck .tar.xz release and the gate failed closed at install (not a shellcheck finding). Extract with python3 stdlib lzma (tarfile 'r:xz') instead, and detect arch (x86_64/aarch64).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/shellcheck-gate.yml:
- Around line 13-14: Update the pull_request trigger in the shellcheck gate
workflow to remove the branches filter, ensuring the gate runs for pull requests
targeting every branch.
- Around line 60-65: Add committed SHA-256 or signature verification for the
downloaded ShellCheck archive before extraction, using the existing
download/version flow and failing on validation errors. Apply the same
integrity-check behavior at .github/workflows/shellcheck-gate.yml lines 60-65
and .github/workflows/ci.yml lines 1147-1150; both sites require direct changes
before their respective extraction steps.
- Line 42: Update the actions/checkout step in the workflow to reference an
immutable commit SHA instead of the mutable v6 tag, matching the commit-pinned
format and version used by the checkout step in ci.yml.
- Around line 49-52: Ensure both shellcheck setup blocks use the pinned
SHELLCHECK_VERSION rather than accepting any preinstalled binary: update
.github/workflows/shellcheck-gate.yml lines 49-52 and .github/workflows/ci.yml
lines 1136-1139 to validate the installed shellcheck version against the pin or
install the pinned tarball unconditionally, removing the unconditional command
-v bypass.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b5908ef-41f8-4ba9-b1ac-aa374aadd346
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/shellcheck-gate.yml.gitignore.pre-commit-config.yamlscripts/ci/check_shell_hygiene.shscripts/lib/merge_sweep_common.shscripts/pr-snapshot.shtests/unit/scripts/test_merge_sweep_common.pytests/unit/scripts/test_shell_hygiene_gate.py
Comment on lines
+13
to
+14
| pull_request: | ||
| branches: [main, dev, develop] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run this gate on every PR as stated.
pull_request.branches restricts the workflow to PRs targeting main, dev, or develop; PRs targeting release or maintenance branches skip the gate. Remove this filter, or narrow the stated objective.
🤖 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 @.github/workflows/shellcheck-gate.yml around lines 13 - 14, Update the
pull_request trigger in the shellcheck gate workflow to remove the branches
filter, ensuring the gate runs for pull requests targeting every branch.
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## files\n'
git ls-files .github/workflows
printf '\n## checkout usages\n'
rg -n "actions/checkout@" .github/workflows -S || true
printf '\n## shellcheck-gate context\n'
sed -n '1,120p' .github/workflows/shellcheck-gate.yml
printf '\n## ci context\n'
sed -n '1,120p' .github/workflows/ci.ymlRepository: OmniNode-ai/omnibase_infra
Length of output: 26479
🏁 Script executed:
set -euo pipefail
git ls-files .github/workflows
rg -n "actions/checkout@" .github/workflows -S || true
printf '\n--- shellcheck-gate ---\n'
sed -n '1,120p' .github/workflows/shellcheck-gate.yml
printf '\n--- ci ---\n'
sed -n '1,120p' .github/workflows/ci.ymlRepository: OmniNode-ai/omnibase_infra
Length of output: 26443
Pin actions/checkout to a commit SHA here.
actions/checkout@v6 is mutable; match the commit-pinned form already used in .github/workflows/ci.yml.
🤖 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 @.github/workflows/shellcheck-gate.yml at line 42, Update the
actions/checkout step in the workflow to reference an immutable commit SHA
instead of the mutable v6 tag, matching the commit-pinned format and version
used by the checkout step in ci.yml.
Comment on lines
+49
to
+52
| if command -v shellcheck >/dev/null 2>&1; then | ||
| shellcheck --version | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## shellcheck-gate.yml\n'
sed -n '1,130p' .github/workflows/shellcheck-gate.yml
printf '\n## ci.yml (excerpt around shellcheck)\n'
sed -n '1100,1185p' .github/workflows/ci.yml
printf '\n## Search for SHELLCHECK_VERSION references\n'
rg -n "SHELLCHECK_VERSION|shellcheck --version|command -v shellcheck|shellcheck" .github/workflows . -g '!**/node_modules/**'Repository: OmniNode-ai/omnibase_infra
Length of output: 16304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## scripts/ci/check_shell_hygiene.sh\n'
sed -n '1,180p' scripts/ci/check_shell_hygiene.sh
printf '\n## Search for any ShellCheck version gating or pinning outside workflows\n'
rg -n "SHELLCHECK_VERSION|shellcheck --version|command -v shellcheck|require.*shellcheck|0\.10\.0" scripts .github tests -g '!**/node_modules/**'Repository: OmniNode-ai/omnibase_infra
Length of output: 5586
Pin ShellCheck instead of accepting any preinstalled binary.
Both jobs still short-circuit on command -v shellcheck, so a runner-provided binary can bypass SHELLCHECK_VERSION and make results vary by environment. Validate shellcheck --version against the pinned release or install the pinned tarball unconditionally in both .github/workflows/shellcheck-gate.yml#L49-L52 and .github/workflows/ci.yml#L1136-L1139.
📍 Affects 2 files
.github/workflows/shellcheck-gate.yml#L49-L52(this comment).github/workflows/ci.yml#L1136-L1139
🤖 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 @.github/workflows/shellcheck-gate.yml around lines 49 - 52, Ensure both
shellcheck setup blocks use the pinned SHELLCHECK_VERSION rather than accepting
any preinstalled binary: update .github/workflows/shellcheck-gate.yml lines
49-52 and .github/workflows/ci.yml lines 1136-1139 to validate the installed
shellcheck version against the pin or install the pinned tarball
unconditionally, removing the unconditional command -v bypass.
Comment on lines
+60
to
+65
| url="https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION}/shellcheck-${SHELLCHECK_VERSION}.linux.${sc_arch}.tar.xz" | ||
| curl -fsSL "${url}" -o /tmp/shellcheck.tar.xz | ||
| # Extract without needing the `xz` binary (self-hosted runner lacks it); | ||
| # python3 stdlib lzma handles .tar.xz. | ||
| python3 -c "import tarfile; tarfile.open('/tmp/shellcheck.tar.xz','r:xz').extractall('/tmp')" | ||
| cp "/tmp/shellcheck-${SHELLCHECK_VERSION}/shellcheck" "${HOME}/.local/bin/shellcheck" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add integrity verification for ShellCheck downloads
Both workflows fetch the ShellCheck tarball from GitHub Releases and extract it directly into ~/.local/bin without a checksum or signature check. Add a committed SHA-256 or signature validation before extraction in:
.github/workflows/shellcheck-gate.yml#L60-L65.github/workflows/ci.yml#L1147-L1150
📍 Affects 2 files
.github/workflows/shellcheck-gate.yml#L60-L65(this comment).github/workflows/ci.yml#L1147-L1150
🤖 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 @.github/workflows/shellcheck-gate.yml around lines 60 - 65, Add committed
SHA-256 or signature verification for the downloaded ShellCheck archive before
extraction, using the existing download/version flow and failing on validation
errors. Apply the same integrity-check behavior at
.github/workflows/shellcheck-gate.yml lines 60-65 and .github/workflows/ci.yml
lines 1147-1150; both sites require direct changes before their respective
extraction steps.
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
1 participant
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.
OMN-14761 (F-22): shellcheck gate + shared merge-sweep bash lib — omnibase_infra half
Closes OMN-14761 (partial — omnibase_infra half; omnimarket half is a sibling PR under the same ticket).
Friction
Hand-typed zsh-fragile probe loops (newline-splitting, quoted
repo prloops that produced invalidghrepository names) caused merge-sweep probe/rerun noise. Source:docs/tracking/friction/2026-07-16-overnight-merge-sweep-friction-report.md(F-22).Change (rule #5: detector ships wired as CI gate + pre-commit hook, same PR, fail-closed)
scripts/lib/merge_sweep_common.sh— the canonical bash surface:REPOS[],for_each_repo(fail-fast by default;MERGE_SWEEP_CONTINUE_ON_ERROR=1to visit all), opt-inmsc_strict(set -euo pipefail), and a heredoc'dmsc_mergequeue_probe(the safe replacement for hand-written probe loops).pr-snapshot.shnow sources it, removing the duplicated inlineREPOSlist. (.gitignorecarvesscripts/lib/out of the generic Pythonlib/ignore.)scripts/ci/check_shell_hygiene.sh— fail-closed shellcheck gate.scripts/lib/**held to--severity=style(strict, new canonical code); the rest of the tree to--severity=error(clean today across all 91 tracked shell scripts → ZERO baseline). Fails closed when shellcheck is absent..github/workflows/shellcheck-gate.yml+.pre-commit-configshell-hygienehook — both invoke the same gate script so local and CI enforcement cannot drift. The workflow runs on every PR (no path filter) so it always reports and can be promoted to a required status check without the path-filtered-wedge trap.dod_evidence
tests/unit/scripts/test_shell_hygiene_gate.py+test_merge_sweep_common.py(17 tests, green viauv run), git driven in disposable tmp repos withGIT_*stripped (OMN-14746/14744):$() and ascripts/lib/**style defect (backticks/SC2006), passes a style-only defect on non-lib scripts (proves the two tiers differ), scans extensionless shell scripts, fails closed with no shellcheck on PATH — all assert-vs-broken, never green-on-absence;REPOS/for_each_repo/msc_strict, fails fast vs continue-on-error, refuses direct execution, is shellcheck-style-clean, no hardcoded paths.actionlintclean on the workflow; theshell-hygienepre-commit hook dogfooded on this PR's own files (passed); whole-tree gate run = "91 shell script(s) clean".run_delegation_cost_projection_process.shlacks strict mode was inaccurate — it already hasset -Eeuo pipefail; no single-script fix was owed.Deferred (honest)
shellcheck-gateto a required status check — left as an additive-then-subtractive follow-up (needs prove-fires-on-all-PR-classes + rollback captured), per repo policy; not flipping branch protection from a subagent.merge_sweep_common.sh— deferred (per triage, lowest value since the live merge path is the Python nodenode_pr_lifecycle_orchestrator, not shell).pr-snapshot.shis the one demonstrated consumer.DO NOT MERGE — Codex owns the merge.
Evidence-Ticket: OMN-14761
Evidence-Source: OCC#4382
Evidence-Commit: acea032f9a519cebfca316776867cc02e259c202
Evidence-Head: f9905772c0c4d55782ad549dcfd250ae2589a833
Summary by CodeRabbit
New Features
Bug Fixes
Tests