Repository navigation
fix(OMN-17017): repair wave scheduler + dispatch watchdog — honest contracts, real effect boundary, true dependency propagation, runtime reachability - #2237
Merged
jonahgabriel merged 1 commit intoAug 31, 2026
Conversation
…ntracts, real effect boundary, true dependency propagation, runtime reachability Root cause RC-J (2026-08-29 beta off-the-rails analysis rev 2, operator top-three #2): the canonical assignment layer was already built as ONEX nodes, was contract-complete and CLI-registered, and was structurally incapable of executing. The planner shipped; the executor never did. Contracts honest (A10 step 1) - wave scheduler: deleted "monitor stalls", the agent_healthcheck integration claim, and four publish topics the node never published; publish_topics is now exactly the terminal event (published by the runtime). timeout_ms 4h -> 10m and implementation_wave / handlers_deferred:true removed — the handler waits on nothing and is no longer deferred. - watchdog: deleted two publish topics the node never published; the two state path promises (friction event, dispatch-log line) are now implemented rather than deleted. - new gate scripts/ci/check_contract_publish_parity.py, wired as a pre-commit hook and a CI step, burn-down ratchet over scripts/ci/contract_publish_parity_baseline.py (131 frozen offenders; the two repaired nodes are NOT in it). RED-first: the checker fails on the pre-repair wave-scheduler contract and passes after. Effect boundary (A10 step 2) - HandlerWaveDispatchStateStore implements ProtocolWaveDispatcher: append-only dispatch-lifecycle NDJSON under the resolved state dir plus an observed-outcome read-back. dry_run=false no longer raises unconditionally. - HandlerWatchdogStateStore implements ProtocolWatchdogRecoveryAdapter, including record_check, so watchdog.json + dispatch-log/<date>.ndjson are really written and their paths reported instead of hardcoded None. Dependency propagation (A10 step 3) - waves are computed incrementally from OBSERVED completion; selection is no longer completion. - a non-COMPLETED ticket removes its transitive dependent closure from the remaining waves — those tickets are reported BLOCKED and never dispatched. - a missing dispatcher status is UNREPORTED, never "completed", and forbids run_status=COMPLETED; a status outside the enum raises. - fail_fast is live (ABORTED + SKIPPED); resumed reports a real checkpoint read; healthcheck_config (inert and not CLI-expressible) deleted. Reachability (A10 step 5/6) - HandlerWatchdogEpicStateReader implements the epic-state loading the request model already advertised, so --epic-id does something observable instead of raising. Missing state names the exact path. - substrate: durable dispatch-lifecycle records under $ONEX_STATE_DIR — no daemon, no new runner, shaped for the OMN-16176 obligation projection to consume when it lands. Baselines shrunk, never grown: 6 entries out of contract_topic_graph_baseline, 6 out of dep_health_baseline, 5 out of state_coverage_baseline. Census note (DoD item 6) recorded in the classifier artifact's own header: node_dispatch_queue_drainer has zero entries there, so a node inventory built from the classifier alone silently omits it. Refs OMN-17017. Related OMN-17121 (org-wide publish-parity gate), OMN-16176. Commit-message provenance (this reword changes no content) An earlier `git commit --amend` at 2026-08-30T12:03:07-0400 was run from the wrong worktree: it replaced this commit's message with "fix(OMN-17187): remove the orphaned error-triaged.v1 emission from node_runtime_error_triage_effect", an omnibase_infra message, while the tree it was amending was this omnimarket OMN-17017 change. The mislabel was purely textual — that same amend ADDED more OMN-17017 content (wave-scheduler contract tightening, handler_watchdog + handler_wave_scheduler_orchestrator fixes, the new tests/nodes/node_wave_scheduler_orchestrator/test_dependency_propagation.py, and the expanded tests/test_node_dispatch_watchdog_orchestrator.py) and touched no node_runtime_error_triage_effect file at all. OMN-17187's real fix lives in omnibase_infra and was released separately as 0.38.15; nothing here overlaps it. This commit therefore carries the full OMN-17017 repair INCLUDING the dependency-propagation tests added by that amend: the two-wave synthetic epic in which a wave-1 failure proves wave 2 is never dispatched, the transitive BLOCKED closure, and the partial-status-map / UNREPORTED cases. Two fixes found by rebasing this commit onto current dev and re-running: - scripts/ci/contract_publish_parity_baseline.py: dropped node_projection_receipt_gate. OMN-17210 (#2225) landed HandlerReceiptGateProjectionRunner on dev, giving that node a real writer for its declared snapshot topic, so it is no longer a parity offender. The new test_baseline_only_shrinks caught it — the ratchet is a burn-down list, and leaving a repaired node frozen in it would have turned it into an allowlist. 132 -> 131 frozen offenders. - tests/nodes/node_dispatch_watchdog_orchestrator/test_reachability_and_state_paths.py: all three HandlerWatchdogStateStore constructions now pass now_utc=_NOW. They injected the frozen clock into the handler but not into the state store, so the dispatch-log filename came from the wall clock and the `name == "2026-08-30.ndjson"` assertion passed only on the day it was written (it failed the moment the clock rolled to 2026-08-31 UTC). The assertion is kept as-is and is now deterministic, which also makes it prove that the injected clock — not wall time — governs the log name.
|
Warning Review limit reachedNext included review available in 20 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 137 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
Comment |
|
| 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_dispatch_watchdog_orchestrator/handlers/handler_watchdog.py:165: adapter = self._recovery_adapter or HandlerWatchdogStateStore(
src/omnimarket/nodes/node_dispatch_watchdog_orchestrator/handlers/handler_watchdog.py:233: reader = self._epic_state_reader or HandlerWatchdogEpicStateReader(
src/omnimarket/nodes/node_wave_scheduler_orchestrator/handlers/handler_wave_scheduler_orchestrator.py:128: dispatcher = self._dispatcher or HandlerWaveDispatchStateStore(\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).
onexbot-occ-writer Bot
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 31, 2026
…nimarket#2237 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)
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| class ProtocolWatchdogEpicStateReader(Protocol): | ||
| """Adapter boundary for loading a wave's task state by epic id.""" | ||
|
|
||
| def read_wave_tasks(self, epic_id: str) -> tuple[ModelWaveTask, ...]: ... |
|
|
||
| def escalate_to_blocked(self, task: ModelWaveTask) -> str | None: ... | ||
|
|
||
| def record_check(self, result: ModelWatchdogResult) -> ModelWatchdogStatePaths: ... |
| def dispatch_wave( | ||
| self, assignment: ModelWaveAssignment | ||
| ) -> dict[str, EnumTicketExecutionStatus | str]: ... | ||
| ) -> Mapping[str, EnumTicketExecutionStatus]: ... |
jonahgabriel
enabled auto-merge (squash)
August 31, 2026 01:36
jonahgabriel
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 31, 2026
…imarket#2237 (#7776) * evidence(OMN-17017): author OCC companion for OmniNode-ai/omnimarket#2237 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 0777c28f9f8bd041069cb2e05745d7e52718a083. * evidence(OMN-17017): self-bind OCC#7776 + rebind contract_sha256 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#2237 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) * style(OMN-17017): yamlfmt the executed test_passes supersede receipt for omnimarket#2237 The occ-receipt-runner minted drift/dod_receipts/OMN-17017/dod-occ-diff-derived-behavior-proof/test_passes.supersede.2237.yaml in unformatted YAML, so the required Pre-commit check failed on the yamlfmt hook and left omnimarket#2237's Evidence-Source companion BLOCKED. Running yamlfmt alone was NOT sufficient and would have corrupted evidence. probe_stdout was a single-quoted scalar carrying a real newline, and the formatter rewrote it by injecting its internal marker into the value itself - caught by the OMN-15479 contamination ratchet: 1 yamlfmt sentinel occurrence(s) in parsed values, baseline allows 0 Per that gate's own remediation, probe_stdout is re-authored as a literal block scalar (|-) instead, which the formatter leaves alone. Verified byte-identical across the change by parsing before and after: '......................... [100%]\n25 passed in 6.21s' yamlfmt is now idempotent on this file (second pass Passed) and no sentinel remains. The frozen OMN-15479 baseline was NOT touched - it may only shrink. Formatting only. Every evidence value is preserved: status PASS, exit_code 0, commit_sha 0777c28f9f8bd041069cb2e05745d7e52718a083, and the product-repo run URL https://github.com/OmniNode-ai/omnimarket/actions/runs/33347816324. No receipt was re-scored, re-dated, or re-bound. Refs OMN-17017. * evidence(OMN-17017): attribute the supersede receipt's commit_sha to omnimarket so COMMIT_SHA_EXISTS resolves The receipt hardening gate failed the executed test_passes supersede receipt: [COMMIT_SHA_EXISTS] commit_sha '0777c28f9f8bd041069cb2e05745d7e52718a083' does not resolve to a real, remote-reachable commit The SHA is real. It is the head of omnimarket#2237 and resolves live: gh api repos/OmniNode-ai/omnimarket/commits/0777c28f9f8bd041069cb2e05745d7e52718a083 -> 0777c28f9f8bd041069cb2e05745d7e52718a083, 2026-08-31T01:04:04Z gh api repos/OmniNode-ai/onex_change_control/commits/0777c28f... -> HTTP 422 The gate resolves a commit_sha against THIS repo unless the receipt embeds a repos/<owner>/<repo>/ or --repo <owner>/<repo> citation in check_value, probe_command or actual_output (_repo_hint, allowlisted by _KNOWN_REPO_HINTS). The occ-receipt-runner minted this supersede with a bare pytest probe_command carrying no repo attribution, so a genuinely pushed product-repo commit read as unresolvable -- the gate was right to refuse it, and the missing fact was the repo, not the commit. probe_command is re-authored as a literal block scalar with the true provenance as a leading comment naming repos/OmniNode-ai/omnimarket/ and the exact head SHA. The pytest invocation is unchanged. Verified by parsing before and after that status, exit_code, commit_sha, probe_stdout, check_value, actual_output and run_timestamp are all byte-identical -- nothing was re-scored or re-dated. check_receipt_hardening.py on this file now exits 0, and yamlfmt is idempotent. No gate was bypassed and no baseline was touched. Refs OMN-17017. --------- Co-authored-by: omnimarket-bot <bot@omninode.ai> Co-authored-by: onexbot-occ-writer[bot] <onexbot-occ-writer[bot]@users.noreply.github.com> Co-authored-by: jonahgabriel <jonah@omninode.ai>
jonahgabriel
deleted the
jonah/omn-17017-repair-wave-scheduler-dispatch-watchdog
branch
August 31, 2026 02:54
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
2 participants
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.
Refs OMN-17017.
Summary
Root cause RC-J (2026-08-29 beta off-the-rails analysis rev 2, operator top-three #2): the canonical assignment layer was already built as ONEX nodes, was contract-complete and CLI-registered, and was structurally incapable of executing. The planner shipped; the executor never did.
node_wave_scheduler_orchestratorandnode_dispatch_watchdog_orchestratorboth raised unconditionally on any live invocation, published none of the events their contracts promised, and computed waves as if selection were completion.What changed
1. Contracts honest (DoD step 1)
agent_healthcheckintegration claim, and four publish topics the node never published.publish_topicsis now exactly the terminal event.timeout_ms4h → 10m;implementation_wave/handlers_deferred: trueremoved — the handler waits on nothing and is no longer deferred.scripts/ci/check_contract_publish_parity.py, wired as a pre-commit hook and a CI step, with a burn-down ratchet inscripts/ci/contract_publish_parity_baseline.py. The two repaired nodes are deliberately not in the baseline.2. Real effect boundary (DoD step 2)
HandlerWaveDispatchStateStoreimplementsProtocolWaveDispatcher— append-only dispatch-lifecycle NDJSON under the resolved state dir, plus an observed-outcome read-back.dry_run=falseno longer raises.HandlerWatchdogStateStoreimplementsProtocolWatchdogRecoveryAdapterincludingrecord_check, sowatchdog.jsonanddispatch-log/<date>.ndjsonare really written and their real paths reported instead of hardcodedNone.Both protocols previously had no implementation anywhere in the repo.
3. True dependency propagation (DoD step 3)
COMPLETEDticket removes its transitive dependent closure from remaining waves; those tickets are reportedBLOCKEDand never dispatched.UNREPORTED, never"completed", and forbidsrun_status=COMPLETED. A status outside the enum raises.fail_fastis live (ABORTED+SKIPPED);resumedreports a real checkpoint read; the inert, non-CLI-expressiblehealthcheck_configis deleted.4. Reachability (DoD steps 4–6)
HandlerWatchdogEpicStateReaderimplements the epic-state loading the request model already advertised, so--epic-iddoes something observable instead of raising. A missing state file names the exact path.$ONEX_STATE_DIR— no daemon, no new runner, shaped so the OMN-16176 obligation projection can consume it when it lands. This deliberately picks the option that adds no new always-on process.node_dispatch_queue_drainerhas zero entries there, so any node inventory built from the classifier alone silently omits it.Baselines shrunk, never grown: 6 out of
contract_topic_graph_baseline, 6 out ofdep_health_baseline, 5 out ofstate_coverage_baseline, 1 out ofcontract_publish_parity_baseline.dod_evidence
RED-first, proven by isolation rather than asserted
The repaired tests were overlaid onto an unmodified
origin/devsource tree (detached worktree at34922a7c;git status -- src/empty, so only tests moved) and run:origin/devsourceThe 2 collection errors are themselves the DoD-step-2 proof:
handler_wave_dispatch_state_storeandhandler_watchdog_state_storedo not exist on dev (ModuleNotFoundError), which is exactly the "protocol has no implementation anywhere" finding.The ticket's four falsifiable done-proofs, each mapped to a named test
test_no_new_publish_parity_offenders,test_repaired_nodes_declare_only_publishable_topics[both nodes]test_failed_wave_one_does_not_dispatch_wave_two(+test_failure_cascade_is_transitive)COMPLETEDresulttest_partial_status_map_is_unreported_not_completed(+test_unreported_dependency_blocks_dependents)--fail-fastand watchdog--epic-iddo something observable, notRuntimeErrortest_fail_fast_aborts_remaining_waves,test_live_run_without_injected_dispatcher_does_not_raise,test_live_redispatch_uses_the_shipped_default_adapterGates
is_full_suite=True reason=changed_test_unnarrowable) and passed — 17253 passed, 143 skipped, 3 deselected, 3 xfailed in 910s. NoPREPUSH_*override, no--no-verify. The host-capacity guard refused the first attempt; I waited for capacity and retried rather than settingPREPUSH_ALLOW_LOCAL_FULL_SUITE.mypy --stricton all touched sources: Success, no issues in 16 source files.ruff format+ruff checkonsrc/ tests/: All checks passed.pre-commit run --all-files: exit 0.Two defects found by rebasing onto current dev
node_projection_receipt_gatedropped from the parity baseline — OMN-17210 (feat(OMN-17210): author HandlerReceiptGateProjectionRunner so receipt_gate_rows gains a writer #2225) landed a real writer for it on dev, so it is no longer an offender. The newtest_baseline_only_shrinkscaught it; leaving it frozen would have turned the burn-down list into an allowlist. 132 → 131.HandlerWatchdogStateStoreconstructions in the reachability tests now passnow_utc=_NOW. They injected the frozen clock into the handler but not the state store, so the dispatch-log filename came from the wall clock and thename == "2026-08-30.ndjson"assertion passed only on the day it was written. The assertion is kept and is now deterministic — and now actually proves the injected clock governs the log name.Commit-message provenance
This branch's commit was mislabeled. An earlier
git commit --amendrun from the wrong worktree replaced its message with an omnibase_infra one (fix(OMN-17187): …) while the tree was this OMN-17017 change; that same amend also added more OMN-17017 content. The message has been corrected — content byte-identical, treebe96ef23before and after the reword. Nothing here touches OMN-17187's real scope, which shipped separately in omnibase_infra 0.38.15.Side effect worth flagging: while mislabeled, this work was invisible to
git log --all --grep=OMN-17017, which caused the r1 evidence-first plan §18.0 to record "no work begun" for OMN-17017. That is corrected in omni_home#247 and on OMN-17017 / OMN-17273.Related: OMN-17121 (org-wide publish-parity gate), OMN-16176 (obligation projection).
Evidence-Ticket: OMN-17017
Evidence-Source: OCC#7776