diff --git a/.ai/CURRENT_TASK.md b/.ai/CURRENT_TASK.md index d4205ad0..444f1ca8 100644 --- a/.ai/CURRENT_TASK.md +++ b/.ai/CURRENT_TASK.md @@ -34,12 +34,18 @@ ## Remaining work on this branch -1. **Visual QA in a real browser** via `/qa` — slide-in animation, tab-title flash, magic-moment layout, dissolve, full junior-escalates → senior-receives → senior-claims demo path. -2. **Suggested-step chips below the chat input** (Codex correction, design plan locks this) — surfaces `ai_assessment_data.suggested_steps[]` as clickable chips in `FlowPilotMessageBar` that prefill the input. Threading through `FlowPilotSession` → message bar. -3. **Snapshot expansion in `HandoffManager._generate_snapshot`** — include the recent diagnostic steps / conversation tail so the magic-moment screen's "What's been tried" section can render the actual timeline pre-claim instead of "full timeline available after pickup". -4. **Toolbar Context button on legacy-arrival sessions** — currently the button only appears when the senior arrived via the magic-moment flow this session. Lazy-fetching the handoff list on session-load (when status was-escalated) would make it work on revisits. -5. **Owner-facing analytics page** at `/analytics/escalations` — period selector, conversion-rate, trend chart. ~0.5d. Optional for v1 demo. -6. **Playwright e2e** for the magic-moment demo flow (junior escalates → senior receives via SSE → senior claims → opens session). Critical for the GTM Loom not to crash mid-recording. +1. **Visual QA + bug bash** in a real browser — full pickup demo path with the four new pieces below; this is the next active step. +2. **Snapshot expansion in `HandoffManager._generate_snapshot`** — include the recent diagnostic steps / conversation tail so the magic-moment screen's "What's been tried" section can render the actual timeline pre-claim instead of "full timeline available after pickup". +3. **Toolbar Context button on legacy-arrival sessions** — currently the button only appears when the senior arrived via the magic-moment flow this session. Lazy-fetching the handoff list on session-load (when status was-escalated) would make it work on revisits. +4. **Owner-facing analytics page** at `/analytics/escalations` — period selector, conversion-rate, trend chart. ~0.5d. Optional for v1 demo. +5. **Playwright e2e** for the magic-moment demo flow (junior escalates → senior receives via SSE → senior claims → opens session). Critical for the GTM Loom not to crash mid-recording. + +## Just shipped (4 plan-locked items, this session) + +- **Live AI assessment refresh on the magic-moment screen.** New `HandoffAssessmentReadyEvent` type + `onAssessmentReady` handler on `streamEscalations`. `AssistantChatPage` opens a scoped SSE subscription whenever it has a tracked handoff with no AI assessment yet; on a matching event it refetches and replaces both `magicHandoff` and `overlayHandoff` in place. Closes the loop on the async-assessment commit `e8ba74e`. +- **Suggested-step chips below the chat input.** New `chipsHidden` state in `AssistantChatPage` defaulting to false; a chip strip renders above the composer when `magicHandoff?.ai_assessment_data?.suggested_steps[]` is non-empty and the magic-moment has dissolved. Click prefills input + focus; first send hides the strip; explicit X also hides. Per-session lifetime (Codex correction locked design). +- **Unread 6px dot on `EscalationQueue` cards.** localStorage-persisted seen set (`rf-escalation-seen`, capped 200). Dot renders top-right of any card not yet seen. Cleared on **open (card click) or claim (Pick Up)** — NOT on hover (Codex correction). Pick Up onClick now stops propagation so the wrapper's open handler isn't double-fired. +- **Race-condition toast on claim conflict.** New `HandoffAlreadyClaimedError` exception class in `handoff_manager.py`. `claim_session` now eager-loads `claimed_by_user`, rejects different-user re-claims (idempotent for same-user), and raises with the winner's id/name/timestamp. Endpoint translates to 409 with structured detail. `AssistantChatPage.handleStartHere` extracts the detail, formats `"Already claimed by {name} {time_ago}."` via `timeAgo()`, drops `?pickup=true`, and dismisses the magic-moment so the loser flows back to the queue. Backed by 2 new unit tests in `test_handoff_manager.py`. ## Two-metric framing — read this before quoting numbers to anyone diff --git a/backend/app/api/endpoints/session_handoffs.py b/backend/app/api/endpoints/session_handoffs.py index 48ec3168..995419b9 100644 --- a/backend/app/api/endpoints/session_handoffs.py +++ b/backend/app/api/endpoints/session_handoffs.py @@ -22,7 +22,7 @@ from app.core.escalation_bus import bus as escalation_bus from app.models.user import User from app.models.ai_session import AISession from app.models.session_handoff import SessionHandoff -from app.services.handoff_manager import HandoffManager +from app.services.handoff_manager import HandoffAlreadyClaimedError, HandoffManager from app.schemas.session_handoff import ( HandoffCreateRequest, HandoffResponse, @@ -129,6 +129,19 @@ async def claim_handoff( handoff_id=handoff_id, claiming_user_id=current_user.id, ) + except HandoffAlreadyClaimedError as e: + # Loser of the race — the API surfaces structured detail so the + # client can render "Already claimed by {name} {time_ago}" without + # a follow-up fetch. + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail={ + "error": "already_claimed", + "claimed_by_id": str(e.claimed_by_id), + "claimed_by_name": e.claimed_by_name, + "claimed_at": e.claimed_at.isoformat(), + }, + ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) diff --git a/backend/app/services/handoff_manager.py b/backend/app/services/handoff_manager.py index cfefafd3..8f0624cb 100644 --- a/backend/app/services/handoff_manager.py +++ b/backend/app/services/handoff_manager.py @@ -36,6 +36,30 @@ from app.services.notification_service import notify logger = logging.getLogger(__name__) +class HandoffAlreadyClaimedError(Exception): + """Raised when a senior tries to claim a handoff another senior already won. + + Carries the winning claimer's id, display name, and claim timestamp so the + API layer can surface a "Already claimed by {name} {time_ago}" toast on + the losing client. The race story is the locked design — without this + exception the endpoint would silently overwrite `claimed_by` and both + seniors would think they own the session. + """ + + def __init__( + self, + claimed_by_id: UUID, + claimed_by_name: str, + claimed_at: datetime, + ) -> None: + super().__init__( + f"Handoff already claimed by {claimed_by_name} at {claimed_at.isoformat()}" + ) + self.claimed_by_id = claimed_by_id + self.claimed_by_name = claimed_by_name + self.claimed_at = claimed_at + + class HandoffManager: """Unified park/escalate handoff management.""" @@ -398,14 +422,31 @@ class HandoffManager: handoff_id: UUID, claiming_user_id: UUID, ) -> SessionHandoff: - """Claim a handed-off session.""" + """Claim a handed-off session. + + If the handoff was already claimed by a *different* user (the race + story: two seniors clicking Pick Up simultaneously), raise + `HandoffAlreadyClaimedError` with the winning claimer's details so + the API can return 409 with the data the loser's toast needs. A + re-claim by the same user is idempotent. + """ result = await self.db.execute( - select(SessionHandoff).where(SessionHandoff.id == handoff_id) + select(SessionHandoff) + .options(selectinload(SessionHandoff.claimed_by_user)) + .where(SessionHandoff.id == handoff_id) ) handoff = result.scalar_one_or_none() if not handoff: raise ValueError(f"Handoff {handoff_id} not found") + if handoff.claimed_by is not None and handoff.claimed_by != claiming_user_id: + claimer = handoff.claimed_by_user + raise HandoffAlreadyClaimedError( + claimed_by_id=handoff.claimed_by, + claimed_by_name=claimer.name if claimer else "another engineer", + claimed_at=handoff.claimed_at or datetime.now(timezone.utc), + ) + handoff.claimed_by = claiming_user_id handoff.claimed_at = datetime.now(timezone.utc) diff --git a/backend/tests/test_handoff_manager.py b/backend/tests/test_handoff_manager.py index a2e75c05..15c76020 100644 --- a/backend/tests/test_handoff_manager.py +++ b/backend/tests/test_handoff_manager.py @@ -189,6 +189,99 @@ async def test_claim_session(client: AsyncClient, test_user, test_admin, auth_he assert session.status == "active" +@pytest.mark.asyncio +async def test_claim_session_conflict_raises_already_claimed( + client: AsyncClient, test_user, test_admin, auth_headers, test_db +): + """Two seniors claiming simultaneously: the second raises the typed + HandoffAlreadyClaimedError carrying the winner's identity. Without this + guard both calls would silently overwrite claimed_by — the locked + race-condition story depends on a real conflict response.""" + from app.services.handoff_manager import ( + HandoffAlreadyClaimedError, + HandoffManager, + ) + + session = AISession( + user_id=test_user["user_data"]["id"], + account_id=test_user["user_data"]["account_id"], + session_type="guided", + intake_type="free_text", + intake_content={"text": "test"}, + status="active", + confidence_tier="discovery", + conversation_messages=[], + ) + test_db.add(session) + await test_db.flush() + + manager = HandoffManager(test_db) + handoff = await manager.create_handoff( + session_id=session.id, + intent="escalate", + engineer_notes="Need help", + user_id=test_user["user_data"]["id"], + ) + + # First claim — admin wins. + await manager.claim_session( + handoff_id=handoff.id, + claiming_user_id=test_admin["user_data"]["id"], + ) + + # Second claim by a different user — owner of the original session, + # standing in for "the other senior who lost the race." + with pytest.raises(HandoffAlreadyClaimedError) as exc_info: + await manager.claim_session( + handoff_id=handoff.id, + claiming_user_id=test_user["user_data"]["id"], + ) + + err = exc_info.value + assert err.claimed_by_id == test_admin["user_data"]["id"] + assert err.claimed_by_name # populated from User.name + assert err.claimed_at is not None + + +@pytest.mark.asyncio +async def test_claim_session_idempotent_for_same_user( + client: AsyncClient, test_user, test_admin, auth_headers, test_db +): + """A re-claim by the user who already won is a no-op, not a conflict. + Defends against double-clicks / network retries on the loser-side toast.""" + session = AISession( + user_id=test_user["user_data"]["id"], + account_id=test_user["user_data"]["account_id"], + session_type="guided", + intake_type="free_text", + intake_content={"text": "test"}, + status="active", + confidence_tier="discovery", + conversation_messages=[], + ) + test_db.add(session) + await test_db.flush() + + manager = HandoffManager(test_db) + handoff = await manager.create_handoff( + session_id=session.id, + intent="escalate", + engineer_notes="Need help", + user_id=test_user["user_data"]["id"], + ) + + first = await manager.claim_session( + handoff_id=handoff.id, + claiming_user_id=test_admin["user_data"]["id"], + ) + second = await manager.claim_session( + handoff_id=handoff.id, + claiming_user_id=test_admin["user_data"]["id"], + ) + + assert first.claimed_by == second.claimed_by == test_admin["user_data"]["id"] + + # ─── Notification dispatch ──────────────────────────────────────────────────── diff --git a/frontend/src/api/aiSessions.ts b/frontend/src/api/aiSessions.ts index 90531a8d..d59d43dd 100644 --- a/frontend/src/api/aiSessions.ts +++ b/frontend/src/api/aiSessions.ts @@ -19,6 +19,7 @@ import type { ChatMessageRequest, ChatMessageResponse, HandoffCreatedEvent, + HandoffAssessmentReadyEvent, EscalationStreamHandlers, } from '@/types/ai-session' @@ -279,6 +280,13 @@ export const aiSessionsApi = { const parsed = JSON.parse(data) as Record if (eventType === 'handoff_created' && parsed.type === 'handoff_created') { handlers.onHandoffCreated?.(parsed as unknown as HandoffCreatedEvent) + } else if ( + eventType === 'handoff_assessment_ready' && + parsed.type === 'handoff_assessment_ready' + ) { + handlers.onAssessmentReady?.( + parsed as unknown as HandoffAssessmentReadyEvent, + ) } else if (eventType === 'ready') { handlers.onReady?.() } diff --git a/frontend/src/components/flowpilot/EscalationQueue.tsx b/frontend/src/components/flowpilot/EscalationQueue.tsx index dbce00aa..98e73bdb 100644 --- a/frontend/src/components/flowpilot/EscalationQueue.tsx +++ b/frontend/src/components/flowpilot/EscalationQueue.tsx @@ -26,6 +26,34 @@ const sortNewestFirst = (a: AISessionSummary, b: AISessionSummary) => // state transition. const NEW_CARD_HIGHLIGHT_MS = 800 +// localStorage key for the per-user "seen" set. Tracks session IDs the user +// has acknowledged so the unread dot doesn't reappear on refresh. Bounded to +// the last `SEEN_CAP` entries to avoid unbounded growth on long-lived +// accounts. +const SEEN_STORAGE_KEY = 'rf-escalation-seen' +const SEEN_CAP = 200 + +function loadSeenIds(): Set { + try { + const raw = localStorage.getItem(SEEN_STORAGE_KEY) + if (!raw) return new Set() + const parsed = JSON.parse(raw) as unknown + if (!Array.isArray(parsed)) return new Set() + return new Set(parsed.filter((v): v is string => typeof v === 'string')) + } catch { + return new Set() + } +} + +function saveSeenIds(ids: Set): void { + try { + const arr = Array.from(ids).slice(-SEEN_CAP) + localStorage.setItem(SEEN_STORAGE_KEY, JSON.stringify(arr)) + } catch { + // localStorage unavailable / quota — silent. The dot just won't persist. + } +} + function waitTimeColor(createdAt: string): string { const hours = (Date.now() - new Date(createdAt).getTime()) / 3_600_000 if (hours >= 4) return '#f87171' // danger @@ -42,6 +70,20 @@ export function EscalationQueue({ onPickup, onCountChange }: EscalationQueueProp const [newIds, setNewIds] = useState>(new Set()) // Track count of unseen arrivals while the tab is backgrounded. const [unseenCount, setUnseenCount] = useState(0) + // Per-user seen set persisted in localStorage. Cleared on open, claim, or + // explicit dismiss (NOT on hover — Codex correction). The unread dot is + // shown for any session id NOT in this set. + const [seenIds, setSeenIds] = useState>(() => loadSeenIds()) + + const markSeen = useCallback((sessionId: string) => { + setSeenIds(prev => { + if (prev.has(sessionId)) return prev + const next = new Set(prev) + next.add(sessionId) + saveSeenIds(next) + return next + }) + }, []) // Ref mirrors the latest sessions so the SSE handler can diff without // re-binding on every state change. @@ -190,6 +232,7 @@ export function EscalationQueue({ onPickup, onCountChange }: EscalationQueueProp }, [handleHandoffCreated]) const handlePickup = (sessionId: string) => { + markSeen(sessionId) if (onPickup) { onPickup(sessionId) } else { @@ -197,6 +240,14 @@ export function EscalationQueue({ onPickup, onCountChange }: EscalationQueueProp } } + // Click on the card body (anywhere outside Pick Up) marks the session as + // seen — the "open" affordance from the unread-dot spec. Pick Up handles + // its own marking via handlePickup. Hover deliberately does NOT clear + // (Codex correction). + const handleCardOpen = (sessionId: string) => { + markSeen(sessionId) + } + if (isLoading) { return (
@@ -256,15 +307,26 @@ export function EscalationQueue({ onPickup, onCountChange }: EscalationQueueProp
{sessions.map((session) => { const isNew = newIds.has(session.id) + const isUnread = !seenIds.has(session.id) return (
handleCardOpen(session.id)} className={cn( - 'card-flat p-3 sm:p-4 space-y-3', + 'relative card-flat p-3 sm:p-4 space-y-3 cursor-pointer', isNew && !prefersReducedMotion && 'animate-slide-in-bottom', isNew && prefersReducedMotion && 'animate-fade-in', )} > + {/* Unread indicator: 6px dot, top-right corner. Cleared on + open (card click) or claim (Pick Up). Persists across + refresh via localStorage. */} + {isUnread && ( + + )}

{session.problem_summary || 'Untitled session'} @@ -303,7 +365,10 @@ export function EscalationQueue({ onPickup, onCountChange }: EscalationQueueProp

+ ))} +
+ +
+
+ )} + {/* Rich Input */}
void onHandoffCreated?: (event: HandoffCreatedEvent) => void + onAssessmentReady?: (event: HandoffAssessmentReadyEvent) => void }