diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts index bebb6e12c9c..b514633654a 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts @@ -7,7 +7,6 @@ import { getSimpleGitWithShellPath, } from "../workspaces/utils/git-client"; import { - clearGitHubStatusCacheForWorktree, fetchGitHubPRStatus, getPullRequestRepoArgs, getRepoContext, @@ -144,7 +143,7 @@ async function resolveExistingPullRequestPushTarget({ worktreePath: string; fallbackRemote: string; }): Promise { - clearGitHubStatusCacheForWorktree(worktreePath); + clearWorktreeStatusCaches(worktreePath); const githubStatus = await fetchGitHubPRStatus(worktreePath); const pr = githubStatus?.pr; if (!pr || !isOpenPullRequestState(pr.state) || !pr.headRefName?.trim()) { diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts index a91078ad4ec..c732b6df83e 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts @@ -1,7 +1,7 @@ -import { clearGitHubStatusCacheForWorktree } from "../../workspaces/utils/github"; +import { clearGitHubCachesForWorktree } from "../../workspaces/utils/github"; import { clearStatusCacheForWorktree } from "./status-cache"; export function clearWorktreeStatusCaches(worktreePath: string): void { - clearGitHubStatusCacheForWorktree(worktreePath); + clearGitHubCachesForWorktree(worktreePath); clearStatusCacheForWorktree(worktreePath); } diff --git a/apps/desktop/src/lib/trpc/routers/external/index.ts b/apps/desktop/src/lib/trpc/routers/external/index.ts index 85487f1cb52..4dde4b6e196 100644 --- a/apps/desktop/src/lib/trpc/routers/external/index.ts +++ b/apps/desktop/src/lib/trpc/routers/external/index.ts @@ -144,6 +144,10 @@ export const createExternalRouter = () => { clipboard.writeText(input); }), + copyText: publicProcedure.input(z.string()).mutation(async ({ input }) => { + clipboard.writeText(input); + }), + resolvePath: publicProcedure .input( z.object({ diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts index c07ff59097c..a21e718c4cb 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts @@ -1,4 +1,5 @@ import { existsSync } from "node:fs"; +import type { GitHubStatus } from "@superset/local-db"; import { workspaces, worktrees } from "@superset/local-db"; import { and, eq, isNull } from "drizzle-orm"; import { localDb } from "main/lib/local-db"; @@ -17,7 +18,52 @@ import { listExternalWorktrees, refreshDefaultBranch, } from "../utils/git"; -import { fetchGitHubPRStatus } from "../utils/github"; +import { + fetchGitHubPRComments, + fetchGitHubPRStatus, + type PullRequestCommentsTarget, +} from "../utils/github"; + +const gitHubPRCommentsInputSchema = z.object({ + workspaceId: z.string(), + prNumber: z.number().int().positive().optional(), + repoUrl: z.string().optional(), + upstreamUrl: z.string().optional(), + isFork: z.boolean().optional(), +}); + +function resolveCommentsPullRequestTarget({ + input, + githubStatus, +}: { + input: z.infer; + githubStatus: GitHubStatus | null | undefined; +}): PullRequestCommentsTarget | null { + const prNumber = input.prNumber ?? githubStatus?.pr?.number; + if (!prNumber) { + return null; + } + + const repoUrl = input.repoUrl ?? githubStatus?.repoUrl; + if (!repoUrl) { + return null; + } + + const upstreamUrl = + input.upstreamUrl ?? githubStatus?.upstreamUrl ?? githubStatus?.repoUrl; + if (!upstreamUrl) { + return null; + } + + return { + prNumber, + repoContext: { + repoUrl, + upstreamUrl, + isFork: input.isFork ?? githubStatus?.isFork ?? false, + }, + }; +} export const createGitStatusProcedures = () => { return router({ @@ -130,6 +176,32 @@ export const createGitStatusProcedures = () => { return freshStatus; }), + getGitHubPRComments: publicProcedure + .input(gitHubPRCommentsInputSchema) + .query(async ({ input }) => { + const workspace = getWorkspace(input.workspaceId); + if (!workspace) { + return []; + } + + const worktree = workspace.worktreeId + ? getWorktree(workspace.worktreeId) + : null; + if (!worktree) { + return []; + } + + const cachedGitHubStatus = worktree.githubStatus ?? null; + + return fetchGitHubPRComments({ + worktreePath: worktree.path, + pullRequest: resolveCommentsPullRequestTarget({ + input, + githubStatus: cachedGitHubStatus, + }), + }); + }), + getWorktreeInfo: publicProcedure .input(z.object({ workspaceId: z.string() })) .query(({ input }) => { diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.test.ts new file mode 100644 index 00000000000..9b0003738de --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; +import { + clearGitHubCachesForWorktree, + getCachedGitHubStatus, + getCachedPullRequestComments, + getCachedRepoContext, + makePullRequestCommentsCacheKey, + setCachedGitHubStatus, + setCachedPullRequestComments, + setCachedRepoContext, +} from "./cache"; + +describe("clearGitHubCachesForWorktree", () => { + test("clears status, repo context, and comment entries for one worktree", () => { + const worktreePath = "/tmp/worktrees/review-cache-test"; + const otherWorktreePath = "/tmp/worktrees/review-cache-test-other"; + + const status: GitHubStatus = { + pr: null, + repoUrl: "https://github.com/superset-sh/superset", + upstreamUrl: "https://github.com/superset-sh/superset", + isFork: false, + branchExistsOnRemote: true, + lastRefreshed: Date.now(), + }; + const comments: PullRequestComment[] = [ + { + id: "review-1", + authorLogin: "octocat", + body: "Looks good", + kind: "review", + }, + ]; + + setCachedGitHubStatus(worktreePath, status); + setCachedRepoContext(worktreePath, { + repoUrl: "https://github.com/superset-sh/superset", + upstreamUrl: "https://github.com/superset-sh/superset", + isFork: false, + }); + + const commentsCacheKey = makePullRequestCommentsCacheKey({ + worktreePath, + repoNameWithOwner: "superset-sh/superset", + pullRequestNumber: 2681, + }); + const otherCommentsCacheKey = makePullRequestCommentsCacheKey({ + worktreePath: otherWorktreePath, + repoNameWithOwner: "superset-sh/superset", + pullRequestNumber: 2682, + }); + + setCachedPullRequestComments(commentsCacheKey, comments); + setCachedPullRequestComments(otherCommentsCacheKey, comments); + + clearGitHubCachesForWorktree(worktreePath); + + expect(getCachedGitHubStatus(worktreePath)).toBeNull(); + expect(getCachedRepoContext(worktreePath)).toBeNull(); + expect(getCachedPullRequestComments(commentsCacheKey)).toBeNull(); + expect(getCachedPullRequestComments(otherCommentsCacheKey)).toEqual( + comments, + ); + + clearGitHubCachesForWorktree(otherWorktreePath); + }); +}); diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.ts new file mode 100644 index 00000000000..a0f2375d14f --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/cache.ts @@ -0,0 +1,210 @@ +import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; +import type { RepoContext } from "./types"; + +const GITHUB_STATUS_CACHE_TTL_MS = 10_000; +const GITHUB_PR_COMMENTS_CACHE_TTL_MS = 30_000; +const GITHUB_REPO_CONTEXT_CACHE_TTL_MS = 300_000; + +const MAX_GITHUB_STATUS_CACHE_ENTRIES = 256; +const MAX_GITHUB_PR_COMMENTS_CACHE_ENTRIES = 512; +const MAX_GITHUB_REPO_CONTEXT_CACHE_ENTRIES = 256; + +interface CacheEntry { + value: T; + expiresAt: number; +} + +const githubStatusCache = new Map>(); +const githubStatusInFlight = new Map>(); + +const pullRequestCommentsCache = new Map< + string, + CacheEntry +>(); +const pullRequestCommentsInFlight = new Map< + string, + Promise +>(); + +const repoContextCache = new Map>(); +const repoContextInFlight = new Map>(); + +function getCachedValue( + cache: Map>, + cacheKey: string, +): T | null { + const cached = cache.get(cacheKey); + if (!cached) { + return null; + } + + if (cached.expiresAt <= Date.now()) { + cache.delete(cacheKey); + return null; + } + + return cached.value; +} + +function setCachedValue( + cache: Map>, + cacheKey: string, + value: T, + ttlMs: number, + maxEntries: number, +): void { + if (!cache.has(cacheKey) && cache.size >= maxEntries) { + cache.clear(); + } + + cache.set(cacheKey, { + value, + expiresAt: Date.now() + ttlMs, + }); +} + +function clearEntriesWithPrefix( + cache: Map, + cacheKeyPrefix: string, +): void { + for (const cacheKey of cache.keys()) { + if (cacheKey.startsWith(cacheKeyPrefix)) { + cache.delete(cacheKey); + } + } +} + +export function getCachedGitHubStatus( + worktreePath: string, +): GitHubStatus | null { + return getCachedValue(githubStatusCache, worktreePath); +} + +export function setCachedGitHubStatus( + worktreePath: string, + value: GitHubStatus, +): void { + setCachedValue( + githubStatusCache, + worktreePath, + value, + GITHUB_STATUS_CACHE_TTL_MS, + MAX_GITHUB_STATUS_CACHE_ENTRIES, + ); +} + +export function getInFlightGitHubStatus( + worktreePath: string, +): Promise | null { + return githubStatusInFlight.get(worktreePath) ?? null; +} + +export function setInFlightGitHubStatus( + worktreePath: string, + promise: Promise, +): void { + githubStatusInFlight.set(worktreePath, promise); +} + +export function clearInFlightGitHubStatus(worktreePath: string): void { + githubStatusInFlight.delete(worktreePath); +} + +export function makePullRequestCommentsCachePrefix( + worktreePath: string, +): string { + return `${worktreePath}::comments::`; +} + +export function makePullRequestCommentsCacheKey({ + worktreePath, + repoNameWithOwner, + pullRequestNumber, +}: { + worktreePath: string; + repoNameWithOwner: string; + pullRequestNumber: number; +}): string { + return `${makePullRequestCommentsCachePrefix(worktreePath)}${repoNameWithOwner}#${pullRequestNumber}`; +} + +export function getCachedPullRequestComments( + cacheKey: string, +): PullRequestComment[] | null { + return getCachedValue(pullRequestCommentsCache, cacheKey); +} + +export function setCachedPullRequestComments( + cacheKey: string, + value: PullRequestComment[], +): void { + setCachedValue( + pullRequestCommentsCache, + cacheKey, + value, + GITHUB_PR_COMMENTS_CACHE_TTL_MS, + MAX_GITHUB_PR_COMMENTS_CACHE_ENTRIES, + ); +} + +export function getInFlightPullRequestComments( + cacheKey: string, +): Promise | null { + return pullRequestCommentsInFlight.get(cacheKey) ?? null; +} + +export function setInFlightPullRequestComments( + cacheKey: string, + promise: Promise, +): void { + pullRequestCommentsInFlight.set(cacheKey, promise); +} + +export function clearInFlightPullRequestComments(cacheKey: string): void { + pullRequestCommentsInFlight.delete(cacheKey); +} + +export function getCachedRepoContext(worktreePath: string): RepoContext | null { + return getCachedValue(repoContextCache, worktreePath); +} + +export function setCachedRepoContext( + worktreePath: string, + value: RepoContext, +): void { + setCachedValue( + repoContextCache, + worktreePath, + value, + GITHUB_REPO_CONTEXT_CACHE_TTL_MS, + MAX_GITHUB_REPO_CONTEXT_CACHE_ENTRIES, + ); +} + +export function getInFlightRepoContext( + worktreePath: string, +): Promise | null { + return repoContextInFlight.get(worktreePath) ?? null; +} + +export function setInFlightRepoContext( + worktreePath: string, + promise: Promise, +): void { + repoContextInFlight.set(worktreePath, promise); +} + +export function clearInFlightRepoContext(worktreePath: string): void { + repoContextInFlight.delete(worktreePath); +} + +export function clearGitHubCachesForWorktree(worktreePath: string): void { + githubStatusCache.delete(worktreePath); + githubStatusInFlight.delete(worktreePath); + repoContextCache.delete(worktreePath); + repoContextInFlight.delete(worktreePath); + + const commentsCachePrefix = makePullRequestCommentsCachePrefix(worktreePath); + clearEntriesWithPrefix(pullRequestCommentsCache, commentsCachePrefix); + clearEntriesWithPrefix(pullRequestCommentsInFlight, commentsCachePrefix); +} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/comments.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/comments.ts new file mode 100644 index 00000000000..f09241b08a7 --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/comments.ts @@ -0,0 +1,201 @@ +import type { PullRequestComment } from "@superset/local-db"; +import { execWithShellEnv } from "../shell-env"; +import { GHIssueCommentSchema, GHReviewCommentSchema } from "./types"; + +function parseTimestamp(value?: string): number | undefined { + if (!value) { + return undefined; + } + + const timestamp = new Date(value).getTime(); + return Number.isNaN(timestamp) ? undefined : timestamp; +} + +export function parsePaginatedApiArray(stdout: string): unknown[] { + const trimmed = stdout.trim(); + if (!trimmed || trimmed === "null") { + return []; + } + + try { + const raw = JSON.parse(trimmed); + if (!Array.isArray(raw)) { + return []; + } + + return raw.flatMap((page) => (Array.isArray(page) ? page : [page])); + } catch (error) { + console.warn( + "[GitHub] Failed to parse paginated API array response:", + error instanceof Error ? error.message : String(error), + ); + return []; + } +} + +export function parseReviewCommentsResponse( + raw: unknown[], +): PullRequestComment[] { + return raw + .flatMap((item) => { + const result = GHReviewCommentSchema.safeParse(item); + if (!result.success) { + return []; + } + + const comment = result.data; + const body = comment.body?.trim(); + if (!body) { + return []; + } + + return [ + { + id: `review-${comment.id}`, + authorLogin: comment.user?.login || "github", + ...(comment.user?.avatar_url + ? { avatarUrl: comment.user.avatar_url } + : {}), + body, + createdAt: parseTimestamp(comment.created_at), + url: comment.html_url, + kind: "review" as const, + path: comment.path, + line: comment.line ?? comment.original_line ?? undefined, + }, + ]; + }) + .sort((a, b) => (b.createdAt ?? 0) - (a.createdAt ?? 0)); +} + +export function parseConversationCommentsResponse( + raw: unknown[], +): PullRequestComment[] { + return raw + .flatMap((item) => { + const result = GHIssueCommentSchema.safeParse(item); + if (!result.success) { + return []; + } + + const comment = result.data; + const body = comment.body?.trim(); + if (!body) { + return []; + } + + return [ + { + id: `conversation-${comment.id}`, + authorLogin: comment.user?.login || "github", + ...(comment.user?.avatar_url + ? { avatarUrl: comment.user.avatar_url } + : {}), + body, + createdAt: parseTimestamp(comment.created_at), + url: comment.html_url, + kind: "conversation" as const, + }, + ]; + }) + .sort((a, b) => (b.createdAt ?? 0) - (a.createdAt ?? 0)); +} + +export function mergePullRequestComments( + ...commentGroups: PullRequestComment[][] +): PullRequestComment[] { + const commentsById = new Map(); + + for (const group of commentGroups) { + for (const comment of group) { + commentsById.set(comment.id, comment); + } + } + + return [...commentsById.values()].sort( + (a, b) => (b.createdAt ?? 0) - (a.createdAt ?? 0), + ); +} + +async function fetchPaginatedCommentsEndpoint( + worktreePath: string, + endpoint: string, +): Promise { + const { stdout } = await execWithShellEnv( + "gh", + ["api", "--paginate", "--slurp", endpoint], + { cwd: worktreePath }, + ); + + return parsePaginatedApiArray(stdout); +} + +async function fetchReviewCommentsForPullRequest( + worktreePath: string, + repoNameWithOwner: string, + pullRequestNumber: number, +): Promise { + const pages = await fetchPaginatedCommentsEndpoint( + worktreePath, + `repos/${repoNameWithOwner}/pulls/${pullRequestNumber}/comments?per_page=100`, + ); + + return parseReviewCommentsResponse(pages); +} + +async function fetchConversationCommentsForPullRequest( + worktreePath: string, + repoNameWithOwner: string, + pullRequestNumber: number, +): Promise { + const pages = await fetchPaginatedCommentsEndpoint( + worktreePath, + `repos/${repoNameWithOwner}/issues/${pullRequestNumber}/comments?per_page=100`, + ); + + return parseConversationCommentsResponse(pages); +} + +export async function fetchPullRequestComments({ + worktreePath, + repoNameWithOwner, + pullRequestNumber, +}: { + worktreePath: string; + repoNameWithOwner: string; + pullRequestNumber: number; +}): Promise { + const [reviewResult, conversationResult] = await Promise.allSettled([ + fetchReviewCommentsForPullRequest( + worktreePath, + repoNameWithOwner, + pullRequestNumber, + ), + fetchConversationCommentsForPullRequest( + worktreePath, + repoNameWithOwner, + pullRequestNumber, + ), + ]); + + const reviewComments = + reviewResult.status === "fulfilled" ? reviewResult.value : []; + const conversationComments = + conversationResult.status === "fulfilled" ? conversationResult.value : []; + + if (reviewResult.status === "rejected") { + console.warn( + "[GitHub] Failed to fetch pull request review comments:", + reviewResult.reason, + ); + } + + if (conversationResult.status === "rejected") { + console.warn( + "[GitHub] Failed to fetch pull request conversation comments:", + conversationResult.reason, + ); + } + + return mergePullRequestComments(reviewComments, conversationComments); +} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts index 80e01c9fabe..03a50ef9d73 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts @@ -1,4 +1,10 @@ import { describe, expect, test } from "bun:test"; +import { + mergePullRequestComments, + parseConversationCommentsResponse, + parsePaginatedApiArray, + parseReviewCommentsResponse, +} from "./comments"; import { resolveRemoteBranchNameForGitHubStatus } from "./github"; import { branchMatchesPR, @@ -64,6 +70,128 @@ describe("getPullRequestRepoArgs", () => { }); }); +describe("parseReviewCommentsResponse", () => { + test("normalizes inline review comments with file metadata", () => { + expect( + parseReviewCommentsResponse([ + { + id: 42, + user: { + login: "octocat", + avatar_url: "https://avatars.githubusercontent.com/u/1?v=4", + }, + body: "Please rename this helper.", + created_at: "2026-03-21T04:19:41Z", + html_url: + "https://github.com/superset-sh/superset/pull/2681#discussion_r42", + path: "apps/desktop/src/file.ts", + line: 19, + }, + ]), + ).toEqual([ + { + id: "review-42", + authorLogin: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/1?v=4", + body: "Please rename this helper.", + createdAt: new Date("2026-03-21T04:19:41Z").getTime(), + url: "https://github.com/superset-sh/superset/pull/2681#discussion_r42", + kind: "review", + path: "apps/desktop/src/file.ts", + line: 19, + }, + ]); + }); +}); + +describe("parsePaginatedApiArray", () => { + test("flattens slurped paginated arrays", () => { + expect( + parsePaginatedApiArray( + JSON.stringify([[{ id: 1 }, { id: 2 }], [{ id: 3 }]]), + ), + ).toEqual([{ id: 1 }, { id: 2 }, { id: 3 }]); + }); + + test("keeps single-page arrays intact", () => { + expect( + parsePaginatedApiArray(JSON.stringify([{ id: 1 }, { id: 2 }])), + ).toEqual([{ id: 1 }, { id: 2 }]); + }); +}); + +describe("parseConversationCommentsResponse", () => { + test("normalizes top-level PR conversation comments", () => { + expect( + parseConversationCommentsResponse([ + { + id: 7, + user: { + login: "hubot", + avatar_url: "https://avatars.githubusercontent.com/u/2?v=4", + }, + body: "Looks good overall.", + created_at: "2026-03-21T04:08:13Z", + html_url: + "https://github.com/superset-sh/superset/pull/2681#issuecomment-7", + }, + ]), + ).toEqual([ + { + id: "conversation-7", + authorLogin: "hubot", + avatarUrl: "https://avatars.githubusercontent.com/u/2?v=4", + body: "Looks good overall.", + createdAt: new Date("2026-03-21T04:08:13Z").getTime(), + url: "https://github.com/superset-sh/superset/pull/2681#issuecomment-7", + kind: "conversation", + }, + ]); + }); +}); + +describe("mergePullRequestComments", () => { + test("sorts mixed comment kinds by recency", () => { + expect( + mergePullRequestComments( + [ + { + id: "review-42", + authorLogin: "octocat", + body: "Inline note", + createdAt: 200, + kind: "review", + }, + ], + [ + { + id: "conversation-7", + authorLogin: "hubot", + body: "Top-level note", + createdAt: 100, + kind: "conversation", + }, + ], + ), + ).toEqual([ + { + id: "review-42", + authorLogin: "octocat", + body: "Inline note", + createdAt: 200, + kind: "review", + }, + { + id: "conversation-7", + authorLogin: "hubot", + body: "Top-level note", + createdAt: 100, + kind: "conversation", + }, + ]); + }); +}); + describe("getPRHeadBranchCandidates", () => { test("returns exact branch first", () => { expect(getPRHeadBranchCandidates("kitenite/feature")).toEqual([ diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts index 9881bf48e0a..8be88618a7a 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts @@ -1,8 +1,23 @@ -import type { GitHubStatus } from "@superset/local-db"; +import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; import { branchExistsOnRemote } from "../git"; import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; import { parseUpstreamRef } from "../upstream-ref"; +import { + clearGitHubCachesForWorktree, + clearInFlightGitHubStatus, + clearInFlightPullRequestComments, + getCachedGitHubStatus, + getCachedPullRequestComments, + getInFlightGitHubStatus, + getInFlightPullRequestComments, + makePullRequestCommentsCacheKey, + setCachedGitHubStatus, + setCachedPullRequestComments, + setInFlightGitHubStatus, + setInFlightPullRequestComments, +} from "./cache"; +import { fetchPullRequestComments } from "./comments"; import { getPRForBranch } from "./pr-resolution"; import { extractNwoFromUrl, getRepoContext } from "./repo-context"; import { @@ -11,11 +26,55 @@ import { type RepoContext, } from "./types"; -const cache = new Map(); -const CACHE_TTL_MS = 10_000; +export interface PullRequestCommentsTarget { + prNumber: number; + repoContext: Pick; +} + +export { clearGitHubCachesForWorktree }; + +function getPullRequestCommentsRepoNameWithOwner( + target: PullRequestCommentsTarget, +): string | null { + const targetUrl = target.repoContext.isFork + ? target.repoContext.upstreamUrl + : target.repoContext.repoUrl; + + return extractNwoFromUrl(targetUrl); +} + +async function resolvePullRequestCommentsTarget( + worktreePath: string, +): Promise { + const repoContext = await getRepoContext(worktreePath); + if (!repoContext) { + return null; + } + + const [branchResult, shaResult] = await Promise.all([ + execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { + cwd: worktreePath, + }), + execGitWithShellPath(["rev-parse", "HEAD"], { + cwd: worktreePath, + }), + ]); + const branchName = branchResult.stdout.trim(); + const headSha = shaResult.stdout.trim(); + const prInfo = await getPRForBranch( + worktreePath, + branchName, + repoContext, + headSha, + ); + if (!prInfo) { + return null; + } -export function clearGitHubStatusCacheForWorktree(worktreePath: string): void { - cache.delete(worktreePath); + return { + prNumber: prInfo.number, + repoContext, + }; } export function resolveRemoteBranchNameForGitHubStatus({ @@ -37,88 +96,156 @@ export function resolveRemoteBranchNameForGitHubStatus({ export async function fetchGitHubPRStatus( worktreePath: string, ): Promise { - const cached = cache.get(worktreePath); - if (cached && Date.now() - cached.timestamp < CACHE_TTL_MS) { - return cached.data; + const cached = getCachedGitHubStatus(worktreePath); + if (cached) { + return cached; } - try { - const repoContext = await getRepoContext(worktreePath); - if (!repoContext) { - return null; - } + const inFlight = getInFlightGitHubStatus(worktreePath); + if (inFlight) { + return inFlight; + } - const [branchResult, shaResult, upstreamResult] = await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: worktreePath, - }), - execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), - execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { - cwd: worktreePath, - }).catch(() => ({ stdout: "", stderr: "" })), - ]); - const branchName = branchResult.stdout.trim(); - const headSha = shaResult.stdout.trim(); - const parsedUpstreamRef = parseUpstreamRef(upstreamResult.stdout.trim()); - const trackingRemote = parsedUpstreamRef?.remoteName ?? "origin"; - - const [prInfo, previewUrl] = await Promise.all([ - getPRForBranch(worktreePath, branchName, repoContext, headSha), - fetchPreviewDeploymentUrl( - worktreePath, - headSha, - resolveRemoteBranchNameForGitHubStatus({ - localBranchName: branchName, - upstreamBranchName: parsedUpstreamRef?.branchName, - }), - repoContext, - ), - ]); - - const remoteBranchName = resolveRemoteBranchNameForGitHubStatus({ - localBranchName: branchName, - upstreamBranchName: parsedUpstreamRef?.branchName, - prHeadRefName: prInfo?.headRefName, - }); + const promise = (async () => { + try { + const repoContext = await getRepoContext(worktreePath); + if (!repoContext) { + return null; + } - const branchCheck = await branchExistsOnRemote( - worktreePath, - remoteBranchName, - trackingRemote, - ); + const [branchResult, shaResult, upstreamResult] = await Promise.all([ + execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { + cwd: worktreePath, + }), + execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), + execGitWithShellPath(["rev-parse", "--abbrev-ref", "@{upstream}"], { + cwd: worktreePath, + }).catch(() => ({ stdout: "", stderr: "" })), + ]); + const branchName = branchResult.stdout.trim(); + const headSha = shaResult.stdout.trim(); + const parsedUpstreamRef = parseUpstreamRef(upstreamResult.stdout.trim()); + const trackingRemote = parsedUpstreamRef?.remoteName ?? "origin"; + const previewBranchName = resolveRemoteBranchNameForGitHubStatus({ + localBranchName: branchName, + upstreamBranchName: parsedUpstreamRef?.branchName, + }); - // If no preview URL found via SHA/branch, try the PR merge ref - // (GitHub Actions pull_request triggers use refs/pull/N/merge) - let finalPreviewUrl = previewUrl; - if (!finalPreviewUrl && prInfo?.number) { - const targetUrl = repoContext.isFork - ? repoContext.upstreamUrl - : repoContext.repoUrl; - const nwo = extractNwoFromUrl(targetUrl); - if (nwo) { - finalPreviewUrl = await queryDeploymentUrl( + const [prInfo, previewUrl] = await Promise.all([ + getPRForBranch(worktreePath, branchName, repoContext, headSha), + fetchPreviewDeploymentUrl( worktreePath, - nwo, - `ref=${encodeURIComponent(`refs/pull/${prInfo.number}/merge`)}`, - ); + headSha, + previewBranchName, + repoContext, + ), + ]); + + const remoteBranchName = resolveRemoteBranchNameForGitHubStatus({ + localBranchName: branchName, + upstreamBranchName: parsedUpstreamRef?.branchName, + prHeadRefName: prInfo?.headRefName, + }); + + const branchCheck = await branchExistsOnRemote( + worktreePath, + remoteBranchName, + trackingRemote, + ); + + // If no preview URL found via SHA/branch, try the PR merge ref + // (GitHub Actions pull_request triggers use refs/pull/N/merge) + let finalPreviewUrl = previewUrl; + if (!finalPreviewUrl && prInfo?.number) { + const targetUrl = repoContext.isFork + ? repoContext.upstreamUrl + : repoContext.repoUrl; + const nwo = extractNwoFromUrl(targetUrl); + if (nwo) { + finalPreviewUrl = await queryDeploymentUrl( + worktreePath, + nwo, + `ref=${encodeURIComponent(`refs/pull/${prInfo.number}/merge`)}`, + ); + } } + + const result: GitHubStatus = { + pr: prInfo, + repoUrl: repoContext.repoUrl, + upstreamUrl: repoContext.upstreamUrl, + isFork: repoContext.isFork, + branchExistsOnRemote: branchCheck.status === "exists", + previewUrl: finalPreviewUrl, + lastRefreshed: Date.now(), + }; + + setCachedGitHubStatus(worktreePath, result); + return result; + } catch { + return null; + } finally { + clearInFlightGitHubStatus(worktreePath); + } + })(); + + setInFlightGitHubStatus(worktreePath, promise); + return promise; +} + +export async function fetchGitHubPRComments({ + worktreePath, + pullRequest, +}: { + worktreePath: string; + pullRequest?: PullRequestCommentsTarget | null; +}): Promise { + try { + const pullRequestTarget = + pullRequest ?? (await resolvePullRequestCommentsTarget(worktreePath)); + if (!pullRequestTarget) { + return []; + } + + const repoNameWithOwner = + getPullRequestCommentsRepoNameWithOwner(pullRequestTarget); + if (!repoNameWithOwner) { + return []; + } + + const cacheKey = makePullRequestCommentsCacheKey({ + worktreePath, + repoNameWithOwner, + pullRequestNumber: pullRequestTarget.prNumber, + }); + const cached = getCachedPullRequestComments(cacheKey); + if (cached) { + return cached; } - const result: GitHubStatus = { - pr: prInfo, - repoUrl: repoContext.repoUrl, - upstreamUrl: repoContext.upstreamUrl, - isFork: repoContext.isFork, - branchExistsOnRemote: branchCheck.status === "exists", - previewUrl: finalPreviewUrl, - lastRefreshed: Date.now(), - }; + const inFlight = getInFlightPullRequestComments(cacheKey); + if (inFlight) { + return await inFlight; + } - cache.set(worktreePath, { data: result, timestamp: Date.now() }); + const promise = (async () => { + try { + const comments = await fetchPullRequestComments({ + worktreePath, + repoNameWithOwner, + pullRequestNumber: pullRequestTarget.prNumber, + }); + setCachedPullRequestComments(cacheKey, comments); + return comments; + } finally { + clearInFlightPullRequestComments(cacheKey); + } + })(); - return result; + setInFlightPullRequestComments(cacheKey, promise); + return await promise; } catch { - return null; + return []; } } @@ -155,9 +282,13 @@ async function queryDeploymentUrl( const deploymentIds: number[] = []; for (const raw of rawDeployments) { const result = GHDeploymentSchema.safeParse(raw); - if (result.success) deploymentIds.push(result.data.id); + if (result.success) { + deploymentIds.push(result.data.id); + } + } + if (deploymentIds.length === 0) { + return undefined; } - if (deploymentIds.length === 0) return undefined; const urls = await Promise.all( deploymentIds.map(async (id): Promise => { @@ -172,7 +303,9 @@ async function queryDeploymentUrl( return undefined; } const statusResult = GHDeploymentStatusSchema.safeParse(rawStatuses[0]); - if (!statusResult.success) return undefined; + if (!statusResult.success) { + return undefined; + } if ( statusResult.data.state === "success" && statusResult.data.environment_url && @@ -209,11 +342,15 @@ async function fetchPreviewDeploymentUrl( ? repoContext.upstreamUrl : repoContext.repoUrl; const nwo = extractNwoFromUrl(targetUrl); - if (!nwo) return undefined; + if (!nwo) { + return undefined; + } // Try by commit SHA (works for Vercel, Netlify official integrations) const bySha = await queryDeploymentUrl(worktreePath, nwo, `sha=${headSha}`); - if (bySha) return bySha; + if (bySha) { + return bySha; + } // Fall back to branch name (works for some CI configurations) return await queryDeploymentUrl( diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts index 66cedc65192..2253ee3f295 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts @@ -1,5 +1,7 @@ +export type { PullRequestCommentsTarget } from "./github"; export { - clearGitHubStatusCacheForWorktree, + clearGitHubCachesForWorktree, + fetchGitHubPRComments, fetchGitHubPRStatus, } from "./github"; export { getPRForBranch } from "./pr-resolution"; diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts index 75bef322e2a..9c47d4af2c3 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts @@ -1,75 +1,85 @@ import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; +import { + clearInFlightRepoContext, + getCachedRepoContext, + getInFlightRepoContext, + setCachedRepoContext, + setInFlightRepoContext, +} from "./cache"; import { GHRepoResponseSchema, type RepoContext } from "./types"; -const repoContextCache = new Map< - string, - { data: RepoContext; timestamp: number } ->(); -const REPO_CONTEXT_CACHE_TTL_MS = 300_000; // 5 minutes - export async function getRepoContext( worktreePath: string, ): Promise { - const cached = repoContextCache.get(worktreePath); - if (cached && Date.now() - cached.timestamp < REPO_CONTEXT_CACHE_TTL_MS) { - return cached.data; + const cached = getCachedRepoContext(worktreePath); + if (cached) { + return cached; } - try { - const { stdout } = await execWithShellEnv( - "gh", - ["repo", "view", "--json", "url,isFork,parent"], - { cwd: worktreePath }, - ); - const raw = JSON.parse(stdout); - const result = GHRepoResponseSchema.safeParse(raw); - if (!result.success) { - console.error("[GitHub] Repo schema validation failed:", result.error); - console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); - return null; - } - - const data = result.data; - let context: RepoContext; - - if (data.isFork && data.parent) { - context = { - repoUrl: data.url, - upstreamUrl: data.parent.url, - isFork: true, - }; - } else { - const originUrl = await getOriginUrl(worktreePath); - const ghUrl = normalizeGitHubUrl(data.url); + const inFlight = getInFlightRepoContext(worktreePath); + if (inFlight) { + return inFlight; + } - if (data.isFork) { + const promise = (async () => { + try { + const { stdout } = await execWithShellEnv( + "gh", + ["repo", "view", "--json", "url,isFork,parent"], + { cwd: worktreePath }, + ); + const raw = JSON.parse(stdout); + const result = GHRepoResponseSchema.safeParse(raw); + if (!result.success) { + console.error("[GitHub] Repo schema validation failed:", result.error); + console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); return null; } - if (originUrl && ghUrl && originUrl !== ghUrl) { + const data = result.data; + let context: RepoContext; + + if (data.isFork && data.parent) { context = { - repoUrl: originUrl, - upstreamUrl: ghUrl, + repoUrl: data.url, + upstreamUrl: data.parent.url, isFork: true, }; } else { - context = { - repoUrl: data.url, - upstreamUrl: data.url, - isFork: false, - }; + const originUrl = await getOriginUrl(worktreePath); + const ghUrl = normalizeGitHubUrl(data.url); + + if (data.isFork) { + return null; + } + + if (originUrl && ghUrl && originUrl !== ghUrl) { + context = { + repoUrl: originUrl, + upstreamUrl: ghUrl, + isFork: true, + }; + } else { + context = { + repoUrl: data.url, + upstreamUrl: data.url, + isFork: false, + }; + } } + + setCachedRepoContext(worktreePath, context); + return context; + } catch { + return null; + } finally { + clearInFlightRepoContext(worktreePath); } + })(); - repoContextCache.set(worktreePath, { - data: context, - timestamp: Date.now(), - }); - return context; - } catch { - return null; - } + setInFlightRepoContext(worktreePath, promise); + return promise; } async function getOriginUrl(worktreePath: string): Promise { diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts index f9e8dc8f308..089d3d344d9 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts @@ -32,6 +32,38 @@ export const GHReviewRequestSchema = z.object({ type: z.enum(["User", "Team"]).optional(), }); +export const GHCommentAuthorSchema = z.object({ + login: z.string().optional(), + avatar_url: z.string().optional(), +}); + +export const GHCommentSchema = z.object({ + id: z.string().optional(), + author: GHCommentAuthorSchema.nullable().optional(), + body: z.string().optional(), + createdAt: z.string().optional(), + url: z.string().optional(), +}); + +export const GHReviewCommentSchema = z.object({ + id: z.number(), + user: GHCommentAuthorSchema.nullable().optional(), + body: z.string().optional(), + created_at: z.string().optional(), + html_url: z.string().optional(), + path: z.string().optional(), + line: z.number().nullable().optional(), + original_line: z.number().nullable().optional(), +}); + +export const GHIssueCommentSchema = z.object({ + id: z.number(), + user: GHCommentAuthorSchema.nullable().optional(), + body: z.string().optional(), + created_at: z.string().optional(), + html_url: z.string().optional(), +}); + export const GHPRResponseSchema = z.object({ number: z.number(), title: z.string(), @@ -61,6 +93,7 @@ export const GHPRResponseSchema = z.object({ .nullable(), // statusCheckRollup is an array directly, not { contexts: [...] } statusCheckRollup: z.array(GHCheckContextSchema).nullable(), + comments: z.array(GHCommentSchema).nullable().optional(), reviewRequests: z.array(GHReviewRequestSchema).nullable().optional(), }); diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx index 0b7a042fd43..c3966d959c3 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx @@ -8,6 +8,7 @@ import { } from "@superset/ui/alert-dialog"; import { Button } from "@superset/ui/button"; import { toast } from "@superset/ui/sonner"; +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@superset/ui/tabs"; import { useParams } from "@tanstack/react-router"; import { useEffect, useMemo, useRef, useState } from "react"; import { electronTrpc } from "renderer/lib/electron-trpc"; @@ -22,10 +23,11 @@ import { } from "shared/absolute-paths"; import type { ChangeCategory, ChangedFile } from "shared/changes-types"; import type { FileSystemChangeEvent } from "shared/file-tree-types"; +import { sidebarHeaderTabTriggerClassName } from "../headerTabStyles"; import { CategorySection } from "./components/CategorySection"; import { ChangesHeader } from "./components/ChangesHeader"; import { CommitInput } from "./components/CommitInput"; -import { PRChecksStatus } from "./components/PRChecksStatus"; +import { ReviewPanel } from "./components/ReviewPanel"; import { useOrderedSections } from "./hooks"; import { getPRActionState, shouldAutoCreatePRAfterPublish } from "./utils"; @@ -40,12 +42,18 @@ interface ChangesViewProps { } const INACTIVE_BRANCH_REFETCH_INTERVAL_MS = 10_000; +const GITHUB_STATUS_STALE_TIME_MS = 10_000; +const GITHUB_STATUS_REFETCH_INTERVAL_MS = 10_000; +const GITHUB_PR_COMMENTS_STALE_TIME_MS = 30_000; +const GITHUB_PR_COMMENTS_REFETCH_INTERVAL_MS = 30_000; interface PendingChangesRefresh { invalidateBranches: boolean; invalidateSelectedFile: boolean; } +type ChangesSidebarTab = "diffs" | "review"; + function eventTargetsSelectedFile( event: FileSystemChangeEvent, selectedAbsolutePath: string | null, @@ -105,21 +113,11 @@ export function ChangesView({ { workspaceId: workspaceId ?? "" }, { enabled: !!workspaceId && isActive, - refetchInterval: isActive ? 10000 : false, + refetchInterval: isActive ? GITHUB_STATUS_REFETCH_INTERVAL_MS : false, + staleTime: GITHUB_STATUS_STALE_TIME_MS, }, ); - useBranchSyncInvalidation({ - gitBranch: status?.branch ?? branchData?.currentBranch ?? undefined, - workspaceBranch: workspace?.branch, - workspaceId: workspaceId ?? "", - }); - - const handleRefresh = () => { - refetch(); - refetchGithubStatus(); - }; - const stageAllMutation = electronTrpc.changes.stageAll.useMutation({ onSuccess: () => refetch(), onError: (error) => { @@ -256,11 +254,53 @@ export function ChangesView({ const [showDiscardUnstagedDialog, setShowDiscardUnstagedDialog] = useState(false); const [showDiscardStagedDialog, setShowDiscardStagedDialog] = useState(false); + const [activeTab, setActiveTab] = useState("diffs"); + const activePullRequest = githubStatus?.pr ?? null; const refreshTimerRef = useRef | null>(null); const pendingRefreshRef = useRef({ invalidateBranches: false, invalidateSelectedFile: false, }); + const { + data: githubComments = [], + isLoading: isGitHubCommentsLoading, + refetch: refetchGitHubComments, + } = electronTrpc.workspaces.getGitHubPRComments.useQuery( + { + workspaceId: workspaceId ?? "", + ...(activePullRequest + ? { + prNumber: activePullRequest.number, + repoUrl: githubStatus?.repoUrl, + upstreamUrl: githubStatus?.upstreamUrl, + isFork: githubStatus?.isFork, + } + : {}), + }, + { + enabled: !!workspaceId && isActive && !!activePullRequest, + refetchInterval: + isActive && activePullRequest + ? GITHUB_PR_COMMENTS_REFETCH_INTERVAL_MS + : false, + staleTime: GITHUB_PR_COMMENTS_STALE_TIME_MS, + refetchOnWindowFocus: false, + }, + ); + + useBranchSyncInvalidation({ + gitBranch: status?.branch ?? branchData?.currentBranch ?? undefined, + workspaceBranch: workspace?.branch, + workspaceId: workspaceId ?? "", + }); + + const handleRefresh = () => { + refetch(); + refetchGithubStatus(); + if (activePullRequest) { + refetchGitHubComments(); + } + }; const handleDiscard = (file: ChangedFile) => { if (!worktreePath) return; @@ -299,6 +339,7 @@ export function ChangesView({ // biome-ignore lint/correctness/useExhaustiveDependencies: reset on workspace change useEffect(() => { setExpandedCommits(new Set()); + setActiveTab("diffs"); }, [worktreePath]); useEffect(() => { @@ -521,8 +562,8 @@ export function ChangesView({ ]); const hasStagedChanges = stagedFiles.length > 0; - const hasExistingPR = !!githubStatus?.pr; - const prUrl = githubStatus?.pr?.url; + const hasExistingPR = !!activePullRequest; + const prUrl = activePullRequest?.url; const hasGitHubRepo = !!githubStatus?.repoUrl; const defaultBranch = branchData?.defaultBranch ?? status?.defaultBranch ?? ""; @@ -642,69 +683,122 @@ export function ChangesView({ ); } + const againstMainCount = status.againstBase.length; + const reviewCommentCount = activePullRequest ? githubComments.length : 0; + return (
- stashMutation.mutate({ worktreePath })} - onStashIncludeUntracked={() => - stashIncludeUntrackedMutation.mutate({ worktreePath }) - } - onStashPop={() => stashPopMutation.mutate({ worktreePath })} - isStashPending={ - stashMutation.isPending || - stashIncludeUntrackedMutation.isPending || - stashPopMutation.isPending - } - /> - -
- {githubStatus?.pr && } - -
- - {!hasChanges ? ( -
- No changes detected -
- ) : ( -
- {orderedSections - .filter((section) => section.count > 0) - .map((section) => ( - - {section.content} - - ))} + setActiveTab(value as ChangesSidebarTab)} + className="flex flex-1 min-h-0 flex-col gap-0" + > +
+ + + Diffs + + {againstMainCount} + + + + Review + + {reviewCommentCount} + + +
- )} + + +
+ stashMutation.mutate({ worktreePath })} + onStashIncludeUntracked={() => + stashIncludeUntrackedMutation.mutate({ worktreePath }) + } + onStashPop={() => stashPopMutation.mutate({ worktreePath })} + isStashPending={ + stashMutation.isPending || + stashIncludeUntrackedMutation.isPending || + stashPopMutation.isPending + } + /> +
+
+ +
+ + {!hasChanges ? ( +
+ No changes detected +
+ ) : ( +
+ {orderedSections + .filter((section) => section.count > 0) + .map((section) => ( + + {section.content} + + ))} +
+ )} + + + + + + void; viewMode: ChangesViewMode; onViewModeChange: (mode: ChangesViewMode) => void; + showViewModeToggle?: boolean; worktreePath: string; pr: GitHubStatus["pr"] | null; isPRStatusLoading: boolean; @@ -238,47 +239,11 @@ function RefreshButton({ onRefresh }: { onRefresh: () => void }) { ); } -const reviewTagStyles = { - approved: - "border border-emerald-500/20 bg-emerald-500/10 text-emerald-700 dark:text-emerald-300", - changes_requested: - "border border-red-500/20 bg-red-500/10 text-red-700 dark:text-red-300", - pending: - "border border-amber-500/20 bg-amber-500/10 text-amber-700 dark:text-amber-300", -} as const; - -const reviewTagLabels = { - approved: "Approved", - changes_requested: "Changes req.", - pending: "Review pending", -} as const; - -function ReviewTag({ - status, - requestedReviewers, -}: { - status: "approved" | "changes_requested" | "pending"; - requestedReviewers?: string[]; -}) { - const label = - status === "pending" && requestedReviewers && requestedReviewers.length > 0 - ? `Awaiting ${requestedReviewers.join(", ")}` - : reviewTagLabels[status]; - - return ( - - {label} - - ); -} - export function ChangesHeader({ onRefresh, viewMode, onViewModeChange, + showViewModeToggle = true, worktreePath, pr, isPRStatusLoading, @@ -298,14 +263,13 @@ export function ChangesHeader({ onStashPop={onStashPop} isPending={isStashPending} /> - - - {pr && pr.state === "open" && ( - )} + ; -} - -const checkIconConfig = { - success: { - icon: LuCheck, - className: "text-emerald-600 dark:text-emerald-400", - }, - failure: { icon: LuX, className: "text-red-600 dark:text-red-400" }, - pending: { - icon: LuLoaderCircle, - className: "text-amber-600 dark:text-amber-400", - }, - skipped: { icon: LuMinus, className: "text-muted-foreground" }, - cancelled: { icon: LuMinus, className: "text-muted-foreground" }, -} as const; - -function CheckRow({ check }: { check: CheckItem }) { - const { icon: Icon, className } = checkIconConfig[check.status]; - - const content = ( - - - {check.name} - - ); - - if (check.url) { - return ( - - {content} - - ); - } - - return
{content}
; -} - -export function PRChecksStatus({ pr }: PRChecksStatusProps) { - const [checksExpanded, setChecksExpanded] = useState(false); - - if (pr.state !== "open") return null; - - const relevantChecks = pr.checks.filter( - (c) => c.status !== "skipped" && c.status !== "cancelled", - ); - const passing = relevantChecks.filter((c) => c.status === "success").length; - const total = relevantChecks.length; - - if (total === 0) return null; - - const checksIcon = - checkIconConfig[pr.checksStatus === "none" ? "pending" : pr.checksStatus]; - - return ( -
- - - {checksExpanded && ( -
- {relevantChecks.map((check) => ( - - ))} -
- )} -
- ); -} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/PRChecksStatus/index.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/PRChecksStatus/index.ts deleted file mode 100644 index 1e46e5df80e..00000000000 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/PRChecksStatus/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { PRChecksStatus } from "./PRChecksStatus"; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx new file mode 100644 index 00000000000..915f2a23cc8 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/ReviewPanel.tsx @@ -0,0 +1,473 @@ +import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; +import { Avatar, AvatarFallback, AvatarImage } from "@superset/ui/avatar"; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from "@superset/ui/collapsible"; +import { Skeleton } from "@superset/ui/skeleton"; +import { toast } from "@superset/ui/sonner"; +import { cn } from "@superset/ui/utils"; +import { useEffect, useRef, useState } from "react"; +import { + LuArrowUpRight, + LuCheck, + LuCopy, + LuMessageSquareText, +} from "react-icons/lu"; +import { VscChevronRight } from "react-icons/vsc"; +import { electronTrpc } from "renderer/lib/electron-trpc"; +import { PRIcon } from "renderer/screens/main/components/PRIcon"; +import { + ALL_COMMENTS_COPY_ACTION_KEY, + buildAllCommentsClipboardText, + buildCommentClipboardText, + checkIconConfig, + checkSummaryIconConfig, + formatShortAge, + getCommentAvatarFallback, + getCommentCopyActionKey, + getCommentKindText, + getCommentPreviewText, + prStateLabel, + resolveCheckDestinationUrl, + reviewDecisionConfig, +} from "./utils"; + +interface ReviewPanelProps { + pr: GitHubStatus["pr"] | null; + comments?: PullRequestComment[]; + isLoading?: boolean; + isCommentsLoading?: boolean; +} + +export function ReviewPanel({ + pr, + comments = [], + isLoading = false, + isCommentsLoading = false, +}: ReviewPanelProps) { + const [checksOpen, setChecksOpen] = useState(true); + const [commentsOpen, setCommentsOpen] = useState(true); + const [copiedActionKey, setCopiedActionKey] = useState(null); + const copiedActionResetTimeoutRef = useRef | null>(null); + const copyToClipboardMutation = electronTrpc.external.copyText.useMutation(); + + useEffect(() => { + return () => { + if (copiedActionResetTimeoutRef.current) { + clearTimeout(copiedActionResetTimeoutRef.current); + } + }; + }, []); + + const markCopiedAction = (actionKey: string) => { + if (copiedActionResetTimeoutRef.current) { + clearTimeout(copiedActionResetTimeoutRef.current); + } + + setCopiedActionKey(actionKey); + copiedActionResetTimeoutRef.current = setTimeout(() => { + setCopiedActionKey(null); + copiedActionResetTimeoutRef.current = null; + }, 1500); + }; + + const copyTextToClipboard = async ({ + text, + actionKey, + errorLabel, + }: { + text: string; + actionKey: string; + errorLabel: string; + }) => { + try { + await copyToClipboardMutation.mutateAsync(text); + markCopiedAction(actionKey); + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown error"; + toast.error(`${errorLabel}: ${message}`); + } + }; + + const handleCopySingleComment = (comment: PullRequestComment) => { + void copyTextToClipboard({ + text: buildCommentClipboardText(comment), + actionKey: getCommentCopyActionKey(comment.id), + errorLabel: "Failed to copy comment", + }); + }; + + const handleCopyCommentsList = () => { + void copyTextToClipboard({ + text: buildAllCommentsClipboardText(comments), + actionKey: ALL_COMMENTS_COPY_ACTION_KEY, + errorLabel: "Failed to copy comments", + }); + }; + + if (isLoading && !pr) { + return ( +
+
+
+ + + +
+
+ + +
+
+
+
+ + +
+
+ + +
+
+
+
+ + +
+
+ + + +
+
+
+ ); + } + + if (!pr) { + return ( +
+ Open a pull request to view review status, checks, and comments. +
+ ); + } + + const requestedReviewers = pr.requestedReviewers ?? []; + const reviewLabel = + pr.reviewDecision === "pending" && requestedReviewers.length > 0 + ? `Awaiting ${requestedReviewers.join(", ")}` + : reviewDecisionConfig[pr.reviewDecision].label; + + const relevantChecks = pr.checks.filter( + (check) => check.status !== "skipped" && check.status !== "cancelled", + ); + const passingChecks = relevantChecks.filter( + (check) => check.status === "success", + ).length; + const checksSummary = + relevantChecks.length > 0 + ? `${passingChecks}/${relevantChecks.length} checks passing` + : "No checks reported"; + const checksStatus = relevantChecks.length > 0 ? pr.checksStatus : "none"; + const checksStatusConfig = checkSummaryIconConfig[checksStatus]; + const ChecksStatusIcon = checksStatusConfig.icon; + const hasComments = comments.length > 0; + const commentsCountLabel = isCommentsLoading ? "..." : comments.length; + const copyAllCommentsLabel = + copiedActionKey === ALL_COMMENTS_COPY_ACTION_KEY ? "Copied" : "Copy all"; + + return ( +
+
+
+ + + {pr.title} + + + #{pr.number} + +
+ +
+ + {reviewDecisionConfig[pr.reviewDecision].label} + + + {requestedReviewers.length > 0 + ? reviewLabel + : prStateLabel[pr.state]} + +
+
+ + + +
+ + Checks + + {relevantChecks.length} + +
+
+ + + {checksSummary} + +
+ + + {relevantChecks.length === 0 ? ( +
+ No active checks reported for this pull request yet. +
+ ) : ( + relevantChecks.map((check) => { + const { icon: CheckIcon, className } = + checkIconConfig[check.status]; + const checkUrl = resolveCheckDestinationUrl(check, pr.url); + + return checkUrl ? ( + +
+ +
+ {check.name} + +
+ {check.durationText && ( + + {check.durationText} + + )} +
+ + ) : ( +
+ + {check.name} + {check.durationText && ( + + {check.durationText} + + )} +
+ ); + }) + )} + + + + +
+ + + + Comments + + {commentsCountLabel} + + + {hasComments && ( + + )} +
+ +
+ {isCommentsLoading ? ( +
+ + + +
+ ) : comments.length === 0 ? ( +
+ No comments yet. +
+ ) : ( + comments.map((comment) => { + const age = formatShortAge(comment.createdAt); + const commentCopyActionKey = getCommentCopyActionKey( + comment.id, + ); + const isCopied = copiedActionKey === commentCopyActionKey; + const content = ( + <> + + {comment.avatarUrl ? ( + + ) : null} + + {getCommentAvatarFallback(comment.authorLogin)} + + +
+
+ + {comment.authorLogin} + + + {getCommentKindText(comment)} + + {age && ( + + {age} + + )} +
+

+ {getCommentPreviewText(comment.body)} +

+
+ + ); + + return ( +
+ {comment.url ? ( + + {content} + + ) : ( +
+ {content} +
+ )} +
+ {comment.url ? ( + + + + ) : null} + +
+
+ ); + }) + )} +
+ + +
+ ); +} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/index.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/index.ts new file mode 100644 index 00000000000..a5ab0593815 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/index.ts @@ -0,0 +1 @@ +export { ReviewPanel } from "./ReviewPanel"; diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/utils.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/utils.ts new file mode 100644 index 00000000000..7c512011773 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ReviewPanel/utils.ts @@ -0,0 +1,180 @@ +import type { GitHubStatus, PullRequestComment } from "@superset/local-db"; +import { LuCheck, LuLoaderCircle, LuMinus, LuX } from "react-icons/lu"; + +export type PullRequestCheck = NonNullable< + GitHubStatus["pr"] +>["checks"][number]; + +export const ALL_COMMENTS_COPY_ACTION_KEY = "comments:all"; + +export const reviewDecisionConfig = { + approved: { + label: "Approved", + className: + "border border-emerald-500/20 bg-emerald-500/10 text-emerald-700 dark:text-emerald-300", + }, + changes_requested: { + label: "Changes requested", + className: + "border border-red-500/20 bg-red-500/10 text-red-700 dark:text-red-300", + }, + pending: { + label: "Review pending", + className: + "border border-amber-500/20 bg-amber-500/10 text-amber-700 dark:text-amber-300", + }, +} as const; + +export const checkIconConfig = { + success: { + icon: LuCheck, + className: "text-emerald-600 dark:text-emerald-400", + label: "Passed", + }, + failure: { + icon: LuX, + className: "text-red-600 dark:text-red-400", + label: "Failed", + }, + pending: { + icon: LuLoaderCircle, + className: "text-amber-600 dark:text-amber-400", + label: "Pending", + }, + skipped: { + icon: LuMinus, + className: "text-muted-foreground", + label: "Skipped", + }, + cancelled: { + icon: LuMinus, + className: "text-muted-foreground", + label: "Cancelled", + }, +} as const; + +export const checkSummaryIconConfig = { + success: checkIconConfig.success, + failure: checkIconConfig.failure, + pending: checkIconConfig.pending, + none: { + icon: LuMinus, + className: "text-muted-foreground", + label: "No checks", + }, +} as const; + +export const prStateLabel = { + open: "Open", + draft: "Draft", + merged: "Merged", + closed: "Closed", +} as const; + +export function resolveCheckDestinationUrl( + check: PullRequestCheck, + prUrl: string, +): string | undefined { + if (check.url) { + return check.url; + } + + const normalizedName = check.name.trim().toLowerCase(); + if ( + normalizedName.includes("coderabbit") || + normalizedName.includes("code rabbit") + ) { + return prUrl; + } + + return undefined; +} + +export function getCommentPreviewText(body: string): string { + return ( + body + .replace(//g, "\n") + .split(/\r?\n/) + .map((line) => line.trim()) + .find(Boolean) + ?.replace(/^[-*+>]\s*/, "") + ?.replace(/\s+/g, " ") ?? "No preview available" + ); +} + +export function getCommentAvatarFallback(authorLogin: string): string { + return authorLogin.slice(0, 2).toUpperCase(); +} + +export function formatShortAge(timestamp?: number): string | null { + if (!timestamp || Number.isNaN(timestamp)) { + return null; + } + + const deltaMs = Math.max(0, Date.now() - timestamp); + const deltaSeconds = Math.round(deltaMs / 1000); + + if (deltaSeconds < 60) { + return `${Math.max(1, deltaSeconds)}s`; + } + + const deltaMinutes = Math.round(deltaSeconds / 60); + if (deltaMinutes < 60) { + return `${deltaMinutes}m`; + } + + const deltaHours = Math.round(deltaMinutes / 60); + if (deltaHours < 24) { + return `${deltaHours}h`; + } + + return `${Math.round(deltaHours / 24)}d`; +} + +function getCommentClipboardLocation( + comment: PullRequestComment, +): string | null { + if (comment.path) { + return comment.line ? `${comment.path}:${comment.line}` : comment.path; + } + + return comment.kind === "conversation" ? "Conversation" : null; +} + +export function getCommentKindText(comment: PullRequestComment): string { + return comment.kind === "review" ? "Review" : "Comment"; +} + +export function buildCommentClipboardText( + comment: PullRequestComment, + includeMetadata = false, +): string { + const body = comment.body.trim() || "No comment body"; + + if (!includeMetadata) { + return body; + } + + const location = getCommentClipboardLocation(comment); + const metadata = [ + comment.authorLogin, + getCommentKindText(comment), + location, + ].filter(Boolean); + + return [metadata.join(" • "), body].filter(Boolean).join("\n"); +} + +export function buildAllCommentsClipboardText( + comments: PullRequestComment[], +): string { + return comments + .map((comment) => buildCommentClipboardText(comment, true)) + .join("\n\n---\n\n"); +} + +export function getCommentCopyActionKey( + commentId: PullRequestComment["id"], +): string { + return `comment:${commentId}`; +} diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/headerTabStyles.ts b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/headerTabStyles.ts new file mode 100644 index 00000000000..627cb742654 --- /dev/null +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/headerTabStyles.ts @@ -0,0 +1,29 @@ +import { cn } from "@superset/ui/utils"; + +const SIDEBAR_HEADER_TAB_ACTIVE_CLASS_NAME = "text-foreground bg-border/30"; +const SIDEBAR_HEADER_TAB_INACTIVE_CLASS_NAME = + "text-muted-foreground/70 hover:text-muted-foreground hover:bg-tertiary/20"; + +export function getSidebarHeaderTabButtonClassName({ + isActive, + compact = false, +}: { + isActive: boolean; + compact?: boolean; +}) { + return cn( + "h-full shrink-0 transition-all", + compact + ? "flex w-10 items-center justify-center" + : "flex items-center gap-2 px-3 text-sm", + isActive + ? SIDEBAR_HEADER_TAB_ACTIVE_CLASS_NAME + : SIDEBAR_HEADER_TAB_INACTIVE_CLASS_NAME, + ); +} + +export const sidebarHeaderTabTriggerClassName = cn( + "flex h-full flex-none shrink-0 items-center gap-2 rounded-none border-0 bg-transparent px-3 text-sm font-normal shadow-none transition-all outline-none", + "data-[state=active]:bg-border/30 data-[state=active]:text-foreground data-[state=active]:shadow-none", + "data-[state=inactive]:text-muted-foreground/70 data-[state=inactive]:hover:bg-tertiary/20 data-[state=inactive]:hover:text-muted-foreground", +); diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx index 7ab8514ce48..4fd295c53d0 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx @@ -1,6 +1,5 @@ import { Button } from "@superset/ui/button"; import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; -import { cn } from "@superset/ui/utils"; import { useParams } from "@tanstack/react-router"; import { useCallback } from "react"; import { @@ -23,6 +22,7 @@ import type { ChangeCategory, ChangedFile } from "shared/changes-types"; import { useScrollContext } from "../ChangesContent"; import { ChangesView } from "./ChangesView"; import { FilesView } from "./FilesView"; +import { getSidebarHeaderTabButtonClassName } from "./headerTabStyles"; function TabButton({ isActive, @@ -44,12 +44,10 @@ function TabButton({ @@ -65,12 +63,7 @@ function TabButton({