mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-18 22:33:08 +00:00
feat: Add process abort control and improve auth detection
This commit is contained in:
@@ -27,6 +27,9 @@ const logger = createLogger('GitLib');
|
||||
* These are merged on top of the current process environment. Pass
|
||||
* `{ LC_ALL: 'C' }` to force git to emit English output regardless of the
|
||||
* system locale so that text-based output parsing remains reliable.
|
||||
* @param abortController - Optional AbortController to cancel the git process.
|
||||
* When the controller is aborted the underlying process is sent SIGTERM and
|
||||
* the returned promise rejects with an Error whose message is 'Process aborted'.
|
||||
* @returns Promise resolving to stdout output
|
||||
* @throws Error with stderr/stdout message if command fails. The thrown error
|
||||
* also has `stdout` and `stderr` string properties for structured access.
|
||||
@@ -39,6 +42,15 @@ const logger = createLogger('GitLib');
|
||||
* // Force English output for reliable text parsing:
|
||||
* await execGitCommand(['rebase', '--', 'main'], worktreePath, { LC_ALL: 'C' });
|
||||
*
|
||||
* // With a process-level timeout:
|
||||
* const controller = new AbortController();
|
||||
* const timerId = setTimeout(() => controller.abort(), 30_000);
|
||||
* try {
|
||||
* await execGitCommand(['fetch', '--all', '--quiet'], cwd, undefined, controller);
|
||||
* } finally {
|
||||
* clearTimeout(timerId);
|
||||
* }
|
||||
*
|
||||
* // Instead of unsafe:
|
||||
* // await execAsync(`git branch -D ${branchName}`, { cwd });
|
||||
* ```
|
||||
@@ -46,13 +58,15 @@ const logger = createLogger('GitLib');
|
||||
export async function execGitCommand(
|
||||
args: string[],
|
||||
cwd: string,
|
||||
env?: Record<string, string>
|
||||
env?: Record<string, string>,
|
||||
abortController?: AbortController
|
||||
): Promise<string> {
|
||||
const result = await spawnProcess({
|
||||
command: 'git',
|
||||
args,
|
||||
cwd,
|
||||
...(env !== undefined ? { env } : {}),
|
||||
...(abortController !== undefined ? { abortController } : {}),
|
||||
});
|
||||
|
||||
// spawnProcess returns { stdout, stderr, exitCode }
|
||||
|
||||
@@ -8,8 +8,7 @@
|
||||
import { query, type Options, type SDKUserMessage } from '@anthropic-ai/claude-agent-sdk';
|
||||
import { BaseProvider } from './base-provider.js';
|
||||
import { classifyError, getUserFriendlyErrorMessage, createLogger } from '@automaker/utils';
|
||||
|
||||
const logger = createLogger('ClaudeProvider');
|
||||
import { getClaudeAuthIndicators } from '@automaker/platform';
|
||||
import {
|
||||
getThinkingTokenBudget,
|
||||
validateBareModelId,
|
||||
@@ -17,6 +16,14 @@ import {
|
||||
type ClaudeCompatibleProvider,
|
||||
type Credentials,
|
||||
} from '@automaker/types';
|
||||
import type {
|
||||
ExecuteOptions,
|
||||
ProviderMessage,
|
||||
InstallationStatus,
|
||||
ModelDefinition,
|
||||
} from './types.js';
|
||||
|
||||
const logger = createLogger('ClaudeProvider');
|
||||
|
||||
/**
|
||||
* ProviderConfig - Union type for provider configuration
|
||||
@@ -25,12 +32,6 @@ import {
|
||||
* Both share the same connection settings structure.
|
||||
*/
|
||||
type ProviderConfig = ClaudeApiProfile | ClaudeCompatibleProvider;
|
||||
import type {
|
||||
ExecuteOptions,
|
||||
ProviderMessage,
|
||||
InstallationStatus,
|
||||
ModelDefinition,
|
||||
} from './types.js';
|
||||
|
||||
// System vars are always passed from process.env regardless of profile
|
||||
const SYSTEM_ENV_VARS = ['PATH', 'HOME', 'SHELL', 'TERM', 'USER', 'LANG', 'LC_ALL'];
|
||||
@@ -240,7 +241,7 @@ export class ClaudeProvider extends BaseProvider {
|
||||
promptPayload = (async function* () {
|
||||
const multiPartPrompt = {
|
||||
type: 'user' as const,
|
||||
session_id: '',
|
||||
session_id: sdkSessionId || undefined,
|
||||
message: {
|
||||
role: 'user' as const,
|
||||
content: prompt,
|
||||
@@ -313,13 +314,37 @@ export class ClaudeProvider extends BaseProvider {
|
||||
*/
|
||||
async detectInstallation(): Promise<InstallationStatus> {
|
||||
// Claude SDK is always available since it's a dependency
|
||||
const hasApiKey = !!process.env.ANTHROPIC_API_KEY;
|
||||
// Check all four supported auth methods, mirroring the logic in buildEnv():
|
||||
// 1. ANTHROPIC_API_KEY environment variable
|
||||
// 2. ANTHROPIC_AUTH_TOKEN environment variable
|
||||
// 3. credentials?.apiKeys?.anthropic (credentials file, checked via platform indicators)
|
||||
// 4. Claude Max CLI OAuth (SDK handles this automatically; detected via getClaudeAuthIndicators)
|
||||
const hasEnvApiKey = !!process.env.ANTHROPIC_API_KEY;
|
||||
const hasEnvAuthToken = !!process.env.ANTHROPIC_AUTH_TOKEN;
|
||||
|
||||
// Check credentials file and CLI OAuth indicators (same sources used by buildEnv)
|
||||
let hasCredentialsApiKey = false;
|
||||
let hasCliOAuth = false;
|
||||
try {
|
||||
const indicators = await getClaudeAuthIndicators();
|
||||
hasCredentialsApiKey = !!indicators.credentials?.hasApiKey;
|
||||
hasCliOAuth = !!(
|
||||
indicators.credentials?.hasOAuthToken ||
|
||||
indicators.hasStatsCacheWithActivity ||
|
||||
(indicators.hasSettingsFile && indicators.hasProjectsSessions)
|
||||
);
|
||||
} catch {
|
||||
// If we can't check indicators, fall back to env vars only
|
||||
}
|
||||
|
||||
const hasApiKey = hasEnvApiKey || hasCredentialsApiKey;
|
||||
const authenticated = hasEnvApiKey || hasEnvAuthToken || hasCredentialsApiKey || hasCliOAuth;
|
||||
|
||||
const status: InstallationStatus = {
|
||||
installed: true,
|
||||
method: 'sdk',
|
||||
hasApiKey,
|
||||
authenticated: hasApiKey,
|
||||
authenticated,
|
||||
};
|
||||
|
||||
return status;
|
||||
|
||||
@@ -884,8 +884,9 @@ export class CodexProvider extends BaseProvider {
|
||||
) {
|
||||
enhancedError =
|
||||
`${errorText}\n\nTip: The model '${options.model}' may not be available on your OpenAI plan. ` +
|
||||
`Some models (like gpt-5.3-codex) require a ChatGPT Pro/Plus subscription and OAuth login via 'codex login'. ` +
|
||||
`Try using a different model (e.g., gpt-5.1 or gpt-5.2), or authenticate with 'codex login' instead of an API key.`;
|
||||
`Available models include: ${CODEX_MODELS.map((m) => m.id).join(', ')}. ` +
|
||||
`Some models require a ChatGPT Pro/Plus subscription—authenticate with 'codex login' instead of an API key. ` +
|
||||
`For the current list of compatible models, visit https://platform.openai.com/docs/models.`;
|
||||
} else if (
|
||||
errorLower.includes('stream disconnected') ||
|
||||
errorLower.includes('stream ended') ||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
* POST /stage-files endpoint - Stage or unstage files in the main project
|
||||
*/
|
||||
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import type { Request, Response } from 'express';
|
||||
import { getErrorMessage, logError } from '../common.js';
|
||||
@@ -24,7 +25,7 @@ export function createStageFilesHandler() {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!files || files.length === 0) {
|
||||
if (!Array.isArray(files) || files.length === 0) {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: 'files array required and must not be empty',
|
||||
@@ -32,6 +33,16 @@ export function createStageFilesHandler() {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const file of files) {
|
||||
if (typeof file !== 'string' || file.trim() === '') {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: 'Each element of files must be a non-empty string',
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (operation !== 'stage' && operation !== 'unstage') {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
@@ -40,8 +51,23 @@ export function createStageFilesHandler() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve the canonical (symlink-dereferenced) project path so that
|
||||
// startsWith(base) reliably prevents symlink traversal attacks.
|
||||
// If projectPath does not exist or is unreadable, realpath rejects and
|
||||
// we return a 400 instead of letting the error propagate as a 500.
|
||||
let canonicalRoot: string;
|
||||
try {
|
||||
canonicalRoot = await fs.promises.realpath(projectPath);
|
||||
} catch {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: `Invalid projectPath (non-existent or unreadable): ${projectPath}`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Validate and sanitize each file path to prevent path traversal attacks
|
||||
const base = path.resolve(projectPath) + path.sep;
|
||||
const base = path.resolve(canonicalRoot) + path.sep;
|
||||
const sanitizedFiles: string[] = [];
|
||||
for (const file of files) {
|
||||
// Reject absolute paths
|
||||
@@ -61,8 +87,8 @@ export function createStageFilesHandler() {
|
||||
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)) {
|
||||
const resolved = path.resolve(path.join(canonicalRoot, file));
|
||||
if (resolved !== path.resolve(canonicalRoot) && !resolved.startsWith(base)) {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: `Invalid file path (outside project directory): ${file}`,
|
||||
@@ -73,9 +99,9 @@ export function createStageFilesHandler() {
|
||||
}
|
||||
|
||||
if (operation === 'stage') {
|
||||
await execGitCommand(['add', '--', ...sanitizedFiles], projectPath);
|
||||
await execGitCommand(['add', '--', ...sanitizedFiles], canonicalRoot);
|
||||
} else {
|
||||
await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], projectPath);
|
||||
await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], canonicalRoot);
|
||||
}
|
||||
|
||||
res.json({
|
||||
|
||||
@@ -9,11 +9,9 @@
|
||||
* 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';
|
||||
import { stageFiles, StageFilesValidationError } from '../../../services/stage-files-service.js';
|
||||
|
||||
export function createStageFilesHandler() {
|
||||
return async (req: Request, res: Response): Promise<void> => {
|
||||
@@ -32,7 +30,7 @@ export function createStageFilesHandler() {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!files || files.length === 0) {
|
||||
if (!Array.isArray(files) || files.length === 0) {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: 'files array required and must not be empty',
|
||||
@@ -40,6 +38,16 @@ export function createStageFilesHandler() {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const file of files) {
|
||||
if (typeof file !== 'string' || file.trim() === '') {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
error: 'Each element of files must be a non-empty string',
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (operation !== 'stage' && operation !== 'unstage') {
|
||||
res.status(400).json({
|
||||
success: false,
|
||||
@@ -48,73 +56,17 @@ 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', '--', ...sanitizedFiles], worktreePath);
|
||||
} else {
|
||||
// Unstage the specified files
|
||||
await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], worktreePath);
|
||||
}
|
||||
const result = await stageFiles(worktreePath, files, operation);
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
result: {
|
||||
operation,
|
||||
filesCount: sanitizedFiles.length,
|
||||
},
|
||||
result,
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof StageFilesValidationError) {
|
||||
res.status(400).json({ success: false, error: error.message });
|
||||
return;
|
||||
}
|
||||
logError(error, `${(req.body as { operation?: string })?.operation ?? 'stage'} files failed`);
|
||||
res.status(500).json({ success: false, error: getErrorMessage(error) });
|
||||
}
|
||||
|
||||
@@ -182,23 +182,18 @@ export class AutoModeServiceFacade {
|
||||
return facadeInstance;
|
||||
};
|
||||
|
||||
// PipelineOrchestrator - runAgentFn is a stub; routes use AutoModeService directly
|
||||
const pipelineOrchestrator = new PipelineOrchestrator(
|
||||
eventBus,
|
||||
featureStateManager,
|
||||
agentExecutor,
|
||||
testRunnerService,
|
||||
worktreeResolver,
|
||||
concurrencyManager,
|
||||
settingsService,
|
||||
// Callbacks
|
||||
(pPath, featureId, status) =>
|
||||
featureStateManager.updateFeatureStatus(pPath, featureId, status),
|
||||
loadContextFiles,
|
||||
buildFeaturePrompt,
|
||||
(pPath, featureId, useWorktrees, _isAutoMode, _model, opts) =>
|
||||
getFacade().executeFeature(featureId, useWorktrees, false, undefined, opts),
|
||||
// runAgentFn - delegates to AgentExecutor
|
||||
/**
|
||||
* Shared agent-run helper used by both PipelineOrchestrator and ExecutionService.
|
||||
*
|
||||
* Resolves the model string, looks up the custom provider/credentials via
|
||||
* getProviderByModelId, then delegates to agentExecutor.execute with the
|
||||
* full payload. The opts parameter uses an index-signature union so it
|
||||
* accepts both the typed ExecutionService opts object and the looser
|
||||
* Record<string, unknown> used by PipelineOrchestrator without requiring
|
||||
* type casts at the call sites.
|
||||
*/
|
||||
const createRunAgentFn =
|
||||
() =>
|
||||
async (
|
||||
workDir: string,
|
||||
featureId: string,
|
||||
@@ -207,8 +202,17 @@ export class AutoModeServiceFacade {
|
||||
pPath: string,
|
||||
imagePaths?: string[],
|
||||
model?: string,
|
||||
opts?: Record<string, unknown>
|
||||
) => {
|
||||
opts?: {
|
||||
planningMode?: PlanningMode;
|
||||
requirePlanApproval?: boolean;
|
||||
previousContent?: string;
|
||||
systemPrompt?: string;
|
||||
autoLoadClaudeMd?: boolean;
|
||||
thinkingLevel?: ThinkingLevel;
|
||||
branchName?: string | null;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
): Promise<void> => {
|
||||
const resolvedModel = resolveModelString(model, 'claude-sonnet-4-6');
|
||||
const provider = ProviderFactory.getProviderForModel(resolvedModel);
|
||||
const effectiveBareModel = stripProviderPrefix(resolvedModel);
|
||||
@@ -218,7 +222,7 @@ export class AutoModeServiceFacade {
|
||||
| import('@automaker/types').ClaudeCompatibleProvider
|
||||
| undefined;
|
||||
let credentials: import('@automaker/types').Credentials | undefined;
|
||||
if (resolvedModel && settingsService) {
|
||||
if (settingsService) {
|
||||
const providerResult = await getProviderByModelId(
|
||||
resolvedModel,
|
||||
settingsService,
|
||||
@@ -270,7 +274,25 @@ export class AutoModeServiceFacade {
|
||||
},
|
||||
}
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
// PipelineOrchestrator - runAgentFn delegates to AgentExecutor via shared helper
|
||||
const pipelineOrchestrator = new PipelineOrchestrator(
|
||||
eventBus,
|
||||
featureStateManager,
|
||||
agentExecutor,
|
||||
testRunnerService,
|
||||
worktreeResolver,
|
||||
concurrencyManager,
|
||||
settingsService,
|
||||
// Callbacks
|
||||
(pPath, featureId, status) =>
|
||||
featureStateManager.updateFeatureStatus(pPath, featureId, status),
|
||||
loadContextFiles,
|
||||
buildFeaturePrompt,
|
||||
(pPath, featureId, useWorktrees, _isAutoMode, _model, opts) =>
|
||||
getFacade().executeFeature(featureId, useWorktrees, false, undefined, opts),
|
||||
createRunAgentFn()
|
||||
);
|
||||
|
||||
// AutoLoopCoordinator - ALWAYS create new with proper execution callbacks
|
||||
@@ -312,92 +334,13 @@ export class AutoModeServiceFacade {
|
||||
async (pPath) => featureLoader.getAll(pPath)
|
||||
);
|
||||
|
||||
// ExecutionService - runAgentFn calls AgentExecutor.execute
|
||||
// ExecutionService - runAgentFn delegates to AgentExecutor via shared helper
|
||||
const executionService = new ExecutionService(
|
||||
eventBus,
|
||||
concurrencyManager,
|
||||
worktreeResolver,
|
||||
settingsService,
|
||||
// runAgentFn - delegates to AgentExecutor
|
||||
async (
|
||||
workDir: string,
|
||||
featureId: string,
|
||||
prompt: string,
|
||||
abortController: AbortController,
|
||||
pPath: string,
|
||||
imagePaths?: string[],
|
||||
model?: string,
|
||||
opts?: {
|
||||
projectPath?: string;
|
||||
planningMode?: PlanningMode;
|
||||
requirePlanApproval?: boolean;
|
||||
systemPrompt?: string;
|
||||
autoLoadClaudeMd?: boolean;
|
||||
thinkingLevel?: ThinkingLevel;
|
||||
branchName?: string | null;
|
||||
}
|
||||
) => {
|
||||
const resolvedModel = resolveModelString(model, 'claude-sonnet-4-6');
|
||||
const provider = ProviderFactory.getProviderForModel(resolvedModel);
|
||||
const effectiveBareModel = stripProviderPrefix(resolvedModel);
|
||||
|
||||
// Resolve custom provider (GLM, MiniMax, etc.) for baseUrl and credentials
|
||||
let claudeCompatibleProvider:
|
||||
| import('@automaker/types').ClaudeCompatibleProvider
|
||||
| undefined;
|
||||
let credentials: import('@automaker/types').Credentials | undefined;
|
||||
if (resolvedModel && settingsService) {
|
||||
const providerResult = await getProviderByModelId(
|
||||
resolvedModel,
|
||||
settingsService,
|
||||
'[AutoModeFacade]'
|
||||
);
|
||||
if (providerResult.provider) {
|
||||
claudeCompatibleProvider = providerResult.provider;
|
||||
credentials = providerResult.credentials;
|
||||
}
|
||||
}
|
||||
|
||||
await agentExecutor.execute(
|
||||
{
|
||||
workDir,
|
||||
featureId,
|
||||
prompt,
|
||||
projectPath: pPath,
|
||||
abortController,
|
||||
imagePaths,
|
||||
model: resolvedModel,
|
||||
planningMode: opts?.planningMode,
|
||||
requirePlanApproval: opts?.requirePlanApproval,
|
||||
systemPrompt: opts?.systemPrompt,
|
||||
autoLoadClaudeMd: opts?.autoLoadClaudeMd,
|
||||
thinkingLevel: opts?.thinkingLevel,
|
||||
branchName: opts?.branchName,
|
||||
provider,
|
||||
effectiveBareModel,
|
||||
credentials,
|
||||
claudeCompatibleProvider,
|
||||
},
|
||||
{
|
||||
waitForApproval: (fId, projPath) => planApprovalService.waitForApproval(fId, projPath),
|
||||
saveFeatureSummary: (projPath, fId, summary) =>
|
||||
featureStateManager.saveFeatureSummary(projPath, fId, summary),
|
||||
updateFeatureSummary: (projPath, fId, summary) =>
|
||||
featureStateManager.saveFeatureSummary(projPath, fId, summary),
|
||||
buildTaskPrompt: (task, allTasks, taskIndex, planContent, template, feedback) => {
|
||||
let taskPrompt = template
|
||||
.replace(/\{\{taskName\}\}/g, task.description || `Task ${task.id}`)
|
||||
.replace(/\{\{taskIndex\}\}/g, String(taskIndex + 1))
|
||||
.replace(/\{\{totalTasks\}\}/g, String(allTasks.length))
|
||||
.replace(/\{\{taskDescription\}\}/g, task.description || `Task ${task.id}`);
|
||||
if (feedback) {
|
||||
taskPrompt = taskPrompt.replace(/\{\{userFeedback\}\}/g, feedback);
|
||||
}
|
||||
return taskPrompt;
|
||||
},
|
||||
}
|
||||
);
|
||||
},
|
||||
createRunAgentFn(),
|
||||
(context) => pipelineOrchestrator.executePipeline(context),
|
||||
(pPath, featureId, status) =>
|
||||
featureStateManager.updateFeatureStatus(pPath, featureId, status),
|
||||
|
||||
@@ -195,15 +195,11 @@ export async function performMerge(
|
||||
|
||||
// Delete the branch (but not main/master)
|
||||
if (branchName !== 'main' && branchName !== 'master') {
|
||||
if (!isValidBranchName(branchName)) {
|
||||
logger.warn(`Invalid branch name detected, skipping deletion: ${branchName}`);
|
||||
} else {
|
||||
try {
|
||||
await execGitCommand(['branch', '-D', branchName], projectPath);
|
||||
branchDeleted = true;
|
||||
} catch {
|
||||
logger.warn(`Failed to delete branch: ${branchName}`);
|
||||
}
|
||||
try {
|
||||
await execGitCommand(['branch', '-D', branchName], projectPath);
|
||||
branchDeleted = true;
|
||||
} catch {
|
||||
logger.warn(`Failed to delete branch: ${branchName}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
*/
|
||||
|
||||
import { createLogger, getErrorMessage } from '@automaker/utils';
|
||||
import { getConflictFiles } from '@automaker/git-utils';
|
||||
import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js';
|
||||
|
||||
const logger = createLogger('PullService');
|
||||
@@ -118,7 +119,7 @@ export async function stashChanges(worktreePath: string, branchName: string): Pr
|
||||
* @returns The stdout from stash pop
|
||||
*/
|
||||
export async function popStash(worktreePath: string): Promise<string> {
|
||||
return await execGitCommand(['stash', 'pop'], worktreePath);
|
||||
return await execGitCommandWithLockRetry(['stash', 'pop'], worktreePath);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -129,7 +130,7 @@ export async function popStash(worktreePath: string): Promise<string> {
|
||||
*/
|
||||
async function tryPopStash(worktreePath: string): Promise<boolean> {
|
||||
try {
|
||||
await execGitCommand(['stash', 'pop'], worktreePath);
|
||||
await execGitCommandWithLockRetry(['stash', 'pop'], worktreePath);
|
||||
return true;
|
||||
} catch (stashPopError) {
|
||||
// Stash pop failed - leave it in stash list for manual recovery
|
||||
@@ -141,6 +142,14 @@ async function tryPopStash(worktreePath: string): Promise<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of the upstream/remote branch check.
|
||||
* - 'tracking': the branch has a configured upstream tracking ref
|
||||
* - 'remote': no tracking ref, but the remote branch exists
|
||||
* - 'none': neither a tracking ref nor a remote branch was found
|
||||
*/
|
||||
export type UpstreamStatus = 'tracking' | 'remote' | 'none';
|
||||
|
||||
/**
|
||||
* Check whether the branch has an upstream tracking ref, or whether
|
||||
* the remote branch exists.
|
||||
@@ -148,48 +157,27 @@ async function tryPopStash(worktreePath: string): Promise<boolean> {
|
||||
* @param worktreePath - Path to the git worktree
|
||||
* @param branchName - Current branch name
|
||||
* @param remote - Remote name
|
||||
* @returns true if upstream or remote branch exists
|
||||
* @returns UpstreamStatus indicating tracking ref, remote branch, or neither
|
||||
*/
|
||||
export async function hasUpstreamOrRemoteBranch(
|
||||
worktreePath: string,
|
||||
branchName: string,
|
||||
remote: string
|
||||
): Promise<boolean> {
|
||||
): Promise<UpstreamStatus> {
|
||||
try {
|
||||
await execGitCommand(['rev-parse', '--abbrev-ref', `${branchName}@{upstream}`], worktreePath);
|
||||
return true;
|
||||
return 'tracking';
|
||||
} catch {
|
||||
// No upstream tracking - check if the remote branch exists
|
||||
try {
|
||||
await execGitCommand(['rev-parse', '--verify', `${remote}/${branchName}`], worktreePath);
|
||||
return true;
|
||||
return 'remote';
|
||||
} catch {
|
||||
return false;
|
||||
return 'none';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the list of files with unresolved merge conflicts.
|
||||
*
|
||||
* @param worktreePath - Path to the git worktree
|
||||
* @returns Array of file paths with conflicts
|
||||
*/
|
||||
export async function getConflictFiles(worktreePath: string): Promise<string[]> {
|
||||
try {
|
||||
const diffOutput = await execGitCommand(
|
||||
['diff', '--name-only', '--diff-filter=U'],
|
||||
worktreePath
|
||||
);
|
||||
return diffOutput
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter((f) => f.trim().length > 0);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether an error output string indicates a merge conflict.
|
||||
*/
|
||||
@@ -233,7 +221,15 @@ export async function performPull(
|
||||
const stashIfNeeded = options?.stashIfNeeded ?? false;
|
||||
|
||||
// 1. Get current branch name
|
||||
const branchName = await getCurrentBranch(worktreePath);
|
||||
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') {
|
||||
@@ -254,7 +250,16 @@ export async function performPull(
|
||||
}
|
||||
|
||||
// 4. Check for local changes
|
||||
const { hasLocalChanges, localChangedFiles } = await getLocalChanges(worktreePath);
|
||||
let hasLocalChanges: boolean;
|
||||
let localChangedFiles: string[];
|
||||
try {
|
||||
({ hasLocalChanges, localChangedFiles } = await getLocalChanges(worktreePath));
|
||||
} catch (err) {
|
||||
return {
|
||||
success: false,
|
||||
error: `Failed to get local changes: ${getErrorMessage(err)}`,
|
||||
};
|
||||
}
|
||||
|
||||
// 5. If there are local changes and stashIfNeeded is not requested, return info
|
||||
if (hasLocalChanges && !stashIfNeeded) {
|
||||
@@ -284,8 +289,8 @@ export async function performPull(
|
||||
}
|
||||
|
||||
// 7. Verify upstream tracking or remote branch exists
|
||||
const hasUpstream = await hasUpstreamOrRemoteBranch(worktreePath, branchName, targetRemote);
|
||||
if (!hasUpstream) {
|
||||
const upstreamStatus = await hasUpstreamOrRemoteBranch(worktreePath, branchName, targetRemote);
|
||||
if (upstreamStatus === 'none') {
|
||||
let stashRecoveryFailed = false;
|
||||
if (didStash) {
|
||||
const stashPopped = await tryPopStash(worktreePath);
|
||||
@@ -294,15 +299,18 @@ export async function performPull(
|
||||
return {
|
||||
success: false,
|
||||
error: `Branch '${branchName}' has no upstream branch on remote '${targetRemote}'. Push it first or set upstream with: git branch --set-upstream-to=${targetRemote}/${branchName}${stashRecoveryFailed ? ' Local changes remain stashed and need manual recovery (run: git stash pop).' : ''}`,
|
||||
stashRecoveryFailed: stashRecoveryFailed || undefined,
|
||||
stashRecoveryFailed: stashRecoveryFailed ? stashRecoveryFailed : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
// 8. Pull latest changes
|
||||
// When the branch has a configured upstream tracking ref, let Git use it automatically.
|
||||
// When only the remote branch exists (no tracking ref), explicitly specify remote and branch.
|
||||
const pullArgs = upstreamStatus === 'tracking' ? ['pull'] : ['pull', targetRemote, branchName];
|
||||
let pullConflict = false;
|
||||
let pullConflictFiles: string[] = [];
|
||||
try {
|
||||
const pullOutput = await execGitCommand(['pull', targetRemote, branchName], worktreePath);
|
||||
const pullOutput = await execGitCommand(pullArgs, worktreePath);
|
||||
|
||||
const alreadyUpToDate = pullOutput.includes('Already up to date');
|
||||
|
||||
@@ -339,14 +347,14 @@ export async function performPull(
|
||||
return {
|
||||
success: false,
|
||||
error: `Branch '${branchName}' has no upstream branch. Push it first or set upstream with: git branch --set-upstream-to=${targetRemote}/${branchName}${stashRecoveryFailed ? ' Local changes remain stashed and need manual recovery (run: git stash pop).' : ''}`,
|
||||
stashRecoveryFailed: stashRecoveryFailed || undefined,
|
||||
stashRecoveryFailed: stashRecoveryFailed ? stashRecoveryFailed : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
success: false,
|
||||
error: `${errorMsg}${stashRecoveryFailed ? ' Local changes remain stashed and need manual recovery (run: git stash pop).' : ''}`,
|
||||
stashRecoveryFailed: stashRecoveryFailed || undefined,
|
||||
stashRecoveryFailed: stashRecoveryFailed ? stashRecoveryFailed : undefined,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -391,27 +399,9 @@ export async function performPull(
|
||||
*/
|
||||
async function reapplyStash(worktreePath: string, branchName: string): Promise<PullResult> {
|
||||
try {
|
||||
const stashPopOutput = await popStash(worktreePath);
|
||||
const stashPopCombined = stashPopOutput || '';
|
||||
await popStash(worktreePath);
|
||||
|
||||
// Check if stash pop had conflicts
|
||||
if (isStashConflict(stashPopCombined)) {
|
||||
const stashConflictFiles = await getConflictFiles(worktreePath);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
branch: branchName,
|
||||
pulled: true,
|
||||
hasConflicts: true,
|
||||
conflictSource: 'stash',
|
||||
conflictFiles: stashConflictFiles,
|
||||
stashed: true,
|
||||
stashRestored: true, // Stash was applied but with conflicts
|
||||
message: 'Pull succeeded but reapplying your stashed changes resulted in merge conflicts.',
|
||||
};
|
||||
}
|
||||
|
||||
// Stash pop succeeded cleanly
|
||||
// Stash pop succeeded cleanly (popStash throws on non-zero exit)
|
||||
return {
|
||||
success: true,
|
||||
branch: branchName,
|
||||
@@ -426,6 +416,7 @@ async function reapplyStash(worktreePath: string, branchName: string): Promise<P
|
||||
const errorOutput = `${err.stderr || ''} ${err.stdout || ''} ${err.message || ''}`;
|
||||
|
||||
// Check if stash pop failed due to conflicts
|
||||
// The stash remains in the stash list when conflicts occur, so stashRestored is false
|
||||
if (isStashConflict(errorOutput)) {
|
||||
const stashConflictFiles = await getConflictFiles(worktreePath);
|
||||
|
||||
@@ -437,7 +428,7 @@ async function reapplyStash(worktreePath: string, branchName: string): Promise<P
|
||||
conflictSource: 'stash',
|
||||
conflictFiles: stashConflictFiles,
|
||||
stashed: true,
|
||||
stashRestored: true,
|
||||
stashRestored: false,
|
||||
message: 'Pull succeeded but reapplying your stashed changes resulted in merge conflicts.',
|
||||
};
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import fs from 'fs/promises';
|
||||
import path from 'path';
|
||||
import { createLogger } from '@automaker/utils';
|
||||
import { getConflictFiles } from '@automaker/git-utils';
|
||||
import { execGitCommand, getCurrentBranch } from '../lib/git.js';
|
||||
|
||||
const logger = createLogger('RebaseService');
|
||||
@@ -186,24 +187,3 @@ export async function abortRebase(worktreePath: string): Promise<boolean> {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the list of files with unresolved conflicts.
|
||||
*
|
||||
* @param worktreePath - Path to the git worktree
|
||||
* @returns Array of file paths with conflicts
|
||||
*/
|
||||
export async function getConflictFiles(worktreePath: string): Promise<string[]> {
|
||||
try {
|
||||
const diffOutput = await execGitCommand(
|
||||
['diff', '--name-only', '--diff-filter=U'],
|
||||
worktreePath
|
||||
);
|
||||
return diffOutput
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter((f) => f.trim().length > 0);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
109
apps/server/src/services/stage-files-service.ts
Normal file
109
apps/server/src/services/stage-files-service.ts
Normal file
@@ -0,0 +1,109 @@
|
||||
/**
|
||||
* stageFilesService - Path validation and git staging/unstaging operations
|
||||
*
|
||||
* Extracted from createStageFilesHandler to centralise path canonicalization,
|
||||
* path-traversal validation, and git invocation so they can be tested and
|
||||
* reused independently of the HTTP layer.
|
||||
*/
|
||||
|
||||
import path from 'path';
|
||||
import fs from 'fs/promises';
|
||||
import { execGitCommand } from '../lib/git.js';
|
||||
|
||||
/**
|
||||
* Result returned by `stageFiles` on success.
|
||||
*/
|
||||
export interface StageFilesResult {
|
||||
operation: string;
|
||||
filesCount: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Error thrown when one or more file paths fail validation (e.g. absolute
|
||||
* paths, path-traversal attempts, or paths that resolve outside the worktree
|
||||
* root, or when the worktree path itself does not exist).
|
||||
*
|
||||
* Handlers can catch this to return an HTTP 400 response instead of 500.
|
||||
*/
|
||||
export class StageFilesValidationError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'StageFilesValidationError';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the canonical path of the worktree root, validate every file path
|
||||
* against it to prevent path-traversal attacks, and then invoke the
|
||||
* appropriate git command (`add` or `reset`) to stage or unstage the files.
|
||||
*
|
||||
* @param worktreePath - Absolute path to the git worktree root directory.
|
||||
* @param files - Relative file paths to stage or unstage.
|
||||
* @param operation - `'stage'` runs `git add`, `'unstage'` runs `git reset HEAD`.
|
||||
*
|
||||
* @returns An object containing the operation name and the number of files
|
||||
* that were staged/unstaged.
|
||||
*
|
||||
* @throws {StageFilesValidationError} When `worktreePath` is inaccessible or
|
||||
* any entry in `files` fails the path-traversal checks.
|
||||
* @throws {Error} When the underlying git command fails.
|
||||
*/
|
||||
export async function stageFiles(
|
||||
worktreePath: string,
|
||||
files: string[],
|
||||
operation: 'stage' | 'unstage'
|
||||
): Promise<StageFilesResult> {
|
||||
// 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 {
|
||||
throw new StageFilesValidationError('worktreePath does not exist or is not accessible');
|
||||
}
|
||||
|
||||
// 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)) {
|
||||
throw new StageFilesValidationError(
|
||||
`Invalid file path (absolute paths not allowed): ${file}`
|
||||
);
|
||||
}
|
||||
// Reject entries containing '..'
|
||||
if (file.includes('..')) {
|
||||
throw new StageFilesValidationError(
|
||||
`Invalid file path (path traversal not allowed): ${file}`
|
||||
);
|
||||
}
|
||||
// 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)) {
|
||||
throw new StageFilesValidationError(
|
||||
`Invalid file path (outside worktree directory): ${file}`
|
||||
);
|
||||
}
|
||||
// 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', '--', ...sanitizedFiles], worktreePath);
|
||||
} else {
|
||||
// Unstage the specified files
|
||||
await execGitCommand(['reset', 'HEAD', '--', ...sanitizedFiles], worktreePath);
|
||||
}
|
||||
|
||||
return {
|
||||
operation,
|
||||
filesCount: sanitizedFiles.length,
|
||||
};
|
||||
}
|
||||
@@ -129,26 +129,52 @@ async function popStash(
|
||||
}
|
||||
}
|
||||
|
||||
/** Timeout for git fetch operations (30 seconds) */
|
||||
const FETCH_TIMEOUT_MS = 30_000;
|
||||
|
||||
/**
|
||||
* Fetch latest from all remotes (silently, with timeout)
|
||||
* Fetch latest from all remotes (silently, with timeout).
|
||||
*
|
||||
* A process-level timeout is enforced via an AbortController so that a
|
||||
* slow or unresponsive remote does not block the branch-switch flow
|
||||
* indefinitely. Timeout errors are logged and treated as non-fatal
|
||||
* (the same as network-unavailable errors) so the rest of the workflow
|
||||
* continues normally.
|
||||
*/
|
||||
async function fetchRemotes(cwd: string): Promise<void> {
|
||||
const controller = new AbortController();
|
||||
const timerId = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
|
||||
|
||||
try {
|
||||
await execGitCommand(['fetch', '--all', '--quiet'], cwd);
|
||||
} catch {
|
||||
// Ignore fetch errors - we may be offline
|
||||
await execGitCommand(['fetch', '--all', '--quiet'], cwd, undefined, controller);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message === 'Process aborted') {
|
||||
// Fetch timed out - log and continue; callers should not be blocked by a slow remote
|
||||
logger.warn(
|
||||
`fetchRemotes timed out after ${FETCH_TIMEOUT_MS}ms - continuing without latest remote refs`
|
||||
);
|
||||
}
|
||||
// Ignore all fetch errors (timeout or otherwise) - we may be offline or the
|
||||
// remote may be temporarily unavailable. The branch switch itself has
|
||||
// already succeeded at this point.
|
||||
} finally {
|
||||
clearTimeout(timerId);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a remote branch name like "origin/feature-branch" into its parts
|
||||
* Parse a remote branch name like "origin/feature-branch" into its parts.
|
||||
* Splits on the first slash so the remote is the segment before the first '/'
|
||||
* and the branch is everything after it (preserving any subsequent slashes).
|
||||
* For example, "origin/feature/my-branch" → { remote: "origin", branch: "feature/my-branch" }.
|
||||
* Returns null if the input contains no slash.
|
||||
*/
|
||||
function parseRemoteBranch(branchName: string): { remote: string; branch: string } | null {
|
||||
const lastSlash = branchName.lastIndexOf('/');
|
||||
if (lastSlash === -1) return null;
|
||||
const firstSlash = branchName.indexOf('/');
|
||||
if (firstSlash === -1) return null;
|
||||
return {
|
||||
remote: branchName.substring(0, lastSlash),
|
||||
branch: branchName.substring(lastSlash + 1),
|
||||
remote: branchName.substring(0, firstSlash),
|
||||
branch: branchName.substring(firstSlash + 1),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -453,6 +479,12 @@ export async function performSwitchBranch(
|
||||
}
|
||||
// popResult.success === true: stash was cleanly restored, re-throw the checkout error
|
||||
}
|
||||
const checkoutErrorMsg = getErrorMessage(checkoutError);
|
||||
events?.emit('switch:error', {
|
||||
worktreePath,
|
||||
branchName,
|
||||
error: checkoutErrorMsg,
|
||||
});
|
||||
throw checkoutError;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user