Skip to content
Draft
18 changes: 18 additions & 0 deletions .github/workflows/app-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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
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
Expand Down
7 changes: 7 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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` 접근 가능 이름으로도 검출하도록 바꿔, 아이콘 전용 버튼 회귀를 놓치지 않습니다.

Expand Down
7 changes: 7 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions backend/alembic/versions/0001_initial_control_plane.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
52 changes: 41 additions & 11 deletions backend/alembic/versions/0011_email_read_state.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"
):
Comment on lines +27 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add a new revision for already-stamped databases

For any installation whose Alembic history already includes 0011_email_read_state—for example, a database bootstrapped and then stamped before adopting the managed migration path—command.upgrade(..., "head") will never execute this edited upgrade() again, so email_records.is_read remains absent and ORM reads or inserts can still fail. Keep this guard for fresh installs if needed, but also ship the guarded email_records addition in a new revision descending from the current head and verify upgrading a schema already stamped past 0011.

Useful? React with 👍 / 👎.

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)
)
24 changes: 12 additions & 12 deletions backend/scripts/bootstrap_db.py
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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__":
Expand Down
27 changes: 26 additions & 1 deletion backend/tests/test_alembic_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
9 changes: 4 additions & 5 deletions backend/tests/test_bootstrap_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -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
"""),
Expand Down
12 changes: 6 additions & 6 deletions backend/tests/test_data_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand All @@ -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
"""
Expand All @@ -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
"""
Expand Down
Loading