Skip to content

fix(worktree): gate sessions on bootstrap readiness#2

Merged
bashrusakh merged 1 commit into
win-artifact-pr-fixes-v2from
fix/worktree-bootstrap-session-gate-pr
Jun 22, 2026
Merged

fix(worktree): gate sessions on bootstrap readiness#2
bashrusakh merged 1 commit into
win-artifact-pr-fixes-v2from
fix/worktree-bootstrap-session-gate-pr

Conversation

@bashrusakh

Copy link
Copy Markdown
Owner

No description provided.

@bashrusakh bashrusakh merged commit d7cdcfb into win-artifact-pr-fixes-v2 Jun 22, 2026
1 check failed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to wait for worktree bootstrapping across various session creation and worktree management flows, refactors session draft materialization into a dedicated function, and adds preflight validation checks to prevent creating candidate directories when a branch is already in use. The review feedback highlights critical error-handling gaps where a failure or timeout during the bootstrap wait can leave orphaned worktrees on disk, recommending try-catch blocks to ensure proper cleanup. It also identifies redundant draft closing and folder assignment logic in the newly introduced draft materialization function.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 1376 to 1381
sessionDirectory = normalizePath(createdWorktree.path)
if (!sessionDirectory) {
throw new Error("Worktree create missing name/path")
}
await waitForWorktreeBootstrap(sessionDirectory)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If waitForWorktreeBootstrap throws an error (e.g., due to a timeout or bootstrap failure), the function will abort immediately. This leaves the newly created worktree (createdWorktree) orphaned on disk and in git without any associated session or cleanup. Wrapping the bootstrap wait in a try-catch block ensures that the worktree is properly cleaned up on failure.

      sessionDirectory = normalizePath(createdWorktree.path)
      if (!sessionDirectory) {
        throw new Error("Worktree create missing name/path")
      }
      try {
        await waitForWorktreeBootstrap(sessionDirectory)
      } catch (error) {
        if (createdWorktree && createdWorktreeProject) {
          const { removeProjectWorktree } = await import("@/lib/worktrees/worktreeManager")
          await removeProjectWorktree(createdWorktreeProject, createdWorktree, { deleteLocalBranch: true }).catch(() => undefined)
        }
        throw error
      }
    }

Comment on lines 859 to +861
if (shouldCreateSession) {
await waitForWorktreeBootstrap(metadata.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If waitForWorktreeBootstrap fails or times out, it throws an error which is caught by the outer try-catch block. However, the newly created worktree is not cleaned up, leaving an orphaned worktree and local branch. Wrapping the bootstrap wait in a try-catch block to clean up the worktree on failure prevents resource leaks.

Suggested change
if (shouldCreateSession) {
await waitForWorktreeBootstrap(metadata.path);
if (shouldCreateSession) {
try {
await waitForWorktreeBootstrap(metadata.path);
} catch (error) {
const { removeProjectWorktree } = await import('@/lib/worktrees/worktreeManager');
await removeProjectWorktree(projectRef, metadata, { deleteLocalBranch: true }).catch(() => undefined);
throw error;
}

kind,
};

await waitForWorktreeBootstrap(metadata.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If waitForWorktreeBootstrap fails or times out, the error is propagated to the caller, but the newly created worktree is not cleaned up. Wrapping the bootstrap wait in a try-catch block to clean up the worktree on failure prevents resource leaks.

    try {
      await waitForWorktreeBootstrap(metadata.path);
    } catch (error) {
      await removeProjectWorktree(projectRef, metadata, { deleteLocalBranch: true }).catch(() => undefined);
      throw error;
    }

kind,
};

await waitForWorktreeBootstrap(metadata.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If waitForWorktreeBootstrap fails or times out, the error is propagated to the caller, but the newly created worktree is not cleaned up. Wrapping the bootstrap wait in a try-catch block to clean up the worktree on failure prevents resource leaks.

      try {
        await waitForWorktreeBootstrap(metadata.path);
      } catch (error) {
        await removeProjectWorktree(projectRef, metadata, { deleteLocalBranch: true }).catch(() => undefined);
        throw error;
      }

kind: 'standard' as const,
};

await waitForWorktreeBootstrap(worktreeMetadata.path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If waitForWorktreeBootstrap fails or times out, the error is caught by the outer try-catch block, but the newly created worktree is not cleaned up, leaving orphaned worktrees on disk for any failed runs. Wrapping the bootstrap wait in a nested try-catch block to clean up the worktree on failure prevents resource leaks.

Suggested change
await waitForWorktreeBootstrap(worktreeMetadata.path);
try {
await waitForWorktreeBootstrap(worktreeMetadata.path);
} catch (err) {
const { removeProjectWorktree } = await import('@/lib/worktrees/worktreeManager');
await removeProjectWorktree(project, worktreeMetadata, { deleteLocalBranch: true }).catch(() => undefined);
throw err;
}

Comment on lines +465 to +473
store.closeNewSessionDraft()
store.setCurrentSession(created.id, createdDirectory)

if (draftTargetFolderId) {
const scopeKey = draftDirectoryOverride || created.directory || null
if (scopeKey) {
useSessionFoldersStore.getState().addSessionToFolder(scopeKey, draftTargetFolderId, created.id)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The call to store.closeNewSessionDraft() and the subsequent logic to add the session to the folder are redundant here. store.createSession already internally retrieves the draft's targetFolderId, closes the draft via closeNewSessionDraft(), and adds the created session to the folder. Removing these redundant calls avoids duplicate state updates and side effects.

Suggested change
store.closeNewSessionDraft()
store.setCurrentSession(created.id, createdDirectory)
if (draftTargetFolderId) {
const scopeKey = draftDirectoryOverride || created.directory || null
if (scopeKey) {
useSessionFoldersStore.getState().addSessionToFolder(scopeKey, draftTargetFolderId, created.id)
}
}
store.setCurrentSession(created.id, createdDirectory)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant