fix(presets): reject duplicate provides.templates name+type entries - #4191
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
fix(presets): reject duplicate provides.templates name+type entries#4191Noor-ul-ain001 wants to merge 1 commit into
Noor-ul-ain001 wants to merge 1 commit into
Conversation
PresetResolver._manifest_declared_template returns the FIRST
'provides.templates' entry matching a given (name, type) pair:
for tmpl in manifest.templates:
if tmpl.get("name") == template_name and tmpl.get("type") == template_type:
...
return tmpl, ...
So a preset.yml declaring two templates with the same (name, type) --
e.g. two "command"/"specify" entries pointing at different files -- had
its second entry silently unreachable, while PresetManifest.templates
still counted and exposed both. PresetManifest._validate never checked
for this.
Reject the duplicate at manifest-validation time instead, matching the
sibling fix already applied to ExtensionManifest's provides.templates/
provides.scripts (commit 11e3176, PR github#4016): "The resolver returns the
first entry matching a declared name, so a later duplicate ... was
silently unreachable while still counted". Presets use a (name, type)
composite key rather than extensions' bare name, since the same name can
legitimately recur across different template types (e.g. a "specify"
template and a "specify" command); the fix only rejects a duplicate
within the exact same (name, type) pair.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PresetResolver._manifest_declared_templatereturns the firstprovides.templatesentry matching a given(name, type)pair:So a
preset.ymldeclaring two templates with the same(name, type)— e.g. twocommand/specifyentries pointing at different files — had its second entry silently unreachable, whilePresetManifest.templatesstill counted and exposed both.PresetManifest._validatenever checked for this:Reject the duplicate at manifest-validation time instead, matching the sibling fix already applied to
ExtensionManifest'sprovides.templates/provides.scripts(commit 11e3176, #4016): "The resolver returns the first entry matching a declared name, so a later duplicate ... was silently unreachable while still counted."Presets use a
(name, type)composite key rather than extensions' bare name, since the same name can legitimately recur across different template types (e.g. atemplate-type "specify" template and acommand-type "specify" command) — the fix only rejects a duplicate within the exact same(name, type)pair.Test plan
test_duplicate_template_name_and_type_raises_validation_errorandtest_same_name_different_type_templates_allowedtotests/test_presets.pyDID NOT RAISE) and passes with it; verified the same-name-different-type test passes both before and after (no false positive)pytest tests/test_presets.py— 602 passed, 8 pre-existing failures unrelated to this change (symlink-elevation tests that require admin rights on Windows, per this repo's documented Windows test-environment gaps), 2 skippedruff checkon both changed files — clean🤖 Generated with Claude Code