Reverses a shipped, tested, merged decision (D4, PR #2) rather than building on it — see docs/DECISIONS.md D15 for the full record. The user asked for a single-container deploy; when
that meant Postgres either stayed a second container or got bundled inside the single one via a
process supervisor, they chose SQLite instead. I raised the concrete costs before building this
(no database-level RLS, no PostGIS, procrastinate needs replacing) and they reaffirmed — that's
their call to make about their own self-hosted instance, and this PR is the honest execution of it,
not a token driver swap.
The one invariant-critical consequence: isolation between users now rests entirely on the
repository-layer scope (db.py's Scope.select()), not two layers. CLAUDE.md's invariant #4 is
revised accordingly. This is not a downgrade-and-hope — Scope is built so an unfiltered query
against a user-owned table is structurally harder to write than a scoped one (there is no method
on Scope that returns one).
How this was verified
ruff check, ruff format --check, mypy --strict — all clean
pytest — 13/13 passing against a real SQLite file, including with DeprecationWarning
promoted to an error (confirms a fix actually holds, not just that it's quiet by default)
Full alembic upgrade head -> downgrade -1 -> upgrade head cycle
alembic check — clean, with no include_object filter needed at all now (SQLite starts
with nothing but what our own migrations create — no PostGIS/TIGER noise to filter, unlike the
Postgres version)
CI's exact migration command sequence reproduced locally, end to end, before touching the
workflow file
CI is green (pending this push)
Tests added for the behaviour that changed — including a deliberately alarming one
(test_unscoped_session_can_see_every_user_when_misused) that shows exactly what a reviewer
must now catch, since nothing else will
Verified manually — six real SQLite behaviours below were each confirmed empirically with a
minimal standalone repro before being written into the actual fix, not assumed from docs
Six real bugs found by actually running this against real SQLite
Each one below was reproduced in isolation first, then fixed, then re-verified — not assumed:
Foreign keys (incl. ON DELETE CASCADE) are OFF by default per connection. Deleting a user
silently left orphaned sessions/api_tokens rows, no error either way. Fixed: PRAGMA foreign_keys=ON on every connect.
Transactions default to DEFERRED, taking a write lock only on the first actual write — a
real check-then-act race on invite redemption (two concurrent redemptions could both read used_count < max_uses as true before either commits). Fixed: BEGIN IMMEDIATE on every
transaction — SQLAlchemy's own documented recipe for this, not improvised.
DateTime(timezone=True) does not round-trip tzinfo on SQLite — a tz-aware datetime goes
in, a naive one comes out, and every expires_at < datetime.now(UTC) comparison in auth/service.py then raises TypeError. Fixed once at the Base level with a UTCDateTime TypeDecorator, not per-column.
Uuid(as_uuid=True) stores as 32-char hex with no hyphens, not str(uuid)'s form. A test
fixture that raw-inserted the hyphenated form left rows the ORM's own later UPDATE (via invite.used_count += 1's autoflush) could never match by primary key — StaleDataError,
0 rows matched. Fixed: use .hex, matching exactly what the ORM itself writes.
BEGIN IMMEDIATE applies to every transaction, reads included — a long-lived test fixture
that autobegins a transaction via a bare read and never explicitly closes it holds the
exclusive write lock for the rest of the test; a later scoped_session() call then fails with
"database is locked". Not an app-code bug (every real session block closes cleanly on exit) but
documented since the next test against the db_auth fixture will hit it too.
Python's sqlite3 module deprecates its own implicit datetime adapter as of 3.12 — silent
today, warns on every raw-SQL datetime bind. Confirmed it only ever hit test fixture code (ran
the ORM-only health test with warnings-as-errors and it stayed clean) and fixed there with an
explicit .isoformat() rather than leaving it for a future Python version to turn into a real
failure.
Also: .with_for_update() silently no-ops on SQLite (confirmed — SQLAlchemy emits no SQL for it,
no error either), so it's removed from register()'s invite-redemption query, and the comment now
correctly attributes the concurrency guarantee to BEGIN IMMEDIATE, where it actually lives.
Invariants
Raw ingested bytes remain immutable; derived tables stay rebuildable — n/a, no ingestion yet
No stored odometer added; wear still derived from installs — n/a, no components yet
Physical quantities stored as SI integers — unaffected by this change
New user-owned tables have user_id + repository-layer scope — this is the invariant this
whole PR is about; see invariant #4's revised wording in CLAUDE.md and the two isolation
tests in tests/test_auth.py
No secret can reach a response model, log line, or error message — unaffected
One PR, not several, for the same reason PR #2 was: the migration, the models, db.py, and
the docs recording why are five views of one decision — splitting them wouldn't make review
easier, just disconnected. 552 insertions / 548 deletions across 17 files, most of it
unavoidably touching what PR #2 shipped rather than net-new code.
Deliberately deferred, not solved here (none of these tables exist yet, so none of it is
currently broken — docs/DECISIONS.md D15 records exactly what each future phase needs to
decide): PostGIS's replacement for spatial storage (Phase 1/3), procrastinate's replacement
for background jobs (Phase 1), and EXCLUDE USING gist's replacement for enforcing component_installs' "one place at a time" invariant (Phase 2).
PR #4 (the deploy pipeline built for the old 3-container Postgres compose stack) was closed
as superseded rather than merged. The single-container image build (Caddy + API via a process
supervisor, SQLite file on a persistent volume) is real follow-up work this PR does not include.
## What and why
Reverses a shipped, tested, merged decision (D4, PR #2) rather than building on it — see
`docs/DECISIONS.md` D15 for the full record. The user asked for a single-container deploy; when
that meant Postgres either stayed a second container or got bundled inside the single one via a
process supervisor, they chose SQLite instead. I raised the concrete costs before building this
(no database-level RLS, no PostGIS, `procrastinate` needs replacing) and they reaffirmed — that's
their call to make about their own self-hosted instance, and this PR is the honest execution of it,
not a token driver swap.
**The one invariant-critical consequence:** isolation between users now rests entirely on the
repository-layer scope (`db.py`'s `Scope.select()`), not two layers. `CLAUDE.md`'s invariant #4 is
revised accordingly. This is not a downgrade-and-hope — `Scope` is built so an unfiltered query
against a user-owned table is structurally harder to write than a scoped one (there is no method
on `Scope` that returns one).
## How this was verified
- `ruff check`, `ruff format --check`, `mypy --strict` — all clean
- `pytest` — 13/13 passing against a real SQLite file, **including with `DeprecationWarning`
promoted to an error** (confirms a fix actually holds, not just that it's quiet by default)
- Full `alembic upgrade head -> downgrade -1 -> upgrade head` cycle
- `alembic check` — clean, with **no `include_object` filter needed at all now** (SQLite starts
with nothing but what our own migrations create — no PostGIS/TIGER noise to filter, unlike the
Postgres version)
- CI's exact migration command sequence reproduced locally, end to end, before touching the
workflow file
- [x] CI is green (pending this push)
- [x] Tests added for the behaviour that changed — including a deliberately alarming one
(`test_unscoped_session_can_see_every_user_when_misused`) that shows exactly what a reviewer
must now catch, since nothing else will
- [x] Verified manually — six real SQLite behaviours below were each confirmed empirically with a
minimal standalone repro before being written into the actual fix, not assumed from docs
## Six real bugs found by actually running this against real SQLite
Each one below was reproduced in isolation first, then fixed, then re-verified — not assumed:
1. **Foreign keys (incl. `ON DELETE CASCADE`) are OFF by default per connection.** Deleting a user
silently left orphaned `sessions`/`api_tokens` rows, no error either way. Fixed:
`PRAGMA foreign_keys=ON` on every connect.
2. **Transactions default to DEFERRED**, taking a write lock only on the first actual write — a
real check-then-act race on invite redemption (two concurrent redemptions could both read
`used_count < max_uses` as true before either commits). Fixed: `BEGIN IMMEDIATE` on every
transaction — SQLAlchemy's own documented recipe for this, not improvised.
3. **`DateTime(timezone=True)` does not round-trip tzinfo on SQLite** — a tz-aware datetime goes
in, a naive one comes out, and every `expires_at < datetime.now(UTC)` comparison in
`auth/service.py` then raises `TypeError`. Fixed once at the `Base` level with a `UTCDateTime`
`TypeDecorator`, not per-column.
4. **`Uuid(as_uuid=True)` stores as 32-char hex with no hyphens**, not `str(uuid)`'s form. A test
fixture that raw-inserted the hyphenated form left rows the ORM's own later `UPDATE` (via
`invite.used_count += 1`'s autoflush) could never match by primary key — `StaleDataError`,
0 rows matched. Fixed: use `.hex`, matching exactly what the ORM itself writes.
5. **`BEGIN IMMEDIATE` applies to every transaction, reads included** — a long-lived test fixture
that autobegins a transaction via a bare read and never explicitly closes it holds the
exclusive write lock for the rest of the test; a later `scoped_session()` call then fails with
"database is locked". Not an app-code bug (every real session block closes cleanly on exit) but
documented since the next test against the `db_auth` fixture will hit it too.
6. **Python's `sqlite3` module deprecates its own implicit datetime adapter as of 3.12** — silent
today, warns on every raw-SQL datetime bind. Confirmed it only ever hit test fixture code (ran
the ORM-only health test with warnings-as-errors and it stayed clean) and fixed there with an
explicit `.isoformat()` rather than leaving it for a future Python version to turn into a real
failure.
Also: `.with_for_update()` silently no-ops on SQLite (confirmed — SQLAlchemy emits no SQL for it,
no error either), so it's removed from `register()`'s invite-redemption query, and the comment now
correctly attributes the concurrency guarantee to `BEGIN IMMEDIATE`, where it actually lives.
## Invariants
- [x] Raw ingested bytes remain immutable; derived tables stay rebuildable — n/a, no ingestion yet
- [x] No stored odometer added; wear still derived from installs — n/a, no components yet
- [x] Physical quantities stored as SI integers — unaffected by this change
- [x] New user-owned tables have `user_id` + repository-layer scope — **this is the invariant this
whole PR is about**; see invariant #4's revised wording in `CLAUDE.md` and the two isolation
tests in `tests/test_auth.py`
- [x] No secret can reach a response model, log line, or error message — unaffected
- [x] Migration survives `upgrade -> downgrade -1 -> upgrade` — verified above
## Risks and follow-ups
- **One PR, not several**, for the same reason PR #2 was: the migration, the models, `db.py`, and
the docs recording why are five views of one decision — splitting them wouldn't make review
easier, just disconnected. 552 insertions / 548 deletions across 17 files, most of it
unavoidably touching what PR #2 shipped rather than net-new code.
- **Deliberately deferred, not solved here** (none of these tables exist yet, so none of it is
currently broken — `docs/DECISIONS.md` D15 records exactly what each future phase needs to
decide): PostGIS's replacement for spatial storage (Phase 1/3), `procrastinate`'s replacement
for background jobs (Phase 1), and `EXCLUDE USING gist`'s replacement for enforcing
`component_installs`' "one place at a time" invariant (Phase 2).
- **PR #4** (the deploy pipeline built for the old 3-container Postgres compose stack) was closed
as superseded rather than merged. The single-container image build (Caddy + API via a process
supervisor, SQLite file on a persistent volume) is real follow-up work this PR does not include.
Reverses a shipped, tested, merged decision (D4/PR #2) rather than building
on it — see docs/DECISIONS.md D15 for the full record: what was rejected
(Postgres as a second container; Postgres+PostGIS bundled inside the single
container via a supervisor), what this costs (no database-level RLS, no
PostGIS, procrastinate needs replacing — all stated as a concern before this
was decided, and reaffirmed anyway, which is the user's call to make about
their own instance).
The one invariant-critical consequence: isolation between users now rests
entirely on the repository-layer scope (db.py's `Scope.select()`), not two
layers. CLAUDE.md's invariant #4 is revised accordingly. This is not a
downgrade-and-hope — `Scope` is built so an unfiltered query against a
user-owned table is structurally harder to write than a scoped one (there is
no method on `Scope` that returns one), and
tests/test_auth.py::test_scoped_session_blocks_cross_user_reads replaces the
old RLS proof with the same empirical standard: it doesn't trust the query
builder filters correctly because the code reads correctly, it registers two
real users and checks.
test_unscoped_session_can_see_every_user_when_misused is the deliberately
alarming companion — it demonstrates exactly what a reviewer must now catch,
since nothing else will.
Six real, non-obvious SQLite behaviours found and fixed by actually running
this against a real file, not assumed from docs:
- Foreign keys, ON DELETE CASCADE included, are OFF by default per
connection — deleting a user silently left orphaned sessions/api_tokens,
no error either way. Fixed with PRAGMA foreign_keys=ON on every connect.
- Transactions default to DEFERRED, which only takes a write lock on the
first actual write — a real check-then-act race for invite redemption
(two concurrent redemptions could both read used_count < max_uses as true
before either commits). Fixed by disabling the driver's implicit BEGIN and
issuing BEGIN IMMEDIATE ourselves — SQLAlchemy's own documented recipe for
this, not improvised.
- DateTime(timezone=True) does NOT round-trip tzinfo on SQLite — a tz-aware
datetime goes in, a naive one comes back out, and every
`expires_at < datetime.now(UTC)` comparison in auth/service.py then raises
TypeError. Fixed once at the Base level with a UTCDateTime TypeDecorator
rather than per-column.
- Uuid(as_uuid=True) stores as 32-char hex with NO hyphens on SQLite, not
str(uuid)'s hyphenated form. A test fixture that raw-inserted the
hyphenated form left rows the ORM's own later UPDATE (via
invite.used_count += 1's autoflush) could never match by primary key,
updating zero rows and raising StaleDataError. Fixed by using .hex to
match exactly what the ORM itself writes.
- BEGIN IMMEDIATE applies to every transaction, reads included — a
long-lived test fixture that autobegins a transaction via a bare read and
never explicitly closes it holds SQLite's exclusive write lock for the
rest of the test, and a later scoped_session() call fails with "database
is locked". Not an app-code bug (every real session block closes cleanly
on exit), but real enough to document since the next person writing a
test against the db_auth fixture will hit it too.
- Python's sqlite3 module deprecates its own implicit datetime adapter as of
3.12 — silent today, warns on every raw-SQL datetime bind. Only ever hit
test fixture code (the ORM path never uses it, confirmed by running the
ORM-only health test with warnings promoted to errors and it stayed
clean); fixed there with an explicit .isoformat() rather than left for a
future Python version to turn into a real failure.
Also, since with_for_update() silently no-ops on SQLite (confirmed —
SQLAlchemy emits no SQL for it, no error either) rather than actually
locking anything: removed it from register()'s invite-redemption query and
corrected the comment to attribute the concurrency guarantee to BEGIN
IMMEDIATE, where it now actually lives.
One PR, not several, for the same reason PR #2 was: the migration, the
models, db.py, and the docs recording why are five views of one decision —
splitting them wouldn't make review easier, just disconnected. 552
insertions / 548 deletions across 17 files, most of it necessarily touching
what PR #2 shipped rather than net-new code.
Deliberately deferred, not solved here: PostGIS's replacement for spatial
storage, procrastinate's replacement for background jobs, and the
EXCLUDE USING gist constraint's replacement for component_installs — none
of those tables exist yet (Phase 1-2), so none of it is broken, and
docs/DECISIONS.md D15 records exactly what each future phase needs to
decide before it can be built. .gitea/workflows/deploy pipeline (PR #4,
built for the old 3-container Postgres compose stack) was closed as
superseded rather than merged; the single-container image build is
follow-up work, not part of this change.
Verified: ruff check, ruff format --check, and mypy --strict all clean.
13/13 pytest passing against a real SQLite file, including with
DeprecationWarning promoted to an error (confirms the sqlite3 adapter
deprecation fix actually holds, not just that it's quiet by default). Full
alembic upgrade -> downgrade -1 -> upgrade cycle run clean. alembic check
clean with no include_object filter needed at all now (SQLite starts with
nothing but what our own migrations create — no PostGIS/TIGER noise to
filter out in the first place). CI's exact migration command sequence
reproduced locally end to end before touching the workflow file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BBergle
merged commit 9fa50cb2ea into main2026-09-21 15:44:55 -04:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What and why
Reverses a shipped, tested, merged decision (D4, PR #2) rather than building on it — see
docs/DECISIONS.mdD15 for the full record. The user asked for a single-container deploy; whenthat meant Postgres either stayed a second container or got bundled inside the single one via a
process supervisor, they chose SQLite instead. I raised the concrete costs before building this
(no database-level RLS, no PostGIS,
procrastinateneeds replacing) and they reaffirmed — that'stheir call to make about their own self-hosted instance, and this PR is the honest execution of it,
not a token driver swap.
The one invariant-critical consequence: isolation between users now rests entirely on the
repository-layer scope (
db.py'sScope.select()), not two layers.CLAUDE.md's invariant #4 isrevised accordingly. This is not a downgrade-and-hope —
Scopeis built so an unfiltered queryagainst a user-owned table is structurally harder to write than a scoped one (there is no method
on
Scopethat returns one).How this was verified
ruff check,ruff format --check,mypy --strict— all cleanpytest— 13/13 passing against a real SQLite file, including withDeprecationWarningpromoted to an error (confirms a fix actually holds, not just that it's quiet by default)
Full
alembic upgrade head -> downgrade -1 -> upgrade headcyclealembic check— clean, with noinclude_objectfilter needed at all now (SQLite startswith nothing but what our own migrations create — no PostGIS/TIGER noise to filter, unlike the
Postgres version)
CI's exact migration command sequence reproduced locally, end to end, before touching the
workflow file
CI is green (pending this push)
Tests added for the behaviour that changed — including a deliberately alarming one
(
test_unscoped_session_can_see_every_user_when_misused) that shows exactly what a reviewermust now catch, since nothing else will
Verified manually — six real SQLite behaviours below were each confirmed empirically with a
minimal standalone repro before being written into the actual fix, not assumed from docs
Six real bugs found by actually running this against real SQLite
Each one below was reproduced in isolation first, then fixed, then re-verified — not assumed:
ON DELETE CASCADE) are OFF by default per connection. Deleting a usersilently left orphaned
sessions/api_tokensrows, no error either way. Fixed:PRAGMA foreign_keys=ONon every connect.real check-then-act race on invite redemption (two concurrent redemptions could both read
used_count < max_usesas true before either commits). Fixed:BEGIN IMMEDIATEon everytransaction — SQLAlchemy's own documented recipe for this, not improvised.
DateTime(timezone=True)does not round-trip tzinfo on SQLite — a tz-aware datetime goesin, a naive one comes out, and every
expires_at < datetime.now(UTC)comparison inauth/service.pythen raisesTypeError. Fixed once at theBaselevel with aUTCDateTimeTypeDecorator, not per-column.Uuid(as_uuid=True)stores as 32-char hex with no hyphens, notstr(uuid)'s form. A testfixture that raw-inserted the hyphenated form left rows the ORM's own later
UPDATE(viainvite.used_count += 1's autoflush) could never match by primary key —StaleDataError,0 rows matched. Fixed: use
.hex, matching exactly what the ORM itself writes.BEGIN IMMEDIATEapplies to every transaction, reads included — a long-lived test fixturethat autobegins a transaction via a bare read and never explicitly closes it holds the
exclusive write lock for the rest of the test; a later
scoped_session()call then fails with"database is locked". Not an app-code bug (every real session block closes cleanly on exit) but
documented since the next test against the
db_authfixture will hit it too.sqlite3module deprecates its own implicit datetime adapter as of 3.12 — silenttoday, warns on every raw-SQL datetime bind. Confirmed it only ever hit test fixture code (ran
the ORM-only health test with warnings-as-errors and it stayed clean) and fixed there with an
explicit
.isoformat()rather than leaving it for a future Python version to turn into a realfailure.
Also:
.with_for_update()silently no-ops on SQLite (confirmed — SQLAlchemy emits no SQL for it,no error either), so it's removed from
register()'s invite-redemption query, and the comment nowcorrectly attributes the concurrency guarantee to
BEGIN IMMEDIATE, where it actually lives.Invariants
user_id+ repository-layer scope — this is the invariant thiswhole PR is about; see invariant #4's revised wording in
CLAUDE.mdand the two isolationtests in
tests/test_auth.pyupgrade -> downgrade -1 -> upgrade— verified aboveRisks and follow-ups
db.py, andthe docs recording why are five views of one decision — splitting them wouldn't make review
easier, just disconnected. 552 insertions / 548 deletions across 17 files, most of it
unavoidably touching what PR #2 shipped rather than net-new code.
currently broken —
docs/DECISIONS.mdD15 records exactly what each future phase needs todecide): PostGIS's replacement for spatial storage (Phase 1/3),
procrastinate's replacementfor background jobs (Phase 1), and
EXCLUDE USING gist's replacement for enforcingcomponent_installs' "one place at a time" invariant (Phase 2).as superseded rather than merged. The single-container image build (Caddy + API via a process
supervisor, SQLite file on a persistent volume) is real follow-up work this PR does not include.