diff --git a/AGENTS.md b/AGENTS.md index a022b123..1e16e876 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -88,7 +88,7 @@ There is intentionally no after-agent safety net that opens a PR for the agent. All tools live in `agent/tools/` and are flat-imported via `agent/tools/__init__.py`. The set is intentionally small and curated — see README "Tools — Curated, Not Accumulated". Wired into `get_agent`: -`http_request`, `fetch_url`, `web_search`, `linear_comment`, `linear_create_issue`, `linear_delete_issue`, `linear_get_issue`, `linear_get_issue_comments`, `linear_list_teams`, `linear_update_issue`, `jira_comment`, `jira_create_issue`, `jira_get_issue`, `jira_get_issue_comments`, `jira_list_projects`, `jira_update_issue`, `confluence_get_page`, `confluence_create_page`, `confluence_update_page`, `confluence_comment`, `confluence_search`, `request_pr_review`, `schedule_thread_wakeup`, `slack_add_reaction`, `slack_read_thread_messages`, `slack_thread_reply`. +`http_request`, `fetch_url`, `web_search`, `linear_comment`, `linear_create_issue`, `linear_delete_issue`, `linear_get_issue`, `linear_get_issue_comments`, `linear_list_teams`, `linear_search_issues`, `linear_update_issue`, `jira_comment`, `jira_create_issue`, `jira_get_issue`, `jira_get_issue_comments`, `jira_list_projects`, `jira_update_issue`, `confluence_get_page`, `confluence_create_page`, `confluence_update_page`, `confluence_comment`, `confluence_search`, `request_pr_review`, `schedule_thread_wakeup`, `slack_add_reaction`, `slack_read_thread_messages`, `slack_thread_reply`. Reviewer-only tools (in `agent/reviewer.py`): `add_finding`, `update_finding`, `list_findings`, `publish_review`. The review-style analyzer uses `save_review_style` (exported as `save_review_style_prompt`). diff --git a/CLAUDE.md b/CLAUDE.md index d5d541fa..6afe44f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +89,7 @@ There is intentionally no after-agent safety net that opens a PR for the agent. All tools live in `agent/tools/` and are flat-imported via `agent/tools/__init__.py`. The set is intentionally small and curated — see README "Tools — Curated, Not Accumulated". Wired into `get_agent`: -`http_request`, `fetch_url`, `web_search`, `linear_comment`, `linear_create_issue`, `linear_delete_issue`, `linear_get_issue`, `linear_get_issue_comments`, `linear_list_teams`, `linear_update_issue`, `jira_comment`, `jira_create_issue`, `jira_get_issue`, `jira_get_issue_comments`, `jira_list_projects`, `jira_update_issue`, `confluence_get_page`, `confluence_create_page`, `confluence_update_page`, `confluence_comment`, `confluence_search`, `request_pr_review`, `schedule_thread_wakeup`, `slack_add_reaction`, `slack_read_thread_messages`, `slack_thread_reply`. +`http_request`, `fetch_url`, `web_search`, `linear_comment`, `linear_create_issue`, `linear_delete_issue`, `linear_get_issue`, `linear_get_issue_comments`, `linear_list_teams`, `linear_search_issues`, `linear_update_issue`, `jira_comment`, `jira_create_issue`, `jira_get_issue`, `jira_get_issue_comments`, `jira_list_projects`, `jira_update_issue`, `confluence_get_page`, `confluence_create_page`, `confluence_update_page`, `confluence_comment`, `confluence_search`, `request_pr_review`, `schedule_thread_wakeup`, `slack_add_reaction`, `slack_read_thread_messages`, `slack_thread_reply`. Jira uses a service-account REST client (`agent/utils/jira.py`, Basic auth) with ADF↔markdown conversion (`agent/utils/adf.py`); Confluence likewise (`agent/utils/confluence.py`, XHTML storage-format). Both are dark-safe: unset env returns a clean error. diff --git a/README.md b/README.md index 8232d10c..9332a968 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,7 @@ Stripe's key insight: *tool curation matters more than tool quantity.* Open SWE | `fetch_url` | Fetch web pages as markdown | | `http_request` | API calls (GET, POST, etc.) | | `linear_comment` | Post updates to Linear tickets | +| `linear_search_issues` | Search Linear issues by free text | | `jira_*` | Read/comment/create/update Jira issues | | `confluence_*` | Read/write Confluence pages + comments | | `slack_add_reaction` | React to Slack messages | diff --git a/agent/chat.py b/agent/chat.py index 45dae8a4..4798407c 100644 --- a/agent/chat.py +++ b/agent/chat.py @@ -115,8 +115,10 @@ async def _resolve_chat_model(configurable: dict) -> tuple[str, str]: async def get_chat_agent(config: RunnableConfig) -> Pregel: """Get a read-only PR chat agent. No sandbox; PR context comes via config.""" + config = config.copy() + config["configurable"] = config["configurable"].copy() + config.setdefault("recursion_limit", DEFAULT_RECURSION_LIMIT) thread_id = config["configurable"].get("thread_id") - config["recursion_limit"] = DEFAULT_RECURSION_LIMIT if thread_id is None or not graph_loaded_for_execution(config): return create_deep_agent(system_prompt="", tools=[]).with_config(config) diff --git a/agent/dashboard/repo_snapshots.py b/agent/dashboard/repo_snapshots.py index e35329f0..e2afb401 100644 --- a/agent/dashboard/repo_snapshots.py +++ b/agent/dashboard/repo_snapshots.py @@ -342,9 +342,9 @@ def _build_snapshot_sync(record: dict[str, Any], snapshot_name: str) -> tuple[st """ from langsmith.sandbox import SandboxClient - from agent.integrations.langsmith import _get_langsmith_api_key + from agent.integrations.langsmith import _get_sandbox_api_endpoint, _get_sandbox_api_key - api_key = _get_langsmith_api_key() + api_key = _get_sandbox_api_key() if not api_key: raise RuntimeError("LANGSMITH_API_KEY is not configured") @@ -356,7 +356,7 @@ def _build_snapshot_sync(record: dict[str, Any], snapshot_name: str) -> tuple[st timeout = int( os.environ.get("REPO_SNAPSHOT_BUILD_TIMEOUT_SECONDS", DEFAULT_BUILD_TIMEOUT_SECONDS) ) - client = SandboxClient(api_key=api_key) + client = SandboxClient(api_key=api_key, api_endpoint=_get_sandbox_api_endpoint()) try: with tempfile.TemporaryDirectory(prefix="openswe-snapshot-") as context_dir: dockerfile_path = Path(context_dir) / "Dockerfile" diff --git a/agent/integrations/langsmith.py b/agent/integrations/langsmith.py index e8adc694..4e045954 100644 --- a/agent/integrations/langsmith.py +++ b/agent/integrations/langsmith.py @@ -4,9 +4,11 @@ from __future__ import annotations import asyncio import base64 +import json import logging import os import time +import uuid from abc import ABC, abstractmethod from concurrent.futures import ThreadPoolExecutor from concurrent.futures import TimeoutError as FuturesTimeout @@ -44,6 +46,69 @@ def _get_langsmith_api_key() -> str | None: return os.environ.get("LANGSMITH_API_KEY") or os.environ.get("LANGSMITH_API_KEY_PROD") +def _get_sandbox_api_key() -> str | None: + """LangSmith API key for sandbox operations. + + ``SANDBOX_LANGSMITH_API_KEY`` lets sandboxes run against a different + LangSmith workspace than the one used for tracing/other API calls; falls + back to the standard key. + """ + return os.environ.get("SANDBOX_LANGSMITH_API_KEY") or _get_langsmith_api_key() + + +def _get_sandbox_endpoint() -> str: + """LangSmith API **root** for sandbox operations. + + Overridable via ``SANDBOX_LANGSMITH_ENDPOINT`` to pair with + ``SANDBOX_LANGSMITH_API_KEY``; falls back to ``LANGSMITH_ENDPOINT``. This is + the bare root (e.g. ``https://api.smith.langchain.com``) used to build the + proxy-config URL; the SDK clients take :func:`_get_sandbox_api_endpoint`. + """ + return ( + os.environ.get("SANDBOX_LANGSMITH_ENDPOINT") + or os.environ.get("LANGSMITH_ENDPOINT") + or "https://api.smith.langchain.com" + ) + + +def _get_sandbox_api_endpoint() -> str: + """Sandbox API base URL for the langsmith SDK clients. + + The SDK's ``api_endpoint`` is the sandbox base (root + ``/v2/sandboxes``), + not the API root, and its methods append ``/boxes``, ``/snapshots``, etc. + """ + root = _get_sandbox_endpoint().rstrip("/") + suffix = "/v2/sandboxes" + return root if root.endswith(suffix) else f"{root}{suffix}" + + +def _current_thread_id() -> str | None: + """The LangGraph thread id for the active run, if any.""" + try: + from langgraph.config import get_config + + return get_config().get("configurable", {}).get("thread_id") + except Exception: + return None + + +def _sandbox_name_for_thread(thread_id: str | None) -> str | None: + """Deterministic, thread-traceable sandbox name: ``openswe-``. + + The thread id (a UUID) is base32-encoded lowercase without padding so the + name is a compact, hyphen-free token that maps back to the thread. Returns + None when the thread id is missing or not a UUID, leaving the name unset. + """ + if not thread_id: + return None + try: + raw = uuid.UUID(thread_id).bytes + except ValueError: + return None + encoded = base64.b32encode(raw).decode("ascii").rstrip("=").lower() + return f"openswe-{encoded}" + + def _parse_optional_int(name: str, default: int) -> int: raw = os.environ.get(name) if not raw: @@ -87,6 +152,43 @@ def _get_sandbox_snapshot_config() -> tuple[str | None, int, int, int, int, int] ) +def _get_sandbox_create_extra_fields() -> dict[str, Any]: + """Parse SANDBOX_CREATE_EXTRA_JSON into extra fields merged into the + sandbox-create request body, e.g. ``{"_internal_runtime": "v2"}``.""" + raw = os.environ.get("SANDBOX_CREATE_EXTRA_JSON") + if not raw or not raw.strip(): + return {} + try: + parsed = json.loads(raw) + except json.JSONDecodeError as e: + msg = f"SANDBOX_CREATE_EXTRA_JSON must be valid JSON, got {raw!r}" + raise ValueError(msg) from e + if not isinstance(parsed, dict): + msg = f"SANDBOX_CREATE_EXTRA_JSON must be a JSON object, got {type(parsed).__name__}" + raise ValueError(msg) + return parsed + + +def _install_create_extra_fields(client: SandboxClient, extra: dict[str, Any]) -> None: + """Merge ``extra`` into the JSON body of the sandbox-create request. + + The SDK's ``create_sandbox`` builds a fixed payload with no passthrough, so + wrap the HTTP client's ``post`` to inject the fields on the ``POST /boxes`` + request only (other endpoints post to ``/boxes/{name}/...``). + """ + if not extra: + return + original_post = client._http.post + + def post_with_extra(url: Any, *args: Any, **kwargs: Any) -> Any: + payload = kwargs.get("json") + if str(url).endswith("/boxes") and isinstance(payload, dict): + kwargs["json"] = {**payload, **extra} + return original_post(url, *args, **kwargs) + + client._http.post = post_with_extra + + def _github_proxy_rules(github_token: str) -> list[dict[str, Any]]: basic_auth = base64.b64encode(f"x-access-token:{github_token}".encode()).decode() return [ @@ -134,6 +236,22 @@ def _is_retryable_proxy_config_error(exc: BaseException) -> bool: return isinstance(exc, httpx.TransportError) +def _release_sandbox_name(client: SandboxClient, name: str | None) -> None: + """Best-effort delete of any existing sandbox holding ``name``. + + Sandbox names are unique in LangSmith and thread-deterministic, so the only + box that can hold this name is this thread's own — typically a dead one + (idle-stopped past its TTL) we're recreating. Provisioning is serialized per + thread, so this never races a live box. Without this, recreate would 409. + """ + if not name: + return + try: + client.delete_sandbox(name) + except Exception as exc: # noqa: BLE001 - name is free if nothing to delete + logger.debug("No pre-existing sandbox %s to release (%s)", name, type(exc).__name__) + + def _configure_github_proxy(sandbox_name: str, github_token: str) -> None: """Configure sandbox proxy to inject GitHub auth for GitHub traffic. @@ -145,11 +263,11 @@ def _configure_github_proxy(sandbox_name: str, github_token: str) -> None: sandbox_name: The sandbox name/ID returned by the LangSmith API. github_token: GitHub token to inject as Authorization header. """ - api_key = _get_langsmith_api_key() + api_key = _get_sandbox_api_key() if not api_key: logger.warning("No LangSmith API key found, skipping GitHub proxy configuration") return - langsmith_endpoint = os.environ.get("LANGSMITH_ENDPOINT", "https://api.smith.langchain.com") + langsmith_endpoint = _get_sandbox_endpoint() url = f"{langsmith_endpoint}/v2/sandboxes/boxes/{sandbox_name}" payload = {"proxy_config": {"rules": _github_proxy_rules(github_token)}} with httpx.Client(timeout=PROXY_CONFIG_TIMEOUT_SECONDS) as client: @@ -211,7 +329,7 @@ def create_langsmith_sandbox( Returns: SandboxBackendProtocol instance """ - api_key = _get_langsmith_api_key() + api_key = _get_sandbox_api_key() ( default_snapshot_id, fs_capacity_bytes, @@ -227,6 +345,7 @@ def create_langsmith_sandbox( backend = provider.get_or_create( sandbox_id=sandbox_id, snapshot_id=effective_snapshot_id, + name=_sandbox_name_for_thread(_current_thread_id()), fs_capacity_bytes=fs_capacity_bytes, vcpus=vcpus, mem_bytes=mem_bytes, @@ -246,11 +365,9 @@ def _update_thread_sandbox_metadata(sandbox_id: str) -> None: try: import asyncio - from langgraph.config import get_config from langgraph_sdk import get_client - config = get_config() - thread_id = config.get("configurable", {}).get("thread_id") + thread_id = _current_thread_id() if not thread_id: return client = get_client() @@ -416,11 +533,14 @@ class LangSmithProvider(SandboxProvider): def __init__(self, api_key: str | None = None) -> None: from langsmith import sandbox - self._api_key = api_key or _get_langsmith_api_key() + self._api_key = api_key or _get_sandbox_api_key() + self._api_endpoint = _get_sandbox_api_endpoint() if not self._api_key: msg = "LANGSMITH_API_KEY (or LANGSMITH_API_KEY_PROD) not set" raise ValueError(msg) - self._client: SandboxClient = sandbox.SandboxClient(api_key=self._api_key) + self._client: SandboxClient = sandbox.SandboxClient( + api_key=self._api_key, api_endpoint=self._api_endpoint + ) @classmethod def validate_startup_config(cls) -> None: @@ -453,6 +573,7 @@ class LangSmithProvider(SandboxProvider): ): msg = f"{name} must be >= 0, got {value}" raise ValueError(msg) + _get_sandbox_create_extra_fields() def get_or_create( self, @@ -460,6 +581,7 @@ class LangSmithProvider(SandboxProvider): sandbox_id: str | None = None, timeout: int = 180, snapshot_id: str | None = None, + name: str | None = None, fs_capacity_bytes: int | None = None, vcpus: int | None = None, mem_bytes: int | None = None, @@ -483,9 +605,13 @@ class LangSmithProvider(SandboxProvider): msg = "DEFAULT_SANDBOX_SNAPSHOT_ID must be set when SANDBOX_TYPE=langsmith" raise ValueError(msg) + _install_create_extra_fields(self._client, _get_sandbox_create_extra_fields()) + _release_sandbox_name(self._client, name) + try: sandbox = self._client.create_sandbox( snapshot_id=snapshot_id, + name=name, fs_capacity_bytes=fs_capacity_bytes, vcpus=vcpus, mem_bytes=mem_bytes, diff --git a/agent/middleware/refresh_slack_status.py b/agent/middleware/refresh_slack_status.py index cfcf1dd0..fce7332b 100644 --- a/agent/middleware/refresh_slack_status.py +++ b/agent/middleware/refresh_slack_status.py @@ -63,6 +63,7 @@ _TOOL_STATUS: dict[str, str] = { "linear_get_issue": "checking Linear...", "linear_get_issue_comments": "checking Linear...", "linear_list_teams": "checking Linear...", + "linear_search_issues": "searching Linear...", "linear_update_issue": "updating Linear...", "linear_delete_issue": "updating Linear...", "add_finding": "recording review findings...", diff --git a/agent/reviewer.py b/agent/reviewer.py index 6c2db45a..ac7285fa 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -854,10 +854,11 @@ async def _resolve_grouping_model( async def get_reviewer_agent(config: RunnableConfig) -> Pregel: """Get or create a reviewer agent with a sandbox + prepped repo.""" + config = config.copy() + config["configurable"] = config["configurable"].copy() + config.setdefault("recursion_limit", DEFAULT_RECURSION_LIMIT) thread_id = config["configurable"].get("thread_id", None) - config["recursion_limit"] = DEFAULT_RECURSION_LIMIT - if thread_id is None or not graph_loaded_for_execution(config): logger.info("No thread_id or not for execution, returning reviewer agent without sandbox") return create_deep_agent(system_prompt="", tools=[]).with_config(config) diff --git a/agent/server.py b/agent/server.py index 665bfc67..4db8aeb7 100644 --- a/agent/server.py +++ b/agent/server.py @@ -105,6 +105,7 @@ from .tools import ( linear_get_issue, linear_get_issue_comments, linear_list_teams, + linear_search_issues, linear_update_issue, open_pull_request, report_platform_issue, @@ -1019,6 +1020,7 @@ async def get_agent(config: RunnableConfig) -> Pregel: linear_get_issue, linear_get_issue_comments, linear_list_teams, + linear_search_issues, linear_update_issue, jira_comment, jira_create_issue, diff --git a/agent/tools/__init__.py b/agent/tools/__init__.py index 544c8d95..f408aed8 100644 --- a/agent/tools/__init__.py +++ b/agent/tools/__init__.py @@ -24,6 +24,7 @@ _TOOL_MODULES = { "linear_get_issue": ".linear_get_issue", "linear_get_issue_comments": ".linear_get_issue_comments", "linear_list_teams": ".linear_list_teams", + "linear_search_issues": ".linear_search_issues", "linear_update_issue": ".linear_update_issue", "list_findings": ".list_findings", "list_review_findings": ".list_review_findings", @@ -67,6 +68,7 @@ __all__ = [ "linear_get_issue", "linear_get_issue_comments", "linear_list_teams", + "linear_search_issues", "linear_update_issue", "list_findings", "list_review_findings", @@ -110,6 +112,7 @@ if TYPE_CHECKING: from .linear_get_issue import linear_get_issue from .linear_get_issue_comments import linear_get_issue_comments from .linear_list_teams import linear_list_teams + from .linear_search_issues import linear_search_issues from .linear_update_issue import linear_update_issue from .list_findings import list_findings from .list_review_findings import list_review_findings diff --git a/agent/tools/linear_search_issues.py b/agent/tools/linear_search_issues.py new file mode 100644 index 00000000..bb223cf7 --- /dev/null +++ b/agent/tools/linear_search_issues.py @@ -0,0 +1,34 @@ +from typing import Any + +from ..utils.linear import search_issues + + +async def linear_search_issues( + query: str, + team_id: str | None = None, + limit: int = 10, + include_archived: bool = False, + include_comments: bool = False, + after: str | None = None, +) -> dict[str, Any]: + """Search Linear issues by title, description, and optionally comments. + + Args: + query: Free-text search query. + team_id: Optional team UUID used to restrict matches to that team. + limit: Maximum results to return, from 1 to 50. + include_archived: Whether to include archived issues. + include_comments: Whether to search issue comments in addition to issue content. + after: Optional pagination cursor from a previous result's page_info.endCursor. + + Returns: + Matching issues plus total_count and page_info for pagination. + """ + return await search_issues( + query=query, + team_id=team_id, + limit=limit, + include_archived=include_archived, + include_comments=include_comments, + after=after, + ) diff --git a/agent/utils/linear.py b/agent/utils/linear.py index 6e45fec8..5fce95b5 100644 --- a/agent/utils/linear.py +++ b/agent/utils/linear.py @@ -132,6 +132,84 @@ async def get_issue(issue_id: str) -> dict[str, Any]: return {"issue": result.get("issue")} +async def search_issues( + query: str, + team_id: str | None = None, + limit: int = 10, + include_archived: bool = False, + include_comments: bool = False, + after: str | None = None, +) -> dict[str, Any]: + """Search Linear issues by free-text query.""" + query = query.strip() + if not query: + return {"error": "Search query must not be empty"} + if not 1 <= limit <= 50: + return {"error": "Search limit must be between 1 and 50"} + + search_query = """ + query SearchIssues( + $query: String! + $filter: IssueFilter + $limit: Int! + $includeArchived: Boolean + $includeComments: Boolean + $after: String + ) { + searchIssues( + term: $query + filter: $filter + first: $limit + includeArchived: $includeArchived + includeComments: $includeComments + after: $after + ) { + totalCount + pageInfo { + hasNextPage + endCursor + } + nodes { + id + identifier + title + priority + priorityLabel + state { id name type } + assignee { id name email } + team { id name key } + project { id name } + labels { nodes { id name } } + createdAt + updatedAt + archivedAt + url + } + } + } + """ + result = await _graphql_request( + search_query, + { + "query": query, + "filter": {"team": {"id": {"eq": team_id}}} if team_id else None, + "limit": limit, + "includeArchived": include_archived, + "includeComments": include_comments, + "after": after, + }, + ) + if "error" in result: + return result + + search_results = result.get("searchIssues", {}) + return { + "issues": search_results.get("nodes", []), + "total_count": search_results.get("totalCount", 0), + "page_info": search_results.get("pageInfo", {}), + } + + async def create_issue( team_id: str, title: str, diff --git a/docs/CUSTOMIZATION.md b/docs/CUSTOMIZATION.md index b6165937..71ff4fcd 100644 --- a/docs/CUSTOMIZATION.md +++ b/docs/CUSTOMIZATION.md @@ -69,6 +69,8 @@ Set the `SANDBOX_TYPE` environment variable to switch providers. Each provider h > **Warning**: `local` runs commands directly on your host with no sandboxing. Only use for local development with human-in-the-loop enabled. +For `langsmith`, sandboxes default to the same LangSmith credentials as tracing. To run sandboxes against a **different** LangSmith workspace, set `SANDBOX_LANGSMITH_API_KEY` (falls back to `LANGSMITH_API_KEY` / `LANGSMITH_API_KEY_PROD`) and optionally `SANDBOX_LANGSMITH_ENDPOINT` (falls back to `LANGSMITH_ENDPOINT`). These apply to sandbox create/connect/delete, the GitHub proxy config, and repo snapshot builds — the `DEFAULT_SANDBOX_SNAPSHOT_ID` must exist in whichever workspace these credentials point at. + ### Adding a new sandbox provider 1. **Create an integration file** at `agent/integrations/my_provider.py` with a factory function matching this signature: diff --git a/tests/reviewer/test_factory_config_isolation.py b/tests/reviewer/test_factory_config_isolation.py new file mode 100644 index 00000000..14fe78bf --- /dev/null +++ b/tests/reviewer/test_factory_config_isolation.py @@ -0,0 +1,122 @@ +"""Tests that get_reviewer_agent and get_chat_agent do not mutate the caller's config.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +from langgraph.graph.state import RunnableConfig + + +def _make_config(recursion_limit: int = 25) -> RunnableConfig: + return { + "configurable": {"thread_id": None}, + "recursion_limit": recursion_limit, + } + + +@pytest.mark.asyncio +async def test_get_reviewer_agent_does_not_mutate_caller_config() -> None: + """get_reviewer_agent must not overwrite the caller's recursion_limit.""" + from agent import reviewer + + config = _make_config(recursion_limit=25) + original_limit = config["recursion_limit"] + + fake_pregel = MagicMock() + fake_pregel.with_config = MagicMock(return_value=fake_pregel) + + with patch("agent.reviewer.create_deep_agent", return_value=fake_pregel): + await reviewer.get_reviewer_agent(config) + + assert config["recursion_limit"] == original_limit, ( + f"get_reviewer_agent mutated caller's recursion_limit: " + f"expected {original_limit}, got {config['recursion_limit']}" + ) + + +@pytest.mark.asyncio +async def test_get_reviewer_agent_applies_default_when_limit_unset() -> None: + """get_reviewer_agent should apply DEFAULT_RECURSION_LIMIT when the caller didn't set one.""" + from agent import reviewer + + config: RunnableConfig = {"configurable": {"thread_id": None}} + + fake_pregel = MagicMock() + fake_pregel.with_config = MagicMock(return_value=fake_pregel) + + with patch("agent.reviewer.create_deep_agent", return_value=fake_pregel): + await reviewer.get_reviewer_agent(config) + + assert "recursion_limit" not in config + + +@pytest.mark.asyncio +async def test_get_chat_agent_does_not_mutate_caller_config() -> None: + """get_chat_agent must not overwrite the caller's recursion_limit.""" + from agent import chat + + config = _make_config(recursion_limit=50) + original_limit = config["recursion_limit"] + + fake_pregel = MagicMock() + fake_pregel.with_config = MagicMock(return_value=fake_pregel) + + with patch("agent.chat.create_deep_agent", return_value=fake_pregel): + await chat.get_chat_agent(config) + + assert config["recursion_limit"] == original_limit, ( + f"get_chat_agent mutated caller's recursion_limit: " + f"expected {original_limit}, got {config['recursion_limit']}" + ) + + +@pytest.mark.asyncio +async def test_get_chat_agent_applies_default_when_limit_unset() -> None: + """get_chat_agent should apply DEFAULT_RECURSION_LIMIT when the caller didn't set one.""" + from agent import chat + + config: RunnableConfig = {"configurable": {"thread_id": None}} + + fake_pregel = MagicMock() + fake_pregel.with_config = MagicMock(return_value=fake_pregel) + + with patch("agent.chat.create_deep_agent", return_value=fake_pregel): + await chat.get_chat_agent(config) + + assert "recursion_limit" not in config + + +@pytest.mark.parametrize( + ("module_name", "factory_name"), + [("agent.reviewer", "get_reviewer_agent"), ("agent.chat", "get_chat_agent")], +) +@pytest.mark.asyncio +async def test_factory_copies_config_dicts_but_preserves_runtime_objects( + module_name: str, factory_name: str +) -> None: + """Factory config isolation must preserve callback and configurable value identities.""" + module = __import__(module_name, fromlist=[factory_name]) + factory = getattr(module, factory_name) + callback = object() + configurable_value = object() + callbacks = [callback] + config: RunnableConfig = { + "configurable": {"thread_id": None, "custom_key": configurable_value}, + "callbacks": callbacks, + } + + fake_pregel = MagicMock() + fake_pregel.with_config = MagicMock(return_value=fake_pregel) + + with patch(f"{module_name}.create_deep_agent", return_value=fake_pregel): + await factory(config) + + bound_config = fake_pregel.with_config.call_args.args[0] + assert bound_config is not config + assert bound_config["configurable"] is not config["configurable"] + assert bound_config["configurable"]["custom_key"] is configurable_value + assert bound_config["callbacks"] is callbacks + assert bound_config["callbacks"][0] is callback + assert "recursion_limit" not in config + assert config["configurable"] == {"thread_id": None, "custom_key": configurable_value} diff --git a/tests/reviewer/test_reviewer.py b/tests/reviewer/test_reviewer.py index 7a5fb3a1..1073c356 100644 --- a/tests/reviewer/test_reviewer.py +++ b/tests/reviewer/test_reviewer.py @@ -1288,13 +1288,17 @@ async def test_reviewer_populates_diff_line_set_from_github_api() -> None: patch("agent.utils.deferred_model.make_model", return_value=MagicMock()), patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent), ): - await reviewer.get_reviewer_agent(config) + agent = await reviewer.get_reviewer_agent(config) mock_fetch_diff.assert_awaited_once_with( owner="acme", repo="repo", pr_number=42, token="gh-token" ) - assert config["configurable"]["diff_text"] == pr_diff - assert config["configurable"]["diff_line_set"] == {"in_diff.py": {"RIGHT": {10}, "LEFT": {1}}} + # get_reviewer_agent copies the caller's config before mutating it (it must not + # mutate the caller's dict in place), so assert against the config actually + # bound to the returned agent rather than the original input dict. + bound_configurable = agent.config["configurable"] + assert bound_configurable["diff_text"] == pr_diff + assert bound_configurable["diff_line_set"] == {"in_diff.py": {"RIGHT": {10}, "LEFT": {1}}} @pytest.mark.asyncio @@ -1353,10 +1357,14 @@ async def test_reviewer_leaves_validation_disabled_when_diff_fetch_fails() -> No patch("agent.utils.deferred_model.make_model", return_value=MagicMock()), patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent), ): - await reviewer.get_reviewer_agent(config) + agent = await reviewer.get_reviewer_agent(config) - assert config["configurable"]["diff_text"] == "" - assert config["configurable"]["diff_line_set"] is None + # get_reviewer_agent copies the caller's config before mutating it (it must not + # mutate the caller's dict in place), so assert against the config actually + # bound to the returned agent rather than the original input dict. + bound_configurable = agent.config["configurable"] + assert bound_configurable["diff_text"] == "" + assert bound_configurable["diff_line_set"] is None @pytest.mark.asyncio diff --git a/tests/sandbox/test_langsmith_sandbox_config.py b/tests/sandbox/test_langsmith_sandbox_config.py index 37113cb9..eb361311 100644 --- a/tests/sandbox/test_langsmith_sandbox_config.py +++ b/tests/sandbox/test_langsmith_sandbox_config.py @@ -1,6 +1,8 @@ """Tests for LangSmith sandbox env-var configuration parsing.""" -from unittest.mock import patch +import base64 +import uuid +from unittest.mock import MagicMock, patch import pytest @@ -11,10 +13,62 @@ from agent.integrations.langsmith import ( DEFAULT_SANDBOX_VCPUS, DEFAULT_SNAPSHOT_FS_CAPACITY_BYTES, LangSmithProvider, + _get_sandbox_api_endpoint, + _get_sandbox_create_extra_fields, _get_sandbox_snapshot_config, + _install_create_extra_fields, + _release_sandbox_name, + _sandbox_name_for_thread, ) +def test_sandbox_api_endpoint_appends_v2_sandboxes() -> None: + with patch.dict("os.environ", {"LANGSMITH_ENDPOINT": "https://eu.smith.langchain.com"}): + assert _get_sandbox_api_endpoint() == "https://eu.smith.langchain.com/v2/sandboxes" + + +def test_sandbox_api_endpoint_no_double_suffix() -> None: + with patch.dict( + "os.environ", + {"SANDBOX_LANGSMITH_ENDPOINT": "https://x.smith.langchain.com/v2/sandboxes"}, + ): + assert _get_sandbox_api_endpoint() == "https://x.smith.langchain.com/v2/sandboxes" + + +def test_sandbox_name_for_thread_encodes_uuid() -> None: + thread_id = "12345678-1234-5678-1234-567812345678" + name = _sandbox_name_for_thread(thread_id) + assert name is not None + prefix, _, encoded = name.partition("-") + assert prefix == "openswe" + assert encoded == encoded.lower() + assert "=" not in encoded and "-" not in encoded + # Round-trips back to the original UUID. + padded = encoded.upper() + "=" * (-len(encoded) % 8) + assert uuid.UUID(bytes=base64.b32decode(padded)) == uuid.UUID(thread_id) + + +def test_sandbox_name_for_thread_none_or_invalid() -> None: + assert _sandbox_name_for_thread(None) is None + assert _sandbox_name_for_thread("not-a-uuid") is None + + +def test_release_sandbox_name_deletes_stale_box() -> None: + client = MagicMock() + _release_sandbox_name(client, "openswe-abc") + client.delete_sandbox.assert_called_once_with("openswe-abc") + + +def test_release_sandbox_name_swallows_missing_and_skips_none() -> None: + client = MagicMock() + client.delete_sandbox.side_effect = RuntimeError("not found") + _release_sandbox_name(client, "openswe-abc") # must not raise + + client.delete_sandbox.reset_mock(side_effect=True) + _release_sandbox_name(client, None) + client.delete_sandbox.assert_not_called() + + def test_defaults_when_env_unset() -> None: with patch.dict( "os.environ", @@ -97,3 +151,67 @@ def test_validate_startup_accepts_valid_config() -> None: clear=True, ): LangSmithProvider.validate_startup_config() + + +def test_extra_fields_unset_is_empty() -> None: + with patch.dict("os.environ", {}, clear=True): + assert _get_sandbox_create_extra_fields() == {} + with patch.dict("os.environ", {"SANDBOX_CREATE_EXTRA_JSON": " "}, clear=True): + assert _get_sandbox_create_extra_fields() == {} + + +def test_extra_fields_parsed() -> None: + with patch.dict( + "os.environ", + {"SANDBOX_CREATE_EXTRA_JSON": '{"_internal_runtime": "v2"}'}, + clear=True, + ): + assert _get_sandbox_create_extra_fields() == {"_internal_runtime": "v2"} + + +def test_extra_fields_rejects_invalid_json() -> None: + with patch.dict("os.environ", {"SANDBOX_CREATE_EXTRA_JSON": "{not json"}, clear=True): + with pytest.raises(ValueError, match="valid JSON"): + _get_sandbox_create_extra_fields() + + +def test_extra_fields_rejects_non_object() -> None: + with patch.dict("os.environ", {"SANDBOX_CREATE_EXTRA_JSON": "[1, 2]"}, clear=True): + with pytest.raises(ValueError, match="JSON object"): + _get_sandbox_create_extra_fields() + + +def test_install_create_extra_fields_merges_only_boxes_post() -> None: + calls: list[tuple[str, dict]] = [] + + class _FakeHttp: + def post(self, url, **kwargs): # noqa: ANN001, ANN003 + calls.append((url, kwargs.get("json"))) + return "ok" + + class _FakeClient: + def __init__(self) -> None: + self._http = _FakeHttp() + + client = _FakeClient() + _install_create_extra_fields(client, {"_internal_runtime": "v2"}) + + client._http.post("https://api/v2/sandboxes/boxes", json={"snapshot_id": "s"}) + client._http.post("https://api/v2/sandboxes/boxes/abc/start", json={"foo": "bar"}) + + assert calls[0][1] == {"snapshot_id": "s", "_internal_runtime": "v2"} + assert calls[1][1] == {"foo": "bar"} + + +def test_install_create_extra_fields_noop_when_empty() -> None: + class _FakeHttp: + def __init__(self) -> None: + self.post = "sentinel" + + class _FakeClient: + def __init__(self) -> None: + self._http = _FakeHttp() + + client = _FakeClient() + _install_create_extra_fields(client, {}) + assert client._http.post == "sentinel" diff --git a/tests/sandbox/test_proxy_auth.py b/tests/sandbox/test_proxy_auth.py index a00d7ca0..0c257324 100644 --- a/tests/sandbox/test_proxy_auth.py +++ b/tests/sandbox/test_proxy_auth.py @@ -125,6 +125,35 @@ class TestConfigureGithubProxy: headers = mock_client.patch.call_args.kwargs["headers"] assert headers == {"X-API-Key": "my-api-key"} + def test_sandbox_overrides_take_precedence(self) -> None: + """SANDBOX_LANGSMITH_* override the shared key/endpoint for the proxy call.""" + with ( + patch("agent.integrations.langsmith.httpx.Client") as mock_client_cls, + patch.dict( + "os.environ", + { + "LANGSMITH_API_KEY": "shared-key", + "LANGSMITH_ENDPOINT": "https://shared.smith.langchain.com", + "SANDBOX_LANGSMITH_API_KEY": "sandbox-key", + "SANDBOX_LANGSMITH_ENDPOINT": "https://sandbox.smith.langchain.com", + }, + ), + ): + mock_client = MagicMock() + mock_response = MagicMock() + mock_response.raise_for_status = MagicMock() + mock_client.patch.return_value = mock_response + mock_client_cls.return_value.__enter__ = MagicMock(return_value=mock_client) + mock_client_cls.return_value.__exit__ = MagicMock(return_value=False) + + _configure_github_proxy("sandbox-abc", "token") + + assert ( + mock_client.patch.call_args.args[0] + == "https://sandbox.smith.langchain.com/v2/sandboxes/boxes/sandbox-abc" + ) + assert mock_client.patch.call_args.kwargs["headers"] == {"X-API-Key": "sandbox-key"} + def test_retries_transient_http_error(self) -> None: """Transient proxy API errors should be retried on the same sandbox.""" request = httpx.Request( diff --git a/tests/tools/test_linear_search_issues.py b/tests/tools/test_linear_search_issues.py new file mode 100644 index 00000000..233a80bb --- /dev/null +++ b/tests/tools/test_linear_search_issues.py @@ -0,0 +1,131 @@ +from __future__ import annotations + +import importlib +from typing import Any + +import pytest + +from agent.utils import linear + +linear_search_tool = importlib.import_module("agent.tools.linear_search_issues") + + +async def test_search_issues_returns_results_and_pagination( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + async def fake_graphql_request( + query: str, variables: dict[str, Any] | None = None + ) -> dict[str, Any]: + captured.update({"query": query, "variables": variables}) + return { + "searchIssues": { + "nodes": [ + { + "id": "issue-id", + "identifier": "DCD-20", + "title": "User-message styling improvement", + } + ], + "totalCount": 12, + "pageInfo": {"hasNextPage": True, "endCursor": "next-page"}, + } + } + + monkeypatch.setattr(linear, "_graphql_request", fake_graphql_request) + + result = await linear.search_issues( + " user message styling ", + team_id="team-id", + limit=5, + include_archived=True, + include_comments=True, + after="current-page", + ) + + assert "searchIssues" in captured["query"] + assert captured["variables"] == { + "query": "user message styling", + "filter": {"team": {"id": {"eq": "team-id"}}}, + "limit": 5, + "includeArchived": True, + "includeComments": True, + "after": "current-page", + } + assert result == { + "issues": [ + { + "id": "issue-id", + "identifier": "DCD-20", + "title": "User-message styling improvement", + } + ], + "total_count": 12, + "page_info": {"hasNextPage": True, "endCursor": "next-page"}, + } + + +async def test_search_issues_rejects_blank_query(monkeypatch: pytest.MonkeyPatch) -> None: + async def unexpected_request(*_args: Any, **_kwargs: Any) -> dict[str, Any]: + pytest.fail("GraphQL request should not be made") + + monkeypatch.setattr(linear, "_graphql_request", unexpected_request) + + result = await linear.search_issues(" ") + + assert result == {"error": "Search query must not be empty"} + + +@pytest.mark.parametrize("limit", [0, 51]) +async def test_search_issues_rejects_invalid_limit( + monkeypatch: pytest.MonkeyPatch, limit: int +) -> None: + async def unexpected_request(*_args: Any, **_kwargs: Any) -> dict[str, Any]: + pytest.fail("GraphQL request should not be made") + + monkeypatch.setattr(linear, "_graphql_request", unexpected_request) + + result = await linear.search_issues("styling", limit=limit) + + assert result == {"error": "Search limit must be between 1 and 50"} + + +async def test_search_issues_propagates_graphql_errors(monkeypatch: pytest.MonkeyPatch) -> None: + async def fake_graphql_request( + _query: str, _variables: dict[str, Any] | None = None + ) -> dict[str, Any]: + return {"error": "rate limited"} + + monkeypatch.setattr(linear, "_graphql_request", fake_graphql_request) + + assert await linear.search_issues("styling") == {"error": "rate limited"} + + +async def test_linear_search_issues_tool_delegates(monkeypatch: pytest.MonkeyPatch) -> None: + captured: dict[str, Any] = {} + + async def fake_search_issues(**kwargs: Any) -> dict[str, Any]: + captured.update(kwargs) + return {"issues": []} + + monkeypatch.setattr(linear_search_tool, "search_issues", fake_search_issues) + + result = await linear_search_tool.linear_search_issues( + "styling", + team_id="team-id", + limit=20, + include_archived=True, + include_comments=True, + after="cursor", + ) + + assert result == {"issues": []} + assert captured == { + "query": "styling", + "team_id": "team-id", + "limit": 20, + "include_archived": True, + "include_comments": True, + "after": "cursor", + } diff --git a/ui/src/features/agents/components/AgentGitPanel.tsx b/ui/src/features/agents/components/AgentGitPanel.tsx index 540b0d9f..12bfe404 100644 --- a/ui/src/features/agents/components/AgentGitPanel.tsx +++ b/ui/src/features/agents/components/AgentGitPanel.tsx @@ -88,9 +88,6 @@ function commonDirPrefix(paths: Array): string { } const PANEL_STORAGE_WIDTH = "open-swe.gitpanel.width" -const PANEL_STORAGE_COLLAPSED = "open-swe.gitpanel.collapsed" -const COLLAPSED_STATE_TRUE = "1" -const COLLAPSED_STATE_FALSE = "0" const PANEL_DEFAULT_WIDTH = 420 const PANEL_MIN_WIDTH = 320 // Keep at least this much room for the chat so the panel can grow to nearly the @@ -119,23 +116,6 @@ function readStoredPanelWidth(): number { return clampPanelWidth(parsed) } -export function readStoredPanelCollapsed(): boolean { - if (typeof window === "undefined") return true - // Default to collapsed until the user opens it once. - return ( - window.localStorage.getItem(PANEL_STORAGE_COLLAPSED) !== - COLLAPSED_STATE_FALSE - ) -} - -export function writeStoredPanelCollapsed(collapsed: boolean): void { - if (typeof window === "undefined") return - window.localStorage.setItem( - PANEL_STORAGE_COLLAPSED, - collapsed ? COLLAPSED_STATE_TRUE : COLLAPSED_STATE_FALSE - ) -} - function PanelResizeHandle({ width, onResize, diff --git a/ui/src/features/agents/components/AgentThreadView.tsx b/ui/src/features/agents/components/AgentThreadView.tsx index c1b3fb9a..ce3e8b91 100644 --- a/ui/src/features/agents/components/AgentThreadView.tsx +++ b/ui/src/features/agents/components/AgentThreadView.tsx @@ -12,10 +12,12 @@ import type { ModelSelection } from "@/features/agents/lib/provider/useModelOpti import { AgentGitPanel, PANEL_MIN_CHAT_WIDTH, - readStoredPanelCollapsed, - writeStoredPanelCollapsed, } from "@/features/agents/components/AgentGitPanel" import { AgentPromptBar } from "@/features/agents/components/AgentPromptBar" +import { + readStoredPanelCollapsed, + writeStoredPanelCollapsed, +} from "@/features/agents/lib/gitPanelPreferences" import { Messages } from "@/features/agents/components/messages" import { streamMessagesToUi } from "@/features/agents/lib/streamMessagesToUi" import { messageArrivalTimestamp } from "@/features/agents/lib/messageTimestamps" diff --git a/ui/src/features/agents/components/AgentsThreadsPage.tsx b/ui/src/features/agents/components/AgentsThreadsPage.tsx index 1d89dd01..04f96cc3 100644 --- a/ui/src/features/agents/components/AgentsThreadsPage.tsx +++ b/ui/src/features/agents/components/AgentsThreadsPage.tsx @@ -61,6 +61,10 @@ function triToBool(value: TriState): boolean | undefined { return undefined } +function displayStatus(status: AgentStatus): string { + return STATUS_OPTIONS.find((option) => option.value === status)?.label ?? status +} + export function AgentsThreadsPage({ filters, onFiltersChange, @@ -279,8 +283,8 @@ function ThreadListItem({ thread }: { thread: AgentThread }) {

{thread.title}

- {thread.repoFullName || "no repo"} · {thread.status} - {isResolved ? " · resolved" : ""} + {thread.repoFullName || "No repo"} · {displayStatus(thread.status)} + {isResolved ? " · Resolved" : ""}