Repository navigation
feat(OMN-18901): publish the DoD verify verdict in a shape the projection accepts - #2779
Merged
jonahgabriel merged 10 commits intoSep 25, 2026
Merged
Conversation
…tion accepts Onex-Lane: jev-dod-producer-2 Onex-Session: e0ac582ced0a487f860f431365ab85f4
# Conflicts: # pyproject.toml # uv.lock
|
| Rule | Description |
|---|---|
| ARCH-TOPIC-001 | No hardcoded Kafka topic strings in handler code |
| ARCH-DI-001 | No event_bus=None bypass |
| ARCH-DI-002 | No direct Handler instantiation outside workflow_runner/adapters |
| ARCH-DI-003 | No reinvented DI containers |
| ARCH-TOPIC-002 | contract.yaml topics follow `onex.{cmd |
ARCH-DI-002: Direct Handler instantiation (warning)\n\n\nsrc/omnimarket/nodes/node_projection_dod_verdict/handlers/handler_dod_verdict_runner.py:150: self._fold = HandlerProjectionDodVerdict()\n\n\nHandler classes should be wired only in workflow_runner.py or adapter modules. Direct instantiation elsewhere bypasses the contract system.\n\n
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
marked this pull request as ready for review
September 22, 2026 19:21
Onex-Lane: land-3959-1330 Onex-Session: 825bf9dc4dcf44d4ba2d44657d592eab
onexbot-occ-writer Bot
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Sep 22, 2026
…nimarket#2779 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/35773290336
onexbot-occ-writer Bot
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Sep 22, 2026
…nimarket#2779 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/35773416475
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Sep 22, 2026
…imarket#2779 (#10823) * evidence(OMN-18901): author OCC companion for OmniNode-ai/omnimarket#2779 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head e573a3dd7d2ba824ac4e302714e7894331d6916e. * evidence(OMN-18901): self-bind OCC#10823 + rebind contract_sha256 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#2779 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/35773290336 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#2779 The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited. Ticket: OMN-16859 (AC3b) Onex-Workflow: OCC Receipt Runner Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/35773416475 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai> Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
jonahgabriel
added a commit
that referenced
this pull request
Sep 22, 2026
…to infra (#2783) * fix(OMN-19194): raise shadow-slice timeout budget and route exit 124 to infra market-drain measured (2026-09-22, OMN-17427): the scoped shadow slice (-m "unit and not kafka") takes 621-663s of pytest time plus ~90s of collection, 711-753s total. The old --timeout 900 left only 150-190s of margin, and a ~25% slower runner (observed on PR #2777/#2779) pushed the same correctly-scoped slice past the wall -- exit 124 -- which this step fell through to the generic "failure" branch on, misreporting reason-graph and product-readiness/evaluate as PRODUCT_FAILED for a hang/timeout, not a real product defect. Two-part fix, matching the OMN-18820 exit-125 precedent: - raise the budget to 1200s, giving 300s+ (27%+) margin over the worst-observed timed-out case -- this is runner-speed variance on an already-narrow slice, unlike the sibling onex_change_control finding under this same ticket, where the slice itself needed narrowing; - mirror the exit-125 branch for exit 124: report product_conclusion=timeout and succeed the step, so a genuine remaining hang still routes to the infra dimension instead of a false PRODUCT_FAILED root. * fix(OMN-19194): declare exit-124 timeout as an explicit INFRA conclusion The exit-124 -> product_conclusion=timeout branch this ticket wires into product-readiness-shadow.yml relies on product_readiness.py's fail-closed default (an unrecognized conclusion falls through to INFRA) rather than an explicit vocabulary entry. OMN-18820 declared runner_crashed explicitly for exactly this reason: its own test (now mirrored here) proves the entry is load-bearing documentation, not a functional no-op, because the fail-closed default could be tightened later with nothing left to catch it. Adds "timeout" to _INFRA_CONCLUSIONS with the same measured justification already in the workflow comment, plus a focused test file mirroring test_omn18820_shadow_runner_crash_class.py's TestConclusionVocabulary and TestReasonGraphRoot classes, scoped to this addition only (the harness's own EXIT_HANG=124 classification is unchanged and already covered by test_run_shadow_slice.py).
…l, and require the run window - Contract: the pure fold is no longer a handler_routing entry. With no event_model it was dispatched on the verdict topic beside the writer and failed every dispatch (22 validation errors for the request model), so each stored verdict was also dead-lettered, replayed five times and quarantined on the .201 dev lane (2026-09-24T22:56Z). Even with the bare dict accepted, a reducer's returned model becomes a projection intent the auto-wired applier has no effect for (RuntimeHostError, probed in-container). The writer calls the fold in-process. - Rule 7a: the dry-run decision moves from the writer into the pure fold (row=None); the writer persists only what the fold hands it. - A declined rehearsal reports rows_refused_by_ordering_guard=1 beside rows_upserted=0, so the runtime logs it as expected (OMN-18992) instead of the 'wrote zero rows' ERROR, and its apply counter does not score it as a silent write-path gap. Pinned against real Postgres. - ModelDodVerifyState.started_at/completed_at are required: no defaulted, construction-time window the projection would store as history. - make_completed_event drops its caller-supplied started_at: one source for the window. - The dod_verify contract's dry_run description says what a rehearsal does. Onex-Lane: dod-verdict-publisher-2779 Onex-Session: e0ac582ced0a487f860f431365ab85f4
…ease tag) into OMN-18901 Onex-Lane: jev-dod-producer-2 Onex-Session: e0ac582ced0a487f860f431365ab85f4
…rify-publishes-completed-event
…anged, so this branch passes the release-identity gate; #2874 lands first Onex-Lane: jev-dod-producer-2 Onex-Session: e0ac582ced0a487f860f431365ab85f4
…routed writer constructs as reachable The rule-7a projection split leaves the pure fold unrouted and runs it inside the writer. The missing-routing check read that as an undispatchable handler and flagged node_projection_dod_verdict. A routed module in the node's own package that imports AND calls the canonical class now proves reachability; an import alone, or a writer in another node, still fires. Onex-Lane: jev-dod-producer-2 Onex-Session: e0ac582ced0a487f860f431365ab85f4
…rify-publishes-completed-event
jonahgabriel
deleted the
jonah/omn-18901-dod-verify-publishes-completed-event
branch
September 25, 2026 04:31
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-18901: the DoD verify verdict reaches the bus in a shape the projection accepts, and the projection stores it cleanly
This is the producer half of Task 5. The consumer (OMN-18900, omnimarket#2722) merged first, so this change's job is to make the two halves meet. Head
668a83a24carries a review-and-fix pass (lanedod-verdict-publisher-2779, 2026-09-24). That pass found three more defects on the path, and one of them is in the merged consumer. All three are fixed here and proven on the lab.How the verdict reaches the bus
HandlerDodVerifyholds no publisher. For a definition-B handler the runtime publishes the returned model._normalize_handler_resultwraps a returnedBaseModelas an output event, and_select_dispatch_result_output_topicroutes it to the contract'sterminal_event. The node already declared that terminal and listed it underpublish_topics. The lab readback below confirms this: the deployed runtime consumed a start command and loggedPublished output event to onex.evt.omnimarket.dod-verify-completed.v1.So the node was already publishing. What it published was a payload the merged consumer could only reject:
ModelDodVerifyStatecarried no run window, andModelDodVerdictWirerequiresstarted_atandcompleted_at.What changed
Producer (first pass,
aca4dd496)ModelDodVerifyStatecarriesstarted_atandcompleted_at. The start time is read at the top of the verify path, so the window spans the actual work.make_completed_eventreads the window off the state, so the state and the event built from it cannot disagree.dry_run(defaultFalse). A rehearsal is not projected.Review-and-fix pass (
a4f37d4b6)4. The pure fold is no longer a
handler_routingentry (a defect in the merged consumer). A routing entry with noevent_modelis dispatched on every subscribe topic (_topics_for_handler_entry). SoHandlerProjectionDodVerdictran beside the writer on every verdict and failed every dispatch with22 validation errors for ModelDodVerdictProjectionRequest ... event Field required. On the lab the writer stored the row, and the same dispatch was then dead-lettered, replayed five times and quarantined. Accepting the bare dict does not fix it. An in-container probe of the runtime's own callback and applier factory shows that the fold's return then becomes a projection intent, and the auto-wired applier has no effect configured for it (RuntimeHostError: Dispatch result contains 1 projection intent(s) but no ProtocolProjectionEffect is configured). The fold stays the contract's declared def-Bhandler, and the writer calls it in-process.5. Rule 7a: the fold decides a rehearsal, and the writer only persists.
ModelDodVerdictProjectionResult.rowisNonefor a dry run. This decision used to sit in the writer.6. A declined rehearsal is reported as a refusal, not as a silent zero. Returning bare
rows_upserted: 0made the runtime logProjection handler wrote zero rowsat ERROR, and the OMN-18910 apply counter scored it as a write path that stored nothing. The writer now also reportsrows_refused_by_ordering_guard: 1, the key the runtime reads for deliberate declines (OMN-18992).7. Fail closed on the window.
started_atandcompleted_atare required, with nodefault_factory. A defaulted window would have been the time the model was built, and the table would have stored it as run history. The only construction site in source already supplies both.8.
make_completed_eventno longer takes a caller-suppliedstarted_at, so the window has one source.9. The dod_verify contract's
dry_rundescription now says what a rehearsal does: it is published and marked, and the projection declines it.The return type stays
ModelDodVerifyState, pinned by a test. The evidence-autoclose sweep matches that class string literally.Lab readback (rule 24): .201 dev lane, compose project
omnibase-infraNothing on the lane was restarted, hotpatched or held. A throwaway container of the deployed runtime image
omnibase-infra-omninode-runtime(omnimarket 0.4.216) ran onomnibase-infra-network, with the branch'snode_dod_verifymounted read-only and the runtime's Kafka environment. The consumer in every run is the deployedDodVerdictProjectionWriterinsideomninode-runtime.The same positive dispatch also exposed defect 4. The runtime logged
Dispatcher 'dispatcher.auto.projection_dod_verdict.HandlerProjectionDodVerdict...' failed: ValidationError: 22 validation errors, thendlq_routed=true, thenQUARANTINED da1a2e9e... (Exceeded max replay count: 5 >= 5). Those five replays re-drove the writer each time and the table still holds one row for that run. That is a live idempotent-replay proof of the(ticket_id, correlation_id, completed_at)key.Routing, probed with the runtime's own
discover_contracts_from_pathsand_topics_for_handler_entryin a network-less container:Not exercised on the lab: the dry-run decline and the unrouted fold running inside the live runtime. Both need a runtime hotpatch or this merge plus the lab redeploy. The decline is pinned by the real-Postgres test below. The post-merge compose-dev receipt and a verdict published after the redeploy will cover both.
RED then GREEN
The new and changed tests were run against the previous head
e573a3dd7in a scratch detached worktree:Real-Postgres write-path gate
tests/test_omn18901_real_postgres_producer_payload_write_path.pygainstest_the_runtime_report_names_a_declined_rehearsal_as_a_refusal. It drives the in-process entry against the real migration in a throwaway schema and checks both outcomes bySELECT count(*). A declined rehearsal reports 0 rows and 1 refusal and leaves the table empty. The real run beside it reports 1 row and 0 refusals, and the table holds exactly one row. The run used a local throwaway Postgres 16: 0 skipped.Acceptance criteria
skippedverdict produced its row.Residual, stated rather than hidden
completed_at. It really is a second execution, so the key is doing its job. A reader counting attempts should count distinct correlation ids.onex skill dod_verifypath still runs on an in-memory bus, so its verdicts die with the process. The runtime path (a start command on the bus) is the one that reaches the table.Verification
Base sync: this head merges
origin/devthrough8ddb07c58, which is the commit before thev0.4.216release tag. That is 58 of the 63 commits dev carries past the old base. The branch cannot take the tag commit yet. Dev sits at 0.4.216, which is already published, and the post-release bump to 0.4.217 (omnimarket#2874) is still open. Merging past the tag would put this change's source onto a published version, which the release-identity gate (OMN-16344) refuses. None of the five dev commits after8ddb07c58touch these files.Second pass, head
75f25debf(lanedod-loop-verdict-bus, 2026-09-25)Three reds at
668a83a24, each read from its job log:node_projection_dod_verdict. The reason: the fold was unrouted, and the check's only notion of "reachable" was a routing entry. Routing the fold again would bring back the dead-lettering proven on the lab. So the check now also accepts a canonical handler that a routed module in the same node's package imports from the canonical module and calls. That is the rule-7a projection split. An import without a call still fires, and so does a writer in another node. RED: 2 failed (the new positive case and the real-tree regression). GREEN: 10 passed.test_step_by_step_directive_reaches_the_model_omn19400was fixed on dev by fix(OMN-19400): negative control no longer collides with widened causal markers #2870. Mergingorigin/devbrought that fix in, and it now passes.origin/devis merged in at75f25debf, so this diff no longer touchespyproject.toml.Locally: dod_verify unit tests, both golden chains, the OMN-18900 and OMN-18901 acceptance tests and the OMN-19400 test all pass, 1171 passed and 1 skipped. mypy --strict is clean. Hooks were not bypassed.
Lab readback still holds (
.201dev lane,omninode_internal.dod_verify_runs). The control rowOMN-18901-LABPOS-20260924T225558Z(correlationda1a2e9e-89d7-4f5e-95c1-ed128bf3f1f2) is present. A realonex skill dod_verify OMN-18900verdict produced by this branch's producer, correlationf10705c4-4a2e-4d27-9ffc-367f3d4fe445with status verified and outcome done, now sits beside it. It was published by the OMN-19152 publisher (omnibase_infra#4100). The deployed consumer still routes the fold, so it dead-lettered that same verdict. That is defect 4 above, observed again live at 02:07Z, and this PR fixes it.The change-control companion needs a re-mint for this head.
Merge sequencing (OMN-17427): runtime-affecting, so it lands in a runtime-train slot. The hold-family label stays on. It was placed as a merge-order gate under the 2026-09-22 ruling, and its reason (omnibase_infra#3939) is gone, but it still keeps the repo's auto-merge workflow from arming this PR before its slot. The landing lane removes it.
Evidence-Ticket: OMN-18901
Evidence-Source: OCC#10823