release.yml's first real run failed: docker/login-action against 192.168.0.3:3000 hit "server gave HTTP response to HTTPS client" — Docker refuses any non-localhost registry over plain HTTP by default, so this was never actually a workflow bug. Rejected insecure-registries in daemon.json after reading this Unraid host's own rc.docker script: applying it needs a full dockerd restart, and with Live Restore disabled here, that stops every one of the ~40 other containers on the box first. Also rejected a real Let's Encrypt cert on a public bbergle.com subdomain — this host's other subdomains are Cloudflare-proxied, which would terminate TLS at Cloudflare's edge and never reach our own cert at all. Chosen instead, scoped to touch nothing already working: a self-signed cert for registry.bbergle.com behind a new NPMplus proxy host (found its real HTTPS port, 9537, by reading `docker port NPMplus` rather than assuming 443, which is a different nginx process on this box entirely); an /etc/hosts entry on the Unraid host so only that host needs to resolve the name (no DNS record, no router/NAT dependency); and its CA dropped into /etc/docker/certs.d, which Docker's own docs confirm is read per-connection with no daemon restart required. Also pins buildx to driver: docker instead of setup-buildx-action's default docker-container driver, which runs an isolated builder that doesn't see /etc/docker/certs.d and would have quietly defeated all of the above. Full record, including what was rejected and why, in docs/DECISIONS.md D17. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R2ZKeWkZV7ehf7fivrAkkG
304 lines
22 KiB
Markdown
304 lines
22 KiB
Markdown
# Decisions
|
|
|
|
Settled during the planning session on 2026-09-20. Each entry records what was chosen, what was
|
|
rejected, and why — so we don't relitigate them.
|
|
|
|
---
|
|
|
|
### D1 — Build fresh, don't fork
|
|
**Chosen:** greenfield, borrowing data models only.
|
|
**Rejected:** forking Endurain (AGPL, FastAPI+Vue, already has gear/component tracking, but in a
|
|
feature freeze with a thin maps story) or FitTrackee (AGPL, mature heatmaps, but its "equipment" is
|
|
one flat tag per workout with no wear intervals — the entire parts/maintenance system would be
|
|
bolted on anyway).
|
|
**Why:** nothing existing covers rides + real parts inventory + maintenance + the self-host
|
|
features. Take Endurain's gear/component table structure and strava-gear's retroactive wear
|
|
computation as references; own every line; avoid AGPL entanglement.
|
|
|
|
### D2 — PWA, not a native iOS app
|
|
**Chosen:** SvelteKit static SPA, installed to the iPhone home screen.
|
|
**Rejected:** a native SwiftUI app.
|
|
**Why:** a home-screen PWA gets the icon, standalone display, offline caching, and — since iOS 16.4
|
|
— real push notifications, which was the only thing that used to force native. Native would cost
|
|
$99/yr for an Apple Developer account, TestFlight/sideloading to reach family phones, and a second
|
|
codebase forever. The two real PWA gaps on iOS (Web Bluetooth, Background Sync) are irrelevant here:
|
|
Bryton BLE is a dead end regardless, and the *server* does all syncing.
|
|
**Kept as insurance:** the backend stays strictly API-first with a CI-enforced OpenAPI contract, so
|
|
if Apple ever makes the PWA route untenable, a native client is a code-generation exercise.
|
|
|
|
### D3 — Bryton cloud poller is the primary ingestion path
|
|
**Chosen:** server-side poller against the reverse-engineered Bryton Active API, every 15-20 min.
|
|
**Rejected:** Strava as a source (no `export_original` — decoded smoothed streams only; plus the
|
|
June 2026 tier restructure caps new apps at 10 users and requires a paid dev subscription);
|
|
BLE/ANT-FS direct (nobody has reverse-engineered Bryton's BLE — weeks of work, breaks on firmware
|
|
updates); depending on the Bryton Active phone app (the original complaint).
|
|
**Why:** the Rider 650 has on-device Wi-Fi (`Main Menu -> Data Sync`) and uploads to Bryton's cloud
|
|
with no phone involved, and the cloud API returns the **original unmodified FIT bytes**. That's both
|
|
zero-touch *and* higher fidelity than the current Strava route.
|
|
**Fallbacks, both built:** USB watch folder (also the historical-backfill mechanism, so it stays
|
|
exercised rather than bit-rotting) and manual upload.
|
|
|
|
### D4 — Python / FastAPI / Postgres+PostGIS — **database choice superseded by D15**
|
|
**Chosen:** Python 3.12, FastAPI, Pydantic v2, SQLAlchemy 2.0 async, Alembic, PostgreSQL 16 + PostGIS 3.4.
|
|
**Rejected:** TypeScript full-stack, Go.
|
|
**Why:** `fitdecode` is Python-only, and the Bryton poller reference implementation is Python
|
|
(~200 lines of Meteor DDP to vendor rather than reimplement — DDP over SockJS is fiddlier than it
|
|
looks in JS). FastAPI emits OpenAPI 3.1 for free. PostGIS is needed for heatmaps, bbox queries, and
|
|
self-segment matching. Go has the best raw performance and a fine FIT library but the weakest
|
|
data-analysis ecosystem for the later analytics work.
|
|
|
|
The Python/FastAPI half of this still stands. **The database half — Postgres+PostGIS — was
|
|
replaced with SQLite in D15**, after Phase 0 was already built and merged against Postgres. Kept
|
|
here, marked superseded rather than deleted, so the PostGIS-specific reasoning (heatmaps, bbox
|
|
queries, segment matching) is still visible as context for whatever Phase 3 ends up doing about
|
|
spatial storage without it.
|
|
|
|
### D5 — procrastinate for background jobs — **needs a replacement, see D15**
|
|
**Chosen:** `procrastinate` (Postgres-backed queue).
|
|
**Rejected:** Celery (needs Redis/RabbitMQ, heavyweight, weak async, second-class Postgres broker),
|
|
arq (Redis-only — a whole container for ~50 tasks/day), APScheduler (a scheduler, not a durable
|
|
queue: no retries, no dead-lettering, no multi-worker coordination).
|
|
**Why:** the decisive property is **transactional enqueue** — the `raw_files` INSERT and the parse-job
|
|
enqueue commit atomically on one connection, so there are no orphaned blobs and no jobs pointing at
|
|
rolled-back rows. That's impossible with a Redis broker without inventing an outbox. It also has
|
|
built-in cron, which removes the scheduler container, and keeps job state inside the same `pg_dump`.
|
|
|
|
`procrastinate` is Postgres-only — no SQLite backend exists, so **D15's move to SQLite invalidates
|
|
this choice**. Nothing consumes a job queue yet (no ingestion pipeline exists), so this is a
|
|
deferred decision, not an urgent one: whatever replaces it (APScheduler for a *scheduler*, or a
|
|
hand-rolled `SELECT ... WHERE claimed_at IS NULL LIMIT 1` polling table for real job durability —
|
|
SQLite's single-writer model makes even a crude polling table viable at this app's scale) needs
|
|
picking before Phase 1's ingestion pipeline, not before.
|
|
|
|
### D6 — Opaque bearer tokens, no JWT
|
|
**Chosen:** Argon2id passwords + opaque tokens in a `sessions` table, HttpOnly cookie for the PWA.
|
|
**Rejected:** JWT.
|
|
**Why:** at 5-15 users, verification is one indexed PK lookup (~0.1ms), and you get instant
|
|
revocation, a real device list, and no key-rotation or clock-skew bug class. JWT's only advantage is
|
|
stateless horizontal scale, which will never arrive — choosing it would be a permanent complexity
|
|
tax against a benefit that never materialises.
|
|
|
|
### D7 — Raw bytes are the only truth
|
|
**Chosen:** every ingested file is written to a content-addressed blob store *before* parsing, and is
|
|
never mutated or deleted. All tables are rebuildable projections.
|
|
**Why:** this converts "a parser bug wrote wrong elevation to 4,000 rides" and "reprocess a decade of
|
|
history against a better DEM" from incidents/migrations into routine batch jobs (`parser_version`
|
|
bump + requeue). It is the single most load-bearing rule in the design, and it's also the primary
|
|
data-loss control.
|
|
|
|
### D8 — No odometer column anywhere; wear is derived
|
|
**Chosen:** `component_installs` as a time-ranged association (strava-gear's model, made relational
|
|
with a GIST `EXCLUDE` constraint), with wear computed by replaying the activity stream.
|
|
**Rejected:** a stored running odometer per component.
|
|
**Why:** correcting "I actually swapped that chain a week earlier" becomes one `UPDATE` and every
|
|
downstream number self-corrects. Parts moving between bikes is two rows. A stored counter can do
|
|
neither without a reconciliation nightmare — which is exactly where FitTrackee's flat equipment tag
|
|
falls over in year two.
|
|
|
|
The time-ranged-association *model* doesn't depend on Postgres. The `EXCLUDE USING gist` constraint
|
|
enforcing "a component is in exactly one place at a time" does — SQLite has no range types and no
|
|
exclusion constraints. `component_installs` doesn't exist yet (Phase 2), so this is another
|
|
deferred casualty of D15, not an active one: the same invariant will need enforcing at the
|
|
application layer (check-then-insert inside a transaction) instead of the database refusing an
|
|
overlapping row outright.
|
|
|
|
### D9 — Streams as columnar int32 arrays
|
|
**Chosen:** one row per channel per activity, `values_i32[]` with a scale factor.
|
|
**Rejected:** a normalized per-sample table (~5x larger with index, and every real query wants the
|
|
whole stream anyway), JSONB (untyped, 3-5x larger, slow to deserialise), TimescaleDB (solves
|
|
cross-entity firehose scans; we do per-entity blob reads — and it would mean abandoning the
|
|
`postgis/postgis` base image and taking on extension-version coupling at every Postgres upgrade).
|
|
**Bonus:** FIT stores position as int32 semicircles, so lat/lon are lossless and free in this encoding.
|
|
Aggregates are precomputed at ingest into `activity_stats`, never scanned from streams.
|
|
|
|
### D10 — Imperial display units
|
|
**Chosen:** `users.unit_system` defaults to imperial. Storage stays SI integers throughout
|
|
(metres, seconds, mm/s, minor currency units); units are strictly a presentation concern.
|
|
|
|
### D11 — Notification dedupe by cycle sequence
|
|
**Chosen:** `notification_log` with `UNIQUE (user_id, dedupe_key)` where the key is
|
|
`service_due:<rule_id>:<component_id>:<cycle_seq>:<threshold>` and `cycle_seq` counts service
|
|
events logged against that (component, rule).
|
|
**Why:** a naive nightly evaluator nags you about the same chain every night until you fix it, and
|
|
you learn to ignore it. This fires exactly once at 80%, once at 100%, then goes quiet; logging the
|
|
service increments the cycle and re-arms the next 200 miles. It's how recurrence works without a
|
|
cron-style recurrence engine.
|
|
|
|
### D12 — ntfy first, Web Push second
|
|
**Chosen:** `apprise` -> ntfy as the primary notification channel; Web Push (VAPID/`pywebpush`) as
|
|
the nicer layer on top; every notification is also an in-app inbox row.
|
|
**Why:** `apprise` is already in the stack for poller alerts and works on iOS with no PWA-install
|
|
requirement, so notifications can ship early. Apple's `web.push.apple.com` does speak standard RFC
|
|
8291 (no Apple Developer account needed), but only for home-screen-installed PWAs, and iOS silently
|
|
drops subscriptions after OS updates. Push must never be the only path to the information.
|
|
|
|
### D13 — Monorepo
|
|
**Chosen:** one repo for API + web + deploy + workflows.
|
|
**Why:** one maintainer, and API and client change together constantly. D2 removed the only real
|
|
argument for splitting (a native app would have needed macOS runners that a Linux act_runner can't
|
|
provide) — now every artefact builds on the same runner.
|
|
|
|
### D14 — Research agent models
|
|
**Chosen:** Opus for the Bryton protocol research and the architecture design (ambiguous,
|
|
reverse-engineering, synthesis-heavy); Sonnet for the two breadth surveys (existing self-hosted
|
|
apps, Gitea CI patterns) where material is well-documented and the work is volume.
|
|
|
|
### D15 — SQLite, single container, no database-level RLS
|
|
**Chosen:** SQLite as the database, and the whole app (Caddy + API, static web build baked in) as
|
|
a single container. Made explicitly *after* Phase 0 was already built, tested, and merged against
|
|
Postgres+PostGIS with a two-role RLS architecture (D4, and the `velodrome_app`/`velodrome_auth`
|
|
split in `apps/api/velodrome/db.py`) — this is a deliberate reversal of a shipped decision, not a
|
|
greenfield choice, and it was made with the costs stated plainly first.
|
|
|
|
**Rejected, with reasons on the record:** keeping Postgres as a second container (rejected —
|
|
explicitly wanted exactly one container total); bundling Postgres+PostGIS *inside* the single
|
|
container via a process supervisor (offered as the way to get "one container" without losing RLS
|
|
or PostGIS — rejected in favour of SQLite specifically).
|
|
|
|
**What this costs, stated once here rather than re-litigated every time it's felt:**
|
|
- **Row-level security is gone.** SQLite has no roles, no session variables, no policy engine —
|
|
there is no database-enforced layer left, only the repository-layer scope. CLAUDE.md's
|
|
invariant #4 is revised accordingly (see the file) to describe app-layer scoping as the sole
|
|
mechanism rather than one of two layers. The user isolation test in `tests/test_auth.py` that
|
|
used to prove RLS itself now proves the repository-layer scope does the same job in its
|
|
absence — read it before touching any query that filters by `user_id`.
|
|
- **PostGIS is gone.** No native geometry columns, no GIST spatial indexes, no `ST_Envelope`.
|
|
Nothing in the schema uses it yet (Phase 0 has no `activities` table), so this is a live
|
|
decision for Phase 1/3 to make, not a retrofit — options include SpatiaLite, or storing tracks
|
|
as GeoJSON/WKB in a `TEXT`/`BLOB` column with spatial math done in application code.
|
|
- **`procrastinate` is gone** (D5) — Postgres-only, no SQLite backend. Also nothing consumes it
|
|
yet; a replacement gets picked before Phase 1's ingestion pipeline needs one, not now.
|
|
- **The `EXCLUDE USING gist` constraint design for `component_installs`** (D8) — doesn't exist
|
|
yet either (Phase 2); the "one place at a time" invariant will need application-layer
|
|
enforcement instead of the database refusing an overlapping row outright.
|
|
|
|
**Why proceed anyway:** raised as a concern in-session, with each cost above stated before this
|
|
decision was made; the user heard the full list and confirmed SQLite regardless. That's their call
|
|
to make about their own single-user/family-scale instance, not an oversight to correct for them.
|
|
|
|
**What's unchanged:** invariants #1 (raw bytes immutable), #3 (SI integers in storage), #5 (secret
|
|
containment), #6 (single ingestion path) — none of those were ever Postgres-specific. Auth design
|
|
(D6, opaque bearer tokens) is unaffected. FastAPI/SQLAlchemy/Alembic stay exactly as chosen in D4;
|
|
only the database engine underneath them changed.
|
|
|
|
### D16 — Single-container packaging: entrypoint migrations, tini + a two-line supervisor, Caddy binary copy, Unraid template
|
|
|
|
**Chosen:** one Docker image (root `Dockerfile`), built by copying the SvelteKit static build and
|
|
the API's venv into a runtime stage alongside a copied-out `caddy` binary. `deploy/entrypoint.sh`
|
|
runs `alembic upgrade head`, then starts uvicorn (loopback-only) and Caddy as two background
|
|
processes under `tini` as PID 1, and kills+exits if either one dies. Config surfaces as env vars
|
|
read by the existing `VELODROME_`-prefixed Pydantic settings; `deploy/unraid-template.xml` exposes
|
|
the required ones as Unraid Community Applications web UI fields instead of a `.env` file.
|
|
|
|
**Why not a real process manager (s6-overlay, supervisord):** two long-running processes with no
|
|
dependency graph between them (Caddy doesn't need to wait on uvicorn — it just proxies) doesn't
|
|
need a supervisor with restart policies, readiness ordering, or log multiplexing. A ~20-line bash
|
|
script under `tini` (for correct signal forwarding and zombie reaping, which a bare shell script as
|
|
PID 1 doesn't do) gets the one property that matters — if either process dies, the whole container
|
|
exits non-zero so Docker/Unraid restarts it — without a new dependency or a config format to learn.
|
|
Revisit if a third long-running process gets added later; two is the reasonable ceiling for "just
|
|
write the script."
|
|
|
|
**Why migrations run from the entrypoint, contradicting what apps/api/Dockerfile's own comment
|
|
used to say** ("Migrations run as an explicit step before this in deploy.yml... never from the
|
|
entrypoint, so a failed migration fails the deploy visibly instead of crash-looping here"): that
|
|
comment described the 3-container Postgres plan, where a separate `run --rm api alembic upgrade
|
|
head` step existed *before* `compose up -d`. A single container has nowhere else to put that step.
|
|
The property it was protecting — a failed migration must be visible, not silently served — still
|
|
holds: `set -e` means the script exits non-zero on migration failure, so the container never starts
|
|
serving traffic and shows as exited/restarting in `docker ps`/Unraid, which is the same visibility
|
|
by a different mechanism. What's genuinely lost is the *old* mechanism's failure mode of "the
|
|
previous version keeps running while the bad migration is investigated" — a single container that
|
|
fails to start migrations has no previous version still up. Acceptable for a single-instance
|
|
home-lab deployment; would need reconsidering (e.g. a blue/green swap) if this ever needed
|
|
zero-downtime deploys.
|
|
|
|
**Why the Caddy binary is copied from `caddy:2` rather than using a Caddy base image:** the runtime
|
|
needs both Python (for uvicorn) and Caddy; picking either official base image as the starting
|
|
point means installing the other stack into it by hand. Caddy's official images are a single
|
|
statically-linked Go binary with no CGO, so `COPY --from=caddy:2 /usr/bin/caddy /usr/bin/caddy`
|
|
into a `python:3.12-slim` base is the documented, standard way to get both without a second
|
|
package manager or a source build.
|
|
|
|
**Why an Unraid template file, not just documentation:** the earlier decision (in-session) was to
|
|
move configuration out of a `.env` file and into fields the Unraid web UI can fill in — a plain env
|
|
var table in a README doesn't do that by itself, since Unraid still needs a `Config`-tagged XML
|
|
entry per field to render one. `deploy/unraid-template.xml` is that; every field stays hand-editable
|
|
in the UI afterward regardless of what the template pre-fills, so getting a default slightly wrong
|
|
here isn't load-bearing.
|
|
|
|
**What this doesn't do:** `.gitea/workflows/release.yml` builds and pushes the image to the Gitea
|
|
registry; it does not SSH into the Unraid host and recreate the running container. Rolling a new
|
|
image out is a manual/Unraid-side action (pull + Apply, or Unraid's own update check), not
|
|
something CI does unattended — consistent with treating "affects a shared, already-running system"
|
|
as something a human triggers, not automation.
|
|
|
|
### D17 — Registry TLS: self-signed cert behind NPMplus, not `insecure-registries`, not a real domain
|
|
|
|
**Problem:** `release.yml`'s first real run failed — `docker/login-action` against
|
|
`192.168.0.3:3000` (Gitea's plain-HTTP address) hit `server gave HTTP response to HTTPS client`.
|
|
Docker refuses TLS-less registries by default; this was never a workflow misconfiguration, it's
|
|
expected Docker behaviour for any non-localhost registry.
|
|
|
|
**Rejected: `insecure-registries` in `daemon.json`.** The obvious fix. Rejected after actually
|
|
reading `/etc/rc.d/rc.docker` on the Unraid host rather than assuming: applying a `daemon.json`
|
|
change requires a full `dockerd` restart, and (with `Live Restore` disabled on this host) both
|
|
Unraid's own restart path *and* a raw `kill` of `dockerd` stop every one of the ~40 other
|
|
containers running on the box first, as part of the restart/shutdown sequence — Plex, Home
|
|
Assistant, Vaultwarden, everything. Correct fix for the narrow problem, unacceptable blast radius
|
|
for this specific host.
|
|
|
|
**Rejected: a real Let's Encrypt cert on a new `bbergle.com` subdomain routed publicly.** The
|
|
user's other NPMplus-fronted subdomains resolve through Cloudflare's proxy (orange-cloud), not
|
|
directly to the home IP. A Cloudflare-proxied hostname would have terminated TLS at Cloudflare's
|
|
edge with Cloudflare's own cert, never reaching our self-signed cert or NPMplus's own TLS
|
|
config at all — the entire trust chain would depend on Cloudflare's origin SSL mode, and likely on
|
|
firewall rules restricting port 443 to Cloudflare's IP ranges, neither of which this problem
|
|
needed to involve.
|
|
|
|
**Chosen:** a small, fully self-contained fix, scoped to touch nothing already working:
|
|
- A 10-year self-signed cert for `registry.bbergle.com` (SAN-only, no real domain dependency).
|
|
- An NPMplus proxy host (`registry.bbergle.com` -> `192.168.0.3:3000` over plain HTTP internally)
|
|
terminating TLS with that cert, on NPMplus's existing HTTPS port (`9537` on this host — found by
|
|
reading `docker port NPMplus` rather than assuming 443, which is a *different* nginx process on
|
|
this box entirely).
|
|
- `/etc/hosts` on the Unraid host mapping `registry.bbergle.com` -> `192.168.0.103` (itself) —
|
|
chosen over a real DNS record specifically because the only client that ever needs to resolve
|
|
this hostname is the Unraid host's own `dockerd` (Gitea Actions runs in DooD mode against that
|
|
same host's Docker socket). This sidesteps Cloudflare, the router's NAT/hairpin behaviour, and
|
|
any port-forwarding question entirely — verified separately that hairpin NAT works by default on
|
|
this user's UniFi gateway, but it turned out to be unnecessary for this fix regardless.
|
|
- `/etc/docker/certs.d/registry.bbergle.com:9537/ca.crt` on the Unraid host, trusting that cert for
|
|
that host:port specifically. Confirmed (Docker's own docs) that `certs.d` is read per-connection,
|
|
not baked in at daemon start — no `dockerd` restart, no impact on any other container.
|
|
- `docker/setup-buildx-action@v3` pinned to `driver: docker` in `release.yml` instead of its
|
|
default `docker-container` driver — the default runs BuildKit in an isolated builder container
|
|
that does not see the host's `/etc/docker/certs.d`, which would have silently defeated the whole
|
|
point of the trust setup above. We don't build multi-platform images, so nothing the
|
|
`docker-container` driver offers is actually needed here.
|
|
|
|
**Not persisted across a reboot, deliberately, for now:** neither the `/etc/hosts` line nor the
|
|
`certs.d` file are wired into `/boot/config/go` — both live under `/`, which Unraid rebuilds fresh
|
|
from `/boot` on every boot. Raised explicitly rather than assumed: the user was (rightly) wary of
|
|
hand-editing anything under `/boot` after an earlier, unrelated discussion of what a broken `go`
|
|
script could do to boot. Persisting this is a five-minute follow-up (append two lines to `go`) once
|
|
they're ready to make that call deliberately, not bundled into this fix.
|
|
|
|
**What's unaffected:** Gitea's own web UI, git remote, and API — all still plain
|
|
`http://192.168.0.3:3000`, exactly as CLAUDE.md documents. NPMplus's existing public proxy hosts
|
|
and certs (`vaultwarden.bbergle.com` etc.) — untouched, new proxy host only. No other container on
|
|
the Unraid host was restarted, reconfigured, or otherwise touched to make this work.
|
|
|
|
---
|
|
|
|
## Deliberately deferred
|
|
|
|
- **Routing** (Valhalla/Photon/Overpass) — Phase 5, optional. Several GB of RAM for something
|
|
Komoot already does well.
|
|
- **Local LLM ride summaries** (Ollama) — Phase 4, behind a compose profile.
|
|
- **Friends/family cross-visibility** — Phase 4, as an *additive* widening of the repository-layer
|
|
scope (an RLS policy pre-D15; see D15 for why that's no longer the mechanism), never as removal
|
|
of the default per-user scope.
|
|
- **Legacy Bryton format support** — out of scope entirely. The Rider 650 writes `.fit`.
|
|
- **Reverse-engineering Bryton's BLE** — explicitly rejected. See RESEARCH.md §1.
|