-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,234 @@ | ||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" | ||
| import * as vscode from "vscode" | ||
| import { | ||
| isRemoteEnvironment, | ||
| getRemoteType, | ||
| getRemoteWorkspaceId, | ||
| getEnvironmentStoragePath, | ||
| } from "../remoteEnvironment" | ||
|
|
||
| // Mock vscode module | ||
| vi.mock("vscode", () => ({ | ||
| env: { | ||
| remoteName: undefined, | ||
| }, | ||
| workspace: { | ||
| workspaceFolders: undefined, | ||
| }, | ||
| })) | ||
|
|
||
| describe("remoteEnvironment", () => { | ||
| let mockVscode: any | ||
|
|
||
| beforeEach(() => { | ||
| mockVscode = vi.mocked(vscode) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| // Reset mocks after each test | ||
| mockVscode.env.remoteName = undefined | ||
| mockVscode.workspace.workspaceFolders = undefined | ||
| }) | ||
|
|
||
| describe("isRemoteEnvironment", () => { | ||
| it("should return false when not in remote environment", () => { | ||
| mockVscode.env.remoteName = undefined | ||
| expect(isRemoteEnvironment()).toBe(false) | ||
| }) | ||
|
|
||
| it("should return true when in SSH remote environment", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| expect(isRemoteEnvironment()).toBe(true) | ||
| }) | ||
|
|
||
| it("should return true when in WSL environment", () => { | ||
| mockVscode.env.remoteName = "wsl" | ||
| expect(isRemoteEnvironment()).toBe(true) | ||
| }) | ||
|
|
||
| it("should return true when in dev container environment", () => { | ||
| mockVscode.env.remoteName = "dev-container" | ||
| expect(isRemoteEnvironment()).toBe(true) | ||
| }) | ||
|
|
||
| it("should return true when in codespaces environment", () => { | ||
| mockVscode.env.remoteName = "codespaces" | ||
| expect(isRemoteEnvironment()).toBe(true) | ||
| }) | ||
| }) | ||
|
|
||
| describe("getRemoteType", () => { | ||
| it("should return undefined when not in remote environment", () => { | ||
| mockVscode.env.remoteName = undefined | ||
| expect(getRemoteType()).toBeUndefined() | ||
| }) | ||
|
|
||
| it("should return 'ssh-remote' for SSH remote", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| expect(getRemoteType()).toBe("ssh-remote") | ||
| }) | ||
|
|
||
| it("should return 'wsl' for WSL", () => { | ||
| mockVscode.env.remoteName = "wsl" | ||
| expect(getRemoteType()).toBe("wsl") | ||
| }) | ||
| }) | ||
|
|
||
| describe("getRemoteWorkspaceId", () => { | ||
| it("should return undefined when not in remote environment", () => { | ||
| mockVscode.env.remoteName = undefined | ||
| expect(getRemoteWorkspaceId()).toBeUndefined() | ||
| }) | ||
|
|
||
| it("should return remote name when no workspace folders", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| mockVscode.workspace.workspaceFolders = undefined | ||
| expect(getRemoteWorkspaceId()).toBe("ssh-remote") | ||
| }) | ||
|
|
||
| it("should return remote name when workspace folders is empty", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| mockVscode.workspace.workspaceFolders = [] | ||
| expect(getRemoteWorkspaceId()).toBe("ssh-remote") | ||
| }) | ||
|
|
||
| it("should include workspace folder name and hash in ID", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "my-project", | ||
| uri: { | ||
| toString: () => "file:///home/user/projects/my-project", | ||
| }, | ||
| }, | ||
| ] | ||
| const id = getRemoteWorkspaceId() | ||
| expect(id).toMatch(/^ssh-remote-my-project-[a-z0-9]+$/) | ||
| }) | ||
|
|
||
| it("should create consistent hash for same workspace", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| const workspaceUri = "file:///home/user/projects/my-project" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "my-project", | ||
| uri: { | ||
| toString: () => workspaceUri, | ||
| }, | ||
| }, | ||
| ] | ||
| const id1 = getRemoteWorkspaceId() | ||
| const id2 = getRemoteWorkspaceId() | ||
| expect(id1).toBe(id2) | ||
| }) | ||
|
|
||
| it("should create different hashes for different workspaces", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
|
|
||
| // First workspace | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "project1", | ||
| uri: { | ||
| toString: () => "file:///home/user/projects/project1", | ||
| }, | ||
| }, | ||
| ] | ||
| const id1 = getRemoteWorkspaceId() | ||
|
|
||
| // Second workspace | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "project2", | ||
| uri: { | ||
| toString: () => "file:///home/user/projects/project2", | ||
| }, | ||
| }, | ||
| ] | ||
| const id2 = getRemoteWorkspaceId() | ||
|
|
||
| expect(id1).not.toBe(id2) | ||
| }) | ||
| }) | ||
|
|
||
| describe("getEnvironmentStoragePath", () => { | ||
| const basePath = "/home/user/.vscode/extensions/storage" | ||
|
|
||
| it("should return base path unchanged when not in remote environment", () => { | ||
| mockVscode.env.remoteName = undefined | ||
| expect(getEnvironmentStoragePath(basePath)).toBe(basePath) | ||
| }) | ||
|
|
||
| it("should add remote subdirectory for SSH remote", () => { | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "my-project", | ||
| uri: { | ||
| toString: () => "file:///home/user/projects/my-project", | ||
| }, | ||
| }, | ||
| ] | ||
| const result = getEnvironmentStoragePath(basePath) | ||
| expect(result).toMatch( | ||
| /^\/home\/user\/.vscode\/extensions\/storage\/remote\/ssh-remote-my-project-[a-z0-9]+$/, | ||
| ) | ||
| }) | ||
|
|
||
| it("should add remote subdirectory for WSL", () => { | ||
| mockVscode.env.remoteName = "wsl" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "linux-project", | ||
| uri: { | ||
| toString: () => "file:///home/user/linux-project", | ||
| }, | ||
| }, | ||
| ] | ||
| const result = getEnvironmentStoragePath(basePath) | ||
| expect(result).toMatch(/^\/home\/user\/.vscode\/extensions\/storage\/remote\/wsl-linux-project-[a-z0-9]+$/) | ||
| }) | ||
|
|
||
| it("should add remote subdirectory for dev containers", () => { | ||
| mockVscode.env.remoteName = "dev-container" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "container-app", | ||
| uri: { | ||
| toString: () => "file:///workspace/container-app", | ||
| }, | ||
| }, | ||
| ] | ||
| const result = getEnvironmentStoragePath(basePath) | ||
| expect(result).toMatch( | ||
| /^\/home\/user\/.vscode\/extensions\/storage\/remote\/dev-container-container-app-[a-z0-9]+$/, | ||
| ) | ||
| }) | ||
|
|
||
| it("should handle Windows paths correctly", () => { | ||
| const windowsBasePath = "C:\\Users\\user\\AppData\\Roaming\\Code\\User\\globalStorage\\roo-cline" | ||
| mockVscode.env.remoteName = "ssh-remote" | ||
| mockVscode.workspace.workspaceFolders = [ | ||
| { | ||
| name: "remote-project", | ||
| uri: { | ||
| toString: () => "file:///home/remote/project", | ||
| }, | ||
| }, | ||
| ] | ||
| const result = getEnvironmentStoragePath(windowsBasePath) | ||
| // The path.join will use forward slashes even on Windows in Node.js context | ||
| expect(result).toMatch( | ||
| /^C:\\Users\\user\\AppData\\Roaming\\Code\\User\\globalStorage\\roo-cline\/remote\/ssh-remote-remote-project-[a-z0-9]+$/, | ||
| ) | ||
| }) | ||
|
|
||
| it("should fallback to base path when remote ID cannot be determined", () => { | ||
| mockVscode.env.remoteName = "unknown-remote" | ||
| mockVscode.workspace.workspaceFolders = undefined | ||
| // In this case, getRemoteWorkspaceId returns just "unknown-remote" | ||
| const result = getEnvironmentStoragePath(basePath) | ||
| expect(result).toBe(`${basePath}/remote/unknown-remote`) | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||||||||||||||||
| import * as vscode from "vscode" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Detects if the extension is running in a remote environment | ||||||||||||||||||||||||
| * (Remote SSH, WSL, Dev Containers, Codespaces, etc.) | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function isRemoteEnvironment(): boolean { | ||||||||||||||||||||||||
| // Check if we're in a remote extension host | ||||||||||||||||||||||||
| // vscode.env.remoteName will be defined in remote contexts | ||||||||||||||||||||||||
| // It will be 'ssh-remote' for SSH, 'wsl' for WSL, 'dev-container' for containers, etc. | ||||||||||||||||||||||||
| return typeof vscode.env.remoteName !== "undefined" | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Gets the type of remote environment if in one | ||||||||||||||||||||||||
| * @returns The remote name (e.g., 'ssh-remote', 'wsl', 'dev-container') or undefined if local | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function getRemoteType(): string | undefined { | ||||||||||||||||||||||||
| return vscode.env.remoteName | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Gets a unique identifier for the remote workspace | ||||||||||||||||||||||||
| * This combines the remote type with workspace information to create | ||||||||||||||||||||||||
| * a stable identifier for the remote session | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function getRemoteWorkspaceId(): string | undefined { | ||||||||||||||||||||||||
| if (!isRemoteEnvironment()) { | ||||||||||||||||||||||||
| return undefined | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const remoteName = vscode.env.remoteName || "remote" | ||||||||||||||||||||||||
| const workspaceFolders = vscode.workspace.workspaceFolders | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (workspaceFolders && workspaceFolders.length > 0) { | ||||||||||||||||||||||||
| // Use the first workspace folder's name and URI as part of the identifier | ||||||||||||||||||||||||
| const firstFolder = workspaceFolders[0] | ||||||||||||||||||||||||
| const folderName = firstFolder.name | ||||||||||||||||||||||||
| // Create a stable hash from the URI to avoid issues with special characters | ||||||||||||||||||||||||
| const uriHash = hashString(firstFolder.uri.toString()) | ||||||||||||||||||||||||
| return `${remoteName}-${folderName}-${uriHash}` | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Fallback to just remote name if no workspace | ||||||||||||||||||||||||
| return remoteName | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Simple string hash function for creating stable identifiers | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| function hashString(str: string): string { | ||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
Suggested change
|
||||||||||||||||||||||||
| let hash = 0 | ||||||||||||||||||||||||
| for (let i = 0; i < str.length; i++) { | ||||||||||||||||||||||||
| const char = str.charCodeAt(i) | ||||||||||||||||||||||||
| hash = (hash << 5) - hash + char | ||||||||||||||||||||||||
| hash = hash & hash // Convert to 32-bit integer | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| return Math.abs(hash).toString(36) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Gets the appropriate storage base path for the current environment | ||||||||||||||||||||||||
| * In remote environments, this will include a remote-specific subdirectory | ||||||||||||||||||||||||
| * to keep remote and local chat histories separate | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export function getEnvironmentStoragePath(basePath: string): string { | ||||||||||||||||||||||||
| if (!isRemoteEnvironment()) { | ||||||||||||||||||||||||
| // Local environment - use the base path as-is | ||||||||||||||||||||||||
| return basePath | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Remote environment - add a remote-specific subdirectory | ||||||||||||||||||||||||
| const remoteId = getRemoteWorkspaceId() | ||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add error handling here? If
Suggested change
|
||||||||||||||||||||||||
| if (remoteId) { | ||||||||||||||||||||||||
| // Use path.join for proper path construction | ||||||||||||||||||||||||
| const path = require("path") | ||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional? Using
Suggested change
Then you can remove line 76 and the dynamic require. |
||||||||||||||||||||||||
| return path.join(basePath, "remote", remoteId) | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Fallback to base path if we can't determine remote ID | ||||||||||||||||||||||||
| return 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.
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.