Repository navigation
feat(OMN-18116): originate the causal edge and carry it on the wire - #3399
Conversation
The runtime discarded the one fact a chain replay needs. Every event published as a consequence of consuming another was built at a single site that already held the consumed envelope, and threw the parent identity away. A chain read back was a set of hops sharing a correlation id, with no statement of what caused what, so a replay_green verdict could only ever be a claim. Origination is one site: the dispatch-result applier reads the consumed envelope from the context it is already bound to and stamps parent_envelope_id on every event of that dispatch. No handler participates -- the canonical signature is handle(request: ModelX) -> ModelY and never sees an envelope. No consumed envelope means the hop is a chain head, which is a checkable statement, not a gap. A self-edge is refused rather than published, because it would let a broken chain re-derive as a valid one-hop chain. The wire header half is not optional decoration. event_ledger fills its per-hop identity column from the message_id HEADER, and that header was a uuid4 default nobody set from the envelope. Measured read-only on the dev lane over a two-hour window: of 737 ledger rows whose decoded body carried an envelope_id, the column matched the body on 0 and differed on 737. An edge recorded only in the body would be compared against a column holding an unrelated transport id and would fail every hop. So identity and edge are bound from the envelope, in one helper, at all three envelope publish seams -- the Kafka broadcast path, the in-memory bus and the typed publish adapter. A rule that holds on one transport is a rule a verifier cannot rely on. The edge decodes through a new optional-UUID parser rather than the existing one. That one mints a fresh UUID when a header is absent or malformed, which is right for an identity every message must have and catastrophic for an edge: it would invent a parent for a chain head. Absent means absent. No migration. The projection dumps the whole header model into the onex_headers JSONB, so the edge is readable today at onex_headers ->> 'parent_message_id'. A dedicated column is a convenience for the verifier and is deliberately left to land beside the ledger_chain work, whose PR is already re-scoping the forward stream's ordinal pin. Core pin moves to 0.47.8, which carries the envelope field, with the version matrix regenerated from pyproject rather than hand-edited.
The seam test freezes the exact released omnibase-core version infra consumes, so a pin bump that does not move the fixture is a red gate by design. The governed pre-push full suite on the lab host caught it, which is the gate doing its job rather than a surprise. Version string only. Every symbol and field type the fixture pins is unchanged by 0.47.8, whose entire diff is one additive optional field on the event envelope.
…ready Three consecutive governed full-suite runs on the lab host errored in this module -- a varying subset each time, 4 then 3 then 3 -- while the other 27421 tests passed. A set that changes run to run on identical bytes is a clock or a race, not a defect in the code under test, and the traceback names both. The postgres image's entrypoint runs initdb against a TEMPORARY server on a local socket, stops it, then starts the real one. pg_isready answers affirmatively for that temporary server, so the probe could pass while the server the next statement targets did not yet exist. Measured on the lab host: CREATE ROLE role_omnibase_infra returning psql exit 2 immediately after a passing readiness probe, surfacing as an ERROR in setup under check=True rather than the skip this module intends for an unavailable environment. Readiness is now a real SELECT 1 over TCP as the same user and host the tests themselves use. It can only succeed against a server accepting the connections the tests make, which closes the window rather than widening the wait. The module-scoped container bring-up is also charged to the first test's clock, and pulling and starting an image on a loaded host exceeds the suite-wide 60s cap. That module now carries its own timeout. Neither change touches an assertion or a skip condition: an absent Docker daemon still skips, a server that never comes up still skips, and every statement under test still runs under check=True. Unrelated to the causal edge, and on this branch only because it is what the governed pre-push gate blocked on. Never overridden.
…bump The shared CI env digest is computed over the dependency set, so moving the omnibase-core pin to 0.47.8 moves it. The recorded value is regenerated by scripts/ci/runner_image_identity.py rather than hand-edited; the smoke gate that caught the drift is the reason the file is machine-generated.
|
| 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)
![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)
Contextvar-based parent binding is unsafe under concurrent dispatch | The causal edge is read from a contextvar (current_dispatch_envelope) at publish time. Contextvars are copied at task creation, but nothing in the diff shows where bind_dispatch_envelope is entered or exited relative to the dispatch lifecycle. If a dispatcher processes envelopes concurrently on shared tasks, or if a handler awaits across a boundary where the contextvar was rebound (nested dispatch, fan-out workers, task groups), an output
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Test readiness probe may never succeed depending on pg_hba configuration | The replacement probe authenticates over TCP as postgres with PGPASSWORD=scratch. Whether that succeeds depends on the image's pg_hba.conf and POSTGRES_PASSWORD being exactly 'scratch' for the superuser; the diff does not show the container env. If POSTGRES_HOST_AUTH_METHOD=trust is set (common for ephemeral test containers), password auth is ignored or rejected per rule ordering; if the password differs, the probe fails 120 times, w
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
parent_message_id accepted from untrusted wire input without provenance | _kafka_headers_to_model trusts parent_message_id from arbitrary Kafka headers. Any producer on the topic, including a compromised or buggy one, can assert arbitrary causal edges, and the verifier described will treat these as ground truth. Correlation_id and message_id have the same property but were pre-existing; this diff introduces a new forgeable field that downstream chain verification consumes. The malformed-header warning also
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Correlation precedence can desynchronize header correlation from envelope correlation | In the adapter, the splat order means header_identity_fields_from_envelope's correlation_id is always overridden by resolved_correlation_id. When the block above mints a new correlation_id (the stated reason), the wire header's correlation no longer matches the envelope body's correlation_id, while message_id now does match the envelope. The ledger therefore records a hop whose identity and causal edge come from the enve
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Self-edge refusal records a head, converting an anomaly into a plausible chain terminator | When parent_envelope_id equals the deterministic output id, the code logs a warning and publishes with parent_envelope_id=None. The comment asserts this 'reads as a broken chain to a verifier,' but a chain HEAD is precisely the valid terminal the verifier accepts; a uuid5 collision (astronomically improbable) or, more realistically, a deterministic-id derivation bug that repeats the consumed id would be recorded as a
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)
Identity binding applied to three seams, not all seams | The new module's docstring argues against 'repeating the rule at each seam,' yet the diff applies header_identity_fields_from_envelope to only the Kafka adapter, the in-memory bus, and the broadcast mixin. EventBusKafka.publish() itself still mints headers with uuid4 defaults when called directly with a headerless path, and any other publish_envelope-style wrapper is untouched. Non-envelope publishes still mint a message_id unrelated to anything, so t
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
UUID isinstance check silently drops string-typed envelope identifiers | header_identity_fields_from_envelope binds a field only when the value is exactly a UUID instance. Envelopes deserialized from JSON, produced by other runtimes, or constructed via model_construct with string identifiers will carry str values. Those publishes silently fall back to uuid4 minting, recreating the exact defect (header identity differs from envelope identity) with no error and no log. A verifier reading the ledger would see
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Timeout override masks the fixture cost rather than fixing it | Raising the per-test timeout to 600s is attributed to container bring-up charged to the first test's clock. This makes the first test in the module behave differently from the rest and inflates worst-case CI wall time. It also means the readiness loop (120 iterations x up to 30s subprocess timeout) can theoretically consume 600s by itself, leaving no budget for the test body. The empirical reasoning in the comment is reasonable but the fix is a
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Ledger readback test depends on private _normalize_headers behavior | TestLedgerReadsTheEdgeWithoutASchemaChange constructs HandlerLedgerProjection via new and calls the private _normalize_headers. It asserts explicit-null presence in stored headers, but the docstring's claim that readers can distinguish 'recorded as head' from 'pre-edge producer' depends on JSONB serialization emitting the null key rather than dropping it; the test verifies only the Python dict. No test covers the actual projection han
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Mixed-version rollout semantics for parent_message_id unexamined | Older consumers ignore the new header (harmless), but new consumers reading streams produced by pre-change versions will see no parent_message_id and treat every historical hop as a chain HEAD. The test suite explicitly blesses 'present-and-null' as distinguishable from absence, yet no mechanism marks the rollout boundary; a verifier replaying a chain that spans the deployment will see the chain shatter into heads at every pre-upgrade hop. I
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
…ibase_infra#3399 (#8938) * evidence(OMN-18116): author OCC companion for OmniNode-ai/omnibase_infra#3399 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 7989e8380098457e9bc037ac9d6eb9b818249192. * evidence(OMN-18116): self-bind OCC#8938 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
|
Thanks Jonah for merging PR 3399 and getting causal-edge origination onto dev. The follow-up chain-canary run 34497375182 still hit the known runtime ingress timeout and reported terminal_missing (1/5 links proven). I will coordinate with OMN-18116 and will not duplicate that work; the parked replay core still needs to be provided or pushed before continuing. |
|
No OCC evidence companion was minted for this PR. this PR is already bound to OCC#8938; its evidence companion exists and nothing needs authoring To clear this: Nothing to do — the companion already exists. Reported by |
…3399) * feat(OMN-18116): originate the causal edge and carry it on the wire The runtime discarded the one fact a chain replay needs. Every event published as a consequence of consuming another was built at a single site that already held the consumed envelope, and threw the parent identity away. A chain read back was a set of hops sharing a correlation id, with no statement of what caused what, so a replay_green verdict could only ever be a claim. Origination is one site: the dispatch-result applier reads the consumed envelope from the context it is already bound to and stamps parent_envelope_id on every event of that dispatch. No handler participates -- the canonical signature is handle(request: ModelX) -> ModelY and never sees an envelope. No consumed envelope means the hop is a chain head, which is a checkable statement, not a gap. A self-edge is refused rather than published, because it would let a broken chain re-derive as a valid one-hop chain. The wire header half is not optional decoration. event_ledger fills its per-hop identity column from the message_id HEADER, and that header was a uuid4 default nobody set from the envelope. Measured read-only on the dev lane over a two-hour window: of 737 ledger rows whose decoded body carried an envelope_id, the column matched the body on 0 and differed on 737. An edge recorded only in the body would be compared against a column holding an unrelated transport id and would fail every hop. So identity and edge are bound from the envelope, in one helper, at all three envelope publish seams -- the Kafka broadcast path, the in-memory bus and the typed publish adapter. A rule that holds on one transport is a rule a verifier cannot rely on. The edge decodes through a new optional-UUID parser rather than the existing one. That one mints a fresh UUID when a header is absent or malformed, which is right for an identity every message must have and catastrophic for an edge: it would invent a parent for a chain head. Absent means absent. No migration. The projection dumps the whole header model into the onex_headers JSONB, so the edge is readable today at onex_headers ->> 'parent_message_id'. A dedicated column is a convenience for the verifier and is deliberately left to land beside the ledger_chain work, whose PR is already re-scoping the forward stream's ordinal pin. Core pin moves to 0.47.8, which carries the envelope field, with the version matrix regenerated from pyproject rather than hand-edited. * fix(OMN-18116): move the released-core seam fixture with the pin The seam test freezes the exact released omnibase-core version infra consumes, so a pin bump that does not move the fixture is a red gate by design. The governed pre-push full suite on the lab host caught it, which is the gate doing its job rather than a surprise. Version string only. Every symbol and field type the fixture pins is unchanged by 0.47.8, whose entire diff is one additive optional field on the event envelope. * fix(OMN-18116): prove postgres readiness with a real query, not pg_isready Three consecutive governed full-suite runs on the lab host errored in this module -- a varying subset each time, 4 then 3 then 3 -- while the other 27421 tests passed. A set that changes run to run on identical bytes is a clock or a race, not a defect in the code under test, and the traceback names both. The postgres image's entrypoint runs initdb against a TEMPORARY server on a local socket, stops it, then starts the real one. pg_isready answers affirmatively for that temporary server, so the probe could pass while the server the next statement targets did not yet exist. Measured on the lab host: CREATE ROLE role_omnibase_infra returning psql exit 2 immediately after a passing readiness probe, surfacing as an ERROR in setup under check=True rather than the skip this module intends for an unavailable environment. Readiness is now a real SELECT 1 over TCP as the same user and host the tests themselves use. It can only succeed against a server accepting the connections the tests make, which closes the window rather than widening the wait. The module-scoped container bring-up is also charged to the first test's clock, and pulling and starting an image on a loaded host exceeds the suite-wide 60s cap. That module now carries its own timeout. Neither change touches an assertion or a skip condition: an absent Docker daemon still skips, a server that never comes up still skips, and every statement under test still runs under check=True. Unrelated to the causal edge, and on this branch only because it is what the governed pre-push gate blocked on. Never overridden. * chore(OMN-18116): restamp the runner image identity for the core pin bump The shared CI env digest is computed over the dependency set, so moving the omnibase-core pin to 0.47.8 moves it. The recorded value is regenerated by scripts/ci/runner_image_identity.py rather than hand-edited; the smoke gate that caught the drift is the reason the file is machine-generated.
OMN-18116 — originate the causal edge and carry it on the wire
Core released the field (0.47.8,
parent_envelope_idonModelEventEnvelope). This is the half that fills it in.Approved design:
beta/plans/2026-09-10-causal-edge-origination-proposal.md(option A), merged at squashe91826e7ba023ac9e9b34fcd603066f717d8902e. Blocks OMN-16964 link 5.Origination is one site
The dispatch-result applier already builds every outgoing envelope and already has the consumed envelope bound to the dispatch on a context variable. It read neither. Now it reads the consumed envelope once per dispatch and stamps the parent on every event that dispatch emits.
handle(request: ModelX) -> ModelYand never sees an envelope, so origination belongs in the runtime adapter. No node contract changes.The wire header half is not optional
event_ledgerfills its per-hop identity column from themessage_idheader, not from the envelope body, and that header was auuid4default nobody set from the envelope. Measured read-only on the dev lane over a two-hour window:envelope_idcorrelation_idcolumn agrees with bodyAn edge recorded only in the body would be compared against a column holding an unrelated transport identity and would fail every hop.
correlation_idagrees only because OMN-14962 explicitly forced that agreement, for correlation and nothing else.So identity and edge are now bound from the envelope, in one helper, at all three envelope publish seams: the Kafka broadcast path, the in-memory bus, and the typed publish adapter. A rule that holds on one transport is a rule a verifier cannot rely on. A payload that is not an envelope contributes nothing and keeps the previous behaviour.
The edge decodes through a new optional-UUID parser rather than the existing one. That one mints a fresh UUID when a header is absent or malformed, which is right for an identity every message must have and catastrophic for an edge: it would invent a parent for a chain head. Absent means absent, and a malformed value logs and records no edge.
No migration, deliberately
The ledger projection dumps the whole header model into the
onex_headersJSONB, so the edge is readable today atonex_headers ->> 'parent_message_id', with a chain head recorded as present-and-null rather than missing. A test pins both.A dedicated
event_ledger.parent_envelope_idcolumn is a convenience for the verifier, not a precondition. It was built and then removed from this change: the forward stream's ordinal pin is currently being re-scoped by the ledger_chain work, and a second migration against the same pin would collide with a peer branch for no gain this change needs. It lands beside that work.Verification
tests/unit/runtime/test_causal_edge_origination.py(new, 13 tests)tests/unit/event_bus/plus both applier suitesEvery positive assertion has a negative control beside it: a chain head records no edge, a non-envelope payload contributes no identity, a malformed edge header records nothing while the identity header on the same message still resolves.
Two fixes the gate found, both on this branch and neither a bypass
pg_isreadyanswers for that one, and the next statement then hit exit 2 against a server that did not yet exist. Readiness is now a realSELECT 1over TCP as the same user and host the tests use, and the module carries its own timeout because container bring-up is charged to the first test's clock. No assertion and no skip condition changed. Unrelated to the causal edge, and on this branch only because it is what the gate blocked on.No override of any kind was used at any point. No lane was deployed, restarted or rebuilt; all dev-lane access was read-only.
dod_evidence
event_ledger, with the positive and negative controls stated above.Evidence-Ticket: OMN-18116
Evidence-Source: OCC#8938