From 41d7a839b9d0f0ab8dd988d666a07301deefa0c6 Mon Sep 17 00:00:00 2001 From: liuxuezhuo Date: Thu, 20 Aug 2026 16:15:35 +0800 Subject: [PATCH] fix(chat): refresh external context on nested folder re-selection Re-selecting a folder in the external context picker that nests inside (or contains) an existing entry previously showed a conflict Notice and left the list unchanged, which read as 'the workspace folder never updates'. The picker now applies refresh semantics: conflicting entries are replaced by the new pick (persistence inherited from a replaced locked entry), with a 'Replaced X with Y' Notice. Unrelated folders still append and exact duplicates are still rejected. Co-authored-by: QoderAI (Qwen 3.8 Max) --- CHANGELOG.md | 8 ++ src/core/context/external-context.ts | 25 ++++++ src/features/chat/ui/input-toolbar.ts | 43 +++++++++-- .../ui/input-toolbar.external-context.test.ts | 76 +++++++++++++++++++ 4 files changed, 144 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb3d0cd..fd57081 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,14 @@ version with its date and start a fresh empty `[Unreleased]` above it. - Editing a queued message now replaces the composer content instead of appending to it. +### Fixed + +- Re-selecting a folder in the external context picker that nests inside + (or contains) an existing entry now refreshes the selection: the + conflicting entry is replaced by the new pick (persistence inherited) + instead of being silently rejected, so the workspace folder visibly + updates after re-selection. + ## [1.0.5] - 2026-08-18 ### Added diff --git a/src/core/context/external-context.ts b/src/core/context/external-context.ts index e903b54..a478eca 100644 --- a/src/core/context/external-context.ts +++ b/src/core/context/external-context.ts @@ -50,6 +50,31 @@ export function findConflictingPath( return null; } +/** + * Returns every existing path that nests with the new path (either direction). + * Used by the folder picker's refresh semantics: re-selecting a nested folder + * replaces the conflicting entries instead of being rejected. + */ +export function findAllConflictingPaths( + newPath: string, + existingPaths: string[] +): PathConflict[] { + const normalizedNew = normalizePathForComparison(newPath); + const conflicts: PathConflict[] = []; + + for (const existing of existingPaths) { + const normalizedExisting = normalizePathForComparison(existing); + + if (normalizedNew.startsWith(normalizedExisting + '/')) { + conflicts.push({ path: existing, type: 'parent' }); + } else if (normalizedExisting.startsWith(normalizedNew + '/')) { + conflicts.push({ path: existing, type: 'child' }); + } + } + + return conflicts; +} + export function getFolderName(p: string): string { const normalized = normalizePathForDisplay(p); const segments = normalized.split('/'); diff --git a/src/features/chat/ui/input-toolbar.ts b/src/features/chat/ui/input-toolbar.ts index 0ec25d8..330bf82 100644 --- a/src/features/chat/ui/input-toolbar.ts +++ b/src/features/chat/ui/input-toolbar.ts @@ -2,7 +2,8 @@ import { Notice, setIcon } from 'obsidian'; import * as os from 'os'; import * as path from 'path'; -import { filterValidPaths, findConflictingPath, isDuplicatePath, isValidDirectoryPath, validateDirectoryPath } from '../../../core/context/external-context'; +import type { PathConflict } from '../../../core/context/external-context'; +import { filterValidPaths, findAllConflictingPaths, findConflictingPath, isDuplicatePath, isValidDirectoryPath, validateDirectoryPath } from '../../../core/context/external-context'; import { expandHomePath, normalizePathForFilesystem } from '../../../core/fs/path'; import type { ManagedMcpServer, @@ -261,14 +262,14 @@ export class ExternalContextSelector { return; } - // Check for nested/overlapping paths - const conflict = findConflictingPath(selectedPath, this.externalContextPaths); - if (conflict) { - new Notice(this.formatConflictMessage(selectedPath, conflict), 5000); - return; + // Re-selecting a nested folder refreshes the selection: conflicting + // entries are replaced by the new pick instead of being rejected. + const conflicts = findAllConflictingPaths(selectedPath, this.externalContextPaths); + if (conflicts.length > 0) { + this.replaceConflictingPaths(conflicts, selectedPath); + } else { + this.externalContextPaths = [...this.externalContextPaths, selectedPath]; } - - this.externalContextPaths = [...this.externalContextPaths, selectedPath]; this.onChangeCallback?.(this.externalContextPaths); this.updateDisplay(); this.renderDropdown(); @@ -278,6 +279,32 @@ export class ExternalContextSelector { } } + /** + * Refresh semantics for the folder picker: entries nesting with the + * re-selected folder are replaced by it. Persistence (lock) is inherited + * when any replaced entry was persistent. + */ + private replaceConflictingPaths(conflicts: PathConflict[], selectedPath: string): void { + const replaced = new Set(conflicts.map((conflict) => conflict.path)); + const hadPersistent = conflicts.some((conflict) => this.persistentPaths.has(conflict.path)); + + this.externalContextPaths = [ + ...this.externalContextPaths.filter((p) => !replaced.has(p)), + selectedPath, + ]; + + if (hadPersistent) { + for (const p of replaced) { + this.persistentPaths.delete(p); + } + this.persistentPaths.add(selectedPath); + this.onPersistenceChangeCallback?.([...this.persistentPaths]); + } + + const replacedNames = conflicts.map((conflict) => this.shortenPath(conflict.path)).join(', '); + new Notice(`Replaced ${replacedNames} with "${this.shortenPath(selectedPath)}"`, 5000); + } + /** Formats a conflict error message for display. */ private formatConflictMessage(newPath: string, conflict: { path: string; type: 'parent' | 'child' }): string { const shortNew = this.shortenPath(newPath); diff --git a/tests/unit/features/chat/ui/input-toolbar.external-context.test.ts b/tests/unit/features/chat/ui/input-toolbar.external-context.test.ts index ab98c56..6d82dd3 100644 --- a/tests/unit/features/chat/ui/input-toolbar.external-context.test.ts +++ b/tests/unit/features/chat/ui/input-toolbar.external-context.test.ts @@ -14,6 +14,22 @@ jest.mock('obsidian', () => ({ // Mock fs jest.mock('fs'); +// Mock electron remote dialog used by the native folder picker +jest.mock( + 'electron', + () => ({ + remote: { + dialog: { showOpenDialog: jest.fn() }, + }, + }), + { virtual: true } +); + +// eslint-disable-next-line @typescript-eslint/no-require-imports +const electronMock = require('electron') as { + remote: { dialog: { showOpenDialog: jest.Mock } }; +}; + // Mock callbacks function createMockCallbacks() { return { @@ -534,4 +550,64 @@ describe('ExternalContextSelector', () => { expect(onPersistenceChange).toHaveBeenCalledWith([]); }); }); + + describe('folder picker refresh semantics', () => { + const showOpenDialog = electronMock.remote.dialog.showOpenDialog; + + async function pickFolder(paths: string[]): Promise { + showOpenDialog.mockResolvedValue({ canceled: false, filePaths: paths }); + // openFolderPicker is private; exercised end-to-end here. + await (selector as unknown as { openFolderPicker(): Promise }).openFolderPicker(); + } + + beforeEach(() => { + showOpenDialog.mockReset(); + }); + + it('replaces a child entry when its parent is re-selected', async () => { + selector.addExternalContext('/vault/kb/qa'); + + await pickFolder(['/vault/kb']); + + expect(selector.getExternalContexts()).toEqual(['/vault/kb']); + }); + + it('replaces a parent entry when a child is re-selected', async () => { + selector.addExternalContext('/vault/kb'); + + await pickFolder(['/vault/kb/qa']); + + expect(selector.getExternalContexts()).toEqual(['/vault/kb/qa']); + }); + + it('inherits persistence from a replaced entry', async () => { + const onPersistenceChange = jest.fn(); + selector.setOnPersistenceChange(onPersistenceChange); + selector.addExternalContext('/vault/kb'); + selector.togglePersistence('/vault/kb'); + onPersistenceChange.mockClear(); + + await pickFolder(['/vault/kb/qa']); + + expect(selector.getExternalContexts()).toEqual(['/vault/kb/qa']); + expect(selector.getPersistentPaths()).toEqual(['/vault/kb/qa']); + expect(onPersistenceChange).toHaveBeenCalledWith(['/vault/kb/qa']); + }); + + it('still appends unrelated folders', async () => { + selector.addExternalContext('/vault/kb'); + + await pickFolder(['/other/docs']); + + expect(selector.getExternalContexts()).toEqual(['/vault/kb', '/other/docs']); + }); + + it('still rejects exact duplicates', async () => { + selector.addExternalContext('/vault/kb'); + + await pickFolder(['/vault/kb']); + + expect(selector.getExternalContexts()).toEqual(['/vault/kb']); + }); + }); });