From b9cea49f106eacd9a689c3b83d1a1695d26c19dc Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 12 May 2026 21:08:47 +0800 Subject: [PATCH] fix(app): auto-set ref picker from a single attached PR (#951) * fix(app): auto-set ref picker from a single attached PR Attaching a PR in the new-workspace composer signals "I want this PR's context", and almost always also means "base the worktree on this PR". Until now the two paths were independent: the ref picker stayed on the default branch, so the worktree was branched off main even though the sidebar chip looked like a PR checkout. Auto-promote the picker when exactly one PR is attached and the user hasn't manually picked a ref. Manual picker selections are preserved; auto-promoted ones are cleared when their backing attachment is removed. * refactor(app): derive auto-promoted picker selection instead of syncing The previous version drove `pickerSelection` from a useEffect that inspected attachments and overwrote state. That meant the shared `PickerSelection` type had to grow a `source: "manual" | "attachment"` discriminator just so the effect could tell its own writes apart from manual picks. Classic effect-as-derived-state. Split the state instead. `manualPickerSelection` holds explicit picks; `autoPickerItem` is a useMemo over composer attachments; the effective `pickerSelection` is `manual ?? auto` via a small precedence helper. No effect, no `source` flag, and `PickerSelection` stays a screen-local shape since nothing outside needs it. --- .../new-workspace-picker-state.test.ts | 37 ++++++++++++++++++- .../src/screens/new-workspace-picker-state.ts | 16 ++++++++ .../app/src/screens/new-workspace-screen.tsx | 28 ++++++++++++-- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/packages/app/src/screens/new-workspace-picker-state.test.ts b/packages/app/src/screens/new-workspace-picker-state.test.ts index 66f6995d6..82a938d81 100644 --- a/packages/app/src/screens/new-workspace-picker-state.test.ts +++ b/packages/app/src/screens/new-workspace-picker-state.test.ts @@ -1,7 +1,10 @@ import { describe, expect, it } from "vitest"; import type { UserComposerAttachment } from "@/attachments/types"; import type { GitHubSearchItem } from "@server/shared/messages"; -import { syncPickerPrAttachment } from "./new-workspace-picker-state"; +import { + deriveAutoPickerItemFromAttachments, + syncPickerPrAttachment, +} from "./new-workspace-picker-state"; function makePrItem(number: number, title: string, headRefName = "feature/x"): GitHubSearchItem { return { @@ -94,3 +97,35 @@ describe("syncPickerPrAttachment", () => { expect(result.attachments).toHaveLength(1); }); }); + +describe("deriveAutoPickerItemFromAttachments", () => { + it("returns null when there are no attachments", () => { + expect(deriveAutoPickerItemFromAttachments([])).toBeNull(); + }); + + it("returns the PR when exactly one is attached", () => { + const pr = makePrItem(923, "Nix overridable npm deps hash"); + expect(deriveAutoPickerItemFromAttachments([prAttachment(pr)])).toEqual({ + kind: "github-pr", + item: pr, + }); + }); + + it("returns null when multiple PRs are attached", () => { + const a = makePrItem(101, "A"); + const b = makePrItem(202, "B"); + expect(deriveAutoPickerItemFromAttachments([prAttachment(a), prAttachment(b)])).toBeNull(); + }); + + it("ignores non-PR attachments", () => { + expect(deriveAutoPickerItemFromAttachments([issueAttachment(44)])).toBeNull(); + }); + + it("returns the lone PR even when other non-PR attachments are present", () => { + const pr = makePrItem(923, "Nix overridable npm deps hash"); + expect(deriveAutoPickerItemFromAttachments([issueAttachment(44), prAttachment(pr)])).toEqual({ + kind: "github-pr", + item: pr, + }); + }); +}); diff --git a/packages/app/src/screens/new-workspace-picker-state.ts b/packages/app/src/screens/new-workspace-picker-state.ts index d9aac0f49..c9c33d2f3 100644 --- a/packages/app/src/screens/new-workspace-picker-state.ts +++ b/packages/app/src/screens/new-workspace-picker-state.ts @@ -33,3 +33,19 @@ export function syncPickerPrAttachment(input: { return { attachments: nextAttachments, attachedPrNumber }; } + +// Suggest a ref picker target derived from composer attachments alone. When the +// user attaches exactly one PR they almost always want the worktree based on +// that PR — surface it as the picker's default so the worktree isn't silently +// branched off main. The caller is responsible for letting manual picks win. +export function deriveAutoPickerItemFromAttachments( + attachments: ReadonlyArray, +): PickerItem | null { + let onlyPr: Extract | null = null; + for (const attachment of attachments) { + if (attachment.kind !== "github_pr") continue; + if (onlyPr) return null; + onlyPr = attachment; + } + return onlyPr ? { kind: "github-pr", item: onlyPr.item } : null; +} diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index 1caab7b14..f63e8efbe 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -33,7 +33,10 @@ import type { AgentAttachment, GitHubSearchItem } from "@server/shared/messages" import type { AgentProvider } from "@server/server/agent/agent-sdk-types"; import { isEmptyWorkspaceSubmission, runCreateEmptyWorkspace } from "./new-workspace-empty"; import { pickerItemToCheckoutRequest, type PickerItem } from "./new-workspace-picker-item"; -import { syncPickerPrAttachment } from "./new-workspace-picker-state"; +import { + deriveAutoPickerItemFromAttachments, + syncPickerPrAttachment, +} from "./new-workspace-picker-state"; interface NewWorkspaceScreenProps { serverId: string; @@ -51,6 +54,17 @@ interface PickerSelection { attachedPrNumber: number | null; } +// Manual picks always win; the auto-promoted item is a fallback so the user +// doesn't silently get "main" when they meant the PR they just attached. +function combinePickerSelection( + manual: PickerSelection | null, + autoItem: PickerItem | null, +): PickerSelection | null { + if (manual) return manual; + if (autoItem) return { item: autoItem, attachedPrNumber: null }; + return null; +} + const BRANCH_OPTION_PREFIX = "branch:"; const PR_OPTION_PREFIX = "github-pr:"; @@ -400,7 +414,7 @@ export function NewWorkspaceScreen({ typeof normalizeWorkspaceDescriptor > | null>(null); const [pendingAction, setPendingAction] = useState<"chat" | "empty" | null>(null); - const [pickerSelection, setPickerSelection] = useState(null); + const [manualPickerSelection, setManualPickerSelection] = useState(null); const [pickerOpen, setPickerOpen] = useState(false); const [pickerSearchQuery, setPickerSearchQuery] = useState(""); const [debouncedPickerSearchQuery, setDebouncedPickerSearchQuery] = useState(""); @@ -414,7 +428,6 @@ export function NewWorkspaceScreen({ const displayName = displayNameProp?.trim() ?? ""; const workspace = createdWorkspace; - const selectedItem = pickerSelection?.item ?? null; const isPending = pendingAction !== null; const client = useHostRuntimeClient(serverId); const isConnected = useHostRuntimeIsConnected(serverId); @@ -430,6 +443,13 @@ export function NewWorkspaceScreen({ }); const composerState = chatDraft.composerState; + const autoPickerItem = useMemo( + () => deriveAutoPickerItemFromAttachments(chatDraft.attachments), + [chatDraft.attachments], + ); + const pickerSelection = combinePickerSelection(manualPickerSelection, autoPickerItem); + const selectedItem = pickerSelection?.item ?? null; + const withConnectedClient = useCallback(() => { if (!client || !isConnected) { throw new Error("Host is not connected"); @@ -525,7 +545,7 @@ export function NewWorkspaceScreen({ item, }); - setPickerSelection({ + setManualPickerSelection({ item, attachedPrNumber: next.attachedPrNumber, });