mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-14 13:22:37 +00:00
Keep background composer work out of creation progress (#2794)
* fix(composer): keep background context out of loading state Pull request link resolution can block a new workspace until its checkout target is known, but it is not submission work. Keep that safety gate while reserving the send-button spinner and drop lockout for actual submission or upload progress. * test(composer): cover link resolution at browser boundary Exercise the real new-workspace composer while pull request context resolves, including the submit presentation and file-drop path. Keep the two presentation booleans local to their only caller instead of exporting policy solely for an internal-state unit test. * fix(composer): preserve background lookup ownership Composer edits could release the resolving gate while a pull request lookup still owned the request, briefly allowing workspace creation from the wrong branch. Track logical candidates independently of unrelated draft changes and settle each reference independently so only invalid work releases early. * fix(composer): preserve pasted PR selection order * fix(composer): renew re-added PR lookups * fix(composer): track pasted PR source order * fix(composer): preserve resolved attachment order * fix(composer): coordinate active lookup order
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<ForgeSearchPayload>();
|
||||
const secondLookup = deferred<ForgeSearchPayload>();
|
||||
@@ -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<ForgeSearchPayload>();
|
||||
const secondLookup = deferred<ForgeSearchPayload>();
|
||||
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<ForgeSearchPayload>();
|
||||
const secondLookup = deferred<ForgeSearchPayload>();
|
||||
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<ForgeSearchPayload>();
|
||||
const secondLookup = deferred<ForgeSearchPayload>();
|
||||
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<ForgeSearchPayload>();
|
||||
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<ForgeSearchPayload>();
|
||||
const secondLookup = deferred<ForgeSearchPayload>();
|
||||
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<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.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();
|
||||
});
|
||||
|
||||
@@ -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<string>());
|
||||
const pendingRefKeysRef = useRef(new Set<string>());
|
||||
const presentPullRequestKeysRef = useRef(new Set<string>());
|
||||
const activeLookupsRef = useRef(new Set<ActiveGithubLookup>());
|
||||
const previousTargetRef = useRef({ serverId: params.serverId, cwd: params.cwd });
|
||||
const [resolvingRefCounts, setResolvingRefCounts] = useState<ReadonlyMap<string, number>>(
|
||||
() => 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>,
|
||||
): string {
|
||||
return getLookupCandidateRefs(params, removedRefKeys).map(githubRefKey).join("|");
|
||||
}
|
||||
|
||||
function getLookupCandidateRefs(
|
||||
params: ComposerGithubAutoAttachInput,
|
||||
removedRefKeys: ReadonlySet<string>,
|
||||
): 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<string>,
|
||||
): 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>,
|
||||
): 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<string>;
|
||||
}): 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<ComposerGithubAutoAttachInput>;
|
||||
removedRefKeys: Set<string>;
|
||||
pendingRefKeys: Set<string>;
|
||||
}): Promise<void> {
|
||||
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<ComposerGithubAutoAttachInput>;
|
||||
removedRefKeys: Set<string>;
|
||||
}): Promise<void> {
|
||||
}): Promise<ForgeSearchItem | null> {
|
||||
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<string>;
|
||||
pendingRefKeys: Set<string>;
|
||||
activeLookups: ReadonlySet<ActiveGithubLookup>;
|
||||
}): 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)
|
||||
);
|
||||
});
|
||||
|
||||
@@ -2026,11 +2026,9 @@ export function Composer({
|
||||
|
||||
const messageInputContainerRef = useRef<View>(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}
|
||||
|
||||
Reference in New Issue
Block a user