-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: persist chat history in Remote SSH sessions #8029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add remote environment detection utility to identify SSH, WSL, Dev Containers, etc. - Modify storage paths to include remote-specific subdirectories - Add configuration option "persistChatInRemote" to control the feature - Ensure chat history is saved per remote workspace and restored on reconnection - Add comprehensive tests for remote storage functionality Fixes #8028
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| const remoteId = getRemoteWorkspaceId() | ||
| if (remoteId) { | ||
| // Use path.join for proper path construction | ||
| const path = require("path") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? Using require("path") dynamically inside a function could cause bundling issues with esbuild or webpack. Consider importing path at the top of the file instead:
| const path = require("path") | |
| import * as path from "path" |
Then you can remove line 76 and the dynamic require.
| /** | ||
| * Simple string hash function for creating stable identifiers | ||
| */ | ||
| function hashString(str: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we improve the hash function to reduce collision risk? The current implementation might produce the same hash for different URIs, potentially mixing chat histories from different workspaces. Consider using a more robust hashing approach or including more entropy:
| function hashString(str: string): string { | |
| function hashString(str: string): string { | |
| let hash = 5381 | |
| for (let i = 0; i < str.length; i++) { | |
| const char = str.charCodeAt(i) | |
| hash = ((hash << 5) + hash) + char // hash * 33 + char | |
| } | |
| return Math.abs(hash).toString(36).padStart(8, '0') | |
| } |
| } | ||
|
|
||
| // Remote environment - add a remote-specific subdirectory | ||
| const remoteId = getRemoteWorkspaceId() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add error handling here? If getRemoteWorkspaceId() throws an error or returns something unexpected, we're silently falling back to the base path. Consider adding a try-catch with logging to help debug issues:
| const remoteId = getRemoteWorkspaceId() | |
| const remoteId = getRemoteWorkspaceId() | |
| if (remoteId) { | |
| try { | |
| // Use path.join for proper path construction | |
| const path = require("path") | |
| return path.join(basePath, "remote", remoteId) | |
| } catch (error) { | |
| console.warn() | |
| } | |
| } |
| ] | ||
| const result = getEnvironmentStoragePath(windowsBasePath) | ||
| // The path.join will use forward slashes even on Windows in Node.js context | ||
| expect(result).toMatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the mixed path separator intentional here? The comment mentions forward slashes but the regex expects a mix of forward and backward slashes. This could cause test failures on Windows. Consider normalizing the path or adjusting the regex to handle both separators consistently.
| await fs.access(customStoragePath, fsConstants.R_OK | fsConstants.W_OK | fsConstants.X_OK) | ||
| // Apply remote environment path adjustment if enabled | ||
| // This will add a remote-specific subdirectory if in a remote environment | ||
| const finalPath = persistChatInRemote ? getEnvironmentStoragePath(basePath) : basePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider adding migration logic for existing remote users? They might already have chat history stored in the default location that would become inaccessible after this change. We could check for existing data and offer to migrate it to the new location.
|
This doesn’t seem to be quite there yet. The PR title suggests “Persist chat history in Remote SSH sessions”, but what it’s really doing looks closer to an automatic setup of the The catch is that all the filesystem operations ( So while this does help organize things locally (by creating subdirectories like Unless VS Code exposes the remote filesystem directly to the extension (or the extension implements its own sync strategy), this won’t achieve true persistence on the remote. Right now it feels more like “local organization for remote sessions” than actual “remote persistence.” |
Description
This PR implements persistent chat history for Remote SSH sessions, addressing the issue where chat history disappears after closing VS Code when using Roo Code over VS Code Remote SSH.
Problem
When using Roo Code over VS Code Remote SSH, chat history disappears after closing VS Code. In local sessions, past conversations are available, but in SSH sessions they are not. This breaks continuity and forces users to re-explain context.
Solution
persistChatInRemotesetting (default: true) to control the featureImplementation Details
vscode.env.remoteNameAPI for reliable remote detection/remote/{workspace-id}subdirectory for remote sessionsTesting
Configuration
New setting added:
roo-cline.persistChatInRemote(boolean, default: true) - Enable/disable chat persistence in remote sessionsFixes #8028
Important
Adds persistent chat history for Remote SSH sessions with remote environment detection and configurable storage paths.
getEnvironmentStoragePath()inremoteEnvironment.ts.persistChatInRemotesetting inpackage.jsonto enable/disable this feature (default: true).isRemoteEnvironment(),getRemoteType(), andgetRemoteWorkspaceId()inremoteEnvironment.tsto detect remote environments and generate workspace IDs.vscode.env.remoteNamefor environment detection.getStorageBasePath()instorage.tsto adjust paths for remote environments usinggetEnvironmentStoragePath().remoteEnvironment.spec.tsto test remote environment utilities.storage.spec.tsto test storage path logic with remote settings.package.nls.jsonfor new setting descriptions.This description was created by
for e319f8b. You can customize this summary. It will automatically update as commits are pushed.