Skip to content

ci: SHA-pin actions, drop net9 from benchmarks, build timeout - #768

Merged
tylerkron merged 2 commits into
mainfrom
cursor/gha-sha-pin-ci-benchmarks-3e34
Sep 27, 2026
Merged

tylerkron merged 2 commits into
mainfrom
cursor/gha-sha-pin-ci-benchmarks-3e34

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

release.yml already SHA-pins actions/checkout, actions/setup-dotnet, and NuGet/login. ci.yml and benchmarks.yml still used floating tags (@v7 / @v6). If an upstream tag were compromised or moved, what CI and the on-demand benchmark job run would change with no change in this repo.

benchmarks.yml also installed 9.0.x even though the benchmark host targets net10.0 only. The required build aggregator job (if: always(), no checkout) had no timeout, so it fell back to GitHub's 360-minute default.

How it was fixed

  • SHA-pin every action in ci.yml and benchmarks.yml the same way release.yml does: a full 40-character commit SHA plus a trailing # vX.Y.Z comment. Dependabot's github-actions updater (.github/dependabot.yml, weekly) keeps both the SHA and the comment current.
    • actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 (v7.0.1)
    • actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 (v6.0.0)
    • actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 (v6.1.0)
    • actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a (v7.0.1)
  • benchmarks.yml: install 10.0.x only. A short comment at that step says 9.0.x has to come back if a benchmark class adds [SimpleJob(RuntimeMoniker.Net90)], which the benchmarks README suggests for measuring on .NET 9.
  • ci.yml build job: timeout-minutes: 5. The job only echoes the matrix result and takes a few seconds.

Verification

  • Checked each pinned SHA with gh api repos/<owner>/<repo>/git/ref/tags/<tag>. All four are lightweight tags that point directly at the commit, and each SHA is also where the floating major tag (v7 / v6) points today. So the pins change nothing about what runs now.
  • Daqifi.Core.Benchmarks.csproj is net10.0 only. There is no global.json, and no benchmark uses a net9 RuntimeMoniker. I dispatched the Benchmarks workflow on this branch with only 10.0.x installed (*ChannelScaling*, short job). The run built with SDK 10.0.401, executed 3 benchmarks on .NET 10.0.12, and uploaded the report artifact.
  • actionlint reports nothing for ci.yml or benchmarks.yml.

Out of scope: release.yml (pinned in #747). Overlap: #774 and #781 (ci.yml) and #791 (benchmarks.yml) edit different lines. Test-merging all three on top of this branch produces no conflicts.

🤖 Generated with Claude Code

Co-authored-by: Tyler Kron <tylerkron@gmail.com>
@tylerkron
tylerkron requested a review from a team as a code owner September 21, 2026 10:14
@qodo-code-review

qodo-code-review Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 60002a8 🚀 Fast

Results up to commit 3977d12 ⚖️ Balanced


No changes from previous review

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Pin CI actions, simplify benchmark SDKs, and bound build gate

⚙️ Configuration changes ✨ Enhancement 🕐 Less than 10 minutes

Grey Divider

AI Description

• Pins CI and benchmark actions to immutable, reviewed commit SHAs.
• Removes the unused .NET 9 SDK from the net10.0 benchmark workflow.
• Limits the required build aggregator job to five minutes.
Diagram

graph TD
  Dispatch["Manual Dispatch"] --> Benchmark["Benchmark Job"] --> DotNet10[".NET 10 SDK"] --> Reports["Report Artifact"]
  Events["PR Queue Push"] --> Test["Test Matrix"] --> Build["5m Build Gate"]
  Pinned["Pinned Actions"] --> Benchmark & Test
Loading
High-Level Assessment

The chosen approach is appropriate: full commit SHAs provide stronger supply-chain guarantees than floating major-version tags, while inline version comments preserve readability and Dependabot can maintain revisions. Reusing the action versions already established in release.yml also keeps workflow policy consistent.

Files changed (2) +10 / -11

Other (2) +10 / -11
benchmarks.ymlPin benchmark actions and install only .NET 10 +5/-7

Pin benchmark actions and install only .NET 10

• Pins checkout, setup-dotnet, cache, and upload-artifact to immutable release SHAs. Removes the unnecessary .NET 9 SDK installation because the benchmark host targets net10.0 only.

.github/workflows/benchmarks.yml

ci.ymlPin CI actions and bound the required build gate +5/-4

Pin CI actions and bound the required build gate

• Pins checkout, setup-dotnet, cache, and upload-artifact to immutable release SHAs. Adds a five-minute timeout to the always-running build aggregator so stalled checks cannot consume GitHub's default six-hour window.

.github/workflows/ci.yml

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 60002a8

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review

@tylerkron
tylerkron added this pull request to the merge queue Sep 27, 2026
Merged via the queue into main with commit fb8bf42 Sep 27, 2026
5 checks passed
@tylerkron
tylerkron deleted the cursor/gha-sha-pin-ci-benchmarks-3e34 branch September 27, 2026 17:36
tylerkron added a commit that referenced this pull request Sep 28, 2026
Resolves the pack-step comment conflict with #774: keep main's -c Release
run: lines and #768's SHA pins, and state that pack reuses the Release build
(skipping the compile) while still running ApiCompat.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants