From 72b2706c24c37702d5f61c621bfc419d7d55761a Mon Sep 17 00:00:00 2001 From: extoci Date: Tue, 3 Nov 2026 22:49:32 +0000 Subject: [PATCH 1/2] fix(web): reuse checked-out threads for PR fixes --- .../BranchToolbarBranchSelector.tsx | 5 ++ .../chat/ThreadDetailsPrRow.browser.test.tsx | 60 ++++++++++++++++- .../components/chat/ThreadDetailsPrRow.tsx | 25 +++++-- .../components/chat/ThreadDetailsPrRows.tsx | 4 ++ .../pullRequest/usePullRequestActions.ts | 67 +++++++++++++------ 5 files changed, 132 insertions(+), 29 deletions(-) diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 9782f258afe19..fd9088f139fd7 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -738,6 +738,11 @@ export function BranchToolbarBranchSelector({ {displayMode === "panel" && prNumber !== undefined && prUrl !== undefined ? ( ({ status: "success" as PullRequestCheck["status"], + conflicting: false, extraStatus: null as PullRequestCheck["status"] | null, mergeMethods: ["merge"] as Array<"merge" | "squash" | "rebase">, perform: vi.fn(), @@ -34,7 +35,7 @@ const queryData = () => ({ title: "Test PR", state: "open", isDraft: false, - mergeability: "mergeable", + mergeability: state.conflicting ? "conflicting" : "mergeable", headBranch: "feature", baseBranch: "main", changedFiles: 1, @@ -177,6 +178,55 @@ it("requires a new merge click after passing checks become pending and pass agai expect(state.perform).not.toHaveBeenCalled(); }); +it.each([ + { matches: true, conflicting: false, draft: false }, + { matches: false, conflicting: false, draft: false }, + { matches: true, conflicting: true, draft: false }, + { matches: true, conflicting: false, draft: true }, +])("uses the matching checkout for PR tasks: %j", async ({ matches, conflicting, draft }) => { + await using screen = await setup(); + state.status = "failure"; + state.conflicting = conflicting; + const threadRef = { + environmentId: EnvironmentId.make("environment"), + threadId: ThreadId.make("existing-thread"), + }; + const target = draft ? DraftId.make("existing-draft") : threadRef; + useComposerDraftStore.getState().setPrompt(target, "Keep my notes"); + await screen.rerender( + , + ); + await screen.getByRole("button", { name: conflicting ? "Resolve" : "Fix", exact: true }).click(); + if (matches) { + await vi.waitFor(() => { + expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toContain( + "https://github.com/owner/repo/pull/1", + ); + }); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toContain( + "Keep my notes", + ); + expect(state.newThread).not.toHaveBeenCalled(); + expect(state.prepareThread).not.toHaveBeenCalled(); + } else { + await vi.waitFor(() => expect(state.prepareThread).toHaveBeenCalled()); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toBe("Keep my notes"); + } +}); + async function setup() { state.perform.mockReset(); state.checkoutError = null; @@ -184,11 +234,15 @@ async function setup() { draftId: DraftId.make("new-draft"), threadId: ThreadId.make("new-thread"), }); - state.prepareThread.mockReset(); + state.prepareThread.mockReset().mockResolvedValue({ + _tag: "Success", + value: { branch: "feature", worktreePath: "/workspace/pr", isOnPullRequestHead: true }, + }); state.addToast.mockReset().mockReturnValue("checkout-toast"); state.updateToast.mockReset(); state.status = "success"; + state.conflicting = false; state.extraStatus = null; state.mergeMethods = ["merge"]; useUiStateStore.getState().setPullRequestMergeMethod("merge"); diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index 8c3f814427991..d6d6239b1c8b1 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -53,6 +53,7 @@ import { usePullRequestActionRunner, usePullRequestDefaultMergeMethodResolver, usePullRequestHandoffs, + type PullRequestHandoffThread, } from "../pullRequest/usePullRequestActions"; import { ChangeRequestStatusIcon, @@ -93,7 +94,9 @@ export function ThreadDetailsPrRow({ onOpen, onActed, onStopWatching, + currentThread, }: { + currentThread?: PullRequestHandoffThread | undefined; environmentId: EnvironmentId; pr: ThreadPr; number: number; @@ -168,7 +171,11 @@ export function ThreadDetailsPrRow({ onActed?.(); }, }); - const { handoff, startHandoff } = usePullRequestHandoffs({ environmentId, detail }); + const { handoff, startHandoff, handoffInCurrentThread } = usePullRequestHandoffs({ + environmentId, + detail, + currentThread, + }); const lastSelectedMergeMethod = useUiStateStore((state) => state.pullRequestMergeMethod); const resolveProjectDefaultMergeMethod = usePullRequestDefaultMergeMethodResolver( environmentId, @@ -324,8 +331,12 @@ export function ThreadDetailsPrRow({ pendingLabel: "Preparing...", pending: handoff === "conflicts", destructive: true, - suffix: , - tooltip: "Check the branch out and resolve the conflicts in a new thread", + suffix: handoffInCurrentThread ? null : ( + + ), + tooltip: handoffInCurrentThread + ? "Resolve the conflicts in this thread" + : "Check the branch out and resolve the conflicts in a new thread", onClick: startResolveConflicts, } : rowAction === "ready" @@ -344,8 +355,12 @@ export function ThreadDetailsPrRow({ pendingLabel: "Preparing...", pending: handoff === "findings", destructive: true, - suffix: , - tooltip: "Fix the failing checks in a new thread", + suffix: handoffInCurrentThread ? null : ( + + ), + tooltip: handoffInCurrentThread + ? "Fix the failing checks in this thread" + : "Fix the failing checks in a new thread", onClick: startFixChecks, } : rowAction === "merge" diff --git a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx index 2da6019787f5a..b99e2e4f1cf54 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx @@ -28,7 +28,9 @@ function ThreadDetailsPrLinkRow({ onOpen, onActed, onStopWatching, + currentThread, }: { + currentThread?: ComponentProps["currentThread"]; environmentId: EnvironmentId; link: ThreadPullRequestLink; onOpen: (event: ReactMouseEvent) => void; @@ -48,6 +50,7 @@ function ThreadDetailsPrLinkRow({ const pr = linked?.pr ?? null; return ( ( onOpenLink(event, link.url)} diff --git a/apps/web/src/components/pullRequest/usePullRequestActions.ts b/apps/web/src/components/pullRequest/usePullRequestActions.ts index 82ce86ab8f429..113c2640f3fca 100644 --- a/apps/web/src/components/pullRequest/usePullRequestActions.ts +++ b/apps/web/src/components/pullRequest/usePullRequestActions.ts @@ -29,7 +29,12 @@ import { buildPhysicalToLogicalProjectKeyMap } from "~/sidebarProjectGrouping"; import { useProjects } from "~/state/entities"; import { usePrimaryEnvironmentId } from "~/state/environments"; -import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; +import { + composerTargetKey, + type ComposerThreadTarget, + type DraftId, + useComposerDraftStore, +} from "~/composerDraftStore"; import { useNewThreadHandler } from "~/hooks/useHandleNewThread"; import { usePreparePullRequestThreadAction } from "~/state/sourceControlActions"; import type { ReviewCommentContext } from "~/reviewCommentContext"; @@ -246,14 +251,21 @@ export type PullRequestHandoffDetail = Pick< "projectId" | "workspaceRoot" | "url" >; -const lastHandoffPromptByDraft = new Map(); +export interface PullRequestHandoffThread { + target: ComposerThreadTarget; + pullRequestUrl: string; +} + +const lastHandoffPromptByTarget = new Map(); export function usePullRequestHandoffs({ environmentId, detail, + currentThread, }: { environmentId: EnvironmentId; detail: PullRequestHandoffDetail | null; + currentThread?: PullRequestHandoffThread | undefined; }) { const newThread = useNewThreadHandler(); const prepareThread = usePreparePullRequestThreadAction({ @@ -268,6 +280,27 @@ export function usePullRequestHandoffs({ sourceControlClients.get(undefined) ).changeRequest; + const handoffInCurrentThread = + currentThread !== undefined && detail?.url === currentThread.pullRequestUrl; + + const setTask = (target: ComposerThreadTarget, task: PullRequestThreadTask) => { + const store = useComposerDraftStore.getState(); + const draft = store.getComposerDraft(target); + const key = composerTargetKey(target); + store.setPrompt( + target, + handoffPrompt( + { prompt: draft?.prompt ?? "", lastHandoffPrompt: lastHandoffPromptByTarget.get(key) }, + task.prompt, + ), + ); + lastHandoffPromptByTarget.set(key, task.prompt); + store.setReviewComments( + target, + handoffReviewComments(draft?.reviewComments ?? [], task.reviewComments ?? []), + ); + }; + // Questions must not change checkouts or create worktrees. const openThreadWithTask = async ( projectRef: ReturnType, @@ -281,24 +314,7 @@ export function usePullRequestHandoffs({ () => null, )); if (session === null) return null; - const store = useComposerDraftStore.getState(); - if (task === null) return session; - const draft = store.getComposerDraft(session.draftId); - const existingComments = draft?.reviewComments ?? []; - const prompt = handoffPrompt( - { - prompt: draft?.prompt ?? "", - lastHandoffPrompt: lastHandoffPromptByDraft.get(session.draftId), - }, - task.prompt, - ); - // Track only the handoff contribution so later handoffs preserve user text. - lastHandoffPromptByDraft.set(session.draftId, task.prompt); - store.setPrompt(session.draftId, prompt); - store.setReviewComments( - session.draftId, - handoffReviewComments(existingComments, task.reviewComments ?? []), - ); + if (task !== null) setTask(session.draftId, task); return session; }; @@ -332,6 +348,15 @@ export function usePullRequestHandoffs({ mode: "worktree" | "local" = "worktree", ) => { if (!detail || handoff !== null) return; + if (handoffInCurrentThread && currentThread !== undefined && task !== null) { + setTask(currentThread.target, task); + toastManager.add({ + type: "success", + title: "Task ready", + description: "The task is in the composer. Read it over, then send.", + }); + return; + } setHandoff(kind); // Explicit timeouts survive toast updates and would pin the result after loading. const toastId = toastManager.add({ @@ -416,5 +441,5 @@ export function usePullRequestHandoffs({ ); }; - return { handoff, startAsk, startHandoff }; + return { handoff, startAsk, startHandoff, handoffInCurrentThread }; } From 093c20c6b273b50ce083bf03a147dfbf3eb85e4f Mon Sep 17 00:00:00 2001 From: extoci Date: Fri, 6 Nov 2026 06:26:48 +0000 Subject: [PATCH 2/2] fix(web): wait for PR thread checkouts and match canonical identities --- .../BranchToolbarBranchSelector.tsx | 7 ++- .../chat/ThreadDetailsPrRow.browser.test.tsx | 63 ++++++++++++++++--- .../components/chat/ThreadDetailsPrRow.tsx | 12 +++- .../components/chat/ThreadDetailsPrRows.tsx | 4 ++ .../pullRequest/usePullRequestActions.ts | 4 +- 5 files changed, 75 insertions(+), 15 deletions(-) diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index fd9088f139fd7..df1b0ff200f18 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -242,8 +242,8 @@ export function BranchToolbarBranchSelector({ const branchByName = new Map(refs.map((refName) => [refName.name, refName] as const)); const normalizedBranchQuery = trimmedBranchQuery.toLowerCase(); const prReference = parsePullRequestReference(trimmedBranchQuery); - const isSelectingWorktreeBase = - effectiveEnvMode === "worktree" && !envLocked && !activeWorktreePath; + const needsWorktree = effectiveEnvMode === "worktree" && !activeWorktreePath; + const isSelectingWorktreeBase = needsWorktree && !envLocked; const checkoutPullRequestItemValue = canChangeThreadBranch && prReference && onCheckoutPullRequestRequest ? `__checkout_pull_request__:${prReference}` @@ -738,8 +738,9 @@ export function BranchToolbarBranchSelector({ {displayMode === "panel" && prNumber !== undefined && prUrl !== undefined ? ( ({ status: "success" as PullRequestCheck["status"], + url: "https://github.com/owner/repo/pull/1", conflicting: false, extraStatus: null as PullRequestCheck["status"] | null, mergeMethods: ["merge"] as Array<"merge" | "squash" | "rebase">, @@ -30,10 +31,10 @@ vi.mock(import("~/state/entities"), async (importOriginal) => ({ const queryData = () => ({ projectId: ProjectId.make("project"), workspaceRoot: "/workspace", - url: "https://github.com/owner/repo/pull/1", + url: state.url, number: 1, title: "Test PR", - state: "open", + state: "open" as const, isDraft: false, mergeability: state.conflicting ? "conflicting" : "mergeable", headBranch: "feature", @@ -104,6 +105,10 @@ vi.mock(import("../ui/toast"), async (original) => { import { act, render as renderUi } from "../../../test/render"; import { useUiStateStore } from "~/uiStateStore"; import { ThreadDetailsPrRow } from "./ThreadDetailsPrRow"; +import { + buildFixFindingsHandoff, + buildResolveConflictsPrompt, +} from "../pullRequest/pullRequestDetail.logic"; it("merges with the last selected merge method", async () => { await using screen = await setup(); @@ -183,18 +188,38 @@ it.each([ { matches: false, conflicting: false, draft: false }, { matches: true, conflicting: true, draft: false }, { matches: true, conflicting: false, draft: true }, -])("uses the matching checkout for PR tasks: %j", async ({ matches, conflicting, draft }) => { + { + matches: true, + conflicting: false, + draft: false, + url: "https://git.cafe/Owner/Repo/pulls/1", + currentUrl: "https://git.cafe/owner/repo/pull/1", + }, + { + matches: true, + conflicting: true, + draft: false, + url: "https://git.cafe/Owner/Repo/pulls/1", + currentUrl: "https://git.cafe/owner/repo/pull/1", + }, + { matches: false, conflicting: false, draft: false, checkoutPending: true }, + { matches: false, conflicting: true, draft: false, checkoutPending: true }, +])("uses the matching checkout for PR tasks: %j", async (scenario) => { + const { matches, conflicting, draft } = scenario; + const checkoutPending = "checkoutPending" in scenario && scenario.checkoutPending; await using screen = await setup(); state.status = "failure"; state.conflicting = conflicting; + if ("url" in scenario) state.url = scenario.url; const threadRef = { environmentId: EnvironmentId.make("environment"), threadId: ThreadId.make("existing-thread"), }; const target = draft ? DraftId.make("existing-draft") : threadRef; useComposerDraftStore.getState().setPrompt(target, "Keep my notes"); - await screen.rerender( + const render = (pending: boolean) => ( , + /> ); - await screen.getByRole("button", { name: conflicting ? "Resolve" : "Fix", exact: true }).click(); + await screen.rerender(render(checkoutPending)); + const button = screen.getByRole("button", { name: conflicting ? "Resolve" : "Fix", exact: true }); + if (checkoutPending) { + await expect.element(button).toBeDisabled(); + expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toBe("Keep my notes"); + expect(state.newThread).not.toHaveBeenCalled(); + await screen.rerender(render(false)); + } + await button.click(); if (matches) { await vi.waitFor(() => { expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toContain( - "https://github.com/owner/repo/pull/1", + state.url, ); }); expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toContain( "Keep my notes", ); + const detail = queryData(); + const task = conflicting + ? buildResolveConflictsPrompt(detail) + : buildFixFindingsHandoff({ + ...detail, + reviewThreads: [], + comments: [], + commentsTruncated: false, + }).prompt; + expect(useComposerDraftStore.getState().getComposerDraft(target)?.prompt).toContain(task); expect(state.newThread).not.toHaveBeenCalled(); expect(state.prepareThread).not.toHaveBeenCalled(); } else { @@ -242,6 +288,7 @@ async function setup() { state.updateToast.mockReset(); state.status = "success"; + state.url = "https://github.com/owner/repo/pull/1"; state.conflicting = false; state.extraStatus = null; state.mergeMethods = ["merge"]; diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index 0e995fccee81e..6d7d187ab78a4 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -95,8 +95,10 @@ export function ThreadDetailsPrRow({ onActed, onStopWatching, currentThread, + handoffDisabled = false, }: { currentThread?: PullRequestHandoffThread | undefined; + handoffDisabled?: boolean | undefined; environmentId: EnvironmentId; pr: ThreadPr; number: number; @@ -197,7 +199,7 @@ export function ThreadDetailsPrRow({ ); const startResolveConflicts = () => { - if (detail === null) return; + if (detail === null || handoffDisabled) return; void startHandoff("conflicts", { prompt: buildResolveConflictsPrompt({ number: detail.number, @@ -209,7 +211,7 @@ export function ThreadDetailsPrRow({ }; const startFixChecks = () => { - if (detail === null) return; + if (detail === null || handoffDisabled) return; void startHandoff( "findings", buildFixFindingsHandoff({ @@ -476,7 +478,11 @@ export function ThreadDetailsPrRow({ size="sm" part="meta" tone={trailingAction.destructive ? "destructive" : "primary"} - disabled={actionPending || handoff !== null} + disabled={ + actionPending || + handoff !== null || + (handoffDisabled && (rowAction === "fix" || rowAction === "resolve")) + } onClick={trailingAction.onClick} /> } diff --git a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx index b99e2e4f1cf54..c1781b40da1f7 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx @@ -29,8 +29,10 @@ function ThreadDetailsPrLinkRow({ onActed, onStopWatching, currentThread, + handoffDisabled, }: { currentThread?: ComponentProps["currentThread"]; + handoffDisabled?: boolean | undefined; environmentId: EnvironmentId; link: ThreadPullRequestLink; onOpen: (event: ReactMouseEvent) => void; @@ -50,6 +52,7 @@ function ThreadDetailsPrLinkRow({ const pr = linked?.pr ?? null; return ( (