diff --git a/.ai/CURRENT_TASK.md b/.ai/CURRENT_TASK.md index f80d2b89..09085589 100644 --- a/.ai/CURRENT_TASK.md +++ b/.ai/CURRENT_TASK.md @@ -2,61 +2,41 @@ **Task:** Build **Escalation Mode** — the wedge for ResolutionFlow's GTM (first paying-customer push). When a junior tech escalates a FlowPilot session, the senior tech sees structured handoff context in seconds instead of running a 5-minute verbal "tell me what you tried" call. -**Status:** in-flight on `feat/escalation-metric-endpoint`. Branch pushed; **draft PR #155** open ([gitea.resolutionflow.com/chihlasm/resolutionflow/pulls/155](https://gitea.resolutionflow.com/chihlasm/resolutionflow/pulls/155)). Live QA found one architectural issue blocking the demo — see "Active blocker" below. +**Status:** ✅ **Engineering complete.** Browser QA passed (2026-04-30). Branch `feat/escalation-metric-endpoint`; PR #155 ready to mark ready-for-review. **Plan:** [`docs/plans/2026-04-27-escalation-mode-wedge-design.md`](../docs/plans/2026-04-27-escalation-mode-wedge-design.md). Reviewed by `/office-hours`, `/plan-eng-review`, `/plan-design-review`, `/codex review`. Eng + Design CLEARED. **Test plan artifact:** [`docs/plans/2026-04-27-escalation-mode-wedge-test-plan.md`](../docs/plans/2026-04-27-escalation-mode-wedge-test-plan.md). -## Active blocker — AI assessment still empty after pickup +## What's done (all sessions combined) -**The bug** (live-test confirmed 2026-04-29): senior picks up an escalation, magic-moment screen renders with the "AI assessment is still generating" placeholder, and **the placeholder never clears**. Bus event fires with `has_assessment: false` because `_generate_ai_assessment` is hitting Sonnet tail latency or some other generation issue we haven't traced yet. Bumping `ESCALATION_AI_ASSESSMENT_TIMEOUT_SECONDS` from 15 → 45 (commit `0d1b305`) didn't fix it in the field. +All plan items complete. Key commits on `feat/escalation-metric-endpoint`: -**Why patching is the wrong move:** the real architectural issue is that we make **three** AI calls per escalation, all summarizing the same source material: +| Commit | What it ships | +|---|---| +| `d51e95c` | Plan + test-plan artifacts | +| `52f6d03` | `GET /analytics/flowpilot/escalations` — time-to-first-action metric | +| `7a5b853` | Role-gate claim to engineer-or-admin | +| `07d0db9` | Email notifications on escalation | +| `9f0bfd4` | `EscalationMetricCard` on `/escalations` | +| `b8627f4` | SSE live-arrival animations in `EscalationQueue` | +| `8e9d22e` | Magic-moment handoff-context screen | +| `641853a` | Bell-icon opens pickup flow | +| `029680a` | Unify `/escalate` through `HandoffManager` | +| `0f00ee5` | Plan-locked polish: chips, unread dot, race toast, AI refresh | +| `665530f` | Structural task-lane race fix | +| `db717b0` | 3-option CTA, copy button fix, post-escalation redirect, claim 500 fix | +| `dc69c9d` | Allow `escalated_to_id` to send chat (GET AI analysis fix) | -1. `_build_escalation_package_enhanced` (Sonnet) — rich JSON payload, runs in the background. -2. `_generate_ai_assessment` (Sonnet, 500 tokens) — magic-moment fields (`likely_cause`, `suggested_steps[]`, `confidence`), background. -3. `generate_status_update` (Sonnet) — the PSA prose the engineer clicks "Ticket Notes" / "Client Update" / "Email Draft" to produce in `ConcludeSessionModal`, on demand. +**Browser QA results (2026-04-30):** -User's correct observation (2026-04-29): the engineer is *typically* generating a status update during the escalate flow anyway. There's no reason to do that work three times. - -**Next active task: consolidate the three calls into one.** See `## Active task — AI generation consolidation` below. - -## Active task — AI generation consolidation - -**Goal:** ONE AI call per escalation that produces a single structured payload covering both the magic-moment screen's diagnostic fields AND the PSA-ready prose. Magic-moment populates immediately. The conclude modal's audience buttons become tone-shift transformations of the saved payload, not fresh API calls. - -**Proposed shape** (decide during implementation): - -```python -# Persist on SessionHandoff: -{ - "summary_prose": "", - "what_we_know": ["", ...], - "likely_cause": "", - "suggested_steps": ["", ""], - "confidence": "low" | "medium" | "high", - "audience_variants": { - # Filled lazily on first request; transformations not regenerations. - "client_update": null, - "email_draft": null, - } -} -``` - -**Implementation order (suggested):** - -1. **Backend:** Replace `_generate_ai_assessment` with `_generate_handoff_summary` (or rename — pick the right noun). One Sonnet call, structured JSON response, persisted to `handoff.ai_assessment_data` + a new `handoff.summary_prose` column (migration needed) OR repurpose the existing `ai_assessment` text column to hold the prose. -2. **Backend:** Make `generate_status_update` for `audience='ticket_notes'` / `context='escalation'` read from the saved payload first; only call the model if the payload is missing (fallback for legacy sessions). For `client_update` / `email_draft`, run a cheaper transformation pass (Haiku is fine for tone-shift) over the saved prose. -3. **Backend:** Drop `_build_escalation_package_enhanced` from the background path — its content overlaps heavily with the new summary, and the magic-moment screen already gets what it needs from the structured fields. Keep it only if downstream PSA push depends on it (verify by grep). Migration concern: the `ai_session.escalation_package` JSON column has live data — leave it readable, just stop *writing* the enhanced payload from `enrich_escalation_async`. -4. **Frontend:** `HandoffContextScreen` reads from the new structured fields. The `ConcludeSessionModal`'s "Ticket Notes" button stops generating fresh — it just copies the saved prose to clipboard / posts to PSA. "Client Update" and "Email Draft" buttons trigger the transformation endpoint. -5. **Test plan:** Magic-moment screen populates within ~5s instead of ~25s. Engineer's "Ticket Notes" button is instant. Token spend per escalation drops by ~60%. - -**Watch-outs:** - -- The schema for the structured response needs to be enforced — past calls returned freeform prose that the frontend can't parse into chips. Use Anthropic's tool-use / structured output if needed. -- Don't break the existing `escalation_package` JSON readers (PSA push, queue summaries). Stop *writing* the enhanced one but keep the dual-write of the basic snapshot. -- `_generate_ai_assessment` is referenced in tests (`test_handoff_manager.py` stubs it via `AsyncMock`). Update test fixtures when renaming. +- ✅ Post-escalation redirect (dashboard + toast) +- ✅ Magic-moment screen: header, AI assessment, 2-option CTA +- ✅ "I'll take it from here": claim → dismiss → composer focused +- ✅ "Get AI analysis": claim → briefing → AI responds → task lane populates +- ✅ Task lane copy button: toast + checkmark +- ✅ Chip expansion: inline detail + "Open in Tasks panel" +- ✅ Post-claim overlay: dismissible mode, Close only ## Done on `feat/escalation-metric-endpoint` (branched from `main` @ `c0ed6d9`) diff --git a/.ai/HANDOFF.md b/.ai/HANDOFF.md index 4551d3b6..ff70fc58 100644 --- a/.ai/HANDOFF.md +++ b/.ai/HANDOFF.md @@ -2,34 +2,33 @@ # HANDOFF.md -**Last updated:** 2026-04-30 (session 3 — QA pass) +**Last updated:** 2026-04-30 (Codex review-fix pass) -**Active task:** **Escalation Mode** wedge — BROWSER QA COMPLETE. Branch: `feat/escalation-metric-endpoint`. PR #155 ready to mark ready-for-review. +**Active task:** **Escalation Mode** wedge — BROWSER QA COMPLETE + review fixes applied. Branch: `feat/escalation-metric-endpoint`. PR #155 ready to mark ready-for-review after committing this fix pass. -## Where the previous session ended +## Where this session ended -Browser QA pass completed. One critical bug found and fixed during QA. +Code-review fixes were applied after browser QA: -**Bug found + fixed (commit dc69c9d):** -- `POST /ai-sessions/{id}/chat → 400` when senior clicks "Get AI analysis" — `send_chat_message` checked `session.user_id == user_id` but the senior is `escalated_to_id`, not `user_id`. Fixed by adding `OR escalated_to_id == user_id` in the WHERE clause. +- `claim_session` now uses atomic conditional `UPDATE ... WHERE claimed_by IS NULL` instead of read-then-write, so simultaneous senior pickup cannot silently overwrite `claimed_by`. +- Original escalators cannot claim their own handoff. The escalation queue also excludes the current user's own escalated sessions, preventing the post-escalation dashboard from showing the junior their own handoff. +- `session.escalation_package["handoff_id"]` is now populated from a preassigned UUID instead of `None` before flush. +- Frontend build blockers removed: deleted unused legacy `claiming` / `handleStartHere` path in `AssistantChatPage` and unused `onStartHere` destructuring in `HandoffContextScreen`. -**All QA checks passed (17/17 backend tests pass):** +**Validation:** -- Post-escalation redirect: junior gets "Session escalated. Heading back to your dashboard." toast + navigates to `/` -- Magic-moment screen: header, metadata, two-column AI assessment, 2-option CTA (no task lane) all render correctly -- "I'll take it from here": claim → dismiss → chat surface → composer focused ✅ -- "Get AI analysis": claim → briefing sent → AI responds → task lane populates ✅ (fixed) -- Task lane copy button: toast + checkmark visual feedback ✅ -- Chip expansion: inline detail card + "Open in Tasks panel" scroll ✅ -- Post-claim overlay: "Context" toolbar button → dismissible mode → only Close button ✅ +- `git diff --check` ✅ +- `cd backend && pytest --override-ini='addopts=' tests/test_handoff_manager.py tests/test_session_handoffs_api.py tests/test_escalation_bus.py` ✅ `28 passed in 42.23s` +- `cd frontend && /config/.bun/bin/bunx tsc -p tsconfig.app.json --noEmit --pretty false && /config/.bun/bin/bunx tsc -p tsconfig.node.json --noEmit --pretty false` ✅ +- Full frontend build could not complete because generated dirs are root-owned in this workspace: `frontend/node_modules/.tmp`, `frontend/node_modules/.vite-temp`, and likely `frontend/dist` produce EACCES. Type errors from review are fixed. **Not testable in dev (known limitations):** - "Continue where X left off": requires senior to have existing task lane for session (won't occur on first pickup) -- 409 race condition: requires two distinct senior accounts; backend logic reviewed and correct +- Browser-level 409 race toast still requires two distinct senior accounts. Backend claim write is now atomic and covered by service/API tests for conflict, self-claim, and idempotent same-user retry. ## Resume point — DO THIS NEXT -**Ship:** Mark PR #155 ready-for-review and demo to stakeholder. No engineering work remaining. +**Ship:** Commit this review-fix pass, then mark PR #155 ready-for-review and demo to stakeholder. Optional before shipping: - Record Loom demo walking through the escalation flow end-to-end @@ -37,6 +36,8 @@ Optional before shipping: ## Key files changed this session - `backend/app/services/handoff_manager.py` — `_generate_handoff_summary` replaces old assessment pair; `enrich_escalation_async` unified; `claim_session` eager-loads `handed_off_by_user` +- `backend/app/api/endpoints/ai_sessions.py` — escalation queue excludes the current user's own escalations +- `backend/app/api/endpoints/session_handoffs.py` — self-claim returns 403 - `backend/app/services/flowpilot_engine.py` — `generate_status_update` early-returns saved prose for `context='escalation'` - `backend/app/schemas/session_handoff.py` — `handed_off_by_name: str | None = None` added - `backend/app/api/endpoints/session_handoffs.py` — both create + claim endpoints pass `handed_off_by_name` @@ -44,11 +45,12 @@ Optional before shipping: - `frontend/src/components/flowpilot/HandoffContextScreen.tsx` — 3-option CTA; `hasTaskLane`, `activeOptionKey`, `onContinue/onAIAnalysis/onOwnThing` props - `frontend/src/components/assistant/TaskLane.tsx` — `id="task-lane-card-{idx}"` on all card variants - `frontend/src/pages/AssistantChatPage.tsx` — `handleContinue`, `handleAIAnalysis`, `handleOwnThing` handlers; chip → card navigation; `activeOptionKey` state +- `backend/tests/test_handoff_manager.py`, `backend/tests/test_session_handoffs_api.py` — regression coverage for atomic/idempotent claim, self-claim rejection, queue self-exclusion, and pre-flush handoff ID ## Watch-outs - Dev stack: backend `:8000`, frontend `:5173`, postgres `:5433` (docker-compose). HMR works. - Test users (Acme MSP, password `TestPass123!`): `engineer@resolutionflow.example.com` (junior), `teamadmin@resolutionflow.example.com` (senior). - `handleAIAnalysis` pre-adds `urlSessionId` to `loadedChatIdsRef` before dismissing so the normal selectChat effect doesn't double-fire. It then calls `selectChat` manually before sending the briefing. -- `claiming` state is now only used by the legacy `handleStartHere` (which is no longer wired to any UI). `activeOptionKey !== null` is the new `isProcessing` signal. +- Legacy `claiming` / `handleStartHere` on `AssistantChatPage` was removed; `activeOptionKey !== null` is the active pre-claim processing signal. - The bus is acceptable for v1 pilot scale only (Railway single-replica). Redis pub/sub is the swap when horizontal scaling appears. diff --git a/.ai/SESSION_LOG.md b/.ai/SESSION_LOG.md index 8945a0cd..29369f6a 100644 --- a/.ai/SESSION_LOG.md +++ b/.ai/SESSION_LOG.md @@ -12,6 +12,17 @@ --- +## 2026-04-30 06:25 UTC — Codex — Apply Escalation Mode review fixes + +- Reviewed the recent Escalation Mode wedge work and fixed the actionable findings before PR #155 is marked ready. +- Reworked `HandoffManager.claim_session` from read-then-write to an atomic conditional update, preserving idempotent same-user retries and returning a typed conflict for a different claimant. +- Blocked original engineers from claiming their own handoffs and filtered their own escalated sessions out of `/ai-sessions/escalation-queue`, preventing the post-escalation dashboard from showing a junior their own handoff. +- Fixed the compatibility payload so `session.escalation_package["handoff_id"]` is populated from a preassigned UUID before flush. +- Removed unused legacy frontend pickup state (`claiming`, `handleStartHere`, unused `onStartHere` destructuring) that made `tsc -b` fail under `noUnusedLocals`. +- Added regression coverage for pre-flush handoff IDs, conflict handling, self-claim rejection, successful non-owner claim, and own-escalation queue exclusion. +- Verified `git diff --check`; focused backend tests passed (`28 passed in 42.23s`); frontend `tsc --noEmit` checks passed for app and node configs. Full Vite/build script remains blocked by root-owned generated directories under `frontend/node_modules` / `frontend/dist` in this workspace, not by TypeScript errors. +- Files touched: `backend/app/services/handoff_manager.py`, `backend/app/api/endpoints/ai_sessions.py`, `backend/app/api/endpoints/session_handoffs.py`, `backend/tests/test_handoff_manager.py`, `backend/tests/test_session_handoffs_api.py`, `frontend/src/components/flowpilot/HandoffContextScreen.tsx`, `frontend/src/pages/AssistantChatPage.tsx`, `.ai/HANDOFF.md`, `.ai/SESSION_LOG.md`. + ## 2026-04-30 — Claude Code — Browser QA pass complete; chat ownership bug found and fixed; PR #155 ready - Ran full browser QA pass on the escalation mode feature using gstack `/qa` skill. diff --git a/backend/app/api/endpoints/ai_sessions.py b/backend/app/api/endpoints/ai_sessions.py index 4fe4ab28..88504264 100644 --- a/backend/app/api/endpoints/ai_sessions.py +++ b/backend/app/api/endpoints/ai_sessions.py @@ -689,6 +689,7 @@ async def get_escalation_queue( .where( scope_filter, AISession.status.in_(("requesting_escalation", "escalated")), + AISession.user_id != current_user.id, ) .order_by(AISession.created_at.desc()) ) diff --git a/backend/app/api/endpoints/session_handoffs.py b/backend/app/api/endpoints/session_handoffs.py index d13cb67b..247b1f22 100644 --- a/backend/app/api/endpoints/session_handoffs.py +++ b/backend/app/api/endpoints/session_handoffs.py @@ -144,6 +144,8 @@ async def claim_handoff( "claimed_at": e.claimed_at.isoformat(), }, ) + except PermissionError as e: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(e)) 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 dba0fd49..b414e07c 100644 --- a/backend/app/services/handoff_manager.py +++ b/backend/app/services/handoff_manager.py @@ -18,9 +18,9 @@ import json import logging from datetime import datetime, timezone from typing import Any -from uuid import UUID +from uuid import UUID, uuid4 -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -88,6 +88,10 @@ class HandoffManager: to produce), and merges the handoff metadata into it. Self-targeting is rejected with ValueError, matching legacy behavior. """ + user_id = UUID(str(user_id)) + if target_user_id: + target_user_id = UUID(str(target_user_id)) + # Eager-load steps + user — _build_escalation_package_enhanced and # finalize_escalation iterate over session.steps to compose the # legacy enriched package and the SessionDocumentation, and the @@ -125,7 +129,9 @@ class HandoffManager: # immediately with `ai_assessment=None`; the magic-moment screen # shows "Assessment still computing" until enrich_async finishes # and the senior refreshes (or, eventually, polls). + handoff_id = uuid4() handoff = SessionHandoff( + id=handoff_id, session_id=session_id, account_id=session.account_id, handed_off_by=user_id, @@ -159,7 +165,7 @@ class HandoffManager: "snapshot": snapshot, "intent": intent, "engineer_notes": engineer_notes, - "handoff_id": str(handoff.id), + "handoff_id": str(handoff_id), } await self.db.flush() @@ -432,6 +438,21 @@ class HandoffManager: the API can return 409 with the data the loser's toast needs. A re-claim by the same user is idempotent. """ + claiming_user_id = UUID(str(claiming_user_id)) + claimed_at = datetime.now(timezone.utc) + + update_result = await self.db.execute( + update(SessionHandoff) + .where( + SessionHandoff.id == handoff_id, + SessionHandoff.claimed_by.is_(None), + SessionHandoff.handed_off_by != claiming_user_id, + ) + .values(claimed_by=claiming_user_id, claimed_at=claimed_at) + .returning(SessionHandoff.id) + ) + claimed_now = update_result.scalar_one_or_none() is not None + result = await self.db.execute( select(SessionHandoff) .options( @@ -444,17 +465,22 @@ class HandoffManager: 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: + handed_off_by = UUID(str(handoff.handed_off_by)) + claimed_by = ( + UUID(str(handoff.claimed_by)) if handoff.claimed_by is not None else None + ) + + if handed_off_by == claiming_user_id: + raise PermissionError("Cannot claim your own handoff") + + if not claimed_now and claimed_by != claiming_user_id: claimer = handoff.claimed_by_user raise HandoffAlreadyClaimedError( - claimed_by_id=handoff.claimed_by, + claimed_by_id=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) - # Reactivate session session_result = await self.db.execute( select(AISession).where(AISession.id == handoff.session_id) diff --git a/backend/tests/test_handoff_manager.py b/backend/tests/test_handoff_manager.py index ff0f76a2..cdb5f066 100644 --- a/backend/tests/test_handoff_manager.py +++ b/backend/tests/test_handoff_manager.py @@ -99,6 +99,7 @@ async def test_create_escalate_handoff(client: AsyncClient, test_user, auth_head assert session.status == "escalated" assert session.escalation_package is not None assert "branch_map" in session.escalation_package or "snapshot" in session.escalation_package + assert session.escalation_package["handoff_id"] == str(handoff.id) @pytest.mark.asyncio @@ -181,7 +182,7 @@ async def test_claim_session(client: AsyncClient, test_user, test_admin, auth_he claiming_user_id=test_admin["user_data"]["id"], ) - assert claimed.claimed_by == test_admin["user_data"]["id"] + assert str(claimed.claimed_by) == test_admin["user_data"]["id"] assert claimed.claimed_at is not None await test_db.refresh(session) @@ -212,6 +213,15 @@ async def test_claim_session_conflict_raises_already_claimed( conversation_messages=[], ) test_db.add(session) + loser = User( + email="race-loser@example.com", + password_hash="x", + name="Race Loser", + role="engineer", + account_id=test_user["user_data"]["account_id"], + account_role="engineer", + ) + test_db.add(loser) await test_db.flush() manager = HandoffManager(test_db) @@ -228,16 +238,16 @@ async def test_claim_session_conflict_raises_already_claimed( 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." + # Second claim by a different user — 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"], + claiming_user_id=loser.id, ) err = exc_info.value - assert err.claimed_by_id == test_admin["user_data"]["id"] + assert str(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 @@ -278,7 +288,40 @@ async def test_claim_session_idempotent_for_same_user( claiming_user_id=test_admin["user_data"]["id"], ) - assert first.claimed_by == second.claimed_by == test_admin["user_data"]["id"] + assert str(first.claimed_by) == str(second.claimed_by) == test_admin["user_data"]["id"] + + +@pytest.mark.asyncio +async def test_claim_session_rejects_self_claim( + client: AsyncClient, test_user, auth_headers, test_db +): + """The engineer who escalated a session cannot pick up their own handoff.""" + 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"], + ) + + with pytest.raises(PermissionError): + await manager.claim_session( + handoff_id=handoff.id, + claiming_user_id=test_user["user_data"]["id"], + ) # ─── Notification dispatch ──────────────────────────────────────────────────── diff --git a/backend/tests/test_session_handoffs_api.py b/backend/tests/test_session_handoffs_api.py index 010137fb..314b5e98 100644 --- a/backend/tests/test_session_handoffs_api.py +++ b/backend/tests/test_session_handoffs_api.py @@ -9,6 +9,7 @@ from sqlalchemy import select from app.api.endpoints.session_handoffs import stream_escalations from app.core.escalation_bus import bus as escalation_bus from app.models.ai_session import AISession +from app.models.session_handoff import SessionHandoff from app.models.user import User from app.services.handoff_manager import HandoffManager @@ -196,8 +197,19 @@ async def test_claim_allowed_for_engineer_role( client: AsyncClient, test_user, auth_headers, test_db ): """POST /handoffs/{id}/claim succeeds for engineer-or-admin roles.""" + original_engineer = User( + email="original-engineer@example.com", + password_hash="x", + name="Original Engineer", + role="engineer", + account_id=test_user["user_data"]["account_id"], + account_role="engineer", + ) + test_db.add(original_engineer) + await test_db.flush() + session = AISession( - user_id=test_user["user_data"]["id"], + user_id=original_engineer.id, account_id=test_user["user_data"]["account_id"], session_type="guided", intake_type="free_text", @@ -207,21 +219,106 @@ async def test_claim_allowed_for_engineer_role( conversation_messages=[], ) test_db.add(session) - await test_db.commit() + await test_db.flush() - create_resp = await client.post( - f"/api/v1/ai-sessions/{session.id}/handoff", - headers=auth_headers, - json={"intent": "escalate", "engineer_notes": "Need help"}, + handoff = SessionHandoff( + session_id=session.id, + account_id=test_user["user_data"]["account_id"], + handed_off_by=original_engineer.id, + intent="escalate", + snapshot={"problem_summary": "test"}, + engineer_notes="Need help", ) - assert create_resp.status_code == 201 - handoff_id = create_resp.json()["id"] + test_db.add(handoff) + await test_db.commit() # Default test_user role is "owner", which passes engineer-or-admin. claim_resp = await client.post( - f"/api/v1/ai-sessions/{session.id}/handoffs/{handoff_id}/claim", + f"/api/v1/ai-sessions/{session.id}/handoffs/{handoff.id}/claim", headers=auth_headers, ) assert claim_resp.status_code == 200 assert claim_resp.json()["claimed_by"] == test_user["user_data"]["id"] assert claim_resp.json()["claimed_at"] is not None + + +@pytest.mark.asyncio +async def test_claim_rejects_self_claim( + client: AsyncClient, test_user, auth_headers, test_db +): + """POST /handoffs/{id}/claim returns 403 for the original escalator.""" + 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="escalated", + confidence_tier="discovery", + conversation_messages=[], + ) + test_db.add(session) + await test_db.flush() + + handoff = SessionHandoff( + session_id=session.id, + account_id=test_user["user_data"]["account_id"], + handed_off_by=test_user["user_data"]["id"], + intent="escalate", + snapshot={"problem_summary": "test"}, + engineer_notes="Need help", + ) + test_db.add(handoff) + await test_db.commit() + + claim_resp = await client.post( + f"/api/v1/ai-sessions/{session.id}/handoffs/{handoff.id}/claim", + headers=auth_headers, + ) + assert claim_resp.status_code == 403 + assert "own handoff" in claim_resp.json()["detail"] + + +@pytest.mark.asyncio +async def test_escalation_queue_excludes_own_escalations( + client: AsyncClient, test_user, auth_headers, test_db +): + """The post-escalation dashboard queue should not show your own handoff.""" + own_session = AISession( + user_id=test_user["user_data"]["id"], + account_id=test_user["user_data"]["account_id"], + session_type="chat", + intake_type="free_text", + intake_content={"text": "own"}, + status="escalated", + confidence_tier="discovery", + conversation_messages=[], + ) + other_engineer = User( + email="other-engineer@example.com", + password_hash="x", + name="Other Engineer", + role="engineer", + account_id=test_user["user_data"]["account_id"], + account_role="engineer", + ) + test_db.add_all([own_session, other_engineer]) + await test_db.flush() + other_session = AISession( + user_id=other_engineer.id, + account_id=test_user["user_data"]["account_id"], + session_type="chat", + intake_type="free_text", + intake_content={"text": "other"}, + status="escalated", + confidence_tier="discovery", + conversation_messages=[], + ) + test_db.add(other_session) + await test_db.commit() + + resp = await client.get("/api/v1/ai-sessions/escalation-queue", headers=auth_headers) + assert resp.status_code == 200 + ids = {item["id"] for item in resp.json()} + assert str(own_session.id) not in ids + assert str(other_session.id) in ids diff --git a/frontend/src/components/flowpilot/HandoffContextScreen.tsx b/frontend/src/components/flowpilot/HandoffContextScreen.tsx index fbdbd962..5f56d550 100644 --- a/frontend/src/components/flowpilot/HandoffContextScreen.tsx +++ b/frontend/src/components/flowpilot/HandoffContextScreen.tsx @@ -90,7 +90,6 @@ export function HandoffContextScreen({ onContinue, onAIAnalysis, onOwnThing, - onStartHere, onDismiss, dismissible = false, isProcessing = false, diff --git a/frontend/src/pages/AssistantChatPage.tsx b/frontend/src/pages/AssistantChatPage.tsx index 98e5323d..906bd387 100644 --- a/frontend/src/pages/AssistantChatPage.tsx +++ b/frontend/src/pages/AssistantChatPage.tsx @@ -82,7 +82,6 @@ export default function AssistantChatPage() { const [magicHandoff, setMagicHandoff] = useState(null) const [overlayHandoff, setOverlayHandoff] = useState(null) const [overlayLoading, setOverlayLoading] = useState(false) - const [claiming, setClaiming] = useState(false) const [activeOptionKey, setActiveOptionKey] = useState<'continue' | 'ai' | 'own' | null>(null) // Codex correction (locked design): once the magic-moment dissolves, the // AI's `suggested_steps[]` should still be reachable as chips below the @@ -331,52 +330,6 @@ export default function AssistantChatPage() { return () => { cancelled = true } }, [isPickup, urlSessionId, magicState, setSearchParams]) - const handleStartHere = useCallback(async () => { - if (!urlSessionId || !magicHandoff) return - setClaiming(true) - try { - await handoffsApi.claimHandoff(urlSessionId, magicHandoff.id) - // Drop ?pickup=true and dismiss the magic-moment. The session-load - // effect above will then fire because magicState !== 'loading'/'visible' - // and selectChat will populate the chat surface — the senior is now - // escalated_to_id, so GET succeeds and the conversation_messages render - // as chat history. - setSearchParams({}) - setMagicState('dismissed') - // Refresh the sidebar list. Pre-claim the session was invisible to - // listSessions because escalated_to_id was null (junior didn't - // specify a target on /escalate). Post-claim claim_session sets - // escalated_to_id = teamadmin.id, so the session is now in scope. - // Without this re-fetch the senior lands on a session with no - // sidebar entry — looks like the page navigated to a different - // session. - void loadChats() - } catch (e: unknown) { - // Race-condition path (locked design): the loser of the simultaneous - // Pick Up gets a 409 with structured detail so we can name the - // winner and approximate "how long ago." Drop the magic-moment - // (the session is no longer theirs to claim) and let them go back - // to the queue. - if (axios.isAxiosError(e) && e.response?.status === 409) { - const detail = e.response.data?.detail as - | { error?: string; claimed_by_name?: string; claimed_at?: string } - | undefined - if (detail?.error === 'already_claimed') { - const name = detail.claimed_by_name || 'another engineer' - const when = detail.claimed_at ? timeAgo(detail.claimed_at) : 'just now' - toast.info(`Already claimed by ${name} ${when}.`) - setSearchParams({}) - setMagicState('dismissed') - return - } - } - const message = e instanceof Error ? e.message : 'Failed to pick up session' - toast.error(message) - } finally { - setClaiming(false) - } - }, [urlSessionId, magicHandoff, setSearchParams]) - const handleContinue = useCallback(async () => { if (!urlSessionId || !magicHandoff) return setActiveOptionKey('continue')