From 0117766dc51ac9e6e3d0351f0c99645c7470aa82 Mon Sep 17 00:00:00 2001 From: Y Micker <60364890+Ghoscro@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:18:30 +0800 Subject: [PATCH 1/4] fix(artifacts): read Python module descriptions after header comments Assisted-by: OpenAI Codex (model: GPT-6, autonomous) --- src/specify_cli/artifacts/catalog.py | 16 +++++++- tests/specify_cli/artifacts/test_catalog.py | 43 +++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 2780201cbf..17979ae21b 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -11,6 +11,7 @@ from __future__ import annotations +import ast import re import shlex from collections.abc import Iterable @@ -139,7 +140,7 @@ def _extract_frontmatter_description(text: str) -> str: return value if isinstance(value, str) else "" -def _extract_script_description(text: str) -> str: +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,6 +153,17 @@ def _extract_script_description(text: str) -> str: Anything unrecognized yields the empty string. """ + if is_python: + # Coding declarations and license comments may precede a module's + # docstring. Parse without executing the project script so those + # comments are not mistaken for its user-facing description. + try: + description = ast.get_docstring(ast.parse(text)) + except SyntaxError: + description = None + if description and description.strip(): + return description.strip().splitlines()[0].strip() + py_match = re.match(r'^(?:#![^\n]*\n)?\s*(?:"""|\'\'\')(.*?)(?:"""|\'\'\')', text, re.DOTALL) if py_match: first = py_match.group(1).strip().splitlines() @@ -186,7 +198,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 ): From db3b52aea84494828b504be81bb8d02ef14e42ef Mon Sep 17 00:00:00 2001 From: Y Micker <60364890+Ghoscro@users.noreply.github.com> Date: Sat, 24 Oct 2026 03:14:29 +0800 Subject: [PATCH 2/4] fix(artifacts): keep Python metadata parsing best effort Handle parser resource errors with the existing comment fallback and suppress metadata-only parser warnings locally. Assisted-by: OpenAI Codex (model: GPT-6, autonomous) --- src/specify_cli/artifacts/catalog.py | 7 +- .../artifacts/test_description_parsing.py | 91 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/specify_cli/artifacts/test_description_parsing.py diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 17979ae21b..148cc6be84 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -14,6 +14,7 @@ import ast import re import shlex +import warnings from collections.abc import Iterable from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, Literal @@ -158,8 +159,10 @@ def _extract_script_description(text: str, *, is_python: bool = False) -> str: # docstring. Parse without executing the project script so those # comments are not mistaken for its user-facing description. try: - description = ast.get_docstring(ast.parse(text)) - except SyntaxError: + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + description = ast.get_docstring(ast.parse(text)) + except (SyntaxError, RecursionError, MemoryError): description = None if description and description.strip(): return description.strip().splitlines()[0].strip() 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..95f391bca3 --- /dev/null +++ b/tests/specify_cli/artifacts/test_description_parsing.py @@ -0,0 +1,91 @@ +"""Best-effort Python metadata parsing must not disrupt artifact commands.""" + +from __future__ import annotations + +import json +import warnings +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from specify_cli import app + + +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"], + ids=["deep-addition", "deep-unary"], +) +@pytest.mark.parametrize("command", COMMANDS) +def test_parser_resource_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.") From ffe33b666d91bcd2ce1130998506ac4763c91c3d Mon Sep 17 00:00:00 2001 From: Y Micker <60364890+Ghoscro@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:36:24 +0800 Subject: [PATCH 3/4] fix(artifacts): retain fallback for legacy parser errors Assisted-by: OpenAI Codex (model: GPT-6, autonomous) --- src/specify_cli/artifacts/catalog.py | 2 +- .../artifacts/test_description_parsing.py | 39 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 148cc6be84..a94fac0c7d 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -162,7 +162,7 @@ def _extract_script_description(text: str, *, is_python: bool = False) -> str: with warnings.catch_warnings(): warnings.simplefilter("ignore") description = ast.get_docstring(ast.parse(text)) - except (SyntaxError, RecursionError, MemoryError): + except (SyntaxError, ValueError, RecursionError, MemoryError): description = None if description and description.strip(): return description.strip().splitlines()[0].strip() diff --git a/tests/specify_cli/artifacts/test_description_parsing.py b/tests/specify_cli/artifacts/test_description_parsing.py index 95f391bca3..59066a5730 100644 --- a/tests/specify_cli/artifacts/test_description_parsing.py +++ b/tests/specify_cli/artifacts/test_description_parsing.py @@ -2,14 +2,17 @@ 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 = [ @@ -49,11 +52,11 @@ def _assert_script_description(payload, command, expected): @pytest.mark.parametrize( "expression", - ["1+" * 9999 + "1", "+" * 6000 + "1"], - ids=["deep-addition", "deep-unary"], + ["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_resource_failures_use_comment_fallback( +def test_parser_failures_use_comment_fallback( spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, expression: str, @@ -89,3 +92,33 @@ def test_metadata_parse_suppresses_warnings_without_changing_filters( assert result.stderr == "" assert emitted == [] _assert_script_description(json.loads(result.stdout), command, "Deployment helper.") + + +@pytest.mark.parametrize("command", COMMANDS) +def test_legacy_nul_parser_value_error_uses_comment_fallback( + spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch, command: list[str] +): + _write_python_script(spec_kit_project, "# Deployment helper\n\0\n") + monkeypatch.chdir(spec_kit_project) + parse = ast.parse + filters = warnings.filters.copy() + + def parse_with_legacy_nul_error(source, *args, **kwargs): + if isinstance(source, str) and "\0" in source: + raise ValueError("source code string cannot contain null bytes") + return parse(source, *args, **kwargs) + + monkeypatch.setattr( + catalog, + "ast", + SimpleNamespace( + parse=parse_with_legacy_nul_error, get_docstring=ast.get_docstring + ), + ) + 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") From c271a47f26f7f065d85061a3067f6e60e113ddd1 Mon Sep 17 00:00:00 2001 From: Y Micker <60364890+Ghoscro@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:32:56 +0800 Subject: [PATCH 4/4] fix(artifacts): isolate leading docstring extraction Assisted-by: OpenAI Codex (model: GPT-6, autonomous) --- src/specify_cli/artifacts/catalog.py | 82 +++++++-- .../artifacts/test_description_parsing.py | 159 ++++++++++++++++-- 2 files changed, 217 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index a94fac0c7d..cd92ae4c5e 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -12,9 +12,11 @@ from __future__ import annotations import ast +import inspect +import io import re import shlex -import warnings +import tokenize from collections.abc import Iterable from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, Literal @@ -141,6 +143,55 @@ def _extract_frontmatter_description(text: str) -> str: return value if isinstance(value, str) else "" +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 ``""``. @@ -155,23 +206,26 @@ def _extract_script_description(text: str, *, is_python: bool = False) -> str: Anything unrecognized yields the empty string. """ if is_python: - # Coding declarations and license comments may precede a module's - # docstring. Parse without executing the project script so those - # comments are not mistaken for its user-facing description. try: - with warnings.catch_warnings(): - warnings.simplefilter("ignore") - description = ast.get_docstring(ast.parse(text)) - except (SyntaxError, ValueError, RecursionError, MemoryError): + 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() - - 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() + 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: diff --git a/tests/specify_cli/artifacts/test_description_parsing.py b/tests/specify_cli/artifacts/test_description_parsing.py index 59066a5730..81ce93e9b5 100644 --- a/tests/specify_cli/artifacts/test_description_parsing.py +++ b/tests/specify_cli/artifacts/test_description_parsing.py @@ -95,25 +95,24 @@ def test_metadata_parse_suppresses_warnings_without_changing_filters( @pytest.mark.parametrize("command", COMMANDS) -def test_legacy_nul_parser_value_error_uses_comment_fallback( +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, "# Deployment helper\n\0\n") + _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 parse_with_legacy_nul_error(source, *args, **kwargs): - if isinstance(source, str) and "\0" in source: - raise ValueError("source code string cannot contain null bytes") - return parse(source, *args, **kwargs) + def reject_module_parse(*args, **kwargs): + raise AssertionError("Metadata must not parse the full module") monkeypatch.setattr( catalog, "ast", - SimpleNamespace( - parse=parse_with_legacy_nul_error, get_docstring=ast.get_docstring - ), + SimpleNamespace(parse=reject_module_parse, literal_eval=ast.literal_eval), ) result = CliRunner().invoke(app, command) @@ -121,4 +120,144 @@ def parse_with_legacy_nul_error(source, *args, **kwargs): assert warnings.filters == filters assert result.exit_code == 0, result.exception assert result.stderr == "" - _assert_script_description(json.loads(result.stdout), command, "Deployment helper") + _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, "")