Skip to content

feat(desktop): show a restored-session separator when a v2 terminal cold-restores #5448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kitenite merged 4 commits into from
Jul 4, 2026
Merged

feat(desktop): show a restored-session separator when a v2 terminal cold-restores #5448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
25e1065
feat(desktop): show a restored-session separator when a v2 terminal c…
Kitenite Jul 3, 2026
fc7870b
fix(desktop): keep the restored-session separator out of the bounded …
Kitenite Jul 3, 2026
4a0847f
fix(desktop): don't clear replay state when the attach socket is alre…
Kitenite Jul 4, 2026
5595d16
chore(desktop): drop redundant Buffer.from copies before Buffer.conca…
Kitenite Jul 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 118 additions & 3 deletions packages/host-service/src/terminal/terminal.adoption.node-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
createTerminalSessionInternal,
disposeSessionAndWait,
listTerminalSessions,
replayBuffer,
} from "./terminal.ts";
import { __setAccountShellForTesting } from "./user-shell.ts";

Expand Down Expand Up @@ -343,6 +344,106 @@ describe("createTerminalSessionInternal — host-service restart adoption", () =
await disposeSessionAndWait(terminalId, db);
});

test("restoredNotice delivers the separator ahead of shell output on first replay only", async () => {
const terminalId = `e2e-notice-${randomUUID().slice(0, 8)}`;
const result = await createTerminalSessionInternal({
terminalId,
workspaceId,
db,
listed: true,
restoredNotice: true,
});
Comment on lines +353 to +355

@greptile-apps greptile-apps Bot Jul 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Dead-code guard after a throwing assertion

assert.ok(!("error" in result)) throws when the condition is false, so the if ("error" in result) return; immediately below is unreachable and adds noise. The identical pattern appears in the second test (assert.ok(!("error" in second)) then if ("error" in second) return;). Pick one: keep only the assert.ok (preferred — gives a clear failure message), or drop the assert.ok and rely on the type-narrowing if.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/host-service/src/terminal/terminal.adoption.node-test.ts
Line: 352-354

Comment:
**Dead-code guard after a throwing assertion**

`assert.ok(!("error" in result))` throws when the condition is false, so the `if ("error" in result) return;` immediately below is unreachable and adds noise. The identical pattern appears in the second test (`assert.ok(!("error" in second))` then `if ("error" in second) return;`). Pick one: keep only the `assert.ok` (preferred — gives a clear failure message), or drop the `assert.ok` and rely on the type-narrowing `if`.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@Kitenite Kitenite Jul 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping as-is: assert.ok gives the failure message and the if guard does the TypeScript narrowing — this pattern is the established idiom throughout this file (19 occurrences); changing only the new tests would be inconsistent.

assert.ok(!("error" in result));
if ("error" in result) return;

const first = makeCaptureSocket();
replayBuffer(result, first.socket);
assert.match(
first.received(),
/Session Contents Restored/,
"first replay should carry the restored-session separator",
);

const second = makeCaptureSocket();
replayBuffer(result, second.socket);
assert.doesNotMatch(
second.received(),
/Session Contents Restored/,
"separator should not repeat on later replays",
);

await disposeSessionAndWait(terminalId, db);
});

test("restoredNotice survives FIFO eviction when the shell floods output before attach", async () => {
const terminalId = `e2e-notice-flood-${randomUUID().slice(0, 8)}`;
const suffix = randomUUID().slice(0, 6);
const result = await createTerminalSessionInternal({
terminalId,
workspaceId,
db,
listed: true,
restoredNotice: true,
// > MAX_BUFFER_BYTES (64 KiB) so the FIFO drops its oldest chunks
// before any socket attaches. The marker is assembled by printf so
// the PTY echo of the command line doesn't match it.
initialCommand: `head -c 200000 /dev/zero | tr '\\0' x; printf 'flood-done-%s\\n' "${suffix}"`,
});
assert.ok(!("error" in result));
if ("error" in result) return;

await waitFor(
() => sessionBufferText(result).includes(`flood-done-${suffix}`),
15_000,
);

const capture = makeCaptureSocket();
replayBuffer(result, capture.socket);
const replayed = capture.received();
const noticeIndex = replayed.indexOf("Session Contents Restored");
assert.ok(noticeIndex >= 0, "separator should survive buffer eviction");
assert.ok(
noticeIndex < replayed.indexOf("xxxx"),
"separator should precede the flooded shell output",
);

await disposeSessionAndWait(terminalId, db);
});

test("restoredNotice is skipped when the daemon session is adopted", async () => {
const terminalId = `e2e-notice-adopt-${randomUUID().slice(0, 8)}`;
const first = await createTerminalSessionInternal({
terminalId,
workspaceId,
db,
listed: true,
});
assert.ok(!("error" in first));

__resetSessionsForTesting();
await disposeDaemonClient();

const second = await createTerminalSessionInternal({
terminalId,
workspaceId,
db,
listed: true,
restoredNotice: true,
});
assert.ok(!("error" in second));
if ("error" in second) return;

const capture = makeCaptureSocket();
replayBuffer(second, capture.socket);
assert.doesNotMatch(
capture.received(),
/Session Contents Restored/,
"adopted (still-live) session should not get the restored separator",
);

await disposeSessionAndWait(terminalId, db);
});

test("adopted session keeps listed/exited bookkeeping", async () => {
const terminalId = `e2e-bookkeeping-${randomUUID().slice(0, 8)}`;
const first = await createTerminalSessionInternal({
Expand Down Expand Up @@ -733,7 +834,21 @@ async function waitForOutput(
}

function sessionBufferText(session: { buffer: Uint8Array[] }): string {
return Buffer.concat(session.buffer.map((b) => Buffer.from(b))).toString(
"utf8",
);
return Buffer.concat(session.buffer).toString("utf8");
}

function makeCaptureSocket() {
const chunks: Uint8Array[] = [];
return {
socket: {
send: (data: string | Uint8Array) => {
chunks.push(
typeof data === "string" ? Buffer.from(data, "utf8") : data,
);
},
close: () => {},
readyState: 1, // SOCKET_OPEN
},
received: () => Buffer.concat(chunks).toString("utf8"),
};
}
47 changes: 40 additions & 7 deletions packages/host-service/src/terminal/terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ type TerminalServerMessage =
| { type: "title"; title: string | null };

const MAX_BUFFER_BYTES = 64 * 1024;
// Dim separator delivered ahead of a respawned shell's output so users can
// tell restored scrollback from the fresh session (cf. VS Code's "History
// restored" line).
const SESSION_RESTORED_NOTICE = new TextEncoder().encode(
"\r\n\x1b[90m─── Session Contents Restored ───\x1b[0m\r\n\r\n",
);
// Cap on a single renderer socket's unflushed WebSocket send buffer. With no
// ACK flow control, a renderer that stops draining (slow paint, pinned main
// thread, dead tab) would let this buffer grow without bound → host OOM (the
Expand Down Expand Up @@ -219,6 +225,12 @@ interface TerminalSession {
*/
buffer: Uint8Array[];
bufferBytes: number;
/**
* Deliver SESSION_RESTORED_NOTICE ahead of the next replay. Kept out of
* the FIFO so MAX_BUFFER_BYTES eviction can't drop it before a client
* attaches. Cleared on first replay.
*/
restoredNoticePending: boolean;
createdAt: number;
exited: boolean;
exitCode: number;
Expand Down Expand Up @@ -538,27 +550,38 @@ function broadcastBytes(session: TerminalSession, bytes: Uint8Array): number {
return sent;
}

function replayBuffer(session: TerminalSession, socket: TerminalSocket) {
// Preamble first, then FIFO. Mode-setting escapes (kitty keyboard,
// bracketed paste, focus, …) are typically emitted once at startup and
// broadcast away rather than buffered, so a fresh xterm needs them
// re-asserted on every attach — even when the FIFO is empty.
export function replayBuffer(session: TerminalSession, socket: TerminalSocket) {
// sendBytes below no-ops on a non-open socket — bail before clearing the
// buffer/notice so the next attach can still replay them.
if (socket.readyState !== SOCKET_OPEN) return;
// Preamble first, then the restored notice, then FIFO. Mode-setting
// escapes (kitty keyboard, bracketed paste, focus, …) are typically
// emitted once at startup and broadcast away rather than buffered, so a
// fresh xterm needs them re-asserted on every attach — even when the
// FIFO is empty.
const preamble = session.modeTracker.buildPreamble();
const notice = session.restoredNoticePending ? SESSION_RESTORED_NOTICE : null;
let bufferTotal = 0;
for (const b of session.buffer) bufferTotal += b.byteLength;
const preambleLen = preamble?.byteLength ?? 0;
if (preambleLen === 0 && bufferTotal === 0) return;
const noticeLen = notice?.byteLength ?? 0;
if (preambleLen === 0 && noticeLen === 0 && bufferTotal === 0) return;

const combined = new Uint8Array(preambleLen + bufferTotal);
const combined = new Uint8Array(preambleLen + noticeLen + bufferTotal);
let offset = 0;
if (preamble) {
combined.set(preamble, offset);
offset += preamble.byteLength;
}
if (notice) {
combined.set(notice, offset);
offset += notice.byteLength;
}
for (const b of session.buffer) {
combined.set(b, offset);
offset += b.byteLength;
}
session.restoredNoticePending = false;
session.buffer.length = 0;
session.bufferBytes = 0;
sendBytes(socket, combined);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
Expand Down Expand Up @@ -820,6 +843,12 @@ interface CreateTerminalSessionOptions {
* the WS-down window are dropped (sub-second on a daemon swap).
*/
replayOnAdoption?: boolean;
/**
* Deliver a "session restored" separator ahead of the first replay. Set on
* the cold-restore respawn path, where the renderer paints stale scrollback
* above a brand-new shell.
*/
restoredNotice?: boolean;
}

function resolveTerminalCwd(
Expand Down Expand Up @@ -867,6 +896,7 @@ export async function createTerminalSessionInternal({
rows: requestedRows,
adoptOnly = false,
replayOnAdoption = true,
restoredNotice = false,
}: CreateTerminalSessionOptions): Promise<TerminalSession | { error: string }> {
const existing = sessions.get(terminalId);
if (existing) {
Expand Down Expand Up @@ -1051,6 +1081,8 @@ export async function createTerminalSessionInternal({
sockets: new Set(),
buffer: [],
bufferBytes: 0,
// Adopted sessions kept a live shell — nothing was restored.
restoredNoticePending: restoredNotice && !isAdopted,
createdAt,
exited: false,
exitCode: 0,
Expand Down Expand Up @@ -1352,6 +1384,7 @@ export function registerWorkspaceTerminalRoute({
themeType,
db,
eventBus,
restoredNotice: true,
});
};

Expand Down
Loading