Repository navigation
fix(OMN-14560): wire the omnimarket drift guard into cli_node.py - #2294
Conversation
The `onex node`/`onex run` dispatch path had ZERO drift-guard wiring --
it raised a bare "Unknown node '<name>'" whenever the omnimarket
co-install was drifted or absent, with no pointer to the actual cause
or the repair command. This is the identical gap OMN-14531 fixed for
`onex skill` (cli_skill.py), flagged there as a known follow-up.
Mirrors that fix exactly, reusing the same
`omnimarket_drift_guard.check_omnimarket_drift()` helper (no
duplicated logic):
1. Added `--omni-home` bound to the `$OMNI_HOME` envvar.
2. Call `check_omnimarket_drift()` at the top of `run_node_by_name`,
before `_resolve_packaged_contract`, converting
`OmnimarketDriftError` to a `click.ClickException`.
Since `onex node` and `onex run` are both entry-point aliases for the
same `run_node_by_name` function (pyproject.toml
`[project.entry-points."onex.cli"]`), one wiring point fixes both.
RED -> GREEN evidence (dod_evidence): new
`test_drift_guard_fires_before_unknown_node_lookup` in
tests/unit/cli/test_cli_node.py fails against the pre-fix cli_node.py
(no --omni-home option, no guard call -- reports the old bare
"Unknown node" error) and passes against the fix (raises the loud
OmnimarketDriftError with repair command, before node resolution ever
runs).
Wiring the guard surfaced the same non-hermeticity gap OMN-14531 had
to guard against, but across three test files that all reach
run_node_by_name (test_cli_node.py, test_cli_node_receipt.py,
test_onex_run.py) rather than one -- added a shared
tests/unit/cli/conftest.py autouse fixture (neutralize + delenv
OMNI_HOME) instead of duplicating a file-local fixture three times.
Manually reproduced against the real `onex node`/`onex run` CLI on
this operator's drifted venv (omnimarket absent, canonical clone
present):
$ uv run onex node definitely_not_a_real_node
Error: omnimarket is NOT INSTALLED from git in this interpreter
..., but a canonical clone exists at $OMNI_HOME/omnimarket (HEAD
48c03808155e). ... Repair with:
scripts/install-node-skill-package.sh --execute ...
exit 1
$ uv run onex run definitely_not_a_real_node
(identical output -- same underlying function via the "run" alias)
Local gates: `ruff format --check` / `ruff check` / `mypy` clean on
touched files; full `pre-commit run` clean; tests/unit/cli/ full
suite: 342 passed, 19 skipped (pre-existing omnimarket-not-in-CI-venv
skips, unrelated).
Stacked on OMN-14531 (#2291, not yet merged) -- this branch is built
on top of jonah/omn-14531-aislop-sweep-dispatch to reuse its
omnimarket_drift_guard.py fix rather than duplicating the reordering
logic. This PR's diff will resolve down to just the cli_node.py +
test changes once #2291 merges to dev.
OCC companion owed to Codex (per operating rules -- no self-authored
OCC in this lane).
Evidence-Ticket: OMN-14560
5d04cbc to
d3cf8ce
Compare
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 35 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 (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary
OMN-14560: the
onex node/onex rundispatch path (cli_node.py) has ZEROdrift-guard wiring -- the identical gap OMN-14531 (#2291) fixed for
onex skill(cli_skill.py), flagged there as a known out-of-scopefollow-up. Before this fix, a machine with a canonical
$OMNI_HOME/omnimarketclone but omnimarket drifted/absent from the venv got a bare
Unknown node '<name>'fromrun_node_by_name-- no pointer to the actualcause or the repair command.
Mirrors OMN-14531's fix exactly, reusing the same
omnimarket_drift_guard.check_omnimarket_drift()helper (no duplicatedlogic):
--omni-home, bound to the$OMNI_HOMEenvvar (same pattern ascli_skill.py).check_omnimarket_drift()at the top ofrun_node_by_name, before_resolve_packaged_contract, convertingOmnimarketDriftErrorto aclick.ClickException.onex nodeandonex runare both[project.entry-points."onex.cli"]aliases for the same
run_node_by_namefunction, so one wiring point fixesboth dispatch surfaces.
Stack status
#2291 / OMN-14531 has merged to dev; this PR is now rebased to a single top commit on current dev and the diff is limited to
cli_node.py,tests/unit/cli/conftest.py, andtests/unit/cli/test_cli_node.py.RED -> GREEN evidence (dod_evidence)
New
test_drift_guard_fires_before_unknown_node_lookupintests/unit/cli/test_cli_node.py:cli_node.pyfix, kept the test): all 8 tests in thefile error out --
AttributeError: <module cli_node> has no attribute 'check_omnimarket_drift'(proves the pre-fix module has zero guardwiring for the fixture/test to even attach to).
OMN-14531 regression shape (canonical clone present, omnimarket not
installed from git) through the CLI and asserts the loud
OmnimarketDriftErrormessage fires before node resolution ever runs,instead of the CLI's formatted
Unknown node '<name>'.error.Wiring the guard surfaced the same non-hermeticity gap OMN-14531 had to
guard against in
test_cli_skill.py, but here across three files that allreach
run_node_by_name(test_cli_node.py,test_cli_node_receipt.py,test_onex_run.py-- the last invokes it via theonex runalias throughthe full
cligroup). Added a sharedtests/unit/cli/conftest.pyautousefixture (neutralize +
delenv OMNI_HOME) instead of duplicating afile-local fixture three times.
Manual repro against the real
onex node/onex runCLI on thisoperator's actually-drifted venv (omnimarket absent, canonical clone
present):
Local gates:
ruff format --check/ruff check/mypyclean on touchedfiles; full
pre-commit runclean;tests/unit/cli/full suite: 342passed, 19 skipped (pre-existing omnimarket-not-in-CI-venv skips,
unrelated).
OCC
Central OCC companion is being authored by Codex after #2291 landed and this branch was cleaned to a one-commit diff.
Evidence-Source: OCC#4104
Evidence-Ticket: OMN-14560
Evidence-Commit: db334ebb3298811dcdea8f7cf3dd7c9bd4e2eb1c