From 342e981a92ed4560ff6941bd6f20be3467811a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 20:16:50 +0800 Subject: [PATCH 1/8] feat: support postgres schema overrides --- .env.example | 2 + app/core/config/settings.py | 11 ++ app/db/alembic/env.py | 4 +- app/db/migrate.py | 6 +- app/db/migration_url.py | 35 +++++ app/db/session.py | 7 +- .../proxy/durable_bridge_repository.py | 2 +- deploy/helm/codex-lb/README.md | 23 +++- deploy/helm/codex-lb/templates/configmap.yaml | 3 + .../helm/codex-lb/templates/deployment.yaml | 8 ++ .../templates/hooks/migration-job.yaml | 4 + deploy/helm/codex-lb/values-external-db.yaml | 3 + deploy/helm/codex-lb/values.schema.json | 3 + deploy/helm/codex-lb/values.yaml | 2 + docs/reference/settings.md | 3 +- .../.openspec.yaml | 2 + .../proposal.md | 36 +++++ .../specs/database-backends/spec.md | 44 +++++++ .../specs/database-migrations/spec.md | 49 +++++++ .../specs/deployment-installation/spec.md | 24 ++++ .../tasks.md | 29 ++++ tests/unit/test_db_migrate.py | 124 +++++++++++++++++- tests/unit/test_db_session.py | 22 ++++ tests/unit/test_durable_bridge_sessions.py | 29 ++++ tests/unit/test_helm_external_secrets.py | 37 ++++++ tests/unit/test_settings_reference.py | 6 +- 26 files changed, 509 insertions(+), 9 deletions(-) create mode 100644 openspec/changes/support-postgres-schema-overrides/.openspec.yaml create mode 100644 openspec/changes/support-postgres-schema-overrides/proposal.md create mode 100644 openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md create mode 100644 openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md create mode 100644 openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md create mode 100644 openspec/changes/support-postgres-schema-overrides/tasks.md diff --git a/.env.example b/.env.example index 1e75b20df7..9b5eae1c77 100644 --- a/.env.example +++ b/.env.example @@ -15,6 +15,8 @@ # Database. SQLite (default) needs nothing. For PostgreSQL: # CODEX_LB_DATABASE_URL=postgresql+asyncpg://codex_lb:codex_lb@127.0.0.1:5432/codex_lb +# Optional shared-DB schema isolation (search_path ",public"): +# CODEX_LB_DATABASE_POSTGRES_SCHEMA=codex_lb_prod # Encryption key file (pin for Docker volumes; must be shared across replicas). # CODEX_LB_ENCRYPTION_KEY_FILE=/var/lib/codex-lb/encryption.key diff --git a/app/core/config/settings.py b/app/core/config/settings.py index e38723400d..7c24b5dfd3 100644 --- a/app/core/config/settings.py +++ b/app/core/config/settings.py @@ -249,6 +249,7 @@ class Settings(BaseSettings): database_sqlite_pre_migrate_backup_enabled: bool = True database_sqlite_pre_migrate_backup_max_files: int = Field(default=5, ge=1) database_sqlite_startup_check_mode: Literal["quick", "full", "off"] = "quick" + database_postgres_schema: str | None = None database_alembic_auto_remap_enabled: bool = True database_migration_lock_timeout_seconds: float = Field(default=300.0, gt=0) upstream_base_url: str = "https://chatgpt.com/backend-api" @@ -491,6 +492,16 @@ def _expand_database_url(cls, value: str) -> str: return f"{prefix}{Path(path).expanduser()}" return value + @field_validator("database_postgres_schema", mode="before") + @classmethod + def _normalize_database_postgres_schema(cls, value: OptionalStringInput) -> str | None: + if value is None: + return None + if isinstance(value, str): + stripped = value.strip() + return stripped or None + raise TypeError("database_postgres_schema must be a string") + @field_validator("encryption_key_file", mode="before") @classmethod def _expand_encryption_key_file(cls, value: str | Path) -> Path: diff --git a/app/db/alembic/env.py b/app/db/alembic/env.py index ab5d485c7b..2c9e4905bb 100644 --- a/app/db/alembic/env.py +++ b/app/db/alembic/env.py @@ -6,7 +6,7 @@ from sqlalchemy import engine_from_config, pool from app.core.config.settings import get_settings -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import apply_postgres_search_path, ensure_postgres_schema_exists, to_sync_database_url from app.db.models import Base config = context.config @@ -53,6 +53,8 @@ def run_migrations_online() -> None: ) with connectable.connect() as connection: + ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) + apply_postgres_search_path(connection, get_settings().database_postgres_schema) context.configure( connection=connection, target_metadata=target_metadata, diff --git a/app/db/migrate.py b/app/db/migrate.py index a75e404255..a49f884cb1 100644 --- a/app/db/migrate.py +++ b/app/db/migrate.py @@ -22,7 +22,7 @@ from app.core.config.settings import get_settings from app.db.alembic.revision_ids import LEGACY_MIGRATION_TO_NEW_REVISION, OLD_TO_NEW_REVISION_MAP, REVISION_ID_PATTERN from app.db.migration_lock import migration_lock -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import apply_postgres_search_path, ensure_postgres_schema_exists, to_sync_database_url from app.db.models import Base logger = logging.getLogger(__name__) @@ -178,6 +178,8 @@ def _sync_connection(sync_database_url: str) -> Iterator[Connection]: engine = create_engine(sync_database_url, future=True) try: with engine.connect() as connection: + ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) + apply_postgres_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() @@ -188,6 +190,8 @@ def _sync_transaction(sync_database_url: str) -> Iterator[Connection]: engine = create_engine(sync_database_url, future=True) try: with engine.begin() as connection: + ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) + apply_postgres_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() diff --git a/app/db/migration_url.py b/app/db/migration_url.py index f9dadbf5f9..9dcd2aa24e 100644 --- a/app/db/migration_url.py +++ b/app/db/migration_url.py @@ -1,6 +1,9 @@ from __future__ import annotations +from sqlalchemy import text +from sqlalchemy.engine import Connection from sqlalchemy.engine import make_url +from sqlalchemy.schema import CreateSchema from app.db.sqlite_utils import normalize_sqlite_url @@ -16,3 +19,35 @@ def to_sync_database_url(database_url: str) -> str: parsed = parsed.set(drivername="postgresql+psycopg") return parsed.render_as_string(hide_password=False) + + +def postgres_search_path(schema: str | None) -> str | None: + if schema is None: + return None + normalized = schema.strip() + if not normalized: + return None + if normalized.casefold() == "public": + return "public" + return f"{normalized},public" + + +def ensure_postgres_schema_exists(connection: Connection, schema: str | None) -> None: + if connection.dialect.name != "postgresql" or schema is None: + return + + normalized = schema.strip() + if not normalized or normalized.casefold() == "public": + return + + connection.execute(CreateSchema(normalized, if_not_exists=True)) + + +def apply_postgres_search_path(connection: Connection, schema: str | None) -> None: + search_path = postgres_search_path(schema) + if search_path is None or connection.dialect.name != "postgresql": + return + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": search_path}, + ) diff --git a/app/db/session.py b/app/db/session.py index cd8a45a729..7ab8bf3591 100644 --- a/app/db/session.py +++ b/app/db/session.py @@ -17,6 +17,7 @@ from sqlalchemy.pool import NullPool from app.core.config.settings import get_settings +from app.db.migration_url import postgres_search_path from app.db.sqlite_utils import ( SqliteIntegrityCheckMode, check_sqlite_integrity, @@ -76,7 +77,11 @@ def _postgres_async_connect_args(url: str) -> dict[str, object] | None: # bridge-session cleanup stop running, and account/stream lease expiry is # mis-evaluated. Forcing UTC keeps stored timestamps correct regardless of # the container time zone. - connect_args: dict[str, object] = {"server_settings": {"timezone": "UTC"}} + server_settings: dict[str, str] = {"timezone": "UTC"} + search_path = postgres_search_path(_settings.database_postgres_schema) + if search_path is not None: + server_settings["search_path"] = search_path + connect_args: dict[str, object] = {"server_settings": server_settings} if os.environ.get("CODEX_LB_TEST_DATABASE_URL"): connect_args["prepared_statement_cache_size"] = 0 return connect_args diff --git a/app/modules/proxy/durable_bridge_repository.py b/app/modules/proxy/durable_bridge_repository.py index bcea92b62b..4b73e2993c 100644 --- a/app/modules/proxy/durable_bridge_repository.py +++ b/app/modules/proxy/durable_bridge_repository.py @@ -1786,7 +1786,7 @@ async def missing_durable_bridge_tables(session: AsyncSession) -> tuple[str, ... result = await session.execute( text( "SELECT table_name FROM information_schema.tables " - "WHERE table_schema = 'public' " + "WHERE table_schema = ANY (current_schemas(false)) " "AND table_name IN (" "'http_bridge_sessions', 'http_bridge_session_aliases', 'http_bridge_retry_circuits', " "'http_bridge_recovery_attempts'" diff --git a/deploy/helm/codex-lb/README.md b/deploy/helm/codex-lb/README.md index b661342553..72114fd95d 100644 --- a/deploy/helm/codex-lb/README.md +++ b/deploy/helm/codex-lb/README.md @@ -76,13 +76,15 @@ Supported DB wiring: - `externalDatabase.host`, `externalDatabase.port`, `externalDatabase.database`, `externalDatabase.user` - `externalDatabase.existingSecret` - `auth.existingSecret` if one secret contains both `database-url` and `encryption-key` +- `config.databasePostgresSchema` when you share one PostgreSQL database and want codex-lb isolated in its own schema Example using a direct URL: ```bash helm install codex-lb oci://ghcr.io/soju06/charts/codex-lb \ --set postgresql.enabled=false \ - --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' + --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' \ + --set config.databasePostgresSchema=codex_lb_prod ``` Example using separate secrets: @@ -100,7 +102,8 @@ helm install codex-lb oci://ghcr.io/soju06/charts/codex-lb \ ```bash helm upgrade --install codex-lb deploy/helm/codex-lb/ \ -f deploy/helm/codex-lb/values-external-db.yaml \ - --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' + --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' \ + --set config.databasePostgresSchema=codex_lb_prod ``` @@ -216,6 +219,7 @@ This chart intentionally keeps migration behavior explicit by install mode. - Application pods use a schema gate initContainer when `migration.enabled=true`, `config.databaseMigrateOnStartup=false`, and `migration.schemaGate.enabled=true`. - That initContainer runs `python -m app.db.migrate wait-for-head` and blocks the app container until the database is at Alembic head. - In bundled mode, `values-bundled.yaml` enables startup migration instead of the schema gate so fresh self-contained installs do not deadlock on `helm install --wait`. +- When `config.databasePostgresSchema` is non-empty, the migration Job, schema-gate initContainers, and the main workload all use the same PostgreSQL `search_path` prefix so a shared database stays isolated by schema. This means: @@ -240,6 +244,21 @@ Use `externalDatabase.existingSecret` for the database URL and let the chart man When `externalDatabase.existingSecret` is set and `auth.existingSecret` is not, the chart-managed app secret contains only the encryption key; the StatefulSet reads `CODEX_LB_DATABASE_URL` from the external DB secret. +## Shared PostgreSQL Databases + +If your platform shares one PostgreSQL database across multiple applications, set `config.databasePostgresSchema` so codex-lb uses its own schema while continuing to keep `public` on the search path for standard PostgreSQL behavior: + +```yaml +config: + databasePostgresSchema: codex_lb_prod +``` + +On the first migration run codex-lb attempts `CREATE SCHEMA IF NOT EXISTS` +before it reads or advances Alembic state. If your deployment user is not +allowed to create schemas, pre-create the schema once and keep using the same +`config.databasePostgresSchema` value for the workload, migration Job, and +schema gate. + ## Network Policy When `networkPolicy.enabled=true`, the chart now fails closed for the main HTTP ingress port. diff --git a/deploy/helm/codex-lb/templates/configmap.yaml b/deploy/helm/codex-lb/templates/configmap.yaml index f4833af865..3bed31053f 100644 --- a/deploy/helm/codex-lb/templates/configmap.yaml +++ b/deploy/helm/codex-lb/templates/configmap.yaml @@ -14,6 +14,9 @@ data: CODEX_LB_DATABASE_MIGRATE_ON_STARTUP: {{ .Values.config.databaseMigrateOnStartup | toString | quote }} CODEX_LB_DATABASE_POOL_SIZE: {{ .Values.config.databasePoolSize | toString | quote }} CODEX_LB_DATABASE_MAX_OVERFLOW: {{ .Values.config.databaseMaxOverflow | toString | quote }} + {{- if .Values.config.databasePostgresSchema }} + CODEX_LB_DATABASE_POSTGRES_SCHEMA: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} # Upstream {{- if .Values.config.upstreamBaseUrl }} CODEX_LB_UPSTREAM_BASE_URL: {{ .Values.config.upstreamBaseUrl | quote }} diff --git a/deploy/helm/codex-lb/templates/deployment.yaml b/deploy/helm/codex-lb/templates/deployment.yaml index b2e39df3b6..e271f13e79 100644 --- a/deploy/helm/codex-lb/templates/deployment.yaml +++ b/deploy/helm/codex-lb/templates/deployment.yaml @@ -209,6 +209,10 @@ spec: secretKeyRef: name: {{ include "codex-lb.databaseUrlSecretName" . }} key: {{ .Values.auth.secretKeys.databaseUrl }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} - name: CODEX_LB_ENCRYPTION_KEY_FILE value: /var/lib/codex-lb/encryption.key volumeMounts: @@ -240,6 +244,10 @@ spec: secretKeyRef: name: {{ include "codex-lb.databaseUrlSecretName" . }} key: {{ .Values.auth.secretKeys.databaseUrl }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} - name: CODEX_LB_ENCRYPTION_KEY_FILE value: /var/lib/codex-lb/encryption.key volumeMounts: diff --git a/deploy/helm/codex-lb/templates/hooks/migration-job.yaml b/deploy/helm/codex-lb/templates/hooks/migration-job.yaml index 0d0bb9993c..ba0f5fb1f0 100644 --- a/deploy/helm/codex-lb/templates/hooks/migration-job.yaml +++ b/deploy/helm/codex-lb/templates/hooks/migration-job.yaml @@ -95,6 +95,10 @@ spec: - name: CODEX_LB_DATABASE_URL value: {{ include "codex-lb.databaseUrl" . | quote }} {{- end }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} {{- with .Values.extraEnv }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/deploy/helm/codex-lb/values-external-db.yaml b/deploy/helm/codex-lb/values-external-db.yaml index 82a19c65ac..353fea18fa 100644 --- a/deploy/helm/codex-lb/values-external-db.yaml +++ b/deploy/helm/codex-lb/values-external-db.yaml @@ -12,3 +12,6 @@ migration: # - externalDatabase.host/user/database (+ optional port) # - externalDatabase.existingSecret # - auth.existingSecret +# +# Optional for shared PostgreSQL databases: +# config.databasePostgresSchema: codex_lb_prod diff --git a/deploy/helm/codex-lb/values.schema.json b/deploy/helm/codex-lb/values.schema.json index 292d550eb0..4d380f921d 100644 --- a/deploy/helm/codex-lb/values.schema.json +++ b/deploy/helm/codex-lb/values.schema.json @@ -207,6 +207,9 @@ "config": { "type": "object", "properties": { + "databasePostgresSchema": { + "type": "string" + }, "shutdownDrainTimeoutSeconds": { "type": "integer", "minimum": 1 diff --git a/deploy/helm/codex-lb/values.yaml b/deploy/helm/codex-lb/values.yaml index c1b67733b6..035278f5cb 100644 --- a/deploy/helm/codex-lb/values.yaml +++ b/deploy/helm/codex-lb/values.yaml @@ -123,6 +123,8 @@ serviceAccount: config: # @param config.databaseMigrateOnStartup Disable auto-migration (migration Job handles it) databaseMigrateOnStartup: false + # @param config.databasePostgresSchema Optional PostgreSQL schema search_path prefix for shared databases + databasePostgresSchema: "" # @param config.databasePoolSize SQLAlchemy connection pool size # Each supported replica has one worker with two pools (request + background). # Reserve 20 of PostgreSQL's default 100 slots for PG internals, migrations, and operations. diff --git a/docs/reference/settings.md b/docs/reference/settings.md index e7486b2321..2551bac015 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -7,7 +7,7 @@ Regenerate with `uv run python scripts/generate_settings_reference.py`; `tests/unit/test_settings_reference.py` fails when this page drifts from `app/core/config/settings.py`. -codex-lb currently exposes 118 settings. Every setting is an environment +codex-lb currently exposes 119 settings. Every setting is an environment variable with the `CODEX_LB_` prefix (process environment or `.env` / `.env.local` next to the process). All defaults work with zero configuration — start from [Configuration](../configuration.md) for the handful that matter, @@ -37,6 +37,7 @@ the host side of the compose `ports` mapping instead. | `CODEX_LB_DATABASE_MIGRATION_LOCK_TIMEOUT_SECONDS` | `float` | `300.0` | | `CODEX_LB_DATABASE_MIGRATIONS_FAIL_FAST` | `bool` | `True` | | `CODEX_LB_DATABASE_POOL_SIZE` | `int` | `15` | +| `CODEX_LB_DATABASE_POSTGRES_SCHEMA` | `str \| None` | `None` | | `CODEX_LB_DATABASE_SQLITE_PRE_MIGRATE_BACKUP_ENABLED` | `bool` | `True` | | `CODEX_LB_DATABASE_SQLITE_PRE_MIGRATE_BACKUP_MAX_FILES` | `int` | `5` | | `CODEX_LB_DATABASE_SQLITE_STARTUP_CHECK_MODE` | `'quick' \| 'full' \| 'off'` | `'quick'` | diff --git a/openspec/changes/support-postgres-schema-overrides/.openspec.yaml b/openspec/changes/support-postgres-schema-overrides/.openspec.yaml new file mode 100644 index 0000000000..5081c98763 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-12 diff --git a/openspec/changes/support-postgres-schema-overrides/proposal.md b/openspec/changes/support-postgres-schema-overrides/proposal.md new file mode 100644 index 0000000000..0f4a0743bc --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/proposal.md @@ -0,0 +1,36 @@ +# Change: support-postgres-schema-overrides + +## Why + +CHEK's production deployment reuses an existing PostgreSQL instance instead of +provisioning a dedicated database per application. `codex-lb` currently assumes +the PostgreSQL session operates in `public`, so runtime queries, migration +checks, and durable-bridge table guards all target the default schema. In a +shared database that can leak into the wrong schema, report missing tables even +after successful migrations, or require operators to create one database per +deployment just to isolate objects. + +## What Changes + +- Add one explicit `database_postgres_schema` setting that, when configured, + pins PostgreSQL runtime sessions and migration paths to `,public`. +- Make Alembic, startup drift checks, migration waits, and durable-bridge table + detection honor the configured active PostgreSQL schemas instead of assuming + `public`. +- Ensure migration/bootstrap paths create the configured PostgreSQL schema on + first use when the database user is allowed to do so, so shared-database + installs do not accidentally fall back to `public` Alembic state. +- Surface the setting through the Helm chart and environment examples so shared + database installs can keep the app, migration job, and readiness guards on + the same schema contract. +- Document the shared-database installation path and cover the new wiring with + focused unit tests. + +## Impact + +- Affected specs: `database-backends`, `database-migrations`, + `deployment-installation` +- Affected code: settings, database session wiring, migration helpers, + durable-bridge schema detection, Helm values/templates, and documentation +- Existing dedicated-database installs remain unchanged because the default is + unset and preserves PostgreSQL's normal search path diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md new file mode 100644 index 0000000000..fcc9960520 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: PostgreSQL runtime sessions honor an optional application schema + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string, the application MUST configure every runtime SQLAlchemy +asyncpg connection with a PostgreSQL search path of `,public`. +This requirement applies to the request-path engine, the optional background +engine, and any other runtime PostgreSQL async engine created through the +shared engine helper. When the setting is omitted or empty, PostgreSQL runtime +behavior MUST remain unchanged. + +#### Scenario: Runtime asyncpg connections search the configured schema first + +- **GIVEN** `database_url` uses `postgresql+asyncpg://` +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** the application opens a new runtime PostgreSQL connection +- **THEN** the connection uses `search_path = codex_lb_prod,public` +- **AND** unqualified table reads and writes resolve to `codex_lb_prod` before + `public` + +#### Scenario: Omitted schema preserves existing runtime behavior + +- **GIVEN** `database_url` uses `postgresql+asyncpg://` +- **AND** `database_postgres_schema` is omitted or empty +- **WHEN** the application opens a new runtime PostgreSQL connection +- **THEN** no search-path override is configured + +### Requirement: Durable bridge table guards honor the active PostgreSQL schemas + +When durable-bridge readiness or cleanup code checks whether the bridge tables +exist on PostgreSQL, it MUST resolve table presence from the connection's +currently active non-temporary schemas instead of assuming `public`. This keeps +shared-database installs compatible with a schema-specific search path while +preserving existing SQLite behavior. + +#### Scenario: Non-public schema tables satisfy the durable-bridge guard + +- **GIVEN** the database backend is PostgreSQL +- **AND** the active search path includes `codex_lb_prod,public` +- **AND** the required durable-bridge tables exist in `codex_lb_prod` +- **WHEN** the durable-bridge table guard runs +- **THEN** it reports that the tables are present +- **AND** it does not require duplicate copies in `public` diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md new file mode 100644 index 0000000000..39690b1531 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -0,0 +1,49 @@ +## ADDED Requirements + +### Requirement: Migration paths honor an optional PostgreSQL application schema + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string, every sync migration path MUST operate with a PostgreSQL +search path of `,public`. This requirement applies to the +Alembic environment, startup migration upgrade path, startup drift check, +`wait-for-head`, and `wait-for-connection` / schema-inspection helpers that use +the shared sync connection factories. When the setting is omitted or empty, +existing migration behavior MUST remain unchanged. + +#### Scenario: Startup migrations run inside the configured schema + +- **GIVEN** `database_url` resolves to PostgreSQL +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** startup migration or drift-check code opens a sync PostgreSQL + connection +- **THEN** the connection uses `search_path = codex_lb_prod,public` +- **AND** Alembic upgrades and ORM drift checks resolve unqualified schema + objects inside `codex_lb_prod` + +#### Scenario: Shared-schema installs do not require public duplicates + +- **GIVEN** PostgreSQL objects exist only in `codex_lb_prod` +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** `wait-for-head` or another schema-state check runs after migrations +- **THEN** the check succeeds against `codex_lb_prod` +- **AND** it does not report missing tables solely because `public` is empty + +### Requirement: Migration bootstrap creates the configured PostgreSQL schema before reading schema state + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string other than `public`, migration and schema-inspection paths +MUST ensure that schema exists before they read Alembic state or run upgrades. +This requirement applies to startup migration entrypoints, Alembic online +migrations, drift checks, and migration wait helpers that use the shared sync +connection factories. If the database user cannot create schemas, the failure +MUST surface directly instead of silently falling back to `public`. + +#### Scenario: First shared-schema install does not reuse public Alembic state + +- **GIVEN** `database_url` resolves to PostgreSQL +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **AND** `codex_lb_prod` does not exist yet +- **WHEN** startup migration or Alembic online migration begins +- **THEN** codex-lb creates `codex_lb_prod` before setting `search_path` +- **AND** subsequent Alembic state reads and upgrades resolve inside + `codex_lb_prod` diff --git a/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md new file mode 100644 index 0000000000..4e6292a507 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: Helm external PostgreSQL installs support an optional schema override + +The Helm chart MUST expose one optional `config.databasePostgresSchema` value +for PostgreSQL installs that reuse a shared database. When set, the rendered +runtime workload, migration job, and startup database guard init containers +MUST all receive `CODEX_LB_DATABASE_POSTGRES_SCHEMA` with the same value. When +unset, the chart MUST preserve the current no-override behavior. + +#### Scenario: Shared PostgreSQL schema is wired consistently + +- **WHEN** the chart renders with `postgresql.enabled=false` +- **AND** `config.databasePostgresSchema=codex_lb_prod` +- **THEN** the main workload receives + `CODEX_LB_DATABASE_POSTGRES_SCHEMA=codex_lb_prod` +- **AND** the migration job receives the same environment variable +- **AND** startup database init containers receive the same environment variable + +#### Scenario: Default installs remain unchanged + +- **WHEN** the chart renders without `config.databasePostgresSchema` +- **THEN** `CODEX_LB_DATABASE_POSTGRES_SCHEMA` is not emitted +- **AND** existing dedicated-database installs remain unchanged diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md new file mode 100644 index 0000000000..7d42378511 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -0,0 +1,29 @@ +# Tasks: support-postgres-schema-overrides + +## 1. Runtime and migration wiring + +- [x] 1.1 Add one normalized `database_postgres_schema` setting for shared + PostgreSQL database installs +- [x] 1.2 Configure asyncpg runtime engines to apply `,public` when the + setting is present +- [x] 1.3 Apply the same PostgreSQL search path to sync migration, Alembic, and + startup schema-check connections +- [x] 1.3.1 Create the configured PostgreSQL schema before migration/bootstrap + state reads so first installs do not fall back to `public` +- [x] 1.4 Make durable-bridge table guards resolve against the active + PostgreSQL schemas instead of hard-coding `public` + +## 2. Install contract and docs + +- [x] 2.1 Expose the schema setting through Helm values, templates, and example + overlays +- [x] 2.2 Document shared PostgreSQL installs in the Helm README and + `.env.example` +- [x] 2.3 Regenerate the checked-in settings reference page + +## 3. Verification + +- [x] 3.1 Add focused unit coverage for runtime engine kwargs, migration search + path helpers, Helm rendering, and durable-bridge schema detection +- [ ] 3.2 Run focused unit tests for the new schema wiring +- [ ] 3.3 Run `openspec validate --specs` diff --git a/tests/unit/test_db_migrate.py b/tests/unit/test_db_migrate.py index 92ab87f8ed..febebc1610 100644 --- a/tests/unit/test_db_migrate.py +++ b/tests/unit/test_db_migrate.py @@ -15,6 +15,7 @@ from alembic.util.exc import CommandError from sqlalchemy import create_engine, inspect, text from sqlalchemy import exc as sa_exc +from sqlalchemy.dialects import postgresql from sqlalchemy.engine import Connection import app.db.migrate as migrate_module @@ -34,7 +35,12 @@ wait_for_connection, wait_for_head, ) -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import ( + apply_postgres_search_path, + ensure_postgres_schema_exists, + postgres_search_path, + to_sync_database_url, +) from app.db.models import Base from app.modules.usage.additional_quota_keys import clear_additional_quota_registry_cache @@ -213,6 +219,122 @@ def test_wait_for_connection_times_out_when_database_stays_unreachable(monkeypat wait_for_connection("sqlite+aiosqlite:///tmp/test.db", timeout_seconds=2.0, interval_seconds=1.0) +def test_postgres_search_path_helper_normalizes_schema() -> None: + assert postgres_search_path(None) is None + assert postgres_search_path("") is None + assert postgres_search_path(" ") is None + assert postgres_search_path("public") == "public" + assert postgres_search_path("codex_lb_prod") == "codex_lb_prod,public" + + +def test_apply_postgres_search_path_is_noop_for_non_postgres() -> None: + class _Connection: + dialect = SimpleNamespace(name="sqlite") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("execute should not be called for non-PostgreSQL backends") + + apply_postgres_search_path(cast(Connection, _Connection()), "codex_lb_prod") + + +def test_apply_postgres_search_path_sets_transaction_scope_path_for_postgres() -> None: + calls: list[tuple[object, object]] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: object, params: object) -> None: + calls.append((statement, params)) + + connection = cast(Connection, _Connection()) + + apply_postgres_search_path(connection, "codex_lb_prod") + + assert len(calls) == 1 + _statement, params = calls[0] + assert params == {"search_path": "codex_lb_prod,public"} + + +def test_ensure_postgres_schema_exists_is_noop_for_non_postgres() -> None: + class _Connection: + dialect = SimpleNamespace(name="sqlite") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("execute should not be called for non-PostgreSQL backends") + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "codex_lb_prod") + + +def test_ensure_postgres_schema_exists_skips_public_schema() -> None: + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("public schema should not trigger CREATE SCHEMA") + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "public") + + +def test_ensure_postgres_schema_exists_creates_missing_postgres_schema() -> None: + calls: list[object] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: object) -> None: + calls.append(statement) + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "codex_lb_prod") + + assert len(calls) == 1 + statement = calls[0] + compiled = str(statement.compile(dialect=postgresql.dialect())) + assert compiled == "CREATE SCHEMA IF NOT EXISTS codex_lb_prod" + + +def test_sync_connection_creates_schema_before_search_path(monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[tuple[str, str | None]] = [] + + class _ConnectionContext: + def __enter__(self) -> Connection: + return cast(Connection, object()) + + def __exit__(self, exc_type, exc, tb) -> bool: + return False + + class _Engine: + def connect(self) -> _ConnectionContext: + return _ConnectionContext() + + def dispose(self) -> None: + return None + + monkeypatch.setattr(migrate_module, "create_engine", lambda *_args, **_kwargs: _Engine()) + monkeypatch.setattr( + migrate_module, + "get_settings", + lambda: SimpleNamespace(database_postgres_schema="codex_lb_prod"), + ) + monkeypatch.setattr( + migrate_module, + "ensure_postgres_schema_exists", + lambda _connection, schema: calls.append(("ensure", schema)), + ) + monkeypatch.setattr( + migrate_module, + "apply_postgres_search_path", + lambda _connection, schema: calls.append(("search_path", schema)), + ) + + with migrate_module._sync_connection("postgresql+psycopg://codex_lb:codex_lb@127.0.0.1:5432/codex_lb"): + pass + + assert calls == [ + ("ensure", "codex_lb_prod"), + ("search_path", "codex_lb_prod"), + ] + + def test_schema_migration_contract_matches_after_upgrade(tmp_path: Path) -> None: """Prisma-style contract: migrated schema must match ORM metadata and policy.""" db_path = tmp_path / "contract.db" diff --git a/tests/unit/test_db_session.py b/tests/unit/test_db_session.py index 7cddca872a..44dbc6a19c 100644 --- a/tests/unit/test_db_session.py +++ b/tests/unit/test_db_session.py @@ -29,6 +29,7 @@ class _FakeSettings: database_sqlite_pre_migrate_backup_enabled: bool = False database_sqlite_pre_migrate_backup_max_files: int = 5 database_sqlite_startup_check_mode: str = "quick" + database_postgres_schema: str | None = None database_migrations_fail_fast: bool = False @@ -328,6 +329,27 @@ def test_postgres_connect_args_pin_session_timezone_to_utc(monkeypatch) -> None: assert connect_args == {"server_settings": {"timezone": "UTC"}} +def test_postgres_connect_args_include_search_path_when_schema_is_configured(monkeypatch) -> None: + monkeypatch.delenv("CODEX_LB_TEST_DATABASE_URL", raising=False) + monkeypatch.setattr( + session_module, + "_settings", + _FakeSettings( + database_url="postgresql+asyncpg://u:p@h/db", + database_postgres_schema="codex_lb_prod", + ), + ) + + connect_args = session_module._postgres_async_connect_args("postgresql+asyncpg://u:p@h/db") + + assert connect_args == { + "server_settings": { + "timezone": "UTC", + "search_path": "codex_lb_prod,public", + } + } + + def test_postgres_connect_args_pin_utc_and_keep_test_db_url_tuning(monkeypatch) -> None: monkeypatch.setenv("CODEX_LB_TEST_DATABASE_URL", "1") diff --git a/tests/unit/test_durable_bridge_sessions.py b/tests/unit/test_durable_bridge_sessions.py index dd154d75c1..15455dc7ff 100644 --- a/tests/unit/test_durable_bridge_sessions.py +++ b/tests/unit/test_durable_bridge_sessions.py @@ -9,6 +9,7 @@ from unittest.mock import AsyncMock import pytest +from sqlalchemy import text from sqlalchemy import delete, select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine @@ -32,6 +33,7 @@ from app.modules.proxy.durable_bridge_repository import ( DurableBridgeAliasRegistration, DurableBridgeRepository, + missing_durable_bridge_tables, ) pytestmark = pytest.mark.unit @@ -139,6 +141,33 @@ async def test_durable_bridge_lookup_prefers_turn_state_then_previous_response_t assert by_session.canonical_key == "sid-123" +@pytest.mark.asyncio +async def test_missing_durable_bridge_tables_checks_current_postgres_schemas() -> None: + engine = create_async_engine("sqlite+aiosqlite:///:memory:") + session_maker = async_sessionmaker(engine, expire_on_commit=False) + try: + async with session_maker() as session: + captured_sql: list[str] = [] + original_execute = session.execute + + async def recording_execute(statement: object, *args: object, **kwargs: object): + if isinstance(statement, type(text(""))): + captured_sql.append(str(statement)) + return SimpleNamespace(fetchall=lambda: [("http_bridge_sessions",)]) + return await original_execute(statement, *args, **kwargs) + + session.bind.dialect.name = "postgresql" # type: ignore[assignment] + session.execute = recording_execute # type: ignore[method-assign] + + missing = await missing_durable_bridge_tables(session) + + assert "http_bridge_session_aliases" in missing + assert any("current_schemas(false)" in sql for sql in captured_sql) + assert all("table_schema = 'public'" not in sql for sql in captured_sql) + finally: + await engine.dispose() + + @pytest.mark.asyncio async def test_reversible_recovery_turn_state_registration_restores_previous_owner( coordinator: DurableBridgeSessionCoordinator, diff --git a/tests/unit/test_helm_external_secrets.py b/tests/unit/test_helm_external_secrets.py index 379e691e32..a59f22af0a 100644 --- a/tests/unit/test_helm_external_secrets.py +++ b/tests/unit/test_helm_external_secrets.py @@ -1,8 +1,10 @@ from __future__ import annotations +import os import re import shutil import subprocess +import tempfile from pathlib import Path import pytest @@ -13,6 +15,24 @@ _REPO_ROOT = Path(__file__).resolve().parents[2] _CHART_DIR = _REPO_ROOT / "deploy" / "helm" / "codex-lb" _DEPENDENCY_BUILD_COMPLETE = False +_HELM_HOME = Path(tempfile.mkdtemp(prefix="codex-lb-helm-")) +_HELM_ENV = { + "HELM_CONFIG_HOME": str(_HELM_HOME / "config"), + "HELM_CACHE_HOME": str(_HELM_HOME / "cache"), + "HELM_DATA_HOME": str(_HELM_HOME / "data"), + "HELM_REGISTRY_CONFIG": str(_HELM_HOME / "config" / "registry.json"), + "HELM_REPOSITORY_CONFIG": str(_HELM_HOME / "config" / "repositories.yaml"), + "HELM_REPOSITORY_CACHE": str(_HELM_HOME / "repository"), +} + +for path in ( + _HELM_HOME / "config", + _HELM_HOME / "cache", + _HELM_HOME / "data", + _HELM_HOME / "repository", +): + path.mkdir(parents=True, exist_ok=True) +(_HELM_HOME / "config" / "repositories.yaml").touch() def _ensure_chart_dependencies() -> None: @@ -29,6 +49,7 @@ def _ensure_chart_dependencies() -> None: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) _DEPENDENCY_BUILD_COMPLETE = True @@ -43,6 +64,7 @@ def _helm_template(*args: str) -> str: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) return completed.stdout @@ -58,6 +80,7 @@ def _helm_template_failure(*args: str) -> subprocess.CalledProcessError: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) return exc_info.value @@ -927,6 +950,20 @@ def test_external_database_url_is_rendered_into_chart_managed_secret_when_postgr assert 'database-url: "postgresql+asyncpg://user:pass@db.example.com:5432/codexlb"' in rendered +def test_database_postgres_schema_is_rendered_for_runtime_and_migration_paths() -> None: + rendered = _helm_template( + "--set", + "postgresql.enabled=false", + "--set", + "externalDatabase.url=postgresql+asyncpg://user:pass@db.example.com:5432/codexlb", + "--set", + "config.databasePostgresSchema=codex_lb_prod", + ) + + assert 'CODEX_LB_DATABASE_POSTGRES_SCHEMA: "codex_lb_prod"' in rendered + assert rendered.count("name: CODEX_LB_DATABASE_POSTGRES_SCHEMA") >= 3 + + def test_network_policy_does_not_allow_http_ingress_from_all_namespaces_by_default() -> None: rendered = _helm_template( "-f", diff --git a/tests/unit/test_settings_reference.py b/tests/unit/test_settings_reference.py index 428b5057b1..d4c129e892 100644 --- a/tests/unit/test_settings_reference.py +++ b/tests/unit/test_settings_reference.py @@ -54,7 +54,11 @@ def _isolated_settings(**overrides: Any) -> Settings: # 117 -> 118: http_responses_session_bridge_anchor_poison_failure_threshold # (bridge restart anchor poisoning). Not hardcoded because operators need a # bounded deployment-specific poison threshold while recovery telemetry matures. -MAX_SETTINGS_FIELDS = 118 +# 118 -> 119: database_postgres_schema. Shared-database deployments need one +# explicit schema knob so runtime engines, migration jobs, and schema gates use +# the same non-public search_path; there is no safe fixed default when a single +# PostgreSQL database is reused by multiple applications. +MAX_SETTINGS_FIELDS = 119 def test_generated_settings_reference_matches_code() -> None: From 83e6d8cfc8d26fce3dd5c850b667aec603b1fb3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 21:48:17 +0800 Subject: [PATCH 2/8] test: fix postgres schema CI coverage --- .all-contributorsrc | 11 ++++++ README.md | 3 ++ app/db/migration_url.py | 5 ++- tests/unit/test_db_migrate.py | 5 +-- tests/unit/test_durable_bridge_sessions.py | 39 +++++++++------------- tests/unit/test_helm_external_secrets.py | 2 +- 6 files changed, 35 insertions(+), 30 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 1a7e2eb7bb..f8898e5f97 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -1222,6 +1222,17 @@ "contributions": [ "code" ] + }, + { + "login": "hongzexin", + "name": "Jason HONG", + "avatar_url": "https://avatars.githubusercontent.com/u/136784169?v=4", + "profile": "https://github.com/hongzexin", + "contributions": [ + "code", + "test", + "doc" + ] } ], "contributorsPerLine": 7, diff --git a/README.md b/README.md index 5fa37d9099..6b0bd131aa 100644 --- a/README.md +++ b/README.md @@ -283,6 +283,9 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/e glopyglerky
glopyglerky
💻 ⚠️ Ahmad Maulana Iqbal
Ahmad Maulana Iqbal
💻 ⚠️ + + Jason HONG
Jason HONG
💻 ⚠️ 📖 + diff --git a/app/db/migration_url.py b/app/db/migration_url.py index 9dcd2aa24e..382fb889b8 100644 --- a/app/db/migration_url.py +++ b/app/db/migration_url.py @@ -1,8 +1,7 @@ from __future__ import annotations from sqlalchemy import text -from sqlalchemy.engine import Connection -from sqlalchemy.engine import make_url +from sqlalchemy.engine import Connection, make_url from sqlalchemy.schema import CreateSchema from app.db.sqlite_utils import normalize_sqlite_url @@ -33,7 +32,7 @@ def postgres_search_path(schema: str | None) -> str | None: def ensure_postgres_schema_exists(connection: Connection, schema: str | None) -> None: - if connection.dialect.name != "postgresql" or schema is None: + if schema is None or connection.dialect.name != "postgresql": return normalized = schema.strip() diff --git a/tests/unit/test_db_migrate.py b/tests/unit/test_db_migrate.py index febebc1610..4d30dfa5e7 100644 --- a/tests/unit/test_db_migrate.py +++ b/tests/unit/test_db_migrate.py @@ -17,6 +17,7 @@ from sqlalchemy import exc as sa_exc from sqlalchemy.dialects import postgresql from sqlalchemy.engine import Connection +from sqlalchemy.schema import CreateSchema import app.db.migrate as migrate_module from app.db.alembic.revision_ids import OLD_TO_NEW_REVISION_MAP @@ -276,12 +277,12 @@ def execute(self, *_args, **_kwargs) -> None: def test_ensure_postgres_schema_exists_creates_missing_postgres_schema() -> None: - calls: list[object] = [] + calls: list[CreateSchema] = [] class _Connection: dialect = SimpleNamespace(name="postgresql") - def execute(self, statement: object) -> None: + def execute(self, statement: CreateSchema) -> None: calls.append(statement) ensure_postgres_schema_exists(cast(Connection, _Connection()), "codex_lb_prod") diff --git a/tests/unit/test_durable_bridge_sessions.py b/tests/unit/test_durable_bridge_sessions.py index 15455dc7ff..69201a089a 100644 --- a/tests/unit/test_durable_bridge_sessions.py +++ b/tests/unit/test_durable_bridge_sessions.py @@ -9,7 +9,6 @@ from unittest.mock import AsyncMock import pytest -from sqlalchemy import text from sqlalchemy import delete, select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine @@ -143,29 +142,21 @@ async def test_durable_bridge_lookup_prefers_turn_state_then_previous_response_t @pytest.mark.asyncio async def test_missing_durable_bridge_tables_checks_current_postgres_schemas() -> None: - engine = create_async_engine("sqlite+aiosqlite:///:memory:") - session_maker = async_sessionmaker(engine, expire_on_commit=False) - try: - async with session_maker() as session: - captured_sql: list[str] = [] - original_execute = session.execute - - async def recording_execute(statement: object, *args: object, **kwargs: object): - if isinstance(statement, type(text(""))): - captured_sql.append(str(statement)) - return SimpleNamespace(fetchall=lambda: [("http_bridge_sessions",)]) - return await original_execute(statement, *args, **kwargs) - - session.bind.dialect.name = "postgresql" # type: ignore[assignment] - session.execute = recording_execute # type: ignore[method-assign] - - missing = await missing_durable_bridge_tables(session) - - assert "http_bridge_session_aliases" in missing - assert any("current_schemas(false)" in sql for sql in captured_sql) - assert all("table_schema = 'public'" not in sql for sql in captured_sql) - finally: - await engine.dispose() + captured_sql: list[str] = [] + + class _PostgresSession: + def get_bind(self) -> SimpleNamespace: + return SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) + + async def execute(self, statement: object) -> SimpleNamespace: + captured_sql.append(str(statement)) + return SimpleNamespace(fetchall=lambda: [("http_bridge_sessions",)]) + + missing = await missing_durable_bridge_tables(cast(AsyncSession, _PostgresSession())) + + assert "http_bridge_session_aliases" in missing + assert any("current_schemas(false)" in sql for sql in captured_sql) + assert all("table_schema = 'public'" not in sql for sql in captured_sql) @pytest.mark.asyncio diff --git a/tests/unit/test_helm_external_secrets.py b/tests/unit/test_helm_external_secrets.py index a59f22af0a..30cba23d8d 100644 --- a/tests/unit/test_helm_external_secrets.py +++ b/tests/unit/test_helm_external_secrets.py @@ -961,7 +961,7 @@ def test_database_postgres_schema_is_rendered_for_runtime_and_migration_paths() ) assert 'CODEX_LB_DATABASE_POSTGRES_SCHEMA: "codex_lb_prod"' in rendered - assert rendered.count("name: CODEX_LB_DATABASE_POSTGRES_SCHEMA") >= 3 + assert rendered.count("name: CODEX_LB_DATABASE_POSTGRES_SCHEMA") == 2 def test_network_policy_does_not_allow_http_ingress_from_all_namespaces_by_default() -> None: From 2b5ef94eba7ca42380ac8dcebb9f277835ffb5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 21:49:51 +0800 Subject: [PATCH 3/8] ci: publish CHEK internal images --- .github/workflows/internal-image.yml | 60 ++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .github/workflows/internal-image.yml diff --git a/.github/workflows/internal-image.yml b/.github/workflows/internal-image.yml new file mode 100644 index 0000000000..1386a63a60 --- /dev/null +++ b/.github/workflows/internal-image.yml @@ -0,0 +1,60 @@ +name: Internal Image + +on: + push: + branches: + - main + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + packages: write + +jobs: + publish: + name: Build and publish CHEK image + runs-on: ubuntu-24.04 + + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + persist-credentials: false + + - name: Set up QEMU + uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c + + - name: Log in to GHCR + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Extract image metadata + id: meta + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 + with: + images: ghcr.io/${{ github.repository }} + tags: | + type=sha,prefix=sha-,format=short + type=raw,value=main,enable=${{ github.ref == 'refs/heads/main' }} + + - name: Build and push image + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a + with: + context: . + file: Dockerfile + push: true + platforms: linux/amd64,linux/arm64 + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha,scope=codex-lb-internal + cache-to: type=gha,mode=max,scope=codex-lb-internal From 63737df687a2bfeaf1f120f54242682375eb1703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 22:37:16 +0800 Subject: [PATCH 4/8] fix(db): isolate postgres migration schema Address Codex review by scoping migration state and Alembic version tables to the configured schema, quoting PostgreSQL identifiers, and removing the unrelated image publisher from this PR. --- .env.example | 2 +- .github/workflows/internal-image.yml | 60 ----------------- app/db/alembic/env.py | 19 ++++-- app/db/migrate.py | 52 +++++++++------ app/db/migration_url.py | 55 ++++++++++++++-- deploy/helm/codex-lb/README.md | 2 +- .../proposal.md | 6 +- .../specs/database-backends/spec.md | 2 +- .../specs/database-migrations/spec.md | 14 +++- .../tasks.md | 10 +-- tests/integration/test_migrations.py | 43 ++++++++++++- tests/unit/test_db_migrate.py | 64 ++++++++++++++++--- tests/unit/test_db_session.py | 2 +- 13 files changed, 223 insertions(+), 108 deletions(-) delete mode 100644 .github/workflows/internal-image.yml diff --git a/.env.example b/.env.example index 9b5eae1c77..96322c1f04 100644 --- a/.env.example +++ b/.env.example @@ -15,7 +15,7 @@ # Database. SQLite (default) needs nothing. For PostgreSQL: # CODEX_LB_DATABASE_URL=postgresql+asyncpg://codex_lb:codex_lb@127.0.0.1:5432/codex_lb -# Optional shared-DB schema isolation (search_path ",public"): +# Optional shared-DB schema isolation (runtime ",public"; migrations ""): # CODEX_LB_DATABASE_POSTGRES_SCHEMA=codex_lb_prod # Encryption key file (pin for Docker volumes; must be shared across replicas). diff --git a/.github/workflows/internal-image.yml b/.github/workflows/internal-image.yml deleted file mode 100644 index 1386a63a60..0000000000 --- a/.github/workflows/internal-image.yml +++ /dev/null @@ -1,60 +0,0 @@ -name: Internal Image - -on: - push: - branches: - - main - workflow_dispatch: - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -permissions: - contents: read - packages: write - -jobs: - publish: - name: Build and publish CHEK image - runs-on: ubuntu-24.04 - - steps: - - name: Checkout repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 - with: - persist-credentials: false - - - name: Set up QEMU - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c - - - name: Log in to GHCR - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f - with: - registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - - - name: Extract image metadata - id: meta - uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 - with: - images: ghcr.io/${{ github.repository }} - tags: | - type=sha,prefix=sha-,format=short - type=raw,value=main,enable=${{ github.ref == 'refs/heads/main' }} - - - name: Build and push image - uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a - with: - context: . - file: Dockerfile - push: true - platforms: linux/amd64,linux/arm64 - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - cache-from: type=gha,scope=codex-lb-internal - cache-to: type=gha,mode=max,scope=codex-lb-internal diff --git a/app/db/alembic/env.py b/app/db/alembic/env.py index 2c9e4905bb..d35bf48bc1 100644 --- a/app/db/alembic/env.py +++ b/app/db/alembic/env.py @@ -3,10 +3,15 @@ from logging.config import fileConfig from alembic import context -from sqlalchemy import engine_from_config, pool +from sqlalchemy import engine_from_config, make_url, pool from app.core.config.settings import get_settings -from app.db.migration_url import apply_postgres_search_path, ensure_postgres_schema_exists, to_sync_database_url +from app.db.migration_url import ( + apply_postgres_migration_search_path, + ensure_postgres_schema_exists, + normalize_postgres_schema, + to_sync_database_url, +) from app.db.models import Base config = context.config @@ -27,6 +32,9 @@ def _sync_database_url() -> str: def run_migrations_offline() -> None: url = _sync_database_url() config.set_main_option("sqlalchemy.url", url) + schema = None + if make_url(url).get_backend_name() == "postgresql": + schema = normalize_postgres_schema(get_settings().database_postgres_schema) context.configure( url=url, @@ -35,6 +43,7 @@ def run_migrations_offline() -> None: dialect_opts={"paramstyle": "named"}, compare_type=True, render_as_batch=url.startswith("sqlite"), + version_table_schema=schema, ) with context.begin_transaction(): @@ -53,13 +62,15 @@ def run_migrations_online() -> None: ) with connectable.connect() as connection: - ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) - apply_postgres_search_path(connection, get_settings().database_postgres_schema) + schema = normalize_postgres_schema(get_settings().database_postgres_schema) + ensure_postgres_schema_exists(connection, schema) + apply_postgres_migration_search_path(connection, schema) context.configure( connection=connection, target_metadata=target_metadata, compare_type=True, render_as_batch=connection.dialect.name == "sqlite", + version_table_schema=schema if connection.dialect.name == "postgresql" else None, ) with context.begin_transaction(): diff --git a/app/db/migrate.py b/app/db/migrate.py index a49f884cb1..35b41a05d1 100644 --- a/app/db/migrate.py +++ b/app/db/migrate.py @@ -22,7 +22,13 @@ from app.core.config.settings import get_settings from app.db.alembic.revision_ids import LEGACY_MIGRATION_TO_NEW_REVISION, OLD_TO_NEW_REVISION_MAP, REVISION_ID_PATTERN from app.db.migration_lock import migration_lock -from app.db.migration_url import apply_postgres_search_path, ensure_postgres_schema_exists, to_sync_database_url +from app.db.migration_url import ( + apply_postgres_migration_search_path, + ensure_postgres_schema_exists, + normalize_postgres_schema, + postgres_qualified_name, + to_sync_database_url, +) from app.db.models import Base logger = logging.getLogger(__name__) @@ -179,7 +185,7 @@ def _sync_connection(sync_database_url: str) -> Iterator[Connection]: try: with engine.connect() as connection: ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) - apply_postgres_search_path(connection, get_settings().database_postgres_schema) + apply_postgres_migration_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() @@ -191,7 +197,7 @@ def _sync_transaction(sync_database_url: str) -> Iterator[Connection]: try: with engine.begin() as connection: ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) - apply_postgres_search_path(connection, get_settings().database_postgres_schema) + apply_postgres_migration_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() @@ -199,18 +205,31 @@ def _sync_transaction(sync_database_url: str) -> Iterator[Connection]: def _read_table_names(connection: Connection) -> set[str]: inspector = inspect(connection) - return set(inspector.get_table_names()) + schema = _configured_postgres_schema(connection) + return set(inspector.get_table_names(schema=schema)) + + +def _configured_postgres_schema(connection: Connection) -> str | None: + if connection.dialect.name != "postgresql": + return None + return normalize_postgres_schema(get_settings().database_postgres_schema) + + +def _migration_table_name(connection: Connection, table_name: str) -> str: + return postgres_qualified_name(table_name, _configured_postgres_schema(connection)) def _read_legacy_migration_names(connection: Connection) -> set[str]: - result = connection.execute(text(f"SELECT name FROM {_LEGACY_MIGRATIONS_TABLE}")) + table_name = _migration_table_name(connection, _LEGACY_MIGRATIONS_TABLE) + result = connection.execute(text(f"SELECT name FROM {table_name}")) names = {str(row[0]) for row in result.fetchall() if row and row[0] is not None} return names def _read_current_revisions_from_connection(connection: Connection) -> tuple[str, ...]: + table_name = _migration_table_name(connection, _ALEMBIC_VERSION_TABLE) try: - rows = connection.execute(text(f"SELECT {_ALEMBIC_VERSION_COLUMN} FROM {_ALEMBIC_VERSION_TABLE}")).fetchall() + rows = connection.execute(text(f"SELECT {_ALEMBIC_VERSION_COLUMN} FROM {table_name}")).fetchall() except (sa_exc.ProgrammingError, sa_exc.OperationalError) as exc: # PostgreSQL can still raise UndefinedTable here on a fresh database if # the alembic_version table is absent when startup migration state is @@ -359,7 +378,9 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection return inspector = inspect(connection) - if not inspector.has_table(_ALEMBIC_VERSION_TABLE): + schema = _configured_postgres_schema(connection) + table_name = postgres_qualified_name(_ALEMBIC_VERSION_TABLE, schema) + if not inspector.has_table(_ALEMBIC_VERSION_TABLE, schema=schema): # IF NOT EXISTS is defense-in-depth against concurrent out-of-band # `alembic upgrade` invocations that bypass run_upgrade's migration # lock; the product paths are already serialized by migration_lock. @@ -367,7 +388,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection text( " ".join( ( - f"CREATE TABLE IF NOT EXISTS {_ALEMBIC_VERSION_TABLE} (", + f"CREATE TABLE IF NOT EXISTS {table_name} (", f"{_ALEMBIC_VERSION_COLUMN} VARCHAR({required_length}) NOT NULL,", f"PRIMARY KEY ({_ALEMBIC_VERSION_COLUMN})", ")", @@ -377,7 +398,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection ) return - columns = inspector.get_columns(_ALEMBIC_VERSION_TABLE) + columns = inspector.get_columns(_ALEMBIC_VERSION_TABLE, schema=schema) version_num_column = next((column for column in columns if column.get("name") == _ALEMBIC_VERSION_COLUMN), None) if version_num_column is None: raise MigrationBootstrapError( @@ -389,10 +410,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection return connection.execute( - text( - f"ALTER TABLE {_ALEMBIC_VERSION_TABLE} " - f"ALTER COLUMN {_ALEMBIC_VERSION_COLUMN} TYPE VARCHAR({required_length})" - ) + text(f"ALTER TABLE {table_name} ALTER COLUMN {_ALEMBIC_VERSION_COLUMN} TYPE VARCHAR({required_length})") ) @@ -482,13 +500,11 @@ def _remap_legacy_alembic_revisions(config: Config) -> tuple[str, ...]: if remapped == current_revisions: return () - connection.execute(text(f"DELETE FROM {_ALEMBIC_VERSION_TABLE}")) + table_name = _migration_table_name(connection, _ALEMBIC_VERSION_TABLE) + connection.execute(text(f"DELETE FROM {table_name}")) for revision in remapped: connection.execute( - text( - f"INSERT INTO {_ALEMBIC_VERSION_TABLE} ({_ALEMBIC_VERSION_COLUMN}) " - f"VALUES (:{_ALEMBIC_VERSION_COLUMN})" - ), + text(f"INSERT INTO {table_name} ({_ALEMBIC_VERSION_COLUMN}) VALUES (:{_ALEMBIC_VERSION_COLUMN})"), {_ALEMBIC_VERSION_COLUMN: revision}, ) diff --git a/app/db/migration_url.py b/app/db/migration_url.py index 382fb889b8..3145ae0eaf 100644 --- a/app/db/migration_url.py +++ b/app/db/migration_url.py @@ -20,23 +20,52 @@ def to_sync_database_url(database_url: str) -> str: return parsed.render_as_string(hide_password=False) -def postgres_search_path(schema: str | None) -> str | None: +def normalize_postgres_schema(schema: str | None) -> str | None: if schema is None: return None normalized = schema.strip() if not normalized: return None - if normalized.casefold() == "public": - return "public" - return f"{normalized},public" + return normalized + + +def quote_postgres_identifier(identifier: str) -> str: + return f'"{identifier.replace(chr(34), chr(34) * 2)}"' + + +def postgres_search_path(schema: str | None) -> str | None: + normalized = normalize_postgres_schema(schema) + if normalized is None: + return None + quoted = quote_postgres_identifier(normalized) + if normalized == "public": + return quoted + return f"{quoted},public" + + +def postgres_migration_search_path(schema: str | None) -> str | None: + normalized = normalize_postgres_schema(schema) + if normalized is None: + return None + return quote_postgres_identifier(normalized) + + +def postgres_qualified_name(identifier: str, schema: str | None) -> str: + normalized = normalize_postgres_schema(schema) + quoted_identifier = quote_postgres_identifier(identifier) + if normalized is None: + return quoted_identifier + return f"{quote_postgres_identifier(normalized)}.{quoted_identifier}" def ensure_postgres_schema_exists(connection: Connection, schema: str | None) -> None: if schema is None or connection.dialect.name != "postgresql": return - normalized = schema.strip() - if not normalized or normalized.casefold() == "public": + normalized = normalize_postgres_schema(schema) + if normalized is None: + return + if normalized == "public": return connection.execute(CreateSchema(normalized, if_not_exists=True)) @@ -50,3 +79,17 @@ def apply_postgres_search_path(connection: Connection, schema: str | None) -> No text("SELECT set_config('search_path', :search_path, false)"), {"search_path": search_path}, ) + + +def apply_postgres_migration_search_path(connection: Connection, schema: str | None) -> None: + normalized = normalize_postgres_schema(schema) + search_path = postgres_migration_search_path(normalized) + if search_path is None or connection.dialect.name != "postgresql": + return + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": search_path}, + ) + # Reflection caches PostgreSQL's default schema before this deployment- + # specific search path is applied. Keep it aligned with unqualified DDL. + connection.dialect.default_schema_name = normalized diff --git a/deploy/helm/codex-lb/README.md b/deploy/helm/codex-lb/README.md index 72114fd95d..1e9a171dcd 100644 --- a/deploy/helm/codex-lb/README.md +++ b/deploy/helm/codex-lb/README.md @@ -219,7 +219,7 @@ This chart intentionally keeps migration behavior explicit by install mode. - Application pods use a schema gate initContainer when `migration.enabled=true`, `config.databaseMigrateOnStartup=false`, and `migration.schemaGate.enabled=true`. - That initContainer runs `python -m app.db.migrate wait-for-head` and blocks the app container until the database is at Alembic head. - In bundled mode, `values-bundled.yaml` enables startup migration instead of the schema gate so fresh self-contained installs do not deadlock on `helm install --wait`. -- When `config.databasePostgresSchema` is non-empty, the migration Job, schema-gate initContainers, and the main workload all use the same PostgreSQL `search_path` prefix so a shared database stays isolated by schema. +- When `config.databasePostgresSchema` is non-empty, the migration Job and schema-gate initContainers use only that schema, while the main workload keeps `public` as a read fallback. Alembic state cannot leak across schemas. This means: diff --git a/openspec/changes/support-postgres-schema-overrides/proposal.md b/openspec/changes/support-postgres-schema-overrides/proposal.md index 0f4a0743bc..fb98d139b6 100644 --- a/openspec/changes/support-postgres-schema-overrides/proposal.md +++ b/openspec/changes/support-postgres-schema-overrides/proposal.md @@ -13,13 +13,17 @@ deployment just to isolate objects. ## What Changes - Add one explicit `database_postgres_schema` setting that, when configured, - pins PostgreSQL runtime sessions and migration paths to `,public`. + pins PostgreSQL runtime sessions to `,public` and migration paths to + the configured schema only. - Make Alembic, startup drift checks, migration waits, and durable-bridge table detection honor the configured active PostgreSQL schemas instead of assuming `public`. - Ensure migration/bootstrap paths create the configured PostgreSQL schema on first use when the database user is allowed to do so, so shared-database installs do not accidentally fall back to `public` Alembic state. +- Scope migration search paths and Alembic's version table to the configured + schema only, while retaining the documented `public` fallback for runtime + application queries. - Surface the setting through the Helm chart and environment examples so shared database installs can keep the app, migration job, and readiness guards on the same schema contract. diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md index fcc9960520..2ede91e3ee 100644 --- a/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md @@ -15,7 +15,7 @@ behavior MUST remain unchanged. - **GIVEN** `database_url` uses `postgresql+asyncpg://` - **AND** `database_postgres_schema = "codex_lb_prod"` - **WHEN** the application opens a new runtime PostgreSQL connection -- **THEN** the connection uses `search_path = codex_lb_prod,public` +- **THEN** the connection uses `search_path = "codex_lb_prod",public` - **AND** unqualified table reads and writes resolve to `codex_lb_prod` before `public` diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md index 39690b1531..8ea4943b47 100644 --- a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -4,7 +4,7 @@ When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a non-empty string, every sync migration path MUST operate with a PostgreSQL -search path of `,public`. This requirement applies to the +search path containing only ``. This requirement applies to the Alembic environment, startup migration upgrade path, startup drift check, `wait-for-head`, and `wait-for-connection` / schema-inspection helpers that use the shared sync connection factories. When the setting is omitted or empty, @@ -16,7 +16,7 @@ existing migration behavior MUST remain unchanged. - **AND** `database_postgres_schema = "codex_lb_prod"` - **WHEN** startup migration or drift-check code opens a sync PostgreSQL connection -- **THEN** the connection uses `search_path = codex_lb_prod,public` +- **THEN** the connection uses a migration-only `search_path = "codex_lb_prod"` - **AND** Alembic upgrades and ORM drift checks resolve unqualified schema objects inside `codex_lb_prod` @@ -47,3 +47,13 @@ MUST surface directly instead of silently falling back to `public`. - **THEN** codex-lb creates `codex_lb_prod` before setting `search_path` - **AND** subsequent Alembic state reads and upgrades resolve inside `codex_lb_prod` + +#### Scenario: Public migration state cannot mask an uninitialized application schema + +- **GIVEN** `public.alembic_version` is already at the current head +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **AND** `codex_lb_prod` has no Alembic version table +- **WHEN** codex-lb inspects or upgrades the configured schema +- **THEN** it treats `codex_lb_prod` as uninitialized +- **AND** it creates and migrates tables in `codex_lb_prod` instead of reusing + the migration state from `public` diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md index 7d42378511..53fc0ca3da 100644 --- a/openspec/changes/support-postgres-schema-overrides/tasks.md +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -6,10 +6,12 @@ PostgreSQL database installs - [x] 1.2 Configure asyncpg runtime engines to apply `,public` when the setting is present -- [x] 1.3 Apply the same PostgreSQL search path to sync migration, Alembic, and - startup schema-check connections +- [x] 1.3 Apply a schema-only PostgreSQL search path to sync migration, Alembic, + and startup schema-check connections - [x] 1.3.1 Create the configured PostgreSQL schema before migration/bootstrap state reads so first installs do not fall back to `public` +- [x] 1.3.2 Scope migration state and Alembic's version table to the configured + schema without the runtime `public` fallback - [x] 1.4 Make durable-bridge table guards resolve against the active PostgreSQL schemas instead of hard-coding `public` @@ -25,5 +27,5 @@ - [x] 3.1 Add focused unit coverage for runtime engine kwargs, migration search path helpers, Helm rendering, and durable-bridge schema detection -- [ ] 3.2 Run focused unit tests for the new schema wiring -- [ ] 3.3 Run `openspec validate --specs` +- [x] 3.2 Run focused unit tests for the new schema wiring +- [x] 3.3 Run `openspec validate --specs` diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index b20c38dbb9..40d64e69d4 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -1,10 +1,11 @@ from __future__ import annotations from collections.abc import Callable +from uuid import uuid4 import pytest from anyio import to_thread -from sqlalchemy import text +from sqlalchemy import create_engine, inspect, text from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.core.auth import DEFAULT_PLAN @@ -34,6 +35,7 @@ run_startup_migrations, run_upgrade, ) +from app.db.migration_url import to_sync_database_url from app.db.models import Account, AccountStatus from app.db.session import SessionLocal from app.modules.accounts.repository import AccountsRepository @@ -55,6 +57,45 @@ def _is_postgresql_database_url(url: str) -> bool: return url.startswith("postgresql+") +@pytest.mark.skipif(not _is_postgresql_database_url(_DATABASE_URL), reason="PostgreSQL-only schema isolation") +def test_configured_schema_does_not_reuse_public_alembic_state(monkeypatch: pytest.MonkeyPatch) -> None: + schema = f"codex_lb_test_{uuid4().hex}" + sync_url = to_sync_database_url(_DATABASE_URL) + + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + public_result = run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + + monkeypatch.setenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", schema) + get_settings.cache_clear() + try: + state_before = inspect_migration_state(_DATABASE_URL) + assert state_before.current_revision is None + assert state_before.has_alembic_version_table is False + assert state_before.needs_upgrade is True + + schema_result = run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + assert schema_result.current_revision == public_result.current_revision + + engine = create_engine(sync_url, future=True) + try: + with engine.connect() as connection: + inspector = inspect(connection) + assert inspector.has_table("alembic_version", schema=schema) + assert inspector.has_table("accounts", schema=schema) + finally: + engine.dispose() + finally: + engine = create_engine(sync_url, future=True) + try: + with engine.begin() as connection: + connection.execute(text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + finally: + engine.dispose() + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + + def _make_account(account_id: str, email: str, plan_type: str) -> Account: encryptor = TokenEncryptor() return Account( diff --git a/tests/unit/test_db_migrate.py b/tests/unit/test_db_migrate.py index 4d30dfa5e7..f54c137769 100644 --- a/tests/unit/test_db_migrate.py +++ b/tests/unit/test_db_migrate.py @@ -37,8 +37,10 @@ wait_for_head, ) from app.db.migration_url import ( + apply_postgres_migration_search_path, apply_postgres_search_path, ensure_postgres_schema_exists, + postgres_migration_search_path, postgres_search_path, to_sync_database_url, ) @@ -224,8 +226,10 @@ def test_postgres_search_path_helper_normalizes_schema() -> None: assert postgres_search_path(None) is None assert postgres_search_path("") is None assert postgres_search_path(" ") is None - assert postgres_search_path("public") == "public" - assert postgres_search_path("codex_lb_prod") == "codex_lb_prod,public" + assert postgres_search_path("public") == '"public"' + assert postgres_search_path("codex_lb_prod") == '"codex_lb_prod",public' + assert postgres_search_path('tenant"blue') == '"tenant""blue",public' + assert postgres_migration_search_path("codex_lb_prod") == '"codex_lb_prod"' def test_apply_postgres_search_path_is_noop_for_non_postgres() -> None: @@ -253,7 +257,25 @@ def execute(self, statement: object, params: object) -> None: assert len(calls) == 1 _statement, params = calls[0] - assert params == {"search_path": "codex_lb_prod,public"} + assert params == {"search_path": '"codex_lb_prod",public'} + + +def test_apply_postgres_migration_search_path_excludes_public_fallback() -> None: + calls: list[tuple[object, object]] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: object, params: object) -> None: + calls.append((statement, params)) + + connection = cast(Connection, _Connection()) + apply_postgres_migration_search_path(connection, "codex_lb_prod") + + assert len(calls) == 1 + _statement, params = calls[0] + assert params == {"search_path": '"codex_lb_prod"'} + assert connection.dialect.default_schema_name == "codex_lb_prod" def test_ensure_postgres_schema_exists_is_noop_for_non_postgres() -> None: @@ -323,7 +345,7 @@ def dispose(self) -> None: ) monkeypatch.setattr( migrate_module, - "apply_postgres_search_path", + "apply_postgres_migration_search_path", lambda _connection, schema: calls.append(("search_path", schema)), ) @@ -336,6 +358,26 @@ def dispose(self) -> None: ] +def test_read_table_names_scopes_postgres_inspection_to_configured_schema(monkeypatch) -> None: + calls: list[str | None] = [] + + class _Inspector: + def get_table_names(self, *, schema: str | None) -> list[str]: + calls.append(schema) + return ["alembic_version"] + + connection = cast(Connection, SimpleNamespace(dialect=SimpleNamespace(name="postgresql"))) + monkeypatch.setattr(migrate_module, "inspect", lambda _connection: _Inspector()) + monkeypatch.setattr( + migrate_module, + "get_settings", + lambda: SimpleNamespace(database_postgres_schema="codex_lb_prod"), + ) + + assert migrate_module._read_table_names(connection) == {"alembic_version"} + assert calls == ["codex_lb_prod"] + + def test_schema_migration_contract_matches_after_upgrade(tmp_path: Path) -> None: """Prisma-style contract: migrated schema must match ORM metadata and policy.""" db_path = tmp_path / "contract.db" @@ -2106,6 +2148,8 @@ def execute(self, statement: object) -> None: class _MissingAlembicVersionConnection: + dialect = SimpleNamespace(name="postgresql") + def execute(self, statement: object) -> None: raise sa_exc.ProgrammingError( str(statement), @@ -2115,6 +2159,8 @@ def execute(self, statement: object) -> None: class _MissingAlembicVersionSQLiteConnection: + dialect = SimpleNamespace(name="sqlite") + def execute(self, statement: object) -> None: raise sa_exc.OperationalError( str(statement), @@ -2128,12 +2174,14 @@ def __init__(self, *, has_table: bool, version_num_length: int | None = None) -> self._has_table = has_table self._version_num_length = version_num_length - def has_table(self, table_name: str) -> bool: + def has_table(self, table_name: str, *, schema: str | None = None) -> bool: assert table_name == "alembic_version" + assert schema is None return self._has_table - def get_columns(self, table_name: str) -> list[dict[str, object]]: + def get_columns(self, table_name: str, *, schema: str | None = None) -> list[dict[str, object]]: assert table_name == "alembic_version" + assert schema is None return [ { "name": "version_num", @@ -2150,7 +2198,7 @@ def test_ensure_alembic_version_table_capacity_creates_table_when_missing(monkey _ensure_alembic_version_table_capacity_for_connection(cast(Connection, connection), required_length=64) assert connection.executed_sql == [ - "CREATE TABLE IF NOT EXISTS alembic_version ( version_num VARCHAR(64) NOT NULL, PRIMARY KEY (version_num) )" + 'CREATE TABLE IF NOT EXISTS "alembic_version" ( version_num VARCHAR(64) NOT NULL, PRIMARY KEY (version_num) )' ] @@ -2161,7 +2209,7 @@ def test_ensure_alembic_version_table_capacity_alters_short_column(monkeypatch) _ensure_alembic_version_table_capacity_for_connection(cast(Connection, connection), required_length=64) - assert connection.executed_sql == ["ALTER TABLE alembic_version ALTER COLUMN version_num TYPE VARCHAR(64)"] + assert connection.executed_sql == ['ALTER TABLE "alembic_version" ALTER COLUMN version_num TYPE VARCHAR(64)'] def test_read_current_revisions_returns_empty_when_alembic_version_table_is_missing() -> None: diff --git a/tests/unit/test_db_session.py b/tests/unit/test_db_session.py index 44dbc6a19c..3792f1cdf0 100644 --- a/tests/unit/test_db_session.py +++ b/tests/unit/test_db_session.py @@ -345,7 +345,7 @@ def test_postgres_connect_args_include_search_path_when_schema_is_configured(mon assert connect_args == { "server_settings": { "timezone": "UTC", - "search_path": "codex_lb_prod,public", + "search_path": '"codex_lb_prod",public', } } From 3f6b2a1536ada670376cf3089749e9418f2aaf2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 22:46:28 +0800 Subject: [PATCH 5/8] fix(db): commit and scope postgres schema setup Address Codex review by committing schema/search_path setup before Alembic transaction ownership and constraining historical enum catalog probes to the active schema. --- app/db/alembic/env.py | 4 ++++ ...20260310_000000_fix_postgresql_enum_value_casing.py | 4 +++- ...260604_000000_add_reauth_required_account_status.py | 4 +++- .../specs/database-migrations/spec.md | 2 ++ .../changes/support-postgres-schema-overrides/tasks.md | 2 ++ tests/integration/test_migrations.py | 10 ++++++++++ 6 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/db/alembic/env.py b/app/db/alembic/env.py index d35bf48bc1..917b6e5f74 100644 --- a/app/db/alembic/env.py +++ b/app/db/alembic/env.py @@ -65,6 +65,10 @@ def run_migrations_online() -> None: schema = normalize_postgres_schema(get_settings().database_postgres_schema) ensure_postgres_schema_exists(connection, schema) apply_postgres_migration_search_path(connection, schema) + if connection.dialect.name == "postgresql" and connection.in_transaction(): + # Schema creation and set_config autobegin a transaction. Commit it + # before Alembic owns transaction boundaries and autocommit blocks. + connection.commit() context.configure( connection=connection, target_metadata=target_metadata, diff --git a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py index e5b34ca550..bb38bcdffa 100644 --- a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py +++ b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py @@ -43,7 +43,9 @@ def _enum_value_exists(bind: sa.engine.Connection, enum_type_name: str, enum_val sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "WHERE t.typname = :type_name AND e.enumlabel = :value" + "JOIN pg_namespace n ON t.typnamespace = n.oid " + "WHERE n.nspname = current_schema() " + "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, ) diff --git a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py index 4b6d686890..aa0fe0459c 100644 --- a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py +++ b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py @@ -73,7 +73,9 @@ def _enum_value_exists(enum_type_name: str, enum_value: str) -> bool: sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "WHERE t.typname = :type_name AND e.enumlabel = :value" + "JOIN pg_namespace n ON t.typnamespace = n.oid " + "WHERE n.nspname = current_schema() " + "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, ) diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md index 8ea4943b47..b4738f462f 100644 --- a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -57,3 +57,5 @@ MUST surface directly instead of silently falling back to `public`. - **THEN** it treats `codex_lb_prod` as uninitialized - **AND** it creates and migrates tables in `codex_lb_prod` instead of reusing the migration state from `public` +- **AND** PostgreSQL catalog probes for enum types resolve only against + `codex_lb_prod` diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md index 53fc0ca3da..f45a930e32 100644 --- a/openspec/changes/support-postgres-schema-overrides/tasks.md +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -12,6 +12,8 @@ state reads so first installs do not fall back to `public` - [x] 1.3.2 Scope migration state and Alembic's version table to the configured schema without the runtime `public` fallback +- [x] 1.3.3 Commit PostgreSQL schema setup before Alembic owns transaction + boundaries and scope historical enum catalog probes to the active schema - [x] 1.4 Make durable-bridge table guards resolve against the active PostgreSQL schemas instead of hard-coding `public` diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index 40d64e69d4..0a616fbdeb 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -83,6 +83,16 @@ def test_configured_schema_does_not_reuse_public_alembic_state(monkeypatch: pyte inspector = inspect(connection) assert inspector.has_table("alembic_version", schema=schema) assert inspector.has_table("accounts", schema=schema) + enum_values = connection.execute( + text( + "SELECT e.enumlabel FROM pg_enum e " + "JOIN pg_type t ON e.enumtypid = t.oid " + "JOIN pg_namespace n ON t.typnamespace = n.oid " + "WHERE n.nspname = :schema AND t.typname = 'account_status'" + ), + {"schema": schema}, + ).scalars() + assert "reauth_required" in set(enum_values) finally: engine.dispose() finally: From 50ade0d76331a05b1dc0ad1c7e54375e6727edad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 22:57:09 +0800 Subject: [PATCH 6/8] fix(db): follow postgres enum visibility Address Codex review by resolving historical enum probes through pg_type_is_visible so configured and default search paths both target the actually visible type. --- ...000000_fix_postgresql_enum_value_casing.py | 3 +- ...0000_add_reauth_required_account_status.py | 3 +- .../specs/database-migrations/spec.md | 13 +++++++-- .../tasks.md | 2 +- tests/integration/test_migrations.py | 28 +++++++++++++++++++ 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py index bb38bcdffa..95a32f61b6 100644 --- a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py +++ b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py @@ -43,8 +43,7 @@ def _enum_value_exists(bind: sa.engine.Connection, enum_type_name: str, enum_val sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "JOIN pg_namespace n ON t.typnamespace = n.oid " - "WHERE n.nspname = current_schema() " + "WHERE pg_type_is_visible(t.oid) " "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, diff --git a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py index aa0fe0459c..04b9c1e5b3 100644 --- a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py +++ b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py @@ -73,8 +73,7 @@ def _enum_value_exists(enum_type_name: str, enum_value: str) -> bool: sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "JOIN pg_namespace n ON t.typnamespace = n.oid " - "WHERE n.nspname = current_schema() " + "WHERE pg_type_is_visible(t.oid) " "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md index b4738f462f..e407b6038f 100644 --- a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -57,5 +57,14 @@ MUST surface directly instead of silently falling back to `public`. - **THEN** it treats `codex_lb_prod` as uninitialized - **AND** it creates and migrates tables in `codex_lb_prod` instead of reusing the migration state from `public` -- **AND** PostgreSQL catalog probes for enum types resolve only against - `codex_lb_prod` +- **AND** PostgreSQL catalog probes use the enum type visible through the active + migration search path + +#### Scenario: Existing default search paths retain visible public enum probes + +- **GIVEN** no schema override is configured +- **AND** a user schema precedes `public` on the default search path +- **AND** the visible codex-lb enum type exists in `public` +- **WHEN** a historical migration probes that enum's labels +- **THEN** it resolves the visible `public` type rather than assuming the first + schema owns the type diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md index f45a930e32..f1908d903d 100644 --- a/openspec/changes/support-postgres-schema-overrides/tasks.md +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -13,7 +13,7 @@ - [x] 1.3.2 Scope migration state and Alembic's version table to the configured schema without the runtime `public` fallback - [x] 1.3.3 Commit PostgreSQL schema setup before Alembic owns transaction - boundaries and scope historical enum catalog probes to the active schema + boundaries and scope historical enum catalog probes to the visible type - [x] 1.4 Make durable-bridge table guards resolve against the active PostgreSQL schemas instead of hard-coding `public` diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index 0a616fbdeb..f39609e68f 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -1,5 +1,6 @@ from __future__ import annotations +import importlib from collections.abc import Callable from uuid import uuid4 @@ -106,6 +107,33 @@ def test_configured_schema_does_not_reuse_public_alembic_state(monkeypatch: pyte get_settings.cache_clear() +@pytest.mark.skipif(not _is_postgresql_database_url(_DATABASE_URL), reason="PostgreSQL-only enum visibility") +def test_enum_probe_follows_visible_public_type_without_schema_override(monkeypatch: pytest.MonkeyPatch) -> None: + schema = f"codex_lb_path_{uuid4().hex}" + sync_url = to_sync_database_url(_DATABASE_URL) + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + + engine = create_engine(sync_url, future=True) + try: + with engine.begin() as connection: + connection.execute(text(f'CREATE SCHEMA "{schema}"')) + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": f'"{schema}",public'}, + ) + migration = importlib.import_module( + "app.db.alembic.versions.20260310_000000_fix_postgresql_enum_value_casing" + ) + + assert migration._enum_value_exists(connection, "account_status", "active") is True + finally: + with engine.begin() as connection: + connection.execute(text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + engine.dispose() + + def _make_account(account_id: str, email: str, plan_type: str) -> Account: encryptor = TokenEncryptor() return Account( From c36aed9b9632305ceb640cb8f01240db90f2f875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 23:06:18 +0800 Subject: [PATCH 7/8] docs(db): move shared schema guidance to docs Source sidechat: 019dc7df-46ed-71f0-84cf-0cb475d1857f --- deploy/helm/codex-lb/README.md | 24 ++------------ docs/deployment/kubernetes.md | 33 +++++++++++++++++++ .../tasks.md | 2 +- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/deploy/helm/codex-lb/README.md b/deploy/helm/codex-lb/README.md index 1e9a171dcd..b39326bec0 100644 --- a/deploy/helm/codex-lb/README.md +++ b/deploy/helm/codex-lb/README.md @@ -76,15 +76,13 @@ Supported DB wiring: - `externalDatabase.host`, `externalDatabase.port`, `externalDatabase.database`, `externalDatabase.user` - `externalDatabase.existingSecret` - `auth.existingSecret` if one secret contains both `database-url` and `encryption-key` -- `config.databasePostgresSchema` when you share one PostgreSQL database and want codex-lb isolated in its own schema Example using a direct URL: ```bash helm install codex-lb oci://ghcr.io/soju06/charts/codex-lb \ --set postgresql.enabled=false \ - --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' \ - --set config.databasePostgresSchema=codex_lb_prod + --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' ``` Example using separate secrets: @@ -102,8 +100,7 @@ helm install codex-lb oci://ghcr.io/soju06/charts/codex-lb \ ```bash helm upgrade --install codex-lb deploy/helm/codex-lb/ \ -f deploy/helm/codex-lb/values-external-db.yaml \ - --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' \ - --set config.databasePostgresSchema=codex_lb_prod + --set externalDatabase.url='postgresql+asyncpg://user:pass@db.example.com:5432/codexlb' ``` @@ -219,7 +216,7 @@ This chart intentionally keeps migration behavior explicit by install mode. - Application pods use a schema gate initContainer when `migration.enabled=true`, `config.databaseMigrateOnStartup=false`, and `migration.schemaGate.enabled=true`. - That initContainer runs `python -m app.db.migrate wait-for-head` and blocks the app container until the database is at Alembic head. - In bundled mode, `values-bundled.yaml` enables startup migration instead of the schema gate so fresh self-contained installs do not deadlock on `helm install --wait`. -- When `config.databasePostgresSchema` is non-empty, the migration Job and schema-gate initContainers use only that schema, while the main workload keeps `public` as a read fallback. Alembic state cannot leak across schemas. +- Shared PostgreSQL installs can isolate codex-lb in an application schema; see the [Kubernetes deployment guide](https://soju06.github.io/codex-lb/deployment/kubernetes/#shared-postgresql-databases). This means: @@ -244,21 +241,6 @@ Use `externalDatabase.existingSecret` for the database URL and let the chart man When `externalDatabase.existingSecret` is set and `auth.existingSecret` is not, the chart-managed app secret contains only the encryption key; the StatefulSet reads `CODEX_LB_DATABASE_URL` from the external DB secret. -## Shared PostgreSQL Databases - -If your platform shares one PostgreSQL database across multiple applications, set `config.databasePostgresSchema` so codex-lb uses its own schema while continuing to keep `public` on the search path for standard PostgreSQL behavior: - -```yaml -config: - databasePostgresSchema: codex_lb_prod -``` - -On the first migration run codex-lb attempts `CREATE SCHEMA IF NOT EXISTS` -before it reads or advances Alembic state. If your deployment user is not -allowed to create schemas, pre-create the schema once and keep using the same -`config.databasePostgresSchema` value for the workload, migration Job, and -schema gate. - ## Network Policy When `networkPolicy.enabled=true`, the chart now fails closed for the main HTTP ingress port. diff --git a/docs/deployment/kubernetes.md b/docs/deployment/kubernetes.md index 6031e1978f..1cb2545532 100644 --- a/docs/deployment/kubernetes.md +++ b/docs/deployment/kubernetes.md @@ -12,6 +12,39 @@ kubectl port-forward svc/codex-lb 2455:2455 Open [localhost:2455](http://localhost:2455) → Add account → Done. +## Shared PostgreSQL databases + +For an external PostgreSQL database shared by multiple applications, assign +codex-lb its own schema with `config.databasePostgresSchema`: + +```yaml +postgresql: + enabled: false +externalDatabase: + existingSecret: codex-lb-app-secret +config: + databasePostgresSchema: codex_lb_prod +``` + +The runtime searches `codex_lb_prod` first and retains `public` as a read +fallback. Migration jobs and schema-gate init containers operate only in +`codex_lb_prod`, so another application's `public.alembic_version` cannot make +an uninitialized codex-lb schema appear current. + +On the first migration run, codex-lb executes `CREATE SCHEMA IF NOT EXISTS` +before inspecting or advancing Alembic state. If the deployment user cannot +create schemas, create `codex_lb_prod` once as a database administrator. Keep +the same `config.databasePostgresSchema` value for the workload, migration job, +and schema gate; the chart wires it to all three paths. + +The setting is optional. Omitting it preserves PostgreSQL's existing default +search-path behavior. See the owning +[database-backends](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/database-backends), +[database-migrations](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/database-migrations), +and +[deployment-installation](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/deployment-installation) +contracts. + ## Multi-replica behavior The Helm chart auto-configures HTTP `/responses` owner handoff for multi-replica installs using a headless-service DNS name per pod. The default cluster domain is `cluster.local`; set Helm `clusterDomain` if your cluster uses a different suffix. Override `config.sessionBridgeAdvertiseBaseUrl` only if pods must be reached through a different internal address. diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md index f1908d903d..2f0a5164f2 100644 --- a/openspec/changes/support-postgres-schema-overrides/tasks.md +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -21,7 +21,7 @@ - [x] 2.1 Expose the schema setting through Helm values, templates, and example overlays -- [x] 2.2 Document shared PostgreSQL installs in the Helm README and +- [x] 2.2 Document shared PostgreSQL installs in the Kubernetes guide and `.env.example` - [x] 2.3 Regenerate the checked-in settings reference page From 5cbd43b610d93535c8d1cdb6680caecf9e8b3744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Wed, 12 Aug 2026 23:15:59 +0800 Subject: [PATCH 8/8] fix(db): validate schema and scope offline migrations Source sidechat: 019dc7df-46ed-71f0-84cf-0cb475d1857f --- app/core/config/settings.py | 6 ++++- app/db/alembic/env.py | 5 ++++ .../specs/database-migrations/spec.md | 17 ++++++++++++++ .../tasks.md | 2 ++ tests/unit/test_db_migrate.py | 23 +++++++++++++++++++ tests/unit/test_settings_postgres.py | 20 ++++++++++++++++ 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_settings_postgres.py diff --git a/app/core/config/settings.py b/app/core/config/settings.py index 7c24b5dfd3..b05907572f 100644 --- a/app/core/config/settings.py +++ b/app/core/config/settings.py @@ -499,7 +499,11 @@ def _normalize_database_postgres_schema(cls, value: OptionalStringInput) -> str return None if isinstance(value, str): stripped = value.strip() - return stripped or None + if not stripped: + return None + if len(stripped.encode("utf-8")) > 63: + raise ValueError("database_postgres_schema must be at most 63 UTF-8 bytes") + return stripped raise TypeError("database_postgres_schema must be a string") @field_validator("encryption_key_file", mode="before") diff --git a/app/db/alembic/env.py b/app/db/alembic/env.py index 917b6e5f74..2d3fce6627 100644 --- a/app/db/alembic/env.py +++ b/app/db/alembic/env.py @@ -10,6 +10,7 @@ apply_postgres_migration_search_path, ensure_postgres_schema_exists, normalize_postgres_schema, + postgres_migration_search_path, to_sync_database_url, ) from app.db.models import Base @@ -46,6 +47,10 @@ def run_migrations_offline() -> None: version_table_schema=schema, ) + search_path = postgres_migration_search_path(schema) + if search_path is not None: + context.execute(f"SET search_path TO {search_path}") + with context.begin_transaction(): context.run_migrations() diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md index e407b6038f..0b45ab941c 100644 --- a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -10,6 +10,10 @@ Alembic environment, startup migration upgrade path, startup drift check, the shared sync connection factories. When the setting is omitted or empty, existing migration behavior MUST remain unchanged. +Configured PostgreSQL schema names MUST NOT exceed PostgreSQL's 63-byte UTF-8 +identifier limit. Alembic offline SQL generation MUST emit a safely quoted, +schema-only search path before migration operations. + #### Scenario: Startup migrations run inside the configured schema - **GIVEN** `database_url` resolves to PostgreSQL @@ -28,6 +32,19 @@ existing migration behavior MUST remain unchanged. - **THEN** the check succeeds against `codex_lb_prod` - **AND** it does not report missing tables solely because `public` is empty +#### Scenario: Offline migration SQL targets only the configured schema + +- **GIVEN** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** Alembic generates an offline upgrade script +- **THEN** the script sets `search_path` to the safely quoted configured schema +- **AND** migration operations do not fall back to `public` + +#### Scenario: Overlong PostgreSQL schema names fail during configuration + +- **GIVEN** `database_postgres_schema` exceeds 63 bytes when UTF-8 encoded +- **WHEN** application settings are loaded +- **THEN** validation fails before PostgreSQL can truncate the identifier + ### Requirement: Migration bootstrap creates the configured PostgreSQL schema before reading schema state When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md index 2f0a5164f2..409e618d75 100644 --- a/openspec/changes/support-postgres-schema-overrides/tasks.md +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -14,6 +14,8 @@ schema without the runtime `public` fallback - [x] 1.3.3 Commit PostgreSQL schema setup before Alembic owns transaction boundaries and scope historical enum catalog probes to the visible type +- [x] 1.3.4 Reject PostgreSQL schema names beyond the 63-byte identifier limit + and emit the schema-only search path in offline Alembic SQL - [x] 1.4 Make durable-bridge table guards resolve against the active PostgreSQL schemas instead of hard-coding `public` diff --git a/tests/unit/test_db_migrate.py b/tests/unit/test_db_migrate.py index f54c137769..9b1589eb93 100644 --- a/tests/unit/test_db_migrate.py +++ b/tests/unit/test_db_migrate.py @@ -5,6 +5,7 @@ import sqlite3 import sys from datetime import datetime, timedelta, timezone +from io import StringIO from pathlib import Path from types import SimpleNamespace from typing import cast @@ -232,6 +233,28 @@ def test_postgres_search_path_helper_normalizes_schema() -> None: assert postgres_migration_search_path("codex_lb_prod") == '"codex_lb_prod"' +def test_postgres_offline_migration_sets_quoted_schema_only_search_path( + monkeypatch: pytest.MonkeyPatch, +) -> None: + output = StringIO() + config = _build_alembic_config("postgresql+asyncpg://user:pass@db.example.com:5432/codex_lb") + config.output_buffer = output + monkeypatch.setenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", 'tenant"blue') + from app.core.config.settings import get_settings + + get_settings.cache_clear() + try: + # An empty head:head range still executes env.py without asking older + # data-aware revisions to reflect from Alembic's offline mock bind. + command.upgrade(config, "head:head", sql=True) + finally: + get_settings.cache_clear() + + generated_sql = output.getvalue() + assert 'SET search_path TO "tenant""blue";' in generated_sql + assert 'SET search_path TO "tenant""blue",public;' not in generated_sql + + def test_apply_postgres_search_path_is_noop_for_non_postgres() -> None: class _Connection: dialect = SimpleNamespace(name="sqlite") diff --git a/tests/unit/test_settings_postgres.py b/tests/unit/test_settings_postgres.py new file mode 100644 index 0000000000..507ac297b8 --- /dev/null +++ b/tests/unit/test_settings_postgres.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from app.core.config.settings import Settings + + +def test_database_postgres_schema_accepts_postgres_identifier_byte_limit() -> None: + schema = "a" * 63 + + settings = Settings(_env_file=None, database_postgres_schema=schema) + + assert settings.database_postgres_schema == schema + + +@pytest.mark.parametrize("schema", ["a" * 64, "数" * 22]) +def test_database_postgres_schema_rejects_more_than_63_utf8_bytes(schema: str) -> None: + with pytest.raises(ValidationError, match="at most 63 UTF-8 bytes"): + Settings(_env_file=None, database_postgres_schema=schema)