ci: kill coverage Docker pull on PR runs + tighten claude-review group - #129
Merged
Conversation
…ten claude-review concurrency Three bundled changes against the per-PR-push overhead observed in the WaveHouse CI graph: 1. Split `Publish coverage badge` into its own main-only job. GitHub pre-pulls Docker-typed actions for every job they appear in regardless of step-level `if:` gating, so the previous inline step was taxing PR runs with ~14-20s of `ghcr.io/vladopajic/ go-test-coverage:v2.18.8` pull time on every push even though the publish step itself was skipped. Moving the action into a job that's `if:`-gated at the JOB level (so the job doesn't materialize on PRs at all) eliminates the pre-pull entirely on the PR feedback path. On main, the publish-badge job runs in parallel-ish with the CI job's tail (needs: ci), so the visible main-push end-to-end isn't worse either. Coverage profile + `.testcoverage.yml` are passed between jobs via `actions/upload-artifact` → `download-artifact` so the action sees the same inputs the inline version did. 2. Restrict `claude-review` `cancel-in-progress` to `pull_request` events only. Push to the PR's HEAD ref still cancels the in-flight (current) Claude review because the new HEAD makes the running review immediately stale; comment-triggered re-runs (`@claude` / `/review`) and `workflow_dispatch` retries now queue instead. Solves the lingering "comment cancels Claude mid-run, requiring a manual re-trigger" case the prior sender-type gate (#111) missed when the GitHub App acts via PAT. 3. Bump `actions/cache/restore` to v5.0.5 (matches `cache/save` already in ci.yml) and `pnpm/action-setup` to v6.0.8 — both fix the Node 20 deprecation warning that fires on every CI run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR upgrades GitHub Actions infrastructure by introducing dynamic pnpm store path resolution in the setup action, updating cache restore versions to v5.0.5, refining CI workflow cache-save conditions from main-only to cache-hit-based gates, adjusting review workflow concurrency for pull request events, and improving documentation across workflows, configuration, and contributor guides. ChangesCI/CD Infrastructure and Documentation Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces several optimizations to the CI pipeline to reduce execution time and improve the developer experience. By offloading non-essential tasks and refining the concurrency logic for automated reviews, the changes aim to streamline the feedback loop and resolve ongoing infrastructure warnings. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
github_actions
EricAndrechek
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the CI environment setup by switching to a custom actions results URL for caching and bumping several GitHub Action versions, including actions/cache/restore to v5.0.5 and pnpm/action-setup to v6.0.8. Feedback indicates that these notable changes must be documented in CHANGELOG.md under the [Unreleased] section to comply with the repository's documentation sync requirements.
The previous `github.ref == 'refs/heads/main'` gate was protecting against a ~3-min post-step the unified actions/cache@v4 was running to upload 1.7 GB to GitHub's WAN-hosted cache on every PR push. With cache traffic now landing on caches.dmz.local:3000 at LAN speeds, the break-even shifts: a one-time ~30-60s tar+upload on the first push per (go.sum + lockfile) tuple buys ~220s of saved `make ci` time on every subsequent push to that branch (or any other PR with the same key) — and the cache-hit gate already short-circuits the no-op re-upload case, so a same-key re-push is essentially free. Empirical motivator: re-runs of PR #129 against the empty post-v9 cache server saw setup-env at 19s (LAN restore working) but make ci at 269s (fully cold local build, no cache to hit). Each PR push was silently paying the cold-build cost AGAIN despite the cache server being ready to serve. This change makes the first PR push to a fresh branch slightly slower than before — visible end-of-job ~30-60s. Every push after that lands warm. Net win across a typical PR lifetime is large. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three CI tweaks on top of the cache-server rollout: - pnpm store cache was silently failing. The hard-coded path `~/.local/share/pnpm/store` is pnpm's documented default but it only applies when $HOME and $GITHUB_WORKSPACE are on the same mount. ARC pods split those, so pnpm uses `/home/runner/_work/.pnpm-store/v10/` instead, and every save step logged `Path Validation Error: Path(s) ... do(es) not exist` and skipped the upload. Result: pnpm reinstalled cold every run. Fix: install pnpm before the pnpm cache restore, resolve the actual store dir via `pnpm store path --silent`, expose it as a composite-action output so ci.yml's save step uses the same value. - Bump `actions/upload-artifact` v4.6.2 → v7.0.1 and `actions/download-artifact` v6.0.0 → v8.0.1 to clear the Node 20 deprecation warnings runners now emit. Audited the rest — every other pinned action is already on node24. - Coverage badge moves out of ci.yml into its own workflow file (publish-coverage-badge.yml) triggered via `workflow_run` after CI completes on main. PR runs no longer instantiate a publish-badge job at all (no "Skipped" entry, no Docker image pre-pull on PR pods), at the cost of ~5-15s of dispatch latency on the main-side badge update. - `pnpm store prune` runs before save so cache size stays bounded as transitive deps churn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3c2c0ea fixed the pnpm cache path (was hitting `Path Validation Error: Path(s) ... do(es) not exist` at save time and silently skipping the upload). That run produced the first real pnpm cache under the correct `/home/runner/_work/.pnpm-store/v10/` path. This empty commit triggers a fresh CI run so we can see the exact-key restore hit and the save step short-circuit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced May 13, 2026
The README coverage badge pointed at a `raw.githubusercontent.com` URL on the `badges` branch — which 404s for anonymous viewers on private repos, so the badge has been a broken image for everyone outside the team since the repo was made private. Rather than work around that with GitHub Pages on a paid plan or a public-Gist hack, just remove the apparatus entirely. Coverage is still computed and gated by `make ci` and surfaced in each run's job-summary panel for anyone who needs the actual number. - Drop the coverage badge line from README. - Delete `.github/workflows/publish-coverage-badge.yml` (and its upload-artifact handoff in ci.yml). The `vladopajic/go-test-coverage` Docker action goes with it. - Strip stale badge-related comments from `.testcoverage.yml` and ci.yml; point both at issue #133 for the post-OSS-launch decision. The `badges` branch on origin is now orphaned — deleting in a follow-up step. `.testcoverage.yml` stays load-bearing for `scripts/cov`'s threshold gates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
documentation
Pre-OSS-launch cleanup of the comments in the workflow files this PR touches. Implementation is unchanged; only comment text is rewritten. - setup-env composite preamble: drop the on-prem cache server, runner-image, and 1.7 GB blob references. Keep the technical rationale for each cache, the setup-go issue link, and the pnpm-store dynamic-path explanation (in generic container-runner terms rather than naming a specific runner pool). - ci.yml: drop the historical six-jobs-to-one collapse reasoning, pod-budget specifics, internal cache-server hostnames, LAN RTT, and `~270 s` cold-build figures. Replace with a short statement of the actual behavior. - ci.yml: fix the coverage-summary comment's issue ref (#132 → #133, the post-OSS-launch coverage decision). - claude-review.yml: tighten the concurrency-rationale block — drop the historical PR ref and the OAuth-token explanation paragraph; keep the three-bullet "what queues, what cancels" summary. Net change: 16 fewer lines on the branch's diff vs main despite all the rewrites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second-pass cleanup before going public. Implementation logic is byte-identical across every file; only comments and AGENTS.md prose changed. Net: ~310 lines lighter. - Tighter comments in the three files this PR already touched (ci.yml, setup-env composite, claude-review.yml). Drop the preamble's verbose "what is this workflow" paragraphs in favor of one-liners; collapse multi-paragraph rationale blocks. - Scrub + tighten comments in the rest of the workflow set (admin-approval, dependabot-automerge, housekeeping, triage, cleanup-ghcr, release, publish-dev, goreleaser-validate). Same rules: drop internal-infra references, drop historical-migration asides, drop specific PR/issue back-references, keep security-relevant and non-obvious-behavior comments. - AGENTS.md: rewrite the Local-First Validation section to drop specific runner-side ops detail; rewrite the Review Response and Repository Automation sections to drop project-board internal numbers and the "things we used to automate" migration narrative; collapse the Documentation Sync section's three overlapping tables into one cross-reference table. Same PR description rewrite already pushed via `gh pr edit`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous scrub commit (43c6e32) hard-wrapped the rewritten prose at ~72 chars, breaking the rest-of-file convention of one paragraph or bullet per source line. Match the surrounding style: rejoin each paragraph/bullet into a single line. No content change — every line is the same character sequence with intra-paragraph newlines replaced by single spaces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolved conflicts in admin-approval.yml, housekeeping.yml, and AGENTS.md between main's #134 patch/minor-only Dependabot bypass and this branch's comment-tightening pass. Took main's new logic (UPDATE_TYPE env var, scoped bypass, Dependabot title-length exemption) and re-tightened the accompanying comments / AGENTS.md prose to match this branch's style. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from In progress
to Ready
in WaveHouse Task Board
This was referenced May 18, 2026
EricAndrechek
added a commit
that referenced
this pull request
Jun 5, 2026
Round-2 reviewer findings: "nothing else uses a browser" overclaimed — the manual scripts/screenshot.mjs QA helper launches the same Playwright Chromium (scoped the claim to the docs *build* in development.md and the install-playwright-docs Makefile comment), and development.md still quoted AGENTS.md §"Review Response (MANDATORY)", renamed to §"Review Response" in #129. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 9, 2026
This was referenced Aug 10, 2026
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
2 participants
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
CI speedup + supply-chain hygiene + a real bug fix that was hiding in plain sight. Drops typical PR-run wall time from ~5 min to ~2 min on warm cache.
Fixes
~/.local/share/pnpm/store(pnpm's documented default), but pnpm only uses that path when$HOMEand the project tree share a mount. On the CI runner pods they don't, so pnpm falls back to a workspace-relative path. Every save loggedPath Validation Error: Path(s) ... do(es) not existand skipped the upload — pnpm reinstalled cold every run. Fix: install pnpm first, ask it for the actual store path viapnpm store path --silent, expose that through the composite action's outputs so ci.yml's save uses the same value. Validated end-to-end on this branch: first run after the fix uploaded the pnpm cache for real (19s); subsequent run hit the exact key and skipped the save.Cleanups
badgesbranch + Docker publish-action plumbing. The badge URLraw.githubusercontent.comrequired auth on a private repo, so the README image rendered broken for anonymous viewers. Re-evaluate post-OSS-launch (Codecov vs. resurrect existing) — tracked in tracking: re-evaluate coverage reporting / badge after OSS launch #133. Coverage data still lands in the run's job summary panel for the team.actions/upload-artifactv4 → v7 andactions/download-artifactv6 → v8 to clear Node 20 deprecation warnings.pnpm store prunebefore save so the cache stays bounded as transitive deps churn over months.claude-reviewcancel-in-progress topull_requestevents. Comment-triggered reruns (@claude//review) andworkflow_dispatchretries now queue behind in-flight reviews instead of cancelling them. Strictly more semantic than the previoussender.type != 'Bot'filter and not fooled by PAT-acting-as-bot edges.Test plan
3c2c0eashows pnpm save uploaded underpnpm-Linux-…(~19s)a9e34eeshowsCache hit for: pnpm-Linux-…andSave pnpm store cache: skippedFiled for follow-up
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Tests