You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR replaces the useless split "Changes / Terminal placeholder" panels in the right sidebar with a functional full-height TaskTerminalPanel on both the project page (no task selected) and the home page, and fixes the PTY fallback CWD from process.cwd() to os.homedir(). The changes are mostly solid, but two issues need addressing:
Dead code in RightSidebar.tsx: useAppSettings is imported and const { settings } = useAppSettings() is called (line 48), but settings is never used anywhere in the component. This is a leftover from an earlier iteration where settings-based fallback was planned; it should be removed to avoid an unnecessary React Query subscription.
Invisible task-placeholder PTY on the home page: Removing the if (!task && !projectPath) early-return guard means TaskTerminalPanel now renders its full terminal tree on the home page. taskKey evaluates to 'task-placeholder' when task is null, and useTaskTerminals eagerly creates a default terminal entry. The resulting TerminalPane (with keepAlive) is mounted but invisible — spawning a PTY in os.homedir() that the user can never access. This is a resource leak even if not user-visible.
Confidence Score: 3/5
Safe to merge after cleaning up dead code and addressing the invisible PTY leak on the home page.
The ptyManager.ts fix is correct and minimal. The RightSidebar.tsx cleanup is straightforward. However, the removal of the early-return guard in TaskTerminalPanel.tsx introduces a new resource leak (invisible PTY on home page) that should be fixed before merging. The core feature works (stable home key, correct scope labels, full-height terminal), but the unintended side effect needs attention.
src/renderer/components/TaskTerminalPanel.tsx (invisible PTY regression) and src/renderer/components/RightSidebar.tsx (unused settings import).
Comments Outside Diff (1)
General comment
Invisible task-placeholder PTY now spawns unconditionally on the home page
taskKey is 'task-placeholder' whenever task is null, and useTaskTerminals('task-placeholder', undefined) creates a default terminal state (including a PTY-backed TerminalPane). Before this PR, the early-return guard if (!task && !projectPath) prevented the component from reaching this code on the home page. That guard has now been removed, so every home-page render mounts an invisible TerminalPane with keepAlive under the 'task-placeholder' key — spawning a PTY in os.homedir() that is never shown to the user (the Worktree SelectGroup only renders when task is truthy).
Consider keeping the early return for the fully no-task/no-project case, or alternatively delaying useTaskTerminals initialization to only when task is non-null:
This way the 'task-placeholder' state is never eagerly created when there is no task.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary:
changes: