fix(alembic): refuse to run without a named migration target (T-3)

`alembic/env.py` took its target from `get_settings().database_url`,
composed from `.env`'s production `POSTGRES_*` values, so the most natural
command in the repo — a bare `alembic current` — talked to production.
`-x target=test` and `-x target=production` now name the database
explicitly; anything else exits non-zero before any engine is built, and
the one line it prints before migrating never shows a credential.

`run_alembic` (tests/conftest.py) passes `-x target=test` and sets
`TEST_DATABASE_URL` in the subprocess environment instead of overriding
`POSTGRES_*` — the `_components()` shim is gone; nothing else used it.

`make migrate-test`, `make migrate-production` and `make dev-test-server`
are the three documented ways to reach either database now; `dev-test-server`
refuses to start if the resolved database is not `core_api_test`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-20 00:26:48 +02:00
co-authored by Claude Sonnet 5
parent 237bdf9306
commit 3a3dc959be
10 changed files with 242 additions and 33 deletions
@@ -184,3 +184,6 @@ Raised again by the T-5 round: every round rebuilds the `:8777` dev server again
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNBXC7H3JBESKFS20BV71BJG', 'description', 'Each migration file''s round-trip test re-upgrades to its own revision, not to head (tests/test_tenants_migration.py ends on 008), so since 009 a full suite run leaves core_api_test one behind head and the next manual check starts from a stale schema. End every round-trip test on head. Found in round core-api-T-13-001.', 'Each migration file''s round-trip test re-upgrades to its own revision, not to head (tests/test_tenants_migration.py ends on 008), so since 009 a full suite run leaves core_api_test one behind head and the next manual check starts from a stale schema. End every round-trip test on head. Found in round core-api-T-13-001.
Grows by one with every migration: after 010, a full suite run leaves core_api_test at 008, two behind head (round core-api-T-15-001).', NULL, '2026-10-19 21:40:25', '2026-10-19 21:40:25.820', '2026-10-19 21:40:25.820', NULL, '62065aa1d8fd7ad21e9a7abd2f70f0c1', 2) ON CONFLICT(hash) DO NOTHING;
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNCC8BZT7RYQC16XK1MXBBRM', 'description', 'For the Office''s polling (tatlock-office T-11): GET /scribe/mailbox takes since_message_id=N — only the caller''s entries whose message number is above N, combined with view/unread/tag as today — and total counts what matched, so unread=true&limit=1 is a one-row unread count. Waits for round core-api-T-3-001, which is in this repo now.', 'For the Office''s polling (tatlock-office T-11): GET /scribe/mailbox takes since_message_id=N — only the caller''s entries whose message number is above N, combined with view/unread/tag as today — and total counts what matched, so unread=true&limit=1 is a one-row unread count. Waits for round core-api-T-3-001, which is in this repo now.
**Also: the newest message number.** The mailbox response carries the caller''s newest message number across its whole mailbox, whatever the filters (`newest_message_id`). The Office''s `glance` needs it: the unread-first sort makes `limit=1` return an older unread message when a newer one is already read (found in round tatlock-office-T-11-001).', NULL, '2026-10-19 22:25:51', '2026-10-19 22:25:51.654', '2026-10-19 22:25:51.654', NULL, '257d4c93e0c8ff4691ec032c7dd7f0f8', 2) ON CONFLICT(hash) DO NOTHING;
+1
View File
@@ -13,3 +13,4 @@ INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNBR1VBM8SZP8DDK93SFS608', 'T-13', '2026-10-19 20:35:21.053', '2026-10-19 20:35:21.053', NULL, 'aab7f828f5f01bd8310c85f53ff0dc19', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNBXC7H3JBESKFS20BV71BJG', 'T-14', '2026-10-19 20:58:36.809', '2026-10-19 20:58:36.809', NULL, '65086d233f5bc652cec0aa79dcaec2c4', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNBZZR0RQKSAJ5FKF4X540KC', 'T-15', '2026-10-19 21:10:00.967', '2026-10-19 21:10:00.967', NULL, '799679cb2e45f2a0c5ea07da08aa93e1', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
INSERT INTO ticket_idmap (record_id, ticket_id, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNCC8BZT7RYQC16XK1MXBBRM', 'T-16', '2026-10-19 22:03:37.343', '2026-10-19 22:03:37.343', NULL, '092ac223950852535456b31c45a8ae04', 2) ON CONFLICT(record_id) DO UPDATE SET ticket_id=excluded.ticket_id, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= ticket_idmap.updated_at;
+4
View File
@@ -217,3 +217,7 @@ Raised again by the T-5 round: every round rebuilds the `:8777` dev server again
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNBXC7H3JBESKFS20BV71BJG', 'task', NULL, 'Migration round-trip tests leave core_api_test behind head', 'Each migration file''s round-trip test re-upgrades to its own revision, not to head (tests/test_tenants_migration.py ends on 008), so since 009 a full suite run leaves core_api_test one behind head and the next manual check starts from a stale schema. End every round-trip test on head. Found in round core-api-T-13-001.
Grows by one with every migration: after 010, a full suite run leaves core_api_test at 008, two behind head (round core-api-T-15-001).', 'backlog', 'low', NULL, NULL, NULL, '2026-10-19 20:58:36.809', '2026-10-19 21:40:25.820', NULL, '4501f0022158bcc0083cd2309d79c115', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNCC8BZT7RYQC16XK1MXBBRM', 'task', NULL, 'The mailbox answers only messages after N, with a count', 'For the Office''s polling (tatlock-office T-11): GET /scribe/mailbox takes since_message_id=N — only the caller''s entries whose message number is above N, combined with view/unread/tag as today — and total counts what matched, so unread=true&limit=1 is a one-row unread count. Waits for round core-api-T-3-001, which is in this repo now.', 'backlog', 'high', NULL, NULL, NULL, '2026-10-19 22:03:37.343', '2026-10-19 22:03:37.343', NULL, 'fd4a1b4c6ed521ffaaa246665f7e45ca', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06GNCC8BZT7RYQC16XK1MXBBRM', 'task', NULL, 'The mailbox answers only messages after N, with a count', 'For the Office''s polling (tatlock-office T-11): GET /scribe/mailbox takes since_message_id=N — only the caller''s entries whose message number is above N, combined with view/unread/tag as today — and total counts what matched, so unread=true&limit=1 is a one-row unread count. Waits for round core-api-T-3-001, which is in this repo now.
**Also: the newest message number.** The mailbox response carries the caller''s newest message number across its whole mailbox, whatever the filters (`newest_message_id`). The Office''s `glance` needs it: the unread-first sort makes `limit=1` return an older unread message when a newer one is already read (found in round tatlock-office-T-11-001).', 'backlog', 'high', NULL, NULL, NULL, '2026-10-19 22:03:37.343', '2026-10-19 22:25:51.653', NULL, '8f6daa3a748e4b26d6c7ce40fc9ee8a0', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
+10
View File
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Changed
- **Alembic refuses to run without a named migration target** (core-api T-3) —
`alembic/env.py` exits non-zero before building any connection unless `-x target=test`
or `-x target=production` is given; nothing defaults, so the most natural command in
the repo, a bare `alembic current`, can no longer reach production by accident.
`make migrate-test`, `make migrate-production` and `make dev-test-server` are the three
documented ways to reach either database; none of them print a password or a
credential-bearing URL.
## [1.15.0] - 2026-10-19
### Added
+18
View File
@@ -83,6 +83,24 @@ live under `src/domains/<name>/models.py` and must be imported in `alembic/env.p
register with `Base.metadata` — check that file when adding a new model or `alembic
revision --autogenerate` will silently miss it.
**A bare `alembic` refuses — it names no target (core-api T-3).** `alembic/env.py`
exits non-zero before building any connection unless told explicitly which database:
`-x target=test` (the database `TEST_DATABASE_URL` names) or `-x target=production`
(today's settings-derived database — real production). Nothing defaults, because a bare
`.venv/bin/alembic current` once reached production by accident, read-only but still
unintended. Three `make` targets are the only documented way to reach either database:
```bash
make migrate-test # upgrade core_api_test to head
make migrate-production # the one documented way to migrate production — says what
# it's about to do, then alembic -x target=production upgrade head
make dev-test-server # uvicorn on :8777 against core_api_test; refuses to start
# if TEST_DATABASE_URL resolves to any other database name
```
None of the three ever print a password or a credential-bearing URL — only the
database name, and (for `migrate-production`) an announcement before it runs.
## Working here
**Plan, act, reflect.** Outline which files you will touch and the side effects before
+29
View File
@@ -38,6 +38,35 @@ test: ## Run the test suite
@test -x $(VENV)/bin/python || { echo "FAIL — no venv; run: make setup"; exit 69; }
$(VENV)/bin/python -m pytest tests/
# Alembic refuses to run with no named target, or any other one (core-api
# T-3) — `alembic/env.py` exits before connecting rather than falling back
# to production. These three targets are the only documented way to reach
# either database.
.PHONY: migrate-test
migrate-test: ## Upgrade core_api_test (TEST_DATABASE_URL) to head
@test -x $(VENV)/bin/alembic || { echo "FAIL — no venv; run: make setup"; exit 69; }
$(VENV)/bin/alembic -x target=test upgrade head
.PHONY: migrate-production
migrate-production: ## The one documented way to migrate production: alembic -x target=production upgrade head
@test -x $(VENV)/bin/alembic || { echo "FAIL — no venv; run: make setup"; exit 69; }
@echo "migrate-production: about to run 'alembic -x target=production upgrade head' against the production database"
$(VENV)/bin/alembic -x target=production upgrade head
.PHONY: dev-test-server
dev-test-server: ## uvicorn on :8777 against core_api_test only (refuses on any other resolved database; never prints credentials)
@test -x $(VENV)/bin/python || { echo "FAIL — no venv; run: make setup"; exit 69; }
@url="$$TEST_DATABASE_URL"; \
if [ -z "$$url" ]; then url=$$(grep -m1 '^TEST_DATABASE_URL=' .env 2>/dev/null | cut -d= -f2-); fi; \
if [ -z "$$url" ]; then echo "FAIL — TEST_DATABASE_URL is not set (environment or .env)"; exit 69; fi; \
rest=$${url#postgresql://}; creds=$${rest%%@*}; hostdb=$${rest#*@}; \
puser=$${creds%%:*}; ppass=$${creds#*:}; phost=$${hostdb%%/*}; pdb=$${hostdb#*/}; \
if [ "$$pdb" != "core_api_test" ]; then echo "FAIL — resolved database is '$$pdb', not core_api_test; refusing to start"; exit 1; fi; \
echo "dev-test-server: database=$$pdb"; \
POSTGRES_HOST="$$phost" POSTGRES_USER="$$puser" POSTGRES_PASSWORD="$$ppass" POSTGRES_DATABASE="$$pdb" \
$(VENV)/bin/python -m uvicorn src.main:app --reload --host 0.0.0.0 --port 8777
# No `lint` target, deliberately. This repo configures no linter — no ruff or
# flake8 config, and neither in requirements. Per D-27 the name is reserved for
# repos that lint; an empty target here would report clean for something never
+75 -8
View File
@@ -2,18 +2,30 @@
Alembic Environment Configuration
Async migration environment for SQLAlchemy 2.0 with asyncpg.
Alembic refuses to run without an explicit target (core-api T-3): a bare
`alembic current` used to fall back to `get_settings().database_url`, which
is composed from `.env`'s production `POSTGRES_*` values — the most natural
command in the repo was quietly talking to production. `-x target=test`
names the database `TEST_DATABASE_URL` points at; `-x target=production`
names today's settings-derived database, deliberately. Nothing else
defaults, and the check runs — and can exit — before any engine is built.
"""
import asyncio
import os
import re
import sys
from logging.config import fileConfig
from pathlib import Path
from sqlalchemy import pool
from sqlalchemy.engine import Connection
from sqlalchemy.engine import Connection, make_url
from sqlalchemy.ext.asyncio import async_engine_from_config
from alembic import context
# Import our models and config
from src.shared.config import get_settings
# Import our models (config is imported lazily, inside _production_url, so
# that resolving a bad or missing target never has to construct Settings).
from src.shared.database import Base
# Import all models to ensure they're registered with Base.metadata
@@ -27,6 +39,8 @@ from src.domains.scribe.models import ( # noqa: F401
)
from src.domains.tenants.models import Tenant, TenantConsumer # noqa: F401
REPO_ROOT = Path(__file__).resolve().parent.parent
# Alembic Config object
config = context.config
@@ -37,11 +51,64 @@ if config.config_file_name is not None:
# Model metadata for autogenerate support
target_metadata = Base.metadata
# Get database URL from our settings
settings = get_settings()
db_url = settings.database_url
if db_url.startswith("postgresql://"):
db_url = db_url.replace("postgresql://", "postgresql+asyncpg://", 1)
def _test_url() -> str:
"""TEST_DATABASE_URL from the environment, else the repo's own `.env` —
read that one variable only; the file is never loaded or sourced whole."""
value = os.environ.get("TEST_DATABASE_URL")
if value:
return value
env_path = REPO_ROOT / ".env"
if env_path.is_file():
with open(env_path) as f:
for line in f:
match = re.match(r"^TEST_DATABASE_URL=(.*)$", line.rstrip("\n"))
if match:
return match.group(1).strip().strip("'\"")
raise SystemExit(
"alembic: -x target=test needs TEST_DATABASE_URL (environment, or "
"the repo's .env) — none found."
)
def _production_url() -> str:
"""Today's settings-derived database — deliberately, only when asked
for by name."""
from src.shared.config import get_settings
return get_settings().database_url
def _resolve_target_url() -> str:
"""-x target=test|production names the database to migrate. No target,
or anything else, exits non-zero here — before any engine is built."""
x_args = context.get_x_argument(as_dictionary=True)
target = x_args.get("target")
if target == "test":
url = _test_url()
elif target == "production":
url = _production_url()
else:
raise SystemExit(
"alembic: no migration target named. Pass one of:\n"
" -x target=test (the database TEST_DATABASE_URL names)\n"
" -x target=production (today's settings-derived database)\n"
f"Got -x target={target!r}. Nothing defaults."
)
if url.startswith("postgresql://"):
url = url.replace("postgresql://", "postgresql+asyncpg://", 1)
return url
db_url = _resolve_target_url()
# Name the target before migrating anything — never a host's credentials,
# never the URL itself.
print(f"alembic target: database={make_url(db_url).database!r}", file=sys.stderr)
def run_migrations_offline() -> None:
+13 -25
View File
@@ -8,15 +8,16 @@ other `POSTGRES_*` values point there too, so this is the one guard that
stands between a test run and the production database.
Running the actual migration against the test database is done through the
`alembic` CLI in a subprocess rather than in-process, because
`alembic/env.py` always builds its connection URL from `get_settings()`
(composed from `POSTGRES_*`), with no override hook of its own — and
`get_settings()` is `lru_cache`d, so by the time a test runs, other test
`alembic` CLI in a subprocess rather than in-process, because `alembic/env.py`
builds its connection in a fresh interpreter each time, unaffected by this
process's `lru_cache`d `get_settings()` — by the time a test runs, other test
modules have almost certainly already imported `src.main` and cached a
Settings instance pointed at production. A subprocess gets a clean
environment and a clean cache; overriding `POSTGRES_HOST`/`USER`/
`PASSWORD`/`DATABASE` in that child's environment, derived from
`TEST_DATABASE_URL`, is what actually redirects the migration.
Settings instance pointed at production. `run_alembic` always passes
`-x target=test` (core-api T-3: alembic refuses to run with no target, or
any other one) and sets `TEST_DATABASE_URL` in the child's environment to
`url`, which is what actually redirects the migration — the subprocess's own
`alembic/env.py` reads that variable itself; nothing here overrides
`POSTGRES_*` any more.
"""
import asyncio
import os
@@ -33,26 +34,13 @@ ALEMBIC_BIN = REPO_ROOT / ".venv" / "bin" / "alembic"
load_dotenv(REPO_ROOT / ".env")
def _components(url: str) -> dict:
"""POSTGRES_* overrides for a subprocess, derived from a database URL."""
parts = urlsplit(url)
host = parts.hostname or ""
if parts.port:
host = f"{host}:{parts.port}"
return {
"POSTGRES_HOST": host,
"POSTGRES_USER": parts.username or "",
"POSTGRES_PASSWORD": parts.password or "",
"POSTGRES_DATABASE": parts.path.lstrip("/"),
}
def run_alembic(url: str, *args: str) -> subprocess.CompletedProcess:
"""Run `alembic <args>` against `url`, never against the repo's own .env database."""
"""Run `alembic -x target=test <args>` against `url`, never against the
repo's own .env database."""
env = os.environ.copy()
env.update(_components(url))
env["TEST_DATABASE_URL"] = url
result = subprocess.run(
[str(ALEMBIC_BIN), *args],
[str(ALEMBIC_BIN), "-x", "target=test", *args],
cwd=str(REPO_ROOT),
env=env,
capture_output=True,
+49
View File
@@ -0,0 +1,49 @@
"""Tests for core-api T-3: alembic refuses to run without a named target.
`alembic/env.py` resolves `-x target=test` or `-x target=production`; no
target, or anything else, must exit non-zero before any engine is built —
not skip, not fall back to the production settings a bare `alembic current`
used to reach by accident. Both tests point the *production* settings
(`POSTGRES_*`) at an unreachable host: if the refusal ever started
connecting before checking the target, these would hang (and the
subprocess timeout below would fire) instead of returning quickly.
"""
import os
import subprocess
from tests.conftest import ALEMBIC_BIN, REPO_ROOT
UNREACHABLE_HOST = "10.255.255.1" # non-routed; a real connection attempt would hang
def _run_with_unreachable_production(*args: str) -> subprocess.CompletedProcess:
env = os.environ.copy()
env["POSTGRES_HOST"] = UNREACHABLE_HOST
env["POSTGRES_USER"] = "probe"
env["POSTGRES_PASSWORD"] = "probe"
env["POSTGRES_DATABASE"] = "probe"
env.pop("TEST_DATABASE_URL", None)
return subprocess.run(
[str(ALEMBIC_BIN), *args],
cwd=str(REPO_ROOT),
env=env,
capture_output=True,
text=True,
timeout=10, # a real connection attempt to an unreachable host hangs well past this
)
def test_no_target_refuses_before_connecting():
result = _run_with_unreachable_production("current")
assert result.returncode != 0
combined = result.stdout + result.stderr
assert "target=test" in combined
assert "target=production" in combined
def test_nonsense_target_refuses_before_connecting():
result = _run_with_unreachable_production("-x", "target=nonsense", "current")
assert result.returncode != 0
combined = result.stdout + result.stderr
assert "target=test" in combined
assert "target=production" in combined
+40
View File
@@ -0,0 +1,40 @@
"""Tests for core-api T-3's `make dev-test-server` target: it must refuse to
start when the resolved database is not `core_api_test`, before uvicorn ever
binds the port, and it must never print credentials while doing so."""
import os
import socket
import subprocess
from urllib.parse import urlsplit
from tests.conftest import REPO_ROOT
PORT = 8777
def _port_is_free(port: int) -> bool:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
return s.connect_ex(("localhost", port)) != 0
def test_refuses_when_resolved_database_is_not_core_api_test(test_database_url):
password = urlsplit(test_database_url).password
wrong = test_database_url.rsplit("/", 1)[0] + "/core_api"
env = os.environ.copy()
env["TEST_DATABASE_URL"] = wrong
result = subprocess.run(
["make", "dev-test-server"],
cwd=str(REPO_ROOT),
env=env,
capture_output=True,
text=True,
timeout=15,
)
combined = result.stdout + result.stderr
assert result.returncode != 0
assert "core_api_test" in combined
if password:
assert password not in combined
assert _port_is_free(PORT), "dev-test-server must never bind the port when it refuses"