Merge pull request #482 from fluxlabs/fix/tui-resource-leaks-and-quality
fix: TUI resource leaks, code quality, and regression tests
This commit is contained in:
commit
ee355b52d1
12 changed files with 181 additions and 18 deletions
|
|
@ -0,0 +1,45 @@
|
|||
// pi-tui CancellableLoader component regression tests
|
||||
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
|
||||
|
||||
import { describe, it, mock, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { CancellableLoader } from "../cancellable-loader.js";
|
||||
|
||||
function makeMockTUI() {
|
||||
return { requestRender: mock.fn() } as any;
|
||||
}
|
||||
|
||||
describe("CancellableLoader", () => {
|
||||
let loader: CancellableLoader;
|
||||
let tui: ReturnType<typeof makeMockTUI>;
|
||||
|
||||
beforeEach(() => {
|
||||
tui = makeMockTUI();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
loader?.dispose();
|
||||
});
|
||||
|
||||
it("dispose() aborts the AbortController signal", () => {
|
||||
loader = new CancellableLoader(tui, (s) => s, (s) => s, "test");
|
||||
assert.equal(loader.aborted, false);
|
||||
loader.dispose();
|
||||
assert.equal(loader.aborted, true);
|
||||
});
|
||||
|
||||
it("dispose() clears the onAbort callback", () => {
|
||||
loader = new CancellableLoader(tui, (s) => s, (s) => s, "test");
|
||||
loader.onAbort = () => {};
|
||||
loader.dispose();
|
||||
assert.equal(loader.onAbort, undefined);
|
||||
});
|
||||
|
||||
it("signal is aborted after dispose()", () => {
|
||||
loader = new CancellableLoader(tui, (s) => s, (s) => s, "test");
|
||||
const signal = loader.signal;
|
||||
assert.equal(signal.aborted, false);
|
||||
loader.dispose();
|
||||
assert.equal(signal.aborted, true);
|
||||
});
|
||||
});
|
||||
35
packages/pi-tui/src/components/__tests__/input.test.ts
Normal file
35
packages/pi-tui/src/components/__tests__/input.test.ts
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
// pi-tui Input component regression tests
|
||||
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
|
||||
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { Input } from "../input.js";
|
||||
|
||||
describe("Input", () => {
|
||||
it("paste buffer is cleared when focus is lost", () => {
|
||||
const input = new Input();
|
||||
input.focused = true;
|
||||
|
||||
// Simulate starting a paste (bracket paste start marker)
|
||||
input.handleInput("\x1b[200~partial");
|
||||
|
||||
// Now lose focus mid-paste
|
||||
input.focused = false;
|
||||
|
||||
// Regain focus — should not have stale paste state
|
||||
input.focused = true;
|
||||
|
||||
// Typing normal text should work without paste buffer corruption
|
||||
input.handleInput("hello");
|
||||
assert.equal(input.getValue(), "hello");
|
||||
});
|
||||
|
||||
it("focused getter/setter works correctly", () => {
|
||||
const input = new Input();
|
||||
assert.equal(input.focused, false);
|
||||
input.focused = true;
|
||||
assert.equal(input.focused, true);
|
||||
input.focused = false;
|
||||
assert.equal(input.focused, false);
|
||||
});
|
||||
});
|
||||
45
packages/pi-tui/src/components/__tests__/loader.test.ts
Normal file
45
packages/pi-tui/src/components/__tests__/loader.test.ts
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
// pi-tui Loader component regression tests
|
||||
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
|
||||
|
||||
import { describe, it, mock, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { Loader } from "../loader.js";
|
||||
|
||||
function makeMockTUI() {
|
||||
return { requestRender: mock.fn() } as any;
|
||||
}
|
||||
|
||||
describe("Loader", () => {
|
||||
let loader: Loader;
|
||||
let tui: ReturnType<typeof makeMockTUI>;
|
||||
|
||||
beforeEach(() => {
|
||||
tui = makeMockTUI();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
loader?.stop();
|
||||
});
|
||||
|
||||
it("start() is idempotent — calling twice does not leak intervals", () => {
|
||||
loader = new Loader(tui, (s) => s, (s) => s, "test");
|
||||
// Constructor calls start() once, call it again
|
||||
loader.start();
|
||||
// stop() should clear the interval cleanly without orphaned timers
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("dispose() stops the interval and nulls the TUI reference", () => {
|
||||
loader = new Loader(tui, (s) => s, (s) => s, "test");
|
||||
loader.dispose();
|
||||
// After dispose, calling stop() again should be safe (no-op)
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("stop() is safe to call multiple times", () => {
|
||||
loader = new Loader(tui, (s) => s, (s) => s, "test");
|
||||
loader.stop();
|
||||
loader.stop();
|
||||
loader.stop();
|
||||
});
|
||||
});
|
||||
|
|
@ -35,6 +35,8 @@ export class CancellableLoader extends Loader {
|
|||
}
|
||||
|
||||
dispose(): void {
|
||||
this.abortController.abort();
|
||||
this.onAbort = undefined;
|
||||
this.stop();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2064,6 +2064,10 @@ https://github.com/EsotericSoftware/spine-runtimes/actions/runs/19536643416/job/
|
|||
this.lastAutocompleteLookupPrefix = null;
|
||||
}
|
||||
|
||||
public dispose(): void {
|
||||
this.clearAutocompleteDebounce();
|
||||
}
|
||||
|
||||
public isShowingAutocomplete(): boolean {
|
||||
return this.autocompleteState !== null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,7 +23,17 @@ export class Input implements Component, Focusable {
|
|||
public placeholder: string = "";
|
||||
|
||||
/** Focusable interface - set by TUI when focus changes */
|
||||
focused: boolean = false;
|
||||
private _focused: boolean = false;
|
||||
get focused(): boolean {
|
||||
return this._focused;
|
||||
}
|
||||
set focused(value: boolean) {
|
||||
this._focused = value;
|
||||
if (!value) {
|
||||
this.isInPaste = false;
|
||||
this.pasteBuffer = "";
|
||||
}
|
||||
}
|
||||
|
||||
// Bracketed paste mode buffering
|
||||
private pasteBuffer: string = "";
|
||||
|
|
|
|||
|
|
@ -26,6 +26,9 @@ export class Loader extends Text {
|
|||
}
|
||||
|
||||
start() {
|
||||
if (this.intervalId) {
|
||||
clearInterval(this.intervalId);
|
||||
}
|
||||
this.updateDisplay();
|
||||
this.intervalId = setInterval(() => {
|
||||
this.currentFrame = (this.currentFrame + 1) % this.frames.length;
|
||||
|
|
@ -40,6 +43,11 @@ export class Loader extends Text {
|
|||
}
|
||||
}
|
||||
|
||||
dispose() {
|
||||
this.stop();
|
||||
this.ui = null;
|
||||
}
|
||||
|
||||
setMessage(message: string) {
|
||||
this.message = message;
|
||||
this.updateDisplay();
|
||||
|
|
|
|||
|
|
@ -441,6 +441,15 @@ export class TUI extends Container {
|
|||
|
||||
stop(): void {
|
||||
this.stopped = true;
|
||||
|
||||
// Dispose all overlays to stop any running timers
|
||||
for (const entry of this.overlayStack) {
|
||||
if ("dispose" in entry.component && typeof (entry.component as any).dispose === "function") {
|
||||
(entry.component as any).dispose();
|
||||
}
|
||||
}
|
||||
this.overlayStack = [];
|
||||
|
||||
// Move cursor to the end of the content to prevent overwriting/artifacts on exit
|
||||
if (this.previousLines.length > 0) {
|
||||
const targetRow = this.previousLines.length; // Line after the last content
|
||||
|
|
|
|||
19
src/cli.ts
19
src/cli.ts
|
|
@ -17,6 +17,7 @@ import { ensureManagedTools } from './tool-bootstrap.js'
|
|||
import { loadStoredEnvKeys } from './wizard.js'
|
||||
import { getPiDefaultModelAndProvider, migratePiCredentials } from './pi-migration.js'
|
||||
import { shouldRunOnboarding, runOnboarding } from './onboarding.js'
|
||||
import chalk from 'chalk'
|
||||
import { checkForUpdates } from './update-check.js'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -42,15 +43,10 @@ function exitIfManagedResourcesAreNewer(currentAgentDir: string): void {
|
|||
return
|
||||
}
|
||||
|
||||
const yellow = '\x1b[33m'
|
||||
const dim = '\x1b[2m'
|
||||
const reset = '\x1b[0m'
|
||||
const bold = '\x1b[1m'
|
||||
|
||||
process.stderr.write(
|
||||
`[gsd] ${yellow}Version mismatch detected${reset}\n` +
|
||||
`[gsd] Synced resources are from ${bold}v${managedVersion}${reset}, but this \`gsd\` binary is ${dim}v${currentVersion}${reset}.\n` +
|
||||
`[gsd] Run ${bold}npm install -g gsd-pi@latest${reset} or ${bold}gsd update${reset}, then try again.\n`,
|
||||
`[gsd] ${chalk.yellow('Version mismatch detected')}\n` +
|
||||
`[gsd] Synced resources are from ${chalk.bold(`v${managedVersion}`)}, but this \`gsd\` binary is ${chalk.dim(`v${currentVersion}`)}.\n` +
|
||||
`[gsd] Run ${chalk.bold('npm install -g gsd-pi@latest')} or ${chalk.bold('gsd update')}, then try again.\n`,
|
||||
)
|
||||
process.exit(1)
|
||||
}
|
||||
|
|
@ -153,6 +149,13 @@ if (!isPrintMode) {
|
|||
checkForUpdates().catch(() => {})
|
||||
}
|
||||
|
||||
// Warn if terminal is too narrow for readable output
|
||||
if (!isPrintMode && process.stdout.columns && process.stdout.columns < 40) {
|
||||
process.stderr.write(
|
||||
chalk.yellow(`[gsd] Terminal width is ${process.stdout.columns} columns (minimum recommended: 40). Output may be unreadable.\n`),
|
||||
)
|
||||
}
|
||||
|
||||
const modelRegistry = new ModelRegistry(authStorage)
|
||||
const settingsManager = SettingsManager.create(agentDir)
|
||||
|
||||
|
|
|
|||
|
|
@ -89,6 +89,7 @@ export class GSDDashboardOverlay {
|
|||
private loading = true;
|
||||
private loadedDashboardIdentity?: string;
|
||||
private refreshInFlight: Promise<void> | null = null;
|
||||
private disposed = false;
|
||||
|
||||
constructor(
|
||||
tui: { requestRender: () => void },
|
||||
|
|
@ -108,7 +109,7 @@ export class GSDDashboardOverlay {
|
|||
}
|
||||
|
||||
private scheduleRefresh(initial = false): void {
|
||||
if (this.refreshInFlight) return;
|
||||
if (this.refreshInFlight || this.disposed) return;
|
||||
this.refreshInFlight = this.refreshDashboard(initial)
|
||||
.finally(() => {
|
||||
this.refreshInFlight = null;
|
||||
|
|
@ -136,11 +137,13 @@ export class GSDDashboardOverlay {
|
|||
}
|
||||
|
||||
private async refreshDashboard(initial = false): Promise<void> {
|
||||
if (this.disposed) return;
|
||||
this.dashData = getAutoDashboardData();
|
||||
const nextIdentity = this.computeDashboardIdentity(this.dashData);
|
||||
|
||||
if (initial || nextIdentity !== this.loadedDashboardIdentity) {
|
||||
const loaded = await this.loadData();
|
||||
if (this.disposed) return;
|
||||
if (loaded) {
|
||||
this.loadedDashboardIdentity = nextIdentity;
|
||||
}
|
||||
|
|
@ -529,6 +532,7 @@ export class GSDDashboardOverlay {
|
|||
}
|
||||
|
||||
dispose(): void {
|
||||
this.disposed = true;
|
||||
clearInterval(this.refreshTimer);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -131,7 +131,9 @@ export function checkAutoStartAfterDiscuss(): boolean {
|
|||
try { unlinkSync(manifestPath); } catch { /* may not exist for single-milestone */ }
|
||||
|
||||
pendingAutoStart = null;
|
||||
startAuto(ctx, pi, basePath, false, { step }).catch(() => {});
|
||||
startAuto(ctx, pi, basePath, false, { step }).catch((err) => {
|
||||
if (process.env.GSD_DEBUG) console.error('[gsd] auto start error:', err);
|
||||
});
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs'
|
||||
import { dirname, join } from 'node:path'
|
||||
import chalk from 'chalk'
|
||||
import { appRoot } from './app-paths.js'
|
||||
|
||||
const CACHE_FILE = join(appRoot, '.update-check')
|
||||
|
|
@ -46,14 +47,9 @@ export function writeUpdateCache(cache: UpdateCheckCache, cachePath: string = CA
|
|||
}
|
||||
|
||||
function printUpdateBanner(current: string, latest: string): void {
|
||||
const yellow = '\x1b[33m'
|
||||
const dim = '\x1b[2m'
|
||||
const reset = '\x1b[0m'
|
||||
const bold = '\x1b[1m'
|
||||
|
||||
process.stderr.write(
|
||||
` ${yellow}Update available:${reset} ${dim}v${current}${reset} → ${bold}v${latest}${reset}\n` +
|
||||
` ${dim}Run${reset} npm update -g gsd-pi ${dim}or${reset} /gsd:update ${dim}to upgrade${reset}\n\n`,
|
||||
` ${chalk.yellow('Update available:')} ${chalk.dim(`v${current}`)} → ${chalk.bold(`v${latest}`)}\n` +
|
||||
` ${chalk.dim('Run')} npm update -g gsd-pi ${chalk.dim('or')} /gsd:update ${chalk.dim('to upgrade')}\n\n`,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue