Skip to content

fix(tui): resume sessions in current terminal (#222)#233

Open
feiyehua wants to merge 1 commit into
SaladDay:mainfrom
feiyehua:feiyehua/session-resume-fix
Open

fix(tui): resume sessions in current terminal (#222)#233
feiyehua wants to merge 1 commit into
SaladDay:mainfrom
feiyehua:feiyehua/session-resume-fix

Conversation

@feiyehua
Copy link
Copy Markdown
Collaborator

@feiyehua feiyehua commented Jun 4, 2026

Replace the external terminal launch approach for session resume with direct exec in the current terminal.

Previously, resuming a session would attempt to open an external terminal application (Terminal.app, iTerm2, Ghostty, etc.) via platform-specific launchers.
Now, session resume uses exec (on Unix) to replace the current process with the resume command in the user's shell, giving a seamless in-terminal experience. On non-Unix platforms, the resume command is shown as a fallback for manual execution.

Changes

  • Remove session_manager::terminal module — deletes all platform-specific terminal launchers (macOS Terminal, iTerm2, Ghostty, Kitty, WezTerm, Kaku, Alacritty, Warp, custom)
  • uses CommandExt::exec() to replace the current process with the user's shell running the resume command
  • Deprecate preferred_terminal setting — field is retained for serde compatibility but marked #[allow(dead_code)]

I didn't add confirmation before launch agent apps to align with the behavior of existing "launch temp"

@SaladDay
Copy link
Copy Markdown
Owner

SaladDay commented Jun 4, 2026

Thanks for working on this. I do not think this PR fixes #222 yet.

The issue asks the Sessions restore path to reuse the existing Claude temp launch flow and pass --resume <UUID> as native Claude arguments. That temp launch path builds the effective provider snapshot, writes the temporary Claude settings file, and hands off as claude --settings <temp> ....

This PR removes the external terminal launcher, but Action::SessionResume still just executes the saved resume_command through the user shell ($SHELL -c). That means a Claude session restore can still run against whatever live Claude config is currently on disk, instead of the provider selected in cc-switch. In practice, this keeps the main bug class from #222: restoring a session may not use the current provider configuration.

I would not merge this as-is. The better fix is to make the session resume action provider-aware, carry provider_id, session_id, and project_dir, and for Claude reuse the existing temp launch implementation with native args like --resume <session_id> while setting the session cwd. The current-provider temp settings machinery should stay single-sourced there, rather than adding or preserving a generic shell-command resume path for Claude.

@feiyehua
Copy link
Copy Markdown
Collaborator Author

feiyehua commented Jun 4, 2026

Thanks for working on this. I do not think this PR fixes #222 yet.

The issue asks the Sessions restore path to reuse the existing Claude temp launch flow and pass --resume <UUID> as native Claude arguments. That temp launch path builds the effective provider snapshot, writes the temporary Claude settings file, and hands off as claude --settings <temp> ....

This PR removes the external terminal launcher, but Action::SessionResume still just executes the saved resume_command through the user shell ($SHELL -c). That means a Claude session restore can still run against whatever live Claude config is currently on disk, instead of the provider selected in cc-switch. In practice, this keeps the main bug class from #222: restoring a session may not use the current provider configuration.

I would not merge this as-is. The better fix is to make the session resume action provider-aware, carry provider_id, session_id, and project_dir, and for Claude reuse the existing temp launch implementation with native args like --resume <session_id> while setting the session cwd. The current-provider temp settings machinery should stay single-sourced there, rather than adding or preserving a generic shell-command resume path for Claude.

After carefully analyzing the original "launch temp" code, i realized this path is too complicated. Actually what needed to restore a session is only a session UUID, instead of creating a temp config file and create a shell script to delete it. So i think simply exec the CLI app suffice.
And in sessions page we can not select providers at all. Launching with currently selected config (in providers page) is the expected behavior.

The main problem in #222 is the current code launch a new terminal window, which is intuitive for an app already running in a terminal. This problem makes session resume hard to use on headless machines. This PR fixes this.

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.

2 participants