fix(bundler): reject unsupported schema_version in _merge_config (align readers)#3711
Open
jawwad-ali wants to merge 1 commit into
Open
fix(bundler): reject unsupported schema_version in _merge_config (align readers)#3711jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
…gn readers) bundle-catalogs.yml has two readers that are meant to agree: commands_impl/ catalog_config._read (bundle catalog list/add/remove) and models/catalog. _merge_config (the resolution path feeding bundle search/info/install via load_source_stack). _read rejects an unsupported MAJOR schema_version; _merge_config never checked it, so a file written by a newer/incompatible Spec Kit (e.g. schema_version '2.0') was silently parsed under v1 assumptions on the exact path where install_policy governs trust — the two readers disagreed. github#3623 (non-list catalogs) and github#3659 (top-level non-mapping) already aligned these two readers guard-by-guard; this is the last unaligned guard. Add the same forward-compatible major-version check to _merge_config. Promote CONFIG_SCHEMA_VERSION to models/catalog.py as the single source of truth and import it in catalog_config.py (was a local duplicate) so the two cannot drift. Absent schema_version stays valid (backward compatible); matching major stays valid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns both bundle catalog readers to reject unsupported major schema versions consistently.
Changes:
- Centralizes
CONFIG_SCHEMA_VERSION. - Adds schema validation to
_merge_config. - Tests incompatible, compatible, and absent versions.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/bundler/models/catalog.py |
Adds shared version constant and validation. |
src/specify_cli/bundler/commands_impl/catalog_config.py |
Imports the shared constant. |
tests/contract/test_catalog_schema.py |
Covers schema-version behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Medium
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.
Problem
.specify/bundle-catalogs.ymlhas two readers that are meant to agree:commands_impl/catalog_config._read— used byspecify bundle catalog list/add/removemodels/catalog._merge_config— used byload_source_stack, which feedsbundle search/info/installviaCatalogStack_readrejects an unsupported majorschema_version(its lines 54–64)._merge_configvalidates the top-level mapping and thecatalogslist shape but never checksschema_version. So a project/user config written by a future incompatible Spec Kit (e.g.schema_version: '2.0') is rejected by_readbut silently parsed under v1 assumptions by_merge_config— on the exact path (search/install) where a source'sinstall_policygoverns whether bundles may be installed from it. The two readers of the same file disagree.#3623(non-listcatalogs) and#3659(top-level non-mapping) already aligned these two readers guard-by-guard — the in-code comments say so. This is the last unaligned guard.Fix
_merge_config, immediately after its top-level-mapping guard (identical wording/logic to_read).CONFIG_SCHEMA_VERSION = "1.0"tomodels/catalog.pyas the single source of truth and import it incatalog_config.py(it was a local duplicate), so the two readers cannot drift apart.An absent
schema_versionstays valid (backward compatible with configs that omit it); a matching major (1.x) stays valid. Only a different major now raises — consistently across both readers.Verification
test_load_source_stack_rejects_unknown_schema_version(2.0): fails before (DID NOT RAISE), passes after.test_load_source_stack_accepts_matching_or_absent_schema_version(1.5and no key): passes — backward-compat regression guard.test_catalog_schema.py+test_bundler_catalog_config.py): all pass except one pre-existing Windows symlink-privilege test (WinError 1314, environmental — passes on CI/Linux).ruffclean.AI-assisted: authored with Claude Code. I reproduced the reader disagreement on
main(v2.0 accepted by_merge_config, rejected by_read) before adding the aligned guard.