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.
This commit is contained in:
@@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<UserComposerAttachment>,
|
||||
): PickerItem | null {
|
||||
let onlyPr: Extract<UserComposerAttachment, { kind: "github_pr" }> | 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;
|
||||
}
|
||||
|
||||
@@ -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<PickerSelection | null>(null);
|
||||
const [manualPickerSelection, setManualPickerSelection] = useState<PickerSelection | null>(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,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user