Repository navigation
fix(create-bestax): scaffold polish — ship .gitignore (with *.tsbuildinfo), PM-aware next steps, strictPort recovery guidance - #506
Conversation
…gnore *.tsbuildinfo npm strips literal .gitignore files from published tarballs, so the templates' tracked .gitignore never reached published scaffolds — apps created from the registry got no .gitignore at all. Track the file as _gitignore instead and have copyDirectory rename it back on copy. Both ignore files also gain *.tsbuildinfo so an incremental tsc run can never commit its build info, and the vite-ts template redirects tsBuildInfoFile into node_modules/.tmp (mirroring tsconfig.node.json) as belt and braces. The e2e scaffold helper now hard-fails if a real scaffold lacks .gitignore. Refs #371
The success screen hardcoded pnpm commands even when the CLI was run via npm, yarn, or bun. Detect the invoking package manager from npm_config_user_agent (falling back to npm) and print `<pm> install` / `<pm> run dev`, a shape valid for all four supported PMs. Refs #371
…nerated CLAUDE.md When 5173 is busy, --strictPort makes the dev server fail loudly by design — but the generated CLAUDE.md never said what to do next, and an agent's natural workaround (moving the app to another port) points the browser-preview manifest at nothing. Tell agents the usual cause is an orphaned dev server from an earlier session and give the exact recovery (lsof -ti:5173 | xargs kill, then relaunch). LAUNCH_JSON itself is unchanged. Closes #371
…uide Mirror the recovery guidance the scaffolder now writes into generated CLAUDE.md files: a busy 5173 usually means an orphaned dev server, and the fix is killing it, not moving the app to another port.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe scaffold detects the invoking package manager, converts ChangesScaffold polish
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR improves generated project files, package-manager-specific next steps, and strict-port recovery guidance without a supplied actionable merge-blocking risk; it is merge-ready after normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Polishes create-bestax scaffolds with reliable ignore files, package-manager-aware next steps, and port-collision guidance.
Changes:
- Restores
.gitignoreduring template copying and relocates TypeScript build metadata. - Detects npm, pnpm, Yarn, or Bun for displayed commands.
- Documents strict-port recovery and expands automated coverage.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docs/docs/guides/getting-started/ai-development.md |
Documents port-collision recovery. |
create-bestax/templates/vite/_gitignore |
Ignores TypeScript build metadata. |
create-bestax/templates/vite-ts/_gitignore |
Ignores TypeScript build metadata. |
create-bestax/templates/vite-ts/tsconfig.json |
Relocates build metadata under node_modules. |
create-bestax/src/project-creator.ts |
Configures template-file renaming. |
create-bestax/src/package-manager.ts |
Detects the invoking package manager. |
create-bestax/src/file-system.ts |
Adds post-copy renaming support. |
create-bestax/src/display.ts |
Prints package-manager-specific commands. |
create-bestax/src/constants.ts |
Adds strict-port recovery guidance. |
create-bestax/src/__tests__/project-creator.test.ts |
Verifies rename configuration. |
create-bestax/src/__tests__/package-manager.test.ts |
Tests package-manager detection. |
create-bestax/src/__tests__/file-system.test.ts |
Tests file renaming behavior. |
create-bestax/src/__tests__/display.test.ts |
Tests generated next steps. |
create-bestax/src/__tests__/constants.test.ts |
Tests collision guidance. |
create-bestax/e2e/utils/scaffold.ts |
Requires scaffolded .gitignore. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| If 5173 is already taken, the usual cause is an orphaned dev server from an earlier session; | ||
| the generated CLAUDE.md tells agents to kill it (`lsof -ti:5173 | xargs kill`) rather than | ||
| move ports. See the [LLMs guide](/docs/guides/llms) for the full AI tooling story. |
| the command. \`--strictPort\` failing because 5173 is busy means an orphaned dev server from an | ||
| earlier session owns the port — kill it (\`lsof -ti:5173 | xargs kill\`) and relaunch; don't | ||
| move the app to another port. |
There was a problem hiding this comment.
Fixed in 5fe5397 — the generated guidance now uses lsof -tiTCP:5173 -sTCP:LISTEN | xargs kill, which selects only the listening process, so connected clients (e.g. the preview browser) can no longer be caught. On confirming orphan-hood first: the listener on the app's own strict port is the dev server in practice, and the guidance stays deliberately terse; an agent that wants to inspect can run the same lsof invocation without -t.
Preview DeploymentPreview URL: https://3c3f71e0.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
create-bestax/src/__tests__/constants.test.ts (1)
245-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the assertion with the safer recovery command.
The test currently requires
lsof -ti:5173 | xargs kill. After the production guidance is narrowed to the TCP listener, assert the listener-only command and keep the orphaned-server wording check.This follows the cross-layer contract between
CLAUDE_MDand the generated scaffold.🤖 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 `@create-bestax/src/__tests__/constants.test.ts` around lines 245 - 253, Update the test for CLAUDE_MD in the strictPort collision case to assert the safer TCP-listener-only recovery command instead of the current broad lsof command, while retaining the --strictPort and orphaned dev server assertions.
🤖 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.
Inline comments:
In `@create-bestax/src/constants.ts`:
- Around line 222-224: The strict-port recovery guidance must identify only the
TCP listener orphaned dev server before terminating it. Update
create-bestax/src/constants.ts lines 222-224 to use listener-only discovery,
verify the PID, and kill only that process; update
create-bestax/src/__tests__/constants.test.ts lines 245-253 to assert the safe
command; and mirror the same guidance in
docs/docs/guides/getting-started/ai-development.md lines 208-210.
---
Nitpick comments:
In `@create-bestax/src/__tests__/constants.test.ts`:
- Around line 245-253: Update the test for CLAUDE_MD in the strictPort collision
case to assert the safer TCP-listener-only recovery command instead of the
current broad lsof command, while retaining the --strictPort and orphaned dev
server assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2058f30e-b5ca-4021-b985-a494333306e7
📒 Files selected for processing (15)
create-bestax/e2e/utils/scaffold.tscreate-bestax/src/__tests__/constants.test.tscreate-bestax/src/__tests__/display.test.tscreate-bestax/src/__tests__/file-system.test.tscreate-bestax/src/__tests__/package-manager.test.tscreate-bestax/src/__tests__/project-creator.test.tscreate-bestax/src/constants.tscreate-bestax/src/display.tscreate-bestax/src/file-system.tscreate-bestax/src/package-manager.tscreate-bestax/src/project-creator.tscreate-bestax/templates/vite-ts/_gitignorecreate-bestax/templates/vite-ts/tsconfig.jsoncreate-bestax/templates/vite/_gitignoredocs/docs/guides/getting-started/ai-development.md
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 1 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | The strictPort recovery command shipped to every generated CLAUDE.md is Unix-only (lsof -ti:5173 | xargs kill); an agent following it literally on Windows fails, though the substantive guidance ("kill the orphan, don't move ports") still lands. |
create-bestax/src/constants.ts:223 |
Overall: The change is sound and cleanly scoped to three small, independent polish items in create-bestax. The riskiest piece — the _gitignore rename-on-copy — is the standard CRA/Vite workaround for npm stripping literal .gitignore from tarballs, and it is implemented correctly: copyDirectory's new renames param is optional and guarded, TEMPLATE_RENAMES is only threaded through copyTemplate, fs.move uses overwrite: true, and _gitignore (not stripped) ships via the package's files: ["templates"]. PM detection and <pm> run dev are valid across all four supported managers, and the guidance changes are docs-only. Nothing urgent for the human here — 222 tests pass and the logic held up under inspection.
Residual risk: the "scaffold ships without .gitignore" failure class could still recur only if —
- a new dotfile template (e.g.
.npmrc,.env.example) gets added later: npm would strip it too and there is no automated check that a.-prefixed template file has a matchingTEMPLATE_RENAMESentry. Refuted for now — the only dotfile in either template dir is_gitignore, and the e2escaffold.tsguard hard-fails if.gitignoreregresses. Worth keeping in mind if more dotfiles get templated. - the copy path silently no-ops: refuted — the rename is
existsSync-guarded and unit-tested for present/absent/omitted cases, and_gitignoreis tracked in git and lands viafiles: ["templates"]. *.tsbuildinfoleaks anyway: refuted — the real buildinfo comes fromtsconfig.node.json(composite: true), already redirected undernode_modules/.tmp, and both ignore files now list*.tsbuildinfo. The addedtsBuildInfoFileon the maintsconfig.jsonis a harmless no-op today (noincremental/composite+noEmit) — defensible belt-and-braces, not a defect.
🏄 Mellow little clean-up set, dude — patched the missing
.gitignoreleak, made the next-steps mirror whatever PM paddled in, and left a note to stop bailing to a new port when 5173's clogged. No gnarly wipeouts in the lineup, tests all glassy green. Ship it. 🌊
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
create-bestax/src/constants.ts:223
lsof -ti:5173matches every TCP/UDP socket involving this port, not just the listener, so an existing browser connection can add the browser PID toxargs kill. A collision also does not prove that the listener is orphaned. Have the agent inspect the listener first and, once confirmed, target onlylsof -tiTCP:5173 -sTCP:LISTEN; keep the mirrored docs and test expectation in sync.
the command. \`--strictPort\` failing because 5173 is busy means an orphaned dev server from an
earlier session owns the port — kill it (\`lsof -ti:5173 | xargs kill\`) and relaunch; don't
docs/docs/guides/getting-started/ai-development.md:209
- This recovery command is broader than the text claims:
lsof -ti:5173can return client and UDP processes in addition to the listening server, and piping those PIDs tokillmay terminate unrelated applications. Document inspection of the owner first and restrict the eventual lookup tolsof -tiTCP:5173 -sTCP:LISTEN, synchronized with the generated CLAUDE.md guidance.
If 5173 is already taken, the usual cause is an orphaned dev server from an earlier session;
the generated CLAUDE.md tells agents to kill it (`lsof -ti:5173 | xargs kill`) rather than
…tener A plain lsof -ti:5173 matches every socket touching the port — clients with 5173 as their remote endpoint included — so an agent following the generated guidance could kill the preview browser along with the orphan. lsof -tiTCP:5173 -sTCP:LISTEN selects only the listening dev server. Refs #371
Keep the AI development guide in sync with the generated CLAUDE.md: the recovery kill targets only the TCP listener on 5173, not every process with a socket touching the port.
|
Review follow-up, all threads addressed:
|
Preview DeploymentPreview URL: https://42cb5d2b.bestax.pages.dev |
|
🎉 This PR is included in version 5.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Three scaffold-polish items from the 10-run skill-loop eval (#363), all in create-bestax.
1. Ship a scaffold
.gitignore(via rename-on-copy) and ignore*.tsbuildinfonpm strips literal
.gitignorefiles from published tarballs, so although both templates tracked one, apps scaffolded from the registry shipped with no.gitignoreat all. The templates now track_gitignoreandcopyDirectoryrenames it back on copy (fs.movewithoverwrite: true, keyed by aTEMPLATE_RENAMESmap).Both ignore files also gain
*.tsbuildinfo, and the vite-ts template redirectstsBuildInfoFileintonode_modules/.tmp(mirroring whattsconfig.node.jsonalready did) as belt and braces.Eval evidence: run i09 burned ~7 diagnosis turns on a stale-buildinfo TS5083 after a
pnpm add, and its artifact shipped with the buildinfo committed — both impossible once the buildinfo lives undernode_modulesand is ignored anyway.2. Print next steps for the invoking package manager
display.tshardcodedpnpm install/pnpm deveven though the scaffold is deliberately PM-agnostic (.claude/launch.jsonusesnpm run devfor exactly that reason). The success screen now detects the invoking PM fromnpm_config_user_agent(falling back to npm) and prints<pm> install/<pm> run dev— a shape valid for all four supported PMs. The template README's pnpm commands are deliberately untouched (out of scope for this pass).3. strictPort port-collision recovery guidance
--strictPortstays — a busy 5173 must fail loudly, not point the browser preview at nothing. What was missing is what to do next: eval run i04 hit exactly this collision when an orphaned dev server from an earlier session owned the port. The generated CLAUDE.md now names that cause and the recovery — kill the orphan (lsof -ti:5173 | xargs kill) and relaunch, never move the app to another port.LAUNCH_JSONis byte-identical; the AI development docs guide gets the same note.Testing
pnpm --filter create-bestax test: 8 suites, 222 tests green; coverage 98.36% statements / 89.16% branches / 100% functions / 98.30% lines (thresholds 95/78/95/95).npm pack --dry-run:templates/vite/_gitignoreandtemplates/vite-ts/_gitignoreboth appear in the tarball file list.--ignore-snapshots— 10/10 pass (the visual baselines are Linux-only by design and update via the CI workflow). The e2e scaffold helper now hard-fails if a scaffold lacks.gitignore, alongside the existing launch.json guard..gitignorewith*.tsbuildinfo(and no_gitignoreresidue);npm_config_user_agent="pnpm/…"flips next steps to pnpm, unset falls back to npm.pnpm all: green.Closes #371
Summary by CodeRabbit
New Features
.gitignorefile.Bug Fixes
Documentation