Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions apps/dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
"@diffkit/ui": "workspace:*",
"@pierre/diffs": "^1.1.12",
"@tailwindcss/vite": "^4.1.18",
"@tanstack/react-devtools": "latest",
"@tanstack/react-query": "latest",
"@tanstack/react-router": "latest",
"@tanstack/react-router-devtools": "latest",
"@tanstack/react-router-ssr-query": "latest",
"@tanstack/react-start": "latest",
"@tanstack/router-plugin": "^1.132.0",
"@tanstack/react-devtools": "~0.10.2",
"@tanstack/react-query": "~5.97.0",
"@tanstack/react-router": "~1.168.13",
"@tanstack/react-router-devtools": "~1.166.11",
"@tanstack/react-router-ssr-query": "~1.166.10",
"@tanstack/react-start": "~1.167.23",
"@tanstack/router-plugin": "~1.167.12",
"agentation": "^3.0.2",
"better-auth": "^1.6.0",
"drizzle-orm": "^0.45.2",
Expand All @@ -47,7 +47,7 @@
"@biomejs/biome": "2.4.5",
"@cloudflare/workers-types": "^4.20260405.1",
"@diffkit/typescript-config": "workspace:*",
"@tanstack/devtools-vite": "latest",
"@tanstack/devtools-vite": "~0.6.0",
"@testing-library/dom": "^10.4.1",
"@testing-library/react": "^16.3.0",
"@types/node": "^22.10.2",
Expand Down
34 changes: 20 additions & 14 deletions apps/dashboard/src/components/layouts/dashboard-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,20 @@ export function DashboardLayout() {
...githubMyIssuesQueryOptions(scope),
enabled: hasMounted,
});
const pullCount = pullsQuery.data
? pullsQuery.data.reviewRequested.length +
pullsQuery.data.assigned.length +
pullsQuery.data.authored.length +
pullsQuery.data.mentioned.length +
pullsQuery.data.involved.length
: undefined;
const issueCount = issuesQuery.data
? issuesQuery.data.assigned.length +
issuesQuery.data.authored.length +
issuesQuery.data.mentioned.length
: undefined;
const pullCount =
hasMounted && pullsQuery.data
? pullsQuery.data.reviewRequested.length +
pullsQuery.data.assigned.length +
pullsQuery.data.authored.length +
pullsQuery.data.mentioned.length +
pullsQuery.data.involved.length
: undefined;
const issueCount =
hasMounted && issuesQuery.data
? issuesQuery.data.assigned.length +
issuesQuery.data.authored.length +
issuesQuery.data.mentioned.length
: undefined;
const tabsReady = hasMounted && Boolean(pullsQuery.data && issuesQuery.data);

useEffect(() => {
Expand All @@ -87,7 +89,9 @@ export function DashboardLayout() {
counts={{
pulls: pullCount,
issues: issueCount,
reviews: pullsQuery.data?.reviewRequested.length,
reviews: hasMounted
? pullsQuery.data?.reviewRequested.length
: undefined,
}}
/>
<div className="flex flex-1 flex-col overflow-hidden p-2 pt-0">
Expand All @@ -104,7 +108,9 @@ export function DashboardLayout() {
counts={{
pulls: pullCount,
issues: issueCount,
reviews: pullsQuery.data?.reviewRequested.length,
reviews: hasMounted
? pullsQuery.data?.reviewRequested.length
: undefined,
}}
/>
<Suspense>
Expand Down
20 changes: 17 additions & 3 deletions apps/dashboard/src/components/layouts/dashboard-topbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,23 @@ export function DashboardTopbar({
useEffect(() => {
if (!tabsReady) return;

void Promise.allSettled(
primaryNavRoutes.map((to) => routerRef.current.preloadRoute({ to })),
);
// Preload routes serially to avoid a burst of concurrent server function
// RPCs that can overwhelm the Cloudflare Worker.
let cancelled = false;
(async () => {
for (const to of primaryNavRoutes) {
if (cancelled) break;
try {
await routerRef.current.preloadRoute({ to });
} catch {
// preload is best-effort
}
}
})();

return () => {
cancelled = true;
};
}, [tabsReady]);

function navigateToTab(tab: Tab | undefined) {
Expand Down
15 changes: 2 additions & 13 deletions apps/dashboard/src/components/pulls/review/review-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ function useIsDesktop() {

export function ReviewPage() {
const { user } = routeApi.useRouteContext();
const loaderData = routeApi.useLoaderData();
const { owner, repo, pullId } = routeApi.useParams();
const pullNumber = Number(pullId);
const scope = { userId: user.id };
Expand All @@ -110,7 +109,6 @@ export function ReviewPage() {
refetchOnWindowFocus: false,
});

const firstFilesPage = loaderData?.firstFilesPage ?? null;
const filesQuery = useInfiniteQuery({
queryKey: githubQueryKeys.pulls.files(scope, input),
initialPageParam: 1,
Expand All @@ -123,14 +121,6 @@ export function ReviewPage() {
},
}),
getNextPageParam: (lastPage) => lastPage.nextPage ?? undefined,
...(firstFilesPage
? {
initialData: {
pages: [firstFilesPage],
pageParams: [1],
},
}
: {}),
refetchOnMount: false,
refetchOnWindowFocus: false,
});
Expand All @@ -142,9 +132,8 @@ export function ReviewPage() {
refetchOnWindowFocus: false,
});

const pr = pageQuery.data?.detail ?? loaderData?.pageData?.detail ?? null;
const sidebarFiles =
fileSummariesQuery.data ?? loaderData?.fileSummaries ?? [];
const pr = pageQuery.data?.detail ?? null;
const sidebarFiles = fileSummariesQuery.data ?? [];
const diffFiles = useMemo(
() => filesQuery.data?.pages.flatMap((page) => page.files) ?? [],
[filesQuery.data],
Expand Down
13 changes: 11 additions & 2 deletions apps/dashboard/src/lib/auth-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getGitHubAppUserAccessTokenByUserId,
getGitHubOAuthConfig,
} from "./github-app.server";
import { configureGitHubRequestPolicies } from "./github-request-policy";

const authDb = drizzle(env.DB, { schema });

Expand Down Expand Up @@ -52,11 +53,15 @@ export async function getRequestSession() {
export async function getGitHubClientByUserId(
userId: string,
): Promise<OctokitType> {
return new Octokit({
const octokit = new Octokit({
auth: await getGitHubAccessTokenByUserId(userId),
retry: { enabled: false },
throttle: { enabled: false },
});

configureGitHubRequestPolicies(octokit);

return octokit;
}

export async function getGitHubAppUserClientByUserId(
Expand All @@ -67,9 +72,13 @@ export async function getGitHubAppUserClientByUserId(
return null;
}

return new Octokit({
const octokit = new Octokit({
auth: token,
retry: { enabled: false },
throttle: { enabled: false },
});

configureGitHubRequestPolicies(octokit);

return octokit;
}
2 changes: 2 additions & 0 deletions apps/dashboard/src/lib/github-app.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { and, eq } from "drizzle-orm";
import { getDb } from "../db";
import { account } from "../db/schema";
import { normalizeGitHubAppPrivateKey } from "./github-private-key";
import { GITHUB_REQUEST_TIMEOUT_MS } from "./github-request-policy";

type WorkerEnvRecord = typeof env & Record<string, string | undefined>;

Expand Down Expand Up @@ -150,6 +151,7 @@ async function requestGitHubAppUserToken(params: Record<string, string>) {
Accept: "application/json",
"Content-Type": "application/x-www-form-urlencoded",
},
signal: AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS),
body: new URLSearchParams(params),
});
const payload = (await response.json()) as GitHubTokenResponse;
Expand Down
35 changes: 35 additions & 0 deletions apps/dashboard/src/lib/github-request-policy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { Octokit as OctokitType } from "octokit";

const GITHUB_READ_RETRY_COUNT = 1;
export const GITHUB_REQUEST_TIMEOUT_MS = 12_000;

type GitHubRequestOptions = Parameters<
OctokitType["hook"]["before"]
>[1] extends (options: infer Options) => unknown
? Options & {
method?: string;
request?: {
retries?: number;
signal?: AbortSignal;
};
}
: never;

function isSafeGitHubRetryMethod(method: string | undefined) {
return method === "GET" || method === "HEAD" || method === "OPTIONS";
}

function createGitHubRequestTimeoutSignal() {
return AbortSignal.timeout(GITHUB_REQUEST_TIMEOUT_MS);
}

export function configureGitHubRequestPolicies(octokit: OctokitType) {
octokit.hook.before("request", (options: GitHubRequestOptions) => {
const requestOptions = options.request ?? {};
options.request = requestOptions;
requestOptions.retries = isSafeGitHubRetryMethod(options.method)
? GITHUB_READ_RETRY_COUNT
: 0;
requestOptions.signal ??= createGitHubRequestTimeoutSignal();
});
}
16 changes: 2 additions & 14 deletions apps/dashboard/src/lib/github.query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ export function githubPullDetailQueryOptions(
queryFn: () => getPullFromRepo({ data: input }),
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -265,10 +264,8 @@ export function githubPullPageQueryOptions(
return queryOptions({
queryKey: githubQueryKeys.pulls.page(scope, input),
queryFn: () => getPullPageData({ data: input }),
staleTime: githubCachePolicy.activity.staleTimeMs,
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
refetchOnWindowFocus: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -282,7 +279,6 @@ export function githubPullCommentsQueryOptions(
queryFn: () => getPullComments({ data: input }),
staleTime: githubCachePolicy.activity.staleTimeMs,
gcTime: githubCachePolicy.activity.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -296,7 +292,6 @@ export function githubPullStatusQueryOptions(
queryFn: () => getPullStatus({ data: input }),
staleTime: githubCachePolicy.status.staleTimeMs,
gcTime: githubCachePolicy.status.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -310,7 +305,6 @@ export function githubPullFilesQueryOptions(
queryFn: () => getPullFiles({ data: input }),
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -324,7 +318,6 @@ export function githubPullFileSummariesQueryOptions(
queryFn: () => getPullFileSummaries({ data: input }),
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -338,7 +331,6 @@ export function githubPullReviewCommentsQueryOptions(
queryFn: () => getPullReviewComments({ data: input }),
staleTime: githubCachePolicy.activity.staleTimeMs,
gcTime: githubCachePolicy.activity.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand Down Expand Up @@ -424,7 +416,6 @@ export function githubIssueDetailQueryOptions(
queryFn: () => getIssueFromRepo({ data: input }),
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -436,10 +427,8 @@ export function githubIssuePageQueryOptions(
return queryOptions({
queryKey: githubQueryKeys.issues.page(scope, input),
queryFn: () => getIssuePageData({ data: input }),
staleTime: githubCachePolicy.activity.staleTimeMs,
staleTime: githubCachePolicy.detail.staleTimeMs,
gcTime: githubCachePolicy.detail.gcTimeMs,
refetchOnMount: "always",
refetchOnWindowFocus: "always",
meta: tabPersistedMeta,
});
}
Expand All @@ -453,7 +442,6 @@ export function githubIssueCommentsQueryOptions(
queryFn: () => getIssueComments({ data: input }),
staleTime: githubCachePolicy.activity.staleTimeMs,
gcTime: githubCachePolicy.activity.gcTimeMs,
refetchOnMount: "always",
meta: tabPersistedMeta,
});
}
Expand Down
19 changes: 12 additions & 7 deletions apps/dashboard/src/lib/github.server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ beforeEach(() => {
});

describe("getGitHubClient", () => {
it("configures Octokit throttling and safe-method retries", async () => {
it("configures Octokit throttling, bounded retries, and request timeouts", async () => {
const { getGitHubClient } = await import("./github.server");

await getGitHubClient("user-123");
Expand Down Expand Up @@ -102,23 +102,28 @@ describe("getGitHubClient", () => {
expect(instance.hookBefore).toHaveBeenCalledTimes(1);
const [hookEvent, hookHandler] = instance.hookBefore.mock.calls[0] as [
string,
(options: { method?: string; request?: { retries?: number } }) => void,
(options: {
method?: string;
request?: { retries?: number; signal?: AbortSignal };
}) => void,
];
expect(hookEvent).toBe("request");

const getOptions = { method: "GET" } as {
method?: string;
request?: { retries?: number };
request?: { retries?: number; signal?: AbortSignal };
};
hookHandler(getOptions);
expect(getOptions.request?.retries).toBe(2);
expect(getOptions.request?.retries).toBe(1);
expect(getOptions.request?.signal).toBeInstanceOf(AbortSignal);

const postOptions = { method: "POST" } as {
method?: string;
request?: { retries?: number };
request?: { retries?: number; signal?: AbortSignal };
};
hookHandler(postOptions);
expect(postOptions.request?.retries).toBe(0);
expect(postOptions.request?.signal).toBeInstanceOf(AbortSignal);

expect(
options.throttle.onRateLimit(
Expand All @@ -129,7 +134,7 @@ describe("getGitHubClient", () => {
},
0,
),
).toBe(true);
).toBe(false);
expect(
options.throttle.onRateLimit(
30,
Expand All @@ -149,7 +154,7 @@ describe("getGitHubClient", () => {
),
).toBe(false);
expect(instance.log.warn).toHaveBeenCalled();
expect(instance.log.info).toHaveBeenCalledTimes(1);
expect(instance.log.info).not.toHaveBeenCalled();
});

it("creates GitHub App installation clients from app credentials", async () => {
Expand Down
Loading
Loading