Skip to content

Commit b1a8dfc

Browse files
fix: ignore stale GitHub Skill Sync previews
1 parent f8af9e7 commit b1a8dfc

3 files changed

Lines changed: 110 additions & 5 deletions

File tree

src/components/GitHubSkillSyncConfiguration.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Input } from "./ui/input";
66
import { Label } from "./ui/label";
77
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "./ui/select";
88

9-
export type GitHubSkillSyncPublisherOption = {
9+
type GitHubSkillSyncPublisherOption = {
1010
publisher: {
1111
_id: Id<"publishers">;
1212
handle: string;
@@ -27,7 +27,7 @@ export type GitHubSkillSyncRepository = {
2727
unavailableReason: "disabled" | null;
2828
};
2929

30-
export type GitHubSkillSyncPreviewItem = {
30+
type GitHubSkillSyncPreviewItem = {
3131
slug: string;
3232
displayName: string;
3333
path: string;
@@ -297,6 +297,7 @@ function classificationLabel(classification: GitHubSkillSyncPreviewItem["classif
297297
case "ownership-conflict":
298298
return "Ownership conflict";
299299
}
300+
return assertNever(classification);
300301
}
301302

302303
function classificationTone(classification: GitHubSkillSyncPreviewItem["classification"]) {
@@ -309,6 +310,11 @@ function classificationTone(classification: GitHubSkillSyncPreviewItem["classifi
309310
case "ownership-conflict":
310311
return "text-status-error-fg";
311312
}
313+
return assertNever(classification);
314+
}
315+
316+
function assertNever(value: never): never {
317+
throw new Error(`Unsupported GitHub Skill Sync classification: ${String(value)}`);
312318
}
313319

314320
function previewReasonLabel(reason: string) {

src/routes/-settings.test.tsx

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* @vitest-environment jsdom */
2-
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
2+
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
33
import { getFunctionName } from "convex/server";
44
import type { FunctionReturnType } from "convex/server";
55
import type { ReactNode } from "react";
@@ -801,6 +801,93 @@ describe("Settings", () => {
801801
).toBe(true);
802802
});
803803

804+
it("ignores a preview response after the selected repository changes", async () => {
805+
const listRepositories = vi.fn().mockResolvedValue({
806+
publisher: { _id: "publisher_patrick", handle: "patrick", kind: "user" },
807+
page: 1,
808+
perPage: 100,
809+
hasMore: false,
810+
repositories: [
811+
{
812+
repositoryId: "1",
813+
repo: "patrick-erichsen/skills",
814+
ownerId: "123",
815+
ownerLogin: "patrick-erichsen",
816+
defaultBranch: "main",
817+
archived: false,
818+
disabled: false,
819+
fork: false,
820+
pushedAt: "2026-07-23T12:00:00Z",
821+
selectable: true,
822+
unavailableReason: null,
823+
},
824+
],
825+
});
826+
let resolvePreview: ((value: unknown) => void) | undefined;
827+
const previewRepository = vi.fn(
828+
() =>
829+
new Promise((resolve) => {
830+
resolvePreview = resolve;
831+
}),
832+
);
833+
useActionMock.mockImplementation((action) => {
834+
const actionName = getFunctionName(action);
835+
if (actionName === "githubSkillSyncSettings:listRepositories") return listRepositories;
836+
if (actionName === "githubSkillSyncSettings:previewRepository") return previewRepository;
837+
return vi.fn();
838+
});
839+
mockSignedInSettings({
840+
search: { view: "githubSources" },
841+
memberships: [personalMembership],
842+
});
843+
844+
render(<Settings />);
845+
846+
await waitFor(() => expect(listRepositories).toHaveBeenCalled());
847+
fireEvent.click(screen.getByRole("button", { name: "Preview repository" }));
848+
await waitFor(() => expect(previewRepository).toHaveBeenCalled());
849+
850+
fireEvent.change(screen.getByLabelText("Repository URL"), {
851+
target: { value: "patrick-erichsen/other-skills" },
852+
});
853+
await act(async () => {
854+
resolvePreview?.({
855+
publisher: { _id: "publisher_patrick", handle: "patrick", kind: "user" },
856+
repository: {
857+
requestedRepo: "patrick-erichsen/skills",
858+
repositoryId: "1",
859+
repo: "patrick-erichsen/skills",
860+
redirected: false,
861+
defaultBranch: "main",
862+
commit: "a".repeat(40),
863+
},
864+
summary: {
865+
total: 1,
866+
newDestinations: 1,
867+
replacements: 0,
868+
unavailable: 0,
869+
conflicts: 0,
870+
},
871+
items: [
872+
{
873+
slug: "html",
874+
displayName: "HTML",
875+
path: "skills/html",
876+
contentHash: "hash-html",
877+
classification: "new-destination",
878+
eligible: true,
879+
destination: null,
880+
},
881+
],
882+
});
883+
});
884+
885+
expect(screen.getByLabelText<HTMLInputElement>("Repository URL").value).toBe(
886+
"patrick-erichsen/other-skills",
887+
);
888+
expect(screen.queryByRole("heading", { name: "Repository preview" })).toBeNull();
889+
});
890+
804891
it("shows synced repos as separate cards and lets owners delete a source", async () => {
805892
const deleteSource = vi.fn().mockResolvedValue({ ok: true, deletedSkills: 0 });
806893
useMutationMock.mockImplementation((mutation) =>

src/routes/settings.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
type FormEvent,
3737
type ReactNode,
3838
useEffect,
39+
useRef,
3940
useState,
4041
} from "react";
4142
import { toast } from "sonner";
@@ -331,6 +332,7 @@ export function Settings() {
331332
const [isLoadingGitHubRepositories, setIsLoadingGitHubRepositories] = useState(false);
332333
const [githubSyncPreview, setGitHubSyncPreview] = useState<GitHubSkillSyncPreview | null>(null);
333334
const [isPreviewingGitHubSource, setIsPreviewingGitHubSource] = useState(false);
335+
const githubSyncPreviewRequestId = useRef(0);
334336
const [deletingSourceId, setDeletingSourceId] = useState<Id<"githubSkillSources"> | null>(null);
335337
const [sourceToDelete, setSourceToDelete] = useState<GitHubSkillSource | null>(null);
336338
const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
@@ -472,7 +474,7 @@ export function Settings() {
472474
) {
473475
setGitHubRepositories([]);
474476
setGitHubRepositoriesError(null);
475-
return;
477+
return undefined;
476478
}
477479
let cancelled = false;
478480
setIsLoadingGitHubRepositories(true);
@@ -788,19 +790,25 @@ export function Settings() {
788790
if (!selectedSourcePublisher) return;
789791
const repo = parseGitHubRepoInput(githubRepo);
790792
if (!repo) return;
793+
const requestId = githubSyncPreviewRequestId.current + 1;
794+
githubSyncPreviewRequestId.current = requestId;
791795
setIsPreviewingGitHubSource(true);
792796
setGitHubSyncPreview(null);
793797
try {
794798
const result = await previewGitHubSyncRepository({
795799
publisherId: selectedSourcePublisher.publisher._id,
796800
repo,
797801
});
802+
if (githubSyncPreviewRequestId.current !== requestId) return;
798803
setGithubRepo(result.repository.repo);
799804
setGitHubSyncPreview(result as GitHubSkillSyncPreview);
800805
} catch (error) {
806+
if (githubSyncPreviewRequestId.current !== requestId) return;
801807
toast.error(getUserFacingConvexError(error, "GitHub repository could not be previewed."));
802808
} finally {
803-
setIsPreviewingGitHubSource(false);
809+
if (githubSyncPreviewRequestId.current === requestId) {
810+
setIsPreviewingGitHubSource(false);
811+
}
804812
}
805813
}
806814

@@ -1706,17 +1714,21 @@ export function Settings() {
17061714
publisherOptions={githubSourcePublishers}
17071715
selectedPublisherId={selectedSourcePublisher.publisher._id}
17081716
onPublisherChange={(publisherId) => {
1717+
githubSyncPreviewRequestId.current += 1;
17091718
setSelectedSourcePublisherId(publisherId);
17101719
setGithubRepo("");
17111720
setGitHubSyncPreview(null);
1721+
setIsPreviewingGitHubSource(false);
17121722
}}
17131723
repositories={githubRepositories}
17141724
repositoriesError={githubRepositoriesError}
17151725
isLoadingRepositories={isLoadingGitHubRepositories}
17161726
githubRepo={githubRepo}
17171727
onGithubRepoChange={(repo) => {
1728+
githubSyncPreviewRequestId.current += 1;
17181729
setGithubRepo(repo);
17191730
setGitHubSyncPreview(null);
1731+
setIsPreviewingGitHubSource(false);
17201732
}}
17211733
onPreview={onPreviewGitHubSource}
17221734
isPreviewing={isPreviewingGitHubSource}

0 commit comments

Comments
 (0)