Repository navigation
fix(OMN-20159): the projection read node gets its own binding, so a reader never becomes the claim store's login - #3364
Conversation
…from the delegate claim store's One overlay variable selected both the read node's database and the delegate-skill claim store's. When a lane gave its runtime-effects a read binding that logs in as the dashboard reader, the claim store followed it and every bus delegation failed writing its claim with the reader's credentials (UndefinedTable on delegate_skill_command_claims). The read path now has its own variable, OMNIMARKET_PROJECTION_READ_BINDING_OVERLAY, read only in projection/runner.py through projection_read_binding_from_overlay_env(): the read overlay's binding when that variable is set, otherwise the runtime binding exactly as before. Only the two read resolvers follow it, the projection read node and the local dashboard. The claim store and the evidence store are unchanged and keep following the runtime binding, so a reader's credentials can never become their write principal. A read variable that is set but names a missing or invalid file raises; it never falls back to the runtime binding. A blank value is unset. The unconfigured refusal names both variables, and a refusal about the read binding's database no longer blames the runtime binding. No contract, topic, migration or schema changes, and no lane changes behaviour until one sets the new variable. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…d-binding-overlay The branch sat on 0.4.286, which is now published, so the release-identity gate refuses any further change to packaged source on top of it. dev carries 0.4.287. No file of this branch's change conflicts with dev's. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…ters are held to the runtime binding A read overlay that is set but does not load (missing, a directory, not a mapping, no database, invalid YAML) made the read node's handle() raise the loader's own exception, because the handler answers only ProjectionReadError. projection_read_binding_from_overlay_env() now raises ProjectionReadBindingOverlayError, a RuntimeError naming the variable, the path and the cause's type only, never the file's contents: a validation error quotes its input, which can be a database URL with its password. The read node answers it with the refusal projection_binding_invalid; the local dashboard surfaces it as it is. It still never falls back to the runtime binding, and with the read variable unset the read path is the runtime path exactly. The refusals about the binding's database name the file the binding came from, in place of "the read overlay's, else the runtime overlay's". New tests: a projection writer (BaseProjectionRunner) never takes the read overlay's binding or consumer group; a broken runtime overlay does not reach reads while a read overlay is set; the environment scan also finds a settings field named like the variable, a second name the runner binds to its constant (plain, annotated or chained) and the constant's name as a string. The real-PostgreSQL test skips with the wording CI's skip guard recognises, and each of its cleanup statements runs even when an earlier one fails. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
The reader role is granted CONNECT on its database explicitly, and the
grant is revoked before the role is dropped. The lakshman lane's
omnibase_infra revokes CONNECT from PUBLIC (datacl {=T/postgres,...}),
so the role could not log in and the test failed before it reached its
claim assertions (2026-10-04T06:56Z).
The local claim file is compared with the claim module's own default,
not with a path under the test's state root. The suite pins that file
once per session (tests/conftest.py, _session_delegation_claim_store),
so the old assertion could never hold inside the suite (07:01Z).
Against the lakshman lane's Postgres at 07:03Z: 1 passed in 26.18 s,
0 roles and 0 schemas left behind.
Onex-Lane: omn20159-read-binding
Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…d-binding-overlay dev (2332f6f) is at 0.4.288. This branch sat on 0.4.287, which is now released, and the release-identity check refuses packaged-source changes on a released version. No file conflicted, and dev touches none of the four source files this branch changes. Merged tree: the contract check files 53 passed; tests/unit/nodes/node_projection_read_effect, tests/unit/nodes/node_delegate_skill_orchestrator, tests/projection/test_api_server_local_mode.py and tests/unit/projection 837 passed, 5 skipped. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…dence The repo evidence gate (receipt-gate 44aa5aeb) re-runs as a merge-base control only the dod_evidence a pull request adds or changes, and omnimarket had no contract for this ticket yet. contracts/OMN-20159.yaml binds AC2's clause that the node takes its database handle from the runtime's binding, not from a DSN the handler reads, to three unit files. At the merge base 2332f6f, with this branch's tests laid on top, they fail (27 of 53 tests): the read resolver does not exist there and the read node follows the runtime overlay only. On this branch all 53 pass. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
|
| 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_local_dashboard_serve_effect/handlers/handler_local_dashboard_serve.py:227: handler=HandlerProjectionRead(topic_map=topics, row_source=source),\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) |
| await handler.close() | ||
|
|
||
|
|
||
| def _raised(call: Any) -> BaseException: |
| pytest.fail(f"{call.__name__} raised nothing") | ||
|
|
||
|
|
||
| def _returned(call: Any) -> Any: |
| pytest.fail(f"{call.__name__} raised {type(exc).__name__}") | ||
|
|
||
|
|
||
| async def _answer_from_the_node() -> Any: |
CodeQL on #3364 reported "Local variable 'conn' may be used before it is initialized" at the fixture's connect: pytest.skip() raises, but static analysis cannot see that, so it read conn as possibly unset below. The except branch now raises pytest.skip.Exception itself, which is what pytest.skip does, with the same message the integration skip guard matches. Against the lakshman lane's Postgres at 08:12Z: 1 passed in 22.06 s, 0 roles and 0 schemas left behind. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…overlay' into lakshman/OMN-20159-read-binding-overlay The OCC writer bot merged dev (7960513) into this branch on GitHub at 08:23Z, after the pull request opened. Taking that merge in changes only pyproject.toml and uv.lock: the post-release bump to 0.4.289, which is unreleased, so the release-identity gate passes. No file conflicted. Merged tree: this change's contract check, 53 passed. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
…3364 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 1b92bb879d1aab72a5dd7fd248b26e645c762876.
…nimarket#3364 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/37189043654
…nimarket#3364 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/37190086322
…aracter limit contract-validation on #3364 refused contracts/OMN-20159.yaml: the dod_evidence id dod-omn20159-ac2-read-binding-apart-from-claim-store is 52 characters and the ticket contract allows 50. It is now dod-omn20159-ac2-read-binding-apart-from-claims (47). The check, its binding and its description are unchanged. onex_change_control's validate-yaml at the pinned 802b5f710, run locally: the old file fails with string_too_long, the new one is valid. Onex-Lane: omn20159-read-binding Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
) * evidence(OMN-20159): author OCC companion for OmniNode-ai/omnimarket#3364 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 1b92bb879d1aab72a5dd7fd248b26e645c762876. * evidence(OMN-20159): self-bind OCC#12753 + rebind contract_sha256 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#3364 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/37189043654 * evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#3364 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/37190086322 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai> Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
…d-binding-overlay # Conflicts: # tests/projection/test_api_server_local_mode.py
|
OCC autobind rebound this PR's evidence-source stamp line to The receipt gate's own eligibility validator, run against the change-control tree the gate pins for Reported by |
|
Thanks, Lakshman — omnimarket#3364 for OMN-20159 was squash-merged as 97a7522 on 2026-10-04T23:33:10Z. The landing lane merged the base branch into your pull request branch to resolve a conflict in a test file docstring with a just-merged sibling pull request (no rebase, no force-push), and it landed under the operator ruling of 2026-10-04T23:07:50Z, which overrode the report-only landing policy for the collaborator pull requests listed in the standups ask. |
|
Close-out for OMN-20159: squash-merged as What landed: the projection read node has its own binding variable, so a read binding no longer moves the delegate claim store to Postgres under the reader's login (the dev-lane failure in the 2026-10-02 comment on the ticket). How it was verified: 150 passed, 0 failed at The four CodeQL notes, answered:
What is left: omnibase_infra#4556 (merged since), and the dev lane's own binding, which stays with your lane (#4481's test says when to flip it back). Thank you! |
…-row The branch sat on #3368's head d7153cf, 21 commits behind dev. Dev changed port_local_delegation_dispatch.py (#3390, #3382) and the projection read handler (#3364, #3363), both of which this change uses; the merge is textually clean, and the metering, dispatch and projection-read suites were re-run on the merged tree. #3368 is still unmerged, so its six files stay on this branch unchanged.
Summary
For OMN-20159. One variable,
OMNIMARKET_PROJECTION_RUNTIME_BINDING_OVERLAY, chose the database for two things: the projection read node and the delegate-skill claim store. When the dev lane's runtime-effects got a read binding that logs in asrole_omnidash, the claim store followed it, and every bus delegation then failed writing claims (UndefinedTable: relation "delegate_skill_command_claims" does not exist). omnibase_infra#4481 took the binding back off. Its test says to change it once the claim store gets its write principal separately from the read binding. This change makes that split. The read node gets its own variable,OMNIMARKET_PROJECTION_READ_BINDING_OVERLAY. The claim store and the evidence store keep following the runtime binding only, so a reader's credentials never become their write principal.What changed
src/omnimarket/projection/runner.py: a new variableOMNIMARKET_PROJECTION_READ_BINDING_OVERLAYandprojection_read_binding_from_overlay_env(). It returns the read overlay's binding when the variable is set, and otherwise the runtime binding exactly as before. A blank value counts as unset. A read variable that names a missing or invalid file raisesProjectionReadBindingOverlayError, aRuntimeErrorthat names the variable, the path and the error's class, never the file's contents. It never falls back to the runtime binding.node_projection_read_effect/ports/read_source_resolution.py: the read node resolves its database through the read resolver. A broken read overlay is the refusalprojection_binding_invalid. The unconfigured refusal names both variables. A refusal about the read binding's database names the file that binding came from.node_local_dashboard_serve_effect/handlers/handler_local_dashboard_serve.py: the local dashboard picks its store with the read resolver.node_projection_read_effect/handlers/handler_projection_read.py: docstrings only.tests/conftest.py: the autouse fixture also clears the new variable.contracts/OMN-20159.yaml: this change's evidence item (below).Deliberately not changed: the claim store, the evidence store and the projection writers. They still resolve the runtime binding only: the stores through
projection_runtime_binding_from_overlay_env(), the writers through the runner's internal_projection_runtime_binding_from_overlay_env(). No node contract, topic, migration or schema change. No lane changes behaviour until it sets the new variable. Dev is merged in (2332f6fd7, then the bot's update to796051348, 0.4.289), because 0.4.287 and 0.4.288 are released; dev touches none of the four source files above.Rollout note: when a lane moves runtime-effects from the runtime binding to the read binding, its claim store moves back to the container's SQLite file. A delivery in flight at that moment could be claimed in both stores.
How it was verified
The evidence item in
contracts/OMN-20159.yamlruns three unit files: the newtests/unit/nodes/node_projection_read_effect/test_read_binding_apart_from_claim_store_omn20159.py(F1 to F5), the existingtest_projection_read_effect_omn20159.py(it holds the scan that no module of the node reads the environment), and the newtests/unit/projection/test_projection_binding_env_discipline_omn20159.py(F6).2332f6fd7, with this branch'stests/laid over it: 27 failed, 26 passed. At the earlier merge base504d0028bthe same 27 failures were 15AttributeError(the base runner has noprojection_read_binding_from_overlay_env), 5 assertion failures and 7DID NOT RAISEor "raised nothing", with no collection errors.The local-mode file
tests/projection/test_api_server_local_mode.pyholds F7 (the dashboard reads through the read binding alone) and F5 on the dashboard. At the merge base504d0028bwith this branch'stests/laid over it, 5 of its 30 tests fail; on this branch all 30 pass.The failure modes the tests are written against:
projection_binding_unconfigured, and the claim store still uses its state-root SQLite file.projection/runner.pyreads either variable. A positive control plants a reader by each route the scan covers.Related suites, never an integration-marked test (
tests/unit/nodes/node_projection_read_effect,tests/unit/nodes/node_delegate_skill_orchestrator,tests/projection/test_api_server_local_mode.py,tests/unit/projection): on this branch merged with dev2332f6fd7, 837 passed, 5 skipped, 0 failed. On an export of0bbc41afabefore the merge, those four plustests/test_projection_runner.pyandtests/test_delegation_env_scanner.pygave 895 passed, 5 skipped, 0 failed. On03bfea107(source identical to0bbc41afa),ruff checkpasses,ruff formatreports every file formatted, andmypy src/omnimarketexits 0. Mutations of the change, made before the final source commit (at41f2629b6), each turned at least one test red; they were not repeated on0bbc41afa.The real-Postgres F1 test (
tests/integration/nodes/node_projection_read_effect/test_read_binding_reader_role_real_postgres_omn20159.py) creates a temporary login role and two scratch schemas. Its first two runs against the lakshman lane's PostgreSQL on .201 failed for reasons in the test itself. At 06:56:57Z the role could not log in:permission denied for database "omnibase_infra",User does not have CONNECT privilege, because that server revokes CONNECT from PUBLIC. At 07:01:25Z the last assertion failed, because it expected the local claim file under the test's state root while the suite pins that file once per session. Commit03bfea107changes only this integration test: it grants CONNECT to the scratch role and revokes it before the role is dropped, and its last assertion now compares the local claim file with the claim module's own default path. At 07:03:11Z, with those fixes, it gave 1 passed in 26.18 s, and 0 roles and 0 schemas were left behind. CodeQL on this PR then reported that the fixture'sconnmay be used before it is set: it cannot see thatpytest.skipraises. Commit451a0b49fraisespytest.skip.Exceptionitself, with the same message the integration skip guard matches, and at 08:12:24Z the test gave 1 passed in 22.06 s, again with nothing left behind.Lab readback, lakshman lane on .201, 2026-10-04T06:48Z to 06:54Z, real data only (the ticket's probe shape, with runtime-effects carrying only the read variable). runtime-effects ran an image built from
0bbc41afa(omnimarket 0.4.287). A lab-only override gave it onlyOMNIMARKET_PROJECTION_READ_BINDING_OVERLAY(the dev-lane shape). It was healthy with 0 restarts and 0 import errors at 06:50:18Z, and migration 0053's two columns were present.delegation.decisions.v1matched the projection API for the same tenant. At limit 50 both returned 50 rows with the same correlation ids. At limit 500 both returned 92 rows with the same correlation ids. The tenant had 92 rows.de3fd64a-1568-4dc5-8d14-8ff551d6a951, status success, a real Qwen3.8-27B writer row) was claimed in the container's SQLite store (1 row). It was not claimed in Postgres (0 rows), and the Postgres claims total stayed at 69 while the lane was switched.9245e8e873d9with its original compose files. Its config hash was18d5df85e3fa…, the same as before the switch. It was healthy with 0 restarts and only the runtime overlay set. Only runtime-effects was recreated in this window.e2206873-910e-45b0-bb84-d227dc9e5335, against 47 Postgres claims. Its readback records that no row was seeded and no database was written by hand.Caveat: the tenant had 92 rows and the served window is 2000 rows (limit 500 × 4), so this readback cannot show whether the window is right at volume. This branch does not contain the OMN-19971 window fix, and this readback is not evidence for it.
Failure paths
ok: falsewithprojection_binding_invalid. The refusal names the variable and the path, never the file's contents, and the node never falls back to the runtime binding. The local dashboard stops at start-up withProjectionReadBindingOverlayError.projection_binding_unconfigured, and the refusal names both variables.projection_database_unavailable; this change does not alter that path.ok: falsewith a named error), never a partial one.Open defects
none
Local gates
--no-verify, noSKIP=, no--no-gpg-sign, nocore.hooksPathoverride. Every pre-commit and pre-push hook ran.Overlapping open PRs
Overlap-Reviewed: #3319 — it also edits
tests/projection/test_api_server_local_mode.py(OMN-19976), and both start from dev's same version of that file (blob0f123312f). A three-way merge of the two versions has one conflict, in the module docstring, where each PR adds its own bullet to the same list; keep both bullets. Their source changes are in different modules (sqlite_database.pythere; the runner and the read resolvers here). The other 7 open PRs (read 2026-10-04T07:51Z) touch none of these files.Not in this change
TableRowSourcelives.role_omnidashor another reader.Evidence-Ticket: OMN-20159
Evidence-Source: OCC#12807