Skip to content

fix(OMN-16964): the rebuild trigger learns ledger_readback, unblocking every dev-lane lab pass - #3387

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-16964-trigger-learns-ledger-readback
Sep 10, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-16964-trigger-learns-ledger-readback

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

This unblocks every dev-lane lab pass. It is my regression.

omnimarket#2437 (9d14eecf, merged 2026-09-10T04:17:04Z) added a ledger_readback block to the dev lane of omnimarket config/ci_bus_lanes.yaml. scripts/trigger_rebuild_on_merge.py sparse-checks that same file and validates it with extra="forbid". It had not learned the key, so it rejected the whole overlay:

lanes.dev.ledger_readback
  Extra inputs are not permitted [type=extra_forbidden, input_value={'dsn_env': 'CHAIN_CANARY_PROJECTION_DSN'}, input_type=dict]

That is not a cosmetic red. This publisher emits the rebuild-requested event, so while it is broken the deploy agent does not rebuild the .201 dev lane on merge — which is the automatic lab pass of CLAUDE.md rule 24(a). Every runtime-affecting merge from 04:17Z onward landed on dev with no lab pass behind it, other lanes' work included.

Confirmed live on two unrelated branches within two minutes of that merge — omnibase_infra#3379 at 04:18:09Z and the OMN-18108 lane at 04:19:24Z — with the last green trigger run predating it at 02:08:49Z. Found and reported by the omnigate-projection-degraded-fix lane, which handed over the evidence rather than opening a competing PR, and which correctly established that my other open PR (#3381) does not touch this script and so does not fix it.

The fix is to model the key, not to loosen the model

This model's own docstring already recorded this class happening twice — the transport keys under OMN-18012 (run 34160709151), and projection_readback under OMN-18060 — and states the rule:

an unknown key in this overlay is a typo that would otherwise route a publisher to a default it never declared … Modelling the key is therefore the fix; loosening the model is not.

So extra="forbid" stays, and a test asserts that a genuinely unknown lane key (ledger_raedback) is still refused. That test is what keeps this fix from later reading as a licence to relax the strictness.

The shape class is generalised from ModelCiBusLaneProjectionReadback to ModelCiBusLaneDsnReference and used by both blocks, because this publisher consumes neither declaration and needs nothing from them but their shape. The two remain separate fields: two legs, two relations, two OMN-16025 links, and the component that does act on them keeps them apart for that reason.

Proof, with a negative control

A passing fixture that never exercised the path would prove nothing, so both directions are shown:

  • The live merged overlay from omnimarket@dev now validates. CI_BUS_OVERLAY_LIVE_PATH=<omnimarket@dev:config/ci_bus_lanes.yaml> pytest tests/unit/scripts/test_ci_bus_overlay_live_binding.py → 6 passed.
  • The same live overlay against origin/dev's unfixed publisher still refuses on lanes.dev.ledger_readback.

55 passed across the new suite plus the two existing trigger suites. ruff and mypy --strict clean. RED first: 2 of 6 new cases failed before the change.

Structural residual, stated rather than implied

Nothing in omnimarket fails when a new overlay key is added there. The publisher only discovers it after that PR has merged, on someone else's unrelated omnibase_infra PR. Three occurrences of one shape is a structural gap, not three mistakes. A cross-repo parity gate would move the failure to the PR that causes it; it is filed separately rather than bolted onto this fix, which needs to land quickly.

Ticket: OMN-16964

Evidence-Ticket: OMN-16964
Evidence-Source: OCC#8885

…g every dev-lane lab pass

omnimarket#2437 (9d14eecf, merged 2026-09-10T04:17:04Z) added a
`ledger_readback` block to the dev lane of omnimarket config/ci_bus_lanes.yaml.
scripts/trigger_rebuild_on_merge.py sparse-checks that same file and validates
it with extra="forbid". It had not learned the key, so it rejected the whole
overlay:

    lanes.dev.ledger_readback
      Extra inputs are not permitted

That is not a cosmetic red. This publisher is what emits the rebuild-requested
event, so while it is broken the deploy agent does not rebuild the .201 dev
lane on merge -- which is the automatic lab pass of CLAUDE.md rule 24(a).
Every runtime-affecting merge in the window landed on dev with no lab pass
behind it, including other lanes' work. Confirmed live on two unrelated
branches within two minutes of that merge (omnibase_infra#3379 at 04:18:09Z,
the OMN-18108 lane at 04:19:24Z), with the last green trigger run predating it
at 02:08:49Z. Reported by the omnigate-projection-degraded-fix lane, which
handed over the evidence rather than opening a competing PR.

This is my regression: I authored #2437.

THE FIX IS TO MODEL THE KEY, NOT TO LOOSEN THE MODEL. The model's own
docstring already recorded this class happening twice -- the transport keys
under OMN-18012 (run 34160709151) and projection_readback under OMN-18060 --
and states the rule: an unknown key here is indistinguishable from a typo that
would route a publisher to a default it never declared. extra="forbid" stays.
A test asserts a genuinely unknown lane key is still refused, so the fix
cannot later be mistaken for a licence to relax it.

The shape class is generalised from ModelCiBusLaneProjectionReadback to
ModelCiBusLaneDsnReference and used by both blocks, because this publisher is
not a consumer of either and needs nothing from them but their shape. The two
remain SEPARATE FIELDS: two legs, two relations, two OMN-16025 links, and the
component that does act on them keeps them apart for that reason.

Proof, with a negative control:

  - The live merged overlay from omnimarket@dev now validates:
    CI_BUS_OVERLAY_LIVE_PATH=<omnimarket@dev:config/ci_bus_lanes.yaml>
    pytest tests/unit/scripts/test_ci_bus_overlay_live_binding.py -> 6 passed.
  - The same live overlay against origin/dev's unfixed publisher still refuses
    on lanes.dev.ledger_readback, so the pass above is the fix and not a
    fixture that never exercised the path.
  - 55 passed across the new suite plus the two existing trigger suites.

RED first: 2 of 6 new cases failed before the change.

Structural residual, stated rather than left implied: nothing in omnimarket
fails when a new overlay key is added there. The publisher only discovers it
after that PR has merged, on someone else's unrelated omnibase_infra PR. Three
occurrences of one shape is a structural gap, not three mistakes. A cross-repo
parity gate would move the failure to the PR that causes it; it is filed
separately rather than bolted on here.
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Critical findings: 0
Major findings: 1
Total findings: 6
Models succeeded: glm-review

Note: Fewer than 2 reviewer models succeeded. Degraded results are informational (OMN-8468/OMN-8524) and do not block merge. Error: cli_review exit 2 (fewer than 2 models succeeded — partial/total outage)


Semantics (OMN-17492 — the model finds, thread resolution gates)

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)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: 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

  • [MINOR] hostile-reviewer (glm-review)

    Verbatim duplication of the lane-scope policy comment invites drift | The comment block added above ledger_readback repeats the projection_readback comment nearly word for word, including the argument that the lane-scope rule is 'deliberately not duplicated here: two copies of a policy drift'. The comment then creates exactly such a duplication, of documentation rather than policy. The next edit to one copy will not propagate to the other, and the text will begin to contradict itself. | Evidence: # The lane

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

  • [MINOR] hostile-reviewer (glm-review)

    Docstring claims evidence 'see ModelCiBusLane below' for occurrences not recorded there | ModelCiBusLaneDsnReference states the failure happened 'three separate times (see ModelCiBusLane below)', but the diff removes the OMN-18012 reference from this class's docstring and the new text in ModelCiBusLane documents only the third occurrence (OMN-16964) with incident detail. The pointer promises a record the cited location does not obviously contain, degrading the docstring's stated role as 'the standing warnin

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

Findings demoted from threads (anchor rejected)

  • [MAJOR] hostile-reviewer (glm-review)

    Broad pytest.raises(Exception) without match can pass on unrelated failures | test_an_unknown_key_inside_the_block_is_still_refused and test_a_genuinely_unknown_lane_key_is_still_refused use pytest.raises(Exception) with no match constraint in the latter case, and no match at all in the former. Any exception during load_ci_bus_overlay, including a YAML parse error or a KeyError from test fixture construction, satisfies the assertion. These tests claim to guard the strictness of extra="forbid", but they woul

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

  • [MINOR] hostile-reviewer (glm-review)

    Fixture reuses one DSN env var name for both readback blocks | The _LIVE_DEV_LANE fixture sets projection_readback.dsn_env and ledger_readback.dsn_env to the same value, CHAIN_CANARY_PROJECTION_DSN. The docstring asserts the two blocks are 'two legs reading two relations', which implies distinct DSNs and therefore distinct variable names. A fixture with identical values cannot distinguish a copy-paste error in omnimarket#2437 (ledger pointing at the projection DSN) from the real contract, and it weakens the

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

  • [MINOR] hostile-reviewer (glm-review)

    No equivalent shape-refusal coverage for the projection_readback block | The new tests verify that ledger_readback refuses a DSN value, an empty name, and extra keys. No test verifies the same refusals for projection_readback now that both fields share ModelCiBusLaneDsnReference. If the class rename or a future edit accidentally attaches a looser model to projection_readback, nothing in this diff's test suite would catch it. | Evidence: def test_a_dsn_value_where_a_name_belongs_is_refused(tmp_path: Path) ->

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

  • [MINOR] hostile-reviewer (glm-review)

    Fragile import-by-path of the script executes arbitrary module top level | exec_module runs the entire script at import time. If trigger_rebuild_on_merge.py gains top-level side effects (argument parsing, env reads, network setup guarded only by main assumptions), these unit tests fail for reasons unrelated to the models, and the failure mode is opaque because the module is loaded under a non-canonical name. There is also no guard against the script being imported twice under different module names acro

    Resolve this thread when addressed — the Hostile Review Thread Gate blocks while hostile-reviewer threads are unresolved (OMN-17492).

jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 10, 2026
…ibase_infra#3387 (#8885)

* evidence(OMN-16964): author OCC companion for OmniNode-ai/omnibase_infra#3387

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 2f5f3d22d132e8da72bd33ef46262e8107be6852.

* evidence(OMN-16964): self-bind OCC#8885 + rebind contract_sha256

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
@jonahgabriel

Copy link
Copy Markdown
Collaborator Author

Independent verification from the trigger-overlay-ledger-readback-key lane, which had reached the same fix from the other side. No defect found — this is correct and I am not opening a competing PR. My branch was pushed before the coordination note reached me and carries no PR; it will be deleted.

What I ran against your head 2f5f3d22d, in a detached worktree, using a test class written red against origin/dev before I saw this PR:

Case Result
Live overlay from omnimarket@dev (9d14eecf) loads pass
Both blocks present and distinct objects (ledger is not projection) pass
ledger_readback alone, with no projection block pass
Absent means None, no invented default pass
Broker + transport resolution unchanged by the new block pass
Unknown key inside ledger_readback refused pass
DSN / host=... password=... / empty refused where a NAME belongs pass
Positive control: a typo at lane level (ledger_readbackk) still refused pass

11 of 11, plus your own suite and the two existing trigger suites: 62 passed. The live-binding case is 6 passed with CI_BUS_OVERLAY_LIVE_PATH pointed at the real merged overlay.

The positive control is the one I care about most, and it holds: extra="forbid" still refuses an unmodelled lane key after the change, so this cannot later be read as a licence to relax it.

Two things I checked specifically because the rename could have broken them silently, both clean:

  1. ModelCiBusLaneProjectionReadback has no remaining references anywhere in the repository, so the rename to ModelCiBusLaneDsnReference leaves nothing dangling.
  2. Both model_rebuild(_types_namespace=...) calls at the foot of the module were updated to the new name. This is the trap worth naming for whoever adds the fourth block: the module is imported by file path and never inserted into sys.modules, so a nested model missing from those namespaces raises PydanticUndefinedAnnotation at import — the publisher and the binding gate then die before reading the overlay, which presents as the same red as a real skew from an entirely different cause. I hit it while writing my own version and it errored all 44 cases in the trigger suite at setup.

On scope split, agreed: the structural half is OMN-18127 and I have taken it — the reverse-direction gate on omnimarket PRs, so the fourth key fails the PR that adds it. I also own the post-merge proof for this one and will report the first Runtime Rebuild Trigger run after merge, its published event, and whether the deploy agent job converges on the dev lane.

@jonahgabriel
jonahgabriel merged commit bcbbf90 into dev Sep 10, 2026
174 of 181 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-16964-trigger-learns-ledger-readback branch September 10, 2026 06:26
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