From 6a49805e816aaa999c4da971db45fb9da4f56039 Mon Sep 17 00:00:00 2001 From: coseto6125 <80243681+coseto6125@users.noreply.github.com> Date: Sun, 24 May 2026 02:22:58 +0800 Subject: [PATCH] refactor(ci): use event-payload SHAs instead of network fetch of base ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause for today's recurring 'could not read Username for https://github.com' flakes (#388 macos, #393 ubuntu, #395 dep-review, #397 main-push, #401 ubuntu Test, #402 instrumentation): the runner image ships with a default credential.helper in /etc/gitconfig that errors with ENXIO when git falls back to it. actions/checkout sets up http.extraheader scoped to the repo URL, but on certain runner image revisions the auth setup escapes our isolation and the system helper gets invoked anyway. Rather than work around the broken helper (which would leave a permanent shellcheck-style `-c credential.helper=` debt on every git command), we eliminate the failure surface entirely: the only steps that do post-checkout fetches all want the same thing — main's tip SHA — and GitHub already provides that in the pull_request event payload (`github.event.pull_request.base.sha`). # ci.yml — `Detect code changes` job Was: git fetch --no-tags origin "$BASE_REF" diff_range="origin/$BASE_REF...HEAD" Now: # Event payload exposes base.sha for free; checkout used default # ref (refs/pull/N/merge) so both sides are in local object DB. diff_range="$BASE_SHA...HEAD" Three-dot range still gives merge-base..HEAD semantics — equivalent to the old behavior, no network needed. # ecp-pr-analyze.yml — drop `Fetch base ref` + recompute branch point locally Was: - uses: actions/checkout@v6.0.2 with: ref: ${{ pull_request.head.sha }} # only PR head ancestors fetched - name: Fetch base ref run: git fetch origin "$BASE_REF:..." # network — triggers ENXIO flake ... BASE=$(git merge-base "origin/$BASE_REF" HEAD) Now: - uses: actions/checkout@v6.0.2 with: fetch-depth: 0 # No `ref:` override. Default refs/pull/N/merge brings both PR # head AND base history into local object DB. - name: Compute branch point + switch HEAD to PR head run: | PR_HEAD=$(git rev-parse HEAD^1) # merge ref's parent 1 BASE_TIP=$(git rev-parse HEAD^2) # merge ref's parent 2 BRANCH_POINT=$(git merge-base "$PR_HEAD" "$BASE_TIP") git checkout "$PR_HEAD" Branch point is the SAME value the old `git merge-base origin/` would produce — but derived purely from local objects (the merge ref's two parents) instead of a network fetch. # Edge cases - PR with merge conflicts: GitHub doesn't compute refs/pull/N/merge, checkout fails. This is correct — conflicted PRs can't merge, so ecp impact analysis would be meaningless. Author resolves conflict, ref recomputed, next run works. - Push to main / merge_group / workflow_dispatch: unchanged code path (already used BEFORE_SHA / blanket 'code=true', no fetch). # Result - One entire class of CI flake eliminated: no post-checkout git fetch means no credential-helper invocation means no ENXIO. - No upstream-bug workaround comment debt. - Slightly faster CI (one fewer network round-trip per PR job). - Closes the path that diagnostic instrumentation in PR #402 was trying to capture; PR #402 can be closed once this lands. --- .github/workflows/ci.yml | 18 ++++++---- .github/workflows/ecp-pr-analyze.yml | 49 ++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72b32942..7ebf6f9b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,12 @@ jobs: shell: bash env: BASE_REF: ${{ github.base_ref }} + # PR event payload exposes the merge-base-equivalent SHA for free. + # Using it avoids the post-checkout `git fetch origin ` that + # intermittently 401s on this runner image — the fetch path was + # the only thing requiring git credentials in this job, and dropping + # it eliminates an entire class of "could not read Username" flake. + BASE_SHA: ${{ github.event.pull_request.base.sha }} EVENT_NAME: ${{ github.event_name }} BEFORE_SHA: ${{ github.event.before }} run: | @@ -91,12 +97,12 @@ jobs: case "$EVENT_NAME" in pull_request) - # No --depth=1: checkout used fetch-depth: 0, so we already - # have full history. A shallow fetch here would truncate - # origin/$BASE_REF to its tip and break the merge-base lookup - # (origin/main...HEAD → "no merge base"). - git fetch --no-tags origin "$BASE_REF" - diff_range="origin/$BASE_REF...HEAD" + # base.sha (PR's branch-point on the base ref) is always an + # ancestor of HEAD when checkout uses fetch-depth: 0, so it's + # already in our local object DB — no network fetch needed. + # Three-dot diff range gives merge-base...HEAD semantics + # which is what we want for "files this PR changes". + diff_range="$BASE_SHA...HEAD" ;; push) # $BEFORE_SHA is the previous HEAD on this ref; 0..0 means diff --git a/.github/workflows/ecp-pr-analyze.yml b/.github/workflows/ecp-pr-analyze.yml index 85699dee..29413627 100644 --- a/.github/workflows/ecp-pr-analyze.yml +++ b/.github/workflows/ecp-pr-analyze.yml @@ -40,12 +40,39 @@ jobs: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: fetch-depth: 0 - ref: ${{ github.event.pull_request.head.sha }} - - - name: Fetch base ref - env: - BASE_REF: ${{ github.event.pull_request.base.ref }} - run: git fetch origin "$BASE_REF:refs/remotes/origin/$BASE_REF" + # No `ref:` override. The default for pull_request events is + # refs/pull/N/merge — a synthetic merge commit GitHub computes + # whose two parents are (^1) PR head and (^2) current base tip. + # fetch-depth: 0 pulls full history reachable from this merge + # ref, which is exactly { PR head ancestors ∪ base tip ancestors }. + # That gives us everything we need locally — no follow-up + # `git fetch origin ` (which triggers the runner's + # credential-helper flake: "could not read Username … ENXIO"). + # + # If a PR has merge conflicts, GitHub doesn't compute this ref + # and checkout fails — that's correct behavior (a conflicting + # PR can't merge anyway, ecp analysis would be meaningless). + + - name: Compute branch point + switch HEAD to PR head + id: refs + run: | + # Branch point = merge-base of PR head and current base tip, + # both available as parents of the merge ref's HEAD commit. + # This is the SAME value `git merge-base origin/ ` + # would produce — derived purely from local objects, no network. + PR_HEAD=$(git rev-parse HEAD^1) + BASE_TIP=$(git rev-parse HEAD^2) + BRANCH_POINT=$(git merge-base "$PR_HEAD" "$BASE_TIP") + echo "pr-head=$PR_HEAD" >> "$GITHUB_OUTPUT" + echo "branch-point=$BRANCH_POINT" >> "$GITHUB_OUTPUT" + echo "pr head: $PR_HEAD" + echo "base tip: $BASE_TIP" + echo "branch point: $BRANCH_POINT" + # Detach HEAD onto PR head so subsequent steps (cargo build / + # ecp index / pr-analyze) see the PR's actual tree, not the + # synthetic merged tree (which can differ from PR head if any + # merge resolution applied). + git checkout "$PR_HEAD" - uses: actions-rust-lang/setup-rust-toolchain@v1 with: @@ -91,15 +118,9 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} BASE_REF: ${{ github.event.pull_request.base.ref }} + BASE: ${{ steps.refs.outputs.branch-point }} run: | - # Compute merge-base against the PR's actual target branch so the - # diff reflects this PR's delta rather than divergence vs. wherever - # the target branch happens to be right now. Target branch can be - # main / beta / develop / release-* etc. — never hardcode. - # (PR #382 first smoke produced 20+ phantom symbols because raw - # `origin/main` had moved past the PR's branch point.) - BASE=$(git merge-base "origin/$BASE_REF" HEAD) - echo "target: $BASE_REF baseline (merge-base): $BASE" + echo "target: $BASE_REF baseline (branch point): $BASE" ./target/release/ecp \ dev pr-analyze \ --baseline "$BASE" \