mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
perf(sessions): closing a session no longer waits on the server
Closing a tab took ~0.55-0.7s on an idle machine, more with child processes or recently active subagents. Most of it was waiting: - The web UI kept the tab until DELETE returned, then removed it on the 100ms tab-render debounce, which every session update restarts. closeSession() is now optimistic: the tab, tile and split go and the next session is selected before the request is sent, rendered at once. A refused delete (checked with a GET, since a delete can land and lose its reply) puts the row back at its old index with the error toast. This also fixes a latent bug: _apiDelete never throws, so an HTTP error used to report "Session closed" while the session kept running. SSE upserts skip ids that are being closed. - The kill path slept fixed intervals (100ms PTY grace, 200ms for the pane's children, 100ms for the process group) and verified in 100ms steps. waitForProcessesExit() (utils/process-exit-wait.ts) keeps every deadline but returns once the processes are gone, counting a zombie as exited. Signal decisions keep kill(pid, 0). - tmux kill-session and the pane-pid lookup ran via execSync, freezing the server for ~70ms per close. Now async. - killSubagentsForSession() ran a full `pgrep -f claude` scan per active/idle subagent (~85ms each with ~100 matching processes). It now scans once for all of them. Measured on an isolated instance: click to tab gone 540-690ms -> 58-95ms; DELETE of a claude session ~450ms -> ~200-260ms. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9644892a5a
commit
89177651a6
12 files changed
+487
-65
No files matched your search
@@ -16,6 +16,11 @@
|
||||
* leaves the active-session handoff alone for a close this tab started. A delete
|
||||
* from anywhere else still lands on the welcome screen.
|
||||
*
|
||||
* The close is also OPTIMISTIC: the tab goes and the next one is selected before
|
||||
* the DELETE is even sent (the server's kill takes a few hundred ms, and a tab
|
||||
* sitting there that long read as a dead button). A refused delete puts the row
|
||||
* back where it was; one whose reply was lost is checked with a GET first.
|
||||
*
|
||||
* Loaded via `vm` with a stubbed context (no jsdom), like input-send-order.test.ts.
|
||||
* Port: N/A.
|
||||
*/
|
||||
@@ -74,7 +79,11 @@ function makeApp(active: string | null, order = [A, B]): TestApp {
|
||||
app.isSoloWindow = false;
|
||||
app._wsSessionId = null;
|
||||
app.terminal = { clear: vi.fn() };
|
||||
app._apiDelete = vi.fn(async () => ({ success: true }));
|
||||
// Shaped like the Response the real helper resolves with (or null: no answer).
|
||||
app._apiDelete = vi.fn(async () => ({ ok: true, status: 200 }));
|
||||
// The failure path's "is it still there?" check: yes, by default.
|
||||
app._api = vi.fn(async () => ({ ok: true, status: 200 }));
|
||||
app.saveSessionOrder = vi.fn();
|
||||
// The real one touches ~20 maps; the parts this behavior depends on are the
|
||||
// session map and the tab order, so those are pruned for real.
|
||||
app._cleanupSessionData = vi.fn((id: string) => {
|
||||
@@ -101,7 +110,7 @@ describe('closing the active session', () => {
|
||||
// delete lands before the request resolves.
|
||||
app._apiDelete = vi.fn(async () => {
|
||||
app._onSessionDeleted({ id: A });
|
||||
return { success: true };
|
||||
return { ok: true, status: 200 };
|
||||
});
|
||||
|
||||
await app.closeSession(A);
|
||||
@@ -174,3 +183,84 @@ describe('closing the active session', () => {
|
||||
expect(app.showToast).toHaveBeenCalledWith('Failed to close session', 'error');
|
||||
});
|
||||
});
|
||||
|
||||
describe('closing is optimistic', () => {
|
||||
it('drops the tab and selects the next one BEFORE the server answers', async () => {
|
||||
const app = makeApp(A);
|
||||
let answer: (value: unknown) => void = () => {};
|
||||
app._apiDelete = vi.fn(() => new Promise((r) => (answer = r)));
|
||||
|
||||
const closing = app.closeSession(A);
|
||||
|
||||
expect(app.sessions.has(A)).toBe(false);
|
||||
expect(app.selectSession).toHaveBeenCalledWith(B, { auto: true });
|
||||
// Rendered now, not on the debounce that every session update restarts.
|
||||
expect(app.renderSessionTabs).toHaveBeenCalledWith({ immediate: true });
|
||||
expect(app.showToast).not.toHaveBeenCalled();
|
||||
|
||||
answer({ ok: true, status: 200 });
|
||||
await closing;
|
||||
expect(app.showToast).toHaveBeenCalledWith('Session closed and tmux killed', 'success');
|
||||
});
|
||||
|
||||
it('a refused delete puts the row back where it was, and says so', async () => {
|
||||
const app = makeApp(B, [A, B]);
|
||||
app._apiDelete = vi.fn(async () => ({ ok: false, status: 500 }));
|
||||
|
||||
await app.closeSession(A);
|
||||
|
||||
expect(app._api).toHaveBeenCalledWith(`/api/sessions/${A}`);
|
||||
expect(app.sessions.has(A)).toBe(true);
|
||||
expect(app.sessionOrder).toEqual([A, B]);
|
||||
expect(app.showToast).toHaveBeenCalledWith('Failed to close session', 'error');
|
||||
});
|
||||
|
||||
it('a delete that landed but lost its reply counts as closed', async () => {
|
||||
const app = makeApp(A);
|
||||
app._apiDelete = vi.fn(async () => null);
|
||||
app._api = vi.fn(async () => ({ ok: false, status: 404 }));
|
||||
|
||||
await app.closeSession(A);
|
||||
|
||||
expect(app.sessions.has(A)).toBe(false);
|
||||
expect(app.showToast).toHaveBeenCalledWith('Session closed and tmux killed', 'success');
|
||||
});
|
||||
|
||||
it('a 404 from the delete is a close that already happened', async () => {
|
||||
const app = makeApp(A);
|
||||
app._apiDelete = vi.fn(async () => ({ ok: false, status: 404 }));
|
||||
|
||||
await app.closeSession(A);
|
||||
|
||||
expect(app._api).not.toHaveBeenCalled();
|
||||
expect(app.sessions.has(A)).toBe(false);
|
||||
expect(app.showToast).toHaveBeenCalledWith('Session closed and tmux killed', 'success');
|
||||
});
|
||||
|
||||
it('a session update arriving mid-close does not bring the tab back', async () => {
|
||||
const app = makeApp(A);
|
||||
app.updateCost = vi.fn();
|
||||
app.updateSubagentParentNames = vi.fn();
|
||||
app._apiDelete = vi.fn(async () => {
|
||||
(app as unknown as { _onSessionUpdated: (d: unknown) => void })._onSessionUpdated({ id: A, status: 'idle' });
|
||||
return { ok: true, status: 200 };
|
||||
});
|
||||
|
||||
await app.closeSession(A);
|
||||
|
||||
expect(app.sessions.has(A)).toBe(false);
|
||||
});
|
||||
|
||||
it('a second close of the same tab while the first is in flight is a no-op', async () => {
|
||||
const app = makeApp(A);
|
||||
let answer: (value: unknown) => void = () => {};
|
||||
app._apiDelete = vi.fn(() => new Promise((r) => (answer = r)));
|
||||
|
||||
const first = app.closeSession(A);
|
||||
await app.closeSession(A);
|
||||
answer({ ok: true, status: 200 });
|
||||
await first;
|
||||
|
||||
expect(app._apiDelete).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user