Repository navigation
fix(OMN-15521): remediation round — real rollback retention, seam build-args, digest-verified reload, AC5 restart proof - #2613
Merged
Conversation
…-ref build-args, digest-verified reload, and a real AC5 restart-safety proof
Fixes 5 adversarial-verifier-confirmed defects from the first OMN-15521
pass, on the same branch:
1. AC6 rollback target was nominally satisfied but functionally dead:
current_gateway_image_digest() awk-ed GATEWAY_IMAGE out of gateway.env
(which can drift from what is actually running) with no existence
check, and the previous image was never retagged before the build
moved BUILD_IMAGE_TAG onto the new image, so it became dangling and
prune-eligible immediately. Fixed via resolve_running_container_image()
(docker inspect --format {{.Image}}) + retain_previous_image() (docker
tag under docker-gateway-forwarder:previous BEFORE build_image()).
Live on .201: previous registry.json previous_digest
(sha256:b51b380d...) resolved to "No such image" -- confirmed dead.
2. Seam mismatch: OMNIBASE_COMPAT_REF / OMNIMARKET_REF /
ONEX_CHANGE_CONTROL_REF -- the three sibling-ref build-args
deploy-runtime.sh always passes -- were silently dropped, so the
gateway image fell back to the Dockerfile's hardcoded ARG defaults.
Live on .201 this showed as onex-change-control 0.5.3 on the gateway
container vs 0.5.1 on the omnibase-infra runtime container on the same
box. Fixed via read_repo_ref_or_main() (mirrors deploy-runtime.sh) and
added to resolve_build_args().
3. AC5 had zero implementing work: the PR/runbook converted the ticket's
own "that receipt lands on OMN-12912, not this ticket" filing
instruction into a blanket scope exclusion, skipping the restart proof
even though the container was force-recreated in the same session.
Added scripts/gateway_restart_safety_proof.sh -- a real restart-
durability smoke proof (snapshot idempotency store row count, reload,
wait-healthy, re-snapshot, assert no marker loss) -- and ran it live
against the deployed omninode-gateway-forwarder container on .201:
rows survived 333->334 across a real restart, container returned
genuinely healthy. Receipt filed on OMN-12912 per the ticket's own
AC5 wording (this ticket does not claim AC5 as its own closing
criterion).
4. Live evidence was unreproducible: the .201 canonical omnibase_infra
clone was still pre-merge (beebd1d) and did not have
scripts/deploy-gateway.sh at all, so the prior "ran against the
canonical clone" PR claim could not be substantiated. Pulled the
.201 clone to merged dev (6644803, confirmed ancestor-clean,
deploy-gateway.sh byte-identical to git show HEAD:...) before running
the AC5 proof above.
5. verify_deployment() never compared the running container's actual
image against the digest it just built -- only checked labels and
file presence, both of which a STALE container also satisfies after
the first successful deploy. A systemctl reload that silently fails
to recreate the container would report success on every subsequent
redeploy. Fixed: verify_deployment() now takes the new digest and
fails closed if docker inspect --format {{.Image}} does not match it.
Seam definition (fields/paths read or written across a boundary, delta
from the original PR):
- docker inspect <container> --format '{{.Image}}' (NEW read) -- the
rollback-target and post-reload-verification source of truth,
replacing the gateway.env GATEWAY_IMAGE= awk read.
- docker tag <previous_digest> docker-gateway-forwarder:previous (NEW
write) -- retention tag applied before build_image() runs.
- --build-arg OMNIBASE_COMPAT_REF/OMNIMARKET_REF/ONEX_CHANGE_CONTROL_REF
(NEW writes to the build command) -- resolved from OMNI_HOME sibling
clone HEAD SHAs via read_repo_ref_or_main(), falling back to
main/dev/main.
Tests: 12 new/changed cases across tests/scripts/test_deploy_gateway.py
(rollback-from-running-container RED/GREEN, retention-tag-before-build
ordering, missing-image fail-closed, first-deploy no-target, sibling-ref
build-args, digest-verify RED-before-reload-silently-fails) and a new
tests/scripts/test_gateway_restart_safety_proof.py (6 cases: green
survival, red data-loss, red never-healthy, empty-store warning) -- all
subprocess-driven against the real scripts with fake docker/sudo, same
convention as the original PR. 28/28 pass locally and on .200.
Also fixes a latent bug the remediation surfaced: usage()'s heredoc used
backtick-quoted `docker image prune` inside an unquoted <<EOF heredoc,
which would have executed docker image prune as a side effect of running
--help (shellcheck SC2006 caught it; confirmed zero backtick usage in the
original heredoc).
Evidence-Ticket: OMN-15521
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 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 selected for processing (5)
Comment |
Contributor
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
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 1, 2026
#5872) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2613 * evidence: OCC companion self-bind for #5872 * fix(OMN-15521): repair OCC companion append-only evidence --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai> Co-authored-by: Jonah Gray <jonah@omninode.ai>
jonahgabriel
added a commit
that referenced
this pull request
Aug 1, 2026
…e-mode staging, and a genuine AC5 restart-recreate proof (#2614) Fixes four defects an adversarial verifier found in the round-2 build (PR #2613, already merged to dev): 1. registry.json rollback_command jq bug (deploy-gateway.sh write_registry): the previous_digest interpolation used jq default operator (//) against an empty STRING, which never falls through, so a first deploy / pruned-previous-image deploy emitted a rollback_command with an EMPTY GATEWAY_IMAGE= value instead of null. Running that command would have sed-corrupted gateway.env, wedging the systemd unit ExecStartPre digest-format assertion on the next restart. Fixed with an explicit if/else in the jq filter, matching previous_digests own null semantics. Runbook rollback recipe now reads rollback_command directly instead of reconstructing it from previous_digest via jq -r (which prints the literal string null on a JSON null). 2. BUILD_SOURCE=workspace never invoked stage_workspace.sh (unlike deploy-runtime.sh, which always calls stage_workspace_if_needed() before build_images()). docker/Dockerfile.runtime unconditionally COPYs workspace/sibling-repos/, so a workspace-mode build silently used the committed placeholder / stale staging while still stamping workspace-provenance labels. Added stage_workspace_if_needed() + check_sibling_lock_pins(), invoking the SAME scripts/runtime_build/ stage_workspace.sh + check_sibling_lock_pins.py deploy-runtime.sh uses, in main() before build_image(). 3. gateway_restart_safety_proof.sh false-green: a fake sudo that exits 0 and does nothing (never actually reloads) satisfied both the health check and the row-count check, since neither compares container identity across the restart. Added container_identity() (Id + StartedAt) captured before/after and asserted to CHANGE, and require_reachable() so an unreachable container fails loudly instead of snapshot()s own exception handler silently collapsing to 0\t0 (which made the after<before durability check vacuous). 4. AC2/AC3/AC4/AC6 live evidence on .201 came from the round-2 (pre-this-fix) deploy — this PR re-runs the fixed script against .201 and captures fresh readback (see PR body). Seam: registry.json rollback_command (string|null) and previous_digest (string|null) must agree on presence; gateway_restart_safety_proof.sh container_identity() format {{.Id}}|{{.State.StartedAt}} vs deploy-gateway.shs own verify_deployment() digest-only check (different signal, same container). Tests: RED-before/GREEN-after for all four, driven via subprocess against the real scripts with docker/sudo faked (docker/tests/scripts/test_deploy_gateway.py, tests/scripts/test_gateway_restart_safety_proof.py). 34/34 passed on .200. OMN-15521
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.
OMN-15521 — remediation round (5 confirmed defects from the first pass)
Follow-up PR on the same branch (
jonah/omn-15521-build-lane) after thefirst PR (#2611, merged) shipped the sanctioned
.201gateway-forwarderdeploy path but an independent adversarial verifier found 5 defects in it.
This PR fixes exactly those 5, on the same branch/ticket. It does not touch
AC1-4's core mechanism, which the verifier confirmed held on its own
evidence.
Operator ruling (2026-08-01T17:20Z, cited in #2611): the
.201gateway-forwarder lane is the bridge connecting cloud architecture to
.201for local inference — load-bearing for the beta; instruction was "deploy the
gateway cleanly."
The 5 defects and their fixes
1. AC6 rollback target nominally satisfied but functionally dead (CONFIRMED live).
current_gateway_image_digest()awk'dGATEWAY_IMAGE=out ofgateway.env— a value that can drift from what the container is actuallyrunning — with no existence check, and the previous image was never
retagged before the build moved
docker-gateway-forwarder:buildonto thenew image, so it became dangling and prune-eligible immediately. Live on
.201:registry.jsonprevious_digest(
sha256:b51b380dfb351ad6740743d8e1e093951d15b67c06e98687ba4308d5e91d6546)resolves to
docker image inspect: No such image.Fix:
resolve_running_container_image()(docker inspect omninode-gateway-forwarder --format '{{.Image}}') replaces the env-fileread as the rollback-target source of truth, and
retain_previous_image()retags that digest as
docker-gateway-forwarder:previousbeforebuild_image()runs, so it survives the build tag move. If the previousimage no longer resolves locally (
docker tagfails), no rollback targetis recorded rather than a digest
docker image inspectcan't find.2. Seam mismatch: 3 sibling-ref build-args silently dropped (CONFIRMED live).
deploy-runtime.sh'sbuild_images()always passesOMNIBASE_COMPAT_REF/
OMNIMARKET_REF/ONEX_CHANGE_CONTROL_REF;deploy-gateway.shomittedall three, so the image fell back to
docker/Dockerfile.runtime'shardcoded ARG defaults (
v0.5.5/dev/v0.5.3). Live on.201:omninode-gateway-forwardercarriesonex-change-control 0.5.3whileomninode-runtimeon the same box carries0.5.1— the exactruntime-parity gap OMN-15625 tracks.
Fix:
read_repo_ref_or_main()(mirrorsdeploy-runtime.sh's helper of thesame name) + all three args added to
resolve_build_args().3. AC5 had zero implementing work (CONFIRMED).
The ticket's AC5 text is "the OMN-12912 restart/redelivery proof
(source-offset-ack / dedupe receipt) is re-run against the newly deployed
forwarder — that receipt lands on OMN-12912, not this ticket" — a filing
instruction, not a scope exclusion. The first pass's PR body and runbook
converted it into "Out of scope per the ticket text," skipping the proof
even though the container was force-recreated in the same session (the one
moment it was cheap to capture).
Fix:
scripts/gateway_restart_safety_proof.sh— a real restart-durabilitysmoke proof (snapshot the running container's idempotency-store row count,
sudo systemctl reload onex-gateway-forwarder, wait for Docker-healthy,re-snapshot, fail if any durable marker was lost). Ran it live this
session against the already-deployed
omninode-gateway-forwardercontainer on
.201:Receipt filed on
OMN-12912 per the ticket's
own filing instruction — this PR does not claim AC5 as its own closing
criterion. This proof is a restart-durability smoke test, not the full
cross-broker at-least-once/exactly-once redelivery proof (a deliberately
killed in-flight message never duplicating/dropping on the far side) —
PR #2556 itself scoped that out of live proof, and it still hasn't run
against a real deployed container; that gap is unchanged by this PR.
4. Live evidence was unreproducible (CONFIRMED).
The
.201canonicalomnibase_infraclone(
/data/omninode/omni_home/omnibase_infra) was still pre-merge(
beebd1d636b9) and did not havescripts/deploy-gateway.shat all — thefirst pass's "ran against the canonical clone" claim could not be
substantiated.
Fix (this session): pulled the
.201clone to merged dev(
664480335f2d33ae578ce41acb5087d5f6ac9a4,git merge-base --is-ancestorclean), confirmed
scripts/deploy-gateway.shis byte-identical togit show HEAD:scripts/deploy-gateway.sh(no ephemeral copy), then ran the AC5proof above against that clone's checked-out state. AC2/AC3/AC4 were not
re-executed live in this round (they already held on the previous
session's independently re-verified evidence per #2611); the code fixes
for defects 1/2/5 are proven by the test suite below, not a live
--executerun — deploying my unmerged branch's code to the shared
.201lab lanewould itself repeat the "ran an unmerged/ephemeral artifact" pattern this
defect flags. They'll get their live proof once this PR merges and
.201'sclone advances.
5.
verify_deployment()never compared the running digest (CONFIRMED).It only checked image labels and file presence — both of which a STALE
(already-deployed) container also satisfies. A
systemctl reloadthatsilently fails to recreate the container would report success on every
subsequent redeploy.
Fix:
verify_deployment()now takes the new digest as an argument andfails closed (
AC-VERIFY FAILED) ifdocker inspect ... --format '{{.Image}}'doesn't match it, before checking labels/files.Bonus fix surfaced by shellcheck during remediation:
usage()'sheredoc used backtick-quoted
`docker image prune`inside an unquoted<<EOF— which performs command substitution on backticks, so running--helpwould have executeddocker image pruneas a side effect(SC2006). Replaced with single quotes; confirmed the original heredoc had
zero backtick usage anywhere else.
Seam definition (delta from #2611)
docker inspect <container> --format '{{.Image}}'sha256:<64 hex>or empty — replaces thegateway.envGATEWAY_IMAGE=awk read as the rollback-target + post-reload-verification source of truthdocker tag <previous_digest> docker-gateway-forwarder:previousbuild_image()runs--build-arg OMNIBASE_COMPAT_REF=<sha|main>/OMNIMARKET_REF=<sha|dev>/ONEX_CHANGE_CONTROL_REF=<sha|main>docker compose build(NEW)$OMNI_HOME/{omnibase_compat,omnimarket,onex_change_control}HEAD SHAs viaread_repo_ref_or_main(), falling back per-repo when the sibling clone orOMNI_HOMEis absentdocker exec <container> python3 -c "..."against/app/data/gateway/delivery.sqlite3SELECT COUNT(*), MAX(processed_at) FROM idempotency_records— read-only, stdlibsqlite3(nosqlite3CLI in the runtime image)Acceptance-criterion → test map (delta only; unchanged ACs keep #2611's map)
test_build_command_stamps_sibling_ref_build_argstests/scripts/test_gateway_restart_safety_proof.py(6 cases) + live receipt on OMN-12912test_rollback_target_derived_from_running_container_not_env_file,test_previous_image_retagged_for_retention_before_build,test_rollback_target_not_recorded_if_previous_image_missing,test_first_deploy_records_no_rollback_targettest_verify_deployment_red_before_reload_silently_fails28/28 tests pass (22 in
test_deploy_gateway.py, 6 in the newtest_gateway_restart_safety_proof.py) — subprocess-driven against the realscripts with fake
docker/sudo, same convention as #2611. Each new test isRED-before/GREEN-after against the specific defect it closes (verified by
running with the pre-fix code before committing the fix).
Notes
stickybeatz-studio(.200) via patch-transfer withsha256 verification of every changed file (
gates_host=.200). Governedpre-push selector (
scripts/tests/ tests/scripts/ tests/unit/scripts/):1547 passed / 5 skipped (pre-existing platform-gated skips, unrelated).
.201's canonicalomnibase_infraclone advanced frombeebd1d636b9to664480335f2d(merged dev, includes feat(OMN-15521): sanctioned .201 gateway-forwarder deploy path #2611) as part of closing defect 4 —a
git pull --ff-only, no commits made there.Evidence-Ticket: OMN-15521
Evidence-Source: b40fd17ba390110f6e0e8470e67b90161702241b