fix(create-rezi): make CLI main-entry detection symlink-safe#275
fix(create-rezi): make CLI main-entry detection symlink-safe#275RtlZeroMemory merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds a symlink-safe main-entry detection helper Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/create-rezi/src/__tests__/mainEntry.test.ts (1)
1-38: Optional: clean up temp dirs created by tests.Not a blocker, but removing test temp directories avoids
/tmpbuildup during repeated local runs.♻️ Suggested cleanup patch
-import { mkdtemp, symlink, writeFile } from "node:fs/promises"; +import { mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; @@ test("isMainModuleEntry returns true for direct script execution", async () => { const root = await mkdtemp(join(tmpdir(), "rezi-main-entry-")); - const scriptPath = join(root, "create-rezi.mjs"); - await writeFile(scriptPath, "export {};\n", "utf8"); - - const moduleUrl = pathToFileURL(scriptPath).href; - assert.equal(isMainModuleEntry(scriptPath, moduleUrl), true); + try { + const scriptPath = join(root, "create-rezi.mjs"); + await writeFile(scriptPath, "export {};\n", "utf8"); + + const moduleUrl = pathToFileURL(scriptPath).href; + assert.equal(isMainModuleEntry(scriptPath, moduleUrl), true); + } finally { + await rm(root, { recursive: true, force: true }); + } }); @@ test("isMainModuleEntry resolves symlinked launchers", async () => { const root = await mkdtemp(join(tmpdir(), "rezi-main-entry-")); - const scriptPath = join(root, "dist-index.mjs"); - const launcherPath = join(root, "launcher.mjs"); - await writeFile(scriptPath, "export {};\n", "utf8"); - await symlink(scriptPath, launcherPath); - - const moduleUrl = pathToFileURL(scriptPath).href; - assert.equal(isMainModuleEntry(launcherPath, moduleUrl), true); + try { + const scriptPath = join(root, "dist-index.mjs"); + const launcherPath = join(root, "launcher.mjs"); + await writeFile(scriptPath, "export {};\n", "utf8"); + await symlink(scriptPath, launcherPath); + + const moduleUrl = pathToFileURL(scriptPath).href; + assert.equal(isMainModuleEntry(launcherPath, moduleUrl), true); + } finally { + await rm(root, { recursive: true, force: true }); + } }); @@ test("isMainModuleEntry rejects non-matching entry paths", async () => { const root = await mkdtemp(join(tmpdir(), "rezi-main-entry-")); - const scriptPath = join(root, "dist-index.mjs"); - const otherPath = join(root, "other.mjs"); - await writeFile(scriptPath, "export {};\n", "utf8"); - await writeFile(otherPath, "export {};\n", "utf8"); - - const moduleUrl = pathToFileURL(scriptPath).href; - assert.equal(isMainModuleEntry(otherPath, moduleUrl), false); - assert.equal(isMainModuleEntry(undefined, moduleUrl), false); + try { + const scriptPath = join(root, "dist-index.mjs"); + const otherPath = join(root, "other.mjs"); + await writeFile(scriptPath, "export {};\n", "utf8"); + await writeFile(otherPath, "export {};\n", "utf8"); + + const moduleUrl = pathToFileURL(scriptPath).href; + assert.equal(isMainModuleEntry(otherPath, moduleUrl), false); + assert.equal(isMainModuleEntry(undefined, moduleUrl), false); + } finally { + await rm(root, { recursive: true, force: true }); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-rezi/src/__tests__/mainEntry.test.ts` around lines 1 - 38, Tests create temporary directories via mkdtemp assigned to variable root but never remove them; add cleanup to delete each temp dir after the test runs (use fs.promises.rm or fs.promises.rmdir with recursive:true) — either call await rm(root, { recursive: true, force: true }) at the end of each test that creates root or add a shared afterEach/after hook that removes any created roots; update the tests that define root (the three tests using mkdtemp and variables root, scriptPath, launcherPath, otherPath) to ensure the temp directory is removed even on failures (try/finally or test framework teardown).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/create-rezi/src/__tests__/mainEntry.test.ts`:
- Around line 1-38: Tests create temporary directories via mkdtemp assigned to
variable root but never remove them; add cleanup to delete each temp dir after
the test runs (use fs.promises.rm or fs.promises.rmdir with recursive:true) —
either call await rm(root, { recursive: true, force: true }) at the end of each
test that creates root or add a shared afterEach/after hook that removes any
created roots; update the tests that define root (the three tests using mkdtemp
and variables root, scriptPath, launcherPath, otherPath) to ensure the temp
directory is removed even on failures (try/finally or test framework teardown).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 749fe751-40ef-4d8c-b44f-763709e2112a
📒 Files selected for processing (4)
CHANGELOG.mdpackages/create-rezi/src/__tests__/mainEntry.test.tspackages/create-rezi/src/index.tspackages/create-rezi/src/mainEntry.ts
d5c1a60 to
ce14cae
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/create-rezi/src/__tests__/mainEntry.test.ts (1)
8-38: Clean up the temp fixtures after each test.Each case leaves a
rezi-main-entry-*directory behind in the system temp folder. Wrapping the fixture setup intry/finallyand removingrootwould keep repeated local runs tidy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-rezi/src/__tests__/mainEntry.test.ts` around lines 8 - 38, Each test that creates a temporary root via mkdtemp should clean it up in a finally block: wrap the body of each test that sets const root = await mkdtemp(...) in try { ... } finally { await fs.rm(root, { recursive: true, force: true }) } (or fs.rmdir/fs.promises.rm equivalent), so update the tests in packages/create-rezi/src/__tests__/mainEntry.test.ts that call mkdtemp and create files (the three tests using root, scriptPath, launcherPath, otherPath) to remove the created temporary directory in a finally block to ensure fixtures are deleted even on failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/create-rezi/src/__tests__/mainEntry.test.ts`:
- Around line 8-38: Each test that creates a temporary root via mkdtemp should
clean it up in a finally block: wrap the body of each test that sets const root
= await mkdtemp(...) in try { ... } finally { await fs.rm(root, { recursive:
true, force: true }) } (or fs.rmdir/fs.promises.rm equivalent), so update the
tests in packages/create-rezi/src/__tests__/mainEntry.test.ts that call mkdtemp
and create files (the three tests using root, scriptPath, launcherPath,
otherPath) to remove the created temporary directory in a finally block to
ensure fixtures are deleted even on failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0fb1057-1a5d-48dc-a215-401a14cd65f0
📒 Files selected for processing (4)
CHANGELOG.mdpackages/create-rezi/src/__tests__/mainEntry.test.tspackages/create-rezi/src/index.tspackages/create-rezi/src/mainEntry.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- packages/create-rezi/src/index.ts
Summary
create-rezimain-entry detection so the CLI executes when invoked through symlinked launchers (node_modules/.bin/create-rezi)mainEntry.tsand canonicalize both paths viarealpathSyncRoot Cause
index.tscomparedfileURLToPath(import.meta.url)withresolve(process.argv[1])directly.When launched via
npm create rezi/bun create rezi,process.argv[1]points at a symlink (.bin/create-rezi), somain()was never called.Validation
node --import tsx --test packages/create-rezi/src/__tests__/mainEntry.test.ts packages/create-rezi/src/__tests__/scaffold.test.tsnpx tsc -b packages/create-rezinpm run check:create-rezi-templatesnpx create-rezi demo --template minimal --no-install(project scaffolded successfully)npm run typechecknpm run lint(fails due pre-existing unrelated workspace formatting/import-order issues inpackages/core; no new diagnostics from touchedcreate-rezifiles)Closes #274
Summary by CodeRabbit
Bug Fixes
Tests