From 046aa89d002113bc11e21677baac9a1828c09504 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 21 Aug 2026 11:34:06 -0400 Subject: [PATCH] fix(witan): escape stored text at the CLI's render boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rich reads `[...]` in `Console.print` as a style tag, so stored content printed straight into a markup string loses every bracketed substring. Closing a task printed its resolution back as "code_transport is not set on , so code graphs remain local" — `[targets.production]`, the part naming which target was misconfigured, was the part that went. `task_get` showed the stored text intact, so this was display-only throughout. Rich drops an unresolvable style silently; `[rank]`, `[targets.production]` and a markdown `[link]` all just vanish. A bracketed absolute path is worse: `[/var/lib/witan]` parses as a closing tag with nothing open and raises MarkupError, taking the command down. #261 escaped the four `witan serve` startup sites it had just written, one call site at a time, and every other renderer stayed broken — which is the argument for fixing it at the boundary instead: `render_table` escapes per cell, and `esc()`/`print_error()` cover the line-oriented renderers. `witan task show`, `task close`, `project show`, `project status`, `trace show`, `session list`, `migrate`, `whoami` and the pickers print stored text whole again. Dry-run prompt output is the one place escaping is wrong — it exists to show the exact text the agent will receive — and `markup=False` alone was only half of that: Rich substitutes emoji codes independently of markup, so a prompt saying `:warning:` displayed as ⚠ while the agent got the eight literal characters. Both dry-run prints now pass `emoji=False, highlight=False` too, pinned by a test asserting the rendered output equals the prompt exactly. REBASED onto 0.22.0, which is why this is smaller than the version first pushed. #272 landed the other half — routing `session list`, `project show` and `trace show` off `s.client.read(...)` and through the tool surface, with the same `include_superseded` parameter — while this branch was in review, and its tests for those three paths are more thorough than the ones here (a real proxy over a real server, not a scripted client), so that work and its tests are dropped rather than duplicated. Two deliberate differences from the version that dropped out: - `CLIENT_READ_ATTRS` keeps `read`. #272 left it as a documented escape hatch for the next command that needs it; this branch had removed it along with an AST test forbidding `.client.read` anywhere in the CLI. Deferring to the merged decision — the guard on that is worth re-raising separately, not smuggling in here. - The `phases` field is joined and escaped in `project show`, matching how `trace show` already renders it. Copilot flagged the raw list as swallowed markup; it is not (a quoted repr is an invalid tag, so `['implementation']` prints whole) — the join is for consistency with its neighbour. 910 tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RXkw58FkNQuFUxbab2B5NH --- mcp/servers/witan/CHANGELOG.md | 31 ++++ mcp/servers/witan/pyproject.toml | 4 +- mcp/servers/witan/tests/test_cli_markup.py | 170 ++++++++++++++++++ mcp/servers/witan/witan/cli/__init__.py | 14 +- mcp/servers/witan/witan/cli/_common.py | 38 +++- mcp/servers/witan/witan/cli/auth.py | 30 ++-- mcp/servers/witan/witan/cli/local_dispatch.py | 4 +- mcp/servers/witan/witan/cli/migrate.py | 32 ++-- mcp/servers/witan/witan/cli/projects.py | 32 ++-- mcp/servers/witan/witan/cli/run_helpers.py | 14 +- mcp/servers/witan/witan/cli/session.py | 14 +- mcp/servers/witan/witan/cli/targets.py | 6 +- mcp/servers/witan/witan/cli/tasks.py | 25 +-- mcp/servers/witan/witan/cli/traces.py | 20 ++- uv.lock | 2 +- 15 files changed, 341 insertions(+), 95 deletions(-) create mode 100644 mcp/servers/witan/tests/test_cli_markup.py diff --git a/mcp/servers/witan/CHANGELOG.md b/mcp/servers/witan/CHANGELOG.md index c418f007..00fe396b 100644 --- a/mcp/servers/witan/CHANGELOG.md +++ b/mcp/servers/witan/CHANGELOG.md @@ -6,6 +6,37 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/) (pre-1.0: a MINOR bump may include breaking changes). +## [0.23.0] - 2026-08-21 + +### Fixed + +- **CLI renderers no longer drop bracketed text from stored content.** Rich + reads `[...]` in `Console.print` as a style tag, so a task resolution saying + `code_transport is not set on [targets.production]` printed as + `code_transport is not set on ,` — the identifier naming *which* target was + misconfigured was the part removed, and nothing indicated anything was + missing. Reading the same task back through `task_get` showed the stored text + intact, so this was always display-only. + + 0.18.0 escaped the four `witan serve` startup sites it had just written, one + call site at a time; every other renderer kept printing stored text straight + into markup. The escaping now happens at the two shared boundaries — + `render_table` per cell, and `esc()`/`print_error()` for the line-oriented + renderers — rather than per call site, since per-site escaping is exactly how + the first fix left the rest broken. `witan task show`, `task close`, + `project show`, `project status`, `trace show`, `session list`, `migrate`, + `whoami` and the pickers all print stored text whole again. + + Dry-run prompt output (`witan task run --dry-run`) turns off all three of + Rich's substitutions rather than escaping — it exists to show the exact text + the agent will receive, and `markup=False` alone still rendered a prompt + saying `:warning:` as ⚠ while the agent got the literal characters. + + Most of these went silently: Rich drops a tag it cannot resolve to a style, + so `[rank]`, `[targets.production]` and a markdown `[link]` all just vanish. + One shape is worse — a bracketed absolute path (`[/var/lib/witan]`) parses as + a *closing* tag and raises `MarkupError`, taking the whole command down. + ## [0.22.0] - 2026-08-21 ### Fixed diff --git a/mcp/servers/witan/pyproject.toml b/mcp/servers/witan/pyproject.toml index ea86da53..ab6a24db 100644 --- a/mcp/servers/witan/pyproject.toml +++ b/mcp/servers/witan/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "witan-council" -version = "0.22.0" +version = "0.23.0" description = "witan — agent memory, planning, and collaboration graph (work-coordination layer + umbrella CLI)" readme = "README.md" license = "BSD-3-Clause" @@ -180,7 +180,7 @@ packages = ["witan"] "schema" = "schema" [tool.bumpversion] -current_version = "0.22.0" +current_version = "0.23.0" allow_dirty = true [[tool.bumpversion.files]] diff --git a/mcp/servers/witan/tests/test_cli_markup.py b/mcp/servers/witan/tests/test_cli_markup.py new file mode 100644 index 00000000..11c77a3f --- /dev/null +++ b/mcp/servers/witan/tests/test_cli_markup.py @@ -0,0 +1,170 @@ +"""Stored text reaches the terminal whole, brackets included. + +Rich reads ``[...]`` in ``Console.print`` as a style tag, so any stored value +holding a TOML section, a Python repr, or a markdown link renders with that +substring silently removed — and nothing says it was. It was found on a task +resolution that named the misconfigured target: ``[targets.production]`` was +gone from the printed sentence while ``task_get`` showed it stored intact. + +agent-kit#261 fixed the two ``witan serve`` call sites it had just written and +every other renderer stayed broken, so these tests assert the escaping at the +two shared boundaries (``esc``/``print_error`` and ``render_table``) and then +through a command that prints stored content. They render through a real +console: markup is eaten at render time, so a test that captures ``print``'s +arguments passes while the user sees the hole. +""" + +from __future__ import annotations + +import pytest + +# The exact string from the report: a TOML table header, which Rich reads as a +# style named `targets.production` and drops when it cannot resolve it. +BRACKETED = ( + "code_transport is not set on [targets.production], so code graphs stay local" +) + + +@pytest.fixture +def render(): + """Run a CLI renderer and return what a terminal would actually show.""" + from witan.cli._common import console + + console.width = 200 # wide, so wrapping never splits a string under test + + def _render(fn, *args, **kwargs): + with console.capture() as capture: + fn(*args, **kwargs) + return capture.get() + + return _render + + +def test_esc_keeps_a_toml_section_in_the_rendered_line(render): + from witan.cli._common import console, esc + + out = render(console.print, f"resolution: {esc(BRACKETED)}") + + assert "[targets.production]" in out + + +def test_an_error_naming_the_config_block_to_fix_still_names_it(render): + # Error text is the worst place to lose brackets: witan's own refusals name + # the block to edit, and that name is the whole point of the sentence. + from witan.cli._common import print_error + + out = render(print_error, ValueError("unset `remote_url` on target [qa]")) + + assert "[qa]" in out + + +def test_a_table_cell_keeps_its_brackets(render): + from witan.cli._common import render_table + + out = render( + render_table, + title="Tasks", + columns=["slug", "title"], + rows=[{"slug": "tk-x", "title": "fix [targets.production]"}], + ) + + assert "[targets.production]" in out + + +def test_styling_a_column_still_works_after_escaping(render): + # The escape must not swallow the styles the renderer itself applies — + # those are markup we wrote, not data we were handed. + from witan.cli._common import _STATUS_STYLE, render_table + + out = render( + render_table, + title="Tasks", + columns=["status", "title"], + rows=[{"status": "blocked", "title": "x"}], + styles={"status": _STATUS_STYLE}, + ) + + assert "blocked" in out + assert "[red]" not in out # consumed as a style, not printed as text + + +def _stub_server(**tools): + """A server exposing only the named tools, and no ``client``.""" + + class _Stub: + def __getattr__(self, name): + if name in tools: + return tools[name] + raise AssertionError(f"unexpected attribute: {name}") + + return _Stub() + + +def test_task_show_prints_a_resolution_containing_a_toml_section(render, monkeypatch): + """The originally observed defect, end to end.""" + from witan.cli import _common + from witan.cli.tasks import _task_show + + task = { + "slug": "tk-x", + "title": "witan serve falls back to the local store", + "type": "bug", + "priority": "p2", + "status": "closed", + "description": f"Seen when {BRACKETED}", + "resolution": BRACKETED, + } + monkeypatch.setattr( + _common, + "_server", + _stub_server(task_get=lambda slug: task, task_list=lambda parent: []), + ) + + out = render(_task_show, "tk-x") + + assert out.count("[targets.production]") == 2 # description and resolution + + +def test_a_bracketed_path_does_not_take_the_command_down(render, monkeypatch): + # The one shape that fails loudly instead of silently: `[/var/lib/witan]` + # parses as a CLOSING tag with nothing open, which raises MarkupError — + # so an un-escaped store path in a description kills the whole command + # rather than losing its own substring. + from witan.cli import _common + from witan.cli.tasks import _task_show + + task = { + "slug": "tk-y", + "title": "recall() drops rows where [rank] is null", + "status": "open", + "priority": "p1", + "type": "bug", + "description": "the store at [/var/lib/witan/graph.omni] is stale", + } + monkeypatch.setattr( + _common, + "_server", + _stub_server(task_get=lambda slug: task, task_list=lambda parent: []), + ) + + out = render(_task_show, "tk-y") + + assert "[rank]" in out + assert "[/var/lib/witan/graph.omni]" in out + + +def test_a_dry_run_prompt_is_shown_exactly_as_the_agent_will_receive_it(render): + """The one place escaping is the wrong answer, and `markup=False` is half of it. + + A dry run exists to show the text that will be handed to the agent, so + escapes must not appear in it — but Rich substitutes emoji codes and + highlights literals independently of markup, so `:warning:` in a task + description rendered as ⚠ while the agent received the eight original + characters. Same class of lie, different Rich feature. + """ + from witan.cli.run_helpers import _launch_agent + + prompt = "Fix the :warning: banner in [targets.production] before 3.14" + out = render(_launch_agent, None, "claude", None, prompt, True) + + assert out.strip() == prompt diff --git a/mcp/servers/witan/witan/cli/__init__.py b/mcp/servers/witan/witan/cli/__init__.py index a1521591..87abd981 100644 --- a/mcp/servers/witan/witan/cli/__init__.py +++ b/mcp/servers/witan/witan/cli/__init__.py @@ -34,7 +34,7 @@ tasks, # noqa: F401 traces, # noqa: F401 ) -from ._common import app, console, stderr_console +from ._common import app, console, print_error, stderr_console from .migrate import migrate_app from .output import OutputFormat, set_output_format from .run_helpers import _run_task_slug @@ -156,7 +156,7 @@ def _serve_target(transport: str): try: remote = cfg_module.load_remote_config() except ValueError as exc: - stderr_console.print(f"[red]{escape(str(exc))}[/red]") + print_error(exc, stderr=True) raise SystemExit(1) from None if remote is None: @@ -165,9 +165,7 @@ def _serve_target(transport: str): return witan_mcp if transport != "stdio": - stderr_console.print( - f"[red]{escape(remote_serving_needs_stdio(remote, transport))}[/red]" - ) + print_error(remote_serving_needs_stdio(remote, transport), stderr=True) raise SystemExit(1) from None from ..remote.serve import build_remote_server @@ -175,9 +173,7 @@ def _serve_target(transport: str): try: server = asyncio.run(build_remote_server(remote)) except Exception as exc: # noqa: BLE001 — every failure mode is the same answer - stderr_console.print( - f"[red]{escape(remote_startup_failure(remote, exc))}[/red]" - ) + print_error(remote_startup_failure(remote, exc), stderr=True) raise SystemExit(1) from None _warn_if_code_graph_is_local() return server @@ -300,7 +296,7 @@ def run( try: cfg = cfg_module.load(target=target) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None _run_task_slug( slug, cfg=cfg, agent=agent, model=model, claim=claim, dry_run=dry_run diff --git a/mcp/servers/witan/witan/cli/_common.py b/mcp/servers/witan/witan/cli/_common.py index 5c815bd9..2cb8c382 100644 --- a/mcp/servers/witan/witan/cli/_common.py +++ b/mcp/servers/witan/witan/cli/_common.py @@ -8,6 +8,7 @@ from typing import Literal from rich.console import Console +from rich.markup import escape from rich.table import Table from witan_core.cli import make_app @@ -70,7 +71,7 @@ def _srv(): try: remote = cfg_module.load_remote_config() except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if remote is not None: _server = remote_proxy(remote) @@ -144,9 +145,36 @@ def _split_csv(items: list[str] | None) -> list[str] | None: } +def esc(value: object) -> str: + """Escape stored graph content for interpolation into a markup string. + + Rich reads square brackets in ``Console.print`` as style tags, so text + holding a TOML section (``[targets.production]``), a Python repr, or a + markdown link renders with that substring silently dropped — a resolution + that named which target was misconfigured turns into one that names none. + Nothing indicates anything was removed. + + Every renderer that prints stored content goes through this or + :func:`render_table`, which applies it per cell. Escape at the boundary + where graph text meets markup, not at each call site: agent-kit#261 fixed + two sites that way and every other one stayed broken. + """ + return escape("" if value is None else str(value)) + + +def print_error(message: object, *, stderr: bool = False) -> None: + """Print ``message`` in red, escaped. + + Error text is the worst place to drop a bracketed substring: witan's own + refusals name the config section to fix (``[targets.production]``), and an + omnigraph error quotes the query it choked on. Both are markup to Rich. + """ + (stderr_console if stderr else console).print(f"[red]{esc(message)}[/red]") + + def _styled(value: str, table: dict) -> str: style = table.get(value) - return f"[{style}]{value}[/{style}]" if style else (value or "") + return f"[{style}]{esc(value)}[/{style}]" if style else esc(value) def _short_repo(uri: str | None) -> str: @@ -206,12 +234,12 @@ def render_table( for col in columns: value = str(r.get(col, "")) if not value and col in placeholders: - cells.append(f"[dim]{placeholders[col]}[/dim]") + cells.append(f"[dim]{esc(placeholders[col])}[/dim]") elif col in styles: cells.append(_styled(value, styles[col])) elif value and col in dim_if_present: - cells.append(f"[dim]{value}[/dim]") + cells.append(f"[dim]{esc(value)}[/dim]") else: - cells.append(value) + cells.append(esc(value)) table.add_row(*cells) console.print(table) diff --git a/mcp/servers/witan/witan/cli/auth.py b/mcp/servers/witan/witan/cli/auth.py index bd39fbd1..856f8009 100644 --- a/mcp/servers/witan/witan/cli/auth.py +++ b/mcp/servers/witan/witan/cli/auth.py @@ -12,14 +12,14 @@ from .. import config as cfg_module from ..identity import derive_actor_id from ..remote import oidc -from ._common import app, console +from ._common import app, console, esc, print_error def _remote_or_exit(target: str | None = None) -> cfg_module.RemoteConfig: try: remote = cfg_module.load_remote_config(target=target) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if remote is None: console.print( @@ -53,19 +53,21 @@ def _prompt(device: dict) -> None: code = device.get("user_code", "") console.print("\n[bold]Authenticate witan CLI[/bold]") if complete: - console.print(f" Open: [cyan underline]{complete}[/cyan underline]") + console.print(f" Open: [cyan underline]{esc(complete)}[/cyan underline]") console.print( - f" Or go to [cyan underline]{uri}[/cyan underline] and enter " - f"code [bold]{code}[/bold]\n Waiting for approval…" + f" Or go to [cyan underline]{esc(uri)}[/cyan underline] and enter " + f"code [bold]{esc(code)}[/bold]\n Waiting for approval…" ) try: claims = oidc.login(remote, on_prompt=_prompt) except oidc.RemoteAuthError as exc: - console.print(f"[red]Login failed:[/red] {exc}") + console.print(f"[red]Login failed:[/red] {esc(exc)}") raise SystemExit(1) from None who = claims.get("preferred_username") or claims.get("sub", "?") - console.print(f"[green]Logged in[/green] as [bold]{who}[/bold] → {remote.url}") + console.print( + f"[green]Logged in[/green] as [bold]{esc(who)}[/bold] → {esc(remote.url)}" + ) @app.command @@ -78,7 +80,7 @@ def logout(*, target: str | None = None) -> None: """ remote = _remote_or_exit(target) if oidc.logout(remote): - console.print(f"[green]Logged out[/green] of {remote.url}") + console.print(f"[green]Logged out[/green] of {esc(remote.url)}") else: console.print("[yellow]No cached session to clear.[/yellow]") @@ -126,22 +128,24 @@ def whoami(*, target: str | None = None) -> None: try: token = oidc.get_valid_token(remote) except oidc.NeedsLogin as exc: - console.print(f"[yellow]{exc}[/yellow]") + console.print(f"[yellow]{esc(exc)}[/yellow]") raise SystemExit(1) from None except oidc.RemoteAuthError as exc: # Caught separately from NeedsLogin above so a token endpoint that is # merely unreachable does not read as "log in again" — the whole point # of classifying the two. - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None claims = oidc.decode_claims(token) sub = claims.get("sub", "") if remote.target_name: console.print(f"[bold]Target[/bold] {remote.target_name}") - console.print(f"[bold]Endpoint[/bold] {remote.url}") - console.print(f"[bold]User[/bold] {claims.get('preferred_username', '?')}") + console.print(f"[bold]Endpoint[/bold] {esc(remote.url)}") + console.print( + f"[bold]User[/bold] {esc(claims.get('preferred_username', '?'))}" + ) if claims.get("email"): - console.print(f"[bold]Email[/bold] {claims['email']}") + console.print(f"[bold]Email[/bold] {esc(claims['email'])}") console.print(f"[bold]sub[/bold] {sub}") if sub: console.print(f"[bold]actor[/bold] {derive_actor_id(sub)}") diff --git a/mcp/servers/witan/witan/cli/local_dispatch.py b/mcp/servers/witan/witan/cli/local_dispatch.py index 446724d7..1bbec8f9 100644 --- a/mcp/servers/witan/witan/cli/local_dispatch.py +++ b/mcp/servers/witan/witan/cli/local_dispatch.py @@ -31,7 +31,7 @@ from __future__ import annotations from ..config import LocalDispatch -from ._common import stderr_console +from ._common import print_error, stderr_console __all__ = [ "CLIENT_READ_ATTRS", @@ -215,7 +215,7 @@ def _allow(self): return self._inner def _refuse(self, what: str) -> None: - stderr_console.print(f"[red]{local_write_refused(what, self._diagnosis)}[/red]") + print_error(local_write_refused(what, self._diagnosis), stderr=True) raise SystemExit(1) def __getattr__(self, name: str): diff --git a/mcp/servers/witan/witan/cli/migrate.py b/mcp/servers/witan/witan/cli/migrate.py index 9bf6fe83..b8ba3efc 100644 --- a/mcp/servers/witan/witan/cli/migrate.py +++ b/mcp/servers/witan/witan/cli/migrate.py @@ -7,7 +7,7 @@ import cyclopts -from ._common import _srv, console, remote_proxy +from ._common import _srv, console, esc, print_error, remote_proxy migrate_app = cyclopts.App( name="migrate", @@ -19,9 +19,9 @@ def _apply_schema() -> None: try: result = _srv().apply_schema() except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None - console.print(result["output"] or f"schema applied to {result['store']}") + console.print(esc(result["output"] or f"schema applied to {result['store']}")) def _repo_keys() -> None: @@ -29,7 +29,7 @@ def _repo_keys() -> None: try: result = s.migrate_repo_keys() except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None console.print( f"Updated {result['tasks_updated']} task(s), {result['memories_updated']} " @@ -45,7 +45,7 @@ def _repo_keys() -> None: "re-derivable cache, not covered by this migration):[/yellow]" ) for old, new in changed.items(): - console.print(f" {old} -> {new}") + console.print(f" {esc(old)} -> {esc(new)}") def _backfill_topics() -> None: @@ -59,7 +59,7 @@ def _backfill_topics() -> None: raise SystemExit(1) result = s.migrate_topics() except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None console.print( f"Scanned {result['memories_scanned']} memories; " @@ -69,7 +69,7 @@ def _backfill_topics() -> None: def _fail(message: str) -> NoReturn: - console.print(f"[red]{message}[/red]") + print_error(message) raise SystemExit(1) @@ -192,13 +192,13 @@ def _merge( try: result = s.merge_store(source, target=target, dry_run=dry_run) except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if dry_run: console.print(f"[yellow]Dry run[/yellow] against {result['target']}:") for d in result["decisions"]: - console.print(f" {d['decision']:12} {d['type']:16} {d['slug']}") + console.print(f" {d['decision']:12} {d['type']:16} {esc(d['slug'])}") console.print( f"{result['added']} to add, {result['updated']} to update, " f"{result['kept_target']} kept (target already newer-or-equal)." @@ -235,16 +235,16 @@ def _migrate_storage(old_binary: str | None, yes: bool) -> None: try: result = s.migrate_storage_format(old_binary) except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if not result["migrated"]: - console.print(result["reason"]) + console.print(esc(result["reason"])) return console.print( f"[green]Migrated[/green] {result['store']} " f"(old binary: {result['old_binary']}, backup: {result['backup']})." ) - console.print(result["verify"]) + console.print(esc(result["verify"])) @migrate_app.command @@ -417,7 +417,7 @@ def dedupe_sessions( try: result = _srv().migrate_dedupe_sessions(apply=apply, extra_marks=extra or None) except RuntimeError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None marked = result["marked"] @@ -426,7 +426,7 @@ def dedupe_sessions( verb = "Marked" if result["applied"] else "[yellow]Would mark[/yellow]" console.print(f"{verb} {len(marked)} duplicate session(s):") for dup, survivor in marked.items(): - console.print(f" {dup} -> {survivor}") + console.print(f" {esc(dup)} -> {esc(survivor)}") else: console.print("No duplicate sessions to mark.") @@ -438,8 +438,8 @@ def dedupe_sessions( "in fact one session:" ) for sess in run["sessions"]: - console.print(f" {sess['slug']} {sess['started_at']}") - console.print(f" {sess['summary']}") + console.print(f" {sess['slug']} {esc(sess['started_at'])}") + console.print(f" {esc(sess['summary'])}") if result["sealed_traces"]: console.print( diff --git a/mcp/servers/witan/witan/cli/projects.py b/mcp/servers/witan/witan/cli/projects.py index 078660de..5535fe34 100644 --- a/mcp/servers/witan/witan/cli/projects.py +++ b/mcp/servers/witan/witan/cli/projects.py @@ -19,10 +19,12 @@ _split_csv, _srv, _styled, - WorkflowPhase, app, console, + esc, + print_error, render_table, + WorkflowPhase, ) from .run_helpers import ( _launch_agent, @@ -84,7 +86,7 @@ def _project_show(slug: str) -> None: if not p: console.print(f"[red]No project {slug!r}.[/red]") return - console.print(f"[bold]{p['slug']}[/bold] {p.get('title', '')}") + console.print(f"[bold]{p['slug']}[/bold] {esc(p.get('title'))}") console.print( f" status={_styled(p.get('status', ''), _STATUS_STYLE)} " f"phase={p.get('phase')} repos={', '.join(p.get('repos') or []) or '—'}" @@ -103,22 +105,21 @@ def _project_show(slug: str) -> None: console.print(f" blocked by {blocker} [{_styled(st, _STATUS_STYLE)}]") if p.get("blocks"): console.print(f" blocks: {', '.join(p['blocks'])}") - console.print(f"\n{p.get('description') or '(no description)'}\n") + console.print(f"\n{esc(p.get('description') or '(no description)')}\n") sessions = _fn(s.workflow_session_list)(project_slug=slug) console.print(f" sessions: {len(sessions)}") for sess in sessions: - console.print( - f" {sess['slug']} [{sess.get('phase')}] " - f"{sess.get('summary') or '(in progress)'}"[:120] - ) + summary = (sess.get("summary") or "(in progress)")[:80] + console.print(f" {sess['slug']} ({esc(sess.get('phase'))}) {esc(summary)}") project_tasks = _fn(s.task_list)(project_slug=slug) if project_tasks: console.print(f" tasks: {len(project_tasks)}") for t in project_tasks: console.print( - f" {t['slug']} [{_styled(t.get('status', ''), _STATUS_STYLE)}] {t.get('title', '')}" + f" {t['slug']} [{_styled(t.get('status', ''), _STATUS_STYLE)}] " + f"{esc(t.get('title'))}" ) if p.get("status") == "completed": @@ -126,10 +127,11 @@ def _project_show(slug: str) -> None: if tr: console.print( f"\n [blue]trace[/blue]: {tr.get('session_count')} sessions, " - f"phases={tr.get('phases')}, duration={tr.get('duration')}h" + f"phases={esc(', '.join(tr.get('phases') or []))}, " + f"duration={tr.get('duration')}h" ) if tr.get("outcome"): - console.print(f" outcome: {tr['outcome']}"[:200]) + console.print(f" outcome: {esc(tr['outcome'][:187])}") console.print( f" lessons: {', '.join(tr.get('lessons_slug') or []) or '(none mined yet)'}" ) @@ -174,7 +176,7 @@ def project_status( p = st["project"] repos_s = ", ".join(_short_repo(r) for r in (p.get("repos") or [])) or "—" - console.print(f"[bold]{p['slug']}[/bold] {escape(p.get('title', ''))}") + console.print(f"[bold]{p['slug']}[/bold] {esc(p.get('title'))}") console.print( f" phase={p.get('phase')} " f"status={_styled(p.get('status', ''), _STATUS_STYLE)} repos={repos_s}" @@ -282,7 +284,7 @@ def _status_of(task_slug: str) -> str: if not blockers and not deps: continue any_edges = True - console.print(f" [bold]{r['slug']}[/bold] {escape(r.get('title', ''))}") + console.print(f" [bold]{r['slug']}[/bold] {esc(r.get('title'))}") for b in blockers: console.print( f" ↑ blocked by {b} [{_styled(_status_of(b), _STATUS_STYLE)}]" @@ -401,7 +403,7 @@ def project_update( raise SystemExit(1) console.print(f"[green]Updated[/green] [bold]{slug}[/bold]") - console.print(f" title: {escape(result.get('title') or '')}") + console.print(f" title: {esc(result.get('title'))}") if result.get("repos"): console.print(f" repos: {', '.join(_short_repo(r) for r in result['repos'])}") console.print(f" phase: {result.get('phase')} status: {result.get('status')}") @@ -538,7 +540,7 @@ def project_run( try: cfg = cfg_module.load(target=target) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None resolved_agent = agent or cfg.agent @@ -563,7 +565,7 @@ def _render_project(p: dict) -> str: repos_s = ( f" [dim]{', '.join(_short_repo(r) for r in (p.get('repos') or []))}[/dim]" ) - return f"{p['slug']} {phase} {p.get('title', '')}{repos_s}" + return f"{p['slug']} {phase} {esc(p.get('title'))}{repos_s}" selected = _pick_items(active, _render_project) if not selected: diff --git a/mcp/servers/witan/witan/cli/run_helpers.py b/mcp/servers/witan/witan/cli/run_helpers.py index 21b60573..0804a81b 100644 --- a/mcp/servers/witan/witan/cli/run_helpers.py +++ b/mcp/servers/witan/witan/cli/run_helpers.py @@ -8,6 +8,7 @@ _fn, _srv, console, + esc, ) @@ -54,7 +55,12 @@ def _launch_agent( cfg, resolved_agent: str, resolved_model: str | None, prompt: str, dry_run: bool ) -> None: if dry_run: - console.print(prompt) + # Not esc(): a dry run exists to show the exact text the agent will + # receive, so it must not be shown with escapes in it. All three of + # Rich's substitutions are off — `emoji=False` because a prompt saying + # `:warning:` renders as ⚠ while the agent receives the literal + # characters, which is the same class of lie the escaping fixes. + console.print(prompt, markup=False, emoji=False, highlight=False) return cmd = [resolved_agent] if resolved_model: @@ -131,7 +137,7 @@ def _run_task_slug( prompt = _run_prompt(t) if dry_run: - console.print(prompt) + console.print(prompt, markup=False, emoji=False, highlight=False) return if claim: @@ -142,9 +148,9 @@ def _run_task_slug( res = _fn(s.task_claim)(slug=slug, force=force) or {} if not res.get("claimed"): reason = res.get("held_by") or res.get("reason") or "unavailable" - console.print(f"[red]Could not claim {slug} ({reason}).[/red]") + console.print(f"[red]Could not claim {slug} ({esc(reason)}).[/red]") if res.get("remedy"): - console.print(f" [yellow]{res['remedy']}[/yellow]") + console.print(f" [yellow]{esc(res['remedy'])}[/yellow]") raise SystemExit(1) console.print(f"[cyan]Claimed {slug} (assignee={res.get('assignee')}).[/cyan]") diff --git a/mcp/servers/witan/witan/cli/session.py b/mcp/servers/witan/witan/cli/session.py index 345b8a63..e4667baf 100644 --- a/mcp/servers/witan/witan/cli/session.py +++ b/mcp/servers/witan/witan/cli/session.py @@ -15,16 +15,17 @@ import uuid import cyclopts -from rich.markup import escape from .. import session_state from ._common import ( _fn, _split_csv, _srv, - WorkflowPhase, app, console, + esc, + print_error, + WorkflowPhase, ) session_app = cyclopts.App( @@ -165,7 +166,7 @@ def session_sweep( try: max_age = timedelta(seconds=_parse_duration(older_than)) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None s = _srv() @@ -258,6 +259,7 @@ def session_list(project_slug: str) -> None: summary = (sess.get("summary") or "(in progress)").splitlines() # Escape the free-text first line and use parentheses (not brackets) for # phase/state — Rich would parse "[implementation/open]" as a malformed - # markup tag and could error out on it. - first = escape(summary[0] if summary else "(in progress)") - console.print(f" {sess['slug']} ({sess.get('phase')}/{state}) {first}"[:140]) + # markup tag and could error out on it. Truncate before escaping, so a + # cut never lands inside an escape sequence. + first = esc((summary[0] if summary else "(in progress)")[:80]) + console.print(f" {sess['slug']} ({esc(sess.get('phase'))}/{state}) {first}") diff --git a/mcp/servers/witan/witan/cli/targets.py b/mcp/servers/witan/witan/cli/targets.py index c0e6e212..753d7117 100644 --- a/mcp/servers/witan/witan/cli/targets.py +++ b/mcp/servers/witan/witan/cli/targets.py @@ -26,7 +26,7 @@ import tomli_w from .. import config as cfg_module -from ._common import _split_csv, app, console, render_table +from ._common import _split_csv, app, console, print_error, render_table targets_app = cyclopts.App( name="target", @@ -203,7 +203,7 @@ def _existing_names() -> list[str]: try: return [t.name for t in cfg_module._parse_targets(cfg_module._load_toml())] except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None @@ -405,7 +405,7 @@ def list_targets() -> None: try: targets = cfg_module._parse_targets(cfg_module._load_toml()) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if not targets: console.print( diff --git a/mcp/servers/witan/witan/cli/tasks.py b/mcp/servers/witan/witan/cli/tasks.py index 8ef5839e..f580a727 100644 --- a/mcp/servers/witan/witan/cli/tasks.py +++ b/mcp/servers/witan/witan/cli/tasks.py @@ -22,6 +22,8 @@ TaskType, app, console, + esc, + print_error, render_table, ) from .run_helpers import ( @@ -138,7 +140,7 @@ def _task_show(slug: str) -> None: console.print(f"[red]No task {slug!r}.[/red]") return - console.print(f"[bold]{t['slug']}[/bold] {t.get('title', '')}") + console.print(f"[bold]{t['slug']}[/bold] {esc(t.get('title'))}") console.print( f" type={t.get('type')} " f"priority={_styled(t.get('priority', ''), _PRIORITY_STYLE)} " @@ -156,14 +158,14 @@ def _task_show(slug: str) -> None: project = _fn(s.workflow_project_get)(slug=t["project_slug"]) if project: console.print( - f" project: {project['slug']} — {project.get('title', '')} " - f"[{project.get('phase', '')}]" + f" project: {project['slug']} — {esc(project.get('title'))} " + f"({esc(project.get('phase'))})" ) else: console.print(f" project: {t['project_slug']}") if t.get("symbol_refs"): console.print(f" code symbols: {', '.join(t['symbol_refs'])}") - console.print(f"\n{t.get('description') or '(no description)'}\n") + console.print(f"\n{esc(t.get('description') or '(no description)')}\n") for blocker in t.get("blocked_by") or []: b = _fn(s.task_get)(slug=blocker) @@ -173,10 +175,11 @@ def _task_show(slug: str) -> None: children = _fn(s.task_list)(parent=slug) for c in children: console.print( - f" ↳ {c['slug']} [{_styled(c.get('status', ''), _STATUS_STYLE)}] {c.get('title', '')}" + f" ↳ {c['slug']} [{_styled(c.get('status', ''), _STATUS_STYLE)}] " + f"{esc(c.get('title'))}" ) if t.get("resolution"): - console.print(f"\n resolution: {t['resolution']}") + console.print(f"\n resolution: {esc(t['resolution'])}") task_app = cyclopts.App( @@ -259,7 +262,7 @@ def task_close_cmd(slug: str, *, resolution: str | None = None) -> None: raise SystemExit(1) console.print(f"[green]Closed[/green] [bold]{slug}[/bold]") if resolution: - console.print(f" resolution: {resolution}") + console.print(f" resolution: {esc(resolution)}") @task_app.command(name="claim") @@ -293,9 +296,9 @@ def task_claim_cmd( ) return reason = result.get("held_by") or result.get("reason") or "unavailable" - console.print(f"[yellow]Not claimed[/yellow] ({reason}).") + console.print(f"[yellow]Not claimed[/yellow] ({esc(reason)}).") if result.get("remedy"): - console.print(f" {result['remedy']}") + console.print(f" {esc(result['remedy'])}") raise SystemExit(1) @@ -487,7 +490,7 @@ def task_run( try: cfg = cfg_module.load(target=target) except ValueError as exc: - console.print(f"[red]{exc}[/red]") + print_error(exc) raise SystemExit(1) from None if slug: @@ -514,7 +517,7 @@ def task_run( def _render_task(t: dict) -> str: pri = _styled(t.get("priority", ""), _PRIORITY_STYLE) repo_s = f" [dim]{_short_repo(t.get('repo'))}[/dim]" if t.get("repo") else "" - return f"{t['slug']} {pri} {t.get('title', '')}{repo_s}" + return f"{t['slug']} {pri} {esc(t.get('title'))}{repo_s}" selected = _pick_items(ready, _render_task) if not selected: diff --git a/mcp/servers/witan/witan/cli/traces.py b/mcp/servers/witan/witan/cli/traces.py index e395ef21..ea5824e2 100644 --- a/mcp/servers/witan/witan/cli/traces.py +++ b/mcp/servers/witan/witan/cli/traces.py @@ -19,6 +19,7 @@ _srv, app, console, + esc, render_table, ) @@ -81,22 +82,25 @@ def _trace_show(slug: str) -> None: console.print(f"[red]No trace {slug!r}.[/red]") return - console.print(f"[bold]{tr['slug']}[/bold] {tr.get('title', '')}") + console.print(f"[bold]{tr['slug']}[/bold] {esc(tr.get('title'))}") console.print( f" project={tr.get('project_slug')} sessions={tr.get('session_count')} " - f"phases={', '.join(tr.get('phases') or [])} duration={tr.get('duration')}h " + f"phases={esc(', '.join(tr.get('phases') or []))} " + f"duration={tr.get('duration')}h " f"repos={', '.join(_short_repo(u) for u in (tr.get('repos') or [])) or '—'}" ) - console.print(f"\n{tr.get('description') or '(no description)'}\n") - console.print(f"[bold]Outcome[/bold]\n{tr.get('outcome') or '(none recorded)'}\n") + console.print(f"\n{esc(tr.get('description') or '(no description)')}\n") + console.print( + f"[bold]Outcome[/bold]\n{esc(tr.get('outcome') or '(none recorded)')}\n" + ) sessions = _fn(s.workflow_session_list)(project_slug=tr.get("project_slug")) if sessions: console.print("[bold]Sessions[/bold]") for sess in sessions: + summary = (sess.get("summary") or "(in progress)")[:80] console.print( - f" {sess['slug']} [{sess.get('phase')}] " - f"{sess.get('summary') or '(in progress)'}"[:120] + f" {sess['slug']} ({esc(sess.get('phase'))}) {esc(summary)}" ) console.print() @@ -111,8 +115,8 @@ def _trace_show(slug: str) -> None: for mslug in slugs: m = _fn(s.memory_get)(slug=mslug) if m: - console.print(f" [cyan]{mslug}[/cyan] {m.get('title', '')}") - console.print(f" {m.get('content', '')}"[:240]) + console.print(f" [cyan]{mslug}[/cyan] {esc(m.get('title'))}") + console.print(f" {esc((m.get('content') or '')[:236])}") else: console.print(f" [dim]{mslug} (missing)[/dim]") console.print() diff --git a/uv.lock b/uv.lock index c8f5a8ea..d3bb1a66 100644 --- a/uv.lock +++ b/uv.lock @@ -2413,7 +2413,7 @@ test = [ [[package]] name = "witan-council" -version = "0.22.0" +version = "0.23.0" source = { editable = "mcp/servers/witan" } dependencies = [ { name = "agent-config-kit" },