mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 06:53:14 +00:00
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] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
9f6ccd7382
commit
faa27479c3
9 changed files with 694 additions and 113 deletions
|
|
@ -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]:
|
||||
|
|
|
|||
|
|
@ -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", [])
|
||||
|
|
|
|||
|
|
@ -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},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
87
tests/test_github_oauth_refresh.py
Normal file
87
tests/test_github_oauth_refresh.py
Normal file
|
|
@ -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()
|
||||
89
tests/test_review_style_sync.py
Normal file
89
tests/test_review_style_sync.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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 (
|
||||
<div className="grid grid-cols-1 gap-4 p-4 md:grid-cols-[260px_1fr]">
|
||||
<div className="space-y-4">
|
||||
<div className="flex flex-col gap-6 p-4">
|
||||
{githubReauth && (
|
||||
<div className="rounded-md border border-destructive/40 bg-destructive/5 px-3 py-2 text-xs text-destructive">
|
||||
Your GitHub connection expired.{" "}
|
||||
<a href={loginUrl()} className="font-medium underline underline-offset-2">
|
||||
Sign in with GitHub again
|
||||
</a>{" "}
|
||||
to list installed repos and run style analysis.
|
||||
</div>
|
||||
)}
|
||||
<section className="space-y-4">
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="add-repo">Add repository</Label>
|
||||
<Input
|
||||
id="add-repo"
|
||||
placeholder="owner/repo"
|
||||
value={addRepo}
|
||||
onChange={(e) => setAddRepo(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
handleAdd();
|
||||
}
|
||||
}}
|
||||
/>
|
||||
<div className="flex flex-col gap-2 sm:flex-row sm:items-end">
|
||||
<Input
|
||||
id="add-repo"
|
||||
placeholder="owner/repo"
|
||||
value={addRepo}
|
||||
onChange={(e) => setAddRepo(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
handleAdd();
|
||||
}
|
||||
}}
|
||||
className="sm:flex-1"
|
||||
/>
|
||||
<Button
|
||||
size="sm"
|
||||
className="shrink-0 sm:w-auto"
|
||||
disabled={!canAdd || createStyle.isPending}
|
||||
onClick={handleAdd}
|
||||
>
|
||||
Add
|
||||
</Button>
|
||||
</div>
|
||||
{suggestedRepos.length > 0 && (
|
||||
<Combobox
|
||||
items={suggestedRepos.map((r) => r.full_name)}
|
||||
|
|
@ -169,45 +225,47 @@ export function ReviewStylesPanel() {
|
|||
</ComboboxContent>
|
||||
</Combobox>
|
||||
)}
|
||||
<Button
|
||||
size="sm"
|
||||
className="w-full"
|
||||
disabled={!canAdd || createStyle.isPending}
|
||||
onClick={handleAdd}
|
||||
>
|
||||
Add
|
||||
</Button>
|
||||
</div>
|
||||
<ul className="space-y-1">
|
||||
{(styles.data ?? []).map((s) => (
|
||||
<li key={s.full_name}>
|
||||
<button
|
||||
type="button"
|
||||
className={`flex w-full items-center justify-between rounded-md px-2 py-1.5 text-left text-xs hover:bg-muted ${
|
||||
selected === s.full_name ? "bg-muted font-medium" : ""
|
||||
}`}
|
||||
onClick={() => setSelected(s.full_name)}
|
||||
>
|
||||
<span className="truncate">{s.full_name}</span>
|
||||
<Badge variant={statusVariant(s.status)} className="ml-2 shrink-0">
|
||||
{s.status}
|
||||
</Badge>
|
||||
</button>
|
||||
</li>
|
||||
))}
|
||||
{(styles.data ?? []).length === 0 && (
|
||||
<li className="px-2 py-1 text-xs text-muted-foreground">No repositories yet.</li>
|
||||
)}
|
||||
</ul>
|
||||
</div>
|
||||
|
||||
<div className="space-y-3">
|
||||
<div className="space-y-2">
|
||||
<p className="text-xs font-medium text-foreground">Repositories</p>
|
||||
{(styles.data ?? []).length === 0 ? (
|
||||
<p className="text-xs text-muted-foreground">No repositories yet.</p>
|
||||
) : (
|
||||
<ul className="flex flex-wrap gap-2">
|
||||
{(styles.data ?? []).map((s) => (
|
||||
<li key={s.full_name}>
|
||||
<button
|
||||
type="button"
|
||||
className={`inline-flex max-w-full items-center gap-2 rounded-md border px-2.5 py-1.5 text-left text-xs transition-colors hover:bg-muted ${
|
||||
selected === s.full_name
|
||||
? "border-primary bg-muted font-medium"
|
||||
: "border-border"
|
||||
}`}
|
||||
onClick={() => setSelected(s.full_name)}
|
||||
>
|
||||
<span className="truncate">{s.full_name}</span>
|
||||
<Badge variant={statusVariant(s.status)} className="shrink-0">
|
||||
{s.status}
|
||||
</Badge>
|
||||
</button>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
)}
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<div className="border-t border-border" />
|
||||
|
||||
<section className="space-y-3">
|
||||
{!selected || !active ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
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.
|
||||
</p>
|
||||
) : (
|
||||
<>
|
||||
<p className="text-sm font-medium text-foreground">{active.full_name}</p>
|
||||
<div className="flex flex-wrap items-center gap-2 text-xs">
|
||||
<Badge variant={statusVariant(active.status)}>{active.status}</Badge>
|
||||
{active.top_reviewers.length > 0 && (
|
||||
|
|
@ -225,15 +283,27 @@ export function ReviewStylesPanel() {
|
|||
<p className="text-xs text-muted-foreground">{active.analysis_summary}</p>
|
||||
)}
|
||||
{active.error && <p className="text-xs text-destructive">{active.error}</p>}
|
||||
<div className="flex gap-2">
|
||||
<div className="flex flex-wrap gap-2">
|
||||
<Button
|
||||
size="sm"
|
||||
variant="secondary"
|
||||
disabled={active.status === "running" || analyze.isPending}
|
||||
onClick={() => void analyze.mutateAsync(active.full_name)}
|
||||
onClick={() => {
|
||||
void analyze.mutateAsync(active.full_name).catch(() => undefined);
|
||||
}}
|
||||
>
|
||||
{active.status === "running" ? "Analyzing…" : "Run analysis"}
|
||||
</Button>
|
||||
{active.status === "running" && (
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
disabled={cancelAnalysis.isPending}
|
||||
onClick={() => void cancelAnalysis.mutateAsync(active.full_name)}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
)}
|
||||
<Button
|
||||
size="sm"
|
||||
disabled={!draftPrompt.trim() || savePrompt.isPending}
|
||||
|
|
@ -246,9 +316,26 @@ export function ReviewStylesPanel() {
|
|||
>
|
||||
Save prompt
|
||||
</Button>
|
||||
<Button
|
||||
size="sm"
|
||||
variant="destructive"
|
||||
disabled={removeStyle.isPending}
|
||||
onClick={() => {
|
||||
if (
|
||||
!window.confirm(
|
||||
`Remove ${active.full_name} from review style prompts? This cannot be undone.`,
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
void removeStyle.mutateAsync(active.full_name);
|
||||
}}
|
||||
>
|
||||
Remove
|
||||
</Button>
|
||||
</div>
|
||||
<Textarea
|
||||
className="min-h-[320px] font-mono text-xs"
|
||||
className="min-h-[320px] w-full font-mono text-xs"
|
||||
value={draftPrompt}
|
||||
onChange={(e) => setDraftPrompt(e.target.value)}
|
||||
placeholder={
|
||||
|
|
@ -261,7 +348,7 @@ export function ReviewStylesPanel() {
|
|||
</>
|
||||
)}
|
||||
{error && <p className="text-xs text-destructive">{error}</p>}
|
||||
</div>
|
||||
</section>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,6 +18,12 @@ export class ApiError extends Error {
|
|||
}
|
||||
}
|
||||
|
||||
export function isGithubReauthError(error: unknown): boolean {
|
||||
if (!(error instanceof ApiError)) return false;
|
||||
if (error.status === 401) return true;
|
||||
return /github token|re-login required/i.test(error.message);
|
||||
}
|
||||
|
||||
async function request<T>(path: string, init: RequestInit = {}): Promise<T> {
|
||||
const res = await fetch(`${API_BASE}/dashboard/api${path}`, {
|
||||
...init,
|
||||
|
|
@ -154,6 +160,14 @@ export const api = {
|
|||
request<ReviewStyle>(`/review-styles/${encodeURIComponent(full_name)}/analyze`, {
|
||||
method: "POST",
|
||||
}),
|
||||
cancelReviewStyle: (full_name: string) =>
|
||||
request<ReviewStyle>(`/review-styles/${encodeURIComponent(full_name)}/cancel`, {
|
||||
method: "POST",
|
||||
}),
|
||||
deleteReviewStyle: (full_name: string) =>
|
||||
request<void>(`/review-styles/${encodeURIComponent(full_name)}`, {
|
||||
method: "DELETE",
|
||||
}),
|
||||
getTeamSettings: () => request<TeamSettings>("/team-settings"),
|
||||
saveTeamSettings: (body: TeamSettings) =>
|
||||
request<TeamSettings>("/team-settings", { method: "PUT", body: JSON.stringify(body) }),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue