Skip to content

[Routine - VT] fix(FileManager): use normalized membership check for add/remove files#79

Draft
winterdrive wants to merge 1 commit into
mainfrom
routine/vt-fix-filemanager-relpath-260706
Draft

[Routine - VT] fix(FileManager): use normalized membership check for add/remove files#79
winterdrive wants to merge 1 commit into
mainfrom
routine/vt-fix-filemanager-relpath-260706

Conversation

@winterdrive

Copy link
Copy Markdown
Owner

Pre-flight Check

Open PRs / active branches checked before selecting this fix:

This PR touches only src/core/FileManager.ts and adds a new test file src/test/unit/fileManagerRelpath.test.ts. No overlap with provider.ts, extension.ts, or any release metadata files.

Changes

FileManager.addFilesToGroup and FileManager.removeFilesFromGroup compared file paths against group.files using plain Array.includes() / Array.indexOf() against a freshly-built file:// URI. However, group.files can legitimately contain workspace-relative path strings (e.g. "src/foo.ts") instead of file:// URIs — this is the same class of bug already fixed for BookmarkManager.createBookmark in commit e12c0d9 via matchesStoredFileEntry.

Because these two FileManager methods back the MCP server's add_files_to_group / remove_files_from_group tools (mcp-server/src/tools/fileTools.tsmcp-server/src/managers/FileManager.ts, a re-export of this class), the bug was reachable from the MCP-facing API even though the in-editor path (provider.ts) already uses the normalized matcher:

  • addFilesToGroup would fail to recognize a relative-path entry as a duplicate and push a second (URI) entry for the same file instead of reporting it as skipped.
  • removeFilesFromGroup would fail to find a relative-path entry and incorrectly report it as notFound instead of removing it.

Fix: both methods now use matchesStoredFileEntry (from src/core/FileEntryMatcher.ts) with the group's workspace root, matching the pattern already used by BookmarkManager and GroupFileRemoval.

Confirms this PR does not modify: command registrations, package.json contributed configuration keys, dependencies, or the tab-group serialization/storage format. Only comparison logic inside two existing methods changed; the on-disk files array shape and virtualTab.json format are untouched.

Safety Verification

Commands run locally, all passed:

  • npx tsc -p ./ — passed, no errors
  • npm run test (tsc -p ./ && jest --runInBand) — 26 suites / 178 tests passed, including 3 new tests in src/test/unit/fileManagerRelpath.test.ts covering: skip-on-duplicate for a relative-path entry, remove-by-relative-path entry, and notFound still reported for genuinely absent files.
  • No lint or format:check script exists in package.json, so those steps were skipped.

CI / Release Gate Note

This is a daily routine Draft PR. Package version bump and CHANGELOG.md updates are intentionally deferred to the weekend release/integration PR. If CI fails solely due to the repository's version-bump release gate, that is expected release-readiness behavior for a routine PR, not a code validation failure.

group.files can hold workspace-relative entries (e.g. "src/foo.ts")
alongside file:// URIs, but addFilesToGroup/removeFilesFromGroup only
compared against file:// URIs via includes()/indexOf(). This let the
MCP-facing add_files_to_group/remove_files_from_group tools duplicate
a file already stored as a relative path, or silently no-op when
removing one. Mirrors the matchesStoredFileEntry fix already applied
to BookmarkManager.createBookmark (e12c0d9).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant