mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(presets): coerce non-string catalog tags before joining (#3743)
* fix(presets): coerce non-string catalog tags before joining Preset catalog payloads are user-editable YAML/JSON, so a `tags:` list can legitimately contain non-strings (e.g. numeric tags). The preset list/search/info display paths and the catalog search backend joined tags with a raw `", ".join(...)` / used `t.lower()`, which raised `TypeError: sequence item N: expected str instance, int found` (or `AttributeError` on `.lower()`) and crashed the command. Sibling command surfaces already guard this — extensions, integrations, and workflows coerce with `str(t) for t in ...`. This aligns presets: - `_commands.py`: `preset list`, `preset search`, and both `preset info` branches now join `str(t) for t in ...`. - `__init__.py` `PresetCatalog.search`: tag filter uses `str(t).lower()` and the searchable-text join coerces tags to `str`. Adds regression tests driving `preset search` and `preset info` through CliRunner with numeric tags; both fail before the fix with the TypeError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -4504,7 +4504,7 @@ class PresetCatalog:
|
||||
continue
|
||||
|
||||
if tag and tag.lower() not in [
|
||||
t.lower() for t in pack_data.get("tags", [])
|
||||
str(t).lower() for t in pack_data.get("tags", [])
|
||||
]:
|
||||
continue
|
||||
|
||||
@@ -4516,7 +4516,7 @@ class PresetCatalog:
|
||||
pack_data.get("description", ""),
|
||||
pack_id,
|
||||
]
|
||||
+ pack_data.get("tags", [])
|
||||
+ [str(t) for t in pack_data.get("tags", [])]
|
||||
).lower()
|
||||
|
||||
if query_lower not in searchable_text:
|
||||
|
||||
@@ -61,7 +61,7 @@ def preset_list():
|
||||
console.print(f" [bold]{pack['name']}[/bold] ({pack['id']}) v{pack['version']} — {status} — priority {pri}")
|
||||
console.print(f" {pack['description']}")
|
||||
if pack.get("tags"):
|
||||
tags_str = ", ".join(pack["tags"])
|
||||
tags_str = _escape_markup(", ".join(str(t) for t in pack["tags"]))
|
||||
console.print(f" [dim]Tags: {tags_str}[/dim]")
|
||||
console.print(f" [dim]Templates: {pack['template_count']}[/dim]")
|
||||
console.print()
|
||||
@@ -288,7 +288,7 @@ def preset_search(
|
||||
console.print(f" [bold]{pack.get('name', pack['id'])}[/bold] ({pack['id']}) v{pack.get('version', '?')}")
|
||||
console.print(f" {pack.get('description', '')}")
|
||||
if pack.get("tags"):
|
||||
tags_str = ", ".join(pack["tags"])
|
||||
tags_str = ", ".join(str(t) for t in pack["tags"])
|
||||
console.print(f" [dim]Tags: {tags_str}[/dim]")
|
||||
console.print()
|
||||
|
||||
@@ -379,7 +379,7 @@ def preset_info(
|
||||
if local_pack.author:
|
||||
console.print(f" Author: {local_pack.author}")
|
||||
if local_pack.tags:
|
||||
console.print(f" Tags: {', '.join(local_pack.tags)}")
|
||||
console.print(f" Tags: {', '.join(str(t) for t in local_pack.tags)}")
|
||||
console.print(f" Templates: {len(local_pack.templates)}")
|
||||
for tmpl in local_pack.templates:
|
||||
console.print(f" - {tmpl['name']} ({tmpl['type']}): {tmpl.get('description', '')}")
|
||||
@@ -415,7 +415,7 @@ def preset_info(
|
||||
if pack_info.get("author"):
|
||||
console.print(f" Author: {pack_info['author']}")
|
||||
if pack_info.get("tags"):
|
||||
console.print(f" Tags: {', '.join(pack_info['tags'])}")
|
||||
console.print(f" Tags: {', '.join(str(t) for t in pack_info['tags'])}")
|
||||
if pack_info.get("repository"):
|
||||
console.print(f" Repository: {pack_info['repository']}")
|
||||
if pack_info.get("license"):
|
||||
|
||||
@@ -11705,3 +11705,70 @@ class TestEnsureConstitutionResolverAware:
|
||||
assert "{CORE_TEMPLATE}" not in content
|
||||
assert "# Ensure Wrapper" in content
|
||||
assert "[PROJECT_NAME]" in content
|
||||
|
||||
|
||||
class TestPresetTagsNonString:
|
||||
"""Non-string catalog tags must not crash preset display commands.
|
||||
|
||||
Catalog payloads are user-editable YAML/JSON, so a `tags:` list can contain
|
||||
numbers or other non-strings. The display path joins them; a raw
|
||||
``", ".join(...)`` blows up with ``TypeError: sequence item 0: expected str``.
|
||||
Sibling command surfaces (extensions/integrations/workflows) already guard
|
||||
this with ``str(t) for t in ...`` — presets must match.
|
||||
"""
|
||||
|
||||
def _seed_catalog(self, project_dir, tags):
|
||||
catalog = PresetCatalog(project_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog_data = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {
|
||||
"numeric-tags": {
|
||||
"name": "Numeric Tags",
|
||||
"description": "Preset with non-string tags",
|
||||
"version": "1.0.0",
|
||||
"tags": tags,
|
||||
},
|
||||
},
|
||||
}
|
||||
catalog.cache_file.write_text(json.dumps(catalog_data))
|
||||
catalog.cache_metadata_file.write_text(json.dumps({
|
||||
"cached_at": datetime.now(timezone.utc).isoformat(),
|
||||
}))
|
||||
return catalog
|
||||
|
||||
def test_search_renders_non_string_tags(self, project_dir):
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
catalog = self._seed_catalog(project_dir, [1, 2])
|
||||
default_only = [PresetCatalogEntry(
|
||||
url=catalog.DEFAULT_CATALOG_URL, name="default", priority=1, install_allowed=True
|
||||
)]
|
||||
|
||||
with patch.object(Path, "cwd", return_value=project_dir), \
|
||||
patch.object(PresetCatalog, "get_active_catalogs", return_value=default_only):
|
||||
result = CliRunner().invoke(app, ["preset", "search", "Numeric"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
plain = strip_ansi(result.output)
|
||||
assert "Tags: 1, 2" in plain
|
||||
|
||||
def test_info_renders_non_string_tags(self, project_dir):
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
catalog = self._seed_catalog(project_dir, [1, 2])
|
||||
default_only = [PresetCatalogEntry(
|
||||
url=catalog.DEFAULT_CATALOG_URL, name="default", priority=1, install_allowed=True
|
||||
)]
|
||||
|
||||
with patch.object(Path, "cwd", return_value=project_dir), \
|
||||
patch.object(PresetCatalog, "get_active_catalogs", return_value=default_only):
|
||||
result = CliRunner().invoke(app, ["preset", "info", "numeric-tags"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
plain = strip_ansi(result.output)
|
||||
assert "Tags: 1, 2" in plain
|
||||
|
||||
Reference in New Issue
Block a user