Skip to content

fix(web): skip close confirmation for idle terminals - #9728

Open
nobottomline wants to merge 3 commits into
pingdotgg:mainfrom
nobottomline:fix/skip-idle-terminal-close-confirmation
Open

fix(web): skip close confirmation for idle terminals#9728
nobottomline wants to merge 3 commits into
pingdotgg:mainfrom
nobottomline:fix/skip-idle-terminal-close-confirmation

Conversation

@nobottomline

@nobottomline nobottomline commented Sep 4, 2026

Copy link
Copy Markdown

What Changed

Terminal close actions now skip the destructive confirmation when every targeted terminal is known to be idle. Immediately before deciding, the client asks the server for a fresh process-table snapshot; terminals with a running subprocess still prompt, and missing or failed inspection state is treated conservatively and still prompts.

The existing behavior remains unchanged for auto-exit cleanup, bulk closes, and clients without the desktop dialog API.

Why

The confirmation added in #7592 protects running work and terminal history, but it also interrupts closing a fresh terminal or one whose command has already exited. The terminal manager already owns subprocess inspection, so the close flow can preserve the safeguard only when work may actually be interrupted without adding a setting or trusting the one-second background polling cache.

UI Changes

Verified in an isolated desktop profile:

  • a fresh idle terminal closes immediately from the trash action
  • a terminal running sleep 120 still shows Close terminal "sleep"?
  • the same shared close flow covers the terminal drawer, right panel, terminal surface, and Command/Ctrl+W
Idle terminal After idle close
Idle terminal before close Idle terminal closed without a confirmation

Running-process safeguard:

Running terminal still requires confirmation

Verification

  • vp test run src/terminal/Manager.test.ts (58 tests)
  • vp test run src/auth/RpcAuthorization.test.ts (6 tests)
  • vp test run src/rpc.test.ts src/terminal.test.ts (25 tests)
  • vp test run src/lib/terminalCloseConfirm.test.ts src/components/ThreadTerminalDrawer.test.ts (11 tests)
  • vp run typecheck in apps/server, apps/web, packages/contracts, and packages/client-runtime
  • targeted vp lint on the 12 changed files (no errors; existing React warnings remain outside the changed blocks)
  • manual desktop pass with an isolated --home-dir: idle close bypassed the dialog; running close retained it

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • No animation behavior changed

Built with gpt-5.6-sol through the Codex harness in T3 Code.

Note

Skip terminal close confirmation for idle terminals via subprocess inspection

  • Adds the terminalInspectSubprocesses WebSocket RPC, with an input capped at 64 terminal IDs and a per-terminal activity result of true, false, or null (unavailable).
  • Implements TerminalManager.inspectSubprocesses in Manager.ts under the existing per-thread lock. It deduplicates requested IDs, returns false for missing/non-running sessions, and returns null when a running session cannot be inspected.
  • Updates terminal close flows in ChatView and ThreadTerminalDrawer to inspect subprocess activity before closing. Known-idle terminals skip the destructive confirmation dialog; running, missing, or unknown activity still prompts.
  • Authorization requires the orchestration operate scope for the new RPC.
  • Risk: confirmTerminalClose and confirmInspectedTerminalClose in terminalCloseConfirm.ts now require labeled target objects instead of a label-only array; existing callers must be migrated.

Macroscope summarized ef2f694.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 4, 2026
Comment thread apps/web/src/components/ChatView.tsx Outdated
@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes the existing terminal-close safety flow and adds a cross-layer subprocess-inspection RPC. Human review is required because it modifies the server auth package and introduces non-opt-in runtime behavior changes across the client and server.

You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Sep 4, 2026
@nobottomline
nobottomline force-pushed the fix/skip-idle-terminal-close-confirmation branch from 04ff253 to 22ff7b9 Compare September 6, 2026 00:04
Comment thread packages/contracts/src/terminal.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant