Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 3 additions & 17 deletions apps/server/src/routes/auto-mode/routes/run-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,9 @@ export function createRunFeatureHandler(autoModeService: AutoModeServiceCompat)
return;
}

// Check per-worktree capacity before starting
const capacity = await autoModeService.checkWorktreeCapacity(projectPath, featureId);
if (!capacity.hasCapacity) {
const worktreeDesc = capacity.branchName
? `worktree "${capacity.branchName}"`
: 'main worktree';
res.status(429).json({
success: false,
error: `Agent limit reached for ${worktreeDesc} (${capacity.currentAgents}/${capacity.maxAgents}). Wait for running tasks to complete or increase the limit.`,
details: {
currentAgents: capacity.currentAgents,
maxAgents: capacity.maxAgents,
branchName: capacity.branchName,
},
});
return;
}
// Note: No concurrency limit check here. Manual feature starts always run
// immediately and bypass the concurrency limit. Their presence IS counted
// by the auto-loop coordinator when deciding whether to dispatch new auto-mode tasks.

// Start execution in background
// executeFeature derives workDir from feature.branchName
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/routes/git/routes/diffs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export function createDiffsHandler() {
diff: result.diff,
files: result.files,
hasChanges: result.hasChanges,
...(result.mergeState ? { mergeState: result.mergeState } : {}),
});
} catch (innerError) {
logError(innerError, 'Git diff failed');
Expand Down
34 changes: 34 additions & 0 deletions apps/server/src/routes/worktree/routes/checkout-branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,36 @@ import { getErrorMessage, logError, isValidBranchName } from '../common.js';
import { execGitCommand } from '../../../lib/git.js';
import type { EventEmitter } from '../../../lib/events.js';
import { performCheckoutBranch } from '../../../services/checkout-branch-service.js';
import { createLogger } from '@automaker/utils';

const logger = createLogger('CheckoutBranchRoute');

/** Timeout for git fetch operations (30 seconds) */
const FETCH_TIMEOUT_MS = 30_000;

/**
* Fetch latest from all remotes (silently, with timeout).
* Non-fatal: fetch errors are logged and swallowed so the workflow continues.
*/
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, undefined, controller);
} catch (error) {
if (error instanceof Error && error.message === 'Process aborted') {
logger.warn(
`fetchRemotes timed out after ${FETCH_TIMEOUT_MS}ms - continuing without latest remote refs`
);
} else {
logger.warn(`fetchRemotes failed: ${getErrorMessage(error)} - continuing with local refs`);
}
Comment on lines 42 to 49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message for a timed-out fetch operation is quite verbose. It could be shortened to improve readability in logs while still conveying the essential information.

    if (error instanceof Error && error.message === 'Process aborted') {
      logger.warn(`fetchRemotes timed out after ${FETCH_TIMEOUT_MS}ms. Continuing with local refs.`);
    }

// Non-fatal: continue with locally available refs
} finally {
clearTimeout(timerId);
}
}

export function createCheckoutBranchHandler(events?: EventEmitter) {
return async (req: Request, res: Response): Promise<void> => {
Expand Down Expand Up @@ -127,6 +157,10 @@ export function createCheckoutBranchHandler(events?: EventEmitter) {
}

// Original simple flow (no stash handling)
// Fetch latest remote refs before creating the branch so that
// base branch validation works for remote references like "origin/main"
await fetchRemotes(resolvedPath);

const currentBranchOutput = await execGitCommand(
['rev-parse', '--abbrev-ref', 'HEAD'],
resolvedPath
Expand Down
73 changes: 19 additions & 54 deletions apps/server/src/routes/worktree/routes/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import { runInitScript } from '../../../services/init-script-service.js';

const logger = createLogger('Worktree');

/** Timeout for git fetch operations (30 seconds) */
const FETCH_TIMEOUT_MS = 30_000;

const execAsync = promisify(exec);

/**
Expand Down Expand Up @@ -83,41 +86,6 @@ async function findExistingWorktreeForBranch(
}
}

/**
* Detect whether a base branch reference is a remote branch (e.g. "origin/main").
* Returns the remote name if it matches a known remote, otherwise null.
*/
async function detectRemoteBranch(
projectPath: string,
baseBranch: string
): Promise<{ remote: string; branch: string } | null> {
const slashIndex = baseBranch.indexOf('/');
if (slashIndex <= 0) return null;

const possibleRemote = baseBranch.substring(0, slashIndex);

try {
// Check if this is actually a remote name by listing remotes
const stdout = await execGitCommand(['remote'], projectPath);
const remotes = stdout
.trim()
.split('\n')
.map((r: string) => r.trim())
.filter(Boolean);

if (remotes.includes(possibleRemote)) {
return {
remote: possibleRemote,
branch: baseBranch.substring(slashIndex + 1),
};
}
} catch {
// Not a git repo or no remotes — fall through
}

return null;
}

export function createCreateHandler(events: EventEmitter, settingsService?: SettingsService) {
const worktreeService = new WorktreeService();

Expand Down Expand Up @@ -206,26 +174,23 @@ export function createCreateHandler(events: EventEmitter, settingsService?: Sett
// Create worktrees directory if it doesn't exist
await secureFs.mkdir(worktreesDir, { recursive: true });

// If a base branch is specified and it's a remote branch, fetch from that remote first
// This ensures we have the latest refs before creating the worktree
if (baseBranch && baseBranch !== 'HEAD') {
const remoteBranchInfo = await detectRemoteBranch(projectPath, baseBranch);
if (remoteBranchInfo) {
logger.info(
`Fetching from remote "${remoteBranchInfo.remote}" before creating worktree (base: ${baseBranch})`
);
try {
await execGitCommand(
['fetch', remoteBranchInfo.remote, remoteBranchInfo.branch],
projectPath
);
} catch (fetchErr) {
// Non-fatal: log but continue — the ref might already be cached locally
logger.warn(
`Failed to fetch from remote "${remoteBranchInfo.remote}": ${getErrorMessage(fetchErr)}`
);
}
// Fetch latest from all remotes before creating the worktree.
// This ensures remote refs are up-to-date for:
// - Remote base branches (e.g. "origin/main")
// - Existing remote branches being checked out as worktrees
// - Branch existence checks against fresh remote state
logger.info('Fetching from all remotes before creating worktree');
try {
const controller = new AbortController();
const timerId = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
try {
await execGitCommand(['fetch', '--all', '--quiet'], projectPath, undefined, controller);
} finally {
clearTimeout(timerId);
}
} catch (fetchErr) {
// Non-fatal: log but continue — refs might already be cached locally
logger.warn(`Failed to fetch from remotes: ${getErrorMessage(fetchErr)}`);
}

// Check if branch exists (using array arguments to prevent injection)
Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/routes/worktree/routes/diffs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function createDiffsHandler() {
diff: result.diff,
files: result.files,
hasChanges: result.hasChanges,
...(result.mergeState ? { mergeState: result.mergeState } : {}),
});
return;
}
Expand All @@ -55,6 +56,7 @@ export function createDiffsHandler() {
diff: result.diff,
files: result.files,
hasChanges: result.hasChanges,
...(result.mergeState ? { mergeState: result.mergeState } : {}),
});
} catch (innerError) {
// Worktree doesn't exist - fallback to main project path
Expand All @@ -71,6 +73,7 @@ export function createDiffsHandler() {
diff: result.diff,
files: result.files,
hasChanges: result.hasChanges,
...(result.mergeState ? { mergeState: result.mergeState } : {}),
});
} catch (fallbackError) {
logError(fallbackError, 'Fallback to main project also failed');
Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/routes/worktree/routes/pull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ function mapResultToResponse(res: Response, result: PullResult): void {
stashed: result.stashed,
stashRestored: result.stashRestored,
message: result.message,
isMerge: result.isMerge,
isFastForward: result.isFastForward,
mergeAffectedFiles: result.mergeAffectedFiles,
},
});
}
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/routes/worktree/routes/switch-branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* For remote branches (e.g., "origin/feature"), automatically creates a
* local tracking branch and checks it out.
*
* Also fetches the latest remote refs after switching.
* Also fetches the latest remote refs before switching to ensure accurate branch detection.
*
* Git business logic is delegated to worktree-branch-service.ts.
* Events are emitted at key lifecycle points for WebSocket subscribers.
Expand Down
14 changes: 12 additions & 2 deletions apps/server/src/services/auto-loop-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ export class AutoLoopCoordinator {
const { projectPath, branchName } = projectState.config;
while (projectState.isRunning && !projectState.abortController.signal.aborted) {
try {
// Count ALL running features (both auto and manual) against the concurrency limit.
// This ensures auto mode is aware of the total system load and does not over-subscribe
// resources. Manual tasks always bypass the limit and run immediately, but their
// presence is accounted for when deciding whether to dispatch new auto-mode tasks.
const runningCount = await this.getRunningCountForWorktree(projectPath, branchName);
if (runningCount >= projectState.config.maxConcurrency) {
await this.sleep(5000, projectState.abortController.signal);
Expand Down Expand Up @@ -298,11 +302,17 @@ export class AutoLoopCoordinator {
return Array.from(activeProjects);
}

/**
* Get the number of running features for a worktree.
* By default counts ALL running features (both auto-mode and manual).
* Pass `autoModeOnly: true` to count only auto-mode features.
*/
async getRunningCountForWorktree(
projectPath: string,
branchName: string | null
branchName: string | null,
options?: { autoModeOnly?: boolean }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The options parameter is passed directly to concurrencyManager.getRunningCountForWorktree. However, the options object is defined as { autoModeOnly?: boolean } in the getRunningCountForWorktree method signature, but the comment for autoModeOnly states "The auto-loop coordinator now counts ALL running features (not just auto-mode) to ensure total system load is respected." This creates a contradiction. If the coordinator is always counting all features, then passing options with autoModeOnly: true would lead to incorrect behavior or the option is unused by the coordinator itself. It should either be removed from the coordinator's call or clarified why it's passed if it's not intended to filter.

    return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName);

): Promise<number> {
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName);
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName, options);
}
Comment on lines 310 to 316
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, examine the file and the specific lines mentioned
wc -l apps/server/src/services/auto-loop-coordinator.ts

Repository: AutoMaker-Org/automaker

Length of output: 120


🏁 Script executed:

# Read the file to check the method signatures at the referenced lines
cat -n apps/server/src/services/auto-loop-coordinator.ts | sed -n '90,100p;240,280p;310,320p'

Repository: AutoMaker-Org/automaker

Length of output: 2892


🏁 Script executed:

# Search for all invocations of getRunningCountForWorktree
rg -n --type=ts 'getRunningCountForWorktree' -B 1 -A 2

Repository: AutoMaker-Org/automaker

Length of output: 19906


🏁 Script executed:

# Check if there are any calls to getRunningCountForWorktree that might omit the branchName parameter
rg -n --type=ts 'getRunningCountForWorktree\s*\(\s*[^,)]+\s*\)' -v 'branchName'

Repository: AutoMaker-Org/automaker

Length of output: 117


🏁 Script executed:

# Also search the entire codebase (including route handlers and other sources) that might call this method
rg -n 'getRunningCountForWorktree\(' | head -50

Repository: AutoMaker-Org/automaker

Length of output: 3523


🏁 Script executed:

# Check the auto-mode/facade.ts call context
cat -n apps/server/src/services/auto-mode/facade.ts | sed -n '888,896p'

Repository: AutoMaker-Org/automaker

Length of output: 323


🏁 Script executed:

# Search for any route handlers that might call getRunningCountForWorktree
rg -n 'getRunningCountForWorktree' apps/server/src/routes/ 2>/dev/null || echo "No matches in routes/"

Repository: AutoMaker-Org/automaker

Length of output: 88


🏁 Script executed:

# Verify the method signature in ConcurrencyManager for comparison
cat -n apps/server/src/services/concurrency-manager.ts | sed -n '179,185p'

Repository: AutoMaker-Org/automaker

Length of output: 335


Add = null default to branchName parameter for API consistency.

The branchName parameter on getRunningCountForWorktree lacks the = null default present on every other public method in this class (startAutoLoopForProject at line 95, stopAutoLoopForProject at line 242, isAutoLoopRunningForProject at line 261, getAutoLoopConfigForProject at line 274). All current callers pass branchName explicitly, but this inconsistency makes the API surface harder to use and maintains parity with the class's other methods.

🔧 Proposed fix
  async getRunningCountForWorktree(
    projectPath: string,
-   branchName: string | null,
+   branchName: string | null = null,
    options?: { autoModeOnly?: boolean }
  ): Promise<number> {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async getRunningCountForWorktree(
projectPath: string,
branchName: string | null
branchName: string | null,
options?: { autoModeOnly?: boolean }
): Promise<number> {
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName);
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName, options);
}
async getRunningCountForWorktree(
projectPath: string,
branchName: string | null = null,
options?: { autoModeOnly?: boolean }
): Promise<number> {
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName, options);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/services/auto-loop-coordinator.ts` around lines 310 - 316,
The method getRunningCountForWorktree has a branchName parameter missing the
class-wide default of = null; update its signature to declare branchName: string
| null = null so it matches other public methods (e.g., startAutoLoopForProject,
stopAutoLoopForProject, isAutoLoopRunningForProject,
getAutoLoopConfigForProject) and keeps the API consistent — no further logic
changes needed since callers already pass branchName explicitly.


trackFailureAndCheckPauseForProject(
Expand Down
50 changes: 46 additions & 4 deletions apps/server/src/services/auto-mode/facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,23 @@ export class AutoModeServiceFacade {
async (pPath) => featureLoader.getAll(pPath)
);

/**
* Iterate all active worktrees for this project, falling back to the
* main worktree (null) when none are active.
*/
const forEachProjectWorktree = (fn: (branchName: string | null) => void): void => {
const projectWorktrees = autoLoopCoordinator
.getActiveWorktrees()
.filter((w) => w.projectPath === projectPath);
if (projectWorktrees.length === 0) {
fn(null);
} else {
for (const w of projectWorktrees) {
fn(w.branchName);
}
}
};

// ExecutionService - runAgentFn delegates to AgentExecutor via shared helper
const executionService = new ExecutionService(
eventBus,
Expand All @@ -357,11 +374,36 @@ export class AutoModeServiceFacade {
(pPath, featureId) => getFacade().contextExists(featureId),
(pPath, featureId, useWorktrees, _calledInternally) =>
getFacade().resumeFeature(featureId, useWorktrees, _calledInternally),
(errorInfo) =>
autoLoopCoordinator.trackFailureAndCheckPauseForProject(projectPath, null, errorInfo),
(errorInfo) => autoLoopCoordinator.signalShouldPauseForProject(projectPath, null, errorInfo),
(errorInfo) => {
// Track failure against ALL active worktrees for this project.
// The ExecutionService callbacks don't receive branchName, so we
// iterate all active worktrees. Uses a for-of loop (not .some()) to
// ensure every worktree's failure counter is incremented.
let shouldPause = false;
forEachProjectWorktree((branchName) => {
if (
autoLoopCoordinator.trackFailureAndCheckPauseForProject(
projectPath,
branchName,
errorInfo
)
) {
shouldPause = true;
}
});
return shouldPause;
},
(errorInfo) => {
forEachProjectWorktree((branchName) =>
autoLoopCoordinator.signalShouldPauseForProject(projectPath, branchName, errorInfo)
);
},
() => {
/* recordSuccess - no-op */
// Record success to clear failure tracking. This prevents failures
// from accumulating over time and incorrectly pausing auto mode.
forEachProjectWorktree((branchName) =>
autoLoopCoordinator.recordSuccessForProject(projectPath, branchName)
);
},
(_pPath) => getFacade().saveExecutionState(),
loadContextFiles
Expand Down
48 changes: 47 additions & 1 deletion apps/server/src/services/checkout-branch-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Follows the same pattern as worktree-branch-service.ts (performSwitchBranch).
*
* The workflow:
* 0. Fetch latest from all remotes (ensures remote refs are up-to-date)
* 1. Validate inputs (branch name, base branch)
* 2. Get current branch name
* 3. Check if target branch already exists
Expand All @@ -19,11 +20,51 @@
* 7. Handle error recovery (restore stash if checkout fails)
*/

import { getErrorMessage } from '@automaker/utils';
import { createLogger, getErrorMessage } from '@automaker/utils';
import { execGitCommand } from '../lib/git.js';
import type { EventEmitter } from '../lib/events.js';
import { hasAnyChanges, stashChanges, popStash, localBranchExists } from './branch-utils.js';

const logger = createLogger('CheckoutBranchService');

// ============================================================================
// Local Helpers
// ============================================================================

/** Timeout for git fetch operations (30 seconds) */
const FETCH_TIMEOUT_MS = 30_000;

/**
* 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 creation 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. This is called before creating the new branch to
* ensure remote refs are up-to-date when a remote base branch is used.
*/
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, undefined, controller);
} catch (error) {
if (controller.signal.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`
);
} else {
logger.warn(`fetchRemotes failed: ${getErrorMessage(error)} - continuing with local refs`);
}
// Non-fatal: continue with locally available refs regardless of failure type
} finally {
clearTimeout(timerId);
}
}

// ============================================================================
// Types
// ============================================================================
Expand Down Expand Up @@ -78,6 +119,11 @@ export async function performCheckoutBranch(
// Emit start event
events?.emit('switch:start', { worktreePath, branchName, operation: 'checkout' });

// 0. Fetch latest from all remotes before creating the branch
// This ensures remote refs are up-to-date so that base branch validation
// works correctly for remote branch references (e.g. "origin/main").
await fetchRemotes(worktreePath);

// 1. Get current branch
let previousBranch: string;
try {
Expand Down
Loading