fix(app): give worktree creation a setup-length request deadline (#47694)

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<string, unknown> }> = []
   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<Response>((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()

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<string, unknown>) => 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)