Skip to content

cicd: pin CI helper/action checkouts to PR head (#479) - #491

Open
BenjSz wants to merge 8 commits into
mainfrom
benjams/issue_479
Open

cicd: pin CI helper/action checkouts to PR head (#479)#491
BenjSz wants to merge 8 commits into
mainfrom
benjams/issue_479

Conversation

@BenjSz

@BenjSz BenjSz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Inconsistent code revisions in CI runs #479: CI jobs checked out two different commits for the same PR. build-hf-adapters-based test jobs used the PR head (${PR_HEAD_SHA:-$GITHUB_SHA}), while helper jobs and the Spyre jobs' .github/actions sparse checkouts used bare actions/checkout, which defaults to the synthetic merge commit GITHUB_SHA.
  • When a PR is behind main, this drift meant generate-matrix could emit legs for adapters that exist only in the merge commit, which then fail against the PR-head code; likewise a PR modifying a composite action ran the merged version, not the PR's.
  • Introduces one workflow-level env.TARGET_REF (= inputs.ref || github.event.pull_request.head.sha || github.sha, mirroring build-hf-adapters) and pins every actions/checkout ref: to it — 6 ubuntu-latest helper jobs plus all 26 Spyre test jobs (13 suites × first-run + retry) sparse .github/actions checkouts.
  • Simplifies generate-module-config-matrix to reuse the shared TARGET_REF instead of re-deriving it.

Test plan

  • YAML parses (yaml.safe_load)
  • Structural check: all 32 actions/checkout steps carry ref: env.TARGET_REF in their own with:; none leaked into setup-python/gather-runner-info/save-failed-suite-descriptor/build-hf-adapters
  • pre-commit (check-yaml, whitespace, etc.) passes
  • CI: on a PR intentionally behind main, confirm generate-matrix and the Spyre jobs report the same checked-out SHA and no phantom model legs appear
  • CI: confirm push-to-main and merge_group runs still resolve to github.sha

🤖 Generated with Claude Code

BenjSz and others added 2 commits September 8, 2026 11:25
The ubuntu-latest helper jobs and every Spyre job's sparse .github/actions
checkout used bare actions/checkout, resolving to the synthetic merge commit
GITHUB_SHA instead of the PR head that build-hf-adapters tests. This made the
generated matrix and composite actions drift from the code under test whenever
a PR was behind main. Resolve all checkouts to a single env.TARGET_REF mirroring
build-hf-adapters' ${PR_HEAD_SHA:-$GITHUB_SHA} plus the Test-With inputs.ref.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
@BenjSz
BenjSz marked this pull request as ready for review September 8, 2026 08:28
@BenjSz
BenjSz requested a review from kmehant as a code owner September 8, 2026 08:28

@arielge arielge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably makes more sense to use the GITHUB_SHA for everything - this means changing the behavior of build-hf-adapters, not the ref used in the test matrix

BenjSz and others added 3 commits September 8, 2026 12:23
Per review, resolve the PR/merge-commit drift by pointing build-hf-adapters
at GITHUB_SHA (what actions/checkout already defaults to) instead of pinning
every helper/action checkout to the PR head. Reverts the _test_matrix ref
overrides.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Benjamin Sznajder <benjams@il.ibm.com>
@BenjSz
BenjSz requested a review from arielge September 8, 2026 10:17
@BenjSz

BenjSz commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

It probably makes more sense to use the GITHUB_SHA for everything - this means changing the behavior of build-hf-adapters, not the ref used in the test matrix

I fixed according to your recommendation

@BenjSz
BenjSz enabled auto-merge September 8, 2026 13:11
@spyre-ci

spyre-ci Bot commented Sep 8, 2026

Copy link
Copy Markdown

⚙️ Auto-merge enabled — dispatched /spyre-test for ce02d2d0 because the required Spyre Test gate had not run on this commit.

Automated: enabling auto-merge requests the gate, so the PR is not left waiting on a check nobody asked for. Re-enabling auto-merge on this same commit will not dispatch again.

@spyre-ci

spyre-ci Bot commented Sep 8, 2026

Copy link
Copy Markdown

❌ spyre-test: failure

Triggered by: /spyre-test comment

Plan (build waves + dependencies, per arch)

amd64

flowchart LR
  subgraph Lamd64_0["amd64 L0 · 1 parallel"]
    n_amd64_torch_spyre_torch_spyre_dev["torch-spyre/torch-spyre-dev 🔴<br/>image · 72411d836e22"]
  end
  subgraph Lamd64_1["amd64 L1 · 1 parallel"]
    n_amd64_hf_adapters_hf_adapters_dev["hf-adapters/hf-adapters-dev<br/>image · 3bc9e820f99f"]
  end
  subgraph Lamd64_2["amd64 L2 · 1 parallel"]
    n_amd64_spyre_inference_spyre_inference_dev["spyre-inference/spyre-inference-dev<br/>image · 5b917b35d3f4"]
  end
  n_amd64_torch_spyre_torch_spyre_dev --> n_amd64_hf_adapters_hf_adapters_dev
  n_amd64_hf_adapters_hf_adapters_dev --> n_amd64_spyre_inference_spyre_inference_dev
  classDef sPending fill:#eceff1,stroke:#90a4ae,color:#37474f
  classDef sBuilding fill:#fff8e1,stroke:#f9a825,color:#5d4037,stroke-width:2px
  classDef sOk fill:#e8f5e9,stroke:#43a047,color:#1b5e20
  classDef sReused fill:#e3f2fd,stroke:#1e88e5,color:#0d47a1
  classDef sFailed fill:#ffebee,stroke:#e53935,color:#b71c1c,stroke-width:2px
  classDef sDropped fill:#f5f5f5,stroke:#bdbdbd,color:#9e9e9e
  class n_amd64_torch_spyre_torch_spyre_dev sOk;
  class n_amd64_hf_adapters_hf_adapters_dev sFailed;
  class n_amd64_spyre_inference_spyre_inference_dev sDropped;
Loading

⚠️ orch trigger-pr-validationyellow · arches amd64 · fp amd64=9847eaa0

level component arch build smoke unit integration trunk regression perf
L0 torch-spyre/torch-spyre-dev amd64 ✅ ok · 🟡 gha: not dispatched · · ·
L1 hf-adapters/hf-adapters-dev amd64 ❌ failed ⏭️ · ⏭️ · · ·
L2 spyre-inference/spyre-inference-dev amd64 ⛔ dropped ⏭️ · ⏭️ · · ·

❌ build failed (1) — do not merge

Build: built 1 · failed 1 · dropped 1

  • build · hf-adapters/hf-adapters-dev/amd64: failed
    • depends on: torch-spyre

Tests: passed 0 · blocking 0 · advisory 1 · infra/inconclusive 0 · no signal 0

Failures by kind
  • ⚠️ advisory (does not block) · torch-spyre/amd64 integration: ABORTED

Before merging, consider:

  • hf-adapters/hf-adapters-dev/amd64 failed to BUILD — this blocks the merge. No test result below says anything about this change: those legs never dispatched. Fix the build first, then re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent code revisions in CI runs

2 participants