diff --git a/backend/app/api/endpoints/session_suggested_fixes.py b/backend/app/api/endpoints/session_suggested_fixes.py index 0804b013..769cd663 100644 --- a/backend/app/api/endpoints/session_suggested_fixes.py +++ b/backend/app/api/endpoints/session_suggested_fixes.py @@ -217,6 +217,68 @@ async def record_decision( ) +# ── Suggested fix: apply (stamp applied_at) ────────────────────────────── + +@router.post( + "/suggested-fixes/{fix_id}/apply", + response_model=SessionSuggestedFixResponse, +) +async def apply_suggested_fix( + session_id: UUID, + fix_id: UUID, + current_user: Annotated[User, Depends(get_current_active_user)], + db: Annotated[AsyncSession, Depends(get_db)], + _: None = Depends(require_engineer_or_admin), +) -> SessionSuggestedFixResponse: + """Stamp applied_at when the engineer clicks Apply in the ProposalBanner. + + This does NOT change status (fix remains 'proposed'). Status only flips + when the engineer records an outcome via PATCH /outcome. + + Rules: + - Fix must be in 'proposed' status; any other status → 409. + - Idempotent: if applied_at is already set, returns 200 with the unchanged row. + - Bumps ai_sessions.state_version so resolve/escalate preview generators + know the fix has entered the verifying phase. + """ + await _load_session_or_404(db, session_id) + + result = await db.execute( + select(SessionSuggestedFix).where( + SessionSuggestedFix.id == fix_id, + SessionSuggestedFix.session_id == session_id, + ) + ) + fix = result.scalar_one_or_none() + if fix is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Suggested fix not found" + ) + + if fix.status != "proposed": + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Apply is only valid from 'proposed'; fix is already '{fix.status}'", + ) + + # Idempotent: already stamped → return as-is without bumping state_version again. + if fix.applied_at is not None: + return SessionSuggestedFixResponse.model_validate(fix) + + fix.applied_at = datetime.now(timezone.utc) + + # Bump state_version so preview generators see the verifying-phase signal. + await db.execute( + update(AISession) + .where(AISession.id == session_id) + .values(state_version=AISession.state_version + 1) + ) + + await db.commit() + await db.refresh(fix) + return SessionSuggestedFixResponse.model_validate(fix) + + # ── Suggested fix: outcome ──────────────────────────────────────────────── @router.patch( diff --git a/backend/tests/test_fix_outcome_endpoint.py b/backend/tests/test_fix_outcome_endpoint.py index b0969417..89a4fb15 100644 --- a/backend/tests/test_fix_outcome_endpoint.py +++ b/backend/tests/test_fix_outcome_endpoint.py @@ -327,3 +327,113 @@ async def test_resolution_note_preview_reflects_outcome_after_patch( assert distinct_failure_reason in bundle_b, ( "Bundle for second preview should include the failure_reason text" ) + + +# ── Apply endpoint ───────────────────────────────────────────────────────── + +@pytest.mark.asyncio +async def test_apply_stamps_applied_at( + client: AsyncClient, test_user, auth_headers, test_db +): + """POST /apply stamps applied_at and bumps state_version.""" + from uuid import UUID + session_id, fix_id = await _make_session_with_fix(test_db, test_user) + + result = await test_db.execute( + select(AISession).where(AISession.id == UUID(session_id)) + ) + session_obj = result.scalar_one() + initial_version = session_obj.state_version + + r = await client.post( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/apply", + headers=auth_headers, + ) + assert r.status_code == 200, r.text + body = r.json() + assert body["applied_at"] is not None, "applied_at must be set after /apply" + assert body["status"] == "proposed", "status must remain 'proposed' after /apply" + + await test_db.refresh(session_obj) + assert session_obj.state_version == initial_version + 1, ( + "/apply must bump state_version so preview cache is invalidated" + ) + + +@pytest.mark.asyncio +async def test_apply_is_idempotent( + client: AsyncClient, test_user, auth_headers, test_db +): + """Second POST /apply returns 200 with applied_at unchanged (no double-bump).""" + from uuid import UUID + session_id, fix_id = await _make_session_with_fix(test_db, test_user) + + r1 = await client.post( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/apply", + headers=auth_headers, + ) + assert r1.status_code == 200, r1.text + applied_at_first = r1.json()["applied_at"] + + result = await test_db.execute( + select(AISession).where(AISession.id == UUID(session_id)) + ) + session_obj = result.scalar_one() + version_after_first = session_obj.state_version + + r2 = await client.post( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/apply", + headers=auth_headers, + ) + assert r2.status_code == 200, r2.text + assert r2.json()["applied_at"] == applied_at_first, ( + "applied_at must not change on second /apply call" + ) + + await test_db.refresh(session_obj) + assert session_obj.state_version == version_after_first, ( + "state_version must not be bumped a second time on idempotent /apply" + ) + + +@pytest.mark.asyncio +async def test_apply_rejects_non_proposed( + client: AsyncClient, test_user, auth_headers, test_db +): + """POST /apply returns 409 when fix status is 'applied_success'.""" + session_id, fix_id = await _make_session_with_fix(test_db, test_user) + + # Advance the fix to a terminal status via the outcome endpoint. + r_outcome = await client.patch( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome", + headers=auth_headers, + json={"outcome": "applied_success"}, + ) + assert r_outcome.status_code == 200 + + r = await client.post( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/apply", + headers=auth_headers, + ) + assert r.status_code == 409, r.text + + +@pytest.mark.asyncio +async def test_apply_rejects_dismissed( + client: AsyncClient, test_user, auth_headers, test_db +): + """POST /apply returns 409 when fix status is 'dismissed'.""" + session_id, fix_id = await _make_session_with_fix(test_db, test_user) + + r_outcome = await client.patch( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/outcome", + headers=auth_headers, + json={"outcome": "dismissed"}, + ) + assert r_outcome.status_code == 200 + + r = await client.post( + f"/api/v1/ai-sessions/{session_id}/suggested-fixes/{fix_id}/apply", + headers=auth_headers, + ) + assert r.status_code == 409, r.text diff --git a/frontend/src/api/sessionSuggestedFixes.ts b/frontend/src/api/sessionSuggestedFixes.ts index 078d0d87..fbbf654c 100644 --- a/frontend/src/api/sessionSuggestedFixes.ts +++ b/frontend/src/api/sessionSuggestedFixes.ts @@ -111,6 +111,19 @@ export const sessionSuggestedFixesApi = { return r.data }, + /** + * Stamp applied_at when the engineer clicks Apply in the ProposalBanner. + * Does NOT change status (fix remains 'proposed'). Status flips only on + * a subsequent PATCH /outcome. Idempotent if applied_at is already set. + * Returns 409 if the fix is no longer in 'proposed' status. + */ + async applyFix(sessionId: string, fixId: string): Promise { + const r = await apiClient.post( + `/ai-sessions/${sessionId}/suggested-fixes/${fixId}/apply`, + ) + return r.data + }, + /** * Record the outcome of applying a suggested fix. Transition rules: * - from `proposed` or `applied_partial`: any outcome is valid (partial is diff --git a/frontend/src/pages/AssistantChatPage.tsx b/frontend/src/pages/AssistantChatPage.tsx index 20936fd1..ff7bb2ea 100644 --- a/frontend/src/pages/AssistantChatPage.tsx +++ b/frontend/src/pages/AssistantChatPage.tsx @@ -135,20 +135,19 @@ export default function AssistantChatPage() { const isNarrow = useMediaQuery('(max-width: 1199px)') // Phase 8: ProposalBanner + EscalateInterceptDialog state. const [bannerCollapsed, setBannerCollapsed] = useState(false) - const [bannerApplied, setBannerApplied] = useState(false) const [postApplyMsgCount, setPostApplyMsgCount] = useState(0) const [nudgeSilenced, setNudgeSilenced] = useState(false) const [escalateIntercept, setEscalateIntercept] = useState<{ fixId: string; fixTitle: string } | null>(null) - // Phase 8: compute the current banner mode from activeFix + client-side flags. - // bannerApplied is a client-side-only flag that flips on Apply click so we - // don't need a server round-trip just to stamp applied_at for mode computation. + // Phase 8: compute the current banner mode from activeFix. + // applied_at is now persisted on the server (stamped by POST /apply), + // so bannerMode is derived entirely from server state — no client-side flag. const bannerMode: BannerMode | null = (() => { if (!activeFix) return null if (activeFix.status === 'dismissed') return null if (activeFix.ai_outcome_proposal) return 'ai_confirming' if (activeFix.status === 'applied_partial') return 'partial' if (activeFix.status === 'applied_success' || activeFix.status === 'applied_failed') return null - if (bannerApplied || activeFix.applied_at) { + if (activeFix.applied_at) { if (postApplyMsgCount >= 3 && !nudgeSilenced) return 'nudge' return 'verifying' } @@ -341,7 +340,6 @@ export default function AssistantChatPage() { setScriptPanelOpen(false) // Phase 8: banner state reset setBannerCollapsed(false) - setBannerApplied(false) setPostApplyMsgCount(0) setNudgeSilenced(false) setEscalateIntercept(null) @@ -513,16 +511,23 @@ export default function AssistantChatPage() { refreshPreview(activeChatId, kind) }, [activeChatId, previewKind, refreshPreview]) - // Phase 8: handleApplyFix — opens the existing script panel (same as clicking - // the SuggestedFix card) and stamps the client-side bannerApplied flag so - // bannerMode transitions to 'verifying' without a server round-trip. - const handleApplyFix = useCallback(() => { - if (!activeFix) return - setBannerApplied(true) + // Phase 8: handleApplyFix — stamps applied_at on the server so Verifying state + // survives refresh/reselect/multi-tab, then opens the script panel. + const handleApplyFix = useCallback(async () => { + if (!activeFix || !activeChatId) return setPostApplyMsgCount(0) setNudgeSilenced(false) + try { + const updated = await sessionSuggestedFixesApi.applyFix(activeChatId, activeFix.id) + setActiveFix(updated) + } catch (err: unknown) { + // Non-fatal: the script panel still opens. The banner will stay in + // 'proposed' mode until the next refreshActiveFix succeeds, which is + // a cosmetic gap only — no data loss. + console.error('[AssistantChat] applyFix failed:', err) + } setScriptPanelOpen(true) - }, [activeFix]) + }, [activeFix, activeChatId]) // Phase 8: record a terminal outcome for the active fix. Updates local state // on success. For applied_success also opens the Resolve preview. @@ -532,7 +537,6 @@ export default function AssistantChatPage() { const updated = await sessionSuggestedFixesApi.patchOutcome(activeChatId, activeFix.id, outcome, notes) setActiveFix(updated) // Reset apply tracking state since we now have a terminal outcome. - setBannerApplied(false) setPostApplyMsgCount(0) setNudgeSilenced(false) if (outcome === 'applied_success') { @@ -586,7 +590,7 @@ export default function AssistantChatPage() { const handleEscalateClick = useCallback(() => { const inVerifyState = activeFix && ( - ((bannerApplied || !!activeFix.applied_at) && activeFix.status === 'proposed') || + (!!activeFix.applied_at && activeFix.status === 'proposed') || activeFix.status === 'applied_partial' ) if (inVerifyState && activeFix) { @@ -594,7 +598,7 @@ export default function AssistantChatPage() { return } setShowConclude(true) - }, [activeFix, bannerApplied]) + }, [activeFix]) const handleInterceptChoice = useCallback(async (choice: InterceptChoice) => { const stored = escalateIntercept @@ -614,7 +618,7 @@ export default function AssistantChatPage() { // Phase 8: Resolve click — auto-mark applied_success if in verifying state // before opening the resolution note preview. const handleResolveClick = useCallback(async () => { - if (activeFix && (bannerApplied || activeFix.applied_at) && activeFix.status === 'proposed' && activeChatId) { + if (activeFix && activeFix.applied_at && activeFix.status === 'proposed' && activeChatId) { try { const updated = await sessionSuggestedFixesApi.patchOutcome(activeChatId, activeFix.id, 'applied_success') setActiveFix(updated) @@ -623,7 +627,7 @@ export default function AssistantChatPage() { } } setShowConclude(true) - }, [activeChatId, activeFix, bannerApplied]) + }, [activeChatId, activeFix]) const handleClosePreview = () => { setPreviewKind(null) @@ -841,8 +845,7 @@ export default function AssistantChatPage() { // Only increments when fix is still in 'proposed' (verifying) state — // partial/dismissed/terminal states don't render the nudge, and a // partial→verifying transition could inherit an already-saturated counter. - if (activeFix && (bannerApplied || activeFix.applied_at) && - activeFix.status === 'proposed') { + if (activeFix && activeFix.applied_at && activeFix.status === 'proposed') { setPostApplyMsgCount(c => c + 1) } // Refetch facts + active fix; preview refreshes if open. @@ -917,8 +920,7 @@ export default function AssistantChatPage() { } // Phase 8: increment post-apply message counter for nudge logic (mirrors handleSend). // Only increments in 'proposed' (verifying) state — same rationale as handleSend. - if (activeFix && (bannerApplied || activeFix.applied_at) && - activeFix.status === 'proposed') { + if (activeFix && activeFix.applied_at && activeFix.status === 'proposed') { setPostApplyMsgCount(c => c + 1) } // Refetch facts + active fix; answering tasks is the primary trigger.