| author | |
| committer | |
| log | 7ffa2184a7cfc78d870bf6cbfa4064c51868aef2 |
| tree | b61f3beffe7701b9a453a5ecbed84d0722c4ed8b |
| parent | 7812230572f2d7e042d8d14ada2381fc71377560 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>13 files changed, 550 insertions(+), 67 deletions(-)
apps/server/src/pullRequest/PullRequestService.test.ts+68| ... | @@ -2,9 +2,11 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; | ... | @@ -2,9 +2,11 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; |
| 2 | import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; | 2 | import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; |
| 3 | import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore"; | 3 | import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore"; |
| 4 | import { assert, it } from "@effect/vitest"; | 4 | import { assert, it } from "@effect/vitest"; |
| 5 | import * as Cause from "effect/Cause"; | ||
| 5 | import * as Clock from "effect/Clock"; | 6 | import * as Clock from "effect/Clock"; |
| 6 | import * as Deferred from "effect/Deferred"; | 7 | import * as Deferred from "effect/Deferred"; |
| 7 | import * as Effect from "effect/Effect"; | 8 | import * as Effect from "effect/Effect"; |
| 9 | import * as Exit from "effect/Exit"; | ||
| 8 | import * as Fiber from "effect/Fiber"; | 10 | import * as Fiber from "effect/Fiber"; |
| 9 | import * as Layer from "effect/Layer"; | 11 | import * as Layer from "effect/Layer"; |
| 10 | import * as Option from "effect/Option"; | 12 | import * as Option from "effect/Option"; |
| ... | @@ -3348,6 +3350,72 @@ it.effect("answers a repeated listing from cache, and concurrent readers share o | ... | @@ -3348,6 +3350,72 @@ it.effect("answers a repeated listing from cache, and concurrent readers share o |
| 3348 | }), | 3350 | }), |
| 3349 | ); | 3351 | ); |
| 3350 | 3352 | ||
| 3353 | it.effect.each(["list", "listStats"] as const)( | ||
| 3354 | "retries a replacement %s read that joins a canceled lookup", | ||
| 3355 | (operation) => | ||
| 3356 | Effect.gen(function* () { | ||
| 3357 | const started = yield* Deferred.make<void>(); | ||
| 3358 | const stopping = yield* Deferred.make<void>(); | ||
| 3359 | const finishShutdown = yield* Deferred.make<void>(); | ||
| 3360 | let hostCalls = 0; | ||
| 3361 | const lookup = Effect.gen(function* () { | ||
| 3362 | if (++hostCalls !== 1) return; | ||
| 3363 | yield* Deferred.succeed(started, undefined); | ||
| 3364 | return yield* Effect.never.pipe( | ||
| 3365 | Effect.onInterrupt(() => | ||
| 3366 | Deferred.succeed(stopping, undefined).pipe( | ||
| 3367 | Effect.andThen(Deferred.await(finishShutdown)), | ||
| 3368 | ), | ||
| 3369 | ), | ||
| 3370 | ); | ||
| 3371 | }); | ||
| 3372 | const service = yield* makeService({ | ||
| 3373 | projects: [ | ||
| 3374 | project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), | ||
| 3375 | ], | ||
| 3376 | providers: [ | ||
| 3377 | fakeProvider("github", { | ||
| 3378 | listChangeRequests: () => | ||
| 3379 | lookup.pipe( | ||
| 3380 | Effect.as({ | ||
| 3381 | items: [changeRequest(1, "2026-07-02T00:00:00Z")], | ||
| 3382 | truncated: false, | ||
| 3383 | continues: false, | ||
| 3384 | }), | ||
| 3385 | ), | ||
| 3386 | listChangeRequestStats: () => | ||
| 3387 | lookup.pipe( | ||
| 3388 | Effect.as([{ repository: "acme/web", number: 1, additions: 3, deletions: 1 }]), | ||
| 3389 | ), | ||
| 3390 | }), | ||
| 3391 | ], | ||
| 3392 | }); | ||
| 3393 | const read = | ||
| 3394 | operation === "list" | ||
| 3395 | ? service | ||
| 3396 | .list({ state: "open" }) | ||
| 3397 | .pipe(Effect.map((result) => result.entries.map((entry) => entry.number))) | ||
| 3398 | : service | ||
| 3399 | .listStats({ | ||
| 3400 | refs: [{ projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }], | ||
| 3401 | }) | ||
| 3402 | .pipe(Effect.map((result) => result.stats.map((stat) => stat.number))); | ||
| 3403 | const original = yield* Effect.forkScoped(read); | ||
| 3404 | yield* Deferred.await(started); | ||
| 3405 | const cancellation = yield* Effect.forkScoped(Fiber.interrupt(original)); | ||
| 3406 | yield* Deferred.await(stopping); | ||
| 3407 | // Join the old entry while its lookup is still finishing cancellation. | ||
| 3408 | const replacement = yield* Effect.forkScoped(read, { startImmediately: true }); | ||
| 3409 | yield* Deferred.succeed(finishShutdown, undefined); | ||
| 3410 | yield* Fiber.join(cancellation); | ||
| 3411 | const canceled = yield* Fiber.await(original); | ||
| 3412 | assert.isTrue(Exit.isFailure(canceled)); | ||
| 3413 | if (Exit.isFailure(canceled)) assert.isTrue(Cause.hasInterruptsOnly(canceled.cause)); | ||
| 3414 | assert.deepStrictEqual(yield* Fiber.join(replacement), [1]); | ||
| 3415 | assert.strictEqual(hostCalls, 2); | ||
| 3416 | }), | ||
| 3417 | ); | ||
| 3418 | |||
| 3351 | it.effect("shares one cold viewer lookup across distinct concurrent lists", () => | 3419 | it.effect("shares one cold viewer lookup across distinct concurrent lists", () => |
| 3352 | Effect.gen(function* () { | 3420 | Effect.gen(function* () { |
| 3353 | let viewerCalls = 0; | 3421 | let viewerCalls = 0; |
apps/server/src/pullRequest/PullRequestService.ts+12-2| ... | @@ -5,6 +5,7 @@ import { | ... | @@ -5,6 +5,7 @@ import { |
| 5 | } from "@t3tools/shared/sourceControl"; | 5 | } from "@t3tools/shared/sourceControl"; |
| 6 | import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; | 6 | import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; |
| 7 | import * as Cache from "effect/Cache"; | 7 | import * as Cache from "effect/Cache"; |
| 8 | import * as Cause from "effect/Cause"; | ||
| 8 | import * as Clock from "effect/Clock"; | 9 | import * as Clock from "effect/Clock"; |
| 9 | import * as Context from "effect/Context"; | 10 | import * as Context from "effect/Context"; |
| 10 | import * as DateTime from "effect/DateTime"; | 11 | import * as DateTime from "effect/DateTime"; |
| ... | @@ -2887,7 +2888,12 @@ export const make = Effect.gen(function* () { | ... | @@ -2887,7 +2888,12 @@ export const make = Effect.gen(function* () { |
| 2887 | ? null | 2888 | ? null |
| 2888 | : Object.entries(input.cursors).toSorted(([left], [right]) => left.localeCompare(right)), | 2889 | : Object.entries(input.cursors).toSorted(([left], [right]) => left.localeCompare(right)), |
| 2889 | ]); | 2890 | ]); |
| 2890 | return Cache.get(listCache, key); | 2891 | // A replacement reader can join a lookup still finishing its previous reader's cancellation. |
| 2892 | return Cache.get(listCache, key).pipe( | ||
| 2893 | Effect.catchCause((cause) => | ||
| 2894 | Cause.hasInterruptsOnly(cause) ? Cache.get(listCache, key) : Effect.failCause(cause), | ||
| 2895 | ), | ||
| 2896 | ); | ||
| 2891 | }; | 2897 | }; |
| 2892 | 2898 | ||
| 2893 | const checksCache = yield* Cache.makeWith( | 2899 | const checksCache = yield* Cache.makeWith( |
| ... | @@ -3134,7 +3140,11 @@ export const make = Effect.gen(function* () { | ... | @@ -3134,7 +3140,11 @@ export const make = Effect.gen(function* () { |
| 3134 | } | 3140 | } |
| 3135 | if (missing.size === 0) return { stats: held }; | 3141 | if (missing.size === 0) return { stats: held }; |
| 3136 | const key = statsBatchKey(missing.values()); | 3142 | const key = statsBatchKey(missing.values()); |
| 3137 | const { result, at } = yield* Cache.get(listStatsCache, key); | 3143 | const { result, at } = yield* Cache.get(listStatsCache, key).pipe( |
| 3144 | Effect.catchCause((cause) => | ||
| 3145 | Cause.hasInterruptsOnly(cause) ? Cache.get(listStatsCache, key) : Effect.failCause(cause), | ||
| 3146 | ), | ||
| 3147 | ); | ||
| 3138 | for (const [key, ref] of missing) { | 3148 | for (const [key, ref] of missing) { |
| 3139 | const stat = result.stats.find( | 3149 | const stat = result.stats.find( |
| 3140 | (stat) => | 3150 | (stat) => |
apps/web/src/components/Sidebar.pointer.test.ts+4-4| ... | @@ -166,9 +166,9 @@ describe("sidebar pointer lifecycle", () => { | ... | @@ -166,9 +166,9 @@ describe("sidebar pointer lifecycle", () => { |
| 166 | expect(drag.onCancel).not.toHaveBeenCalled(); | 166 | expect(drag.onCancel).not.toHaveBeenCalled(); |
| 167 | }); | 167 | }); |
| 168 | 168 | ||
| 169 | it("suppresses a delayed release click after cancellation, then accepts the next click", () => { | 169 | it.each([false, true])("suppresses a cancelled release click, started=%s", (started) => { |
| 170 | const drag = gesture(); | 170 | const drag = gesture(); |
| 171 | document.dispatchEvent(pointer("pointermove", { clientY: 20 })); | 171 | if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); |
| 172 | drag.sensor.cancel(); | 172 | drag.sensor.cancel(); |
| 173 | vi.advanceTimersByTime(1000); | 173 | vi.advanceTimersByTime(1000); |
| 174 | const releaseClick = new Event("click"); | 174 | const releaseClick = new Event("click"); |
| ... | @@ -182,9 +182,9 @@ describe("sidebar pointer lifecycle", () => { | ... | @@ -182,9 +182,9 @@ describe("sidebar pointer lifecycle", () => { |
| 182 | expect(nextPropagation).not.toHaveBeenCalled(); | 182 | expect(nextPropagation).not.toHaveBeenCalled(); |
| 183 | }); | 183 | }); |
| 184 | 184 | ||
| 185 | it("allows the next click when the cancelled release happened outside the document", () => { | 185 | it.each([false, true])("allows a fresh click after an outside release, started=%s", (started) => { |
| 186 | const drag = gesture(); | 186 | const drag = gesture(); |
| 187 | document.dispatchEvent(pointer("pointermove", { clientY: 20 })); | 187 | if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); |
| 188 | drag.sensor.cancel(); | 188 | drag.sensor.cancel(); |
| 189 | document.dispatchEvent(pointer("pointerdown")); | 189 | document.dispatchEvent(pointer("pointerdown")); |
| 190 | const click = new Event("click"); | 190 | const click = new Event("click"); |
apps/web/src/components/Sidebar.pointer.ts+2-1| ... | @@ -137,7 +137,8 @@ export class SidebarPointerSensor { | ... | @@ -137,7 +137,8 @@ export class SidebarPointerSensor { |
| 137 | // Cancellation can precede release by an arbitrary amount of time. Consume | 137 | // Cancellation can precede release by an arbitrary amount of time. Consume |
| 138 | // that release click, or let a fresh pointerdown end suppression if release | 138 | // that release click, or let a fresh pointerdown end suppression if release |
| 139 | // happened outside the document. Ordinary clicks never install this guard. | 139 | // happened outside the document. Ordinary clicks never install this guard. |
| 140 | if (!aborted) { | 140 | if (!aborted || cancelled) { |
| 141 | this.document.addEventListener("click", this.suppressClick, { capture: true }); | ||
| 141 | this.document.addEventListener("pointerdown", this.clearClickSuppression, { capture: true }); | 142 | this.document.addEventListener("pointerdown", this.clearClickSuppression, { capture: true }); |
| 142 | } | 143 | } |
| 143 | try { | 144 | try { |
apps/web/src/components/pullRequest/PullRequestRow.tsx+15-1| ... | @@ -91,6 +91,9 @@ function PullRequestRowImpl({ | ... | @@ -91,6 +91,9 @@ function PullRequestRowImpl({ |
| 91 | onSelect, | 91 | onSelect, |
| 92 | speedMode, | 92 | speedMode, |
| 93 | onActed, | 93 | onActed, |
| 94 | closing = false, | ||
| 95 | sweeping = false, | ||
| 96 | onCloseSweepStart, | ||
| 94 | }: { | 97 | }: { |
| 95 | entry: EnvironmentPullRequestEntry; | 98 | entry: EnvironmentPullRequestEntry; |
| 96 | selected: boolean; | 99 | selected: boolean; |
| ... | @@ -110,12 +113,16 @@ function PullRequestRowImpl({ | ... | @@ -110,12 +113,16 @@ function PullRequestRowImpl({ |
| 110 | onSelect: (entry: PullRequestRowTarget) => void; | 113 | onSelect: (entry: PullRequestRowTarget) => void; |
| 111 | speedMode: boolean; | 114 | speedMode: boolean; |
| 112 | onActed: (result: PullRequestSpeedActionResult) => void; | 115 | onActed: (result: PullRequestSpeedActionResult) => void; |
| 116 | closing?: boolean; | ||
| 117 | sweeping?: boolean; | ||
| 118 | onCloseSweepStart?: (entry: EnvironmentPullRequestEntry, event: PointerEvent) => void; | ||
| 113 | }) { | 119 | }) { |
| 114 | const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider); | 120 | const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider); |
| 115 | return ( | 121 | return ( |
| 116 | <div | 122 | <div |
| 117 | ref={statsRef} | 123 | ref={statsRef} |
| 118 | data-pull-request-stats-key={statsKey} | 124 | data-pull-request-stats-key={statsKey} |
| 125 | data-pull-request-key={statsKey} | ||
| 119 | className={cn( | 126 | className={cn( |
| 120 | // Keep the action buttons inside the same skipped row, but outside its selection button. | 127 | // Keep the action buttons inside the same skipped row, but outside its selection button. |
| 121 | "flex items-center rounded-md [content-visibility:auto] [contain-intrinsic-block-size:56.5px]", | 128 | "flex items-center rounded-md [content-visibility:auto] [contain-intrinsic-block-size:56.5px]", |
| ... | @@ -249,7 +256,14 @@ function PullRequestRowImpl({ | ... | @@ -249,7 +256,14 @@ function PullRequestRowImpl({ |
| 249 | /> | 256 | /> |
| 250 | </button> | 257 | </button> |
| 251 | {entry.state !== "merged" && entry.provider === "github" ? ( | 258 | {entry.state !== "merged" && entry.provider === "github" ? ( |
| 252 | <PullRequestSpeedActions entry={entry} visible={speedMode} onActed={onActed} /> | 259 | <PullRequestSpeedActions |
| 260 | entry={entry} | ||
| 261 | visible={speedMode} | ||
| 262 | onActed={onActed} | ||
| 263 | closing={closing} | ||
| 264 | sweeping={sweeping} | ||
| 265 | {...(onCloseSweepStart ? { onCloseSweepStart } : {})} | ||
| 266 | /> | ||
| 253 | ) : null} | 267 | ) : null} |
| 254 | </div> | 268 | </div> |
| 255 | ); | 269 | ); |
apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx+20-34| ... | @@ -1,8 +1,4 @@ | ... | @@ -1,8 +1,4 @@ |
| 1 | import type { PullRequestAction } from "@t3tools/contracts"; | 1 | import type { PullRequestAction } from "@t3tools/contracts"; |
| 2 | import { Effect } from "effect"; | ||
| 3 | import { AtomRegistry } from "effect/unstable/reactivity"; | ||
| 4 | import { appAtomRegistry } from "~/rpc/atomRegistry"; | ||
| 5 | import { pullRequestEnvironment, pullRequestStackAtom } from "~/state/pullRequests"; | ||
| 6 | import { useUiStateStore } from "~/uiStateStore"; | 2 | import { useUiStateStore } from "~/uiStateStore"; |
| 7 | import { Button } from "../ui/button"; | 3 | import { Button } from "../ui/button"; |
| 8 | import { Spinner } from "../ui/spinner"; | 4 | import { Spinner } from "../ui/spinner"; |
| ... | @@ -25,10 +21,16 @@ export function PullRequestSpeedActions({ | ... | @@ -25,10 +21,16 @@ export function PullRequestSpeedActions({ |
| 25 | entry, | 21 | entry, |
| 26 | visible, | 22 | visible, |
| 27 | onActed, | 23 | onActed, |
| 24 | closing = false, | ||
| 25 | sweeping = false, | ||
| 26 | onCloseSweepStart, | ||
| 28 | }: { | 27 | }: { |
| 29 | entry: EnvironmentPullRequestEntry; | 28 | entry: EnvironmentPullRequestEntry; |
| 30 | visible: boolean; | 29 | visible: boolean; |
| 31 | onActed: (result: PullRequestSpeedActionResult) => void; | 30 | onActed: (result: PullRequestSpeedActionResult) => void; |
| 31 | closing?: boolean; | ||
| 32 | sweeping?: boolean; | ||
| 33 | onCloseSweepStart?: (entry: EnvironmentPullRequestEntry, event: PointerEvent) => void; | ||
| 32 | }) { | 34 | }) { |
| 33 | const resolveProjectDefault = usePullRequestDefaultMergeMethodResolver( | 35 | const resolveProjectDefault = usePullRequestDefaultMergeMethodResolver( |
| 34 | entry.environmentId, | 36 | entry.environmentId, |
| ... | @@ -44,32 +46,7 @@ export function PullRequestSpeedActions({ | ... | @@ -44,32 +46,7 @@ export function PullRequestSpeedActions({ |
| 44 | environmentId: entry.environmentId, | 46 | environmentId: entry.environmentId, |
| 45 | reference, | 47 | reference, |
| 46 | onSuccess: (action) => onActed({ entry, action }), | 48 | onSuccess: (action) => onActed({ entry, action }), |
| 47 | resolveMergeMethod: async () => { | 49 | resolveMergeMethod: (detail) => { |
| 48 | const target = { environmentId: entry.environmentId, input: reference }; | ||
| 49 | const detailAtom = pullRequestEnvironment.detail({ | ||
| 50 | ...target, | ||
| 51 | input: { ...reference, allowStale: false }, | ||
| 52 | }); | ||
| 53 | appAtomRegistry.refresh(detailAtom); | ||
| 54 | const detail = await Effect.runPromise( | ||
| 55 | AtomRegistry.getResult(appAtomRegistry, detailAtom, { suspendOnWaiting: true }), | ||
| 56 | ); | ||
| 57 | if ( | ||
| 58 | detail.state !== "open" || | ||
| 59 | detail.isDraft || | ||
| 60 | !detail.capabilities.actions.includes("merge") || | ||
| 61 | !detail.viewerPermissions.actions.includes("merge") | ||
| 62 | ) { | ||
| 63 | throw new Error("This pull request cannot be merged."); | ||
| 64 | } | ||
| 65 | if (detail.capabilities.stackActions) { | ||
| 66 | const stackAtom = pullRequestStackAtom(target); | ||
| 67 | appAtomRegistry.refresh(stackAtom); | ||
| 68 | const stack = await Effect.runPromise( | ||
| 69 | AtomRegistry.getResult(appAtomRegistry, stackAtom, { suspendOnWaiting: true }), | ||
| 70 | ); | ||
| 71 | if (stack !== null) throw new Error("Open this pull request to merge its stack."); | ||
| 72 | } | ||
| 73 | const allowed = detail.capabilities.mergeMethods.filter( | 50 | const allowed = detail.capabilities.mergeMethods.filter( |
| 74 | (method) => detail.mergeCapabilities[method], | 51 | (method) => detail.mergeCapabilities[method], |
| 75 | ); | 52 | ); |
| ... | @@ -89,12 +66,14 @@ export function PullRequestSpeedActions({ | ... | @@ -89,12 +66,14 @@ export function PullRequestSpeedActions({ |
| 89 | : entry.isDraft | 66 | : entry.isDraft |
| 90 | ? (["close", "ready"] as const) | 67 | ? (["close", "ready"] as const) |
| 91 | : (["close", "merge"] as const); | 68 | : (["close", "merge"] as const); |
| 69 | const busy = actionPending || closing || sweeping; | ||
| 92 | return ( | 70 | return ( |
| 93 | <div | 71 | <div |
| 94 | className="shrink-0 items-center gap-1 pr-3" | 72 | className="shrink-0 items-center gap-1 pr-3" |
| 95 | style={{ display: visible || actionPending ? "flex" : "none" }} | 73 | style={{ display: visible || busy ? "flex" : "none" }} |
| 96 | role="group" | 74 | role="group" |
| 97 | aria-label={`Quick actions for pull request #${entry.number}`} | 75 | aria-label={`Quick actions for pull request #${entry.number}`} |
| 76 | data-pull-request-action-pending={actionPending || closing} | ||
| 98 | > | 77 | > |
| 99 | {actions.map((action) => { | 78 | {actions.map((action) => { |
| 100 | const label = ACTIONS[action].label; | 79 | const label = ACTIONS[action].label; |
| ... | @@ -106,19 +85,26 @@ export function PullRequestSpeedActions({ | ... | @@ -106,19 +85,26 @@ export function PullRequestSpeedActions({ |
| 106 | <Button | 85 | <Button |
| 107 | variant={action === "close" ? "destructive-outline" : "outline"} | 86 | variant={action === "close" ? "destructive-outline" : "outline"} |
| 108 | size="xs" | 87 | size="xs" |
| 109 | disabled={actionPending || (action === "merge" && entry.stack !== undefined)} | 88 | disabled={busy || (action === "merge" && entry.stack !== undefined)} |
| 110 | aria-label={`${label} #${entry.number}`} | 89 | aria-label={`${label} #${entry.number}`} |
| 111 | onClick={() => void perform(action)} | 90 | onClick={() => void perform(action)} |
| 91 | onPointerDown={(event) => { | ||
| 92 | if (action !== "close" || !event.isPrimary || event.button !== 0) return; | ||
| 93 | event.stopPropagation(); | ||
| 94 | onCloseSweepStart?.(entry, event.nativeEvent); | ||
| 95 | }} | ||
| 112 | /> | 96 | /> |
| 113 | } | 97 | } |
| 114 | > | 98 | > |
| 115 | {actionPending ? <Spinner size="xs" /> : <Icon aria-hidden className="size-3" />} | 99 | {busy ? <Spinner size="xs" /> : <Icon aria-hidden className="size-3" />} |
| 116 | {label} | 100 | {label} |
| 117 | </TooltipTrigger> | 101 | </TooltipTrigger> |
| 118 | <TooltipPopup> | 102 | <TooltipPopup> |
| 119 | {action === "merge" && entry.stack | 103 | {action === "merge" && entry.stack |
| 120 | ? "Open this pull request to merge its stack" | 104 | ? "Open this pull request to merge its stack" |
| 121 | : `${label} immediately`} | 105 | : action === "close" |
| 106 | ? "Close immediately, or drag across rows to close several" | ||
| 107 | : `${label} immediately`} | ||
| 122 | </TooltipPopup> | 108 | </TooltipPopup> |
| 123 | </Tooltip> | 109 | </Tooltip> |
| 124 | ); | 110 | ); |
apps/web/src/components/pullRequest/pullRequestList.logic.test.ts+25| ... | @@ -1650,6 +1650,31 @@ describe("pull request list overrides", () => { | ... | @@ -1650,6 +1650,31 @@ describe("pull request list overrides", () => { |
| 1650 | expect(applyPullRequestOverrides(rows, new Map(), key, "open")).toBe(rows); | 1650 | expect(applyPullRequestOverrides(rows, new Map(), key, "open")).toBe(rows); |
| 1651 | }); | 1651 | }); |
| 1652 | 1652 | ||
| 1653 | it.each([ | ||
| 1654 | ["open", [1]], | ||
| 1655 | ["closed", [2]], | ||
| 1656 | ["merged", [3]], | ||
| 1657 | ["all", [1, 2, 3]], | ||
| 1658 | ] as const)("filters host rows by %s after pending actions settle", (state, expected) => { | ||
| 1659 | const rows = [entry(1, "open"), entry(2, "closed"), entry(3, "merged")]; | ||
| 1660 | expect(applyPullRequestOverrides(rows, new Map(), key, state).map((row) => row.number)).toEqual( | ||
| 1661 | expected, | ||
| 1662 | ); | ||
| 1663 | }); | ||
| 1664 | |||
| 1665 | it("keeps a confirmed close hidden when its override is cleared", () => { | ||
| 1666 | const at = 1_000_000; | ||
| 1667 | const rows = [entry(1, "closed"), entry(2, "open"), entry(3, "merged")]; | ||
| 1668 | const overrides = new Map([ | ||
| 1669 | ["#1", { state: "closed" as const, updatedAt: "2026-07-03T00:00:00Z", token: 1, at }], | ||
| 1670 | ]); | ||
| 1671 | const settled = settlePullRequestOverrides(overrides, rows, key, at + 5_000); | ||
| 1672 | expect(settled.size).toBe(0); | ||
| 1673 | expect(applyPullRequestOverrides(rows, settled, key, "open").map((row) => row.number)).toEqual([ | ||
| 1674 | 2, | ||
| 1675 | ]); | ||
| 1676 | }); | ||
| 1677 | |||
| 1653 | it("hands back the held object for a row a refresh did not change", () => { | 1678 | it("hands back the held object for a row a refresh did not change", () => { |
| 1654 | const previous = [entry(1, "open"), entry(2, "open")]; | 1679 | const previous = [entry(1, "open"), entry(2, "open")]; |
| 1655 | const next = [{ ...entry(1, "open") }, { ...entry(2, "open"), state: "merged" as const }]; | 1680 | const next = [{ ...entry(1, "open") }, { ...entry(2, "open"), state: "merged" as const }]; |
apps/web/src/components/pullRequest/pullRequestList.logic.ts+4-8| ... | @@ -1189,18 +1189,14 @@ export function applyPullRequestOverrides<Entry extends PullRequestListEntry>( | ... | @@ -1189,18 +1189,14 @@ export function applyPullRequestOverrides<Entry extends PullRequestListEntry>( |
| 1189 | keyOf: (entry: Entry) => string, | 1189 | keyOf: (entry: Entry) => string, |
| 1190 | state: PullRequestListState, | 1190 | state: PullRequestListState, |
| 1191 | ): ReadonlyArray<Entry> { | 1191 | ): ReadonlyArray<Entry> { |
| 1192 | if (overrides.size === 0) return entries; | 1192 | if (overrides.size === 0 && state === "all") return entries; |
| 1193 | const out: Entry[] = []; | 1193 | const out: Entry[] = []; |
| 1194 | for (const entry of entries) { | 1194 | for (const entry of entries) { |
| 1195 | const override = overrides.get(keyOf(entry)); | 1195 | const override = overrides.get(keyOf(entry)); |
| 1196 | if (override === undefined) { | 1196 | if (state !== "all" && (override?.state ?? entry.state) !== state) continue; |
| 1197 | out.push(entry); | 1197 | out.push(override === undefined ? entry : { ...entry, ...override }); |
| 1198 | continue; | ||
| 1199 | } | ||
| 1200 | if (state !== "all" && override.state !== state) continue; | ||
| 1201 | out.push({ ...entry, ...override }); | ||
| 1202 | } | 1198 | } |
| 1203 | return out; | 1199 | return overrides.size === 0 && out.length === entries.length ? entries : out; |
| 1204 | } | 1200 | } |
| 1205 | 1201 | ||
| 1206 | /** | 1202 | /** |
apps/web/src/components/pullRequest/usePullRequestActions.ts+65-3| ... | @@ -35,6 +35,7 @@ import { useAtomCommand } from "~/state/use-atom-command"; | ... | @@ -35,6 +35,7 @@ import { useAtomCommand } from "~/state/use-atom-command"; |
| 35 | 35 | ||
| 36 | import { toastManager } from "../ui/toast"; | 36 | import { toastManager } from "../ui/toast"; |
| 37 | import { handoffPrompt, handoffReviewComments, readableFailure } from "./pullRequestDetail.logic"; | 37 | import { handoffPrompt, handoffReviewComments, readableFailure } from "./pullRequestDetail.logic"; |
| 38 | import { pullRequestEntryKey, type EnvironmentPullRequestEntry } from "./pullRequestList.logic"; | ||
| 38 | 39 | ||
| 39 | /** Resolve on demand so hidden quick actions do not rebuild the legacy project grouping. */ | 40 | /** Resolve on demand so hidden quick actions do not rebuild the legacy project grouping. */ |
| 40 | export function usePullRequestDefaultMergeMethodResolver( | 41 | export function usePullRequestDefaultMergeMethodResolver( |
| ... | @@ -137,7 +138,7 @@ export function usePullRequestActionRunner({ | ... | @@ -137,7 +138,7 @@ export function usePullRequestActionRunner({ |
| 137 | reference: PullRequestRef | null; | 138 | reference: PullRequestRef | null; |
| 138 | onSuccess?: (action: PullRequestAction) => void; | 139 | onSuccess?: (action: PullRequestAction) => void; |
| 139 | /** Small surfaces resolve repository settings on the click, not for every visible row. */ | 140 | /** Small surfaces resolve repository settings on the click, not for every visible row. */ |
| 140 | resolveMergeMethod?: () => Promise<PullRequestMergeMethod>; | 141 | resolveMergeMethod?: (detail: PullRequestDetail) => PullRequestMergeMethod; |
| 141 | }) { | 142 | }) { |
| 142 | const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); | 143 | const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); |
| 143 | const [actionPending, setActionPending] = useState(false); | 144 | const [actionPending, setActionPending] = useState(false); |
| ... | @@ -148,10 +149,13 @@ export function usePullRequestActionRunner({ | ... | @@ -148,10 +149,13 @@ export function usePullRequestActionRunner({ |
| 148 | pendingRef.current = true; | 149 | pendingRef.current = true; |
| 149 | setActionPending(true); | 150 | setActionPending(true); |
| 150 | try { | 151 | try { |
| 151 | const mergeMethod = method ?? (action === "merge" ? await resolveMergeMethod?.() : undefined); | ||
| 152 | const result = await runAction({ | 152 | const result = await runAction({ |
| 153 | environmentId, | 153 | environmentId, |
| 154 | input: { ...reference, action, ...(mergeMethod ? { mergeMethod } : {}) }, | 154 | input: { |
| 155 | ...reference, | ||
| 156 | action, | ||
| 157 | ...(method ? { mergeMethod: method } : resolveMergeMethod ? { resolveMergeMethod } : {}), | ||
| 158 | }, | ||
| 155 | }); | 159 | }); |
| 156 | if (result._tag === "Failure") throw squashAtomCommandFailure(result); | 160 | if (result._tag === "Failure") throw squashAtomCommandFailure(result); |
| 157 | toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); | 161 | toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); |
| ... | @@ -171,6 +175,64 @@ export function usePullRequestActionRunner({ | ... | @@ -171,6 +175,64 @@ export function usePullRequestActionRunner({ |
| 171 | return { actionPending, perform }; | 175 | return { actionPending, perform }; |
| 172 | } | 176 | } |
| 173 | 177 | ||
| 178 | /** Queue a close sweep through the same environment lanes as individual actions. */ | ||
| 179 | export function usePullRequestCloseBatch(onClosed: (entry: EnvironmentPullRequestEntry) => void) { | ||
| 180 | const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); | ||
| 181 | const pending = useRef(new Set<string>()); | ||
| 182 | const [closingKeys, setClosingKeys] = useState<ReadonlySet<string>>(() => new Set()); | ||
| 183 | const close = useCallback( | ||
| 184 | async (entries: readonly EnvironmentPullRequestEntry[]) => { | ||
| 185 | const batch = entries.filter((entry) => { | ||
| 186 | const key = pullRequestEntryKey(entry); | ||
| 187 | if (entry.state !== "open" || entry.provider !== "github" || pending.current.has(key)) | ||
| 188 | return false; | ||
| 189 | pending.current.add(key); | ||
| 190 | return true; | ||
| 191 | }); | ||
| 192 | if (batch.length === 0) return; | ||
| 193 | setClosingKeys(new Set(pending.current)); | ||
| 194 | let closed = 0; | ||
| 195 | const failures: string[] = []; | ||
| 196 | await Promise.all( | ||
| 197 | batch.map(async (entry) => { | ||
| 198 | try { | ||
| 199 | const result = await runAction({ | ||
| 200 | environmentId: entry.environmentId, | ||
| 201 | input: { | ||
| 202 | projectId: entry.projectId, | ||
| 203 | host: entry.host, | ||
| 204 | repository: entry.repository, | ||
| 205 | number: entry.number, | ||
| 206 | action: "close", | ||
| 207 | }, | ||
| 208 | }); | ||
| 209 | if (result._tag === "Failure") throw squashAtomCommandFailure(result); | ||
| 210 | closed++; | ||
| 211 | onClosed(entry); | ||
| 212 | } catch (failure) { | ||
| 213 | failures.push( | ||
| 214 | `#${entry.number}: ${readableFailure(failure, ACTION_FAILURE_HINTS.close)}`, | ||
| 215 | ); | ||
| 216 | } finally { | ||
| 217 | pending.current.delete(pullRequestEntryKey(entry)); | ||
| 218 | setClosingKeys(new Set(pending.current)); | ||
| 219 | } | ||
| 220 | }), | ||
| 221 | ); | ||
| 222 | toastManager.add({ | ||
| 223 | type: failures.length > 0 ? "error" : "success", | ||
| 224 | title: | ||
| 225 | failures.length > 0 | ||
| 226 | ? `Closed ${closed} of ${batch.length} pull requests` | ||
| 227 | : `Closed ${closed} pull request${closed === 1 ? "" : "s"}`, | ||
| 228 | ...(failures.length > 0 ? { description: failures.slice(0, 3).join("\n") } : {}), | ||
| 229 | }); | ||
| 230 | }, | ||
| 231 | [onClosed, runAction], | ||
| 232 | ); | ||
| 233 | return { close, closingKeys }; | ||
| 234 | } | ||
| 235 | |||
| 174 | export interface PullRequestThreadTask { | 236 | export interface PullRequestThreadTask { |
| 175 | prompt: string; | 237 | prompt: string; |
| 176 | reviewComments?: ReadonlyArray<ReviewCommentContext>; | 238 | reviewComments?: ReadonlyArray<ReviewCommentContext>; |
apps/web/src/routes/_chat.pull-requests.tsx+123-10| ... | @@ -2,6 +2,9 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; | ... | @@ -2,6 +2,9 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; |
| 2 | import { Spinner } from "~/components/ui/spinner"; | 2 | import { Spinner } from "~/components/ui/spinner"; |
| 3 | import { useShortcutModifierState } from "~/shortcutModifierState"; | 3 | import { useShortcutModifierState } from "~/shortcutModifierState"; |
| 4 | import type { PullRequestSpeedActionResult } from "~/components/pullRequest/PullRequestSpeedActions"; | 4 | import type { PullRequestSpeedActionResult } from "~/components/pullRequest/PullRequestSpeedActions"; |
| 5 | import { usePullRequestCloseBatch } from "~/components/pullRequest/usePullRequestActions"; | ||
| 6 | import { SidebarPointerSensor } from "~/components/Sidebar.pointer"; | ||
| 7 | import { resolveSidebarSweepKeys } from "~/components/Sidebar.logic"; | ||
| 5 | import { pullRequestHostOf, resolveEnvironmentMachineKind } from "@t3tools/contracts"; | 8 | import { pullRequestHostOf, resolveEnvironmentMachineKind } from "@t3tools/contracts"; |
| 6 | import type { | 9 | import type { |
| 7 | EnvironmentId, | 10 | EnvironmentId, |
| ... | @@ -793,6 +796,11 @@ function PullRequestsRouteView() { | ... | @@ -793,6 +796,11 @@ function PullRequestsRouteView() { |
| 793 | ], | 796 | ], |
| 794 | ); | 797 | ); |
| 795 | const baselineQuery = usePullRequestList(baselineTargets); | 798 | const baselineQuery = usePullRequestList(baselineTargets); |
| 799 | const baselineEmpty = | ||
| 800 | baselineQuery.data?.entries.length === 0 && | ||
| 801 | baselineQuery.data.errors.length === 0 && | ||
| 802 | !baselineQuery.isPending && | ||
| 803 | baselineQuery.error === null; | ||
| 796 | const facetTargets = useMemo(() => { | 804 | const facetTargets = useMemo(() => { |
| 797 | if (!filtersOpen) return NO_LIST_TARGETS; | 805 | if (!filtersOpen) return NO_LIST_TARGETS; |
| 798 | return environmentQueries.map(({ environmentId, projectIds }) => ({ | 806 | return environmentQueries.map(({ environmentId, projectIds }) => ({ |
| ... | @@ -961,12 +969,14 @@ function PullRequestsRouteView() { | ... | @@ -961,12 +969,14 @@ function PullRequestsRouteView() { |
| 961 | speedActionRef.current = ({ entry, action }) => { | 969 | speedActionRef.current = ({ entry, action }) => { |
| 962 | // Some hosts accept a merge before it completes. Let the next host read declare it merged. | 970 | // Some hosts accept a merge before it completes. Let the next host read declare it merged. |
| 963 | if (action !== "merge") overrideEntry(entry, action); | 971 | if (action !== "merge") overrideEntry(entry, action); |
| 964 | setDetailRefreshToken((token) => token + 1); | ||
| 965 | refreshListAndStats(undefined, entry.environmentId); | ||
| 966 | }; | 972 | }; |
| 967 | const onSpeedAction = useCallback((result: PullRequestSpeedActionResult) => { | 973 | const onSpeedAction = useCallback((result: PullRequestSpeedActionResult) => { |
| 968 | speedActionRef.current(result); | 974 | speedActionRef.current(result); |
| 969 | }, []); | 975 | }, []); |
| 976 | const onBatchClosed = useCallback((entry: EnvironmentPullRequestEntry) => { | ||
| 977 | speedActionRef.current({ entry, action: "close" }); | ||
| 978 | }, []); | ||
| 979 | const { close: closeBatch, closingKeys } = usePullRequestCloseBatch(onBatchClosed); | ||
| 970 | // A reload recreates the registry the queries live in, so with nothing held the page would | 980 | // A reload recreates the registry the queries live in, so with nothing held the page would |
| 971 | // cold-start into skeletons even though almost every row is unchanged. The last answer for | 981 | // cold-start into skeletons even though almost every row is unchanged. The last answer for |
| 972 | // this set of environments is kept across reloads and hydrated here as the carried rows: they | 982 | // this set of environments is kept across reloads and hydrated here as the carried rows: they |
| ... | @@ -1005,13 +1015,15 @@ function PullRequestsRouteView() { | ... | @@ -1005,13 +1015,15 @@ function PullRequestsRouteView() { |
| 1005 | // stay — hydrated or previously answered — rather than being dropped for a feed that | 1015 | // stay — hydrated or previously answered — rather than being dropped for a feed that |
| 1006 | // merely settled first. | 1016 | // merely settled first. |
| 1007 | const partitions = | 1017 | const partitions = |
| 1008 | partitionsWanted && authoredQuery.data !== null && reviewingQuery.data !== null | 1018 | partitionsWanted && baselineEmpty |
| 1009 | ? { authored: authoredQuery.data.entries, reviewing: reviewingQuery.data.entries } | 1019 | ? { authored: [], reviewing: [] } |
| 1010 | : current !== null && | 1020 | : partitionsWanted && authoredQuery.data !== null && reviewingQuery.data !== null |
| 1011 | current.environmentKey === environmentKey && | 1021 | ? { authored: authoredQuery.data.entries, reviewing: reviewingQuery.data.entries } |
| 1012 | current.scope === scopeKey | 1022 | : current !== null && |
| 1013 | ? current.partitions | 1023 | current.environmentKey === environmentKey && |
| 1014 | : undefined; | 1024 | current.scope === scopeKey |
| 1025 | ? current.partitions | ||
| 1026 | : undefined; | ||
| 1015 | // A search's answer is the search's, not the workspace's, so only unsearched lists | 1027 | // A search's answer is the search's, not the workspace's, so only unsearched lists |
| 1016 | // persist. Written here where the held partitions are in reach, so a feed settling | 1028 | // persist. Written here where the held partitions are in reach, so a feed settling |
| 1017 | // ahead of them cannot overwrite a stored snapshot that already had both groups. | 1029 | // ahead of them cannot overwrite a stored snapshot that already had both groups. |
| ... | @@ -1052,6 +1064,7 @@ function PullRequestsRouteView() { | ... | @@ -1052,6 +1064,7 @@ function PullRequestsRouteView() { |
| 1052 | sentQuery, | 1064 | sentQuery, |
| 1053 | listQuery.data, | 1065 | listQuery.data, |
| 1054 | listQuery.isPending, | 1066 | listQuery.isPending, |
| 1067 | baselineEmpty, | ||
| 1055 | partitionsWanted, | 1068 | partitionsWanted, |
| 1056 | authoredQuery.data, | 1069 | authoredQuery.data, |
| 1057 | reviewingQuery.data, | 1070 | reviewingQuery.data, |
| ... | @@ -1343,6 +1356,11 @@ function PullRequestsRouteView() { | ... | @@ -1343,6 +1356,11 @@ function PullRequestsRouteView() { |
| 1343 | */ | 1356 | */ |
| 1344 | const groups = useMemo(() => { | 1357 | const groups = useMemo(() => { |
| 1345 | if (search.involvement !== "all") return [{ key: "others" as const, label: "", entries }]; | 1358 | if (search.involvement !== "all") return [{ key: "others" as const, label: "", entries }]; |
| 1359 | // An empty whole-list answer also empties the priority groups. Their old snapshot must | ||
| 1360 | // not restore the last merged rows after the partition reads are no longer mounted. | ||
| 1361 | if (baselineEmpty) { | ||
| 1362 | return groupPullRequestsByInvolvement(entries, viewers); | ||
| 1363 | } | ||
| 1346 | // Until both partitions have answered, the snapshot's stand in — they are yesterday's | 1364 | // Until both partitions have answered, the snapshot's stand in — they are yesterday's |
| 1347 | // groups, but whole ones, where grouping the feed's first page locally loses every | 1365 | // groups, but whole ones, where grouping the feed's first page locally loses every |
| 1348 | // authored row older than it. Once the live reads land they take over; with neither, | 1366 | // authored row older than it. Once the live reads land they take over; with neither, |
| ... | @@ -1372,6 +1390,7 @@ function PullRequestsRouteView() { | ... | @@ -1372,6 +1390,7 @@ function PullRequestsRouteView() { |
| 1372 | return partitionPullRequestsWithPriority(entries, authored, reviewing); | 1390 | return partitionPullRequestsWithPriority(entries, authored, reviewing); |
| 1373 | }, [ | 1391 | }, [ |
| 1374 | hasLocalFilters, | 1392 | hasLocalFilters, |
| 1393 | baselineEmpty, | ||
| 1375 | localFilters, | 1394 | localFilters, |
| 1376 | authoredQuery.data?.entries, | 1395 | authoredQuery.data?.entries, |
| 1377 | entries, | 1396 | entries, |
| ... | @@ -1573,6 +1592,97 @@ function PullRequestsRouteView() { | ... | @@ -1573,6 +1592,97 @@ function PullRequestsRouteView() { |
| 1573 | ]); | 1592 | ]); |
| 1574 | /** What is actually on screen once the reader's pending answers are on the rows. */ | 1593 | /** What is actually on screen once the reader's pending answers are on the rows. */ |
| 1575 | const shownCount = displayGroups.reduce((count, group) => count + group.entries.length, 0); | 1594 | const shownCount = displayGroups.reduce((count, group) => count + group.entries.length, 0); |
| 1595 | const [closeSweepKeys, setCloseSweepKeys] = useState<ReadonlySet<string>>(() => new Set()); | ||
| 1596 | const closeSensorRef = useRef<SidebarPointerSensor | null>(null); | ||
| 1597 | const closeSweepRows = useMemo( | ||
| 1598 | () => | ||
| 1599 | new Map( | ||
| 1600 | displayGroups.flatMap((group) => | ||
| 1601 | group.entries.map( | ||
| 1602 | (entry) => [pullRequestEntryKey(entry), { entry, groupKey: group.key }] as const, | ||
| 1603 | ), | ||
| 1604 | ), | ||
| 1605 | ), | ||
| 1606 | [displayGroups], | ||
| 1607 | ); | ||
| 1608 | const closeSweepRef = useRef({ displayGroups, closeSweepRows, closingKeys, closeBatch }); | ||
| 1609 | closeSweepRef.current = { displayGroups, closeSweepRows, closingKeys, closeBatch }; | ||
| 1610 | useEffect(() => () => closeSensorRef.current?.cancel(), [filterKey, search.q, sort]); | ||
| 1611 | useEffect(() => { | ||
| 1612 | const onKeyDown = (event: KeyboardEvent) => { | ||
| 1613 | if (event.key !== "Escape" || closeSensorRef.current === null) return; | ||
| 1614 | event.preventDefault(); | ||
| 1615 | closeSensorRef.current.cancel(); | ||
| 1616 | }; | ||
| 1617 | document.addEventListener("keydown", onKeyDown, true); | ||
| 1618 | return () => document.removeEventListener("keydown", onKeyDown, true); | ||
| 1619 | }, []); | ||
| 1620 | const startCloseSweep = useCallback((entry: EnvironmentPullRequestEntry, event: PointerEvent) => { | ||
| 1621 | closeSensorRef.current?.cancel(); | ||
| 1622 | const originKey = pullRequestEntryKey(entry); | ||
| 1623 | const group = closeSweepRef.current.displayGroups.find((candidate) => | ||
| 1624 | candidate.entries.some((row) => pullRequestEntryKey(row) === originKey), | ||
| 1625 | ); | ||
| 1626 | if (!group || closeSweepRef.current.closingKeys.has(originKey)) return; | ||
| 1627 | const orderedKeys = closeSweepRef.current.displayGroups.flatMap((candidate) => | ||
| 1628 | candidate.entries.map(pullRequestEntryKey), | ||
| 1629 | ); | ||
| 1630 | const canClose = (key: string) => { | ||
| 1631 | const row = closeSweepRef.current.closeSweepRows.get(key); | ||
| 1632 | return ( | ||
| 1633 | row?.groupKey === group.key && | ||
| 1634 | row.entry.state === "open" && | ||
| 1635 | row.entry.provider === "github" && | ||
| 1636 | !closeSweepRef.current.closingKeys.has(key) && | ||
| 1637 | !scrollRef.current?.querySelector( | ||
| 1638 | `[data-pull-request-key="${CSS.escape(key)}"] [data-pull-request-action-pending="true"]`, | ||
| 1639 | ) | ||
| 1640 | ); | ||
| 1641 | }; | ||
| 1642 | let sweptKeys: string[] = []; | ||
| 1643 | let targetKey: string | null = null; | ||
| 1644 | const sweepTo = (key: string) => { | ||
| 1645 | if (key === targetKey) return; | ||
| 1646 | targetKey = key; | ||
| 1647 | sweptKeys = resolveSidebarSweepKeys(orderedKeys, originKey, key, canClose); | ||
| 1648 | setCloseSweepKeys(new Set(sweptKeys)); | ||
| 1649 | }; | ||
| 1650 | closeSensorRef.current = new SidebarPointerSensor({ | ||
| 1651 | active: originKey, | ||
| 1652 | event, | ||
| 1653 | options: { | ||
| 1654 | distance: 6, | ||
| 1655 | onAttach: () => {}, | ||
| 1656 | onFinish: () => { | ||
| 1657 | closeSensorRef.current = null; | ||
| 1658 | setCloseSweepKeys(new Set()); | ||
| 1659 | }, | ||
| 1660 | }, | ||
| 1661 | onPending: () => {}, | ||
| 1662 | onStart: () => sweepTo(originKey), | ||
| 1663 | onMove: ({ y }) => { | ||
| 1664 | const viewport = scrollRef.current; | ||
| 1665 | if (!viewport) return; | ||
| 1666 | const bounds = viewport.getBoundingClientRect(); | ||
| 1667 | const visibleY = Math.min(Math.max(y, bounds.top), bounds.bottom - 1); | ||
| 1668 | let key: string | null = null; | ||
| 1669 | for (const row of viewport.querySelectorAll<HTMLElement>("[data-pull-request-key]")) { | ||
| 1670 | if (key !== null && row.getBoundingClientRect().top > visibleY) break; | ||
| 1671 | key = row.dataset.pullRequestKey ?? null; | ||
| 1672 | } | ||
| 1673 | if (key !== null) sweepTo(key); | ||
| 1674 | }, | ||
| 1675 | onEnd: () => { | ||
| 1676 | const batch = sweptKeys.filter(canClose).flatMap((key) => { | ||
| 1677 | const row = closeSweepRef.current.closeSweepRows.get(key); | ||
| 1678 | return row ? [row.entry] : []; | ||
| 1679 | }); | ||
| 1680 | void closeSweepRef.current.closeBatch(batch); | ||
| 1681 | }, | ||
| 1682 | onCancel: () => {}, | ||
| 1683 | onAbort: () => {}, | ||
| 1684 | }); | ||
| 1685 | }, []); | ||
| 1576 | const heldPullRequestsBySurface = useMemo( | 1686 | const heldPullRequestsBySurface = useMemo( |
| 1577 | () => | 1687 | () => |
| 1578 | new Map( | 1688 | new Map( |
| ... | @@ -1801,7 +1911,7 @@ function PullRequestsRouteView() { | ... | @@ -1801,7 +1911,7 @@ function PullRequestsRouteView() { |
| 1801 | onLoadMore={loadMore} | 1911 | onLoadMore={loadMore} |
| 1802 | /> | 1912 | /> |
| 1803 | ) : ( | 1913 | ) : ( |
| 1804 | <div className="space-y-3"> | 1914 | <div className={cn("space-y-3", closeSweepKeys.size > 0 && "**:pointer-events-none")}> |
| 1805 | {displayGroups.map((group) => ( | 1915 | {displayGroups.map((group) => ( |
| 1806 | <div key={group.key} className="space-y-0.5"> | 1916 | <div key={group.key} className="space-y-0.5"> |
| 1807 | {group.label ? <PullRequestGroupHeader group={group} /> : null} | 1917 | {group.label ? <PullRequestGroupHeader group={group} /> : null} |
| ... | @@ -1834,6 +1944,9 @@ function PullRequestsRouteView() { | ... | @@ -1834,6 +1944,9 @@ function PullRequestsRouteView() { |
| 1834 | onSelect={selectEntry} | 1944 | onSelect={selectEntry} |
| 1835 | speedMode={speedMode} | 1945 | speedMode={speedMode} |
| 1836 | onActed={onSpeedAction} | 1946 | onActed={onSpeedAction} |
| 1947 | closing={closingKeys.has(entryKey)} | ||
| 1948 | sweeping={closeSweepKeys.has(entryKey)} | ||
| 1949 | onCloseSweepStart={startCloseSweep} | ||
| 1837 | /> | 1950 | /> |
| 1838 | ); | 1951 | ); |
| 1839 | })} | 1952 | })} |
docs/user/source-control.md+4| ... | @@ -113,6 +113,10 @@ Open **Pull requests** to review changes and comments, request reviewers, check | ... | @@ -113,6 +113,10 @@ Open **Pull requests** to review changes and comments, request reviewers, check |
| 113 | or merge. You can edit review titles and descriptions and your own comments where the host allows it. | 113 | or merge. You can edit review titles and descriptions and your own comments where the host allows it. |
| 114 | GitLab calls these merge requests. | 114 | GitLab calls these merge requests. |
| 115 | 115 | ||
| 116 | On web and desktop, hold **Shift** in the GitHub pull request list for quick actions. | ||
| 117 | To close several, press **Close**, drag across the rows in the same group, and release. | ||
| 118 | Press **Escape** before releasing to cancel. Failed closes stay in the list so you can retry them. | ||
| 119 | |||
| 116 | GitHub, GitLab, and Azure DevOps support auto-merge while checks are outstanding. GitHub also | 120 | GitHub, GitLab, and Azure DevOps support auto-merge while checks are outstanding. GitHub also |
| 117 | supports approving waiting fork workflows and opening a revert pull request for a merged change. | 121 | supports approving waiting fork workflows and opening a revert pull request for a merged change. |
| 118 | 122 |
packages/client-runtime/src/state/pullRequests.test.ts+153| ... | @@ -891,6 +891,159 @@ it.effect("keeps concurrent diff file reads on different hosts separate", () => | ... | @@ -891,6 +891,159 @@ it.effect("keeps concurrent diff file reads on different hosts separate", () => |
| 891 | ), | 891 | ), |
| 892 | ); | 892 | ); |
| 893 | 893 | ||
| 894 | it.effect("queues merge preparation with actions without restarting it on refresh", () => | ||
| 895 | Effect.scoped( | ||
| 896 | Effect.gen(function* () { | ||
| 897 | const started = yield* Deferred.make<void>(); | ||
| 898 | const release = yield* Deferred.make<void>(); | ||
| 899 | const refreshEvents = yield* PubSub.unbounded<number>(); | ||
| 900 | const calls: string[] = []; | ||
| 901 | const client = { | ||
| 902 | [WS_METHODS.pullRequestsSubscribeRefreshes]: () => Stream.fromPubSub(refreshEvents), | ||
| 903 | [WS_METHODS.pullRequestsDetail]: (input: { number: number; allowStale?: boolean }) => | ||
| 904 | Effect.gen(function* () { | ||
| 905 | expect(input.allowStale).toBe(false); | ||
| 906 | calls.push(`detail:${input.number}`); | ||
| 907 | yield* Deferred.succeed(started, undefined); | ||
| 908 | yield* Deferred.await(release); | ||
| 909 | return { | ||
| 910 | state: "open", | ||
| 911 | isDraft: false, | ||
| 912 | capabilities: { actions: ["merge"], stackActions: true }, | ||
| 913 | viewerPermissions: { actions: ["merge"] }, | ||
| 914 | }; | ||
| 915 | }), | ||
| 916 | [WS_METHODS.pullRequestsStack]: (input: { number: number; allowStale?: boolean }) => | ||
| 917 | Effect.sync(() => { | ||
| 918 | expect(input.allowStale).toBe(false); | ||
| 919 | calls.push(`stack:${input.number}`); | ||
| 920 | return null; | ||
| 921 | }), | ||
| 922 | [WS_METHODS.pullRequestsRunAction]: (input: { | ||
| 923 | action: string; | ||
| 924 | number: number; | ||
| 925 | mergeMethod?: string; | ||
| 926 | }) => | ||
| 927 | Effect.sync(() => { | ||
| 928 | if (input.action === "merge") expect(input.mergeMethod).toBe("squash"); | ||
| 929 | expect(input).not.toHaveProperty("resolveMergeMethod"); | ||
| 930 | calls.push(`${input.action}:${input.number}`); | ||
| 931 | }), | ||
| 932 | } as unknown as WsRpcProtocolClient; | ||
| 933 | const { atoms, registry } = yield* makeTestRuntime(client); | ||
| 934 | const unmount = registry.mount( | ||
| 935 | atoms.refreshes({ environmentId: TARGET.environmentId, input: {} }), | ||
| 936 | ); | ||
| 937 | yield* Effect.addFinalizer(() => Effect.sync(unmount)); | ||
| 938 | const reference = { projectId: ProjectId.make("project-1"), repository: "acme/web" }; | ||
| 939 | const first = atoms.runAction.run(registry, { | ||
| 940 | environmentId: TARGET.environmentId, | ||
| 941 | input: { ...reference, number: 1, action: "merge", resolveMergeMethod: () => "squash" }, | ||
| 942 | }); | ||
| 943 | yield* Deferred.await(started); | ||
| 944 | const second = atoms.runAction.run(registry, { | ||
| 945 | environmentId: TARGET.environmentId, | ||
| 946 | input: { ...reference, number: 2, action: "close" }, | ||
| 947 | }); | ||
| 948 | yield* PubSub.publish(refreshEvents, 1); | ||
| 949 | expect(calls).toEqual(["detail:1"]); | ||
| 950 | yield* Deferred.succeed(release, undefined); | ||
| 951 | const results = yield* Effect.promise(() => Promise.all([first, second])); | ||
| 952 | expect(results.every(AsyncResult.isSuccess)).toBe(true); | ||
| 953 | expect(calls).toEqual(["detail:1", "stack:1", "merge:1", "close:2"]); | ||
| 954 | }), | ||
| 955 | ), | ||
| 956 | ); | ||
| 957 | |||
| 958 | it.effect.each(["closed", "draft", "permission", "stack", "method"] as const)( | ||
| 959 | "rejects an unsafe quick merge with %s and keeps later actions running", | ||
| 960 | (reason) => | ||
| 961 | Effect.scoped( | ||
| 962 | Effect.gen(function* () { | ||
| 963 | const calls: string[] = []; | ||
| 964 | const client = { | ||
| 965 | [WS_METHODS.pullRequestsDetail]: () => | ||
| 966 | Effect.succeed({ | ||
| 967 | state: reason === "closed" ? "closed" : "open", | ||
| 968 | isDraft: reason === "draft", | ||
| 969 | capabilities: { actions: ["merge"], stackActions: true }, | ||
| 970 | viewerPermissions: { actions: reason === "permission" ? [] : ["merge"] }, | ||
| 971 | }), | ||
| 972 | [WS_METHODS.pullRequestsStack]: () => Effect.succeed(reason === "stack" ? {} : null), | ||
| 973 | [WS_METHODS.pullRequestsRunAction]: (input: { action: string }) => | ||
| 974 | Effect.sync(() => calls.push(input.action)), | ||
| 975 | } as unknown as WsRpcProtocolClient; | ||
| 976 | const { atoms, registry } = yield* makeTestRuntime(client); | ||
| 977 | const target = { | ||
| 978 | environmentId: TARGET.environmentId, | ||
| 979 | input: { projectId: ProjectId.make("project-1"), repository: "acme/web", number: 1 }, | ||
| 980 | }; | ||
| 981 | const merge = atoms.runAction.run(registry, { | ||
| 982 | ...target, | ||
| 983 | input: { | ||
| 984 | ...target.input, | ||
| 985 | action: "merge", | ||
| 986 | resolveMergeMethod: () => { | ||
| 987 | if (reason === "method") throw new Error("No merge method is available."); | ||
| 988 | return "squash"; | ||
| 989 | }, | ||
| 990 | }, | ||
| 991 | }); | ||
| 992 | const close = atoms.runAction.run(registry, { | ||
| 993 | ...target, | ||
| 994 | input: { ...target.input, action: "close" }, | ||
| 995 | }); | ||
| 996 | const results = yield* Effect.promise(() => Promise.all([merge, close])); | ||
| 997 | expect(results.map((result) => result._tag)).toEqual(["Failure", "Success"]); | ||
| 998 | expect(calls).toEqual(["close"]); | ||
| 999 | }), | ||
| 1000 | ), | ||
| 1001 | ); | ||
| 1002 | |||
| 1003 | it.effect("keeps a close batch ordered and continues after a refused close", () => | ||
| 1004 | Effect.scoped( | ||
| 1005 | Effect.gen(function* () { | ||
| 1006 | const calls: number[] = []; | ||
| 1007 | const started = yield* Deferred.make<void>(); | ||
| 1008 | const release = yield* Deferred.make<void>(); | ||
| 1009 | const client = { | ||
| 1010 | [WS_METHODS.pullRequestsRunAction]: (input: { number: number; action: string }) => | ||
| 1011 | Effect.gen(function* () { | ||
| 1012 | expect(input.action).toBe("close"); | ||
| 1013 | calls.push(input.number); | ||
| 1014 | if (input.number === 6) { | ||
| 1015 | yield* Deferred.succeed(started, undefined); | ||
| 1016 | yield* Deferred.await(release); | ||
| 1017 | } | ||
| 1018 | if (input.number === 5) | ||
| 1019 | return yield* new PullRequestOperationError({ | ||
| 1020 | operation: "runAction", | ||
| 1021 | detail: "You cannot close this pull request.", | ||
| 1022 | }); | ||
| 1023 | }), | ||
| 1024 | } as unknown as WsRpcProtocolClient; | ||
| 1025 | const { atoms, registry } = yield* makeTestRuntime(client); | ||
| 1026 | const batch = [6, 5, 4].map((number) => | ||
| 1027 | atoms.runAction.run(registry, { | ||
| 1028 | environmentId: TARGET.environmentId, | ||
| 1029 | input: { | ||
| 1030 | projectId: ProjectId.make("project-1"), | ||
| 1031 | repository: "acme/web", | ||
| 1032 | number, | ||
| 1033 | action: "close", | ||
| 1034 | }, | ||
| 1035 | }), | ||
| 1036 | ); | ||
| 1037 | yield* Deferred.await(started); | ||
| 1038 | expect(calls).toEqual([6]); | ||
| 1039 | yield* Deferred.succeed(release, undefined); | ||
| 1040 | const results = yield* Effect.promise(() => Promise.all(batch)); | ||
| 1041 | expect(results.map((result) => result._tag)).toEqual(["Success", "Failure", "Success"]); | ||
| 1042 | expect(calls).toEqual([6, 5, 4]); | ||
| 1043 | }), | ||
| 1044 | ), | ||
| 1045 | ); | ||
| 1046 | |||
| 894 | it.effect("keeps hover previews fresh after edits and turns", () => | 1047 | it.effect("keeps hover previews fresh after edits and turns", () => |
| 895 | Effect.scoped( | 1048 | Effect.scoped( |
| 896 | Effect.gen(function* () { | 1049 | Effect.gen(function* () { |
packages/client-runtime/src/state/pullRequests.ts+55-4| ... | @@ -2,9 +2,12 @@ import { | ... | @@ -2,9 +2,12 @@ import { |
| 2 | WS_METHODS, | 2 | WS_METHODS, |
| 3 | type EnvironmentId, | 3 | type EnvironmentId, |
| 4 | type PullRequestActor, | 4 | type PullRequestActor, |
| 5 | type PullRequestActionInput, | ||
| 5 | type PullRequestDetail, | 6 | type PullRequestDetail, |
| 6 | type PullRequestDiffInput, | 7 | type PullRequestDiffInput, |
| 7 | type PullRequestRef, | 8 | type PullRequestRef, |
| 9 | type PullRequestMergeMethod, | ||
| 10 | PullRequestOperationError, | ||
| 8 | type PullRequestSummary, | 11 | type PullRequestSummary, |
| 9 | type VcsStatusResult, | 12 | type VcsStatusResult, |
| 10 | } from "@t3tools/contracts"; | 13 | } from "@t3tools/contracts"; |
| ... | @@ -16,6 +19,7 @@ import { AsyncResult, Atom, AtomRegistry } from "effect/unstable/reactivity"; | ... | @@ -16,6 +19,7 @@ import { AsyncResult, Atom, AtomRegistry } from "effect/unstable/reactivity"; |
| 16 | 19 | ||
| 17 | import { | 20 | import { |
| 18 | createAtomCommandScheduler, | 21 | createAtomCommandScheduler, |
| 22 | createEnvironmentCommand, | ||
| 19 | createEnvironmentRpcCommand, | 23 | createEnvironmentRpcCommand, |
| 20 | createEnvironmentRpcQueryAtomFamily, | 24 | createEnvironmentRpcQueryAtomFamily, |
| 21 | createEnvironmentRpcSubscriptionAtomFamily, | 25 | createEnvironmentRpcSubscriptionAtomFamily, |
| ... | @@ -316,11 +320,58 @@ export function createPullRequestEnvironmentAtoms<R, E>( | ... | @@ -316,11 +320,58 @@ export function createPullRequestEnvironmentAtoms<R, E>( |
| 316 | JSON.stringify([environmentId, input.projectId, input.repository, input.number]), | 320 | JSON.stringify([environmentId, input.projectId, input.repository, input.number]), |
| 317 | }, | 321 | }, |
| 318 | }), | 322 | }), |
| 319 | runAction: createEnvironmentRpcCommand(runtime, { | 323 | runAction: createEnvironmentCommand(runtime, { |
| 320 | label: "environment-data:pull-requests:run-action", | 324 | label: "environment-data:pull-requests:run-action", |
| 321 | tag: WS_METHODS.pullRequestsRunAction, | 325 | // Preparation belongs to the write's lane. Refreshable queries would restart it after |
| 322 | execute: (input) => routedRequest(WS_METHODS.pullRequestsRunAction, input), | 326 | // every preceding action, and preparing outside the lane could reorder the clicks. |
| 323 | onSuccess: (target, registry) => Effect.sync(() => registry.refresh(preview(target))), | 327 | execute: ( |
| 328 | input: PullRequestActionInput & { | ||
| 329 | readonly resolveMergeMethod?: (detail: PullRequestDetail) => PullRequestMergeMethod; | ||
| 330 | }, | ||
| 331 | registry, | ||
| 332 | environmentId, | ||
| 333 | ) => | ||
| 334 | Effect.gen(function* () { | ||
| 335 | const { resolveMergeMethod, ...actionInput } = input; | ||
| 336 | let preparedInput = actionInput; | ||
| 337 | if (actionInput.action === "merge" && resolveMergeMethod !== undefined) { | ||
| 338 | const { projectId, host, repository, number } = actionInput; | ||
| 339 | const reference = { projectId, host, repository, number, allowStale: false }; | ||
| 340 | const detail = yield* routedRequest(WS_METHODS.pullRequestsDetail, reference); | ||
| 341 | if ( | ||
| 342 | detail.state !== "open" || | ||
| 343 | detail.isDraft || | ||
| 344 | !detail.capabilities.actions.includes("merge") || | ||
| 345 | !detail.viewerPermissions.actions.includes("merge") | ||
| 346 | ) { | ||
| 347 | return yield* new PullRequestOperationError({ | ||
| 348 | operation: "runAction", | ||
| 349 | detail: "This pull request cannot be merged.", | ||
| 350 | }); | ||
| 351 | } | ||
| 352 | if (detail.capabilities.stackActions) { | ||
| 353 | const stack = yield* routedRequest(WS_METHODS.pullRequestsStack, reference); | ||
| 354 | if (stack !== null) { | ||
| 355 | return yield* new PullRequestOperationError({ | ||
| 356 | operation: "runAction", | ||
| 357 | detail: "Open this pull request to merge its stack.", | ||
| 358 | }); | ||
| 359 | } | ||
| 360 | } | ||
| 361 | const mergeMethod = yield* Effect.try({ | ||
| 362 | try: () => resolveMergeMethod(detail), | ||
| 363 | catch: (cause) => | ||
| 364 | new PullRequestOperationError({ | ||
| 365 | operation: "runAction", | ||
| 366 | detail: | ||
| 367 | cause instanceof Error ? cause.message : "Could not choose a merge method.", | ||
| 368 | }), | ||
| 369 | }); | ||
| 370 | preparedInput = { ...actionInput, mergeMethod }; | ||
| 371 | } | ||
| 372 | yield* routedRequest(WS_METHODS.pullRequestsRunAction, preparedInput); | ||
| 373 | registry.refresh(preview({ environmentId, input: actionInput })); | ||
| 374 | }), | ||
| 324 | scheduler: commandScheduler, | 375 | scheduler: commandScheduler, |
| 325 | concurrency: serialPerEnvironment, | 376 | concurrency: serialPerEnvironment, |
| 326 | }), | 377 | }), |