From 7b9eff62e96d84c8d18a200e382663277256d906 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:51:17 -0400 Subject: [PATCH 1/2] fix: enforce terse Slack tool messages (#1717) (#197) (cherry picked from commit 092abafa4cb955c3823f727d35d7bad94e1147ab) Co-authored-by: open-swe[bot] Co-authored-by: Johannes du Plessis --- agent/prompt.py | 2 +- agent/tools/slack_thread_reply.py | 12 +++++++----- tests/test_github_comment_prompts.py | 14 ++++++++++++++ tests/test_slack_thread_reply_tool.py | 9 +++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/agent/prompt.py b/agent/prompt.py index 5998075a..2af21ed8 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -84,7 +84,7 @@ OPEN_SWE_SHARED_BASE = """You are **Open SWE**, an open-source agent built on La ### Communication - Focus on the substance and keep summaries brief. Use light markdown (`###`/`####` headings, bold, code) — avoid `#`/`##` titles. -- In Slack, keep every reply terse — a few sentences at most. Lead with the answer or outcome; skip preamble, restating the request, and step-by-step recaps. Do not paste long output, diffs, file listings, or multi-section write-ups into a Slack reply. When the response would genuinely be long — a detailed report, a design/analysis write-up, a large code or log excerpt — write that content to a Markdown file under `/workspace/plans/` and publish it with the `save_plan` tool, then post a terse Slack reply with a one-line summary and the plan-review link so the user can read the full version there. This non-plan share path does not enter plan mode; use it instead of splitting a long answer across multiple Slack messages. +- Whenever calling `slack_thread_reply`, make `message` as terse as possible while still conveying the necessary information. Default to one sentence containing only the outcome/status and link, or one blocking question. Omit greetings, preambles, headings, recaps, implementation details, and redundant context; use bullets only when multiple items are essential. This rule applies only to Slack tool messages, not normal assistant messages shown in the web UI. Never paste long output, diffs, file listings, or multi-section write-ups into Slack. When detail is necessary, write it to a Markdown file under `/workspace/plans/`, publish it with `save_plan`, and send only a one-line summary plus the plan-review link. This non-plan share path does not enter plan mode. - In Slack, when a user asks to “break out,” “split out,” or “start a separate thread” for part of the work, summarize the requested aspect and relevant context into self-contained instructions, then call `slack_start_new_thread` instead of only replying in the current thread. - In Slack, when acknowledging a user follow-up while you continue working, prefer `slack_add_reaction` with the default `eyes` reaction over posting a perfunctory “Updating…” / “I’ll check…” confirmation reply. - For Slack-triggered information-only answers, post only a concise summary in the associated Slack thread with `slack_thread_reply`, then provide the complete answer inline in your final assistant response. For other Slack updates, keep thread replies brief and avoid duplicating the same text later. diff --git a/agent/tools/slack_thread_reply.py b/agent/tools/slack_thread_reply.py index e9d64974..208dab5d 100644 --- a/agent/tools/slack_thread_reply.py +++ b/agent/tools/slack_thread_reply.py @@ -32,11 +32,13 @@ async def slack_thread_reply( ) -> dict[str, Any]: """Post a message to the current Slack thread. - Use this for clarifying questions, mid-run progress updates, and the final - summary. You can call this multiple times during a run — if you're about to - do long-running work (cloning, large refactors, big test runs) consider - posting a brief status update first so the user knows what's happening. - Always end the run with a final reply summarizing what you did. + Use this for clarifying questions, essential progress updates, and the final + outcome. Make `message` as terse as possible: default to one sentence with + only the outcome/status and link, or one blocking question. Omit greetings, + preambles, headings, recaps, implementation details, and redundant context; + use bullets only when multiple items are essential. This terseness rule is + specific to Slack tool messages, not normal web UI assistant messages. + Always end the run with a terse final outcome. Format messages using Slack's mrkdwn format, NOT standard Markdown. Key differences: *bold*, _italic_, ~strikethrough~, , diff --git a/tests/test_github_comment_prompts.py b/tests/test_github_comment_prompts.py index c4224f07..6dd04251 100644 --- a/tests/test_github_comment_prompts.py +++ b/tests/test_github_comment_prompts.py @@ -88,6 +88,20 @@ def test_construct_system_prompt_identifies_own_repo() -> None: assert "Open SWE" in OPEN_SWE_SHARED_BASE +def test_shared_base_requires_terse_slack_replies_with_share_path() -> None: + from agent.prompt import OPEN_SWE_SHARED_BASE + + assert "calling `slack_thread_reply`" in OPEN_SWE_SHARED_BASE + assert "as terse as possible" in OPEN_SWE_SHARED_BASE + assert "Default to one sentence" in OPEN_SWE_SHARED_BASE + assert "applies only to Slack tool messages" in OPEN_SWE_SHARED_BASE + assert "not normal assistant messages shown in the web UI" in OPEN_SWE_SHARED_BASE + assert "Never paste long output" in OPEN_SWE_SHARED_BASE + assert "`save_plan`" in OPEN_SWE_SHARED_BASE + assert "plan-review link" in OPEN_SWE_SHARED_BASE + assert "does not enter plan mode" in OPEN_SWE_SHARED_BASE + + def test_harness_profile_replaces_deepagents_base_for_supported_providers() -> None: """The Open SWE base prompt is registered per provider and replaces the SDK base.""" import deepagents.profiles.harness.harness_profiles as hp diff --git a/tests/test_slack_thread_reply_tool.py b/tests/test_slack_thread_reply_tool.py index cf76d5ff..b34353da 100644 --- a/tests/test_slack_thread_reply_tool.py +++ b/tests/test_slack_thread_reply_tool.py @@ -19,6 +19,15 @@ def _config() -> dict[str, Any]: } +def test_slack_thread_reply_prompt_requires_slack_only_terseness() -> None: + prompt = slack_reply_tool.slack_thread_reply.__doc__ or "" + + assert "as terse as possible" in prompt + assert "default to one sentence" in prompt + assert "specific to Slack tool messages" in prompt + assert "not normal web UI assistant messages" in prompt + + async def test_slack_thread_reply_returns_structured_error_for_msg_too_long( monkeypatch: pytest.MonkeyPatch, ) -> None: From 22c517a54fec6ddacfc7b016ea3580cfad3f174b Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:03:44 -0400 Subject: [PATCH 2/2] fix: stale admin model defaults after model upgrades (#1709) (#200) * fix: migrate stale admin model defaults Normalize retired model IDs before validation and in settings responses so full admin updates remain saveable after model upgrades. * fix: restrict retired model migration Only migrate explicitly retired model IDs so malformed provider model names and efforts continue to fail validation. (cherry picked from commit 62e0ca2d4898ebc3c2ae8887030a364779d907cb) Co-authored-by: Johannes du Plessis --- agent/dashboard/team_settings.py | 82 ++++++++++++++++++++++++- tests/test_model_fallback_resolution.py | 69 ++++++++++++++++++++- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/agent/dashboard/team_settings.py b/agent/dashboard/team_settings.py index 5c65b971..a484af47 100644 --- a/agent/dashboard/team_settings.py +++ b/agent/dashboard/team_settings.py @@ -110,6 +110,39 @@ class TeamSettingsUpdate(BaseModel): @model_validator(mode="after") def _validate_model_pairs(self) -> TeamSettingsUpdate: + self.default_agent_model, self.default_agent_reasoning_effort = _normalize_stale_model_pair( + self.default_agent_model, + self.default_agent_reasoning_effort, + ) + self.default_agent_subagent_model, self.default_agent_subagent_reasoning_effort = ( + _normalize_stale_model_pair( + self.default_agent_subagent_model, + self.default_agent_subagent_reasoning_effort, + ) + ) + self.default_reviewer_model, self.default_reviewer_reasoning_effort = ( + _normalize_stale_model_pair( + self.default_reviewer_model, + self.default_reviewer_reasoning_effort, + ) + ) + ( + self.default_reviewer_subagent_model, + self.default_reviewer_subagent_reasoning_effort, + ) = _normalize_stale_model_pair( + self.default_reviewer_subagent_model, + self.default_reviewer_subagent_reasoning_effort, + ) + self.default_grouping_model, self.default_grouping_reasoning_effort = ( + _normalize_stale_model_pair( + self.default_grouping_model, + self.default_grouping_reasoning_effort, + ) + ) + self.default_chat_model, self.default_chat_reasoning_effort = _normalize_stale_model_pair( + self.default_chat_model, + self.default_chat_reasoning_effort, + ) _validate_model_effort_pair( self.default_agent_model, self.default_agent_reasoning_effort, "agent" ) @@ -168,6 +201,53 @@ def _validate_model_effort_pair(model: str | None, effort: str | None, role: str raise ValueError(f"effort {effort!r} not supported by {role} model {model!r}") +# Model ids retired from SUPPORTED_MODELS mapped to their current successors, +# so stored admin defaults stay saveable/readable across model upgrades. Direct +# anthropic: ids moved to the Bedrock inference profile in the provider +# migration (#62); OpenAI/Google were dropped entirely and take the global +# default. anthropic:claude-fable-5 deliberately maps to Opus, not Bedrock +# Fable: Fable is gated behind the provider-data-share opt-in and a stale +# record must never resurface it. +_RETIRED_MODEL_REPLACEMENTS: dict[str, str] = { + "anthropic:claude-opus-4-7": "bedrock_converse:us.anthropic.claude-opus-4-8", + "anthropic:claude-opus-4-8": "bedrock_converse:us.anthropic.claude-opus-4-8", + "anthropic:claude-fable-5": "bedrock_converse:us.anthropic.claude-opus-4-8", + "openai:gpt-5.5": "bedrock_converse:us.anthropic.claude-opus-4-8", + "google_genai:gemini-3.5-flash": "bedrock_converse:us.anthropic.claude-opus-4-8", +} + + +def _normalize_stale_model_pair( + model: str | None, effort: str | None +) -> tuple[str | None, str | None]: + if model is None: + return model, effort + return _RETIRED_MODEL_REPLACEMENTS.get(model, model), effort + + +_MODEL_PAIR_FIELDS: tuple[tuple[str, str], ...] = ( + ("default_agent_model", "default_agent_reasoning_effort"), + ("default_agent_subagent_model", "default_agent_subagent_reasoning_effort"), + ("default_reviewer_model", "default_reviewer_reasoning_effort"), + ("default_reviewer_subagent_model", "default_reviewer_subagent_reasoning_effort"), + ("default_grouping_model", "default_grouping_reasoning_effort"), + ("default_chat_model", "default_chat_reasoning_effort"), +) + + +def normalize_team_settings_for_response(settings: dict[str, Any]) -> dict[str, Any]: + value = dict(settings) + for model_field, effort_field in _MODEL_PAIR_FIELDS: + model = value.get(model_field) + effort = value.get(effort_field) + if isinstance(model, str): + value[model_field], value[effort_field] = _normalize_stale_model_pair( + model, + effort if isinstance(effort, str) else None, + ) + return value + + def _client(): return get_client() @@ -241,7 +321,7 @@ async def get_team_settings() -> dict[str, Any]: "review_author_context_enabled", ): merged.pop(stale_field, None) - return merged + return normalize_team_settings_for_response(merged) async def upsert_team_settings(update: TeamSettingsUpdate) -> dict[str, Any]: diff --git a/tests/test_model_fallback_resolution.py b/tests/test_model_fallback_resolution.py index 8f0305f3..5b08ae3e 100644 --- a/tests/test_model_fallback_resolution.py +++ b/tests/test_model_fallback_resolution.py @@ -11,12 +11,18 @@ from agent.dashboard.options import ( gate_fable_model, provider_fallback_pair, ) -from agent.dashboard.team_settings import get_team_default_model +from agent.dashboard.team_settings import ( + TeamSettingsUpdate, + get_team_default_model, + normalize_team_settings_for_response, +) STALE_ANTHROPIC = "bedrock_converse:us.anthropic.claude-opus-4-7" SUPPORTED_ANTHROPIC = "bedrock_converse:us.anthropic.claude-opus-4-8" STALE_SONNET = "bedrock_converse:us.anthropic.claude-sonnet-4-9" SUPPORTED_SONNET = "bedrock_converse:us.anthropic.claude-sonnet-5" +RETIRED_DIRECT_ANTHROPIC = "anthropic:claude-opus-4-8" +RETIRED_FABLE = "anthropic:claude-fable-5" def test_provider_fallback_preserves_provider_and_effort() -> None: @@ -79,6 +85,67 @@ def test_profile_stale_anthropic_upgrades_to_supported() -> None: assert normalize_profile_overrides(profile) == (SUPPORTED_ANTHROPIC, "high") +def test_team_settings_update_normalizes_retired_models() -> None: + update = TeamSettingsUpdate( + default_agent_model=SUPPORTED_ANTHROPIC, + default_agent_reasoning_effort="medium", + default_agent_subagent_model=RETIRED_DIRECT_ANTHROPIC, + default_agent_subagent_reasoning_effort="medium", + default_reviewer_model=RETIRED_DIRECT_ANTHROPIC, + default_reviewer_reasoning_effort="medium", + default_reviewer_subagent_model=RETIRED_DIRECT_ANTHROPIC, + default_reviewer_subagent_reasoning_effort="low", + ) + + assert update.default_agent_subagent_model == SUPPORTED_ANTHROPIC + assert update.default_reviewer_model == SUPPORTED_ANTHROPIC + assert update.default_reviewer_subagent_model == SUPPORTED_ANTHROPIC + + +def test_team_settings_update_maps_retired_fable_to_opus_not_fable() -> None: + update = TeamSettingsUpdate( + fable_enabled=True, + default_agent_model=RETIRED_FABLE, + default_agent_reasoning_effort="high", + ) + + assert update.default_agent_model == SUPPORTED_ANTHROPIC + assert update.default_agent_model not in FABLE_MODEL_IDS + + +def test_team_settings_update_rejects_unknown_model() -> None: + with pytest.raises(ValueError, match="unsupported agent model"): + TeamSettingsUpdate( + default_agent_model="openai:gpt-6", + default_agent_reasoning_effort="medium", + ) + + +def test_team_settings_update_rejects_invalid_effort_for_retired_model() -> None: + with pytest.raises(ValueError, match="effort 'bogus' not supported"): + TeamSettingsUpdate( + default_agent_model=RETIRED_DIRECT_ANTHROPIC, + default_agent_reasoning_effort="bogus", + ) + + +def test_team_settings_response_normalizes_retired_models() -> None: + settings = normalize_team_settings_for_response( + { + "default_agent_subagent_model": RETIRED_DIRECT_ANTHROPIC, + "default_agent_subagent_reasoning_effort": "medium", + "default_reviewer_model": "openai:gpt-5.5", + "default_reviewer_reasoning_effort": "medium", + "default_reviewer_subagent_model": RETIRED_FABLE, + "default_reviewer_subagent_reasoning_effort": "low", + } + ) + + assert settings["default_agent_subagent_model"] == SUPPORTED_ANTHROPIC + assert settings["default_reviewer_model"] == SUPPORTED_ANTHROPIC + assert settings["default_reviewer_subagent_model"] == SUPPORTED_ANTHROPIC + + def test_profile_without_model_defers_to_team_default() -> None: assert normalize_profile_overrides({"reasoning_effort": "high"}) == (None, None)