Repository navigation
fix(OMN-15724): reduce needs-list guard job checkout/setup-uv cost - #6267
Conversation
validate-prod-promotion-grants and check-bot-authored-authz-guard pay unneeded fetch-depth:0 checkout + cold setup-uv provisioning on every run, the durable root cause behind OMN-15722's timeout-cancellation stopgap (measured OCC run 31222755192, head 2df62fc: checkout alone ranged 6s-22m38s across jobs in one run). AC1 (fetch-depth split): validate-prod-promotion-grants -> fetch-depth 1 (its validator has zero git/subprocess usage, single-file read). check-bot-authored-authz-guard stays fetch-depth 0 (shells git merge-base for the base-ref diff range). Guard logic and the @main grant-fetch construction are unchanged. AC2 (scoped devpi-cache activation): both jobs' setup-uv step now reaches the existing OMN-14027 devpi pull-through cache on .201 (healthy, soaked 12d, confirmed live) via job-level env (UV_DEFAULT_INDEX / UV_INDEX_STRATEGY / PIP_INDEX_URL / PIP_EXTRA_INDEX_URL), falling back to public PyPI for fork-origin PRs that resolve to ubuntu-latest and can't reach the Tailscale-only cache host. This is scoped to these 2 jobs only -- not the omnibase_infra fleet-wide rollout the 2026-08-08 canary measurement left unauthorized (whole-job p95 did not clear the operator standing rule at fleet scale); no runner-fleet compose/env mutation, no recreate, no image rebuild. See OMN-15724 ticket comment for full provenance and the AC2 scoping decision. AC3: p95 Checkout code baseline measured this session (25 most-recent completed ci.yml runs, gh api job/step timings): validate-prod-promotion-grants checkout p95=110s (n=18, median=16s), setup-uv p95=158s; check-bot-authored-authz-guard checkout p95=95s (n=17, median=19s), setup-uv p95=58s. Post-merge remeasurement is follow-up (AC3 threshold + AC4 observation window recorded in ticket evidence, not closed here). Refs: OMN-15724, OMN-15722, OMN-14027, OMN-15765
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 48 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 (6)
📝 WalkthroughWalkthroughThe CI workflow configures internal devpi and public PyPI fallback behavior for two validation jobs. The prod promotion validator uses shallow checkout, while the bot-authored authorization guard retains full history for merge-base validation. ChangesCI validation configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
OCC autobind did not mint a companion for this PR: no changed-file candidate could be proven RED against the merge base, and emitting a PR-existence probe instead would be non-falsifiable evidence (OMN-15247). Hand-authored evidence is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ci.yml:
- Around line 1577-1597: In .github/workflows/ci.yml ranges 1577-1597 and
1728-1748, add a health-check step before the shared setup-uv action that tests
the configured devpi index and switches UV_DEFAULT_INDEX to public PyPI when
unavailable. Ensure the fallback is expressed through uv’s supported index
configuration and applies before uv sync --all-groups runs in both job
definitions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| env: | ||
| UV_DEFAULT_INDEX: >- | ||
| ${{ | ||
| ( | ||
| github.event_name == 'pull_request' | ||
| && github.event.pull_request.head.repo.full_name != github.repository | ||
| ) | ||
| && 'https://pypi.org/simple/' | ||
| || 'http://omninode-pc.tail75df5e.ts.net:3141/root/pypi/+simple/' | ||
| }} | ||
| UV_INDEX_STRATEGY: unsafe-best-match | ||
| PIP_INDEX_URL: >- | ||
| ${{ | ||
| ( | ||
| github.event_name == 'pull_request' | ||
| && github.event.pull_request.head.repo.full_name != github.repository | ||
| ) | ||
| && 'https://pypi.org/simple/' | ||
| || 'http://omninode-pc.tail75df5e.ts.net:3141/root/pypi/+simple/' | ||
| }} | ||
| PIP_EXTRA_INDEX_URL: https://pypi.org/simple/ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect every index setting that can affect the two `uv run` commands.
rg -n -C 4 \
'UV_(DEFAULT_INDEX|EXTRA_INDEX_URL|INDEX_STRATEGY)|PIP_(INDEX_URL|EXTRA_INDEX_URL)|uv run' \
.github/workflows/ci.yml
# Inspect repository-level uv index configuration that can affect resolution.
fd -HI -a '^(pyproject\.toml|uv\.toml)$' . -x \
rg -n -C 3 'index|source|UV_' {}Repository: OmniNode-ai/onex_change_control
Length of output: 24284
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- trusted job ---'
sed -n '1528,1618p' .github/workflows/ci.yml
printf '%s\n' '--- authorization guard ---'
sed -n '1679,1782p' .github/workflows/ci.yml
printf '%s\n' '--- setup-uv action and project metadata ---'
fd -HI -a 'setup-uv|pyproject.toml|uv.toml' . | sort
rg -n -C 5 \
'devpi|tail75df5e|UV_DEFAULT_INDEX|UV_INDEX_URL|UV_EXTRA_INDEX_URL|PIP_EXTRA_INDEX_URL|uv version|astral-sh/setup-uv|setup-uv' \
.github pyproject.toml uv.toml 2>/dev/null || trueRepository: OmniNode-ai/onex_change_control
Length of output: 50387
🌐 Web query:
uv documentation environment variables UV_DEFAULT_INDEX UV_INDEX_URL UV_EXTRA_INDEX_URL index failure unavailable default index
💡 Result:
In uv, index configuration is managed through both environment variables and configuration files (pyproject.toml or uv.toml). Environment Variables uv supports several environment variables to configure package indexes [1][2][3]: - UV_DEFAULT_INDEX: Replaces the default index (equivalent to --default-index) [1][3]. - UV_INDEX: Sets additional indexes to search (equivalent to --index, added in 0.4.23) [2][3]. - UV_INDEX_URL: A legacy variable (deprecated) equivalent to --index-url, which acts as a default index [1][3]. - UV_EXTRA_INDEX_URL: A legacy variable (deprecated) equivalent to --extra-index-url, which acts as an additional index [1][3]. Index Behavior and Failure Handling By default, uv uses PyPI as the "default" index [4][5]. If you define custom indexes, uv generally treats them as part of a prioritized list [4][5]. Regarding index availability and failures: - Standard Behavior: If an index is configured and becomes unavailable (e.g., due to network issues, 401/403 errors, or a missing path), uv may attempt to fall back to other configured indexes, including the default PyPI index, if the package is found there [6][7]. This behavior has been a subject of user discussion regarding dependency confusion risks [7]. - Error Handling: Currently, uv typically treats index connectivity issues as errors rather than skipping them gracefully, unless specific configurations like explicit=true are used [8][9]. Users have requested features to treat missing indexes as warnings, but by default, an unreachable index can cause resolution to fail [8]. - Mitigation: To prevent unwanted fallbacks to PyPI, you can set authenticate = "always" for private indexes to fail fast if credentials are missing or invalid [7], or use the explicit = true attribute in your configuration to ensure an index is only consulted when explicitly requested or necessary [9][10]. For more complex requirements, such as restricting an index to specific packages or preventing fallback, using the [[tool.uv.index]] configuration in pyproject.toml or uv.toml is recommended, as it offers more granular control (e.g., setting default=true/false or explicit=true) than legacy environment variables [4][5][11].
Citations:
- 1: https://docs.astral.sh/uv/configuration/environment/
- 2: https://docs.astral.sh/uv/reference/environment/
- 3: https://github.com/astral-sh/uv/blob/main/crates/uv-static/src/env_vars.rs
- 4: https://docs.astral.sh/uv/concepts/indexes/index.md
- 5: https://docs.astral.sh/uv/concepts/indexes/
- 6: UV_DEFAULT_INDEX set but it falls back to pypi.org in some cases astral-sh/uv#11808
- 7: Prevent fallback to PyPi when private index returns 401/403 (to mitigate dependency confusion risk) astral-sh/uv#12362
- 8: Feature request: Add option to ignore missing/unavailable indexes astral-sh/uv#16301
- 9:
uv pip installtries to connect and validate non-default indexes. astral-sh/uv#14148 - 10:
tool.uv.indexsupercedes default PyPI for package resolution astral-sh/uv#12021 - 11: https://docs.astral.sh/uv/concepts/configuration-files/
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- setup-uv implementation ---'
find .github/actions/setup-uv -maxdepth 2 -type f -print -exec sed -n '1,220p' {} \;
printf '%s\n' '--- targeted index configuration ---'
rg -n \
'UV_DEFAULT_INDEX|UV_INDEX_URL|UV_EXTRA_INDEX_URL|UV_INDEX=|PIP_INDEX_URL|PIP_EXTRA_INDEX_URL|devpi|health|curl|wget|uv (sync|run)' \
.github/workflows/ci.yml .github/actions/setup-uv pyproject.toml
printf '%s\n' '--- read-only structural check of the two jobs ---'
python3 - <<'PY'
from pathlib import Path
text = Path(".github/workflows/ci.yml").read_text()
jobs = {
"validate-prod-promotion-grants": (text.index(" validate-prod-promotion-grants:"), text.index(" check-platform-leads-review-tripwire:")),
"check-bot-authored-authz-guard": (text.index(" check-bot-authored-authz-guard:"), text.index(" merge-hold-gate:")),
}
for name, (start, end) in jobs.items():
block = text[start:end]
print(name)
for line in block.splitlines():
stripped = line.strip()
if any(token in stripped for token in (
"UV_DEFAULT_INDEX", "UV_INDEX_URL", "UV_EXTRA_INDEX_URL",
"PIP_INDEX_URL", "PIP_EXTRA_INDEX_URL", "uv run", "uv sync",
"curl", "wget", "health",
)):
print(f" {stripped}")
print(" has_uv_public_extra_index:",
"UV_EXTRA_INDEX_URL" in block or "UV_INDEX=" in block)
print(" has_devpi_health_check:",
any(x in block.lower() for x in ("health check", "curl ", "wget ")))
PYRepository: OmniNode-ai/onex_change_control
Length of output: 9130
🌐 Web query:
site:docs.astral.sh/uv/concepts/indexes uv unreachable index fallback error default index
💡 Result:
In uv, there is no specific error message explicitly named unreachable index fallback. Instead, uv manages how it handles index errors and accessibility during dependency resolution through its internal configuration and error handling strategies [1]. Key behaviors regarding index errors and searching include: 1. Handling of HTTP Errors: By default, if uv encounters an HTTP error (other than 404) when fetching metadata or archives, it stops the resolution process for that specific package version, treating it as unavailable so the resolver can attempt to find a different version [1][2]. 2. Index Searching Strategy: When using multiple indexes, uv will stop searching across them if it encounters an HTTP 401 Unauthorized or HTTP 403 Forbidden error, with the exception of the pytorch index, which is permitted to return 403s when a package is missing [1][2]. 3. Ignoring Error Codes: You can customize this behavior using the ignore-error-codes setting within the [[tool.uv.index]] configuration [1][2]. This allows you to specify which HTTP status codes should be ignored rather than halting the resolution process. 4. 404 Not Found: uv will always continue to search across other configured indexes when it encounters a 404 Not Found error; this behavior is mandatory and cannot be overridden [1][2]. If an index is completely unreachable due to network issues (such as connection refused), it generally results in a failure to fetch metadata, which follows the default behavior of marking affected package versions as unavailable unless specific error handling is configured or the network environment is resolved [1][2].
Citations:
- 1: https://docs.astral.sh/uv/concepts/indexes/
- 2: https://docs.astral.sh/uv/concepts/indexes/index.md
Configure a public-PyPI fallback before dependency setup.
On trusted-runner paths, UV_DEFAULT_INDEX selects devpi, but PIP_INDEX_URL and PIP_EXTRA_INDEX_URL do not configure uv. The shared setup-uv action runs uv sync --all-groups before either validator, so a devpi outage can fail both jobs. Add a uv-compatible health-check fallback before setup-uv in both job definitions.
📍 Affects 1 file
.github/workflows/ci.yml#L1577-L1597(this comment).github/workflows/ci.yml#L1728-L1748
🤖 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 @.github/workflows/ci.yml around lines 1577 - 1597, In
.github/workflows/ci.yml ranges 1577-1597 and 1728-1748, add a health-check step
before the shared setup-uv action that tests the configured devpi index and
switches UV_DEFAULT_INDEX to public PyPI when unavailable. Ensure the fallback
is expressed through uv’s supported index configuration and applies before uv
sync --all-groups runs in both job definitions.
OCC PRs validate contracts/receipts from their own in-tree checkout (invariant I3) -- this contract binds the AC1/AC2 dod_evidence for the guard-job cost-reduction fix in the prior commit plus the PR self-bind.
Receipts for dod-occ-guard-fetch-depth-split, dod-occ-guard-cache- activation-scoped, dod-occ-guards-still-fail-closed (all executed directly against the committed ci.yml at f2e8c1a, exit 0, stdout "true"), and occ-self-bind-pr-6267 (live gh api readback of PR #6267 state, admissible shape per OMN-15309, non-count fields to avoid the file-count staleness trap OMN-15722's contract corrected twice).
…nvention The bare literal words "ubuntu-latest" / "self-hosted" in prose tripped the org-wide hardcoded-runner-label scan (omni-standards-compliance.yml), which greps all of .github/workflows/ for those exact tokens outside the sanctioned fromJSON(vars.OMNI_*) pattern -- it does not distinguish comments from runs-on directives. No behavior change; only comment wording. Reproduced the exact CI check logic locally before and after.
…on chain, admissible self-bind Two independent defects found by reproducing Contract Compliance Check locally before pushing (matches the exact 0/4 PASS, 4 WARN, 0 BLOCK -> BLOCK failure seen in CI): 1. All 4 dod_evidence entries were mistakenly chained via evidence_artifact: supersedes_dod_evidence:<prior-id>, copied from the OMN-15722 sibling ticket's pattern without understanding its semantics -- that field marks the PRIOR entry as a superseded correction, not "next item in an ordered list". Chaining 4 independent, non-corrective checks left only the last one "effective", so 0 hosted-and-local checks actually counted. Removed the supersedes chain; all 4 entries now stand alone. 2. occ-self-bind-pr-6267 used the bare `gh api .../pulls/6267` metadata endpoint, which the OMN-15309 admissibility predicate refuses as INSIDE_OWN_DIFF (metadata-only gh api endpoints prove the change exists, not that anything it claims is true -- see evidence_admissibility.py _GH_METADATA_ENDPOINT_RE). Replaced with the admissible pulls/6267/files endpoint, asserting membership (ci.yml present) plus a floor count (>=6) rather than an exact count, so the assertion does not go stale on further non-file- adding commits -- avoiding the exact-count staleness trap OMN-15722's own contract hit and corrected twice. Reproduced locally: `uv run python scripts/ci/run_contract_compliance_ check.py --pr 6267 --repo OmniNode-ai/onex_change_control --contracts-dir contracts` -> 4/4 PASS, 0 WARN, 0 BLOCK, matching what CI will run.
jonahgabriel
added
the
ci:ready
|
Note: the 'Evidence-Source: OCC#6268' reference in this PR is superseded. #6268 was the autobind companion minted against a pre-merge base; it carried only the generic template version of contracts/OMN-15724.yaml and add/add-conflicted with dev after this PR's own hand-authored contract (with substantive dod_evidence) merged at ff851a2. #6268 has been closed unmerged per the occ#6259 stale-companion precedent. Evidence for OMN-15724 lives in this PR's own merged contract entry at ff851a2. |
1 participant
OMN-15724: reduce needs-list guard job checkout/setup-uv cost
Durable follow-up to OMN-15722's timeout-cancellation stopgap (PR #6173). Root cause:
validate-prod-promotion-grantsandcheck-bot-authored-authz-guardpayfetch-depth: 0checkout + coldsetup-uvprovisioning on every run — measured OCC run31222755192(head2df62fc62):Checkout codealone ranged 6s-22m38s across jobs in one run, ~99.8% ofvalidate-prod-promotion-grants' total runtime is preamble, not the 2s check itself.AC1 — fetch-depth split (falsifiable, re-derived from source, not assumed)
validate-prod-promotion-grantsvalidate_prod_promotion_grants.pyhas zero git/subprocess usage (grep confirms no hits) — single-file read ofgrants/prod_promotion_grants.yaml.--pr-author/--base-refwere removed by OMN-14814 (dual-control unsatisfiable for a sole CODEOWNER).check-bot-authored-authz-guardgit fetch origin $BASE_REF(ci.yml) +git merge-base(check_bot_authored_authz_guard.pyL293/L304) to resolve the diff range. Genuinely needs base history.Neither trust-anchor guard's logic or the
@maingrant-fetch construction is touched — only checkout depth.AC2 — scoped devpi-cache activation (ticket comment posted before build, full provenance there)
Both jobs'
setup-uvstep now reaches the existing OMN-14027 devpi pull-through cache (omninode-pypi-cacheon .201 — confirmeddocker pshealthy, 12d uptime,curl .../root/pypi/+simple/200 this session) via job-levelenv:(UV_DEFAULT_INDEX/UV_INDEX_STRATEGY=unsafe-best-match/PIP_INDEX_URL/PIP_EXTRA_INDEX_URL=https://pypi.org/simple/), falling back to public PyPI when the existing fork-vs-trusted runner ternary resolves toubuntu-latest(fork PRs can't reach the Tailscale-only cache host).Important scoping note, found reading the ledger tail (not anticipated by the original dispatch): the
[mergesweep-0808-pypiroll]measurement (2026-08-08T18:32:39Z) found OMN-14027's canary did not clear the operator's fleet-roll criterion (whole-job p95 363s canary vs 321s control) and explicitly states "fleet roll NOT authorized." That verdict blocks theomnibase_infrafleet-wide path (uncommenting the runner-fleetdocker-compose.runners.ymlblock + rebuilding/recreating all 64 containers). This PR does not do that. It sets cache env at the job level in this repo's ownci.yml, for exactly these 2 jobs — noomnibase_inframutation, no runner recreate, no image rebuild, no change to any other job or repo. Reversible by deleting 4 env lines per job. Stays inside OMN-15724's own guardrail ("no runner-fleet capacity/scale/restart mutation").Honest caveat: the fleet-wide measurement found no proven whole-job-wall-time win, so this scoped change is a low-blast-radius application of a healthy, reachable, already-built mechanism to the 2 jobs this ticket is about — not itself a proven win. AC3 below measures these 2 jobs directly rather than inheriting the fleet verdict's inconclusive number.
AC3 — p95 baseline (measurement method + threshold stated, per ticket delegation to implementer)
Method:
gh api repos/OmniNode-ai/onex_change_control/actions/runs/{id}/jobs, pulled the 25 most-recent completedci.ymlruns (2026-08-09), extractedCheckout codeandsetup-uvstep wall-clock per job.Pre-fix baseline (this session):
validate-prod-promotion-grantscheck-bot-authored-authz-guardThreshold: post-merge p95
Checkout codeforvalidate-prod-promotion-grantsmust not exceed its own pre-fix p95 (110s) over the next ~20 completed runs — i.e. no regression, tracked as improvement where observed.Honest caveat (measured, not assumed): an already-
fetch-depth-default(1) sibling job in the same needs-list (check-platform-leads-review-tripwire) has a worse checkout p95 (303s, n=18, max 559s) thanvalidate-prod-promotion-grants' ownfetch-depth: 0baseline (110s) — confirming the ticket's own finding that checkout latency on this fleet is dominated by shared-host I/O contention, not fetch-depth.fetch-depth: 1reduces bytes transferred and is expected to help mean-case latency; it is not expected to fully tame contention-driven tail latency by itself. Stated honestly rather than claiming an unproven win — full closure of AC3/AC4 is a post-merge observation task.AC4 — post-landing observation window
Opens at merge. Not closed here — ticket stays In Review, not Done, until the observation window (zero
CI Summaryneeds:-memberconclusion=cancelledon timeout) is confirmed live.Out of scope (per ticket guardrails)
No runner-fleet capacity/scale/restart mutation (OMN-14016). No blanket timeout increases (OMN-15722 already covers the stopgap). No test/gate coverage weakened —
fetch-depthchange is job-scoped per the AC1 table above,check-bot-authored-authz-guard's guard logic and history requirement are untouched.Gates run
actionlintclean, YAML parse verified (fetch-depth/env values confirmed programmatically),pre-commit run --files .github/workflows/ci.ymlall-green including the yamlfmt-contamination ratchet (.github/is out of that gate's scope — yamlfmt itself excludes it, confirmed via direct script invocation) and its wiring anchor. No Python source touched; full local suite / mypy not applicable to this change.OMN-15724, dod_evidence: this PR + ticket comment (AC2 ruling, posted before build) + AC3/AC1 measurements above.
Summary by CodeRabbit
Evidence-Ticket: OMN-15724
Evidence-Source: OCC#6268