diff --git a/Dockerfile b/Dockerfile index a68901e4d..7d48e15fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -209,9 +209,10 @@ COPY libs ./libs COPY apps/ui ./apps/ui # Build packages in dependency order, then build UI -# VITE_SERVER_URL tells the UI where to find the API server -# Use ARG to allow overriding at build time: --build-arg VITE_SERVER_URL=http://api.example.com -ARG VITE_SERVER_URL=http://localhost:3008 +# When VITE_SERVER_URL is empty, the UI uses relative URLs (e.g., /api/...) which nginx proxies +# to the server container. This avoids CORS issues entirely in Docker Compose setups. +# Override at build time if needed: --build-arg VITE_SERVER_URL=http://api.example.com +ARG VITE_SERVER_URL= ENV VITE_SKIP_ELECTRON=true ENV VITE_SERVER_URL=${VITE_SERVER_URL} RUN npm run build:packages && npm run build --workspace=apps/ui diff --git a/apps/server/package.json b/apps/server/package.json index 4a7a75a8b..75818b182 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -1,6 +1,6 @@ { "name": "@automaker/server", - "version": "0.13.0", + "version": "0.15.0", "description": "Backend server for Automaker - provides API for both web and Electron modes", "author": "AutoMaker Team", "license": "SEE LICENSE IN LICENSE", diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index 0acea6c9b..dcd45da80 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -267,6 +267,26 @@ app.use( // CORS configuration // When using credentials (cookies), origin cannot be '*' // We dynamically allow the requesting origin for local development + +// Check if origin is a local/private network address +function isLocalOrigin(origin: string): boolean { + try { + const url = new URL(origin); + const hostname = url.hostname; + return ( + hostname === 'localhost' || + hostname === '127.0.0.1' || + hostname === '[::1]' || + hostname === '0.0.0.0' || + hostname.startsWith('192.168.') || + hostname.startsWith('10.') || + /^172\.(1[6-9]|2[0-9]|3[0-1])\./.test(hostname) + ); + } catch { + return false; + } +} + app.use( cors({ origin: (origin, callback) => { @@ -277,35 +297,25 @@ app.use( } // If CORS_ORIGIN is set, use it (can be comma-separated list) - const allowedOrigins = process.env.CORS_ORIGIN?.split(',').map((o) => o.trim()); - if (allowedOrigins && allowedOrigins.length > 0 && allowedOrigins[0] !== '*') { + const allowedOrigins = process.env.CORS_ORIGIN?.split(',') + .map((o) => o.trim()) + .filter(Boolean); + if (allowedOrigins && allowedOrigins.length > 0) { + if (allowedOrigins.includes('*')) { + callback(null, true); + return; + } if (allowedOrigins.includes(origin)) { callback(null, origin); - } else { - callback(new Error('Not allowed by CORS')); + return; } - return; + // Fall through to local network check below } - // For local development, allow all localhost/loopback origins (any port) - try { - const url = new URL(origin); - const hostname = url.hostname; - - if ( - hostname === 'localhost' || - hostname === '127.0.0.1' || - hostname === '::1' || - hostname === '0.0.0.0' || - hostname.startsWith('192.168.') || - hostname.startsWith('10.') || - hostname.startsWith('172.') - ) { - callback(null, origin); - return; - } - } catch { - // Ignore URL parsing errors + // Allow all localhost/loopback/private network origins (any port) + if (isLocalOrigin(origin)) { + callback(null, origin); + return; } // Reject other origins by default for security diff --git a/apps/server/src/lib/sdk-options.ts b/apps/server/src/lib/sdk-options.ts index 915b55c2d..7044221e1 100644 --- a/apps/server/src/lib/sdk-options.ts +++ b/apps/server/src/lib/sdk-options.ts @@ -133,12 +133,16 @@ export const TOOL_PRESETS = { 'Read', 'Write', 'Edit', + 'MultiEdit', 'Glob', 'Grep', + 'LS', 'Bash', 'WebSearch', 'WebFetch', 'TodoWrite', + 'Task', + 'Skill', ] as const, /** Tools for chat/interactive mode */ @@ -146,12 +150,16 @@ export const TOOL_PRESETS = { 'Read', 'Write', 'Edit', + 'MultiEdit', 'Glob', 'Grep', + 'LS', 'Bash', 'WebSearch', 'WebFetch', 'TodoWrite', + 'Task', + 'Skill', ] as const, } as const; @@ -282,11 +290,15 @@ function buildThinkingOptions(thinkingLevel?: ThinkingLevel): Partial { } /** - * Build system prompt configuration based on autoLoadClaudeMd setting. - * When autoLoadClaudeMd is true: - * - Uses preset mode with 'claude_code' to enable CLAUDE.md auto-loading - * - If there's a custom systemPrompt, appends it to the preset - * - Sets settingSources to ['project'] for SDK to load CLAUDE.md files + * Build system prompt and settingSources based on two independent settings: + * - useClaudeCodeSystemPrompt: controls whether to use the 'claude_code' preset as the base prompt + * - autoLoadClaudeMd: controls whether to add settingSources for SDK to load CLAUDE.md files + * + * These combine independently (4 possible states): + * 1. Both ON: preset + settingSources (full Claude Code experience) + * 2. useClaudeCodeSystemPrompt ON, autoLoadClaudeMd OFF: preset only (no CLAUDE.md auto-loading) + * 3. useClaudeCodeSystemPrompt OFF, autoLoadClaudeMd ON: plain string + settingSources + * 4. Both OFF: plain string only * * @param config - The SDK options config * @returns Object with systemPrompt and settingSources for SDK options @@ -295,27 +307,34 @@ function buildClaudeMdOptions(config: CreateSdkOptionsConfig): { systemPrompt?: string | SystemPromptConfig; settingSources?: Array<'user' | 'project' | 'local'>; } { - if (!config.autoLoadClaudeMd) { - // Standard mode - just pass through the system prompt as-is - return config.systemPrompt ? { systemPrompt: config.systemPrompt } : {}; - } - - // Auto-load CLAUDE.md mode - use preset with settingSources const result: { - systemPrompt: SystemPromptConfig; - settingSources: Array<'user' | 'project' | 'local'>; - } = { - systemPrompt: { + systemPrompt?: string | SystemPromptConfig; + settingSources?: Array<'user' | 'project' | 'local'>; + } = {}; + + // Determine system prompt format based on useClaudeCodeSystemPrompt + if (config.useClaudeCodeSystemPrompt) { + // Use Claude Code's built-in system prompt as the base + const presetConfig: SystemPromptConfig = { type: 'preset', preset: 'claude_code', - }, - // Load both user (~/.claude/CLAUDE.md) and project (.claude/CLAUDE.md) settings - settingSources: ['user', 'project'], - }; + }; + // If there's a custom system prompt, append it to the preset + if (config.systemPrompt) { + presetConfig.append = config.systemPrompt; + } + result.systemPrompt = presetConfig; + } else { + // Standard mode - just pass through the system prompt as-is + if (config.systemPrompt) { + result.systemPrompt = config.systemPrompt; + } + } - // If there's a custom system prompt, append it to the preset - if (config.systemPrompt) { - result.systemPrompt.append = config.systemPrompt; + // Determine settingSources based on autoLoadClaudeMd + if (config.autoLoadClaudeMd) { + // Load both user (~/.claude/CLAUDE.md) and project (.claude/CLAUDE.md) settings + result.settingSources = ['user', 'project']; } return result; @@ -323,12 +342,14 @@ function buildClaudeMdOptions(config: CreateSdkOptionsConfig): { /** * System prompt configuration for SDK options - * When using preset mode with claude_code, CLAUDE.md files are automatically loaded + * The 'claude_code' preset provides the system prompt only — it does NOT auto-load + * CLAUDE.md files. CLAUDE.md auto-loading is controlled independently by + * settingSources (set via autoLoadClaudeMd). These two settings are orthogonal. */ export interface SystemPromptConfig { - /** Use preset mode with claude_code to enable CLAUDE.md auto-loading */ + /** Use preset mode to select the base system prompt */ type: 'preset'; - /** The preset to use - 'claude_code' enables CLAUDE.md loading */ + /** The preset to use - 'claude_code' uses the Claude Code system prompt */ preset: 'claude_code'; /** Optional additional prompt to append to the preset */ append?: string; @@ -362,6 +383,9 @@ export interface CreateSdkOptionsConfig { /** Enable auto-loading of CLAUDE.md files via SDK's settingSources */ autoLoadClaudeMd?: boolean; + /** Use Claude Code's built-in system prompt (claude_code preset) as the base prompt */ + useClaudeCodeSystemPrompt?: boolean; + /** MCP servers to make available to the agent */ mcpServers?: Record; diff --git a/apps/server/src/lib/settings-helpers.ts b/apps/server/src/lib/settings-helpers.ts index d0855f940..48c06383d 100644 --- a/apps/server/src/lib/settings-helpers.ts +++ b/apps/server/src/lib/settings-helpers.ts @@ -34,10 +34,10 @@ import { const logger = createLogger('SettingsHelper'); /** Default number of agent turns used when no value is configured. */ -export const DEFAULT_MAX_TURNS = 1000; +export const DEFAULT_MAX_TURNS = 10000; /** Upper bound for the max-turns clamp; values above this are capped here. */ -export const MAX_ALLOWED_TURNS = 2000; +export const MAX_ALLOWED_TURNS = 10000; /** * Get the autoLoadClaudeMd setting, with project settings taking precedence over global. @@ -80,6 +80,49 @@ export async function getAutoLoadClaudeMdSetting( } } +/** + * Get the useClaudeCodeSystemPrompt setting, with project settings taking precedence over global. + * Falls back to global settings and defaults to true when unset. + * Returns true if settings service is not available. + * + * @param projectPath - Path to the project + * @param settingsService - Optional settings service instance + * @param logPrefix - Prefix for log messages (e.g., '[AgentService]') + * @returns Promise resolving to the useClaudeCodeSystemPrompt setting value + */ +export async function getUseClaudeCodeSystemPromptSetting( + projectPath: string, + settingsService?: SettingsService | null, + logPrefix = '[SettingsHelper]' +): Promise { + if (!settingsService) { + logger.info( + `${logPrefix} SettingsService not available, useClaudeCodeSystemPrompt defaulting to true` + ); + return true; + } + + try { + // Check project settings first (takes precedence) + const projectSettings = await settingsService.getProjectSettings(projectPath); + if (projectSettings.useClaudeCodeSystemPrompt !== undefined) { + logger.info( + `${logPrefix} useClaudeCodeSystemPrompt from project settings: ${projectSettings.useClaudeCodeSystemPrompt}` + ); + return projectSettings.useClaudeCodeSystemPrompt; + } + + // Fall back to global settings + const globalSettings = await settingsService.getGlobalSettings(); + const result = globalSettings.useClaudeCodeSystemPrompt ?? true; + logger.info(`${logPrefix} useClaudeCodeSystemPrompt from global settings: ${result}`); + return result; + } catch (error) { + logger.error(`${logPrefix} Failed to load useClaudeCodeSystemPrompt setting:`, error); + throw error; + } +} + /** * Get the default max turns setting from global settings. * diff --git a/apps/server/src/providers/claude-provider.ts b/apps/server/src/providers/claude-provider.ts index 0cb9fce90..0923a626c 100644 --- a/apps/server/src/providers/claude-provider.ts +++ b/apps/server/src/providers/claude-provider.ts @@ -33,8 +33,23 @@ const logger = createLogger('ClaudeProvider'); */ type ProviderConfig = ClaudeApiProfile | ClaudeCompatibleProvider; -// System vars are always passed from process.env regardless of profile -const SYSTEM_ENV_VARS = ['PATH', 'HOME', 'SHELL', 'TERM', 'USER', 'LANG', 'LC_ALL']; +// System vars are always passed from process.env regardless of profile. +// Includes filesystem, locale, and temp directory vars that the Claude CLI +// needs internally for config resolution and temp file creation. +const SYSTEM_ENV_VARS = [ + 'PATH', + 'HOME', + 'SHELL', + 'TERM', + 'USER', + 'LANG', + 'LC_ALL', + 'TMPDIR', + 'XDG_CONFIG_HOME', + 'XDG_DATA_HOME', + 'XDG_CACHE_HOME', + 'XDG_STATE_HOME', +]; /** * Check if the config is a ClaudeCompatibleProvider (new system) @@ -213,6 +228,8 @@ export class ClaudeProvider extends BaseProvider { env: buildEnv(providerConfig, credentials), // Pass through allowedTools if provided by caller (decided by sdk-options.ts) ...(allowedTools && { allowedTools }), + // Restrict available built-in tools if specified (tools: [] disables all tools) + ...(options.tools && { tools: options.tools }), // AUTONOMOUS MODE: Always bypass permissions for fully autonomous operation permissionMode: 'bypassPermissions', allowDangerouslySkipPermissions: true, diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index 10146591c..63d410362 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -33,7 +33,6 @@ import { supportsReasoningEffort, validateBareModelId, calculateReasoningTimeout, - DEFAULT_TIMEOUT_MS, type CodexApprovalPolicy, type CodexSandboxMode, type CodexAuthStatus, @@ -52,6 +51,7 @@ import { CODEX_MODELS } from './codex-models.js'; const CODEX_COMMAND = 'codex'; const CODEX_EXEC_SUBCOMMAND = 'exec'; +const CODEX_RESUME_SUBCOMMAND = 'resume'; const CODEX_JSON_FLAG = '--json'; const CODEX_MODEL_FLAG = '--model'; const CODEX_VERSION_FLAG = '--version'; @@ -98,7 +98,7 @@ const TEXT_ENCODING = 'utf-8'; * * @see calculateReasoningTimeout from @automaker/types */ -const CODEX_CLI_TIMEOUT_MS = DEFAULT_TIMEOUT_MS; +const CODEX_CLI_TIMEOUT_MS = 120000; // 2 minutes — matches CLI provider base timeout const CODEX_FEATURE_GENERATION_BASE_TIMEOUT_MS = 300000; // 5 minutes for feature generation const SYSTEM_PROMPT_SEPARATOR = '\n\n'; const CODEX_INSTRUCTIONS_DIR = '.codex'; @@ -127,11 +127,16 @@ const DEFAULT_ALLOWED_TOOLS = [ 'Read', 'Write', 'Edit', + 'MultiEdit', 'Glob', 'Grep', + 'LS', 'Bash', 'WebSearch', 'WebFetch', + 'TodoWrite', + 'Task', + 'Skill', ] as const; const SEARCH_TOOL_NAMES = new Set(['WebSearch', 'WebFetch']); const MIN_MAX_TURNS = 1; @@ -356,9 +361,14 @@ function resolveSystemPrompt(systemPrompt?: unknown): string | null { return null; } +function buildPromptText(options: ExecuteOptions): string { + return typeof options.prompt === 'string' + ? options.prompt + : extractTextFromContent(options.prompt); +} + function buildCombinedPrompt(options: ExecuteOptions, systemPromptText?: string | null): string { - const promptText = - typeof options.prompt === 'string' ? options.prompt : extractTextFromContent(options.prompt); + const promptText = buildPromptText(options); const historyText = options.conversationHistory ? formatHistoryAsText(options.conversationHistory) : ''; @@ -371,6 +381,11 @@ function buildCombinedPrompt(options: ExecuteOptions, systemPromptText?: string return `${historyText}${systemSection}${HISTORY_HEADER}${promptText}`; } +function buildResumePrompt(options: ExecuteOptions): string { + const promptText = buildPromptText(options); + return `${HISTORY_HEADER}${promptText}`; +} + function formatConfigValue(value: string | number | boolean): string { return String(value); } @@ -794,16 +809,22 @@ export class CodexProvider extends BaseProvider { } const searchEnabled = codexSettings.enableWebSearch || resolveSearchEnabled(resolvedAllowedTools, restrictTools); - const schemaPath = await writeOutputSchemaFile(options.cwd, options.outputFormat); - const imageBlocks = codexSettings.enableImages ? extractImageBlocks(options.prompt) : []; - const imagePaths = await writeImageFiles(options.cwd, imageBlocks); + const isResumeQuery = Boolean(options.sdkSessionId); + const schemaPath = isResumeQuery + ? null + : await writeOutputSchemaFile(options.cwd, options.outputFormat); + const imageBlocks = + !isResumeQuery && codexSettings.enableImages ? extractImageBlocks(options.prompt) : []; + const imagePaths = isResumeQuery ? [] : await writeImageFiles(options.cwd, imageBlocks); const approvalPolicy = hasMcpServers && options.mcpAutoApproveTools !== undefined ? options.mcpAutoApproveTools ? 'never' : 'on-request' : codexSettings.approvalPolicy; - const promptText = buildCombinedPrompt(options, combinedSystemPrompt); + const promptText = isResumeQuery + ? buildResumePrompt(options) + : buildCombinedPrompt(options, combinedSystemPrompt); const commandPath = executionPlan.cliPath || CODEX_COMMAND; // Build config overrides for max turns and reasoning effort @@ -833,21 +854,30 @@ export class CodexProvider extends BaseProvider { const preExecArgs: string[] = []; // Add additional directories with write access - if (codexSettings.additionalDirs && codexSettings.additionalDirs.length > 0) { + if ( + !isResumeQuery && + codexSettings.additionalDirs && + codexSettings.additionalDirs.length > 0 + ) { for (const dir of codexSettings.additionalDirs) { preExecArgs.push(CODEX_ADD_DIR_FLAG, dir); } } - // If images were written to disk, add the image directory so the CLI can access them + // If images were written to disk, add the image directory so the CLI can access them. + // Note: imagePaths is set to [] when isResumeQuery is true, so this check is sufficient. if (imagePaths.length > 0) { const imageDir = path.join(options.cwd, CODEX_INSTRUCTIONS_DIR, IMAGE_TEMP_DIR); preExecArgs.push(CODEX_ADD_DIR_FLAG, imageDir); } // Model is already bare (no prefix) - validated by executeQuery + const codexCommand = isResumeQuery + ? [CODEX_EXEC_SUBCOMMAND, CODEX_RESUME_SUBCOMMAND] + : [CODEX_EXEC_SUBCOMMAND]; + const args = [ - CODEX_EXEC_SUBCOMMAND, + ...codexCommand, CODEX_YOLO_FLAG, CODEX_SKIP_GIT_REPO_CHECK_FLAG, ...preExecArgs, @@ -856,6 +886,7 @@ export class CodexProvider extends BaseProvider { CODEX_JSON_FLAG, ...configOverrideArgs, ...(schemaPath ? [CODEX_OUTPUT_SCHEMA_FLAG, schemaPath] : []), + ...(options.sdkSessionId ? [options.sdkSessionId] : []), '-', // Read prompt from stdin to avoid shell escaping issues ]; diff --git a/apps/server/src/providers/copilot-provider.ts b/apps/server/src/providers/copilot-provider.ts index 34cfcbce2..5ccdfbf0d 100644 --- a/apps/server/src/providers/copilot-provider.ts +++ b/apps/server/src/providers/copilot-provider.ts @@ -30,6 +30,7 @@ import { type CopilotRuntimeModel, } from '@automaker/types'; import { createLogger, isAbortError } from '@automaker/utils'; +import { resolveModelString } from '@automaker/model-resolver'; import { CopilotClient, type PermissionRequest } from '@github/copilot-sdk'; import { normalizeTodos, @@ -116,6 +117,12 @@ export interface CopilotError extends Error { suggestion?: string; } +type CopilotSession = Awaited>; +type CopilotSessionOptions = Parameters[0]; +type ResumableCopilotClient = CopilotClient & { + resumeSession?: (sessionId: string, options: CopilotSessionOptions) => Promise; +}; + // ============================================================================= // Tool Name Normalization // ============================================================================= @@ -516,7 +523,11 @@ export class CopilotProvider extends CliProvider { } const promptText = this.extractPromptText(options); - const bareModel = options.model || DEFAULT_BARE_MODEL; + // resolveModelString may return dash-separated canonical names (e.g. "claude-sonnet-4-6"), + // but the Copilot SDK expects dot-separated version suffixes (e.g. "claude-sonnet-4.6"). + // Normalize by converting the last dash-separated numeric pair to dot notation. + const resolvedModel = resolveModelString(options.model || DEFAULT_BARE_MODEL); + const bareModel = resolvedModel.replace(/-(\d+)-(\d+)$/, '-$1.$2'); const workingDirectory = options.cwd || process.cwd(); logger.debug( @@ -554,12 +565,14 @@ export class CopilotProvider extends CliProvider { }); }; + // Declare session outside try so it's accessible in the catch block for cleanup. + let session: CopilotSession | undefined; + try { await client.start(); logger.debug(`CopilotClient started with cwd: ${workingDirectory}`); - // Create session with streaming enabled for real-time events - const session = await client.createSession({ + const sessionOptions: CopilotSessionOptions = { model: bareModel, streaming: true, // AUTONOMOUS MODE: Auto-approve all permission requests. @@ -572,13 +585,33 @@ export class CopilotProvider extends CliProvider { logger.debug(`Permission request: ${request.kind}`); return { kind: 'approved' }; }, - }); + }; + + // Resume the previous Copilot session when possible; otherwise create a fresh one. + const resumableClient = client as ResumableCopilotClient; + let sessionResumed = false; + if (options.sdkSessionId && typeof resumableClient.resumeSession === 'function') { + try { + session = await resumableClient.resumeSession(options.sdkSessionId, sessionOptions); + sessionResumed = true; + logger.debug(`Resumed Copilot session: ${session.sessionId}`); + } catch (resumeError) { + logger.warn( + `Failed to resume Copilot session "${options.sdkSessionId}", creating a new session: ${resumeError}` + ); + session = await client.createSession(sessionOptions); + } + } else { + session = await client.createSession(sessionOptions); + } - const sessionId = session.sessionId; - logger.debug(`Session created: ${sessionId}`); + // session is always assigned by this point (both branches above assign it) + const activeSession = session!; + const sessionId = activeSession.sessionId; + logger.debug(`Session ${sessionResumed ? 'resumed' : 'created'}: ${sessionId}`); // Set up event handler to push events to queue - session.on((event: SdkEvent) => { + activeSession.on((event: SdkEvent) => { logger.debug(`SDK event: ${event.type}`); if (event.type === 'session.idle') { @@ -596,7 +629,7 @@ export class CopilotProvider extends CliProvider { }); // Send the prompt (non-blocking) - await session.send({ prompt: promptText }); + await activeSession.send({ prompt: promptText }); // Process events as they arrive while (!sessionComplete || eventQueue.length > 0) { @@ -604,7 +637,7 @@ export class CopilotProvider extends CliProvider { // Check for errors first (before processing events to avoid race condition) if (sessionError) { - await session.destroy(); + await activeSession.destroy(); await client.stop(); throw sessionError; } @@ -624,11 +657,19 @@ export class CopilotProvider extends CliProvider { } // Cleanup - await session.destroy(); + await activeSession.destroy(); await client.stop(); logger.debug('CopilotClient stopped successfully'); } catch (error) { - // Ensure client is stopped on error + // Ensure session is destroyed and client is stopped on error to prevent leaks. + // The session may have been created/resumed before the error occurred. + if (session) { + try { + await session.destroy(); + } catch (sessionCleanupError) { + logger.debug(`Failed to destroy session during cleanup: ${sessionCleanupError}`); + } + } try { await client.stop(); } catch (cleanupError) { diff --git a/apps/server/src/providers/cursor-provider.ts b/apps/server/src/providers/cursor-provider.ts index 8684417a2..450b3a74a 100644 --- a/apps/server/src/providers/cursor-provider.ts +++ b/apps/server/src/providers/cursor-provider.ts @@ -450,6 +450,11 @@ export class CursorProvider extends CliProvider { cliArgs.push('--model', model); } + // Resume an existing chat when a provider session ID is available + if (options.sdkSessionId) { + cliArgs.push('--resume', options.sdkSessionId); + } + // Use '-' to indicate reading prompt from stdin cliArgs.push('-'); diff --git a/apps/server/src/providers/gemini-provider.ts b/apps/server/src/providers/gemini-provider.ts index 764c57eba..e4e6f9dcf 100644 --- a/apps/server/src/providers/gemini-provider.ts +++ b/apps/server/src/providers/gemini-provider.ts @@ -270,6 +270,11 @@ export class GeminiProvider extends CliProvider { cliArgs.push('--include-directories', options.cwd); } + // Resume an existing Gemini session when one is available + if (options.sdkSessionId) { + cliArgs.push('--resume', options.sdkSessionId); + } + // Note: Gemini CLI doesn't have a --thinking-level flag. // Thinking capabilities are determined by the model selection (e.g., gemini-2.5-pro). // The model handles thinking internally based on the task complexity. diff --git a/apps/server/src/routes/auto-mode/routes/analyze-project.ts b/apps/server/src/routes/auto-mode/routes/analyze-project.ts index 9ba22c50c..cae70c368 100644 --- a/apps/server/src/routes/auto-mode/routes/analyze-project.ts +++ b/apps/server/src/routes/auto-mode/routes/analyze-project.ts @@ -19,10 +19,11 @@ export function createAnalyzeProjectHandler(autoModeService: AutoModeServiceComp return; } - // Start analysis in background - autoModeService.analyzeProject(projectPath).catch((error) => { - logger.error(`[AutoMode] Project analysis error:`, error); - }); + // Kick off analysis in the background; attach a rejection handler so + // unhandled-promise warnings don't surface and errors are at least logged. + // Synchronous throws (e.g. "not implemented") still propagate here. + const analysisPromise = autoModeService.analyzeProject(projectPath); + analysisPromise.catch((err) => logError(err, 'Background analyzeProject failed')); res.json({ success: true, message: 'Project analysis started' }); } catch (error) { diff --git a/apps/server/src/routes/backlog-plan/common.ts b/apps/server/src/routes/backlog-plan/common.ts index a1797a3fb..27993e95e 100644 --- a/apps/server/src/routes/backlog-plan/common.ts +++ b/apps/server/src/routes/backlog-plan/common.ts @@ -114,9 +114,20 @@ export function mapBacklogPlanError(rawMessage: string): string { return 'Claude CLI could not be launched. Make sure the Claude CLI is installed and available in PATH, or check that Node.js is correctly installed. Try running "which claude" or "claude --version" in your terminal to verify.'; } - // Claude Code process crash + // Claude Code process crash - extract exit code for diagnostics if (rawMessage.includes('Claude Code process exited')) { - return 'Claude exited unexpectedly. Try again. If it keeps happening, re-run `claude login` or update your API key in Setup.'; + const exitCodeMatch = rawMessage.match(/exited with code (\d+)/); + const exitCode = exitCodeMatch ? exitCodeMatch[1] : 'unknown'; + logger.error(`[BacklogPlan] Claude process exit code: ${exitCode}`); + return `Claude exited unexpectedly (exit code: ${exitCode}). This is usually a transient issue. Try again. If it keeps happening, re-run \`claude login\` or update your API key in Setup.`; + } + + // Claude Code process killed by signal + if (rawMessage.includes('Claude Code process terminated by signal')) { + const signalMatch = rawMessage.match(/terminated by signal (\w+)/); + const signal = signalMatch ? signalMatch[1] : 'unknown'; + logger.error(`[BacklogPlan] Claude process terminated by signal: ${signal}`); + return `Claude was terminated by signal ${signal}. This may indicate a resource issue. Try again.`; } // Rate limiting diff --git a/apps/server/src/routes/backlog-plan/generate-plan.ts b/apps/server/src/routes/backlog-plan/generate-plan.ts index c2548f24c..2f47d2930 100644 --- a/apps/server/src/routes/backlog-plan/generate-plan.ts +++ b/apps/server/src/routes/backlog-plan/generate-plan.ts @@ -3,6 +3,9 @@ * * Model is configurable via phaseModels.backlogPlanningModel in settings * (defaults to Sonnet). Can be overridden per-call via model parameter. + * + * Includes automatic retry for transient CLI failures (e.g., "Claude Code + * process exited unexpectedly") to improve reliability. */ import type { EventEmitter } from '../../lib/events.js'; @@ -12,8 +15,10 @@ import { isCursorModel, stripProviderPrefix, type ThinkingLevel, + type SystemPromptPreset, } from '@automaker/types'; import { resolvePhaseModel } from '@automaker/model-resolver'; +import { getCurrentBranch } from '@automaker/git-utils'; import { FeatureLoader } from '../../services/feature-loader.js'; import { ProviderFactory } from '../../providers/provider-factory.js'; import { extractJsonWithArray } from '../../lib/json-extractor.js'; @@ -27,10 +32,27 @@ import { import type { SettingsService } from '../../services/settings-service.js'; import { getAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting, getPromptCustomization, getPhaseModelWithOverrides, } from '../../lib/settings-helpers.js'; +/** Maximum number of retry attempts for transient CLI failures */ +const MAX_RETRIES = 2; +/** Delay between retries in milliseconds */ +const RETRY_DELAY_MS = 2000; + +/** + * Check if an error is retryable (transient CLI process failure) + */ +function isRetryableError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return ( + message.includes('Claude Code process exited') || + message.includes('Claude Code process terminated by signal') + ); +} + const featureLoader = new FeatureLoader(); /** @@ -84,6 +106,53 @@ function parsePlanResponse(response: string): BacklogPlanResult { }; } +/** + * Try to parse a valid plan response without fallback behavior. + * Returns null if parsing fails. + */ +function tryParsePlanResponse(response: string): BacklogPlanResult | null { + if (!response || response.trim().length === 0) { + return null; + } + return extractJsonWithArray(response, 'changes', { logger }); +} + +/** + * Choose the most reliable response text between streamed assistant chunks + * and provider final result payload. + */ +function selectBestResponseText(accumulatedText: string, providerResultText: string): string { + const hasAccumulated = accumulatedText.trim().length > 0; + const hasProviderResult = providerResultText.trim().length > 0; + + if (!hasProviderResult) { + return accumulatedText; + } + if (!hasAccumulated) { + return providerResultText; + } + + const accumulatedParsed = tryParsePlanResponse(accumulatedText); + const providerParsed = tryParsePlanResponse(providerResultText); + + if (providerParsed && !accumulatedParsed) { + logger.info('[BacklogPlan] Using provider result (parseable JSON)'); + return providerResultText; + } + if (accumulatedParsed && !providerParsed) { + logger.info('[BacklogPlan] Keeping accumulated text (parseable JSON)'); + return accumulatedText; + } + + if (providerResultText.length > accumulatedText.length) { + logger.info('[BacklogPlan] Using provider result (longer content)'); + return providerResultText; + } + + logger.info('[BacklogPlan] Keeping accumulated text (longer content)'); + return accumulatedText; +} + /** * Generate a backlog modification plan based on user prompt */ @@ -93,11 +162,40 @@ export async function generateBacklogPlan( events: EventEmitter, abortController: AbortController, settingsService?: SettingsService, - model?: string + model?: string, + branchName?: string ): Promise { try { // Load current features - const features = await featureLoader.getAll(projectPath); + const allFeatures = await featureLoader.getAll(projectPath); + + // Filter features by branch if specified (worktree-scoped backlog) + let features: Feature[]; + if (branchName) { + // Determine the primary branch so unassigned features show for the main worktree + let primaryBranch: string | null = null; + try { + primaryBranch = await getCurrentBranch(projectPath); + } catch { + // If git fails, fall back to 'main' so unassigned features are visible + // when branchName matches a common default branch name + primaryBranch = 'main'; + } + const isMainBranch = branchName === primaryBranch; + + features = allFeatures.filter((f) => { + if (!f.branchName) { + // Unassigned features belong to the main/primary worktree + return isMainBranch; + } + return f.branchName === branchName; + }); + logger.info( + `[BacklogPlan] Filtered to ${features.length}/${allFeatures.length} features for branch: ${branchName}` + ); + } else { + features = allFeatures; + } events.emit('backlog-plan:event', { type: 'backlog_plan_progress', @@ -162,17 +260,23 @@ export async function generateBacklogPlan( // Strip provider prefix - providers expect bare model IDs const bareModel = stripProviderPrefix(effectiveModel); - // Get autoLoadClaudeMd setting + // Get autoLoadClaudeMd and useClaudeCodeSystemPrompt settings const autoLoadClaudeMd = await getAutoLoadClaudeMdSetting( projectPath, settingsService, '[BacklogPlan]' ); + const useClaudeCodeSystemPrompt = await getUseClaudeCodeSystemPromptSetting( + projectPath, + settingsService, + '[BacklogPlan]' + ); // For Cursor models, we need to combine prompts with explicit instructions // because Cursor doesn't support systemPrompt separation like Claude SDK let finalPrompt = userPrompt; - let finalSystemPrompt: string | undefined = systemPrompt; + let finalSystemPrompt: string | SystemPromptPreset | undefined = systemPrompt; + let finalSettingSources: Array<'user' | 'project' | 'local'> | undefined; if (isCursorModel(effectiveModel)) { logger.info('[BacklogPlan] Using Cursor model - adding explicit no-file-write instructions'); @@ -187,54 +291,141 @@ CRITICAL INSTRUCTIONS: ${userPrompt}`; finalSystemPrompt = undefined; // System prompt is now embedded in the user prompt + } else if (useClaudeCodeSystemPrompt) { + // Use claude_code preset for Claude models so the SDK subprocess + // authenticates via CLI OAuth or API key the same way all other SDK calls do. + finalSystemPrompt = { + type: 'preset', + preset: 'claude_code', + append: systemPrompt, + }; + } + // Include settingSources when autoLoadClaudeMd is enabled + if (autoLoadClaudeMd) { + finalSettingSources = ['user', 'project']; } - // Execute the query - const stream = provider.executeQuery({ + // Execute the query with retry logic for transient CLI failures + const queryOptions = { prompt: finalPrompt, model: bareModel, cwd: projectPath, systemPrompt: finalSystemPrompt, maxTurns: 1, - allowedTools: [], // No tools needed for this + tools: [] as string[], // Disable all built-in tools - plan generation only needs text output abortController, - settingSources: autoLoadClaudeMd ? ['user', 'project'] : undefined, - readOnly: true, // Plan generation only generates text, doesn't write files + settingSources: finalSettingSources, thinkingLevel, // Pass thinking level for extended thinking claudeCompatibleProvider, // Pass provider for alternative endpoint configuration credentials, // Pass credentials for resolving 'credentials' apiKeySource - }); + }; let responseText = ''; + let bestResponseText = ''; // Preserve best response across all retry attempts + let recoveredResult: BacklogPlanResult | null = null; + let lastError: unknown = null; - for await (const msg of stream) { + for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { if (abortController.signal.aborted) { throw new Error('Generation aborted'); } - if (msg.type === 'assistant') { - if (msg.message?.content) { - for (const block of msg.message.content) { - if (block.type === 'text') { - responseText += block.text; + if (attempt > 0) { + logger.info( + `[BacklogPlan] Retry attempt ${attempt}/${MAX_RETRIES} after transient failure` + ); + events.emit('backlog-plan:event', { + type: 'backlog_plan_progress', + content: `Retrying... (attempt ${attempt + 1}/${MAX_RETRIES + 1})`, + }); + await new Promise((resolve) => setTimeout(resolve, RETRY_DELAY_MS)); + } + + let accumulatedText = ''; + let providerResultText = ''; + + try { + const stream = provider.executeQuery(queryOptions); + + for await (const msg of stream) { + if (abortController.signal.aborted) { + throw new Error('Generation aborted'); + } + + if (msg.type === 'assistant') { + if (msg.message?.content) { + for (const block of msg.message.content) { + if (block.type === 'text') { + accumulatedText += block.text; + } + } } + } else if (msg.type === 'result' && msg.subtype === 'success' && msg.result) { + providerResultText = msg.result; + logger.info( + '[BacklogPlan] Received result from provider, length:', + providerResultText.length + ); + logger.info('[BacklogPlan] Accumulated response length:', accumulatedText.length); + } + } + + responseText = selectBestResponseText(accumulatedText, providerResultText); + + // If we got here, the stream completed successfully + lastError = null; + break; + } catch (error) { + lastError = error; + const errorMessage = error instanceof Error ? error.message : String(error); + responseText = selectBestResponseText(accumulatedText, providerResultText); + + // Preserve the best response text across all attempts so that if a retry + // crashes immediately (empty response), we can still recover from an earlier attempt + bestResponseText = selectBestResponseText(bestResponseText, responseText); + + // Claude SDK can occasionally exit non-zero after emitting a complete response. + // If we already have valid JSON, recover instead of failing the entire planning flow. + if (isRetryableError(error)) { + const parsed = tryParsePlanResponse(bestResponseText); + if (parsed) { + logger.warn( + '[BacklogPlan] Recovered from transient CLI exit using accumulated valid response' + ); + recoveredResult = parsed; + lastError = null; + break; + } + + // On final retryable failure, degrade gracefully if we have text from any attempt. + if (attempt >= MAX_RETRIES && bestResponseText.trim().length > 0) { + logger.warn( + '[BacklogPlan] Final retryable CLI failure with non-empty response, attempting fallback parse' + ); + recoveredResult = parsePlanResponse(bestResponseText); + lastError = null; + break; } } - } else if (msg.type === 'result' && msg.subtype === 'success' && msg.result) { - // Use result if it's a final accumulated message (from Cursor provider) - logger.info('[BacklogPlan] Received result from Cursor, length:', msg.result.length); - logger.info('[BacklogPlan] Previous responseText length:', responseText.length); - if (msg.result.length > responseText.length) { - logger.info('[BacklogPlan] Using Cursor result (longer than accumulated text)'); - responseText = msg.result; - } else { - logger.info('[BacklogPlan] Keeping accumulated text (longer than Cursor result)'); + + // Only retry on transient CLI failures, not on user aborts or other errors + if (!isRetryableError(error) || attempt >= MAX_RETRIES) { + throw error; } + + logger.warn( + `[BacklogPlan] Transient CLI failure (attempt ${attempt + 1}/${MAX_RETRIES + 1}): ${errorMessage}` + ); } } + // If we exhausted retries, throw the last error + if (lastError) { + throw lastError; + } + // Parse the response - const result = parsePlanResponse(responseText); + const result = recoveredResult ?? parsePlanResponse(responseText); await saveBacklogPlan(projectPath, { savedAt: new Date().toISOString(), diff --git a/apps/server/src/routes/backlog-plan/routes/generate.ts b/apps/server/src/routes/backlog-plan/routes/generate.ts index cd67d3dbf..befe96e8b 100644 --- a/apps/server/src/routes/backlog-plan/routes/generate.ts +++ b/apps/server/src/routes/backlog-plan/routes/generate.ts @@ -17,10 +17,11 @@ import type { SettingsService } from '../../../services/settings-service.js'; export function createGenerateHandler(events: EventEmitter, settingsService?: SettingsService) { return async (req: Request, res: Response): Promise => { try { - const { projectPath, prompt, model } = req.body as { + const { projectPath, prompt, model, branchName } = req.body as { projectPath: string; prompt: string; model?: string; + branchName?: string; }; if (!projectPath) { @@ -42,28 +43,30 @@ export function createGenerateHandler(events: EventEmitter, settingsService?: Se return; } - setRunningState(true); + const abortController = new AbortController(); + setRunningState(true, abortController); setRunningDetails({ projectPath, prompt, model, startedAt: new Date().toISOString(), }); - const abortController = new AbortController(); - setRunningState(true, abortController); // Start generation in background - // Note: generateBacklogPlan handles its own error event emission, - // so we only log here to avoid duplicate error toasts - generateBacklogPlan(projectPath, prompt, events, abortController, settingsService, model) - .catch((error) => { - // Just log - error event already emitted by generateBacklogPlan - logError(error, 'Generate backlog plan failed (background)'); - }) - .finally(() => { - setRunningState(false, null); - setRunningDetails(null); - }); + // Note: generateBacklogPlan handles its own error event emission + // and state cleanup in its finally block, so we only log here + generateBacklogPlan( + projectPath, + prompt, + events, + abortController, + settingsService, + model, + branchName + ).catch((error) => { + // Just log - error event already emitted by generateBacklogPlan + logError(error, 'Generate backlog plan failed (background)'); + }); res.json({ success: true }); } catch (error) { diff --git a/apps/server/src/routes/context/routes/describe-image.ts b/apps/server/src/routes/context/routes/describe-image.ts index 018a932c7..1b645d5d0 100644 --- a/apps/server/src/routes/context/routes/describe-image.ts +++ b/apps/server/src/routes/context/routes/describe-image.ts @@ -142,11 +142,33 @@ function mapDescribeImageError(rawMessage: string | undefined): { if (!rawMessage) return baseResponse; - if (rawMessage.includes('Claude Code process exited')) { + if ( + rawMessage.includes('Claude Code process exited') || + rawMessage.includes('Claude Code process terminated by signal') + ) { + const exitCodeMatch = rawMessage.match(/exited with code (\d+)/); + const signalMatch = rawMessage.match(/terminated by signal (\w+)/); + const detail = exitCodeMatch + ? ` (exit code: ${exitCodeMatch[1]})` + : signalMatch + ? ` (signal: ${signalMatch[1]})` + : ''; + + // Crash/OS-kill signals suggest a process crash, not an auth failure — + // omit auth recovery advice and suggest retry/reporting instead. + const crashSignals = ['SIGSEGV', 'SIGABRT', 'SIGKILL', 'SIGBUS', 'SIGTRAP']; + const isCrashSignal = signalMatch ? crashSignals.includes(signalMatch[1]) : false; + + if (isCrashSignal) { + return { + statusCode: 503, + userMessage: `Claude crashed unexpectedly${detail} while describing the image. This may be a transient condition. Please try again. If the problem persists, collect logs and report the issue.`, + }; + } + return { statusCode: 503, - userMessage: - 'Claude exited unexpectedly while describing the image. Try again. If it keeps happening, re-run `claude login` or update your API key in Setup so Claude can restart cleanly.', + userMessage: `Claude exited unexpectedly${detail} while describing the image. This is usually a transient issue. Try again. If it keeps happening, re-run \`claude login\` or update your API key in Setup.`, }; } diff --git a/apps/server/src/routes/features/index.ts b/apps/server/src/routes/features/index.ts index 0c92a4467..a4ea03b45 100644 --- a/apps/server/src/routes/features/index.ts +++ b/apps/server/src/routes/features/index.ts @@ -44,7 +44,11 @@ export function createFeaturesRoutes( validatePathParams('projectPath'), createCreateHandler(featureLoader, events) ); - router.post('/update', validatePathParams('projectPath'), createUpdateHandler(featureLoader)); + router.post( + '/update', + validatePathParams('projectPath'), + createUpdateHandler(featureLoader, events) + ); router.post( '/bulk-update', validatePathParams('projectPath'), diff --git a/apps/server/src/routes/features/routes/update.ts b/apps/server/src/routes/features/routes/update.ts index 4d5e7a00e..89e2dde09 100644 --- a/apps/server/src/routes/features/routes/update.ts +++ b/apps/server/src/routes/features/routes/update.ts @@ -5,6 +5,7 @@ import type { Request, Response } from 'express'; import { FeatureLoader } from '../../../services/feature-loader.js'; import type { Feature, FeatureStatus } from '@automaker/types'; +import type { EventEmitter } from '../../../lib/events.js'; import { getErrorMessage, logError } from '../common.js'; import { createLogger } from '@automaker/utils'; @@ -13,7 +14,7 @@ const logger = createLogger('features/update'); // Statuses that should trigger syncing to app_spec.txt const SYNC_TRIGGER_STATUSES: FeatureStatus[] = ['verified', 'completed']; -export function createUpdateHandler(featureLoader: FeatureLoader) { +export function createUpdateHandler(featureLoader: FeatureLoader, events?: EventEmitter) { return async (req: Request, res: Response): Promise => { try { const { @@ -54,8 +55,18 @@ export function createUpdateHandler(featureLoader: FeatureLoader) { preEnhancementDescription ); - // Trigger sync to app_spec.txt when status changes to verified or completed + // Emit completion event and sync to app_spec.txt when status transitions to verified/completed if (newStatus && SYNC_TRIGGER_STATUSES.includes(newStatus) && previousStatus !== newStatus) { + events?.emit('feature:completed', { + featureId, + featureName: updated.title, + projectPath, + passes: true, + message: + newStatus === 'verified' ? 'Feature verified manually' : 'Feature completed manually', + executionMode: 'manual', + }); + try { const synced = await featureLoader.syncFeatureToAppSpec(projectPath, updated); if (synced) { diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index a788bb483..492264cd2 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -67,6 +67,8 @@ import { createAbortOperationHandler } from './routes/abort-operation.js'; import { createContinueOperationHandler } from './routes/continue-operation.js'; import { createStageFilesHandler } from './routes/stage-files.js'; import { createCheckChangesHandler } from './routes/check-changes.js'; +import { createSetTrackingHandler } from './routes/set-tracking.js'; +import { createSyncHandler } from './routes/sync.js'; import type { SettingsService } from '../../services/settings-service.js'; export function createWorktreeRoutes( @@ -118,6 +120,18 @@ export function createWorktreeRoutes( requireValidWorktree, createPullHandler() ); + router.post( + '/sync', + validatePathParams('worktreePath'), + requireValidWorktree, + createSyncHandler() + ); + router.post( + '/set-tracking', + validatePathParams('worktreePath'), + requireValidWorktree, + createSetTrackingHandler() + ); router.post( '/checkout-branch', validatePathParams('worktreePath'), diff --git a/apps/server/src/routes/worktree/routes/delete.ts b/apps/server/src/routes/worktree/routes/delete.ts index 06703ff13..fcb42f590 100644 --- a/apps/server/src/routes/worktree/routes/delete.ts +++ b/apps/server/src/routes/worktree/routes/delete.ts @@ -5,6 +5,7 @@ import type { Request, Response } from 'express'; import { exec } from 'child_process'; import { promisify } from 'util'; +import fs from 'fs/promises'; import { isGitRepo } from '@automaker/git-utils'; import { getErrorMessage, logError, isValidBranchName } from '../common.js'; import { execGitCommand } from '../../../lib/git.js'; @@ -46,20 +47,79 @@ export function createDeleteHandler() { }); branchName = stdout.trim(); } catch { - // Could not get branch name + // Could not get branch name - worktree directory may already be gone + logger.debug('Could not determine branch for worktree, directory may be missing'); } // Remove the worktree (using array arguments to prevent injection) + let removeSucceeded = false; try { await execGitCommand(['worktree', 'remove', worktreePath, '--force'], projectPath); - } catch { - // Try with prune if remove fails - await execGitCommand(['worktree', 'prune'], projectPath); + removeSucceeded = true; + } catch (removeError) { + // `git worktree remove` can fail if the directory is already missing + // or in a bad state. Try pruning stale worktree entries as a fallback. + logger.debug('git worktree remove failed, trying prune', { + error: getErrorMessage(removeError), + }); + try { + await execGitCommand(['worktree', 'prune'], projectPath); + + // Verify the specific worktree is no longer registered after prune. + // `git worktree prune` exits 0 even if worktreePath was never registered, + // so we must explicitly check the worktree list to avoid false positives. + const { stdout: listOut } = await execAsync('git worktree list --porcelain', { + cwd: projectPath, + }); + // Parse porcelain output and check for an exact path match. + // Using substring .includes() can produce false positives when one + // worktree path is a prefix of another (e.g. /foo vs /foobar). + const stillRegistered = listOut + .split('\n') + .filter((line) => line.startsWith('worktree ')) + .map((line) => line.slice('worktree '.length).trim()) + .some((registeredPath) => registeredPath === worktreePath); + if (stillRegistered) { + // Prune didn't clean up our entry - treat as failure + throw removeError; + } + removeSucceeded = true; + } catch (pruneError) { + // If pruneError is the original removeError re-thrown, propagate it + if (pruneError === removeError) { + throw removeError; + } + logger.warn('git worktree prune also failed', { + error: getErrorMessage(pruneError), + }); + // If both remove and prune fail, still try to return success + // if the worktree directory no longer exists (it may have been + // manually deleted already). + let dirExists = false; + try { + await fs.access(worktreePath); + dirExists = true; + } catch { + // Directory doesn't exist + } + if (dirExists) { + // Directory still exists - this is a real failure + throw removeError; + } + // Directory is gone, treat as success + removeSucceeded = true; + } } - // Optionally delete the branch + // Optionally delete the branch (only if worktree was successfully removed) let branchDeleted = false; - if (deleteBranch && branchName && branchName !== 'main' && branchName !== 'master') { + if ( + removeSucceeded && + deleteBranch && + branchName && + branchName !== 'main' && + branchName !== 'master' + ) { // Validate branch name to prevent command injection if (!isValidBranchName(branchName)) { logger.warn(`Invalid branch name detected, skipping deletion: ${branchName}`); diff --git a/apps/server/src/routes/worktree/routes/discard-changes.ts b/apps/server/src/routes/worktree/routes/discard-changes.ts index 914eff677..eb2c9399d 100644 --- a/apps/server/src/routes/worktree/routes/discard-changes.ts +++ b/apps/server/src/routes/worktree/routes/discard-changes.ts @@ -5,12 +5,12 @@ * 1. Discard ALL changes (when no files array is provided) * - Resets staged changes (git reset HEAD) * - Discards modified tracked files (git checkout .) - * - Removes untracked files and directories (git clean -fd) + * - Removes untracked files and directories (git clean -ffd) * * 2. Discard SELECTED files (when files array is provided) * - Unstages selected staged files (git reset HEAD -- ) * - Reverts selected tracked file changes (git checkout -- ) - * - Removes selected untracked files (git clean -fd -- ) + * - Removes selected untracked files (git clean -ffd -- ) * * Note: Git repository validation (isGitRepo) is handled by * the requireGitRepoOnly middleware in index.ts @@ -52,6 +52,22 @@ function validateFilePath(filePath: string, worktreePath: string): boolean { } } +/** + * Parse a file path from git status --porcelain output, handling renames. + * For renamed files (R status), git reports "old_path -> new_path" and + * we need the new path to match what parseGitStatus() returns in git-utils. + */ +function parseFilePath(rawPath: string, indexStatus: string, workTreeStatus: string): string { + const trimmedPath = rawPath.trim(); + if (indexStatus === 'R' || workTreeStatus === 'R') { + const arrowIndex = trimmedPath.indexOf(' -> '); + if (arrowIndex !== -1) { + return trimmedPath.slice(arrowIndex + 4); + } + } + return trimmedPath; +} + export function createDiscardChangesHandler() { return async (req: Request, res: Response): Promise => { try { @@ -91,11 +107,16 @@ export function createDiscardChangesHandler() { // Parse the status output to categorize files // Git --porcelain format: XY PATH where X=index status, Y=worktree status - // Preserve the exact two-character XY status (no trim) to keep index vs worktree info + // For renamed files: XY OLD_PATH -> NEW_PATH const statusLines = status.trim().split('\n').filter(Boolean); const allFiles = statusLines.map((line) => { const fileStatus = line.substring(0, 2); - const filePath = line.slice(3).trim(); + const rawPath = line.slice(3); + const indexStatus = fileStatus.charAt(0); + const workTreeStatus = fileStatus.charAt(1); + // Parse path consistently with parseGitStatus() in git-utils, + // which extracts the new path for renames + const filePath = parseFilePath(rawPath, indexStatus, workTreeStatus); return { status: fileStatus, path: filePath }; }); @@ -122,8 +143,12 @@ export function createDiscardChangesHandler() { const untrackedFiles: string[] = []; // Untracked files (?) const warnings: string[] = []; + // Track which requested files were matched so we can handle unmatched ones + const matchedFiles = new Set(); + for (const file of allFiles) { if (!filesToDiscard.has(file.path)) continue; + matchedFiles.add(file.path); // file.status is the raw two-character XY git porcelain status (no trim) // X = index/staging status, Y = worktree status @@ -151,6 +176,16 @@ export function createDiscardChangesHandler() { } } + // Handle files from the UI that didn't match any entry in allFiles. + // This can happen due to timing differences between the UI loading diffs + // and the discard request, or path format differences. + // Attempt to clean unmatched files directly as untracked files. + for (const requestedFile of files) { + if (!matchedFiles.has(requestedFile)) { + untrackedFiles.push(requestedFile); + } + } + // 1. Unstage selected staged files (using execFile to bypass shell) if (stagedFiles.length > 0) { try { @@ -174,9 +209,10 @@ export function createDiscardChangesHandler() { } // 3. Remove selected untracked files + // Use -ffd (double force) to also handle nested git repositories if (untrackedFiles.length > 0) { try { - await execGitCommand(['clean', '-fd', '--', ...untrackedFiles], worktreePath); + await execGitCommand(['clean', '-ffd', '--', ...untrackedFiles], worktreePath); } catch (error) { const msg = getErrorMessage(error); logError(error, `Failed to clean untracked files: ${msg}`); @@ -234,11 +270,12 @@ export function createDiscardChangesHandler() { } // 3. Remove untracked files and directories + // Use -ffd (double force) to also handle nested git repositories try { - await execGitCommand(['clean', '-fd'], worktreePath); + await execGitCommand(['clean', '-ffd', '--'], worktreePath); } catch (error) { const msg = getErrorMessage(error); - logError(error, `git clean -fd failed: ${msg}`); + logError(error, `git clean -ffd failed: ${msg}`); warnings.push(`Failed to remove untracked files: ${msg}`); } diff --git a/apps/server/src/routes/worktree/routes/generate-pr-description.ts b/apps/server/src/routes/worktree/routes/generate-pr-description.ts index 4f42b47ea..a588f82de 100644 --- a/apps/server/src/routes/worktree/routes/generate-pr-description.ts +++ b/apps/server/src/routes/worktree/routes/generate-pr-description.ts @@ -53,7 +53,9 @@ Rules: - Focus on the user-facing impact when possible - If there are breaking changes, mention them prominently - The diff may include both committed changes and uncommitted working directory changes. Treat all changes as part of the PR since uncommitted changes will be committed when the PR is created -- Do NOT distinguish between committed and uncommitted changes in the output - describe all changes as a unified set of PR changes`; +- Do NOT distinguish between committed and uncommitted changes in the output - describe all changes as a unified set of PR changes +- EXCLUDE any files that are gitignored (e.g., node_modules, dist, build, .env files, lock files, generated files, binary artifacts, coverage reports, cache directories). These should not be mentioned in the description even if they appear in the diff +- Focus only on meaningful source code changes that are tracked by git and relevant to reviewers`; /** * Wraps an async generator with a timeout. @@ -168,127 +170,125 @@ export function createGeneratePRDescriptionHandler( // Determine the base branch for comparison const base = baseBranch || 'main'; - // Get the diff between current branch and base branch (committed changes) - // Track whether the diff method used only includes committed changes. - // `git diff base...HEAD` and `git diff origin/base...HEAD` only show committed changes, - // while the fallback methods (`git diff HEAD`, `git diff --cached + git diff`) already - // include uncommitted working directory changes. - let diff = ''; - let diffIncludesUncommitted = false; + // Collect diffs in three layers and combine them: + // 1. Committed changes on the branch: `git diff base...HEAD` + // 2. Staged (cached) changes not yet committed: `git diff --cached` + // 3. Unstaged changes to tracked files: `git diff` (no --cached flag) + // + // Untracked files are intentionally excluded — they are typically build artifacts, + // planning files, hidden dotfiles, or other files unrelated to the PR. + // `git diff` and `git diff --cached` only show changes to files already tracked by git, + // which is exactly the correct scope. + // + // We combine all three sources and deduplicate by file path so that a file modified + // in commits AND with additional uncommitted changes is not double-counted. + + /** Parse a unified diff into per-file hunks keyed by file path */ + function parseDiffIntoFileHunks(diffText: string): Map { + const fileHunks = new Map(); + if (!diffText.trim()) return fileHunks; + + // Split on "diff --git" boundaries (keep the delimiter) + const sections = diffText.split(/(?=^diff --git )/m); + for (const section of sections) { + if (!section.trim()) continue; + // Use a back-reference pattern so the "b/" side must match the "a/" capture, + // correctly handling paths that contain " b/" in their name. + // Falls back to a two-capture pattern to handle renames (a/ and b/ differ). + const backrefMatch = section.match(/^diff --git a\/(.+) b\/\1$/m); + const renameMatch = !backrefMatch ? section.match(/^diff --git a\/(.+) b\/(.+)$/m) : null; + const match = backrefMatch || renameMatch; + if (match) { + // Prefer the backref capture (identical paths); for renames use the destination (match[2]) + const filePath = backrefMatch ? match[1] : match[2]; + // Merge hunks if the same file appears in multiple diff sources + const existing = fileHunks.get(filePath) ?? ''; + fileHunks.set(filePath, existing + section); + } + } + return fileHunks; + } + + // --- Step 1: committed changes (branch vs base) --- + let committedDiff = ''; try { - // First, try to get diff against the base branch - const { stdout: branchDiff } = await execFileAsync('git', ['diff', `${base}...HEAD`], { + const { stdout } = await execFileAsync('git', ['diff', `${base}...HEAD`], { cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, // 5MB buffer + maxBuffer: 1024 * 1024 * 5, }); - diff = branchDiff; - // git diff base...HEAD only shows committed changes - diffIncludesUncommitted = false; + committedDiff = stdout; } catch { - // If branch comparison fails (e.g., base branch doesn't exist locally), - // try fetching and comparing against remote base + // Base branch may not exist locally; try the remote tracking branch try { - const { stdout: remoteDiff } = await execFileAsync( - 'git', - ['diff', `origin/${base}...HEAD`], - { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - } - ); - diff = remoteDiff; - // git diff origin/base...HEAD only shows committed changes - diffIncludesUncommitted = false; + const { stdout } = await execFileAsync('git', ['diff', `origin/${base}...HEAD`], { + cwd: worktreePath, + maxBuffer: 1024 * 1024 * 5, + }); + committedDiff = stdout; } catch { - // Fall back to getting all uncommitted + committed changes - try { - const { stdout: allDiff } = await execFileAsync('git', ['diff', 'HEAD'], { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); - diff = allDiff; - // git diff HEAD includes uncommitted changes - diffIncludesUncommitted = true; - } catch { - // Last resort: get staged + unstaged changes - const { stdout: stagedDiff } = await execFileAsync('git', ['diff', '--cached'], { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); - const { stdout: unstagedDiff } = await execFileAsync('git', ['diff'], { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); - diff = stagedDiff + unstagedDiff; - // These already include uncommitted changes - diffIncludesUncommitted = true; - } + // Cannot compare against base — leave committedDiff empty; the uncommitted + // changes gathered below will still be included. + logger.warn(`Could not get committed diff against ${base} or origin/${base}`); } } - // Check for uncommitted changes (staged + unstaged) to include in the description. - // When creating a PR, uncommitted changes will be auto-committed, so they should be - // reflected in the generated description. We only need to fetch uncommitted diffs - // when the primary diff method (base...HEAD) was used, since it only shows committed changes. - let hasUncommittedChanges = false; + // --- Step 2: staged changes (tracked files only) --- + let stagedDiff = ''; try { - const { stdout: statusOutput } = await execFileAsync('git', ['status', '--porcelain'], { + const { stdout } = await execFileAsync('git', ['diff', '--cached'], { cwd: worktreePath, + maxBuffer: 1024 * 1024 * 5, }); - hasUncommittedChanges = statusOutput.trim().length > 0; - - if (hasUncommittedChanges && !diffIncludesUncommitted) { - logger.info('Uncommitted changes detected, including in PR description context'); - - let uncommittedDiff = ''; + stagedDiff = stdout; + } catch (err) { + // Non-fatal — staged diff is a best-effort supplement + logger.debug('Failed to get staged diff', err); + } - // Get staged changes - try { - const { stdout: stagedDiff } = await execFileAsync('git', ['diff', '--cached'], { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); - if (stagedDiff.trim()) { - uncommittedDiff += stagedDiff; - } - } catch { - // Ignore staged diff errors - } + // --- Step 3: unstaged changes (tracked files only) --- + let unstagedDiff = ''; + try { + const { stdout } = await execFileAsync('git', ['diff'], { + cwd: worktreePath, + maxBuffer: 1024 * 1024 * 5, + }); + unstagedDiff = stdout; + } catch (err) { + // Non-fatal — unstaged diff is a best-effort supplement + logger.debug('Failed to get unstaged diff', err); + } - // Get unstaged changes (tracked files only) - try { - const { stdout: unstagedDiff } = await execFileAsync('git', ['diff'], { - cwd: worktreePath, - maxBuffer: 1024 * 1024 * 5, - }); - if (unstagedDiff.trim()) { - uncommittedDiff += unstagedDiff; - } - } catch { - // Ignore unstaged diff errors + // --- Combine and deduplicate --- + // Build a map of filePath → diff content by concatenating hunks from all sources + // in chronological order (committed → staged → unstaged) so that no changes + // are lost when a file appears in multiple diff sources. + const combinedFileHunks = new Map(); + + for (const source of [committedDiff, stagedDiff, unstagedDiff]) { + const hunks = parseDiffIntoFileHunks(source); + for (const [filePath, hunk] of hunks) { + if (combinedFileHunks.has(filePath)) { + combinedFileHunks.set(filePath, combinedFileHunks.get(filePath)! + hunk); + } else { + combinedFileHunks.set(filePath, hunk); } + } + } - // Get list of untracked files for context - const untrackedFiles = statusOutput - .split('\n') - .filter((line) => line.startsWith('??')) - .map((line) => line.substring(3).trim()); - - if (untrackedFiles.length > 0) { - // Add a summary of untracked (new) files as context - uncommittedDiff += `\n# New untracked files:\n${untrackedFiles.map((f) => `# + ${f}`).join('\n')}\n`; - } + const diff = Array.from(combinedFileHunks.values()).join(''); - // Append uncommitted changes to the committed diff - if (uncommittedDiff.trim()) { - diff = diff + uncommittedDiff; - } - } - } catch { - // Ignore errors checking for uncommitted changes + // Log what files were included for observability + if (combinedFileHunks.size > 0) { + logger.info(`PR description scope: ${combinedFileHunks.size} file(s)`); + logger.debug( + `PR description scope files: ${Array.from(combinedFileHunks.keys()).join(', ')}` + ); } - // Also get the commit log for context + // Also get the commit log for context — always scoped to the selected base branch + // so the log only contains commits that are part of this PR. + // We do NOT fall back to an unscoped `git log` because that would include commits + // from the base branch itself and produce misleading AI context. let commitLog = ''; try { const { stdout: logOutput } = await execFileAsync( @@ -301,11 +301,11 @@ export function createGeneratePRDescriptionHandler( ); commitLog = logOutput.trim(); } catch { - // If comparing against base fails, fall back to recent commits + // Base branch not available locally — try the remote tracking branch try { const { stdout: logOutput } = await execFileAsync( 'git', - ['log', '--oneline', '-10', '--no-decorate'], + ['log', `origin/${base}..HEAD`, '--oneline', '--no-decorate'], { cwd: worktreePath, maxBuffer: 1024 * 1024, @@ -313,7 +313,9 @@ export function createGeneratePRDescriptionHandler( ); commitLog = logOutput.trim(); } catch { - // Ignore commit log errors + // Cannot scope commit log to base branch — leave empty rather than + // including unscoped commits that would pollute the AI context. + logger.warn(`Could not get commit log against ${base} or origin/${base}`); } } @@ -339,10 +341,6 @@ export function createGeneratePRDescriptionHandler( userPrompt += `\nCommit History:\n${commitLog}\n`; } - if (hasUncommittedChanges) { - userPrompt += `\nNote: This branch has uncommitted changes that will be included in the PR.\n`; - } - if (truncatedDiff) { userPrompt += `\n\`\`\`diff\n${truncatedDiff}\n\`\`\``; } diff --git a/apps/server/src/routes/worktree/routes/list-branches.ts b/apps/server/src/routes/worktree/routes/list-branches.ts index 724a57614..ca2a33c47 100644 --- a/apps/server/src/routes/worktree/routes/list-branches.ts +++ b/apps/server/src/routes/worktree/routes/list-branches.ts @@ -9,6 +9,7 @@ import type { Request, Response } from 'express'; import { exec, execFile } from 'child_process'; import { promisify } from 'util'; import { getErrorMessage, logWorktreeError } from '../common.js'; +import { getRemotesWithBranch } from '../../../services/worktree-service.js'; const execAsync = promisify(exec); const execFileAsync = promisify(execFile); @@ -131,6 +132,8 @@ export function createListBranchesHandler() { let behindCount = 0; let hasRemoteBranch = false; let trackingRemote: string | undefined; + // List of remote names that have a branch matching the current branch name + let remotesWithBranch: string[] = []; try { // First check if there's a remote tracking branch const { stdout: upstreamOutput } = await execFileAsync( @@ -172,6 +175,12 @@ export function createListBranchesHandler() { } } + // Check which remotes have a branch matching the current branch name. + // This helps the UI distinguish between "branch exists on tracking remote" vs + // "branch was pushed to a different remote" (e.g., pushed to 'upstream' but tracking 'origin'). + // Use for-each-ref to check cached remote refs (already fetched above if includeRemote was true) + remotesWithBranch = await getRemotesWithBranch(worktreePath, currentBranch, hasAnyRemotes); + res.json({ success: true, result: { @@ -182,6 +191,7 @@ export function createListBranchesHandler() { hasRemoteBranch, hasAnyRemotes, trackingRemote, + remotesWithBranch, }, }); } catch (error) { diff --git a/apps/server/src/routes/worktree/routes/push.ts b/apps/server/src/routes/worktree/routes/push.ts index 0e082b3f0..0bf7bc3c3 100644 --- a/apps/server/src/routes/worktree/routes/push.ts +++ b/apps/server/src/routes/worktree/routes/push.ts @@ -1,24 +1,24 @@ /** * POST /push endpoint - Push a worktree branch to remote * + * Git business logic is delegated to push-service.ts. + * * Note: Git repository validation (isGitRepo, hasCommits) is handled by * the requireValidWorktree middleware in index.ts */ import type { Request, Response } from 'express'; -import { exec } from 'child_process'; -import { promisify } from 'util'; import { getErrorMessage, logError } from '../common.js'; - -const execAsync = promisify(exec); +import { performPush } from '../../../services/push-service.js'; export function createPushHandler() { return async (req: Request, res: Response): Promise => { try { - const { worktreePath, force, remote } = req.body as { + const { worktreePath, force, remote, autoResolve } = req.body as { worktreePath: string; force?: boolean; remote?: string; + autoResolve?: boolean; }; if (!worktreePath) { @@ -29,34 +29,28 @@ export function createPushHandler() { return; } - // Get branch name - const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { - cwd: worktreePath, - }); - const branchName = branchOutput.trim(); - - // Use specified remote or default to 'origin' - const targetRemote = remote || 'origin'; + const result = await performPush(worktreePath, { remote, force, autoResolve }); - // Push the branch - const forceFlag = force ? '--force' : ''; - try { - await execAsync(`git push -u ${targetRemote} ${branchName} ${forceFlag}`, { - cwd: worktreePath, - }); - } catch { - // Try setting upstream - await execAsync(`git push --set-upstream ${targetRemote} ${branchName} ${forceFlag}`, { - cwd: worktreePath, + if (!result.success) { + const statusCode = isClientError(result.error ?? '') ? 400 : 500; + res.status(statusCode).json({ + success: false, + error: result.error, + diverged: result.diverged, + hasConflicts: result.hasConflicts, + conflictFiles: result.conflictFiles, }); + return; } res.json({ success: true, result: { - branch: branchName, - pushed: true, - message: `Successfully pushed ${branchName} to ${targetRemote}`, + branch: result.branch, + pushed: result.pushed, + diverged: result.diverged, + autoResolved: result.autoResolved, + message: result.message, }, }); } catch (error) { @@ -65,3 +59,15 @@ export function createPushHandler() { } }; } + +/** + * Determine whether an error message represents a client error (400) + * vs a server error (500). + */ +function isClientError(errorMessage: string): boolean { + return ( + errorMessage.includes('detached HEAD') || + errorMessage.includes('rejected') || + errorMessage.includes('diverged') + ); +} diff --git a/apps/server/src/routes/worktree/routes/set-tracking.ts b/apps/server/src/routes/worktree/routes/set-tracking.ts new file mode 100644 index 000000000..9d63e013d --- /dev/null +++ b/apps/server/src/routes/worktree/routes/set-tracking.ts @@ -0,0 +1,76 @@ +/** + * POST /set-tracking endpoint - Set the upstream tracking branch for a worktree + * + * Sets `git branch --set-upstream-to=/` for the current branch. + * + * Note: Git repository validation (isGitRepo, hasCommits) is handled by + * the requireValidWorktree middleware in index.ts + */ + +import type { Request, Response } from 'express'; +import { execGitCommand } from '@automaker/git-utils'; +import { getErrorMessage, logError } from '../common.js'; +import { getCurrentBranch } from '../../../lib/git.js'; + +export function createSetTrackingHandler() { + return async (req: Request, res: Response): Promise => { + try { + const { worktreePath, remote, branch } = req.body as { + worktreePath: string; + remote: string; + branch?: string; + }; + + if (!worktreePath) { + res.status(400).json({ success: false, error: 'worktreePath required' }); + return; + } + + if (!remote) { + res.status(400).json({ success: false, error: 'remote required' }); + return; + } + + // Get current branch if not provided + let targetBranch = branch; + if (!targetBranch) { + try { + targetBranch = await getCurrentBranch(worktreePath); + } catch (err) { + res.status(400).json({ + success: false, + error: `Failed to get current branch: ${getErrorMessage(err)}`, + }); + return; + } + + if (targetBranch === 'HEAD') { + res.status(400).json({ + success: false, + error: 'Cannot set tracking in detached HEAD state.', + }); + return; + } + } + + // Set upstream tracking (pass local branch name as final arg to be explicit) + await execGitCommand( + ['branch', '--set-upstream-to', `${remote}/${targetBranch}`, targetBranch], + worktreePath + ); + + res.json({ + success: true, + result: { + branch: targetBranch, + remote, + upstream: `${remote}/${targetBranch}`, + message: `Set tracking branch to ${remote}/${targetBranch}`, + }, + }); + } catch (error) { + logError(error, 'Set tracking branch failed'); + res.status(500).json({ success: false, error: getErrorMessage(error) }); + } + }; +} diff --git a/apps/server/src/routes/worktree/routes/sync.ts b/apps/server/src/routes/worktree/routes/sync.ts new file mode 100644 index 000000000..acd2ec3b3 --- /dev/null +++ b/apps/server/src/routes/worktree/routes/sync.ts @@ -0,0 +1,66 @@ +/** + * POST /sync endpoint - Pull then push a worktree branch + * + * Performs a full sync operation: pull latest from remote, then push + * local commits. Handles divergence automatically. + * + * Git business logic is delegated to sync-service.ts. + * + * Note: Git repository validation (isGitRepo, hasCommits) is handled by + * the requireValidWorktree middleware in index.ts + */ + +import type { Request, Response } from 'express'; +import { getErrorMessage, logError } from '../common.js'; +import { performSync } from '../../../services/sync-service.js'; + +export function createSyncHandler() { + return async (req: Request, res: Response): Promise => { + try { + const { worktreePath, remote } = req.body as { + worktreePath: string; + remote?: string; + }; + + if (!worktreePath) { + res.status(400).json({ + success: false, + error: 'worktreePath required', + }); + return; + } + + const result = await performSync(worktreePath, { remote }); + + if (!result.success) { + const statusCode = result.hasConflicts ? 409 : 500; + res.status(statusCode).json({ + success: false, + error: result.error, + hasConflicts: result.hasConflicts, + conflictFiles: result.conflictFiles, + conflictSource: result.conflictSource, + pulled: result.pulled, + pushed: result.pushed, + }); + return; + } + + res.json({ + success: true, + result: { + branch: result.branch, + pulled: result.pulled, + pushed: result.pushed, + isFastForward: result.isFastForward, + isMerge: result.isMerge, + autoResolved: result.autoResolved, + message: result.message, + }, + }); + } catch (error) { + logError(error, 'Sync worktree failed'); + res.status(500).json({ success: false, error: getErrorMessage(error) }); + } + }; +} diff --git a/apps/server/src/services/agent-executor-types.ts b/apps/server/src/services/agent-executor-types.ts index d449a25e0..e84964a11 100644 --- a/apps/server/src/services/agent-executor-types.ts +++ b/apps/server/src/services/agent-executor-types.ts @@ -5,6 +5,7 @@ import type { PlanningMode, ThinkingLevel, + ReasoningEffort, ParsedTask, ClaudeCompatibleProvider, Credentials, @@ -24,11 +25,14 @@ export interface AgentExecutionOptions { previousContent?: string; systemPrompt?: string; autoLoadClaudeMd?: boolean; + useClaudeCodeSystemPrompt?: boolean; thinkingLevel?: ThinkingLevel; + reasoningEffort?: ReasoningEffort; branchName?: string | null; credentials?: Credentials; claudeCompatibleProvider?: ClaudeCompatibleProvider; mcpServers?: Record; + sdkSessionId?: string; sdkOptions?: { maxTurns?: number; allowedTools?: string[]; diff --git a/apps/server/src/services/agent-executor.ts b/apps/server/src/services/agent-executor.ts index 5f45c6002..a3c88f6c8 100644 --- a/apps/server/src/services/agent-executor.ts +++ b/apps/server/src/services/agent-executor.ts @@ -38,7 +38,7 @@ export type { const logger = createLogger('AgentExecutor'); -const DEFAULT_MAX_TURNS = 1000; +const DEFAULT_MAX_TURNS = 10000; export class AgentExecutor { private static readonly WRITE_DEBOUNCE_MS = 500; @@ -93,6 +93,7 @@ export class AgentExecutor { credentials, claudeCompatibleProvider, mcpServers, + sdkSessionId, sdkOptions, } = options; const { content: promptContent } = await buildPromptWithImages( @@ -127,8 +128,10 @@ export class AgentExecutor { ? (mcpServers as Record) : undefined, thinkingLevel: options.thinkingLevel, + reasoningEffort: options.reasoningEffort, credentials, claudeCompatibleProvider, + sdkSessionId, }; const featureDirForOutput = getFeatureDir(projectPath, featureId); const outputPath = path.join(featureDirForOutput, 'agent-output.md'); @@ -217,6 +220,9 @@ export class AgentExecutor { try { const stream = provider.executeQuery(executeOptions); streamLoop: for await (const msg of stream) { + if (msg.session_id && msg.session_id !== options.sdkSessionId) { + options.sdkSessionId = msg.session_id; + } receivedAnyStreamMessage = true; appendRawEvent(msg); if (abortController.signal.aborted) { @@ -385,6 +391,9 @@ export class AgentExecutor { taskCompleteDetected = false; for await (const msg of taskStream) { + if (msg.session_id && msg.session_id !== options.sdkSessionId) { + options.sdkSessionId = msg.session_id; + } if (msg.type === 'assistant' && msg.message?.content) { for (const b of msg.message.content) { if (b.type === 'text') { @@ -599,6 +608,9 @@ export class AgentExecutor { for await (const msg of provider.executeQuery( this.buildExecOpts(options, revPrompt, sdkOptions?.maxTurns ?? DEFAULT_MAX_TURNS) )) { + if (msg.session_id && msg.session_id !== options.sdkSessionId) { + options.sdkSessionId = msg.session_id; + } if (msg.type === 'assistant' && msg.message?.content) for (const b of msg.message.content) if (b.type === 'text') { @@ -692,12 +704,14 @@ export class AgentExecutor { allowedTools: o.sdkOptions?.allowedTools as string[] | undefined, abortController: o.abortController, thinkingLevel: o.thinkingLevel, + reasoningEffort: o.reasoningEffort, mcpServers: o.mcpServers && Object.keys(o.mcpServers).length > 0 ? (o.mcpServers as Record) : undefined, credentials: o.credentials, claudeCompatibleProvider: o.claudeCompatibleProvider, + sdkSessionId: o.sdkSessionId, }; } @@ -717,6 +731,9 @@ export class AgentExecutor { for await (const msg of provider.executeQuery( this.buildExecOpts(options, contPrompt, options.sdkOptions?.maxTurns ?? DEFAULT_MAX_TURNS) )) { + if (msg.session_id && msg.session_id !== options.sdkSessionId) { + options.sdkSessionId = msg.session_id; + } if (msg.type === 'assistant' && msg.message?.content) for (const b of msg.message.content) { if (b.type === 'text') { diff --git a/apps/server/src/services/agent-service.ts b/apps/server/src/services/agent-service.ts index 8d2275e54..7e7677539 100644 --- a/apps/server/src/services/agent-service.ts +++ b/apps/server/src/services/agent-service.ts @@ -21,6 +21,7 @@ import { createChatOptions, validateWorkingDirectory } from '../lib/sdk-options. import type { SettingsService } from './settings-service.js'; import { getAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting, filterClaudeMdFromContext, getMCPServersFromSettings, getPromptCustomization, @@ -329,12 +330,6 @@ export class AgentService { timestamp: new Date().toISOString(), }; - // Build conversation history from existing messages BEFORE adding current message - const conversationHistory = session.messages.map((msg) => ({ - role: msg.role, - content: msg.content, - })); - session.messages.push(userMessage); session.isRunning = true; session.abortController = new AbortController(); @@ -363,6 +358,22 @@ export class AgentService { '[AgentService]' ); + // Load useClaudeCodeSystemPrompt setting (project setting takes precedence over global) + // Wrap in try/catch so transient settingsService errors don't abort message processing + let useClaudeCodeSystemPrompt = true; + try { + useClaudeCodeSystemPrompt = await getUseClaudeCodeSystemPromptSetting( + effectiveWorkDir, + this.settingsService, + '[AgentService]' + ); + } catch (err) { + this.logger.error( + '[AgentService] getUseClaudeCodeSystemPromptSetting failed, defaulting to true', + err + ); + } + // Load MCP servers from settings (global setting only) const mcpServers = await getMCPServersFromSettings(this.settingsService, '[AgentService]'); @@ -406,6 +417,7 @@ export class AgentService { } } + let combinedSystemPrompt: string | undefined; // Load project context files (CLAUDE.md, CODE_QUALITY.md, etc.) and memory files // Use the user's message as task context for smart memory selection const contextResult = await loadContextFiles({ @@ -423,7 +435,7 @@ export class AgentService { // Build combined system prompt with base prompt and context files const baseSystemPrompt = await this.getSystemPrompt(); - const combinedSystemPrompt = contextFilesPrompt + combinedSystemPrompt = contextFilesPrompt ? `${contextFilesPrompt}\n\n${baseSystemPrompt}` : baseSystemPrompt; @@ -448,6 +460,7 @@ export class AgentService { systemPrompt: combinedSystemPrompt, abortController: session.abortController!, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, thinkingLevel: effectiveThinkingLevel, // Pass thinking level for Claude models maxTurns: userMaxTurns, // User-configured max turns from settings mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, @@ -474,7 +487,19 @@ export class AgentService { Object.keys(customSubagents).length > 0; // Base tools that match the provider's default set - const baseTools = ['Read', 'Write', 'Edit', 'Glob', 'Grep', 'Bash', 'WebSearch', 'WebFetch']; + const baseTools = [ + 'Read', + 'Write', + 'Edit', + 'MultiEdit', + 'Glob', + 'Grep', + 'LS', + 'Bash', + 'WebSearch', + 'WebFetch', + 'TodoWrite', + ]; if (allowedTools) { allowedTools = [...allowedTools]; // Create a copy to avoid mutating SDK options @@ -513,6 +538,14 @@ export class AgentService { : stripProviderPrefix(effectiveModel); // Build options for provider + const conversationHistory = session.messages + .slice(0, -1) + .map((msg) => ({ + role: msg.role, + content: msg.content, + })) + .filter((msg) => msg.content.trim().length > 0); + const options: ExecuteOptions = { prompt: '', // Will be set below based on images model: bareModel, // Bare model ID (e.g., "gpt-5.1-codex-max", "composer-1") @@ -522,7 +555,8 @@ export class AgentService { maxTurns: maxTurns, allowedTools: allowedTools, abortController: session.abortController!, - conversationHistory: conversationHistory.length > 0 ? conversationHistory : undefined, + conversationHistory: + conversationHistory && conversationHistory.length > 0 ? conversationHistory : undefined, settingSources: settingSources.length > 0 ? settingSources : undefined, sdkSessionId: session.sdkSessionId, // Pass SDK session ID for resuming mcpServers: Object.keys(mcpServers).length > 0 ? mcpServers : undefined, // Pass MCP servers configuration diff --git a/apps/server/src/services/auto-mode/facade.ts b/apps/server/src/services/auto-mode/facade.ts index c00dfdb81..af660ea57 100644 --- a/apps/server/src/services/auto-mode/facade.ts +++ b/apps/server/src/services/auto-mode/facade.ts @@ -14,7 +14,7 @@ import path from 'path'; import { exec } from 'child_process'; import { promisify } from 'util'; -import type { Feature, PlanningMode, ThinkingLevel } from '@automaker/types'; +import type { Feature, PlanningMode, ThinkingLevel, ReasoningEffort } from '@automaker/types'; import { DEFAULT_MAX_CONCURRENCY, DEFAULT_MODELS, stripProviderPrefix } from '@automaker/types'; import { resolveModelString } from '@automaker/model-resolver'; import { createLogger, loadContextFiles, classifyError } from '@automaker/utils'; @@ -213,7 +213,9 @@ export class AutoModeServiceFacade { previousContent?: string; systemPrompt?: string; autoLoadClaudeMd?: boolean; + useClaudeCodeSystemPrompt?: boolean; thinkingLevel?: ThinkingLevel; + reasoningEffort?: ReasoningEffort; branchName?: string | null; [key: string]: unknown; } @@ -244,6 +246,7 @@ export class AutoModeServiceFacade { // internal defaults which may be much lower than intended (e.g., Codex CLI's // default turn limit can cause feature runs to stop prematurely). const autoLoadClaudeMd = opts?.autoLoadClaudeMd ?? false; + const useClaudeCodeSystemPrompt = opts?.useClaudeCodeSystemPrompt ?? true; let mcpServers: Record | undefined; try { if (settingsService) { @@ -265,6 +268,7 @@ export class AutoModeServiceFacade { systemPrompt: opts?.systemPrompt, abortController, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, thinkingLevel: opts?.thinkingLevel, maxTurns: userMaxTurns, mcpServers: mcpServers as @@ -292,7 +296,9 @@ export class AutoModeServiceFacade { previousContent: opts?.previousContent as string | undefined, systemPrompt: opts?.systemPrompt as string | undefined, autoLoadClaudeMd: opts?.autoLoadClaudeMd as boolean | undefined, + useClaudeCodeSystemPrompt, thinkingLevel: opts?.thinkingLevel as ThinkingLevel | undefined, + reasoningEffort: opts?.reasoningEffort as ReasoningEffort | undefined, branchName: opts?.branchName as string | null | undefined, provider, effectiveBareModel, @@ -761,6 +767,7 @@ export class AutoModeServiceFacade { featureId, featureName: feature?.title, branchName: feature?.branchName ?? null, + executionMode: 'auto', passes: allPassed, message: allPassed ? 'All verification checks passed' @@ -823,6 +830,7 @@ export class AutoModeServiceFacade { featureId, featureName: feature?.title, branchName: feature?.branchName ?? null, + executionMode: 'auto', passes: true, message: `Changes committed: ${hash.trim().substring(0, 8)}`, projectPath: this.projectPath, @@ -910,7 +918,7 @@ export class AutoModeServiceFacade { if (feature) { title = feature.title; description = feature.description; - branchName = feature.branchName; + branchName = feature.branchName ?? undefined; } } catch { // Silently ignore @@ -1140,10 +1148,31 @@ export class AutoModeServiceFacade { // =========================================================================== /** - * Save execution state for recovery + * Save execution state for recovery. + * + * Uses the active auto-loop config for each worktree so that the persisted + * state reflects the real branch and maxConcurrency values rather than the + * hard-coded fallbacks (null / DEFAULT_MAX_CONCURRENCY). */ private async saveExecutionState(): Promise { - return this.saveExecutionStateForProject(null, DEFAULT_MAX_CONCURRENCY); + const projectWorktrees = this.autoLoopCoordinator + .getActiveWorktrees() + .filter((w) => w.projectPath === this.projectPath); + + if (projectWorktrees.length === 0) { + // No active auto loops — save with defaults as a best-effort fallback. + return this.saveExecutionStateForProject(null, DEFAULT_MAX_CONCURRENCY); + } + + // Save state for every active worktree using its real config values. + for (const { branchName } of projectWorktrees) { + const config = this.autoLoopCoordinator.getAutoLoopConfigForProject( + this.projectPath, + branchName + ); + const maxConcurrency = config?.maxConcurrency ?? DEFAULT_MAX_CONCURRENCY; + await this.saveExecutionStateForProject(branchName, maxConcurrency); + } } /** diff --git a/apps/server/src/services/auto-mode/global-service.ts b/apps/server/src/services/auto-mode/global-service.ts index 459562ebc..478f48b3f 100644 --- a/apps/server/src/services/auto-mode/global-service.ts +++ b/apps/server/src/services/auto-mode/global-service.ts @@ -159,7 +159,7 @@ export class GlobalAutoModeService { if (feature) { title = feature.title; description = feature.description; - branchName = feature.branchName; + branchName = feature.branchName ?? undefined; } } catch { // Silently ignore diff --git a/apps/server/src/services/event-hook-service.ts b/apps/server/src/services/event-hook-service.ts index ff14a993f..3d748ec24 100644 --- a/apps/server/src/services/event-hook-service.ts +++ b/apps/server/src/services/event-hook-service.ts @@ -60,10 +60,13 @@ interface AutoModeEventPayload { featureId?: string; featureName?: string; passes?: boolean; + executionMode?: 'auto' | 'manual'; message?: string; error?: string; errorType?: string; projectPath?: string; + /** Status field present when type === 'feature_status_changed' */ + status?: string; } /** @@ -75,6 +78,52 @@ interface FeatureCreatedPayload { projectPath: string; } +/** + * Feature completed event payload structure + */ +interface FeatureCompletedPayload { + featureId: string; + featureName?: string; + projectPath: string; + passes?: boolean; + message?: string; + executionMode?: 'auto' | 'manual'; +} + +/** + * Feature status changed event payload structure + */ +interface FeatureStatusChangedPayload { + featureId: string; + projectPath: string; + status: string; +} + +/** + * Type guard to safely narrow AutoModeEventPayload to FeatureStatusChangedPayload + */ +function isFeatureStatusChangedPayload( + payload: AutoModeEventPayload +): payload is AutoModeEventPayload & FeatureStatusChangedPayload { + return ( + typeof payload.featureId === 'string' && + typeof payload.projectPath === 'string' && + typeof payload.status === 'string' + ); +} + +/** + * Feature completed event payload structure + */ +interface FeatureCompletedPayload { + featureId: string; + featureName?: string; + projectPath: string; + passes?: boolean; + message?: string; + executionMode?: 'auto' | 'manual'; +} + /** * Event Hook Service * @@ -82,12 +131,30 @@ interface FeatureCreatedPayload { * Also stores events to history for debugging and replay. */ export class EventHookService { + /** Feature status that indicates agent work is done and awaiting human review (tests skipped) */ + private static readonly STATUS_WAITING_APPROVAL = 'waiting_approval'; + /** Feature status that indicates agent work passed automated verification */ + private static readonly STATUS_VERIFIED = 'verified'; + private emitter: EventEmitter | null = null; private settingsService: SettingsService | null = null; private eventHistoryService: EventHistoryService | null = null; private featureLoader: FeatureLoader | null = null; private unsubscribe: (() => void) | null = null; + /** + * Track feature IDs that have already had hooks fired via auto_mode_feature_complete + * to prevent double-firing when feature_status_changed also fires for the same feature. + * Entries are automatically cleaned up after 30 seconds. + */ + private recentlyHandledFeatures = new Set(); + + /** + * Timer IDs for pending cleanup of recentlyHandledFeatures entries, + * keyed by featureId. Stored so they can be cancelled in destroy(). + */ + private recentlyHandledTimers = new Map>(); + /** * Initialize the service with event emitter, settings service, event history service, and feature loader */ @@ -108,6 +175,8 @@ export class EventHookService { this.handleAutoModeEvent(payload as AutoModeEventPayload); } else if (type === 'feature:created') { this.handleFeatureCreatedEvent(payload as FeatureCreatedPayload); + } else if (type === 'feature:completed') { + this.handleFeatureCompletedEvent(payload as FeatureCompletedPayload); } }); @@ -122,6 +191,12 @@ export class EventHookService { this.unsubscribe(); this.unsubscribe = null; } + // Cancel all pending cleanup timers to avoid cross-session mutations + for (const timerId of this.recentlyHandledTimers.values()) { + clearTimeout(timerId); + } + this.recentlyHandledTimers.clear(); + this.recentlyHandledFeatures.clear(); this.emitter = null; this.settingsService = null; this.eventHistoryService = null; @@ -139,15 +214,31 @@ export class EventHookService { switch (payload.type) { case 'auto_mode_feature_complete': + // Only map explicit auto-mode completion events. + // Manual feature completions are emitted as feature:completed. + if (payload.executionMode !== 'auto') return; trigger = payload.passes ? 'feature_success' : 'feature_error'; + // Track this feature so feature_status_changed doesn't double-fire hooks + if (payload.featureId) { + this.markFeatureHandled(payload.featureId); + } break; case 'auto_mode_error': // Feature-level error (has featureId) vs auto-mode level error trigger = payload.featureId ? 'feature_error' : 'auto_mode_error'; + // Track this feature so feature_status_changed doesn't double-fire hooks + if (payload.featureId) { + this.markFeatureHandled(payload.featureId); + } break; case 'auto_mode_idle': trigger = 'auto_mode_complete'; break; + case 'feature_status_changed': + if (isFeatureStatusChangedPayload(payload)) { + this.handleFeatureStatusChanged(payload); + } + return; default: // Other event types don't trigger hooks return; @@ -187,6 +278,43 @@ export class EventHookService { await this.executeHooksForTrigger(trigger, context, { passes: payload.passes }); } + /** + * Handle feature:completed events and trigger matching hooks + */ + private async handleFeatureCompletedEvent(payload: FeatureCompletedPayload): Promise { + if (!payload.featureId || !payload.projectPath) return; + + const passes = payload.passes ?? true; + const trigger: EventHookTrigger = passes ? 'feature_success' : 'feature_error'; + + // Load feature name if we have featureId but no featureName + let featureName: string | undefined = undefined; + if (payload.projectPath && this.featureLoader) { + try { + const feature = await this.featureLoader.get(payload.projectPath, payload.featureId); + if (feature?.title) { + featureName = feature.title; + } + } catch (error) { + logger.warn(`Failed to load feature ${payload.featureId} for event hook:`, error); + } + } + + const isErrorTrigger = trigger === 'feature_error'; + const context: HookContext = { + featureId: payload.featureId, + featureName: featureName || payload.featureName, + projectPath: payload.projectPath, + projectName: this.extractProjectName(payload.projectPath), + error: isErrorTrigger ? payload.message : undefined, + errorType: undefined, + timestamp: new Date().toISOString(), + eventType: trigger, + }; + + await this.executeHooksForTrigger(trigger, context, { passes }); + } + /** * Handle feature:created events and trigger matching hooks */ @@ -203,6 +331,74 @@ export class EventHookService { await this.executeHooksForTrigger('feature_created', context); } + /** + * Handle feature_status_changed events for non-auto-mode feature completion. + * + * Auto-mode features already emit auto_mode_feature_complete which triggers hooks. + * This handler catches manual (non-auto-mode) feature completions by detecting + * status transitions to completion states (verified, waiting_approval). + */ + private async handleFeatureStatusChanged(payload: FeatureStatusChangedPayload): Promise { + // Skip if this feature was already handled via auto_mode_feature_complete + if (this.recentlyHandledFeatures.has(payload.featureId)) { + return; + } + + let trigger: EventHookTrigger | null = null; + + if ( + payload.status === EventHookService.STATUS_VERIFIED || + payload.status === EventHookService.STATUS_WAITING_APPROVAL + ) { + trigger = 'feature_success'; + } else { + // Only completion statuses trigger hooks from status changes + return; + } + + // Load feature name + let featureName: string | undefined = undefined; + if (this.featureLoader) { + try { + const feature = await this.featureLoader.get(payload.projectPath, payload.featureId); + if (feature?.title) { + featureName = feature.title; + } + } catch (error) { + logger.warn(`Failed to load feature ${payload.featureId} for status change hook:`, error); + } + } + + const context: HookContext = { + featureId: payload.featureId, + featureName, + projectPath: payload.projectPath, + projectName: this.extractProjectName(payload.projectPath), + timestamp: new Date().toISOString(), + eventType: trigger, + }; + + await this.executeHooksForTrigger(trigger, context, { passes: true }); + } + + /** + * Mark a feature as recently handled to prevent double-firing hooks. + * Entries are cleaned up after 30 seconds. + */ + private markFeatureHandled(featureId: string): void { + // Cancel any existing timer for this feature before setting a new one + const existing = this.recentlyHandledTimers.get(featureId); + if (existing !== undefined) { + clearTimeout(existing); + } + this.recentlyHandledFeatures.add(featureId); + const timerId = setTimeout(() => { + this.recentlyHandledFeatures.delete(featureId); + this.recentlyHandledTimers.delete(featureId); + }, 30000); + this.recentlyHandledTimers.set(featureId, timerId); + } + /** * Execute all enabled hooks matching the given trigger and store event to history */ diff --git a/apps/server/src/services/execution-service.ts b/apps/server/src/services/execution-service.ts index c98e9d895..7dc1f0bac 100644 --- a/apps/server/src/services/execution-service.ts +++ b/apps/server/src/services/execution-service.ts @@ -12,6 +12,7 @@ import * as secureFs from '../lib/secure-fs.js'; import { getPromptCustomization, getAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting, filterClaudeMdFromContext, } from '../lib/settings-helpers.js'; import { validateWorkingDirectory } from '../lib/sdk-options.js'; @@ -241,6 +242,11 @@ ${feature.spec} this.settingsService, '[ExecutionService]' ); + const useClaudeCodeSystemPrompt = await getUseClaudeCodeSystemPromptSetting( + projectPath, + this.settingsService, + '[ExecutionService]' + ); const prompts = await getPromptCustomization(this.settingsService, '[ExecutionService]'); let prompt: string; const contextResult = await this.loadContextFilesFn({ @@ -289,7 +295,9 @@ ${feature.spec} requirePlanApproval: feature.requirePlanApproval, systemPrompt: combinedSystemPrompt || undefined, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, thinkingLevel: feature.thinkingLevel, + reasoningEffort: feature.reasoningEffort, branchName: feature.branchName ?? null, } ); @@ -353,7 +361,9 @@ Please continue from where you left off and complete all remaining tasks. Use th requirePlanApproval: false, systemPrompt: combinedSystemPrompt || undefined, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, thinkingLevel: feature.thinkingLevel, + reasoningEffort: feature.reasoningEffort, branchName: feature.branchName ?? null, } ); @@ -388,6 +398,7 @@ Please continue from where you left off and complete all remaining tasks. Use th branchName: feature.branchName ?? null, abortController, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, testAttempts: 0, maxTestAttempts: 5, }); @@ -446,6 +457,7 @@ Please continue from where you left off and complete all remaining tasks. Use th featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: true, message: completionMessage, projectPath, @@ -462,6 +474,7 @@ Please continue from where you left off and complete all remaining tasks. Use th featureId, featureName: feature?.title, branchName: feature?.branchName ?? null, + executionMode: 'auto', passes: false, message: 'Feature stopped by user', projectPath, diff --git a/apps/server/src/services/execution-types.ts b/apps/server/src/services/execution-types.ts index 6cb9cb5f1..8a98b2437 100644 --- a/apps/server/src/services/execution-types.ts +++ b/apps/server/src/services/execution-types.ts @@ -5,7 +5,7 @@ * allowing the service to delegate to other services without circular dependencies. */ -import type { Feature, PlanningMode, ThinkingLevel } from '@automaker/types'; +import type { Feature, PlanningMode, ThinkingLevel, ReasoningEffort } from '@automaker/types'; import type { loadContextFiles } from '@automaker/utils'; import type { PipelineContext } from './pipeline-orchestrator.js'; @@ -31,7 +31,9 @@ export type RunAgentFn = ( previousContent?: string; systemPrompt?: string; autoLoadClaudeMd?: boolean; + useClaudeCodeSystemPrompt?: boolean; thinkingLevel?: ThinkingLevel; + reasoningEffort?: ReasoningEffort; branchName?: string | null; } ) => Promise; diff --git a/apps/server/src/services/pipeline-orchestrator.ts b/apps/server/src/services/pipeline-orchestrator.ts index 2e79b24b0..c8564b180 100644 --- a/apps/server/src/services/pipeline-orchestrator.ts +++ b/apps/server/src/services/pipeline-orchestrator.ts @@ -16,6 +16,7 @@ import * as secureFs from '../lib/secure-fs.js'; import { getPromptCustomization, getAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting, filterClaudeMdFromContext, } from '../lib/settings-helpers.js'; import { validateWorkingDirectory } from '../lib/sdk-options.js'; @@ -70,8 +71,16 @@ export class PipelineOrchestrator { ) {} async executePipeline(ctx: PipelineContext): Promise { - const { projectPath, featureId, feature, steps, workDir, abortController, autoLoadClaudeMd } = - ctx; + const { + projectPath, + featureId, + feature, + steps, + workDir, + abortController, + autoLoadClaudeMd, + useClaudeCodeSystemPrompt, + } = ctx; const prompts = await getPromptCustomization(this.settingsService, '[AutoMode]'); const contextResult = await this.loadContextFilesFn({ projectPath, @@ -121,7 +130,9 @@ export class PipelineOrchestrator { previousContent: previousContext, systemPrompt: contextFilesPrompt || undefined, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, thinkingLevel: feature.thinkingLevel, + reasoningEffort: feature.reasoningEffort, } ); try { @@ -232,6 +243,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: true, message: 'Pipeline step no longer exists', projectPath, @@ -281,6 +293,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: true, message: 'Pipeline completed (remaining steps excluded)', projectPath, @@ -306,6 +319,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: true, message: 'Pipeline completed (all steps excluded)', projectPath, @@ -354,6 +368,11 @@ export class PipelineOrchestrator { this.settingsService, '[AutoMode]' ); + const useClaudeCodeSystemPrompt = await getUseClaudeCodeSystemPromptSetting( + projectPath, + this.settingsService, + '[AutoMode]' + ); const context: PipelineContext = { projectPath, featureId, @@ -364,6 +383,7 @@ export class PipelineOrchestrator { branchName: branchName ?? null, abortController, autoLoadClaudeMd, + useClaudeCodeSystemPrompt, testAttempts: 0, maxTestAttempts: 5, }; @@ -384,6 +404,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: true, message: 'Pipeline resumed successfully', projectPath, @@ -397,6 +418,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName: feature.branchName ?? null, + executionMode: 'auto', passes: false, message: 'Pipeline stopped by user', projectPath, @@ -462,7 +484,14 @@ export class PipelineOrchestrator { projectPath, undefined, undefined, - { projectPath, planningMode: 'skip', requirePlanApproval: false } + { + projectPath, + planningMode: 'skip', + requirePlanApproval: false, + useClaudeCodeSystemPrompt: context.useClaudeCodeSystemPrompt, + autoLoadClaudeMd: context.autoLoadClaudeMd, + reasoningEffort: context.feature.reasoningEffort, + } ); } } @@ -556,6 +585,7 @@ export class PipelineOrchestrator { featureId, featureName: feature.title, branchName, + executionMode: 'auto', passes: true, message: 'Pipeline completed and merged', projectPath, @@ -596,7 +626,7 @@ export class PipelineOrchestrator { } // Only capture assertion details when they appear in failure context // or match explicit assertion error / expect patterns - if (trimmed.includes('AssertionError') || trimmed.includes('AssertionError')) { + if (trimmed.includes('AssertionError')) { failedTests.push(trimmed); } else if ( inFailureContext && diff --git a/apps/server/src/services/pipeline-types.ts b/apps/server/src/services/pipeline-types.ts index be41f3317..67957b6a7 100644 --- a/apps/server/src/services/pipeline-types.ts +++ b/apps/server/src/services/pipeline-types.ts @@ -14,6 +14,7 @@ export interface PipelineContext { branchName: string | null; abortController: AbortController; autoLoadClaudeMd: boolean; + useClaudeCodeSystemPrompt?: boolean; testAttempts: number; maxTestAttempts: number; } diff --git a/apps/server/src/services/push-service.ts b/apps/server/src/services/push-service.ts new file mode 100644 index 000000000..f1619f5b3 --- /dev/null +++ b/apps/server/src/services/push-service.ts @@ -0,0 +1,258 @@ +/** + * PushService - Push git operations without HTTP + * + * Encapsulates the full git push workflow including: + * - Branch name and detached HEAD detection + * - Safe array-based command execution (no shell interpolation) + * - Divergent branch detection and auto-resolution via pull-then-retry + * - Structured result reporting + * + * Mirrors the pull-service.ts pattern for consistency. + */ + +import { createLogger, getErrorMessage } from '@automaker/utils'; +import { execGitCommand } from '@automaker/git-utils'; +import { getCurrentBranch } from '../lib/git.js'; +import { performPull } from './pull-service.js'; + +const logger = createLogger('PushService'); + +// ============================================================================ +// Types +// ============================================================================ + +export interface PushOptions { + /** Remote name to push to (defaults to 'origin') */ + remote?: string; + /** Force push */ + force?: boolean; + /** When true and push is rejected due to divergence, pull then retry push */ + autoResolve?: boolean; +} + +export interface PushResult { + success: boolean; + error?: string; + branch?: string; + pushed?: boolean; + /** Whether the push was initially rejected because the branches diverged */ + diverged?: boolean; + /** Whether divergence was automatically resolved via pull-then-retry */ + autoResolved?: boolean; + /** Whether the auto-resolve pull resulted in merge conflicts */ + hasConflicts?: boolean; + /** Files with merge conflicts (only when hasConflicts is true) */ + conflictFiles?: string[]; + message?: string; +} + +// ============================================================================ +// Helper Functions +// ============================================================================ + +/** + * Detect whether push error output indicates a diverged/non-fast-forward rejection. + */ +function isDivergenceError(errorOutput: string): boolean { + const lower = errorOutput.toLowerCase(); + // Require specific divergence indicators rather than just 'rejected' alone, + // which could match pre-receive hook rejections or protected branch errors. + const hasNonFastForward = lower.includes('non-fast-forward'); + const hasFetchFirst = lower.includes('fetch first'); + const hasFailedToPush = lower.includes('failed to push some refs'); + const hasRejected = lower.includes('rejected'); + return hasNonFastForward || hasFetchFirst || (hasRejected && hasFailedToPush); +} + +// ============================================================================ +// Main Service Function +// ============================================================================ + +/** + * Perform a git push on the given worktree. + * + * The workflow: + * 1. Get current branch name (detect detached HEAD) + * 2. Attempt `git push ` with safe array args + * 3. If push fails with divergence and autoResolve is true: + * a. Pull from the same remote (with stash support) + * b. If pull succeeds without conflicts, retry push + * 4. If push fails with "no upstream" error, retry with --set-upstream + * 5. Return structured result + * + * @param worktreePath - Path to the git worktree + * @param options - Push options (remote, force, autoResolve) + * @returns PushResult with detailed status information + */ +export async function performPush( + worktreePath: string, + options?: PushOptions +): Promise { + const targetRemote = options?.remote || 'origin'; + const force = options?.force ?? false; + const autoResolve = options?.autoResolve ?? false; + + // 1. Get current branch name + let branchName: string; + try { + branchName = await getCurrentBranch(worktreePath); + } catch (err) { + return { + success: false, + error: `Failed to get current branch: ${getErrorMessage(err)}`, + }; + } + + // 2. Check for detached HEAD state + if (branchName === 'HEAD') { + return { + success: false, + error: 'Cannot push in detached HEAD state. Please checkout a branch first.', + }; + } + + // 3. Build push args (no -u flag; upstream is set in the fallback path only when needed) + const pushArgs = ['push', targetRemote, branchName]; + if (force) { + pushArgs.push('--force'); + } + + // 4. Attempt push + try { + await execGitCommand(pushArgs, worktreePath); + + return { + success: true, + branch: branchName, + pushed: true, + message: `Successfully pushed ${branchName} to ${targetRemote}`, + }; + } catch (pushError: unknown) { + const err = pushError as { stderr?: string; stdout?: string; message?: string }; + const errorOutput = `${err.stderr || ''} ${err.stdout || ''} ${err.message || ''}`; + + // 5. Check if the error is a divergence rejection + if (isDivergenceError(errorOutput)) { + if (!autoResolve) { + return { + success: false, + branch: branchName, + pushed: false, + diverged: true, + error: `Push rejected: remote has changes not present locally. Use sync or pull first, or enable auto-resolve.`, + message: `Push to ${targetRemote} was rejected because the remote branch has diverged.`, + }; + } + + // 6. Auto-resolve: pull then retry push + logger.info('Push rejected due to divergence, attempting auto-resolve via pull', { + worktreePath, + remote: targetRemote, + branch: branchName, + }); + + try { + const pullResult = await performPull(worktreePath, { + remote: targetRemote, + stashIfNeeded: true, + }); + + if (!pullResult.success) { + return { + success: false, + branch: branchName, + pushed: false, + diverged: true, + autoResolved: false, + error: `Auto-resolve failed during pull: ${pullResult.error}`, + }; + } + + if (pullResult.hasConflicts) { + return { + success: false, + branch: branchName, + pushed: false, + diverged: true, + autoResolved: false, + hasConflicts: true, + conflictFiles: pullResult.conflictFiles, + error: + 'Auto-resolve pull resulted in merge conflicts. Resolve conflicts and push again.', + }; + } + + // 7. Retry push after successful pull + try { + await execGitCommand(pushArgs, worktreePath); + + return { + success: true, + branch: branchName, + pushed: true, + diverged: true, + autoResolved: true, + message: `Push succeeded after auto-resolving divergence (pulled from ${targetRemote} first).`, + }; + } catch (retryError: unknown) { + const retryErr = retryError as { stderr?: string; message?: string }; + return { + success: false, + branch: branchName, + pushed: false, + diverged: true, + autoResolved: false, + error: `Push failed after auto-resolve pull: ${retryErr.stderr || retryErr.message || 'Unknown error'}`, + }; + } + } catch (pullError) { + return { + success: false, + branch: branchName, + pushed: false, + diverged: true, + autoResolved: false, + error: `Auto-resolve pull failed: ${getErrorMessage(pullError)}`, + }; + } + } + + // 6b. Non-divergence error (e.g. no upstream configured) - retry with --set-upstream + const isNoUpstreamError = + errorOutput.toLowerCase().includes('no upstream') || + errorOutput.toLowerCase().includes('has no upstream branch') || + errorOutput.toLowerCase().includes('set-upstream'); + if (isNoUpstreamError) { + try { + const setUpstreamArgs = ['push', '--set-upstream', targetRemote, branchName]; + if (force) { + setUpstreamArgs.push('--force'); + } + await execGitCommand(setUpstreamArgs, worktreePath); + + return { + success: true, + branch: branchName, + pushed: true, + message: `Successfully pushed ${branchName} to ${targetRemote} (set upstream)`, + }; + } catch (upstreamError: unknown) { + const upstreamErr = upstreamError as { stderr?: string; message?: string }; + return { + success: false, + branch: branchName, + pushed: false, + error: upstreamErr.stderr || upstreamErr.message || getErrorMessage(pushError), + }; + } + } + + // 6c. Other push error - return as-is + return { + success: false, + branch: branchName, + pushed: false, + error: err.stderr || err.message || getErrorMessage(pushError), + }; + } +} diff --git a/apps/server/src/services/settings-service.ts b/apps/server/src/services/settings-service.ts index 6a3d804eb..7b3ffa707 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -31,6 +31,7 @@ import type { WorktreeInfo, PhaseModelConfig, PhaseModelEntry, + FeatureTemplate, ClaudeApiProfile, ClaudeCompatibleProvider, ProviderModel, @@ -40,6 +41,7 @@ import { DEFAULT_CREDENTIALS, DEFAULT_PROJECT_SETTINGS, DEFAULT_PHASE_MODELS, + DEFAULT_FEATURE_TEMPLATES, SETTINGS_VERSION, CREDENTIALS_VERSION, PROJECT_SETTINGS_VERSION, @@ -139,6 +141,11 @@ export class SettingsService { // Migrate model IDs to canonical format const migratedModelSettings = this.migrateModelSettings(settings); + // Merge built-in feature templates: ensure all built-in templates exist in user settings. + // User customizations (enabled/disabled state, order overrides) are preserved. + // New built-in templates added in code updates are injected for existing users. + const mergedFeatureTemplates = this.mergeBuiltInTemplates(settings.featureTemplates); + // Apply any missing defaults (for backwards compatibility) let result: GlobalSettings = { ...DEFAULT_GLOBAL_SETTINGS, @@ -149,6 +156,7 @@ export class SettingsService { ...settings.keyboardShortcuts, }, phaseModels: migratedPhaseModels, + featureTemplates: mergedFeatureTemplates, }; // Version-based migrations @@ -250,6 +258,32 @@ export class SettingsService { return result; } + /** + * Merge built-in feature templates with user's stored templates. + * + * Ensures new built-in templates added in code updates are available to existing users + * without overwriting their customizations (e.g., enabled/disabled state, custom order). + * Built-in templates missing from stored settings are appended with their defaults. + * + * @param storedTemplates - Templates from user's settings file (may be undefined for new installs) + * @returns Merged template list with all built-in templates present + */ + private mergeBuiltInTemplates(storedTemplates: FeatureTemplate[] | undefined): FeatureTemplate[] { + if (!storedTemplates) { + return DEFAULT_FEATURE_TEMPLATES; + } + + const storedIds = new Set(storedTemplates.map((t) => t.id)); + const missingBuiltIns = DEFAULT_FEATURE_TEMPLATES.filter((t) => !storedIds.has(t.id)); + + if (missingBuiltIns.length === 0) { + return storedTemplates; + } + + // Append missing built-in templates after existing ones + return [...storedTemplates, ...missingBuiltIns]; + } + /** * Migrate legacy enhancementModel/validationModel fields to phaseModels structure * @@ -573,6 +607,17 @@ export class SettingsService { ignoreEmptyArrayOverwrite('claudeApiProfiles'); // Note: claudeCompatibleProviders intentionally NOT guarded - users should be able to delete all providers + // Check for explicit permission to clear eventHooks (escape hatch for intentional clearing) + const allowEmptyEventHooks = + (sanitizedUpdates as Record).__allowEmptyEventHooks === true; + // Remove the flag so it doesn't get persisted + delete (sanitizedUpdates as Record).__allowEmptyEventHooks; + + // Only guard eventHooks if explicit permission wasn't granted + if (!allowEmptyEventHooks) { + ignoreEmptyArrayOverwrite('eventHooks'); + } + // Empty object overwrite guard const ignoreEmptyObjectOverwrite = (key: K): void => { const nextVal = sanitizedUpdates[key] as unknown; diff --git a/apps/server/src/services/sync-service.ts b/apps/server/src/services/sync-service.ts new file mode 100644 index 000000000..f47055c92 --- /dev/null +++ b/apps/server/src/services/sync-service.ts @@ -0,0 +1,209 @@ +/** + * SyncService - Pull then push in a single operation + * + * Composes performPull() and performPush() to synchronize a branch + * with its remote. Always uses stashIfNeeded for the pull step. + * If push fails with divergence after pull, retries once. + * + * Follows the same pattern as pull-service.ts and push-service.ts. + */ + +import { createLogger, getErrorMessage } from '@automaker/utils'; +import { performPull } from './pull-service.js'; +import { performPush } from './push-service.js'; +import type { PullResult } from './pull-service.js'; +import type { PushResult } from './push-service.js'; + +const logger = createLogger('SyncService'); + +// ============================================================================ +// Types +// ============================================================================ + +export interface SyncOptions { + /** Remote name (defaults to 'origin') */ + remote?: string; +} + +export interface SyncResult { + success: boolean; + error?: string; + branch?: string; + /** Whether the pull step was performed */ + pulled?: boolean; + /** Whether the push step was performed */ + pushed?: boolean; + /** Pull resulted in conflicts */ + hasConflicts?: boolean; + /** Files with merge conflicts */ + conflictFiles?: string[]; + /** Source of conflicts ('pull' | 'stash') */ + conflictSource?: 'pull' | 'stash'; + /** Whether the pull was a fast-forward */ + isFastForward?: boolean; + /** Whether the pull resulted in a merge commit */ + isMerge?: boolean; + /** Whether push divergence was auto-resolved */ + autoResolved?: boolean; + message?: string; +} + +// ============================================================================ +// Main Service Function +// ============================================================================ + +/** + * Perform a sync operation (pull then push) on the given worktree. + * + * The workflow: + * 1. Pull from remote with stashIfNeeded: true + * 2. If pull has conflicts, stop and return conflict info + * 3. Push to remote + * 4. If push fails with divergence after pull, retry once + * + * @param worktreePath - Path to the git worktree + * @param options - Sync options (remote) + * @returns SyncResult with detailed status information + */ +export async function performSync( + worktreePath: string, + options?: SyncOptions +): Promise { + const targetRemote = options?.remote || 'origin'; + + // 1. Pull from remote + logger.info('Sync: starting pull', { worktreePath, remote: targetRemote }); + + let pullResult: PullResult; + try { + pullResult = await performPull(worktreePath, { + remote: targetRemote, + stashIfNeeded: true, + }); + } catch (pullError) { + return { + success: false, + error: `Sync pull failed: ${getErrorMessage(pullError)}`, + }; + } + + if (!pullResult.success) { + return { + success: false, + branch: pullResult.branch, + pulled: false, + pushed: false, + error: `Sync pull failed: ${pullResult.error}`, + hasConflicts: pullResult.hasConflicts, + conflictFiles: pullResult.conflictFiles, + conflictSource: pullResult.conflictSource, + }; + } + + // 2. If pull had conflicts, stop and return conflict info + if (pullResult.hasConflicts) { + return { + success: false, + branch: pullResult.branch, + pulled: true, + pushed: false, + hasConflicts: true, + conflictFiles: pullResult.conflictFiles, + conflictSource: pullResult.conflictSource, + isFastForward: pullResult.isFastForward, + isMerge: pullResult.isMerge, + error: 'Sync stopped: pull resulted in merge conflicts. Resolve conflicts and try again.', + message: pullResult.message, + }; + } + + // 3. Push to remote + logger.info('Sync: pull succeeded, starting push', { worktreePath, remote: targetRemote }); + + let pushResult: PushResult; + try { + pushResult = await performPush(worktreePath, { + remote: targetRemote, + }); + } catch (pushError) { + return { + success: false, + branch: pullResult.branch, + pulled: true, + pushed: false, + isFastForward: pullResult.isFastForward, + isMerge: pullResult.isMerge, + error: `Sync push failed: ${getErrorMessage(pushError)}`, + }; + } + + if (!pushResult.success) { + // 4. If push diverged after pull, retry once with autoResolve + if (pushResult.diverged) { + logger.info('Sync: push diverged after pull, retrying with autoResolve', { + worktreePath, + remote: targetRemote, + }); + + try { + const retryResult = await performPush(worktreePath, { + remote: targetRemote, + autoResolve: true, + }); + + if (retryResult.success) { + return { + success: true, + branch: retryResult.branch, + pulled: true, + pushed: true, + autoResolved: true, + isFastForward: pullResult.isFastForward, + isMerge: pullResult.isMerge, + message: 'Sync completed (push required auto-resolve).', + }; + } + + return { + success: false, + branch: retryResult.branch, + pulled: true, + pushed: false, + hasConflicts: retryResult.hasConflicts, + conflictFiles: retryResult.conflictFiles, + error: retryResult.error, + }; + } catch (retryError) { + return { + success: false, + branch: pullResult.branch, + pulled: true, + pushed: false, + error: `Sync push retry failed: ${getErrorMessage(retryError)}`, + }; + } + } + + return { + success: false, + branch: pushResult.branch, + pulled: true, + pushed: false, + isFastForward: pullResult.isFastForward, + isMerge: pullResult.isMerge, + error: `Sync push failed: ${pushResult.error}`, + }; + } + + return { + success: true, + branch: pushResult.branch, + pulled: pullResult.pulled ?? true, + pushed: true, + isFastForward: pullResult.isFastForward, + isMerge: pullResult.isMerge, + message: pullResult.pulled + ? 'Sync completed: pulled latest changes and pushed.' + : 'Sync completed: already up to date, pushed local commits.', + }; +} diff --git a/apps/server/src/services/worktree-service.ts b/apps/server/src/services/worktree-service.ts index 8490c1082..0cb7a2516 100644 --- a/apps/server/src/services/worktree-service.ts +++ b/apps/server/src/services/worktree-service.ts @@ -8,9 +8,60 @@ import path from 'path'; import fs from 'fs/promises'; +import { execGitCommand } from '@automaker/git-utils'; import type { EventEmitter } from '../lib/events.js'; import type { SettingsService } from './settings-service.js'; +/** + * Get the list of remote names that have a branch matching the given branch name. + * + * Uses `git for-each-ref` to check cached remote refs, returning the names of + * any remotes that already have a branch with the same name as `currentBranch`. + * Returns an empty array when `hasAnyRemotes` is false or when no matching + * remote refs are found. + * + * This helps the UI distinguish between "branch exists on the tracking remote" + * vs "branch was pushed to a different remote". + * + * @param worktreePath - Path to the git worktree + * @param currentBranch - Branch name to search for on remotes + * @param hasAnyRemotes - Whether the repository has any remotes configured + * @returns Array of remote names (e.g. ["origin", "upstream"]) that contain the branch + */ +export async function getRemotesWithBranch( + worktreePath: string, + currentBranch: string, + hasAnyRemotes: boolean +): Promise { + if (!hasAnyRemotes) { + return []; + } + + try { + const remoteRefsOutput = await execGitCommand( + ['for-each-ref', '--format=%(refname:short)', `refs/remotes/*/${currentBranch}`], + worktreePath + ); + + if (!remoteRefsOutput.trim()) { + return []; + } + + return remoteRefsOutput + .trim() + .split('\n') + .map((ref) => { + // Extract remote name from "remote/branch" format + const slashIdx = ref.indexOf('/'); + return slashIdx !== -1 ? ref.slice(0, slashIdx) : ref; + }) + .filter((name) => name.length > 0); + } catch { + // Ignore errors - return empty array + return []; + } +} + /** * Error thrown when one or more file copy operations fail during * `copyConfiguredFiles`. The caller can inspect `failures` for details. diff --git a/apps/server/src/types/settings.ts b/apps/server/src/types/settings.ts index 6863b314a..30ce17220 100644 --- a/apps/server/src/types/settings.ts +++ b/apps/server/src/types/settings.ts @@ -23,6 +23,7 @@ export type { PhaseModelConfig, PhaseModelKey, PhaseModelEntry, + FeatureTemplate, // Claude-compatible provider types ApiKeySource, ClaudeCompatibleProviderType, @@ -41,6 +42,7 @@ export { DEFAULT_CREDENTIALS, DEFAULT_PROJECT_SETTINGS, DEFAULT_PHASE_MODELS, + DEFAULT_FEATURE_TEMPLATES, SETTINGS_VERSION, CREDENTIALS_VERSION, PROJECT_SETTINGS_VERSION, diff --git a/apps/server/tests/unit/lib/enhancement-prompts.test.ts b/apps/server/tests/unit/lib/enhancement-prompts.test.ts index 13d615550..77d118d3e 100644 --- a/apps/server/tests/unit/lib/enhancement-prompts.test.ts +++ b/apps/server/tests/unit/lib/enhancement-prompts.test.ts @@ -168,7 +168,7 @@ describe('enhancement-prompts.ts', () => { const prompt = buildUserPrompt('improve', testText); expect(prompt).toContain('Example 1:'); expect(prompt).toContain(testText); - expect(prompt).toContain('Now, please enhance the following task description:'); + expect(prompt).toContain('Please enhance the following task description:'); }); it('should build prompt without examples when includeExamples is false', () => { diff --git a/apps/server/tests/unit/providers/codex-provider.test.ts b/apps/server/tests/unit/providers/codex-provider.test.ts index 03cd5591d..1e150ee16 100644 --- a/apps/server/tests/unit/providers/codex-provider.test.ts +++ b/apps/server/tests/unit/providers/codex-provider.test.ts @@ -170,6 +170,30 @@ describe('codex-provider.ts', () => { expect(call.args).toContain('--json'); }); + it('uses exec resume when sdkSessionId is provided', async () => { + vi.mocked(spawnJSONLProcess).mockReturnValue((async function* () {})()); + + await collectAsyncGenerator( + provider.executeQuery({ + prompt: 'Continue', + model: 'gpt-5.2', + cwd: '/tmp', + sdkSessionId: 'codex-session-123', + outputFormat: { type: 'json_schema', schema: { type: 'object', properties: {} } }, + codexSettings: { additionalDirs: ['/extra/dir'] }, + }) + ); + + const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; + expect(call.args[0]).toBe('exec'); + expect(call.args[1]).toBe('resume'); + expect(call.args).toContain('codex-session-123'); + expect(call.args).toContain('--json'); + // Resume queries must not include --output-schema or --add-dir + expect(call.args).not.toContain('--output-schema'); + expect(call.args).not.toContain('--add-dir'); + }); + it('overrides approval policy when MCP auto-approval is enabled', async () => { // Note: With full-permissions always on (--dangerously-bypass-approvals-and-sandbox), // approval policy is bypassed, not configured via --config @@ -320,8 +344,10 @@ describe('codex-provider.ts', () => { ); const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; - // High reasoning effort should have 3x the default timeout (90000ms) - expect(call.timeout).toBe(DEFAULT_TIMEOUT_MS * REASONING_TIMEOUT_MULTIPLIERS.high); + // High reasoning effort should have 3x the CLI base timeout (120000ms) + // CODEX_CLI_TIMEOUT_MS = 120000, multiplier for 'high' = 3.0 → 360000ms + const CODEX_CLI_TIMEOUT_MS = 120000; + expect(call.timeout).toBe(CODEX_CLI_TIMEOUT_MS * REASONING_TIMEOUT_MULTIPLIERS.high); }); it('passes extended timeout for xhigh reasoning effort', async () => { @@ -357,8 +383,10 @@ describe('codex-provider.ts', () => { ); const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; - // No reasoning effort should use the default timeout - expect(call.timeout).toBe(DEFAULT_TIMEOUT_MS); + // No reasoning effort should use the CLI base timeout (2 minutes) + // CODEX_CLI_TIMEOUT_MS = 120000ms, no multiplier applied + const CODEX_CLI_TIMEOUT_MS = 120000; + expect(call.timeout).toBe(CODEX_CLI_TIMEOUT_MS); }); }); diff --git a/apps/server/tests/unit/providers/copilot-provider.test.ts b/apps/server/tests/unit/providers/copilot-provider.test.ts index ccd7ae28a..552e75304 100644 --- a/apps/server/tests/unit/providers/copilot-provider.test.ts +++ b/apps/server/tests/unit/providers/copilot-provider.test.ts @@ -1,17 +1,35 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { CopilotProvider, CopilotErrorCode } from '@/providers/copilot-provider.js'; +import { collectAsyncGenerator } from '../../utils/helpers.js'; +import { CopilotClient } from '@github/copilot-sdk'; + +const createSessionMock = vi.fn(); +const resumeSessionMock = vi.fn(); + +function createMockSession(sessionId = 'test-session') { + let eventHandler: ((event: any) => void) | null = null; + return { + sessionId, + send: vi.fn().mockImplementation(async () => { + if (eventHandler) { + eventHandler({ type: 'assistant.message', data: { content: 'hello' } }); + eventHandler({ type: 'session.idle' }); + } + }), + destroy: vi.fn().mockResolvedValue(undefined), + on: vi.fn().mockImplementation((handler: (event: any) => void) => { + eventHandler = handler; + }), + }; +} // Mock the Copilot SDK vi.mock('@github/copilot-sdk', () => ({ CopilotClient: vi.fn().mockImplementation(() => ({ start: vi.fn().mockResolvedValue(undefined), stop: vi.fn().mockResolvedValue(undefined), - createSession: vi.fn().mockResolvedValue({ - sessionId: 'test-session', - send: vi.fn().mockResolvedValue(undefined), - destroy: vi.fn().mockResolvedValue(undefined), - on: vi.fn(), - }), + createSession: createSessionMock, + resumeSession: resumeSessionMock, })), })); @@ -49,6 +67,16 @@ describe('copilot-provider.ts', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(CopilotClient).mockImplementation(function () { + return { + start: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + createSession: createSessionMock, + resumeSession: resumeSessionMock, + } as any; + }); + createSessionMock.mockResolvedValue(createMockSession()); + resumeSessionMock.mockResolvedValue(createMockSession('resumed-session')); // Mock fs.existsSync for CLI path validation vi.mocked(fs.existsSync).mockReturnValue(true); @@ -514,4 +542,45 @@ describe('copilot-provider.ts', () => { expect(todoInput.todos[0].status).toBe('completed'); }); }); + + describe('executeQuery resume behavior', () => { + it('uses resumeSession when sdkSessionId is provided', async () => { + const results = await collectAsyncGenerator( + provider.executeQuery({ + prompt: 'Hello', + model: 'claude-sonnet-4.6', + cwd: '/tmp/project', + sdkSessionId: 'session-123', + }) + ); + + expect(resumeSessionMock).toHaveBeenCalledWith( + 'session-123', + expect.objectContaining({ model: 'claude-sonnet-4.6', streaming: true }) + ); + expect(createSessionMock).not.toHaveBeenCalled(); + expect(results.some((msg) => msg.session_id === 'resumed-session')).toBe(true); + }); + + it('falls back to createSession when resumeSession fails', async () => { + resumeSessionMock.mockRejectedValueOnce(new Error('session not found')); + createSessionMock.mockResolvedValueOnce(createMockSession('fresh-session')); + + const results = await collectAsyncGenerator( + provider.executeQuery({ + prompt: 'Hello', + model: 'claude-sonnet-4.6', + cwd: '/tmp/project', + sdkSessionId: 'stale-session', + }) + ); + + expect(resumeSessionMock).toHaveBeenCalledWith( + 'stale-session', + expect.objectContaining({ model: 'claude-sonnet-4.6', streaming: true }) + ); + expect(createSessionMock).toHaveBeenCalledTimes(1); + expect(results.some((msg) => msg.session_id === 'fresh-session')).toBe(true); + }); + }); }); diff --git a/apps/server/tests/unit/providers/cursor-provider.test.ts b/apps/server/tests/unit/providers/cursor-provider.test.ts new file mode 100644 index 000000000..9eff5d306 --- /dev/null +++ b/apps/server/tests/unit/providers/cursor-provider.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from 'vitest'; +import { CursorProvider } from '@/providers/cursor-provider.js'; + +describe('cursor-provider.ts', () => { + describe('buildCliArgs', () => { + it('adds --resume when sdkSessionId is provided', () => { + const provider = Object.create(CursorProvider.prototype) as CursorProvider & { + cliPath?: string; + }; + provider.cliPath = '/usr/local/bin/cursor-agent'; + + const args = provider.buildCliArgs({ + prompt: 'Continue the task', + model: 'gpt-5', + cwd: '/tmp/project', + sdkSessionId: 'cursor-session-123', + }); + + const resumeIndex = args.indexOf('--resume'); + expect(resumeIndex).toBeGreaterThan(-1); + expect(args[resumeIndex + 1]).toBe('cursor-session-123'); + }); + + it('does not add --resume when sdkSessionId is omitted', () => { + const provider = Object.create(CursorProvider.prototype) as CursorProvider & { + cliPath?: string; + }; + provider.cliPath = '/usr/local/bin/cursor-agent'; + + const args = provider.buildCliArgs({ + prompt: 'Start a new task', + model: 'gpt-5', + cwd: '/tmp/project', + }); + + expect(args).not.toContain('--resume'); + }); + }); +}); diff --git a/apps/server/tests/unit/providers/gemini-provider.test.ts b/apps/server/tests/unit/providers/gemini-provider.test.ts new file mode 100644 index 000000000..a2d410d45 --- /dev/null +++ b/apps/server/tests/unit/providers/gemini-provider.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { GeminiProvider } from '@/providers/gemini-provider.js'; + +describe('gemini-provider.ts', () => { + let provider: GeminiProvider; + + beforeEach(() => { + provider = new GeminiProvider(); + }); + + describe('buildCliArgs', () => { + it('should include --resume when sdkSessionId is provided', () => { + const args = provider.buildCliArgs({ + prompt: 'Hello', + model: '2.5-flash', + cwd: '/tmp/project', + sdkSessionId: 'gemini-session-123', + }); + + const resumeIndex = args.indexOf('--resume'); + expect(resumeIndex).toBeGreaterThan(-1); + expect(args[resumeIndex + 1]).toBe('gemini-session-123'); + }); + + it('should not include --resume when sdkSessionId is missing', () => { + const args = provider.buildCliArgs({ + prompt: 'Hello', + model: '2.5-flash', + cwd: '/tmp/project', + }); + + expect(args).not.toContain('--resume'); + }); + }); +}); diff --git a/apps/server/tests/unit/routes/backlog-plan/generate-plan.test.ts b/apps/server/tests/unit/routes/backlog-plan/generate-plan.test.ts new file mode 100644 index 000000000..92556a35d --- /dev/null +++ b/apps/server/tests/unit/routes/backlog-plan/generate-plan.test.ts @@ -0,0 +1,218 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { BacklogPlanResult, ProviderMessage } from '@automaker/types'; + +const { + mockGetAll, + mockExecuteQuery, + mockSaveBacklogPlan, + mockSetRunningState, + mockSetRunningDetails, + mockGetPromptCustomization, + mockGetAutoLoadClaudeMdSetting, + mockGetUseClaudeCodeSystemPromptSetting, +} = vi.hoisted(() => ({ + mockGetAll: vi.fn(), + mockExecuteQuery: vi.fn(), + mockSaveBacklogPlan: vi.fn(), + mockSetRunningState: vi.fn(), + mockSetRunningDetails: vi.fn(), + mockGetPromptCustomization: vi.fn(), + mockGetAutoLoadClaudeMdSetting: vi.fn(), + mockGetUseClaudeCodeSystemPromptSetting: vi.fn(), +})); + +vi.mock('@/services/feature-loader.js', () => ({ + FeatureLoader: class { + getAll = mockGetAll; + }, +})); + +vi.mock('@/providers/provider-factory.js', () => ({ + ProviderFactory: { + getProviderForModel: vi.fn(() => ({ + executeQuery: mockExecuteQuery, + })), + }, +})); + +vi.mock('@/routes/backlog-plan/common.js', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, + setRunningState: mockSetRunningState, + setRunningDetails: mockSetRunningDetails, + getErrorMessage: (error: unknown) => (error instanceof Error ? error.message : String(error)), + saveBacklogPlan: mockSaveBacklogPlan, +})); + +vi.mock('@/lib/settings-helpers.js', () => ({ + getPromptCustomization: mockGetPromptCustomization, + getAutoLoadClaudeMdSetting: mockGetAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting: mockGetUseClaudeCodeSystemPromptSetting, + getPhaseModelWithOverrides: vi.fn(), +})); + +import { generateBacklogPlan } from '@/routes/backlog-plan/generate-plan.js'; + +function createMockEvents() { + return { + emit: vi.fn(), + }; +} + +describe('generateBacklogPlan', () => { + beforeEach(() => { + vi.clearAllMocks(); + + mockGetAll.mockResolvedValue([]); + mockGetPromptCustomization.mockResolvedValue({ + backlogPlan: { + systemPrompt: 'System instructions', + userPromptTemplate: + 'Current features:\n{{currentFeatures}}\n\nUser request:\n{{userRequest}}', + }, + }); + mockGetAutoLoadClaudeMdSetting.mockResolvedValue(false); + mockGetUseClaudeCodeSystemPromptSetting.mockResolvedValue(true); + }); + + it('salvages valid streamed JSON when Claude process exits with code 1', async () => { + const partialResult: BacklogPlanResult = { + changes: [ + { + type: 'add', + feature: { + title: 'Add signup form', + description: 'Create signup UI and validation', + category: 'frontend', + }, + reason: 'Required for user onboarding', + }, + ], + summary: 'Adds signup feature to the backlog', + dependencyUpdates: [], + }; + + const responseJson = JSON.stringify(partialResult); + + async function* streamWithExitError(): AsyncGenerator { + yield { + type: 'assistant', + message: { + role: 'assistant', + content: [{ type: 'text', text: responseJson }], + }, + }; + throw new Error('Claude Code process exited with code 1'); + } + + mockExecuteQuery.mockReturnValueOnce(streamWithExitError()); + + const events = createMockEvents(); + const abortController = new AbortController(); + + const result = await generateBacklogPlan( + '/tmp/project', + 'Please add a signup feature', + events as any, + abortController, + undefined, + 'claude-opus' + ); + + expect(mockExecuteQuery).toHaveBeenCalledTimes(1); + expect(result).toEqual(partialResult); + expect(mockSaveBacklogPlan).toHaveBeenCalledWith( + '/tmp/project', + expect.objectContaining({ + prompt: 'Please add a signup feature', + model: 'claude-opus-4-6', + result: partialResult, + }) + ); + expect(events.emit).toHaveBeenCalledWith('backlog-plan:event', { + type: 'backlog_plan_complete', + result: partialResult, + }); + expect(mockSetRunningState).toHaveBeenCalledWith(false, null); + expect(mockSetRunningDetails).toHaveBeenCalledWith(null); + }); + + it('prefers parseable provider result over longer non-JSON accumulated text on exit', async () => { + const recoveredResult: BacklogPlanResult = { + changes: [ + { + type: 'add', + feature: { + title: 'Add reset password flow', + description: 'Implement reset password request and token validation UI', + category: 'frontend', + }, + reason: 'Supports account recovery', + }, + ], + summary: 'Adds password reset capability', + dependencyUpdates: [], + }; + + const validProviderResult = JSON.stringify(recoveredResult); + const invalidAccumulatedText = `${validProviderResult}\n\nAdditional commentary that breaks raw JSON parsing.`; + + async function* streamWithResultThenExit(): AsyncGenerator { + yield { + type: 'assistant', + message: { + role: 'assistant', + content: [{ type: 'text', text: invalidAccumulatedText }], + }, + }; + yield { + type: 'result', + subtype: 'success', + duration_ms: 10, + duration_api_ms: 10, + is_error: false, + num_turns: 1, + result: validProviderResult, + session_id: 'session-1', + total_cost_usd: 0, + usage: { + input_tokens: 10, + cache_creation_input_tokens: 0, + cache_read_input_tokens: 0, + output_tokens: 10, + server_tool_use: { + web_search_requests: 0, + }, + service_tier: 'standard', + }, + }; + throw new Error('Claude Code process exited with code 1'); + } + + mockExecuteQuery.mockReturnValueOnce(streamWithResultThenExit()); + + const events = createMockEvents(); + const abortController = new AbortController(); + + const result = await generateBacklogPlan( + '/tmp/project', + 'Add password reset support', + events as any, + abortController, + undefined, + 'claude-opus' + ); + + expect(result).toEqual(recoveredResult); + expect(mockSaveBacklogPlan).toHaveBeenCalledWith( + '/tmp/project', + expect.objectContaining({ + result: recoveredResult, + }) + ); + }); +}); diff --git a/apps/server/tests/unit/services/agent-service.test.ts b/apps/server/tests/unit/services/agent-service.test.ts index 96090d2b9..c8ae1cba4 100644 --- a/apps/server/tests/unit/services/agent-service.test.ts +++ b/apps/server/tests/unit/services/agent-service.test.ts @@ -303,6 +303,36 @@ describe('agent-service.ts', () => { expect(fs.writeFile).toHaveBeenCalled(); }); + + it('should include context/history preparation for Gemini requests', async () => { + let capturedOptions: any; + const mockProvider = { + getName: () => 'gemini', + executeQuery: async function* (options: any) { + capturedOptions = options; + yield { + type: 'result', + subtype: 'success', + }; + }, + }; + + vi.mocked(ProviderFactory.getProviderForModelName).mockReturnValue('gemini'); + vi.mocked(ProviderFactory.getProviderForModel).mockReturnValue(mockProvider as any); + vi.mocked(promptBuilder.buildPromptWithImages).mockResolvedValue({ + content: 'Hello', + hasImages: false, + }); + + await service.sendMessage({ + sessionId: 'session-1', + message: 'Hello', + model: 'gemini-2.5-flash', + }); + + expect(contextLoader.loadContextFiles).toHaveBeenCalled(); + expect(capturedOptions).toBeDefined(); + }); }); describe('stopExecution', () => { diff --git a/apps/server/tests/unit/services/event-hook-service.test.ts b/apps/server/tests/unit/services/event-hook-service.test.ts index 1448fb809..ab06f9c1f 100644 --- a/apps/server/tests/unit/services/event-hook-service.test.ts +++ b/apps/server/tests/unit/services/event-hook-service.test.ts @@ -116,6 +116,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: true, @@ -144,6 +145,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: false, @@ -171,6 +173,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: true, @@ -200,6 +203,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: false, @@ -217,6 +221,55 @@ describe('EventHookService', () => { // Error field should be populated for error triggers expect(storeCall.error).toBe('Feature stopped by user'); }); + + it('should ignore feature complete events without explicit auto execution mode', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + mockEmitter.simulateEvent('auto-mode:event', { + type: 'auto_mode_feature_complete', + featureId: 'feat-1', + featureName: 'Manual Feature', + passes: true, + message: 'Manually verified', + projectPath: '/test/project', + }); + + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(mockEventHistoryService.storeEvent).not.toHaveBeenCalled(); + }); + }); + + describe('event mapping - feature:completed', () => { + it('should map manual completion to feature_success', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + mockEmitter.simulateEvent('feature:completed', { + featureId: 'feat-1', + featureName: 'Manual Feature', + projectPath: '/test/project', + passes: true, + executionMode: 'manual', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalled(); + }); + + const storeCall = (mockEventHistoryService.storeEvent as ReturnType).mock + .calls[0][0]; + expect(storeCall.trigger).toBe('feature_success'); + expect(storeCall.passes).toBe(true); + }); }); describe('event mapping - auto_mode_error', () => { @@ -400,6 +453,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: true, @@ -420,7 +474,6 @@ describe('EventHookService', () => { it('should NOT execute error hooks when feature completes successfully', async () => { // This is the key regression test for the bug: // "Error event hook fired when a feature completes successfully" - const errorHookCommand = vi.fn(); const hooks = [ { id: 'hook-error', @@ -444,6 +497,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Test Feature', passes: true, @@ -480,6 +534,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', passes: true, message: 'Done', @@ -507,6 +562,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', featureName: 'Fallback Name', passes: true, @@ -524,6 +580,204 @@ describe('EventHookService', () => { }); }); + describe('event mapping - feature_status_changed (non-auto-mode completion)', () => { + it('should trigger feature_success when status changes to verified', async () => { + mockFeatureLoader = createMockFeatureLoader({ + 'feat-1': { title: 'Manual Feature' }, + }); + + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-1', + projectPath: '/test/project', + status: 'verified', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalled(); + }); + + const storeCall = (mockEventHistoryService.storeEvent as ReturnType).mock + .calls[0][0]; + expect(storeCall.trigger).toBe('feature_success'); + expect(storeCall.featureName).toBe('Manual Feature'); + expect(storeCall.passes).toBe(true); + }); + + it('should trigger feature_success when status changes to waiting_approval', async () => { + mockFeatureLoader = createMockFeatureLoader({ + 'feat-1': { title: 'Manual Feature' }, + }); + + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-1', + projectPath: '/test/project', + status: 'waiting_approval', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalled(); + }); + + const storeCall = (mockEventHistoryService.storeEvent as ReturnType).mock + .calls[0][0]; + expect(storeCall.trigger).toBe('feature_success'); + expect(storeCall.passes).toBe(true); + expect(storeCall.featureName).toBe('Manual Feature'); + }); + + it('should NOT trigger hooks for non-completion status changes', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-1', + projectPath: '/test/project', + status: 'in_progress', + }); + + // Give it time to process + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(mockEventHistoryService.storeEvent).not.toHaveBeenCalled(); + }); + + it('should NOT double-fire hooks when auto_mode_feature_complete already fired', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + // First: auto_mode_feature_complete fires (auto-mode path) + mockEmitter.simulateEvent('auto-mode:event', { + type: 'auto_mode_feature_complete', + executionMode: 'auto', + featureId: 'feat-1', + featureName: 'Auto Feature', + passes: true, + message: 'Feature completed', + projectPath: '/test/project', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1); + }); + + // Then: feature_status_changed fires for the same feature + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-1', + projectPath: '/test/project', + status: 'verified', + }); + + // Give it time to process + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Should still only have been called once (from auto_mode_feature_complete) + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1); + }); + + it('should NOT double-fire hooks when auto_mode_error already fired for feature', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + // First: auto_mode_error fires for a feature + mockEmitter.simulateEvent('auto-mode:event', { + type: 'auto_mode_error', + featureId: 'feat-1', + error: 'Something failed', + errorType: 'execution', + projectPath: '/test/project', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1); + }); + + // Then: feature_status_changed fires for the same feature (e.g., reset to backlog) + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-1', + projectPath: '/test/project', + status: 'verified', // unlikely after error, but tests the dedup + }); + + // Give it time to process + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Should still only have been called once + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1); + }); + + it('should fire hooks for different features independently', async () => { + service.initialize( + mockEmitter, + mockSettingsService, + mockEventHistoryService, + mockFeatureLoader + ); + + // Auto-mode completion for feat-1 + mockEmitter.simulateEvent('auto-mode:event', { + type: 'auto_mode_feature_complete', + executionMode: 'auto', + featureId: 'feat-1', + passes: true, + message: 'Done', + projectPath: '/test/project', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1); + }); + + // Manual completion for feat-2 (different feature) + mockEmitter.simulateEvent('auto-mode:event', { + type: 'feature_status_changed', + featureId: 'feat-2', + projectPath: '/test/project', + status: 'verified', + }); + + await vi.waitFor(() => { + expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(2); + }); + + // feat-2 should have triggered feature_success + const secondCall = (mockEventHistoryService.storeEvent as ReturnType).mock + .calls[1][0]; + expect(secondCall.trigger).toBe('feature_success'); + expect(secondCall.featureId).toBe('feat-2'); + }); + }); + describe('error context for error events', () => { it('should use payload.error when available for error triggers', async () => { service.initialize( @@ -561,6 +815,7 @@ describe('EventHookService', () => { mockEmitter.simulateEvent('auto-mode:event', { type: 'auto_mode_feature_complete', + executionMode: 'auto', featureId: 'feat-1', passes: false, message: 'Feature stopped by user', diff --git a/apps/server/tests/unit/services/execution-service.test.ts b/apps/server/tests/unit/services/execution-service.test.ts index f56e5c411..a5d001f8e 100644 --- a/apps/server/tests/unit/services/execution-service.test.ts +++ b/apps/server/tests/unit/services/execution-service.test.ts @@ -34,6 +34,7 @@ import { getFeatureDir } from '@automaker/platform'; import { getPromptCustomization, getAutoLoadClaudeMdSetting, + getUseClaudeCodeSystemPromptSetting, filterClaudeMdFromContext, } from '../../../src/lib/settings-helpers.js'; import { extractSummary } from '../../../src/services/spec-parser.js'; @@ -67,6 +68,7 @@ vi.mock('../../../src/lib/settings-helpers.js', () => ({ }, }), getAutoLoadClaudeMdSetting: vi.fn().mockResolvedValue(true), + getUseClaudeCodeSystemPromptSetting: vi.fn().mockResolvedValue(true), filterClaudeMdFromContext: vi.fn().mockReturnValue('context prompt'), })); @@ -230,6 +232,7 @@ describe('execution-service.ts', () => { }, } as Awaited>); vi.mocked(getAutoLoadClaudeMdSetting).mockResolvedValue(true); + vi.mocked(getUseClaudeCodeSystemPromptSetting).mockResolvedValue(true); vi.mocked(filterClaudeMdFromContext).mockReturnValue('context prompt'); // Re-setup spec-parser mock diff --git a/apps/server/tests/unit/services/pipeline-orchestrator.test.ts b/apps/server/tests/unit/services/pipeline-orchestrator.test.ts index 079db0e71..d4f342656 100644 --- a/apps/server/tests/unit/services/pipeline-orchestrator.test.ts +++ b/apps/server/tests/unit/services/pipeline-orchestrator.test.ts @@ -57,6 +57,7 @@ vi.mock('../../../src/lib/settings-helpers.js', () => ({ }, }), getAutoLoadClaudeMdSetting: vi.fn().mockResolvedValue(true), + getUseClaudeCodeSystemPromptSetting: vi.fn().mockResolvedValue(true), filterClaudeMdFromContext: vi.fn().mockReturnValue('context prompt'), })); diff --git a/apps/server/tests/unit/services/settings-service.test.ts b/apps/server/tests/unit/services/settings-service.test.ts index 70511af86..e54358fce 100644 --- a/apps/server/tests/unit/services/settings-service.test.ts +++ b/apps/server/tests/unit/services/settings-service.test.ts @@ -740,8 +740,11 @@ describe('settings-service.ts', () => { // Legacy fields should be migrated to phaseModels with canonical IDs expect(settings.phaseModels.enhancementModel).toEqual({ model: 'claude-haiku' }); expect(settings.phaseModels.validationModel).toEqual({ model: 'claude-opus' }); - // Other fields should use defaults (canonical IDs) - expect(settings.phaseModels.specGenerationModel).toEqual({ model: 'claude-opus' }); + // Other fields should use defaults (canonical IDs) - specGenerationModel includes thinkingLevel from DEFAULT_PHASE_MODELS + expect(settings.phaseModels.specGenerationModel).toEqual({ + model: 'claude-opus', + thinkingLevel: 'adaptive', + }); }); it('should use default phase models when none are configured', async () => { @@ -755,10 +758,13 @@ describe('settings-service.ts', () => { const settings = await settingsService.getGlobalSettings(); - // Should use DEFAULT_PHASE_MODELS (with canonical IDs) + // Should use DEFAULT_PHASE_MODELS (with canonical IDs) - specGenerationModel includes thinkingLevel from DEFAULT_PHASE_MODELS expect(settings.phaseModels.enhancementModel).toEqual({ model: 'claude-sonnet' }); expect(settings.phaseModels.fileDescriptionModel).toEqual({ model: 'claude-haiku' }); - expect(settings.phaseModels.specGenerationModel).toEqual({ model: 'claude-opus' }); + expect(settings.phaseModels.specGenerationModel).toEqual({ + model: 'claude-opus', + thinkingLevel: 'adaptive', + }); }); it('should deep merge phaseModels on update', async () => { diff --git a/apps/ui/nginx.conf b/apps/ui/nginx.conf index 2d96d1589..da56165d0 100644 --- a/apps/ui/nginx.conf +++ b/apps/ui/nginx.conf @@ -1,9 +1,30 @@ +# Map for conditional WebSocket upgrade header +map $http_upgrade $connection_upgrade { + default upgrade; + '' close; +} + server { listen 80; server_name localhost; root /usr/share/nginx/html; index index.html; + # Proxy API and WebSocket requests to the backend server container + # Handles both HTTP API calls and WebSocket upgrades (/api/events, /api/terminal/ws) + location /api/ { + proxy_pass http://server:3008; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_buffering off; + proxy_read_timeout 3600s; + } + location / { try_files $uri $uri/ /index.html; } diff --git a/apps/ui/package.json b/apps/ui/package.json index b93fd7c60..7b2c35f1b 100644 --- a/apps/ui/package.json +++ b/apps/ui/package.json @@ -1,6 +1,6 @@ { "name": "@automaker/ui", - "version": "0.13.0", + "version": "0.15.0", "description": "An autonomous AI development studio that helps you build software faster using AI-powered agents", "homepage": "https://github.com/AutoMaker-Org/automaker", "repository": { @@ -56,6 +56,7 @@ "@codemirror/lang-xml": "6.1.0", "@codemirror/language": "^6.12.1", "@codemirror/legacy-modes": "^6.5.2", + "@codemirror/merge": "^6.12.0", "@codemirror/search": "^6.6.0", "@codemirror/state": "^6.5.4", "@codemirror/theme-one-dark": "6.1.3", diff --git a/apps/ui/src/components/dialogs/pr-comment-resolution-dialog.tsx b/apps/ui/src/components/dialogs/pr-comment-resolution-dialog.tsx index fa54ffcdc..b0f86aa1d 100644 --- a/apps/ui/src/components/dialogs/pr-comment-resolution-dialog.tsx +++ b/apps/ui/src/components/dialogs/pr-comment-resolution-dialog.tsx @@ -914,7 +914,7 @@ export function PRCommentResolutionDialog({ {!loading && !error && allComments.length > 0 && ( <> {/* Controls Bar */} -
+
{/* Select All - only interactive when there are visible comments */}
-
+
{/* Show/Hide Resolved Filter Toggle - always visible */} +
+ ); + } + + return this.props.children; + } +} diff --git a/apps/ui/src/components/views/board-view/components/add-feature-button.tsx b/apps/ui/src/components/views/board-view/components/add-feature-button.tsx new file mode 100644 index 000000000..abf352704 --- /dev/null +++ b/apps/ui/src/components/views/board-view/components/add-feature-button.tsx @@ -0,0 +1,188 @@ +import { useState } from 'react'; +import { Button } from '@/components/ui/button'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/components/ui/dropdown-menu'; +import { Plus, ChevronDown, Zap, FileText } from 'lucide-react'; +import type { FeatureTemplate } from '@automaker/types'; +import { cn } from '@/lib/utils'; + +interface AddFeatureButtonProps { + /** Handler for the primary "Add Feature" action (opens full dialog) */ + onAddFeature: () => void; + /** Handler for Quick Add submission */ + onQuickAdd: () => void; + /** Handler for template selection */ + onTemplateSelect: (template: FeatureTemplate) => void; + /** Available templates (filtered to enabled ones) */ + templates: FeatureTemplate[]; + /** Whether to show as a small icon button or full button */ + compact?: boolean; + /** Whether the button should take full width */ + fullWidth?: boolean; + /** Additional className */ + className?: string; + /** Test ID prefix */ + testIdPrefix?: string; + /** Shortcut text to display (optional) */ + shortcut?: string; +} + +export function AddFeatureButton({ + onAddFeature, + onQuickAdd, + onTemplateSelect, + templates, + compact = false, + fullWidth = false, + className, + testIdPrefix = 'add-feature', + shortcut, +}: AddFeatureButtonProps) { + const [dropdownOpen, setDropdownOpen] = useState(false); + + // Filter to only enabled templates and sort by order + const enabledTemplates = templates + .filter((t) => t.enabled !== false) + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); + + const handleTemplateClick = (template: FeatureTemplate) => { + setDropdownOpen(false); + onTemplateSelect(template); + }; + + if (compact) { + // Compact mode: Three small icon segments + return ( +
+ {/* Segment 1: Add Feature */} + + {/* Segment 2: Quick Add */} + + {/* Segment 3: Templates dropdown */} + {enabledTemplates.length > 0 && ( + + + + + + {enabledTemplates.map((template) => ( + handleTemplateClick(template)} + data-testid={`template-menu-item-${template.id}`} + > + + {template.name} + + ))} + + + )} +
+ ); + } + + // Full mode: Three-segment button + return ( +
+ {/* Segment 1: Add Feature */} + + {/* Segment 2: Quick Add */} + + {/* Segment 3: Templates dropdown */} + + + + + + {enabledTemplates.length > 0 ? ( + enabledTemplates.map((template) => ( + handleTemplateClick(template)} + data-testid={`template-menu-item-${template.id}`} + > + + {template.name} + + )) + ) : ( + + No templates configured + + )} + + +
+ ); +} diff --git a/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx b/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx index 814bd0e30..dac9b302d 100644 --- a/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx +++ b/apps/ui/src/components/views/board-view/components/list-view/list-view.tsx @@ -5,12 +5,13 @@ import { Button } from '@/components/ui/button'; import { getBlockingDependencies } from '@automaker/dependency-resolver'; import { useAppStore, formatShortcut } from '@/store/app-store'; import type { Feature } from '@/store/app-store'; -import type { PipelineConfig, FeatureStatusWithPipeline } from '@automaker/types'; +import type { PipelineConfig, FeatureStatusWithPipeline, FeatureTemplate } from '@automaker/types'; import { ListHeader } from './list-header'; import { ListRow, sortFeatures } from './list-row'; import { createRowActionHandlers, type RowActionHandlers } from './row-actions'; import { getStatusOrder } from './status-badge'; import { getColumnsWithPipeline } from '../../constants'; +import { AddFeatureButton } from '../add-feature-button'; import type { SortConfig, SortColumn } from '../../hooks/use-list-view-state'; /** Empty set constant to avoid creating new instances on each render */ @@ -65,6 +66,12 @@ export interface ListViewProps { pipelineConfig?: PipelineConfig | null; /** Callback to add a new feature */ onAddFeature?: () => void; + /** Callback for quick add */ + onQuickAdd?: () => void; + /** Callback for template selection */ + onTemplateSelect?: (template: FeatureTemplate) => void; + /** Available feature templates */ + templates?: FeatureTemplate[]; /** Whether selection mode is enabled */ isSelectionMode?: boolean; /** Set of selected feature IDs */ @@ -125,7 +132,22 @@ const StatusGroupHeader = memo(function StatusGroupHeader({ /** * EmptyState displays a message when there are no features */ -const EmptyState = memo(function EmptyState({ onAddFeature }: { onAddFeature?: () => void }) { +const EmptyState = memo(function EmptyState({ + onAddFeature, + onQuickAdd, + onTemplateSelect, + templates, + shortcut, +}: { + onAddFeature?: () => void; + onQuickAdd?: () => void; + onTemplateSelect?: (template: FeatureTemplate) => void; + templates?: FeatureTemplate[]; + shortcut?: string; +}) { + // Only show AddFeatureButton if all required handlers are provided + const canShowSplitButton = onAddFeature && onQuickAdd && onTemplateSelect; + return (

No features to display

- {onAddFeature && ( + {canShowSplitButton ? ( + + ) : onAddFeature ? ( - )} + ) : null}
); }); @@ -190,6 +221,9 @@ export const ListView = memo(function ListView({ runningAutoTasks, pipelineConfig = null, onAddFeature, + onQuickAdd, + onTemplateSelect, + templates = [], isSelectionMode = false, selectedFeatureIds = EMPTY_SET, onToggleFeatureSelection, @@ -388,7 +422,13 @@ export const ListView = memo(function ListView({ if (totalFeatures === 0) { return (
- +
); } @@ -452,21 +492,17 @@ export const ListView = memo(function ListView({
{/* Footer with Add Feature button, styled like board view */} - {onAddFeature && ( + {onAddFeature && onQuickAdd && onTemplateSelect && (
- +
)}
diff --git a/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx index 3fdd93e62..86a684646 100644 --- a/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx @@ -28,7 +28,11 @@ import { cn } from '@/lib/utils'; import { modelSupportsThinking } from '@/lib/utils'; import { useAppStore, ThinkingLevel, FeatureImage, PlanningMode, Feature } from '@/store/app-store'; import type { ReasoningEffort, PhaseModelEntry, AgentModel } from '@automaker/types'; -import { supportsReasoningEffort, isAdaptiveThinkingModel } from '@automaker/types'; +import { + supportsReasoningEffort, + isAdaptiveThinkingModel, + getThinkingLevelsForModel, +} from '@automaker/types'; import { PrioritySelector, WorkModeSelector, @@ -211,6 +215,7 @@ export function AddFeatureDialog({ defaultRequirePlanApproval, useWorktrees, defaultFeatureModel, + defaultThinkingLevel, currentProject, } = useAppStore(); @@ -240,7 +245,22 @@ export function AddFeatureDialog({ ); setPlanningMode(defaultPlanningMode); setRequirePlanApproval(defaultRequirePlanApproval); - setModelEntry(effectiveDefaultFeatureModel); + + // Apply defaultThinkingLevel from settings to the model entry. + // This ensures the "Quick-Select Defaults" thinking level setting is respected + // even when the user doesn't change the model in the dropdown. + const modelId = + typeof effectiveDefaultFeatureModel.model === 'string' + ? effectiveDefaultFeatureModel.model + : ''; + const availableLevels = getThinkingLevelsForModel(modelId); + const effectiveThinkingLevel = availableLevels.includes(defaultThinkingLevel) + ? defaultThinkingLevel + : availableLevels[0]; + setModelEntry({ + ...effectiveDefaultFeatureModel, + thinkingLevel: effectiveThinkingLevel, + }); // Initialize description history (empty for new feature) setDescriptionHistory([]); @@ -269,6 +289,7 @@ export function AddFeatureDialog({ defaultPlanningMode, defaultRequirePlanApproval, effectiveDefaultFeatureModel, + defaultThinkingLevel, useWorktrees, selectedNonMainWorktreeBranch, forceCurrentBranchMode, @@ -394,7 +415,19 @@ export function AddFeatureDialog({ // When a non-main worktree is selected, use its branch name for custom mode setBranchName(selectedNonMainWorktreeBranch || ''); setPriority(2); - setModelEntry(effectiveDefaultFeatureModel); + // Apply defaultThinkingLevel to the model entry (same logic as dialog open) + const resetModelId = + typeof effectiveDefaultFeatureModel.model === 'string' + ? effectiveDefaultFeatureModel.model + : ''; + const resetAvailableLevels = getThinkingLevelsForModel(resetModelId); + const resetThinkingLevel = resetAvailableLevels.includes(defaultThinkingLevel) + ? defaultThinkingLevel + : resetAvailableLevels[0]; + setModelEntry({ + ...effectiveDefaultFeatureModel, + thinkingLevel: resetThinkingLevel, + }); setWorkMode( getDefaultWorkMode(useWorktrees, selectedNonMainWorktreeBranch, forceCurrentBranchMode) ); diff --git a/apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx index afc770e7d..226cf359a 100644 --- a/apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/backlog-plan-dialog.tsx @@ -29,7 +29,10 @@ import { useAppStore } from '@/store/app-store'; /** * Normalize PhaseModelEntry or string to PhaseModelEntry */ -function normalizeEntry(entry: PhaseModelEntry | string): PhaseModelEntry { +function normalizeEntry(entry: PhaseModelEntry | string | undefined | null): PhaseModelEntry { + if (!entry) { + return { model: 'claude-sonnet' as ModelAlias }; + } if (typeof entry === 'string') { return { model: entry as ModelAlias | CursorModelId }; } @@ -110,7 +113,12 @@ export function BacklogPlanDialog({ // Use model override if set, otherwise use global default (extract model string from PhaseModelEntry) const effectiveModelEntry = modelOverride || normalizeEntry(phaseModels.backlogPlanningModel); const effectiveModel = effectiveModelEntry.model; - const result = await api.backlogPlan.generate(projectPath, prompt, effectiveModel); + const result = await api.backlogPlan.generate( + projectPath, + prompt, + effectiveModel, + currentBranch + ); if (!result.success) { logger.error('Backlog plan generation failed to start', { error: result.error, @@ -131,7 +139,15 @@ export function BacklogPlanDialog({ }); setPrompt(''); onClose(); - }, [projectPath, prompt, modelOverride, phaseModels, setIsGeneratingPlan, onClose]); + }, [ + projectPath, + prompt, + modelOverride, + phaseModels, + setIsGeneratingPlan, + onClose, + currentBranch, + ]); const handleApply = useCallback(async () => { if (!pendingPlanResult) return; diff --git a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx index c31ff482d..10ed992b2 100644 --- a/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/commit-worktree-dialog.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useMemo, useCallback } from 'react'; +import { useState, useEffect, useMemo, useCallback, useRef } from 'react'; import { Dialog, DialogContent, @@ -30,13 +30,17 @@ import { ChevronDown, ChevronRight, Upload, + RefreshCw, } from 'lucide-react'; import { Spinner } from '@/components/ui/spinner'; import { getElectronAPI } from '@/lib/electron'; +import { getHttpApiClient } from '@/lib/http-api-client'; import { toast } from 'sonner'; import { useAppStore } from '@/store/app-store'; +import { resolveModelString } from '@automaker/model-resolver'; import { cn } from '@/lib/utils'; import { TruncatedFilePath } from '@/components/ui/truncated-file-path'; +import { ModelOverrideTrigger, useModelOverride } from '@/components/shared'; import type { FileStatus, MergeStateInfo } from '@/types/electron'; import { parseDiff, type ParsedFileDiff } from '@/lib/diff-utils'; @@ -206,6 +210,11 @@ export function CommitWorktreeDialog({ const [error, setError] = useState(null); const enableAiCommitMessages = useAppStore((state) => state.enableAiCommitMessages); + // Commit message model override + const commitModelOverride = useModelOverride({ phase: 'commitMessageModel' }); + const { effectiveModel: commitEffectiveModel, effectiveModelEntry: commitEffectiveModelEntry } = + commitModelOverride; + // File selection state const [files, setFiles] = useState([]); const [diffContent, setDiffContent] = useState(''); @@ -532,6 +541,46 @@ export function CommitWorktreeDialog({ } }; + // Generate AI commit message + const generateCommitMessage = useCallback(async () => { + if (!worktree) return; + + setIsGenerating(true); + try { + const resolvedCommitModel = resolveModelString(commitEffectiveModel); + const api = getHttpApiClient(); + const result = await api.worktree.generateCommitMessage( + worktree.path, + resolvedCommitModel, + commitEffectiveModelEntry?.thinkingLevel, + commitEffectiveModelEntry?.providerId + ); + + if (result.success && result.message) { + setMessage(result.message); + } else { + console.warn('Failed to generate commit message:', result.error); + toast.error('Failed to generate commit message', { + description: result.error || 'Unknown error', + }); + } + } catch (err) { + console.warn('Error generating commit message:', err); + toast.error('Failed to generate commit message', { + description: err instanceof Error ? err.message : 'Unknown error', + }); + } finally { + setIsGenerating(false); + } + }, [worktree, commitEffectiveModel, commitEffectiveModelEntry]); + + // Keep a stable ref to generateCommitMessage so the open-dialog effect + // doesn't re-fire (and erase user edits) when the model override changes. + const generateCommitMessageRef = useRef(generateCommitMessage); + useEffect(() => { + generateCommitMessageRef.current = generateCommitMessage; + }); + // Generate AI commit message when dialog opens (if enabled) useEffect(() => { if (open && worktree) { @@ -543,45 +592,7 @@ export function CommitWorktreeDialog({ return; } - setIsGenerating(true); - let cancelled = false; - - const generateMessage = async () => { - try { - const api = getElectronAPI(); - if (!api?.worktree?.generateCommitMessage) { - if (!cancelled) { - setIsGenerating(false); - } - return; - } - - const result = await api.worktree.generateCommitMessage(worktree.path); - - if (cancelled) return; - - if (result.success && result.message) { - setMessage(result.message); - } else { - console.warn('Failed to generate commit message:', result.error); - setMessage(''); - } - } catch (err) { - if (cancelled) return; - console.warn('Error generating commit message:', err); - setMessage(''); - } finally { - if (!cancelled) { - setIsGenerating(false); - } - } - }; - - generateMessage(); - - return () => { - cancelled = true; - }; + generateCommitMessageRef.current(); } }, [open, worktree, enableAiCommitMessages]); @@ -589,12 +600,12 @@ export function CommitWorktreeDialog({ const allSelected = selectedFiles.size === files.length && files.length > 0; - // Prevent the dialog from being dismissed while a push is in progress. + // Prevent the dialog from being dismissed while a push or generation is in progress. // Overlay clicks and Escape key both route through onOpenChange(false); we - // intercept those here so the UI stays open until the push completes. + // intercept those here so the UI stays open until the operation completes. const handleOpenChange = (nextOpen: boolean) => { - if (!nextOpen && isPushing) { - // Ignore close requests during an active push. + if (!nextOpen && (isLoading || isPushing || isGenerating)) { + // Ignore close requests during an active commit, push, or generation. return; } onOpenChange(nextOpen); @@ -813,15 +824,46 @@ export function CommitWorktreeDialog({ {/* Commit Message */}
- +
+ +
+ {enableAiCommitMessages && ( + <> + + + + )} +
+