diff --git a/alembic/versions/036_partial_index_user_agent_slug_active.py b/alembic/versions/036_partial_index_user_agent_slug_active.py new file mode 100644 index 0000000..195f3b7 --- /dev/null +++ b/alembic/versions/036_partial_index_user_agent_slug_active.py @@ -0,0 +1,137 @@ +"""OSS port — slug uniqueness scoped to active rows via partial index. + +Surface: ``agents`` table — replace the full UNIQUE constraint +``unique_user_agent_slug (user_id, slug)`` with a partial UNIQUE INDEX +of the same name, scoped to ``WHERE deleted_at IS NULL``. + +## Provenance (parity port from private cueapi) + +Cherry-pick of private cueapi's migration 080 +(``080_partial_index_user_agent_slug_active.py``), shipped in private +cueapi PR #921 (merge_commit ``b770983e``, prod-verified via G14 5/5 +PASS on api.cueapi.ai @ commit 454aac3). Renumbered 080 → 036 to +chain off public's current alembic head (035_agent_live_sessions_ipc_attachment). + +Code shape verbatim; downgrade-safety-rail verbatim. The only +deviation is the migration number (public OSS-track has its own +alembic ordering). + +## Why + +Jingim's cue.dock.svc (which vendors public cueapi-core) hit this +empirically 2026-05-20: soft-deleting an agent on cueapi side leaves +the slug locked. ``POST /v1/agents`` for the same slug returns 409 +``slug_taken``; ``GET /v1/agents/{slug}`` returns 404 (default +``include_deleted=false``). Net: no way to recreate a previously- +soft-deleted slug without going through ``?include_deleted=true`` + +reanimate semantics that don't exist. + +Pre-fix workaround: migration 029 (private's 071 equivalent in OSS +order, if it ports) ``_build_archived_slug`` renames soft-deleted rows +to ``-archived-<8hex>`` to free the original slug for re-use. +Works in cueapi-internal code paths but cross-language consumers +(Dock / @trydock/mcp / presence-runtime) hit the constraint directly. + +Postgres native fix: partial UNIQUE INDEX. The index enforces +uniqueness ONLY across rows with ``deleted_at IS NULL`` (active). +Soft-deleted rows keep their original slug; uniqueness no longer +applies once deleted. + +## What this migration does + +1. Drop the existing full UNIQUE constraint ``unique_user_agent_slug``. +2. Create a UNIQUE INDEX of the same name with + ``WHERE deleted_at IS NULL``. + +The index name is preserved so the existing IntegrityError-error-text +match at ``app/services/agent_service.py`` (``if "unique_user_agent_slug" +in str(e.orig)``) continues to work without code change. + +## Effect on existing data + +NONE. Existing soft-deleted rows stay as-is. They're all +``deleted_at IS NOT NULL`` → excluded from the partial index → no +uniqueness check applies. Future soft-deletes don't need a rename. + +## Backward compatibility + +The companion route change at ``app/services/agent_service.py`` +(this PR's other half) preserves the existing 409 envelope shape + +adds an ``existing_uuid`` field. Existing clients that ignore +``existing_uuid`` continue to work; new clients (Jingim's Dock, +@trydock/mcp, etc.) read it to skip a GET round-trip. + +## Downgrade safety + +Refuses downgrade if any ``(user_id, slug)`` pair has BOTH an active +AND a soft-deleted row (or multiple soft-deleted rows with the same +slug). Recreating the full UNIQUE constraint would fail at execute +time, leaving the schema in a broken state. Force a deliberate +decision by failing the migration. + +## OSS track-lag note + +Private cueapi additionally raised ``agents.slug`` from ``VARCHAR(64)`` +to ``VARCHAR(128)`` in migration 079 (cmpdl2bam PR #42 Substrate). That +column-width raise is NOT ported here; OSS keeps ``VARCHAR(64)``. Self- +hosters whose slugs fit in 64 chars (the common case) are unaffected. +If OSS users need the 128-char ceiling for labeled-Live composite +slugs, that's a separate port. +""" +from alembic import op +import sqlalchemy as sa + + +revision = "036" +down_revision = "035" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Drop the existing full UNIQUE constraint. + op.drop_constraint( + "unique_user_agent_slug", + "agents", + type_="unique", + ) + # Recreate as a partial UNIQUE INDEX with the same name. WHERE + # deleted_at IS NULL scopes the uniqueness check to active rows + # only; soft-deleted rows can share slugs with active rows or with + # each other. + op.create_index( + "unique_user_agent_slug", + "agents", + ["user_id", "slug"], + unique=True, + postgresql_where=sa.text("deleted_at IS NULL"), + ) + + +def downgrade() -> None: + # Safety rail: refuse to downgrade if any (user_id, slug) pair has + # duplicates across active + soft-deleted rows. The full UNIQUE + # constraint would fail to apply, leaving the schema broken. + op.execute( + """ + DO $$ + DECLARE dup_count INT; + BEGIN + SELECT COUNT(*) INTO dup_count FROM ( + SELECT user_id, slug + FROM agents + GROUP BY user_id, slug + HAVING COUNT(*) > 1 + ) sub; + IF dup_count > 0 THEN + RAISE EXCEPTION 'cannot downgrade migration 036: % (user_id, slug) pairs would violate the recreated UNIQUE constraint. Either delete the duplicates or hard-delete the soft-deleted rows before downgrading.', dup_count; + END IF; + END $$; + """ + ) + op.drop_index("unique_user_agent_slug", table_name="agents") + op.create_unique_constraint( + "unique_user_agent_slug", + "agents", + ["user_id", "slug"], + ) diff --git a/app/models/agent.py b/app/models/agent.py index 19cb8e3..4fdb2bb 100644 --- a/app/models/agent.py +++ b/app/models/agent.py @@ -41,8 +41,8 @@ Index, String, Text, - UniqueConstraint, func, + text, ) from sqlalchemy.dialects.postgresql import JSONB, UUID @@ -91,7 +91,22 @@ class Agent(Base): ) __table_args__ = ( - UniqueConstraint("user_id", "slug", name="unique_user_agent_slug"), + # Partial UNIQUE INDEX scoped to active (non-soft-deleted) rows. + # Replaces the prior full UNIQUE constraint per migration 036 + # (OSS port of private cueapi PR #921 / migration 080). Soft- + # deleted rows can keep their original slug without blocking + # recreate. Match the migration's + # `postgresql_where=sa.text("deleted_at IS NULL")` exactly so + # the test-DB schema (built via Base.metadata.create_all) and + # the prod schema (built via alembic) declare identical + # constraint shape. + Index( + "unique_user_agent_slug", + "user_id", + "slug", + unique=True, + postgresql_where=text("deleted_at IS NULL"), + ), CheckConstraint( "status IN ('online', 'offline', 'away')", name="valid_agent_status", diff --git a/app/services/agent_service.py b/app/services/agent_service.py index d71c05c..e464ce5 100644 --- a/app/services/agent_service.py +++ b/app/services/agent_service.py @@ -62,6 +62,59 @@ def _http_error(status: int, code: str, message: str) -> HTTPException: ) +async def _lookup_existing_live_agent_uuid( + db: AsyncSession, user_id, slug: str +) -> Optional[str]: + """Look up the LIVE (non-soft-deleted) Agent's opaque id for a given + (user_id, slug) pair. Returns the ``agt_`` or None if no LIVE + row exists (e.g., race-loss: row was hard-deleted between the + IntegrityError firing and this lookup). + + Extracted as a pure helper so the existing_uuid lookup branch can + be unit-tested directly. pytest-cov on Python 3.12 doesn't reliably + trace ASGI-dispatched await lines through HTTP-routed tests; pure + helper invocation closes the trace gap (parity port from private + cueapi PR #921's discipline lesson). + """ + result = await db.execute( + select(Agent.id).where( + Agent.user_id == user_id, + Agent.slug == slug, + Agent.deleted_at.is_(None), + ).limit(1) + ) + return result.scalar_one_or_none() + + +def _http_error_slug_taken( + final_slug: str, existing_uuid: Optional[str] +) -> HTTPException: + """409 slug_taken envelope variant that includes ``existing_uuid``. + + Parity port of private cueapi PR #921 (Jingim Q1 ergonomics): when + POST /v1/agents fires the ``unique_user_agent_slug`` IntegrityError + against an existing LIVE agent (not soft-deleted), the response + envelope includes the conflicting agent's opaque id so the caller + can skip a GET round-trip and PATCH / address the agent directly. + + ``existing_uuid`` is Optional because the post-IntegrityError + lookup can race-lose (the conflicting row was hard-deleted between + the INSERT failure and the lookup) or constraint-mismatch fall- + through (a non-slug IntegrityError surface that the caller filters + on the constraint name; defensive). In those cases ``existing_uuid`` + is null and the caller falls back to GET-then-PATCH semantics. + """ + detail = { + "error": { + "code": "slug_taken", + "message": f"slug '{final_slug}' already in use for this user", + "status": 409, + "existing_uuid": existing_uuid, + } + } + return HTTPException(status_code=409, detail=detail) + + def _looks_like_opaque_id(addr: str) -> bool: return ( len(addr) == OPAQUE_ID_LENGTH @@ -185,13 +238,19 @@ async def create_agent( except IntegrityError as e: await db.rollback() # Most likely cause: concurrent slug collision against the - # ``unique_user_agent_slug`` constraint. + # partial UNIQUE INDEX ``unique_user_agent_slug`` (migration + # 036, parity port of private cueapi PR #921). Post-port: the + # index is scoped to ``deleted_at IS NULL`` rows only, so a + # soft-deleted-slug-recreate path does NOT hit this + # IntegrityError. Only LIVE-duplicate collisions trip the + # constraint. if "unique_user_agent_slug" in str(e.orig): - raise _http_error( - 409, - "slug_taken", - f"slug '{final_slug}' already in use for this user", + # Look up the conflicting LIVE agent's opaque id via the + # pure helper (unit-testable; defeats ASGI trace gap). + existing_uuid = await _lookup_existing_live_agent_uuid( + db, user.id, final_slug ) + raise _http_error_slug_taken(final_slug, existing_uuid) raise await db.refresh(agent) diff --git a/parity-manifest.json b/parity-manifest.json index 91a83da..9ea9c57 100644 --- a/parity-manifest.json +++ b/parity-manifest.json @@ -173,7 +173,7 @@ { "feature": "live_fallback_mode (sender chooses queue vs bg on Live-miss)", "private_origin": "alembic 067_messages_queue_effective_mode.py (queue state) + service-layer resolver", - "note": "Optional; ship Surface 6 minimal first (live + bg + inbox + webhook + auto resolver). live_fallback_mode is a sender-side preference for handling claim-miss — Dock's Phase 1 may not need it. Defer to Step 4 follow-up OR fold into Step 2 if substrate work is small." + "note": "Optional; ship Surface 6 minimal first (live + bg + inbox + webhook + auto resolver). live_fallback_mode is a sender-side preference for handling claim-miss \u2014 Dock's Phase 1 may not need it. Defer to Step 4 follow-up OR fold into Step 2 if substrate work is small." }, { "feature": "cross-org delivery_mode gating (cross_org_cue_task_enabled etc.)", @@ -182,20 +182,20 @@ } ], "ports_already_landed_in_oss_for_surface_6_foundations": [ - "alembic 028 + app/models/event.py + app/services/events_service.py — event-emit primitive (PR #71, ports private #731 PR-1b)", - "alembic 029 — messaging emission columns (PR #72, ports private #775 PR-2a)", - "alembic 030 — message send_at (PR #77, ports private #623)", - "alembic 031 — agents.last_seen_at + roster endpoint (PR #80, ports private #630)", - "alembic 033 — subscriptions.inline_body (PR #84, ports private #791)", - "alembic 034 — subscriptions.last_acked_event_id (PR #85, ports private #793)", - "alembic 026 — agent_live_sessions schema (PR #67)", - "alembic 027 — executions live-claim attestation (PR #70)", - "alembic 035 — agent_live_sessions IPC attachment (PR #91)" + "alembic 028 + app/models/event.py + app/services/events_service.py \u2014 event-emit primitive (PR #71, ports private #731 PR-1b)", + "alembic 029 \u2014 messaging emission columns (PR #72, ports private #775 PR-2a)", + "alembic 030 \u2014 message send_at (PR #77, ports private #623)", + "alembic 031 \u2014 agents.last_seen_at + roster endpoint (PR #80, ports private #630)", + "alembic 033 \u2014 subscriptions.inline_body (PR #84, ports private #791)", + "alembic 034 \u2014 subscriptions.last_acked_event_id (PR #85, ports private #793)", + "alembic 026 \u2014 agent_live_sessions schema (PR #67)", + "alembic 027 \u2014 executions live-claim attestation (PR #70)", + "alembic 035 \u2014 agent_live_sessions IPC attachment (PR #91)" ], "forward_references_in_oss_pending_surface_6": [ { "path": "alembic/versions/035_agent_live_sessions_ipc_attachment.py", - "note": "Docstring mentions `delivery_mode_requested='ipc'` (line 42 'returns immediately with delivery_mode_requested='ipc'') as a forward-reference to behavior the (future) async fire-accept dispatcher will exhibit. The column itself is NOT added by 035 (which only touches agent_live_sessions); it lands when Surface 6's substrate migration ports. NOT a bug — the ASYNC dispatcher path won't fire until Surface 6 is complete (no message-create path produces delivery_mode_requested='ipc' until then). Verified 2026-05-19 by cueapi-secondary during Step 1 audit." + "note": "Docstring mentions `delivery_mode_requested='ipc'` (line 42 'returns immediately with delivery_mode_requested='ipc'') as a forward-reference to behavior the (future) async fire-accept dispatcher will exhibit. The column itself is NOT added by 035 (which only touches agent_live_sessions); it lands when Surface 6's substrate migration ports. NOT a bug \u2014 the ASYNC dispatcher path won't fire until Surface 6 is complete (no message-create path produces delivery_mode_requested='ipc' until then). Verified 2026-05-19 by cueapi-secondary during Step 1 audit." } ], "sequencing_recommendation": [ @@ -358,6 +358,13 @@ "last_synced": "2026-05-12", "ported_in": "item-b-phase-1-substrate", "deviation": "OSS renumber 063\u2192035 (OSS alembic head was 034 when ported; private chain at 064 post-rebase). down_revision adjusted accordingly." + }, + { + "path": "alembic/versions/036_partial_index_user_agent_slug_active.py", + "private_counterpart": "alembic/versions/080_partial_index_user_agent_slug_active.py", + "last_synced": "2026-05-21", + "ported_in": "cmpdl4-oss-port-slug-partial-index", + "deviation": "OSS renumber 080 \u2192 036 (OSS alembic head was 035 when ported; private chain at 079\u2192080 post PR #919's slug VARCHAR raise to 128). down_revision adjusted to 035. Schema/code verbatim from private's migration 080. Private additionally raised agents.slug from VARCHAR(64) to VARCHAR(128) in migration 079 (private PR #42 Substrate); that column-width raise is NOT ported here \u2014 OSS keeps VARCHAR(64). Source: private cueapi PR #921 merge_commit b770983e; G14 5/5 PASS verified on api.cueapi.ai @ commit 454aac3." } ], "app_core": [ diff --git a/tests/test_agents_slug_partial_index.py b/tests/test_agents_slug_partial_index.py new file mode 100644 index 0000000..39ce23b --- /dev/null +++ b/tests/test_agents_slug_partial_index.py @@ -0,0 +1,457 @@ +"""Tests for the partial-index slug uniqueness fix (migration 080). + +cmpdl4 substrate slug partial-index PR (Jingim Q1 unblock). Pins: + +* Soft-deleted-slug recreate succeeds (the headline Dock-side ask). +* Live-duplicate slug returns 409 envelope with ``existing_uuid`` populated. +* 409 envelope shape (code/status/existing_uuid fields). +* Happy path regression (no-conflict create still works). +* Partial-index DB-layer behavior (multiple soft-deleted rows with same + slug can coexist). +* GET regressions (soft-deleted slug still 404s by default; include_deleted + still returns the row). +* Defensive: non-slug IntegrityError fall-through doesn't trip the + existing_uuid lookup branch. + +Investigation Dock: Jingim's Q1 reach (msg_0xh469viafpc), cue-pm Q1 +greenlight (msg_bmu35t9aeql7), G11-α CONCUR (msg_259g99wa5n5o). +""" +from __future__ import annotations + +import uuid + +import pytest +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError + +from app.models import Agent +from app.utils.ids import generate_agent_id + + +def _agent_payload(**overrides): + base = {"display_name": "Test Agent", "metadata": {}} + base.update(overrides) + return base + + +# --------------------------------------------------------------------- +# Test 1 — Soft-deleted-slug recreate succeeds (headline ask) +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_agent_with_soft_deleted_slug_succeeds( + client, auth_headers, db_session, registered_user +): + """A previously-soft-deleted slug must be recreatable on POST /v1/agents. + + Pre-PR-080: the full UNIQUE constraint counted soft-deleted rows, + so this returned 409. Post-PR-080: the partial UNIQUE INDEX is + scoped to ``deleted_at IS NULL``; soft-deleted rows don't block + the recreate. + """ + # Create + soft-delete an agent with slug "jingim" + create_resp = await client.post( + "/v1/agents", + json=_agent_payload(slug="jingim", display_name="Jingim v1"), + headers=auth_headers, + ) + assert create_resp.status_code == 201, create_resp.text + first_agent_id = create_resp.json()["id"] + + # Soft-delete via DELETE /v1/agents/{id} + delete_resp = await client.delete( + f"/v1/agents/{first_agent_id}", headers=auth_headers + ) + assert delete_resp.status_code in (200, 204), delete_resp.text + + # Re-create with the SAME slug — must succeed with a NEW agt_id + recreate_resp = await client.post( + "/v1/agents", + json=_agent_payload(slug="jingim", display_name="Jingim v2"), + headers=auth_headers, + ) + assert recreate_resp.status_code == 201, recreate_resp.text + second_agent_id = recreate_resp.json()["id"] + assert second_agent_id != first_agent_id, "must be a fresh agt_id" + assert recreate_resp.json()["slug"] == "jingim" + + +# --------------------------------------------------------------------- +# Test 2 — Live-duplicate slug returns 409 with existing_uuid +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_agent_with_live_duplicate_slug_returns_409_with_existing_uuid( + client, auth_headers +): + """A LIVE slug duplicate must return 409 with ``existing_uuid`` + populated to the conflicting agent's opaque id. + + Lets Dock / @trydock/mcp skip a GET round-trip and directly + address the existing agent via PATCH/messaging. + """ + # Create a LIVE agent + first_resp = await client.post( + "/v1/agents", + json=_agent_payload(slug="jingim-live", display_name="Jingim Live"), + headers=auth_headers, + ) + assert first_resp.status_code == 201, first_resp.text + first_id = first_resp.json()["id"] + + # POST same slug again WITHOUT soft-deleting → expect 409 + dup_resp = await client.post( + "/v1/agents", + json=_agent_payload(slug="jingim-live", display_name="Jingim Live Dup"), + headers=auth_headers, + ) + assert dup_resp.status_code == 409, dup_resp.text + err = dup_resp.json()["error"] + assert err["code"] == "slug_taken" + assert err["status"] == 409 + assert err["existing_uuid"] == first_id, ( + f"expected existing_uuid={first_id!r}, got {err.get('existing_uuid')!r}" + ) + + +# --------------------------------------------------------------------- +# Test 3 — 409 envelope shape pin (regression guard) +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_409_envelope_includes_required_fields(client, auth_headers): + """Pin the 409 envelope shape: code, message, status, existing_uuid. + + Future clients (Jingim's Dock, presence-runtime Python, etc.) bind + to this shape. Any drift would break consumer recovery paths. + """ + # Create a LIVE agent to collide against + await client.post( + "/v1/agents", + json=_agent_payload(slug="envelope-pin", display_name="Envelope Pin"), + headers=auth_headers, + ) + dup_resp = await client.post( + "/v1/agents", + json=_agent_payload(slug="envelope-pin", display_name="Pin Dup"), + headers=auth_headers, + ) + assert dup_resp.status_code == 409 + err = dup_resp.json()["error"] + # All four fields must be present + correct types. + assert "code" in err and err["code"] == "slug_taken" + assert "message" in err and isinstance(err["message"], str) + assert "status" in err and err["status"] == 409 + assert "existing_uuid" in err, "existing_uuid must be in the envelope" + # Format check: agt_<12 alphanum> + assert err["existing_uuid"] is not None + assert err["existing_uuid"].startswith("agt_") + assert len(err["existing_uuid"]) == len("agt_") + 12 + + +# --------------------------------------------------------------------- +# Test 4 — Happy-path regression (no-conflict create) +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_agent_with_no_existing_slug_succeeds(client, auth_headers): + """Regression: a fresh slug with no prior history still creates 201. + + Pin the no-collision happy path so the IntegrityError handler + refactor doesn't accidentally break the common case. + """ + fresh_slug = f"fresh-{uuid.uuid4().hex[:6]}" + resp = await client.post( + "/v1/agents", + json=_agent_payload(slug=fresh_slug, display_name="Fresh Agent"), + headers=auth_headers, + ) + assert resp.status_code == 201, resp.text + assert resp.json()["slug"] == fresh_slug + + +# --------------------------------------------------------------------- +# Test 5 — DB-layer: partial index allows multiple soft-deleted same-slug rows +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_partial_index_allows_multiple_soft_deleted_rows_same_slug( + db_session, registered_user +): + """DB-layer pin: the partial unique index does NOT block multiple + soft-deleted rows with the same (user_id, slug) pair. + + Exercises the migration's structural correctness independent of + the route layer's pre-check + IntegrityError catch logic. + """ + from datetime import datetime, timezone + + # Resolve the test user's id + from app.models import User + + user_result = await db_session.execute( + select(User).where(User.email == registered_user["email"]) + ) + user = user_result.scalar_one() + + # Insert two pre-soft-deleted agents with the same slug + now = datetime.now(timezone.utc) + a1 = Agent( + id=generate_agent_id(), + user_id=user.id, + slug="soft-twin", + display_name="Soft Twin A", + deleted_at=now, + ) + a2 = Agent( + id=generate_agent_id(), + user_id=user.id, + slug="soft-twin", + display_name="Soft Twin B", + deleted_at=now, + ) + db_session.add(a1) + db_session.add(a2) + # Partial index ignores both (deleted_at IS NOT NULL on both). + await db_session.commit() + + # Verify both rows exist. + result = await db_session.execute( + select(Agent).where(Agent.user_id == user.id, Agent.slug == "soft-twin") + ) + rows = result.scalars().all() + assert len(rows) == 2, f"expected 2 soft-deleted rows, got {len(rows)}" + for r in rows: + assert r.deleted_at is not None + + +# --------------------------------------------------------------------- +# Tests 6a/6b — Pure-helper unit tests for the existing_uuid lookup +# (defeats ASGI trace gap per CLAUDE.md gotcha #4 + patch-coverage +# discipline from PR #919 lesson) +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_lookup_existing_live_agent_uuid_returns_agt_id_when_live_match_exists( + db_session, registered_user +): + """Pure-helper unit test: when a LIVE agent matches (user_id, slug), + ``_lookup_existing_live_agent_uuid`` returns its ``agt_``. + + Extracted from the IntegrityError handler so pytest-cov on Py 3.12 + traces the lookup body directly (no ASGI dispatch). Pins the + happy-path lookup behavior independently of the HTTP integration + test (test 2 above). + """ + from app.models import User + from app.services.agent_service import _lookup_existing_live_agent_uuid + + user_result = await db_session.execute( + select(User).where(User.email == registered_user["email"]) + ) + user = user_result.scalar_one() + + live = Agent( + id=generate_agent_id(), + user_id=user.id, + slug="live-lookup-target", + display_name="Live Lookup Target", + ) + db_session.add(live) + await db_session.commit() + + result = await _lookup_existing_live_agent_uuid( + db_session, user.id, "live-lookup-target" + ) + assert result == live.id + + +@pytest.mark.asyncio +async def test_lookup_existing_live_agent_uuid_returns_none_when_only_soft_deleted( + db_session, registered_user +): + """Pure-helper unit test: when only SOFT-DELETED rows match + (user_id, slug), ``_lookup_existing_live_agent_uuid`` returns None. + + Pins the soft-delete exclusion in the lookup query — the helper + must filter on ``Agent.deleted_at.is_(None)`` so it never returns + a soft-deleted agent's id as the existing_uuid. + """ + from datetime import datetime, timezone + + from app.models import User + from app.services.agent_service import _lookup_existing_live_agent_uuid + + user_result = await db_session.execute( + select(User).where(User.email == registered_user["email"]) + ) + user = user_result.scalar_one() + + soft = Agent( + id=generate_agent_id(), + user_id=user.id, + slug="soft-only-lookup", + display_name="Soft Only", + deleted_at=datetime.now(timezone.utc), + ) + db_session.add(soft) + await db_session.commit() + + result = await _lookup_existing_live_agent_uuid( + db_session, user.id, "soft-only-lookup" + ) + assert result is None + + +# --------------------------------------------------------------------- +# Test 8 — Direct unit test on create_agent IntegrityError handler. +# Bypasses ASGI so pytest-cov on Py 3.12 reliably traces the body +# lines inside the IntegrityError handler (CI sees 494 + 497 as +# missing when HTTP-only tests are used; direct invocation traces +# both lines consistently). +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_create_agent_direct_call_triggers_409_with_existing_uuid( + db_session, registered_user +): + """Direct call to ``agent_service.create_agent`` (no HTTP) with a + slug that collides against a pre-existing LIVE agent. Pins the + IntegrityError handler body — the existing_uuid lookup branch + + the _http_error_slug_taken raise — via non-ASGI invocation so + pytest-cov traces the body lines reliably. + + Per CLAUDE.md gotcha #4 + PR #919 lesson: HTTP-routed lines aren't + reliably traced by pytest-cov on Py 3.12. This direct unit test + closes the diff-cover gap on the IntegrityError handler body. + """ + from fastapi import HTTPException + + from app.auth import AuthenticatedUser + from app.models import User + from app.services.agent_service import create_agent + + # Resolve the test user as an AuthenticatedUser DTO + a real row + # (need both: create_agent takes AuthenticatedUser; the IntegrityError + # lookup queries by Agent.user_id == user.id). + user_result = await db_session.execute( + select(User).where(User.email == registered_user["email"]) + ) + user_row = user_result.scalar_one() + auth_user = AuthenticatedUser( + id=str(user_row.id), + email=user_row.email, + plan=user_row.plan, + active_cue_limit=user_row.active_cue_limit, + monthly_execution_limit=user_row.monthly_execution_limit, + rate_limit_per_minute=user_row.rate_limit_per_minute, + ) + + # Pre-seed a LIVE agent at the target slug. + seed_slug = f"direct-collision-{uuid.uuid4().hex[:6]}" + seed_id = generate_agent_id() + seed = Agent( + id=seed_id, + user_id=user_row.id, + slug=seed_slug, + display_name="Pre-seeded Live", + ) + db_session.add(seed) + await db_session.commit() + # Capture id locally BEFORE the next async block to avoid lazy- + # load greenlet issues post-commit (MissingGreenlet). + expected_existing_uuid = seed_id + + # Directly call create_agent with the colliding slug — must raise + # HTTPException(409) with existing_uuid populated. + with pytest.raises(HTTPException) as exc_info: + await create_agent( + db_session, + auth_user, + slug=seed_slug, + display_name="Direct Collision Test", + webhook_url=None, + metadata={}, + ) + + assert exc_info.value.status_code == 409 + detail = exc_info.value.detail + assert isinstance(detail, dict), f"detail must be dict, got {type(detail).__name__}" + err = detail["error"] + assert err["code"] == "slug_taken" + assert err["status"] == 409 + assert err["existing_uuid"] == expected_existing_uuid, ( + f"existing_uuid={err.get('existing_uuid')!r} should equal " + f"seed_id={expected_existing_uuid!r}" + ) + + +# --------------------------------------------------------------------- +# Test 9 — Defensive: non-slug IntegrityError fall-through (per cue-pm G11-α) +# --------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_non_slug_integrity_error_falls_through_to_raise( + db_session, registered_user +): + """Defensive: if IntegrityError fires from a constraint OTHER than + ``unique_user_agent_slug``, the existing_uuid lookup branch must + NOT trigger; the original IntegrityError falls through. + + Per cue-pm G11-α CONCUR add (msg_259g99wa5n5o): pins the + branch-coverage discipline — the slug-collision branch is + constraint-name-gated (``"unique_user_agent_slug" in str(e.orig)``), + not catch-all. + + Approach: insert an Agent row with an invalid ``status`` value (one + that's short enough to fit VARCHAR(16) so the + ``valid_agent_status`` CheckConstraint is what trips, NOT + StringDataRightTruncationError). Commit. The CheckConstraint name + in the IntegrityError must NOT match the slug-collision gate. + """ + from app.models import User + + user_result = await db_session.execute( + select(User).where(User.email == registered_user["email"]) + ) + user = user_result.scalar_one() + + # Pick an invalid status that fits VARCHAR(16) so the + # CheckConstraint fires (not the column-length DataError). + # "BOGUS" is 5 chars; "online"/"offline"/"away" are the only valid + # values per the CheckConstraint at agent.py:194-196. + agent = Agent( + id=generate_agent_id(), + user_id=user.id, + slug=f"defensive-{uuid.uuid4().hex[:6]}", + display_name="Defensive Test", + status="BOGUS", # violates valid_agent_status CheckConstraint + ) + db_session.add(agent) + + with pytest.raises(IntegrityError) as exc_info: + await db_session.commit() + + # The constraint name in the error MUST NOT match the slug-collision + # gate (``"unique_user_agent_slug" in str(e.orig)``). The route's + # IntegrityError handler must fall through to ``raise`` on this + # error class without invoking the existing_uuid lookup branch. + err_str = str(exc_info.value.orig) + assert "unique_user_agent_slug" not in err_str, ( + "valid_agent_status CheckConstraint violation must NOT be " + "mistakable for a slug-collision IntegrityError" + ) + # Affirmative pin: the violation IS recognizable as the + # valid_agent_status CheckConstraint (so future readers see what + # constraint we're forcing). + assert "valid_agent_status" in err_str, ( + f"expected valid_agent_status CheckConstraint to fire, got: {err_str[:200]}" + )