fix: bound lsp shutdown cleanup

- Add shutdown deadlines and unref LSP timers

- Share TUI test rendering helpers after simplification

Co-Authored-By: devagent <devagent@egavrin>

Author: egavrin

packages/cli/src/double-check-wiring.test.ts

@@ -240,7 +240,7 @@ describe("LSPRouter", () => {
     expect(router.getClientForFile("src/foo.ts")).toBeNull();
     expect(router.getClients()).toHaveLength(0);
     expect(router.getLanguages()).toHaveLength(0);
-    expect(mockClient.stop).toHaveBeenCalled();
+    expect(mockClient.stop).toHaveBeenCalledWith({ deadlineMs: 2000 });
   });
 });
 

packages/cli/src/double-check-wiring.ts

@@ -165,6 +165,7 @@ export class LSPRouter {
   /** Map from file extension to language ID. */
   private extensionMap = new Map<string, string>();
   private readonly rootPath: string;
+  private readonly shutdownDeadlineMs = 2_000;
 
   constructor(rootPath: string) {
     this.rootPath = rootPath;
@@ -223,13 +224,13 @@ export class LSPRouter {
   /** Stop all LSP servers. */
   async stopAll(): Promise<void> {
     const uniqueClients = new Set(this.clients.values());
-    for (const client of uniqueClients) {
+    await Promise.all([...uniqueClients].map(async (client) => {
       try {
-        await client.stop();
+        await client.stop({ deadlineMs: this.shutdownDeadlineMs });
       } catch {
         /* already dead */
       }
-    }
+    }));
     this.clients.clear();
     this.extensionMap.clear();
   }

packages/cli/src/main.ts

@@ -429,14 +429,14 @@ function toInteractiveQueryResult(result: import("@devagent/runtime").TaskLoopRe
 async function finalizeAgentSession(ctx: AgentSessionContext): Promise<void> {
   writeLspUsageSummary(ctx.tools, ctx.cliArgs);
   writeSessionSummary(ctx);
+  ctx.persistence.close();
   if (ctx.lsp.lspRouter) {
     try {
       await ctx.lsp.lspRouter.stopAll();
     } catch {
       // Servers might already be dead.
     }
   }
-  ctx.persistence.close();
 }
 
 function writeLspUsageSummary(tools: ToolsSetupResult, cliArgs: CliArgs): void {

packages/cli/src/tui/App.commands.test.ts

@@ -1,102 +1,19 @@
 import { EventBus } from "@devagent/runtime";
-import { render } from "ink";
-import { PassThrough, Writable } from "node:stream";
 import React from "react";
 import { afterEach, describe, expect, it } from "vitest";
 
 import { App, handleCancelShortcut } from "./App.js";
-
-class TestInput extends PassThrough {
-  readonly isTTY = true;
-
-  setRawMode(_value: boolean): void {}
-
-  ref(): this {
-    return this;
-  }
-
-  unref(): this {
-    return this;
-  }
-}
-
-class TestOutput extends Writable {
-  readonly isTTY = true;
-  readonly columns: number;
-  readonly rows = 40;
-  private readonly chunks: string[] = [];
-
-  constructor(columns: number = 120) {
-    super();
-    this.columns = columns;
-  }
-
-  override _write(
-    chunk: string | Uint8Array,
-    _encoding: BufferEncoding,
-    callback: (error?: Error | null) => void,
-  ): void {
-    this.chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"));
-    callback();
-  }
-
-  readAll(): string {
-    return this.chunks.join("");
-  }
-
-  clear(): void {
-    this.chunks.length = 0;
-  }
-}
-
-const instances: Array<{ unmount: () => void; cleanup: () => void }> = [];
-
-afterEach(() => {
-  while (instances.length > 0) {
-    const instance = instances.pop();
-    instance?.unmount();
-    instance?.cleanup();
-  }
-});
-
-async function settle(): Promise<void> {
-  await new Promise((resolve) => setTimeout(resolve, 20));
-}
-
-async function waitForRenders(cycles: number = 6): Promise<void> {
-  for (let index = 0; index < cycles; index++) {
-    await settle();
-  }
-}
-
-function stripAnsi(text: string): string {
-  return text.replace(/\x1b\[[0-9;]*m/g, "");
-}
-
-function countPromptPlaceholders(text: string): number {
-  return (stripAnsi(text).match(/Ask anything…/g) ?? []).length;
-}
-
-async function typeAndSubmit(stdin: TestInput, text: string): Promise<void> {
-  stdin.write(text);
-  await settle();
-  stdin.write("\r");
-}
-
-function renderForTest(node: React.ReactElement): { readonly stdout: TestOutput; readonly stdin: TestInput } {
-  const stdout = new TestOutput();
-  const stdin = new TestInput();
-  const stderr = new TestOutput();
-  instances.push(render(node, {
-    stdout: stdout as unknown as NodeJS.WriteStream,
-    stdin: stdin as unknown as NodeJS.ReadStream,
-    stderr: stderr as unknown as NodeJS.WriteStream,
-    debug: true,
-    exitOnCtrlC: false,
-    patchConsole: false,
-  }));
-  return { stdout, stdin };
-}
+import {
+  cleanupRenderedInstances,
+  countPromptPlaceholders,
+  renderForTest,
+  settle,
+  stripAnsi,
+  typeAndSubmit,
+  waitForRenders,
+} from "./App.test-utils.js";
+
+afterEach(cleanupRenderedInstances);
 
 function makeIdleAppProps(overrides?: { readonly bus?: EventBus; readonly onClear?: () => void }) {
   return {

packages/cli/src/tui/App.test-utils.ts

@@ -0,0 +1,118 @@
+import { render } from "ink";
+import { PassThrough, Writable } from "node:stream";
+
+import type React from "react";
+
+export class TestInput extends PassThrough {
+  readonly isTTY = true;
+
+  setRawMode(_value: boolean): void {}
+
+  ref(): this {
+    return this;
+  }
+
+  unref(): this {
+    return this;
+  }
+}
+
+export class TestOutput extends Writable {
+  readonly isTTY = true;
+  readonly columns: number;
+  readonly rows = 40;
+  private readonly chunks: string[] = [];
+
+  constructor(columns: number = 120) {
+    super();
+    this.columns = columns;
+  }
+
+  override _write(
+    chunk: string | Uint8Array,
+    _encoding: BufferEncoding,
+    callback: (error?: Error | null) => void,
+  ): void {
+    const text = typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8");
+    this.chunks.push(text);
+    callback();
+  }
+
+  readAll(): string {
+    return this.chunks.join("");
+  }
+
+  clear(): void {
+    this.chunks.length = 0;
+  }
+}
+
+const instances: Array<{ unmount: () => void; cleanup: () => void }> = [];
+
+export function cleanupRenderedInstances(): void {
+  while (instances.length > 0) {
+    const instance = instances.pop();
+    instance?.unmount();
+    instance?.cleanup();
+  }
+}
+
+export async function settle(): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, 20));
+}
+
+export async function waitForRenders(cycles: number = 6): Promise<void> {
+  for (let index = 0; index < cycles; index++) {
+    await settle();
+  }
+}
+
+export function stripAnsi(text: string): string {
+  return text.replace(/\x1b\[[0-9;]*m/g, "");
+}
+
+export function countPromptPlaceholders(text: string): number {
+  return (stripAnsi(text).match(/Ask anything…/g) ?? []).length;
+}
+
+export async function typeAndSubmit(stdin: TestInput, text: string): Promise<void> {
+  stdin.write(text);
+  await settle();
+  stdin.write("\r");
+}
+
+export async function insertModifiedReturn(stdin: TestInput): Promise<void> {
+  stdin.write("\x1b\r");
+  await settle();
+}
+
+export function renderForTest(
+  node: React.ReactElement,
+  options?: { readonly columns?: number },
+): {
+  readonly stdout: TestOutput;
+  readonly stdin: TestInput;
+  readonly rerender: (tree: React.ReactElement) => void;
+  readonly unmount: () => void;
+  readonly cleanup: () => void;
+} {
+  const stdout = new TestOutput(options?.columns);
+  const stdin = new TestInput();
+  const stderr = new TestOutput(options?.columns);
+  const instance = render(node, {
+    stdout: stdout as unknown as NodeJS.WriteStream,
+    stdin: stdin as unknown as NodeJS.ReadStream,
+    stderr: stderr as unknown as NodeJS.WriteStream,
+    debug: true,
+    exitOnCtrlC: false,
+    patchConsole: false,
+  });
+  instances.push(instance);
+  return {
+    stdout,
+    stdin,
+    rerender: instance.rerender,
+    unmount: instance.unmount,
+    cleanup: instance.cleanup,
+  };
+}