From ecb3c886015c8468969130ef72c3a46a219ec605 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:44:45 +1000 Subject: [PATCH] fix(app): give worktree creation a setup-length request deadline (#47694) --- .../src/runtime/server/request-queue.test.ts | 42 ++++++++++++++++--- .../app/src/runtime/server/request-queue.ts | 13 +++++- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/packages/app/src/runtime/server/request-queue.test.ts b/packages/app/src/runtime/server/request-queue.test.ts index c3a2ba2af8d..984b1789989 100644 --- a/packages/app/src/runtime/server/request-queue.test.ts +++ b/packages/app/src/runtime/server/request-queue.test.ts @@ -1,7 +1,13 @@ import { describe, expect, test } from "bun:test" -import { createRequestQueue, isSlowRequest } from "./request-queue" +import { createRequestQueue, isSetupRequest, isSlowRequest } from "./request-queue" -function setup(input?: { limit?: number; slowLimit?: number; stallMs?: number; headersTimeoutMs?: number }) { +function setup(input?: { + limit?: number + slowLimit?: number + stallMs?: number + headersTimeoutMs?: number + setupHeadersTimeoutMs?: number +}) { const pending: Array<{ url: string; signal: AbortSignal; resolve: () => void }> = [] const logs: Array<{ message: string; data: Record }> = [] let clock = 0 @@ -10,12 +16,13 @@ function setup(input?: { limit?: number; slowLimit?: number; stallMs?: number; h slowLimit: input?.slowLimit, stallMs: input?.stallMs, headersTimeoutMs: input?.headersTimeoutMs, + setupHeadersTimeoutMs: input?.setupHeadersTimeoutMs, now: () => clock, log: (message, data) => logs.push({ message, data }), fetch: Object.assign( - (resource: RequestInfo | URL) => + (resource: RequestInfo | URL, init?: RequestInit) => new Promise((resolve, reject) => { - const request = new Request(resource) + const request = new Request(resource, init) request.signal.addEventListener("abort", () => reject(request.signal.reason), { once: true }) pending.push({ url: request.url, signal: request.signal, resolve: () => resolve(new Response("ok")) }) }), @@ -43,7 +50,12 @@ describe("createRequestQueue", () => { test("slow endpoints hold at most their share of slots so small reads go first", async () => { const input = setup({ limit: 4, slowLimit: 2 }) - const paths = ["/api/vcs?location[directory]=%2Fa", "/api/vcs/diff?location[directory]=%2Fa", "/api/worktree", "/api/session/ses_1"] + const paths = [ + "/api/vcs?location[directory]=%2Fa", + "/api/vcs/diff?location[directory]=%2Fa", + "/api/worktree", + "/api/session/ses_1", + ] const responses = paths.map((path) => input.queue.fetch(`http://server${path}`)) await input.settle() const started = () => input.pending.map((item) => new URL(item.url).pathname) @@ -109,6 +121,26 @@ describe("createRequestQueue", () => { expect(input.queue.inflight()).toBe(0) }) + test("worktree creation gets the setup deadline while worktree reads keep the normal one", async () => { + const input = setup({ limit: 4, headersTimeoutMs: 10, setupHeadersTimeoutMs: 200 }) + const create = input.queue.fetch("http://server/api/worktree?location[directory]=%2Fa", { method: "POST" }) + const list = input.queue.fetch("http://server/api/worktree?location[directory]=%2Fa") + const listError = await list.catch((cause: unknown) => cause) + expect((listError as DOMException).name).toBe("TimeoutError") + // Past the normal deadline, the create is still on the wire. + expect(input.pending[0]!.signal.aborted).toBe(false) + input.pending[0]!.resolve() + await expect(create).resolves.toBeInstanceOf(Response) + expect(input.queue.inflight()).toBe(0) + }) + + test("only worktree creation counts as a setup request", () => { + expect(isSetupRequest("POST", "/api/worktree")).toBe(true) + expect(isSetupRequest("GET", "/api/worktree")).toBe(false) + expect(isSetupRequest("POST", "/api/worktree/refresh")).toBe(false) + expect(isSetupRequest("DELETE", "/api/worktree")).toBe(false) + }) + test("caller aborts still reach the underlying request", async () => { const input = setup({ limit: 1 }) const controller = new AbortController() diff --git a/packages/app/src/runtime/server/request-queue.ts b/packages/app/src/runtime/server/request-queue.ts index c0719e8a57e..304c30ad610 100644 --- a/packages/app/src/runtime/server/request-queue.ts +++ b/packages/app/src/runtime/server/request-queue.ts @@ -19,16 +19,26 @@ export const requestStallMs = 2_000 // instead of wedging every later API call; the body may still stream for as long as it needs. export const requestHeadersTimeoutMs = 60_000 +// Creating a worktree runs the project's setup script (dependency installs, fetches) before the +// server answers, so it needs a budget measured in minutes rather than seconds. Cutting it off +// kills the script midway and leaves a registered but half-initialised worktree behind. +export const setupRequestHeadersTimeoutMs = 10 * 60_000 + export function isSlowRequest(pathname: string) { return slowRequestPaths.some((path) => pathname === path || pathname.startsWith(`${path}/`)) } +export function isSetupRequest(method: string, pathname: string) { + return method === "POST" && pathname === "/api/worktree" +} + export function createRequestQueue(input: { fetch: typeof globalThis.fetch limit?: number slowLimit?: number stallMs?: number headersTimeoutMs?: number + setupHeadersTimeoutMs?: number log?: (message: string, data: Record) => void now?: () => number }) { @@ -36,6 +46,7 @@ export function createRequestQueue(input: { const slowLimit = input.slowLimit ?? requestQueueSlowLimit const stallMs = input.stallMs ?? requestStallMs const headersTimeoutMs = input.headersTimeoutMs ?? requestHeadersTimeoutMs + const setupHeadersTimeoutMs = input.setupHeadersTimeoutMs ?? setupRequestHeadersTimeoutMs // Call the browser fetch unbound; `input.fetch(...)` would make `this` the options object. const base = input.fetch const now = input.now ?? Date.now @@ -101,7 +112,7 @@ export function createRequestQueue(input: { request.signal.addEventListener("abort", () => controller.abort(request.signal.reason), { once: true }) const timer = setTimeout( () => controller.abort(new DOMException("Timed out waiting for the server to respond", "TimeoutError")), - headersTimeoutMs, + isSetupRequest(request.method, pathname) ? setupHeadersTimeoutMs : headersTimeoutMs, ) return base(new Request(request, { signal: controller.signal })).finally(() => { clearTimeout(timer)