Skip to content

Dedup skill files by exact path, not case-folded path#531

Open
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanave:fix-fsskills-case-dedup
Open

Dedup skill files by exact path, not case-folded path#531
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanave:fix-fsskills-case-dedup

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

Problem

discoverResourceFiles and discoverScriptFiles deduplicate discovered files using a case-folded key:

key := strings.ToLower(filePath)
if seen[key] { return }
seen[key] = true

but keep the original-case path as the file Name. On a case-sensitive filesystem (Linux — the primary CI target) two genuinely distinct files in the same skill directory that differ only in case collapse to one seen key, so the second is silently dropped:

  • Data.json and data.json → only one resource discovered
  • Run.py and run.py → only one script discovered

The lowercasing isn't needed for the dedup's actual purpose (guarding against the same path being visited twice) and has no comment justifying case-insensitivity.

Fix

Dedup by the exact filePath. Same-path revisits are still collapsed; distinct files differing only in case are both kept.

Test

TestFileSource_CaseSensitiveFS_KeepsDistinctlyCasedFiles (black-box) uses a case-sensitive fstest.MapFS (map keys are always case-sensitive, unlike os.DirFS on a case-insensitive host such as macOS) containing Data.json/data.json and Run.py/run.py, and asserts 2 resources and 2 scripts are discovered. Fails on main (gets 1 each), passes with the fix.

discoverResourceFiles and discoverScriptFiles deduplicated discovered
files using strings.ToLower(filePath) as the seen-set key while keeping
the original-case path as the file Name. On a case-sensitive filesystem
(e.g. Linux, the primary CI target) two genuinely distinct files in the
same skill directory that differ only in case — Data.json vs data.json,
or Run.py vs run.py — collapse to one key, so the second is silently
dropped.

Dedup by the exact path instead, which still guards against visiting the
same file twice but no longer conflates distinct files. Adds a black-box
test using a case-sensitive fstest.MapFS.
Copilot AI review requested due to automatic review settings July 17, 2026 19:02
@PratikDhanave
PratikDhanave requested a review from a team as a code owner July 17, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes file discovery deduplication in the file-system-backed skills source so that it deduplicates by the exact discovered path rather than a case-folded path, preserving distinct files that differ only by case on case-sensitive filesystems (e.g., Linux).

Changes:

  • Update discoverResourceFiles to deduplicate by filePath (exact path) instead of strings.ToLower(filePath).
  • Update discoverScriptFiles to deduplicate by filePath (exact path) instead of strings.ToLower(filePath).
  • Add a regression test using fstest.MapFS to ensure distinctly-cased resource and script files are both discovered.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
agent/skills/fsskills/source.go Switch resource/script dedup keys from case-folded paths to exact paths to avoid collapsing distinct files on case-sensitive FS.
agent/skills/fsskills/source_script_test.go Add a black-box regression test validating discovery keeps Data.json/data.json and Run.py/run.py as separate entries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants