fix(validation): accept any-case SKILL.md filename on upload - #460
fix(validation): accept any-case SKILL.md filename on upload#460XiaoSeS wants to merge 1 commit into
Conversation
Users packaging skills on case-insensitive filesystems frequently submit
`skill.md` (lowercase). All five `equals("SKILL.md")` call sites then fail
with "Missing required file: SKILL.md at root".
Fold the basename to the canonical `SKILL.md` inside
`SkillPackagePolicy.normalizeEntryPath` — the single point both the
validator and the archive extractor route every path through — so every
downstream comparison succeeds without further changes.
There was a problem hiding this comment.
Code Review
This pull request introduces automatic canonicalization of the SKILL.md filename to ensure case-insensitive matching. It includes logic in SkillPackagePolicy to fold case variants (like skill.md or Skill.MD) to the uppercase SKILL.md and adds comprehensive unit tests in SkillPackageArchiveExtractorTest and SkillPackageValidatorTest to verify this behavior across different directory structures and validation scenarios. I have no feedback to provide.
|
PR Review Helper seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
这个方案我们采用了(extractor 层 canonicalize + 回归测试比只改 |
Closes #458.
Summary
skill.md(lowercase, or any other case variant) previously failed withMissing required file: SKILL.md at root. This was a frequent papercut on macOS / Windows because their case-insensitive filesystems happily let authors save the file with non-canonical casing.SKILL_MD_PATH = "SKILL.md"using case-sensitive.equals(...).SKILL.mdinsideSkillPackagePolicy.normalizeEntryPath(...)— the single normalization point bothSkillPackageValidatorandSkillPackageArchiveExtractoralready route every path through. Every downstream comparison then succeeds without further changes.The OpenSkills protocol declares uppercase
SKILL.mdas the canonical filename, so accepting any case but storing the canonical form is the right semantic — not a workaround.Test plan
SkillPackageArchiveExtractorTest— 3 new regression tests: lowercaseskill.mdat root, mixed-caseSkill.MDinside a single root directory, lowercaseskill.mdrequiring directory promotion. 18/18 pass.SkillPackageValidatorTest— 1 new test covering the validator-only path (e.g., CLI dry-run that doesn't go through the extractor). 17/17 pass.SkillPublishControllerTest,CliDryRunValidateTest,BasicPrePublishValidatorTest,SkillPublishServiceTest— no regressions. 27 tests, 0 failures.