Skip to content

fix(terminal): a mounted terminal keeps its live output across a same-id shell respawn - #1333

Merged
Juliusolsson05 merged 3 commits into
mainfrom
fix/terminal-attach-survives-respawn
Sep 27, 2026
Merged

Juliusolsson05 merged 3 commits into
mainfrom
fix/terminal-attach-survives-respawn

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Fixes #1281 (the terminal half; the agent half shipped in 2ab60d45). Refs #1283: this closes item 3, "attach counts not tied to a page", for plain terminals. #1311 (6cf6f054) already did it for agent PTYs. Items 1 (orchestration tombstones) and 2 (Codex window lease) remain open there.

Plan: docs/plans/2026-09-26-terminal-attach-survives-respawn.md (first commit).

Problem

A plain terminal's live output depended on a manager flag, terminalAttached. cleanupSessionState deleted it when the shell exited.

  • The flag didn't survive a respawn. TerminalLeaf attaches only once per session id. A shell respawned under the same id (recover, from a delivery, a control call or a wake) buffered its bytes but never forwarded them. The xterm stayed on the dead shell while keystrokes reached the new one unseen, until a remount.
  • Nothing could release the flag. There was no terminal detach at all, and the IPC handler ignored which renderer page attached, so a reloaded or crashed renderer could never give a reference back.

What merges

  • SessionManager: terminalAttachCounts replaces the flag. It is a per-view count, because two lanes can show one shell. Process cleanup leaves it alone, the same rule the agent PTY count follows since Same-id wake of a mounted terminal-view agent leaves the xterm frozen on the dead process #1281's agent half. The new detachTerminal releases one reference.
    • attachTerminal returns null, and takes no reference, only for a non-terminal id.
    • A shell that is down still takes a reference, so its respawn forwards to the mounted leaf.
  • IPC: the per-page ownership table that fix(sessions): a raw PTY view keeps receiving output across a same-id wake (#1281) #1311 wrote for agent PTYs becomes one factory, rawViewOwnership, used for both agent PTY and terminal views, with one shared page-document map.
    • Terminal attach and detach carry the page document.
    • A reload or destroy releases the page's terminal references.
    • A dead page's late detach is ignored, and its late attach takes nothing.
  • Preload: attachTerminal now sends the document; a new detachTerminal does the same.
  • TerminalLeaf detaches in its effect cleanup. An attach that resolves after unmount releases itself.

Tests

  • sessionManager.terminalAttachRespawn.test.ts (new, 4 tests, drives the real manager with a fake PTY; the manager is imported once at module scope):
    • a still-attached view keeps forwarding across exit and a same-id respawn, and its detach ends it;
    • a detach while the shell is down is remembered;
    • a view that attached while the shell was down gets the respawn's output;
    • two views in one renderer, one detached, still forward. Two windows cannot both receive one shell's bytes, because the router gives a session one owner.
  • ipc/session.test.ts, "terminal attach ownership": reload and destroy release a page's references; a stale page's detach is ignored and its attach takes nothing; a non-terminal attach takes no reference; one detach passes through per attach.
  • TerminalLeaf.retention.renderer.test.tsx: unmount releases; a late attach releases itself.
  • Red checks: all new tests are red on origin/main.
  • Gates: npx tsc -b is clean. Full suite (Node 24): 7247 passed, 1 skipped, 1 failed. The failure is control.system.test.ts, which times out the same way on origin/main.

Reviews: one round (a, b, c) plus a and b's verification, all MERGE-READY; the disposition is in the PR comments.

🤖 Generated with Claude Code

Juliusolsson05 and others added 2 commits September 26, 2026 18:05
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-id shell respawn

The shell's view reference was a manager flag cleared when the shell exited,
so a still-mounted leaf froze on the dead shell after a same-id respawn. It
is now a view-scoped count with a detachTerminal release, owned per renderer
page like agent PTY attaches (one shared ownership table), and TerminalLeaf
detaches on unmount.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05 Juliusolsson05 added type:bug Something works wrong class:C2-identity Identity/ownership across reload, replace, restore, wake sev:P2 Real bug with a workaround labels Sep 27, 2026
…anager once

Round-1 review of #1333: the absent-session reference had no test, and a
cold per-test import of the manager hit the default timeout under load.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review disposition (round 1 + verification, head cb76010c)

Finding Verdict Disposition
a, b, c: the attach-while-shell-down path (a missing session takes a reference) had no test; removing it survived valid Test added in cb76010c: attach after exit, then a same-id respawn forwards. It kills the mutant. Verified by a and b
a: the new manager test hit the default 5 s timeout on a cold, contended run valid Root cause: the manager graph was imported inside the first test (~4 s). It is now imported once at module scope, and the tests take ms. Verified by a
c: the "second window's lane" test comment overclaims, because the window router gives a session one owner valid Comments corrected in the test and in sessionManager.ts: the count is about views within the receiving renderer
a (suspicion): an attach whose IPC reply is lost while the page lives could leave a reference not reproduced A page reload or destroy still releases it. No Electron path was found that loses only the reply

Verification: a and b are MERGE-READY at cb76010c; c was MERGE-READY in round 1.

@Juliusolsson05
Juliusolsson05 merged commit 509e317 into main Sep 27, 2026
2 checks passed
Juliusolsson05 added a commit that referenced this pull request Sep 27, 2026
@Juliusolsson05
Juliusolsson05 deleted the fix/terminal-attach-survives-respawn branch September 27, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

class:C2-identity Identity/ownership across reload, replace, restore, wake sev:P2 Real bug with a workaround type:bug Something works wrong

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Same-id wake of a mounted terminal-view agent leaves the xterm frozen on the dead process

1 participant