fix(scaffolding): remove partially created project directory on failure

.changeset/bitter-rats-fail.md

@@ -0,0 +1,5 @@
+---
+"scaffolder-toolkit": patch
+---
+
+fix(scaffolding): Remove partially created project directory on failure

packages/devkit/TODO.md

@@ -66,7 +66,7 @@ This document tracks all planned and completed tasks for the Dev Kit project.
 - [x] add short keys to get config using `dk config` like `dk config lang` for `dk config language`
 - [x] ** Enhance for organization Purpose **: Add new language `Typescript(ts)` with same code as javascript(js), also support for nodejs(node) template name for those who prefer it than the programming language name
 - [x] **Testing**: Stabilize the integration test of the `new` command
-- [ ] Make sure to clean up if the `dk new` command fail
+- [x] Make sure to clean up if the `dk new` command fail
 - [ ] Add a configuration validation step when updating the config file to ensure all required fields are present and correctly formatted.
 - [ ] **Dynamic Help Text**: Programmatically generate help text for options with constrained values (e.g., `--cache-strategy`) to ensure it's always up to date.
 

packages/devkit/__tests__/integrations/new.spec.ts

@@ -286,7 +286,6 @@ describe("dk new", () => {
         { all: true, cwd: tempDir },
       );
       expect(exitCode).toBe(0);
-
       expect(
         await fs.pathExists(path.join(tempDir, "my-vue-app", "package.json")),
       ).toBe(true);

packages/devkit/__tests__/units/scaffolding/javascript.spec.ts

@@ -6,98 +6,132 @@ import {
 import { DevkitError } from "../../../src/utils/errors/base.js";
 import { mockSpinner, mockLogger } from "../../../vitest.setup.js";
 
-const {
-  mockRunCliCommand,
-  mockInstallDependencies,
-  mockCopyLocalTemplate,
-  mockGetTemplateFromCache,
-} = vi.hoisted(() => ({
-  mockRunCliCommand: vi.fn(),
-  mockInstallDependencies: vi.fn(),
-  mockGetTemplateFromCache: vi.fn(),
-  mockCopyLocalTemplate: vi.fn(),
-}));
-
-vi.mock("#scaffolding/cli-runner.js", () => ({
-  runCliCommand: mockRunCliCommand,
-}));
-
-vi.mock("#core/cache/index.js", () => ({
-  getTemplateFromCache: mockGetTemplateFromCache,
-}));
+const { mockScaffoldTemplate, mockInstallDependencies, mockFsRemove } =
+  vi.hoisted(() => ({
+    mockScaffoldTemplate: vi.fn(),
+    mockInstallDependencies: vi.fn(),
+    mockFsRemove: vi.fn(),
+  }));
 
-vi.mock("#scaffolding/local-template.js", () => ({
-  copyLocalTemplate: mockCopyLocalTemplate,
+vi.mock("../../../src/scaffolding/scaffold-template.js", () => ({
+  scaffoldTemplate: mockScaffoldTemplate,
 }));
 
 vi.mock("#scaffolding/dependencies.js", () => ({
   installDependencies: mockInstallDependencies,
 }));
 
+vi.mock("#utils/fs/file.js", () => ({
+  default: {
+    remove: mockFsRemove,
+  },
+}));
+
 describe("scaffoldProject", () => {
   const options = {
     projectName: "my-project",
     packageManager: "npm",
     cacheStrategy: "daily",
+    templateConfig: { location: "mock-location" },
   } as ScaffoldJavascriptProjectOptions;
 
   beforeEach(() => {
     vi.clearAllMocks();
+
+    mockScaffoldTemplate.mockResolvedValue({
+      isOfficialCli: false,
+      projectDirCreated: false,
+    });
+    mockInstallDependencies.mockResolvedValue(undefined);
   });
 
-  it("should run official CLI command for a {pm} template", async () => {
-    const templateConfig = { location: "{pm} create vue" };
-    await scaffoldProject({ ...options, templateConfig });
-    expect(mockRunCliCommand).toHaveBeenCalledOnce();
-    expect(mockRunCliCommand).toHaveBeenCalledWith({
-      command: "{pm} create vue",
-      projectName: options.projectName,
-      packageManager: options.packageManager,
-      spinner: mockSpinner,
+  it("should skip dependency install and success log for official CLI templates", async () => {
+    mockScaffoldTemplate.mockResolvedValue({
+      isOfficialCli: true,
+      projectDirCreated: false,
     });
+
+    await scaffoldProject(options);
+
+    expect(mockScaffoldTemplate).toHaveBeenCalledOnce();
     expect(mockInstallDependencies).not.toHaveBeenCalled();
-    expect(mockSpinner.fail).not.toHaveBeenCalled();
+    expect(mockLogger.log).not.toHaveBeenCalledWith(
+      expect.stringContaining("messages.success.scaffolding_complete"),
+    );
   });
 
-  it("should get template from cache for a remote Git URL", async () => {
-    const templateConfig = { location: "https://github.com/repo/test.git" };
-    await scaffoldProject({ ...options, templateConfig });
-    expect(mockGetTemplateFromCache).toHaveBeenCalledWith({
-      url: templateConfig.location,
-      projectName: options.projectName,
-      spinner: mockSpinner,
-      strategy: options.cacheStrategy,
+  it("should run dependency install and log success for custom templates", async () => {
+    mockScaffoldTemplate.mockResolvedValue({
+      isOfficialCli: false,
+      projectDirCreated: true,
     });
-    expect(mockInstallDependencies).toHaveBeenCalled();
+
+    await scaffoldProject(options);
+
+    expect(mockScaffoldTemplate).toHaveBeenCalledOnce();
+    expect(mockInstallDependencies).toHaveBeenCalledOnce();
+    expect(mockLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining("messages.success.scaffolding_complete"),
+    );
   });
 
-  it("should copy local template for a relative path", async () => {
-    const templateConfig = { location: "./templates/local" };
-    await scaffoldProject({ ...options, templateConfig });
-    expect(mockCopyLocalTemplate).toHaveBeenCalledWith({
-      sourcePath: templateConfig.location,
-      projectName: options.projectName,
-      spinner: mockSpinner,
-    });
-    expect(mockInstallDependencies).toHaveBeenCalled();
+  it("should NOT call fs.remove if projectDirCreated is false on failure", async () => {
+    mockScaffoldTemplate.mockRejectedValueOnce(new DevkitError("CLI error"));
+
+    await scaffoldProject(options);
+
+    expect(mockSpinner.fail).toHaveBeenCalledOnce();
+    expect(mockFsRemove).not.toHaveBeenCalled();
+    expect(mockLogger.error).toHaveBeenCalled();
   });
 
-  it("should call spinner.fail and console.error on any exception", async () => {
-    const templateConfig = { location: "http://invalid-url" };
-    vi.mocked(mockGetTemplateFromCache).mockRejectedValueOnce(
-      new DevkitError("Test error"),
+  it("should call fs.remove and log cleanup warning if projectDirCreated is true on failure", async () => {
+    mockScaffoldTemplate.mockResolvedValueOnce({
+      isOfficialCli: false,
+      projectDirCreated: true,
+    });
+    vi.mocked(mockInstallDependencies).mockRejectedValueOnce(
+      new DevkitError("Install error"),
     );
-    await scaffoldProject({ ...options, templateConfig });
-    expect(mockSpinner.fail).toHaveBeenCalled();
+
+    await scaffoldProject(options);
+
+    expect(mockSpinner.fail).toHaveBeenCalledOnce();
+    expect(mockFsRemove).toHaveBeenCalledWith(options.projectName);
     expect(mockLogger.error).toHaveBeenCalled();
+    expect(mockLogger.warning).toHaveBeenCalledWith(
+      expect.stringContaining("messages.status.project_removed"),
+    );
   });
 
-  it("should log success messages and next steps for non-CLI templates", async () => {
-    const templateConfig = { location: "http://example.com" };
-    await scaffoldProject({ ...options, templateConfig });
-    expect(mockInstallDependencies).toHaveBeenCalled();
-    expect(mockLogger.log).toHaveBeenCalledWith(
-      expect.stringContaining("messages.success.scaffolding_complete"),
+  it("should log cleanup failure error if fs.remove itself fails", async () => {
+    mockScaffoldTemplate.mockResolvedValueOnce({
+      isOfficialCli: false,
+      projectDirCreated: true,
+    });
+    vi.mocked(mockInstallDependencies).mockRejectedValueOnce(
+      new DevkitError("Install error"),
+    );
+    vi.mocked(mockFsRemove).mockRejectedValueOnce(
+      new Error("Permissions denied"),
+    );
+
+    await scaffoldProject(options);
+
+    expect(mockSpinner.fail).toHaveBeenCalledOnce();
+    expect(mockFsRemove).toHaveBeenCalledWith(options.projectName);
+    expect(mockLogger.error).toHaveBeenCalledTimes(2);
+    expect(mockLogger.error).toHaveBeenCalledWith(
+      expect.stringContaining("errors.scaffolding.fail"),
+      "CLEANUP",
     );
   });
+
+  it("should call spinner.fail and console.error on any exception", async () => {
+    mockScaffoldTemplate.mockRejectedValueOnce(new Error("Generic error"));
+    await scaffoldProject(options);
+    expect(mockSpinner.fail).toHaveBeenCalled();
+    expect(mockLogger.error).toHaveBeenCalled();
+    expect(mockFsRemove).not.toHaveBeenCalled();
+  });
 });

packages/devkit/__tests__/units/scaffolding/scaffold-template.spec.ts

@@ -0,0 +1,146 @@
+import { vi, describe, it, expect, beforeEach } from "vitest";
+import { scaffoldTemplate } from "../../../src/scaffolding/scaffold-template.js";
+import { DevkitError } from "../../../src/utils/errors/base.js";
+import { mockSpinner } from "../../../vitest.setup.js";
+
+const { mockRunCliCommand, mockGetTemplateFromCache, mockCopyLocalTemplate } =
+  vi.hoisted(() => ({
+    mockRunCliCommand: vi.fn(),
+    mockGetTemplateFromCache: vi.fn(),
+    mockCopyLocalTemplate: vi.fn(),
+  }));
+
+vi.mock("#scaffolding/cli-runner.js", () => ({
+  runCliCommand: mockRunCliCommand,
+}));
+
+vi.mock("#core/cache/index.js", () => ({
+  getTemplateFromCache: mockGetTemplateFromCache,
+}));
+
+vi.mock("#scaffolding/local-template.js", () => ({
+  copyLocalTemplate: mockCopyLocalTemplate,
+}));
+
+describe("scaffoldTemplate", () => {
+  const projectName = "my-test-app";
+  const packageManager = "npm";
+  const cacheStrategy = "daily";
+  const spinner = mockSpinner;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("should run the official CLI command and return isOfficialCli: true", async () => {
+    const templateConfig = { location: "{pm} create vue" };
+
+    const result = await scaffoldTemplate(
+      projectName,
+      templateConfig,
+      packageManager,
+      cacheStrategy,
+      spinner,
+    );
+
+    expect(mockRunCliCommand).toHaveBeenCalledWith({
+      command: templateConfig.location,
+      projectName,
+      packageManager,
+      spinner,
+    });
+    expect(mockGetTemplateFromCache).not.toHaveBeenCalled();
+    expect(mockCopyLocalTemplate).not.toHaveBeenCalled();
+
+    expect(result).toEqual({ isOfficialCli: true, projectDirCreated: false });
+
+    expect(spinner.text).not.toBe("messages.scaffolding.copy_start");
+    expect(spinner.stop).toHaveBeenCalled();
+  });
+
+  it("should get the template from cache and return projectDirCreated: true", async () => {
+    const templateConfig = { location: "https://github.com/repo/test.git" };
+
+    const result = await scaffoldTemplate(
+      projectName,
+      templateConfig,
+      packageManager,
+      cacheStrategy,
+      spinner,
+    );
+
+    expect(mockGetTemplateFromCache).toHaveBeenCalledWith({
+      url: templateConfig.location,
+      projectName,
+      spinner,
+      strategy: cacheStrategy,
+    });
+    expect(mockRunCliCommand).not.toHaveBeenCalled();
+    expect(mockCopyLocalTemplate).not.toHaveBeenCalled();
+
+    expect(result).toEqual({ isOfficialCli: false, projectDirCreated: true });
+
+    expect(spinner.stop).not.toHaveBeenCalled();
+    expect(spinner.succeed).not.toHaveBeenCalled();
+  });
+
+  it("should get template from cache for a git@ url", async () => {
+    const templateConfig = { location: "git@github.com:repo/test.git" };
+
+    await scaffoldTemplate(
+      projectName,
+      templateConfig,
+      packageManager,
+      cacheStrategy,
+      spinner,
+    );
+
+    expect(mockGetTemplateFromCache).toHaveBeenCalledWith(
+      expect.objectContaining({ url: templateConfig.location }),
+    );
+  });
+
+  it("should copy the local template and return projectDirCreated: true", async () => {
+    const templateConfig = { location: "./templates/local" };
+
+    const result = await scaffoldTemplate(
+      projectName,
+      templateConfig,
+      packageManager,
+      cacheStrategy,
+      spinner,
+    );
+
+    expect(mockCopyLocalTemplate).toHaveBeenCalledWith({
+      sourcePath: templateConfig.location,
+      projectName,
+      spinner,
+    });
+    expect(mockRunCliCommand).not.toHaveBeenCalled();
+    expect(mockGetTemplateFromCache).not.toHaveBeenCalled();
+
+    expect(result).toEqual({ isOfficialCli: false, projectDirCreated: true });
+
+    expect(spinner.text).toBe("messages.scaffolding.copy_start");
+    expect(spinner.succeed).toHaveBeenCalled();
+  });
+
+  it("should throw an exception if any underlying scaffolding call fails", async () => {
+    const templateConfig = { location: "http://invalid-url" };
+    const error = new DevkitError("Cache download failed");
+
+    vi.mocked(mockGetTemplateFromCache).mockRejectedValueOnce(error);
+
+    await expect(
+      scaffoldTemplate(
+        projectName,
+        templateConfig,
+        packageManager,
+        cacheStrategy,
+        spinner,
+      ),
+    ).rejects.toThrow(error);
+
+    expect(spinner.fail).not.toHaveBeenCalled();
+  });
+});

packages/devkit/locales/en.json

@@ -268,7 +268,8 @@
       "config_source_local": "Using local configuration.",
       "config_source_global": "Using global configuration.",
       "templates_using_global_fallback": "No local configuration found. Using templates from global configuration.",
-      "including_defaults_suffix": "(including default templates)"
+      "including_defaults_suffix": "(including default templates)",
+      "project_removed": "Project '{project}' removed successfully."
     },
     "scaffolding": {
       "start": "Scaffolding {language} project: {project}",

packages/devkit/locales/fr.json

@@ -268,7 +268,8 @@
       "config_source_local": "Utilisation de la configuration locale.",
       "config_source_global": "Utilisation de la configuration globale.",
       "templates_using_global_fallback": "Aucune configuration locale trouvée. Utilisation des modèles de la configuration globale.",
-      "including_defaults_suffix": "(incluant les modèles par défaut)"
+      "including_defaults_suffix": "(incluant les modèles par défaut)",
+      "project_removed": "Projet '{project}' supprimé avec succès."
     },
     "scaffolding": {
       "start": "Échafaudage du projet {language} : {project}",

packages/devkit/src/core/template/update-project-name.ts

@@ -17,9 +17,11 @@ export async function updateJavascriptProjectName(
 
   try {
     const packageJson = await fs.readJson(packageJsonPath);
-    packageJson.name = newProjectName;
 
-    await fs.writeJson(packageJsonPath, packageJson);
+    await fs.writeJson(packageJsonPath, {
+      ...packageJson,
+      name: newProjectName,
+    });
   } catch (error) {
     const errorMessage = t("errors.system.package_name_update_fail");