fix(client): preserve background service startup failures

packages/client/src/effect/service.ts

@@ -58,6 +58,7 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti
   let announced = false
   let lastSpawn = 0
   let spawnDelay = timing.spawnDelay
+  let failure: Error | undefined
   const announce = (reason: "missing" | "version-mismatch", previousVersion?: string) =>
     Effect.sync(() => {
       if (announced) return
@@ -115,14 +116,15 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti
     } else if (lastSpawn === 0 && info !== undefined) lastSpawn = Date.now()
 
     const finished = [...contenders].filter(contenderFinished)
-    const failure = finished.map(contenderFailure).find((error): error is Error => error !== undefined)
+    failure ??= finished.map(contenderFailure).find((error): error is Error => error !== undefined)
     if (finished.some((item) => item.child.exitCode === 0)) {
       spawnDelay = Math.min(spawnDelay * 2, timing.maxSpawnDelay)
     }
     finished.forEach((item) => contenders.delete(item))
     if (failure !== undefined && contenders.size === 0) return yield* Effect.fail(failure)
-    // Keep one candidate plus one lock probe so a pre-lock stall cannot block recovery.
-    if (contenders.size < 2 && Date.now() - lastSpawn >= spawnDelay) {
+    // Keep one candidate plus one lock probe for pre-lock stalls. After a failure, let the
+    // survivors finish without recruiting replacements that could hide the error indefinitely.
+    if (failure === undefined && contenders.size < 2 && Date.now() - lastSpawn >= spawnDelay) {
       yield* announce("missing")
       contenders.add(yield* spawnContender)
       lastSpawn = Date.now()
@@ -138,7 +140,7 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti
     Effect.ensuring(Effect.sync(() => contenders.forEach((contender) => contender.release()))),
   )
   if (Option.isNone(found))
-    return yield* Effect.fail(new Error("Timed out waiting for the background service to start"))
+    return yield* Effect.fail(failure ?? new Error("Timed out waiting for the background service to start"))
   return found.value.endpoint
 })
 

packages/client/src/promise/service.ts

@@ -38,6 +38,7 @@ export async function ensure(options: EnsureOptions = {}): Promise<Endpoint> {
   let announced = false
   let lastSpawn = 0
   let spawnDelay = timing.spawnDelay
+  let failure: Error | undefined
 
   const announce = (reason: "missing" | "version-mismatch", previousVersion?: string) => {
     if (announced) return
@@ -56,7 +57,7 @@ export async function ensure(options: EnsureOptions = {}): Promise<Endpoint> {
 
   try {
     while (true) {
-      if (Date.now() >= deadline) throw new Error("Timed out waiting for the background service to start")
+      if (Date.now() >= deadline) throw failure ?? new Error("Timed out waiting for the background service to start")
       const registration = await registered(options.file, timing.requestTimeout)
       if (registration.timedOut && registration.info !== undefined) {
         timeouts = {
@@ -95,14 +96,15 @@ export async function ensure(options: EnsureOptions = {}): Promise<Endpoint> {
       } else {
         if (lastSpawn === 0 && registration.info !== undefined) lastSpawn = Date.now()
         const finished = [...contenders].filter(contenderFinished)
-        const failure = finished.map(contenderFailure).find((error) => error !== undefined)
+        failure ??= finished.map(contenderFailure).find((error) => error !== undefined)
         if (finished.some((item) => item.child.exitCode === 0)) {
           spawnDelay = Math.min(spawnDelay * 2, timing.maxSpawnDelay)
         }
         finished.forEach((item) => contenders.delete(item))
         if (failure !== undefined && contenders.size === 0) throw failure
-        // Keep one candidate plus one lock probe so a pre-lock stall cannot block recovery.
-        if (contenders.size < 2 && Date.now() - lastSpawn >= spawnDelay) {
+        // Keep one candidate plus one lock probe for pre-lock stalls. After a failure, let the
+        // survivors finish without recruiting replacements that could hide the error indefinitely.
+        if (failure === undefined && contenders.size < 2 && Date.now() - lastSpawn >= spawnDelay) {
           announce("missing")
           contenders.add(await spawnContender())
           lastSpawn = Date.now()

packages/client/test/fixture/service-fixture.ts

@@ -1,4 +1,4 @@
-import { mkdtemp, rm } from "node:fs/promises"
+import { mkdtemp, rename, rm } from "node:fs/promises"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
 import { waitForExit } from "./service-timing"
@@ -8,6 +8,16 @@ export async function serviceFixture() {
   const registration = join(directory, "service.json")
   const processes: Bun.Subprocess[] = []
   const pids = new Set<number>()
+  const starts = async () =>
+    (
+      await Bun.file(registration + ".starts")
+        .text()
+        .catch(() => "")
+    )
+      .trim()
+      .split("\n")
+      .filter(Boolean)
+      .map(Number)
   const command = (mode: string, ...args: string[]) => [
     process.execPath,
     join(import.meta.dir, "service.ts"),
@@ -20,6 +30,21 @@ export async function serviceFixture() {
     directory,
     registration,
     command,
+    starts,
+    async waitForStarts(count: number) {
+      for (let attempt = 0; attempt < 600; attempt++) {
+        const result = await starts()
+        if (result.length >= count) return result
+        await Bun.sleep(5)
+      }
+      throw new Error(`Timed out waiting for ${count} contenders`)
+    },
+    async release(pid: number, action: "fail" | "ready") {
+      const file = registration + `.release-${pid}`
+      await Bun.write(file + ".tmp", action)
+      await rename(file + ".tmp", file)
+      if (action === "fail") await waitForExit(pid)
+    },
     spawn(mode: string, ...args: string[]) {
       const subprocess = Bun.spawn(command(mode, ...args), { stdout: "ignore", stderr: "inherit" })
       processes.push(subprocess)
@@ -37,6 +62,8 @@ export async function serviceFixture() {
       throw new Error(`Timed out waiting for ${file}`)
     },
     async [Symbol.asyncDispose]() {
+      // Include detached contenders that never registered, even when an assertion fails.
+      for (const pid of await starts()) pids.add(pid)
       await Promise.all([
         ...processes.map(async (subprocess) => {
           subprocess.kill("SIGTERM")

packages/client/test/fixture/service.ts

@@ -17,6 +17,16 @@ if (mode === "environment") {
 }
 if (mode === "signal") process.kill(process.pid, process.platform === "win32" ? "SIGTERM" : "SIGKILL")
 
+if (mode === "controlled") {
+  await appendFile(registration + ".starts", process.pid + "\n")
+  const release = registration + `.release-${process.pid}`
+  while (!(await Bun.file(release).exists())) await Bun.sleep(5)
+  if ((await Bun.file(release).text()) === "fail") {
+    process.stderr.write("actionable startup failure: storage initialization denied\n")
+    process.exit(23)
+  }
+}
+
 if (mode === "delayed" || mode === "delayed-failed" || mode === "coordinated" || mode === "coordinated-failed-loser") {
   await appendFile(registration + ".starts", process.pid + "\n")
   const owner = await writeFile(registration + ".owner", String(process.pid), { flag: "wx" })
@@ -44,7 +54,7 @@ const handoff = {
 }
 const server = Bun.serve({
   port: 0,
-  async fetch(request) {
+  async fetch(request): Promise<Response> {
     const pathname = new URL(request.url).pathname
     if (pathname === "/api/experimental/persistent-pty/handoff" && mode === "handoff") {
       if (request.method !== "POST" || request.headers.get("authorization") !== "Basic " + btoa("opencode:private"))

packages/client/test/promise-service.test.ts

@@ -111,6 +111,60 @@ test("reports a failed registered service", async () => {
   )
 })
 
+test("reports overlapping contender failures without recruiting replacements", async () => {
+  await using fixture = await serviceFixture()
+  const started = Date.now()
+  const pending = ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") }).catch(
+    (error: unknown) => error,
+  )
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  // Let discovery observe the exit across two accelerated spawn windows before the survivor exits.
+  await Bun.sleep(450)
+  await fixture.release(second, "fail")
+  const error = await pending
+
+  expect(Date.now() - started).toBeLessThan(3_000)
+  expect(error).toBeInstanceOf(Error)
+  if (!(error instanceof Error)) throw error
+  expect(error.message).toContain("Server process exited with code 23")
+  expect(error.message).toContain("storage initialization denied")
+  expect(await fixture.starts()).toHaveLength(2)
+})
+
+test("retains a contender failure until the deadline while its survivor stalls", async () => {
+  await using fixture = await serviceFixture()
+  const started = Date.now()
+  const pending = ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") }).catch(
+    (error: unknown) => error,
+  )
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  const error = await pending
+
+  expect(Date.now() - started).toBeGreaterThanOrEqual(3_000)
+  expect(error).toBeInstanceOf(Error)
+  if (!(error instanceof Error)) throw error
+  expect(error.message).toContain("Server process exited with code 23")
+  expect(error.message).toContain("storage initialization denied")
+  expect(await fixture.starts()).toHaveLength(2)
+  expect(() => process.kill(second, 0)).not.toThrow()
+})
+
+test("accepts a surviving contender after a failure without recruiting replacements", async () => {
+  await using fixture = await serviceFixture()
+  const pending = ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") })
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  await Bun.sleep(450)
+  await fixture.release(second, "ready")
+  const endpoint = await pending
+
+  expect((await Service.discover({ file: fixture.registration, version: "test" }))?.url).toBe(endpoint.url)
+  expect((await Bun.file(fixture.registration).json()).pid).toBe(second)
+  expect(await fixture.starts()).toHaveLength(2)
+})
+
 test("reports a bounded contender stderr tail with native promises", async () => {
   await using fixture = await serviceFixture()
   const registration = fixture.registration

packages/client/test/service.test.ts

@@ -228,6 +228,60 @@ test("reports a contender that fails to start", async () => {
   ).rejects.toThrow("Server process exited with code 1")
 })
 
+test("reports overlapping contender failures without recruiting replacements", async () => {
+  await using fixture = await serviceFixture()
+  const started = Date.now()
+  const pending = run(
+    ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") }),
+  ).catch((error: unknown) => error)
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  // Let discovery observe the exit across two accelerated spawn windows before the survivor exits.
+  await Bun.sleep(450)
+  await fixture.release(second, "fail")
+  const error = await pending
+
+  expect(Date.now() - started).toBeLessThan(3_000)
+  expect(error).toBeInstanceOf(Error)
+  if (!(error instanceof Error)) throw error
+  expect(error.message).toContain("Server process exited with code 23")
+  expect(error.message).toContain("storage initialization denied")
+  expect(await fixture.starts()).toHaveLength(2)
+})
+
+test("retains a contender failure until the deadline while its survivor stalls", async () => {
+  await using fixture = await serviceFixture()
+  const started = Date.now()
+  const pending = run(
+    ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") }),
+  ).catch((error: unknown) => error)
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  const error = await pending
+
+  expect(Date.now() - started).toBeGreaterThanOrEqual(3_000)
+  expect(error).toBeInstanceOf(Error)
+  if (!(error instanceof Error)) throw error
+  expect(error.message).toContain("Server process exited with code 23")
+  expect(error.message).toContain("storage initialization denied")
+  expect(await fixture.starts()).toHaveLength(2)
+  expect(() => process.kill(second, 0)).not.toThrow()
+})
+
+test("accepts a surviving contender after a failure without recruiting replacements", async () => {
+  await using fixture = await serviceFixture()
+  const pending = run(ensure({ file: fixture.registration, version: "test", command: fixture.command("controlled") }))
+  const [first, second] = await fixture.waitForStarts(2)
+  await fixture.release(first, "fail")
+  await Bun.sleep(450)
+  await fixture.release(second, "ready")
+  const endpoint = await pending
+
+  expect((await run(Service.discover({ file: fixture.registration, version: "test" })))?.url).toBe(endpoint.url)
+  expect((await Bun.file(fixture.registration).json()).pid).toBe(second)
+  expect(await fixture.starts()).toHaveLength(2)
+})
+
 test("reports a bounded contender stderr tail", async () => {
   await using fixture = await serviceFixture()
   const registration = fixture.registration