Merge branch 'develop' into feat-gitlab-full-integration
This commit is contained in:
@@ -17,7 +17,7 @@ import {
|
||||
} from './plan-file-utils';
|
||||
import { findTaskWorktree } from '../../worktree-paths';
|
||||
import { projectStore } from '../../project-store';
|
||||
import { getIsolatedGitEnv } from '../../utils/git-isolation';
|
||||
import { getIsolatedGitEnv, detectWorktreeBranch } from '../../utils/git-isolation';
|
||||
|
||||
/**
|
||||
* Atomic file write to prevent TOCTOU race conditions.
|
||||
@@ -597,21 +597,13 @@ export function registerTaskExecutionHandlers(
|
||||
console.warn(`[TASK_UPDATE_STATUS] Cleaning up worktree for task ${taskId} (user confirmed)`);
|
||||
try {
|
||||
// Get the branch name before removing the worktree
|
||||
let branch = '';
|
||||
let usingFallbackBranch = false;
|
||||
try {
|
||||
branch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], {
|
||||
cwd: worktreePath,
|
||||
encoding: 'utf-8',
|
||||
timeout: 30000,
|
||||
env: getIsolatedGitEnv()
|
||||
}).trim();
|
||||
} catch (branchError) {
|
||||
// If we can't get branch name, use the default pattern
|
||||
branch = `auto-claude/${task.specId}`;
|
||||
usingFallbackBranch = true;
|
||||
console.warn(`[TASK_UPDATE_STATUS] Could not get branch name, using fallback pattern: ${branch}`, branchError);
|
||||
}
|
||||
// Use shared utility to validate detected branch matches expected pattern
|
||||
// This prevents deleting wrong branch when worktree is corrupted/orphaned
|
||||
const { branch, usingFallback: usingFallbackBranch } = detectWorktreeBranch(
|
||||
worktreePath,
|
||||
task.specId,
|
||||
{ timeout: 30000, logPrefix: '[TASK_UPDATE_STATUS]' }
|
||||
);
|
||||
|
||||
// Remove the worktree
|
||||
execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], {
|
||||
@@ -637,7 +629,10 @@ export function registerTaskExecutionHandlers(
|
||||
// More concerning - fallback pattern didn't match actual branch
|
||||
console.warn(`[TASK_UPDATE_STATUS] Could not delete branch ${branch} using fallback pattern. Actual branch may still exist and need manual cleanup.`, branchDeleteError);
|
||||
} else {
|
||||
console.warn(`[TASK_UPDATE_STATUS] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`);
|
||||
console.warn(
|
||||
`[TASK_UPDATE_STATUS] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`,
|
||||
branchDeleteError
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ import {
|
||||
findTaskWorktree,
|
||||
} from '../../worktree-paths';
|
||||
import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils';
|
||||
import { getIsolatedGitEnv, refreshGitIndex } from '../../utils/git-isolation';
|
||||
import { getIsolatedGitEnv, detectWorktreeBranch, refreshGitIndex } from '../../utils/git-isolation';
|
||||
import { killProcessGracefully } from '../../platform';
|
||||
|
||||
// Regex pattern for validating git branch names
|
||||
@@ -2588,25 +2588,37 @@ export function registerWorktreeHandlers(
|
||||
|
||||
try {
|
||||
// Get the branch name before removing
|
||||
const branch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], {
|
||||
cwd: worktreePath,
|
||||
encoding: 'utf-8'
|
||||
}).trim();
|
||||
// Use shared utility to validate detected branch matches expected pattern
|
||||
// This prevents deleting wrong branch when worktree is corrupted/orphaned
|
||||
const { branch, usingFallback } = detectWorktreeBranch(
|
||||
worktreePath,
|
||||
task.specId,
|
||||
{ timeout: 30000, logPrefix: '[TASK_WORKTREE_DISCARD]' }
|
||||
);
|
||||
|
||||
// Remove the worktree
|
||||
execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], {
|
||||
cwd: project.path,
|
||||
encoding: 'utf-8'
|
||||
encoding: 'utf-8',
|
||||
env: getIsolatedGitEnv(),
|
||||
timeout: 30000
|
||||
});
|
||||
|
||||
// Delete the branch
|
||||
try {
|
||||
execFileSync(getToolPath('git'), ['branch', '-D', branch], {
|
||||
cwd: project.path,
|
||||
encoding: 'utf-8'
|
||||
encoding: 'utf-8',
|
||||
env: getIsolatedGitEnv(),
|
||||
timeout: 30000
|
||||
});
|
||||
} catch {
|
||||
} catch (branchDeleteError) {
|
||||
// Branch might already be deleted or not exist
|
||||
if (usingFallback) {
|
||||
console.warn(`[TASK_WORKTREE_DISCARD] Could not delete branch ${branch} using fallback pattern. Actual branch may still exist and need manual cleanup.`, branchDeleteError);
|
||||
} else {
|
||||
console.warn(`[TASK_WORKTREE_DISCARD] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`, branchDeleteError);
|
||||
}
|
||||
}
|
||||
|
||||
// Only send status change to backlog if not skipped
|
||||
|
||||
@@ -109,6 +109,79 @@ export function getIsolatedGitSpawnOptions(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Result type for detectWorktreeBranch function.
|
||||
*/
|
||||
export interface WorktreeBranchDetectionResult {
|
||||
/** The branch name to use for deletion */
|
||||
branch: string;
|
||||
/** Whether the fallback branch pattern was used */
|
||||
usingFallback: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detects the branch name in a worktree with safety validation.
|
||||
*
|
||||
* This function prevents a critical bug where git rev-parse in a corrupted/orphaned
|
||||
* worktree can return the main project's current branch instead of the worktree's branch.
|
||||
* It validates the detected branch matches the expected pattern before using it.
|
||||
*
|
||||
* @param worktreePath - Path to the worktree directory
|
||||
* @param specId - The spec ID used to generate the expected branch name
|
||||
* @param options - Optional configuration
|
||||
* @param options.timeout - Timeout in milliseconds for git commands (default: 30000)
|
||||
* @param options.logPrefix - Prefix for log messages (e.g., "[TASK_UPDATE_STATUS]")
|
||||
* @returns Object containing the branch name and whether fallback was used
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* import { detectWorktreeBranch } from './utils/git-isolation';
|
||||
* import { getToolPath } from './cli-tool-manager';
|
||||
*
|
||||
* const { branch, usingFallback } = detectWorktreeBranch(
|
||||
* worktreePath,
|
||||
* task.specId,
|
||||
* { timeout: 30000, logPrefix: '[TASK_WORKTREE_DISCARD]' }
|
||||
* );
|
||||
* ```
|
||||
*/
|
||||
export function detectWorktreeBranch(
|
||||
worktreePath: string,
|
||||
specId: string,
|
||||
options: { timeout?: number; logPrefix?: string } = {}
|
||||
): WorktreeBranchDetectionResult {
|
||||
const { timeout = 30000, logPrefix = '[WORKTREE_BRANCH_DETECTION]' } = options;
|
||||
const expectedBranch = `auto-claude/${specId}`;
|
||||
let branch = expectedBranch;
|
||||
let usingFallback = false;
|
||||
|
||||
try {
|
||||
const detectedBranch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], {
|
||||
cwd: worktreePath,
|
||||
encoding: 'utf-8',
|
||||
timeout,
|
||||
env: getIsolatedGitEnv()
|
||||
}).trim();
|
||||
|
||||
// SECURITY: Use strict exact-match validation (not prefix matching) to prevent
|
||||
// accidentally deleting a different task's auto-claude branch. When git rev-parse
|
||||
// returns an unexpected branch, we MUST fall back to the expected pattern rather
|
||||
// than risking deletion of the wrong branch. This is critical for data safety.
|
||||
if (detectedBranch === expectedBranch) {
|
||||
branch = detectedBranch;
|
||||
} else {
|
||||
console.warn(`${logPrefix} Detected branch '${detectedBranch}' doesn't match expected branch '${expectedBranch}', using fallback: ${expectedBranch}`);
|
||||
usingFallback = true;
|
||||
}
|
||||
} catch (branchError) {
|
||||
// If we can't get branch name, use the default pattern
|
||||
usingFallback = true;
|
||||
console.warn(`${logPrefix} Could not get branch name, using fallback pattern: ${branch}`, branchError);
|
||||
}
|
||||
|
||||
return { branch, usingFallback };
|
||||
}
|
||||
|
||||
/**
|
||||
* Refreshes the git index to ensure accurate status after external commits.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user