-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: persist task history in Dev Containers across rebuilds #8682
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
base: main
Are you sure you want to change the base?
Conversation
- Add Dev Container detection logic to identify when running in containers - Implement automatic storage path detection for persistent storage locations - Add user notification system to warn about ephemeral storage in Dev Containers - Integrate Dev Container support into existing storage utility - Add comprehensive test coverage for Dev Container scenarios Fixes #8681
Review SummaryI've reviewed the changes in this PR and identified one issue that should be addressed: Issues to Resolve
Review completed by Roo Code |
return | ||
} | ||
|
||
const message = |
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.
Replace hardcoded user-facing messages with translatable strings via the t() function. For example, the notification text and button labels at lines 121–129 should use translation keys to support localization.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
return ephemeralPaths.some( | ||
(ephemeral) => | ||
normalizedPath.startsWith(ephemeral.toLowerCase()) || | ||
normalizedPath.includes("/.vscode") || | ||
normalizedPath.includes("/vscode-") || | ||
normalizedPath.includes("/tmp/") || | ||
normalizedPath.includes("/temp/"), | ||
) |
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.
The substring matching for /tmp/
and /temp/
can produce false positives. For example, paths like /home/tmperson/data
or /var/temperature/logs
would incorrectly be flagged as ephemeral storage because they contain these substrings. Consider checking for these as complete path segments instead. For instance, you could split the path by /
and check if tmp
or temp
exists as a complete segment, or use a regex pattern like /\/tmp\/|\/temp\//
to ensure they're matched as directory separators.
Summary
This PR addresses Issue #8681 by implementing automatic detection and handling of Dev Container environments to ensure task history persists across container rebuilds.
Problem
When working in Dev Containers, the default storage location (
context.globalStorageUri.fsPath
) is often inside the container's ephemeral file system. This causes all task history to be lost when the container is rebuilt.Solution
Dev Container Detection: Implements robust detection of Dev Container environments through:
Automatic Storage Path Migration: When running in a Dev Container with ephemeral storage:
User Notification System:
Changes
src/utils/devContainer.ts
with Dev Container detection and storage utilitiessrc/utils/storage.ts
to integrate Dev Container supportsrc/extension.ts
to check and notify about Dev Container storage on activationsrc/utils/__tests__/devContainer.spec.ts
Testing
Fixes
Fixes #8681
Important
Adds Dev Container detection and storage path migration to persist task history across container rebuilds, with user notifications and preferences.
devContainer.ts
.devContainer.ts
for Dev Container detection and storage utilities.storage.ts
to integrate Dev Container support for storage paths.extension.ts
to notify users about storage setup in Dev Containers.devContainer.spec.ts
for detection logic, path handling, and user notifications.This description was created by
for e361447. You can customize this summary. It will automatically update as commits are pushed.