diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 2780201cbf..cd92ae4c5e 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -11,8 +11,12 @@ from __future__ import annotations +import ast +import inspect +import io import re import shlex +import tokenize from collections.abc import Iterable from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, Literal @@ -139,7 +143,56 @@ def _extract_frontmatter_description(text: str) -> str: return value if isinstance(value, str) else "" -def _extract_script_description(text: str) -> str: +def _normalize_python_string(literal: str) -> str | None: + """Keep string values while avoiding compiler warnings for legacy escapes.""" + prefix = re.match(r"[^\"']*", literal).group().lower() + if prefix not in {"", "r", "u"}: + return None + if prefix != "r": + + def escape(match: re.Match[str]) -> str: + value = match.group() + if value[1] in "01234567": + number = int(value[1:], 8) + return f"\\U{number:08x}" if number > 0o377 else value + if value[1] in "\\\"'abfnrtv\n\rxuUN": + return value + return "\\" + value + + literal = re.sub(r"\\(?:[0-7]{1,3}|[\s\S])", escape, literal) + return literal + + +def _leading_python_docstring(text: str) -> str | None: + """Read only the leading string expression, independent of later grammar.""" + expression = [] + depth = 0 + for token in tokenize.generate_tokens(io.StringIO(text).readline): + if token.type in {tokenize.COMMENT, tokenize.NL}: + continue + if token.type == tokenize.STRING: + literal = _normalize_python_string(token.string) + if literal is None: + return None + expression.append(literal) + elif token.type == tokenize.OP and token.string in {"(", ")"}: + depth += 1 if token.string == "(" else -1 + if depth < 0: + return None + expression.append(token.string) + elif token.type in {tokenize.NEWLINE, tokenize.ENDMARKER} or ( + token.type == tokenize.OP and token.string == ";" and not depth + ): + if not expression or depth: + return None + value = ast.literal_eval(" ".join(expression)) + return inspect.cleandoc(value) if isinstance(value, str) else None + else: + return None + return None + + +def _extract_script_description(text: str, *, is_python: bool = False) -> str: """Return the first docstring/comment line of a script, else ``""``. Supports the three script runtimes Spec Kit ships: @@ -152,11 +205,27 @@ def _extract_script_description(text: str) -> str: Anything unrecognized yields the empty string. """ - py_match = re.match(r'^(?:#![^\n]*\n)?\s*(?:"""|\'\'\')(.*?)(?:"""|\'\'\')', text, re.DOTALL) - if py_match: - first = py_match.group(1).strip().splitlines() - if first: - return first[0].strip() + if is_python: + try: + description = _leading_python_docstring(text) + except ( + SyntaxError, + ValueError, + RecursionError, + MemoryError, + tokenize.TokenError, + ): + description = None + if description and description.strip(): + return description.strip().splitlines()[0].strip() + else: + py_match = re.match( + r'^(?:#![^\n]*\n)?\s*(?:"""|\'\'\')(.*?)(?:"""|\'\'\')', text, re.DOTALL + ) + if py_match: + first = py_match.group(1).strip().splitlines() + if first: + return first[0].strip() ps_block = re.match(r'^(?:<#\s*(.*?)#>)', text, re.DOTALL) if ps_block: @@ -186,7 +255,7 @@ def _describe_artifact_file(path: Path, kind: ArtifactKind) -> str: except (OSError, UnicodeDecodeError): return "" if kind == "script": - return _extract_script_description(text) + return _extract_script_description(text, is_python=path.suffix == ".py") return _extract_frontmatter_description(text) diff --git a/tests/specify_cli/artifacts/test_catalog.py b/tests/specify_cli/artifacts/test_catalog.py index c3fa25eab8..963ca6bdcd 100644 --- a/tests/specify_cli/artifacts/test_catalog.py +++ b/tests/specify_cli/artifacts/test_catalog.py @@ -471,6 +471,49 @@ def test_includes_project_local_core_assets(self, spec_kit_project: Path): assert layer["lookupId"] is None assert layer["sourcePath"] is None + @pytest.mark.parametrize( + ("header", "docstring", "expected"), + [ + ( + "# -*- coding: utf-8 -*-\n", + '"""Deploy the selected feature."""', + "Deploy the selected feature.", + ), + ( + "# Copyright Example Team\n# SPDX-License-Identifier: MIT\n", + '"""Deploy the selected feature.\n\nAdditional details.\n"""', + "Deploy the selected feature.", + ), + ("# Custom helper\n", '""" """', "Custom helper"), + ("# Custom helper\n", "", "Custom helper"), + ], + ) + def test_python_script_description_skips_header_comments( + self, spec_kit_project: Path, header: str, docstring: str, expected: str + ): + templates = spec_kit_project / ".specify" / "templates" + scripts = templates / "scripts" / "python" + scripts.mkdir(parents=True) + commands = templates / "commands" + commands.mkdir() + (commands / "deploy.md").write_text( + "---\nscripts:\n py: scripts/python/deploy.py\n---\nDeploy.\n", encoding="utf-8" + ) + (spec_kit_project / ".specify" / "init-options.json").write_text( + json.dumps({"script": "py"}), encoding="utf-8" + ) + (scripts / "deploy.py").write_text( + '#!/usr/bin/env python3\n' + header + + "\n" + docstring + "\n" + + 'raise RuntimeError("Inventory must never execute this script")\n', + encoding="utf-8", + ) + + catalog = ArtifactCatalog(spec_kit_project) + row = next(a for a in catalog.list_artifacts() if a.id == "script:deploy") + assert row.description == expected + assert catalog.get_artifact_info("script:deploy")["description"] == row.description + def test_includes_root_level_pack_templates( self, spec_kit_project: Path ): diff --git a/tests/specify_cli/artifacts/test_description_parsing.py b/tests/specify_cli/artifacts/test_description_parsing.py new file mode 100644 index 0000000000..81ce93e9b5 --- /dev/null +++ b/tests/specify_cli/artifacts/test_description_parsing.py @@ -0,0 +1,263 @@ +"""Best-effort Python metadata parsing must not disrupt artifact commands.""" + +from __future__ import annotations + +import ast +import json +import warnings +from pathlib import Path +from types import SimpleNamespace + +import pytest +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli.artifacts import catalog + + +COMMANDS = [ + ["artifact", "list", "--json"], + ["artifact", "info", "script:deploy", "--json"], + ["artifact", "info", "command:speckit.plan", "--json"], +] + + +def _write_python_script(project: Path, source: str) -> None: + templates = project / ".specify" / "templates" + scripts = templates / "scripts" / "python" + scripts.mkdir(parents=True) + commands = templates / "commands" + commands.mkdir() + (commands / "deploy.md").write_text( + "---\nscripts:\n py: scripts/python/deploy.py\n---\nDeploy.\n", + encoding="utf-8", + ) + (project / ".specify" / "init-options.json").write_text( + json.dumps({"script": "py"}), encoding="utf-8" + ) + (scripts / "deploy.py").write_text(source, encoding="utf-8") + + +def _assert_script_description(payload, command, expected): + if command[1] == "list": + assert ( + next(row for row in payload if row["id"] == "script:deploy")["description"] + == expected + ) + elif command[2] == "script:deploy": + assert payload["description"] == expected + else: + assert payload["id"] == "command:speckit.plan" + + +@pytest.mark.parametrize( + "expression", + ["1+" * 9999 + "1", "+" * 6000 + "1", "def broken(:", "\0"], + ids=["deep-addition", "deep-unary", "invalid-syntax", "nul-character"], +) +@pytest.mark.parametrize("command", COMMANDS) +def test_parser_failures_use_comment_fallback( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + expression: str, + command: list[str], +): + _write_python_script(spec_kit_project, "# Deployment helper\n" + expression + "\n") + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke(app, command) + + assert result.exit_code == 0, result.exception + assert result.stderr == "" + _assert_script_description(json.loads(result.stdout), command, "Deployment helper") + + +@pytest.mark.parametrize("command", COMMANDS) +def test_metadata_parse_suppresses_warnings_without_changing_filters( + spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, command: list[str] +): + _write_python_script( + spec_kit_project, + '# Header\n"""Deployment helper."""\nimport re\nre.compile("\\d+")\n' + 'raise RuntimeError("Inventory must never execute this script")\n', + ) + monkeypatch.chdir(spec_kit_project) + with warnings.catch_warnings(record=True) as emitted: + warnings.simplefilter("always") + filters = warnings.filters.copy() + result = CliRunner().invoke(app, command) + assert warnings.filters == filters + + assert result.exit_code == 0, result.exception + assert result.stderr == "" + assert emitted == [] + _assert_script_description(json.loads(result.stdout), command, "Deployment helper.") + + +@pytest.mark.parametrize("command", COMMANDS) +def test_metadata_does_not_invoke_full_module_parser( + spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, command: list[str] +): + _write_python_script( + spec_kit_project, + '# Header\n"""Deployment helper."""\ntype FeatureId = str\n', + ) + monkeypatch.chdir(spec_kit_project) + parse = ast.parse + filters = warnings.filters.copy() + + def reject_module_parse(*args, **kwargs): + raise AssertionError("Metadata must not parse the full module") + + monkeypatch.setattr( + catalog, + "ast", + SimpleNamespace(parse=reject_module_parse, literal_eval=ast.literal_eval), + ) + result = CliRunner().invoke(app, command) + + assert ast.parse is parse + assert warnings.filters == filters + assert result.exit_code == 0, result.exception + assert result.stderr == "" + _assert_script_description(json.loads(result.stdout), command, "Deployment helper.") + + +@pytest.mark.parametrize("command", COMMANDS) +def test_description_does_not_parse_newer_syntax_after_docstring( + spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, command: list[str] +): + _write_python_script( + spec_kit_project, + '# -*- coding: utf-8 -*-\n"""Deployment helper."""\ntype FeatureId = str\n', + ) + monkeypatch.chdir(spec_kit_project) + result = CliRunner().invoke(app, command) + assert result.exit_code == 0, result.exception + assert result.stderr == "" + _assert_script_description(json.loads(result.stdout), command, "Deployment helper.") + + +def test_overlapping_metadata_requests_preserve_warning_filters(): + import subprocess + import sys + import textwrap + + source = textwrap.dedent("""\ + import ast + import warnings + from concurrent.futures import ThreadPoolExecutor + from threading import Event, Lock + from types import SimpleNamespace + from specify_cli.artifacts import catalog + + first_entered, second_entered, first_finished = Event(), Event(), Event() + lock = Lock() + calls = 0 + def gated(function): + def run(source): + global calls + with lock: + position = calls + calls += 1 + if position == 0: + first_entered.set() + assert second_entered.wait(5) + elif position == 1: + second_entered.set() + assert first_finished.wait(5) + return function(source) + return run + catalog.ast = SimpleNamespace(parse=gated(ast.parse), + literal_eval=gated(ast.literal_eval), get_docstring=ast.get_docstring) + before = warnings.filters.copy() + with ThreadPoolExecutor(max_workers=2) as pool: + first = pool.submit(catalog._extract_script_description, + '\\"\\"\\"Deployment helper.\\"\\"\\"', is_python=True) + assert first_entered.wait(5) + second = pool.submit(catalog._extract_script_description, + '\\"\\"\\"Deployment helper.\\"\\"\\"', is_python=True) + try: + assert first.result(timeout=10) == 'Deployment helper.' + finally: + first_finished.set() + assert second.result(timeout=10) == 'Deployment helper.' + assert warnings.filters == before, 'metadata requests leaked process warning filters' + """) + result = subprocess.run( + [sys.executable, "-c", source], capture_output=True, text=True, timeout=20 + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize( + ("statement", "expected"), + [ + ('"""Deployment helper."""', "Deployment helper."), + ("'''Deployment helper.'''", "Deployment helper."), + ('"Deployment " "helper."', "Deployment helper."), + ('(("Deployment "\n# joining comment\nr"helper."))', "Deployment helper."), + ('u"Deployment " R"helper."', "Deployment helper."), + ('"Deployment " \\\n"helper."', "Deployment helper."), + ('"""\n Deployment helper.\n Details.\n"""', "Deployment helper."), + ('"Deployment helper."; type FeatureId = str', "Deployment helper."), + (r'r"Path \d+\new"', r"Path \d+\new"), + (r'"Path \d+"', r"Path \d+"), + (r'"Deploy\thelper."', "Deploy helper."), + (r'"Escaped \\d"', r"Escaped \d"), + (r'"Octal \400 \7777"', "Octal " + chr(0o400) + " " + chr(0o777) + "7"), + (r'"Unicode \u0041 \U00000042 \N{LATIN CAPITAL LETTER C}"', "Unicode A B C"), + ('"Line\\\ncontinued."', "Linecontinued."), + ('b"Not a docstring"', "Deployment header"), + ('f"Not a docstring"', "Deployment header"), + ('rf"Not a docstring"', "Deployment header"), + ('("Not a docstring",)', "Deployment header"), + ('("Not a docstring") "extra"', "Deployment header"), + ('"Not a docstring" ("argument")', "Deployment header"), + ('(("Not a docstring") "extra")', "Deployment header"), + ('("Not a docstring" + "extra")', "Deployment header"), + ('(() "Not a docstring")', "Deployment header"), + (r'"Malformed \x4"', "Deployment header"), + (r'"Malformed \u12"', "Deployment header"), + (r'"Malformed \U00110000"', "Deployment header"), + (r'"Malformed \N{UNKNOWN UNICODE NAME}"', "Deployment header"), + ('"""Unterminated', "Deployment header"), + ('(" ")', "Deployment header"), + ], +) +@pytest.mark.parametrize("command", COMMANDS[:2]) +def test_leading_string_semantics_without_compiler_warnings( + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + statement: str, + expected: str, + command: list[str], +): + _write_python_script( + spec_kit_project, + "# Deployment header\n" + + statement + + '\ntype FeatureId = str\nraise RuntimeError("must not execute")\n', + ) + monkeypatch.chdir(spec_kit_project) + with warnings.catch_warnings(record=True) as emitted: + warnings.simplefilter("always") + filters = warnings.filters.copy() + result = CliRunner().invoke(app, command) + assert warnings.filters == filters + assert result.exit_code == 0, result.exception + assert result.stderr == "" + assert emitted == [] + _assert_script_description(json.loads(result.stdout), command, expected) + + +@pytest.mark.parametrize("command", COMMANDS[:2]) +def test_python_string_expression_is_not_a_docstring( + spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, command: list[str] +): + _write_python_script(spec_kit_project, '"""Not a docstring""" + "suffix"\n') + monkeypatch.chdir(spec_kit_project) + result = CliRunner().invoke(app, command) + assert result.exit_code == 0, result.exception + assert result.stderr == "" + _assert_script_description(json.loads(result.stdout), command, "")