Skip to content

fix(OMN-14637): re-anchor dod_verify live-OPEN gate to MERGED after product PR merges - #1774

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-14637-dod-verify-merged-pr
Jul 15, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-14637-dod-verify-merged-pr

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes OMN-14637.

node_dod_verify re-executes every contract command/check_value check literally. A DoD evidence check that asserts its product PR is still live-OPEN — the canonical gh pr view <n> --repo <r> --json state,... --jq '.state == "OPEN" and ...' idiom — becomes permanently false the instant the PR squash-merges: GitHub flips .state to MERGED and deletes the head branch. The sanctioned onex skill dod_verify closeout then fails-closed forever on a normal, successful merge (13/26 checks failed re-running dod_verify OMN-11878 post-merge, even though the work is verifiably merged and correct).

Fix (sanctioned mechanism #2 from the ticket — substitute a merge-commit-content check on MERGED)

When an evidence item is authoritatively bound to a CONFIRMED-MERGED PR — reusing the exact OMN-14207 binding (_resolve_pr_bindings: explicit pr field or durable receipt pr_number+repo) and live-probe (_fetch_pr_merge_state) machinery — the collector re-anchors only the .state == "OPEN" equality to the merged terminal state (.state == "MERGED").

  • Every other predicate in the command (.headRefOid, .files, .title, .baseRefName, receipt greps — all of which gh pr view still reports for a merged PR) still runs. A genuinely-incomplete ticket whose merge commit lacks the expected content STILL FAILS. The relaxation is verification-preserving and non-vacuous.
  • The merge fact is never caller-supplied — read from the live GitHub surface, mirroring the fail-closed posture of the OMN-14207 live check.
  • Fail-safe: the command runs verbatim (no relaxation) when the live check is disabled (DOD_VERIFY_LIVE_PR_CHECK=0), the item binds to no PR, or the PR is OPEN/CLOSED/unresolved.
  • The rewrite is surgical: only the .state OPEN equality (single- or double-quoted, quote style preserved) moves; an unrelated "OPEN" literal (e.g. .title == "OPEN") is untouched.

Changed surface

  • EvidenceCollector._item_bound_to_merged_pr — authoritative, live-probe-gated, fail-safe merged-binding detection.
  • EvidenceCollector._relax_merged_pr_state_predicate — pure, surgical .state == "OPEN" → .state == "MERGED" rewrite.
  • Wired into _check_evidence_item → _run_command_check (applied after placeholder resolution, gated per-item).

Scope note

This closes the dominant .state == "OPEN" self-referential class (the ticket's headline example, dod-001). The separate gh repo clone --branch <deleted-branch> anti-pattern (dod-004/005) is a contract-authoring issue — the durable content-at-SHA form (gh api contents?ref=<sha>, used by dod-002/003) already survives merge, which is the ticket's suggestion #1 and is OCC-authoring territory, not a node change.

dod_evidence

  • New regression test tests/unit/nodes/node_dod_verify/test_omn_14637_merged_pr_state_relaxation.py (16 tests) proves:
    • RED→GREEN on the identical artifact: the same command FAILS run verbatim (pre-fix, PR merged) and PASSES once the merged-state relaxation is applied.
    • Non-vacuous / prove-RED-against-exists-but-wrong: a merged PR whose declared check asserts genuinely-absent content STILL FAILS.
    • Predicate-rewrite precision (only .state's OPEN moves; unrelated "OPEN" and .state == "CLOSED" untouched).
    • Merged-binding gate: True only on a CONFIRMED-MERGED live probe; False for OPEN/unresolved/unbound/live-check-disabled.
    • End-to-end wiring through collect() and the handler.
  • uv run pytest tests/unit/nodes/node_dod_verify/ tests/integration/node_dod_verify/ tests/test_golden_chain_dod_verify.py → 218 passed (16 new + 202 pre-existing, no regressions).
  • ruff format + ruff check clean; mypy --strict clean on evidence_collector.py; pre-commit run --files green.

Not in scope for this PR

  • No OCC/Evidence-Source companion authored here (Codex/autogen owns that). An occ-preflight/receipt-gate red is expected until the companion lands.
  • No merge armed (Codex owns landing).

Evidence-Source: OCC#4223
Evidence-Ticket: OMN-14637
Evidence-Commit: e511ad5109f38e561b75f6501ed2b2c1b011d23e
Evidence-Head: 7e8477a

…roduct PR merges

node_dod_verify re-executes every contract command/check_value literally. A DoD
evidence check that asserts its product PR is still live-OPEN (the canonical
`gh pr view <n> --repo <r> --json state,... --jq '.state == "OPEN" and ...'`
idiom) becomes permanently false the instant the PR squash-merges: GitHub flips
.state to MERGED and deletes the head branch. The sanctioned closeout then
fails-closed forever on a normal, successful merge (13/26 checks failed
re-running `dod_verify OMN-11878` post-merge).

Fix: when an evidence item is authoritatively bound to a CONFIRMED-MERGED PR
(reusing the OMN-14207 binding + live-probe machinery), the collector re-anchors
ONLY the `.state == "OPEN"` equality to the merged terminal state
(`.state == "MERGED"`). Every other predicate (.headRefOid/.files/.title/
.baseRefName/receipt greps — all retained by gh for a merged PR) still runs, so a
genuinely-incomplete ticket STILL FAILS. Verification-preserving and non-vacuous.

- EvidenceCollector._item_bound_to_merged_pr: authoritative, live-probe-gated,
  fail-safe (disabled live check / unbound / OPEN / unresolved probe -> verbatim).
- EvidenceCollector._relax_merged_pr_state_predicate: surgical rewrite of only
  the .state OPEN equality (single/double quoted), never an unrelated "OPEN".
- Wired into _check_evidence_item -> _run_command_check (post placeholder resolve).

Regression test proves RED-verbatim -> GREEN-relaxed on the identical command,
a non-vacuous control (unmet content still FAILS for a merged PR), predicate
precision, and the merged-binding gate. 16 new tests; 218 dod_verify tests green.

dod_evidence: omnimarket tests/unit/nodes/node_dod_verify/test_omn_14637_merged_pr_state_relaxation.py (16 pass); full node_dod_verify unit+integration+golden-chain suites 218 pass; ruff format+check clean; mypy --strict clean on evidence_collector.py; pre-commit green (topic-naming-lint env-only).
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e1c1aa74-668f-4fbd-9c72-ab5d2989edcb

📥 Commits

Reviewing files that changed from the base of the PR and between 846fda8 and 7e8477a.

📒 Files selected for processing (2)
  • src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py
  • tests/unit/nodes/node_dod_verify/test_omn_14637_merged_pr_state_relaxation.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-14637-dod-verify-merged-pr

Comment @coderabbitai help to get the list of available commands.

@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Handoff note (background LANE B, not merging — Codex owns landing):

Code is proven green locally; the only CI reds are the OCC-companion dependency cascade.

Root cause of every red: occ-preflight / eligibility FAILED (no OCC Evidence-Source companion exists for OMN-14637 yet) → OCC Preflight Dependency refuses to start every dependent workflow → the whole test / verify / typecheck / lint / golden-chain matrix is SKIPPED → the fail-closed aggregators (CI Summary per OMN-14590, Omni Standards Gate, Hostile Review Gate, PR Arch Review Gate) then report FAILED because skipped ≠ passed. None of my code's own gates ran red — they never ran.

To unjam: author the OCC Evidence-Source companion for OMN-14637 (Codex/autogen path). Once occ-preflight passes, the real matrix runs.

Local proof on this branch:

  • uv run pytest tests/unit/nodes/node_dod_verify/ tests/integration/node_dod_verify/ tests/test_golden_chain_dod_verify.py → 218 passed (16 new OMN-14637 tests + 202 pre-existing, no regressions)
  • ruff format + ruff check clean; mypy --strict clean on evidence_collector.py; pre-commit run --files green (topic-naming-lint needed OMNIBASE_INFRA_PATH, env-only)

@jonahgabriel
jonahgabriel merged commit 4d20ccf into dev Jul 15, 2026
638 of 717 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-14637-dod-verify-merged-pr branch July 15, 2026 18:31
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