Start new workspaces from pasted pull requests (#2290)

* fix(workspaces): start pasted pull requests from their branch Recognized pull request links now select their branch directly instead of waiting for a separate confirmation. Ensure single-branch clones also record the fetched branch so the new worktree can track it. * fix(workspaces): keep pasted PR checkout target-safe * fix(workspaces): make pasted PR resolution deterministic * fix(workspaces): preserve explicit checkout selection A recognized pull request selects itself only when its attachment is added. Once the user chooses a branch, later composer edits must not derive checkout state from existing attachments. * fix(composer): ignore stale pull request lookups A lookup may finish after the workspace target changes. Only apply its attachment and selection event when the host, working directory, client, and remote still match the target that started it. * fix(workspaces): make checkout choices authoritative Arm automatic pull request selection only when a new PR link is detected. An explicit picker choice cancels that pending selection, existing draft attachments never infer checkout state, and target changes suppress carried-over lookups. * fix(workspaces): stabilize pull request auto-selection Keep the first PR selected when one edit contains several links, and accept a completed lookup when only the transport client changed for the same host and checkout. * fix(composer): release removed pull request lookups Release each auto-attach invocation when its composer state is abandoned, while continuing to discard any stale response.
2026-07-21 15:20:05 +02:00
parent fd4e13735c
commit 2cb1b041dd
20 changed files with 768 additions and 313 deletions
@@ -48,7 +48,9 @@ interface SearchCall {

 interface HarnessInput {
  initialAttachments?: UserComposerAttachment[];
+  initialCwd?: string;
  initialText?: string;
+  onPullRequestDetected?: () => void;
  remote?: string | null;
 }

@@ -72,6 +74,14 @@ function createSearchClient(items: ForgeSearchItem[]): ForgeSearchClient & { cal
  };
 }

+function deferred<T>() {
+  let resolve!: (value: T) => void;
+  const promise = new Promise<T>((resolvePromise) => {
+    resolve = resolvePromise;
+  });
+  return { promise, resolve };
+}
+
 function createWrapper() {
  const queryClient = new QueryClient({
    defaultOptions: {
@@ -87,6 +97,8 @@ function createWrapper() {

 function useHarness(client: ForgeSearchClient, input: HarnessInput = {}) {
  const [text, setText] = useState(input.initialText ?? "");
+  const [searchClient, setSearchClient] = useState(client);
+  const [workingDirectory, setWorkingDirectory] = useState(input.initialCwd ?? cwd);
  const [attachments, setAttachments] = useState<UserComposerAttachment[]>(
    input.initialAttachments ?? [],
  );
@@ -94,18 +106,22 @@ function useHarness(client: ForgeSearchClient, input: HarnessInput = {}) {
    text,
    remoteUrl: input.remote ?? remoteUrl,
    attachments,
-    client,
+    client: searchClient,
    isConnected: true,
    serverId: "server-1",
-    cwd,
+    cwd: workingDirectory,
    setAttachments,
+    onPullRequestDetected: input.onPullRequestDetected,
  });

  return {
    text,
    setText,
+    setSearchClient,
+    setWorkingDirectory,
    attachments,
    setAttachments,
+    isResolving: autoAttach.isResolving,
    markGithubAttachmentRemoved: autoAttach.markGithubAttachmentRemoved,
  };
 }
@@ -121,14 +137,20 @@ describe("useComposerGithubAutoAttach", () => {
  it("adds a matching pasted GitHub PR URL as a composer attachment", async () => {
    vi.useFakeTimers();
    const client = createSearchClient([pr101]);
-    const { result } = renderHook(() => useHarness(client), { wrapper: createWrapper() });
+    const onPullRequestDetected = vi.fn();
+    const { result } = renderHook(() => useHarness(client, { onPullRequestDetected }), {
+      wrapper: createWrapper(),
+    });

    act(() => {
      result.current.setText("Please review https://github.com/acme/paseo/pull/101");
    });
+    expect(result.current.isResolving).toBe(true);
+    expect(onPullRequestDetected).toHaveBeenCalledTimes(1);
    await flushDebounce();

    expect(result.current.attachments).toEqual([{ kind: "forge_change_request", item: pr101 }]);
+    expect(result.current.isResolving).toBe(false);
    expect(client.calls).toEqual([{ cwd, query: "101", limit: 20 }]);
    vi.useRealTimers();
  });
@@ -208,4 +230,115 @@ describe("useComposerGithubAutoAttach", () => {
    ]);
    vi.useRealTimers();
  });
+
+  it("stays resolving while overlapping lookups share a ref", async () => {
+    vi.useFakeTimers();
+    const firstLookup = deferred<ForgeSearchPayload>();
+    const secondLookup = deferred<ForgeSearchPayload>();
+    const client: ForgeSearchClient = {
+      searchForge: vi
+        .fn()
+        .mockReturnValueOnce(firstLookup.promise)
+        .mockReturnValueOnce(secondLookup.promise),
+    };
+    const { result } = renderHook(() => useHarness(client), { wrapper: createWrapper() });
+
+    act(() => {
+      result.current.setText(
+        "Refs https://github.com/acme/paseo/pull/101 and https://github.com/acme/paseo/pull/202",
+      );
+    });
+    await flushDebounce();
+
+    act(() => {
+      result.current.setText("Still https://github.com/acme/paseo/pull/202");
+    });
+    await flushDebounce();
+
+    await act(async () => {
+      firstLookup.resolve(githubPayload([], "search-101"));
+      await Promise.resolve();
+    });
+    expect(result.current.isResolving).toBe(true);
+
+    await act(async () => {
+      secondLookup.resolve(githubPayload([], "search-202"));
+      await Promise.resolve();
+    });
+    expect(result.current.isResolving).toBe(false);
+    vi.useRealTimers();
+  });
+
+  it("stops resolving when an in-flight PR URL is removed", async () => {
+    vi.useFakeTimers();
+    const lookup = deferred<ForgeSearchPayload>();
+    const client: ForgeSearchClient = {
+      searchForge: vi.fn().mockReturnValue(lookup.promise),
+    };
+    const { result } = renderHook(() => useHarness(client), { wrapper: createWrapper() });
+
+    act(() => {
+      result.current.setText("Review https://github.com/acme/paseo/pull/101");
+    });
+    await flushDebounce();
+    act(() => {
+      result.current.setText("");
+    });
+
+    expect(result.current.isResolving).toBe(false);
+    vi.useRealTimers();
+  });
+
+  it("ignores a lookup that finishes after the target changes", async () => {
+    vi.useFakeTimers();
+    const lookup = deferred<ForgeSearchPayload>();
+    const client: ForgeSearchClient = {
+      searchForge: vi.fn().mockReturnValue(lookup.promise),
+    };
+    const { result } = renderHook(() => useHarness(client), { wrapper: createWrapper() });
+
+    act(() => {
+      result.current.setText("Review https://github.com/acme/paseo/pull/101");
+    });
+    await flushDebounce();
+
+    act(() => {
+      result.current.setWorkingDirectory("/other-repo");
+    });
+    await flushDebounce();
+    await act(async () => {
+      lookup.resolve(githubPayload([pr101], "search-101"));
+      await Promise.resolve();
+    });
+
+    expect(result.current.attachments).toEqual([]);
+    expect(client.searchForge).toHaveBeenCalledTimes(1);
+    vi.useRealTimers();
+  });
+
+  it("accepts a lookup after the transport client is replaced for the same target", async () => {
+    vi.useFakeTimers();
+    const lookup = deferred<ForgeSearchPayload>();
+    const firstClient: ForgeSearchClient = {
+      searchForge: vi.fn().mockReturnValue(lookup.promise),
+    };
+    const replacementClient = createSearchClient([pr101]);
+    const { result } = renderHook(() => useHarness(firstClient), { wrapper: createWrapper() });
+
+    act(() => {
+      result.current.setText("Review https://github.com/acme/paseo/pull/101");
+    });
+    await flushDebounce();
+    act(() => {
+      result.current.setSearchClient(replacementClient);
+    });
+    await act(async () => {
+      lookup.resolve(githubPayload([pr101], "search-101"));
+      await Promise.resolve();
+    });
+
+    expect(result.current.attachments).toEqual([{ kind: "forge_change_request", item: pr101 }]);
+    expect(replacementClient.calls).toEqual([]);
+    vi.useRealTimers();
+  });
 });
@@ -4,6 +4,7 @@ import {
  useEffect,
  useMemo,
  useRef,
+  useState,
  type Dispatch,
  type RefObject,
  type SetStateAction,
@@ -26,9 +27,12 @@ interface ComposerGithubAutoAttachInput {
  cwd: string;
  supportsForgeSearch?: boolean;
  setAttachments: Dispatch<SetStateAction<UserComposerAttachment[]>>;
+  onPullRequestDetected?: () => void;
+  onPullRequestAdded?: (item: ForgeSearchItem) => void;
 }

 interface ComposerGithubAutoAttachResult {
+  isResolving: boolean;
  markGithubAttachmentRemoved: (attachment: ComposerAttachment | undefined) => void;
 }

@@ -39,10 +43,24 @@ export function useComposerGithubAutoAttach(
  const latestRef = useRef(params);
  const removedRefKeysRef = useRef(new Set<string>());
  const pendingRefKeysRef = useRef(new Set<string>());
+  const presentPullRequestKeysRef = useRef(new Set<string>());
+  const previousTargetRef = useRef({ serverId: params.serverId, cwd: params.cwd });
+  const [resolvingRefCounts, setResolvingRefCounts] = useState<ReadonlyMap<string, number>>(
+    () => new Map(),
+  );

  latestRef.current = params;

  useEffect(() => {
+    suppressRefsCarriedAcrossTargets({
+      params: latestRef.current,
+      previousTargetRef,
+      removedRefKeys: removedRefKeysRef.current,
+    });
+    notifyNewPullRequestRefs({
+      params: latestRef.current,
+      presentPullRequestKeysRef,
+    });
    const refs = refsReadyForLookup({
      params: latestRef.current,
      removedRefKeys: removedRefKeysRef.current,
@@ -52,6 +70,15 @@ export function useComposerGithubAutoAttach(
      return;
    }

+    const refKeys = refs.map(githubRefKey);
+    setResolvingRefCounts((current) => addKeys(current, refKeys));
+    let resolvingReleased = false;
+    const releaseResolving = () => {
+      if (resolvingReleased) return;
+      resolvingReleased = true;
+      clearResolvingKeys(setResolvingRefCounts, refKeys);
+    };
+
    const timerId = setTimeout(() => {
      void attachRefs({
        refs,
@@ -59,11 +86,12 @@ export function useComposerGithubAutoAttach(
        latestRef,
        removedRefKeys: removedRefKeysRef.current,
        pendingRefKeys: pendingRefKeysRef.current,
-      });
+      }).finally(releaseResolving);
    }, AUTO_ATTACH_DEBOUNCE_MS);

    return () => {
      clearTimeout(timerId);
+      releaseResolving();
    };
  }, [
    params.text,
@@ -86,12 +114,84 @@ export function useComposerGithubAutoAttach(

  return useMemo(
    () => ({
+      isResolving: resolvingRefCounts.size > 0,
      markGithubAttachmentRemoved,
    }),
-    [markGithubAttachmentRemoved],
+    [markGithubAttachmentRemoved, resolvingRefCounts.size],
  );
 }

+function suppressRefsCarriedAcrossTargets({
+  params,
+  previousTargetRef,
+  removedRefKeys,
+}: {
+  params: ComposerGithubAutoAttachInput;
+  previousTargetRef: RefObject<{ serverId: string; cwd: string }>;
+  removedRefKeys: Set<string>;
+}): void {
+  const previous = previousTargetRef.current;
+  const targetChanged =
+    previous.cwd.trim().length > 0 &&
+    params.cwd.trim().length > 0 &&
+    (previous.serverId !== params.serverId || previous.cwd !== params.cwd);
+  previousTargetRef.current = { serverId: params.serverId, cwd: params.cwd };
+  if (!targetChanged) return;
+
+  for (const ref of extractGithubRefs(params.text, params.remoteUrl)) {
+    removedRefKeys.add(githubRefKey(ref));
+  }
+}
+
+function notifyNewPullRequestRefs({
+  params,
+  presentPullRequestKeysRef,
+}: {
+  params: ComposerGithubAutoAttachInput;
+  presentPullRequestKeysRef: RefObject<Set<string>>;
+}): void {
+  const currentKeys = new Set(
+    extractGithubRefs(params.text, params.remoteUrl)
+      .filter((ref) => ref.kind === "pull")
+      .map(githubRefKey),
+  );
+  for (const key of currentKeys) {
+    if (!presentPullRequestKeysRef.current.has(key)) {
+      params.onPullRequestDetected?.();
+    }
+  }
+  presentPullRequestKeysRef.current = currentKeys;
+}
+
+function addKeys(
+  current: ReadonlyMap<string, number>,
+  keys: readonly string[],
+): ReadonlyMap<string, number> {
+  const nextCounts = new Map(current);
+  for (const key of keys) nextCounts.set(key, (nextCounts.get(key) ?? 0) + 1);
+  return nextCounts;
+}
+
+function removeKeys(
+  current: ReadonlyMap<string, number>,
+  keys: readonly string[],
+): ReadonlyMap<string, number> {
+  const next = new Map(current);
+  for (const key of keys) {
+    const count = next.get(key) ?? 0;
+    if (count <= 1) next.delete(key);
+    else next.set(key, count - 1);
+  }
+  return next;
+}
+
+function clearResolvingKeys(
+  setResolvingRefCounts: Dispatch<SetStateAction<ReadonlyMap<string, number>>>,
+  keys: readonly string[],
+): void {
+  setResolvingRefCounts((current) => removeKeys(current, keys));
+}
+
 async function attachRefs({
  refs,
  queryClient,
@@ -142,16 +242,28 @@ async function attachRef({
    return;
  }
  const item = search.items.find((candidate) => githubItemMatchesRef(candidate, ref));
-  if (!item || removedRefKeys.has(key) || !isRefStillPresent(ref, latestRef.current)) {
+  const current = latestRef.current;
+  if (
+    !item ||
+    removedRefKeys.has(key) ||
+    !isSameLookupTarget(snapshot, current) ||
+    !isRefStillPresent(ref, current)
+  ) {
    return;
  }

-  latestRef.current.setAttachments((current) => {
-    if (removedRefKeys.has(key) || isAttachmentSelectedForGithubItem(current, item)) {
-      return current;
+  if (isAttachmentSelectedForGithubItem(current.attachments, item)) {
+    return;
+  }
+  current.setAttachments((attachments) => {
+    if (removedRefKeys.has(key) || isAttachmentSelectedForGithubItem(attachments, item)) {
+      return attachments;
    }
-    return toggleGithubAttachment(current, item);
+    return toggleGithubAttachment(attachments, item);
  });
+  if (item.kind === "change_request") {
+    current.onPullRequestAdded?.(item);
+  }
 }

 function refsReadyForLookup({
@@ -212,6 +324,17 @@ function isRefStillPresent(ref: GithubRef, params: ComposerGithubAutoAttachInput
  );
 }

+function isSameLookupTarget(
+  initial: ComposerGithubAutoAttachInput,
+  current: ComposerGithubAutoAttachInput,
+): boolean {
+  return (
+    initial.serverId === current.serverId &&
+    initial.cwd === current.cwd &&
+    initial.remoteUrl === current.remoteUrl
+  );
+}
+
 function hasGithubAttachment(attachments: UserComposerAttachment[], ref: GithubRef): boolean {
  return attachments.some((attachment) => attachmentKey(attachment) === githubRefKey(ref));
 }