Repository navigation
fix(OMN-15660): arm the terminal correlation predicate in host mode and stop reading a missing status as success (OMN-17567) - #1645
Merged
github-actions[bot] merged 1 commit intoSep 4, 2026
Conversation
…nd stop reading a missing status as success (OMN-17567)
RuntimeLocal defaults host_handlers=True and no production caller passes
False, so `onex delegate` always runs host mode. Both OMN-17304 remedies
were gated behind `if not self.host_handlers` and therefore never executed
in production: the terminal correlation predicate was never armed and the
terminal consumer group was never run-scoped. On a Kafka-backed lane every
invocation joined one fixed group, inherited the previous run's committed
offset, and adopted whatever terminal the topic still retained.
Observed live 2026-09-02: a delegation returned a 12-minute-old terminal
belonging to a different correlation, in the same second, with no LLM call,
and left its own terminal uncommitted for the next run — a self-perpetuating
ratchet. Only the CLI-side OMN-17295 correlation join caught it.
Second defect (OMN-17567): _on_terminal_event read `payload.get("status",
"success")` at the envelope top level. A ModelEventEnvelope-shaped terminal
carries status under `payload.status`, so a FAILED terminal was classified
COMPLETED — envelope-shaped terminals could never surface failure at all.
Changes:
- arm _expected_correlation_id from the resolved wire correlation id in BOTH
modes; the host/client distinction is gone.
- run-scope the terminal consumer group whenever that predicate is armed.
Gated on armed rather than unconditional: a brand-new group reads the
retained log from the beginning, so scoping without a filter would trade
the uncommitted tail for the whole retention window.
- read terminal status at the envelope AND payload level with the same
precedence _terminal_correlation_matches uses for correlation ids; a
failure declared at either level fails closed; an envelope that declares
no status anywhere is UNKNOWN and is refused, never read as success. Bare
domain terminals with no status keep the offline _classify_result path.
- the OMN-17304 test that pinned "host mode is deliberately unfiltered" is
inverted, with the superseding reason recorded in its docstring.
- the run-scoped group name is added to the OMN-15639 MSK IAM gate, since it
is now the name `onex delegate` actually mints against the broker.
- version 0.47.2 -> 0.47.3 (OMN-13411 release-identity gate).
Tests: tests/unit/runtime/test_runtime_local_terminal_isolation.py — a
retaining broker double with per-group committed offsets (the in-memory bus
dies with the process and cannot express this defect). 12 of its 14 cases
fail on dev; the 2 that pass are the counter-assertions that pin what must
not break.
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Sep 4, 2026
…e_core#1645 (#8192) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_core#1645 * evidence: OCC companion self-bind for #8192 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
github-actions
Bot
deleted the
jonah/omn-15660-terminal-crosstalk-correlation
branch
September 4, 2026 13:16
jonahgabriel
added a commit
that referenced
this pull request
Sep 4, 2026
…push workflows fire (#1646) auto-merge.yml armed 'gh pr merge --auto' with the default workflow token. GitHub completes an armed auto-merge as the identity that armed it, and fires no push event for commits authored by that default token, so every push-on-dev workflow was skipped on every PR that landed through this job. Measured 2026-09-04T15:20-15:25Z over the 30 most recently merged dev PRs (#1613-#1645), via gh api "repos/OmniNode-ai/omnibase_core/actions/runs?head_sha=<full-oid>&event=push" --jq .total_count all 20 bot merges had 0 dev-branch push runs; all 10 jonahgabriel merges had 12-14. The human counts are the positive control that makes those zeros evidence of absence rather than a broken query. Swap the two merge-state-mutating steps to secrets.CROSS_REPO_PAT (existing org secret, visibility=all, already consumed by seven workflows in this repo). The read-only pre-check resolve step stays on the default token: it creates no commit. No fallback expression, which would restore the defect silently. Port of omnibase_infra#3178. Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>
jonahgabriel
added a commit
that referenced
this pull request
Sep 5, 2026
…ogy DB binding + tenant-projection runtime profile (#1648) * feat(OMN-17556): typed store-resolved credential carrier on the topology DB binding + tenant-projection runtime profile A topology database binding now declares exactly ONE credential carrier: `dsn_env` (legacy, a process env var holding the DSN) or `secret_ref` (a dotted logical secret name the runtime resolves through SecretResolver at the binding boundary). Both-set and neither-set are rejected by a model validator that names the principal either way -- two carriers let a stale env var silently shadow the store, and zero carriers must fail at contract-load time rather than at first connect. Neither field is defaulted: a defaulted carrier would reintroduce the silent-fallback class one layer up. Also registers `tenant-projection` in REGISTERED_RUNTIME_PROFILES and in CONSUMER_ATTACHED_RUNTIME_PROFILES. It is a real consumer lane -- one consolidated writer process that owns every TENANT-domain projection contract, because it is the only process holding that binding's store-resolved credential. Membership in the consumer-attached set is a statement of fact about the deployed process (the ONEX runtime booted under a different profile, not a bespoke daemon), not a validator workaround: all eight contracts moving there are subscribing REDUCER/EFFECT archetypes naming ONLY this profile, so omitting it would leave them registered-but-undrained. Why: no onex-dev pod binds the `tenant_projection` DSN, so under ONEX_WIRING_STRICT_MODE=1 eight projection handlers failed auto-wiring and held omninode-runtime/-effects in CrashLoopBackOff (OMN-17519/OMN-17557). Operator ruling 2026-09-03 forbids a credential env var on ANY pod, including dedicated writer pods, so the fix is a store-resolved carrier plus a profile that moves the work to the one process that holds the credential -- not a fifth raw DSN env var. Head of a four-repo chain: omnibase_infra reads this field at the binding boundary and cannot parse the new topology until this releases. No version bump in this commit. The original form of this change carried 0.47.2 -> 0.47.3; on rebase onto dev that hunk collapsed to a no-op because #1645 (OMN-15660) already declared 0.47.3 on dev. The latest published tag and PyPI release are both v0.47.2, and scripts/check_release_identity.py compares project.version against the latest PUBLISHED tag rather than against the previous dev commit, so the 0.47.3 already on dev satisfies it; asserting a second bump would fork the release identity three lanes are coordinating (OMN-16606 / OMN-17841 / OMN-17842). The wheel that will carry this code is therefore 0.47.3, and cutting it is deliberately NOT part of this commit. Tests: tests/unit/constants/test_constants_runtime_profiles_tenant_projection.py, tests/unit/models/core/test_model_deployment_topology_database_binding.py (19 passed). * test(OMN-17556): hoist the binding test's synthetic DSN fixtures behind allowlist pragmas Detect Secrets failed the PR at two use sites in tests/unit/models/core/test_model_deployment_topology_database_binding.py (line 75, Secret Keyword; line 88, Basic Auth Credentials). Both are deliberately-synthetic negative fixtures: a dotted logical secret NAME the model must accept, and DSN-shaped literals the model must REJECT. Neither is a credential. Hoist all four to module-level constants carrying a single `pragma: allowlist secret` each, so the fact is asserted once at the top of the file instead of being repeated at every use site (and silently dropped when a new case is added). Behaviour is unchanged: 19 passed. This unblocks the whole gate cascade on the PR -- Quality Gate failed closed on `secrets returned unrecognised result 'failure'`, Tests Gate then failed closed on the resulting skipped test-parallel, and CI Summary followed. --------- Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>
jonahgabriel
added a commit
that referenced
this pull request
Sep 9, 2026
…ling invocation's handler input (#1671) AC3's handler half and AC2's handler half — the two the terminal-leg fix (#1645) did not touch. runtime_local.py subscribed to the command topic with derive_runtime_local_group_id(entry.handler_name), a group id identical for every invocation of the same handler, so two concurrent invocations shared one group and the broker handed each command to exactly one of them: a run's own work executed in the sibling's process. LocalRuntimeBusAdapter.on_message read correlation_id for logging only, with no conditional between deserialize and invoke, so nothing refused a foreign record either. The subscribe group is now suffixed with the invocation's run_id, and the adapter refuses a message declaring a correlation that is not this invocation's. Both are gated on the correlation predicate being armed, for the reason the terminal group already documents: a brand-new group reads the retained log from the beginning, so scoping without a filter trades the uncommitted tail for the entire retention window. A message declaring NO correlation is still invoked on — the lesson #1655 paid for on the terminal leg, since the canonical def-B message is a bare domain model with no field to carry one. A refusal is not routed through on_error: a sibling's record is not this run's failure, and failing on one would make every concurrent invocation fail the moment a sibling published. RED control (source stashed, tests kept): 9 failed / 14 passed. GREEN: 23 passed. Version bumped to 0.47.7 — packaged source changed on an already-published 0.47.6. Co-authored-by: jonahgabriel <jonah.neugass@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
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-15660 — the terminal isolation the runtime was never actually applying
RuntimeLocal.__init__defaultshost_handlers=True, and no production caller passesFalse— the only occurrences outsideruntime_local.pyare intests/unit/runtime/test_runtime_local_client_mode.py.onex delegatetherefore always runs host mode. Both OMN-17304 remedies sat behindif not self.host_handlers, so neither has ever executed in production:_expected_correlation_id) was never armed, and_terminal_correlation_matchesreturnsTrueunconditionally when unarmed;local.omnibase_core.runtime_local_terminal.consume.v1.__i.<instance>.__t.onex.evt.omnimarket.delegate-skill-completed.v1.On a durable broker that means a run inherits the previous run's committed offset and adopts whatever terminal is still retained.
Observed live, 2026-09-02
On a Kafka-backed single-tenant lane (read-only
rpkprobes, no mutation): the terminal topic held 3 records under 7-day retention with the group at CURRENT-OFFSET 2 / LOG-END 3 / LAG 1. Run61ff0925joined that group, was served the 11:59 terminal for correlation109c2285sitting at offset 0, accepted it with no correlation check, and returnedresult=completedin the same second with no LLM call. Its own terminal arrived 3s later at offset 2 and was never joined — that is the standing lag. The state is self-perpetuating: a run that adopts a stale terminal returns before publishing its own, leaving that one retained-and-uncommitted for the next run.Only the CLI-side OMN-17295 correlation join (
omnibase_infra/src/omnibase_infra/cli/receipt_mode.py:474-500) caught it, because the CLI is the last boundary that still knows which correlation id it minted.Not lane-specific: the same shared group exists on the stability-test lane. Tier-0 (in-memory bus, dies with the process) is immune; any configured Kafka/Redpanda transport is exposed. This is the beta delegation path.
OMN-17567 — a FAILED terminal reported as completed
_on_terminal_eventreadpayload.get("status", "success")against the envelope top level. Two terminal shapes coexist on that one topic: the single-output adapter path publishes the bare domain payload (statusat the top), the fan-out path publishes aModelEventEnvelope(statusunderpayload). An envelope-shaped terminal has no top-levelstatus, so it defaulted to"success"and_classify_resultreturned COMPLETED. Envelope-shaped terminals could never surface a failure through this path at all — and the terminal the probe above adopted wasstatus=failedwithquality_gates_failedpopulated.What changed
src/omnibase_core/runtime/runtime_local.py:_expected_correlation_idis now set from the already-resolved wire correlation id with nohost_handlersguard. This is the actual isolation boundary.Noneonly when nothing correlated reached the wire; client mode already refuses that case, host mode keeps first-terminal-wins there.runtime_local_terminal_run_<run_id[:12]>). Deliberately gated on armed rather than made unconditional: a brand-new group reads the retained log from the beginning, so scoping without a filter would trade "the uncommitted tail" for "the entire retention window" — strictly worse. Group uniqueness alone was never sufficient, which is why OMN-15660 AC3 required both._terminal_status_declarationscollectsstatusfrom the envelope and the domain payload using the same precedence_terminal_correlation_matchesalready uses for correlation ids. A failure declared at either level fails closed. An envelope that declares no status anywhere is UNKNOWN and is refused — symmetric with the existing refusal of an envelope that names no correlation. A bare domain terminal with no status (e.g.{"kind": "completed"}) is the long-standing offline shape and keeps the_classify_resultheuristics; the refusal is scoped to envelope-shaped messages, which structurally carry a domain payload that declares its own status.tests/unit/runtime/test_runtime_local_client_mode.py::test_host_mode_terminal_is_not_correlation_filteredasserted the behaviour this PR removes. It is inverted to..._is_correlation_filtered_too, with the superseding reason in its docstring rather than a silent deletion.tests/gates/test_consumer_group_name_authorization.pynow also derives the run-scoped terminal group name. That is the nameonex delegateactually mints against MSK now, and an unauthorized name here is the OMN-15639 failure mode again (GroupAuthorizationFailedErrorbefore publish). Verified authorized under the managed environment:onex-dev.omnibase_core.runtime_local_terminal_run_<hex>.consume.v1matchesonex-dev.*.pyproject.toml+uv.lock: 0.47.2 -> 0.47.3 (OMN-13411 release-identity gate — packaged source changed).Production callers checked, none relied on first-terminal-wins
grepover omnibase_core / omnibase_infra / omnimarket / omniclaude / omnibase_spi / omnibase_compat: the only productionRuntimeLocal(...)sites areomnibase_infra/src/omnibase_infra/cli/receipt_mode.py:669andomnibase_infra/src/omnibase_infra/cli/cli_node.py:289. Neither passeshost_handlers, andreceipt_mode.pywas compensating for the missing filter with its own correlation join, not depending on its absence. No caller to fix.Tests (RED first)
New:
tests/unit/runtime/test_runtime_local_terminal_isolation.py. It stands up a retaining broker double with per-(group, topic)committed offsets and broker-level subscribers, because the in-memory bus dies with the process and cannot express this defect at all. 12 of the 14 cases fail ondev; the 2 that pass are counter-assertions pinning what must not break.test_host_mode_never_adopts_a_retained_terminal_from_an_earlier_run— the sequential case that actually bit: run A gone, its terminal still retained-and-uncommitted, run B must ignore it.test_host_mode_refuses_a_retained_failed_terminal_from_an_earlier_run— the compounded case; passes only when both fixes are present.test_host_mode_terminal_group_is_run_scoped,test_two_sequential_host_runs_do_not_share_a_terminal_group.test_a_slow_host_run_does_not_adopt_a_concurrent_runs_terminal— OMN-15660's original concurrent framing, on the host path. The handler's think-time is driven off the prompt so the slow run is deterministically parked in its terminal wait when the fast run's terminal lands; without that, two gatheredrun_asynccoroutines run one after the other and the "concurrent" assertion passes vacuously.failed/failure/error/ERRORall classify FAILED; payload-level failure beats an envelope-level success; no-status-anywhere is refused and names the missing status inlast_error.{"kind": "completed"}still COMPLETED.Local proof, re-run on the rebased tree (
da55f8d7, offorigin/dev@d191ffb2) rather than inherited from the earlier lane:The RED was re-proven, not asserted
The claim "12 of the 14 cases fail on
dev" is easy to inherit and wrong to assume, so it was re-run:origin/dev's pre-fixruntime_local.pywas swapped into this tree under a trap that always restores, and the new module was run against it.The two that pass are the counter-assertions, as claimed. The on-point failures are the defect stated in its own words:
The inverted client-mode test was checked the same way — against the pre-fix source
test_runtime_local_client_mode.pyis1 failed, 9 passed, the failure beingtest_host_mode_terminal_is_correlation_filtered_too. That confirms the inversion is a real behavioural flip, not a rename. Restore was verified: zero dirty paths afterwards.Hook status
pre-commit run --files <the 6 changed paths>— the check that actually gates a commit — is RC=0, zero failures.pre-commit run --all-filesreports 19 failed / 98 passed. None of the 19 names any file this PR touches, checked mechanically: grepping the log forruntime/runtime_local.py,test_consumer_group_name_authorization,test_runtime_local_client_mode,test_runtime_local_terminal_isolation,pyproject.tomlanduv.lockreturns 0 hits each. Those 19 are the repo's standing--all-filesbaseline (print-statement, exception-handling, hardcoded-path and TODO-marker scans over pre-existing source). The one hook that was attributable to this change,check-release-identity(OMN-13411), is fixed by the version bump and now passes.Residual, stated rather than hidden
_run_single_handlerstill subscribes its terminal topic with the bare shared group (runtime_local.py:787). That path resolves no correlation id at all, so there is nothing to arm — run-scoping it alone would be the strictly-worse trade described above. Left unchanged deliberately; it is not the delegation path.Version bump: 0.47.3 is a pyproject bump only — no tag, no release cut
scripts/check_release_identity.pyrequiresproject.versionto be strictly greater than thehighest published tag, and the highest published tag is
v0.47.2. Packaged source undersrc/changed here, so the gate demands a bump; without it
check-release-identityis the one hook thatfails and it fails because of this diff.
This PR bumps
pyproject.toml+uv.lockto0.47.3and does nothing else about releases: nogit tag is created, no release is cut, nothing is published. That is deliberate rather than
incidental —
0.47.3is spoken for by OMN-17842 (delegation v2, Task 1.3), whose own step 1 reads"bump the version — reclaiming
0.47.3from the abandoned worktree (fact 25) or taking the nextfree number". This worktree is that abandoned worktree, so landing
0.47.3ondevis thatticket's documented first option, not a collision with it. The release cut remains OMN-17842's to
make.
Evidence-Ticket: OMN-15660
Evidence-Ticket: OMN-17567
Evidence-Source: OCC#8192