vscode: builder cleanup no longer closes the terminal tab — regression of 3.0.2 behavior

[amrmelsayed]

Symptom

When a builder is cleaned up (via afx cleanup, the VSCode "Cleanup Builder" command, or any other removal path), the builder's terminal tab in VSCode stays open as a dead "Process exited" entry. User has to close it manually.

Past behavior (expected)

This worked correctly in 3.0.2. From packages/vscode/CHANGELOG.md (3.0.2 bug fixes):

"Builder terminal tabs close automatically on cleanup instead of lingering as dead 'Process exited' tabs."

So the closing-on-cleanup is an explicit shipped feature, not an emergent property. It has regressed.

Reproduce

1. Spawn a builder (any protocol): afx spawn <issue> --protocol bugfix
2. Open the builder's terminal in VSCode (single-click the row in the Builders view, or use Codev: Open Builder Terminal)
3. Wait for the builder to do its work and the PR to merge
4. Clean up the builder via any of:
   • afx cleanup -p <id> from a CLI
   • Right-click builder row → Codev: Cleanup Builder (per vscode: sidebar UX polish — reduce Builders menu density, add Cleanup to row menu, rename worktree-open commands #795 — if landed; or via palette)
5. Observe: VSCode's terminal tab stays open with Process exited instead of disappearing

Code path the regression broke

The auto-close mechanism is two-piece:

Piece 1 — Present→absent diff in extension.ts:199-234

let prevBuilderIds: Set<string> | null = null;
let pruneInFlight = false;
const pruneClosedBuilderTerminals = async () => {
  if (pruneInFlight) { return; }
  if (connectionManager?.getState() !== 'connected') { return; }
  // ...
  const state = await client.getWorkspaceState(workspacePath);
  if (!state?.builders) { return; }
  const currIds = new Set(state.builders.map(b => b.id));
  if (prevBuilderIds !== null) {
    for (const prev of prevBuilderIds) {
      if (!currIds.has(prev)) {
        terminalManager?.closeBuilderTerminal(prev);
      }
    }
  }
  prevBuilderIds = currIds;
  // ...
};

Pre-conditions for this to fire on cleanup:

• pruneClosedBuilderTerminals is wired to a trigger (overview tick, SSE event, etc.)
• connectionManager.getState() === 'connected'
• A prior tick populated prevBuilderIds with the soon-to-be-cleaned-up builder
• A subsequent tick fetches state where that builder is absent from state.builders

Piece 2 — Disposal in terminal-manager.ts:262

closeBuilderTerminal(builderId: string): void {
  for (const key of [`builder-${builderId}`, `dev-${builderId}`]) {
    const existing = this.terminals.get(key);
    if (!existing) { continue; }
    existing.pty.close();
    existing.terminal.dispose();
    this.terminals.delete(key);
  }
}

Plausible regression causes (for plan-phase investigation)

The PIR builder should investigate which of these is happening — the symptom is the same, but the fix differs per cause:

1. pruneClosedBuilderTerminals is no longer wired to a trigger — maybe a refactor of the overview cache subscription dropped the call. Check overviewCache.onDidChange(...) registrations in extension.ts.
2. Tower's state.builders is no longer dropping the cleaned-up builder — could be a Tower-side regression where afx cleanup succeeds but the DB row stays. Symptom-side, the diff in step 1 never sees absence. Particularly worth investigating given the recent Tower DB migration issues (the orphan builders_new table that wedged afx cleanup during 2026-05-26 sessions). If afx cleanup is silently failing to remove the row, the diff has nothing to detect.
3. prevBuilderIds initialization race — if cleanup happens before the first successful state fetch, prevBuilderIds is null and the diff is skipped entirely. The freshly-cleaned builder gets recorded in currIds (wait — actually if it's already gone, it wouldn't be in currIds either; needs careful trace). May need a sentinel or different initialization to handle this edge.
4. The pruneInFlight guard wedged — if a state fetch hangs and never sets pruneInFlight = false, subsequent ticks no-op forever. Add a timeout or finally-only release.
5. closeBuilderTerminal finds nothing in the map — if the key encoding has changed (e.g. builder ID format drift between when openBuilder registers and when closeBuilderTerminal looks up), the iteration silently skips both keys.
6. existing.terminal.dispose() no longer kills the VSCode tab — VSCode API regression, unlikely but worth verifying with a manual dispose call.

The plan phase should identify which one (or which combination) is the real cause via reproduction with logging, BEFORE writing the fix.

Why PIR

Per the protocol-selection guide:

• UI / UX change requiring visual confirmation in running VSCode — the symptom is observable only in a live VSCode session. Unit tests on the diff logic won't catch a real regression in terminal-tab disposal.
• Root cause is ambiguous — 6 plausible causes listed above. Plan-approval is the right place to lock in the diagnosis before coding.
• High-blast-radius area — terminal lifecycle touches a frequently-used surface (every builder spawn opens a terminal; every cleanup needs to close one). Fix needs to be confident, not a guess.

Acceptance criteria

• Plan phase identifies the actual root cause (or causes) with reproduction logs — not just a hypothesis
• Fix applied at the identified failure point(s)
• Reproduce the symptom with the fix in place — terminal tab closes within ~5 seconds of afx cleanup completing
• Visual verification in a running VSCode session at dev-approval gate (open VSCode, spawn a builder, open terminal, run afx cleanup -p <id>, confirm tab disappears)
• Both builder-<id> and dev-<id> keys disposed (the existing loop handles both — verify the dev-terminal path too)
• No regression: spawning a fresh builder after a cleanup still opens its terminal correctly (i.e. the cleanup didn't break the openBuilder map state)
• Regression test if the failure is unit-testable (e.g. mock getWorkspaceState to return a builder absent-after-present and assert closeBuilderTerminal is called)

Out of scope

• Auto-cleaning ghost terminal_sessions in ~/.agent-farm/global.db — separate concern; that's Tower-side bookkeeping, not VSCode-side terminal tabs
• Closing terminal tabs for cleaned-up architects — different mechanism, different code path
• Closing dev terminals on PR merge or other lifecycle events — only afx cleanup (= builder removed from Tower state) is the trigger this issue addresses

References

• packages/vscode/CHANGELOG.md — 3.0.2 entry that documents the original feature
• packages/vscode/src/extension.ts:199-234 — present→absent diff (pruneClosedBuilderTerminals)
• packages/vscode/src/terminal-manager.ts:262 — the actual closeBuilderTerminal disposal logic
• packages/vscode/src/commands/cleanup.ts:50 — VSCode side's Codev: Cleanup Builder command that shells to afx cleanup

Discovered while

Observed during 2026-05-27 batch operations — cleaning up merged bugfix builders left their terminal tabs lingering in VSCode. User confirmed this was working in earlier versions.

[amrmelsayed]

On it! Working on this with the PIR protocol (plan + dev-approval gates before PR).