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
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,30 @@ function resolveCommentsPullRequestTarget({
};
}

function stripGitHubStatusTimestamp(
status: GitHubStatus | null | undefined,
): Omit<GitHubStatus, "lastRefreshed"> | null {
if (!status) {
return null;
}

const { lastRefreshed: _lastRefreshed, ...rest } = status;
return rest;
}

function hasMeaningfulGitHubStatusChange({
current,
next,
}: {
current: GitHubStatus | null | undefined;
next: GitHubStatus;
}): boolean {
return (
JSON.stringify(stripGitHubStatusTimestamp(current)) !==
JSON.stringify(stripGitHubStatusTimestamp(next))
);
}

export const createGitStatusProcedures = () => {
return router({
refreshGitStatus: publicProcedure
Expand Down Expand Up @@ -165,7 +189,13 @@ export const createGitStatusProcedures = () => {

const freshStatus = await fetchGitHubPRStatus(worktree.path);

if (freshStatus) {
if (
freshStatus &&
hasMeaningfulGitHubStatusChange({
current: worktree.githubStatus,
next: freshStatus,
})
) {
localDb
.update(worktrees)
.set({ githubStatus: freshStatus })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import {
getPRHeadBranchCandidates,
prMatchesLocalBranch,
} from "./pr-resolution";
import { getPullRequestRepoArgs } from "./repo-context";
import {
getPullRequestRepoArgs,
shouldRefreshCachedRepoContext,
} from "./repo-context";

describe("branchMatchesPR", () => {
test("matches same-repo branch exactly", () => {
Expand Down Expand Up @@ -70,6 +73,69 @@ describe("getPullRequestRepoArgs", () => {
});
});

describe("shouldRefreshCachedRepoContext", () => {
test("returns false when no cached repo context exists", () => {
expect(
shouldRefreshCachedRepoContext({
originUrl: "https://github.com/superset-sh/superset",
cachedRepoContext: null,
}),
).toBe(false);
});

test("returns false when the cached repo still matches origin", () => {
expect(
shouldRefreshCachedRepoContext({
originUrl: "https://github.com/superset-sh/superset",
cachedRepoContext: {
repoUrl: "https://github.com/superset-sh/superset",
upstreamUrl: "https://github.com/superset-sh/superset",
isFork: false,
},
}),
).toBe(false);
});

test("returns false when origin is missing", () => {
expect(
shouldRefreshCachedRepoContext({
originUrl: null,
cachedRepoContext: {
repoUrl: "https://github.com/superset-sh/superset",
upstreamUrl: "https://github.com/superset-sh/superset",
isFork: false,
},
}),
).toBe(false);
});

test("treats SSH and HTTPS forms of the same repo as equal", () => {
expect(
shouldRefreshCachedRepoContext({
originUrl: "git@github.com:Superset-Sh/superset.git",
cachedRepoContext: {
repoUrl: "https://github.com/superset-sh/superset",
upstreamUrl: "https://github.com/superset-sh/superset",
isFork: false,
},
}),
).toBe(false);
});

test("returns true when origin no longer matches the cached repo", () => {
expect(
shouldRefreshCachedRepoContext({
originUrl: "https://github.com/Kitenite/superset",
cachedRepoContext: {
repoUrl: "https://github.com/superset-sh/superset",
upstreamUrl: "https://github.com/superset-sh/superset",
isFork: false,
},
}),
).toBe(true);
});
});

describe("parseReviewThreadCommentsResponse", () => {
test("normalizes inline review-thread comments with file metadata", () => {
expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function getPullRequestCommentsRepoNameWithOwner(
async function resolvePullRequestCommentsTarget(
worktreePath: string,
): Promise<PullRequestCommentsTarget | null> {
const repoContext = await getRepoContext(worktreePath, { forceFresh: true });
const repoContext = await getRepoContext(worktreePath);
if (!repoContext) {
return null;
}
Expand Down Expand Up @@ -86,9 +86,7 @@ async function refreshGitHubPRStatus(
worktreePath: string,
): Promise<GitHubStatus | null> {
try {
const repoContext = await getRepoContext(worktreePath, {
forceFresh: true,
});
const repoContext = await getRepoContext(worktreePath);
if (!repoContext) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { execGitWithShellPath } from "../git-client";
import { execWithShellEnv } from "../shell-env";
import { readCachedRepoContext } from "./cache";
import { getCachedRepoContextState, readCachedRepoContext } from "./cache";
import { GHRepoResponseSchema, type RepoContext } from "./types";

async function refreshRepoContext(
Expand Down Expand Up @@ -65,15 +65,50 @@ export async function getRepoContext(
forceFresh?: boolean;
},
): Promise<RepoContext | null> {
const originUrl = await getOriginUrl(worktreePath);
const cachedRepoContext =
getCachedRepoContextState(worktreePath)?.value ?? null;
const forceFresh =
Boolean(options?.forceFresh) ||
shouldRefreshCachedRepoContext({
originUrl,
cachedRepoContext,
});

return readCachedRepoContext(
worktreePath,
() => refreshRepoContext(worktreePath),
{
forceFresh: options?.forceFresh,
forceFresh,
},
);
}

export function shouldRefreshCachedRepoContext({
originUrl,
cachedRepoContext,
}: {
originUrl: string | null;
cachedRepoContext: RepoContext | null;
}): boolean {
if (!cachedRepoContext) {
return false;
}

const normalizedOriginUrl = normalizeGitHubUrl(
originUrl ?? "",
)?.toLowerCase();
const normalizedCachedRepoUrl = normalizeGitHubUrl(
cachedRepoContext.repoUrl,
)?.toLowerCase();

if (!normalizedOriginUrl || !normalizedCachedRepoUrl) {
return false;
}

return normalizedCachedRepoUrl !== normalizedOriginUrl;
}

async function getOriginUrl(worktreePath: string): Promise<string | null> {
try {
const { stdout } = await execGitWithShellPath(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import { describe, expect, test } from "bun:test";
import {
getGitHubPRCommentsQueryPolicy,
getGitHubStatusQueryPolicy,
} from "./githubQueryPolicy";

describe("getGitHubStatusQueryPolicy", () => {
test("enables focus-only refresh for the active changes sidebar diffs view", () => {
expect(
getGitHubStatusQueryPolicy("changes-sidebar", {
hasWorkspaceId: true,
isActive: true,
isReviewTabActive: false,
}),
).toEqual({
enabled: true,
refetchInterval: false,
refetchOnWindowFocus: true,
staleTime: 0,
});
});

test("enables polling for the active changes sidebar review view", () => {
expect(
getGitHubStatusQueryPolicy("changes-sidebar", {
hasWorkspaceId: true,
isActive: true,
isReviewTabActive: true,
}),
).toEqual({
enabled: true,
refetchInterval: 10_000,
refetchOnWindowFocus: true,
staleTime: 10_000,
});
});

test("disables changes sidebar status when the surface is inactive", () => {
expect(
getGitHubStatusQueryPolicy("changes-sidebar", {
hasWorkspaceId: true,
isActive: false,
isReviewTabActive: true,
}),
).toEqual({
enabled: false,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 10_000,
});
});

test("keeps the workspace page active without interval polling", () => {
expect(
getGitHubStatusQueryPolicy("workspace-page", {
hasWorkspaceId: true,
isActive: true,
}),
).toEqual({
enabled: true,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 300_000,
});
});

test("keeps hover-card surfaces lazy without focus refresh", () => {
expect(
getGitHubStatusQueryPolicy("workspace-hover-card", {
hasWorkspaceId: true,
isActive: true,
}),
).toEqual({
enabled: true,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 300_000,
});
});

test("keeps workspace list items cheaper than full-page PR surfaces", () => {
expect(
getGitHubStatusQueryPolicy("workspace-list-item", {
hasWorkspaceId: true,
isActive: true,
}),
).toEqual({
enabled: true,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 30_000,
});
});

test("disables passive hover surfaces when they are not visible", () => {
expect(
getGitHubStatusQueryPolicy("workspace-row", {
hasWorkspaceId: true,
isActive: false,
}),
).toEqual({
enabled: false,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 300_000,
});
});
});

describe("getGitHubPRCommentsQueryPolicy", () => {
test("fetches review comments without polling when changes is open on diffs", () => {
expect(
getGitHubPRCommentsQueryPolicy({
hasWorkspaceId: true,
hasActivePullRequest: true,
isActive: true,
isReviewTabActive: false,
}),
).toEqual({
enabled: true,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 30_000,
});
});

test("polls review comments while the review tab is active", () => {
expect(
getGitHubPRCommentsQueryPolicy({
hasWorkspaceId: true,
hasActivePullRequest: true,
isActive: true,
isReviewTabActive: true,
}),
).toEqual({
enabled: true,
refetchInterval: 30_000,
refetchOnWindowFocus: true,
staleTime: 30_000,
});
});

test("disables comments when there is no active pull request", () => {
expect(
getGitHubPRCommentsQueryPolicy({
hasWorkspaceId: true,
hasActivePullRequest: false,
isActive: true,
isReviewTabActive: true,
}),
).toEqual({
enabled: false,
refetchInterval: false,
refetchOnWindowFocus: false,
staleTime: 30_000,
});
});
});
Loading
Loading