Skip to content

Commit 4ac779e

Browse files
authored
feat: module data-compatibility checks and smpy migrations prepare (#711) (#721)
* feat: module data-compatibility checks and smpy migrations prepare (#711) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 1) Setup wizard answers 409 on a required data upgrade; token check samples rows and accepts any column wide enough for a digest; doctor skips a SQLite file that doesn't exist and warns when the DB is unreachable; docs and CLI skill. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: review findings (round 1, pass 1, maintainability) and full-table token probe Token probe checks the whole table (LIMIT 1) so plaintext rows behind hashed ones are found; smpy doctor errors clearly without the framework installed; shared alembic_host fixture; recipe ids tied to module constants; docs. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 2) Defer the boot check on a partly migrated first run; doctor fails on a crashing module check and warns only when the DB is unreachable; modules without the hook open no session. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 * fix: address code review findings (round 1, pass 3) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
1 parent a4d3c87 commit 4ac779e

39 files changed

Lines changed: 1393 additions & 12 deletions

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ All notable changes to this project are documented in this file. The format is b
2626
ones) above `direct_upload_threshold_bytes` now use bounded multipart, including
2727
unknown-length streams, with size/part bounds and abort on failure; the S3
2828
credentials need `s3:AbortMultipartUpload` (#708).
29+
- **Behaviour change:** module data compatibility checks run before startup and in
30+
Doctor (`SM033`); legacy bearer-token storage refuses boot outside development,
31+
while obsolete mail settings warn (#711). `smpy migrations prepare` writes
32+
frozen host-owned recipes for mail namespace changes (`d7b3a9e5c1f2`) and token
33+
invalidation/column changes (`a626b1c8d023`), without applying migrations. `smpy doctor` is a new alias for `make doctor`
34+
on scaffolded hosts.
2935
- Users auth emails support ordered host template directories through the
3036
restart-required DB setting `mail_template_dirs`, with per-file bundled
3137
fallback and optional autoescaped HTML alternatives (#712).

‎CLAUDE.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,10 @@ every Tailwind utility, so keep tokens in `theme.css` and rules in `styles.css`
173173
`register_design_packs`,
174174
`register_audit_links`, `register_resource_types`, `register_exception_handlers`, `register_middleware`,
175175
`register_routes(api_router, view_router)`, `register_admin_routes(admin_router)`
176-
(only with `ModuleMeta.admin_view_prefix`), then async `on_startup` /
177-
`on_shutdown` (reverse order). Non-hook overrides: `locale_dirs()`,
176+
(only with `ModuleMeta.admin_view_prefix`), then async `check_data_compatibility(db)`
177+
(read-only upgrade checks before settings hydration; an `error` refuses boot
178+
outside development, `smpy migrations prepare <upgrade-id>` writes the fix into
179+
the host), then async `on_startup` / `on_shutdown` (reverse order). Non-hook overrides: `locale_dirs()`,
178180
`db_extensions()`, `template_dirs()`, `static_mounts()`, `spa_mounts()`.
179181

180182
- A module gets one view router per prefix. All-admin modules point
@@ -475,8 +477,9 @@ not run the suite** (only `SM025` is reported there). What fails a production
475477
boot is strict discovery (a bad entry point, missing `meta`, or a missing
476478
`depends_on` target), the
477479
auth-provider check, placeholder secrets (`SM_SECRET_KEY`, users token
478-
secrets), a localhost Celery broker, and the `SM010` migration check (except
479-
on a never-migrated first run, which goes to `/setup`). Catch everything else
480+
secrets), a localhost Celery broker, the `SM010` migration check (except
481+
on a never-migrated first run, which goes to `/setup`) and an `error` from a
482+
module's `check_data_compatibility` (`SM033`). Catch everything else
480483
with `make doctor` before shipping. Codes that
481484
change how you write code: `SM003`/`SM004` render-key mismatch, `SM009`
482485
framework→plugin import, `SM011` table missing from migrations, `SM012`

‎docs/.vitepress/config.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ export default defineConfig({
9292
{ text: "Quickstart", link: "/guide/quickstart" },
9393
{ text: "Project structure", link: "/guide/project-structure" },
9494
{ text: "Configuration", link: "/guide/configuration" },
95+
{ text: "Upgrading a host", link: "/guide/upgrading" },
9596
{ text: "Your first module", link: "/guide/first-module" },
9697
{ text: "OpenAPI & clients", link: "/guide/openapi-clients" },
9798
],

‎docs/database/migrations.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,29 @@ All migrations live in `host/migrations/versions/` — **not** in module package
88
- **Autogenerate sees everything.** `host/migrations/env.py` calls `build_module_metadata()` to union every installed module's `MetaData`. Autogenerate diffs the DB against that union and writes one migration covering all changes.
99
- **Operators run one command.** `make migrate` is the only target. No "did you also run `orders/migrate`?" footgun.
1010

11+
## Module data upgrades in consuming hosts (#711)
12+
13+
Scaffolded hosts own their migration graphs. A schema-only baseline generated
14+
from current models does not include this repository's historical data changes.
15+
Before deploying upgraded packages, run `make doctor` or `uv run smpy doctor`.
16+
Semantic module checks (`SM033`) recognize equivalent/manual ports independently
17+
of upstream revision ids.
18+
19+
`uv run smpy migrations prepare <upgrade-id> --head <host-head-or-label@head>`
20+
copies a frozen, self-contained recipe into the host's own `migrations/versions`;
21+
it never applies it. An explicit current head is required. The bundled ids are
22+
`mail.settings_namespace_v1` (system settings move, target wins, matching
23+
`d7b3a9e5c1f2`) and `users.hashed_bearer_tokens_v1` (invalidate credentials before
24+
altering both token columns, matching `a626b1c8d023`). The first recipe for an
25+
unlabelled module declares `mail`/`users`; existing labels are not redeclared.
26+
Review the generated revision before applying it with `upgrade heads`.
27+
28+
Boot checks run before module settings hydration/startup; token incompatibility
29+
refuses boot outside development, while obsolete mail keys warn. Never-migrated
30+
first runs defer checks until the wizard applies migrations. No automatic
31+
credential deletion happens at startup. See the [scaffolded host upgrade
32+
guide](../guide/upgrading.md) for the operator workflow and token sign-out risk.
33+
1134
## The real graph
1235

1336
The history is **not** a single linear chain. It has several roots and several heads, and `make migrate` runs `upgrade heads` for that reason. On `main` today:

‎docs/framework/lifecycle.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ register_exception_handlers
2727
register_middleware
2828
register_routes
2929
register_admin_routes (only when meta.admin_view_prefix is set)
30+
-- check_data_compatibility (async, before module settings hydration)
3031
-- on_startup (async, after middleware is installed)
3132
```
3233

@@ -350,6 +351,17 @@ def register_resource_types(self, registry: ResourceTypeRegistry) -> None:
350351
)
351352
```
352353

354+
## `check_data_compatibility(db)`
355+
356+
This async read-only hook returns a sequence of `DataCompatibilityIssue` frozen
357+
dataclasses (`upgrade_id`, `message`, `severity="warning"|"error"`), empty by default.
358+
Use stable semantic ids and inspect actual schema/data state so hosts with
359+
independently ported revisions pass. Checks run before module DB-settings
360+
hydration and `on_startup`; errors refuse boot outside development. A never-
361+
migrated first run defers checks until migrations have been applied. Doctor
362+
reports the same issues as `SM033`. Checks must never clear credentials or apply
363+
migrations; see [Migrations](../database/migrations.md).
364+
353365
## `on_startup()` / `on_shutdown()`
354366

355367
Async lifespan hooks that run after all modules are registered.

‎docs/guide/upgrading.md‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# Upgrading a scaffolded host
2+
3+
A host generated by `smpy new` or `smpy create-host` owns its Alembic history.
4+
Its initial revision reflects the models installed at generation time; it does
5+
not import the framework repository's historical revisions. Updating packages
6+
and running schema autogenerate cannot reproduce historical data transformations.
7+
8+
## Check and prepare
9+
10+
Back up the database and review the release notes before upgrading packages.
11+
From the project root, with the host's Python environment active:
12+
13+
```bash
14+
uv run smpy doctor
15+
uv run alembic -c host/alembic.ini heads
16+
uv run smpy migrations prepare mail.settings_namespace_v1 --head <your-host-head>
17+
uv run smpy migrations prepare users.hashed_bearer_tokens_v1 --head users@head
18+
```
19+
20+
Replace `<your-host-head>` with an actual head in **your** graph. Use `users@head`
21+
only if your graph already declares that label; otherwise pass the relevant host
22+
head id. Preparing the mail revision may advance the same branch, so inspect
23+
`heads` again before preparing the next revision. For flat hosts use
24+
`alembic.ini` instead of `host/alembic.ini`. The preparation command detects both
25+
layouts, or accepts `--config path/to/alembic.ini`.
26+
27+
Preparation writes one self-contained revision into that host's
28+
`migrations/versions`. It never connects to the database or applies the revision.
29+
The frozen recipes ship in the CLI package, and generated revisions import only
30+
Alembic and SQLAlchemy: future module/package upgrades cannot change their code.
31+
The first prepared recipe for an unlabelled module declares `mail` or `users`;
32+
existing labels are reused without redeclaration. `--head` is required and must
33+
resolve to one current host head. No upstream roots are grafted onto your graph.
34+
35+
## Review and apply
36+
37+
Review and keep the generated revision in your application's version control.
38+
Do not generate a token invalidation revision for a baseline that is already
39+
compatible: applying it deliberately signs clients out.
40+
41+
| Upgrade id | Effect |
42+
|---|---|
43+
| `mail.settings_namespace_v1` | Move system-scope `users.mailer` to `mail.backend` and the six `users.smtp_*` transport keys to `mail.*`. Existing target values win; stale source rows are removed. Tenant/user settings are untouched. Downgrade reverses the mapping with the same target-wins rule. |
44+
| `users.hashed_bearer_tokens_v1` | Delete access and refresh credentials **before** changing both token columns to `String(64)`. Downgrade deletes credentials again and restores the legacy column types; it cannot recover secrets. |
45+
46+
Stop every old application worker, including background workers, before applying
47+
the token upgrade. Mixed old/new workers must not mint or use plaintext tokens
48+
during this upgrade. Clients using bearer tokens or `sm_auth` must sign in again.
49+
Other session cookies are unaffected. After reviewing and scheduling the upgrade:
50+
51+
```bash
52+
uv run alembic -c host/alembic.ini upgrade heads
53+
uv run smpy doctor
54+
```
55+
56+
Restart the application with the upgraded packages. Role provisioning remains an
57+
idempotent startup step; it needs no prepared data revision.
58+
59+
## Compatibility checks
60+
61+
Modules expose read-only `check_data_compatibility(db)` hooks with semantic upgrade
62+
ids. Boot runs them before module settings hydration and module startup. Doctor
63+
(`make doctor` or `smpy doctor`) reports them as `SM033` and exits nonzero on errors.
64+
65+
Unmoved system mail keys warn. Incompatible token columns or legacy token
66+
representations refuse boot outside development; development logs warnings.
67+
Checks inspect state, so equivalent manual ports are accepted without requiring
68+
upstream revision ids. A current schema-only baseline with empty token tables
69+
passes and does not trigger credential deletion.
70+
71+
A never-migrated database skips the initial check so the first-run wizard stays
72+
reachable. Checks replay after wizard/out-of-band migrations before deferred
73+
settings hydration/startup; an incompatibility keeps the setup gate closed.
74+
This is detection and explicit preparation for the two bundled upgrades, not an
75+
automatic migration system for arbitrary third-party module data.

‎docs/reference/diagnostic-codes.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ The framework runs a set of static checks over installed modules at app boot. Th
4141
| `SM030` | WARNING at a dev boot; ERROR in `make doctor` | A module's `depends_on` names a module that is not loaded (not installed, left out of `SM_MODULES_ENABLED`, or dropped because another auth provider was selected — `keycloak` drops `Users`). Outside development the boot raises `MissingDependencyError` listing every pair. | Install and enable the dependency, or leave the dependent module out. A relationship that is only about order belongs in `optional_depends_on`. See [Discovery](/framework/discovery#modulemeta). |
4242
| `SM031` | WARNING | An `/api` route has no permission, `TokenAllowed` or `Authenticated` gate anywhere in its dependency tree (include-level dependencies count) and no public-route match, so the schema cannot say who may call it. Needs the built app: reported at a **dev boot** (and in the Doctor screen), not by `make doctor`. | Add `Depends(RequiresPermission(...))`, or `Depends(Authenticated())` when any signed-in user may call it, or register it in `register_public_routes`. |
4343
| `SM032` | ERROR (fails boot in every environment) | A `ModuleBase.spa_mounts()` prefix overlaps a module's `route_prefix`/`view_prefix`/`admin_view_prefix`, a `static_mounts()` prefix, a framework path (`/api`, `/admin`, `/setup`, `/i18n`, `/health`, `/static`, `/modules`) or another SPA mount. The mount would shadow, or be shadowed by, that router. | Pick a prefix nothing else uses. See [Legacy SPA mounts](/framework/spa-mounts). |
44+
| `SM033` | ERROR or WARNING | A module data compatibility check found a missing semantic upgrade (a WARNING when the database could not be checked; a SQLite file that does not exist yet is skipped). Token schema/legacy credentials are errors; unmoved mail system keys warn. Errors refuse boot outside development. Never-migrated first runs defer checks. | Run the reported `smpy migrations prepare <upgrade-id> --head <host-head-or-label@head>`, review the host revision and apply `upgrade heads`; see [Upgrading a host](/guide/upgrading). |
4445

4546
`SM022`/`SM023` are the two halves of the same invariant: a module's optional [`theme.css` is imported unlayered and `styles.css` into `layer(components)`](/module-authoring#styling), and CSS put in the wrong one silently does nothing (or silently wins everything).
4647

‎framework/cli/simple_module_cli/cli.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from simple_module_cli.add_cmd import add_module, run_add
2424
from simple_module_cli.case import to_kebab_case
2525
from simple_module_cli.docker_cmd import docker_app
26+
from simple_module_cli.migrations_cmd import migrations_app
2627
from simple_module_cli.module_cmd import module_app
2728
from simple_module_cli.new import new_project
2829
from simple_module_cli.openapi_cmd import openapi_app
@@ -49,6 +50,22 @@
4950
app.add_typer(skills_app, name="skills")
5051
app.add_typer(module_app, name="module")
5152
app.add_typer(openapi_app, name="openapi")
53+
app.add_typer(migrations_app, name="migrations")
54+
55+
56+
@app.command("doctor")
57+
def doctor() -> None:
58+
"""Check installed modules and their database compatibility."""
59+
try:
60+
from simple_module_core.__main__ import main as run_doctor
61+
except ImportError:
62+
# The CLI installs on its own (to scaffold); doctor needs a project's framework.
63+
typer.echo(
64+
"ERROR: smpy doctor needs simple_module_core; run it inside a project.", err=True
65+
)
66+
raise typer.Exit(1) from None
67+
68+
raise typer.Exit(run_doctor())
5269

5370

5471
@app.command("create-host")
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
"""Frozen system mail-settings move from d7b3a9e5c1f2 (#711).
2+
3+
An existing target value wins; the obsolete source row is removed.
4+
"""
5+
6+
import sqlalchemy as sa
7+
from alembic import op
8+
9+
_TABLE = "settings_setting"
10+
_SCOPE_SYSTEM = "system"
11+
_SYSTEM_SCOPE_ID = ""
12+
13+
_KEY_PAIRS = (
14+
("users.mailer", "mail.backend"),
15+
*((f"users.smtp_{f}", f"mail.smtp_{f}") for f in ("host", "port", "username", "password")),
16+
("users.smtp_from", "mail.smtp_from"),
17+
("users.smtp_tls", "mail.smtp_tls"),
18+
)
19+
20+
21+
def _rename(old: str, new: str) -> None:
22+
scope = {"scope": _SCOPE_SYSTEM, "scope_id": _SYSTEM_SCOPE_ID, "old": old, "new": new}
23+
bind = op.get_bind()
24+
bind.execute(
25+
sa.text(
26+
f"DELETE FROM {_TABLE} WHERE scope = :scope AND scope_id = :scope_id AND key = :old"
27+
f" AND EXISTS (SELECT 1 FROM {_TABLE} AS t WHERE t.scope = :scope"
28+
" AND t.scope_id = :scope_id AND t.key = :new)"
29+
),
30+
scope,
31+
)
32+
bind.execute(
33+
sa.text(
34+
f"UPDATE {_TABLE} SET key = :new"
35+
" WHERE scope = :scope AND scope_id = :scope_id AND key = :old"
36+
),
37+
scope,
38+
)
39+
40+
41+
def upgrade() -> None:
42+
for old, new in _KEY_PAIRS:
43+
_rename(old, new)
44+
45+
46+
def downgrade() -> None:
47+
for old, new in _KEY_PAIRS:
48+
_rename(new, old)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
"""Frozen token-storage upgrade from a626b1c8d023 (#711).
2+
3+
Upgrade and downgrade invalidate all access/refresh credentials. Stop all old
4+
application workers before upgrading; clients must sign in again.
5+
"""
6+
7+
import sqlalchemy as sa
8+
from alembic import op
9+
10+
11+
def _clear_tokens() -> None:
12+
for table in ("users_refresh_token", "users_access_token"):
13+
op.execute(sa.table(table).delete())
14+
15+
16+
def upgrade() -> None:
17+
_clear_tokens()
18+
with op.batch_alter_table("users_refresh_token") as batch:
19+
batch.alter_column(
20+
"token", existing_type=sa.Uuid(), type_=sa.String(64), postgresql_using="token::text"
21+
)
22+
with op.batch_alter_table("users_access_token") as batch:
23+
batch.alter_column("token", existing_type=sa.String(43), type_=sa.String(64))
24+
25+
26+
def downgrade() -> None:
27+
# Digests cannot be restored to UUIDs or bearer secrets.
28+
_clear_tokens()
29+
with op.batch_alter_table("users_refresh_token") as batch:
30+
batch.alter_column(
31+
"token", existing_type=sa.String(64), type_=sa.Uuid(), postgresql_using="token::uuid"
32+
)
33+
with op.batch_alter_table("users_access_token") as batch:
34+
batch.alter_column("token", existing_type=sa.String(64), type_=sa.String(43))

0 commit comments

Comments
 (0)