Repository navigation
fix(OMN-16249): match the projection injected-key seam across the infra/market boundary - #2109
Merged
github-actions[bot] merged 1 commit intoAug 19, 2026
Conversation
The gateway does not inject `_envelope_id`. Its wire envelope names that field `envelope_id`, no underscore (onex-api models/model_workflow_envelope.py:222, to_canonical_wire_envelope). The sole producer of the underscore key is omnibase_infra's projection dispatch arm, handler_wiring._make_projection_dispatch_callback, which injects `_db`/`_event_type`/`_topic`/`_envelope_id` into the plain dict it hands handle(). That is a platform convention, not a defect. The defect is that the convention is enumerated independently on both sides of a repo boundary: the producer grew a fourth key, and this repo's canonical stripper still enumerated three. `_envelope_id` therefore survived into the payload and failed extra_forbidden against ModelHookEventCaptureRequest -- 202 accepted, DLQ'd, zero rows. It hid because the producer injects the key only when the envelope carries a coercible envelope_id. Reducers fed by internal events never saw it; gateway-published envelopes always carry one, so node_hook_event_capture failed on its first real message. Widens RUNTIME_INJECTED_KEYS to cover it and returns it in injected_meta rather than dropping it, preserving the idempotency-key capability the producer injects it for. The durable half is the drift guard: tests harvest the key set by EXECUTING the installed producer and fail closed when this repo's set does not cover it, so the next upstream key lands as a red test here with no cross-repo release coordination needed to detect it. Refs OMN-16249.
|
Important Review skippedThis review includes 2 billable files. This on-demand review is free during your promotion. Your included review limit has been reached. Run
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
You can disable this status message by setting the Comment |
✅ Architectural Review — PASSEDErrors: 0 What this checks
No architectural violations found. Static architectural lint — no model inference (OMN-14176). |
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
jonahgabriel
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 19, 2026
…6725) * evidence(OMN-16249): OCC companion for OmniNode-ai/omnimarket#2109 OCC companion contract + receipts for OmniNode-ai/omnimarket PR #2109 (matches the projection injected-key seam across the infra/market boundary). No OCC companion or contract existed for this ticket. Net-new contract `contracts/OMN-16249.yaml` + 4 PASS receipts: - dod-OmniNode-ai-omnimarket-pr-2109 -- live PR existence/head/scope probe - dod-omn16249-producer-injected-keys -- omnibase_infra's handler_wiring.py at the pinned rev injects exactly the 4 documented keys (_db, _event_type, _topic, _envelope_id), verified via the GitHub contents API against the immutable ref - dod-omn16249-consumer-widened-key-set -- omnimarket's handler_shim.py at the PR head declares RUNTIME_INJECTED_KEYS as the matching 4-key frozenset - dod-omn16249-fail-closed-drift-guard-and-tests -- differential proof, live-executed on the .200 gate host: the seam regression suite (19 passed) and an independent live introspection of the installed omnibase_infra module reproducing the same 4-key set Self-bind entry to follow in a second commit once this PR's number is known (established pattern -- see OCC#6677/OMN-16170, OCC#6702/OMN-16089). Cites: OMN-16249 * evidence(OMN-16249): self-bind OCC companion PR #6723 Appends occ-self-bind-pr-6723 to contracts/OMN-16249.yaml + its PASS receipt, binding this companion to its own OCC PR number now that it is known. The four prior dod_evidence entries are unchanged (append-only, per-entry hash verified stable). Cites: OMN-16249 * evidence(OMN-16249): correct fabricated commit_sha in the self-bind receipt The self-bind receipt recorded commit_sha 3f35bfa939ddb6da... which does not exist in this repository — 'git cat-file -t' fails on it. It shares only the first 10 hex characters with the real companion commit (3f35bfa...), a prefix match that cannot occur by chance, so the value was a real short prefix with an invented remainder rather than a stale or mis-copied SHA. A receipt asserting PASS against a commit that does not exist is exactly the class of unverifiable evidence receipts exist to prevent. Corrected to the actual parent commit and re-verified with git cat-file. The other four receipts' cross-repo SHAs were audited and are genuine: 8b66eaa3... is omnimarket#2109's real head, and 6fdbba6d... is a real omnibase_infra commit (the OMN-14498 #2672 merge). Refs OMN-16249. * evidence(OMN-16249): rebind self-bind receipt to OCC#6725 PR #6723's CI run never materialized into jobs — status pending, zero jobs, updated_at frozen at creation, reproduced across three head SHAs and four re-trigger attempts while other OCC PRs expanded 30 jobs normally. The stall was bound to that PR's identity, so this branch is opened as a fresh PR rather than retried a fifth time. Rebinds the self-bind evidence item and receipt from #6723 to #6725 and recomputes contract_entry_sha256 for the changed entry. The probe was re-executed against the new PR, not edited: 'gh pr view 6725 --json number,state' returns {"number":6725,"state":"OPEN"} verbatim. All five receipts re-verified against their contract entries with omnibase_core compute_contract_entry_sha256 — all match. Refs OMN-16249. * evidence(OMN-16249): fix the two Contract Compliance findings on #6725 Contract Compliance Check reported 3/5 PASS, 1 WARN, 1 BLOCK. Both findings are this companion's own content, not repo-wide noise, so both are fixed here rather than waived. BLOCK -- dod-omn16249-producer-injected-keys. The probe fetched handler_wiring.py through the contents API's default JSON+base64 representation and died mid-transfer: "stream error: stream ID 1; CANCEL; received from peer" after ~15s. That file is 371,665 bytes, which base64 inflates to ~495KB wrapped in a JSON string. Switched to "Accept: application/vnd.github.raw", which streams the file directly and completes in well under a second. Re-run verbatim: exit 0, output 4. The match pattern now anchors on the assignment operator so it counts injection SITES rather than incidental mentions of the same keys. WARN -- occ-self-bind-pr-6725. The checker classified the probe INERT: "no admissible probe in command position ... proves nothing", because a bare gh pr view is not one of the admissible probe verbs. Switched to gh api, which is admissible and asserts the same fact against the live API. Re-run verbatim: exit 0, output "6725 open". Both probes were re-executed by this lane, not edited to match a desired result, and the runner field now names the lane that actually ran them. All five contract_entry_sha256 values recomputed and verified; validate-yaml passes. The producer description is a literal block scalar, not folded: the OMN-15479 yamlfmt-contamination gate correctly rejected the folded form once the description carried an internal newline. The remaining reds (no-noncanonical-lifecycle-classes, reason-graph, tests+coverage shadow) sit outside CI Summary's gating set and are shared with the unrelated #6722, so they are repo-wide. Refs OMN-16249.
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
1 participant
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.
Summary
Fixes OMN-16249. The live canary on onex-dev submitted a real batch through the deployed gateway (
POST /v1/workflows,workflow_type: hook-event-capture), got HTTP 202, the consumer received the message — andModelHookEventCaptureRequestthen rejected it with a pydanticextra_forbiddenon_envelope_id. DLQ'd, zero rows, nothing in the 202 telling the caller.Adjudication — both of the ticket's hypotheses are wrong
The ticket asked whether this is a gateway defect or a wiring bug local to
hook-event-capture. It is neither.The gateway does not inject
_envelope_id.docker/onex-api/contains zero occurrences of that token. Its wire envelope names the fieldenvelope_id, no leading underscore, built into_canonical_wire_envelope(omninode_infra docker/onex-api/models/model_workflow_envelope.py:222) and published fromrouters/workflows.py:475.The sole producer of the underscore key is the runtime, consumer-side, after deserialization.
omnibase_infra's projection dispatch arm injects four bookkeeping keys into the plain dict it handshandle():_dbProjectionDatabaseOperationshandler_wiring.py:3087_event_typestrhandler_wiring.py:3088_topicstrhandler_wiring.py:3089_envelope_iduuid.UUIDhandler_wiring.py:3091-3095_extract_projection_envelope_id()coerces a UUIDThat is a platform convention, not a defect —
handler_wiring.py:3092-3094documents_envelope_idas a durable idempotency key for redeliveries.The actual defect is that the convention is enumerated independently on both sides of a repo boundary.
node_hook_event_capturedeclaresdb_io.db_tables, sowire_from_manifestroutes it through that arm, and its handler correctly strips injected keys via this repo's canonical shim. Butsplit_projection_input's_RUNTIME_INJECTED_KEYSenumerated three keys while the producer injects four._envelope_idsurvived intomodel_payloadand detonated againstextra="forbid".Why it hid. Injection is conditional. Reducers fed by internal events without envelope identity never saw the key, so all six
split_projection_inputcallers looked healthy. Gateway-published envelopes always carry one, so the first node on that path failed on its first real message. This was latent for every strict-model projection handler in this repo, not specific tohook-event-capture.None of the three forbidden shapes were used: the request model is unchanged, no
extratolerance was added anywhere, and there is no gateway special-case.Seams matched field-by-field (OMN-14208)
Seam 1 — runtime → projection
handle()argument. Positionaldict[str, object]. Producer key set as tabled above; consumer contract isRUNTIME_INJECTED_KEYSinsrc/omnimarket/projection/handler_shim.py, which must be equal to it — a superset silently eats legitimate underscore-aliased domain fields (_runtime_backend, OMN-13825), a subset is this bug. Both directions are asserted.Seam 2 —
split_projection_inputreturn triple.(db: DatabaseAdapter, model_payload: dict, injected_meta: dict).model_payload= input minusRUNTIME_INJECTED_KEYS, safe to splat into a strict model.injected_meta=RUNTIME_INJECTED_KEYS - {_db}, now carrying_envelope_idso reducers keep the idempotency-key capability the producer injects it for rather than having it dropped on the floor.Seam 3 — batch payload →
ModelHookEventCaptureRequest(frozen,extra="forbid"):source,batch_sha,events[],correlation_id,emitted_at,tenant_id,tenant_principal_id. Unchanged by this PR; the test fixture is built field-for-field against it.The durable half: a fail-closed drift guard
Widening a frozenset fixes today's break and nothing else — the seam would drift again the next time the producer grows a key.
tests/test_omn16249_projection_injected_key_seam.pytherefore executes the installedomnibase_infradispatch callback and harvests what it actually injects, rather than restating the key set. The next upstream key lands as a red test in this repo's own CI, with no cross-repo release coordination needed to detect it. A static cross-check fails closed if an upstream refactor ever makes the harvest stop observing the injection path, so a shape change is loud rather than a vacuous pass.Test plan
Cross-boundary, driving the real seam — the producer side is the executed
omnibase_infracallback, the consumer side is the real handler, real shim, and realextra="forbid"model. Not two independent doubles.1 validation error for ModelHookEventCaptureRequest / _envelope_id / Extra inputs are not permitted [type=extra_forbidden].tests/test_omn16249_projection_injected_key_seam.py+tests/test_projection_handler_shim_omn13825.py— 19 passed on the.200gate host..200— 1590 passed, 30 skipped, 0 failed.mypy src/omnimarket/ --strict— clean, 3028 source files.ruff format --check+ruff checkclean; full pre-commit hook chain passed on commit.One test defect was caught by running on the clean gate host rather than this workstation: the harvest originally inherited an ambient
OMNIDASH_ANALYTICS_DB_URL, so it passed on a developer Mac and failed on.200where the binding DSNs are unset. The suite now sets them explicitly, and was re-verified locally with those vars cleared.Refs OMN-16249.
Evidence-Ticket: OMN-16249
Evidence-Source: OCC#6725