Skip to content

vscode: builder terminal unopenable after move-between-groups + close (silent failure, requires VS Code restart) #803

@amrmelsayed

Description

@amrmelsayed

Symptom

Intermittent, hard-to-replicate failure mode in the Codev VS Code extension:

  1. Open a builder terminal (sidebar row click, command palette, or any path).
  2. Move the terminal between editor groups (the default spawn is the right / second group; drag it to the left or anywhere else).
  3. Close that terminal.
  4. Try to re-open the same builder's terminal — via sidebar row, command palette, or otherwise.
  5. Nothing happens. No new tab, no toast, no error in the Codev OutputChannel, no console error.
  6. The only known recovery: fully close and reopen VS Code. After restart, the same builder opens cleanly.

The pattern points strongly at terminal-move-between-groups: when the user opens, closes, and re-opens without the intermediate move, the second open works as expected. The failure only manifests when the move happens before the close.

Hypothesis

Likely a stale-reference / stale-PTY-session bug in terminalManager.

Suspect code path (packages/vscode/src/terminal-manager.ts):

  • Spawn (line 342): this.terminals.set(mapKey, { terminal, pty, type, id: terminalId }) stores a reference to the vscode.Terminal instance.
  • Per-terminal close listener (lines 348–354):
    const disposable = vscode.window.onDidCloseTerminal((t) => {
      if (t !== terminal) return;
      if (this.terminals.get(mapKey)?.terminal === terminal) {
        this.terminals.delete(mapKey);
      }
      disposable.dispose();
    });
  • Re-open path (openBuilder, lines 135–148):
    const existing = this.terminals.get(key);
    if (existing) {
      if (existing.id === terminalId) { existing.terminal.show(!focus); return; }
      existing.pty.close();
      existing.terminal.dispose();
      this.terminals.delete(key);
    }
    await this.openTerminal(terminalId, 'builder', ..., key, focus);

Two viable failure modes to investigate:

(a) Stale terminal reference in the map

VS Code's "move terminal to another group" may keep the same vscode.Terminal object but mutate its parent group. After the user closes the moved terminal, onDidCloseTerminal fires; the guard at line 351 (this.terminals.get(mapKey)?.terminal === terminal) holds, the map is cleared. Next openBuilder falls through to openTerminal(), which tries to connect to the Tower PTY using the stored terminalId — but Tower may have invalidated that PTY when the original closed, so the WS handshake fails silently inside terminal-adapter.ts / sse-client.ts without surfacing to the user. Result: no new terminal renders, no error logged.

(b) Listener never fires

Alternatively: moving the terminal between groups may not trigger onDidCloseTerminal in the way the code assumes, leaving stale entries. The next openBuilder finds existing present (terminals.get(key) succeeds), sees existing.id === terminalId, calls existing.terminal.show(!focus) on a disposed terminal handle, which silently no-ops.

The "VS Code restart fixes it" detail is consistent with both: a restart blows away the in-memory map and forces a fresh openTerminal / Tower PTY handshake.

Investigation tasks

  • Add temporary debug logging at every entry/exit of openBuilder, around the onDidCloseTerminal callback, and at the WS-connection setup in openTerminal / terminal-adapter.ts. Print: mapKey, terminalId, existing?.id, whether the terminal was previously moved (test terminal.location), and the WS-connection outcome.
  • Reproduce manually with the move-then-close sequence and capture the logs to determine which of (a) or (b) is in play.
  • If (a): the Tower-side PTY's lifecycle relative to the VS Code terminal needs verification. Does Tower retain the PTY across onDidCloseTerminal? If not, on re-open we need to spawn a fresh PTY (new terminalId) rather than reusing the old one. Plumb the fresh-terminalId lookup through openBuilderByRoleOrId().
  • If (b): the onDidCloseTerminal guard at line 351 may be missing a sibling event (e.g. onDidChangeTerminalState, or detecting terminal.exitStatus). Track when a terminal moves to a different terminal.location and refresh the listener / clean state proactively.
  • Confirm whether the disposable at line 354 (disposable.dispose()) is actually firing when the user moves a terminal — if VS Code emits onDidCloseTerminal for the move itself, we may be tearing down the listener before the real close.

Acceptance criteria

  • Root cause identified (which of (a), (b), or some third path).
  • Reproduce with a deterministic script (even if it requires a specific VS Code action sequence).
  • Fix lands: after move + close, the builder terminal re-opens on next request without requiring a VS Code restart.
  • A unit or integration test covers the regression.
  • No regression to the existing close-and-reopen-without-move path.

Workaround (current)

Restart VS Code (Cmd+Shift+P → "Developer: Reload Window" is faster than a full quit).

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions