Conversation
- Add CSS stylesheets for webview components (reset, vscode, main) - Add Promptitude extension icon (SVG) - Implement modern card-based UI design system
- Add PromptCardsWebviewProvider for interactive card view - Add PromptDetailsWebviewProvider for prompt details panel - Add PromptTreeDataProvider for tree-based prompt navigation - Add PromptCommandManager for handling prompt actions - Implement search, filtering, and category organization - Support for repository source tracking and linking
- ConfigManager now accepts extension context - Use globalStorageUri for profile-aware prompt storage - Fallback to legacy global storage for backward compatibility
- Add directoryExists() method - Add readDirectory() method for scanning directories - Support for better file system operations
- Only show sync success notification when items are updated - Prevent unnecessary notifications on zero-change syncs
- Separate repository storage from active prompts workspace - Implement symlink creation/removal for prompt activation - Add repository storage migration from old location - Add orphaned prompts cleanup functionality - Add broken symlinks auto-repair - Track repository URLs for each prompt - Update prompts via symlinks when source changes - Add cleanup command for removing orphaned files
- Register card and details webview providers - Initialize PromptTreeDataProvider - Add PromptCommandManager for handling UI interactions - Register cleanup orphaned prompts command - Wire up configuration change listeners for UI refresh - Pass extension context to ConfigManager for profile support
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a prompt management UI (tree, cards, details) with new webviews and commands, introduces per-repository storage and reversible base64url repo slugging, symlink-based active prompt management and orphan cleanup, simplifies config path resolution, and ships new CSS assets and utility helpers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Webviews/UI
participant Cmd as PromptCommandManager
participant Tree as PromptTreeDataProvider
participant Sync as SyncManager
participant Repo as RepositoryStorage
participant FS as FileSystem
User->>UI: view/filter/toggle/edit prompt
UI->>Tree: request prompts / details
Tree->>Repo: read repo storage (decode slugs)
Repo-->>Tree: return repo-stored prompts
Tree-->>UI: render prompts
User->>Cmd: action (toggle/view/edit)
Cmd->>Sync: activatePrompt / deactivatePrompt (if needed)
Sync->>Repo: ensure repo storage path, migrate slugs
Sync->>FS: create/remove symlink in workspace
FS-->>Sync: result
Sync->>Tree: refresh (update active flags)
Tree-->>UI: update UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus on:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive UI for the Promptitude extension, adding webview-based interfaces for viewing, managing, and activating prompts synced from Git repositories. The changes implement a symlink-based architecture where prompts are stored in repository-specific directories and activated by creating symlinks to the workspace prompts directory.
Key Changes:
- New UI components with cards and details webviews for prompt management
- Symlink-based activation system separating repository storage from active prompts
- Repository storage migration from
.promptitude/repostoreposdirectory - Profile-specific storage support via VS Code extension context
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension.ts | Wires up new UI components (tree provider, webviews, commands) and integrates them with SyncManager |
| src/ui/promptTreeProvider.ts | New tree data provider for displaying prompts by category with active/inactive states |
| src/ui/promptDetailsWebview.ts | New webview for displaying and editing individual prompt details |
| src/ui/promptCardsWebview.ts | New card-based webview UI with filtering and search functionality |
| src/ui/promptCommands.ts | Command handlers for prompt operations (activate, duplicate, delete, etc.) |
| src/syncManager.ts | Extended with symlink management, repository storage, and cleanup operations |
| src/configManager.ts | Added profile-specific storage support via extension context |
| src/utils/fileSystem.ts | Added directory existence check and directory reading methods |
| src/utils/notifications.ts | Suppresses notifications when no items are updated |
| package.json | Added views, view containers, commands, and menus for the new UI |
| media/*.css | CSS files for webview styling with VS Code theme integration |
| resources/promptitude-icon.svg | New icon for the activity bar view |
| CHANGELOG.md | Updated version to 1.5.0 with changes description |
src/ui/promptDetailsWebview.ts
Outdated
| .toLowerCase(); | ||
|
|
||
| // Store outside prompts directory to prevent Copilot from scanning | ||
| const repoStoragePath = path.join(userDir, '.promptitude', 'repos', encodedUrl, prompt.name); |
There was a problem hiding this comment.
Hardcoded path construction for repository storage that duplicates logic from SyncManager. The path construction .promptitude/repos should match the actual storage location used by SyncManager.
Looking at syncManager.ts line 629, the repository storage is at path.join(parentDir, 'repos') where parentDir is the parent of the prompts directory. This code constructs path.join(userDir, '.promptitude', 'repos', ...) which doesn't match.
This inconsistency will cause the webview to look in the wrong location for repository files.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/configManager.ts (2)
26-158: Restore the logger field before shipping.
Dropping the logger member from the class makes everythis.logger.*call fail the build (TS2339: Property 'logger' does not exist on type 'ConfigManager'in CI). Please reintroduce an optional logger field (and update the constructor) or strip the log calls so the file compiles.Apply this diff:
@@ - private context?: vscode.ExtensionContext; - - constructor(context?: vscode.ExtensionContext) { - this.context = context; - } + private context?: vscode.ExtensionContext; + private logger?: Logger; + + constructor(context?: vscode.ExtensionContext, logger?: Logger) { + this.context = context; + this.logger = logger; + } @@ - this.logger.info(`Using custom prompts path: ${this.customPath}`); + this.logger?.info(`Using custom prompts path: ${this.customPath}`); @@ - this.logger.warn('Using fallback hardcoded prompts directory paths'); + this.logger?.warn('Using fallback hardcoded prompts directory paths'); @@ - if (this.debug) { - this.logger.debug(`Fallback prompts directory: ${promptsPath}`); + if (this.debug) { + this.logger?.debug(`Fallback prompts directory: ${promptsPath}`);
112-124: Keep thepromptssubfolder in the legacy path.
The new fallback branch returns<Code/User>rather than<Code/User/prompts>. The oldgetFallbackPromptsDirectory()still appendsprompts, and the rest of the sync logic expects that subfolder. Without it we’ll read/write into the profile root instead of the prompts store. Please keep thepromptssuffix when context is missing.Apply this diff:
switch (process.platform) { case 'win32': - return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User'); + return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User', 'prompts'); case 'darwin': - return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User'); + return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User', 'prompts'); case 'linux': - return path.join(os.homedir(), '.config', 'Code', 'User'); + return path.join(os.homedir(), '.config', 'Code', 'User', 'prompts'); default: - return path.join(os.homedir(), '.vscode', 'User'); + return path.join(os.homedir(), '.vscode', 'User', 'prompts'); }media/main.css (1)
544-555: Fix invalid RGBA usage for themed green.
rgba(var(--vscode-charts-green), 0.15)never resolves because--vscode-charts-greenexpands to a hex string (e.g.#3FB950), andrgba()cannot accept that form. As a result the background color drops entirely, so active cards lose their visual state. Swap to a valid construct (e.g.color-mix(in srgb, var(--vscode-charts-green) 15%, transparent)or bake the RGB literal) so the styling actually applies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
resources/promptitude-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
CHANGELOG.md(1 hunks)media/main.css(1 hunks)media/reset.css(1 hunks)media/vscode.css(1 hunks)package.json(3 hunks)src/configManager.ts(2 hunks)src/extension.ts(4 hunks)src/syncManager.ts(11 hunks)src/ui/promptCardsWebview.ts(1 hunks)src/ui/promptCommands.ts(1 hunks)src/ui/promptDetailsWebview.ts(1 hunks)src/ui/promptTreeProvider.ts(1 hunks)src/utils/fileSystem.ts(2 hunks)src/utils/notifications.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/utils/fileSystem.ts (1)
src/utils/logger.ts (1)
error(68-77)
src/ui/promptCommands.ts (4)
src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/ui/promptTreeProvider.ts (2)
PromptTreeDataProvider(105-605)PromptInfo(8-18)src/ui/promptDetailsWebview.ts (1)
PromptDetailsWebviewProvider(6-527)src/syncManager.ts (1)
SyncManager(32-1051)
src/extension.ts (5)
src/syncManager.ts (1)
SyncManager(32-1051)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(105-605)src/ui/promptDetailsWebview.ts (1)
PromptDetailsWebviewProvider(6-527)src/ui/promptCardsWebview.ts (1)
PromptCardsWebviewProvider(6-1046)src/ui/promptCommands.ts (1)
PromptCommandManager(9-378)
src/syncManager.ts (3)
src/configManager.ts (2)
ConfigManager(17-229)repositories(44-54)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(105-605)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)
src/ui/promptCardsWebview.ts (2)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(105-605)
src/ui/promptDetailsWebview.ts (3)
src/ui/promptTreeProvider.ts (1)
PromptInfo(8-18)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/utils/logger.ts (2)
Logger(3-88)error(68-77)
src/ui/promptTreeProvider.ts (3)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/configManager.ts (1)
ConfigManager(17-229)
🪛 GitHub Actions: CI/CD Pipeline
src/configManager.ts
[error] 98-98: TS2339: Property 'logger' does not exist on type 'ConfigManager'.
🔇 Additional comments (4)
src/utils/notifications.ts (1)
43-46: Thanks for suppressing empty sync toasts.
Glad to see the success toast only fires when work happened; this keeps the UI quieter during no-op syncs.package.json (1)
93-232: Manifest wiring for the new views looks solid.
The activity bar container, paired webviews, and view-title menu groups line up with the new commands, so the UI should light up without manual activation tweaks.media/reset.css (1)
1-49: Baseline reset is a welcome addition.
Using the VS Code font/color tokens here should keep the webviews consistent with the host theme.media/vscode.css (1)
1-42: Nice VS Code token palette.
These shared variables and focus styles will make the new webviews feel native, and the scrollbar tweaks keep accessibility intact.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/configManager.ts (2)
116-125: Fix path inconsistency in fallback logic.The fallback returns the User directory without appending the 'prompts' subdirectory, but the
getFallbackPromptsDirectory()method (lines 136-149) appends 'prompts' to each path. This inconsistency could cause prompts to be stored in the wrong location when context is unavailable.Apply this diff to append 'prompts' subdirectory:
switch (process.platform) { case 'win32': - return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User'); + return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User', 'prompts'); case 'darwin': - return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User'); + return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User', 'prompts'); case 'linux': - return path.join(os.homedir(), '.config', 'Code', 'User'); + return path.join(os.homedir(), '.config', 'Code', 'User', 'prompts'); default: - return path.join(os.homedir(), '.vscode', 'User'); + return path.join(os.homedir(), '.vscode', 'User', 'prompts'); }
131-160: Remove unused method or refactor to eliminate duplication.The
getFallbackPromptsDirectory()method is never called and represents dead code. Additionally, its logic duplicates the fallback switch statement ingetPromptsDirectory()(lines 116-125).Consider either:
- Remove this method if it's no longer needed, or
- Refactor
getPromptsDirectory()to call this method for the fallback case, eliminating duplicationIf you choose option 2, apply this diff to eliminate duplication:
// Fallback to global user data directory (legacy behavior) - const os = require('os'); - - switch (process.platform) { - case 'win32': - return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User', 'prompts'); - case 'darwin': - return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User', 'prompts'); - case 'linux': - return path.join(os.homedir(), '.config', 'Code', 'User', 'prompts'); - default: - return path.join(os.homedir(), '.vscode', 'User', 'prompts'); - } + return this.getFallbackPromptsDirectory(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/configManager.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/configManager.ts (1)
src/utils/logger.ts (1)
Logger(3-88)
🔇 Additional comments (1)
src/configManager.ts (1)
26-31: I found one instantiation of ConfigManager where context is properly passed. Let me verify there are no other instantiations and check how the ConfigManager methods handle the optional context:Let me search for broader context usage and any other instantiations or test files:
Based on my verification, I found that ConfigManager has a single instantiation in
src/extension.ts:38where the context is correctly passed:const configManager = new ConfigManager(context);The instantiation properly provides the VS Code
ExtensionContextwhen available. The optional parameter design is appropriate for scenarios where context might not be available. However, I cannot fully verify the internal implementation without examining the methods that usethis.context. To ensure complete correctness, the ConfigManager methods (particularlygetPromptsDirectory()and any other methods accessing the context) should include proper null/undefined checks to handle both cases gracefully.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/configManager.ts (1)
97-109: Fallback prompts path is incorrect; points to the user directory, not “…/User/prompts”.This will place files in the wrong location and break repo storage sibling path computations.
- // Fallback to global user data directory (legacy behavior) - switch (process.platform) { - case 'win32': - return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User'); - case 'darwin': - return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User'); - case 'linux': - return path.join(os.homedir(), '.config', 'Code', 'User'); - default: - return path.join(os.homedir(), '.vscode', 'User'); - } + // Fallback to global user data directory (legacy behavior) — ensure '/prompts' + switch (process.platform) { + case 'win32': + return path.join(os.homedir(), 'AppData', 'Roaming', 'Code', 'User', 'prompts'); + case 'darwin': + return path.join(os.homedir(), 'Library', 'Application Support', 'Code', 'User', 'prompts'); + case 'linux': + return path.join(os.homedir(), '.config', 'Code', 'User', 'prompts'); + default: + return path.join(os.homedir(), '.vscode', 'prompts'); + }Alternatively, just delegate here:
- // Fallback to global user data directory (legacy behavior) - switch (process.platform) { - ... - } + return this.getFallbackPromptsDirectory();Also applies to: 111-121
♻️ Duplicate comments (6)
src/ui/promptCardsWebview.ts (1)
541-545: Invalid CSS color function; rule is dropped.
rgba(var(--vscode-charts-green), 0.15)won’t parse (token resolves to hex). Use color-mix or explicit RGB to restore the highlight.- .action-btn.active-btn { - background-color: rgba(var(--vscode-charts-green), 0.15); - color: var(--chatmode-color); - border: 1px solid var(--chatmode-color); - } + .action-btn.active-btn { + background-color: color-mix(in srgb, var(--vscode-charts-green) 15%, transparent); + color: var(--chatmode-color); + border: 1px solid var(--chatmode-color); + }CHANGELOG.md (1)
5-13: Changelog misses major “Added” UI/storage features; entries too generic.Document the new webview UI, command set, and repo storage changes under “Added”, and keep “Fixed” specific. This aligns the notes with the PR impact.
Suggested patch (edit wording as needed):
## [Unreleased] ### Fixed - **Repository Slug Encoding**: Replaced lossy underscore-based repository slug encoding with reversible Base64 URL encoding - Moved repository slugging into shared helper `src/storage/repositoryStorage.ts` - Added `encodeRepositorySlug` and `decodeRepositorySlug` functions using Base64 URL encoding - Implemented automatic migration from legacy underscore-based slugs to new format - Prevents data loss from ambiguous URL-to-slug conversions (e.g., URLs with underscores) +### Added +- **UI**: Cards view (`promptitude.cards`) and Details view (`promptitude.details`) webviews. +- **Commands**: Centralized prompt actions via `PromptCommandManager` (`prompts.*`) and cleanup command. +- **Repo Storage**: Per-repo storage under `<promptsParent>/repos`, active‑prompt symlinks, and orphan cleanup. ## [1.5.0] - 2025-11-12 -### Fixed -- Enhanced logging and removed console.logs -- Improved code quality and performance optimizations +### Added +- Webview UI for browsing and managing prompts. +- Repository storage migration and legacy slug migration. + +### Fixed +- Removed stray console logs; improved structured logging. +- Sync performance and reliability improvements in file filtering and symlink handling.Also applies to: 15-20
src/extension.ts (1)
52-63: Register provider disposables with context.subscriptions.Capture the return values from both registerWebviewViewProvider calls and push them into context.subscriptions to avoid leaks across reload/deactivate.
- // Register cards webview provider - vscode.window.registerWebviewViewProvider( + // Register cards webview provider + const cardsDisposable = vscode.window.registerWebviewViewProvider( PromptCardsWebviewProvider.viewType, promptCardsProvider ); - - // Register details webview provider - vscode.window.registerWebviewViewProvider( + // Register details webview provider + const detailsDisposable = vscode.window.registerWebviewViewProvider( PromptDetailsWebviewProvider.viewType, promptDetailsProvider ); + context.subscriptions.push(cardsDisposable, detailsDisposable);src/syncManager.ts (2)
672-684: Migration usescp -R(Unix-only). Breaks on Windows.Use Node’s cross‑platform copy (Node 16+), then remove the old dir.
- this.logger.warn('Could not move repository storage, attempting copy...'); - const ncp = require('child_process').spawnSync('cp', ['-R', oldRepoStorage, newRepoStorage]); - - if (ncp.status === 0) { - // Delete old directory after successful copy - await fs.rm(oldRepoStorage, { recursive: true, force: true }); - this.logger.info('Successfully copied and removed old repository storage'); - } else { - this.logger.error('Failed to migrate repository storage'); - } + this.logger.warn('Could not move repository storage, attempting copy...'); + await fs.cp(oldRepoStorage, newRepoStorage, { recursive: true }); + await fs.rm(oldRepoStorage, { recursive: true, force: true }); + this.logger.info('Successfully copied and removed old repository storage');
934-954: Decode repo URL from symlink target uses legacy heuristic; fails with new base64url slugs.Decode with
decodeRepositorySlugand fallback to legacy only when needed.- private extractRepositoryUrlFromTargetPath(targetPath: string): string | undefined { + private extractRepositoryUrlFromTargetPath(targetPath: string): string | undefined { try { const pathParts = targetPath.split(path.sep); const reposIndex = pathParts.findIndex(part => part === 'repos'); if (reposIndex !== -1 && reposIndex + 1 < pathParts.length) { - const encodedRepoUrl = pathParts[reposIndex + 1]; - // Decode the repository URL - const decodedUrl = encodedRepoUrl - .replace(/_/g, '/') - .replace(/^/, 'https://'); - return decodedUrl; + const encodedRepoUrl = pathParts[reposIndex + 1]; + try { + return decodeRepositorySlug(encodedRepoUrl); + } catch { + // Legacy underscore-based slug fallback + return legacySlugToUrl(encodedRepoUrl); + } } return undefined; } catch (error) {Make sure to import:
+import { decodeRepositorySlug, legacySlugToUrl } from './storage/repositoryStorage';src/ui/promptTreeProvider.ts (1)
134-142: Race condition still present in refresh() method.As previously noted, the
isLoadingcheck inrefresh()happens before the flag is set inloadPrompts(). Two concurrent calls torefresh()can both pass the check before either sets the flag to true.Apply this diff to set the flag synchronously:
refresh(): void { if (this.isLoading) { this.logger.debug('Refresh already in progress, skipping concurrent refresh'); return; } + this.isLoading = true; this.logger.debug('Starting tree refresh'); - this.refreshAsync(); + this.refreshAsync().finally(() => { + this.isLoading = false; + }); }And remove the flag management from
loadPrompts()since it will now be handled byrefresh():private async loadPrompts(): Promise<void> { - if (this.isLoading) { - this.logger.debug('loadPrompts already in progress, skipping'); - return; - } - - this.isLoading = true; this.prompts.clear(); // ... rest of method - } finally { - this.isLoading = false; - } }
🧹 Nitpick comments (3)
src/ui/promptCardsWebview.ts (1)
131-139: Add CSP + nonce for the webview script (parity with Details view).Harden the cards view by adding a CSP and a nonce on the inline script, mirroring
PromptDetailsWebviewProvider.- <style> + <!-- Add CSP meta similar to details view and nonce on script --> + <!-- At top of _getHtmlForWebview(): const nonce = getNonce(); --> + <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${webview.cspSource}; script-src 'nonce-${nonce}';"> + <style> @@ - <script> + <script nonce="${nonce}"> @@ -</html>`; +</html>`;Add a helper in this module (outside the class) like in details:
+function getNonce() { + let text = ''; + const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + for (let i = 0; i < 32; i++) text += possible.charAt(Math.floor(Math.random() * possible.length)); + return text; +}Also applies to: 618-636, 1033-1038
src/syncManager.ts (1)
64-71: Update repoStorageDir when configuration changes.If
promptitude.customPathor profiles change, recompute the storage root so subsequent operations use the new location.- const configDisposable = this.config.onConfigurationChanged(() => { + const configDisposable = this.config.onConfigurationChanged(() => { this.logger.debug('Configuration changed, reinitializing sync schedule'); + this.repoStorageDir = this.getRepositoryStorageDirectory(); this.scheduleNextSync(); });src/ui/promptDetailsWebview.ts (1)
102-139: Prefer FileSystemManager for existence checks (consistency, testability).
getActualFilePathmixes syncfs.existsSynccalls with async FS elsewhere. Consider refactoring to an async helper usingthis.fileSystem.fileExists(...)for both workspace and repo paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)src/configManager.ts(2 hunks)src/extension.ts(4 hunks)src/storage/repositoryStorage.ts(1 hunks)src/syncManager.ts(11 hunks)src/ui/promptCardsWebview.ts(1 hunks)src/ui/promptCommands.ts(1 hunks)src/ui/promptDetailsWebview.ts(1 hunks)src/ui/promptTreeProvider.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/promptCommands.ts
🧰 Additional context used
🧬 Code graph analysis (7)
src/ui/promptCardsWebview.ts (3)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/configManager.ts (1)
ConfigManager(17-226)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(106-617)
src/syncManager.ts (4)
src/configManager.ts (1)
ConfigManager(17-226)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(106-617)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/storage/repositoryStorage.ts (3)
isLegacySlug(41-67)migrateLegacySlug(94-97)encodeRepositorySlug(10-18)
src/ui/promptDetailsWebview.ts (4)
src/ui/promptTreeProvider.ts (1)
PromptInfo(9-19)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/storage/repositoryStorage.ts (1)
encodeRepositorySlug(10-18)
src/storage/repositoryStorage.ts (1)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)
src/extension.ts (5)
src/syncManager.ts (1)
SyncManager(34-1106)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(106-617)src/ui/promptDetailsWebview.ts (1)
PromptDetailsWebviewProvider(9-524)src/ui/promptCardsWebview.ts (1)
PromptCardsWebviewProvider(6-1039)src/ui/promptCommands.ts (1)
PromptCommandManager(9-385)
src/ui/promptTreeProvider.ts (4)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/configManager.ts (1)
ConfigManager(17-226)src/storage/repositoryStorage.ts (3)
isLegacySlug(41-67)legacySlugToUrl(75-87)decodeRepositorySlug(25-33)
src/configManager.ts (1)
src/utils/logger.ts (1)
Logger(3-88)
🔇 Additional comments (2)
src/storage/repositoryStorage.ts (1)
10-18: Now I'll search for the Node and Electron versions bundled with VS Code 1.70.0 and whether they support base64url:Based on the verification, VS Code 1.70.0 bundles Electron 18.3.5 and Node.js 16.13.2. The review comment's concern about runtime support can be resolved: the 'base64url' encoding was introduced in Node.js, and specifically, it was added in Node.js v15.7.0. Since your extension targets Node 16.13.2 (bundled with your minimum VS Code version), the
Buffer.toString('base64url')andBuffer.from(..., 'base64url')APIs are fully supported. The code is compatible with all supported VS Code versions.No action required — the implementation correctly uses base64url encoding supported by your minimum Node runtime.
src/ui/promptTreeProvider.ts (1)
601-616: URL decoding implementation correctly addresses previous concerns.The encoding/decoding now properly handles both legacy underscore-based slugs and the new base64url format using the helper functions from
repositoryStorage.ts. This resolves the lossy encoding issues flagged in previous reviews.Based on past review comments.
…lation and improve path resolution
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
CHANGELOG.md (1)
46-51: The v1.5.0 entries remain too vague.The same generic entries flagged in the previous review are still present. These provide no actionable information for users. Consider either removing this release section if it was published with these entries, or updating them to match the specificity now shown in the [Unreleased] section.
src/ui/promptTreeProvider.ts (2)
134-142: Race condition still present in refresh().The guard at line 135 checks
isLoadingbefore callingrefreshAsync(), butisLoadingis only set totrueinsideloadPrompts()(line 238). Ifrefresh()is called twice in quick succession, both calls can pass the check before either sets the flag. The previous review suggested setting the flag synchronously:refresh(): void { if (this.isLoading) { this.logger.debug('Refresh already in progress, skipping concurrent refresh'); return; } + this.isLoading = true; this.logger.debug('Starting tree refresh'); - this.refreshAsync(); + this.refreshAsync().finally(() => { + this.isLoading = false; + }); }Then update
loadPrompts()to skip setting the flag since it's already managed:private async loadPrompts(): Promise<void> { - if (this.isLoading) { - this.logger.debug('loadPrompts already in progress, skipping'); - return; - } - - this.isLoading = true; this.prompts.clear(); this.logger.debug('Starting to load prompts - cleared existing prompts'); try { // ... loading logic ... } catch (error) { this.logger.error('Failed to load prompts', error instanceof Error ? error : undefined); - } finally { - this.isLoading = false; } }
172-178: Potential empty tree during concurrent initial load.If
getChildren()is called whileisLoadingis true but beforeinitialLoad()populates the prompts map,ensurePromptsLoaded()returns immediately (line 174 condition fails), causing the tree to render empty. The previous review suggested tracking the in-flight promise:private isLoading = false; +private loadingPromise?: Promise<void>; private async initialLoad(): Promise<void> { try { this.logger.debug('Starting initial prompt load'); - await this.loadPrompts(); + this.loadingPromise = this.loadPrompts(); + await this.loadingPromise; this._onDidChangeTreeData.fire(); this.logger.debug('Initial prompt load completed'); } catch (error) { this.logger.error('Failed to perform initial prompt load', error instanceof Error ? error : undefined); + } finally { + this.loadingPromise = undefined; } } private async ensurePromptsLoaded(): Promise<void> { + if (this.loadingPromise) { + await this.loadingPromise; + return; + } if (this.prompts.size === 0 && !this.isLoading) { this.logger.debug('No prompts loaded, triggering load in getChildren'); - await this.loadPrompts(); + this.loadingPromise = this.loadPrompts(); + await this.loadingPromise; + this.loadingPromise = undefined; } }
🧹 Nitpick comments (1)
CHANGELOG.md (1)
5-44: Consider adding an "Added" section for new features.While the [Unreleased] entries now provide excellent specificity (addressing the previous concern), some items introduce new functionality rather than fixing bugs:
- Repository Slug Encoding (lines 9-13): Introduces new
encodeRepositorySluganddecodeRepositorySlugfunctions- Duplicate Prompt Detection (lines 26-29): Introduces new
findPromptByNameAndRepository()methodFollowing Keep a Changelog conventions, consider splitting these into:
- Added: New features, functions, or methods
- Fixed: Bug fixes and corrections
This helps users distinguish between new capabilities and issue resolutions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/ui/promptTreeProvider.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/ui/promptTreeProvider.ts (3)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/storage/repositoryStorage.ts (3)
isLegacySlug(41-67)legacySlugToUrl(75-87)decodeRepositorySlug(25-33)
🔇 Additional comments (2)
src/ui/promptTreeProvider.ts (2)
365-375: LGTM! Duplicate detection now uses composite key.The new
findPromptByNameAndRepository()method correctly prevents conflation of prompts with the same filename from different repositories by using a composite key (filename + repository URL). This addresses the previous review concern.
615-630: LGTM! Repository URL encoding is now reversible.The implementation correctly uses base64url encoding (via
decodeRepositorySlug()) with automatic migration from legacy underscore-based slugs. This addresses the previous review concern about data loss from ambiguous URL-to-slug conversions.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @MatFillion. * #19 (comment) The following files were modified: * `src/extension.ts` * `src/ui/promptDetailsWebview.ts`
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/syncManager.ts (1)
675-683: Platform-specific command will fail on Windows.The
cp -Rcommand only works on Unix-like systems and will fail on Windows. UseFileSystemManager.copyDirectory()or Node.jsfs.cp()for cross-platform compatibility.Based on past review feedback.
src/ui/promptTreeProvider.ts (1)
134-151: Race condition in concurrent refresh calls.The
isLoadingcheck at line 135 happens before the flag is set inloadPrompts(). Ifrefresh()is called twice in quick succession, both calls can pass the check before either setsisLoading = true. Set the flag immediately inrefresh()and clear it when done.refresh(): void { if (this.isLoading) { this.logger.debug('Refresh already in progress, skipping concurrent refresh'); return; } + this.isLoading = true; this.logger.debug('Starting tree refresh'); - this.refreshAsync(); + this.refreshAsync().finally(() => { + this.isLoading = false; + }); } private async refreshAsync(): Promise<void> { try { await this.loadPrompts(); this._onDidChangeTreeData.fire(); } catch (error) { this.logger.error('Failed to refresh prompts', error instanceof Error ? error : undefined); } }And update
loadPrompts()to not setisLoadingsincerefresh()now manages it:private async loadPrompts(): Promise<void> { - if (this.isLoading) { - this.logger.debug('loadPrompts already in progress, skipping'); - return; - } - - this.isLoading = true; this.prompts.clear(); // ... rest of method - } finally { - this.isLoading = false; - } }Based on past review feedback.
🧹 Nitpick comments (1)
src/syncManager.ts (1)
869-892: Consider improving legacy URL extraction.The decoding at lines 881-884 is lossy and may not correctly reconstruct URLs from legacy slugs (e.g., URLs with underscores). This is a best-effort migration helper, but consider logging a warning when extraction is attempted, indicating that the URL might be inaccurate.
if (reposIndex !== -1 && reposIndex + 1 < pathParts.length) { const encodedRepoUrl = pathParts[reposIndex + 1]; + // Attempt to decode as base64url first (new format) + try { + return decodeRepositorySlug(encodedRepoUrl); + } catch { + // Fall back to legacy format (lossy) + this.logger.warn(`Using legacy URL decoding for: ${encodedRepoUrl} - URL may be inaccurate`); + } // Decode the repository URL const decodedUrl = encodedRepoUrl .replace(/_/g, '/') .replace(/^/, 'https://'); return decodedUrl; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)src/storage/repositoryStorage.ts(1 hunks)src/syncManager.ts(11 hunks)src/ui/promptTreeProvider.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- src/storage/repositoryStorage.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/ui/promptTreeProvider.ts (4)
src/utils/logger.ts (2)
Logger(3-88)error(68-77)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/configManager.ts (1)
ConfigManager(17-226)src/storage/repositoryStorage.ts (1)
decodeRepositorySlug(25-33)
src/syncManager.ts (4)
src/configManager.ts (2)
ConfigManager(17-226)repositories(45-55)src/ui/promptTreeProvider.ts (1)
PromptTreeDataProvider(106-617)src/utils/fileSystem.ts (1)
FileSystemManager(11-73)src/storage/repositoryStorage.ts (1)
encodeRepositorySlug(10-18)
🔇 Additional comments (22)
src/syncManager.ts (13)
2-2: LGTM!The new imports are properly structured and align with the repository storage refactoring.
Also applies to: 8-8, 12-12
40-40: LGTM!The constructor properly initializes the new repository storage directory and accepts an optional tree provider for UI integration.
Also applies to: 42-51
61-63: LGTM!Repository storage migration during initialization ensures backward compatibility with existing installations.
99-133: LGTM!The sync flow properly orchestrates repository sync, symlink recreation, orphan cleanup, and UI refresh. All async operations are correctly awaited.
198-260: LGTM!The refactored sync flow correctly saves files to repository storage and updates symlinks for active prompts. This separation of concerns improves maintainability.
622-631: LGTM!The repository storage directory is correctly placed as a sibling to the prompts directory, ensuring profile-specific storage.
696-703: LGTM!The method correctly uses reversible base64url encoding via
encodeRepositorySlug(), addressing previous concerns about lossy URL encoding.
705-728: LGTM!Symlink creation is properly implemented with correct await usage and cleanup of existing files before creating new symlinks.
730-751: LGTM!The method correctly verifies the file is a symlink before removal, preventing accidental deletion of regular files.
753-797: LGTM!The method correctly recreates missing symlinks for active prompts with proper error handling and awaits.
894-921: LGTM!The method correctly checks for and updates symlinks for active prompts with proper async/await usage.
923-947: LGTM!The
activatePrompt()method correctly validates the source file exists before creating the symlink.
963-1032: LGTM!The cleanup logic correctly iterates over
repositoryConfigsand uses the base URL (line 1001) without the branch suffix, ensuring proper alignment with the storage layout. This addresses previous concerns about branch-qualified repositories.Based on past review feedback.
src/ui/promptTreeProvider.ts (9)
1-19: LGTM!The imports and
PromptInfointerface are well-structured with appropriate optional fields for repository URL and description.
21-68: LGTM!The
PromptTreeItemimplementation properly integrates with VS Code's tree view API and uses theme colors to distinguish active prompts.
70-104: LGTM!The
CategoryTreeItemclearly displays prompt counts and active status.
106-132: LGTM!The asynchronous initialization pattern correctly loads prompts without blocking the constructor.
286-345: LGTM!The repository storage loading correctly uses
findPromptByNameAndRepository()for duplicate detection, preventing conflicts between prompts with the same filename from different repositories. This addresses previous concerns.Based on past review feedback.
361-375: LGTM!The composite key lookup properly distinguishes prompts with the same name from different repositories.
486-520: LGTM!The description extraction handles YAML frontmatter gracefully with a sensible fallback to content-based extraction.
611-616: LGTM!The method now correctly uses reversible base64url decoding via
decodeRepositorySlug(), addressing previous concerns about lossy URL reconstruction.Based on past review feedback.
552-560: No action needed — symlinks are properly removed.The command handler's
deselectAll()method (lines 161–171 in promptCommands.ts) correctly invokestoggleSelection()for each selected prompt, which in turn callssyncManager.deactivatePrompt()(line 120) to remove symlinks when prompts are deactivated. The tree provider'sdeselectAll()method is intentionally just a UI state manager; symlink removal is properly orchestrated at the command handler level.
Summary by CodeRabbit
New Features
Bug Fixes
Style / UI