authorgravatar for 97427849+flamboh@users.noreply.github.comoliver <97427849+flamboh@users.noreply.github.com> 2026-10-03 18:40:10-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-03 22:40:10-03:00
log7bc161f8692dfb969489f40bf3eb5bd27bc01f10
tree6347c1971ca81f236521532e87ba7672d4207d09
parent2b8bb0c1881583428adbc07e3b641b21067ab41a
signature Signed by PGP key B5690EEEBB952194

fix(web): links to issues no longer strand the pull request viewer (#14242)

12 files changed, 73 insertions(+), 6 deletions(-)

apps/server/src/pullRequest/AzureDevOpsPullRequestProvider.ts+1
...@@ -117,6 +117,7 @@ export function azureDevOpsProviderFailure(...@@ -117,6 +117,7 @@ export function azureDevOpsProviderFailure(
117 if (error._tag === "AzureDevOpsCliUnavailableError") return { reason: "missing-tool" };117 if (error._tag === "AzureDevOpsCliUnavailableError") return { reason: "missing-tool" };
118 if (error._tag === "AzureDevOpsCliAuthenticationError") return { reason: "unauthenticated" };118 if (error._tag === "AzureDevOpsCliAuthenticationError") return { reason: "unauthenticated" };
119 if (error._tag === "AzureDevOpsCliRateLimitError") return { reason: "rate-limited" };119 if (error._tag === "AzureDevOpsCliRateLimitError") return { reason: "rate-limited" };
120 if (error._tag === "AzureDevOpsPullRequestNotFoundError") return { reason: "not-found" };
120 return { reason: "failed" };121 return { reason: "failed" };
121}122}
122123
apps/server/src/pullRequest/BitbucketPullRequestProvider.ts+6
...@@ -82,6 +82,12 @@ export function bitbucketProviderFailure(...@@ -82,6 +82,12 @@ export function bitbucketProviderFailure(
82 ...(error.retryAt === undefined ? {} : { retryAt: error.retryAt }),82 ...(error.retryAt === undefined ? {} : { retryAt: error.retryAt }),
83 };83 };
84 }84 }
85 if (
86 (error._tag === "BitbucketResponseError" || error._tag === "BitbucketResponseBodyReadError") &&
87 error.status === 404
88 ) {
89 return { reason: "not-found" };
90 }
85 return { reason: "failed" };91 return { reason: "failed" };
86}92}
8793
apps/server/src/pullRequest/ForgejoPullRequestProvider.ts+3-1
...@@ -89,7 +89,9 @@ export const make = Effect.gen(function* () {...@@ -89,7 +89,9 @@ export const make = Effect.gen(function* () {
89 ? "unauthenticated"89 ? "unauthenticated"
90 : error.reason === "rate-limit"90 : error.reason === "rate-limit"
91 ? "rate-limited"91 ? "rate-limited"
92 : "failed",92 : error.reason === "not-found"
93 ? "not-found"
94 : "failed",
93 }),95 }),
94 ),96 ),
95 );97 );
apps/server/src/pullRequest/GitHubPullRequestProvider.ts+1
...@@ -115,6 +115,7 @@ export function gitHubProviderFailure(...@@ -115,6 +115,7 @@ export function gitHubProviderFailure(
115 if (error._tag === "SourceControlRateLimitPausedError") {115 if (error._tag === "SourceControlRateLimitPausedError") {
116 return { reason: "rate-limited", retryAt: error.retryAt };116 return { reason: "rate-limited", retryAt: error.retryAt };
117 }117 }
118 if (error._tag === "GitHubPullRequestNotFoundError") return { reason: "not-found" };
118 return { reason: "failed" };119 return { reason: "failed" };
119}120}
120121
apps/server/src/pullRequest/GitLabPullRequestProvider.ts+1
...@@ -102,6 +102,7 @@ export function gitLabProviderFailure(...@@ -102,6 +102,7 @@ export function gitLabProviderFailure(
102 if (error._tag === "GitLabCliUnavailableError") return { reason: "missing-tool" };102 if (error._tag === "GitLabCliUnavailableError") return { reason: "missing-tool" };
103 if (error._tag === "GitLabCliAuthenticationError") return { reason: "unauthenticated" };103 if (error._tag === "GitLabCliAuthenticationError") return { reason: "unauthenticated" };
104 if (error._tag === "GitLabCliRateLimitError") return { reason: "rate-limited" };104 if (error._tag === "GitLabCliRateLimitError") return { reason: "rate-limited" };
105 if (error._tag === "GitLabMergeRequestNotFoundError") return { reason: "not-found" };
105 return { reason: "failed" };106 return { reason: "failed" };
106}107}
107108
apps/server/src/pullRequest/PullRequestProvider.ts+7-1
...@@ -52,7 +52,13 @@ export class PullRequestProviderError extends Schema.TaggedError<PullRequestProv...@@ -52,7 +52,13 @@ export class PullRequestProviderError extends Schema.TaggedError<PullRequestProv
52 {52 {
53 provider: SourceControlProviderKindSchema,53 provider: SourceControlProviderKindSchema,
54 operation: Schema.String,54 operation: Schema.String,
55 reason: Schema.Literals(["missing-tool", "unauthenticated", "rate-limited", "failed"]),55 reason: Schema.Literals([
56 "missing-tool",
57 "unauthenticated",
58 "rate-limited",
59 "not-found",
60 "failed",
61 ]),
56 detail: Schema.String,62 detail: Schema.String,
57 retryAt: Schema.optional(Schema.Number),63 retryAt: Schema.optional(Schema.Number),
58 cause: Schema.optional(Schema.Defect()),64 cause: Schema.optional(Schema.Defect()),
apps/server/src/pullRequest/PullRequestService.test.ts+27
...@@ -4815,6 +4815,33 @@ it.effect("does not let a still-cached detail overwrite a fresher linked summary...@@ -4815,6 +4815,33 @@ it.effect("does not let a still-cached detail overwrite a fresher linked summary
4815 }),4815 }),
4816);4816);
48174817
4818it.effect("tells the client when the host has no pull request under that number", () =>
4819 Effect.gen(function* () {
4820 const reference = { projectId: "p1" as ProjectId, repository: "acme/web", number: 121 };
4821 const service = yield* makeService({
4822 projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })],
4823 providers: [
4824 fakeProvider("github", {
4825 getChangeRequest: () =>
4826 Effect.fail(
4827 new PullRequestProviderError({
4828 provider: "github",
4829 operation: "getChangeRequest",
4830 reason: "not-found",
4831 detail: "Pull request not found. Check the PR number or URL and try again.",
4832 }),
4833 ),
4834 }),
4835 ],
4836 });
4837
4838 const error = yield* Effect.flip(service.detail(reference));
4839
4840 assert.strictEqual(error._tag, "PullRequestOperationError");
4841 assert.strictEqual(error._tag === "PullRequestOperationError" && error.reason, "not-found");
4842 }),
4843);
4844
4818it.effect("keeps recent detail on a transient refresh failure but not after invalidation", () =>4845it.effect("keeps recent detail on a transient refresh failure but not after invalidation", () =>
4819 Effect.gen(function* () {4846 Effect.gen(function* () {
4820 let failing = false;4847 let failing = false;
apps/server/src/pullRequest/PullRequestService.ts+6-1
...@@ -477,7 +477,12 @@ function toPullRequestError(...@@ -477,7 +477,12 @@ function toPullRequestError(
477 return (error) =>477 return (error) =>
478 isProviderUnusable(error)478 isProviderUnusable(error)
479 ? toUnavailableError(error)479 ? toUnavailableError(error)
480 : new PullRequestOperationError({ operation, detail: error.detail, cause: error });480 : new PullRequestOperationError({
481 operation,
482 detail: error.detail,
483 ...(error.reason === "not-found" ? { reason: "not-found" as const } : {}),
484 cause: error,
485 });
481}486}
482487
483function withRateLimitBackoff(488function withRateLimitBackoff(
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+8-1
...@@ -144,6 +144,7 @@ import {...@@ -144,6 +144,7 @@ import {
144 handoffReviewComments,144 handoffReviewComments,
145 latestPullRequestReviewOutcomes,145 latestPullRequestReviewOutcomes,
146 loadingPullRequestCheckoutCommand,146 loadingPullRequestCheckoutCommand,
147 isPullRequestNotFound,
147 isStackedPullRequestBase,148 isStackedPullRequestBase,
148 pullRequestActionMenuHasGroup,149 pullRequestActionMenuHasGroup,
149 pullRequestActionNeedsHostRefresh,150 pullRequestActionNeedsHostRefresh,
...@@ -2719,7 +2720,13 @@ export function PullRequestDetailPanel({...@@ -2719,7 +2720,13 @@ export function PullRequestDetailPanel({
2719 >2720 >
2720 {detailQuery.error && !detail ? (2721 {detailQuery.error && !detail ? (
2721 <PullRequestsUnavailableState2722 <PullRequestsUnavailableState
2722 error={detailQuery.error}2723 {...(isPullRequestNotFound(detailQuery.failure)
2724 ? {
2725 title: `Pull request #${reference.number} not found`,
2726 error:
2727 "It may be an issue rather than a pull request, or this account can't see it.",
2728 }
2729 : { error: detailQuery.error })}
2723 refreshing={detailQuery.isPending}2730 refreshing={detailQuery.isPending}
2724 onRetry={refreshDetail}2731 onRetry={refreshDetail}
2725 {...(unavailableGitHubUrl ? { gitHubUrl: unavailableGitHubUrl } : {})}2732 {...(unavailableGitHubUrl ? { gitHubUrl: unavailableGitHubUrl } : {})}
apps/web/src/components/pullRequest/pullRequestDetail.logic.ts+7
...@@ -2,6 +2,7 @@ import * as Schema from "effect/Schema";...@@ -2,6 +2,7 @@ import * as Schema from "effect/Schema";
22
3import {3import {
4 PullRequestDetail,4 PullRequestDetail,
5 PullRequestOperationError,
5 pullRequestHostOf,6 pullRequestHostOf,
6 type PullRequestAction,7 type PullRequestAction,
7 type PullRequestActor,8 type PullRequestActor,
...@@ -1161,6 +1162,12 @@ export function buildAddSelectionToAgentHandoff(input: {...@@ -1161,6 +1162,12 @@ export function buildAddSelectionToAgentHandoff(input: {
1161 };1162 };
1162}1163}
11631164
1165const isPullRequestOperationError = Schema.is(PullRequestOperationError);
1166
1167export function isPullRequestNotFound(failure: unknown): boolean {
1168 return isPullRequestOperationError(failure) && failure.reason === "not-found";
1169}
1170
1164/**1171/**
1165 * The internal wrapper every failed operation arrives in: which operation ran, and which tool1172 * The internal wrapper every failed operation arrives in: which operation ran, and which tool
1166 * said no. A reader has no use for either.1173 * said no. A reader has no use for either.
apps/web/src/state/query.ts+5-2
...@@ -7,10 +7,11 @@ const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(fals...@@ -7,10 +7,11 @@ const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial<never, never>(fals
7 Atom.withLabel("web-environment-query:empty"),7 Atom.withLabel("web-environment-query:empty"),
8);8);
99
10export interface EnvironmentQueryView<A> {10export interface EnvironmentQueryView<A, E = unknown> {
11 readonly data: A | null;11 readonly data: A | null;
12 readonly dataUpdatedAt: number;12 readonly dataUpdatedAt: number;
13 readonly error: string | null;13 readonly error: string | null;
14 readonly failure: E | null;
14 readonly isPending: boolean;15 readonly isPending: boolean;
15 readonly isSuccess: boolean;16 readonly isSuccess: boolean;
16 readonly refresh: () => void;17 readonly refresh: () => void;
...@@ -25,7 +26,7 @@ export function formatEnvironmentQueryError(cause: Cause.Cause<unknown>): string...@@ -25,7 +26,7 @@ export function formatEnvironmentQueryError(cause: Cause.Cause<unknown>): string
2526
26export function useEnvironmentQuery<A, E>(27export function useEnvironmentQuery<A, E>(
27 atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,28 atom: Atom.Atom<AsyncResult.AsyncResult<A, E>> | null,
28): EnvironmentQueryView<A> {29): EnvironmentQueryView<A, E> {
29 const selectedAtom = atom ?? EMPTY_ASYNC_RESULT_ATOM;30 const selectedAtom = atom ?? EMPTY_ASYNC_RESULT_ATOM;
30 const result = useAtomValue(selectedAtom);31 const result = useAtomValue(selectedAtom);
31 const refresh = useAtomRefresh(selectedAtom);32 const refresh = useAtomRefresh(selectedAtom);
...@@ -38,6 +39,8 @@ export function useEnvironmentQuery<A, E>(...@@ -38,6 +39,8 @@ export function useEnvironmentQuery<A, E>(
38 ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? 0)39 ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? 0)
39 : 0,40 : 0,
40 error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null,41 error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null,
42 failure:
43 result._tag === "Failure" ? Option.getOrNull(Cause.findErrorOption(result.cause)) : null,
41 isPending: atom !== null && result.waiting,44 isPending: atom !== null && result.waiting,
42 isSuccess: result._tag === "Success",45 isSuccess: result._tag === "Success",
43 refresh,46 refresh,
packages/contracts/src/pullRequest.ts+1
...@@ -1407,6 +1407,7 @@ export class PullRequestOperationError extends Schema.TaggedError<PullRequestOpe...@@ -1407,6 +1407,7 @@ export class PullRequestOperationError extends Schema.TaggedError<PullRequestOpe
1407 {1407 {
1408 operation: Schema.String,1408 operation: Schema.String,
1409 detail: TrimmedNonEmptyString,1409 detail: TrimmedNonEmptyString,
1410 reason: Schema.optional(Schema.Literal("not-found")),
1410 cause: Schema.optional(Schema.Defect()),1411 cause: Schema.optional(Schema.Defect()),
1411 },1412 },
1412 { httpApiStatus: 502 },1413 { httpApiStatus: 502 },