diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index d248d51d9855388ca492b6f6ab503ae1919bb76b..14785b3f8e6b8bf57f80d30c43a817ca80167492 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -2,9 +2,11 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore"; import { assert, it } from "@effect/vitest"; +import * as Cause from "effect/Cause"; import * as Clock from "effect/Clock"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -3348,6 +3350,72 @@ it.effect("answers a repeated listing from cache, and concurrent readers share o }), ); +it.effect.each(["list", "listStats"] as const)( + "retries a replacement %s read that joins a canceled lookup", + (operation) => + Effect.gen(function* () { + const started = yield* Deferred.make(); + const stopping = yield* Deferred.make(); + const finishShutdown = yield* Deferred.make(); + let hostCalls = 0; + const lookup = Effect.gen(function* () { + if (++hostCalls !== 1) return; + yield* Deferred.succeed(started, undefined); + return yield* Effect.never.pipe( + Effect.onInterrupt(() => + Deferred.succeed(stopping, undefined).pipe( + Effect.andThen(Deferred.await(finishShutdown)), + ), + ), + ); + }); + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + ], + providers: [ + fakeProvider("github", { + listChangeRequests: () => + lookup.pipe( + Effect.as({ + items: [changeRequest(1, "2026-07-02T00:00:00Z")], + truncated: false, + continues: false, + }), + ), + listChangeRequestStats: () => + lookup.pipe( + Effect.as([{ repository: "acme/web", number: 1, additions: 3, deletions: 1 }]), + ), + }), + ], + }); + const read = + operation === "list" + ? service + .list({ state: "open" }) + .pipe(Effect.map((result) => result.entries.map((entry) => entry.number))) + : service + .listStats({ + refs: [{ projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }], + }) + .pipe(Effect.map((result) => result.stats.map((stat) => stat.number))); + const original = yield* Effect.forkScoped(read); + yield* Deferred.await(started); + const cancellation = yield* Effect.forkScoped(Fiber.interrupt(original)); + yield* Deferred.await(stopping); + // Join the old entry while its lookup is still finishing cancellation. + const replacement = yield* Effect.forkScoped(read, { startImmediately: true }); + yield* Deferred.succeed(finishShutdown, undefined); + yield* Fiber.join(cancellation); + const canceled = yield* Fiber.await(original); + assert.isTrue(Exit.isFailure(canceled)); + if (Exit.isFailure(canceled)) assert.isTrue(Cause.hasInterruptsOnly(canceled.cause)); + assert.deepStrictEqual(yield* Fiber.join(replacement), [1]); + assert.strictEqual(hostCalls, 2); + }), +); + it.effect("shares one cold viewer lookup across distinct concurrent lists", () => Effect.gen(function* () { let viewerCalls = 0; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index afe2d9683cadb5c00ea79baca0aa8719a8cebdb8..a5a1a5b01d624b1a1c5ae8725864b17503d3642a 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -5,6 +5,7 @@ import { } from "@t3tools/shared/sourceControl"; import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; import * as Cache from "effect/Cache"; +import * as Cause from "effect/Cause"; import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; @@ -2887,7 +2888,12 @@ export const make = Effect.gen(function* () { ? null : Object.entries(input.cursors).toSorted(([left], [right]) => left.localeCompare(right)), ]); - return Cache.get(listCache, key); + // A replacement reader can join a lookup still finishing its previous reader's cancellation. + return Cache.get(listCache, key).pipe( + Effect.catchCause((cause) => + Cause.hasInterruptsOnly(cause) ? Cache.get(listCache, key) : Effect.failCause(cause), + ), + ); }; const checksCache = yield* Cache.makeWith( @@ -3134,7 +3140,11 @@ export const make = Effect.gen(function* () { } if (missing.size === 0) return { stats: held }; const key = statsBatchKey(missing.values()); - const { result, at } = yield* Cache.get(listStatsCache, key); + const { result, at } = yield* Cache.get(listStatsCache, key).pipe( + Effect.catchCause((cause) => + Cause.hasInterruptsOnly(cause) ? Cache.get(listStatsCache, key) : Effect.failCause(cause), + ), + ); for (const [key, ref] of missing) { const stat = result.stats.find( (stat) => diff --git a/apps/web/src/components/Sidebar.pointer.test.ts b/apps/web/src/components/Sidebar.pointer.test.ts index 3a54498c230408652844722e78dda7e35a236eee..0c171313612e8452238e1070b7e04cb1871576b0 100644 --- a/apps/web/src/components/Sidebar.pointer.test.ts +++ b/apps/web/src/components/Sidebar.pointer.test.ts @@ -166,9 +166,9 @@ describe("sidebar pointer lifecycle", () => { expect(drag.onCancel).not.toHaveBeenCalled(); }); - it("suppresses a delayed release click after cancellation, then accepts the next click", () => { + it.each([false, true])("suppresses a cancelled release click, started=%s", (started) => { const drag = gesture(); - document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); drag.sensor.cancel(); vi.advanceTimersByTime(1000); const releaseClick = new Event("click"); @@ -182,9 +182,9 @@ describe("sidebar pointer lifecycle", () => { expect(nextPropagation).not.toHaveBeenCalled(); }); - it("allows the next click when the cancelled release happened outside the document", () => { + it.each([false, true])("allows a fresh click after an outside release, started=%s", (started) => { const drag = gesture(); - document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); drag.sensor.cancel(); document.dispatchEvent(pointer("pointerdown")); const click = new Event("click"); diff --git a/apps/web/src/components/Sidebar.pointer.ts b/apps/web/src/components/Sidebar.pointer.ts index 5ae83901ed1e1b2cd3f0f6bd7f1de5422ccf9fed..23f95481485dc104876cc4470987e83b2f837a6c 100644 --- a/apps/web/src/components/Sidebar.pointer.ts +++ b/apps/web/src/components/Sidebar.pointer.ts @@ -137,7 +137,8 @@ export class SidebarPointerSensor { // Cancellation can precede release by an arbitrary amount of time. Consume // that release click, or let a fresh pointerdown end suppression if release // happened outside the document. Ordinary clicks never install this guard. - if (!aborted) { + if (!aborted || cancelled) { + this.document.addEventListener("click", this.suppressClick, { capture: true }); this.document.addEventListener("pointerdown", this.clearClickSuppression, { capture: true }); } try { diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index 1f4de90f55b81f4cf2670d5f2fbc25f5db6108e8..29bfa3702d9d280ba4082e6b2fff2ad88fd35d76 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -91,6 +91,9 @@ function PullRequestRowImpl({ onSelect, speedMode, onActed, + closing = false, + sweeping = false, + onCloseSweepStart, }: { entry: EnvironmentPullRequestEntry; selected: boolean; @@ -110,12 +113,16 @@ function PullRequestRowImpl({ onSelect: (entry: PullRequestRowTarget) => void; speedMode: boolean; onActed: (result: PullRequestSpeedActionResult) => void; + closing?: boolean; + sweeping?: boolean; + onCloseSweepStart?: (entry: EnvironmentPullRequestEntry, event: PointerEvent) => void; }) { const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider); return (
{entry.state !== "merged" && entry.provider === "github" ? ( - + ) : null}
); diff --git a/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx index 613928733d37fed2af96d64d213182cd5ca72e9a..ee1bc5dcc02c8d4c86b53307b2d0b7c925553196 100644 --- a/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx @@ -1,8 +1,4 @@ import type { PullRequestAction } from "@t3tools/contracts"; -import { Effect } from "effect"; -import { AtomRegistry } from "effect/unstable/reactivity"; -import { appAtomRegistry } from "~/rpc/atomRegistry"; -import { pullRequestEnvironment, pullRequestStackAtom } from "~/state/pullRequests"; import { useUiStateStore } from "~/uiStateStore"; import { Button } from "../ui/button"; import { Spinner } from "../ui/spinner"; @@ -25,10 +21,16 @@ export function PullRequestSpeedActions({ entry, visible, onActed, + closing = false, + sweeping = false, + onCloseSweepStart, }: { entry: EnvironmentPullRequestEntry; visible: boolean; onActed: (result: PullRequestSpeedActionResult) => void; + closing?: boolean; + sweeping?: boolean; + onCloseSweepStart?: (entry: EnvironmentPullRequestEntry, event: PointerEvent) => void; }) { const resolveProjectDefault = usePullRequestDefaultMergeMethodResolver( entry.environmentId, @@ -44,32 +46,7 @@ export function PullRequestSpeedActions({ environmentId: entry.environmentId, reference, onSuccess: (action) => onActed({ entry, action }), - resolveMergeMethod: async () => { - const target = { environmentId: entry.environmentId, input: reference }; - const detailAtom = pullRequestEnvironment.detail({ - ...target, - input: { ...reference, allowStale: false }, - }); - appAtomRegistry.refresh(detailAtom); - const detail = await Effect.runPromise( - AtomRegistry.getResult(appAtomRegistry, detailAtom, { suspendOnWaiting: true }), - ); - if ( - detail.state !== "open" || - detail.isDraft || - !detail.capabilities.actions.includes("merge") || - !detail.viewerPermissions.actions.includes("merge") - ) { - throw new Error("This pull request cannot be merged."); - } - if (detail.capabilities.stackActions) { - const stackAtom = pullRequestStackAtom(target); - appAtomRegistry.refresh(stackAtom); - const stack = await Effect.runPromise( - AtomRegistry.getResult(appAtomRegistry, stackAtom, { suspendOnWaiting: true }), - ); - if (stack !== null) throw new Error("Open this pull request to merge its stack."); - } + resolveMergeMethod: (detail) => { const allowed = detail.capabilities.mergeMethods.filter( (method) => detail.mergeCapabilities[method], ); @@ -89,12 +66,14 @@ export function PullRequestSpeedActions({ : entry.isDraft ? (["close", "ready"] as const) : (["close", "merge"] as const); + const busy = actionPending || closing || sweeping; return (
{actions.map((action) => { const label = ACTIONS[action].label; @@ -106,19 +85,26 @@ export function PullRequestSpeedActions({