Repository navigation
ci(OMN-13502): flip no-faked-boundary gate report-only -> enforcing - #1860
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughCI workflows and the pre-commit hook are updated to enforce boundary checks, adjust checkout and dependency setup, harden security scanning, and make the golden-chain-live Redpanda job manage its own container lifecycle. ChangesWorkflow enforcement and reliability
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
❓ Hostile Reviewer — UNKNOWNBlocking findings (critical/error): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/validate-pr-contracts.yml (1)
40-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials.
actions/checkoutwill leave the token in.git/configunlesspersist-credentials: falseis set. This workflow processes PR content, so keeping write-capable credentials in the workspace is unnecessary exposure.🔧 Suggested patch
- name: Checkout if: github.event_name != 'merge_group' uses: actions/checkout@v7 with: + persist-credentials: false fetch-depth: 1🤖 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/validate-pr-contracts.yml around lines 40 - 44, The Checkout step in the validate-pr-contracts workflow leaves credentials persisted in the workspace, which is unnecessary for processing PR content. Update the actions/checkout usage in the Checkout step to explicitly disable persisted credentials by setting persist-credentials to false, keeping the existing fetch-depth behavior unchanged.Source: Linters/SAST tools
🧹 Nitpick comments (3)
.github/workflows/hostile-reviewer.yml (2)
59-63: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnnecessary sleep before final failure.
On the 3rd (last) attempt, the loop still runs
rm -rfandsleep $((attempt * 10))(30s) before falling through toexit 1, wasting ~30s per failing clone with no retry benefit.⏱️ Skip sleep on last attempt
for attempt in 1 2 3; do git clone --depth=1 --branch main https://github.com/OmniNode-ai/omniintelligence.git ../omniintelligence && exit 0 rm -rf ../omniintelligence - sleep $((attempt * 10)) + [ "$attempt" -lt 3 ] && sleep $((attempt * 10)) done exit 1Also applies to: 69-73, 79-83
🤖 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/hostile-reviewer.yml around lines 59 - 63, Skip the backoff sleep on the final retry in the clone loop so the workflow fails immediately after the last unsuccessful attempt. Update the retry logic in the hostile reviewer workflow around the git clone loop to only run the cleanup and sleep when another attempt remains, using the existing attempt counter and loop structure that includes git clone, rm -rf, sleep, and exit 1.
55-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRetry-clone logic duplicated three times.
The retry loop (3 attempts, cleanup, backoff sleep, final failure) is copy-pasted for
omniintelligence,omnibase_core, andomnibase_compat, differing only in repo URL and directory name. Consider extracting this into a small reusable script (e.g.,scripts/ci/clone-with-retry.sh <repo-url> <dest-dir>) invoked from each step, or a composite action, to avoid triplicated maintenance.♻️ Example extraction
# scripts/ci/clone-with-retry.sh #!/bin/bash set -uo pipefail repo_url="$1"; dest="$2"; branch="${3:-main}" for attempt in 1 2 3; do git clone --depth=1 --branch "$branch" "$repo_url" "$dest" && exit 0 rm -rf "$dest" [ "$attempt" -lt 3 ] && sleep $((attempt * 10)) done exit 1- name: Clone omniintelligence working-directory: ${{ github.workspace }} run: bash scripts/ci/clone-with-retry.sh https://github.com/OmniNode-ai/omniintelligence.git ../omniintelligence🤖 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/hostile-reviewer.yml around lines 55 - 83, The clone retry logic in the workflow is duplicated across the omniintelligence, omnibase_core, and omnibase_compat steps. Extract the shared 3-attempt clone/cleanup/backoff behavior into a reusable helper such as a scripts/ci/clone-with-retry.sh script or a composite action, then update each clone step to call it with the repo URL and destination directory. Keep the existing branch, retry, rm -rf cleanup, and sleep behavior centralized in the helper so future changes only need to be made once..github/workflows/ci.yml (1)
1346-1354: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
aio-max-nrtuning can hard-fail the step underset -euo pipefail.The
sudobranch runsecho 1048576 | sudo tee /proc/sys/fs/aio-max-nrunguarded. Ifsudoexists but is not passwordless (or/proc/sys/fs/aio-max-nris not writable),teereturns non-zero andset -eaborts the whole "Start Redpanda" step — even though theelif/elsebranches explicitly treat inability to set this tunable as a non-fatal::notice::. This is an inconsistent failure mode for what is a best-effort optimization.♻️ Make the sudo path non-fatal to match the other branches
if command -v sudo >/dev/null 2>&1; then - echo 1048576 | sudo tee /proc/sys/fs/aio-max-nr >/dev/null + echo 1048576 | sudo tee /proc/sys/fs/aio-max-nr >/dev/null \ + || echo "::notice::cannot adjust aio-max-nr on this runner" elif [ -w /proc/sys/fs/aio-max-nr ]; then🤖 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/ci.yml around lines 1346 - 1354, The aio-max-nr tuning in the Start Redpanda step is still fatal in the sudo path, unlike the fallback branches. Update the shell block in the workflow so the `sudo`/`tee` attempt is treated as best-effort and does not abort under `set -euo pipefail`, matching the non-fatal `::notice::` behavior already used when `/proc/sys/fs/aio-max-nr` cannot be adjusted. Keep the change localized to the existing `sudo`, `tee`, and `::notice::` logic.
🤖 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/ci.yml:
- Around line 1355-1368: The `golden-chain-live` Redpanda setup in the workflow
uses a fixed container name and host port, which can collide across concurrent
self-hosted runs. Update the `docker run` step to use a unique container name
and an ephemeral mapped port, or add serialization via `concurrency:` so
simultaneous jobs do not stop each other’s `omniclaude-golden-chain-redpanda`
container or bind to port `9092`.
In @.github/workflows/security-scan.yml:
- Around line 265-282: The Checkov soft-fail matcher is only being written to a
file and never registered, so GitHub Actions won’t apply it. In the workflow
step that creates checkov-problem-matcher-softfail.json, add the matcher
registration using the same filename so the annotations are enabled, and ensure
it is later removed with the matching owner value checkov-softfail. Use the step
named Provide Checkov soft-fail problem matcher to locate the change.
---
Outside diff comments:
In @.github/workflows/validate-pr-contracts.yml:
- Around line 40-44: The Checkout step in the validate-pr-contracts workflow
leaves credentials persisted in the workspace, which is unnecessary for
processing PR content. Update the actions/checkout usage in the Checkout step to
explicitly disable persisted credentials by setting persist-credentials to
false, keeping the existing fetch-depth behavior unchanged.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1346-1354: The aio-max-nr tuning in the Start Redpanda step is
still fatal in the sudo path, unlike the fallback branches. Update the shell
block in the workflow so the `sudo`/`tee` attempt is treated as best-effort and
does not abort under `set -euo pipefail`, matching the non-fatal `::notice::`
behavior already used when `/proc/sys/fs/aio-max-nr` cannot be adjusted. Keep
the change localized to the existing `sudo`, `tee`, and `::notice::` logic.
In @.github/workflows/hostile-reviewer.yml:
- Around line 59-63: Skip the backoff sleep on the final retry in the clone loop
so the workflow fails immediately after the last unsuccessful attempt. Update
the retry logic in the hostile reviewer workflow around the git clone loop to
only run the cleanup and sleep when another attempt remains, using the existing
attempt counter and loop structure that includes git clone, rm -rf, sleep, and
exit 1.
- Around line 55-83: The clone retry logic in the workflow is duplicated across
the omniintelligence, omnibase_core, and omnibase_compat steps. Extract the
shared 3-attempt clone/cleanup/backoff behavior into a reusable helper such as a
scripts/ci/clone-with-retry.sh script or a composite action, then update each
clone step to call it with the repo URL and destination directory. Keep the
existing branch, retry, rm -rf cleanup, and sleep behavior centralized in the
helper so future changes only need to be made once.
🪄 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: b95a8749-2496-4629-b22f-b400ccb3ca04
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/hostile-reviewer.yml.github/workflows/no-faked-boundary.yml.github/workflows/onex-schema-compat.yml.github/workflows/security-scan.yml.github/workflows/validate-pr-contracts.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/no-faked-boundary.yml
a835539 to
0ac4528
Compare
The OMN-13500 (#1849) burn-down took omniclaude's detector self-scan from 61 findings to zero. With the population burned down, this drops --report-only from both the CI workflow and its pre-commit mirror so the gate fails the build on any new faked-boundary finding — matching omnibase_core's blocking gate. Job/hook renamed from "... (report-only)" to the bare enforcing name so the dev branch-protection required context can be added once this lands and reports once (OMN-13501/13502 Phase 3).
…ncellation The check ran uv sync --frozen --group dev before a stdlib-only AST validator (imports: ast, sys, pathlib), which exceeded even the 10-minute job timeout under self-hosted fleet saturation — cancelling a check that runs in <1s. Run it with python3 directly (no dependency install); also reduces fleet load. No code datetime.utcnow() violation exists (validator passes clean locally).
0ac4528 to
7ad3388
Compare
…x + routine) (#1879) ## omniclaude dev→main promotion Promotes `dev` (5fa434d) to `main` (currently e614e57). 9 commits, all routine previously-merged/reviewed ci/build/docs/fix work. The load-bearing change is the OMN-14244 deploy-gate matcher fix, which callers (omnibase_core/omnibase_infra/omnimarket) resolve from omniclaude `@main` — this promotion is required to unblock omnibase_infra#2245 (OMN-14190 Piece-4). Promotion branch tree equals `origin/dev` (`-s ours --allow-unrelated-histories` merge of `origin/main` for mergeability only — `origin/main` and `origin/dev` have no common git ancestor in this repo's history; verified `git diff origin/dev` on the promotion branch is empty). dev-head (5fa434d) is an ancestor of this branch so the cited OCC receipt binds. Tracked under a dedicated promotion ticket (OMN-14249, following the OMN-13828 promotion-ticket precedent) so the OCC receipt binds cleanly to this PR's number without superseding the already-merged OMN-14244/OMN-14190 contracts. Branch renamed from an earlier `promotion/omn-14244-...` attempt (#1878, closed) because the repo's DoD Evidence Check hard gate derives its required ticket from the branch name. ### Commits (origin/main..origin/dev) - `5fa434d06` fix(OMN-14244): narrow deploy-gate runtime-path matcher (stop false-positiving non-deploy CLIs, catch real deploy CLIs) (#1876) - `1b74ec1f0` feat(OMN-14190): wire OCC stamp/validate pre-commit hook (#1875) - `62308da2e` docs(OMN-14105): genericize onex skills — strip internal operating detail for public exposure (#1869) - `60cfec6fd` ci(OMN-14176): keep hostile reviewer on dev torch-free path (#1873) - `df599a05c` docs(OMN-13961): handoff skill writes canonical docs/handoff/ (singular) not docs/handoffs/ (#1864) - `8e19db1b2` build(OMN-14178): bump omniintelligence pin to torch-free base (post-#768 rl split) (#1874) - `a6272b37d` ci(OMN-13502): flip no-faked-boundary gate report-only -> enforcing (#1860) - `b94d846db` ci(OMN-13902): sibling-lock-refresh workflow (fan-out) (#1857) - `12fe25f4d` fix(OMN-13940): unquarantine executable merge sweep cron (#1861) ### Note on `a6272b37d` (no-faked-boundary: report-only -> enforcing) This is a hardening flip, not a behavior regression — omniclaude's self-scan was burned to zero findings before the flip (OMN-13500/#1849) and the job is not yet a required branch-protection context (documented as a deliberate follow-up in #1860's own body), so it cannot wedge this or any other PR. ### Why this is not a prod/runtime action This is a `dev -> main` release-branch promotion for the omniclaude repo (a Claude Code plugin repo). It does not touch `.201`, any prod compose lane, or any live runtime. Operator-authorized (OMN-14190 Piece-4 chain). promotion-receipt: OCC-3791 Evidence-Source: OCC#3791 Evidence-Ticket: OMN-14249 Evidence-Commit: 40b994cd457bdd5cc71b123c8787114ea891c0ce Refs OMN-14249, OMN-14244, OMN-14190, OMN-13502. Supersedes #1877/#1878 (closed). --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Phase 3 of OMN-13502 (epic OMN-13498, golden-chain de-fake) for the omniclaude side of the flip.
no-faked-boundarydetector self-scan down from 61 findings (55mock_assigned_to_boundary+ 6patch_httpx_egress) to zero, verified live on this branch viauv run python -m omnibase_core.validation.no_faked_boundary.runtime_no_faked_boundary src tests→No faked-boundary violations found.--report-onlyfrom both surfaces that ran the detector:.github/workflows/no-faked-boundary.yml(CI mirror).pre-commit-config.yamlcheck-no-faked-boundaryhook (local mirror)"No Faked Boundary Gate (report-only)"to the bare"No Faked Boundary Gate"so it can become a dev branch-protection required context after it lands and reports once on a dev commit — required-context addition is a separate follow-up step, not part of this PR, per the ordering constraint (a required context that has never reported wedges every PR).DoD evidence
Ticket OMN-13502 carries dod_evidence from the parallel omnibase_core/omnimarket legs of this same phase-3 flip (PRs #1377, #1378, #1608, OCC receipts #3530, #3533, #3535, #3587). This PR is the omniclaude leg of the same de-fake flip.
Test plan
uv run python -m omnibase_core.validation.no_faked_boundary.runtime_no_faked_boundary src tests→ 0 findings (confirms flip is safe)pre-commit run --all-files—check-no-faked-boundarypasses; 3 pre-existing/unrelated failures confirmed present identically onorigin/devwithout this diff (Windows-path string inarchitecture-handshakes/validator-requirements.yaml, cross-repo@mainref incall-occ-preflight.yml,hook-bits-driftmissing siblingomnibase_corecheckout in this worktree)uv run pytest tests/ -v -m "unit and not integration"→ 9103 passed, 26 skipped, 1 pre-existing failure (test_checkout_occ_contracts_shell_suite, missinggtimeout/timeoutbinary on this Mac — confirmed identical failure onorigin/devwithout this diff)Summary by CodeRabbit
Evidence-Source: OCC#3599
Evidence-Ticket: OMN-13502