Repository navigation
fix(OMN-17985): refuse an unregistered RUNTIME_PROFILE instead of silently falling back - #3238
Conversation
…ently falling back RUNTIME_PROFILE was read twice by two code paths that disagreed about what it resolved to. load_runtime_profile fell back to the "default" profile with a warning, discarding the role identity the value carries. The auto-wiring ownership filter never consulted that resolved profile at all -- it read the raw env string. An unregistered value matches no contract's declared runtime_profiles list AND is != "main", so every contract is skipped: the manifest empties, zero subscriptions are wired, and the process still passes readiness. The only signal was one warning from a different function. - load_runtime_profile now raises ProtocolConfigurationError on an unknown name, naming the value and listing the known profiles. Unset/blank still resolves to "default"; that is "unset", not "unknown". - resolve_runtime_profile_name() is the single validated read for ownership decisions. Its unset fallback is "main", exactly the value the ownership filter already used -- consolidating adds validation, it does not move a default. Twelve raw reads across service_kernel and runtime_host_process now go through it, including the boot banner, which defaulted to "default" while every ownership read defaulted to "main" and so could name a role the process was not wiring as. - Dockerfile.runtime shipped RUNTIME_PROFILE=all as an image default: a stale service-selection value from an older meaning of this variable. Any container from that image not overriding it wired zero contracts. Now "main". _current_runtime_profile_name in runtime_host_process deliberately keeps its own "default" fallback -- it matches operator-written enabled_profiles lists, so moving that fallback would be a gating change, not a validation change. It is safe because an unregistered value can no longer reach a booted runtime. RED first: the new test module failed at import (no resolve_runtime_profile_name) and the pre-existing test_unknown_runtime_profile_still_falls_back_to_default is inverted here -- the behaviour it pinned is the defect.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 5
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[MAJOR] hostile-reviewer (glm-review)
Raw read in _current_runtime_profile_name undermines the fail-closed invariant | The comment on _current_runtime_profile_name asserts safety because 'an unregistered value can no longer reach a booted runtime, because load_runtime_profile refuses it during kernel boot'. That guarantee only holds if every boot path calls load_runtime_profile. RuntimeHostProcess.main() and ServiceKernel may not be the only entrypoints; test harnesses, scripts, or alternate kernels that construct wiring helpers without going t
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
The two readers still disagree on the unset fallback, contradicting the stated goal | The diff's stated purpose is that both readers of RUNTIME_PROFILE resolve the value through one validated function so they 'must not disagree about one variable again'. They still disagree: resolve_runtime_profile_name() maps unset/blank to "main" while load_runtime_profile() maps unset/blank to "default". The new test test_agrees_with_load_runtime_profile_on_every_known_name only exercises explicitly set, registered value
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
resolve_runtime_profile_name can now raise inside long-lived async loops | The replacement of os.getenv reads with resolve_runtime_profile_name() converts previously infallible reads into raising calls at call sites inside background loops (_savings_correlation_loop, _wiring_health_loop) and message handlers (_triage_on_message). If the environment variable is somehow absent at these call sites but the process booted via a path that never validated (see prior finding), or if a container re-exec's these code
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Dockerfile default flip from 'all' to 'main' silently enables baseline subscriptions for existing default deployments | The image default changes from RUNTIME_PROFILE=all to RUNTIME_PROFILE=main. Under the old default, _baseline_subscription_wiring_disabled returned True (non-main), so containers that never overrode the variable ran no baseline contract-registry subscriptions. The new default flips that gate: any lane that inherited the image default and was (correctly or accidentally) relying on no baselin
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
No test that boot actually refuses an unregistered profile end to end | The new fail-closed tests exercise load_runtime_profile and resolve_runtime_profile_name in isolation. There is no test that RuntimeHostProcess.main() or ServiceKernel boot propagates ProtocolConfigurationError into a non-Ready state (nonzero exit, readiness gate) rather than crashing with a traceback in a partially initialized state or being caught by an over-broad exception handler upstream. The ticket's central claim ('raises during
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
test_core_registry_names_are_all_bootable does not cover whitespace/case forms of registered names | The positive control iterates REGISTERED_RUNTIME_PROFILES verbatim, but the normalization path (strip/lower) is exercised only by the single hardcoded ' MAIN ' case. If REGISTERED_RUNTIME_PROFILES contains mixed-case entries and _PROFILES keys were ever populated without normalization, the loop would pass while real env values normalized to a mismatched key and were refused. Low likelihood given _resolve_p
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
#8379) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3238 * evidence: OCC companion self-bind for #8379 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
…5 registered (#3253) * feat(OMN-17985): boot the seven projection-writer profiles core 0.47.5 registered omnibase_core 0.47.5 admits projection-writer-{delegation,hook-ledger, live-events,registration,savings,tenant-credentials,tenant-registry} into REGISTERED_RUNTIME_PROFILES and CONSUMER_ATTACHED_RUNTIME_PROFILES. This repo owns the behaviour half, and two guards make the pairing mandatory: - test_profiles_match_core_registry asserts frozenset(_PROFILES) equals the core registry exactly. - test_consumer_attached_profiles_actually_load requires every consumer-attached name to resolve through load_runtime_profile. Since #3238 made that function REFUSE an unknown name, a name core blesses that this dict lacks is now a hard boot failure rather than a silent fallback to "default". RED proves it: bumping the core pin to 0.47.5 with _PROFILES unchanged failed both guards, the second with ProtocolConfigurationError on 'projection-writer-live-events' -- the drift the parity guard exists to catch, now loud instead of silent. prefetch_policy stays "disabled" on all seven, like every other role-based profile. No lane's behaviour changes: these names are already the deployed RUNTIME_PROFILE values on onex-dev, and the processes carrying them are standalone BaseProjectionRunner runners that never read the variable. * chore(OMN-17985): rebind the three surfaces that mirror the core pin Every commit that moves pyproject.toml or uv.lock rebinds these, per the same companion edit #3242 made for the 0.47.3 -> 0.47.4 move: the seam fixture that freezes the released core version, the version_compatibility min_version floor, and the runner image identity/shared-env digests (regenerated with scripts/ci/runner_image_identity.py --mode generate, then verified). The governed pre-push caught the first two as real reds on h201.2; the third was found by running the verifier rather than waiting for CI to say so.
1 participant
OMN-17985 — the unregistered-
RUNTIME_PROFILEfallback now fails closedRUNTIME_PROFILEwas read twice, by two code paths that disagreed about what it resolved to.load_runtime_profilefell back to thedefaultprofile with alogger.warning. The resolvedprofile.namebecamedefault, so the ROLE IDENTITY the value carries was silently discarded.filter_manifest_for_runtime_profile.profile_ownership.pykeeps a contract only when the profile is named in the contract'sruntime_profileslist, or (for undeclared contracts) when the profile ismain. An unregistered string satisfies neither, so every contract is skipped: the manifest empties, zero subscriptions are wired, and the process still passes readiness. The only signal was one warning emitted from a different function. That is the OMN-12950 orphan mechanism, and a fallback is the wrong shape for it — a role the runtime cannot resolve is not a milder role, it is an unknown deployment.What changed
load_runtime_profilerefuses an unknown name — raisesProtocolConfigurationErrornaming the value and listing the known profiles, so an operator can fix it from the error alone. Unset or blank still resolves todefault; that is unset, not unknown.resolve_runtime_profile_name()is the single validated read for ownership decisions. Its unset fallback ismain— exactly the value the ownership filter already used, so consolidating adds validation and does not move a default. Twelve raw reads acrossservice_kernel.pyandruntime_host_process.pynow go through it.defaultwhile every ownership read defaulted tomain, so the process could name one role in its banner while wiring as another. It now reports the resolved identity.docker/Dockerfile.runtimeshippedRUNTIME_PROFILE=allas an image-level default — a stale service-selection value from an older meaning of this variable, documented in the same file asall | minimal | kafka | secrets. No registry knows it, so any container built from this image that did not override it matched no contract's list and was!= main: it wired zero contracts and stayed Ready. Nowmain. Every deployed lane (docker-compose.infra.yml,.judge.yml,.lakshman.yml, all three k8s clusters) overrides the value explicitly, so this changes only the un-overridden case, which was broken._current_runtime_profile_nameinruntime_host_process.pydeliberately keeps its owndefaultfallback and is documented in place: it is compared against operator-writtenenabled_profileslists, so moving that fallback would be a gating change rather than a validation change. It is safe to leave because an unregistered value can no longer reach a booted runtime.RED then GREEN
tests/unit/runtime/test_runtime_profile_fail_closed.pyis new and failed at import before the change (noresolve_runtime_profile_name); it now passes 31 assertions covering the refusal, the unset/blank paths, normalization, and that both readers agree on every known name. It carries a positive control (test_core_registry_names_are_all_bootable) so a gate that refused everything could not read as a pass.tests/unit/runtime/test_runtime_profile.py::test_unknown_runtime_profile_still_falls_back_to_defaultis inverted here intotest_unknown_runtime_profile_is_refused. The behaviour it pinned is the defect.Verification
uv run pytest tests/unit/runtime/test_runtime_profile.py tests/unit/runtime/test_runtime_profile_fail_closed.py tests/integration/runtime/test_runtime_profile_registry_integration.py tests/unit/runtime/test_check_env_reads_gate.py tests/unit/runtime/test_service_kernel_introspection_identity_wiring_omn10856.py tests/ci/test_lane_projection_writer_coverage_omn17562.py=> 97 passed, 1 skipped.uv run pytest tests/unit/runtime/test_kernel.py tests/integration/test_writer_owned_projection_wiring_omn17519.py tests/integration/test_runtime_health_monitor_profile_filter.py tests/unit/runtime/test_pr_state_cross_profile_intent_routing.py=> 65 passed, 2 skipped.uv run ruff format/ruff check/mypy --stricton all three changed modules => clean.pre-commit run --files ...on the six changed paths => all pass.test_policy_registry_performance.py::test_registration_throughput_regression, a 500 ms wall-clock threshold) is a load-sensitive benchmark in a module with prior flake history; it passes 3/3 locally in 0.36–0.57 s and shares no code with anything this PR touches. No override was minted — the push was simply re-run.Scope note
This is the RUNTIME half. The registry half (registering the seven
projection-writer-*names that already ship ononex-dev) is anomnibase_corechange, and the manifest half is a gate inomninode_infra. Adding those names to_PROFILEShere has to wait for the core release, becausetest_profiles_match_core_registryasserts exact equality with the core registry.DoD evidence: OMN-17985, acceptance criterion 3 (the two readers now resolve one validated value) and criterion 4 (red-then-green).
Evidence-Ticket: OMN-17985
Evidence-Source: OCC#8379