From 2070a770c269bef6104b860f68b4619580923527 Mon Sep 17 00:00:00 2001 From: Brace Sproul Date: Thu, 4 Jun 2026 09:33:51 -0700 Subject: [PATCH] fix: stop storing GitHub tokens in metadata (#1405) * fix: stop storing GitHub tokens in metadata Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com> * fix: bound in-process GitHub token cache with 24h TTL + sweep --------- Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com> Co-authored-by: open-swe[bot] --- AGENTS.md | 2 +- agent/dashboard/thread_api.py | 17 +-- agent/reviewer.py | 27 ++--- agent/reviewer_findings.py | 2 +- agent/server.py | 4 +- agent/utils/auth.py | 74 +++++-------- agent/utils/github_token.py | 158 ++++++++++++---------------- agent/webapp.py | 57 ++++------ tests/test_agent_subagent_models.py | 4 +- tests/test_auth_sources.py | 25 ++--- tests/test_github_issue_webhook.py | 52 ++++----- tests/test_github_token_ttl.py | 95 +++++++---------- tests/test_pr_ready_auto_review.py | 12 +-- tests/test_proxy_auth.py | 4 +- tests/test_reviewer.py | 20 ++-- tests/test_reviewer_watch.py | 24 ++--- 16 files changed, 228 insertions(+), 349 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 430f5498..6cb08d3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,7 +98,7 @@ Supported model IDs and per-model effort/reasoning rules live in `agent/dashboar ### Auth -- **GitHub**: dual-mode. User OAuth tokens are encrypted-at-rest in thread metadata (`agent/encryption.py`, `utils/auth.py:resolve_github_token`). When no user token is available, falls back to a GitHub App installation token (`utils/github_app.py`). The installation token is also what configures the LangSmith sandbox's GitHub proxy. +- **GitHub**: dual-mode. User OAuth tokens are encrypted at rest in the dashboard OAuth store and cached only in process during a run (`utils/auth.py:resolve_github_token`, `utils/github_token.py`). When no user token is available, falls back to a GitHub App installation token (`utils/github_app.py`). The installation token is also what configures the LangSmith sandbox's GitHub proxy. - **Webhooks**: GitHub signatures verified in `utils/github_comments.py:verify_github_signature`; Slack/Linear handled in their respective utils. - **Dashboard / UI**: GitHub OAuth login lives in `agent/dashboard/oauth.py` and `routes.py` (`/auth/login`, `/auth/callback`, `/auth/logout`, `/me`). diff --git a/agent/dashboard/thread_api.py b/agent/dashboard/thread_api.py index 64e1957c..da14944c 100644 --- a/agent/dashboard/thread_api.py +++ b/agent/dashboard/thread_api.py @@ -14,12 +14,10 @@ from fastapi import HTTPException from langgraph_sdk.errors import InternalServerError from pydantic import BaseModel, Field -from ..utils.auth import persist_encrypted_github_token from ..utils.thread_ops import is_thread_active, langgraph_client, queue_message_for_thread from .message_adapter import state_messages_to_ui from .options import SUPPORTED_MODEL_IDS, model_supports_effort -from .profiles import OAUTH_TOKENS_NAMESPACE, get_profile, get_valid_access_token -from .profiles import _get_value as get_oauth_record +from .profiles import get_profile, get_valid_access_token from .user_mappings import email_for_login logger = logging.getLogger(__name__) @@ -86,17 +84,10 @@ def _parse_repo(full_name: str | None) -> dict[str, str] | None: return {"owner": owner, "name": name} -async def _persist_dashboard_github_token(thread_id: str, login: str) -> None: +async def _ensure_dashboard_github_token(login: str) -> None: token = await get_valid_access_token(login) if not token: raise HTTPException(401, "github token unavailable, re-login required") - record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, login) - expires_at = record.get("token_expires_at") if isinstance(record, dict) else None - await persist_encrypted_github_token( - thread_id, - token, - expires_at=expires_at if isinstance(expires_at, str) else None, - ) def _thread_owner_login(metadata: dict[str, Any]) -> str | None: @@ -323,7 +314,7 @@ async def _start_agent_run( client = langgraph_client() await client.threads.create(thread_id=thread_id, metadata=metadata, if_exists="do_nothing") await client.threads.update(thread_id=thread_id, metadata=metadata) - await _persist_dashboard_github_token(thread_id, login) + await _ensure_dashboard_github_token(login) configurable: dict[str, Any] = { "thread_id": thread_id, @@ -401,7 +392,7 @@ async def send_dashboard_message( thread if isinstance(thread, dict) else {"thread_id": thread_id, "metadata": metadata} ) - await _persist_dashboard_github_token(thread_id, login) + await _ensure_dashboard_github_token(login) profile = await get_profile(login) or {} thread_source = _thread_source(metadata) configurable: dict[str, Any] = { diff --git a/agent/reviewer.py b/agent/reviewer.py index 915b1f2f..9b0b6509 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -64,7 +64,8 @@ from .tools import ( ) from .utils.agents_md import fetch_agents_md from .utils.auth import resolve_github_token -from .utils.github_token import get_github_token_from_thread +from .utils.github_app import get_github_app_installation_token_with_expiry +from .utils.github_token import cache_github_token_for_thread, get_github_token_from_thread from .utils.model import DEFAULT_LLM_REASONING, make_model, provider_model_kwargs from .utils.sandbox_paths import aresolve_sandbox_work_dir @@ -622,22 +623,24 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: logger.info("No thread_id or not for execution, returning reviewer agent without sandbox") return create_deep_agent(system_prompt="", tools=[]).with_config(config) + repo_config = config["configurable"].get("repo") or {} github_token: str | None = None if config["configurable"].get("source"): - cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread( - thread_id - ) - if cached_token and cached_encrypted: - config["metadata"]["github_token_encrypted"] = cached_encrypted - config["metadata"]["github_token_expires_at"] = cached_expires_at + cached_token, _cached_expires_at = await get_github_token_from_thread(thread_id) + if cached_token: github_token = cached_token else: - _token, new_encrypted, new_expires_at = await resolve_github_token(config, thread_id) - config["metadata"]["github_token_encrypted"] = new_encrypted - config["metadata"]["github_token_expires_at"] = new_expires_at - github_token = _token + try: + github_token, _expires_at = await resolve_github_token(config, thread_id) + except RuntimeError: + github_token, expires_at = await get_github_app_installation_token_with_expiry( + repositories=[str(repo_config.get("name"))] if repo_config.get("name") else None + ) + if github_token: + cache_github_token_for_thread(thread_id, github_token, expires_at=expires_at) + else: + raise - repo_config = config["configurable"].get("repo") or {} repo_private = config["configurable"].get("repo_private") github_proxy_token = github_token if repo_private is False else None sandbox_backend = await ensure_sandbox_for_thread( diff --git a/agent/reviewer_findings.py b/agent/reviewer_findings.py index 0973d487..05b7fd1d 100644 --- a/agent/reviewer_findings.py +++ b/agent/reviewer_findings.py @@ -7,7 +7,7 @@ the reviewer's tools and webhook handlers go through. Why thread metadata: it survives sandbox eviction, is queryable cross-thread via the langgraph SDK (a future UI lists all reviewer threads by filtering on ``metadata.kind == "reviewer"``), and matches existing patterns the codebase -already uses for ``sandbox_id``, ``github_token_encrypted``, etc. +already uses for durable non-secret run state like ``sandbox_id``. """ from __future__ import annotations diff --git a/agent/server.py b/agent/server.py index 12f05093..15667949 100644 --- a/agent/server.py +++ b/agent/server.py @@ -410,9 +410,7 @@ async def get_agent(config: RunnableConfig) -> Pregel: tools=[], ).with_config(config) - github_token, new_encrypted, new_expires_at = await resolve_github_token(config, thread_id) - config["metadata"]["github_token_encrypted"] = new_encrypted - config["metadata"]["github_token_expires_at"] = new_expires_at + github_token, _expires_at = await resolve_github_token(config, thread_id) triggering_user_identity = await asyncio.to_thread( resolve_triggering_user_identity, config, github_token ) diff --git a/agent/utils/auth.py b/agent/utils/auth.py index caba12eb..d35d2643 100644 --- a/agent/utils/auth.py +++ b/agent/utils/auth.py @@ -13,9 +13,8 @@ from langgraph.config import get_config from langgraph.graph.state import RunnableConfig from langgraph_sdk import get_client -from ..encryption import encrypt_token from .github_app import get_github_app_installation_token_with_expiry -from .github_token import get_github_token_from_thread +from .github_token import cache_github_token_for_thread, get_github_token_from_thread from .linear import comment_on_linear_issue from .slack import post_slack_thread_reply @@ -287,24 +286,18 @@ async def leave_failure_comment( raise ValueError(f"Unknown source: {source}") -async def persist_encrypted_github_token( +def _cache_resolved_github_token( thread_id: str, token: str, expires_at: str | None = None -) -> str: - """Encrypt a GitHub token and store it (and its expiry) on the thread metadata.""" - encrypted = encrypt_token(token) - metadata: dict[str, Any] = { - "github_token_encrypted": encrypted, - "github_token_expires_at": expires_at, - } - await client.threads.update(thread_id=thread_id, metadata=metadata) - return encrypted +) -> tuple[str, str | None]: + cache_github_token_for_thread(thread_id, token, expires_at=expires_at) + return token, expires_at -async def save_encrypted_token_from_email( +async def resolve_token_from_email( email: str | None, source: str, -) -> tuple[str, str, str | None]: - """Resolve, encrypt, and store a GitHub token based on user email.""" +) -> tuple[str, str | None]: + """Resolve and cache a GitHub token based on user email.""" config = get_config() configurable = config.get("configurable", {}) thread_id = configurable.get("thread_id") @@ -363,23 +356,15 @@ async def save_encrypted_token_from_email( raise ValueError(f"No token found: {error}") expires_at = auth_result.get("expires_at") if isinstance(auth_result, dict) else None - encrypted = await persist_encrypted_github_token(thread_id, token, expires_at=expires_at) - return token, encrypted, expires_at + return _cache_resolved_github_token( + thread_id, token, expires_at=expires_at if isinstance(expires_at, str) else None + ) async def _resolve_dashboard_user_token( thread_id: str, github_login: str -) -> tuple[str, str, str | None] | None: - """Resolve a per-user GitHub token from the dashboard OAuth store. - - Returns the ``(token, encrypted, expires_at)`` tuple, or ``None`` when the - user has no valid token (never linked, or expired/revoked beyond refresh). - - The thread-metadata token cache is intentionally NOT consulted here: Slack - thread ids are shared across everyone in a conversation, so a cached token - from a prior triggering user would impersonate the current ``github_login``. - We always resolve by login from the dashboard store instead. - """ +) -> tuple[str, str | None] | None: + """Resolve a per-user GitHub token from the dashboard OAuth store.""" login = github_login.strip() if not login: raise ValueError("missing github_login") @@ -392,13 +377,13 @@ async def _resolve_dashboard_user_token( return None record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, login) expires_at = record.get("token_expires_at") if isinstance(record, dict) else None - expires_at = expires_at if isinstance(expires_at, str) else None - encrypted = await persist_encrypted_github_token(thread_id, token, expires_at=expires_at) - return token, encrypted, expires_at + return _cache_resolved_github_token( + thread_id, token, expires_at=expires_at if isinstance(expires_at, str) else None + ) -async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str, str | None]: - """Get a GitHub App installation token and persist it for the thread.""" +async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str | None]: + """Get a GitHub App installation token and cache it for the thread.""" bot_token, expires_at = await get_github_app_installation_token_with_expiry() if not bot_token: raise RuntimeError( @@ -409,13 +394,10 @@ async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str, str logger.info( "Using GitHub App installation token for thread %s (bot-token-only mode)", thread_id ) - encrypted = await persist_encrypted_github_token(thread_id, bot_token, expires_at=expires_at) - return bot_token, encrypted, expires_at + return _cache_resolved_github_token(thread_id, bot_token, expires_at=expires_at) -async def resolve_github_token( - config: RunnableConfig, thread_id: str -) -> tuple[str, str, str | None]: +async def resolve_github_token(config: RunnableConfig, thread_id: str) -> tuple[str, str | None]: """Resolve a GitHub token from the run config based on the source. Routes to the correct auth method depending on whether the run was @@ -425,10 +407,6 @@ async def resolve_github_token( X_SERVICE_AUTH_JWT_SECRET), the GitHub App installation token is used for all operations instead of per-user OAuth tokens. - Returns: - (github_token, new_encrypted, expires_at) tuple. ``expires_at`` is the - ISO-8601 expiry persisted alongside the ciphertext, or ``None``. - Raises: RuntimeError: If source is missing or token resolution fails. """ @@ -462,18 +440,16 @@ async def resolve_github_token( try: if source == "github": - cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread( - thread_id - ) - if cached_token and cached_encrypted: - return cached_token, cached_encrypted, cached_expires_at + cached_token, cached_expires_at = await get_github_token_from_thread(thread_id) + if cached_token: + return cached_token, cached_expires_at from ..dashboard.user_mappings import email_for_login email = await email_for_login(github_login) if not email: raise ValueError(f"No email mapping found for GitHub user '{github_login}'") - return await save_encrypted_token_from_email(email, source) - return await save_encrypted_token_from_email(configurable.get("user_email"), source) + return await resolve_token_from_email(email, source) + return await resolve_token_from_email(configurable.get("user_email"), source) except ValueError as exc: logger.error("GitHub auth failed for thread %s: %s", thread_id, str(exc)) raise RuntimeError(str(exc)) from exc diff --git a/agent/utils/github_token.py b/agent/utils/github_token.py index 3957c3ed..1738bf3c 100644 --- a/agent/utils/github_token.py +++ b/agent/utils/github_token.py @@ -4,51 +4,40 @@ from __future__ import annotations import logging from collections.abc import Mapping -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any from langgraph.config import get_config -from langgraph_sdk import get_client -from langgraph_sdk.errors import NotFoundError - -from ..encryption import decrypt_token logger = logging.getLogger(__name__) -_GITHUB_TOKEN_METADATA_KEY = "github_token_encrypted" -_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY = "github_token_expires_at" +# Treat tokens with <= this many seconds remaining as expired so we re-auth +# before kicking off long agent runs. +_GITHUB_TOKEN_EXPIRY_SKEW_SECONDS = 60 +# Hard cap on how long an entry stays cached regardless of the token's own +# expiry, so entries for threads that are never read again don't accumulate. +_GITHUB_TOKEN_MAX_TTL = timedelta(hours=24) +# thread_id -> (token, token_expires_at, cached_at) +_GITHUB_TOKEN_CACHE: dict[str, tuple[str, str | None, datetime]] = {} class GitHubAuthError(Exception): """Raised when a GitHub call returns 401, signalling a stale/revoked token.""" -# Treat tokens with <= this many seconds remaining as expired so we re-auth -# before kicking off long agent runs. -_GITHUB_TOKEN_EXPIRY_SKEW_SECONDS = 60 - -client = get_client() - - -def _read_encrypted_github_token(metadata: dict[str, Any]) -> str | None: - encrypted_token = metadata.get(_GITHUB_TOKEN_METADATA_KEY) - return encrypted_token if isinstance(encrypted_token, str) and encrypted_token else None - - -def _decrypt_github_token(encrypted_token: str | None) -> str | None: - if not encrypted_token: - return None - - return decrypt_token(encrypted_token) +def cache_github_token_for_thread( + thread_id: str, token: str, expires_at: str | None = None +) -> None: + """Cache a GitHub token in process for the current thread.""" + if not thread_id or not token: + return + now = datetime.now(UTC) + _GITHUB_TOKEN_CACHE[thread_id] = (token, expires_at, now) + _evict_expired(now=now) def _is_expired(expires_at: Any, *, now: datetime | None = None) -> bool: - """Return True when ``expires_at`` is past (or close to) ``now``. - - Accepts ISO-8601 strings (with or without trailing Z) and unix timestamps. - Unparseable values are treated as not expired so we don't break callers - that haven't started persisting an expiry yet. - """ + """Return True when ``expires_at`` is past (or close to) ``now``.""" if expires_at is None: return False @@ -78,76 +67,59 @@ def _is_expired(expires_at: Any, *, now: datetime | None = None) -> bool: return (parsed - current).total_seconds() <= _GITHUB_TOKEN_EXPIRY_SKEW_SECONDS -def _read_token_if_fresh(metadata: dict[str, Any]) -> str | None: - """Decrypt the cached token only if it has not expired.""" - encrypted = _read_encrypted_github_token(metadata) - if not encrypted: +def _entry_expired(expires_at: str | None, cached_at: datetime, *, now: datetime) -> bool: + """Expired when past the token's own expiry or the 24h cache cap.""" + if now - cached_at >= _GITHUB_TOKEN_MAX_TTL: + return True + return _is_expired(expires_at, now=now) + + +def _evict_expired(*, now: datetime | None = None) -> None: + current = now or datetime.now(UTC) + stale = [ + tid + for tid, (_token, expires_at, cached_at) in _GITHUB_TOKEN_CACHE.items() + if _entry_expired(expires_at, cached_at, now=current) + ] + for tid in stale: + _GITHUB_TOKEN_CACHE.pop(tid, None) + + +def _cached_token_if_fresh(thread_id: str | None) -> tuple[str | None, str | None]: + if not thread_id: + return None, None + cached = _GITHUB_TOKEN_CACHE.get(thread_id) + if not cached: + return None, None + token, expires_at, cached_at = cached + if _entry_expired(expires_at, cached_at, now=datetime.now(UTC)): + _GITHUB_TOKEN_CACHE.pop(thread_id, None) + logger.info("Cached GitHub token for thread %s has expired; re-resolving", thread_id) + return None, None + return token, expires_at + + +def _thread_id_from_config(run_config: Mapping[str, Any]) -> str | None: + configurable = run_config.get("configurable", {}) + if not isinstance(configurable, Mapping): return None - if _is_expired(metadata.get(_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY)): - return None - return _decrypt_github_token(encrypted) + thread_id = configurable.get("thread_id") + return thread_id if isinstance(thread_id, str) and thread_id else None def get_github_token(run_config: Mapping[str, Any] | None = None) -> str | None: - """Resolve a GitHub token from run metadata. - - Pass ``run_config`` when LangGraph runnable config is already available (e.g. after - ``get_config()`` in callers). Omit to read from ``get_config()`` (required runnable - context). Returns ``None`` for tokens whose ``github_token_expires_at`` is past. - """ + """Resolve the current thread's GitHub token from process memory.""" resolved = run_config if run_config is not None else get_config() - return _read_token_if_fresh(resolved.get("metadata", {})) + token, _expires_at = _cached_token_if_fresh(_thread_id_from_config(resolved)) + return token -async def get_github_token_from_thread( - thread_id: str, -) -> tuple[str | None, str | None, str | None]: - """Resolve a GitHub token from LangGraph thread metadata. - - Returns ``(None, None, None)`` when no token is cached or when the cached - token's ``github_token_expires_at`` has elapsed — callers must treat the - cache as missing in that case and re-resolve. On a fresh hit, returns the - decrypted token, its ciphertext, and the persisted expiry (or ``None``). - """ - try: - thread = await client.threads.get(thread_id) - except NotFoundError: - logger.debug("Thread %s not found while looking up GitHub token", thread_id) - return None, None, None - except Exception: # noqa: BLE001 - logger.exception("Failed to fetch thread metadata for %s", thread_id) - return None, None, None - - metadata = (thread or {}).get("metadata", {}) - encrypted_token = _read_encrypted_github_token(metadata) - if not encrypted_token: - return None, None, None - expires_at_raw = metadata.get(_GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY) - if _is_expired(expires_at_raw): - logger.info("Cached GitHub token for thread %s has expired; re-resolving", thread_id) - return None, None, None - - token = _decrypt_github_token(encrypted_token) - if token: - logger.info("Found GitHub token in thread metadata for thread %s", thread_id) - expires_at = expires_at_raw if isinstance(expires_at_raw, str) else None - return token, encrypted_token, expires_at +async def get_github_token_from_thread(thread_id: str) -> tuple[str | None, str | None]: + """Resolve the current process's cached GitHub token for a thread.""" + return _cached_token_if_fresh(thread_id) async def invalidate_cached_github_token(thread_id: str) -> None: - """Clear a cached GitHub token from thread metadata. - - Called when a downstream GitHub API call returns 401, so the next run - re-resolves a fresh token instead of replaying the revoked ciphertext. - """ - try: - await client.threads.update( - thread_id=thread_id, - metadata={ - _GITHUB_TOKEN_METADATA_KEY: None, - _GITHUB_TOKEN_EXPIRES_AT_METADATA_KEY: None, - }, - ) - logger.info("Invalidated cached GitHub token for thread %s", thread_id) - except Exception: - logger.exception("Failed to invalidate cached GitHub token for thread %s", thread_id) + """Clear a cached GitHub token for a thread.""" + _GITHUB_TOKEN_CACHE.pop(thread_id, None) + logger.info("Invalidated cached GitHub token for thread %s", thread_id) diff --git a/agent/webapp.py b/agent/webapp.py index 7e12299e..4d70fe13 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -52,7 +52,6 @@ from .reviewer_publish import fetch_pr_review_threads from .reviewer_reconcile import reconcile_findings_with_review_threads from .utils.auth import ( is_bot_token_only_mode, - persist_encrypted_github_token, resolve_github_token_from_email, ) from .utils.comments import get_recent_comments @@ -74,7 +73,11 @@ from .utils.github_comments import ( verify_github_signature, ) from .utils.github_org_membership import INTERNAL_BOT_LOGINS, is_user_active_org_member -from .utils.github_token import get_github_token_from_thread, invalidate_cached_github_token +from .utils.github_token import ( + cache_github_token_for_thread, + get_github_token_from_thread, + invalidate_cached_github_token, +) from .utils.linear import post_linear_trace_comment from .utils.linear_team_repo_map import LINEAR_TEAM_TO_REPO from .utils.multimodal import dedupe_urls, extract_image_urls, fetch_image_block @@ -1725,11 +1728,7 @@ async def trigger_pr_review_from_ref( if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client): return {"success": False, "error": "Could not create reviewer thread"} - try: - await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at) - except Exception: - logger.warning("Could not persist bot token for reviewer thread %s", thread_id) - return {"success": False, "error": "Could not persist reviewer token"} + cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at) pr_meta: ReviewerPRMeta = { "owner": pr_ref.owner, @@ -1919,11 +1918,7 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client): return - try: - await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at) - except Exception: - logger.warning("Could not persist bot token for reviewer thread %s", thread_id) - return + cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at) await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True, head_sha=head_sha) @@ -1986,9 +1981,9 @@ async def process_github_pr_ready(payload: dict[str, Any]) -> None: author_login or "", ) return - # Use source="github" so the auth resolver finds the bot token persisted on - # the thread; "github_auto" would fall through to the email-based path, - # which has no user_email to route on for webhook-triggered runs. + # Use source="github" so the reviewer resolver can use the GitHub App token; + # "github_auto" would fall through to the email-based path, which has no + # user_email to route on for webhook-triggered runs. await _dispatch_first_review_from_pr_payload(payload, source="github") @@ -2267,11 +2262,7 @@ async def process_github_push_event(payload: dict[str, Any]) -> None: langgraph_client = get_client(url=LANGGRAPH_URL) if not await _ensure_thread_exists_for_metadata(thread_id, langgraph_client): return - try: - await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at) - except Exception: - logger.warning("Could not persist bot token for reviewer thread %s", thread_id) - return + cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at) try: threads = await fetch_pr_review_threads( owner=repo_config["owner"], @@ -2341,24 +2332,20 @@ async def _refresh_thread_github_token_after_401(thread_id: str, email: str) -> async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str | None: - """Resolve and persist a GitHub token for a thread when available. + """Resolve and cache a GitHub token for a thread when available. - Skips the cached ciphertext when its ``github_token_expires_at`` is past. In bot-token-only mode, returns a fresh GitHub App installation token instead of resolving per-user OAuth tokens. """ if is_bot_token_only_mode(): bot_token, expires_at = await get_github_app_installation_token_with_expiry() if bot_token: - try: - await persist_encrypted_github_token(thread_id, bot_token, expires_at=expires_at) - except Exception: - logger.warning("Could not persist bot token for thread %s", thread_id) + cache_github_token_for_thread(thread_id, bot_token, expires_at=expires_at) return bot_token logger.warning("Bot-token-only mode but GitHub App token unavailable") return None - github_token, _encrypted_token, _expires_at = await get_github_token_from_thread(thread_id) + github_token, _expires_at = await get_github_token_from_thread(thread_id) if github_token: return github_token @@ -2367,12 +2354,10 @@ async def _get_or_resolve_thread_github_token(thread_id: str, email: str) -> str if not github_token: return None - try: - await persist_encrypted_github_token( - thread_id, github_token, expires_at=auth_result.get("expires_at") - ) - except Exception: - logger.warning("Could not persist GitHub token for thread %s", thread_id) + expires_at = auth_result.get("expires_at") + cache_github_token_for_thread( + thread_id, github_token, expires_at=expires_at if isinstance(expires_at, str) else None + ) return github_token @@ -2586,11 +2571,7 @@ async def process_github_review_finding_reply(payload: dict[str, Any]) -> None: ) if not app_token: return - try: - await persist_encrypted_github_token(thread_id, app_token, expires_at=app_token_expires_at) - except Exception: - logger.warning("Could not persist bot token for reviewer thread %s", thread_id) - return + cache_github_token_for_thread(thread_id, app_token, expires_at=app_token_expires_at) threads = await fetch_pr_review_threads( owner=repo_config["owner"], diff --git a/tests/test_agent_subagent_models.py b/tests/test_agent_subagent_models.py index 8af29244..f6f6d3c5 100644 --- a/tests/test_agent_subagent_models.py +++ b/tests/test_agent_subagent_models.py @@ -34,7 +34,7 @@ async def test_agent_uses_profile_subagent_model_override() -> None: patch( "agent.server.resolve_github_token", new_callable=AsyncMock, - return_value=("ghp", "enc", None), + return_value=("ghp", None), ), patch("agent.server.resolve_triggering_user_identity", return_value=None), patch( @@ -112,7 +112,7 @@ async def test_agent_subagent_inherits_profile_model_override_without_explicit_p patch( "agent.server.resolve_github_token", new_callable=AsyncMock, - return_value=("ghp", "enc", None), + return_value=("ghp", None), ), patch("agent.server.resolve_triggering_user_identity", return_value=None), patch( diff --git a/tests/test_auth_sources.py b/tests/test_auth_sources.py index 23b05881..9ae658aa 100644 --- a/tests/test_auth_sources.py +++ b/tests/test_auth_sources.py @@ -60,7 +60,7 @@ def _stub_dashboard_store( *, token: str | None, expires_at: str | None = "2099-01-01T00:00:00Z", - cached: tuple[str | None, str | None, str | None] = (None, None, None), + cached: tuple[str | None, str | None] = (None, None), ) -> None: from agent.dashboard import profiles @@ -73,11 +73,7 @@ def _stub_dashboard_store( async def fake_get_value(namespace, key): return {"token_expires_at": expires_at} - async def fake_persist(thread_id: str, tok: str, expires_at: str | None = None): - return "enc" - monkeypatch.setattr(auth, "get_github_token_from_thread", fake_get_from_thread) - monkeypatch.setattr(auth, "persist_encrypted_github_token", fake_persist) monkeypatch.setattr(profiles, "get_valid_access_token", fake_get_valid) monkeypatch.setattr(profiles, "_get_value", fake_get_value) @@ -88,10 +84,9 @@ def test_resolve_github_token_slack_uses_dashboard_store( _stub_dashboard_store(monkeypatch, token="user-tok") monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False) - token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + token, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) assert token == "user-tok" - assert encrypted == "enc" assert expires_at == "2099-01-01T00:00:00Z" @@ -103,11 +98,11 @@ def test_resolve_github_token_slack_ignores_stale_thread_cache( _stub_dashboard_store( monkeypatch, token="bob-token", - cached=("alice-token", "alice-enc", "2099-01-01T00:00:00Z"), + cached=("alice-token", "2099-01-01T00:00:00Z"), ) monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False) - token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + token, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) assert token == "bob-token" @@ -133,7 +128,7 @@ def test_resolve_github_token_per_user_wins_over_bot_only_mode( monkeypatch.setattr(auth, "_resolve_bot_installation_token", fail_bot) - token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + token, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) assert token == "user-tok" @@ -144,12 +139,12 @@ def test_resolve_github_token_slack_no_token_falls_back_to_bot_in_bot_only_mode( monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True) async def fake_bot(thread_id: str): - return ("bot-tok", "bot-enc", None) + return ("bot-tok", None) monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot) - token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) - assert (token, encrypted, expires_at) == ("bot-tok", "bot-enc", None) + token, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + assert (token, expires_at) == ("bot-tok", None) @pytest.mark.parametrize("source", ["github", "linear"]) @@ -159,10 +154,10 @@ def test_resolve_github_token_bot_only_mode_non_slack_uses_bot( monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True) async def fake_bot(thread_id: str): - return ("bot-tok", "bot-enc", None) + return ("bot-tok", None) monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot) config = {"configurable": {"source": source, "github_login": "octo", "thread_id": "t1"}} - token, _, _ = asyncio.run(auth.resolve_github_token(config, "t1")) + token, _ = asyncio.run(auth.resolve_github_token(config, "t1")) assert token == "bot-tok" diff --git a/tests/test_github_issue_webhook.py b/tests/test_github_issue_webhook.py index da6cb868..867c9adf 100644 --- a/tests/test_github_issue_webhook.py +++ b/tests/test_github_issue_webhook.py @@ -305,11 +305,8 @@ def test_process_github_review_finding_reply_uses_rereview_config(monkeypatch) - async def fake_get_token_with_expiry() -> tuple[str, str]: return "app-token", "2026-01-01T00:00:00Z" - async def fake_persist_token( - thread_id: str, token: str, *, expires_at: str | None = None - ) -> str: - captured["persist"] = (thread_id, token, expires_at) - return "encrypted" + def fake_cache_token(thread_id: str, token: str, *, expires_at: str | None = None) -> None: + captured["cache"] = (thread_id, token, expires_at) async def fake_fetch_threads(**_kwargs: object) -> list[dict[str, object]]: return [] @@ -346,7 +343,7 @@ def test_process_github_review_finding_reply_uses_rereview_config(monkeypatch) - monkeypatch.setattr( webapp, "get_github_app_installation_token_with_expiry", fake_get_token_with_expiry ) - monkeypatch.setattr(webapp, "persist_encrypted_github_token", fake_persist_token) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_token) monkeypatch.setattr(webapp, "fetch_pr_review_threads", fake_fetch_threads) monkeypatch.setattr(webapp, "reconcile_findings_with_review_threads", fake_reconcile) monkeypatch.setattr(webapp, "list_reviewer_findings", fake_list_findings) @@ -393,11 +390,8 @@ def test_process_github_review_finding_reply_queues_reply_body_when_active(monke async def fake_get_token_with_expiry() -> tuple[str, str]: return "app-token", "2026-01-01T00:00:00Z" - async def fake_persist_token( - _thread_id: str, _token: str, *, expires_at: str | None = None - ) -> str: + def fake_cache_token(_thread_id: str, _token: str, *, expires_at: str | None = None) -> None: captured["expires_at"] = expires_at - return "encrypted" async def fake_fetch_threads(**_kwargs: object) -> list[dict[str, object]]: return [] @@ -427,7 +421,7 @@ def test_process_github_review_finding_reply_queues_reply_body_when_active(monke monkeypatch.setattr( webapp, "get_github_app_installation_token_with_expiry", fake_get_token_with_expiry ) - monkeypatch.setattr(webapp, "persist_encrypted_github_token", fake_persist_token) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_token) monkeypatch.setattr(webapp, "fetch_pr_review_threads", fake_fetch_threads) monkeypatch.setattr(webapp, "reconcile_findings_with_review_threads", fake_reconcile) monkeypatch.setattr(webapp, "list_reviewer_findings", fake_list_findings) @@ -739,13 +733,12 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None: async def fake_get_github_app_installation_token_with_expiry() -> tuple[str | None, str | None]: return "app-token", None - async def fake_persist_encrypted_github_token( + def fake_cache_github_token( thread_id: str, token: str, *, expires_at: str | None = None - ) -> str: - captured["persist_thread_id"] = thread_id - captured["persist_token"] = token - captured["persist_expires_at"] = expires_at - return "encrypted-token" + ) -> None: + captured["cache_thread_id"] = thread_id + captured["cache_token"] = token + captured["cache_expires_at"] = expires_at async def fake_is_thread_active(thread_id: str) -> bool: captured["active_thread_id"] = thread_id @@ -774,9 +767,7 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None: "get_github_app_installation_token_with_expiry", fake_get_github_app_installation_token_with_expiry, ) - monkeypatch.setattr( - webapp, "persist_encrypted_github_token", fake_persist_encrypted_github_token - ) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_github_token) monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active) monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", fake_set_reviewer_thread_metadata) monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClient()) @@ -806,8 +797,8 @@ def test_process_github_pr_ready_creates_reviewer_run(monkeypatch) -> None: "thread_id": captured["thread_id"], "if_exists": "do_nothing", } - assert captured["persist_token"] == "app-token" - assert captured["persist_thread_id"] == captured["thread_id"] + assert captured["cache_token"] == "app-token" + assert captured["cache_thread_id"] == captured["thread_id"] assert "https://github.com/langchain-ai/open-swe/pull/1244" in prompt assert "Base SHA: base-sha" in prompt assert "Head SHA: head-sha" in prompt @@ -836,13 +827,12 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: "head": {"sha": "head-sha", "ref": "feature-branch"}, } - async def fake_persist_encrypted_github_token( + def fake_cache_github_token( thread_id: str, token: str, *, expires_at: str | None = None - ) -> str: - captured["persist_thread_id"] = thread_id - captured["persist_token"] = token - captured["persist_expires_at"] = expires_at - return "encrypted-token" + ) -> None: + captured["cache_thread_id"] = thread_id + captured["cache_token"] = token + captured["cache_expires_at"] = expires_at async def fake_is_thread_active(thread_id: str) -> bool: captured["active_thread_id"] = thread_id @@ -875,9 +865,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: fake_get_github_app_installation_token_with_expiry, ) monkeypatch.setattr(webapp, "fetch_github_pr_metadata", fake_fetch_github_pr_metadata) - monkeypatch.setattr( - webapp, "persist_encrypted_github_token", fake_persist_encrypted_github_token - ) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", fake_cache_github_token) monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active) monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", fake_set_reviewer_thread_metadata) monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClient()) @@ -906,7 +894,7 @@ def test_trigger_pr_review_from_ref_creates_reviewer_run(monkeypatch) -> None: "if_exists": "do_nothing", } assert captured["metadata_token"] == "app-token" - assert captured["persist_token"] == "app-token" + assert captured["cache_token"] == "app-token" assert "Base SHA: base-sha" in prompt assert "Head SHA: head-sha" in prompt assert config["source"] == "slack" diff --git a/tests/test_github_token_ttl.py b/tests/test_github_token_ttl.py index 70ba4022..bd3ee158 100644 --- a/tests/test_github_token_ttl.py +++ b/tests/test_github_token_ttl.py @@ -2,7 +2,7 @@ Covers: - (a) expired-cache reads return None / fall through to re-auth -- (b) 401 on a downstream GitHub call invalidates the cached ciphertext and +- (b) 401 on a downstream GitHub call invalidates the cached token and triggers a fresh resolve in the webapp - (c) ``publish_review`` invalidates the cached token and returns a clean failure when GitHub responds 401 @@ -13,25 +13,16 @@ from __future__ import annotations import asyncio from datetime import UTC, datetime, timedelta from typing import Any -from unittest.mock import AsyncMock, patch import httpx import pytest from agent.utils import github_comments, github_token -_TEST_FERNET_KEY = "GMI8FNqVnhFzVfKDUTpGAUq8a2cm14kU0SyXzMTM4Yc=" - @pytest.fixture(autouse=True) -def _set_encryption_key(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("TOKEN_ENCRYPTION_KEY", _TEST_FERNET_KEY) - - -def _encrypted(token: str) -> str: - from agent.encryption import encrypt_token - - return encrypt_token(token) +def _clear_token_cache() -> None: + github_token._GITHUB_TOKEN_CACHE.clear() # (a) expired-cache reads ----------------------------------------------------- @@ -57,73 +48,65 @@ def test_is_expired_treats_unparseable_as_not_expired() -> None: assert github_token._is_expired("not-a-date") is False -def test_get_github_token_returns_none_for_expired_run_metadata() -> None: +def test_get_github_token_returns_none_for_expired_cache() -> None: past = (datetime.now(UTC) - timedelta(hours=1)).isoformat() - metadata = { - "github_token_encrypted": _encrypted("ghp_secret"), - "github_token_expires_at": past, - } - assert github_token.get_github_token({"metadata": metadata}) is None + github_token.cache_github_token_for_thread("tid", "ghp_secret", expires_at=past) + assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) is None -def test_get_github_token_returns_decrypted_for_fresh_metadata() -> None: +def test_get_github_token_returns_fresh_cached_token() -> None: future = (datetime.now(UTC) + timedelta(hours=1)).isoformat() - metadata = { - "github_token_encrypted": _encrypted("ghp_secret"), - "github_token_expires_at": future, - } - assert github_token.get_github_token({"metadata": metadata}) == "ghp_secret" + github_token.cache_github_token_for_thread("tid", "ghp_secret", expires_at=future) + assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) == "ghp_secret" -def test_get_github_token_returns_decrypted_when_no_expires_at() -> None: - metadata = {"github_token_encrypted": _encrypted("ghp_secret")} - assert github_token.get_github_token({"metadata": metadata}) == "ghp_secret" +def test_get_github_token_returns_cached_token_when_no_expires_at() -> None: + github_token.cache_github_token_for_thread("tid", "ghp_secret") + assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) == "ghp_secret" + + +def test_cached_token_expires_after_max_ttl() -> None: + """A token with no/far expiry is still dropped once it's older than the 24h cap.""" + far_future = (datetime.now(UTC) + timedelta(days=30)).isoformat() + old_cached_at = datetime.now(UTC) - timedelta(hours=25) + github_token._GITHUB_TOKEN_CACHE["tid"] = ("ghp_secret", far_future, old_cached_at) + assert github_token.get_github_token({"configurable": {"thread_id": "tid"}}) is None + + +def test_cache_write_sweeps_other_expired_entries() -> None: + """Writing one entry evicts unrelated entries that have passed their expiry.""" + past = (datetime.now(UTC) - timedelta(hours=1)).isoformat() + github_token.cache_github_token_for_thread("stale", "ghp_stale", expires_at=past) + github_token.cache_github_token_for_thread("fresh", "ghp_fresh") + assert "stale" not in github_token._GITHUB_TOKEN_CACHE + assert "fresh" in github_token._GITHUB_TOKEN_CACHE @pytest.mark.asyncio async def test_get_github_token_from_thread_skips_expired() -> None: past = (datetime.now(UTC) - timedelta(hours=1)).isoformat() - fake_client = AsyncMock() - fake_client.threads.get.return_value = { - "metadata": { - "github_token_encrypted": _encrypted("ghp_revoked"), - "github_token_expires_at": past, - } - } - with patch.object(github_token, "client", fake_client): - token, encrypted, expires_at = await github_token.get_github_token_from_thread("tid") + github_token.cache_github_token_for_thread("tid", "ghp_revoked", expires_at=past) + token, expires_at = await github_token.get_github_token_from_thread("tid") assert token is None - assert encrypted is None assert expires_at is None @pytest.mark.asyncio async def test_get_github_token_from_thread_returns_fresh() -> None: future = (datetime.now(UTC) + timedelta(hours=1)).isoformat() - enc = _encrypted("ghp_live") - fake_client = AsyncMock() - fake_client.threads.get.return_value = { - "metadata": { - "github_token_encrypted": enc, - "github_token_expires_at": future, - } - } - with patch.object(github_token, "client", fake_client): - token, encrypted, expires_at = await github_token.get_github_token_from_thread("tid") + github_token.cache_github_token_for_thread("tid", "ghp_live", expires_at=future) + token, expires_at = await github_token.get_github_token_from_thread("tid") assert token == "ghp_live" - assert encrypted == enc assert expires_at == future @pytest.mark.asyncio -async def test_invalidate_cached_github_token_clears_metadata() -> None: - fake_client = AsyncMock() - with patch.object(github_token, "client", fake_client): - await github_token.invalidate_cached_github_token("tid-42") - fake_client.threads.update.assert_awaited_once_with( - thread_id="tid-42", - metadata={"github_token_encrypted": None, "github_token_expires_at": None}, - ) +async def test_invalidate_cached_github_token_clears_cache() -> None: + github_token.cache_github_token_for_thread("tid-42", "ghp_live") + await github_token.invalidate_cached_github_token("tid-42") + token, expires_at = await github_token.get_github_token_from_thread("tid-42") + assert token is None + assert expires_at is None # (b) 401 on a downstream GitHub call ----------------------------------------- diff --git a/tests/test_pr_ready_auto_review.py b/tests/test_pr_ready_auto_review.py index a3fbbc4a..2cb3be96 100644 --- a/tests/test_pr_ready_auto_review.py +++ b/tests/test_pr_ready_auto_review.py @@ -43,7 +43,7 @@ def _patch_dispatch_deps(monkeypatch: pytest.MonkeyPatch, fake_client: Any) -> N AsyncMock(return_value=("token", None)), ) monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True)) - monkeypatch.setattr(webapp, "persist_encrypted_github_token", AsyncMock(return_value="enc")) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", MagicMock()) monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock()) monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False)) monkeypatch.setattr(webapp, "get_client", lambda url: fake_client) @@ -74,8 +74,8 @@ async def test_pr_ready_public_repo_uses_scoped_reviewer_token( get_token = AsyncMock(return_value=("scoped-token", "expires")) monkeypatch.setattr(webapp, "get_github_app_installation_token_with_expiry", get_token) monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True)) - persist_token = AsyncMock(return_value="enc") - monkeypatch.setattr(webapp, "persist_encrypted_github_token", persist_token) + cache_token = MagicMock() + monkeypatch.setattr(webapp, "cache_github_token_for_thread", cache_token) monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock()) monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False)) monkeypatch.setattr(webapp, "get_client", lambda url: fake_client) @@ -85,8 +85,8 @@ async def test_pr_ready_public_repo_uses_scoped_reviewer_token( await webapp.process_github_pr_ready(_pr_payload(action="opened", draft=False, private=False)) get_token.assert_awaited_once_with(repository_ids=[123]) - persist_token.assert_awaited_once() - assert persist_token.await_args.args[1] == "scoped-token" + cache_token.assert_called_once() + assert cache_token.call_args.args[1] == "scoped-token" _, kwargs = fake_client.runs.create.await_args assert kwargs["config"]["configurable"]["repo_private"] is False @@ -100,7 +100,7 @@ async def test_pr_ready_private_repo_uses_full_reviewer_token( get_token = AsyncMock(return_value=("full-token", "expires")) monkeypatch.setattr(webapp, "get_github_app_installation_token_with_expiry", get_token) monkeypatch.setattr(webapp, "_ensure_thread_exists_for_metadata", AsyncMock(return_value=True)) - monkeypatch.setattr(webapp, "persist_encrypted_github_token", AsyncMock(return_value="enc")) + monkeypatch.setattr(webapp, "cache_github_token_for_thread", MagicMock()) monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", AsyncMock()) monkeypatch.setattr(webapp, "is_thread_active", AsyncMock(return_value=False)) monkeypatch.setattr(webapp, "get_client", lambda url: fake_client) diff --git a/tests/test_proxy_auth.py b/tests/test_proxy_auth.py index dfc40fb3..45c46b8d 100644 --- a/tests/test_proxy_auth.py +++ b/tests/test_proxy_auth.py @@ -209,7 +209,7 @@ class TestRefreshProxyOnSandboxReuse: patch( "agent.server.resolve_github_token", new_callable=AsyncMock, - return_value=("ghp", "enc", None), + return_value=("ghp", None), ), patch( "agent.server.get_sandbox_id_from_metadata", @@ -258,7 +258,7 @@ class TestRefreshProxyOnSandboxReuse: patch( "agent.server.resolve_github_token", new_callable=AsyncMock, - return_value=("ghp", "enc", None), + return_value=("ghp", None), ), patch( "agent.server.get_sandbox_id_from_metadata", diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py index 0a56397e..ed617510 100644 --- a/tests/test_reviewer.py +++ b/tests/test_reviewer.py @@ -75,7 +75,7 @@ async def test_reviewer_uses_cached_thread_token_for_slack_review_request() -> N patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("app-token", "encrypted-token", None), + return_value=("app-token", None), ) as mock_get_thread_token, patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock) as mock_resolve_token, patch( @@ -95,7 +95,7 @@ async def test_reviewer_uses_cached_thread_token_for_slack_review_request() -> N metadata = config["metadata"] assert isinstance(metadata, dict) - assert metadata["github_token_encrypted"] == "encrypted-token" + assert "github_token_encrypted" not in metadata mock_get_thread_token.assert_awaited_once_with("reviewer-thread-id") mock_resolve_token.assert_not_called() middleware = create_agent.call_args.kwargs["middleware"] @@ -290,7 +290,7 @@ async def test_reviewer_inlines_agents_md_into_system_prompt() -> None: patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -650,7 +650,7 @@ async def test_reviewer_injects_pr_review_threads_into_first_review_context() -> patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -722,7 +722,7 @@ async def test_reviewer_injects_pr_review_threads_into_re_review_context() -> No patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -786,7 +786,7 @@ async def test_reviewer_omits_threads_block_when_fetch_returns_empty() -> None: patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -846,7 +846,7 @@ async def test_reviewer_continues_when_thread_fetch_raises() -> None: patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -914,7 +914,7 @@ async def test_reviewer_populates_diff_line_set_from_github_api() -> None: patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -980,7 +980,7 @@ async def test_reviewer_leaves_validation_disabled_when_diff_fetch_fails() -> No patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( @@ -1042,7 +1042,7 @@ async def test_reviewer_injects_pr_title_and_body_into_context() -> None: patch( "agent.reviewer.get_github_token_from_thread", new_callable=AsyncMock, - return_value=("gh-token", "encrypted-token", None), + return_value=("gh-token", None), ), patch("agent.reviewer.resolve_github_token", new_callable=AsyncMock), patch( diff --git a/tests/test_reviewer_watch.py b/tests/test_reviewer_watch.py index 3a04ba3b..014369cc 100644 --- a/tests/test_reviewer_watch.py +++ b/tests/test_reviewer_watch.py @@ -188,11 +188,7 @@ async def test_push_event_queues_when_thread_active_even_if_pr_diff_unchanged() new_callable=AsyncMock, return_value=True, ), - patch( - "agent.webapp.persist_encrypted_github_token", - new_callable=AsyncMock, - return_value="enc", - ), + patch("agent.webapp.cache_github_token_for_thread"), patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock), patch("agent.webapp.is_thread_active", new_callable=AsyncMock, return_value=True), patch("agent.webapp.queue_message_for_thread", new=queue_message), @@ -257,11 +253,7 @@ async def test_push_event_triggers_re_review_run_when_watching() -> None: new_callable=AsyncMock, return_value=True, ), - patch( - "agent.webapp.persist_encrypted_github_token", - new_callable=AsyncMock, - return_value="enc", - ), + patch("agent.webapp.cache_github_token_for_thread"), patch( "agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock, @@ -384,7 +376,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None: fake_client = MagicMock() fake_client.runs.create = AsyncMock() get_token = AsyncMock(return_value=("scoped-token", "exp")) - persist = AsyncMock(return_value="enc") + cache_token = MagicMock() with ( patch( @@ -402,7 +394,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None: new_callable=AsyncMock, return_value=True, ), - patch("agent.webapp.persist_encrypted_github_token", persist), + patch("agent.webapp.cache_github_token_for_thread", cache_token), patch("agent.webapp.fetch_pr_review_threads", new_callable=AsyncMock, return_value=[]), patch("agent.webapp.reconcile_findings_with_review_threads", new_callable=AsyncMock), patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock), @@ -412,7 +404,7 @@ async def test_push_event_public_repo_uses_scoped_token() -> None: await webapp.process_github_push_event(payload) get_token.assert_awaited_once_with(repository_ids=[123]) - assert persist.await_args.args[1] == "scoped-token" + assert cache_token.call_args.args[1] == "scoped-token" _, kwargs = fake_client.runs.create.await_args assert kwargs["config"]["configurable"]["repo_private"] is False @@ -430,7 +422,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No fake_client = MagicMock() fake_client.runs.create = AsyncMock() get_token = AsyncMock(side_effect=[("full-token", "e1"), ("scoped-token", "e2")]) - persist = AsyncMock(return_value="enc") + cache_token = MagicMock() with ( patch( @@ -448,7 +440,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No new_callable=AsyncMock, return_value=True, ), - patch("agent.webapp.persist_encrypted_github_token", persist), + patch("agent.webapp.cache_github_token_for_thread", cache_token), patch("agent.webapp.fetch_pr_review_threads", new_callable=AsyncMock, return_value=[]), patch("agent.webapp.reconcile_findings_with_review_threads", new_callable=AsyncMock), patch("agent.webapp.set_reviewer_thread_metadata", new_callable=AsyncMock), @@ -458,7 +450,7 @@ async def test_push_event_rescopes_token_when_pr_metadata_reveals_public() -> No await webapp.process_github_push_event(payload) assert get_token.await_args_list == [call(), call(repository_ids=[456])] - assert persist.await_args.args[1] == "scoped-token" + assert cache_token.call_args.args[1] == "scoped-token" _, kwargs = fake_client.runs.create.await_args assert kwargs["config"]["configurable"]["repo_private"] is False