From f3512841fcd8e0d3f2c1dd8e77b0394cac1b990a Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Fri, 5 Jun 2026 13:49:30 -0700 Subject: [PATCH] feat: inject org-wide guidelines into reviewer prompt (#1431) Adds an admin-managed, org-wide review guidelines field to team settings that the reviewer injects into every PR review across all repos, alongside the existing per-repo style prompt and AGENTS.md context. Repo-specific rules take precedence when they conflict. Co-authored-by: open-swe[bot] --- agent/dashboard/team_settings.py | 34 ++++++++- agent/reviewer.py | 24 +++++- tests/test_reviewer.py | 87 ++++++++++++++++++++++ tests/test_team_settings_org_guidelines.py | 47 ++++++++++++ ui/src/lib/api.ts | 1 + ui/src/routes/review.tsx | 47 +++++++++++- 6 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 tests/test_team_settings_org_guidelines.py diff --git a/agent/dashboard/team_settings.py b/agent/dashboard/team_settings.py index bf130f1a..51a204c7 100644 --- a/agent/dashboard/team_settings.py +++ b/agent/dashboard/team_settings.py @@ -13,7 +13,7 @@ from datetime import UTC, datetime from typing import Any, Literal from langgraph_sdk import get_client -from pydantic import BaseModel, model_validator +from pydantic import BaseModel, field_validator, model_validator from .options import ( SUPPORTED_MODEL_IDS, @@ -30,6 +30,10 @@ TEAM_SETTINGS_KEY = "default" TriggerMode = Literal["every_push", "once_per_pr", "manual"] AutofixMode = Literal["off", "low", "medium", "high"] +# Cap the org-wide guidelines so a runaway value can't dominate the reviewer +# prompt. Generous enough for a detailed policy, small enough to stay bounded. +ORG_GUIDELINES_MAX_CHARS = 10_000 + class TeamSettingsUpdate(BaseModel): trigger_mode: TriggerMode = "every_push" @@ -38,6 +42,7 @@ class TeamSettingsUpdate(BaseModel): review_trace_links: bool = True autofix_mode: AutofixMode = "off" autofix_severity_threshold: AutofixMode = "medium" + org_guidelines: str | None = None default_agent_model: str | None = None default_agent_reasoning_effort: str | None = None default_agent_subagent_model: str | None = None @@ -48,6 +53,22 @@ class TeamSettingsUpdate(BaseModel): default_reviewer_subagent_model: str | None = None default_reviewer_subagent_reasoning_effort: str | None = None + @field_validator("org_guidelines", mode="before") + @classmethod + def _normalize_org_guidelines(cls, v: object) -> str | None: + if v is None: + return None + if not isinstance(v, str): + raise ValueError("org_guidelines must be a string") + text = v.strip() + if not text: + return None + if len(text) > ORG_GUIDELINES_MAX_CHARS: + raise ValueError( + f"org_guidelines must be at most {ORG_GUIDELINES_MAX_CHARS} characters" + ) + return text + @model_validator(mode="after") def _validate_model_pairs(self) -> TeamSettingsUpdate: _validate_model_effort_pair( @@ -108,6 +129,7 @@ def _default_settings() -> dict[str, Any]: "review_trace_links": True, "autofix_mode": "off", "autofix_severity_threshold": "medium", + "org_guidelines": None, "default_agent_model": fallback_model, "default_agent_reasoning_effort": fallback_effort, "default_agent_subagent_model": fallback_model, @@ -152,6 +174,7 @@ async def upsert_team_settings(update: TeamSettingsUpdate) -> dict[str, Any]: "review_trace_links": update.review_trace_links, "autofix_mode": update.autofix_mode, "autofix_severity_threshold": update.autofix_severity_threshold, + "org_guidelines": update.org_guidelines, "default_agent_model": update.default_agent_model, "default_agent_reasoning_effort": update.default_agent_reasoning_effort, "default_agent_subagent_model": update.default_agent_subagent_model, @@ -225,6 +248,15 @@ async def get_team_review_trace_links_enabled() -> bool: return bool(settings.get("review_trace_links", True)) +async def get_org_review_guidelines() -> str | None: + """Return the org-wide reviewer guidelines supplement, if configured.""" + settings = await get_team_settings() + value = settings.get("org_guidelines") + if isinstance(value, str) and value.strip(): + return value.strip() + return None + + async def get_team_default_subagent_model( role: Literal["agent", "reviewer"], ) -> tuple[str, str]: diff --git a/agent/reviewer.py b/agent/reviewer.py index 50ed144c..35efd79e 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -31,7 +31,7 @@ warnings.filterwarnings("ignore", message=".*Pydantic V1.*", category=UserWarnin from deepagents import create_deep_agent from langchain.agents.middleware import ModelCallLimitMiddleware -from .dashboard.team_settings import get_team_default_model_pair +from .dashboard.team_settings import get_org_review_guidelines, get_team_default_model_pair from .middleware import ( SanitizeThinkingBlocksMiddleware, SanitizeToolInputsMiddleware, @@ -275,6 +275,7 @@ def _reviewer_system_prompt( repo_name: str, pr_number: int | str, reviewer_eval: bool = False, + org_guidelines: str | None = None, repo_style_prompt: str | None = None, agents_md_content: str | None = None, ) -> str: @@ -286,6 +287,17 @@ def _reviewer_system_prompt( ) if reviewer_eval: prompt = f"{prompt}\n{REVIEWER_EVAL_PROMPT_SUFFIX}" + if org_guidelines: + prompt = ( + f"{prompt}\n\n" + "# Organization-wide review guidelines\n\n" + "These guidelines were set by a workspace admin and apply to every " + "repository this reviewer covers. Apply them when they agree with the " + "global bar above; they refine tone, severity, and what this " + "organization typically flags. Repository-specific rules below take " + "precedence when they conflict.\n\n" + f"{org_guidelines}" + ) if repo_style_prompt: prompt = ( f"{prompt}\n\n" @@ -743,6 +755,13 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: return await get_repo_custom_prompt(repo_owner, repo_name) + async def _fetch_org_guidelines() -> str | None: + try: + return await get_org_review_guidelines() + except Exception: # noqa: BLE001 + logger.exception("Failed to load org-wide review guidelines; continuing without them") + return None + async def _fetch_agents_md_context() -> str | None: if not repo_owner or not repo_name or not base_sha: return None @@ -768,12 +787,14 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: existing_threads_block, repo_style_prompt, agents_md_content, + org_guidelines, ) = await asyncio.gather( _fetch_diff_context(), _fetch_pr_overview(), _fetch_existing_threads_block(), _fetch_repo_style_prompt(), _fetch_agents_md_context(), + _fetch_org_guidelines(), ) pr_diff_text, pr_diff_line_set = diff_context pr_title, pr_body = pr_overview @@ -879,6 +900,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: repo_name=repo_name, pr_number=pr_number if isinstance(pr_number, int) else "", reviewer_eval=reviewer_eval, + org_guidelines=org_guidelines, repo_style_prompt=repo_style_prompt, agents_md_content=agents_md_content, ) diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py index f4285af3..bd4cc6b4 100644 --- a/tests/test_reviewer.py +++ b/tests/test_reviewer.py @@ -36,6 +36,32 @@ def test_reviewer_system_prompt_includes_repo_style_section() -> None: assert "missing tests for API" in prompt +def test_reviewer_system_prompt_includes_org_guidelines_section() -> None: + prompt = reviewer._reviewer_system_prompt( + "/workspace/repo", + repo_owner="acme", + repo_name="repo", + pr_number=42, + org_guidelines="Flag any new endpoint that lacks input validation.", + ) + assert "Organization-wide review guidelines" in prompt + assert "lacks input validation" in prompt + + +def test_reviewer_system_prompt_org_guidelines_precede_repo_style() -> None: + prompt = reviewer._reviewer_system_prompt( + "/workspace/repo", + repo_owner="acme", + repo_name="repo", + pr_number=42, + org_guidelines="Org rule text.", + repo_style_prompt="Repo rule text.", + ) + assert prompt.index("Organization-wide review guidelines") < prompt.index( + "Repository-specific review style" + ) + + def test_finding_reply_context_wraps_reply_as_untrusted_data() -> None: prompt = reviewer._build_finding_reply_context( pr_url="https://github.com/acme/repo/pull/1", @@ -338,6 +364,67 @@ async def test_reviewer_injects_repo_style_during_eval() -> None: assert "Flag table rerender regressions" in captured["system_prompt"] +@pytest.mark.asyncio +async def test_reviewer_inlines_org_guidelines_into_system_prompt() -> None: + config: RunnableConfig = { + "configurable": { + "__is_for_execution__": True, + "thread_id": "reviewer-thread-id", + "source": "github", + "repo": {"owner": "acme", "name": "repo"}, + "pr_number": 9, + "pr_url": "https://github.com/acme/repo/pull/9", + "base_sha": "base", + "head_sha": "head", + }, + "metadata": {}, + } + captured: dict[str, str] = {} + + def fake_create_deep_agent(*, system_prompt: str, **kwargs: object) -> _DummyAgent: + captured["system_prompt"] = system_prompt + return _DummyAgent() + + with ( + patch( + "agent.reviewer.get_github_app_installation_token_with_expiry", + new_callable=AsyncMock, + return_value=("gh-token", None), + ), + patch( + "agent.reviewer.ensure_sandbox_for_thread", + new_callable=AsyncMock, + return_value=MagicMock(), + ), + patch( + "agent.reviewer.aresolve_sandbox_work_dir", + new_callable=AsyncMock, + return_value="/workspace", + ), + patch( + "agent.reviewer.get_org_review_guidelines", + new_callable=AsyncMock, + return_value="Never approve a PR that disables a CI gate.", + ), + patch( + "agent.dashboard.review_styles.get_repo_custom_prompt", + new_callable=AsyncMock, + return_value=None, + ), + patch( + "agent.reviewer.fetch_agents_md", + new_callable=AsyncMock, + return_value=None, + ), + patch("agent.reviewer.make_model", return_value=MagicMock()), + patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent), + ): + await reviewer.get_reviewer_agent(config) + + assert "Organization-wide review guidelines" in captured["system_prompt"] + assert "disables a CI gate" in captured["system_prompt"] + + def test_reviewer_system_prompt_includes_agents_md_section() -> None: prompt = reviewer._reviewer_system_prompt( "/workspace/repo", diff --git a/tests/test_team_settings_org_guidelines.py b/tests/test_team_settings_org_guidelines.py new file mode 100644 index 00000000..594ca623 --- /dev/null +++ b/tests/test_team_settings_org_guidelines.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest +from pydantic import ValidationError + +from agent.dashboard.team_settings import ( + ORG_GUIDELINES_MAX_CHARS, + TeamSettingsUpdate, + get_org_review_guidelines, +) + + +def test_org_guidelines_blank_normalizes_to_none() -> None: + assert TeamSettingsUpdate(org_guidelines=" ").org_guidelines is None + assert TeamSettingsUpdate(org_guidelines=None).org_guidelines is None + + +def test_org_guidelines_trimmed() -> None: + update = TeamSettingsUpdate(org_guidelines=" Flag CI gate removals.\n") + assert update.org_guidelines == "Flag CI gate removals." + + +def test_org_guidelines_rejects_oversized() -> None: + with pytest.raises(ValidationError): + TeamSettingsUpdate(org_guidelines="x" * (ORG_GUIDELINES_MAX_CHARS + 1)) + + +@pytest.mark.asyncio +async def test_get_org_review_guidelines_returns_trimmed_text() -> None: + with patch( + "agent.dashboard.team_settings.get_team_settings", + new_callable=AsyncMock, + return_value={"org_guidelines": " Always check auth.\n"}, + ): + assert await get_org_review_guidelines() == "Always check auth." + + +@pytest.mark.asyncio +async def test_get_org_review_guidelines_returns_none_when_unset() -> None: + with patch( + "agent.dashboard.team_settings.get_team_settings", + new_callable=AsyncMock, + return_value={"org_guidelines": None}, + ): + assert await get_org_review_guidelines() is None diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index a06955b5..3c96e7c6 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -109,6 +109,7 @@ export interface TeamSettings { review_trace_links: boolean; autofix_mode: AutofixMode; autofix_severity_threshold: AutofixMode; + org_guidelines?: string | null; default_agent_model?: string | null; default_agent_reasoning_effort?: string | null; default_agent_subagent_model?: string | null; diff --git a/ui/src/routes/review.tsx b/ui/src/routes/review.tsx index 69916595..851bcf15 100644 --- a/ui/src/routes/review.tsx +++ b/ui/src/routes/review.tsx @@ -13,8 +13,10 @@ import { SelectTrigger, SelectValue, } from "@/components/ui/select"; +import { Button } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; import { Switch } from "@/components/ui/switch"; +import { Textarea } from "@/components/ui/textarea"; import { ApiError, api } from "@/lib/api"; import { useSession } from "@/lib/session"; @@ -52,6 +54,7 @@ const DEFAULT_SETTINGS: TeamSettings = { review_trace_links: true, autofix_mode: "off", autofix_severity_threshold: "medium", + org_guidelines: null, default_agent_model: null, default_agent_reasoning_effort: null, default_agent_subagent_model: null, @@ -71,10 +74,14 @@ function ReviewPage() { enabled: !!session.data, }); const [local, setLocal] = useState(DEFAULT_SETTINGS); + const [guidelinesDraft, setGuidelinesDraft] = useState(""); const [error, setError] = useState(null); useEffect(() => { - if (settings.data) setLocal(settings.data); + if (settings.data) { + setLocal(settings.data); + setGuidelinesDraft(settings.data.org_guidelines ?? ""); + } }, [settings.data]); const save = useMutation({ @@ -108,6 +115,15 @@ function ReviewPage() { TRIGGER_MODES.find((m) => m.value === current.trigger_mode)?.description ?? "Open SWE Review will automatically review every push to a PR"; + const trimmedGuidelines = guidelinesDraft.trim(); + const savedGuidelines = current.org_guidelines ?? ""; + const guidelinesDirty = trimmedGuidelines !== savedGuidelines.trim(); + + const saveGuidelines = () => { + if (!canEdit) return; + persist({ org_guidelines: trimmedGuidelines || null }); + }; + return ( + +
+