From 4a087861d0764654c0e9f450f40015aea03d50a7 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Thu, 5 Nov 2026 20:07:47 -0800 Subject: [PATCH 001/227] feat(scheduled-tasks): tasks link to their thread (#31764) Co-authored-by: Claude Opus 5.5 --- .../SettingsScheduledTasksRouteScreen.tsx | 14 ++++++++++ .../settings/ScheduledTasksSettings.tsx | 27 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx index 34dd75e7500e8..6e90e449359ca 100644 --- a/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsScheduledTasksRouteScreen.tsx @@ -18,7 +18,9 @@ import { MAX_WEBHOOK_DELIVERY_AGE_MINUTES, MAX_WEBHOOK_FILTERS, resolveEnvironmentMachineKind, + scheduledTaskThreadMode, } from "@t3tools/contracts"; +import { useSelectWorkspaceDetail } from "../layout/workspace/useSelectWorkspaceDetail"; import type { MenuAction } from "@react-native-menu/menu"; import { DateTimePicker } from "@expo/ui/community/datetime-picker"; import { @@ -1412,6 +1414,7 @@ function EnvironmentTasks({ label: "scheduled task delete", reportFailure: false, }); + const selectDetail = useSelectWorkspaceDetail(); const failure = (title: string, result: AtomCommandResult) => { if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { Dialog.alert(title, String(squashAtomCommandFailure(result))); @@ -1493,6 +1496,9 @@ function EnvironmentTasks({ {task.lastRunStatus} ) : null} + {task.threadId !== null && scheduledTaskThreadMode(task) !== "new_thread" ? ( + + ) : null} {task.lastRunError ? {task.lastRunError} : null} } @@ -847,6 +852,26 @@ function ScheduledTaskRow({ ); } +function ScheduledTaskThreadLink({ + environmentId, + threadId, +}: { + readonly environmentId: EnvironmentId; + readonly threadId: ThreadId; +}) { + const ref = scopeThreadRef(environmentId, threadId); + const title = useQuietThreadShell(ref)?.title.trim(); + return ( + + {title || "Open thread"} + + ); +} + function WebhookDeliveriesDialog({ environmentId, task, From 260d84387fbfbf97c0b7391e3f95e9d10429cdd2 Mon Sep 17 00:00:00 2001 From: maco Date: Fri, 6 Nov 2026 05:20:50 +0100 Subject: [PATCH 002/227] fix(shared): recognize numeric bare-repository config (#27500) Co-authored-by: maria-rcks --- packages/shared/src/git.test.ts | 41 +++++++++++++++++++++++++++++++++ packages/shared/src/git.ts | 17 +++++++++++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/git.test.ts b/packages/shared/src/git.test.ts index a788abb1d1d7e..a21a52cb70e7c 100644 --- a/packages/shared/src/git.test.ts +++ b/packages/shared/src/git.test.ts @@ -66,6 +66,47 @@ describe("normalizeGitRemoteUrl", () => { }); describe("isBareGitConfig", () => { + it("reads Git's numeric booleans without accepting invalid or overflowing values", () => { + for (const value of [ + "true", + "yes", + "on", + "1", + "2", + "-1", + "+1", + "01", + "0x10", + "1k", + "1m", + "1g", + "2147483647", + "-2147483648", + ]) { + expect(isBareGitConfig(`[core]\nbare = ${value}\n`), value).toBe(true); + } + for (const value of [ + "false", + "no", + "off", + "0", + "-0", + "00", + "0x0", + "0k", + "08", + "1.5", + "2no", + "2g", + "2147483648", + "-2147483649", + ]) { + expect(isBareGitConfig(`[core]\nbare = ${value}\n`), value).toBe(false); + } + expect(isBareGitConfig("[core]\nbare = 2\nbare = 0\n")).toBe(false); + expect(isBareGitConfig("[core]\nbare = false\nbare = -2\n")).toBe(true); + }); + it("reads continued bare values after comments and at end of input", () => { const config = ["[core] bare = false # ignored backslash \\", 'bare = "tr\\', 'ue"'].join("\n"); diff --git a/packages/shared/src/git.ts b/packages/shared/src/git.ts index ba7c9f0a97b70..9f7c6c3227ef5 100644 --- a/packages/shared/src/git.ts +++ b/packages/shared/src/git.ts @@ -213,6 +213,21 @@ function parseGitConfigValue(raw: string): string { return out.trim(); } +function isTrueGitConfigValue(value: string): boolean { + if (value === "true" || value === "yes" || value === "on") return true; + const numeric = /^([+-]?)(0x[0-9a-f]+|0[0-7]*|[1-9][0-9]*)([kmg]?)$/.exec(value); + if (numeric === null) return false; + const digits = numeric[2]!; + const magnitude = digits.startsWith("0x") + ? Number.parseInt(digits.slice(2), 16) + : Number.parseInt(digits, digits.startsWith("0") ? 8 : 10); + const scale = + numeric[3] === "k" ? 1024 : numeric[3] === "m" ? 1024 ** 2 : numeric[3] === "g" ? 1024 ** 3 : 1; + const number = magnitude * scale * (numeric[1] === "-" ? -1 : 1); + // Git's boolean parser accepts integers only within its signed 32-bit range. + return number !== 0 && number >= -2147483648 && number <= 2147483647; +} + export function isBareGitConfig(configText: string): boolean { let inCore = false; let bare = false; @@ -228,7 +243,7 @@ export function isBareGitConfig(configText: string): boolean { const match = /^bare\s*(?:=(.*)|[#;].*)?$/i.exec(line); if (match === null) continue; const value = match[1] === undefined ? "true" : parseGitConfigValue(match[1]).toLowerCase(); - bare = value === "true" || value === "yes" || value === "on" || value === "1"; + bare = isTrueGitConfigValue(value); } return bare; } From a530cab2bdcec09360fb69ec730bc9a1dca5821d Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 6 Nov 2026 04:21:08 +0000 Subject: [PATCH 003/227] fix(server): a command id reused for two threads at once no longer succeeds for both (#18385) Co-authored-by: SAPHID Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks --- .../Orchestrator.receipt-race.test.ts | 168 ++++++++++++++++++ .../src/orchestration-v2/Orchestrator.ts | 118 +++++++----- 2 files changed, 241 insertions(+), 45 deletions(-) create mode 100644 apps/server/src/orchestration-v2/Orchestrator.receipt-race.test.ts diff --git a/apps/server/src/orchestration-v2/Orchestrator.receipt-race.test.ts b/apps/server/src/orchestration-v2/Orchestrator.receipt-race.test.ts new file mode 100644 index 0000000000000..3e6fdd19eb9dc --- /dev/null +++ b/apps/server/src/orchestration-v2/Orchestrator.receipt-race.test.ts @@ -0,0 +1,168 @@ +import { vi } from "vite-plus/test"; +import { assert, it } from "@effect/vitest"; +import { + CommandId, + ProjectId, + ProviderDriverKind, + ProviderInstanceId, + ThreadId, +} from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import * as Deferred from "effect/Deferred"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Layer from "effect/Layer"; +import * as Ref from "effect/Ref"; +import * as SqlitePersistence from "../persistence/Sqlite.ts"; +import { CodexProviderCapabilitiesV2 } from "./Adapters/CodexAdapterV2.ts"; +import * as Orchestrator from "./Orchestrator.ts"; +import * as ProjectionStore from "./ProjectionStore.ts"; +import * as ProviderAdapterRegistry from "./ProviderAdapterRegistry.ts"; +import * as ProviderReplayHarness from "./testkit/ProviderReplayHarness.ts"; +import type * as ProviderAdapter from "@t3tools/provider-core/server/ProviderAdapter"; + +const instanceId = ProviderInstanceId.make("codex"); +const modelSelection = { instanceId, model: "gpt-5.1-codex" }; +// Holds model-selection planning until both racing commands have read no receipt. +let capabilityGate: Effect.Effect | undefined; +const adapter = { + instanceId, + driver: ProviderDriverKind.make("codex"), + getCapabilities: () => + (capabilityGate ?? Effect.void).pipe(Effect.as(CodexProviderCapabilitiesV2)), + planSelectionTransition: () => Effect.succeed({ type: "apply_on_next_turn" as const }), + openSession: () => Effect.die("No provider process needed for metadata controls"), +} as ProviderAdapter.ProviderAdapterV2["Service"]; +const layerDatabase = SqlitePersistence.layerMemory; +const layerTest = Layer.mergeAll( + layerDatabase, + ProjectionStore.layer.pipe(Layer.provide(layerDatabase)), + ProviderReplayHarness.layerWithRegistry( + { name: "receipt-race" }, + ProviderAdapterRegistry.layerFromAdapters([adapter]), + { databaseLayer: layerDatabase, runEffectWorker: false }, + ), +); + +it.effect("a command id reused concurrently for two threads succeeds for only one", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projections = yield* ProjectionStore.ProjectionStoreV2; + const threadIds = [ThreadId.make("thread:race-a"), ThreadId.make("thread:race-b")]; + for (const threadId of threadIds) { + yield* orchestrator.dispatch({ + type: "thread.create", + commandId: CommandId.make(`create:${threadId}`), + threadId, + projectId: ProjectId.make("project:receipt-race"), + title: threadId, + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + createdBy: "user", + creationSource: "web", + }); + } + + const arrived = yield* Ref.make(0); + const bothArrived = yield* Deferred.make(); + capabilityGate = Effect.gen(function* () { + if ((yield* Ref.updateAndGet(arrived, (count) => count + 1)) === 2) { + yield* Deferred.succeed(bothArrived, undefined); + } + yield* Deferred.await(bothArrived); + }); + const setModel = (threadId: ThreadId) => + orchestrator.dispatch({ + type: "thread.model-selection.set", + commandId: CommandId.make("command:shared"), + threadId, + modelSelection: { ...modelSelection, model: "gpt-6" }, + }); + const exits = yield* Effect.forEach(threadIds, (threadId) => Effect.exit(setModel(threadId)), { + concurrency: "unbounded", + }).pipe(Effect.ensuring(Effect.sync(() => (capabilityGate = undefined)))); + + const winner = exits.findIndex(Exit.isSuccess); + const loser = exits.findIndex(Exit.isFailure); + assert.isAtLeast(winner, 0); + assert.isAtLeast(loser, 0); + const winnerExit = exits[winner]!; + const loserExit = exits[loser]!; + assert.isTrue(Exit.isSuccess(winnerExit) && Exit.isFailure(loserExit)); + if (Exit.isFailure(loserExit)) { + const error = Cause.findErrorOption(loserExit.cause); + assert.isTrue(error._tag === "Some"); + if (error._tag === "Some") { + assert.equal(error.value._tag, "OrchestratorCommandIdConflictError"); + if (error.value._tag === "OrchestratorCommandIdConflictError") { + assert.equal(error.value.receiptThreadId, threadIds[winner]); + assert.equal(error.value.commandThreadId, threadIds[loser]); + } + } + } + + const models = yield* Effect.forEach(threadIds, (threadId) => + projections.getThreadShell(threadId).pipe(Effect.map((shell) => shell?.modelSelection.model)), + ); + assert.equal(models[winner], "gpt-6"); + assert.equal(models[loser], "gpt-5.1-codex"); + + const noOpArrived = yield* Ref.make(0); + const bothNoOpsArrived = yield* Deferred.make(); + const getThreadRecords = projections.getThreadRecords; + using _records = vi.spyOn(projections, "getThreadRecords").mockImplementation((...args) => + Effect.gen(function* () { + if ((yield* Ref.updateAndGet(noOpArrived, (count) => count + 1)) === 2) { + yield* Deferred.succeed(bothNoOpsArrived, undefined); + } + yield* Deferred.await(bothNoOpsArrived); + return yield* getThreadRecords(...args); + }), + ); + const closeShare = (threadId: ThreadId) => + orchestrator.dispatch({ + type: "thread.guest-share.close", + commandId: CommandId.make("command:shared-noop"), + threadId, + }); + const noOpExits = yield* Effect.forEach( + threadIds, + (threadId) => Effect.exit(closeShare(threadId)), + { + concurrency: "unbounded", + }, + ); + const noOpWinner = noOpExits.findIndex(Exit.isSuccess); + const noOpLoser = noOpExits.findIndex(Exit.isFailure); + assert.isAtLeast(noOpWinner, 0); + assert.isAtLeast(noOpLoser, 0); + const noOpFailure = noOpExits[noOpLoser]!; + if (Exit.isFailure(noOpFailure)) { + const error = Cause.findErrorOption(noOpFailure.cause); + assert.isTrue(error._tag === "Some"); + if (error._tag === "Some") { + assert.equal(error.value._tag, "OrchestratorCommandIdConflictError"); + } + } + const noOpReplay = yield* closeShare(threadIds[noOpWinner]!); + const noOpSuccess = noOpExits[noOpWinner]!; + if (Exit.isSuccess(noOpSuccess)) { + assert.equal(noOpReplay.sequence, noOpSuccess.value.sequence); + assert.deepEqual(noOpReplay.storedEvents, []); + } + + // The winner's own thread still replays its receipt. + const replay = yield* setModel(threadIds[winner]!); + if (Exit.isSuccess(winnerExit)) { + assert.equal(replay.sequence, winnerExit.value.sequence); + assert.isAbove(replay.storedEvents.length, 0); + assert.deepEqual( + replay.storedEvents.map((stored) => stored.sequence), + winnerExit.value.storedEvents.map((stored) => stored.sequence), + ); + } + }).pipe(Effect.provide(layerTest)), +); diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index d40ca624a11d2..8bf0c20de98eb 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -110,7 +110,7 @@ import { isNativeMaintenanceCommand, resolveMessageDispatchIntent, } from "./CommandPolicy.ts"; -import { CommandReceiptStoreV2 } from "./CommandReceiptStore.ts"; +import { CommandReceiptStoreV2, type CommandReceiptV2 } from "./CommandReceiptStore.ts"; import { ContextHandoffServiceV2, readForkHistoryItems, @@ -13741,6 +13741,54 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }; }); + const replayCommandReceipt = Effect.fn("orchestrationV2.dispatch.replayReceipt")(function* ( + command: OrchestrationV2ServerCommand, + receipt: CommandReceiptV2, + ) { + if (receipt.status === "rejected") { + return yield* new OrchestratorCommandPreviouslyRejectedError({ + commandId: command.commandId, + commandType: command.type, + detail: receipt.error ?? "Previously rejected.", + }); + } + const dispatchThreadId = commandThreadId(command); + // Receipts only prove execution for the thread they were recorded against. + if (receipt.threadId !== dispatchThreadId) { + return yield* new OrchestratorCommandIdConflictError({ + commandId: command.commandId, + commandType: command.type, + receiptThreadId: receipt.threadId, + commandThreadId: dispatchThreadId, + }); + } + const storedEvents = yield* eventSink.readByCommandId({ commandId: command.commandId }).pipe( + Stream.runCollect, + Effect.map((events): ReadonlyArray => Array.from(events)), + Effect.mapError( + (cause) => + new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause, + }), + ), + ); + if (command.type === "queue.resume" || command.type === "queued-run.start") { + yield* mapDispatchError(command)( + startNextQueuedRun( + command.threadId, + command.type === "queued-run.start" ? { queuedRunId: command.runId } : undefined, + ), + ); + } + return { + sequence: receipt.resultSequence, + storedEvents, + replayed: true, + } satisfies OrchestratorV2DispatchResult; + }); + /** `sideChatIds` are the live side chats a `thread.delete` cascades to, read under their locks. */ const dispatchWithReceiptEffect = Effect.fn("orchestrationV2.dispatch.withReceipt")(function* ( command: OrchestrationV2ServerCommand, @@ -13764,49 +13812,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ); if (Option.isSome(existingReceipt)) { - const receipt = existingReceipt.value; - if (receipt.status === "rejected") { - return yield* new OrchestratorCommandPreviouslyRejectedError({ - commandId: command.commandId, - commandType: command.type, - detail: receipt.error ?? "Previously rejected.", - }); - } - const dispatchThreadId = commandThreadId(command); - // Receipts only prove execution for the thread they were recorded against. - if (receipt.threadId !== dispatchThreadId) { - return yield* new OrchestratorCommandIdConflictError({ - commandId: command.commandId, - commandType: command.type, - receiptThreadId: receipt.threadId, - commandThreadId: dispatchThreadId, - }); - } - const storedEvents = yield* eventSink.readByCommandId({ commandId: command.commandId }).pipe( - Stream.runCollect, - Effect.map((events): ReadonlyArray => Array.from(events)), - Effect.mapError( - (cause) => - new OrchestratorDispatchError({ - commandId: command.commandId, - commandType: command.type, - cause, - }), - ), - ); - if (command.type === "queue.resume" || command.type === "queued-run.start") { - yield* mapDispatchError(command)( - startNextQueuedRun( - command.threadId, - command.type === "queued-run.start" ? { queuedRunId: command.runId } : undefined, - ), - ); - } - return { - sequence: receipt.resultSequence, - storedEvents, - replayed: true, - } satisfies OrchestratorV2DispatchResult; + return yield* replayCommandReceipt(command, existingReceipt.value); } // Recheck under the lock; the user may have raised modes since sender preflight. @@ -13921,10 +13927,22 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio }), ), ); + if (!inserted) { + const receipt = yield* mapDispatchError(command)( + commandReceipts.getByCommandId(command.commandId), + ); + if (Option.isNone(receipt)) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: "Conflicting receipt is not a thread command receipt.", + }); + } + return yield* replayCommandReceipt(command, receipt.value); + } return { sequence: resultSequence, storedEvents: [], - ...(inserted ? {} : { replayed: true as const }), } satisfies OrchestratorV2DispatchResult; } const acceptedAt = plan.events.at(-1)?.occurredAt ?? (yield* DateTime.now); @@ -13961,6 +13979,16 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio detail: committed.receipt.error ?? "Previously rejected.", }); } + // A concurrent command with this id for another thread can commit first, + // under that thread's lock; its receipt is not this command's result. + if (committed.receipt.threadId !== commandThreadId(command)) { + return yield* new OrchestratorCommandIdConflictError({ + commandId: command.commandId, + commandType: command.type, + receiptThreadId: committed.receipt.threadId, + commandThreadId: commandThreadId(command), + }); + } if ( command.type === "queue.resume" || command.type === "queued-run.start" || From 693ac5f5df23a8fd8432ce73c356d05edc40bd0a Mon Sep 17 00:00:00 2001 From: Alyx Buckmann <57874862+snarkyalyx@users.noreply.github.com> Date: Fri, 6 Nov 2026 05:21:26 +0100 Subject: [PATCH 004/227] fix(server): coalesce tool output before it fills the live buffer (#16317) Co-authored-by: maria-rcks --- .../orchestration-v2/LiveStreamBudget.test.ts | 57 +++++++++++++++++ .../ThreadLiveEventCoalescer.test.ts | 63 ++++++++++++++++++- .../ThreadLiveEventCoalescer.ts | 50 ++++++++++----- 3 files changed, 151 insertions(+), 19 deletions(-) diff --git a/apps/server/src/orchestration-v2/LiveStreamBudget.test.ts b/apps/server/src/orchestration-v2/LiveStreamBudget.test.ts index 39f31bfedc7db..44e66412fcd52 100644 --- a/apps/server/src/orchestration-v2/LiveStreamBudget.test.ts +++ b/apps/server/src/orchestration-v2/LiveStreamBudget.test.ts @@ -87,6 +87,63 @@ describe("bufferLatestLiveStream", () => { }); describe("replayAndBufferProjectedLiveEvents", () => { + it.effect.each(["items", "bytes"] as const)( + "replays the undelivered tail after a %s overflow without losing or duplicating events", + (limit) => + Effect.scoped( + Effect.gen(function* () { + const pubsub = yield* PubSub.unbounded(); + const subscribed = yield* Deferred.make>(); + const events = [ + { sequence: 1, text: "one" }, + { sequence: 2, text: "two" }, + { sequence: 3, text: "three" }, + ]; + const limits = limit === "items" ? { maxItems: 1 } : { maxSerializedBytes: 40 }; + const pull = yield* Stream.toPull( + replayAndBufferProjectedLiveEvents( + { + project: (event: Event) => event, + subscribe: PubSub.subscribe(pubsub).pipe( + Effect.tap((subscription) => Deferred.succeed(subscribed, subscription)), + ), + latestSequence: Effect.succeed(1), + replay: () => Stream.succeed(events[0]!), + }, + limits, + ), + ); + const received = [...(yield* pull)]; + // Hold the ACK while more durable events arrive. + yield* PubSub.publishAll(pubsub, events.slice(1)); + yield* (yield* Deferred.await(subscribed)).shutdownHook.await; + const overflow = yield* pull.pipe(Effect.result); + expect(overflow._tag).toBe("Failure"); + if (overflow._tag === "Failure") + expect(overflow.failure._tag).toBe("LiveStreamBufferError"); + + const afterSequence = received.at(-1)!.sequence; + const resumed = yield* replayAndBufferProjectedLiveEvents( + { + project: (event: Event) => event, + subscribe: PubSub.subscribe(pubsub), + afterSequence, + latestSequence: Effect.succeed(3), + replay: (throughSequence) => + Stream.fromIterable( + events.filter( + (event) => event.sequence > afterSequence && event.sequence <= throughSequence, + ), + ), + }, + limits, + ).pipe(Stream.take(2), Stream.runCollect); + expect([...received, ...resumed]).toEqual(events); + expect(yield* PubSub.size(pubsub)).toBe(0); + }), + ), + ); + it.effect.each(["high-water", "replay"] as const)( "unsubscribes and cancels a blocked %s read on live overflow", (phase) => diff --git a/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.test.ts b/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.test.ts index 294ed0622d62e..7b91e479b98ef 100644 --- a/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.test.ts +++ b/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.test.ts @@ -36,6 +36,7 @@ function makeToolActivity( readonly kind?: "tool.updated" | "tool.completed"; readonly toolCallId?: string; readonly turnId?: RunId; + readonly output?: string; } = {}, ): ThreadEvent { const itemRunId = options.turnId ?? runId; @@ -66,7 +67,7 @@ function makeToolActivity( completedAt: status === "completed" ? now : null, updatedAt: now, input: "echo app.ts", - output: `output-${sequence}`, + output: options.output ?? `output-${sequence}`, }, }, }; @@ -155,6 +156,62 @@ function makeTextItem( } describe("ThreadLiveEventCoalescer", () => { + it.effect.each(["items", "bytes"] as const)( + "replaces superseded tool output before charging the %s budget", + (limit) => + Effect.scoped( + Effect.gen(function* () { + const updates = Array.from({ length: 1_100 }, (_, index) => makeToolActivity(index + 1)); + const latest = updates.at(-1)!; + const coalescer = yield* makeThreadLiveEventCoalescer( + limit === "items" + ? { maxItems: 1 } + : { maxSerializedBytes: Buffer.byteLength(encodeEvent(latest)) }, + ); + yield* coalescer.offerAll(updates); + expect(yield* coalescer.usage).toEqual({ + retainedItems: 1, + retainedSerializedBytes: Buffer.byteLength(encodeEvent(latest)), + }); + yield* coalescer.end; + expect(yield* coalescer.stream.pipe(Stream.runCollect)).toEqual([latest]); + expect(yield* coalescer.usage).toEqual({ retainedItems: 0, retainedSerializedBytes: 0 }); + }), + ), + ); + + it.effect("handles a tool output burst larger than the default 8 MiB buffer", () => + Effect.scoped( + Effect.gen(function* () { + const coalescer = yield* makeThreadLiveEventCoalescer(); + const updates = Array.from({ length: 150 }, (_, index) => + makeToolActivity(index + 1, { output: "x".repeat(64 * 1024) }), + ); + yield* coalescer.offerAll(updates); + yield* coalescer.end; + expect(yield* coalescer.stream.pipe(Stream.runCollect)).toEqual([updates.at(-1)]); + }), + ), + ); + + it.effect("still fails when a single replacement exceeds the byte budget", () => + Effect.scoped( + Effect.gen(function* () { + const first = makeToolActivity(1); + const coalescer = yield* makeThreadLiveEventCoalescer({ + maxSerializedBytes: Buffer.byteLength(encodeEvent(first)), + }); + yield* coalescer.offer(first); + const overflow = yield* coalescer + .offer(makeToolActivity(2, { output: "x".repeat(2_000) })) + .pipe(Effect.result); + expect(overflow._tag).toBe("Failure"); + yield* coalescer.closed; + expect(yield* coalescer.usage).toEqual({ retainedItems: 0, retainedSerializedBytes: 0 }); + }), + ), + ); + it("preserves tool-call identity switches as insertion-order boundaries", () => { const events = [ makeToolActivity(1, { toolCallId: "call-a" }), @@ -399,8 +456,8 @@ describe("ThreadLiveEventCoalescer", () => { yield* TestClock.adjust("40 millis"); yield* coalescer.offerAll(latest); expect(yield* coalescer.usage).toEqual({ - retainedItems: 4, - retainedSerializedBytes: serializedBytes([...first, ...latest]), + retainedItems: 2, + retainedSerializedBytes: serializedBytes(latest), }); yield* TestClock.adjust("10 millis"); const expectedUsage = { diff --git a/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts b/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts index 00adbb0cb5445..33c82dfe4ef4c 100644 --- a/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts +++ b/apps/server/src/orchestration-v2/ThreadLiveEventCoalescer.ts @@ -90,7 +90,9 @@ export const makeThreadLiveEventCoalescer = Effect.fn( E | LiveStreamBufferError | Cause.Done >(); const mutex = yield* Semaphore.make(1); - let pendingUpdates: Array> = []; + const pendingUpdates = new Set>(); + const latestUpdates = new Map>(); + const pendingKeys = new Map(); let windowGeneration = 0; let windowFiber: Fiber.Fiber | null = null; let closed = false; @@ -106,14 +108,13 @@ export const makeThreadLiveEventCoalescer = Effect.fn( }); const flushPending = Effect.fnUntraced(function* () { - if (pendingUpdates.length === 0) { + if (pendingUpdates.size === 0) { return; } - const items = yield* budget.replace( - pendingUpdates, - coalesceLiveUpdatedEvents(pendingUpdates.map((item) => item.value)), - ); - pendingUpdates = []; + const items = Array.from(pendingUpdates); + pendingUpdates.clear(); + latestUpdates.clear(); + pendingKeys.clear(); yield* Queue.offerAll(output, items); }, Effect.uninterruptible); @@ -131,7 +132,6 @@ export const makeThreadLiveEventCoalescer = Effect.fn( } }), ), - Effect.catchTags({ LiveStreamBufferError: () => Effect.void }), ); // Keep each source batch together so a synchronization marker cannot pass @@ -143,18 +143,28 @@ export const makeThreadLiveEventCoalescer = Effect.fn( (input) => Effect.gen(function* () { yield* budget.check; - if (input.kind === "event") { - yield* budget.retain(input).pipe( - Effect.tap((item) => Effect.sync(() => pendingUpdates.push(item))), + const startsWindow = pendingUpdates.size === 0; + const key = input.kind === "event" ? coalescibleUpdateKey(input) : null; + if (input.kind === "event" && key !== null) { + const pendingKey = pendingKeys.get(input.event.type); + if (pendingKey !== undefined && pendingKey !== key) latestUpdates.clear(); + pendingKeys.set(input.event.type, key); + const previous = latestUpdates.get(key); + yield* budget.replace(previous ? [previous] : [], [input]).pipe( + Effect.tap(([item]) => + Effect.sync(() => { + if (previous) pendingUpdates.delete(previous); + pendingUpdates.add(item!); + latestUpdates.set(key, item!); + }), + ), Effect.uninterruptible, ); - } - if (input.kind === "event" && coalescibleUpdateKey(input) !== null) { - if (pendingUpdates.length === 1) { + if (startsWindow) { const generation = ++windowGeneration; windowFiber = yield* Effect.forkIn(flushWindow(generation), coalescerScope); } - if (pendingUpdates.length >= MAX_PENDING_UPDATES) { + if (pendingUpdates.size >= MAX_PENDING_UPDATES) { yield* cancelWindow(); windowGeneration += 1; yield* flushPending(); @@ -162,6 +172,12 @@ export const makeThreadLiveEventCoalescer = Effect.fn( return; } + if (input.kind === "event") { + yield* budget.retain(input).pipe( + Effect.tap((item) => Effect.sync(() => pendingUpdates.add(item))), + Effect.uninterruptible, + ); + } yield* cancelWindow(); windowGeneration += 1; yield* flushPending(); @@ -187,7 +203,9 @@ export const makeThreadLiveEventCoalescer = Effect.fn( windowGeneration += 1; yield* cancelWindow(); budget.release(pendingUpdates); - pendingUpdates = []; + pendingUpdates.clear(); + latestUpdates.clear(); + pendingKeys.clear(); budget.release(yield* Queue.clear(output).pipe(Effect.orElseSucceed(() => []))); if (cause) { yield* Queue.failCause(output, cause); From cedaa0de48e6ab2b6fc5d18ee115df2e262b797a Mon Sep 17 00:00:00 2001 From: Kayzel Date: Fri, 6 Nov 2026 05:21:44 +0100 Subject: [PATCH 005/227] perf(server): Windows git reads no longer start a console host (#19288) --- apps/server/src/vcs/GitVcsDriverCore.ts | 5 +++- packages/shared/src/shell.test.ts | 31 +++++++++++++++++++ packages/shared/src/shell.ts | 40 +++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 94404b488b136..d1199f39c6542 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -36,7 +36,7 @@ import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { compactTraceAttributes } from "@t3tools/shared/observability"; import { decodeJsonResult } from "@t3tools/shared/schemaJson"; -import { resolveSpawnCommand } from "@t3tools/shared/shell"; +import { resolveSpawnCommand, runsGitWithoutConsole } from "@t3tools/shared/shell"; import { parseT3ProjectFile } from "@t3tools/shared/t3ProjectFile"; import { resolveProjectFileBackedSetting } from "@t3tools/shared/projectSettings"; import { gitCommandDuration, gitCommandsTotal, withMetrics } from "../observability/Metrics.ts"; @@ -993,6 +993,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* cwd: commandInput.cwd, env: spawnEnv, forceKillAfter: input.forceKillAfterMs, + ...(runsGitWithoutConsole(hostPlatform, executable, commandInput.args) + ? { detached: true, windowsHide: true } + : {}), }), ) .pipe( diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index 5f268ffa507ea..16dc69ab14c05 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -32,6 +32,7 @@ import { resolveCommandPath, resolveSpawnCommand, resolveWindowsEnvironment, + runsGitWithoutConsole, SpawnExecutableResolution, WindowsShellEnvironment, withPathDirectoryListings, @@ -561,6 +562,36 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => { ); }); +describe("runsGitWithoutConsole", () => { + const git = "C:\\Program Files\\Git\\ucrt64\\bin\\git.exe"; + + it.each([ + [["rev-parse", "--git-path", "index"], true], + [["--git-dir", "C:\\repo\\.git", "symbolic-ref", "refs/remotes/origin/HEAD"], true], + [["remote", "get-url", "--", "origin"], true], + [["config", "--get", "branch.main.pushRemote"], true], + [["symbolic-ref", "--quiet", "HEAD"], true], + [["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], true], + [["rev-parse", "HEAD"], false], + [["symbolic-ref", "HEAD", "refs/heads/x"], false], + [["symbolic-ref", "-qd", "refs/remotes/origin/HEAD"], false], + [["config", "--get", "--edit"], false], + [["-c", "core.pager=x", "config", "--get", "user.name"], false], + ])("Git for Windows %j without a console: %s", (args, expected) => { + expect(runsGitWithoutConsole("win32", git, args)).toBe(expected); + }); + + it.each([ + ["win32", "C:\\Program Files\\Git\\cmd\\git.exe"], + ["win32", "C:\\Users\\x\\scoop\\shims\\git.exe"], + ["linux", git], + ] as const)("keeps the console on %s for %s", (platform, executable) => { + expect(runsGitWithoutConsole(platform, executable, ["rev-parse", "--show-toplevel"])).toBe( + false, + ); + }); +}); + describe("preferGitForWindowsBinary", () => { const files = (...paths: Array) => diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 28b666ce05531..ad30c152a07c0 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -827,6 +827,46 @@ export interface SpawnCommandOptions extends CommandAvailabilityOptions { readonly bypassNpmShim?: boolean; } +// `*` is one argument that is not an option. These forms read refs or config +// only, so git starts no hook, helper, pager, editor or lazy fetch for them. +const CONSOLE_FREE_GIT_READS = [ + "rev-parse --git-path index", + "rev-parse --git-common-dir", + "rev-parse --show-toplevel", + "rev-parse --symbolic-full-name HEAD", + "rev-parse --abbrev-ref --symbolic-full-name @{upstream}", + "symbolic-ref --quiet HEAD", + "symbolic-ref *", + "remote", + "remote get-url *", + "remote get-url -- *", + "config --get *", +].map((form) => form.split(" ")); + +export function runsGitWithoutConsole( + platform: NodeJS.Platform, + executable: string, + args: ReadonlyArray, +): boolean { + if (platform !== "win32") return false; + const { basename, dirname } = NodePath.win32; + const bin = dirname(executable); + const build = basename(dirname(bin)).toLowerCase(); + if (basename(executable).toLowerCase() !== "git.exe" || basename(bin).toLowerCase() !== "bin") { + return false; + } + if (!GIT_FOR_WINDOWS_BUILDS.some((candidate) => candidate === build)) return false; + const rest = + args[0] === "--git-dir" && args[1] !== undefined && !args[1].startsWith("-") + ? args.slice(2) + : args; + return CONSOLE_FREE_GIT_READS.some( + (form) => + form.length === rest.length && + form.every((part, i) => part === rest[i] || (part === "*" && !rest[i]!.startsWith("-"))), + ); +} + export const resolveSpawnCommand = Effect.fnUntraced(function* ( command: string, args: ReadonlyArray, From 37f9134bea549a18c6bf798678aabcb87d9cffd4 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 6 Nov 2026 04:22:03 +0000 Subject: [PATCH 006/227] fix(server): a publish whose first push fails still shows the new remote (#29688) Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com> --- apps/server/src/ws.test.ts | 92 ++++++++++++++++++++++++++++++++++++++ apps/server/src/ws.ts | 8 +++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/apps/server/src/ws.test.ts b/apps/server/src/ws.test.ts index bf2298427e411..855e59440ace4 100644 --- a/apps/server/src/ws.test.ts +++ b/apps/server/src/ws.test.ts @@ -4,12 +4,15 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import { AuthSessionId, + AuthSourceControlWriteScope, EnvironmentId, ProjectId, ThreadId, ProviderDriverKind, ProviderInstanceId, + SourceControlRepositoryError, WS_METHODS, + WsRpcGroup, WsServerRefreshProvidersRpc, WsSubscribeServerConfigRpc, type OrchestrationV2StoredEvent, @@ -63,6 +66,7 @@ import * as ModelManifest from "./provider/ModelManifest.ts"; import * as Orchestrator from "./orchestration-v2/Orchestrator.ts"; import * as OtlpEndpointCheck from "./observability/OtlpEndpointCheck.ts"; import * as PairingGrantStore from "./auth/PairingGrantStore.ts"; +import * as RpcAuthorization from "./auth/RpcAuthorization.ts"; import * as PeerLinkRequests from "./peer/PeerLinkRequests.ts"; import * as PeerLinks from "./peer/PeerLinks.ts"; import * as PortScanner from "./preview/PortScanner.ts"; @@ -1240,6 +1244,94 @@ it.effect("on-demand clients get provider config without workspace snapshots", ( ), ); +it.effect("a publish whose push fails still refreshes the new remote", () => + Effect.gen(function* () { + const refreshes = yield* Queue.unbounded(); + const exits = yield* Queue.unbounded(); + const services = Layer.mergeAll( + layerUnusedConfigRpcServices, + Layer.mock(SourceControlRepositoryService.SourceControlRepositoryService)({ + publishRepository: (input) => + Effect.fail( + new SourceControlRepositoryError({ + provider: input.provider, + operation: "publishRepository", + detail: 'Created owner/repo and added it as remote "origin", but the push failed.', + }), + ), + }), + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ + resolve: (cwd, options) => + Queue.offer(refreshes, `identity ${cwd} refresh=${options?.refresh === true}`).pipe( + Effect.as(null), + ), + }), + Layer.mock(VcsStatusBroadcaster.VcsStatusBroadcaster)({ + refreshStatus: (cwd) => + Queue.offer(refreshes, `status ${cwd}`).pipe(Effect.andThen(Effect.die("unused"))), + }), + Layer.mock(ExternalLauncher.ExternalLauncher)({}), + Layer.mock(ProviderRegistry.ProviderRegistry)({}), + Layer.mock(UsageLimitSources.UsageLimitSources)({}), + Layer.mock(RemoteOpenTargets.RemoteOpenTargets)({}), + Layer.mock(DirectEndpoints.DirectEndpoints)({}), + Layer.mock(ManagedProjectFolders.ManagedProjectFolders)({ namedProjectsRoot: "/unused" }), + Layer.mock(ScratchThreadPromotion.ScratchThreadPromotion)({}), + Layer.mock(ServerEnvironment.ServerEnvironment)({}), + Layer.mock(EnvironmentAuth.EnvironmentAuth)({}), + ); + const server = yield* RpcServer.makeNoSerialization(WsRpcGroup, { + ...WS_RPC_SERVER_OPTIONS, + onFromServer: (response) => + response._tag === "Exit" ? Queue.offer(exits, response.exit._tag) : Effect.void, + }).pipe( + Effect.provide( + layerWsRpc( + { + sessionId: AuthSessionId.make("publish-test"), + subject: "publish-test", + method: "browser-session-cookie", + scopes: [AuthSourceControlWriteScope], + }, + {}, + yield* ServerBrowser.ServerBrowser, + () => Effect.void, + { secretReferences: true }, + ).pipe( + Layer.provide(services), + Layer.merge(RpcAuthorization.layer([AuthSourceControlWriteScope])), + ), + ), + ); + yield* server.write(0, { + _tag: "Request", + id: RpcMessage.RequestId("publish"), + tag: WS_METHODS.sourceControlPublishRepository, + payload: { + cwd: "/repo", + provider: "github", + repository: "owner/repo", + visibility: "private", + }, + headers: Headers.empty, + }); + assert.equal(yield* Queue.take(exits), "Failure"); + assert.deepEqual( + [yield* Queue.take(refreshes), yield* Queue.take(refreshes)], + ["identity /repo refresh=true", "status /repo"], + ); + }).pipe( + Effect.provide( + Layer.mergeAll( + Layer.fresh(layerConfigSources), + Layer.mock(PreviewAutomationBroker.PreviewAutomationBroker)({}), + Layer.mock(ServerBrowser.ServerBrowser)({}), + ), + ), + Effect.scoped, + ), +); + // Pruned history cannot be replayed: a cursor below the boundary must resync from a snapshot. it.effect.each(["thread", "shell"] as const)( "%s resume from a cursor below the pruned boundary loads a snapshot", diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index e3c7fdaa47476..079adfb19ce44 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2921,8 +2921,12 @@ export const layerWsRpc = ( [WS_METHODS.subscribeProjectClones]: () => projectCloneTracker.stream, [WS_METHODS.sourceControlPublishRepository]: (input) => sourceControlRepositories.publishRepository(input).pipe( - Effect.tap(() => repositoryIdentityResolver.resolve(input.cwd, { refresh: true })), - Effect.tap(() => refreshGitStatus(input.cwd)), + // A failed push still leaves the new remote added. + Effect.ensuring( + repositoryIdentityResolver + .resolve(input.cwd, { refresh: true }) + .pipe(Effect.andThen(refreshGitStatus(input.cwd))), + ), ), [WS_METHODS.projectsSearchEntries]: (input) => workspaceEntries.search(input).pipe( From 68d5f7ab6c0a3103f5d0de582e81189c2397609e Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 6 Nov 2026 04:22:20 +0000 Subject: [PATCH 007/227] fix(server): reverting files removes what a new .gitignore from the turn was hiding (#29795) Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks --- apps/server/src/vcs/GitVcsDriver.test.ts | 40 ++++++++++++++++++ apps/server/src/vcs/GitVcsDriver.ts | 54 +++++++++++++++++++----- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriver.test.ts b/apps/server/src/vcs/GitVcsDriver.test.ts index b79f93c192d4e..ec8809d895f6d 100644 --- a/apps/server/src/vcs/GitVcsDriver.test.ts +++ b/apps/server/src/vcs/GitVcsDriver.test.ts @@ -193,6 +193,7 @@ it.effect("checkpoint capture skips untracked nested repositories without a comm `160000 commit ${nestedHead}\tcommitted\n`, ); assert.deepEqual(yield* fileSystem.readFile(path.join(cwd, ".git", "index")), originalIndex); + assert.isTrue(yield* driver.checkpoints.restoreCheckpoint({ cwd, checkpointRef })); assert.strictEqual( yield* fileSystem.readFileString(path.join(cwd, nested, "private.txt")), "nested\n", @@ -1126,6 +1127,8 @@ it.effect("restores empty checkpoints without changing paths outside the workspa yield* fileSystem.writeFileString(path.join(cwd, "ignored.txt"), "keep\n"); yield* fileSystem.makeDirectory(path.join(cwd, "untracked")); yield* fileSystem.writeFileString(path.join(cwd, "untracked", "file.txt"), "remove\n"); + yield* fileSystem.writeFileString(path.join(cwd, "untracked", ".gitignore"), "hidden.txt\n"); + yield* fileSystem.writeFileString(path.join(cwd, "untracked", "hidden.txt"), "remove\n"); assert.isTrue( yield* driver.checkpoints.restoreCheckpoint({ cwd, checkpointRef, fallbackToHead: false }), ); @@ -1143,6 +1146,43 @@ it.effect("restores empty checkpoints without changing paths outside the workspa }); assert.strictEqual(staged.stdout.trim(), "outside.txt"); } + yield* fileSystem.writeFileString(path.join(cwd, "checkpoint.txt"), "checkpoint\n"); + yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef }); + yield* fileSystem.writeFileString(path.join(cwd, "checkpoint.txt"), "later\n"); + let deepest = cwd; + for (let depth = 0; depth < 4; depth++) { + yield* fileSystem.writeFileString(path.join(deepest, ".gitignore"), "d/.gitignore\n"); + deepest = path.join(deepest, "d"); + yield* fileSystem.makeDirectory(deepest); + } + yield* fileSystem.writeFileString(path.join(deepest, ".gitignore"), "hidden.txt\n"); + yield* fileSystem.writeFileString(path.join(deepest, "hidden.txt"), "remove\n"); + const incomplete = yield* driver.checkpoints + .restoreCheckpoint({ cwd, checkpointRef, fallbackToHead: false }) + .pipe(Effect.flip); + assert.strictEqual(incomplete._tag, "VcsProcessExitError"); + assert.strictEqual( + yield* fileSystem.readFileString(path.join(cwd, "checkpoint.txt")), + "checkpoint\n", + ); + assert.strictEqual( + (yield* driver.execute({ + operation: "test", + cwd, + args: ["diff", "--cached", "--name-only", "--", "."], + })).stdout, + "", + ); + if (nested) { + assert.strictEqual( + (yield* driver.execute({ + operation: "test", + cwd: root, + args: ["diff", "--cached", "--name-only"], + })).stdout.trim(), + "outside.txt", + ); + } } }).pipe(Effect.scoped, Effect.provide(layerGitContract)), ); diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index a2cd548bcc0b9..d120605d6961f 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -1343,13 +1343,52 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( }), ), ); + const resetIndex = Effect.gen(function* () { + // On Windows clean can remove './' and exit 0. + yield* recreateWorkspace; + yield* execute({ + operation, + cwd: input.cwd, + args: [...longPaths, "reset", "--quiet", "--", "."], + }); + }); yield* recreateWorkspace; - const cleaned = yield* execute({ + const clean = execute({ operation, cwd: input.cwd, args: [...longPaths, "clean", "-fd", "--", "."], + env: { LC_ALL: "C" }, allowNonZeroExit: true, }); + let cleaned = yield* clean; + // Removing a new .gitignore exposes the files it hid; clean again until nothing is left. + for ( + let pass = 1; + pass < 4 && cleaned.exitCode === 0 && /^Removing /m.test(cleaned.stdout); + pass++ + ) { + yield* recreateWorkspace; + cleaned = yield* clean; + } + if (cleaned.exitCode === 0 && /^Removing /m.test(cleaned.stdout)) { + yield* recreateWorkspace; + const remaining = yield* execute({ + operation, + cwd: input.cwd, + args: [...longPaths, "clean", "-dn", "--", "."], + env: { LC_ALL: "C" }, + }); + if (/^Would remove /m.test(remaining.stdout)) { + yield* resetIndex; + return yield* new VcsProcessExitError({ + operation, + command: "git clean", + cwd: input.cwd, + exitCode: 0, + detail: "Could not clean the checkpoint workspace: untracked files remain.", + }); + } + } if (cleaned.exitCode !== 0) { // Git can remove every child, then fail trying to remove './' itself. const emptiedWorkspace = @@ -1359,7 +1398,8 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( Effect.map((entries) => entries.length === 0), Effect.orElseSucceed(() => false), )); - if (!emptiedWorkspace) + if (!emptiedWorkspace) { + yield* resetIndex; return yield* new VcsProcessExitError({ operation, command: "git clean", @@ -1367,15 +1407,9 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* ( exitCode: cleaned.exitCode, detail: cleaned.stderr.trim() || "Could not clean the checkpoint workspace.", }); + } } - // On Windows the same clean can remove './' and exit 0. - yield* recreateWorkspace; - - yield* execute({ - operation, - cwd: input.cwd, - args: [...longPaths, "reset", "--quiet", "--", "."], - }); + yield* resetIndex; return true; }), From fba1997db4438ff2cd3fea72751af07be35f8b51 Mon Sep 17 00:00:00 2001 From: Muzammil Hussain <58479501+Muzammil-Bit@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:22:38 +0500 Subject: [PATCH 008/227] fix(server): Codex titles no longer fail on ChatGPT accounts without gpt-6-luna (#30078) Co-authored-by: Claude Opus 5.5 --- .../CodexTextGeneration.test.ts | 93 +++++++++++++++++-- .../src/textGeneration/CodexTextGeneration.ts | 31 +++++-- 2 files changed, 109 insertions(+), 15 deletions(-) diff --git a/apps/server/src/textGeneration/CodexTextGeneration.test.ts b/apps/server/src/textGeneration/CodexTextGeneration.test.ts index 9dc4f176e3078..926f17053fb11 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.test.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.test.ts @@ -163,6 +163,8 @@ function withFakeCodexEnv( launchArgs?: string; environment?: NodeJS.ProcessEnv; models?: ReadonlyArray; + defaultModel?: string; + customModels?: ReadonlyArray; managedRuntime?: boolean; }, effectFn: (textGeneration: TextGeneration.TextGeneration["Service"]) => Effect.Effect, @@ -171,7 +173,11 @@ function withFakeCodexEnv( const fs = yield* FileSystem.FileSystem; const tempDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-codex-text-" }); const codexPath = yield* makeFakeCodexBinary(tempDir, input); - const config = decodeCodexSettings({ binaryPath: codexPath, launchArgs: input.launchArgs }); + const config = decodeCodexSettings({ + binaryPath: codexPath, + launchArgs: input.launchArgs, + customModels: input.customModels, + }); const textGeneration = yield* makeCodexTextGeneration( config, input.environment === undefined ? undefined : { ...process.env, ...input.environment }, @@ -180,6 +186,7 @@ function withFakeCodexEnv( slug, name: slug, isCustom: false, + ...(slug === input.defaultModel ? { isDefault: true } : {}), capabilities: null, })), ), @@ -224,15 +231,40 @@ it.layer(layerCodexTextGenerationTest)("CodexTextGeneration", (it) => { }), ); - it.effect.each(["gpt-5.6-luna", "openai.gpt-5.6-luna"])( - "dispatches the qualified live model for %s", - (selectedModel) => + it.effect.each([ + { + selectedModel: "gpt-5.6-luna", + models: ["openai.gpt-5.6-luna"], + expected: "openai.gpt-5.6-luna", + }, + { + selectedModel: "openai.gpt-5.6-luna", + models: ["openai.gpt-5.6-luna"], + expected: "openai.gpt-5.6-luna", + }, + { + selectedModel: "openai.gpt-5.6-luna", + models: ["gpt-6-astra", "gpt-5.6-luna"], + defaultModel: "gpt-6-astra", + expected: "gpt-5.6-luna", + }, + { + selectedModel: "gpt-6-luna", + models: ["gpt-5.6-luna", "gpt-6-astra"], + defaultModel: "gpt-6-astra", + expected: "gpt-6-astra", + }, + { selectedModel: "gpt-6-luna", models: ["gpt-5.6-luna"], expected: "gpt-5.6-luna" }, + { selectedModel: "gpt-6-luna", models: [], expected: "gpt-6-luna" }, + ])( + "dispatches $expected for $selectedModel from $models", + ({ selectedModel, models, defaultModel, expected }) => withFakeCodexEnv( { - output: JSON.stringify({ title: "Bedrock title" }), - models: ["openai.gpt-5.6-luna"], - requireArg: "--model openai.gpt-5.6-luna", - forbidArg: "--model gpt-5.6-luna", + output: JSON.stringify({ title: "Live title" }), + models, + ...(defaultModel ? { defaultModel } : {}), + requireArg: `--model ${expected}`, }, (textGeneration) => Effect.gen(function* () { @@ -241,11 +273,54 @@ it.layer(layerCodexTextGenerationTest)("CodexTextGeneration", (it) => { message: "Describe this change", modelSelection: createModelSelection(ProviderInstanceId.make("codex"), selectedModel), }); - expect(result.title).toBe("Bedrock title"); + expect(result.title).toBe("Live title"); }), ), ); + it.effect("drops the requested model's options when falling back to another model", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "Live title" }), + models: ["gpt-6-astra"], + requireArg: '--model gpt-6-astra --config model_reasoning_effort="low"', + forbidArg: 'service_tier="priority"', + }, + (textGeneration) => + textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Describe this change", + modelSelection: createModelSelection(ProviderInstanceId.make("codex"), "gpt-6-luna", [ + { id: "reasoningEffort", value: "none" }, + { id: "serviceTier", value: "priority" }, + ]), + }), + ), + ); + + it.effect("sends a configured custom model the managed snapshot leaves out", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "Live title" }), + models: ["gpt-6-astra", "gpt-5.6-luna"], + defaultModel: "gpt-6-astra", + customModels: ["gpt-5.6-luna-preview"], + managedRuntime: true, + requireArg: '--model gpt-5.6-luna-preview --config model_reasoning_effort="high"', + }, + (textGeneration) => + textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Describe this change", + modelSelection: createModelSelection( + ProviderInstanceId.make("codex"), + "gpt-5.6-luna-preview", + [{ id: "reasoningEffort", value: "high" }], + ), + }), + ), + ); + it.effect("omits a persisted service tier for managed ChatGPT text generation", () => withFakeCodexEnv( { diff --git a/apps/server/src/textGeneration/CodexTextGeneration.ts b/apps/server/src/textGeneration/CodexTextGeneration.ts index e17fa4c4c00e7..cbf095b303abc 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.ts @@ -24,7 +24,11 @@ import { normalizeCliError, toJsonSchemaObject, } from "@t3tools/provider-core/server/textGenerationUtils"; -import { codexModelFamily, getModelSelectionStringOptionValue } from "@t3tools/shared/model"; +import { + codexModelFamily, + getModelSelectionStringOptionValue, + readCustomModelEntries, +} from "@t3tools/shared/model"; import { getCodexServiceTierOptionValue } from "../codexModelOptions.ts"; import * as HostProcess from "@t3tools/shared/HostProcess"; @@ -245,17 +249,32 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func const effectiveEnvironment = resolved?.environment ?? resolvedEnvironment; const models = yield* getModels; const requestedModel = modelSelection.model; - const model = + const listed = models.filter((candidate) => !candidate.isCustom); + // An unlisted slug is one this account can't run, e.g. gpt-6-luna on a ChatGPT login. + const requestedFamily = codexModelFamily(requestedModel); + const matchedModel = models.find((candidate) => candidate.slug === requestedModel)?.slug ?? - models.find( - (candidate) => !candidate.isCustom && codexModelFamily(candidate.slug) === requestedModel, + // Managed snapshots omit configured custom models; send those as requested. + readCustomModelEntries(codexConfig.customModels).find( + (entry) => entry.slug === requestedModel, )?.slug ?? + listed.find((candidate) => codexModelFamily(candidate.slug) === requestedFamily)?.slug; + const model = + matchedModel ?? + listed.find((candidate) => candidate.isDefault)?.slug ?? + listed.find((candidate) => !candidate.isLegacy)?.slug ?? + listed[0]?.slug ?? requestedModel; + // The selection's options were chosen for the requested model and may not fit a fallback. + const fellBack = matchedModel === undefined && model !== requestedModel; const launchArgs = resolveCodexLaunchArgs(effectiveConfig.launchArgs, effectiveEnvironment); const reasoningEffort = - getModelSelectionStringOptionValue(modelSelection, "reasoningEffort") ?? + (fellBack + ? undefined + : getModelSelectionStringOptionValue(modelSelection, "reasoningEffort")) ?? DEFAULT_TEXT_GENERATION_REASONING_EFFORT; - const serviceTier = resolved ? undefined : getCodexServiceTierOptionValue(modelSelection); + const serviceTier = + resolved || fellBack ? undefined : getCodexServiceTierOptionValue(modelSelection); const binary = effectiveConfig.binaryPath || "codex"; const env = { ...effectiveEnvironment, From 69726aac2f6f194c3e3582c134bfabcb44e6ca8c Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 6 Nov 2026 04:22:56 +0000 Subject: [PATCH 009/227] fix(desktop): quit no longer orphans a backend that is slow to stop (#22572) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: SAPHID Co-authored-by: maria-rcks --- apps/desktop/src/app/DesktopApp.ts | 4 +- .../src/backend/DesktopBackendManager.test.ts | 54 +++++++++++++++++++ .../src/backend/DesktopBackendManager.ts | 6 +++ apps/desktop/src/updates/DesktopUpdates.ts | 4 +- 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/app/DesktopApp.ts b/apps/desktop/src/app/DesktopApp.ts index 151c227e47ff4..5d877206d0e7d 100644 --- a/apps/desktop/src/app/DesktopApp.ts +++ b/apps/desktop/src/app/DesktopApp.ts @@ -1,6 +1,5 @@ import * as Cause from "effect/Cause"; import * as Config from "effect/Config"; -import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as Ref from "effect/Ref"; @@ -19,6 +18,7 @@ import * as DesktopClerk from "./DesktopClerk.ts"; import * as DesktopApplicationMenu from "../window/DesktopApplicationMenu.ts"; import * as DesktopWindow from "../window/DesktopWindow.ts"; import * as PreviewPasskeys from "../preview/Passkeys.ts"; +import * as DesktopBackendManager from "../backend/DesktopBackendManager.ts"; import * as DesktopBackendPool from "../backend/DesktopBackendPool.ts"; import * as DesktopEnvironment from "./DesktopEnvironment.ts"; import * as DesktopLegacyLocalStorage from "./DesktopLegacyLocalStorage.ts"; @@ -160,7 +160,7 @@ const stopAllPoolInstances = Effect.fn("desktop.app.stopAllPoolInstances")( const instances = yield* pool.list; yield* Effect.forEach( instances, - (instance) => instance.stop({ timeout: Duration.seconds(5) }), + (instance) => instance.stop({ timeout: DesktopBackendManager.BACKEND_QUIT_STOP_TIMEOUT }), { concurrency: "unbounded" }, ); }, diff --git a/apps/desktop/src/backend/DesktopBackendManager.test.ts b/apps/desktop/src/backend/DesktopBackendManager.test.ts index e4714673777d9..2741daa1ddd07 100644 --- a/apps/desktop/src/backend/DesktopBackendManager.test.ts +++ b/apps/desktop/src/backend/DesktopBackendManager.test.ts @@ -720,4 +720,58 @@ describe("DesktopBackendManager", () => { }).pipe(Effect.provide(TestClock.layer())), ), ); + + it.effect("the quit stop deadline waits for a backend that only exits at the force kill", () => + Effect.scoped( + Effect.gen(function* () { + const spawned = yield* Deferred.make(); + const teardownStarted = yield* Deferred.make(); + const teardownFinished = yield* Deferred.make(); + // The real spawner sends SIGTERM, waits the grace, then SIGKILLs. A backend that + // ignores SIGTERM therefore finishes tearing down just after the grace. + const instance = yield* makeTestInstance({ + spawnerLayer: Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => + Effect.gen(function* () { + const scope = yield* Scope.Scope; + yield* Deferred.succeed(spawned, undefined); + yield* Scope.addFinalizer( + scope, + Deferred.succeed(teardownStarted, undefined).pipe( + Effect.andThen(Effect.sleep(Duration.seconds(6))), + Effect.andThen(Deferred.succeed(teardownFinished, undefined)), + Effect.asVoid, + ), + ); + return makeProcess({ + exitCode: Deferred.await(teardownFinished).pipe( + Effect.as(ChildProcessSpawner.ExitCode(0)), + ), + }); + }), + ), + ), + httpClientLayer: layerHttpClient(() => Effect.never), + }); + yield* instance.start; + yield* Deferred.await(spawned); + + // App quit and the updater stop every backend with this deadline. + const quitDone = yield* Deferred.make(); + const quitFiber = yield* instance + .stop({ timeout: DesktopBackendManager.BACKEND_QUIT_STOP_TIMEOUT }) + .pipe(Effect.andThen(Deferred.succeed(quitDone, undefined)), Effect.forkChild); + + yield* Deferred.await(teardownStarted); + // Past the SIGTERM grace but before the force kill lands: quit must still be waiting. + yield* TestClock.adjust(Duration.millis(5_500)); + assert.isFalse(yield* Deferred.isDone(quitDone)); + + yield* TestClock.adjust(Duration.millis(500)); + yield* Fiber.join(quitFiber); + assert.isTrue(yield* Deferred.isDone(teardownFinished)); + }).pipe(Effect.provide(TestClock.layer())), + ), + ); }); diff --git a/apps/desktop/src/backend/DesktopBackendManager.ts b/apps/desktop/src/backend/DesktopBackendManager.ts index 2f5b57c4e3c95..6e356a7c856bd 100644 --- a/apps/desktop/src/backend/DesktopBackendManager.ts +++ b/apps/desktop/src/backend/DesktopBackendManager.ts @@ -41,6 +41,12 @@ const DEFAULT_BACKEND_READINESS_INTERVAL = Duration.millis(100); const DEFAULT_BACKEND_READINESS_REQUEST_TIMEOUT = Duration.seconds(1); // Match the service launcher's grace period so backends can stop provider sessions. const DEFAULT_BACKEND_TERMINATE_GRACE = Duration.seconds(5); +// Allow SIGKILL and the spawner exit wait after the SIGTERM grace so quit cannot +// orphan a backend whose shutdown hangs. +export const BACKEND_QUIT_STOP_TIMEOUT = Duration.sum( + DEFAULT_BACKEND_TERMINATE_GRACE, + Duration.seconds(2), +); const DEFAULT_BACKEND_OUTPUT_DRAIN_TIMEOUT = Duration.seconds(5); const BACKEND_READINESS_PATH = "/.well-known/t3/environment"; // Three restarts without readiness is about 2 s of backoff: past a blip, early enough to help. diff --git a/apps/desktop/src/updates/DesktopUpdates.ts b/apps/desktop/src/updates/DesktopUpdates.ts index 6279e8b6141a8..14688e5258067 100644 --- a/apps/desktop/src/updates/DesktopUpdates.ts +++ b/apps/desktop/src/updates/DesktopUpdates.ts @@ -25,6 +25,7 @@ import * as Scope from "effect/Scope"; import * as Semaphore from "effect/Semaphore"; import * as Stream from "effect/Stream"; +import * as DesktopBackendManager from "../backend/DesktopBackendManager.ts"; import * as DesktopBackendPool from "../backend/DesktopBackendPool.ts"; import * as DesktopConfig from "../app/DesktopConfig.ts"; import * as DesktopEnvironment from "../app/DesktopEnvironment.ts"; @@ -819,7 +820,8 @@ export const make = Effect.gen(function* () { yield* Ref.set(installRunningBackendsRef, runningInstances); yield* Effect.forEach( instances, - (instance) => instance.stop({ timeout: Duration.seconds(5) }), + (instance) => + instance.stop({ timeout: DesktopBackendManager.BACKEND_QUIT_STOP_TIMEOUT }), { concurrency: "unbounded" }, ); yield* electronUpdater.quitAndInstall({ From 596752cffe9fa1cce1c27159bfabe777fa45046f Mon Sep 17 00:00:00 2001 From: Andrei <108398860+Yuruzuu@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:23:15 +0800 Subject: [PATCH 010/227] =?UTF-8?q?perf(server):=20GitHub=20activity=20rea?= =?UTF-8?q?ds=20skip=20PR-wide=20fields=20on=20later=20review-thread=20pag?= =?UTF-8?q?es=20(8.9=20s=20=E2=86=92=206.8=20s=20median)=20(#30294)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks --- .../src/server/GitHubPullRequestApi.test.ts | 130 +++++++++--------- .../src/server/GitHubPullRequestApi.ts | 70 +++++----- .../src/server/gitHubPullRequestJson.ts | 64 ++++++--- .../src/server/pullRequestReviews.graphql | 89 +++++++----- .../src/server/pullRequestReviews.graphql.ts | 88 +++++++++--- 5 files changed, 265 insertions(+), 176 deletions(-) diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index 705a25b13e85f..f46c91fd69570 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -338,13 +338,14 @@ function threadComments( ids: ReadonlyArray, endCursor: string | null, totalCount = ids.length, + author: Readonly> | null = null, ) { return { totalCount, pageInfo: { hasNextPage: endCursor !== null, endCursor }, nodes: ids.map((id) => ({ id, - author: null, + author, body: id, createdAt: "2026-07-01T00:00:00Z", lastEditedAt: null, @@ -3455,67 +3456,31 @@ layer("GitHubPullRequestApi.layer", (it) => { }), ); - it.effect.each([false, true])( - "keeps thread cursors and reviewers after a refusal (later page: %s)", - (later) => - Effect.gen(function* () { - if (later) - mockedExecute.mockReturnValueOnce( - Effect.succeed( - output( - reviewThreadsPage([thread("T1", "c1")], "next", [ - { - __typename: "User", - login: "prior", - name: null, - avatarUrl: "https://avatars/prior", - }, - ]), - ), - ), - ); - mockedExecute.mockReturnValueOnce(refusedReviewerResponse()); - mockedExecute.mockReturnValueOnce( - Effect.succeed(output(reviewThreadsPage([thread("T2", "c2")], "last"))), - ); - mockedExecute.mockReturnValueOnce( - Effect.succeed(output(reviewThreadsPage([thread("T3", "c3")], null))), - ); - const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; - const read = cli.listReviewThreadComments({ - ...pullRequestInput, - repository: `acme/refused-threads-${later}`, - }); - const conversation = yield* read; - expect( - variablesOf("ReviewThreads").map((v) => [v["after"], v["requestedReviewers"]]), - ).toEqual( - later - ? [ - [null, 50], - ["next", 50], - ["next", 0], - ["last", 0], - ] - : [ - [null, 50], - [null, 0], - ["last", 0], - ], - ); - expect(conversation.comments.map((c) => c.id)).toEqual( - later ? ["c1", "c2", "c3"] : ["c2", "c3"], - ); - expect(conversation.reviewers.map((r) => r.login)).toEqual(later ? ["prior"] : []); - expect(conversation.truncated).toBe(false); - - mockedExecute.mockReturnValueOnce(Effect.succeed(output(reviewThreadsPage([], null)))); - yield* read; - expect(variablesOf("ReviewThreads").at(-1)).toMatchObject({ - after: null, - requestedReviewers: 0, - }); - }), + it.effect("keeps thread cursors after a reviewer refusal", () => + Effect.gen(function* () { + mockedExecute.mockReturnValueOnce(refusedReviewerResponse()); + mockedExecute.mockReturnValueOnce( + Effect.succeed(output(reviewThreadsPage([thread("T2", "c2")], "last"))), + ); + mockedExecute.mockReturnValueOnce( + Effect.succeed(output(reviewThreadsPage([thread("T3", "c3")], null))), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const read = cli.listReviewThreadComments({ + ...pullRequestInput, + repository: "acme/refused-threads", + }); + const conversation = yield* read; + expect(variablesOf("ReviewThreads").map((v) => v["requestedReviewers"])).toEqual([50, 0]); + expect(variablesOf("ReviewThreadPages").map((v) => v["after"])).toEqual(["last"]); + expect(conversation.comments.map((c) => c.id)).toEqual(["c2", "c3"]); + expect(conversation.reviewers).toEqual([]); + expect(conversation.truncated).toBe(false); + + mockedExecute.mockReturnValueOnce(Effect.succeed(output(reviewThreadsPage([], null)))); + yield* read; + expect(variablesOf("ReviewThreads").at(-1)).toMatchObject({ requestedReviewers: 0 }); + }), ); it.effect("decodes reviewers, labels and workflow checks without another detail read", () => @@ -3767,10 +3732,32 @@ layer("GitHubPullRequestApi.layer", (it) => { it.effect("follows the cursor to the review threads the first page left behind", () => Effect.gen(function* () { mockedExecute.mockReturnValueOnce( - Effect.succeed(output(reviewThreadsPage([thread("PRRT_1", "c1")], "Y3Vyc29yOjE"))), + Effect.succeed( + output( + reviewThreadsPage([thread("PRRT_1", "c1")], "Y3Vyc29yOjE", [ + { __typename: "User", login: "prior", name: null, avatarUrl: "https://avatars/p" }, + ]), + ), + ), ); mockedExecute.mockReturnValueOnce( - Effect.succeed(output(reviewThreadsPage([thread("PRRT_2", "c2")], null))), + Effect.succeed( + output( + reviewThreadsPage( + [ + { + ...thread("PRRT_2", "c2"), + comments: threadComments(["c2"], null, 1, { + __typename: "Bot", + login: "later-bot", + avatarUrl: "https://avatars/later-bot", + }), + }, + ], + null, + ), + ), + ), ); const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; @@ -3781,9 +3768,20 @@ layer("GitHubPullRequestApi.layer", (it) => { number: 7, }); - expect(varsAt(0)["after"]).toBeNull(); - expect(varsAt(1)["after"]).toBe("Y3Vyc29yOjE"); + expect(callAt(0)).toMatchObject({ operationName: "ReviewThreads" }); + // Later pages ask for the threads alone and keep the first page's PR-wide fields. + const later = callAt(1); + assert(later.kind === "graphql"); + expect(later).toMatchObject({ + operationName: "ReviewThreadPages", + variables: { after: "Y3Vyc29yOjE" }, + }); + for (const field of ["reviewRequests", "commits(", "reviewDismissals", "latestReviews"]) + expect(later.query).not.toContain(field); expect(conversation.comments.map((comment) => comment.id)).toEqual(["c1", "c2"]); + expect(conversation.reviewers.map((reviewer) => reviewer.login)).toEqual(["prior"]); + expect(conversation.avatarsByLogin.get("later-bot")).toBe("https://avatars/later-bot"); + expect(conversation.botLogins.has("later-bot")).toBe(true); assert.isFalse(conversation.truncated); }), ); diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index c4fbbfcd9f64e..ce38cc726a249 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -33,7 +33,6 @@ import { type PullRequestListState, type PullRequestMergeMethod, type PullRequestOmittedFileStat, - type PullRequestReaction, type PullRequestReactionContent, type PullRequestReviewCommentDraft, type PullRequestReviewVerdict, @@ -95,6 +94,7 @@ import { toReviewDismissalsPage, toReviewerCandidates, toReviewThreadCommentsPage, + toLaterReviewThreadPage, toReviewThreadPage, toStackMembership, toViewerPermissions, @@ -2216,45 +2216,39 @@ export const make = Effect.gen(function* () { function* (input) { const { owner, name } = parseRepositorySelector(input.repository); const target = { ...input, operation: "listReviewThreadComments" }; - const threadPage = ( - after: string | null, - ): Effect.Effect => - readRequestedReviewers(input, 50, (requestedReviewers) => - client.ReviewThreads({ owner, name, number: input.number, after, requestedReviewers }), - ).pipe(graphql(target), Effect.map(toReviewThreadPage), Effect.flatMap(found(target))); - const entries: GitHubReviewThreadEntry[] = []; - const avatarsByLogin = new Map(); - const botLogins = new Set(); - let reviewers: ReadonlyArray = []; - let reactions: GitHubReviewThreadPage["reactions"] = []; - const reactionsById = new Map>(); - const editedAtById = new Map(); - let commits: GitHubReviewThreadPage["commits"] = []; - let viewer: GitHubReviewThreadPage["viewer"] = { canUpdate: true, didAuthor: false }; - const dismissalsByReviewId = new Map(); - let dismissalCursor: string | null = null; - let cursor: string | null = null; - let page = 0; - do { - const read: GitHubReviewThreadPage = yield* threadPage(cursor); - entries.push(...read.threads); - for (const login of read.botLogins) botLogins.add(login); - for (const [login, avatarUrl] of read.avatarsByLogin) + const first: GitHubReviewThreadPage = yield* readRequestedReviewers( + input, + 50, + (requestedReviewers) => + client.ReviewThreads({ owner, name, number: input.number, requestedReviewers }), + ).pipe(graphql(target), Effect.map(toReviewThreadPage), Effect.flatMap(found(target))); + const entries: GitHubReviewThreadEntry[] = [...first.threads]; + const avatarsByLogin = new Map(first.avatarsByLogin); + const botLogins = new Set(first.botLogins); + const { reviewers, reactions, reactionsById, editedAtById, commits, viewer } = first; + const dismissalsByReviewId = new Map(first.dismissalsByReviewId); + const dismissalCursor = first.nextDismissalCursor; + let cursor = first.nextCursor; + let page = 1; + // Later pages carry the threads alone; the PR-wide fields above come from the first. + while (cursor !== null && page < REVIEW_THREAD_PAGES) { + const later: Pick< + GitHubReviewThreadPage, + "threads" | "nextCursor" | "avatarsByLogin" | "botLogins" + > = yield* client + .ReviewThreadPages({ owner, name, number: input.number, after: cursor }) + .pipe( + graphql(target), + Effect.map(toLaterReviewThreadPage), + Effect.flatMap(found(target)), + ); + entries.push(...later.threads); + for (const login of later.botLogins) botLogins.add(login); + for (const [login, avatarUrl] of later.avatarsByLogin) avatarsByLogin.set(login, avatarUrl); - if (page === 0) { - reviewers = read.reviewers; - reactions = read.reactions; - for (const [id, entry] of read.reactionsById) reactionsById.set(id, entry); - for (const [id, editedAt] of read.editedAtById) editedAtById.set(id, editedAt); - commits = read.commits; - viewer = read.viewer; - for (const [id, message] of read.dismissalsByReviewId) - dismissalsByReviewId.set(id, message); - dismissalCursor = read.nextDismissalCursor; - } - cursor = read.nextCursor; + cursor = later.nextCursor; page += 1; - } while (cursor !== null && page < REVIEW_THREAD_PAGES); + } if (dismissalCursor !== null) { let after: string | null = dismissalCursor; diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index 98a95a363e31d..a4c509e944488 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -75,6 +75,8 @@ import type { ReactionGroup, ReviewDismissals, ReviewThreadComments, + ReviewThreadPage, + ReviewThreadPages, ReviewThreads, ThreadComment, } from "./pullRequestReviews.graphql.ts"; @@ -1313,11 +1315,8 @@ type ReviewThreadsPullRequest = NonNullable< NonNullable["pullRequest"] >; -export function toReviewThreadPage(result: ReviewThreads.Result): GitHubReviewThreadPage | null { - const pullRequest = result.repository?.pullRequest; - if (pullRequest == null) return null; - const viewer = trimmed(result.viewer.login); - const threads = pullRequest.reviewThreads; +/** A page of threads and the people commenting on them, whichever query carried it. */ +function toReviewThreadEntries(threads: ReviewThreadPage, viewer: string | null) { const threadNodes = nodesOf(threads); const entries = threadNodes.flatMap((thread): ReadonlyArray => { const path = trimmed(thread.path); @@ -1341,27 +1340,58 @@ export function toReviewThreadPage(result: ReviewThreads.Result): GitHubReviewTh }, ]; }); + return { + entries, + nextCursor: nextCursorOf(threads.pageInfo), + authors: threadNodes.flatMap((thread) => + nodesOf(thread.comments).map((comment) => comment.author), + ), + }; +} + +function collectActors(raws: ReadonlyArray) { + const avatarsByLogin = new Map(); + const botLogins = new Set(); + for (const raw of raws) { + const login = trimmed(raw?.login); + const avatarUrl = trimmed(raw?.avatarUrl); + if (login !== null && avatarUrl !== null) avatarsByLogin.set(login, avatarUrl); + if (login !== null && toActor(raw)?.isBot) botLogins.add(login); + } + return { avatarsByLogin, botLogins }; +} + +export function toLaterReviewThreadPage( + result: ReviewThreadPages.Result, +): Pick | null { + const threads = result.repository?.pullRequest?.reviewThreads; + if (threads === undefined) return null; + const { entries, nextCursor, authors } = toReviewThreadEntries( + threads, + trimmed(result.viewer.login), + ); + return { threads: entries, nextCursor, ...collectActors(authors) }; +} + +export function toReviewThreadPage(result: ReviewThreads.Result): GitHubReviewThreadPage | null { + const pullRequest = result.repository?.pullRequest; + if (pullRequest == null) return null; + const viewer = trimmed(result.viewer.login); + const page = toReviewThreadEntries(pullRequest.reviewThreads, viewer); const requestedReviewers = nodesOf(pullRequest.reviewRequests).map( (node): ActorFields | null => node.requestedReviewer, ); const latestReviews = nodesOf(pullRequest.latestReviews); const comments = nodesOf(pullRequest.comments); const reviews = nodesOf(pullRequest.reviews); - const avatarsByLogin = new Map(); - const botLogins = new Set(); - for (const raw of [ + const { avatarsByLogin, botLogins } = collectActors([ pullRequest.author, ...comments.map((node) => node.author), ...reviews.map((node) => node.author), ...requestedReviewers, ...latestReviews.map((node) => node.author), - ...threadNodes.flatMap((thread) => nodesOf(thread.comments).map((comment) => comment.author)), - ]) { - const login = trimmed(raw?.login); - const avatarUrl = trimmed(raw?.avatarUrl); - if (login !== null && avatarUrl !== null) avatarsByLogin.set(login, avatarUrl); - if (login !== null && toActor(raw)?.isBot) botLogins.add(login); - } + ...page.authors, + ]); const reviewers = new Map(); for (const raw of [...requestedReviewers, ...latestReviews.map((node) => node.author)]) { const actor = toActor(raw) ?? toRequestedTeam(raw); @@ -1391,8 +1421,8 @@ export function toReviewThreadPage(result: ReviewThreads.Result): GitHubReviewTh if (reactions.length > 0) reactionsById.set(id, reactions); } return { - threads: entries, - nextCursor: nextCursorOf(threads.pageInfo), + threads: page.entries, + nextCursor: page.nextCursor, reactions: toReactions(pullRequest.reactionGroups, viewer), reactionsById, editedAtById, diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql b/packages/source-control-github/src/server/pullRequestReviews.graphql index 96c35783dc1b6..94646874d803d 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql @@ -40,9 +40,39 @@ fragment ThreadComment on PullRequestReviewComment { ...Reactions } -# One page of review threads with their comments, and the people on the review. `$after` is null -# for the first page and the last page's `endCursor` after that, so a pull request with more -# threads than one page holds is walked rather than cut off. +# One page of review threads. Only ten comments ride with each thread. A hundred threads times a +# hundred comments made GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads +# are paged from their own cursor by `ReviewThreadComments`. +fragment ReviewThreadPage on PullRequestReviewThreadConnection { + totalCount + pageInfo { + hasNextPage + endCursor + } + nodes { + id + isResolved + isOutdated + path + # Null once the thread's line has left the diff, which `isOutdated` reports. + line + diffSide + comments(first: 10) { + totalCount + pageInfo { + hasNextPage + endCursor + } + nodes { + ...ThreadComment + } + } + } +} + +# The first page of review threads with their comments, and the people on the review. Later pages +# come from `ReviewThreadPages`, so a pull request with more threads than one page holds is walked +# rather than cut off. # # `viewer` rides along: GitHub names who reacted but never says whether that is the reader, so the # comparison is made locally rather than paid for with a request of its own. @@ -53,46 +83,14 @@ fragment ThreadComment on PullRequestReviewComment { # # `viewerCanUpdate` and `viewerDidAuthor` belong to the pull request this query is already # standing on, so what the reader may do with it arrives with the conversation. -query ReviewThreads( - $owner: String! - $name: String! - $number: Int! - $after: String - $requestedReviewers: Int! -) { +query ReviewThreads($owner: String!, $name: String!, $number: Int!, $requestedReviewers: Int!) { viewer { login } repository(owner: $owner, name: $name) { pullRequest(number: $number) { - reviewThreads(first: 100, after: $after) { - totalCount - pageInfo { - hasNextPage - endCursor - } - nodes { - id - isResolved - isOutdated - path - # Null once the thread's line has left the diff, which `isOutdated` reports. - line - diffSide - # Only ten comments ride with each thread. A hundred threads times a hundred comments - # made GitHub reserve 10,000 nested rows and charge 104 points; unfinished threads are - # paged from their own cursor by `ReviewThreadComments`. - comments(first: 10) { - totalCount - pageInfo { - hasNextPage - endCursor - } - nodes { - ...ThreadComment - } - } - } + reviewThreads(first: 100) { + ...ReviewThreadPage } viewerCanUpdate viewerDidAuthor @@ -198,6 +196,21 @@ query ReviewThreads( } } +# The review threads past the first page. Everything else `ReviewThreads` asks for describes the +# whole pull request and arrived with that page, so later pages carry the threads alone. +query ReviewThreadPages($owner: String!, $name: String!, $number: Int!, $after: String!) { + viewer { + login + } + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $after) { + ...ReviewThreadPage + } + } + } +} + # The rest of one thread's conversation. GraphQL pages a connection nested inside another only # from the inner node itself, so a thread longer than a page is followed on its own: a request # GitHub makes necessary, and one no ordinary pull request ever provokes. diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts index 0711ed9ebcfa5..9a1351f8cd138 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts @@ -112,13 +112,6 @@ export const ThreadComment = Schema.Struct({ }) export interface ThreadComment extends Schema.Struct.Type {} -/** A user is an individual's account on GitHub that owns repositories and can make new content. */ -export const ViewerUser = Schema.Struct({ - /** The username used to login. */ - login: Schema.String -}) -export interface ViewerUser extends Schema.Struct.Type {} - /** Information about pagination in a connection. */ export const PageInfo = Schema.Struct({ /** When paginating forwards, are there more items? */ @@ -158,8 +151,12 @@ export const NodesPullRequestReviewThread = Schema.Struct({ }) export interface NodesPullRequestReviewThread extends Schema.Struct.Type {} -/** Review comment threads for a pull request review. */ -export const PullRequestReviewThreadConnection = Schema.Struct({ +/** + * `fragment ReviewThreadPage on PullRequestReviewThreadConnection` + * + * Review comment threads for a pull request review. + */ +export const ReviewThreadPage = Schema.Struct({ /** Identifies the total count of items in the connection. */ totalCount: Shared.Int, /** Information to aid in pagination. */ @@ -167,7 +164,14 @@ export const PullRequestReviewThreadConnection = Schema.Struct({ /** A list of nodes. */ nodes: Schema.NullOr(Schema.Array(Schema.NullOr(NodesPullRequestReviewThread))) }) -export interface PullRequestReviewThreadConnection extends Schema.Struct.Type {} +export interface ReviewThreadPage extends Schema.Struct.Type {} + +/** A user is an individual's account on GitHub that owns repositories and can make new content. */ +export const ViewerUser = Schema.Struct({ + /** The username used to login. */ + login: Schema.String +}) +export interface ViewerUser extends Schema.Struct.Type {} /** Represents a comment on an Issue. */ export const IssueComment = Schema.Struct({ @@ -370,7 +374,7 @@ export interface PullRequestCommitConnection extends Schema.Struct.Type {} + +/** A repository contains the content for a project. */ +export const ReviewThreadPagesRepositoryRepository = Schema.Struct({ + /** Returns a single pull request from the current repository by number. */ + pullRequest: Schema.NullOr(ReviewThreadPagesRepositoryPullRequestPullRequest) +}) +export interface ReviewThreadPagesRepositoryRepository extends Schema.Struct.Type {} + +/** `query ReviewThreadPages` */ +export const ReviewThreadPages = GraphQL.query("ReviewThreadPages", { + document: { + variables: [ + { name: "owner", body: "String!" }, + { name: "name", body: "String!" }, + { name: "number", body: "Int!" }, + { name: "after", body: "String!" } + ], + selections: [ + { name: "viewer", body: ["{login}"] }, + { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){reviewThreads(first:100 after:$", "after", "){...ReviewThreadPage}}}"] } + ], + fragments: [ + { name: "ReviewThreadPage", body: ["on PullRequestReviewThreadConnection{totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] }, + { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url...Reactions}"] }, + { name: "Reactions", body: ["on Reactable{__typename reactionGroups{content viewerHasReacted reactors(first:10){totalCount nodes{__typename...on User{login}...on Bot{login}...on Organization{login}...on Mannequin{login}}}}}"] } + ] + }, + variables: { + owner: Schema.String, + name: Schema.String, + number: Shared.Int, + after: Schema.String + }, + result: Schema.Struct({ + /** The currently authenticated user. */ + viewer: ViewerUser, + /** Lookup a given repository by the owner and repository name. */ + repository: Schema.NullOr(ReviewThreadPagesRepositoryRepository) + }) +}) +export declare namespace ReviewThreadPages { + export type Variables = typeof ReviewThreadPages.variables.Type + export type Result = typeof ReviewThreadPages.result.Type +} + /** A repository pull request. */ export const ReviewThreadCommentsRepositoryPullRequestPullRequest = Schema.Struct({ /** The Node ID of the PullRequest object */ @@ -567,4 +621,4 @@ export declare namespace ReviewDismissals { } /** Every operation in pullRequestReviews.graphql. */ -export const PullRequestReviewsGraphQLGroup = GraphQLGroup.make(ReviewThreads, ReviewThreadComments, ReviewDismissals) +export const PullRequestReviewsGraphQLGroup = GraphQLGroup.make(ReviewThreads, ReviewThreadPages, ReviewThreadComments, ReviewDismissals) From 93dbf53db31387e18866ac64bf4488e8994ad86d Mon Sep 17 00:00:00 2001 From: Andrei <108398860+Yuruzuu@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:23:32 +0800 Subject: [PATCH 011/227] =?UTF-8?q?perf(server):=20the=20headless=20browse?= =?UTF-8?q?r=20closes=20after=20five=20idle=20minutes=20(517=20MiB=20?= =?UTF-8?q?=E2=86=92=201.6=20MiB=20median)=20(#30306)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Claude Opus 5.5 --- apps/server/src/preview/ServerBrowser.ts | 1 + .../src/preview/ServerBrowserContexts.test.ts | 42 ++++++++++++++++-- .../src/preview/ServerBrowserContexts.ts | 43 +++++++++++++++++-- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 6b92e33ce960f..264a875bf6fdf 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -501,6 +501,7 @@ const make = Effect.gen(function* () { const contexts = new ServerBrowserContexts({ profilesDir: NodePath.join(config.stateDir, "server-browser", "profiles"), + idleBrowserMs: 5 * 60_000, executable: () => runPromise(previewBrowser.executable), // An agent scope keeps Chrome out of the server's memory protection and OOM score. launcher: async (executable) => { diff --git a/apps/server/src/preview/ServerBrowserContexts.test.ts b/apps/server/src/preview/ServerBrowserContexts.test.ts index 522df30fb66c8..99504e86cb1b2 100644 --- a/apps/server/src/preview/ServerBrowserContexts.test.ts +++ b/apps/server/src/preview/ServerBrowserContexts.test.ts @@ -22,6 +22,8 @@ const makeContext = () => { return { on: vi.fn((event: string, listener: () => void) => events.on(event, listener)), pages: vi.fn(() => []), + newPage: vi.fn(async () => ({ once: vi.fn() })), + isClosed: () => closed, close: vi.fn(async () => { if (!closed) { closed = true; @@ -35,7 +37,8 @@ const makeBrowser = () => { const events = new NodeEvents.EventEmitter(); const contexts: ReturnType[] = []; return { - contexts, + created: contexts, + contexts: () => contexts.filter((context) => !context.isClosed()), on: vi.fn((event: string, listener: () => void) => events.on(event, listener)), newContext: vi.fn(async () => { const context = makeContext(); @@ -74,6 +77,37 @@ beforeEach(() => { }); describe("ServerBrowserContexts", () => { + it("closes the shared browser only after it holds no context for the idle period", async () => { + vi.useFakeTimers(); + try { + const first = makeBrowser(); + const second = makeBrowser(); + launches.launch + .mockResolvedValueOnce(first as unknown as Browser) + .mockResolvedValueOnce(second as unknown as Browser); + const pool = new ServerBrowserContexts({ ...options(), idleBrowserMs: 10 }); + const agent = await pool.contextFor("default", "agent"); + await pool.scratchPage(); + await agent.close(); + await vi.advanceTimersByTimeAsync(10); + expect(first.close).not.toHaveBeenCalled(); + await first.created[1]!.close(); + await vi.advanceTimersByTimeAsync(9); + const next = await pool.contextFor("default", "next"); + await vi.advanceTimersByTimeAsync(10); + expect(first.close).not.toHaveBeenCalled(); + await next.close(); + await vi.advanceTimersByTimeAsync(10); + expect(first.close).toHaveBeenCalledOnce(); + await pool.contextFor("default", "later"); + expect(launches.launch).toHaveBeenCalledTimes(2); + expect(second.newContext).toHaveBeenCalledOnce(); + await pool.close(); + } finally { + vi.useRealTimers(); + } + }); + it("lazily shares one browser across concurrent isolated sessions and reuses each context", async () => { const browser = makeBrowser(); const launch = Promise.withResolvers(); @@ -107,7 +141,7 @@ describe("ServerBrowserContexts", () => { }); await pool.close(); expect(browser.close).toHaveBeenCalledTimes(1); - for (const context of browser.contexts) expect(context.close).toHaveBeenCalled(); + for (const context of browser.created) expect(context.close).toHaveBeenCalled(); }); it("keeps human profiles persistent without sharing their storage with agents", async () => { @@ -172,7 +206,7 @@ describe("ServerBrowserContexts", () => { force: true, }); expect(await pool.contextFor("work/team", "agent")).toBe(agent); - expect(browser.contexts[0]?.close).not.toHaveBeenCalled(); + expect(browser.created[0]?.close).not.toHaveBeenCalled(); launches.persistent.mockResolvedValue(makeContext() as unknown as BrowserContext); expect(await pool.contextFor("work/team")).not.toBe(persistent); await pool.clearProfile("never-opened"); @@ -291,6 +325,6 @@ describe("ServerBrowserContexts", () => { await expect(pool.contextFor("default", "agent")).rejects.toThrow("closed"); await pool.close(); expect(browser.close).toHaveBeenCalledTimes(1); - expect(browser.contexts[0]?.close).toHaveBeenCalled(); + expect(browser.created[0]?.close).toHaveBeenCalled(); }); }); diff --git a/apps/server/src/preview/ServerBrowserContexts.ts b/apps/server/src/preview/ServerBrowserContexts.ts index 2a7c342d30d81..365add7565c69 100644 --- a/apps/server/src/preview/ServerBrowserContexts.ts +++ b/apps/server/src/preview/ServerBrowserContexts.ts @@ -1,4 +1,4 @@ -// @effect-diagnostics nodeBuiltinImport:off - Owns Playwright resources outside the Effect runtime. +// @effect-diagnostics nodeBuiltinImport:off globalTimers:off -- Owns Playwright resources and their idle timer outside the Effect runtime. import { INCOGNITO_BROWSER_PROFILE_ID } from "@t3tools/contracts"; import { constVoid } from "effect/Function"; import * as NodeFSP from "node:fs/promises"; @@ -26,6 +26,8 @@ interface Options { ) => Promise<{ readonly path: string; readonly release: () => Promise } | undefined>; readonly diagnose?: (executable: string, cause: unknown) => Promise; readonly env?: NodeJS.ProcessEnv; + /** Closes the shared browser once it has held no context for this long. */ + readonly idleBrowserMs?: number; readonly onContextClose?: (context: BrowserContext) => void; } @@ -80,6 +82,8 @@ export class ServerBrowserContexts { private readonly clearing = new Map>(); private browser: Promise | undefined; private closing: Promise | undefined; + private idleTimer: ReturnType | undefined; + private pendingSharedContexts = 0; constructor(options: Options) { this.options = options; @@ -136,6 +140,37 @@ export class ServerBrowserContexts { return this.browser; } + private async sharedContext(options?: Parameters[0]) { + clearTimeout(this.idleTimer); + this.pendingSharedContexts++; + const launched = this.sharedBrowser(); + try { + const context = await (await launched).newContext(options); + context.on("close", () => this.retireWhenIdle(launched)); + return context; + } finally { + this.pendingSharedContexts--; + this.retireWhenIdle(launched); + } + } + + private retireWhenIdle(launched: Promise) { + const delay = this.options.idleBrowserMs; + if (delay === undefined || this.closing || this.pendingSharedContexts > 0) return; + clearTimeout(this.idleTimer); + this.idleTimer = setTimeout(() => void this.retire(launched), delay); + this.idleTimer.unref(); + } + + private async retire(launched: Promise) { + const browser = await launched.catch(() => undefined); + if (!browser || this.browser !== launched || this.pendingSharedContexts > 0) return; + if (browser.contexts().length > 0) return; + // Cleared first, so a request during the close launches a fresh browser. + this.browser = undefined; + await browser.close().catch(constVoid); + } + contextFor(profileId: string, isolationKey?: string): Promise { if (this.closing) return Promise.reject(new Error("The preview browser is closed.")); const key = JSON.stringify([profileId, isolationKey ?? null]); @@ -166,8 +201,7 @@ export class ServerBrowserContexts { private async createContext(profileId: string, isolationKey?: string) { const contextOptions = { viewport: { width: 1280, height: 800 }, deviceScaleFactor: 2 }; if (isolationKey !== undefined || profileId === INCOGNITO_BROWSER_PROFILE_ID) { - const browser = await this.sharedBrowser(); - return browser.newContext(contextOptions); + return this.sharedContext(contextOptions); } const directory = this.profileDirectory(profileId); await NodeFSP.mkdir(directory, { recursive: true }); @@ -192,7 +226,7 @@ export class ServerBrowserContexts { async scratchPage() { if (this.closing) throw new Error("The preview browser is closed."); - const context = await (await this.sharedBrowser()).newContext(); + const context = await this.sharedContext(); const page = await context.newPage(); page.once("close", () => void context.close().catch(constVoid)); return page; @@ -237,6 +271,7 @@ export class ServerBrowserContexts { } private async dispose() { + clearTimeout(this.idleTimer); await Promise.allSettled( [...this.contexts.values()].map(async (pending) => (await pending).close()), ); From b15f772ebee6bab65950cb2b3404736d480eec0d Mon Sep 17 00:00:00 2001 From: Nico Date: Fri, 6 Nov 2026 05:23:51 +0100 Subject: [PATCH 012/227] fix(provider-acp-registry): surface agent error message on sign-in failure (#30295) --- .../src/server/auth.test.ts | 138 +++++++++++++++++- .../provider-acp-registry/src/server/auth.ts | 30 +++- 2 files changed, 165 insertions(+), 3 deletions(-) diff --git a/packages/provider-acp-registry/src/server/auth.test.ts b/packages/provider-acp-registry/src/server/auth.test.ts index b81e2e3fba73e..4bb647bd94069 100644 --- a/packages/provider-acp-registry/src/server/auth.test.ts +++ b/packages/provider-acp-registry/src/server/auth.test.ts @@ -5,6 +5,7 @@ import { assert, it } from "@effect/vitest"; import { ProviderInstanceId, type ProviderAuthState } from "@t3tools/contracts"; import { AcpRegistrySettings } from "../settings.ts"; import * as Deferred from "effect/Deferred"; +import * as Cause from "effect/Cause"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; @@ -13,7 +14,7 @@ import type * as AcpSchema from "effect-acp/compat"; import { AcpRequestError } from "effect-acp/errors"; import * as PtyAdapter from "@t3tools/shared/PtyAdapter"; -import { makeAcpRegistryAuth } from "./auth.ts"; +import { formatFailureDetail, makeAcpRegistryAuth } from "./auth.ts"; import * as AcpRegistrySupport from "./AcpRegistrySupport.ts"; import type * as AcpSessionRuntime from "@t3tools/provider-acp/server/AcpSessionRuntime"; @@ -61,6 +62,7 @@ const makeHarness = ( failVerification = false, settings = decodeSettings({ agentId: "test-agent" }), providerInstanceId = instanceId, + authenticateError?: AcpRequestError, ) => Effect.gen(function* () { const verify = yield* Deferred.make(); @@ -111,6 +113,9 @@ const makeHarness = ( authenticate: (methodId) => Effect.gen(function* () { authenticated.push(methodId); + if (authenticateError !== undefined) { + return yield* authenticateError; + } if (elicitation) yield* elicitation( { @@ -342,8 +347,137 @@ it.effect("does not report authentication when post-login verification fails", ( h.exit(0); yield* h.phase("verifying"); yield* Deferred.succeed(h.verify, undefined); - yield* h.phase("failed"); + const failed = yield* h.phase("failed"); assert.deepEqual(h.changed, []); assert.isTrue(h.killed()); + assert.strictEqual( + failed.message, + "The provider could not create a session after terminal sign-in. Authentication required", + ); + }).pipe(Effect.scoped, Effect.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))), +); + +it.effect("surfaces the agent error message when authenticate fails with AcpRequestError", () => + Effect.gen(function* () { + const error = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: "Connect to the VPN, then sign in again.", + }); + const h = yield* makeHarness( + browserMethod, + false, + decodeSettings({ agentId: "test-agent" }), + instanceId, + error, + ); + yield* h.controller.start("owner", Effect.void, "browser"); + const failed = yield* h.phase("failed"); + assert.strictEqual( + failed.message, + "The ACP agent could not complete sign-in. Connect to the VPN, then sign in again.", + ); + assert.deepEqual(h.changed, []); }).pipe(Effect.scoped, Effect.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))), ); + +it.effect( + "surfaces the agent error message when post-sign-in verification fails for agent auth", + () => + Effect.gen(function* () { + const h = yield* makeHarness(browserMethod, true); + const flow = yield* h.controller.start("owner", Effect.void, "browser"); + yield* h.phase("waiting"); + yield* h.controller.respond!("owner", { + instanceId, + flowId: flow.flowId!, + interactionId: "consent", + response: { type: "browser", action: "accept" }, + }); + yield* h.phase("verifying"); + yield* Deferred.succeed(h.verify, undefined); + const failed = yield* h.phase("failed"); + assert.strictEqual( + failed.message, + "The provider could not create a session after sign-in. Authentication required", + ); + assert.deepEqual(h.changed, []); + }).pipe(Effect.scoped, Effect.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))), +); + +it("formatFailureDetail handles all cause types and formatting edge cases cleanly", () => { + const baseWithPeriod = "The ACP agent could not complete sign-in."; + const baseWithoutPeriod = "The ACP agent could not complete sign-in"; + + // 1. undefined or null cause + assert.strictEqual(formatFailureDetail(baseWithPeriod, undefined), baseWithPeriod); + assert.strictEqual(formatFailureDetail(baseWithPeriod, null), baseWithPeriod); + + // 2. plain Error object, string, number, or object defect + assert.strictEqual( + formatFailureDetail(baseWithPeriod, new Error("Something broke")), + baseWithPeriod, + ); + assert.strictEqual(formatFailureDetail(baseWithPeriod, "string error"), baseWithPeriod); + assert.strictEqual(formatFailureDetail(baseWithPeriod, { foo: "bar" }), baseWithPeriod); + + // 3. AcpRequestError with empty or whitespace-only errorMessage + const emptyError = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: "", + }); + const whitespaceError = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: " \t\n ", + }); + assert.strictEqual(formatFailureDetail(baseWithPeriod, emptyError), baseWithPeriod); + assert.strictEqual(formatFailureDetail(baseWithPeriod, whitespaceError), baseWithPeriod); + + // 4. AcpRequestError with valid message: ensure no double periods, clean separation + const validError = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: "Connect to the VPN, then sign in again.", + }); + assert.strictEqual( + formatFailureDetail(baseWithPeriod, validError), + "The ACP agent could not complete sign-in. Connect to the VPN, then sign in again.", + ); + assert.strictEqual( + formatFailureDetail(baseWithoutPeriod, validError), + "The ACP agent could not complete sign-in. Connect to the VPN, then sign in again.", + ); + + // 5. AcpRequestError with trimmed message and message without ending period + const unpunctError = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: " Invalid credentials ", + }); + assert.strictEqual( + formatFailureDetail(baseWithPeriod, unpunctError), + "The ACP agent could not complete sign-in. Invalid credentials", + ); + + // 6. Nested Cause (e.g. Cause.fail or Cause.die wrapping AcpRequestError) + assert.strictEqual( + formatFailureDetail(baseWithPeriod, Cause.fail(validError)), + "The ACP agent could not complete sign-in. Connect to the VPN, then sign in again.", + ); + assert.strictEqual( + formatFailureDetail(baseWithPeriod, Cause.die(validError)), + "The ACP agent could not complete sign-in. Connect to the VPN, then sign in again.", + ); + + // 7. Extremely long error message: truncates to 500 characters with ellipsis + const longText = "A".repeat(600); + const longError = new AcpRequestError({ + method: "authenticate", + code: -32000, + errorMessage: longText, + }); + const expectedLong = `The ACP agent could not complete sign-in. ${"A".repeat(500)}…`; + assert.strictEqual(formatFailureDetail(baseWithPeriod, longError), expectedLong); +}); diff --git a/packages/provider-acp-registry/src/server/auth.ts b/packages/provider-acp-registry/src/server/auth.ts index 240bca67cea57..9f8d36880f27b 100644 --- a/packages/provider-acp-registry/src/server/auth.ts +++ b/packages/provider-acp-registry/src/server/auth.ts @@ -5,6 +5,7 @@ import { } from "@t3tools/contracts"; import type { AcpRegistrySettings } from "../settings.ts"; import * as SecureRandom from "effect/crypto/SecureRandom"; +import * as Cause from "effect/Cause"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; @@ -13,7 +14,9 @@ import * as Option from "effect/Option"; import * as Queue from "effect/Queue"; import * as Scope from "effect/Scope"; import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; +import * as Schema from "effect/Schema"; import type * as AcpSchema from "effect-acp/compat"; +import * as EffectAcpErrors from "effect-acp/errors"; import * as PtyAdapter from "@t3tools/shared/PtyAdapter"; import * as ProviderAuthFlow from "@t3tools/provider-core/server/providerAuthFlow"; @@ -22,6 +25,26 @@ import * as AcpRegistrySupport from "./AcpRegistrySupport.ts"; import * as AcpRegistryRuntimeCoordinator from "./AcpRegistryRuntimeCoordinator.ts"; import * as AcpSessionRuntime from "@t3tools/provider-acp/server/AcpSessionRuntime"; +const isAcpRequestError = Schema.is(EffectAcpErrors.AcpRequestError); + +const MAX_ERROR_MESSAGE_LENGTH = 500; + +export const formatFailureDetail = (detail: string, cause?: unknown): string => { + const unwrapped = Cause.isCause(cause) ? Cause.squash(cause) : cause; + if (isAcpRequestError(unwrapped)) { + let message = unwrapped.errorMessage.trim(); + if (message.length > MAX_ERROR_MESSAGE_LENGTH) { + message = `${message.slice(0, MAX_ERROR_MESSAGE_LENGTH).trimEnd()}…`; + } + if (message.length > 0) { + const trimmed = detail.trimEnd(); + const prefix = trimmed.endsWith(".") ? `${trimmed} ` : `${trimmed}. `; + return `${prefix}${message}`; + } + } + return detail; +}; + type Runtime = Pick< AcpSessionRuntime.AcpSessionRuntime["Service"], "initialize" | "authenticate" | "start" | "handleElicitation" | "logout" @@ -47,7 +70,12 @@ export const makeAcpRegistryAuth = Effect.fn("makeAcpRegistryAuth")(function* (o const agentKey = options.settings.source === "local" ? `local:${options.instanceId}` : options.settings.agentId; const failure = (operation: string, detail: string, cause?: unknown) => - new ProviderSetupError({ instanceId: options.instanceId, operation, detail, cause }); + new ProviderSetupError({ + instanceId: options.instanceId, + operation, + detail: formatFailureDetail(detail, cause), + cause, + }); const resolve = catalog .resolve(options.settings, options.cwd, options.environment) .pipe( From 56a85428d5b46b81e1706e5644ad0a459edf3318 Mon Sep 17 00:00:00 2001 From: Andrei <108398860+Yuruzuu@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:24:14 +0800 Subject: [PATCH 013/227] =?UTF-8?q?perf(server):=20a=20slow=20host=20no=20?= =?UTF-8?q?longer=20delays=20other=20watched=20PRs'=20wakes=20(935=20ms=20?= =?UTF-8?q?=E2=86=92=2065=20ms=20median)=20(#30291)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Claude Opus 5.5 --- .../PullRequestWatchReactor.ts | 35 +++++---- .../src/orchestration-v2/runtimeLayer.test.ts | 77 +++++++++++++++++++ 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 2b588b1d2e23e..369bead0ec428 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -24,6 +24,7 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; +import * as Semaphore from "effect/Semaphore"; import type * as Scope from "effect/Scope"; import { @@ -43,6 +44,8 @@ import { /** Checks take minutes; faster sweeps waste the account's shared host quota. */ const SWEEP_MINUTES = 2; const READ_FAILURE_LIMIT = 8; +/** Group reads in flight at once, inside the host's own request limit. */ +const GROUP_READS = 4; /** Without fingerprints, reread quiet PRs on snapshot changes or this interval to catch edits. */ const QUIET_REREAD_MS = 10 * 60_000; /** Reread despite unchanged fingerprints; they miss review-thread comment edits. */ @@ -626,22 +629,26 @@ export const make = Effect.gen(function* () { ), { discard: true }, ); - const fingerprints = yield* Effect.forEach(groups, fingerprintOf, { - concurrency: "unbounded", - }); + // Every fingerprint is asked for at once so the host can batch them, and each group reads + // as soon as its own answer lands rather than waiting on the slowest host. + const reads = yield* Semaphore.make(GROUP_READS); yield* Effect.forEach( groups, - (group, index) => { - const fingerprint = fingerprints[index]!; - return fingerprint === "paused" - ? Effect.void - : readGroup(group, fingerprint).pipe( - Effect.catchCause( - logFailure("pull request watch check failed", { pullRequest: group.key }), - ), - ); - }, - { concurrency: 4, discard: true }, + (group) => + fingerprintOf(group).pipe( + Effect.flatMap((fingerprint) => + fingerprint === "paused" + ? Effect.void + : reads.withPermit( + readGroup(group, fingerprint).pipe( + Effect.catchCause( + logFailure("pull request watch check failed", { pullRequest: group.key }), + ), + ), + ), + ), + ), + { concurrency: "unbounded", discard: true }, ); }).pipe( Effect.catchCause(logFailure("pull request watch sweep failed", {})), diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index 83f77d0073001..af95d71e6121b 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -4334,6 +4334,83 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { }), ); + it.effect("reads each watched PR as soon as its own fingerprint lands", () => + Effect.gen(function* () { + const orchestrator = yield* Orchestrator.OrchestratorV2; + const projectId = ProjectId.make("pr-watch-pipeline-project"); + yield* seedProject({ + projectId, + title: "Watch pipeline", + workspaceRoot: "/workspace/watch-pipeline", + defaultModelSelection: null, + createdAt: "2026-10-01T00:00:00.000Z", + }); + for (const number of [21, 22]) { + const threadId = ThreadId.make(`runtime-pull-request-watch-pipeline-${number}`); + yield* orchestrator.dispatch({ + type: "thread.create", + createdBy: "user", + creationSource: "web", + commandId: CommandId.make(`pr-watch-pipeline-create-${number}`), + threadId, + projectId, + title: "Watch pipeline", + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + }); + yield* orchestrator.dispatch({ + type: "thread.pull-request.watch", + commandId: CommandId.make(`pr-watch-pipeline-start-${number}`), + threadId, + host: "github.com", + repository: "pingdotgg/t3code", + number, + watching: true, + link: { url: `https://github.com/pingdotgg/t3code/pull/${number}`, source: "agent" }, + }); + } + const at = "2026-10-02T12:00:00.000Z"; + const texts = (number: number) => + orchestrator + .getThreadRecords(ThreadId.make(`runtime-pull-request-watch-pipeline-${number}`), [ + "messages", + ]) + .pipe(Effect.map(({ messages }) => messages.map((message) => message.text).join("\n"))); + // #21's host answers only after #22 has been read, so waiting on every fingerprint first + // never finishes. + const read22 = yield* Deferred.make(); + const host = Layer.mergeAll( + NodeServices.layer, + NodeCrypto.layer, + Layer.mock(PullRequestService.PullRequestService)({ + watchFingerprint: (input) => + input.number === 21 + ? Deferred.await(read22).pipe(Effect.as(null)) + : Effect.succeed(null), + detail: (input) => + (input.number === 22 ? Deferred.succeed(read22, undefined) : Effect.void).pipe( + Effect.as(watchedPullRequestDetail({ projectId, number: input.number, at })), + ), + activity: () => + Effect.succeed({ + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: [], + }), + }), + ); + const reactor = yield* PullRequestWatchReactor.make.pipe(Effect.provide(host)); + yield* reactor.sweep; + assert.include(yield* texts(21), "#21"); + assert.include(yield* texts(22), "#22"); + }), + ); + it.effect("persists rejected command receipts across retries", () => Effect.gen(function* () { const orchestrator = yield* Orchestrator.OrchestratorV2; From 8dbf2621fa1b2a201086d6b9535b385d7c3990a8 Mon Sep 17 00:00:00 2001 From: Andrei <108398860+Yuruzuu@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:24:32 +0800 Subject: [PATCH 014/227] =?UTF-8?q?perf(server):=20PR=20cache=20invalidati?= =?UTF-8?q?on=20no=20longer=20waits=20on=20other=20projects'=20reads=20(1,?= =?UTF-8?q?163=20ms=20=E2=86=92=200.12=20ms=20median)=20(#30293)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Claude Opus 5.5 --- .../src/pullRequest/PullRequestReadCache.test.ts | 3 +++ .../src/pullRequest/PullRequestReadCache.ts | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/server/src/pullRequest/PullRequestReadCache.test.ts b/apps/server/src/pullRequest/PullRequestReadCache.test.ts index b73774700fcf8..071f1d4978a7d 100644 --- a/apps/server/src/pullRequest/PullRequestReadCache.test.ts +++ b/apps/server/src/pullRequest/PullRequestReadCache.test.ts @@ -44,6 +44,9 @@ it.layer(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))("PR filesystem ca const invalidate = yield* cache .invalidate("pr") .pipe(Effect.forkChild({ startImmediately: true })); + // Another PR's invalidation does not wait on this read; this one's does. + yield* cache.invalidate("other-pr"); + assert.isUndefined(invalidate.pollUnsafe()); yield* Deferred.succeed(release, undefined); yield* Fiber.join(read); yield* Fiber.join(invalidate); diff --git a/apps/server/src/pullRequest/PullRequestReadCache.ts b/apps/server/src/pullRequest/PullRequestReadCache.ts index f3ec665ca9786..7f03beef3a245 100644 --- a/apps/server/src/pullRequest/PullRequestReadCache.ts +++ b/apps/server/src/pullRequest/PullRequestReadCache.ts @@ -26,6 +26,8 @@ import { detachStackFrame } from "../git/detachStackFrame.ts"; import { forkParked } from "../serverActivation.ts"; const CONCURRENT_READS = 512; +/** Invalidation excludes only reads whose scopes share its stripe, not every PR read. */ +const SCOPE_STRIPES = 64; // Entry filenames are the URI-encoded `:`; the scope revisions file has // no store id. Effect before 4.1 wrote ``, still pruned here. const STORE_ID = "pr-v2"; @@ -87,6 +89,11 @@ export const make = Effect.gen(function* () { const clock = yield* Clock.Clock; let enabled = true; const lock = yield* Semaphore.make(CONCURRENT_READS); + const stripes = yield* Effect.forEach(Array.from({ length: SCOPE_STRIPES }), () => + Semaphore.make(CONCURRENT_READS), + ); + const stripeOf = (scope: string) => (Hash.string(scope) >>> 0) % SCOPE_STRIPES; + const revisionWrites = yield* Semaphore.make(1); const digest = (key: string) => digester.digest(Digest.sha256, new TextEncoder().encode(key)).pipe(Effect.map(Hex.encode)); const revisions = yield* Cache.makeWith( @@ -162,6 +169,12 @@ export const make = Effect.gen(function* () { PersistenceError: () => read, SchemaError: () => read, }), + // A read stores its result under the revisions it started with, so an invalidation of + // one of its scopes must wait for it; reads of other scopes run on. + (effect) => + [...new Set(scopes.map(stripeOf))] + .toSorted((left, right) => left - right) + .reduceRight((inner, stripe) => stripes[stripe]!.withPermits(1)(inner), effect), lock.withPermits(1), ); }, detachStackFrame), @@ -182,7 +195,8 @@ export const make = Effect.gen(function* () { return Effect.logWarning("PR cache disabled after clearing failed"); }), Effect.uninterruptible, - lock.withPermits(CONCURRENT_READS), + revisionWrites.withPermits(1), + (effect, scope) => stripes[stripeOf(scope)]!.withPermits(CONCURRENT_READS)(effect), detachStackFrame, ), }); From 43dfdcc84eb2f0bddeca9e4e1659c6c35e8fcf03 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Fri, 6 Nov 2026 04:24:50 +0000 Subject: [PATCH 015/227] fix(opencode): tools stopped mid-run show as interrupted, not failed (#29762) Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks --- .../src/server/adapter.test.ts | 66 +++++++++++++++++-- .../provider-opencode/src/server/adapter.ts | 16 ++--- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/packages/provider-opencode/src/server/adapter.test.ts b/packages/provider-opencode/src/server/adapter.test.ts index 6fb5263b6e3bc..70462070cff51 100644 --- a/packages/provider-opencode/src/server/adapter.test.ts +++ b/packages/provider-opencode/src/server/adapter.test.ts @@ -784,19 +784,35 @@ describe("OpenCodeAdapterV2", () => { error: "Command failed", time: { start: 3, end: 4 }, }, + { + status: "error", + input: { command: "sleep 60" }, + error: "Interrupted", + metadata: { interrupted: true }, + time: { start: 5, end: 6 }, + }, + // OpenCode 1.3 reports a stopped tool only through this message. + { + status: "error", + input: { command: "sleep 90" }, + error: "Tool execution aborted", + time: { start: 6, end: 7 }, + }, ] satisfies ReadonlyArray; for (const state of states) { + const callID = + state.status === "error" ? `call-ended-${state.time.start}` : "call-working"; yield* Effect.promise(() => nativeEvents.push({ type: "message.part.updated", properties: { sessionID: nativeSessionId, part: { - id: state.status === "error" ? "part-failed" : "part-working", + id: `part-${callID}`, sessionID: nativeSessionId, messageID: "late-assistant", type: "tool", - callID: state.status === "error" ? "call-failed" : "call-working", + callID, tool: "bash", state, }, @@ -804,6 +820,34 @@ describe("OpenCodeAdapterV2", () => { }), ); } + for (const error of ["Task cancelled", "Task failed"]) { + yield* Effect.promise(() => + nativeEvents.push({ + type: "message.part.updated", + properties: { + sessionID: nativeSessionId, + part: { + id: `part-${error}`, + sessionID: nativeSessionId, + messageID: "late-assistant", + type: "tool", + callID: `call-${error}`, + tool: "task", + state: { + status: "error", + input: { + description: "Sleep task", + prompt: "sleep 120", + subagent_type: "general", + }, + error, + time: { start: 7, end: 8 }, + }, + }, + }, + }), + ); + } yield* Effect.promise(() => nativeEvents.push({ type: "message.part.updated", @@ -902,21 +946,31 @@ describe("OpenCodeAdapterV2", () => { const commands = items.filter((item) => item.type === "command_execution"); assert.deepEqual( commands.map((item) => item.status), - ["pending", "running", "completed", "failed", "failed"], + ["pending", "running", "completed", "failed", "interrupted", "interrupted", "failed"], ); assert.deepEqual( commands.map((item) => item.input), - ["pwd", "pwd", "pwd", "pwd", "vp test"], + ["pwd", "pwd", "pwd", "pwd", "sleep 60", "sleep 90", "vp test"], ); assert.equal(commands[2]?.output, "/repo\n"); assert.equal(commands[3]?.output, "Command failed"); - assert.equal(commands[4]?.exitCode, 1); + assert.equal(commands[6]?.exitCode, 1); assert.equal( events .flatMap((event) => (event.type === "node.updated" ? [event.node] : [])) - .findLast((node) => node.id === commands[4]?.nodeId)?.status, + .findLast((node) => node.id === commands[6]?.nodeId)?.status, "failed", ); + assert.deepEqual( + items.filter((item) => item.type === "subagent").map((item) => item.status), + ["interrupted", "failed"], + ); + assert.deepEqual( + events.flatMap((event) => + event.type === "subagent.updated" ? [event.subagent.status] : [], + ), + ["interrupted", "failed"], + ); assert.isTrue( events.some( (event) => diff --git a/packages/provider-opencode/src/server/adapter.ts b/packages/provider-opencode/src/server/adapter.ts index 1daf5ec1f4f57..4c72c5f6ef09f 100644 --- a/packages/provider-opencode/src/server/adapter.ts +++ b/packages/provider-opencode/src/server/adapter.ts @@ -747,7 +747,12 @@ function toolStatus(part: ToolPart): { ? { node: "failed", item: "failed" } : { node: "completed", item: "completed" }; case "error": - return { node: "failed", item: "failed" }; + // Stop marks in-flight tools this way; OpenCode's own UI shows them as cancelled. + return part.state.metadata?.["interrupted"] === true || + part.state.error === "Tool execution aborted" || + (part.tool === "task" && part.state.error === "Task cancelled") + ? { node: "interrupted", item: "interrupted" } + : { node: "failed", item: "failed" }; } } @@ -1439,14 +1444,7 @@ export const makeOpenCodeAdapterV2 = Effect.fn("makeOpenCodeAdapterV2")(function if (part.state.status === "completed" && output !== undefined) context.result = output; const status = toolStatus(part); const completedAt = toolCompletedAt(part, now); - const subagentStatus: OrchestrationV2Subagent["status"] = - status.item === "failed" - ? "failed" - : status.item === "completed" - ? "completed" - : status.item === "pending" - ? "pending" - : "running"; + const subagentStatus: OrchestrationV2Subagent["status"] = status.item; const subagent: OrchestrationV2Subagent = { id: nodeId, threadId: turn.threadId, From 4821696509d268eac35a8c3fa04cb4211d68a504 Mon Sep 17 00:00:00 2001 From: MarcoDotIO Date: Thu, 5 Nov 2026 23:25:08 -0500 Subject: [PATCH 016/227] fix(client-runtime): pull request bodies with CRLF line endings keep their code blocks whole (#30352) Co-authored-by: Claude Opus 5.5 Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com> --- .../src/pullRequestMarkdown.test.ts | 21 +++++++++++++++++++ .../client-runtime/src/pullRequestMarkdown.ts | 3 ++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/client-runtime/src/pullRequestMarkdown.test.ts b/packages/client-runtime/src/pullRequestMarkdown.test.ts index 8fab1fdd3b632..a5508e8197421 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.test.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.test.ts @@ -80,6 +80,27 @@ describe("pull request body segmentation", () => { ]); }); + // GitHub keeps CRLF in a body written in its web editor. + it("recognizes fences in a body with CRLF line endings", () => { + const body = "```\r\nhttps://example.com/demo.mp4\r\n```"; + expect(splitPullRequestBody(body)).toEqual([ + { id: "markdown:0", kind: "markdown", text: body, sourceOffset: 0 }, + ]); + }); + + it("closes a CRLF fence, so a video after it still lifts out", () => { + const code = '```html\r\n\r\n```'; + expect(splitPullRequestBody(`${code}\r\n\r\nhttps://example.com/demo.mp4\r\n`)).toEqual([ + { id: "markdown:0", kind: "markdown", text: code, sourceOffset: 0 }, + { + id: "attachment:1", + kind: "attachment", + url: "https://example.com/demo.mp4", + media: "video", + }, + ]); + }); + it("does not let a different fence marker close an open fence", () => { const body = "```\n~~~\nhttps://example.com/demo.mp4\n```"; expect(splitPullRequestBody(body)).toEqual([ diff --git a/packages/client-runtime/src/pullRequestMarkdown.ts b/packages/client-runtime/src/pullRequestMarkdown.ts index 9524f3e660e04..92c7d6de70cf6 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.ts @@ -16,7 +16,8 @@ export type PullRequestBodySegment = readonly media: "video" | "image" | "unknown"; }; -const FENCE_PATTERN = /^\s{0,3}((?:`{3,})|(?:~{3,}))(.*)$/u; +// `.` stops at the `\r` of a CRLF body, which is how GitHub stores one written in its web editor. +const FENCE_PATTERN = /^\s{0,3}((?:`{3,})|(?:~{3,}))(.*)\r?$/u; /** * How far a `