Show agent terminals created in workspace subdirectories (#1245)
Terminals were bucketed and looked up by exact cwd, so a terminal an agent opened in a subdirectory of the workspace never appeared in the workspace panel and its live updates were dropped — the panel only ever queried and subscribed the exact root. getTerminals(root) now aggregates every bucket at or below the root, and the session controller delivers a subdirectory change to each subscribed root above it, keyed by that root and carrying the full aggregated list. Keying by the root with the complete set is what keeps the client's cache replacement from dropping the terminals that live at the root.
This commit is contained in:
@@ -96,6 +96,19 @@ it("creates separate terminals for different cwds", async () => {
|
||||
expect(tmpTerminals[0].id).not.toBe(homeTerminals[0].id);
|
||||
});
|
||||
|
||||
it("lists subdirectory terminals when querying the workspace root", async () => {
|
||||
manager = createTerminalManager();
|
||||
const rootCwd = mkdtempSync(join(tmpdir(), "terminal-manager-subdir-root-"));
|
||||
const subdirCwd = join(rootCwd, "apps", "mobile");
|
||||
mkdirSync(subdirCwd, { recursive: true });
|
||||
temporaryDirs.push(rootCwd);
|
||||
|
||||
const created = await manager.createTerminal({ cwd: subdirCwd, name: "Mobile" });
|
||||
|
||||
const rootTerminals = await manager.getTerminals(rootCwd);
|
||||
expect(rootTerminals.map((terminal) => terminal.id)).toEqual([created.id]);
|
||||
});
|
||||
|
||||
it("creates additional terminal with auto-incrementing name", async () => {
|
||||
manager = createTerminalManager();
|
||||
const cwd = realpathSync(tmpdir());
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
} from "./terminal.js";
|
||||
import { captureTerminalLines, type CaptureTerminalLinesResult } from "./terminal-capture.js";
|
||||
import { resolve, sep, win32, posix } from "node:path";
|
||||
import { isSameOrDescendantPath } from "../server/path-utils.js";
|
||||
|
||||
export interface TerminalListItem {
|
||||
id: string;
|
||||
@@ -169,7 +170,16 @@ export function createTerminalManager(): TerminalManager {
|
||||
async getTerminals(cwd: string): Promise<TerminalSession[]> {
|
||||
assertAbsolutePath(cwd);
|
||||
|
||||
return terminalsByCwd.get(cwd) ?? [];
|
||||
// Terminals are bucketed by exact cwd, but an agent can open a terminal in
|
||||
// a subdirectory of the workspace. A query for the workspace root must
|
||||
// surface those too, so aggregate every bucket at or below `cwd`.
|
||||
const sessions: TerminalSession[] = [];
|
||||
for (const [bucketCwd, bucketSessions] of terminalsByCwd) {
|
||||
if (isSameOrDescendantPath(cwd, bucketCwd)) {
|
||||
sessions.push(...bucketSessions);
|
||||
}
|
||||
}
|
||||
return sessions;
|
||||
},
|
||||
|
||||
async createTerminal(options: {
|
||||
|
||||
@@ -10,7 +10,8 @@ import {
|
||||
import type { TerminalCell, TerminalState } from "@getpaseo/protocol/messages";
|
||||
import type { ServerMessage, TerminalSession, TerminalStateSnapshot } from "./terminal.js";
|
||||
import { TerminalSessionController } from "./terminal-session-controller.js";
|
||||
import type { TerminalManager } from "./terminal-manager.js";
|
||||
import type { TerminalManager, TerminalsChangedEvent } from "./terminal-manager.js";
|
||||
import { isSameOrDescendantPath } from "../server/path-utils.js";
|
||||
|
||||
function deferred<T>(): {
|
||||
promise: Promise<T>;
|
||||
@@ -140,3 +141,97 @@ describe("terminal-session-controller restore", () => {
|
||||
expect(new TextDecoder().decode(binaryFrames[1]?.payload)).toBe("restore-after\n");
|
||||
});
|
||||
});
|
||||
|
||||
function listSession(input: { id: string; name: string; cwd: string }): TerminalSession {
|
||||
return {
|
||||
id: input.id,
|
||||
name: input.name,
|
||||
cwd: input.cwd,
|
||||
send: vi.fn(),
|
||||
subscribe: () => vi.fn(),
|
||||
onExit: () => vi.fn(),
|
||||
onCommandFinished: () => vi.fn(),
|
||||
onTitleChange: () => vi.fn(),
|
||||
getSize: () => ({ rows: 1, cols: 80 }),
|
||||
getState: () => terminalState(""),
|
||||
getStateSnapshot: () => ({ state: terminalState(""), revision: 0 }),
|
||||
getReplayPreamble: () => "",
|
||||
getTitle: () => undefined,
|
||||
setTitle: vi.fn(),
|
||||
getExitInfo: () => null,
|
||||
kill: vi.fn(),
|
||||
killAndWait: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
async function flushMicrotasks(): Promise<void> {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
}
|
||||
|
||||
describe("terminal-session-controller subdirectory aggregation", () => {
|
||||
test("delivers a subdirectory change to a root subscriber as an aggregated, root-keyed snapshot", async () => {
|
||||
const rootCwd = "/work/repo";
|
||||
const subdirCwd = "/work/repo/apps/mobile";
|
||||
// Aggregating subdirectory buckets into the root query is the manager's
|
||||
// contract, covered by terminal-manager.test.ts. Here we only assert the
|
||||
// controller re-fetches by root and keys the snapshot by root, so the fake
|
||||
// returns a fixed aggregated list for the root and nothing otherwise.
|
||||
const aggregatedRootTerminals = [
|
||||
listSession({ id: "root-term", name: "Terminal 1", cwd: rootCwd }),
|
||||
listSession({ id: "subdir-term", name: "Mobile", cwd: subdirCwd }),
|
||||
];
|
||||
|
||||
let changedListener: ((event: TerminalsChangedEvent) => void) | null = null;
|
||||
const terminalManager: TerminalManager = {
|
||||
getTerminals: vi.fn(async (cwd: string) => (cwd === rootCwd ? aggregatedRootTerminals : [])),
|
||||
createTerminal: vi.fn(),
|
||||
registerCwdEnv: vi.fn(),
|
||||
getTerminal: vi.fn(),
|
||||
getTerminalState: vi.fn(),
|
||||
setTerminalTitle: vi.fn(),
|
||||
killTerminal: vi.fn(),
|
||||
killTerminalAndWait: vi.fn(),
|
||||
captureTerminal: vi.fn(),
|
||||
listDirectories: vi.fn(() => [rootCwd, subdirCwd]),
|
||||
killAll: vi.fn(),
|
||||
subscribeTerminalsChanged: vi.fn((listener) => {
|
||||
changedListener = listener;
|
||||
return vi.fn();
|
||||
}),
|
||||
};
|
||||
|
||||
const outboundMessages: SessionOutboundMessage[] = [];
|
||||
const controller = new TerminalSessionController({
|
||||
terminalManager,
|
||||
emit: (message) => outboundMessages.push(message),
|
||||
emitBinary: vi.fn(),
|
||||
hasBinaryChannel: () => true,
|
||||
isPathWithinRoot: isSameOrDescendantPath,
|
||||
sessionLogger: createLogger(),
|
||||
});
|
||||
controller.start();
|
||||
|
||||
controller.dispatch({ type: "subscribe_terminals_request", cwd: rootCwd });
|
||||
await flushMicrotasks();
|
||||
outboundMessages.length = 0;
|
||||
|
||||
changedListener?.({
|
||||
cwd: subdirCwd,
|
||||
terminals: [{ id: "subdir-term", name: "Mobile", cwd: subdirCwd }],
|
||||
});
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(outboundMessages).toEqual([
|
||||
{
|
||||
type: "terminals_changed",
|
||||
payload: {
|
||||
cwd: rootCwd,
|
||||
terminals: [
|
||||
{ id: "root-term", name: "Terminal 1" },
|
||||
{ id: "subdir-term", name: "Mobile" },
|
||||
],
|
||||
},
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -125,9 +125,9 @@ export class TerminalSessionController {
|
||||
if (!this.terminalManager) {
|
||||
return;
|
||||
}
|
||||
this.unsubscribeTerminalsChanged = this.terminalManager.subscribeTerminalsChanged((event) =>
|
||||
this.handleTerminalsChanged(event),
|
||||
);
|
||||
this.unsubscribeTerminalsChanged = this.terminalManager.subscribeTerminalsChanged((event) => {
|
||||
void this.handleTerminalsChanged(event);
|
||||
});
|
||||
}
|
||||
|
||||
getMetrics(): TerminalSessionControllerMetrics {
|
||||
@@ -293,31 +293,30 @@ export class TerminalSessionController {
|
||||
};
|
||||
}
|
||||
|
||||
private handleTerminalsChanged(event: TerminalsChangedEvent): void {
|
||||
if (!this.subscribedDirectories.has(event.cwd)) {
|
||||
return;
|
||||
private async handleTerminalsChanged(event: TerminalsChangedEvent): Promise<void> {
|
||||
// A terminal can live in a subdirectory of a subscribed workspace root (an
|
||||
// agent can open one there). Deliver the change to every subscribed root at
|
||||
// or above the terminal's cwd, keyed by that root, carrying the full
|
||||
// aggregated list — so the client's cache replacement doesn't drop the
|
||||
// terminals that live directly at the root.
|
||||
const matchingRoots = Array.from(this.subscribedDirectories).filter((root) =>
|
||||
this.isPathWithinRoot(root, event.cwd),
|
||||
);
|
||||
for (const root of matchingRoots) {
|
||||
await this.emitTerminalsSnapshotForRoot(root);
|
||||
}
|
||||
this.emitTerminalsChangedSnapshot({
|
||||
cwd: event.cwd,
|
||||
terminals: event.terminals.map((terminal) =>
|
||||
Object.assign(
|
||||
{ id: terminal.id, name: terminal.name },
|
||||
terminal.title ? { title: terminal.title } : {},
|
||||
),
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
private handleSubscribeTerminalsRequest(msg: SubscribeTerminalsRequest): void {
|
||||
this.subscribedDirectories.add(msg.cwd);
|
||||
void this.emitInitialTerminalsChangedSnapshot(msg.cwd);
|
||||
void this.emitTerminalsSnapshotForRoot(msg.cwd);
|
||||
}
|
||||
|
||||
private handleUnsubscribeTerminalsRequest(msg: UnsubscribeTerminalsRequest): void {
|
||||
this.subscribedDirectories.delete(msg.cwd);
|
||||
}
|
||||
|
||||
private async emitInitialTerminalsChangedSnapshot(cwd: string): Promise<void> {
|
||||
private async emitTerminalsSnapshotForRoot(cwd: string): Promise<void> {
|
||||
if (!this.terminalManager || !this.subscribedDirectories.has(cwd)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, expect, it } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { createWorkerTerminalManager } from "./worker-terminal-manager.js";
|
||||
@@ -332,6 +332,24 @@ it("starts the default shell through the worker and accepts quoted commands", as
|
||||
expect(readFileSync(markerPath, "utf8")).toBe("shell-ok");
|
||||
});
|
||||
|
||||
it("lists subdirectory terminals when querying the workspace root", async () => {
|
||||
const rootCwd = mkdtempSync(join(tmpdir(), "worker-terminal-manager-subdir-root-"));
|
||||
const subdirCwd = join(rootCwd, "apps", "mobile");
|
||||
mkdirSync(subdirCwd, { recursive: true });
|
||||
temporaryDirs.push(rootCwd);
|
||||
manager = createWorkerTerminalManager();
|
||||
const created = trackTerminal(
|
||||
await manager.createTerminal({
|
||||
cwd: subdirCwd,
|
||||
...nodeTerminalCommand("setInterval(() => {}, 1000);"),
|
||||
}),
|
||||
);
|
||||
|
||||
const rootTerminals = await manager.getTerminals(rootCwd);
|
||||
|
||||
expect(rootTerminals.map((terminal) => terminal.id)).toEqual([created.id]);
|
||||
});
|
||||
|
||||
it("removes worker terminals after killAndWait", async () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "worker-terminal-manager-kill-"));
|
||||
temporaryDirs.push(cwd);
|
||||
|
||||
Reference in New Issue
Block a user