fix(init): accept an existing directory unless a scaffold file would be overwritten - #4002
Conversation
…be overwritten `veryfront init app` refused any existing `app/`, including an empty one or a fresh clone holding only `.git`, with "Directory already exists". Every mainstream scaffolder accepts those, and `mkdir app && veryfront init app` is the first thing many developers type. A conflict is now a file the scaffold would write over, not the directory existing. `createProject` is the single authority: the named path uses the same `findExistingPaths` check the current-directory path already used, and both directory-existence checks in `initCommand` are gone. The refusal names the files and points at `--force`: Directory "app" already contains README.md. Use --force to overwrite. `.gitignore` is merged rather than replaced, so it never conflicts. The interactive wizard now runs before a refusal for a taken name; the message it ends on says exactly which files are in the way. The `vf_create_project` MCP tool keeps its own directory check and message; aligning it is a separate change. Tests: empty directory and unrelated-file cases at the `createProject`, `initCommand`, and subprocess levels; the conflict message for a named directory; existing expectations updated from "already exists" to the file-level message. API reference pins regenerated with CI's Deno.
|
Warning Review limit reached
Next review available in: 42 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c67d0ded7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Accepting an existing target directory means the scaffold now meets states
the old "directory already exists" check never let it reach. One of them
wrote outside the project.
`findExistingPaths` asks whether `app/page.tsx` exists. When `app` is a
regular file, that path cannot resolve, so the check reports no conflict.
`writeScaffoldFiles` then writes the root files (`README.md`, `AGENTS.md`)
and fails on `ensureDir("app")` with a raw stat error, leaving a half
scaffold behind. When `app` is a link to another directory, nothing fails
at all: `veryfront init app` exits 0, prints "app ready", and leaves
`page.tsx`, `layout.tsx` and `about/page.mdx` in the link target instead of
the project you named.
`createProject` now checks every directory the scaffold has to create,
before it writes anything, and refuses when one is already a file or a
link:
Directory "app" already contains app as a file or a link, and the
scaffold needs a directory there. Move it aside or use a different name.
The check runs whatever the conflict policy is. `--force` says you accept
your own files being replaced, not the scaffold writing somewhere else.
Every segment is checked, not just the first, so a real `app/` with a file
at `app/about` is caught before `app/page.tsx` is written. A real directory
that is already there is never blocked: it is exactly what the scaffold is
about to create.
The current-directory path had the same hole, so `cd repo && veryfront
init` with a linked `app/` wrote outside the repo too. The check covers
both paths because it sits in `createProject`.
Tests: a file and a link at a scaffold directory, for the named path, the
current-directory path, and under `--force`, plus a block one level down at
`app/about`, each asserting nothing was written through or beside it; and an
existing real `app/` that must still scaffold. Every refusal test fails
without the check.
|
@coderabbitai review The first review attempt on this PR was rate limited, so the green CodeRabbit check on the earlier head carried no signal. The branch has since gained a commit that closes a path where the scaffold wrote outside the project directory, so a real pass over the current head is worth having. |
|
|
|
@codex review Your P2 on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 865e20004c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…h directories Two more places where accepting an existing directory let a write land somewhere it was never asked to go. A link at the scaffold path itself escaped the preflight, which only walked the directories above it. `findExistingPaths` resolves a dangling link to nothing and reports it absent, so `proj/README.md -> ../outside.md` made `veryfront init proj` exit 0, print "proj ready", and write the README to `outside.md` outside the project. The check now walks every segment, including the last, and refuses a link anywhere along the path: Directory "proj" already contains README.md as a file or a link the scaffold cannot write through. Move it aside or use a different name. A real file at a scaffold path is deliberately not refused here. It resolves fine and stays the ordinary conflict pointing at `--force`, pinned by a test so this cannot drift into refusing any directory with a file in it. The named target being a link is still allowed on purpose. `ln -s /mnt/big/app app && veryfront init app` puts the project on another volume and every file is reachable at the path you named. Only a link you did not name can surprise you. The TUI is the second caller of `createProject` with a fail policy, and it relied on the directory check this branch removed. It reserves a new remote slug, then scaffolds into `projects/<slug>`, then writes the link for that slug. With the check gone it would adopt an existing `projects/<slug>` that holds none of the template files, and repoint a directory that is already another project. It now refuses before scaffolding. The constraint belongs in that caller, not in `createProject`: `veryfront init` accepting a directory that exists is the point of this branch, and the TUI wanting a fresh one is the opposite requirement. Tests: a dangling link at a scaffold path, the same under `--force`, a real file at a scaffold path that must stay an overwritable conflict, and a TUI slug whose directory already exists and is linked elsewhere. All fail without these changes.
|
@codex review Your three P2s on |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deep review — merge confidence 55/100 (→ 85+ once the one blocker below lands)Verdict: right approach, real-CLI matrix holds up for everything the body lists — except one reproducible hole in the exact class the last two commits exist to close. What I verified (isolated worktree at 090585f)
BLOCKING
Reproduced twice (reviewer agent, then independently by me) with the real CLI: ( This contradicts the body's "a link anywhere along the path is refused whatever the conflict policy" / "Nothing is ever overwritten through a link". On Fix is small: pass Non-blocking
Not re-verified"10 mutants all killed"; Reviewed with Claude Code; the blocking case was reproduced with the real CLI in a sandbox outside the repo. |
Deep reviewMerge confidence: 38/100 Finding
StandardsThe single-authority move into SpecPartial. Empty/unrelated directories and normal scaffold-path links are handled, but the stated link-anywhere protection is not complete. Architecture
Verification
|
Summary
Follow-up to #3983 and #3985.
veryfront init apprefused any existingapp/— including an empty one (mkdir app && veryfront init app) or a fresh clone holding only.git. Every mainstream scaffolder accepts those.createProjectis the single authority for both the named and current-directory paths (samefindExistingPathscheck fix(init): refuse to overwrite files when scaffolding into the current directory #3985 introduced); the two directory-existence checks ininitCommandare removed. The refusal names the files:.gitignoreis merged, never a conflict. Behaviour change to be aware of: the interactive wizard now runs before a refusal for a taken name (the old pre-wizard check could not know the template's file list). The message it ends on says exactly what is in the way.vf_create_projectMCP tool keeps its ownDirectory already existscheck/message (cli/mcp/tools/catalog-tools.ts). Aligning it is a separate change.docs/api-reference/veryfront/scaffold.mdpins regenerated with CI's Deno 2.7.7.Found by the
vf-dx-dogfoodedge-case pass againstveryfront@0.1.1251; recorded there as friction and deliberately kept out of #3983.Test plan
createProjectinto an empty named dir and beside.git/LICENSEthrew "already exists";initCommandinto an empty dir was refused pre-wizardcli/commands/init/(incl.init.integration.test.ts, subprocess-level empty-dir + conflict cases),cli/shared/project-creation.test.ts,cli/app/— 24 files passdeno run -A cli/main.ts: empty dir → scaffolded;.git-only dir → scaffolded beside.git; dir withREADME.md→ refused, exit 1, file intact; no-name into non-empty cwd → still refused (fix(init): refuse to overwrite files when scaffolding into the current directory #3985 guard)deno lint,deno fmt --check,deno check,lint:anti-slop,lint:sanitizer-baseline,lint:test-typecheck,docs:api-reference:checkcleanUpdate: refuse a file or a link where the scaffold needs a directory
Accepting an existing target directory lets the scaffold reach states the old
Directory already existscheck never let it see. One of them wrote outside theproject, so this branch now closes it.
findExistingPathsasks whetherapp/page.tsxexists. Whenappis a regularfile that path cannot resolve, so the conflict check reports nothing:
appis a file:README.mdandAGENTS.mdget written, thenensureDir("app")fails with a rawNot a directory (os error 20), leaving ahalf scaffold behind.
appis a link to another directory: nothing fails at all.veryfront init projexits 0, printsproj ready, and leavespage.tsx,layout.tsxandabout/page.mdxin the link target instead ofproj/.createProjectnow checks every directory the scaffold has to create, before itwrites anything:
--forcesays you accept yourown files being replaced, not the scaffold writing somewhere else.
app/holding a file atapp/aboutiscaught before
app/page.tsxis written.this PR:
mkdir app && veryfront init appstill scaffolds.ln -s /mnt/big/app app && veryfront init appis a legitimate setup and still works.cd repo && veryfront initwith a linkedapp/wrote outside the repo onmaintoo. The check sits increateProjectand covers both paths.Nothing is ever overwritten through a link:
findExistingPathsresolves throughit, so an existing
elsewhere/page.tsxis still reported as a conflict andrefused with the file byte-identical.
Verification
origin/mainand on this branch side by side: target holdingREADME.md,partial scaffold holding only
app/page.tsx, symlink to a populateddirectory, a directory where a file is expected, a file where a directory is
expected, non-empty and unreadable, empty, unrelated files plus an existing
.gitignore,.git-only, no-name non-empty cwd, and--force. No case wheremainrefused and this branch overwrites. Where behaviour opens up (emptydirectory, unrelated files,
.git-only) the user's files are byte-identicalafterwards and
.gitignoreis merged, not replaced.missing path as blocked, refusing any existing ancestor, refusing only when
every scaffold path exists, refusing any non-empty directory, dropping
deno.jsonor the env files fromscaffoldWritePaths, and removing eitherguard outright are each caught by the suite.
deno task typecheck,deno task lint:ci,deno fmt --check, anddeno test cli/shared/project-creation.test.ts cli/commands/init/all exit 0.Update 2: a link at a scaffold path, and the TUI caller
Codex found two more consequences of accepting an existing directory. Both are fixed.
A link at the scaffold path itself. The first preflight only walked the
directories above a path, so a dangling
proj/README.md -> ../outside.mdslippedthrough:
findExistingPathsresolves a dangling link to nothing and reports itabsent.
veryfront init projexited 0, printedproj ready, and wrote the READMEto
outside.mdoutside the project. Every segment is now checked, including thelast, and a link anywhere along the path is refused whatever the conflict policy:
A real file at a scaffold path is still the ordinary conflict pointing at
--force, pinned by a test so this cannot drift into refusing any non-emptydirectory. The named target being a link stays allowed on purpose:
ln -s /mnt/big/app app && veryfront init appputs the project on another volumeand every file is reachable at the path you named. Only a link you did not name
can surprise you.
The TUI caller.
cli/app/operations/project-creation.tsis the second callerof
createProjectwith a fail policy, and it relied on the directory check thisbranch removes. It reserves a new remote slug, scaffolds into
projects/<slug>,then writes the link for that slug, so with the check gone it would adopt an
existing
projects/<slug>holding none of the template files and repoint adirectory that is already another project. It now refuses first. The constraint
lives in that caller rather than in
createProject, because the two want oppositethings:
veryfront initaccepting an existing directory is the point of this PR,and a freshly reserved slug wanting a fresh directory is the opposite requirement.
Considered and not changed: an orphan
package-lock.jsonis rewritten bynpm install. A real project is already refused, becausepackage.jsonis inscaffoldWritePaths;--skip-installnever touches the lockfile; so the exposureis a lockfile with no manifest, which npm regenerates from the manifest the
scaffold just wrote. Adding lockfiles to the preflight would refuse a directory
holding a stray lockfile, which is exactly the behaviour this PR exists to allow.
Reasoning and evidence are on the thread.
Verification
--force, and the same on the no-name path all exit 1 and create nothing,inside the project or outside it.
unrelated files (
notes.txtintact,.gitignoremerged), and a.git-onlydirectory.
when every scaffold path exists; refuse whenever the directory is non-empty)
are still killed, and the drift test is still the sole killer of the two
scaffoldWritePathsmutants.deno task typecheck,deno task lint:ci,deno fmt --check, anddeno test cli/shared/project-creation.test.ts cli/app/ cli/commands/init/(24 files, 334 steps) all exit 0.