From ae49a871517f56150818358cceadc66ec1221695 Mon Sep 17 00:00:00 2001 From: Chirag Chandrashekhar Date: Thu, 16 Apr 2026 20:37:08 +0530 Subject: [PATCH 1/2] fix: normalize paths in workspace collection storage to prevent ghost entries Collections reappeared after removal because of path format mismatches: - addCollectionToWorkspace stored absolute paths but normalizeCollectionEntry (which converts to relative + posixified) was never called - Duplicate entries were created when the exists-check compared a stored relative path against an incoming absolute path - Removal comparison used strict equality without path.normalize, failing on Windows where / and \ differ Fix all three: call normalizeCollectionEntry on add, resolve paths before the exists-check, and use path.normalize in the removal filter. Add e2e tests that create a collection, remove it, create another, and verify the removed one stays gone. Also fix the test fixture to dismiss the VS Code 1.116+ GitHub sign-in dialog on startup. --- src/extension/store/default-workspace.ts | 29 ++++---- src/extension/utils/workspace-config.ts | 11 ++-- tests/e2e/fixtures/index.ts | 17 +++++ tests/e2e/specs/collection-removal.spec.ts | 77 ++++++++++++++++++++++ tests/e2e/utils/actions.ts | 49 ++++++++++++++ 5 files changed, 165 insertions(+), 18 deletions(-) create mode 100644 tests/e2e/specs/collection-removal.spec.ts diff --git a/src/extension/store/default-workspace.ts b/src/extension/store/default-workspace.ts index 8579047..771b13c 100644 --- a/src/extension/store/default-workspace.ts +++ b/src/extension/store/default-workspace.ts @@ -221,22 +221,25 @@ class DefaultWorkspaceManager { config.collections = []; } - const exists = config.collections.some(c => c.path === collectionPath); - if (exists) return true; - - const name = collectionName || path.basename(collectionPath); - + // Normalize to relative path for consistent storage let relativePath = collectionPath; try { - relativePath = path.relative(workspacePath!, collectionPath); - // If the relative path goes outside the workspace, use absolute path - if (relativePath.startsWith('..')) { - relativePath = collectionPath; + const rel = path.relative(workspacePath!, collectionPath); + if (!rel.startsWith('..')) { + relativePath = rel; } - } catch { - relativePath = collectionPath; - } + } catch { } + // Check for existing entry by resolving stored paths to absolute for comparison + const exists = config.collections.some(c => { + const resolved = path.isAbsolute(c.path) + ? c.path + : path.resolve(workspacePath!, c.path); + return path.normalize(resolved) === path.normalize(collectionPath); + }); + if (exists) return true; + + const name = collectionName || path.basename(collectionPath); config.collections.push({ name, path: relativePath @@ -255,7 +258,7 @@ class DefaultWorkspaceManager { const resolvedPath = path.isAbsolute(c.path) ? c.path : path.resolve(workspacePath!, c.path); - return resolvedPath !== collectionPath; + return path.normalize(resolvedPath) !== path.normalize(collectionPath); }); return this.saveWorkspaceConfig(config); diff --git a/src/extension/utils/workspace-config.ts b/src/extension/utils/workspace-config.ts index f9feaaa..f6a3123 100644 --- a/src/extension/utils/workspace-config.ts +++ b/src/extension/utils/workspace-config.ts @@ -318,16 +318,17 @@ const addCollectionToWorkspace = async ( config.collections = []; } - const normalizedCollection: CollectionEntry = { - name: collection.name.trim(), - path: collection.path.trim() - }; + const normalizedCollection = normalizeCollectionEntry(workspacePath, collection); if (collection.remote && typeof collection.remote === 'string') { normalizedCollection.remote = collection.remote.trim(); } - const existingIndex = config.collections.findIndex((c) => c.path === normalizedCollection.path); + // Compare normalized paths to avoid duplicates from absolute/relative mismatches + const existingIndex = config.collections.findIndex((c) => { + const existingNormalized = normalizeCollectionEntry(workspacePath, c); + return existingNormalized.path === normalizedCollection.path; + }); if (existingIndex >= 0) { config.collections[existingIndex] = normalizedCollection; diff --git a/tests/e2e/fixtures/index.ts b/tests/e2e/fixtures/index.ts index 5b83e98..fdaff48 100644 --- a/tests/e2e/fixtures/index.ts +++ b/tests/e2e/fixtures/index.ts @@ -74,6 +74,10 @@ function writeVSCodeSettings(userDataDir: string): void { 'workbench.editor.untitled.hint': 'hidden', 'security.workspace.trust.enabled': false, 'extensions.ignoreRecommendations': true, + 'github.copilot.enable': false, + 'workbench.welcomePage.walkthroughs.openOnInstall': false, + 'workbench.accounts.experimental.showEntitlements': false, + 'accessibility.signUpPlaceholder': false, }, null, 2) ); } @@ -111,6 +115,19 @@ async function waitForWorkbench(page: Page): Promise { await page.waitForSelector('.monaco-workbench', { timeout: 30_000 }); // Wait for the activity bar to be rendered await page.waitForSelector('.activitybar', { timeout: 20_000 }); + + // Dismiss the GitHub sign-in dialog if it appears (VS Code 1.116+) + try { + const skipButton = page.locator('text=Skip'); + const continueButton = page.locator('text=Continue without Signing In'); + const dismissTarget = skipButton.or(continueButton); + await dismissTarget.first().click({ timeout: 3_000 }); + // Wait for the dialog to close + await page.waitForTimeout(1_000); + } catch { + // Dialog didn't appear — that's fine + } + // Small extra buffer for extension activation await new Promise(r => setTimeout(r, 2_000)); } diff --git a/tests/e2e/specs/collection-removal.spec.ts b/tests/e2e/specs/collection-removal.spec.ts new file mode 100644 index 0000000..74b222f --- /dev/null +++ b/tests/e2e/specs/collection-removal.spec.ts @@ -0,0 +1,77 @@ +import { test, expect } from '../fixtures'; +import { + openBrunoSidebar, + createCollection, + removeCollection, +} from '../utils/actions'; + +test.describe('Collection removal', () => { + + test('Removed collection does not reappear when creating a new collection', async ({ page, tmpDir }) => { + const sidebar = await openBrunoSidebar(page); + + // Step 1: Create collection A + const collectionA = 'Collection A'; + await createCollection(page, sidebar, collectionA, tmpDir); + + const rowA = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionA }); + await expect(rowA).toBeVisible(); + + // Step 2: Remove collection A + await removeCollection(page, sidebar, collectionA); + await expect(rowA).not.toBeVisible({ timeout: 10_000 }); + + // Step 3: Create collection B — this triggers workspace-config-updated + // which re-reads workspace.yml. If collection A wasn't properly removed, + // it will reappear here. + const collectionB = 'Collection B'; + await createCollection(page, sidebar, collectionB, tmpDir); + + const rowB = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionB }); + await expect(rowB).toBeVisible(); + + // Step 4: Verify collection A is still gone + await expect(rowA).not.toBeVisible(); + + // Step 5: Count total collections — should be exactly 1 (Collection B) + const allCollections = sidebar.locator('[data-testid="sidebar-collection-row"]'); + // Filter to only the ones we created (exclude any pre-existing workspace collections) + const ourCollections = allCollections.filter({ hasText: /Collection [AB]/ }); + await expect(ourCollections).toHaveCount(1); + }); + + test('Removing and recreating a collection with the same name works', async ({ page, tmpDir }) => { + const fs = require('fs'); + const path = require('path'); + const sidebar = await openBrunoSidebar(page); + const collectionName = 'Ephemeral Collection'; + + // Create → remove → recreate in a different subfolder + const dir1 = path.join(tmpDir, 'round1'); + fs.mkdirSync(dir1, { recursive: true }); + await createCollection(page, sidebar, collectionName, dir1); + await removeCollection(page, sidebar, collectionName); + + const row = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionName }); + await expect(row).not.toBeVisible({ timeout: 10_000 }); + + // Recreate with the same name in a different folder to avoid filesystem conflict + const dir2 = path.join(tmpDir, 'round2'); + fs.mkdirSync(dir2, { recursive: true }); + await createCollection(page, sidebar, collectionName, dir2); + await expect(row).toBeVisible(); + + // Should be exactly 1 instance, not 2 + const matches = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionName }); + await expect(matches).toHaveCount(1); + }); + +}); diff --git a/tests/e2e/utils/actions.ts b/tests/e2e/utils/actions.ts index 7165c4c..4898572 100644 --- a/tests/e2e/utils/actions.ts +++ b/tests/e2e/utils/actions.ts @@ -404,6 +404,55 @@ export async function sendRequest( } } +/** + * Mock the next `sidebar:confirm-remove` IPC call to auto-confirm removal. + * Bypasses the native VS Code modal dialog which is hard to interact with in e2e. + */ +async function mockConfirmRemove(frame: Frame): Promise { + await frame.evaluate(() => { + const ipc = (window as any).ipcRenderer; + const originalInvoke = ipc.invoke.bind(ipc); + ipc.invoke = async (channel: string, ...args: any[]) => { + if (channel === 'sidebar:confirm-remove') { + ipc.invoke = originalInvoke; + return true; + } + return originalInvoke(channel, ...args); + }; + }); +} + +/** + * Remove a collection from the sidebar by opening the context menu, + * clicking Remove, and auto-confirming via IPC mock. + * + * @param page - Playwright Page (VS Code workbench) + * @param sidebar - The sidebar webview Frame + * @param collectionName - Name of the collection to remove + */ +export async function removeCollection( + page: Page, + sidebar: Frame, + collectionName: string +): Promise { + const collectionRow = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionName }); + await collectionRow.hover(); + + // Mock the confirmation dialog before triggering removal + await mockConfirmRemove(sidebar); + + // Open the 3-dot context menu + await collectionRow.locator('[data-testid="collection-actions"]').click(); + + // Click "Remove" from the dropdown + await sidebar.locator('[role="menuitem"]').filter({ hasText: 'Remove' }).click(); + + // Wait for the collection to disappear from the sidebar + await expect(collectionRow).not.toBeVisible({ timeout: 15_000 }); +} + /** * Run a VS Code command via the Command Palette. */ From f36ad48e3fff09479472d005b79bc1a665774c65 Mon Sep 17 00:00:00 2001 From: Chirag Chandrashekhar Date: Thu, 16 Apr 2026 20:52:36 +0530 Subject: [PATCH 2/2] fix: normalize posixified paths before UID hashing in collection IPC On Windows, the sidebar sends posixified collection paths (C:/Users/...) but generateUidBasedOnHash was called on them directly, producing a different UID than the native-path UID stored in Redux. This caused unlinkDir events from folder/request deletion to be silently dropped by the sidebar because collectionExists() couldn't match the UID. Wrap all webview-sourced collection paths in path.resolve() before hashing in renderer:delete-item, renderer:rename-item, and renderer:get-collection-tree handlers. Add e2e test for folder deletion visibility, plus createFolder and deleteItem test helpers with IPC mocks for native dialogs. --- src/extension/ipc/collection.ts | 8 +- tests/e2e/specs/collection-removal.spec.ts | 26 ++++++ tests/e2e/utils/actions.ts | 99 ++++++++++++++++++++++ 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/src/extension/ipc/collection.ts b/src/extension/ipc/collection.ts index 888df1d..4eb18b1 100644 --- a/src/extension/ipc/collection.ts +++ b/src/extension/ipc/collection.ts @@ -623,7 +623,7 @@ const registerCollectionIpc = (watcher: CollectionWatcherInterface): void => { // Collect request files before rename for UID mapping updates const itemCollectionPath = collectionPath; - const collectionUid = itemCollectionPath ? generateUidBasedOnHash(itemCollectionPath) : null; + const collectionUid = itemCollectionPath ? generateUidBasedOnHash(path.resolve(itemCollectionPath)) : null; const requestFiles = itemCollectionPath ? searchForRequestFiles(oldPath, itemCollectionPath) : []; fs.renameSync(oldPath, newPath); @@ -736,7 +736,9 @@ const registerCollectionIpc = (watcher: CollectionWatcherInterface): void => { } }; - const collectionUid = generateUidBasedOnHash(collectionPath); + // path.resolve converts posixified paths (from Redux) back to native separators + // so the UID matches what was generated when the collection was first opened + const collectionUid = generateUidBasedOnHash(path.resolve(collectionPath)); const stats = fs.statSync(itemPath); if (stats.isDirectory()) { const requestFiles = await searchForRequestFiles(itemPath, collectionPath); @@ -1427,7 +1429,7 @@ const registerCollectionIpc = (watcher: CollectionWatcherInterface): void => { return null; } - const uid = generateUidBasedOnHash(collectionPath); + const uid = generateUidBasedOnHash(path.resolve(collectionPath)); const collectionName = path.basename(collectionPath); const buildTree = (dirPath: string): Array<{ diff --git a/tests/e2e/specs/collection-removal.spec.ts b/tests/e2e/specs/collection-removal.spec.ts index 74b222f..180d0fc 100644 --- a/tests/e2e/specs/collection-removal.spec.ts +++ b/tests/e2e/specs/collection-removal.spec.ts @@ -3,6 +3,9 @@ import { openBrunoSidebar, createCollection, removeCollection, + createFolder, + deleteItem, + expandCollection, } from '../utils/actions'; test.describe('Collection removal', () => { @@ -74,4 +77,27 @@ test.describe('Collection removal', () => { await expect(matches).toHaveCount(1); }); + test('Deleted folder disappears from the sidebar', async ({ page, tmpDir }) => { + const sidebar = await openBrunoSidebar(page); + const collectionName = 'Folder Delete Test'; + const folderName = 'my-folder'; + + // Create a collection and a folder inside it + await createCollection(page, sidebar, collectionName, tmpDir); + await createFolder(sidebar, collectionName, folderName); + + // Expand the collection to see the folder + await expandCollection(sidebar, collectionName); + const folderRow = sidebar + .locator('[data-testid="sidebar-collection-item-row"]') + .filter({ hasText: folderName }); + await expect(folderRow).toBeVisible({ timeout: 10_000 }); + + // Delete the folder + await deleteItem(sidebar, folderName); + + // Verify it's gone + await expect(folderRow).not.toBeVisible({ timeout: 10_000 }); + }); + }); diff --git a/tests/e2e/utils/actions.ts b/tests/e2e/utils/actions.ts index 4898572..ff70718 100644 --- a/tests/e2e/utils/actions.ts +++ b/tests/e2e/utils/actions.ts @@ -453,6 +453,105 @@ export async function removeCollection( await expect(collectionRow).not.toBeVisible({ timeout: 15_000 }); } +/** + * Mock the next `sidebar:prompt-new-folder` IPC call to return a folder name. + * Bypasses the native VS Code input box. + */ +async function mockNewFolderPrompt(frame: Frame, folderName: string): Promise { + await frame.evaluate((name) => { + const ipc = (window as any).ipcRenderer; + const originalInvoke = ipc.invoke.bind(ipc); + ipc.invoke = async (channel: string, ...args: any[]) => { + if (channel === 'sidebar:prompt-new-folder') { + ipc.invoke = originalInvoke; + return name; + } + return originalInvoke(channel, ...args); + }; + }, folderName); +} + +/** + * Mock the next `sidebar:confirm-delete` IPC call to auto-confirm deletion. + * Bypasses the native VS Code confirmation dialog. + */ +async function mockConfirmDelete(frame: Frame): Promise { + await frame.evaluate(() => { + const ipc = (window as any).ipcRenderer; + const originalInvoke = ipc.invoke.bind(ipc); + ipc.invoke = async (channel: string, ...args: any[]) => { + if (channel === 'sidebar:confirm-delete') { + ipc.invoke = originalInvoke; + return true; + } + return originalInvoke(channel, ...args); + }; + }); +} + +/** + * Create a new folder inside a collection via the sidebar context menu. + * + * @param sidebar - The sidebar webview Frame + * @param collectionName - Name of the parent collection + * @param folderName - Name for the new folder + */ +export async function createFolder( + sidebar: Frame, + collectionName: string, + folderName: string +): Promise { + // Expand collection first so it's mounted + await expandCollection(sidebar, collectionName); + + const collectionRow = sidebar + .locator('[data-testid="sidebar-collection-row"]') + .filter({ hasText: collectionName }); + await collectionRow.hover(); + + // Mock the folder name input before opening the menu + await mockNewFolderPrompt(sidebar, folderName); + + // Open the 3-dot context menu + await collectionRow.locator('[data-testid="collection-actions"]').click(); + + // Click "New Folder" + await sidebar.locator('[role="menuitem"]').filter({ hasText: 'New Folder' }).click(); + + // Wait for the folder to appear in the sidebar + await expect( + sidebar.locator('[data-testid="sidebar-collection-item-row"]').filter({ hasText: folderName }) + ).toBeVisible({ timeout: 15_000 }); +} + +/** + * Delete a folder or request from the sidebar by right-clicking and confirming. + * + * @param sidebar - The sidebar webview Frame + * @param itemName - Name of the folder/request to delete + */ +export async function deleteItem( + sidebar: Frame, + itemName: string +): Promise { + const itemRow = sidebar + .locator('[data-testid="sidebar-collection-item-row"]') + .filter({ hasText: itemName }); + await itemRow.hover(); + + // Mock the confirmation dialog + await mockConfirmDelete(sidebar); + + // Open the context menu (3-dot icon on the item) + await itemRow.locator('[data-testid="collection-item-menu"]').click(); + + // Click "Delete" + await sidebar.locator('[role="menuitem"]').filter({ hasText: 'Delete' }).click(); + + // Wait for the item to disappear + await expect(itemRow).not.toBeVisible({ timeout: 15_000 }); +} + /** * Run a VS Code command via the Command Palette. */