Files
bike-app/.gitea/workflows/ci.yml
BBergleandClaude Opus 5 e7392a5723
CI / Repo hygiene (pull_request) Successful in 2s
CI / Web (lint, typecheck, build) (pull_request) Successful in 13s
CI / Migrations reversible (pull_request) Successful in 6s
CI / API (lint, types, tests) (pull_request) Successful in 52s
refactor(api): move from Postgres+RLS to single-engine SQLite
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>
2026-09-21 15:42:44 -04:00

185 lines
5.8 KiB
YAML

name: CI
on:
push:
branches: [main]
pull_request:
workflow_dispatch:
concurrency:
group: ci-${{ gitea.ref }}
cancel-in-progress: true
jobs:
meta:
name: Repo hygiene
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Branch name follows convention
if: gitea.event_name == 'pull_request'
run: |
BRANCH="${{ gitea.head_ref }}"
echo "Branch: $BRANCH"
if echo "$BRANCH" | grep -Eq '^(feat|fix|refactor|test|docs|chore|ci)/[a-z0-9._-]+$'; then
echo "OK"
else
echo "::error::Branch '$BRANCH' does not match <type>/<desc>."
echo "Valid types: feat fix refactor test docs chore ci"
exit 1
fi
- name: No secrets committed
run: |
# Deliberately narrow: high-signal patterns only, so this never cries wolf.
PATTERN='BEGIN (RSA|OPENSSH|EC|PGP) PRIVATE KEY|xoxb-[0-9A-Za-z-]{10,}|AKIA[0-9A-Z]{16}'
if git grep -nIE "$PATTERN" -- . ':!.gitea/workflows/*' ; then
echo "::error::Possible secret committed (see matches above)."
exit 1
fi
echo "No secret patterns found."
- name: No ride data committed
run: |
if git ls-files | grep -E '\.(fit|gpx|tcx)$' | grep -v '^apps/api/tests/fixtures/fit/'; then
echo "::error::Ride files belong in the blob store, not git."
echo "Only apps/api/tests/fixtures/fit/ may contain .fit files (golden fixtures)."
exit 1
fi
echo "Clean."
api:
name: API (lint, types, tests)
runs-on: ubuntu-latest
# No postgres service container — SQLite (docs/DECISIONS.md D15) needs no server to talk to;
# the test suite creates its own temp file (see tests/conftest.py). This also makes the job
# meaningfully faster: no service container to start and health-check before tests can run.
env:
VELODROME_ENVIRONMENT: test
steps:
- uses: actions/checkout@v4
- name: Is there API code yet?
id: guard
run: |
if [ -n "$(find apps/api -name '*.py' -not -path '*/.*' 2>/dev/null | head -1)" ]; then
echo "present=true" >> "$GITHUB_OUTPUT"
else
echo "present=false" >> "$GITHUB_OUTPUT"
echo "No Python sources under apps/api yet — skipping."
fi
- uses: actions/setup-python@v5
if: steps.guard.outputs.present == 'true'
with:
python-version: '3.12'
- name: Cache uv
if: steps.guard.outputs.present == 'true'
uses: actions/cache@v4
with:
path: ~/.cache/uv
key: uv-${{ runner.os }}-${{ hashFiles('apps/api/pyproject.toml') }}
restore-keys: uv-${{ runner.os }}-
- name: Install
if: steps.guard.outputs.present == 'true'
working-directory: apps/api
run: |
curl -LsSf https://astral.sh/uv/install.sh | sh
export PATH="$HOME/.local/bin:$PATH"
uv sync --all-extras
- name: Lint (ruff)
if: steps.guard.outputs.present == 'true'
working-directory: apps/api
run: |
export PATH="$HOME/.local/bin:$PATH"
uv run ruff check .
uv run ruff format --check .
- name: Types (mypy --strict)
if: steps.guard.outputs.present == 'true'
working-directory: apps/api
run: |
export PATH="$HOME/.local/bin:$PATH"
uv run mypy --strict velodrome
- name: Tests
if: steps.guard.outputs.present == 'true'
working-directory: apps/api
run: |
export PATH="$HOME/.local/bin:$PATH"
uv run pytest -q
web:
name: Web (lint, typecheck, build)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Is there web code yet?
id: guard
run: |
if [ -f apps/web/package.json ]; then
echo "present=true" >> "$GITHUB_OUTPUT"
else
echo "present=false" >> "$GITHUB_OUTPUT"
echo "No apps/web/package.json yet — skipping."
fi
- uses: actions/setup-node@v4
if: steps.guard.outputs.present == 'true'
with:
node-version: '22'
- name: Install, lint, build
if: steps.guard.outputs.present == 'true'
working-directory: apps/web
run: |
corepack enable
pnpm install --frozen-lockfile
pnpm run lint
pnpm run check
pnpm run build
migrations:
name: Migrations reversible
runs-on: ubuntu-latest
# No postgres service container — see the api job's comment; same reasoning.
env:
VELODROME_DATABASE_URL: sqlite+aiosqlite:///./velodrome-ci-migrations.db
steps:
- uses: actions/checkout@v4
- name: Are there migrations yet?
id: guard
run: |
if [ -d apps/api/alembic/versions ] && [ -n "$(ls -A apps/api/alembic/versions/*.py 2>/dev/null)" ]; then
echo "present=true" >> "$GITHUB_OUTPUT"
else
echo "present=false" >> "$GITHUB_OUTPUT"
echo "No migrations yet — skipping."
fi
- uses: actions/setup-python@v5
if: steps.guard.outputs.present == 'true'
with:
python-version: '3.12'
- name: upgrade -> downgrade -> upgrade, and check for model drift
if: steps.guard.outputs.present == 'true'
working-directory: apps/api
run: |
curl -LsSf https://astral.sh/uv/install.sh | sh
export PATH="$HOME/.local/bin:$PATH"
uv sync --all-extras
uv run alembic upgrade head
uv run alembic downgrade -1
uv run alembic upgrade head
# Fails if the models have drifted from the migrations.
uv run alembic check