Show Theme Picker during On Boarding #186

coderabbitai · 2025-12-20T07:14:37Z

⚠️ Potential issue | 🟠 Major

Use GET method for retrieving project settings.

Line 31 uses POST for retrieving project settings, which violates REST conventions. GET requests should be used for read operations.

Consider either:

Change to router.get("/project") and accept projectPath as a query parameter

Use a path parameter: router.get("/project/:projectPath") (requires URL encoding for paths)

🔎 Recommended refactor

Option 1 - Query parameter approach:

// Project settings - router.post("/project", createGetProjectHandler(settingsService)); + router.get("/project", createGetProjectHandler(settingsService)); router.put("/project", createUpdateProjectHandler(settingsService));

Then update the handler in apps/server/src/routes/settings/routes/get-project.ts to read from query params:

const { projectPath } = req.query as { projectPath?: string };

Committable suggestion skipped: line range outside the PR's diff.

-Original file line number
+Diff line change
@@ Expand Up / @@ -8,7 +8,7 @@ @@
     >
     > Automaker itself was built by a group of engineers using AI and agentic coding techniques to build features faster than ever. By leveraging tools like Cursor IDE and Claude Code CLI, the team orchestrated AI agents to implement complex functionality in days instead of weeks.
     >
-    > **Learn how:** Master these same techniques and workflows in the [Agentic Jumpstart course](https://agenticjumpstart.com/?utm=automaker).
+    > **Learn how:** Master these same techniques and workflows in the [Agentic Jumpstart course](https://agenticjumpstart.com/?utm=automaker-gh).
     # Automaker
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -37,10 +37,12 @@ import { @@
       isTerminalEnabled,
       isTerminalPasswordRequired,
     } from "./routes/terminal/index.js";
+    import { createSettingsRoutes } from "./routes/settings/index.js";
     import { AgentService } from "./services/agent-service.js";
     import { FeatureLoader } from "./services/feature-loader.js";
     import { AutoModeService } from "./services/auto-mode-service.js";
     import { getTerminalService } from "./services/terminal-service.js";
+    import { SettingsService } from "./services/settings-service.js";
     import { createSpecRegenerationRoutes } from "./routes/app-spec/index.js";
     // Load environment variables
@@ Expand Down Expand Up / @@ -108,6 +110,7 @@ const events: EventEmitter = createEventEmitter(); @@
     const agentService = new AgentService(DATA_DIR, events);
     const featureLoader = new FeatureLoader();
     const autoModeService = new AutoModeService(events);
+    const settingsService = new SettingsService(DATA_DIR);
     // Initialize services
     (async () => {
@@ Expand Down Expand Up @@
     app.use("/api/workspace", createWorkspaceRoutes());
     app.use("/api/templates", createTemplatesRoutes());
     app.use("/api/terminal", createTerminalRoutes());
+    app.use("/api/settings", createSettingsRoutes(settingsService));
     // Create HTTP server
     const server = createServer(app);
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
       await fs.mkdir(automakerDir, { recursive: true });
       return automakerDir;
     }
+    // ============================================================================
+    // Global Settings Paths (stored in DATA_DIR from app.getPath('userData'))
+    // ============================================================================
+    /**
+     * Get the global settings file path
+     * DATA_DIR is typically ~/Library/Application Support/automaker (macOS)
+     * or %APPDATA%\automaker (Windows) or ~/.config/automaker (Linux)
+     */
+    export function getGlobalSettingsPath(dataDir: string): string {
+      return path.join(dataDir, "settings.json");
+    }
+    /**
+     * Get the credentials file path (separate from settings for security)
+     */
+    export function getCredentialsPath(dataDir: string): string {
+      return path.join(dataDir, "credentials.json");
+    }
+    /**
+     * Get the project settings file path within a project's .automaker directory
+     */
+    export function getProjectSettingsPath(projectPath: string): string {
+      return path.join(getAutomakerDir(projectPath), "settings.json");
+    }
+    /**
+     * Ensure the global data directory exists
+     */
+    export async function ensureDataDir(dataDir: string): Promise<string> {
+      await fs.mkdir(dataDir, { recursive: true });
+      return dataDir;
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,15 @@
+    /**
+     * Common utilities for settings routes
+     */
+    import { createLogger } from "../../lib/logger.js";
+    import {
+      getErrorMessage as getErrorMessageShared,
+      createLogError,
+    } from "../common.js";
+    export const logger = createLogger("Settings");
+    // Re-export shared utilities
+    export { getErrorMessageShared as getErrorMessage };
+    export const logError = createLogError(logger);

-Original file line number
+Diff line change
@@ -0,0 +1,38 @@
+    /**
+     * Settings routes - HTTP API for persistent file-based settings
+     */
+    import { Router } from "express";
+    import type { SettingsService } from "../../services/settings-service.js";
+    import { createGetGlobalHandler } from "./routes/get-global.js";
+    import { createUpdateGlobalHandler } from "./routes/update-global.js";
+    import { createGetCredentialsHandler } from "./routes/get-credentials.js";
+    import { createUpdateCredentialsHandler } from "./routes/update-credentials.js";
+    import { createGetProjectHandler } from "./routes/get-project.js";
+    import { createUpdateProjectHandler } from "./routes/update-project.js";
+    import { createMigrateHandler } from "./routes/migrate.js";
+    import { createStatusHandler } from "./routes/status.js";
+    export function createSettingsRoutes(settingsService: SettingsService): Router {
+      const router = Router();
+      // Status endpoint (check if migration needed)
+      router.get("/status", createStatusHandler(settingsService));
+      // Global settings
+      router.get("/global", createGetGlobalHandler(settingsService));
+      router.put("/global", createUpdateGlobalHandler(settingsService));
+      // Credentials (separate for security)
+      router.get("/credentials", createGetCredentialsHandler(settingsService));
+      router.put("/credentials", createUpdateCredentialsHandler(settingsService));
+      // Project settings
+      router.post("/project", createGetProjectHandler(settingsService));
+      router.put("/project", createUpdateProjectHandler(settingsService));
+      // Migration from localStorage
+      router.post("/migrate", createMigrateHandler(settingsService));
+      return router;
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show Theme Picker during On Boarding #186

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Dec 20, 2025

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -0,0 +1,23 @@
+    /**
+     * GET /api/settings/credentials - Get credentials (masked for security)
+     */
+    import type { Request, Response } from "express";
+    import type { SettingsService } from "../../../services/settings-service.js";
+    import { getErrorMessage, logError } from "../common.js";
+    export function createGetCredentialsHandler(settingsService: SettingsService) {
+      return async (_req: Request, res: Response): Promise<void> => {
+        try {
+          const credentials = await settingsService.getMaskedCredentials();
+          res.json({
+            success: true,
+            credentials,
+          });
+        } catch (error) {
+          logError(error, "Get credentials failed");
+          res.status(500).json({ success: false, error: getErrorMessage(error) });
+        }
+      };
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,34 @@
+    /**
+     * POST /api/settings/project - Get project settings
+     * Uses POST because projectPath may contain special characters
+     */
+    import type { Request, Response } from "express";
+    import type { SettingsService } from "../../../services/settings-service.js";
+    import { getErrorMessage, logError } from "../common.js";
+    export function createGetProjectHandler(settingsService: SettingsService) {
+      return async (req: Request, res: Response): Promise<void> => {
+        try {
+          const { projectPath } = req.body as { projectPath?: string };
+          if (!projectPath || typeof projectPath !== "string") {
+            res.status(400).json({
+              success: false,
+              error: "projectPath is required",
+            });
+            return;
+          }
+          const settings = await settingsService.getProjectSettings(projectPath);
+          res.json({
+            success: true,
+            settings,
+          });
+        } catch (error) {
+          logError(error, "Get project settings failed");
+          res.status(500).json({ success: false, error: getErrorMessage(error) });
+        }
+      };
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,54 @@
+    /**
+     * POST /api/settings/migrate - Migrate settings from localStorage
+     */
+    import type { Request, Response } from "express";
+    import type { SettingsService } from "../../../services/settings-service.js";
+    import { getErrorMessage, logError, logger } from "../common.js";
+    export function createMigrateHandler(settingsService: SettingsService) {
+      return async (req: Request, res: Response): Promise<void> => {
+        try {
+          const { data } = req.body as {
+            data?: {
+              "automaker-storage"?: string;
+              "automaker-setup"?: string;
+              "worktree-panel-collapsed"?: string;
+              "file-browser-recent-folders"?: string;
+              "automaker:lastProjectDir"?: string;
+            };
+          };
+          if (!data || typeof data !== "object") {
+            res.status(400).json({
+              success: false,
+              error: "data object is required containing localStorage data",
+            });
+            return;
+          }
+          logger.info("Starting settings migration from localStorage");
+          const result = await settingsService.migrateFromLocalStorage(data);
+          if (result.success) {
+            logger.info(
+              `Migration successful: ${result.migratedProjectCount} projects migrated`
+            );
+          } else {
+            logger.warn(`Migration completed with errors: ${result.errors.join(", ")}`);
+          }
+          res.json({
+            success: result.success,
+            migratedGlobalSettings: result.migratedGlobalSettings,
+            migratedCredentials: result.migratedCredentials,
+            migratedProjectCount: result.migratedProjectCount,
+            errors: result.errors,
+          });
+        } catch (error) {
+          logError(error, "Migration failed");
+          res.status(500).json({ success: false, error: getErrorMessage(error) });
+        }
+      };
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,28 @@
+    /**
+     * GET /api/settings/status - Get settings migration status
+     * Returns whether settings files exist (to determine if migration is needed)
+     */
+    import type { Request, Response } from "express";
+    import type { SettingsService } from "../../../services/settings-service.js";
+    import { getErrorMessage, logError } from "../common.js";
+    export function createStatusHandler(settingsService: SettingsService) {
+      return async (_req: Request, res: Response): Promise<void> => {
+        try {
+          const hasGlobalSettings = await settingsService.hasGlobalSettings();
+          const hasCredentials = await settingsService.hasCredentials();
+          res.json({
+            success: true,
+            hasGlobalSettings,
+            hasCredentials,
+            dataDir: settingsService.getDataDir(),
+            needsMigration: !hasGlobalSettings,
+          });
+        } catch (error) {
+          logError(error, "Get settings status failed");
+          res.status(500).json({ success: false, error: getErrorMessage(error) });
+        }
+      };
+    }

Show Theme Picker during On Boarding #186

Uh oh!

Show Theme Picker during On Boarding #186

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!