mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:19:05 +08:00
fix(agent-profiles): restore resolved cloud launches
This commit is contained in:
@@ -841,6 +841,8 @@ class TestPersistedVsResolvedSettingsView:
|
||||
name='reviewer',
|
||||
llm_profile_ref='Default',
|
||||
mcp_server_refs=mcp_server_refs,
|
||||
tools=[],
|
||||
system_message_suffix='PROFILE_SUFFIX',
|
||||
),
|
||||
)
|
||||
await _set_agent_profiles(async_session_maker, org_id, ap)
|
||||
@@ -999,6 +1001,11 @@ class TestPersistedVsResolvedSettingsView:
|
||||
assert set(settings.agent_settings.mcp_config) == {'a'}
|
||||
# The resolved LLM is the referenced 'Default' org LLM profile.
|
||||
assert settings.agent_settings.llm.model == 'gpt-4o'
|
||||
assert settings.agent_settings.tools == []
|
||||
assert (
|
||||
settings.agent_settings.agent_context.system_message_suffix
|
||||
== 'PROFILE_SUFFIX'
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match='resolved Agent-Profile'):
|
||||
await store.store(settings)
|
||||
|
||||
@@ -131,7 +131,7 @@ from openhands.sdk.plugin import PluginSource
|
||||
from openhands.sdk.secret import LookupSecret, StaticSecret
|
||||
from openhands.sdk.settings import ACPAgentSettings
|
||||
from openhands.sdk.subagent import get_registered_agent_definitions
|
||||
from openhands.sdk.tool.builtins import SwitchLLMTool
|
||||
from openhands.sdk.tool import Tool
|
||||
from openhands.sdk.utils.redact import (
|
||||
redact_api_key_literals,
|
||||
redact_text_secrets,
|
||||
@@ -239,24 +239,48 @@ def append_system_context(existing: str | None, block: str) -> str:
|
||||
|
||||
|
||||
def effective_disabled_skills(user: UserInfo) -> list[str]:
|
||||
"""Union of the member-level and launched-profile-level skill deny-lists.
|
||||
|
||||
A skill disabled at EITHER level stays off. The member's deny-list rides
|
||||
``user.disabled_skills``; the launched Agent Profile's rides the resolved
|
||||
``agent_settings.agent_context.disabled_skills`` (the SDK resolver stamps the
|
||||
profile's ``disabled_skills`` there — #4017). On a non-profile launch the
|
||||
resolved context's deny-list is empty, so this is just the member's list.
|
||||
Order-preserving de-dup. Because it is a deny-list, a name absent from the
|
||||
discovered catalog is a harmless no-op, so no reconciliation is needed
|
||||
between the two sources.
|
||||
"""
|
||||
"""Combine member and profile skill deny-lists."""
|
||||
member = list(user.disabled_skills or [])
|
||||
agent_settings = getattr(user, 'agent_settings', None)
|
||||
agent_context = getattr(agent_settings, 'agent_context', None)
|
||||
profile = list(getattr(agent_context, 'disabled_skills', None) or [])
|
||||
agent_context = user.agent_settings.agent_context
|
||||
profile = list(agent_context.disabled_skills) if agent_context else []
|
||||
return list(dict.fromkeys([*member, *profile]))
|
||||
|
||||
|
||||
def _merge_launch_context(
|
||||
context: AgentContext | None,
|
||||
system_message_suffix: str | None,
|
||||
secrets: Mapping[str, Any] | None = None,
|
||||
disabled_skills: Sequence[str] | None = None,
|
||||
) -> AgentContext:
|
||||
fresh_context = AgentContext()
|
||||
context = context or fresh_context
|
||||
updates: dict[str, Any] = {
|
||||
'current_datetime': fresh_context.current_datetime,
|
||||
}
|
||||
if system_message_suffix:
|
||||
updates['system_message_suffix'] = append_system_context(
|
||||
context.system_message_suffix, system_message_suffix
|
||||
)
|
||||
if secrets:
|
||||
updates['secrets'] = {**(context.secrets or {}), **secrets}
|
||||
if disabled_skills is not None:
|
||||
effective_disabled = list(dict.fromkeys(disabled_skills))
|
||||
disabled = set(effective_disabled)
|
||||
updates['disabled_skills'] = effective_disabled
|
||||
updates['skills'] = [
|
||||
skill for skill in context.skills if skill.name not in disabled
|
||||
]
|
||||
return context.model_copy(update=updates)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class _ConversationLaunchSnapshot:
|
||||
user: UserInfo
|
||||
agent_profile_id: str | None
|
||||
agent_profile_revision: int | None
|
||||
disabled_skills: tuple[str, ...]
|
||||
|
||||
|
||||
@dataclass
|
||||
class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
"""AppConversationService which combines live status info from the sandbox with stored data."""
|
||||
@@ -286,6 +310,30 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
export_lock_refresh_interval_seconds: int = 30
|
||||
export_lock_required: bool | None = None
|
||||
|
||||
@staticmethod
|
||||
def _launch_snapshot_from_user(user: UserInfo) -> _ConversationLaunchSnapshot:
|
||||
profile_id = user.active_agent_profile_id
|
||||
profile_revision = user.active_agent_profile_revision
|
||||
return _ConversationLaunchSnapshot(
|
||||
user=user,
|
||||
agent_profile_id=profile_id
|
||||
if isinstance(profile_id, str) and profile_id
|
||||
else None,
|
||||
agent_profile_revision=profile_revision
|
||||
if isinstance(profile_revision, int)
|
||||
else None,
|
||||
disabled_skills=tuple(effective_disabled_skills(user)),
|
||||
)
|
||||
|
||||
async def _resolve_conversation_launch_snapshot(
|
||||
self, agent_profile_id: str | None
|
||||
) -> _ConversationLaunchSnapshot:
|
||||
user = await self.user_context.get_user_info(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=agent_profile_id,
|
||||
)
|
||||
return self._launch_snapshot_from_user(user)
|
||||
|
||||
def _maybe_append_shallow_clone_context(
|
||||
self,
|
||||
user: UserInfo,
|
||||
@@ -466,6 +514,9 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
'llm_model_override': request.llm_model,
|
||||
},
|
||||
)
|
||||
launch_snapshot = await self._resolve_conversation_launch_snapshot(
|
||||
request.agent_profile_id
|
||||
)
|
||||
|
||||
# Build the start request
|
||||
start_conversation_request = (
|
||||
@@ -485,6 +536,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
plugins=request.plugins,
|
||||
api_secrets=request.secrets,
|
||||
agent_profile_id=request.agent_profile_id,
|
||||
launch_snapshot=launch_snapshot,
|
||||
)
|
||||
)
|
||||
|
||||
@@ -548,25 +600,12 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
# archive captures the right directory without re-deriving the path
|
||||
# from settings (e.g. grouping) that may change before delete.
|
||||
tags[ARCHIVE_WORKSPACE_PATH_TAG_KEY] = working_dir
|
||||
# Stamp Agent Profile provenance. The launched profile resolved into
|
||||
# the launched ``agent_settings`` (a resolve-requested load carries
|
||||
# its id + revision onto UserInfo); ride the tags dict so it
|
||||
# round-trips and surfaces as the ``launched_agent_profile``
|
||||
# computed field. Resolves with the same override the launch itself
|
||||
# used, so provenance reflects what actually ran even when the
|
||||
# request carried a one-off ``agent_profile_id``.
|
||||
profile_user = await self.user_context.get_user_info(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=request.agent_profile_id,
|
||||
)
|
||||
launched_profile_id = getattr(profile_user, 'active_agent_profile_id', None)
|
||||
if isinstance(launched_profile_id, str) and launched_profile_id:
|
||||
tags[AGENT_PROFILE_ID_TAG_KEY] = launched_profile_id
|
||||
launched_revision = getattr(
|
||||
profile_user, 'active_agent_profile_revision', None
|
||||
)
|
||||
if isinstance(launched_revision, int):
|
||||
tags[AGENT_PROFILE_REVISION_TAG_KEY] = str(launched_revision)
|
||||
if launch_snapshot.agent_profile_id:
|
||||
tags[AGENT_PROFILE_ID_TAG_KEY] = launch_snapshot.agent_profile_id
|
||||
if launch_snapshot.agent_profile_revision is not None:
|
||||
tags[AGENT_PROFILE_REVISION_TAG_KEY] = str(
|
||||
launch_snapshot.agent_profile_revision
|
||||
)
|
||||
if request_agent.agent_kind == 'acp':
|
||||
llm_model = request_agent.acp_model
|
||||
agent_kind = 'acp'
|
||||
@@ -574,12 +613,10 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
# can resolve a brand label ("Claude Code", "Codex", …) via
|
||||
# the SDK registry without keeping a per-conversation column.
|
||||
# Surfaced to the UI as the projected ``acp_server`` field.
|
||||
# Reuses ``profile_user`` (resolved above with the same
|
||||
# override) rather than re-fetching — a second fetch would
|
||||
# both double the settings-resolution cost and risk a
|
||||
# different profile resolving if it changed in between.
|
||||
if isinstance(profile_user.agent_settings, ACPAgentSettings):
|
||||
tags[ACP_SERVER_TAG_KEY] = profile_user.agent_settings.acp_server
|
||||
if isinstance(launch_snapshot.user.agent_settings, ACPAgentSettings):
|
||||
tags[ACP_SERVER_TAG_KEY] = (
|
||||
launch_snapshot.user.agent_settings.acp_server
|
||||
)
|
||||
else:
|
||||
llm_model = request_agent.llm.model
|
||||
agent_kind = 'openhands'
|
||||
@@ -1883,6 +1920,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
plugins: list[PluginSpec] | None = None,
|
||||
api_secrets: dict[str, SecretStr] | None = None,
|
||||
agent_profile_id: str | None = None,
|
||||
launch_snapshot: _ConversationLaunchSnapshot | None = None,
|
||||
) -> StartConversationRequest:
|
||||
"""Build a complete StartConversationRequest for a user.
|
||||
|
||||
@@ -1919,10 +1957,10 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
# Conversation start builds the agent, so it consumes the RESOLVED
|
||||
# (effective launch) view; plain settings reads/round-trips elsewhere
|
||||
# stay on the persisted view.
|
||||
user = await self.user_context.get_user_info(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=agent_profile_id,
|
||||
launch_snapshot = launch_snapshot or (
|
||||
await self._resolve_conversation_launch_snapshot(agent_profile_id)
|
||||
)
|
||||
user = launch_snapshot.user
|
||||
llm_settings = getattr(user.agent_settings, 'llm', None)
|
||||
_logger.debug(
|
||||
'managed_llm_key_refresh:build_request_context',
|
||||
@@ -1966,7 +2004,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
remote_workspace=remote_workspace,
|
||||
plugins=plugins,
|
||||
api_secrets=api_secrets,
|
||||
agent_profile_id=agent_profile_id,
|
||||
launch_snapshot=launch_snapshot,
|
||||
)
|
||||
if remote_workspace:
|
||||
acp_request = await self._load_skills_onto_request(
|
||||
@@ -1975,7 +2013,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
remote_workspace,
|
||||
selected_repository,
|
||||
get_project_dir(working_dir, selected_repository),
|
||||
effective_disabled_skills(user),
|
||||
list(launch_snapshot.disabled_skills),
|
||||
registered_marketplaces,
|
||||
)
|
||||
return acp_request
|
||||
@@ -2040,57 +2078,43 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
|
||||
# --- tools ----------------------------------------------------------
|
||||
agent_definitions: list[Any] = []
|
||||
tools: list[Tool]
|
||||
if agent_type == AgentType.PLAN:
|
||||
# Planning launches always use the constrained planning toolset.
|
||||
plan_path = None
|
||||
if project_dir:
|
||||
plan_path = self._compute_plan_path(project_dir, git_provider)
|
||||
tools = get_planning_tools(plan_path=plan_path)
|
||||
else:
|
||||
register_builtins_agents(enable_browser=True)
|
||||
tools = get_default_tools(
|
||||
enable_browser=True,
|
||||
enable_sub_agents=user.agent_settings.enable_sub_agents,
|
||||
)
|
||||
profile_tools = user.agent_settings.tools
|
||||
if profile_tools is None:
|
||||
tools = get_default_tools(
|
||||
enable_browser=True,
|
||||
enable_sub_agents=user.agent_settings.enable_sub_agents,
|
||||
)
|
||||
else:
|
||||
tools = profile_tools
|
||||
if user.agent_settings.enable_sub_agents:
|
||||
agent_definitions = list(get_registered_agent_definitions())
|
||||
|
||||
# --- build AgentSettings and create agent ---------------------------
|
||||
agent_context = _merge_launch_context(
|
||||
user.agent_settings.agent_context,
|
||||
effective_suffix,
|
||||
secrets,
|
||||
disabled_skills=launch_snapshot.disabled_skills,
|
||||
)
|
||||
configured_agent_settings = user.agent_settings.model_copy(
|
||||
update={
|
||||
'llm': llm,
|
||||
'tools': tools,
|
||||
'mcp_config': mcp_config if mcp_config else {},
|
||||
'agent_context': AgentContext(
|
||||
system_message_suffix=effective_suffix,
|
||||
secrets=secrets,
|
||||
),
|
||||
'agent_context': agent_context,
|
||||
}
|
||||
)
|
||||
agent = configured_agent_settings.create_agent()
|
||||
|
||||
# SaaS profiles live on the user/org record, not the sandbox
|
||||
# filesystem, so we attach the agent's built-in switch_llm tool
|
||||
# ourselves rather than relying on create_agent()'s gating. Enabled
|
||||
# whenever there are at least two valid saved profiles (a switch needs
|
||||
# a target).
|
||||
valid_profile_names = [
|
||||
name
|
||||
for name in user.llm_profiles.profiles
|
||||
if PROFILE_NAME_REGEX.match(name)
|
||||
]
|
||||
if (
|
||||
len(valid_profile_names) >= 2
|
||||
and SwitchLLMTool.__name__ not in agent.include_default_tools
|
||||
):
|
||||
agent = agent.model_copy(
|
||||
update={
|
||||
'include_default_tools': [
|
||||
*agent.include_default_tools,
|
||||
SwitchLLMTool.__name__,
|
||||
]
|
||||
}
|
||||
)
|
||||
|
||||
agent = self._apply_server_agent_overrides(
|
||||
agent,
|
||||
agent_type,
|
||||
@@ -2198,7 +2222,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
remote_workspace,
|
||||
selected_repository,
|
||||
project_dir,
|
||||
effective_disabled_skills(user),
|
||||
list(launch_snapshot.disabled_skills),
|
||||
registered_marketplaces,
|
||||
)
|
||||
|
||||
@@ -2268,6 +2292,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
conversation_id: UUID,
|
||||
initial_message: SendMessageRequest | None,
|
||||
working_dir: str,
|
||||
launch_snapshot: _ConversationLaunchSnapshot,
|
||||
system_message_suffix: str | None = None,
|
||||
trigger: ConversationTrigger | None = None,
|
||||
git_provider: ProviderType | None = None,
|
||||
@@ -2276,7 +2301,6 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
remote_workspace: AsyncRemoteWorkspace | None = None,
|
||||
plugins: list[PluginSpec] | None = None,
|
||||
api_secrets: dict[str, SecretStr] | None = None,
|
||||
agent_profile_id: str | None = None,
|
||||
) -> StartConversationRequest:
|
||||
"""Build a StartConversationRequest for ACP agent conversations.
|
||||
|
||||
@@ -2304,14 +2328,9 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
resolve the HEAD commit for the Laminar trace metadata.
|
||||
plugins: Optional list of plugins to load
|
||||
api_secrets: Optional secrets passed directly via the API.
|
||||
agent_profile_id: One-off Agent Profile override for this
|
||||
conversation only (cloud-only; does not change the member's
|
||||
active pointer). ``None`` uses the ambient active profile.
|
||||
launch_snapshot: Resolved user and Agent Profile provenance.
|
||||
"""
|
||||
user = await self.user_context.get_user_info(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=agent_profile_id,
|
||||
)
|
||||
user = launch_snapshot.user
|
||||
|
||||
project_dir = get_project_dir(working_dir, selected_repository)
|
||||
workspace = LocalWorkspace(working_dir=project_dir)
|
||||
@@ -2399,10 +2418,11 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
self._merge_custom_mcp_config(acp_mcp_servers, user)
|
||||
if acp_mcp_servers:
|
||||
settings_update['mcp_config'] = acp_mcp_servers
|
||||
if system_message_suffix:
|
||||
settings_update['agent_context'] = AgentContext(
|
||||
system_message_suffix=system_message_suffix
|
||||
)
|
||||
settings_update['agent_context'] = _merge_launch_context(
|
||||
acp_settings.agent_context,
|
||||
system_message_suffix,
|
||||
disabled_skills=launch_snapshot.disabled_skills,
|
||||
)
|
||||
acp_settings_for_agent = acp_settings.model_copy(update=settings_update)
|
||||
acp_agent = acp_settings_for_agent.create_agent()
|
||||
|
||||
|
||||
@@ -33,6 +33,7 @@ from openhands.app_server.app_conversation.app_conversation_service import (
|
||||
from openhands.app_server.app_conversation.live_status_app_conversation_service import (
|
||||
LiveStatusAppConversationService,
|
||||
_exception_detail,
|
||||
_merge_launch_context,
|
||||
_resolve_title_llm_profile,
|
||||
effective_disabled_skills,
|
||||
)
|
||||
@@ -56,8 +57,11 @@ from openhands.app_server.user.user_context import UserContext
|
||||
from openhands.app_server.utils.redis_lock import RedisLockUnavailable
|
||||
from openhands.sdk import Agent, AgentContext, Event
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.profiles import OpenHandsAgentProfile, resolve_agent_profile
|
||||
from openhands.sdk.secret import LookupSecret, StaticSecret
|
||||
from openhands.sdk.settings import ConversationSettings, OpenHandsAgentSettings
|
||||
from openhands.sdk.skills import Skill
|
||||
from openhands.sdk.tool import Tool
|
||||
from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace
|
||||
|
||||
|
||||
@@ -105,6 +109,12 @@ def _build_test_user_agent_settings(user: SimpleNamespace) -> OpenHandsAgentSett
|
||||
|
||||
|
||||
class _TestUserInfo(SimpleNamespace):
|
||||
def __init__(self, **kwargs):
|
||||
kwargs.setdefault('disabled_skills', [])
|
||||
kwargs.setdefault('active_agent_profile_id', None)
|
||||
kwargs.setdefault('active_agent_profile_revision', None)
|
||||
super().__init__(**kwargs)
|
||||
|
||||
@property
|
||||
def agent_settings(self) -> OpenHandsAgentSettings:
|
||||
override = getattr(self, '_agent_settings_override', None)
|
||||
@@ -158,6 +168,46 @@ class _TestUserInfo(SimpleNamespace):
|
||||
return self.agent_settings
|
||||
|
||||
|
||||
class _TestLLMProfileLoader:
|
||||
def load(self, name, *, cipher=None):
|
||||
assert name == 'Default'
|
||||
return LLM(model='gpt-4', api_key=SecretStr('test-key'))
|
||||
|
||||
|
||||
def _resolved_profile_user(
|
||||
*,
|
||||
tools=None,
|
||||
system_message_suffix='PROFILE_SUFFIX',
|
||||
disabled_skills=None,
|
||||
available_skills=None,
|
||||
enable_switch_llm_tool=True,
|
||||
):
|
||||
profile = OpenHandsAgentProfile(
|
||||
name='reviewer',
|
||||
revision=7,
|
||||
llm_profile_ref='Default',
|
||||
tools=tools,
|
||||
system_message_suffix=system_message_suffix,
|
||||
disabled_skills=disabled_skills or [],
|
||||
enable_switch_llm_tool=enable_switch_llm_tool,
|
||||
)
|
||||
settings = resolve_agent_profile(
|
||||
profile,
|
||||
llm_store=_TestLLMProfileLoader(),
|
||||
mcp_config={},
|
||||
available_skills=available_skills or [],
|
||||
)
|
||||
user = _TestUserInfo(
|
||||
id='test_user_123',
|
||||
disabled_skills=[],
|
||||
git_full_clone=True,
|
||||
)
|
||||
user.agent_settings = settings
|
||||
user.active_agent_profile_id = str(profile.id)
|
||||
user.active_agent_profile_revision = profile.revision
|
||||
return user
|
||||
|
||||
|
||||
class TestEffectiveDisabledSkills:
|
||||
"""effective_disabled_skills() unions the member- and profile-level deny-lists.
|
||||
|
||||
@@ -198,6 +248,43 @@ class TestEffectiveDisabledSkills:
|
||||
assert effective_disabled_skills(self._user([], [])) == []
|
||||
|
||||
|
||||
class TestConversationLaunchSnapshot:
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
'override_id', [None, '11111111-1111-1111-1111-111111111111']
|
||||
)
|
||||
async def test_resolves_ambient_or_requested_profile_once(self, override_id):
|
||||
user = _resolved_profile_user()
|
||||
user_context = Mock(spec=UserContext)
|
||||
user_context.get_user_info = AsyncMock(return_value=user)
|
||||
service = LiveStatusAppConversationService.__new__(
|
||||
LiveStatusAppConversationService
|
||||
)
|
||||
service.user_context = user_context
|
||||
|
||||
snapshot = await service._resolve_conversation_launch_snapshot(override_id)
|
||||
|
||||
assert snapshot.user is user
|
||||
assert snapshot.agent_profile_id == user.active_agent_profile_id
|
||||
assert snapshot.agent_profile_revision == 7
|
||||
user_context.get_user_info.assert_awaited_once_with(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=override_id,
|
||||
)
|
||||
|
||||
def test_merge_refreshes_persisted_current_datetime(self):
|
||||
persisted = datetime(2020, 1, 1)
|
||||
context = AgentContext(
|
||||
current_datetime=persisted,
|
||||
user_message_suffix='PERSISTED_SUFFIX',
|
||||
)
|
||||
|
||||
merged = _merge_launch_context(context, None)
|
||||
|
||||
assert merged.current_datetime != persisted
|
||||
assert merged.user_message_suffix == 'PERSISTED_SUFFIX'
|
||||
|
||||
|
||||
# Env var used by openhands SDK LLM to skip context-window validation (e.g. for gpt-4 in tests)
|
||||
_ALLOW_SHORT_CONTEXT_WINDOWS = 'ALLOW_SHORT_CONTEXT_WINDOWS'
|
||||
|
||||
@@ -1437,6 +1524,159 @@ class TestLiveStatusAppConversationService:
|
||||
self.mock_user, 'gpt-4', test_conversation_id
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
('profile_tools', 'expected_tools', 'uses_cloud_defaults'),
|
||||
[
|
||||
(None, [Tool(name='browser')], True),
|
||||
([], [], False),
|
||||
(
|
||||
[Tool(name='terminal', params={'path': '/workspace'})],
|
||||
[Tool(name='terminal', params={'path': '/workspace'})],
|
||||
False,
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_resolved_profile_tools_are_tri_state(
|
||||
self, profile_tools, expected_tools, uses_cloud_defaults
|
||||
):
|
||||
user = _resolved_profile_user(tools=profile_tools)
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=user)
|
||||
self.service._setup_conversation_secrets = AsyncMock(return_value=({}, None))
|
||||
self.service._configure_llm_and_mcp = AsyncMock(
|
||||
return_value=(user.agent_settings.llm, {})
|
||||
)
|
||||
|
||||
with patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools',
|
||||
return_value=[Tool(name='browser')],
|
||||
) as mock_defaults:
|
||||
result = await self.service._build_start_conversation_request_for_user(
|
||||
sandbox=self.mock_sandbox,
|
||||
conversation_id=uuid4(),
|
||||
initial_message=None,
|
||||
system_message_suffix=None,
|
||||
git_provider=None,
|
||||
working_dir='/test/dir',
|
||||
)
|
||||
|
||||
assert result.agent.tools == expected_tools
|
||||
assert mock_defaults.called is uses_cloud_defaults
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolved_profile_can_disable_switch_llm_tool(self):
|
||||
user = _resolved_profile_user(
|
||||
tools=[],
|
||||
enable_switch_llm_tool=False,
|
||||
)
|
||||
user.llm_profiles = LLMProfiles(
|
||||
profiles={
|
||||
'first': LLM(model='openai/gpt-4o', usage_id='first'),
|
||||
'second': LLM(model='openai/gpt-4.1', usage_id='second'),
|
||||
}
|
||||
)
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=user)
|
||||
self.service._setup_conversation_secrets = AsyncMock(return_value=({}, None))
|
||||
self.service._configure_llm_and_mcp = AsyncMock(
|
||||
return_value=(user.agent_settings.llm, {})
|
||||
)
|
||||
|
||||
result = await self.service._build_start_conversation_request_for_user(
|
||||
sandbox=self.mock_sandbox,
|
||||
conversation_id=uuid4(),
|
||||
initial_message=None,
|
||||
system_message_suffix=None,
|
||||
git_provider=None,
|
||||
working_dir='/test/dir',
|
||||
)
|
||||
|
||||
assert 'SwitchLLMTool' not in result.agent.include_default_tools
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolved_profile_context_is_launch_base(self):
|
||||
profile_skill = Skill(name='profile-skill', content='Profile skill content')
|
||||
member_disabled_skill = Skill(
|
||||
name='member-disabled-skill', content='Member-disabled skill content'
|
||||
)
|
||||
user = _resolved_profile_user(
|
||||
tools=[],
|
||||
disabled_skills=['disabled-profile-skill'],
|
||||
available_skills=[profile_skill, member_disabled_skill],
|
||||
)
|
||||
user.disabled_skills = ['member-disabled-skill']
|
||||
profile_secret = StaticSecret(value=SecretStr('profile-value'))
|
||||
profile_context = user.agent_settings.agent_context.model_copy(
|
||||
update={
|
||||
'user_message_suffix': 'PROFILE_USER_SUFFIX',
|
||||
'secrets': {'PROFILE_SECRET': profile_secret},
|
||||
}
|
||||
)
|
||||
user.agent_settings = user.agent_settings.model_copy(
|
||||
update={'agent_context': profile_context}
|
||||
)
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=user)
|
||||
launch_secret = StaticSecret(value=SecretStr('launch-value'))
|
||||
self.service._setup_conversation_secrets = AsyncMock(
|
||||
return_value=({'LAUNCH_SECRET': launch_secret}, 'REQUEST_SUFFIX')
|
||||
)
|
||||
self.service._configure_llm_and_mcp = AsyncMock(
|
||||
return_value=(user.agent_settings.llm, {})
|
||||
)
|
||||
|
||||
result = await self.service._build_start_conversation_request_for_user(
|
||||
sandbox=self.mock_sandbox,
|
||||
conversation_id=uuid4(),
|
||||
initial_message=None,
|
||||
system_message_suffix='ignored-by-mock',
|
||||
git_provider=None,
|
||||
working_dir='/test/dir',
|
||||
)
|
||||
|
||||
context = result.agent.agent_context
|
||||
suffix = context.system_message_suffix
|
||||
assert suffix.index('PROFILE_SUFFIX') < suffix.index('REQUEST_SUFFIX')
|
||||
assert suffix.index('REQUEST_SUFFIX') < suffix.index('<HOST>')
|
||||
assert context.user_message_suffix == 'PROFILE_USER_SUFFIX'
|
||||
assert [loaded.name for loaded in context.skills] == ['profile-skill']
|
||||
assert context.disabled_skills == [
|
||||
'member-disabled-skill',
|
||||
'disabled-profile-skill',
|
||||
]
|
||||
assert context.secrets['PROFILE_SECRET'] is profile_secret
|
||||
assert context.secrets['LAUNCH_SECRET'] is launch_secret
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_planning_launch_uses_planning_tools_over_profile_tools(self):
|
||||
user = _resolved_profile_user(tools=[])
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=user)
|
||||
self.service._setup_conversation_secrets = AsyncMock(return_value=({}, None))
|
||||
self.service._configure_llm_and_mcp = AsyncMock(
|
||||
return_value=(user.agent_settings.llm, {})
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools'
|
||||
) as mock_defaults,
|
||||
patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_tools',
|
||||
return_value=[Tool(name='task_tracker')],
|
||||
) as mock_planning,
|
||||
):
|
||||
result = await self.service._build_start_conversation_request_for_user(
|
||||
sandbox=self.mock_sandbox,
|
||||
conversation_id=uuid4(),
|
||||
initial_message=None,
|
||||
system_message_suffix=None,
|
||||
git_provider=None,
|
||||
working_dir='/test/dir',
|
||||
agent_type=AgentType.PLAN,
|
||||
)
|
||||
|
||||
assert [tool.name for tool in result.agent.tools] == ['task_tracker']
|
||||
mock_planning.assert_called_once()
|
||||
mock_defaults.assert_not_called()
|
||||
|
||||
@patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools',
|
||||
return_value=[],
|
||||
@@ -1609,6 +1849,9 @@ class TestLiveStatusAppConversationService:
|
||||
acp_server='claude-code',
|
||||
llm=LLM(model='claude-sonnet-4-5', api_key=SecretStr('k')),
|
||||
)
|
||||
profile_id = str(uuid4())
|
||||
self.mock_user.active_agent_profile_id = profile_id
|
||||
self.mock_user.active_agent_profile_revision = 5
|
||||
self.mock_user_context.get_user_info.return_value = self.mock_user
|
||||
self.mock_user_context.get_secrets = AsyncMock(return_value={})
|
||||
self.mock_user_context.get_provider_tokens = AsyncMock(return_value=None)
|
||||
@@ -1628,6 +1871,7 @@ class TestLiveStatusAppConversationService:
|
||||
remote_workspace=remote_workspace,
|
||||
selected_repository='test/repo',
|
||||
selected_branch='feature-x',
|
||||
agent_profile_id=profile_id,
|
||||
)
|
||||
|
||||
assert result.agent.agent_kind == 'acp'
|
||||
@@ -1642,6 +1886,10 @@ class TestLiveStatusAppConversationService:
|
||||
'git_provider': 'github',
|
||||
'commit': 'def456sha',
|
||||
}
|
||||
self.mock_user_context.get_user_info.assert_awaited_once_with(
|
||||
resolve_agent_profile=True,
|
||||
override_agent_profile_id=profile_id,
|
||||
)
|
||||
|
||||
@patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools',
|
||||
@@ -2440,6 +2688,9 @@ class TestLiveStatusAppConversationService:
|
||||
|
||||
# Mock user context
|
||||
self.mock_user_context.get_user_id = AsyncMock(return_value='test_user_123')
|
||||
ambient_profile_id = str(uuid4())
|
||||
self.mock_user.active_agent_profile_id = ambient_profile_id
|
||||
self.mock_user.active_agent_profile_revision = 3
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=self.mock_user)
|
||||
|
||||
# Mock sandbox and sandbox spec
|
||||
@@ -2526,6 +2777,15 @@ class TestLiveStatusAppConversationService:
|
||||
f'but got "{saved_info.title}"'
|
||||
)
|
||||
assert saved_info.id == conversation_id
|
||||
assert saved_info.tags['agentprofileid'] == ambient_profile_id
|
||||
assert saved_info.tags['agentprofilerevision'] == '3'
|
||||
resolved_calls = [
|
||||
call
|
||||
for call in self.mock_user_context.get_user_info.await_args_list
|
||||
if call.kwargs.get('resolve_agent_profile')
|
||||
]
|
||||
assert len(resolved_calls) == 1
|
||||
assert resolved_calls[0].kwargs['override_agent_profile_id'] is None
|
||||
|
||||
@patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.AsyncRemoteWorkspace'
|
||||
@@ -2545,6 +2805,9 @@ class TestLiveStatusAppConversationService:
|
||||
acp_server='claude-code',
|
||||
llm=LLM(model='claude-sonnet-4-5', api_key=SecretStr('sk-ui-key')),
|
||||
)
|
||||
requested_profile_id = str(uuid4())
|
||||
self.mock_user.active_agent_profile_id = requested_profile_id
|
||||
self.mock_user.active_agent_profile_revision = 11
|
||||
self.mock_user_context.get_user_id = AsyncMock(return_value='test_user_123')
|
||||
self.mock_user_context.get_user_info = AsyncMock(return_value=self.mock_user)
|
||||
|
||||
@@ -2603,6 +2866,7 @@ class TestLiveStatusAppConversationService:
|
||||
selected_repository='OpenHands/OpenHands',
|
||||
selected_branch='main',
|
||||
git_provider=ProviderType.GITHUB,
|
||||
agent_profile_id=requested_profile_id,
|
||||
)
|
||||
|
||||
async for _ in self.service._start_app_conversation(request):
|
||||
@@ -2615,6 +2879,18 @@ class TestLiveStatusAppConversationService:
|
||||
assert saved_info.tags['repo_name'] == 'OpenHands/OpenHands'
|
||||
assert saved_info.tags['git_provider'] == 'github'
|
||||
assert saved_info.tags['selected_branch'] == 'main'
|
||||
assert saved_info.tags['agentprofileid'] == requested_profile_id
|
||||
assert saved_info.tags['agentprofilerevision'] == '11'
|
||||
resolved_calls = [
|
||||
call
|
||||
for call in self.mock_user_context.get_user_info.await_args_list
|
||||
if call.kwargs.get('resolve_agent_profile')
|
||||
]
|
||||
assert len(resolved_calls) == 1
|
||||
assert (
|
||||
resolved_calls[0].kwargs['override_agent_profile_id']
|
||||
== requested_profile_id
|
||||
)
|
||||
|
||||
@patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.AsyncRemoteWorkspace'
|
||||
@@ -4300,9 +4576,9 @@ class TestBuildAcpStartConversationRequestSecrets:
|
||||
selected_repository=None,
|
||||
selected_branch=None,
|
||||
remote_workspace=None,
|
||||
system_message_suffix=None,
|
||||
):
|
||||
"""Wire user_context and call _build_acp_start_conversation_request."""
|
||||
service.user_context.get_user_info = AsyncMock(return_value=user)
|
||||
service.user_context.get_user_email = AsyncMock(return_value=None)
|
||||
service.user_context.get_secrets = AsyncMock(return_value=secrets or {})
|
||||
service.user_context.get_provider_tokens = AsyncMock(return_value=None)
|
||||
@@ -4312,6 +4588,8 @@ class TestBuildAcpStartConversationRequestSecrets:
|
||||
conversation_id=uuid4(),
|
||||
initial_message=None,
|
||||
working_dir=str(tmp_path),
|
||||
launch_snapshot=service._launch_snapshot_from_user(user),
|
||||
system_message_suffix=system_message_suffix,
|
||||
git_provider=git_provider,
|
||||
selected_repository=selected_repository,
|
||||
selected_branch=selected_branch,
|
||||
@@ -4441,6 +4719,50 @@ class TestBuildAcpStartConversationRequestSecrets:
|
||||
# No panel secrets → request.secrets is empty (agent_context.secrets is a separate channel).
|
||||
assert 'ANTHROPIC_API_KEY' not in request.secrets
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize('system_message_suffix', [None, 'SHALLOW_CLONE_SUFFIX'])
|
||||
async def test_disabled_skills_reach_runtime_context(
|
||||
self, service, tmp_path, system_message_suffix
|
||||
):
|
||||
user = self._make_acp_user()
|
||||
user.disabled_skills = ['disabled-project-skill']
|
||||
user.agent_settings = user.agent_settings.model_copy(
|
||||
update={
|
||||
'agent_context': AgentContext(
|
||||
current_datetime=None,
|
||||
load_project_skills=True,
|
||||
system_message_suffix='PROFILE_SUFFIX',
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
request = await self._call_build(
|
||||
service,
|
||||
user,
|
||||
tmp_path,
|
||||
system_message_suffix=system_message_suffix,
|
||||
)
|
||||
|
||||
context = request.agent.agent_context
|
||||
assert context is not None
|
||||
assert isinstance(context.current_datetime, datetime)
|
||||
assert context.load_project_skills is True
|
||||
assert context.disabled_skills == ['disabled-project-skill']
|
||||
assert context.system_message_suffix.startswith('PROFILE_SUFFIX')
|
||||
if system_message_suffix:
|
||||
assert context.system_message_suffix.endswith(system_message_suffix)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_without_context_gets_launch_datetime(self, service, tmp_path):
|
||||
user = self._make_acp_user()
|
||||
|
||||
request = await self._call_build(service, user, tmp_path)
|
||||
|
||||
context = request.agent.agent_context
|
||||
assert context is not None
|
||||
assert isinstance(context.current_datetime, datetime)
|
||||
assert context.disabled_skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_secrets_forwarded_via_request_secrets(self, service, tmp_path):
|
||||
"""Panel secrets flow through request.secrets; not pre-resolved into agent_context."""
|
||||
|
||||
Reference in New Issue
Block a user