Skip to content

fix(extensions): reject duplicate provides.templates/scripts names - #4016

Open
chelsealong wants to merge 1 commit into
github:mainfrom
chelsealong:fix/4012-followup-duplicate-provides-names
Open

fix(extensions): reject duplicate provides.templates/scripts names#4016
chelsealong wants to merge 1 commit into
github:mainfrom
chelsealong:fix/4012-followup-duplicate-provides-names

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #4012 (#4010), which merged before the last round of Copilot review feedback could be addressed. This carries forward the two outstanding items that are still valid against main:

  • provides.templates/provides.scripts entries with a duplicate name within the same section are now rejected with a ValidationError. The resolver (PresetResolver._extension_manifest_declared_template) returns the first entry matching a name, so a later duplicate was silently unreachable while still being exposed by ExtensionManifest.templates/.scripts — this closes that gap at manifest-validation time instead. Added parametrized regression tests for both templates and scripts.
  • extensions/EXTENSION-DEVELOPMENT-GUIDE.md's provides section now distinguishes its own sub-fields (commands/templates/scripts) from hooks/events, which are top-level manifest fields read from the manifest root, not nested under provides — the prior wording could lead an author to indent them under provides, where they wouldn't satisfy validation.

The other two items from that last review round were doc-only / description-only and don't apply here: #4012's PR description has already been corrected in place, and the AGENTS.md acceptance-criterion item was intentionally left alone since that file documents only the AI-agent integration subsystem, not the extension system (which has its own docs, already updated).

Testing

New parametrized test test_provides_entry_duplicate_name_rejected in tests/test_extensions.py::TestExtensionManifestTemplatesAndScripts covers both templates and scripts. Confirmed it fails with DID NOT RAISE ValidationError against the pre-fix source (git checkout HEAD~1 -- src/specify_cli/extensions/__init__.py), then passes after restoring the fix.

$ .venv/bin/python -m pytest tests/test_extensions.py tests/test_presets.py -q
====================== 1110 passed, 2 warnings in 11.30s ======================

$ uvx ruff@0.15.0 check src tests
All checks passed!

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Implemented autonomously by Claude Code (model: Claude Sonnet 5) under human direction, in response to outstanding automated review feedback on #4012 that had not been incorporated before that PR merged.

The resolver returns the first entry matching a declared name, so a
later duplicate within provides.templates or provides.scripts was
silently unreachable while still counted by ExtensionManifest
properties. Reject duplicates at manifest-validation time instead.

Also clarify EXTENSION-DEVELOPMENT-GUIDE.md's provides section: hooks
and events are top-level manifest fields, not provides sub-fields, so
the "at least one of ..." wording doesn't imply they can be nested
under provides.
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