From b21935b2eb41e120eebd65328db54439acb9a9d5 Mon Sep 17 00:00:00 2001 From: Ramiz Wachtler Date: Tue, 29 Sep 2026 08:20:47 +0200 Subject: [PATCH] fix(coding-agent): validate client settings before opening runtime --- .../src/experimental/client-tui.ts | 97 ++++++++++--------- .../test/experimental-client-tui.test.ts | 39 +++++++- 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/packages/coding-agent/src/experimental/client-tui.ts b/packages/coding-agent/src/experimental/client-tui.ts index 0f0915dba..9cf750499 100644 --- a/packages/coding-agent/src/experimental/client-tui.ts +++ b/packages/coding-agent/src/experimental/client-tui.ts @@ -739,57 +739,62 @@ export async function runClientTui(command: ClientCommand, options: RunClientTui }); await resourceLoader.reload(); setRegisteredThemes(resourceLoader.getThemes().themes); + const mouseWheel = settingsManager.getMouseWheelSettings(); const runtime = await openClientRuntime(command, options); - const tui = createInteractiveTui({ - tuiMode: "fullscreen", - showHardwareCursor: settingsManager.getShowHardwareCursor(), - logDirectory: agentDir, - }); - tui.setClearOnShrink(settingsManager.getClearOnShrink()); - let component: ExperimentalClientTui | undefined; - let tuiStarted = false; - const themeController = new InteractiveThemeController(tui, { - getSettingsManager: () => settingsManager, - showError: (error) => component?.showError(error), - onChanged: () => component?.refreshTheme(), - }); try { - let finish!: () => void; - const finished = new Promise((resolve) => { - finish = () => { - themeController.disableAutoSync(); - if (tuiStarted) { - tui.stop(); - tuiStarted = false; - } - resolve(); - }; + const tui = createInteractiveTui({ + tuiMode: "fullscreen", + mouseWheel, + showHardwareCursor: settingsManager.getShowHardwareCursor(), + logDirectory: agentDir, }); - component = await ExperimentalClientTui.create({ - command, - ui: tui, - servers: runtime.servers.map((server) => ({ - serverId: server.route.serverId, - radius: server.route.transport === "radius", - server: server.server, - session: server.session, - })), - facetLoader: options.facetLoader, - requestRender: () => tui.requestRender(), - finish, + tui.setClearOnShrink(settingsManager.getClearOnShrink()); + let component: ExperimentalClientTui | undefined; + let tuiStarted = false; + const themeController = new InteractiveThemeController(tui, { + getSettingsManager: () => settingsManager, + showError: (error) => component?.showError(error), + onChanged: () => component?.refreshTheme(), }); - tui.addChild(component); - tui.setLayoutRoot(component.layoutRoot); - tui.setFocus(component); - tuiStarted = true; - tui.start(); - await themeController.applyFromSettings(); - await finished; + try { + let finish!: () => void; + const finished = new Promise((resolve) => { + finish = () => { + themeController.disableAutoSync(); + if (tuiStarted) { + tui.stop(); + tuiStarted = false; + } + resolve(); + }; + }); + component = await ExperimentalClientTui.create({ + command, + ui: tui, + servers: runtime.servers.map((server) => ({ + serverId: server.route.serverId, + radius: server.route.transport === "radius", + server: server.server, + session: server.session, + })), + facetLoader: options.facetLoader, + requestRender: () => tui.requestRender(), + finish, + }); + tui.addChild(component); + tui.setLayoutRoot(component.layoutRoot); + tui.setFocus(component); + tuiStarted = true; + tui.start(); + await themeController.applyFromSettings(); + await finished; + } finally { + themeController.dispose(); + stopThemeWatcher(); + if (tuiStarted) tui.stop(); + await component?.close(); + } } finally { - themeController.dispose(); - stopThemeWatcher(); - if (tuiStarted) tui.stop(); - await component?.close(); await runtime.dispose(); } } diff --git a/packages/coding-agent/test/experimental-client-tui.test.ts b/packages/coding-agent/test/experimental-client-tui.test.ts index 242fe9582..a81caa889 100644 --- a/packages/coding-agent/test/experimental-client-tui.test.ts +++ b/packages/coding-agent/test/experimental-client-tui.test.ts @@ -21,8 +21,11 @@ import { reduceLaneSnapshot, } from "@earendil-works/pi-agent-core"; import { ProcessTerminal, TuiMainScreen } from "@earendil-works/pi-tui"; -import { beforeAll, describe, expect, test, vi } from "vitest"; -import { type ClientTuiServer, ExperimentalClientTui } from "../src/experimental/client-tui.ts"; +import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from "vitest"; +import { DefaultResourceLoader } from "../src/core/resource-loader.ts"; +import { SettingsManager } from "../src/core/settings-manager.ts"; +import * as clientRuntime from "../src/experimental/client-runtime.ts"; +import { type ClientTuiServer, ExperimentalClientTui, runClientTui } from "../src/experimental/client-tui.ts"; import { createPresentationFacetData } from "../src/experimental/plugins/bundled.ts"; import { AgentController } from "../src/experimental/services/agent-controller.ts"; import { createAgentController } from "../src/experimental/services/agent-controller-provider.ts"; @@ -42,6 +45,7 @@ import { } from "../src/experimental/services/sessions.ts"; import { Transcript, type TranscriptState } from "../src/experimental/services/transcript.ts"; import { initTheme } from "../src/modes/interactive/theme/theme.ts"; +import * as tuiRenderer from "../src/modes/interactive/tui-renderer.ts"; const serverId = "00000000-0000-4000-8000-000000000001"; @@ -94,6 +98,37 @@ function laneSnapshot(): LaneSnapshot { }; } +// #9758: validation and startup failures must not leak client connections. +describe("runClientTui startup", () => { + beforeEach(() => { + vi.spyOn(DefaultResourceLoader.prototype, "reload").mockResolvedValue(undefined); + }); + afterEach(() => vi.restoreAllMocks()); + + test("rejects invalid mouse wheel settings before acquiring a runtime", async () => { + vi.spyOn(SettingsManager, "create").mockReturnValue( + SettingsManager.inMemory({ mouseWheel: { normalLines: Number.MAX_VALUE } }), + ); + const open = vi.spyOn(clientRuntime, "openClientRuntime").mockRejectedValue(new Error("unexpected acquisition")); + + await expect(runClientTui({ command: "client" })).rejects.toThrow("Invalid mouseWheel.normalLines setting"); + expect(open).not.toHaveBeenCalled(); + }); + + test("disposes the runtime when TUI construction throws", async () => { + vi.spyOn(SettingsManager, "create").mockReturnValue(SettingsManager.inMemory()); + const dispose = vi.fn(async () => {}); + vi.spyOn(clientRuntime, "openClientRuntime").mockResolvedValue({ servers: [], dispose }); + const error = new Error("TUI startup failed"); + vi.spyOn(tuiRenderer, "createInteractiveTui").mockImplementation(() => { + throw error; + }); + + await expect(runClientTui({ command: "client" })).rejects.toBe(error); + expect(dispose).toHaveBeenCalledOnce(); + }); +}); + describe("experimental client TUI", () => { beforeAll(() => initTheme("dark"));