Repository navigation
fix(OMN-15628): repair #2620/#2621 duplicate-key collision that put dev red + duplicate-key gate - #2622
Conversation
…ut dev red 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
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 8 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 (5)
Comment |
|
| 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)
1 participant
omnibase_infradev is RED right now — this fixes itTwo PRs merged 14 minutes apart on 2026-08-02 each added
DELEGATION_ROUTING_TIERS_PATHto the samedocker-compose.infra.ymlx-runtime-envanchor with a different value:/app/config/delegation/routing_tiers.yaml/app/.venv/lib/python3.12/site-packages/omnimarket/configs/routing_tiers.yamlYAML keeps the last occurrence. The earlier declaration became silently dead and the elected value stopped matching
judge/e2eand the onex-dev Deployment pin. Both PRs were individually green — the failure exists only in the merge product, which is why nothing went red pre-merge.Reproduced live, dev's own committed test against dev's own compose tree at
04b4b43e:It landed red because
.github/workflows/env-parity.ymlis in neither dev's required set (["CI Summary"]) norCI Summary's 17-nameEXPECTED_EXTERNAL_CONTEXTS. The gate ran, reported, and blocked nothing.Fix
One canonical compose value survives:
/app/config/delegation/routing_tiers.yaml. #2620's choice is the better one and it wins on merit, not on merge order:docker/Dockerfile.runtime:851-867bakes it from the installed omnimarket package with apython*glob, so it carries no interpreter minor version./app/contractsis bind-mounted (../contracts:/app/contracts:ro), which would shadow image-baked content;/app/config/has no bind-mount in any lane file.docker-compose.judge.ymlanddocker-compose.e2e.ymlare aligned to the same literal — all three lanes build the sameDockerfile.runtimeimage.Gates added — each proven RED against the merged-dev tree first
1.
test_no_duplicate_keys_in_compose_files— the check that would have caught this.Node-level (
yaml.compose) duplicate-key scan over all 10docker-compose*.yml. No loader-based check can catch this class:yaml.safe_loadcollapses the duplicate before any assertion runs. RED on the merged tree:2. Value lock extended to every runtime lane, and now requires cross-lane agreement.
Was
infra.yml-only with a presence-only check covering judge/e2e — the exact "bound on both sides, pointing at different paths" gap. Now coversinfra(3 services),judge(2),e2e(1) by value.Compose and k8s legitimately differ now, so the lock accepts the k8s pin or a path
Dockerfile.runtimebakes it to — and those aliases are parsed from theCOPY, never hardcoded, so they cannot drift from the Dockerfile. A typo still fails:Round-1 verifier findings, all addressed
test_k8s_family_surface_requires_all_runtime_deploymentslocks it against the real manifests with the binding removed from exactly one Deployment; the probe key is derived, not hardcoded. RED when the surface is reverted to a union.infra.yml; judge/e2e presence-onlyinfra.yml-only but the docstring implied broader!overridecheck was a raw-text regexenvironment: !overrideonly and was blind to!reset, which severs inheritance identically — demonstrated: injecting!resetgaveold regex catches it? NO, new check fails.docker-compose.generated.ymldev-lane, e2e, gateway, infisical-stability, infra, judge, prod, pypi-cache, runners, stability-test. Not bound:gateway(runsonex-gateway-forwarder),infisical-stability/pypi-cache/runners(no runtime service),prod/stability-test/dev-lane(delta-only overlays, inheritance asserted not assumed).docs/tracking/ROLLING_WORK_LEDGER.mdviascripts/ledger_lock.pybefore this build started.Second live dev breakage, also fixed
tests/unit/infra/test_compose_no_silent_fallbacks.pywas also red on04b4b43e(verified by stashing this branch's diff and running against unmodified dev). It scanned raw file text, so #2620's comment explaining the ban by quoting it registered as a violation on a variable literally namedVAR:Per CLAUDE.md rule 10 ("matcher too broad → narrow the matcher") the matcher now strips comments, quote-aware.
test_comment_stripping_does_not_defang_the_banproves the narrowing kept the teeth: a real${VAR:-}in live YAML is still caught, a#inside a quoted scalar is not eaten, and only the prose case is exempt.Known, deliberately not asserted
omninode_infraconfigmap.yaml:86pins this key to a third path,/app/contracts/delegation/routing_tiers.yaml. All three runtime Deployments override it inline and inlineenvbeatsenvFrom, so the runtime family is unaffected — but a workload thatenvFromsonex-runtime-configwithout an inline override would get the stale path. That is anomninode_infrafix; asserting it here would leave a permanently-red gate onomnibase_infrafor a defect it cannot resolve. Documented in the test docstring so the next reader is not surprised.Verification
All gates ran on the
.200gate host (stickybeatz-studio; the raw192.168.86.200route is down —Host is down). Patch-transfer verified before any gate ran: sha256 of all 5 changed files identical local vs.200. Remoteomninode_infrawas stale at3c3f5eb9and was pulled tobeba4c44first, so both hosts read the same k8s bytes.tests/ci/ tests/unit/infra/ tests/unit/docker/ tests/test_compose_profile_teardown_policy.py→ 1670 passed, 3 skipped (3 are pre-existing Docker-daemon integration skips). Before this branch: 1 failed, 1668 passed.tests/ci/test_env_parity.py→ 8 passed, 0 skipped.ruff format --check(2023 files),ruff check,mypy→ clean.No skip tokens, no
--no-verify, no-k/-mnarrowing, noxfail, no weakened assertions, no mocking of the seam under test. No deploy, restart, or.201mutation.Deployment note
Compose env is read at container-create time. Every
.201lane (dev/lab, stability-test, judge, prod) still runs with whatever it was created with; merge alone does not close that. Affected containers must be recreated, and prod recreation still requires a fresh@main-resolved, digest-scoped grant.Ticket: OMN-15628
Evidence-Source: bd3db80ca5270619d5182da3299f9909e4bc668a
Evidence-Ticket: OMN-15628