From 41ed35e1f5adde81f09bb4bc486a89cf29ed9960 Mon Sep 17 00:00:00 2001 From: Cameron Date: Sun, 25 Oct 2026 18:09:54 +0000 Subject: [PATCH 1/4] feat(pull-requests): show checks on the merge commit --- .github/fork-features.yml | 37 +++ .../pull-requests/PullRequestRouteScreen.tsx | 4 + .../pull-requests/PullRequestStatusRows.tsx | 72 ++++- .../usePullRequestMergeCommitRefresh.test.tsx | 139 +++++++++ .../usePullRequestMergeCommitRefresh.ts | 40 +++ .../pullRequest/PullRequestService.test.ts | 60 ++++ .../src/pullRequest/PullRequestService.ts | 3 + .../src/telemetry/ClientFeatureUsage.test.ts | 51 ++++ .../pullRequest/PullRequestChecksTab.tsx | 4 +- .../pullRequest/PullRequestDetailPanel.tsx | 102 +++++-- ...lRequestMergeCommitChecks.browser.test.tsx | 93 ++++++ .../PullRequestMergeCommitChecks.tsx | 147 ++++++++++ .../pullRequest/PullRequestSummaryTab.tsx | 11 + .../pullRequest/PullRequestTimelineTab.tsx | 26 +- .../pullRequestDetail.logic.test.ts | 17 ++ .../pullRequest/pullRequestDetail.logic.ts | 11 +- docs/operations/feature-usage.md | 2 + docs/user/source-control.md | 6 + docs/user/styal-differences.md | 5 + docs/user/telemetry.md | 2 + packages/client-runtime/package.json | 4 + .../src/pullRequestMergeCommit.test.ts | 74 +++++ .../src/pullRequestMergeCommit.ts | 49 ++++ packages/contracts/src/forkClientUsage.ts | 5 + packages/contracts/src/pullRequest.test.ts | 18 ++ packages/contracts/src/pullRequest.ts | 11 + .../src/server/PullRequestProvider.ts | 2 + .../src/server/GitHubPullRequestApi.test.ts | 272 ++++++++++++++++++ .../src/server/GitHubPullRequestApi.ts | 72 +++++ .../src/server/gitHubPullRequestJson.ts | 20 +- .../src/server/pullRequestReads.graphql | 39 ++- .../src/server/pullRequestReads.graphql.ts | 191 +++++++++--- 32 files changed, 1507 insertions(+), 82 deletions(-) create mode 100644 apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.test.tsx create mode 100644 apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.ts create mode 100644 apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx create mode 100644 apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx create mode 100644 packages/client-runtime/src/pullRequestMergeCommit.test.ts create mode 100644 packages/client-runtime/src/pullRequestMergeCommit.ts diff --git a/.github/fork-features.yml b/.github/fork-features.yml index 48a5d6855e8e..3f39357b2037 100644 --- a/.github/fork-features.yml +++ b/.github/fork-features.yml @@ -4013,6 +4013,43 @@ features: - https://github.com/pingdotgg/t3code/pull/9496 retire_when: Upstream's automatic pull request refresh reads fresh host state rather than the held detail value, without unnecessary activity reads or invalidating cached diffs. + - id: pull-request-merge-commit-checks + title: Inspect checks on a merged GitHub pull request's resulting commit + status: maintained + prs: [] + invariants: + - A merged GitHub pull request reads the resulting SHA reported after merging, including squash and rebase, without substituting a base tip or temporary test merge. + - Merge commit checks, commit links, and check details remain distinct from source-branch checks on web, desktop, and mobile; a merged status does not imply passing checks. + - Pending checks, no reported checks, unavailable commits, and unreadable or incomplete checks remain distinct; visible views refresh through merging and later check updates. + implementation_paths: + - apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx + - apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.ts + - apps/server/src/pullRequest/PullRequestService.ts + - apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx + - apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx + - packages/client-runtime/src/pullRequestMergeCommit.ts + - packages/contracts/src/pullRequest.ts + - packages/source-control-github/src/server/GitHubPullRequestApi.ts + - packages/source-control-github/src/server/pullRequestReads.graphql + upstream_paths: + - apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx + - apps/server/src/pullRequest/PullRequestService.ts + - apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx + - packages/contracts/src/pullRequest.ts + - packages/source-control-github/src/server/GitHubPullRequestApi.ts + - packages/source-control-github/src/server/pullRequestReads.graphql + tests: + - apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.test.tsx + - apps/server/src/pullRequest/PullRequestService.test.ts + - apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx + - packages/client-runtime/src/pullRequestMergeCommit.test.ts + - packages/contracts/src/pullRequest.test.ts + - packages/source-control-github/src/server/GitHubPullRequestApi.test.ts + upstream: + status: unassessed + tracking: [] + retire_when: Upstream separately displays and refreshes the actual resulting commit's checks for merged GitHub pull requests on web, desktop, and mobile, preserving unavailable states. + - id: pull-request-merge-controls-during-refresh title: Keep pull request merge controls visible while the stack lookup refreshes status: maintained diff --git a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx index 10d8c17584c7..123a62e6db1a 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx @@ -34,6 +34,7 @@ import { usePullRequestReviewDraft } from "./usePullRequestReviewDraft"; import { usePullRequestFilesViewed } from "./usePullRequestFilesViewed"; import { usePullRequestPermissions } from "./usePullRequestPermissions"; import { usePullRequestAction } from "./usePullRequestAction"; +import { usePullRequestMergeCommitRefresh } from "./usePullRequestMergeCommitRefresh"; import { annotatePullRequestDiff, reviewPosition } from "./pullRequestDiffComments"; import { usePullRequestFileFilter } from "./usePullRequestFileFilter"; import { isPullRequestTestFile } from "./pullRequestFileFilters"; @@ -113,6 +114,7 @@ import { PullRequestMarkdownRepository } from "./PullRequestMarkdown"; import { PullRequestStatusMenu, PullRequestChecksRow, + PullRequestMergeCommitRow, PullRequestBranchRow, PullRequestReviewersRow, } from "./PullRequestStatusRows"; @@ -256,6 +258,7 @@ function PullRequestOverview(props: { + @@ -348,6 +351,7 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps refreshDetail()); const pr = result.data; + usePullRequestMergeCommitRefresh(params, pr?.provider === "github" && pr.state !== "closed"); const summary = params.initialSummary; const identity = pr ?? summary; const permissions = usePullRequestPermissions(params, pr); diff --git a/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx b/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx index 28034207cfaa..80b3652c0277 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx @@ -1,4 +1,8 @@ import { usePullRequestConnected } from "./usePullRequestConnected"; +import { + mergedPullRequestCommit, + mergeCommitChecksSummary, +} from "@t3tools/client-runtime/pull-request-merge-commit"; import { PullRequestMergeIntentIcon } from "./PullRequestMergeIntentIcon"; import { useAtomValue } from "@effect/atom-react"; import { useNavigation } from "@react-navigation/native"; @@ -21,6 +25,7 @@ import { checkStatus, pullRequestBranchSummary, pullRequestChecksSummary, + pullRequestChecksStatus, pullRequestReviewSummary, pullRequestStatus, } from "./pullRequestStatus"; @@ -110,7 +115,12 @@ export function PullRequestChecksRow(props: RowProps) { return ( (); + const status = summary.state === null ? null : pullRequestChecksStatus(summary.state); + return ( + + setExpanded(!expanded) : undefined} + /> + {expanded ? ( + + {commit ? ( + void Linking.openURL(commit.url)} + /> + ) : null} + {commit?.checks?.map((check) => { + const status = checkStatus(check); + const url = check.url; + const keyBase = `${check.name}:${url}:${check.status}:${check.description}`; + const occurrence = keyOccurrences.get(keyBase) ?? 0; + keyOccurrences.set(keyBase, occurrence + 1); + return ( + void Linking.openURL(url) : undefined} + /> + ); + })} + + ) : null} + + ); +} + export function PullRequestBranchRow(props: RowProps) { const action = usePullRequestAction(props.params, props.pr); const agent = usePullRequestAgentActions(props.params, props.pr); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.test.tsx b/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.test.tsx new file mode 100644 index 000000000000..12c0a4f736ff --- /dev/null +++ b/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.test.tsx @@ -0,0 +1,139 @@ +// @vitest-environment jsdom +import { act, useCallback, useEffect, useSyncExternalStore } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; + +const harness = vi.hoisted(() => { + const listeners = new Set<() => void>(); + const appListeners = new Set<(state: string) => void>(); + const state = { focused: true, app: "active" }; + return { + state, + listeners, + appListeners, + invalidate: vi.fn(), + refresh: vi.fn(), + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + focus: (focused: boolean) => { + state.focused = focused; + for (const listener of listeners) listener(); + }, + app: (app: string) => { + state.app = app; + for (const listener of appListeners) listener(app); + }, + }; +}); +vi.mock("@react-navigation/native", () => ({ + useFocusEffect: (callback: () => (() => void) | undefined) => { + const focused = useSyncExternalStore(harness.subscribe, () => harness.state.focused); + useEffect(useCallback(() => (focused ? callback() : undefined), [focused, callback])); + }, +})); +vi.mock("react-native", () => ({ + AppState: { + get currentState() { + return harness.state.app; + }, + addEventListener: (_: string, listener: (state: string) => void) => { + harness.appListeners.add(listener); + return { + remove: () => { + harness.appListeners.delete(listener); + }, + }; + }, + }, +})); +vi.mock("@effect/atom-react", () => ({ useAtomRefresh: () => harness.refresh })); +vi.mock("../../state/pull-requests", () => ({ + composerPullRequests: { detail: () => null, invalidate: {} }, +})); +vi.mock("../../state/use-atom-command", () => ({ useAtomCommand: () => harness.invalidate })); +vi.mock("./usePullRequestDiff", () => ({ + pullRequestTarget: (params: Record) => { + const { environmentId, ...input } = params; + return { environmentId, input }; + }, +})); + +import { usePullRequestMergeCommitRefresh } from "./usePullRequestMergeCommitRefresh"; + +const params = { + environmentId: "environment", + projectId: "project", + repository: "acme/web", + number: 7, +}; +function Probe({ enabled }: { enabled: boolean }) { + usePullRequestMergeCommitRefresh(params, enabled); + return null; +} +let root: Root; +let container: HTMLDivElement; +beforeEach(() => { + vi.useFakeTimers(); + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.clearAllMocks(); + harness.state.focused = true; + harness.state.app = "active"; + harness.invalidate.mockResolvedValue({ _tag: "Success" }); + container = document.createElement("div"); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +it("reads fresh host detail while focused, including merged results, and stops when hidden", async () => { + await act(async () => root.render()); + await act(async () => vi.advanceTimersByTime(60_000)); + expect(harness.invalidate).toHaveBeenCalledExactlyOnceWith({ + environmentId: "environment", + input: { + reference: { projectId: "project", repository: "acme/web", number: 7 }, + scope: "detail", + }, + }); + expect(harness.refresh).toHaveBeenCalledOnce(); + await act(async () => harness.app("background")); + await act(async () => vi.advanceTimersByTime(120_000)); + expect(harness.refresh).toHaveBeenCalledOnce(); + await act(async () => harness.app("active")); + expect(harness.refresh).toHaveBeenCalledTimes(2); + await act(async () => harness.focus(false)); + await act(async () => vi.advanceTimersByTime(120_000)); + expect(harness.refresh).toHaveBeenCalledTimes(2); + expect(harness.appListeners.size).toBe(0); + await act(async () => harness.focus(true)); + await act(async () => vi.advanceTimersByTime(60_000)); + expect(harness.refresh).toHaveBeenCalledTimes(3); + await act(async () => root.render()); + await act(async () => vi.advanceTimersByTime(120_000)); + expect(harness.refresh).toHaveBeenCalledTimes(3); +}); + +it("does not overlap host requests or refresh a route left during an in-flight read", async () => { + let resolve: () => void = () => {}; + harness.invalidate.mockImplementation( + () => + new Promise((done) => { + resolve = done; + }), + ); + await act(async () => root.render()); + await act(async () => vi.advanceTimersByTime(180_000)); + expect(harness.invalidate).toHaveBeenCalledOnce(); + expect(harness.refresh).not.toHaveBeenCalled(); + await act(async () => harness.focus(false)); + await act(async () => resolve()); + expect(harness.refresh).not.toHaveBeenCalled(); +}); diff --git a/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.ts b/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.ts new file mode 100644 index 000000000000..11988aa0d651 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/usePullRequestMergeCommitRefresh.ts @@ -0,0 +1,40 @@ +import { useFocusEffect } from "@react-navigation/native"; +import { useCallback } from "react"; +import { AppState } from "react-native"; +import { useAtomRefresh } from "@effect/atom-react"; + +import { composerPullRequests } from "../../state/pull-requests"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; + +export function usePullRequestMergeCommitRefresh(params: PullRequestParams, enabled: boolean) { + const invalidate = useAtomCommand(composerPullRequests.invalidate, { reportFailure: false }); + const refresh = useAtomRefresh(composerPullRequests.detail(pullRequestTarget(params))); + useFocusEffect( + useCallback(() => { + if (!enabled) return; + let mounted = true; + let pending = false; + const read = async () => { + if (!mounted || pending || AppState.currentState !== "active") return; + pending = true; + const target = pullRequestTarget(params); + await invalidate({ + environmentId: target.environmentId, + input: { reference: target.input, scope: "detail" }, + }); + if (mounted) refresh(); + pending = false; + }; + const timer = setInterval(() => void read(), 60_000); + const subscription = AppState.addEventListener("change", (state) => { + if (state === "active") void read(); + }); + return () => { + mounted = false; + clearInterval(timer); + subscription.remove(); + }; + }, [enabled, invalidate, params, refresh]), + ); +} diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 5d5637b5abeb..76dc8c1bf460 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -178,6 +178,66 @@ it.effect("refreshes deployment status and environment links with PR detail", () }), ); +it.effect( + "refreshes a merged PR's resulting commit checks independently of unchanged passing PR checks", + () => + Effect.gen(function* () { + let phase: "open" | "no-checks" | "pending" | "failure" | "unavailable" = "open"; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" }), + ], + providers: [ + fakeProvider(SourceControlProviderKind.make("github"), { + getChangeRequest: () => + Effect.succeed({ + ...changeRequestDetail(1), + state: phase === "open" ? "open" : "merged", + checks: [{ name: "Tests", status: "success", description: null, url: null }], + ...(phase === "open" + ? {} + : { + mergeCommit: + phase === "unavailable" + ? null + : { + oid: "b".repeat(40), + url: "https://github.com/acme/web/commit/" + "b".repeat(40), + checks: + phase === "no-checks" + ? [] + : [ + { + name: "Deploy", + status: phase, + description: null, + url: null, + }, + ], + }, + }), + }), + }), + ], + }); + const reference = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + assert.strictEqual((yield* service.detail(reference)).mergeCommit, undefined); + for (const next of ["no-checks", "pending", "failure", "unavailable"] as const) { + phase = next; + yield* service.invalidate({ reference, scope: "detail" }); + const detail = yield* service.detail(reference); + assert.strictEqual(detail.state, "merged"); + assert.strictEqual(detail.checks[0]?.status, "success"); + if (next === "unavailable") assert.strictEqual(detail.mergeCommit, null); + else + assert.deepStrictEqual( + detail.mergeCommit?.checks?.map((check) => check.status), + next === "no-checks" ? [] : [next], + ); + } + }), +); + it.effect("caches narrow previews and invalidates them after refresh or mutation", () => Effect.gen(function* () { let reads = 0; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 369dfd5e4cca..f4d2963dd90f 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1637,6 +1637,9 @@ export const make = Effect.gen(function* () { reviewers: changeRequest.reviewers, labels: changeRequest.labels, checks: changeRequest.checks, + ...(changeRequest.mergeCommit === undefined + ? {} + : { mergeCommit: changeRequest.mergeCommit }), ...(changeRequest.deployments === undefined ? {} : { deployments: changeRequest.deployments }), diff --git a/apps/server/src/telemetry/ClientFeatureUsage.test.ts b/apps/server/src/telemetry/ClientFeatureUsage.test.ts index ed927170159f..d6d56d2ce9b8 100644 --- a/apps/server/src/telemetry/ClientFeatureUsage.test.ts +++ b/apps/server/src/telemetry/ClientFeatureUsage.test.ts @@ -10,6 +10,57 @@ import * as ClientFeatureUsage from "./ClientFeatureUsage.ts"; const decode = Schema.decodeUnknownEffect(ForkClientUsageInput); const isUsage = Schema.is(ForkClientUsageInput); +it.effect( + "records merge commit links once with an outcome and excludes commit and check content", + () => + Effect.gen(function* () { + const events: unknown[] = []; + const report = yield* ClientFeatureUsage.reporter({ surface: "web" }).pipe( + Effect.provideService(AnalyticsService.AnalyticsService, { + record: (event, properties) => + Effect.sync(() => { + events.push({ event, properties }); + }), + flush: Effect.void, + }), + ); + for (const action of ["open_commit", "open_check"] as const) { + for (const outcome of ["success", "failure"] as const) { + yield* report( + yield* decode({ + feature: "pull-request-merge-commit-checks", + action, + outcome, + oid: "b".repeat(40), + url: "https://example.com/check", + name: "Deploy", + number: 7, + }), + ); + expect(events.at(-1)).toEqual({ + event: "feature.used", + properties: { + feature: "pull-request-merge-commit-checks", + action, + outcome, + source: "user", + surface: "web", + forkClientUsageVersion: FORK_CLIENT_USAGE_VERSION, + }, + }); + } + } + expect(events).toHaveLength(4); + expect( + isUsage({ + feature: "pull-request-merge-commit-checks", + action: "rerun", + outcome: "success", + }), + ).toBe(false); + }), +); + it.effect( "records deployment interactions with bounded outcomes and no PR or environment content", () => diff --git a/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx b/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx index b547d424493d..7e10adea1e03 100644 --- a/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx @@ -208,6 +208,7 @@ export function PullRequestChecksTab({ pendingFinding, fixCheckLabel = "Fix", onFixFinding, + embedded = false, }: { checks: ReadonlyArray; /** Newer status contradicts `checks`; the runs are hidden until the detail is refreshed. */ @@ -229,6 +230,7 @@ export function PullRequestChecksTab({ pendingFinding?: string | null; fixCheckLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; + embedded?: boolean; }) { const state = pullRequestChecksState(checks); // Null for a set nobody can call passed or failed — every run skipped, say. The list still @@ -291,7 +293,7 @@ export function PullRequestChecksTab({ } return ( -
+
{mergePresentation || rollup || requiredBranchUpdate || waitingRuns.length > 0 ? ( // The verdict rides the top of the scroll box the way the summary's section headings do, // so a long list of runs never scrolls its own answer out of sight. diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 1711e49adeda..945fefe4c2ed 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -92,6 +92,11 @@ import { useEnvironments } from "~/state/environments"; import { useEnvironmentQuery } from "~/state/query"; import { readEnvironmentScope, useEnvironmentScope } from "~/state/session"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; +import { mergedPullRequestCommit } from "@t3tools/client-runtime/pull-request-merge-commit"; +import { + PullRequestMergeCommitChecks, + PullRequestMergeCommitStatus, +} from "./PullRequestMergeCommitChecks"; import { pullRequestEnvironment, pullRequestListEntryToSummary, @@ -914,6 +919,7 @@ function PullRequestDetailPanelBody({ // different pull request every time it is opened. useLiveRefresh(() => void refreshDetailFromHost("detail"), { key: `pull-request:${environmentId}:${pullRequestKey}`, + ...(detail?.provider === "github" && detail.state !== "closed" ? { intervalMs: 60_000 } : {}), }); const [isInvalidating, setIsInvalidating] = useState(false); const refreshing = isInvalidating || detailQuery.isPending; @@ -1595,14 +1601,16 @@ function PullRequestDetailPanelBody({ (entry) => entry.outcome === "approved" && !entry.stale, ).length : 0; - const mergeVerdict = detail - ? pullRequestMergeVerdict({ - checks: detail.checks, - isDraft: detail.isDraft, - mergeReadiness: detail.mergeReadiness, - compact: true, - }) - : null; + const mergeCommit = detail === null ? undefined : mergedPullRequestCommit(detail); + const mergeVerdict = + detail && detail.state !== "merged" + ? pullRequestMergeVerdict({ + checks: detail.checks, + isDraft: detail.isDraft, + mergeReadiness: detail.mergeReadiness, + compact: true, + }) + : null; const checksMark = detail && checksState !== null ? ( {detail.title} - {mergeVerdict ? ( + {mergeCommit !== undefined ? ( + setTab("checks")} + /> + ) : mergeVerdict ? ( {mergeVerdict.policy === null ? ( checksMark @@ -2777,13 +2790,20 @@ function PullRequestDetailPanelBody({ {tab === "summary" ? ( {approveWorkflowsButton} - setTab("checks")} - /> + {mergeCommit !== undefined ? ( + setTab("checks")} + /> + ) : ( + setTab("checks")} + /> + )} ) : tab === "timeline" ? (
@@ -2927,20 +2947,42 @@ function PullRequestDetailPanelBody({ ) : null} {mountedTabs.has("checks") ? (
- void perform("update-branch", undefined, method)} - pendingFinding={handoff} - fixCheckLabel={handoffLabels.fixCheck} - {...(canFixFindings ? { onFixFinding: startFixFinding } : {})} - /> +
+ {mergeCommit !== undefined ? ( +
+ +
+ ) : null} + {detail.state === "merged" ? ( +

+ Pull request + {detail.headSha ? ( + + {detail.headSha.slice(0, 7)} + + ) : null} +

+ ) : null} + void perform("update-branch", undefined, method)} + pendingFinding={handoff} + fixCheckLabel={handoffLabels.fixCheck} + {...(canFixFindings ? { onFixFinding: startFixFinding } : {})} + /> +
) : null} {mountedTabs.has("guide") ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx new file mode 100644 index 000000000000..a37a98cc6bad --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx @@ -0,0 +1,93 @@ +import { EnvironmentId, type PullRequestMergeCommit } from "@t3tools/contracts"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; + +const { openLink, report } = vi.hoisted(() => ({ openLink: vi.fn(), report: vi.fn() })); +vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => openLink })); +vi.mock("~/forkClientUsage", () => ({ reportForkClientUsage: report })); + +import { PullRequestMergeCommitChecks } from "./PullRequestMergeCommitChecks"; + +const environmentId = EnvironmentId.make("merge-checks-test"); +const commit: PullRequestMergeCommit = { + oid: "b".repeat(40), + url: `https://github.com/acme/web/commit/${"b".repeat(40)}`, + checks: [ + { name: "Deploy", status: "pending", description: null, url: "https://example.com/runs/1" }, + ], +}; +let container: HTMLDivElement; +let root: Root; +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.clearAllMocks(); + openLink.mockResolvedValue(undefined); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.unstubAllGlobals(); +}); + +const render = (value: PullRequestMergeCommit | null) => + act(async () => + root.render(), + ); + +it("opens the resulting commit and its own check details, and updates while expanded", async () => { + await render(commit); + expect(container.textContent).toContain("1 check pending"); + expect(container.textContent).not.toContain("Deploy"); + await act(async () => userEvent.click(container.querySelector("a")!)); + expect(openLink).toHaveBeenLastCalledWith(commit.url); + await act(async () => userEvent.click(container.querySelector("button")!)); + expect(container.textContent).toContain("Deploy"); + const failed: PullRequestMergeCommit = { + ...commit, + checks: [{ ...commit.checks![0]!, status: "failure", description: "Deployment failed" }], + }; + await render(failed); + expect(container.textContent).toContain("1 check failed"); + expect(container.textContent).toContain("Deployment failed"); + expect(container.textContent).not.toContain("pending"); + await act(async () => userEvent.click(container.querySelectorAll("a")[1]!)); + expect(openLink).toHaveBeenLastCalledWith("https://example.com/runs/1"); + expect(report.mock.calls).toEqual([ + [ + environmentId, + { feature: "pull-request-merge-commit-checks", action: "open_commit", outcome: "success" }, + ], + [ + environmentId, + { feature: "pull-request-merge-commit-checks", action: "open_check", outcome: "success" }, + ], + ]); + openLink.mockRejectedValueOnce(new Error("Unavailable")); + await act(async () => userEvent.click(container.querySelectorAll("a")[1]!)); + expect(report).toHaveBeenLastCalledWith(environmentId, { + feature: "pull-request-merge-commit-checks", + action: "open_check", + outcome: "failure", + }); +}); + +it("keeps commit links usable when checks are unavailable and updates when checks arrive", async () => { + await render(null); + expect(container.textContent).toContain("Merge commit unavailable"); + expect(container.querySelector("a")).toBeNull(); + await render({ ...commit, checks: null }); + expect(container.textContent).toContain("Checks unavailable"); + await act(async () => userEvent.click(container.querySelector("a")!)); + expect(openLink).toHaveBeenLastCalledWith(commit.url); + await render({ ...commit, checks: [] }); + expect(container.textContent).toContain("No checks reported"); + await render(commit); + await act(async () => userEvent.click(container.querySelector("button")!)); + expect(container.textContent).toContain("Deploy"); + expect(container.textContent).toContain("Running"); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx new file mode 100644 index 000000000000..82bfec1b7f69 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx @@ -0,0 +1,147 @@ +import { mergeCommitChecksSummary } from "@t3tools/client-runtime/pull-request-merge-commit"; +import type { EnvironmentId, PullRequestMergeCommit } from "@t3tools/contracts"; +import { CircleHelpIcon, CircleDotIcon } from "lucide-react"; +import { useState } from "react"; + +import { useOpenLink } from "~/browser/useOpenLink"; +import { reportForkClientUsage } from "~/forkClientUsage"; +import { cn } from "~/lib/utils"; + +import { Button } from "../ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { + PullRequestCheckStatusIcon, + pullRequestCheckStatusLabel, + pullRequestChecksStatePresentation, +} from "./pullRequestPresentation"; + +export function PullRequestMergeCommitStatus({ + commit, + onSelect, +}: { + commit: PullRequestMergeCommit | null; + onSelect: () => void; +}) { + const summary = mergeCommitChecksSummary(commit); + const presentation = + summary.state === null ? null : pullRequestChecksStatePresentation(summary.state); + const Icon = + presentation?.Icon ?? + (commit?.checks === null || commit === null ? CircleHelpIcon : CircleDotIcon); + return ( + + ); +} + +export function PullRequestMergeCommitChecks({ + commit, + environmentId, + expanded = false, +}: { + commit: PullRequestMergeCommit | null; + environmentId?: EnvironmentId | undefined; + expanded?: boolean; +}) { + const [showChecks, setShowChecks] = useState(expanded); + const openLink = useOpenLink(null); + const summary = mergeCommitChecksSummary(commit); + const keyOccurrences = new Map(); + const open = (url: string, action: "open_commit" | "open_check") => { + void openLink(url).then( + () => + reportForkClientUsage(environmentId, { + feature: "pull-request-merge-commit-checks", + action, + outcome: "success", + }), + () => + reportForkClientUsage(environmentId, { + feature: "pull-request-merge-commit-checks", + action, + outcome: "failure", + }), + ); + }; + return ( +
+
+ {commit === null ? summary.label : "Merge commit"} + {commit ? ( + + { + event.preventDefault(); + open(commit.url, "open_commit"); + }} + className="font-mono text-muted-foreground hover:underline" + /> + } + > + {commit.oid.slice(0, 7)} + + {commit.oid} + + ) : null} + {commit ? ( + {summary.label} + ) : null} + {!expanded && commit?.checks && commit.checks.length > 0 ? ( + + ) : null} +
+ {showChecks && commit?.checks && commit.checks.length > 0 ? ( +
    + {commit.checks.map((check) => { + const keyBase = `${check.name}:${check.url}:${check.status}:${check.description}`; + const occurrence = keyOccurrences.get(keyBase) ?? 0; + keyOccurrences.set(keyBase, occurrence + 1); + return ( +
  • + +
    +

    {check.name}

    + {check.description ? ( +

    {check.description}

    + ) : null} +
    + {pullRequestCheckStatusLabel(check)} + {check.url ? ( + { + event.preventDefault(); + if (check.url) open(check.url, "open_check"); + }} + > + Details + + ) : null} +
    • + ); + })} +
+ ) : null} +
+ ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index ccadd6ff9421..65ca913a4274 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -1,4 +1,5 @@ import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; +import { mergedPullRequestCommit } from "@t3tools/client-runtime/pull-request-merge-commit"; import { useAtomCommand } from "~/state/use-atom-command"; import type { EnvironmentId, @@ -29,6 +30,7 @@ import { RelativeTime } from "~/components/RelativeTime"; import { Button } from "../ui/button"; import { PullRequestEditButton } from "./PullRequestEditButton"; import { PullRequestDeployments } from "./PullRequestDeployments"; +import { PullRequestMergeCommitChecks } from "./PullRequestMergeCommitChecks"; import { pullRequestDeploymentsTitle } from "./pullRequestDeployments.logic"; import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; import { @@ -717,6 +719,15 @@ export function PullRequestSummaryTab({
+ {mergedPullRequestCommit(detail) !== undefined ? ( +
+ +
+ ) : null} + {detail.deployments && detail.deployments.length > 0 ? (
void; +}) { const presentation = event.kind === "opened" ? { @@ -417,7 +423,7 @@ function LifecycleEvent({ event }: { event: PullRequestTimelineEvent }) { : event.kind === "merged" ? { icon: , - label: "Pull request merged", + label: event.title, } : { icon: , @@ -430,7 +436,19 @@ function LifecycleEvent({ event }: { event: PullRequestTimelineEvent }) {
{event.actor ? : null} - {presentation.label} + {event.url ? ( + + ) : ( + {presentation.label} + )}
@@ -600,7 +618,7 @@ export function PullRequestTimelineTab({ /> ); } - return ; + return ; })}
diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 9e0587376a2b..9be64a18d5c0 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -581,6 +581,23 @@ describe("review verdicts", () => { }); describe("pull request timeline", () => { + it("links the merged lifecycle event to the resulting commit even when its checks failed", () => { + const mergeCommit = { + oid: "b".repeat(40), + url: `https://github.com/acme/web/commit/${"b".repeat(40)}`, + checks: [{ name: "Deploy", status: "failure" as const, description: null, url: null }], + }; + const merged = buildPullRequestTimeline({ + ...TIMELINE_SOURCE, + mergedAt: "2026-07-03T00:00:00Z", + baseBranch: "main", + mergeCommit, + }).find((event) => event.kind === "merged"); + expect(merged).toMatchObject({ + title: "merged commit bbbbbbb into main", + url: mergeCommit.url, + }); + }); it("orders creation, commits and comments newest first", () => { expect(buildPullRequestTimeline(TIMELINE_SOURCE).map((event) => event.id)).toEqual([ "c1", diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 3794eb910292..eab5d5ab644f 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -543,8 +543,9 @@ export function visibleBody(body: string): string | null { export function buildPullRequestTimeline( detail: Pick< PullRequestDetailView, - "createdAt" | "author" | "commits" | "comments" | "mergedAt" | "closedAt" - >, + "createdAt" | "author" | "commits" | "comments" | "mergedAt" | "closedAt" | "mergeCommit" + > & + Partial>, ): ReadonlyArray { return [ { @@ -601,10 +602,12 @@ export function buildPullRequestTimeline( id: "merged", at: detail.mergedAt, kind: "merged" as const, - title: "Pull request merged", + title: detail.mergeCommit + ? `merged commit ${detail.mergeCommit.oid.slice(0, 7)}${detail.baseBranch ? ` into ${detail.baseBranch}` : ""}` + : "Pull request merged", body: null, markdown: false, - url: null, + url: detail.mergeCommit?.url ?? null, actor: null, commitAuthors: [], additions: null, diff --git a/docs/operations/feature-usage.md b/docs/operations/feature-usage.md index 76b2454f73fe..e7f28fd1b550 100644 --- a/docs/operations/feature-usage.md +++ b/docs/operations/feature-usage.md @@ -12,6 +12,8 @@ Upstream's `mcp.tool.invoked` records one event per agent MCP tool call with `to Additional measured operations are: +- `pull-request-merge-commit-checks`, `source = user`: `open_commit` and `open_check` record explicit links from the merge commit section on web and desktop with `outcome = success | failure`. Success means the browser accepted the handoff. Reports require client vocabulary version 11; display and refresh do not count. Only the action, outcome, and existing client attribution are sent. Mobile does not report these interactions, so these counts cannot establish whole-feature adoption. + - `pull-request-deployments`, `source = user`: `open_environment`, `open_logs`, and `show_all` record explicit deployment panel actions with `outcome = success | failure`. Link success means the browser accepted the handoff, not that the destination loaded. Display, polling, rerenders and Show less do not emit events. Reports require client vocabulary version 2 and go to the PR's environment without names, URLs, repositories, PR numbers or commit hashes. - `devices`, `source = user`: Device panel open, close, shutdown and device actions from a client that reports its surface. Requests forwarded from another environment and older clients without a surface remain unattributed instead of being counted again as manual use. diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 770f3bb92dd1..76b41dea408e 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -204,6 +204,12 @@ GitHub, GitLab, and Azure DevOps support auto-merge while checks are outstanding after auto-merge is on are merged too, unless the host turns auto-merge off. GitHub also supports approving waiting workflows and opening a revert pull request for a merged change. +For a merged GitHub pull request, **Merge commit** shows the resulting commit and its own checks +in Summary and Checks on web and desktop, and in the mobile overview. Open the commit SHA or a +check's **Details** to inspect it on GitHub. Pull request checks remain separate: their passing +result does not say whether the merge commit passed. Squash and rebase merges show the resulting +commit GitHub reports. Missing commit data or unreadable checks are shown as unavailable. + GitHub holds some workflow runs until someone with write access approves them: runs from fork pull requests, depending on the repository's settings, and runs on a pull request that a workflow opened or updated with `GITHUB_TOKEN`, such as a Changesets or release-please release pull request. diff --git a/docs/user/styal-differences.md b/docs/user/styal-differences.md index 7c0ba8f9bab3..d16eab229780 100644 --- a/docs/user/styal-differences.md +++ b/docs/user/styal-differences.md @@ -161,6 +161,11 @@ works. This overview focuses on desktop and web; the mobile app is a work in pro update methods. [Code review](./source-control.md#review-and-merge). Compared with [T3 Code revision a493946](https://github.com/pingdotgg/t3code/tree/a493946bb42ab16e1d18285dac3e9e4603651330), checked September 23, 2026. +- **Checks on the merge commit.** Merged GitHub pull requests show the resulting commit and its + checks separately from the pull request's checks on web, desktop, and mobile, with links to the + commit and individual checks. [Code review](./source-control.md#review-and-merge). + Compared with [T3 Code revision d0b9e9d](https://github.com/pingdotgg/t3code/tree/d0b9e9d3c72b83bb92670fc1fa3afac5bf60e4e8), + checked October 25, 2026, which reads the last pull request commit's checks. - **Workflows waiting for approval on release pull requests.** T3 Code shows and approves fork workflows waiting for a maintainer. styal also finds runs GitHub holds on pull requests a workflow opened with `GITHUB_TOKEN`, such as Changesets release pull requests, offers diff --git a/docs/user/telemetry.md b/docs/user/telemetry.md index 51dd7b5973c1..800826f1accb 100644 --- a/docs/user/telemetry.md +++ b/docs/user/telemetry.md @@ -73,6 +73,8 @@ Each time the desktop app starts, it records how long its local server and windo Pull request deployment controls record whether someone opens an environment, opens deployment logs, or expands the environment list, and whether the action succeeded. Environment names, URLs, repositories, pull request numbers, and commit hashes are not collected. +On web and desktop, links in a pull request's merge commit section record whether someone opens the commit or a check's details, and whether the browser accepted the link. Commit hashes, check names, URLs, repositories, and pull request numbers are not collected. + Usage events do not include prompts, responses, file contents, authentication tokens, conversation IDs, raw provider events, or child-agent output. Child-agent token use is excluded from the totals. diff --git a/packages/client-runtime/package.json b/packages/client-runtime/package.json index 4c4d43663cf3..674c29fabed4 100644 --- a/packages/client-runtime/package.json +++ b/packages/client-runtime/package.json @@ -470,6 +470,10 @@ "./pull-request-query": { "types": "./src/pullRequestQuery.ts", "default": "./src/pullRequestQuery.ts" + }, + "./pull-request-merge-commit": { + "types": "./src/pullRequestMergeCommit.ts", + "default": "./src/pullRequestMergeCommit.ts" } }, "scripts": { diff --git a/packages/client-runtime/src/pullRequestMergeCommit.test.ts b/packages/client-runtime/src/pullRequestMergeCommit.test.ts new file mode 100644 index 000000000000..41e41131c221 --- /dev/null +++ b/packages/client-runtime/src/pullRequestMergeCommit.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vite-plus/test"; +import { + SourceControlProviderKind, + type PullRequestCheck, + type PullRequestDetail, +} from "@t3tools/contracts"; +import { mergedPullRequestCommit, mergeCommitChecksSummary } from "./pullRequestMergeCommit.ts"; + +const check = (status: PullRequestCheck["status"]): PullRequestCheck => ({ + name: "Deploy", + status, + description: null, + url: null, +}); +const commit = { + oid: "b".repeat(40), + url: `https://github.com/acme/web/commit/${"b".repeat(40)}`, + checks: [check("failure")], +}; +const pr = { + state: "merged", + provider: SourceControlProviderKind.make("github"), + checks: [check("success")], + mergeCommit: commit, +} satisfies Pick; + +describe("merge commit checks", () => { + it("keeps passing PR checks out of the resulting commit's failed rollup", () => { + const selected = mergedPullRequestCommit(pr); + expect(selected).toBe(commit); + expect(mergeCommitChecksSummary(selected!)).toEqual({ + label: "1 check failed", + state: "failing", + }); + }); + it("only exposes a resulting commit after merging, including an unavailable GitHub result", () => { + expect(mergedPullRequestCommit({ ...pr, state: "open" })).toBeUndefined(); + expect(mergedPullRequestCommit({ ...pr, state: "closed" })).toBeUndefined(); + expect(mergedPullRequestCommit({ ...pr, mergeCommit: undefined })).toBeNull(); + expect( + mergedPullRequestCommit({ + ...pr, + provider: SourceControlProviderKind.make("gitlab"), + mergeCommit: undefined, + }), + ).toBeUndefined(); + }); + it("preserves missing commit, unreadable checks, no checks, pending, and terminal outcomes", () => { + expect(mergeCommitChecksSummary(null)).toEqual({ + label: "Merge commit unavailable", + state: null, + }); + expect(mergeCommitChecksSummary({ ...commit, checks: null })).toEqual({ + label: "Checks unavailable", + state: null, + }); + expect(mergeCommitChecksSummary({ ...commit, checks: [] })).toEqual({ + label: "No checks reported", + state: null, + }); + expect( + mergeCommitChecksSummary({ ...commit, checks: [check("success"), check("pending")] }), + ).toEqual({ label: "1 check pending, 1 check passed", state: "pending" }); + expect( + mergeCommitChecksSummary({ ...commit, checks: [check("success"), check("cancelled")] }), + ).toEqual({ label: "1 check cancelled, 1 check passed", state: "failing" }); + expect( + mergeCommitChecksSummary({ ...commit, checks: [check("skipped"), check("neutral")] }), + ).toEqual({ label: "1 check skipped, 1 check neutral", state: null }); + expect( + mergeCommitChecksSummary({ ...commit, checks: [check("success"), check("success")] }), + ).toEqual({ label: "2 checks passed", state: "passing" }); + }); +}); diff --git a/packages/client-runtime/src/pullRequestMergeCommit.ts b/packages/client-runtime/src/pullRequestMergeCommit.ts new file mode 100644 index 000000000000..2b2fb535c78a --- /dev/null +++ b/packages/client-runtime/src/pullRequestMergeCommit.ts @@ -0,0 +1,49 @@ +import type { + PullRequestChecksState, + PullRequestDetail, + PullRequestMergeCommit, +} from "@t3tools/contracts"; + +/** Undefined hides unsupported hosts; null keeps an unavailable GitHub result visible. */ +export function mergedPullRequestCommit( + pr: Pick, +) { + if (pr.state !== "merged") return undefined; + return pr.provider === "github" || pr.mergeCommit !== undefined + ? (pr.mergeCommit ?? null) + : undefined; +} + +export function mergeCommitChecksSummary(commit: PullRequestMergeCommit | null): { + label: string; + state: PullRequestChecksState | null; +} { + if (commit === null) return { label: "Merge commit unavailable", state: null }; + const checks = commit.checks; + if (checks === null) return { label: "Checks unavailable", state: null }; + if (checks.length === 0) return { label: "No checks reported", state: null }; + const counts = { + failed: checks.filter((check) => check.status === "failure").length, + cancelled: checks.filter((check) => check.status === "cancelled").length, + pending: checks.filter((check) => check.status === "pending" || check.status === "expected") + .length, + "action required": checks.filter((check) => check.status === "action-required").length, + passed: checks.filter((check) => check.status === "success").length, + skipped: checks.filter((check) => check.status === "skipped").length, + neutral: checks.filter((check) => check.status === "neutral").length, + }; + return { + label: Object.entries(counts) + .filter(([, count]) => count > 0) + .map(([status, count]) => `${count} ${count === 1 ? "check" : "checks"} ${status}`) + .join(", "), + state: + counts.failed || counts.cancelled + ? "failing" + : counts.pending || counts["action required"] + ? "pending" + : counts.passed + ? "passing" + : null, + }; +} diff --git a/packages/contracts/src/forkClientUsage.ts b/packages/contracts/src/forkClientUsage.ts index ec6f0ec5cfcc..66d4c5c7bb7f 100644 --- a/packages/contracts/src/forkClientUsage.ts +++ b/packages/contracts/src/forkClientUsage.ts @@ -127,6 +127,11 @@ export type WebBuildUpdateUsageAction = typeof WebBuildUpdateUsageAction.Type; /** Closed UI observations only: content, paths and identifiers stay in the client. */ export const ForkClientUsageInput = Schema.Union([ + Schema.Struct({ + feature: Schema.Literal("pull-request-merge-commit-checks"), + action: Schema.Literals(["open_commit", "open_check"]), + outcome: Schema.Literals(["success", "failure"]), + }), Schema.Struct({ feature: Schema.Literal("pull-request-deployments"), action: Schema.Literals(["open_environment", "open_logs", "show_all"]), diff --git a/packages/contracts/src/pullRequest.test.ts b/packages/contracts/src/pullRequest.test.ts index 7ea0807d9045..c0dc6a35b231 100644 --- a/packages/contracts/src/pullRequest.test.ts +++ b/packages/contracts/src/pullRequest.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vite-plus/test"; import { PullRequestActionInput, PullRequestCapabilities, + PullRequestDetail, PullRequestInvalidateInput, PullRequestFilesViewedResult, PullRequestListInput, @@ -23,6 +24,23 @@ const decodeAction = Schema.decodeUnknownSync(PullRequestActionInput); const decodeSetFilesViewed = Schema.decodeUnknownSync(PullRequestSetFilesViewedInput); const decodeFilesViewed = Schema.decodeUnknownSync(PullRequestFilesViewedResult); +it("preserves unsupported, unavailable and empty merge checks through the RPC codec", () => { + const schema = Schema.Struct({ mergeCommit: PullRequestDetail.fields.mergeCommit }); + const codec = Schema.toCodecJson(schema); + const commit = { + oid: "a".repeat(40), + url: "https://github.com/acme/web/commit/" + "a".repeat(40), + }; + for (const value of [ + {}, + { mergeCommit: null }, + { mergeCommit: { ...commit, checks: null } }, + { mergeCommit: { ...commit, checks: [] } }, + ]) { + expect(Schema.decodeSync(codec)(Schema.encodeUnknownSync(codec)(value))).toEqual(value); + } +}); + const LIST_RESULT: PullRequestListResult = { viewers: { "github.com": "bilal", "gitlab.com": "bilal.hassan" }, providers: [ diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index ed7393705c42..21702cd52907 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -128,6 +128,15 @@ export const PullRequestCheck = Schema.Struct({ }); export type PullRequestCheck = typeof PullRequestCheck.Type; +export const PullRequestMergeCommit = Schema.Struct({ + /** The resulting commit reported by the host after merging, including squash and rebase. */ + oid: TrimmedNonEmptyString, + url: TrimmedNonEmptyString, + /** Null when checks could not be read completely; an empty array means no checks reported. */ + checks: Schema.NullOr(Schema.Array(PullRequestCheck)), +}); +export type PullRequestMergeCommit = typeof PullRequestMergeCommit.Type; + export const PullRequestReactionContent = Schema.Literals([ "thumbs-up", "thumbs-down", @@ -636,6 +645,8 @@ export const PullRequestDetail = Schema.Struct({ reviewers: Schema.Array(PullRequestActor), labels: Schema.Array(PullRequestLabel), checks: Schema.Array(PullRequestCheck), + /** Absent on unsupported hosts/older servers; null when the resulting commit is unavailable. */ + mergeCommit: Schema.optional(Schema.NullOr(PullRequestMergeCommit)), /** Latest deployment per environment, where the host reports deployments for this PR. */ deployments: Schema.optional(Schema.Array(PullRequestDeployment)), deploymentsTruncated: Schema.optional(Schema.Boolean), diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 90b06fdeebd9..6a64fc2888c6 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -19,6 +19,7 @@ import type { PullRequestListFilters, PullRequestListState, PullRequestMergeCapabilities, + PullRequestMergeCommit, PullRequestMergeMethod, PullRequestMergeReadiness, PullRequestMergeability, @@ -186,6 +187,7 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { readonly closedAt: string | null; readonly reviewers: ReadonlyArray; readonly checks: ReadonlyArray; + readonly mergeCommit?: PullRequestMergeCommit | null; readonly deployments?: ReadonlyArray; readonly deploymentsTruncated?: boolean; readonly mergeCapabilities: PullRequestMergeCapabilities; diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index a4f25ea47dcd..b81656aea82b 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -767,6 +767,278 @@ it.effect( ); layer("GitHubPullRequestApi.layer", (it) => { + const mergedOid = "b".repeat(40); + const mergeInput = { cwd: "/w", repository: "acme/web", host: "github.com", number: 7 }; + const commitChecksPage = (checks: ReadonlyArray, after: string | null = null) => ({ + data: { + repository: { + object: { + __typename: "Commit", + oid: mergedOid, + statusCheckRollup: { + contexts: { + nodes: checks, + pageInfo: { hasNextPage: after !== null, endCursor: after }, + }, + }, + }, + }, + }, + }); + + it.effect( + "keeps checks on the resulting merge, squash or rebase SHA separate from a deleted source branch", + () => + Effect.gen(function* () { + for (const method of ["merge", "squash", "rebase"]) { + const oid = { merge: "b", squash: "c", rebase: "d" }[method]!.repeat(40); + mockedExecute.mockImplementation((call) => { + if (call.kind === "rest") + return Effect.succeed(output(encodeJson({ merged: true, merge_commit_sha: oid }))); + if (call.operationName === "CommitCheckContexts") + return Effect.succeed( + output( + encodeJson({ + data: { + repository: { + object: { + ...commitChecksPage([ + checkRun({ + name: "Deploy", + conclusion: "FAILURE", + detailsUrl: "https://example.com/runs/merge", + }), + ]).data.repository.object, + oid, + }, + }, + }, + }), + ), + ); + return Effect.succeed( + output( + encodeJson( + coreResponse({ + state: "MERGED", + mergedAt: "2026-07-02T00:00:00Z", + headRef: null, + baseRef: null, + commits: { + nodes: [ + { + commit: { + statusCheckRollup: { + contexts: { + nodes: [checkRun({ name: "Tests", conclusion: "SUCCESS" })], + pageInfo: { hasNextPage: false }, + }, + }, + }, + }, + ], + }, + }), + ), + ), + ); + }); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const detail = yield* cli.getPullRequestDetail(mergeInput); + expect(detail.state).toBe("merged"); + expect(detail.checks.map((check) => check.status)).toEqual(["success"]); + expect(detail.mergeCommit).toEqual({ + oid, + url: `https://github.com/acme/web/commit/${oid}`, + checks: [ + { + name: "Deploy", + status: "failure", + description: null, + url: "https://example.com/runs/merge", + }, + ], + }); + expect(variablesOf("CommitCheckContexts").at(-1)?.["oid"]).toBe(oid); + } + expect( + mockedExecute.mock.calls + .filter(([call]) => call.kind === "rest") + .every( + ([call]) => + call.kind === "rest" && + call.path === "repos/acme/web/pulls/7" && + (call.method === undefined || call.method === "GET"), + ), + ).toBe(true); + }), + ); + + it.effect("reads merge checks after an open PR merges and checks are reported later", () => + Effect.gen(function* () { + let merged = false; + let checks: ReadonlyArray = []; + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson( + call.kind === "rest" + ? { merged, merge_commit_sha: mergedOid } + : call.operationName === "CommitCheckContexts" + ? commitChecksPage(checks) + : coreResponse({ + state: merged ? "MERGED" : "OPEN", + mergedAt: merged ? "2026-07-02T00:00:00Z" : null, + }), + ), + ), + ), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit).toBeUndefined(); + expect(mockedExecute.mock.calls.some(([call]) => call.kind === "rest")).toBe(false); + merged = true; + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit?.checks).toEqual([]); + checks = [checkRun({ name: "Deploy", status: "IN_PROGRESS" })]; + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit?.checks?.[0]?.status).toBe( + "pending", + ); + checks = [checkRun({ name: "Deploy", conclusion: "FAILURE" })]; + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit?.checks?.[0]?.status).toBe( + "failure", + ); + }), + ); + + it.effect( + "does not use a temporary test merge SHA or invent a commit when host data is unavailable", + () => + Effect.gen(function* () { + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + for (const response of [ + { merged: false, merge_commit_sha: mergedOid }, + { merged: true, merge_commit_sha: null }, + {}, + ]) { + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson(call.kind === "rest" ? response : coreResponse({ state: "MERGED" })), + ), + ), + ); + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit).toBeNull(); + } + expect(variablesOf("CommitCheckContexts")).toEqual([]); + }), + ); + + it.effect( + "retains the resulting commit when checks are unreadable rather than claiming no checks", + () => + Effect.gen(function* () { + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson( + call.kind === "rest" + ? { merged: true, merge_commit_sha: mergedOid } + : call.operationName === "CommitCheckContexts" + ? { data: { repository: { object: null } } } + : coreResponse({ state: "MERGED" }), + ), + ), + ), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const detail = yield* cli.getPullRequestDetail(mergeInput); + expect(detail.mergeCommit).toEqual({ + oid: mergedOid, + url: `https://github.com/acme/web/commit/${mergedOid}`, + checks: null, + }); + }), + ); + + it.effect( + "keeps unreadable and incomplete merge checks unavailable, with the commit still linked", + () => + Effect.gen(function* () { + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + for (const scenario of ["rest-error", "checks-error", "incomplete"] as const) { + mockedExecute.mockImplementation((call) => { + if (call.kind === "rest") + return scenario === "rest-error" + ? Effect.fail( + new GitHubRequestPolicy.GitHubApiResponseError({ + host: call.host, + operation: call.operation, + status: 403, + }), + ) + : Effect.succeed(output(encodeJson({ merged: true, merge_commit_sha: mergedOid }))); + if (call.operationName === "CommitCheckContexts") + return scenario === "checks-error" + ? Effect.fail( + new GitHubRequestPolicy.GitHubApiResponseError({ + host: call.host, + operation: call.operation, + status: 403, + }), + ) + : Effect.succeed( + output( + encodeJson( + commitChecksPage( + [checkRun({ name: "Tests", conclusion: "SUCCESS" })], + "repeated-cursor", + ), + ), + ), + ); + return Effect.succeed(output(encodeJson(coreResponse({ state: "MERGED" })))); + }); + const detail = yield* cli.getPullRequestDetail(mergeInput); + expect(detail.state).toBe("merged"); + if (scenario === "rest-error") expect(detail.mergeCommit).toBeNull(); + else + expect(detail.mergeCommit).toEqual({ + oid: mergedOid, + url: `https://github.com/acme/web/commit/${mergedOid}`, + checks: null, + }); + } + }), + ); + + it.effect("includes failures beyond the first page of merge checks", () => + Effect.gen(function* () { + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson( + call.kind === "rest" + ? { merged: true, merge_commit_sha: mergedOid } + : call.operationName === "CommitCheckContexts" + ? call.variables?.["after"] + ? commitChecksPage([checkRun({ name: "Deploy", conclusion: "FAILURE" })]) + : commitChecksPage([checkRun({ name: "Tests", conclusion: "SUCCESS" })], "next") + : coreResponse({ state: "MERGED" }), + ), + ), + ), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + expect( + (yield* cli.getPullRequestDetail(mergeInput)).mergeCommit?.checks?.map( + (check) => check.status, + ), + ).toEqual(["success", "failure"]); + expect( + variablesOf("CommitCheckContexts").map((variables) => variables["after"] ?? null), + ).toEqual([null, "next"]); + }), + ); it.effect("holds background previews once an answer reports the reserve, until it resets", () => Effect.gen(function* () { const now = yield* Clock.currentTimeMillis; diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index 972131d5a129..ea00872051b8 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -30,6 +30,7 @@ import { type PullRequestListFilters, type PullRequestListState, type PullRequestMergeMethod, + type PullRequestMergeCommit, type PullRequestOmittedFileStat, type PullRequestReaction, type PullRequestReactionContent, @@ -77,6 +78,7 @@ import { toLabelCandidates, toPullRequestActivityPage, toPullRequestCheckContextsPage, + toCommitCheckContextsPage, toPullRequestCore, toPullRequestFilesViewedPage, toPullRequestHeads, @@ -1583,11 +1585,81 @@ export const make = Effect.gen(function* () { })), ), ), + Effect.flatMap((core) => + core.state !== "merged" + ? Effect.succeed(core) + : readMergeCommit(input, allowReserve).pipe( + Effect.map((mergeCommit) => ({ ...core, mergeCommit })), + ), + ), ); }), ); }; + const decodeMergeResult = decodeJsonResult( + Schema.Struct({ + merged: Schema.Boolean, + merge_commit_sha: Schema.NullOr(TrimmedNonEmptyString), + }), + ); + const readMergeCommit = Effect.fnUntraced(function* ( + input: Parameters[0], + allowReserve: boolean, + ) { + const { owner, name } = parseRepositorySelector(input.repository); + const target = { ...input, operation: "getPullRequestDetail", allowReserve }; + // REST defines this field for merge, squash and rebase. Before merging it names a temporary + // test commit, so both the core and this response must independently confirm the merge. + const result = yield* api + .rest({ + ...target, + path: `repos/${owner}/${name}/pulls/${input.number}`, + }) + .pipe( + Effect.flatMap((response) => + response.truncated + ? Effect.succeed(null) + : decodeWith(input.cwd, target.operation, decodeMergeResult, response.body), + ), + Effect.orElseSucceed(() => null), + ); + const oid = result?.merged ? result.merge_commit_sha : null; + if (oid == null || !/^(?:[a-f0-9]{40}|[a-f0-9]{64})$/i.test(oid)) return null; + const checks = yield* readPages( + GraphQLClient.pages(client.CommitCheckContexts, { + variables: { owner, name, oid }, + connection: (result) => { + const page = toCommitCheckContextsPage(result); + return page === null + ? null + : { + ...page, + pageInfo: { hasNextPage: page.nextCursor !== null, endCursor: page.nextCursor }, + }; + }, + }), + { + ...target, + maxPages: CHECK_CONTEXT_PAGES, + hasNextPage: (page) => page.nextCursor !== null, + }, + ).pipe( + graphql(target), + Effect.map(({ pages, truncated }) => + truncated || pages.length === 0 || pages.some((page) => page.oid !== oid) + ? null + : pullRequestChecksFromContexts(pages.flatMap((page) => page.contexts)).checks, + ), + Effect.orElseSucceed(() => null), + ); + return { + oid, + url: `https://${input.host}/${owner}/${name}/commit/${oid}`, + checks, + } satisfies PullRequestMergeCommit; + }); + const workflowApprovalLimit = 1_000; const listHeadsByBranch = (input: { diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index bd9095909a79..1dbb5a6a0d4d 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -16,6 +16,7 @@ import type { PullRequestFileViewedState, PullRequestLabel, PullRequestMergeCapabilities, + PullRequestMergeCommit, PullRequestMergeMethod, PullRequestMergeReadiness, PullRequestOmittedFileStat, @@ -58,7 +59,8 @@ import type { Deployment, WorkflowApprovalSuites, ActorAvatars, - CheckContext, + CommitCheckContext, + CommitCheckContexts, LabelCandidates, PullRequestActivity, PullRequestCheckContexts, @@ -540,6 +542,7 @@ export interface GitHubPullRequestDetail extends GitHubPullRequestListItem { readonly mergedAt: string | null; readonly closedAt: string | null; readonly checks: ReadonlyArray; + readonly mergeCommit?: PullRequestMergeCommit | null; readonly deployments?: ReadonlyArray; readonly deploymentsTruncated?: boolean; /** Absent where GitHub did not report its repository-policy-aware verdict. */ @@ -1576,7 +1579,9 @@ export function toPullRequestActivityPage( /** A check context as the rollup reports it; opaque outside this module. */ export type GitHubCheckContext = CheckFields; -function toCheckContexts(nodes: ReadonlyArray): ReadonlyArray { +function toCheckContexts( + nodes: ReadonlyArray, +): ReadonlyArray { return nodes.map((check) => check.__typename === "CheckRun" ? { ...check, workflowName: check.checkSuite.workflowRun?.workflow.name ?? null } @@ -1584,6 +1589,17 @@ function toCheckContexts(nodes: ReadonlyArray): ReadonlyArray/merge. +query CommitCheckContexts($owner: String!, $name: String!, $oid: GitObjectID!, $after: String) { + repository(owner: $owner, name: $name) { + object(oid: $oid) { + ... on Commit { + oid + statusCheckRollup { + contexts(first: 100, after: $after) { + nodes { + ...CommitCheckContext + } + pageInfo { + hasNextPage + endCursor + } + } + } + } + } + } +} + # The detail of a pull request with the viewer's standing on it. Pull refs let the comparison # (`$headRef` is `refs/pull//head`) share this read without first resolving a fork branch. query PullRequestCore( diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql.ts b/packages/source-control-github/src/server/pullRequestReads.graphql.ts index 3e1b9b2a4fd0..40de9efb3df8 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReads.graphql.ts @@ -4,7 +4,7 @@ import * as Schema from "effect/Schema" import * as Shared from "../../schema/github.graphql.ts" /** Represents an individual commit status context */ -export const StatusContext = Schema.Struct({ +export const CommitCheckContextStatusContext = Schema.Struct({ __typename: Schema.Literal("StatusContext"), /** The name of this status context. */ context: Schema.String, @@ -15,11 +15,9 @@ export const StatusContext = Schema.Struct({ /** Identifies the date and time when the object was created. */ createdAt: Shared.DateTime, /** The description for this status context. */ - description: Schema.NullOr(Schema.String), - /** Whether this is required to pass before merging for a specific pull request. */ - isRequired: Schema.optionalKey(Schema.Boolean) + description: Schema.NullOr(Schema.String) }) -export interface StatusContext extends Schema.Struct.Type {} +export interface CommitCheckContextStatusContext extends Schema.Struct.Type {} /** A workflow contains meta information about an Actions workflow file. */ export const Workflow = Schema.Struct({ @@ -43,7 +41,7 @@ export const CheckSuiteCheckSuite = Schema.Struct({ export interface CheckSuiteCheckSuite extends Schema.Struct.Type {} /** A check run. */ -export const CheckRun = Schema.Struct({ +export const CommitCheckContextCheckRun = Schema.Struct({ __typename: Schema.Literal("CheckRun"), /** The name of the check for this check run. */ name: Schema.String, @@ -57,12 +55,10 @@ export const CheckRun = Schema.Struct({ completedAt: Schema.NullOr(Shared.DateTime), /** The URL from which to find full details of the check run on the integrator's site. */ detailsUrl: Schema.NullOr(Shared.URI), - /** Whether this is required to pass before merging for a specific pull request. */ - isRequired: Schema.optionalKey(Schema.Boolean), /** The check suite that this run is a part of. */ checkSuite: CheckSuiteCheckSuite }) -export interface CheckRun extends Schema.Struct.Type {} +export interface CommitCheckContextCheckRun extends Schema.Struct.Type {} /** Types that can be inside a StatusCheckRollup context. */ export const StatusCheckRollupContextOther = Schema.Struct({ @@ -70,14 +66,66 @@ export const StatusCheckRollupContextOther = Schema.Struct({ }) export interface StatusCheckRollupContextOther extends Schema.Struct.Type {} +/** + * `fragment CommitCheckContext on StatusCheckRollupContext` + * + * Types that can be inside a StatusCheckRollup context. + */ +export const CommitCheckContext = Schema.Union([ + CommitCheckContextStatusContext, + CommitCheckContextCheckRun, + StatusCheckRollupContextOther +]) +export type CommitCheckContext = typeof CommitCheckContext.Type + +/** Represents an individual commit status context */ +export const CheckContextStatusContext = Schema.Struct({ + __typename: Schema.Literal("StatusContext"), + /** The name of this status context. */ + context: Schema.String, + /** The state of this status context. */ + state: GraphQL.enumLiterals(Shared.StatusState.literals), + /** The URL for this status context. */ + targetUrl: Schema.NullOr(Shared.URI), + /** Identifies the date and time when the object was created. */ + createdAt: Shared.DateTime, + /** The description for this status context. */ + description: Schema.NullOr(Schema.String), + /** Whether this is required to pass before merging for a specific pull request. */ + isRequired: Schema.optionalKey(Schema.Boolean) +}) +export interface CheckContextStatusContext extends Schema.Struct.Type {} + +/** A check run. */ +export const CheckContextCheckRun = Schema.Struct({ + __typename: Schema.Literal("CheckRun"), + /** The name of the check for this check run. */ + name: Schema.String, + /** The current status of the check run. */ + status: GraphQL.enumLiterals(Shared.CheckStatusState.literals), + /** The conclusion of the check run. */ + conclusion: Schema.NullOr(GraphQL.enumLiterals(Shared.CheckConclusionState.literals)), + /** Identifies the date and time when the check run was started. */ + startedAt: Schema.NullOr(Shared.DateTime), + /** Identifies the date and time when the check run was completed. */ + completedAt: Schema.NullOr(Shared.DateTime), + /** The URL from which to find full details of the check run on the integrator's site. */ + detailsUrl: Schema.NullOr(Shared.URI), + /** The check suite that this run is a part of. */ + checkSuite: CheckSuiteCheckSuite, + /** Whether this is required to pass before merging for a specific pull request. */ + isRequired: Schema.optionalKey(Schema.Boolean) +}) +export interface CheckContextCheckRun extends Schema.Struct.Type {} + /** * `fragment CheckContext on StatusCheckRollupContext` * * Types that can be inside a StatusCheckRollup context. */ export const CheckContext = Schema.Union([ - StatusContext, - CheckRun, + CheckContextStatusContext, + CheckContextCheckRun, StatusCheckRollupContextOther ]) export type CheckContext = typeof CheckContext.Type @@ -174,6 +222,89 @@ export declare namespace ActorAvatars { export type Result = typeof ActorAvatars.result.Type } +/** Information about pagination in a connection. */ +export const ContextsPageInfoPageInfo = Schema.Struct({ + /** When paginating forwards, are there more items? */ + hasNextPage: Schema.Boolean, + /** When paginating forwards, the cursor to continue. */ + endCursor: Schema.NullOr(Schema.String) +}) +export interface ContextsPageInfoPageInfo extends Schema.Struct.Type {} + +/** The connection type for StatusCheckRollupContext. */ +export const ObjectStatusCheckRollupContextsStatusCheckRollupContextConnection = Schema.Struct({ + /** A list of nodes. */ + nodes: Schema.NullOr(Schema.Array(Schema.NullOr(CommitCheckContext))), + /** Information to aid in pagination. */ + pageInfo: ContextsPageInfoPageInfo +}) +export interface ObjectStatusCheckRollupContextsStatusCheckRollupContextConnection extends Schema.Struct.Type {} + +/** Represents the rollup for both the check runs and status for a commit. */ +export const ObjectStatusCheckRollupStatusCheckRollup = Schema.Struct({ + /** A list of status contexts and check runs for this commit. */ + contexts: ObjectStatusCheckRollupContextsStatusCheckRollupContextConnection +}) +export interface ObjectStatusCheckRollupStatusCheckRollup extends Schema.Struct.Type {} + +/** Represents a Git commit. */ +export const CommitCheckContextsRepositoryObjectCommit = Schema.Struct({ + __typename: Schema.Literal("Commit"), + /** The Git object ID */ + oid: Shared.GitObjectID, + /** Check and Status rollup information for this commit. */ + statusCheckRollup: Schema.NullOr(ObjectStatusCheckRollupStatusCheckRollup) +}) +export interface CommitCheckContextsRepositoryObjectCommit extends Schema.Struct.Type {} + +/** Represents a Git object. */ +export const GitObjectOther = Schema.Struct({ + __typename: GraphQL.otherTypename()(["Commit"]) +}) +export interface GitObjectOther extends Schema.Struct.Type {} + +/** A repository contains the content for a project. */ +export const CommitCheckContextsRepositoryRepository = Schema.Struct({ + /** A Git object in the repository */ + object: Schema.NullOr(Schema.Union([ + CommitCheckContextsRepositoryObjectCommit, + GitObjectOther + ])) +}) +export interface CommitCheckContextsRepositoryRepository extends Schema.Struct.Type {} + +/** `query CommitCheckContexts` */ +export const CommitCheckContexts = GraphQL.query("CommitCheckContexts", { + document: { + variables: [ + { name: "owner", body: "String!" }, + { name: "name", body: "String!" }, + { name: "oid", body: "GitObjectID!" }, + { name: "after", body: "String" } + ], + selections: [ + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){object(oid:$", "oid", "){__typename...on Commit{oid statusCheckRollup{contexts(first:100 after:$", "after", "){nodes{__typename...CommitCheckContext}pageInfo{hasNextPage endCursor}}}}}}"] } + ], + fragments: [ + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } + ] + }, + variables: { + owner: Schema.String, + name: Schema.String, + oid: Shared.GitObjectID, + after: Schema.optional(Schema.NullOr(Schema.String)) + }, + result: Schema.Struct({ + /** Lookup a given repository by the owner and repository name. */ + repository: Schema.NullOr(CommitCheckContextsRepositoryRepository) + }) +}) +export declare namespace CommitCheckContexts { + export type Variables = typeof CommitCheckContexts.variables.Type + export type Result = typeof CommitCheckContexts.result.Type +} + /** Represents an owner of a Repository. */ export const RepositoryOwner = Schema.Struct({ __typename: GraphQL.otherTypename()([]), @@ -521,7 +652,8 @@ export const PullRequestCore = GraphQL.query("PullRequestCore", { ], fragments: [ { name: "Deployment", body: ["on Deployment{id environment state commitOid createdAt updatedAt latestStatus{state updatedAt description environmentUrl logUrl}}"] }, - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...CommitCheckContext...on StatusContext{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}}"] }, + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } ] }, variables: { @@ -673,7 +805,8 @@ export const PullRequestCoreWithoutDeployments = GraphQL.query("PullRequestCoreW { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission pullRequest(number:$", "number", "){isInMergeQueue@include(if:$", "required", ")mergeStateStatus viewerCanEnableAutoMerge@include(if:$", "required", ")viewerCanDisableAutoMerge@include(if:$", "required", ")number title url body state isDraft mergeable reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:100){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage}}}}}}}}"] } ], fragments: [ - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...CommitCheckContext...on StatusContext{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}}"] }, + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } ] }, variables: { @@ -801,7 +934,8 @@ export const PullRequestCoreEnterprise = GraphQL.query("PullRequestCoreEnterpris ], fragments: [ { name: "Deployment", body: ["on Deployment{id environment state commitOid createdAt updatedAt latestStatus{state updatedAt description environmentUrl logUrl}}"] }, - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...CommitCheckContext...on StatusContext{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}}"] }, + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } ] }, variables: { @@ -926,7 +1060,8 @@ export const PullRequestCoreEnterpriseWithoutDeployments = GraphQL.query("PullRe { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed viewerPermission pullRequest(number:$", "number", "){mergeStateStatus number title url body state isDraft mergeable reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:100){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage}}}}}}}}"] } ], fragments: [ - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...CommitCheckContext...on StatusContext{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}}"] }, + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } ] }, variables: { @@ -946,15 +1081,6 @@ export declare namespace PullRequestCoreEnterpriseWithoutDeployments { export type Result = typeof PullRequestCoreEnterpriseWithoutDeployments.result.Type } -/** Information about pagination in a connection. */ -export const ContextsPageInfoPageInfo = Schema.Struct({ - /** When paginating forwards, are there more items? */ - hasNextPage: Schema.Boolean, - /** When paginating forwards, the cursor to continue. */ - endCursor: Schema.NullOr(Schema.String) -}) -export interface ContextsPageInfoPageInfo extends Schema.Struct.Type {} - /** The connection type for StatusCheckRollupContext. */ export const PullRequestCheckContextsRepositoryPullRequestCommitsNodesCommitStatusCheckRollupContextsStatusCheckRollupContextConnection = Schema.Struct({ /** A list of nodes. */ @@ -1022,7 +1148,8 @@ export const PullRequestCheckContexts = GraphQL.query("PullRequestCheckContexts" { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){headRefOid commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100 after:$", "after", "){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage endCursor}}}}}}}}"] } ], fragments: [ - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...CommitCheckContext...on StatusContext{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}}"] }, + { name: "CommitCheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description}...on CheckRun{name status conclusion startedAt completedAt detailsUrl checkSuite{workflowRun{workflow{name}}}}}"] } ] }, variables: { @@ -2114,24 +2241,18 @@ export const CheckSuiteConnection = Schema.Struct({ export interface CheckSuiteConnection extends Schema.Struct.Type {} /** Represents a Git commit. */ -export const ObjectCommit = Schema.Struct({ +export const WorkflowApprovalSuitesRepositoryObjectCommit = Schema.Struct({ __typename: Schema.Literal("Commit"), /** The check suites associated with a commit. */ checkSuites: Schema.NullOr(CheckSuiteConnection) }) -export interface ObjectCommit extends Schema.Struct.Type {} - -/** Represents a Git object. */ -export const GitObjectOther = Schema.Struct({ - __typename: GraphQL.otherTypename()(["Commit"]) -}) -export interface GitObjectOther extends Schema.Struct.Type {} +export interface WorkflowApprovalSuitesRepositoryObjectCommit extends Schema.Struct.Type {} /** A repository contains the content for a project. */ export const WorkflowApprovalSuitesRepositoryRepository = Schema.Struct({ /** A Git object in the repository */ object: Schema.NullOr(Schema.Union([ - ObjectCommit, + WorkflowApprovalSuitesRepositoryObjectCommit, GitObjectOther ])) }) @@ -2168,4 +2289,4 @@ export declare namespace WorkflowApprovalSuites { } /** Every operation in pullRequestReads.graphql. */ -export const PullRequestReadsGraphQLGroup = GraphQLGroup.make(ActorAvatars, PullRequestCore, PullRequestCoreWithoutDeployments, PullRequestCoreEnterprise, PullRequestCoreEnterpriseWithoutDeployments, PullRequestCheckContexts, PullRequestPreview, PullRequestActivity, PullRequestHeads, PullRequestNodeId, PullRequestSubject, ViewerPermissions, ReviewerCandidates, LabelCandidates, PullRequestFilesViewed, PullRequestMergeMessage, PullRequestActionState, PullRequestBranchToDelete, PullRequestSourceBranch, AutoMergePermissions, BranchProtectionRequiredChecks, WorkflowApprovalSuites) +export const PullRequestReadsGraphQLGroup = GraphQLGroup.make(ActorAvatars, CommitCheckContexts, PullRequestCore, PullRequestCoreWithoutDeployments, PullRequestCoreEnterprise, PullRequestCoreEnterpriseWithoutDeployments, PullRequestCheckContexts, PullRequestPreview, PullRequestActivity, PullRequestHeads, PullRequestNodeId, PullRequestSubject, ViewerPermissions, ReviewerCandidates, LabelCandidates, PullRequestFilesViewed, PullRequestMergeMessage, PullRequestActionState, PullRequestBranchToDelete, PullRequestSourceBranch, AutoMergePermissions, BranchProtectionRequiredChecks, WorkflowApprovalSuites) From a0f16adcc636a30b4830fd8d0cce28a29fe2ec4b Mon Sep 17 00:00:00 2001 From: Cameron Date: Sun, 25 Oct 2026 21:25:25 +0000 Subject: [PATCH 2/4] fix(pull-requests): keep unnamed merge checks unavailable --- .../src/server/GitHubPullRequestApi.test.ts | 34 +++++++++++++++++++ .../src/server/GitHubPullRequestApi.ts | 11 +++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index b81656aea82b..15f7413c8cdd 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -960,6 +960,40 @@ layer("GitHubPullRequestApi.layer", (it) => { }), ); + it.effect("keeps merge checks unavailable when a nameless context could be dropped", () => + Effect.gen(function* () { + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + for (const hidden of [ + checkRun({ name: "", conclusion: "FAILURE" }), + checkRun({ name: " ", status: "IN_PROGRESS" }), + ]) { + for (const checks of [ + [hidden], + [checkRun({ name: "Tests", conclusion: "SUCCESS" }), hidden], + ]) { + mockedExecute.mockImplementation((call) => + Effect.succeed( + output( + encodeJson( + call.kind === "rest" + ? { merged: true, merge_commit_sha: mergedOid } + : call.operationName === "CommitCheckContexts" + ? commitChecksPage(checks) + : coreResponse({ state: "MERGED" }), + ), + ), + ), + ); + expect((yield* cli.getPullRequestDetail(mergeInput)).mergeCommit).toEqual({ + oid: mergedOid, + url: `https://github.com/acme/web/commit/${mergedOid}`, + checks: null, + }); + } + } + }), + ); + it.effect( "keeps unreadable and incomplete merge checks unavailable, with the commit still linked", () => diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index ea00872051b8..98fb23f59a98 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -1646,11 +1646,14 @@ export const make = Effect.gen(function* () { }, ).pipe( graphql(target), - Effect.map(({ pages, truncated }) => - truncated || pages.length === 0 || pages.some((page) => page.oid !== oid) + Effect.map(({ pages, truncated }) => { + if (truncated || pages.length === 0 || pages.some((page) => page.oid !== oid)) return null; + const contexts = pages.flatMap((page) => page.contexts); + // Dropping unnamed contexts could hide a failed or pending check from the summary. + return contexts.some((context) => !(context.name?.trim() || context.context?.trim())) ? null - : pullRequestChecksFromContexts(pages.flatMap((page) => page.contexts)).checks, - ), + : pullRequestChecksFromContexts(contexts).checks; + }), Effect.orElseSucceed(() => null), ); return { From d46f1f055c3e75d68c9d108840135df0072c0407 Mon Sep 17 00:00:00 2001 From: Cameron Date: Sun, 25 Oct 2026 21:28:11 +0000 Subject: [PATCH 3/4] docs(fork): link merge commit checks to its pull request --- .github/fork-features.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/fork-features.yml b/.github/fork-features.yml index 3f39357b2037..f4eaadc66bc5 100644 --- a/.github/fork-features.yml +++ b/.github/fork-features.yml @@ -4016,7 +4016,7 @@ features: - id: pull-request-merge-commit-checks title: Inspect checks on a merged GitHub pull request's resulting commit status: maintained - prs: [] + prs: [1074] invariants: - A merged GitHub pull request reads the resulting SHA reported after merging, including squash and rebase, without substituting a base tip or temporary test merge. - Merge commit checks, commit links, and check details remain distinct from source-branch checks on web, desktop, and mobile; a merged status does not imply passing checks. From dbdacecda76f61b6343ae1abdb063067060c1cfe Mon Sep 17 00:00:00 2001 From: Cameron Date: Sun, 25 Oct 2026 22:41:12 +0000 Subject: [PATCH 4/4] fix(pull-requests): use one list for merge and PR checks --- .github/fork-features.yml | 2 +- .../pullRequest/PullRequestChecksTab.tsx | 11 +- .../pullRequest/PullRequestDetailPanel.tsx | 49 +++--- ...lRequestMergeCommitChecks.browser.test.tsx | 95 +++++++++--- .../PullRequestMergeCommitChecks.tsx | 139 +++++++++--------- .../pullRequest/PullRequestSummaryTab.tsx | 20 --- docs/user/source-control.md | 5 +- 7 files changed, 172 insertions(+), 149 deletions(-) diff --git a/.github/fork-features.yml b/.github/fork-features.yml index f4eaadc66bc5..9b978a92925c 100644 --- a/.github/fork-features.yml +++ b/.github/fork-features.yml @@ -4019,7 +4019,7 @@ features: prs: [1074] invariants: - A merged GitHub pull request reads the resulting SHA reported after merging, including squash and rebase, without substituting a base tip or temporary test merge. - - Merge commit checks, commit links, and check details remain distinct from source-branch checks on web, desktop, and mobile; a merged status does not imply passing checks. + - Merge commit checks, commit links, and check details remain distinct from source-branch checks on web, desktop, and mobile; web and desktop switch between the two scopes in one shared checklist, and a merged status does not imply passing checks. - Pending checks, no reported checks, unavailable commits, and unreadable or incomplete checks remain distinct; visible views refresh through merging and later check updates. implementation_paths: - apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx diff --git a/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx b/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx index 7e10adea1e03..8e69ff0d5385 100644 --- a/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestChecksTab.tsx @@ -208,7 +208,7 @@ export function PullRequestChecksTab({ pendingFinding, fixCheckLabel = "Fix", onFixFinding, - embedded = false, + onOpenCheck, }: { checks: ReadonlyArray; /** Newer status contradicts `checks`; the runs are hidden until the detail is refreshed. */ @@ -230,7 +230,7 @@ export function PullRequestChecksTab({ pendingFinding?: string | null; fixCheckLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; - embedded?: boolean; + onOpenCheck?: (url: string) => void; }) { const state = pullRequestChecksState(checks); // Null for a set nobody can call passed or failed — every run skipped, say. The list still @@ -276,7 +276,8 @@ export function PullRequestChecksTab({ const keyOccurrences = new Map(); const openCheck = (url: string) => { - void readLocalApi()?.shell.openExternal(url); + if (onOpenCheck) onOpenCheck(url); + else void readLocalApi()?.shell.openExternal(url); }; if (stale) { @@ -293,7 +294,7 @@ export function PullRequestChecksTab({ } return ( -
+
{mergePresentation || rollup || requiredBranchUpdate || waitingRuns.length > 0 ? ( // The verdict rides the top of the scroll box the way the summary's section headings do, // so a long list of runs never scrolls its own answer out of sight. @@ -387,7 +388,7 @@ export function PullRequestChecksTab({

No checks reported.

- Runs appear here as soon as the host reports one for this branch. + Runs appear here as soon as the host reports one.

) : ( diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 945fefe4c2ed..d8909db8b449 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -96,6 +96,7 @@ import { mergedPullRequestCommit } from "@t3tools/client-runtime/pull-request-me import { PullRequestMergeCommitChecks, PullRequestMergeCommitStatus, + type PullRequestChecksScope, } from "./PullRequestMergeCommitChecks"; import { pullRequestEnvironment, @@ -683,6 +684,20 @@ function PullRequestDetailPanelBody({ const [threadPickerOpen, setThreadPickerOpen] = useState(false); const [openReviewRequest, setOpenReviewRequest] = useState(0); const [tab, setTab] = useState("summary"); + const [checksScopeSelection, setChecksScopeSelection] = useState<{ + pullRequestKey: string; + scope: PullRequestChecksScope; + } | null>(null); + const checksScope = + checksScopeSelection?.pullRequestKey === pullRequestKey + ? checksScopeSelection.scope + : "merge-commit"; + const selectChecksScope = (scope: PullRequestChecksScope) => + setChecksScopeSelection({ pullRequestKey, scope }); + const openMergeCommitChecks = () => { + selectChecksScope("merge-commit"); + setTab("checks"); + }; const [timelineOrder, setTimelineOrder] = useState<"newest" | "oldest">("newest"); const [codeCommitScope, setCodeCommitScope] = useState<{ readonly pullRequestKey: string; @@ -1946,7 +1961,7 @@ function PullRequestDetailPanelBody({ {mergeCommit !== undefined ? ( setTab("checks")} + onSelect={openMergeCommitChecks} /> ) : mergeVerdict ? ( @@ -2793,7 +2808,7 @@ function PullRequestDetailPanelBody({ {mergeCommit !== undefined ? ( setTab("checks")} + onSelect={openMergeCommitChecks} /> ) : ( -
- {mergeCommit !== undefined ? ( -
- -
- ) : null} - {detail.state === "merged" ? ( -

- Pull request - {detail.headSha ? ( - - {detail.headSha.slice(0, 7)} - - ) : null} -

- ) : null} + -
+
) : null} {mountedTabs.has("guide") ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx index a37a98cc6bad..be08904d0cce 100644 --- a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.browser.test.tsx @@ -1,14 +1,21 @@ import { EnvironmentId, type PullRequestMergeCommit } from "@t3tools/contracts"; -import { act } from "react"; +import { act, useState } from "react"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; import { userEvent } from "vite-plus/test/browser"; -const { openLink, report } = vi.hoisted(() => ({ openLink: vi.fn(), report: vi.fn() })); +const { openLink, report, openSourceCheck } = vi.hoisted(() => ({ + openLink: vi.fn(), + report: vi.fn(), + openSourceCheck: vi.fn(), +})); vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => openLink })); vi.mock("~/forkClientUsage", () => ({ reportForkClientUsage: report })); - -import { PullRequestMergeCommitChecks } from "./PullRequestMergeCommitChecks"; +import { PullRequestChecksTab } from "./PullRequestChecksTab"; +import { + PullRequestMergeCommitChecks, + type PullRequestChecksScope, +} from "./PullRequestMergeCommitChecks"; const environmentId = EnvironmentId.make("merge-checks-test"); const commit: PullRequestMergeCommit = { @@ -18,6 +25,30 @@ const commit: PullRequestMergeCommit = { { name: "Deploy", status: "pending", description: null, url: "https://example.com/runs/1" }, ], }; +function Harness({ value }: { value: PullRequestMergeCommit | null | undefined }) { + const [scope, setScope] = useState("merge-commit"); + return ( + + + + ); +} let container: HTMLDivElement; let root: Root; beforeEach(() => { @@ -33,29 +64,35 @@ afterEach(async () => { container.remove(); vi.unstubAllGlobals(); }); +const render = (value: PullRequestMergeCommit | null | undefined) => + act(async () => root.render()); +const scopeButton = (label: string) => + Array.from(container.querySelectorAll("button")).find((button) => button.textContent === label)!; +const checkButton = (name: string) => + container.querySelector(`[aria-label="Open ${name} on the host"]`)!; -const render = (value: PullRequestMergeCommit | null) => - act(async () => - root.render(), - ); - -it("opens the resulting commit and its own check details, and updates while expanded", async () => { +it("switches scopes without mixing their check rows, SHAs or links", async () => { await render(commit); - expect(container.textContent).toContain("1 check pending"); - expect(container.textContent).not.toContain("Deploy"); + expect(container.textContent).toContain("Deploy"); + expect(container.textContent).toContain("bbbbbbb"); + expect(container.textContent).not.toContain("Source tests"); await act(async () => userEvent.click(container.querySelector("a")!)); expect(openLink).toHaveBeenLastCalledWith(commit.url); - await act(async () => userEvent.click(container.querySelector("button")!)); - expect(container.textContent).toContain("Deploy"); - const failed: PullRequestMergeCommit = { + await act(async () => userEvent.click(scopeButton("Pull request"))); + expect(container.textContent).toContain("Source tests"); + expect(container.textContent).toContain("aaaaaaa"); + expect(container.textContent).not.toContain("Deploy"); + await act(async () => userEvent.click(checkButton("Source tests"))); + expect(openSourceCheck).toHaveBeenCalledExactlyOnceWith("https://example.com/source"); + expect(report).toHaveBeenCalledTimes(1); + await act(async () => userEvent.click(scopeButton("Merge commit"))); + await render({ ...commit, checks: [{ ...commit.checks![0]!, status: "failure", description: "Deployment failed" }], - }; - await render(failed); - expect(container.textContent).toContain("1 check failed"); - expect(container.textContent).toContain("Deployment failed"); - expect(container.textContent).not.toContain("pending"); - await act(async () => userEvent.click(container.querySelectorAll("a")[1]!)); + }); + expect(container.textContent).toContain("Failed"); + expect(container.textContent).not.toContain("Running"); + await act(async () => userEvent.click(checkButton("Deploy"))); expect(openLink).toHaveBeenLastCalledWith("https://example.com/runs/1"); expect(report.mock.calls).toEqual([ [ @@ -68,7 +105,7 @@ it("opens the resulting commit and its own check details, and updates while expa ], ]); openLink.mockRejectedValueOnce(new Error("Unavailable")); - await act(async () => userEvent.click(container.querySelectorAll("a")[1]!)); + await act(async () => userEvent.click(checkButton("Deploy"))); expect(report).toHaveBeenLastCalledWith(environmentId, { feature: "pull-request-merge-commit-checks", action: "open_check", @@ -76,18 +113,28 @@ it("opens the resulting commit and its own check details, and updates while expa }); }); -it("keeps commit links usable when checks are unavailable and updates when checks arrive", async () => { +it("keeps unavailable and empty merge results distinct while retaining source checks and commit links", async () => { await render(null); expect(container.textContent).toContain("Merge commit unavailable"); expect(container.querySelector("a")).toBeNull(); + await act(async () => userEvent.click(scopeButton("Pull request"))); + expect(container.textContent).toContain("Source tests"); + await act(async () => userEvent.click(scopeButton("Merge commit"))); await render({ ...commit, checks: null }); expect(container.textContent).toContain("Checks unavailable"); + expect(container.textContent).not.toContain("No checks reported"); await act(async () => userEvent.click(container.querySelector("a")!)); expect(openLink).toHaveBeenLastCalledWith(commit.url); await render({ ...commit, checks: [] }); expect(container.textContent).toContain("No checks reported"); await render(commit); - await act(async () => userEvent.click(container.querySelector("button")!)); expect(container.textContent).toContain("Deploy"); expect(container.textContent).toContain("Running"); }); + +it("preserves ordinary pull request checks when merge commit data is unsupported", async () => { + await render(undefined); + expect(container.textContent).toContain("Source tests"); + expect(container.textContent).not.toContain("Merge commit"); + expect(container.querySelector("a")).toBeNull(); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx index 82bfec1b7f69..d5d64b6aac5d 100644 --- a/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMergeCommitChecks.tsx @@ -1,19 +1,18 @@ import { mergeCommitChecksSummary } from "@t3tools/client-runtime/pull-request-merge-commit"; import type { EnvironmentId, PullRequestMergeCommit } from "@t3tools/contracts"; import { CircleHelpIcon, CircleDotIcon } from "lucide-react"; -import { useState } from "react"; +import type { ReactNode } from "react"; import { useOpenLink } from "~/browser/useOpenLink"; import { reportForkClientUsage } from "~/forkClientUsage"; import { cn } from "~/lib/utils"; -import { Button } from "../ui/button"; +import { Toggle, ToggleGroup } from "../ui/toggle-group"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; -import { - PullRequestCheckStatusIcon, - pullRequestCheckStatusLabel, - pullRequestChecksStatePresentation, -} from "./pullRequestPresentation"; +import { PullRequestChecksTab } from "./PullRequestChecksTab"; +import { pullRequestChecksStatePresentation } from "./pullRequestPresentation"; + +export type PullRequestChecksScope = "merge-commit" | "pull-request"; export function PullRequestMergeCommitStatus({ commit, @@ -45,17 +44,20 @@ export function PullRequestMergeCommitStatus({ export function PullRequestMergeCommitChecks({ commit, + headSha, environmentId, - expanded = false, + scope, + onScopeChange, + children, }: { - commit: PullRequestMergeCommit | null; + commit: PullRequestMergeCommit | null | undefined; + headSha: string | null; environmentId?: EnvironmentId | undefined; - expanded?: boolean; + scope: PullRequestChecksScope; + onScopeChange: (scope: PullRequestChecksScope) => void; + children: ReactNode; }) { - const [showChecks, setShowChecks] = useState(expanded); const openLink = useOpenLink(null); - const summary = mergeCommitChecksSummary(commit); - const keyOccurrences = new Map(); const open = (url: string, action: "open_commit" | "open_check") => { void openLink(url).then( () => @@ -72,76 +74,67 @@ export function PullRequestMergeCommitChecks({ }), ); }; + if (commit === undefined) return children; + const mergeSelected = scope === "merge-commit"; + const sha = mergeSelected ? commit?.oid : headSha; return ( -
-
- {commit === null ? summary.label : "Merge commit"} - {commit ? ( +
+ - {showChecks && commit?.checks && commit.checks.length > 0 ? ( - - ) : null} +
+ {!mergeSelected ? ( + children + ) : commit === null || commit.checks === null ? ( +
+ + {commit === null ? "Merge commit unavailable" : "Checks unavailable"} +
+ ) : ( + open(url, "open_check")} + /> + )} +
); } diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index 65ca913a4274..5c22ec5f2322 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -1,5 +1,4 @@ import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; -import { mergedPullRequestCommit } from "@t3tools/client-runtime/pull-request-merge-commit"; import { useAtomCommand } from "~/state/use-atom-command"; import type { EnvironmentId, @@ -24,13 +23,11 @@ import { useRef, useState, type ReactNode } from "react"; import { pullRequestEnvironment } from "~/state/pullRequests"; import { useLocalStorage } from "~/hooks/useLocalStorage"; import { cn } from "~/lib/utils"; -import { useOpenLink } from "~/browser/useOpenLink"; import { RelativeTime } from "~/components/RelativeTime"; import { Button } from "../ui/button"; import { PullRequestEditButton } from "./PullRequestEditButton"; import { PullRequestDeployments } from "./PullRequestDeployments"; -import { PullRequestMergeCommitChecks } from "./PullRequestMergeCommitChecks"; import { pullRequestDeploymentsTitle } from "./pullRequestDeployments.logic"; import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; import { @@ -458,14 +455,6 @@ export function PullRequestSummaryTab({ })), ]; - const openLink = useOpenLink(threadRef); - const openCheck = (url: string) => { - void openLink(url).catch((error: unknown) => { - console.error(error); - toastManager.add({ type: "error", title: "Unable to open check details" }); - }); - }; - const update = useAtomCommand(pullRequestEnvironment.update, { reportFailure: false }); const updateComment = useAtomCommand(pullRequestEnvironment.updateComment, { reportFailure: false, @@ -719,15 +708,6 @@ export function PullRequestSummaryTab({
- {mergedPullRequestCommit(detail) !== undefined ? ( -
- -
- ) : null} - {detail.deployments && detail.deployments.length > 0 ? (