Skip to content

fix(OMN-19004): derive a terminal's attempt count from its record, and refuse one that contradicts it - #2760

Merged
jonahgabriel merged 2 commits into
devfrom
jonah/omn-19004-terminal-derives-its-summary-fields
Sep 21, 2026
Merged

jonahgabriel merged 2 commits into
devfrom
jonah/omn-19004-terminal-derives-its-summary-fields

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Opened as a draft on purpose. Ruling 3707 requires the changed path to be proven on a leased surface before auto-merge is armed, and this repository's auto-merge workflow arms every pull request about a minute after it opens. Draft is the only ordering that holds. It will be marked ready once the proof comment is posted.

The invariant, and the half of it that can land today

A summary field on a terminal should be derived from the record it summarises, not set beside it, and a terminal that contradicts its own attempt records should be refused at construction with the contradiction named. This lands the clause that needs no vocabulary the wire cannot yet carry.

Derived

attempts_count carried default=1, so a terminal that supplied an attempt list and no count asserted one attempt regardless of how many it had just recorded.

This was not found by reading. Adding the refusal below turned two existing tests red immediately, both naming the same shape: the bus-less dispatch port builds its terminal payload with the attempt list and no count, so a three-rung refusal published attempts_count=1 beside three records. The count is now derived from the list when the field is absent. An explicit count stays authoritative.

Refused

A count below the recorded list has no reading at all. Nothing can record more attempts than it made.

Measured on correlation 8371bb34-3aa4-48d6-bdce-dffae3eb4b7f: a terminal reporting attempts_count=2 while carrying an escalation history of four rejected attempts in the same payload. OMN-15464 closed that in the producer, through _truthful_attempts_count. A derivation that lives only in the producer protects only the producers that call it, so the shape stayed constructible for the bus-less port, a direct construction, or a future adapter. The model refuses it now, and the message names both numbers so a reader does not have to work out which two fields disagreed or by how much.

The asymmetry is the design

A count above the list is legitimate and common. The escalation-history fallback carries rejected attempts only, and _authoritative_attempt_ladder_verdict reads exactly that inequality to decide the ladder is incomplete and withhold a verdict. Refusing that direction would break the incomplete-ladder reading and every terminal that uses it.

There is a control for each direction and a third for the empty list.

Deliberately not here, and the reason is the wire rather than the argument

The gate-decided terminal_failure_cause. The vocabulary has no member for a run the quality gate decided, so the truthful value does not exist to assign. That member is OMN-19060 and it lands in the core library, which means a release and a pin bump before anything may emit it. Refusing the contradiction here first would make the two measured fixtures unconstructible with nothing to put in their place, so both keep a control asserting they still construct, each saying why. The follow-up that refuses them has to delete a test explaining the allowance rather than quietly tightening a clause and meeting the fixtures at replay time.

A zero attempts_count for a run that made no attempt at all. Twenty-five recorded receipts on this host report attempts_count=1 beside an empty attempt list, because the field is floored at one and cannot say "none". Lowering that bound widens the accepted range on a wire field, which a released consumer pinned to the old range would refuse. Same consumer-first hazard as the enum, and it lands the same way.

Why this clause is safe to land alone

It only narrows what constructs, and it only moves emitted values within the range a released consumer already accepts. No field is added, no range is widened, no new value becomes emittable. The wire-compatibility gate has nothing to object to and, more to the point, nothing this change does could slip past it.

Tests

Red first. Three of the ten new tests fail against the parent, two on the refusal and one on the derivation. The seven controls pass both before and after, which is what a control is for.

The delegation, delegate-skill and projection suites: 1528 passed, 5 skipped. mypy --strict clean over 3326 source files. All 96 pre-commit hooks pass over the whole tree.

Version

0.4.182, enumerated live across every open head immediately before the push: dev holds 0.4.178, #2753 holds 0.4.179, #2758 and #2757 both hold 0.4.180 which is a collision those two lanes own, #2756 holds 0.4.181, and every remaining open head is at or below dev.

Ticket: OMN-19004. Parent: OMN-18978.

Evidence-Ticket: OMN-19004
Evidence-Source: OCC#10732

…d refuse one that contradicts it

The standing invariant on this ticket is that a summary field is DERIVED from
the record rather than SET alongside it, and that a terminal violating that is
refused at construction with the contradiction named. This lands the clause of
it that needs no vocabulary the wire cannot yet carry.

TWO HALVES, ONE PAIR OF FIELDS.

DERIVED. attempts_count carried default=1, so a terminal that supplied an
attempt list and no count asserted ONE attempt regardless of how many it had
just recorded. That is the defect the ticket is named for in its purest form:
the summary disagreeing with the record while nothing notices.

It is not hypothetical, and it was not found by reading. The bus-less dispatch
port builds its terminal payload with the attempt list and no count, so a
three-rung refusal published attempts_count=1 beside three records. Two tests
in the existing suites failed the moment the refusal below was added, naming
that exact shape. The count is now derived from the list when the field is
absent, and an explicit count is still authoritative.

REFUSED. A count BELOW the recorded list has no reading at all: nothing can
record more attempts than it made. Measured on correlation
8371bb34-3aa4-48d6-bdce-dffae3eb4b7f, where a terminal reported
attempts_count=2 while carrying an escalation history of FOUR rejected attempts
in the same payload. OMN-15464 closed that in the producer, via
_truthful_attempts_count. A derivation that lives only in the producer protects
only the producers that call it, so the shape stayed constructible for the
bus-less port, a direct construction, or a future adapter. The model refuses it
now, and the message names both numbers so a reader does not have to work out
which two fields disagreed or by how much.

THE ASYMMETRY IS THE DESIGN, NOT AN OVERSIGHT. A count ABOVE the list is
legitimate and common: the escalation-history fallback carries rejected
attempts only, and _authoritative_attempt_ladder_verdict reads exactly that
inequality to decide the ladder is incomplete and withhold a verdict. Refusing
that direction would break the incomplete-ladder reading and every terminal
that uses it. There is a control for each direction, and a third for the empty
list.

DELIBERATELY NOT HERE, AND THE REASON IS THE WIRE RATHER THAN THE ARGUMENT.

The gate-decided terminal_failure_cause. The vocabulary has no member for a run
the quality gate decided, so the truthful value does not exist to assign. That
member is OMN-19060 and it lands in the core library, which means a release and
a pin bump before anything may emit it. Refusing the contradiction here first
would make the two measured fixtures unconstructible with nothing to put in
their place, so both have a control asserting they still construct, each saying
why. The follow-up that does refuse them has to delete a test that explains the
allowance rather than quietly tightening a clause.

A zero attempts_count for a run that made no attempt at all. Twenty-five
recorded receipts on this host report attempts_count=1 beside an empty attempt
list, because the field is floored at one and cannot say none. Lowering that
bound widens the accepted range on a wire field, which a released consumer
pinned to the old range would refuse. Same consumer-first hazard as the enum,
and it lands the same way.

This clause is safe to land alone because it only NARROWS what constructs and
only moves emitted values within the range a released consumer already accepts.

Red first. Three of the ten new tests fail against the parent, two on the
refusal and one on the derivation, and the seven controls pass both before and
after, which is what a control is for. The delegation, delegate-skill and
projection suites: 1528 passed, 5 skipped. mypy --strict clean over 3326 source
files. All 96 pre-commit hooks pass over the whole tree.

Onex-Lane: routing-fix-2
Onex-Session: 27922f8337c74d20a163861fa51fca8d
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Pre-PR proof: surface=105 receipt=.onex_state/prepr-proof/omn19004-105-20260921T204500Z/receipt.txt sha256=4763f0485371bc388182fc718b41d66f2c3a196972c75e1ea73a7d1c6d97b50c result=PASS

12 assertions, 0 failures, two negative controls. The changed module was hot-patched into the two runtime containers on the leased proof surface, both were restarted so the assertions describe a process that loaded it, and both were restored from a digest-verified backup afterwards. Ten containers healthy throughout.

Both defects were measured before the patch, on a released consumer. That surface runs omnimarket 0.4.155, two releases behind this tree, which is what makes the controls worth taking. Before the patch it constructed a terminal claiming attempts_count=2 beside four records, the OMN-15464 shape that was closed in the producer and left constructible in the model. It also defaulted attempts_count=1 beside three records, which is the live bus-less dispatch-port shape and the defect this ticket is named for. Neither reading can be an artifact of the patch.

After the patch: the count below the record is refused with both numbers named, an absent count derives from the list, and every control holds. The three that matter most are the controls rather than the refusal. A count above the list still constructs, so the legitimate incomplete-ladder shape survives a change that refuses the other direction. An explicit count stays authoritative. An empty list keeps its floor, because the zero case is a separate wire widening.

The two measured fixtures still construct, deliberately. 73aba966 and 6ce51f77 both name a provider cause for a run the quality gate decided. That is the parent ticket's other half and it is not repaired here, because the vocabulary that would replace their cause only merged into the core library minutes ago and has not been released or pinned. Refusing them now would take a wrong-but-present value and leave nothing.

The live runtime came up under the changed model with all 218 contracts attached and none not-ready, and its 330 consumer groups stayed in sync with zero lag carrying the lane's real traffic. Both are read over the runtime's own HTTP surface, which only the long-running process can answer.

One assertion failed first and it was the harness, recorded in the receipt rather than quietly re-run: the consumer-sync check read a health field that is null until the first observation cycle completes after a restart, and this surface had just been through another lane's three force-recreate cycles. Polled directly it returned healthy on the next cycle.

No model-calling delegation ran. The surface has never minted a local tenant identity and a synthetic tenant is refused for having no provider key; both refusals are correct, predate this patch, and minting either would change the surface's own attribution state. It does not bear on this change, which is a validator on a wire model whose boundary is decode and construction.

Teardown re-read the in-container digest and matched the original in both containers, then re-verified pre-patch behaviour independently in each. No database written, no image rebuilt, no deployment, protected runner untouched.

Marking ready for review now, which is what arms auto-merge and starts the CI wave on the proven head.

@jonahgabriel
jonahgabriel marked this pull request as ready for review September 21, 2026 20:36
@jonahgabriel
jonahgabriel enabled auto-merge (squash) September 21, 2026 20:37
onexbot-occ-writer Bot added a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 21, 2026
…nimarket#2760

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/35652096595
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Pre-PR proof countersign: receipt sha256=4763f0485371bc388182fc718b41d66f2c3a196972c75e1ea73a7d1c6d97b50c actor=prepr-countersign verdict=COUNTERSIGNED

I recomputed the receipt digest and it matches the value quoted above and the ledger RELEASE row. The commit named in the receipt is the current head with nothing pushed since, and the ordering is correct: the proof comment went up at 20:35:54Z and auto-merge was armed at 20:37:11Z, so the comment preceded the arming rather than chasing it.

Both negative controls are measured on the deployed released consumer before the patch, so neither can be an artifact of it, and they measure two genuinely different defects rather than one defect twice. The surgical patch approach is the right call, and its refusal condition is the part that makes it trustworthy, since each anchor was asserted to appear exactly once and the patcher was built to refuse rather than force if one had moved.

The two assertions I would have asked for are already there. Refusing one direction is easy to prove and easy to overshoot, and showing that the legitimate incomplete-ladder shape and the empty-list floor both still construct is what distinguishes a validator from a blanket rejection. The teardown is read back twice over, by digest and then by re-measuring the pre-patch behaviour in each container independently.

jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 21, 2026
…imarket#2760 (#10732)

* evidence(OMN-19004): author OCC companion for OmniNode-ai/omnimarket#2760

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

* evidence(OMN-19004): self-bind OCC#10732 + rebind contract_sha256

* evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#2760

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/35652096595

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
…al-derives-its-summary-fields

# Conflicts:
#	pyproject.toml
#	uv.lock
@github-actions

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

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

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Proof provenance after the dev merge, stated rather than left for a reader to work out.

The pre-PR proof above was taken at 868b830f5. The head is now 19cc32487, because the default branch moved past this branch's version twice while it was in CI and the second move also conflicted. The merge resolved pyproject.toml to 0.4.185 and regenerated the lock.

The proof still covers the changed path, and that is checkable rather than asserted. Both files the proof exercised are byte-identical across the merge:

model_delegate_skill_response.py  bc52bbb33b8c04de82ab5cabd242512d2327624bfee6fac0dd4e099c8bf877ed  (both heads)
test_omn19004_terminal_derives_from_its_record.py  7d2e1f180090a55b504e6a79d70640a5a87c999dac5a197f5c91cd00b102e88e  (both heads)

git diff 868b830f5..HEAD over those two paths is empty. The merge brought in other lanes' work and the version, and touched neither the validator that was proven nor the tests that prove it.

Version is now 0.4.185, re-enumerated live across every open head after the second move: the default branch holds 0.4.183, one open head also holds 0.4.183, another holds 0.4.184, and two hold 0.4.182 including this one before the bump.

@github-actions

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 14
Models succeeded: qwen3-review,qwen3-review-b


Gate semantics

Verdict Meaning Blocks merge?
passed >=2 models succeeded, no critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Fewer than 2 models succeeded (infra unavailable/timeout) Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

@jonahgabriel
jonahgabriel merged commit d75f913 into dev Sep 21, 2026
135 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-19004-terminal-derives-its-summary-fields branch September 21, 2026 21:18
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Pre-PR proof countersign: receipt sha256=4763f0485371bc388182fc718b41d66f2c3a196972c75e1ea73a7d1c6d97b50c actor=prepr-countersign verdict=COUNTERSIGNED

Re-countersign at the merged head, superseding my 20:35Z one, which ruled before the head moved. This pull request merged at 21:18:14Z while I was working, so this verdict is retroactive and says so. It does not change: the evidence is stronger now than when I first read it.

The head-drift claim is correct, and I checked a commit the lane did not. The provenance comment compares the proof commit against the arming head. I compared both of those against the squash that actually landed, since that is the artifact. All three agree on both proven files.

file proof head arming head squash
the validator bc52bbb3… bc52bbb3… bc52bbb3…
its test 7d2e1f18… 7d2e1f18… 7d2e1f18…

The digests are the ones the lane posted, recomputed independently from the repository rather than taken from the comment. The merge brought in other lanes' work and the version and touched neither.

I reproduced both negative controls myself, rather than reading them off the receipt. The receipt measures them on a deployed consumer two releases behind. I did the same comparison from the outside: the released wheel for that version installed from the index, and the merged tree at the squash, with one script run against each so the only variable is the code under it. The staged merged tree was verified against the repository digest before use.

case released 0.4.155 merged 0.4.185
count 2 declared beside 4 records constructs, reports 2 refused, naming both numbers
no count declared beside 3 records constructs, reports 1 derives 3
count 3 beside 3 records constructs, 3 constructs, 3
count 5 beside 1 record constructs, 5 constructs, 5
empty attempt list constructs, 1 constructs, 1

Both defects reproduce on the released consumer exactly as the receipt describes, and both are closed on the merged code. The three positive controls are unchanged on both sides, which is the part that matters most: the change refuses exactly one direction and does not overshoot into a blanket rejection. The incomplete-ladder shape and the floor of one both survive, and the refusal message does name both numbers as the receipt claims.

Two corrections to what I was asked to check.

There is no delegation control in this receipt to verify the broker of. The receipt states plainly that no model-calling delegation ran, gives the two surface preconditions that refused it, and notes that both predate the patch and that minting past them would change the surface attribution state. That is the right call and it is the reason the broker defect found on another receipt today cannot apply here: a control that was never run cannot have been run against the wrong broker. Declining to run one and saying why is better than running one that proves the shared lane is healthy.

On staging, there was no source root to clone or copy. The proof applied three edits by anchor into the deployed module rather than staging a tree or building anything, and each anchor was asserted to appear exactly once with the patcher built to refuse rather than force if one had moved. So the staging hazard is structurally absent. One thing the receipt could add: it does not say where the patch text came from. The anchors and the measured behaviour make the provenance risk small, but a reader cannot tell from the receipt alone.

Ordering was correct. The proof comment went up at 20:35:54Z and auto-merge was armed at 20:37:11Z. The head then moved after arming, which the lane disclosed in its own provenance comment rather than leaving to be discovered, and which I have now confirmed was inert for the proven path.

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