Skip to content

Fix scaffold audit gaps (install, names, writer, doc links)#10

Open
eternalwaitt wants to merge 2 commits into
mainfrom
fix/scaffold-audit-findings
Open

Fix scaffold audit gaps (install, names, writer, doc links)#10
eternalwaitt wants to merge 2 commits into
mainfrom
fix/scaffold-audit-findings

Conversation

@eternalwaitt

@eternalwaitt eternalwaitt commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Implement post-scaffold git init and optional dependency install (bun/npm/pnpm) without failing the scaffold on install errors.
  • Centralize project name sanitization and validation in @veloz-stack/types; align CLI and web stack builder on the same rules.
  • Harden template writeTree with path containment checks and batched concurrent writes.
  • Read CLI version from package.json instead of a hardcoded string.
  • Skip fenced code blocks in docs:links so markdown examples do not false-positive.
  • Add unit tests for project names, post-scaffold helpers, writer safety, and web build command naming.

Test plan

  • pnpm lint
  • pnpm --filter @veloz-stack/types test
  • pnpm --filter create-veloz-stack test
  • pnpm --filter @veloz-stack/template-generator test (after pnpm gen)
  • pnpm docs:links

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Project name normalization and clearer validation errors.
    • CLI shows its real version from package metadata.
  • Bug Fixes

    • Hardened scaffold writes to prevent path traversal.
    • Git initialization during scaffolding is more tolerant and safe.
    • Dependency install step reports and tolerates failures; post-scaffold messages adjusted.
  • Tests

    • Added tests for project-name logic, scaffold writing, git/init and install behaviors.

Review Change Stack

Implement post-scaffold git init and package install, centralize project name sanitization, harden template writes with path checks and batching, and stop doc link checker from scanning fenced code blocks.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fe74346b-d116-4bfa-96b6-14f3e2cb6053

📥 Commits

Reviewing files that changed from the base of the PR and between 3095a14 and f7e456e.

📒 Files selected for processing (3)
  • apps/cli/src/post-scaffold.test.ts
  • apps/cli/src/post-scaffold.ts
  • packages/template-generator/src/writer.ts

📝 Walkthrough

Walkthrough

Centralizes project-name sanitization/validation, extracts post-scaffold git/env/install helpers and tests, loads CLI version from package.json, hardens template writer against path traversal with bounded concurrency, and strips fenced code blocks during markdown link checks.

Changes

Project Name Validation and Centralization

Layer / File(s) Summary
Core project-name validation utilities
packages/types/src/project-name.ts, packages/types/src/project-name.test.ts
New module exports PROJECT_NAME_FALLBACK, ProjectNameSchema (npm-safe lowercase slug), sanitizeProjectName (normalizes arbitrary input), and projectNameValidationError (returns validation error or null). Tests verify sanitization preserves separators, strips invalid characters, handles symbol-only input fallback, and validates against schema.
Types package integration and re-export
packages/types/src/index.ts, packages/types/package.json
ProjectConfig.projectName now uses centralized ProjectNameSchema instead of inline regex. Four project-name utilities re-exported from package public surface. Vitest test script and dependency added to types package.
Web build-command integration
apps/web/lib/build-command.ts, apps/web/lib/build-command.test.ts
buildCommand imports sanitizeProjectName from centralized types package, removing local implementation. Test verifies sanitization matches CLI behavior by checking normalized slug presence and original format absence.
CLI create flow project-name validation
apps/cli/src/create.ts
resolveTargetDirectory now sanitizes and validates config.projectName using centralized utilities, printing red error and returning null on validation failure. runCreate exits with code 1 when target is null. Removed basename import.

Post-Scaffold Operations and CLI Enhancements

Layer / File(s) Summary
Post-scaffold helpers module
apps/cli/src/post-scaffold.ts, apps/cli/src/post-scaffold.test.ts
New module exports initGitRepo (runs git init with yellow warning fallback), ensureEnvFile (copies .env.example if needed), ensureGitForHooks (verifies/creates git for lefthook), installArgs (selects bun/npm/pnpm command), and installDependencies (runs install with stdio inheritance and failure tolerance). Tests verify git init calls with correct cwd/fallback behavior, and install behavior including bun install and failure swallowing.
CLI create flow post-scaffold integration
apps/cli/src/create.ts
Git initialization delegates to initGitRepo instead of inline execSync. Post-scaffold success output: "pm install" hint conditional on !config.install; "pm dev" shown unconditionally.
CLI version loading from package.json
apps/cli/src/index.ts
Entrypoint uses createRequire(import.meta.url) to load ../package.json at runtime, validates version field presence/type, throws on invalid shape, and passes computed CLI_VERSION to Cli.create instead of hardcoded "0.0.1".

Template Writer Security Hardening

Layer / File(s) Summary
Path traversal protection and concurrency control
packages/template-generator/src/writer.ts
writeTree now resolves rootDir once, validates each destination path via new assertPathInsideRoot helper (rejects paths escaping root), and uses fixed WRITE_CONCURRENCY batch size instead of unbounded Promise.all for parallel writes.
Writer test coverage
packages/template-generator/src/writer.test.ts
Tests verify writeTree writes expected files with correct counts and content. Path traversal test confirms rejection of out-of-scaffold writes with "outside scaffold root" error message.

Documentation Link Validation

Layer / File(s) Summary
Fenced code block stripping
scripts/check-doc-links.mjs
New stripFencedCodeBlocks helper removes triple-backtick and triple-tilde fenced regions from markdown before link extraction. Link validation preprocesses files through stripper to exclude code-block links from existence checks. Doc comment updated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • PierreAndreis

🐰 A rabbit's ode to validated names and safer scripts,
Where post-scaffold ops flow and path traversal slips,
From CLI versions fresh to docs links stripped,
Clean configurations hop—no edge cases gripped!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly reflects the main changes: post-scaffold setup (install, git init), centralized project name handling, hardened template writer, and documentation link checker improvements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/scaffold-audit-findings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@apps/cli/src/post-scaffold.ts`:
- Around line 55-78: The installDependencies function currently always runs
installs even though callers (e.g., the call site in create.ts) gate it by
config.install; add an explicit guard at the top of installDependencies that
returns immediately when config.install is false so direct callers (like unit
tests) honor the flag, and update the function docstring to state that
installDependencies will no-op if config.install is false; reference the
installDependencies function and the config.install flag when making the change.

In `@packages/template-generator/src/writer.ts`:
- Around line 25-41: The current batching logic creates all batches then starts
every batch in parallel, so WRITE_CONCURRENCY is not enforced; change the outer
Promise.all over batches to a sequential loop that awaits each batch's
Promise.all before proceeding. Locate the code that builds batches (batches:
[string, string][][]) and the batches.map(...) call and replace it with a
for..of over batches where for each batch you run await
Promise.all(batch.map(async ([path, content]) => { const abs = resolve(root,
path); assertPathInsideRoot(root, abs); await mkdir(dirname(abs), { recursive:
true }); await writeFile(abs, content, "utf8"); }))). This ensures entries are
written in groups of size WRITE_CONCURRENCY rather than all at once.
🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 3d8ed905-eee1-487f-8c97-6c447e95e492

📥 Commits

Reviewing files that changed from the base of the PR and between 723747b and 3095a14.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (13)
  • apps/cli/src/create.ts
  • apps/cli/src/index.ts
  • apps/cli/src/post-scaffold.test.ts
  • apps/cli/src/post-scaffold.ts
  • apps/web/lib/build-command.test.ts
  • apps/web/lib/build-command.ts
  • packages/template-generator/src/writer.test.ts
  • packages/template-generator/src/writer.ts
  • packages/types/package.json
  • packages/types/src/index.ts
  • packages/types/src/project-name.test.ts
  • packages/types/src/project-name.ts
  • scripts/check-doc-links.mjs

Comment thread apps/cli/src/post-scaffold.ts
Comment thread packages/template-generator/src/writer.ts Outdated
Honor config.install inside installDependencies, use execFileSync for git/install commands, and write template files in bounded batches without unbounded parallel Promise.all.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants