Repository navigation
feat(OMN-16964): create the ledger_chain relation and point chain-canary link 5 at it by name - #3381
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 8
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 0
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings demoted from threads (anchor rejected)
-
[CRITICAL] hostile-reviewer (glm-review)
No writer for ledger_chain exists; the migration cannot produce a passing link 5 | The migration's own header states that the string 'ledger_chain' has zero references across omnibase_infra, omnibase_core, omnimarket and omnimemory outside the canary reader. This diff creates the relation and grants read access, but lands no producer. Even once the omnimarket overlay declares ledger_readback.dsn_env and the secret is injected, the leg will query an empty table, find no rows for the correlation id, and repor
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Direct asyncpg transport tests now pass an env var NAME where a DSN is required | test_close_failure_does_not_replace_the_replay_result and test_connect_and_query_share_one_deadline call _replay_ledger_chain_via_asyncpg with _LEDGER_SOURCE_ENV (a variable NAME) as the first argument. The function's signature takes a DSN and connects to it; a name like CHAIN_CANARY_LEDGER_DSN_FOR_TESTS is not a connection string. The tests only pass because _FakeAsyncpg accepts anything, so they no longer exercise the real D
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Workflow depends on a cross-repo overlay change not present in this diff | The new 'Resolve the declared ledger-readback DSN reference' step loads ledger_readback from .ci-bus-overlay/config/ci_bus_lanes.yaml, which lives in the omnimarket repo. No overlay change is included here, so until the sibling PR merges, every canary run hits the ValueError path, warns, and exits 0. The comment acknowledges the contract but nothing gates the merge order; the guard test in test_lane_ledger_readback.py skips when the
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Expected hop topology duplicated between workflow YAML and node contracts | EXPECTED_LEDGER_HOPS hardcodes four topic names in the workflow file, with the comment claiming they come 'per the node contracts that publish them'. Nothing in this diff derives them from those contracts or cross-checks them. A topic rename in a publishing contract silently leaves the workflow declaring a stale chain; link 5 then reports CHAIN_INCOMPLETE against a complete chain, exactly the failure mode the validator for expected_
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
No test covers _decide for the new REFUSED verdict end-to-end | The new EnumChainCanaryVerdict.LEDGER_REPLAY_REFUSED branch in _decide is only exercised via _link_five directly in test_a_refused_leg_is_not_a_passing_link_five. The full handle() path from a DSN-shaped argv to a receipt carrying ledger_replay_refused is untested, including the interaction ordering against a failing link 2 and against quarantine failures. The mirroring projection leg presumably has such coverage; symmetry is not established he
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Migration guard catches only insufficient_privilege | The DO block's inner EXCEPTION handler catches insufficient_privilege alone. Other failures inside the GRANT block, such as undefined_schema if a lane's public schema differs or a future rename of the ledger_chain relation by a concurrent migration, will abort the whole migration rather than degrade to the NOTICE-and-skip contract the surrounding comments promise. The stated contract is 'skip with a NOTICE, canary reports SKIPPED_NOT_CONFIGURED', but onl
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
New tests reach into private handler internals via asyncio.run inside sync tests | test_an_unset_variable_makes_no_claim_about_link_five and test_a_dsn_on_any_flag_refuses_the_leg_without_echoing_it call handler._replay_ledger directly with asyncio.run inside unit tests. This couples the tests to a private method name and to a manually constructed call signature, and skips the handler's own orchestration; a rename or signature change to _replay_ledger breaks these tests even if public behavior is unchanged.
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
GITHUB_ENV append assumes single-line, non-conflicting values | The workflow step writes f'{key}={value}\n' lines without validating that the value contains no newline or '=' characters. The dsn_env value is constrained to a POSIX name by the loader, so this is currently safe, but the pattern is copied from the projection step and neither guards against a future declaration field being exported here. A multi-line value in GITHUB_ENV is interpreted as a multiline variable and could overwrite or bleed into su
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 3
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 0
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[MAJOR] hostile-reviewer (glm-review)
Transport tests now pass an env NAME where a DSN is required | Two tests in test_handler_chain_canary_ledger.py were mechanically renamed from _LEDGER_SOURCE to _LEDGER_SOURCE_ENV as the first positional argument to _replay_ledger_chain_via_asyncpg. That function's signature is (dsn, correlation_id, window_s); it expects a connection string, not a variable name. The fake asyncpg presumably does not parse the DSN, so the tests still pass while exercising a nonsensical input. The rename conflates the model-le
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
No cross-layer test that workflow hop topics match node hop vocabulary | The workflow hardcodes EXPECTED_LEDGER_HOPS as four topic strings, while unit tests stub the ledger replay with hops like ('received', 'routed', 'inference_completed', 'terminal'). Nothing in this diff asserts that the producer's hop values (once a writer exists) equal the topic strings declared in the workflow. The header comment itself admits the list is transcribed 'per the node contracts that publish them', which is exactly the kin
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
REFUSED status absent from the never-disagree parametrization | test_success_and_link_five_can_never_disagree parametrizes over ERROR, verifier skip/fail, short chains and empty source, but not the new REFUSED status. The regression class that test exists to catch (a new ledger status or _decide branch quietly rendering green) applies equally to REFUSED. The REFUSED branches in _decide and _link_five are only covered by a direct call to _link_five, not through handle(), so the scalar-verdict path for REFUSE
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Ledger resolution step swallows all ValueError causes as benign | The new workflow step catches ValueError, prints a warning, and exits 0. The loader raises ValueError for both 'lane legitimately declares nothing' and for refusal conditions (disallowed lane, DSN pasted into dsn_env, non-POSIX name, malformed overlay). Conflating misconfiguration that indicates a leaked credential attempt in committed YAML with a benign absent declaration reduces both to a warning line in a long log. The comment frames only
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Sibling retirement test asserting the stream tops out at 103 is unmodified | The diff narrows test_surviving_stream_is_byte_identical_to_the_pre_3190_stream to ordinals <= 104 precisely because adding 105 broke whole-directory fingerprinting. The immediately preceding test, test_stream_tops_out_at_103_until_the_reissue, appears in the diff context as unchanged. Its name and stated purpose assert no migration above 103 until the reissue; landing 105 violates that literal condition. Either the test was update
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
No writer for ledger_chain; link 5 remains structurally red | The migration creates and grants ledger_chain, but nothing in this change writes rows: no publisher, no writer grant, no INSERT path. The canary's replay query against an empty table yields zero hops, which the new EXPECTED_LEDGER_HOPS logic classifies as CHAIN_INCOMPLETE. The header acknowledges the link-2 grant gap is closed 'by half' but does not acknowledge that link 5 itself remains non-dischargeable after this change. The elaborate consumer
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
CREATE TABLE IF NOT EXISTS can mask a divergent pre-existing table | 105 uses CREATE TABLE IF NOT EXISTS, so on a lane where a same-named relation exists with an incompatible shape (different PK, missing columns), the statement is a no-op and the subsequent column-scoped GRANT either fails confusingly or succeeds against a table the consumer's SELECT cannot actually satisfy. The idempotency rationale in the header covers warm-volume reconciliation but not shape divergence. | Evidence: CREATE TABLE IF NOT EX
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 3
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 2
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[CRITICAL] hostile-reviewer (glm-review)
Ledger DSN secret is never actually injected into the probe step | The workflow comments assert the DSN is injected as a job secret under the lane-declared name, but the probe step's env block adds no such mapping. GitHub Actions cannot perform dynamic secret lookup (secrets[env] is unavailable in expressions), so unless the lane's declared dsn_env happens to equal CHAIN_CANARY_PROJECTION_DSN and the existing static secret entry is reused, the resolved variable name will point at an unset environment variab
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Tests pass a variable NAME into a function that takes a DSN | _replay_ledger_chain_via_asyncpg's first parameter is a Postgres DSN consumed by asyncpg.connect. Two tests in the ledger suite were mechanically renamed from _LEDGER_SOURCE to _LEDGER_SOURCE_ENV, so the transport now receives the literal string 'CHAIN_CANARY_LEDGER_DSN_FOR_TESTS'. The tests only pass because the asyncpg module is faked; the fakes evidently accept any string as a connection target. This masks exactly the class of bug the migratio
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
No unit test covers the new ValueError-swallowing workflow path | The new resolve step deliberately catches ValueError and exits 0 so unresolvable lanes degrade to a warning. That branch is the one place where a mistake in _load_lane_dsn_declaration's error taxonomy (e.g. a new exception type like OSError or yaml.YAMLError from a malformed overlay) would crash the job instead of warning. The refactor widened the exception surface of the shared loader, and the workflow is the only caller relying on 'ValueErr
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Stale GITHUB_ENV export can survive a failed lane resolution | The resolve step appends CHAIN_CANARY_LEDGER_DSN_ENV to GITHUB_ENV only on success. If the variable were previously exported in the same job (reusable workflow composition, or a retry within one job after a partial GITHUB_ENV write), a lane that later fails to resolve warns and exits 0 while the stale name persists, pointing the probe at a declaration the lane no longer makes. The 'distinct from the projection one' comment addresses cross-leg co
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
ledger_chain has no writer in this change | The migration creates the relation, the workflow declares expected hops, and the consumer reads the relation, but no producer of ledger_chain rows exists anywhere in the diff or, per the migration's own grep evidence, anywhere in the four sibling repos. On the next canary run the SELECT returns zero rows for every correlation id, link 5 reports CHAIN_INCOMPLETE, and the migration header's framing ('this migration creates the relation' closing the gap) overstates w
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Unrelated live prod drift committed into the snapshot artifact | The census-snapshot.json change carries the prod container_absent critical finding (pre-existing per the prior revision), two new dev unexpected_container findings, and a fresh emitted_at timestamp, none of which relate to OMN-16964. Committing a regenerated live-state artifact inside an unrelated PR entangles the drift history with this change and makes bisection of census regressions harder; if the regen was incidental, the diff now also cer
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
New tests exercise private methods directly | test_an_unset_variable_makes_no_claim_about_link_five and test_a_dsn_on_any_flag_refuses_the_leg_without_echoing_it call handler._replay_ledger and _link_five directly, bypassing handle(). This verifies the branch logic in isolation but not that _decide maps REFUSED to LEDGER_REPLAY_REFUSED through the real entry point, and it couples tests to private names that the diff itself just reshaped. | Evidence: status, detail = asyncio.run(
handler._replay_ledger(rResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
#8874) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3381 * evidence: OCC companion self-bind for #8874 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 7
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Tests pass an env-var NAME as a DSN to the raw asyncpg transport | The rename of _LEDGER_SOURCE to _LEDGER_SOURCE_ENV was applied mechanically to two call sites that pass the value directly to _replay_ledger_chain_via_asyncpg, whose first parameter is a Postgres DSN. The new value, 'CHAIN_CANARY_LEDGER_DSN_FOR_TESTS', is not a DSN; it is the NAME of an environment variable that is never set in these tests. The fake asyncpg in test_close_failure_does_not_replace_the_replay_result parses the connection string
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Migration uses IF NOT EXISTS with no shape verification on pre-existing tables | On a lane where a divergent ledger_chain table already exists (hand-created during development, or created by an out-of-band script), CREATE TABLE IF NOT EXISTS silently no-ops. The consumer then queries columns that may not exist, and the CHECK constraint and column-scoped grant never get applied, producing an ERROR at read time that will be diagnosed as a canary wiring problem rather than schema drift. The migration header is
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
DO block can leave a partial grant state on non-insufficient_privilege errors | The EXCEPTION handler catches only insufficient_privilege. If GRANT USAGE succeeds but the column-scoped GRANT SELECT fails with a different condition (e.g. undefined_column after drift, or a syntax-level condition on older Postgres), the block propagates and aborts the migration mid-grant, leaving USAGE granted and SELECT refused with no NOTICE explaining the asymmetry. The fall-through contract described in the header ('fails
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Handler couples link-5 refusal to process-global sys.argv | _replay_ledger consults dsn_shaped_argv_flags(sys.argv) inside library code. This makes the transport behavior dependent on process-global state that is meaningless when the node is invoked in-process (contract-driven construction, test harnesses, any future embedding). The projection leg already makes the same check, so the pattern is established, but replicating it extends the coupling rather than extracting it. A caller embedding the handler beh
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Expected hop topology duplicated between workflow and node contracts | EXPECTED_LEDGER_HOPS is hard-coded as a multi-line workflow env literal with the comment 'per the node contracts that publish them'. Nothing in this diff or its tests verifies that the four topics in the workflow match the topics the node contracts actually publish, or that the ordering matches hop_index semantics. A topic rename in a contract will silently desynchronize, and the canary will report CHAIN_INCOMPLETE against a chain that i
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Migration ordinal scoping test silently weakens the byte-identity guarantee for future retirements | The narrowed include predicate is hard-coded to ordinal <= 104. When the next migration is retired and reissued (the retirement record contemplates this), this test must be edited again, and the edit will look like the exact 'conflation' the new docstring warns against unless the bound is parameterized. The predicate also silently excludes any file whose name has no parseable ordinal, so a stray 9999_notes.s
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Cross-repo overlay agreement test skips in the environment that most needs it | test_the_repo_declaration_resolves_when_the_overlay_is_present asserts the lane declaration resolves against a sibling omnimarket checkout, but skips whenever that checkout is absent, and its own docstring admits the real assertion happens 'in CI where both checkouts exist'. No such CI assertion appears in this diff. The workflow resolves the overlay from .ci-bus-overlay/config/ci_bus_lanes.yaml at run time; if the omnimarket si
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 4
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 3
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings not anchored to a changed file
-
[MINOR] hostile-reviewer (glm-review)
Ledger DSN secret is referenced but never provisioned in the workflow | The resolve step exports only the NAME via ledger_readback_env, and the comment asserts the DSN is 'injected as a secret on the probe step under that same name', while admitting 'for this leg no NEW secret exists'. The probe step's env block shown in the diff contains no mapping from the declared dsn_env to a secrets context for the ledger leg. If the repo's lane overlay and the GitHub secret happen to share a name today this works by c
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Consumer-contract columns have no test pinning reader query to schema | The migration header states the five columns 'are a contract with the merged consumer above and may not be renamed', but the only enforcement is prose. There is no test asserting that the SELECT in _replay_ledger_chain_via_asyncpg names exactly columns that migration 105 creates, nor that a mismatch fails the migration rather than the canary. | Evidence: -- Consumer-contract columns (renaming any of these breaks a merged reader):
-- cResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Tests pass an env var NAME into the asyncpg transport as a DSN | The mechanical rename converted direct calls to _replay_ledger_chain_via_asyncpg to pass _LEDGER_SOURCE_ENV, which is an environment variable NAME ('CHAIN_CANARY_LEDGER_DSN_FOR_TESTS'), as the function's DSN parameter. The whole point of this change is that the transport receives a resolved DSN string, never a name. The tests only pass because a fake asyncpg module accepts any string; if these tests ever run against the real transport (or the
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
CREATE TABLE IF NOT EXISTS skips drift detection on divergent schemas | Migration 105 uses IF NOT EXISTS, so on lanes where a stale or hand-created ledger_chain exists with divergent columns (e.g. missing the CHECK constraint or a consumer column), the migration no-ops and the merged reader's SELECT fails at runtime with undefined-column, reported as LEDGER_REPLAY_UNREADABLE. The header explicitly says five columns are a rename-proof contract, yet nothing verifies the existing relation satisfies it. | Evide
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Ledger argv refusal fires on any DSN-shaped value anywhere in argv | dsn_shaped_argv_flags(sys.argv) scans the entire process argv. A DSN-shaped string legitimately present for another purpose (for example a diagnostic flag, or a probe URL whose query string happens to parse as a DSN) now refuses link 5 even though no credential was exposed via this leg. The refusal is fail-closed, so impact is a spurious red, but there is no test covering a false-positive argv and no way to scope the check to flags this no
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Cross-repo overlay agreement test skips silently outside CI | test_the_repo_declaration_resolves_when_the_overlay_is_present skips whenever the sibling omnimarket checkout is absent, so locally and in any CI job without both clones the only check that the dev lane's declared dsn_env matches what the workflow expects (and that a secret exists under it) never runs. A typo in the overlay's declared name ships green and surfaces as a permanent NOT_CONFIGURED. | Evidence: if not overlay.is_file():
pytest.skiResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 7
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 1
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings demoted from threads (anchor rejected)
-
[CRITICAL] hostile-reviewer (glm-review)
No writer for ledger_chain; the canary will still report red | The migration creates the relation and the reader queries it, but nothing in this diff, and per the migration's own comment nothing anywhere in the repo, writes rows to ledger_chain. The migration header states the string ledger_chain has zero references outside the canary node. After this change a fully configured run will query an empty table, find zero hops against EXPECTED_LEDGER_HOPS, and report CHAIN_INCOMPLETE on every run. The workflow c
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
Probe step never injects the ledger DSN value into the environment | The resolve step exports only the NAME via CHAIN_CANARY_LEDGER_DSN_ENV, and the comment asserts the value is "injected as a secret on the probe step under that same name". The probe step's env block in this diff adds no such secret mapping and declares "for this leg no NEW secret exists" on the grounds that both relations share one database and reader. The projection leg's secret (CHAIN_CANARY_PROJECTION_DSN) covers the projection DSN name
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
argv DSN refusal is bypassed when the leg is unconfigured | _replay_ledger returns SKIPPED_NOT_CONFIGURED on an empty ledger_source_env before inspecting argv. A run wired with a DSN on some other flag but no declared ledger name reports the benign not-configured status and never surfaces the argv exposure the refusal exists to catch. The disclosure condition and the configuration condition are checked in the wrong order relative to the feature's stated intent. | Evidence: if not declared_name: return ... S
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Transport tests now pass a variable NAME where a DSN is expected | test_close_failure_does_not_replace_the_replay_result and test_connect_and_query_share_one_deadline call _replay_ledger_chain_via_asyncpg with _LEDGER_SOURCE_ENV (a name like CHAIN_CANARY_LEDGER_DSN_FOR_TESTS) as the dsn parameter. The fake asyncpg presumably ignores the value, so the tests no longer exercise DSN parsing or connection-string handling at all and would pass with any garbage in that argument. The parameter's contract is degrade
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Fingerprint scope: PRE_3190 count arithmetic must be verified against the include predicate | The narrowed assertion fingerprints only ordinals <= 104 and compares against PRE_3190_STREAM_SHA256 / PRE_3190_STREAM_FILE_COUNT. If the pre-3190 stamped count included the retired 104 file, the predicate now excludes it and the count comparison fails or, worse, the digest comparison was over a different corpus all along. The docstring asserts 'the same stamped digest, the same file count' without showing that the
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
ledger_chain has no retention or pruning strategy | Every canary run writes at least four rows keyed by a fresh correlation_id into a table whose PK leads with unbounded TEXT and which has no TTL, partitioning, or documented purge job. A scheduled canary is exactly the workload that accumulates a diagnostic ledger monotonically. The migration's own header is careful about index cost but silent on growth. | Evidence: CONSTRAINT pk_ledger_chain PRIMARY KEY (correlation_id, hop_index) ... correlation_id TEXT N
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Historic events carrying ledger_source fail model revalidation | ledger_source is removed with no alias. Any persisted envelope, receipt, or replayed event that serialised a ModelChainCanaryRequest before this change contains ledger_source; pydantic default extra handling aside, deserialisation of archived payloads through this model will not map the old field, and the new required semantics of ledger_source_env mean old receipts cannot round-trip. The claim 'no caller can be broken' covers CLI callers only
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
… it by name OMN-16025 link 5 reads "complete ledger chain + replay green through an HONEST tier-2 verifier (SKIP != PASS)". The READER for that link merged in #3072 and called ledger_chain. That relation exists in no database on any lane, so every run since link 2 was closed has reported ledger_replay_not_configured at 4 of 5 links (34355201941, 34356615155). Two changes, both on the consumer side of the leg. 1. Migration 105 creates ledger_chain in the omnibase_infra database, beside its two nearest neighbours 088 overseer_tick_ledger and 089 verification_receipt_ledger, which are verification ledgers in that same flat set. Five columns are a contract with the merged reader and are commented as such. verifier_verdict carries a CHECK pinning it to pass / fail / skip, so a column that cannot represent SKIP can never force a writer to lie. There is deliberately no payload column. The migration also grants chain_canary_reader column-scoped SELECT on exactly the five columns the reader selects, guarded on role existence with a NOTICE, matching the contract already documented against CHAIN_CANARY_READER_PASSWORD in docker-compose.infra.yml. No credential is minted or rotated. 2. The leg's source becomes a variable NAME. #3072 shipped a raw-DSN ledger_source field, which is the exposure OMN-18060 removed from the sibling projection leg: onex skill serialises every arg into the node payload, onto the bus and into the event log, and the flag lands in argv. ledger_source_env replaces it outright with no shim, because no CLI row ever set the old field and so no caller can break. The model refuses a value that parses as a connection string, and the handler refuses a DSN that arrived on any other flag, reporting a new REFUSED status that renders as a non-passing link 5 rather than as an error. The lane declaration is read through a new load_lane_ledger_readback, which shares one core with its projection sibling so a refusal cannot be lost from one copy. Its lane set and its block key are separate constants: widening one leg's scope must be a deliberate edit to that leg. expected_ledger_hops now gets the same comma-splitting as the terminal topic fields, because the workflow passes it as one scalar flag; without it the declared hops would arrive as one string matching nothing and a complete chain would report CHAIN_INCOMPLETE. Effect on the canary today: the relation is real and empty, so link 5 moves from "no source configured" to CHAIN_INCOMPLETE naming the hops it did not find. That is a changed, falsifiable status reached through a real connection, and it stays red until the writer lands. Ordinal 105, not 104: 104 is BURNED. OMN-17923 retired 104_create_validator_ro_role.sql and its record requires the migration to come back as a new number, never as 104. This is the first new forward migration since that retirement, which surfaced a conflation in the retirement gate: its byte-identity test fingerprinted the WHOLE forward directory, so it asserted not only "the retirement removed 104 and nothing in 001..103 changed" -- the invariant it owns and its docstring states -- but also "no forward migration may ever land again", which it never claimed and which its sibling test deliberately allows for by bounding itself to ordinal <= 104. The assertion is now scoped to ordinals at or below the retired one, via a new optional include predicate on compute_migration_fingerprint. Same algorithm, same stamped digest, same file count: the retirement proof is unchanged and now stays true as the stream grows. The artifact is restamped, 88 -> 89 files, which the unscoped sibling test enforces. docker-compose.infra.yml's CHAIN_CANARY_READER_PASSWORD comment named "migration 104" as the file granting that role. It is corrected: 104 is a burned ordinal, 105 issues the link-5 grant, and the link-2 grant it described is issued by no migration at all (OMN-18115). RED first. 6 of 7 canary-side tests and 3 of 4 lane-loader tests failed before the implementation. 1072 tests green after across the db and canary suites, mypy --strict clean.
The Application Database Domain Enforcement gate (OMN-15361, ci.yml step "Enforce schema qualification in changed SQL") rejected the migration. It inspects only the DIFF, so the unqualified references in the older files of this set are grandfathered rather than permitted, and a new file does not get to copy them. Qualified: the CREATE TABLE target, the GRANT target, and all three COMMENT targets. The table lands in `public` either way — that is the search_path default the runner uses — so this changes nothing about what is created and everything about whether the statement says so explicitly. A note in the header records why, so the next reader does not simplify it back. The stream fingerprint is restamped, since the migration's bytes changed.
… issued An earlier commit on this branch asserted, in docker-compose.infra.yml and again in migration 105's header, that no file issues chain_canary_reader's link-2 grant and that it is live on the .201 dev lane only because it was applied out of band. That is wrong, and it was my error. The grant IS issued from this repository, on every migration run, by scripts/run-forward-migrations.sh in its login-only role grant seam (OMN-18060, #3352, 0e9106d, merged 2026-09-09T08:46Z): "chain_canary_reader:public.delegation_workflow_state:correlation_id,state" reasserted through reassert_login_only_role_grants, with tests/unit/infra/test_login_only_role_grants_omn18060.py pinning the entry. The lane's forward-migration one-shot logs it verbatim today. How I got it wrong: I searched docker/migrations/forward/*.sql and src/, found nothing, and concluded the grant was unissued. I never searched the migration RUNNER, which is where it lives. Absence from the two places I looked is not absence from the repository — the exact shape of error the standing rule about proving a zero with a positive control exists to prevent, applied here to a grep rather than to a sweep. Both comment sites now attribute the grant to the seam, by script name and by the test that pins it, and say plainly that it is a runner seam rather than a .sql file so the next reader does not repeat the inference. Migration 105 keeps issuing the link-5 grant as a .sql migration, because that one accompanies the CREATE TABLE that needs it. Comments only; no SQL statement, no grant and no behaviour changes. The stream fingerprint is restamped because 105's bytes changed. Reported by the team lead against lane omn18115-grant-by-migration's read-only measurement. OMN-18115, which I filed on the same false premise, is corrected separately.
9d9ee80 to
c7e3cb1
Compare
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 6
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 2
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (glm-review)
Asyncpg transport tests now pass an env var NAME where a DSN belongs | The diff rewrites two transport tests to call _replay_ledger_chain_via_asyncpg with _LEDGER_SOURCE_ENV (the string 'CHAIN_CANARY_LEDGER_DSN_FOR_TESTS') instead of a DSN. The transport's first parameter is a DSN; production resolves the name via _ledger_dsn_lookup before calling it. The tests only pass because _FakeAsyncpg accepts any string, so the test suite now exercises a call signature that production never makes and can never detect
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (glm-review)
No test asserts the resolved DSN value reaches the replay transport | Every existing ledger test stubs both ledger_dsn_lookup and ledger_replay, but none asserts that the value returned by the lookup is the string handed to _ledger_replay. The resolution step, the single seam this change introduces between name and credential, is unverified: a wiring mistake that passes the NAME, an empty string, or the wrong lookup result to the transport would still yield green unit tests while link 5 fails in CI with a c
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
argv DSN refusal is skipped when the ledger leg is unconfigured | In _replay_ledger, the empty-name early return for SKIPPED_NOT_CONFIGURED precedes the dsn_shaped_argv_flags(sys.argv) check. A connection string sitting on argv is exactly as exposed in /proc whether or not the leg is configured; the refusal fires only when the leg would otherwise run. Inconsistent with the stated rationale that the refusal targets the disclosure itself, not the ledger's configuration. | Evidence: if not declared_name:
rResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
REFUSED maps to link ERROR while the verdict claims it is not an error | _decide introduces LEDGER_REPLAY_REFUSED with the message 'A refusal is not a pass, and it is not an error either', while _link_five maps the same status to EnumChainLinkStatus.ERROR. Receipt consumers reading link status and verdict will see contradictory classifications for one condition. | Evidence: if ledger_status is EnumLedgerReplayStatus.REFUSED:
return (
EnumChainLinkStatus.ERROR,
"the ledger replay was refuResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Migration has no reverse counterpart | docker/migrations/forward/105_create_ledger_chain.sql is added with no corresponding reverse migration. Whether the flat stream requires one is not established in the diff; if sibling migrations in this set ship reverse files, the stream is now asymmetric and rollback of lanes that applied 105 is undefined. | Evidence: +CREATE TABLE IF NOT EXISTS public.ledger_chain ( | Fix: Confirm the retirement/flat-stream convention; add a reverse DROP TABLE IF EXISTS public.ledger
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Grant assertion ordering depends on relation existing before the shell seam runs | The new LOGIN_ONLY_ROLE_GRANT_MAP entry is commented in the SQL as 'gates on relation existence', but the grant loop change is unconditionally appended. On any lane where 105 has not yet applied, the grant for public.ledger_chain either aborts the grant reassertion step or is skipped silently; neither behavior is visible in this diff, and a failed GRANT loop that also carries the link-2 grant could leave both legs unauthorize
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
Thanks Jonah for merging PR 3381. The follow-up dev-lane canary run 34497375182 reached the declared broker, projection, and ledger readback stages, then ended honestly with terminal_missing after the ingress timed out at 120.398 seconds (1/5 links proven). The remaining live blocker is the runtime and ingress timeout; I am not treating this as a green link-5 result. |
…ary link 5 at it by name (#3381) * feat(OMN-16964): create the ledger_chain relation and point link 5 at it by name OMN-16025 link 5 reads "complete ledger chain + replay green through an HONEST tier-2 verifier (SKIP != PASS)". The READER for that link merged in #3072 and called ledger_chain. That relation exists in no database on any lane, so every run since link 2 was closed has reported ledger_replay_not_configured at 4 of 5 links (34355201941, 34356615155). Two changes, both on the consumer side of the leg. 1. Migration 105 creates ledger_chain in the omnibase_infra database, beside its two nearest neighbours 088 overseer_tick_ledger and 089 verification_receipt_ledger, which are verification ledgers in that same flat set. Five columns are a contract with the merged reader and are commented as such. verifier_verdict carries a CHECK pinning it to pass / fail / skip, so a column that cannot represent SKIP can never force a writer to lie. There is deliberately no payload column. The migration also grants chain_canary_reader column-scoped SELECT on exactly the five columns the reader selects, guarded on role existence with a NOTICE, matching the contract already documented against CHAIN_CANARY_READER_PASSWORD in docker-compose.infra.yml. No credential is minted or rotated. 2. The leg's source becomes a variable NAME. #3072 shipped a raw-DSN ledger_source field, which is the exposure OMN-18060 removed from the sibling projection leg: onex skill serialises every arg into the node payload, onto the bus and into the event log, and the flag lands in argv. ledger_source_env replaces it outright with no shim, because no CLI row ever set the old field and so no caller can break. The model refuses a value that parses as a connection string, and the handler refuses a DSN that arrived on any other flag, reporting a new REFUSED status that renders as a non-passing link 5 rather than as an error. The lane declaration is read through a new load_lane_ledger_readback, which shares one core with its projection sibling so a refusal cannot be lost from one copy. Its lane set and its block key are separate constants: widening one leg's scope must be a deliberate edit to that leg. expected_ledger_hops now gets the same comma-splitting as the terminal topic fields, because the workflow passes it as one scalar flag; without it the declared hops would arrive as one string matching nothing and a complete chain would report CHAIN_INCOMPLETE. Effect on the canary today: the relation is real and empty, so link 5 moves from "no source configured" to CHAIN_INCOMPLETE naming the hops it did not find. That is a changed, falsifiable status reached through a real connection, and it stays red until the writer lands. Ordinal 105, not 104: 104 is BURNED. OMN-17923 retired 104_create_validator_ro_role.sql and its record requires the migration to come back as a new number, never as 104. This is the first new forward migration since that retirement, which surfaced a conflation in the retirement gate: its byte-identity test fingerprinted the WHOLE forward directory, so it asserted not only "the retirement removed 104 and nothing in 001..103 changed" -- the invariant it owns and its docstring states -- but also "no forward migration may ever land again", which it never claimed and which its sibling test deliberately allows for by bounding itself to ordinal <= 104. The assertion is now scoped to ordinals at or below the retired one, via a new optional include predicate on compute_migration_fingerprint. Same algorithm, same stamped digest, same file count: the retirement proof is unchanged and now stays true as the stream grows. The artifact is restamped, 88 -> 89 files, which the unscoped sibling test enforces. docker-compose.infra.yml's CHAIN_CANARY_READER_PASSWORD comment named "migration 104" as the file granting that role. It is corrected: 104 is a burned ordinal, 105 issues the link-5 grant, and the link-2 grant it described is issued by no migration at all (OMN-18115). RED first. 6 of 7 canary-side tests and 3 of 4 lane-loader tests failed before the implementation. 1072 tests green after across the db and canary suites, mypy --strict clean. * fix(OMN-16964): schema-qualify every relation reference in migration 105 The Application Database Domain Enforcement gate (OMN-15361, ci.yml step "Enforce schema qualification in changed SQL") rejected the migration. It inspects only the DIFF, so the unqualified references in the older files of this set are grandfathered rather than permitted, and a new file does not get to copy them. Qualified: the CREATE TABLE target, the GRANT target, and all three COMMENT targets. The table lands in `public` either way — that is the search_path default the runner uses — so this changes nothing about what is created and everything about whether the statement says so explicitly. A note in the header records why, so the next reader does not simplify it back. The stream fingerprint is restamped, since the migration's bytes changed. * fix(OMN-16964): correct a false claim about where the link-2 grant is issued An earlier commit on this branch asserted, in docker-compose.infra.yml and again in migration 105's header, that no file issues chain_canary_reader's link-2 grant and that it is live on the .201 dev lane only because it was applied out of band. That is wrong, and it was my error. The grant IS issued from this repository, on every migration run, by scripts/run-forward-migrations.sh in its login-only role grant seam (OMN-18060, #3352, 0e9106d, merged 2026-09-09T08:46Z): "chain_canary_reader:public.delegation_workflow_state:correlation_id,state" reasserted through reassert_login_only_role_grants, with tests/unit/infra/test_login_only_role_grants_omn18060.py pinning the entry. The lane's forward-migration one-shot logs it verbatim today. How I got it wrong: I searched docker/migrations/forward/*.sql and src/, found nothing, and concluded the grant was unissued. I never searched the migration RUNNER, which is where it lives. Absence from the two places I looked is not absence from the repository — the exact shape of error the standing rule about proving a zero with a positive control exists to prevent, applied here to a grep rather than to a sweep. Both comment sites now attribute the grant to the seam, by script name and by the test that pins it, and say plainly that it is a runner seam rather than a .sql file so the next reader does not repeat the inference. Migration 105 keeps issuing the link-5 grant as a .sql migration, because that one accompanies the CREATE TABLE that needs it. Comments only; no SQL statement, no grant and no behaviour changes. The stream fingerprint is restamped because 105's bytes changed. Reported by the team lead against lane omn18115-grant-by-migration's read-only measurement. OMN-18115, which I filed on the same false premise, is corrected separately. * fix(OMN-16964): use pinned ledger ownership proof
What this is
OMN-16964 builds the missing leg for OMN-16025 link 5: "Complete ledger chain + replay green through an HONEST tier-2 verifier (SKIP != PASS)."
The reader for that link merged in #3072 and #3079. It selects
hop,replay_greenandverifier_verdictout of a relation calledledger_chain, ordered byhop_index. That relation exists in no database on any lane — probed read-only on the.201dev lane,to_regclassreturns NULL inomnibase_infra,omnidash_analyticsandomninode_cloudalike, and the string has zero references across four repos outside the canary node. So every run since link 2 was closed has reportedledger_replay_not_configuredat 4 of 5 links (runs 34355201941, 34356615155).This PR is the consumer-side half of closing that: the relation, and a source declaration that is a name rather than a credential. It does not include a writer — see "What this deliberately does not do".
1. Migration 105 creates
ledger_chainIn the
omnibase_infradatabase, via the flat forward-migration set. Its two nearest neighbours in that same set are088_create_overseer_tick_ledgerand089_create_verification_receipt_ledger— both verification ledgers, both here. It is not an analytics projection; nothing renders it.The other reason is least privilege:
chain_canary_readeralready holdsCONNECThere,USAGEon schemapublic, and column-scopedSELECTondelegation_workflow_state (correlation_id, state)for link 2. Putting link 5's relation in the same database means one added column grant rather than a second identity, a second credential and a second job secret. No credential is minted, rotated or written by this PR.Five columns are a contract with the merged reader and are commented as such.
verifier_verdictcarries aCHECKpinning it topass/fail/skip, so a column that cannot represent SKIP can never force a writer to lie. There is deliberately no payload column — the chain is evidence about shape, and a payload column would make this a second ungoverned copy of tenant data.correlation_idisTEXT, notuuid, because the merged reader binds a Pythonstr; asyncpg type-checks query arguments, so auuidcolumn would reject that bind and the leg would report ERROR on every run.delegation_workflow_statemakes the same choice for the same reason. The rationale is in the migration header.Ordinal 105, not 104. 104 is BURNED: OMN-17923 retired
104_create_validator_ro_role.sqland its record requires the migration to come back as a new number, never as 104. The governed pre-push caught my first attempt at 104 on the lab host — 4 failed of 27,415 — and the fix was to renumber, not to override.That renumber surfaced a conflation in the retirement gate. Its byte-identity test fingerprinted the whole forward directory, so it asserted not only the invariant it owns and states — "the retirement removed 104 and nothing in 001..103 changed" — but also a second claim it never made: "no forward migration may ever land again". Its own sibling test deliberately allows for growth by bounding itself to
ordinal <= 104, and its docstring says a new migration "must land as its own change with its own restamp". The assertion is now scoped to ordinals at or below the retired one, through a new optionalincludepredicate oncompute_migration_fingerprint. Same algorithm, same stamped digest, same file count — the retirement proof is byte-for-byte unchanged and now stays true as the stream grows. The artifact is restamped, 88 to 89 files, which the unscoped sibling test enforces.The
CHAIN_CANARY_READER_PASSWORDcomment indocker-compose.infra.ymlnamed "migration 104" as the file granting that role. Corrected here: 104 is a burned ordinal, 105 issues the link-5 grant, and the link-2 grant that comment described is issued by no migration at all.The grant is role-guarded through a
DOblock following103's pattern: a lane with no canary provisioned skips with aNOTICE, and the canary reportsskipped_not_configured— red and honest, never a green from a check that never ran.2. The leg's source becomes a variable NAME
#3072 shipped a raw-DSN
ledger_sourcefield. That is precisely the exposure OMN-18060 removed from the sibling projection leg two weeks later:onex skillserialises every arg into the node payload, onto the bus and into the event log, and the flag itself lands in argv, world-readable through/proc. Three durable copies of a credential from one flag.ledger_source_envreplaces it outright, with no shim. Nothing sets the old field — there was never a CLI row for it — so no caller can break, and keeping a raw-DSN field alive "just in case" would keep the exposure alive with it.The model refuses a value that parses as a connection string. The handler additionally refuses a DSN that reached the process on any other flag, which is the same disclosure with a different spelling. That refusal reports a new
REFUSEDstatus, distinct fromERRORon purpose: an error sends you to look at the database, a refusal sends you to look at how the canary was wired. It renders as a non-passing link 5, never as green.load_lane_ledger_readbackreads the declaration. It shares one core with its projection sibling so that a refusal cannot be lost from one copy while the other keeps it; the block key and the lane set are per-leg parameters, so widening one leg's scope is a deliberate edit to that leg.expected_ledger_hopsgains the same comma-splitting the terminal topic fields already have, because the workflow passes it as one scalar flag. Without it the declared hops would arrive as a single string matching nothing, and a complete chain would report CHAIN_INCOMPLETE.Effect on the canary today
Link 5 moves from "no source was configured" to CHAIN_INCOMPLETE, naming the hops it did not find, reached through a real connection to a real empty relation. That is a changed, falsifiable status. It stays red until a writer populates the relation, which is correct.
What this deliberately does not do
It does not add a writer, because a writer cannot honestly make link 5 green today. Filed as OMN-18116 while building this: the causal edge a chain replay would re-derive against is never recorded.
parent_span_idis declared on every envelope and populated on none — 717 of 717 rows in a two-hour window on the dev lane carry the key withcorrelation_idset on all 717 andparent_span_idset on 0 — and a repo-wide grep finds it propagated but never originated. The bus envelope has nocausation_idfield at all.The merged reader computes
replay_green = all(...)and only inspects the tier-2 verdict once that is true, so any writer built today could only ever writereplay_green = false. A writer that set it true because the hop arrived would be the exact defect this link exists to catch. That was not built.Also filed: OMN-18115 —
chain_canary_reader's link-2 grant is live on the dev lane but issued by no migration in this repo, so a lane rebuilt from source loses link 2 silently. This PR makes the link-5 grant reproducible and deliberately does not retrofit link 2's.Tests
RED first. 6 of 7 canary-side tests and 3 of 4 lane-loader tests failed before the implementation, including one that asserts the raw-DSN field is gone.
Green after: 1072 passed, 22 skipped across the db and canary suites plus the workflow CI test.
mypy --strictclean.ruffclean.New coverage: a DSN refused at the model boundary without echoing the value; a DSN on any other flag refusing the leg; an unset declared variable making no claim and naming the variable; REFUSED rendering as non-passing; the exported name var being distinct from the projection one so resolving one leg cannot overwrite the other; and every lane but
devrefused.Companion
The lane declaration this reads is omnimarket#2437.
Ticket: OMN-16964
Evidence-Ticket: OMN-16964
Evidence-Source: OCC#8874