Skip to content

feat(OMN-18918): carry the source message coordinates to the projection seam - #3898

Merged
jonahgabriel merged 12 commits into
devfrom
jonah/omn-18918-inject-delivery-coordinates
Sep 21, 2026
Merged

jonahgabriel merged 12 commits into
devfrom
jonah/omn-18918-inject-delivery-coordinates

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

OMN-18918 — step 3 of 4: carry the source coordinates to the projection seam

This is the step where the defect stops. Steps 1 and 2 added a model and a protocol parameter and changed no behaviour. This wires them end to end.

The defect

Every in-process projection writer builds its message metadata from _partition/_offset. The runtime injected neither, so every writer defaulted to (0, 0) and published every snapshot delta at source offset 0. The serving cache refuses a delta whose offset does not exceed the one it already holds for that key, and a constant never exceeds itself — so every delta after the first per key was discarded as a replay. Panels froze at their oldest retained record, at zero consumer lag, behind a green readiness endpoint.

Measured on the .201 dev lane, host 192.168.86.201: a trace subscribed to onex.snapshot.projection.runner-fleet.v1 alone replayed to the end of the topic, position 16,698, and still served rows stamped 06:06:13Z. Full trace on OMN-18905.

What this wires

The consume loop builds the coordinates from the record it already holds — it commits that same offset a few lines above. The engine forwards them only to a dispatcher that declared a keyword-only delivery. The projection seam injects _partition and _offset into the payload the writer reads.

Fails closed in both directions. A consumer that cannot determine the coordinates passes None rather than a zero, because a fabricated coordinate is indistinguishable downstream from a measured one and that indistinguishability is the defect. The seam then injects nothing — today's behaviour preserved bit-for-bit — and logs, scoped to in-process writers, because silence is what made the original defect take a live trace to find.

Three things found while building, each fixed rather than worked around

  1. The context inspector counted 2+ parameters of any kind. Adding a keyword-only delivery would have reclassified a one-positional-parameter dispatcher as context-aware, and the engine would then pass a context positionally into a slot that does not exist. It now counts positional parameters only, with a test pinning it — that failure would have been invisible on the projection path this ticket is about and loud somewhere unrelated.
  2. Context.run takes keyword arguments of its own, so passing delivery= to it directly would be swallowed as one of its kwargs. The sync paths bind it with functools.partial first.
  3. The concrete engine satisfied the widened protocol on dispatch but not on dispatch_with_transaction — mypy caught it as five conformance errors across two files. Both entries now take it, which is what the protocol asked for anyway.

A type-honest narrowing rather than a protocol widening

Core's ProtocolEventMessage, the type the consume loop is written against, declares no delivery coordinates at all. The concrete object the Kafka bus delivers is this repo's own ModelEventMessage, which does. The helper narrows to that model rather than widening a shared protocol in a lower layer to serve one consumer. Anything else — the in-memory bus, a test double — takes the same refusal as a missing coordinate.

Generated surfaces regenerated, not hand-edited

The pin move drifted two, and both gates named their own remedy:

  • The fallback version matrix, regenerated via scripts/update_version_matrix.py.
  • The released-core seam fixture, whose update procedure requires confirming every consumer type-checks first. That confirmation is mypy --strict over all 3120 source files, clean. The consumed surface itself did not drift — only the version line — because 0.47.20 purely adds a model.

Pins: core 0.47.18 → 0.47.20, spi 0.23.3 → 0.23.5, both exact.

What is NOT claimed here

No lab readback yet. The proof that the board cache actually unfreezes requires this to deploy, and it follows on the ticket — two consecutive deltas for one key with strictly increasing source offsets, and the exposure's newest row advancing. I will post it rather than assert it.

What the fixture failures were (added after the first CI run)

The first CI run was red with five integration failures. All five were mine, and none was in the tree I had run locally.

  • Three fixtures froze previous omnibase-core and omnibase-spi pins and failed when the pins advanced to core 0.47.20 and spi 0.23.5; each was advanced following its own file's precedent (the spi sentinel is renamed per its own history, _match_0233_floor -> _match_0235_floor).
  • One assertion checked a pin by substring over raw pyproject.toml text and was satisfied by a comment quoting a retired pin, so it reported green across every pin advance since OMN-14600; it now parses the dependency table. That is a silent gate (S2: fails but reports green), not a fixture refresh, and it is fixed as one.
  • The caller passed the newly added optional keyword-only parameter to every implementation of the protocol, breaking two pre-existing implementations that still have the old signature; it now inspects the implementation class and passes the keyword only when declared.

The third is the one that matters. delivery is optional on ProtocolDispatchEngine precisely so that existing implementors stay valid, and an unconditional keyword at the call site converts that optionality into a TypeError on the first message -- a consumer break dressed as an additive change. The engine already probed its dispatchers this way; the wiring did not probe the engine. _engine_type_accepts_delivery now does: name matched, KEYWORD_ONLY required, cached on the class so nothing inspects a signature per message, and uninspectable refuses.

The RED proof per consumer is tests/integration/nodes/test_node_remote_agent_invoke_effect_integration.py -- both of its tests failed with TypeError: ... got an unexpected keyword argument 'delivery' and pass now. Seven further cases pin the probe's own edges, including the two negatives (a positionally declared delivery, and a **kwargs engine that would not raise but declared nothing) and a positive control on the real engine.

Local verification after the fix: 33 passed across the six touched files. One of the two remote-agent tests failed once in a batch and passed in isolation and in three subsequent identical batches; recorded as a flake in that real-Kafka wiring test rather than a consequence of this change.

Supersedes #3900 (OMN-18955)

#3900 merged to dev while this was in review and fixes the same defect at the same seam by the contextvar route. That is the option the operator declined on 2026-09-20 when they chose the typed protocol parameter, on the ground that partition and offset are facts about a delivery rather than about the event, so the protocol should say what it carries. The team lead ruled (a) on 2026-09-21: C3 stands, and the supersession is folded into this pull request rather than taken as a separate revert, so the lane is never left with zero mechanisms.

Why this could not be left to sort itself out. Merging the two auto-merges clean in handler_wiring.py and leaves both injections live, writing input_data["_partition"] and ["_offset"] about sixty lines apart. The later writer wins, silently. No conflict marker, no failing test, and CI would very likely have been green. Only event_bus_subcontract_wiring.py conflicts visibly.

Removed here: the _CURRENT_SOURCE_COORDINATE channel and its two helpers, the two consume-boundary binds, the contextvar-sourced injection, and the bind at the subcontract call site, which resolves to the typed call.

One correction to the ruling as written, because the premise moved: dispatch_envelope_context.py is not a #3900 file. It was added by OMN-15418 (#2548) and carries the dispatch-envelope and projection-tenant-authority channels that other code depends on. Deleting the module would have taken those with it. Only what #3900 added to it is removed, and a second positive control asserts the surviving channels are untouched, so the deletion is a scalpel rather than a claim that the module is empty.

Kept: #3900's integration test, re-expressed against the typed parameter and keeping all four of its properties (records carry coordinates, offsets advance, a stripped record resolves to None, one record's coordinates cannot reach another). Its measurement is carried into the surviving comment: 6,210,195 lifetime drops on the consumer-flow exposure on the .201 dev lane, with readiness answering 503 because it correctly refused to call a discarding cache healthy. Its unit test is deleted; its subject was the channel that no longer exists.

Added: a guard that fails if more than one source writes either key, if any ambient source-coordinate channel reappears anywhere in the runtime package (matched by substance, not by import name), or if the deletion overreaches. Red-proved by injecting a second assignment and watching it report assert 2 == 1.

Known flake, not introduced here. test_remote_agent_invoke_contract_wires_real_kafka_dispatch failed twice in about seven batch runs and passes in isolation every time. Its RecordingDispatchEngine.received accumulates entries from an earlier test in the same process, so it is a test-isolation weakness in that file rather than a consequence of this change. Recorded rather than papered over.

Sequencing

  1. core model merged, released 0.47.20. 2. spi parameter merged, released 0.23.5. 3. This PR. 4. omnimarket floor raise, which the already-merged reader tolerates in both states.

Releases are cut by core-release-successor-1915 on request; this lane cuts none.

Ticket: OMN-18918. Grandparent: OMN-18905. Epic: OMN-18767.

Evidence-Ticket: OMN-18918
Evidence-Source: OCC#10618

Step 3 of 4, and the step where the defect actually stops. Steps 1 and 2
added a model and a protocol parameter; nothing observable moved. This
wires them end to end.

THE DEFECT

Every in-process projection writer builds its message metadata from
_partition/_offset. The runtime injected neither, so every writer defaulted
to (0, 0) and published every snapshot delta at source offset 0. The
serving cache refuses a delta whose offset does not exceed the one it
already holds for that key, and a constant never exceeds itself, so every
delta after the FIRST for a given key was discarded as a replay. Panels
froze at their oldest retained record, at zero consumer lag, behind a green
readiness endpoint. Measured on the .201 dev lane: a trace subscribed to
the runner-fleet snapshot topic alone replayed to the end of the topic,
position 16,698, and still served rows ten hours old (OMN-18905).

WHAT THIS WIRES

The consume loop builds the coordinates from the record it already holds,
and it commits that same offset a few lines above. The engine forwards them
only to a dispatcher that declared a keyword-only delivery parameter. The
projection seam injects _partition and _offset into the payload the writer
reads.

FAIL CLOSED, in both directions. A consumer that cannot determine the
coordinates passes None rather than a zero, because a fabricated coordinate
is indistinguishable downstream from a measured one, and that is the whole
defect. The seam then injects nothing, preserving today's behaviour
bit-for-bit, and LOGS, scoped to in-process writers, because silence is
what made the original defect take a live trace to find.

THREE THINGS FOUND WHILE BUILDING, each fixed rather than worked around

The context inspector counted 2+ parameters of ANY kind, so adding a
keyword-only delivery would have reclassified a one-positional-parameter
dispatcher as context-aware and made the engine pass a context POSITIONALLY
into a slot that does not exist. It now counts positional parameters only.
A test pins it, because that failure would have been invisible on the
projection path this ticket is about and loud somewhere unrelated.

Context.run takes keyword arguments of its own, so passing delivery to it
directly would be swallowed as one of ITS kwargs. The sync paths bind it
with functools.partial first.

The concrete engine satisfied the widened protocol on dispatch but not on
dispatch_with_transaction, which mypy caught as five conformance errors
across two files. Both entries now take it, which is what the protocol
asked for anyway.

TYPE-HONEST NARROWING

Core's ProtocolEventMessage, the type the consume loop is written against,
declares no delivery coordinates at all. The concrete object the Kafka bus
delivers is this repo's own ModelEventMessage, which does. The helper
narrows to that model rather than widening a shared protocol in a lower
layer for one consumer; anything else, the in-memory bus or a test double,
takes the same refusal as a missing coordinate.

Two generated surfaces moved with the pin and were regenerated rather than
hand-edited: the fallback version matrix, via its own script, and the
released-core seam fixture, whose update procedure requires confirming
every consumer type-checks first. That confirmation is mypy strict mode
over all 3120 source files, clean. The consumed surface itself did not
drift; only the version line did, because 0.47.20 purely adds a model.

Pins: core 0.47.18 to 0.47.20, spi 0.23.3 to 0.23.5, both exact.

Local: 6365 passed, 6 skipped across tests/unit/runtime, including 15 new.
mypy strict mode clean over the whole source tree.

The lab readback that the board cache actually unfreezes follows the
deploy, on the ticket. Step 4 raises the omnimarket floor.
dev moved to 0.38.43 while this branch was building. Enumerated live across
all open heads before choosing: 0.38.43 was the highest claimed and equals
dev, so 0.38.44 is the lowest free number.
@jonahgabriel
jonahgabriel enabled auto-merge (squash) September 21, 2026 03:00
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 21, 2026
…ibase_infra#3898 (#10618)

* evidence(OMN-18918): author OCC companion for OmniNode-ai/omnibase_infra#3898

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 4cf870b59f46d99d89f011ae93ee79d9dc104140.

* evidence(OMN-18918): self-bind OCC#10618 + rebind contract_sha256

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
The Integration Test Coverage gate refused this PR, and it was right to.
Every unit test beside it asserts ONE side of the seam: the consume loop
builds a context, the engine inspects a signature, the seam injects two
keys. All of them can pass while the coordinate still fails to travel,
because none exercises the path between a consumed record and the
dispatcher that reads it.

That gap is the original defect's own shape. OMN-18905 was never a wrong
value anywhere; it was two correct halves that had never been run together.

So this drives a real MessageDispatchEngine through its actual lifecycle --
register a dispatcher, register a route, freeze, dispatch a real envelope
through the public entry -- and asserts what the dispatcher received.

Four cases. A dispatcher that declared the parameter gets the real
coordinates. One that did not is called unchanged even when a delivery IS
supplied, which is the property that made this whole chain landable one
repo at a time. An absent delivery arrives as None rather than a
fabricated zero, because a zero is indistinguishable downstream from a
measured one and that is exactly what froze the board. And a SYNC
dispatcher gets it too: Context.run consumes keyword arguments of its own,
so a naive delivery= there is swallowed, and without this case the async
paths would stay green while every sync dispatcher silently lost its
coordinate.

Writing it caught two things worth keeping in the test rather than in a
comment: the engine refuses dispatch before freeze(), and a dispatcher with
no matching route is never called at all -- which is a way for a coordinate
to "arrive" in a test that proves nothing.
@github-actions

github-actions Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Critical findings: 0
Major findings: 0
Total findings: 0
Models succeeded: glm-review

Note: Fewer than 2 reviewer models succeeded. Degraded results are informational (OMN-8468/OMN-8524) and do not block merge. Error: cli_review exit 2 (fewer than 2 models succeeded — partial/total outage)


Semantics (OMN-17492 — the model finds, thread resolution gates)

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 github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 7
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.

Below quorum: 7 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] tests/unit/runtime/test_omn18918_delivery_coordinates.py (glm-review) — Seam unit tests re-implement the logic instead of exercising it | test_the_seam_injects_the_coordinates_it_is_given and test_an_absent_delivery_injects_nothing copy the injection logic into the test b
  • [MAJOR] tests/integration/test_omn18918_delivery_coordinates_end_to_end.py (glm-review) — No end-to-end coverage of the projection seam's delivery consumption | The new integration test drives MessageDispatchEngine directly with hand-registered dispatchers. It verifies the engine half of t
  • [MAJOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py; src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — ERROR log per message on the in-memory bus path | _delivery_context_from_message returns None for every message that is not omnibase_infra's ModelEventMessage, including everything the in-memory bus d
  • [MINOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — Coordinates injected for all handlers, not only writers, clobbering producer keys | In the projection seam callback, delivery coordinates are written into input_data whenever delivery is not None, reg
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py (glm-review) — Context-detection narrowing silently changes behavior for VAR_POSITIONAL dispatchers | The _dispatcher_accepts_context filter now excludes VAR_POSITIONAL and VAR_KEYWORD parameters. A pre-existing dis
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py (glm-review) — accepts_delivery detection ignores missing default and type | _dispatcher_accepts_delivery matches any KEYWORD_ONLY parameter named delivery, including one declared without a default. The engine alway
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py (glm-review) — int() offset parsing accepts forms that are not broker offsets | int(raw_offset) on a string accepts leading/trailing whitespace, a leading sign already handled, and embedded underscores ('1_6_6' pars

…vance the pin fixtures

The wiring passed `delivery=` to every ProtocolDispatchEngine. The parameter
is OPTIONAL on the protocol, so every engine written before it is still a
valid implementor with a two-parameter `dispatch` -- and an unconditional
keyword turns that optionality into a TypeError on the first message. Two
engines in this repo's own remote-agent integration test failed exactly that
way. The caller now probes the engine CLASS the same way the engine probes
its dispatchers: name matched, keyword-only required, cached per class,
uninspectable refuses.

Also advances the three fixtures that freeze the previous core/spi pins to
core 0.47.20 / spi 0.23.5, following each one's own precedent.

The backmerge identity assertion was a silent gate: `"omnibase-core==0.46.8"
in pyproject` has been satisfied by a COMMENT quoting that retired pin since
the pin moved off 0.46.8, so it reported green across every advance since
OMN-14600. It now reads the parsed dependency table.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 8
Nit-level findings suppressed: 2

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.

Below quorum: 8 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] tests/unit/runtime/test_omn18918_delivery_coordinates.py (glm-review) — Seam injection tests validate a reimplementation, not the code | test_the_seam_injects_the_coordinates_it_is_given and test_an_absent_delivery_injects_nothing do not call any production function. They
  • [MAJOR] tests/integration/test_omn18918_delivery_coordinates_end_to_end.py (glm-review) — No end-to-end coverage of the wiring callback path | The integration test invokes MessageDispatchEngine.dispatch directly with a hand-built delivery kwarg. The code that actually produces the delivery
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py, _delivery_context_from_message (glm-review) — int() coercion of offset accepts lossy and boolean inputs | int(raw_offset) silently truncates a float offset (16698.5 becomes 16698) and accepts bool (True becomes 1). Both produce a fabricated-looki
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py (glm-review) — Engine probe inspects the class, not the instance | _engine_type_accepts_delivery(type(self._dispatch_engine)) assumes the instance's dispatch signature matches the class definition. An instance whose
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py, _dispatcher_accepts_context (glm-review) — Context-detection narrowing lacks a variadic-dispatcher test | The filter in _dispatcher_accepts_context now excludes VAR_POSITIONAL, VAR_KEYWORD, and KEYWORD_ONLY parameters. A dispatcher declared as
  • [MINOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — Refusal logged at ERROR per message invites log flooding | When an in-process projection writer is dispatched without delivery context, an ERROR-level log with a multi-line message is emitted for ever
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py, _delivery_context_from_message (glm-review) — Context topic may diverge from message topic | _delivery_context_from_message takes topic from the subscription argument rather than message.topic. If the two ever differ (wildcard subscriptions, topi
  • [MINOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — No test covers the handler_wiring seam with a delivery present | The handler_wiring change adds injection of _partition/_offset into input_data and the is_inprocess_projection_writer refusal branch. T

… change

runner-image-build-smoke refused the build: the recorded shared_env_digest no
longer matched the recomputed one, because this branch's uv.lock moved.
Regenerated with scripts/ci/runner_image_identity.py --mode generate; only the
two digests change.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 8
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.

Below quorum: 8 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] tests/unit/runtime/test_omn18918_delivery_coordinates.py (glm-review) — Seam tests re-implement production logic instead of exercising it | test_the_seam_injects_the_coordinates_it_is_given and test_an_absent_delivery_injects_nothing in tests/unit/runtime/test_omn18918_de
  • [MAJOR] src/omnibase_infra/runtime/message_dispatch_engine.py:_dispatcher_accepts_context (glm-review) — Context-detection narrowing silently reclassifies existing dispatchers | The change to _dispatcher_accepts_context filters to positional parameters, which fixes the delivery regression but changes cla
  • [MAJOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py:_callback (glm-review) — Missing delivery context degrades to per-message ERROR spam under non-Kafka buses | The in-memory bus and test doubles yield None from _delivery_context_from_message by design. For every in-process pr
  • [MAJOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — Fail-closed refusal does not fail anything observable | The stated goal is that the seam 'refuses loudly rather than inventing a coordinate', yet when delivery is None the code proceeds: the writer de
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py (glm-review) — Probe caches by class only; per-instance dispatch overrides are misclassified | _engine_type_accepts_delivery is lru_cached on type(self._dispatch_engine). An instance that overrides dispatch (assignm
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py:_dispatcher_accepts_delivery (glm-review) — Inconsistent exception sets between the two signature probes | _engine_type_accepts_delivery catches (ValueError, TypeError, AttributeError); _dispatcher_accepts_delivery catches only (ValueError, Typ
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py:_delivery_context_from_message (glm-review) — int(raw_offset) coerces lossy and non-text inputs silently | int() truncates floats (int(16.9) == 16) and accepts booleans (int(True) == 1), producing a fabricated-but-valid coordinate from a malforme
  • [MINOR] tests/integration/test_omn18918_delivery_coordinates_end_to_end.py (glm-review) — No test for the projection seam receiving delivery through the real engine | The integration tests cover MessageDispatchEngine passing delivery to dispatchers and the unit tests cover _delivery_contex

Both tickets fixed the same defect at the same seam, independently. #3900
landed a task-local context channel; this branch carries the typed parameter
the operator ruled for on 2026-09-20. Merging the two auto-merged CLEAN and
left BOTH injections live, writing input_data _partition and _offset about
sixty lines apart. Last writer wins, no conflict marker, no failing test.

Ruling (a): C3 stands, and the supersession is folded in here rather than
taken as a separate revert, so the lane is never left with zero mechanisms.

Removed: the _CURRENT_SOURCE_COORDINATE channel and its two helpers from
dispatch_envelope_context (the module itself predates OMN-18955 and keeps its
envelope and tenant-authority channels untouched), the two consume-boundary
binds in handler_wiring, the contextvar-sourced injection, and the bind at the
subcontract wiring call site, which is resolved to the typed call.

Kept: the OMN-18955 integration test, re-expressed against the typed
parameter - records carry coordinates, offsets advance, a stripped record
resolves to None, one record cannot reach another. Its measurement is carried
into the surviving comment. Its unit test is deleted, its subject being the
channel that no longer exists.

Added: a guard that fails if more than one source writes either key, if any
ambient source-coordinate channel returns anywhere in the runtime package, or
if the deletion overreaches into the channels that legitimately live in that
module. Red-proved by injecting a second assignment.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 7
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.

Below quorum: 7 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [CRITICAL] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py, consume callback branches (glm-review) — Two of three consumer boundaries no longer deliver coordinates at all | The deleted test (test_source_coordinate_injection_omn18955.py) explicitly asserted that handler_wiring binds the source coordin
  • [MAJOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py, _callback (glm-review) — Type of injected _offset silently changes from str to int | The retired injection passed the offset as the transport-declared str (source_coordinate[1], with an explicit comment that narrowing at two
  • [MAJOR] tests/unit/runtime/test_omn18918_delivery_coordinates.py (glm-review) — Two unit tests exercise local re-implementations, not production code | test_the_seam_injects_the_coordinates_it_is_given and test_an_absent_delivery_injects_nothing copy the if/else injection logic i
  • [MINOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py, _callback (glm-review) — Unbounded per-message ERROR logging on the missing-context path | When an in-process projection writer is dispatched without delivery context, _callback logs at ERROR on every message with no rate lim
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py, _dispatcher_accepts_delivery (glm-review) — Inconsistent exception handling between the two signature probes | _engine_type_accepts_delivery catches (ValueError, TypeError, AttributeError); MessageDispatchEngine._dispatcher_accepts_delivery cat
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py (glm-review) — Engine-level probe inspects the class, missing instance-bound dispatch | _engine_type_accepts_delivery(type(self._dispatch_engine)) inspects the class attribute. An engine that binds dispatch per inst
  • [MINOR] src/omnibase_infra/runtime/event_bus_subcontract_wiring.py, _delivery_context_from_message (glm-review) — Delivery context topic taken from subscription, not the record | _delivery_context_from_message(message, topic) stamps the context with the caller-supplied topic argument rather than message.topic. Mo

@jonahgabriel

Copy link
Copy Markdown
Collaborator Author

Second-actor review from the lane that diagnosed OMN-18955 and built the withdrawn C2 arm. The design is better than mine and I am not relitigating it. One blocking question and two smaller notes.

Blocking: the live in-process writers reach a boundary that supplies no delivery

handler_wiring.py in this PR consumes delivery — the dispatcher-callback keyword and the _partition/_offset injection — but never constructs or passes one. Mechanical check over the diff, added lines in that file matching ModelMessageDeliveryContext(, delivery= or _delivery_context_from_message: zero. Only event_bus_subcontract_wiring.py builds and passes a context, and _dispatch_to_contract_scope (handler_wiring.py:6643) is unchanged, still calling dispatch_scoped(topic, envelope, allowed_dispatcher_ids=...) with no delivery.

That matters because of which boundary the five in-process writers actually arrive on. Measured on the .201 dev lane just now, against the running runtime:

WIRING-CALLBACK topics dispatched by EventBusSubcontractWiring (104 calls total):
  36  onex.evt.omnibase-infra.routing-decision.v1
  28  onex.evt.omnibase-infra.quality-gate-result.v1
  28  onex.evt.omnibase-infra.inference-response.v1
  12  onex.cmd.omnibase-infra.delegation-request.v1

No projection source topic appears, including onex.evt.omnibase-infra.runner-fleet.v1, whose exposure is one of the two that was frozen. The five writers carrying onex_runtime_inprocess_dispatch declare db_tables and no consumer_purpose, which puts them on handler_wiring's own _make_event_bus_callback branch rather than the subcontract wiring.

If that holds, then on the dev lane this PR injects nothing for exactly the writers the defect is about, is_inprocess_projection_writer is true, and your new error log fires on every message while the exposures stay frozen. That is a much better failure than the silent one we had — it is loud and correctly worded — but it is not yet the fix.

I could be wrong about the branch: I am reading a subscription log, not your call graph. Please confirm which boundary the runner-fleet and consumer-flow writers dispatch through. If it is handler_wiring, that consume callback needs to build a context from the record it already holds (message.partition / message.offset) and thread it through _dispatch_to_contract_scope into dispatch_scoped. The withdrawn arm bound at three boundaries for this reason, and I think the three-boundary requirement survives the C2-to-C3 change even though the mechanism does not.

A cheap discriminator after this deploys: grep the runtime log for Projection writer dispatched with no delivery context. Silence means I am wrong; a steady stream means the injection is not reaching the writers.

Note: the failing surface is the ready endpoint, not the projections endpoint

Recorded because it is easy to prove a fix on the wrong one. GET /projections on 3002 answered 200 with 64 topics throughout the whole incident. GET /ready answered 503 and never reached 200 inside the 900s budget, which is what timed_out_before_ready fails on. The readback that settles this PR is the ready endpoint.

Note: the drift guard cannot retire its own forward declaration

omnimarket/tests/test_omn16249_projection_injected_key_seam.py harvests injected keys by calling the projection callback directly, with no engine and no consume loop, so it will not observe _partition/_offset however they travel — the typed parameter defaults to None there just as a context variable would have been unbound. PENDING_UPSTREAM_INJECTED_KEYS therefore cannot be deleted on that guard's evidence. OMN-18565 hit the same wall for _tenant_id and fixed it by giving the probe envelope a tenant; the equivalent here is passing a delivery in the harvest.

Baseline for your evidence section, if useful

Pre-fix, .201 dev lane, read twice forty minutes apart:

exposure lag dropped_since_apply dropped_total
consumer-flow 0 130,729 → 152,895 6,232,361
runner-fleet 0 36,087 → 36,708 36,708

Post-deployment of the coordinate change, lane sha 4277d6e9: ready 503 → 200, status not_ready → ready, bus_backed false on two topics → none of 16, lagging_topics 2 → 0. So injecting real coordinates is demonstrably sufficient to unfreeze the exposures; the open question above is only whether this PR's wiring reaches the writers that need them.

Thanks for the direct heads-up earlier rather than letting me find it in a conflict.

…es, not one

Second-actor review caught this before merge: the change supplied a delivery
context at the subcontract seam only, and the in-process projection writers do
not arrive there. They declare db_tables and no consumer_purpose, which routes
them to handler_wiring, whose two callbacks dispatch through dispatch_scoped -
a different protocol, left untouched. A live subscription readback on the .201
dev lane measured the subcontract seam handling 104 calls across four topics,
none of them a projection source.

So the change looked complete, every test passed, and on the lane it would have
injected nothing for exactly the writers the defect is about, logged the absence
on every message, and left the exposures frozen. The retired context-channel arm
bound at three boundaries for this reason; the three-boundary requirement
survives the change of mechanism even though the mechanism does not.

ProtocolContractScopedDispatchEngine is infra-local, so dispatch_scoped takes the
same optional keyword-only delivery here with no further release. Both
handler_wiring callbacks build the context from the record they already hold and
thread it through, on both arms of the raw callback's flow-counter branch,
because a writer needs the coordinates whether or not the lane is counting flow.

The builder and the caller-side probe move to runtime/delivery_context.py. Three
callers, one definition: leaving the builder inside one boundary's module is why
the other two had no access to it. The probe now takes the method name, because
the two protocols name their entry differently and a probe hardcoded to one
would answer False for the other - indistinguishable from a legacy engine, and
silently injecting nothing.

Five integration assertions drive the real engine through the real
_dispatch_to_contract_scope, including a legacy scoped engine that must still be
called unchanged. Red-proved: with the threading disabled, three of the five
fail. Two further guards assert the boundary count and the builder having one
home, so a boundary that stops building a context is a red test rather than a
frozen exposure.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: glm-review
Models failed: codex
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 6
Nit-level findings suppressed: 2

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.

Below quorum: 6 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] tests/unit/runtime/test_omn18918_delivery_coordinates.py (glm-review) — Unit tests re-implement the seam instead of exercising it | test_the_seam_injects_the_coordinates_it_is_given and test_an_absent_delivery_injects_nothing copy the production injection logic into the t
  • [MINOR] src/omnibase_infra/runtime/delivery_context.py (glm-review) — Boolean partition no longer refused by the builder | The deleted bind_source_coordinate explicitly rejected bool partition (isinstance(partition, int) and not isinstance(partition, bool)). The replace
  • [MINOR] src/omnibase_infra/runtime/delivery_context.py (glm-review) — Class-keyed lru_cache ignores instance-level dispatch overrides | engine_type_accepts_delivery resolves the method via getattr on the engine type and caches on (class, method_name). An engine instance
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py (glm-review) — Context-detection narrowing may change behavior for keyword-only legacy dispatchers | _dispatcher_accepts_context previously counted all parameters; the diff restricts it to positional kinds. Any exis
  • [MINOR] src/omnibase_infra/runtime/auto_wiring/handler_wiring.py (glm-review) — No negative test for the in-process writer error-log branch | The new elif is_inprocess_projection_writer logging path is the change's observability contract (the operator-facing signal that coordinat
  • [MINOR] src/omnibase_infra/runtime/message_dispatch_engine.py (glm-review) — dispatch_with_transaction forwards delivery but its callers and protocol are unaddressed | dispatch_with_transaction now accepts and forwards delivery to dispatch, but the diff shows no caller updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant