Repository navigation
feat(OMN-14889): git-tag-driven release-train trigger for dev+stability lab lanes (re-cut onto dev) - #2388
Merged
Conversation
…ty lab lanes Re-cut of closed PR #2377 (commit c2ec0d2) onto current dev after #2370 (OMN-14873) merged as 54938fd. File-scoped restore of the 6 paths only; dev's newer refresh_stability_lane.sh (detached-checkout fix) is left untouched. - .github/workflows/release-train-lab.yml (new) - docker/docker-compose.runners.yml (+74, additive) - docs/runbooks/release-train-lab.md (new) - scripts/runtime_build/cut_release_train_tag.sh (new) - scripts/runtime_build/refresh_dev_lane.sh (new) - scripts/runtime_build/verify_dev_refresh.py (new)
📝 WalkthroughWalkthroughAdds a tag-driven release-train workflow for dev and stability lab lanes, a dedicated deployment runner, tag-cutting automation, a cold-aware dev refresh with rollback, and a fail-closed health gate producing durable JSON receipts. ChangesRelease-train lab deployment
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant TagScript
participant GitHubWorkflow
participant DeployRunner
participant RefreshScript
participant HealthGate
Operator->>TagScript: cut and push lab tag
TagScript->>GitHubWorkflow: trigger tag-push workflow
GitHubWorkflow->>DeployRunner: run lane deployment
DeployRunner->>RefreshScript: invoke refresh script
RefreshScript->>HealthGate: verify refreshed runtime
HealthGate-->>RefreshScript: return gate result
RefreshScript-->>GitHubWorkflow: write receipt and exit status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The workflow header, runbook, and runners compose block all asserted the
omnibase-deploy runner was unprovisioned and that the deploy job "will queue
but never pick up." That is FALSE and safety-relevant: a reviewer would
conclude a lab/** tag push is inert when it in fact triggers a real lane
refresh in execute mode.
Verified live 2026-07-21:
gh api repos/OmniNode-ai/omnibase_infra/actions/runners
-> omninode-deploy-runner status=online,
labels [self-hosted, Linux, X64, omnibase-deploy]
That is the exact label set both jobs target, and registration is at the
REPOSITORY level (not the org level the runbook recipe assumed).
Corrected all three sites to state: the runner is online; a lab/dev/** or
lab/stability/** tag push DOES refresh the lane in execute mode (intended,
since dev+stability are pre-authorized, but not a no-op); and the remaining
blocker is host-side git access to the ambient $OMNI_HOME clone, not
registration.
Five runs on 2026-07-21T03:59-04:19Z all failed in the "Refresh stability
lane" step, in three distinct forms as intermediate fixes were attempted:
dubious ownership (128), .git/FETCH_HEAD Permission denied (255), and 'dev'
already checked out in a sibling worktree (128). All died before any
container action -- zero docker compose / up -d / --force-recreate lines in
any of the five logs -- so no lane was mutated.
Prod boundary statements preserved and reinforced: lab/** and v* remain
disjoint namespaces and prod promotion still routes through
node_redeploy_orchestrator's grant gate.
No functional change; comments and docs only.
Closes OMN-14889
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
scripts/runtime_build/verify_dev_refresh.py (2)
318-367: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail-closed health gate has no test coverage yet.
run_health_gate/HealthGateReport.overallgate a production-adjacent deploy path and already accept injectablerunner/openerfor testability, but per PR objectives this file has no test coverage. Given the criticality (fail-closed AND-of-all-checks) and that it hasn't been exercised end-to-end yet, unit tests foroverall's PASS/FAIL/INFRA_ERROR branching would be cheap to add given the existing DI seams.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/runtime_build/verify_dev_refresh.py` around lines 318 - 367, ** Add unit tests in the existing test area for run_health_gate using injected runner and opener doubles, covering HealthGateReport.overall’s PASS, FAIL, and INFRA_ERROR branches. Exercise the fail-closed AND behavior by varying service, manifest, health, and cluster checks, and assert the resulting overall status and relevant report fields.
264-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
status_code != 200branch is effectively unreachable —urlopenraisesHTTPError(a subclass ofURLError) for non-2xx responses before this check runs.Since
except (urllib.error.URLError, OSError)already catchesHTTPError, a real non-200 health response surfaces as the generic"health fetch failed: ..."message instead of the dedicated"health endpoint returned HTTP {status_code}"message. PASS/FAIL correctness is unaffected, but this reduces diagnostic clarity right as this gate is about to be exercised for the first time in production.♻️ Proposed fix: distinguish HTTPError explicitly
try: with open_fn(health_url, timeout=10) as resp: status_code = getattr(resp, "status", 200) raw = resp.read() + except urllib.error.HTTPError as exc: + return False, f"health endpoint returned HTTP {exc.code}" except (urllib.error.URLError, OSError) as exc: return False, f"health fetch failed: {exc}" - if status_code and status_code != 200: - return False, f"health endpoint returned HTTP {status_code}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/runtime_build/verify_dev_refresh.py` around lines 264 - 282, Update check_health to catch urllib.error.HTTPError separately from the broader URLError/OSError handler, return the dedicated “health endpoint returned HTTP” diagnostic using the exception’s HTTP status, and retain the existing generic fetch-failure handling for other URL or OS errors.docker/docker-compose.runners.yml (2)
948-949: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHardcoded Docker socket GID is host-specific and fragile.
group_add: - "984"bakes in one host's Docker group GID. If this compose file is ever run on a different host/distro (or the host's docker group GID changes), DinD access silently breaks with socket permission errors, which would fail exactly the builds/deploys this runner exists to perform.🔧 Suggested fix
+ environment: + DOCKER_GID: ${DOCKER_GID:-984} group_add: - - "984" # Match host Docker socket GID for DinD access + - "${DOCKER_GID:-984}" # Match host Docker socket GID for DinD access; override via DOCKER_GID if host differs🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/docker-compose.runners.yml` around lines 948 - 949, Replace the hardcoded Docker group ID in the runner service’s group_add configuration with a runtime-configurable value sourced from the host environment, while preserving the existing Docker socket access behavior and providing an appropriate fallback or required-variable validation.
925-947: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared runner anchors here.
omninode-deploy-runnerduplicates the shared runner config instead of merging*runner-base/*runner-env, so future changes to common settings can drift out of sync. Merge the anchors and keep only the deploy-specific overrides.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/docker-compose.runners.yml` around lines 925 - 947, Update the omninode-deploy-runner service to merge the shared runner-base and runner-env anchors, removing duplicated common configuration while retaining only deploy-specific overrides such as the runner name, labels, group, token, and deploy-specific mounts or settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-train-lab.yml:
- Around line 151-184: Update the “Publish receipt to job summary” step to
handle an unset or unrecognized LANE explicitly before selecting a state
directory. For an unknown lane, report that no receipt can be associated with
the run and skip reading any receipt file; retain the existing stability and dev
directory mappings only for recognized lane values.
In `@scripts/runtime_build/refresh_dev_lane.sh`:
- Around line 60-70: Move the ~/.omnibase/.env existence check and source
operation outside the runtime-policy.env conditional in the refresh script. Keep
runtime-policy.env loading guarded by its own check, while ensuring
~/.omnibase/.env is sourced independently whenever present.
- Around line 282-341: The warm-path ancestry check in the NEW_REFS loop reads
stale sibling clone heads because only INFRA_CLONE is refreshed. Before
collecting NEW_REFS when BRANCH is warm, refresh every sibling in
ALL_TRACKED_REPOS to the requested REF using the same fetch/checkout/reset flow,
then perform the existing ancestry comparisons; preserve the cold-aware path
unchanged.
In `@scripts/runtime_build/verify_dev_refresh.py`:
- Around line 285-315: Update check_cluster_health to obtain the broker address
from the appropriate environment variable instead of hardcoding "redpanda:9092"
in the rpk arguments. Preserve the existing health-check command and behavior,
including the current broker-container parameter.
---
Nitpick comments:
In `@docker/docker-compose.runners.yml`:
- Around line 948-949: Replace the hardcoded Docker group ID in the runner
service’s group_add configuration with a runtime-configurable value sourced from
the host environment, while preserving the existing Docker socket access
behavior and providing an appropriate fallback or required-variable validation.
- Around line 925-947: Update the omninode-deploy-runner service to merge the
shared runner-base and runner-env anchors, removing duplicated common
configuration while retaining only deploy-specific overrides such as the runner
name, labels, group, token, and deploy-specific mounts or settings.
In `@scripts/runtime_build/verify_dev_refresh.py`:
- Around line 318-367: **
Add unit tests in the existing test area for run_health_gate using injected
runner and opener doubles, covering HealthGateReport.overall’s PASS, FAIL, and
INFRA_ERROR branches. Exercise the fail-closed AND behavior by varying service,
manifest, health, and cluster checks, and assert the resulting overall status
and relevant report fields.
- Around line 264-282: Update check_health to catch urllib.error.HTTPError
separately from the broader URLError/OSError handler, return the dedicated
“health endpoint returned HTTP” diagnostic using the exception’s HTTP status,
and retain the existing generic fetch-failure handling for other URL or OS
errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a2915a9-1db9-4af8-bfc7-7361c0918875
📒 Files selected for processing (6)
.github/workflows/release-train-lab.ymldocker/docker-compose.runners.ymldocs/runbooks/release-train-lab.mdscripts/runtime_build/cut_release_train_tag.shscripts/runtime_build/refresh_dev_lane.shscripts/runtime_build/verify_dev_refresh.py
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-14889 — release-train lab-lane trigger (re-cut onto dev)
Re-cut of closed PR #2377 (single commit
c2ec0d2e, branch deleted) onto currentdev.#2377 was a stacked branch on top of #2370 (OMN-14873); #2370 merged to
devby squash as54938fd3at 2026-07-21T19:35:17Z and #2377 was closed unmerged 2 seconds later. Because theparent landed squashed, a cherry-pick/rebase of
c2ec0d2eproduces a spurious add/add conflict onscripts/runtime_build/refresh_stability_lane.sh. The first commit here therefore used afile-scoped restore of only the 6 paths that are actually #2377's own diff — conflict-free by
construction. Two later commits deliberately diverge from #2377 (see "Commit provenance").
Closes OMN-14889
What this ships
Git-tag-driven release-train trigger for the dev and stability lab lanes.
cut-tag(
workflow_dispatch) pushes alab/<lane>/<utc>-<shortsha>tag; that tag push firesdeploy,which calls
refresh_stability_lane.sh(OMN-14873, reused verbatim) or the newrefresh_dev_lane.sh, then publishes the lane's JSON receipt to the job summary.Prod is untouched —
lab/**is a disjoint tag namespace fromv*, and prod promotion stillroutes through
node_redeploy_orchestrator's grant gate (CLAUDE.md 2a/12). Nothing here cansatisfy or bypass that gate.
Files (7)
Counts are cumulative additions/deletions vs the merge base (
git diff --stat <merge-base>..c9b1cc6f),not per-commit.
.github/workflows/release-train-lab.ymldocker/docker-compose.runners.ymlomninode-deploy-runnerservice + its named volume) + stale-claim repairdocs/runbooks/release-train-lab.mdscripts/runtime_build/cut_release_train_tag.shscripts/runtime_build/refresh_dev_lane.shscripts/runtime_build/verify_dev_refresh.pyscripts/runtime_build/tests/test_verify_dev_refresh.pyTotal: 7 files, 1723 insertions, 0 deletions.
scripts/runtime_build/refresh_stability_lane.shis NOT in this diff. dev's copy (from #2370)is newer than the one #2377's stacked branch carried (+13/-2 vs
refs/pull/2377/head:git checkout dev→git checkout --force --detach "${RESOLVED_REF_SHA}", fixing the OMN-12618sibling-worktree collision). dev's version is kept; this branch does not touch that file.
Commit provenance — the branch is NOT byte-exact to #2377
Three commits, only the first of which is a restore:
792c3b7agit diff refs/pull/2377/head 792c3b7a -- <the 6 paths>is empty.git status --shortat that commit listed exactly those 6 paths, nothing else.3d4dee67c9b1cc6fc9b1cc6fcontains four distinct changes. Only (a) is the scoped work:(a)
refresh_dev_lane.shhardening — the scoped change. Replaces thegit checkout dev+git reset --hard "${REF}"pair withrev-parse "${REF}^{commit}"followed bycheckout --force --detach "${RESOLVED_REF_SHA}", and routes every ambient-clone git call through anew
git_clone()wrapper that applies per-invocationgit -c safe.directory=<clone>. It also addsa new loop that refreshes the four sibling ambient clones (see "Reviewer attention" below).
This addresses OMN-14900 failure modes 3 and 1 for the dev lane only.
(b)
.github/workflows/release-train-lab.yml— unrecognized-lane fallback. The receipt-summarystep's
if/elseonLANEbecameif/elif/elsewithstate_dir=""in the terminal branch, plus a"No receipt is associated with an unrecognized release-train lane."message. Previously anunrecognized lane silently reused the dev receipt path. Unrelated to (a).
(c)
verify_dev_refresh.pybroker-address parameterization. AddsDEFAULT_BROKER_ADDRESS = os.environ.get("OMNIBASE_DEV_BROKER_ADDRESS", "redpanda:9092")and a newbroker_addressparameter oncheck_cluster_health(), replacing the hardcodedbrokers=redpanda:9092. Ships with the newtests/test_verify_dev_refresh.py(
test_cluster_health_uses_configured_broker_address). Unrelated to (a).(d)
~/.omnibase/.envsourcing restructure. The.envsource block inrefresh_dev_lane.shwas de-nested from inside the
docker/runtime-policy.envguard into its own top-levelif, withset -a/set +arebalanced so each block brackets its own source. Behavior change:.envis nowsourced even when
runtime-policy.envis absent. Unrelated to (a).Reviewer attention — sibling-clone refresh loop (
refresh_dev_lane.sh)c9b1cc6fadds a loop that runs, for every repo inALL_TRACKED_REPOS=(omnibase_infra omnibase_core omnibase_compat onex_change_control omnimarket):Two risks a reviewer must weigh before approving:
--forceagainst shared ambient clones. This force-checks-out four additional sharedclones under
$OMNI_HOME, discarding any local modifications in them and leaving each on adetached HEAD rather than the branch its other consumers expect. These clones are contended
multi-consumer resources — the 04:14/04:19 run failures below name a live sibling worktree on the
deploy host's
omnibase_infraclone (runtime-sync-worktrees/OMN-12618/...), and on theworkstation clone this branch was cut from
git worktree listreports 23 entries (the cloneitself plus 22 linked worktrees).
The sibling clones' worktree state on the deploy host has not been probed. This is in
tension with OMN-14900's ranked-feat: PostgreSQL Adapter with Comprehensive Tests and Structured Logging #1 proposed fix ("stop writing to the shared ambient clone") —
it expands CI's write surface on shared clones from one repo to five.
lab/dev/**tagexists only in
omnibase_infra, so in every sibling clone therev-parsefails and the loopfalls back to
origin/devwith no warning, no receipt field, and no refusal. The lane can beassembled from
omnibase_infra@<tag-sha>plus four siblings at whateverorigin/devwas atfetch time, while the receipt records a single
REF.Both are filed as OMN-14901 (child of OMN-14889) rather than fixed here.
Stale-claim repair (commit
3d4dee67)Three files asserted the runner was unprovisioned. Corrected to live-verified fact:
.github/workflows/release-train-lab.yml—PROVISIONING STATUSheader blockdocs/runbooks/release-train-lab.md— provisioning section + "manual canary" preambledocker/docker-compose.runners.yml—omninode-deploy-runnercomment blockLive facts, re-verified 2026-07-21:
runs-on: [self-hosted, omnibase-deploy]).It is registered at the repository level, not the org level the original runbook recipe
assumed — the docs now point at the
repos/...endpoint for verification.release-train-lab.ymlto exist at the tagged commit, so a tag cutfrom a feature branch fires it exactly as a tag on
devwould. That is how the runs belowhappened while this workflow is still unmerged.
dod_evidence
Verifiable from remote surfaces (re-checked at head
c9b1cc6f):792c3b7ais byte-exact torefs/pull/2377/headover the 6 restored paths(empty
git diff), and scoped —git status --shortlisted exactly those 6 paths.c9b1cc6fat the time of this body edit: 130 pass, 5 skipping, 0 fail;CI Summary,verify / verify,deploy-gate / deploy-gate,occ-preflight / eligibilityandreceipt-honestyall pass.Evidence-Source: OCC#4555—onex_change_control#4553, branchcodex-letter/omn-14889-infra-2388-occ, MERGED 2026-07-21T20:36:27Z.Author-reported local runs (not independently re-verified in this body):
uv run ruff format scripts/→ 196 files unchanged;uv run ruff check --fix scripts/→ allchecks passed.
pre-commit run --files <changed paths>→ zero failures (all hooks Passed/Skipped).-k, no bypass flag,no
--no-verify) at the3d4dee67push. The authoritative post-c9b1cc6fsignal is the live CIline above.
verify_dev_refresh.py --helpexits 0 — the module imports and its argparse is well-formed.yaml.safe_load).Trigger path IS live (corrected — was previously claimed dormant):
omnibase-deployrunner is registered and online; thedeployjob does pick up.lab/stability/*tags exist on origin, 5 of them pointing atc2ec0d2e. 5 realrelease-train-labruns executed 2026-07-21T03:59–04:19Z — run ids29799976126,29800099028,29800399010,29800684577,29800954943. All 5conclusion=failure.43935c84…,e5404e36…) produced no runs — the workflow file does notexist at those tagged commits.
lab/dev/**tags exist. The dev lane has never been triggered.All 5 runs failed BEFORE any mutation — benign, but not "nothing happened":
Refresh stability lanefatal: detected dubious ownership in repository at /data/omninode/omni_home/omnibase_infraRefresh stability laneerror: cannot open .git/FETCH_HEAD: Permission deniedRefresh stability lanefatal: 'dev' is already checked out at /data/omninode/runtime-sync-worktrees/OMN-12618/...Zero
docker compose/up -d/--force-recreatelines appear in any of the 5 job logs, so nolane was mutated. The remaining blocker is the runner container's ownership/permissions and
worktree contention against the bind-mounted
$OMNI_HOMEclone — not registration.OMN-14900 status after
c9b1cc6f— the deploy hop is still blockeddubious ownershipgit_clone()wrapper applies-c safe.directory=<clone>per invocation.refresh_stability_lane.shondevstill contains zerosafe.directoryhandling, so mode 1 remains open for the stability lane..git/FETCH_HEAD: Permission deniedfetch origin --prunestill writes the shared clone as the runner uid, and a detachedcheckout --forceneeds strictly more write access than the fetch that already failed.'dev' is already checked out at …Mode 2 blocks the deploy hop on both lanes. Do not read this PR as making the deploy hop
work. The three-mode analysis, ranked fixes, and acceptance criteria live in OMN-14900.
NOT proven — do not read this PR as a successful deploy:
lab/dev/**tag has beencut, no dev lane refreshed, no dev receipt produced, zero dev-lane runtime readback. (The
stability-lane runs above all died pre-mutation, so there is no successful lane readback of any
kind.)
c9b1cc6fhardening in (a) is untested against a real runner. It is a fix for observedfailure modes, not a fix proven by a green run.
verify_dev_refresh.pydeliberately omits consumer-group Stable checking (present in thestability-test gate). It was built against a lane whose core services were not live, so real
group names could not be captured; declaring unverified names would be a
plausible-but-unverified check. Flagged as follow-up, not faked.
verify_dev_refresh.pytest coverage is one unit test on one seam(
test_cluster_health_uses_configured_broker_address, added inc9b1cc6f). It covers thebroker-address parameterization only — not gate behavior, not the manifest/health/contract-count
checks, not
refresh_dev_lane.shat all.Pre-existing dev breakage (not introduced here)
pre-commit run --all-filesfails one hook on a file this PR does not touch:tests/ci/test_runner_routing_audit.pyhasSPDX-FileCopyrightText: 2026where the validatorrequires
2025. It is unchanged since2f6fc271on dev and is absent from this diff. Left alonerather than smuggled into an unrelated PR.
Notes
evidence. The pre-push deploy-scope DoD hook returned
NOTICE_COMPANION_UNMERGEDfordocker/docker-compose.runners.yml; the hosted deploy-gate resolves it via Evidence-Source.OMN-14901 (sibling-clone
--force+ mixed-ref receipt).Summary by CodeRabbit
New Features
Documentation
Evidence-Ticket: OMN-14889
Evidence-Source: OCC#4555