diff --git a/packages/app/e2e/browser/new-workspace.spec.ts b/packages/app/e2e/browser/new-workspace.spec.ts index b50d79acf..8342dca68 100644 --- a/packages/app/e2e/browser/new-workspace.spec.ts +++ b/packages/app/e2e/browser/new-workspace.spec.ts @@ -50,6 +50,13 @@ import { waitForSidebarHydration, waitForWorkspaceInSidebar, } from "../support/helpers/workspace-ui"; +import { dropFileOnComposer, expectAttachmentPill } from "../support/helpers/composer"; + +const BACKGROUND_RESOLUTION_FILE = { + name: "background-context.json", + mimeType: "application/json", + buffer: Buffer.from(JSON.stringify({ composer: "background-resolution" })), +}; interface WorkspaceStatusGroupEvent { rowTestId: string; @@ -848,7 +855,13 @@ test.describe("New workspace flow", () => { await pasteGithubPrUrl(page, context, pr.url); - await expect(page.getByTestId("workspace-create-submit")).toBeDisabled(); + const createButton = page.getByTestId("workspace-create-submit"); + await expect(createButton).toBeDisabled(); + await expect(createButton.getByRole("progressbar")).toHaveCount(0); + + await dropFileOnComposer(page, BACKGROUND_RESOLUTION_FILE); + await expectAttachmentPill(page, "composer-file-attachment-pill"); + await expectComposerGithubAttachmentPill(page, { number: pr.number, title: pr.title, diff --git a/packages/app/src/composer/github/auto-attach.test.tsx b/packages/app/src/composer/github/auto-attach.test.tsx index 8aee13f80..2abb55ba3 100644 --- a/packages/app/src/composer/github/auto-attach.test.tsx +++ b/packages/app/src/composer/github/auto-attach.test.tsx @@ -28,6 +28,14 @@ const pr101: ForgeSearchItem = { headRefName: "feature", }; +const pr202: ForgeSearchItem = { + ...pr101, + number: 202, + title: "Attach second PR", + url: "https://github.com/acme/paseo/pull/202", + headRefName: "feature-two", +}; + const issue202: ForgeSearchItem = { kind: "issue", number: 202, @@ -51,6 +59,7 @@ interface HarnessInput { initialCwd?: string; initialText?: string; onPullRequestDetected?: () => void; + onPullRequestAdded?: (item: ForgeSearchItem) => void; remote?: string | null; } @@ -112,6 +121,7 @@ function useHarness(client: ForgeSearchClient, input: HarnessInput = {}) { cwd: workingDirectory, setAttachments, onPullRequestDetected: input.onPullRequestDetected, + onPullRequestAdded: input.onPullRequestAdded, }); return { @@ -231,7 +241,7 @@ describe("useComposerGithubAutoAttach", () => { vi.useRealTimers(); }); - it("stays resolving while overlapping lookups share a ref", async () => { + it("reports pasted pull requests in source order when lookups finish out of order", async () => { vi.useFakeTimers(); const firstLookup = deferred(); const secondLookup = deferred(); @@ -241,7 +251,10 @@ describe("useComposerGithubAutoAttach", () => { .mockReturnValueOnce(firstLookup.promise) .mockReturnValueOnce(secondLookup.promise), }; - const { result } = renderHook(() => useHarness(client), { wrapper: createWrapper() }); + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + wrapper: createWrapper(), + }); act(() => { result.current.setText( @@ -250,21 +263,314 @@ describe("useComposerGithubAutoAttach", () => { }); await flushDebounce(); + await act(async () => { + secondLookup.resolve(githubPayload([pr202], "search-202")); + await Promise.resolve(); + }); + expect(onPullRequestAdded).not.toHaveBeenCalled(); + expect(result.current.attachments).toEqual([]); + + await act(async () => { + firstLookup.resolve(githubPayload([pr101], "search-101")); + await Promise.resolve(); + }); + await vi.waitFor(() => { + expect(onPullRequestAdded.mock.calls).toEqual([[pr101], [pr202]]); + expect(result.current.attachments).toEqual([ + { kind: "forge_change_request", item: pr101 }, + { kind: "forge_change_request", item: pr202 }, + ]); + }); + vi.useRealTimers(); + }); + + it("preserves pull request order across lookup batches", async () => { + vi.useFakeTimers(); + const firstLookup = deferred(); + const secondLookup = deferred(); + const client: ForgeSearchClient = { + searchForge: vi + .fn() + .mockReturnValueOnce(firstLookup.promise) + .mockReturnValueOnce(secondLookup.promise), + }; + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.setText("Review https://github.com/acme/paseo/pull/101"); + }); + await flushDebounce(); + act(() => { + result.current.setText( + "Review https://github.com/acme/paseo/pull/101 and https://github.com/acme/paseo/pull/202", + ); + }); + expect(result.current.isResolving).toBe(true); + await flushDebounce(); + + await act(async () => { + secondLookup.resolve(githubPayload([pr202], "search-202")); + await Promise.resolve(); + }); + expect(onPullRequestAdded).not.toHaveBeenCalled(); + expect(result.current.attachments).toEqual([]); + expect(result.current.isResolving).toBe(true); + + await act(async () => { + firstLookup.resolve(githubPayload([pr101], "search-101")); + await Promise.resolve(); + }); + await vi.waitFor(() => { + expect(onPullRequestAdded.mock.calls).toEqual([[pr101], [pr202]]); + expect(result.current.attachments).toEqual([ + { kind: "forge_change_request", item: pr101 }, + { kind: "forge_change_request", item: pr202 }, + ]); + expect(result.current.isResolving).toBe(false); + }); + expect(client.searchForge).toHaveBeenCalledTimes(2); + vi.useRealTimers(); + }); + + it("uses the latest pull request order when URLs are reordered during debounce", async () => { + vi.useFakeTimers(); + const client = createSearchClient([pr101, pr202]); + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.setText( + "Refs https://github.com/acme/paseo/pull/101 and https://github.com/acme/paseo/pull/202", + ); + }); + act(() => { + result.current.setText( + "Refs https://github.com/acme/paseo/pull/202 and https://github.com/acme/paseo/pull/101", + ); + }); + await flushDebounce(); + + expect(onPullRequestAdded.mock.calls).toEqual([[pr202], [pr101]]); + expect(client.calls).toEqual([ + { cwd, query: "202", limit: 20 }, + { cwd, query: "101", limit: 20 }, + ]); + vi.useRealTimers(); + }); + + it("uses the latest pull request order when active lookups are reordered", async () => { + vi.useFakeTimers(); + const firstLookup = deferred(); + const secondLookup = deferred(); + const client: ForgeSearchClient = { + searchForge: vi + .fn() + .mockReturnValueOnce(firstLookup.promise) + .mockReturnValueOnce(secondLookup.promise), + }; + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + 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( + "Refs https://github.com/acme/paseo/pull/202 and https://github.com/acme/paseo/pull/101", + ); + }); + await flushDebounce(); + + await act(async () => { + firstLookup.resolve(githubPayload([pr101], "search-101")); + await Promise.resolve(); + }); + expect(onPullRequestAdded).not.toHaveBeenCalled(); + expect(result.current.attachments).toEqual([]); + expect(result.current.isResolving).toBe(true); + + await act(async () => { + secondLookup.resolve(githubPayload([pr202], "search-202")); + await Promise.resolve(); + }); + await vi.waitFor(() => { + expect(onPullRequestAdded.mock.calls).toEqual([[pr202], [pr101]]); + expect(result.current.attachments).toEqual([ + { kind: "forge_change_request", item: pr202 }, + { kind: "forge_change_request", item: pr101 }, + ]); + expect(result.current.isResolving).toBe(false); + }); + expect(client.searchForge).toHaveBeenCalledTimes(2); + vi.useRealTimers(); + }); + + it("does not report a buffered pull request after its URL is removed", async () => { + vi.useFakeTimers(); + const firstLookup = deferred(); + const secondLookup = deferred(); + const client: ForgeSearchClient = { + searchForge: vi + .fn() + .mockReturnValueOnce(firstLookup.promise) + .mockReturnValueOnce(secondLookup.promise), + }; + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.setText( + "Refs https://github.com/acme/paseo/pull/101 and https://github.com/acme/paseo/pull/202", + ); + }); + await flushDebounce(); + + await act(async () => { + secondLookup.resolve(githubPayload([pr202], "search-202")); + await Promise.resolve(); + }); + act(() => { + result.current.setText("Still https://github.com/acme/paseo/pull/101"); + }); + await flushDebounce(); + + await act(async () => { + firstLookup.resolve(githubPayload([pr101], "search-101")); + await Promise.resolve(); + }); + await vi.waitFor(() => { + expect(onPullRequestAdded.mock.calls).toEqual([[pr101]]); + }); + vi.useRealTimers(); + }); + + it("keeps resolving when a pending pull request URL is removed and re-added", async () => { + vi.useFakeTimers(); + const lookup = deferred(); + const client: ForgeSearchClient = { + searchForge: vi.fn().mockReturnValue(lookup.promise), + }; + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + 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); + + act(() => { + result.current.setText("Review https://github.com/acme/paseo/pull/101"); + }); + expect(result.current.isResolving).toBe(true); + await flushDebounce(); + expect(client.searchForge).toHaveBeenCalledTimes(1); + + await act(async () => { + lookup.resolve(githubPayload([pr101], "search-101")); + await Promise.resolve(); + }); + await vi.waitFor(() => { + expect(result.current.attachments).toEqual([{ kind: "forge_change_request", item: pr101 }]); + expect(result.current.isResolving).toBe(false); + expect(onPullRequestAdded.mock.calls).toEqual([[pr101]]); + }); + vi.useRealTimers(); + }); + + it("releases a removed ref without waiting for its lookup to settle", async () => { + vi.useFakeTimers(); + const firstLookup = deferred(); + const secondLookup = deferred(); + const client: ForgeSearchClient = { + searchForge: vi + .fn() + .mockReturnValueOnce(firstLookup.promise) + .mockReturnValueOnce(secondLookup.promise), + }; + const onPullRequestAdded = vi.fn(); + const { result } = renderHook(() => useHarness(client, { onPullRequestAdded }), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.setText( + "Refs https://github.com/acme/paseo/pull/101 and https://github.com/acme/paseo/pull/202", + ); + }); + await flushDebounce(); + + await act(async () => { + secondLookup.resolve(githubPayload([pr202], "search-202")); + await Promise.resolve(); + }); + expect(onPullRequestAdded).not.toHaveBeenCalled(); + expect(result.current.isResolving).toBe(true); + act(() => { result.current.setText("Still https://github.com/acme/paseo/pull/202"); }); await flushDebounce(); + await vi.waitFor(() => { + expect(onPullRequestAdded.mock.calls).toEqual([[pr202]]); + expect(result.current.isResolving).toBe(false); + }); + await act(async () => { firstLookup.resolve(githubPayload([], "search-101")); await Promise.resolve(); }); + vi.useRealTimers(); + }); + + it("stays resolving when an unrelated attachment is added during lookup", async () => { + vi.useFakeTimers(); + const lookup = deferred(); + 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.setAttachments([{ kind: "forge_issue", item: issue202 }]); + result.current.setText("Review https://github.com/acme/paseo/pull/101 please"); + }); + expect(result.current.isResolving).toBe(true); await act(async () => { - secondLookup.resolve(githubPayload([], "search-202")); + lookup.resolve(githubPayload([pr101], "search-101")); await Promise.resolve(); }); + + expect(result.current.attachments).toEqual([ + { kind: "forge_issue", item: issue202 }, + { kind: "forge_change_request", item: pr101 }, + ]); expect(result.current.isResolving).toBe(false); vi.useRealTimers(); }); diff --git a/packages/app/src/composer/github/auto-attach.ts b/packages/app/src/composer/github/auto-attach.ts index d73e1dc69..5cb19e47f 100644 --- a/packages/app/src/composer/github/auto-attach.ts +++ b/packages/app/src/composer/github/auto-attach.ts @@ -36,35 +36,64 @@ interface ComposerGithubAutoAttachResult { markGithubAttachmentRemoved: (attachment: ComposerAttachment | undefined) => void; } +interface ActiveGithubLookup { + invalidate: (keys: readonly string[]) => void; + invalidateIrrelevant: (current: ComposerGithubAutoAttachInput) => void; + hasPending: (key: string) => boolean; +} + export function useComposerGithubAutoAttach( params: ComposerGithubAutoAttachInput, ): ComposerGithubAutoAttachResult { const queryClient = useQueryClient(); const latestRef = useRef(params); const removedRefKeysRef = useRef(new Set()); - const pendingRefKeysRef = useRef(new Set()); const presentPullRequestKeysRef = useRef(new Set()); + const activeLookupsRef = useRef(new Set()); const previousTargetRef = useRef({ serverId: params.serverId, cwd: params.cwd }); const [resolvingRefCounts, setResolvingRefCounts] = useState>( () => new Map(), ); latestRef.current = params; + const lookupCandidateKey = getLookupCandidateKey(params, removedRefKeysRef.current); + const lookupRelevanceKey = getLookupRelevanceKey(params, removedRefKeysRef.current); + const presentPullRequestKey = getPresentPullRequestKey(params); + const hasClient = params.client !== null; useEffect(() => { - suppressRefsCarriedAcrossTargets({ - params: latestRef.current, - previousTargetRef, - removedRefKeys: removedRefKeysRef.current, - }); notifyNewPullRequestRefs({ params: latestRef.current, presentPullRequestKeysRef, }); + }, [presentPullRequestKey]); + + useEffect(() => { + const current = latestRef.current; + for (const activeLookup of activeLookupsRef.current) { + activeLookup.invalidateIrrelevant(current); + } + }, [ + lookupRelevanceKey, + params.remoteUrl, + hasClient, + params.isConnected, + params.serverId, + params.cwd, + ]); + + useEffect(() => { + const initial = latestRef.current; + const removedRefKeys = removedRefKeysRef.current; + suppressRefsCarriedAcrossTargets({ + params: initial, + previousTargetRef, + removedRefKeys, + }); const refs = refsReadyForLookup({ params: latestRef.current, - removedRefKeys: removedRefKeysRef.current, - pendingRefKeys: pendingRefKeysRef.current, + removedRefKeys, + activeLookups: activeLookupsRef.current, }); if (refs.length === 0) { return; @@ -72,36 +101,49 @@ export function useComposerGithubAutoAttach( const refKeys = refs.map(githubRefKey); setResolvingRefCounts((current) => addKeys(current, refKeys)); - let resolvingReleased = false; - const releaseResolving = () => { - if (resolvingReleased) return; - resolvingReleased = true; - clearResolvingKeys(setResolvingRefCounts, refKeys); + const unreleasedRefKeys = new Set(refKeys); + let lookupStarted = false; + let lookup: ActiveGithubLookup | null = null; + const releaseResolving = (keys: readonly string[]) => { + const keysToRelease = keys.filter((key) => unreleasedRefKeys.delete(key)); + if (keysToRelease.length === 0) return; + clearResolvingKeys(setResolvingRefCounts, keysToRelease); }; const timerId = setTimeout(() => { - void attachRefs({ + lookupStarted = true; + lookup = attachRefs({ refs, + initial, queryClient, latestRef, - removedRefKeys: removedRefKeysRef.current, - pendingRefKeys: pendingRefKeysRef.current, - }).finally(releaseResolving); + removedRefKeys, + onSettled: (key) => releaseResolving([key]), + onComplete: (completedLookup) => activeLookupsRef.current.delete(completedLookup), + }); + activeLookupsRef.current.add(lookup); }, AUTO_ATTACH_DEBOUNCE_MS); return () => { clearTimeout(timerId); - releaseResolving(); + if (!lookupStarted) { + releaseResolving([...unreleasedRefKeys]); + return; + } + const current = latestRef.current; + if (didLookupSourceOrderChange(refKeys, current, removedRefKeys)) { + lookup?.invalidate(refKeys); + } else { + lookup?.invalidateIrrelevant(current); + } }; }, [ - params.text, + lookupCandidateKey, params.remoteUrl, - params.attachments, - params.client, + hasClient, params.isConnected, params.serverId, params.cwd, - params.supportsForgeSearch, queryClient, ]); @@ -121,6 +163,73 @@ export function useComposerGithubAutoAttach( ); } +function getLookupCandidateKey( + params: ComposerGithubAutoAttachInput, + removedRefKeys: ReadonlySet, +): string { + return getLookupCandidateRefs(params, removedRefKeys).map(githubRefKey).join("|"); +} + +function getLookupCandidateRefs( + params: ComposerGithubAutoAttachInput, + removedRefKeys: ReadonlySet, +): GithubRef[] { + return extractGithubRefs(params.text, params.remoteUrl).filter((ref) => { + const key = githubRefKey(ref); + return !removedRefKeys.has(key) && !hasGithubAttachment(params.attachments, ref); + }); +} + +function didLookupSourceOrderChange( + originalKeys: readonly string[], + current: ComposerGithubAutoAttachInput, + removedRefKeys: ReadonlySet, +): boolean { + const currentKeys = getLookupCandidateRefs(current, removedRefKeys).map(githubRefKey); + const currentKeySet = new Set(currentKeys); + const retainedOriginalKeys = originalKeys.filter((key) => currentKeySet.has(key)); + return currentKeys.join("|") !== retainedOriginalKeys.join("|"); +} + +function getLookupRelevanceKey( + params: ComposerGithubAutoAttachInput, + removedRefKeys: ReadonlySet, +): string { + return extractGithubRefs(params.text, params.remoteUrl) + .map(githubRefKey) + .filter((key) => !removedRefKeys.has(key)) + .sort() + .join("|"); +} + +function getPresentPullRequestKey(params: ComposerGithubAutoAttachInput): string { + return extractGithubRefs(params.text, params.remoteUrl) + .filter((ref) => ref.kind === "pull") + .map(githubRefKey) + .sort() + .join("|"); +} + +function isLookupContextStillRelevant({ + ref, + initial, + current, + removedRefKeys, +}: { + ref: GithubRef; + initial: ComposerGithubAutoAttachInput; + current: ComposerGithubAutoAttachInput; + removedRefKeys: ReadonlySet; +}): boolean { + return ( + current.client !== null && + current.isConnected && + !removedRefKeys.has(githubRefKey(ref)) && + isSameLookupTarget(initial, current) && + isRefStillPresent(ref, current) + ); +} + function suppressRefsCarriedAcrossTargets({ params, previousTargetRef, @@ -192,31 +301,99 @@ function clearResolvingKeys( setResolvingRefCounts((current) => removeKeys(current, keys)); } -async function attachRefs({ +function attachRefs({ refs, + initial, queryClient, latestRef, removedRefKeys, - pendingRefKeys, + onSettled, + onComplete, }: { refs: GithubRef[]; + initial: ComposerGithubAutoAttachInput; queryClient: QueryClient; latestRef: RefObject; removedRefKeys: Set; - pendingRefKeys: Set; -}): Promise { - for (const ref of refs) { + onSettled: (key: string) => void; + onComplete: (lookup: ActiveGithubLookup) => void; +}): ActiveGithubLookup { + const outcomes = refs.map(() => ({ + settled: false, + item: null as ForgeSearchItem | null, + })); + let nextOutcomeIndex = 0; + let didComplete = false; + const drainOutcomes = () => { + while (outcomes[nextOutcomeIndex]?.settled) { + const item = outcomes[nextOutcomeIndex].item; + const ref = refs[nextOutcomeIndex]; + nextOutcomeIndex += 1; + if (item) { + latestRef.current.setAttachments((attachments) => { + if ( + removedRefKeys.has(githubRefKey(ref)) || + isAttachmentSelectedForGithubItem(attachments, item) + ) { + return attachments; + } + return toggleGithubAttachment(attachments, item); + }); + } + if (item?.kind === "change_request") { + latestRef.current.onPullRequestAdded?.(item); + } + } + if (!didComplete && nextOutcomeIndex === outcomes.length) { + didComplete = true; + onComplete(lookup); + } + }; + const settleOutcome = (index: number, item: ForgeSearchItem | null): boolean => { + if (outcomes[index].settled) return false; + outcomes[index] = { settled: true, item }; + drainOutcomes(); + return true; + }; + + refs.forEach((ref, index) => { const key = githubRefKey(ref); - if (pendingRefKeys.has(key)) { - continue; - } - pendingRefKeys.add(key); - try { - await attachRef({ ref, key, queryClient, latestRef, removedRefKeys }); - } finally { - pendingRefKeys.delete(key); - } - } + void attachRef({ ref, key, queryClient, latestRef, removedRefKeys }) + .then((item) => settleOutcome(index, item)) + .finally(() => { + onSettled(key); + }); + }); + + const lookup: ActiveGithubLookup = { + invalidate(keys) { + const invalidKeys = new Set(keys); + const newlySettledKeys: string[] = []; + refs.forEach((ref, index) => { + const key = githubRefKey(ref); + if (!invalidKeys.has(key) || index < nextOutcomeIndex) return; + if (!outcomes[index].settled) newlySettledKeys.push(key); + outcomes[index] = { settled: true, item: null }; + }); + for (const key of newlySettledKeys) onSettled(key); + drainOutcomes(); + }, + invalidateIrrelevant(current) { + lookup.invalidate( + refs.flatMap((ref, index) => + isLookupContextStillRelevant({ ref, initial, current, removedRefKeys }) && + (outcomes[index].settled || !hasGithubAttachment(current.attachments, ref)) + ? [] + : [githubRefKey(ref)], + ), + ); + }, + hasPending(key) { + const index = refs.findIndex((ref) => githubRefKey(ref) === key); + return index >= 0 && !outcomes[index].settled; + }, + }; + return lookup; } async function attachRef({ @@ -231,15 +408,15 @@ async function attachRef({ queryClient: QueryClient; latestRef: RefObject; removedRefKeys: Set; -}): Promise { +}): Promise { const snapshot = latestRef.current; if (!snapshot.client || !snapshot.isConnected || !isRefStillPresent(ref, snapshot)) { - return; + return null; } const search = await fetchGithubRefSearch({ ref, snapshot, queryClient }); if (!search) { - return; + return null; } const item = search.items.find((candidate) => githubItemMatchesRef(candidate, ref)); const current = latestRef.current; @@ -249,31 +426,23 @@ async function attachRef({ !isSameLookupTarget(snapshot, current) || !isRefStillPresent(ref, current) ) { - return; + return null; } if (isAttachmentSelectedForGithubItem(current.attachments, item)) { - return; - } - current.setAttachments((attachments) => { - if (removedRefKeys.has(key) || isAttachmentSelectedForGithubItem(attachments, item)) { - return attachments; - } - return toggleGithubAttachment(attachments, item); - }); - if (item.kind === "change_request") { - current.onPullRequestAdded?.(item); + return null; } + return item; } function refsReadyForLookup({ params, removedRefKeys, - pendingRefKeys, + activeLookups, }: { params: ComposerGithubAutoAttachInput; removedRefKeys: Set; - pendingRefKeys: Set; + activeLookups: ReadonlySet; }): GithubRef[] { if (!params.client || !params.isConnected || params.cwd.trim().length === 0) { return []; @@ -283,7 +452,7 @@ function refsReadyForLookup({ const key = githubRefKey(ref); return ( !removedRefKeys.has(key) && - !pendingRefKeys.has(key) && + ![...activeLookups].some((lookup) => lookup.hasPending(key)) && !hasGithubAttachment(params.attachments, ref) ); }); diff --git a/packages/app/src/composer/index.tsx b/packages/app/src/composer/index.tsx index 9a535d9e2..22873a2d2 100644 --- a/packages/app/src/composer/index.tsx +++ b/packages/app/src/composer/index.tsx @@ -2026,11 +2026,9 @@ export function Composer({ const messageInputContainerRef = useRef(null); - const isSubmitBusy = - isProcessing || - isSubmitLoading || - isUploadingFile || - (waitForGithubAutoAttachOnSubmit && githubAutoAttach.isResolving); + const isSubmitLoadingVisible = isProcessing || isSubmitLoading || isUploadingFile; + const isSubmitDisabled = + isSubmitLoadingVisible || (waitForGithubAutoAttachOnSubmit && githubAutoAttach.isResolving); // Disable drops while submitting/uploading: the submit path clears and restores attachments, // so a drop in that window would be lost or land on a locked draft. `disabled` hides the @@ -2041,7 +2039,7 @@ export function Composer({ onGenericFiles: handleGenericFilesDropped, onWorkspaceFile: handleWorkspaceFileDropped, }, - { disabled: isSubmitBusy }, + { disabled: isSubmitLoadingVisible }, ); const messageInputAutoFocus = autoFocus && isDesktopWebBreakpoint; @@ -2089,8 +2087,8 @@ export function Composer({ submitButtonAccessibilityLabel={submitButtonAccessibilityLabel} submitButtonTestID={submitButtonTestID} submitIcon={submitIcon} - isSubmitDisabled={isSubmitBusy} - isSubmitLoading={isSubmitBusy} + isSubmitDisabled={isSubmitDisabled} + isSubmitLoading={isSubmitLoadingVisible} preserveHeightOnSubmit={submitBehavior === "preserve-and-lock"} attachments={selectedAttachments} cwd={cwd}