diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index cfc876dc39c1..b5b5fa2e4a3c 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -145,6 +145,7 @@ import { PULL_REQUEST_MERGE_METHOD_LABELS, readableFailure, readPullRequestDetailSnapshot, + resolvePullRequestReferenceHost, resolveDisplayedPullRequestDetail, resolvePullRequestPrimaryControl, allowsSinglePullRequestMerge, @@ -523,18 +524,23 @@ export function PullRequestDetailPanel({ onBack?: (() => void) | undefined; }) { const environmentConfigs = useServerConfigs(); + const projects = useProjects(); + const repositoryIdentity = projects.find( + (project) => + project.id === requestedReference.projectId && project.environmentId === environmentId, + )?.repositoryIdentity; const supportsThreadPullRequests = environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true; const reference = useMemo( () => supportsThreadPullRequests - ? requestedReference + ? resolvePullRequestReferenceHost(requestedReference, repositoryIdentity) : { projectId: requestedReference.projectId, repository: requestedReference.repository, number: requestedReference.number, }, - [requestedReference, supportsThreadPullRequests], + [requestedReference, repositoryIdentity, supportsThreadPullRequests], ); const pullRequestKey = `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`; const matchingListEntry = @@ -881,7 +887,6 @@ export function PullRequestDetailPanel({ const newThread = useNewThreadHandler(); const { environments } = useEnvironments(); const primaryEnvironmentId = usePrimaryEnvironmentId(); - const projects = useProjects(); const unavailableGitHubUrl = useMemo(() => { const identity = projects.find( (project) => project.id === reference.projectId && project.environmentId === environmentId, diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index ae5a5067bf12..96133395f048 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -1,6 +1,7 @@ import { resolvePlanFollowUpSubmission } from "../../proposedPlan"; import { serializeLegacyContextMessage } from "@t3tools/shared/composerContextLegacySend"; import { + ProjectId, PullRequestAction, type PullRequestCheck, type PullRequestComment, @@ -40,6 +41,7 @@ import { readableFailure, readPullRequestDetailSnapshot, resolveDisplayedPullRequestDetail, + resolvePullRequestReferenceHost, resolvePullRequestPrimaryControl, allowsSinglePullRequestMerge, shouldRefreshPullRequestActivity, @@ -1440,7 +1442,7 @@ describe("which actions need the host read again after they run", () => { }); describe("cached pull request detail", () => { - const reference = { projectId: "project-1", repository: "acme/web", number: 7 }; + const reference = { projectId: ProjectId.make("project-1"), repository: "acme/web", number: 7 }; const detail = (overrides: Partial = {}): PullRequestDetail => ({ provider: "github", @@ -1511,6 +1513,106 @@ describe("cached pull request detail", () => { expect(snapshot?.deletions).toBe(3); }); + it("reuses a host-qualified snapshot when reopening a thread link without a host", () => { + const storage = makeStorage(); + writePullRequestDetailSnapshot( + storage, + "env-1", + { ...reference, host: "github.com" }, + detail(), + ); + const resolved = resolvePullRequestReferenceHost(reference, { + canonicalKey: "github.com/acme/web", + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "https://github.com/acme/web.git", + }, + provider: "github", + }); + expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe( + "Cache the title", + ); + const explicit = { ...reference, host: "github.example.com" }; + expect( + resolvePullRequestReferenceHost(explicit, { + canonicalKey: "github.com/acme/web", + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "https://github.com/acme/web.git", + }, + }), + ).toBe(explicit); + }); + + it("leaves server-resolved Azure SSH references unchanged", () => { + expect( + resolvePullRequestReferenceHost(reference, { + canonicalKey: "ssh.dev.azure.com/v3/org/project/web", + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "git@ssh.dev.azure.com:v3/org/project/web", + }, + provider: "azure-devops", + }), + ).toBe(reference); + expect(resolvePullRequestReferenceHost(reference, undefined)).toBe(reference); + }); + + it("hydrates legacy hostless snapshots only for the matching host", () => { + const storage = makeStorage(); + writePullRequestDetailSnapshot(storage, "env-1", reference, detail()); + expect( + readPullRequestDetailSnapshot(storage, "env-1", { ...reference, host: "github.com" })?.title, + ).toBe("Cache the title"); + expect( + readPullRequestDetailSnapshot(storage, "env-1", { + ...reference, + host: "github.example.com", + }), + ).toBeNull(); + }); + + it("keeps Forgejo ports isolated when recovering legacy snapshots", () => { + const storage = makeStorage(); + const cached = detail({ + provider: "forgejo", + url: "https://forge.example:8443/acme/web/pulls/7", + }); + writePullRequestDetailSnapshot(storage, "env-1", reference, cached); + const resolved = { ...reference, host: "forge.example:8443" }; + expect(readPullRequestDetailSnapshot(storage, "env-1", resolved)?.title).toBe(cached.title); + expect( + readPullRequestDetailSnapshot(storage, "env-1", { + ...reference, + host: "forge.example:9443", + }), + ).toBeNull(); + expect( + readPullRequestDetailSnapshot(storage, "env-1", { + ...reference, + host: "forge.example", + }), + ).toBeNull(); + }); + + it.each(["github", "gitlab"] as const)( + "retains portless %s snapshot identities for custom web ports", + (provider) => { + const storage = makeStorage(); + const host = `${provider}.example.com`; + const hosted = { ...reference, host }; + const cached = detail({ + provider, + url: `https://${host}:8443/acme/web/${provider === "github" ? "pull" : "-/merge_requests"}/7`, + }); + writePullRequestDetailSnapshot(storage, "env-1", hosted, cached); + expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)?.title).toBe(cached.title); + }, + ); + it("keeps a cached tab painted while the live read replaces the counts", () => { const cached = detail(); const live = detail({ additions: 40, deletions: 9, title: "Cache the title" }); @@ -1534,11 +1636,11 @@ describe("cached pull request detail", () => { it("isolates stored and displayed details between hosts with the same repository and number", () => { const storage = makeStorage(); const publicRef = { ...reference, host: "github.com" }; - const enterpriseRef = { ...reference, host: "github.example.com" }; + const enterpriseRef = { ...reference, host: "ghe.example.com" }; const publicDetail = detail(); const enterpriseDetail = detail({ title: "Enterprise change", - url: "https://github.example.com/acme/web/pull/7", + url: "https://ghe.example.com/acme/web/pull/7", }); writePullRequestDetailSnapshot(storage, "env-1", publicRef, publicDetail); expect(readPullRequestDetailSnapshot(storage, "env-1", enterpriseRef)).toBeNull(); @@ -1572,6 +1674,9 @@ describe("cached pull request detail", () => { storage.setItem("t3.pullRequests.detail:env-1:project-1:acme/web#7", "{not json"); expect(readPullRequestDetailSnapshot(storage, "env-1", reference)).toBeNull(); expect(readPullRequestDetailSnapshot(undefined, "env-1", reference)).toBeNull(); + const hosted = { ...reference, host: "github.com" }; + writePullRequestDetailSnapshot(storage, "env-1", hosted, detail({ url: "invalid url" })); + expect(readPullRequestDetailSnapshot(storage, "env-1", hosted)).toBeNull(); }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index be71c95ddc40..56e22e643e1e 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -1,8 +1,8 @@ import * as Schema from "effect/Schema"; -import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; import { PullRequestDetail, + pullRequestHostOf, type PullRequestAction, type PullRequestActor, type PullRequestBaseComparison, @@ -15,6 +15,8 @@ import { type PullRequestMergeability, type PullRequestMergeMethod, type PullRequestReaction, + type PullRequestRef, + type RepositoryIdentity, type PullRequestReviewThread, type PullRequestState, type PullRequestUpdateMethod, @@ -1092,6 +1094,15 @@ export function pullRequestActionNeedsHostRefresh(action: PullRequestAction): bo type SnapshotStorage = Pick; +export function resolvePullRequestReferenceHost( + reference: PullRequestRef, + identity: RepositoryIdentity | null | undefined, +): PullRequestRef { + // Other providers may resolve an SSH remote to a different web authority on the server. + if (reference.host !== undefined || identity?.provider !== "github") return reference; + return { ...reference, host: pullRequestHostOf(identity, "github") }; +} + export interface PullRequestDetailSnapshotRef { readonly host?: string | undefined; readonly projectId: string; @@ -1121,7 +1132,13 @@ export function readPullRequestDetailSnapshot( reference: PullRequestDetailSnapshotRef, ): PullRequestDetail | null { try { - const raw = storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference)); + const raw = + storage?.getItem(pullRequestDetailSnapshotKey(environmentId, reference)) ?? + (reference.host === undefined + ? null + : storage?.getItem( + pullRequestDetailSnapshotKey(environmentId, { ...reference, host: undefined }), + )); if (!raw) return null; const decoded = decodeDetailSnapshot(JSON.parse(raw)); return decoded._tag === "Some" @@ -1157,14 +1174,22 @@ export function resolveDisplayedPullRequestDetail(input: { }): PullRequestDetail | null { if (input.live !== null) return input.live; if ( - input.cached !== null && - input.cached.projectId === input.reference.projectId && - input.cached.repository.toLowerCase() === input.reference.repository.toLowerCase() && - input.cached.number === input.reference.number && - (input.reference.host === undefined || - parseChangeRequestUrl(input.cached.url)?.host === input.reference.host.toLowerCase()) + input.cached === null || + input.cached.projectId !== input.reference.projectId || + input.cached.repository.toLowerCase() !== input.reference.repository.toLowerCase() || + input.cached.number !== input.reference.number ) { - return input.cached; + return null; + } + if (input.reference.host === undefined) return input.cached; + try { + const url = new URL(input.cached.url); + const host = input.cached.provider === "forgejo" ? url.host : url.hostname; + return (url.protocol === "https:" || url.protocol === "http:") && + host.toLowerCase() === input.reference.host.toLowerCase() + ? input.cached + : null; + } catch { + return null; } - return null; } diff --git a/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts b/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts index 20d31c6392a0..6719eaf89e40 100644 --- a/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts +++ b/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts @@ -5,6 +5,7 @@ import { useMemo } from "react"; import { readPullRequestDetailSnapshot, resolveDisplayedPullRequestDetail, + resolvePullRequestReferenceHost, } from "../components/pullRequest/pullRequestDetail.logic"; import { gitHubPullRequestBrowserUrl } from "../lib/openPullRequestLink"; import { selectActiveRightPanelSurface, useRightPanelStore } from "../rightPanelStore"; @@ -17,28 +18,38 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) { const surface = useRightPanelStore((state) => selectActiveRightPanelSurface(state.byThreadKey, threadRef), ); - const reference = surface?.kind === "pull-request" ? surface : null; - const environmentId = reference?.environmentId - ? EnvironmentId.make(reference.environmentId) + const requestedReference = surface?.kind === "pull-request" ? surface : null; + const environmentId = requestedReference?.environmentId + ? EnvironmentId.make(requestedReference.environmentId) : threadRef?.environmentId; const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId ?? null); const project = useProject( - reference && environmentId - ? scopeProjectRef(environmentId, ProjectId.make(reference.projectId)) + requestedReference && environmentId + ? scopeProjectRef(environmentId, ProjectId.make(requestedReference.projectId)) : null, ); + const reference = useMemo(() => { + if (requestedReference === null) return null; + const input = { + projectId: ProjectId.make(requestedReference.projectId), + repository: requestedReference.repository, + number: requestedReference.number, + }; + return supportsMultiplePullRequests + ? resolvePullRequestReferenceHost( + { + ...input, + ...(requestedReference.host === undefined ? {} : { host: requestedReference.host }), + }, + project?.repositoryIdentity, + ) + : input; + }, [requestedReference, supportsMultiplePullRequests, project?.repositoryIdentity]); const detail = useEnvironmentQuery( reference && environmentId ? pullRequestEnvironment.detail({ environmentId, - input: { - projectId: ProjectId.make(reference.projectId), - ...(supportsMultiplePullRequests && reference.host !== undefined - ? { host: reference.host } - : {}), - repository: reference.repository, - number: reference.number, - }, + input: reference, }) : null, ).data; @@ -55,7 +66,7 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) { ); return reference ? (resolveDisplayedPullRequestDetail({ live: detail, cached: cachedDetail, reference })?.url ?? - reference.url ?? + requestedReference?.url ?? gitHubPullRequestBrowserUrl( project?.repositoryIdentity, reference.repository,