Skip to content

Config to settings, and a first-run setup wizard - #290

Merged
antosubash merged 19 commits into
mainfrom
worktree-config-to-settings
Aug 28, 2026
Merged

Config to settings, and a first-run setup wizard#290
antosubash merged 19 commits into
mainfrom
worktree-config-to-settings

Conversation

@antosubash

@antosubash antosubash commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Reduces the operator-facing env surface to a Postgres URL, a Redis URL and an optional bootstrap admin, and gives installs with no administrator a browser setup wizard that tests its own connections.

Spec: docs/superpowers/specs/2026-08-28-config-to-settings-design.md
Plan: docs/superpowers/plans/2026-08-28-config-to-settings.md

The load-bearing change

create_app is synchronous and consumes config in Phase 1 (module discovery, auth-provider selection, i18n registry) and Phase 8 (middleware) — both before the lifespan opens the database. So host settings marked DB-backed were read from pydantic defaults and never re-read: editing i18n_default_locale or multi_tenant in the admin UI wrote a row nothing consumed. A synchronous pre-app read fixes that and is what makes everything else possible.

A related trap: host/main.py built its own Settings() and passed it to create_app, so create_app's settings or merge_host_settings() fallback never fired in the real host — the read would have been dead code outside the tests.

Env surface

Kept Why
SM_DATABASE_URL Required. Opens the DB, so it can't live in it.
SM_REDIS_URL New, replaces SM_BG_TASKS_BROKER_URL + _RESULT_BACKEND. The Celery worker has no app.state.
SM_ENVIRONMENT, SM_DEBUG, SM_VITE_DEV_URL Process facts, not configuration.
SM_USERS_BOOTSTRAP_* Optional; absence opens the wizard.

Moved to DB-backed with env override: trusted_proxy, log_level, log_format, auth_provider, auth_public_paths, and the four db_pool_* knobs. SM_SECRET_KEY is now optional — generated and persisted atomically on first boot.

Backwards compatible. Precedence stays env → DB → default, and the legacy Redis vars still work (with a deprecation warning) because smpy_gis, smpy_saas, laco_wiki_python and the nodes-k8s manifests all set them.

Setup mode

A SetupRegistry + register_setup_steps hook, not a superuser count in the host. That distinction is load-bearing: a Keycloak install has an empty local users table by design, and a hardcoded count would lock those installs out of their own application permanently. Keycloak registers no step, so the gate never engages there.

What review and QA found

Five review passes and three browser-QA iterations found 18 issues; all are fixed. The ones worth a reviewer's attention, because each was a real defect in this branch rather than a style note:

  • Migration state was fake. resolve_head_revision() used Alembic's singular get_current_head(), which raises on a multi-head history — and this repo's history is legitimately multi-head, since every module's first migration sets its own branch_labels. The exception was swallowed as "Alembic not configured", so every database reported up to date unconditionally. Three things built on it were inert: the wizard's migration card, the host.migrations step, and the boot-time drift check.
  • Privilege escalation on /setup/administrator. It was gated on setup mode generally, but the host always registers host.migrations — so a live install whose schema drifts re-enters setup mode with its administrators intact, and an anonymous request could mint a superuser. It now requires its own step to be pending.
  • The session-signing key was readable. Making SM_SECRET_KEY optional put host.secret_key in the same key/value table the settings API lists; reading it lets you forge a cookie for any account. Masked on every read path — and then masked writes had to be handled too, because the admin edit form round-trips the masked value and would have written "********" over the real key.
  • /setup was an anonymous-access prefix, so any route starting with those six characters (/setup-guide) bypassed auth.
  • A whitespace-only password was accepted on the unauthenticated admin-creation route: " " is eight characters, so both the client minLength and the server's min_length passed it. The route now delegates to UserManager.validate_password instead of reimplementing a subset of it, and that policy measures length after stripping — closing the same hole for signup and password reset.

Verification

  • make test-py: 2326 passed, 0 failed
  • make test-js: 134 passed
  • make lint: full pass (ruff, ty, biome, per-workspace tsc, file-size cap, i18n gate)
  • make doctor: 0 diagnostics
  • Live, on a fresh database: / → 302 /setup → both connection checks healthy → create admin → / serves the app → /setup and /setup/migrations return 404

Rebased onto 52ebdf5. Two conflicts resolved by hand: migrations.py (main extracted a script_directory() helper while this branch made head resolution plural — kept both) and users/locales/en.json (both sides added different sibling keys — kept both).

Known limitation

A zero-table database still cannot boot: module on_startup hooks (users, permissions, …) query their own tables. So the wizard's "Apply migrations" step is only reachable once the schema exists — in practice the Docker image and make dev migrate first. Guarding modules one at a time swaps one opaque crash for another; making startup tolerate an unmigrated schema is its own change.

Test plan

  • Reviewer runs make migrate && make dev against an empty database and confirms / lands on /setup
  • Creating the administrator releases the gate and /setup then 404s
  • CI is green

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6abdaaf
Status: ✅  Deploy successful!
Preview URL: https://61bb45d1.simple-module-python.pages.dev
Branch Preview URL: https://worktree-config-to-settings.simple-module-python.pages.dev

View logs

Reduce the operator-facing env surface to a Postgres URL, a Redis URL and an
optional bootstrap admin, and add a browser setup wizard for installs with no
administrator configured.

The load-bearing part is the epoch boundary: create_app() consumes config in
Phase 1, before the DB opens, so HostSettings fields that are declared
DB-backed (i18n_*, multi_tenant) are read from defaults and never re-read —
editing them in the admin UI does nothing today. A synchronous pre-app config
read fixes that and is a prerequisite for moving anything else.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
create_app builds the module list, the i18n registry and the middleware stack
in Phases 1 and 8, all before the lifespan opens the database. The hydration
that runs there can only swap what a request handler reads later — it cannot
rebuild what has already been constructed. So HostSettings fields declared
DB-backed (i18n_*, multi_tenant, tenant_header) were read from pydantic
defaults and never re-read: editing them in the admin UI wrote a row nothing
consumed.

merge_host_settings does one short-lived read before Phase 1, folding DB
overrides in under anything the environment sets. Precedence stays env → DB →
default; a mutation test confirms the two precedence tests actually fail when
the env check is removed.

Returns {} for an unreachable DB, an unmigrated DB and an empty table alike.
All three are ordinary first-boot states and all three are what the setup
wizard exists to repair, so none may fail the boot.

app_builder was sitting exactly on the 300-line cap, so the startup/shutdown
sequence moved to _lifespan.py — app_builder now owns wiring, _lifespan owns
ordering.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Nine fields move from BootstrapSettings to HostSettings: trusted_proxy,
log_level, log_format, auth_provider, auth_public_paths and the four
db_pool_* knobs. The pool fields carry requires_restart=True because the
engine is built once at boot, so the admin UI says so rather than letting the
edit look like it took effect.

What stays env-only is what cannot come from the DB without a chicken-and-egg
problem — database_url opens the DB, modules_enabled decides whether the
settings module loads at all — plus environment/debug/vite_dev_url, which
describe the process rather than the configuration.

host/main.py now calls merge_host_settings() instead of Settings(). It passes
settings to create_app explicitly, so create_app's own
`settings or merge_host_settings()` fallback never fired in the real host: the
DB read would have been dead code outside the tests. The scaffold template
gets the same fix so new apps aren't born with the bug.

HostSettings gains env_prefix="SM_" — without it a bare HostSettings() reads
unprefixed names, and LOG_LEVEL is a common env var with nothing to do with
this app.

The two settings CLI tests asserted exact import counts and now isolate
themselves from SM_AUTH_PROVIDER, which the suite-wide pinned_auth_provider
fixture sets and which is a genuine host setting to import now.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
One SM_REDIS_URL seeds both the broker and the result backend. Celery
namespaces result keys as celery-task-meta-*, so sharing one Redis database is
safe and is what upstream's quickstart does; the per-field DB overrides still
allow splitting them across databases for anyone who wants that.

SM_BG_TASKS_BROKER_URL and SM_BG_TASKS_RESULT_BACKEND keep working and take
precedence over SM_REDIS_URL — a deployment that set them meant them — but log
a deprecation warning naming the replacement. smpy_gis, smpy_saas,
laco_wiki_python and the nodes-k8s manifests all set these, so removing them
outright would break those deployments on upgrade for no benefit.

A test asserts the supported path stays silent: a warning that fires on the
recommended configuration trains people to ignore it.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
SM_SECRET_KEY was mandatory in production, so a fresh deployment failed to
boot before it could ever show an operator the setup wizard. It is optional
now: env → DB → generate once and store.

The insert is insert-then-suppress-IntegrityError rather than an upsert
because SQLite and Postgres spell that differently, and it re-reads afterwards
instead of returning the key it just generated. That re-read is the whole
point: several workers booting against an empty DB otherwise mint different
keys and invalidate each other's sessions, which presents as users being
randomly logged out only under multiple workers. A mutation check confirms the
concurrency test catches exactly that (8 workers, 8 different keys).

An explicit env value wins and is deliberately not copied into the DB — a
stored duplicate would outlive a deliberate rotation of the env var.

The production placeholder check now fires only when the placeholder was
supplied explicitly (via model_fields_set), and create_app re-checks the
resolved key so callers that build Settings themselves are still covered.

Note for future readers: ruff's SIM117 autofix for the suppress block is wrong
here — contextlib.suppress is a sync context manager and cannot be a second
item in `async with`. It raises TypeError at runtime.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
A wrong Redis URL was invisible until it mattered: the enqueue succeeds, the
message lands in a database no worker listens on, and nothing raises. This
surfaces it on /health/ready, on the module-settings "Test connection" button,
and in the setup wizard's connection step.

The detail string carries the underlying exception verbatim — "connection
refused", "NOAUTH authentication required" and a wrong database number need
three different fixes, and a bare "unhealthy" tells an operator none of them.
Tests assert the connection is released on both the success and failure paths,
since this runs on a probe timer.

The host database check already existed (_db_health.host.database, SELECT 1
with the exception as detail), so only the Redis half was missing. Migration
status stays on app.state.migration rather than being duplicated into the
check; the wizard reads it from there.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
…eware

While any required setup step reports incomplete, SetupMiddleware serves
/setup instead of the app. Without it a fresh deployment has no administrator,
so every route either redirects to a login nobody can pass or fails outright.

The gate is a registry, not a superuser count in the host. That distinction is
load-bearing: a Keycloak install has an empty local users table by design, and
a hardcoded count would lock those installs out of their own application
permanently. Keycloak registers no step, so the gate never engages there.

Completion is recomputed per request rather than latched in a one-way flag, so
an install that loses its administrators is recoverable through the browser
instead of needing shell access. A step whose predicate raises counts as
complete — failing closed on a transient DB error would open an anonymous
admin-creation form, which is failing open on security.

Placed inside InertiaCache (its redirect must not be cached) and before
Maintenance (an install that was never set up has nothing to put into
maintenance). Inertia requests get 409 + X-Inertia-Location rather than 302,
which its client would follow with an XHR and choke on the HTML.

Test fixtures now model a *configured* install — the shared `app` and
`users_app` fixtures seed the admin they already used elsewhere, and
`setup_pending_app` / `users_app_empty` opt into an unconfigured one. Without
that, ~95 existing tests correctly began redirecting to /setup.

Four files sat within a few lines of the 300-line cap, so this splits by
responsibility rather than trimming: ModuleMeta out of core module.py, the
users setup step into users/setup.py, the users test app-builders into
_users_app_builders.py, and the secret-key guard into _secret_key.py.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Four steps at /setup: live pass/fail for Postgres and Redis (showing the
failure reason, since refused and auth-failed need different fixes),
migrations, administrator, site basics. Creating the administrator is what
releases the gate.

Every /setup route refuses once setup completes, asserted by two tests rather
than assumed. That is what bounds /setup/migrations — an unauthenticated
endpoint that can run Alembic — and, more sharply, keeps /setup/administrator
from being an open admin-creation form on a live install. The routes check
this themselves rather than relying on the middleware, which deliberately
exempts /setup from its own redirect.

check_migrations raised on a behind-head database, which would have made the
wizard unreachable in exactly the state it exists to repair. Split into a
non-raising migration_status plus the raising wrapper; the lifespan reports
first and only fails the boot when setup is already complete, preserving
SM010 for configured installs.

/setup also had to join the public-route registry: the setup gate exempted it
from its own redirect, but AuthMiddleware still sent anonymous visitors to a
login page nobody could pass.

Host locale keys live under keys.host.*, so the wizard's strings are
keys.host.setup.*; keys.generated.ts regenerated (33 additions, no namespaces
dropped).

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
.env.example drops from ~30 variables to the three an operator sets:
SM_DATABASE_URL (the only required one), SM_REDIS_URL, and the optional
bootstrap admin whose absence opens the wizard. The moved variables are listed
under a Deprecated heading with the import-from-env command, since env still
overrides DB and existing deployments keep working unchanged.

CLAUDE.md and framework-conventions.md both described host settings as
DB-backed without noting that anything consumed during create_app was read
from defaults — the bug this branch fixes. They now spell out the two reads
and, specifically, that an entry point building its own Settings() and passing
it to create_app skips the pre-app read entirely.

module-authoring.md documents register_setup_steps with the three things that
are not obvious from the signature: registering nothing is how a module opts
out (Keycloak), a raising predicate counts as complete because failing closed
would open an anonymous admin form, and completion is recomputed rather than
latched so a locked-out install can recover through the browser.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Each module's first migration sets its own branch_labels, so this repo's
history legitimately has several heads and `alembic upgrade head` raises
CommandError("Multiple head revisions are present"). `make migrate` has always
used "heads" for this reason; the wizard's endpoint did not.

Caught by booting a genuinely fresh database and driving the wizard end to
end, not by the unit tests — they mount the router but never reach the Alembic
call.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
…e steps "remaining"

Two defects that only showed up by running the thing.

The wizard rendered inside PublicLayout, whose nav offers "Log in" — during
setup that points at a sign-in page no account can pass, and which the gate
redirects straight back to /setup. A dead end offered to someone who has not
finished setting up. The wizard now has its own focused shell: brand mark,
no nav, no footer links. PublicLayout has no prop for suppressing that CTA and
adding one for a single page would be the wrong shape.

The last card listed every step with a tick or an empty circle, under the
heading "Remaining steps" — so a completed migration step sat, ticked, under a
heading saying it was outstanding. It is "Setup steps" now, which is what the
card actually shows.

Verified in a browser against a fresh database: / redirects to /setup, both
connection checks report with detail, creating the administrator lands on the
app, and /setup plus /setup/migrations then return 404.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
From the review, plus two problems the review's own fixes introduced.

HIGH, from the review: migration state was reported as "up to date"
unconditionally. resolve_head_revision() calls Alembic's singular
get_current_head(), which RAISES on a multi-head history — and this project's
history is legitimately multi-head, because each module's first migration sets
its own branch_labels (the same reason make migrate runs `upgrade heads`).
migration_status swallowed that as "Alembic isn't configured" and returned the
no-migrations sentinel. So three things this branch adds were silently inert:
the wizard's migration card, the host.migrations setup step, and the boot-time
drift check. Now resolved via get_heads()/get_current_heads() and a set
difference. Verified: an empty database reports 14 pending, a stamped one
reports current.

Applied by the review: /setup/administrator now requires ITS OWN step to be
pending rather than "setup mode" generally — the host always registers
host.migrations, so a live install whose schema falls behind re-enters setup
mode with its administrators intact, and the broader gate would have let an
anonymous request mint a superuser there. Also a password-length policy on that
route (create_admin writes the hash directly, bypassing
UserManager.validate_password), SQLModel DTOs instead of pydantic BaseModel per
CLAUDE.md, concurrent connection probes, and omitting a check the install
doesn't have rather than showing it as failed.

Introduced by the review, caught here: the new verdict cache stored BOTH
answers, which stranded the operator exactly at success — the wizard creates
the admin, sends the browser to /, a stale negative redirects to /setup, and
/setup has just started 404ing. Reproduced end to end, then fixed by caching
only the positive verdict, which is the half worth caching anyway (a configured
install holds it for months). Regression tests cover both directions.

Test fixtures stamped a single head into alembic_version, which was invisible
while migration state was fake and put 330 tests behind the setup gate once it
became real. They stamp every head now.

The new multi-head test stamps alembic_version rather than running a real
upgrade: alembic's env.py calls fileConfig(), which disables every existing
logger for the rest of the pytest session and silently broke caplog assertions
in 19 unrelated tests.

2255 passed, 1 pre-existing failure (typer's moved _click.exceptions.Exit).

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Pass 2 found the boot-time drift check was unreachable, and it was right: the
host always registers host.migrations, so a behind-head schema is itself one of
the incomplete steps — gating the check on "is setup complete" meant it could
never fire. SM010 was effectively disabled, and the comment claiming otherwise
was false.

Fixed by drawing the distinction the check actually needs. Two situations leave
host.migrations outstanding and want opposite handling: a fresh deployment,
where the wizard should run the migrations, and a configured install whose
schema drifted, where serving traffic is what SM010 exists to prevent — and
where /setup's unauthenticated migration endpoint would become reachable on a
live system. _is_first_run distinguishes them by looking at every step EXCEPT
the schema one. Four tests pin it, including that host.migrations alone never
counts as first-run.

Pass 2 also corrected pending_count: iterate_revisions(head, None) walks to the
base, so a branch one release behind counted its entire history as pending. It
stops at the first already-applied revision now.

Closes pass 1's remaining finding rather than deferring it. Setup-step titles
reach the wizard as backend data and cannot go through useT() at the call site,
but this codebase already has the idiom — MenuItem carries label + label_key and
renders the key with the literal as fallback. SetupStep now does the same, the
route resolves the keys through the existing TranslatorDep, and both shipped
steps ship catalogue entries. A module with no catalogue still renders readable
English rather than a raw key.

routes_setup.py crossed the 300-line cap, so the read-only display shaping moved
to host/setup_payloads.py: routes and their gating in one file, what the page
renders in the other.

2260 passed. Re-verified end to end against a fresh database: / redirects,
both checks report, creating the admin releases the gate, /setup then 404s.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Pass 3 found a severe bug in pass 2's own fix. _is_first_run delegated to
registry.incomplete(), which treats a step whose predicate RAISES as complete —
correct for the steady-state gate, wrong here. On a genuinely fresh database
users.administrator queries a table the migrations create, so it raises, gets
swallowed, and leaves only host.migrations pending — making _is_first_run
return False and failing the boot with SM010 on the very first deploy of any
install. It now walks required_steps directly and treats a raising step as
pending, which is itself evidence the install is new. Reproduced live before
fixing.

Also in this commit, from the review fork (each a real bug in code this branch
introduced):

- host.secret_key was readable back through the settings API and the admin
  browse screen. Making SM_SECRET_KEY optional put the session-signing key in
  the same key/value table the API lists, and anyone who can read it can forge
  a cookie for any account. Masked on every read path; the boot-time reader
  goes through raw SQL and is unaffected. Tests cover all five read paths, that
  an ordinary setting is untouched, and that the stored row keeps the real value.
- attach_public_routes registered a bare "/setup" PREFIX as anonymous, so any
  unrelated route starting with those characters ("/setup-guide") was exempt
  from auth. Exact + trailing-slash now, and SetupMiddleware's exemption list
  gets the same split — the two are mirrors, and a sloppy match there is an
  auth bypass rather than a missed redirect.
- The wizard rendered optional steps as complete regardless of their predicate,
  because incomplete() only ever walks required_steps. Split into
  incomplete()/incomplete_all() — what the gate acts on versus what the page
  displays.
- The admin form rendered "[object Object]" for a 422, which is what the new
  password-length rule produces. It reads FastAPI's array-of-objects detail
  now, and the input carries a matching minLength.

Known limitation, deliberately not fixed here: a zero-table database still
cannot boot, because module on_startup hooks (users, permissions, ...) query
their tables. So the wizard's "Apply migrations" step is only reachable once
the schema exists. Guarding modules one at a time swaps one opaque crash for
another; making startup tolerate an unmigrated schema is its own change.

2265 Python + 127 JS tests pass; one pre-existing typer failure. Live wizard
flow re-verified end to end.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Pass 4 found that pass 3's masking created a worse hole than the one it closed.

Masking covered every READ path but not the write. The admin edit form GETs the
row, pre-fills its input from the response, and PUTs it back — so an admin who
opened host.secret_key and clicked Save, without touching the field, wrote the
literal "********" over the session-signing key. That invalidates every session
and leaves the key a fixed, publicly-known string, making all future cookies
forgeable. Reading the secret was bad; replacing it with a known constant is
worse.

A write whose value is exactly the placeholder is now treated as "leave it
alone" on both update() and upsert_scoped(), rather than rejected — the rest of
the form still saves, and an admin who genuinely types a new key can still
rotate it. A mutation check confirms the test fails without the guard.

Pass 4's other two findings were missing regression tests on security-relevant
behaviour, both now covered:
- the /setup-guide auth bypass (a bare "/setup" prefix exempting any route
  starting with those characters), parametrised over both the exempt and the
  must-not-be-exempt cases;
- incomplete_all(), added last commit specifically so optional steps display
  their real state, which nothing exercised.

test_setup_gate.py crossed the 300-line cap, so it splits along the seam it
already documented: registry behaviour to test_setup_registry.py, middleware
behaviour stays.

2278 passed, 1 pre-existing typer failure.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
…to guard

Pass 5's one finding, and a fair one: test_exemptions_match_exactly covers
SetupMiddleware's own tuples, which decide whether a request is REDIRECTED. The
surface where a loose match is an actual auth bypass is attach_public_routes,
which decides whether it is AUTHENTICATED — and that had no test anywhere. The
test's docstring claimed it was "mirrored by attach_public_routes", so the suite
looked like it guarded the bypass while leaving it open to regression.

test_setup_public_routes.py exercises the real surface: attach_public_routes
against a real PublicRouteRegistry, asserting /setup and its subpaths are
anonymous while /setup-guide, /setupadmin and /setup-wizard/secrets are not,
that the exemption spans every method (the wizard POSTs), and that
SM_AUTH_PUBLIC_PATHS still works alongside it. Reintroducing the bare-prefix
rule fails three of its cases, so it catches the bug it was written for.

The misleading docstring is corrected to say which side it covers.

2288 passed, 1 pre-existing typer failure. Stage A of /ship is clean: five
review passes, 17 findings, all addressed.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
Browser QA found that "        " — eight spaces — was accepted as the first
superuser's password on /setup/administrator, which is unauthenticated. Both
the client minLength and the server's Field(min_length=8) count whitespace, so
a whitespace-only password passed each of them and created a working account.

Two causes, both fixed.

The route reimplemented a subset of the policy instead of calling it. Its own
comment claimed to "mirror UserManager.validate_password", but that method has
three rules (length, not contained in the address, not all digits) and the route
enforced only the first. It delegates now, so the two cannot drift again — and
create_admin writes the hash directly without going through the manager, which
is why this route is the only thing standing between an anonymous caller and a
weak first superuser.

The length rule itself measured raw characters. It strips first now, which
closes the same hole for signup and password reset — every caller of the policy
had it, not just the wizard.

Also on the error path: the wizard's fetches sent Accept: */*, so the host
rendered an Inertia error PAGE for the 422 rather than JSON. resp.json() then
threw and the operator saw "Unprocessable Entity" instead of the reason. All
three wizard fetches now ask for JSON, and a test asserts the refusal actually
carries a usable message.

2295 passed, 1 pre-existing typer failure.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
@antosubash
antosubash force-pushed the worktree-config-to-settings branch from d58cceb to e393d62 Compare August 28, 2026 20:17
@antosubash
antosubash marked this pull request as ready for review August 28, 2026 20:18
CI caught what the dev machine hid. The Redis check was registered probe=True,
so it joined the /health/ready aggregate — and CI has no Redis, which turned
four pre-existing test_health assertions red. Locally the shared dev-services
Redis is up, so the aggregate stayed healthy and every local run passed.

The original justification was that Redis is infrastructure this app owns
rather than a third-party API, so a broker the workers cannot reach is a
genuine readiness failure. That conflates two different questions: whether
this process can serve requests, and whether the system is fully functional.
Readiness answers the first. Failing it pulls the web tier out of the load
balancer, turning "background jobs are backed up" into "the site is down" —
and it marked every Redis-less deployment permanently unready, which is a far
wider blast radius than the wizard this check was added for.

probe=False now, matching the mailer and storage checks and the rule already
written into HealthCheck.probe: a dependency the app serves pages without is
not a readiness signal. The check still runs on demand, which is what the
wizard's connection step and the settings screen's "Test connection" use.

Also fixes test isolation this branch weakened: test_bg_settings_env cleared
the two legacy SM_BG_TASKS_* vars but not the SM_REDIS_URL that now seeds
both, so "env unset" was untrue for anyone exporting it.

Verified both ways — 2327 passed with Redis reachable, and 2327 passed with
SM_REDIS_URL pointed at a dead port, which is the run that reproduces CI.

Claude-Session: https://claude.ai/code/session_013e4RhfCEjHPwxj5Dczqhkz
@antosubash
antosubash merged commit ce8ad91 into main Aug 28, 2026
13 checks passed
antosubash pushed a commit that referenced this pull request Aug 28, 2026
#290 landed a config-to-settings pass that overlaps this branch's #283 work,
so three of the five conflicts are about which env story wins:

- HostSettings takes main's side. #290 makes it deliberately env-readable
  ("Precedence is env → DB → defaults") and reads it pre-app in
  merge_host_settings, so DbBackedSettings would break the setup wizard.
  main's env_prefix="SM_" already closes the bare-name hole #283 reported,
  which also makes the Settings shim's source override unnecessary — reverted.
- users, keycloak and the other module settings keep DbBackedSettings: main
  left those on bare SettingsConfigDict(extra="ignore"), so the hole is still
  open there.
- background_tasks keeps env_prefix and gains main's SM_REDIS_URL fallback.
  With the prefix set, pydantic's env source answers the legacy
  SM_BG_TASKS_BROKER_URL / _RESULT_BACKEND before any default factory runs,
  so the deprecation warning moves to a validator that fires when the legacy
  var is the value's actual source.

SidebarLayout hit 301 lines once main's growth met the footerLinks prop;
hoisting the prop to a local keeps the call on one line and the file at 299.

uv run pytest 2379 passed, make test-js 139 passed, make lint clean,
make doctor 0 diagnostics.

Claude-Session: https://claude.ai/code/session_012k5QBVkMXJqJLVqtWbZJht
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant