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
3 changes: 3 additions & 0 deletions packages/ui/src/components/session/NewWorktreeDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import * as sessionActions from '@/sync/session-actions';
import { useConfigStore } from '@/stores/useConfigStore';
import { validateWorktreeCreate, createWorktree } from '@/lib/worktrees/worktreeManager';
import { withWorktreeUpstreamDefaults } from '@/lib/worktrees/worktreeCreate';
import { waitForWorktreeBootstrap } from '@/lib/worktrees/worktreeBootstrap';
import { getWorktreeSetupCommands } from '@/lib/openchamberConfig';
import { getRootBranch } from '@/lib/worktrees/worktreeStatus';
import { generateBranchSlug } from '@/lib/git/branchNameGenerator';
Expand Down Expand Up @@ -856,6 +857,8 @@ export function NewWorktreeDialog({
let createdSessionId: string | null = null;

if (shouldCreateSession) {
await waitForWorktreeBootstrap(metadata.path);

Comment on lines 859 to +861

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;
}

const sessionTitle = linkedIssue
? `#${linkedIssue.number} ${linkedIssue.title}`.trim()
: linkedPrState
Expand Down
5 changes: 5 additions & 0 deletions packages/ui/src/lib/worktreeSessionCreator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
rejectPendingDraftWorktreeRequest,
resolvePendingDraftWorktreeRequest,
} from '@/lib/worktrees/pendingDraftWorktree';
import { waitForWorktreeBootstrap } from '@/lib/worktrees/worktreeBootstrap';

const normalizePath = (value: string): string => value.replace(/\\/g, '/').replace(/\/+$/, '') || value;

Expand Down Expand Up @@ -417,6 +418,8 @@ export async function createWorktreeSessionForBranch(
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;
    }


// Create the session
const sessionStore = useSessionUIStore.getState();
const session = await sessionStore.createSession(undefined, metadata.path);
Expand Down Expand Up @@ -520,6 +523,8 @@ export async function createWorktreeSessionForNewBranch(
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;
      }


const sessionStore = useSessionUIStore.getState();
const session = await sessionStore.createSession(undefined, metadata.path);
if (!session) {
Expand Down
32 changes: 26 additions & 6 deletions packages/ui/src/stores/useMultiRunStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const registeredDirectories: Array<{ sessionID: string; directory: string }> = [
const ensureChildCalls: Array<{ directory: string; bootstrap?: boolean }> = [];
const worktreeMetadataCalls: Array<{ sessionId: string; path: string }> = [];
const worktreeCreateCalls: Array<{ project: { id?: string; path: string }; args: Record<string, unknown>; options: unknown }> = [];
const worktreeBootstrapWaitCalls: string[] = [];
const operationOrder: string[] = [];
let isGitRepository = false;
const createWorktreeWithDefaultsMock = mock((project: { id?: string; path: string }, args: Record<string, unknown>, options: unknown) => {
worktreeCreateCalls.push({ project, args, options });
Expand Down Expand Up @@ -52,12 +54,15 @@ mock.module('@/lib/opencode/client', () => ({
currentDirectory = previous;
}
},
createSession: async (params?: { title?: string }): Promise<Session> => ({
id: 'ses_multirun',
title: params?.title ?? '',
directory: currentDirectory,
time: { created: 1, updated: 1 },
} as Session),
createSession: async (params?: { title?: string }): Promise<Session> => {
operationOrder.push(`createSession:${currentDirectory}`);
return {
id: 'ses_multirun',
title: params?.title ?? '',
directory: currentDirectory,
time: { created: 1, updated: 1 },
} as Session;
},
},
}));

Expand All @@ -70,6 +75,14 @@ mock.module('@/lib/worktrees/worktreeCreate', () => ({
resolveRootTrackingRemote: mock(() => Promise.resolve(null)),
}));

mock.module('@/lib/worktrees/worktreeBootstrap', () => ({
waitForWorktreeBootstrap: (directory: string) => {
worktreeBootstrapWaitCalls.push(directory);
operationOrder.push(`wait:${directory}`);
return Promise.resolve();
},
}));

mock.module('@/lib/worktrees/worktreeStatus', () => ({
getRootBranch: mock(() => Promise.resolve('main')),
}));
Expand Down Expand Up @@ -139,6 +152,8 @@ describe('useMultiRunStore', () => {
ensureChildCalls.length = 0;
worktreeMetadataCalls.length = 0;
worktreeCreateCalls.length = 0;
worktreeBootstrapWaitCalls.length = 0;
operationOrder.length = 0;
isGitRepository = false;
childState.session = [];
childState.sessionTotal = 0;
Expand Down Expand Up @@ -181,6 +196,11 @@ describe('useMultiRunStore', () => {
expect(worktreeCreateCalls[0]?.project).toEqual({ id: 'project-1', path: '/repo' });
expect(worktreeCreateCalls[0]?.args.returnAfterDirectoryCreated).toBe(true);
expect(worktreeCreateCalls[0]?.options).toEqual({ resolvedRootTrackingRemote: null });
expect(worktreeBootstrapWaitCalls).toEqual(['/repo-worktrees/fix-thing']);
expect(operationOrder).toEqual([
'wait:/repo-worktrees/fix-thing',
'createSession:/repo-worktrees/fix-thing',
]);
expect(registeredDirectories).toEqual([{ sessionID: 'ses_multirun', directory: '/repo-worktrees/fix-thing' }]);
expect(worktreeMetadataCalls).toEqual([{ sessionId: 'ses_multirun', path: '/repo-worktrees/fix-thing' }]);
});
Expand Down
3 changes: 3 additions & 0 deletions packages/ui/src/stores/useMultiRunStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { opencodeClient } from '@/lib/opencode/client';
import { saveWorktreeSetupCommands } from '@/lib/openchamberConfig';
import type { ProjectRef } from '@/lib/worktrees/worktreeManager';
import { createWorktreeWithDefaults, resolveRootTrackingRemote } from '@/lib/worktrees/worktreeCreate';
import { waitForWorktreeBootstrap } from '@/lib/worktrees/worktreeBootstrap';
import { getRootBranch } from '@/lib/worktrees/worktreeStatus';
import { checkIsGitRepository } from '@/lib/gitApi';
import { useDirectoryStore } from './useDirectoryStore';
Expand Down Expand Up @@ -244,6 +245,8 @@ export const useMultiRunStore = create<MultiRunStore>()(
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;
}


const session = await opencodeClient.withDirectory(
worktreeMetadata.path,
() => opencodeClient.createSession({ title: sessionTitle }),
Expand Down
147 changes: 96 additions & 51 deletions packages/ui/src/sync/session-ui-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { markPendingUserSendAnimation } from "@/lib/userSendAnimation"
import { flattenAssistantTextParts } from "@/lib/messages/messageText"
import { composeForkSessionMessage } from "@/lib/messages/executionMeta"
import { waitForPendingDraftWorktreeRequest } from "@/lib/worktrees/pendingDraftWorktree"
import { waitForWorktreeBootstrap } from "@/lib/worktrees/worktreeBootstrap"
import { resolveProjectForSessionDirectory } from "@/lib/projectResolution"
import {
getSyncSessions,
Expand Down Expand Up @@ -401,6 +402,84 @@ const writeRuntimeSessionMemory = (key: string, patch: Partial<RuntimeSessionMem
})
}

type MaterializedDraftSession = {
sessionId: string
directory: string | null
agent?: string
syntheticParts?: SyntheticContextPart[]
}

export async function materializeOpenDraftSession(selection: {
providerID: string
modelID: string
agent?: string
variant?: string
}): Promise<MaterializedDraftSession | null> {
const store = useSessionUIStore.getState()
const draft = store.newSessionDraft
if (!draft?.open) return null

const trimmedAgent = typeof selection.agent === "string" && selection.agent.trim().length > 0
? selection.agent.trim()
: undefined
const draftTargetFolderId = draft.targetFolderId
let draftDirectoryOverride = draft.bootstrapPendingDirectory ?? draft.directoryOverride ?? null
const draftProjectId = draft.selectedProjectId ?? null

if (draft.pendingWorktreeRequestId) {
draftDirectoryOverride = await waitForPendingDraftWorktreeRequest(draft.pendingWorktreeRequestId)
store.resolvePendingDraftWorktreeTarget(draft.pendingWorktreeRequestId, draftDirectoryOverride)
}

if (draftDirectoryOverride) {
await waitForWorktreeBootstrap(draftDirectoryOverride)
}

const created = await store.createSession(draft.title, draftDirectoryOverride, draft.parentID ?? null)
if (!created?.id) throw new Error("Failed to create session")

persistDraftTarget({
projectId: draftProjectId,
directory: normalizePath(draftDirectoryOverride ?? created.directory ?? null),
})

const draftSyntheticParts = draft.syntheticParts
const createdDirectory = normalizePath(draftDirectoryOverride ?? created.directory ?? null)
const configState = useConfigStore.getState()
void activateConfigForDirectory(createdDirectory).catch((error) => {
console.warn("Failed to activate directory after creating session:", error)
})

const effectiveDraftAgent = trimmedAgent ?? configState.currentAgentName

useSelectionStore.getState().saveSessionModelSelection(created.id, selection.providerID, selection.modelID)

if (effectiveDraftAgent) {
useSelectionStore.getState().saveSessionAgentSelection(created.id, effectiveDraftAgent)
useSelectionStore.getState().saveAgentModelForSession(created.id, effectiveDraftAgent, selection.providerID, selection.modelID)
useSelectionStore.getState().saveAgentModelVariantForSession(created.id, effectiveDraftAgent, selection.providerID, selection.modelID, selection.variant)
}

store.initializeNewOpenChamberSession(created.id, configState.agents ?? [])

store.closeNewSessionDraft()
store.setCurrentSession(created.id, createdDirectory)

if (draftTargetFolderId) {
const scopeKey = draftDirectoryOverride || created.directory || null
if (scopeKey) {
useSessionFoldersStore.getState().addSessionToFolder(scopeKey, draftTargetFolderId, created.id)
}
}
Comment on lines +465 to +473

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)


return {
sessionId: created.id,
directory: createdDirectory,
agent: effectiveDraftAgent,
syntheticParts: draftSyntheticParts,
}
}

// ---------------------------------------------------------------------------
// Store
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -913,59 +992,21 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({

// ---- New session from draft ----
if (!options?.sessionId && draft?.open) {
const draftTargetFolderId = draft.targetFolderId
let draftDirectoryOverride = draft.bootstrapPendingDirectory ?? draft.directoryOverride ?? null
const draftProjectId = draft.selectedProjectId ?? null

if (draft.pendingWorktreeRequestId) {
draftDirectoryOverride = await waitForPendingDraftWorktreeRequest(draft.pendingWorktreeRequestId)
get().resolvePendingDraftWorktreeTarget(draft.pendingWorktreeRequestId, draftDirectoryOverride)
}

const created = await get().createSession(draft.title, draftDirectoryOverride, draft.parentID ?? null)
if (!created?.id) throw new Error("Failed to create session")

persistDraftTarget({
projectId: draftProjectId,
directory: normalizePath(draftDirectoryOverride ?? created.directory ?? null),
})

const draftSyntheticParts = draft.syntheticParts
const createdDirectory = normalizePath(draftDirectoryOverride ?? created.directory ?? null)
const configState = useConfigStore.getState()
void activateConfigForDirectory(createdDirectory).catch((error) => {
console.warn("Failed to activate directory after creating session:", error)
const createdDraftSession = await materializeOpenDraftSession({
providerID,
modelID,
agent: trimmedAgent,
variant,
})
if (!createdDraftSession) throw new Error("Failed to create session")

const effectiveDraftAgent = trimmedAgent ?? configState.currentAgentName

useSelectionStore.getState().saveSessionModelSelection(created.id, providerID, modelID)

if (effectiveDraftAgent) {
useSelectionStore.getState().saveSessionAgentSelection(created.id, effectiveDraftAgent)
useSelectionStore.getState().saveAgentModelForSession(created.id, effectiveDraftAgent, providerID, modelID)
useSelectionStore.getState().saveAgentModelVariantForSession(created.id, effectiveDraftAgent, providerID, modelID, variant)
}

get().initializeNewOpenChamberSession(created.id, configState.agents ?? [])

get().closeNewSessionDraft()
get().setCurrentSession(created.id, createdDirectory)

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

const mergedAdditionalParts = draftSyntheticParts?.length
? [...(additionalParts || []), ...draftSyntheticParts]
const mergedAdditionalParts = createdDraftSession.syntheticParts?.length
? [...(additionalParts || []), ...createdDraftSession.syntheticParts]
: additionalParts

notifyMessageSent(created.id)
notifyMessageSent(createdDraftSession.sessionId)

markPendingUserSendAnimation(created.id)
markPendingUserSendAnimation(createdDraftSession.sessionId)

const files = attachments?.map((a) => ({
type: "file" as const,
Expand All @@ -975,12 +1016,12 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
}))

await routeMessage({
sessionId: created.id,
directory: createdDirectory,
sessionId: createdDraftSession.sessionId,
directory: createdDraftSession.directory,
content,
providerID,
modelID,
agent: effectiveDraftAgent,
agent: createdDraftSession.agent,
agentMentionName,
variant,
inputMode,
Expand Down Expand Up @@ -1333,6 +1374,10 @@ export const useSessionUIStore = create<SessionUIState>()((set, get) => ({
returnAfterDirectoryCreated: true,
})
sessionDirectory = normalizePath(createdWorktree.path)
if (!sessionDirectory) {
throw new Error("Worktree create missing name/path")
}
await waitForWorktreeBootstrap(sessionDirectory)
}
Comment on lines 1376 to 1381

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
      }
    }


const session = await get().createSession(undefined, sessionDirectory || null, null)
Expand Down
18 changes: 18 additions & 0 deletions packages/vscode/src/gitService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1754,6 +1754,19 @@ export async function validateWorktreeCreate(directory: string, input: CreateGit
}
}

const assertWorktreeCreatePreflight = async (directory: string, input: CreateGitWorktreePayload = {}): Promise<void> => {
const validation = await validateWorktreeCreate(directory, input);
if (validation?.ok) {
return;
}

const message = validation?.errors
?.map((error) => error?.message)
.filter(Boolean)
.join('\n') || 'Failed to validate worktree creation';
throw new Error(message);
};

export async function previewWorktreeCreate(directory: string, input: CreateGitWorktreePayload = {}): Promise<GitWorktreeInfo> {
const mode = input?.mode === 'existing' ? 'existing' : 'new';
const context = await resolveWorktreeProjectContext(directory);
Expand Down Expand Up @@ -1907,6 +1920,11 @@ async function attachGitWorktreeToCandidate(
export async function createWorktree(directory: string, input: CreateGitWorktreePayload = {}): Promise<GitWorktreeInfo> {
const mode = input?.mode === 'existing' ? 'existing' : 'new';
const context = await resolveWorktreeProjectContext(directory);

if (input?.returnAfterDirectoryCreated === true) {
await assertWorktreeCreatePreflight(directory, input);
}

await fs.promises.mkdir(context.worktreeRoot, { recursive: true });

const preferredName = String(input?.worktreeName || input?.name || '').trim();
Expand Down
18 changes: 18 additions & 0 deletions packages/web/server/lib/git/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -3544,6 +3544,19 @@ export async function validateWorktreeCreate(directory, input = {}) {
}
}

const assertWorktreeCreatePreflight = async (directory, input = {}) => {
const validation = await validateWorktreeCreate(directory, input);
if (validation?.ok) {
return;
}

const message = validation?.errors
?.map((error) => error?.message)
.filter(Boolean)
.join('\n') || 'Failed to validate worktree creation';
throw new Error(message);
};

export async function previewWorktreeCreate(directory, input = {}) {
const mode = input?.mode === 'existing' ? 'existing' : 'new';
const context = await resolveWorktreeProjectContext(directory);
Expand Down Expand Up @@ -3692,6 +3705,11 @@ async function attachGitWorktreeToCandidate(context, candidate, input = {}) {
export async function createWorktree(directory, input = {}) {
const mode = input?.mode === 'existing' ? 'existing' : 'new';
const context = await resolveWorktreeProjectContext(directory);

if (input?.returnAfterDirectoryCreated === true) {
await assertWorktreeCreatePreflight(directory, input);
}

await fsp.mkdir(context.worktreeRoot, { recursive: true });

const preferredName = String(input?.worktreeName || input?.name || '').trim();
Expand Down
Loading
Loading