Sign up
View stargazersSign up

fix(scripts): check:changed lints Astro components and runs knip for them#29573

merged1
main
GitHub fork: fix/check-changed-astro
opened2d agobySsaphid
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

When a branch changes only .astro files, such as a marketing page or component, vp run check:changed skips lint and prints knip skipped: no code or manifest changed, then reports a pass. CI checks these files: vp check lints Astro frontmatter, where t3code/* error rules fire, and knip:check follows Astro imports. So the local pass is false, and the failure first shows up after the push.

Change

check:changed decides what counts as code with one pattern, CODE. That pattern now also matches .astro, so those files go to the lint step and trigger knip, the same as a .ts change. This is the same class of fix as #28296 and #22784.

Scope and approval

Very small, focused fix for an obvious bug, so no prior issue: the local runner reported success on checks it skipped, while CI runs them. It is one pattern plus one regression test. Overlap search: the closest open PRs touching this area are #28820 and #28753 (ci-changes.cjs package gating) and #24049 (a new pre-merge script). None of them changes which files check:changed lints or when it runs knip.

Verification

  • Red on main: the new launcher-fixture test failed with expected undefined to equal [ 'lint', 'packages/example/Page.astro' ].
  • Green: vp test run scripts/check-changed.test.ts passes 15/15.
  • Runtime check before the fix: I added // TODO later to the frontmatter of apps/marketing/src/components/RetroBox.astro. vp lint reports error t3code(no-low-value-comments), but check:changed ran only format and typecheck, skipped knip, and passed. Separately, I removed the only import of src/lib/tweets.ts from index.astro. knip --include files,dependencies then reports it as an unused file, but check:changed skipped knip.
  • Runtime check after the fix: the same RetroBox edit makes check:changed report FAIL lint with that error, and both knip passes run.
  • CI=true vp run check:changed passes: format, lint, knip 1/2 and 2/2, typecheck, test.
  • Not checked: .vue/.svelte files (none in the repo).

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

Checks

25 successful · 12 skipped
All checks passed
9284b2b

Added 1 commit

  • fix(scripts): check:changed lints Astro components and runs knip for them9284b2b
MmacroscopeappBot2d ago
Synced from GitHub

Approvability

Verdict: Approved at 9284b2b

Macroscope's review found this PR approvable — Astro changes now receive lint and trigger knip in check:changed, aligning local validation with CI. The change is confined to developer checks and does not alter product behavior or suppress diagnostics.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot2d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between cbd3e149a130f1d99d2d067c77b9889d64e28755 and 9284b2b1885e38ed7740bd343cceebfac0b38e37.

External image Files selected for processing: 2
  • scripts/check-changed.test.ts
  • scripts/check-changed.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

The CODE matcher now includes .astro files. A launcher integration test checks that a changed Astro component is linted, knip runs, and the command exits successfully.

Changes: Astro changed-file checks

Layer / File(s)Summary
Match and verify Astro files
scripts/check-changed.ts, scripts/check-changed.test.ts		The CODE matcher now includes .astro files. The integration test changes packages/example/Page.astro and checks successful exit, linting of the file, and execution of knip.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, only21mil

Merge Risk: External image Minimal · up to 9284b

The matcher now includes Astro components and the launcher test covers lint and knip execution. No merge-blocking risk is identified.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly identifies the main change: check:changed now lints Astro components and runs knip for them. It is concise and uses a conventional commit format.
Description checkPassedThe description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the bug, implementation, approval rationale, regression test, runtime checks, passing…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the bug, implementation, approval rationale, regression test, runtime checks, passing results, and unverified file types.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Mmaria1d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 9284b2b1885e38ed7740bd343cceebfac0b38e37 on a disposable linux vm.

  • reproduced on github/main at 60aec28a9c6f: adding // TODO later to RetroBox.astro made direct lint fail, but check:changed passed and skipped lint and knip.
  • on the exact pr head, the identical edit makes check:changed fail lint with t3code(no-low-value-comments), and both knip passes execute.
  • a valid astro edit passes lint, both knip passes, and astro typechecking. moving the existing tweets data into index.astro removes its sole import, and knip reports apps/marketing/src/lib/tweets.ts as unused. all temporary edits were restored.
  • started main and pr dev servers sequentially with isolated state. used playwright from repo dependencies and /usr/bin/google-chrome, added real repository projects through the ui, selected codex gpt-6.1-sol, and completed real turns on both revisions. a pr follow-up turn ran the check through blacksmith and returned the expected lint failure and two passing knip passes to the client.
  • final CI=true vp run check:changed HEAD^ --timeout 300 passed through blacksmith-check: format, lint, both knip passes, scripts typecheck, and all 15 existing tests. the worktree is clean. no fix or push was needed.

command transcript

real client/provider result, cropped to the chat.

codex runs the astro check and returns the lint failure and both knip results

Not verified

native desktop/mobile clients, macos/windows command execution, and other providers were not exercised on this linux vm. this pr changes developer check selection rather than a shipped client interaction. no visual styling changes require matching before/after screenshots.