From f41a42010c93b5b0107e5af368783d49cb69d776 Mon Sep 17 00:00:00 2001 From: Shirone Date: Mon, 12 Jan 2026 18:41:56 +0100 Subject: [PATCH] fix: address pr comments --- .../src/routes/backlog-plan/routes/apply.ts | 12 ++- .../worktrees/worktrees-section.tsx | 43 +++++++++-- .../src/hooks/use-project-settings-loader.ts | 26 +++++++ apps/ui/src/store/app-store.ts | 31 ++++++-- docs/worktree-init-script-example.sh | 8 +- libs/platform/src/system-paths.ts | 74 +++++++++++++------ 6 files changed, 158 insertions(+), 36 deletions(-) diff --git a/apps/server/src/routes/backlog-plan/routes/apply.ts b/apps/server/src/routes/backlog-plan/routes/apply.ts index 09ec6696..b6c257a0 100644 --- a/apps/server/src/routes/backlog-plan/routes/apply.ts +++ b/apps/server/src/routes/backlog-plan/routes/apply.ts @@ -12,12 +12,22 @@ const featureLoader = new FeatureLoader(); export function createApplyHandler() { return async (req: Request, res: Response): Promise => { try { - const { projectPath, plan, branchName } = req.body as { + const { + projectPath, + plan, + branchName: rawBranchName, + } = req.body as { projectPath: string; plan: BacklogPlanResult; branchName?: string; }; + // Validate branchName: must be undefined or a non-empty trimmed string + const branchName = + typeof rawBranchName === 'string' && rawBranchName.trim().length > 0 + ? rawBranchName.trim() + : undefined; + if (!projectPath) { res.status(400).json({ success: false, error: 'projectPath required' }); return; diff --git a/apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx b/apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx index 20eb8680..2d232a65 100644 --- a/apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx +++ b/apps/ui/src/components/views/settings-view/worktrees/worktrees-section.tsx @@ -17,6 +17,7 @@ import { cn } from '@/lib/utils'; import { apiGet, apiPut, apiDelete } from '@/lib/api-fetch'; import { toast } from 'sonner'; import { useAppStore } from '@/store/app-store'; +import { getHttpApiClient } from '@/lib/http-api-client'; interface WorktreesSectionProps { useWorktrees: boolean; @@ -217,9 +218,19 @@ export function WorktreesSection({ useWorktrees, onUseWorktreesChange }: Worktre { + onCheckedChange={async (checked) => { if (currentProject?.path) { - setShowInitScriptIndicator(currentProject.path, checked === true); + const value = checked === true; + setShowInitScriptIndicator(currentProject.path, value); + // Persist to server + try { + const httpClient = getHttpApiClient(); + await httpClient.settings.updateProject(currentProject.path, { + showInitScriptIndicator: value, + }); + } catch (error) { + console.error('Failed to persist showInitScriptIndicator:', error); + } } }} className="mt-1" @@ -246,9 +257,19 @@ export function WorktreesSection({ useWorktrees, onUseWorktreesChange }: Worktre { + onCheckedChange={async (checked) => { if (currentProject?.path) { - setAutoDismissInitScriptIndicator(currentProject.path, checked === true); + const value = checked === true; + setAutoDismissInitScriptIndicator(currentProject.path, value); + // Persist to server + try { + const httpClient = getHttpApiClient(); + await httpClient.settings.updateProject(currentProject.path, { + autoDismissInitScriptIndicator: value, + }); + } catch (error) { + console.error('Failed to persist autoDismissInitScriptIndicator:', error); + } } }} className="mt-1" @@ -273,9 +294,19 @@ export function WorktreesSection({ useWorktrees, onUseWorktreesChange }: Worktre { + onCheckedChange={async (checked) => { if (currentProject?.path) { - setDefaultDeleteBranch(currentProject.path, checked === true); + const value = checked === true; + setDefaultDeleteBranch(currentProject.path, value); + // Persist to server + try { + const httpClient = getHttpApiClient(); + await httpClient.settings.updateProject(currentProject.path, { + defaultDeleteBranch: value, + }); + } catch (error) { + console.error('Failed to persist defaultDeleteBranch:', error); + } } }} className="mt-1" diff --git a/apps/ui/src/hooks/use-project-settings-loader.ts b/apps/ui/src/hooks/use-project-settings-loader.ts index 62784f5f..4da50473 100644 --- a/apps/ui/src/hooks/use-project-settings-loader.ts +++ b/apps/ui/src/hooks/use-project-settings-loader.ts @@ -18,6 +18,11 @@ export function useProjectSettingsLoader() { const setCardBorderOpacity = useAppStore((state) => state.setCardBorderOpacity); const setHideScrollbar = useAppStore((state) => state.setHideScrollbar); const setWorktreePanelVisible = useAppStore((state) => state.setWorktreePanelVisible); + const setShowInitScriptIndicator = useAppStore((state) => state.setShowInitScriptIndicator); + const setDefaultDeleteBranch = useAppStore((state) => state.setDefaultDeleteBranch); + const setAutoDismissInitScriptIndicator = useAppStore( + (state) => state.setAutoDismissInitScriptIndicator + ); const loadingRef = useRef(null); const currentProjectRef = useRef(null); @@ -78,6 +83,27 @@ export function useProjectSettingsLoader() { if (result.settings.worktreePanelVisible !== undefined) { setWorktreePanelVisible(requestedProjectPath, result.settings.worktreePanelVisible); } + + // Apply showInitScriptIndicator if present + if (result.settings.showInitScriptIndicator !== undefined) { + setShowInitScriptIndicator( + requestedProjectPath, + result.settings.showInitScriptIndicator + ); + } + + // Apply defaultDeleteBranch if present + if (result.settings.defaultDeleteBranch !== undefined) { + setDefaultDeleteBranch(requestedProjectPath, result.settings.defaultDeleteBranch); + } + + // Apply autoDismissInitScriptIndicator if present + if (result.settings.autoDismissInitScriptIndicator !== undefined) { + setAutoDismissInitScriptIndicator( + requestedProjectPath, + result.settings.autoDismissInitScriptIndicator + ); + } } } catch (error) { console.error('Failed to load project settings:', error); diff --git a/apps/ui/src/store/app-store.ts b/apps/ui/src/store/app-store.ts index 25c431f3..e9593f3c 100644 --- a/apps/ui/src/store/app-store.ts +++ b/apps/ui/src/store/app-store.ts @@ -80,6 +80,9 @@ export type ThemeMode = // LocalStorage key for theme persistence (fallback when server settings aren't available) export const THEME_STORAGE_KEY = 'automaker:theme'; +// Maximum number of output lines to keep in init script state (prevents unbounded memory growth) +export const MAX_INIT_OUTPUT_LINES = 500; + /** * Get the theme from localStorage as a fallback * Used before server settings are loaded (e.g., on login/setup pages) @@ -1823,16 +1826,28 @@ export const useAppStore = create()((set, get) => ({ await syncSettingsToServer(); }, setPlanUseSelectedWorktreeBranch: async (enabled) => { + const previous = get().planUseSelectedWorktreeBranch; set({ planUseSelectedWorktreeBranch: enabled }); // Sync to server settings file const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); - await syncSettingsToServer(); + const ok = await syncSettingsToServer(); + if (!ok) { + logger.error('Failed to sync planUseSelectedWorktreeBranch setting to server - reverting'); + set({ planUseSelectedWorktreeBranch: previous }); + } }, setAddFeatureUseSelectedWorktreeBranch: async (enabled) => { + const previous = get().addFeatureUseSelectedWorktreeBranch; set({ addFeatureUseSelectedWorktreeBranch: enabled }); // Sync to server settings file const { syncSettingsToServer } = await import('@/hooks/use-settings-migration'); - await syncSettingsToServer(); + const ok = await syncSettingsToServer(); + if (!ok) { + logger.error( + 'Failed to sync addFeatureUseSelectedWorktreeBranch setting to server - reverting' + ); + set({ addFeatureUseSelectedWorktreeBranch: previous }); + } }, // Worktree Settings actions @@ -3282,14 +3297,20 @@ export const useAppStore = create()((set, get) => ({ appendInitScriptOutput: (projectPath, branch, content) => { const key = `${projectPath}::${branch}`; - const current = get().initScriptState[key]; - if (!current) return; + // Initialize state if absent to avoid dropping output due to event-order races + const current = get().initScriptState[key] || { + status: 'idle' as const, + branch, + output: [], + }; + // Append new content and enforce fixed-size buffer to prevent memory bloat + const newOutput = [...current.output, content].slice(-MAX_INIT_OUTPUT_LINES); set({ initScriptState: { ...get().initScriptState, [key]: { ...current, - output: [...current.output, content], + output: newOutput, }, }, }); diff --git a/docs/worktree-init-script-example.sh b/docs/worktree-init-script-example.sh index 6e57389f..2f942544 100644 --- a/docs/worktree-init-script-example.sh +++ b/docs/worktree-init-script-example.sh @@ -14,8 +14,12 @@ echo "" # Install dependencies echo "[1/1] Installing npm dependencies..." if [ -f "package.json" ]; then - npm install - echo "Dependencies installed successfully!" + if npm install; then + echo "Dependencies installed successfully!" + else + echo "ERROR: npm install failed with exit code $?" + exit 1 + fi else echo "No package.json found, skipping npm install" fi diff --git a/libs/platform/src/system-paths.ts b/libs/platform/src/system-paths.ts index 3f1a1f4b..31382a33 100644 --- a/libs/platform/src/system-paths.ts +++ b/libs/platform/src/system-paths.ts @@ -232,6 +232,27 @@ export function getClaudeProjectsDir(): string { return path.join(getClaudeConfigDir(), 'projects'); } +/** + * Enumerate directories matching a prefix pattern and return full paths + * Used to resolve dynamic directory names like version numbers + */ +function enumerateMatchingPaths( + parentDir: string, + prefix: string, + ...subPathParts: string[] +): string[] { + try { + if (!fsSync.existsSync(parentDir)) { + return []; + } + const entries = fsSync.readdirSync(parentDir); + const matching = entries.filter((entry) => entry.startsWith(prefix)); + return matching.map((entry) => path.join(parentDir, entry, ...subPathParts)); + } catch { + return []; + } +} + /** * Get common Git Bash installation paths on Windows * Git Bash is needed for running shell scripts cross-platform @@ -242,12 +263,38 @@ export function getGitBashPaths(): string[] { } const homeDir = os.homedir(); + const localAppData = process.env.LOCALAPPDATA || ''; + + // Dynamic paths that require directory enumeration + // winget installs to: LocalAppData\Microsoft\WinGet\Packages\Git.Git_\bin\bash.exe + const wingetGitPaths = localAppData + ? enumerateMatchingPaths( + path.join(localAppData, 'Microsoft', 'WinGet', 'Packages'), + 'Git.Git_', + 'bin', + 'bash.exe' + ) + : []; + + // GitHub Desktop bundles Git at: LocalAppData\GitHubDesktop\app-\resources\app\git\cmd\bash.exe + const githubDesktopPaths = localAppData + ? enumerateMatchingPaths( + path.join(localAppData, 'GitHubDesktop'), + 'app-', + 'resources', + 'app', + 'git', + 'cmd', + 'bash.exe' + ) + : []; + return [ // Standard Git for Windows installations 'C:\\Program Files\\Git\\bin\\bash.exe', 'C:\\Program Files (x86)\\Git\\bin\\bash.exe', // User-local installations - path.join(process.env.LOCALAPPDATA || '', 'Programs', 'Git', 'bin', 'bash.exe'), + path.join(localAppData, 'Programs', 'Git', 'bin', 'bash.exe'), // Scoop package manager path.join(homeDir, 'scoop', 'apps', 'git', 'current', 'bin', 'bash.exe'), // Chocolatey @@ -259,27 +306,10 @@ export function getGitBashPaths(): string[] { 'bin', 'bash.exe' ), - // winget typical location - path.join( - process.env.LOCALAPPDATA || '', - 'Microsoft', - 'WinGet', - 'Packages', - 'Git.Git_*', - 'bin', - 'bash.exe' - ), - // GitHub Desktop bundled Git - path.join( - process.env.LOCALAPPDATA || '', - 'GitHubDesktop', - 'app-*', - 'resources', - 'app', - 'git', - 'cmd', - 'bash.exe' - ), + // winget installations (dynamically resolved) + ...wingetGitPaths, + // GitHub Desktop bundled Git (dynamically resolved) + ...githubDesktopPaths, ].filter(Boolean); }