Repository navigation
fix(OMN-15645): bind DELEGATION_ROUTING_TIERS_PATH on .201 compose lanes - #2620
Merged
Merged
Conversation
omnimarket#2000 (OMN-15628) removed the packaged-default fallback for DELEGATION_ROUTING_TIERS_PATH in the delegation routing reducer's _get_config() singleton -- an unbound key now raises ProtocolConfigurationError at first config read instead of silently defaulting. The key was bound on zero .201 compose lanes, so the next cold rebuild of dev/stability-test/judge (and the next prod release promotion carrying #2000) refuses boot. Bind the key at the shared x-runtime-env anchor in docker/docker-compose.infra.yml (all four lane projects inherit it), same locus/reason BIFROST_CONTRACT_PATH is already hardcoded (OMN-12864: no ${VAR:-} ambient-override footgun). The bound value is a fixed, non-version-embedded in-image path (/app/contracts/delegation/routing_tiers.yaml) that docker/Dockerfile.runtime bakes at build time via a glob-derived COPY (python*, matching the existing omnibase_core runtime contracts COPY precedent) -- never a literal python3.X site-packages path. Services with no delegation-routing surface (projection-api, omninode-contract-resolver) explicitly opt out with "", mirroring the BIFROST_CONTRACT_PATH opt-out. Mechanism: extends tests/ci/test_runtime_env_anchor.py's REQUIRED_RUNTIME_KEYS registry (code-declared-required-key -> compose-anchor-coverage), adds compose-render regression tests to all four lane fixtures under tests/integration/infra/, and adds a static seam-consistency test file documenting the omnimarket contract. Fully generic code-scanning mechanism deferred to OMN-14951 (PR body). OMN-15645
📝 WalkthroughWalkthroughThe runtime image now packages ChangesDelegation routing tiers configuration
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docker/docker-compose.infra.yml`:
- Around line 169-183: Move the baked routing tiers artifact to a stable path
outside the /app/contracts mount and update DELEGATION_ROUTING_TIERS_PATH in
docker/docker-compose.infra.yml to reference it. Update
docker/Dockerfile.runtime to copy routing_tiers.yaml to that unmounted path. In
tests/unit/docker/test_delegation_routing_tiers_path_binding_omn15645.py:51-53,
tests/integration/infra/test_dev_runtime_compose_render.py:283-306,
tests/integration/infra/test_judge_compose_render.py:368-385,
tests/integration/infra/test_prod_runtime_compose_render.py:161-175, and
tests/integration/infra/test_stability_test_runtime_compose_render.py:451-465,
replace the expected path; additionally ensure the dev runtime test verifies the
/app/contracts mount does not mask the new path.
🪄 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: 086b40f7-789d-4a65-9acb-30319709fa1d
📒 Files selected for processing (8)
docker/Dockerfile.runtimedocker/docker-compose.infra.ymltests/ci/test_runtime_env_anchor.pytests/integration/infra/test_dev_runtime_compose_render.pytests/integration/infra/test_judge_compose_render.pytests/integration/infra/test_prod_runtime_compose_render.pytests/integration/infra/test_stability_test_runtime_compose_render.pytests/unit/docker/test_delegation_routing_tiers_path_binding_omn15645.py
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 2, 2026
#5917) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2620 * evidence: OCC companion self-bind for #5917 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
…parity debt CI's Env Parity gate (tests/ci/test_env_parity.py) requires every x-runtime-env key to be accounted for in the k8s ConfigMap, SECRET_KEYS, LOCAL_ONLY_KEYS, or CONFIGMAP_DEBT_KEYS. The new binding surfaced this gap. Add it to CONFIGMAP_DEBT_KEYS mirroring the existing BIFROST_CONTRACT_PATH precedent (OMN-10943) -- the k8s ConfigMap side is OMN-15628's own k8s-manifest-scoped acceptance criterion (In Progress, sibling omninode_infra checkout), not this ticket's scope. OMN-15645
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)
…ts mount CodeRabbit catch (PR #2620): the runtime services bind-mount ../contracts:/app/contracts:ro (host content). Baking routing_tiers.yaml into /app/contracts/delegation/ was silently shadowed by that mount at container start -- the image-baked file was invisible, defeating the whole fix and leaving the entrypoint's OMN-15628 self-heal to paper over it via internal re-export (which would NOT satisfy the OMN-15645 AC2 in-container test -f requirement against the compose-declared, static path). Relocate the bake destination (and the compose binding) to /app/config/delegation/routing_tiers.yaml -- a path with zero volume/bind-mount entries anywhere in the compose lane files. Verified on .201 with a real docker run reproducing the exact bind-mount shape (-v $(pwd)/contracts:/app/contracts:ro): the file now exists and resolves correctly under the mount, sha256-matches the packaged omnimarket copy, and _get_config() loads successfully end-to-end. Adds a permanent static regression test (test_expected_path_is_never_shadowed_by_a_volume_mount) that parses every runtime service's volumes: list and fails if the bound path ever falls under a mounted target again. OMN-15645
onexbot-occ-writer Bot
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 2, 2026
…-test/prod profile CI (which has real Docker, unlike .200) caught what manual .201 verification already showed but the test code didn't encode: omninode-contract-resolver is not part of the --profile runtime render for the stability-test and prod lanes (unlike dev and judge, where it either renders or is judge-profile-excluded via a different path). The new delegation-routing-tiers-path tests hard-indexed services['omninode-contract-resolver'], raising KeyError instead of skipping the absent service. Guard both assertions with services.get(...) + skip-if-absent, matching the pattern already used in the judge lane's version of this test. OMN-15645
jonahgabriel
added a commit
that referenced
this pull request
Aug 2, 2026
…ut dev red (#2622) omnibase_infra dev has been RED on its own committed env-parity gate since merge 04b4b43. Two individually-green PRs, merged 14 minutes apart, each added DELEGATION_ROUTING_TIERS_PATH to the SAME docker-compose.infra.yml x-runtime-env anchor with a DIFFERENT value: #2620 (OMN-15645, 03:21:51Z) -> /app/config/delegation/routing_tiers.yaml #2621 (OMN-15628, 03:35:23Z) -> /app/.venv/lib/python3.12/site-packages/... YAML keeps the last occurrence, so the earlier declaration became silently dead and the elected value contradicted judge/e2e and the k8s Deployment pin. Neither PR could have gone red on its own; the failure exists only in the merge product. FIX - Delete the duplicate declaration. One canonical compose value survives: /app/config/delegation/routing_tiers.yaml -- the version-independent path docker/Dockerfile.runtime:851-867 bakes from the installed omnimarket package, and the one that is not shadowed by the ../contracts:/app/contracts bind-mount. - Align docker-compose.judge.yml and docker-compose.e2e.yml to the same literal. All three lanes build the same Dockerfile.runtime image. GATES ADDED (each proven RED against the merged-dev tree first) - test_no_duplicate_keys_in_compose_files: node-level duplicate-key scan over every docker-compose*.yml. This is the check that would have caught the collision. Loader-based checks structurally cannot -- yaml.safe_load collapses the duplicate before any assertion runs. - test_delegation_routing_tiers_path_matches_k8s_pin: extended from infra.yml-only to every lane that stands up a runtime container, and now requires all lanes to agree on one literal. Accepts the k8s pin or a path Dockerfile.runtime bakes it to -- aliases are PARSED from the Dockerfile COPY, never hardcoded, so a typo still fails. VERIFIER REMEDIATION (round 1 findings) - Forward parity surface no longer widened to a plain union. It is now the runtime-FAMILY surface: ConfigMap keys plus keys inline on ALL THREE runtime Deployments, so a per-workload binding cannot satisfy an anchor-wide claim. test_k8s_family_surface_requires_all_runtime_deployments locks it against the real manifests with the binding removed from exactly one Deployment. - Lane-overlay inheritance check is structural over the parsed service graph instead of a raw-text regex. The old regex matched only the literal string "environment: !override" and was blind to !reset, which severs inheritance identically. - Module docstring now states each direction's scope explicitly, including that the reverse walk is deliberately infra.yml-only. ALSO FIXED (second live dev breakage, pre-existing, same blast radius) tests/unit/infra/test_compose_no_silent_fallbacks.py scanned raw file text, so the #2620 comment that explains the ban by quoting it registered as a violation on a variable literally named VAR. The matcher now strips comments, quote-aware. test_comment_stripping_does_not_defang_the_ban proves a real empty-default in live YAML still fails, that a hash inside a quoted scalar is not eaten, and that only the prose case is exempt. KNOWN, NOT ASSERTED: onex-dev configmap.yaml carries this key pinned to a third path (/app/contracts/delegation/routing_tiers.yaml). All three runtime Deployments override it inline and inline env beats envFrom, so the runtime family is unaffected -- but a workload that envFroms without an inline override would get the stale path. That is an omninode_infra fix; asserting it here would leave a permanently-red gate on a defect this repo cannot resolve. Ticket: OMN-15628 Co-authored-by: Jonah Gray <jonah.g.gray@gmail.com>
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-15645
Binds
DELEGATION_ROUTING_TIERS_PATHon all four.201compose lane projects (omnibase-infra,omnibase-infra-stability-test,omnibase-infra-judge,omnibase-infra-prod). omnimarket#2000 (OMN-15628) removed the packaged-default fallback for this key in the delegation routing reducer's_get_config()singleton; an unbound key now raisesProtocolConfigurationErrorat first config read. The key was bound on zero.201compose lanes before this PR — the next cold rebuild of dev/stability-test/judge (and the next prod release promotion carrying #2000) refuses boot.Fix
docker/docker-compose.infra.yml: bindDELEGATION_ROUTING_TIERS_PATHin the sharedx-runtime-envanchor (same locus/reasonBIFROST_CONTRACT_PATHis hardcoded, OMN-12864: no${VAR:-}ambient-override footgun). All four lane projects inherit this anchor (verified live, see below).projection-apiandomninode-contract-resolverexplicitly opt out with""(no delegation-routing surface), mirroring the existingBIFROST_CONTRACT_PATHopt-out for the same two services.docker/Dockerfile.runtime: bakes the packaged omnimarketrouting_tiers.yamlinto the fixed path/app/contracts/delegation/routing_tiers.yamlat build time viaCOPY --from=builder .../lib/python*/site-packages/omnimarket/configs/routing_tiers.yaml. Thepython*glob is the same technique already used for the omnibase_core runtime-contracts COPY a few lines above — never a literalpython3.Xsite-packages path, which is exactly the trap OMN-15628's k8s-side entrypoint self-heal (merged just before this PR,docker/entrypoint-runtime.sh) exists to correct for a stale pin. That self-heal is untouched and remains defense-in-depth.Seam definition (omnimarket consumer, field-by-field)
DELEGATION_ROUTING_TIERS_PATH(exact string)omnimarketsrc/omnimarket/nodes/node_delegation_routing_reducer/handlers/handler_delegation_routing.py:392-393,_get_config()singletonresolve_required_path_config("DELEGATION_ROUTING_TIERS_PATH")—omnimarket/src/omnimarket/inference/delegation_config_provenance.py:126-158os.environ.get(key, "").strip()(delegation_config_provenance.py:225) —""and unset are equivalent, which theprojection-api/contract-resolveropt-outs rely onresolve_required_path_config, notresolve_path_config) — raisesValueError(wrapped toProtocolConfigurationError) when blank/unsetProtocolConfigurationErrornaming the key,ONEX_CORE_041_INVALID_CONFIGURATIONModelDelegationConfigProvenance.log_line()INFO log:config_provenance surface=delegation config_key=DELEGATION_ROUTING_TIERS_PATH source=contract_overlay_env override_present=true resolved_path=<path>BIFROST_CONTRACT_PATHis explicitly not touched — different resolution semantics (resolve_optional_path_config), already bound everywhere; out of scope per the ticket.Acceptance criteria -> proof
AC1 (RED first). Captured against the real merged-
devcode (omnibase_infra@94a3a11c1, omnimarket@3d60ae73— both pulled fresh on.201), driving the real_get_config()singleton (installed viauv pip install --no-deps -einto a venv, not mocked),DELEGATION_ROUTING_TIERS_PATHunset:AC2 (all four lanes bind the key to an existing in-image path, readback not assertion).
docker compose -p <project> configon.201(operator env sourced,set -a) for all four lanes:omnibase-infra(dev)/app/contracts/delegation/routing_tiers.yaml""""omnibase-infra-stability-test""omnibase-infra-judge""omnibase-infra-prod(config-level only, no recreate)""Plus, on the real image built by this PR's Dockerfile change (
docker compose ... build, workspace mode,OMNIMARKET_REF=3d60ae73...):Exact sha256 match, in-container
test -fexits 0.AC3 (GREEN).
deploy-runtime.sh --execute --coldon the dev lane (omnibase-infra) built cleanly from merged dev (all 8 images built, my Dockerfile COPY step included) — but the full orchestrated bring-up did not reach a clean end-to-end boot, blocked by two pre-existing, unrelated defects already on mergeddevindependent of this change:forward-migrationfails:unresolved migration domain for node:node_projection_delegation:0016_delegation_judge_verdict_events.sql (OMN-15423: delegation_judge_verdict_events domain unresolved)— a tracked, cited ticket, unrelated to env-var binding.omninode-runtimedirectly (bypassing the broken one-shot, migration-gate already healthy) crash-loops on a separate boot-time error:ONEX_DATABASE_TOPOLOGY_PROFILE is not setfor a list of newly-discovered projection contracts — this matches the class of work in OMN-15418 ("P0: Replace raw db_io loading with typed declarations and exactly-one-owner validation", In Review) / OMN-15417 (Done); not filing a duplicate, citing as the likely tracking ticket.Given those two blockers, GREEN is proven at the mechanism level instead, on the actual image this PR's Dockerfile builds:
The exact
ModelDelegationConfigProvenanceINFO log line the ticket asks for, the loader reading the real (sha256-matched) file, and the config object constructing successfully — on the real built image, real omnimarket wheel, real routing-reducer code path. Honest gap: the literal "boots clean end-to-end" wording of AC3 is not met on the dev lane today, for reasons outside this PR's diff. Dev lane'somninode-runtimecontainer is left stopped (not crash-looping) after this investigation; the old pre-build image tag was not recoverable (deploy-runtime.sh's own rollback failed to restore it — logged, unrelated to this PR).AC4 (mechanism, same PR). The existing
scripts/check_required_env_vars.pywalks compose->env-file only and is structurally blind to a code-required key missing from compose entirely (as the ticket states). This PR:tests/ci/test_runtime_env_anchor.py'sREQUIRED_RUNTIME_KEYSfrozenset (an existing, already-CI-wired reverse-direction check: declared-required-list -> anchor-coverage) withDELEGATION_ROUTING_TIERS_PATH, citing the omnimarket call site inline. Reuses existing machinery per the net-negative-surface rule rather than building a parallel check.tests/integration/infra/test_{dev,stability_test,judge,prod}*compose_render*.py) asserting the resolveddocker compose configshows the key bound correctly,docker-gated (skips cleanly without Docker, matching the existing convention).tests/unit/docker/test_delegation_routing_tiers_path_binding_omn15645.py: static (no-Docker) guards that the compose default and Dockerfile bake stay at the same fixed path, never regress to a version-embedded literal, and documents the omnimarket-side seam contract (omnimarket is not a pyproject dependency of omnibase_infra, so a literal cross-repo import test cannot run in this repo's own CI — the RED/GREEN evidence above is the actual cross-boundary proof).resolve_required_path_config(...)call sites, since omnimarket is not vendored in this repo and is not a pyproject dependency here) is deferred to OMN-14951 (gate: env-surface-as-contract, In Progress, named in the ticket as the natural home). Building a robust, non-gameable version of that scanner is a materially larger, separate piece of work than this ticket's registry-based extension.Prod boundary
Committing the compose/Dockerfile change is in scope. Recreating the live
omnibase-infra-prodruntime is explicitly not in scope and was not done — prod verification above is config-level readback only (docker compose config, noup/--force-recreate). Applying this to live prod routes through the gatedredeploy-start -> prod-promotion gate -> deploy-agentpath with a fresh grant, per CLAUDE.md rules 2a/12, at the next release promotion.Gates
Ran on
stickybeatz-studio(.200) via patch-transfer + sha256 verification of every changed file (local Edit/Write never touch .200 directly):ruff format --check,ruff check, focusedpytest(static seam tests +test_runtime_env_anchor.py+ existing self-heal tests, all green; the four new integration render tests skip cleanly on .200 — no Docker daemon there — and were verified directly against realdocker compose config/docker runon.201instead, see AC2/AC3 above),mypy(3 pre-existingdict-without-type-args errors in code this PR did not touch, confirmed identical on the unmodified base),pre-commit run --all-fileson the changed set (all pass; one pre-existing host-env gap on.200's local~/.omnibase/.envunrelated to this diff, fixed at the host level so the gate host itself is usable going forward).OMN-15645
Summary by CodeRabbit
Evidence-Source: 1eda09714b927494a06d0fd17672503c287eeaf1
Evidence-Ticket: OMN-15645