fix(presets): tolerate non-string and non-list catalog fields in preset search/info - #3769
Open
Noor-ul-ain001 wants to merge 1 commit into
Open
Conversation
…et search/info `preset search` and `preset info` crashed with a raw traceback on catalog payloads that are valid YAML/JSON but not string-typed. Catalog files are user-editable, so these shapes reach the code unvalidated. `PresetCatalog.search` had three unguarded assumptions: - `--author` called `.lower()` on the raw value → `AttributeError: 'int' object has no attribute 'lower'` for `author: 789`. - the query searchable-text join passed raw `name`/`description` through → `TypeError: sequence item 0: expected str instance, int found`. - the `--tag` filter iterated `tags` without a list check, so a scalar `tags: 5` (truthy, not iterable) raised `TypeError: 'int' object is not iterable`. PR github#3743 fixed only the non-string *elements* of `tags` here; a non-list `tags` and the `author`/`name`/`description` fields were still unguarded. The sibling catalogs already handle all of these — `extensions/__init__.py` and `integrations/catalog.py` coerce with `str(...)` and gate on `isinstance(raw_tags, list)`. This aligns presets with them. The same scalar-`tags` crash reached the four display sites in `presets/_commands.py`, so those now gate on `isinstance(tags, list)`, matching `integrations/_query_commands.py`. Note `PresetManifest.tags` returns `self.data.get("tags", [])` and manifest validation does not enforce list-ness, so a local `preset.yaml` with `tags: 5` validates successfully and then crashed `preset info` — hence the guard on the local-manifest branch too. While here, `preset search` printed tags unescaped, so a tag containing `[bold]` was silently swallowed as a Rich style tag; it now routes through `_escape_markup` like the `preset list` line directly above it. Regression tests in `TestPresetTagsNonString` drive the full CLI path via CliRunner. All five fail before the fix, each with the exact exception it targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks
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
preset searchandpreset infocrash with a raw Python traceback on catalog payloads that are valid YAML/JSON but not string-typed. Catalog files are user-editable, so these shapes reach the code unvalidated.PresetCatalog.searchhad three unguarded assumptions:author: 789+--author 789AttributeError: 'int' object has no attribute 'lower'name: 123/description: 456+ a queryTypeError: sequence item 0: expected str instance, int foundtags: 5(scalar, not a list) +--tag ciTypeError: 'int' object is not iterableReproduced directly against
PresetCatalog.search:#3743 fixed only the non-string elements of
tagsin this method. A non-listtags, and theauthor/name/descriptionfields, were still unguarded.Fix
The sibling catalogs already handle every one of these cases —
extensions/__init__.py::searchandintegrations/catalog.py::searchcoerce withstr(...)and gate onisinstance(raw_tags, list). This PR aligns presets with that existing reference pattern rather than inventing a new one.The same scalar-
tagscrash also reached the four display sites inpresets/_commands.py, which now gate onisinstance(tags, list), matchingintegrations/_query_commands.py:331.That includes the local-manifest branch of
preset info:PresetManifest.tagsreturnsself.data.get("tags", [])and manifest validation does not enforce list-ness, so a localpreset.yamlcontainingtags: 5validates successfully and then crashed the display — the same validate-accepts / render-crashes split seen in prior fixes.While here:
preset searchprinted tags unescaped, so a tag containing[bold]was silently swallowed as a Rich style tag. It now routes through_escape_markup, like thepreset listline directly above it.Tests
Five regression tests added to
TestPresetTagsNonString, all driving the full CLI path viaCliRunner(testing only the display helper would have missed the backendsearchcrashes):test_search_by_author_tolerates_non_string_authortest_search_query_tolerates_non_string_name_and_descriptiontest_search_tolerates_non_list_tagstest_info_tolerates_non_list_tagstest_search_escapes_rich_markup_in_tagsEach fails before the fix with the exact exception it targets, verified by stashing the source changes:
tests/test_presets.pyafter the fix: 462 passed, 7 failed. The 7 failures are identical on cleanmain(457 passed, 7 failed) — pre-existingTestPresetSkillssymlink cases that need elevation on Windows. Net effect is +5 passing, no new failures.🤖 Generated with Claude Code