fin/.claude/skills/fin-change-control/SKILL.md

name, description

name

description

fin-change-control

Change-control rulebook for the fin repo — how to classify a change (trivial / behavior / schema / accounting-logic / deploy-affecting), which gate each class requires, the non-negotiable invariants with the historical incident behind each, the prod-DB-is-sacred rule, Alembic migration discipline, the spec→plan workflow in docs/superpowers, docs-of-record house style, and the done-checklist. Load BEFORE making any code, schema, or accounting change to fin; before writing or reviewing an Alembic migration; before touching crud.py transfer/settlement/holding/spending logic; before anything that alters Dockerfiles, docker-compose.yml, or env vars; and when writing specs, plans, commit messages, or updating CLAUDE.md/README.md.

fin change control

How changes to fin are classified, gated, reviewed, and documented. fin is a single-user personal-finance app whose production SQLite database holds the owner's real financial history — the entire process below exists to protect that ledger. Read /CLAUDE.md (rulebook) and /PLAN.md (original design) alongside this skill; nothing here overrides CLAUDE.md.

All file paths are repo-relative. Line numbers and the migration head are correct as of 2026-07-05 — re-verify with the commands in "Provenance and maintenance" if time has passed.

When NOT to use this skill

You actually want to…	Use instead
Query/record finances via the fin CLI	fin (.claude/skills/fin/SKILL.md)
Debug a live bug	fin-debugging-playbook
Read the full story behind past incidents	fin-failure-archaeology
Understand layering/architecture rules in depth	fin-architecture-contract
Understand the double-entry/Splitwise accounting model	fin-accounting-reference
Build images / set up an environment	fin-build-and-env
Deploy, operate, back up on the homelab	fin-run-and-operate
Run tests / QA a change hands-on	fin-validation-and-qa
Write new API tests (the coverage campaign)	fin-test-coverage-campaign
Analyze ledger data	fin-ledger-analysis-toolkit

This skill is the gatekeeper: it tells you what a change must satisfy before it counts as done.

1. Change classification

Classify every change before writing code. A change belongs to the highest matching row (a schema change that alters spending math is an accounting-logic change).

Class	Examples	Required gate
Trivial fix	Typo, label text, log message, comment, dead-code removal	Compile/build check (§7 items 1 & 3). Confirm no invariant file (crud.py, net_worth.py, migrations) was touched.
Behavior change	New endpoint, new QuickAdd flow, new CLI command, changed validation	Trivial gate + pytest cli/tests green + a test covering the new behavior + README/CLAUDE.md update if a convention or config var changed.
Schema change	New column/table/index, retyped column	Behavior gate + Alembic migration per §4 (autogenerated, reviewed, backward-safe) + scratch-DB alembic upgrade head dry-run + spec/plan (§5) if it also spans the frontend.
Accounting-logic change	Anything touching balances, spending/income math, holdings recompute, net-worth calc, transfer/settlement legs (api/app/crud.py, api/app/services/net_worth.py)	Schema gate (if schema involved) + written spec in docs/superpowers/specs/ + re-verify invariants 1, 2, 5, 6 in §3 + if the meaning of existing rows changes, a data migration retyping history (precedent: h8c9d0e1f2a3, §4.4).
Deploy-affecting change	api/Dockerfile, web/Dockerfile, docker-compose.yml, web/nginx.conf, any migration, new env var	Relevant gate above + prod DB backup before rollout (§2.3) + README "Environment / Config" table updated for new vars + remember: the api container runs alembic upgrade head automatically on start (§2.4).

"Gate" means: do not declare the change done, and do not deploy, until every item passes.

2. Non-negotiable zero: the production database is SACRED

The prod DB is the SQLite file /data/fin.db inside the Docker named volume sqlite_data on the homelab (see docker-compose.yml; DATABASE_URL=sqlite:////data/fin.db). It contains real, irreplaceable financial history. There is no upstream to re-sync from.

2.1 Never test against it

• Tests use throwaway databases only. cli/tests/conftest.py creates a temp-dir SQLite file and sets DATABASE_URL before importing the app — keep that pattern for all future tests.
• Never point a local checkout, a REPL, alembic, or the fin CLI's write commands at the prod volume "just to try something".
• Analytical reads against a copy of the DB are fine (see fin-ledger-analysis-toolkit); take the copy with the backup command below.

2.2 Backward-safe migrations only

• New columns must be nullable or carry server_default so existing rows stay valid (precedent: f6a7b8c9d0e1 adds categories.type with server_default='expense'; design rationale in docs/superpowers/specs/2026-07-01-category-types-receivable-redesign.md).
• No destructive column/table drops without a copy-forward plan (write the data somewhere first, verify, then drop in a later migration).
• Data migrations that rewrite rows must be surgical and documented in the migration docstring; if the downgrade cannot restore the data, say so explicitly in downgrade() (precedent: h8c9d0e1f2a3).
• SQLite cannot ALTER COLUMN/DROP COLUMN freely — use Alembic batch_alter_table when a migration needs more than ADD COLUMN.

2.3 Back up before any risky operation

Before deploying a migration or running any manual DB surgery, snapshot the prod DB. (Canonical backup/restore runbook: fin-run-and-operate §3 — it uses the equivalent Python sqlite3.backup() API; keep the two in sync.) VACUUM INTO produces a consistent single-file copy even under WAL, and the python:3.12-slim image bundles a new-enough SQLite (≥ 3.27). Run on the homelab host (verified statically against docker-compose.yml and api/Dockerfile — no docker in the authoring sandbox):

# 1. Snapshot inside the container (writes into the same volume)
docker compose exec api python -c "import sqlite3; sqlite3.connect('/data/fin.db').execute(\"VACUUM INTO '/data/fin-backup-$(date +%F).db'\")"
# 2. Copy the snapshot off the container
docker compose cp api:/data/fin-backup-$(date +%F).db ./

Cold-backup alternative (stack stopped): docker run --rm -v <project>_sqlite_data:/data -v "$PWD":/out alpine tar czf /out/fin-db-backup.tgz -C /data . — note the volume's real name carries the compose project prefix; find it with docker volume ls | grep sqlite_data.

2.4 Migrations auto-run on container start

api/Dockerfile CMD: python -m app.preflight && alembic upgrade head && uvicorn app.main:app --host 0.0.0.0 --port 8000. Pushing an image with a bad migration to the Harbor registry means it executes against prod on the next restart. That is why migration review (§4) is a hard gate, not a courtesy.

3. The non-negotiables — invariant, rationale, incident, blast radius

Every invariant from CLAUDE.md, each with the commit history that motivated it. git show <hash> for the full story; fin-failure-archaeology for narratives.

3.1 Atomic two-leg writes

• Rule: create_transfer and create_settlement in api/app/crud.py write both legs with db.add_all([...]); db.commit() in one transaction (lines 233/269; the split-expense triple at line 350). Legs are cross-linked via linked_transaction_id. Never create, update, or delete a single leg.
• Rationale: each leg is one side of a double-entry pair; a lone leg silently corrupts two account balances at once.
• Incidents: a06f921 — deletes left orphaned paired rows "corrupting balances"; a1f109b — the account-pair lookup used for paired deletes was ambiguous, so linked_transaction_id was added for precise pairing; a1289f6 / 2bd4730 — API and UI had to block edits on linked legs; 9d06e1b — UI still showed orphan legs after delete; bc256f6 — split expense was two sequential HTTP requests that could partially fail, replaced by the atomic POST /api/transactions/split-expense; e36d719 — delete_transaction now follows linked_transaction_id in both directions and deletes the whole group atomically. The CLI enforces this too: fin tx add refuses transfer/settlement types (082b608).
• If violated: balances on two accounts diverge from the transaction log; net worth is wrong; no error is raised anywhere.

3.2 Holding recompute on delete

• Rule: delete_stock_transaction (crud.py:522) calls recompute_holding_from_stock_transactions after the delete and before db.commit(). Holdings are derived state — never edit them directly.
• Rationale: Holding rows are a cache of the stock_transactions log; any log mutation that skips the recompute desynchronizes portfolio value and cost basis.
• Incidents: 1480088 — stock-transaction delete and funded-buy bugs plus an over-sell guard; bc256f6 — deleting a vest transaction hit an FK 500 and made vested grants undeletable (deadlock: 422 on grant delete, 500 on transaction delete); fixed by reverting the grant to pending inside the same atomic delete.
• If violated: portfolio shows shares that were never bought or misses shares that were; avg cost basis and P&L are fiction.

3.3 WAL mode on every connection

• Rule: PRAGMA journal_mode=WAL is applied per-connection in api/app/database.py:22 and in the scheduler's private engine api/app/services/scheduler.py:25. The scheduler thread must never share a request-scoped session.
• Rationale: founding design decision (PLAN.md: "Multiple concurrent reads (API + scheduler) without locking; mandatory for this pattern"). Preventive — no incident on record, keep it that way.
• If violated: database is locked errors when the midnight snapshot job overlaps API traffic.

3.4 Non-root Docker user

• Rule: api/Dockerfile creates user app (uid 1000) and runs as it. Do not revert to root.
• Incident: ffa28bb — "security hardening and startup performance — non-root user, disable docs, async prices, remove blocking startup fetch".
• If violated: a container escape or dependency compromise gets root on the homelab host's kernel; also /data ownership breaks (chown app:app /data is baked into the image).

3.5 Balance = SUM(amount − fee)

• Rule: for bank/splitwise/receivable accounts, balance is func.sum(Transaction.amount - Transaction.fee) (api/app/services/net_worth.py:88). Never sum amount alone.
• Incident: 1480088 — "fee balance" was one of the bug-hunt findings: fees were paid in reality but missing from computed balances.
• If violated: every account with fee-bearing transactions drifts upward from its true bank statement.

3.6 Spending counts the user's net share, and only real consumption

• Rule: in get_monthly_spending (crud.py:624), splitwise_pay expenses count at amount + splitwise_partner_share (the user's net share, not the full amount). Internal asset movements (transfers, settlements, funded stock-buy cash legs) are typed transfer/settlement precisely so spending/income queries exclude them.
• Incidents: a9f045f — payable/receivable excluded from spending; e61b254 — then payable expenses had to be re-included (both directions of this mistake exist in history); e36d719 + data migration h8c9d0e1f2a3 — funded stock buys were typed expense and receivable/payable settles typed income/expense, inflating spending and income; new rows retyped and historical rows migrated.
• If violated: monthly spending/income analytics are wrong, which silently poisons every future forecasting/analytics ambition.

3.7 Never fabricate an FX rate

• Rule: if the EUR/USD (or EUR/GBP) rate is missing, fail loudly or return 0 with a warning — never fall back to 1.0 or the wrong currency. (Not a numbered CLAUDE.md invariant, but hard-won; treat as one.)
• Incidents: 6b7d19b — _to_eur silently fell back to treating USD as EUR; 81091fc — removed a 1.0 FX fallback in the price fetcher, skipping the upsert instead; 9b78d0e — GBP fetch error handling aligned with USD; 424b926 / bc256f6 — frontend hardcoded EUR instead of deriving currency from the selected account; bc256f6 also made the remaining 1.0 fallback in calculate_net_worth log a warning.
• If violated: net worth is off by the full FX spread and nobody notices until a snapshot chart looks weird.

3.8 The remaining "What NOT to do" rules and their grounding

Rule (CLAUDE.md)	Grounding
No async SQLAlchemy sessions	Design decision, PLAN.md: aiosqlite adds a threading layer anyway; FastAPI threadpools sync handlers.
No raw DDL / no hand-edits to the SQLite file; Alembic only	§2 + §4. Base.metadata.create_all will NOT fix drift (§4.3).
No new model columns without a migration	Same — create_all never alters existing tables, so prod silently lacks the column while tests (which create fresh) pass.
Never bypass web/src/api/client.ts	Single API layer; the auth work (082b608) shows why: headers/auth must be changeable in one place.
No auth/multi-user without understanding single-user assumptions	082b608 added opt-in bearer auth and CLAUDE.md documents the trap: setting FIN_API_TOKEN on the API locks out the React frontend, which sends no Authorization header.
Never call fetch_all_prices synchronously on the hot path	ffa28bb removed a blocking startup fetch; POST /api/prices/refresh uses FastAPI BackgroundTasks (api/app/routers/prices.py:20-22). It does blocking HTTP with retry sleeps.
Use local dates, not UTC toISOString()	bc256f6: UTC dates booked trades/vests/settles to yesterday before ~1–2 AM local; frontend uses a todayLocalISO() helper.

4. Alembic migration discipline

4.1 Workflow (exact)

cd api
# Point alembic at a scratch DB, never at anything real (env.py reads DATABASE_URL first):
export DATABASE_URL=sqlite:////tmp/mig-scratch.db
alembic upgrade head                       # bring scratch DB to current head
# 1. Edit app/models.py
alembic revision --autogenerate -m "add X to Y"
# 2. READ the generated file. Autogenerate misses server_default intent, index
#    names, and SQLite batch requirements. Fix by hand.
alembic upgrade head                       # 3. Prove it applies
alembic downgrade -1 && alembic upgrade head   # 4. Prove downgrade at least runs
# 5. Commit the migration together with the model change.

(Verified statically: api/alembic/env.py resolves the URL as DATABASE_URL env → -x db_url → app.db_config.get_database_url(); the alembic.ini fallback is sqlite:///./fin.db, which is why you must export DATABASE_URL — otherwise you create a stray api/fin.db. This sandbox cannot execute alembic.)

4.2 The chain (as of 2026-07-05, head = h8c9d0e1f2a3)

Revision	down_revision	What
a1b2c3d4e5f6	(None)	initial schema
b2c3d4e5f6a7	a1b2c3d4e5f6	transaction indexes
c3d4e5f6a7b8	b2c3d4e5f6a7	missing indexes
d4e5f6a7b8c9	c3d4e5f6a7b8	rsu_grants table
e5f6a7b8c9d0	d4e5f6a7b8c9	linked_transaction_id (invariant 3.1)
f6a7b8c9d0e1	e5f6a7b8c9d0	categories.type, server_default='expense'
g7b8c9d0e1f2	f6a7b8c9d0e1	price_cache.name
h8c9d0e1f2a3	g7b8c9d0e1f2	data migration: retype internal cash flows (invariant 3.6)

Single linear chain, no branches. A new revision's down_revision must be the current head — run alembic heads (or read the files) before generating.

4.3 The create_all gotcha

api/app/main.py:19 calls Base.metadata.create_all(bind=engine) in the lifespan. This is first-boot convenience only (fresh dev DBs, tests) — it creates missing tables but never alters existing ones. It is NOT a migration path. In prod the Dockerfile CMD runs alembic upgrade head before uvicorn ever starts, so migrations are the real mechanism. Consequence: a model change without a migration works in tests (fresh DB via create_all) and fails in prod — the most dangerous drift in this repo.

4.4 Data-migration template

h8c9d0e1f2a3_retype_internal_cash_flows.py is the house pattern: docstring explains the semantic bug and the fix; op.get_bind() + sa.text() for surgical UPDATEs; a compiled regex to avoid retyping user-entered rows that merely look similar (Buy groceries vs generated Buy 5 VWCE); an honest no-op downgrade() with a comment stating the original typing is not restored.

5. Spec → plan → implement workflow

Big changes get a written spec and plan before implementation, stored in-repo:

• Spec: docs/superpowers/specs/YYYY-MM-DD-slug.md
• Plan: docs/superpowers/plans/YYYY-MM-DD-slug-plan.md (same slug + -plan)

When required (any one triggers it): a schema change coupled to frontend work; any accounting-logic change (§1); any change touching ≥3 files across both api/ and web/; any redesign of an existing flow's semantics. Precedent trail: spec e86df27 → plan 196bef0 → implementation commits 07ad335…062ba52.

Spec format (mirror 2026-07-01-category-types-receivable-redesign.md): title # Design: …, Date: line, ## Overview with numbered goals, one ## section per goal covering data model, migration notes (including server_default decisions and what the migration must NOT touch), backend schema/model deltas, and UI behavior.

Plan format (mirror 2026-07-01-category-types-receivable-redesign-plan.md): goal/architecture/tech-stack header; a ## Global Constraints section that restates the relevant invariants from §3; a ## File Map table (file → change); then ## Task N sections with - [ ] checkboxes, each listing files, interfaces produced, and verification steps.

Specs and plans are immutable once implemented — they are the historical record; corrections go in a new dated spec.

6. Docs of record — what lives where

Document	Role	Must be updated when
CLAUDE.md	Agent rulebook: conventions, invariants, structure	A new invariant is learned (add to "Key invariants" with rationale); a convention changes; a top-level directory is added (e.g. cli/ in 082b608)
README.md	Human quickstart + config reference	New env var → row in the "Environment / Config" table; new user-facing feature → Features list
PLAN.md	Original design doc — frozen historical reference (only ever touched at repo birth, a9de3c8)	Never retrofit it. Where reality diverged, CLAUDE.md wins.
docs/superpowers/specs/, plans/	Per-change design docs, dated, immutable	Every change that meets the §5 threshold
.claude/skills/fin/SKILL.md	Agent-facing fin CLI usage	CLI commands or flags change
.claude/skills/fin-*/SKILL.md	Process/knowledge skills (this library)	The process itself changes

Commit message house style (mined from git log, as of 2026-07-05 — 41 fix:, 33 feat:, 5 chore:, 2 docs:, plus stray perf:/polish:):

• Subject: type: imperative summary, lower-case after the prefix, no trailing period; em-dash sub-clauses for multi-part fixes (fix: bug hunt findings — currency derivation, local dates, …).
• Body for anything non-trivial: bulleted, grouped under Backend: / Frontend: (see e36d719, bc256f6); each bullet names the file/function and the user-visible effect.
• Migration-bearing commits mention the revision id in the body (Migration h8c9d0e1f2a3 retypes historical rows the same way).
• Agent-authored commits carry Co-Authored-By: and Claude-Session: trailers (observed on 082b608, e36d719).
• Doc commits: docs: spec for … then docs: implementation plan for … as separate commits before the feature commits.

7. Done-checklist — run before declaring any change complete

Copy-paste block (verified statically; this sandbox has no pip/docker — expect them to work in a normal dev env with pip install -e cli/ + api requirements installed):

# 1. Syntax check (stdlib only, no deps needed)
python -m compileall -q api/app cli/fin_cli
# 2. E2E suite: FinClient → real app via TestClient → throwaway SQLite
python -m pytest cli/tests -q
# 3. Frontend type-check + build (package.json: "build": "tsc && vite build")
cd web && npm run build && cd ..
# 4. If a migration exists: dry-run on a scratch DB
cd api && DATABASE_URL=sqlite:////tmp/mig-check.db alembic upgrade head && cd ..

Then the manual gate:

• Change classified per §1 and its full gate satisfied
• No single-leg transfer/settlement create/update/delete anywhere in the diff (§3.1)
• Any stock_transactions mutation recomputes holdings before commit (§3.2)
• No new balance query summing amount without - fee (§3.5)
• Spending/income queries still exclude transfer/settlement rows; splitwise net-share math untouched or spec'd (§3.6)
• No 1.0/wrong-currency FX fallback introduced (§3.7)
• No async session, no raw DDL, no fetch() outside web/src/api/client.ts
• Migration reviewed by hand: backward-safe (nullable/server_default), correct down_revision = current head, honest downgrade() (§4)
• New behavior has a test in cli/tests/ (the API test-coverage gap is the project's known weak point — never widen it; see fin-test-coverage-campaign)
• CLAUDE.md updated if an invariant/convention changed; README env table updated for new config vars; spec/plan written if §5 applies
• If deploy-affecting: prod DB backed up (§2.3) before the image restarts and auto-migrates (§2.4)
• Commit message follows §6 style

Provenance and maintenance

All claims verified against the repo on 2026-07-05 (commit 082b608 at HEAD). Commands were verified statically where noted (no pip/venv/docker in the authoring sandbox). One-line re-verification for anything that may drift:

git -C /path/to/fin log --oneline -5                                  # HEAD moved? re-check volatile facts
ls api/alembic/versions/ && grep -h "down_revision" api/alembic/versions/*.py   # §4.2 chain + head
grep -n "create_all" api/app/main.py                                  # §4.3 gotcha still at lifespan
grep -n "CMD" api/Dockerfile                                          # §2.4 auto-migrate on start
grep -n "useradd" api/Dockerfile                                      # §3.4 non-root user
grep -n "sqlite_data\|DATABASE_URL" docker-compose.yml                # §2 volume + URL
grep -n "add_all" api/app/crud.py                                     # §3.1 atomic legs
grep -n "def delete_stock_transaction" -A 25 api/app/crud.py | grep recompute   # §3.2
grep -rn "journal_mode=WAL" api/app                                   # §3.3 both engines
grep -n "Transaction.amount - " api/app/services/net_worth.py         # §3.5 fee formula
grep -n "def get_monthly_spending" api/app/crud.py                    # §3.6
grep -n '"build"' web/package.json                                    # §7 build command
git -C /path/to/fin log --format='%s' | grep -oE '^[a-z]+:' | sort | uniq -c    # §6 prefix stats
ls docs/superpowers/specs docs/superpowers/plans                      # §5 naming convention
git show a06f921 a1f109b bc256f6 e36d719 1480088 ffa28bb 6b7d19b 81091fc --stat # incident evidence

If any command's output contradicts this file, update this file — CLAUDE.md and the repo are the ground truth, this skill is the map.