ci: prepare for trusted publishing #6032
Conversation
WalkthroughThe changes consolidate and upgrade CI/CD workflows with action version bumps, add explicit permissions declarations, refactor npm scripts from multiple prettier variants to a unified format command, convert a config generator script to TypeScript with proper type annotations, and update TypeScript compilation settings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12–15 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 1907632
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/generate-labeler-config.ts (1)
99-106: Fix async error handling aroundrun()Wrapping
run().then(...)in atry/catchwon't catch errors thrown asynchronously insiderun(); they become unhandled promise rejections instead of going through your error logging path.Handle errors reliably by attaching a
.catch():-try { - run().then(() => { - process.exit(0) - }) -} catch (error) { - console.error('Error generating labeler config:', error) - process.exit(1) -} +run() + .then(() => { + process.exit(0) + }) + .catch((error) => { + console.error('Error generating labeler config:', error) + process.exit(1) + })
🧹 Nitpick comments (2)
.github/workflows/pr.yml (1)
1-1: PR workflow permissions and action bumps look reasonable—double‑check necessity ofpull-requests: writeRenaming the workflow, upgrading to
actions/checkout@v6.0.1andnrwl/nx-set-shas@v4.4.0, and addingpull-requests: writeare all consistent with the rest of the CI hardening. If you don’t currently post PR comments or otherwise mutate PRs in this workflow, you could consider reducing that permission later for least privilege, but it’s fine if you plan to use it.Also applies to: 14-17, 26-26, 34-35, 47-47
.github/workflows/release.yml (1)
1-1: Release workflow triggers/permissions look aligned with trusted publishing—confirmpull-requests: writeusageRenaming to “Release”, adding
workflow_dispatchwith an optionaltag, and upgrading toactions/checkout@v6.0.1make sense for a controlled release flow. The addedpull-requests: writepermission is fine if this job interacts with PRs (e.g. changelog or release PR updates); otherwise you could consider tightening it later.Also applies to: 20-24, 27-27, 32-32
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/autofix.yml(1 hunks).github/workflows/labeler.yml(1 hunks).github/workflows/pr.yml(4 hunks).github/workflows/release.yml(2 hunks)AGENTS.md(1 hunks)package.json(1 hunks)scripts/generate-labeler-config.ts(3 hunks)tsconfig.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety throughout the codebase
Files:
scripts/generate-labeler-config.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace protocol for internal dependencies (workspace:*)
Files:
package.json
🧠 Learnings (6)
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Always run pnpm test:eslint, pnpm test:types, and pnpm test:unit before committing
Applied to files:
AGENTS.md.github/workflows/release.ymlpackage.json
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Test changes in relevant example apps by running cd examples/[framework]/[example] && pnpm dev
Applied to files:
AGENTS.mdpackage.json
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
AGENTS.md
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to packages/react-router/**/*.{ts,tsx} : React Router components and hooks should use the tanstack/react-router package
Applied to files:
AGENTS.md
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to packages/solid-router/**/*.{ts,tsx} : Solid Router components and primitives should use the tanstack/solid-router package
Applied to files:
AGENTS.md
📚 Learning: 2025-11-25T00:18:21.282Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T00:18:21.282Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript strict mode with extensive type safety throughout the codebase
Applied to files:
tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (5)
tsconfig.json (1)
14-14: ESNext module target and broadened include look appropriateUsing
module: "ESNext"withmoduleResolution: "Bundler"and includingscriptsis consistent with modern tooling and ensures the new TS scripts are checked under strict mode. Just confirm there aren’t any stray root*.config.*files you don’t want pulled into this tsconfig (and runpnpm test:typesto validate).Also applies to: 27-27
scripts/generate-labeler-config.ts (1)
5-12: TypedLabelerPairand function signatures are clear and strict‑friendlyDefining
type LabelerPair = [string, string]and usingArray<LabelerPair>forreadPairsFromFsandgenerateLabelerYamlnicely tightens the contract and keeps everything strict‑mode safe. No issues from a typing/structure perspective here.Also applies to: 37-37
.github/workflows/autofix.yml (1)
22-22: Autofix workflow updates align with new toolingBumping to
actions/checkout@v6.0.1and standardizing onpnpm formatmatches the new root script and keeps this workflow consistent with the rest of the CI. Worth just confirming checkout v6 behaves as expected for your use cases.Also applies to: 26-26
AGENTS.md (1)
51-51: Docs now correctly point topnpm formatThe formatting instruction matches the new root script and CI usage; this keeps contributor guidance aligned with the tooling.
.github/workflows/labeler.yml (1)
1-1: Labeler workflow is wired correctly tolabeler-config.ymlRenaming the workflow, adding explicit
contents: read/pull-requests: write, and pointingactions/labeleratlabeler-config.ymlall line up with the new generator script. The extra job‑level permissions duplicate the global ones but are harmless and can be kept for clarity.Also applies to: 6-8
| "dev": "pnpm run watch", | ||
| "prettier": "prettier --experimental-cli --ignore-unknown '**/*'", | ||
| "prettier:write": "pnpm run prettier --write", | ||
| "format": "prettier --experimental-cli --ignore-unknown '**/*' --write", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
prettier npm script glob pattern Windows shell '**/*' vs .
💡 Result:
Short answer: use prettier --write . (recommended) or quote your glob in package.json so Prettier (not the shell) sees it. On Unix shells */ is expanded by the shell; on Windows (cmd.exe) the shell does not expand globs, so behaviour differs unless you let Prettier handle the pattern. See Prettier docs: quote globs for cross‑platform use and/or use . to format the current directory. [1][2][3]
Examples:
- Recommended simple cross‑platform: "format": "prettier . --write". [1]
- If you want a glob: "format": "prettier "**/*.{js,ts,css,json,md}" --write" (quotes ensure Prettier receives the glob instead of the shell expanding it). [1][2]
Sources:
- Prettier CLI (quotes, '.' vs globs, glob resolution). [1]
- npm scripts / shell differences (npm runs cmd.exe on Windows vs /bin/sh on Unix). [2]
- glob behavior / globbing libraries used by tools. [3]
🌐 Web query:
npm scripts shell expansion glob patterns Windows cross-platform
💡 Result:
Short answer
- Shell globbing (e.g. src/**/*.js) is performed by the shell, not by npm. POSIX shells expand globs before npm sees them; Windows cmd.exe does not, so relying on shell expansion breaks on Windows. [1][6][3]
- Options: (A) let the CLI/tool do globbing (pass quoted pattern) or use a Node glob library; (B) use a cross-platform tool that implements its own globbing (rimraf, fast-glob/glob, etc.); (C) run a POSIX shell on Windows or change npm’s script-shell. [1][5][4][6]
What to do (concise recipes)
- Prefer letting the tool handle the pattern (safe cross-platform):
- Example: "lint": "eslint "src/**/*.ts"" — quote the pattern so eslint (which implements globbing) receives it. [1]
- Use a Node-based glob library or small script when you need custom file lists:
- Example: "lint": "node scripts/lint.js" and in scripts/lint.js use fast-glob or glob to resolve files. [5]
- Use cross-platform CLI packages for OS-mismatched commands:
- rm -> rimraf (supports --glob and runs the glob itself). [4]
- set env vars -> cross-env / cross-env-shell. [2]
- If you must rely on a shell that expands globs, change npm’s shell on Windows:
- Set script-shell in .npmrc or install/run Git Bash / WSL / PowerShell and configure accordingly (npm uses cmd.exe by default on Windows). [6][2]
Notes / pitfalls
- Quoting matters: don't let cmd interpret unquoted globs. Use double quotes in package.json so node tools receive the literal pattern. [1]
- Some useful packages: glob / fast-glob for programmatic expansion; rimraf for safe deletions; cross-env for env vars. [5][4][2]
- For complex cross-platform inline scripts, prefer a small Node script (most robust).
Sources
- Cross-platform npm-scripts and quoting/globbing guidance. [1]
- cross-env README (env handling, cross-env-shell note). [2]
- StackOverflow examples and discussion about shell expansion on Windows. [3]
- rimraf CLI (glob handling, cross-platform delete). [4]
- glob npm package (programmatic globbing). [5]
- npm scripts run-shell behavior / script-shell discussion. [6]
Consider a more portable target for the format script
The Prettier v3 --experimental-cli usage is fine, but shell glob expansion behaves differently across platforms: POSIX shells expand **/* before Prettier sees it, while Windows cmd.exe does not. Using . (current directory) is simpler and explicitly recommended by Prettier for cross-platform compatibility:
- "format": "prettier --experimental-cli --ignore-unknown '**/*' --write",
+ "format": "prettier --experimental-cli --ignore-unknown . --write",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "format": "prettier --experimental-cli --ignore-unknown '**/*' --write", | |
| "format": "prettier --experimental-cli --ignore-unknown . --write", |
🤖 Prompt for AI Agents
In package.json around line 27, the format script uses a shell glob '**/*' which
is expanded differently on POSIX vs Windows; change the script to use the
portable current-directory target '.' so Prettier runs consistently across
platforms (e.g. update the script to use "prettier --experimental-cli
--ignore-unknown . --write"), keeping the existing flags intact.
| "gpt-generate": "node gpt/generate.js", | ||
| "set-ts-version": "node scripts/set-ts-version.js", | ||
| "labeler-generate": "node scripts/generate-labeler-config.mjs", | ||
| "labeler-generate": "node scripts/generate-labeler-config.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n package.json | head -50Repository: TanStack/router
Length of output: 2694
🏁 Script executed:
rg "tsx|ts-node" package.jsonRepository: TanStack/router
Length of output: 41
🏁 Script executed:
cat -n scripts/generate-labeler-config.ts | head -20Repository: TanStack/router
Length of output: 881
🏁 Script executed:
rg "type.*module" package.jsonRepository: TanStack/router
Length of output: 78
🏁 Script executed:
cat package.json | grep -A 100 '"devDependencies"' | grep -E '"(tsx|ts-node|ts-loader|tsx-loader)"'Repository: TanStack/router
Length of output: 41
🏁 Script executed:
rg "tsx|ts-node|@ts" package.jsonRepository: TanStack/router
Length of output: 41
🏁 Script executed:
cat package.json | grep -A 200 '"devDependencies"' | head -120Repository: TanStack/router
Length of output: 3411
🏁 Script executed:
rg "labeler-generate|verify-links" --type json --type yaml --type mdRepository: TanStack/router
Length of output: 199
🏁 Script executed:
git log --oneline scripts/generate-labeler-config.ts | head -5Repository: TanStack/router
Length of output: 103
🏁 Script executed:
ls -la scripts/Repository: TanStack/router
Length of output: 589
labeler-generate and test:docs scripts point Node directly at .ts files, which will fail without a TypeScript loader
With "type": "module" configured and no TS runtime like tsx or ts-node in devDependencies, node scripts/generate-labeler-config.ts and node scripts/verify-links.ts will fail at runtime. Node doesn't understand TypeScript syntax or .ts imports natively. Resolve by either:
- Installing and using a TS loader (e.g.,
tsxorts-node) in the script command, or - Compiling these scripts to JavaScript and pointing to the compiled output.
🤖 Prompt for AI Agents
In package.json around line 31 the "labeler-generate" (and similarly
"test:docs") script invokes Node directly on a .ts file which will fail at
runtime because Node cannot execute TypeScript files without a loader; fix by
either adding a TypeScript runner (e.g., install a devDependency like tsx or
ts-node and update the script to use it, e.g., "tsx
scripts/generate-labeler-config.ts") or compile the script to JS and point the
script to the compiled .js output; ensure the chosen tool is added to
devDependencies and update package.json scripts accordingly.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.