mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(workflows,extensions): tolerate non-list catalog tags in search/info display (#3770)
`workflow search`, `workflow info`, `extension search` and `extension info`
crashed with `TypeError: 'int' object is not iterable` when a catalog entry
carried a scalar `tags:` value (e.g. `tags: 5`). Catalog payloads are
user-editable YAML/JSON, so this shape reaches the display unvalidated.
Both backends already guard their tag *filter* with
`isinstance(raw_tags, list)` — `WorkflowCatalog.search` and
`ExtensionCatalog.search` skip a non-list `tags` cleanly. Only the display
paths were unguarded: they tested truthiness (`if info.get("tags"):`) and
then iterated. A scalar is truthy but not iterable, so `--tag` filtering
survived while plain `search`/`info` rendering blew up.
Note this is distinct from the non-string *element* handling added in
#3746/#3747: coercing elements with `str(t) for t in ...` does not help when
`tags` is not a sequence at all. The fix is the guard the sibling
integration commands already use — `integrations/_query_commands.py:332,402`
gate on `isinstance(tags, list) and tags`. This aligns workflows and
extensions with that reference pattern, leaving all four tag-join display
sites in these modules consistent.
Regression tests drive the full CLI via CliRunner and cover search + info in
one case per module; both fail before the fix with the exact TypeError.
Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -801,8 +801,9 @@ def extension_search(
|
||||
|
||||
# Metadata
|
||||
console.print(f"\n [dim]Author:[/dim] {_escape_markup(str(ext.get('author', 'Unknown')))}")
|
||||
if ext.get('tags'):
|
||||
tags_str = ", ".join(str(t) for t in ext['tags'])
|
||||
ext_tags = ext.get('tags', [])
|
||||
if isinstance(ext_tags, list) and ext_tags:
|
||||
tags_str = ", ".join(str(t) for t in ext_tags)
|
||||
console.print(f" [dim]Tags:[/dim] {_escape_markup(tags_str)}")
|
||||
|
||||
# Source catalog
|
||||
@@ -1025,8 +1026,9 @@ def _print_extension_info(ext_info: dict, manager):
|
||||
console.print()
|
||||
|
||||
# Tags
|
||||
if ext_info.get('tags'):
|
||||
tags_str = ", ".join(str(t) for t in ext_info['tags'])
|
||||
info_tags = ext_info.get('tags', [])
|
||||
if isinstance(info_tags, list) and info_tags:
|
||||
tags_str = ", ".join(str(t) for t in info_tags)
|
||||
console.print(f"[bold]Tags:[/bold] {_escape_markup(tags_str)}")
|
||||
console.print()
|
||||
|
||||
|
||||
@@ -2326,7 +2326,7 @@ def workflow_search(
|
||||
if desc:
|
||||
console.print(f" {_escape_markup(str(desc))}")
|
||||
tags = wf.get("tags", [])
|
||||
if tags:
|
||||
if isinstance(tags, list) and tags:
|
||||
safe_tags = _escape_markup(", ".join(str(t) for t in tags))
|
||||
console.print(f" [dim]Tags: {safe_tags}[/dim]")
|
||||
console.print()
|
||||
@@ -2424,8 +2424,9 @@ def workflow_info(
|
||||
console.print(f" Version: {_escape_markup(str(info.get('version', '?')))}")
|
||||
if info.get("description"):
|
||||
console.print(f" Description: {_escape_markup(str(info['description']))}")
|
||||
if info.get("tags"):
|
||||
safe_tags = _escape_markup(", ".join(str(t) for t in info["tags"]))
|
||||
info_tags = info.get("tags", [])
|
||||
if isinstance(info_tags, list) and info_tags:
|
||||
safe_tags = _escape_markup(", ".join(str(t) for t in info_tags))
|
||||
console.print(f" Tags: {safe_tags}")
|
||||
console.print(" [yellow]Not installed[/yellow]")
|
||||
else:
|
||||
|
||||
@@ -4412,6 +4412,44 @@ class TestExtensionCatalog:
|
||||
results = catalog.search(query="jira")
|
||||
assert {r["id"] for r in results} == {"jira"}
|
||||
|
||||
def test_search_and_info_tolerate_non_list_tags(self, temp_dir):
|
||||
"""A scalar ``tags:`` value must not crash the search/info display.
|
||||
|
||||
``ExtensionCatalog.search`` guards its tag *filter* with
|
||||
``isinstance(raw_tags, list)``, but the ``extension search`` and
|
||||
``extension info`` display paths only tested truthiness before
|
||||
iterating. ``tags: 5`` is truthy and not iterable, so both raised
|
||||
``TypeError: 'int' object is not iterable``.
|
||||
"""
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch
|
||||
from specify_cli import app
|
||||
|
||||
project_dir = temp_dir / "project"
|
||||
project_dir.mkdir()
|
||||
(project_dir / ".specify").mkdir()
|
||||
|
||||
merged = [{
|
||||
"id": "jira",
|
||||
"name": "Jira",
|
||||
"version": "1.0.0",
|
||||
"description": "Jira",
|
||||
"tags": 5,
|
||||
}]
|
||||
|
||||
with patch.object(ExtensionCatalog, "_get_merged_extensions", return_value=merged), \
|
||||
patch("specify_cli.extensions._commands._require_specify_project",
|
||||
return_value=project_dir):
|
||||
searched = CliRunner().invoke(app, ["extension", "search", "Jira"])
|
||||
info = CliRunner().invoke(app, ["extension", "info", "jira"])
|
||||
|
||||
assert searched.exit_code == 0, searched.output
|
||||
assert "Jira" in searched.output
|
||||
assert "Tags:" not in searched.output
|
||||
|
||||
assert info.exit_code == 0, info.output
|
||||
assert "Tags:" not in info.output
|
||||
|
||||
def test_search_tolerates_non_string_author_and_name(self, temp_dir):
|
||||
"""Non-string catalog author/name must not crash author/query search.
|
||||
|
||||
|
||||
@@ -10560,6 +10560,44 @@ steps:
|
||||
assert "desc [with] brackets" in result.output
|
||||
assert "tag[1]" in result.output
|
||||
|
||||
def test_search_and_info_tolerate_non_list_tags(self, project_dir, monkeypatch):
|
||||
"""A scalar ``tags:`` value must not crash the search/info display.
|
||||
|
||||
``WorkflowCatalog.search`` guards its tag *filter* with
|
||||
``isinstance(raw_tags, list)``, but the ``workflow search`` and
|
||||
``workflow info`` display paths only tested truthiness before
|
||||
iterating. ``tags: 5`` is truthy and not iterable, so both raised
|
||||
``TypeError: 'int' object is not iterable``.
|
||||
"""
|
||||
from typer.testing import CliRunner
|
||||
from specify_cli import app
|
||||
from specify_cli.workflows.catalog import WorkflowCatalog
|
||||
|
||||
monkeypatch.chdir(project_dir)
|
||||
workflows = {
|
||||
"wf-a": {
|
||||
"name": "Workflow A",
|
||||
"version": "1.0.0",
|
||||
"description": "desc",
|
||||
"tags": 5,
|
||||
},
|
||||
}
|
||||
monkeypatch.setattr(
|
||||
WorkflowCatalog,
|
||||
"_get_merged_workflows",
|
||||
lambda self, force_refresh=False: {k: dict(v) for k, v in workflows.items()},
|
||||
)
|
||||
runner = CliRunner()
|
||||
searched = runner.invoke(app, ["workflow", "search"])
|
||||
info = runner.invoke(app, ["workflow", "info", "wf-a"])
|
||||
|
||||
assert searched.exit_code == 0, searched.output
|
||||
assert "Workflow A" in searched.output
|
||||
assert "Tags:" not in searched.output
|
||||
|
||||
assert info.exit_code == 0, info.output
|
||||
assert "Tags:" not in info.output
|
||||
|
||||
def test_catalog_list_escapes_rich_markup(self, project_dir, monkeypatch):
|
||||
"""User-editable catalog name/url/description must not be parsed as Rich markup."""
|
||||
from typer.testing import CliRunner
|
||||
|
||||
Reference in New Issue
Block a user