Skip to content

feat(OMN-16795): wire the subscribe-wiring checker as a CI gate and enforce allowlist expiry - #2948

Merged
github-actions[bot] merged 1 commit into
devfrom
jonah/omn-16795-subscribe-wiring-gate
Aug 27, 2026
Merged

github-actions[bot] merged 1 commit into
devfrom
jonah/omn-16795-subscribe-wiring-gate

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Aug 27, 2026 •

Copy link
Copy Markdown
Collaborator

The gap: a ratchet nobody ran

scripts/check_subscribe_wiring_health.py has existed since OMN-7385. Before this PR:

grep -rn "check_subscribe_wiring_health" .github/ .pre-commit-config.yaml Makefile* scripts/
  -> only the script's own docstring and its unit test

Not a CI job. Not a pre-commit hook. Per doctrine rule 5 that makes it advisory, and advisory checks get ignored — which is how its allowlist reached 91 entries, 23 of them for topics no contract subscribes to any more, carrying expiry dates nothing ever read.

This PR does not build a second ratchet. It wires the existing one.

Why the check is load-bearing

runtime_host_process.py:3716 gates on subcontract.subscribe_topics, calls TopicProvisioner.ensure_topic_exists(), then wire_subscriptions(); event_bus_subcontract_wiring.py:414 mints one consumer group per topic. A subscribe declaration for a topic nobody publishes therefore manufactures a permanently Stable-and-empty Kafka consumer group that reads as a live-but-idle consumer — the exact false signal behind the OMN-16755 dead-chain false alarm.

AC1 — the gate (same PR, rule 5)

  • ci.yml job subscribe-wiring-health ("Subscribe Wiring Health"). Unconditional (no needs:/if:), zero infrastructure — pure static analysis over contract.yaml.
  • Registered in ci_summary_gate.py::STRICT_GATE_JOBS. This is half the mechanism, not a formality: the default-deny sweep already fails on a job that fails, but an unregistered job that is skipped or absent yields SUCCESS. Without the registration, deleting or skipping the job silently restores advisory-only state. CI Summary is dev's only required context, so this blocks merge with no branch-protection edit.
  • Pre-commit hook, scoped to contract.yaml + the checker. pass_filenames: false because the check is inherently cross-contract — it must scan the whole node tree to know whether some other contract publishes the topic, so it can never run against just the staged files.

AC2 — expiry enforcement (TDD)

expiry: was decoration; nothing read it. Three failure modes now fail closed:

meaning
EXPIRED the owner's own deadline passed (inclusive)
MALFORMED no parseable expiry, so the entry could never expire
STALE no contract subscribes to it any more — dead weight inflating the debt number

Tests were written first and observed RED (ImportError) before implementation. today is injected so the enforcement is itself testable, plus a live row that runs against the checked-in allowlists with the real clock — that is the row that turns the gate red the day an entry lapses.

check_allowlist_hygiene runs from main() against the real tree, deliberately not inside check_wiring_health — folding it in would make every caller scanning a partial or synthetic directory report the entire allowlist as stale.

Allowlist dispositions — verified per entry, NOT bulk-extended

Deleted 23 entries no contract subscribes to. Mechanically proven, not judged. Allowlist 91 → 68.

Renewed 40 entries dated 2026-09-01 (five days out), re-dated by evidence class, with the basis recorded in each reason string:

class evidence new expiry n
VERIFIED a non-enum source reference confirms the stated in-repo publisher exists 2026-12-01 8
EXTERNAL publisher is outside omnibase_infra, so the claim is not falsifiable from this repo — the reason now says so 2026-12-01 7
UNVERIFIED enum-only and the reason names an in-repo publisher or calls itself pending 2026-10-01 (short leash) 25

The weak signal — "the topic string appears in some .py" — was rejected as evidence: it is dominated by topic enum modules, which say nothing about a publisher. Three entries (service-lifecycle, system-alert, tool-update) claim "published by <subsystem>" and have zero non-enum references anywhere, so those claims are marked unsupported rather than renewed on their own say-so.

Incident replay (OMN-15547 default-deny)

Newly wired enforcement must ship with a real regression case, not a baseline exemption. Added omn16755-dead-subscribe-mints-a-permanently-empty-consumer-group: the verbatim contract of node_pr_state_write_effect, read out of the git object at dev e4932fe4 (4768 bytes, sha256 2bb43d97…), which declares a subscribe topic no contract publishes.

The replay drives the real check_wiring_health over those bytes with both allowlists emptied and requires DEAD_LETTER. The emptying is deliberate: the live tree exempts this topic because the write path genuinely routes by intent, and a guard that only ever sees allowlisted input is never exercised at all.

Explicitly NOT in scope

  • AC3 (decide the two briefed topics) and AC4 (suppress the empty consumer group at source) are untouched and remain open. The declarations are not removed: runtime_local_ingress.py:133-144 registers no route when subscribe_topics is absent, so deletion would silently drop each node's local-ingress route and every handler-operation alias, and tests/fixtures/dispatch_parity/baseline-selection-v2.json names them in ~20 places.
  • OMN-16783's flow-expectation ratchet is strictly stronger and supersedes this check when it lands. Noted in the job comment and the registry entry. Do not build both.

Verification

uv run pytest tests/ci/test_ci_summary_gate.py \
  tests/unit/scripts/test_check_subscribe_wiring_health.py   -> 107+ passed
uv run python scripts/check_subscribe_wiring_health.py       -> PASS (allowlists clean)
uv run python scripts/ci/check_incident_replay_coverage.py   -> OK, 18 guards covered
pre-push governed impacted-test selector                     -> 24211 passed, 45 skipped
ruff format / ruff check / mypy                              -> clean

Refs: OMN-16795, OMN-7385, OMN-16755, OMN-16783, OMN-16776, OMN-15547

Evidence-Ticket: OMN-16795
Evidence-Source: OCC#7339

…rce allowlist expiry

`scripts/check_subscribe_wiring_health.py` has existed since OMN-7385 and was
referenced by NOTHING — `grep -rn check_subscribe_wiring_health .github/
.pre-commit-config.yaml Makefile* scripts/` returned only its own docstring and
its unit test. Per doctrine rule 5 that made it advisory, and advisory checks
get ignored: its allowlist reached 91 entries, 23 of them for topics no
contract subscribes to any more, with expiry dates nothing ever read.

This does NOT build a second ratchet. It wires the existing one.

Why the check matters

runtime_host_process.py:3716 gates on subcontract.subscribe_topics, calls
TopicProvisioner.ensure_topic_exists, then wire_subscriptions;
event_bus_subcontract_wiring.py:414 mints one consumer group per topic. So a
subscribe declaration for a topic nobody publishes manufactures a permanently
Stable-and-empty consumer group that reads as a live-but-idle consumer — the
exact false signal behind the OMN-16755 dead-chain false alarm.

AC1 — the gate (same PR, rule 5)
- ci.yml job `subscribe-wiring-health` ("Subscribe Wiring Health"),
  unconditional (no needs/if), zero infrastructure (static YAML analysis).
- Registered in ci_summary_gate.py::STRICT_GATE_JOBS. This is half the
  mechanism: the default-deny sweep already fails on a job that FAILS, but an
  unregistered job that is `skipped` or absent yields SUCCESS, so without the
  registration deleting the job would silently restore advisory-only state.
  `CI Summary` is dev's only required context, so this blocks merge with no
  branch-protection edit.
- pre-commit hook, scoped to contract.yaml + the checker itself.
  `pass_filenames: false` because the check is inherently CROSS-contract — it
  must scan the whole node tree to know whether some OTHER contract publishes
  the topic, so it can never run against just the staged files.

AC2 — expiry enforcement (TDD)

`expiry:` was decoration; nothing read it. Now three failure modes fail closed:
  EXPIRED    the owner's own deadline passed (inclusive).
  MALFORMED  no parseable expiry, so the entry could never expire.
  STALE      no contract subscribes to it any more — dead weight that inflates
             the debt number and makes the real entries easy to ignore.

Tests were written first and observed RED (ImportError) before implementation.
`today` is injected so the enforcement is itself testable, plus a live row that
runs against the checked-in allowlists with the real clock — that is the row
that turns the gate red the day an entry lapses.

Incident replay (OMN-15547 default-deny)

Newly wired enforcement must ship with a real regression case, not a baseline
exemption. Added `omn16755-dead-subscribe-mints-a-permanently-empty-consumer-group`:
the verbatim contract of node_pr_state_write_effect read out of the git object
at dev e4932fe (4768 bytes, sha256 2bb43d97…), which declares a subscribe topic
no contract publishes. The replay drives the real check_wiring_health over those
bytes with both allowlists emptied and requires DEAD_LETTER — a guard that only
ever sees allowlisted input is never exercised at all.

Allowlist dispositions — verified per entry, NOT bulk-extended

  DELETED 23 entries no contract subscribes to. Mechanically proven, not judged.
  Allowlist 91 -> 68.

  RENEWED 40 entries dated 2026-09-01 (five days out), re-dated by EVIDENCE
  CLASS with the basis recorded in each reason string:
    - 8 VERIFIED    a non-enum source reference confirms the stated in-repo
                    publisher exists -> 2026-12-01
    - 7 EXTERNAL    the stated publisher is outside omnibase_infra, so the claim
                    is NOT falsifiable from this repo; reason now says so
                    explicitly -> 2026-12-01
    - 25 UNVERIFIED enum-only AND the reason names an in-repo publisher or calls
                    itself pending. The claim could NOT be confirmed here, so
                    these get a SHORT leash -> 2026-10-01

  The weak signal (topic string appears in some .py) was rejected as evidence:
  it is dominated by topic ENUM modules, which say nothing about a publisher.
  Three entries — service-lifecycle, system-alert, tool-update — claim
  "published by <subsystem>" and have ZERO non-enum references anywhere, so
  those claims are unsupported in this repo and are marked as such rather than
  renewed on their own say-so.

Not in scope

AC3 (decide the two briefed topics) and AC4 (suppress the empty consumer group
at the source) are untouched and remain open on the ticket. OMN-16783's
flow-expectation ratchet is strictly stronger and supersedes this check when it
lands — noted in the job comment and the registry entry. Do not build both.

Verification
  uv run pytest tests/ci/test_ci_summary_gate.py \
    tests/unit/scripts/test_check_subscribe_wiring_health.py  -> 107+ passed
  uv run python scripts/check_subscribe_wiring_health.py      -> PASS
  uv run python scripts/ci/check_incident_replay_coverage.py  -> OK, 18 covered
  ruff format / ruff check / mypy                             -> clean

Refs: OMN-16795, OMN-7385, OMN-16755, OMN-16783, OMN-16776, OMN-15547
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 56 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 130 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.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee9ef96c-73c8-4a1e-b1b6-c77bb8aaa303

📥 Commits

Reviewing files that changed from the base of the PR and between 35b7bd7 and a2be7e8.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • .pre-commit-config.yaml
  • scripts/check_subscribe_wiring_health.py
  • scripts/ci/ci_summary_gate.py
  • tests/fixtures/omn16795/node_pr_state_write_effect-contract.yaml.captured
  • tests/incident_replays/registry.yaml
  • tests/unit/scripts/test_check_subscribe_wiring_health.py

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

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8468/OMN-8524) and do not block merge. Error: all review endpoints [192.168.86.201:8000 192.168.86.201:8000 ] unreachable — preflight short-circuit (no models available)


Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical findings No
blocked CRITICAL findings found Yes
degraded All models unavailable (infra) No (pilot)

Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review (OMN-8468/OMN-8524)

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