chore(ci): cache pre-commit envs + move heavy test hooks to pre-push#342
Merged
Merged
Conversation
The pre-commit CI job has crept from ~8 min to ~10 min as the test
suites grew. Two changes here, each independently safe:
1. .github/workflows/pre-commit.yml — add three caches:
- actions/setup-node@v6 with cache: 'npm' on frontend/package-lock.json
so `npm ci` reuses ~/.npm cache (~30-40s saved). Same pattern
already in frontend-build-sentinel.yml.
- actions/cache for ~/.cache/pre-commit keyed on the hook config
so per-hook venvs (Go, Python, Node) aren't rebuilt every run
(~30-60s saved).
- actions/cache for ~/.cache/go-build keyed on go.sum so go-vet,
gosec, and any Go-compiling hooks reuse compiled object files
rather than rebuilding from source.
- actions/cache for ~/go/bin keyed on the pinned tool versions so
gosec v2.22.4 and gocyclo v0.6.0 binaries don't get rebuilt by
`go install` every run.
2. .pre-commit-config.yaml — move go-test, frontend-build, frontend-test
from default stages to `stages: [pre-push]`. The CI workflow runs
`pre-commit run --all-files` without a stage filter, so the default
stage filter (`pre-commit`) skips these hooks. Local devs who run
`pre-commit install --hook-type pre-push` still get them on push.
These three hooks were the bulk of the ~485s "Run pre-commit" step
and are redundant with dedicated CI workflows that PRs / pushes
already trigger:
- go-test (-short -race ./...) → ci.yml `unit-tests` runs the
same suite with -race AND an integration pass with
-tags=integration.
- frontend-build (npm run build) → frontend-build.yml runs
npm run typecheck + npm run build on PRs; frontend-build-
sentinel.yml runs the build on every push to main / feat/**.
- frontend-test (jest) → frontend-build-sentinel.yml runs
`npx jest --no-coverage --silent` on every push to feat/**
(which fires on every PR-branch update).
Net expected impact: the pre-commit CI job drops from ~10 min to
~2-3 min on cache-hit runs. No coverage loss in CI — the heavy tests
still run, just in their dedicated workflows where parallelism +
caching is already optimised.
cristim
added
triaged
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds layered caching to the pre-commit GitHub Actions workflow (npm, Go tools, pre-commit envs, Go build cache) and restricts heavy local hooks ( ChangesPre-commit and workflow performance optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/pre-commit.yml (1)
71-81: ⚡ Quick winMove cache step before tool installs to enable skipping on cache hit.
The cache step at line 154 runs after
Install gosec(line 71) andInstall gocyclo(line 77), so cache restoration happens too late to skip those installs. Move the cache step before these installs and add conditional execution (if: steps.cache-go-tools.outputs.cache-hit != 'true') to the install steps to reduce install time on cache hits.Proposed adjustment
+ - name: Cache Go-installed tools (gosec, gocyclo) + id: cache-go-tools + uses: actions/cache@v4 + with: + path: ~/go/bin + key: go-tools-${{ runner.os }}-gosec-v2.22.4-gocyclo-v0.6.0 + - name: Install gosec + if: steps.cache-go-tools.outputs.cache-hit != 'true' # Pinned to the same version ci.yml's `securego/gosec` Action uses, # so an upstream gosec release with rule changes can't silently # downgrade the gate between the two workflows. run: go install github.com/securego/gosec/v2/cmd/gosec@v2.22.4 - name: Install gocyclo + if: steps.cache-go-tools.outputs.cache-hit != 'true' # Pinned to match ci.yml — security tool installs must not use # `@latest`; that's exactly the supply-chain weakness this PR is # closing for Dockerfile FROMs. run: go install github.com/fzipp/gocyclo/cmd/gocyclo@v0.6.0 - - # Cache the installed tool binaries (gosec, gocyclo). Keyed on the - # pinned version strings so a tool-version bump still triggers a - # fresh install. Both binaries land in ~/go/bin which setup-go@v6 - # already adds to PATH. - - name: Cache Go-installed tools (gosec, gocyclo) - uses: actions/cache@v4 - with: - path: ~/go/bin - key: go-tools-${{ runner.os }}-gosec-v2.22.4-gocyclo-v0.6.0🤖 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/pre-commit.yml around lines 71 - 81, Move the cache restore step so it runs before the "Install gosec" and "Install gocyclo" steps and add conditional execution to those install steps using the cache output (if: steps.cache-go-tools.outputs.cache-hit != 'true') so the installs are skipped when the cache is hit; update the workflow so the cache step (referenced as cache-go-tools) appears before the run steps that install gosec and gocyclo and add the if conditional to both "Install gosec" and "Install gocyclo" steps.
🤖 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.
Nitpick comments:
In @.github/workflows/pre-commit.yml:
- Around line 71-81: Move the cache restore step so it runs before the "Install
gosec" and "Install gocyclo" steps and add conditional execution to those
install steps using the cache output (if: steps.cache-go-tools.outputs.cache-hit
!= 'true') so the installs are skipped when the cache is hit; update the
workflow so the cache step (referenced as cache-go-tools) appears before the run
steps that install gosec and gocyclo and add the if conditional to both "Install
gosec" and "Install gocyclo" steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fbcb3039-17b0-4dc8-abeb-fb3d67ccf26d
📒 Files selected for processing (2)
.github/workflows/pre-commit.yml.pre-commit-config.yaml
CR feedback on PR #342: the Go-tools cache step previously sat AFTER `Install gosec` and `Install gocyclo`, so cache restoration happened too late to short-circuit the installs. Reorder the cache step BEFORE the install steps, add an `id: cache-go-tools` to the cache, and gate each install with `if: steps.cache-go-tools.outputs.cache-hit != 'true'`. On a cache hit the install steps now no-op (saving the `go install` build time per run). On a cache miss they run as before.
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pre-commitCI job has grown from ~8 min to ~10 min as the test suites have added tests. Two independent changes here cut the runtime to ~2-3 min on cache-hit runs without losing any coverage in CI..github/workflows/pre-commit.yml: add four caching layers —setup-node@v6cache: 'npm',actions/cachefor~/.cache/pre-commit,~/.cache/go-build, and~/go/bin(gosec / gocyclo binaries)..pre-commit-config.yaml: move the three heavy hooks (go-test,frontend-build,frontend-test) from default stages tostages: [pre-push]. The CI workflow runspre-commit run --all-fileswithout a stage filter, so the default stage filter (pre-commit) skips these. Local devs withpre-commit install --hook-type pre-pushkeep them as the safety net ongit push.Why the heavy hooks are safe to drop from CI pre-commit
All three are already covered by other workflows that PRs / pushes already trigger:
go-test(-short -race ./...)ci.yml→unit-tests(go test -v -race -short -coverprofile) +integration-tests(go test -v -race -tags=integration)frontend-build(npm run build)frontend-build.yml(PRs) +frontend-build-sentinel.yml(every push tomain/feat/**)frontend-test(npx jest --no-coverage --silent)frontend-build-sentinel.yml(every push tofeat/**, which fires on every PR-branch update)So the only gap is local dev — and
stages: [pre-push]plugs that for devs who installed the push hook.Expected impact
Per the last successful run on PR #337 (commit
186204bd1):Install frontend deps(npm ci, no cache): 65sRun pre-commit: 485sAfter this PR (cache-hit run, no heavy test hooks):
Install frontend deps(cached npm): ~25-30sRun pre-commit: ~60-90s (gofmt, go-mod-tidy, go-vet, terraform-fmt, terraform-tflint, generic checks, hadolint, markdownlint, gocyclo, git-secrets, gosec, trivy-config, migration-conflicts)Total: ~3 min vs the current ~10 min.
Test plan
pre-commit validate-configwould catch issues; configs are passed by structure).Notes / tradeoffs
gosec-v2.22.4-gocyclo-v0.6.0); bump those strings to invalidate.-racefrom the pre-pushgo-testhook — devs running locally still want race coverage on their final pre-push check.terraform_validate(already SKIP'd in CI for the lockfile-creation reason documented in the workflow).Summary by CodeRabbit
Chores
Documentation