diff --git a/.github/workflows/app-ci.yml b/.github/workflows/app-ci.yml index 4934de58c..08167eea8 100644 --- a/.github/workflows/app-ci.yml +++ b/.github/workflows/app-ci.yml @@ -24,6 +24,24 @@ jobs: env: PYTHONWARNINGS: error DISABLE_BACKGROUND_WORKERS: "1" + DATABASE_URL: postgresql+asyncpg://test:test@localhost:5432/test_db + services: + postgres: + # Bundles the pgvector extension so backend/scripts/bootstrap_db.py's + # `CREATE EXTENSION IF NOT EXISTS vector` needs no separate install + # step; matches the blessed local stack in docker-compose.yml. + image: pgvector/pgvector:pg16@sha256:ccc6e83d6e35e931dc7c5def2022729d5a6c370318d099181995567ff1fb4d6b + env: + POSTGRES_USER: test + POSTGRES_PASSWORD: test + POSTGRES_DB: test_db + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U test -d test_db" + --health-interval=5s + --health-timeout=5s + --health-retries=10 steps: - name: Harden the runner (Audit all outbound calls) uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 diff --git a/AGENTS.md b/AGENTS.md index 9104dd1f4..e934d7e8f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -623,6 +623,13 @@ in this repo. (`created_at`, `observed_at`, `parse_content_type`, `parser_key`). Under asyncpg, `INSERT ... SELECT`/`UNION` parameters default to `text`, so cast integer FK params explicitly (`CAST(:email_id AS INTEGER)`). +- The `backend` CI job runs these real-Postgres tests against an actual + `pgvector/pgvector:pg16` `services:` container (`.github/workflows/app-ci.yml`), + not a soft-skip: a broken seeding helper or migration now fails the job + instead of silently `pytest.skip`ping with "PostgreSQL smoke database/path + unavailable". Reproduce locally with any Postgres 16 + pgvector instance + (the `docker-compose.yml` `db` service works) before assuming a change is + green. - Postgres smoke seeding of `EncryptedString` columns (`credentials_encrypted`, provider `api_key`, runner tokens) must set a Fernet `ENCRYPTION_KEY` for the test (monkeypatch `settings.ENCRYPTION_KEY`) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec84c36f..d648725a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,26 @@ ## [Unreleased] +- **(CI ์ธํ”„๋ผ, ๐Ÿ”ด critical) `backend` job์— Postgres ์„œ๋น„์Šค ์ปจํ…Œ์ด๋„ˆ๊ฐ€ ์—†์–ด `@pytest.mark.postgres` + real-Postgres ํ…Œ์ŠคํŠธ๊ฐ€ CI์—์„œ ๋‹จ ํ•œ ๋ฒˆ๋„ ์‹ค์ œ๋กœ ์‹คํ–‰๋˜์ง€ ์•Š๊ณ  ํ•ญ์ƒ ์กฐ์šฉํžˆ skip๋˜๋˜ ๋ฌธ์ œ๋ฅผ + ๊ณ ์ณค์Šต๋‹ˆ๋‹ค.** `.github/workflows/app-ci.yml`์˜ `backend` job์— `pgvector/pgvector:pg16` + ์„œ๋น„์Šค ์ปจํ…Œ์ด๋„ˆ(`test`/`test`/`test_db`, `pg_isready` ํ—ฌ์Šค์ฒดํฌ)๋ฅผ ์ถ”๊ฐ€ํ•˜๊ณ  `DATABASE_URL`์„ + ๊ฐ™์€ ์ž๊ฒฉ์ฆ๋ช…์œผ๋กœ ์„ค์ •ํ•ด, `tests/conftest.py`์˜ ๊ธฐ๋ณธ๊ฐ’๊ณผ ๊ทธ๋Œ€๋กœ ๋งž๋ฌผ๋ฆฌ๋„๋ก ํ–ˆ์Šต๋‹ˆ๋‹ค. ์‹ค์ œ๋กœ + CI ํ™˜๊ฒฝ๊ณผ ๋™์ผํ•˜๊ฒŒ(๋กœ์ปฌ PostgreSQL 16 + pgvector) ์ฒ˜์Œ ๋Œ๋ ค๋ณด์ž, ์™„์ „ํžˆ ์ƒˆ ๋ฐ์ดํ„ฐ๋ฒ ์ด์Šค์— ๋Œ€ํ•œ + `alembic upgrade head`๊ฐ€ `0011_email_read_state`์—์„œ ๋ ˆ๊ฑฐ์‹œ `emails` ํ…Œ์ด๋ธ”์„ ์ง์ ‘ ๋Œ€์ƒ์œผ๋กœ + ํ•ด `relation "emails" does not exist`๋กœ ๊นจ์ง€๋Š” ๊ฒฐํ•จ๊ณผ, `tests/test_bootstrap_db.py`/ + `tests/test_data_api.py`์˜ raw SQL `INSERT INTO email_records`๊ฐ€ `is_read`(ORM ์ชฝ + Python-side `default=True`๋ฟ, DB ์„œ๋ฒ„์ธก default ์—†์Œ)๋ฅผ ๋น ๋œจ๋ ค real Postgres์—์„œ + `NotNullViolationError`๋กœ ํ•˜๋“œ ์‹คํŒจํ•˜๋Š” ๊ฒฐํ•จ์ด ํ•จ๊ป˜ ๋“œ๋Ÿฌ๋‚ฌ์Šต๋‹ˆ๋‹ค. `#1503`์ด ๋™์ผํ•œ ๊ทผ๋ณธ + ์›์ธ์„ ๋…๋ฆฝ์ ์œผ๋กœ ์žฌํ˜„ยท์ˆ˜์ •(ํ˜„์žฌ `email_records`/๋ ˆ๊ฑฐ์‹œ `emails` ์–‘์ชฝ์„ ์ปฌ๋Ÿผ ์กด์žฌ ์—ฌ๋ถ€๋กœ + ๊ฐ€๋“œ)ํ–ˆ๊ธฐ์—, ์„œ๋กœ ๋‹ค๋ฅธ ๋‘ ๊ตฌํ˜„์ด ์ถฉ๋Œํ•˜์ง€ ์•Š๋„๋ก `0011_email_read_state.py`์™€ + `backend/scripts/bootstrap_db.py`๋Š” `#1503`์˜ ๊ตฌํ˜„์œผ๋กœ ์ˆ˜๋ ด์‹œ์ผฐ์Šต๋‹ˆ๋‹ค โ€” ์ด PR์€ CI + service-container ์ถ”๊ฐ€์™€ ๊ทธ๊ฒƒ์ด ์ฒ˜์Œ์œผ๋กœ ๋“œ๋Ÿฌ๋‚ธ `is_read` raw-SQL ์‹œ๋”ฉ ๊ฒฐํ•จ ์ˆ˜์ •๋งŒ + ๋‹ด๋‹นํ•˜๋Š” ์˜์กด์„ฑ ๋ฃจํŠธ ์Šฌ๋ผ์ด์Šค๋กœ ๋ฒ”์œ„๋ฅผ ์ขํ˜”์Šต๋‹ˆ๋‹ค(owner ์š”์ฒญ, 2026-09-02). PR-governance/ + stacked-PR ํŠธ๋ฆฌ๊ฑฐ ๊ด€๋ จ ๋ฌด๊ด€ํ•œ ๋ณ€๊ฒฝ์€ `#1531`๋กœ ๋ถ„๋ฆฌํ–ˆ์Šต๋‹ˆ๋‹ค. + ์ „์ฒด ๋ฐฑ์—”๋“œ ์Šค์œ„ํŠธ๋ฅผ ์‹ค์ œ PostgreSQL 16(+pgvector)๋กœ ๊ฒ€์ฆ: **1837 passed, 2 skipped** + (๋‚จ์€ 2๊ฐœ๋Š” `LIVE_BASE_URL` ๋ฏธ์„ค์ •์— ๋”ฐ๋ฅธ ๋ฌด๊ด€ํ•œ live-API smoke skip), `ruff check` clean. + `CLAUDE.md`/`AGENTS.md`์— ์ด job์ด ์ด์ œ real-Postgres ํ…Œ์ŠคํŠธ๋ฅผ ํ•˜๋“œ ๊ฒŒ์ดํŠธ๋กœ ์‹คํ–‰ํ•œ๋‹ค๋Š” ๊ฒƒ๊ณผ + ๋กœ์ปฌ ์žฌํ˜„ ๋ฐฉ๋ฒ•์„ ๊ธฐ๋ก. ํ›„์† ๊ณผ์ œ๋กœ ๋‚จ๊ฒจ๋‘์—ˆ๋˜ ํ•ญ๋ชฉ(`docs/product-technical-gap-baseline.md`, + `.github` repo)์„ ๋‹ซ์Šต๋‹ˆ๋‹ค. - ๊ธด ์ด๋ฉ”์ผยท์ฒจ๋ถ€ ๋ณธ๋ฌธ์„ ์˜๋ฏธ ๋‹จ์œ„ ์ฒญํฌ๋กœ ์ž„๋ฒ ๋”ฉํ•œ ๋’ค ๊ธฐ์กด email/attachment ๋ฒกํ„ฐ ๊ณ„์•ฝ์œผ๋กœ ํ‰๊ท ํ™”ํ•˜๊ณ , ์ฒญํฌ ์š”์ฒญยท๋ฒกํ„ฐ ๋ˆ„์ ์„ ์ œํ•œ๋œ ์ฐฝ์œผ๋กœ ์ฒ˜๋ฆฌํ•ฉ๋‹ˆ๋‹ค. OpenAI `text-embedding-3-*`์—๋Š” ์ €์žฅ ์ฐจ์›(`1536`)์„ ์ง์ ‘ ์š”์ฒญํ•˜๋„๋ก ๋ณด๊ฐ•ํ–ˆ์Šต๋‹ˆ๋‹ค. ํ•ฉ์„ฑ ๋ฉ”์ผ fixture 5๊ฑด(70์ฒญํฌ)๊ณผ provider ์š”์ฒญ ๊ณ„์•ฝ์œผ๋กœ 1,536์ฐจ์› ๋ฒกํ„ฐ ๊ฒฝ๋กœ๋ฅผ ๊ฒ€์ฆํ–ˆ์œผ๋ฉฐ, ์‹คํ–‰ ์‹œ ์„ ํƒํ•œ ์ž„๋ฒ ๋”ฉ ์ œ๊ณต์ž์— ๋ณธ๋ฌธยทํŒŒ์‹ฑ๋œ ์ฒจ๋ถ€ ํ…์ŠคํŠธ๋ฅผ ์ „์†กํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค. ํšŒ์‚ฌ ๊ธฐ๋ฐ€ ๋ฐ์ดํ„ฐ๋Š” fixtureยทcommitยทPRยทlog์— ํฌํ•จํ•˜์ง€ ์•Š์Šต๋‹ˆ๋‹ค. - EmailDetail ํ…Œ์ŠคํŠธ๊ฐ€ ์ง€์›ํ•˜์ง€ ์•Š๋Š” ์Šค๋ ˆ๋“œ ๋ณ‘ํ•ฉ/๋ถ„๋ฆฌ ๋ฒ„ํŠผ์„ `textContent`๋ฟ ์•„๋‹ˆ๋ผ `aria-label`๊ณผ `title` ์ ‘๊ทผ ๊ฐ€๋Šฅ ์ด๋ฆ„์œผ๋กœ๋„ ๊ฒ€์ถœํ•˜๋„๋ก ๋ฐ”๊ฟ”, ์•„์ด์ฝ˜ ์ „์šฉ ๋ฒ„ํŠผ ํšŒ๊ท€๋ฅผ ๋†“์น˜์ง€ ์•Š์Šต๋‹ˆ๋‹ค. diff --git a/CLAUDE.md b/CLAUDE.md index be67bc80c..6ade80507 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,6 +36,13 @@ uvicorn main:app --reload # local dev server only `DISABLE_BACKGROUND_WORKERS=1`, then fails the job if the pytest output contains `Timeout`, `Fatal`, `Warn`, or `Denied`. Match that locally for merge evidence. +- The `backend` job provisions a real `pgvector/pgvector:pg16` Postgres + `services:` container (`.github/workflows/app-ci.yml`), so every + `@pytest.mark.postgres`/real-Postgres smoke test actually runs in CI โ€” it + is a hard gate, not a soft-skip. Locally, point `DATABASE_URL` at a + Postgres+pgvector instance (`docker-compose.yml`'s `db` service, or any + local Postgres 16) before running the full suite, or those tests skip with + `PostgreSQL smoke database/path unavailable` instead of proving anything. - Containers never run `uvicorn main:app` directly; the entrypoint is `python scripts/start_backend.py`, which validates required settings first. - `scripts/bootstrap_db.py` is the local/dev-only schema compatibility path; diff --git a/backend/alembic/versions/0001_initial_control_plane.py b/backend/alembic/versions/0001_initial_control_plane.py index cc14ce39b..31b187f55 100644 --- a/backend/alembic/versions/0001_initial_control_plane.py +++ b/backend/alembic/versions/0001_initial_control_plane.py @@ -9,7 +9,7 @@ from sqlalchemy import text from db.models import Base -from scripts.bootstrap_db import schema_backfill_sql +from scripts.bootstrap_db import execute_schema_backfill revision = "0001_initial_control_plane" down_revision = None @@ -19,8 +19,7 @@ def upgrade() -> None: connection = op.get_bind() connection.execute(text("CREATE EXTENSION IF NOT EXISTS vector")) Base.metadata.create_all(connection) - for statement in schema_backfill_sql(): - connection.execute(statement) + execute_schema_backfill(connection) def downgrade() -> None: diff --git a/backend/alembic/versions/0011_email_read_state.py b/backend/alembic/versions/0011_email_read_state.py index 716590cd1..04fa068ce 100644 --- a/backend/alembic/versions/0011_email_read_state.py +++ b/backend/alembic/versions/0011_email_read_state.py @@ -1,6 +1,14 @@ -"""Add is_read to emails (IMAP \\Seen read state). +"""Add is_read to email_records (IMAP \\Seen read state). Existing rows default to read so historical/file imports do not surface as unread. + +Checks both "email_records" (the real, current table -- a database whose own +0001_initial_control_plane ran before ``is_read`` was added to the ``Email`` +model has this table without the column, and needs it added) and "emails" +(a legacy name that, per 0011_email_model_reconciliation's docstring, no +migration in this repo's history ever actually created for a real managed +database, but is checked defensively in case one somehow exists). Guarded by +column existence, not just table existence, so it is safely idempotent. """ from alembic import op @@ -12,18 +20,40 @@ branch_labels = None depends_on = None +_CANDIDATE_TABLES = ("email_records", "emails") + def upgrade() -> None: - op.add_column( - "emails", - sa.Column( - "is_read", - sa.Boolean(), - nullable=False, - server_default=sa.text("true"), - ), - ) + inspector = sa.inspect(op.get_bind()) + for table_name in _CANDIDATE_TABLES: + if inspector.has_table(table_name) and not _has_column( + inspector, table_name, "is_read" + ): + op.add_column( + table_name, + sa.Column( + "is_read", + sa.Boolean(), + nullable=False, + server_default=sa.text("true"), + ), + ) def downgrade() -> None: - op.drop_column("emails", "is_read") + # Same ownership-ambiguity problem as 0018_workspace_registry's downgrade: + # a fresh database gets email_records.is_read from 0001's live + # Base.metadata.create_all, not from this revision, so there is no way to + # tell "this revision added the column" apart from "the baseline already + # had it" -- and is_read holds real per-message read/unread state, not + # rebuildable derived data. As with 0001_initial_control_plane and + # 0018_workspace_registry: production rollbacks should restore from + # backup or a later explicit down revision rather than dropping + # customer-owned data. + return None + + +def _has_column(inspector, table_name: str, column_name: str) -> bool: + return any( + column["name"] == column_name for column in inspector.get_columns(table_name) + ) diff --git a/backend/scripts/bootstrap_db.py b/backend/scripts/bootstrap_db.py index 1047103e8..3a1211181 100644 --- a/backend/scripts/bootstrap_db.py +++ b/backend/scripts/bootstrap_db.py @@ -1,16 +1,12 @@ import asyncio import os -from collections.abc import Sequence - -from sqlalchemy import Executable, text +from sqlalchemy import Executable, Index, MetaData, Table, inspect, text from sqlalchemy.engine import Connection from db.models import Base from db.session import engine INVALID_EMAIL_BACKFILL_OWNER_IDS = {None, "", "default"} - - def _static_bootstrap_sql(statement: str) -> Executable: # ponytail: repo-authored static bootstrap SQL only; bind params before runtime input. return text(statement) @@ -186,10 +182,6 @@ def _get_create_indexes_statements() -> list[Executable]: "CREATE INDEX IF NOT EXISTS ix_email_records_owner_date " "ON email_records (user_id, organization_id, date)" ), - text( - "CREATE INDEX IF NOT EXISTS ix_emails_owner_date " - "ON emails (user_id, organization_id, date)" - ), text( "CREATE INDEX IF NOT EXISTS ix_sender_relationships_owner_source " "ON sender_relationships " @@ -527,16 +519,24 @@ def schema_backfill_sql() -> list[Executable]: return statements -def _execute_statements(conn: Connection, statements: Sequence[Executable]) -> None: - for statement in statements: +def execute_schema_backfill(conn: Connection) -> None: + for statement in schema_backfill_sql(): conn.execute(statement) + if inspect(conn).has_table("emails"): + emails = Table("emails", MetaData(), autoload_with=conn) + Index( + "ix_emails_owner_date", + emails.c.user_id, + emails.c.organization_id, + emails.c.date, + ).create(conn, checkfirst=True) async def bootstrap_db() -> None: async with engine.begin() as conn: await conn.execute(text("CREATE EXTENSION IF NOT EXISTS vector")) await conn.run_sync(Base.metadata.create_all) - await conn.run_sync(_execute_statements, schema_backfill_sql()) + await conn.run_sync(execute_schema_backfill) if __name__ == "__main__": diff --git a/backend/tests/test_alembic_migrations.py b/backend/tests/test_alembic_migrations.py index f8f3ffeae..0045cab10 100644 --- a/backend/tests/test_alembic_migrations.py +++ b/backend/tests/test_alembic_migrations.py @@ -32,7 +32,32 @@ def test_initial_alembic_revision_records_current_schema_path(): assert "down_revision = None" in revision_text assert "CREATE EXTENSION IF NOT EXISTS vector" in revision_text assert "Base.metadata.create_all" in revision_text - assert "schema_backfill_sql" in revision_text + assert "execute_schema_backfill" in revision_text + + +def test_email_read_state_guards_both_legacy_and_current_table_names(): + """0011_email_read_state must add is_read to a genuinely historical + email_records table missing it (a database whose own 0001 ran before + is_read was added to the Email model), not just a legacy "emails" table + that, per 0011_email_model_reconciliation's docstring, no migration in + this repo's history ever actually created for a real managed database. + The upgrade check must guard on column existence, not just table + existence, so it stays idempotent against a table that already has the + column. downgrade is a no-op: a fresh database's email_records.is_read + comes from 0001's live create_all, not from this revision, so there is + no way to tell "this revision added it" apart from "the baseline already + had it" -- and is_read holds real read/unread state, not rebuildable + derived data (same ownership-ambiguity reasoning as + 0018_workspace_registry's downgrade).""" + revision_path = BACKEND_ROOT / "alembic" / "versions" / "0011_email_read_state.py" + revision_text = revision_path.read_text() + + assert '"email_records"' in revision_text + assert '"emails"' in revision_text + assert "has_table" in revision_text + assert "_has_column" in revision_text + assert "op.add_column(" in revision_text + assert "op.drop_column(" not in revision_text def test_provider_writeback_retry_queue_has_incremental_revision(): diff --git a/backend/tests/test_bootstrap_db.py b/backend/tests/test_bootstrap_db.py index 5af0540f0..95a4e72d9 100644 --- a/backend/tests/test_bootstrap_db.py +++ b/backend/tests/test_bootstrap_db.py @@ -6,7 +6,7 @@ from core.config import settings from db.models import Base -from scripts.bootstrap_db import schema_backfill_sql +from scripts.bootstrap_db import execute_schema_backfill, schema_backfill_sql from db.models import ( AgentRunRecord, CalendarWritebackSource, @@ -30,8 +30,7 @@ def _get_schema_statements(monkeypatch): def _execute_schema_backfill(sync_conn): - for statement in schema_backfill_sql(): - sync_conn.execute(statement) + execute_schema_backfill(sync_conn) def test_schema_backfill_adds_email_columns(monkeypatch): @@ -770,11 +769,11 @@ async def test_connector_signal_events_real_postgres_bootstrap_smoke(): text(""" INSERT INTO email_records ( user_id, organization_id, message_id, sender, recipients, - subject, "date", body + subject, "date", body, is_read ) VALUES ( :user_id, :organization_id, :message_id, :sender, - :recipients, :subject, now(), :body + :recipients, :subject, now(), :body, true ) RETURNING id """), diff --git a/backend/tests/test_data_api.py b/backend/tests/test_data_api.py index cd0b7bf37..c47005ca1 100644 --- a/backend/tests/test_data_api.py +++ b/backend/tests/test_data_api.py @@ -2951,11 +2951,11 @@ async def _seed_smoke_test_data(conn, ids: dict): """ INSERT INTO email_records ( user_id, organization_id, message_id, thread_id, - fingerprint, sender, recipients, subject, "date", body + fingerprint, sender, recipients, subject, "date", body, is_read ) VALUES ( :user_id, :organization_id, :message_id, :thread_id, - :fingerprint, :sender, :recipients, :subject, now(), :body + :fingerprint, :sender, :recipients, :subject, now(), :body, true ) RETURNING id """ @@ -2977,11 +2977,11 @@ async def _seed_smoke_test_data(conn, ids: dict): """ INSERT INTO email_records ( user_id, organization_id, message_id, sender, recipients, - subject, "date", body + subject, "date", body, is_read ) VALUES ( :user_id, :organization_id, :message_id, :sender, - :recipients, :subject, now(), :body + :recipients, :subject, now(), :body, true ) RETURNING id """ @@ -3001,11 +3001,11 @@ async def _seed_smoke_test_data(conn, ids: dict): """ INSERT INTO email_records ( user_id, organization_id, message_id, thread_id, - fingerprint, sender, recipients, subject, "date", body + fingerprint, sender, recipients, subject, "date", body, is_read ) VALUES ( :user_id, :organization_id, :message_id, :thread_id, - :fingerprint, :sender, :recipients, :subject, now(), :body + :fingerprint, :sender, :recipients, :subject, now(), :body, true ) RETURNING id """ diff --git a/backend/tests/test_release_governance.py b/backend/tests/test_release_governance.py index 394b1e39f..01a43b19b 100644 --- a/backend/tests/test_release_governance.py +++ b/backend/tests/test_release_governance.py @@ -663,6 +663,11 @@ def test_app_ci_runs_backend_and_frontend_checks_without_duplicate_release_pushe assert "uses: actions/checkout@v" not in workflow assert "uses: actions/setup-python@v" not in workflow assert "uses: actions/setup-node@v" not in workflow + assert ( + "image: pgvector/pgvector:pg16@sha256:" + "ccc6e83d6e35e931dc7c5def2022729d5a6c370318d099181995567ff1fb4d6b" + in workflow + ) push_block = workflow.split("push:", 1)[1].split("pull_request:", 1)[0] assert "master" in push_block