diff --git a/docs/module-authoring.md b/docs/module-authoring.md index f28fc892..36a9ff81 100644 --- a/docs/module-authoring.md +++ b/docs/module-authoring.md @@ -23,6 +23,17 @@ my-module/ └── tests/ ``` +### Standalone vs in-repo: `.github/` workflows + +`smpy create-module` ships a `.github/` with `ci.yml` + `publish.yml` (PyPI +trusted-publishing on a `v*` tag) — useful when the module lives in its **own +repo**. When you scaffold a module **inside an existing repo/host** (the +documented `modules/*` monorepo layout), the CLI omits `.github/` by default: +GitHub only runs workflows from the repository-root `.github/workflows/`, so a +nested per-module one never runs, and `publish.yml` would be a publish footgun. +Pass `--standalone` to force the workflows for a module destined for its own +repo. See GH #210. + ### Service types: concrete class, not Protocol Export the concrete service class from `.service` and have consumers diff --git a/framework/cli/simple_module_cli/app_project.py b/framework/cli/simple_module_cli/app_project.py index 6c5b85c4..41d847bf 100644 --- a/framework/cli/simple_module_cli/app_project.py +++ b/framework/cli/simple_module_cli/app_project.py @@ -15,7 +15,6 @@ import json as _json import secrets as _secrets -import shutil as _shutil from collections.abc import Sequence from pathlib import Path from typing import Any @@ -193,10 +192,15 @@ def _scaffold_sample_module(target: Path) -> None: # Pin the sample's framework deps to the exact framework version so the # workspace resolves (the template's >=1.0,<2.0 ranges don't exist on PyPI # pre-1.0). See GH #195. - create_module(sample_dest, name=_SAMPLE_MODULE_NAME, framework_version=_FRAMEWORK_VERSION) - # GitHub only reads workflows from the repo root, so the template's - # .github/ is dead inside a workspace. - _shutil.rmtree(sample_dest / ".github") + # The sample lives inside the workspace, so it gets no per-module .github/: + # GitHub only reads workflows from the repo root, where the template's + # .github/ would be dead anyway. See GH #210. + create_module( + sample_dest, + name=_SAMPLE_MODULE_NAME, + framework_version=_FRAMEWORK_VERSION, + include_ci=False, + ) _seed_static_dist_placeholder(sample_dest / _SAMPLE_MODULE_NAME / "static" / "dist") diff --git a/framework/cli/simple_module_cli/cli.py b/framework/cli/simple_module_cli/cli.py index cec0998c..68026ef2 100644 --- a/framework/cli/simple_module_cli/cli.py +++ b/framework/cli/simple_module_cli/cli.py @@ -23,7 +23,7 @@ from simple_module_cli.plugins import discover_and_mount from simple_module_cli.scaffolding import create_host as _create_host from simple_module_cli.scaffolding import create_module as _create_module -from simple_module_cli.scaffolding import resolve_framework_version +from simple_module_cli.scaffolding import is_inside_existing_repo, resolve_framework_version from simple_module_cli.skills_cmd import app as skills_app app = typer.Typer( @@ -90,21 +90,44 @@ def create_module( Path | None, typer.Option("--dest", help="Destination dir. Defaults to ./simple_module_."), ] = None, + standalone: Annotated[ + bool, + typer.Option( + "--standalone", + help="Emit the module's own .github/ CI + PyPI publish workflows. " + "By default they are omitted when the module lands inside an " + "existing repo/host (nested workflows never run there).", + ), + ] = False, ) -> None: """Scaffold a publishable SimpleModule module package.""" slug = to_kebab_case(name) package = slug.replace("-", "_") target = dest or Path.cwd() / f"simple_module_{package}" + # An in-repo module (the documented modules/* layout) gets no .github/: those + # nested workflows never run and publish.yml is a PyPI footgun. --standalone + # forces them for a module that lives in its own repo. See GH #210. + include_ci = standalone or not is_inside_existing_repo(target) try: # Pin framework deps to the installed framework version so the module # resolves against the app that created it (the template's >=1.0,<2.0 # ranges don't exist on PyPI pre-1.0). See GH #195. - _create_module(target, name=name, framework_version=resolve_framework_version()) + _create_module( + target, + name=name, + framework_version=resolve_framework_version(), + include_ci=include_ci, + ) except FileExistsError as exc: typer.echo(f"ERROR: {exc}", err=True) raise typer.Exit(code=1) from exc typer.echo(f"Created module 'simple_module_{package}' at {target}") + if not include_ci: + typer.echo( + "Skipped .github/ workflows: this module is inside an existing repo, " + "where nested workflows never run. Use --standalone to emit them." + ) typer.echo("\nNext steps:") typer.echo(f" cd {target}") typer.echo(" uv sync --extra dev") diff --git a/framework/cli/simple_module_cli/scaffolding.py b/framework/cli/simple_module_cli/scaffolding.py index fad87030..3230a559 100644 --- a/framework/cli/simple_module_cli/scaffolding.py +++ b/framework/cli/simple_module_cli/scaffolding.py @@ -37,6 +37,7 @@ "create_host", "create_module", "create_workspace", + "is_inside_existing_repo", "pin_framework_deps", "resolve_framework_version", ] @@ -61,6 +62,31 @@ def _module_to_pypi_name(name: str) -> str: return f"simple_module_{name.lower()}" +def is_inside_existing_repo(dest: Path) -> bool: + """Return True when ``dest`` lands inside an existing repo / host project. + + A module scaffolded under an existing host application (the documented + monorepo ``modules/*`` layout) is an *in-repo* module: GitHub only runs + workflows from the repository-root ``.github/workflows/``, so a per-module + ``.github/`` is dead weight there — and the bundled ``publish.yml`` (which + publishes ``simple_module_`` to PyPI on any ``v*`` tag) is a footgun if + it ever surfaces at the repo root. We detect this by walking up from + ``dest``'s parent for a ``.git`` directory or a ``pyproject.toml`` (an + existing repo / host / workspace member). + + ``dest`` itself is *excluded* from the walk — the module's own scaffolded + ``pyproject.toml`` must not count as "an existing host". A truly standalone + target (no repo/pyproject above it) returns False. See GH #210. + """ + # ``resolve()`` allows ``dest`` to not exist yet; the walk is over its + # absolute parents so a relative ``--dest`` is handled the same way. + start = Path(dest).resolve().parent + for parent in (start, *start.parents): + if (parent / ".git").exists() or (parent / "pyproject.toml").is_file(): + return True + return False + + def _should_pin_framework_version(version: str | None) -> bool: """Whether ``version`` is a concrete pin rather than a skip sentinel. @@ -222,6 +248,7 @@ def create_module( template_root: Path | None = None, *, framework_version: str | None = None, + include_ci: bool = True, ) -> Path: """Scaffold a module package at ``dest``. @@ -230,6 +257,12 @@ def create_module( the module resolves against that framework version (e.g. ``uv add`` into the workspace that created it). Left as ``None`` (or the ``"*"`` sentinel), the template's ranges are kept verbatim. See GH #195. + + When ``include_ci`` is False, the scaffolded ``.github/`` (CI + PyPI publish + workflows) is omitted. Those nested workflows never run inside an existing + host repo (GitHub only reads the repo-root ``.github/``) and ``publish.yml`` + is a footgun there, so callers creating an *in-repo* module pass + ``include_ci=False``. See GH #210. """ dest = Path(dest) existed_before = dest.exists() @@ -249,6 +282,8 @@ def create_module( }, path_rewrites={_PACKAGE_PATH_TOKEN: package_name}, ) + if not include_ci: + shutil.rmtree(dest / ".github", ignore_errors=True) if _should_pin_framework_version(framework_version): pin_framework_deps(dest / "pyproject.toml", framework_version) except Exception: diff --git a/framework/cli/tests/test_create_module_ci_skip.py b/framework/cli/tests/test_create_module_ci_skip.py new file mode 100644 index 00000000..52c1a556 --- /dev/null +++ b/framework/cli/tests/test_create_module_ci_skip.py @@ -0,0 +1,107 @@ +"""GH #210: create-module omits the per-module .github/ for in-repo modules. + +Nested ``.github/workflows`` never run (GitHub only reads the repo-root +``.github/``) and the bundled ``publish.yml`` is a PyPI-publish footgun, so a +module scaffolded inside an existing repo/host gets no ``.github/`` by default. +``--standalone`` forces it for a module that lives in its own repo. +""" + +from __future__ import annotations + + +class TestCreateModuleIncludeCi: + async def test_include_ci_false_omits_github(self, tmp_path): + from simple_module_cli.scaffolding import create_module + + dest = tmp_path / "simple-module-orders" + create_module(dest, name="Orders", include_ci=False) + + assert not (dest / ".github").exists() + # The rest of the package is still scaffolded. + assert (dest / "orders" / "module.py").is_file() + + async def test_include_ci_true_default_ships_github(self, tmp_path): + from simple_module_cli.scaffolding import create_module + + dest = tmp_path / "simple-module-orders" + create_module(dest, name="Orders") # include_ci defaults to True + + assert (dest / ".github" / "workflows" / "ci.yml").is_file() + assert (dest / ".github" / "workflows" / "publish.yml").is_file() + + +class TestCreateModuleCliContext: + async def test_cli_omits_github_inside_repo(self, tmp_path): + """A dest inside an existing repo (parent ``.git``) omits ``.github/`` + notes it.""" + from simple_module_cli.cli import app + from typer.testing import CliRunner + + (tmp_path / ".git").mkdir() # simulate an existing host repo + dest = tmp_path / "modules" / "orders" + + result = CliRunner().invoke(app, ["create-module", "Orders", "--dest", str(dest)]) + assert result.exit_code == 0, result.output + assert not (dest / ".github").exists(), ".github/ must be omitted for in-repo modules" + assert (dest / "orders" / "module.py").is_file() + # The skip is discoverable from the command output. + assert ".github" in result.output and "--standalone" in result.output + + async def test_cli_standalone_forces_github_inside_repo(self, tmp_path): + """``--standalone`` emits ``.github/`` even inside an existing repo.""" + from simple_module_cli.cli import app + from typer.testing import CliRunner + + (tmp_path / ".git").mkdir() + dest = tmp_path / "modules" / "orders" + + result = CliRunner().invoke( + app, ["create-module", "Orders", "--dest", str(dest), "--standalone"] + ) + assert result.exit_code == 0, result.output + assert (dest / ".github" / "workflows" / "ci.yml").is_file() + assert (dest / ".github" / "workflows" / "publish.yml").is_file() + + async def test_cli_emits_github_for_standalone_target(self, tmp_path): + """A clean target (no repo/pyproject ancestor) keeps ``.github/`` by default.""" + import pytest + from simple_module_cli.cli import app + from simple_module_cli.scaffolding import is_inside_existing_repo + from typer.testing import CliRunner + + dest = tmp_path / "simple-module-orders" + # This test's premise is that ``dest`` has no repo/pyproject ancestor. + # Under ``pytest --basetemp=`` that wouldn't hold, so the + # emit-by-default behaviour can't be exercised — skip rather than fail + # misleadingly. + if is_inside_existing_repo(dest): + pytest.skip( + "tmp_path resolves inside an existing repo (e.g. --basetemp under the repo)" + ) + + result = CliRunner().invoke(app, ["create-module", "Orders", "--dest", str(dest)]) + assert result.exit_code == 0, result.output + assert (dest / ".github" / "workflows" / "ci.yml").is_file() + assert "Skipped .github" not in result.output + + +class TestIsInsideExistingRepo: + async def test_detects_git_parent(self, tmp_path): + from simple_module_cli.scaffolding import is_inside_existing_repo + + (tmp_path / ".git").mkdir() + assert is_inside_existing_repo(tmp_path / "modules" / "orders") + + async def test_detects_pyproject_parent(self, tmp_path): + from simple_module_cli.scaffolding import is_inside_existing_repo + + (tmp_path / "pyproject.toml").write_text("[project]\n", encoding="utf-8") + assert is_inside_existing_repo(tmp_path / "modules" / "orders") + + async def test_clean_target_is_not_in_repo(self, tmp_path): + from simple_module_cli.scaffolding import is_inside_existing_repo + + # The module's own scaffolded pyproject.toml at dest must not count. + dest = tmp_path / "standalone" + dest.mkdir() + (dest / "pyproject.toml").write_text("[project]\n", encoding="utf-8") + assert not is_inside_existing_repo(dest)