mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix: escape Rich markup in workflow resolve output (#3879)
`workflow resolve` printed two lines through `console.print`, which has
Rich markup enabled, without escaping:
1. The layer tier was wrapped in literal brackets:
`f" • [{layer.tier}] {layer.source} ..."`. Rich parsed `[base]`
and `[project-overlay]` as style tags, so the tier label was swallowed
on *every* invocation -- no untrusted input required. The column has
never rendered.
2. Step attribution interpolated `composed.step_id` raw. Step IDs come
from base-workflow / overlay YAML and are only validated against `:`
(see `_parse_edit`), so brackets pass validation. A balanced
`[stuff]` is silently swallowed; an unbalanced `[/red]` raises
`rich.errors.MarkupError`, producing an uncaught traceback and exit 1
-- the workflow cannot be inspected at all.
Route the interpolated fields through `rich.markup.escape` and escape
the literal tier bracket as `\[`, matching the existing pattern in
`workflow info`'s step graph and `workflow_list`'s `\[disabled]`.
Only display is affected; the returned payload was already unescaped and
is unchanged.
Adds 3 regression tests, all of which fail without the fix: the tier
label renders, and a step ID survives both the swallowing and the
crashing markup cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 5 (1M context)
This commit is contained in:
@@ -7,6 +7,7 @@ from typing import Any
|
||||
|
||||
import typer
|
||||
import yaml
|
||||
from rich.markup import escape as _escape_markup
|
||||
|
||||
from ..._console import console, err_console
|
||||
from ...extensions import normalize_priority
|
||||
@@ -412,14 +413,23 @@ def workflow_resolve(project_root: Path, workflow_id: str) -> dict[str, Any] | N
|
||||
priority = (
|
||||
"n/a" if layer.tier == "base" else str(normalize_priority(layer.priority))
|
||||
)
|
||||
# ``\[`` keeps the literal bracket: unescaped, Rich parses ``[base]`` /
|
||||
# ``[project-overlay]`` as a style tag and swallows the tier label whole.
|
||||
console.print(
|
||||
f" \u2022 [{layer.tier}] {layer.source} "
|
||||
f" \u2022 \\[{_escape_markup(layer.tier)}] "
|
||||
f"{_escape_markup(layer.source)} "
|
||||
f"(priority={priority})"
|
||||
)
|
||||
|
||||
console.print("Step attribution:")
|
||||
for composed in attribution:
|
||||
console.print(f" \u2022 {composed.step_id}: {composed.source}")
|
||||
# Step IDs come from base-workflow / overlay YAML, which only bans ``:``
|
||||
# \u2014 brackets pass validation, so they reach Rich as markup. A balanced
|
||||
# ``[stuff]`` is swallowed; an unbalanced ``[/red]`` raises MarkupError.
|
||||
console.print(
|
||||
f" \u2022 {_escape_markup(composed.step_id)}: "
|
||||
f"{_escape_markup(composed.source)}"
|
||||
)
|
||||
|
||||
return {
|
||||
"workflow_id": workflow_id,
|
||||
|
||||
@@ -509,6 +509,86 @@ class TestOverlayCli:
|
||||
assert payload["layers"][-1]["tier"] == "base"
|
||||
assert payload["layers"][-1]["priority"] is None
|
||||
|
||||
def test_workflow_resolve_prints_tier_labels(self, project_dir, monkeypatch):
|
||||
"""Layer tiers render literally; an unescaped ``[base]`` is eaten as markup."""
|
||||
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
||||
_write_workflow(
|
||||
project_dir,
|
||||
"wf",
|
||||
{
|
||||
"schema_version": "1.0",
|
||||
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
||||
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
||||
},
|
||||
)
|
||||
_write_overlay(
|
||||
project_dir,
|
||||
"wf",
|
||||
"ov1",
|
||||
{
|
||||
"id": "ov1",
|
||||
"extends": "wf",
|
||||
"priority": 10,
|
||||
"edits": [
|
||||
{
|
||||
"operation": "insert_after",
|
||||
"anchor": "a",
|
||||
"step": {"id": "new", "type": "command", "command": "echo"},
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
result = runner.invoke(app, ["workflow", "resolve", "wf"])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "[base]" in result.output
|
||||
assert "[project-overlay]" in result.output
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"step_id",
|
||||
[
|
||||
# Balanced tag: silently swallowed, so the step vanishes from output.
|
||||
"new[stuff]",
|
||||
# Unbalanced closer: raises MarkupError -> traceback and exit 1.
|
||||
"new[/red]",
|
||||
],
|
||||
)
|
||||
def test_workflow_resolve_escapes_rich_markup_in_step_id(
|
||||
self, project_dir, monkeypatch, step_id
|
||||
):
|
||||
"""Step IDs are unvalidated for brackets, so they must be escaped."""
|
||||
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
||||
_write_workflow(
|
||||
project_dir,
|
||||
"wf",
|
||||
{
|
||||
"schema_version": "1.0",
|
||||
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
||||
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
||||
},
|
||||
)
|
||||
_write_overlay(
|
||||
project_dir,
|
||||
"wf",
|
||||
"ov1",
|
||||
{
|
||||
"id": "ov1",
|
||||
"extends": "wf",
|
||||
"priority": 10,
|
||||
"edits": [
|
||||
{
|
||||
"operation": "insert_after",
|
||||
"anchor": "a",
|
||||
"step": {"id": step_id, "type": "command", "command": "echo"},
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
result = runner.invoke(app, ["workflow", "resolve", "wf"])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert step_id in result.output
|
||||
|
||||
def test_workflow_resolve_equal_priority_layers_sort_by_source(self, project_dir, monkeypatch):
|
||||
"""Equal-priority overlays are listed alphabetically by source."""
|
||||
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
||||
|
||||
Reference in New Issue
Block a user