Skip to content

fix(OMN-15617): remediation — silent-skip ordering + PATH word-split - #2652

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-15617-remediation-silent-skip-path-split
Aug 4, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-15617-remediation-silent-skip-path-split

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Aug 4, 2026 •

Copy link
Copy Markdown
Collaborator

OMN-15617 — remediation round 1: silent-skip ordering + PATH word-split

Follow-up to #2651, which merged to dev (commit 16b1d7a4, 2026-08-04T16:40:18Z)
while this remediation was in flight — the adversarial-verify fixes below did
NOT make it into that merge.
Operative consequence, stated plainly:
dev is currently live-shipping the unfixed resolver form — confirmed live
via git show origin/dev:scripts/ci/resolve_modern_bash.sh (CANDIDATES="" /
unquoted for _candidate in $CANDIDATES word-split, no array) and
git show origin/dev:tests/.../test_runner_monitor_wedge_detection.py
(tool-skip loop still precedes resolve_modern_bash()). This PR (#2652) is
the only carrier of both fixes and is currently CI-blocked (see below), so
there is no landed remediation on dev as of this report.

Fixes

  • Silent-skip ordering (test_runner_monitor_wedge_detection.py,
    test_runner_monitor_auto_bounce.py): resolve_modern_bash() now runs
    BEFORE the jq/flock tool-availability pytest.skip(). Previously a
    missing secondary tool short-circuited the bash>=5 canary via skip, so a
    host with both the wrong interpreter AND a missing tool would report
    green-by-absence instead of the RED ticket AC Add Claude Code GitHub Workflow #2 requires ("the canary
    fails RED when bash resolves <5").
  • PATH word-splitting (scripts/ci/resolve_modern_bash.sh): CANDIDATES
    is now a bash-3.2-safe array instead of a space-joined string iterated with
    unquoted word-splitting. The prior form silently dropped any interpreter
    path or $PATH entry containing a space — a silent-wrong-answer mode in
    the exact script whose purpose is eliminating that failure class.

Seams

Seam Detail
Files scripts/ci/resolve_modern_bash.sh, tests/unit/observability/runner_health/test_runner_monitor_wedge_detection.py, tests/unit/observability/runner_health/test_runner_monitor_auto_bounce.py — same three seams #2651 declared (OMNIBASE_INFRA_BASH_BIN, the resolver's stdout/stderr/exit contract, _resolve_modern_bash.py::resolve_modern_bash() import) are unchanged; no new seam introduced
Behavior change Call-order only in the two test files; try_candidate/bash_major_version logic in the resolver is unchanged, only the CANDIDATES container type (string → array)

Verification

# quiet run, full related scope
102 passed, 4 skipped (flock-only, unrelated) — env -i PATH=/usr/bin:/bin
  • Reordering proof: full scope above shows resolve_modern_bash() executes
    (and passes, proving bash>=5 resolves) even on this host's pre-existing
    missing-flock gap, before that tool-skip fires — the skip no longer
    masks the canary.
  • Fail-closed RED proof: OMNIBASE_INFRA_MIN_BASH_MAJOR=99 bash scripts/ci/resolve_modern_bash.sh → exit 1, pointed stderr, empty stdout.
  • Space-in-path proof: OMNIBASE_INFRA_BASH_BIN="/tmp/bash test dir/bash"
    now resolves correctly post-fix (pre-fix would silently drop the
    space-split fragment and fall through to a different candidate).

CI status (live, re-verified 2026-08-04 ~17:35Z UTC)

Six checks currently fail on this PR, all downstream of one root cause
(OCC-eligibility class), plus one adversarial gate unrelated to this diff:

Check Root cause
verify / verify (job 92073754869, step "Run OCC Eligibility") OCC-eligibility mismatch — confirmed via job log: occ_commit_sha resolves to 198a738a (companion #6047, bound to PR #2651), reason: pr_ticket_mismatch, eligible: false
occ-preflight / eligibility (×2 job instances) same OCC-eligibility mismatch
call-reject-skip-token / occ-preflight / eligibility same OCC-eligibility mismatch
CI Summary umbrella fail-closed on the above
Hostile Review Gate separate — not diagnosed as OCC-eligibility class

Correction to round-1 report: the round-1 enumeration ("5 fail") omitted
verify / verify — six checks fail, not five.
Its failing step and payload
were pulled directly from the job log and confirm it is the same
OCC-eligibility class as the other four, not a distinct defect.

OCC companion status: #6054 (OmniNode-ai/onex_change_control#6054,
bot-authored app/onexbot-occ-writer, correctly bound to this PR per its own
body) is OPEN, not yet merged to OCC main. At round-1 report time it showed
three failing gates (OCC Append-Only Gate, Pre-commit,
Supersession Binding Ratchet (OMN-15459)). Re-verified live just now:
all three are now green
(OCC Append-Only Gate success, Supersession Binding Ratchet (OMN-15459) success, Pre-commit success — confirmed via
gh api .../check-runs), self-resolved by the bot without action from this
lane. The remaining blocker on verify / verify / occ-preflight / eligibility is structural, not a defect in #6054: eligibility is evaluated
against OCC main, and #6054 has not merged there yet, so no PASS receipt
bound to PR #2652 exists on OCC main. Merging OCC PRs is outside this lane's
authority (bot/Codex-owned); this PR cannot flip eligibility green on its
own.

.200 live validation — still blocked, and a residual gap disclosed

ssh -o BatchMode=yes -o ConnectTimeout=8 stickybeatz-studio still returns
Permission denied (publickey,password,keyboard-interactive) for this
session's identity (re-verified 2026-08-04). Same disclosed gap as #2651,
unchanged by this PR. AC #1 ("a clean-dev run of the 15 monitor.sh tests
passes over plain non-interactive ssh with no per-lane PATH surgery") is
NOT met by this PR
— no run against stickybeatz-studio exists from any
session to date. The fix demonstrably fires on this Mac only; per the
ticket's own OMN-13980 precedent warning, a canary that only fires off-target
does not close the ticket.

Residual gap: the ticket's Fix section offered two options — PATH
correction in the ssh non-login shell init on .200 itself, OR an explicit
interpreter pin in the affected test harness. This PR takes the harness-pin
route (permitted by the ticket), which fixes resolve_modern_bash.sh
consumers but leaves .200's ssh non-interactive PATH itself unchanged. Any
OTHER bash>=4 consumer invoked over non-interactive ssh on .200 (outside
this resolver's call sites) remains exposed to the same wrong-interpreter
failure mode this ticket targets. Neither this PR body nor the round-1
report previously stated that residual; stating it now.

Ticket: OMN-15617

Evidence-Ticket: OMN-15617

Evidence-Source: OCC#6057

…d-split

Addresses two adversarial-verify defects on PR #2651:

- Reorder resolve_modern_bash() before the jq/flock tool-availability skip
  in test_runner_monitor_wedge_detection.py and
  test_runner_monitor_auto_bounce.py. Previously a missing jq (or flock)
  triggered pytest.skip() before the bash>=5 canary ran, so a host with the
  wrong interpreter AND a missing secondary tool would report green-by-
  absence instead of the RED the ticket's AC2 requires.
- Fix scripts/ci/resolve_modern_bash.sh to build CANDIDATES as a bash-3.2-
  safe array instead of a space-joined string. The prior unquoted
  word-splitting silently dropped any interpreter path or PATH entry
  containing a space, risking a silent-wrong-answer resolution in the exact
  script whose purpose is to eliminate that failure mode.

Verified: 102 passed / 4 skipped (flock-only, unrelated) under
env -i PATH=/usr/bin:/bin; resolver returns exit 1 with pointed stderr and
empty stdout when OMNIBASE_INFRA_MIN_BASH_MAJOR is set unreachably high
(proves genuine fail-closed RED); resolver correctly resolves a
space-containing OMNIBASE_INFRA_BASH_BIN path post-fix (previously would
silently drop the fragment).

Ticket: OMN-15617
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a02cfe4f-9e8b-499c-b609-0bb1d3d28a8d

📥 Commits

Reviewing files that changed from the base of the PR and between 888907b and 0ec3865.

📒 Files selected for processing (3)
  • scripts/ci/resolve_modern_bash.sh
  • tests/unit/observability/runner_health/test_runner_monitor_auto_bounce.py
  • tests/unit/observability/runner_health/test_runner_monitor_wedge_detection.py

Comment @coderabbitai help to get the list of available commands.

jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request Aug 4, 2026
#6054)

* evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2652

* evidence: OCC companion self-bind for #6054

* fix(OMN-15617): repair OCC 6054 receipt bindings

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
Co-authored-by: Jonah Gray <jonah@omninode.ai>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8468/OMN-8524) and do not block merge. Error: all review endpoints [192.168.86.201:8000 192.168.86.201:8001 ] unreachable — preflight short-circuit (no models available)


Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical findings No
blocked CRITICAL findings found Yes
degraded All models unavailable (infra) No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)

@jonahgabriel
jonahgabriel merged commit b3c7635 into dev Aug 4, 2026
209 of 239 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-15617-remediation-silent-skip-path-split branch August 4, 2026 20:55
@jonahgabriel

Copy link
Copy Markdown
Collaborator Author

AC#1 proof over plain non-interactive ssh (OMN-15617)

Ran the full RED/GREEN/guard protocol over real non-interactive ssh (ssh -o BatchMode=yes stickybeatz-studio '<cmd>' — this session runs on that same host, so loopback ssh gives the genuine stock non-interactive PATH: /Users/jonah/.local/bin:/Users/jonah/.cargo/bin:/usr/bin:/bin:/usr/sbin:/sbin, which resolves bare bash to system 3.2.57).

RED control (detached worktree at 234f7bd412, dev before this PR but after #2651): the literal 15 test_runner_monitor_wedge_detection.py tests already pass (#2651 already fixed PATH-independent resolution for that suite). The auto_bounce suite (4 tests) silently SKIPs because flock is genuinely absent on this host — and forcing OMNIBASE_INFRA_MIN_BASH_MAJOR=99 on this RED baseline proves the actual bug this PR fixes: even with the interpreter forced unreachable, the pre-fix ordering never reaches resolve_modern_bash() — the flock-missing skip fires first and silently masks the broken interpreter (4 skipped, no failure surfaced).

GREEN (clean origin/dev @ d53e3cfa90, includes this PR): 15/15 wedge_detection tests pass over plain non-interactive ssh, zero PATH exports, zero per-lane workaround — AC#1 satisfied verbatim. Same forced-interpreter sub-test now produces 4 loud FAILs with the pointed resolver message instead of silent skips — the ordering fix confirmed working.

Guard proof (AC#2): OMNIBASE_INFRA_MIN_BASH_MAJOR=99 /bin/bash scripts/ci/resolve_modern_bash.sh under system bash 3.2 → exit 1, pointed ERROR:/REMEDIATION: message, no silent fallback.

Full command-by-command transcript posted on OMN-15617. Not merging/flipping anything here — proof only.

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