fix(cli): recover unresponsive service restarts

2026-07-21 18:26:09 +00:00 · 2026-07-14 18:00:03 -04:00
parent a149a61a89
commit 69d3f7d0d9
13 changed files with 378 additions and 50 deletions
@@ -0,0 +1,6 @@
+---
+"@opencode-ai/client": patch
+"@opencode-ai/cli": patch
+---
+
+Recover explicitly restarted background services that still own the service lock but no longer answer health checks.
@@ -42,6 +42,7 @@ framework.
 | Lifecycle shell           | Implemented; the owner binds and registers before application boot    |
 | Failed-state latching     | Implemented; deterministic boot failure stays bound and actionable    |
 | Recovery diagnostics      | Implemented; the TUI shows status instead of transport internals      |
+| Explicit recovery         | Implemented; restart can replace an unchanged unresponsive owner      |
 | Cross-platform validation | macOS runtime verified; Linux and Windows run in the unit-test matrix |

 ## Context
@@ -453,13 +454,14 @@ After a bounded diagnostic threshold, the client may show:

 ```text
 The background service owns the service lock but is not responding.
-Run `opencode service restart` to recover it.
+[r] Restart service
 ```

-Only explicit `service restart` may perform destructive recovery. It verifies
-the complete registration and process instance before signaling, waits for
-graceful exit, re-checks identity before escalation, and refuses to kill a
-process it cannot positively identify.
+Only explicit `service restart` or the TUI restart action may perform
+destructive recovery. It verifies that the complete registration is unchanged
+before signaling, waits for graceful exit, and re-checks registration before
+escalation. This deliberately accepts the narrow risk that a stale PID was
+reused by an unrelated process; automatic reconnect never takes that risk.

 Automatic frozen-owner recovery is deferred.

@@ -504,8 +506,11 @@ Automatic frozen-owner recovery is deferred.

 1. Health fails, but the process still holds the service lock.
 2. Contenders fail lock acquisition and exit.
-3. Clients wait and eventually show explicit recovery guidance.
-4. No TUI kills the owner automatically.
+3. Clients wait and the TUI offers explicit restart.
+4. The user presses `r`, or runs `opencode service restart`.
+5. Restart rechecks the complete registration before each signal.
+6. Once the process exits, normal election starts one replacement.
+7. No TUI kills the owner automatically.

 ## TDD Verification

@@ -609,6 +614,8 @@ was the observed incident cost.
 - Registration corruption cannot produce two owners.
 - A deleted registration heals without restarting the owner or any client.
 - An unresponsive owner is not killed without an explicit recovery command.
+- Explicit restart replaces an unchanged unresponsive owner and returns only
+  when the replacement is ready.
 - Raw transport defects never escape to the terminal.

 ## Follow-ups
@@ -617,7 +624,8 @@ was the observed incident cost.
 - Application protocol compatibility and automatic local TUI re-exec.
 - Durable execution recovery for provider attempts and tools.
 - Shell, sub-agent, permission, question, and background-job continuity.
- Automatic recovery for a positively identified frozen owner.
+- Automatic recovery for a positively identified frozen owner without the
+  explicit-restart PID-reuse tradeoff.
 - Cold-boot concurrency limits and interaction-prioritized location loading.
 - A steward or socket-handoff architecture if zero-downtime replacement becomes
  a real requirement.
@@ -9,8 +9,7 @@ export default Runtime.handler(
  Commands.commands.service.commands.restart,
  Effect.fn("cli.service.restart")(function* () {
    const options = yield* ServiceConfig.options()
-    yield* Service.stop(options, { targetVersion: options.version })
-    const transport = yield* Service.start(options)
+    const transport = yield* Service.restart(options)
    process.stdout.write(transport.url + EOL)
  }),
 )
@@ -17,7 +17,7 @@ export type Args = {
 export type Resolved = {
  readonly endpoint: Service.Endpoint
  readonly reconnect?: (onStatus: (status: Service.Status) => void, signal: AbortSignal) => Promise<Service.Endpoint>
-  readonly reload?: () => Promise<void>
+  readonly reload?: (signal?: AbortSignal) => Promise<void>
 }

 export const resolve = Effect.fn("cli.server.resolve")(function* (args: Args) {
@@ -53,13 +53,8 @@ export const resolve = Effect.fn("cli.server.resolve")(function* (args: Args) {
      Effect.runPromise(Service.start({ ...reconnectOptions, onStatus }).pipe(Effect.provide(NodeFileSystem.layer)), {
        signal,
      }),
-    reload: () =>
-      Effect.runPromise(
-        Effect.gen(function* () {
-          yield* Service.stop(options, { targetVersion: options.version })
-          yield* Service.start(options)
-        }).pipe(Effect.provide(NodeFileSystem.layer)),
-      ),
+    reload: (signal?: AbortSignal) =>
+      Effect.runPromise(Service.restart(options).pipe(Effect.asVoid, Effect.provide(NodeFileSystem.layer)), { signal }),
  } satisfies Resolved
 })

@@ -199,6 +199,22 @@ export const stop = Effect.fn("service.stop")(function* (options: Options = {},
  if (existing !== undefined) yield* kill(existing, options, metadata.targetVersion)
 })

+// Explicit recovery path: unlike stop(), restart may terminate an unchanged
+// registered process that no longer answers authenticated health checks.
+export const restart = Effect.fn("service.restart")(function* (options: StartOptions = {}) {
+  const existing = yield* registered(options.file, true)
+  if (existing.service !== undefined) {
+    const result = yield* kill(existing.service, options, options.version)
+    if (result === "rejected") return yield* Effect.fail(new Error("Background service rejected restart"))
+    if (result === "changed") {
+      const current = yield* read(options.file)
+      if (current !== undefined && same(current, existing.info))
+        yield* terminate(existing.info, read(options.file), true)
+    }
+  } else if (existing.info !== undefined) yield* terminate(existing.info, read(options.file), true)
+  return yield* start(options)
+})
+
 function fallback() {
  const state = process.env["XDG_STATE_HOME"] ?? join(homedir(), ".local", "state")
  return join(state, "opencode", "service.json")
@@ -217,6 +233,7 @@ export const Info = Schema.Struct({
  password: Schema.optional(Schema.String),
 })
 export type Info = typeof Info.Type
+const same = Schema.toEquivalence(Info)

 const decode = Schema.decodeUnknownEffect(Schema.fromJsonString(Info))
 const decodeHealth = Schema.decodeUnknownOption(ServiceStatus.Health)
@@ -305,27 +322,34 @@ const stopped = Effect.fnUntraced(function* (pid: number) {
  return yield* Effect.fail(new Error(`Server process ${pid} is still running`))
 })

-function same(left: Info, right: Info) {
-  return left.id === right.id && left.version === right.version && left.url === right.url && left.pid === right.pid
-}
+const terminate = Effect.fnUntraced(function* (
+  info: Info,
+  current: Effect.Effect<Info | undefined, never, FileSystem.FileSystem>,
+  graceful: boolean,
+) {
+  if (graceful) {
+    const owner = yield* current
+    if (owner === undefined || !same(owner, info)) return "changed" as const
+    yield* signal(info.pid, "SIGTERM")
+  }
+  const done = yield* stopped(info.pid).pipe(Effect.retry(poll), Effect.option)
+  if (Option.isSome(done)) return "stopped" as const
+
+  const latest = yield* current
+  if (latest === undefined || !same(latest, info)) return "changed" as const
+  yield* signal(info.pid, "SIGKILL")
+  yield* stopped(info.pid).pipe(Effect.retry(poll))
+  return "stopped" as const
+})

 const kill = Effect.fnUntraced(function* (service: LocalService, options: Options, targetVersion?: string) {
  const requested = yield* requestStop(service, targetVersion)
-  if (requested === "rejected") return
-  if (requested === "unsupported") {
-    // A stale registration may point at a reused PID. Authenticate again
-    // immediately before the legacy signal fallback.
-    const current = yield* find(options)
-    if (current === undefined || !same(current.info, service.info)) return
-    yield* signal(service.info.pid, "SIGTERM")
-  }
-  const done = yield* stopped(service.info.pid).pipe(Effect.retry(poll), Effect.option)
-  if (Option.isSome(done)) return
-
-  const latest = yield* find(options)
-  if (latest === undefined || !same(latest.info, service.info)) return
-  yield* signal(service.info.pid, "SIGKILL")
-  yield* stopped(service.info.pid).pipe(Effect.retry(poll))
+  if (requested === "rejected") return "rejected" as const
+  return yield* terminate(
+    service.info,
+    registered(options.file, true).pipe(Effect.map((result) => result.service?.info)),
+    requested === "unsupported",
+  )
 })

 const decodeStopResponse = Schema.decodeUnknownOption(ServiceStatus.StopResponse)
@@ -1,4 +1,4 @@
-import { appendFile, rename, writeFile } from "node:fs/promises"
+import { appendFile, rename, rm, writeFile } from "node:fs/promises"

 const [registration, mode, delay] = process.argv.slice(2)
 if (registration === undefined || mode === undefined) throw new Error("Missing service fixture arguments")
@@ -8,6 +8,35 @@ if (mode === "record-start") {
  process.exit(1)
 }
 if (mode === "signal") process.kill(process.pid, process.platform === "win32" ? "SIGTERM" : "SIGKILL")
+if (mode === "unresponsive" || mode === "unresponsive-stubborn" || mode === "unresponsive-slow") {
+  const stalled =
+    mode === "unresponsive-slow"
+      ? Bun.serve({
+          port: 0,
+          fetch: () => {
+            void writeFile(registration + ".health-request", "")
+            return new Promise<Response>(() => {})
+          },
+        })
+      : undefined
+  await writeFile(
+    registration,
+    JSON.stringify({
+      id: crypto.randomUUID(),
+      version: "test",
+      url: stalled?.url.toString() ?? "http://127.0.0.1:1",
+      pid: process.pid,
+      password: "secret",
+    }),
+    { mode: 0o600 },
+  )
+  process.on("SIGTERM", async () => {
+    if (mode === "unresponsive-stubborn") return
+    await rm(registration, { force: true })
+    process.exit()
+  })
+  await new Promise(() => {})
+}

 if (mode === "delayed" || mode === "delayed-failed" || mode === "coordinated") {
  await appendFile(registration + ".starts", process.pid + "\n")
@@ -22,6 +51,7 @@ if (mode === "delayed" || mode === "delayed-failed" || mode === "coordinated") {
 }

 let requests = 0
+let stopDropped = false
 const version = mode === "old" || mode === "reject-stop" ? "old" : "test"
 const id = crypto.randomUUID()
 const server = Bun.serve({
@@ -32,6 +62,10 @@ const server = Bun.serve({
      await writeFile(registration + ".stop-attempt", "")
      return Response.json({ accepted: false })
    }
+    if (pathname === "/api/service/stop" && mode === "drop-stop") {
+      stopDropped = true
+      return new Promise<Response>(() => {})
+    }
    if (pathname === "/api/service/stop" && mode === "graceful") {
      const body = await request.json()
      if (typeof body !== "object" || body === null || body.instanceID !== id) return Response.json({ accepted: false })
@@ -40,6 +74,7 @@ const server = Bun.serve({
      return Response.json({ accepted: true })
    }
    if (pathname !== "/api/health") return new Response(null, { status: 404 })
+    if (stopDropped) return new Promise<Response>(() => {})
    requests += 1
    if (mode === "modern" && requests === 1) {
      await writeFile(registration + ".first-request", "")
@@ -63,7 +98,7 @@ const server = Bun.serve({
        },
        { status: 503 },
      )
-    if (mode === "starting" || mode === "graceful" || mode === "reject-stop")
+    if (mode === "starting" || mode === "graceful" || mode === "reject-stop" || mode === "drop-stop")
      return Response.json({ healthy: true, version, pid: process.pid, instanceID: id, status: { type: "ready" } })
    return Response.json({ healthy: true, version, pid: process.pid })
  },
@@ -81,8 +116,9 @@ await writeFile(
 )
 await rename(registration + ".tmp", registration)

-function shutdown() {
+async function shutdown() {
  server.stop(true)
+  await rm(registration, { force: true })
  process.exit()
 }
 process.on("SIGTERM", shutdown)
@@ -11,8 +11,14 @@ const processes: Bun.Subprocess[] = []
 const directories: string[] = []

 afterEach(async () => {
-  processes.forEach((process) => process.kill("SIGTERM"))
-  await Promise.all(processes.splice(0).map((process) => process.exited))
+  await Promise.all(
+    processes.splice(0).map(async (process) => {
+      process.kill("SIGTERM")
+      const exited = await Promise.race([process.exited.then(() => true), Bun.sleep(1_000).then(() => false)])
+      if (!exited) process.kill("SIGKILL")
+      await process.exited
+    }),
+  )
  await Promise.all(directories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })))
 })

@@ -89,6 +95,124 @@ test("requests graceful replacement of the exact service instance", async () =>
  expect(await Bun.file(registration + ".stop").json()).toEqual({ instanceID: info.id, targetVersion: "next" })
 })

+test("explicit restart replaces an unresponsive registered process", async () => {
+  const directory = await temp()
+  const registration = join(directory, "service.json")
+  const existing = spawn(registration, "unresponsive")
+  await waitForFile(registration)
+
+  const endpoint = await run(
+    Service.restart({
+      file: registration,
+      version: "test",
+      command: [process.execPath, fixture, registration, "ready"],
+    }),
+  )
+  await existing.exited
+  const info = await Bun.file(registration).json()
+
+  try {
+    expect(endpoint.url).toBe(info.url)
+    expect(info.pid).not.toBe(existing.pid)
+    expect(await health(endpoint.url)).toMatchObject({ healthy: true, version: "test", pid: info.pid })
+  } finally {
+    await run(Service.stop({ file: registration }))
+  }
+}, 15_000)
+
+test("restart recovers when a healthy owner stops responding during shutdown", async () => {
+  const directory = await temp()
+  const registration = join(directory, "service.json")
+  const existing = spawn(registration, "drop-stop")
+  await waitForFile(registration)
+
+  const endpoint = await run(
+    Service.restart({
+      file: registration,
+      version: "test",
+      command: [process.execPath, fixture, registration, "ready"],
+    }),
+  )
+  await existing.exited
+  const info = await Bun.file(registration).json()
+
+  try {
+    expect(endpoint.url).toBe(info.url)
+    expect(info.pid).not.toBe(existing.pid)
+  } finally {
+    await run(Service.stop({ file: registration }))
+  }
+}, 15_000)
+
+test("restart fails when a responsive owner rejects shutdown", async () => {
+  const directory = await temp()
+  const registration = join(directory, "service.json")
+  const existing = spawn(registration, "reject-stop")
+  await waitForFile(registration)
+
+  await expect(run(Service.restart({ file: registration, version: "test" }))).rejects.toThrow(
+    "Background service rejected restart",
+  )
+  expect(existing.exitCode).toBe(null)
+})
+
+test("ordinary stop never signals an unresponsive registered process", async () => {
+  const directory = await temp()
+  const registration = join(directory, "service.json")
+  const existing = spawn(registration, "unresponsive")
+  await waitForFile(registration)
+
+  await run(Service.stop({ file: registration }))
+
+  expect(existing.exitCode).toBe(null)
+})
+
+test.skipIf(process.platform === "win32")(
+  "restart escalates when an unresponsive process ignores SIGTERM",
+  async () => {
+    const directory = await temp()
+    const registration = join(directory, "service.json")
+    const existing = spawn(registration, "unresponsive-stubborn")
+    await waitForFile(registration)
+
+    const endpoint = await run(
+      Service.restart({
+        file: registration,
+        version: "test",
+        command: [process.execPath, fixture, registration, "ready"],
+      }),
+    )
+    await existing.exited
+    const info = await Bun.file(registration).json()
+
+    try {
+      expect(endpoint.url).toBe(info.url)
+      expect(existing.signalCode).toBe("SIGKILL")
+    } finally {
+      await run(Service.stop({ file: registration }))
+    }
+  },
+  20_000,
+)
+
+test("restart does not signal a process after registration changes", async () => {
+  const directory = await temp()
+  const registration = join(directory, "service.json")
+  const existing = spawn(registration, "unresponsive-slow")
+  await waitForFile(registration)
+  const restarting = run(
+    Service.restart({ file: registration, version: "test", command: [] }),
+  )
+
+  await waitForFile(registration + ".health-request")
+  const replacement = spawn(registration, "ready")
+  await waitForRegistration(registration, replacement.pid)
+  const endpoint = await restarting
+
+  expect(existing.exitCode).toBe(null)
+  expect(endpoint.url).toBe((await Bun.file(registration).json()).url)
+})
+
 test("does not spawn contenders while an incompatible service rejects replacement", async () => {
  const directory = await temp()
  const registration = join(directory, "service.json")
@@ -241,6 +365,17 @@ async function waitForFile(file: string) {
  throw new Error(`Timed out waiting for ${file}`)
 }

+async function waitForRegistration(file: string, pid: number) {
+  for (let attempt = 0; attempt < 600; attempt++) {
+    const info = await Bun.file(file)
+      .json()
+      .catch(() => undefined)
+    if (info?.pid === pid) return
+    await Bun.sleep(5)
+  }
+  throw new Error(`Timed out waiting for registration from ${pid}`)
+}
+
 async function health(url: string) {
  return fetch(new URL("/api/health", url), { signal: AbortSignal.timeout(1_000) }).then((response) => response.json())
 }
@@ -115,6 +115,10 @@ Node application:
  a process.
 - `Service.start()` reuses a compatible service or starts one when needed.
 - `Service.stop()` stops the registered service.
+- `Service.restart()` explicitly replaces the registered service, including
+  signaling an unchanged registered PID that no longer answers health checks.
+  Reserve it for deliberate user recovery; automatic reconnect should use
+  `Service.start()` to avoid the narrow stale-PID reuse risk.
 - `Service.headers(endpoint)` creates the authentication headers for a client.

 ```sh
@@ -140,7 +140,7 @@ export type TuiInput = {
  server: {
    endpoint: Service.Endpoint
    reconnect?: (onStatus: (status: Service.Status) => void, signal: AbortSignal) => Promise<Service.Endpoint>
-    reload?: () => Promise<void>
+    reload?: (signal?: AbortSignal) => Promise<void>
  }
  args: Args
  config: Config.Interface
@@ -1121,7 +1121,7 @@ function App(props: { pair?: DialogPairCredentials }) {
        <StartupLoading ready={plugins.ready} />
      </Show>
      <Show when={showReconnecting()}>
-        <Reconnecting status={client.connection.service()} />
+        <Reconnecting status={client.connection.service()} restart={client.reload} />
      </Show>
    </box>
  )
@@ -1,11 +1,52 @@
 import type { Service } from "@opencode-ai/client/effect"
-import { Show } from "solid-js"
+import { createSignal, onCleanup, Show } from "solid-js"
+import { Keymap } from "../context/keymap"
 import { useTheme } from "../context/theme"
+import { errorMessage } from "../util/error"
 import { Spinner } from "./spinner"

-export function Reconnecting(props: { status?: Service.Status }) {
+const restartCommand = "service.restart"
+
+export function Reconnecting(props: { status?: Service.Status; restart?: (signal?: AbortSignal) => Promise<void> }) {
  const theme = useTheme().theme
-  const copy = () => reconnectingCopy(props.status)
+  const shortcuts = Keymap.useShortcuts()
+  const [restarting, setRestarting] = createSignal(false)
+  const [failure, setFailure] = createSignal<string>()
+  let controller: AbortController | undefined
+  const copy = () =>
+    restarting()
+      ? { loading: true, message: "Restarting background service..." }
+      : reconnectingCopy(props.status, shortcuts.get(restartCommand))
+
+  Keymap.createLayer(() => ({
+    mode: "global",
+    priority: 1000,
+    commands: [
+      {
+        id: restartCommand,
+        bind: "r",
+        title: "Restart service",
+        enabled: props.status?.type === "unresponsive" && !!props.restart,
+        run: () => {
+          if (!props.restart || restarting()) return
+          controller = new AbortController()
+          setFailure(undefined)
+          setRestarting(true)
+          void props
+            .restart(controller.signal)
+            .then(
+              () => setFailure(undefined),
+              (error) => {
+                setFailure(errorMessage(error))
+                setRestarting(false)
+              },
+            )
+        },
+      },
+    ],
+  }))
+
+  onCleanup(() => controller?.abort())

  return (
    <box
@@ -37,12 +78,19 @@ export function Reconnecting(props: { status?: Service.Status }) {
            )}
          </Show>
        </Show>
+        <Show when={failure()}>
+          {(message) => (
+            <text fg={theme.error} wrapMode="word">
+              {message()}
+            </text>
+          )}
+        </Show>
      </box>
    </box>
  )
 }

-export function reconnectingCopy(status?: Service.Status) {
+export function reconnectingCopy(status?: Service.Status, restart = "r") {
  if (status?.type === "starting")
    return {
      loading: true,
@@ -59,7 +107,7 @@ export function reconnectingCopy(status?: Service.Status) {
    return {
      loading: false,
      message: "Background service is not responding",
-      action: "Run `opencode service restart` to recover it.",
+      action: `[${restart}] Restart service`,
    }
  return { loading: true, message: "Waiting for background service..." }
 }
@@ -28,7 +28,7 @@ export const { use: useClient, provider: ClientProvider } = createSimpleContext(
    api: OpenCodeClient
    reconnect?: (onStatus: (status: Service.Status) => void, signal: AbortSignal) => Promise<{ api: OpenCodeClient }>
    // Stops and starts the managed service; present only in service mode.
-    reload?: () => Promise<void>
+    reload?: (signal?: AbortSignal) => Promise<void>
  }) => {
    const log = useLog({ component: "client" })
    const abort = new AbortController()
@@ -0,0 +1,73 @@
+/** @jsxImportSource @opentui/solid */
+import { testRender } from "@opentui/solid"
+import { expect, test } from "bun:test"
+import { ConfigProvider } from "../../../src/config"
+import { Reconnecting } from "../../../src/component/reconnecting"
+import { Keymap } from "../../../src/context/keymap"
+import { ThemeProvider } from "../../../src/context/theme"
+import { TestTuiContexts } from "../../fixture/tui-environment"
+import { createTuiResolvedConfig } from "../../fixture/tui-runtime"
+
+function render(restart: (signal?: AbortSignal) => Promise<void>) {
+  return testRender(() => (
+    <TestTuiContexts>
+      <ConfigProvider config={createTuiResolvedConfig()}>
+        <Keymap.Provider>
+          <ThemeProvider mode="dark" source={{ discover: () => Promise.resolve({}) }}>
+            <Reconnecting status={{ type: "unresponsive" }} restart={restart} />
+          </ThemeProvider>
+        </Keymap.Provider>
+      </ConfigProvider>
+    </TestTuiContexts>
+  ))
+}
+
+test("restarts an unresponsive service when r is pressed", async () => {
+  let restarts = 0
+  let complete!: () => void
+  const restarting = new Promise<void>((resolve) => {
+    complete = resolve
+  })
+  const app = await render(() => {
+    restarts += 1
+    return restarting
+  })
+  app.renderer.start()
+
+  try {
+    await app.waitForFrame((frame) => frame.includes("[r] Restart service"))
+    app.mockInput.pressKey("r")
+    await app.waitForFrame((frame) => frame.includes("Restarting background service..."))
+    app.mockInput.pressKey("r")
+    expect(restarts).toBe(1)
+    complete()
+    await Bun.sleep(0)
+    app.mockInput.pressKey("r")
+    expect(restarts).toBe(1)
+  } finally {
+    complete()
+    app.renderer.destroy()
+  }
+})
+
+test("cancels recovery when the reconnecting overlay unmounts", async () => {
+  let aborted = false
+  const app = await render(
+    (signal) =>
+      new Promise((_, reject) => {
+        signal?.addEventListener("abort", () => {
+          aborted = true
+          reject(signal.reason)
+        })
+      }),
+  )
+  app.renderer.start()
+
+  await app.waitForFrame((frame) => frame.includes("[r] Restart service"))
+  app.mockInput.pressKey("r")
+  await app.waitForFrame((frame) => frame.includes("Restarting background service..."))
+  app.renderer.destroy()
+  await Bun.sleep(0)
+
+  expect(aborted).toBe(true)
+})
@@ -25,7 +25,7 @@ test("describes service status without transport diagnostics", () => {
  expect(reconnectingCopy({ type: "unresponsive" })).toEqual({
    loading: false,
    message: "Background service is not responding",
-    action: "Run `opencode service restart` to recover it.",
+    action: "[r] Restart service",
  })
  expect(JSON.stringify(reconnectingCopy())).not.toMatch(/Attempt|ECONNREFUSED|Event stream disconnected/)
 })