chore: expand pnpm check to mirror full CI merge gate#2065
chore: expand pnpm check to mirror full CI merge gate#2065nick-inkeep wants to merge 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
pnpm check previously only ran the turbo subset (lint + typecheck + test). CI also ran format:check, check:env-descriptions, and knip as separate steps. Now pnpm check chains all four, so "green locally = green in CI." Also fixes contributing docs: pre-push hook description was wrong (claimed check:prepush, actually runs pnpm format) and removes the stale caveat about CI running extra checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
01f509f to
c485b98
Compare
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
AGENTS.md:269Inconsistent terminology — "format" vs "format:check"
💡 APPROVE WITH SUGGESTIONS
Summary: This is a clean, well-scoped PR that achieves its stated goal of local-CI parity. The documentation fixes are accurate (verified the pre-push hook does run pnpm format), and the script expansion correctly chains all CI steps. The only minor issue is a terminology inconsistency within AGENTS.md — one location says "format" while another says "format:check". A quick fix to use "format:check" consistently would improve clarity. Nice work! 🎉
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/workflows/ci.yml |
CI runs steps separately while local script chains them | Intentional design per PR description — CI gets granular failure attribution, local gets fail-fast. Not an issue. |
.github/workflows/ci.yml:21-34 |
Third-party GitHub Actions use mutable tags instead of SHA pins | Pre-existing issue not introduced by this PR. Out of scope. |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-devops |
2 | 0 | 0 | 0 | 0 | 0 | 2 |
pr-review-consistency |
1 | 0 | 0 | 0 | 1 | 0 | 0 |
| Total | 3 | 0 | 0 | 0 | 1 | 0 | 2 |
Addresses review feedback — line 269 said "format" but the actual script runs format:check (verification, not auto-fix). Now matches line 13 which correctly uses "format:check". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Re-Review
This is a re-review scoped to changes since the previous automated review.
Delta: 1 commit (e8c708a9) · 1 file (AGENTS.md) · 1 line changed
Previous feedback status:
- 🟡
Inconsistent "format" vs "format:check" terminology→ ✅ Resolved — Line 269 now usesformat:check, matching line 13
No new issues identified in the delta. The fix is correct and minimal.
✅ APPROVE
Summary: The previous review's feedback has been addressed with a clean 1-line fix. Both references to the check script now consistently use format:check terminology. Ship it! 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| N/A — trivial delta, no sub-reviewers dispatched | — | — | — | — | — | — | — |
Note: Delta consisted of a single editorial fix addressing prior feedback. Sub-reviewer dispatch was skipped per orchestrator judgment.
|
CI Status Note: The failing All other checks pass:
|
Summary
pnpm checkto chainturbo check+format:check+check:env-descriptions+knip— matching exactly what CI runspnpm checknow mirrors the full CI merge gateMotivation
pnpm checkwas incomplete — it only ran the turbo subset (lint + typecheck + test), while CI also ranformat:check,check:env-descriptions, andknipas separate steps. The CI failure summary misleadingly told developers "Runpnpm checklocally to reproduce the CI checks." The extra steps add <8s to a ~2min run (measured locally), so there's no reason for them to be separate.Part of the Local CI Parity spec. Playwright browser install deferred to after PR #2054 merges (introduces
setup-dev.js).Changes
package.json— expandcheckscript to chain all CI stepsAGENTS.md— addpnpm checkto Essential Commands, simplify verification commands sectionagents-docs/.../contributing/overview.mdx— remove CI extra-checks caveat, fix pre-push hook description (was wrong: claimedcheck:prepushbut actually runspnpm format)What's NOT affected
check:huskyandcheck:prepush— these reference turbo tasks directly inturbo.json, not the rootcheckscriptTest plan
pnpm checkscript correctly chains:turbo check && format:check && check:env-descriptions && knipcheck:huskyandcheck:prepushscripts unchanged (verified vianode -e)format:check,check:env-descriptions,knip).husky/pre-push) confirmed to runpnpm format(matches updated docs)🤖 Generated with Claude Code