From efaf7f9899669c9785f3693a7bf3ec42e110d1f0 Mon Sep 17 00:00:00 2001 From: Rodrigo Brechard Date: Mon, 2 Nov 2026 16:07:41 +0100 Subject: [PATCH 1/2] fix(web): pull request header no longer flashes back to the pre-action state After a header action succeeded (ready for review, convert to draft, close, reopen, auto-merge), the panel cleared its pending state and refetched without waiting, so the header redrew the pre-action detail until the refetch landed. The detail now applies each confirmed action outcome over the fetched data and drops it once the fetched detail agrees or after 60 seconds. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestDetailPanel.tsx | 4 + .../pullRequestDetail.logic.test.ts | 76 ++++++++++++++++ .../pullRequest/pullRequestDetail.logic.ts | 89 +++++++++++++++++++ .../pullRequest/pullRequestList.logic.ts | 2 +- .../pullRequest/usePullRequestDetail.ts | 23 ++++- 5 files changed, 192 insertions(+), 2 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 4c8e1e2acbcd6..e470a670c891c 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -136,6 +136,7 @@ import { allowsSinglePullRequestMerge, canDeletePullRequestBranch, isPullRequestConflicting, + pullRequestDetailPatchAfterAction, resolveBaseFreshness, resolvePullRequestMergeMethod, type PullRequestFinding, @@ -347,6 +348,7 @@ export function PullRequestDetailPanel({ const [threadPickerOpen, setThreadPickerOpen] = useState(false); const [openReviewRequest, setOpenReviewRequest] = useState(0); const { + recordActionOutcome, canWriteSourceControl, detailQuery, activityQuery, @@ -687,6 +689,8 @@ export function PullRequestDetailPanel({ onActed?.(action, "failed"); return false; } + const outcome = pullRequestDetailPatchAfterAction(action); + if (outcome !== null) recordActionOutcome(outcome); if (action === "revert") { toastManager.add({ type: "success", title: `Revert ${terminology.singular} opened` }); } diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 7c1cc47883f6f..0f6161efddebd 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -40,6 +40,10 @@ import { pullRequestPanelContext, threadPullRequestPanelTarget, latestPullRequestReviewOutcomes, + applyPullRequestDetailOverride, + pullRequestDetailPatchAfterAction, + recordPullRequestDetailOverride, + settlePullRequestDetailOverride, newestPullRequestCommitAt, mergePullRequestThreadComments, orderPullRequestComments, @@ -315,6 +319,78 @@ describe("review verdicts", () => { }); }); +describe("confirmed action outcome on the detail", () => { + const scope = "env:project:host:repo#1"; + const detail = { state: "open" as const, isDraft: true, autoMergeEnabled: false }; + const at = 1_000_000; + const record = (patch: NonNullable>) => + recordPullRequestDetailOverride(null, scope, patch, at); + + it("maps each action to the state it confirms, and leaves merge and the rest alone", () => { + expect( + ( + ["ready", "draft", "close", "reopen", "enable-auto-merge", "disable-auto-merge"] as const + ).map(pullRequestDetailPatchAfterAction), + ).toEqual([ + { isDraft: false }, + { isDraft: true }, + { state: "closed" }, + { state: "open" }, + { autoMergeEnabled: true }, + { autoMergeEnabled: false }, + ]); + expect(pullRequestDetailPatchAfterAction("merge")).toBeNull(); + }); + + it("shows the outcome over the fetched detail", () => { + expect(applyPullRequestDetailOverride(detail, record({ isDraft: false }))).toMatchObject({ + state: "open", + isDraft: false, + }); + expect(applyPullRequestDetailOverride(detail, record({ state: "closed" })).state).toBe( + "closed", + ); + expect(applyPullRequestDetailOverride(detail, null)).toBe(detail); + }); + + it("accumulates outcomes within a pull request and starts over for another", () => { + const first = record({ isDraft: false }); + expect(recordPullRequestDetailOverride(first, scope, { state: "closed" }, at + 1)).toEqual({ + scope, + at: at + 1, + isDraft: false, + state: "closed", + }); + expect(recordPullRequestDetailOverride(first, "other", { state: "closed" }, at + 1)).toEqual({ + scope: "other", + at: at + 1, + state: "closed", + }); + }); + + it("retires a field once the fetched detail agrees", () => { + const override = recordPullRequestDetailOverride( + record({ isDraft: false }), + scope, + { autoMergeEnabled: true }, + at, + ); + expect(settlePullRequestDetailOverride(override, detail, scope)).toBe(override); + expect(settlePullRequestDetailOverride(override, { ...detail, isDraft: false }, scope)).toEqual( + { scope, at, autoMergeEnabled: true }, + ); + expect( + settlePullRequestDetailOverride( + override, + { ...detail, isDraft: false, autoMergeEnabled: true }, + scope, + ), + ).toBeNull(); + expect(settlePullRequestDetailOverride(override, detail, "other")).toBeNull(); + expect(settlePullRequestDetailOverride(override, null, scope)).toBe(override); + }); +}); + describe("pull request timeline", () => { it("orders host timestamps by instant across offsets and fractional seconds", () => { const events = buildPullRequestTimeline({ diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 2a7dc7151b92d..3bea99d8ecea6 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -1403,3 +1403,92 @@ export function resolveDisplayedPullRequestDetail(input: { ? input.cached : null; } + +export interface PullRequestDetailPatch { + readonly state?: PullRequestState; + readonly isDraft?: boolean; + readonly autoMergeEnabled?: boolean; +} + +export interface PullRequestDetailOverride extends PullRequestDetailPatch { + readonly scope: string; + readonly at: number; +} + +export function pullRequestDetailPatchAfterAction( + action: PullRequestAction, +): PullRequestDetailPatch | null { + switch (action) { + case "ready": + return { isDraft: false }; + case "draft": + return { isDraft: true }; + case "close": + return { state: "closed" }; + case "reopen": + return { state: "open" }; + case "enable-auto-merge": + return { autoMergeEnabled: true }; + case "disable-auto-merge": + return { autoMergeEnabled: false }; + default: + return null; + } +} + +export function recordPullRequestDetailOverride( + previous: PullRequestDetailOverride | null, + scope: string, + patch: PullRequestDetailPatch, + now: number, +): PullRequestDetailOverride { + return { ...(previous?.scope === scope ? previous : {}), ...patch, scope, at: now }; +} + +type OverridableDetail = Pick; + +export function applyPullRequestDetailOverride( + detail: Detail, + override: PullRequestDetailOverride | null, +): Detail { + if (override === null) return detail; + const { state, isDraft, autoMergeEnabled } = override; + return { + ...detail, + ...(state === undefined ? {} : { state }), + ...(isDraft === undefined ? {} : { isDraft }), + ...(autoMergeEnabled === undefined ? {} : { autoMergeEnabled }), + }; +} + +/** Drop what the fetched detail now agrees with; the rest holds until it expires. */ +export function settlePullRequestDetailOverride( + override: PullRequestDetailOverride | null, + detail: OverridableDetail | null, + scope: string, +): PullRequestDetailOverride | null { + if (override === null) return null; + if (override.scope !== scope) return null; + if (detail === null) return override; + const { state, isDraft, autoMergeEnabled } = override; + const open = { + state: state !== undefined && detail.state !== state, + isDraft: isDraft !== undefined && detail.isDraft !== isDraft, + autoMergeEnabled: + autoMergeEnabled !== undefined && detail.autoMergeEnabled !== autoMergeEnabled, + }; + if (!Object.values(open).some(Boolean)) return null; + if ( + open.state === (state !== undefined) && + open.isDraft === (isDraft !== undefined) && + open.autoMergeEnabled === (autoMergeEnabled !== undefined) + ) + return override; + return { + scope: override.scope, + at: override.at, + ...(open.state ? { state } : {}), + ...(open.isDraft ? { isDraft } : {}), + ...(open.autoMergeEnabled ? { autoMergeEnabled } : {}), + }; +} diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.ts index b3e447a82e598..162a27ab8e5a1 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.ts @@ -866,7 +866,7 @@ export function reusePullRequestEntries( : out; } -const PULL_REQUEST_OVERRIDE_TRUST_MS = 60_000; +export const PULL_REQUEST_OVERRIDE_TRUST_MS = 60_000; /** Retire overrides only when returned rows agree; partial-feed absence proves nothing. */ export function settlePullRequestOverrides( diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.ts b/apps/web/src/components/pullRequest/usePullRequestDetail.ts index 7451ee38c5fee..49cb55323842a 100644 --- a/apps/web/src/components/pullRequest/usePullRequestDetail.ts +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.ts @@ -18,9 +18,15 @@ import { useSharedPullRequestSummary, } from "~/state/pullRequests"; import { + applyPullRequestDetailOverride, readPullRequestDetailSnapshot, + recordPullRequestDetailOverride, resolveDisplayedPullRequestDetail, + settlePullRequestDetailOverride, + type PullRequestDetailOverride, + type PullRequestDetailPatch, } from "./pullRequestDetail.logic"; +import { PULL_REQUEST_OVERRIDE_TRUST_MS } from "./pullRequestList.logic"; export function pullRequestRefKey(reference: PullRequestRef) { return `${reference.projectId}:${reference.host ?? ""}:${reference.repository}#${reference.number}`; @@ -88,6 +94,7 @@ export function usePullRequestDetail( } else if (detailQuery.data !== null && detailQuery.data !== cachedDetail) { setCachedDetail(detailQuery.data); } + const [override, setOverride] = useState(null); const observedSummary = useSharedPullRequestSummary(environmentId, reference); const activity = activityQuery.data; const resolvedCoreDetail = resolveDisplayedPullRequestDetail({ @@ -128,7 +135,7 @@ export function usePullRequestDetail( : sharedSummary.mergedAt, isDraft: sharedSummary.isDraft ?? resolvedCoreDetail.isDraft, }; - const detail = + const fetchedDetail = coreDetail === null ? null : { @@ -169,7 +176,21 @@ export function usePullRequestDetail( commits: activity?.commits ?? [], reactions: activity?.reactions ?? [], }; + const settledOverride = settlePullRequestDetailOverride(override, fetchedDetail, scopeKey); + if (settledOverride !== override) setOverride(settledOverride); + const detail = + fetchedDetail === null ? null : applyPullRequestDetailOverride(fetchedDetail, settledOverride); + const recordActionOutcome = (patch: PullRequestDetailPatch) => { + const at = Date.now(); + setOverride((previous) => recordPullRequestDetailOverride(previous, scopeKey, patch, at)); + // A fetch that never agrees must not pin the confirmed outcome forever. + window.setTimeout( + () => setOverride((previous) => (previous?.at === at ? null : previous)), + PULL_REQUEST_OVERRIDE_TRUST_MS, + ); + }; return { + recordActionOutcome, canWriteSourceControl, detailQuery, activityQuery, From d08a04d00508dc2950ababbb27f4ed48f6ba1389 Mon Sep 17 00:00:00 2001 From: Rodrigo Brechard Date: Mon, 2 Nov 2026 16:34:53 +0100 Subject: [PATCH 2/2] fix(web): each confirmed header outcome expires on its own deadline A second action within the trust window replaced the shared timestamp, so the first action's field outlived its own deadline. Each overridden field now carries its own timestamp and the timer expires only the fields it stamped. Co-Authored-By: Claude Opus 5.5 --- .../pullRequestDetail.logic.test.ts | 24 +++++++- .../pullRequest/pullRequestDetail.logic.ts | 57 +++++++++++++------ .../pullRequest/usePullRequestDetail.ts | 3 +- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 0f6161efddebd..9e53fd03c1904 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -41,6 +41,7 @@ import { threadPullRequestPanelTarget, latestPullRequestReviewOutcomes, applyPullRequestDetailOverride, + expirePullRequestDetailOverride, pullRequestDetailPatchAfterAction, recordPullRequestDetailOverride, settlePullRequestDetailOverride, @@ -357,13 +358,13 @@ describe("confirmed action outcome on the detail", () => { const first = record({ isDraft: false }); expect(recordPullRequestDetailOverride(first, scope, { state: "closed" }, at + 1)).toEqual({ scope, - at: at + 1, + at: { isDraft: at, state: at + 1 }, isDraft: false, state: "closed", }); expect(recordPullRequestDetailOverride(first, "other", { state: "closed" }, at + 1)).toEqual({ scope: "other", - at: at + 1, + at: { state: at + 1 }, state: "closed", }); }); @@ -377,7 +378,7 @@ describe("confirmed action outcome on the detail", () => { ); expect(settlePullRequestDetailOverride(override, detail, scope)).toBe(override); expect(settlePullRequestDetailOverride(override, { ...detail, isDraft: false }, scope)).toEqual( - { scope, at, autoMergeEnabled: true }, + { scope, at: { autoMergeEnabled: at }, autoMergeEnabled: true }, ); expect( settlePullRequestDetailOverride( @@ -389,6 +390,23 @@ describe("confirmed action outcome on the detail", () => { expect(settlePullRequestDetailOverride(override, detail, "other")).toBeNull(); expect(settlePullRequestDetailOverride(override, null, scope)).toBe(override); }); + + it("expires each field on its own deadline", () => { + const override = recordPullRequestDetailOverride( + record({ isDraft: false }), + scope, + { autoMergeEnabled: true }, + at + 1, + ); + expect(expirePullRequestDetailOverride(override, at)).toEqual({ + scope, + at: { autoMergeEnabled: at + 1 }, + autoMergeEnabled: true, + }); + expect( + expirePullRequestDetailOverride(expirePullRequestDetailOverride(override, at), at + 1), + ).toBeNull(); + }); }); describe("pull request timeline", () => { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 3bea99d8ecea6..68a41b9e645be 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -1412,9 +1412,11 @@ export interface PullRequestDetailPatch { export interface PullRequestDetailOverride extends PullRequestDetailPatch { readonly scope: string; - readonly at: number; + readonly at: Partial>; } +const PATCH_FIELDS = ["state", "isDraft", "autoMergeEnabled"] as const; + export function pullRequestDetailPatchAfterAction( action: PullRequestAction, ): PullRequestDetailPatch | null { @@ -1442,7 +1444,43 @@ export function recordPullRequestDetailOverride( patch: PullRequestDetailPatch, now: number, ): PullRequestDetailOverride { - return { ...(previous?.scope === scope ? previous : {}), ...patch, scope, at: now }; + const base = previous?.scope === scope ? previous : null; + return { + ...base, + ...patch, + scope, + at: { + ...base?.at, + ...Object.fromEntries( + PATCH_FIELDS.filter((f) => patch[f] !== undefined).map((f) => [f, now]), + ), + }, + }; +} + +function retainPullRequestDetailOverride( + override: PullRequestDetailOverride, + keep: (field: (typeof PATCH_FIELDS)[number]) => boolean, +): PullRequestDetailOverride | null { + const held = PATCH_FIELDS.filter((f) => override[f] !== undefined); + const kept = held.filter(keep); + if (kept.length === 0) return null; + if (kept.length === held.length) return override; + return { + scope: override.scope, + ...Object.fromEntries(kept.map((f) => [f, override[f]])), + at: Object.fromEntries(kept.map((f) => [f, override.at[f]])), + }; +} + +/** Drop the fields stamped at `at`, leaving fields recorded by later actions. */ +export function expirePullRequestDetailOverride( + override: PullRequestDetailOverride | null, + at: number, +): PullRequestDetailOverride | null { + return override === null + ? null + : retainPullRequestDetailOverride(override, (f) => override.at[f] !== at); } type OverridableDetail = Pick; @@ -1477,18 +1515,5 @@ export function settlePullRequestDetailOverride( autoMergeEnabled: autoMergeEnabled !== undefined && detail.autoMergeEnabled !== autoMergeEnabled, }; - if (!Object.values(open).some(Boolean)) return null; - if ( - open.state === (state !== undefined) && - open.isDraft === (isDraft !== undefined) && - open.autoMergeEnabled === (autoMergeEnabled !== undefined) - ) - return override; - return { - scope: override.scope, - at: override.at, - ...(open.state ? { state } : {}), - ...(open.isDraft ? { isDraft } : {}), - ...(open.autoMergeEnabled ? { autoMergeEnabled } : {}), - }; + return retainPullRequestDetailOverride(override, (f) => open[f]); } diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.ts b/apps/web/src/components/pullRequest/usePullRequestDetail.ts index 49cb55323842a..71d6ac0ad0030 100644 --- a/apps/web/src/components/pullRequest/usePullRequestDetail.ts +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.ts @@ -19,6 +19,7 @@ import { } from "~/state/pullRequests"; import { applyPullRequestDetailOverride, + expirePullRequestDetailOverride, readPullRequestDetailSnapshot, recordPullRequestDetailOverride, resolveDisplayedPullRequestDetail, @@ -185,7 +186,7 @@ export function usePullRequestDetail( setOverride((previous) => recordPullRequestDetailOverride(previous, scopeKey, patch, at)); // A fetch that never agrees must not pin the confirmed outcome forever. window.setTimeout( - () => setOverride((previous) => (previous?.at === at ? null : previous)), + () => setOverride((previous) => expirePullRequestDetailOverride(previous, at)), PULL_REQUEST_OVERRIDE_TRUST_MS, ); };