mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 22:49:41 +02:00
Merge pull request #453 from timkjr/feat/split-pane-sessions
feat: split-pane sessions — view two live terminals side by side
This commit is contained in:
@@ -99,7 +99,10 @@ describe('App Settings modal structure', () => {
|
||||
for (const [, attrs, body] of previewed) {
|
||||
const kind = attrs.match(/data-preview="([a-z]+)"/)?.[1];
|
||||
expect(['header', 'panel', 'toolbar', 'float']).toContain(kind);
|
||||
expect(attrs, `chip ${body} needs a preview order`).toMatch(/data-preview-order="\d+"/);
|
||||
// A decimal (e.g. "11.5") is allowed — Split sits between Multi-monitor
|
||||
// (11) and Ultracode Agents (12) in the real header, and Number()
|
||||
// parses it fine for the preview's own sort.
|
||||
expect(attrs, `chip ${body} needs a preview order`).toMatch(/data-preview-order="\d+(\.\d+)?"/);
|
||||
// A text token replaces the icon for readouts (plan usage, CPU, font size).
|
||||
const hasIcon = body.includes('class="set-chip-ico') || attrs.includes('data-preview-text=');
|
||||
expect(hasIcon, `chip ${body} has nothing to render in the preview`).toBe(true);
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
// test/split-pane-auto-collapse-unit.test.ts
|
||||
// Port: N/A (no server/browser — loaded via `vm`, like session-close-fallback.test.ts).
|
||||
//
|
||||
// Fast, CI-visible unit coverage for the `_onSessionDeleted` prototype patch in
|
||||
// terminal-split.js (whole-branch review finding I6). The "Pane B ends" branch
|
||||
// already has real-Chromium coverage in test/split-pane-auto-collapse.browser.test.ts,
|
||||
// but that suite is excluded from `npm test` (see Testing in CLAUDE.md), and the
|
||||
// "Pane A ends, Pane B gets promoted" branch had NO coverage anywhere — it is the
|
||||
// one whose correctness depends on exact ordering: `_splitSessionId` must be
|
||||
// captured BEFORE `closeSplitPane()` runs (which nulls it), or the promoted
|
||||
// session id is lost. This file pins that ordering plus the sibling branches
|
||||
// (Pane B ends, unrelated session ends) so a regression fails in the normal CI
|
||||
// gate rather than only in the browser suite nobody runs by default.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
function loadCodemanAppClass() {
|
||||
const dir = resolve(import.meta.dirname, '../src/web/public');
|
||||
const terminalSplitSrc = readFileSync(resolve(dir, 'terminal-split.js'), 'utf8');
|
||||
const context = vm.createContext({
|
||||
console: { ...console, log: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
window: {},
|
||||
});
|
||||
// A minimal fake CodemanApp — terminal-split.js only needs `_onSessionDeleted`
|
||||
// and `selectSession` to already exist on the prototype (it wraps both), and
|
||||
// neither wrapped body executes at module-load time (only inside method
|
||||
// calls), so no xterm/WebSocket/CodemanSplitPane globals are needed here.
|
||||
const fakeAppSrc = `
|
||||
class CodemanApp {
|
||||
_onSessionDeleted(data) {
|
||||
(this.__originalDeletedCalls ??= []).push(data);
|
||||
}
|
||||
selectSession(id) {
|
||||
(this.__originalSelectSessionCalls ??= []).push(id);
|
||||
}
|
||||
}
|
||||
`;
|
||||
vm.runInContext(`${fakeAppSrc}\n${terminalSplitSrc}\nglobalThis.__CodemanApp = CodemanApp;`, context);
|
||||
return (context as { __CodemanApp: new () => unknown }).__CodemanApp as {
|
||||
prototype: {
|
||||
_onSessionDeleted: (this: unknown, data: { id: string }) => unknown;
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
const CodemanApp = loadCodemanAppClass();
|
||||
|
||||
type TestApp = {
|
||||
activeSessionId: string | null;
|
||||
_splitSessionId: string | null;
|
||||
_splitPane: { destroy: ReturnType<typeof vi.fn> } | null;
|
||||
_closingSessions: Set<string>;
|
||||
closeSplitPane: ReturnType<typeof vi.fn>;
|
||||
selectSession: ReturnType<typeof vi.fn>;
|
||||
__originalDeletedCalls?: Array<{ id: string }>;
|
||||
};
|
||||
|
||||
/** A split-active instance: Pane A === activeSessionId, Pane B === _splitSessionId. */
|
||||
function makeSplitActiveApp(): TestApp {
|
||||
const app = Object.create((CodemanApp as { prototype: object }).prototype) as TestApp;
|
||||
app.activeSessionId = 'session-a';
|
||||
app._splitSessionId = 'session-b';
|
||||
app._splitPane = { destroy: vi.fn() };
|
||||
// Empty by default: the app's OWN closeSession() is not mid-await for this
|
||||
// delete, so the promotion below is expected to fire. See the dedicated
|
||||
// test further down for the non-empty (_closingSessions owns it) case.
|
||||
app._closingSessions = new Set();
|
||||
// closeSplitPane is mocked but mirrors the REAL implementation's one
|
||||
// observable side effect relevant here: it nulls _splitPane/_splitSessionId.
|
||||
// If the wrapper captured _splitSessionId AFTER calling closeSplitPane
|
||||
// instead of before, this would surface as selectSession(undefined) below.
|
||||
app.closeSplitPane = vi.fn(() => {
|
||||
app._splitPane = null;
|
||||
app._splitSessionId = null;
|
||||
});
|
||||
app.selectSession = vi.fn();
|
||||
return app;
|
||||
}
|
||||
|
||||
describe('terminal-split.js _onSessionDeleted wrapper (I6)', () => {
|
||||
it('Pane A ends: closes the split and promotes Pane B via selectSession(ORIGINAL splitSessionId)', () => {
|
||||
const app = makeSplitActiveApp();
|
||||
|
||||
CodemanApp.prototype._onSessionDeleted.call(app, { id: 'session-a' });
|
||||
|
||||
expect(app.closeSplitPane).toHaveBeenCalledTimes(1);
|
||||
// Pinned ordering: selectSession must receive the id _splitSessionId held
|
||||
// BEFORE closeSplitPane ran (which nulls it), not whatever it holds after.
|
||||
// { auto: true } because this is an app-driven promotion, not the user
|
||||
// clicking a tab — it must not spend the promoted session's idle alert.
|
||||
expect(app.selectSession).toHaveBeenCalledWith('session-b', { auto: true });
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-a' }]);
|
||||
});
|
||||
|
||||
it('Pane B ends: closes the split without promoting anything', () => {
|
||||
const app = makeSplitActiveApp();
|
||||
|
||||
CodemanApp.prototype._onSessionDeleted.call(app, { id: 'session-b' });
|
||||
|
||||
expect(app.closeSplitPane).toHaveBeenCalledTimes(1);
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-b' }]);
|
||||
});
|
||||
|
||||
it('an unrelated session ending leaves the split untouched', () => {
|
||||
const app = makeSplitActiveApp();
|
||||
|
||||
CodemanApp.prototype._onSessionDeleted.call(app, { id: 'session-c' });
|
||||
|
||||
expect(app.closeSplitPane).not.toHaveBeenCalled();
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
expect(app._splitPane).not.toBeNull();
|
||||
expect(app._splitSessionId).toBe('session-b');
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-c' }]);
|
||||
});
|
||||
|
||||
it('the original _onSessionDeleted always fires, split-active or not', () => {
|
||||
const app = Object.create((CodemanApp as { prototype: object }).prototype) as TestApp;
|
||||
app.activeSessionId = 'session-a';
|
||||
app._splitSessionId = null;
|
||||
app._splitPane = null;
|
||||
app._closingSessions = new Set();
|
||||
app.closeSplitPane = vi.fn();
|
||||
app.selectSession = vi.fn();
|
||||
|
||||
CodemanApp.prototype._onSessionDeleted.call(app, { id: 'session-a' });
|
||||
|
||||
expect(app.closeSplitPane).not.toHaveBeenCalled();
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-a' }]);
|
||||
});
|
||||
|
||||
it('Pane A ends via the user closing its OWN tab: still collapses the split, but skips the promotion', () => {
|
||||
// closeSession() (app.js) adds the id to _closingSessions BEFORE awaiting
|
||||
// the delete, then owns the follow-up selection itself once it lands —
|
||||
// selecting Pane B's session here too would race it for which tab wins.
|
||||
const app = makeSplitActiveApp();
|
||||
app._closingSessions.add('session-a');
|
||||
|
||||
CodemanApp.prototype._onSessionDeleted.call(app, { id: 'session-a' });
|
||||
|
||||
expect(app.closeSplitPane).toHaveBeenCalledTimes(1);
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
expect(app.__originalDeletedCalls).toEqual([{ id: 'session-a' }]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,76 @@
|
||||
// test/split-pane-auto-collapse.browser.test.ts
|
||||
/** @fileoverview Real Chromium coverage for split auto-collapse when either session ends (Task 6). */
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { chromium, type Browser, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3177;
|
||||
const BASE_URL = `http://localhost:${PORT}`;
|
||||
|
||||
describe('split-pane auto-collapse in a real browser', () => {
|
||||
let server: WebServer;
|
||||
let browser: Browser;
|
||||
let page: Page;
|
||||
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT, false, true);
|
||||
await server.start();
|
||||
browser = await chromium.launch({ headless: true });
|
||||
page = await browser.newPage();
|
||||
await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
|
||||
await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 });
|
||||
}, 90000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (browser) await browser.close();
|
||||
if (server) await server.stop();
|
||||
}, 60000);
|
||||
|
||||
async function createShellSession(): Promise<string> {
|
||||
return page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', mode: 'shell' }),
|
||||
});
|
||||
// POST /api/sessions nests the session under data.session, and mode:'shell'
|
||||
// does not spawn a PTY on creation alone (pid: null, no pane) — an explicit
|
||||
// POST .../shell is what actually starts it (both found and fixed by Task 4's
|
||||
// implementer against this exact pattern; carried forward here so this task
|
||||
// does not rediscover the same two bugs).
|
||||
const id = (await res.json()).data.session.id;
|
||||
await fetch(`/api/sessions/${id}/shell`, { method: 'POST' });
|
||||
return id;
|
||||
});
|
||||
}
|
||||
|
||||
it('deleting the Pane B session auto-collapses the split', async () => {
|
||||
const idA = await createShellSession();
|
||||
const idB = await createShellSession();
|
||||
|
||||
await page.evaluate((id) => (window as any).app.selectSession(id), idA);
|
||||
await page.waitForFunction((id) => (window as any).app.activeSessionId === id, idA, { timeout: 10000 });
|
||||
await page.evaluate((id) => (window as any).app.openSplitPane(id), idB);
|
||||
await page.waitForSelector('.terminal-pane-b', { timeout: 10000 });
|
||||
|
||||
// Delete Pane B's session from "outside" (simulating the SSE event another
|
||||
// client's delete would produce, by hitting the DELETE route directly).
|
||||
await page.evaluate(async (id) => {
|
||||
await fetch(`/api/sessions/${id}`, { method: 'DELETE' });
|
||||
}, idB);
|
||||
await page.waitForFunction(() => document.querySelector('.terminal-split-container') === null, null, {
|
||||
timeout: 10000,
|
||||
});
|
||||
|
||||
const collapsed = await page.evaluate(() => ({
|
||||
hasContainer: document.querySelector('.terminal-split-container') === null,
|
||||
splitPaneNulled: (window as any).app._splitPane === null,
|
||||
}));
|
||||
expect(collapsed.hasContainer).toBe(true);
|
||||
expect(collapsed.splitPaneNulled).toBe(true);
|
||||
|
||||
await page.evaluate(async (id) => {
|
||||
await fetch(`/api/sessions/${id}`, { method: 'DELETE' });
|
||||
}, idA);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,88 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
function loadSplitPaneHelper() {
|
||||
const context = vm.createContext({ window: {}, globalThis: {} });
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
|
||||
vm.runInContext(source, context, { filename: 'constants.js' });
|
||||
return (context.window as { CodemanSplitPane: any }).CodemanSplitPane;
|
||||
}
|
||||
|
||||
describe('CodemanSplitPane.clampDividerPercent', () => {
|
||||
it('passes through a value inside the clamp range', () => {
|
||||
const { clampDividerPercent } = loadSplitPaneHelper();
|
||||
expect(clampDividerPercent(50)).toBe(50);
|
||||
expect(clampDividerPercent(35.5)).toBe(35.5);
|
||||
});
|
||||
|
||||
it('clamps below the floor to the floor', () => {
|
||||
const { clampDividerPercent } = loadSplitPaneHelper();
|
||||
expect(clampDividerPercent(5)).toBe(20);
|
||||
});
|
||||
|
||||
it('clamps above the ceiling to the ceiling', () => {
|
||||
const { clampDividerPercent } = loadSplitPaneHelper();
|
||||
expect(clampDividerPercent(95)).toBe(80);
|
||||
});
|
||||
|
||||
it('honors custom min/max', () => {
|
||||
const { clampDividerPercent } = loadSplitPaneHelper();
|
||||
expect(clampDividerPercent(10, 15, 85)).toBe(15);
|
||||
expect(clampDividerPercent(90, 15, 85)).toBe(85);
|
||||
});
|
||||
});
|
||||
|
||||
describe('CodemanSplitPane.buildSplitPickerSessions', () => {
|
||||
it('excludes the active session and preserves tab order', () => {
|
||||
const { buildSplitPickerSessions } = loadSplitPaneHelper();
|
||||
const sessions = new Map([
|
||||
['a', { name: 'w1-codeman' }],
|
||||
['b', { name: 'w1-mcp-memory' }],
|
||||
['c', { name: null }],
|
||||
]);
|
||||
const sessionOrder = ['a', 'b', 'c'];
|
||||
const result = buildSplitPickerSessions(sessions, sessionOrder, 'a');
|
||||
expect(result).toEqual([
|
||||
{ id: 'b', label: 'w1-mcp-memory' },
|
||||
{ id: 'c', label: 'Session' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('drops order entries with no matching session (stale ids)', () => {
|
||||
const { buildSplitPickerSessions } = loadSplitPaneHelper();
|
||||
const sessions = new Map([['a', { name: 'w1-codeman' }]]);
|
||||
const sessionOrder = ['a', 'ghost'];
|
||||
const result = buildSplitPickerSessions(sessions, sessionOrder, null);
|
||||
expect(result).toEqual([{ id: 'a', label: 'w1-codeman' }]);
|
||||
});
|
||||
|
||||
it('returns an empty list when only the excluded session exists', () => {
|
||||
const { buildSplitPickerSessions } = loadSplitPaneHelper();
|
||||
const sessions = new Map([['a', { name: 'w1-codeman' }]]);
|
||||
const result = buildSplitPickerSessions(sessions, ['a'], 'a');
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('excludes a session with no PTY attached (pid === null)', () => {
|
||||
const { buildSplitPickerSessions } = loadSplitPaneHelper();
|
||||
const sessions = new Map([
|
||||
['a', { name: 'w1-codeman' }],
|
||||
['b', { name: 'w2-exited', pid: null }],
|
||||
['c', { name: 'w3-alive', pid: 12345 }],
|
||||
]);
|
||||
const result = buildSplitPickerSessions(sessions, ['a', 'b', 'c'], 'a');
|
||||
expect(result).toEqual([{ id: 'c', label: 'w3-alive' }]);
|
||||
});
|
||||
|
||||
it('excludes a detached session even when it also has no PTY', () => {
|
||||
const { buildSplitPickerSessions } = loadSplitPaneHelper();
|
||||
const sessions = new Map([
|
||||
['a', { name: 'w1-codeman' }],
|
||||
['b', { name: 'w2-detached', pid: null }],
|
||||
]);
|
||||
const result = buildSplitPickerSessions(sessions, ['a', 'b'], 'a', new Set(['b']));
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,86 @@
|
||||
// test/split-pane-hidden-button-css.test.ts
|
||||
// Port: none (pure static analysis — runs in CI, no browser/server).
|
||||
//
|
||||
// Regression guard for the split-pane whole-branch review finding C1: the
|
||||
// header ships `.btn-split.btn-split--hidden` in index.html (an opt-in
|
||||
// header button, gated behind `showSplitButton`), but no CSS anywhere gave
|
||||
// `--hidden` markers meaning for that class, so the Split button rendered
|
||||
// VISIBLE to every user on every viewport regardless of the setting.
|
||||
//
|
||||
// Every OTHER opt-in header button follows a marker-class pattern: the base
|
||||
// rule is `display:inline-flex !important` and a more-specific
|
||||
// `.btn-x.btn-x--hidden { display: none !important; }` rule hides it
|
||||
// (`.btn-multimonitor--hidden` etc. in styles.css). C1 fixed the missing rule
|
||||
// for `.btn-split--hidden`; this test is the guard so the NEXT such class
|
||||
// fails loudly here instead of shipping invisible-until-noticed, the same
|
||||
// static-parse shape as test/mobile-header-buttons-policy.test.ts (read
|
||||
// first for the parsing conventions this file reuses).
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { join } from 'node:path';
|
||||
import postcss from 'postcss';
|
||||
|
||||
const HERE = fileURLToPath(new URL('.', import.meta.url));
|
||||
const PUBLIC = join(HERE, '../src/web/public');
|
||||
|
||||
/** Every distinct `*--hidden` class token referenced anywhere in index.html. */
|
||||
function loadHiddenMarkerClasses(): Set<string> {
|
||||
const html = readFileSync(join(PUBLIC, 'index.html'), 'utf-8');
|
||||
const classes = new Set<string>();
|
||||
for (const m of html.matchAll(/class="([^"]*)"/g)) {
|
||||
for (const token of m[1].split(/\s+/)) {
|
||||
if (token.endsWith('--hidden')) classes.add(token);
|
||||
}
|
||||
}
|
||||
return classes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every `*--hidden` class that has a CSS rule (anywhere — top-level or inside
|
||||
* any at-rule, e.g. a phone-only @media block) whose selector targets that
|
||||
* exact class and whose declarations set `display: none` (with or without
|
||||
* `!important`).
|
||||
*/
|
||||
function loadCssHiddenClasses(cssFile: string): Set<string> {
|
||||
const css = readFileSync(join(PUBLIC, cssFile), 'utf-8');
|
||||
const hidden = new Set<string>();
|
||||
postcss.parse(css).walkRules((rule) => {
|
||||
let hides = false;
|
||||
rule.walkDecls('display', (decl) => {
|
||||
if (decl.value.replace(/!important/i, '').trim() === 'none') hides = true;
|
||||
});
|
||||
if (!hides) return;
|
||||
for (const token of rule.selector.match(/\.[a-z0-9-]*--hidden\b/gi) || []) {
|
||||
hidden.add(token.slice(1));
|
||||
}
|
||||
});
|
||||
return hidden;
|
||||
}
|
||||
|
||||
describe('Every "*--hidden" marker class has a matching CSS hide rule (static guard)', () => {
|
||||
const markerClasses = loadHiddenMarkerClasses();
|
||||
const cssHidden = new Set([...loadCssHiddenClasses('styles.css'), ...loadCssHiddenClasses('mobile.css')]);
|
||||
|
||||
it('finds at least one *--hidden marker class in index.html (sanity)', () => {
|
||||
// If this drops to 0 the parser/markup drifted — fix the parser, don't delete the test.
|
||||
expect(markerClasses.size).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('every "*--hidden" class in index.html has a `display: none` rule in styles.css or mobile.css', () => {
|
||||
for (const cls of markerClasses) {
|
||||
expect(
|
||||
cssHidden.has(cls),
|
||||
`index.html references class "${cls}" (an opt-in-hide marker) but no rule in styles.css or ` +
|
||||
`mobile.css sets "display: none" for it — the element it marks ships VISIBLE regardless of the ` +
|
||||
`setting that is supposed to gate it. Add ".${cls} { display: none !important; }" (see the sibling ` +
|
||||
`.btn-multimonitor--hidden / .btn-redraw-terminal--hidden rules in styles.css for the pattern).`
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('locks the split-pane Split button specifically (C1 regression)', () => {
|
||||
expect(markerClasses.has('btn-split--hidden')).toBe(true);
|
||||
expect(cssHidden.has('btn-split--hidden')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,219 @@
|
||||
/** @fileoverview Real Chromium coverage for split open/close orchestration and the session picker (Task 5). */
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { chromium, type Browser, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3176;
|
||||
const BASE_URL = `http://localhost:${PORT}`;
|
||||
|
||||
describe('split-pane orchestration in a real browser', () => {
|
||||
let server: WebServer;
|
||||
let browser: Browser;
|
||||
let page: Page;
|
||||
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT, false, true);
|
||||
await server.start();
|
||||
browser = await chromium.launch({ headless: true });
|
||||
page = await browser.newPage();
|
||||
await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
|
||||
await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 });
|
||||
}, 90000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (browser) await browser.close();
|
||||
if (server) await server.stop();
|
||||
}, 60000);
|
||||
|
||||
async function createShellSession(): Promise<string> {
|
||||
return page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', mode: 'shell' }),
|
||||
});
|
||||
// POST /api/sessions nests the session under data.session, and mode:'shell'
|
||||
// does not spawn a PTY on creation alone (pid: null, no pane) — an explicit
|
||||
// POST .../shell is what actually starts it (both found and fixed by Task 4's
|
||||
// implementer against this exact pattern; carried forward here so this task
|
||||
// does not rediscover the same two bugs).
|
||||
const id = (await res.json()).data.session.id;
|
||||
await fetch(`/api/sessions/${id}/shell`, { method: 'POST' });
|
||||
return id;
|
||||
});
|
||||
}
|
||||
|
||||
it('opening and closing a split reparents and restores .terminal-wrap', async () => {
|
||||
const idA = await createShellSession();
|
||||
const idB = await createShellSession();
|
||||
|
||||
await page.evaluate((id) => (window as any).app.selectSession(id), idA);
|
||||
await page.waitForFunction((id) => (window as any).app.activeSessionId === id, idA, { timeout: 10000 });
|
||||
|
||||
expect(await page.evaluate(() => document.querySelector('.terminal-split-container') === null)).toBe(true);
|
||||
|
||||
await page.evaluate((id) => (window as any).app.openSplitPane(id), idB);
|
||||
await page.waitForSelector('.terminal-pane-b', { timeout: 10000 });
|
||||
|
||||
const duringSplit = await page.evaluate(() => ({
|
||||
hasContainer: document.querySelector('.terminal-split-container') !== null,
|
||||
wrapIsChildOfContainer: document.querySelector('.terminal-split-container > .terminal-wrap') !== null,
|
||||
hasPaneB: document.querySelector('.terminal-pane-b') !== null,
|
||||
}));
|
||||
expect(duringSplit.hasContainer).toBe(true);
|
||||
expect(duringSplit.wrapIsChildOfContainer).toBe(true);
|
||||
expect(duringSplit.hasPaneB).toBe(true);
|
||||
|
||||
await page.evaluate(() => (window as any).app.closeSplitPane());
|
||||
await page.waitForFunction(() => document.querySelector('.terminal-split-container') === null, null, {
|
||||
timeout: 10000,
|
||||
});
|
||||
|
||||
const afterClose = await page.evaluate(() => ({
|
||||
hasContainer: document.querySelector('.terminal-split-container') === null,
|
||||
wrapRestored: document.querySelector('.main .terminal-wrap') !== null,
|
||||
}));
|
||||
expect(afterClose.hasContainer).toBe(true);
|
||||
expect(afterClose.wrapRestored).toBe(true);
|
||||
|
||||
await page.evaluate(
|
||||
async (ids) => {
|
||||
await fetch(`/api/sessions/${ids.a}`, { method: 'DELETE' });
|
||||
await fetch(`/api/sessions/${ids.b}`, { method: 'DELETE' });
|
||||
},
|
||||
{ a: idA, b: idB }
|
||||
);
|
||||
});
|
||||
|
||||
it('the split picker excludes the active session', async () => {
|
||||
const id = await createShellSession();
|
||||
|
||||
await page.evaluate((sid) => (window as any).app.selectSession(sid), id);
|
||||
await page.waitForFunction((sid) => (window as any).app.activeSessionId === sid, id, { timeout: 10000 });
|
||||
|
||||
const pickerExcludesActive = await page.evaluate((sid) => {
|
||||
(window as any).app.openSplitPicker();
|
||||
const items = Array.from(document.querySelectorAll('.split-picker-item'));
|
||||
return !items.some((el) => el.getAttribute('data-session-id') === sid);
|
||||
}, id);
|
||||
expect(pickerExcludesActive).toBe(true);
|
||||
|
||||
await page.evaluate(async (sid) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, id);
|
||||
});
|
||||
|
||||
it('force-resizes Pane A immediately when a split opens', async () => {
|
||||
// Regression guard: opening a split moved Pane A from full width to 50%
|
||||
// in the DOM, but nothing told its session's PTY/tmux window about the
|
||||
// new size — only the passive, 300ms-debounced ResizeObserver in
|
||||
// terminal-ui.js eventually caught up, leaving stale-width content on
|
||||
// screen until the user manually hit "Redraw Terminal". openSplitPane()
|
||||
// now force-resizes Pane A synchronously as part of the same call.
|
||||
const idA = await createShellSession();
|
||||
const idB = await createShellSession();
|
||||
|
||||
await page.evaluate((id) => (window as any).app.selectSession(id), idA);
|
||||
await page.waitForFunction((id) => (window as any).app.activeSessionId === id, idA, { timeout: 10000 });
|
||||
|
||||
await page.evaluate(() => {
|
||||
const app = window as any as { app: any };
|
||||
(window as any).__resizeCalls = [];
|
||||
(window as any).__origSendResize = (window as any).app.sendResize;
|
||||
(window as any).app.sendResize = function (...args: any[]) {
|
||||
(window as any).__resizeCalls.push(args);
|
||||
return (window as any).__origSendResize.apply(app.app, args);
|
||||
};
|
||||
});
|
||||
|
||||
await page.evaluate((id) => (window as any).app.openSplitPane(id), idB);
|
||||
await page.waitForSelector('.terminal-pane-b', { timeout: 10000 });
|
||||
|
||||
const forcedResize = await page.evaluate(
|
||||
(id) =>
|
||||
((window as any).__resizeCalls as Array<[string, { force?: boolean }]>).some(
|
||||
([sessionId, opts]) => sessionId === id && opts?.force === true
|
||||
),
|
||||
idA
|
||||
);
|
||||
expect(forcedResize).toBe(true);
|
||||
|
||||
await page.evaluate(() => {
|
||||
(window as any).app.sendResize = (window as any).__origSendResize;
|
||||
});
|
||||
await page.evaluate(() => (window as any).app.closeSplitPane());
|
||||
await page.waitForFunction(() => document.querySelector('.terminal-split-container') === null, null, {
|
||||
timeout: 10000,
|
||||
});
|
||||
|
||||
await page.evaluate(
|
||||
async (ids) => {
|
||||
await fetch(`/api/sessions/${ids.a}`, { method: 'DELETE' });
|
||||
await fetch(`/api/sessions/${ids.b}`, { method: 'DELETE' });
|
||||
},
|
||||
{ a: idA, b: idB }
|
||||
);
|
||||
});
|
||||
|
||||
it('force-resizes Pane A once at the end of a divider drag', async () => {
|
||||
// Regression guard: the divider's onMove handler only called
|
||||
// fitAddon.fit() for Pane A — a LOCAL xterm reflow that never told Pane
|
||||
// A's own PTY/tmux window the new size, so existing content stayed laid
|
||||
// out for the pre-drag width. onUp now force-resizes Pane A once, at
|
||||
// drag end (not per-move, to avoid flooding the PTY with SIGWINCHes
|
||||
// during a fast drag).
|
||||
const idA = await createShellSession();
|
||||
const idB = await createShellSession();
|
||||
|
||||
await page.evaluate((id) => (window as any).app.selectSession(id), idA);
|
||||
await page.waitForFunction((id) => (window as any).app.activeSessionId === id, idA, { timeout: 10000 });
|
||||
await page.evaluate((id) => (window as any).app.openSplitPane(id), idB);
|
||||
await page.waitForSelector('.split-divider', { timeout: 10000 });
|
||||
|
||||
await page.evaluate(() => {
|
||||
const app = window as any as { app: any };
|
||||
(window as any).__resizeCalls = [];
|
||||
(window as any).__origSendResize = (window as any).app.sendResize;
|
||||
(window as any).app.sendResize = function (...args: any[]) {
|
||||
(window as any).__resizeCalls.push(args);
|
||||
return (window as any).__origSendResize.apply(app.app, args);
|
||||
};
|
||||
});
|
||||
|
||||
const divider = await page.$('.split-divider');
|
||||
const box = await divider!.boundingBox();
|
||||
if (!box) throw new Error('divider has no bounding box');
|
||||
const startX = box.x + box.width / 2;
|
||||
const startY = box.y + box.height / 2;
|
||||
|
||||
await page.mouse.move(startX, startY);
|
||||
await page.mouse.down();
|
||||
await page.mouse.move(startX + 80, startY, { steps: 5 });
|
||||
await page.mouse.up();
|
||||
|
||||
const forcedResize = await page.evaluate(
|
||||
(id) =>
|
||||
((window as any).__resizeCalls as Array<[string, { force?: boolean }]>).some(
|
||||
([sessionId, opts]) => sessionId === id && opts?.force === true
|
||||
),
|
||||
idA
|
||||
);
|
||||
expect(forcedResize).toBe(true);
|
||||
|
||||
await page.evaluate(() => {
|
||||
(window as any).app.sendResize = (window as any).__origSendResize;
|
||||
});
|
||||
await page.evaluate(() => (window as any).app.closeSplitPane());
|
||||
await page.waitForFunction(() => document.querySelector('.terminal-split-container') === null, null, {
|
||||
timeout: 10000,
|
||||
});
|
||||
|
||||
await page.evaluate(
|
||||
async (ids) => {
|
||||
await fetch(`/api/sessions/${ids.a}`, { method: 'DELETE' });
|
||||
await fetch(`/api/sessions/${ids.b}`, { method: 'DELETE' });
|
||||
},
|
||||
{ a: idA, b: idB }
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,52 @@
|
||||
// test/split-pane-per-device-setting.test.ts
|
||||
// Port: none (pure static analysis — runs in CI, no browser/server).
|
||||
//
|
||||
// Regression guard for the review finding that landed the blocker: moving
|
||||
// showSplitButton into settings-ui.js's per-device `displayKeys` set is only
|
||||
// HALF of making a setting per-device. The other half is stripping it out of
|
||||
// the object `saveAppSettings()` PUTs to `/api/settings` — displayKeys is a
|
||||
// client-side merge policy, not a wire filter. Without the strip, every save
|
||||
// sent `showSplitButton` in the body, `SettingsUpdateSchema` (.strict()) does
|
||||
// not declare it, the server answered 400 INVALID_INPUT, and because the
|
||||
// call site never checked `res.ok` the UI still reported "Settings saved"
|
||||
// while NOTHING persisted — workspaceHooksEnabled, agentSkillEnabled,
|
||||
// tunnelEnabled, claudeModel, every toggle, on every save, on every device.
|
||||
//
|
||||
// Mirrors test/terminal-auto-copy.test.ts's "keeps the toggle per-device"
|
||||
// guard for autoCopySelection — same three-way rule, same shape of test.
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { join } from 'node:path';
|
||||
|
||||
const HERE = fileURLToPath(new URL('.', import.meta.url));
|
||||
const PUBLIC = join(HERE, '../src/web/public');
|
||||
|
||||
function read(file: string): string {
|
||||
return readFileSync(join(PUBLIC, file), 'utf8');
|
||||
}
|
||||
|
||||
describe('showSplitButton stays per-device: display key, stripped from the PUT, absent from the schema', () => {
|
||||
const settingsUi = read('settings-ui.js');
|
||||
const schemas = readFileSync(join(HERE, '../src/web/schemas.ts'), 'utf8');
|
||||
|
||||
it('is in the client-side displayKeys merge policy', () => {
|
||||
const displayKeys = settingsUi.slice(
|
||||
settingsUi.indexOf('const displayKeys = new Set(['),
|
||||
settingsUi.indexOf('])', settingsUi.indexOf('const displayKeys = new Set(['))
|
||||
);
|
||||
expect(displayKeys).toContain("'showSplitButton'");
|
||||
});
|
||||
|
||||
it('is stripped out of the object saveAppSettings() PUTs to the server', () => {
|
||||
// The strip is a destructure: `showSplitButton: _ssp,` pulls the key out
|
||||
// of `settings` so it never reaches `...serverSettings` in the PUT body.
|
||||
expect(settingsUi).toContain('showSplitButton: _ssp,');
|
||||
});
|
||||
|
||||
it('is never declared in the .strict() SettingsUpdateSchema', () => {
|
||||
// Not even in a comment — a mention there reads as "this is a real
|
||||
// field" to the next person grepping schemas.ts for it.
|
||||
expect(schemas).not.toContain('showSplitButton');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,262 @@
|
||||
// test/split-pane-picker-listener-leak-unit.test.ts
|
||||
// Port: N/A (no server/browser — loaded via `vm`, like split-pane-auto-collapse-unit.test.ts).
|
||||
//
|
||||
// Fast, CI-visible regression coverage for a listener leak in `openSplitPicker()` /
|
||||
// `_dismissSplitPicker()` (terminal-split.js): repeatedly opening the split picker
|
||||
// (e.g. clicking the `.btn-split` button, whose real clicks land on its inner
|
||||
// <svg>) used to leave a `click`/`keydown` listener pair attached to `document`
|
||||
// on every cycle, because a pre-existing menu was torn down with a raw
|
||||
// `existing.remove()` instead of through `_dismissSplitPicker()`, and the
|
||||
// single-slot `_splitPickerDismissHandlers` field was overwritten rather than
|
||||
// used to clean up the previous pair first.
|
||||
//
|
||||
// This is a `vm`-driven DOM-listener-count assertion rather than a real-browser
|
||||
// interaction test (`test/split-pane-orchestration.browser.test.ts` already
|
||||
// covers the real click-through-svg interaction and is excluded from `npm test`
|
||||
// per CLAUDE.md's Testing section) — it exercises the exact document
|
||||
// addEventListener/removeEventListener calls the fix and the bug both hinge on,
|
||||
// with no xterm/WebSocket/tmux involved.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
/** A tiny fake DOM element: enough surface for createElement/appendChild/contains/remove/closest. */
|
||||
function makeFakeElement(tag: string) {
|
||||
const children: Array<ReturnType<typeof makeFakeElement>> = [];
|
||||
let id: string | undefined;
|
||||
let parent: ReturnType<typeof makeFakeElement> | null = null;
|
||||
const el = {
|
||||
tag,
|
||||
style: {} as Record<string, string>,
|
||||
innerHTML: '',
|
||||
className: '',
|
||||
get id() {
|
||||
return id;
|
||||
},
|
||||
set id(v: string | undefined) {
|
||||
id = v;
|
||||
},
|
||||
appendChild(child: ReturnType<typeof makeFakeElement>) {
|
||||
children.push(child);
|
||||
(child as { _parent: unknown })._parent = el;
|
||||
return child;
|
||||
},
|
||||
contains(target: unknown): boolean {
|
||||
if (target === el) return true;
|
||||
return children.some((c) => c.contains(target));
|
||||
},
|
||||
// Real Element.closest(): walk up from THIS element (inclusive), matching
|
||||
// a bare class selector like `.btn-split` against a space-separated
|
||||
// className — enough to exercise the fix's `e.target.closest('.btn-split')`
|
||||
// check without a real DOM.
|
||||
closest(selector: string): ReturnType<typeof makeFakeElement> | null {
|
||||
const cls = selector.replace(/^\./, '');
|
||||
let node: ReturnType<typeof makeFakeElement> | null = el;
|
||||
while (node) {
|
||||
if (node.className.split(/\s+/).includes(cls)) return node;
|
||||
node = (node as unknown as { __parent: ReturnType<typeof makeFakeElement> | null }).__parent;
|
||||
}
|
||||
return null;
|
||||
},
|
||||
remove() {
|
||||
if (parent) {
|
||||
parent = null;
|
||||
}
|
||||
},
|
||||
getBoundingClientRect() {
|
||||
return { bottom: 0, right: 0, left: 0, top: 0, width: 0, height: 0 };
|
||||
},
|
||||
set _parent(p: ReturnType<typeof makeFakeElement>) {
|
||||
parent = p;
|
||||
(el as unknown as { __parent: ReturnType<typeof makeFakeElement> }).__parent = p;
|
||||
},
|
||||
};
|
||||
return el;
|
||||
}
|
||||
|
||||
/** A minimal `document` mock tracking real addEventListener/removeEventListener identity. */
|
||||
function makeFakeDocument() {
|
||||
const elementsById = new Map<string, ReturnType<typeof makeFakeElement>>();
|
||||
const listeners: Record<string, Array<(...args: unknown[]) => unknown>> = {
|
||||
click: [],
|
||||
keydown: [],
|
||||
};
|
||||
|
||||
return {
|
||||
createElement(tag: string) {
|
||||
return makeFakeElement(tag);
|
||||
},
|
||||
body: {
|
||||
appendChild(el: ReturnType<typeof makeFakeElement>) {
|
||||
if (el.id) elementsById.set(el.id, el);
|
||||
},
|
||||
},
|
||||
getElementById(id: string) {
|
||||
const el = elementsById.get(id);
|
||||
if (el) {
|
||||
// Support the real `?.remove()` call site removing it from the registry.
|
||||
const originalRemove = el.remove.bind(el);
|
||||
el.remove = () => {
|
||||
elementsById.delete(id);
|
||||
originalRemove();
|
||||
};
|
||||
}
|
||||
return el;
|
||||
},
|
||||
querySelector(sel: string) {
|
||||
return sel === '.btn-split' ? makeFakeElement('button') : null;
|
||||
},
|
||||
addEventListener(type: string, fn: (...args: unknown[]) => unknown) {
|
||||
(listeners[type] ??= []).push(fn);
|
||||
},
|
||||
removeEventListener(type: string, fn: (...args: unknown[]) => unknown) {
|
||||
listeners[type] = (listeners[type] ?? []).filter((f) => f !== fn);
|
||||
},
|
||||
__listenerCount(type: string) {
|
||||
return (listeners[type] ?? []).length;
|
||||
},
|
||||
// Test-only helpers: fire a snapshot of the currently-registered
|
||||
// listeners (a handler removing itself mid-dispatch must not skip or
|
||||
// double-invoke a sibling — snapshotting avoids that ambiguity here).
|
||||
__dispatchClick(target: unknown) {
|
||||
for (const fn of [...(listeners.click ?? [])]) fn({ target });
|
||||
},
|
||||
__dispatchKeydown(key: string) {
|
||||
for (const fn of [...(listeners.keydown ?? [])]) fn({ key });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function loadCodemanAppClass(documentMock: ReturnType<typeof makeFakeDocument>) {
|
||||
const dir = resolve(import.meta.dirname, '../src/web/public');
|
||||
const terminalSplitSrc = readFileSync(resolve(dir, 'terminal-split.js'), 'utf8');
|
||||
const context = vm.createContext({
|
||||
console: { ...console, log: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
window: { innerWidth: 1024 },
|
||||
document: documentMock,
|
||||
setTimeout,
|
||||
// terminal-split.js's picker markup calls the global `escapeHtml()` helper
|
||||
// (defined in constants.js at runtime); a plain identity stub is enough
|
||||
// here since no candidate labels/ids in this test contain HTML.
|
||||
escapeHtml: (s: unknown) => String(s),
|
||||
});
|
||||
const fakeAppSrc = `
|
||||
class CodemanApp {
|
||||
constructor() {
|
||||
this.sessions = new Map();
|
||||
this.sessionOrder = [];
|
||||
this.activeSessionId = 'active-session';
|
||||
this._splitPane = null;
|
||||
}
|
||||
}
|
||||
`;
|
||||
vm.runInContext(
|
||||
`${fakeAppSrc}\nwindow.CodemanSplitPane = { buildSplitPickerSessions: () => [] };\n${terminalSplitSrc}\nglobalThis.__CodemanApp = CodemanApp;`,
|
||||
context
|
||||
);
|
||||
return (
|
||||
context as { __CodemanApp: new () => { openSplitPicker: (e?: unknown) => void; _dismissSplitPicker: () => void } }
|
||||
).__CodemanApp;
|
||||
}
|
||||
|
||||
describe('terminal-split.js split-picker document-listener leak (repeated open/close)', () => {
|
||||
let documentMock: ReturnType<typeof makeFakeDocument>;
|
||||
let CodemanApp: ReturnType<typeof loadCodemanAppClass>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
documentMock = makeFakeDocument();
|
||||
CodemanApp = loadCodemanAppClass(documentMock);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('does not grow document click/keydown listeners across repeated open cycles with no dismissal', () => {
|
||||
const app = new CodemanApp();
|
||||
|
||||
// Simulates the real failure sequence: the button's onclick re-fires
|
||||
// `openSplitPicker()` on every click (e.g. landing on the button's inner
|
||||
// <svg>) without an intervening outside click or Escape ever resolving.
|
||||
for (let i = 0; i < 5; i++) {
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers(); // flush the deferred `setTimeout(() => addEventListener('click', ...))`
|
||||
}
|
||||
|
||||
// Exactly one pair should be live — the CURRENT menu's — never one per cycle.
|
||||
expect(documentMock.__listenerCount('click')).toBe(1);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(1);
|
||||
});
|
||||
|
||||
it('drops to zero listeners after the outside-click handler fires', () => {
|
||||
const app = new CodemanApp();
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers();
|
||||
expect(documentMock.__listenerCount('click')).toBe(1);
|
||||
|
||||
app._dismissSplitPicker();
|
||||
|
||||
expect(documentMock.__listenerCount('click')).toBe(0);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(0);
|
||||
});
|
||||
|
||||
it('never accumulates listeners across many open→dismiss cycles', () => {
|
||||
const app = new CodemanApp();
|
||||
|
||||
for (let i = 0; i < 20; i++) {
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers();
|
||||
app._dismissSplitPicker();
|
||||
}
|
||||
|
||||
expect(documentMock.__listenerCount('click')).toBe(0);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(0);
|
||||
});
|
||||
|
||||
it("does not dismiss when the click lands on the split button's own inner icon (closest() match, not exact-node equality)", () => {
|
||||
const app = new CodemanApp();
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers();
|
||||
expect(documentMock.__listenerCount('click')).toBe(1);
|
||||
|
||||
// Mirrors the real DOM: the button carries the onclick and the
|
||||
// `.btn-split` class, but the actual click target is its inner <svg>.
|
||||
// The old `e.target !== splitBtn` check matched this (target !== button)
|
||||
// and dismissed the menu the same click had just (re)opened.
|
||||
const button = makeFakeElement('button');
|
||||
button.className = 'btn-split';
|
||||
const svg = button.appendChild(makeFakeElement('svg'));
|
||||
|
||||
documentMock.__dispatchClick(svg);
|
||||
|
||||
expect(documentMock.__listenerCount('click')).toBe(1);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(1);
|
||||
});
|
||||
|
||||
it('dismisses and removes both listeners by identity when a genuine outside click fires', () => {
|
||||
const app = new CodemanApp();
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers();
|
||||
expect(documentMock.__listenerCount('click')).toBe(1);
|
||||
|
||||
const outside = makeFakeElement('div');
|
||||
documentMock.__dispatchClick(outside);
|
||||
|
||||
expect(documentMock.__listenerCount('click')).toBe(0);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(0);
|
||||
});
|
||||
|
||||
it('dismisses and removes both listeners by identity when Escape fires', () => {
|
||||
const app = new CodemanApp();
|
||||
app.openSplitPicker();
|
||||
vi.runAllTimers();
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(1);
|
||||
|
||||
documentMock.__dispatchKeydown('Escape');
|
||||
|
||||
expect(documentMock.__listenerCount('click')).toBe(0);
|
||||
expect(documentMock.__listenerCount('keydown')).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,316 @@
|
||||
/** @fileoverview Real Chromium + real WebSocket coverage for SplitTerminalPane (Task 4 of the split-pane-sessions plan). */
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { chromium, type Browser, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3175;
|
||||
const BASE_URL = `http://localhost:${PORT}`;
|
||||
|
||||
describe('SplitTerminalPane in a real browser', () => {
|
||||
let server: WebServer;
|
||||
let browser: Browser;
|
||||
let page: Page;
|
||||
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT, false, true);
|
||||
await server.start();
|
||||
browser = await chromium.launch({ headless: true });
|
||||
page = await browser.newPage();
|
||||
await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
|
||||
await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 });
|
||||
}, 90000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (browser) await browser.close();
|
||||
if (server) await server.stop();
|
||||
}, 60000);
|
||||
|
||||
it('connects, echoes real PTY output, and cleans up on destroy', async () => {
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', mode: 'shell' }),
|
||||
});
|
||||
const id = (await res.json()).data.session.id;
|
||||
// Session creation alone leaves pid:null and no pane (per CLAUDE.md's
|
||||
// Testing section) — the shell PTY only spawns once this is called, and
|
||||
// without it the WS opens but no bytes ever flow, and the echo assertion
|
||||
// below would hang until its own timeout for reasons unrelated to
|
||||
// SplitTerminalPane.
|
||||
await fetch(`/api/sessions/${id}/shell`, { method: 'POST' });
|
||||
return id;
|
||||
});
|
||||
|
||||
const result = await page.evaluate(async (id) => {
|
||||
const mount = document.createElement('div');
|
||||
mount.style.width = '400px';
|
||||
mount.style.height = '300px';
|
||||
document.body.appendChild(mount);
|
||||
|
||||
const pane = new (window as any).SplitTerminalPane(id, mount);
|
||||
pane.connect();
|
||||
|
||||
// Wait for the WS to open, then send a real input frame — testMode's
|
||||
// echo PTY (TEST_PTY_SCRIPT) echoes each byte back exactly once, which
|
||||
// is what proves the WS round-trip actually reaches a real PTY and back,
|
||||
// not just that xterm can render locally-written text.
|
||||
await new Promise((resolve) => {
|
||||
const check = () => (pane._wsReady ? resolve(undefined) : setTimeout(check, 100));
|
||||
check();
|
||||
});
|
||||
pane.ws.send(JSON.stringify({ t: 'i', d: 'SPLITPANE_MARKER\r' }));
|
||||
|
||||
const hasEcho = await new Promise((resolve) => {
|
||||
const deadline = Date.now() + 5000;
|
||||
const poll = () => {
|
||||
const buf = pane.terminal.buffer.active;
|
||||
for (let i = 0; i < buf.length; i++) {
|
||||
if (buf.getLine(i)?.translateToString(true).includes('SPLITPANE_MARKER')) {
|
||||
resolve(true);
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (Date.now() > deadline) resolve(false);
|
||||
else setTimeout(poll, 100);
|
||||
};
|
||||
poll();
|
||||
});
|
||||
|
||||
pane.destroy();
|
||||
const cleanedUp = mount.querySelector('.xterm') === null;
|
||||
document.body.removeChild(mount);
|
||||
|
||||
return { hasEcho, cleanedUp };
|
||||
}, sessionId);
|
||||
|
||||
expect(result.hasEcho).toBe(true);
|
||||
expect(result.cleanedUp).toBe(true);
|
||||
|
||||
await page.evaluate(async (id) => {
|
||||
await fetch(`/api/sessions/${id}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
|
||||
it('shows existing scrollback immediately on connect, before any new output', async () => {
|
||||
// Regression guard: connect() previously only opened the WS and waited for
|
||||
// live 'terminal' events (ws-routes.ts sends nothing on connect), so a pane
|
||||
// opened onto an already-quiet session stayed blank until either new output
|
||||
// arrived or a resize happened to trigger a tmux repaint. Writing a marker
|
||||
// and letting the echo settle BEFORE connect() proves the fetched buffer,
|
||||
// not a live echo, is what populates the pane.
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', mode: 'shell' }),
|
||||
});
|
||||
const id = (await res.json()).data.session.id;
|
||||
await fetch(`/api/sessions/${id}/shell`, { method: 'POST' });
|
||||
// Write directly to the session (not through SplitTerminalPane, which
|
||||
// does not exist yet). Poll the real ?full=1 capture (same endpoint
|
||||
// connect() below will use) rather than a fixed delay — the shell's
|
||||
// own startup can race an early write and, on this box, a startup
|
||||
// script issues a `clear` that erases scrollback (modern ncurses
|
||||
// `clear` emits \x1b[3J) if the input lands before the shell is ready.
|
||||
await fetch(`/api/sessions/${id}/input`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ input: 'PRE_EXISTING_MARKER\r' }),
|
||||
});
|
||||
const deadline = Date.now() + 5000;
|
||||
for (;;) {
|
||||
const res2 = await fetch(`/api/sessions/${id}/terminal?full=1`);
|
||||
const buffer = (await res2.json())?.data?.terminalBuffer ?? '';
|
||||
if (buffer.includes('PRE_EXISTING_MARKER')) break;
|
||||
if (Date.now() > deadline) throw new Error('marker never landed in ?full=1 capture: ' + JSON.stringify(buffer));
|
||||
await new Promise((r) => setTimeout(r, 200));
|
||||
}
|
||||
return id;
|
||||
});
|
||||
|
||||
const hasMarker = await page.evaluate(async (id) => {
|
||||
const mount = document.createElement('div');
|
||||
mount.style.width = '400px';
|
||||
mount.style.height = '300px';
|
||||
document.body.appendChild(mount);
|
||||
|
||||
const pane = new (window as any).SplitTerminalPane(id, mount);
|
||||
await pane.connect();
|
||||
|
||||
// xterm's write() parses asynchronously (it queues data and processes it
|
||||
// on a later microtask/frame), so the fetched buffer connect() writes is
|
||||
// not necessarily in the rendered buffer the instant connect() resolves.
|
||||
// Poll rather than check once — no new input is sent here, so any pass
|
||||
// still comes from the ?full=1 fetch inside connect(), never a live echo.
|
||||
let found = false;
|
||||
const deadline = Date.now() + 3000;
|
||||
while (!found && Date.now() < deadline) {
|
||||
const buf = pane.terminal.buffer.active;
|
||||
for (let i = 0; i < buf.length; i++) {
|
||||
if (buf.getLine(i)?.translateToString(true).includes('PRE_EXISTING_MARKER')) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!found) await new Promise((r) => setTimeout(r, 50));
|
||||
}
|
||||
|
||||
pane.destroy();
|
||||
document.body.removeChild(mount);
|
||||
return found;
|
||||
}, sessionId);
|
||||
|
||||
expect(hasMarker).toBe(true);
|
||||
|
||||
await page.evaluate(async (id) => {
|
||||
await fetch(`/api/sessions/${id}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
|
||||
it('gates app-level chords out of Pane B instead of forwarding their raw bytes', async () => {
|
||||
// Regression guard for PR #453's Ctrl+K/Alt+1/Alt+B leak: Pane B had no
|
||||
// attachCustomKeyEventHandler of its own, so the document capture-phase
|
||||
// shortcut handler's preventDefault() (which does not stop xterm) left
|
||||
// every one of these chords ALSO writing its raw byte/escape sequence into
|
||||
// Pane B's live PTY on top of whatever the app action did to Pane A.
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', mode: 'shell' }),
|
||||
});
|
||||
const id = (await res.json()).data.session.id;
|
||||
await fetch(`/api/sessions/${id}/shell`, { method: 'POST' });
|
||||
return id;
|
||||
});
|
||||
|
||||
const result = await page.evaluate(async (id) => {
|
||||
const mount = document.createElement('div');
|
||||
mount.style.width = '400px';
|
||||
mount.style.height = '300px';
|
||||
document.body.appendChild(mount);
|
||||
|
||||
const pane = new (window as any).SplitTerminalPane(id, mount);
|
||||
await pane.connect();
|
||||
await new Promise((resolve) => {
|
||||
const check = () => (pane._wsReady ? resolve(undefined) : setTimeout(check, 100));
|
||||
check();
|
||||
});
|
||||
|
||||
const sent: string[] = [];
|
||||
const realSend = pane.ws.send.bind(pane.ws);
|
||||
pane.ws.send = (payload: string) => {
|
||||
sent.push(payload);
|
||||
return realSend(payload);
|
||||
};
|
||||
|
||||
pane.terminal.focus();
|
||||
// Dispatch straight at xterm's own textarea, matching how a real
|
||||
// keypress reaches attachCustomKeyEventHandler — page.keyboard.press()
|
||||
// goes through the OS/CDP input pipeline and would also trigger the
|
||||
// app's document-capture handler (opening a real command palette),
|
||||
// which is not what this test is isolating.
|
||||
const textarea = (pane.terminal as any)._core?.textarea || (pane.terminal as any).textarea;
|
||||
const fire = (init: KeyboardEventInit) => {
|
||||
const event = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init });
|
||||
textarea.dispatchEvent(event);
|
||||
return event.defaultPrevented;
|
||||
};
|
||||
// keyCode is what xterm's evaluateKeyboardEvent switches on to decide
|
||||
// whether to produce a data frame at all — at keyCode 0 (unset) it can
|
||||
// never emit bytes, so the assertion below held regardless of whether
|
||||
// the custom key handler's gate actually fired. Real values (K=75,
|
||||
// 1=49, B=66) are what a real keypress carries.
|
||||
const app = window.app as any;
|
||||
fire({ key: 'k', code: 'KeyK', keyCode: 75, ctrlKey: true }); // command palette
|
||||
fire({ key: '1', code: 'Digit1', keyCode: 49, altKey: true }); // Alt+1 tab switch
|
||||
|
||||
// Ctrl+Z (SIGTSTP): this pane's own sessionMode is undefined (no `mode`
|
||||
// opt passed to the constructor above), so `this.sessionMode !== 'shell'`
|
||||
// holds and the gate must block it, mirroring a non-shell (agent) mode.
|
||||
fire({ key: 'z', code: 'KeyZ', keyCode: 90, ctrlKey: true });
|
||||
|
||||
// Shift+Enter: must never reach the PTY as a bare \r (that would submit
|
||||
// an incomplete prompt instead of inserting a newline) — it goes out as
|
||||
// a POST to /api/sessions/:id/send-key instead.
|
||||
const sendKeyCalls: unknown[] = [];
|
||||
const realFetch = window.fetch.bind(window);
|
||||
window.fetch = ((...args: Parameters<typeof fetch>) => {
|
||||
const url = String(args[0]);
|
||||
if (url.includes('/send-key')) {
|
||||
sendKeyCalls.push(args[1] ? JSON.parse((args[1] as RequestInit).body as string) : null);
|
||||
}
|
||||
return realFetch(...args);
|
||||
}) as typeof fetch;
|
||||
fire({ key: 'Enter', code: 'Enter', keyCode: 13, shiftKey: true });
|
||||
window.fetch = realFetch;
|
||||
|
||||
// Smart-copy Ctrl+C: with a real selection in THIS pane's own terminal,
|
||||
// Ctrl+C must copy it (never send 0x03) and must copy Pane B's
|
||||
// selection, not Pane A's. app._copyText is stubbed rather than relying
|
||||
// on a real clipboard, which headless Chromium may refuse permission
|
||||
// for.
|
||||
pane.terminal.write('SPLITPANE_COPY_MARKER');
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
pane.terminal.selectAll();
|
||||
let copiedText: string | null = null;
|
||||
const realCopyText = app._copyText;
|
||||
app._copyText = async (text: string) => {
|
||||
copiedText = text;
|
||||
return true;
|
||||
};
|
||||
fire({ key: 'c', code: 'KeyC', keyCode: 67, ctrlKey: true });
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
app._copyText = realCopyText;
|
||||
|
||||
// Ctrl+Shift+C with NO selection: the blanket "no 'i' frames" check
|
||||
// below is NOT what proves this gate works — xterm's own
|
||||
// evaluateKeyboardEvent never emits data for a shifted ctrl-letter in
|
||||
// the first place (verified live: removing the gate entirely still
|
||||
// produces zero WS frames for this exact key), so an absent 'i' frame
|
||||
// is true whether or not the app-level shiftKey branch fires. What the
|
||||
// branch actually buys is `preventDefault()`, so the browser's own
|
||||
// handling of the chord (e.g. Chrome's Inspect-Element binding) is
|
||||
// pre-empted, mirroring Pane A's own "never falls through" contract —
|
||||
// asserted directly via the dispatched event's defaultPrevented.
|
||||
pane.terminal.clearSelection();
|
||||
const ctrlShiftCPrevented = fire({ key: 'c', code: 'KeyC', keyCode: 67, ctrlKey: true, shiftKey: true });
|
||||
|
||||
// Alt+B only reaches shouldToggleSessionSidebarFromShortcut's gate when
|
||||
// the sidebar layout is actually active (app.js:4325) — under the
|
||||
// default header-strip layout the app doesn't treat Alt+B as its own
|
||||
// shortcut either, so Pane A forwards the same `ESC b` to its own PTY.
|
||||
// Assert the gate where it is meant to hold: sidebar layout active.
|
||||
//
|
||||
// Setting only the `data-session-list` attribute is not enough: this
|
||||
// event bubbles (matching how a real keypress reaches xterm), so it
|
||||
// also reaches app.js's OWN document-level capture-phase shortcut
|
||||
// dispatcher, which matches the same Alt+B binding and calls the real
|
||||
// toggleSessionSidebar() — that reads the persisted settings (still
|
||||
// 'header'), re-runs applySessionListLayout(), and resets the
|
||||
// attribute back to 'header' before xterm's own (later, non-capture)
|
||||
// key handler ever sees it. Persisting the setting through the app's
|
||||
// own settings cache keeps the attribute stable across that bubble.
|
||||
const prevSettings = { ...app.loadAppSettingsFromStorage() };
|
||||
app._cachedAppSettings = { ...prevSettings, sessionListLayout: 'sidebar' };
|
||||
app.applySessionListLayout();
|
||||
fire({ key: 'b', code: 'KeyB', keyCode: 66, altKey: true }); // Alt+B sidebar toggle
|
||||
app._cachedAppSettings = prevSettings;
|
||||
app.applySessionListLayout();
|
||||
|
||||
pane.destroy();
|
||||
document.body.removeChild(mount);
|
||||
return { sent, sendKeyCalls, copiedText, ctrlShiftCPrevented };
|
||||
}, sessionId);
|
||||
|
||||
expect(result.sent.every((f) => JSON.parse(f).t !== 'i')).toBe(true);
|
||||
expect(result.sendKeyCalls).toEqual([{ key: 'S-Enter' }]);
|
||||
expect(result.copiedText).toContain('SPLITPANE_COPY_MARKER');
|
||||
expect(result.ctrlShiftCPrevented).toBe(true);
|
||||
|
||||
await page.evaluate(async (id) => {
|
||||
await fetch(`/api/sessions/${id}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user