fix: escape Rich markup in workflow resolve output - #3879
Merged
Conversation
`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)
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Rich markup handling in workflow resolve output.
Changes:
- Escapes tier, source, and step-attribution values.
- Adds regression tests for tier labels and bracket-containing step IDs.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/overlays/_commands.py |
Escapes Rich markup in resolution summaries. |
tests/workflows/test_overlay_commands.py |
Tests literal tiers and markup-like step IDs. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
Collaborator
|
Thank you! |
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
specify workflow resolve <id>prints its two summary sections throughconsole.print, which has Rich markup enabled. Neither line escaped its interpolated fields.1. The tier label never rendered — on every invocation.
f" • [{layer.tier}] {layer.source} (priority={priority})"Rich parses
[base]and[project-overlay]as style tags, so the bracketed tier is swallowed. This needs no untrusted input; the column has simply never worked:2. Step IDs reach Rich as markup.
Step IDs come from base-workflow / overlay YAML and are validated only against
:(_parse_editinoverlays/schema.py) — brackets pass validation. Two failure modes:[stuff]is silently swallowed, sonew[stuff]prints asnewand the step appears to vanish.[/red]raisesrich.errors.MarkupError→ uncaught traceback, exit 1. The workflow can't be inspected at all:Fix
Route the interpolated fields through
rich.markup.escapeand escape the literal tier bracket as\[. This matches the pattern already used byworkflow info's step graph andworkflow_list's\[disabled].Display-only: the returned payload was already unescaped and is unchanged, so machine-readable callers see no difference.
Tests
Three regression tests in
TestOverlayCli, all verified to fail without the fix:test_workflow_resolve_prints_tier_labels— asserts[base]and[project-overlay]appear in outputtest_workflow_resolve_escapes_rich_markup_in_step_id— parametrized overnew[stuff](swallowed) andnew[/red](crashes)tests/workflows/test_overlay_commands.pypasses 23/23. The widertests/workflows/suite shows 10 pre-existing failures, all symlink-permission tests that fail identically on a clean tree on Windows without elevation.🤖 Generated with Claude Code