Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .github/fork-features.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: [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; 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
- 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -113,6 +114,7 @@ import { PullRequestMarkdownRepository } from "./PullRequestMarkdown";
import {
PullRequestStatusMenu,
PullRequestChecksRow,
PullRequestMergeCommitRow,
PullRequestBranchRow,
PullRequestReviewersRow,
} from "./PullRequestStatusRows";
Expand Down Expand Up @@ -256,6 +258,7 @@ function PullRequestOverview(props: {
<View className="gap-5">
<PullRequestStackCard params={props.params} state={props.stack} />
<PullRequestCardGroup>
<PullRequestMergeCommitRow pr={pr} params={props.params} />
<PullRequestChecksRow pr={pr} params={props.params} />
<PullRequestReviewersRow pr={pr} params={props.params} />
<Animated.View layout={PR_ROW_LAYOUT}>
Expand Down Expand Up @@ -348,6 +351,7 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps<PullRequestP
const { refresh: refreshDetail } = result;
useFocusEffect(() => 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);
Expand Down
72 changes: 71 additions & 1 deletion apps/mobile/src/features/pull-requests/PullRequestStatusRows.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -21,6 +25,7 @@ import {
checkStatus,
pullRequestBranchSummary,
pullRequestChecksSummary,
pullRequestChecksStatus,
pullRequestReviewSummary,
pullRequestStatus,
} from "./pullRequestStatus";
Expand Down Expand Up @@ -110,7 +115,12 @@ export function PullRequestChecksRow(props: RowProps) {
return (
<Animated.View layout={PR_ROW_LAYOUT}>
<PullRequestCardRow
title={checks.title}
title={props.pr.state === "merged" ? "Pull request" : checks.title}
detail={
props.pr.state === "merged"
? `${props.pr.headSha?.slice(0, 7) ?? props.pr.headBranch} · ${checks.label}`
: undefined
}
icon={checks.icon}
tint={checks.tint}
value={checks.count}
Expand Down Expand Up @@ -179,6 +189,66 @@ export function PullRequestChecksRow(props: RowProps) {
);
}

export function PullRequestMergeCommitRow(props: RowProps) {
const [expanded, setExpanded] = useState(false);
const commit = mergedPullRequestCommit(props.pr);
if (commit === undefined) return null;
const summary = mergeCommitChecksSummary(commit);
const keyOccurrences = new Map<string, number>();
const status = summary.state === null ? null : pullRequestChecksStatus(summary.state);
return (
<Animated.View layout={PR_ROW_LAYOUT}>
<PullRequestCardRow
title="Merge commit"
icon={status?.icon ?? "git.commit"}
tint={status?.tint}
value={commit?.oid.slice(0, 7)}
detail={summary.label}
accessibilityLabel={
commit ? `Merge commit, ${commit.oid.slice(0, 7)}, ${summary.label}` : summary.label
}
expanded={commit ? expanded : undefined}
onPress={commit ? () => setExpanded(!expanded) : undefined}
/>
{expanded ? (
<Animated.View
layout={PR_ROW_LAYOUT}
entering={PR_DISCLOSURE_ENTER}
exiting={PR_ROW_EXIT}
className="pl-8"
>
{commit ? (
<PullRequestCardRow
title={commit.oid.slice(0, 7)}
icon="git.commit"
value="Details"
onPress={() => 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 (
<PullRequestCardRow
key={`${keyBase}:${occurrence}`}
title={check.name}
icon={status.icon}
tint={status.tint}
value={check.url ? `${status.label} · Details` : status.label}
detail={check.description ?? undefined}
onPress={url ? () => void Linking.openURL(url) : undefined}
/>
);
})}
</Animated.View>
) : null}
</Animated.View>
);
}

export function PullRequestBranchRow(props: RowProps) {
const action = usePullRequestAction(props.params, props.pr);
const agent = usePullRequestAgentActions(props.params, props.pr);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, unknown>) => {
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(<Probe enabled />));
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(<Probe enabled={false} />));
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<void>((done) => {
resolve = done;
}),
);
await act(async () => root.render(<Probe enabled />));
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();
});
Original file line number Diff line number Diff line change
@@ -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]),
);
}
Loading
Loading