mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-17 10:03:08 +00:00
Fix concurrency limits and remote branch fetching issues (#788)
* Changes from fix/bug-fixes * feat: Refactor worktree iteration and improve error logging across services * feat: Extract URL/port patterns to module level and fix abort condition * fix: Improve IPv6 loopback handling, select component layout, and terminal UI * feat: Add thinking level defaults and adjust list row padding * Update apps/ui/src/store/app-store.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: Add worktree-aware terminal creation and split options, fix npm security issues from audit * feat: Add tracked remote detection to pull dialog flow * feat: Add merge state tracking to git operations * feat: Improve merge detection and add post-merge action preferences * Update apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update apps/ui/src/components/views/board-view/dialogs/git-pull-dialog.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Pass merge detection info to stash reapplication and handle merge state consistently * fix: Call onPulled callback in merge handlers and add validation checks --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
gsxdsm
@@ -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);
|
||||
@@ -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 }
|
||||
): Promise<number> {
|
||||
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName);
|
||||
return this.concurrencyManager.getRunningCountForWorktree(projectPath, branchName, options);
|
||||
}
|
||||
|
||||
trackFailureAndCheckPauseForProject(
|
||||
|
||||
@@ -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,
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
// ============================================================================
|
||||
@@ -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 {
|
||||
|
||||
@@ -170,17 +170,28 @@ export class ConcurrencyManager {
|
||||
* @param projectPath - The project path
|
||||
* @param branchName - The branch name, or null for main worktree
|
||||
* (features without branchName or matching primary branch)
|
||||
* @param options.autoModeOnly - If true, only count features started by auto mode.
|
||||
* Note: The auto-loop coordinator now counts ALL
|
||||
* running features (not just auto-mode) to ensure
|
||||
* total system load is respected. This option is
|
||||
* retained for other callers that may need filtered counts.
|
||||
* @returns Number of running features for the worktree
|
||||
*/
|
||||
async getRunningCountForWorktree(
|
||||
projectPath: string,
|
||||
branchName: string | null
|
||||
branchName: string | null,
|
||||
options?: { autoModeOnly?: boolean }
|
||||
): Promise<number> {
|
||||
// Get the actual primary branch name for the project
|
||||
const primaryBranch = await this.getCurrentBranch(projectPath);
|
||||
|
||||
let count = 0;
|
||||
for (const [, feature] of this.runningFeatures) {
|
||||
// If autoModeOnly is set, skip manually started features
|
||||
if (options?.autoModeOnly && !feature.isAutoMode) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Filter by project path AND branchName to get accurate worktree-specific count
|
||||
const featureBranch = feature.branchName ?? null;
|
||||
if (branchName === null) {
|
||||
|
||||
@@ -19,6 +19,69 @@ const logger = createLogger('DevServerService');
|
||||
// Maximum scrollback buffer size (characters) - matches TerminalService pattern
|
||||
const MAX_SCROLLBACK_SIZE = 50000; // ~50KB per dev server
|
||||
|
||||
// URL patterns for detecting full URLs from dev server output.
|
||||
// Defined once at module level to avoid reallocation on every call to detectUrlFromOutput.
|
||||
// Ordered from most specific (framework-specific) to least specific.
|
||||
const URL_PATTERNS: Array<{ pattern: RegExp; description: string }> = [
|
||||
// Vite / Nuxt / SvelteKit / Astro / Angular CLI format: "Local: http://..."
|
||||
{
|
||||
pattern: /(?:Local|Network|External):\s+(https?:\/\/[^\s]+)/i,
|
||||
description: 'Vite/Nuxt/SvelteKit/Astro/Angular format',
|
||||
},
|
||||
// Next.js format: "ready - started server on 0.0.0.0:3000, url: http://localhost:3000"
|
||||
// Next.js 14+: "▲ Next.js 14.0.0\n- Local: http://localhost:3000"
|
||||
{
|
||||
pattern: /(?:ready|started server).*?(?:url:\s*)?(https?:\/\/[^\s,]+)/i,
|
||||
description: 'Next.js format',
|
||||
},
|
||||
// Remix format: "started at http://localhost:3000"
|
||||
// Django format: "Starting development server at http://127.0.0.1:8000/"
|
||||
// Rails / Puma: "Listening on http://127.0.0.1:3000"
|
||||
// Generic: "listening at http://...", "available at http://...", "running at http://..."
|
||||
{
|
||||
pattern:
|
||||
/(?:starting|started|listening|running|available|serving|accessible)\s+(?:at|on)\s+(https?:\/\/[^\s,)]+)/i,
|
||||
description: 'Generic "starting/started/listening at" format',
|
||||
},
|
||||
// PHP built-in server: "Development Server (http://localhost:8000) started"
|
||||
{
|
||||
pattern: /(?:server|development server)\s*\(\s*(https?:\/\/[^\s)]+)\s*\)/i,
|
||||
description: 'PHP server format',
|
||||
},
|
||||
// Webpack Dev Server: "Project is running at http://localhost:8080/"
|
||||
{
|
||||
pattern: /(?:project|app|application)\s+(?:is\s+)?running\s+(?:at|on)\s+(https?:\/\/[^\s,]+)/i,
|
||||
description: 'Webpack/generic "running at" format',
|
||||
},
|
||||
// Go / Rust / generic: "Serving on http://...", "Server on http://..."
|
||||
{
|
||||
pattern: /(?:serving|server)\s+(?:on|at)\s+(https?:\/\/[^\s,]+)/i,
|
||||
description: 'Generic "serving on" format',
|
||||
},
|
||||
// Localhost URL with port (conservative - must be localhost/127.0.0.1/[::]/0.0.0.0)
|
||||
// This catches anything that looks like a dev server URL
|
||||
{
|
||||
pattern: /(https?:\/\/(?:localhost|127\.0\.0\.1|\[::\]|0\.0\.0\.0):\d+\S*)/i,
|
||||
description: 'Generic localhost URL with port',
|
||||
},
|
||||
];
|
||||
|
||||
// Port-only patterns for detecting port numbers from dev server output
|
||||
// when a full URL is not present in the output.
|
||||
// Defined once at module level to avoid reallocation on every call to detectUrlFromOutput.
|
||||
const PORT_PATTERNS: Array<{ pattern: RegExp; description: string }> = [
|
||||
// "listening on port 3000", "server on port 3000", "started on port 3000"
|
||||
{
|
||||
pattern: /(?:listening|running|started|serving|available)\s+on\s+port\s+(\d+)/i,
|
||||
description: '"listening on port" format',
|
||||
},
|
||||
// "Port: 3000", "port 3000" (at start of line or after whitespace)
|
||||
{
|
||||
pattern: /(?:^|\s)port[:\s]+(\d{4,5})(?:\s|$|[.,;])/im,
|
||||
description: '"port:" format',
|
||||
},
|
||||
];
|
||||
|
||||
// Throttle output to prevent overwhelming WebSocket under heavy load
|
||||
const OUTPUT_THROTTLE_MS = 4; // ~250fps max update rate for responsive feedback
|
||||
const OUTPUT_BATCH_SIZE = 4096; // Smaller batches for lower latency
|
||||
@@ -105,9 +168,52 @@ class DevServerService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip ANSI escape codes from a string
|
||||
* Dev server output often contains color codes that can interfere with URL detection
|
||||
*/
|
||||
private stripAnsi(str: string): string {
|
||||
// Matches ANSI escape sequences: CSI sequences, OSC sequences, and simple escapes
|
||||
// eslint-disable-next-line no-control-regex
|
||||
return str.replace(/\x1B(?:\[[0-9;]*[a-zA-Z]|\].*?(?:\x07|\x1B\\)|\[[?]?[0-9;]*[hl])/g, '');
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract port number from a URL string.
|
||||
* Returns the explicit port if present, or null if no port is specified.
|
||||
* Default protocol ports (80/443) are intentionally NOT returned to avoid
|
||||
* overwriting allocated dev server ports with protocol defaults.
|
||||
*/
|
||||
private extractPortFromUrl(url: string): number | null {
|
||||
try {
|
||||
const parsed = new URL(url);
|
||||
if (parsed.port) {
|
||||
return parseInt(parsed.port, 10);
|
||||
}
|
||||
return null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect actual server URL from output
|
||||
* Parses stdout/stderr for common URL patterns from dev servers
|
||||
* Parses stdout/stderr for common URL patterns from dev servers.
|
||||
*
|
||||
* Supports detection of URLs from:
|
||||
* - Vite: "Local: http://localhost:5173/"
|
||||
* - Next.js: "ready - started server on 0.0.0.0:3000, url: http://localhost:3000"
|
||||
* - Nuxt: "Local: http://localhost:3000/"
|
||||
* - Remix: "started at http://localhost:3000"
|
||||
* - Astro: "Local http://localhost:4321/"
|
||||
* - SvelteKit: "Local: http://localhost:5173/"
|
||||
* - CRA/Webpack: "On Your Network: http://192.168.1.1:3000"
|
||||
* - Angular: "Local: http://localhost:4200/"
|
||||
* - Express/Fastify/Koa: "Server listening on port 3000"
|
||||
* - Django: "Starting development server at http://127.0.0.1:8000/"
|
||||
* - Rails: "Listening on http://127.0.0.1:3000"
|
||||
* - PHP: "Development Server (http://localhost:8000) started"
|
||||
* - Generic: Any localhost URL with a port
|
||||
*/
|
||||
private detectUrlFromOutput(server: DevServerInfo, content: string): void {
|
||||
// Skip if URL already detected
|
||||
@@ -115,39 +221,95 @@ class DevServerService {
|
||||
return;
|
||||
}
|
||||
|
||||
// Common URL patterns from various dev servers:
|
||||
// - Vite: "Local: http://localhost:5173/"
|
||||
// - Next.js: "ready - started server on 0.0.0.0:3000, url: http://localhost:3000"
|
||||
// - CRA/Webpack: "On Your Network: http://192.168.1.1:3000"
|
||||
// - Generic: Any http:// or https:// URL
|
||||
const urlPatterns = [
|
||||
/(?:Local|Network):\s+(https?:\/\/[^\s]+)/i, // Vite format
|
||||
/(?:ready|started server).*?(?:url:\s*)?(https?:\/\/[^\s,]+)/i, // Next.js format
|
||||
/(https?:\/\/(?:localhost|127\.0\.0\.1|\[::\]):\d+)/i, // Generic localhost URL
|
||||
/(https?:\/\/[^\s<>"{}|\\^`[\]]+)/i, // Any HTTP(S) URL
|
||||
];
|
||||
// Strip ANSI escape codes to prevent color codes from breaking regex matching
|
||||
const cleanContent = this.stripAnsi(content);
|
||||
|
||||
for (const pattern of urlPatterns) {
|
||||
const match = content.match(pattern);
|
||||
// Phase 1: Try to detect a full URL from output
|
||||
// Patterns are defined at module level (URL_PATTERNS) and reused across calls
|
||||
for (const { pattern, description } of URL_PATTERNS) {
|
||||
const match = cleanContent.match(pattern);
|
||||
if (match && match[1]) {
|
||||
const detectedUrl = match[1].trim();
|
||||
// Validate it looks like a reasonable URL
|
||||
let detectedUrl = match[1].trim();
|
||||
// Remove trailing punctuation that might have been captured
|
||||
detectedUrl = detectedUrl.replace(/[.,;:!?)\]}>]+$/, '');
|
||||
|
||||
if (detectedUrl.startsWith('http://') || detectedUrl.startsWith('https://')) {
|
||||
// Normalize 0.0.0.0 to localhost for browser accessibility
|
||||
detectedUrl = detectedUrl.replace(
|
||||
/\/\/0\.0\.0\.0(:\d+)?/,
|
||||
(_, port) => `//localhost${port || ''}`
|
||||
);
|
||||
// Normalize [::] to localhost for browser accessibility
|
||||
detectedUrl = detectedUrl.replace(
|
||||
/\/\/\[::\](:\d+)?/,
|
||||
(_, port) => `//localhost${port || ''}`
|
||||
);
|
||||
// Normalize [::1] (IPv6 loopback) to localhost for browser accessibility
|
||||
detectedUrl = detectedUrl.replace(
|
||||
/\/\/\[::1\](:\d+)?/,
|
||||
(_, port) => `//localhost${port || ''}`
|
||||
);
|
||||
|
||||
server.url = detectedUrl;
|
||||
server.urlDetected = true;
|
||||
logger.info(
|
||||
`Detected actual server URL: ${detectedUrl} (allocated port was ${server.port})`
|
||||
);
|
||||
|
||||
// Update the port to match the detected URL's actual port
|
||||
const detectedPort = this.extractPortFromUrl(detectedUrl);
|
||||
if (detectedPort && detectedPort !== server.port) {
|
||||
logger.info(
|
||||
`Port mismatch: allocated ${server.port}, detected ${detectedPort} from ${description}`
|
||||
);
|
||||
server.port = detectedPort;
|
||||
}
|
||||
|
||||
logger.info(`Detected server URL via ${description}: ${detectedUrl}`);
|
||||
|
||||
// Emit URL update event
|
||||
if (this.emitter) {
|
||||
this.emitter.emit('dev-server:url-detected', {
|
||||
worktreePath: server.worktreePath,
|
||||
url: detectedUrl,
|
||||
port: server.port,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
break;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 2: Try to detect just a port number from output (no full URL)
|
||||
// Some servers only print "listening on port 3000" without a full URL
|
||||
// Patterns are defined at module level (PORT_PATTERNS) and reused across calls
|
||||
for (const { pattern, description } of PORT_PATTERNS) {
|
||||
const match = cleanContent.match(pattern);
|
||||
if (match && match[1]) {
|
||||
const detectedPort = parseInt(match[1], 10);
|
||||
// Sanity check: port should be in a reasonable range
|
||||
if (detectedPort > 0 && detectedPort <= 65535) {
|
||||
const detectedUrl = `http://localhost:${detectedPort}`;
|
||||
server.url = detectedUrl;
|
||||
server.urlDetected = true;
|
||||
|
||||
if (detectedPort !== server.port) {
|
||||
logger.info(
|
||||
`Port mismatch: allocated ${server.port}, detected ${detectedPort} from ${description}`
|
||||
);
|
||||
server.port = detectedPort;
|
||||
}
|
||||
|
||||
logger.info(`Detected server port via ${description}: ${detectedPort} → ${detectedUrl}`);
|
||||
|
||||
// Emit URL update event
|
||||
if (this.emitter) {
|
||||
this.emitter.emit('dev-server:url-detected', {
|
||||
worktreePath: server.worktreePath,
|
||||
url: detectedUrl,
|
||||
port: server.port,
|
||||
timestamp: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -673,6 +835,7 @@ class DevServerService {
|
||||
worktreePath: string;
|
||||
port: number;
|
||||
url: string;
|
||||
urlDetected: boolean;
|
||||
}>;
|
||||
};
|
||||
} {
|
||||
@@ -680,6 +843,7 @@ class DevServerService {
|
||||
worktreePath: s.worktreePath,
|
||||
port: s.port,
|
||||
url: s.url,
|
||||
urlDetected: s.urlDetected,
|
||||
}));
|
||||
|
||||
return {
|
||||
|
||||
@@ -46,6 +46,12 @@ export interface PullResult {
|
||||
conflictSource?: 'pull' | 'stash';
|
||||
conflictFiles?: string[];
|
||||
message?: string;
|
||||
/** Whether the pull resulted in a merge commit (not fast-forward) */
|
||||
isMerge?: boolean;
|
||||
/** Whether the pull was a fast-forward (no merge commit needed) */
|
||||
isFastForward?: boolean;
|
||||
/** Files affected by the merge (only present when isMerge is true) */
|
||||
mergeAffectedFiles?: string[];
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
@@ -178,6 +184,31 @@ function isConflictError(errorOutput: string): boolean {
|
||||
return errorOutput.includes('CONFLICT') || errorOutput.includes('Automatic merge failed');
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine whether the current HEAD commit is a merge commit by checking
|
||||
* whether it has two or more parent hashes.
|
||||
*
|
||||
* Runs `git show -s --pretty=%P HEAD` which prints the parent SHAs separated
|
||||
* by spaces. A merge commit has at least two parents; a regular commit has one.
|
||||
*
|
||||
* @param worktreePath - Path to the git worktree
|
||||
* @returns true if HEAD is a merge commit, false otherwise
|
||||
*/
|
||||
async function isMergeCommit(worktreePath: string): Promise<boolean> {
|
||||
try {
|
||||
const output = await execGitCommand(['show', '-s', '--pretty=%P', 'HEAD'], worktreePath);
|
||||
// Each parent SHA is separated by a space; two or more means it's a merge
|
||||
const parents = output
|
||||
.trim()
|
||||
.split(/\s+/)
|
||||
.filter((p) => p.length > 0);
|
||||
return parents.length >= 2;
|
||||
} catch {
|
||||
// If the check fails for any reason, assume it is not a merge commit
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether an output string indicates a stash conflict.
|
||||
*/
|
||||
@@ -302,10 +333,39 @@ export async function performPull(
|
||||
const pullArgs = upstreamStatus === 'tracking' ? ['pull'] : ['pull', targetRemote, branchName];
|
||||
let pullConflict = false;
|
||||
let pullConflictFiles: string[] = [];
|
||||
|
||||
// Declare merge detection variables before the try block so they are accessible
|
||||
// in the stash reapplication path even when didStash is true.
|
||||
let isMerge = false;
|
||||
let isFastForward = false;
|
||||
let mergeAffectedFiles: string[] = [];
|
||||
|
||||
try {
|
||||
const pullOutput = await execGitCommand(pullArgs, worktreePath);
|
||||
|
||||
const alreadyUpToDate = pullOutput.includes('Already up to date');
|
||||
// Detect fast-forward from git pull output
|
||||
isFastForward = pullOutput.includes('Fast-forward') || pullOutput.includes('fast-forward');
|
||||
// Detect merge by checking whether the new HEAD has two parents (more reliable
|
||||
// than string-matching localised pull output which may not contain 'Merge').
|
||||
isMerge = !alreadyUpToDate && !isFastForward ? await isMergeCommit(worktreePath) : false;
|
||||
|
||||
// If it was a real merge (not fast-forward), get the affected files
|
||||
if (isMerge) {
|
||||
try {
|
||||
// Get files changed in the merge commit
|
||||
const diffOutput = await execGitCommand(
|
||||
['diff', '--name-only', 'HEAD~1', 'HEAD'],
|
||||
worktreePath
|
||||
);
|
||||
mergeAffectedFiles = diffOutput
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter((f: string) => f.trim().length > 0);
|
||||
} catch {
|
||||
// Ignore errors - this is best-effort
|
||||
}
|
||||
}
|
||||
|
||||
// If no stash to reapply, return success
|
||||
if (!didStash) {
|
||||
@@ -317,6 +377,8 @@ export async function performPull(
|
||||
stashed: false,
|
||||
stashRestored: false,
|
||||
message: alreadyUpToDate ? 'Already up to date' : 'Pulled latest changes',
|
||||
...(isMerge ? { isMerge: true, mergeAffectedFiles } : {}),
|
||||
...(isFastForward ? { isFastForward: true } : {}),
|
||||
};
|
||||
}
|
||||
} catch (pullError: unknown) {
|
||||
@@ -374,7 +436,11 @@ export async function performPull(
|
||||
|
||||
// 10. Pull succeeded, now try to reapply stash
|
||||
if (didStash) {
|
||||
return await reapplyStash(worktreePath, branchName);
|
||||
return await reapplyStash(worktreePath, branchName, {
|
||||
isMerge,
|
||||
isFastForward,
|
||||
mergeAffectedFiles,
|
||||
});
|
||||
}
|
||||
|
||||
// Shouldn't reach here, but return a safe default
|
||||
@@ -392,9 +458,21 @@ export async function performPull(
|
||||
*
|
||||
* @param worktreePath - Path to the git worktree
|
||||
* @param branchName - Current branch name
|
||||
* @param mergeInfo - Merge/fast-forward detection info from the pull step
|
||||
* @returns PullResult reflecting stash reapplication status
|
||||
*/
|
||||
async function reapplyStash(worktreePath: string, branchName: string): Promise<PullResult> {
|
||||
async function reapplyStash(
|
||||
worktreePath: string,
|
||||
branchName: string,
|
||||
mergeInfo: { isMerge: boolean; isFastForward: boolean; mergeAffectedFiles: string[] }
|
||||
): Promise<PullResult> {
|
||||
const mergeFields: Partial<PullResult> = {
|
||||
...(mergeInfo.isMerge
|
||||
? { isMerge: true, mergeAffectedFiles: mergeInfo.mergeAffectedFiles }
|
||||
: {}),
|
||||
...(mergeInfo.isFastForward ? { isFastForward: true } : {}),
|
||||
};
|
||||
|
||||
try {
|
||||
await popStash(worktreePath);
|
||||
|
||||
@@ -406,6 +484,7 @@ async function reapplyStash(worktreePath: string, branchName: string): Promise<P
|
||||
hasConflicts: false,
|
||||
stashed: true,
|
||||
stashRestored: true,
|
||||
...mergeFields,
|
||||
message: 'Pulled latest changes and restored your stashed changes.',
|
||||
};
|
||||
} catch (stashPopError: unknown) {
|
||||
@@ -431,6 +510,7 @@ async function reapplyStash(worktreePath: string, branchName: string): Promise<P
|
||||
conflictFiles: stashConflictFiles,
|
||||
stashed: true,
|
||||
stashRestored: false,
|
||||
...mergeFields,
|
||||
message: 'Pull succeeded but reapplying your stashed changes resulted in merge conflicts.',
|
||||
};
|
||||
}
|
||||
@@ -445,6 +525,7 @@ async function reapplyStash(worktreePath: string, branchName: string): Promise<P
|
||||
hasConflicts: false,
|
||||
stashed: true,
|
||||
stashRestored: false,
|
||||
...mergeFields,
|
||||
message:
|
||||
'Pull succeeded but failed to reapply stashed changes. Your changes are still in the stash list.',
|
||||
};
|
||||
|
||||
@@ -9,7 +9,8 @@
|
||||
* 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.
|
||||
* Fetches the latest remote refs before switching to ensure remote branch
|
||||
* references are up-to-date for accurate detection and checkout.
|
||||
*
|
||||
* Extracted from the worktree switch-branch route to improve organization
|
||||
* and testability. Follows the same pattern as pull-service.ts and
|
||||
@@ -57,7 +58,8 @@ const FETCH_TIMEOUT_MS = 30_000;
|
||||
* 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.
|
||||
* continues normally. This is called before the branch switch to
|
||||
* ensure remote refs are up-to-date for branch detection and checkout.
|
||||
*/
|
||||
async function fetchRemotes(cwd: string): Promise<void> {
|
||||
const controller = new AbortController();
|
||||
@@ -66,15 +68,15 @@ async function fetchRemotes(cwd: string): Promise<void> {
|
||||
try {
|
||||
await execGitCommand(['fetch', '--all', '--quiet'], cwd, undefined, controller);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message === 'Process aborted') {
|
||||
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`);
|
||||
}
|
||||
// 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.
|
||||
// Non-fatal: continue with locally available refs regardless of failure type
|
||||
} finally {
|
||||
clearTimeout(timerId);
|
||||
}
|
||||
@@ -126,13 +128,13 @@ async function isRemoteBranch(cwd: string, branchName: string): Promise<boolean>
|
||||
* Perform a full branch switch workflow on the given worktree.
|
||||
*
|
||||
* The workflow:
|
||||
* 1. Get current branch name
|
||||
* 2. Detect remote vs local branch and determine target
|
||||
* 3. Return early if already on target branch
|
||||
* 4. Validate branch existence
|
||||
* 5. Stash local changes if any
|
||||
* 6. Checkout the target branch
|
||||
* 7. Fetch latest from remotes
|
||||
* 1. Fetch latest from all remotes (ensures remote refs are up-to-date)
|
||||
* 2. Get current branch name
|
||||
* 3. Detect remote vs local branch and determine target
|
||||
* 4. Return early if already on target branch
|
||||
* 5. Validate branch existence
|
||||
* 6. Stash local changes if any
|
||||
* 7. Checkout the target branch
|
||||
* 8. Reapply stashed changes (detect conflicts)
|
||||
* 9. Handle error recovery (restore stash if checkout fails)
|
||||
*
|
||||
@@ -149,14 +151,20 @@ export async function performSwitchBranch(
|
||||
// Emit start event
|
||||
events?.emit('switch:start', { worktreePath, branchName });
|
||||
|
||||
// 1. Get current branch
|
||||
// 1. Fetch latest from all remotes before switching
|
||||
// This ensures remote branch refs are up-to-date so that isRemoteBranch()
|
||||
// can detect newly created remote branches and local tracking branches
|
||||
// are aware of upstream changes.
|
||||
await fetchRemotes(worktreePath);
|
||||
|
||||
// 2. Get current branch
|
||||
const currentBranchOutput = await execGitCommand(
|
||||
['rev-parse', '--abbrev-ref', 'HEAD'],
|
||||
worktreePath
|
||||
);
|
||||
const previousBranch = currentBranchOutput.trim();
|
||||
|
||||
// 2. Determine the actual target branch name for checkout
|
||||
// 3. Determine the actual target branch name for checkout
|
||||
let targetBranch = branchName;
|
||||
let isRemote = false;
|
||||
|
||||
@@ -180,7 +188,7 @@ export async function performSwitchBranch(
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Return early if already on the target branch
|
||||
// 4. Return early if already on the target branch
|
||||
if (previousBranch === targetBranch) {
|
||||
events?.emit('switch:done', {
|
||||
worktreePath,
|
||||
@@ -198,7 +206,7 @@ export async function performSwitchBranch(
|
||||
};
|
||||
}
|
||||
|
||||
// 4. Check if target branch exists as a local branch
|
||||
// 5. Check if target branch exists as a local branch
|
||||
if (!isRemote) {
|
||||
if (!(await localBranchExists(worktreePath, branchName))) {
|
||||
events?.emit('switch:error', {
|
||||
@@ -213,7 +221,7 @@ export async function performSwitchBranch(
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Stash local changes if any exist
|
||||
// 6. Stash local changes if any exist
|
||||
const hadChanges = await hasAnyChanges(worktreePath, { excludeWorktreePaths: true });
|
||||
let didStash = false;
|
||||
|
||||
@@ -242,7 +250,7 @@ export async function performSwitchBranch(
|
||||
}
|
||||
|
||||
try {
|
||||
// 6. Switch to the target branch
|
||||
// 7. Switch to the target branch
|
||||
events?.emit('switch:checkout', {
|
||||
worktreePath,
|
||||
targetBranch,
|
||||
@@ -265,9 +273,6 @@ export async function performSwitchBranch(
|
||||
await execGitCommand(['checkout', targetBranch], worktreePath);
|
||||
}
|
||||
|
||||
// 7. Fetch latest from remotes after switching
|
||||
await fetchRemotes(worktreePath);
|
||||
|
||||
// 8. Reapply stashed changes if we stashed earlier
|
||||
let hasConflicts = false;
|
||||
let conflictMessage = '';
|
||||
@@ -347,7 +352,7 @@ export async function performSwitchBranch(
|
||||
};
|
||||
}
|
||||
} catch (checkoutError) {
|
||||
// 9. If checkout failed and we stashed, try to restore the stash
|
||||
// 9. Error recovery: if checkout failed and we stashed, try to restore the stash
|
||||
if (didStash) {
|
||||
const popResult = await popStash(worktreePath);
|
||||
if (popResult.hasConflicts) {
|
||||
|
||||
Reference in New Issue
Block a user