From 9c3c7eea60641d02728815fb50e2bfe4a9f9dff0 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:44:30 +0100 Subject: [PATCH 01/28] perf(pr): actions, title edits and labels show at once on every host Close, reopen, ready, draft, and auto-merge on/off, title and body edits, and label changes now show in every loaded reader from the click. A host refusal takes the change back, and nothing guessed is saved. Merges wait for the host, because a merge queue can accept without merging. Created with Claude Opus 5.5 in Claude Code via T3 Code. --- .../src/state/pullRequests.test.ts | 478 ++++++++++++++++++ .../client-runtime/src/state/pullRequests.ts | 427 +++++++++++++--- 2 files changed, 827 insertions(+), 78 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index f4571c44b03a7..088c94bab017d 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2287,3 +2287,481 @@ it.effect("publishes the accepted guide run before the follow-up read finishes", }), ), ); + +const listResult = (entries: ReadonlyArray) => ({ + entries, + viewers: {}, + providers: [], + errors: [], + truncated: false, + nextCursors: {}, +}); + +it.effect.each([ + ["close", { state: "closed" }, "open"], + ["reopen", { state: "open" }, "closed"], + ["ready", { isDraft: false }, "open"], + ["draft", { isDraft: true }, "open"], + ["enable-auto-merge", { autoMergeEnabled: true }, "open"], + ["disable-auto-merge", { autoMergeEnabled: false }, "open"], +] as const)("shows %s in loaded readers before the host replies", ([action, shown, from]) => + Effect.scoped( + Effect.gen(function* () { + let reads = 0; + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const before = { + ...actionReference, + provider: "github", + title: "Ship it", + url: actionReply.url, + author: null, + headBranch: "feature", + baseBranch: "main", + state: from, + isDraft: action === "ready", + autoMergeEnabled: action === "disable-auto-merge", + createdAt: "2026-10-27T08:00:00Z", + updatedAt: "2026-10-27T09:00:00Z", + observedAt: 100, + }; + const read = () => + Effect.sync(() => { + reads++; + return before; + }); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: read, + [WS_METHODS.pullRequestsPreview]: read, + [WS_METHODS.pullRequestsSummary]: read, + [WS_METHODS.pullRequestsList]: () => + Effect.sync(() => { + reads++; + return listResult([before]); + }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + return { ...before, ...shown, updatedAt: "2026-10-27T10:00:00Z", observedAt: 200 }; + }), + }); + const { atoms, registry, runtime } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const readers: ReadonlyArray>> = [ + atoms.detail(target), + atoms.preview(target), + createLinkedPullRequestSummaryAtomFamily(runtime, atoms.refreshes)(target), + ]; + const list = atoms.list({ environmentId: target.environmentId, input: { state: from } }); + for (const atom of [...readers, list]) { + registry.mount(atom); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + reads = 0; + + const acting = yield* Effect.promise(() => + atoms.runAction.run(registry, { ...target, input: { ...actionReference, action } }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + + for (const atom of readers.slice(0, action.includes("auto-merge") ? 1 : readers.length)) { + expect(Option.getOrThrow(AsyncResult.value(registry.get(atom)))).toMatchObject(shown); + } + const rows = Option.getOrThrow(AsyncResult.value(registry.get(list))).entries; + if ("state" in shown) expect(rows).toEqual([]); + else if ("isDraft" in shown) expect(rows).toMatchObject([shown]); + else expect(rows).toHaveLength(1); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(acting); + expect(Option.getOrThrow(AsyncResult.value(registry.get(readers[0]!)))).toMatchObject({ + ...shown, + updatedAt: "2026-10-27T10:00:00Z", + }); + // Only a reopen re-reads, for the branch data and permissions its reply lacks. + expect(reads).toBe(action === "reopen" ? 1 : 0); + }), + ), +); + +it.effect("puts readers back when the host refuses an action, and saves nothing it guessed", () => + Effect.scoped( + Effect.gen(function* () { + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const open = { ...actionReference, state: "open", isDraft: false, observedAt: 100 }; + const saved = new Map(); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: () => Effect.succeed(open), + [WS_METHODS.pullRequestsList]: () => Effect.succeed(listResult([open])), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + return yield* new PullRequestOperationError({ + operation: "runAction", + detail: "You cannot close this pull request.", + }); + }), + }); + const { atoms, registry } = yield* makeTestRuntime( + client, + undefined, + false, + {}, + { + load: (_environmentId, key) => Effect.sync(() => Option.fromUndefinedOr(saved.get(key))), + save: (_environmentId, key, payload) => Effect.sync(() => void saved.set(key, payload)), + }, + ); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + const list = atoms.list({ environmentId: target.environmentId, input: { state: "open" } }); + const loaded: ReadonlyArray>> = [ + detail, + list, + ]; + for (const atom of loaded) { + registry.mount(atom); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + + const acting = yield* Effect.promise(() => + atoms.runAction.run(registry, { + ...target, + input: { ...actionReference, action: "close" }, + }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).state).toBe("closed"); + yield* Deferred.succeed(release, undefined); + expect((yield* Fiber.join(acting))._tag).toBe("Failure"); + + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).state).toBe("open"); + expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries).toMatchObject([ + { state: "open" }, + ]); + expect([...saved.values()].some((payload) => payload.includes('"closed"'))).toBe(false); + }), + ), +); + +it.effect( + "keeps an accepted change on screen until the re-read when the reply has no summary", + () => + Effect.scoped( + Effect.gen(function* () { + let state = "open"; + let reads = 0; + const rereadStarted = yield* Deferred.make(); + const releaseReread = yield* Deferred.make(); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: () => + Effect.gen(function* () { + reads++; + const answer = { ...actionReference, state, observedAt: reads }; + if (reads === 2) { + yield* Deferred.succeed(rereadStarted, undefined); + yield* Deferred.await(releaseReread); + } + return answer; + }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.sync(() => { + state = "closed"; + return undefined; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + registry.mount(detail); + yield* Effect.promise(() => executeAtomQuery(registry, detail)); + + yield* Effect.promise(() => + atoms.runAction.run(registry, { + ...target, + input: { ...actionReference, action: "close" }, + }), + ); + yield* Deferred.await(rereadStarted); + + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).state).toBe("closed"); + yield* Deferred.succeed(releaseReread, undefined); + expect( + (yield* AtomRegistry.getResult(registry, detail, { suspendOnWaiting: true })).observedAt, + ).toBe(2); + }), + ), +); + +it.effect("shows a queued close before the action ahead of it answers", () => + Effect.scoped( + Effect.gen(function* () { + const firstSent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const rows = [1, 2].map((number) => ({ + ...actionReference, + number, + state: "open", + isDraft: false, + updatedAt: `2026-10-27T0${number}:00:00Z`, + })); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsList]: () => Effect.succeed(listResult(rows)), + [WS_METHODS.pullRequestsRunAction]: (input: { readonly number: number }) => + Effect.gen(function* () { + if (input.number === 1) { + yield* Deferred.succeed(firstSent, undefined); + yield* Deferred.await(release); + } + return undefined; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const list = atoms.list({ environmentId: TARGET.environmentId, input: { state: "open" } }); + registry.mount(list); + yield* Effect.promise(() => executeAtomQuery(registry, list)); + + const closes = [1, 2].map((number) => + atoms.runAction.run(registry, { + environmentId: TARGET.environmentId, + input: { ...actionReference, number, action: "close" }, + }), + ); + yield* Deferred.await(firstSent); + + expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries).toEqual([]); + yield* Deferred.succeed(release, undefined); + yield* Effect.promise(() => Promise.all(closes)); + }), + ), +); + +it.effect("leaves a merge as it is until the host says what it did", () => + Effect.scoped( + Effect.gen(function* () { + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: () => + Effect.succeed({ ...actionReference, state: "open", observedAt: 100 }), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + // A merge queue takes the merge and merges later. + return { ...actionReply, state: "open", autoMergeEnabled: true }; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + registry.mount(detail); + yield* Effect.promise(() => executeAtomQuery(registry, detail)); + + const acting = yield* Effect.promise(() => + atoms.runAction.run(registry, { + ...target, + input: { ...actionReference, action: "merge" }, + }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).state).toBe("open"); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(acting); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject({ + state: "open", + autoMergeEnabled: true, + }); + }), + ), +); + +it.effect.each(["accepted", "refused"] as const)( + "shows a title edit at once and keeps it only when the host takes it (%s)", + (outcome) => + Effect.scoped( + Effect.gen(function* () { + let title = "Original"; + let reads = 0; + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const read = () => + Effect.sync(() => { + reads++; + return { ...actionReference, state: "open", title, observedAt: reads }; + }); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: read, + [WS_METHODS.pullRequestsPreview]: read, + [WS_METHODS.pullRequestsList]: () => + Effect.sync(() => { + reads++; + return listResult([ + { ...actionReference, state: "open", title, updatedAt: actionReply.updatedAt }, + ]); + }), + [WS_METHODS.pullRequestsUpdate]: (input: { readonly title: string }) => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + if (outcome === "refused") return yield* new MutationRefused(); + title = input.title; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + const preview = atoms.preview(target); + const list = atoms.list({ environmentId: target.environmentId, input: { state: "open" } }); + const loaded: ReadonlyArray>> = [ + detail, + preview, + list, + ]; + for (const atom of loaded) { + registry.mount(atom); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + const titles = () => [ + Option.getOrThrow(AsyncResult.value(registry.get(detail))).title, + Option.getOrThrow(AsyncResult.value(registry.get(preview))).title, + Option.getOrThrow(AsyncResult.value(registry.get(list))).entries[0]?.title, + ]; + + const editing = yield* Effect.promise(() => + atoms.update.run(registry, { ...target, input: { ...actionReference, title: "Edited" } }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + expect(titles()).toEqual(["Edited", "Edited", "Edited"]); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(editing); + + expect(titles()).toEqual( + outcome === "refused" + ? ["Original", "Original", "Original"] + : ["Edited", "Edited", "Edited"], + ); + // The edit is known, so no reader asks the host again. + expect(reads).toBe(3); + }), + ), +); + +it.effect("shows a label change at once and takes it back when the host refuses it", () => + Effect.scoped( + Effect.gen(function* () { + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const existing = { name: "existing", color: "111111" }; + const added = { name: "new", color: "abcdef" }; + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: () => + Effect.succeed({ ...actionReference, state: "open", labels: [existing] }), + [WS_METHODS.pullRequestsLabelCandidates]: () => + Effect.succeed({ + candidates: [ + { ...existing, description: null, isApplied: true }, + { ...added, description: null, isApplied: false }, + ], + truncated: false, + }), + [WS_METHODS.pullRequestsSetLabels]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + return yield* new MutationRefused(); + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + const candidates = atoms.labelCandidates(target); + const loaded: ReadonlyArray>> = [ + detail, + candidates, + ]; + for (const atom of loaded) { + registry.mount(atom); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + + const labelling = yield* Effect.promise(() => + atoms.setLabels.run(registry, { + ...target, + input: { ...actionReference, labels: ["new"], applied: true }, + }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).labels).toEqual([ + existing, + added, + ]); + expect( + Option.getOrThrow(AsyncResult.value(registry.get(candidates))).candidates.map( + (candidate) => candidate.isApplied, + ), + ).toEqual([true, true]); + yield* Deferred.succeed(release, undefined); + expect((yield* Fiber.join(labelling))._tag).toBe("Failure"); + + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).labels).toEqual([existing]); + expect( + Option.getOrThrow(AsyncResult.value(registry.get(candidates))).candidates.map( + (candidate) => candidate.isApplied, + ), + ).toEqual([true, false]); + }), + ), +); + +it.effect("keeps an accepted label change when a candidates read sent before it answers late", () => + Effect.scoped( + Effect.gen(function* () { + let applied = false; + let reads = 0; + const readStarted = yield* Deferred.make(); + const releaseRead = yield* Deferred.make(); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsLabelCandidates]: () => + Effect.gen(function* () { + reads++; + const answer = { + candidates: [{ name: "new", color: null, description: null, isApplied: applied }], + truncated: false, + }; + if (reads === 2) { + yield* Deferred.succeed(readStarted, undefined); + yield* Deferred.await(releaseRead); + } + return answer; + }), + [WS_METHODS.pullRequestsSetLabels]: () => + Effect.sync(() => { + applied = true; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const candidates = atoms.labelCandidates(target); + registry.mount(candidates); + yield* Effect.promise(() => executeAtomQuery(registry, candidates)); + registry.refresh(candidates); + yield* Deferred.await(readStarted); + + yield* Effect.promise(() => + atoms.setLabels.run(registry, { + ...target, + input: { ...actionReference, labels: ["new"], applied: true }, + }), + ); + yield* Deferred.succeed(releaseRead, undefined); + + expect( + (yield* AtomRegistry.getResult(registry, candidates, { suspendOnWaiting: true })) + .candidates[0]?.isApplied, + ).toBe(true); + }), + ), +); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index e0ded133128a6..22208487ec82b 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -16,11 +16,15 @@ import { type PullRequestListInput, type PullRequestListStatsInput, type PullRequestListStatsResult, + type PullRequestUpdateInput, type PullRequestDetail, type PullRequestDiffInput, type PullRequestListEntry, type PullRequestRef, type PullRequestMergeMethod, + type PullRequestLabel, + type PullRequestLabelCandidateList, + type PullRequestState, PullRequestOperationError, PullRequestSummary, type VcsStatusResult, @@ -34,6 +38,7 @@ import * as SubscriptionRef from "effect/SubscriptionRef"; import { AsyncResult, Atom, AtomRegistry } from "effect/reactivity"; import { + type AtomCommand, createAtomCommandScheduler, createEnvironmentRpcCommand, createEnvironmentRpcQueryAtomFamily, @@ -119,10 +124,11 @@ function writableQueryFamily( /** Restart pre-mutation reads before patching so they cannot restore stale values. */ function updateCached( registry: AtomRegistry.AtomRegistry, - atom: Atom.Writable>, + shown: Atom.Writable>, update: (value: A) => A, refresh = false, ) { + const atom = (uneditedReaders.get(shown) ?? shown) as typeof shown; if (refresh || registry.get(atom).waiting) registry.refresh(atom); registry.update(atom, AsyncResult.map(update)); } @@ -142,6 +148,198 @@ interface ActionReplyPatch { // Module-wide so readers built by separate factories, such as linked summaries, are patched too. const actionReplyPatches = new WeakMap(); +/** What a write in flight will change, shown by every reader until the host answers. */ +interface PullRequestPendingEdit { + readonly reference: PullRequestRef; + readonly state?: PullRequestState; + readonly isDraft?: boolean; + readonly autoMergeEnabled?: boolean; + readonly title?: string; + readonly body?: string; + readonly labels?: { + readonly applied: boolean; + readonly labels: ReadonlyArray; + }; +} + +type EditedField = "state" | "isDraft" | "autoMergeEnabled" | "title" | "body"; + +interface EditableReader { + readonly environmentId: EnvironmentId; + readonly concerns: (reference: PullRequestRef) => boolean; + readonly edit: (value: never, edit: PullRequestPendingEdit) => unknown; +} + +// Module-wide like the reply patches, so separately built readers see the same writes. +const pendingEdits = Atom.family((_environmentId: EnvironmentId) => + Atom.make>([]).pipe(Atom.keepAlive), +); +const editableReaders = new WeakMap(); +const editedReaders = new WeakMap(); +const uneditedReaders = new WeakMap(); + +function loadedValue(registry: AtomRegistry.AtomRegistry, atom: Atom.Atom) { + return Option.flatMap(registry.getNodes().get(atom)?.valueOption() ?? Option.none(), (result) => + AsyncResult.value(result as AsyncResult.AsyncResult), + ); +} + +function withEdit( + value: T, + edit: PullRequestPendingEdit, + fields: ReadonlyArray, +): T { + const held = value as Record; + const changed = fields.filter( + (field) => edit[field] !== undefined && held[field] !== edit[field], + ); + let next = + changed.length === 0 + ? held + : { ...held, ...Object.fromEntries(changed.map((field) => [field, edit[field]])) }; + if (edit.autoMergeEnabled === false && next.autoMergeMethod !== undefined) { + const { autoMergeMethod: _disarmed, ...withoutAutoMerge } = next; + next = withoutAutoMerge; + } + return next as T; +} + +function withLabelEdit }>( + value: T, + edit: PullRequestPendingEdit, +): T { + if (edit.labels === undefined) return value; + const { applied, labels: changed } = edit.labels; + const names = new Set(changed.map((label) => label.name)); + const labels = applied + ? [ + ...value.labels, + ...changed.filter((label) => !value.labels.some((held) => held.name === label.name)), + ] + : value.labels.filter((label) => !names.has(label.name)); + return labels.length === value.labels.length ? value : { ...value, labels }; +} + +/** + * The reader with every write in flight for its pull request applied. Writes go to the reader + * underneath, so a refused edit never stays behind in what the host said. + */ +function editedQuery( + inner: Atom.Writable>, + reader: Omit & { + readonly edit: (value: A, edit: PullRequestPendingEdit) => A; + }, + idleTtlMs = 5 * 60_000, +): Atom.Writable> { + const existing = editedReaders.get(inner); + if (existing !== undefined) return existing as Atom.Writable>; + const shown = Atom.writable( + (get) => { + const result = get(inner); + const edits = get(pendingEdits(reader.environmentId)); + const value = AsyncResult.value(result); + if (edits.length === 0 || Option.isNone(value)) return result; + const next = edits.reduce( + (current, edit) => (reader.concerns(edit.reference) ? reader.edit(current, edit) : current), + value.value, + ); + return next === value.value ? result : AsyncResult.map(result, () => next); + }, + (context, value: AsyncResult.AsyncResult) => context.set(inner, value), + (refresh) => refresh(inner), + ).pipe(Atom.setIdleTTL(idleTtlMs)); + editableReaders.set(inner, reader as EditableReader); + editedReaders.set(inner, shown); + uneditedReaders.set(shown, inner); + return shown; +} + +/** Shows the command's edit from the click until it settles, queued behind others or not. */ +function showWhileInFlight( + command: C & AtomCommand, + editOf: (registry: AtomRegistry.AtomRegistry, target: Target) => PullRequestPendingEdit | null, +): C { + return { + ...command, + run: async (registry, target) => { + const edit = editOf(registry, target); + if (edit === null) return command.run(registry, target); + const edits = pendingEdits(target.environmentId); + registry.update(edits, (current) => [...current, edit]); + try { + return await command.run(registry, target); + } finally { + registry.update(edits, (current) => current.filter((entry) => entry !== edit)); + } + }, + } satisfies AtomCommand; +} + +/** Keeps an accepted edit in loaded readers once it stops being in flight. */ +function settleEdit( + registry: AtomRegistry.AtomRegistry, + environmentId: EnvironmentId, + edit: PullRequestPendingEdit | null, + readsAfterWrite = false, +) { + if (edit === null) return; + for (const atom of registry.getNodes().keys()) { + const reader = typeof atom === "string" ? undefined : editableReaders.get(atom); + if (reader?.environmentId !== environmentId || !reader.concerns(edit.reference)) continue; + const shown = loadedValue(registry, atom as Atom.Atom); + if (Option.isNone(shown) || reader.edit(shown.value, edit) === shown.value) continue; + const writable = atom as Atom.Writable>; + // A read still waiting may predate the write, unless the caller started it after the commit. + if (!readsAfterWrite && registry.get(writable).waiting) registry.refresh(writable); + registry.update( + writable, + AsyncResult.map((value) => reader.edit(value, edit) as never), + ); + } +} + +function pullRequestReference(input: PullRequestRef): PullRequestRef { + const { projectId, host, repository, number, expectedAccountId } = input; + return { + projectId, + ...(host === undefined ? {} : { host }), + repository, + number, + ...(expectedAccountId === undefined ? {} : { expectedAccountId }), + }; +} + +/** What an action changes, left out where only the host knows, as when a queue takes a merge. */ +function actionEdit(input: PullRequestActionInput): PullRequestPendingEdit | null { + if (input.stackNumber !== undefined) return null; + const reference = pullRequestReference(input); + switch (input.action) { + case "close": + return { reference, state: "closed" }; + case "reopen": + return { reference, state: "open" }; + case "ready": + return { reference, isDraft: false }; + case "draft": + return { reference, isDraft: true }; + case "enable-auto-merge": + return { reference, autoMergeEnabled: true }; + case "disable-auto-merge": + return { reference, autoMergeEnabled: false }; + default: + return null; + } +} + +function updateEdit(input: PullRequestUpdateInput): PullRequestPendingEdit | null { + if (input.title === undefined && input.body === undefined) return null; + return { + reference: pullRequestReference(input), + ...(input.title === undefined ? {} : { title: input.title }), + ...(input.body === undefined ? {} : { body: input.body }), + }; +} + function samePullRequest(reader: PullRequestRef, acted: PullRequestRef) { return ( reader.projectId === acted.projectId && @@ -214,6 +412,22 @@ export function patchPullRequestListEntries( : entries; } +function editListEntries( + input: PullRequestListInput, + entries: ReadonlyArray, + edit: PullRequestPendingEdit, +): ReadonlyArray { + let changed = false; + const edited = entries.flatMap((entry): Entry[] => { + if (!samePullRequest(entry, edit.reference)) return [entry]; + const next = withLabelEdit(withEdit(entry, edit, ["state", "isDraft", "title"]), edit); + if (next === entry) return [entry]; + changed = true; + return listAccepts(input, next.state, next.isDraft) ? [next] : []; + }); + return changed ? edited : entries; +} + function patchListWithReply( input: PullRequestListInput, value: PullRequestListResult, @@ -252,6 +466,8 @@ function actionPatchedQuery( readonly rereadWithoutReply?: boolean; /** False for reads the refresh stream does not restart; defaults to true. */ readonly refreshedByServer?: (input: Input) => boolean; + /** Shows a write in flight; absent where the reader holds nothing such a write changes. */ + readonly edit?: (input: Input, value: A, edit: PullRequestPendingEdit) => A; }, ) { const writable = Atom.family((atom: Atom.Atom>) => @@ -260,7 +476,20 @@ function actionPatchedQuery( return (target: { readonly environmentId: EnvironmentId; readonly input: Input }) => { const query = source(target); const atom = writable(query); - if (actionReplyPatches.has(atom)) return atom; + const edit = options.edit; + const reader = + edit === undefined + ? atom + : editedQuery( + atom, + { + environmentId: target.environmentId, + concerns: (reference) => options.concerns(target.input, reference), + edit: (value, pending) => edit(target.input, value, pending), + }, + options.idleTtlMs, + ); + if (actionReplyPatches.has(atom)) return reader; const pendingRead = (registry: AtomRegistry.AtomRegistry) => { const result = Option.getOrUndefined( registry.getNodes().get(query)?.valueOption() ?? Option.none(), @@ -296,7 +525,9 @@ function actionPatchedQuery( if (next === shown.value) return lacking || (unrefreshed && pending !== undefined) ? reread() : undefined; yield* options.onAnswer?.(target, next, registry) ?? Effect.void; - const mounted = (registry.getNodes().get(atom)?.listeners.size ?? 0) > 0; + const mounted = [atom, reader].some( + (shownBy) => (registry.getNodes().get(shownBy)?.listeners.size ?? 0) > 0, + ); if (!freshRead && (pending !== undefined || lacking || (unrefreshed && mounted))) registry.refresh(atom); registry.update( @@ -313,7 +544,7 @@ function actionPatchedQuery( }, ), }); - return atom; + return reader; }; } @@ -380,6 +611,8 @@ export function createLinkedPullRequestSummaryAtomFamily( number: value.number, ...(value.observedAt === undefined ? {} : { observedAt: value.observedAt }), }, + edit: (_input, value, edit) => + withEdit(value, edit, ["state", "isDraft", "autoMergeEnabled", "title"]), }, ); } @@ -515,6 +748,11 @@ export function createPullRequestEnvironmentAtoms( const { autoMergeMethod: _disarmed, ...withoutAutoMerge } = patched; return withoutAutoMerge; }, + edit: (_input, value, edit) => + withLabelEdit( + withEdit(value, edit, ["state", "isDraft", "autoMergeEnabled", "title", "body"]), + edit, + ), }, ), ); @@ -577,6 +815,7 @@ export function createPullRequestEnvironmentAtoms( "observedAt", ]), }, + edit: (_input, value, edit) => withEdit(value, edit, ["state", "isDraft", "title"]), }, ), ); @@ -584,7 +823,7 @@ export function createPullRequestEnvironmentAtoms( // Hover cards read their own atoms, so a pending or failed hover read never decides where a // click on the same link goes. const previewCard = previewQuery("environment-data:pull-requests:preview-card"); - const labelCandidates = writableQueryFamily( + const labelCandidateQueries = writableQueryFamily( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:label-candidates", tag: WS_METHODS.pullRequestsLabelCandidates, @@ -592,6 +831,51 @@ export function createPullRequestEnvironmentAtoms( staleTimeMs: 60_000, }), ); + const labelCandidates = normalizePullRequestQuery((target) => + editedQuery(labelCandidateQueries(target), { + environmentId: target.environmentId, + concerns: (reference) => samePullRequest(target.input, reference), + edit: (value, edit) => { + if (edit.labels === undefined) return value; + const { applied, labels } = edit.labels; + let changed = false; + const candidates = value.candidates.map((candidate) => { + if ( + candidate.isApplied === applied || + !labels.some((label) => label.name === candidate.name) + ) + return candidate; + changed = true; + return { ...candidate, isApplied: applied }; + }); + return changed ? { ...value, candidates } : value; + }, + }), + ); + const labelEdit = ( + registry: AtomRegistry.AtomRegistry, + target: { + readonly environmentId: EnvironmentId; + readonly input: PullRequestRef & { + readonly labels: ReadonlyArray; + readonly applied: boolean; + }; + }, + ): PullRequestPendingEdit => { + const candidates = Option.getOrUndefined( + loadedValue(registry, labelCandidates(target)), + )?.candidates; + return { + reference: pullRequestReference(target.input), + labels: { + applied: target.input.applied, + labels: target.input.labels.map((name) => ({ + name, + color: candidates?.find((candidate) => candidate.name === name)?.color ?? null, + })), + }, + }; + }; const reviewGuide = writableQueryFamily( createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:review-guide", @@ -744,6 +1028,10 @@ export function createPullRequestEnvironmentAtoms( input.host.toLowerCase() === reference.host.toLowerCase()), refreshedByServer: (input) => input.cursors === undefined, patch: patchListWithReply, + edit: (input, value, edit) => { + const entries = editListEntries(input, value.entries, edit); + return entries === value.entries ? value : { ...value, entries }; + }, }, ), /** @@ -848,41 +1136,48 @@ export function createPullRequestEnvironmentAtoms( JSON.stringify([environmentId, input.projectId, input.repository, input.number]), }, }), - runAction: createEnvironmentRpcCommand(runtime, { - tag: WS_METHODS.pullRequestsRunAction, - label: "environment-data:pull-requests:run-action", - // Preparation belongs to the write's lane. Refreshable queries would restart it after - // every preceding action, and preparing outside the lane could reorder the clicks. - execute: runAction, - onStart: (_target, registry) => readsInFlight(registry), - // Loaded readers show the reply, or re-read when the host's reply lacked the pull request. - onSuccess: ({ environmentId, input }, registry, reply, readsBeforeSend) => { - const { projectId, host, repository, number, expectedAccountId } = input; - return applyActionReply( - registry, - environmentId, - { - projectId, - ...(host === undefined ? {} : { host }), - repository, - number, - ...(expectedAccountId === undefined ? {} : { expectedAccountId }), - }, - reply, - readsBeforeSend ?? new Set(), - ); - }, - scheduler: commandScheduler, - concurrency: serialPerEnvironment, - }), - update: createEnvironmentRpcCommand(runtime, { - label: "environment-data:pull-requests:update", - tag: WS_METHODS.pullRequestsUpdate, - execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), - onSuccess: (target, registry) => Effect.sync(() => registry.refresh(preview(target))), - scheduler: commandScheduler, - concurrency: serialPerEnvironment, - }), + runAction: showWhileInFlight( + createEnvironmentRpcCommand(runtime, { + tag: WS_METHODS.pullRequestsRunAction, + label: "environment-data:pull-requests:run-action", + // Preparation belongs to the write's lane. Refreshable queries would restart it after + // every preceding action, and preparing outside the lane could reorder the clicks. + execute: runAction, + onStart: (_target, registry) => readsInFlight(registry), + // Loaded readers show the reply, or re-read when the host's reply lacked the pull request. + onSuccess: ({ environmentId, input }, registry, reply, readsBeforeSend) => + applyActionReply( + registry, + environmentId, + pullRequestReference(input), + reply, + readsBeforeSend ?? new Set(), + ).pipe( + // The re-read a bare reply starts would otherwise show the old state until it answers. + Effect.andThen( + Effect.sync(() => { + if (reply === undefined) + settleEdit(registry, environmentId, actionEdit(input), true); + }), + ), + ), + scheduler: commandScheduler, + concurrency: serialPerEnvironment, + }), + (_registry, { input }) => actionEdit(input), + ), + update: showWhileInFlight( + createEnvironmentRpcCommand(runtime, { + label: "environment-data:pull-requests:update", + tag: WS_METHODS.pullRequestsUpdate, + execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), + onSuccess: ({ environmentId, input }, registry) => + Effect.sync(() => settleEdit(registry, environmentId, updateEdit(input))), + scheduler: commandScheduler, + concurrency: serialPerEnvironment, + }), + (_registry, { input }) => updateEdit(input), + ), comment: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:comment", tag: WS_METHODS.pullRequestsComment, @@ -1018,44 +1313,20 @@ export function createPullRequestEnvironmentAtoms( }), }), labelCandidates, - setLabels: createEnvironmentRpcCommand(runtime, { - label: "environment-data:pull-requests:set-labels", - tag: WS_METHODS.pullRequestsSetLabels, - execute: (input) => routedRequest(WS_METHODS.pullRequestsSetLabels, input), - scheduler: commandScheduler, - concurrency: serialPerEnvironment, - onSuccess: (target, registry) => - Effect.sync(() => { - const { labels, applied } = target.input; - const candidatesAtom = labelCandidates(target); - const candidates = AsyncResult.getOrElse(registry.get(candidatesAtom), () => null); - const names = new Set(labels); - updateCached(registry, candidatesAtom, (value) => ({ - ...value, - candidates: value.candidates.map((candidate) => - names.has(candidate.name) ? { ...candidate, isApplied: applied } : candidate, - ), - })); - for (const atom of loadedDetails(registry, target)) { - updateCached(registry, atom, (value) => ({ - ...value, - labels: applied - ? [ - ...value.labels, - ...labels - .filter((name) => !value.labels.some((label) => label.name === name)) - .map((name) => ({ - name, - color: - candidates?.candidates.find((candidate) => candidate.name === name) - ?.color ?? null, - })), - ] - : value.labels.filter((label) => !names.has(label.name)), - })); - } - }), - }), + setLabels: showWhileInFlight( + createEnvironmentRpcCommand(runtime, { + label: "environment-data:pull-requests:set-labels", + tag: WS_METHODS.pullRequestsSetLabels, + execute: (input) => routedRequest(WS_METHODS.pullRequestsSetLabels, input), + scheduler: commandScheduler, + concurrency: serialPerEnvironment, + onSuccess: (target, registry) => + Effect.sync(() => + settleEdit(registry, target.environmentId, labelEdit(registry, target)), + ), + }), + labelEdit, + ), setThreadResolution: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:set-thread-resolution", tag: WS_METHODS.pullRequestsSetThreadResolution, From 4a19a4646230329b94ebee8c7b06ac331dcda297 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 02:46:16 +0100 Subject: [PATCH 02/28] perf(web): title editor and label picker stop waiting on the host The title editor closes at once and no longer re-reads detail, activity, and stack after a save. The label picker stays usable while a change is in flight; changes queue on the host in order. Created with Claude Opus 5.5 in Claude Code via T3 Code. --- .../src/components/pullRequest/PullRequestDetailPanel.tsx | 1 - .../src/components/pullRequest/PullRequestLabelPicker.tsx | 7 ++----- .../src/components/pullRequest/PullRequestTitleEditor.tsx | 8 +++----- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 9d0f6193f7e3f..c5998929bae38 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1521,7 +1521,6 @@ export function PullRequestDetailPanel({ pullRequestKey={pullRequestKey} detail={detail} canWriteSourceControl={canWriteSourceControl} - onSaved={refreshDetail} /> } author={ diff --git a/apps/web/src/components/pullRequest/PullRequestLabelPicker.tsx b/apps/web/src/components/pullRequest/PullRequestLabelPicker.tsx index e32ef26053bd2..24b4d1eedd7b5 100644 --- a/apps/web/src/components/pullRequest/PullRequestLabelPicker.tsx +++ b/apps/web/src/components/pullRequest/PullRequestLabelPicker.tsx @@ -32,7 +32,6 @@ export function PullRequestLabelPicker({ }) { const [open, setOpen] = useState(false); const [query, setQuery] = useState(""); - const [pending, setPending] = useState(null); const candidatesQuery = useEnvironmentQuery( open ? pullRequestEnvironment.labelCandidates({ environmentId, input: reference }) : null, @@ -45,14 +44,12 @@ export function PullRequestLabelPicker({ matches(entry, query), ); + // Each change shows at once and queues behind the one before it on the host. const toggle = async (candidate: PullRequestLabelCandidate) => { - if (pending !== null) return; - setPending(candidate.name); const result = await setLabels({ environmentId, input: { ...reference, labels: [candidate.name], applied: !candidate.isApplied }, }); - setPending(null); if (result._tag === "Failure") { toastManager.add({ type: "error", @@ -88,7 +85,7 @@ export function PullRequestLabelPicker({ truncated={candidatesQuery.data?.truncated === true} truncatedLabel="This repository has more labels than are listed here. Apply the rest on the host." candidateKey={(candidate) => candidate.name} - disabled={pending !== null} + disabled={false} onSelect={(candidate) => void toggle(candidate)} > {(candidate) => { diff --git a/apps/web/src/components/pullRequest/PullRequestTitleEditor.tsx b/apps/web/src/components/pullRequest/PullRequestTitleEditor.tsx index ce6e8a801074d..992e785f018b8 100644 --- a/apps/web/src/components/pullRequest/PullRequestTitleEditor.tsx +++ b/apps/web/src/components/pullRequest/PullRequestTitleEditor.tsx @@ -24,14 +24,12 @@ export function PullRequestTitleEditor({ pullRequestKey, detail, canWriteSourceControl, - onSaved, }: { environmentId: EnvironmentId; reference: PullRequestRef; pullRequestKey: string; detail: PullRequestDetail; canWriteSourceControl: boolean; - onSaved: () => void; }) { const update = useAtomCommand(pullRequestEnvironment.update, { reportFailure: false }); const [titleScope, setTitleScope] = useState<{ @@ -49,9 +47,12 @@ export function PullRequestTitleEditor({ return; } setTitleSaving(true); + // The new title shows while the host takes it, so the editor closes at once. + setTitleScope(null); const result = await update({ environmentId, input: { ...reference, title } }); setTitleSaving(false); if (result._tag === "Failure") { + setTitleScope({ pullRequestKey, text: title }); toastManager.add({ type: "error", title: "The title could not be saved", @@ -60,10 +61,7 @@ export function PullRequestTitleEditor({ "The host refused the new title.", ), }); - return; } - setTitleScope(null); - onSaved(); }; if (titleDraft === null) { From d29511373b7d3f04e2c52ee05899835f004e2401 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Tue, 10 Nov 2026 04:37:24 +0000 Subject: [PATCH 03/28] fix(pr): preserve optimistic titles across shared summaries --- apps/web/src/state/pullRequests.ts | 23 +++++++++++- .../src/state/pullRequests.test.ts | 32 ++++++++++++++--- .../client-runtime/src/state/pullRequests.ts | 36 +++++++++++++++---- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index 914b6e6c72056..96c931b8490b4 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -1,9 +1,11 @@ import { useAtomValue } from "@effect/atom-react"; import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; import { + applyPullRequestSummaryEdits, createLinkedPullRequestSummaryAtomFamily, createPullRequestEnvironmentAtoms, createPullRequestStackAtomFamily, + pullRequestPendingEditsAtom, } from "@t3tools/client-runtime/state/pull-requests"; import { PullRequestListResult, @@ -368,6 +370,20 @@ export function restoredPullRequestList( // Answers are stamped on arrival: older servers leave no other ordering for same-dated reads. export const pullRequestEnvironment = createPullRequestEnvironmentAtoms(connectionAtomRuntime, { + acceptedEdit: ({ environmentId, input }, edit, registry) => + Effect.map(Clock.currentTimeMillis, (observedAt) => { + const saved = registry.get( + savedPullRequestSummaryAtom(savedPullRequestKey(environmentId, input)), + ); + recordObservation(registry, pullRequestSummaryKey(environmentId, input), (current) => { + const summary = applyPullRequestSummaryEdits( + input, + newestPullRequestSummary(observedPullRequestSummary(current), saved), + [edit], + ); + return summary === null ? current : { ...current, metadata: { summary, observedAt } }; + }); + }), list: ({ environmentId, input }, answer, registry) => Effect.map(Clock.currentTimeMillis, (observedAt) => { rememberPullRequestListAnswer(snapshotStorage(), { environmentId, input }, answer); @@ -503,6 +519,7 @@ export function useSharedPullRequestSummary( ? "none" : pullRequestSummaryKey(environmentId, reference); const observations = useAtomValue(pullRequestObservationsAtom(key)); + const edits = useAtomValue(pullRequestPendingEditsAtom(environmentId)); const saved = useAtomValue( savedPullRequestSummaryAtom( environmentId === null || reference === null @@ -510,7 +527,11 @@ export function useSharedPullRequestSummary( : savedPullRequestKey(environmentId, reference), ), ); - return newestPullRequestSummary(observedPullRequestSummary(observations), saved); + return applyPullRequestSummaryEdits( + reference, + newestPullRequestSummary(observedPullRequestSummary(observations), saved), + edits, + ); } export const pullRequestStackAtom = createPullRequestStackAtomFamily( connectionAtomRuntime, diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 088c94bab017d..7ef4ccde50964 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -7,6 +7,7 @@ import { AuthOrchestrationOperateScope, ProjectId, PullRequestOperationError, + SourceControlProviderKind, WS_METHODS, type AuthSessionState, type PullRequestListInput, @@ -39,8 +40,10 @@ import * as EnvironmentSupervisor from "../connection/supervisor.ts"; import type { WsRpcProtocolClient } from "../rpc/protocol.ts"; import type { RpcSession } from "../rpc/session.ts"; import { + applyPullRequestSummaryEdits, createLinkedPullRequestSummaryAtomFamily, createPullRequestEnvironmentAtoms, + pullRequestPendingEditsAtom, } from "./pullRequests.ts"; import * as PullRequestDiffLoader from "./pullRequestDiffHttp.ts"; import { executeAtomQuery, queryPersistenceKey } from "./runtime.ts"; @@ -2610,7 +2613,18 @@ it.effect.each(["accepted", "refused"] as const)( title = input.title; }), }); - const { atoms, registry } = yield* makeTestRuntime(client); + let observedTitle = "Original"; + const handlers = { + detail: (_target: unknown, detail: { readonly title: string }) => + Effect.sync(() => { + observedTitle = detail.title; + }), + acceptedEdit: (_target: unknown, edit: { readonly title?: string }) => + Effect.sync(() => { + observedTitle = edit.title ?? observedTitle; + }), + }; + const { atoms, registry } = yield* makeTestRuntime(client, undefined, false, handlers); const target = { environmentId: TARGET.environmentId, input: actionReference }; const detail = atoms.detail(target); const preview = atoms.preview(target); @@ -2628,21 +2642,31 @@ it.effect.each(["accepted", "refused"] as const)( Option.getOrThrow(AsyncResult.value(registry.get(detail))).title, Option.getOrThrow(AsyncResult.value(registry.get(preview))).title, Option.getOrThrow(AsyncResult.value(registry.get(list))).entries[0]?.title, + applyPullRequestSummaryEdits( + actionReference, + { + ...actionReply, + provider: SourceControlProviderKind.make("github"), + title: observedTitle, + }, + registry.get(pullRequestPendingEditsAtom(target.environmentId)), + )?.title, ]; const editing = yield* Effect.promise(() => atoms.update.run(registry, { ...target, input: { ...actionReference, title: "Edited" } }), ).pipe(Effect.forkScoped); yield* Deferred.await(sent); - expect(titles()).toEqual(["Edited", "Edited", "Edited"]); + expect(titles()).toEqual(["Edited", "Edited", "Edited", "Edited"]); yield* Deferred.succeed(release, undefined); yield* Fiber.join(editing); expect(titles()).toEqual( outcome === "refused" - ? ["Original", "Original", "Original"] - : ["Edited", "Edited", "Edited"], + ? ["Original", "Original", "Original", "Original"] + : ["Edited", "Edited", "Edited", "Edited"], ); + expect(observedTitle).toBe(outcome === "refused" ? "Original" : "Edited"); // The edit is known, so no reader asks the host again. expect(reads).toBe(3); }), diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 22208487ec82b..7dc73cdfafcfb 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -149,7 +149,7 @@ interface ActionReplyPatch { const actionReplyPatches = new WeakMap(); /** What a write in flight will change, shown by every reader until the host answers. */ -interface PullRequestPendingEdit { +export interface PullRequestPendingEdit { readonly reference: PullRequestRef; readonly state?: PullRequestState; readonly isDraft?: boolean; @@ -171,7 +171,7 @@ interface EditableReader { } // Module-wide like the reply patches, so separately built readers see the same writes. -const pendingEdits = Atom.family((_environmentId: EnvironmentId) => +export const pullRequestPendingEditsAtom = Atom.family((_environmentId: EnvironmentId | null) => Atom.make>([]).pipe(Atom.keepAlive), ); const editableReaders = new WeakMap(); @@ -236,7 +236,7 @@ function editedQuery( const shown = Atom.writable( (get) => { const result = get(inner); - const edits = get(pendingEdits(reader.environmentId)); + const edits = get(pullRequestPendingEditsAtom(reader.environmentId)); const value = AsyncResult.value(result); if (edits.length === 0 || Option.isNone(value)) return result; const next = edits.reduce( @@ -264,7 +264,7 @@ function showWhileInFlight { const edit = editOf(registry, target); if (edit === null) return command.run(registry, target); - const edits = pendingEdits(target.environmentId); + const edits = pullRequestPendingEditsAtom(target.environmentId); registry.update(edits, (current) => [...current, edit]); try { return await command.run(registry, target); @@ -351,6 +351,21 @@ function samePullRequest(reader: PullRequestRef, acted: PullRequestRef) { ); } +export function applyPullRequestSummaryEdits( + reference: PullRequestRef | null, + summary: PullRequestSummary | null, + edits: ReadonlyArray, +): PullRequestSummary | null { + if (reference === null || summary === null) return summary; + return edits.reduce( + (current, edit) => + samePullRequest(reference, edit.reference) + ? withEdit(current, edit, ["state", "isDraft", "autoMergeEnabled", "title"]) + : current, + summary, + ); +} + function pickDefined( source: T, keys: ReadonlyArray, @@ -626,6 +641,7 @@ export interface PullRequestAnswerHandlers { readonly reviewGuide?: QueryAnswerHandler; /** Once per action reply, after the loaded readers show it; unscoped references only. */ readonly actionReply?: QueryAnswerHandler; + readonly acceptedEdit?: QueryAnswerHandler; } export function createPullRequestStackAtomFamily( @@ -1171,8 +1187,16 @@ export function createPullRequestEnvironmentAtoms( label: "environment-data:pull-requests:update", tag: WS_METHODS.pullRequestsUpdate, execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), - onSuccess: ({ environmentId, input }, registry) => - Effect.sync(() => settleEdit(registry, environmentId, updateEdit(input))), + onSuccess: (target, registry) => { + const edit = updateEdit(target.input); + return Effect.sync(() => settleEdit(registry, target.environmentId, edit)).pipe( + Effect.andThen(() => + edit === null + ? Effect.void + : (onAnswer.acceptedEdit?.(target, edit, registry) ?? Effect.void), + ), + ); + }, scheduler: commandScheduler, concurrency: serialPerEnvironment, }), From 8e4a2bc36fe06279fd4e9289a04d37f0ed02e4ef Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Tue, 10 Nov 2026 04:53:22 +0000 Subject: [PATCH 04/28] fix(pr): retain accepted titles when saved summaries tie --- apps/web/src/state/pullRequests.test.ts | 2 ++ apps/web/src/state/pullRequests.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/web/src/state/pullRequests.test.ts b/apps/web/src/state/pullRequests.test.ts index 87b1483f992df..10efe769bcf83 100644 --- a/apps/web/src/state/pullRequests.test.ts +++ b/apps/web/src/state/pullRequests.test.ts @@ -94,6 +94,8 @@ describe("pull request summary cache", () => { const older = summary({ updatedAt: "2026-09-11T00:00:00Z", observedAt: 999 }); expect(newestPullRequestSummary(older, reopened)).toBe(reopened); expect(newestPullRequestSummary(reopened, older)).toBe(reopened); + const edited = summary({ title: "Accepted title" }); + expect(newestPullRequestSummary(edited, summary())).toBe(edited); }); it("reuses saved thread state while detail is stale and lets newer detail replace it", () => { diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index 96c931b8490b4..17a02a8093555 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -64,7 +64,7 @@ export function newestPullRequestSummary( ): PullRequestSummary | null { if (current === null) return observed; if (observed === null) return current; - return compareSummaries(current, observed) >= 0 ? observed : current; + return compareSummaries(current, observed) > 0 ? observed : current; } export function pullRequestListEntryToSummary(entry: PullRequestListEntry): PullRequestSummary { From fcf73a0206bd058d5e1ef1e54aa176e2eb484c53 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:36:34 +0100 Subject: [PATCH 05/28] fix(pr): a queued reopen keeps a closed pull request in open lists An open-only list dropped a pull request when a queued close was followed by a queued reopen, because each pending edit filtered the row on its own. Readers now get all pending edits for their pull request at once, and the list checks membership against the row's final state. Claude Opus 5.5 via Claude Code --- .../src/state/pullRequests.test.ts | 10 ++- .../client-runtime/src/state/pullRequests.ts | 88 +++++++++++-------- 2 files changed, 62 insertions(+), 36 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 7ef4ccde50964..cf8c37edf673d 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2535,8 +2535,16 @@ it.effect("shows a queued close before the action ahead of it answers", () => yield* Deferred.await(firstSent); expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries).toEqual([]); + + const reopen = atoms.runAction.run(registry, { + environmentId: TARGET.environmentId, + input: { ...actionReference, number: 1, action: "reopen" }, + }); + expect( + Option.getOrThrow(AsyncResult.value(registry.get(list))).entries.map((row) => row.number), + ).toEqual([1]); yield* Deferred.succeed(release, undefined); - yield* Effect.promise(() => Promise.all(closes)); + yield* Effect.promise(() => Promise.all([...closes, reopen])); }), ), ); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 7dc73cdfafcfb..979c1443866a4 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -167,7 +167,7 @@ type EditedField = "state" | "isDraft" | "autoMergeEnabled" | "title" | "body"; interface EditableReader { readonly environmentId: EnvironmentId; readonly concerns: (reference: PullRequestRef) => boolean; - readonly edit: (value: never, edit: PullRequestPendingEdit) => unknown; + readonly edit: (value: never, edits: ReadonlyArray) => unknown; } // Module-wide like the reply patches, so separately built readers see the same writes. @@ -220,6 +220,22 @@ function withLabelEdit { + if (candidate.isApplied === applied || !labels.some((label) => label.name === candidate.name)) + return candidate; + changed = true; + return { ...candidate, isApplied: applied }; + }); + return changed ? { ...value, candidates } : value; +} + /** * The reader with every write in flight for its pull request applied. Writes go to the reader * underneath, so a refused edit never stays behind in what the host said. @@ -227,7 +243,7 @@ function withLabelEdit( inner: Atom.Writable>, reader: Omit & { - readonly edit: (value: A, edit: PullRequestPendingEdit) => A; + readonly edit: (value: A, edits: ReadonlyArray) => A; }, idleTtlMs = 5 * 60_000, ): Atom.Writable> { @@ -236,13 +252,12 @@ function editedQuery( const shown = Atom.writable( (get) => { const result = get(inner); - const edits = get(pullRequestPendingEditsAtom(reader.environmentId)); + const edits = get(pullRequestPendingEditsAtom(reader.environmentId)).filter((edit) => + reader.concerns(edit.reference), + ); const value = AsyncResult.value(result); if (edits.length === 0 || Option.isNone(value)) return result; - const next = edits.reduce( - (current, edit) => (reader.concerns(edit.reference) ? reader.edit(current, edit) : current), - value.value, - ); + const next = reader.edit(value.value, edits); return next === value.value ? result : AsyncResult.map(result, () => next); }, (context, value: AsyncResult.AsyncResult) => context.set(inner, value), @@ -287,13 +302,13 @@ function settleEdit( const reader = typeof atom === "string" ? undefined : editableReaders.get(atom); if (reader?.environmentId !== environmentId || !reader.concerns(edit.reference)) continue; const shown = loadedValue(registry, atom as Atom.Atom); - if (Option.isNone(shown) || reader.edit(shown.value, edit) === shown.value) continue; + if (Option.isNone(shown) || reader.edit(shown.value, [edit]) === shown.value) continue; const writable = atom as Atom.Writable>; // A read still waiting may predate the write, unless the caller started it after the commit. if (!readsAfterWrite && registry.get(writable).waiting) registry.refresh(writable); registry.update( writable, - AsyncResult.map((value) => reader.edit(value, edit) as never), + AsyncResult.map((value) => reader.edit(value, [edit]) as never), ); } } @@ -430,12 +445,17 @@ export function patchPullRequestListEntries( function editListEntries( input: PullRequestListInput, entries: ReadonlyArray, - edit: PullRequestPendingEdit, + edits: ReadonlyArray, ): ReadonlyArray { let changed = false; const edited = entries.flatMap((entry): Entry[] => { - if (!samePullRequest(entry, edit.reference)) return [entry]; - const next = withLabelEdit(withEdit(entry, edit, ["state", "isDraft", "title"]), edit); + const next = edits.reduce( + (current, edit) => + samePullRequest(entry, edit.reference) + ? withLabelEdit(withEdit(current, edit, ["state", "isDraft", "title"]), edit) + : current, + entry, + ); if (next === entry) return [entry]; changed = true; return listAccepts(input, next.state, next.isDraft) ? [next] : []; @@ -482,7 +502,7 @@ function actionPatchedQuery( /** False for reads the refresh stream does not restart; defaults to true. */ readonly refreshedByServer?: (input: Input) => boolean; /** Shows a write in flight; absent where the reader holds nothing such a write changes. */ - readonly edit?: (input: Input, value: A, edit: PullRequestPendingEdit) => A; + readonly edit?: (input: Input, value: A, edits: ReadonlyArray) => A; }, ) { const writable = Atom.family((atom: Atom.Atom>) => @@ -626,8 +646,12 @@ export function createLinkedPullRequestSummaryAtomFamily( number: value.number, ...(value.observedAt === undefined ? {} : { observedAt: value.observedAt }), }, - edit: (_input, value, edit) => - withEdit(value, edit, ["state", "isDraft", "autoMergeEnabled", "title"]), + edit: (_input, value, edits) => + edits.reduce( + (current, edit) => + withEdit(current, edit, ["state", "isDraft", "autoMergeEnabled", "title"]), + value, + ), }, ); } @@ -764,10 +788,14 @@ export function createPullRequestEnvironmentAtoms( const { autoMergeMethod: _disarmed, ...withoutAutoMerge } = patched; return withoutAutoMerge; }, - edit: (_input, value, edit) => - withLabelEdit( - withEdit(value, edit, ["state", "isDraft", "autoMergeEnabled", "title", "body"]), - edit, + edit: (_input, value, edits) => + edits.reduce( + (current, edit) => + withLabelEdit( + withEdit(current, edit, ["state", "isDraft", "autoMergeEnabled", "title", "body"]), + edit, + ), + value, ), }, ), @@ -831,7 +859,11 @@ export function createPullRequestEnvironmentAtoms( "observedAt", ]), }, - edit: (_input, value, edit) => withEdit(value, edit, ["state", "isDraft", "title"]), + edit: (_input, value, edits) => + edits.reduce( + (current, edit) => withEdit(current, edit, ["state", "isDraft", "title"]), + value, + ), }, ), ); @@ -851,21 +883,7 @@ export function createPullRequestEnvironmentAtoms( editedQuery(labelCandidateQueries(target), { environmentId: target.environmentId, concerns: (reference) => samePullRequest(target.input, reference), - edit: (value, edit) => { - if (edit.labels === undefined) return value; - const { applied, labels } = edit.labels; - let changed = false; - const candidates = value.candidates.map((candidate) => { - if ( - candidate.isApplied === applied || - !labels.some((label) => label.name === candidate.name) - ) - return candidate; - changed = true; - return { ...candidate, isApplied: applied }; - }); - return changed ? { ...value, candidates } : value; - }, + edit: (value, edits) => edits.reduce(withLabelCandidateEdit, value), }), ); const labelEdit = ( From b42e869974a6c1de89e5ec9a5a5ced894da26dc3 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:50:08 +0100 Subject: [PATCH 06/28] fix(pr): a stored detail snapshot no longer hides a pending title When the detail read had no value yet, the panel fell back to its stored snapshot. Against an older server that sends no observedAt, the snapshot tied the shared summary, won the tie, and hid a pending or accepted title until a detail read succeeded. A shared summary now wins a tie against the stored snapshot. A loaded detail keeps winning ties, since it already carries the edits. Claude Opus 5.5 via Claude Code --- .../components/pullRequest/usePullRequestDetail.ts | 4 +++- apps/web/src/state/pullRequests.test.ts | 4 ++++ apps/web/src/state/pullRequests.ts | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.ts b/apps/web/src/components/pullRequest/usePullRequestDetail.ts index 7d6ab0c7a6154..2f1392c403b86 100644 --- a/apps/web/src/components/pullRequest/usePullRequestDetail.ts +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.ts @@ -13,6 +13,7 @@ import { useServerConfigs } from "~/state/entities"; import { useEnvironmentQuery } from "~/state/query"; import { useEnvironmentScope } from "~/state/session"; import { + newestDisplayedPullRequestSummary, newestPullRequestSummary, pullRequestEnvironment, pullRequestListEntryToSummary, @@ -115,8 +116,9 @@ export function usePullRequestDetail( // `detail` would cost it its cache scope. const listSummary = listEntry === null ? null : pullRequestListEntryToSummary(listEntry); // The row can come from the page's stored snapshot, which no answer has published yet. - const sharedSummary = newestPullRequestSummary( + const sharedSummary = newestDisplayedPullRequestSummary( resolvedCoreDetail, + liveDetail === null, newestPullRequestSummary(observedSummary, listSummary), ); const coreDetail = diff --git a/apps/web/src/state/pullRequests.test.ts b/apps/web/src/state/pullRequests.test.ts index 10efe769bcf83..3078d72d4e16b 100644 --- a/apps/web/src/state/pullRequests.test.ts +++ b/apps/web/src/state/pullRequests.test.ts @@ -15,6 +15,7 @@ import { observePullRequestChecks, observePullRequestMetadata, newestPullRequestSummary, + newestDisplayedPullRequestSummary, rememberPullRequestListAnswer, restorePullRequestList, restoredPullRequestList, @@ -96,6 +97,9 @@ describe("pull request summary cache", () => { expect(newestPullRequestSummary(reopened, older)).toBe(reopened); const edited = summary({ title: "Accepted title" }); expect(newestPullRequestSummary(edited, summary())).toBe(edited); + const loaded = summary(); + expect(newestDisplayedPullRequestSummary(loaded, false, edited)).toBe(loaded); + expect(newestDisplayedPullRequestSummary(summary(), true, edited)).toBe(edited); }); it("reuses saved thread state while detail is stale and lets newer detail replace it", () => { diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index 17a02a8093555..5aa309c1a54ee 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -67,6 +67,17 @@ export function newestPullRequestSummary( return compareSummaries(current, observed) > 0 ? observed : current; } +/** A stored snapshot carries no edit, so a shared summary dated the same wins over it. */ +export function newestDisplayedPullRequestSummary( + detail: PullRequestSummary | null, + fromSnapshot: boolean, + shared: PullRequestSummary | null, +): PullRequestSummary | null { + return fromSnapshot + ? newestPullRequestSummary(shared, detail) + : newestPullRequestSummary(detail, shared); +} + export function pullRequestListEntryToSummary(entry: PullRequestListEntry): PullRequestSummary { return { provider: entry.provider, From 3fbca2431da1e9c2f4eafd6324b1120673dd8d34 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:59:55 +0100 Subject: [PATCH 07/28] fix(pr): a label read still loading restarts when a label change lands A label-candidate read that had not answered yet was skipped when an accepted change settled, so its reply, sent before the change, could show the label as not applied once the overlay was gone. Claude Opus 5.5 via Claude Code --- .../src/state/pullRequests.test.ts | 96 ++++++++++--------- .../client-runtime/src/state/pullRequests.ts | 4 +- 2 files changed, 53 insertions(+), 47 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index cf8c37edf673d..b696bfee6aa40 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2748,52 +2748,56 @@ it.effect("shows a label change at once and takes it back when the host refuses ), ); -it.effect("keeps an accepted label change when a candidates read sent before it answers late", () => - Effect.scoped( - Effect.gen(function* () { - let applied = false; - let reads = 0; - const readStarted = yield* Deferred.make(); - const releaseRead = yield* Deferred.make(); - const client = actionReplyClient({ - [WS_METHODS.pullRequestsLabelCandidates]: () => - Effect.gen(function* () { - reads++; - const answer = { - candidates: [{ name: "new", color: null, description: null, isApplied: applied }], - truncated: false, - }; - if (reads === 2) { - yield* Deferred.succeed(readStarted, undefined); - yield* Deferred.await(releaseRead); - } - return answer; - }), - [WS_METHODS.pullRequestsSetLabels]: () => - Effect.sync(() => { - applied = true; - }), - }); - const { atoms, registry } = yield* makeTestRuntime(client); - const target = { environmentId: TARGET.environmentId, input: actionReference }; - const candidates = atoms.labelCandidates(target); - registry.mount(candidates); - yield* Effect.promise(() => executeAtomQuery(registry, candidates)); - registry.refresh(candidates); - yield* Deferred.await(readStarted); +it.effect.each(["loaded", "loading"] as const)( + "keeps an accepted label change when a candidates read sent before it answers late (%s)", + (shown) => + Effect.scoped( + Effect.gen(function* () { + let applied = false; + let reads = 0; + const readStarted = yield* Deferred.make(); + const releaseRead = yield* Deferred.make(); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsLabelCandidates]: () => + Effect.gen(function* () { + reads++; + const answer = { + candidates: [{ name: "new", color: null, description: null, isApplied: applied }], + truncated: false, + }; + if (reads === (shown === "loaded" ? 2 : 1)) { + yield* Deferred.succeed(readStarted, undefined); + yield* Deferred.await(releaseRead); + } + return answer; + }), + [WS_METHODS.pullRequestsSetLabels]: () => + Effect.sync(() => { + applied = true; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const candidates = atoms.labelCandidates(target); + registry.mount(candidates); + if (shown === "loaded") { + yield* Effect.promise(() => executeAtomQuery(registry, candidates)); + registry.refresh(candidates); + } + yield* Deferred.await(readStarted); - yield* Effect.promise(() => - atoms.setLabels.run(registry, { - ...target, - input: { ...actionReference, labels: ["new"], applied: true }, - }), - ); - yield* Deferred.succeed(releaseRead, undefined); + yield* Effect.promise(() => + atoms.setLabels.run(registry, { + ...target, + input: { ...actionReference, labels: ["new"], applied: true }, + }), + ); + yield* Deferred.succeed(releaseRead, undefined); - expect( - (yield* AtomRegistry.getResult(registry, candidates, { suspendOnWaiting: true })) - .candidates[0]?.isApplied, - ).toBe(true); - }), - ), + expect( + (yield* AtomRegistry.getResult(registry, candidates, { suspendOnWaiting: true })) + .candidates[0]?.isApplied, + ).toBe(true); + }), + ), ); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 979c1443866a4..54dc54c2edd48 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -302,10 +302,12 @@ function settleEdit( const reader = typeof atom === "string" ? undefined : editableReaders.get(atom); if (reader?.environmentId !== environmentId || !reader.concerns(edit.reference)) continue; const shown = loadedValue(registry, atom as Atom.Atom); - if (Option.isNone(shown) || reader.edit(shown.value, [edit]) === shown.value) continue; const writable = atom as Atom.Writable>; + const unchanged = Option.isSome(shown) && reader.edit(shown.value, [edit]) === shown.value; + if (unchanged) continue; // A read still waiting may predate the write, unless the caller started it after the commit. if (!readsAfterWrite && registry.get(writable).waiting) registry.refresh(writable); + if (Option.isNone(shown)) continue; registry.update( writable, AsyncResult.map((value) => reader.edit(value, [edit]) as never), From 2ca25848b46b2c9874eac69615cc362d3ecaf8b1 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 07:10:25 +0100 Subject: [PATCH 08/28] perf(pr): merge shows its result at once where the host says whether it queues GitHub (gated for older Enterprise) and Forgejo report whether a merge joins a queue, so the client shows merged, or auto-merge on, before the host replies. A refusal puts it back. The checks read also shows writes and takes action replies, since the panel can take its state from it. Claude Opus 5.5 via Claude Code --- .../pullRequest/PullRequestService.test.ts | 27 +++++ .../src/pullRequest/PullRequestService.ts | 3 + .../src/state/pullRequests.test.ts | 108 ++++++++++++++++++ .../client-runtime/src/state/pullRequests.ts | 71 +++++++++--- packages/contracts/src/pullRequest.ts | 2 + .../src/server/PullRequestProvider.ts | 1 + .../src/server/ForgejoPullRequestProvider.ts | 2 + .../src/server/gitHubPullRequestJson.test.ts | 6 + .../src/server/gitHubPullRequestJson.ts | 2 + .../src/server/pullRequestReads.graphql | 2 + .../src/server/pullRequestReads.graphql.ts | 4 +- 11 files changed, 212 insertions(+), 16 deletions(-) diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index e559bc84dd978..1ebee1b82be81 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -117,6 +117,33 @@ function hostedChangeRequest(body: string, additions = 1) { }; } +it.effect( + "passes on whether a merge joins the host's queue, and says nothing it was not told", + () => + Effect.gen(function* () { + let mergeQueue: boolean | undefined = true; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/w", repository: "acme/web" }), + ], + providers: [ + fakePullRequestProvider(SourceControlProviderKind.make("github"), { + getChangeRequest: () => + Effect.sync(() => ({ + ...hostedChangeRequest("Body"), + ...(mergeQueue === undefined ? {} : { mergeQueue }), + })), + }), + ], + }); + const ref = { projectId: "p1" as ProjectId, repository: "acme/web", number: 1 }; + assert.strictEqual((yield* service.detail(ref)).mergeQueue, true); + mergeQueue = undefined; + const other = { ...ref, number: 2 }; + assert.isFalse("mergeQueue" in (yield* service.detail(other))); + }), +); + it.effect("reads maintainer edits and changes them through the update command", () => Effect.gen(function* () { let enabled = true; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 0140a3aa96e2c..6ff99b0e4fe44 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1853,6 +1853,9 @@ export const make = Effect.gen(function* () { ...(changeRequest.maintainerCanModify === undefined ? {} : { maintainerCanModify: changeRequest.maintainerCanModify }), + ...(changeRequest.mergeQueue === undefined + ? {} + : { mergeQueue: changeRequest.mergeQueue }), ...(changeRequest.workflowApprovalsRequired === undefined ? {} : { workflowApprovalsRequired: changeRequest.workflowApprovalsRequired }), diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index b696bfee6aa40..96f2be1ef2d22 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2801,3 +2801,111 @@ it.effect.each(["loaded", "loading"] as const)( }), ), ); + +it.effect.each([ + [false, { state: "merged" }], + [true, { state: "open", autoMergeEnabled: true }], +] as const)( + "shows a merge the host lands at once or queues before it replies (queue %s)", + ([mergeQueue, shown]) => + Effect.scoped( + Effect.gen(function* () { + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const open = { + ...actionReference, + state: "open", + isDraft: false, + autoMergeEnabled: false, + mergeQueue, + updatedAt: "2026-10-27T09:00:00Z", + observedAt: 100, + }; + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: () => Effect.succeed(open), + [WS_METHODS.pullRequestsList]: () => Effect.succeed(listResult([open])), + [WS_METHODS.pullRequestsRunAction]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + return { ...actionReply, ...shown }; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const detail = atoms.detail(target); + const list = atoms.list({ environmentId: target.environmentId, input: { state: "open" } }); + const loaded: ReadonlyArray>> = [ + detail, + list, + ]; + for (const atom of loaded) { + registry.mount(atom); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + + const acting = yield* Effect.promise(() => + atoms.runAction.run(registry, { + ...target, + input: { ...actionReference, action: "merge" }, + }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject(shown); + expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries).toHaveLength( + mergeQueue ? 1 : 0, + ); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(acting); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject(shown); + }), + ), +); + +it.effect.each(["accepted", "refused"] as const)( + "shows a close in the checks read the panel takes its state from (%s)", + (outcome) => + Effect.scoped( + Effect.gen(function* () { + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + let reads = 0; + const client = actionReplyClient({ + [WS_METHODS.pullRequestsChecks]: () => + reads++ === 0 + ? Effect.succeed({ state: "open", checks: [], observedAt: 100 }) + : Effect.never, + [WS_METHODS.pullRequestsRunAction]: () => + Effect.gen(function* () { + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + if (outcome === "accepted") return { ...actionReply, state: "closed" as const }; + return yield* new PullRequestOperationError({ + operation: "runAction", + detail: "You cannot close this pull request.", + }); + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const checks = atoms.checks(target); + registry.mount(checks); + yield* Effect.promise(() => executeAtomQuery(registry, checks)); + const state = () => Option.getOrThrow(AsyncResult.value(registry.get(checks)))?.state; + + const acting = yield* Effect.promise(() => + atoms.runAction.run(registry, { + ...target, + input: { ...actionReference, action: "close" }, + }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + expect(state()).toBe("closed"); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(acting); + // An accepted close stays while the next checks read is still out. + expect(state()).toBe(outcome === "accepted" ? "closed" : "open"); + }), + ), +); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 54dc54c2edd48..a8196366ad8d9 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -326,11 +326,20 @@ function pullRequestReference(input: PullRequestRef): PullRequestRef { }; } -/** What an action changes, left out where only the host knows, as when a queue takes a merge. */ -function actionEdit(input: PullRequestActionInput): PullRequestPendingEdit | null { +/** What an action changes, left out where only the host knows, as a merge where queues are unknown. */ +function actionEdit( + input: PullRequestActionInput, + mergeQueue: boolean | undefined, +): PullRequestPendingEdit | null { if (input.stackNumber !== undefined) return null; const reference = pullRequestReference(input); switch (input.action) { + case "merge": + return mergeQueue === undefined + ? null + : mergeQueue + ? { reference, autoMergeEnabled: true } + : { reference, state: "merged" }; case "close": return { reference, state: "closed" }; case "reopen": @@ -812,6 +821,13 @@ export function createPullRequestEnvironmentAtoms( detail({ ...target, input: { ...target.input, allowStale: undefined } }), detail({ ...target, input: { ...target.input, allowStale: false } }), ].filter((atom) => registry.getNodes().has(atom)); + const loadedMergeQueue = ( + registry: AtomRegistry.AtomRegistry, + target: { readonly environmentId: EnvironmentId; readonly input: PullRequestRef }, + ) => + loadedDetails(registry, target) + .map((atom) => Option.getOrUndefined(loadedValue(registry, atom))) + .find((shown) => shown?.mergeQueue !== undefined)?.mergeQueue; const detailReadVersions = Atom.family((source: ReturnType) => Atom.family((answer: ReturnType) => Atom.make((get) => { @@ -1019,6 +1035,36 @@ export function createPullRequestEnvironmentAtoms( ); } }); + const checksLabel = "environment-data:pull-requests:checks"; + const checksPersistence = persistQuery(Schema.NullOr(PullRequestChecks)); + // Panels take the state from the newest of detail and checks, so checks show writes too. + const checks = normalizePullRequestQuery( + actionPatchedQuery( + createEnvironmentRpcQueryAtomFamily(runtime, { + label: checksLabel, + persistence: checksPersistence, + tag: WS_METHODS.pullRequestsChecks, + execute: (input) => routedRequest(WS_METHODS.pullRequestsChecks, input), + staleTimeMs: 45_000, + refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), + ...(onAnswer.checks === undefined ? {} : { onAnswer: onAnswer.checks }), + }), + { + label: checksLabel, + persistence: checksPersistence, + onAnswer: onAnswer.checks, + concerns: samePullRequest, + patch: (_input, value, _reference, reply) => + value === null || value.state === reply.state || isStalePullRequestReply(value, reply) + ? value + : { ...value, state: reply.state }, + edit: (_input, value, edit) => + value === null || edit.state === undefined || value.state === edit.state + ? value + : { ...value, state: edit.state }, + }, + ), + ); const listLabel = "environment-data:pull-requests:list"; const listPersistence = persistQuery(PullRequestListResult); @@ -1092,17 +1138,7 @@ export function createPullRequestEnvironmentAtoms( detailReadVersion, preview, previewCard, - checks: normalizePullRequestQuery( - createEnvironmentRpcQueryAtomFamily(runtime, { - label: "environment-data:pull-requests:checks", - persistence: persistQuery(Schema.NullOr(PullRequestChecks)), - tag: WS_METHODS.pullRequestsChecks, - execute: (input) => routedRequest(WS_METHODS.pullRequestsChecks, input), - staleTimeMs: 45_000, - refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), - ...(onAnswer.checks === undefined ? {} : { onAnswer: onAnswer.checks }), - }), - ), + checks, activity, threadComments: createEnvironmentRpcCommand(runtime, { label: "environment-data:pull-requests:thread-comments", @@ -1193,14 +1229,19 @@ export function createPullRequestEnvironmentAtoms( Effect.andThen( Effect.sync(() => { if (reply === undefined) - settleEdit(registry, environmentId, actionEdit(input), true); + settleEdit( + registry, + environmentId, + actionEdit(input, loadedMergeQueue(registry, { environmentId, input })), + true, + ); }), ), ), scheduler: commandScheduler, concurrency: serialPerEnvironment, }), - (_registry, { input }) => actionEdit(input), + (registry, target) => actionEdit(target.input, loadedMergeQueue(registry, target)), ), update: showWhileInFlight( createEnvironmentRpcCommand(runtime, { diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index f0f692da3f8da..91c7d2f4f9e20 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -638,6 +638,8 @@ export const PullRequestDetail = Schema.Struct({ /** Absent means the host does not report auto-merge state, not that it is off. */ autoMergeEnabled: Schema.optional(Schema.Boolean), autoMergeMethod: Schema.optional(PullRequestMergeMethod), + /** Whether a merge joins a queue rather than landing at once; absent where the host cannot say. */ + mergeQueue: Schema.optional(Schema.Boolean), workflowApprovalsRequired: Schema.optional(NonNegativeInt), maintainerCanModify: Schema.optional(Schema.Boolean), }); diff --git a/packages/source-control-core/src/server/PullRequestProvider.ts b/packages/source-control-core/src/server/PullRequestProvider.ts index 0c58f64c47a1f..4ce7ed1b897e3 100644 --- a/packages/source-control-core/src/server/PullRequestProvider.ts +++ b/packages/source-control-core/src/server/PullRequestProvider.ts @@ -209,6 +209,7 @@ export interface ProviderChangeRequestDetail extends ProviderChangeRequest { readonly behindBy?: number; readonly autoMergeEnabled?: boolean; readonly autoMergeMethod?: PullRequestMergeMethod; + readonly mergeQueue?: boolean; readonly workflowApprovalsRequired?: number; } diff --git a/packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.ts b/packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.ts index 3a089e65c04bf..47fbfd834ecf6 100644 --- a/packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.ts @@ -381,6 +381,8 @@ export const make = Effect.gen(function* () { ? "up-to-date" : "behind", viewerPermissions: permissions(repo, pr, viewer), + // Forgejo merges at once or refuses; it has no merge queue. + mergeQueue: false, mergeCapabilities: { merge: repo.allow_merge_commits ?? true, squash: repo.allow_squash_merge ?? true, diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index acccedcb40f38..f1533b3f6d79e 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -403,6 +403,7 @@ describe("pull request detail decoding", () => { headRef: null, author: null, autoMergeRequest: null, + isMergeQueueEnabled: false, viewerCanUpdate: true, viewerDidAuthor: false, viewerCanUpdateBranch: false, @@ -558,6 +559,11 @@ describe("pull request detail decoding", () => { expect(detailOf([], { autoMergeRequest: null }).autoMergeEnabled).toBe(false); }); + it("reads whether a merge joins the base branch's merge queue", () => { + expect(detailOf([], { isMergeQueueEnabled: true }).mergeQueue).toBe(true); + expect(detailOf([]).mergeQueue).toBe(false); + }); + it("shows a re-running check once, as the run that is happening now", () => { const detail = detailOf([ inWorkflow({ diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index df4756fcc0dda..1a6862dd50d51 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -1023,6 +1023,7 @@ export function toPullRequestWatchFingerprint( export interface GitHubPullRequestCore extends GitHubPullRequestDetail { readonly maintainerCanModify: boolean; + readonly mergeQueue?: boolean; readonly canEditMaintainerEdits: boolean; readonly reviewRequestTeams: ReadonlyArray; readonly viewerAccess: GitHubViewerAccess & GitHubRepositoryAccess; @@ -1070,6 +1071,7 @@ export function toPullRequestCore(result: PullRequestCore.Result): GitHubPullReq }), headRepositoryNameWithOwner: trimmed(pr.headRepository?.nameWithOwner), maintainerCanModify: pr.maintainerCanModify, + ...(pr.isMergeQueueEnabled === undefined ? {} : { mergeQueue: pr.isMergeQueueEnabled }), canEditMaintainerEdits: pr.state === "OPEN" && pr.isCrossRepository && diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql b/packages/source-control-github/src/server/pullRequestReads.graphql index ef40a4ab46bcd..c5e0f2c3faed7 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql +++ b/packages/source-control-github/src/server/pullRequestReads.graphql @@ -88,6 +88,8 @@ query PullRequestCore( headRefOid isCrossRepository maintainerCanModify + # Older Enterprise servers lack it, as they lack `isRequired`. + isMergeQueueEnabled @include(if: $required) headRepositoryOwner { login } diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql.ts b/packages/source-control-github/src/server/pullRequestReads.graphql.ts index 0a0f1aec68a3a..8306b52da7ff6 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReads.graphql.ts @@ -441,6 +441,8 @@ export const PullRequestCoreRepositoryPullRequestPullRequest = Schema.Struct({ isCrossRepository: Schema.Boolean, /** Indicates whether maintainers can modify the pull request. */ maintainerCanModify: Schema.Boolean, + /** Indicates whether the pull request's base ref has a merge queue enabled. */ + isMergeQueueEnabled: Schema.optionalKey(Schema.Boolean), /** The owner of the repository associated with this pull request's head Ref. */ headRepositoryOwner: Schema.NullOr(RepositoryOwner), /** The repository associated with this pull request's head Ref. */ @@ -503,7 +505,7 @@ export const PullRequestCore = GraphQL.query("PullRequestCore", { { name: "requestedReviewers", body: "Int!" } ], selections: [ - { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed viewerPermission pullRequest(number:$", "number", "){number title url body state isDraft mergeable mergeStateStatus reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository maintainerCanModify headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage endCursor}}}}}}}}"] } + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed viewerPermission pullRequest(number:$", "number", "){number title url body state isDraft mergeable mergeStateStatus reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository maintainerCanModify isMergeQueueEnabled@include(if:$", "required", ")headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage endCursor}}}}}}}}"] } ], fragments: [ { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } From d9655bfe7ee06b46f8c2cd1a3cb286db8078330d Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:32:57 +0100 Subject: [PATCH 09/28] perf(web): hover on a PR row also reads its activity Resting on a PR row now reads its activity too, so the timeline paints on click. The server caches it, so it asks the host nothing new. Claude Opus 5.5 via Claude Code --- ...PullRequestIntentPrefetch.browser.test.tsx | 27 +++++++++---------- .../usePullRequestIntentPrefetch.ts | 21 ++++++++++----- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.browser.test.tsx b/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.browser.test.tsx index 366fb4cfe76e4..81b558f96312f 100644 --- a/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.browser.test.tsx +++ b/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.browser.test.tsx @@ -9,6 +9,7 @@ vi.mock(import("~/state/pullRequests"), async (original) => { pullRequestEnvironment: { ...actual.pullRequestEnvironment, detail: ((target: unknown) => target) as never, + activity: ((target: unknown) => ({ activity: target })) as never, }, }; }); @@ -46,7 +47,7 @@ function Rows() { ); } -it("reads the panel's detail once intent rests on a row for 150 ms", async () => { +it("reads the panel's detail and activity once intent rests on a row for 150 ms", async () => { state.reads = []; await using screen = await render(, { // A bare hook probe; nothing here navigates. @@ -68,19 +69,17 @@ it("reads the panel's detail once intent rests on a row for 150 ms", async () => expect(state.reads).toEqual([]); await act(() => vi.advanceTimersByTime(1)); // Query atoms key on JSON order: this is the order the PRs page panel reads. - expect(JSON.stringify(state.reads.at(-1))).toBe( - JSON.stringify({ - environmentId, - input: { - projectId: row.projectId, - repository: "owner/repo", - number: 7, - host: "github.com", - allowStale: false, - }, - }), - ); + const reference = { + projectId: row.projectId, + repository: "owner/repo", + number: 7, + host: "github.com", + }; + expect(state.reads.slice(-2).map((read) => JSON.stringify(read))).toEqual([ + JSON.stringify({ environmentId, input: { ...reference, allowStale: false } }), + JSON.stringify({ activity: { environmentId, input: reference } }), + ]); await act(() => leave.click()); - expect(state.reads.at(-1)).toBeNull(); + expect(state.reads.slice(-2)).toEqual([null, null]); }); diff --git a/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.ts b/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.ts index a078ce2c94ff7..5d70a208b12a5 100644 --- a/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.ts +++ b/apps/web/src/components/pullRequest/usePullRequestIntentPrefetch.ts @@ -18,8 +18,9 @@ interface PullRequestIntentTarget { } /** - * Reads the detail a row's panel will ask for once the pointer or focus rests on it, so the - * click paints from it. Passing over rows asks the host nothing, and leaving a row stops reading it. + * Reads the detail and activity a row's panel will ask for once the pointer or focus rests on it, + * so the click paints from them. Passing over rows asks the host nothing, and leaving a row stops + * reading it. */ export function usePullRequestIntentPrefetch( isListed: (target: PullRequestIntentTarget) => boolean, @@ -29,17 +30,23 @@ export function usePullRequestIntentPrefetch( // A removed row fires no leave event. const listed = target !== null && isListed(target); if (target !== null && !listed) setTarget(null); - useEnvironmentQuery( + const reference = target === null || !listed + ? null + : pullRequestPanelReference({ ...target, host: target.host.toLowerCase() }); + useEnvironmentQuery( + target === null || reference === null ? null : pullRequestEnvironment.detail({ environmentId: target.environmentId, - input: { - ...pullRequestPanelReference({ ...target, host: target.host.toLowerCase() }), - allowStale: false, - }, + input: { ...reference, allowStale: false }, }), ); + useEnvironmentQuery( + target === null || reference === null + ? null + : pullRequestEnvironment.activity({ environmentId: target.environmentId, input: reference }), + ); return useStableCallback((next: PullRequestIntentTarget | null) => { clearTimeout(timer.current); if (next === null) { From 2ac76f4cdfb90a7b3b35726fe6b183b9d38b768d Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:39:38 +0100 Subject: [PATCH 10/28] test(pr): fold the unknown-queue merge case into the queue-status table The merge left alone where the host cannot say whether it queues is now a row of the table that covers merges that land or queue, rather than its own test. Claude Opus 5.5 via Claude Code --- .../src/state/pullRequests.test.ts | 47 ++----------------- 1 file changed, 5 insertions(+), 42 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 96f2be1ef2d22..e7939323ef548 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2549,46 +2549,6 @@ it.effect("shows a queued close before the action ahead of it answers", () => ), ); -it.effect("leaves a merge as it is until the host says what it did", () => - Effect.scoped( - Effect.gen(function* () { - const sent = yield* Deferred.make(); - const release = yield* Deferred.make(); - const client = actionReplyClient({ - [WS_METHODS.pullRequestsDetail]: () => - Effect.succeed({ ...actionReference, state: "open", observedAt: 100 }), - [WS_METHODS.pullRequestsRunAction]: () => - Effect.gen(function* () { - yield* Deferred.succeed(sent, undefined); - yield* Deferred.await(release); - // A merge queue takes the merge and merges later. - return { ...actionReply, state: "open", autoMergeEnabled: true }; - }), - }); - const { atoms, registry } = yield* makeTestRuntime(client); - const target = { environmentId: TARGET.environmentId, input: actionReference }; - const detail = atoms.detail(target); - registry.mount(detail); - yield* Effect.promise(() => executeAtomQuery(registry, detail)); - - const acting = yield* Effect.promise(() => - atoms.runAction.run(registry, { - ...target, - input: { ...actionReference, action: "merge" }, - }), - ).pipe(Effect.forkScoped); - yield* Deferred.await(sent); - expect(Option.getOrThrow(AsyncResult.value(registry.get(detail))).state).toBe("open"); - yield* Deferred.succeed(release, undefined); - yield* Fiber.join(acting); - expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject({ - state: "open", - autoMergeEnabled: true, - }); - }), - ), -); - it.effect.each(["accepted", "refused"] as const)( "shows a title edit at once and keeps it only when the host takes it (%s)", (outcome) => @@ -2805,6 +2765,7 @@ it.effect.each(["loaded", "loading"] as const)( it.effect.each([ [false, { state: "merged" }], [true, { state: "open", autoMergeEnabled: true }], + [undefined, { state: "open", autoMergeEnabled: true }], ] as const)( "shows a merge the host lands at once or queues before it replies (queue %s)", ([mergeQueue, shown]) => @@ -2852,9 +2813,11 @@ it.effect.each([ ).pipe(Effect.forkScoped); yield* Deferred.await(sent); - expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject(shown); + expect(Option.getOrThrow(AsyncResult.value(registry.get(detail)))).toMatchObject( + mergeQueue === undefined ? { state: "open", autoMergeEnabled: false } : shown, + ); expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries).toHaveLength( - mergeQueue ? 1 : 0, + mergeQueue === false ? 0 : 1, ); yield* Deferred.succeed(release, undefined); yield* Fiber.join(acting); From 5839e47231fba80a0b1415134977ff4861293110 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:40:25 +0100 Subject: [PATCH 11/28] fix(pr): the checks read shows every queued state change The checks reader took one pending edit while the other readers now take all of a pull request's queued edits, so a close or reopen in flight left its state stale and the package no longer typechecked. Claude Opus 5.5 via Claude Code --- packages/client-runtime/src/state/pullRequests.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index a8196366ad8d9..f49b2f99665a6 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -1058,10 +1058,12 @@ export function createPullRequestEnvironmentAtoms( value === null || value.state === reply.state || isStalePullRequestReply(value, reply) ? value : { ...value, state: reply.state }, - edit: (_input, value, edit) => - value === null || edit.state === undefined || value.state === edit.state + edit: (_input, value, edits) => { + const state = edits.findLast((edit) => edit.state !== undefined)?.state; + return value === null || state === undefined || value.state === state ? value - : { ...value, state: edit.state }, + : { ...value, state }; + }, }, ), ); From 150b938a8b3238f2e4820928e9648f339538adba Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 11:54:05 +0100 Subject: [PATCH 12/28] perf(pr): a saved stack paints at once and merging still waits for the host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stack read is saved on the device, so a reopened panel paints its stack instead of 'Checking stack membership…'. Merging one layer alone, or the whole stack, still needs this session's settled host answer: a saved or still-reading answer never clears a merge, on web and mobile. --- .../pull-requests/usePullRequestStack.ts | 9 +++- .../pullRequest/PullRequestStackMenu.tsx | 2 +- .../pullRequestStackSnapshot.test.ts | 10 +++- .../pullRequest/pullRequestStackSnapshot.ts | 5 +- apps/web/src/state/usePullRequestStack.ts | 11 +++- .../src/state/pullRequests.test.ts | 50 +++++++++++++++++++ .../client-runtime/src/state/pullRequests.ts | 20 +++++++- 7 files changed, 99 insertions(+), 8 deletions(-) diff --git a/apps/mobile/src/features/pull-requests/usePullRequestStack.ts b/apps/mobile/src/features/pull-requests/usePullRequestStack.ts index 74a19737ac85d..6efe6143856c5 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestStack.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestStack.ts @@ -1,6 +1,8 @@ import { threadPullRequestLinkMode } from "@t3tools/client-runtime/thread-pull-request-compatibility"; import type { PullRequestDetail } from "@t3tools/contracts"; import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; +import { useAtomValue } from "@effect/atom-react"; +import { Atom } from "effect/reactivity"; import { useMemo } from "react"; import { useEnvironmentServerConfig, useNavigationThreadShells } from "../../state/entities"; import { pullRequestStackAtom } from "../../state/pull-requests"; @@ -8,6 +10,8 @@ import { useEnvironmentQuery } from "../../state/query"; import { savedPullRequestStack } from "./pullRequestStack"; import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; +const NOT_ANSWERED = Atom.make(false); + export function usePullRequestStack(params: PullRequestParams, pr: PullRequestDetail | null) { const target = pullRequestTarget(params); const config = useEnvironmentServerConfig(target.environmentId); @@ -37,11 +41,14 @@ export function usePullRequestStack(params: PullRequestParams, pr: PullRequestDe const query = useEnvironmentQuery( supported ? pullRequestStackAtom({ ...target, input: reference }) : null, ); + const answered = useAtomValue( + supported ? pullRequestStackAtom.answered({ ...target, input: reference }) : NOT_ANSWERED, + ); const stack = query.isSuccess ? query.data : (query.data ?? saved); return { ...query, stack, - isFresh: supported && query.isSuccess && !query.isPending, + isFresh: supported && query.isSuccess && !query.isPending && answered, supportsStackActions: supported && pr?.capabilities.stackActions === true, }; } diff --git a/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx b/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx index 8ed8ba2d8ed33..6575560a4ee51 100644 --- a/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx +++ b/apps/web/src/components/pullRequest/PullRequestStackMenu.tsx @@ -229,7 +229,7 @@ export function PullRequestStackMenu({ /> {stale - ? notice + ? (notice ?? "Checking the stack with the host…") : `Merge stack through #${reference.number} into ${stack.base} (${mergeLayers.length} ${mergeLayers.length === 1 ? terminology.singular : terminology.plural})`} diff --git a/apps/web/src/components/pullRequest/pullRequestStackSnapshot.test.ts b/apps/web/src/components/pullRequest/pullRequestStackSnapshot.test.ts index 4b6f947232b38..87fa54d5cec61 100644 --- a/apps/web/src/components/pullRequest/pullRequestStackSnapshot.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestStackSnapshot.test.ts @@ -88,10 +88,16 @@ describe("saved stack navigation", () => { isFresh: true, notice: null, }); - expect(pullRequestStackView({ ...query, isPending: true }, saved).isFresh).toBe(true); + // A shown answer being read again may be one restored from the device, so it cannot clear a + // merge; it shows as it is, with no refresh notice. + expect(pullRequestStackView({ ...query, isPending: true }, saved)).toEqual({ + data: saved, + isFresh: false, + notice: null, + }); expect(pullRequestStackView({ ...query, data: null, isPending: true }, saved)).toMatchObject({ data: null, - isFresh: true, + isFresh: false, }); }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestStackSnapshot.ts b/apps/web/src/components/pullRequest/pullRequestStackSnapshot.ts index f2f4f8f2de9a3..9dc0c31cfa80c 100644 --- a/apps/web/src/components/pullRequest/pullRequestStackSnapshot.ts +++ b/apps/web/src/components/pullRequest/pullRequestStackSnapshot.ts @@ -59,13 +59,14 @@ export function pullRequestStackView( const data = query.isSuccess ? query.data : (query.data ?? saved); return { data, - isFresh: query.isSuccess, + // A settled read: one still out may be showing an answer restored from the device. + isFresh: query.isSuccess && !query.isPending, notice: data === null ? null : query.error ? "Stack data may be stale. We couldn’t refresh it." - : !query.isSuccess || query.isPending + : !query.isSuccess ? "Refreshing stack… Showing saved data." : null, }; diff --git a/apps/web/src/state/usePullRequestStack.ts b/apps/web/src/state/usePullRequestStack.ts index 99619a5f9ff98..9776ced468dfc 100644 --- a/apps/web/src/state/usePullRequestStack.ts +++ b/apps/web/src/state/usePullRequestStack.ts @@ -10,6 +10,8 @@ import { pullRequestStackAtom } from "./pullRequests"; import { useEnvironmentQuery } from "./query"; import { environmentThreadShells } from "./threads"; +const NOT_ANSWERED = Atom.make(false); + const sameLinks = Schema.toEquivalence(Schema.Array(ThreadPullRequestLink)); // Shell pushes recreate link arrays; reuse equal lists so streaming does not re-render the stack. @@ -34,5 +36,12 @@ export function usePullRequestStack( const query = useEnvironmentQuery( reference === null ? null : pullRequestStackAtom({ environmentId, input: reference }), ); - return { ...query, ...pullRequestStackView(query, saved) }; + const answered = useAtomValue( + reference === null + ? NOT_ANSWERED + : pullRequestStackAtom.answered({ environmentId, input: reference }), + ); + const view = pullRequestStackView(query, saved); + // A saved answer paints; merging waits for this session's own host answer. + return { ...query, ...view, isFresh: view.isFresh && answered }; } diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index e7939323ef548..b41086312647a 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -43,6 +43,7 @@ import { applyPullRequestSummaryEdits, createLinkedPullRequestSummaryAtomFamily, createPullRequestEnvironmentAtoms, + createPullRequestStackAtomFamily, pullRequestPendingEditsAtom, } from "./pullRequests.ts"; import * as PullRequestDiffLoader from "./pullRequestDiffHttp.ts"; @@ -2872,3 +2873,52 @@ it.effect.each(["accepted", "refused"] as const)( }), ), ); + +it.effect("paints a saved stack answer at once while the host read is out", () => + Effect.scoped( + Effect.gen(function* () { + const saved = new Map(); + const store = { + load: (_environmentId: EnvironmentId, key: string) => + Effect.sync(() => Option.fromUndefinedOr(saved.get(key))), + save: (_environmentId: EnvironmentId, key: string, payload: string) => + Effect.sync(() => void saved.set(key, payload)), + }; + const earlier = yield* makeTestRuntime( + { + [WS_METHODS.pullRequestsStack]: () => Effect.succeed(null), + } as unknown as WsRpcProtocolClient, + undefined, + false, + {}, + store, + ); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const stacks = createPullRequestStackAtomFamily(earlier.runtime); + const first = stacks(target); + earlier.registry.mount(first); + yield* Effect.promise(() => executeAtomQuery(earlier.registry, first)); + expect(earlier.registry.get(stacks.answered(target))).toBe(true); + + // A later session finds the answer saved while its own read waits on the host. + const sent = yield* Deferred.make(); + const waiting = yield* makeTestRuntime( + { + [WS_METHODS.pullRequestsStack]: () => + Deferred.succeed(sent, undefined).pipe(Effect.andThen(Effect.never)), + } as unknown as WsRpcProtocolClient, + undefined, + false, + {}, + store, + ); + const waitingStacks = createPullRequestStackAtomFamily(waiting.runtime); + const stack = waitingStacks(target); + waiting.registry.mount(stack); + yield* Deferred.await(sent); + expect(waiting.registry.get(stack)).toMatchObject({ _tag: "Success", value: null }); + // Offline, a saved answer settles without the host; only a host answer may clear a merge. + expect(waiting.registry.get(waitingStacks.answered(target))).toBe(false); + }), + ), +); diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index f49b2f99665a6..ecf3f94d4d036 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -4,6 +4,7 @@ import { PullRequestDetail as PullRequestDetailSchema, PullRequestPreview, PullRequestReviewGuideResult, + PullRequestStack, PullRequestListResult, PullRequestChecks, PullRequestDiffResult, @@ -684,14 +685,31 @@ export function createPullRequestStackAtomFamily( refreshes = createPullRequestRefreshAtomFamily(runtime), ) { const routedRequest = createPullRequestRouter(); - return createEnvironmentRpcQueryAtomFamily(runtime, { + const hostAnswers = Atom.family((_key: string) => + Atom.make(false).pipe(Atom.setIdleTTL(5 * 60_000)), + ); + const answeredKey = (target: { + readonly environmentId: EnvironmentId; + readonly input: PullRequestRef; + }) => JSON.stringify([target.environmentId, target.input]); + const stack = createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:pull-requests:stack", + // A saved answer paints at once; `answered` stays false until the host answers. + persistence: persistQuery(Schema.NullOr(PullRequestStack)), tag: WS_METHODS.pullRequestsStack, execute: (input) => routedRequest(WS_METHODS.pullRequestsStack, input), staleTimeMs: 60_000, idleTtlMs: LINKED_PULL_REQUEST_IDLE_TTL_MS, refreshTrigger: ({ environmentId }) => refreshes({ environmentId, input: {} }), + onAnswer: (target, _value, registry) => + Effect.sync(() => registry.set(hostAnswers(answeredKey(target)), true)), }); + /** Whether this session has the host's own answer, which is what merging one layer alone needs. */ + const answered = (target: { + readonly environmentId: EnvironmentId; + readonly input: PullRequestRef; + }) => hostAnswers(answeredKey(target)); + return Object.assign(stack, { answered }); } export function pullRequestDetailToVcsStatus( From 098d030e0835e3d7915965bba79909b433c18131 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 11:55:46 +0100 Subject: [PATCH 13/28] perf(web): PR links, thread rows and the Code tab read ahead on intent An open PR link card (bare #N included), a thread's linked-PR row under the pointer or focus, and a pointer on the Code tab read what the click will show, under the key the click opens. The panel menu spins only for a refresh the user asked for; row checks share the panel's detail read; linked rows name the PR instead of 'Waiting for host state'. --- .../threads/LinkedPullRequestList.tsx | 4 +- .../PullRequestChangeBreakdownStat.tsx | 2 - ...stDetailPanel.permissions.browser.test.tsx | 45 ++++++++- .../pullRequest/PullRequestDetailPanel.tsx | 14 ++- .../PullRequestLinkPreview.browser.test.tsx | 80 ++++++++++++--- .../pullRequest/PullRequestLinkPreview.tsx | 37 +++++++ .../components/pullRequest/PullRequestRow.tsx | 6 +- .../ThreadPullRequestsPanel.browser.test.tsx | 98 +++++++++++++++++++ .../pullRequest/ThreadPullRequestsPanel.tsx | 60 +++++++++++- apps/web/src/lib/openPullRequestLink.ts | 29 ++++-- 10 files changed, 340 insertions(+), 35 deletions(-) create mode 100644 apps/web/src/components/pullRequest/ThreadPullRequestsPanel.browser.test.tsx diff --git a/apps/mobile/src/features/threads/LinkedPullRequestList.tsx b/apps/mobile/src/features/threads/LinkedPullRequestList.tsx index fe4fddd3e2c37..85e9e7f3fabc5 100644 --- a/apps/mobile/src/features/threads/LinkedPullRequestList.tsx +++ b/apps/mobile/src/features/threads/LinkedPullRequestList.tsx @@ -87,7 +87,7 @@ export const LinkedPullRequestRow = memo(function LinkedPullRequestRow(props: { const row = ( { void props.openPullRequest(link.url).then((opened) => { @@ -161,7 +161,7 @@ export const LinkedPullRequestRow = memo(function LinkedPullRequestRow(props: { numberOfLines={2} ellipsizeMode="tail" > - {snapshot?.title ?? "Waiting for host state"} + {snapshot?.title ?? `${link.repository}#${link.number}`} diff --git a/apps/web/src/components/pullRequest/PullRequestChangeBreakdownStat.tsx b/apps/web/src/components/pullRequest/PullRequestChangeBreakdownStat.tsx index e4d6765fd20dd..f67202ecd44ce 100644 --- a/apps/web/src/components/pullRequest/PullRequestChangeBreakdownStat.tsx +++ b/apps/web/src/components/pullRequest/PullRequestChangeBreakdownStat.tsx @@ -63,8 +63,6 @@ export function PullRequestChangeBreakdownStat({ - ) : query.isPending ? ( - Loading breakdown ) : null} ); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.permissions.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.permissions.browser.test.tsx index ea544da84ecfd..48aead1297e3f 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.permissions.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.permissions.browser.test.tsx @@ -7,11 +7,13 @@ import { type PullRequestDetailView, } from "@t3tools/contracts"; import { expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; import { DraftId, useComposerDraftStore } from "~/composerDraftStore"; import { act, render } from "../../../test/render"; -const { permissions } = vi.hoisted(() => ({ +const { permissions, reads } = vi.hoisted(() => ({ permissions: { checkout: true, operate: true }, + reads: { detailPending: false, diff: [] as unknown[] }, })); vi.mock(import("~/state/session"), async (original) => ({ @@ -33,7 +35,7 @@ vi.mock(import("./usePullRequestDetail"), async (original) => ({ ...(await original()), usePullRequestDetail: (() => ({ canWriteSourceControl: permissions.checkout, - detailQuery: { isPending: false, refresh: vi.fn() }, + detailQuery: { isPending: reads.detailPending, refresh: vi.fn() }, activityQuery: { isPending: false, refresh: vi.fn(), error: null }, activity: null, sharedSummary: detail, @@ -51,6 +53,19 @@ vi.mock(import("~/state/usePullRequestStack"), async (original) => ({ refresh: vi.fn(), })) as never, })); +vi.mock(import("~/state/pullRequests"), async (original) => { + const actual = await original(); + return { + ...actual, + pullRequestEnvironment: { + ...actual.pullRequestEnvironment, + diff: ((target: unknown) => { + reads.diff.push(target); + return actual.pullRequestEnvironment.diff(target as never); + }) as never, + }, + }; +}); vi.mock(import("~/state/use-atom-command"), async (original) => ({ ...(await original()), useAtomCommand: (() => vi.fn()) as never, @@ -133,6 +148,8 @@ const panel = (attach = false) => ( function setup() { permissions.checkout = true; permissions.operate = true; + reads.detailPending = false; + reads.diff = []; useComposerDraftStore.setState({ draftsByThreadKey: {} }); } @@ -162,3 +179,27 @@ it("can still add conflict resolution to an existing composer without checkout s expect(draft?.prompt).toContain("conflicts"); expect(draft?.prompt).toContain(detail.url); }); + +it("keeps the actions menu still while a shown pull request is read again in the background", async () => { + setup(); + reads.detailPending = true; + await using screen = await render(panel()); + await expect.element(screen.getByRole("button", { name: "Resolve conflicts" })).toBeVisible(); + expect(screen.getByRole("button", { name: /^Refreshing / }).query()).toBeNull(); + await expect.element(screen.getByRole("button", { name: /^More .* actions$/ })).toBeVisible(); +}); + +it("reads the first diff page once the pointer rests on the Code tab", async () => { + setup(); + await using screen = await render(panel()); + const code = screen.getByRole("button", { name: "Code", exact: true }); + await expect.element(code).toBeVisible(); + expect(reads.diff).toEqual([]); + + await userEvent.hover(code); + // The page the Code tab reads first, keyed as the tab keys it, so a click paints from it. + expect(reads.diff).toContainEqual({ + environmentId, + input: { projectId: detail.projectId, repository: detail.repository, number: detail.number }, + }); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index c5998929bae38..9f4cffbb0b717 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -412,6 +412,13 @@ export function PullRequestDetailPanel({ } else if (!tabMountState.tabs.has(tab)) { setTabMountState({ key: tabScopeKey, tabs: new Set(tabMountState.tabs).add(tab) }); } + // A pointer on the Code tab reads the page it opens on; only intent costs the host a read. + const [codeIntentKey, setCodeIntentKey] = useState(null); + useEnvironmentQuery( + codeIntentKey === pullRequestKey && !mountedTabs.has("code") + ? pullRequestEnvironment.diff({ environmentId, input: { ...reference } }) + : null, + ); // Each tab restores the condensed state its own scrolling left behind. const [condensedByTab, setCondensedByTab] = useState< Partial> @@ -600,7 +607,8 @@ export function PullRequestDetailPanel({ ); const invalidate = useAtomCommand(pullRequestEnvironment.invalidate, { reportFailure: false }); const [isInvalidating, setIsInvalidating] = useState(false); - const refreshing = isInvalidating || detailQuery.isPending; + // Background reads repaint in place; only a refresh the user asked for shows motion. + const refreshing = isInvalidating; const refreshFromHost = async () => { setIsInvalidating(true); // Atom commands settle without throwing, so reset always runs. @@ -1585,7 +1593,9 @@ export function PullRequestDetailPanel({ value={tab} onValueChange={selectTab} onTabIntent={(item) => { - if (item === "code") void loadCodeTab(); + if (item !== "code") return; + void loadCodeTab(); + setCodeIntentKey(pullRequestKey); }} > {tab === "summary" ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestLinkPreview.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestLinkPreview.browser.test.tsx index 29f0211eaef83..2dc703562f4cb 100644 --- a/apps/web/src/components/pullRequest/PullRequestLinkPreview.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestLinkPreview.browser.test.tsx @@ -17,24 +17,35 @@ const known = vi.hoisted( updatedAt: "2026-09-01T00:00:00Z", }) as PullRequestSummary, ); -const state = vi.hoisted(() => ({ restoredPreview: null as unknown })); +const state = vi.hoisted(() => ({ restoredPreview: null as unknown, reads: [] as unknown[] })); vi.mock(import("~/state/query"), async (original) => ({ ...(await original()), // The host has not answered the preview read yet. - useEnvironmentQuery: ((atom: unknown) => ({ - data: state.restoredPreview, - dataUpdatedAt: 0, - error: null, - failure: null, - isPending: atom !== null, - isSuccess: false, - refresh: () => undefined, - })) as never, -})); -vi.mock(import("~/state/pullRequests"), async (original) => ({ - ...(await original()), - useSharedPullRequestSummary: (() => known) as never, + useEnvironmentQuery: ((atom: unknown) => { + state.reads.push(atom); + return { + data: state.restoredPreview, + dataUpdatedAt: 0, + error: null, + failure: null, + isPending: atom !== null, + isSuccess: false, + refresh: () => undefined, + }; + }) as never, })); +vi.mock(import("~/state/pullRequests"), async (original) => { + const actual = await original(); + return { + ...actual, + useSharedPullRequestSummary: (() => known) as never, + pullRequestEnvironment: { + ...actual.pullRequestEnvironment, + detail: ((target: unknown) => ({ detail: target })) as never, + activity: ((target: unknown) => ({ activity: target })) as never, + }, + }; +}); import { render } from "../../../test/render"; import { PullRequestLinkPreview } from "./PullRequestLinkPreview"; @@ -90,3 +101,44 @@ it("shows the known summary while the preview loads", async () => { await expect.element(page.getByText("Known title")).toBeVisible(); }); + +it.each([ + ["a pull request URL", "https://github.com/owner/repo/pull/1"], + // GitHub autolinks a bare #1 to its issues URL, which the panel opens as the pull request. + ["a bare reference", "https://github.com/owner/repo/issues/1"], +])("reads the panel's detail and activity once the preview of %s opens", async (_case, url) => { + state.restoredPreview = null; + state.reads = []; + await using screen = await render( + #1} + originalUrl={url} + target={{ + environmentId: EnvironmentId.make("environment"), + input: { projectId: ProjectId.make("project"), repository: "owner/repo", number: 1 }, + }} + />, + ); + expect( + state.reads.filter( + (read) => + read !== null && typeof read === "object" && ("detail" in read || "activity" in read), + ), + ).toEqual([]); + + await userEvent.hover(screen.getByRole("link")); + await expect.element(page.getByText("Known title")).toBeVisible(); + + // The panel a click opens reads the link's own host and repository. + const reference = { + projectId: ProjectId.make("project"), + host: "github.com", + repository: "owner/repo", + number: 1, + }; + const environmentId = EnvironmentId.make("environment"); + expect(state.reads).toContainEqual({ + detail: { environmentId, input: { ...reference, allowStale: false } }, + }); + expect(state.reads).toContainEqual({ activity: { environmentId, input: reference } }); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx b/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx index 8e3ee75602342..886a3973876dd 100644 --- a/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx +++ b/apps/web/src/components/pullRequest/PullRequestLinkPreview.tsx @@ -1,4 +1,5 @@ import type { PreviewCard as PreviewCardPrimitive } from "@base-ui/react/preview-card"; +import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; import { isAtomCommandInterrupted } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, PullRequestRef } from "@t3tools/contracts"; import { @@ -12,6 +13,7 @@ import { } from "react"; import { RelativeTime } from "~/components/RelativeTime"; +import { useClientSettings } from "~/hooks/useSettings"; import { pullRequestEnvironment, useSharedPullRequestSummary } from "~/state/pullRequests"; import { useAtomQueryRunner } from "~/state/use-atom-query-runner"; import { useEnvironmentQuery } from "~/state/query"; @@ -19,6 +21,7 @@ import { useEnvironmentQuery } from "~/state/query"; import { PreviewCard, PreviewCardPopup, PreviewCardTrigger } from "../ui/preview-card"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { PullRequestActorAvatar, resolvePullRequestState } from "./pullRequestPresentation"; +import { pullRequestPanelReference } from "./usePullRequestDetail"; interface PullRequestLinkPreviewTarget { readonly environmentId: EnvironmentId; @@ -61,6 +64,40 @@ export function PullRequestLinkPreview({ open ? target.environmentId : null, open ? target.input : null, ); + // An open card is intent to click: read what the panel will, keyed as a click names it. + const opensInApp = useClientSettings((settings) => settings.pullRequestLinksOpenInApp); + const linked = + open && opensInApp + ? sourceControlClients.parseChangeRequestUrl( + // A bare #N links to its issues URL; the panel opens it as the change request. + sourceControlClients.changeRequestUrlFromReferenceAutolink(originalUrl) ?? originalUrl, + )?.link + : undefined; + const panelReference = + linked === undefined + ? null + : pullRequestPanelReference({ + projectId: target.input.projectId, + repository: linked.repository, + number: linked.number, + host: linked.authority ?? linked.host, + }); + useEnvironmentQuery( + panelReference === null + ? null + : pullRequestEnvironment.detail({ + environmentId: target.environmentId, + input: { ...panelReference, allowStale: false }, + }), + ); + useEnvironmentQuery( + panelReference === null + ? null + : pullRequestEnvironment.activity({ + environmentId: target.environmentId, + input: panelReference, + }), + ); const readPreview = useAtomQueryRunner(pullRequestEnvironment.preview, { reportFailure: false, reportDefect: false, diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index 71fbf1100d554..142edcbc3cbe8 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -14,6 +14,7 @@ import { getSourceControlPresentationForKind } from "~/sourceControlPresentation import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; +import { pullRequestPanelReference } from "./usePullRequestDetail"; import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; import { @@ -164,9 +165,8 @@ function PullRequestRowImpl({ checksState={entry.checksState} environmentId={entry.environmentId} reference={{ - projectId: entry.projectId, - repository: entry.repository, - number: entry.number, + ...pullRequestPanelReference({ ...entry, host: entry.host.toLowerCase() }), + allowStale: false, }} /> )} diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.browser.test.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.browser.test.tsx new file mode 100644 index 0000000000000..857481c7b9026 --- /dev/null +++ b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.browser.test.tsx @@ -0,0 +1,98 @@ +import { EnvironmentId, ThreadId, type ThreadPullRequestLink } from "@t3tools/contracts"; +import { scopeThreadRef } from "@t3tools/client-runtime/environment"; +import { expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; + +const environmentId = EnvironmentId.make("local"); +const state = vi.hoisted(() => ({ reads: [] as unknown[] })); +const project = vi.hoisted(() => ({ + id: "project", + environmentId: "local", + workspaceRoot: "/repo", + repositoryIdentity: { + provider: "github", + canonicalKey: "github.com/effect-ts/effect", + displayName: "Effect-TS/effect", + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "https://github.com/Effect-TS/effect.git", + }, + }, +})); +const link = vi.hoisted((): ThreadPullRequestLink => ({ + host: "github.com", + repository: "Effect-TS/effect", + number: 7, + url: "https://github.com/Effect-TS/effect/pull/7", + source: "manual", + linkedAt: "2026-09-09T10:00:00Z", + snapshot: null, + stack: null, +})); +vi.mock("~/state/entities", async (original) => ({ + ...(await original()), + readProjects: () => [project], + useProjects: () => [project], + useThreadShell: () => ({ projectId: "project", pullRequests: [link] }), + useServerConfigs: () => new Map([["local", { environment: { capabilities: {} } }]]), +})); +vi.mock("~/state/server", async (original) => { + const { Atom } = await import("effect/reactivity"); + return { + ...(await original()), + environmentServerConfigsAtom: Atom.make(new Map([["local", {}]])), + }; +}); +vi.mock(import("~/state/pullRequests"), async (original) => { + const actual = await original(); + return { + ...actual, + pullRequestEnvironment: { + ...actual.pullRequestEnvironment, + detail: ((target: unknown) => ({ detail: target })) as never, + activity: ((target: unknown) => ({ activity: target })) as never, + }, + }; +}); +vi.mock(import("~/state/query"), async (original) => ({ + ...(await original()), + useEnvironmentQuery: ((atom: unknown) => { + if (atom !== null && typeof atom === "object" && "detail" in atom) state.reads.push(atom); + return { data: null, isPending: false, error: null, refresh: () => undefined }; + }) as never, +})); + +import { act, fakeTimers, render } from "../../../test/render"; +import { AppAtomRegistryProvider } from "../../rpc/atomRegistry"; +import { ThreadPullRequestsPanel } from "./ThreadPullRequestsPanel"; + +it("reads a linked pull request's panel once the pointer rests on its row", async () => { + state.reads = []; + await using screen = await render( + + + , + ); + using _timers = fakeTimers({ now: 0 }); + const row = screen.getByRole("link", { name: /#7/ }); + await expect.element(row).toBeVisible(); + expect(state.reads).toEqual([]); + + await userEvent.hover(row); + await act(() => vi.advanceTimersByTime(150)); + // The key a click on the link opens the panel with. + expect(state.reads).toContainEqual({ + detail: { + environmentId, + input: { + projectId: "project", + // A click resolves the link's URL, which names the repository in lower case. + repository: "effect-ts/effect", + number: 7, + host: "github.com", + allowStale: false, + }, + }, + }); +}); diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx index 4b1ccae1597a0..851f5cfde9e3a 100644 --- a/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx +++ b/apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx @@ -1,5 +1,6 @@ import type { ProjectId, ScopedThreadRef, ThreadPullRequestLink } from "@t3tools/contracts"; import { + capitalizeTerm, sourceControlClients, UNKNOWN_SOURCE_CONTROL_CLIENT, } from "@t3tools/client-runtime/source-control-clients"; @@ -26,7 +27,10 @@ import { } from "~/components/chat/composerMentionDrag"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; import { findProjectForChangeRequest } from "@t3tools/client-runtime/change-request-links"; -import { useOpenPrLink } from "~/lib/openPullRequestLink"; +import { pullRequestLinkPanelTarget, useOpenPrLink } from "~/lib/openPullRequestLink"; +import { useClientSettings } from "~/hooks/useSettings"; +import { usePrimaryEnvironmentId } from "~/state/environments"; +import { usePullRequestIntentPrefetch } from "./usePullRequestIntentPrefetch"; import { cn } from "~/lib/utils"; import { useShortcutModifierState } from "~/shortcutModifierState"; import { useProjects, useServerConfigs, useThreadShell } from "~/state/entities"; @@ -92,6 +96,7 @@ function LinkRowImpl({ speedMode, onUnlink, onSetWatching, + onIntent, }: { line: PullRequestListLine; threadRef: ScopedThreadRef; @@ -99,6 +104,7 @@ function LinkRowImpl({ speedMode: boolean; onUnlink: (link: ThreadPullRequestLink) => void; onSetWatching: ((link: ThreadPullRequestLink, watching: boolean) => void) | null; + onIntent: (link: ThreadPullRequestLink | null) => void; }) { const openPrLink = useOpenPrLink(threadRef); const [menuOpen, setMenuOpen] = useState(false); @@ -131,6 +137,14 @@ function LinkRowImpl({ return (
onIntent(link)} + onPointerLeave={(event) => { + if (!event.currentTarget.contains(document.activeElement)) onIntent(null); + }} + onFocus={() => onIntent(link)} + onBlur={(event) => { + if (!event.currentTarget.matches(":hover")) onIntent(null); + }} onContextMenu={(event) => { event.preventDefault(); event.stopPropagation(); @@ -142,7 +156,7 @@ function LinkRowImpl({ {depth > 0 ? : null} {snapshot === null ? ( ) : ( @@ -397,6 +411,37 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread const unlink = useAtomCommand(threadEnvironment.unlinkPullRequest, { reportFailure: true }); const watch = useAtomCommand(threadEnvironment.watchPullRequest, { reportFailure: true }); const capabilities = useServerConfigs().get(threadRef.environmentId)?.environment.capabilities; + const primaryEnvironmentId = usePrimaryEnvironmentId(); + const opensInApp = useClientSettings((settings) => settings.pullRequestLinksOpenInApp); + const prefetch = usePullRequestIntentPrefetch((target) => + links.some( + (link) => + link.number === target.number && + link.repository.toLowerCase() === target.repository.toLowerCase(), + ), + ); + const intendLink = (link: ThreadPullRequestLink | null) => { + const target = + link === null || !opensInApp + ? null + : pullRequestLinkPanelTarget({ + url: link.url, + environmentId: threadRef.environmentId, + primaryEnvironmentId, + lendHostProject: true, + }); + prefetch( + target?.host === undefined + ? null + : { + environmentId: target.project.environmentId, + projectId: target.project.id, + host: target.host, + repository: target.repository, + number: target.number, + }, + ); + }; const supportsWatch = capabilities?.threadPullRequestWatch === true; const handleUnlink = (link: ThreadPullRequestLink) => { void unlink({ @@ -423,7 +468,15 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread }; // Legend rows update only for data/extraData changes; include all external inputs. - const rowInputs = { speedMode, environmentProjects, capabilities, supportsWatch, thread }; + const rowInputs = { + speedMode, + environmentProjects, + capabilities, + supportsWatch, + thread, + opensInApp, + primaryEnvironmentId, + }; if (linkCount === 0) { return ( @@ -465,6 +518,7 @@ function EnabledThreadPullRequestsPanel({ threadRef }: { threadRef: ScopedThread speedMode={speedMode} onUnlink={handleUnlink} onSetWatching={supportsWatch ? handleSetWatching : null} + onIntent={intendLink} /> )} estimatedItemSize={44} diff --git a/apps/web/src/lib/openPullRequestLink.ts b/apps/web/src/lib/openPullRequestLink.ts index 31c04b6263e5f..7424b2315e663 100644 --- a/apps/web/src/lib/openPullRequestLink.ts +++ b/apps/web/src/lib/openPullRequestLink.ts @@ -39,6 +39,27 @@ export function usePullRequestPreviewTarget(environmentId: EnvironmentId | null, }); } +/** Where a click on this link opens the in-app panel, read at call time as the click reads it. */ +export function pullRequestLinkPanelTarget(input: { + readonly url: string; + readonly environmentId: EnvironmentId | undefined; + readonly primaryEnvironmentId: EnvironmentId | null; + readonly lendHostProject: boolean; +}) { + // Read at click time: every provider status push replaces the config map, and + // subscribing would hand every rendered link a new handler. Projects likewise, so a + // link can resolve against a snapshot that arrived after this handler was built. + const serverConfigs = appAtomRegistry.get(environmentServerConfigsAtom); + return resolveChangeRequestLinkTarget({ + url: input.url, + projects: readProjects(), + configLoaded: (environmentId) => serverConfigs.has(environmentId), + environmentId: input.environmentId, + primaryEnvironmentId: input.primaryEnvironmentId, + lendHostProject: input.lendHostProject, + }); +} + function shouldOpenPullRequestExternally( event: Pick, "metaKey" | "ctrlKey"> & Partial, "button">>, @@ -75,14 +96,8 @@ export function useOpenChangeRequestLink( const resolvedThreadRef = targetThreadRef ?? threadRef; const resolvedPanelRef = panelRef ?? resolvedThreadRef; const openPullRequest = () => { - // Read at click time: every provider status push replaces the config map, and - // subscribing would hand every rendered link a new handler. Projects likewise, so a - // link can resolve against a snapshot that arrived after this handler was built. - const serverConfigs = appAtomRegistry.get(environmentServerConfigsAtom); - const target = resolveChangeRequestLinkTarget({ + const target = pullRequestLinkPanelTarget({ url: targetUrl, - projects: readProjects(), - configLoaded: (environmentId) => serverConfigs.has(environmentId), environmentId: resolvedThreadRef?.environmentId ?? targetEnvironmentId, primaryEnvironmentId, lendHostProject: resolvedPanelRef !== undefined, From 52ad114457f14abc9acb8d08b36c1f156f6564b9 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 15:26:28 +0100 Subject: [PATCH 14/28] fix(pr): an accepted edit ends on the host's own title When a read during a title or description save shows different text, the panel reads that PR again, so a title the host trimmed or rewrote replaces the typed one at once. Co-Authored-By: Claude Opus 5.5 --- .../src/state/pullRequests.test.ts | 63 +++++++++++++++++++ .../client-runtime/src/state/pullRequests.ts | 55 +++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index b41086312647a..7386bbe5ffb87 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2642,6 +2642,69 @@ it.effect.each(["accepted", "refused"] as const)( ), ); +it.effect.each(["rewrites the title", "answers a read before it commits"] as const)( + "ends on the host's own title when a read answers during an accepted edit (host %s)", + (timing) => + Effect.scoped( + Effect.gen(function* () { + let title = "Cached"; + const sent = yield* Deferred.make(); + const release = yield* Deferred.make(); + const read = () => Effect.sync(() => ({ ...actionReference, state: "open", title })); + const client = actionReplyClient({ + [WS_METHODS.pullRequestsDetail]: read, + [WS_METHODS.pullRequestsPreview]: read, + [WS_METHODS.pullRequestsList]: () => + Effect.sync(() => + listResult([ + { ...actionReference, state: "open", title, updatedAt: actionReply.updatedAt }, + ]), + ), + [WS_METHODS.pullRequestsUpdate]: (input: { readonly title: string }) => + Effect.gen(function* () { + title = timing === "rewrites the title" ? `Draft: ${input.title}` : "Original"; + yield* Deferred.succeed(sent, undefined); + yield* Deferred.await(release); + if (timing !== "rewrites the title") title = input.title; + }), + }); + const { atoms, registry } = yield* makeTestRuntime(client); + const target = { environmentId: TARGET.environmentId, input: actionReference }; + const readers: ReadonlyArray< + Atom.Atom> + > = [atoms.detail(target), atoms.preview(target)]; + // Pages past the first are not refreshed by the server. + const page = atoms.list({ + environmentId: target.environmentId, + input: { state: "open", cursors: { "github.example.com": "page-2" } }, + }); + const loaded: ReadonlyArray>> = [ + ...readers, + page, + ]; + for (const atom of loaded) { + const unmount = registry.mount(atom); + yield* Effect.addFinalizer(() => Effect.sync(unmount)); + yield* Effect.promise(() => executeAtomQuery(registry, atom)); + } + const editing = yield* Effect.promise(() => + atoms.update.run(registry, { ...target, input: { ...actionReference, title: "Edited" } }), + ).pipe(Effect.forkScoped); + yield* Deferred.await(sent); + for (const atom of loaded) + yield* Effect.promise(() => executeAtomQuery(registry, atom, { refresh: true })); + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(editing); + // Readers whose title changed during the write read it again. + for (const atom of loaded) yield* Effect.promise(() => executeAtomQuery(registry, atom)); + expect([ + ...readers.map((atom) => Option.getOrThrow(AsyncResult.value(registry.get(atom))).title), + Option.getOrThrow(AsyncResult.value(registry.get(page))).entries[0]?.title, + ]).toEqual(Array(3).fill(title)); + }), + ), +); + it.effect("shows a label change at once and takes it back when the host refuses it", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index ecf3f94d4d036..0f6df29c3b7ef 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -291,12 +291,53 @@ function showWhileInFlight; } +function editedResults( + registry: AtomRegistry.AtomRegistry, + environmentId: EnvironmentId, + edit: PullRequestPendingEdit | null, +) { + const results = new Map>(); + if (edit === null) return results; + for (const [atom, node] of registry.getNodes()) { + const reader = typeof atom === "string" ? undefined : editableReaders.get(atom); + if (reader?.environmentId !== environmentId || !reader.concerns(edit.reference)) continue; + const result = Option.getOrUndefined(node.valueOption()); + if (result !== undefined) + results.set(atom, result as AsyncResult.AsyncResult); + } + return results; +} + +function editedText( + value: unknown, + reference: PullRequestRef, +): { readonly title?: string; readonly body?: string } | undefined { + const held = value as { + readonly entries?: ReadonlyArray; + readonly title?: string; + readonly body?: string; + } | null; + if (held?.entries === undefined) return held ?? undefined; + return held.entries.find((entry) => samePullRequest(entry, reference)); +} + +/** Whether the edited text changed while the write was out. */ +function hostRewroteEdit(sent: unknown, shown: unknown, edit: PullRequestPendingEdit) { + const before = editedText(sent, edit.reference); + const after = editedText(shown, edit.reference); + if (before === undefined || after === undefined) return false; + return (["title", "body"] as const).some( + (field) => edit[field] !== undefined && before[field] !== after[field], + ); +} + /** Keeps an accepted edit in loaded readers once it stops being in flight. */ function settleEdit( registry: AtomRegistry.AtomRegistry, environmentId: EnvironmentId, edit: PullRequestPendingEdit | null, readsAfterWrite = false, + atSend?: ReadonlyMap>, ) { if (edit === null) return; for (const atom of registry.getNodes().keys()) { @@ -309,10 +350,16 @@ function settleEdit( // A read still waiting may predate the write, unless the caller started it after the commit. if (!readsAfterWrite && registry.get(writable).waiting) registry.refresh(writable); if (Option.isNone(shown)) continue; + // Text that changed during the write may be the host's own version of it, or an older read. + const sent = atSend?.get(atom); + const changed = + sent !== undefined && + hostRewroteEdit(Option.getOrUndefined(AsyncResult.value(sent)), shown.value, edit); registry.update( writable, AsyncResult.map((value) => reader.edit(value, [edit]) as never), ); + if (changed) registry.refresh(writable); } } @@ -1268,9 +1315,13 @@ export function createPullRequestEnvironmentAtoms( label: "environment-data:pull-requests:update", tag: WS_METHODS.pullRequestsUpdate, execute: (input) => routedRequest(WS_METHODS.pullRequestsUpdate, input), - onSuccess: (target, registry) => { + onStart: ({ environmentId, input }, registry) => + editedResults(registry, environmentId, updateEdit(input)), + onSuccess: (target, registry, _reply, atSend) => { const edit = updateEdit(target.input); - return Effect.sync(() => settleEdit(registry, target.environmentId, edit)).pipe( + return Effect.sync(() => + settleEdit(registry, target.environmentId, edit, false, atSend), + ).pipe( Effect.andThen(() => edit === null ? Effect.void From bccd49ee95a84758b2495eff4c6c95b850f4fd75 Mon Sep 17 00:00:00 2001 From: Bil0000 <62337003+Bil0000@users.noreply.github.com> Date: Sun, 8 Nov 2026 18:52:47 +0100 Subject: [PATCH 15/28] perf(pr): a first visit to the PR list paints from a read made ahead --- apps/mobile/src/App.tsx | 2 + .../pull-requests/PullRequestListWarmup.tsx | 28 ++++++ .../pull-requests/PullRequestsScreen.tsx | 32 +------ .../usePullRequestListControls.ts | 46 +++++++++- apps/web/src/components/AppSidebarLayout.tsx | 2 + .../pullRequest/PullRequestListWarmup.tsx | 26 ++++++ .../pullRequest/PullRequestsPage.tsx | 19 ++--- .../pullRequest/usePullRequestListView.ts | 41 +++++++-- .../src/state/pullRequests.test.ts | 64 ++++++++++++++ .../client-runtime/src/state/pullRequests.ts | 85 ++++++++++++------- 10 files changed, 264 insertions(+), 81 deletions(-) create mode 100644 apps/mobile/src/features/pull-requests/PullRequestListWarmup.tsx create mode 100644 apps/web/src/components/pullRequest/PullRequestListWarmup.tsx diff --git a/apps/mobile/src/App.tsx b/apps/mobile/src/App.tsx index 6287a05956519..02a5439cd68a2 100644 --- a/apps/mobile/src/App.tsx +++ b/apps/mobile/src/App.tsx @@ -75,6 +75,7 @@ import { import { useMobileNavigationTheme } from "./lib/useMobileNavigationTheme"; import { installUiRuntimeMemoryWarningGc } from "./lib/useUiRuntimeMemoryWarningGc"; import { SubscriptionUsageCoordinator } from "./widgets/SubscriptionUsageCoordinator"; +import { PullRequestListWarmup } from "./features/pull-requests/PullRequestListWarmup"; import { installSubscriptionUsageRefresh } from "./widgets/useSubscriptionUsage"; import { GlobalVoiceInputControl } from "./features/voice-input/GlobalVoiceInputControl"; import { restoreSettingsLinkState } from "./features/settings/settings-navigation"; @@ -243,6 +244,7 @@ function AppContent() { <> + diff --git a/apps/mobile/src/features/pull-requests/PullRequestListWarmup.tsx b/apps/mobile/src/features/pull-requests/PullRequestListWarmup.tsx new file mode 100644 index 0000000000000..8a426a2f10499 --- /dev/null +++ b/apps/mobile/src/features/pull-requests/PullRequestListWarmup.tsx @@ -0,0 +1,28 @@ +import { useAtomMount, useAtomValue } from "@effect/atom-react"; +import type { EnvironmentId, PullRequestListInput } from "@t3tools/contracts"; +import { AsyncResult } from "effect/reactivity"; + +import { mobilePreferencesAtom } from "../../state/preferences"; +import { composerPullRequests } from "../../state/pull-requests"; +import { useEnabledNavigationTabs } from "../layout/workspace/navigation-tabs"; +import { usePullRequestListTargets } from "./usePullRequestListControls"; + +/** Fills an empty saved list for the scope the PRs tab opens on, so its first visit paints. */ +export function PullRequestListWarmup() { + // Until the saved filters load, the tab's scope is not known and a read could be wasted. + const loaded = AsyncResult.isSuccess(useAtomValue(mobilePreferencesAtom)); + const enabled = useEnabledNavigationTabs().includes("pullRequests"); + return loaded && enabled ? : null; +} + +function WarmLists() { + const { targets } = usePullRequestListTargets(""); + return targets.map((target) => ); +} + +function WarmList(props: { + target: { readonly environmentId: EnvironmentId; readonly input: PullRequestListInput }; +}) { + useAtomMount(composerPullRequests.warmList(props.target)); + return null; +} diff --git a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx index 4a4dec4e7a6bc..18bf2281e4716 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestsScreen.tsx @@ -29,16 +29,14 @@ import { type PullRequestFilterMenu, } from "./PullRequestListFilters"; import { usePullRequestList } from "./usePullRequestList"; -import { usePullRequestListControls } from "./usePullRequestListControls"; +import { usePullRequestListTargets } from "./usePullRequestListControls"; import { usePullRequestListActions } from "./usePullRequestListActions"; import { filterPullRequestList, pullRequestListFacets, - pullRequestListInput, pullRequestListRowsAreEqual, scopedPullRequestListKey as pullRequestKey, type EnvironmentPullRequestListEntry, - withoutScratchProjectScope, pullRequestProjectChoices, } from "./pullRequestList"; import { useDebouncedValue } from "../../state/queries"; @@ -120,11 +118,7 @@ export function PullRequestsScreen( const query = tab.searchMode === "resting" ? "" : debouncedQuery; const typedQuery = tab.searchMode === "resting" ? "" : tab.query; const typedDraft = parsePullRequestQuery(typedQuery).filters.draft; - const listControls = usePullRequestListControls(); - const selectedEnvironmentId = listControls.environmentId; - const environmentId = environments.some((item) => item.environmentId === selectedEnvironmentId) - ? selectedEnvironmentId - : null; + const { listControls, environmentId, controls, targets } = usePullRequestListTargets(query); const selectDetail = useSelectWorkspaceDetail(); const selectPullRequest = (item: EnvironmentPullRequestListEntry) => { Keyboard.dismiss(); @@ -139,28 +133,6 @@ export function PullRequestsScreen( selectDetail({ name: "PullRequest", params }); }; const allProjects = useProjects(); - const savedControls = listControls.controlsFor(environmentId); - const controls = useMemo( - () => withoutScratchProjectScope(savedControls, allProjects), - [allProjects, savedControls], - ); - const targets = useMemo( - () => - environments - .filter( - (environment) => - configs.get(environment.environmentId) !== undefined && - (environmentId === null || environment.environmentId === environmentId) && - (!controls.projectEnvironmentId || - environment.environmentId === controls.projectEnvironmentId), - ) - .sort((left, right) => left.environmentId.localeCompare(right.environmentId)) - .map((environment) => ({ - environmentId: environment.environmentId, - input: pullRequestListInput(controls, query), - })), - [environments, environmentId, configs, controls, query], - ); const result = usePullRequestList(targets); const connectedTargets = targets.filter((target) => environments.some( diff --git a/apps/mobile/src/features/pull-requests/usePullRequestListControls.ts b/apps/mobile/src/features/pull-requests/usePullRequestListControls.ts index 6b97390908abc..e9a583e05ee6a 100644 --- a/apps/mobile/src/features/pull-requests/usePullRequestListControls.ts +++ b/apps/mobile/src/features/pull-requests/usePullRequestListControls.ts @@ -1,8 +1,17 @@ import { useAtomSet, useAtomValue } from "@effect/atom-react"; import { AsyncResult } from "effect/reactivity"; +import { useMemo } from "react"; +import { useProjects } from "../../state/entities"; import { mobilePreferencesAtom, updateMobilePreferencesAtom } from "../../state/preferences"; -import { DEFAULT_PR_LIST_CONTROLS, type PullRequestListControls } from "./pullRequestList"; +import { environmentServerConfigsAtom } from "../../state/server"; +import { useWorkspaceState } from "../../state/workspace"; +import { + DEFAULT_PR_LIST_CONTROLS, + pullRequestListInput, + withoutScratchProjectScope, + type PullRequestListControls, +} from "./pullRequestList"; export function usePullRequestListControls() { const preferences = useAtomValue(mobilePreferencesAtom); @@ -45,3 +54,38 @@ function withEnvironmentControls( ): Record { return { ...saved, [environment]: controls }; } + +/** The list reads for the saved controls; the PRs tab and its warm-up share these keys. */ +export function usePullRequestListTargets(query: string) { + const { environments } = useWorkspaceState(); + const configs = useAtomValue(environmentServerConfigsAtom); + const allProjects = useProjects(); + const listControls = usePullRequestListControls(); + const selectedEnvironmentId = listControls.environmentId; + const environmentId = environments.some((item) => item.environmentId === selectedEnvironmentId) + ? selectedEnvironmentId + : null; + const savedControls = listControls.controlsFor(environmentId); + const controls = useMemo( + () => withoutScratchProjectScope(savedControls, allProjects), + [allProjects, savedControls], + ); + const targets = useMemo( + () => + environments + .filter( + (environment) => + configs.get(environment.environmentId) !== undefined && + (environmentId === null || environment.environmentId === environmentId) && + (!controls.projectEnvironmentId || + environment.environmentId === controls.projectEnvironmentId), + ) + .sort((left, right) => left.environmentId.localeCompare(right.environmentId)) + .map((environment) => ({ + environmentId: environment.environmentId, + input: pullRequestListInput(controls, query), + })), + [environments, environmentId, configs, controls, query], + ); + return { listControls, environmentId, controls, targets }; +} diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx index f9cb8c5170cf9..97138d221147d 100644 --- a/apps/web/src/components/AppSidebarLayout.tsx +++ b/apps/web/src/components/AppSidebarLayout.tsx @@ -39,6 +39,7 @@ import { } from "../panelAnimations"; import { BackgroundThreadAnnouncer } from "./BackgroundThreadAnnouncer"; import { HistorySwipeNavigation } from "./HistorySwipeNavigation"; +import { PullRequestListWarmup } from "./pullRequest/PullRequestListWarmup"; import ThreadSidebar from "./Sidebar"; import { SidebarBrandWidthProbe } from "./sidebar/SidebarChrome"; import { isAppModalPage } from "./sidebar/mainAppLocation"; @@ -488,6 +489,7 @@ export function AppSidebarLayout({ ) : null} {children} + {isElectron ? : null} diff --git a/apps/web/src/components/pullRequest/PullRequestListWarmup.tsx b/apps/web/src/components/pullRequest/PullRequestListWarmup.tsx new file mode 100644 index 0000000000000..2244f8fad7051 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestListWarmup.tsx @@ -0,0 +1,26 @@ +import { useAtomMount } from "@effect/atom-react"; +import type { EnvironmentId, PullRequestListInput } from "@t3tools/contracts"; +import { useState } from "react"; + +import { pullRequestEnvironment } from "~/state/pullRequests"; + +import { readPullRequestListPreferences } from "./pullRequestListPreferences"; +import { pullRequestBaselineTargets, usePullRequestScope } from "./usePullRequestListView"; + +/** Fills an empty saved list for the scope the PRs page opens on, so its first visit paints. */ +export function PullRequestListWarmup() { + const [preferences] = useState(readPullRequestListPreferences); + const scope = usePullRequestScope(preferences); + // Before the projects load the page asks with another key, which would cost a second read. + if (!scope.projectsKnown) return null; + return pullRequestBaselineTargets(preferences, scope).map((target) => ( + + )); +} + +function WarmList(props: { + target: { readonly environmentId: EnvironmentId; readonly input: PullRequestListInput }; +}) { + useAtomMount(pullRequestEnvironment.warmList(props.target)); + return null; +} diff --git a/apps/web/src/components/pullRequest/PullRequestsPage.tsx b/apps/web/src/components/pullRequest/PullRequestsPage.tsx index 2af8383bebcd4..a7b63ae8860f8 100644 --- a/apps/web/src/components/pullRequest/PullRequestsPage.tsx +++ b/apps/web/src/components/pullRequest/PullRequestsPage.tsx @@ -68,6 +68,8 @@ import { PullRequestListGroups } from "./PullRequestListGroups"; import { pullRequestPanelReference } from "./usePullRequestDetail"; import { usePullRequestIntentPrefetch } from "./usePullRequestIntentPrefetch"; import { + PAGE_SIZE, + pullRequestBaselineTargets, usePullRequestDisplayGroups, usePullRequestQueryFilters, usePullRequestScope, @@ -133,7 +135,6 @@ function getShortcutContext() { * its "is there more" probe, and GitHub serves a hundred per request — so asking for ninety-nine * costs one round trip where a hundred costs two. */ -const PAGE_SIZE = 99; const MAX_PAGE_SIZE = 500; const EMPTY_VIEWERS: PullRequestListResult["viewers"] = {}; const EMPTY_PARTITIONS: PullRequestPartitionsSnapshot = { authored: [], reviewing: [] }; @@ -337,18 +338,10 @@ export function PullRequestsPage() { * say which question it belongs to: mid-switch the text has already changed and the data has * not, and a search's answer would file itself under the workspace. */ - const baselineTargets = environmentQueries.map(({ environmentId, projectIds }) => ({ - environmentId, - input: { - state: search.state, - involvement: search.involvement, - limit: PAGE_SIZE, - ...(scopedProjectId ? { projectId: scopedProjectId } : {}), - ...(projectIds ? { projectIds } : {}), - ...(search.host ? { host: search.host } : {}), - ...(menuFiltered ? { filters: menuFilters } : {}), - } satisfies PullRequestListInput, - })); + const baselineTargets = pullRequestBaselineTargets(search, { + environmentQueries, + scopedProjectId, + }); const baselineQuery = usePullRequestList(baselineTargets); const baselineEmpty = baselineQuery.data?.entries.length === 0 && diff --git a/apps/web/src/components/pullRequest/usePullRequestListView.ts b/apps/web/src/components/pullRequest/usePullRequestListView.ts index fe22fe5af2838..ca9bdaebdcd4a 100644 --- a/apps/web/src/components/pullRequest/usePullRequestListView.ts +++ b/apps/web/src/components/pullRequest/usePullRequestListView.ts @@ -5,6 +5,7 @@ import { type ProjectId, type PullRequestInvolvement, type PullRequestListFilters, + type PullRequestListInput, type PullRequestListResult, type PullRequestListState, } from "@t3tools/contracts"; @@ -181,6 +182,38 @@ export function usePullRequestScope(search: PullRequestsSearch) { }; } +export const PAGE_SIZE = 99; + +function pullRequestMenuFilters(search: PullRequestListPreferences) { + return { + ...(search.draft ? { draft: search.draft } : {}), + ...(search.review ? { review: search.review } : {}), + ...(search.checks ? { checks: search.checks } : {}), + ...(search.author ? { author: search.author } : {}), + ...(search.labels ? { labels: search.labels.map((label) => [label]) } : {}), + }; +} + +/** The first page of a scope, without the typed search; the list reads the same key when unsearched. */ +export function pullRequestBaselineTargets( + search: PullRequestListPreferences, + scope: Pick, "environmentQueries" | "scopedProjectId">, +) { + const menuFilters = pullRequestMenuFilters(search); + return scope.environmentQueries.map(({ environmentId, projectIds }) => ({ + environmentId, + input: { + state: search.state, + involvement: search.involvement, + limit: PAGE_SIZE, + ...(scope.scopedProjectId ? { projectId: scope.scopedProjectId } : {}), + ...(projectIds ? { projectIds } : {}), + ...(search.host ? { host: search.host } : {}), + ...(Object.keys(menuFilters).length > 0 ? { filters: menuFilters } : {}), + } satisfies PullRequestListInput, + })); +} + export function usePullRequestQueryFilters(search: PullRequestsSearch, typedQuery: string) { const [sentQuery, setSentQuery] = useState(typedQuery); // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- debounce timer that sends the typed query after SEARCH_DEBOUNCE_MS @@ -191,13 +224,7 @@ export function usePullRequestQueryFilters(search: PullRequestsSearch, typedQuer const typedParsed = parsePullRequestQuery(typedQuery); const sentParsed = parsePullRequestQuery(sentQuery); - const menuFilters = { - ...(search.draft ? { draft: search.draft } : {}), - ...(search.review ? { review: search.review } : {}), - ...(search.checks ? { checks: search.checks } : {}), - ...(search.author ? { author: search.author } : {}), - ...(search.labels ? { labels: search.labels.map((label) => [label]) } : {}), - }; + const menuFilters = pullRequestMenuFilters(search); const menuFiltered = Object.keys(menuFilters).length > 0; const filters = { ...menuFilters, ...sentParsed.filters }; const hasFilters = Object.keys(filters).length > 0; diff --git a/packages/client-runtime/src/state/pullRequests.test.ts b/packages/client-runtime/src/state/pullRequests.test.ts index 7386bbe5ffb87..441d5d54078e3 100644 --- a/packages/client-runtime/src/state/pullRequests.test.ts +++ b/packages/client-runtime/src/state/pullRequests.test.ts @@ -2388,6 +2388,70 @@ it.effect.each([ ), ); +it.effect.each([false, true])( + "reads a list ahead once online, only when the device saved none for it (saved: %s)", + (alreadySaved) => + Effect.scoped( + Effect.gen(function* () { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + yield* Effect.addFinalizer(() => Effect.sync(() => vi.useRealTimers())); + let reads = 0; + const saved = new Map(); + const input = { state: "open", limit: 99 } as const; + if (alreadySaved) { + saved.set( + queryPersistenceKey("environment-data:pull-requests:list", input), + JSON.stringify(listResult([])), + ); + } + const client = actionReplyClient({ + [WS_METHODS.pullRequestsList]: () => + Effect.sync(() => { + reads++; + return listResult([{ ...actionReference, state: "open", isDraft: false }]); + }), + }); + const { atoms, registry, supervisor } = yield* makeTestRuntime( + client, + undefined, + false, + {}, + { + load: (_environmentId, key) => + Effect.sync(() => Option.fromUndefinedOr(saved.get(key))), + save: (_environmentId, key, payload) => Effect.sync(() => void saved.set(key, payload)), + }, + ); + yield* SubscriptionRef.update(supervisor.state, (state) => ({ + ...state, + phase: "offline" as const, + })); + const target = { environmentId: TARGET.environmentId, input }; + const warm = atoms.warmList(target); + const unmount = registry.mount(warm); + yield* Effect.addFinalizer(() => Effect.sync(unmount)); + yield* AtomRegistry.getResult(registry, warm, { suspendOnWaiting: true }); + expect(reads).toBe(0); + // The list's idle wait ends while still offline, so only the warm-up can read again. + yield* Effect.promise(() => vi.advanceTimersByTimeAsync(11 * 60_000)); + + yield* SubscriptionRef.update(supervisor.state, (state) => ({ + ...state, + phase: "connected" as const, + })); + yield* Effect.yieldNow; + yield* AtomRegistry.getResult(registry, warm, { suspendOnWaiting: true }); + expect(reads > 0).toBe(!alreadySaved); + // The page's own read of the same key then paints at once from what the warm-up saved. + const list = atoms.list(target); + registry.mount(list); + expect(Option.getOrThrow(AsyncResult.value(registry.get(list))).entries.length).toBe( + alreadySaved ? 0 : 1, + ); + }), + ), +); + it.effect("puts readers back when the host refuses an action, and saves nothing it guessed", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/client-runtime/src/state/pullRequests.ts b/packages/client-runtime/src/state/pullRequests.ts index 0f6df29c3b7ef..02fb806070c7a 100644 --- a/packages/client-runtime/src/state/pullRequests.ts +++ b/packages/client-runtime/src/state/pullRequests.ts @@ -45,6 +45,7 @@ import { createEnvironmentRpcQueryAtomFamily, createEnvironmentRpcSubscriptionAtomFamily, createEnvironmentQueryAtomFamily, + environmentSessionGenerationAtom, queryPersistenceKey, type QueryAnswerHandler, } from "./runtime.ts"; @@ -1135,6 +1136,55 @@ export function createPullRequestEnvironmentAtoms( const listLabel = "environment-data:pull-requests:list"; const listPersistence = persistQuery(PullRequestListResult); + const list = actionPatchedQuery( + createEnvironmentRpcQueryAtomFamily(runtime, { + label: listLabel, + persistence: listPersistence, + tag: WS_METHODS.pullRequestsList, + staleTimeMs: 30_000, + refreshTrigger: ({ environmentId, input }) => + input.cursors === undefined ? refreshes({ environmentId, input: {} }) : undefined, + ...(onAnswer.list === undefined ? {} : { onAnswer: onAnswer.list }), + }), + { + label: listLabel, + persistence: listPersistence, + onAnswer: onAnswer.list, + // Rows carry no account scope, so a reply read through another account leaves them alone. + concerns: (input, reference) => + reference.expectedAccountId === undefined && + (input.projectId === undefined || input.projectId === reference.projectId) && + (input.projectIds === undefined || input.projectIds.includes(reference.projectId)) && + (input.host === undefined || + reference.host === undefined || + input.host.toLowerCase() === reference.host.toLowerCase()), + refreshedByServer: (input) => input.cursors === undefined, + patch: patchListWithReply, + edit: (input, value, edit) => { + const entries = editListEntries(input, value.entries, edit); + return entries === value.entries ? value : { ...value, entries }; + }, + }, + ); + // A first visit paints from a saved answer; with none saved, one read now fills it. + const listWarmups = Atom.family((key: string) => { + const [environmentId, input] = JSON.parse(key) as [EnvironmentId, PullRequestListInput]; + const target = { environmentId, input }; + return runtime.atom((get) => { + // A read that failed offline tries again on the next session. + get(environmentSessionGenerationAtom(runtime, environmentId)); + return Effect.gen(function* () { + const saved = yield* listPersistence.load( + environmentId, + queryPersistenceKey(listLabel, input), + ); + if (Option.isSome(saved)) return; + yield* AtomRegistry.mount(get.registry, list(target)); + yield* AtomRegistry.getResult(get.registry, list(target), { suspendOnWaiting: true }); + }).pipe(Effect.scoped, Effect.ignore); + }); + }); + return { refreshes, linkedThreads: createEnvironmentRpcQueryAtomFamily(runtime, { @@ -1153,36 +1203,11 @@ export function createPullRequestEnvironmentAtoms( staleTimeMs: 0, idleTtlMs: 0, }), - list: actionPatchedQuery( - createEnvironmentRpcQueryAtomFamily(runtime, { - label: listLabel, - persistence: listPersistence, - tag: WS_METHODS.pullRequestsList, - staleTimeMs: 30_000, - refreshTrigger: ({ environmentId, input }) => - input.cursors === undefined ? refreshes({ environmentId, input: {} }) : undefined, - ...(onAnswer.list === undefined ? {} : { onAnswer: onAnswer.list }), - }), - { - label: listLabel, - persistence: listPersistence, - onAnswer: onAnswer.list, - // Rows carry no account scope, so a reply read through another account leaves them alone. - concerns: (input, reference) => - reference.expectedAccountId === undefined && - (input.projectId === undefined || input.projectId === reference.projectId) && - (input.projectIds === undefined || input.projectIds.includes(reference.projectId)) && - (input.host === undefined || - reference.host === undefined || - input.host.toLowerCase() === reference.host.toLowerCase()), - refreshedByServer: (input) => input.cursors === undefined, - patch: patchListWithReply, - edit: (input, value, edit) => { - const entries = editListEntries(input, value.entries, edit); - return entries === value.entries ? value : { ...value, entries }; - }, - }, - ), + list, + warmList: (target: { + readonly environmentId: EnvironmentId; + readonly input: PullRequestListInput; + }) => listWarmups(JSON.stringify([target.environmentId, target.input])), /** * Its own query because the listing is quicker without them — measured over twelve * repositories, ~4.0s against ~7.1s — so the rows arrive first and their stats a moment From ab5fd5d44c0db3e0c503f613ce22b877465089dc Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Tue, 10 Nov 2026 05:29:12 +0000 Subject: [PATCH 16/28] fix(web): Code tab reads ahead only after the pointer rests, stale merge reason reaches screen readers --- .../pullRequest/PullRequestDetailLayout.tsx | 14 ++- ...stDetailPanel.permissions.browser.test.tsx | 7 +- .../pullRequest/PullRequestStackMenu.tsx | 5 +- .../ThreadPullRequestsPanel.browser.test.tsx | 98 ------------------- 4 files changed, 20 insertions(+), 104 deletions(-) delete mode 100644 apps/web/src/components/pullRequest/ThreadPullRequestsPanel.browser.test.tsx diff --git a/apps/web/src/components/pullRequest/PullRequestDetailLayout.tsx b/apps/web/src/components/pullRequest/PullRequestDetailLayout.tsx index c9bcfc1a378fd..d86cf10b6629c 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailLayout.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailLayout.tsx @@ -1,5 +1,5 @@ import { ArrowLeftIcon, FileDiffIcon } from "lucide-react"; -import type { ComponentProps, ReactNode } from "react"; +import { useRef, type ComponentProps, type ReactNode } from "react"; import { cn } from "~/lib/utils"; @@ -77,6 +77,8 @@ export function PullRequestDetailTabBar({ inert?: boolean; children?: ReactNode; }) { + // A pointer passing over a tab is not intent; it must rest like a list row does. + const pointerIntent = useRef | undefined>(undefined); return (