Repository navigation
fix(OMN-15122): deploy-runtime.sh step 3b resolves omnibase_core runtime contracts from workspace path - #2444
Merged
Conversation
…ime contracts from workspace path Root cause: importlib.util.find_spec(\"omnibase_core\") returns None on the deploy runners python3 (omnibase_core is not installed there at all), and the editable-install fallback assumed a site-packages-shaped layout (<pkg_dir>/../../contracts/runtime_data) that does not match the real source-tree layout (<repo>/src/omnibase_core/contracts/runtime_data). Both resolution paths failed, aborting every release-train-lab deploy at step 3b. Fix: extract resolution into resolve_core_contracts_dir(), which probes the OMNI_HOME sibling clones real filesystem path FIRST (the deploy source of truth -- the same pinned sibling clone the rest of the workspace build vendors from), then falls back to the python import resolution second for hosts where omnibase_core happens to be pip/editable-installed. Neither resolving is a named, fail-fast error listing every path probed -- no silent default, no hardcoded absolute paths. Adds tests/scripts/test_deploy_runtime_core_contracts_resolution.py, which extracts and executes the pure bash function against a real (fake) filesystem to prove RED against the exact \"directory exists but wrong shape\" failure this ticket diagnosed, with a stubbed python3 so results are deterministic regardless of host venv state.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 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 (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
| 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
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Jul 25, 2026
#4848) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2444 * evidence: OCC companion self-bind for #4848 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
jonahgabriel
added a commit
that referenced
this pull request
Jul 26, 2026
…t catalog, T0/T1 readback, dev gotchas, prod pointer (#2454) Extends docs/runbooks/release-train-lab.md (existing mechanism doc, not a new file) with the operator-facing procedure proven live on run 30180376657 (2026-07-26, tag lab/stability/20260725T235956Z-87ec5b3165ce, overall: PASS) — the terminal success of the six-iteration OMN-14900 hardening chain. - Copy-paste tag-cut + watch procedure with expected output and a tag-content WARNING (never reuse a parked tag name). - Preflight catalog: what each of the 6 landed fixes (#2450/#2446/#2444/ #2452/#2434/#2448) catches, with pre-fix failure signatures. - T0/T1 readback discipline: health 18085/18086, contract-count floor vs. regression, discovery_errors baseline, consumer groups, vcs_ref ancestry; FAILED_ROLLED_BACK equality-proof discipline. - Dev-lane gotchas: stale workspace-build config YAML, OMN-14968 false FAILED on runtime-worker; pointer (not duplicate) to cold-lane-full-bringup.md for cold bring-up. - Prod: pointer-only section citing CLAUDE.md rules 2a/12 and the onex_change_control#4892 prep-only grant-PR pattern; explicit raw-docker-mutation prohibition citing the no-raw-prod-bypass gate. - Verification checklist + rollback section. Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
jonahgabriel
added a commit
that referenced
this pull request
Aug 14, 2026
…ar drift v0.38.4 is a published tag, so packaged-source changes on this branch must carry a version ahead of it or the OMN-13412 release-identity gate fails closed (two distinct code states would alias under one image version). Also corrects a stray SPDX copyright year (2026 -> 2025) in a test file that arrived on dev via #2444; 5030 other headers in the tree use 2025, so this was the lone outlier failing `pre-commit run --all-files`. No behaviour change.
jonahgabriel
added a commit
that referenced
this pull request
Aug 15, 2026
…put model (#2741) * fix(OMN-16050): stop envelope unwrap at the registered input model The auto-wiring dispatch path unwrapped `payload` recursively while a purely structural predicate held: a mapping carrying a `payload` mapping plus any of `_ENVELOPE_MARKER_KEYS`. The module asserted "domain models never declare these keys" — a FALSE invariant. A domain input model that legitimately declares both a `payload` mapping and a transport-plausible marker is indistinguishable from a transport envelope, so the runtime unwrapped THROUGH it and handed the kernel the caller's inner payload; `model_validate` then raised and the command was DLQ'd. An affected node could never be dispatched over the bus at all. `_extract_dispatch_payload` now accepts the dispatcher's contract-registered input model and stops the unwrap at a candidate that IS that model. A candidate is claimed only when BOTH hold: 1. key containment — every key on the candidate is a declared field (or input alias) of the target model. A real transport envelope always carries at least one routing key the domain model does not declare (`source_tool`, `envelope_id`, `__debug_trace`, `__bindings`, ...), so genuine double/triple-wrapped deliveries keep unwrapping through to the domain. 2. full `model_validate` — a partial structural coincidence never halts the unwrap short of the domain payload. The cheap set check runs first, so `model_validate` executes only for the rare candidate whose keys are entirely owned by the target model. Deliberately not a marker denylist: dropping `event_type`/`correlation_id` from the marker set would fix one model and silently break every genuine envelope carrying only those markers. The predicate keys on the CONTRACT-registered target type instead. Threaded at the two call sites where a registered model is in scope: the def-B `handle(request: ModelX)` coercion (the live path) and the contract-declared `event_model` branch. The six remaining call sites read correlation/DLQ metadata with no registered type in scope and are unchanged — `target_model=None` keeps the pre-existing structural behaviour exactly. Tests: RED reproduction of the exact production coercion failure (an envelope-shaped domain payload unwrapped through, 4 validation errors: event_type Field required + 3x extra_forbidden), regressions pinning that genuine nested transport envelopes still unwrap, and the fail-closed predicate in both directions (an undeclared key defeats the claim; an `extra="ignore"` model cannot claim a real envelope; a raising field validator reads as "not the model"). Runtime Startup CI gate: `tests/integration/test_auto_wiring_real_manifest.py` gains a case that loads the real contract manifest from disk via `discover_contracts()`, runs `wire_from_manifest` with the kernel's argument shape against a real `MessageDispatchEngine`, asserts zero unexpected failures, then invokes the dispatcher the wiring registered with the exact bytes captured in-pod. Pre-fix it fails inside the callback with the live ValidationError. Also applies pending ruff-format drift in a keycloak contract test surfaced by `pre-commit run --all-files` while gating this change (no behaviour change). * fix(OMN-16050): bump to 0.38.5 for release-identity + correct SPDX year drift v0.38.4 is a published tag, so packaged-source changes on this branch must carry a version ahead of it or the OMN-13412 release-identity gate fails closed (two distinct code states would alias under one image version). Also corrects a stray SPDX copyright year (2026 -> 2025) in a test file that arrived on dev via #2444; 5030 other headers in the tree use 2025, so this was the lone outlier failing `pre-commit run --all-files`. No behaviour change. * fix(OMN-16050): rebind runner-image identity lock after the 0.38.5 version bump runner-image-build-smoke failed closed: shared_env_digest recorded 638933d1c8de12afe5c8024c, recomputed 9a9a74df5af7a1cddd904744. ci_env_digest.DEFAULT_ENV_INPUTS hashes pyproject.toml and uv.lock, so the 0.38.5 bump this PR needs for the release-identity gate necessarily re-keys the shared CI env digest, which re-binds the runner image identity. Regenerated via scripts/ci/runner_image_identity.py --mode generate; identity v7 602c1ca464270df881bf5916b8d5affe -> ad3c8a1337c6b52dd1519d7614aace19. Verified clean origin/dev passes the same check, so this is caused by this PR's bump and not pre-existing drift. Every prior version-bump PR on dev carries the same companion lock update. No image_version change: the base image, Python, uv, runner, gh and kubectl pins are untouched. * fix(OMN-16050): claim AliasChoices/AliasPath wire keys in the unwrap-stop predicate CodeRabbit (Major, thread PRRT_kwDOPuAjtM6ZMk2X) on handler_wiring.py:1466: _model_declared_wire_keys collected only plain-string aliases, so a registered input model declaring validation_alias=AliasChoices(...) or AliasPath(...) was missing wire keys it genuinely accepts. That is fail-OPEN in exactly this defect's direction. Key containment would reject a candidate that IS the registered model, the while-loop would keep unwrapping into the caller's payload, model_validate would raise, and the OMN-16050 DLQ failure would come back for every contract aliased that way. The finding is correct and the fix is in the fix's own blast radius, so it is not deferrable to a follow-up. _validation_alias_wire_keys resolves all three shapes pydantic allows: str -> the key itself AliasPath("meta","id") -> "meta" (the FIRST segment is the top-level wire key; later segments index inside that value and are not top-level keys) AliasChoices(...) -> the union over its choices, recursively, since a choice may itself be an AliasPath Four new tests, each verified RED against the string-only collector before this commit: AliasChoices claimability under both spellings, AliasPath head-segment extraction, nested AliasChoices-of-AliasPaths flattening, and an end-to-end dispatch asserting the user payload survives intact rather than being unwrapped through. ruff + mypy --strict clean; 508 auto-wiring + real-manifest tests pass.
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-15122
Fixes the release-train-lab deploy abort at
deploy-runtime.shstep 3b:Could not locate omnibase_core runtime contracts.Blocks OMN-14900 (stability deploy hop). Live failure: run https://github.com/OmniNode-ai/omnibase_infra/actions/runs/30166226948 (2026-07-25T16:44Z,FAILED_ROLLED_BACK, rollback clean, no prod/judge impact).Root cause (reproduced directly, per the ticket's live readback -- not inferred)
importlib.util.find_spec('omnibase_core')returnsNoneon the deploy runner'spython3-- omnibase_core is not installed there at all.<pkg_dir>/../../contracts/runtime_data) that does not match the real omnibase_core source-tree layout (<repo>/src/omnibase_core/contracts/runtime_data), so even anlsof the runner's stagedomnibase_corecheckout came up empty.Fix
Extracted the resolution logic into a new pure bash function,
resolve_core_contracts_dir()(inscripts/deploy-runtime.sh, alongside the existingresolve_compose_file_args()/resolve_lane_overlay_filename()helpers):${OMNI_HOME}/omnibase_core/src/omnibase_core/contracts/runtime_data-- the OMNI_HOME sibling clone/checkout filesystem path, which is the deploy source of truth (the same pinned sibling clone the rest of the workspace build vendors from) and works even when omnibase_core is not pip-installed on the runner.find_specimport resolution, kept for hosts where omnibase_core happens to be pip/editable-installed (e.g. a developer workstation with no OMNI_HOME sibling clone)./Users//Volumespaths.Step 3b's call site now delegates to this function instead of inlining the python snippet, passing the probed-paths array and resolved-dir output by name (bash
local -nnameref, matching the existingresolve_compose_file_argspattern in this file).Seam note: the two nameref outputs (
core_contracts_probed,core_contracts_dir) are passed by variable name, never via$(...)command substitution -- a substitution wrapper forks a subshell, and a nameref-populated array inside that subshell does not propagate back to the caller. This was caught by a standalone bash smoke test during development (an earlier draft silently produced an empty resolved-dir output and an incorrect success on the fail-closed case once the caller's own local variable was also namedresolved, due to a nameref/local-variable name collision) -- documented inline in the function's header comment as a recurrence guard.Tests
tests/scripts/test_deploy_runtime_core_contracts_resolution.py(new, 7 tests) extracts and executes the pureresolve_core_contracts_dir()bash function in isolation against a real (fake) filesystem -- proving RED against the exact "directory exists but wrong shape" failure mode this ticket diagnosed, not just asserting the source text mentions the right strings:OMNI_HOME/omnibase_core/src/omnibase_core/contracts/runtime_datalayoutOMNI_HOME/omnibase_core/contracts/runtime_datadirectory (RED, not silently accepted)OMNI_HOMEis unsetA stubbed
python3(always answers as ifomnibase_coreis not importable) is placed first on PATH in every test invocation so results are deterministic across hosts -- without it, a machine whereomnibase_corehappens to be pip/editable-installed into the active venv (e.g. omnibase_infra's own venv, since omnibase_core is a runtime dependency) would make the secondary probe succeed unpredictably, masking whether the primary probe actually did the work.Verification run (
.200,env -u PYTHONPATH uv run pytest):tests/scripts/ -k deploy_runtime: 86/86 passed (no regression in adjacent deploy-runtime coverage).ruff format --check+ruff check: clean.shellcheck --severity=warning scripts/deploy-runtime.sh: clean.pre-commit(full hook set, viagit commit): passed.bash -n scripts/deploy-runtime.sh: syntax OK.dod_evidence
run 30166226948).release-train-lab.yml(that requires a fresh cut/deploy hop, out of scope for this PR); the acceptance criterion "readback: running revision advances pastaaae28f676e7, digest_changed, health-gate PASS, no rollback" should be verified on the next stability deploy hop attempt after this merges. A RED test proving the exact prior failure shape now exists and passes GREEN under this fix (test_wrong_shape_directory_is_red_not_silently_accepted).Not merging this PR myself -- Codex owns merge queue / merge per repo policy.
Evidence-Ticket: OMN-15122
Evidence-Source: OCC#4848
Evidence-Commit: 5aed0fa6d74b0ebe4deb120dded7b255b8454afc