diff --git a/.pql/changelog/ticket_history/2026-10.sql b/.pql/changelog/ticket_history/2026-10.sql index 5dc48f7..3b83e94 100644 --- a/.pql/changelog/ticket_history/2026-10.sql +++ b/.pql/changelog/ticket_history/2026-10.sql @@ -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; diff --git a/.pql/changelog/ticket_idmap/2026-10.sql b/.pql/changelog/ticket_idmap/2026-10.sql index 972d734..460d433 100644 --- a/.pql/changelog/ticket_idmap/2026-10.sql +++ b/.pql/changelog/ticket_idmap/2026-10.sql @@ -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; diff --git a/.pql/changelog/tickets/2026-10.sql b/.pql/changelog/tickets/2026-10.sql index 3560ff2..a09311b 100644 --- a/.pql/changelog/tickets/2026-10.sql +++ b/.pql/changelog/tickets/2026-10.sql @@ -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; diff --git a/CHANGELOG.md b/CHANGELOG.md index 827644b..26eed9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/CLAUDE.md b/CLAUDE.md index fec616a..4fb54fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,6 +83,24 @@ live under `src/domains//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 diff --git a/Makefile b/Makefile index 4045628..8ba155a 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/alembic/env.py b/alembic/env.py index 2123e01..b2565a0 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -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: diff --git a/tests/conftest.py b/tests/conftest.py index d3209f3..7747d91 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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 ` against `url`, never against the repo's own .env database.""" + """Run `alembic -x target=test ` 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, diff --git a/tests/test_alembic_target.py b/tests/test_alembic_target.py new file mode 100644 index 0000000..2685817 --- /dev/null +++ b/tests/test_alembic_target.py @@ -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 diff --git a/tests/test_dev_test_server.py b/tests/test_dev_test_server.py new file mode 100644 index 0000000..55ff3f6 --- /dev/null +++ b/tests/test_dev_test_server.py @@ -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"