Repository navigation
fix(OMN-16114): extend C2 git-mirror rewrite to sibling actions/checkout steps - #2768
Conversation
…out steps The C2 git mirror only ever accelerated two things: the primary GITHUB_REPOSITORY checkout (workspace pre-seed, keyed off GITHUB_REPOSITORY) and uv's git-dependency fetches (a fetch-only insteadOf for the .git-suffixed URL form). Neither mechanism ever looked at a step's `repository:` input, so sibling `actions/checkout` steps -- e.g. dispatcher-route-coverage.yml's "Checkout omnimarket (sibling)" -- went straight to github.com with zero acceleration even when the target repo was already mirrored. Confirmed root cause of ~27% of dispatcher-route-coverage runs timing out at the 30-minute job budget (RPC 408 / GnuTLS -110 / empty-reply, retried unbounded until the timeout killed the job); ~13 workflow files carry the same unmirrored sibling-checkout pattern. Adds wire_sibling_checkout_mirror_rewrite (C2c) to docker/runners/runner-job-started.sh: it reads the current job's own workflow YAML (via GITHUB_WORKFLOW_REF + GITHUB_JOB, fetched at GITHUB_SHA from the already-pre-seeded own-repo workspace) to discover every (repo, ref) pair a sibling actions/checkout step in THIS job will request, proves the mirror can serve every one of them (by-SHA probe for exact-SHA pins, ls-remote for branch/tag literals), and only then installs a fetch-only insteadOf for that repo. A GHA expression ref, a missing ref, or any unservable pin disqualifies the whole repo for the job -- conjunctive gating, since insteadOf rewrites the URL, not one ref, so a partially-servable repo (e.g. ci.yml's application-database-domain-enforcement job checks out omnibase_infra twice at two different pinned SHAs) must not be redirected at all. own_repo is excluded unconditionally, even under a self-referential `repository:` entry, because it shares the primary checkout's URL form and the primary fetches a possibly seconds-old GITHUB_SHA the mirror may not have yet. Refactors wire_uv_git_mirror_rewrite (C2b) to share a new accumulator (_c2_rewrite_add_pair / _c2_rewrite_flush) with the new function: both mechanisms discover fetch-only insteadOf pairs into one GIT_CONFIG_COUNT namespace and flush once, so the two GITHUB_ENV writes can no longer clobber each other's KEY_i/VALUE_i entries. Also factors the GITHUB_SHA delta-fetch (previously inline in C2b) into a shared, idempotent _c2_ensure_head_sha_fetched helper reused by both. Fail-open throughout, matching every existing mirror mechanism: missing GITHUB_ENV, missing git, unseeded workspace, unreadable workflow file, no sibling checkouts in this job's block, or any probe miss all fall through to today's behavior (github.com, unmirrored). No workflow file changes -- this is a mechanism-only fix reachable by all ~13 affected workflows without per-workflow patches. Verified empirically (not assumed) that insteadOf has no server-side fallback: a sibling checkout whose exact SHA is absent from the mirror is never rewritten, because if it were, actions/checkout would hard-fail rather than falling through to github.com. New test file tests/scripts/test_runner_job_started_mirror_rewrite.py exercises this and seven other scenarios (branch-ref present/absent, exact-SHA present/absent, conjunctive gating across two pins of one repo, a dynamic GHA-expression ref, own-repo exclusion, and the shared-accumulator merge) against a real local `git daemon`, matching the production daemon's own invocation shape -- no mocking of git. 9/10 pass locally; the 10th (full-script subprocess run) is gated on GNU `realpath -m` exactly like the existing debris-cleanup test file and runs for real on the Ubuntu 22.04 CI runners. Host-side note: runner-job-started.sh is bind-mounted by inode from /home/jonah/.omnibase/runners/docker/runners/runner-job-started.sh on the runner host. This PR is repo-side only -- the host copy still needs the documented in-place `cat > ...` update (never mv/install, see docker/runners/README-c2b-uv-git-mirror.md's inode-splitting trap) after merge to take effect on the fleet. No other host change is required: this reuses the existing mirror set, refresh timer, and per-mirror allowFilter/allowAnySHA1InWant config as-is. OMN-16114
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 7 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 112 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. 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 (2)
Comment |
✅ 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) |
#6595) * evidence(OMN-16114): author OCC companion for OmniNode-ai/omnibase_infra#2768 The occ-autobind born-path event for this PR was swallowed at the consumer twice in a row -- not a replay-tooling failure, a delivery-guarantee gap. Confirmed forensically via live workflow logs rather than assumed: 1. Original PR-open trigger (pull_request event, run 32008569724, 08:03:41Z, head f18eee68f0933eb3206f909c4d8079e952647bb3): produced no companion. 2. Manual workflow_dispatch replay #1 (run 32010077750, job 95327597955): the publish step ran clean -- "Published onex.cmd.omnimarket.occ-autobind.v1 event_id=9e01122d-9253-40b1-83c5-ef2583280482" at 08:25:14Z, to the dev-lane broker (omninode-pc.tail75df5e.ts.net:19092). No companion appeared after 74+ minutes. 3. Manual workflow_dispatch replay #2, fired from this lane (run 32016316098, job 95346440127): also published clean -- "Published onex.cmd.omnimarket.occ-autobind.v1 event_id=e3defa43-ff9a-4c67-9b7c-72fe9e7bd77c" at 09:44:30Z (the job's overall conclusion shows "cancelled", but that fired AFTER the publish step completed and logged success -- the cancellation did not touch the already-published event). No companion after 5+ minutes at authoring time. Two independent, successfully-published events for the same PR/ticket with zero consumption is new evidence for the delivery-guarantee defect thread -- the failure is at the consumer/effect-handler side, not the publisher. Hand-authored on the occ#6573/OMN-10221 and OMN-16112/occ#6589 manual-companion precedent per controller authorization. Learned from the occ#6589 lane (same session): bare `gh pr view --json ...` is the OMN-15309-inadmissible "PR-existence probe" anti-pattern, so both dod_evidence checks here are built admissible from the start -- content-bound, RED-verified probes, not existence probes: dod-OmniNode-ai-omnibase_infra-pr-2768: gh api .../contents/docker/runners/runner-job-started.sh?ref=<sha> reads the actual file content at #2768's head and asserts the new shared rewrite-flush function (_c2_rewrite_flush) is present. RED-verified: zero matches at the PR's base ref. dod-OmniNode-ai-omnibase_infra-pr-2768-ci: gh api .../pulls/2768/files reads the actual diff (admitted per the module's own guidance) and asserts the exact two-file change set. Locally verified via onex_change_control.validation.evidence_admissibility .classify_evidence directly: both items -> ADMISSIBLE. Local validator_occ_merge_eligibility run against #2768's real title/branch/ commit-sha with this commit's on-disk contract+receipts returns eligible:true. STILL REQUIRED before this can pass occ-preflight on its own PR: the occ-self-bind-pr-<N> entry + receipt, which cannot exist until the PR number does -- same two-commit shape as the merged #6573/#903/#6589 companions. Evidence-Ticket: OMN-16114 * evidence(OMN-16114): occ-self-bind-pr-6595 Appends the mandatory self-bind evidence item for OCC companion PR #6595, per the merged #903/#6492/#6573/#6589 pattern (OMN-14650). This item cannot exist before the PR number does, so it lands as a second commit after opening #6595. Local verification, all green: - validator_occ_append_only --ticket-id OMN-16114 (base = merge-base with origin/dev) -> ok:true. - validator_occ_merge_eligibility run twice: once as onex_change_control's own in-tree PR (#6595, its own title/branch/commit) -> eligible:true, and once simulating omnibase_infra#2768's real body with a live 'Evidence-Source: OCC#6595' line appended -> eligible:true. - check-receipt-honesty, check-contract-substance-floor, check-contract-shape-v1, check-dod-authoring-hygiene, check-receipt-hardening all Passed locally. Evidence-Ticket: OMN-16114 * fix(OMN-16114): SIGPIPE-safe the two content-bound probes (Rule E) #6595's own Contract Corpus Ratchets (OMN-15411) check failed the census baseline: both new dod_evidence check_values matched the measured SIGPIPE-fragile shape. dod-OmniNode-ai-omnibase_infra-pr-2768: `... | base64 -d | grep -q ...` -- the "base64-decoded file body" producer, measured 141,0,141,0,141 across 5 runs against the corpus' own real inputs. dod-OmniNode-ai-omnibase_infra-pr-2768-ci: `--jq '[.[].filename]' | grep -qxF ...` -- the "iterating jq projection" producer (unbounded output by construction, `.[]` in the jq expression). `grep -q`/`grep -qxF` exit at the first match and close stdin; if the upstream producer still has bytes to write it is killed by SIGPIPE (exit 141), which `bash -o pipefail` propagates as a false RED on genuinely-passing evidence. Rewritten per the documented repair idiom (occ#5496/#5523 precedent, fetched and matched field-for-field): buffer the producer's full output into a shell variable first, so it runs to completion before anything reads from it, then pipe a printf of that variable into grep -qF. Re-verified both probes live against #2768's real head -- same PASS result, same RED-control failure against wrong input. Verified against the actual gates, not inferred: - onex_change_control.validation.evidence_admissibility.classify_evidence -> both items still ADMISSIBLE (gh-api + grep in command position unaffected by the buffering). - scripts/lint_contract_check_values._sigpipe_producer_label -> None for all three dod_evidence items (Rule E clean). - uv run pytest tests/unit/scripts/test_lint_contract_check_values_corpus_baseline.py -> 17/17 passed locally (was 3 failed before this fix). - validator_occ_append_only --ticket-id OMN-16114 -> ok:true (all four files still diff as pure 'A' against the origin/dev merge-base). - validator_occ_merge_eligibility (companion's own in-tree binding, both commits cited) -> eligible:true, unchanged. contract_entry_sha256 recomputed for the two edited items via the canonical hashers; occ-self-bind-pr-6595's own per-entry hash is byte-identical (unaffected, confirming append-immunity). Evidence-Ticket: OMN-16114
fix(OMN-16114): extend C2 git-mirror rewrite to sibling actions/checkout steps
The C2 git mirror only ever accelerated two things: the primary
GITHUB_REPOSITORY checkout (workspace pre-seed, keyed off GITHUB_REPOSITORY)
and uv's git-dependency fetches (a fetch-only insteadOf for the .git-suffixed
URL form). Neither mechanism ever looked at a step's
repository:input, sosibling
actions/checkoutsteps -- e.g. dispatcher-route-coverage.yml's"Checkout omnimarket (sibling)" -- went straight to github.com with zero
acceleration even when the target repo was already mirrored. Confirmed root
cause of ~27% of dispatcher-route-coverage runs timing out at the 30-minute
job budget (RPC 408 / GnuTLS -110 / empty-reply, retried unbounded until the
timeout killed the job); ~13 workflow files carry the same unmirrored
sibling-checkout pattern.
Adds wire_sibling_checkout_mirror_rewrite (C2c) to
docker/runners/runner-job-started.sh: it reads the current job's own workflow
YAML (via GITHUB_WORKFLOW_REF + GITHUB_JOB, fetched at GITHUB_SHA from the
already-pre-seeded own-repo workspace) to discover every (repo, ref) pair a
sibling actions/checkout step in THIS job will request, proves the mirror can
serve every one of them (by-SHA probe for exact-SHA pins, ls-remote for
branch/tag literals), and only then installs a fetch-only insteadOf for that
repo. A GHA expression ref, a missing ref, or any unservable pin disqualifies
the whole repo for the job -- conjunctive gating, since insteadOf rewrites the
URL, not one ref, so a partially-servable repo (e.g. ci.yml's
application-database-domain-enforcement job checks out omnibase_infra twice
at two different pinned SHAs) must not be redirected at all. own_repo is
excluded unconditionally, even under a self-referential
repository:entry,because it shares the primary checkout's URL form and the primary fetches a
possibly seconds-old GITHUB_SHA the mirror may not have yet.
Refactors wire_uv_git_mirror_rewrite (C2b) to share a new accumulator
(_c2_rewrite_add_pair / _c2_rewrite_flush) with the new function: both
mechanisms discover fetch-only insteadOf pairs into one GIT_CONFIG_COUNT
namespace and flush once, so the two GITHUB_ENV writes can no longer clobber
each other's KEY_i/VALUE_i entries. Also factors the GITHUB_SHA delta-fetch
(previously inline in C2b) into a shared, idempotent
_c2_ensure_head_sha_fetched helper reused by both.
Fail-open throughout, matching every existing mirror mechanism: missing
GITHUB_ENV, missing git, unseeded workspace, unreadable workflow file, no
sibling checkouts in this job's block, or any probe miss all fall through to
today's behavior (github.com, unmirrored). No workflow file changes -- this
is a mechanism-only fix reachable by all ~13 affected workflows without
per-workflow patches.
Verified empirically (not assumed) that insteadOf has no server-side
fallback: a sibling checkout whose exact SHA is absent from the mirror is
never rewritten, because if it were, actions/checkout would hard-fail rather
than falling through to github.com. New test file
tests/scripts/test_runner_job_started_mirror_rewrite.py exercises this and
seven other scenarios (branch-ref present/absent, exact-SHA present/absent,
conjunctive gating across two pins of one repo, a dynamic GHA-expression ref,
own-repo exclusion, and the shared-accumulator merge) against a real local
git daemon, matching the production daemon's own invocation shape -- nomocking of git. 9/10 pass locally; the 10th (full-script subprocess run) is
gated on GNU
realpath -mexactly like the existing debris-cleanup test fileand runs for real on the Ubuntu 22.04 CI runners.
Host-side note: runner-job-started.sh is bind-mounted by inode from
/home/jonah/.omnibase/runners/docker/runners/runner-job-started.sh on the
runner host. This PR is repo-side only -- the host copy still needs the
documented in-place
cat > ...update (never mv/install, seedocker/runners/README-c2b-uv-git-mirror.md's inode-splitting trap) after
merge to take effect on the fleet. No other host change is required: this
reuses the existing mirror set, refresh timer, and per-mirror
allowFilter/allowAnySHA1InWant config as-is.
OMN-16114
Evidence-Source: OCC#6595
Evidence-Ticket: OMN-16114