From 59853de36881accc479f83e25455af7af81606a8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 13:16:09 +0200 Subject: [PATCH 1/8] ci: hardcoded-strings and lint-parity checks in CI, job timeouts, Node for python tests, make migration LABEL= Closes #598 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .github/workflows/postgres-tests.yml | 8 +- .github/workflows/pr.yml | 27 +++++- .github/workflows/release.yml | 6 ++ CLAUDE.md | 8 +- Makefile | 16 +++- framework/cli/tests/conftest.py | 15 ++++ .../cli/tests/test_cli_asset_prerequisites.py | 19 ++++ .../cli/tests/test_cli_openapi_gen_types.py | 4 +- framework/cli/tests/test_module_css_build.py | 19 ++-- scripts/check_ci_parity.py | 76 ++++++++++++++++ scripts/tests/test_scripts_ci_parity.py | 87 +++++++++++++++++++ .../tests/test_scripts_migration_target.py | 41 +++++++++ 12 files changed, 304 insertions(+), 22 deletions(-) create mode 100644 framework/cli/tests/test_cli_asset_prerequisites.py create mode 100644 scripts/check_ci_parity.py create mode 100644 scripts/tests/test_scripts_ci_parity.py create mode 100644 scripts/tests/test_scripts_migration_target.py diff --git a/.github/workflows/postgres-tests.yml b/.github/workflows/postgres-tests.yml index 9c36d23c5..c136ecee9 100644 --- a/.github/workflows/postgres-tests.yml +++ b/.github/workflows/postgres-tests.yml @@ -27,6 +27,7 @@ permissions: contents: read env: + NODE_VERSION: "24" UV_SYNC_FLAGS: --locked UV_CACHE_GLOB: | uv.lock @@ -39,7 +40,7 @@ jobs: python-tests-pg: name: Python tests (Postgres) runs-on: ubuntu-latest - timeout-minutes: 60 + timeout-minutes: 30 services: postgres: image: postgres:16-alpine @@ -72,4 +73,9 @@ jobs: enable-cache: true cache-dependency-glob: ${{ env.UV_CACHE_GLOB }} - run: make install-py + - uses: actions/setup-node@v7 + with: + node-version: ${{ env.NODE_VERSION }} + cache: "npm" + - run: npm ci - run: make test-py-pg diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index f7f9ddbb0..802538de6 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -39,6 +39,7 @@ jobs: python-lint: name: Python lint & format runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v9.0.0 @@ -50,6 +51,8 @@ jobs: # Package metadata, READMEs, release matrix, and the hand-maintained ty # extra-paths / pytest testpaths lists (#520) must match what is on disk. - run: make ci-check-metadata + - run: make ci-check-hardcoded-strings + - run: make ci-check-ci-parity - run: make ci-check-locales # Backticked repo paths in docs/, CLAUDE.md, README.md and skills must exist (#519). - run: make ci-doc-paths @@ -67,6 +70,7 @@ jobs: docs-build: name: Docs site build runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 @@ -79,6 +83,7 @@ jobs: python-typecheck: name: Python typecheck runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v9.0.0 @@ -91,6 +96,7 @@ jobs: python-tests: name: Python tests runs-on: ubuntu-latest + timeout-minutes: 30 # The invalidation suite covers what a fake Redis client cannot — a real # subscription, and two worker processes talking through one broker. The # runner image does not ship redis-server, so the broker comes from a service @@ -115,6 +121,11 @@ jobs: enable-cache: true cache-dependency-glob: ${{ env.UV_CACHE_GLOB }} - run: make install-py + - uses: actions/setup-node@v7 + with: + node-version: ${{ env.NODE_VERSION }} + cache: "npm" + - run: npm ci # Tee'd so the next step can put the totals line (count + wall time) on # the run's summary page: a suite creeping back toward the serial ~24 min # of #524 should be visible without opening the log. @@ -133,6 +144,7 @@ jobs: js-tests: name: JS tests (Vitest) runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 @@ -145,6 +157,7 @@ jobs: js-lint: name: JS lint & format runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 @@ -160,6 +173,7 @@ jobs: js-typecheck: name: JS typecheck runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 @@ -182,6 +196,7 @@ jobs: js-build: name: JS build (Vite) runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 @@ -199,9 +214,8 @@ jobs: - run: make gen-pages - run: make build # These drive a real gen-pages + vite build to prove module-shipped CSS - # and cross-module npm-name imports actually resolve. They self-skip - # without node_modules, so the `Python tests` job (make install-py only) - # silently skips them — this is the one job that can really run them. + # and cross-module npm-name imports actually resolve. Missing build + # prerequisites fail on CI rather than silently skipping the guards. - name: Module asset build guards run: make ci-css-build @@ -212,6 +226,7 @@ jobs: migrations-roundtrip-pg: name: Migrations round trip (Postgres) runs-on: ubuntu-latest + timeout-minutes: 30 services: postgres: image: postgres:16-alpine @@ -241,6 +256,7 @@ jobs: doctor: name: Module diagnostics (make doctor) runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v9.0.0 @@ -253,6 +269,7 @@ jobs: e2e-smoke: name: E2E smoke (Playwright) runs-on: ubuntu-latest + timeout-minutes: 30 services: redis: image: redis:7-alpine @@ -358,6 +375,7 @@ jobs: perf-guards: name: Perf guards (Playwright) runs-on: ubuntu-latest + timeout-minutes: 30 env: # SQLite keeps the job self-contained — these guards measure asset # delivery, which does not depend on row volumes. @@ -437,6 +455,7 @@ jobs: file-size-check: name: File size (300-line cap) runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v9.0.0 @@ -455,6 +474,7 @@ jobs: package-build: name: Package build (release dry-run) runs-on: ubuntu-latest + timeout-minutes: 20 steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v9.0.0 @@ -468,6 +488,7 @@ jobs: pr-checks: name: PR checks runs-on: ubuntu-latest + timeout-minutes: 15 needs: - python-lint - python-typecheck diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0870f6e56..a7db1093b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,6 +27,7 @@ jobs: # framework/core/pyproject.toml by the chosen level. resolve: runs-on: ubuntu-latest + timeout-minutes: 15 outputs: version: ${{ steps.compute.outputs.version }} steps: @@ -64,6 +65,7 @@ jobs: # The full PR suite (the `pr-checks` aggregate) runs against the dispatched # commit; `build` needs it, so nothing is built or published from a red commit. + # Reusable workflow calls cannot set timeout-minutes; pr.yml bounds each job. checks: uses: ./.github/workflows/pr.yml @@ -73,6 +75,7 @@ jobs: build: needs: [resolve, checks] runs-on: ubuntu-latest + timeout-minutes: 20 steps: - uses: actions/checkout@v7 with: @@ -114,6 +117,7 @@ jobs: publish-pypi: needs: build runs-on: ubuntu-latest + timeout-minutes: 30 permissions: id-token: write # mint OIDC token for PyPI Trusted Publishing strategy: @@ -170,6 +174,7 @@ jobs: publish-npm: needs: build runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: read # explicit so future cache:/checkout: steps don't 403 id-token: write # mint OIDC token for npm Trusted Publishing @@ -216,6 +221,7 @@ jobs: finalize: needs: [resolve, publish-pypi, publish-npm] runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 with: diff --git a/CLAUDE.md b/CLAUDE.md index fe9254520..e59c6bcee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,7 +33,7 @@ Everything goes through `make`. Full list: `docs/reference/make-commands.md`. | `make lint` | Ruff format + Ruff + `ty` + Biome + per-workspace `tsc` + file-size cap + hardcoded-string check + untranslated-string check + package metadata/README checks | | `make doctor` | Module diagnostics (`SM0xx`); a dev boot runs the same plus `SM012`/`SM025` | | `make migrate` / `make downgrade` / `make migration-history` | Alembic `upgrade heads` / `-1` / history | -| `make migration msg="..."` | Autogenerate; **currently fails** ("Multiple heads") — use the recipe in § Database | +| `make migration LABEL= msg="..."` | Autogenerate on the selected branch (`--head @head`); `LABEL` is required | | `make migrations-roundtrip-pg` | Postgres portability gate (upgrade → downgrade base → upgrade → `alembic check`); CI runs it | | `make gen-pages` | Regenerate `host/client_app/modules.{manifest.json,generated.ts,generated.css}` (gitignored; needed before `tsc`/Vite) | | `make gen-i18n` | Regenerate `packages/i18n/src/{keys.generated,generated-resources}.ts` (committed; CI diffs them) | @@ -101,7 +101,8 @@ Run what CI runs, in this order; a red push costs a review cycle. changes). Every feature/fix PR in recent history does this. CI (`.github/workflows/pr.yml`, aggregate job `pr-checks`): python-lint (includes -`make ci-check-metadata` and the `make ci-i18n-fresh` diff and `make ci-check-no-double-cast`), python-typecheck, +`make ci-check-metadata`, `make ci-check-hardcoded-strings`, `make ci-check-ci-parity`, +the `make ci-i18n-fresh` diff and `make ci-check-no-double-cast`), python-typecheck, python-tests (Redis service), js-tests, js-lint (`biome ci --error-on-warnings`, plus the untranslated check), js-typecheck (plus `make ci-api-types-fresh`), js-build (`make ci-css-build`), migrations-roundtrip-pg, doctor (`make ci-doctor`: migrate a throwaway SQLite DB, @@ -111,8 +112,7 @@ so a stale `uv.lock` fails (commit `uv.lock` whenever a `pyproject.toml` changes `make install` stays unlocked). `.github/workflows/postgres-tests.yml` runs `make test-py-pg` nightly, on demand and on PRs touching `framework/db`, `framework/testing`, `modules/tenants` or `host/migrations`; it is not in -`pr-checks`. **Local-only checks** (in `make lint`, not in CI, reviewers still -reject on them): hardcoded strings. `make ci-css-build` is in CI but not in +`pr-checks`. `make ci-css-build` is in CI but not in `make lint` (slow, rewrites generated files). e2e-smoke and perf-guards enable a hardcoded `SM_MODULES_ENABLED` JSON list; a new module is not exercised there until added. diff --git a/Makefile b/Makefile index 7be8dbfcd..7171b6f6d 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: install install-py install-js dev dev-api dev-ui build test test-py test-py-pg test-js test-e2e bench memray-run memray-flamegraph loadtest loadtest-seed loadtest-memray bench-nav lint doctor migrate migration downgrade migration-history migrations-roundtrip-pg docker-up docker-down kill new-module gen-pages openapi gen-i18n docker-build docker-app docker-compose-app sync-module-deps ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-file-size ci-check-test-sleeps ci-check-no-double-cast ci-check-no-raw-fetch ci-check-hardcoded-strings ci-check-untranslated ci-check-metadata ci-check-locales ci-doc-paths ci-docs-build ci-i18n-fresh ci-api-types-fresh gen-api-types ci-css-build ci-doctor ci-build-packages worker beat worker-docker +.PHONY: install install-py install-js dev dev-api dev-ui build test test-py test-py-pg test-js test-e2e bench memray-run memray-flamegraph loadtest loadtest-seed loadtest-memray bench-nav lint doctor migrate migration downgrade migration-history migrations-roundtrip-pg docker-up docker-down kill new-module gen-pages openapi gen-i18n docker-build docker-app docker-compose-app sync-module-deps ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-file-size ci-check-test-sleeps ci-check-no-double-cast ci-check-no-raw-fetch ci-check-hardcoded-strings ci-check-ci-parity ci-check-untranslated ci-check-metadata ci-check-locales ci-doc-paths ci-docs-build ci-i18n-fresh ci-api-types-fresh gen-api-types ci-css-build ci-doctor ci-build-packages worker beat worker-docker # UV_SYNC_FLAGS is empty locally; CI passes --locked so a stale uv.lock fails the build. UV_SYNC_FLAGS ?= @@ -134,7 +134,7 @@ loadtest: ## Run locust against a server already on $(LOCUST_H loadtest-memray: ## Start uvicorn under memray, load-test, emit flamegraph scripts/loadtest_memray.sh $(LOCUST_ARGS) -lint: ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-metadata ci-check-locales ci-doc-paths ci-i18n-fresh ci-api-types-fresh ci-check-file-size ci-check-test-sleeps ci-check-no-double-cast ci-check-no-raw-fetch ci-check-hardcoded-strings ci-check-untranslated +lint: ci-python-lint ci-python-typecheck ci-js-lint ci-js-typecheck ci-check-metadata ci-check-locales ci-doc-paths ci-i18n-fresh ci-api-types-fresh ci-check-file-size ci-check-test-sleeps ci-check-no-double-cast ci-check-no-raw-fetch ci-check-hardcoded-strings ci-check-ci-parity ci-check-untranslated # Kept granular so pr.yml can run them in parallel. ci-python-lint: @@ -229,6 +229,10 @@ ci-check-no-double-cast: ci-check-no-raw-fetch: uv run python scripts/check_no_raw_fetch.py +# Every lint prerequisite must also run in the PR workflow. +ci-check-ci-parity: + uv run python scripts/check_ci_parity.py + # Enforce that permissions, role names, Inertia page ids, and module dependency # names are declared as named constants rather than hardcoded string literals. ci-check-hardcoded-strings: @@ -269,8 +273,12 @@ ci-doctor: migrate: ## Run migrations to head uv run --project host alembic -c host/alembic.ini upgrade heads -migration: ## Create new migration (usage: make migration msg="add foo") - uv run --project host alembic -c host/alembic.ini revision --autogenerate -m "$(msg)" +migration: ## Create new migration (usage: make migration LABEL=users msg="add foo") + @if [ -z "$(strip $(LABEL))" ]; then \ + echo 'error: LABEL= is required (e.g. make migration LABEL=users msg="add foo")' >&2; \ + exit 1; \ + fi + uv run --project host alembic -c host/alembic.ini revision --autogenerate -m "$(msg)" --head "$(LABEL)@head" downgrade: ## Downgrade one revision uv run --project host alembic -c host/alembic.ini downgrade -1 diff --git a/framework/cli/tests/conftest.py b/framework/cli/tests/conftest.py index 71c0baee0..285532369 100644 --- a/framework/cli/tests/conftest.py +++ b/framework/cli/tests/conftest.py @@ -7,6 +7,7 @@ from __future__ import annotations +import os import subprocess import sys from pathlib import Path @@ -154,3 +155,17 @@ def factory( return repo return factory + + +@pytest.fixture +def require_cli_asset(): + """Keep optional local tooling from silently reducing CI coverage.""" + + def require(available: bool, message: str) -> None: + if available: + return + if os.environ.get("CI"): + pytest.fail(f"{message}. On CI this must fail rather than skip.") + pytest.skip(message) + + return require diff --git a/framework/cli/tests/test_cli_asset_prerequisites.py b/framework/cli/tests/test_cli_asset_prerequisites.py new file mode 100644 index 000000000..1378fe0bd --- /dev/null +++ b/framework/cli/tests/test_cli_asset_prerequisites.py @@ -0,0 +1,19 @@ +"""Missing frontend prerequisites may skip locally but must fail on CI.""" + +import pytest + + +@pytest.mark.parametrize("ci", [None, "true"]) +def test_cli_missing_asset_prerequisite_fails_on_ci(monkeypatch, require_cli_asset, ci): + if ci: + monkeypatch.setenv("CI", ci) + else: + monkeypatch.delenv("CI", raising=False) + outcome = pytest.fail.Exception if ci else pytest.skip.Exception + with pytest.raises(outcome, match="missing tool"): + require_cli_asset(False, "missing tool") + + +def test_cli_available_asset_prerequisite_runs_on_ci(monkeypatch, require_cli_asset): + monkeypatch.setenv("CI", "true") + require_cli_asset(True, "missing tool") diff --git a/framework/cli/tests/test_cli_openapi_gen_types.py b/framework/cli/tests/test_cli_openapi_gen_types.py index 2f8cffcf9..99abb06d1 100644 --- a/framework/cli/tests/test_cli_openapi_gen_types.py +++ b/framework/cli/tests/test_cli_openapi_gen_types.py @@ -185,8 +185,8 @@ def test_pinned_version_matches_the_ui_dev_dependency() -> None: assert ui["devDependencies"]["openapi-typescript"] == ot.OPENAPI_TYPESCRIPT_VERSION -@pytest.mark.skipif(not LOCAL_TOOL.is_file(), reason="needs `npm install` (openapi-typescript)") -def test_real_tool_emits_paths(importable: Path) -> None: +def test_real_tool_emits_paths(importable: Path, require_cli_asset) -> None: + require_cli_asset(LOCAL_TOOL.is_file(), "needs `npm ci` (openapi-typescript)") targets = ot.module_targets( [_fake_module(importable, "gt_real", "Widgets", opted_in=True)], importable, [] ) diff --git a/framework/cli/tests/test_module_css_build.py b/framework/cli/tests/test_module_css_build.py index df8d2a539..41f40c009 100644 --- a/framework/cli/tests/test_module_css_build.py +++ b/framework/cli/tests/test_module_css_build.py @@ -6,9 +6,8 @@ that whole chain, so it drives a genuine build rather than inspecting the generated text. -Skipped where a build is impossible: CI's `python-tests` job installs Python -deps only (`make install-py`), so `node_modules` is absent there. The JS jobs -and local runs do have it. +Missing build prerequisites skip locally and fail on CI, where the Python +and JS build jobs install the required Node dependencies. """ from __future__ import annotations @@ -22,17 +21,21 @@ REPO_ROOT = Path(__file__).resolve().parents[3] pytestmark = [ - pytest.mark.skipif( - not (REPO_ROOT / "node_modules").is_dir(), - reason="node_modules absent — run `npm install` to exercise the real build", - ), - pytest.mark.skipif(shutil.which("npm") is None, reason="npm not on PATH"), # Both tests rewrite the shared tree (generated manifests, host/static/dist, a # probe page under audit_log), so under xdist they must share one worker. pytest.mark.xdist_group("repo-build"), ] +@pytest.fixture(autouse=True) +def require_build_tools(require_cli_asset): + require_cli_asset(shutil.which("npm") is not None, "npm not on PATH") + require_cli_asset( + (REPO_ROOT / "node_modules").is_dir(), + "node_modules absent — run `npm ci` to exercise the real build", + ) + + class TestModuleCssReachesBundle: def test_module_class_is_emitted_by_vite_build(self): """A class defined only in a module's styles.css appears in the built CSS. diff --git a/scripts/check_ci_parity.py b/scripts/check_ci_parity.py new file mode 100644 index 000000000..f9ca89e56 --- /dev/null +++ b/scripts/check_ci_parity.py @@ -0,0 +1,76 @@ +"""Require every `make lint` prerequisite to run in the PR workflow. + +Intentional local-only checks belong in LOCAL_ONLY_CHECKS with a reason. +""" + +from __future__ import annotations + +import argparse +import re +import shlex +import sys +from pathlib import Path + +import yaml + +# Target -> reason. Empty because every current lint prerequisite runs in CI. +LOCAL_ONLY_CHECKS: dict[str, str] = {} +WORKFLOW = Path(".github/workflows/pr.yml") + + +def lint_prerequisites(makefile: str) -> set[str]: + """Read lint rules, including continued lines and trailing comments.""" + logical = re.sub(r"\\\n", " ", makefile) + rules = re.findall(r"^lint\s*:(.*)$", logical, re.MULTILINE) + if not rules: + raise ValueError("Makefile has no lint target") + return {word for rule in rules for word in rule.split("#", 1)[0].split()} + + +def workflow_targets(workflow: str) -> set[str]: + """Collect make targets from run steps; comments and step names don't count.""" + targets: set[str] = set() + data = yaml.safe_load(workflow) + for job in data.get("jobs", {}).values(): + for step in job.get("steps", []): + for line in step.get("run", "").replace("\\\n", " ").splitlines(): + lexer = shlex.shlex(line, posix=True, punctuation_chars=";&|()") + lexer.whitespace_split = True + tokens = list(lexer) + in_make = False + for index, token in enumerate(tokens): + if token in {";", "&&", "||", "|", "&", "(", ")"}: + in_make = False + elif token == "make" and ( + index == 0 or tokens[index - 1] in {";", "&&", "||", "|", "&", "("} + ): + in_make = True + elif in_make and not token.startswith("-") and "=" not in token: + targets.add(token) + return targets + + +def check_parity(root: Path) -> list[str]: + prerequisites = lint_prerequisites((root / "Makefile").read_text(encoding="utf-8")) + covered = workflow_targets((root / WORKFLOW).read_text(encoding="utf-8")) + return [ + f"{target}: make lint prerequisite missing from {WORKFLOW} run steps" + for target in sorted(prerequisites - covered - LOCAL_ONLY_CHECKS.keys()) + ] + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--root", type=Path, default=Path(__file__).resolve().parents[1]) + args = parser.parse_args(argv) + try: + errors = check_parity(args.root) + except (OSError, ValueError, yaml.YAMLError) as exc: + errors = [str(exc)] + for error in errors: + print(error, file=sys.stderr) + return int(bool(errors)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_scripts_ci_parity.py b/scripts/tests/test_scripts_ci_parity.py new file mode 100644 index 000000000..ad9ad2cb6 --- /dev/null +++ b/scripts/tests/test_scripts_ci_parity.py @@ -0,0 +1,87 @@ +"""Lint/CI parity catches omitted checks, including its own gate.""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] +SCRIPT = REPO / "scripts/check_ci_parity.py" + + +def run_check(tmp_path, makefile, workflow): + (tmp_path / "Makefile").write_text(makefile) + workflow_path = tmp_path / ".github/workflows/pr.yml" + workflow_path.parent.mkdir(parents=True) + workflow_path.write_text(workflow) + return subprocess.run( + [sys.executable, str(SCRIPT), "--root", str(tmp_path)], + text=True, + capture_output=True, + check=False, + ) + + +@pytest.mark.parametrize( + "run", + ["make ci-one ci-two", "|\n make ci-one\n make ci-two"], +) +def test_scripts_parity_accepts_all_prerequisites(tmp_path, run): + result = run_check( + tmp_path, + "lint: ci-one \\\n ci-two # comment\n", + f"jobs:\n lint:\n steps:\n - run: {run}\n", + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize( + "step", + [ + "# make ci-two\n - run: make ci-one", + "- name: make ci-two\n run: make ci-one", + "- run: make ci-one ci-two-extra", + '- run: echo "make ci-two"', + ], +) +def test_scripts_parity_rejects_checks_not_executed(tmp_path, step): + result = run_check( + tmp_path, "lint: ci-one ci-two\n", f"jobs:\n lint:\n steps:\n {step}\n" + ) + assert result.returncode == 1 + assert "ci-two" in result.stderr + + +def test_scripts_parity_rejects_missing_lint_target(tmp_path): + result = run_check(tmp_path, "other: ci-one\n", "jobs: {}\n") + assert result.returncode == 1 + assert "lint" in result.stderr + + +def test_scripts_repository_lint_is_covered(): + result = subprocess.run( + [sys.executable, str(SCRIPT)], capture_output=True, text=True, check=False + ) + assert result.returncode == 0, result.stderr + + +def test_scripts_parity_honors_explicit_local_only_allowlist(tmp_path, monkeypatch): + from scripts import check_ci_parity + + (tmp_path / "Makefile").write_text("lint: ci-one ci-local\n") + workflow = tmp_path / ".github/workflows/pr.yml" + workflow.parent.mkdir(parents=True) + workflow.write_text("jobs:\n lint:\n steps:\n - run: make ci-one\n") + monkeypatch.setattr(check_ci_parity, "LOCAL_ONLY_CHECKS", {"ci-local": "local tooling"}) + assert check_ci_parity.check_parity(tmp_path) == [] + + +def test_scripts_parity_catches_its_own_missing_step(tmp_path): + result = run_check( + tmp_path, + "lint: ci-check-ci-parity\n", + "jobs:\n lint:\n steps:\n - run: make ci-python-lint\n", + ) + assert result.returncode == 1 + assert "ci-check-ci-parity" in result.stderr diff --git a/scripts/tests/test_scripts_migration_target.py b/scripts/tests/test_scripts_migration_target.py new file mode 100644 index 000000000..59f206118 --- /dev/null +++ b/scripts/tests/test_scripts_migration_target.py @@ -0,0 +1,41 @@ +"""The repo migration target selects a branch before invoking Alembic.""" + +import os +import subprocess +from pathlib import Path + +import pytest + +MAKEFILE = Path(__file__).resolve().parents[2] / "Makefile" + + +@pytest.mark.parametrize("label", [None, "", " ", "users"]) +def test_scripts_migration_requires_label_and_selects_head(tmp_path, label): + # Capture the command at the uv boundary; never create a real migration. + uv = tmp_path / "uv" + uv.write_text('#!/bin/sh\nprintf "%s\\n" "$@"\n') + uv.chmod(0o755) + env = {key: value for key, value in os.environ.items() if key != "LABEL"} + env["PATH"] = f"{tmp_path}:{env['PATH']}" + args = ["make", "-f", str(MAKEFILE), "migration", "msg=add foo"] + if label is not None: + args.append(f"LABEL={label}") + result = subprocess.run(args, cwd=tmp_path, env=env, capture_output=True, text=True) + if label and label.strip(): + assert result.returncode == 0, result.stderr + assert result.stdout.splitlines()[-10:] == [ + "host", + "alembic", + "-c", + "host/alembic.ini", + "revision", + "--autogenerate", + "-m", + "add foo", + "--head", + "users@head", + ] + else: + assert result.returncode != 0 + assert "LABEL=" in result.stderr + assert "--autogenerate" not in result.stdout From 82d5259346353736ab4fdfc2cb9fcc08b6905b3f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 13:16:09 +0200 Subject: [PATCH 2/8] release: pre-flight for new PyPI projects, resumable publish legs; Postgres tests on users/background_tasks/file_storage Closes #599 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- .github/workflows/postgres-tests.yml | 5 +- .github/workflows/release.yml | 28 +++- docs/release.md | 77 +++++----- scripts/release_preflight.py | 42 ++++++ .../tests/test_scripts_release_resumption.py | 136 ++++++++++++++++++ 5 files changed, 250 insertions(+), 38 deletions(-) create mode 100644 scripts/release_preflight.py create mode 100644 scripts/tests/test_scripts_release_resumption.py diff --git a/.github/workflows/postgres-tests.yml b/.github/workflows/postgres-tests.yml index c136ecee9..b67a066eb 100644 --- a/.github/workflows/postgres-tests.yml +++ b/.github/workflows/postgres-tests.yml @@ -4,7 +4,7 @@ name: Postgres tests # `@pytest.mark.postgres` tests that SQLite cannot run. Parallel under xdist, # one database per worker (every test resets its schema), but still slower than # SQLite, so it is not part of the PR aggregate: it runs nightly, on demand, and -# on PRs that touch the database/tenancy layers (#508). +# on PRs that touch database/tenancy or concurrency-sensitive modules (#508, #599). on: schedule: @@ -16,6 +16,9 @@ on: - "framework/db/**" - "framework/testing/**" - "modules/tenants/**" + - "modules/users/**" + - "modules/background_tasks/**" + - "modules/file_storage/**" - "host/migrations/**" - ".github/workflows/postgres-tests.yml" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a7db1093b..4b1b58397 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -69,6 +69,20 @@ jobs: checks: uses: ./.github/workflows/pr.yml + # Public PyPI metadata cannot verify publisher configuration. Block absent + # projects and list all of them before either registry receives uploads. + preflight: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + - uses: astral-sh/setup-uv@v9.0.0 + with: + python-version: ${{ env.PYTHON_VERSION }} + - name: Check PyPI projects + run: uv run --no-project --with tomlkit python -m scripts.release_preflight + # Bump versions in the working tree (no commit yet), build artifacts, upload. # The commit + tag only hit origin in `finalize`, after publishes succeed, # so a failed build never leaves an orphan tag to clean up. @@ -115,7 +129,7 @@ jobs: retention-days: 14 publish-pypi: - needs: build + needs: [build, preflight] runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -170,9 +184,10 @@ jobs: - uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: to-publish + skip-existing: true publish-npm: - needs: build + needs: [build, preflight] runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -212,6 +227,15 @@ jobs: echo "::error::refusing to publish ${files[0]}: package name is '$name', expected '$expected'" exit 1 fi + version=$(tar -xzOf "${files[0]}" package/package.json | python3 -c 'import json,sys; print(json.load(sys.stdin)["version"])') + status=$(curl --silent --show-error --connect-timeout 10 --max-time 30 \ + --output /dev/null --write-out '%{http_code}' \ + "https://registry.npmjs.org/@simple-module-py%2F${{ matrix.package }}/${version}") + case "$status" in + 200) echo "${name}@${version} already exists; skipping"; exit 0 ;; + 404) ;; # Only a missing exact version is safe to publish. + *) echo "::error::npm registry returned HTTP ${status} for ${name}@${version}"; exit 1 ;; + esac # Prefix with ./ so npm treats the arg as a file path, not a # GitHub `user/repo` shorthand (which it does for any bare # single-slash arg, even when the tarball exists on disk). diff --git a/docs/release.md b/docs/release.md index c33acdb7e..1c31520cd 100644 --- a/docs/release.md +++ b/docs/release.md @@ -1,6 +1,6 @@ # Publishing simple_module_python -This repo publishes **13 Python packages** to PyPI and **3 JS packages** to npm in one lockstep version bump. Releases are driven entirely from GitHub Actions — no tokens live on your laptop. +This repo publishes **22 Python packages** to PyPI and **3 JS packages** to npm in one lockstep version bump. Releases are driven entirely from GitHub Actions — no tokens live on your laptop. - **Python packages** (`simple_module_*`) → [pypi.org](https://pypi.org) - **JS packages** (`@simple-module-py/*`) → [npmjs.com](https://www.npmjs.com) @@ -23,7 +23,7 @@ You need to set up Trusted Publisher entries on PyPI and npm *before* running th ### 1. PyPI -For *each* of the 13 published Python project names, on [pypi.org](https://pypi.org/manage/account/publishing/): +For *each* of the 22 published Python project names, on [pypi.org](https://pypi.org/manage/account/publishing/): 1. Log in as the owner account (`antosubash`). 2. Go to **Your account → Publishing** (or click "Add a new pending publisher" if the project doesn't exist yet). @@ -35,27 +35,10 @@ For *each* of the 13 published Python project names, on [pypi.org](https://pypi. - **Environment name**: `pypi` 4. Save. -Repeat for every project in this list: - -``` -simple_module_cli -simple_module_core -simple_module_db -simple_module_hosting -simple_module_test -simple_module_auth -simple_module_background_tasks -simple_module_dashboard -simple_module_feature_flags -simple_module_file_storage -simple_module_permissions -simple_module_settings -simple_module_users -``` - -(`simple_module_audit_log` and `simple_module_keycloak` live in the workspace and -get version-bumped, but are **not** in the `publish-pypi` matrix — they are not -published to PyPI.) +Use the complete `publish-pypi.strategy.matrix.package` list in +[release.yml](../.github/workflows/release.yml) as the source of truth. Every +matrix package is published, including `simple_module_audit_log` and +`simple_module_keycloak`. > **Pending publishers**: if a project doesn't exist on PyPI yet, "pending publisher" is the right flow — you're reserving the project name *and* wiring up auth in one step. The first successful publish creates the project and promotes the pending publisher to a real one. @@ -104,7 +87,7 @@ Both should pass locally. CI should be green on every PR into `main`. ### Step 2. Pick a version -Every workspace package bumps in lockstep to the same version — `scripts/bump_version.py` walks every `framework/*`, `modules/*`, and `packages/*` manifest (so the two unpublished modules, `audit_log` and `keycloak`, stay in sync too). Only the 13 PyPI + 3 npm packages are then published. We follow a relaxed SemVer during the 0.x phase: +Every workspace package bumps in lockstep to the same version — `scripts/bump_version.py` walks every `framework/*`, `modules/*`, and `packages/*` manifest. The 22 PyPI + 3 npm packages are then published. We follow a relaxed SemVer during the 0.x phase: | Situation | Input | |---|---| @@ -125,9 +108,10 @@ The workflow derives the next version from the current `framework/core/pyproject What happens: - `resolve` computes the final version and shares it with every downstream job. - `checks` calls `pr.yml` (`workflow_call`) on the dispatched commit, so the `pr-checks` aggregate must be green. `build` needs it: nothing is built or published from a red commit. +- `preflight` checks every PyPI matrix project via `https://pypi.org/pypi//json`. Both publish jobs require it. A 404 blocks publishing and lists every absent project with a pointer to this guide; other registry errors also block publishing. This public check cannot verify Trusted Publisher configuration or see pending publishers. New projects must have a pending publisher configured and be bootstrapped with their first release before this existence check will pass. - `build` rewrites every version, runs `uv build --all-packages` (a wheel + sdist for every buildable workspace package) plus the 3 npm tarballs, and uploads them as artifacts. -- `publish-pypi` fans out 13 parallel jobs (the matrix package list), each filtering the artifact set down to one package and publishing its wheel+sdist pair via OIDC Trusted Publishing. -- `publish-npm` fans out 3 parallel jobs publishing `@simple-module-py/*` tarballs via OIDC Trusted Publishing. Each job verifies the tarball's `package.json` is actually scoped to `@simple-module-py/*` before invoking `npm publish`. +- `publish-pypi` fans out 22 parallel jobs (the matrix package list), each filtering the artifact set down to one package and publishing its wheel+sdist pair via OIDC Trusted Publishing. Existing distributions are skipped using the publish action’s `skip-existing` option, so a retry can fill in a missing wheel or sdist. +- `publish-npm` fans out 3 parallel jobs publishing `@simple-module-py/*` tarballs via OIDC Trusted Publishing. Each job verifies the tarball's `package.json` is actually scoped to `@simple-module-py/*` before invoking `npm publish`. It queries the exact version from the tarball: HTTP 200 skips it, 404 publishes it, and any other HTTP or network error fails the job. - `finalize` re-applies the bump (the shared `.github/actions/bump-version` action, also used by `build`), cuts the CHANGELOG, commits `release: vX.Y.Z`, tags it, and pushes both. Expected wall time: 5–8 minutes. @@ -191,7 +175,7 @@ uv build --all-packages --out-dir dist-py mkdir -p dist-npm && for p in packages/*/; do npm pack "$p" --pack-destination dist-npm; done # 6. Sanity-check contents -ls dist-py/ | wc -l # expect 30 (a wheel + sdist for each of the 15 buildable packages; only 13 are later published) +ls dist-py/ | wc -l # expect 44 (a wheel + sdist for each of the 22 published packages) ls dist-npm/ | wc -l # expect 3 # 7. Discard only the rehearsal worktree, including its version edits @@ -225,20 +209,34 @@ Fix the entry on the registry, re-run the failed job. Branch protection is blocking `github-actions[bot]`. Add the `RELEASE_PUSH_TOKEN` secret (see First-time setup → Branch protection) and re-dispatch the workflow. The token is consumed automatically when present. -### PyPI publishes succeeded, npm publishes failed (partial release) +### Re-running after a partial publish + +Fix the failed leg's cause (for example, a mismatched Trusted Publisher), then +use **Re-run failed jobs** on the original Actions run. This keeps the resolved +version and original build artifacts. Already uploaded PyPI distributions and +exact npm versions are skipped, and only missing artifacts are published. +`finalize` still commits and tags only after every publish leg succeeds. + +If a new workflow dispatch is necessary, select the same source commit and set +**version** explicitly to the partially published version. Keep the source and +artifacts unchanged: these skip checks do not compare artifact hashes and cannot +replace immutable releases. Do not yank, unpublish, or bump just to resume. -PyPI is immutable — you cannot re-upload `0.0.2` under any circumstance. Options: +### Preflight lists new PyPI projects -1. **Yank** the bad PyPI versions via the PyPI project UI (doesn't delete, but hides them from `pip install`). -2. **Unpublish** the good npm versions within 72 hours: `npm unpublish @simple-module-py/@0.0.2` for each. -3. Fix the root cause. -4. Bump to `0.0.3` and re-run. +The preflight lists every missing matrix project before any publish job runs. +Configure each project's pending publisher as described above, then bootstrap +its first release through that trusted workflow. Pending publishers alone do +not make the project JSON endpoint return 200, so this conservative existence +check will continue to block until the project exists. An existing project can +still have a missing or mismatched publisher; correct that in PyPI settings if +a publish leg returns 403. ### A new module was added — how do I include it in releases? 1. Version-bumping is automatic — `scripts/bump_version.py` discovers any `modules/*/pyproject.toml`, so a new module is picked up with no edit. -2. Add it to `.github/workflows/release.yml` under `publish-pypi` → `strategy.matrix.package` (only matrix packages are published; that's how `audit_log`/`keycloak` stay unpublished). -3. Create the PyPI Trusted Publisher entry for its project name. +2. Add it to `.github/workflows/release.yml` under `publish-pypi` → `strategy.matrix.package` (only matrix packages are published). +3. Create the PyPI pending Trusted Publisher entry and bootstrap its first release; the preflight requires the project to exist on PyPI. 4. Add a substantive README (`check_readmes.py` will fail otherwise). `scripts/check_metadata.py` and `scripts/check_readmes.py` run in `make lint` and will tell you what's missing. @@ -257,14 +255,23 @@ Trusted Publishing is tied to the GitHub repo, not any personal account — so b | PyPI | `simple_module_core` | [framework/core/](../framework/core/) | | PyPI | `simple_module_db` | [framework/db/](../framework/db/) | | PyPI | `simple_module_hosting` | [framework/hosting/](../framework/hosting/) — ships the `sm-host` CLI | +| PyPI | `simple_module_inertia` | [framework/inertia/](../framework/inertia/) | | PyPI | `simple_module_test` | [framework/testing/](../framework/testing/) — pytest plugin | +| PyPI | `simple_module_api_tokens` | [modules/api_tokens/](../modules/api_tokens/) | +| PyPI | `simple_module_audit_log` | [modules/audit_log/](../modules/audit_log/) | | PyPI | `simple_module_auth` | [modules/auth/](../modules/auth/) | | PyPI | `simple_module_background_tasks` | [modules/background_tasks/](../modules/background_tasks/) | +| PyPI | `simple_module_branding` | [modules/branding/](../modules/branding/) | | PyPI | `simple_module_dashboard` | [modules/dashboard/](../modules/dashboard/) | | PyPI | `simple_module_feature_flags` | [modules/feature_flags/](../modules/feature_flags/) | | PyPI | `simple_module_file_storage` | [modules/file_storage/](../modules/file_storage/) | +| PyPI | `simple_module_keycloak` | [modules/keycloak/](../modules/keycloak/) | +| PyPI | `simple_module_mail` | [modules/mail/](../modules/mail/) | | PyPI | `simple_module_permissions` | [modules/permissions/](../modules/permissions/) | | PyPI | `simple_module_settings` | [modules/settings/](../modules/settings/) | +| PyPI | `simple_module_sharing` | [modules/sharing/](../modules/sharing/) | +| PyPI | `simple_module_site_lock` | [modules/site_lock/](../modules/site_lock/) | +| PyPI | `simple_module_tenants` | [modules/tenants/](../modules/tenants/) | | PyPI | `simple_module_users` | [modules/users/](../modules/users/) | | npm | `@simple-module-py/ui` | [packages/ui/](../packages/ui/) | | npm | `@simple-module-py/i18n` | [packages/i18n/](../packages/i18n/) | diff --git a/scripts/release_preflight.py b/scripts/release_preflight.py new file mode 100644 index 000000000..64bc78771 --- /dev/null +++ b/scripts/release_preflight.py @@ -0,0 +1,42 @@ +"""Block releases before publishing if any matrix project is absent from PyPI.""" + +from pathlib import Path +from urllib.error import HTTPError, URLError +from urllib.request import urlopen + +from scripts.check_metadata import parse_publish_matrix + +WORKFLOW = Path(__file__).resolve().parents[1] / ".github/workflows/release.yml" +PYPI_PROJECT_URL = "https://pypi.org/pypi/{name}/json" + + +def main() -> int: + missing: list[str] = [] + errors: list[str] = [] + for name in parse_publish_matrix(WORKFLOW): + try: + with urlopen(PYPI_PROJECT_URL.format(name=name), timeout=30): + pass + except HTTPError as exc: + if exc.code == 404: + missing.append(name) + else: + errors.append(f"{name}: PyPI returned HTTP {exc.code}") + except (URLError, TimeoutError, OSError) as exc: + errors.append(f"{name}: cannot check PyPI: {exc}") + + if missing: + print("::error::New PyPI projects need a pending publisher; see docs/release.md:") + for name in missing: + print(f" - {name}") + print( + "The public API cannot verify pending publishers. Bootstrap these projects " + "with their first release before retrying this existence-only preflight." + ) + for error in errors: + print(f"::error::{error}") + return int(bool(missing or errors)) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_scripts_release_resumption.py b/scripts/tests/test_scripts_release_resumption.py new file mode 100644 index 000000000..c7af8b57b --- /dev/null +++ b/scripts/tests/test_scripts_release_resumption.py @@ -0,0 +1,136 @@ +"""Release publishing must gate new projects and tolerate partial releases (#599).""" + +import io +import json +import os +import subprocess +import tarfile +from pathlib import Path +from unittest.mock import patch +from urllib.error import HTTPError, URLError + +import pytest +import yaml + +ROOT = Path(__file__).resolve().parents[2] +WORKFLOW = ROOT / ".github/workflows/release.yml" + + +def test_every_publish_job_needs_preflight() -> None: + jobs = yaml.safe_load(WORKFLOW.read_text())["jobs"] + assert "preflight" in jobs + assert any( + "python -m scripts.release_preflight" in step.get("run", "") + for step in jobs["preflight"]["steps"] + ) + for name, job in jobs.items(): + if name.startswith("publish-"): + needs = job["needs"] + assert "preflight" in ([needs] if isinstance(needs, str) else needs) + assert set(jobs["finalize"]["needs"]) >= {"publish-pypi", "publish-npm"} + + +def test_pypi_skips_existing_distributions() -> None: + jobs = yaml.safe_load(WORKFLOW.read_text())["jobs"] + publish = next( + step + for step in jobs["publish-pypi"]["steps"] + if step.get("uses", "").startswith("pypa/gh-action-pypi-publish@") + ) + assert str(publish["with"].get("skip-existing")).lower() == "true" + + +def test_postgres_runs_for_concurrency_sensitive_modules() -> None: + workflow = yaml.load( + (ROOT / ".github/workflows/postgres-tests.yml").read_text(), Loader=yaml.BaseLoader + ) + assert set(workflow["on"]["pull_request"]["paths"]) >= { + "modules/users/**", + "modules/background_tasks/**", + "modules/file_storage/**", + } + + +@pytest.mark.parametrize("status", [200, 404, 403, 429, 500, "network"]) +def test_npm_only_publishes_missing_exact_version(tmp_path: Path, status: int | str) -> None: + jobs = yaml.safe_load(WORKFLOW.read_text())["jobs"] + step = next( + step for step in jobs["publish-npm"]["steps"] if step.get("name") == "Publish tarball" + ) + script = step["run"].replace("${{ matrix.package }}", "ui") + dist = tmp_path / "dist-npm" + dist.mkdir() + metadata = json.dumps({"name": "@simple-module-py/ui", "version": "1.2.3"}).encode() + with tarfile.open(dist / "simple-module-py-ui-1.2.3.tgz", "w:gz") as archive: + info = tarfile.TarInfo("package/package.json") + info.size = len(metadata) + archive.addfile(info, io.BytesIO(metadata)) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + # Only the registry and publishing boundaries are replaced; run the real shell step. + curl = bin_dir / "curl" + curl.write_text( + "#!/bin/bash\n" + '[[ "$*" == *"https://registry.npmjs.org/@simple-module-py%2Fui/1.2.3"* ]] || exit 90\n' + + ("exit 7\n" if status == "network" else f"printf '{status}'\n") + ) + npm = bin_dir / "npm" + npm.write_text('#!/bin/bash\nprintf "%s\\n" "$*" > published\n') + curl.chmod(0o755) + npm.chmod(0o755) + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", script], + cwd=tmp_path, + env={**os.environ, "PATH": f"{bin_dir}:{os.environ['PATH']}"}, + capture_output=True, + text=True, + ) + assert (result.returncode == 0) == (status in (200, 404)), result.stderr + assert (tmp_path / "published").exists() == (status == 404) + if status == 404: + assert ( + "publish --access public ./dist-npm/simple-module-py-ui-1.2.3.tgz" + in (tmp_path / "published").read_text() + ) + + +def test_preflight_reports_every_new_matrix_package(capsys: pytest.CaptureFixture[str]) -> None: + from scripts.release_preflight import main + + packages = yaml.safe_load(WORKFLOW.read_text())["jobs"]["publish-pypi"]["strategy"]["matrix"][ + "package" + ] + missing = {packages[0], packages[-1]} + + def registry(url: str, *, timeout: int): + name = url.split("/")[-2] + assert timeout > 0 + assert url == f"https://pypi.org/pypi/{name}/json" + assert name in packages + if name in missing: + raise HTTPError(url, 404, "Not Found", None, None) + return io.BytesIO(b"{}") + + with patch("scripts.release_preflight.urlopen", side_effect=registry): + assert main() == 1 + output = capsys.readouterr().out + assert all(name in output for name in missing) + assert "docs/release.md" in output + assert "pending publisher" in output + + +def test_preflight_accepts_existing_projects() -> None: + from scripts.release_preflight import main + + with patch("scripts.release_preflight.urlopen", side_effect=lambda *a, **kw: io.BytesIO(b"{}")): + assert main() == 0 + + +@pytest.mark.parametrize( + "error", [HTTPError("url", 503, "Unavailable", None, None), URLError("offline")] +) +def test_preflight_fails_closed_on_registry_errors(error: URLError) -> None: + from scripts.release_preflight import main + + with patch("scripts.release_preflight.urlopen", side_effect=error): + assert main() == 1 From 28541a4aef6008f6c5065674c3ac95b8e58ac8f2 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 13:16:09 +0200 Subject: [PATCH 3/8] packaging: cap third-party dependency ranges; users seeds the default role at startup Closes #600 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- framework/cli/pyproject.toml | 4 +- framework/core/pyproject.toml | 10 +-- framework/db/pyproject.toml | 12 +-- framework/hosting/pyproject.toml | 16 ++-- framework/inertia/pyproject.toml | 4 +- framework/testing/pyproject.toml | 8 +- modules/auth/pyproject.toml | 2 +- modules/background_tasks/pyproject.toml | 4 +- modules/file_storage/pyproject.toml | 6 +- modules/keycloak/pyproject.toml | 4 +- modules/mail/pyproject.toml | 2 +- modules/permissions/pyproject.toml | 2 +- modules/settings/pyproject.toml | 2 +- modules/tenants/pyproject.toml | 4 +- modules/users/pyproject.toml | 4 +- .../users/tests/test_users_startup_role.py | 70 ++++++++++++++++ modules/users/users/bootstrap.py | 6 +- modules/users/users/startup.py | 7 +- uv.lock | 84 +++++++++---------- 19 files changed, 163 insertions(+), 88 deletions(-) create mode 100644 modules/users/tests/test_users_startup_role.py diff --git a/framework/cli/pyproject.toml b/framework/cli/pyproject.toml index 1389053c0..dd24fdd82 100644 --- a/framework/cli/pyproject.toml +++ b/framework/cli/pyproject.toml @@ -20,8 +20,8 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "typer>=0.12", - "tomlkit>=0.13", + "typer>=0.12,<0.28", + "tomlkit>=0.13,<0.16", ] [project.scripts] diff --git a/framework/core/pyproject.toml b/framework/core/pyproject.toml index 538073694..1aef596d4 100644 --- a/framework/core/pyproject.toml +++ b/framework/core/pyproject.toml @@ -21,12 +21,12 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "babel>=2.14", + "babel>=2.14,<3", "fastapi>=0.140,<0.157", - "packaging>=23.0", - "pydantic>=2.0", - "pydantic-settings>=2.0", - "pyee>=12.0", + "packaging>=23.0,<27", + "pydantic>=2.0,<3", + "pydantic-settings>=2.0,<3", + "pyee>=12.0,<15", ] [project.urls] diff --git a/framework/db/pyproject.toml b/framework/db/pyproject.toml index 522a8ec75..221d269d8 100644 --- a/framework/db/pyproject.toml +++ b/framework/db/pyproject.toml @@ -21,16 +21,16 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "aiosqlite>=0.20", - "alembic>=1.14", - "asyncpg>=0.30", + "aiosqlite>=0.20,<0.23", + "alembic>=1.14,<2", + "asyncpg>=0.30,<0.33", # psycopg2 is the *sync* Postgres driver Alembic uses for migrations. # The async app code goes through asyncpg; migration env.py rewrites the # URL to +psycopg2 because Alembic's online migrations are sync-only. - "psycopg2-binary>=2.9", + "psycopg2-binary>=2.9,<3", "simple_module_core==0.0.38", - "sqlalchemy[asyncio]>=2.0", - "sqlmodel>=0.0.22", + "sqlalchemy[asyncio]>=2.0,<3", + "sqlmodel>=0.0.22,<0.1", ] [project.urls] diff --git a/framework/hosting/pyproject.toml b/framework/hosting/pyproject.toml index e9b7d767d..837aae9e9 100644 --- a/framework/hosting/pyproject.toml +++ b/framework/hosting/pyproject.toml @@ -21,25 +21,25 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "click>=8.1", + "click>=8.1,<9", "fastapi>=0.140,<0.157", - "httpx>=0.27", + "httpx>=0.27,<0.29", # SessionMiddleware (starlette.middleware.sessions) imports itsdangerous at # module import time; starlette treats it as an optional extra, so declare # it directly — out-of-tree hosts have nothing else pulling it in. - "itsdangerous>=2.0", - "jinja2>=3.1", + "itsdangerous>=2.0,<3", + "jinja2>=3.1,<4", "simple_module_core==0.0.38", "simple_module_db==0.0.38", "simple_module_inertia==0.0.38", - "starlette>=0.44", - "tomlkit>=0.13", - "uvicorn[standard]>=0.34", + "starlette>=0.44,<2", + "tomlkit>=0.13,<0.16", + "uvicorn[standard]>=0.34,<0.55", ] [project.optional-dependencies] # Shared rate-limiter store (SM_REDIS_URL). Without it the limiter counts per worker. -redis = ["redis>=5"] +redis = ["redis>=5,<7"] # Error reporting (SM_SENTRY_DSN). Never imported unless the DSN is set. sentry = ["sentry-sdk[fastapi,celery]>=2.20,<3"] diff --git a/framework/inertia/pyproject.toml b/framework/inertia/pyproject.toml index b6757f06a..e1ead1833 100644 --- a/framework/inertia/pyproject.toml +++ b/framework/inertia/pyproject.toml @@ -20,8 +20,8 @@ classifiers = [ ] dependencies = [ "fastapi>=0.140,<0.157", - "jinja2>=3.1", - "starlette>=0.44", + "jinja2>=3.1,<4", + "starlette>=0.44,<2", ] [project.urls] diff --git a/framework/testing/pyproject.toml b/framework/testing/pyproject.toml index b851b4188..84dc5a0b4 100644 --- a/framework/testing/pyproject.toml +++ b/framework/testing/pyproject.toml @@ -22,13 +22,13 @@ classifiers = [ ] dependencies = [ "fastapi>=0.140,<0.157", - "httpx>=0.27", - "pytest>=8.0", - "pytest-asyncio>=0.24", + "httpx>=0.27,<0.29", + "pytest>=8.0,<10", + "pytest-asyncio>=0.24,<2", "simple_module_core==0.0.38", "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", - "sqlalchemy>=2.0", + "sqlalchemy>=2.0,<3", ] [project.entry-points.pytest11] diff --git a/modules/auth/pyproject.toml b/modules/auth/pyproject.toml index 1745d5365..2e5678bc6 100644 --- a/modules/auth/pyproject.toml +++ b/modules/auth/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "itsdangerous>=2.2", + "itsdangerous>=2.2,<3", "simple_module_core==0.0.38", "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", diff --git a/modules/background_tasks/pyproject.toml b/modules/background_tasks/pyproject.toml index 385cf2ea6..43ffae772 100644 --- a/modules/background_tasks/pyproject.toml +++ b/modules/background_tasks/pyproject.toml @@ -25,8 +25,8 @@ dependencies = [ "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", "simple_module_settings==0.0.38", - "celery[redis]>=5.4", - "redis>=5", + "celery[redis]>=5.4,<6", + "redis>=5,<7", ] [project.entry-points.simple_module] diff --git a/modules/file_storage/pyproject.toml b/modules/file_storage/pyproject.toml index 2dc6aa5a7..05cd9e9a3 100644 --- a/modules/file_storage/pyproject.toml +++ b/modules/file_storage/pyproject.toml @@ -25,12 +25,12 @@ dependencies = [ "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", "simple_module_settings==0.0.38", - "aiofiles>=23", - "pillow>=10", + "aiofiles>=23,<26", + "pillow>=10,<14", ] [project.optional-dependencies] -s3 = ["aioboto3>=13"] +s3 = ["aioboto3>=13,<16"] tasks = ["simple_module_background_tasks==0.0.38"] [project.entry-points.simple_module] diff --git a/modules/keycloak/pyproject.toml b/modules/keycloak/pyproject.toml index b9dcd921f..a70da1821 100644 --- a/modules/keycloak/pyproject.toml +++ b/modules/keycloak/pyproject.toml @@ -25,8 +25,8 @@ dependencies = [ "simple_module_hosting==0.0.38", "simple_module_settings==0.0.38", "simple_module_auth==0.0.38", - "PyJWT[crypto]>=2.8", - "httpx>=0.27", + "PyJWT[crypto]>=2.8,<3", + "httpx>=0.27,<0.29", ] [project.entry-points.simple_module] diff --git a/modules/mail/pyproject.toml b/modules/mail/pyproject.toml index ab6f4384d..45bdc2fa6 100644 --- a/modules/mail/pyproject.toml +++ b/modules/mail/pyproject.toml @@ -11,7 +11,7 @@ keywords = ["simple-module", "mail", "smtp", "email"] dependencies = [ "simple_module_core==0.0.38", "simple_module_settings==0.0.38", - "aiosmtplib>=3.0", + "aiosmtplib>=3.0,<6", ] [project.entry-points.simple_module] diff --git a/modules/permissions/pyproject.toml b/modules/permissions/pyproject.toml index 73997d563..8ec812dfe 100644 --- a/modules/permissions/pyproject.toml +++ b/modules/permissions/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "cachetools>=5.3", + "cachetools>=5.3,<8", "simple_module_core==0.0.38", "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", diff --git a/modules/settings/pyproject.toml b/modules/settings/pyproject.toml index 42f9e49a4..e18629c82 100644 --- a/modules/settings/pyproject.toml +++ b/modules/settings/pyproject.toml @@ -24,7 +24,7 @@ dependencies = [ "simple_module_core==0.0.38", "simple_module_db==0.0.38", "simple_module_hosting==0.0.38", - "typer>=0.12", + "typer>=0.12,<0.28", ] [project.entry-points.simple_module] diff --git a/modules/tenants/pyproject.toml b/modules/tenants/pyproject.toml index eac22d787..b3af6f276 100644 --- a/modules/tenants/pyproject.toml +++ b/modules/tenants/pyproject.toml @@ -15,8 +15,8 @@ dependencies = [ "simple_module_inertia==0.0.38", "simple_module_settings==0.0.38", "simple_module_auth==0.0.38", - "cachetools>=5.3", - "email-validator>=2.1", + "cachetools>=5.3,<8", + "email-validator>=2.1,<3", ] [project.entry-points.simple_module] diff --git a/modules/users/pyproject.toml b/modules/users/pyproject.toml index 8d4b78052..643dcfad3 100644 --- a/modules/users/pyproject.toml +++ b/modules/users/pyproject.toml @@ -33,8 +33,8 @@ dependencies = [ # fields after construction (see reconfigure_cookie_transport in backend.py). # Bumping the major version requires re-checking those field names. "fastapi-users[sqlalchemy,oauth]>=15,<16", - "cachetools>=5.3", - "typer>=0.12", + "cachetools>=5.3,<8", + "typer>=0.12,<0.28", ] [project.entry-points.simple_module] diff --git a/modules/users/tests/test_users_startup_role.py b/modules/users/tests/test_users_startup_role.py new file mode 100644 index 000000000..bdf8ddd27 --- /dev/null +++ b/modules/users/tests/test_users_startup_role.py @@ -0,0 +1,70 @@ +"""Schema-only installs must get a usable signup role at startup (GH #600).""" + +from __future__ import annotations + +import uuid + +import httpx +import pytest +from _users_app_builders import _make_admin_user, _seed_users_settings, _setup_app_db +from simple_module_hosting.app_builder import create_app +from simple_module_hosting.settings import Settings +from sqlalchemy import select +from users.constants import USER_ROLE_ID +from users.models import Role, User, UserRole +from users.module import UsersModule + + +@pytest.mark.parametrize("existing_role", [False, True], ids=["schema-only", "custom-role"]) +async def test_startup_ensures_signup_role_without_overwriting_existing_role(existing_role): + app = create_app( + Settings( + database_url="sqlite+aiosqlite:///:memory:", + environment="testing", + secret_key="test-secret-key", + multi_tenant=False, + redis_url=None, + ) + ) + await _setup_app_db(app) + await _seed_users_settings(app, allow_signup=True) + role_id = uuid.uuid4() if existing_role else USER_ROLE_ID + description = "Operator's custom role" if existing_role else "Standard user" + async with app.state.sm.db.session_factory() as db: + assert (await db.execute(select(Role))).scalars().all() == [] + if existing_role: + db.add(Role(id=role_id, name="user", description=description)) + await db.commit() + + async with app.router.lifespan_context(app): + assert any(role.id == str(role_id) for role in app.state.users.roles_cache) + # A scaffolded app completes admin setup before public signup is usable. + await _make_admin_user(app) + # Re-running startup must preserve the role and avoid duplicate inserts. + await UsersModule().on_startup(app) + assert any(role.id == str(role_id) for role in app.state.users.roles_cache) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://testserver" + ) as client: + response = await client.post( + "/api/users/auth/register", + json={"email": "fresh@example.com", "password": "StrongPass1!"}, + ) + assert response.status_code == 201, response.text + async with app.state.sm.db.session_factory() as db: + roles = (await db.execute(select(Role).where(Role.name == "user"))).scalars().all() + assert len(roles) == 1 + assert (roles[0].id, roles[0].description) == (role_id, description) + assigned = ( + ( + await db.execute( + select(Role.name) + .join(UserRole, UserRole.role_id == Role.id) + .join(User, User.id == UserRole.user_id) + .where(User.email == "fresh@example.com") + ) + ) + .scalars() + .all() + ) + assert assigned == ["user"] diff --git a/modules/users/users/bootstrap.py b/modules/users/users/bootstrap.py index 5bb2876c2..def47d519 100644 --- a/modules/users/users/bootstrap.py +++ b/modules/users/users/bootstrap.py @@ -139,9 +139,9 @@ async def create_admin( async def ensure_user_role(db: AsyncSession) -> Role: """The standard ``user`` Role row, created (and flushed) if it is missing. - Migration ``b5018b617142`` normally seeds it; tests build the schema with - ``create_all`` and get no seed rows, so callers that attach the role must - not assume it exists. + Startup ensures it even in scaffolded hosts whose schema baseline omits + the host's data migration ``b5018b617142``. Bootstrap callers can also use + this before startup has run. """ user_role = ( await db.execute(select(Role).where(col(Role.name) == USER_ROLE_NAME)) diff --git a/modules/users/users/startup.py b/modules/users/users/startup.py index c392de4e3..561579f28 100644 --- a/modules/users/users/startup.py +++ b/modules/users/users/startup.py @@ -41,7 +41,7 @@ async def run_startup(module: UsersModule, app: FastAPI) -> None: """Populate ``app.state.users`` and apply settings-derived configuration.""" from users.backend import reconfigure_cookie_transport - from users.bootstrap import bootstrap_admin_from_env + from users.bootstrap import bootstrap_admin_from_env, ensure_user_role from users.demo import ensure_demo_users from users.deps import auth_backend from users.roles_cache import refresh_roles_cache @@ -64,6 +64,11 @@ async def run_startup(module: UsersModule, app: FastAPI) -> None: # this is the bound on a dropped message rather than on every revocation. configure_session_version_cache(s.session_version_cache_ttl_seconds) + # Scaffolded hosts create a schema baseline without the framework host's + # data migrations. Seed before warming the cache or provisioning accounts. + async with app.state.sm.db.session_factory.begin() as db: + await ensure_user_role(db) + await asyncio.gather( bootstrap_admin_from_env(app), refresh_roles_cache(app), diff --git a/uv.lock b/uv.lock index 2d47c31f7..d576418dd 100644 --- a/uv.lock +++ b/uv.lock @@ -3109,7 +3109,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "itsdangerous", specifier = ">=2.2" }, + { name = "itsdangerous", specifier = ">=2.2,<3" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, @@ -3130,8 +3130,8 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "celery", extras = ["redis"], specifier = ">=5.4" }, - { name = "redis", specifier = ">=5" }, + { name = "celery", extras = ["redis"], specifier = ">=5.4,<6" }, + { name = "redis", specifier = ">=5,<7" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, @@ -3170,8 +3170,8 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "tomlkit", specifier = ">=0.13" }, - { name = "typer", specifier = ">=0.12" }, + { name = "tomlkit", specifier = ">=0.13,<0.16" }, + { name = "typer", specifier = ">=0.12,<0.28" }, ] [[package]] @@ -3189,12 +3189,12 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "babel", specifier = ">=2.14" }, + { name = "babel", specifier = ">=2.14,<3" }, { name = "fastapi", specifier = ">=0.140,<0.157" }, - { name = "packaging", specifier = ">=23.0" }, - { name = "pydantic", specifier = ">=2.0" }, - { name = "pydantic-settings", specifier = ">=2.0" }, - { name = "pyee", specifier = ">=12.0" }, + { name = "packaging", specifier = ">=23.0,<27" }, + { name = "pydantic", specifier = ">=2.0,<3" }, + { name = "pydantic-settings", specifier = ">=2.0,<3" }, + { name = "pyee", specifier = ">=12.0,<15" }, ] [[package]] @@ -3230,13 +3230,13 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "aiosqlite", specifier = ">=0.20" }, - { name = "alembic", specifier = ">=1.14" }, - { name = "asyncpg", specifier = ">=0.30" }, - { name = "psycopg2-binary", specifier = ">=2.9" }, + { name = "aiosqlite", specifier = ">=0.20,<0.23" }, + { name = "alembic", specifier = ">=1.14,<2" }, + { name = "asyncpg", specifier = ">=0.30,<0.33" }, + { name = "psycopg2-binary", specifier = ">=2.9,<3" }, { name = "simple-module-core", editable = "framework/core" }, - { name = "sqlalchemy", extras = ["asyncio"], specifier = ">=2.0" }, - { name = "sqlmodel", specifier = ">=0.0.22" }, + { name = "sqlalchemy", extras = ["asyncio"], specifier = ">=2.0,<3" }, + { name = "sqlmodel", specifier = ">=0.0.22,<0.1" }, ] [[package]] @@ -3279,9 +3279,9 @@ tasks = [ [package.metadata] requires-dist = [ - { name = "aioboto3", marker = "extra == 's3'", specifier = ">=13" }, - { name = "aiofiles", specifier = ">=23" }, - { name = "pillow", specifier = ">=10" }, + { name = "aioboto3", marker = "extra == 's3'", specifier = ">=13,<16" }, + { name = "aiofiles", specifier = ">=23,<26" }, + { name = "pillow", specifier = ">=10,<14" }, { name = "simple-module-background-tasks", marker = "extra == 'tasks'", editable = "modules/background_tasks" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, @@ -3361,19 +3361,19 @@ sentry = [ [package.metadata] requires-dist = [ - { name = "click", specifier = ">=8.1" }, + { name = "click", specifier = ">=8.1,<9" }, { name = "fastapi", specifier = ">=0.140,<0.157" }, - { name = "httpx", specifier = ">=0.27" }, - { name = "itsdangerous", specifier = ">=2.0" }, - { name = "jinja2", specifier = ">=3.1" }, - { name = "redis", marker = "extra == 'redis'", specifier = ">=5" }, + { name = "httpx", specifier = ">=0.27,<0.29" }, + { name = "itsdangerous", specifier = ">=2.0,<3" }, + { name = "jinja2", specifier = ">=3.1,<4" }, + { name = "redis", marker = "extra == 'redis'", specifier = ">=5,<7" }, { name = "sentry-sdk", extras = ["fastapi", "celery"], marker = "extra == 'sentry'", specifier = ">=2.20,<3" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-inertia", editable = "framework/inertia" }, - { name = "starlette", specifier = ">=0.44" }, - { name = "tomlkit", specifier = ">=0.13" }, - { name = "uvicorn", extras = ["standard"], specifier = ">=0.34" }, + { name = "starlette", specifier = ">=0.44,<2" }, + { name = "tomlkit", specifier = ">=0.13,<0.16" }, + { name = "uvicorn", extras = ["standard"], specifier = ">=0.34,<0.55" }, ] provides-extras = ["redis", "sentry"] @@ -3390,8 +3390,8 @@ dependencies = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.140,<0.157" }, - { name = "jinja2", specifier = ">=3.1" }, - { name = "starlette", specifier = ">=0.44" }, + { name = "jinja2", specifier = ">=3.1,<4" }, + { name = "starlette", specifier = ">=0.44,<2" }, ] [[package]] @@ -3410,8 +3410,8 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "httpx", specifier = ">=0.27" }, - { name = "pyjwt", extras = ["crypto"], specifier = ">=2.8" }, + { name = "httpx", specifier = ">=0.27,<0.29" }, + { name = "pyjwt", extras = ["crypto"], specifier = ">=2.8,<3" }, { name = "simple-module-auth", editable = "modules/auth" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, @@ -3431,7 +3431,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "aiosmtplib", specifier = ">=3.0" }, + { name = "aiosmtplib", specifier = ">=3.0,<6" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-settings", editable = "modules/settings" }, ] @@ -3450,7 +3450,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "cachetools", specifier = ">=5.3" }, + { name = "cachetools", specifier = ">=5.3,<8" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, @@ -3524,7 +3524,7 @@ requires-dist = [ { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, - { name = "typer", specifier = ">=0.12" }, + { name = "typer", specifier = ">=0.12,<0.28" }, ] [[package]] @@ -3582,8 +3582,8 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "cachetools", specifier = ">=5.3" }, - { name = "email-validator", specifier = ">=2.1" }, + { name = "cachetools", specifier = ">=5.3,<8" }, + { name = "email-validator", specifier = ">=2.1,<3" }, { name = "simple-module-auth", editable = "modules/auth" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, @@ -3610,13 +3610,13 @@ dependencies = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.140,<0.157" }, - { name = "httpx", specifier = ">=0.27" }, - { name = "pytest", specifier = ">=8.0" }, - { name = "pytest-asyncio", specifier = ">=0.24" }, + { name = "httpx", specifier = ">=0.27,<0.29" }, + { name = "pytest", specifier = ">=8.0,<10" }, + { name = "pytest-asyncio", specifier = ">=0.24,<2" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, - { name = "sqlalchemy", specifier = ">=2.0" }, + { name = "sqlalchemy", specifier = ">=2.0,<3" }, ] [[package]] @@ -3637,7 +3637,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "cachetools", specifier = ">=5.3" }, + { name = "cachetools", specifier = ">=5.3,<8" }, { name = "fastapi-users", extras = ["sqlalchemy", "oauth"], specifier = ">=15,<16" }, { name = "simple-module-auth", editable = "modules/auth" }, { name = "simple-module-core", editable = "framework/core" }, @@ -3645,7 +3645,7 @@ requires-dist = [ { name = "simple-module-hosting", editable = "framework/hosting" }, { name = "simple-module-mail", editable = "modules/mail" }, { name = "simple-module-settings", editable = "modules/settings" }, - { name = "typer", specifier = ">=0.12" }, + { name = "typer", specifier = ">=0.12,<0.28" }, ] [[package]] From 5744d82de9c19bf895cc961bd6c6d33b957a58bb Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 13:16:09 +0200 Subject: [PATCH 4/8] docs: lifecycle hook coverage test, canonical design-doc location, superseded banners, Postgres testing docs Closes #602 Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CLAUDE.md | 16 ++++--- docs/framework/lifecycle.md | 45 ++++++++++++++++++- .../2026-04-13-alembic-migrations-design.md | 2 + docs/plans/2026-04-13-alembic-migrations.md | 2 + docs/plans/README.md | 12 +++++ docs/testing/fixtures.md | 14 ++++++ docs/testing/overview.md | 14 ++++++ .../core/tests/test_core_lifecycle_docs.py | 20 +++++++++ 8 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 docs/plans/README.md create mode 100644 framework/core/tests/test_core_lifecycle_docs.py diff --git a/CLAUDE.md b/CLAUDE.md index e59c6bcee..c6c204923 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,7 +28,7 @@ Everything goes through `make`. Full list: `docs/reference/make-commands.md`. | `make install` | `uv sync --all-packages` + `npm install` | | `make dev` | Shared Docker stack up + `gen-pages` + API (:8000) and Vite (:5050) | | `make test` / `make test-py` / `make test-js` | Pytest (SQLite, e2e/perf excluded; parallel via `pytest-xdist`, `-n $(PYTEST_WORKERS) --dist loadgroup`, default 4 workers, `PYTEST_WORKERS=0` for serial) then Vitest | -| `make test-py-pg` | Pytest on Postgres (`SM_TEST_PG_URL`, default db `sm_test`); parallel like `test-py`, each xdist worker creates its own `sm_test_gw` database (role needs `CREATEDB`); not in `pr-checks` | +| `make test-py-pg` | Pytest on Postgres (`SM_TEST_PG_URL`, default db `sm_test`); parallel like `test-py` (default 4 workers), each xdist worker creates its own `sm_test_gw` database (role needs `CREATEDB`); not in `pr-checks` | | `make test-e2e` | Playwright smoke tests against a running `make dev` | | `make lint` | Ruff format + Ruff + `ty` + Biome + per-workspace `tsc` + file-size cap + hardcoded-string check + untranslated-string check + package metadata/README checks | | `make doctor` | Module diagnostics (`SM0xx`); a dev boot runs the same plus `SM012`/`SM025` | @@ -165,9 +165,9 @@ every Tailwind utility, so keep tokens in `theme.css` and rules in `styles.css` **Lifecycle hooks** on `ModuleBase` (all no-op by default; order and examples in `docs/framework/lifecycle.md`): `register_settings`, `register_menu_items`, `register_permissions`, `register_feature_flags`, `register_event_handlers`, -`register_invalidations`, `register_health_checks`, `register_setup_steps`, +`register_invalidations`, `register_health_checks`, `register_public_routes`, `register_body_limits`, `register_cors_policies`, -`register_rate_limits`, +`register_rate_limits`, `register_setup_steps`, `register_csp_sources`, `register_design_packs`, `register_audit_links`, `register_resource_types`, `register_exception_handlers`, `register_middleware`, @@ -493,8 +493,9 @@ app-level fixtures to every test directory; the root `conftest.py` is thin. - `settings`: in-memory SQLite, `multi_tenant=True`, `auth_provider="users"`. `SM_TEST_DATABASE_URL=postgresql+asyncpg://…` switches the whole suite to - Postgres with a per-test schema reset (`make test-py-pg` does this, with - `-p no:anyio` because an asyncpg connection cannot cross event loops). + Postgres with empty tables per test (schema rebuilt only when changed). + `make test-py-pg` runs 4 workers by default, each with its own database, and + uses `-p no:anyio` because an asyncpg connection cannot cross event loops. - `db_state`, `engine`, `db_session`: fresh `DatabaseState` per test; `db_session` creates all module tables and stamps `alembic_version` at heads. - `app`: `create_app(settings)` with lifespan, seeds `admin@test`; `client` / @@ -570,5 +571,6 @@ When conventions are unclear, read these instead of reverse-engineering code: (entry points, migrations, API stability), `docs/framework/*.md` (lifecycle, middleware, settings, events, i18n, tenancy), `docs/database/*.md`, `docs/testing/*.md`, `docs/reference/*.md` (make commands, env vars, -diagnostic codes), `docs/plans/` (dated design docs; the newest is the intended -state). +diagnostic codes), `docs/superpowers/specs/` (canonical design docs) and +`docs/superpowers/plans/` (implementation plans). `docs/plans/` is historical; +follow status banners and current reference docs rather than dates alone. diff --git a/docs/framework/lifecycle.md b/docs/framework/lifecycle.md index 5eab03f0c..4349bad9e 100644 --- a/docs/framework/lifecycle.md +++ b/docs/framework/lifecycle.md @@ -2,7 +2,9 @@ `ModuleBase` defines a set of lifecycle hooks. All are no-ops by default — override the ones you need. -At boot, for each module (in topological order), the framework calls them in this sequence: +At boot, the framework calls hooks in this sequence. Each phase visits modules +in topological order: settings first for all modules, then registrations per +module, then exception handlers, middleware and routes. ``` register_settings @@ -16,6 +18,7 @@ register_public_routes register_body_limits register_cors_policies register_rate_limits +register_setup_steps register_csp_sources register_design_packs register_audit_links @@ -170,6 +173,38 @@ Per-route rate limits for anonymous and signed-in requests, keyed by user, IP or tenant: `registry.add_prefix("/api/x/", RateLimitRule("10/minute", key="user"))`. See [request-guards.md](request-guards.md). +## `register_setup_steps(registry)` + +Called after `register_rate_limits` and before `register_csp_sources`. Declare +what an install still needs before it is usable: + +```python +from simple_module_core import SetupRegistry, SetupStep + + +def register_setup_steps(self, registry: SetupRegistry) -> None: + registry.add( + SetupStep( + id="orders.catalog", + title="Prepare the order catalog", + title_key="orders.setup.catalog.title", + is_complete=self._has_catalog, + order=30, + ) + ) + + +async def _has_catalog(self, app: FastAPI) -> bool: + return await app.state.orders.catalog_is_ready() +``` + +While any required step is incomplete, `SetupMiddleware` redirects to the +wizard at `/setup`. A step can provide a `SetupAction` to complete it in the +browser; its action runs only while that step is pending. Without an action, +the step must be completed out of band. Registering nothing opts out of the +gate. See [First-run setup steps](../module-authoring.md#first-run-setup-steps) +for action fields, translations and concurrency rules. + ## `register_csp_sources(registry)` Whitelist external origins your frontend loads assets from — a font CDN, a @@ -355,13 +390,21 @@ class OrdersModule(ModuleBase): def register_permissions(self, registry): ... def register_feature_flags(self, registry): ... def register_event_handlers(self, bus, app=None): ... + def register_invalidations(self, bus, app): ... def register_health_checks(self, registry): ... def register_public_routes(self, registry): ... def register_body_limits(self, registry): ... + def register_cors_policies(self, registry): ... + def register_rate_limits(self, registry): ... + def register_setup_steps(self, registry): ... def register_csp_sources(self, registry): ... + def register_design_packs(self, registry): ... + def register_audit_links(self, registry): ... + def register_resource_types(self, registry): ... def register_exception_handlers(self, app): ... def register_middleware(self, app): ... def register_routes(self, api_router, view_router): ... + def register_admin_routes(self, admin_router): ... async def on_startup(self, app): ... async def on_shutdown(self, app): ... diff --git a/docs/plans/2026-04-13-alembic-migrations-design.md b/docs/plans/2026-04-13-alembic-migrations-design.md index a5c5be66b..ccc919307 100644 --- a/docs/plans/2026-04-13-alembic-migrations-design.md +++ b/docs/plans/2026-04-13-alembic-migrations-design.md @@ -1,5 +1,7 @@ # Alembic Database Migrations — Design Document +Status: superseded (see [Migrations — the real graph](../database/migrations.md#the-real-graph)). + > **Note (2026-04-15):** Keycloak integration was removed; see plan > `cryptic-juggling-lightning`. References to "Keycloak" below are > historical context from the original design — the patterns still apply. diff --git a/docs/plans/2026-04-13-alembic-migrations.md b/docs/plans/2026-04-13-alembic-migrations.md index 3fc035592..71dd2c740 100644 --- a/docs/plans/2026-04-13-alembic-migrations.md +++ b/docs/plans/2026-04-13-alembic-migrations.md @@ -1,5 +1,7 @@ # Alembic Database Migrations — Implementation Plan +Status: superseded (see [Migrations — the real graph](../database/migrations.md#the-real-graph)). + > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. **Goal:** Replace auto-`create_all` in module startup with Alembic-managed schema versioning, add migration diagnostics, and extend the health endpoint with migration status. diff --git a/docs/plans/README.md b/docs/plans/README.md new file mode 100644 index 000000000..e5b27c8cf --- /dev/null +++ b/docs/plans/README.md @@ -0,0 +1,12 @@ +# Historical plans and designs + +Status: superseded (see [canonical design specs](../superpowers/specs/)). + +This directory is a historical archive. New design documents belong in +[`docs/superpowers/specs/`](../superpowers/specs/); implementation plans belong +in [`docs/superpowers/plans/`](../superpowers/plans/). + +Use a `Status: current`, `Status: implemented`, or `Status: superseded (see …)` +header on new design documents and update it as work lands or is replaced. +For existing documents, follow status banners and current framework reference +docs; the newest date alone does not establish the intended state. diff --git a/docs/testing/fixtures.md b/docs/testing/fixtures.md index ac276c130..1b724c3a4 100644 --- a/docs/testing/fixtures.md +++ b/docs/testing/fixtures.md @@ -17,6 +17,20 @@ The `simple_module_test` plugin provides app-level fixtures that every test dire All fixtures are **function-scoped** — each test gets its own. The SQLite database is in-memory; tearing down and re-creating is cheap. +## Postgres + +Set `SM_TEST_DATABASE_URL` to an async Postgres URL to use Postgres instead of +in-memory SQLite. `make test-py-pg` sets this from `SM_TEST_PG_URL` and runs +4 workers by default. Under xdist each worker uses its own +`_gw` database, creating it if needed (the role needs `CREATEDB`). +`PYTEST_WORKERS=0 make test-py-pg` runs serially against the named database. + +Each test starts with empty tables. The schema is built once per process and +rebuilt only when changed; Postgres fixtures do not create a separate database +per test. Connections use `NullPool` because pytest-asyncio gives each test +its own event loop. Use `-p no:anyio` when invoking pytest directly on Postgres; +the Make target already includes it. + ## `settings` A pre-built `Settings` object using the in-memory SQLite URL and `multi_tenant=True`. Most tests don't use it directly — the `app` fixture consumes it. diff --git a/docs/testing/overview.md b/docs/testing/overview.md index ba6d8babb..32195ee7b 100644 --- a/docs/testing/overview.md +++ b/docs/testing/overview.md @@ -22,6 +22,20 @@ The default `pytest` invocation pins `-m 'not e2e and not perf'` (your app's `py | `npm test` (or `npx vitest run`) | Run every JS test (from the repo root). | | `npx vitest run ` | Single JS test file. | +## Postgres suite + +`make test-py-pg` runs the Python suite on Postgres in parallel, with 4 workers +by default (`PYTEST_WORKERS` overrides this; `0` runs serially). Set +`SM_TEST_PG_URL` to a disposable test database URL; the default database is +`sm_test`. Each xdist worker uses its own `_gw` database (for +example, `sm_test_gw0`), created if needed; the role needs `CREATEDB` to create +them. Serial runs use the named database itself. + +Tables are emptied between tests and the schema is rebuilt only when changed. +The target sets `SM_TEST_DATABASE_URL` for the fixtures and disables AnyIO's +pytest plugin with `-p no:anyio` so asyncpg connections stay on their event +loop. See [Fixtures](./fixtures.md#postgres) for fixture behavior. + ## Async mode The root `pyproject.toml` sets `asyncio_mode = "auto"`. Async tests don't need `@pytest.mark.asyncio` — any `async def test_*` is picked up automatically. You still need it on fixtures that are async generators. diff --git a/framework/core/tests/test_core_lifecycle_docs.py b/framework/core/tests/test_core_lifecycle_docs.py new file mode 100644 index 000000000..9643bf18c --- /dev/null +++ b/framework/core/tests/test_core_lifecycle_docs.py @@ -0,0 +1,20 @@ +"""Keep the lifecycle reference complete as ModuleBase gains hooks.""" + +import inspect +import re +from pathlib import Path + +from simple_module_core.module import ModuleBase + + +def test_every_public_registration_hook_is_documented(): + root = Path(__file__).resolve().parents[3] + document = (root / "docs/framework/lifecycle.md").read_text(encoding="utf-8") + hooks = { + name + for name, _ in inspect.getmembers(ModuleBase, predicate=inspect.isfunction) + if name.startswith("register_") + } + documented = set(re.findall(r"\bregister_\w+\b", document)) + + assert not (missing := hooks - documented), f"Undocumented lifecycle hooks: {sorted(missing)}" From 392e3702bdcbb2b335fdbde755f0174986185331 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Fri, 23 Oct 2026 13:16:55 +0200 Subject: [PATCH 5/8] changelog: batch 6 entries (#598 #599 #600) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a049f3fcd..db0fc4ac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,14 @@ All notable changes to this project are documented in this file. The format is b installed, otherwise run the durable handlers inline in `db`'s transaction (#583). ### Changed +- Packaging: **Behaviour change.** Published packages now cap their third-party runtime + dependencies (next major for 1.x+ libraries, next minor for 0.x ones such as `sqlmodel`), so + an install can no longer resolve an untested major release (#600). +- `make migration` now requires `LABEL=` and autogenerates against + `