Repository navigation
feat(OMN-15952): declare the unattended renewal cycle in the attach contract - #2746
Merged
Merged
Conversation
…ontract
An unattended runtime attaching to node_gateway_attach_effect was told how
often to heartbeat and nothing else. It could read expires_at off the
session it got back, but every other term of surviving that ceiling -- how
early to act, how much to spread across a fleet, and above all what renewal
even IS -- was undeclared. Each client that guessed would guess differently,
and the most natural guess (a heartbeat keeps the session alive) is wrong.
The contract now declares the cycle:
- EnumGatewayRenewalMode.RE_ATTACH names the mechanism on the wire. There
is deliberately no in-place-renewal member: expires_at is stamped once
at attach from min(token exp, max_session_ttl_seconds), and no path in
this node moves it. A heartbeat proves liveness and non-revocation; it
does not buy time. Renewal is a fresh client_credentials grant against
Keycloak followed by a fresh attach, minting a NEW session_id.
- ModelGatewayRenewalDirective carries the window (renew_not_before,
renew_at) and the ceiling they race, with the ordering invariant
renew_not_before <= renew_at < session_expires_at enforced in the model
rather than in the builder, so every construction path -- including
deserialization off the wire -- is subject to it.
- renewal_margin_seconds (120) and renewal_jitter_seconds (30) are
contract config, so the terms are declared once and served, not
documented and re-derived per client.
- service_gateway_renewal_policy computes the cycle as pure functions over
a session, and carries assert_expiry_not_extended -- the executable form
of the contract's central negative, since every session revision here is
a model_copy(update=...) and one added key is all it takes to turn a
heartbeat into a lifetime extension.
The renewal field on ModelGatewayAttachResponse is REQUIRED, not optional.
Optionality would let a build ship where unattended runtimes are told
nothing about renewal and no test fails, which is the exact silent gap
OMN-15952 was filed against.
Tests: 20 new, red before this change (the module and the model are the
subject). They cover the ordering invariant across token lifetimes
including ones shorter than the margin, the immutability of expires_at
under heartbeats at and past the ceiling (asserted on session-store WRITES,
so it holds whether the handler returns, revokes, or raises), re-attach
after expiry minting a new session while leaving its predecessor untouched,
two runtimes on one tenant getting two sessions, and -- structurally, by
asserting on the resolved secret-ref set rather than on one run's call
graph -- that the cycle reaches no browser-session surface.
Version bumped to 0.38.6, not 0.38.5: this is packaged source, the
release-identity gate requires a version ahead of the published one, and
0.38.5 is already claimed by the in-flight dev->main promotion.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
Comment |
runner-image-build-smoke failed with shared_env_digest stale (recorded=638933d1c8de12afe5c8024c, recomputed=bffad7795606a13c66aefdac). Caused by this PR: the release-identity gate forced a pyproject version bump, that rewrote omnibase-infra's own version in uv.lock, and the runner image's shared_env_digest hashes the resolved environment -- so every version bump necessarily invalidates the bound identity. Regenerated with scripts/ci/runner_image_identity.py --mode generate. Verified the drift is this PR's and not pre-existing: --mode verify on the canonical dev clone passes at v7 602c1ca464270df881bf5916b8d5affe.
jonahgabriel
enabled auto-merge (squash)
August 14, 2026 11:18
Contributor
✅ Hostile Reviewer — PASSEDBlocking findings (critical): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524) |
jonahgabriel
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 14, 2026
…+ omninode_infra#904 (#6496) * evidence(OMN-15952): OCC companion contract for the unattended renewal contract Binds the evidence for OmniNode-ai/omnibase_infra#2746 (the node contract that declares the renewal cycle) and OmniNode-ai/omninode_infra#904 (the onex-api mirror of the directive across an extra=forbid seam). Receipts follow in the next commit, once this PR's own number exists to self-bind against. * evidence(OMN-15952): receipts + self-bind for OCC#6496 Eleven receipts, all PASS, all probed live against the GitHub contents API at the producing head SHAs rather than against a local worktree -- the file this lane edited and the file the reviewer will read are then provably the same bytes. Six of the eleven are content assertions rather than PR-exists assertions: the renewal cycle present in the node contract's config block, the directive required (not optional) on the attach response, the renewal-mode enum having exactly one member, the renew-before-expiry invariant living in the model validator, assert_expiry_not_extended existing as a callable, and the edge mirror asserting field-set equality. A PR-exists receipt proves a branch was pushed; these prove what is in it. * evidence(OMN-15952): bind the self-bind receipt to OCC#6496's own commit occ-merge-eligibility rejected the PR with reason=pr_ticket_mismatch: it requires at least one PASS receipt bound to THIS PR or one of its commit SHAs, and all eleven receipts were bound to the producing omnibase_infra PR instead. The self-bind receipt now carries pr_number 6496, the OCC branch, and commit 2af48ba -- a commit of this PR, so the binding resolves against --pr-commit-sha. * evidence(OMN-15952): add the falsifiable deploy probe the ratchet requires deploy-gate rejected omnibase_infra#2746: OMN-15952 is not grandfathered and the contract declared no falsifiable deploy probe (OMN-14443). The deployed surface this ticket changes is the attach node's response shape -- a runtime image without the renewal directive serves an attach response with no renewal cycle, which is exactly the pre-OMN-15952 defect. The probe reads model_gateway_attach_response.py back from GitHub at the pinned omnibase_infra head and greps for the required field, so it goes RED the moment the field is absent or renamed. It reads a surface outside this repo, never a receipt or contract this PR authors. The live-cluster half -- docker exec against omninode-runtime, then a real attach -> expiry -> re-grant -> re-attach against deployed onex-api -- is the post-deploy acceptance test tracked on the ticket. It cannot run before the image exists, and claiming it here would be the vacuous evidence this ratchet exists to reject. * evidence(OMN-15952): put gh api in command position for the deploy probe The falsifiability parser reads the COMMAND POSITION, not the whole string. Wrapping the probe as a variable assignment put that in command position, so the parser saw ['gh', 'grep'] and rejected it as vacuous even though the call it could not see was a real gh-api readback. Rewritten in the exact form the gate's own error message blesses: gh api ... --jq .content | base64 -d | grep -q '<symbol>'. Same probe, same falsifiability, same PASS -- only the shell shape changed. Receipt regenerated against the new entry hash. * evidence(OMN-15952): rebind contract pins to the heads that actually merge The seven omnibase_infra probes pinned ae1b6e75, a superseded head that no branch reaches after the branch was amended -- the contract claimed to read 'the pinned omnibase_infra head' while reading an orphaned commit. Repin to c8d6d214 (the live PR #2746 head). The edge-mirror probe is repinned from 31102820 to 8289023c after PR #904's branch was updated onto dev. * evidence(OMN-15952): bind the omninode_infra evidence to PR #904, not #2746 Every receipt in this ticket carried pr_number 2746 -- including the two whose checks read omninode_infra -- so no PASS receipt bound PR #904. That is what #904's occ-preflight and receipt gate report as pr_ticket_mismatch: 'no PASS receipt for one or more tickets binds to PR #904 or one of its commit SHAs'. The two omninode_infra items now bind #904, its branch, and its head; all twelve receipts are re-minted by executing their contract-declared check live against the rebound pins, with contract_entry_sha256 recomputed via omnibase_core.validation.validator_receipt_gate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
An unattended runtime attaching to
node_gateway_attach_effectwas told how oftento heartbeat and nothing else. It could read
expires_atoff the session it gotback, but every other term of surviving that ceiling — how early to act, how much
to spread across a fleet, and above all what renewal even is — was undeclared.
Each client that guessed would guess differently, and the most natural guess (a
heartbeat keeps the session alive) is wrong.
What the contract now declares
EnumGatewayRenewalMode.RE_ATTACHnames the mechanism on the wire. There isdeliberately no in-place-renewal member:
expires_atis stamped once at attachfrom
min(token exp, max_session_ttl_seconds)and no path in this node moves it.A heartbeat proves liveness and non-revocation; it does not buy time. Renewal is a
fresh
client_credentialsgrant against Keycloak followed by a fresh attach,minting a new
session_id. In-place renewal is refused, not unimplemented —extending a live ceiling on the strength of a heartbeat would let a credential the
IdP has stopped authorizing hold a session open one heartbeat at a time.
ModelGatewayRenewalDirectivecarries the window (renew_not_before,renew_at) and the ceiling they race. The ordering invariantrenew_not_before <= renew_at < session_expires_atis enforced in the model,not the builder, so every construction path — including deserialization off the
wire — is subject to it.
renewal_margin_seconds(120) /renewal_jitter_seconds(30) are contractconfig. The margin is sized to hold three things, not just the happy-path round
trip: worst-case clock skew, the grant + attach + JWKS-verification round trip,
and at least one backoff-retry. The jitter exists because a fleet provisioned in
one bootstrap batch shares an attach instant, and without spreading it also shares
a renewal instant — forever, because a batch that renews together stays together.
service_gateway_renewal_policycomputes the cycle as pure functions over asession, and carries
assert_expiry_not_extended— the executable form of thecontract's central negative. Every session revision here is a
model_copy(update=...); one added key is all it takes to turn a heartbeat into alifetime extension, and that defect would be invisible in every existing assertion.
renewalonModelGatewayAttachResponseis required, not optional. Optionalitywould let a build ship where unattended runtimes are told nothing about renewal and
no test fails — the exact silent gap this ticket was filed against.
Tests (20 new)
RED before this change — the module and the model are the subject, so there was
nothing to import.
test_directive_ordering_invariant_holds_across_token_lifetimestest_directive_rejects_a_renew_at_that_is_not_before_expirytest_no_heartbeat_at_or_past_the_ceiling_ever_extends_ittest_re_attach_after_expiry_mints_a_new_session_with_a_fresh_granttest_two_runtimes_on_one_tenant_get_two_sessionstest_re_attach_needs_no_browser_session_surfaceThe ceiling-boundary test asserts on session-store writes, not on the handler's
return value, so it holds whether a post-ceiling heartbeat returns, revokes, or
raises — the invariant is "no write ever moved the ceiling", not "this call
succeeded". That keeps it valid across the separately-landing expiry-enforcement
work. The no-browser test asserts on the resolved secret-ref set, not on one
run's call graph: a test that merely observed no browser call would pass against an
implementation that had a fallback it happened not to take.
Seam (OMN-14208)
onex-apimirrors this response withextra="forbid"and validates the node's buspayload through it, so adding a field here is PAIR_INCOMPATIBLE: without the
matching edge, every attach becomes a 502. The edge half is
OmniNode-ai/omninode_infra#904, which acceptsrenewalas optional so the tworepos may deploy in either order.
Version
Bumped to
0.38.6, not0.38.5: this is packaged source, the release-identity gaterequires a version ahead of the published one, and
0.38.5is already claimed bythe in-flight dev→main promotion.
Ticket
OMN-15952 — unattended pairing/renewal contract for persistent runtimes across the
attach-token expiry.
dod_evidence: renewal cycle declared in
contract.yamlconfig, requiredModelGatewayRenewalDirectiveon the attach response, single-member RE_ATTACH modeenum, model-enforced renew-before-expiry invariant, executable
assert_expiry_not_extendedguard, and 20 tests including the at/past-ceilingheartbeat boundary and the re-attach-mints-a-new-session proof.
Evidence-Ticket: OMN-15952
Evidence-Source: OCC#6496