Skip to content

feat(terminal): register PTY processes for lifecycle cleanup#906

Closed
luoye520ww wants to merge 3 commits into
KunAgent:developfrom
luoye520ww:codex/process-registry-terminal
Closed

feat(terminal): register PTY processes for lifecycle cleanup#906
luoye520ww wants to merge 3 commits into
KunAgent:developfrom
luoye520ww:codex/process-registry-terminal

Conversation

@luoye520ww

Copy link
Copy Markdown
Collaborator

Problem

The built-in terminal owns real PTY child processes, but those PIDs were only held inside the terminal session map. Shared shutdown, cancellation, and diagnostics could not see the live process lifecycle.

Root cause

Terminal session state and OS child ownership were not connected to the managed-child registry.

Scope

This PR wires only the built-in terminal PTY path to the registry. LSP, Whisper, extension-host, and subagent integrations remain separate follow-ups.

Changes

  • Register each PTY after spawn with a namespaced terminal:<sessionId> identity.
  • Remove the record on PTY exit and all existing dispose paths, including renderer destruction and app shutdown.
  • If registration fails, kill the newly spawned PTY before returning the error.
  • Add a small lifecycle helper and tests that exercise registration and removal against a real registry instance.

Safety

  • No command, environment, token, path, or output is added to the registry record.
  • Registry cleanup is bookkeeping only; the existing terminal dispose path remains responsible for killing the PTY.
  • Existing terminal IPC payloads and replay behavior are unchanged.

Typecheck

npm.cmd run typecheck

Passed.

Tests

npm.cmd run build:extensions
npm.cmd exec vitest run src/main/terminal/terminal-pty-ipc.test.ts
npm.cmd run lint
npm.cmd run build
git diff --check

Passed: 3 terminal lifecycle tests; root lint and build passed; diff check passed. The first test attempt before building extension workspaces was an environment resolution failure; the same test passed after the required extension build.

Review performed

  • Functional PTY lifecycle
  • Cleanup and stale process handling
  • Failure rollback when registration fails
  • Data and secret safety
  • Cross-platform behavior
  • Test quality and PR scope

Non-goals

  • LSP, Whisper, extension-host, or subagent process registration
  • Persisting records across application restarts
  • Changing terminal UI or IPC payloads

Dependency

Depends on #905 for the managed child process registry. After #905 merges, rebase this branch onto develop and drop dependency commits.

@luoye520ww

Copy link
Copy Markdown
Collaborator Author

Superseded by consolidated tool/process lifecycle PR #946, which passed Typecheck, Linux, macOS, and Windows packaged checks. Closing this duplicate to keep the review surface focused.

@luoye520ww luoye520ww closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant