Files
siftlode/backend/app
peter ed91c83dd7 fix(auth): name the state and the address behind a refused sign-in (E9.5 S4)
"Your account is awaiting admin approval." was true and useless. It never said WHICH mailbox the
account was on, and on 2026-09-03 that silence hid a Gmail alias mismatch for a day: a registration
as np.temu@gmail.com met a Google claim of nptemu@gmail.com, and every screen in the funnel agreed
to say nothing about it.

Every refusal password-login can answer with now uses the codebase's structured {code, reason}
detail, built in one helper. The three that arrive AFTER the password is proven carry the address on
the account plus both gate flags; the two that answer before it (invalid, rate_limited) carry
nothing, so a wrong password and an unknown account stay byte-identical. The frontend renders the
first three as a named screen in place of the form — there is nothing to retype once the password is
correct — with an icon, the two-gate checklist, the address in monospace, and a resend CTA where one
helps. Reporting each gate from its own flag is the payoff of S1: approved-but-unverified is a real
state now, and a screen that assumed a sequence would send that user off to wait for an admin who
has already decided.

The Google access-request notice names the address too, and says so when the request was already on
file under ANOTHER spelling of the same mailbox — the honest version of "we didn't open a second
one", stated only when it applies. It moved from the query string to the fragment: the notice now
carries an address, and a query string lands in every reverse-proxy access log. Same reasoning that
put the verify/reset tokens there, and the same owner already clears it. urlencode, not an f-string,
because URLSearchParams reads a bare + in a fragment as a space and would mangle exactly the
+tag addresses this epic exists to protect.

upsert_pending_invite returns the row it settled on beside the is-new flag, since the row's stored
address is what the notice has to name and what mail will go to.

Deviation from the PendingScreen mockup, deliberately: no "Sign out" button and no separate "Not
your address?" link. The mockup drew the panel in the app shell; the accepted decision (a) keeps the
pending user session-less, so there is no session to end, and the address is named directly above a
"Back to sign in" that answers the same question.

UAT on localdev, both languages: all three refusals with the typed spelling deliberately differing
from the stored one (the panel named the stored one), the resend CTA carrying the typed address into
the resend form, and both access-request banners with a +tag address surviving the fragment round
trip. Throwaway accounts and the language switch reverted.
2026-09-04 01:56:55 +02:00
..
2026-06-14 04:40:22 +02:00

Backend conventions

The rules below are already followed across backend/app — they lived only in a few module docstrings (demo.py, userscope.py, routes/_common.py) until this file collected them (E3 S3, B-core-20). When you add code, match these so the next reader doesn't have to guess "which pattern do I copy".

Module layering

Three layers, by who runs the code — not by feature:

  1. routes/… — request-only. A route module holds HTTP concerns: auth dependencies, request/ response models, status-code mapping, serialization. It owns logic that runs only on a request. Business logic that a request happens to trigger does not live here if anything else runs it too.

  2. plex/, downloads/, sync/ — domain logic shared by more than one caller. Anything the scheduler or the worker runs (not just a request) lives in one of these domain packages, so the route, the worker, and the scheduler share one definition. Examples from this codebase: downloads/service.py owns enqueue + the job state transitions (routes call it); plex/sync.py is run by both the admin route and the scheduler; downloads/pipeline.py is the worker's child pipeline.

  3. Flat app/*.py core modules — cross-cutting primitives. auth, sysconfig, quota, audit, notifications, userscope, realtime, ratelimit, progress, … are single-file primitives many layers import. They depend on nothing above them (a core module must never import a routes/… module).

Extraction trigger: the SECOND consumer. Keep new logic inline in its first caller. The moment a second caller needs it (a worker as well as a route; a scheduler as well as a route), extract it down a layer — into the relevant domain package, or a flat core module if it's cross-cutting. Don't pre-abstract for a single caller; don't leave shared logic duplicated once there are two.

Transaction ownership

This follows CLAUDE.md's canonical rule verbatim — "the handler/job-wrapper commits; helpers flush (only quota owns its session)" — so read it that way, not as an exception to it. The commit belongs to the operation owner; anything called inside another transaction only stages its change (db.add / flush) and lets the owner commit, so the whole operation is one transaction. audit.record() is the reference: it adds the audit row and the caller commits it together with the mutation it records, so an audit entry can never exist without its change (or vice-versa).

  • The operation owner is usually the route handler. A service.* operation the handler delegates a whole transition to (e.g. service.cancel) is that owner and commits its one transition — it isn't a mid-operation helper. A helper that owns its OWN short-lived session (the worker's jobdb writes, the same carve-out CLAUDE.md grants quota) commits that session. Everything else flushes.
  • SessionLocal is expire_on_commit=False: after a raw UPDATE + commit, an ORM object you already loaded is stale. Read the fresh value from the row (RETURNING, or a re-SELECT), not the ORM attribute — see service.decr_ref_count.

Shared counters and status: do it atomically at the DB

  • Never read-modify-write a shared counter in Python (x.count = x.count + 1). Concurrent sessions lose an update. Use a DB-atomic statement — col = col + 1, or greatest(col - 1, 0) with RETURNING so the caller branches on the post-value under the row lock (service.incr_ref_count / decr_ref_count).
  • Download job status is a JobStatus StrEnum (downloads/status.py), not bare strings. The HOLDING set defines which states hold a MediaAsset (so release accounting stays correct); STOPPED_BY_ROUTE is the route-initiated stop set. A worker child's terminal write goes through service.finalize_if_running (an atomic UPDATE … WHERE status='running') so a finishing child can't resurrect a job a route just stopped.

Config: one home per kind

  • app.config.settings — boot/env config (DB URL, download root, worker concurrency): read once at startup, not user-tunable at runtime.
  • sysconfig.SPECS — the runtime admin knobs (DB-override-or-default). A new admin-tunable value goes in SPECS; don't invent a fourth store. (state.py's maintenance_batch predates this and is slated to fold into sysconfig — do not add more like it.)

The gate

Everything below must be green before a change is done (run via siftlode check): ruff (backend) · tsc ×3 + eslint + prettier + knip (frontend) · vitest · pytest. The pytest lane runs in backend/docker-compose.test.yml — an ephemeral Postgres + the backend tree bind-mounted — so the DB-backed integration tests (the db fixture) actually run; SIFTLODE_TEST_ REQUIRE_DB=1 there turns a would-be silent skip into a hard failure. See docs/development.md.