From ccea332bce0646071ba8be4b0a5acd799c85cc1d Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 16 Jun 2026 00:56:12 +0200 Subject: [PATCH 1/2] fix(testing): route-introspection tests for FastAPI 0.137 lazy include_router MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FastAPI 0.137 / Starlette 1.3 made `include_router()` lazy: included routes no longer appear in `app.routes` (they sit behind `_IncludedRouter` wrappers and resolve only at request time). Tests that introspected `app.routes` by `.path` stopped seeing module/health routes — so `test_expected_routes_registered` and `test_registers_module_routes_with_prefix` failed even though the routes still work at runtime. (Surfaced via dependency drift since the last green run.) - Add `simple_module_test.effective_route_paths(app)`: reads the OpenAPI schema (fully-resolved prefixes) unioned with top-level mounts — version-agnostic, and the supported way to assert on registered routes. - Use it in the affected hosting/testing route tests; the absence assertions (`test_isolates_*`, `test_modules_enabled_*`) become meaningful again instead of passing vacuously under lazy inclusion. The dashboard's bare-prefix Inertia alias is `include_in_schema=False`; its behaviour stays covered by `TestProtectedPages`. --- framework/hosting/tests/test_app.py | 15 +++++--- .../testing/simple_module_test/__init__.py | 2 ++ .../testing/simple_module_test/routes.py | 34 +++++++++++++++++++ framework/testing/tests/test_app_factory.py | 13 ++++--- 4 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 framework/testing/simple_module_test/routes.py diff --git a/framework/hosting/tests/test_app.py b/framework/hosting/tests/test_app.py index e938a2d6..6066daff 100644 --- a/framework/hosting/tests/test_app.py +++ b/framework/hosting/tests/test_app.py @@ -27,12 +27,14 @@ async def test_app_state_has_registries(self, app: FastAPI): async def test_modules_enabled_limits_loaded_modules(self, settings: Settings): """Host respects settings.modules_enabled — only listed modules contribute routes.""" + from simple_module_test import effective_route_paths + # Only Auth should be loaded; Dashboard routes must be absent. restricted = settings.model_copy(update={"modules_enabled": ["Auth"]}) app = create_app(restricted) - paths: set[str] = {str(r.path) for r in app.routes if hasattr(r, "path")} + paths = effective_route_paths(app) # Auth is now contracts-only, so it has no routes — only health remains. - assert "/dashboard" not in paths + assert not any(p.startswith("/dashboard") for p in paths) async def test_module_static_mounts_become_app_routes( self, @@ -203,7 +205,9 @@ async def test_empty_env_var_uses_fallback(self, monkeypatch, tmp_path): class TestRouteRegistration: async def test_expected_routes_registered(self, app: FastAPI): """All modules should have their routes registered in the app.""" - route_paths = [r.path for r in app.routes if hasattr(r, "path")] + from simple_module_test import effective_route_paths + + route_paths = effective_route_paths(app) assert "/health" in route_paths assert "/health/live" in route_paths @@ -216,8 +220,9 @@ async def test_expected_routes_registered(self, app: FastAPI): # landing page at "/" is owned by the host and added in host/main.py, # which the create_app fixture doesn't run. assert "/dashboard/" in route_paths - # Bare-prefix alias — see wire_module_routes for the X-Inertia rationale. - assert "/dashboard" in route_paths + # The bare-prefix Inertia alias ("/dashboard" without the slash) is + # registered with include_in_schema=False, so it isn't enumerable here; + # TestProtectedPages::test_dashboard_redirects_unauthenticated covers it. class TestProtectedPages: diff --git a/framework/testing/simple_module_test/__init__.py b/framework/testing/simple_module_test/__init__.py index a658960c..a23fe2c6 100644 --- a/framework/testing/simple_module_test/__init__.py +++ b/framework/testing/simple_module_test/__init__.py @@ -22,11 +22,13 @@ from simple_module_test.app_factory import build_test_app from simple_module_test.fake_events import FakeEventBus, RecordedEvent +from simple_module_test.routes import effective_route_paths from simple_module_test.session_cookie import forge_session_cookie __all__ = [ "FakeEventBus", "RecordedEvent", "build_test_app", + "effective_route_paths", "forge_session_cookie", ] diff --git a/framework/testing/simple_module_test/routes.py b/framework/testing/simple_module_test/routes.py new file mode 100644 index 00000000..b4e7b5d1 --- /dev/null +++ b/framework/testing/simple_module_test/routes.py @@ -0,0 +1,34 @@ +"""Enumerate an app's effective route paths, robust to lazy router inclusion. + +FastAPI 0.137 / Starlette 1.3 made ``include_router()`` lazy: an included router +now appears in ``app.routes`` as a ``_IncludedRouter`` wrapper that carries no +``.path`` and resolves its routes only at request time. Code that introspects +``app.routes`` by ``.path`` therefore no longer sees routes contributed via +``include_router`` — including every module's API/view routes and the health +router. (Routes still resolve correctly at request time; only static +introspection broke.) + +:func:`effective_route_paths` reads the OpenAPI schema instead — a stable public +API that lists every schema-included route with its fully-resolved prefix — and +unions it with any top-level routes/mounts that still carry a ``.path`` directly +(e.g. ``StaticFiles`` mounts). This works across FastAPI versions and is the +supported way to assert on registered routes. +""" + +from __future__ import annotations + +from fastapi import FastAPI + + +def effective_route_paths(app: FastAPI) -> set[str]: + """Return the set of route paths registered on ``app``. + + Includes schema routes contributed via ``include_router`` (resolved through + the OpenAPI schema, so FastAPI's lazy ``_IncludedRouter`` wrappers don't hide + them) plus any top-level mounts. Routes registered with + ``include_in_schema=False`` — e.g. the bare-prefix Inertia aliases — are not + listed here; assert those with a request instead. + """ + paths = set(app.openapi().get("paths", {}).keys()) + paths |= {r.path for r in app.routes if getattr(r, "path", None) is not None} + return paths diff --git a/framework/testing/tests/test_app_factory.py b/framework/testing/tests/test_app_factory.py index 53e16f3e..c93ca625 100644 --- a/framework/testing/tests/test_app_factory.py +++ b/framework/testing/tests/test_app_factory.py @@ -46,11 +46,10 @@ async def test_returns_fastapi_instance_from_instance(self): async def test_registers_module_routes_with_prefix(self): """The module's register_routes() runs and its api routes appear under the prefix.""" - from simple_module_test import build_test_app + from simple_module_test import build_test_app, effective_route_paths app = build_test_app(_EchoModule) - paths = {getattr(r, "path", None) for r in app.routes} - assert "/api/echo/ping" in paths + assert "/api/echo/ping" in effective_route_paths(app) async def test_module_accessible_on_app_state(self): """The instance is stored on app.state.module so tests can poke at it.""" @@ -61,12 +60,12 @@ async def test_module_accessible_on_app_state(self): async def test_isolates_from_other_installed_modules(self): """build_test_app only mounts the given module — Products/Auth routes are absent.""" - from simple_module_test import build_test_app + from simple_module_test import build_test_app, effective_route_paths app = build_test_app(_EchoModule) - paths = {getattr(r, "path", None) for r in app.routes} - assert not any(p and p.startswith("/api/products") for p in paths) - assert not any(p and p.startswith("/auth") for p in paths) + paths = effective_route_paths(app) + assert not any(p.startswith("/api/products") for p in paths) + assert not any(p.startswith("/auth") for p in paths) # ── pytest plugin fixtures ────────────────────────────────────────── From 04e5d23a14dd7901cb89767a7b695e20270cb857 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Tue, 16 Jun 2026 00:32:05 +0200 Subject: [PATCH 2/2] fix(cli): omit per-module .github/ for in-repo create-module (#210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `smpy create-module` scaffolds a `.github/` (ci.yml + publish.yml) that only makes sense for a module living in its own repo. Inside an existing host repo (the documented `modules/*` layout) those nested workflows never run — GitHub only reads the repo-root `.github/workflows/` — and `publish.yml` (PyPI publish on any `v*` tag) is a footgun. - `create_module()` gains `include_ci` (default True); when False, the scaffolded `.github/` is removed. - The CLI detects an in-repo dest (ancestor `.git`/`pyproject.toml`) and omits `.github/` by default with a discoverable note; `--standalone` forces it. - New `is_inside_existing_repo()` helper in `scaffolding.py`. - `app_project._scaffold_sample_module` now passes `include_ci=False` instead of a manual `rmtree`. Closes #210 --- docs/module-authoring.md | 11 ++ .../cli/simple_module_cli/app_project.py | 14 ++- framework/cli/simple_module_cli/cli.py | 27 ++++- .../cli/simple_module_cli/scaffolding.py | 35 ++++++ .../cli/tests/test_create_module_ci_skip.py | 107 ++++++++++++++++++ 5 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 framework/cli/tests/test_create_module_ci_skip.py 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)