From faa27479c3b67ce65ba772cc6912a6b923540bfc Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Thu, 21 May 2026 10:44:04 -0700 Subject: [PATCH] fix: review style prompts UX, stale runs, and OAuth refresh (#1321) * fix(dashboard): review style prompts UX, stale runs, and OAuth refresh Reconcile stuck "running" analysis state, add cancel/remove for style profiles, stack the review styles UI vertically, and auto-refresh expiring GitHub user tokens with proactive and 401-triggered rotation. * fix(dashboard): address review feedback on style prompts PR Restore GitHub repo access checks on create/save with token refresh, and preserve running status when LangGraph sync fails transiently. --------- Co-authored-by: open-swe[bot] --- agent/dashboard/oauth.py | 56 +++++-- agent/dashboard/profiles.py | 133 +++++++++++++++- agent/dashboard/review_style_jobs.py | 49 ++++-- agent/dashboard/review_styles.py | 54 ++++++- agent/dashboard/routes.py | 126 +++++++++++---- tests/test_github_oauth_refresh.py | 87 +++++++++++ tests/test_review_style_sync.py | 89 +++++++++++ ui/src/components/ReviewStylesPanel.tsx | 199 +++++++++++++++++------- ui/src/lib/api.ts | 14 ++ 9 files changed, 694 insertions(+), 113 deletions(-) create mode 100644 tests/test_github_oauth_refresh.py create mode 100644 tests/test_review_style_sync.py diff --git a/agent/dashboard/oauth.py b/agent/dashboard/oauth.py index 8b94c863..97cea7a3 100644 --- a/agent/dashboard/oauth.py +++ b/agent/dashboard/oauth.py @@ -8,6 +8,7 @@ import logging import os import secrets import time +from datetime import UTC, datetime, timedelta from typing import Any from urllib.parse import urlparse @@ -139,26 +140,61 @@ def require_session(request: Request) -> dict[str, Any]: return decode_session(token) -async def exchange_code(code: str) -> str: - """Exchange an OAuth authorization code for a user-to-server access token.""" +def expires_at_from_github_response(data: dict[str, Any], *, field: str) -> str | None: + """Convert GitHub ``expires_in`` / ``refresh_token_expires_in`` to an ISO timestamp.""" + raw = data.get(field) + if not isinstance(raw, int | float) or raw <= 0: + return None + return (datetime.now(UTC) + timedelta(seconds=int(raw))).isoformat() + + +async def _request_github_tokens(body: dict[str, str]) -> dict[str, Any]: if not GITHUB_APP_CLIENT_ID or not GITHUB_APP_CLIENT_SECRET: raise HTTPException(500, "GitHub App OAuth not configured") async with httpx.AsyncClient() as client: resp = await client.post( "https://github.com/login/oauth/access_token", headers={"Accept": "application/json"}, - data={ - "client_id": GITHUB_APP_CLIENT_ID, - "client_secret": GITHUB_APP_CLIENT_SECRET, - "code": code, - }, + data=body, ) resp.raise_for_status() data = resp.json() - token = data.get("access_token") - if not token: + if not isinstance(data, dict): + raise HTTPException(502, "unexpected GitHub OAuth response") + if data.get("error"): + raise HTTPException( + 400, f"github oauth error: {data.get('error_description') or data['error']}" + ) + return data + + +async def exchange_code(code: str) -> dict[str, Any]: + """Exchange an OAuth authorization code for user-to-server tokens.""" + data = await _request_github_tokens( + { + "client_id": GITHUB_APP_CLIENT_ID, + "client_secret": GITHUB_APP_CLIENT_SECRET, + "code": code, + } + ) + if not data.get("access_token"): raise HTTPException(400, f"oauth exchange failed: {data}") - return token + return data + + +async def refresh_user_access_token(refresh_token: str) -> dict[str, Any]: + """Rotate an expiring user access token using its refresh token.""" + data = await _request_github_tokens( + { + "client_id": GITHUB_APP_CLIENT_ID, + "client_secret": GITHUB_APP_CLIENT_SECRET, + "grant_type": "refresh_token", + "refresh_token": refresh_token, + } + ) + if not data.get("access_token"): + raise HTTPException(400, f"oauth refresh failed: {data}") + return data async def fetch_github_user(access_token: str) -> tuple[dict[str, Any], str | None]: diff --git a/agent/dashboard/profiles.py b/agent/dashboard/profiles.py index 0f33db0d..edf81d26 100644 --- a/agent/dashboard/profiles.py +++ b/agent/dashboard/profiles.py @@ -12,15 +12,18 @@ each other's fields even when they interleave. from __future__ import annotations +import asyncio import logging -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any import httpx +from fastapi import HTTPException from langgraph_sdk import get_client from pydantic import BaseModel, field_validator from ..encryption import decrypt_token, encrypt_token +from .oauth import expires_at_from_github_response, refresh_user_access_token from .options import SUPPORTED_MODEL_IDS, model_supports_effort logger = logging.getLogger(__name__) @@ -106,33 +109,147 @@ async def upsert_profile(login: str, email: str, update: ProfileUpdate) -> dict[ return value -async def upsert_access_token(login: str, email: str, access_token: str) -> None: - """Persist (or refresh) the user's encrypted GitHub OAuth token. +_refresh_locks: dict[str, asyncio.Lock] = {} + + +def _refresh_lock(login: str) -> asyncio.Lock: + lock = _refresh_locks.get(login) + if lock is None: + lock = asyncio.Lock() + _refresh_locks[login] = lock + return lock + + +def _token_expired(expires_at: str | None, *, skew_seconds: int = 300) -> bool: + if not isinstance(expires_at, str) or not expires_at: + return False + try: + exp = datetime.fromisoformat(expires_at.replace("Z", "+00:00")) + if exp.tzinfo is None: + exp = exp.replace(tzinfo=UTC) + except ValueError: + return False + return datetime.now(UTC) + timedelta(seconds=skew_seconds) >= exp + + +async def upsert_access_token( + login: str, + email: str, + access_token: str, + *, + refresh_token: str | None = None, + token_expires_at: str | None = None, + refresh_token_expires_at: str | None = None, +) -> None: + """Persist (or refresh) the user's encrypted GitHub OAuth tokens. Only touches ``["oauth_tokens"]`` — the user-editable profile is left intact even if a save is in flight in another request. """ if not access_token: return + existing = await _get_value(OAUTH_TOKENS_NAMESPACE, login) or {} value: dict[str, Any] = { "login": login, - "email": email, + "email": email or existing.get("email", ""), "encrypted_gh_token": encrypt_token(access_token), "updated_at": datetime.now(UTC).isoformat(), } + if refresh_token: + value["encrypted_gh_refresh_token"] = encrypt_token(refresh_token) + elif existing.get("encrypted_gh_refresh_token"): + value["encrypted_gh_refresh_token"] = existing["encrypted_gh_refresh_token"] + if token_expires_at: + value["token_expires_at"] = token_expires_at + if refresh_token_expires_at: + value["refresh_token_expires_at"] = refresh_token_expires_at await _client().store.put_item(OAUTH_TOKENS_NAMESPACE, login, value) -async def get_access_token(login: str) -> str | None: - record = await _get_value(OAUTH_TOKENS_NAMESPACE, login) - if not record: - return None +async def upsert_access_token_from_github_response( + login: str, email: str, data: dict[str, Any] +) -> None: + """Store tokens from a GitHub OAuth code exchange or refresh response.""" + access_token = data.get("access_token") + if not isinstance(access_token, str) or not access_token: + return + refresh_token = data.get("refresh_token") + await upsert_access_token( + login, + email, + access_token, + refresh_token=refresh_token if isinstance(refresh_token, str) else None, + token_expires_at=expires_at_from_github_response(data, field="expires_in"), + refresh_token_expires_at=expires_at_from_github_response( + data, field="refresh_token_expires_in" + ), + ) + + +def _decrypt_access_token(record: dict[str, Any]) -> str | None: encrypted = record.get("encrypted_gh_token") if not encrypted: return None return decrypt_token(encrypted) or None +def _decrypt_refresh_token(record: dict[str, Any]) -> str | None: + encrypted = record.get("encrypted_gh_refresh_token") + if not encrypted: + return None + return decrypt_token(encrypted) or None + + +async def _refresh_stored_token(login: str, record: dict[str, Any]) -> str | None: + refresh_token = _decrypt_refresh_token(record) + if not refresh_token: + return None + try: + data = await refresh_user_access_token(refresh_token) + except HTTPException: + logger.warning("GitHub token refresh failed for %s", login, exc_info=True) + return None + except Exception: + logger.warning("GitHub token refresh failed for %s", login, exc_info=True) + return None + email = record.get("email") if isinstance(record.get("email"), str) else "" + await upsert_access_token_from_github_response(login, email, data) + return data.get("access_token") if isinstance(data.get("access_token"), str) else None + + +async def get_valid_access_token(login: str, *, force_refresh: bool = False) -> str | None: + """Return a GitHub access token, refreshing proactively when near expiry.""" + record = await _get_value(OAUTH_TOKENS_NAMESPACE, login) + if not record: + return None + + access_token = _decrypt_access_token(record) + if not access_token: + return None + + if not force_refresh and not _token_expired(record.get("token_expires_at")): + return access_token + + if not _decrypt_refresh_token(record): + return access_token + + async with _refresh_lock(login): + record = await _get_value(OAUTH_TOKENS_NAMESPACE, login) + if not record: + return None + access_token = _decrypt_access_token(record) + if not access_token: + return None + if not force_refresh and not _token_expired(record.get("token_expires_at")): + return access_token + refreshed = await _refresh_stored_token(login, record) + return refreshed or access_token + + +async def get_access_token(login: str) -> str | None: + return await get_valid_access_token(login) + + async def list_profiles() -> list[dict[str, Any]]: result = await _client().store.search_items(PROFILES_NAMESPACE, limit=1000) items = result.get("items") if isinstance(result, dict) else getattr(result, "items", []) diff --git a/agent/dashboard/review_style_jobs.py b/agent/dashboard/review_style_jobs.py index 26d605f1..cb9e5605 100644 --- a/agent/dashboard/review_style_jobs.py +++ b/agent/dashboard/review_style_jobs.py @@ -15,8 +15,10 @@ from ..review_style_collector import ( ) from .review_styles import ( get_review_style, + has_saved_prompt, mark_analysis_failed, mark_analysis_running, + reconcile_running_status, update_review_style, ) @@ -130,6 +132,8 @@ async def sync_review_style_run_status(full_name: str) -> dict[str, Any]: return record client = _client() + run_status: str | None = None + run_missing = False try: if isinstance(run_id, str) and run_id: run = await client.runs.get(thread_id, run_id) @@ -138,16 +142,39 @@ async def sync_review_style_run_status(full_name: str) -> dict[str, Any]: items = runs if isinstance(runs, list) else (runs.get("runs") or []) run = items[0] if items else None if not run: - return record - status = run.get("status") if isinstance(run, dict) else getattr(run, "status", None) - if status in ("success", "completed"): - return await get_review_style(full_name) or record - if status in ("error", "failed", "timeout", "interrupted"): - logger.warning("Review style analyzer run failed for %s (status=%s)", full_name, status) - return await mark_analysis_failed( - full_name, - "Analysis run failed. Please retry later.", - ) + run_missing = True + else: + raw = run.get("status") if isinstance(run, dict) else getattr(run, "status", None) + run_status = raw.lower() if isinstance(raw, str) else None except Exception: logger.debug("Could not sync run status for %s", full_name, exc_info=True) - return record + return record + + return await reconcile_running_status( + full_name, record, run_status=run_status, run_missing=run_missing + ) + + +async def cancel_review_style_analysis(full_name: str) -> dict[str, Any]: + """Stop an in-flight analyzer run and clear stale ``running`` status.""" + record = await get_review_style(full_name) + if not record: + return {} + + if record.get("status") != "running": + return record + + thread_id = record.get("analysis_thread_id") + run_id = record.get("analysis_run_id") + if isinstance(thread_id, str) and isinstance(run_id, str) and thread_id and run_id: + try: + await _client().runs.cancel(thread_id, run_id, wait=False) + except Exception: + logger.debug("Could not cancel review style run for %s", full_name, exc_info=True) + + if has_saved_prompt(record): + return await update_review_style(full_name, {"status": "completed", "error": None}) + return await update_review_style( + full_name, + {"status": "idle", "error": None, "analysis_run_id": None}, + ) diff --git a/agent/dashboard/review_styles.py b/agent/dashboard/review_styles.py index fc9f9b07..0a0e759e 100644 --- a/agent/dashboard/review_styles.py +++ b/agent/dashboard/review_styles.py @@ -130,8 +130,60 @@ async def update_review_style(full_name: str, patch: dict[str, Any]) -> dict[str return value +def has_saved_prompt(record: dict[str, Any]) -> bool: + prompt = record.get("custom_prompt") + return isinstance(prompt, str) and bool(prompt.strip()) + + async def set_custom_prompt(full_name: str, custom_prompt: str) -> dict[str, Any]: - return await update_review_style(full_name, {"custom_prompt": custom_prompt}) + existing = await get_review_style(full_name) + patch: dict[str, Any] = {"custom_prompt": custom_prompt} + if existing and existing.get("status") == "running": + patch["status"] = "completed" + patch["error"] = None + return await update_review_style(full_name, patch) + + +async def reconcile_running_status( + full_name: str, + record: dict[str, Any], + *, + run_status: str | None, + run_missing: bool = False, +) -> dict[str, Any]: + """Clear stale ``running`` when the analyzer run is done or unreachable.""" + if record.get("status") != "running": + return record + + terminal_success = frozenset({"success", "completed"}) + terminal_failure = frozenset({"error", "failed", "timeout", "interrupted", "cancelled"}) + + if run_status in terminal_success: + if has_saved_prompt(record): + return await update_review_style(full_name, {"status": "completed", "error": None}) + return await mark_analysis_failed( + full_name, + "Analysis finished without saving a prompt. Please retry.", + ) + + if run_status in terminal_failure: + if has_saved_prompt(record): + return await update_review_style(full_name, {"status": "completed", "error": None}) + return await mark_analysis_failed(full_name, "Analysis run ended. Please retry.") + + if run_missing: + if has_saved_prompt(record): + return await update_review_style(full_name, {"status": "completed", "error": None}) + return await mark_analysis_failed( + full_name, + "Analysis was interrupted or the run is no longer available. Please retry.", + ) + + return record + + +async def delete_review_style(full_name: str) -> None: + await _client().store.delete_item(REVIEW_STYLES_NAMESPACE, full_name) async def mark_analysis_running( diff --git a/agent/dashboard/routes.py b/agent/dashboard/routes.py index 205c7fb2..29f32e5b 100644 --- a/agent/dashboard/routes.py +++ b/agent/dashboard/routes.py @@ -35,17 +35,22 @@ from .oauth import ( from .options import SUPPORTED_MODELS from .profiles import ( ProfileUpdate, - get_access_token, get_profile, + get_valid_access_token, list_profiles, - upsert_access_token, + upsert_access_token_from_github_response, upsert_profile, ) -from .review_style_jobs import start_review_style_analysis, sync_review_style_run_status +from .review_style_jobs import ( + cancel_review_style_analysis, + start_review_style_analysis, + sync_review_style_run_status, +) from .review_styles import ( ReviewStyleCreate, ReviewStylePromptUpdate, create_review_style, + delete_review_style, get_review_style, list_review_styles, normalize_repo_full_name, @@ -162,13 +167,16 @@ async def auth_callback(request: Request, code: str, state: str) -> RedirectResp redirect_to = sanitize_redirect_to(state_payload.get("redirect_to")) or _frontend_base_url() - access_token = await exchange_code(code) + token_data = await exchange_code(code) + access_token = token_data.get("access_token") + if not isinstance(access_token, str): + raise HTTPException(400, "oauth exchange missing access_token") user, email = await fetch_github_user(access_token) login = user.get("login") if not login: raise HTTPException(400, "could not resolve GitHub login") - await upsert_access_token(login, email or "", access_token) + await upsert_access_token_from_github_response(login, email or "", token_data) session_jwt = issue_session(login=login, email=email, avatar_url=user.get("avatar_url")) response = RedirectResponse(redirect_to, status_code=302) @@ -337,7 +345,8 @@ async def list_repos( ``/user/installations/{id}/repositories`` so users with multiple installations or >30 accessible repos get the complete set. """ - token = await get_access_token(session["sub"]) + login = session["sub"] + token = await get_valid_access_token(login) if not token: raise HTTPException(401, "github token unavailable, re-login required") headers = { @@ -346,12 +355,26 @@ async def list_repos( "X-GitHub-Api-Version": "2022-11-28", } async with httpx.AsyncClient() as client: - installations = await _paginate( - client, - "https://api.github.com/user/installations", - headers=headers, - items_key="installations", - ) + try: + installations = await _paginate( + client, + "https://api.github.com/user/installations", + headers=headers, + items_key="installations", + ) + except HTTPException as exc: + if exc.status_code != 401: + raise + token = await get_valid_access_token(login, force_refresh=True) + if not token: + raise HTTPException(401, "github token expired, re-login required") from exc + headers["Authorization"] = f"Bearer {token}" + installations = await _paginate( + client, + "https://api.github.com/user/installations", + headers=headers, + items_key="installations", + ) repositories: list[dict[str, Any]] = [] for inst in installations: inst_id = inst.get("id") @@ -386,6 +409,17 @@ async def list_repos( } +def _raise_for_github_repo_status(status_code: int) -> None: + if status_code == 401: + raise HTTPException(401, "github token expired, re-login required") + if status_code == 404: + raise HTTPException(404, "repository not found") + if status_code == 403: + raise HTTPException(403, "no access to this private repository") + if status_code != 200: + raise HTTPException(502, f"github API error ({status_code})") + + async def _assert_repo_available_for_style_analysis(full_name: str, token: str) -> None: """Ensure the repo exists and is readable for style learning. @@ -404,18 +438,30 @@ async def _assert_repo_available_for_style_analysis(full_name: str, token: str) f"https://api.github.com/repos/{owner}/{name}", headers=headers, ) - if r.status_code == 404: - raise HTTPException(404, "repository not found") - if r.status_code == 403: - raise HTTPException(403, "no access to this private repository") - if r.status_code != 200: - raise HTTPException(502, f"github API error ({r.status_code})") + _raise_for_github_repo_status(r.status_code) body = r.json() if body.get("private") is not True: return # Private repo: 200 from GitHub implies the user's token can read it. +async def _require_repo_access_for_user(login: str, full_name: str) -> str: + """Verify the user can read ``full_name`` on GitHub; return a valid access token.""" + token = await get_valid_access_token(login) + if not token: + raise HTTPException(401, "github token unavailable, re-login required") + try: + await _assert_repo_available_for_style_analysis(full_name, token) + except HTTPException as exc: + if exc.status_code != 401: + raise + token = await get_valid_access_token(login, force_refresh=True) + if not token: + raise HTTPException(401, "github token expired, re-login required") from exc + await _assert_repo_available_for_style_analysis(full_name, token) + return token + + @router.get("/review-styles") async def api_list_review_styles( session: dict[str, Any] = _SESSION_DEP, @@ -436,10 +482,7 @@ async def api_create_review_style( body: ReviewStyleCreate, session: dict[str, Any] = _SESSION_DEP, ) -> dict[str, Any]: - token = await get_access_token(session["sub"]) - if not token: - raise HTTPException(401, "github token unavailable, re-login required") - await _assert_repo_available_for_style_analysis(body.full_name, token) + await _require_repo_access_for_user(session["sub"], body.full_name) return await create_review_style(body.full_name, session["sub"]) @@ -467,6 +510,7 @@ async def api_update_review_style_prompt( record = await get_review_style(full_name) if not record: raise HTTPException(404, "review style not found") + await _require_repo_access_for_user(session["sub"], full_name) return await set_custom_prompt(full_name, body.custom_prompt) @@ -476,17 +520,45 @@ async def api_analyze_review_style( session: dict[str, Any] = _SESSION_DEP, ) -> dict[str, Any]: full_name = normalize_repo_full_name(full_name) - token = await get_access_token(session["sub"]) - if not token: - raise HTTPException(401, "github token unavailable, re-login required") - await _assert_repo_available_for_style_analysis(full_name, token) + token = await _require_repo_access_for_user(session["sub"], full_name) record = await get_review_style(full_name) if not record: record = await create_review_style(full_name, session["sub"]) if record.get("status") == "running": - raise HTTPException(409, "analysis already running") + record = await sync_review_style_run_status(full_name) + if record.get("status") == "running": + raise HTTPException(409, "analysis already running") return await start_review_style_analysis( full_name, github_token=token, created_by=session["sub"], ) + + +@router.post("/review-styles/{full_name:path}/cancel") +async def api_cancel_review_style( + full_name: str, + session: dict[str, Any] = _SESSION_DEP, +) -> dict[str, Any]: + del session + full_name = normalize_repo_full_name(full_name) + record = await get_review_style(full_name) + if not record: + raise HTTPException(404, "review style not found") + return await cancel_review_style_analysis(full_name) + + +@router.delete("/review-styles/{full_name:path}") +async def api_delete_review_style( + full_name: str, + session: dict[str, Any] = _SESSION_DEP, +) -> Response: + del session + full_name = normalize_repo_full_name(full_name) + record = await get_review_style(full_name) + if not record: + raise HTTPException(404, "review style not found") + if record.get("status") == "running": + await cancel_review_style_analysis(full_name) + await delete_review_style(full_name) + return Response(status_code=204) diff --git a/tests/test_github_oauth_refresh.py b/tests/test_github_oauth_refresh.py new file mode 100644 index 00000000..4c58bc3e --- /dev/null +++ b/tests/test_github_oauth_refresh.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, patch + +import pytest + +from agent.dashboard.oauth import expires_at_from_github_response +from agent.dashboard.profiles import _token_expired, get_valid_access_token + + +def test_expires_at_from_github_response() -> None: + data = {"expires_in": 3600} + expires = expires_at_from_github_response(data, field="expires_in") + assert expires is not None + exp = datetime.fromisoformat(expires) + assert exp > datetime.now(UTC) + + +def test_token_expired_with_skew() -> None: + past = (datetime.now(UTC) - timedelta(minutes=1)).isoformat() + assert _token_expired(past, skew_seconds=300) is True + future = (datetime.now(UTC) + timedelta(hours=2)).isoformat() + assert _token_expired(future, skew_seconds=300) is False + assert _token_expired(None) is False + + +@pytest.mark.asyncio +async def test_get_valid_access_token_refreshes_when_near_expiry() -> None: + soon = (datetime.now(UTC) + timedelta(minutes=1)).isoformat() + record = { + "email": "u@example.com", + "encrypted_gh_token": "enc-access", + "encrypted_gh_refresh_token": "enc-refresh", + "token_expires_at": soon, + } + with ( + patch( + "agent.dashboard.profiles._get_value", + new_callable=AsyncMock, + return_value=record, + ), + patch("agent.dashboard.profiles._decrypt_access_token", return_value="old-access"), + patch("agent.dashboard.profiles._decrypt_refresh_token", return_value="ghr_test"), + patch( + "agent.dashboard.profiles.refresh_user_access_token", + new_callable=AsyncMock, + return_value={ + "access_token": "new-access", + "refresh_token": "ghr_new", + "expires_in": 28800, + "refresh_token_expires_in": 15897600, + }, + ), + patch( + "agent.dashboard.profiles.upsert_access_token_from_github_response", + new_callable=AsyncMock, + ) as mock_upsert, + ): + token = await get_valid_access_token("octo") + assert token == "new-access" + mock_upsert.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_get_valid_access_token_returns_stored_when_not_expiring() -> None: + future = (datetime.now(UTC) + timedelta(hours=5)).isoformat() + record = { + "encrypted_gh_token": "enc-access", + "encrypted_gh_refresh_token": "enc-refresh", + "token_expires_at": future, + } + with ( + patch( + "agent.dashboard.profiles._get_value", + new_callable=AsyncMock, + return_value=record, + ), + patch("agent.dashboard.profiles._decrypt_access_token", return_value="still-good"), + patch( + "agent.dashboard.profiles.refresh_user_access_token", + new_callable=AsyncMock, + ) as mock_refresh, + ): + token = await get_valid_access_token("octo") + assert token == "still-good" + mock_refresh.assert_not_called() diff --git a/tests/test_review_style_sync.py b/tests/test_review_style_sync.py new file mode 100644 index 00000000..353c6e36 --- /dev/null +++ b/tests/test_review_style_sync.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest + +from agent.dashboard.review_styles import reconcile_running_status + + +@pytest.mark.asyncio +async def test_reconcile_running_marks_completed_when_prompt_saved() -> None: + record = { + "full_name": "acme/repo", + "status": "running", + "custom_prompt": "Prefer concrete runtime checks.", + } + with patch( + "agent.dashboard.review_styles.update_review_style", + new_callable=AsyncMock, + return_value={**record, "status": "completed"}, + ) as mock_up: + out = await reconcile_running_status( + "acme/repo", record, run_status="success", run_missing=False + ) + mock_up.assert_awaited_once() + assert out["status"] == "completed" + + +@pytest.mark.asyncio +async def test_reconcile_running_marks_failed_when_run_success_without_prompt() -> None: + record = {"full_name": "acme/repo", "status": "running", "custom_prompt": None} + with patch( + "agent.dashboard.review_styles.mark_analysis_failed", + new_callable=AsyncMock, + return_value={**record, "status": "failed"}, + ) as mock_fail: + out = await reconcile_running_status( + "acme/repo", record, run_status="completed", run_missing=False + ) + mock_fail.assert_awaited_once() + assert out["status"] == "failed" + + +@pytest.mark.asyncio +async def test_reconcile_running_marks_completed_when_run_missing_but_prompt_exists() -> None: + record = { + "full_name": "keycloak/keycloak", + "status": "running", + "custom_prompt": "Prioritize security boundaries.", + } + with patch( + "agent.dashboard.review_styles.update_review_style", + new_callable=AsyncMock, + return_value={**record, "status": "completed"}, + ) as mock_up: + out = await reconcile_running_status( + "keycloak/keycloak", record, run_status=None, run_missing=True + ) + mock_up.assert_awaited_once() + assert out["status"] == "completed" + + +@pytest.mark.asyncio +async def test_sync_preserves_running_when_langgraph_errors() -> None: + from agent.dashboard.review_style_jobs import sync_review_style_run_status + + record = { + "full_name": "acme/repo", + "status": "running", + "analysis_thread_id": "thread-1", + "analysis_run_id": "run-1", + } + mock_client = AsyncMock() + mock_client.runs.get = AsyncMock(side_effect=RuntimeError("network blip")) + with ( + patch( + "agent.dashboard.review_style_jobs.get_review_style", + new_callable=AsyncMock, + return_value=record, + ), + patch("agent.dashboard.review_style_jobs._client", return_value=mock_client), + patch( + "agent.dashboard.review_style_jobs.reconcile_running_status", + new_callable=AsyncMock, + ) as mock_reconcile, + ): + out = await sync_review_style_run_status("acme/repo") + assert out == record + mock_reconcile.assert_not_called() diff --git a/ui/src/components/ReviewStylesPanel.tsx b/ui/src/components/ReviewStylesPanel.tsx index a97a62a7..3599f498 100644 --- a/ui/src/components/ReviewStylesPanel.tsx +++ b/ui/src/components/ReviewStylesPanel.tsx @@ -1,7 +1,7 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useEffect, useState } from "react"; -import type {ReviewStyle} from "@/lib/api"; +import type { ReviewStyle } from "@/lib/api"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { @@ -16,9 +16,15 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Skeleton } from "@/components/ui/skeleton"; import { Textarea } from "@/components/ui/textarea"; -import { ApiError, api } from "@/lib/api"; +import { ApiError, api, isGithubReauthError, loginUrl } from "@/lib/api"; import { normalizeRepoFullName } from "@/lib/repo"; +function formatMutationError(e: Error): string { + return isGithubReauthError(e) + ? "GitHub token expired — sign in again using the link above." + : e.message; +} + function statusVariant(status: ReviewStyle["status"]) { switch (status) { case "completed": @@ -83,7 +89,7 @@ export function ReviewStylesPanel() { setSelected(record.full_name); setError(null); }, - onError: (e: Error) => setError(e.message), + onError: (e: Error) => setError(formatMutationError(e)), }); const analyze = useMutation({ @@ -93,7 +99,7 @@ export function ReviewStylesPanel() { void qc.invalidateQueries({ queryKey: ["reviewStyle", selected] }); setError(null); }, - onError: (e: Error) => setError(e.message), + onError: (e: Error) => setError(formatMutationError(e)), }); const savePrompt = useMutation({ @@ -104,7 +110,30 @@ export function ReviewStylesPanel() { void qc.invalidateQueries({ queryKey: ["reviewStyle", selected] }); setError(null); }, - onError: (e: Error) => setError(e.message), + onError: (e: Error) => setError(formatMutationError(e)), + }); + + const cancelAnalysis = useMutation({ + mutationFn: (full_name: string) => api.cancelReviewStyle(full_name), + onSuccess: () => { + void qc.invalidateQueries({ queryKey: ["reviewStyles"] }); + void qc.invalidateQueries({ queryKey: ["reviewStyle", selected] }); + setError(null); + }, + onError: (e: Error) => setError(formatMutationError(e)), + }); + + const removeStyle = useMutation({ + mutationFn: (full_name: string) => api.deleteReviewStyle(full_name), + onSuccess: (_data, full_name) => { + void qc.invalidateQueries({ queryKey: ["reviewStyles"] }); + if (selected === full_name) { + setSelected(null); + setDraftPrompt(""); + } + setError(null); + }, + onError: (e: Error) => setError(formatMutationError(e)), }); if (styles.isLoading) { @@ -121,26 +150,53 @@ export function ReviewStylesPanel() { const handleAdd = () => { if (!normalizedAddRepo || !canAdd) return; - void createStyle.mutateAsync(normalizedAddRepo).then(() => setAddRepo("")); + void createStyle + .mutateAsync(normalizedAddRepo) + .then(() => setAddRepo("")) + .catch(() => undefined); }; + const githubReauth = + (repos.isError && isGithubReauthError(repos.error)) || + (error !== null && /github token|re-login required/i.test(error)); + return ( -
-
+
+ {githubReauth && ( +
+ Your GitHub connection expired.{" "} + + Sign in with GitHub again + {" "} + to list installed repos and run style analysis. +
+ )} +
- setAddRepo(e.target.value)} - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); - handleAdd(); - } - }} - /> +
+ setAddRepo(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + handleAdd(); + } + }} + className="sm:flex-1" + /> + +
{suggestedRepos.length > 0 && ( r.full_name)} @@ -169,45 +225,47 @@ export function ReviewStylesPanel() { )} -
-
    - {(styles.data ?? []).map((s) => ( -
  • - -
    • - ))} - {(styles.data ?? []).length === 0 && ( -
  • No repositories yet.
    • - )} -
-
-
+
+

Repositories

+ {(styles.data ?? []).length === 0 ? ( +

No repositories yet.

+ ) : ( +
    + {(styles.data ?? []).map((s) => ( +
  • + +
    • + ))} +
+ )} +
+ + +
+ +
{!selected || !active ? (

- Select a repository on the left to view or edit its review style prompt. + Select a repository above to view or edit its review style prompt.

) : ( <> +

{active.full_name}

{active.status} {active.top_reviewers.length > 0 && ( @@ -225,15 +283,27 @@ export function ReviewStylesPanel() {

{active.analysis_summary}

)} {active.error &&

{active.error}

} -
+
+ {active.status === "running" && ( + + )} +