From 92288f09f92e8a4f4fd6595a26b3f94b87551815 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 17:34:33 -0700 Subject: [PATCH 1/4] feat(recipe): add actionable validation and error feedback for recipe and metadata fields - Centralize Python version check with float format and accepted range (3.8-3.14) - Add validation for container port (1-65535) and GitHub username format - Enforce valid Python identifier for package name and legal characters for project name - Wire metadata validator registry into decode_recipe - Render hints with error messages in TUI preview and propagate errors in headless mode --- src/protostar/cli/tui/recipe/preview.py | 6 +- src/protostar/cli/tui/recipe/screen.py | 2 +- src/protostar/config.py | 8 ++ src/protostar/init_draft.py | 6 +- src/protostar/metadata.py | 71 +++++++++++++- src/protostar/recipe.py | 31 +++--- src/protostar/workspace.py | 53 ++++++++++ tests/test_cli.py | 79 +++++++++++++++ tests/test_config.py | 31 ++++++ tests/test_recipe.py | 123 ++++++++++++++++++++++++ tests/test_tui.py | 61 ++++++++++++ 11 files changed, 455 insertions(+), 16 deletions(-) diff --git a/src/protostar/cli/tui/recipe/preview.py b/src/protostar/cli/tui/recipe/preview.py index 17a1322a..8b68492c 100644 --- a/src/protostar/cli/tui/recipe/preview.py +++ b/src/protostar/cli/tui/recipe/preview.py @@ -61,7 +61,11 @@ async def update_plan(self, draft: InitDraft) -> None: self._show(Text(f"Waiting for values: {', '.join(exc.variables)}.")) return except ProtostarError as exc: - self._show(Text(str(exc)), error=True) + hint = f" {exc.hint}" if exc.hint else "" + self._show( + Text.assemble(str(exc), (hint, "dim") if hint else ""), + error=True, + ) return paths, _ = planned_paths(manifest) dependencies = manifest.dependencies diff --git a/src/protostar/cli/tui/recipe/screen.py b/src/protostar/cli/tui/recipe/screen.py index 00e33fbd..2a968a13 100644 --- a/src/protostar/cli/tui/recipe/screen.py +++ b/src/protostar/cli/tui/recipe/screen.py @@ -429,7 +429,7 @@ def _current_draft(self, variables: Mapping[str, str] | None = None) -> InitDraf variables=tuple(sorted(variables.items())), allowed_secrets=fields.allowed_secrets, metadata=tuple(sorted(metadata.items())), - python_version=str(minimum) if minimum else None, + python_version=str(minimum) if minimum is not None else None, ) @on(VariableFields.Committed) diff --git a/src/protostar/config.py b/src/protostar/config.py index 133f4fde..5a9f6f58 100644 --- a/src/protostar/config.py +++ b/src/protostar/config.py @@ -311,6 +311,14 @@ def __post_init__(self) -> None: "Cannot configure both 'pre_commit = true' and 'prek = true'.", hint="Choose either pre_commit or prek as your default git hook manager in your configuration.", ) + if self.python_version is not None: + from .workspace import check_python_version + + check_python_version(self.python_version) + if self.github_username: + from .metadata import validate_github_username + + validate_github_username(self.github_username) normalized: dict[str, TemplateAliasConfig] = {} for k, v in self.templates.items(): diff --git a/src/protostar/init_draft.py b/src/protostar/init_draft.py index 6963fe0d..2dae071c 100644 --- a/src/protostar/init_draft.py +++ b/src/protostar/init_draft.py @@ -94,8 +94,10 @@ def resolve_init( if draft.analysis is not None and existing is None else ProjectFacts() ) - python = draft.python_version or ( - facts.python_version.value if facts.python_version else None + python = ( + draft.python_version + if draft.python_version is not None + else (facts.python_version.value if facts.python_version else None) ) config = ( replace(user_config, python_version=existing.python, ide=existing.ide) diff --git a/src/protostar/metadata.py b/src/protostar/metadata.py index a684b283..e786d1b4 100644 --- a/src/protostar/metadata.py +++ b/src/protostar/metadata.py @@ -3,12 +3,15 @@ from __future__ import annotations import enum -from collections.abc import Callable +import re +from collections.abc import Callable, Mapping from dataclasses import dataclass from typing import TYPE_CHECKING, Any +from .errors import ConfigurationError from .system import get_git_config from .workflows import TargetOS +from .workspace import check_python_version if TYPE_CHECKING: from .config import UserConfig @@ -20,6 +23,10 @@ "MetadataKey", "PromptType", "resolve_auto_metadata", + "validate_docker_port", + "validate_github_username", + "validate_metadata", + "validate_minimum_python", ] @@ -94,6 +101,57 @@ class MetadataField: choices: list[str] | None auto_resolver: Callable[[UserConfig], Any | None] | None default: Any | None + validator: Callable[[Any], object] | None = None + + +_GITHUB_USERNAME_PATTERN = re.compile( + r"^[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])){0,38}$" +) + + +def validate_github_username(value: object) -> None: + """Validates that a GitHub username matches GitHub naming rules if provided.""" + if value is None or value == "": + return + if not isinstance(value, str): + raise ConfigurationError( + f"Invalid GitHub username: {value!r}.", + hint="GitHub username must be a string.", + ) + if value.startswith("@"): + raise ConfigurationError( + f"Invalid GitHub username: {value!r}.", + hint="Remove the leading '@' from GitHub username.", + ) + if not _GITHUB_USERNAME_PATTERN.fullmatch(value): + raise ConfigurationError( + f"Invalid GitHub username: {value!r}.", + hint="GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + ) + + +def validate_docker_port(value: object) -> None: + """Validates that a container port is an integer in the 1-65535 range.""" + port: int + if isinstance(value, int) and not isinstance(value, bool): + port = value + elif isinstance(value, str) and re.fullmatch(r"[+-]?\d+", value.strip()): + port = int(value.strip()) + else: + raise ConfigurationError( + f"Invalid container port: {value!r}.", + hint="Container port must be an integer (e.g., '8000').", + ) + if not (1 <= port <= 65535): + raise ConfigurationError( + f"Invalid container port: {value!r}.", + hint="Container port is outside the accepted range (1 - 65535).", + ) + + +def validate_minimum_python(value: object) -> None: + """Validates that a minimum Python version is a float within the supported range.""" + check_python_version(value, label="minimum Python version") METADATA_FIELDS: dict[MetadataKey, MetadataField] = { @@ -136,6 +194,7 @@ class MetadataField: choices=None, auto_resolver=lambda cfg: cfg.github_username, default="", + validator=validate_github_username, ), MetadataKey.MINIMUM_PYTHON: MetadataField( key=MetadataKey.MINIMUM_PYTHON, @@ -144,6 +203,7 @@ class MetadataField: choices=None, auto_resolver=lambda cfg: cfg.python_version, default="3.13", + validator=validate_minimum_python, ), MetadataKey.SUPPORTED_OS: MetadataField( key=MetadataKey.SUPPORTED_OS, @@ -160,10 +220,19 @@ class MetadataField: choices=None, auto_resolver=None, default="8000", + validator=validate_docker_port, ), } +def validate_metadata(metadata: Mapping[str, Any]) -> None: + """Validates all metadata fields present in the mapping against their defined validators.""" + for raw_key, field in METADATA_FIELDS.items(): + key_str = raw_key.value if isinstance(raw_key, MetadataKey) else str(raw_key) + if key_str in metadata and field.validator is not None: + field.validator(metadata[key_str]) + + def resolve_auto_metadata( keys: set[MetadataKey | str] | None = None, config: UserConfig | None = None, diff --git a/src/protostar/recipe.py b/src/protostar/recipe.py index 3b60a173..3bce3d0d 100644 --- a/src/protostar/recipe.py +++ b/src/protostar/recipe.py @@ -27,8 +27,15 @@ from .intent import TemplateOrigin, TemplateReference from .interpolation import BUILT_IN_VARIABLES, VARIABLE_NAME from .manifest import ProjectMetadata +from .metadata import validate_metadata from .options import CHOICE_VALUE, OptionValue -from .workspace import resolve_package_name, resolve_project_name +from .workspace import ( + check_python_version, + resolve_package_name, + resolve_project_name, + validate_package_name, + validate_project_name, +) if TYPE_CHECKING: from .config import TemplateSource, UserConfig @@ -340,11 +347,8 @@ def decode_recipe(data: object) -> ProjectRecipe: ): raise _invalid() data = {**{table: {} for table in _OPTIONAL_TABLES}, **data} - if ( - not isinstance(data["python"], str) - or not re.fullmatch(r"3\.\d+(?:\.\d+)?", data["python"]) - or type(data["docker"]) is not bool - ): + check_python_version(data["python"]) + if type(data["docker"]) is not bool: raise _invalid() try: ide = IDEType(data["ide"]) @@ -431,11 +435,10 @@ def tools(key: str) -> tuple[tuple[Tool, bool], ...]: or any(not isinstance(v, str) for v in context.values()) ): raise _invalid() + validate_package_name(context["PACKAGE_NAME"]) + validate_project_name(context["PROJECT_NAME"]) if ( - not context["PACKAGE_NAME"].isidentifier() - or any(c in context["PROJECT_NAME"] for c in ("/", "\\", "\x00")) - or context["PROJECT_NAME"] in {"", ".", ".."} - or not re.fullmatch(r"\d{4}", context["CURRENT_YEAR"]) + not re.fullmatch(r"\d{4}", context["CURRENT_YEAR"]) or context["PYTHON_VERSION"] != data["python"] ): raise _invalid() @@ -469,6 +472,11 @@ def tools(key: str) -> tuple[tuple[Tool, bool], ...]: or any( not ( isinstance(v, str) + or ( + k == "docker_port" + and isinstance(v, int) + and not isinstance(v, bool) + ) or ( k == "supported_os" and isinstance(v, list) @@ -481,6 +489,7 @@ def tools(key: str) -> tuple[tuple[Tool, bool], ...]: raise _invalid() if "supported_os" in metadata and not isinstance(metadata["supported_os"], list): raise _invalid() + validate_metadata(metadata) return ProjectRecipe( source, data["python"], @@ -652,7 +661,7 @@ def establish_recipe( reference = intent.reference python = intent.python docker = intent.docker - version = python or config.python_version or "3.13" + version = python if python is not None else (config.python_version or "3.13") project_name = resolve_project_name(metadata) if not Path("pyproject.toml").exists() and not any( metadata.get(k) for k in ("project_name", "name") diff --git a/src/protostar/workspace.py b/src/protostar/workspace.py index 3235ce4b..9cc296b8 100644 --- a/src/protostar/workspace.py +++ b/src/protostar/workspace.py @@ -7,17 +7,70 @@ from pathlib import Path from typing import Any +from .errors import ConfigurationError + __all__ = [ "PackageName", "ProjectName", "PythonVersion", + "check_python_version", "generate_python_version_range", "resolve_package_name", "resolve_project_name", "resolve_python_version", "sanitize_package_name", + "validate_package_name", + "validate_project_name", ] +_PYTHON_FLOAT_PATTERN = re.compile(r"^\d+\.\d+(?:\.\d+)?$") +MIN_SUPPORTED_PYTHON_MINOR = 8 +MAX_SUPPORTED_PYTHON_MINOR = 14 + + +def check_python_version(version: object, *, label: str = "Python version") -> str: + """Validates that a Python version is a float-like string within the supported range.""" + if not isinstance(version, str) or not _PYTHON_FLOAT_PATTERN.fullmatch(version): + raise ConfigurationError( + f"Invalid {label}: {version!r}.", + hint=f"{label} must be a float value (e.g., '3.13').", + ) + parts = version.split(".") + major, minor = int(parts[0]), int(parts[1]) + if major != 3 or not ( + MIN_SUPPORTED_PYTHON_MINOR <= minor <= MAX_SUPPORTED_PYTHON_MINOR + ): + raise ConfigurationError( + f"Invalid {label}: {version!r}.", + hint=f"{label} is outside the accepted range (3.{MIN_SUPPORTED_PYTHON_MINOR} - 3.{MAX_SUPPORTED_PYTHON_MINOR}).", + ) + return version + + +def validate_package_name(name: object) -> str: + """Validates that a package name is a valid Python identifier.""" + if not isinstance(name, str) or not name.isidentifier(): + raise ConfigurationError( + f"Invalid package name: {name!r}.", + hint="Package name must be a valid Python identifier (e.g., 'my_package').", + ) + return name + + +def validate_project_name(name: object) -> str: + """Validates that a project name contains no illegal path characters.""" + if ( + not isinstance(name, str) + or not name.strip() + or any(c in name for c in ("/", "\\", "\x00")) + or name in {".", ".."} + ): + raise ConfigurationError( + f"Invalid project name: {name!r}.", + hint="Project name cannot contain slashes or null bytes, and cannot be empty, '.', or '..'.", + ) + return name + @dataclass(frozen=True, order=True) class PythonVersion: diff --git a/tests/test_cli.py b/tests/test_cli.py index 3397b9e6..39b4fc61 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1785,3 +1785,82 @@ def test_dry_run_lists_files_generated_outside_the_filesystem_slice(monkeypatch) "settings.json", ): assert leaf in output + + +def test_init_invalid_python_version_headless_json_mode(run_cli): + code, stdout, _stderr, _ = run_cli("init", "--python-version", "invalid", "--json") + assert code != 0 + payload = json.loads(stdout) + assert payload["status"] == "error" + assert payload["error"]["type"] == "ConfigurationError" + assert payload["error"]["message"] == "Invalid Python version: 'invalid'." + assert ( + payload["error"]["hint"] + == "Python version must be a float value (e.g., '3.13')." + ) + + +def test_init_out_of_range_python_version_headless_json_mode(run_cli): + code, stdout, _stderr, _ = run_cli("init", "--python-version", "2.7", "--json") + assert code != 0 + payload = json.loads(stdout) + assert payload["status"] == "error" + assert payload["error"]["type"] == "ConfigurationError" + assert payload["error"]["message"] == "Invalid Python version: '2.7'." + assert ( + payload["error"]["hint"] + == "Python version is outside the accepted range (3.8 - 3.14)." + ) + + +def test_init_invalid_python_version_headless_terminal_mode(run_cli): + code, stdout, stderr, _ = run_cli("init", "--python-version", "invalid") + assert code != 0 + output = stdout + stderr + assert "Invalid Python version: 'invalid'." in output + assert "Hint: Python version must be a float value (e.g., '3.13')." in output + + +def test_sync_invalid_metadata_headless_json_mode(tmp_path, run_cli, monkeypatch): + monkeypatch.chdir(tmp_path) + from protostar.config import UserConfig + from protostar.recipe import edit_recipe, establish_recipe + + rec = establish_recipe(UserConfig()) + base_toml = '[project]\nname = "demo"\nversion = "0.1.0"\n' + toml_content = edit_recipe(base_toml, rec) + # Inject invalid docker_port under metadata + toml_content += '\n[tool.protostar.metadata]\ndocker_port = "invalid_port"\n' + (tmp_path / "pyproject.toml").write_text(toml_content) + + code, stdout, _stderr, _ = run_cli("sync", "--json") + assert code != 0 + payload = json.loads(stdout) + assert payload["status"] == "error" + assert payload["error"]["type"] == "ConfigurationError" + assert payload["error"]["message"] == "Invalid container port: 'invalid_port'." + assert ( + payload["error"]["hint"] == "Container port must be an integer (e.g., '8000')." + ) + + +def test_sync_invalid_github_username_headless_json_mode( + tmp_path, run_cli, monkeypatch +): + monkeypatch.chdir(tmp_path) + from protostar.config import UserConfig + from protostar.recipe import edit_recipe, establish_recipe + + rec = establish_recipe(UserConfig()) + base_toml = '[project]\nname = "demo"\nversion = "0.1.0"\n' + toml_content = edit_recipe(base_toml, rec) + toml_content += '\n[tool.protostar.metadata]\ngithub_username = "@octocat"\n' + (tmp_path / "pyproject.toml").write_text(toml_content) + + code, stdout, _stderr, _ = run_cli("sync", "--json") + assert code != 0 + payload = json.loads(stdout) + assert payload["status"] == "error" + assert payload["error"]["type"] == "ConfigurationError" + assert payload["error"]["message"] == "Invalid GitHub username: '@octocat'." + assert payload["error"]["hint"] == "Remove the leading '@' from GitHub username." diff --git a/tests/test_config.py b/tests/test_config.py index 1050bf81..9de52194 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -816,3 +816,34 @@ def test_disabled_configuration_ignores_the_default_file(mocker, tmp_path): select_config_source(None, disabled=True) assert UserConfig.load().author_name is None + + +def test_user_config_validates_python_version(): + with pytest.raises(ConfigurationError) as exc_info: + UserConfig(python_version="invalid") + assert "Invalid Python version: 'invalid'." in str(exc_info.value) + assert exc_info.value.hint == "Python version must be a float value (e.g., '3.13')." + + with pytest.raises(ConfigurationError) as exc_info: + UserConfig(python_version="2.7") + assert "Invalid Python version: '2.7'." in str(exc_info.value) + assert ( + exc_info.value.hint + == "Python version is outside the accepted range (3.8 - 3.14)." + ) + + +def test_user_config_validates_github_username(): + with pytest.raises(ConfigurationError) as exc_info: + UserConfig(github_username="@octocat") + assert "Invalid GitHub username: '@octocat'." in str(exc_info.value) + assert exc_info.value.hint == "Remove the leading '@' from GitHub username." + + with pytest.raises(ConfigurationError) as exc_info: + UserConfig(github_username="-octocat") + assert "Invalid GitHub username: '-octocat'." in str(exc_info.value) + assert exc_info.value.hint is not None + assert ( + "GitHub username may only contain alphanumeric characters" + in exc_info.value.hint + ) diff --git a/tests/test_recipe.py b/tests/test_recipe.py index 3c57f2c4..4505aba4 100644 --- a/tests/test_recipe.py +++ b/tests/test_recipe.py @@ -88,6 +88,129 @@ def test_strict_recipe_validation(mutation): decode_recipe(recipe().to_dict() | mutation) +@pytest.mark.parametrize( + ("invalid", "expected_hint"), + [ + ("foo", "Python version must be a float value (e.g., '3.13')."), + ("3.x", "Python version must be a float value (e.g., '3.13')."), + ("", "Python version must be a float value (e.g., '3.13')."), + ("3", "Python version must be a float value (e.g., '3.13')."), + ("2.7", "Python version is outside the accepted range (3.8 - 3.14)."), + ("4.0", "Python version is outside the accepted range (3.8 - 3.14)."), + ("3.5", "Python version is outside the accepted range (3.8 - 3.14)."), + ("3.15", "Python version is outside the accepted range (3.8 - 3.14)."), + ], +) +def test_invalid_python_version_recipe_error(invalid, expected_hint): + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"python": invalid}) + assert f"Invalid Python version: {invalid!r}." in str(exc_info.value) + assert exc_info.value.hint == expected_hint + + +@pytest.mark.parametrize( + ("invalid", "expected_hint"), + [ + ("foo", "minimum Python version must be a float value (e.g., '3.13')."), + ("2.7", "minimum Python version is outside the accepted range (3.8 - 3.14)."), + ], +) +def test_invalid_metadata_minimum_python_error(invalid, expected_hint): + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"metadata": {"minimum_python": invalid}}) + assert f"Invalid minimum Python version: {invalid!r}." in str(exc_info.value) + assert exc_info.value.hint == expected_hint + + +@pytest.mark.parametrize( + ("invalid", "expected_hint"), + [ + ("abc", "Container port must be an integer (e.g., '8000')."), + ("", "Container port must be an integer (e.g., '8000')."), + (0, "Container port is outside the accepted range (1 - 65535)."), + (65536, "Container port is outside the accepted range (1 - 65535)."), + ("70000", "Container port is outside the accepted range (1 - 65535)."), + (-1, "Container port is outside the accepted range (1 - 65535)."), + ], +) +def test_invalid_metadata_docker_port_error(invalid, expected_hint): + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"metadata": {"docker_port": invalid}}) + assert f"Invalid container port: {invalid!r}." in str(exc_info.value) + assert exc_info.value.hint == expected_hint + + +@pytest.mark.parametrize("valid", [8000, "8000", 1, 65535, "1", "65535"]) +def test_valid_metadata_docker_port(valid): + decoded = decode_recipe(recipe().to_dict() | {"metadata": {"docker_port": valid}}) + assert dict(decoded.metadata)["docker_port"] == valid + + +@pytest.mark.parametrize( + ("invalid", "expected_hint"), + [ + ("@octocat", "Remove the leading '@' from GitHub username."), + ( + "-user", + "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + ), + ( + "user-", + "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + ), + ( + "user_name", + "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + ), + ( + "a" * 40, + "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + ), + ], +) +def test_invalid_metadata_github_username_error(invalid, expected_hint): + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"metadata": {"github_username": invalid}}) + assert f"Invalid GitHub username: {invalid!r}." in str(exc_info.value) + assert exc_info.value.hint == expected_hint + + +@pytest.mark.parametrize("valid", ["", "octocat", "user-name-123", "a"]) +def test_valid_metadata_github_username(valid): + decoded = decode_recipe( + recipe().to_dict() | {"metadata": {"github_username": valid}} + ) + assert dict(decoded.metadata)["github_username"] == valid + + +@pytest.mark.parametrize("invalid", ["my-package", "123pkg", "pkg name", "pkg.name"]) +def test_invalid_context_package_name_error(invalid): + current = dict(recipe().context) + current["PACKAGE_NAME"] = invalid + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"context": current}) + assert f"Invalid package name: {invalid!r}." in str(exc_info.value) + assert ( + exc_info.value.hint + == "Package name must be a valid Python identifier (e.g., 'my_package')." + ) + + +@pytest.mark.parametrize( + "invalid", ["foo/bar", "foo\\bar", "foo\0bar", "", " ", ".", ".."] +) +def test_invalid_context_project_name_error(invalid): + current = dict(recipe().context) + current["PROJECT_NAME"] = invalid + with pytest.raises(ConfigurationError) as exc_info: + decode_recipe(recipe().to_dict() | {"context": current}) + assert f"Invalid project name: {invalid!r}." in str(exc_info.value) + assert ( + exc_info.value.hint + == "Project name cannot contain slashes or null bytes, and cannot be empty, '.', or '..'." + ) + + def test_recorded_variables_render_and_persist(): recorded = replace(recipe(), variables=(("REGION", "eu-west-1"),)) diff --git a/tests/test_tui.py b/tests/test_tui.py index a514e2cb..a51794d4 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -808,6 +808,67 @@ async def test_preview_lists_collisions(workspace): assert "Already exist: pyproject.toml" in plain(app, "#preview-collisions") +@pytest.mark.asyncio +async def test_invalid_minimum_python_shows_actionable_preview_error(): + app = make_app() + async with app.run_test(size=(110, 45)) as pilot: + await settle(pilot) + field = app.screen.query_one("#meta-minimum_python", Input) + field.focus() + field.value = "invalid" + await pilot.press("enter") + await settle(pilot) + summary = plain(app, "#preview-summary") + assert "Invalid Python version: 'invalid'." in summary + assert "Python version must be a float value (e.g., '3.13')." in summary + field.focus() + field.value = "2.7" + await pilot.press("enter") + await settle(pilot) + summary = plain(app, "#preview-summary") + assert "Invalid Python version: '2.7'." in summary + assert "Python version is outside the accepted range (3.8 - 3.14)." in summary + field.focus() + field.value = "" + await pilot.press("enter") + await settle(pilot) + summary = plain(app, "#preview-summary") + assert "Invalid Python version: ''." in summary + assert "Python version must be a float value (e.g., '3.13')." in summary + + +@pytest.mark.asyncio +async def test_invalid_github_username_shows_actionable_preview_error(): + app = make_app() + async with app.run_test(size=(110, 45)) as pilot: + await settle(pilot) + field = app.screen.query_one("#meta-github_username", Input) + field.focus() + field.value = "@octocat" + await pilot.press("enter") + await settle(pilot) + summary = plain(app, "#preview-summary") + assert "Invalid GitHub username: '@octocat'." in summary + assert "Remove the leading '@' from GitHub username." in summary + + +@pytest.mark.asyncio +async def test_invalid_docker_port_shows_actionable_preview_error(): + app = make_app() + async with app.run_test(size=(110, 45)) as pilot: + await settle(pilot) + app.screen.query_one("#docker", Checkbox).value = True + await settle(pilot) + field = app.screen.query_one("#meta-docker_port", Input) + field.focus() + field.value = "notaport" + await pilot.press("enter") + await settle(pilot) + summary = plain(app, "#preview-summary") + assert "Invalid container port: 'notaport'." in summary + assert "Container port must be an integer (e.g., '8000')." in summary + + @pytest.mark.asyncio async def test_variables_step_focuses_the_missing_value(tmp_path): draft = template_draft( From 5fb6cb0216edd1947ffe1993afa34abc24962fc0 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 17:38:31 -0700 Subject: [PATCH 2/4] feat(tui): block navigation to review when recipe draft has fatal issues - Disable Continue button and guard ctrl+s when recipe draft has errors - Check draft validity synchronously on field change to update button state - Emit PlanUpdated message from PlanPreview to keep continue button in sync - Verify with TUI tests that ctrl+s and continue are blocked until issues resolved --- src/protostar/cli/tui/recipe/preview.py | 15 +++++++ src/protostar/cli/tui/recipe/screen.py | 56 ++++++++++++++++++++----- tests/test_tui.py | 31 ++++++++++++++ 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/src/protostar/cli/tui/recipe/preview.py b/src/protostar/cli/tui/recipe/preview.py index 8b68492c..5aca6ec8 100644 --- a/src/protostar/cli/tui/recipe/preview.py +++ b/src/protostar/cli/tui/recipe/preview.py @@ -7,6 +7,7 @@ from textual import work from textual.app import ComposeResult from textual.containers import VerticalScroll +from textual.message import Message from textual.widgets import Static from protostar.cli.ui import plan_tree, planned_paths @@ -32,9 +33,17 @@ def _count(number: int, noun: str) -> str: class PlanPreview(VerticalScroll): """The tree ``--dry-run`` prints, plus any collisions, for the current draft.""" + class PlanUpdated(Message): + """Posted when planning finishes or fails.""" + + def __init__(self, *, error: ProtostarError | None = None) -> None: + super().__init__() + self.error = error + def __init__(self, config: UserConfig) -> None: super().__init__() self.config = config + self.error: ProtostarError | None = None # Held, not queried: a plan can finish while the app tears its # children down, and updating a removed line is harmless. self._summary = Static("Planning…", id="preview-summary") @@ -58,15 +67,20 @@ async def update_plan(self, draft: InitDraft) -> None: try: manifest = await asyncio.to_thread(_plan, draft, self.config) except MissingTemplateVariablesError as exc: + self.error = None self._show(Text(f"Waiting for values: {', '.join(exc.variables)}.")) + self.post_message(self.PlanUpdated(error=None)) return except ProtostarError as exc: + self.error = exc hint = f" {exc.hint}" if exc.hint else "" self._show( Text.assemble(str(exc), (hint, "dim") if hint else ""), error=True, ) + self.post_message(self.PlanUpdated(error=exc)) return + self.error = None paths, _ = planned_paths(manifest) dependencies = manifest.dependencies packages = ( @@ -94,6 +108,7 @@ async def update_plan(self, draft: InitDraft) -> None: else Text(""), tree=plan_tree(manifest) if paths else Text(""), ) + self.post_message(self.PlanUpdated(error=None)) def _show( self, diff --git a/src/protostar/cli/tui/recipe/screen.py b/src/protostar/cli/tui/recipe/screen.py index 2a968a13..55e5cd48 100644 --- a/src/protostar/cli/tui/recipe/screen.py +++ b/src/protostar/cli/tui/recipe/screen.py @@ -28,8 +28,17 @@ from protostar.analysis import NoteKind, ProjectAnalysis from protostar.config import TemplateSource, UserConfig -from protostar.errors import ConfigurationError, ProtostarError -from protostar.init_draft import DraftTemplate, InitDecision, InitDraft +from protostar.errors import ( + ConfigurationError, + MissingTemplateVariablesError, + ProtostarError, +) +from protostar.init_draft import ( + DraftTemplate, + InitDecision, + InitDraft, + resolve_init, +) from protostar.metadata import MetadataKey from protostar.modules import TOOLING_MODULES from protostar.recipe import ( @@ -110,6 +119,8 @@ def __init__( self._template_error = False self._loading = False self._tools_invalid = False + self._draft_error = False + self._plan_error = False self.catalog = catalog self.base_recipe = draft.existing_recipe or establish_recipe(config) self.overrides = dict(draft.tool_overrides) @@ -408,9 +419,23 @@ def _refresh_tools(self) -> None: def _refresh_continue(self) -> None: self.query_one("#continue", Button).disabled = ( - self._tools_invalid or self._template_error or self._loading + self._tools_invalid + or self._template_error + or self._loading + or self._draft_error + or self._plan_error ) + def _check_draft(self, draft: InitDraft) -> None: + try: + resolve_init(draft, self.config) + self._draft_error = False + except MissingTemplateVariablesError: + self._draft_error = False + except ProtostarError: + self._draft_error = True + self._refresh_continue() + def _current_draft(self, variables: Mapping[str, str] | None = None) -> InitDraft: fields = self.query_one(VariableFields) if variables is None: @@ -447,7 +472,14 @@ def _changed(self) -> None: docker=self._docker(), ) ) - self.query_one(PlanPreview).update_plan(self._current_draft()) + draft = self._current_draft() + self._check_draft(draft) + self.query_one(PlanPreview).update_plan(draft) + + @on(PlanPreview.PlanUpdated) + def _plan_updated(self, event: PlanPreview.PlanUpdated) -> None: + self._plan_error = event.error is not None + self._refresh_continue() @on(Select.Changed, "#template") def select_template(self, event: Select.Changed) -> None: @@ -584,9 +616,13 @@ def action_continue(self) -> None: if self.query_one("#continue", Button).disabled: return variables = self.query_one(VariableFields).values() - if variables is not None: - self.app.push_screen( - ReviewScreen( - self._current_draft(variables), self.config, can_go_back=True - ) - ) + if variables is None: + return + draft = self._current_draft(variables) + try: + resolve_init(draft, self.config) + except ProtostarError: + self._draft_error = True + self._refresh_continue() + return + self.app.push_screen(ReviewScreen(draft, self.config, can_go_back=True)) diff --git a/tests/test_tui.py b/tests/test_tui.py index a51794d4..b4c4e493 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -869,6 +869,37 @@ async def test_invalid_docker_port_shows_actionable_preview_error(): assert "Container port must be an integer (e.g., '8000')." in summary +@pytest.mark.asyncio +async def test_fatal_recipe_issues_block_continue_and_ctrl_s(): + app = make_app() + async with app.run_test(size=(110, 45)) as pilot: + await settle(pilot) + continue_btn = app.screen.query_one("#continue", Button) + assert not continue_btn.disabled + + field = app.screen.query_one("#meta-minimum_python", Input) + field.focus() + field.value = "invalid" + await pilot.press("enter") + await settle(pilot) + + assert continue_btn.disabled + await pilot.press("ctrl+s") + await settle(pilot) + assert isinstance(app.screen, RecipeScreen) + + # Fix the field and verify Continue is re-enabled and ctrl+s works + field.focus() + field.value = "3.13" + await pilot.press("enter") + await settle(pilot) + + assert not continue_btn.disabled + await pilot.press("ctrl+s") + await settle(pilot) + assert isinstance(app.screen, ReviewScreen) + + @pytest.mark.asyncio async def test_variables_step_focuses_the_missing_value(tmp_path): draft = template_draft( From c9a445319281d3eb508aa7f9d90bed8f02f7c187 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 17:55:29 -0700 Subject: [PATCH 3/4] fix(recipe): make recipe validation exact so it never rejects a valid value Validation now rejects only values that are actually invalid: - Python versions: any 3.x[.y] is accepted. The 3.8-3.14 range blocked adopting a project with requires-python >=3.7 and would have broken every recipe and config recording 3.15. A malformed version and a non-3 major get separate hints. - GitHub usernames: legacy names ending in or repeating a hyphen, and Enterprise Managed User `_shortcode` names, are accepted. - Container ports: exactly digits, so '+8000' and ' 80 ' no longer pass and render verbatim into generated files. - Empty metadata is unset, not invalid, and clearing Minimum Python in the editor falls back to the default again. - Project names: the rule main enforced; a whitespace-only name is not newly rejected. The editor checks field values with the new check_draft, which renders nothing, instead of running resolve_init on the UI thread. Nits: validator typing, the dead key branch in validate_metadata, lazy imports in config.py, and PlanPreview's unused error attribute. --- src/protostar/cli/tui/recipe/preview.py | 13 +-- src/protostar/cli/tui/recipe/screen.py | 38 +++----- src/protostar/config.py | 6 +- src/protostar/init_draft.py | 18 ++++ src/protostar/metadata.py | 64 +++++++++----- src/protostar/workspace.py | 60 ++++++++----- tests/test_cli.py | 27 ++++-- tests/test_config.py | 19 ++-- tests/test_recipe.py | 111 +++++++++++++++--------- tests/test_tui.py | 33 ++++--- 10 files changed, 236 insertions(+), 153 deletions(-) diff --git a/src/protostar/cli/tui/recipe/preview.py b/src/protostar/cli/tui/recipe/preview.py index 5aca6ec8..ea6182e3 100644 --- a/src/protostar/cli/tui/recipe/preview.py +++ b/src/protostar/cli/tui/recipe/preview.py @@ -43,7 +43,6 @@ def __init__(self, *, error: ProtostarError | None = None) -> None: def __init__(self, config: UserConfig) -> None: super().__init__() self.config = config - self.error: ProtostarError | None = None # Held, not queried: a plan can finish while the app tears its # children down, and updating a removed line is harmless. self._summary = Static("Planning…", id="preview-summary") @@ -67,20 +66,16 @@ async def update_plan(self, draft: InitDraft) -> None: try: manifest = await asyncio.to_thread(_plan, draft, self.config) except MissingTemplateVariablesError as exc: - self.error = None self._show(Text(f"Waiting for values: {', '.join(exc.variables)}.")) self.post_message(self.PlanUpdated(error=None)) return except ProtostarError as exc: - self.error = exc - hint = f" {exc.hint}" if exc.hint else "" - self._show( - Text.assemble(str(exc), (hint, "dim") if hint else ""), - error=True, - ) + message = Text(str(exc)) + if exc.hint: + message.append(f" {exc.hint}", style="dim") + self._show(message, error=True) self.post_message(self.PlanUpdated(error=exc)) return - self.error = None paths, _ = planned_paths(manifest) dependencies = manifest.dependencies packages = ( diff --git a/src/protostar/cli/tui/recipe/screen.py b/src/protostar/cli/tui/recipe/screen.py index 55e5cd48..2a06021b 100644 --- a/src/protostar/cli/tui/recipe/screen.py +++ b/src/protostar/cli/tui/recipe/screen.py @@ -28,17 +28,8 @@ from protostar.analysis import NoteKind, ProjectAnalysis from protostar.config import TemplateSource, UserConfig -from protostar.errors import ( - ConfigurationError, - MissingTemplateVariablesError, - ProtostarError, -) -from protostar.init_draft import ( - DraftTemplate, - InitDecision, - InitDraft, - resolve_init, -) +from protostar.errors import ConfigurationError, ProtostarError +from protostar.init_draft import DraftTemplate, InitDecision, InitDraft, check_draft from protostar.metadata import MetadataKey from protostar.modules import TOOLING_MODULES from protostar.recipe import ( @@ -426,15 +417,16 @@ def _refresh_continue(self) -> None: or self._plan_error ) - def _check_draft(self, draft: InitDraft) -> None: + def _check_draft(self, draft: InitDraft) -> bool: + """Disable Continue at once for an invalid field; the preview says why.""" try: - resolve_init(draft, self.config) - self._draft_error = False - except MissingTemplateVariablesError: - self._draft_error = False - except ProtostarError: + check_draft(draft) + except ConfigurationError: self._draft_error = True + else: + self._draft_error = False self._refresh_continue() + return not self._draft_error def _current_draft(self, variables: Mapping[str, str] | None = None) -> InitDraft: fields = self.query_one(VariableFields) @@ -454,7 +446,8 @@ def _current_draft(self, variables: Mapping[str, str] | None = None) -> InitDraf variables=tuple(sorted(variables.items())), allowed_secrets=fields.allowed_secrets, metadata=tuple(sorted(metadata.items())), - python_version=str(minimum) if minimum is not None else None, + # An empty minimum leaves the configured or detected default. + python_version=str(minimum) if minimum else None, ) @on(VariableFields.Committed) @@ -619,10 +612,5 @@ def action_continue(self) -> None: if variables is None: return draft = self._current_draft(variables) - try: - resolve_init(draft, self.config) - except ProtostarError: - self._draft_error = True - self._refresh_continue() - return - self.app.push_screen(ReviewScreen(draft, self.config, can_go_back=True)) + if self._check_draft(draft): + self.app.push_screen(ReviewScreen(draft, self.config, can_go_back=True)) diff --git a/src/protostar/config.py b/src/protostar/config.py index 5a9f6f58..3f96fef1 100644 --- a/src/protostar/config.py +++ b/src/protostar/config.py @@ -33,9 +33,11 @@ validate_target, ) from .interpolation import BUILT_IN_VARIABLES, extract_variables, render_template +from .metadata import validate_github_username from .migrations import Migration, parse_migrations from .network import RemoteTemplate, fetch_remote_template from .options import Condition, TemplateOption, parse_condition, parse_options +from .workspace import check_python_version logger = logging.getLogger("protostar") @@ -312,12 +314,8 @@ def __post_init__(self) -> None: hint="Choose either pre_commit or prek as your default git hook manager in your configuration.", ) if self.python_version is not None: - from .workspace import check_python_version - check_python_version(self.python_version) if self.github_username: - from .metadata import validate_github_username - validate_github_username(self.github_username) normalized: dict[str, TemplateAliasConfig] = {} diff --git a/src/protostar/init_draft.py b/src/protostar/init_draft.py index 2dae071c..19419e49 100644 --- a/src/protostar/init_draft.py +++ b/src/protostar/init_draft.py @@ -9,6 +9,7 @@ from .config import TemplateSource, UserConfig from .manifest import CollisionStrategy, ProjectMetadata from .merge import NO_RESOLUTIONS, Resolutions +from .metadata import validate_metadata from .models import InitRequest from .modules import BootstrapModule, PythonCore, SystemWorkspaceModule from .options import OptionValue, resolve_options @@ -22,6 +23,7 @@ ) from .registry import ResolvedHookRevision from .secret_guard import check_variable_values +from .workspace import check_python_version @dataclass(frozen=True) @@ -83,6 +85,22 @@ class InitDecision: resolutions: Resolutions = NO_RESOLUTIONS +def check_draft(draft: InitDraft) -> None: + """Validate the draft's own field values without resolving it. + + It renders nothing, so an editor can call it on every change. The recipe + ``resolve_init`` decodes applies the same checks. + + Raises: + ConfigurationError: If the Python version or a metadata value is + invalid. + """ + if draft.python_version is not None: + check_python_version(draft.python_version) + if draft.metadata is not None: + validate_metadata(dict(draft.metadata)) + + def resolve_init( draft: InitDraft, user_config: UserConfig ) -> tuple[list[BootstrapModule], InitRequest]: diff --git a/src/protostar/metadata.py b/src/protostar/metadata.py index e786d1b4..c83e7f37 100644 --- a/src/protostar/metadata.py +++ b/src/protostar/metadata.py @@ -101,18 +101,23 @@ class MetadataField: choices: list[str] | None auto_resolver: Callable[[UserConfig], Any | None] | None default: Any | None - validator: Callable[[Any], object] | None = None + validator: Callable[[object], None] | None = None -_GITHUB_USERNAME_PATTERN = re.compile( - r"^[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])){0,38}$" -) +# Leading character alphanumeric, then alphanumerics and hyphens. Accounts from +# before GitHub's current rules may end in or repeat a hyphen, and Enterprise +# Managed Users carry an ``_shortcode`` suffix, so neither is rejected. +_GITHUB_USERNAME_PATTERN = re.compile(r"[A-Za-z0-9][A-Za-z0-9_-]{0,38}") +_PORT_PATTERN = re.compile(r"[0-9]+") +_MAX_PORT = 65535 def validate_github_username(value: object) -> None: - """Validates that a GitHub username matches GitHub naming rules if provided.""" - if value is None or value == "": - return + """Validates a GitHub user or organization name. + + Raises: + ConfigurationError: If the value could not name a GitHub account. + """ if not isinstance(value, str): raise ConfigurationError( f"Invalid GitHub username: {value!r}.", @@ -121,36 +126,43 @@ def validate_github_username(value: object) -> None: if value.startswith("@"): raise ConfigurationError( f"Invalid GitHub username: {value!r}.", - hint="Remove the leading '@' from GitHub username.", + hint=f"Drop the leading '@': use {value[1:]!r}.", ) if not _GITHUB_USERNAME_PATTERN.fullmatch(value): raise ConfigurationError( f"Invalid GitHub username: {value!r}.", - hint="GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", + hint="A GitHub username is at most 39 letters, digits, and hyphens, starting with a letter or digit.", ) def validate_docker_port(value: object) -> None: - """Validates that a container port is an integer in the 1-65535 range.""" - port: int + """Validates a container port: a whole number from 1 to 65535. + + Raises: + ConfigurationError: If the value is not a port number. + """ if isinstance(value, int) and not isinstance(value, bool): port = value - elif isinstance(value, str) and re.fullmatch(r"[+-]?\d+", value.strip()): - port = int(value.strip()) + elif isinstance(value, str) and _PORT_PATTERN.fullmatch(value): + port = int(value) else: raise ConfigurationError( f"Invalid container port: {value!r}.", - hint="Container port must be an integer (e.g., '8000').", + hint="Container port must be a whole number, such as '8000'.", ) - if not (1 <= port <= 65535): + if not 1 <= port <= _MAX_PORT: raise ConfigurationError( f"Invalid container port: {value!r}.", - hint="Container port is outside the accepted range (1 - 65535).", + hint=f"Container port must be between 1 and {_MAX_PORT}.", ) def validate_minimum_python(value: object) -> None: - """Validates that a minimum Python version is a float within the supported range.""" + """Validates a minimum Python version such as ``3.10``. + + Raises: + ConfigurationError: If the value is not a Python 3 version. + """ check_python_version(value, label="minimum Python version") @@ -225,12 +237,18 @@ def validate_minimum_python(value: object) -> None: } -def validate_metadata(metadata: Mapping[str, Any]) -> None: - """Validates all metadata fields present in the mapping against their defined validators.""" - for raw_key, field in METADATA_FIELDS.items(): - key_str = raw_key.value if isinstance(raw_key, MetadataKey) else str(raw_key) - if key_str in metadata and field.validator is not None: - field.validator(metadata[key_str]) +def validate_metadata(metadata: Mapping[str, object]) -> None: + """Validates each metadata value that has a validator. + + An empty value leaves the field unset, so it is never invalid. + + Raises: + ConfigurationError: If a value is invalid for its field. + """ + for key, field in METADATA_FIELDS.items(): + value = metadata.get(key) + if value not in (None, "") and field.validator is not None: + field.validator(value) def resolve_auto_metadata( diff --git a/src/protostar/workspace.py b/src/protostar/workspace.py index 9cc296b8..4baedb38 100644 --- a/src/protostar/workspace.py +++ b/src/protostar/workspace.py @@ -23,53 +23,67 @@ "validate_project_name", ] -_PYTHON_FLOAT_PATTERN = re.compile(r"^\d+\.\d+(?:\.\d+)?$") -MIN_SUPPORTED_PYTHON_MINOR = 8 -MAX_SUPPORTED_PYTHON_MINOR = 14 +_PYTHON_VERSION_PATTERN = re.compile(r"(\d+)\.\d+(?:\.\d+)?") -def check_python_version(version: object, *, label: str = "Python version") -> str: - """Validates that a Python version is a float-like string within the supported range.""" - if not isinstance(version, str) or not _PYTHON_FLOAT_PATTERN.fullmatch(version): +def check_python_version(version: object, *, label: str = "Python version") -> None: + """Validates a Python 3 version such as ``3.13`` or ``3.13.1``. + + No minor version is out of range: a project may support an old Python, and + a new one is valid the day it ships. + + Args: + version: The value to check. + label: How error messages name the value. + + Raises: + ConfigurationError: If the value is not a ``major.minor[.patch]`` + version, or its major version is not 3. + """ + match = ( + _PYTHON_VERSION_PATTERN.fullmatch(version) if isinstance(version, str) else None + ) + if match is None: raise ConfigurationError( f"Invalid {label}: {version!r}.", - hint=f"{label} must be a float value (e.g., '3.13').", + hint=f"Write the {label} as major.minor, such as '3.13'.", ) - parts = version.split(".") - major, minor = int(parts[0]), int(parts[1]) - if major != 3 or not ( - MIN_SUPPORTED_PYTHON_MINOR <= minor <= MAX_SUPPORTED_PYTHON_MINOR - ): + if match.group(1) != "3": raise ConfigurationError( - f"Invalid {label}: {version!r}.", - hint=f"{label} is outside the accepted range (3.{MIN_SUPPORTED_PYTHON_MINOR} - 3.{MAX_SUPPORTED_PYTHON_MINOR}).", + f"Unsupported {label}: {version!r}.", + hint="Protostar scaffolds Python 3 projects; choose a 3.x version such as '3.13'.", ) - return version -def validate_package_name(name: object) -> str: - """Validates that a package name is a valid Python identifier.""" +def validate_package_name(name: object) -> None: + """Validates that a package name is a valid Python identifier. + + Raises: + ConfigurationError: If the name is not an identifier. + """ if not isinstance(name, str) or not name.isidentifier(): raise ConfigurationError( f"Invalid package name: {name!r}.", hint="Package name must be a valid Python identifier (e.g., 'my_package').", ) - return name -def validate_project_name(name: object) -> str: - """Validates that a project name contains no illegal path characters.""" +def validate_project_name(name: object) -> None: + """Validates that a project name can name a single directory. + + Raises: + ConfigurationError: If the name is empty, '.', '..', or contains a path + separator or null byte. + """ if ( not isinstance(name, str) - or not name.strip() + or name in {"", ".", ".."} or any(c in name for c in ("/", "\\", "\x00")) - or name in {".", ".."} ): raise ConfigurationError( f"Invalid project name: {name!r}.", hint="Project name cannot contain slashes or null bytes, and cannot be empty, '.', or '..'.", ) - return name @dataclass(frozen=True, order=True) diff --git a/tests/test_cli.py b/tests/test_cli.py index 39b4fc61..9dda69ce 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1796,20 +1796,20 @@ def test_init_invalid_python_version_headless_json_mode(run_cli): assert payload["error"]["message"] == "Invalid Python version: 'invalid'." assert ( payload["error"]["hint"] - == "Python version must be a float value (e.g., '3.13')." + == "Write the Python version as major.minor, such as '3.13'." ) -def test_init_out_of_range_python_version_headless_json_mode(run_cli): +def test_init_python_2_version_headless_json_mode(run_cli): code, stdout, _stderr, _ = run_cli("init", "--python-version", "2.7", "--json") assert code != 0 payload = json.loads(stdout) assert payload["status"] == "error" assert payload["error"]["type"] == "ConfigurationError" - assert payload["error"]["message"] == "Invalid Python version: '2.7'." + assert payload["error"]["message"] == "Unsupported Python version: '2.7'." assert ( payload["error"]["hint"] - == "Python version is outside the accepted range (3.8 - 3.14)." + == "Protostar scaffolds Python 3 projects; choose a 3.x version such as '3.13'." ) @@ -1818,7 +1818,7 @@ def test_init_invalid_python_version_headless_terminal_mode(run_cli): assert code != 0 output = stdout + stderr assert "Invalid Python version: 'invalid'." in output - assert "Hint: Python version must be a float value (e.g., '3.13')." in output + assert "Hint: Write the Python version as major.minor, such as '3.13'." in output def test_sync_invalid_metadata_headless_json_mode(tmp_path, run_cli, monkeypatch): @@ -1840,7 +1840,8 @@ def test_sync_invalid_metadata_headless_json_mode(tmp_path, run_cli, monkeypatch assert payload["error"]["type"] == "ConfigurationError" assert payload["error"]["message"] == "Invalid container port: 'invalid_port'." assert ( - payload["error"]["hint"] == "Container port must be an integer (e.g., '8000')." + payload["error"]["hint"] + == "Container port must be a whole number, such as '8000'." ) @@ -1863,4 +1864,16 @@ def test_sync_invalid_github_username_headless_json_mode( assert payload["status"] == "error" assert payload["error"]["type"] == "ConfigurationError" assert payload["error"]["message"] == "Invalid GitHub username: '@octocat'." - assert payload["error"]["hint"] == "Remove the leading '@' from GitHub username." + assert payload["error"]["hint"] == "Drop the leading '@': use 'octocat'." + + +def test_init_adopts_a_project_supporting_an_old_python(tmp_path, run_cli, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "legacy"\nrequires-python = ">=3.7"\n' + ) + + code, stdout, _stderr, _ = run_cli("init", "--dry-run", "--json") + + assert code == 0, stdout + assert json.loads(stdout)["status"] == "planned" diff --git a/tests/test_config.py b/tests/test_config.py index 9de52194..24775545 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -822,28 +822,31 @@ def test_user_config_validates_python_version(): with pytest.raises(ConfigurationError) as exc_info: UserConfig(python_version="invalid") assert "Invalid Python version: 'invalid'." in str(exc_info.value) - assert exc_info.value.hint == "Python version must be a float value (e.g., '3.13')." + assert ( + exc_info.value.hint + == "Write the Python version as major.minor, such as '3.13'." + ) with pytest.raises(ConfigurationError) as exc_info: UserConfig(python_version="2.7") - assert "Invalid Python version: '2.7'." in str(exc_info.value) + assert "Unsupported Python version: '2.7'." in str(exc_info.value) assert ( exc_info.value.hint - == "Python version is outside the accepted range (3.8 - 3.14)." + == "Protostar scaffolds Python 3 projects; choose a 3.x version such as '3.13'." ) + for valid in ("3.7", "3.15", "3.13.1"): + assert UserConfig(python_version=valid).python_version == valid + def test_user_config_validates_github_username(): with pytest.raises(ConfigurationError) as exc_info: UserConfig(github_username="@octocat") assert "Invalid GitHub username: '@octocat'." in str(exc_info.value) - assert exc_info.value.hint == "Remove the leading '@' from GitHub username." + assert exc_info.value.hint == "Drop the leading '@': use 'octocat'." with pytest.raises(ConfigurationError) as exc_info: UserConfig(github_username="-octocat") assert "Invalid GitHub username: '-octocat'." in str(exc_info.value) assert exc_info.value.hint is not None - assert ( - "GitHub username may only contain alphanumeric characters" - in exc_info.value.hint - ) + assert "starting with a letter or digit" in exc_info.value.hint diff --git a/tests/test_recipe.py b/tests/test_recipe.py index 4505aba4..71cdbdff 100644 --- a/tests/test_recipe.py +++ b/tests/test_recipe.py @@ -88,49 +88,76 @@ def test_strict_recipe_validation(mutation): decode_recipe(recipe().to_dict() | mutation) +PYTHON_FORMAT_HINT = "Write the Python version as major.minor, such as '3.13'." +PYTHON_MAJOR_HINT = ( + "Protostar scaffolds Python 3 projects; choose a 3.x version such as '3.13'." +) +USERNAME_HINT = ( + "A GitHub username is at most 39 letters, digits, and hyphens, " + "starting with a letter or digit." +) + + @pytest.mark.parametrize( - ("invalid", "expected_hint"), + ("invalid", "message", "expected_hint"), [ - ("foo", "Python version must be a float value (e.g., '3.13')."), - ("3.x", "Python version must be a float value (e.g., '3.13')."), - ("", "Python version must be a float value (e.g., '3.13')."), - ("3", "Python version must be a float value (e.g., '3.13')."), - ("2.7", "Python version is outside the accepted range (3.8 - 3.14)."), - ("4.0", "Python version is outside the accepted range (3.8 - 3.14)."), - ("3.5", "Python version is outside the accepted range (3.8 - 3.14)."), - ("3.15", "Python version is outside the accepted range (3.8 - 3.14)."), + ("foo", "Invalid", PYTHON_FORMAT_HINT), + ("3.x", "Invalid", PYTHON_FORMAT_HINT), + ("", "Invalid", PYTHON_FORMAT_HINT), + ("3", "Invalid", PYTHON_FORMAT_HINT), + (" 3.13", "Invalid", PYTHON_FORMAT_HINT), + (3.13, "Invalid", PYTHON_FORMAT_HINT), + ("2.7", "Unsupported", PYTHON_MAJOR_HINT), + ("4.0", "Unsupported", PYTHON_MAJOR_HINT), + ("03.13", "Unsupported", PYTHON_MAJOR_HINT), ], ) -def test_invalid_python_version_recipe_error(invalid, expected_hint): +def test_invalid_python_version_recipe_error(invalid, message, expected_hint): with pytest.raises(ConfigurationError) as exc_info: decode_recipe(recipe().to_dict() | {"python": invalid}) - assert f"Invalid Python version: {invalid!r}." in str(exc_info.value) + assert f"{message} Python version: {invalid!r}." in str(exc_info.value) assert exc_info.value.hint == expected_hint +@pytest.mark.parametrize("valid", ["3.0", "3.7", "3.10", "3.15", "3.30", "3.13.1"]) +def test_any_python_3_version_decodes(valid): + current = dict(recipe().context) | {"PYTHON_VERSION": valid} + decoded = decode_recipe( + recipe().to_dict() + | {"python": valid, "context": current, "metadata": {"minimum_python": valid}} + ) + assert decoded.python == valid + + @pytest.mark.parametrize( - ("invalid", "expected_hint"), + ("invalid", "message", "expected_hint"), [ - ("foo", "minimum Python version must be a float value (e.g., '3.13')."), - ("2.7", "minimum Python version is outside the accepted range (3.8 - 3.14)."), + ( + "foo", + "Invalid", + "Write the minimum Python version as major.minor, such as '3.13'.", + ), + ("2.7", "Unsupported", PYTHON_MAJOR_HINT), ], ) -def test_invalid_metadata_minimum_python_error(invalid, expected_hint): +def test_invalid_metadata_minimum_python_error(invalid, message, expected_hint): with pytest.raises(ConfigurationError) as exc_info: decode_recipe(recipe().to_dict() | {"metadata": {"minimum_python": invalid}}) - assert f"Invalid minimum Python version: {invalid!r}." in str(exc_info.value) + assert f"{message} minimum Python version: {invalid!r}." in str(exc_info.value) assert exc_info.value.hint == expected_hint @pytest.mark.parametrize( ("invalid", "expected_hint"), [ - ("abc", "Container port must be an integer (e.g., '8000')."), - ("", "Container port must be an integer (e.g., '8000')."), - (0, "Container port is outside the accepted range (1 - 65535)."), - (65536, "Container port is outside the accepted range (1 - 65535)."), - ("70000", "Container port is outside the accepted range (1 - 65535)."), - (-1, "Container port is outside the accepted range (1 - 65535)."), + ("abc", "Container port must be a whole number, such as '8000'."), + ("+8000", "Container port must be a whole number, such as '8000'."), + (" 80 ", "Container port must be a whole number, such as '8000'."), + (0, "Container port must be between 1 and 65535."), + ("0", "Container port must be between 1 and 65535."), + (65536, "Container port must be between 1 and 65535."), + ("70000", "Container port must be between 1 and 65535."), + (-1, "Container port must be between 1 and 65535."), ], ) def test_invalid_metadata_docker_port_error(invalid, expected_hint): @@ -146,26 +173,21 @@ def test_valid_metadata_docker_port(valid): assert dict(decoded.metadata)["docker_port"] == valid +@pytest.mark.parametrize("key", ["docker_port", "minimum_python", "github_username"]) +def test_empty_metadata_is_unset_not_invalid(key): + decoded = decode_recipe(recipe().to_dict() | {"metadata": {key: ""}}) + assert dict(decoded.metadata)[key] == "" + + @pytest.mark.parametrize( ("invalid", "expected_hint"), [ - ("@octocat", "Remove the leading '@' from GitHub username."), - ( - "-user", - "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", - ), - ( - "user-", - "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", - ), - ( - "user_name", - "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", - ), - ( - "a" * 40, - "GitHub username may only contain alphanumeric characters and single hyphens, and cannot begin or end with a hyphen (maximum 39 characters).", - ), + ("@octocat", "Drop the leading '@': use 'octocat'."), + ("-user", USERNAME_HINT), + ("_user", USERNAME_HINT), + ("user.name", USERNAME_HINT), + ("user name", USERNAME_HINT), + ("a" * 40, USERNAME_HINT), ], ) def test_invalid_metadata_github_username_error(invalid, expected_hint): @@ -175,7 +197,12 @@ def test_invalid_metadata_github_username_error(invalid, expected_hint): assert exc_info.value.hint == expected_hint -@pytest.mark.parametrize("valid", ["", "octocat", "user-name-123", "a"]) +# Legacy accounts may end in or repeat a hyphen; Enterprise Managed Users +# carry an underscore suffix. +@pytest.mark.parametrize( + "valid", + ["octocat", "user-name-123", "a", "user-", "a--b", "octocat_acme", "a" * 39], +) def test_valid_metadata_github_username(valid): decoded = decode_recipe( recipe().to_dict() | {"metadata": {"github_username": valid}} @@ -196,9 +223,7 @@ def test_invalid_context_package_name_error(invalid): ) -@pytest.mark.parametrize( - "invalid", ["foo/bar", "foo\\bar", "foo\0bar", "", " ", ".", ".."] -) +@pytest.mark.parametrize("invalid", ["foo/bar", "foo\\bar", "foo\0bar", "", ".", ".."]) def test_invalid_context_project_name_error(invalid): current = dict(recipe().context) current["PROJECT_NAME"] = invalid diff --git a/tests/test_tui.py b/tests/test_tui.py index b4c4e493..b6538ad8 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -820,21 +820,32 @@ async def test_invalid_minimum_python_shows_actionable_preview_error(): await settle(pilot) summary = plain(app, "#preview-summary") assert "Invalid Python version: 'invalid'." in summary - assert "Python version must be a float value (e.g., '3.13')." in summary + assert "Write the Python version as major.minor, such as '3.13'." in summary field.focus() field.value = "2.7" await pilot.press("enter") await settle(pilot) summary = plain(app, "#preview-summary") - assert "Invalid Python version: '2.7'." in summary - assert "Python version is outside the accepted range (3.8 - 3.14)." in summary - field.focus() - field.value = "" - await pilot.press("enter") + assert "Unsupported Python version: '2.7'." in summary + assert "Protostar scaffolds Python 3 projects" in summary + + +@pytest.mark.asyncio +async def test_any_python_3_minimum_or_none_is_accepted(): + app = make_app() + async with app.run_test(size=(110, 45)) as pilot: await settle(pilot) - summary = plain(app, "#preview-summary") - assert "Invalid Python version: ''." in summary - assert "Python version must be a float value (e.g., '3.13')." in summary + continue_btn = app.screen.query_one("#continue", Button) + field = app.screen.query_one("#meta-minimum_python", Input) + # Clearing the field falls back to the default, like an old or new + # Python, is never an error. + for value in ("3.7", "3.15", ""): + field.focus() + field.value = value + await pilot.press("enter") + await settle(pilot) + assert "Invalid" not in plain(app, "#preview-summary") + assert not continue_btn.disabled @pytest.mark.asyncio @@ -849,7 +860,7 @@ async def test_invalid_github_username_shows_actionable_preview_error(): await settle(pilot) summary = plain(app, "#preview-summary") assert "Invalid GitHub username: '@octocat'." in summary - assert "Remove the leading '@' from GitHub username." in summary + assert "Drop the leading '@': use 'octocat'." in summary @pytest.mark.asyncio @@ -866,7 +877,7 @@ async def test_invalid_docker_port_shows_actionable_preview_error(): await settle(pilot) summary = plain(app, "#preview-summary") assert "Invalid container port: 'notaport'." in summary - assert "Container port must be an integer (e.g., '8000')." in summary + assert "Container port must be a whole number, such as '8000'." in summary @pytest.mark.asyncio From cabc6747843f641f9490c77f0c7d6be1916b7e81 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 18:00:58 -0700 Subject: [PATCH 4/4] test(tui): wait for tool clicks to be handled before asserting The constraints test asserted right after pilot.click, before the Checkbox.Changed handler had run, which failed on a slow Windows runner. --- tests/test_tui.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_tui.py b/tests/test_tui.py index b6538ad8..2f688b74 100644 --- a/tests/test_tui.py +++ b/tests/test_tui.py @@ -148,18 +148,23 @@ async def test_tools_constraints_and_provenance(): rtd.scroll_visible(immediate=True) await pilot.pause() await pilot.click("#tool-zensical") + await pilot.pause() assert not rtd.disabled await pilot.click("#tool-readthedocs") + await pilot.pause() await pilot.click("#tool-zensical") + await pilot.pause() assert rtd.disabled assert not rtd.value app.screen.query_one("#tool-ruff").scroll_visible(immediate=True) await pilot.pause() await pilot.click("#tool-ruff") + await pilot.pause() assert "your choice" in app.screen.query_one("#tool-ruff", Checkbox).label.plain app.screen.query_one("#tool-prek", RadioButton).scroll_visible(immediate=True) await pilot.pause() await pilot.click("#tool-prek") + await pilot.pause() assert not app.screen.query_one("#tool-pre_commit", RadioButton).value await apply(pilot) choices = dict(app.return_value.draft.tool_choices)