diff --git a/apps/server/src/providers/claude-provider.ts b/apps/server/src/providers/claude-provider.ts index 54ef00f6..5f45900f 100644 --- a/apps/server/src/providers/claude-provider.ts +++ b/apps/server/src/providers/claude-provider.ts @@ -350,7 +350,7 @@ export class ClaudeProvider extends BaseProvider { provider: 'anthropic', description: 'Balanced performance and cost with enhanced reasoning', contextWindow: 200000, - maxOutputTokens: 128000, + maxOutputTokens: 64000, supportsVision: true, supportsTools: true, tier: 'standard' as const, diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index 756d5e95..0ff543f3 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -212,7 +212,6 @@ async function resolveCodexExecutionPlan(options: ExecuteOptions): Promise 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 args = [ CODEX_EXEC_SUBCOMMAND, diff --git a/apps/server/src/providers/opencode-provider.ts b/apps/server/src/providers/opencode-provider.ts index 76d92483..8c58da15 100644 --- a/apps/server/src/providers/opencode-provider.ts +++ b/apps/server/src/providers/opencode-provider.ts @@ -464,7 +464,7 @@ export class OpencodeProvider extends CliProvider { // Without this guard, errors like "ProviderModelNotFoundError" or // "Resource not found: /path/to/config.json" would false-positive. if (cleaned.includes('notfounderror') || cleaned.includes('resource not found')) { - return cleaned.includes('/session/') || cleaned.includes('session'); + return cleaned.includes('/session/') || /\bsession\b/.test(cleaned); } return false; @@ -720,11 +720,29 @@ export class OpencodeProvider extends CliProvider { return null; } - // Final completion (reason: 'stop' or 'end_turn') + // Only treat an explicit allowlist of reasons as true success. + // Reasons like 'length' (context-window truncation) or 'content-filter' + // indicate the model stopped abnormally and must not be surfaced as + // successful completions. + const SUCCESS_REASONS = new Set(['stop', 'end_turn']); + const reason = finishEvent.part?.reason; + + if (reason === undefined || SUCCESS_REASONS.has(reason)) { + // Final completion (reason: 'stop', 'end_turn', or unset) + return { + type: 'result', + subtype: 'success', + session_id: finishEvent.sessionID, + result: (finishEvent.part as OpenCodePart & { result?: string })?.result, + }; + } + + // Non-success, non-tool-calls reason (e.g. 'length', 'content-filter') return { type: 'result', - subtype: 'success', + subtype: 'error', session_id: finishEvent.sessionID, + error: `Step finished with non-success reason: ${reason}`, result: (finishEvent.part as OpenCodePart & { result?: string })?.result, }; } diff --git a/apps/server/src/routes/git/index.ts b/apps/server/src/routes/git/index.ts index da6adabf..e6bf5a0c 100644 --- a/apps/server/src/routes/git/index.ts +++ b/apps/server/src/routes/git/index.ts @@ -13,7 +13,11 @@ export function createGitRoutes(): Router { router.post('/diffs', validatePathParams('projectPath'), createDiffsHandler()); router.post('/file-diff', validatePathParams('projectPath', 'filePath'), createFileDiffHandler()); - router.post('/stage-files', validatePathParams('projectPath'), createStageFilesHandler()); + router.post( + '/stage-files', + validatePathParams('projectPath', 'files[]'), + createStageFilesHandler() + ); return router; } diff --git a/apps/server/src/routes/git/routes/stage-files.ts b/apps/server/src/routes/git/routes/stage-files.ts index b618bdd3..c688f8b5 100644 --- a/apps/server/src/routes/git/routes/stage-files.ts +++ b/apps/server/src/routes/git/routes/stage-files.ts @@ -2,6 +2,7 @@ * POST /stage-files endpoint - Stage or unstage files in the main project */ +import path from 'path'; import type { Request, Response } from 'express'; import { getErrorMessage, logError } from '../common.js'; import { execGitCommand } from '../../../lib/git.js'; @@ -39,17 +40,49 @@ export function createStageFilesHandler() { return; } + // Validate and sanitize each file path to prevent path traversal attacks + const base = path.resolve(projectPath) + path.sep; + const sanitizedFiles: string[] = []; + for (const file of files) { + // Reject absolute paths + if (path.isAbsolute(file)) { + res.status(400).json({ + success: false, + error: `Invalid file path (absolute paths not allowed): ${file}`, + }); + return; + } + // Reject entries containing '..' + if (file.includes('..')) { + res.status(400).json({ + success: false, + error: `Invalid file path (path traversal not allowed): ${file}`, + }); + return; + } + // Ensure the resolved path stays within the project directory + const resolved = path.resolve(path.join(projectPath, file)); + if (resolved !== path.resolve(projectPath) && !resolved.startsWith(base)) { + res.status(400).json({ + success: false, + error: `Invalid file path (outside project directory): ${file}`, + }); + return; + } + sanitizedFiles.push(file); + } + if (operation === 'stage') { - await execGitCommand(['add', '--', ...files], projectPath); + await execGitCommand(['add', '--', ...sanitizedFiles], projectPath); } else { - await execGitCommand(['reset', 'HEAD', '--', ...files], projectPath); + await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], projectPath); } res.json({ success: true, result: { operation, - filesCount: files.length, + filesCount: sanitizedFiles.length, }, }); } catch (error) { diff --git a/apps/server/src/routes/worktree/index.ts b/apps/server/src/routes/worktree/index.ts index 190dde34..983d6bb8 100644 --- a/apps/server/src/routes/worktree/index.ts +++ b/apps/server/src/routes/worktree/index.ts @@ -298,7 +298,7 @@ export function createWorktreeRoutes( // Stage/unstage files route router.post( '/stage-files', - validatePathParams('worktreePath'), + validatePathParams('worktreePath', 'files[]'), requireGitRepoOnly, createStageFilesHandler() ); diff --git a/apps/server/src/routes/worktree/routes/list.ts b/apps/server/src/routes/worktree/routes/list.ts index 19cbb850..333ba7c2 100644 --- a/apps/server/src/routes/worktree/routes/list.ts +++ b/apps/server/src/routes/worktree/routes/list.ts @@ -58,9 +58,9 @@ interface WorktreeInfo { hasChanges?: boolean; changedFilesCount?: number; pr?: WorktreePRInfo; // PR info if a PR has been created for this branch - /** Whether a merge or rebase is in progress (has conflicts) */ + /** Whether there are actual unresolved conflict files (conflictFiles.length > 0) */ hasConflicts?: boolean; - /** Type of conflict operation in progress */ + /** Type of git operation in progress (merge/rebase/cherry-pick), set independently of hasConflicts */ conflictType?: 'merge' | 'rebase' | 'cherry-pick'; /** List of files with conflicts */ conflictFiles?: string[]; @@ -80,6 +80,7 @@ async function detectConflictState(worktreePath: string): Promise<{ // Find the canonical .git directory for this worktree const { stdout: gitDirRaw } = await execAsync('git rev-parse --git-dir', { cwd: worktreePath, + timeout: 15000, }); const gitDir = path.resolve(worktreePath, gitDirRaw.trim()); @@ -122,6 +123,7 @@ async function detectConflictState(worktreePath: string): Promise<{ try { const { stdout: statusOutput } = await execAsync('git diff --name-only --diff-filter=U', { cwd: worktreePath, + timeout: 15000, }); conflictFiles = statusOutput .trim() @@ -132,7 +134,7 @@ async function detectConflictState(worktreePath: string): Promise<{ } return { - hasConflicts: true, + hasConflicts: conflictFiles.length > 0, conflictType, conflictFiles, }; @@ -476,11 +478,14 @@ export function createListHandler() { // Detect merge/rebase/cherry-pick in progress try { const conflictState = await detectConflictState(worktree.path); - if (conflictState.hasConflicts) { - worktree.hasConflicts = true; + // Always propagate conflictType so callers know an operation is in progress, + // even when there are no unresolved conflict files yet. + if (conflictState.conflictType) { worktree.conflictType = conflictState.conflictType; - worktree.conflictFiles = conflictState.conflictFiles; } + // hasConflicts is true only when there are actual unresolved files + worktree.hasConflicts = conflictState.hasConflicts; + worktree.conflictFiles = conflictState.conflictFiles; } catch { // Ignore conflict detection errors } diff --git a/apps/server/src/routes/worktree/routes/stage-files.ts b/apps/server/src/routes/worktree/routes/stage-files.ts index 8dca59fb..618e8b67 100644 --- a/apps/server/src/routes/worktree/routes/stage-files.ts +++ b/apps/server/src/routes/worktree/routes/stage-files.ts @@ -9,6 +9,8 @@ * the requireGitRepoOnly middleware in index.ts */ +import path from 'path'; +import fs from 'fs/promises'; import type { Request, Response } from 'express'; import { getErrorMessage, logError } from '../common.js'; import { execGitCommand } from '../../../lib/git.js'; @@ -46,19 +48,70 @@ export function createStageFilesHandler() { return; } + // Canonicalize the worktree root by resolving symlinks so that + // path-traversal checks are reliable even when symlinks are involved. + let canonicalRoot: string; + try { + canonicalRoot = await fs.realpath(worktreePath); + } catch { + res.status(400).json({ + success: false, + error: 'worktreePath does not exist or is not accessible', + }); + return; + } + + // Validate and sanitize each file path to prevent path traversal attacks. + // Each file entry is resolved against the canonicalized worktree root and + // must remain within that root directory. + const base = canonicalRoot + path.sep; + const sanitizedFiles: string[] = []; + for (const file of files) { + // Reject absolute paths + if (path.isAbsolute(file)) { + res.status(400).json({ + success: false, + error: `Invalid file path (absolute paths not allowed): ${file}`, + }); + return; + } + // Reject entries containing '..' + if (file.includes('..')) { + res.status(400).json({ + success: false, + error: `Invalid file path (path traversal not allowed): ${file}`, + }); + return; + } + // Resolve the file path against the canonicalized worktree root and + // ensure the result stays within the worktree directory. + const resolved = path.resolve(canonicalRoot, file); + if (resolved !== canonicalRoot && !resolved.startsWith(base)) { + res.status(400).json({ + success: false, + error: `Invalid file path (outside worktree directory): ${file}`, + }); + return; + } + // Forward only the original relative path to git — git interprets + // paths relative to its working directory (canonicalRoot / worktreePath), + // so we do not need to pass the resolved absolute path. + sanitizedFiles.push(file); + } + if (operation === 'stage') { // Stage the specified files - await execGitCommand(['add', '--', ...files], worktreePath); + await execGitCommand(['add', '--', ...sanitizedFiles], worktreePath); } else { // Unstage the specified files - await execGitCommand(['reset', 'HEAD', '--', ...files], worktreePath); + await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], worktreePath); } res.json({ success: true, result: { operation, - filesCount: files.length, + filesCount: sanitizedFiles.length, }, }); } catch (error) { diff --git a/apps/server/src/services/auto-mode/facade.ts b/apps/server/src/services/auto-mode/facade.ts index 3a98c131..32957cf2 100644 --- a/apps/server/src/services/auto-mode/facade.ts +++ b/apps/server/src/services/auto-mode/facade.ts @@ -16,6 +16,7 @@ import { exec } from 'child_process'; import { promisify } from 'util'; import type { Feature, PlanningMode, ThinkingLevel } from '@automaker/types'; import { DEFAULT_MAX_CONCURRENCY, stripProviderPrefix } from '@automaker/types'; +import { resolveModelString } from '@automaker/model-resolver'; import { createLogger, loadContextFiles, classifyError } from '@automaker/utils'; import { getFeatureDir } from '@automaker/platform'; import * as secureFs from '../../lib/secure-fs.js'; @@ -208,7 +209,7 @@ export class AutoModeServiceFacade { model?: string, opts?: Record ) => { - const resolvedModel = model || 'claude-sonnet-4-6'; + const resolvedModel = resolveModelString(model, 'claude-sonnet-4-6'); const provider = ProviderFactory.getProviderForModel(resolvedModel); const effectiveBareModel = stripProviderPrefix(resolvedModel); @@ -336,7 +337,7 @@ export class AutoModeServiceFacade { branchName?: string | null; } ) => { - const resolvedModel = model || 'claude-sonnet-4-6'; + const resolvedModel = resolveModelString(model, 'claude-sonnet-4-6'); const provider = ProviderFactory.getProviderForModel(resolvedModel); const effectiveBareModel = stripProviderPrefix(resolvedModel); diff --git a/apps/server/src/services/merge-service.ts b/apps/server/src/services/merge-service.ts index 8e98530d..49cd6aed 100644 --- a/apps/server/src/services/merge-service.ts +++ b/apps/server/src/services/merge-service.ts @@ -160,7 +160,18 @@ export async function performMerge( // If squash merge, need to commit (using safe array-based command) if (options?.squash) { const squashMessage = options?.message || `Merge ${branchName} (squash)`; - await execGitCommand(['commit', '-m', squashMessage], projectPath); + try { + await execGitCommand(['commit', '-m', squashMessage], projectPath); + } catch (commitError: unknown) { + const err = commitError as { message?: string }; + // Emit merge:error so subscribers always receive either merge:success or merge:error + emitter?.emit('merge:error', { + branchName, + targetBranch: mergeTo, + error: err.message || String(commitError), + }); + throw commitError; + } } // Optionally delete the worktree and branch after merging diff --git a/apps/server/src/services/pull-service.ts b/apps/server/src/services/pull-service.ts index fbaf6012..d21e2c7d 100644 --- a/apps/server/src/services/pull-service.ts +++ b/apps/server/src/services/pull-service.ts @@ -15,9 +15,8 @@ * and cherry-pick-service.ts. */ -import { createLogger } from '@automaker/utils'; +import { createLogger, getErrorMessage } from '@automaker/utils'; import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js'; -import { getErrorMessage } from '../routes/worktree/common.js'; const logger = createLogger('PullService'); diff --git a/apps/server/src/services/stash-service.ts b/apps/server/src/services/stash-service.ts index 829acc94..dd0a8737 100644 --- a/apps/server/src/services/stash-service.ts +++ b/apps/server/src/services/stash-service.ts @@ -14,10 +14,9 @@ * merge-service.ts. */ -import { createLogger } from '@automaker/utils'; +import { createLogger, getErrorMessage } from '@automaker/utils'; import type { EventEmitter } from '../lib/events.js'; import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js'; -import { getErrorMessage, logError } from '../routes/worktree/common.js'; const logger = createLogger('StashService'); @@ -242,7 +241,7 @@ export async function applyOrPop( } catch (error) { const errorMessage = getErrorMessage(error); - logError(error, `Stash ${operation} failed`); + logger.error(`Stash ${operation} failed`, { error: getErrorMessage(error) }); events?.emit('stash:failure', { worktreePath, diff --git a/apps/server/src/services/worktree-branch-service.ts b/apps/server/src/services/worktree-branch-service.ts index fe50da13..ca8aeeaf 100644 --- a/apps/server/src/services/worktree-branch-service.ts +++ b/apps/server/src/services/worktree-branch-service.ts @@ -144,11 +144,11 @@ async function fetchRemotes(cwd: string): Promise { * Parse a remote branch name like "origin/feature-branch" into its parts */ function parseRemoteBranch(branchName: string): { remote: string; branch: string } | null { - const slashIndex = branchName.indexOf('/'); - if (slashIndex === -1) return null; + const lastSlash = branchName.lastIndexOf('/'); + if (lastSlash === -1) return null; return { - remote: branchName.substring(0, slashIndex), - branch: branchName.substring(slashIndex + 1), + remote: branchName.substring(0, lastSlash), + branch: branchName.substring(lastSlash + 1), }; } diff --git a/apps/ui/src/components/ui/git-diff-panel.tsx b/apps/ui/src/components/ui/git-diff-panel.tsx index 461244bf..4fe39cf8 100644 --- a/apps/ui/src/components/ui/git-diff-panel.tsx +++ b/apps/ui/src/components/ui/git-diff-panel.tsx @@ -528,6 +528,12 @@ export function GitDiffPanel({ const handleStageFile = useCallback( async (filePath: string) => { if (!worktreePath && !projectPath) return; + if (enableStaging && useWorktrees && !worktreePath) { + toast.error('Failed to stage file', { + description: 'worktreePath required when useWorktrees is enabled', + }); + return; + } setStagingInProgress((prev) => new Set(prev).add(filePath)); try { const api = getElectronAPI(); @@ -574,13 +580,19 @@ export function GitDiffPanel({ }); } }, - [worktreePath, projectPath, useWorktrees, loadDiffs] + [worktreePath, projectPath, useWorktrees, enableStaging, loadDiffs] ); // Unstage a single file const handleUnstageFile = useCallback( async (filePath: string) => { if (!worktreePath && !projectPath) return; + if (enableStaging && useWorktrees && !worktreePath) { + toast.error('Failed to unstage file', { + description: 'worktreePath required when useWorktrees is enabled', + }); + return; + } setStagingInProgress((prev) => new Set(prev).add(filePath)); try { const api = getElectronAPI(); @@ -627,7 +639,7 @@ export function GitDiffPanel({ }); } }, - [worktreePath, projectPath, useWorktrees, loadDiffs] + [worktreePath, projectPath, useWorktrees, enableStaging, loadDiffs] ); const handleStageAll = useCallback(async () => { 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 a3c0f27a..5381627b 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 @@ -226,6 +226,13 @@ export function CommitWorktreeDialog({ setSelectedFiles(new Set(fileList.map((f) => f.path))); } } + } else { + const errorMsg = result.error ?? 'Failed to load diffs'; + console.warn('Failed to load diffs for commit dialog:', errorMsg); + if (!cancelled) { + setError(errorMsg); + toast.error(errorMsg); + } } } } catch (err) { @@ -480,7 +487,7 @@ export function CommitWorktreeDialog({ > {getStatusLabel(file.status)} - {isStaged && !isUntracked && ( + {isStaged && !isUnstaged && !isUntracked && ( Staged diff --git a/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx index d8788a3d..a01c613e 100644 --- a/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/discard-worktree-changes-dialog.tsx @@ -214,6 +214,8 @@ export function DiscardWorktreeChangesDialog({ } else { if (!cancelled) setError(result.error || 'Failed to fetch diffs'); } + } else { + if (!cancelled) setError('Diff API unavailable'); } } catch (err) { if (cancelled) return;