Repository navigation
test(OMN-16814): drive the projection capability seam through a real chain - #2953
Conversation
…chain
OMN-16690: node_hook_event_capture/contract.yaml declared access='write' while
its handler called db.query(). The runtime read seam refused, the projection arm
swallowed the PermissionError and DLQ'd the event, callers kept seeing 202, and
the quarantine sink reached HWM 8,878,932. It ran about a week behind green CI.
The platform testing inventory names why nothing caught it (section 7.1): the
golden-chain DB double carries NO capability enforcement, so a contract/handler
capability mismatch is invisible to all 194 golden chains BY CONSTRUCTION. That
is equally true in THIS repo --
tests/integration/runtime/test_projection_handler_db_injection_integration.py
injects a FakeDb through _build_projection_db_adapter whose upsert/query do no
access checking, so it is green for every declared access value.
omnimarket#2164 closed the statically-visible half (a handler that TEXTUALLY
calls db.query() must declare read_write). It cannot see a read reached through
a helper, a base class or an adapter, and nothing anywhere proved the RUNTIME
refusal is still wired or that a mismatch turns a whole CHAIN red.
Where the seam boundary is drawn
--------------------------------
_assert_read_declared / _assert_write_declared both fire BEFORE the adapter
touches a connection, so the whole capability decision is reachable with zero DB
contact. Real here: _prepare_handler_wiring arm selection,
_make_projection_dispatch_callback, _build_projection_db_adapter,
ProjectionDatabaseOperations domain routing, and ProjectionTableOperation's
assertions and SQL construction. Substituted: the psycopg2 DRIVER only, via
sys.modules -- the technique test_sync_psycopg2_adapter_preserves_text_array_lists
already uses here. Substituting the adapter OBJECT is the fidelity gap being
closed; do not "simplify" this file back into that.
The matrix, both directions (a one-sided fixture can pass by accident):
read_write declared + query -> terminal emitted, SQL executed
read_write declared + upsert -> terminal emitted, SQL executed
write declared + query -> REFUSED (the OMN-16690 shape)
read declared + upsert -> REFUSED (mirror image)
Only `access` varies across the four rows. Same handler, same wiring, same wire
bytes, same table -- so any divergence is attributable to the capability seam and
to nothing else.
Each refused row asserts THREE separate facts, because any one alone is weak:
no terminal event (the chain does not report success -- the half the live outage
got wrong at the HTTP boundary); the event reaches the contract-declared DLQ
carrying the PermissionError (bus-observable, not log-only -- log-only detection
is what let the live defect run a week); and ZERO SQL was constructed (the
refusal precedes statement construction, so it is a gate and not a late check).
Two fixture facts that are load-bearing, both found by measurement
-----------------------------------------------------------------
1. The table is projection_watermarks, a REAL table from the shipped local
topology. A fabricated name cannot reach the capability seam at all:
_require_projection_binding_privileges rejects it three layers earlier, and
the only sanctioned grant-synthesis helper refuses for already-shipped tables.
A fabricated row would have proven nothing about access.
2. The contract must set terminal_event AND list it in publish_topics
(handler_wiring.py:8486-8492) or the arm emits no terminal -- which would make
a positive row's observation identical to a refused row's, i.e. vacuous.
RED-first proof, and what it corrected
--------------------------------------
Stubbing BOTH production assertions to `lambda self: None` does NOT make the
PermissionError disappear -- the chain then fails a SECOND, independent refusal
("Projection operation has no declared workload binding"), because a
write-declared table has no read binding to resolve either.
So an assertion of the form `"PermissionError" in dlq_text` STAYS GREEN with the
capability seam switched off and would gate nothing. Asserting the seam's OWN
WORDING is what makes these rows non-vacuous. Measured, then recorded at the
assertion:
neutered -> 2 failed, 3 passed (both refused rows RED, on that line only)
restored -> 9 passed (whole tests/integration/chains/ suite)
AC3 also anchors by importing ProjectionTableOperation and asserting both refusal
methods exist on the production class. omnimarket's
node_projection_live_events/tests/test_projection_live_events.py:38 re-implements
`access not in {"read","read_write"}` and asserts its own copy -- it was green
throughout the outage, because a mirror of a rule cannot tell you the rule is
still wired.
Enforcement, not detection (rule 5): no workflow edit. The Event Chain Gate job
already runs `uv run pytest tests/integration/chains/` wholesale and is
registered in ci_summary_gate.py::STRICT_GATE_JOBS, so a skipped/absent
conclusion fails CI Summary closed.
NO runtime behavior change. In particular this does NOT implement inventory
section 7.2 (a PermissionError from the access seam is a CONTRACT defect and
should fail the consumer loudly rather than DLQ every event forever). That
finding is correct; the quarantine router is owned by OMN-16798 (#2949, #2951,
landed 2026-08-27), so this suite PINS the current behavior instead, making that
change a visible diff when someone makes it rather than a silent one.
Evidence: uv run pytest tests/integration/chains/ -q -> 9 passed
Evidence-Ticket: OMN-16814
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 131 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 (1)
Comment |
|
| 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)
#7357) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2953 * evidence: OCC companion self-bind for #7357 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
OMN-16814 — drive the projection capability seam through a real chain
OMN-16690:
node_hook_event_capture/contract.yamldeclaredaccess: writewhile its handler calleddb.query(). The runtime read seam refused, the projection arm swallowed thePermissionErrorand DLQ'd the event, callers kept seeing202, and the quarantine sink reached HWM 8,878,932. It ran about a week behind green CI.The platform testing inventory (§7.1) names why nothing caught it:
That is equally true in this repo:
tests/integration/runtime/test_projection_handler_db_injection_integration.pyinjects aFakeDbthrough_build_projection_db_adapterwhoseupsert/querydo no access checking, so it is green for every declaredaccessvalue.omnimarket#2164closed the statically-visible half. It cannot see a read reached through a helper, a base class, or an adapter — and nothing anywhere proved the runtime refusal is still wired, or that a mismatch turns a whole chain red.Where the seam boundary is drawn
_assert_read_declared/_assert_write_declaredboth fire before the adapter touches a connection, so the whole capability decision is reachable with zero DB contact.Real:
_prepare_handler_wiringarm selection ·_make_projection_dispatch_callback·_build_projection_db_adapter·ProjectionDatabaseOperationsdomain routing ·ProjectionTableOperationassertions and SQL construction.Substituted: the
psycopg2driver only, viasys.modules— the techniquetest_sync_psycopg2_adapter_preserves_text_array_listsalready uses here. Substituting the adapter object is the fidelity gap being closed; please don't "simplify" this file back into that.The matrix — both directions
accessread_writequeryread_writeupsertwritequeryreadupsertOnly
accessvaries. Same handler, same wiring, same wire bytes, same table — so any divergence is attributable to the capability seam and nothing else. A one-sided fixture could pass by accident, which is why both directions are present (AC4).Each refused row asserts three facts, because any one alone is weak:
PermissionError— bus-observable, not log-only; log-only detection is what let the live defect run a week;RED-first proof — and the assertion it corrected
Stubbing both production assertions to
lambda self: Nonedoes not make thePermissionErrordisappear. The chain then fails a second, independent refusal:because a
write-declared table has no read binding to resolve either.So an assertion of the form
"PermissionError" in dlq_textstays GREEN with the capability seam switched off, and would gate nothing. Asserting the seam's own wording is what makes these rows non-vacuous. Measured, then recorded at the assertion site so nobody relaxes it later:AC3 also anchors by importing
ProjectionTableOperationand asserting both refusal methods exist on the production class.omnimarket/.../test_projection_live_events.py:38re-implementsaccess not in {"read", "read_write"}and asserts its own copy — it was green throughout the outage, because a mirror of a rule cannot tell you the rule is still wired.Two fixture facts that are load-bearing, both found by measurement
projection_watermarks, a real table from the shipped local topology. A fabricated name cannot reach the capability seam at all —_require_projection_binding_privilegesrejects it three layers earlier, and the only sanctioned grant-synthesis helper refuses for already-shipped tables. A fabricated row would have proven nothing aboutaccess.terminal_eventand list it inpublish_topics(handler_wiring.py:8486-8492) or the arm emits no terminal — which would make a positive row's observation identical to a refused row's, i.e. vacuous.Both are recorded as comments at the point of use, and the harness carries explicit precondition assertions so a future core pin that parses
db_iooraccessaway fails loudly rather than banking a free green.Enforcement, not detection (rule 5)
No workflow edit. The
Event Chain Gatejob already runsuv run pytest tests/integration/chains/wholesale and is registered inscripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS, so askipped/absent conclusion failsCI Summaryclosed.CI Summaryisdev's only required check.Non-goals
No runtime behavior change. In particular this does not implement inventory §7.2 — that a
PermissionErrorfrom the access seam is a contract defect and should fail the consumer loudly rather than DLQ every event forever. That finding is correct, but the quarantine router is owned by OMN-16798 (#2949, #2951, landed 2026-08-27). This suite pins the current behavior instead, so that change is a visible diff when someone makes it rather than a silent one.Acceptance criteria
ProjectionTableOperation, chain terminalizes whenaccessadmits: METRelated: #2952 (OMN-16813) is the sibling chain surface from the same inventory — separate files, no overlap.
Evidence:
uv run pytest tests/integration/chains/ -q→ 9 passedEvidence-Ticket: OMN-16814
Evidence-Source: OCC#7357