From cdd17e9134b082046c04c7aab678e560c8a3cd91 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 13:20:34 +0300 Subject: [PATCH 01/14] fix(web): make new thread workspace intent explicit --- KEYBINDINGS.md | 4 +- apps/web/src/components/BranchToolbar.tsx | 6 +- .../BranchToolbarBranchSelector.tsx | 15 +- apps/web/src/components/ChatView.browser.tsx | 234 +++++++++++++++++- apps/web/src/components/ChatView.tsx | 2 +- .../src/components/DiffPanel.logic.test.ts | 1 + .../src/components/EventRouter.browser.tsx | 1 + apps/web/src/components/Sidebar.logic.test.ts | 145 +++++++++++ apps/web/src/components/Sidebar.logic.ts | 82 ++++++ apps/web/src/components/Sidebar.tsx | 132 ++++++---- .../pullRequest/PullRequestDetailPanel.tsx | 17 +- apps/web/src/composerDraftStore.test.ts | 76 +++++- apps/web/src/composerDraftStore.ts | 22 +- apps/web/src/focusedChatContext.test.ts | 1 + apps/web/src/hooks/useHandleNewThread.ts | 50 ++-- apps/web/src/lib/kanbanDispatch.ts | 2 +- apps/web/src/lib/projectShortcutTargets.ts | 10 +- apps/web/src/lib/startContainerChat.ts | 2 +- apps/web/src/lib/studioProjects.test.ts | 3 + apps/web/src/lib/temporaryThread.test.ts | 3 + apps/web/src/lib/threadBootstrap.test.ts | 177 ++++++------- apps/web/src/lib/threadBootstrap.ts | 181 +++++++------- apps/web/src/routes/_chat.$threadId.tsx | 26 +- apps/web/src/routes/_chat.tsx | 31 +-- 24 files changed, 892 insertions(+), 331 deletions(-) diff --git a/KEYBINDINGS.md b/KEYBINDINGS.md index b9410b056..93a9baf95 100644 --- a/KEYBINDINGS.md +++ b/KEYBINDINGS.md @@ -52,9 +52,9 @@ Invalid rules are ignored. Invalid config files are ignored. Warnings are logged - `terminal.split`: split terminal (in focused terminal context by default) - `terminal.new`: create new terminal (in focused terminal context by default) - `terminal.close`: close/kill the focused terminal (in focused terminal context by default) -- `chat.new`: create a new chat thread preserving the active thread's branch/worktree state +- `chat.new`: create a new chat thread using the target project's configured workspace default - `chat.newLocal`: create a new chat thread for the active project in a new environment (local/worktree determined by app settings (default `local`)) -- `chat.newTerminal`: create a new terminal-first thread preserving the active thread's branch/worktree state +- `chat.newTerminal`: create a new terminal-first thread using the target project's configured workspace default - `composer.focus.toggle`: focus or blur the chat prompt composer - `editor.openFavorite`: open current project/worktree in the last-used editor - `script.{id}.run`: run a project script by id (for example `script.test.run`) diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index 2e3c6dfea..1a7ba7c3f 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -261,7 +261,7 @@ export default function BranchToolbar({ }); const setThreadWorkspace = useCallback( - (patch: ThreadWorkspacePatch) => { + (patch: ThreadWorkspacePatch, options?: { preserveDraftWorkspaceOrigin?: boolean }) => { if (!activeThreadId) return; const branch = patch.branch !== undefined ? patch.branch : activeThreadBranch; const worktreePath = @@ -328,6 +328,9 @@ export default function BranchToolbar({ branch, worktreePath, envMode: nextDraftEnvMode, + ...(options?.preserveDraftWorkspaceOrigin && draftThread + ? { workspaceOrigin: draftThread.workspaceOrigin } + : {}), }); }, [ @@ -335,6 +338,7 @@ export default function BranchToolbar({ activeThreadBranch, serverThread?.session, activeWorktreePath, + draftThread, hasServerThread, setThreadWorkspaceAction, serverThread?.associatedWorktreePath, diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index c218f9b48..69c80af11 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -82,7 +82,10 @@ interface BranchToolbarBranchSelectorProps { effectiveEnvMode: EnvMode; envLocked: boolean; hasServerThread: boolean; - onSetThreadWorkspace: (patch: ThreadWorkspacePatch) => void; + onSetThreadWorkspace: ( + patch: ThreadWorkspacePatch, + options?: { preserveDraftWorkspaceOrigin?: boolean }, + ) => void; onCheckoutPullRequestRequest?: (reference: string) => void; onComposerFocusRequest?: () => void; variant?: BranchSelectorVariant; @@ -457,7 +460,10 @@ export function BranchToolbarBranchSelector({ return; } - onSetThreadWorkspace({ branch: currentGitBranch, worktreePath: null }); + onSetThreadWorkspace( + { branch: currentGitBranch, worktreePath: null }, + { preserveDraftWorkspaceOrigin: true }, + ); }, [ activeThreadBranch, activeWorktreePath, @@ -683,7 +689,10 @@ export function BranchToolbarBranchSelector({ ) { return; } - onSetThreadWorkspace({ branch: currentGitBranch, worktreePath: null }); + onSetThreadWorkspace( + { branch: currentGitBranch, worktreePath: null }, + { preserveDraftWorkspaceOrigin: true }, + ); }, [ activeThreadBranch, activeWorktreePath, diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index ae57dae59..edaaa6a11 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -3,6 +3,7 @@ import "../index.css"; import { AutomationId, + DEFAULT_SERVER_SETTINGS, type AutomationCreateInput, type AutomationDefinition, EventId, @@ -2929,6 +2930,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: "feature/draft-automation", worktreePath: "/repo/worktrees/draft-automation", envMode: "worktree", + workspaceOrigin: "intentional", }, }, projectDraftThreadIdByProjectId: { @@ -3034,6 +3036,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", }, }, projectDraftThreadIdByProjectId: { @@ -3121,6 +3124,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", }, }, projectDraftThreadIdByProjectId: { @@ -3203,6 +3207,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", }, }, projectDraftThreadIdByProjectId: { @@ -3281,6 +3286,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: "feature/draft", worktreePath: "/repo/worktrees/feature-draft", envMode: "worktree", + workspaceOrigin: "intentional", }, }, projectDraftThreadIdByProjectId: { @@ -4526,6 +4532,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "intentional", }, }, projectDraftThreadIdByProjectId: { @@ -4674,6 +4681,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "intentional", }, }, projectDraftThreadIdByProjectId: { @@ -5431,12 +5439,21 @@ describe("ChatView timeline estimator parity (full app)", () => { }); it("creates a new thread from the global chat.new shortcut", async () => { + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-chat-shortcut-test" as MessageId, + targetText: "chat shortcut test", + }); const mounted = await mountChatView({ viewport: DEFAULT_VIEWPORT, - snapshot: createSnapshotForTargetUser({ - targetMessageId: "msg-user-chat-shortcut-test" as MessageId, - targetText: "chat shortcut test", - }), + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/viewed-worktree", + worktreePath: "/repo/worktrees/viewed-worktree", + })), + }, configureFixture: (nextFixture) => { nextFixture.serverConfig = { ...nextFixture.serverConfig, @@ -5467,23 +5484,208 @@ describe("ChatView timeline estimator parity (full app)", () => { const composerEditor = await waitForComposerEditor(); composerEditor.focus(); await waitForLayout(); - await triggerChatNewShortcutUntilPath( + const newThreadPath = await triggerChatNewShortcutUntilPath( mounted.router, (path) => UUID_ROUTE_RE.test(path), "Route should have changed to a new draft thread UUID from the shortcut.", ); + const newThreadId = newThreadPath.slice(1) as ThreadId; + expect(useComposerDraftStore.getState().getDraftThread(newThreadId)).toMatchObject({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "default", + }); } finally { await mounted.cleanup(); } }); - it("promotes terminal-first shortcut threads so they render as terminal rows", async () => { + it("uses the configured New worktree default for provider-specific shortcuts", async () => { const mounted = await mountChatView({ viewport: DEFAULT_VIEWPORT, snapshot: createSnapshotForTargetUser({ - targetMessageId: "msg-user-terminal-shortcut-test" as MessageId, - targetText: "terminal shortcut test", + targetMessageId: "msg-user-provider-shortcut-worktree-default" as MessageId, + targetText: "provider shortcut worktree default", + }), + configureFixture: (nextFixture) => { + nextFixture.serverConfig = { + ...nextFixture.serverConfig, + keybindings: [ + { + command: "chat.newCodex", + shortcut: { + key: "c", + metaKey: false, + ctrlKey: false, + shiftKey: true, + altKey: false, + modKey: true, + }, + whenAst: { + type: "not", + node: { type: "identifier", name: "terminalFocus" }, + }, + }, + ], + }; + }, + configureNativeApi: (api) => ({ + ...api, + server: { + ...api.server, + getSettings: async () => ({ + ...DEFAULT_SERVER_SETTINGS, + defaultThreadEnvMode: "worktree", + }), + }, + }), + }); + + try { + await waitForNewThreadShortcutLabel(); + await waitForServerConfigToApply(); + const composerEditor = await waitForComposerEditor(); + composerEditor.focus(); + await waitForLayout(); + const newThreadPath = await triggerThreadShortcutUntilPath( + mounted.router, + () => dispatchThreadShortcut("c"), + (path) => UUID_ROUTE_RE.test(path), + "Provider shortcut should open a fresh draft thread.", + ); + const newThreadId = newThreadPath.slice(1) as ThreadId; + expect(useComposerDraftStore.getState().getDraftThread(newThreadId)).toMatchObject({ + branch: "main", + worktreePath: null, + envMode: "worktree", + workspaceOrigin: "default", + }); + } finally { + await mounted.cleanup(); + } + }); + + it("starts one fresh thread in the exact worktree from repeated context-menu actions", async () => { + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/exact-worktree", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/exact-worktree", + }, + ], + }; + const branchLookupDeferred = (() => { + let resolve!: (value: Awaited>) => void; + const promise = new Promise>>( + (nextResolve) => { + resolve = nextResolve; + }, + ); + return { promise, resolve }; + })(); + const branchLookup = vi.fn(async () => exactWorktreeBranchResult); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-context-worktree-test" as MessageId, + targetText: "context worktree test", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/exact-worktree", + worktreePath: "/repo/worktrees/exact-worktree", + })), + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, }), + }); + + try { + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalled()); + branchLookup.mockClear(); + branchLookup.mockImplementation(() => branchLookupDeferred.promise); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + const openContextMenu = () => + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + + openContextMenu(); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(1)); + openContextMenu(); + await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(2)); + expect(contextMenuShow.mock.calls[0]?.[0]?.[0]).toMatchObject({ + id: "new-thread-in-workspace", + label: "New thread in worktree (feature/exact-worktree)", + }); + + branchLookupDeferred.resolve(exactWorktreeBranchResult); + const newThreadPath = await waitForURL( + mounted.router, + (path) => UUID_ROUTE_RE.test(path), + "The explicit worktree action should open a fresh draft.", + ); + const newThreadId = newThreadPath.slice(1) as ThreadId; + expect(branchLookup).toHaveBeenCalledTimes(1); + expect(useComposerDraftStore.getState().getDraftThread(newThreadId)).toMatchObject({ + branch: "feature/exact-worktree", + worktreePath: "/repo/worktrees/exact-worktree", + envMode: "worktree", + workspaceOrigin: "intentional", + }); + } finally { + branchLookupDeferred.resolve(exactWorktreeBranchResult); + await mounted.cleanup(); + } + }); + + it("promotes terminal-first shortcut threads so they render as terminal rows", async () => { + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-terminal-shortcut-test" as MessageId, + targetText: "terminal shortcut test", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/viewed-terminal-worktree", + worktreePath: "/repo/worktrees/viewed-terminal-worktree", + })), + }, configureFixture: (nextFixture) => { nextFixture.serverConfig = { ...nextFixture.serverConfig, @@ -5537,6 +5739,21 @@ describe("ChatView timeline estimator parity (full app)", () => { }, { timeout: 20_000, interval: 16 }, ); + const terminalCreateRequest = wsRequests.find( + (request) => + request._tag === ORCHESTRATION_WS_METHODS.dispatchCommand && + typeof request.command === "object" && + request.command !== null && + "type" in request.command && + "threadId" in request.command && + request.command.type === "thread.create" && + request.command.threadId === newThreadId, + ); + expect(terminalCreateRequest?.command).toMatchObject({ + envMode: "local", + branch: null, + worktreePath: null, + }); // The shortcut persists terminal-first presentation separately from the // server thread row. Observe that state before simulating promotion so @@ -5611,6 +5828,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branch: "feature/terminal-title", worktreePath: "/repo/project/.worktrees/terminal-title", envMode: "worktree", + workspaceOrigin: "intentional", }, }, projectDraftThreadIdByProjectId: { diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index c76845b03..d227dddf3 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -9782,7 +9782,7 @@ export default function ChatView({ }); return; } - await handleNewThread(activeProject.id, { entryPoint: "chat" }); + await handleNewThread(activeProject.id); }, handleInteractionModeChange, openForkTargetPicker: () => { diff --git a/apps/web/src/components/DiffPanel.logic.test.ts b/apps/web/src/components/DiffPanel.logic.test.ts index a4afbaf69..16a2b20c8 100644 --- a/apps/web/src/components/DiffPanel.logic.test.ts +++ b/apps/web/src/components/DiffPanel.logic.test.ts @@ -69,6 +69,7 @@ function makeDraftThread(overrides: Partial = {}): DraftThread branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", ...overrides, }; } diff --git a/apps/web/src/components/EventRouter.browser.tsx b/apps/web/src/components/EventRouter.browser.tsx index 14af5e33c..c73a04bd9 100644 --- a/apps/web/src/components/EventRouter.browser.tsx +++ b/apps/web/src/components/EventRouter.browser.tsx @@ -1056,6 +1056,7 @@ describe("EventRouter scoped orchestration sync", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", isTemporary: false, }, }, diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts index 0d9b0062c..ccefcd3c3 100644 --- a/apps/web/src/components/Sidebar.logic.test.ts +++ b/apps/web/src/components/Sidebar.logic.test.ts @@ -36,12 +36,14 @@ import { resolveSidebarThreadListPaging, resolveProjectEmptyState, resolvePendingSidebarViewSelection, + resolveNewThreadInWorkspaceAction, resolveSettingsBackTarget, resolveProjectStatusIndicator, resolveSidebarNewThreadEnvMode, resolveThreadHoverCardMetadata, resolveThreadRowClassName, resolveThreadStatusPill, + validateNewThreadInWorkspaceAction, shouldShowDebugFeatureFlagsMenu, shouldPrunePinnedThreads, shouldClearThreadSelectionOnMouseDown, @@ -103,6 +105,149 @@ describe("resolvePullRequestReviewBadge", () => { }); }); +describe("new thread in workspace actions", () => { + it("names exact local and worktree destinations and hides incomplete targets", () => { + expect( + resolveNewThreadInWorkspaceAction({ + branch: "feature/safe-new-thread", + envMode: "local", + worktreePath: null, + }), + ).toEqual({ + id: "new-thread-in-workspace", + label: "New thread on branch (feature/safe-new-thread)", + workspace: { kind: "existing-local", branch: "feature/safe-new-thread" }, + }); + expect( + resolveNewThreadInWorkspaceAction({ + branch: "feature/worktree", + envMode: "worktree", + worktreePath: "/repo/.scient/worktrees/feature-worktree", + }), + ).toEqual({ + id: "new-thread-in-workspace", + label: "New thread in worktree (feature/worktree)", + workspace: { + kind: "existing-worktree", + branch: "feature/worktree", + worktreePath: "/repo/.scient/worktrees/feature-worktree", + }, + }); + expect( + resolveNewThreadInWorkspaceAction({ + branch: null, + envMode: "local", + worktreePath: null, + }), + ).toBeNull(); + expect( + resolveNewThreadInWorkspaceAction({ + branch: "feature/pending", + envMode: "worktree", + worktreePath: null, + }), + ).toBeNull(); + }); + + it("fails closed when the local branch is missing or no longer checked out", () => { + const action = resolveNewThreadInWorkspaceAction({ + branch: "feature/local", + envMode: "local", + worktreePath: null, + }); + expect(action).not.toBeNull(); + if (!action) return; + + expect( + validateNewThreadInWorkspaceAction({ + action, + branches: [], + isRepo: false, + projectCwd: "/repo/project", + }), + ).toEqual({ + ok: false, + description: "This project folder is no longer a Git repository.", + }); + expect( + validateNewThreadInWorkspaceAction({ + action, + branches: [], + isRepo: true, + projectCwd: "/repo/project", + }), + ).toEqual({ + ok: false, + description: "Branch feature/local no longer exists locally.", + }); + expect( + validateNewThreadInWorkspaceAction({ + action, + branches: [ + { + name: "feature/local", + current: false, + isDefault: false, + worktreePath: null, + }, + ], + isRepo: true, + projectCwd: "/repo/project", + }), + ).toEqual({ + ok: false, + description: "Branch feature/local is no longer checked out in the project folder.", + }); + }); + + it("accepts exact workspace mappings and normalizes Windows path differences", () => { + const localAction = resolveNewThreadInWorkspaceAction({ + branch: "main", + envMode: "local", + worktreePath: null, + }); + const worktreeAction = resolveNewThreadInWorkspaceAction({ + branch: "feature/windows", + envMode: "worktree", + worktreePath: "C:\\Repo\\Worktrees\\Windows\\", + }); + expect(localAction).not.toBeNull(); + expect(worktreeAction).not.toBeNull(); + if (!localAction || !worktreeAction) return; + + expect( + validateNewThreadInWorkspaceAction({ + action: localAction, + branches: [ + { + name: "main", + current: true, + isDefault: true, + worktreePath: "/repo/project", + }, + ], + isRepo: true, + projectCwd: "/repo/project/", + }), + ).toEqual({ ok: true }); + expect( + validateNewThreadInWorkspaceAction({ + action: worktreeAction, + branches: [ + { + name: "feature/windows", + current: false, + isDefault: false, + worktreePath: "c:/repo/worktrees/windows", + }, + ], + isRepo: true, + projectCwd: "C:\\Repo", + }), + ).toEqual({ ok: true }); + }); +}); + describe("pullRequestRepositoryConfigFingerprint", () => { it("changes for repository-affecting project edits but not sidebar ordering or expansion", () => { const first = makeProject({ id: ProjectId.makeUnsafe("project-1"), cwd: "/repo/one" }); diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts index 541f1ce8e..63719ae74 100644 --- a/apps/web/src/components/Sidebar.logic.ts +++ b/apps/web/src/components/Sidebar.logic.ts @@ -4,6 +4,7 @@ import { MAX_PINNED_PROJECTS, + type GitBranch, type KeybindingCommand, type ProjectId, type PullRequestReviewRequestCountResult, @@ -13,6 +14,7 @@ import { pluralize } from "@synara/shared/text"; import { resolveThreadEnvironmentMode } from "@synara/shared/threadEnvironment"; import { isWorkspaceRootWithin, workspaceRootsEqual } from "@synara/shared/threadWorkspace"; import type { SidebarProjectSortOrder, SidebarThreadSortOrder } from "../appSettings"; +import type { NewThreadWorkspaceIntent } from "../lib/threadBootstrap"; import { resolveRestorableThreadRoute, type LastThreadRoute } from "../chatRouteRestore"; import type { ChatMessage, Project, SidebarThreadSummary, Thread } from "../types"; import { cn } from "../lib/utils"; @@ -47,6 +49,86 @@ export const THREAD_SELECTION_SAFE_SELECTOR = "[data-thread-item], [data-thread- export const SIDEBAR_THREAD_PREWARM_LIMIT = 10; export const DEBUG_FEATURE_FLAGS_MENU_STORAGE_KEY = "scient:show-debug-feature-flags-menu"; export type SidebarNewThreadEnvMode = "local" | "worktree"; +export interface NewThreadInWorkspaceAction { + readonly id: "new-thread-in-workspace"; + readonly label: string; + readonly workspace: Extract< + NewThreadWorkspaceIntent, + { readonly kind: "existing-local" | "existing-worktree" } + >; +} + +export function resolveNewThreadInWorkspaceAction(input: { + readonly branch: string | null; + readonly envMode?: "local" | "worktree" | undefined; + readonly worktreePath: string | null; +}): NewThreadInWorkspaceAction | null { + const branch = input.branch?.trim() ?? ""; + if (!branch) return null; + const worktreePath = input.worktreePath?.trim() ?? ""; + if (worktreePath) { + return { + id: "new-thread-in-workspace", + label: `New thread in worktree (${branch})`, + workspace: { kind: "existing-worktree", branch, worktreePath }, + }; + } + if (input.envMode === "worktree") return null; + return { + id: "new-thread-in-workspace", + label: `New thread on branch (${branch})`, + workspace: { kind: "existing-local", branch }, + }; +} + +export type NewThreadInWorkspaceValidation = + | { readonly ok: true } + | { readonly ok: false; readonly description: string }; + +export function validateNewThreadInWorkspaceAction(input: { + readonly action: NewThreadInWorkspaceAction; + readonly branches: readonly GitBranch[]; + readonly isRepo: boolean; + readonly projectCwd: string; +}): NewThreadInWorkspaceValidation { + if (!input.isRepo) { + return { + ok: false, + description: "This project folder is no longer a Git repository.", + }; + } + const branch = input.branches.find( + (candidate) => candidate.name === input.action.workspace.branch && candidate.isRemote !== true, + ); + if (!branch) { + return { + ok: false, + description: `Branch ${input.action.workspace.branch} no longer exists locally.`, + }; + } + if (input.action.workspace.kind === "existing-worktree") { + if ( + !branch.worktreePath || + !workspaceRootsEqual(branch.worktreePath, input.action.workspace.worktreePath) + ) { + return { + ok: false, + description: `The worktree for ${input.action.workspace.branch} was removed or moved.`, + }; + } + return { ok: true }; + } + if ( + !branch.current || + (branch.worktreePath !== null && !workspaceRootsEqual(branch.worktreePath, input.projectCwd)) + ) { + return { + ok: false, + description: `Branch ${input.action.workspace.branch} is no longer checked out in the project folder.`, + }; + } + return { ok: true }; +} export type SidebarView = "threads" | "studio" | "workspace"; export type SidebarActionBadge = { readonly text: string; diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 32affecb8..e9d57cba6 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -312,6 +312,7 @@ import { isLatestPinnedThreadMutation, pruneProjectThreadListPagingForCollapsedProjects, recoverExistingAddProjectTarget, + resolveNewThreadInWorkspaceAction, resolvePullRequestReviewBadge, resolveSidebarThreadListPaging, DEBUG_FEATURE_FLAGS_MENU_STORAGE_KEY, @@ -324,6 +325,7 @@ import { resolveThreadRowClassName, resolveThreadRowTrailingReserveClass, resolveThreadStatusPill, + validateNewThreadInWorkspaceAction, type ThreadStatusPill, type SidebarDerivedProjectData, type SidebarActionBadge, @@ -1707,6 +1709,7 @@ export default function Sidebar() { ); const [projectRunDialogCommandDraft, setProjectRunDialogCommandDraft] = useState(""); const isAddingProjectRef = useRef(false); + const newThreadInWorkspaceInFlightRef = useRef(false); const [projectInitializationPreview, setProjectInitializationPreview] = useState(null); const [projectInitializationError, setProjectInitializationError] = useState(null); @@ -2283,17 +2286,10 @@ export default function Sidebar() { return true; } - void handleNewThread(projectId, { - envMode: appSettings.defaultThreadEnvMode, - }).catch(() => undefined); + void handleNewThread(projectId).catch(() => undefined); return true; }, - [ - appSettings.defaultThreadEnvMode, - appSettings.sidebarThreadSortOrder, - handleNewThread, - navigate, - ], + [appSettings.sidebarThreadSortOrder, handleNewThread, navigate], ); const openExistingProjectFromSnapshot = useCallback( @@ -2326,18 +2322,10 @@ export default function Sidebar() { } setProjectExpanded(projectId, true); - void handleNewThread(projectId, { - envMode: appSettings.defaultThreadEnvMode, - }).catch(() => undefined); + void handleNewThread(projectId).catch(() => undefined); return true; }, - [ - appSettings.defaultThreadEnvMode, - appSettings.sidebarThreadSortOrder, - handleNewThread, - navigate, - setProjectExpanded, - ], + [appSettings.sidebarThreadSortOrder, handleNewThread, navigate, setProjectExpanded], ); // Poll the server read model briefly after project.create so we only recover from fresh state. @@ -2425,18 +2413,9 @@ export default function Sidebar() { return; } - void handleNewThread(typedProjectId, { - envMode: resolveSidebarNewThreadEnvMode({ - defaultEnvMode: appSettings.defaultThreadEnvMode, - }), - }); + void handleNewThread(typedProjectId); }, - [ - appSettings.defaultThreadEnvMode, - focusMostRecentThreadForProject, - handleNewThread, - sidebarThreads, - ], + [focusMostRecentThreadForProject, handleNewThread, sidebarThreads], ); const navigateToWorkspace = useCallback( @@ -2867,9 +2846,7 @@ export default function Sidebar() { // snapshot is just slow to catch up, continue with the local new-thread flow // instead of surfacing a false-negative sidebar sync error. setProjectExpanded(creationResult.projectId, true); - void handleNewThread(creationResult.projectId, { - envMode: appSettings.defaultThreadEnvMode, - }).catch(() => undefined); + void handleNewThread(creationResult.projectId).catch(() => undefined); return true; } catch (error) { const description = @@ -2880,7 +2857,6 @@ export default function Sidebar() { } }, [ - appSettings.defaultThreadEnvMode, handleNewThread, projects, recoverExistingProjectFromServer, @@ -2967,17 +2943,12 @@ export default function Sidebar() { const handlePrimaryNewThread = useCallback(() => { if (currentProjectShortcutTargetId) { prefetchModelsForProjectNewThread(currentProjectShortcutTargetId, { includeDroid: true }); - void handleNewThread(currentProjectShortcutTargetId, { - envMode: resolveSidebarNewThreadEnvMode({ - defaultEnvMode: appSettings.defaultThreadEnvMode, - }), - }); + void handleNewThread(currentProjectShortcutTargetId); return; } handleStartAddProject(); }, [ - appSettings.defaultThreadEnvMode, currentProjectShortcutTargetId, handleNewThread, handleStartAddProject, @@ -3814,9 +3785,26 @@ export default function Sidebar() { envMode: thread.envMode, worktreePath: thread.worktreePath, }); + const newThreadInWorkspaceAction = resolveNewThreadInWorkspaceAction({ + branch: thread.branch, + envMode: thread.envMode, + worktreePath: thread.worktreePath, + }); const clicked = await api.contextMenu.show( [ - { id: "rename", label: "Rename thread" }, + ...(newThreadInWorkspaceAction + ? [ + { + id: newThreadInWorkspaceAction.id, + label: newThreadInWorkspaceAction.label, + }, + ] + : []), + { + id: "rename", + label: "Rename thread", + ...(newThreadInWorkspaceAction ? { separatorBefore: true } : {}), + }, { id: "toggle-pin", label: pinActionLabel("thread", isPinned) }, ...(threadStatus?.dismissible ? [{ id: "clear-notification", label: "Clear notification" }] @@ -3835,6 +3823,54 @@ export default function Sidebar() { position, ); + if (clicked === "new-thread-in-workspace") { + if (!newThreadInWorkspaceAction || newThreadInWorkspaceInFlightRef.current) return; + const projectCwd = projectCwdById.get(thread.projectId) ?? null; + if (!projectCwd) { + showSidebarTransientError({ + title: "Unable to start thread", + description: "The project folder is no longer available.", + }); + return; + } + newThreadInWorkspaceInFlightRef.current = true; + try { + const branchResult = await api.git.listBranches({ cwd: projectCwd }); + const validation = validateNewThreadInWorkspaceAction({ + action: newThreadInWorkspaceAction, + branches: branchResult.branches, + isRepo: branchResult.isRepo, + projectCwd, + }); + if (!validation.ok) { + showSidebarTransientError({ + title: "Workspace changed", + description: validation.description, + }); + return; + } + const createdThreadId = await handleNewThread(thread.projectId, { + fresh: true, + workspace: newThreadInWorkspaceAction.workspace, + }); + if (!createdThreadId) { + showSidebarTransientError({ + title: "Unable to start thread", + description: "The new thread could not be opened. Try again.", + }); + } + } catch (error) { + showSidebarTransientError({ + title: "Unable to start thread", + description: + error instanceof Error ? error.message : "The workspace could not be verified.", + }); + } finally { + newThreadInWorkspaceInFlightRef.current = false; + } + return; + } + if (clicked === "rename") { openRenameThreadDialog(threadId); return; @@ -3977,6 +4013,7 @@ export default function Sidebar() { clearDismissedThreadStatus, clearThreadNotification, handoffThread, + handleNewThread, markThreadUnread, navigate, openRenameThreadDialog, @@ -6123,12 +6160,7 @@ export default function Sidebar() { onClick={(event) => { event.preventDefault(); event.stopPropagation(); - void handleNewThread(project.id, { - envMode: resolveSidebarNewThreadEnvMode({ - defaultEnvMode: appSettings.defaultThreadEnvMode, - }), - entryPoint: "terminal", - }); + void handleNewThread(project.id, { entryPoint: "terminal" }); }} /> diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 455133d60..c73108818 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -203,10 +203,21 @@ export function PullRequestDetailPanel({ try { const mode = settings.defaultThreadEnvMode; const prepared = await prepareThreadMutation.mutateAsync({ reference: detail.url, mode }); + const workspace = (() => { + if (mode === "worktree") { + if (!prepared.worktreePath) { + throw new Error("The pull request worktree was not created."); + } + return { + kind: "existing-worktree" as const, + branch: prepared.branch, + worktreePath: prepared.worktreePath, + }; + } + return { kind: "existing-local" as const, branch: prepared.branch }; + })(); const threadId = await handleNewThread(detail.projectId, { - branch: prepared.branch, - worktreePath: prepared.worktreePath, - envMode: mode, + workspace, // This action is an explicit handoff from the PR browser. Reusing the project's // existing draft can leave the user on the PR route and insert the prompt into a // hidden composer, making the button appear inert. diff --git a/apps/web/src/composerDraftStore.test.ts b/apps/web/src/composerDraftStore.test.ts index e275ae866..90e2b81ee 100644 --- a/apps/web/src/composerDraftStore.test.ts +++ b/apps/web/src/composerDraftStore.test.ts @@ -12,6 +12,7 @@ import { COMPOSER_DRAFT_STORAGE_KEY, type ComposerFileAttachment, type ComposerImageAttachment, + type DraftThreadState, type QueuedComposerTurn, captureComposerPromptHistorySavedDraft, deriveEffectiveComposerModelState, @@ -656,7 +657,7 @@ describe("composerDraftStore prompt history saved draft", () => { setLocalStorageItem( COMPOSER_DRAFT_STORAGE_KEY, { - version: 5, + version: 6, state: { draftsByThreadId: { [threadId]: { @@ -1303,7 +1304,7 @@ describe("composerDraftStore syncPersistedAttachments", () => { setLocalStorageItem( COMPOSER_DRAFT_STORAGE_KEY, { - version: 5, + version: 6, state: { draftsByThreadId: { [threadId]: { @@ -1382,7 +1383,7 @@ describe("composerDraftStore syncPersistedAttachments", () => { setLocalStorageItem( COMPOSER_DRAFT_STORAGE_KEY, { - version: 5, + version: 6, state: { draftsByThreadId: { [threadId]: { @@ -1816,6 +1817,7 @@ describe("composerDraftStore project draft thread mapping", () => { interactionMode: "default", createdAt: "2026-01-01T00:00:00.000Z", lastKnownPr: null, + workspaceOrigin: "intentional", }); expect(useComposerDraftStore.getState().getDraftThread(threadId)).toEqual({ projectId, @@ -1827,6 +1829,7 @@ describe("composerDraftStore project draft thread mapping", () => { interactionMode: "default", createdAt: "2026-01-01T00:00:00.000Z", lastKnownPr: null, + workspaceOrigin: "intentional", }); }); @@ -1844,6 +1847,36 @@ describe("composerDraftStore project draft thread mapping", () => { expect(useComposerDraftStore.getState().getDraftThread(threadId)?.isTemporary).toBeUndefined(); }); + it("tracks whether a draft workspace came from defaults or an intentional choice", () => { + const store = useComposerDraftStore.getState(); + store.setProjectDraftThreadId(projectId, threadId); + expect(useComposerDraftStore.getState().getDraftThread(threadId)?.workspaceOrigin).toBe( + "default", + ); + + store.setDraftThreadContext(threadId, { + branch: "feature/intentional", + worktreePath: "/tmp/feature-intentional", + envMode: "worktree", + }); + expect(useComposerDraftStore.getState().getDraftThread(threadId)?.workspaceOrigin).toBe( + "intentional", + ); + + store.setDraftThreadContext(threadId, { + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "default", + }); + expect(useComposerDraftStore.getState().getDraftThread(threadId)).toMatchObject({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "default", + }); + }); + it("registers a mapping-less temporary terminal draft for staged navigation", () => { const store = useComposerDraftStore.getState(); @@ -3208,6 +3241,43 @@ describe("composerDraftStore sticky composer settings", () => { expect(migratedState.stickyActiveProvider).toBe("claudeAgent"); }); + it("migrates legacy draft workspaces to safe default provenance", () => { + const persistApi = useComposerDraftStore.persist as unknown as { + getOptions: () => { + migrate: (persistedState: unknown, version: number) => unknown; + }; + }; + const threadId = ThreadId.makeUnsafe("thread-legacy-workspace-origin"); + const migratedState = persistApi.getOptions().migrate( + { + draftsByThreadId: {}, + draftThreadsByThreadId: { + [threadId]: { + projectId: "project-a", + createdAt: "2026-07-24T00:00:00.000Z", + runtimeMode: "full-access", + interactionMode: "default", + entryPoint: "chat", + branch: "feature/legacy-inherited", + worktreePath: "/tmp/legacy-inherited", + envMode: "worktree", + }, + }, + projectDraftThreadIdByProjectId: { "project-a": threadId }, + }, + 5, + ) as { + draftThreadsByThreadId: Record; + }; + + expect(migratedState.draftThreadsByThreadId[threadId]).toMatchObject({ + branch: "feature/legacy-inherited", + worktreePath: "/tmp/legacy-inherited", + envMode: "worktree", + workspaceOrigin: "default", + }); + }); + it("applies sticky activeProvider to new drafts", () => { const store = useComposerDraftStore.getState(); const threadId = ThreadId.makeUnsafe("thread-sticky-active-provider"); diff --git a/apps/web/src/composerDraftStore.ts b/apps/web/src/composerDraftStore.ts index 832e51134..b807a689f 100644 --- a/apps/web/src/composerDraftStore.ts +++ b/apps/web/src/composerDraftStore.ts @@ -86,9 +86,11 @@ import { } from "./lib/storage"; export const COMPOSER_DRAFT_STORAGE_KEY = "scient:composer-drafts:v1"; -const COMPOSER_DRAFT_STORAGE_VERSION = 5; +const COMPOSER_DRAFT_STORAGE_VERSION = 6; const DraftThreadEnvModeSchema = Schema.Literals(["local", "worktree"]); export type DraftThreadEnvMode = typeof DraftThreadEnvModeSchema.Type; +const DraftThreadWorkspaceOriginSchema = Schema.Literals(["default", "intentional"]); +export type DraftThreadWorkspaceOrigin = typeof DraftThreadWorkspaceOriginSchema.Type; const DraftThreadEntryPointSchema = Schema.Literals(["chat", "terminal"]); const COMPOSER_PROVIDER_KINDS = [ "codex", @@ -467,6 +469,9 @@ const PersistedDraftThreadState = Schema.Struct({ worktreePath: Schema.NullOr(Schema.String), lastKnownPr: Schema.optionalKey(Schema.NullOr(OrchestrationThreadPullRequest)), envMode: DraftThreadEnvModeSchema, + workspaceOrigin: DraftThreadWorkspaceOriginSchema.pipe( + Schema.withDecodingDefault(() => "default"), + ), isTemporary: Schema.optionalKey(Schema.Boolean), promotedTo: Schema.optionalKey(ThreadId), }); @@ -518,6 +523,7 @@ export interface DraftThreadState { worktreePath: string | null; lastKnownPr?: OrchestrationThreadPullRequest | null; envMode: DraftThreadEnvMode; + workspaceOrigin: DraftThreadWorkspaceOrigin; isTemporary?: boolean; promotedTo?: ThreadId; } @@ -528,6 +534,7 @@ interface DraftThreadMutationOptions { lastKnownPr?: OrchestrationThreadPullRequest | null; createdAt?: string; envMode?: DraftThreadEnvMode; + workspaceOrigin?: DraftThreadWorkspaceOrigin; runtimeMode?: RuntimeMode; interactionMode?: ProviderInteractionMode; entryPoint?: ThreadPrimarySurface; @@ -571,6 +578,7 @@ export interface ComposerDraftStoreState { branch?: string | null; worktreePath?: string | null; envMode?: DraftThreadEnvMode; + workspaceOrigin?: DraftThreadWorkspaceOrigin; runtimeMode?: RuntimeMode; interactionMode?: ProviderInteractionMode; entryPoint?: ThreadPrimarySurface; @@ -802,6 +810,10 @@ function buildDraftThreadState(input: { ? false : existingThread?.isTemporary === true; const nextPromotedTo = existingThread?.promotedTo; + const hasWorkspaceMutation = + options?.branch !== undefined || + options?.worktreePath !== undefined || + options?.envMode !== undefined; return { projectId: input.projectId, @@ -823,6 +835,9 @@ function buildDraftThreadState(input: { : (options.lastKnownPr ?? null), envMode: options?.envMode ?? (nextWorktreePath ? "worktree" : (existingThread?.envMode ?? "local")), + workspaceOrigin: + options?.workspaceOrigin ?? + (hasWorkspaceMutation ? "intentional" : (existingThread?.workspaceOrigin ?? "default")), ...(nextIsTemporary ? { isTemporary: true } : {}), ...(nextPromotedTo ? { promotedTo: nextPromotedTo } : {}), }; @@ -846,6 +861,7 @@ function draftThreadStatesEqual( left.worktreePath === right.worktreePath && Equal.equals(left.lastKnownPr ?? null, right.lastKnownPr ?? null) && left.envMode === right.envMode && + left.workspaceOrigin === right.workspaceOrigin && (left.isTemporary === true) === (right.isTemporary === true) && left.promotedTo === right.promotedTo ); @@ -2573,6 +2589,8 @@ function normalizePersistedDraftThreads( worktreePath: normalizedWorktreePath, ...(lastKnownPr ? { lastKnownPr } : {}), envMode: normalizeDraftThreadEnvMode(candidateDraftThread.envMode, normalizedWorktreePath), + workspaceOrigin: + candidateDraftThread.workspaceOrigin === "intentional" ? "intentional" : "default", ...(isTemporary ? { isTemporary: true } : {}), ...(promotedTo ? { promotedTo } : {}), }; @@ -2606,6 +2624,7 @@ function normalizePersistedDraftThreads( branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", }; } else if (draftThreadsByThreadId[threadId as ThreadId]?.projectId !== projectId) { draftThreadsByThreadId[threadId as ThreadId] = { @@ -3623,6 +3642,7 @@ export const useComposerDraftStore = create()( worktreePath, lastKnownPr: null, envMode: options.envMode ?? (worktreePath ? "worktree" : "local"), + workspaceOrigin: options.workspaceOrigin ?? "default", ...(options.isTemporary ? { isTemporary: true } : {}), }; return { diff --git a/apps/web/src/focusedChatContext.test.ts b/apps/web/src/focusedChatContext.test.ts index 1c3784256..b7382caa0 100644 --- a/apps/web/src/focusedChatContext.test.ts +++ b/apps/web/src/focusedChatContext.test.ts @@ -68,6 +68,7 @@ function makeDraftThread(overrides: Partial = {}): DraftThread branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", ...overrides, }; } diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 539e17113..aaf9f97a0 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -10,7 +10,7 @@ import { useComposerDraftStore, } from "../composerDraftStore"; import { - buildDraftThreadContextPatch, + buildDraftThreadWorkspacePatch, createActiveDraftThreadSnapshot, createActiveThreadSnapshot, createFreshDraftThreadSeed, @@ -144,16 +144,15 @@ export function useHandleNewThread() { const resolveCreationState = ( targetThreadId: ThreadId, draftThread: DraftThreadState | null, - creationOptions: NewThreadOptions | undefined, ) => resolveTerminalThreadCreationState({ activeDraftThread: activeDraftThreadSnapshot, activeThread: activeThreadSnapshot, + defaultEnvMode: settings.defaultThreadEnvMode, defaultProvider: options?.provider ?? settings.defaultProvider, draftComposerState: useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, draftThread, - options: creationOptions, projectDefaultModelSelection, projectId, }); @@ -194,12 +193,13 @@ export function useHandleNewThread() { const preservedComposerDraft = useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; let resolvedStoredDraftThread: DraftThreadState | null = bootstrapPlan.draftThread; - const shouldPreserveStoredTerminalContext = - entryPoint === "terminal" && bootstrapPlan.draftThread.entryPoint === "terminal"; - const draftContextPatch = shouldPreserveStoredTerminalContext - ? null - : buildDraftThreadContextPatch(entryPoint, options); - const creationOptions = shouldPreserveStoredTerminalContext ? undefined : options; + const draftContextPatch = buildDraftThreadWorkspacePatch({ + defaultEnvMode: settings.defaultThreadEnvMode, + draftThread: bootstrapPlan.draftThread, + entryPoint, + options, + reuseKind: "stored", + }); if (draftContextPatch) { setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); resolvedStoredDraftThread = getDraftThread(bootstrapPlan.threadId); @@ -212,11 +212,7 @@ export function useHandleNewThread() { if (entryPoint === "terminal") { await createTerminalThread( bootstrapPlan.threadId, - resolveCreationState( - bootstrapPlan.threadId, - resolvedStoredDraftThread, - creationOptions, - ), + resolveCreationState(bootstrapPlan.threadId, resolvedStoredDraftThread), ); } return bootstrapPlan.threadId; @@ -230,11 +226,7 @@ export function useHandleNewThread() { if (entryPoint === "terminal") { await createTerminalThread( bootstrapPlan.threadId, - resolveCreationState( - bootstrapPlan.threadId, - resolvedStoredDraftThread, - creationOptions, - ), + resolveCreationState(bootstrapPlan.threadId, resolvedStoredDraftThread), ); } return bootstrapPlan.threadId; @@ -249,7 +241,13 @@ export function useHandleNewThread() { const preservedComposerDraft = useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; let resolvedActiveDraftThread: DraftThreadState | null = bootstrapPlan.draftThread; - const draftContextPatch = buildDraftThreadContextPatch(entryPoint, options); + const draftContextPatch = buildDraftThreadWorkspacePatch({ + defaultEnvMode: settings.defaultThreadEnvMode, + draftThread: bootstrapPlan.draftThread, + entryPoint, + options, + reuseKind: "route", + }); if (draftContextPatch) { setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); resolvedActiveDraftThread = getDraftThread(bootstrapPlan.threadId); @@ -261,7 +259,7 @@ export function useHandleNewThread() { if (entryPoint === "terminal") { await createTerminalThread( bootstrapPlan.threadId, - resolveCreationState(bootstrapPlan.threadId, resolvedActiveDraftThread, options), + resolveCreationState(bootstrapPlan.threadId, resolvedActiveDraftThread), ); } return bootstrapPlan.threadId; @@ -274,7 +272,12 @@ export function useHandleNewThread() { markTemporaryThread(threadId); } const createdAt = new Date().toISOString(); - const draftSeed = createFreshDraftThreadSeed({ createdAt, entryPoint, options }); + const draftSeed = createFreshDraftThreadSeed({ + createdAt, + defaultEnvMode: settings.defaultThreadEnvMode, + entryPoint, + options, + }); const committed = await stageDraftNavigation({ // Keep the previous routed draft alive while the destination loads. Replacing the // project's primary slot earlier makes the route guard redirect the old URL to Home. @@ -317,7 +320,7 @@ export function useHandleNewThread() { if (entryPoint === "terminal") { await createTerminalThread( threadId, - resolveCreationState(threadId, getDraftThread(threadId), options), + resolveCreationState(threadId, getDraftThread(threadId)), ); } return threadId; @@ -335,6 +338,7 @@ export function useHandleNewThread() { markTemporaryThread, router, settings.defaultProvider, + settings.defaultThreadEnvMode, ], ); diff --git a/apps/web/src/lib/kanbanDispatch.ts b/apps/web/src/lib/kanbanDispatch.ts index 12590376f..59a14dc42 100644 --- a/apps/web/src/lib/kanbanDispatch.ts +++ b/apps/web/src/lib/kanbanDispatch.ts @@ -242,10 +242,10 @@ async function dispatchKanbanDraftThreadOnce( const creationState = resolveTerminalThreadCreationState({ activeDraftThread: null, activeThread: null, + defaultEnvMode: draftThread?.envMode ?? "local", defaultProvider: input.defaultProvider, draftComposerState, draftThread, - options: undefined, projectDefaultModelSelection: project?.defaultModelSelection ?? null, projectId, }); diff --git a/apps/web/src/lib/projectShortcutTargets.ts b/apps/web/src/lib/projectShortcutTargets.ts index ab4c75fe1..682a508f9 100644 --- a/apps/web/src/lib/projectShortcutTargets.ts +++ b/apps/web/src/lib/projectShortcutTargets.ts @@ -32,12 +32,6 @@ export function resolveLatestProjectTargetId( export interface NewThreadTarget { readonly projectId: ProjectId; - /** - * Whether the new thread should inherit the active surface's branch/worktree/env. - * True only when we target the focused project; on the latest-project fallback that - * context belongs to a project no longer in view, so we defer to its own defaults. - */ - readonly inheritContext: boolean; } // Single rule for which project a "new thread" chord targets: the focused project when @@ -48,10 +42,10 @@ export function resolveNewThreadTarget(input: { latestUsableProjectId: ProjectId | null; }): NewThreadTarget | null { if (input.currentProjectId) { - return { projectId: input.currentProjectId, inheritContext: true }; + return { projectId: input.currentProjectId }; } if (input.latestUsableProjectId) { - return { projectId: input.latestUsableProjectId, inheritContext: false }; + return { projectId: input.latestUsableProjectId }; } return null; } diff --git a/apps/web/src/lib/startContainerChat.ts b/apps/web/src/lib/startContainerChat.ts index 610e81c18..173baef11 100644 --- a/apps/web/src/lib/startContainerChat.ts +++ b/apps/web/src/lib/startContainerChat.ts @@ -54,7 +54,7 @@ export async function startContainerChat(input: { return { ok: false, error: input.errorLabel }; } const threadOptions: NewThreadOptions | undefined = - input.fresh === true ? { fresh: true, envMode: "local", worktreePath: null } : undefined; + input.fresh === true ? { fresh: true, workspace: { kind: "local-container" } } : undefined; const threadId = await input.handleNewThread(projectId, threadOptions); return { ok: true, threadId }; } catch (error) { diff --git a/apps/web/src/lib/studioProjects.test.ts b/apps/web/src/lib/studioProjects.test.ts index 9a5cd9eb3..6dd41d409 100644 --- a/apps/web/src/lib/studioProjects.test.ts +++ b/apps/web/src/lib/studioProjects.test.ts @@ -187,6 +187,7 @@ describe("studioProjects", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "intentional", }, }, }), @@ -214,6 +215,7 @@ describe("studioProjects", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "intentional", promotedTo: "thread-real" as ThreadId, }, [terminalDraftThreadId]: { @@ -225,6 +227,7 @@ describe("studioProjects", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "intentional", }, }, }), diff --git a/apps/web/src/lib/temporaryThread.test.ts b/apps/web/src/lib/temporaryThread.test.ts index 97c626e06..8c2c923fb 100644 --- a/apps/web/src/lib/temporaryThread.test.ts +++ b/apps/web/src/lib/temporaryThread.test.ts @@ -26,6 +26,7 @@ describe("resolveTemporaryThreadIdToDelete", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", isTemporary: true, }, }, @@ -48,6 +49,7 @@ describe("resolveTemporaryThreadIdToDelete", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", }, }, }), @@ -69,6 +71,7 @@ describe("resolveTemporaryThreadIdToDelete", () => { branch: null, worktreePath: null, envMode: "local", + workspaceOrigin: "default", isTemporary: true, }, }, diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index 461c44853..1a18dd3f4 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -2,12 +2,11 @@ import { ProjectId, type ModelSelection, ThreadId } from "@synara/contracts"; import { describe, expect, it } from "vitest"; import { type ComposerThreadDraftState, type DraftThreadState } from "../composerDraftStore"; import { - buildDraftThreadContextPatch, + buildDraftThreadWorkspacePatch, createActiveDraftThreadSnapshot, createActiveThreadSnapshot, createFreshDraftThreadSeed, - hasDraftContextOverrides, - resolveInheritedThreadContext, + resolveNewThreadWorkspace, resolveTerminalThreadCreationState, resolveThreadBootstrapPlan, shouldReuseActiveDraftThread, @@ -38,6 +37,7 @@ function makeDraftThread(partial?: Partial): DraftThreadState branch: "feature/terminal-bootstrap", worktreePath: "/repo/.worktrees/terminal-bootstrap", envMode: "worktree", + workspaceOrigin: "default", ...partial, }; } @@ -70,31 +70,89 @@ function makeComposerDraftState( } describe("threadBootstrap", () => { - it("detects when a draft context override is present", () => { - expect(hasDraftContextOverrides()).toBe(false); - expect(hasDraftContextOverrides({ branch: "feature/new-branch" })).toBe(true); + it("resolves project defaults and exact existing workspaces without partial states", () => { + expect(resolveNewThreadWorkspace({ kind: "project-default" }, "worktree")).toEqual({ + branch: null, + worktreePath: null, + envMode: "worktree", + workspaceOrigin: "default", + }); + expect(resolveNewThreadWorkspace({ kind: "local-container" }, "worktree")).toEqual({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "intentional", + }); + expect( + resolveNewThreadWorkspace( + { + kind: "existing-worktree", + branch: "feature/new-branch", + worktreePath: "/repo/.worktrees/new-branch", + }, + "local", + ), + ).toEqual({ + branch: "feature/new-branch", + worktreePath: "/repo/.worktrees/new-branch", + envMode: "worktree", + workspaceOrigin: "intentional", + }); + expect( + resolveNewThreadWorkspace({ kind: "existing-local", branch: "feature/local" }, "worktree"), + ).toEqual({ + branch: "feature/local", + worktreePath: null, + envMode: "local", + workspaceOrigin: "intentional", + }); }); - it("builds a draft patch only when overrides are provided", () => { - expect(buildDraftThreadContextPatch("terminal")).toBeNull(); + it("recomputes inactive default drafts while preserving active and intentional workspaces", () => { expect( - buildDraftThreadContextPatch("terminal", { - branch: "feature/new-branch", - worktreePath: "/repo/.worktrees/new-branch", + buildDraftThreadWorkspacePatch({ + defaultEnvMode: "local", + draftThread: makeDraftThread(), + entryPoint: "terminal", + reuseKind: "stored", }), ).toEqual({ - branch: "feature/new-branch", - worktreePath: "/repo/.worktrees/new-branch", + branch: null, + envMode: "local", + worktreePath: null, entryPoint: "terminal", + workspaceOrigin: "default", }); expect( - buildDraftThreadContextPatch("terminal", { - envMode: "local", + buildDraftThreadWorkspacePatch({ + defaultEnvMode: "local", + draftThread: makeDraftThread(), + entryPoint: "terminal", + reuseKind: "route", + }), + ).toBeNull(); + expect( + buildDraftThreadWorkspacePatch({ + defaultEnvMode: "local", + draftThread: makeDraftThread({ workspaceOrigin: "intentional" }), + entryPoint: "terminal", + reuseKind: "stored", + }), + ).toBeNull(); + expect( + buildDraftThreadWorkspacePatch({ + defaultEnvMode: "local", + draftThread: makeDraftThread({ workspaceOrigin: "intentional" }), + entryPoint: "terminal", + options: { workspace: { kind: "project-default" } }, + reuseKind: "stored", }), ).toEqual({ + branch: null, envMode: "local", worktreePath: null, entryPoint: "terminal", + workspaceOrigin: "default", }); }); @@ -172,74 +230,18 @@ describe("threadBootstrap", () => { }); }); - it("lets an active draft override inherited branch and worktree context", () => { - expect( - resolveInheritedThreadContext({ - activeThread: { - branch: "feature/server-thread", - worktreePath: "/repo/.worktrees/server-thread", - envMode: "worktree", - }, - activeDraftThread: makeDraftThread({ - branch: "feature/draft-thread", - worktreePath: "/repo/.worktrees/draft-thread", - envMode: "worktree", - }), - }), - ).toEqual({ - branch: "feature/draft-thread", - worktreePath: "/repo/.worktrees/draft-thread", - envMode: "worktree", - }); - }); - - it("lets a local active draft clear active thread branch and worktree context", () => { - expect( - resolveInheritedThreadContext({ - activeThread: { - branch: "feature/server-thread", - worktreePath: "/repo/.worktrees/server-thread", - envMode: "worktree", - }, - activeDraftThread: makeDraftThread({ - branch: null, - worktreePath: null, - envMode: "local", - }), - }), - ).toEqual({ - branch: null, - worktreePath: null, - envMode: "local", - }); - }); - - it("derives inherited environment mode from the active thread when no draft exists", () => { - expect( - resolveInheritedThreadContext({ - activeThread: { - branch: "feature/server-thread", - worktreePath: "/repo/.worktrees/server-thread", - envMode: undefined, - }, - activeDraftThread: null, - }), - ).toEqual({ - branch: "feature/server-thread", - worktreePath: "/repo/.worktrees/server-thread", - envMode: "worktree", - }); - }); - it("builds the fresh draft seed from creation inputs", () => { expect( createFreshDraftThreadSeed({ createdAt: "2026-04-05T10:00:00.000Z", + defaultEnvMode: "local", entryPoint: "terminal", options: { - branch: "feature/new-terminal", - worktreePath: "/repo/.worktrees/new-terminal", - envMode: "worktree", + workspace: { + kind: "existing-worktree", + branch: "feature/new-terminal", + worktreePath: "/repo/.worktrees/new-terminal", + }, }, }), ).toEqual({ @@ -247,6 +249,7 @@ describe("threadBootstrap", () => { branch: "feature/new-terminal", worktreePath: "/repo/.worktrees/new-terminal", envMode: "worktree", + workspaceOrigin: "intentional", runtimeMode: "full-access", entryPoint: "terminal", }); @@ -256,6 +259,7 @@ describe("threadBootstrap", () => { expect( createFreshDraftThreadSeed({ createdAt: "2026-04-05T10:00:00.000Z", + defaultEnvMode: "worktree", entryPoint: "chat", options: { temporary: true, @@ -265,7 +269,8 @@ describe("threadBootstrap", () => { createdAt: "2026-04-05T10:00:00.000Z", branch: null, worktreePath: null, - envMode: "local", + envMode: "worktree", + workspaceOrigin: "default", runtimeMode: "full-access", entryPoint: "chat", isTemporary: true, @@ -282,9 +287,9 @@ describe("threadBootstrap", () => { runtimeMode: "full-access", interactionMode: "default", }, + defaultEnvMode: "local", draftComposerState: makeComposerDraftState(), draftThread: makeDraftThread(), - options: undefined, projectDefaultModelSelection: modelSelection("codex", "gpt-5.4"), projectId: PROJECT_ID, }), @@ -311,9 +316,9 @@ describe("threadBootstrap", () => { runtimeMode: "full-access", interactionMode: "plan", }, + defaultEnvMode: "local", draftComposerState: makeComposerDraftState(), draftThread: null, - options: undefined, projectDefaultModelSelection: modelSelection("codex", "gpt-5.4"), projectId: PROJECT_ID, }).interactionMode, @@ -330,16 +335,16 @@ describe("threadBootstrap", () => { runtimeMode: "full-access", interactionMode: "default", }, + defaultEnvMode: "local", draftComposerState: makeComposerDraftState(), draftThread: makeDraftThread({ interactionMode: "plan" }), - options: undefined, projectDefaultModelSelection: modelSelection("codex", "gpt-5.4"), projectId: PROJECT_ID, }).interactionMode, ).toBe("plan"); }); - it("clears inherited worktree state when an explicit local env override is requested", () => { + it("uses the configured default when no draft workspace exists", () => { expect( resolveTerminalThreadCreationState({ activeDraftThread: null, @@ -350,18 +355,16 @@ describe("threadBootstrap", () => { interactionMode: "default", envMode: "worktree", }, + defaultEnvMode: "local", draftComposerState: makeComposerDraftState(), - draftThread: makeDraftThread(), - options: { - envMode: "local", - }, + draftThread: null, projectDefaultModelSelection: modelSelection("codex", "gpt-5.4"), projectId: PROJECT_ID, }), ).toMatchObject({ envMode: "local", worktreePath: null, - branch: "feature/terminal-bootstrap", + branch: null, }); }); }); diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index 18ddb1ae5..dbc92c975 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -14,57 +14,74 @@ import { type ThreadEnvironmentMode, type ThreadId, } from "@synara/contracts"; -import { resolveThreadEnvironmentMode } from "@synara/shared/threadEnvironment"; import { type ComposerThreadDraftState, type DraftThreadEnvMode, type DraftThreadState, + type DraftThreadWorkspaceOrigin, resolvePreferredComposerModelSelection, } from "../composerDraftStore"; -import { DEFAULT_INTERACTION_MODE, type Thread, type ThreadPrimarySurface } from "../types"; +import { DEFAULT_INTERACTION_MODE, type ThreadPrimarySurface } from "../types"; export interface NewThreadOptions { - branch?: string | null; - worktreePath?: string | null; - envMode?: DraftThreadEnvMode; + workspace?: NewThreadWorkspaceIntent; entryPoint?: ThreadPrimarySurface; temporary?: boolean; provider?: ProviderKind; fresh?: boolean; } -export interface InheritedThreadContext { +export type NewThreadWorkspaceIntent = + | { readonly kind: "project-default" } + | { readonly kind: "local-container" } + | { readonly kind: "existing-local"; readonly branch: string } + | { + readonly kind: "existing-worktree"; + readonly branch: string; + readonly worktreePath: string; + }; + +export interface ResolvedNewThreadWorkspace { branch: string | null; worktreePath: string | null; envMode: DraftThreadEnvMode; + workspaceOrigin: DraftThreadWorkspaceOrigin; } -// Carry the active surface's branch/worktree/env into a new thread bootstrap. -// A pending draft wins outright; otherwise we derive the env mode from the -// active thread's worktree so a fresh thread inherits the same workspace shape. -export function resolveInheritedThreadContext(input: { - activeThread: Pick | null | undefined; - activeDraftThread: - | Pick - | null - | undefined; -}): InheritedThreadContext { - const { activeThread, activeDraftThread } = input; - if (activeDraftThread) { - return { - branch: activeDraftThread.branch, - worktreePath: activeDraftThread.worktreePath, - envMode: activeDraftThread.envMode, - }; +export function resolveNewThreadWorkspace( + intent: NewThreadWorkspaceIntent, + defaultEnvMode: DraftThreadEnvMode, +): ResolvedNewThreadWorkspace { + switch (intent.kind) { + case "project-default": + return { + branch: null, + worktreePath: null, + envMode: defaultEnvMode, + workspaceOrigin: "default", + }; + case "local-container": + return { + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "intentional", + }; + case "existing-local": + return { + branch: intent.branch, + worktreePath: null, + envMode: "local", + workspaceOrigin: "intentional", + }; + case "existing-worktree": + return { + branch: intent.branch, + worktreePath: intent.worktreePath, + envMode: "worktree", + workspaceOrigin: "intentional", + }; } - return { - branch: activeThread?.branch ?? null, - worktreePath: activeThread?.worktreePath ?? null, - envMode: resolveThreadEnvironmentMode({ - envMode: activeThread?.envMode, - worktreePath: activeThread?.worktreePath ?? null, - }), - }; } interface ActiveThreadSnapshot { @@ -97,10 +114,10 @@ export type ThreadBootstrapPlan = DraftReusePlanStored | DraftReusePlanRoute | D interface ResolveTerminalThreadCreationStateInput { activeDraftThread: DraftThreadState | null; activeThread: ActiveThreadSnapshot | null; + defaultEnvMode: DraftThreadEnvMode; defaultProvider?: ProviderKind | null | undefined; draftComposerState: ComposerThreadDraftState | null; draftThread: DraftThreadState | null; - options: NewThreadOptions | undefined; projectDefaultModelSelection: ModelSelection | null; projectId: ProjectId; } @@ -162,6 +179,7 @@ export function createActiveDraftThreadSnapshot( lastKnownPr: activeDraftThread.lastKnownPr ?? null, envMode: activeDraftThread.envMode, ...(activeDraftThread.isTemporary ? { isTemporary: true } : {}), + workspaceOrigin: activeDraftThread.workspaceOrigin, }; } @@ -200,51 +218,57 @@ export function resolveThreadBootstrapPlan(input: { // Build the initial draft-thread metadata for a brand new thread bootstrap. export function createFreshDraftThreadSeed(input: { createdAt: string; + defaultEnvMode: DraftThreadEnvMode; entryPoint: ThreadPrimarySurface; - options: NewThreadOptions | undefined; + options?: NewThreadOptions | undefined; }): Omit { + const workspace = resolveNewThreadWorkspace( + input.options?.workspace ?? { kind: "project-default" }, + input.defaultEnvMode, + ); return { createdAt: input.createdAt, - branch: input.options?.branch ?? null, - worktreePath: input.options?.worktreePath ?? null, - envMode: input.options?.envMode ?? "local", + ...workspace, runtimeMode: DEFAULT_RUNTIME_MODE, entryPoint: input.entryPoint, ...(input.options?.temporary ? { isTemporary: true } : {}), }; } -// Detect whether the caller wants to override stored draft context before reuse. -export function hasDraftContextOverrides(options?: NewThreadOptions): boolean { - return ( - options?.branch !== undefined || - options?.worktreePath !== undefined || - options?.envMode !== undefined - ); -} - -// Build the exact patch we should apply to an existing draft before reusing it. -export function buildDraftThreadContextPatch( - entryPoint: ThreadPrimarySurface, - options?: NewThreadOptions, -): { - branch?: string | null; +// Reopening an inactive default-derived draft recomputes the current project default so +// stale inherited branches cannot return. An active route draft and an intentionally chosen +// workspace are preserved unless a caller explicitly supplies a new workspace intent. +export function buildDraftThreadWorkspacePatch(input: { + defaultEnvMode: DraftThreadEnvMode; + draftThread: DraftThreadState; entryPoint: ThreadPrimarySurface; - envMode?: DraftThreadEnvMode; - worktreePath?: string | null; + options?: NewThreadOptions | undefined; + reuseKind: "route" | "stored"; +}): { + branch: string | null; + entryPoint: ThreadPrimarySurface; + envMode: DraftThreadEnvMode; + workspaceOrigin: DraftThreadWorkspaceOrigin; + worktreePath: string | null; } | null { - if (!hasDraftContextOverrides(options)) { + const workspaceWasSpecified = + input.options !== undefined && Object.hasOwn(input.options, "workspace"); + if (input.reuseKind === "route" && !workspaceWasSpecified) { + return null; + } + if ( + input.reuseKind === "stored" && + !workspaceWasSpecified && + input.draftThread.workspaceOrigin === "intentional" + ) { return null; } - const shouldClearWorktreeForLocalMode = - options?.envMode === "local" && options?.worktreePath === undefined; return { - ...(options?.branch !== undefined ? { branch: options.branch ?? null } : {}), - ...(options?.worktreePath !== undefined || shouldClearWorktreeForLocalMode - ? { worktreePath: options?.worktreePath ?? null } - : {}), - ...(options?.envMode !== undefined ? { envMode: options.envMode } : {}), - entryPoint, + ...resolveNewThreadWorkspace( + input.options?.workspace ?? { kind: "project-default" }, + input.defaultEnvMode, + ), + entryPoint: input.entryPoint, }; } @@ -272,20 +296,6 @@ export function shouldReuseActiveDraftThread(input: { export function resolveTerminalThreadCreationState( input: ResolveTerminalThreadCreationStateInput, ): TerminalThreadCreationState { - const hasExplicitEnvModeOverride = - input.options !== undefined && Object.hasOwn(input.options, "envMode"); - const explicitEnvMode: DraftThreadEnvMode | undefined = hasExplicitEnvModeOverride - ? (input.options?.envMode ?? "local") - : undefined; - const inheritedEnvMode = - input.draftThread?.envMode !== undefined - ? input.draftThread.envMode - : input.activeThread?.projectId === input.projectId - ? input.activeThread.envMode - : input.activeDraftThread?.projectId === input.projectId - ? input.activeDraftThread.envMode - : undefined; - return { modelSelection: resolvePreferredComposerModelSelection({ draft: input.draftComposerState, @@ -316,21 +326,8 @@ export function resolveTerminalThreadCreationState( ? (input.activeDraftThread.lastKnownPr ?? null) : null) ?? null, - envMode: hasExplicitEnvModeOverride - ? (explicitEnvMode ?? "local") - : (inheritedEnvMode ?? "local"), - branch: - input.options?.branch !== undefined - ? (input.options.branch ?? null) - : (input.draftThread?.branch ?? null), - worktreePath: (() => { - if (input.options?.worktreePath !== undefined) { - return input.options.worktreePath ?? null; - } - if (explicitEnvMode === "local") { - return null; - } - return input.draftThread?.worktreePath ?? null; - })(), + envMode: input.draftThread?.envMode ?? input.defaultEnvMode, + branch: input.draftThread?.branch ?? null, + worktreePath: input.draftThread?.worktreePath ?? null, }; } diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index 2fd56322c..8e6052ee4 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -2101,26 +2101,14 @@ function SingleChatSurface(props: { return; } - await handleNewThread( - projectId, - { - envMode: appSettings.defaultThreadEnvMode, - }, - { - search: (previous) => ({ - ...stripEditorViewSearchParams(stripDiffSearchParams(previous)), - view: "editor", - }), - }, - ); + await handleNewThread(projectId, undefined, { + search: (previous) => ({ + ...stripEditorViewSearchParams(stripDiffSearchParams(previous)), + view: "editor", + }), + }); }, - [ - appSettings.defaultThreadEnvMode, - appSettings.sidebarThreadSortOrder, - handleNewThread, - navigate, - threadSummaries, - ], + [appSettings.sidebarThreadSortOrder, handleNewThread, navigate, threadSummaries], ); const handleSelectEditorProject = useCallback( (projectId: ProjectId) => { diff --git a/apps/web/src/routes/_chat.tsx b/apps/web/src/routes/_chat.tsx index a57d2db12..285603437 100644 --- a/apps/web/src/routes/_chat.tsx +++ b/apps/web/src/routes/_chat.tsx @@ -24,7 +24,6 @@ import { resolveLatestProjectTargetId, resolveNewThreadTarget, } from "../lib/projectShortcutTargets"; -import { resolveInheritedThreadContext } from "../lib/threadBootstrap"; import { isTerminalFocused } from "../lib/terminalFocus"; import { serverConfigQueryOptions } from "../lib/serverReactQuery"; import { startFreshChatForActiveSurface } from "../lib/startContainerChat"; @@ -195,14 +194,8 @@ function ChatRouteGlobalShortcuts() { const clearSelection = useThreadSelectionStore((state) => state.clearSelection); const selectedThreadIdsSize = useThreadSelectionStore((state) => state.selectedThreadIds.size); const terminalStateByThreadId = useTerminalStateStore((state) => state.terminalStateByThreadId); - const { - activeContextThreadId, - activeDraftThread, - activeProjectId, - activeThread, - handleNewThread, - projects, - } = useHandleNewThread(); + const { activeContextThreadId, activeDraftThread, activeProjectId, handleNewThread, projects } = + useHandleNewThread(); const { recentSwitcherState, recentViewEntries, @@ -375,9 +368,6 @@ function ChatRouteGlobalShortcuts() { event.preventDefault(); event.stopPropagation(); void handleNewThread(target.projectId, { - ...(target.inheritContext - ? resolveInheritedThreadContext({ activeThread, activeDraftThread }) - : {}), entryPoint: "terminal", }); return; @@ -413,9 +403,6 @@ function ChatRouteGlobalShortcuts() { } await handleNewThread(target.projectId, { provider, - ...(target.inheritContext - ? resolveInheritedThreadContext({ activeThread, activeDraftThread }) - : {}), }); })(); return; @@ -423,19 +410,13 @@ function ChatRouteGlobalShortcuts() { if (command !== "chat.new") return; // Falls back to the most recent project when none is focused (e.g. the landing - // view) so the primary "new thread" chord always creates a thread; on that - // fallback the active branch/worktree context belongs to the absent project, so - // `resolveNewThreadTarget` omits it and we defer to the target's defaults. + // view) so the primary "new thread" chord always creates a thread. Every ordinary + // new-thread command uses the target project's configured workspace default. const target = resolveNewThreadTarget({ currentProjectId, latestUsableProjectId }); if (!target) return; event.preventDefault(); event.stopPropagation(); - void handleNewThread( - target.projectId, - target.inheritContext - ? resolveInheritedThreadContext({ activeThread, activeDraftThread }) - : undefined, - ); + void handleNewThread(target.projectId); }; window.addEventListener("keydown", onWindowKeyDown, { capture: true }); @@ -443,8 +424,6 @@ function ChatRouteGlobalShortcuts() { window.removeEventListener("keydown", onWindowKeyDown, { capture: true }); }; }, [ - activeDraftThread, - activeThread, cancelRecentSwitcher, clearSelection, commitRecentSwitcherSelection, From 0d6eb37ca2a70d38b30bc4d085ce25bd06edbc97 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 13:35:26 +0300 Subject: [PATCH 02/14] fix(web): serialize distinct new thread intents --- apps/web/src/components/ChatView.browser.tsx | 125 +++++++++++++++++- apps/web/src/components/Sidebar.tsx | 50 ++++--- apps/web/src/hooks/useHandleNewThread.ts | 125 ++++++++++-------- .../web/src/lib/stagedDraftNavigation.test.ts | 42 +++++- apps/web/src/lib/stagedDraftNavigation.ts | 67 ++++++++-- apps/web/src/lib/threadBootstrap.test.ts | 23 ++++ apps/web/src/lib/threadBootstrap.ts | 19 +++ 7 files changed, 354 insertions(+), 97 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index edaaa6a11..7b82ff954 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -43,7 +43,9 @@ import { isMacPlatform } from "../lib/utils"; import { readNativeApi } from "../nativeApi"; import { useProviderConnectionDialogStore } from "../providerConnectionDialogStore"; import { resetHomeChatProjectPrewarmStateForTests } from "../lib/chatProjects"; +import { draftNavigationSlotKey, runDraftNavigationOnce } from "../lib/stagedDraftNavigation"; import { resetStudioProjectPrewarmStateForTests } from "../lib/studioProjects"; +import { newThreadNavigationRequestKey } from "../lib/threadBootstrap"; import { getRouter } from "../router"; import { useSplitViewStore } from "../splitViewStore"; import { useStore } from "../store"; @@ -5619,11 +5621,26 @@ describe("ChatView timeline estimator parity (full app)", () => { }, }), }); + const defaultNavigationBlocker = (() => { + let resolve!: () => void; + const promise = new Promise((nextResolve) => { + resolve = nextResolve; + }); + return { promise, resolve }; + })(); + let defaultNavigationOperation: Promise | null = null; try { - await vi.waitFor(() => expect(branchLookup).toHaveBeenCalled()); + await waitForLayout(); branchLookup.mockClear(); branchLookup.mockImplementation(() => branchLookupDeferred.promise); + const projectValidationCallCount = () => + branchLookup.mock.calls.filter(([input]) => input.cwd === "/repo/project").length; + defaultNavigationOperation = runDraftNavigationOnce( + draftNavigationSlotKey(PROJECT_ID, "chat"), + newThreadNavigationRequestKey({ hasCustomSearch: false }), + () => defaultNavigationBlocker.promise, + ); const threadRow = await waitForElement( () => Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( @@ -5642,7 +5659,9 @@ describe("ChatView timeline estimator parity (full app)", () => { ); openContextMenu(); - await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(1)); + await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(1)); + await Promise.resolve(); + expect(projectValidationCallCount()).toBe(0); openContextMenu(); await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(2)); expect(contextMenuShow.mock.calls[0]?.[0]?.[0]).toMatchObject({ @@ -5650,6 +5669,9 @@ describe("ChatView timeline estimator parity (full app)", () => { label: "New thread in worktree (feature/exact-worktree)", }); + defaultNavigationBlocker.resolve(); + await defaultNavigationOperation; + await vi.waitFor(() => expect(projectValidationCallCount()).toBe(1)); branchLookupDeferred.resolve(exactWorktreeBranchResult); const newThreadPath = await waitForURL( mounted.router, @@ -5657,7 +5679,7 @@ describe("ChatView timeline estimator parity (full app)", () => { "The explicit worktree action should open a fresh draft.", ); const newThreadId = newThreadPath.slice(1) as ThreadId; - expect(branchLookup).toHaveBeenCalledTimes(1); + expect(projectValidationCallCount()).toBe(1); expect(useComposerDraftStore.getState().getDraftThread(newThreadId)).toMatchObject({ branch: "feature/exact-worktree", worktreePath: "/repo/worktrees/exact-worktree", @@ -5665,7 +5687,104 @@ describe("ChatView timeline estimator parity (full app)", () => { workspaceOrigin: "intentional", }); } finally { + defaultNavigationBlocker.resolve(); branchLookupDeferred.resolve(exactWorktreeBranchResult); + await defaultNavigationOperation; + await mounted.cleanup(); + } + }); + + it("fails closed with a visible error when an exact worktree moved", async () => { + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/exact-worktree", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/exact-worktree", + }, + ], + }; + const movedWorktreeBranchResult: Awaited> = { + ...exactWorktreeBranchResult, + branches: [ + { + ...exactWorktreeBranchResult.branches[0]!, + worktreePath: "/repo/worktrees/moved-worktree", + }, + ], + }; + const branchLookup = vi.fn(async () => exactWorktreeBranchResult); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-context-worktree-moved" as MessageId, + targetText: "context worktree moved", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/exact-worktree", + worktreePath: "/repo/worktrees/exact-worktree", + })), + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, + }), + }); + + try { + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalled()); + branchLookup.mockClear(); + branchLookup.mockResolvedValue(movedWorktreeBranchResult); + const startingPath = mounted.router.state.location.pathname; + const startingDraftIds = Object.keys( + useComposerDraftStore.getState().draftThreadsByThreadId, + ).toSorted(); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + + await waitForElement( + () => + Array.from(document.querySelectorAll('[data-slot="toast-title"]')).find( + (element) => element.textContent === "Workspace changed", + ) ?? null, + "A moved exact worktree should show a visible Workspace changed error.", + ); + expect(mounted.router.state.location.pathname).toBe(startingPath); + expect( + Object.keys(useComposerDraftStore.getState().draftThreadsByThreadId).toSorted(), + ).toEqual(startingDraftIds); + } finally { await mounted.cleanup(); } }); diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index e9d57cba6..18c5bd47e 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -1709,7 +1709,7 @@ export default function Sidebar() { ); const [projectRunDialogCommandDraft, setProjectRunDialogCommandDraft] = useState(""); const isAddingProjectRef = useRef(false); - const newThreadInWorkspaceInFlightRef = useRef(false); + const newThreadInWorkspaceInFlightThreadIdsRef = useRef(new Set()); const [projectInitializationPreview, setProjectInitializationPreview] = useState(null); const [projectInitializationError, setProjectInitializationError] = useState(null); @@ -3824,7 +3824,12 @@ export default function Sidebar() { ); if (clicked === "new-thread-in-workspace") { - if (!newThreadInWorkspaceAction || newThreadInWorkspaceInFlightRef.current) return; + if ( + !newThreadInWorkspaceAction || + newThreadInWorkspaceInFlightThreadIdsRef.current.has(threadId) + ) { + return; + } const projectCwd = projectCwdById.get(thread.projectId) ?? null; if (!projectCwd) { showSidebarTransientError({ @@ -3833,24 +3838,31 @@ export default function Sidebar() { }); return; } - newThreadInWorkspaceInFlightRef.current = true; + newThreadInWorkspaceInFlightThreadIdsRef.current.add(threadId); + let workspaceValidationFailure: string | null = null; try { - const branchResult = await api.git.listBranches({ cwd: projectCwd }); - const validation = validateNewThreadInWorkspaceAction({ - action: newThreadInWorkspaceAction, - branches: branchResult.branches, - isRepo: branchResult.isRepo, - projectCwd, - }); - if (!validation.ok) { - showSidebarTransientError({ - title: "Workspace changed", - description: validation.description, - }); - return; - } const createdThreadId = await handleNewThread(thread.projectId, { fresh: true, + prepareFreshCreate: async () => { + const currentProjectCwd = + useStore.getState().projects.find((project) => project.id === thread.projectId) + ?.cwd ?? null; + if (!currentProjectCwd || currentProjectCwd !== projectCwd) { + workspaceValidationFailure = "The project folder changed before the thread opened."; + throw new Error(workspaceValidationFailure); + } + const branchResult = await api.git.listBranches({ cwd: currentProjectCwd }); + const validation = validateNewThreadInWorkspaceAction({ + action: newThreadInWorkspaceAction, + branches: branchResult.branches, + isRepo: branchResult.isRepo, + projectCwd: currentProjectCwd, + }); + if (!validation.ok) { + workspaceValidationFailure = validation.description; + throw new Error(validation.description); + } + }, workspace: newThreadInWorkspaceAction.workspace, }); if (!createdThreadId) { @@ -3861,12 +3873,12 @@ export default function Sidebar() { } } catch (error) { showSidebarTransientError({ - title: "Unable to start thread", + title: workspaceValidationFailure ? "Workspace changed" : "Unable to start thread", description: error instanceof Error ? error.message : "The workspace could not be verified.", }); } finally { - newThreadInWorkspaceInFlightRef.current = false; + newThreadInWorkspaceInFlightThreadIdsRef.current.delete(threadId); } return; } diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index aaf9f97a0..44444e4d4 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -14,6 +14,7 @@ import { createActiveDraftThreadSnapshot, createActiveThreadSnapshot, createFreshDraftThreadSeed, + newThreadNavigationRequestKey, resolveTerminalThreadCreationState, resolveThreadBootstrapPlan, type NewThreadOptions, @@ -266,65 +267,73 @@ export function useHandleNewThread() { })(); } - return runDraftNavigationOnce(draftNavigationSlotKey(projectId, entryPoint), async () => { - const threadId = newThreadId(); - if (wantsTemporaryThread) { - markTemporaryThread(threadId); - } - const createdAt = new Date().toISOString(); - const draftSeed = createFreshDraftThreadSeed({ - createdAt, - defaultEnvMode: settings.defaultThreadEnvMode, - entryPoint, + return runDraftNavigationOnce( + draftNavigationSlotKey(projectId, entryPoint), + newThreadNavigationRequestKey({ + hasCustomSearch: navigation?.search !== undefined, options, - }); - const committed = await stageDraftNavigation({ - // Keep the previous routed draft alive while the destination loads. Replacing the - // project's primary slot earlier makes the route guard redirect the old URL to Home. - stage: () => { - registerDraftThread(threadId, { projectId, ...draftSeed }); - markNewThreadLanding(threadId); - activateThreadEntryPoint(threadId); - applyStickyState(threadId); - applyProviderOverride(threadId); - }, - // Mark the draft-landing navigation as a transition so the new route - // subtree renders interruptibly and the browser can paint the composer - // skeleton immediately instead of freezing on the synchronous commit. - navigate: () => - new Promise((resolve, reject) => { - startTransition(() => { - navigate({ - to: "/$threadId", - params: { threadId }, - ...(navigation?.search ? { search: navigation.search } : {}), - }).then(resolve, reject); - }); - }), - // TanStack resolves an older navigate() promise when a newer navigation supersedes it. - // Verify the committed route before deleting the previous project draft. - isDestinationActive: () => router.state.location.pathname === `/${threadId}`, - finalize: () => setProjectDraftThreadId(projectId, threadId, draftSeed), - rollback: () => { - clearNewThreadLanding(threadId); - clearDraftThread(threadId); - clearTerminalState(threadId); - if (wantsTemporaryThread) { - clearTemporaryThread(threadId); - } - }, - }); - if (!committed) { - return null; - } - if (entryPoint === "terminal") { - await createTerminalThread( - threadId, - resolveCreationState(threadId, getDraftThread(threadId)), - ); - } - return threadId; - }); + }), + async () => { + await options?.prepareFreshCreate?.(); + const threadId = newThreadId(); + if (wantsTemporaryThread) { + markTemporaryThread(threadId); + } + const createdAt = new Date().toISOString(); + const draftSeed = createFreshDraftThreadSeed({ + createdAt, + defaultEnvMode: settings.defaultThreadEnvMode, + entryPoint, + options, + }); + const committed = await stageDraftNavigation({ + // Keep the previous routed draft alive while the destination loads. Replacing the + // project's primary slot earlier makes the route guard redirect the old URL to Home. + stage: () => { + registerDraftThread(threadId, { projectId, ...draftSeed }); + markNewThreadLanding(threadId); + activateThreadEntryPoint(threadId); + applyStickyState(threadId); + applyProviderOverride(threadId); + }, + // Mark the draft-landing navigation as a transition so the new route + // subtree renders interruptibly and the browser can paint the composer + // skeleton immediately instead of freezing on the synchronous commit. + navigate: () => + new Promise((resolve, reject) => { + startTransition(() => { + navigate({ + to: "/$threadId", + params: { threadId }, + ...(navigation?.search ? { search: navigation.search } : {}), + }).then(resolve, reject); + }); + }), + // TanStack resolves an older navigate() promise when a newer navigation supersedes it. + // Verify the committed route before deleting the previous project draft. + isDestinationActive: () => router.state.location.pathname === `/${threadId}`, + finalize: () => setProjectDraftThreadId(projectId, threadId, draftSeed), + rollback: () => { + clearNewThreadLanding(threadId); + clearDraftThread(threadId); + clearTerminalState(threadId); + if (wantsTemporaryThread) { + clearTemporaryThread(threadId); + } + }, + }); + if (!committed) { + return null; + } + if (entryPoint === "terminal") { + await createTerminalThread( + threadId, + resolveCreationState(threadId, getDraftThread(threadId)), + ); + } + return threadId; + }, + ); }, [ activeDraftThread, diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index bd210f3f3..f5a49fb68 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -62,7 +62,7 @@ describe("stagedDraftNavigation", () => { expect(rollback).toHaveBeenCalledOnce(); }); - it("coalesces concurrent creation attempts for the same project slot", async () => { + it("coalesces identical requests and serializes different requests for the same slot", async () => { let finishFirst!: (value: string) => void; const firstRun = vi.fn( () => @@ -73,17 +73,45 @@ describe("stagedDraftNavigation", () => { const secondRun = vi.fn(async () => "second"); const slotKey = draftNavigationSlotKey("project-studio", "chat"); - const first = runDraftNavigationOnce(slotKey, firstRun); - const second = runDraftNavigationOnce(slotKey, secondRun); + const first = runDraftNavigationOnce(slotKey, "project-default", firstRun); + const duplicateFirst = runDraftNavigationOnce(slotKey, "project-default", secondRun); + const second = runDraftNavigationOnce(slotKey, "exact-worktree", secondRun); await Promise.resolve(); + expect(secondRun).not.toHaveBeenCalled(); finishFirst("first"); await expect(first).resolves.toBe("first"); - await expect(second).resolves.toBe("first"); + await expect(duplicateFirst).resolves.toBe("first"); + await expect(second).resolves.toBe("second"); expect(firstRun).toHaveBeenCalledOnce(); - expect(secondRun).not.toHaveBeenCalled(); - - await expect(runDraftNavigationOnce(slotKey, secondRun)).resolves.toBe("second"); expect(secondRun).toHaveBeenCalledOnce(); + + await expect(runDraftNavigationOnce(slotKey, "exact-worktree", secondRun)).resolves.toBe( + "second", + ); + expect(secondRun).toHaveBeenCalledTimes(2); + }); + + it("serializes a later project-default request behind an exact-workspace request", async () => { + let finishExact!: (value: string) => void; + const exactRun = vi.fn( + () => + new Promise((resolve) => { + finishExact = resolve; + }), + ); + const defaultRun = vi.fn(async () => "default"); + const slotKey = draftNavigationSlotKey("project-reverse", "chat"); + + const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); + const projectDefault = runDraftNavigationOnce(slotKey, "project-default", defaultRun); + await Promise.resolve(); + expect(defaultRun).not.toHaveBeenCalled(); + finishExact("exact"); + + await expect(exact).resolves.toBe("exact"); + await expect(projectDefault).resolves.toBe("default"); + expect(exactRun).toHaveBeenCalledOnce(); + expect(defaultRun).toHaveBeenCalledOnce(); }); }); diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index 4689019b2..e48f3d146 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -3,27 +3,74 @@ // after their destination route actually commits. // Layer: Web navigation orchestration -const inFlightDraftNavigationBySlot = new Map>(); +interface DraftNavigationSlotState { + tail: Promise; + readonly operationByRequestKey: Map>; +} + +const draftNavigationStateBySlot = new Map(); export function draftNavigationSlotKey(projectId: string, entryPoint: string): string { return `${projectId}\u0000${entryPoint}`; } -/** Coalesces repeated clicks/shortcuts that target the same project + entry-point slot. */ -export function runDraftNavigationOnce(slotKey: string, run: () => Promise): Promise { - const existing = inFlightDraftNavigationBySlot.get(slotKey) as Promise | undefined; +/** + * Coalesces identical requests for one project slot while serializing requests whose workspace, + * provider, or navigation intent differs. This prevents a later exact-workspace action from + * silently joining an earlier project-default navigation (and vice versa). + */ +export function runDraftNavigationOnce( + slotKey: string, + requestKey: string, + run: () => Promise, +): Promise { + let state = draftNavigationStateBySlot.get(slotKey); + if (!state) { + state = { + tail: Promise.resolve(), + operationByRequestKey: new Map(), + }; + draftNavigationStateBySlot.set(slotKey, state); + } + + const existing = state.operationByRequestKey.get(requestKey) as Promise | undefined; if (existing) { return existing; } - const operation = Promise.resolve().then(run); - inFlightDraftNavigationBySlot.set(slotKey, operation); - const clearOperation = () => { - if (inFlightDraftNavigationBySlot.get(slotKey) === operation) { - inFlightDraftNavigationBySlot.delete(slotKey); + const execution = state.tail.then(run, run); + let operation!: Promise; + const clearRequest = () => { + if (state.operationByRequestKey.get(requestKey) === operation) { + state.operationByRequestKey.delete(requestKey); } }; - void operation.then(clearOperation, clearOperation); + operation = execution.then( + (value) => { + clearRequest(); + return value; + }, + (error: unknown) => { + clearRequest(); + throw error; + }, + ); + state.operationByRequestKey.set(requestKey, operation); + + const tail = operation.then( + () => undefined, + () => undefined, + ); + state.tail = tail; + void tail.then(() => { + if ( + draftNavigationStateBySlot.get(slotKey) === state && + state.tail === tail && + state.operationByRequestKey.size === 0 + ) { + draftNavigationStateBySlot.delete(slotKey); + } + }); return operation; } diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index 1a18dd3f4..3b26529ea 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -6,6 +6,7 @@ import { createActiveDraftThreadSnapshot, createActiveThreadSnapshot, createFreshDraftThreadSeed, + newThreadNavigationRequestKey, resolveNewThreadWorkspace, resolveTerminalThreadCreationState, resolveThreadBootstrapPlan, @@ -70,6 +71,28 @@ function makeComposerDraftState( } describe("threadBootstrap", () => { + it("uses distinct navigation request keys for default and exact workspace intents", () => { + const defaultKey = newThreadNavigationRequestKey({ hasCustomSearch: false }); + expect( + newThreadNavigationRequestKey({ + hasCustomSearch: false, + options: { workspace: { kind: "project-default" } }, + }), + ).toBe(defaultKey); + expect( + newThreadNavigationRequestKey({ + hasCustomSearch: false, + options: { + workspace: { + kind: "existing-worktree", + branch: "feature/exact", + worktreePath: "/repo/worktrees/exact", + }, + }, + }), + ).not.toBe(defaultKey); + }); + it("resolves project defaults and exact existing workspaces without partial states", () => { expect(resolveNewThreadWorkspace({ kind: "project-default" }, "worktree")).toEqual({ branch: null, diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index dbc92c975..cd6de5bf4 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -29,6 +29,8 @@ export interface NewThreadOptions { temporary?: boolean; provider?: ProviderKind; fresh?: boolean; + /** Runs after this fresh request owns its project navigation slot and before draft staging. */ + prepareFreshCreate?: () => Promise; } export type NewThreadWorkspaceIntent = @@ -48,6 +50,23 @@ export interface ResolvedNewThreadWorkspace { workspaceOrigin: DraftThreadWorkspaceOrigin; } +export function newThreadNavigationRequestKey(input: { + readonly hasCustomSearch: boolean; + readonly options?: NewThreadOptions | undefined; +}): string { + const workspace = input.options?.workspace ?? { kind: "project-default" as const }; + const branch = "branch" in workspace ? workspace.branch : ""; + const worktreePath = "worktreePath" in workspace ? workspace.worktreePath : ""; + return [ + workspace.kind, + branch, + worktreePath, + input.options?.provider ?? "", + input.options?.temporary === true ? "temporary" : "durable", + input.hasCustomSearch ? "custom-search" : "default-search", + ].join("\u0000"); +} + export function resolveNewThreadWorkspace( intent: NewThreadWorkspaceIntent, defaultEnvMode: DraftThreadEnvMode, From 55e35d66c40586ea7ca65178d4e34841ca8f1312 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 13:37:18 +0300 Subject: [PATCH 03/14] test(web): type exact workspace branch mocks --- apps/web/src/components/ChatView.browser.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 7b82ff954..a3b4065db 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5593,7 +5593,9 @@ describe("ChatView timeline estimator parity (full app)", () => { ); return { promise, resolve }; })(); - const branchLookup = vi.fn(async () => exactWorktreeBranchResult); + const branchLookup = vi.fn( + async () => exactWorktreeBranchResult, + ); const baseSnapshot = createSnapshotForTargetUser({ targetMessageId: "msg-user-context-worktree-test" as MessageId, targetText: "context worktree test", @@ -5719,7 +5721,9 @@ describe("ChatView timeline estimator parity (full app)", () => { }, ], }; - const branchLookup = vi.fn(async () => exactWorktreeBranchResult); + const branchLookup = vi.fn( + async () => exactWorktreeBranchResult, + ); const baseSnapshot = createSnapshotForTargetUser({ targetMessageId: "msg-user-context-worktree-moved" as MessageId, targetText: "context worktree moved", From 3c2e0cb29cb25c5fffc63bffaf7cf117142b6e4d Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 13:40:55 +0300 Subject: [PATCH 04/14] fix(web): preserve new thread request ordering --- .../web/src/lib/stagedDraftNavigation.test.ts | 36 +++++++++++++++++++ apps/web/src/lib/stagedDraftNavigation.ts | 27 +++++++------- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index f5a49fb68..fd83655b5 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -114,4 +114,40 @@ describe("stagedDraftNavigation", () => { expect(exactRun).toHaveBeenCalledOnce(); expect(defaultRun).toHaveBeenCalledOnce(); }); + + it("preserves default-exact-default ordering instead of rejoining the first request", async () => { + let finishFirstDefault!: (value: string) => void; + const calls: string[] = []; + const firstDefaultRun = vi.fn( + () => + new Promise((resolve) => { + calls.push("default:first"); + finishFirstDefault = resolve; + }), + ); + const exactRun = vi.fn(async () => { + calls.push("exact"); + return "exact"; + }); + const lastDefaultRun = vi.fn(async () => { + calls.push("default:last"); + return "default:last"; + }); + const slotKey = draftNavigationSlotKey("project-three-actions", "chat"); + + const firstDefault = runDraftNavigationOnce(slotKey, "project-default", firstDefaultRun); + const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); + const lastDefault = runDraftNavigationOnce(slotKey, "project-default", lastDefaultRun); + await Promise.resolve(); + expect(calls).toEqual(["default:first"]); + finishFirstDefault("default:first"); + + await expect(firstDefault).resolves.toBe("default:first"); + await expect(exact).resolves.toBe("exact"); + await expect(lastDefault).resolves.toBe("default:last"); + expect(calls).toEqual(["default:first", "exact", "default:last"]); + expect(firstDefaultRun).toHaveBeenCalledOnce(); + expect(exactRun).toHaveBeenCalledOnce(); + expect(lastDefaultRun).toHaveBeenCalledOnce(); + }); }); diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index e48f3d146..bab76cf9c 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -5,7 +5,8 @@ interface DraftNavigationSlotState { tail: Promise; - readonly operationByRequestKey: Map>; + latestOperation: Promise | null; + latestRequestKey: string | null; } const draftNavigationStateBySlot = new Map(); @@ -28,34 +29,36 @@ export function runDraftNavigationOnce( if (!state) { state = { tail: Promise.resolve(), - operationByRequestKey: new Map(), + latestOperation: null, + latestRequestKey: null, }; draftNavigationStateBySlot.set(slotKey, state); } - const existing = state.operationByRequestKey.get(requestKey) as Promise | undefined; - if (existing) { - return existing; + if (state.latestRequestKey === requestKey && state.latestOperation) { + return state.latestOperation as Promise; } const execution = state.tail.then(run, run); let operation!: Promise; - const clearRequest = () => { - if (state.operationByRequestKey.get(requestKey) === operation) { - state.operationByRequestKey.delete(requestKey); + const clearLatestRequest = () => { + if (state.latestOperation === operation) { + state.latestOperation = null; + state.latestRequestKey = null; } }; operation = execution.then( (value) => { - clearRequest(); + clearLatestRequest(); return value; }, (error: unknown) => { - clearRequest(); + clearLatestRequest(); throw error; }, ); - state.operationByRequestKey.set(requestKey, operation); + state.latestOperation = operation; + state.latestRequestKey = requestKey; const tail = operation.then( () => undefined, @@ -66,7 +69,7 @@ export function runDraftNavigationOnce( if ( draftNavigationStateBySlot.get(slotKey) === state && state.tail === tail && - state.operationByRequestKey.size === 0 + state.latestOperation === null ) { draftNavigationStateBySlot.delete(slotKey); } From abd74875a7d321e222006280d540ad17fd522b6b Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 16:54:24 +0300 Subject: [PATCH 05/14] fix(web): preserve latest new thread intent --- apps/web/src/components/ChatView.browser.tsx | 144 ++++++++- apps/web/src/components/Sidebar.tsx | 10 +- apps/web/src/hooks/useHandleNewThread.ts | 297 +++++++++--------- .../web/src/lib/stagedDraftNavigation.test.ts | 90 ++++++ apps/web/src/lib/stagedDraftNavigation.ts | 37 ++- apps/web/src/lib/threadBootstrap.test.ts | 6 + apps/web/src/lib/threadBootstrap.ts | 1 + 7 files changed, 421 insertions(+), 164 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index a3b4065db..02ee29edf 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -43,7 +43,11 @@ import { isMacPlatform } from "../lib/utils"; import { readNativeApi } from "../nativeApi"; import { useProviderConnectionDialogStore } from "../providerConnectionDialogStore"; import { resetHomeChatProjectPrewarmStateForTests } from "../lib/chatProjects"; -import { draftNavigationSlotKey, runDraftNavigationOnce } from "../lib/stagedDraftNavigation"; +import { + draftNavigationSlotKey, + runDraftNavigationOnce, + waitForDraftNavigationIdle, +} from "../lib/stagedDraftNavigation"; import { resetStudioProjectPrewarmStateForTests } from "../lib/studioProjects"; import { newThreadNavigationRequestKey } from "../lib/threadBootstrap"; import { getRouter } from "../router"; @@ -5633,6 +5637,9 @@ describe("ChatView timeline estimator parity (full app)", () => { let defaultNavigationOperation: Promise | null = null; try { + await waitForServerConfigToApply(); + await waitForComposerEditor(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); await waitForLayout(); branchLookup.mockClear(); branchLookup.mockImplementation(() => branchLookupDeferred.promise); @@ -5696,6 +5703,141 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("preserves a reused draft when it supersedes a waiting exact-workspace request", async () => { + const reusableDraftThreadId = ThreadId.makeUnsafe("thread-reused-draft-race"); + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/exact-worktree", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/exact-worktree", + }, + ], + }; + let resolveExactValidation!: ( + value: Awaited>, + ) => void; + const exactValidation = new Promise>>( + (resolve) => { + resolveExactValidation = resolve; + }, + ); + const branchLookup = vi.fn( + async () => exactWorktreeBranchResult, + ); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-reused-draft-race" as MessageId, + targetText: "reused draft race", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/exact-worktree", + worktreePath: "/repo/worktrees/exact-worktree", + })), + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, + }), + }); + + try { + await waitForServerConfigToApply(); + await waitForComposerEditor(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); + await waitForLayout(); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + useComposerDraftStore.getState().setProjectDraftThreadId(PROJECT_ID, reusableDraftThreadId, { + entryPoint: "chat", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + useComposerDraftStore.getState().setPrompt(reusableDraftThreadId, "preserve this draft"); + branchLookup.mockClear(); + branchLookup.mockImplementation(() => exactValidation); + + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(1)); + + await page.getByTestId("new-thread-button").click(); + useComposerDraftStore + .getState() + .setPrompt(reusableDraftThreadId, "newer prompt with attachment"); + useComposerDraftStore.getState().addFiles(reusableDraftThreadId, [ + { + type: "file", + id: "reused-draft-notes", + name: "notes.txt", + mimeType: "text/plain", + sizeBytes: 5, + file: new File(["notes"], "notes.txt", { type: "text/plain" }), + }, + ]); + + resolveExactValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey(PROJECT_ID, "chat")); + await waitForURL( + mounted.router, + (path) => path === `/${reusableDraftThreadId}`, + "The later ordinary New Thread action should keep the reusable draft selected.", + ); + + expect(useComposerDraftStore.getState().getDraftThread(reusableDraftThreadId)).toMatchObject({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "default", + }); + expect( + useComposerDraftStore.getState().draftsByThreadId[reusableDraftThreadId], + ).toMatchObject({ + prompt: "newer prompt with attachment", + files: [expect.objectContaining({ id: "reused-draft-notes", name: "notes.txt" })], + }); + expect( + useComposerDraftStore.getState().getDraftThreadByProjectId(PROJECT_ID, "chat")?.threadId, + ).toBe(reusableDraftThreadId); + expect(document.body.textContent).not.toContain("Unable to start thread"); + } finally { + resolveExactValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey(PROJECT_ID, "chat")); + await mounted.cleanup(); + } + }); + it("fails closed with a visible error when an exact worktree moved", async () => { const contextMenuShow = vi.fn( async (_items: Parameters[0]) => "new-thread-in-workspace", diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 18c5bd47e..2cfabbe1a 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -3841,7 +3841,9 @@ export default function Sidebar() { newThreadInWorkspaceInFlightThreadIdsRef.current.add(threadId); let workspaceValidationFailure: string | null = null; try { - const createdThreadId = await handleNewThread(thread.projectId, { + // A null result means a newer New Thread intent superseded this one; + // the newer owner is responsible for the visible destination. + await handleNewThread(thread.projectId, { fresh: true, prepareFreshCreate: async () => { const currentProjectCwd = @@ -3865,12 +3867,6 @@ export default function Sidebar() { }, workspace: newThreadInWorkspaceAction.workspace, }); - if (!createdThreadId) { - showSidebarTransientError({ - title: "Unable to start thread", - description: "The new thread could not be opened. Try again.", - }); - } } catch (error) { showSidebarTransientError({ title: workspaceValidationFailure ? "Workspace changed" : "Unable to start thread", diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 44444e4d4..03ac69ed9 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -62,16 +62,6 @@ export function useHandleNewThread() { ): Promise => { const entryPoint = options?.entryPoint ?? "chat"; const wantsTemporaryThread = options?.temporary === true; - const applyProviderOverride = (threadId: ThreadId) => { - if (!options?.provider) { - return; - } - const defaultModelSelection = getRecommendedDefaultModelSelection(options.provider); - if (!defaultModelSelection) { - return; - } - setModelSelection(threadId, defaultModelSelection); - }; const restoreComposerDraft = ( threadId: ThreadId, draftState: ComposerThreadDraftState | null, @@ -98,70 +88,11 @@ export function useHandleNewThread() { } openChatThreadPage(threadId); }; - const { - getDraftThread, - getDraftThreadByProjectId, - applyStickyState, - clearDraftThread, - registerDraftThread, - setDraftThreadContext, - setProjectDraftThreadId, - setModelSelection, - } = useComposerDraftStore.getState(); - const shouldForceFreshThread = options?.fresh === true; - - const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); - const latestActiveDraftThreadCandidate: DraftThreadState | null = focusedThreadId - ? getDraftThread(focusedThreadId) - : null; - const storedDraftThread = - !shouldForceFreshThread && - !wantsTemporaryThread && - storedDraftThreadCandidate?.isTemporary !== true - ? storedDraftThreadCandidate - : null; - const latestActiveDraftThread: DraftThreadState | null = - !shouldForceFreshThread && - !wantsTemporaryThread && - latestActiveDraftThreadCandidate?.isTemporary !== true - ? latestActiveDraftThreadCandidate - : null; - const bootstrapPlan = resolveThreadBootstrapPlan({ - storedDraftThread, - latestActiveDraftThread, - entryPoint, - projectId, - routeThreadId: focusedThreadId, - }); - // Read from the store at call time so post-sync sidebar flows can use the latest project defaults. - const projectDefaultModelSelection = - useStore.getState().projects.find((project) => project.id === projectId) - ?.defaultModelSelection ?? null; - const activeThreadSnapshot = createActiveThreadSnapshot(activeThread, projectId); - const activeDraftThreadSnapshot = createActiveDraftThreadSnapshot( - activeDraftThread, - projectId, - ); - const resolveCreationState = ( - targetThreadId: ThreadId, - draftThread: DraftThreadState | null, - ) => - resolveTerminalThreadCreationState({ - activeDraftThread: activeDraftThreadSnapshot, - activeThread: activeThreadSnapshot, - defaultEnvMode: settings.defaultThreadEnvMode, - defaultProvider: options?.provider ?? settings.defaultProvider, - draftComposerState: - useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, - draftThread, - projectDefaultModelSelection, - projectId, - }); // Terminal-first threads need a real orchestration thread immediately so // the sidebar can render them as durable rows instead of draft-only routes. const createTerminalThread = async ( threadId: ThreadId, - creationState: ReturnType, + creationState: ReturnType, ): Promise => { const api = readNativeApi(); if (!api) { @@ -186,95 +117,161 @@ export function useHandleNewThread() { api, ); }; - if (bootstrapPlan.kind === "stored") { - return (async (): Promise => { - if (wantsTemporaryThread) { - markTemporaryThread(bootstrapPlan.threadId); + return runDraftNavigationOnce( + draftNavigationSlotKey(projectId, entryPoint), + newThreadNavigationRequestKey({ + hasCustomSearch: navigation?.search !== undefined, + options, + }), + async (ownership) => { + if (!ownership.isCurrent()) { + return null; } - const preservedComposerDraft = - useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; - let resolvedStoredDraftThread: DraftThreadState | null = bootstrapPlan.draftThread; - const draftContextPatch = buildDraftThreadWorkspacePatch({ - defaultEnvMode: settings.defaultThreadEnvMode, - draftThread: bootstrapPlan.draftThread, + const { + getDraftThread, + getDraftThreadByProjectId, + applyStickyState, + clearDraftThread, + registerDraftThread, + setDraftThreadContext, + setProjectDraftThreadId, + setModelSelection, + } = useComposerDraftStore.getState(); + const applyProviderOverride = (threadId: ThreadId) => { + if (!options?.provider) { + return; + } + const defaultModelSelection = getRecommendedDefaultModelSelection(options.provider); + if (defaultModelSelection) { + setModelSelection(threadId, defaultModelSelection); + } + }; + const currentPathMatch = /^\/([^/]+)$/.exec(router.state.location.pathname); + const currentRouteThreadId = currentPathMatch?.[1] + ? ThreadId.makeUnsafe(decodeURIComponent(currentPathMatch[1])) + : null; + const currentFocusedThreadId = + focusedThreadId === routeThreadId ? currentRouteThreadId : focusedThreadId; + const shouldForceFreshThread = options?.fresh === true; + const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); + const latestActiveDraftThreadCandidate: DraftThreadState | null = currentFocusedThreadId + ? getDraftThread(currentFocusedThreadId) + : null; + const storedDraftThread = + !shouldForceFreshThread && + !wantsTemporaryThread && + storedDraftThreadCandidate?.isTemporary !== true + ? storedDraftThreadCandidate + : null; + const latestActiveDraftThread = + !shouldForceFreshThread && + !wantsTemporaryThread && + latestActiveDraftThreadCandidate?.isTemporary !== true + ? latestActiveDraftThreadCandidate + : null; + const bootstrapPlan = resolveThreadBootstrapPlan({ + storedDraftThread, + latestActiveDraftThread, entryPoint, - options, - reuseKind: "stored", + projectId, + routeThreadId: currentFocusedThreadId, }); - if (draftContextPatch) { - setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); - resolvedStoredDraftThread = getDraftThread(bootstrapPlan.threadId); - } - applyProviderOverride(bootstrapPlan.threadId); - setProjectDraftThreadId(projectId, bootstrapPlan.threadId, { entryPoint }); - restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); - activateThreadEntryPoint(bootstrapPlan.threadId); - if (focusedThreadId === bootstrapPlan.threadId) { - if (entryPoint === "terminal") { + const currentAppState = useStore.getState(); + const activeThreadSnapshot = createActiveThreadSnapshot( + currentFocusedThreadId + ? currentAppState.threads.find((thread) => thread.id === currentFocusedThreadId) + : null, + projectId, + ); + const activeDraftThreadSnapshot = createActiveDraftThreadSnapshot( + latestActiveDraftThreadCandidate, + projectId, + ); + const projectDefaultModelSelection = + currentAppState.projects.find((project) => project.id === projectId) + ?.defaultModelSelection ?? null; + const resolveCreationState = ( + targetThreadId: ThreadId, + draftThread: DraftThreadState | null, + ) => + resolveTerminalThreadCreationState({ + activeDraftThread: activeDraftThreadSnapshot, + activeThread: activeThreadSnapshot, + defaultEnvMode: settings.defaultThreadEnvMode, + defaultProvider: options?.provider ?? settings.defaultProvider, + draftComposerState: + useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, + draftThread, + projectDefaultModelSelection, + projectId, + }); + + if (bootstrapPlan.kind !== "fresh") { + const preservedComposerDraft = + useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; + if ( + bootstrapPlan.kind === "stored" && + currentFocusedThreadId !== bootstrapPlan.threadId + ) { + try { + await navigate({ + to: "/$threadId", + params: { threadId: bootstrapPlan.threadId }, + ...(navigation?.search ? { search: navigation.search } : {}), + }); + } catch (error) { + if (!ownership.isCurrent()) { + return null; + } + throw error; + } + if ( + !ownership.isCurrent() || + router.state.location.pathname !== `/${bootstrapPlan.threadId}` + ) { + return null; + } + } + if (!ownership.isCurrent()) { + return null; + } + const draftContextPatch = buildDraftThreadWorkspacePatch({ + defaultEnvMode: settings.defaultThreadEnvMode, + draftThread: bootstrapPlan.draftThread, + entryPoint, + options, + reuseKind: bootstrapPlan.kind, + }); + if (draftContextPatch) { + setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); + } + applyProviderOverride(bootstrapPlan.threadId); + setProjectDraftThreadId(projectId, bootstrapPlan.threadId, { entryPoint }); + restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); + activateThreadEntryPoint(bootstrapPlan.threadId); + if (entryPoint === "terminal" && ownership.isCurrent()) { await createTerminalThread( bootstrapPlan.threadId, - resolveCreationState(bootstrapPlan.threadId, resolvedStoredDraftThread), + resolveCreationState( + bootstrapPlan.threadId, + getDraftThread(bootstrapPlan.threadId), + ), ); } return bootstrapPlan.threadId; } - await navigate({ - to: "/$threadId", - params: { threadId: bootstrapPlan.threadId }, - ...(navigation?.search ? { search: navigation.search } : {}), - }); - restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); - if (entryPoint === "terminal") { - await createTerminalThread( - bootstrapPlan.threadId, - resolveCreationState(bootstrapPlan.threadId, resolvedStoredDraftThread), - ); - } - return bootstrapPlan.threadId; - })(); - } - if (bootstrapPlan.kind === "route") { - return (async (): Promise => { - if (wantsTemporaryThread) { - markTemporaryThread(bootstrapPlan.threadId); - } - const preservedComposerDraft = - useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; - let resolvedActiveDraftThread: DraftThreadState | null = bootstrapPlan.draftThread; - const draftContextPatch = buildDraftThreadWorkspacePatch({ - defaultEnvMode: settings.defaultThreadEnvMode, - draftThread: bootstrapPlan.draftThread, - entryPoint, - options, - reuseKind: "route", - }); - if (draftContextPatch) { - setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); - resolvedActiveDraftThread = getDraftThread(bootstrapPlan.threadId); + try { + await options?.prepareFreshCreate?.(); + } catch (error) { + if (!ownership.isCurrent()) { + return null; + } + throw error; } - applyProviderOverride(bootstrapPlan.threadId); - setProjectDraftThreadId(projectId, bootstrapPlan.threadId, { entryPoint }); - restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); - activateThreadEntryPoint(bootstrapPlan.threadId); - if (entryPoint === "terminal") { - await createTerminalThread( - bootstrapPlan.threadId, - resolveCreationState(bootstrapPlan.threadId, resolvedActiveDraftThread), - ); + if (!ownership.isCurrent()) { + return null; } - return bootstrapPlan.threadId; - })(); - } - - return runDraftNavigationOnce( - draftNavigationSlotKey(projectId, entryPoint), - newThreadNavigationRequestKey({ - hasCustomSearch: navigation?.search !== undefined, - options, - }), - async () => { - await options?.prepareFreshCreate?.(); const threadId = newThreadId(); if (wantsTemporaryThread) { markTemporaryThread(threadId); @@ -287,6 +284,7 @@ export function useHandleNewThread() { options, }); const committed = await stageDraftNavigation({ + isCurrent: ownership.isCurrent, // Keep the previous routed draft alive while the destination loads. Replacing the // project's primary slot earlier makes the route guard redirect the old URL to Home. stage: () => { @@ -325,7 +323,7 @@ export function useHandleNewThread() { if (!committed) { return null; } - if (entryPoint === "terminal") { + if (entryPoint === "terminal" && ownership.isCurrent()) { await createTerminalThread( threadId, resolveCreationState(threadId, getDraftThread(threadId)), @@ -336,8 +334,6 @@ export function useHandleNewThread() { ); }, [ - activeDraftThread, - activeThread, clearTemporaryThread, clearTerminalState, navigate, @@ -346,6 +342,7 @@ export function useHandleNewThread() { focusedThreadId, markTemporaryThread, router, + routeThreadId, settings.defaultProvider, settings.defaultThreadEnvMode, ], diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index fd83655b5..f03eb52ab 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -11,6 +11,7 @@ describe("stagedDraftNavigation", () => { const calls: string[] = []; const committed = await stageDraftNavigation({ + isCurrent: () => true, stage: () => calls.push("stage"), navigate: async () => { calls.push("navigate"); @@ -32,6 +33,7 @@ describe("stagedDraftNavigation", () => { const rollback = vi.fn(); const committed = await stageDraftNavigation({ + isCurrent: () => true, stage: vi.fn(), navigate: async () => undefined, isDestinationActive: () => false, @@ -50,6 +52,7 @@ describe("stagedDraftNavigation", () => { await expect( stageDraftNavigation({ + isCurrent: () => true, stage: vi.fn(), navigate: async () => { throw error; @@ -62,6 +65,65 @@ describe("stagedDraftNavigation", () => { expect(rollback).toHaveBeenCalledOnce(); }); + it("does not stage work after its ownership was superseded", async () => { + const stage = vi.fn(); + const rollback = vi.fn(); + + await expect( + stageDraftNavigation({ + isCurrent: () => false, + stage, + navigate: vi.fn(async () => undefined), + isDestinationActive: () => true, + finalize: vi.fn(), + rollback, + }), + ).resolves.toBe(false); + expect(stage).not.toHaveBeenCalled(); + expect(rollback).not.toHaveBeenCalled(); + }); + + it("rolls back staged work when ownership changes during navigation", async () => { + let current = true; + const finalize = vi.fn(); + const rollback = vi.fn(); + + await expect( + stageDraftNavigation({ + isCurrent: () => current, + stage: vi.fn(), + navigate: async () => { + current = false; + }, + isDestinationActive: () => true, + finalize, + rollback, + }), + ).resolves.toBe(false); + expect(finalize).not.toHaveBeenCalled(); + expect(rollback).toHaveBeenCalledOnce(); + }); + + it("suppresses a stale navigation failure after ownership changes", async () => { + let current = true; + const rollback = vi.fn(); + + await expect( + stageDraftNavigation({ + isCurrent: () => current, + stage: vi.fn(), + navigate: async () => { + current = false; + throw new Error("superseded navigation"); + }, + isDestinationActive: () => false, + finalize: vi.fn(), + rollback, + }), + ).resolves.toBe(false); + expect(rollback).toHaveBeenCalledOnce(); + }); + it("coalesces identical requests and serializes different requests for the same slot", async () => { let finishFirst!: (value: string) => void; const firstRun = vi.fn( @@ -115,6 +177,34 @@ describe("stagedDraftNavigation", () => { expect(defaultRun).toHaveBeenCalledOnce(); }); + it("supersedes an awaited owner as soon as a distinct later intent arrives", async () => { + let releaseFirst!: () => void; + const firstOwnership: Array<{ readonly isCurrent: () => boolean }> = []; + let secondWasCurrent = false; + const slotKey = draftNavigationSlotKey("project-supersession", "chat"); + + const first = runDraftNavigationOnce(slotKey, "exact-worktree", async (ownership) => { + firstOwnership.push(ownership); + await new Promise((resolve) => { + releaseFirst = resolve; + }); + return ownership.isCurrent() ? "stale-commit" : "superseded"; + }); + await Promise.resolve(); + expect(firstOwnership[0]?.isCurrent()).toBe(true); + + const second = runDraftNavigationOnce(slotKey, "project-default", async (ownership) => { + secondWasCurrent = ownership.isCurrent(); + return ownership.isCurrent() ? "latest" : "superseded"; + }); + expect(firstOwnership[0]?.isCurrent()).toBe(false); + releaseFirst(); + + await expect(first).resolves.toBe("superseded"); + await expect(second).resolves.toBe("latest"); + expect(secondWasCurrent).toBe(true); + }); + it("preserves default-exact-default ordering instead of rejoining the first request", async () => { let finishFirstDefault!: (value: string) => void; const calls: string[] = []; diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index bab76cf9c..b6e13b7b3 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -7,6 +7,11 @@ interface DraftNavigationSlotState { tail: Promise; latestOperation: Promise | null; latestRequestKey: string | null; + latestOwner: symbol | null; +} + +export interface DraftNavigationOwnership { + readonly isCurrent: () => boolean; } const draftNavigationStateBySlot = new Map(); @@ -15,15 +20,20 @@ export function draftNavigationSlotKey(projectId: string, entryPoint: string): s return `${projectId}\u0000${entryPoint}`; } +/** Resolves after the operations currently queued for a project slot have drained. */ +export function waitForDraftNavigationIdle(slotKey: string): Promise { + return draftNavigationStateBySlot.get(slotKey)?.tail ?? Promise.resolve(); +} + /** - * Coalesces identical requests for one project slot while serializing requests whose workspace, - * provider, or navigation intent differs. This prevents a later exact-workspace action from - * silently joining an earlier project-default navigation (and vice versa). + * Coalesces adjacent identical requests while serializing distinct requests for one project slot. + * A distinct later request becomes the owner immediately, allowing awaited work to stop before a + * stale navigation or draft-mapping commit can overwrite the user's latest intent. */ export function runDraftNavigationOnce( slotKey: string, requestKey: string, - run: () => Promise, + run: (ownership: DraftNavigationOwnership) => Promise, ): Promise { let state = draftNavigationStateBySlot.get(slotKey); if (!state) { @@ -31,6 +41,7 @@ export function runDraftNavigationOnce( tail: Promise.resolve(), latestOperation: null, latestRequestKey: null, + latestOwner: null, }; draftNavigationStateBySlot.set(slotKey, state); } @@ -39,12 +50,19 @@ export function runDraftNavigationOnce( return state.latestOperation as Promise; } - const execution = state.tail.then(run, run); + const owner = Symbol(requestKey); + const ownership: DraftNavigationOwnership = { + isCurrent: () => state.latestOwner === owner, + }; + state.latestOwner = owner; + const execute = () => run(ownership); + const execution = state.tail.then(execute, execute); let operation!: Promise; const clearLatestRequest = () => { if (state.latestOperation === operation) { state.latestOperation = null; state.latestRequestKey = null; + state.latestOwner = null; } }; operation = execution.then( @@ -82,6 +100,7 @@ export function runDraftNavigationOnce( * rolls the staged draft back without treating the user's newer navigation as an error. */ export async function stageDraftNavigation(input: { + readonly isCurrent: () => boolean; readonly stage: () => void; readonly navigate: () => Promise; readonly isDestinationActive: () => boolean; @@ -98,9 +117,12 @@ export async function stageDraftNavigation(input: { }; try { + if (!input.isCurrent()) { + return false; + } input.stage(); await input.navigate(); - if (!input.isDestinationActive()) { + if (!input.isCurrent() || !input.isDestinationActive()) { rollbackOnce(); return false; } @@ -108,6 +130,9 @@ export async function stageDraftNavigation(input: { return true; } catch (error) { rollbackOnce(); + if (!input.isCurrent()) { + return false; + } throw error; } } diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index 3b26529ea..cf95d82bf 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -91,6 +91,12 @@ describe("threadBootstrap", () => { }, }), ).not.toBe(defaultKey); + expect( + newThreadNavigationRequestKey({ + hasCustomSearch: false, + options: { fresh: true }, + }), + ).not.toBe(defaultKey); }); it("resolves project defaults and exact existing workspaces without partial states", () => { diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index cd6de5bf4..0a662c928 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -63,6 +63,7 @@ export function newThreadNavigationRequestKey(input: { worktreePath, input.options?.provider ?? "", input.options?.temporary === true ? "temporary" : "durable", + input.options?.fresh === true ? "fresh" : "reuse-eligible", input.hasCustomSearch ? "custom-search" : "default-search", ].join("\u0000"); } From cb40a5ec4adbfc4fb860f1a77c65af5c88e16b50 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 17:39:30 +0300 Subject: [PATCH 06/14] fix(web): coordinate all new thread navigation --- apps/web/src/components/ChatView.browser.tsx | 286 +++++++++++++++++- apps/web/src/components/Sidebar.tsx | 2 +- .../pullRequest/PullRequestDetailPanel.tsx | 46 ++- apps/web/src/hooks/useHandleNewThread.ts | 44 +-- .../web/src/lib/stagedDraftNavigation.test.ts | 38 ++- apps/web/src/lib/stagedDraftNavigation.ts | 15 +- apps/web/src/lib/threadBootstrap.test.ts | 42 ++- apps/web/src/lib/threadBootstrap.ts | 43 ++- apps/web/src/routes/_chat.tsx | 35 ++- 9 files changed, 478 insertions(+), 73 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 02ee29edf..980f69b4d 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5572,6 +5572,123 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("does not let a delayed provider preflight replace a later reused draft", async () => { + const reusableDraftThreadId = ThreadId.makeUnsafe("thread-provider-preflight-race"); + const unavailableProvider = { + provider: "codex" as const, + status: "error" as const, + available: false, + authStatus: "unauthenticated" as const, + message: "Codex is temporarily unavailable.", + checkedAt: NOW_ISO, + }; + const availableProvider = { + provider: "codex" as const, + status: "ready" as const, + available: true, + authStatus: "authenticated" as const, + checkedAt: NOW_ISO, + runtime: { + source: "system" as const, + managedVersion: null, + canInstall: false, + canRepair: false, + canRollback: false, + canRemove: false, + message: null, + }, + }; + let resolveProviderRefresh!: (value: { providers: [typeof availableProvider] }) => void; + const providerRefresh = new Promise<{ providers: [typeof availableProvider] }>((resolve) => { + resolveProviderRefresh = resolve; + }); + const refreshProviders = vi.fn(() => providerRefresh); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: createSnapshotForTargetUser({ + targetMessageId: "msg-user-provider-preflight-race" as MessageId, + targetText: "provider preflight race", + }), + configureFixture: (nextFixture) => { + nextFixture.serverConfig = { + ...nextFixture.serverConfig, + providers: [unavailableProvider], + keybindings: [ + { + command: "chat.newCodex", + shortcut: { + key: "c", + metaKey: false, + ctrlKey: false, + shiftKey: true, + altKey: false, + modKey: true, + }, + whenAst: { + type: "not", + node: { type: "identifier", name: "terminalFocus" }, + }, + }, + ], + }; + }, + configureNativeApi: (api) => ({ + ...api, + server: { + ...api.server, + refreshProviders, + }, + }), + }); + + try { + await waitForServerConfigToApply(); + const composerEditor = await waitForComposerEditor(); + composerEditor.focus(); + await waitForLayout(); + useComposerDraftStore.getState().setProjectDraftThreadId(PROJECT_ID, reusableDraftThreadId, { + entryPoint: "chat", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + useComposerDraftStore.getState().setPrompt(reusableDraftThreadId, "later draft prompt"); + + await dispatchThreadShortcut("c"); + await vi.waitFor(() => expect(refreshProviders).toHaveBeenCalled()); + + await page.getByTestId("new-thread-button").click(); + useComposerDraftStore.getState().addFiles(reusableDraftThreadId, [ + { + type: "file", + id: "provider-race-file", + name: "provider-race.txt", + mimeType: "text/plain", + sizeBytes: 5, + file: new File(["notes"], "provider-race.txt", { type: "text/plain" }), + }, + ]); + resolveProviderRefresh({ providers: [availableProvider] }); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + + expect(mounted.router.state.location.pathname).toBe(`/${reusableDraftThreadId}`); + expect( + useComposerDraftStore.getState().draftsByThreadId[reusableDraftThreadId], + ).toMatchObject({ + prompt: "later draft prompt", + files: [expect.objectContaining({ id: "provider-race-file" })], + }); + expect( + useComposerDraftStore.getState().getDraftThreadByProjectId(PROJECT_ID, "chat")?.threadId, + ).toBe(reusableDraftThreadId); + } finally { + resolveProviderRefresh({ providers: [availableProvider] }); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + await mounted.cleanup(); + } + }); + it("starts one fresh thread in the exact worktree from repeated context-menu actions", async () => { const contextMenuShow = vi.fn( async (_items: Parameters[0]) => "new-thread-in-workspace", @@ -5646,8 +5763,8 @@ describe("ChatView timeline estimator parity (full app)", () => { const projectValidationCallCount = () => branchLookup.mock.calls.filter(([input]) => input.cwd === "/repo/project").length; defaultNavigationOperation = runDraftNavigationOnce( - draftNavigationSlotKey(PROJECT_ID, "chat"), - newThreadNavigationRequestKey({ hasCustomSearch: false }), + draftNavigationSlotKey(), + newThreadNavigationRequestKey({}), () => defaultNavigationBlocker.promise, ); const threadRow = await waitForElement( @@ -5808,7 +5925,7 @@ describe("ChatView timeline estimator parity (full app)", () => { ]); resolveExactValidation(exactWorktreeBranchResult); - await waitForDraftNavigationIdle(draftNavigationSlotKey(PROJECT_ID, "chat")); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); await waitForURL( mounted.router, (path) => path === `/${reusableDraftThreadId}`, @@ -5833,7 +5950,168 @@ describe("ChatView timeline estimator parity (full app)", () => { expect(document.body.textContent).not.toContain("Unable to start thread"); } finally { resolveExactValidation(exactWorktreeBranchResult); - await waitForDraftNavigationIdle(draftNavigationSlotKey(PROJECT_ID, "chat")); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + await mounted.cleanup(); + } + }); + + it("keeps a later terminal intent and preserves chat attachments across entry points", async () => { + const reusableChatDraftId = ThreadId.makeUnsafe("thread-cross-entry-chat-draft"); + const reusableTerminalDraftId = ThreadId.makeUnsafe("thread-cross-entry-terminal-draft"); + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/exact-worktree", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/exact-worktree", + }, + ], + }; + let resolveExactValidation!: ( + value: Awaited>, + ) => void; + const exactValidation = new Promise>>( + (resolve) => { + resolveExactValidation = resolve; + }, + ); + const branchLookup = vi.fn( + async () => exactWorktreeBranchResult, + ); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-cross-entry-race" as MessageId, + targetText: "cross entry race", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => + Object.assign({}, thread, { + envMode: "worktree" as const, + branch: "feature/exact-worktree", + worktreePath: "/repo/worktrees/exact-worktree", + }), + ), + }, + configureFixture: (nextFixture) => { + nextFixture.serverConfig = { + ...nextFixture.serverConfig, + keybindings: [ + { + command: "chat.newTerminal", + shortcut: { + key: "t", + metaKey: false, + ctrlKey: false, + shiftKey: true, + altKey: false, + modKey: true, + }, + whenAst: { + type: "not", + node: { type: "identifier", name: "terminalFocus" }, + }, + }, + ], + }; + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, + }), + }); + + try { + await waitForServerConfigToApply(); + const composerEditor = await waitForComposerEditor(); + composerEditor.focus(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); + await waitForLayout(); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + useComposerDraftStore.getState().setProjectDraftThreadId(PROJECT_ID, reusableChatDraftId, { + entryPoint: "chat", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + useComposerDraftStore.getState().setPrompt(reusableChatDraftId, "cross-entry prompt"); + useComposerDraftStore.getState().addFiles(reusableChatDraftId, [ + { + type: "file", + id: "cross-entry-notes", + name: "cross-entry.txt", + mimeType: "text/plain", + sizeBytes: 5, + file: new File(["notes"], "cross-entry.txt", { type: "text/plain" }), + }, + ]); + useComposerDraftStore + .getState() + .setProjectDraftThreadId(PROJECT_ID, reusableTerminalDraftId, { + entryPoint: "terminal", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + branchLookup.mockClear(); + branchLookup.mockImplementation(() => exactValidation); + + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(1)); + + await dispatchTerminalThreadShortcut(); + resolveExactValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + + expect( + useComposerDraftStore.getState().projectDraftThreadIdByProjectId[`${PROJECT_ID}::terminal`], + ).toBe(reusableTerminalDraftId); + expect( + useComposerDraftStore.getState().getDraftThread(reusableTerminalDraftId), + ).toMatchObject({ + entryPoint: "terminal", + promotedTo: reusableTerminalDraftId, + }); + expect(mounted.router.state.location.pathname).toBe(`/${reusableTerminalDraftId}`); + expect( + useComposerDraftStore.getState().getDraftThreadByProjectId(PROJECT_ID, "chat")?.threadId, + ).toBe(reusableChatDraftId); + expect(useComposerDraftStore.getState().draftsByThreadId[reusableChatDraftId]).toMatchObject({ + prompt: "cross-entry prompt", + files: [expect.objectContaining({ id: "cross-entry-notes", name: "cross-entry.txt" })], + }); + } finally { + resolveExactValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); await mounted.cleanup(); } }); diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 2cfabbe1a..577508193 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -3845,7 +3845,7 @@ export default function Sidebar() { // the newer owner is responsible for the visible destination. await handleNewThread(thread.projectId, { fresh: true, - prepareFreshCreate: async () => { + prepareNavigation: async () => { const currentProjectCwd = useStore.getState().projects.find((project) => project.id === thread.projectId) ?.cwd ?? null; diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index c73108818..47ffecf4d 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -200,30 +200,44 @@ export function PullRequestDetailPanel({ if (!detail || preparingThread !== null) return; setPreparingThread(kind); setOperationError(null); + let superseded = false; try { const mode = settings.defaultThreadEnvMode; - const prepared = await prepareThreadMutation.mutateAsync({ reference: detail.url, mode }); - const workspace = (() => { - if (mode === "worktree") { - if (!prepared.worktreePath) { - throw new Error("The pull request worktree was not created."); - } - return { - kind: "existing-worktree" as const, - branch: prepared.branch, - worktreePath: prepared.worktreePath, - }; - } - return { kind: "existing-local" as const, branch: prepared.branch }; - })(); const threadId = await handleNewThread(detail.projectId, { - workspace, // This action is an explicit handoff from the PR browser. Reusing the project's // existing draft can leave the user on the PR route and insert the prompt into a // hidden composer, making the button appear inert. fresh: true, + prepareNavigation: async (ownership) => { + const prepared = await prepareThreadMutation.mutateAsync({ + reference: detail.url, + mode, + }); + if (!ownership.isCurrent()) { + superseded = true; + return false; + } + if (mode === "worktree") { + if (!prepared.worktreePath) { + throw new Error("The pull request worktree was not created."); + } + return { + workspace: { + kind: "existing-worktree" as const, + branch: prepared.branch, + worktreePath: prepared.worktreePath, + }, + }; + } + return { + workspace: { kind: "existing-local" as const, branch: prepared.branch }, + }; + }, }); - if (!threadId) throw new Error("Could not create a draft thread for this pull request."); + if (!threadId) { + if (superseded) return; + throw new Error("Could not create a draft thread for this pull request."); + } appendComposerPromptText(threadId, prompt); } catch (error) { setOperationError( diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 03ac69ed9..46bfd47b1 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -118,15 +118,30 @@ export function useHandleNewThread() { ); }; return runDraftNavigationOnce( - draftNavigationSlotKey(projectId, entryPoint), + draftNavigationSlotKey(), newThreadNavigationRequestKey({ - hasCustomSearch: navigation?.search !== undefined, + customSearch: navigation?.search, options, }), async (ownership) => { if (!ownership.isCurrent()) { return null; } + let effectiveOptions = options; + try { + const preparation = await options?.prepareNavigation?.(ownership); + if (!ownership.isCurrent() || preparation === false) { + return null; + } + if (preparation?.workspace) { + effectiveOptions = { ...options, workspace: preparation.workspace }; + } + } catch (error) { + if (!ownership.isCurrent()) { + return null; + } + throw error; + } const { getDraftThread, getDraftThreadByProjectId, @@ -138,10 +153,12 @@ export function useHandleNewThread() { setModelSelection, } = useComposerDraftStore.getState(); const applyProviderOverride = (threadId: ThreadId) => { - if (!options?.provider) { + if (!effectiveOptions?.provider) { return; } - const defaultModelSelection = getRecommendedDefaultModelSelection(options.provider); + const defaultModelSelection = getRecommendedDefaultModelSelection( + effectiveOptions.provider, + ); if (defaultModelSelection) { setModelSelection(threadId, defaultModelSelection); } @@ -152,7 +169,7 @@ export function useHandleNewThread() { : null; const currentFocusedThreadId = focusedThreadId === routeThreadId ? currentRouteThreadId : focusedThreadId; - const shouldForceFreshThread = options?.fresh === true; + const shouldForceFreshThread = effectiveOptions?.fresh === true; const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); const latestActiveDraftThreadCandidate: DraftThreadState | null = currentFocusedThreadId ? getDraftThread(currentFocusedThreadId) @@ -198,7 +215,7 @@ export function useHandleNewThread() { activeDraftThread: activeDraftThreadSnapshot, activeThread: activeThreadSnapshot, defaultEnvMode: settings.defaultThreadEnvMode, - defaultProvider: options?.provider ?? settings.defaultProvider, + defaultProvider: effectiveOptions?.provider ?? settings.defaultProvider, draftComposerState: useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, draftThread, @@ -239,7 +256,7 @@ export function useHandleNewThread() { defaultEnvMode: settings.defaultThreadEnvMode, draftThread: bootstrapPlan.draftThread, entryPoint, - options, + options: effectiveOptions, reuseKind: bootstrapPlan.kind, }); if (draftContextPatch) { @@ -261,17 +278,6 @@ export function useHandleNewThread() { return bootstrapPlan.threadId; } - try { - await options?.prepareFreshCreate?.(); - } catch (error) { - if (!ownership.isCurrent()) { - return null; - } - throw error; - } - if (!ownership.isCurrent()) { - return null; - } const threadId = newThreadId(); if (wantsTemporaryThread) { markTemporaryThread(threadId); @@ -281,7 +287,7 @@ export function useHandleNewThread() { createdAt, defaultEnvMode: settings.defaultThreadEnvMode, entryPoint, - options, + options: effectiveOptions, }); const committed = await stageDraftNavigation({ isCurrent: ownership.isCurrent, diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index f03eb52ab..fe54215c5 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -133,7 +133,7 @@ describe("stagedDraftNavigation", () => { }), ); const secondRun = vi.fn(async () => "second"); - const slotKey = draftNavigationSlotKey("project-studio", "chat"); + const slotKey = draftNavigationSlotKey(); const first = runDraftNavigationOnce(slotKey, "project-default", firstRun); const duplicateFirst = runDraftNavigationOnce(slotKey, "project-default", secondRun); @@ -163,7 +163,7 @@ describe("stagedDraftNavigation", () => { }), ); const defaultRun = vi.fn(async () => "default"); - const slotKey = draftNavigationSlotKey("project-reverse", "chat"); + const slotKey = draftNavigationSlotKey(); const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); const projectDefault = runDraftNavigationOnce(slotKey, "project-default", defaultRun); @@ -181,7 +181,7 @@ describe("stagedDraftNavigation", () => { let releaseFirst!: () => void; const firstOwnership: Array<{ readonly isCurrent: () => boolean }> = []; let secondWasCurrent = false; - const slotKey = draftNavigationSlotKey("project-supersession", "chat"); + const slotKey = draftNavigationSlotKey(); const first = runDraftNavigationOnce(slotKey, "exact-worktree", async (ownership) => { firstOwnership.push(ownership); @@ -205,6 +205,36 @@ describe("stagedDraftNavigation", () => { expect(secondWasCurrent).toBe(true); }); + it("supersedes delayed work across projects and chat or terminal entry points", async () => { + let releaseProjectChat!: () => void; + let projectChatWasCurrentAfterRelease = true; + const navigationSurface = draftNavigationSlotKey(); + + const projectChat = runDraftNavigationOnce( + navigationSurface, + "project-a-chat-exact", + async (ownership) => { + await new Promise((resolve) => { + releaseProjectChat = resolve; + }); + projectChatWasCurrentAfterRelease = ownership.isCurrent(); + return ownership.isCurrent() ? "stale-navigation" : "superseded"; + }, + ); + await Promise.resolve(); + + const otherProjectTerminal = runDraftNavigationOnce( + navigationSurface, + "project-b-terminal-default", + async (ownership) => (ownership.isCurrent() ? "latest-navigation" : "superseded"), + ); + releaseProjectChat(); + + await expect(projectChat).resolves.toBe("superseded"); + await expect(otherProjectTerminal).resolves.toBe("latest-navigation"); + expect(projectChatWasCurrentAfterRelease).toBe(false); + }); + it("preserves default-exact-default ordering instead of rejoining the first request", async () => { let finishFirstDefault!: (value: string) => void; const calls: string[] = []; @@ -223,7 +253,7 @@ describe("stagedDraftNavigation", () => { calls.push("default:last"); return "default:last"; }); - const slotKey = draftNavigationSlotKey("project-three-actions", "chat"); + const slotKey = draftNavigationSlotKey(); const firstDefault = runDraftNavigationOnce(slotKey, "project-default", firstDefaultRun); const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index b6e13b7b3..9ce4e982f 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -1,5 +1,5 @@ // FILE: stagedDraftNavigation.ts -// Purpose: Serializes draft-route creation per project slot and finalizes staged drafts only +// Purpose: Serializes draft-route creation across the shared navigation surface and finalizes staged drafts only // after their destination route actually commits. // Layer: Web navigation orchestration @@ -15,18 +15,23 @@ export interface DraftNavigationOwnership { } const draftNavigationStateBySlot = new Map(); +const DRAFT_NAVIGATION_SURFACE_KEY = "new-thread-navigation"; -export function draftNavigationSlotKey(projectId: string, entryPoint: string): string { - return `${projectId}\u0000${entryPoint}`; +/** + * Every new-thread action ultimately controls the same visible route. Keep one ownership domain + * across projects and entry points so a delayed earlier action cannot navigate after a newer one. + */ +export function draftNavigationSlotKey(): string { + return DRAFT_NAVIGATION_SURFACE_KEY; } -/** Resolves after the operations currently queued for a project slot have drained. */ +/** Resolves after the operations currently queued for the shared navigation surface have drained. */ export function waitForDraftNavigationIdle(slotKey: string): Promise { return draftNavigationStateBySlot.get(slotKey)?.tail ?? Promise.resolve(); } /** - * Coalesces adjacent identical requests while serializing distinct requests for one project slot. + * Coalesces adjacent identical requests while serializing distinct requests for the shared route. * A distinct later request becomes the owner immediately, allowing awaited work to stop before a * stale navigation or draft-mapping commit can overwrite the user's latest intent. */ diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index cf95d82bf..0079a2442 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -15,6 +15,16 @@ import { const PROJECT_ID = ProjectId.makeUnsafe("project-bootstrap"); const THREAD_ID = ThreadId.makeUnsafe("thread-bootstrap"); +const FIRST_NAVIGATION_SEARCH = (previous: Record) => ({ + ...previous, + editor: "first", +}); +const SECOND_NAVIGATION_SEARCH = (previous: Record) => ({ + ...previous, + editor: "second", +}); +const FIRST_NAVIGATION_PREPARATION = async () => undefined; +const SECOND_NAVIGATION_PREPARATION = async () => undefined; function modelSelection( provider: "codex" | "claudeAgent", @@ -72,16 +82,14 @@ function makeComposerDraftState( describe("threadBootstrap", () => { it("uses distinct navigation request keys for default and exact workspace intents", () => { - const defaultKey = newThreadNavigationRequestKey({ hasCustomSearch: false }); + const defaultKey = newThreadNavigationRequestKey({}); expect( newThreadNavigationRequestKey({ - hasCustomSearch: false, options: { workspace: { kind: "project-default" } }, }), ).toBe(defaultKey); expect( newThreadNavigationRequestKey({ - hasCustomSearch: false, options: { workspace: { kind: "existing-worktree", @@ -93,12 +101,38 @@ describe("threadBootstrap", () => { ).not.toBe(defaultKey); expect( newThreadNavigationRequestKey({ - hasCustomSearch: false, options: { fresh: true }, }), ).not.toBe(defaultKey); }); + it("does not coalesce distinct custom-search or preparation callbacks", () => { + expect(newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH })).toBe( + newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH }), + ); + expect(newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH })).not.toBe( + newThreadNavigationRequestKey({ customSearch: SECOND_NAVIGATION_SEARCH }), + ); + expect( + newThreadNavigationRequestKey({ + options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, + }), + ).toBe( + newThreadNavigationRequestKey({ + options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, + }), + ); + expect( + newThreadNavigationRequestKey({ + options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, + }), + ).not.toBe( + newThreadNavigationRequestKey({ + options: { prepareNavigation: SECOND_NAVIGATION_PREPARATION }, + }), + ); + }); + it("resolves project defaults and exact existing workspaces without partial states", () => { expect(resolveNewThreadWorkspace({ kind: "project-default" }, "worktree")).toEqual({ branch: null, diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index 0a662c928..a5d8fb5a2 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -29,10 +29,25 @@ export interface NewThreadOptions { temporary?: boolean; provider?: ProviderKind; fresh?: boolean; - /** Runs after this fresh request owns its project navigation slot and before draft staging. */ - prepareFreshCreate?: () => Promise; + /** + * Runs after this request owns the shared navigation surface and before any draft lookup or + * mutation. Returning false cancels the request; a workspace result supplies an intent that is + * only known after asynchronous preparation (for example, PR worktree preparation). + */ + prepareNavigation?: ( + ownership: NewThreadNavigationOwnership, + ) => Promise; } +export interface NewThreadNavigationOwnership { + readonly isCurrent: () => boolean; +} + +export type NewThreadPreparationResult = + | void + | false + | { readonly workspace: NewThreadWorkspaceIntent }; + export type NewThreadWorkspaceIntent = | { readonly kind: "project-default" } | { readonly kind: "local-container" } @@ -50,8 +65,27 @@ export interface ResolvedNewThreadWorkspace { workspaceOrigin: DraftThreadWorkspaceOrigin; } +const navigationCallbackIds = new WeakMap<(...args: never[]) => unknown, number>(); +let nextNavigationCallbackId = 1; + +function navigationCallbackKey(callback: ((...args: never[]) => unknown) | undefined): string { + if (!callback) { + return "none"; + } + const existing = navigationCallbackIds.get(callback); + if (existing !== undefined) { + return String(existing); + } + const created = nextNavigationCallbackId; + nextNavigationCallbackId += 1; + navigationCallbackIds.set(callback, created); + return String(created); +} + export function newThreadNavigationRequestKey(input: { - readonly hasCustomSearch: boolean; + readonly customSearch?: + | ((previous: Record) => Record) + | undefined; readonly options?: NewThreadOptions | undefined; }): string { const workspace = input.options?.workspace ?? { kind: "project-default" as const }; @@ -64,7 +98,8 @@ export function newThreadNavigationRequestKey(input: { input.options?.provider ?? "", input.options?.temporary === true ? "temporary" : "durable", input.options?.fresh === true ? "fresh" : "reuse-eligible", - input.hasCustomSearch ? "custom-search" : "default-search", + `search:${navigationCallbackKey(input.customSearch)}`, + `prepare:${navigationCallbackKey(input.options?.prepareNavigation)}`, ].join("\u0000"); } diff --git a/apps/web/src/routes/_chat.tsx b/apps/web/src/routes/_chat.tsx index 285603437..136790fca 100644 --- a/apps/web/src/routes/_chat.tsx +++ b/apps/web/src/routes/_chat.tsx @@ -388,23 +388,26 @@ function ChatRouteGlobalShortcuts() { if (!target) return; event.preventDefault(); event.stopPropagation(); - void (async () => { - const providerAvailability = await resolveProviderSendAvailabilityWithRefresh({ - provider, - statuses: providerStatuses, - refreshStatuses: () => refreshProviderStatuses({ silent: true }), - }); - if (!providerAvailability.usable) { - transientAlertManager.add({ - type: "error", - title: providerAvailability.unavailableReason, + void handleNewThread(target.projectId, { + provider, + prepareNavigation: async (ownership) => { + const providerAvailability = await resolveProviderSendAvailabilityWithRefresh({ + provider, + statuses: providerStatuses, + refreshStatuses: () => refreshProviderStatuses({ silent: true }), }); - return; - } - await handleNewThread(target.projectId, { - provider, - }); - })(); + if (!ownership.isCurrent()) { + return false; + } + if (!providerAvailability.usable) { + transientAlertManager.add({ + type: "error", + title: providerAvailability.unavailableReason, + }); + return false; + } + }, + }); return; } From f65fadc8de0e9b96a39fc5845c39354fe6674288 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:02:15 +0300 Subject: [PATCH 07/14] fix(web): preserve latest navigation across preparation --- apps/web/src/components/ChatView.browser.tsx | 9 +- .../pullRequest/PullRequestDetailPanel.tsx | 5 +- apps/web/src/hooks/useHandleNewChat.ts | 1 + apps/web/src/hooks/useHandleNewStudioChat.ts | 1 + apps/web/src/hooks/useHandleNewThread.ts | 397 +++++++++--------- .../web/src/lib/stagedDraftNavigation.test.ts | 16 +- apps/web/src/lib/stagedDraftNavigation.ts | 18 +- apps/web/src/lib/startContainerChat.test.ts | 31 ++ apps/web/src/lib/startContainerChat.ts | 45 +- apps/web/src/lib/threadBootstrap.test.ts | 47 ++- apps/web/src/lib/threadBootstrap.ts | 4 + 11 files changed, 338 insertions(+), 236 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 980f69b4d..51e9c6016 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5764,7 +5764,7 @@ describe("ChatView timeline estimator parity (full app)", () => { branchLookup.mock.calls.filter(([input]) => input.cwd === "/repo/project").length; defaultNavigationOperation = runDraftNavigationOnce( draftNavigationSlotKey(), - newThreadNavigationRequestKey({}), + newThreadNavigationRequestKey({ projectId: PROJECT_ID, entryPoint: "chat" }), () => defaultNavigationBlocker.promise, ); const threadRow = await waitForElement( @@ -5786,8 +5786,7 @@ describe("ChatView timeline estimator parity (full app)", () => { openContextMenu(); await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(1)); - await Promise.resolve(); - expect(projectValidationCallCount()).toBe(0); + await vi.waitFor(() => expect(projectValidationCallCount()).toBe(1)); openContextMenu(); await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(2)); expect(contextMenuShow.mock.calls[0]?.[0]?.[0]).toMatchObject({ @@ -5795,9 +5794,7 @@ describe("ChatView timeline estimator parity (full app)", () => { label: "New thread in worktree (feature/exact-worktree)", }); - defaultNavigationBlocker.resolve(); - await defaultNavigationOperation; - await vi.waitFor(() => expect(projectValidationCallCount()).toBe(1)); + expect(projectValidationCallCount()).toBe(1); branchLookupDeferred.resolve(exactWorktreeBranchResult); const newThreadPath = await waitForURL( mounted.router, diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 47ffecf4d..a5a400316 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -200,7 +200,6 @@ export function PullRequestDetailPanel({ if (!detail || preparingThread !== null) return; setPreparingThread(kind); setOperationError(null); - let superseded = false; try { const mode = settings.defaultThreadEnvMode; const threadId = await handleNewThread(detail.projectId, { @@ -214,7 +213,6 @@ export function PullRequestDetailPanel({ mode, }); if (!ownership.isCurrent()) { - superseded = true; return false; } if (mode === "worktree") { @@ -235,8 +233,7 @@ export function PullRequestDetailPanel({ }, }); if (!threadId) { - if (superseded) return; - throw new Error("Could not create a draft thread for this pull request."); + return; } appendComposerPromptText(threadId, prompt); } catch (error) { diff --git a/apps/web/src/hooks/useHandleNewChat.ts b/apps/web/src/hooks/useHandleNewChat.ts index 002c804ba..11e95ee3a 100644 --- a/apps/web/src/hooks/useHandleNewChat.ts +++ b/apps/web/src/hooks/useHandleNewChat.ts @@ -22,6 +22,7 @@ export function useHandleNewChat() { return startContainerChat({ ensureProjectId: () => ensureHomeChatProject({ homeDir, chatWorkspaceRoot }), handleNewThread, + navigationTargetKey: "home-chat", fresh: options?.fresh, errorLabel: "Unable to prepare a new chat.", }); diff --git a/apps/web/src/hooks/useHandleNewStudioChat.ts b/apps/web/src/hooks/useHandleNewStudioChat.ts index d3412dc46..b05590d0e 100644 --- a/apps/web/src/hooks/useHandleNewStudioChat.ts +++ b/apps/web/src/hooks/useHandleNewStudioChat.ts @@ -22,6 +22,7 @@ export function useHandleNewStudioChat() { ensureProjectId: () => ensureStudioProject({ homeDir, chatWorkspaceRoot, studioWorkspaceRoot }), handleNewThread, + navigationTargetKey: "studio-chat", fresh: options?.fresh, errorLabel: "Unable to prepare a new Studio chat.", }), diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 46bfd47b1..2949eaeff 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -17,6 +17,7 @@ import { newThreadNavigationRequestKey, resolveTerminalThreadCreationState, resolveThreadBootstrapPlan, + type NewThreadNavigationOwnership, type NewThreadOptions, } from "../lib/threadBootstrap"; import { promoteThreadCreate } from "../lib/threadCreatePromotion"; @@ -59,6 +60,7 @@ export function useHandleNewThread() { projectId: ProjectId, options?: NewThreadOptions, navigation?: NewThreadNavigationOptions, + existingOwnership?: NewThreadNavigationOwnership, ): Promise => { const entryPoint = options?.entryPoint ?? "chat"; const wantsTemporaryThread = options?.temporary === true; @@ -117,226 +119,231 @@ export function useHandleNewThread() { api, ); }; - return runDraftNavigationOnce( - draftNavigationSlotKey(), - newThreadNavigationRequestKey({ - customSearch: navigation?.search, - options, - }), - async (ownership) => { + const runOwnedNavigation = async ( + ownership: NewThreadNavigationOwnership, + ): Promise => { + if (!ownership.isCurrent()) { + return null; + } + let effectiveOptions = options; + try { + const preparation = await options?.prepareNavigation?.(ownership); + if (!ownership.isCurrent() || preparation === false) { + return null; + } + if (preparation?.workspace) { + effectiveOptions = { ...options, workspace: preparation.workspace }; + } + } catch (error) { if (!ownership.isCurrent()) { return null; } - let effectiveOptions = options; - try { - const preparation = await options?.prepareNavigation?.(ownership); - if (!ownership.isCurrent() || preparation === false) { - return null; - } - if (preparation?.workspace) { - effectiveOptions = { ...options, workspace: preparation.workspace }; - } - } catch (error) { - if (!ownership.isCurrent()) { - return null; - } - throw error; + throw error; + } + const { + getDraftThread, + getDraftThreadByProjectId, + applyStickyState, + clearDraftThread, + registerDraftThread, + setDraftThreadContext, + setProjectDraftThreadId, + setModelSelection, + } = useComposerDraftStore.getState(); + const applyProviderOverride = (threadId: ThreadId) => { + if (!effectiveOptions?.provider) { + return; } - const { - getDraftThread, - getDraftThreadByProjectId, - applyStickyState, - clearDraftThread, - registerDraftThread, - setDraftThreadContext, - setProjectDraftThreadId, - setModelSelection, - } = useComposerDraftStore.getState(); - const applyProviderOverride = (threadId: ThreadId) => { - if (!effectiveOptions?.provider) { - return; - } - const defaultModelSelection = getRecommendedDefaultModelSelection( - effectiveOptions.provider, - ); - if (defaultModelSelection) { - setModelSelection(threadId, defaultModelSelection); - } - }; - const currentPathMatch = /^\/([^/]+)$/.exec(router.state.location.pathname); - const currentRouteThreadId = currentPathMatch?.[1] - ? ThreadId.makeUnsafe(decodeURIComponent(currentPathMatch[1])) + const defaultModelSelection = getRecommendedDefaultModelSelection( + effectiveOptions.provider, + ); + if (defaultModelSelection) { + setModelSelection(threadId, defaultModelSelection); + } + }; + const currentPathMatch = /^\/([^/]+)$/.exec(router.state.location.pathname); + const currentRouteThreadId = currentPathMatch?.[1] + ? ThreadId.makeUnsafe(decodeURIComponent(currentPathMatch[1])) + : null; + const currentFocusedThreadId = + focusedThreadId === routeThreadId ? currentRouteThreadId : focusedThreadId; + const shouldForceFreshThread = effectiveOptions?.fresh === true; + const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); + const latestActiveDraftThreadCandidate: DraftThreadState | null = currentFocusedThreadId + ? getDraftThread(currentFocusedThreadId) + : null; + const storedDraftThread = + !shouldForceFreshThread && + !wantsTemporaryThread && + storedDraftThreadCandidate?.isTemporary !== true + ? storedDraftThreadCandidate : null; - const currentFocusedThreadId = - focusedThreadId === routeThreadId ? currentRouteThreadId : focusedThreadId; - const shouldForceFreshThread = effectiveOptions?.fresh === true; - const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); - const latestActiveDraftThreadCandidate: DraftThreadState | null = currentFocusedThreadId - ? getDraftThread(currentFocusedThreadId) + const latestActiveDraftThread = + !shouldForceFreshThread && + !wantsTemporaryThread && + latestActiveDraftThreadCandidate?.isTemporary !== true + ? latestActiveDraftThreadCandidate : null; - const storedDraftThread = - !shouldForceFreshThread && - !wantsTemporaryThread && - storedDraftThreadCandidate?.isTemporary !== true - ? storedDraftThreadCandidate - : null; - const latestActiveDraftThread = - !shouldForceFreshThread && - !wantsTemporaryThread && - latestActiveDraftThreadCandidate?.isTemporary !== true - ? latestActiveDraftThreadCandidate - : null; - const bootstrapPlan = resolveThreadBootstrapPlan({ - storedDraftThread, - latestActiveDraftThread, - entryPoint, + const bootstrapPlan = resolveThreadBootstrapPlan({ + storedDraftThread, + latestActiveDraftThread, + entryPoint, + projectId, + routeThreadId: currentFocusedThreadId, + }); + const currentAppState = useStore.getState(); + const activeThreadSnapshot = createActiveThreadSnapshot( + currentFocusedThreadId + ? currentAppState.threads.find((thread) => thread.id === currentFocusedThreadId) + : null, + projectId, + ); + const activeDraftThreadSnapshot = createActiveDraftThreadSnapshot( + latestActiveDraftThreadCandidate, + projectId, + ); + const projectDefaultModelSelection = + currentAppState.projects.find((project) => project.id === projectId) + ?.defaultModelSelection ?? null; + const resolveCreationState = ( + targetThreadId: ThreadId, + draftThread: DraftThreadState | null, + ) => + resolveTerminalThreadCreationState({ + activeDraftThread: activeDraftThreadSnapshot, + activeThread: activeThreadSnapshot, + defaultEnvMode: settings.defaultThreadEnvMode, + defaultProvider: effectiveOptions?.provider ?? settings.defaultProvider, + draftComposerState: + useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, + draftThread, + projectDefaultModelSelection, projectId, - routeThreadId: currentFocusedThreadId, }); - const currentAppState = useStore.getState(); - const activeThreadSnapshot = createActiveThreadSnapshot( - currentFocusedThreadId - ? currentAppState.threads.find((thread) => thread.id === currentFocusedThreadId) - : null, - projectId, - ); - const activeDraftThreadSnapshot = createActiveDraftThreadSnapshot( - latestActiveDraftThreadCandidate, - projectId, - ); - const projectDefaultModelSelection = - currentAppState.projects.find((project) => project.id === projectId) - ?.defaultModelSelection ?? null; - const resolveCreationState = ( - targetThreadId: ThreadId, - draftThread: DraftThreadState | null, - ) => - resolveTerminalThreadCreationState({ - activeDraftThread: activeDraftThreadSnapshot, - activeThread: activeThreadSnapshot, - defaultEnvMode: settings.defaultThreadEnvMode, - defaultProvider: effectiveOptions?.provider ?? settings.defaultProvider, - draftComposerState: - useComposerDraftStore.getState().draftsByThreadId[targetThreadId] ?? null, - draftThread, - projectDefaultModelSelection, - projectId, - }); - if (bootstrapPlan.kind !== "fresh") { - const preservedComposerDraft = - useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; - if ( - bootstrapPlan.kind === "stored" && - currentFocusedThreadId !== bootstrapPlan.threadId - ) { - try { - await navigate({ - to: "/$threadId", - params: { threadId: bootstrapPlan.threadId }, - ...(navigation?.search ? { search: navigation.search } : {}), - }); - } catch (error) { - if (!ownership.isCurrent()) { - return null; - } - throw error; - } - if ( - !ownership.isCurrent() || - router.state.location.pathname !== `/${bootstrapPlan.threadId}` - ) { + if (bootstrapPlan.kind !== "fresh") { + const preservedComposerDraft = + useComposerDraftStore.getState().draftsByThreadId[bootstrapPlan.threadId] ?? null; + if ( + bootstrapPlan.kind === "stored" && + currentFocusedThreadId !== bootstrapPlan.threadId + ) { + try { + await navigate({ + to: "/$threadId", + params: { threadId: bootstrapPlan.threadId }, + ...(navigation?.search ? { search: navigation.search } : {}), + }); + } catch (error) { + if (!ownership.isCurrent()) { return null; } + throw error; } - if (!ownership.isCurrent()) { + if ( + !ownership.isCurrent() || + router.state.location.pathname !== `/${bootstrapPlan.threadId}` + ) { return null; } - const draftContextPatch = buildDraftThreadWorkspacePatch({ - defaultEnvMode: settings.defaultThreadEnvMode, - draftThread: bootstrapPlan.draftThread, - entryPoint, - options: effectiveOptions, - reuseKind: bootstrapPlan.kind, - }); - if (draftContextPatch) { - setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); - } - applyProviderOverride(bootstrapPlan.threadId); - setProjectDraftThreadId(projectId, bootstrapPlan.threadId, { entryPoint }); - restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); - activateThreadEntryPoint(bootstrapPlan.threadId); - if (entryPoint === "terminal" && ownership.isCurrent()) { - await createTerminalThread( - bootstrapPlan.threadId, - resolveCreationState( - bootstrapPlan.threadId, - getDraftThread(bootstrapPlan.threadId), - ), - ); - } - return bootstrapPlan.threadId; } - - const threadId = newThreadId(); - if (wantsTemporaryThread) { - markTemporaryThread(threadId); + if (!ownership.isCurrent()) { + return null; } - const createdAt = new Date().toISOString(); - const draftSeed = createFreshDraftThreadSeed({ - createdAt, + const draftContextPatch = buildDraftThreadWorkspacePatch({ defaultEnvMode: settings.defaultThreadEnvMode, + draftThread: bootstrapPlan.draftThread, entryPoint, options: effectiveOptions, + reuseKind: bootstrapPlan.kind, }); - const committed = await stageDraftNavigation({ - isCurrent: ownership.isCurrent, - // Keep the previous routed draft alive while the destination loads. Replacing the - // project's primary slot earlier makes the route guard redirect the old URL to Home. - stage: () => { - registerDraftThread(threadId, { projectId, ...draftSeed }); - markNewThreadLanding(threadId); - activateThreadEntryPoint(threadId); - applyStickyState(threadId); - applyProviderOverride(threadId); - }, - // Mark the draft-landing navigation as a transition so the new route - // subtree renders interruptibly and the browser can paint the composer - // skeleton immediately instead of freezing on the synchronous commit. - navigate: () => - new Promise((resolve, reject) => { - startTransition(() => { - navigate({ - to: "/$threadId", - params: { threadId }, - ...(navigation?.search ? { search: navigation.search } : {}), - }).then(resolve, reject); - }); - }), - // TanStack resolves an older navigate() promise when a newer navigation supersedes it. - // Verify the committed route before deleting the previous project draft. - isDestinationActive: () => router.state.location.pathname === `/${threadId}`, - finalize: () => setProjectDraftThreadId(projectId, threadId, draftSeed), - rollback: () => { - clearNewThreadLanding(threadId); - clearDraftThread(threadId); - clearTerminalState(threadId); - if (wantsTemporaryThread) { - clearTemporaryThread(threadId); - } - }, - }); - if (!committed) { - return null; + if (draftContextPatch) { + setDraftThreadContext(bootstrapPlan.threadId, draftContextPatch); } + applyProviderOverride(bootstrapPlan.threadId); + setProjectDraftThreadId(projectId, bootstrapPlan.threadId, { entryPoint }); + restoreComposerDraft(bootstrapPlan.threadId, preservedComposerDraft); + activateThreadEntryPoint(bootstrapPlan.threadId); if (entryPoint === "terminal" && ownership.isCurrent()) { await createTerminalThread( - threadId, - resolveCreationState(threadId, getDraftThread(threadId)), + bootstrapPlan.threadId, + resolveCreationState(bootstrapPlan.threadId, getDraftThread(bootstrapPlan.threadId)), ); } - return threadId; - }, + return bootstrapPlan.threadId; + } + + const threadId = newThreadId(); + if (wantsTemporaryThread) { + markTemporaryThread(threadId); + } + const createdAt = new Date().toISOString(); + const draftSeed = createFreshDraftThreadSeed({ + createdAt, + defaultEnvMode: settings.defaultThreadEnvMode, + entryPoint, + options: effectiveOptions, + }); + const committed = await stageDraftNavigation({ + isCurrent: ownership.isCurrent, + // Keep the previous routed draft alive while the destination loads. Replacing the + // project's primary slot earlier makes the route guard redirect the old URL to Home. + stage: () => { + registerDraftThread(threadId, { projectId, ...draftSeed }); + markNewThreadLanding(threadId); + activateThreadEntryPoint(threadId); + applyStickyState(threadId); + applyProviderOverride(threadId); + }, + // Mark the draft-landing navigation as a transition so the new route + // subtree renders interruptibly and the browser can paint the composer + // skeleton immediately instead of freezing on the synchronous commit. + navigate: () => + new Promise((resolve, reject) => { + startTransition(() => { + navigate({ + to: "/$threadId", + params: { threadId }, + ...(navigation?.search ? { search: navigation.search } : {}), + }).then(resolve, reject); + }); + }), + // TanStack resolves an older navigate() promise when a newer navigation supersedes it. + // Verify the committed route before deleting the previous project draft. + isDestinationActive: () => router.state.location.pathname === `/${threadId}`, + finalize: () => setProjectDraftThreadId(projectId, threadId, draftSeed), + rollback: () => { + clearNewThreadLanding(threadId); + clearDraftThread(threadId); + clearTerminalState(threadId); + if (wantsTemporaryThread) { + clearTemporaryThread(threadId); + } + }, + }); + if (!committed) { + return null; + } + if (entryPoint === "terminal" && ownership.isCurrent()) { + await createTerminalThread( + threadId, + resolveCreationState(threadId, getDraftThread(threadId)), + ); + } + return threadId; + }; + if (existingOwnership) { + return runOwnedNavigation(existingOwnership); + } + return runDraftNavigationOnce( + draftNavigationSlotKey(), + newThreadNavigationRequestKey({ + projectId, + entryPoint, + customSearch: navigation?.search, + options, + }), + runOwnedNavigation, ); }, [ diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index fe54215c5..afa2a7762 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -124,7 +124,7 @@ describe("stagedDraftNavigation", () => { expect(rollback).toHaveBeenCalledOnce(); }); - it("coalesces identical requests and serializes different requests for the same slot", async () => { + it("coalesces identical requests without blocking a distinct later request", async () => { let finishFirst!: (value: string) => void; const firstRun = vi.fn( () => @@ -139,12 +139,12 @@ describe("stagedDraftNavigation", () => { const duplicateFirst = runDraftNavigationOnce(slotKey, "project-default", secondRun); const second = runDraftNavigationOnce(slotKey, "exact-worktree", secondRun); await Promise.resolve(); - expect(secondRun).not.toHaveBeenCalled(); + expect(secondRun).toHaveBeenCalledOnce(); + await expect(second).resolves.toBe("second"); finishFirst("first"); await expect(first).resolves.toBe("first"); await expect(duplicateFirst).resolves.toBe("first"); - await expect(second).resolves.toBe("second"); expect(firstRun).toHaveBeenCalledOnce(); expect(secondRun).toHaveBeenCalledOnce(); @@ -154,7 +154,7 @@ describe("stagedDraftNavigation", () => { expect(secondRun).toHaveBeenCalledTimes(2); }); - it("serializes a later project-default request behind an exact-workspace request", async () => { + it("lets a later project-default request progress during exact-workspace preparation", async () => { let finishExact!: (value: string) => void; const exactRun = vi.fn( () => @@ -168,11 +168,11 @@ describe("stagedDraftNavigation", () => { const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); const projectDefault = runDraftNavigationOnce(slotKey, "project-default", defaultRun); await Promise.resolve(); - expect(defaultRun).not.toHaveBeenCalled(); + expect(defaultRun).toHaveBeenCalledOnce(); + await expect(projectDefault).resolves.toBe("default"); finishExact("exact"); await expect(exact).resolves.toBe("exact"); - await expect(projectDefault).resolves.toBe("default"); expect(exactRun).toHaveBeenCalledOnce(); expect(defaultRun).toHaveBeenCalledOnce(); }); @@ -228,10 +228,10 @@ describe("stagedDraftNavigation", () => { "project-b-terminal-default", async (ownership) => (ownership.isCurrent() ? "latest-navigation" : "superseded"), ); + await expect(otherProjectTerminal).resolves.toBe("latest-navigation"); releaseProjectChat(); await expect(projectChat).resolves.toBe("superseded"); - await expect(otherProjectTerminal).resolves.toBe("latest-navigation"); expect(projectChatWasCurrentAfterRelease).toBe(false); }); @@ -259,7 +259,7 @@ describe("stagedDraftNavigation", () => { const exact = runDraftNavigationOnce(slotKey, "exact-worktree", exactRun); const lastDefault = runDraftNavigationOnce(slotKey, "project-default", lastDefaultRun); await Promise.resolve(); - expect(calls).toEqual(["default:first"]); + expect(calls).toEqual(["default:first", "exact", "default:last"]); finishFirstDefault("default:first"); await expect(firstDefault).resolves.toBe("default:first"); diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index 9ce4e982f..53d2eadac 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -1,6 +1,6 @@ // FILE: stagedDraftNavigation.ts -// Purpose: Serializes draft-route creation across the shared navigation surface and finalizes staged drafts only -// after their destination route actually commits. +// Purpose: Coordinates draft-route creation across the shared navigation surface and finalizes +// staged drafts only after their destination route actually commits. // Layer: Web navigation orchestration interface DraftNavigationSlotState { @@ -25,15 +25,15 @@ export function draftNavigationSlotKey(): string { return DRAFT_NAVIGATION_SURFACE_KEY; } -/** Resolves after the operations currently queued for the shared navigation surface have drained. */ +/** Resolves after the operations currently running on the shared navigation surface have drained. */ export function waitForDraftNavigationIdle(slotKey: string): Promise { return draftNavigationStateBySlot.get(slotKey)?.tail ?? Promise.resolve(); } /** - * Coalesces adjacent identical requests while serializing distinct requests for the shared route. - * A distinct later request becomes the owner immediately, allowing awaited work to stop before a - * stale navigation or draft-mapping commit can overwrite the user's latest intent. + * Coalesces adjacent identical requests while starting distinct requests independently. A distinct + * later request becomes the owner immediately, allowing it to make progress without waiting for a + * stale preparation and allowing awaited older work to stop before a route or draft-mapping commit. */ export function runDraftNavigationOnce( slotKey: string, @@ -60,8 +60,7 @@ export function runDraftNavigationOnce( isCurrent: () => state.latestOwner === owner, }; state.latestOwner = owner; - const execute = () => run(ownership); - const execution = state.tail.then(execute, execute); + const execution = Promise.resolve().then(() => run(ownership)); let operation!: Promise; const clearLatestRequest = () => { if (state.latestOperation === operation) { @@ -83,7 +82,8 @@ export function runDraftNavigationOnce( state.latestOperation = operation; state.latestRequestKey = requestKey; - const tail = operation.then( + const previousTail = state.tail; + const tail = Promise.all([previousTail, operation]).then( () => undefined, () => undefined, ); diff --git a/apps/web/src/lib/startContainerChat.test.ts b/apps/web/src/lib/startContainerChat.test.ts index 3bc516c7d..7d74bc9d1 100644 --- a/apps/web/src/lib/startContainerChat.test.ts +++ b/apps/web/src/lib/startContainerChat.test.ts @@ -6,6 +6,7 @@ import { startFreshChatForActiveSurface, type StartContainerChatResult, } from "./startContainerChat"; +import { draftNavigationSlotKey, runDraftNavigationOnce } from "./stagedDraftNavigation"; const paths = { homeDir: "/Users/tester", @@ -86,9 +87,39 @@ describe("startContainerChat", () => { startContainerChat({ ensureProjectId: async () => projectId, handleNewThread: async () => threadId, + navigationTargetKey: "home-chat", fresh: true, errorLabel: "failed", }), ).resolves.toEqual({ ok: true, threadId }); }); + + it("does not let delayed container resolution retake ownership from a later intent", async () => { + const projectId = ProjectId.makeUnsafe("project-delayed-container"); + let resolveProject!: (value: ProjectId) => void; + const ensuredProject = new Promise((resolve) => { + resolveProject = resolve; + }); + const handleNewThread = vi.fn(async () => ThreadId.makeUnsafe("thread-stale-container")); + + const delayedContainer = startContainerChat({ + ensureProjectId: () => ensuredProject, + handleNewThread, + navigationTargetKey: "home-chat-delayed", + fresh: true, + errorLabel: "failed", + }); + await Promise.resolve(); + + const laterIntent = runDraftNavigationOnce( + draftNavigationSlotKey(), + "project-b-terminal", + async (ownership) => (ownership.isCurrent() ? "later" : "superseded"), + ); + await expect(laterIntent).resolves.toBe("later"); + resolveProject(projectId); + + await expect(delayedContainer).resolves.toEqual({ ok: true, threadId: null }); + expect(handleNewThread).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/lib/startContainerChat.ts b/apps/web/src/lib/startContainerChat.ts index 173baef11..d5ed6e802 100644 --- a/apps/web/src/lib/startContainerChat.ts +++ b/apps/web/src/lib/startContainerChat.ts @@ -6,9 +6,10 @@ import type { ProjectId, ThreadId } from "@synara/contracts"; import type { Project } from "../types"; +import { draftNavigationSlotKey, runDraftNavigationOnce } from "./stagedDraftNavigation"; import { isStudioContainerProject } from "./studioProjects"; import type { ServerWorkspacePaths } from "./serverWorkspacePaths"; -import type { NewThreadOptions } from "./threadBootstrap"; +import type { NewThreadNavigationOwnership, NewThreadOptions } from "./threadBootstrap"; export type StartContainerChatResult = | { ok: true; threadId: ThreadId | null } @@ -44,19 +45,45 @@ export async function startContainerChat(input: { readonly handleNewThread: ( projectId: ProjectId, options?: NewThreadOptions, + navigation?: undefined, + existingOwnership?: NewThreadNavigationOwnership, ) => Promise; + readonly navigationTargetKey: string; readonly fresh?: boolean | undefined; readonly errorLabel: string; }): Promise { try { - const projectId = await input.ensureProjectId(); - if (!projectId) { - return { ok: false, error: input.errorLabel }; - } - const threadOptions: NewThreadOptions | undefined = - input.fresh === true ? { fresh: true, workspace: { kind: "local-container" } } : undefined; - const threadId = await input.handleNewThread(projectId, threadOptions); - return { ok: true, threadId }; + return await runDraftNavigationOnce( + draftNavigationSlotKey(), + `container:${input.navigationTargetKey}:${input.fresh === true ? "fresh" : "reuse"}`, + async (ownership) => { + try { + const projectId = await input.ensureProjectId(); + if (!ownership.isCurrent()) { + return { ok: true, threadId: null }; + } + if (!projectId) { + return { ok: false, error: input.errorLabel }; + } + const threadOptions: NewThreadOptions | undefined = + input.fresh === true + ? { fresh: true, workspace: { kind: "local-container" } } + : undefined; + const threadId = await input.handleNewThread( + projectId, + threadOptions, + undefined, + ownership, + ); + return { ok: true, threadId }; + } catch (error) { + if (!ownership.isCurrent()) { + return { ok: true, threadId: null }; + } + throw error; + } + }, + ); } catch (error) { return { ok: false, diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index 0079a2442..a2d991c68 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -15,6 +15,7 @@ import { const PROJECT_ID = ProjectId.makeUnsafe("project-bootstrap"); const THREAD_ID = ThreadId.makeUnsafe("thread-bootstrap"); +const DEFAULT_NAVIGATION_TARGET = { projectId: PROJECT_ID, entryPoint: "chat" as const }; const FIRST_NAVIGATION_SEARCH = (previous: Record) => ({ ...previous, editor: "first", @@ -82,14 +83,16 @@ function makeComposerDraftState( describe("threadBootstrap", () => { it("uses distinct navigation request keys for default and exact workspace intents", () => { - const defaultKey = newThreadNavigationRequestKey({}); + const defaultKey = newThreadNavigationRequestKey(DEFAULT_NAVIGATION_TARGET); expect( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { workspace: { kind: "project-default" } }, }), ).toBe(defaultKey); expect( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { workspace: { kind: "existing-worktree", @@ -101,38 +104,72 @@ describe("threadBootstrap", () => { ).not.toBe(defaultKey); expect( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { fresh: true }, }), ).not.toBe(defaultKey); }); it("does not coalesce distinct custom-search or preparation callbacks", () => { - expect(newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH })).toBe( - newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH }), + expect( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + customSearch: FIRST_NAVIGATION_SEARCH, + }), + ).toBe( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + customSearch: FIRST_NAVIGATION_SEARCH, + }), ); - expect(newThreadNavigationRequestKey({ customSearch: FIRST_NAVIGATION_SEARCH })).not.toBe( - newThreadNavigationRequestKey({ customSearch: SECOND_NAVIGATION_SEARCH }), + expect( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + customSearch: FIRST_NAVIGATION_SEARCH, + }), + ).not.toBe( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + customSearch: SECOND_NAVIGATION_SEARCH, + }), ); expect( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, }), ).toBe( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, }), ); expect( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { prepareNavigation: FIRST_NAVIGATION_PREPARATION }, }), ).not.toBe( newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, options: { prepareNavigation: SECOND_NAVIGATION_PREPARATION }, }), ); }); + it("keeps project and entry-point targets distinct on the global navigation surface", () => { + const projectChat = newThreadNavigationRequestKey({ + projectId: "project-a", + entryPoint: "chat", + }); + expect(newThreadNavigationRequestKey({ projectId: "project-b", entryPoint: "chat" })).not.toBe( + projectChat, + ); + expect( + newThreadNavigationRequestKey({ projectId: "project-a", entryPoint: "terminal" }), + ).not.toBe(projectChat); + }); + it("resolves project defaults and exact existing workspaces without partial states", () => { expect(resolveNewThreadWorkspace({ kind: "project-default" }, "worktree")).toEqual({ branch: null, diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index a5d8fb5a2..076e9f5f6 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -83,6 +83,8 @@ function navigationCallbackKey(callback: ((...args: never[]) => unknown) | undef } export function newThreadNavigationRequestKey(input: { + readonly projectId: string; + readonly entryPoint: ThreadPrimarySurface; readonly customSearch?: | ((previous: Record) => Record) | undefined; @@ -92,6 +94,8 @@ export function newThreadNavigationRequestKey(input: { const branch = "branch" in workspace ? workspace.branch : ""; const worktreePath = "worktreePath" in workspace ? workspace.worktreePath : ""; return [ + input.projectId, + input.entryPoint, workspace.kind, branch, worktreePath, From 2e191264b9af24c5d5103b0f1ab69b1b074770f8 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:31:18 +0300 Subject: [PATCH 08/14] fix(web): close remaining navigation races --- apps/web/src/components/ChatView.browser.tsx | 261 ++++++++++++++++++ apps/web/src/components/Sidebar.tsx | 10 +- .../pullRequest/PullRequestDetailPanel.tsx | 4 + apps/web/src/hooks/useHandleNewThread.ts | 25 +- apps/web/src/lib/newThreadLanding.test.ts | 20 +- apps/web/src/lib/newThreadLanding.ts | 18 ++ .../web/src/lib/stagedDraftNavigation.test.ts | 34 +++ apps/web/src/lib/stagedDraftNavigation.ts | 20 +- apps/web/src/lib/threadBootstrap.test.ts | 17 ++ apps/web/src/lib/threadBootstrap.ts | 17 +- 10 files changed, 410 insertions(+), 16 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 51e9c6016..52a596b7f 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5817,6 +5817,130 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("lets a renewed exact-workspace action win after an intervening default action", async () => { + const reusableDraftThreadId = ThreadId.makeUnsafe("thread-exact-default-exact-race"); + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/exact-default-exact", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/exact-default-exact", + }, + ], + }; + let resolveFirstValidation!: ( + value: Awaited>, + ) => void; + const firstValidation = new Promise>>( + (resolve) => { + resolveFirstValidation = resolve; + }, + ); + const branchLookup = vi.fn( + async () => exactWorktreeBranchResult, + ); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-exact-default-exact-race" as MessageId, + targetText: "exact default exact race", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/exact-default-exact", + worktreePath: "/repo/worktrees/exact-default-exact", + })), + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, + }), + }); + + try { + await waitForServerConfigToApply(); + await waitForComposerEditor(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); + await waitForLayout(); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + const openContextMenu = () => + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + useComposerDraftStore.getState().setProjectDraftThreadId(PROJECT_ID, reusableDraftThreadId, { + entryPoint: "chat", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + branchLookup.mockClear(); + branchLookup.mockImplementationOnce(() => firstValidation); + branchLookup.mockImplementation(async () => exactWorktreeBranchResult); + + openContextMenu(); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(1)); + + await page.getByTestId("new-thread-button").click(); + await waitForURL( + mounted.router, + (path) => path === `/${reusableDraftThreadId}`, + "The intervening ordinary action should reuse the default draft.", + ); + + openContextMenu(); + await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledTimes(2)); + const renewedExactPath = await waitForURL( + mounted.router, + (path) => UUID_ROUTE_RE.test(path), + "The renewed exact-workspace action should become the latest route.", + ); + const renewedExactThreadId = renewedExactPath.slice(1) as ThreadId; + expect(useComposerDraftStore.getState().getDraftThread(renewedExactThreadId)).toMatchObject({ + branch: "feature/exact-default-exact", + worktreePath: "/repo/worktrees/exact-default-exact", + envMode: "worktree", + workspaceOrigin: "intentional", + }); + + resolveFirstValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + expect(mounted.router.state.location.pathname).toBe(`/${renewedExactThreadId}`); + } finally { + resolveFirstValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + await mounted.cleanup(); + } + }); + it("preserves a reused draft when it supersedes a waiting exact-workspace request", async () => { const reusableDraftThreadId = ThreadId.makeUnsafe("thread-reused-draft-race"); const contextMenuShow = vi.fn( @@ -5952,6 +6076,143 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("does not adopt a staged route draft before its older navigation settles", async () => { + const reusableDraftThreadId = ThreadId.makeUnsafe("thread-staged-route-reuse-race"); + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/staged-route-race", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/staged-route-race", + }, + ], + }; + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-staged-route-reuse-race" as MessageId, + targetText: "staged route reuse race", + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: baseSnapshot.threads.map((thread) => ({ + ...thread, + envMode: "worktree" as const, + branch: "feature/staged-route-race", + worktreePath: "/repo/worktrees/staged-route-race", + })), + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: vi.fn(async () => exactWorktreeBranchResult), + }, + }), + }); + let releaseFirstNavigation!: () => void; + const firstNavigationBlocker = new Promise((resolve) => { + releaseFirstNavigation = resolve; + }); + const originalNavigate = mounted.router.navigate.bind(mounted.router); + let navigationCount = 0; + const navigateSpy = vi.spyOn(mounted.router, "navigate").mockImplementation((options) => { + navigationCount += 1; + const navigation = originalNavigate(options); + return navigationCount === 1 ? navigation.then(() => firstNavigationBlocker) : navigation; + }); + + try { + await waitForServerConfigToApply(); + await waitForComposerEditor(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); + await waitForLayout(); + const threadRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the current thread row.", + ); + useComposerDraftStore.getState().setProjectDraftThreadId(PROJECT_ID, reusableDraftThreadId, { + entryPoint: "chat", + envMode: "local", + branch: null, + worktreePath: null, + workspaceOrigin: "default", + }); + useComposerDraftStore.getState().setPrompt(reusableDraftThreadId, "keep staged-race prompt"); + useComposerDraftStore.getState().addFiles(reusableDraftThreadId, [ + { + type: "file", + id: "staged-route-race-notes", + name: "staged-route-race.txt", + mimeType: "text/plain", + sizeBytes: 5, + file: new File(["notes"], "staged-route-race.txt", { type: "text/plain" }), + }, + ]); + + threadRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + await vi.waitFor(() => expect(contextMenuShow).toHaveBeenCalledOnce()); + const stagedThreadPath = await waitForURL( + mounted.router, + (path) => UUID_ROUTE_RE.test(path), + "The older exact-workspace draft should become visible before navigation settles.", + ); + const stagedThreadId = stagedThreadPath.slice(1) as ThreadId; + expect(stagedThreadId).not.toBe(reusableDraftThreadId); + + await page.getByTestId("new-thread-button").click(); + await waitForURL( + mounted.router, + (path) => path === `/${reusableDraftThreadId}`, + "The later ordinary action should reuse the durable mapped draft.", + ); + releaseFirstNavigation(); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + + expect( + useComposerDraftStore.getState().getDraftThreadByProjectId(PROJECT_ID, "chat")?.threadId, + ).toBe(reusableDraftThreadId); + expect( + useComposerDraftStore.getState().draftsByThreadId[reusableDraftThreadId], + ).toMatchObject({ + prompt: "keep staged-race prompt", + files: [ + expect.objectContaining({ + id: "staged-route-race-notes", + name: "staged-route-race.txt", + }), + ], + }); + expect(useComposerDraftStore.getState().getDraftThread(stagedThreadId)).toBeNull(); + expect(mounted.router.state.location.pathname).toBe(`/${reusableDraftThreadId}`); + } finally { + releaseFirstNavigation(); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + navigateSpy.mockRestore(); + await mounted.cleanup(); + } + }); + it("keeps a later terminal intent and preserves chat attachments across entry points", async () => { const reusableChatDraftId = ThreadId.makeUnsafe("thread-cross-entry-chat-draft"); const reusableTerminalDraftId = ThreadId.makeUnsafe("thread-cross-entry-terminal-draft"); diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 577508193..66808fbbd 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -1709,7 +1709,6 @@ export default function Sidebar() { ); const [projectRunDialogCommandDraft, setProjectRunDialogCommandDraft] = useState(""); const isAddingProjectRef = useRef(false); - const newThreadInWorkspaceInFlightThreadIdsRef = useRef(new Set()); const [projectInitializationPreview, setProjectInitializationPreview] = useState(null); const [projectInitializationError, setProjectInitializationError] = useState(null); @@ -3824,10 +3823,7 @@ export default function Sidebar() { ); if (clicked === "new-thread-in-workspace") { - if ( - !newThreadInWorkspaceAction || - newThreadInWorkspaceInFlightThreadIdsRef.current.has(threadId) - ) { + if (!newThreadInWorkspaceAction) { return; } const projectCwd = projectCwdById.get(thread.projectId) ?? null; @@ -3838,13 +3834,13 @@ export default function Sidebar() { }); return; } - newThreadInWorkspaceInFlightThreadIdsRef.current.add(threadId); let workspaceValidationFailure: string | null = null; try { // A null result means a newer New Thread intent superseded this one; // the newer owner is responsible for the visible destination. await handleNewThread(thread.projectId, { fresh: true, + prepareNavigationKey: `sidebar-exact-workspace:${projectCwd}`, prepareNavigation: async () => { const currentProjectCwd = useStore.getState().projects.find((project) => project.id === thread.projectId) @@ -3873,8 +3869,6 @@ export default function Sidebar() { description: error instanceof Error ? error.message : "The workspace could not be verified.", }); - } finally { - newThreadInWorkspaceInFlightThreadIdsRef.current.delete(threadId); } return; } diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index a5a400316..100182fe2 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -207,6 +207,10 @@ export function PullRequestDetailPanel({ // existing draft can leave the user on the PR route and insert the prompt into a // hidden composer, making the button appear inert. fresh: true, + // Native PR preparation can checkout the root repository and has no cancellation API. + // A later navigation may supersede this request, but must not become active until that + // explicit Git mutation settles and can no longer change the workspace underneath it. + prepareNavigationBlocksFollowing: true, prepareNavigation: async (ownership) => { const prepared = await prepareThreadMutation.mutateAsync({ reference: detail.url, diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 2949eaeff..639f64849 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -3,7 +3,13 @@ import { getRecommendedDefaultModelSelection } from "@synara/shared/model"; import { useNavigate, useRouter } from "@tanstack/react-router"; import { startTransition, useCallback } from "react"; import { useAppSettings } from "../appSettings"; -import { clearNewThreadLanding, markNewThreadLanding } from "../lib/newThreadLanding"; +import { + clearNewThreadDraftStaged, + clearNewThreadLanding, + isNewThreadDraftStaged, + markNewThreadDraftStaged, + markNewThreadLanding, +} from "../lib/newThreadLanding"; import { type ComposerThreadDraftState, type DraftThreadState, @@ -169,9 +175,10 @@ export function useHandleNewThread() { focusedThreadId === routeThreadId ? currentRouteThreadId : focusedThreadId; const shouldForceFreshThread = effectiveOptions?.fresh === true; const storedDraftThreadCandidate = getDraftThreadByProjectId(projectId, entryPoint); - const latestActiveDraftThreadCandidate: DraftThreadState | null = currentFocusedThreadId - ? getDraftThread(currentFocusedThreadId) - : null; + const latestActiveDraftThreadCandidate: DraftThreadState | null = + currentFocusedThreadId && !isNewThreadDraftStaged(currentFocusedThreadId) + ? getDraftThread(currentFocusedThreadId) + : null; const storedDraftThread = !shouldForceFreshThread && !wantsTemporaryThread && @@ -289,6 +296,7 @@ export function useHandleNewThread() { // Keep the previous routed draft alive while the destination loads. Replacing the // project's primary slot earlier makes the route guard redirect the old URL to Home. stage: () => { + markNewThreadDraftStaged(threadId); registerDraftThread(threadId, { projectId, ...draftSeed }); markNewThreadLanding(threadId); activateThreadEntryPoint(threadId); @@ -311,8 +319,12 @@ export function useHandleNewThread() { // TanStack resolves an older navigate() promise when a newer navigation supersedes it. // Verify the committed route before deleting the previous project draft. isDestinationActive: () => router.state.location.pathname === `/${threadId}`, - finalize: () => setProjectDraftThreadId(projectId, threadId, draftSeed), + finalize: () => { + setProjectDraftThreadId(projectId, threadId, draftSeed); + clearNewThreadDraftStaged(threadId); + }, rollback: () => { + clearNewThreadDraftStaged(threadId); clearNewThreadLanding(threadId); clearDraftThread(threadId); clearTerminalState(threadId); @@ -344,6 +356,9 @@ export function useHandleNewThread() { options, }), runOwnedNavigation, + { + blocksFollowingOperations: options?.prepareNavigationBlocksFollowing === true, + }, ); }, [ diff --git a/apps/web/src/lib/newThreadLanding.test.ts b/apps/web/src/lib/newThreadLanding.test.ts index 655aca603..0585af256 100644 --- a/apps/web/src/lib/newThreadLanding.test.ts +++ b/apps/web/src/lib/newThreadLanding.test.ts @@ -1,14 +1,20 @@ import { afterEach, describe, expect, it } from "vitest"; import { + clearNewThreadDraftStaged, clearNewThreadLanding, + isNewThreadDraftStaged, isNewThreadLandingPending, + markNewThreadDraftStaged, markNewThreadLanding, } from "./newThreadLanding"; const THREAD_ID = "thread-new-landing"; -afterEach(() => clearNewThreadLanding(THREAD_ID)); +afterEach(() => { + clearNewThreadLanding(THREAD_ID); + clearNewThreadDraftStaged(THREAD_ID); +}); describe("newThreadLanding", () => { it("marks and clears a one-shot draft landing", () => { @@ -18,4 +24,16 @@ describe("newThreadLanding", () => { clearNewThreadLanding(THREAD_ID); expect(isNewThreadLandingPending(THREAD_ID)).toBe(false); }); + + it("tracks an uncommitted staged draft independently from its one-shot landing", () => { + markNewThreadLanding(THREAD_ID); + markNewThreadDraftStaged(THREAD_ID); + + clearNewThreadLanding(THREAD_ID); + expect(isNewThreadLandingPending(THREAD_ID)).toBe(false); + expect(isNewThreadDraftStaged(THREAD_ID)).toBe(true); + + clearNewThreadDraftStaged(THREAD_ID); + expect(isNewThreadDraftStaged(THREAD_ID)).toBe(false); + }); }); diff --git a/apps/web/src/lib/newThreadLanding.ts b/apps/web/src/lib/newThreadLanding.ts index a23694f8b..d25ddfa76 100644 --- a/apps/web/src/lib/newThreadLanding.ts +++ b/apps/web/src/lib/newThreadLanding.ts @@ -1,6 +1,7 @@ // Tracks only the first route landing for a freshly created local draft. This // is intentionally ephemeral: persisted or reopened drafts must mount normally. const pendingNewThreadLandings = new Set(); +const stagedNewThreadDrafts = new Set(); export function markNewThreadLanding(threadId: string): void { pendingNewThreadLandings.add(threadId); @@ -13,3 +14,20 @@ export function isNewThreadLandingPending(threadId: string): boolean { export function clearNewThreadLanding(threadId: string): void { pendingNewThreadLandings.delete(threadId); } + +/** + * Tracks fresh drafts that are visible in the route but have not yet won navigation ownership. + * A newer New Thread action must not reuse one of these transient drafts while the older + * navigation promise is still pending; the older owner may subsequently roll it back. + */ +export function markNewThreadDraftStaged(threadId: string): void { + stagedNewThreadDrafts.add(threadId); +} + +export function isNewThreadDraftStaged(threadId: string): boolean { + return stagedNewThreadDrafts.has(threadId); +} + +export function clearNewThreadDraftStaged(threadId: string): void { + stagedNewThreadDrafts.delete(threadId); +} diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index afa2a7762..94e361a94 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -177,6 +177,40 @@ describe("stagedDraftNavigation", () => { expect(defaultRun).toHaveBeenCalledOnce(); }); + it("keeps later navigation behind an explicit blocking preparation", async () => { + let releasePreparation!: () => void; + const firstOwnership: Array<{ readonly isCurrent: () => boolean }> = []; + const laterRun = vi.fn(async (ownership: { readonly isCurrent: () => boolean }) => + ownership.isCurrent() ? "latest" : "superseded", + ); + const slotKey = draftNavigationSlotKey(); + + const preparation = runDraftNavigationOnce( + slotKey, + "mutating-pr-preparation", + async (ownership) => { + firstOwnership.push(ownership); + await new Promise((resolve) => { + releasePreparation = resolve; + }); + return ownership.isCurrent() ? "stale-commit" : "superseded"; + }, + { blocksFollowingOperations: true }, + ); + await Promise.resolve(); + expect(firstOwnership[0]?.isCurrent()).toBe(true); + + const later = runDraftNavigationOnce(slotKey, "project-default", laterRun); + await Promise.resolve(); + expect(firstOwnership[0]?.isCurrent()).toBe(false); + expect(laterRun).not.toHaveBeenCalled(); + + releasePreparation(); + await expect(preparation).resolves.toBe("superseded"); + await expect(later).resolves.toBe("latest"); + expect(laterRun).toHaveBeenCalledOnce(); + }); + it("supersedes an awaited owner as soon as a distinct later intent arrives", async () => { let releaseFirst!: () => void; const firstOwnership: Array<{ readonly isCurrent: () => boolean }> = []; diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index 53d2eadac..b81dddca3 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -4,6 +4,7 @@ // Layer: Web navigation orchestration interface DraftNavigationSlotState { + blockingBarrier: Promise | null; tail: Promise; latestOperation: Promise | null; latestRequestKey: string | null; @@ -39,10 +40,12 @@ export function runDraftNavigationOnce( slotKey: string, requestKey: string, run: (ownership: DraftNavigationOwnership) => Promise, + options?: { readonly blocksFollowingOperations?: boolean }, ): Promise { let state = draftNavigationStateBySlot.get(slotKey); if (!state) { state = { + blockingBarrier: null, tail: Promise.resolve(), latestOperation: null, latestRequestKey: null, @@ -60,7 +63,10 @@ export function runDraftNavigationOnce( isCurrent: () => state.latestOwner === owner, }; state.latestOwner = owner; - const execution = Promise.resolve().then(() => run(ownership)); + const priorBlockingBarrier = state.blockingBarrier; + const execution = priorBlockingBarrier + ? priorBlockingBarrier.then(() => run(ownership)) + : Promise.resolve().then(() => run(ownership)); let operation!: Promise; const clearLatestRequest = () => { if (state.latestOperation === operation) { @@ -81,6 +87,18 @@ export function runDraftNavigationOnce( ); state.latestOperation = operation; state.latestRequestKey = requestKey; + if (options?.blocksFollowingOperations === true) { + const blockingBarrier = operation.then( + () => undefined, + () => undefined, + ); + state.blockingBarrier = blockingBarrier; + void blockingBarrier.then(() => { + if (state.blockingBarrier === blockingBarrier) { + state.blockingBarrier = null; + } + }); + } const previousTail = state.tail; const tail = Promise.all([previousTail, operation]).then( diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index a2d991c68..25f515d20 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -155,6 +155,23 @@ describe("threadBootstrap", () => { options: { prepareNavigation: SECOND_NAVIGATION_PREPARATION }, }), ); + expect( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + options: { + prepareNavigation: FIRST_NAVIGATION_PREPARATION, + prepareNavigationKey: "sidebar-exact-workspace", + }, + }), + ).toBe( + newThreadNavigationRequestKey({ + ...DEFAULT_NAVIGATION_TARGET, + options: { + prepareNavigation: SECOND_NAVIGATION_PREPARATION, + prepareNavigationKey: "sidebar-exact-workspace", + }, + }), + ); }); it("keeps project and entry-point targets distinct on the global navigation surface", () => { diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index 076e9f5f6..8b0b2b41c 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -37,6 +37,16 @@ export interface NewThreadOptions { prepareNavigation?: ( ownership: NewThreadNavigationOwnership, ) => Promise; + /** + * Stable identity for preparations whose callback is recreated by the UI. Adjacent identical + * actions coalesce, while the same action after a distinct intent starts a new operation. + */ + prepareNavigationKey?: string; + /** + * Prevents a later route from becoming active while this preparation performs an explicit + * non-cancellable side effect, such as checking out a pull-request branch. + */ + prepareNavigationBlocksFollowing?: boolean; } export interface NewThreadNavigationOwnership { @@ -102,8 +112,13 @@ export function newThreadNavigationRequestKey(input: { input.options?.provider ?? "", input.options?.temporary === true ? "temporary" : "durable", input.options?.fresh === true ? "fresh" : "reuse-eligible", + input.options?.prepareNavigationBlocksFollowing === true + ? "blocking-preparation" + : "concurrent-preparation", `search:${navigationCallbackKey(input.customSearch)}`, - `prepare:${navigationCallbackKey(input.options?.prepareNavigation)}`, + `prepare:${ + input.options?.prepareNavigationKey ?? navigationCallbackKey(input.options?.prepareNavigation) + }`, ].join("\u0000"); } From bc3d97fe36953f28692e0d814e2125974a73ad16 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 21:30:26 +0300 Subject: [PATCH 09/14] chore: clean merged sidebar imports --- apps/web/src/components/Sidebar.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index f52db7b1c..72728fd8e 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -88,7 +88,7 @@ import { type ServerLocalServerProcess, } from "@synara/contracts"; import { isGenericChatThreadTitle } from "@synara/shared/chatThreads"; -import { getDefaultModel, getRecommendedDefaultModelSelection } from "@synara/shared/model"; +import { getRecommendedDefaultModelSelection } from "@synara/shared/model"; import { pluralize } from "@synara/shared/text"; import { localServerAddressLabel, localServerMatchesRun } from "@synara/shared/localServers"; import { resolveThreadWorkspaceCwd } from "@synara/shared/threadEnvironment"; @@ -328,7 +328,6 @@ import { resolveThreadRowTrailingReserveClass, resolveThreadStatusPill, validateNewThreadInWorkspaceAction, - type ThreadStatusPill, type SidebarDerivedProjectData, type SidebarActionBadge, type SidebarView, From 868402261c9519c8daefdd260ea6af62c4a0df5b Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 22:05:42 +0300 Subject: [PATCH 10/14] fix(web): close terminal and route ownership gaps --- apps/web/src/components/ChatView.browser.tsx | 145 +++++++++++++++++- .../useThreadActivationController.test.ts | 31 ++++ .../hooks/useThreadActivationController.ts | 9 +- .../web/src/lib/stagedDraftNavigation.test.ts | 23 +++ apps/web/src/lib/stagedDraftNavigation.ts | 14 ++ apps/web/src/lib/threadBootstrap.test.ts | 65 ++++++++ apps/web/src/lib/threadBootstrap.ts | 54 +++++-- apps/web/src/routes/_chat.tsx | 1 + 8 files changed, 329 insertions(+), 13 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 52a596b7f..42ed9e4d1 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5656,7 +5656,8 @@ describe("ChatView timeline estimator parity (full app)", () => { useComposerDraftStore.getState().setPrompt(reusableDraftThreadId, "later draft prompt"); await dispatchThreadShortcut("c"); - await vi.waitFor(() => expect(refreshProviders).toHaveBeenCalled()); + await dispatchThreadShortcut("c"); + await vi.waitFor(() => expect(refreshProviders).toHaveBeenCalledOnce()); await page.getByTestId("new-thread-button").click(); useComposerDraftStore.getState().addFiles(reusableDraftThreadId, [ @@ -5817,6 +5818,122 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); + it("keeps a newer existing-thread click ahead of delayed exact-workspace validation", async () => { + const otherThreadId = ThreadId.makeUnsafe("thread-existing-navigation-wins"); + const contextMenuShow = vi.fn( + async (_items: Parameters[0]) => "new-thread-in-workspace", + ); + const exactWorktreeBranchResult: Awaited> = { + isRepo: true, + hasOriginRemote: true, + branches: [ + { + name: "feature/delayed-exact", + current: false, + isDefault: false, + worktreePath: "/repo/worktrees/delayed-exact", + }, + ], + }; + let resolveValidation!: (value: Awaited>) => void; + const delayedValidation = new Promise>>( + (resolve) => { + resolveValidation = resolve; + }, + ); + const branchLookup = vi.fn(() => delayedValidation); + const baseSnapshot = createSnapshotForTargetUser({ + targetMessageId: "msg-user-existing-navigation-wins" as MessageId, + targetText: "existing navigation wins", + }); + const sourceThread = baseSnapshot.threads[0]!; + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: { + ...baseSnapshot, + threads: [ + { + ...sourceThread, + envMode: "worktree", + branch: "feature/delayed-exact", + worktreePath: "/repo/worktrees/delayed-exact", + }, + { + ...sourceThread, + id: otherThreadId, + title: "Existing destination thread", + messages: [], + envMode: "local", + branch: "main", + worktreePath: null, + }, + ], + }, + configureNativeApi: (api) => ({ + ...api, + contextMenu: { + ...api.contextMenu, + show: contextMenuShow as NativeApi["contextMenu"]["show"], + }, + git: { + ...api.git, + listBranches: branchLookup, + }, + }), + }); + + try { + await waitForServerConfigToApply(); + await waitForComposerEditor(); + useStore.getState().setProjectExpanded(PROJECT_ID, true); + await waitForLayout(); + const sourceRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes(THREAD_TITLE), + ) ?? null, + "Unable to find the source thread row.", + ); + const destinationRow = await waitForElement( + () => + Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( + (row) => row.textContent?.includes("Existing destination thread"), + ) ?? null, + "Unable to find the destination thread row.", + ); + + sourceRow.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + clientX: 24, + clientY: 24, + }), + ); + await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledOnce()); + + destinationRow.click(); + await waitForURL( + mounted.router, + (path) => path === `/${otherThreadId}`, + "The newer existing-thread click should take control of the route.", + ); + resolveValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + + expect(mounted.router.state.location.pathname).toBe(`/${otherThreadId}`); + expect( + Object.values(useComposerDraftStore.getState().draftThreadsByThreadId).some( + (draft) => draft.worktreePath === "/repo/worktrees/delayed-exact", + ), + ).toBe(false); + } finally { + resolveValidation(exactWorktreeBranchResult); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + await mounted.cleanup(); + } + }); + it("lets a renewed exact-workspace action win after an intervening default action", async () => { const reusableDraftThreadId = ThreadId.makeUnsafe("thread-exact-default-exact-race"); const contextMenuShow = vi.fn( @@ -6509,6 +6626,16 @@ describe("ChatView timeline estimator parity (full app)", () => { ], }; }, + configureNativeApi: (api) => ({ + ...api, + server: { + ...api.server, + getSettings: async () => ({ + ...DEFAULT_SERVER_SETTINGS, + defaultThreadEnvMode: "worktree", + }), + }, + }), }); try { @@ -6572,6 +6699,22 @@ describe("ChatView timeline estimator parity (full app)", () => { useStore.getState().setProjectExpanded(PROJECT_ID, true); useComposerDraftStore.getState().clearDraftThread(newThreadId); + await vi.waitFor( + () => { + expect( + wsRequests.find( + (request) => + request._tag === WS_METHODS.terminalOpen && request.threadId === newThreadId, + ), + ).toMatchObject({ + _tag: WS_METHODS.terminalOpen, + threadId: newThreadId, + cwd: "/repo/project", + }); + }, + { timeout: 8_000, interval: 16 }, + ); + await vi.waitFor( () => { const terminalThreadRow = document.querySelector( diff --git a/apps/web/src/hooks/useThreadActivationController.test.ts b/apps/web/src/hooks/useThreadActivationController.test.ts index f26c45e4d..77b8ccf13 100644 --- a/apps/web/src/hooks/useThreadActivationController.test.ts +++ b/apps/web/src/hooks/useThreadActivationController.test.ts @@ -2,6 +2,11 @@ import { describe, expect, it, vi } from "vitest"; import { ProjectId, ThreadId } from "@synara/contracts"; import type { SplitView } from "../splitViewStore"; +import { + draftNavigationSlotKey, + runDraftNavigationOnce, + waitForDraftNavigationIdle, +} from "../lib/stagedDraftNavigation"; import { activateThreadFromSidebarIntent, type ThreadActivationControllerInput, @@ -112,6 +117,32 @@ function getFirstNavigateArgs(input: { navigate: ReturnType }) { } describe("activateThreadFromSidebarIntent", () => { + it("supersedes a pending new-thread preparation before activating an existing thread", async () => { + let releasePreparation!: () => void; + let pendingOwnerWasCurrent = true; + const pending = runDraftNavigationOnce( + draftNavigationSlotKey(), + "delayed-exact-workspace", + async (ownership) => { + await new Promise((resolve) => { + releasePreparation = resolve; + }); + pendingOwnerWasCurrent = ownership.isCurrent(); + return ownership.isCurrent(); + }, + ); + await Promise.resolve(); + const input = makeControllerInput(); + + activateThreadFromSidebarIntent(input, THREAD_B); + releasePreparation(); + + await expect(pending).resolves.toBe(false); + expect(pendingOwnerWasCurrent).toBe(false); + expect(input.openChatThreadPage).toHaveBeenCalledWith(THREAD_B); + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + }); + it("focuses a target pane in the active split", () => { const activeSplitView = makeSplitViewFixture({ id: "split-active", diff --git a/apps/web/src/hooks/useThreadActivationController.ts b/apps/web/src/hooks/useThreadActivationController.ts index 005bf22ea..fcfe5d0e0 100644 --- a/apps/web/src/hooks/useThreadActivationController.ts +++ b/apps/web/src/hooks/useThreadActivationController.ts @@ -9,6 +9,7 @@ import type { LastThreadRoute } from "../chatRouteRestore"; import { type PaneId, type SplitView, type SplitViewId } from "../splitViewStore"; import { selectThreadTerminalState } from "../terminalStateStore"; import type { SidebarThreadSummary } from "../types"; +import { draftNavigationSlotKey, supersedeDraftNavigation } from "../lib/stagedDraftNavigation"; import { resolvePreferredSplitForCommand, resolveThreadCommandActivation, @@ -76,9 +77,15 @@ export function activateThreadFromSidebarIntent( threadId, }); const targetThread = sidebarThreadSummaryById[threadId]; + if (!targetThread) { + return; + } + // Selecting an existing thread is a route intent on the same visible surface as New Thread. + // Revoke any delayed preparation before deciding whether this activation changes the route. + supersedeDraftNavigation(draftNavigationSlotKey()); const activation = resolveThreadCommandActivation({ threadId, - threadExists: targetThread !== undefined, + threadExists: true, activeSidebarThreadId: routeThreadId, preferredSplitViewId: preferredSplit?.splitViewId ?? null, splitPaneId: preferredSplit?.paneId ?? null, diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index 94e361a94..ed0a5b0be 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -4,6 +4,8 @@ import { draftNavigationSlotKey, runDraftNavigationOnce, stageDraftNavigation, + supersedeDraftNavigation, + waitForDraftNavigationIdle, } from "./stagedDraftNavigation"; describe("stagedDraftNavigation", () => { @@ -239,6 +241,27 @@ describe("stagedDraftNavigation", () => { expect(secondWasCurrent).toBe(true); }); + it("revokes an awaited owner when an existing-route navigation takes control", async () => { + let releasePreparation!: () => void; + let wasCurrentAfterRelease = true; + const slotKey = draftNavigationSlotKey(); + const pending = runDraftNavigationOnce(slotKey, "exact-worktree", async (ownership) => { + await new Promise((resolve) => { + releasePreparation = resolve; + }); + wasCurrentAfterRelease = ownership.isCurrent(); + return ownership.isCurrent() ? "stale-navigation" : "superseded"; + }); + await Promise.resolve(); + + supersedeDraftNavigation(slotKey); + releasePreparation(); + + await expect(pending).resolves.toBe("superseded"); + expect(wasCurrentAfterRelease).toBe(false); + await expect(waitForDraftNavigationIdle(slotKey)).resolves.toBeUndefined(); + }); + it("supersedes delayed work across projects and chat or terminal entry points", async () => { let releaseProjectChat!: () => void; let projectChatWasCurrentAfterRelease = true; diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index b81dddca3..9ed60893e 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -31,6 +31,20 @@ export function waitForDraftNavigationIdle(slotKey: string): Promise { return draftNavigationStateBySlot.get(slotKey)?.tail ?? Promise.resolve(); } +/** + * Transfers visible-route ownership to a navigation that is not creating a new thread. Pending + * preparations may finish their read-only work, but they can no longer stage or commit a route. + */ +export function supersedeDraftNavigation(slotKey: string): void { + const state = draftNavigationStateBySlot.get(slotKey); + if (!state) { + return; + } + state.latestOwner = Symbol("external-navigation"); + state.latestOperation = null; + state.latestRequestKey = null; +} + /** * Coalesces adjacent identical requests while starting distinct requests independently. A distinct * later request becomes the owner immediately, allowing it to make progress without waiting for a diff --git a/apps/web/src/lib/threadBootstrap.test.ts b/apps/web/src/lib/threadBootstrap.test.ts index 25f515d20..7884e1cf0 100644 --- a/apps/web/src/lib/threadBootstrap.test.ts +++ b/apps/web/src/lib/threadBootstrap.test.ts @@ -8,6 +8,7 @@ import { createFreshDraftThreadSeed, newThreadNavigationRequestKey, resolveNewThreadWorkspace, + resolveNewThreadWorkspaceForEntryPoint, resolveTerminalThreadCreationState, resolveThreadBootstrapPlan, shouldReuseActiveDraftThread, @@ -225,6 +226,48 @@ describe("threadBootstrap", () => { }); }); + it("uses an honest local workspace for immediate terminals without a materialized worktree", () => { + expect( + resolveNewThreadWorkspaceForEntryPoint({ + defaultEnvMode: "worktree", + entryPoint: "terminal", + intent: { kind: "project-default" }, + }), + ).toEqual({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "default", + }); + expect( + resolveNewThreadWorkspaceForEntryPoint({ + defaultEnvMode: "local", + entryPoint: "terminal", + intent: { kind: "existing-worktree", branch: "stale", worktreePath: "" }, + }), + ).toEqual({ + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: "intentional", + }); + expect( + resolveNewThreadWorkspaceForEntryPoint({ + defaultEnvMode: "local", + entryPoint: "terminal", + intent: { + kind: "existing-worktree", + branch: "feature/exact", + worktreePath: "/repo/.worktrees/exact", + }, + }), + ).toMatchObject({ + branch: "feature/exact", + worktreePath: "/repo/.worktrees/exact", + envMode: "worktree", + }); + }); + it("recomputes inactive default drafts while preserving active and intentional workspaces", () => { expect( buildDraftThreadWorkspacePatch({ @@ -484,4 +527,26 @@ describe("threadBootstrap", () => { branch: null, }); }); + + it("fails terminal promotion safely to local when worktree metadata has no concrete path", () => { + expect( + resolveTerminalThreadCreationState({ + activeDraftThread: null, + activeThread: null, + defaultEnvMode: "worktree", + draftComposerState: makeComposerDraftState(), + draftThread: makeDraftThread({ + branch: "stale-branch", + envMode: "worktree", + worktreePath: null, + }), + projectDefaultModelSelection: modelSelection("codex", "gpt-5.4"), + projectId: PROJECT_ID, + }), + ).toMatchObject({ + envMode: "local", + branch: null, + worktreePath: null, + }); + }); }); diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index 8b0b2b41c..7075a93f3 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -158,6 +158,30 @@ export function resolveNewThreadWorkspace( } } +// Terminal-first threads are promoted and opened immediately, before the agent lifecycle can +// materialize a brand-new worktree. A project-default worktree without a concrete path must +// therefore become an honest local terminal instead of claiming isolation while running at root. +export function resolveNewThreadWorkspaceForEntryPoint(input: { + readonly defaultEnvMode: DraftThreadEnvMode; + readonly entryPoint: ThreadPrimarySurface; + readonly intent: NewThreadWorkspaceIntent; +}): ResolvedNewThreadWorkspace { + const workspace = resolveNewThreadWorkspace(input.intent, input.defaultEnvMode); + if ( + input.entryPoint !== "terminal" || + workspace.envMode !== "worktree" || + Boolean(workspace.worktreePath?.trim()) + ) { + return workspace; + } + return { + branch: null, + worktreePath: null, + envMode: "local", + workspaceOrigin: workspace.workspaceOrigin, + }; +} + interface ActiveThreadSnapshot { projectId: ProjectId; modelSelection: ModelSelection; @@ -296,10 +320,11 @@ export function createFreshDraftThreadSeed(input: { entryPoint: ThreadPrimarySurface; options?: NewThreadOptions | undefined; }): Omit { - const workspace = resolveNewThreadWorkspace( - input.options?.workspace ?? { kind: "project-default" }, - input.defaultEnvMode, - ); + const workspace = resolveNewThreadWorkspaceForEntryPoint({ + defaultEnvMode: input.defaultEnvMode, + entryPoint: input.entryPoint, + intent: input.options?.workspace ?? { kind: "project-default" }, + }); return { createdAt: input.createdAt, ...workspace, @@ -338,10 +363,11 @@ export function buildDraftThreadWorkspacePatch(input: { return null; } return { - ...resolveNewThreadWorkspace( - input.options?.workspace ?? { kind: "project-default" }, - input.defaultEnvMode, - ), + ...resolveNewThreadWorkspaceForEntryPoint({ + defaultEnvMode: input.defaultEnvMode, + entryPoint: input.entryPoint, + intent: input.options?.workspace ?? { kind: "project-default" }, + }), entryPoint: input.entryPoint, }; } @@ -370,6 +396,9 @@ export function shouldReuseActiveDraftThread(input: { export function resolveTerminalThreadCreationState( input: ResolveTerminalThreadCreationStateInput, ): TerminalThreadCreationState { + const draftEnvMode = input.draftThread?.envMode ?? input.defaultEnvMode; + const draftWorktreePath = input.draftThread?.worktreePath ?? null; + const hasConcreteWorktree = draftEnvMode === "worktree" && Boolean(draftWorktreePath?.trim()); return { modelSelection: resolvePreferredComposerModelSelection({ draft: input.draftComposerState, @@ -400,8 +429,11 @@ export function resolveTerminalThreadCreationState( ? (input.activeDraftThread.lastKnownPr ?? null) : null) ?? null, - envMode: input.draftThread?.envMode ?? input.defaultEnvMode, - branch: input.draftThread?.branch ?? null, - worktreePath: input.draftThread?.worktreePath ?? null, + envMode: draftEnvMode === "worktree" && !hasConcreteWorktree ? "local" : draftEnvMode, + branch: + draftEnvMode === "worktree" && !hasConcreteWorktree + ? null + : (input.draftThread?.branch ?? null), + worktreePath: hasConcreteWorktree ? draftWorktreePath : null, }; } diff --git a/apps/web/src/routes/_chat.tsx b/apps/web/src/routes/_chat.tsx index 136790fca..27dceb820 100644 --- a/apps/web/src/routes/_chat.tsx +++ b/apps/web/src/routes/_chat.tsx @@ -390,6 +390,7 @@ function ChatRouteGlobalShortcuts() { event.stopPropagation(); void handleNewThread(target.projectId, { provider, + prepareNavigationKey: `provider-shortcut:${provider}`, prepareNavigation: async (ownership) => { const providerAvailability = await resolveProviderSendAvailabilityWithRefresh({ provider, From b679d0cafc259de2ef1fbd78b29c11e238184be6 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 22:34:12 +0300 Subject: [PATCH 11/14] fix(web): coordinate every new-thread route intent --- KEYBINDINGS.md | 2 +- .../web/src/components/AppSnapCoordinator.tsx | 6 + apps/web/src/components/ChatView.browser.tsx | 18 +-- apps/web/src/hooks/useHandleNewThread.ts | 7 +- apps/web/src/hooks/useRecentViewSwitcher.ts | 13 +- .../useThreadActivationController.test.ts | 62 ++++++-- .../hooks/useThreadActivationController.ts | 17 +- .../web/src/lib/stagedDraftNavigation.test.ts | 145 +++++++++++++++++- apps/web/src/lib/stagedDraftNavigation.ts | 71 ++++++++- apps/web/src/lib/threadBootstrap.ts | 1 + apps/web/src/routes/__root.tsx | 17 ++ 11 files changed, 315 insertions(+), 44 deletions(-) diff --git a/KEYBINDINGS.md b/KEYBINDINGS.md index 93a9baf95..79a2bd8b3 100644 --- a/KEYBINDINGS.md +++ b/KEYBINDINGS.md @@ -54,7 +54,7 @@ Invalid rules are ignored. Invalid config files are ignored. Warnings are logged - `terminal.close`: close/kill the focused terminal (in focused terminal context by default) - `chat.new`: create a new chat thread using the target project's configured workspace default - `chat.newLocal`: create a new chat thread for the active project in a new environment (local/worktree determined by app settings (default `local`)) -- `chat.newTerminal`: create a new terminal-first thread using the target project's configured workspace default +- `chat.newTerminal`: create a new terminal-first thread using the target project's configured workspace default; if a configured worktree has no concrete path, safely fall back to the project's local checkout - `composer.focus.toggle`: focus or blur the chat prompt composer - `editor.openFavorite`: open current project/worktree in the last-used editor - `script.{id}.run`: run a project script by id (for example `script.test.run`) diff --git a/apps/web/src/components/AppSnapCoordinator.tsx b/apps/web/src/components/AppSnapCoordinator.tsx index 7d487a7a7..180aeff5a 100644 --- a/apps/web/src/components/AppSnapCoordinator.tsx +++ b/apps/web/src/components/AppSnapCoordinator.tsx @@ -49,6 +49,10 @@ import { transientAlertManager } from "../notifications/transientAlert"; import { useSplitViewStore } from "../splitViewStore"; import { useStore } from "../store"; import { useTerminalStateStore } from "../terminalStateStore"; +import { + coordinateExternalRouteNavigation, + draftNavigationSlotKey, +} from "../lib/stagedDraftNavigation"; const MAX_REMEMBERED_CAPTURE_IDS = 100; const APPSNAP_LISTENER_ACTIVITY_KEY = "appsnap:listener"; @@ -327,6 +331,8 @@ export function AppSnapCoordinator() { const activateExistingTarget = useCallback( async (target: AppSnapThreadTarget) => { + const mayActivate = await coordinateExternalRouteNavigation(draftNavigationSlotKey()); + if (!mayActivate) return; openChatThreadPage(target.threadId); // Same thread is only "already active" when the split pane matches too; // a capture aimed at another pane still needs activation below. diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index 42ed9e4d1..a230d77e6 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -5818,7 +5818,7 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); - it("keeps a newer existing-thread click ahead of delayed exact-workspace validation", async () => { + it("keeps a newer direct Kanban route ahead of delayed exact-workspace validation", async () => { const otherThreadId = ThreadId.makeUnsafe("thread-existing-navigation-wins"); const contextMenuShow = vi.fn( async (_items: Parameters[0]) => "new-thread-in-workspace", @@ -5894,14 +5894,6 @@ describe("ChatView timeline estimator parity (full app)", () => { ) ?? null, "Unable to find the source thread row.", ); - const destinationRow = await waitForElement( - () => - Array.from(document.querySelectorAll("[data-thread-entry-point]")).find( - (row) => row.textContent?.includes("Existing destination thread"), - ) ?? null, - "Unable to find the destination thread row.", - ); - sourceRow.dispatchEvent( new MouseEvent("contextmenu", { bubbles: true, @@ -5912,16 +5904,16 @@ describe("ChatView timeline estimator parity (full app)", () => { ); await vi.waitFor(() => expect(branchLookup).toHaveBeenCalledOnce()); - destinationRow.click(); + await mounted.router.navigate({ to: "/kanban" }); await waitForURL( mounted.router, - (path) => path === `/${otherThreadId}`, - "The newer existing-thread click should take control of the route.", + (path) => path === "/kanban", + "The newer direct Kanban route should take control of the route.", ); resolveValidation(exactWorktreeBranchResult); await waitForDraftNavigationIdle(draftNavigationSlotKey()); - expect(mounted.router.state.location.pathname).toBe(`/${otherThreadId}`); + expect(mounted.router.state.location.pathname).toBe("/kanban"); expect( Object.values(useComposerDraftStore.getState().draftThreadsByThreadId).some( (draft) => draft.worktreePath === "/repo/worktrees/delayed-exact", diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 639f64849..11a4b281d 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -239,6 +239,7 @@ export function useHandleNewThread() { await navigate({ to: "/$threadId", params: { threadId: bootstrapPlan.threadId }, + state: { __scientDraftNavigationToken: ownership.routeToken } as never, ...(navigation?.search ? { search: navigation.search } : {}), }); } catch (error) { @@ -293,6 +294,7 @@ export function useHandleNewThread() { }); const committed = await stageDraftNavigation({ isCurrent: ownership.isCurrent, + ownedRouteToken: ownership.routeToken, // Keep the previous routed draft alive while the destination loads. Replacing the // project's primary slot earlier makes the route guard redirect the old URL to Home. stage: () => { @@ -306,12 +308,15 @@ export function useHandleNewThread() { // Mark the draft-landing navigation as a transition so the new route // subtree renders interruptibly and the browser can paint the composer // skeleton immediately instead of freezing on the synchronous commit. - navigate: () => + navigate: (ownedRouteToken) => new Promise((resolve, reject) => { startTransition(() => { navigate({ to: "/$threadId", params: { threadId }, + ...(ownedRouteToken + ? { state: { __scientDraftNavigationToken: ownedRouteToken } as never } + : {}), ...(navigation?.search ? { search: navigation.search } : {}), }).then(resolve, reject); }); diff --git a/apps/web/src/hooks/useRecentViewSwitcher.ts b/apps/web/src/hooks/useRecentViewSwitcher.ts index 0f636c22b..6a8e96454 100644 --- a/apps/web/src/hooks/useRecentViewSwitcher.ts +++ b/apps/web/src/hooks/useRecentViewSwitcher.ts @@ -22,6 +22,10 @@ import { type RecentViewThreadDraftSummary, } from "../recentViews.logic"; import { resolveRecentThreadSplitActivation } from "../recentViewActivation.logic"; +import { + coordinateExternalRouteNavigation, + draftNavigationSlotKey, +} from "../lib/stagedDraftNavigation"; import { useRecentViewsStore } from "../recentViewsStore"; import { collectLeaves } from "../splitView.logic"; import { useSplitViewStore } from "../splitViewStore"; @@ -255,12 +259,19 @@ export function useRecentViewSwitcher(input: UseRecentViewSwitcherInput) { }, [buildRecentViewAvailability, pruneRecentViewsStore, threadsHydrated]); const activateRecentView = useCallback( - (view: RecentView) => { + async (view: RecentView) => { switch (view.kind) { case "thread": { if (!buildRecentViewAvailability().availableThreadIds.has(view.threadId)) { return; } + const mayActivate = await coordinateExternalRouteNavigation(draftNavigationSlotKey()); + if ( + !mayActivate || + !buildRecentViewAvailability().availableThreadIds.has(view.threadId) + ) { + return; + } prewarmThreadDetail(view.threadId); const splitActivation = resolveRecentThreadSplitActivation({ view, diff --git a/apps/web/src/hooks/useThreadActivationController.test.ts b/apps/web/src/hooks/useThreadActivationController.test.ts index 77b8ccf13..871c941e9 100644 --- a/apps/web/src/hooks/useThreadActivationController.test.ts +++ b/apps/web/src/hooks/useThreadActivationController.test.ts @@ -134,16 +134,44 @@ describe("activateThreadFromSidebarIntent", () => { await Promise.resolve(); const input = makeControllerInput(); - activateThreadFromSidebarIntent(input, THREAD_B); + const activation = activateThreadFromSidebarIntent(input, THREAD_B); releasePreparation(); + await activation; await expect(pending).resolves.toBe(false); expect(pendingOwnerWasCurrent).toBe(false); expect(input.openChatThreadPage).toHaveBeenCalledWith(THREAD_B); await waitForDraftNavigationIdle(draftNavigationSlotKey()); }); - it("focuses a target pane in the active split", () => { + it("waits for a blocking preparation before activating an existing thread", async () => { + let releaseMutation!: () => void; + const pending = runDraftNavigationOnce( + draftNavigationSlotKey(), + "mutating-pr-preparation", + async () => + new Promise((resolve) => { + releaseMutation = resolve; + }), + { blocksFollowingOperations: true }, + ); + await Promise.resolve(); + const input = makeControllerInput(); + + const activation = activateThreadFromSidebarIntent(input, THREAD_B); + await Promise.resolve(); + expect(input.openChatThreadPage).not.toHaveBeenCalled(); + expect(input.navigate).not.toHaveBeenCalled(); + + releaseMutation(); + await activation; + await pending; + + expect(input.openChatThreadPage).toHaveBeenCalledWith(THREAD_B); + expect(input.navigate).toHaveBeenCalledOnce(); + }); + + it("focuses a target pane in the active split", async () => { const activeSplitView = makeSplitViewFixture({ id: "split-active", sourceThreadId: THREAD_A, @@ -157,7 +185,7 @@ describe("activateThreadFromSidebarIntent", () => { selectedThreadCount: 1, }); - activateThreadFromSidebarIntent(input, THREAD_B); + await activateThreadFromSidebarIntent(input, THREAD_B); expect(input.prewarmThreadDetailForIntent).toHaveBeenCalledWith(THREAD_B); expect(input.clearSelection).toHaveBeenCalledOnce(); @@ -176,7 +204,7 @@ describe("activateThreadFromSidebarIntent", () => { expect(input.openChatThreadPage).not.toHaveBeenCalled(); }); - it("exits an active split when the target thread is outside it", () => { + it("exits an active split when the target thread is outside it", async () => { const activeSplitView = makeSplitViewFixture({ id: "split-active", sourceThreadId: THREAD_A, @@ -189,7 +217,7 @@ describe("activateThreadFromSidebarIntent", () => { routeSplitViewId: "split-active", }); - activateThreadFromSidebarIntent(input, THREAD_C); + await activateThreadFromSidebarIntent(input, THREAD_C); expect(input.openChatThreadPage).toHaveBeenCalledWith(THREAD_C); expect(input.setSplitFocusedPane).not.toHaveBeenCalled(); @@ -201,7 +229,7 @@ describe("activateThreadFromSidebarIntent", () => { ); }); - it("restores a persisted split from single-chat mode", () => { + it("restores a persisted split from single-chat mode", async () => { const splitView = makeSplitViewFixture({ id: "split-background", sourceThreadId: THREAD_A, @@ -215,7 +243,7 @@ describe("activateThreadFromSidebarIntent", () => { splitViewsById: { "split-background": splitView }, }); - activateThreadFromSidebarIntent(input, THREAD_B); + await activateThreadFromSidebarIntent(input, THREAD_B); expect(input.setSplitFocusedPane).toHaveBeenCalledWith( "split-background", @@ -231,7 +259,7 @@ describe("activateThreadFromSidebarIntent", () => { }); }); - it("switches between two persisted split pairings without separating them", () => { + it("switches between two persisted split pairings without separating them", async () => { const firstSplit = makeSplitViewFixture({ id: "split-first", sourceThreadId: THREAD_A, @@ -256,7 +284,7 @@ describe("activateThreadFromSidebarIntent", () => { }, }); - activateThreadFromSidebarIntent(input, THREAD_C); + await activateThreadFromSidebarIntent(input, THREAD_C); expect(input.setSplitFocusedPane).toHaveBeenCalledWith( "split-second", @@ -269,7 +297,7 @@ describe("activateThreadFromSidebarIntent", () => { }); }); - it("does nothing when the current route already targets the same split pane", () => { + it("does nothing when the current route already targets the same split pane", async () => { const activeSplitView = makeSplitViewFixture({ id: "split-active", sourceThreadId: THREAD_A, @@ -283,14 +311,14 @@ describe("activateThreadFromSidebarIntent", () => { routeThreadId: THREAD_B, }); - activateThreadFromSidebarIntent(input, THREAD_B); + await activateThreadFromSidebarIntent(input, THREAD_B); expect(input.navigate).not.toHaveBeenCalled(); expect(input.setSplitFocusedPane).not.toHaveBeenCalled(); expect(input.rememberLastThreadRouteNow).not.toHaveBeenCalled(); }); - it("preserves terminal entry point when opening a single thread", () => { + it("preserves terminal entry point when opening a single thread", async () => { const terminalStateByThreadId = { [THREAD_C]: { entryPoint: "terminal" }, } as unknown as ThreadActivationControllerInput["terminalStateByThreadId"]; @@ -299,14 +327,14 @@ describe("activateThreadFromSidebarIntent", () => { terminalStateByThreadId, }); - activateThreadFromSidebarIntent(input, THREAD_C); + await activateThreadFromSidebarIntent(input, THREAD_C); expect(input.openTerminalThreadPage).toHaveBeenCalledWith(THREAD_C); expect(input.openChatThreadPage).not.toHaveBeenCalled(); expect(getFirstNavigateArgs(input).params).toEqual({ threadId: THREAD_C }); }); - it("opens sidechat rows beside their source thread when no persisted split exists", () => { + it("opens sidechat rows beside their source thread when no persisted split exists", async () => { const input = makeControllerInput({ routeThreadId: THREAD_A, sidebarThreadSummaryById: { @@ -315,7 +343,7 @@ describe("activateThreadFromSidebarIntent", () => { }, }); - activateThreadFromSidebarIntent(input, THREAD_B); + await activateThreadFromSidebarIntent(input, THREAD_B); expect(input.openSidechatSplit).toHaveBeenCalledWith({ sourceThreadId: THREAD_A, @@ -333,7 +361,7 @@ describe("activateThreadFromSidebarIntent", () => { }); }); - it("opens the active single sidechat as a split when clicked again", () => { + it("opens the active single sidechat as a split when clicked again", async () => { const input = makeControllerInput({ routeThreadId: THREAD_B, sidebarThreadSummaryById: { @@ -342,7 +370,7 @@ describe("activateThreadFromSidebarIntent", () => { }, }); - activateThreadFromSidebarIntent(input, THREAD_B); + await activateThreadFromSidebarIntent(input, THREAD_B); expect(input.openSidechatSplit).toHaveBeenCalledWith({ sourceThreadId: THREAD_A, diff --git a/apps/web/src/hooks/useThreadActivationController.ts b/apps/web/src/hooks/useThreadActivationController.ts index fcfe5d0e0..767c78e3b 100644 --- a/apps/web/src/hooks/useThreadActivationController.ts +++ b/apps/web/src/hooks/useThreadActivationController.ts @@ -9,7 +9,10 @@ import type { LastThreadRoute } from "../chatRouteRestore"; import { type PaneId, type SplitView, type SplitViewId } from "../splitViewStore"; import { selectThreadTerminalState } from "../terminalStateStore"; import type { SidebarThreadSummary } from "../types"; -import { draftNavigationSlotKey, supersedeDraftNavigation } from "../lib/stagedDraftNavigation"; +import { + coordinateExternalRouteNavigation, + draftNavigationSlotKey, +} from "../lib/stagedDraftNavigation"; import { resolvePreferredSplitForCommand, resolveThreadCommandActivation, @@ -47,10 +50,10 @@ export type ThreadActivationControllerInput = { }; // Runs the complete sidebar activation side-effect chain for one thread intent. -export function activateThreadFromSidebarIntent( +export async function activateThreadFromSidebarIntent( input: ThreadActivationControllerInput, threadId: ThreadId, -): void { +): Promise { const { activeSplitView, clearSelection, @@ -81,8 +84,10 @@ export function activateThreadFromSidebarIntent( return; } // Selecting an existing thread is a route intent on the same visible surface as New Thread. - // Revoke any delayed preparation before deciding whether this activation changes the route. - supersedeDraftNavigation(draftNavigationSlotKey()); + // Revoke delayed preparation immediately, then wait for any non-cancellable Git mutation before + // activating the target or changing its terminal/chat presentation. + const mayActivate = await coordinateExternalRouteNavigation(draftNavigationSlotKey()); + if (!mayActivate) return; const activation = resolveThreadCommandActivation({ threadId, threadExists: true, @@ -247,7 +252,7 @@ export function useThreadActivationController(input: ThreadActivationControllerI const activateThread = useCallback( (threadId: ThreadId) => { - activateThreadFromSidebarIntent( + void activateThreadFromSidebarIntent( { activeSplitView, clearSelection, diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index ed0a5b0be..825ba2266 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { + coordinateExternalRouteNavigation, draftNavigationSlotKey, runDraftNavigationOnce, stageDraftNavigation, - supersedeDraftNavigation, waitForDraftNavigationIdle, } from "./stagedDraftNavigation"; @@ -254,7 +254,7 @@ describe("stagedDraftNavigation", () => { }); await Promise.resolve(); - supersedeDraftNavigation(slotKey); + await coordinateExternalRouteNavigation(slotKey); releasePreparation(); await expect(pending).resolves.toBe("superseded"); @@ -262,6 +262,147 @@ describe("stagedDraftNavigation", () => { await expect(waitForDraftNavigationIdle(slotKey)).resolves.toBeUndefined(); }); + it("revokes immediately but holds an external route behind a non-cancellable mutation", async () => { + let releaseMutation!: () => void; + let mutationOwnerCurrent = true; + const slotKey = draftNavigationSlotKey(); + const mutation = runDraftNavigationOnce( + slotKey, + "mutating-pr-preparation", + async (ownership) => { + await new Promise((resolve) => { + releaseMutation = resolve; + }); + mutationOwnerCurrent = ownership.isCurrent(); + return ownership.isCurrent(); + }, + { blocksFollowingOperations: true }, + ); + await Promise.resolve(); + + let externalRouteReleased = false; + const externalRoute = coordinateExternalRouteNavigation(slotKey).then(() => { + externalRouteReleased = true; + }); + await Promise.resolve(); + + expect(mutationOwnerCurrent).toBe(true); + expect(externalRouteReleased).toBe(false); + releaseMutation(); + + await externalRoute; + await expect(mutation).resolves.toBe(false); + expect(mutationOwnerCurrent).toBe(false); + }); + + it("allows only the latest external route claim after a blocking mutation", async () => { + let releaseMutation!: () => void; + const slotKey = draftNavigationSlotKey(); + const mutation = runDraftNavigationOnce( + slotKey, + "mutating-pr-preparation", + async () => + new Promise((resolve) => { + releaseMutation = resolve; + }), + { blocksFollowingOperations: true }, + ); + await Promise.resolve(); + + const firstRoute = coordinateExternalRouteNavigation(slotKey); + const secondRoute = coordinateExternalRouteNavigation(slotKey); + releaseMutation(); + + await expect(firstRoute).resolves.toBe(false); + await expect(secondRoute).resolves.toBe(true); + await mutation; + }); + + it("allows an owned staged destination through the shared route guard", async () => { + let allowNavigate!: () => void; + let ownerRemainedCurrent = false; + const slotKey = draftNavigationSlotKey(); + const operation = runDraftNavigationOnce(slotKey, "fresh-thread", async (ownership) => { + const committed = await stageDraftNavigation({ + ownedRouteToken: ownership.routeToken, + isCurrent: ownership.isCurrent, + stage: vi.fn(), + navigate: async (ownedRouteToken) => { + await coordinateExternalRouteNavigation(slotKey, ownedRouteToken); + await new Promise((resolve) => { + allowNavigate = resolve; + }); + }, + isDestinationActive: () => true, + finalize: vi.fn(), + rollback: vi.fn(), + }); + ownerRemainedCurrent = ownership.isCurrent(); + return committed; + }); + await Promise.resolve(); + await Promise.resolve(); + allowNavigate(); + + await expect(operation).resolves.toBe(true); + expect(ownerRemainedCurrent).toBe(true); + }); + + it("rejects an unconsumed stale route token while a newer operation owns the surface", async () => { + const slotKey = draftNavigationSlotKey(); + let routeToken = ""; + await runDraftNavigationOnce(slotKey, "fresh-thread", async (ownership) => { + routeToken = ownership.routeToken; + }); + await waitForDraftNavigationIdle(slotKey); + + let releaseNewerOperation!: () => void; + const newerOperation = runDraftNavigationOnce(slotKey, "newer-thread", async () => { + await new Promise((resolve) => { + releaseNewerOperation = resolve; + }); + }); + await Promise.resolve(); + + await expect(coordinateExternalRouteNavigation(slotKey, routeToken)).resolves.toBe(false); + releaseNewerOperation(); + await newerOperation; + }); + + it("treats Back to a previously committed owned route as a newer external intent", async () => { + const slotKey = draftNavigationSlotKey(); + let committedRouteToken = ""; + await runDraftNavigationOnce(slotKey, "first-fresh-thread", async (ownership) => { + committedRouteToken = ownership.routeToken; + await expect(coordinateExternalRouteNavigation(slotKey, ownership.routeToken)).resolves.toBe( + true, + ); + }); + await waitForDraftNavigationIdle(slotKey); + + let releaseExactPreparation!: () => void; + let exactOwnerCurrent = true; + const exactPreparation = runDraftNavigationOnce( + slotKey, + "delayed-exact-workspace", + async (ownership) => { + await new Promise((resolve) => { + releaseExactPreparation = resolve; + }); + exactOwnerCurrent = ownership.isCurrent(); + }, + ); + await Promise.resolve(); + + await expect(coordinateExternalRouteNavigation(slotKey, committedRouteToken)).resolves.toBe( + true, + ); + releaseExactPreparation(); + await exactPreparation; + + expect(exactOwnerCurrent).toBe(false); + }); + it("supersedes delayed work across projects and chat or terminal entry points", async () => { let releaseProjectChat!: () => void; let projectChatWasCurrentAfterRelease = true; diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index 9ed60893e..dc15a4017 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -5,18 +5,31 @@ interface DraftNavigationSlotState { blockingBarrier: Promise | null; + ownedRouteToken: string | null; tail: Promise; latestOperation: Promise | null; latestRequestKey: string | null; latestOwner: symbol | null; + latestRouteClaim: symbol | null; } export interface DraftNavigationOwnership { readonly isCurrent: () => boolean; + readonly routeToken: string; } const draftNavigationStateBySlot = new Map(); const DRAFT_NAVIGATION_SURFACE_KEY = "new-thread-navigation"; +let nextOwnedRouteToken = 0; +const consumedOwnedRouteTokens = new Set(); +const MAX_CONSUMED_OWNED_ROUTE_TOKENS = 128; + +function rememberConsumedOwnedRouteToken(routeToken: string): void { + consumedOwnedRouteTokens.add(routeToken); + if (consumedOwnedRouteTokens.size <= MAX_CONSUMED_OWNED_ROUTE_TOKENS) return; + const oldestRouteToken = consumedOwnedRouteTokens.values().next().value; + if (typeof oldestRouteToken === "string") consumedOwnedRouteTokens.delete(oldestRouteToken); +} /** * Every new-thread action ultimately controls the same visible route. Keep one ownership domain @@ -43,6 +56,49 @@ export function supersedeDraftNavigation(slotKey: string): void { state.latestOwner = Symbol("external-navigation"); state.latestOperation = null; state.latestRequestKey = null; + state.ownedRouteToken = null; + state.latestRouteClaim = null; +} + +/** + * Claims the visible route for navigation outside new-thread creation. Ownership changes + * immediately so stale read-only preparation cannot commit later. A non-cancellable mutation, + * such as preparing a pull-request checkout, remains a barrier and must settle before the newer + * route becomes active. + */ +export async function coordinateExternalRouteNavigation( + slotKey: string, + ownedRouteToken?: string, +): Promise { + const state = draftNavigationStateBySlot.get(slotKey); + if (!state) { + // Persisted history state can outlive the in-memory coordinator across reloads. With no live + // owner there is nothing to supersede, so the route is safe to restore. + return true; + } + if (ownedRouteToken) { + if (state.ownedRouteToken === ownedRouteToken) { + rememberConsumedOwnedRouteToken(ownedRouteToken); + return true; + } + // A token that already committed and is revisited through Back/Forward is an external route + // intent. An unconsumed mismatched token belongs to a stale in-flight navigation and fails + // closed instead of stealing ownership from the newer request. + if (!consumedOwnedRouteTokens.has(ownedRouteToken)) return false; + } + const routeClaim = Symbol("external-route-navigation"); + const blockingBarrier = state.blockingBarrier; + state.latestOwner = routeClaim; + state.latestOperation = null; + state.latestRequestKey = null; + state.ownedRouteToken = null; + state.latestRouteClaim = routeClaim; + await blockingBarrier; + const mayCommit = state.latestRouteClaim === routeClaim && state.latestOwner === routeClaim; + if (mayCommit) { + state.latestRouteClaim = null; + } + return mayCommit; } /** @@ -60,10 +116,12 @@ export function runDraftNavigationOnce( if (!state) { state = { blockingBarrier: null, + ownedRouteToken: null, tail: Promise.resolve(), latestOperation: null, latestRequestKey: null, latestOwner: null, + latestRouteClaim: null, }; draftNavigationStateBySlot.set(slotKey, state); } @@ -73,10 +131,14 @@ export function runDraftNavigationOnce( } const owner = Symbol(requestKey); + const routeToken = `draft-route-${(nextOwnedRouteToken += 1)}`; const ownership: DraftNavigationOwnership = { isCurrent: () => state.latestOwner === owner, + routeToken, }; state.latestOwner = owner; + state.ownedRouteToken = routeToken; + state.latestRouteClaim = null; const priorBlockingBarrier = state.blockingBarrier; const execution = priorBlockingBarrier ? priorBlockingBarrier.then(() => run(ownership)) @@ -87,6 +149,7 @@ export function runDraftNavigationOnce( state.latestOperation = null; state.latestRequestKey = null; state.latestOwner = null; + state.ownedRouteToken = null; } }; operation = execution.then( @@ -124,7 +187,8 @@ export function runDraftNavigationOnce( if ( draftNavigationStateBySlot.get(slotKey) === state && state.tail === tail && - state.latestOperation === null + state.latestOperation === null && + state.latestRouteClaim === null ) { draftNavigationStateBySlot.delete(slotKey); } @@ -137,9 +201,10 @@ export function runDraftNavigationOnce( * rolls the staged draft back without treating the user's newer navigation as an error. */ export async function stageDraftNavigation(input: { + readonly ownedRouteToken?: string; readonly isCurrent: () => boolean; readonly stage: () => void; - readonly navigate: () => Promise; + readonly navigate: (ownedRouteToken?: string) => Promise; readonly isDestinationActive: () => boolean; readonly finalize: () => void; readonly rollback: () => void; @@ -158,7 +223,7 @@ export async function stageDraftNavigation(input: { return false; } input.stage(); - await input.navigate(); + await input.navigate(input.ownedRouteToken); if (!input.isCurrent() || !input.isDestinationActive()) { rollbackOnce(); return false; diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index 7075a93f3..c037e66c6 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -51,6 +51,7 @@ export interface NewThreadOptions { export interface NewThreadNavigationOwnership { readonly isCurrent: () => boolean; + readonly routeToken: string; } export type NewThreadPreparationResult = diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx index 0f7639d1f..43e1a8c06 100644 --- a/apps/web/src/routes/__root.tsx +++ b/apps/web/src/routes/__root.tsx @@ -92,6 +92,10 @@ import { PROVIDER_UPDATE_REFRESH_INTERVAL_MS, } from "../providerUpdates"; import { activityManager } from "../notifications/activityStore"; +import { + coordinateExternalRouteNavigation, + draftNavigationSlotKey, +} from "../lib/stagedDraftNavigation"; import { getGitInvalidationThreadIdForEvent, getProjectFileInvalidationThreadIdForEvent, @@ -136,6 +140,19 @@ function reconcilePromotedDraftFromThreadDetail(thread: OrchestrationThread): vo export const Route = createRootRouteWithContext<{ queryClient: QueryClient; }>()({ + beforeLoad: async ({ abortController, location, preload }) => { + if (preload) return; + const ownedRouteToken = ( + location.state as unknown as { readonly __scientDraftNavigationToken?: unknown } + ).__scientDraftNavigationToken; + const mayCommit = await coordinateExternalRouteNavigation( + draftNavigationSlotKey(), + typeof ownedRouteToken === "string" ? ownedRouteToken : undefined, + ); + if (!mayCommit) { + abortController.abort(); + } + }, component: RootRouteView, errorComponent: RootRouteErrorView, head: () => ({ From c2832a4ac7cf6c1ba14e9ac5d7740317c4e637b8 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 23:13:14 +0300 Subject: [PATCH 12/14] fix(web): enforce route ownership before history commits --- apps/web/src/hooks/useHandleNewThread.ts | 23 +++- .../src/lib/stagedDraftNavigation.browser.ts | 100 ++++++++++++++++ .../web/src/lib/stagedDraftNavigation.test.ts | 108 +++++++++++++++++- apps/web/src/lib/stagedDraftNavigation.ts | 44 +++---- apps/web/src/lib/startContainerChat.test.ts | 24 ++++ apps/web/src/lib/startContainerChat.ts | 8 +- apps/web/src/router.ts | 21 +++- apps/web/src/routes/__root.tsx | 17 --- 8 files changed, 284 insertions(+), 61 deletions(-) create mode 100644 apps/web/src/lib/stagedDraftNavigation.browser.ts diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 11a4b281d..516af9bb8 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -28,6 +28,7 @@ import { } from "../lib/threadBootstrap"; import { promoteThreadCreate } from "../lib/threadCreatePromotion"; import { + coordinateExternalRouteNavigation, draftNavigationSlotKey, runDraftNavigationOnce, stageDraftNavigation, @@ -236,7 +237,15 @@ export function useHandleNewThread() { currentFocusedThreadId !== bootstrapPlan.threadId ) { try { + const mayCommit = await coordinateExternalRouteNavigation( + draftNavigationSlotKey(), + ownership.routeToken, + ); + if (!mayCommit || !ownership.isCurrent()) { + return null; + } await navigate({ + ignoreBlocker: true, to: "/$threadId", params: { threadId: bootstrapPlan.threadId }, state: { __scientDraftNavigationToken: ownership.routeToken } as never, @@ -308,10 +317,17 @@ export function useHandleNewThread() { // Mark the draft-landing navigation as a transition so the new route // subtree renders interruptibly and the browser can paint the composer // skeleton immediately instead of freezing on the synchronous commit. - navigate: (ownedRouteToken) => - new Promise((resolve, reject) => { + navigate: async (ownedRouteToken) => { + if (!ownedRouteToken || !ownership.isCurrent()) return; + const mayCommit = await coordinateExternalRouteNavigation( + draftNavigationSlotKey(), + ownedRouteToken, + ); + if (!mayCommit || !ownership.isCurrent()) return; + return new Promise((resolve, reject) => { startTransition(() => { navigate({ + ignoreBlocker: true, to: "/$threadId", params: { threadId }, ...(ownedRouteToken @@ -320,7 +336,8 @@ export function useHandleNewThread() { ...(navigation?.search ? { search: navigation.search } : {}), }).then(resolve, reject); }); - }), + }); + }, // TanStack resolves an older navigate() promise when a newer navigation supersedes it. // Verify the committed route before deleting the previous project draft. isDestinationActive: () => router.state.location.pathname === `/${threadId}`, diff --git a/apps/web/src/lib/stagedDraftNavigation.browser.ts b/apps/web/src/lib/stagedDraftNavigation.browser.ts new file mode 100644 index 000000000..1f4bb1c6d --- /dev/null +++ b/apps/web/src/lib/stagedDraftNavigation.browser.ts @@ -0,0 +1,100 @@ +import { createMemoryHistory } from "@tanstack/react-router"; +import { afterEach, describe, expect, it } from "vitest"; + +import { getRouter, type AppRouter } from "../router"; +import { + draftNavigationSlotKey, + runDraftNavigationOnce, + waitForDraftNavigationIdle, +} from "./stagedDraftNavigation"; + +describe("draft navigation route guard", () => { + let router: AppRouter | null = null; + let unsubscribeHistory: (() => void) | null = null; + + afterEach(async () => { + unsubscribeHistory?.(); + unsubscribeHistory = null; + if (router) { + router.history.destroy(); + router = null; + } + await waitForDraftNavigationIdle(draftNavigationSlotKey()); + }); + + it("keeps the committed destination when a stale owned route tries to navigate", async () => { + router = getRouter(createMemoryHistory({ initialEntries: ["/activity"] })); + unsubscribeHistory = router.history.subscribe(() => void router?.load()); + await router.load(); + expect(router.state.location.href).toBe("/activity"); + + const slotKey = draftNavigationSlotKey(); + let staleRouteToken = ""; + let releaseStaleOperation!: () => void; + const staleOperation = runDraftNavigationOnce( + slotKey, + "stale-owned-route", + async (ownership) => { + staleRouteToken = ownership.routeToken; + await new Promise((resolve) => { + releaseStaleOperation = resolve; + }); + }, + ); + await Promise.resolve(); + + let releaseCurrentOperation!: () => void; + let currentOwnerStayedCurrent = false; + const currentOperation = runDraftNavigationOnce( + slotKey, + "current-owned-route", + async (ownership) => { + await new Promise((resolve) => { + releaseCurrentOperation = resolve; + }); + currentOwnerStayedCurrent = ownership.isCurrent(); + }, + ); + await Promise.resolve(); + + void router.navigate({ + to: "/settings", + state: { __scientDraftNavigationToken: staleRouteToken } as never, + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(router.state.location.href).toBe("/activity"); + releaseStaleOperation(); + releaseCurrentOperation(); + await Promise.all([staleOperation, currentOperation]); + expect(currentOwnerStayedCurrent).toBe(true); + }); + + it("commits only the latest external destination after a blocking mutation", async () => { + router = getRouter(createMemoryHistory({ initialEntries: ["/"] })); + unsubscribeHistory = router.history.subscribe(() => void router?.load()); + await router.load(); + + const slotKey = draftNavigationSlotKey(); + let releaseMutation!: () => void; + const mutation = runDraftNavigationOnce( + slotKey, + "blocking-pr-preparation", + async () => + new Promise((resolve) => { + releaseMutation = resolve; + }), + { blocksFollowingOperations: true }, + ); + await Promise.resolve(); + + void router.navigate({ to: "/settings" }); + await Promise.resolve(); + void router.navigate({ to: "/activity" }); + await Promise.resolve(); + releaseMutation(); + await mutation; + + await expect.poll(() => router?.state.location.href, { timeout: 10_000 }).toBe("/activity"); + }); +}); diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index 825ba2266..404171d53 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -350,11 +350,15 @@ describe("stagedDraftNavigation", () => { it("rejects an unconsumed stale route token while a newer operation owns the surface", async () => { const slotKey = draftNavigationSlotKey(); - let routeToken = ""; - await runDraftNavigationOnce(slotKey, "fresh-thread", async (ownership) => { - routeToken = ownership.routeToken; + let staleRouteToken = ""; + let releaseStaleOperation!: () => void; + const staleOperation = runDraftNavigationOnce(slotKey, "stale-thread", async (ownership) => { + staleRouteToken = ownership.routeToken; + await new Promise((resolve) => { + releaseStaleOperation = resolve; + }); }); - await waitForDraftNavigationIdle(slotKey); + await Promise.resolve(); let releaseNewerOperation!: () => void; const newerOperation = runDraftNavigationOnce(slotKey, "newer-thread", async () => { @@ -364,8 +368,10 @@ describe("stagedDraftNavigation", () => { }); await Promise.resolve(); - await expect(coordinateExternalRouteNavigation(slotKey, routeToken)).resolves.toBe(false); + await expect(coordinateExternalRouteNavigation(slotKey, staleRouteToken)).resolves.toBe(false); + releaseStaleOperation(); releaseNewerOperation(); + await staleOperation; await newerOperation; }); @@ -403,6 +409,98 @@ describe("stagedDraftNavigation", () => { expect(exactOwnerCurrent).toBe(false); }); + it("treats a persisted owned route from an earlier renderer session as external", async () => { + const randomUuid = vi + .spyOn(crypto, "randomUUID") + .mockReturnValueOnce("11111111-1111-4111-8111-111111111111") + .mockReturnValueOnce("22222222-2222-4222-8222-222222222222"); + + try { + vi.resetModules(); + const earlierSession = await import("./stagedDraftNavigation"); + const earlierSlotKey = earlierSession.draftNavigationSlotKey(); + let persistedRouteToken = ""; + await earlierSession.runDraftNavigationOnce( + earlierSlotKey, + "earlier-session-thread", + async (ownership) => { + persistedRouteToken = ownership.routeToken; + await expect( + earlierSession.coordinateExternalRouteNavigation(earlierSlotKey, ownership.routeToken), + ).resolves.toBe(true); + }, + ); + await earlierSession.waitForDraftNavigationIdle(earlierSlotKey); + + vi.resetModules(); + const reloadedSession = await import("./stagedDraftNavigation"); + const reloadedSlotKey = reloadedSession.draftNavigationSlotKey(); + let releaseFreshPreparation!: () => void; + let freshOwnerCurrent = true; + let freshRouteToken = ""; + const freshPreparation = reloadedSession.runDraftNavigationOnce( + reloadedSlotKey, + "fresh-session-thread", + async (ownership) => { + freshRouteToken = ownership.routeToken; + await new Promise((resolve) => { + releaseFreshPreparation = resolve; + }); + freshOwnerCurrent = ownership.isCurrent(); + }, + ); + await Promise.resolve(); + + expect(freshRouteToken).not.toBe(persistedRouteToken); + await expect( + reloadedSession.coordinateExternalRouteNavigation(reloadedSlotKey, persistedRouteToken), + ).resolves.toBe(true); + releaseFreshPreparation(); + await freshPreparation; + + expect(freshOwnerCurrent).toBe(false); + } finally { + randomUuid.mockRestore(); + vi.resetModules(); + } + }); + + it("keeps old committed history external after more than 128 owned navigations", async () => { + const slotKey = draftNavigationSlotKey(); + let oldestCommittedRouteToken = ""; + for (let index = 0; index < 129; index += 1) { + await runDraftNavigationOnce(slotKey, `committed-thread-${index}`, async (ownership) => { + if (index === 0) oldestCommittedRouteToken = ownership.routeToken; + await expect( + coordinateExternalRouteNavigation(slotKey, ownership.routeToken), + ).resolves.toBe(true); + }); + } + await waitForDraftNavigationIdle(slotKey); + + let releaseFreshPreparation!: () => void; + let freshOwnerCurrent = true; + const freshPreparation = runDraftNavigationOnce( + slotKey, + "fresh-after-long-history", + async (ownership) => { + await new Promise((resolve) => { + releaseFreshPreparation = resolve; + }); + freshOwnerCurrent = ownership.isCurrent(); + }, + ); + await Promise.resolve(); + + await expect( + coordinateExternalRouteNavigation(slotKey, oldestCommittedRouteToken), + ).resolves.toBe(true); + releaseFreshPreparation(); + await freshPreparation; + + expect(freshOwnerCurrent).toBe(false); + }); + it("supersedes delayed work across projects and chat or terminal entry points", async () => { let releaseProjectChat!: () => void; let projectChatWasCurrentAfterRelease = true; diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index dc15a4017..9d6d54f20 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -3,6 +3,8 @@ // staged drafts only after their destination route actually commits. // Layer: Web navigation orchestration +import { randomUUID } from "./utils"; + interface DraftNavigationSlotState { blockingBarrier: Promise | null; ownedRouteToken: string | null; @@ -20,16 +22,9 @@ export interface DraftNavigationOwnership { const draftNavigationStateBySlot = new Map(); const DRAFT_NAVIGATION_SURFACE_KEY = "new-thread-navigation"; +const DRAFT_ROUTE_SESSION_PREFIX = `draft-route:${randomUUID()}:`; let nextOwnedRouteToken = 0; -const consumedOwnedRouteTokens = new Set(); -const MAX_CONSUMED_OWNED_ROUTE_TOKENS = 128; - -function rememberConsumedOwnedRouteToken(routeToken: string): void { - consumedOwnedRouteTokens.add(routeToken); - if (consumedOwnedRouteTokens.size <= MAX_CONSUMED_OWNED_ROUTE_TOKENS) return; - const oldestRouteToken = consumedOwnedRouteTokens.values().next().value; - if (typeof oldestRouteToken === "string") consumedOwnedRouteTokens.delete(oldestRouteToken); -} +const pendingOwnedRouteTokens = new Set(); /** * Every new-thread action ultimately controls the same visible route. Keep one ownership domain @@ -44,22 +39,6 @@ export function waitForDraftNavigationIdle(slotKey: string): Promise { return draftNavigationStateBySlot.get(slotKey)?.tail ?? Promise.resolve(); } -/** - * Transfers visible-route ownership to a navigation that is not creating a new thread. Pending - * preparations may finish their read-only work, but they can no longer stage or commit a route. - */ -export function supersedeDraftNavigation(slotKey: string): void { - const state = draftNavigationStateBySlot.get(slotKey); - if (!state) { - return; - } - state.latestOwner = Symbol("external-navigation"); - state.latestOperation = null; - state.latestRequestKey = null; - state.ownedRouteToken = null; - state.latestRouteClaim = null; -} - /** * Claims the visible route for navigation outside new-thread creation. Ownership changes * immediately so stale read-only preparation cannot commit later. A non-cancellable mutation, @@ -78,13 +57,13 @@ export async function coordinateExternalRouteNavigation( } if (ownedRouteToken) { if (state.ownedRouteToken === ownedRouteToken) { - rememberConsumedOwnedRouteToken(ownedRouteToken); return true; } - // A token that already committed and is revisited through Back/Forward is an external route - // intent. An unconsumed mismatched token belongs to a stale in-flight navigation and fails - // closed instead of stealing ownership from the newer request. - if (!consumedOwnedRouteTokens.has(ownedRouteToken)) return false; + // A token still owned by another in-flight request is stale and fails closed. Once its request + // settles, the token becomes ordinary browser history; revisiting it through Back/Forward is a + // new external intent that must supersede current work. This survives renderer reloads and an + // unbounded history because only live requests need to be retained. + if (pendingOwnedRouteTokens.has(ownedRouteToken)) return false; } const routeClaim = Symbol("external-route-navigation"); const blockingBarrier = state.blockingBarrier; @@ -131,7 +110,8 @@ export function runDraftNavigationOnce( } const owner = Symbol(requestKey); - const routeToken = `draft-route-${(nextOwnedRouteToken += 1)}`; + const routeToken = `${DRAFT_ROUTE_SESSION_PREFIX}${(nextOwnedRouteToken += 1)}`; + pendingOwnedRouteTokens.add(routeToken); const ownership: DraftNavigationOwnership = { isCurrent: () => state.latestOwner === owner, routeToken, @@ -154,10 +134,12 @@ export function runDraftNavigationOnce( }; operation = execution.then( (value) => { + pendingOwnedRouteTokens.delete(routeToken); clearLatestRequest(); return value; }, (error: unknown) => { + pendingOwnedRouteTokens.delete(routeToken); clearLatestRequest(); throw error; }, diff --git a/apps/web/src/lib/startContainerChat.test.ts b/apps/web/src/lib/startContainerChat.test.ts index 7d74bc9d1..065e1e1d2 100644 --- a/apps/web/src/lib/startContainerChat.test.ts +++ b/apps/web/src/lib/startContainerChat.test.ts @@ -79,6 +79,30 @@ describe("startFreshChatForActiveSurface", () => { }); describe("startContainerChat", () => { + it("keeps reuse-eligible container chats in the local container workspace", async () => { + const projectId = ProjectId.makeUnsafe("project-container"); + const threadId = ThreadId.makeUnsafe("thread-container"); + const handleNewThread = vi.fn(async () => threadId); + + await expect( + startContainerChat({ + ensureProjectId: async () => projectId, + handleNewThread, + navigationTargetKey: "studio-chat", + errorLabel: "failed", + }), + ).resolves.toEqual({ ok: true, threadId }); + expect(handleNewThread).toHaveBeenCalledWith( + projectId, + { workspace: { kind: "local-container" } }, + undefined, + expect.objectContaining({ + isCurrent: expect.any(Function), + routeToken: expect.any(String), + }), + ); + }); + it("returns the created thread so callers can attach context deterministically", async () => { const projectId = ProjectId.makeUnsafe("project-1"); const threadId = ThreadId.makeUnsafe("thread-1"); diff --git a/apps/web/src/lib/startContainerChat.ts b/apps/web/src/lib/startContainerChat.ts index d5ed6e802..aa148c53b 100644 --- a/apps/web/src/lib/startContainerChat.ts +++ b/apps/web/src/lib/startContainerChat.ts @@ -65,10 +65,10 @@ export async function startContainerChat(input: { if (!projectId) { return { ok: false, error: input.errorLabel }; } - const threadOptions: NewThreadOptions | undefined = - input.fresh === true - ? { fresh: true, workspace: { kind: "local-container" } } - : undefined; + const threadOptions: NewThreadOptions = { + ...(input.fresh === true ? { fresh: true } : {}), + workspace: { kind: "local-container" }, + }; const threadId = await input.handleNewThread( projectId, threadOptions, diff --git a/apps/web/src/router.ts b/apps/web/src/router.ts index a9d3704cc..080d1d2c4 100644 --- a/apps/web/src/router.ts +++ b/apps/web/src/router.ts @@ -4,13 +4,31 @@ import { createRouter } from "@tanstack/react-router"; import { routeTree } from "./routeTree.gen"; import { StoreProvider } from "./store"; +import { + coordinateExternalRouteNavigation, + draftNavigationSlotKey, +} from "./lib/stagedDraftNavigation"; type RouterHistory = NonNullable[0]["history"]>; export function getRouter(history: RouterHistory) { const queryClient = new QueryClient(); - return createRouter({ + history.block({ + enableBeforeUnload: false, + blockerFn: async ({ nextLocation }) => { + const ownedRouteToken = ( + nextLocation.state as unknown as { readonly __scientDraftNavigationToken?: unknown } + ).__scientDraftNavigationToken; + const mayCommit = await coordinateExternalRouteNavigation( + draftNavigationSlotKey(), + typeof ownedRouteToken === "string" ? ownedRouteToken : undefined, + ); + return !mayCommit; + }, + }); + + const router = createRouter({ routeTree, history, // Routes are auto-code-split and have no loaders, so intent preloading only @@ -27,6 +45,7 @@ export function getRouter(history: RouterHistory) { createElement(StoreProvider, null, children), ), }); + return router; } export type AppRouter = ReturnType; diff --git a/apps/web/src/routes/__root.tsx b/apps/web/src/routes/__root.tsx index 43e1a8c06..0f7639d1f 100644 --- a/apps/web/src/routes/__root.tsx +++ b/apps/web/src/routes/__root.tsx @@ -92,10 +92,6 @@ import { PROVIDER_UPDATE_REFRESH_INTERVAL_MS, } from "../providerUpdates"; import { activityManager } from "../notifications/activityStore"; -import { - coordinateExternalRouteNavigation, - draftNavigationSlotKey, -} from "../lib/stagedDraftNavigation"; import { getGitInvalidationThreadIdForEvent, getProjectFileInvalidationThreadIdForEvent, @@ -140,19 +136,6 @@ function reconcilePromotedDraftFromThreadDetail(thread: OrchestrationThread): vo export const Route = createRootRouteWithContext<{ queryClient: QueryClient; }>()({ - beforeLoad: async ({ abortController, location, preload }) => { - if (preload) return; - const ownedRouteToken = ( - location.state as unknown as { readonly __scientDraftNavigationToken?: unknown } - ).__scientDraftNavigationToken; - const mayCommit = await coordinateExternalRouteNavigation( - draftNavigationSlotKey(), - typeof ownedRouteToken === "string" ? ownedRouteToken : undefined, - ); - if (!mayCommit) { - abortController.abort(); - } - }, component: RootRouteView, errorComponent: RootRouteErrorView, head: () => ({ From a2ff780fe1ee4856c77cee3101c862b5b596ce97 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 23:18:47 +0300 Subject: [PATCH 13/14] test(web): use typed routes in navigation guard proof --- apps/web/src/lib/stagedDraftNavigation.browser.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/web/src/lib/stagedDraftNavigation.browser.ts b/apps/web/src/lib/stagedDraftNavigation.browser.ts index 1f4bb1c6d..80f734afd 100644 --- a/apps/web/src/lib/stagedDraftNavigation.browser.ts +++ b/apps/web/src/lib/stagedDraftNavigation.browser.ts @@ -23,10 +23,10 @@ describe("draft navigation route guard", () => { }); it("keeps the committed destination when a stale owned route tries to navigate", async () => { - router = getRouter(createMemoryHistory({ initialEntries: ["/activity"] })); + router = getRouter(createMemoryHistory({ initialEntries: ["/plugins"] })); unsubscribeHistory = router.history.subscribe(() => void router?.load()); await router.load(); - expect(router.state.location.href).toBe("/activity"); + expect(router.state.location.href).toBe("/plugins"); const slotKey = draftNavigationSlotKey(); let staleRouteToken = ""; @@ -63,7 +63,7 @@ describe("draft navigation route guard", () => { }); await new Promise((resolve) => setTimeout(resolve, 0)); - expect(router.state.location.href).toBe("/activity"); + expect(router.state.location.href).toBe("/plugins"); releaseStaleOperation(); releaseCurrentOperation(); await Promise.all([staleOperation, currentOperation]); @@ -88,13 +88,13 @@ describe("draft navigation route guard", () => { ); await Promise.resolve(); - void router.navigate({ to: "/settings" }); + void router.navigate({ to: "/plugins" }); await Promise.resolve(); - void router.navigate({ to: "/activity" }); + void router.navigate({ to: "/settings" }); await Promise.resolve(); releaseMutation(); await mutation; - await expect.poll(() => router?.state.location.href, { timeout: 10_000 }).toBe("/activity"); + await expect.poll(() => router?.state.location.href, { timeout: 10_000 }).toBe("/settings"); }); }); From 71da7ee2dd69dc4fef7d942f30fb6895f8b5946a Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 23:35:41 +0300 Subject: [PATCH 14/14] fix(web): release committed navigation ownership --- apps/web/src/hooks/useHandleNewThread.ts | 2 ++ .../src/lib/stagedDraftNavigation.browser.ts | 33 +++++++++++++++++++ .../web/src/lib/stagedDraftNavigation.test.ts | 29 ++++++++++++++++ apps/web/src/lib/stagedDraftNavigation.ts | 19 ++++++++--- apps/web/src/lib/threadBootstrap.ts | 2 ++ 5 files changed, 80 insertions(+), 5 deletions(-) diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index 516af9bb8..32546aafb 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -263,6 +263,7 @@ export function useHandleNewThread() { ) { return null; } + ownership.markRouteCommitted(); } if (!ownership.isCurrent()) { return null; @@ -358,6 +359,7 @@ export function useHandleNewThread() { if (!committed) { return null; } + ownership.markRouteCommitted(); if (entryPoint === "terminal" && ownership.isCurrent()) { await createTerminalThread( threadId, diff --git a/apps/web/src/lib/stagedDraftNavigation.browser.ts b/apps/web/src/lib/stagedDraftNavigation.browser.ts index 80f734afd..bf0601c61 100644 --- a/apps/web/src/lib/stagedDraftNavigation.browser.ts +++ b/apps/web/src/lib/stagedDraftNavigation.browser.ts @@ -97,4 +97,37 @@ describe("draft navigation route guard", () => { await expect.poll(() => router?.state.location.href, { timeout: 10_000 }).toBe("/settings"); }); + + it("restores a committed terminal destination while native promotion is still pending", async () => { + router = getRouter(createMemoryHistory({ initialEntries: ["/plugins"] })); + unsubscribeHistory = router.history.subscribe(() => void router?.load()); + await router.load(); + + const slotKey = draftNavigationSlotKey(); + let releaseTerminalPromotion!: () => void; + const terminalNavigation = runDraftNavigationOnce( + slotKey, + "terminal-first-thread", + async (ownership) => { + await router?.navigate({ + to: "/settings", + state: { __scientDraftNavigationToken: ownership.routeToken } as never, + }); + expect(router?.state.location.href).toBe("/settings"); + ownership.markRouteCommitted(); + await new Promise((resolve) => { + releaseTerminalPromotion = resolve; + }); + }, + ); + await expect.poll(() => router?.state.location.href).toBe("/settings"); + + await router.navigate({ to: "/plugins" }); + expect(router.state.location.href).toBe("/plugins"); + router.history.back(); + await expect.poll(() => router?.state.location.href).toBe("/settings"); + + releaseTerminalPromotion(); + await terminalNavigation; + }); }); diff --git a/apps/web/src/lib/stagedDraftNavigation.test.ts b/apps/web/src/lib/stagedDraftNavigation.test.ts index 404171d53..20b66e547 100644 --- a/apps/web/src/lib/stagedDraftNavigation.test.ts +++ b/apps/web/src/lib/stagedDraftNavigation.test.ts @@ -409,6 +409,35 @@ describe("stagedDraftNavigation", () => { expect(exactOwnerCurrent).toBe(false); }); + it("allows Back to a committed terminal route while native promotion is still pending", async () => { + const slotKey = draftNavigationSlotKey(); + let committedTerminalRouteToken = ""; + let releaseTerminalPromotion!: () => void; + const terminalNavigation = runDraftNavigationOnce( + slotKey, + "terminal-first-thread", + async (ownership) => { + committedTerminalRouteToken = ownership.routeToken; + await expect( + coordinateExternalRouteNavigation(slotKey, ownership.routeToken), + ).resolves.toBe(true); + ownership.markRouteCommitted(); + await new Promise((resolve) => { + releaseTerminalPromotion = resolve; + }); + }, + ); + await Promise.resolve(); + + await expect(coordinateExternalRouteNavigation(slotKey)).resolves.toBe(true); + await expect( + coordinateExternalRouteNavigation(slotKey, committedTerminalRouteToken), + ).resolves.toBe(true); + + releaseTerminalPromotion(); + await terminalNavigation; + }); + it("treats a persisted owned route from an earlier renderer session as external", async () => { const randomUuid = vi .spyOn(crypto, "randomUUID") diff --git a/apps/web/src/lib/stagedDraftNavigation.ts b/apps/web/src/lib/stagedDraftNavigation.ts index 9d6d54f20..bb82cd657 100644 --- a/apps/web/src/lib/stagedDraftNavigation.ts +++ b/apps/web/src/lib/stagedDraftNavigation.ts @@ -17,6 +17,7 @@ interface DraftNavigationSlotState { export interface DraftNavigationOwnership { readonly isCurrent: () => boolean; + readonly markRouteCommitted: () => void; readonly routeToken: string; } @@ -24,7 +25,9 @@ const draftNavigationStateBySlot = new Map(); const DRAFT_NAVIGATION_SURFACE_KEY = "new-thread-navigation"; const DRAFT_ROUTE_SESSION_PREFIX = `draft-route:${randomUUID()}:`; let nextOwnedRouteToken = 0; -const pendingOwnedRouteTokens = new Set(); +// Route ownership ends when the destination commits, even if post-navigation work (such as +// native terminal promotion) keeps the surrounding operation alive. +const uncommittedOwnedRouteTokens = new Set(); /** * Every new-thread action ultimately controls the same visible route. Keep one ownership domain @@ -63,7 +66,7 @@ export async function coordinateExternalRouteNavigation( // settles, the token becomes ordinary browser history; revisiting it through Back/Forward is a // new external intent that must supersede current work. This survives renderer reloads and an // unbounded history because only live requests need to be retained. - if (pendingOwnedRouteTokens.has(ownedRouteToken)) return false; + if (uncommittedOwnedRouteTokens.has(ownedRouteToken)) return false; } const routeClaim = Symbol("external-route-navigation"); const blockingBarrier = state.blockingBarrier; @@ -111,9 +114,15 @@ export function runDraftNavigationOnce( const owner = Symbol(requestKey); const routeToken = `${DRAFT_ROUTE_SESSION_PREFIX}${(nextOwnedRouteToken += 1)}`; - pendingOwnedRouteTokens.add(routeToken); + uncommittedOwnedRouteTokens.add(routeToken); const ownership: DraftNavigationOwnership = { isCurrent: () => state.latestOwner === owner, + markRouteCommitted: () => { + uncommittedOwnedRouteTokens.delete(routeToken); + if (state.ownedRouteToken === routeToken) { + state.ownedRouteToken = null; + } + }, routeToken, }; state.latestOwner = owner; @@ -134,12 +143,12 @@ export function runDraftNavigationOnce( }; operation = execution.then( (value) => { - pendingOwnedRouteTokens.delete(routeToken); + uncommittedOwnedRouteTokens.delete(routeToken); clearLatestRequest(); return value; }, (error: unknown) => { - pendingOwnedRouteTokens.delete(routeToken); + uncommittedOwnedRouteTokens.delete(routeToken); clearLatestRequest(); throw error; }, diff --git a/apps/web/src/lib/threadBootstrap.ts b/apps/web/src/lib/threadBootstrap.ts index c037e66c6..84842c56a 100644 --- a/apps/web/src/lib/threadBootstrap.ts +++ b/apps/web/src/lib/threadBootstrap.ts @@ -51,6 +51,8 @@ export interface NewThreadOptions { export interface NewThreadNavigationOwnership { readonly isCurrent: () => boolean; + /** Marks the owned destination as committed while any post-navigation work may continue. */ + readonly markRouteCommitted: () => void; readonly routeToken: string; }