Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 53 additions & 19 deletions src/specify_cli/presets/_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ def preset_list():
for pack in installed:
status = "[green]enabled[/green]" if pack.get("enabled", True) else "[red]disabled[/red]"
pri = pack.get('priority', 10)
console.print(f" [bold]{pack['name']}[/bold] ({pack['id']}) v{pack['version']} — {status} — priority {pri}")
console.print(f" {pack['description']}")
name = _escape_markup(str(pack['name']))
pack_id = _escape_markup(str(pack['id']))
version = _escape_markup(str(pack['version']))
console.print(f" [bold]{name}[/bold] ({pack_id}) v{version} — {status} — priority {pri}")
console.print(f" {_escape_markup(str(pack['description']))}")
tags = pack.get("tags", [])
if isinstance(tags, list) and tags:
tags_str = _escape_markup(", ".join(str(t) for t in tags))
Expand Down Expand Up @@ -317,13 +320,20 @@ def preset_resolve(
project_root = _require_specify_project()
resolver = PresetResolver(project_root)
layers = resolver.collect_all_layers(template_name)
safe_template_name = _escape_markup(str(template_name))

if layers:
# Use the highest-priority layer for display because the final output
# may be composed and may not map to resolve_with_source()'s single path.
display_layer = layers[0]
console.print(f" [bold]{template_name}[/bold]: {display_layer['path']}")
console.print(f" [dim](top layer from: {display_layer['source']})[/dim]")
console.print(
f" [bold]{safe_template_name}[/bold]: "
f"{_escape_markup(str(display_layer['path']))}"
)
console.print(
f" [dim](top layer from: "
f"{_escape_markup(str(display_layer['source']))})[/dim]"
)

has_composition = (
layers[0]["strategy"] != "replace"
Expand All @@ -335,7 +345,10 @@ def preset_resolve(
composed = resolver.resolve_content(template_name)
except Exception as exc:
composed = None
console.print(f" [yellow]Warning: composition error: {exc}[/yellow]")
console.print(
f" [yellow]Warning: composition error: "
f"{_escape_markup(str(exc))}[/yellow]"
)
if composed is None:
console.print(" [yellow]Warning: composition cannot produce output (no base layer with 'replace' strategy)[/yellow]")
else:
Expand All @@ -358,15 +371,27 @@ def preset_resolve(
strategy_label = layer["strategy"]
if strategy_label == "replace" and i == 0:
strategy_label = "base"
console.print(f" {i + 1}. [{strategy_label}] {layer['source']} → {layer['path']}")
# Escape the literal bracket (\[) so Rich renders `[<strategy>]`
# instead of parsing it as a style tag and swallowing the label,
# mirroring `workflow info`'s step-graph line.
console.print(
f" {i + 1}. \\[{_escape_markup(str(strategy_label))}] "
f"{_escape_markup(str(layer['source']))} → "
f"{_escape_markup(str(layer['path']))}"
)
else:
# No layers found — fall back to resolve_with_source for non-composition cases
result = resolver.resolve_with_source(template_name)
if result:
console.print(f" [bold]{template_name}[/bold]: {result['path']}")
console.print(f" [dim](from: {result['source']})[/dim]")
console.print(
f" [bold]{safe_template_name}[/bold]: "
f"{_escape_markup(str(result['path']))}"
)
console.print(
f" [dim](from: {_escape_markup(str(result['source']))})[/dim]"
)
else:
console.print(f" [yellow]{template_name}[/yellow]: not found")
console.print(f" [yellow]{safe_template_name}[/yellow]: not found")
console.print(" [dim]No template with this name exists in the resolution stack[/dim]")


Expand All @@ -386,24 +411,32 @@ def preset_info(
local_pack = manager.get_pack(preset_id)

if local_pack:
console.print(f"\n[bold cyan]Preset: {local_pack.name}[/bold cyan]\n")
console.print(f" ID: {local_pack.id}")
console.print(f" Version: {local_pack.version}")
console.print(f" Description: {local_pack.description}")
console.print(
f"\n[bold cyan]Preset: {_escape_markup(str(local_pack.name))}[/bold cyan]\n"
)
console.print(f" ID: {_escape_markup(str(local_pack.id))}")
console.print(f" Version: {_escape_markup(str(local_pack.version))}")
console.print(
f" Description: {_escape_markup(str(local_pack.description))}"
)
if local_pack.author:
console.print(f" Author: {local_pack.author}")
console.print(f" Author: {_escape_markup(str(local_pack.author))}")
local_tags = local_pack.tags
if isinstance(local_tags, list) and local_tags:
console.print(f" Tags: {', '.join(str(t) for t in local_tags)}")
tags_str = _escape_markup(", ".join(str(t) for t in local_tags))
console.print(f" Tags: {tags_str}")
console.print(f" Templates: {len(local_pack.templates)}")
for tmpl in local_pack.templates:
console.print(f" - {tmpl['name']} ({tmpl['type']}): {tmpl.get('description', '')}")
tmpl_name = _escape_markup(str(tmpl['name']))
tmpl_type = _escape_markup(str(tmpl['type']))
tmpl_desc = _escape_markup(str(tmpl.get('description', '')))
console.print(f" - {tmpl_name} ({tmpl_type}): {tmpl_desc}")
repo = local_pack.data.get("preset", {}).get("repository")
if repo:
console.print(f" Repository: {repo}")
console.print(f" Repository: {_escape_markup(str(repo))}")
license_val = local_pack.data.get("preset", {}).get("license")
if license_val:
console.print(f" License: {license_val}")
console.print(f" License: {_escape_markup(str(license_val))}")
console.print("\n [green]Status: installed[/green]")
# Get priority from registry
pack_metadata = manager.registry.get(preset_id)
Expand Down Expand Up @@ -438,7 +471,8 @@ def preset_info(
)
catalog_tags = pack_info.get("tags", [])
if isinstance(catalog_tags, list) and catalog_tags:
console.print(f" Tags: {', '.join(str(t) for t in catalog_tags)}")
catalog_tags_str = _escape_markup(", ".join(str(t) for t in catalog_tags))
console.print(f" Tags: {catalog_tags_str}")
Comment thread
mnriem marked this conversation as resolved.
if pack_info.get("repository"):
console.print(
f" Repository: {_escape_markup(str(pack_info['repository']))}"
Expand Down
210 changes: 210 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -12170,3 +12170,213 @@ def test_info_escapes_catalog_markup(self, project_dir):
):
value = self.MARKUP_PRESET[field]
assert value in output
# Tags are joined into a single line, so assert on the rendered join.
assert ", ".join(self.MARKUP_PRESET["tags"]) in output


class TestInstalledPresetRichMarkup:
"""Locally installed preset metadata must render as literal text.

``preset.yml`` is user-editable, so its fields can contain ``[...]``.
``TestPresetCatalogRichMarkup`` covers the catalog branch of these
commands; the installed-preset branch of ``preset list``/``preset info``
and all of ``preset resolve`` were left unescaped, so a field like
``Does [stuff] nicely`` silently rendered as ``Does nicely`` and an
unbalanced tag such as ``[/red]`` raised ``rich.errors.MarkupError``,
aborting the command with a traceback.
"""

MARKUP_FIELDS = {
"name": "[green]Markup Name[/green]",
"version": "1.0.0",
"description": "[yellow]Markup Description[/yellow]",
"author": "[magenta]Markup Author[/magenta]",
"repository": "[bold]Markup Repository[/bold]",
"license": "[cyan]Markup License[/cyan]",
}

def _install(self, temp_dir, project_dir, preset_overrides=None, strategy=None,
pack_id="markup-pack", priority=10, tmpl_description=None):
"""Install a preset from a directory built with the given manifest fields."""
from specify_cli.presets import PresetManager

src = temp_dir / f"src-{pack_id}"
(src / "templates").mkdir(parents=True)
(src / "templates" / "spec-template.md").write_text("# tmpl\n")

preset_section = {
"id": pack_id,
"name": pack_id,
"version": "1.0.0",
"description": "plain description",
}
preset_section.update(preset_overrides or {})
tmpl = {
"type": "template",
"name": "spec-template",
"file": "templates/spec-template.md",
}
if tmpl_description is not None:
tmpl["description"] = tmpl_description
if strategy:
tmpl["strategy"] = strategy
(src / "preset.yml").write_text(yaml.dump({
"schema_version": "1.0",
"preset": preset_section,
"requires": {"speckit_version": ">=0.0.1"},
"provides": {"templates": [tmpl]},
"tags": ["[italic]markup-tag[/italic]"],
}))

manager = PresetManager(project_dir)
manager.install_from_directory(src, "9.9.9", priority)
return manager

def _invoke(self, project_dir, args):
from typer.testing import CliRunner
from unittest.mock import patch
from specify_cli import app

with patch.object(Path, "cwd", return_value=project_dir):
return CliRunner().invoke(app, args)

def test_list_and_info_escape_installed_markup(self, temp_dir, project_dir):
"""Every ``preset.yml`` field must survive verbatim in list/info output."""
self._install(temp_dir, project_dir, preset_overrides=self.MARKUP_FIELDS)

for args in (["preset", "list"], ["preset", "info", "markup-pack"]):
result = self._invoke(project_dir, args)
assert result.exit_code == 0, result.output
output = " ".join(strip_ansi(result.output).split())
# `preset list` does not render repository/license.
fields = ("name", "description") if args[1] == "list" else self.MARKUP_FIELDS
for field in fields:
assert self.MARKUP_FIELDS[field] in output, (field, args, output)
assert "[italic]markup-tag[/italic]" in output, (args, output)

def test_info_does_not_swallow_template_description(self, temp_dir, project_dir):
"""The per-template line in ``preset info`` must escape the template description.

``name``/``type`` are format-restricted by manifest validation, but
``description`` is free-form, so it is the field that can carry markup.
"""
self._install(
temp_dir,
project_dir,
tmpl_description="Template [desc] here",
)
result = self._invoke(project_dir, ["preset", "info", "markup-pack"])
assert result.exit_code == 0, result.output
output = " ".join(strip_ansi(result.output).split())
assert "spec-template (template): Template [desc] here" in output, output

def test_unbalanced_markup_does_not_crash_list_or_info(self, temp_dir, project_dir):
"""An unbalanced tag must not raise MarkupError and abort the command."""
self._install(
temp_dir,
project_dir,
preset_overrides={"description": "Broken [/red] tag"},
)

for args in (["preset", "list"], ["preset", "info", "markup-pack"]):
result = self._invoke(project_dir, args)
assert result.exit_code == 0, (args, result.output, result.exception)
assert "Broken [/red] tag" in strip_ansi(result.output)

def test_resolve_escapes_template_name(self, project_dir):
"""``preset resolve`` echoes its argument; an unbalanced tag must not crash."""
result = self._invoke(project_dir, ["preset", "resolve", "no[/red]such"])
assert result.exit_code == 0, (result.output, result.exception)
assert "no[/red]such" in strip_ansi(result.output)
Comment thread
mnriem marked this conversation as resolved.

def test_resolve_escapes_layer_path_and_source(self, project_dir):
"""The top-layer path/source lines must render markup literally.

A preset can be installed from any directory, so the resolved path can
contain ``[...]``; the layer source carries the pack id and version.
"""
from unittest.mock import patch
from specify_cli.presets import PresetResolver

# A closing tag cannot live inside a path segment: `Path` treats its
# `/` as a separator on POSIX and rewrites it to `\` on Windows. The
# opening tag covers the swallowing case for the path; the unbalanced
# closing tag rides on `source`, which is a plain string.
layer = {
"path": Path("/tmp/[red]dir/spec-template.md"),
"source": "pack [/red] v1.0.0",
"strategy": "replace",
}
with patch.object(PresetResolver, "collect_all_layers", return_value=[layer]):
result = self._invoke(project_dir, ["preset", "resolve", "spec-template"])

assert result.exit_code == 0, (result.output, result.exception)
output = " ".join(strip_ansi(result.output).split())
assert "[red]dir" in output, output
assert "pack [/red] v1.0.0" in output, output

def test_resolve_escapes_fallback_path_and_source(self, project_dir):
"""The no-layer fallback branch must escape ``resolve_with_source`` output."""
from unittest.mock import patch
from specify_cli.presets import PresetResolver

with patch.object(
PresetResolver, "collect_all_layers", return_value=[]
), patch.object(
PresetResolver,
"resolve_with_source",
return_value={
"path": "/tmp/[blue]fallback[/blue]/spec-template.md",
"source": "fallback [/red] source",
},
):
result = self._invoke(project_dir, ["preset", "resolve", "spec-template"])

assert result.exit_code == 0, (result.output, result.exception)
output = " ".join(strip_ansi(result.output).split())
assert "[blue]fallback[/blue]" in output, output
assert "fallback [/red] source" in output, output

def test_resolve_escapes_composition_error(self, project_dir):
"""A composition exception message must not be parsed as markup."""
from unittest.mock import patch
from specify_cli.presets import PresetResolver

layers = [
{
"path": Path("/tmp/top/spec-template.md"),
"source": "top-pack v1.0.0",
"strategy": "append",
},
{
"path": Path("/tmp/base/spec-template.md"),
"source": "base-pack v1.0.0",
"strategy": "append",
},
]
with patch.object(
PresetResolver, "collect_all_layers", return_value=layers
), patch.object(
PresetResolver,
"resolve_content",
side_effect=RuntimeError("compose failed: [/red] bad layer"),
):
result = self._invoke(project_dir, ["preset", "resolve", "spec-template"])

assert result.exit_code == 0, (result.output, result.exception)
output = " ".join(strip_ansi(result.output).split())
assert "compose failed: [/red] bad layer" in output, output

def test_resolve_renders_composition_strategy_labels(self, temp_dir, project_dir):
"""The composition chain's ``[<strategy>]`` label must not be eaten as a tag."""
self._install(temp_dir, project_dir, strategy="replace",
pack_id="base-pack", priority=20)
self._install(temp_dir, project_dir, strategy="append",
pack_id="app-pack", priority=5)

result = self._invoke(project_dir, ["preset", "resolve", "spec-template"])
assert result.exit_code == 0, (result.output, result.exception)
output = strip_ansi(result.output)
assert "Composition chain" in output, output
assert "[base]" in output, output
assert "[append]" in output, output