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
35 changes: 35 additions & 0 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,41 @@ it.effect("refines unknown self-hosted GitLab projects before listing merge requ
}),
);

it.effect("refines unknown self-hosted GitHub projects before listing pull requests", () =>
Effect.gen(function* () {
let refinementCalls = 0;
const selfHosted = project({
id: "p1",
title: "self-hosted-gh",
workspaceRoot: "/github-enterprise",
repository: "corp/platform",
provider: "unknown",
host: "git.enterprise.test",
});
const service = yield* makeService({
projects: [
selfHosted,
{ ...selfHosted, id: "p2" as ProjectId, workspaceRoot: "/github-enterprise-worktree" },
],
providers: [fakeProvider("github")],
resolveHandle: ({ context }) => {
refinementCalls += 1;
assert.strictEqual(context?.remoteUrl, "https://git.enterprise.test/corp/platform.git");
return Effect.succeed({
context: { ...context!, provider: { ...context!.provider, kind: "github" } },
provider: undefined as never,
});
},
});

const result = yield* service.list({ state: "open" });

assert.strictEqual(refinementCalls, 1);
assert.strictEqual(result.providers[0]?.host, "git.enterprise.test");
assert.strictEqual(result.providers[0]?.kind, "github");
}),
);

it.effect("derives a legacy repository host after refining its provider", () =>
Effect.gen(function* () {
const current = project({
Expand Down
12 changes: 11 additions & 1 deletion apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,19 @@ export const make = Effect.gen(function* () {
else counted.projectCount += 1;
continue;
}
const refinedProject =
kind !== identity.provider
? {
...project,
repositoryIdentity: {
...identity,
provider: kind,
},
}
: project;
supported.push({
cursorKey: key,
project,
project: refinedProject,
api: withRateLimitBackoff(api, host, rateLimits),
repository,
host,
Expand Down
67 changes: 67 additions & 0 deletions apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,70 @@ it("reports an update hint instead of unauthenticated when gh predates --json",
/2\.81\.0/,
);
});

it("refines unknown GitHub remotes with mixed-case provider hosts", () => {
const provider = GitHubSourceControlProvider.discovery.refineUnknownRemote?.({
cwd: "/repo",
context: {
provider: {
kind: "unknown",
name: "Enterprise.Example.Test",
baseUrl: "https://Enterprise.Example.Test",
},
remoteName: "origin",
remoteUrl: "https://Enterprise.Example.Test/org/repo.git",
},
auth: processResult(
JSON.stringify({
hosts: {
"enterprise.example.test": [
{
state: "success",
active: true,
host: "enterprise.example.test",
login: "enterprise-user",
},
],
},
}),
),
});

assert.deepStrictEqual(provider, {
kind: "github",
name: "GitHub Enterprise",
baseUrl: "https://Enterprise.Example.Test",
});
});

it("returns null when refining unknown remote with unauthenticated host", () => {
const provider = GitHubSourceControlProvider.discovery.refineUnknownRemote?.({
cwd: "/repo",
context: {
provider: {
kind: "unknown",
name: "enterprise.example.test",
baseUrl: "https://enterprise.example.test",
},
remoteName: "origin",
remoteUrl: "https://enterprise.example.test/org/repo.git",
},
auth: processResult(
JSON.stringify({
hosts: {
"enterprise.example.test": [
{
state: "error",
active: true,
host: "enterprise.example.test",
login: "enterprise-user",
error: "Token expired",
},
],
},
}),
),
});

assert.strictEqual(provider, null);
});
19 changes: 19 additions & 0 deletions apps/server/src/sourceControl/GitHubSourceControlProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
providerAuth,
type SourceControlAuthProbeInput,
type SourceControlCliDiscoverySpec,
type SourceControlUnknownRemoteRefinementInput,
} from "./SourceControlProviderDiscovery.ts";

function toChangeRequest(summary: GitHubCli.GitHubPullRequestSummary): ChangeRequest {
Expand Down Expand Up @@ -98,6 +99,23 @@ function parseGitHubAuth(input: SourceControlAuthProbeInput) {
});
}

function refineUnknownGitHubRemote(input: SourceControlUnknownRemoteRefinementInput) {
const host = input.context.provider.name.toLowerCase();
const authenticated = parseGitHubAuthStatus(combinedAuthOutput(input.auth)).accounts.some(
(entry) => entry.authenticated && entry.host === host,
);

if (!authenticated) {
return null;
}

return {
kind: "github",
name: "GitHub Enterprise",
baseUrl: input.context.provider.baseUrl,
} as const;
}

export const discovery = {
type: "cli",
kind: "github",
Expand All @@ -106,6 +124,7 @@ export const discovery = {
versionArgs: ["--version"],
authArgs: ["auth", "status", "--json", "hosts"],
parseAuth: parseGitHubAuth,
refineUnknownRemote: refineUnknownGitHubRemote,
installHint:
"Install the GitHub command-line tool (`gh`) via https://cli.github.com/ or your package manager (for example `brew install gh`).",
} satisfies SourceControlCliDiscoverySpec;
Expand Down
47 changes: 31 additions & 16 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ import {
} from "lucide-react";
import { cn, randomHex, randomUUID } from "~/lib/utils";
import { stackedThreadToast, toastManager } from "./ui/toast";
import { gitHubPullRequestBrowserUrl } from "~/lib/openPullRequestLink";
import { fallbackPullRequestBrowserUrl } from "~/lib/openPullRequestLink";
import { usePullRequestProviders } from "~/state/pullRequests";
import { decodeProjectScriptKeybindingRule } from "~/lib/projectScriptKeybindings";
import { type NewProjectScriptInput } from "./ProjectScriptsControl";
import {
Expand Down Expand Up @@ -2659,20 +2660,33 @@ export default function ChatView(props: ChatViewProps) {
});
const pullRequestsCapabilityKnown = serverConfig !== null;
const supportsPullRequests = serverConfig?.environment.capabilities.pullRequests === true;
// Same fallback the load-failure view offers: a pull request stays readable on
// GitHub even when this environment's server is too old to browse it here.
const pullRequestsUnavailableGitHubUrl =
activeRightPanelSurface?.kind === "pull-request"
? gitHubPullRequestBrowserUrl(
allProjects.find(
(project) =>
project.id === activeRightPanelSurface.projectId &&
project.environmentId === activeThread?.environmentId,
)?.repositoryIdentity,
activeRightPanelSurface.repository,
activeRightPanelSurface.number,
)
: null;
// Same fallback the load-failure view offers: a pull request stays readable in
// browser even when this environment's server is too old to browse it here.
const observedPullRequestProviders = usePullRequestProviders();
const pullRequestsUnavailableFallbackLink = useMemo(() => {
if (activeRightPanelSurface?.kind !== "pull-request") return null;
const identity = allProjects.find(
(project) =>
project.id === activeRightPanelSurface.projectId &&
project.environmentId === activeThread?.environmentId,
)?.repositoryIdentity;
const host = activeRightPanelSurface.host ?? identity?.canonicalKey?.split("/")[0];
const providerKind = host
? observedPullRequestProviders.get(host.toLowerCase())?.kind
: undefined;
return fallbackPullRequestBrowserUrl(
identity,
activeRightPanelSurface.repository,
activeRightPanelSurface.number,
providerKind,
);
}, [
activeRightPanelSurface,
activeThread?.environmentId,
allProjects,
observedPullRequestProviders,
]);
const pullRequestsUnavailableGitHubUrl = pullRequestsUnavailableFallbackLink?.url ?? null;
const attachmentEnvironmentConfig = environmentById.get(environmentId)?.serverConfig ?? null;
const attachmentUploadsCapabilityKnown = attachmentEnvironmentConfig !== null;
const supportsQuestionAttachments =
Expand Down Expand Up @@ -10177,9 +10191,10 @@ export default function ChatView(props: ChatViewProps) {
<PullRequestsUnavailableState
title="Pull requests unavailable"
error="Update this environment's Pylon server to browse pull requests."
// The pull request is still readable on GitHub even when this server is
// The pull request is still readable in browser even when this server is
// too old to browse it here, so this surface gets the same way out as
// the load-failure one rather than being a dead end.
externalLink={pullRequestsUnavailableFallbackLink}
{...(pullRequestsUnavailableGitHubUrl
? { gitHubUrl: pullRequestsUnavailableGitHubUrl }
: {})}
Expand Down
31 changes: 27 additions & 4 deletions apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ import {
derivePhysicalProjectKey,
selectProjectGroupingSettings,
} from "~/logicalProject";
import { changeRequestRepositoryUrl, gitHubPullRequestBrowserUrl } from "~/lib/openPullRequestLink";
import {
changeRequestRepositoryUrl,
fallbackPullRequestBrowserUrl,
gitHubPullRequestBrowserUrl,
} from "~/lib/openPullRequestLink";
import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions";
import { cn } from "~/lib/utils";
import { readLocalApi } from "~/localApi";
Expand All @@ -83,6 +87,7 @@ import { useEnvironmentQuery } from "~/state/query";
import { useLiveRefresh } from "~/hooks/useLiveRefresh";
import {
pullRequestEnvironment,
usePullRequestProviders,
pullRequestListEntryToSummary,
newestPullRequestSummary,
usePullRequestTurnRefresh,
Expand Down Expand Up @@ -898,12 +903,29 @@ export function PullRequestDetailPanel({
const newThread = useNewThreadHandler();
const { environments } = useEnvironments();
const primaryEnvironmentId = usePrimaryEnvironmentId();
const unavailableGitHubUrl = useMemo(() => {
const observedProviders = usePullRequestProviders();
const unavailableFallbackLink = useMemo(() => {
const identity = projects.find(
(project) => project.id === reference.projectId && project.environmentId === environmentId,
)?.repositoryIdentity;
return gitHubPullRequestBrowserUrl(identity, reference.repository, reference.number);
}, [environmentId, projects, reference.number, reference.projectId, reference.repository]);
const host = reference.host ?? identity?.canonicalKey?.split("/")[0];
const providerKind = host ? observedProviders.get(host.toLowerCase())?.kind : undefined;
return fallbackPullRequestBrowserUrl(
identity,
reference.repository,
reference.number,
providerKind,
);
}, [
environmentId,
observedProviders,
projects,
reference.host,
reference.number,
reference.projectId,
reference.repository,
]);
const unavailableGitHubUrl = unavailableFallbackLink?.url ?? null;
// Project settings stored the override under the sidebar group's key, which a duplicate row
// borrows from its siblings, so the project alone does not always name the same key.
const legacyProjectDefaultMergeMethod = useMemo(() => {
Expand Down Expand Up @@ -2741,6 +2763,7 @@ export function PullRequestDetailPanel({
error={detailQuery.error}
refreshing={detailQuery.isPending}
onRetry={refreshDetail}
externalLink={unavailableFallbackLink}
{...(unavailableGitHubUrl ? { gitHubUrl: unavailableGitHubUrl } : {})}
/>
) : detail ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,28 @@ import {
} from "../ui/empty";
import { PullRequestGlyph } from "./pullRequestIcons";

export interface PullRequestExternalLink {
readonly url: string;
readonly label?: string;
}

export function PullRequestsUnavailableState({
title = "Could not load pull requests",
error,
onRetry,
refreshing = false,
gitHubUrl,
externalLink,
}: {
title?: string;
error: string;
onRetry?: () => void;
refreshing?: boolean;
gitHubUrl?: string;
externalLink?: PullRequestExternalLink | null;
}) {
const link = externalLink ?? (gitHubUrl ? { url: gitHubUrl, label: "Open on GitHub" } : null);

return (
<Empty className="min-h-0 justify-center-safe overflow-y-auto px-4 py-16 md:px-4 [&>*]:shrink-0">
<EmptyMedia variant="icon">
Expand All @@ -36,7 +45,7 @@ export function PullRequestsUnavailableState({
shows its message rather than trying to infer one from the failure text. */}
<EmptyDescription>{error}</EmptyDescription>
</EmptyHeader>
{onRetry || gitHubUrl ? (
{onRetry || link ? (
<EmptyContent className="flex-row flex-wrap justify-center gap-2">
{onRetry ? (
<Button
Expand All @@ -50,14 +59,14 @@ export function PullRequestsUnavailableState({
Retry
</Button>
) : null}
{gitHubUrl ? (
{link ? (
<Button
size="sm"
variant="outline"
render={<a href={gitHubUrl} target="_blank" rel="noopener noreferrer" />}
render={<a href={link.url} target="_blank" rel="noopener noreferrer" />}
>
<ExternalLinkIcon aria-hidden className="size-3.5" />
Open on GitHub
{link.label ?? "Open in browser"}
</Button>
) : null}
</EmptyContent>
Expand Down
12 changes: 8 additions & 4 deletions apps/web/src/hooks/useOpenPanelPullRequestUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import {
resolveDisplayedPullRequestDetail,
resolvePullRequestReferenceHost,
} from "../components/pullRequest/pullRequestDetail.logic";
import { gitHubPullRequestBrowserUrl } from "../lib/openPullRequestLink";
import { fallbackPullRequestBrowserUrl } from "../lib/openPullRequestLink";
import { selectActiveRightPanelSurface, useRightPanelStore } from "../rightPanelStore";
import { useProject } from "../state/entities";
import { pullRequestEnvironment } from "../state/pullRequests";
import { pullRequestEnvironment, usePullRequestProviders } from "../state/pullRequests";
import { useEnvironmentQuery } from "../state/query";
import { useSupportsMultiplePullRequests } from "./useSupportsMultiplePullRequests";

export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) {
const observedProviders = usePullRequestProviders();
const surface = useRightPanelStore((state) =>
selectActiveRightPanelSurface(state.byThreadKey, threadRef),
);
Expand Down Expand Up @@ -72,10 +73,13 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) {
reference,
})?.url ??
requestedReference?.url ??
gitHubPullRequestBrowserUrl(
fallbackPullRequestBrowserUrl(
project?.repositoryIdentity,
reference.repository,
reference.number,
))
"host" in reference && typeof reference.host === "string" && reference.host.length > 0
? observedProviders.get(reference.host.toLowerCase())?.kind
: undefined,
)?.url)
: undefined;
}
Loading
Loading