Skip to content

Commit a8c4774

Browse files
committed
fix: simplify session completion notifications
Remove PTY tail parsing from completion notifications and use fixed completion hints instead, so terminal UI chrome no longer leaks into notification content.
1 parent b3af113 commit a8c4774

11 files changed

Lines changed: 40 additions & 633 deletions

File tree

packages/web/src/app/providers.lifecycle.test.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ vi.mock("../ws", () => ({
3232

3333
vi.mock("../features/notifications", () => ({
3434
useSessionNotifications: () => {},
35-
appendSessionOutputAtom: null,
36-
clearSessionOutputAtom: null,
3735
}));
3836

3937
function renderProviders(store = createStore()) {

packages/web/src/app/providers.test.tsx

Lines changed: 11 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
workspacesLoadErrorAtom,
1010
workspacesLoadStateAtom,
1111
} from "../atoms/workspaces";
12-
import { SESSION_OUTPUT_TAIL_BYTES, sessionOutputTailAtom } from "../features/notifications/atoms";
1312
import { supervisorCyclesAtom, supervisorsAtom } from "../features/supervisor/atoms";
1413
import { terminalMetaAtomFamily } from "../features/terminal-panel/atoms";
1514
import { fileTreeStaleAtomFamily } from "../features/workspace/atoms";
@@ -114,188 +113,22 @@ describe("routeEventToAtom", () => {
114113
expect(store.get(fileTreeStaleAtomFamily("ws-1"))).toBe(false);
115114
});
116115

117-
it("appends cleaned terminal output text to the matching session tail buffer", () => {
116+
it("marks terminal metadata exited on terminal exit events", () => {
118117
const store = createStore();
119-
store.set(sessionsAtom, {
120-
"sess-1": {
121-
id: "sess-1",
122-
workspaceId: "ws-1",
123-
terminalId: "term-1",
124-
providerId: "claude",
125-
state: "running",
126-
capability: "full",
127-
startedAt: 1,
128-
lastActiveAt: 1,
129-
},
130-
});
131-
132-
routeEventToAtom(
133-
"workspace.ws-1.terminal.term-1.output",
134-
{
135-
transport: "binary",
136-
streamId: 7,
137-
size: 11,
138-
bytes: new TextEncoder().encode("hello\n"),
139-
},
140-
store
141-
);
142-
143-
expect(store.get(sessionOutputTailAtom)).toEqual({
144-
"sess-1": "hello\n",
145-
});
146-
});
147-
148-
it("strips ANSI sequences split across multiple terminal output events", () => {
149-
const store = createStore();
150-
store.set(sessionsAtom, {
151-
"sess-1": {
152-
id: "sess-1",
153-
workspaceId: "ws-1",
154-
terminalId: "term-1",
155-
providerId: "claude",
156-
state: "running",
157-
capability: "full",
158-
startedAt: 1,
159-
lastActiveAt: 1,
160-
},
161-
});
162-
163-
routeEventToAtom(
164-
"workspace.ws-1.terminal.term-1.output",
165-
{
166-
transport: "binary",
167-
streamId: 7,
168-
size: 4,
169-
bytes: new TextEncoder().encode("\x1b[32"),
170-
},
171-
store
172-
);
173-
174-
expect(store.get(sessionOutputTailAtom)).toEqual({});
175-
176-
routeEventToAtom(
177-
"workspace.ws-1.terminal.term-1.output",
178-
{
179-
transport: "binary",
180-
streamId: 8,
181-
size: 11,
182-
bytes: new TextEncoder().encode("mhello\x1b[0m\n"),
183-
},
184-
store
185-
);
186-
187-
expect(store.get(sessionOutputTailAtom)).toEqual({
188-
"sess-1": "hello\n",
189-
});
190-
});
191-
192-
it("preserves multibyte utf-8 characters split across terminal output events", () => {
193-
const store = createStore();
194-
store.set(sessionsAtom, {
195-
"sess-1": {
196-
id: "sess-1",
197-
workspaceId: "ws-1",
198-
terminalId: "term-1",
199-
providerId: "claude",
200-
state: "running",
201-
capability: "full",
202-
startedAt: 1,
203-
lastActiveAt: 1,
204-
},
205-
});
206-
207-
routeEventToAtom(
208-
"workspace.ws-1.terminal.term-1.output",
209-
{
210-
transport: "binary",
211-
streamId: 7,
212-
size: 2,
213-
bytes: new Uint8Array([0x41, 0xe2]),
214-
},
215-
store
216-
);
217-
218-
expect(store.get(sessionOutputTailAtom)).toEqual({
219-
"sess-1": "A",
220-
});
221-
222-
routeEventToAtom(
223-
"workspace.ws-1.terminal.term-1.output",
224-
{
225-
transport: "binary",
226-
streamId: 8,
227-
size: 3,
228-
bytes: new Uint8Array([0x9c, 0x93, 0x42]),
229-
},
230-
store
231-
);
232-
233-
expect(store.get(sessionOutputTailAtom)).toEqual({
234-
"sess-1": "A✓B",
118+
store.set(terminalMetaAtomFamily("term-1"), {
119+
id: "term-1",
120+
workspaceId: "ws-1",
121+
kind: "agent",
122+
alive: true,
123+
title: "Claude",
235124
});
236-
});
237125

238-
it("does not leak ANSI fragments when the cleaned tail buffer trims from the head at the size cap", () => {
239-
const store = createStore();
240-
store.set(sessionsAtom, {
241-
"sess-1": {
242-
id: "sess-1",
243-
workspaceId: "ws-1",
244-
terminalId: "term-1",
245-
providerId: "claude",
246-
state: "running",
247-
capability: "full",
248-
startedAt: 1,
249-
lastActiveAt: 1,
250-
},
251-
});
126+
routeEventToAtom("workspace.ws-1.terminal.term-1.exit", { code: 1 }, store);
252127

253-
store.set(sessionOutputTailAtom, {
254-
"sess-1": "x".repeat(SESSION_OUTPUT_TAIL_BYTES - 2),
128+
expect(store.get(terminalMetaAtomFamily("term-1"))).toMatchObject({
129+
alive: false,
130+
exitCode: 1,
255131
});
256-
257-
routeEventToAtom(
258-
"workspace.ws-1.terminal.term-1.output",
259-
{
260-
transport: "binary",
261-
streamId: 7,
262-
size: 4,
263-
bytes: new TextEncoder().encode("\x1b[32"),
264-
},
265-
store
266-
);
267-
268-
routeEventToAtom(
269-
"workspace.ws-1.terminal.term-1.output",
270-
{
271-
transport: "binary",
272-
streamId: 8,
273-
size: 6,
274-
bytes: new TextEncoder().encode("mOK\n"),
275-
},
276-
store
277-
);
278-
279-
const tail = store.get(sessionOutputTailAtom)["sess-1"];
280-
expect(tail).toBe("x".repeat(SESSION_OUTPUT_TAIL_BYTES - 3) + "OK\n");
281-
expect(tail.startsWith("[32m")).toBe(false);
282-
});
283-
284-
it("ignores terminal output bytes when no matching session exists", () => {
285-
const store = createStore();
286-
287-
routeEventToAtom(
288-
"workspace.ws-1.terminal.term-1.output",
289-
{
290-
transport: "binary",
291-
streamId: 7,
292-
size: 5,
293-
bytes: new TextEncoder().encode("hello"),
294-
},
295-
store
296-
);
297-
298-
expect(store.get(sessionOutputTailAtom)).toEqual({});
299132
});
300133

301134
it("removes local session artifacts on session removed lifecycle events", () => {
@@ -321,14 +154,10 @@ describe("routeEventToAtom", () => {
321154
exitCode: 1,
322155
title: "Claude",
323156
});
324-
store.set(sessionOutputTailAtom, {
325-
"sess-1": "tail output",
326-
});
327157

328158
routeEventToAtom("workspace.ws-1.session.sess-1.lifecycle", { event: "removed" }, store);
329159

330160
expect(store.get(sessionsAtom)).toEqual({});
331161
expect(store.get(terminalMetaAtomFamily("term-1"))).toBeNull();
332-
expect(store.get(sessionOutputTailAtom)).toEqual({});
333162
});
334163
});

packages/web/src/app/providers.tsx

Lines changed: 4 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,7 @@ import {
3434
} from "../atoms";
3535
import { authenticatedAtom } from "../atoms/app-ui";
3636
import type { DispatchCommand } from "../atoms/connection";
37-
import {
38-
appendSessionOutputAtom,
39-
clearSessionOutputAtom,
40-
useSessionNotifications,
41-
} from "../features/notifications";
42-
import { stripAnsiChunk } from "../features/notifications/format";
37+
import { useSessionNotifications } from "../features/notifications";
4338
import { supervisorCyclesAtom, supervisorsAtom } from "../features/supervisor/atoms";
4439
import { terminalMetaAtomFamily } from "../features/terminal-panel/atoms";
4540
import {
@@ -48,7 +43,7 @@ import {
4843
gitBranchListAtomFamily,
4944
gitStateAtomFamily,
5045
} from "../features/workspace/atoms";
51-
import type { ConnectionStatus, EventListener, TerminalBinaryPayload } from "../ws";
46+
import type { ConnectionStatus, EventListener } from "../ws";
5247
import { resolveWsUrl, WsClient } from "../ws";
5348

5449
/**
@@ -57,8 +52,6 @@ import { resolveWsUrl, WsClient } from "../ws";
5752
*/
5853
let globalWsClient: WsClient | null = null;
5954
let pendingDisconnectTimer: NodeJS.Timeout | null = null;
60-
const sessionOutputDecoders = new Map<string, TextDecoder>();
61-
const sessionOutputAnsiCarry = new Map<string, string>();
6255

6356
interface WorkspaceRefreshHint {
6457
refreshGit: boolean;
@@ -84,8 +77,6 @@ export function resetAppProvidersSingletonsForTests() {
8477
pendingDisconnectTimer = null;
8578
}
8679
globalWsClient = null;
87-
sessionOutputDecoders.clear();
88-
sessionOutputAnsiCarry.clear();
8980
}
9081

9182
function mergeRefreshHints(
@@ -144,38 +135,6 @@ function parseWorkspaceRefreshHint(
144135
};
145136
}
146137

147-
/**
148-
* Shared decoder for terminal output chunks. PTY data is a UTF-8 byte stream.
149-
* Decoding once-per-event is cheap, but keeping the TextDecoder around saves
150-
* allocations at high stream rates.
151-
*/
152-
function clearTerminalOutputStreamState(terminalId: string): void {
153-
sessionOutputDecoders.delete(terminalId);
154-
sessionOutputAnsiCarry.delete(terminalId);
155-
}
156-
157-
function decodeTerminalOutputBytes(terminalId: string, chunk: Uint8Array): string {
158-
let decoder = sessionOutputDecoders.get(terminalId);
159-
if (!decoder) {
160-
decoder = new TextDecoder("utf-8", { fatal: false });
161-
sessionOutputDecoders.set(terminalId, decoder);
162-
}
163-
164-
const decoded = decoder.decode(chunk, { stream: true });
165-
if (!decoded) {
166-
return "";
167-
}
168-
169-
const { cleaned, carry } = stripAnsiChunk(decoded, sessionOutputAnsiCarry.get(terminalId) ?? "");
170-
if (carry) {
171-
sessionOutputAnsiCarry.set(terminalId, carry);
172-
} else {
173-
sessionOutputAnsiCarry.delete(terminalId);
174-
}
175-
176-
return cleaned;
177-
}
178-
179138
interface AppProvidersProps {
180139
children: React.ReactNode;
181140
}
@@ -600,10 +559,8 @@ export function routeEventToAtom(topic: string, payload: unknown, store: Store):
600559
if (data.event === "removed") {
601560
const removedSession = store.get(sessionsAtom)[sessionId];
602561
if (removedSession?.terminalId) {
603-
clearTerminalOutputStreamState(removedSession.terminalId);
604562
store.set(terminalMetaAtomFamily(removedSession.terminalId), null);
605563
}
606-
store.set(clearSessionOutputAtom, sessionId);
607564
store.set(sessionsAtom, (prev: Record<string, Session>) => {
608565
if (!(sessionId in prev)) {
609566
return prev;
@@ -695,7 +652,6 @@ export function routeEventToAtom(topic: string, payload: unknown, store: Store):
695652
// workspace.{id}.terminal.{terminalId}.created
696653
if (terminalSubtopic === "created") {
697654
const data = payload as { id: string; kind: string; title?: string; cwd?: string };
698-
clearTerminalOutputStreamState(terminalId);
699655
const atom = terminalMetaAtomFamily(terminalId);
700656
store.set(atom, {
701657
id: data.id,
@@ -708,28 +664,9 @@ export function routeEventToAtom(topic: string, payload: unknown, store: Store):
708664
}
709665

710666
// workspace.{id}.terminal.{terminalId}.output
667+
// Terminal panels consume output directly via xterm.js — no router-level
668+
// handling needed.
711669
if (terminalSubtopic === "output") {
712-
// Terminal panels render output directly into xterm.js. We skim a
713-
// copy here only when the terminal is bound to a known *agent* session,
714-
// so the notification engine can include a tail summary in its body.
715-
// Shell terminals (no session) are ignored to keep the buffer small.
716-
const data = payload as TerminalBinaryPayload;
717-
const bytes = data?.bytes;
718-
if (!bytes || !ArrayBuffer.isView(bytes) || bytes.byteLength === 0) return;
719-
const sessions = store.get(sessionsAtom);
720-
const session = Object.values(sessions).find((s) => s.terminalId === terminalId);
721-
if (!session) return;
722-
723-
const cleaned = decodeTerminalOutputBytes(
724-
terminalId,
725-
new Uint8Array(bytes.buffer, bytes.byteOffset, bytes.byteLength)
726-
);
727-
if (cleaned) {
728-
store.set(appendSessionOutputAtom, {
729-
sessionId: session.id,
730-
text: cleaned,
731-
});
732-
}
733670
return;
734671
}
735672

@@ -745,14 +682,6 @@ export function routeEventToAtom(topic: string, payload: unknown, store: Store):
745682
alive: false,
746683
});
747684
}
748-
// Clean up the output tail buffer for the session this terminal belonged
749-
// to (if any). Avoids unbounded growth across long-lived browser tabs.
750-
const sessions = store.get(sessionsAtom);
751-
const session = Object.values(sessions).find((s) => s.terminalId === terminalId);
752-
clearTerminalOutputStreamState(terminalId);
753-
if (session) {
754-
store.set(clearSessionOutputAtom, session.id);
755-
}
756685
return;
757686
}
758687
}

0 commit comments

Comments
 (0)