diff --git a/packages/shared/src/repository.test.ts b/packages/shared/src/repository.test.ts new file mode 100644 index 0000000000..b964b9ae57 --- /dev/null +++ b/packages/shared/src/repository.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { getTaskRepository, parseRepository } from "./repository"; + +describe("getTaskRepository", () => { + it.each([ + { name: "undefined task", task: undefined, expected: null }, + { name: "null task", task: null, expected: null }, + { name: "missing repository", task: {}, expected: null }, + { name: "null repository", task: { repository: null }, expected: null }, + { + name: "populated repository", + task: { repository: "posthog/code" }, + expected: "posthog/code", + }, + ])("returns $expected for $name", ({ task, expected }) => { + expect(getTaskRepository(task)).toBe(expected); + }); +}); + +describe("parseRepository", () => { + it("splits an org/repo string", () => { + expect(parseRepository("posthog/code")).toEqual({ + organization: "posthog", + repoName: "code", + }); + }); + + it("returns null for malformed input", () => { + expect(parseRepository("code")).toBeNull(); + expect(parseRepository("a/b/c")).toBeNull(); + }); +}); diff --git a/packages/shared/src/repository.ts b/packages/shared/src/repository.ts index 2902588698..c7b70b5d5f 100644 --- a/packages/shared/src/repository.ts +++ b/packages/shared/src/repository.ts @@ -10,8 +10,8 @@ export const parseRepository = ( return { organization: result[0], repoName: result[1] }; }; -export function getTaskRepository(task: { - repository?: string | null; -}): string | null { - return task.repository ?? null; +export function getTaskRepository( + task: { repository?: string | null } | null | undefined, +): string | null { + return task?.repository ?? null; } diff --git a/packages/ui/src/features/task-detail/hooks/useRefreshedTask.test.tsx b/packages/ui/src/features/task-detail/hooks/useRefreshedTask.test.tsx index 9d11d1c35a..166e7f50a5 100644 --- a/packages/ui/src/features/task-detail/hooks/useRefreshedTask.test.tsx +++ b/packages/ui/src/features/task-detail/hooks/useRefreshedTask.test.tsx @@ -5,12 +5,24 @@ import type { PropsWithChildren } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { useRefreshedTask } from "./useRefreshedTask"; -const mocks = vi.hoisted(() => ({ getTask: vi.fn() })); +const mocks = vi.hoisted(() => ({ + getTask: vi.fn(), + useQuery: vi.fn(), + actualUseQuery: undefined as unknown, +})); vi.mock("@posthog/ui/features/auth/authClientImperative", () => ({ getAuthenticatedClient: vi.fn(async () => ({ getTask: mocks.getTask })), })); +vi.mock("@tanstack/react-query", async (importOriginal) => { + const actual = await importOriginal(); + // Keep a handle on the real useQuery so beforeEach can restore delegation + // after a test overrides the mock's return value. + mocks.actualUseQuery = actual.useQuery; + return { ...actual, useQuery: mocks.useQuery }; +}); + function task(runId: string, status: "failed" | "in_progress"): Task { return { id: "task-123", @@ -30,6 +42,27 @@ function task(runId: string, status: "failed" | "in_progress"): Task { describe("useRefreshedTask", () => { beforeEach(() => { mocks.getTask.mockReset(); + mocks.useQuery.mockReset(); + // Delegate to the real useQuery by default; a test can override the return + // value to simulate a specific cache state. + mocks.useQuery.mockImplementation( + mocks.actualUseQuery as typeof import("@tanstack/react-query").useQuery, + ); + }); + + it("falls back to initialTask when the query yields undefined data", () => { + // A separate observer subscribing to the same query key without initialData + // can create the cache entry first while the fetch is in flight, so React + // Query hands this hook `data: undefined` despite the initialData option. + // The hook must never surface undefined (it flows into getTaskRepository). + mocks.useQuery.mockReturnValue({ data: undefined }); + const initialTask = task("run-parent", "in_progress"); + + const { result } = renderHook(() => + useRefreshedTask("task-123", initialTask), + ); + + expect(result.current).toBe(initialTask); }); it("replaces a cached failed run with the authoritative resumed run", async () => { diff --git a/packages/ui/src/features/task-detail/hooks/useRefreshedTask.ts b/packages/ui/src/features/task-detail/hooks/useRefreshedTask.ts index e2f6be477d..f8ddc9a4b5 100644 --- a/packages/ui/src/features/task-detail/hooks/useRefreshedTask.ts +++ b/packages/ui/src/features/task-detail/hooks/useRefreshedTask.ts @@ -9,5 +9,8 @@ export function useRefreshedTask(taskId: string, initialTask: Task): Task { refetchOnMount: "always", }); - return data; + // Guard against `data` being undefined: another observer subscribing to the + // same query key without `initialData` can create the cache entry first while + // the fetch is in flight, causing React Query to drop this hook's initialData. + return data ?? initialTask; }