diff --git a/apps/backend/alembic/env.py b/apps/backend/alembic/env.py index 64cf2b23..61de3f30 100644 --- a/apps/backend/alembic/env.py +++ b/apps/backend/alembic/env.py @@ -41,6 +41,27 @@ def run_migrations_online() -> None: ) with connectable.connect() as connection: + if connection.dialect.name == "sqlite": + # SQLite refuses to toggle `PRAGMA foreign_keys` mid-transaction + # (a documented no-op once a transaction is open), and Alembic's + # own per-migration transaction is already open by the time a + # revision's upgrade() runs. A migration that uses batch mode to + # add a constraint to a table that another table already has a + # deferred foreign key pointing at (e.g. #299's cyclic + # repository_lineages <-> repositories relationship) drops and + # recreates that table; SQLite's deferred-FK bookkeeping does not + # correctly reconcile that recreation against the still-open + # transaction, and a fully self-consistent final state still + # fails at COMMIT with a generic "FOREIGN KEY constraint failed" + # (verified: `PRAGMA foreign_key_check` reports no violation + # immediately beforehand). Disabling enforcement here, before any + # transaction opens, avoids this without weakening runtime + # enforcement: every real application connection still gets + # `PRAGMA foreign_keys=ON` via app.core.database's own + # connect-event listener; this affects only the connection + # Alembic itself uses while migrating. + connection.exec_driver_sql("PRAGMA foreign_keys=OFF") + connection.commit() context.configure(connection=connection, target_metadata=target_metadata) with context.begin_transaction(): diff --git a/apps/backend/alembic/versions/0013_lineage_expand.py b/apps/backend/alembic/versions/0013_lineage_expand.py new file mode 100644 index 00000000..5944da22 --- /dev/null +++ b/apps/backend/alembic/versions/0013_lineage_expand.py @@ -0,0 +1,382 @@ +"""add repository_lineages, expand repositories, and backfill (1 of 2) + +Revision ID: 0013_lineage_expand +Revises: 0012_waitlist_entries +Create Date: 2026-08-27 + +Issue #299 (RFC-0002), authorized for implementation per +docs/architecture/REPOSITORY_LINEAGE_MIGRATION_PLAN.md. Adds the durable +owner-scoped logical grouping above repository revisions: a new +``repository_lineages`` table, nullable ``repositories.lineage_id`` / +``repositories.sequence`` columns, and a strict, deterministic backfill for +resolvable historical GitHub commits. + +This is deliberately split into two revisions (plan §7). This one creates +every constraint that does *not* require the second table to already exist, +runs the backfill, verifies it, then closes every constraint that only needs +one side of the eventual cyclic integrity boundary. The genuinely cyclic +constraint -- the lineage's latest-member pointer proving it names a +repository in that exact lineage -- is Revision B +(0014_lineage_constraints), so a failure here leaves this revision's state +additive, backward-compatible, and inspectable before retrying B. + +Backfill scope (plan §6): only ``source='github'`` rows with a valid 40-hex +commit SHA, a resolved ``refs/heads/...``/``refs/tags/...`` ref, and a +``source_url`` matching one of the exact accepted historical GitHub URL +forms are grouped into a lineage. Uploads, unresolved-ref rows, and anything +outside that strict grammar stay unlineaged standalone imports -- this +migration never guesses a grouping from a name, path, or network call. + +Imports must be quiesced while this migration runs; there is no dual-write +compatibility for a concurrent insert racing the backfill. +""" + +from __future__ import annotations + +import re +import uuid +from typing import Any + +from alembic import op +import sqlalchemy as sa + +revision = "0013_lineage_expand" +down_revision = "0012_waitlist_entries" +branch_labels = None +depends_on = None + +_GIT_SHA_RE = re.compile(r"^[0-9a-f]{40}$") +_REF_RE = re.compile(r"^refs/(?:heads|tags)/[A-Za-z0-9._/-]+$") +# The host is matched case-insensitively (scoped to just that group) because +# the host itself is one of the two things plan §6.1 step 3 requires +# case-folding -- a historical pre-hardening URL may have used "GitHub.com". +# The scheme/userinfo-marker literals stay exact-case; only the host is +# case-variable here. +_HTTPS_GITHUB_RE = re.compile(r"^https://(?i:github\.com)/([^/]+)/([^/]+?)(?:\.git)?/?$") +_SSH_GITHUB_RE = re.compile(r"^git@(?i:github\.com):([^/]+)/([^/]+?)(?:\.git)?$") +_SAFE_COMPONENT_RE = re.compile(r"^[A-Za-z0-9._-]+$") + +_CANONICAL_PARTIAL_WHERE = sa.text("canonical_source_key IS NOT NULL AND canonical_branch IS NOT NULL") + +# Fixed, migration-local UUIDv5 namespace for deterministic backfill lineage +# IDs (plan §6.2). Frozen forever once this revision ships, exactly like +# every other literal in an applied migration -- never imported from, or +# shared with, live application code, which is free to evolve independently. +_BACKFILL_UUID_NAMESPACE = uuid.UUID("ac9ac65f-f0c1-4f7f-8147-9dc3509b4eaa") + + +def _safe_component(value: str) -> bool: + return bool(_SAFE_COMPONENT_RE.fullmatch(value)) and ".." not in value + + +def _canonical_github_source(source_url: str | None) -> str | None: + """Strict, conservative GitHub URL canonicalization, backfill-only (plan + §6.1). Returns ``None`` for anything outside the exact accepted forms -- + the row then stays an unlineaged standalone import, never a guess. + + Deliberately not shared with ``app.services.repository_service``'s live + parser (which only needs to handle its own validator's already-narrow + output): a future change to that live code must never silently change + what this frozen, already-applied migration replays. + """ + if not source_url: + return None + trimmed = source_url.strip() + match = _HTTPS_GITHUB_RE.match(trimmed) or _SSH_GITHUB_RE.match(trimmed) + if not match: + return None + owner, repo = match.group(1), match.group(2) + if not (_safe_component(owner) and _safe_component(repo)): + return None + return f"github.com/{owner.lower()}/{repo.lower()}" + + +def _lineage_id_for(owner_id: str, canonical_source_key: str, canonical_branch: str) -> str: + """Deterministic, length-delimited encoding (plan §6.2) -- avoids the + ambiguity a plain concatenation would have (e.g. two different + owner/key/branch triples producing the same joined string).""" + encoded = ( + f"{len(owner_id)}:{owner_id}|" + f"{len(canonical_source_key)}:{canonical_source_key}|" + f"{len(canonical_branch)}:{canonical_branch}" + ) + return str(uuid.uuid5(_BACKFILL_UUID_NAMESPACE, encoded)) + + +def _create_lineage_table() -> None: + op.create_table( + "repository_lineages", + sa.Column("id", sa.String(length=36), primary_key=True), + sa.Column("owner_id", sa.String(length=36), nullable=False), + sa.Column("canonical_source_key", sa.Text(), nullable=True), + sa.Column("canonical_branch", sa.Text(), nullable=True), + sa.Column("display_name", sa.Text(), nullable=False), + # The cyclic half of this column's FK (proving the pointer names a + # repository in *this* lineage) is Revision B; it is a plain nullable + # column here. + sa.Column("latest_repository_id", sa.String(length=36), nullable=True), + sa.Column("next_sequence", sa.Integer(), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.UniqueConstraint("id", "owner_id", name="uq_repository_lineages_id_owner"), + sa.CheckConstraint( + "(canonical_source_key IS NULL AND canonical_branch IS NULL) OR " + "(canonical_source_key IS NOT NULL AND canonical_branch IS NOT NULL)", + name="ck_repository_lineages_canonical_pair", + ), + sa.CheckConstraint("next_sequence >= 1", name="ck_repository_lineages_next_sequence_positive"), + sa.ForeignKeyConstraint( + ["owner_id"], + ["users.id"], + name="fk_repository_lineages_owner_id_users", + ondelete="CASCADE", + ), + ) + op.create_index("ix_repository_lineages_owner_id", "repository_lineages", ["owner_id"]) + op.create_index( + "uq_repository_lineages_owner_source_branch", + "repository_lineages", + ["owner_id", "canonical_source_key", "canonical_branch"], + unique=True, + sqlite_where=_CANONICAL_PARTIAL_WHERE, + postgresql_where=_CANONICAL_PARTIAL_WHERE, + ) + + +def _add_repository_lineage_columns() -> None: + # Both additions in one batch block (plan §7 step 2): avoids a second + # SQLite table copy for what is otherwise the same operation. + with op.batch_alter_table("repositories") as batch: + batch.add_column(sa.Column("lineage_id", sa.String(length=36), nullable=True)) + batch.add_column(sa.Column("sequence", sa.Integer(), nullable=True)) + + +def _repositories_core_table() -> sa.TableClause: + return sa.table( + "repositories", + sa.column("id", sa.String()), + sa.column("owner_id", sa.String()), + sa.column("name", sa.String()), + sa.column("source", sa.String()), + sa.column("source_url", sa.Text()), + sa.column("revision_kind", sa.String()), + sa.column("revision_value", sa.String()), + sa.column("revision_ref", sa.String()), + sa.column("created_at", sa.DateTime()), + sa.column("lineage_id", sa.String()), + sa.column("sequence", sa.Integer()), + ) + + +def _lineages_core_table() -> sa.TableClause: + return sa.table( + "repository_lineages", + sa.column("id", sa.String()), + sa.column("owner_id", sa.String()), + sa.column("canonical_source_key", sa.Text()), + sa.column("canonical_branch", sa.Text()), + sa.column("display_name", sa.Text()), + sa.column("latest_repository_id", sa.String()), + sa.column("next_sequence", sa.Integer()), + sa.column("created_at", sa.DateTime()), + ) + + +def _eligible_groups(connection: sa.Connection, repositories: sa.TableClause) -> dict[tuple[str, str, str], list[dict]]: + """Group resolvable historical GitHub rows by (owner, canonical source, + canonical branch), sorted deterministically within each group (plan + §6.1/§6.2). Everything else (uploads, unresolved refs, malformed/foreign + source URLs) is excluded and stays standalone.""" + rows = connection.execute( + sa.select( + repositories.c.id, + repositories.c.owner_id, + repositories.c.name, + repositories.c.source, + repositories.c.source_url, + repositories.c.revision_kind, + repositories.c.revision_value, + repositories.c.revision_ref, + repositories.c.created_at, + ) + ).mappings() + + groups: dict[tuple[str, str, str], list[dict[str, Any]]] = {} + for row in rows: + if row["source"] != "github" or row["revision_kind"] != "git": + continue + revision_value = row["revision_value"] + if not revision_value or not _GIT_SHA_RE.fullmatch(revision_value): + continue + revision_ref = row["revision_ref"] + if not revision_ref or not _REF_RE.fullmatch(revision_ref): + continue + canonical_source_key = _canonical_github_source(row["source_url"]) + if canonical_source_key is None: + continue + key = (row["owner_id"], canonical_source_key, revision_ref) + groups.setdefault(key, []).append(dict(row)) + + for members in groups.values(): + members.sort(key=lambda member: (member["created_at"], member["id"])) + return groups + + +def _backfill_lineages() -> dict[tuple[str, str, str], list[dict]]: + connection = op.get_bind() + repositories = _repositories_core_table() + lineages = _lineages_core_table() + + groups = _eligible_groups(connection, repositories) + + for (owner_id, canonical_source_key, canonical_branch), members in groups.items(): + lineage_id = _lineage_id_for(owner_id, canonical_source_key, canonical_branch) + first, last = members[0], members[-1] + next_sequence = len(members) + 1 + + already_present = connection.execute(sa.select(lineages.c.id).where(lineages.c.id == lineage_id)).first() + if already_present is None: + connection.execute( + lineages.insert().values( + id=lineage_id, + owner_id=owner_id, + canonical_source_key=canonical_source_key, + canonical_branch=canonical_branch, + display_name=first["name"], + latest_repository_id=last["id"], + next_sequence=next_sequence, + created_at=first["created_at"], + ) + ) + else: + # A prior interrupted run already created this row (plan §6.2/§7 + # "interruption and rerun"): reconcile its counter and latest + # pointer to this deterministic grouping rather than trusting + # whatever a partial run left behind. + connection.execute( + lineages.update() + .where(lineages.c.id == lineage_id) + .values(latest_repository_id=last["id"], next_sequence=next_sequence) + ) + + for index, member in enumerate(members, start=1): + connection.execute( + repositories.update() + .where(repositories.c.id == member["id"]) + .values(lineage_id=lineage_id, sequence=index) + ) + + return groups + + +def _verify_backfill(groups: dict[tuple[str, str, str], list[dict]]) -> None: + """Abort the migration unless every §6.4 invariant holds. Reuses the same + grouping the write phase just used (rather than re-deriving the + eligibility predicate a second time in SQL), so this proves the write + actually took effect as intended, not just that a second, possibly + independently-buggy, predicate agrees with the first.""" + connection = op.get_bind() + repositories = _repositories_core_table() + lineages = _lineages_core_table() + + touched = 0 + for (owner_id, canonical_source_key, canonical_branch), members in groups.items(): + lineage_id = _lineage_id_for(owner_id, canonical_source_key, canonical_branch) + lineage_row = ( + connection.execute( + sa.select(lineages.c.owner_id, lineages.c.latest_repository_id, lineages.c.next_sequence).where( + lineages.c.id == lineage_id + ) + ) + .mappings() + .first() + ) + if lineage_row is None: + raise RuntimeError(f"Lineage backfill verification failed: {lineage_id} is missing after backfill.") + if lineage_row["owner_id"] != owner_id: + raise RuntimeError(f"Lineage backfill verification failed: {lineage_id} has the wrong owner.") + if lineage_row["latest_repository_id"] != members[-1]["id"]: + raise RuntimeError(f"Lineage backfill verification failed: {lineage_id} latest pointer is wrong.") + if lineage_row["next_sequence"] != len(members) + 1: + raise RuntimeError(f"Lineage backfill verification failed: {lineage_id} next_sequence is wrong.") + + seen_sequences: set[int] = set() + for index, member in enumerate(members, start=1): + repo_row = ( + connection.execute( + sa.select(repositories.c.lineage_id, repositories.c.sequence, repositories.c.owner_id).where( + repositories.c.id == member["id"] + ) + ) + .mappings() + .first() + ) + if repo_row is None or repo_row["lineage_id"] != lineage_id or repo_row["sequence"] != index: + raise RuntimeError( + f"Lineage backfill verification failed: repository {member['id']} is not " + f"correctly attached to lineage {lineage_id}." + ) + if repo_row["owner_id"] != owner_id: + raise RuntimeError(f"Lineage backfill verification failed: repository {member['id']} owner mismatch.") + if repo_row["sequence"] in seen_sequences: + raise RuntimeError(f"Lineage backfill verification failed: duplicate sequence in lineage {lineage_id}.") + seen_sequences.add(repo_row["sequence"]) + touched += 1 + + stray = connection.execute( + sa.select(sa.func.count()) + .select_from(repositories) + .where(sa.or_(repositories.c.lineage_id.isnot(None), repositories.c.sequence.isnot(None))) + ).scalar() + if stray != touched: + raise RuntimeError( + f"Lineage backfill verification failed: {stray} repositories carry a lineage " + f"attachment, expected exactly {touched} from the eligible groups." + ) + + +def _add_repository_lineage_constraints() -> None: + with op.batch_alter_table("repositories") as batch: + batch.create_check_constraint( + "ck_repositories_lineage_sequence_pair", + "(lineage_id IS NULL AND sequence IS NULL) OR " + "(lineage_id IS NOT NULL AND sequence IS NOT NULL AND sequence >= 1)", + ) + # Every NULL is distinct under SQL uniqueness, so standalone rows + # (both null) never collide here (plan §4.2). + batch.create_unique_constraint("uq_repositories_lineage_sequence", ["lineage_id", "sequence"]) + # Composite target proving a lineage's latest-member pointer names a + # repository that actually belongs to that exact lineage (used by + # Revision B's FK). + batch.create_unique_constraint("uq_repositories_id_lineage", ["id", "lineage_id"]) + batch.create_foreign_key( + "fk_repositories_lineage_owner", + "repository_lineages", + ["lineage_id", "owner_id"], + ["id", "owner_id"], + deferrable=True, + initially="DEFERRED", + ) + + +def upgrade() -> None: + _create_lineage_table() + _add_repository_lineage_columns() + groups = _backfill_lineages() + _verify_backfill(groups) + _add_repository_lineage_constraints() + + +def downgrade() -> None: + # New grouping/counter data is lost; every pre-existing repository + # column and value is preserved (plan §7/§11). + with op.batch_alter_table("repositories") as batch: + batch.drop_constraint("fk_repositories_lineage_owner", type_="foreignkey") + batch.drop_constraint("uq_repositories_id_lineage", type_="unique") + batch.drop_constraint("uq_repositories_lineage_sequence", type_="unique") + batch.drop_constraint("ck_repositories_lineage_sequence_pair", type_="check") + batch.drop_column("sequence") + batch.drop_column("lineage_id") + + op.drop_index("uq_repository_lineages_owner_source_branch", table_name="repository_lineages") + op.drop_index("ix_repository_lineages_owner_id", table_name="repository_lineages") + op.drop_table("repository_lineages") diff --git a/apps/backend/alembic/versions/0014_lineage_constraints.py b/apps/backend/alembic/versions/0014_lineage_constraints.py new file mode 100644 index 00000000..30b52042 --- /dev/null +++ b/apps/backend/alembic/versions/0014_lineage_constraints.py @@ -0,0 +1,44 @@ +"""close the repository-lineage cyclic integrity boundary (2 of 2) + +Revision ID: 0014_lineage_constraints +Revises: 0013_lineage_expand +Create Date: 2026-08-27 + +Issue #299 (RFC-0002), part 2 of 2. Adds the one constraint that genuinely +needs both sides of the lineage/repository relationship to already exist: +``repository_lineages.latest_repository_id`` (together with ``id``) must +name a row in ``repositories`` whose own ``lineage_id`` points back at this +exact lineage -- a latest pointer can never name a repository in a +different lineage, or a different owner's repository, even if application +code is wrong. + +Split from Revision A specifically so a failure here leaves A's state +additive, backward-compatible, and inspectable/fixable before rerunning +this one (plan §7). The new application must not start unless Alembic is at +this revision or later -- do not start code that assumes the latest-member +invariant against a database still only at Revision A. +""" + +from alembic import op + +revision = "0014_lineage_constraints" +down_revision = "0013_lineage_expand" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + with op.batch_alter_table("repository_lineages") as batch: + batch.create_foreign_key( + "fk_repository_lineages_latest_member", + "repositories", + ["latest_repository_id", "id"], + ["id", "lineage_id"], + deferrable=True, + initially="DEFERRED", + ) + + +def downgrade() -> None: + with op.batch_alter_table("repository_lineages") as batch: + batch.drop_constraint("fk_repository_lineages_latest_member", type_="foreignkey") diff --git a/apps/backend/app/models/__init__.py b/apps/backend/app/models/__init__.py index 41efa14c..b83ce332 100644 --- a/apps/backend/app/models/__init__.py +++ b/apps/backend/app/models/__init__.py @@ -5,6 +5,7 @@ from app.models.invite_token import InviteToken from app.models.refresh_token import RefreshToken from app.models.repository import RepositoryRecord +from app.models.repository_lineage import RepositoryLineage from app.models.snapshot import ( RiAssertion, RiDerivation, @@ -26,6 +27,7 @@ "InviteToken", "RefreshToken", "RepositoryRecord", + "RepositoryLineage", "RiAssertion", "RiDerivation", "RiDiagnostic", diff --git a/apps/backend/app/models/repository.py b/apps/backend/app/models/repository.py index 208cf17f..913e9ea3 100644 --- a/apps/backend/app/models/repository.py +++ b/apps/backend/app/models/repository.py @@ -5,6 +5,7 @@ CheckConstraint, DateTime, ForeignKey, + ForeignKeyConstraint, Index, Integer, JSON, @@ -44,6 +45,12 @@ class RepositoryRecord(Base): revision_kind: Mapped[str | None] = mapped_column(String(16), nullable=True) revision_value: Mapped[str | None] = mapped_column(String(80), nullable=True) revision_ref: Mapped[str | None] = mapped_column(String(255), nullable=True) + # Durable logical grouping above revisions (#299, RFC-0002). Both remain + # permanently nullable: an upload or an unresolved-ref legacy GitHub row + # is a standalone import with no lineage, by design (RFC §4.3/§6), not a + # transitional state to be tightened later. + lineage_id: Mapped[str | None] = mapped_column(String(36), nullable=True) + sequence: Mapped[int | None] = mapped_column(Integer, nullable=True) local_path: Mapped[str] = mapped_column(Text) size: Mapped[int] = mapped_column(BigInteger, default=0) file_count: Mapped[int] = mapped_column(Integer, default=0) @@ -94,6 +101,36 @@ class RepositoryRecord(Base): name="ck_repositories_git_revision", ), Index("ix_repositories_revision_value", "revision_value"), + CheckConstraint( + "(lineage_id IS NULL AND sequence IS NULL) OR " + "(lineage_id IS NOT NULL AND sequence IS NOT NULL AND sequence >= 1)", + name="ck_repositories_lineage_sequence_pair", + ), + # Standalone rows (both null) never collide under a unique constraint: + # SQL uniqueness treats every NULL as distinct. Also serves ordered + # lineage reads. + UniqueConstraint("lineage_id", "sequence", name="uq_repositories_lineage_sequence"), + # Composite target proving a lineage's latest-member pointer names a + # repository that actually belongs to that exact lineage. + UniqueConstraint("id", "lineage_id", name="uq_repositories_id_lineage"), + # Cross-owner attachment is invalid at the database layer even if + # service code is wrong (#299 §9). Deferred: a lineage and its first + # repository are written in one transaction (RFC §5.2), so this must + # not be checked until commit. No automatic delete action -- deletion + # updates or clears the lineage's latest pointer explicitly first + # (RFC §8.3), it is never left to a database cascade/set-null here. + # This is one half of a cyclic FK pair with `repository_lineages`; + # see the known `create_all()`-on-SQLite enforcement limitation + # documented on `RepositoryLineage.fk_repository_lineages_latest_member` + # (app/models/repository_lineage.py) -- it applies equally to + # whichever of the two constraints ends up as the forward reference. + ForeignKeyConstraint( + ["lineage_id", "owner_id"], + ["repository_lineages.id", "repository_lineages.owner_id"], + name="fk_repositories_lineage_owner", + deferrable=True, + initially="DEFERRED", + ), ) @validates("revision_kind", "revision_value") diff --git a/apps/backend/app/models/repository_lineage.py b/apps/backend/app/models/repository_lineage.py new file mode 100644 index 00000000..734bfdb5 --- /dev/null +++ b/apps/backend/app/models/repository_lineage.py @@ -0,0 +1,113 @@ +from datetime import UTC, datetime + +from sqlalchemy import ( + CheckConstraint, + DateTime, + ForeignKeyConstraint, + Index, + Integer, + String, + Text, + UniqueConstraint, + text, +) +from sqlalchemy.orm import Mapped, mapped_column + +from app.models.base import Base + +_CANONICAL_PARTIAL_WHERE = text("canonical_source_key IS NOT NULL AND canonical_branch IS NOT NULL") + + +class RepositoryLineage(Base): + """Durable, owner-scoped logical grouping above repository revisions (#299, RFC-0002). + + A row here groups repeated imports of the same GitHub repository/branch + into one ordered history. Uploads and unresolved-ref GitHub rows never get + a row here -- they stay unlineaged standalone imports (RFC §4.3/§6) -- so + ``canonical_source_key``/``canonical_branch`` and every ``repositories`` + attachment are permanently optional, not a transitional NULL. + """ + + __tablename__ = "repository_lineages" + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + owner_id: Mapped[str] = mapped_column(String(36), index=True) + canonical_source_key: Mapped[str | None] = mapped_column(Text, nullable=True) + canonical_branch: Mapped[str | None] = mapped_column(Text, nullable=True) + display_name: Mapped[str] = mapped_column(Text) + # The current highest surviving sequence in this lineage, or null for an + # empty lineage. This FK is deferrable/initially-deferred and cyclic with + # `repositories`: a lineage must exist (with a null pointer) before its + # first repository can be inserted, and the pointer is only set afterward, + # in the same transaction (RFC §5.1/§5.2). + latest_repository_id: Mapped[str | None] = mapped_column(String(36), nullable=True) + # Durable, never-reused, transactionally-allocated next ordinal. Starts at + # 1; deleting a repository never decrements it (RFC §4.3). + next_sequence: Mapped[int] = mapped_column(Integer, default=1) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=lambda: datetime.now(UTC)) + + __table_args__ = ( + # Composite ownership FK target for `repositories.lineage_id`. + UniqueConstraint("id", "owner_id", name="uq_repository_lineages_id_owner"), + CheckConstraint( + "(canonical_source_key IS NULL AND canonical_branch IS NULL) OR " + "(canonical_source_key IS NOT NULL AND canonical_branch IS NOT NULL)", + name="ck_repository_lineages_canonical_pair", + ), + CheckConstraint("next_sequence >= 1", name="ck_repository_lineages_next_sequence_positive"), + ForeignKeyConstraint( + ["owner_id"], + ["users.id"], + name="fk_repository_lineages_owner_id_users", + ondelete="CASCADE", + ), + # The owner-scoped canonical lookup index, and the sole source of + # "does this lineage already exist" truth if two imports race to + # create the first one (RFC §5.2). + Index( + "uq_repository_lineages_owner_source_branch", + "owner_id", + "canonical_source_key", + "canonical_branch", + unique=True, + postgresql_where=_CANONICAL_PARTIAL_WHERE, + sqlite_where=_CANONICAL_PARTIAL_WHERE, + ), + # The cyclic half of the lineage/repository integrity boundary (RFC + # §4.2): a latest-pointer can never name a repository outside this + # exact lineage, even if service code is wrong. Declared here (rather + # than only in the migration) so `create_all()` in development/test + # produces the identical *declared* shape a migrated database + # reaches (`PRAGMA foreign_key_list`/`inspector.get_foreign_keys()` + # show it either way, on both dialects). + # + # Known SQLite limitation (confirmed in CI, #299): `create_all()` + # cannot avoid embedding one of these two cyclic FKs (this one, or + # `repositories.fk_repositories_lineage_owner`) as an inline forward + # reference to a table that doesn't exist yet -- SQLite must create + # one of `repositories`/`repository_lineages` before the other, and + # there is no `ALTER TABLE ADD CONSTRAINT` to add the missing half + # afterward the way the migration does. Which of the two ends up as + # the forward reference depends on `create_all()`'s internal + # cyclic-dependency tie-break, not something this code controls. At + # least one real SQLite build does not enforce a deferred FK + # declared that way -- it still reports the constraint correctly, + # but a genuine violation neither raises at COMMIT nor shows up in + # `PRAGMA foreign_key_check`. The + # Alembic migration (0013/0014) never creates this forward + # reference -- it adds each cyclic FK in its own revision, after + # both tables already exist -- so a database built by migrating, + # including every real deployment and this repo's own CI rehearsal, + # is unaffected. This is a `create_all()`-only (development/test + # bootstrap) gap, not a production one; see + # tests/test_repository_lineage_migration.py's + # test_cross_owner_lineage_attachment_is_rejected_by_the_database_even_if_forced + # for the migration-backed, reliable version of this proof. + ForeignKeyConstraint( + ["latest_repository_id", "id"], + ["repositories.id", "repositories.lineage_id"], + name="fk_repository_lineages_latest_member", + deferrable=True, + initially="DEFERRED", + ), + ) diff --git a/apps/backend/app/repositories/repository_repository.py b/apps/backend/app/repositories/repository_repository.py index 17b09ef2..09060ea5 100644 --- a/apps/backend/app/repositories/repository_repository.py +++ b/apps/backend/app/repositories/repository_repository.py @@ -1,7 +1,31 @@ -from sqlalchemy import select +from datetime import UTC, datetime +from uuid import uuid4 + +from sqlalchemy import select, update +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from app.models.repository import RepositoryRecord +from app.models.repository_lineage import RepositoryLineage + +# Bound retries for two distinct races (#299, RFC-0002 §5.2): two importers +# racing to create the *first* lineage for a canonical key (one loses the +# unique index and must reload/retry), and a lineage-scoped duplicate-commit +# rollback. Five matches AiConversationRepository's own sequence-collision cap. +_MAX_LINEAGE_RETRIES = 5 + + +class LineageDuplicateRevision(Exception): + """The transactional insert found this exact commit already in the lineage. + + Carries the existing record so the caller can build the same 409 detail + (`repositoryId`, `name`) the pre-clone fast-path check already returns, + without a second query. + """ + + def __init__(self, existing: RepositoryRecord) -> None: + self.existing = existing + super().__init__("Repository revision already exists in this lineage.") class RepositoryRepository: @@ -36,26 +60,6 @@ def find_by_revision_for_owner(self, revision_value: str, owner_id: str) -> Repo ) return self.db.scalars(statement).first() - def find_by_source_revision_for_owner( - self, - source_url: str, - revision_value: str, - owner_id: str, - ) -> RepositoryRecord | None: - """Find the same GitHub source at the same immutable commit. - - A commit SHA alone is not a repository identity: forks can legitimately - share commits. GitHub duplicate detection is therefore scoped by source - URL as well as immutable revision, while a new commit at the same URL is - accepted as a new repository revision. - """ - statement = select(RepositoryRecord).where( - RepositoryRecord.source_url == source_url, - RepositoryRecord.revision_value == revision_value, - RepositoryRecord.owner_id == owner_id, - ) - return self.db.scalars(statement).first() - def add(self, record: RepositoryRecord) -> RepositoryRecord: self.db.add(record) self.db.commit() @@ -71,3 +75,146 @@ def save(self, record: RepositoryRecord) -> RepositoryRecord: def delete(self, record: RepositoryRecord) -> None: self.db.delete(record) self.db.commit() + + def delete_with_lineage_update(self, record: RepositoryRecord) -> None: + """Delete `record`, rolling its lineage's latest pointer back first if needed. + + A standalone record (``lineage_id is None``) deletes exactly as + ``delete()`` does. A lineaged record's deletion and any resulting + latest-pointer change happen in one transaction (#299 §8.3): the + counter never decreases, and an empty lineage is kept (null latest, + preserved ``next_sequence``) rather than removed. + """ + if record.lineage_id is not None: + lineage = self.db.execute( + select(RepositoryLineage).where(RepositoryLineage.id == record.lineage_id).with_for_update() + ).scalar_one() + if lineage.latest_repository_id == record.id: + next_latest = self.db.scalars( + select(RepositoryRecord.id) + .where( + RepositoryRecord.lineage_id == record.lineage_id, + RepositoryRecord.id != record.id, + ) + .order_by(RepositoryRecord.sequence.desc()) + .limit(1) + ).first() + lineage.latest_repository_id = next_latest + self.db.delete(record) + self.db.commit() + + def add_with_lineage( + self, + record: RepositoryRecord, + *, + owner_id: str, + canonical_source_key: str | None, + canonical_branch: str | None, + display_name: str, + ) -> RepositoryRecord: + """Insert `record`, allocating it into an owner-scoped lineage keyed by + the given canonical pair (#299, RFC-0002 §5.2). A null pair inserts a + standalone import with no lineage, identical to plain ``add()`` -- + uploads and unresolved-ref imports always take this branch. + + Lineage find-or-create, sequence allocation, the same-lineage + duplicate check, the repository insert, and the latest-pointer update + all happen in one transaction, so a partial allocation can never be + observed or committed. + + Raises ``LineageDuplicateRevision`` (not retried) if this exact commit + already exists in the target lineage. + """ + if canonical_source_key is None: + return self.add(record) + + last_error: IntegrityError | None = None + for _ in range(_MAX_LINEAGE_RETRIES): + try: + return self._add_with_lineage_once( + record, + owner_id=owner_id, + canonical_source_key=canonical_source_key, + canonical_branch=canonical_branch, + display_name=display_name, + ) + except IntegrityError as exc: + last_error = exc + self.db.rollback() + assert last_error is not None + raise last_error + + def _find_lineage( + self, owner_id: str, canonical_source_key: str, canonical_branch: str | None + ) -> RepositoryLineage | None: + statement = select(RepositoryLineage).where( + RepositoryLineage.owner_id == owner_id, + RepositoryLineage.canonical_source_key == canonical_source_key, + RepositoryLineage.canonical_branch == canonical_branch, + ) + return self.db.scalars(statement).first() + + def _add_with_lineage_once( + self, + record: RepositoryRecord, + *, + owner_id: str, + canonical_source_key: str, + canonical_branch: str | None, + display_name: str, + ) -> RepositoryRecord: + lineage = self._find_lineage(owner_id, canonical_source_key, canonical_branch) + if lineage is None: + lineage = RepositoryLineage( + id=str(uuid4()), + owner_id=owner_id, + canonical_source_key=canonical_source_key, + canonical_branch=canonical_branch, + display_name=display_name, + latest_repository_id=None, + next_sequence=1, + created_at=datetime.now(UTC), + ) + self.db.add(lineage) + # Flush (not commit) so a racing winner's canonical unique index + # raises IntegrityError here, before this transaction allocates a + # sequence for a lineage that turns out not to be the real one. + self.db.flush() + + # Atomic, race-free allocation (RFC §5.1): the write lock this UPDATE + # takes is held through commit on both dialects, so two concurrent + # allocations against the same lineage always serialize here rather + # than both reading the same `next_sequence`. + result = self.db.execute( + update(RepositoryLineage) + .where(RepositoryLineage.id == lineage.id, RepositoryLineage.owner_id == owner_id) + .values(next_sequence=RepositoryLineage.next_sequence + 1) + ) + if result.rowcount != 1: # type: ignore[attr-defined] + # Unreachable except as a defensive guard: the lineage was just + # selected or inserted in this same transaction and cannot vanish + # underneath it (deletion of an in-flight, uncommitted lineage is + # not possible from another connection). + raise RuntimeError("Repository lineage row vanished during sequence allocation.") + self.db.refresh(lineage) + allocated_sequence = lineage.next_sequence - 1 + + existing = self.db.scalars( + select(RepositoryRecord).where( + RepositoryRecord.lineage_id == lineage.id, + RepositoryRecord.revision_value == record.revision_value, + ) + ).first() + if existing is not None: + self.db.rollback() + raise LineageDuplicateRevision(existing) + + record.lineage_id = lineage.id + record.sequence = allocated_sequence + self.db.add(record) + self.db.flush() + + lineage.latest_repository_id = record.id + self.db.commit() + self.db.refresh(record) + return record diff --git a/apps/backend/app/services/repository_service.py b/apps/backend/app/services/repository_service.py index bc17afc1..c3036fc2 100644 --- a/apps/backend/app/services/repository_service.py +++ b/apps/backend/app/services/repository_service.py @@ -13,7 +13,7 @@ from app.github.client import GitHubClient from app.models.repository import RepositoryRecord from app.parsers.repository_parser import RepositoryFileLimitExceeded, RepositoryParser, UnsafeRepositoryPath -from app.repositories.repository_repository import RepositoryRepository +from app.repositories.repository_repository import LineageDuplicateRevision, RepositoryRepository from app.schemas.repository import ( FileTreeNode, GitHubImportRequest, @@ -69,8 +69,13 @@ def get_repository(self, repository_id: str) -> RepositoryResponse: def delete_repository(self, repository_id: str) -> None: record = self._get_record(repository_id) - self.storage.delete_repository(record.local_path) - self.repository.delete(record) + local_path = record.local_path + # DB transaction (including any lineage latest-pointer rollback, #299 + # §8.3) commits before the filesystem path is removed, so a DB + # failure can never leave a database row whose source directory has + # already vanished. + self.repository.delete_with_lineage_update(record) + self.storage.delete_repository(local_path) def import_github_repository(self, request: GitHubImportRequest) -> RepositoryResponse: repository_id = str(uuid4()) @@ -80,19 +85,18 @@ def import_github_repository(self, request: GitHubImportRequest) -> RepositoryRe # A new commit is a new revision, so duplicate detection is keyed on the # resolved commit SHA rather than URL+branch (#87). That requires cloning # first: URL+branch is only a fallback when git identity is unavailable, - # and it must never block importing a genuinely new revision. + # and it must never block importing a genuinely new revision. Duplicate + # detection itself happens later, transactionally, scoped to the + # resolved ref's lineage (#299 §5.2) -- not here, and deliberately not + # by (source_url, revision_value, owner) alone, since that would also + # reject the same commit legitimately re-imported under a different + # branch (#299 §8.1), which must succeed. destination = self.storage.reset_repository_path(repository_id) try: self.github.clone_public_repository(url, destination, branch) root = self._resolve_repository_root(destination) commit_sha = self.github.read_head_commit(destination) revision_kind, revision_value, revision_ref = self._git_revision(destination, commit_sha, branch) - existing = self.repository.find_by_source_revision_for_owner(url, revision_value, self.owner_id) - if existing: - raise ConflictServiceError( - "Repository has already been imported.", - {"repositoryId": existing.id, "name": existing.name}, - ) tree, meta, total_size = self._parse_repository(root) self._validate_parsed_repository(meta.total_files) except Exception: @@ -100,10 +104,11 @@ def import_github_repository(self, request: GitHubImportRequest) -> RepositoryRe raise now = datetime.now(UTC) + name = self.github.repository_name(url) record = RepositoryRecord( id=repository_id, owner_id=self.owner_id, - name=self.github.repository_name(url), + name=name, description=None, source="github", source_url=url, @@ -122,7 +127,42 @@ def import_github_repository(self, request: GitHubImportRequest) -> RepositoryRe repo_metadata=meta.model_dump(mode="json", by_alias=True), file_tree=[node.model_dump(mode="json", by_alias=True, exclude_none=True) for node in tree], ) - return self.to_response(self.repository.add(record)) + # A resolved ref is guaranteed here (`_git_revision` above raises + # otherwise), so every live GitHub import always gets a canonical + # pair and therefore a lineage (#299 §8.1) -- unlineaged standalone + # GitHub rows are only a backfill-time legacy case, never live. + try: + persisted = self.repository.add_with_lineage( + record, + owner_id=self.owner_id, + canonical_source_key=self._canonical_github_source(url), + canonical_branch=revision_ref, + display_name=name, + ) + except LineageDuplicateRevision as exc: + self.storage.delete_repository_id(repository_id) + raise ConflictServiceError( + "Repository has already been imported.", + {"repositoryId": exc.existing.id, "name": exc.existing.name}, + ) from exc + except Exception: + self.storage.delete_repository_id(repository_id) + raise + return self.to_response(persisted) + + def _canonical_github_source(self, url: str) -> str: + """Owner-scoped lineage grouping key for an already-validated live URL. + + `url` is already normalized by `GitHubClient.validate_public_url` to + exactly ``https://github.com//`` (no trailing slash or + ``.git``); only case-folding the owner/repo remains (#299 §8.1). This + is deliberately simpler than the migration's own backfill parser, + which must additionally accept looser historical forms -- the two are + intentionally not shared code, so a future change to this live parser + can never silently change what the frozen backfill migration does. + """ + owner, repo = url.removeprefix("https://github.com/").split("/", 1) + return f"github.com/{owner.lower()}/{repo.lower()}" async def import_uploaded_repository(self, file: UploadFile) -> RepositoryResponse: repository_id = str(uuid4()) @@ -151,6 +191,10 @@ async def import_uploaded_repository(self, file: UploadFile) -> RepositoryRespon self.storage.delete_upload(archive_path) now = datetime.now(UTC) + # No lineage_id/sequence set (both stay null): uploads are always + # unlineaged standalone imports (#299 §8.2/§4.3) -- nothing here + # proves two archives are revisions of the same logical repository, + # so this never creates or searches a lineage. record = RepositoryRecord( id=repository_id, owner_id=self.owner_id, diff --git a/apps/backend/scripts/rehearse_migrations.py b/apps/backend/scripts/rehearse_migrations.py index 4e54e3b7..a3fe3efc 100644 --- a/apps/backend/scripts/rehearse_migrations.py +++ b/apps/backend/scripts/rehearse_migrations.py @@ -30,7 +30,7 @@ if str(BACKEND_ROOT) not in sys.path: sys.path.insert(0, str(BACKEND_ROOT)) -HEAD_REVISION = "0012_waitlist_entries" +HEAD_REVISION = "0014_lineage_constraints" REPRESENTATIVE_BASELINE = "0004_ai_provider_configs" REQUIRED_HEAD_TABLES = { "users", @@ -41,6 +41,7 @@ "account_deletion_audits", "invite_tokens", "waitlist_entries", + "repository_lineages", } diff --git a/apps/backend/tests/test_repository_lineage_concurrency.py b/apps/backend/tests/test_repository_lineage_concurrency.py new file mode 100644 index 00000000..6a2f0d50 --- /dev/null +++ b/apps/backend/tests/test_repository_lineage_concurrency.py @@ -0,0 +1,456 @@ +"""Real-PostgreSQL concurrency and integrity coverage for #299 (RFC-0002). + +Per the migration plan's own §13/§14: "SQLite tests prove migration +portability, constraint reflection, and serialized-writer behavior. They +cannot prove PostgreSQL row-lock behavior, transaction isolation, +partial-index semantics, or concurrent create reconciliation." Every test in +this file runs against a real, separate-connection PostgreSQL database using +threading.Barrier synchronization -- not thread timing or sleeps -- matching +the existing `test_concurrent_refresh_on_postgres_mints_one_successor` +pattern in test_auth_concurrency.py. This whole file skips without +PARTHA_TEST_PG_URL. +""" + +import os +import threading +import uuid +from datetime import UTC, datetime + +import pytest +from sqlalchemy import create_engine, select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import sessionmaker + +PG_URL = os.environ.get("PARTHA_TEST_PG_URL") + +pytestmark = pytest.mark.skipif( + not PG_URL, reason="set PARTHA_TEST_PG_URL to run the Postgres lineage concurrency tests" +) + + +def _make_session_factory(): + from app.models.base import Base + + engine = create_engine(PG_URL) + Base.metadata.create_all(engine) + return engine, sessionmaker(bind=engine, expire_on_commit=False) + + +def _make_user(session, email: str | None = None) -> str: + from app.models.user import User + + user = User(id=str(uuid.uuid4()), email=email or f"lineage-{uuid.uuid4().hex}@example.com") + session.add(user) + session.commit() + return user.id + + +def _repository_record(owner_id: str, revision_value: str, **overrides) -> "RepositoryRecord": # noqa: F821 + from app.models.repository import RepositoryRecord + + base = dict( + id=str(uuid.uuid4()), + owner_id=owner_id, + name="widgets", + source="github", + source_url="https://github.com/acme/widgets", + branch="main", + revision_kind="git", + revision_value=revision_value, + revision_ref="refs/heads/main", + local_path="/tmp/x", + status="analysing", + ) + base.update(overrides) + return RepositoryRecord(**base) + + +def test_concurrent_imports_into_an_existing_lineage_get_unique_consecutive_sequences(): + """Two different commits, same canonical pair, an already-existing + lineage: both must serialize on the atomic counter update and receive + distinct, consecutive sequences -- never the same one.""" + from app.repositories.repository_repository import RepositoryRepository + + engine, Session = _make_session_factory() + setup = Session() + try: + owner_id = _make_user(setup) + finally: + setup.close() + + results: list[int] = [] + errors: list[BaseException] = [] + results_lock = threading.Lock() + start = threading.Barrier(2) + + def worker(revision_value: str) -> None: + session = Session() + try: + start.wait(timeout=10) + persisted = RepositoryRepository(session).add_with_lineage( + _repository_record(owner_id, revision_value), + owner_id=owner_id, + canonical_source_key="github.com/acme/widgets", + canonical_branch="refs/heads/main", + display_name="widgets", + ) + with results_lock: + results.append(persisted.sequence) + except BaseException as exc: # noqa: BLE001 -- captured for the assertion below, not swallowed + with results_lock: + errors.append(exc) + finally: + session.close() + + threads = [threading.Thread(target=worker, args=(sha,)) for sha in ("a" * 40, "b" * 40)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=15) + + try: + assert not [t for t in threads if t.is_alive()], "a worker thread did not finish within the timeout" + assert not errors, errors + assert sorted(results) == [1, 2], results + + verify = Session() + try: + from app.models.repository_lineage import RepositoryLineage + + lineage = verify.scalars(select(RepositoryLineage).where(RepositoryLineage.owner_id == owner_id)).one() + assert lineage.next_sequence == 3 + finally: + verify.close() + finally: + _cleanup_owner(Session, owner_id) + engine.dispose() + + +def test_two_first_imports_racing_create_exactly_one_lineage(): + """No lineage exists yet for this canonical pair; two callers race to + create it. The canonical partial unique index must let exactly one win; + the loser reloads the winner's lineage and retries, ending with one + lineage and two distinct sequences -- never two lineages.""" + from app.repositories.repository_repository import RepositoryRepository + + engine, Session = _make_session_factory() + setup = Session() + try: + owner_id = _make_user(setup) + finally: + setup.close() + + results: list[int] = [] + errors: list[BaseException] = [] + results_lock = threading.Lock() + start = threading.Barrier(2) + + def worker(revision_value: str) -> None: + session = Session() + try: + start.wait(timeout=10) + persisted = RepositoryRepository(session).add_with_lineage( + _repository_record(owner_id, revision_value), + owner_id=owner_id, + canonical_source_key="github.com/acme/first-race", + canonical_branch="refs/heads/main", + display_name="first-race", + ) + with results_lock: + results.append(persisted.sequence) + except BaseException as exc: # noqa: BLE001 + with results_lock: + errors.append(exc) + finally: + session.close() + + threads = [threading.Thread(target=worker, args=(sha,)) for sha in ("c" * 40, "d" * 40)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=15) + + try: + assert not [t for t in threads if t.is_alive()], "a worker thread did not finish within the timeout" + assert not errors, errors + assert sorted(results) == [1, 2], results + + verify = Session() + try: + from app.models.repository_lineage import RepositoryLineage + + lineages = verify.scalars( + select(RepositoryLineage).where( + RepositoryLineage.owner_id == owner_id, + RepositoryLineage.canonical_source_key == "github.com/acme/first-race", + ) + ).all() + assert len(lineages) == 1, f"expected exactly one lineage, found {len(lineages)}" + finally: + verify.close() + finally: + _cleanup_owner(Session, owner_id) + engine.dispose() + + +def test_two_identical_commits_racing_produce_one_repository_and_one_conflict(): + """The exact same commit imported twice, concurrently: the loser must + observe the duplicate after serialization and be rejected -- never a + second repository row, and never a wasted/skipped sequence number.""" + from app.repositories.repository_repository import LineageDuplicateRevision, RepositoryRepository + + engine, Session = _make_session_factory() + setup = Session() + try: + owner_id = _make_user(setup) + finally: + setup.close() + + outcomes: list[str] = [] + outcomes_lock = threading.Lock() + start = threading.Barrier(2) + shared_commit = "e" * 40 + + def worker() -> None: + session = Session() + outcome = "error" + try: + start.wait(timeout=10) + RepositoryRepository(session).add_with_lineage( + _repository_record(owner_id, shared_commit, id=str(uuid.uuid4())), + owner_id=owner_id, + canonical_source_key="github.com/acme/identical-race", + canonical_branch="refs/heads/main", + display_name="identical-race", + ) + outcome = "ok" + except LineageDuplicateRevision: + outcome = "rejected" + finally: + session.close() + with outcomes_lock: + outcomes.append(outcome) + + threads = [threading.Thread(target=worker) for _ in range(2)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=15) + + try: + assert not [t for t in threads if t.is_alive()], "a worker thread did not finish within the timeout" + assert sorted(outcomes) == ["ok", "rejected"], outcomes + + verify = Session() + try: + from app.models.repository import RepositoryRecord + from app.models.repository_lineage import RepositoryLineage + + repos = verify.scalars( + select(RepositoryRecord).where( + RepositoryRecord.owner_id == owner_id, RepositoryRecord.revision_value == shared_commit + ) + ).all() + assert len(repos) == 1, "exactly one repository row must exist for the winning commit" + lineage = verify.scalars( + select(RepositoryLineage).where( + RepositoryLineage.owner_id == owner_id, + RepositoryLineage.canonical_source_key == "github.com/acme/identical-race", + ) + ).one() + assert lineage.next_sequence == 2, "the rejected duplicate must not have burned a sequence number" + finally: + verify.close() + finally: + _cleanup_owner(Session, owner_id) + engine.dispose() + + +def test_forced_duplicate_lineage_sequence_pair_is_rejected(): + """Direct proof of the `uq_repositories_lineage_sequence` constraint on + real Postgres, bypassing the allocator entirely.""" + from app.models.repository_lineage import RepositoryLineage + + engine, Session = _make_session_factory() + session = Session() + owner_id = None + try: + owner_id = _make_user(session) + lineage = RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=owner_id, + canonical_source_key="github.com/acme/forced-dup", + canonical_branch="refs/heads/main", + display_name="forced-dup", + latest_repository_id=None, + next_sequence=2, + created_at=datetime.now(UTC), + ) + session.add(lineage) + session.add(_repository_record(owner_id, "f" * 40, lineage_id=lineage.id, sequence=1)) + session.commit() + + session.add(_repository_record(owner_id, "1" * 40, lineage_id=lineage.id, sequence=1)) + with pytest.raises(IntegrityError): + session.commit() + session.rollback() + finally: + session.close() + if owner_id: + _cleanup_owner(Session, owner_id) + engine.dispose() + + +def test_cross_owner_composite_membership_is_rejected_on_real_postgres(): + """Direct proof of `fk_repositories_lineage_owner` on real Postgres: a + repository can never attach to a lineage owned by a different user, even + with a forced direct write that bypasses the service layer.""" + from app.models.repository_lineage import RepositoryLineage + + engine, Session = _make_session_factory() + session = Session() + owner_id = None + other_owner_id = None + try: + owner_id = _make_user(session) + other_owner_id = _make_user(session) + lineage = RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=other_owner_id, + canonical_source_key="github.com/acme/cross-owner", + canonical_branch="refs/heads/main", + display_name="cross-owner", + latest_repository_id=None, + next_sequence=1, + created_at=datetime.now(UTC), + ) + session.add(lineage) + session.commit() + + session.add(_repository_record(owner_id, "2" * 40, lineage_id=lineage.id, sequence=1)) + with pytest.raises(IntegrityError): + session.commit() + session.rollback() + finally: + session.close() + for cleanup_owner in (owner_id, other_owner_id): + if cleanup_owner: + _cleanup_owner(Session, cleanup_owner) + engine.dispose() + + +def test_cross_lineage_latest_pointer_is_rejected_on_real_postgres(): + """Direct proof of `fk_repository_lineages_latest_member` on real + Postgres: a lineage's latest pointer can never name a repository that + actually belongs to a different lineage.""" + from app.models.repository_lineage import RepositoryLineage + + engine, Session = _make_session_factory() + session = Session() + owner_id = None + try: + owner_id = _make_user(session) + lineage_a = RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=owner_id, + canonical_source_key="github.com/acme/lineage-a", + canonical_branch="refs/heads/main", + display_name="lineage-a", + latest_repository_id=None, + next_sequence=2, + created_at=datetime.now(UTC), + ) + lineage_b = RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=owner_id, + canonical_source_key="github.com/acme/lineage-b", + canonical_branch="refs/heads/main", + display_name="lineage-b", + latest_repository_id=None, + next_sequence=2, + created_at=datetime.now(UTC), + ) + session.add_all([lineage_a, lineage_b]) + record_in_b = _repository_record(owner_id, "3" * 40, lineage_id=lineage_b.id, sequence=1) + session.add(record_in_b) + session.commit() + + # Point lineage_a's latest at a repository that actually belongs to + # lineage_b -- must be rejected. + lineage_a.latest_repository_id = record_in_b.id + with pytest.raises(IntegrityError): + session.commit() + session.rollback() + finally: + session.close() + if owner_id: + _cleanup_owner(Session, owner_id) + engine.dispose() + + +def test_account_deletion_cascades_lineages_for_that_owner_only(): + from app.models.repository_lineage import RepositoryLineage + from app.models.user import User + + engine, Session = _make_session_factory() + session = Session() + owner_id = None + other_owner_id = None + try: + owner_id = _make_user(session) + other_owner_id = _make_user(session) + session.add( + RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=owner_id, + canonical_source_key="github.com/acme/deleted-owner", + canonical_branch="refs/heads/main", + display_name="deleted-owner", + latest_repository_id=None, + next_sequence=1, + created_at=datetime.now(UTC), + ) + ) + other_lineage_id = str(uuid.uuid4()) + session.add( + RepositoryLineage( + id=other_lineage_id, + owner_id=other_owner_id, + canonical_source_key="github.com/acme/surviving-owner", + canonical_branch="refs/heads/main", + display_name="surviving-owner", + latest_repository_id=None, + next_sequence=1, + created_at=datetime.now(UTC), + ) + ) + session.commit() + + session.delete(session.get(User, owner_id)) + session.commit() + + remaining = session.scalars(select(RepositoryLineage.id)).all() + assert other_lineage_id in remaining + assert not any( + True for _ in session.scalars(select(RepositoryLineage).where(RepositoryLineage.owner_id == owner_id)) + ) + finally: + session.close() + if other_owner_id: + _cleanup_owner(Session, other_owner_id) + engine.dispose() + + +def _cleanup_owner(Session, owner_id: str) -> None: + from app.models.repository import RepositoryRecord + from app.models.repository_lineage import RepositoryLineage + from app.models.user import User + + cleanup = Session() + try: + cleanup.query(RepositoryRecord).filter(RepositoryRecord.owner_id == owner_id).delete() + cleanup.query(RepositoryLineage).filter(RepositoryLineage.owner_id == owner_id).delete() + cleanup.query(User).filter(User.id == owner_id).delete() + cleanup.commit() + finally: + cleanup.close() diff --git a/apps/backend/tests/test_repository_lineage_migration.py b/apps/backend/tests/test_repository_lineage_migration.py new file mode 100644 index 00000000..fdd4b261 --- /dev/null +++ b/apps/backend/tests/test_repository_lineage_migration.py @@ -0,0 +1,521 @@ +"""Migration-level coverage for #299 (RFC-0002), revisions 0013/0014. + +Covers the plan's §13 "Migration tests" matrix: fresh empty DB shape, a +populated DB with the exact grouping/exclusion cases the backfill must get +right, deterministic tie-breaks, idempotent rerun, and full downgrade/ +upgrade round trips on both SQLite and (when ``PARTHA_TEST_PG_URL`` is set) +real PostgreSQL. +""" + +import importlib.util +import os +import uuid +from datetime import UTC, datetime, timedelta +from types import ModuleType + +import pytest +from alembic import command +from alembic.config import Config +from alembic.migration import MigrationContext +from alembic.operations import Operations +from sqlalchemy import MetaData, Table, create_engine, inspect, select +from sqlalchemy.engine import make_url + +from pathlib import Path + +BACKEND_ROOT = Path(__file__).resolve().parents[1] +PG_URL = os.environ.get("PARTHA_TEST_PG_URL") + + +def _alembic_config() -> Config: + cfg = Config(str(BACKEND_ROOT / "alembic.ini")) + cfg.set_main_option("script_location", str(BACKEND_ROOT / "alembic")) + return cfg + + +def _load_migration_module(revision_filename: str) -> ModuleType: + """Load a migration script's own module so its private helper functions + (e.g. ``_backfill_lineages``) can be called directly in a test, the same + way Alembic itself loads and executes it -- not via a package import, + since revision modules are not importable by their filename (it starts + with a digit) and are never meant to be imported by application code.""" + path = BACKEND_ROOT / "alembic" / "versions" / f"{revision_filename}.py" + spec = importlib.util.spec_from_file_location(revision_filename, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _database_url(tmp_path) -> str: + if not PG_URL: + return f"sqlite:///{tmp_path / 'lineage-migration.db'}" + admin_url = make_url(PG_URL) + database_name = f"partha_lineage_migration_{uuid.uuid4().hex}" + admin_engine = create_engine(admin_url, isolation_level="AUTOCOMMIT") + try: + with admin_engine.connect() as connection: + connection.exec_driver_sql(f'CREATE DATABASE "{database_name}"') + finally: + admin_engine.dispose() + return admin_url.set(database=database_name).render_as_string(hide_password=False) + + +def _drop_pg_database(database_url: str) -> None: + if not PG_URL: + return + admin_url = make_url(PG_URL) + target = make_url(database_url) + admin_engine = create_engine(admin_url, isolation_level="AUTOCOMMIT") + try: + with admin_engine.connect() as connection: + connection.exec_driver_sql(f'DROP DATABASE IF EXISTS "{target.database}" WITH (FORCE)') + finally: + admin_engine.dispose() + + +@pytest.fixture() +def lineage_migration_db(tmp_path, monkeypatch): + # Import for its side effect: registers the `PRAGMA foreign_keys=ON` + # connect-event listener on the SQLAlchemy Engine class globally (see + # app/core/database.py). Without this import having already happened + # somewhere in the process, SQLite silently never enforces any foreign + # key at all -- these migration tests would then "pass" while proving + # nothing about the cyclic FK's actual behavior, exactly as happened + # once during development (caught only by incidental test-file + # ordering in a full-suite run, not by this file in isolation). Forcing + # it here makes that guarantee deterministic instead of accidental. + import app.core.database # noqa: F401 + + database_url = _database_url(tmp_path) + monkeypatch.setenv("DATABASE_URL", database_url) + monkeypatch.setenv("CORS_ORIGINS", "http://testserver") + from app.core import config + + config.get_settings.cache_clear() + engine = create_engine(database_url) + try: + yield database_url, engine + finally: + engine.dispose() + config.get_settings.cache_clear() + _drop_pg_database(database_url) + + +def test_fresh_database_reaches_head_with_the_expected_lineage_shape(lineage_migration_db): + database_url, engine = lineage_migration_db + cfg = _alembic_config() + + command.upgrade(cfg, "head") + + insp = inspect(engine) + assert "repository_lineages" in insp.get_table_names() + + lineage_columns = {column["name"] for column in insp.get_columns("repository_lineages")} + assert lineage_columns == { + "id", + "owner_id", + "canonical_source_key", + "canonical_branch", + "display_name", + "latest_repository_id", + "next_sequence", + "created_at", + } + + repo_columns = {column["name"] for column in insp.get_columns("repositories")} + assert {"lineage_id", "sequence"} <= repo_columns + + repo_fk_names = {fk["name"] for fk in insp.get_foreign_keys("repositories")} + assert "fk_repositories_lineage_owner" in repo_fk_names + lineage_fk_names = {fk["name"] for fk in insp.get_foreign_keys("repository_lineages")} + assert {"fk_repository_lineages_owner_id_users", "fk_repository_lineages_latest_member"} <= lineage_fk_names + + latest_member_fk = next( + fk + for fk in insp.get_foreign_keys("repository_lineages") + if fk["name"] == "fk_repository_lineages_latest_member" + ) + assert latest_member_fk["constrained_columns"] == ["latest_repository_id", "id"] + assert latest_member_fk["referred_columns"] == ["id", "lineage_id"] + assert latest_member_fk["options"].get("deferrable") is True + + repo_uk_names = {uk["name"] for uk in insp.get_unique_constraints("repositories")} + assert {"uq_repositories_lineage_sequence", "uq_repositories_id_lineage"} <= repo_uk_names + + index_names = {index["name"] for index in insp.get_indexes("repository_lineages")} + assert "uq_repository_lineages_owner_source_branch" in index_names + assert "ix_repository_lineages_owner_id" in index_names + + command.downgrade(cfg, "base") + assert "repository_lineages" not in inspect(engine).get_table_names() + + command.upgrade(cfg, "head") + assert "repository_lineages" in inspect(engine).get_table_names() + + +def _seed_users_and_repositories(engine, rows: list[dict]) -> tuple[str, str]: + """Insert two users and the given legacy-shaped repository rows at 0012, + before 0013/0014 exist. Returns (owner_a, owner_b).""" + meta = MetaData() + users = Table("users", meta, autoload_with=engine) + repositories = Table("repositories", meta, autoload_with=engine) + + owner_a = str(uuid.uuid4()) + owner_b = str(uuid.uuid4()) + now = datetime.now(UTC) + with engine.begin() as connection: + connection.execute( + users.insert(), + [ + { + "id": owner_a, + "email": f"a-{uuid.uuid4().hex}@example.com", + "is_active": True, + "created_at": now, + "updated_at": now, + "password_hash": None, + }, + { + "id": owner_b, + "email": f"b-{uuid.uuid4().hex}@example.com", + "is_active": True, + "created_at": now, + "updated_at": now, + "password_hash": None, + }, + ], + ) + if rows: + connection.execute(repositories.insert(), rows) + return owner_a, owner_b + + +def _repo_row(owner_id: str, **overrides) -> dict: + now = datetime.now(UTC) + base = dict( + id=str(uuid.uuid4()), + owner_id=owner_id, + name="demo", + description=None, + source="github", + source_url="https://github.com/acme/widgets", + branch="main", + local_path="/x", + size=0, + file_count=1, + status="completed", + analysis_stage=None, + analysis_progress=100, + uploaded_at=now, + analysed_at=now, + error_message=None, + repo_metadata=None, + file_tree=[], + created_at=now, + updated_at=now, + revision_kind="git", + revision_value="a" * 40, + revision_ref="refs/heads/main", + ) + base.update(overrides) + return base + + +def test_backfill_groups_correctly_and_leaves_ineligible_rows_standalone(lineage_migration_db): + """One populated-DB test exercising every §6 grouping/exclusion case at + once: same source/ref groups; different ref, different repo, and + different owner each get a separate lineage; a URL variant (mixed-case + host, .git suffix, trailing slash) still canonicalizes into the same + lineage; a malformed URL, an unresolved ref, and an upload all stay + standalone; and a deterministic timestamp tie breaks on repository id. + """ + database_url, engine = lineage_migration_db + cfg = _alembic_config() + command.upgrade(cfg, "0012_waitlist_entries") + + now = datetime.now(UTC) + owner_a, owner_b = _seed_users_and_repositories(engine, []) + + meta = MetaData() + repositories = Table("repositories", meta, autoload_with=engine) + + group_first = str(uuid.uuid4()) + group_second = str(uuid.uuid4()) + variant_row_id = str(uuid.uuid4()) + dev_row_id = str(uuid.uuid4()) + other_owner_row_id = str(uuid.uuid4()) + malformed_row_id = str(uuid.uuid4()) + unresolved_row_id = str(uuid.uuid4()) + upload_row_id = str(uuid.uuid4()) + tie_a = "20000000-0000-0000-0000-000000000001" + tie_b = "20000000-0000-0000-0000-000000000002" + + rows = [ + _repo_row( + owner_a, + id=group_first, + revision_value="a" * 40, + created_at=now - timedelta(days=2), + ), + _repo_row( + owner_a, + id=group_second, + revision_value="b" * 40, + created_at=now - timedelta(days=1), + ), + _repo_row( + owner_a, + id=variant_row_id, + source_url="https://GitHub.com/Acme/Widgets.git/", + revision_value="c" * 40, + created_at=now, + ), + _repo_row(owner_a, id=dev_row_id, revision_ref="refs/heads/dev", revision_value="d" * 40, created_at=now), + _repo_row(owner_b, id=other_owner_row_id, revision_value="e" * 40, created_at=now), + _repo_row(owner_a, id=malformed_row_id, source_url="not a url at all", revision_value="f" * 40, created_at=now), + _repo_row( + owner_a, + id=unresolved_row_id, + source_url="https://github.com/acme/other", + revision_ref=None, + revision_value="1" * 40, + created_at=now, + ), + dict( + id=upload_row_id, + owner_id=owner_a, + name="upload1", + description=None, + source="upload", + source_url=None, + branch=None, + local_path="/y", + size=0, + file_count=1, + status="completed", + analysis_stage=None, + analysis_progress=100, + uploaded_at=now, + analysed_at=now, + error_message=None, + repo_metadata=None, + file_tree=[], + created_at=now, + updated_at=now, + revision_kind="upload", + revision_value="sha256:" + "0" * 64, + revision_ref=None, + ), + _repo_row( + owner_a, + id=tie_a, + source_url="https://github.com/acme/tied", + revision_value="2" * 40, + created_at=now, + ), + _repo_row( + owner_a, + id=tie_b, + source_url="https://github.com/acme/tied", + revision_value="3" * 40, + created_at=now, + ), + ] + with engine.begin() as connection: + connection.execute(repositories.insert(), rows) + + command.upgrade(cfg, "head") + + meta2 = MetaData() + repos2 = Table("repositories", meta2, autoload_with=engine) + lineages2 = Table("repository_lineages", meta2, autoload_with=engine) + + with engine.connect() as connection: + attached = { + row.id: (row.lineage_id, row.sequence) + for row in connection.execute(select(repos2.c.id, repos2.c.lineage_id, repos2.c.sequence)) + } + + # The primary group: two original commits plus the URL-variant row, + # in creation order. + assert attached[group_first][1] == 1 + assert attached[group_second][1] == 2 + assert attached[variant_row_id][1] == 3 + primary_lineage = attached[group_first][0] + assert attached[group_second][0] == primary_lineage + assert attached[variant_row_id][0] == primary_lineage + + # A different ref is a different lineage. + assert attached[dev_row_id][0] != primary_lineage + assert attached[dev_row_id][1] == 1 + + # A different owner is a different lineage, even for the same + # canonical source/ref. + assert attached[other_owner_row_id][0] != primary_lineage + assert attached[other_owner_row_id][1] == 1 + + # Ineligible rows stay standalone. + assert attached[malformed_row_id] == (None, None) + assert attached[unresolved_row_id] == (None, None) + assert attached[upload_row_id] == (None, None) + + # Deterministic timestamp tie-break: identical created_at, so the + # lexicographically smaller id (tie_a) gets sequence 1. + assert attached[tie_a][1] == 1 + assert attached[tie_b][1] == 2 + assert attached[tie_a][0] == attached[tie_b][0] + + primary_row = ( + connection.execute( + select( + lineages2.c.owner_id, + lineages2.c.canonical_source_key, + lineages2.c.canonical_branch, + lineages2.c.display_name, + lineages2.c.latest_repository_id, + lineages2.c.next_sequence, + ).where(lineages2.c.id == primary_lineage) + ) + .mappings() + .one() + ) + assert primary_row["owner_id"] == owner_a + assert primary_row["canonical_source_key"] == "github.com/acme/widgets" + assert primary_row["canonical_branch"] == "refs/heads/main" + assert primary_row["display_name"] == "demo" + assert primary_row["latest_repository_id"] == variant_row_id + assert primary_row["next_sequence"] == 4 + + # No stray lineage attachment anywhere else: exactly the 7 eligible + # rows (group_first, group_second, variant, dev, other_owner, tie_a, + # tie_b) carry a lineage, and every (lineage_id, sequence) pair is + # unique. + attachments = [ + (row.lineage_id, row.sequence) + for row in connection.execute(select(repos2.c.lineage_id, repos2.c.sequence)) + if row.lineage_id is not None + ] + assert len(attachments) == 7 + assert len(set(attachments)) == 7 + + command.downgrade(cfg, "base") + command.upgrade(cfg, "head") + + +def test_backfill_rerun_is_idempotent(lineage_migration_db): + """Calling the backfill helper twice against the same already-backfilled + data reconciles to the identical final state rather than erroring or + double-counting (plan §6.2/§7 "interruption and rerun").""" + database_url, engine = lineage_migration_db + cfg = _alembic_config() + command.upgrade(cfg, "0012_waitlist_entries") + + owner_a, _owner_b = _seed_users_and_repositories(engine, []) + meta = MetaData() + repositories = Table("repositories", meta, autoload_with=engine) + now = datetime.now(UTC) + first_id, second_id = str(uuid.uuid4()), str(uuid.uuid4()) + with engine.begin() as connection: + connection.execute( + repositories.insert(), + [ + _repo_row(owner_a, id=first_id, revision_value="a" * 40, created_at=now - timedelta(days=1)), + _repo_row(owner_a, id=second_id, revision_value="b" * 40, created_at=now), + ], + ) + + command.upgrade(cfg, "0013_lineage_expand") + + migration = _load_migration_module("0013_lineage_expand") + + with engine.connect() as connection: + migration_context = MigrationContext.configure(connection) + with connection.begin(), Operations.context(migration_context): + groups_first = migration._backfill_lineages() + migration._verify_backfill(groups_first) + groups_second = migration._backfill_lineages() + migration._verify_backfill(groups_second) + + meta2 = MetaData() + repos2 = Table("repositories", meta2, autoload_with=engine) + lineages2 = Table("repository_lineages", meta2, autoload_with=engine) + with engine.connect() as connection: + lineage_rows = connection.execute(select(lineages2.c.id, lineages2.c.next_sequence)).all() + assert len(lineage_rows) == 1 + assert lineage_rows[0].next_sequence == 3 + + sequences = sorted( + row.sequence + for row in connection.execute(select(repos2.c.sequence).where(repos2.c.id.in_([first_id, second_id]))) + ) + assert sequences == [1, 2] + + command.upgrade(cfg, "head") + command.downgrade(cfg, "base") + + +def test_cross_owner_lineage_attachment_is_rejected_by_the_database_even_if_forced(lineage_migration_db): + """The composite deferred ownership FK (`fk_repositories_lineage_owner`) + rejects a forced cross-owner attachment even if application code is + wrong (#299 §9) -- proven against a database built the same way a real + deployment's is, via the actual Alembic migrations, not + `Base.metadata.create_all()`. + + This deliberately does not use the `client`/`auth_client` fixtures + (which bootstrap their schema via `create_all()`): resolving this + table pair's genuine foreign-key cycle there requires emitting one + table with an inline FK referencing the other before it exists, and at + least one SQLite build encountered in CI does not enforce a deferred FK + declared that way -- confirmed directly: the identical scenario built + on `create_all()` failed `PRAGMA foreign_key_check` outright on that + build, while raising `IntegrityError` reliably every time on other + platforms. The migration never creates that inline forward reference -- + 0014 adds this exact constraint in a second revision, after both tables + already exist -- which is what makes this version of the assertion + reliable everywhere instead of platform-dependent. + """ + database_url, engine = lineage_migration_db + cfg = _alembic_config() + command.upgrade(cfg, "head") + + owner_a, owner_b = _seed_users_and_repositories(engine, []) + + meta = MetaData() + lineages = Table("repository_lineages", meta, autoload_with=engine) + repositories = Table("repositories", meta, autoload_with=engine) + + now = datetime.now(UTC) + lineage_id = str(uuid.uuid4()) + with engine.begin() as connection: + connection.execute( + lineages.insert().values( + id=lineage_id, + owner_id=owner_b, + canonical_source_key="github.com/other/repo2", + canonical_branch="refs/heads/main", + display_name="repo2", + latest_repository_id=None, + next_sequence=1, + created_at=now, + ) + ) + + from sqlalchemy.exc import IntegrityError + + with engine.connect() as connection: + transaction = connection.begin() + connection.execute( + repositories.insert().values( + **_repo_row( + owner_a, + id=str(uuid.uuid4()), + name="cross-owner-attempt", + lineage_id=lineage_id, + sequence=1, + ) + ) + ) + with pytest.raises(IntegrityError): + transaction.commit() + transaction.rollback() diff --git a/apps/backend/tests/test_repository_lineage_service.py b/apps/backend/tests/test_repository_lineage_service.py new file mode 100644 index 00000000..a0d3d0c4 --- /dev/null +++ b/apps/backend/tests/test_repository_lineage_service.py @@ -0,0 +1,370 @@ +"""Service-level coverage for #299 (RFC-0002): live import/delete lineage +allocation, ownership, and the cyclic FK's real enforcement. + +HTTP-level tests exercise the actual `/repositories/github` and +`/repositories/upload` routes with a faked git clone (same idiom as +test_ingestion_pipeline.py), so lineage assignment is proven end to end, not +just at the repository-layer unit. Repository-layer tests exercise +`RepositoryRepository.add_with_lineage`/`delete_with_lineage_update` +directly for cases an HTTP request can't force (a same-lineage race, a +deliberately-forced FK violation). +""" + +import uuid +from datetime import UTC, datetime +from pathlib import Path + +import pytest + +from app.core.database import SessionLocal +from app.github.client import GitHubClient +from app.models.repository import RepositoryRecord +from app.models.repository_lineage import RepositoryLineage +from app.repositories.repository_repository import RepositoryRepository +from tests.api_assertions import assert_error_response + + +def _fake_clone(_: GitHubClient, __: str, destination: Path, ___: str | None = None) -> None: + destination.mkdir(parents=True, exist_ok=True) + (destination / "README.md").write_text("# demo\n", encoding="utf-8") + + +def _mock_github(monkeypatch: pytest.MonkeyPatch, commits: list[str], ref: str = "refs/heads/main") -> None: + commit_iter = iter(commits) + monkeypatch.setattr(GitHubClient, "clone_public_repository", _fake_clone) + monkeypatch.setattr(GitHubClient, "read_head_commit", lambda *_: next(commit_iter)) + monkeypatch.setattr(GitHubClient, "read_head_ref", lambda *_: ref) + + +def _lineage_of(response_json: dict) -> tuple[str | None, int | None]: + with SessionLocal() as db: + record = db.get(RepositoryRecord, response_json["id"]) + assert record is not None + return record.lineage_id, record.sequence + + +# -------------------------------------------------------------------------- +# Live import: lineage assignment +# -------------------------------------------------------------------------- + + +def test_first_github_import_creates_a_lineage_with_sequence_one(auth_client, monkeypatch: pytest.MonkeyPatch): + _mock_github(monkeypatch, ["a" * 40]) + + response = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + + assert response.status_code == 201, response.text + lineage_id, sequence = _lineage_of(response.json()) + assert lineage_id is not None + assert sequence == 1 + with SessionLocal() as db: + lineage = db.get(RepositoryLineage, lineage_id) + assert lineage is not None + assert lineage.canonical_source_key == "github.com/acme/widgets" + assert lineage.canonical_branch == "refs/heads/main" + assert lineage.display_name == "widgets" + assert lineage.latest_repository_id == response.json()["id"] + assert lineage.next_sequence == 2 + + +def test_second_commit_on_same_source_and_ref_reuses_the_lineage_and_increments( + auth_client, monkeypatch: pytest.MonkeyPatch +): + _mock_github(monkeypatch, ["a" * 40, "b" * 40]) + + first = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + second = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + + assert first.status_code == 201 and second.status_code == 201 + first_lineage, first_sequence = _lineage_of(first.json()) + second_lineage, second_sequence = _lineage_of(second.json()) + assert first_lineage == second_lineage + assert (first_sequence, second_sequence) == (1, 2) + with SessionLocal() as db: + lineage = db.get(RepositoryLineage, first_lineage) + assert lineage.latest_repository_id == second.json()["id"] + assert lineage.next_sequence == 3 + + +def test_owner_repo_case_variants_of_the_same_url_match_the_same_lineage(auth_client, monkeypatch: pytest.MonkeyPatch): + """Live validation allows mixed-case owner/repo (only the host must be + exact-case), so two spellings of the same repository must still land in + the same lineage once case-folded (#299 §8.1).""" + _mock_github(monkeypatch, ["a" * 40, "b" * 40]) + + first = auth_client.post("/repositories/github", json={"url": "https://github.com/Acme/Widgets"}) + second = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + + assert first.status_code == 201 and second.status_code == 201 + first_lineage, _ = _lineage_of(first.json()) + second_lineage, second_sequence = _lineage_of(second.json()) + assert first_lineage == second_lineage + assert second_sequence == 2 + + +def test_different_ref_gets_a_different_lineage(auth_client, monkeypatch: pytest.MonkeyPatch): + _mock_github(monkeypatch, ["a" * 40], ref="refs/heads/main") + main_response = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + _mock_github(monkeypatch, ["b" * 40], ref="refs/heads/dev") + dev_response = auth_client.post( + "/repositories/github", json={"url": "https://github.com/acme/widgets", "branch": "dev"} + ) + + assert main_response.status_code == 201 and dev_response.status_code == 201 + main_lineage, _ = _lineage_of(main_response.json()) + dev_lineage, dev_sequence = _lineage_of(dev_response.json()) + assert main_lineage != dev_lineage + assert dev_sequence == 1 + + +def test_different_repository_gets_a_different_lineage(auth_client, monkeypatch: pytest.MonkeyPatch): + _mock_github(monkeypatch, ["a" * 40, "b" * 40]) + + widgets = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + gadgets = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/gadgets"}) + + assert widgets.status_code == 201 and gadgets.status_code == 201 + widgets_lineage, _ = _lineage_of(widgets.json()) + gadgets_lineage, gadgets_sequence = _lineage_of(gadgets.json()) + assert widgets_lineage != gadgets_lineage + assert gadgets_sequence == 1 + + +def test_different_owner_gets_a_different_lineage_even_for_the_same_source_and_ref( + auth_client, make_auth_headers, monkeypatch: pytest.MonkeyPatch +): + _mock_github(monkeypatch, ["a" * 40]) + primary = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + assert primary.status_code == 201 + + other = make_auth_headers("other-owner@example.com") + _mock_github(monkeypatch, ["a" * 40]) + other_response = auth_client.post( + "/repositories/github", json={"url": "https://github.com/acme/widgets"}, headers=other["headers"] + ) + + assert other_response.status_code == 201 + primary_lineage, _ = _lineage_of(primary.json()) + other_lineage, other_sequence = _lineage_of(other_response.json()) + assert primary_lineage != other_lineage + assert other_sequence == 1 + with SessionLocal() as db: + assert db.get(RepositoryLineage, other_lineage).owner_id == other["user"]["id"] + + +def test_same_commit_is_allowed_in_two_different_branch_lineages(auth_client, monkeypatch: pytest.MonkeyPatch): + shared_commit = "c" * 40 + _mock_github(monkeypatch, [shared_commit], ref="refs/heads/main") + main_response = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + _mock_github(monkeypatch, [shared_commit], ref="refs/heads/release") + release_response = auth_client.post( + "/repositories/github", json={"url": "https://github.com/acme/widgets", "branch": "release"} + ) + + assert main_response.status_code == 201 + assert release_response.status_code == 201 + assert main_response.json()["revision"]["value"] == shared_commit + assert release_response.json()["revision"]["value"] == shared_commit + main_lineage, _ = _lineage_of(main_response.json()) + release_lineage, _ = _lineage_of(release_response.json()) + assert main_lineage != release_lineage + + +def test_the_same_commit_twice_in_one_lineage_is_still_rejected_as_a_duplicate( + auth_client, monkeypatch: pytest.MonkeyPatch +): + """The pre-existing 409 behaviour must survive #299's rewrite of the + persistence path -- this is the authoritative, transactional duplicate + check now, not the old owner+source_url pre-clone check.""" + _mock_github(monkeypatch, ["a" * 40, "a" * 40]) + + first = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + duplicate = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + + assert first.status_code == 201 + error = assert_error_response(duplicate, 409, "conflict_error") + assert error.details == {"repositoryId": first.json()["id"], "name": first.json()["name"]} + + with SessionLocal() as db: + lineage_id, _ = _lineage_of(first.json()) + lineage = db.get(RepositoryLineage, lineage_id) + # The rejected duplicate must not have burned a sequence number. + assert lineage.next_sequence == 2 + + +def test_upload_never_creates_or_touches_a_lineage(auth_client): + response = auth_client.post( + "/repositories/upload", + files={"file": ("demo.zip", _minimal_zip(), "application/octet-stream")}, + ) + + assert response.status_code == 201, response.text + lineage_id, sequence = _lineage_of(response.json()) + assert (lineage_id, sequence) == (None, None) + + +def _minimal_zip() -> bytes: + import io + import zipfile + + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + archive.writestr("README.md", "# demo\n") + return buffer.getvalue() + + +def test_a_failed_lineage_duplicate_insert_cleans_up_the_staged_repository_directory( + auth_client, monkeypatch: pytest.MonkeyPatch +): + """#299 §5.3 extends the existing pre-clone cleanup across the + transactional insert phase: a rejected duplicate must not leave an + orphaned directory under storage.""" + _mock_github(monkeypatch, ["a" * 40, "a" * 40]) + + first = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + assert first.status_code == 201 + + from app.core.config import get_settings + from app.storage.local import LocalStorage + + storage = LocalStorage(get_settings()) + before = set(storage.repositories_root.iterdir()) if storage.repositories_root.exists() else set() + + duplicate = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + assert duplicate.status_code == 409 + + after = set(storage.repositories_root.iterdir()) if storage.repositories_root.exists() else set() + assert after == before, "the duplicate's staged directory must be removed, not left behind" + + +# -------------------------------------------------------------------------- +# Deletion: latest-pointer rollback +# -------------------------------------------------------------------------- + + +def test_deleting_a_non_latest_member_leaves_the_latest_pointer_unchanged(auth_client, monkeypatch: pytest.MonkeyPatch): + _mock_github(monkeypatch, ["a" * 40, "b" * 40, "c" * 40]) + auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + second = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + third = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + lineage_id, _ = _lineage_of(third.json()) + + delete_response = auth_client.delete(f"/repositories/{second.json()['id']}") + assert delete_response.status_code == 204 + + with SessionLocal() as db: + lineage = db.get(RepositoryLineage, lineage_id) + assert lineage.latest_repository_id == third.json()["id"] + assert lineage.next_sequence == 4 # counter never decreases + + +def test_deleting_the_latest_member_rolls_back_to_the_next_highest_surviving_sequence( + auth_client, monkeypatch: pytest.MonkeyPatch +): + _mock_github(monkeypatch, ["a" * 40, "b" * 40, "c" * 40]) + first = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + second = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + third = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + lineage_id, _ = _lineage_of(first.json()) + + delete_response = auth_client.delete(f"/repositories/{third.json()['id']}") + assert delete_response.status_code == 204 + + with SessionLocal() as db: + lineage = db.get(RepositoryLineage, lineage_id) + assert lineage.latest_repository_id == second.json()["id"] + assert lineage.next_sequence == 4 + + +def test_deleting_the_last_member_keeps_an_empty_lineage_and_never_reuses_its_sequence( + auth_client, monkeypatch: pytest.MonkeyPatch +): + _mock_github(monkeypatch, ["a" * 40]) + only = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + lineage_id, _ = _lineage_of(only.json()) + + delete_response = auth_client.delete(f"/repositories/{only.json()['id']}") + assert delete_response.status_code == 204 + + with SessionLocal() as db: + lineage = db.get(RepositoryLineage, lineage_id) + assert lineage is not None, "an empty lineage is kept, not garbage collected" + assert lineage.latest_repository_id is None + assert lineage.next_sequence == 2 + + _mock_github(monkeypatch, ["b" * 40]) + reimport = auth_client.post("/repositories/github", json={"url": "https://github.com/acme/widgets"}) + assert reimport.status_code == 201 + reimport_lineage, reimport_sequence = _lineage_of(reimport.json()) + assert reimport_lineage == lineage_id + assert reimport_sequence == 2 # sequence 1 is never reused + + +# -------------------------------------------------------------------------- +# Cross-owner isolation +# -------------------------------------------------------------------------- + + +def test_cross_owner_lineage_lookup_never_matches_another_owners_row(auth_client, make_auth_headers): + """A pre-existing lineage owned by another user, with the exact same + canonical key a fresh import will compute, must never be reused -- + proven through the real allocation path (`add_with_lineage`), not by + asserting a private lookup helper's return value in isolation.""" + other = make_auth_headers("owner-b@example.com") + with SessionLocal() as db: + other_lineage = RepositoryLineage( + id=str(uuid.uuid4()), + owner_id=other["user"]["id"], + canonical_source_key="github.com/shared/repo", + canonical_branch="refs/heads/main", + display_name="repo", + latest_repository_id=None, + next_sequence=1, + created_at=datetime.now(UTC), + ) + db.add(other_lineage) + db.commit() + other_lineage_id = other_lineage.id + + with SessionLocal() as db: + repository_repo = RepositoryRepository(db) + record = RepositoryRecord( + id=str(uuid.uuid4()), + owner_id=auth_client.default_user["id"], # type: ignore[attr-defined] + name="repo", + source="github", + source_url="https://github.com/shared/repo", + branch="main", + revision_kind="git", + revision_value="e" * 40, + revision_ref="refs/heads/main", + local_path="/tmp/x", + status="analysing", + ) + persisted = repository_repo.add_with_lineage( + record, + owner_id=auth_client.default_user["id"], # type: ignore[attr-defined] + canonical_source_key="github.com/shared/repo", + canonical_branch="refs/heads/main", + display_name="repo", + ) + assert persisted.lineage_id != other_lineage_id + assert persisted.sequence == 1 + + +# Direct proof that the composite ownership FK rejects a forced cross-owner +# attachment lives in tests/test_repository_lineage_migration.py (SQLite, via +# a real Alembic-migrated database) and +# tests/test_repository_lineage_concurrency.py (real PostgreSQL). See the +# commit history on this file for why: the same assertion built on top of +# this file's `auth_client` fixture (which bootstraps its schema via +# `Base.metadata.create_all()`, not migrations) was not reliable across +# SQLite builds -- `create_all()` must resolve this table pair's genuine +# foreign-key cycle by emitting one table with an inline FK referencing the +# other table before it exists, and at least one SQLite build encountered in +# CI does not enforce a deferred FK declared that way, even though it stores +# and reports the declaration correctly. The Alembic migration never does +# this -- it adds the constraint in a second revision after both tables +# already exist -- which is why the migration-backed version of this +# assertion is reliable and this file's version was removed rather than +# patched a third time. diff --git a/docs/architecture/REPOSITORY_LINEAGE_MIGRATION_PLAN.md b/docs/architecture/REPOSITORY_LINEAGE_MIGRATION_PLAN.md index d6d1e351..6686c8d1 100644 --- a/docs/architecture/REPOSITORY_LINEAGE_MIGRATION_PLAN.md +++ b/docs/architecture/REPOSITORY_LINEAGE_MIGRATION_PLAN.md @@ -402,9 +402,11 @@ Do not silently skip a failed update or coerce corrupt data to satisfy final con ## 7. Concrete Alembic migration sequence -Use two new revisions after `0010_account_deletion`, each with an ID under the existing PostgreSQL -`alembic_version VARCHAR(32)` limit. Suggested IDs are `0011_lineage_expand` and -`0012_lineage_constraints`. +Use two new revisions after the current head, each with an ID under the existing PostgreSQL +`alembic_version VARCHAR(32)` limit. Suggested IDs were `0011_lineage_expand` and +`0012_lineage_constraints`; two unrelated migrations (`0011_invite_tokens`, `0012_waitlist_entries`) +landed on `dev` ahead of this one, so the implementation uses `0013_lineage_expand` and +`0014_lineage_constraints` instead. The mechanics below are unaffected by the renumbering. Imports must be quiesced while the migrations run. The application currently has no dual-write compatibility for lineage and a concurrent repository insert could escape the backfill. Existing diff --git a/docs/operations/DATABASE_MIGRATION_REHEARSAL.md b/docs/operations/DATABASE_MIGRATION_REHEARSAL.md index 3b9dc6f3..8b9cfb4c 100644 --- a/docs/operations/DATABASE_MIGRATION_REHEARSAL.md +++ b/docs/operations/DATABASE_MIGRATION_REHEARSAL.md @@ -7,7 +7,7 @@ It documents a reproducible rehearsal, not a promise that all historical data ca ## Current support and evidence PARTHA uses one linear Alembic chain, from `0001_initial` through current head -`0010_account_deletion`. SQLite is the local-development default and is the local +`0014_lineage_constraints`. SQLite is the local-development default and is the local maintainer rehearsal target. PostgreSQL is the supported deployment and CI dialect: the Backend CI job runs this rehearsal command against its isolated PostgreSQL 16 service after the backend tests. Repository files are outside the database, under `STORAGE_PATH`; a database restore does not recreate missing repository