Enhance unit tests for settings service and error handling

- Add comprehensive unit tests for SettingsService, covering global and project settings management, including creation, updates, and merging with defaults.
- Implement tests for handling credentials, ensuring proper masking and merging of API keys.
- Introduce tests for migration from localStorage, validating successful data transfer and error handling.
- Enhance error handling in subprocess management tests, ensuring robust timeout and output reading scenarios.

apps/server/tests/unit/lib/automaker-paths.test.ts

@@ -13,6 +13,10 @@ import {
   getAppSpecPath,
   getBranchTrackingPath,
   ensureAutomakerDir,
+  getGlobalSettingsPath,
+  getCredentialsPath,
+  getProjectSettingsPath,
+  ensureDataDir,
 } from "@/lib/automaker-paths.js";
 
 describe("automaker-paths.ts", () => {
@@ -136,4 +140,91 @@ describe("automaker-paths.ts", () => {
       expect(result).toBe(automakerDir);
     });
   });
+
+  describe("getGlobalSettingsPath", () => {
+    it("should return path to settings.json in data directory", () => {
+      const dataDir = "/test/data";
+      const result = getGlobalSettingsPath(dataDir);
+      expect(result).toBe(path.join(dataDir, "settings.json"));
+    });
+
+    it("should handle paths with trailing slashes", () => {
+      const dataDir = "/test/data" + path.sep;
+      const result = getGlobalSettingsPath(dataDir);
+      expect(result).toBe(path.join(dataDir, "settings.json"));
+    });
+  });
+
+  describe("getCredentialsPath", () => {
+    it("should return path to credentials.json in data directory", () => {
+      const dataDir = "/test/data";
+      const result = getCredentialsPath(dataDir);
+      expect(result).toBe(path.join(dataDir, "credentials.json"));
+    });
+
+    it("should handle paths with trailing slashes", () => {
+      const dataDir = "/test/data" + path.sep;
+      const result = getCredentialsPath(dataDir);
+      expect(result).toBe(path.join(dataDir, "credentials.json"));
+    });
+  });
+
+  describe("getProjectSettingsPath", () => {
+    it("should return path to settings.json in project .automaker directory", () => {
+      const projectPath = "/test/project";
+      const result = getProjectSettingsPath(projectPath);
+      expect(result).toBe(
+        path.join(projectPath, ".automaker", "settings.json")
+      );
+    });
+
+    it("should handle paths with trailing slashes", () => {
+      const projectPath = "/test/project" + path.sep;
+      const result = getProjectSettingsPath(projectPath);
+      expect(result).toBe(
+        path.join(projectPath, ".automaker", "settings.json")
+      );
+    });
+  });
+
+  describe("ensureDataDir", () => {
+    let testDir: string;
+
+    beforeEach(async () => {
+      testDir = path.join(os.tmpdir(), `data-dir-test-${Date.now()}`);
+    });
+
+    afterEach(async () => {
+      try {
+        await fs.rm(testDir, { recursive: true, force: true });
+      } catch {
+        // Ignore cleanup errors
+      }
+    });
+
+    it("should create data directory and return path", async () => {
+      const result = await ensureDataDir(testDir);
+
+      expect(result).toBe(testDir);
+      const stats = await fs.stat(testDir);
+      expect(stats.isDirectory()).toBe(true);
+    });
+
+    it("should succeed if directory already exists", async () => {
+      await fs.mkdir(testDir, { recursive: true });
+
+      const result = await ensureDataDir(testDir);
+
+      expect(result).toBe(testDir);
+    });
+
+    it("should create nested directories", async () => {
+      const nestedDir = path.join(testDir, "nested", "deep");
+      const result = await ensureDataDir(nestedDir);
+
+      expect(result).toBe(nestedDir);
+      const stats = await fs.stat(nestedDir);
+      expect(stats.isDirectory()).toBe(true);
+    });
+  });
 });

apps/server/tests/unit/lib/error-handler.test.ts

@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
 import {
   isAbortError,
   isAuthenticationError,
+  isCancellationError,
   classifyError,
   getUserFriendlyErrorMessage,
   type ErrorType,
@@ -32,6 +33,34 @@ describe("error-handler.ts", () => {
     });
   });
 
+  describe("isCancellationError", () => {
+    it("should detect 'cancelled' message", () => {
+      expect(isCancellationError("Operation was cancelled")).toBe(true);
+    });
+
+    it("should detect 'canceled' message", () => {
+      expect(isCancellationError("Request was canceled")).toBe(true);
+    });
+
+    it("should detect 'stopped' message", () => {
+      expect(isCancellationError("Process was stopped")).toBe(true);
+    });
+
+    it("should detect 'aborted' message", () => {
+      expect(isCancellationError("Task was aborted")).toBe(true);
+    });
+
+    it("should be case insensitive", () => {
+      expect(isCancellationError("CANCELLED")).toBe(true);
+      expect(isCancellationError("Canceled")).toBe(true);
+    });
+
+    it("should return false for non-cancellation errors", () => {
+      expect(isCancellationError("File not found")).toBe(false);
+      expect(isCancellationError("Network error")).toBe(false);
+    });
+  });
+
   describe("isAuthenticationError", () => {
     it("should detect 'Authentication failed' message", () => {
       expect(isAuthenticationError("Authentication failed")).toBe(true);
@@ -91,6 +120,42 @@ describe("error-handler.ts", () => {
       expect(result.isAbort).toBe(true); // Still detected as abort too
     });
 
+    it("should classify cancellation errors", () => {
+      const error = new Error("Operation was cancelled");
+      const result = classifyError(error);
+
+      expect(result.type).toBe("cancellation");
+      expect(result.isCancellation).toBe(true);
+      expect(result.isAbort).toBe(false);
+      expect(result.isAuth).toBe(false);
+    });
+
+    it("should prioritize abort over cancellation if both match", () => {
+      const error = new Error("Operation aborted");
+      error.name = "AbortError";
+      const result = classifyError(error);
+
+      expect(result.type).toBe("abort");
+      expect(result.isAbort).toBe(true);
+      expect(result.isCancellation).toBe(true); // Still detected as cancellation too
+    });
+
+    it("should classify cancellation errors with 'canceled' spelling", () => {
+      const error = new Error("Request was canceled");
+      const result = classifyError(error);
+
+      expect(result.type).toBe("cancellation");
+      expect(result.isCancellation).toBe(true);
+    });
+
+    it("should classify cancellation errors with 'stopped' message", () => {
+      const error = new Error("Process was stopped");
+      const result = classifyError(error);
+
+      expect(result.type).toBe("cancellation");
+      expect(result.isCancellation).toBe(true);
+    });
+
     it("should classify generic Error as execution error", () => {
       const error = new Error("Something went wrong");
       const result = classifyError(error);

apps/server/tests/unit/lib/sdk-options.test.ts

@@ -144,6 +144,40 @@ describe("sdk-options.ts", () => {
       expect(options.maxTurns).toBe(MAX_TURNS.extended);
       expect(options.allowedTools).toEqual([...TOOL_PRESETS.readOnly]);
     });
+
+    it("should include systemPrompt when provided", async () => {
+      const { createSuggestionsOptions } = await import("@/lib/sdk-options.js");
+
+      const options = createSuggestionsOptions({
+        cwd: "/test/path",
+        systemPrompt: "Custom prompt",
+      });
+
+      expect(options.systemPrompt).toBe("Custom prompt");
+    });
+
+    it("should include abortController when provided", async () => {
+      const { createSuggestionsOptions } = await import("@/lib/sdk-options.js");
+
+      const abortController = new AbortController();
+      const options = createSuggestionsOptions({
+        cwd: "/test/path",
+        abortController,
+      });
+
+      expect(options.abortController).toBe(abortController);
+    });
+
+    it("should include outputFormat when provided", async () => {
+      const { createSuggestionsOptions } = await import("@/lib/sdk-options.js");
+
+      const options = createSuggestionsOptions({
+        cwd: "/test/path",
+        outputFormat: { type: "json" },
+      });
+
+      expect(options.outputFormat).toEqual({ type: "json" });
+    });
   });
 
   describe("createChatOptions", () => {
@@ -205,6 +239,29 @@ describe("sdk-options.ts", () => {
         autoAllowBashIfSandboxed: true,
       });
     });
+
+    it("should include systemPrompt when provided", async () => {
+      const { createAutoModeOptions } = await import("@/lib/sdk-options.js");
+
+      const options = createAutoModeOptions({
+        cwd: "/test/path",
+        systemPrompt: "Custom prompt",
+      });
+
+      expect(options.systemPrompt).toBe("Custom prompt");
+    });
+
+    it("should include abortController when provided", async () => {
+      const { createAutoModeOptions } = await import("@/lib/sdk-options.js");
+
+      const abortController = new AbortController();
+      const options = createAutoModeOptions({
+        cwd: "/test/path",
+        abortController,
+      });
+
+      expect(options.abortController).toBe(abortController);
+    });
   });
 
   describe("createCustomOptions", () => {
@@ -234,5 +291,42 @@ describe("sdk-options.ts", () => {
       expect(options.maxTurns).toBe(MAX_TURNS.maximum);
       expect(options.allowedTools).toEqual([...TOOL_PRESETS.readOnly]);
     });
+
+    it("should include sandbox when provided", async () => {
+      const { createCustomOptions } = await import("@/lib/sdk-options.js");
+
+      const options = createCustomOptions({
+        cwd: "/test/path",
+        sandbox: { enabled: true, autoAllowBashIfSandboxed: false },
+      });
+
+      expect(options.sandbox).toEqual({
+        enabled: true,
+        autoAllowBashIfSandboxed: false,
+      });
+    });
+
+    it("should include systemPrompt when provided", async () => {
+      const { createCustomOptions } = await import("@/lib/sdk-options.js");
+
+      const options = createCustomOptions({
+        cwd: "/test/path",
+        systemPrompt: "Custom prompt",
+      });
+
+      expect(options.systemPrompt).toBe("Custom prompt");
+    });
+
+    it("should include abortController when provided", async () => {
+      const { createCustomOptions } = await import("@/lib/sdk-options.js");
+
+      const abortController = new AbortController();
+      const options = createCustomOptions({
+        cwd: "/test/path",
+        abortController,
+      });
+
+      expect(options.abortController).toBe(abortController);
+    });
   });
 });

apps/server/tests/unit/lib/security.test.ts

@@ -53,9 +53,24 @@ describe("security.ts", () => {
       expect(allowed).toContain(path.resolve("/data/dir"));
     });
 
+    it("should include WORKSPACE_DIR if set", async () => {
+      process.env.ALLOWED_PROJECT_DIRS = "";
+      process.env.DATA_DIR = "";
+      process.env.WORKSPACE_DIR = "/workspace/dir";
+
+      const { initAllowedPaths, getAllowedPaths } = await import(
+        "@/lib/security.js"
+      );
+      initAllowedPaths();
+
+      const allowed = getAllowedPaths();
+      expect(allowed).toContain(path.resolve("/workspace/dir"));
+    });
+
     it("should handle empty ALLOWED_PROJECT_DIRS", async () => {
       process.env.ALLOWED_PROJECT_DIRS = "";
       process.env.DATA_DIR = "/data";
+      delete process.env.WORKSPACE_DIR;
 
       const { initAllowedPaths, getAllowedPaths } = await import(
         "@/lib/security.js"
@@ -70,6 +85,7 @@ describe("security.ts", () => {
     it("should skip empty entries in comma list", async () => {
       process.env.ALLOWED_PROJECT_DIRS = "/path1,,/path2,  ,/path3";
       process.env.DATA_DIR = "";
+      delete process.env.WORKSPACE_DIR;
 
       const { initAllowedPaths, getAllowedPaths } = await import(
         "@/lib/security.js"

apps/server/tests/unit/lib/subprocess-manager.test.ts

@@ -264,9 +264,66 @@ describe("subprocess-manager.ts", () => {
       );
     });
 
-    // Note: Timeout behavior tests are omitted from unit tests as they involve
-    // complex timing interactions that are difficult to mock reliably.
-    // These scenarios are better covered by integration tests with real subprocesses.
+    // Note: Timeout behavior is difficult to test reliably with mocks due to
+    // timing interactions. The timeout functionality is covered by integration tests.
+    // The error handling path (lines 117-118) is tested below.
+
+    it("should reset timeout when output is received", async () => {
+      vi.useFakeTimers();
+      const mockProcess = createMockProcess({
+        stdoutLines: [
+          '{"type":"first"}',
+          '{"type":"second"}',
+          '{"type":"third"}',
+        ],
+        exitCode: 0,
+        delayMs: 50,
+      });
+
+      vi.mocked(cp.spawn).mockReturnValue(mockProcess);
+
+      const generator = spawnJSONLProcess({
+        ...baseOptions,
+        timeout: 200,
+      });
+
+      const promise = collectAsyncGenerator(generator);
+
+      // Advance time but not enough to trigger timeout
+      await vi.advanceTimersByTimeAsync(150);
+      // Process should not be killed yet
+      expect(mockProcess.kill).not.toHaveBeenCalled();
+
+      vi.useRealTimers();
+      await promise;
+    });
+
+    it("should handle errors when reading stdout", async () => {
+      const mockProcess = new EventEmitter() as any;
+      const stdout = new Readable({
+        read() {
+          // Emit an error after a short delay
+          setTimeout(() => {
+            this.emit("error", new Error("Read error"));
+          }, 10);
+        },
+      });
+      const stderr = new Readable({ read() {} });
+
+      mockProcess.stdout = stdout;
+      mockProcess.stderr = stderr;
+      mockProcess.kill = vi.fn();
+
+      vi.mocked(cp.spawn).mockReturnValue(mockProcess);
+
+      const generator = spawnJSONLProcess(baseOptions);
+
+      await expect(collectAsyncGenerator(generator)).rejects.toThrow("Read error");
+      expect(consoleSpy.error).toHaveBeenCalledWith(
+        expect.stringContaining("Error reading stdout"),
+        expect.any(Error)
+      );
+    });
 
     it("should spawn process with correct arguments", async () => {
       const mockProcess = createMockProcess({ exitCode: 0 });

apps/server/tests/unit/lib/worktree-metadata.test.ts

@@ -66,6 +66,32 @@ describe("worktree-metadata.ts", () => {
       const result = await readWorktreeMetadata(testProjectPath, branch);
       expect(result).toEqual(metadata);
     });
+
+    it("should handle empty branch name", async () => {
+      const branch = "";
+      const metadata: WorktreeMetadata = {
+        branch: "branch",
+        createdAt: new Date().toISOString(),
+      };
+
+      // Empty branch name should be sanitized to "_branch"
+      await writeWorktreeMetadata(testProjectPath, branch, metadata);
+      const result = await readWorktreeMetadata(testProjectPath, branch);
+      expect(result).toEqual(metadata);
+    });
+
+    it("should handle branch name that becomes empty after sanitization", async () => {
+      // Test branch that would become empty after removing invalid chars
+      const branch = "///";
+      const metadata: WorktreeMetadata = {
+        branch: "branch",
+        createdAt: new Date().toISOString(),
+      };
+
+      await writeWorktreeMetadata(testProjectPath, branch, metadata);
+      const result = await readWorktreeMetadata(testProjectPath, branch);
+      expect(result).toEqual(metadata);
+    });
   });
 
   describe("readWorktreeMetadata", () => {