diff --git a/agent/utils/repo.py b/agent/utils/repo.py deleted file mode 100644 index 01951937..00000000 --- a/agent/utils/repo.py +++ /dev/null @@ -1,42 +0,0 @@ -"""Utilities for extracting repository configuration from text.""" - -from __future__ import annotations - -import os -import re - -_DEFAULT_REPO_OWNER = os.environ.get("DEFAULT_REPO_OWNER", "langchain-ai") - - -def extract_repo_from_text(text: str, default_owner: str | None = None) -> dict[str, str] | None: - """Extract owner/name repo config from text containing repo: syntax or GitHub URLs. - - Checks for explicit ``repo:owner/name`` or ``repo owner/name`` first, then - falls back to GitHub URL extraction. - - Returns: - A dict with ``owner`` and ``name`` keys, or ``None`` if no repo found. - """ - if default_owner is None: - default_owner = _DEFAULT_REPO_OWNER - owner: str | None = None - name: str | None = None - - if "repo:" in text or "repo " in text: - match = re.search(r"repo[: ]([a-zA-Z0-9_.\-/]+)", text) - if match: - value = match.group(1).rstrip("/") - if "/" in value: - owner, name = value.split("/", 1) - else: - owner = default_owner - name = value - - if not owner or not name: - github_match = re.search(r"github\.com/([a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+)", text) - if github_match: - owner, name = github_match.group(1).split("/", 1) - - if owner and name: - return {"owner": owner, "name": name} - return None diff --git a/agent/webapp.py b/agent/webapp.py index 3e775670..32ab1e53 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -59,7 +59,6 @@ from .utils.github_user_email_map import GITHUB_USER_EMAIL_MAP 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 -from .utils.repo import extract_repo_from_text from .utils.sandbox import validate_sandbox_startup_config from .utils.slack import ( GitHubPrRef, @@ -494,31 +493,29 @@ async def get_slack_repo_config( """Resolve repository configuration for Slack-triggered runs. Priority: - 1. Explicit ``owner/repo`` mention in the message body. - 2. Repo carried over from the existing Slack thread's metadata. - 3. The triggering user's dashboard ``default_repo`` (if they have a + 1. Repo carried over from the existing Slack thread's metadata. + 2. The triggering user's dashboard ``default_repo`` (if they have a profile and their Slack email maps to a known GitHub login). - 4. ``SLACK_REPO_*`` env defaults. + 3. ``SLACK_REPO_*`` env defaults. """ default_owner = SLACK_REPO_OWNER.strip() or DEFAULT_REPO_OWNER default_name = SLACK_REPO_NAME.strip() or DEFAULT_REPO_NAME thread_id = generate_thread_id_from_slack_thread(channel_id, thread_ts) langgraph_client = get_client(url=LANGGRAPH_URL) - repo_config = extract_repo_from_text(message, default_owner=default_owner) + repo_config: dict[str, str] | None = None - if not repo_config: - try: - thread = await langgraph_client.threads.get(thread_id) - thread_repo_config = _extract_repo_config_from_thread(thread) - if thread_repo_config: - repo_config = thread_repo_config - except Exception as exc: # noqa: BLE001 - if not _is_not_found_error(exc): - logger.exception( - "Failed to fetch Slack thread %s for repo resolution", - thread_id, - ) + try: + thread = await langgraph_client.threads.get(thread_id) + thread_repo_config = _extract_repo_config_from_thread(thread) + if thread_repo_config: + repo_config = thread_repo_config + except Exception as exc: # noqa: BLE001 + if not _is_not_found_error(exc): + logger.exception( + "Failed to fetch Slack thread %s for repo resolution", + thread_id, + ) if not repo_config and slack_user_id: try: @@ -1171,31 +1168,21 @@ async def linear_webhook( # noqa: PLR0911, PLR0912, PLR0915 logger.warning("Failed to fetch full issue details, using webhook data") full_issue = issue - repo_config = extract_repo_from_text(comment_body, default_owner=DEFAULT_REPO_OWNER) - - if repo_config: - logger.debug( - "Using repo from comment body: %s/%s", - repo_config["owner"], - repo_config["name"], + repo_config: dict[str, str] | None = None + comment_user_email = (data.get("user") or {}).get("email") + try: + profile_repo = await get_profile_default_repo(resolve_login_from_email(comment_user_email)) + except Exception: # noqa: BLE001 + logger.exception("Failed to apply dashboard default_repo for Linear user") + profile_repo = None + if profile_repo: + logger.info( + "Applying dashboard default_repo for Linear user %s: %s/%s", + comment_user_email, + profile_repo["owner"], + profile_repo["name"], ) - else: - comment_user_email = (data.get("user") or {}).get("email") - try: - profile_repo = await get_profile_default_repo( - resolve_login_from_email(comment_user_email) - ) - except Exception: # noqa: BLE001 - logger.exception("Failed to apply dashboard default_repo for Linear user") - profile_repo = None - if profile_repo: - logger.info( - "Applying dashboard default_repo for Linear user %s: %s/%s", - comment_user_email, - profile_repo["owner"], - profile_repo["name"], - ) - repo_config = profile_repo + repo_config = profile_repo if not repo_config: team = full_issue.get("team", {}) diff --git a/tests/test_repo_extraction.py b/tests/test_repo_extraction.py deleted file mode 100644 index 4ea57bd2..00000000 --- a/tests/test_repo_extraction.py +++ /dev/null @@ -1,159 +0,0 @@ -"""Tests for agent.utils.repo and Linear webhook repo override behavior.""" - -import json -from unittest.mock import AsyncMock, patch - -import pytest - -from agent.utils.repo import extract_repo_from_text - - -class TestExtractRepoFromText: - def test_repo_colon_with_org(self) -> None: - result = extract_repo_from_text("please use repo:my-org/my-repo") - assert result == {"owner": "my-org", "name": "my-repo"} - - def test_repo_space_with_org(self) -> None: - result = extract_repo_from_text("please use repo langchain-ai/langchainjs") - assert result == {"owner": "langchain-ai", "name": "langchainjs"} - - def test_repo_colon_name_only_uses_default_owner(self) -> None: - result = extract_repo_from_text("fix bug in repo:langchainplus") - assert result == {"owner": "langchain-ai", "name": "langchainplus"} - - def test_repo_space_name_only_uses_default_owner(self) -> None: - result = extract_repo_from_text("fix bug in repo open-swe") - assert result == {"owner": "langchain-ai", "name": "open-swe"} - - def test_repo_name_only_custom_default_owner(self) -> None: - result = extract_repo_from_text("repo:my-repo", default_owner="custom-org") - assert result == {"owner": "custom-org", "name": "my-repo"} - - def test_github_url(self) -> None: - result = extract_repo_from_text( - "check https://github.com/langchain-ai/langgraph-api please" - ) - assert result == {"owner": "langchain-ai", "name": "langgraph-api"} - - def test_explicit_repo_beats_github_url(self) -> None: - result = extract_repo_from_text( - "see https://github.com/langchain-ai/langgraph-api but use repo:my-org/my-repo" - ) - assert result == {"owner": "my-org", "name": "my-repo"} - - def test_no_repo_returns_none(self) -> None: - result = extract_repo_from_text("please fix the bug") - assert result is None - - def test_empty_string_returns_none(self) -> None: - result = extract_repo_from_text("") - assert result is None - - def test_trailing_slash_stripped(self) -> None: - result = extract_repo_from_text("repo:my-org/my-repo/") - assert result == {"owner": "my-org", "name": "my-repo"} - - -class TestLinearWebhookRepoOverride: - """Test that the Linear webhook handler checks comment body for repo config first.""" - - @pytest.fixture() - def _base_payload(self) -> dict: - return { - "type": "Comment", - "action": "create", - "data": { - "id": "comment-123", - "body": "@openswe please fix this repo:custom-org/custom-repo", - "issue": { - "id": "issue-456", - "title": "Test issue", - }, - "user": {"id": "user-1", "name": "Test User", "email": "test@test.com"}, - }, - } - - @pytest.mark.asyncio - async def test_comment_repo_overrides_team_mapping(self, _base_payload: dict) -> None: - from agent.webapp import linear_webhook - - with ( - patch("agent.webapp.verify_linear_signature", return_value=True), - patch( - "agent.webapp.fetch_linear_issue_details", - new_callable=AsyncMock, - return_value={ - "id": "issue-456", - "title": "Test issue", - "identifier": "TEST-1", - "url": "https://linear.app/test/issue/TEST-1", - "team": {"id": "t1", "name": "Some Team", "key": "ST"}, - "project": {"id": "p1", "name": "Some Project"}, - "comments": {"nodes": []}, - }, - ), - patch("agent.webapp._is_repo_allowed", return_value=True), - patch("agent.webapp.BackgroundTasks"), - ): - mock_request = AsyncMock() - mock_request.body.return_value = json.dumps(_base_payload).encode() - mock_request.headers = {"Linear-Signature": "valid"} - - bg_tasks = AsyncMock() - result = await linear_webhook(mock_request, bg_tasks) - - assert result["status"] == "accepted" - assert "custom-org/custom-repo" in result["message"] - - call_args = bg_tasks.add_task.call_args - repo_config = call_args[0][2] - assert repo_config == {"owner": "custom-org", "name": "custom-repo"} - - @pytest.mark.asyncio - async def test_falls_back_to_team_mapping_when_no_repo_in_comment(self) -> None: - from agent.webapp import linear_webhook - - payload = { - "type": "Comment", - "action": "create", - "data": { - "id": "comment-123", - "body": "@openswe please fix this bug", - "issue": { - "id": "issue-456", - "title": "Test issue", - }, - "user": {"id": "user-1", "name": "Test User", "email": "test@test.com"}, - }, - } - - with ( - patch("agent.webapp.verify_linear_signature", return_value=True), - patch( - "agent.webapp.fetch_linear_issue_details", - new_callable=AsyncMock, - return_value={ - "id": "issue-456", - "title": "Test issue", - "identifier": "TEST-1", - "url": "https://linear.app/test/issue/TEST-1", - "team": {"id": "t1", "name": "Open SWE", "key": "OS"}, - "project": None, - "comments": {"nodes": []}, - }, - ), - patch("agent.webapp._is_repo_allowed", return_value=True), - ): - mock_request = AsyncMock() - mock_request.body.return_value = json.dumps(payload).encode() - mock_request.headers = {"Linear-Signature": "valid"} - - bg_tasks = AsyncMock() - result = await linear_webhook(mock_request, bg_tasks) - - assert result["status"] == "accepted" - assert "langchain-ai/open-swe" in result["message"] - - call_args = bg_tasks.add_task.call_args - repo_config = call_args[0][2] - assert repo_config == {"owner": "langchain-ai", "name": "open-swe"} diff --git a/tests/test_slack_context.py b/tests/test_slack_context.py index ca60f97c..c71cf126 100644 --- a/tests/test_slack_context.py +++ b/tests/test_slack_context.py @@ -301,46 +301,20 @@ def test_select_slack_context_messages_detects_username_mention() -> None: assert [item["ts"] for item in selected] == ["1.0", "2.0", "3.0"] -def test_get_slack_repo_config_message_repo_overrides_existing_thread_repo( +def test_get_slack_repo_config_uses_thread_metadata_when_set( monkeypatch: pytest.MonkeyPatch, ) -> None: threads_client = _FakeThreadsClient( thread={"metadata": {"repo": {"owner": "saved-owner", "name": "saved-repo"}}} ) - posted = False - - async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool: - nonlocal posted - posted = True - return True - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - monkeypatch.setattr( - webapp, "post_slack_thread_reply", fake_post_slack_thread_reply, raising=False - ) - - repo = asyncio.run( - webapp.get_slack_repo_config("please use repo:new-owner/new-repo", "C123", "1.234") - ) - - assert repo == {"owner": "new-owner", "name": "new-repo"} - assert threads_client.requested_thread_id is None - assert not posted - - -def test_get_slack_repo_config_parses_message_for_new_thread( - monkeypatch: pytest.MonkeyPatch, -) -> None: - threads_client = _FakeThreadsClient(raise_not_found=True) - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) repo = asyncio.run( - webapp.get_slack_repo_config("please use repo:new-owner/new-repo", "C123", "1.234") + webapp.get_slack_repo_config("please just help with this bug", "C123", "1.234") ) - assert repo == {"owner": "new-owner", "name": "new-repo"} + assert repo == {"owner": "saved-owner", "name": "saved-repo"} def test_get_slack_repo_config_existing_thread_without_repo_uses_default( @@ -360,129 +334,6 @@ def test_get_slack_repo_config_existing_thread_without_repo_uses_default( ) -def test_get_slack_repo_config_space_syntax_detected( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """repo owner/name (space instead of colon) should be detected correctly.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "please fix the bug in repo langchain-ai/langchainjs", "C123", "1.234" - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langchainjs"} - - -def test_get_slack_repo_config_github_url_extracted( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """GitHub URL in message should be used to detect the repo.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "I found a bug in https://github.com/langchain-ai/langgraph-api please fix it", - "C123", - "1.234", - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langgraph-api"} - - -def test_get_slack_repo_config_explicit_repo_beats_github_url( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Explicit repo: syntax takes priority over a GitHub URL also present in the message.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "see https://github.com/langchain-ai/langgraph-api but use repo:my-org/my-repo", - "C123", - "1.234", - ) - ) - - assert repo == {"owner": "my-org", "name": "my-repo"} - - -def test_get_slack_repo_config_explicit_space_syntax_beats_thread_metadata( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Explicit repo owner/name (space syntax) takes priority over saved thread metadata.""" - threads_client = _FakeThreadsClient( - thread={"metadata": {"repo": {"owner": "saved-owner", "name": "saved-repo"}}} - ) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "actually use repo langchain-ai/langchainjs today", "C123", "1.234" - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langchainjs"} - - -def test_get_slack_repo_config_github_url_beats_thread_metadata( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """A GitHub URL in the message takes priority over saved thread metadata.""" - threads_client = _FakeThreadsClient( - thread={"metadata": {"repo": {"owner": "saved-owner", "name": "saved-repo"}}} - ) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config( - "I found a bug in https://github.com/langchain-ai/langgraph-api", - "C123", - "1.234", - ) - ) - - assert repo == {"owner": "langchain-ai", "name": "langgraph-api"} - - -def test_get_slack_repo_config_repo_name_only_defaults_org( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """repo:name without org should default owner to langchain-ai.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run( - webapp.get_slack_repo_config("fix bug in repo:langchainplus", "C123", "1.234") - ) - - assert repo == {"owner": "langchain-ai", "name": "langchainplus"} - - -def test_get_slack_repo_config_repo_name_only_space_syntax( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """repo name (space syntax, no org) should default owner to langchain-ai.""" - threads_client = _FakeThreadsClient(raise_not_found=True) - - monkeypatch.setattr(webapp, "get_client", lambda url: _FakeClient(threads_client)) - - repo = asyncio.run(webapp.get_slack_repo_config("fix bug in repo open-swe", "C123", "1.234")) - - assert repo == {"owner": "langchain-ai", "name": "open-swe"} - - def _setup_slack_mention_fakes( monkeypatch: pytest.MonkeyPatch, captured: dict[str, object] ) -> None: