ros/automaker
1
0
Fork 0
mirror of https://github.com/AutoMaker-Org/automaker.git synced 2026-03-17 10:03:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: Address review comments, add stage/unstage functionality, conflict resolution improvements, support for Sonnet 4.6

Browse Source
This commit is contained in:
gsxdsm
2026-02-18 18:58:33 -08:00
parent df9a6314da
commit 983eb21faa
66 changed files with 2317 additions and 823 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
apps/server/src/services/agent-executor.ts
View File

@@ -42,6 +42,27 @@ export class AgentExecutor {
private static readonly WRITE_DEBOUNCE_MS = 500;
private static readonly STREAM_HEARTBEAT_MS = 15_000;
/**
* Sanitize a provider error value into clean text.
* Coalesces to string, removes ANSI codes, strips leading "Error:" prefix,
* trims, and returns 'Unknown error' when empty.
*/
private static sanitizeProviderError(input: string | { error?: string } | undefined): string {
let raw: string;
if (typeof input === 'string') {
raw = input;
} else if (input && typeof input === 'object' && typeof input.error === 'string') {
raw = input.error;
} else {
raw = '';
}
const cleaned = raw
.replace(/\x1b\[[0-9;]*m/g, '')
.replace(/^Error:\s*/i, '')
.trim();
return cleaned || 'Unknown error';
}
constructor(
private eventBus: TypedEventBus,
private featureStateManager: FeatureStateManager,
@@ -255,15 +276,7 @@ export class AgentExecutor {
}
}
} else if (msg.type === 'error') {
// Clean the error: strip ANSI codes and the redundant "Error: " prefix
// that CLI providers add. Without this, wrapping in new Error() produces
// "Error: Error: Session not found" (double-prefixed).
const cleanedError =
(msg.error || 'Unknown error')
.replace(/\x1b\[[0-9;]*m/g, '')
.replace(/^Error:\s*/i, '')
.trim() || 'Unknown error';
throw new Error(cleanedError);
throw new Error(AgentExecutor.sanitizeProviderError(msg.error));
} else if (msg.type === 'result' && msg.subtype === 'success') scheduleWrite();
}
await writeToFile();

43
apps/server/src/services/agent-service.ts
View File

@@ -96,6 +96,20 @@ export class AgentService {
await secureFs.mkdir(this.stateDir, { recursive: true });
}
/**
* Detect provider-side session errors (session not found, expired, etc.).
* Used to decide whether to clear a stale sdkSessionId.
*/
private isStaleSessionError(rawErrorText: string): boolean {
const errorLower = rawErrorText.toLowerCase();
return (
errorLower.includes('session not found') ||
errorLower.includes('session expired') ||
errorLower.includes('invalid session') ||
errorLower.includes('no such session')
);
}
/**
* Start or resume a conversation
*/
@@ -195,7 +209,15 @@ export class AgentService {
const resolvedWorkingDirectory = path.resolve(effectiveWorkingDirectory);
// Validate that the working directory is allowed using centralized validation
validateWorkingDirectory(resolvedWorkingDirectory);
try {
validateWorkingDirectory(resolvedWorkingDirectory);
} catch (validationError) {
this.logger.warn(
`Session "${sessionId}": working directory "${resolvedWorkingDirectory}" is not allowed — ` +
`returning null so callers treat it as a missing session. Error: ${(validationError as Error).message}`
);
return null;
}
// Load persisted queue
const promptQueue = await this.loadQueueState(sessionId);
@@ -411,7 +433,7 @@ export class AgentService {
// When using a custom provider (GLM, MiniMax), use resolved Claude model for SDK config
// (thinking level budgets, allowedTools) but we MUST pass the provider's model ID
// (e.g. "GLM-4.7") to the API - not "claude-sonnet-4-20250514" which causes "model not found"
// (e.g. "GLM-4.7") to the API - not "claude-sonnet-4-6" which causes "model not found"
const modelForSdk = providerResolvedModel || model;
const sessionModelForSdk = providerResolvedModel ? undefined : session.model;
@@ -616,14 +638,7 @@ export class AgentService {
// sdkSessionId so the next attempt starts a fresh provider session.
// This handles providers that don't have built-in session recovery
// (unlike OpenCode which auto-retries without the session flag).
const errorLower = rawErrorText.toLowerCase();
if (
session.sdkSessionId &&
(errorLower.includes('session not found') ||
errorLower.includes('session expired') ||
errorLower.includes('invalid session') ||
errorLower.includes('no such session'))
) {
if (session.sdkSessionId && this.isStaleSessionError(rawErrorText)) {
this.logger.info(
`Clearing stale sdkSessionId for session ${sessionId} after provider session error`
);
@@ -699,13 +714,7 @@ export class AgentService {
// Check if the thrown error is a provider-side session error.
// Clear the stale sdkSessionId so the next retry starts fresh.
if (
session.sdkSessionId &&
(thrownErrorMsg.includes('session not found') ||
thrownErrorMsg.includes('session expired') ||
thrownErrorMsg.includes('invalid session') ||
thrownErrorMsg.includes('no such session'))
) {
if (session.sdkSessionId && this.isStaleSessionError(rawThrownMsg)) {
this.logger.info(
`Clearing stale sdkSessionId for session ${sessionId} after thrown session error`
);

8
apps/server/src/services/auto-mode/facade.ts
View File

@@ -208,7 +208,7 @@ export class AutoModeServiceFacade {
model?: string,
opts?: Record<string, unknown>
) => {
const resolvedModel = model || 'claude-sonnet-4-20250514';
const resolvedModel = model || 'claude-sonnet-4-6';
const provider = ProviderFactory.getProviderForModel(resolvedModel);
const effectiveBareModel = stripProviderPrefix(resolvedModel);
@@ -258,7 +258,7 @@ export class AutoModeServiceFacade {
featureStateManager.saveFeatureSummary(projPath, fId, summary),
buildTaskPrompt: (task, allTasks, taskIndex, _planContent, template, feedback) => {
let taskPrompt = template
.replace(/\{\{taskName\}\}/g, task.description)
.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}`);
@@ -336,7 +336,7 @@ export class AutoModeServiceFacade {
branchName?: string | null;
}
) => {
const resolvedModel = model || 'claude-sonnet-4-20250514';
const resolvedModel = model || 'claude-sonnet-4-6';
const provider = ProviderFactory.getProviderForModel(resolvedModel);
const effectiveBareModel = stripProviderPrefix(resolvedModel);
@@ -385,7 +385,7 @@ export class AutoModeServiceFacade {
featureStateManager.saveFeatureSummary(projPath, fId, summary),
buildTaskPrompt: (task, allTasks, taskIndex, planContent, template, feedback) => {
let taskPrompt = template
.replace(/\{\{taskName\}\}/g, task.description)
.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}`);

5
apps/server/src/services/merge-service.ts
View File

@@ -35,7 +35,10 @@ export interface MergeServiceResult {
*/
function isValidBranchName(name: string): boolean {
// First char must be alphanumeric, dot, underscore, or slash (not dash)
return /^[a-zA-Z0-9._/][a-zA-Z0-9._\-/]*$/.test(name) && name.length < 250;
// Reject names containing '..' to prevent git ref traversal
return (
/^[a-zA-Z0-9._/][a-zA-Z0-9._\-/]*$/.test(name) && name.length < 250 && !name.includes('..')
);
}
/**

7
apps/server/src/services/pull-service.ts
View File

@@ -16,7 +16,7 @@
*/
import { createLogger } from '@automaker/utils';
import { execGitCommand } from '../lib/git.js';
import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js';
import { getErrorMessage } from '../routes/worktree/common.js';
const logger = createLogger('PullService');
@@ -106,7 +106,10 @@ export async function getLocalChanges(
*/
export async function stashChanges(worktreePath: string, branchName: string): Promise<void> {
const stashMessage = `automaker-pull-stash: Pre-pull stash on ${branchName}`;
await execGitCommand(['stash', 'push', '--include-untracked', '-m', stashMessage], worktreePath);
await execGitCommandWithLockRetry(
['stash', 'push', '--include-untracked', '-m', stashMessage],
worktreePath
);
}
/**

122
apps/server/src/services/stash-service.ts
View File

@@ -16,7 +16,7 @@
import { createLogger } from '@automaker/utils';
import type { EventEmitter } from '../lib/events.js';
import { execGitCommand } from '../lib/git.js';
import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js';
import { getErrorMessage, logError } from '../routes/worktree/common.js';
const logger = createLogger('StashService');
@@ -105,6 +105,46 @@ function isConflictOutput(output: string): boolean {
return output.includes('CONFLICT') || output.includes('Merge conflict');
}
/**
* Build a conflict result from stash apply/pop, emit events, and return.
* Extracted to avoid duplicating conflict handling in the try and catch paths.
*/
async function handleStashConflicts(
worktreePath: string,
stashIndex: number,
operation: 'apply' | 'pop',
events?: EventEmitter
): Promise<StashApplyResult> {
const conflictFiles = await getConflictedFiles(worktreePath);
events?.emit('stash:conflicts', {
worktreePath,
stashIndex,
operation,
conflictFiles,
});
const result: StashApplyResult = {
success: true,
applied: true,
hasConflicts: true,
conflictFiles,
operation,
stashIndex,
message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} with conflicts. Please resolve the conflicts.`,
};
events?.emit('stash:success', {
worktreePath,
stashIndex,
operation,
hasConflicts: true,
conflictFiles,
});
return result;
}
// ============================================================================
// Main Service Function
// ============================================================================
@@ -164,34 +204,7 @@ export async function applyOrPop(
// 4. Check if the error is a conflict
if (isConflictOutput(combinedOutput)) {
const conflictFiles = await getConflictedFiles(worktreePath);
events?.emit('stash:conflicts', {
worktreePath,
stashIndex,
operation,
conflictFiles,
});
const result: StashApplyResult = {
success: true,
applied: true,
hasConflicts: true,
conflictFiles,
operation,
stashIndex,
message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} with conflicts. Please resolve the conflicts.`,
};
events?.emit('stash:success', {
worktreePath,
stashIndex,
operation,
hasConflicts: true,
conflictFiles,
});
return result;
return handleStashConflicts(worktreePath, stashIndex, operation, events);
}
// 5. Non-conflict git error – re-throw so the outer catch logs and handles it
@@ -205,34 +218,7 @@ export async function applyOrPop(
events?.emit('stash:progress', { worktreePath, stashIndex, operation, output: combinedOutput });
if (isConflictOutput(combinedOutput)) {
const conflictFiles = await getConflictedFiles(worktreePath);
events?.emit('stash:conflicts', {
worktreePath,
stashIndex,
operation,
conflictFiles,
});
const result: StashApplyResult = {
success: true,
applied: true,
hasConflicts: true,
conflictFiles,
operation,
stashIndex,
message: `Stash ${operation === 'pop' ? 'popped' : 'applied'} with conflicts. Please resolve the conflicts.`,
};
events?.emit('stash:success', {
worktreePath,
stashIndex,
operation,
hasConflicts: true,
conflictFiles,
});
return result;
return handleStashConflicts(worktreePath, stashIndex, operation, events);
}
// 7. Clean success
@@ -296,17 +282,20 @@ export async function applyOrPop(
*/
export async function pushStash(
worktreePath: string,
options?: { message?: string; files?: string[] }
options?: { message?: string; files?: string[] },
events?: EventEmitter
): Promise<StashPushResult> {
const message = options?.message;
const files = options?.files;
logger.info(`[StashService] push stash in ${worktreePath}`);
events?.emit('stash:start', { worktreePath, operation: 'push' });
// 1. Check for any changes to stash
const status = await execGitCommand(['status', '--porcelain'], worktreePath);
if (!status.trim()) {
events?.emit('stash:success', { worktreePath, operation: 'push', stashed: false });
return {
success: true,
stashed: false,
@@ -326,13 +315,20 @@ export async function pushStash(
args.push(...files);
}
// 3. Execute stash push
await execGitCommand(args, worktreePath);
// 3. Execute stash push (with automatic index.lock cleanup and retry)
await execGitCommandWithLockRetry(args, worktreePath);
// 4. Get current branch name
const branchOutput = await execGitCommand(['rev-parse', '--abbrev-ref', 'HEAD'], worktreePath);
const branchName = branchOutput.trim();
events?.emit('stash:success', {
worktreePath,
operation: 'push',
stashed: true,
branch: branchName,
});
return {
success: true,
stashed: true,
@@ -445,14 +441,18 @@ export async function listStash(worktreePath: string): Promise<StashListResult>
*/
export async function dropStash(
worktreePath: string,
stashIndex: number
stashIndex: number,
events?: EventEmitter
): Promise<StashDropResult> {
const stashRef = `stash@{${stashIndex}}`;
logger.info(`[StashService] drop ${stashRef} in ${worktreePath}`);
events?.emit('stash:start', { worktreePath, stashIndex, stashRef, operation: 'drop' });
await execGitCommand(['stash', 'drop', stashRef], worktreePath);
events?.emit('stash:success', { worktreePath, stashIndex, stashRef, operation: 'drop' });
return {
success: true,
dropped: true,

47
apps/server/src/services/worktree-branch-service.ts
View File

@@ -16,9 +16,8 @@
* rebase-service.ts.
*/
import { createLogger } from '@automaker/utils';
import { execGitCommand } from '../lib/git.js';
import { getErrorMessage } from '../routes/worktree/common.js';
import { createLogger, getErrorMessage } from '@automaker/utils';
import { execGitCommand, execGitCommandWithLockRetry } from '../lib/git.js';
import type { EventEmitter } from '../lib/events.js';
const logger = createLogger('WorktreeBranchService');
@@ -66,7 +65,11 @@ async function hasAnyChanges(cwd: string): Promise<boolean> {
return true;
});
return lines.length > 0;
} catch {
} catch (err) {
logger.error('hasAnyChanges: execGitCommand failed — returning false', {
cwd,
error: getErrorMessage(err),
});
return false;
}
}
@@ -78,24 +81,11 @@ async function hasAnyChanges(cwd: string): Promise<boolean> {
*/
async function stashChanges(cwd: string, message: string): Promise<boolean> {
try {
// Get stash count before
const beforeOutput = await execGitCommand(['stash', 'list'], cwd);
const countBefore = beforeOutput
.trim()
.split('\n')
.filter((l) => l.trim()).length;
// Stash including untracked files
await execGitCommand(['stash', 'push', '--include-untracked', '-m', message], cwd);
// Get stash count after to verify something was stashed
const afterOutput = await execGitCommand(['stash', 'list'], cwd);
const countAfter = afterOutput
.trim()
.split('\n')
.filter((l) => l.trim()).length;
return countAfter > countBefore;
// Stash including untracked files — a successful execGitCommand is proof
// the stash was created. No need for a post-push listing which can throw
// and incorrectly report a failed stash.
await execGitCommandWithLockRetry(['stash', 'push', '--include-untracked', '-m', message], cwd);
return true;
} catch (error) {
const errorMsg = getErrorMessage(error);
@@ -127,11 +117,8 @@ async function popStash(
cwd: string
): Promise<{ success: boolean; hasConflicts: boolean; error?: string }> {
try {
const stdout = await execGitCommand(['stash', 'pop'], cwd);
// Check for conflict markers in the output
if (stdout.includes('CONFLICT') || stdout.includes('Merge conflict')) {
return { success: false, hasConflicts: true };
}
await execGitCommand(['stash', 'pop'], cwd);
// If execGitCommand succeeds (zero exit code), there are no conflicts
return { success: true, hasConflicts: false };
} catch (error) {
const errorMsg = getErrorMessage(error);
@@ -274,11 +261,9 @@ export async function performSwitchBranch(
};
}
// 4. Check if target branch exists (locally or as remote ref)
// 4. Check if target branch exists as a local branch
if (!isRemote) {
try {
await execGitCommand(['rev-parse', '--verify', branchName], worktreePath);
} catch {
if (!(await localBranchExists(worktreePath, branchName))) {
events?.emit('switch:error', {
worktreePath,
branchName,