From c0abfc936cbfa8601f7270f0bd9ed4621cc8e7b3 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 14 Sep 2026 16:30:33 +0000 Subject: [PATCH 1/7] feat: dispatch automations through conversation runtimes Co-authored-by: openhands --- README.md | 41 ++++ .../versions/025_add_execution_scope.py | 40 ++++ openhands/automation/backends/__init__.py | 84 +++---- openhands/automation/backends/base.py | 86 ++++--- openhands/automation/backends/conversation.py | 191 +++++++++++++++ .../automation/backends/providers/__init__.py | 14 ++ .../backends/{ => providers}/cloud.py | 27 +-- .../{local.py => providers/existing.py} | 23 +- openhands/automation/backends/run.py | 48 ++++ openhands/automation/config.py | 6 +- openhands/automation/dispatcher.py | 72 +++++- openhands/automation/execution.py | 109 +++++---- openhands/automation/git_sync/loop.py | 4 + openhands/automation/git_sync/serializer.py | 1 + openhands/automation/models.py | 16 ++ openhands/automation/router.py | 13 ++ openhands/automation/schemas.py | 10 + openhands/automation/utils/agent_server.py | 120 +++++----- .../automation/utils/conversation_outcome.py | 64 ++++-- .../automation/utils/conversation_turn.py | 62 +++-- openhands/automation/utils/run.py | 1 + openhands/automation/utils/sandbox.py | 2 +- openhands/automation/utils/webhook.py | 1 + openhands/automation/watchdog.py | 51 ++-- pyproject.toml | 3 +- tests/conftest.py | 25 ++ tests/test_backends.py | 130 ++++++----- tests/test_cancel_run.py | 35 +++ tests/test_conversation_backend.py | 217 ++++++++++++++++++ tests/test_conversation_outcome.py | 100 ++++---- tests/test_conversation_turn.py | 216 +++++++++++------ tests/test_conversations.py | 43 +++- tests/test_db.py | 4 + tests/test_disable_automation.py | 6 +- tests/test_dispatcher.py | 158 ++++++++++++- tests/test_execution.py | 168 +++++++------- tests/test_local_mode.py | 199 ++++------------ tests/test_watchdog.py | 113 ++++++++- uv.lock | 10 +- 39 files changed, 1782 insertions(+), 731 deletions(-) create mode 100644 migrations/versions/025_add_execution_scope.py create mode 100644 openhands/automation/backends/conversation.py create mode 100644 openhands/automation/backends/providers/__init__.py rename openhands/automation/backends/{ => providers}/cloud.py (95%) rename openhands/automation/backends/{local.py => providers/existing.py} (93%) create mode 100644 openhands/automation/backends/run.py create mode 100644 tests/test_conversation_backend.py diff --git a/README.md b/README.md index 52c4404d..f4fc609c 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,36 @@ The Automation Service owns automation definitions, cron scheduling, webhooks, r ## Development +### Conversation execution in local or Docker workspaces + +Set `AUTOMATION_AGENT_SERVER_URL`, `AUTOMATION_AGENT_SERVER_API_KEY`, and +`AUTOMATION_AGENT_PROFILE` (a saved agent profile UUID). The backend reads the +server's authoritative `conversation_runtime`. Definitions with +`execution_scope: "conversation"` provision a conversation using the same API +and profile in either workspace mode; definitions default to +`execution_scope: "run"` and run their entrypoint directly. Profile +configuration never selects the execution scope. No bundle configuration or +workflow branch changes when switching workspace kind. + +Both workspace kinds supply `AUTOMATION_CONVERSATION_ID`, `AGENT_SERVER_URL`, +`SESSION_API_KEY`, and `WORKSPACE_BASE`, and use conversation-scoped upload, +bash execution, and completion verification. Local workspaces live in per-run +subdirectories of the configured workspace root. Docker workspaces use +`/workspace` and receive only the selected inner session key, never the outer +server key or shared callback key. Local mode retains its existing single-tenant +server credential boundary; a local workspace is not a security sandbox. + +Conversation completion is detected by the watchdog through the scoped SDK +runtime on each scan (`AUTOMATION_WATCHDOG_INTERVAL_SECONDS`, default 60 seconds). +This is the primary completion path for conversation-scoped runs: workers +deliberately do not receive the shared Automation callback credential. +Run-scoped execution retains its callback path. + +`AUTOMATION_CONVERSATION_MAX_CONCURRENT_RUNS` defaults to 2. Docker servers must +support runtime credential provisioning and release; bound container CPU, memory, +and PIDs in the server configuration. Completed Docker runtimes are released while +history remains; the persistent local server and its history are retained. + ### Prerequisites - Python 3.12+ @@ -101,3 +131,14 @@ containers/ # Docker configuration ## Deployment This service is deployed via the [deploy repository](https://github.com/All-Hands-AI/deploy). Docker images are automatically built and pushed to `ghcr.io/openhands/automation` on every push to main and on tags. + +### SDK Integration Dependency + +The conversation backend and Agent Server execution helpers reuse `RemoteConversation` and `RemoteWorkspace` +from [software-agent-sdk #5010](https://github.com/OpenHands/software-agent-sdk/pull/5010). +This draft pins the SDK implementation by immutable Git commit so its tests and +source installation are reproducible. Replace that integration pin with the SDK +release before merging. The live factory additionally integrates the server +runtime stack; those server changes are separate from this SDK dependency. +Workflow bundles +receive the same environment contract in local and Docker workspaces. diff --git a/migrations/versions/025_add_execution_scope.py b/migrations/versions/025_add_execution_scope.py new file mode 100644 index 00000000..7227d2f4 --- /dev/null +++ b/migrations/versions/025_add_execution_scope.py @@ -0,0 +1,40 @@ +"""Record whether a run or conversation owns each execution. + +Revision ID: 025 +Revises: 024 +""" + +import sqlalchemy as sa +from alembic import op + + +revision = "025" +down_revision = "024" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "automations", + sa.Column( + "execution_scope", + sa.String(20), + nullable=False, + server_default="run", + ), + ) + op.add_column( + "automation_runs", + sa.Column( + "execution_scope", + sa.String(20), + nullable=False, + server_default="run", + ), + ) + + +def downgrade() -> None: + op.drop_column("automation_runs", "execution_scope") + op.drop_column("automations", "execution_scope") diff --git a/openhands/automation/backends/__init__.py b/openhands/automation/backends/__init__.py index d0e23d08..f0e77915 100644 --- a/openhands/automation/backends/__init__.py +++ b/openhands/automation/backends/__init__.py @@ -1,68 +1,74 @@ -"""Execution backends for automation runs. +"""Execution backends separated from Agent Server provisioning. -Provides pluggable backends for getting and releasing execution contexts: -- CloudSandboxBackend: Creates fresh Cloud sandboxes per run (default) -- LocalAgentServerBackend: Uses a pre-configured local agent server - -Usage: - from openhands.automation.backends import get_backend - - backend = get_backend(run) # Returns backend for this run - ctx = await backend.get_execution_context(client) - try: - # Use ctx.agent_url and ctx.session_key - ... - finally: - await backend.release_context(client, ctx) +An automation chooses whether the run or a conversation owns its execution. +Deployment configuration independently chooses whether run-scoped work uses an +existing Agent Server or one provisioned in an OpenHands Cloud sandbox. """ from __future__ import annotations from typing import TYPE_CHECKING +from uuid import UUID from openhands.automation.backends.base import ExecutionBackend, ExecutionContext -from openhands.automation.backends.cloud import CloudSandboxBackend -from openhands.automation.backends.local import LocalAgentServerBackend +from openhands.automation.backends.conversation import ConversationBackend +from openhands.automation.backends.providers import ( + CloudSandboxAgentServerProvider, + ExistingAgentServerProvider, +) +from openhands.automation.backends.run import RunBackend if TYPE_CHECKING: from openhands.automation.models import AutomationRun -def get_backend(run: AutomationRun) -> ExecutionBackend: - """Get the appropriate execution backend for an automation run. +def _existing_agent_server(run: AutomationRun) -> ExistingAgentServerProvider: + from openhands.automation.config import get_config - Args: - run: The automation run this backend will operate on. + settings = get_config().service + return ExistingAgentServerProvider( + agent_server_url=settings.agent_server_url, + api_key=settings.agent_server_api_key, + run=run, + workspace_base=settings.workspace_base, + callback_api_key=settings.local_api_key, + sandbox_agent_server_url=settings.sandbox_agent_server_url or None, + ) - Returns: - ExecutionBackend: Either CloudSandboxBackend or LocalAgentServerBackend - """ - from openhands.automation.config import get_config - config = get_config() - settings = config.service +def get_backend(run: AutomationRun) -> ExecutionBackend: + """Build the backend for the run's explicit execution scope.""" + from openhands.automation.config import get_config - if settings.is_local_mode: - return LocalAgentServerBackend( - agent_server_url=settings.agent_server_url, - api_key=settings.agent_server_api_key, - run=run, - workspace_base=settings.workspace_base, - callback_api_key=settings.local_api_key, - sandbox_agent_server_url=settings.sandbox_agent_server_url or None, + settings = get_config().service + if run.execution_scope == "conversation": + if not settings.is_local_mode: + raise ValueError("Conversation execution requires an existing Agent Server") + if not settings.agent_profile: + raise ValueError("Conversation execution requires an agent profile") + return ConversationBackend( + _existing_agent_server(run), + agent_profile_id=UUID(settings.agent_profile), ) - else: - return CloudSandboxBackend( + + provider = ( + _existing_agent_server(run) + if settings.is_local_mode + else CloudSandboxAgentServerProvider( api_url=settings.openhands_api_base_url, run=run, ) + ) + return RunBackend(provider) __all__ = [ + "CloudSandboxAgentServerProvider", + "ConversationBackend", "ExecutionBackend", "ExecutionContext", - "CloudSandboxBackend", - "LocalAgentServerBackend", + "ExistingAgentServerProvider", + "RunBackend", "get_backend", ] diff --git a/openhands/automation/backends/base.py b/openhands/automation/backends/base.py index 166985a9..9147b350 100644 --- a/openhands/automation/backends/base.py +++ b/openhands/automation/backends/base.py @@ -1,8 +1,9 @@ -"""Base classes for execution backends.""" +"""Contracts shared by execution backends and Agent Server providers.""" from abc import ABC, abstractmethod from dataclasses import dataclass -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Protocol +from uuid import UUID import httpx @@ -18,9 +19,9 @@ class ExecutionContext: Attributes: agent_url: Base URL of the agent server (e.g., "http://localhost:3000") session_key: API key for authenticating with the agent server - sandbox_id: Sandbox ID (Cloud mode only, None for local mode) - api_url: Cloud API URL (Cloud mode only, needed for sandbox cleanup) - api_key: Cloud API key (Cloud mode only, needed for sandbox cleanup) + sandbox_id: Cloud sandbox ID, when one was provisioned + api_url: Cloud API URL needed to clean up a provisioned sandbox + api_key: Cloud API key needed to clean up a provisioned sandbox """ agent_url: str @@ -28,18 +29,19 @@ class ExecutionContext: sandbox_id: str | None = None api_url: str | None = None api_key: str | None = None + runtime_conversation_id: UUID | None = None -class ExecutionBackend(ABC): - """Abstract base class for execution backends. +class AgentServerProvider(ABC): + """Supply the Agent Server environment used by an execution backend. - Execution backends encapsulate all mode-specific behavior: + Agent Server providers encapsulate endpoint-specific behavior: - Sandbox/agent server lifecycle (acquire/release) - API key acquisition - Environment variable injection - Run verification and cleanup - This keeps dispatcher and watchdog mode-agnostic. + Run and conversation backends compose these providers. """ @abstractmethod @@ -48,10 +50,8 @@ async def get_execution_context( ) -> ExecutionContext: """Get the execution context (agent server URL + credentials). - For Cloud mode: Creates a sandbox, waits for it to be RUNNING, - and extracts the agent server URL from exposed_urls. - - For Local mode: Returns the pre-configured agent server URL. + A Cloud provider creates a sandbox and resolves its Agent Server URL. + An existing-server provider returns its configured URL. Args: client: HTTP client for making requests @@ -61,7 +61,7 @@ async def get_execution_context( Raises: RuntimeError: If context cannot be obtained - TimeoutError: If sandbox doesn't become ready in time (Cloud mode) + TimeoutError: If a sandbox does not become ready in time """ @abstractmethod @@ -70,8 +70,8 @@ async def release_context( ) -> None: """Release the execution context (cleanup). - For Cloud mode: Deletes the sandbox. - For Local mode: No-op (persistent server). + A Cloud provider deletes its sandbox. An existing-server provider does + nothing because it does not own the server. Args: client: HTTP client for making requests @@ -82,8 +82,8 @@ async def release_context( async def get_api_key(self) -> str: """Get the API key for executing an automation run. - For Cloud mode: Mints/returns the per-user API key. - For Local mode: Returns the pre-configured API key. + A Cloud provider mints a per-user key. An existing-server provider + returns its configured key. Returns: API key string @@ -93,8 +93,8 @@ async def get_api_key(self) -> str: def build_env_vars(self) -> dict[str, str]: """Build environment variables to inject into the execution environment. - For Cloud mode: OPENHANDS_API_KEY, OPENHANDS_CLOUD_API_URL - For Local mode: AGENT_SERVER_URL, SESSION_API_KEY + Cloud sandboxes receive OpenHands API settings. Existing Agent Servers + receive their URL and session key. Returns: Dictionary of environment variable name -> value @@ -104,8 +104,8 @@ def build_env_vars(self) -> dict[str, str]: async def verify_run(self, run_id: str) -> "VerificationResult": """Verify the status of a running automation. - For Cloud mode: Discovers sandbox, queries agent server, cleans up. - For Local mode: Queries agent server directly, no cleanup. + A Cloud provider discovers its sandbox before querying the Agent Server. + An existing-server provider queries its configured server directly. Args: run_id: Run ID string for logging @@ -118,8 +118,8 @@ async def verify_run(self, run_id: str) -> "VerificationResult": async def cleanup_after_verification(self, run_id: str) -> None: """Clean up resources after verification fails. - For Cloud mode: Deletes the sandbox when called by policy-aware callers. - For Local mode: No-op (persistent server). + A Cloud provider deletes the sandbox when called by policy-aware + callers. An existing-server provider does nothing. Args: run_id: Run ID string for logging @@ -127,19 +127,47 @@ async def cleanup_after_verification(self, run_id: str) -> None: @property @abstractmethod - def is_local_mode(self) -> bool: - """Whether this backend operates in local mode.""" + def provisions_agent_server(self) -> bool: + """Whether acquiring a context creates an Agent Server.""" @abstractmethod def get_work_dir(self, run_id: str) -> str: """Get the working directory for tarball extraction and execution. - For Cloud mode: Returns /workspace/project (container filesystem). - For Local mode: Returns {workspace_base}/automation-runs/{run_id}/ + A Cloud provider returns its container workspace. An existing-server + provider returns an isolated directory below its configured root. Args: - run_id: The automation run ID (used for isolation in local mode) + run_id: The automation run ID used for workspace isolation Returns: Absolute path to the working directory """ + + +class ExecutionBackend(Protocol): + """Execution behavior consumed by the dispatcher and watchdog.""" + + @property + def provider(self) -> AgentServerProvider: ... + + @property + def provisions_agent_server(self) -> bool: ... + + async def get_execution_context( + self, client: httpx.AsyncClient + ) -> ExecutionContext: ... + + async def release_context( + self, client: httpx.AsyncClient, ctx: ExecutionContext + ) -> None: ... + + async def get_api_key(self) -> str: ... + + def build_env_vars(self) -> dict[str, str]: ... + + async def verify_run(self, run_id: str) -> "VerificationResult": ... + + async def cleanup_after_verification(self, run_id: str) -> None: ... + + def get_work_dir(self, run_id: str) -> str: ... diff --git a/openhands/automation/backends/conversation.py b/openhands/automation/backends/conversation.py new file mode 100644 index 00000000..230dac98 --- /dev/null +++ b/openhands/automation/backends/conversation.py @@ -0,0 +1,191 @@ +"""One bundle-facing conversation contract for local and Docker workspaces.""" + +from __future__ import annotations + +import asyncio +from uuid import UUID + +import httpx + +from openhands.automation.backends.base import ExecutionContext +from openhands.automation.backends.providers.existing import ( + ExistingAgentServerProvider, +) +from openhands.automation.subjects import conversation_id_for +from openhands.automation.utils.agent_server import ( + VerificationResult, + verify_run_on_agent_server, +) +from openhands.sdk import RemoteConversation +from openhands.sdk.conversation.request import StartConversationRequest +from openhands.sdk.workspace import ( + AsyncRemoteWorkspace, + LocalWorkspace, + RemoteWorkspace, +) + + +class ConversationBackend: + def __init__( + self, + provider: ExistingAgentServerProvider, + *, + agent_profile_id: UUID, + ) -> None: + self._provider = provider + self.agent_profile_id = agent_profile_id + self.runtime_api_key = "" + self._runtime_kind: str | None = None + + @property + def provisions_agent_server(self) -> bool: + return False + + @property + def provider(self) -> ExistingAgentServerProvider: + return self._provider + + async def get_api_key(self) -> str: + return await self.provider.get_api_key() + + @property + def conversation_id(self) -> UUID: + automation = self.provider.run.automation + source = (automation.trigger or {}).get("source") + if self.provider.run.subject_key and source: + return UUID( + conversation_id_for( + automation.org_id, + automation.id, + source, + self.provider.run.subject_key, + ) + ) + return self.provider.run.id + + async def _resolve_runtime(self) -> str: + if self._runtime_kind is None: + async with AsyncRemoteWorkspace( + host=self.provider.agent_server_url, + api_key=self.provider.api_key, + working_dir="/", + ) as workspace: + info = await workspace.get_server_info() + self._runtime_kind = info["conversation_runtime"] + if self._runtime_kind not in ("local", "docker"): + raise ValueError("Unsupported agent-server conversation runtime") + return self._runtime_kind + + def _create_conversation(self) -> None: + workspace = RemoteWorkspace( + host=self.provider.agent_server_url, + api_key=self.provider.api_key, + working_dir=self.get_work_dir(str(self.provider.run.id)), + ) + conversation = None + try: + conversation = RemoteConversation.create( + workspace=workspace, + request=StartConversationRequest( + workspace=LocalWorkspace(working_dir=workspace.working_dir), + conversation_id=self.conversation_id, + agent_profile_id=self.agent_profile_id, + max_iterations=160, + tags={"automationrun": str(self.provider.run.id)}, + ), + visualizer=None, + ) + conversation.set_title(self.provider.run.automation.name) + finally: + if conversation is not None: + conversation.close() + workspace.reset_client() + + async def get_execution_context( + self, client: httpx.AsyncClient + ) -> ExecutionContext: + runtime_kind = await self._resolve_runtime() + await asyncio.to_thread(self._create_conversation) + self.runtime_api_key = self.provider.api_key if runtime_kind == "local" else "" + context = ExecutionContext( + agent_url=self.provider.agent_server_url, + session_key=self.provider.api_key, + runtime_conversation_id=self.conversation_id, + ) + if runtime_kind == "docker": + try: + async with AsyncRemoteWorkspace( + host=context.agent_url, + api_key=self.provider.api_key, + working_dir="/", + runtime_conversation_id=context.runtime_conversation_id, + ) as workspace: + self.runtime_api_key = await workspace.get_runtime_session_key() + except Exception: + await self.release_context(client, context) + raise + return context + + def build_env_vars(self) -> dict[str, str]: + # Completion is polled through the scoped SDK runtime. Never expose the + # shared Automation callback key to a profile-scoped worker. + if not self.runtime_api_key: + raise RuntimeError("Runtime credentials have not been provisioned") + return { + "AGENT_SERVER_URL": ( + self.provider.sandbox_agent_server_url + or ( + "http://127.0.0.1:8000" + if self._runtime_kind == "docker" + else self.provider.agent_server_url + ) + ), + "AUTOMATION_CONVERSATION_ID": str(self.conversation_id), + "WORKSPACE_BASE": self.get_work_dir(str(self.provider.run.id)), + "SESSION_API_KEY": self.runtime_api_key, + } + + def get_work_dir(self, run_id: str) -> str: + if self._runtime_kind is None: + raise RuntimeError( + "Resolve the server runtime before selecting a workspace" + ) + return ( + "/workspace" + if self._runtime_kind == "docker" + else self.provider.get_work_dir(run_id) + ) + + async def release_context( + self, + client: httpx.AsyncClient, # noqa: ARG002 + ctx: ExecutionContext, + ) -> None: + if await self._resolve_runtime() == "local": + return # Persistent server and conversation history belong to the host. + async with AsyncRemoteWorkspace( + host=ctx.agent_url, + api_key=self.provider.api_key, + working_dir="/", + runtime_conversation_id=self.conversation_id, + ) as workspace: + await workspace.release_runtime() + + async def verify_run(self, run_id: str) -> VerificationResult: + return await verify_run_on_agent_server( + agent_url=self.provider.agent_server_url, + session_key=self.provider.api_key, + run_id=run_id, + bash_command_id=self.provider.run.bash_command_id, + runtime_conversation_id=self.conversation_id, + ) + + async def cleanup_after_verification(self, run_id: str) -> None: # noqa: ARG002 + async with httpx.AsyncClient() as client: + await self.release_context( + client, + ExecutionContext( + self.provider.agent_server_url, + self.provider.api_key, + ), + ) diff --git a/openhands/automation/backends/providers/__init__.py b/openhands/automation/backends/providers/__init__.py new file mode 100644 index 00000000..b832b655 --- /dev/null +++ b/openhands/automation/backends/providers/__init__.py @@ -0,0 +1,14 @@ +"""Ways to obtain an Agent Server endpoint for automation execution.""" + +from openhands.automation.backends.providers.cloud import ( + CloudSandboxAgentServerProvider, +) +from openhands.automation.backends.providers.existing import ( + ExistingAgentServerProvider, +) + + +__all__ = [ + "CloudSandboxAgentServerProvider", + "ExistingAgentServerProvider", +] diff --git a/openhands/automation/backends/cloud.py b/openhands/automation/backends/providers/cloud.py similarity index 95% rename from openhands/automation/backends/cloud.py rename to openhands/automation/backends/providers/cloud.py index 12c6c700..e701e976 100644 --- a/openhands/automation/backends/cloud.py +++ b/openhands/automation/backends/providers/cloud.py @@ -1,7 +1,4 @@ -"""Cloud sandbox execution backend. - -Creates a fresh Cloud sandbox for each automation run. -""" +"""Provider that creates an Agent Server endpoint in a Cloud sandbox.""" from __future__ import annotations @@ -19,7 +16,7 @@ wait_exponential, ) -from openhands.automation.backends.base import ExecutionBackend, ExecutionContext +from openhands.automation.backends.base import AgentServerProvider, ExecutionContext from openhands.automation.config import get_config from openhands.automation.exceptions import ConcurrencyLimitReachedError from openhands.automation.models import AutomationRun @@ -74,8 +71,8 @@ def _concurrency_limit_detail(resp: httpx.Response) -> dict | None: return None -class CloudSandboxBackend(ExecutionBackend): - """Execution backend that creates Cloud sandboxes per run. +class CloudSandboxAgentServerProvider(AgentServerProvider): + """Agent Server provider that creates a Cloud sandbox per run. This is the default backend for OpenHands Cloud deployments. Each automation run gets a fresh, isolated sandbox. @@ -93,7 +90,7 @@ def __init__(self, api_url: str, run: AutomationRun): run: The automation run (used to extract user info for API key) """ self.api_url = api_url.rstrip("/") - self._run = run + self.run = run self._api_key: str | None = None # Lazily minted # Load sandbox config for retry/timeout settings @@ -114,8 +111,8 @@ def __init__(self, api_url: str, run: AutomationRun): ) @property - def is_local_mode(self) -> bool: - return False + def provisions_agent_server(self) -> bool: + return True def get_work_dir(self, run_id: str) -> str: # noqa: ARG002 """Return the standard container working directory. @@ -130,13 +127,13 @@ async def _ensure_api_key(self) -> str: The key is minted lazily on first call and cached for reuse. """ if self._api_key is None: - self._api_key = await get_api_key_for_automation_run(self._run) + self._api_key = await get_api_key_for_automation_run(self.run) return self._api_key async def _refresh_api_key(self) -> str: """Force refresh the API key (e.g., after auth failure).""" logger.info("Refreshing API key after authentication failure") - self._api_key = await get_api_key_for_automation_run(self._run) + self._api_key = await get_api_key_for_automation_run(self.run) return self._api_key async def _with_auth_retry( @@ -202,7 +199,7 @@ def build_env_vars(self) -> dict[str, str]: async def verify_run(self, run_id: str) -> VerificationResult: """Verify run status via sandbox discovery.""" - sandbox_id = self._run.sandbox_id + sandbox_id = self.run.sandbox_id if not sandbox_id: from openhands.automation.utils.agent_server import ( VerificationOutcome, @@ -220,14 +217,14 @@ async def _do_verify() -> VerificationResult: api_key=await self._ensure_api_key(), sandbox_id=sandbox_id, run_id=run_id, - bash_command_id=self._run.bash_command_id, + bash_command_id=self.run.bash_command_id, ) return await self._with_auth_retry(_do_verify) async def cleanup_after_verification(self, run_id: str) -> None: """Clean up sandbox after verification failure.""" - sandbox_id = self._run.sandbox_id + sandbox_id = self.run.sandbox_id if sandbox_id: async def _do_cleanup() -> None: diff --git a/openhands/automation/backends/local.py b/openhands/automation/backends/providers/existing.py similarity index 93% rename from openhands/automation/backends/local.py rename to openhands/automation/backends/providers/existing.py index 6d6404ff..e29ac4dc 100644 --- a/openhands/automation/backends/local.py +++ b/openhands/automation/backends/providers/existing.py @@ -1,7 +1,4 @@ -"""Local agent-server execution backend. - -Uses a pre-configured local agent server instead of creating Cloud sandboxes. -""" +"""Provider for an existing Agent Server endpoint.""" from __future__ import annotations @@ -12,7 +9,7 @@ import httpx -from openhands.automation.backends.base import ExecutionBackend, ExecutionContext +from openhands.automation.backends.base import AgentServerProvider, ExecutionContext from openhands.automation.utils.agent_server import ( VerificationResult, verify_run_on_agent_server, @@ -51,8 +48,8 @@ def local_runs_root(workspace_base: str | os.PathLike[str] | None) -> Path: return Path(resolve_local_workspace_base(workspace_base)) / "automation-runs" -class LocalAgentServerBackend(ExecutionBackend): - """Execution backend for local/self-hosted deployments. +class ExistingAgentServerProvider(AgentServerProvider): + """Provider for local, self-hosted, or otherwise pre-provisioned Agent Servers. Uses a persistent, pre-configured agent server. No sandbox creation or cleanup is performed — the agent server is assumed to be running @@ -73,7 +70,7 @@ def __init__( callback_api_key: str | None = None, sandbox_agent_server_url: str | None = None, ): - """Initialize the local agent-server backend for a specific run. + """Initialize an existing Agent Server provider for a specific run. Args: agent_server_url: URL of the local agent server @@ -96,7 +93,7 @@ def __init__( """ self.agent_server_url = agent_server_url.rstrip("/") self.api_key = api_key - self._run = run + self.run = run self.workspace_base = workspace_base self.callback_api_key = callback_api_key self.sandbox_agent_server_url = ( @@ -104,8 +101,8 @@ def __init__( ) @property - def is_local_mode(self) -> bool: - return True + def provisions_agent_server(self) -> bool: + return False async def get_execution_context( self, @@ -156,7 +153,7 @@ def build_env_vars(self) -> dict[str, str]: dispatcher after calling this method. """ # Use run-specific workspace directory for isolation - run_workspace = self.get_work_dir(str(self._run.id)) + run_workspace = self.get_work_dir(str(self.run.id)) env_vars = { "AGENT_SERVER_URL": self.sandbox_agent_server_url or self.agent_server_url, "SESSION_API_KEY": self.api_key, @@ -191,7 +188,7 @@ async def verify_run(self, run_id: str) -> VerificationResult: agent_url=self.agent_server_url, session_key=self.api_key, run_id=run_id, - bash_command_id=self._run.bash_command_id, + bash_command_id=self.run.bash_command_id, ) async def cleanup_after_verification( diff --git a/openhands/automation/backends/run.py b/openhands/automation/backends/run.py new file mode 100644 index 00000000..c089d2fa --- /dev/null +++ b/openhands/automation/backends/run.py @@ -0,0 +1,48 @@ +"""Execution whose workspace and lifecycle belong to one automation run.""" + +from __future__ import annotations + +import httpx + +from openhands.automation.backends.base import AgentServerProvider, ExecutionContext +from openhands.automation.utils.agent_server import VerificationResult + + +class RunBackend: + """Execute one automation bundle through an Agent Server provider.""" + + def __init__(self, provider: AgentServerProvider) -> None: + self._provider = provider + + @property + def provider(self) -> AgentServerProvider: + return self._provider + + @property + def provisions_agent_server(self) -> bool: + return self.provider.provisions_agent_server + + async def get_execution_context( + self, client: httpx.AsyncClient + ) -> ExecutionContext: + return await self.provider.get_execution_context(client) + + async def release_context( + self, client: httpx.AsyncClient, ctx: ExecutionContext + ) -> None: + await self.provider.release_context(client, ctx) + + async def get_api_key(self) -> str: + return await self.provider.get_api_key() + + def build_env_vars(self) -> dict[str, str]: + return self.provider.build_env_vars() + + async def verify_run(self, run_id: str) -> VerificationResult: + return await self.provider.verify_run(run_id) + + async def cleanup_after_verification(self, run_id: str) -> None: + await self.provider.cleanup_after_verification(run_id) + + def get_work_dir(self, run_id: str) -> str: + return self.provider.get_work_dir(run_id) diff --git a/openhands/automation/config.py b/openhands/automation/config.py index 6e7acfb6..d8ad6fce 100644 --- a/openhands/automation/config.py +++ b/openhands/automation/config.py @@ -581,8 +581,12 @@ class ServiceSettings(BaseSettings): # - Authenticates using local_api_key instead of OpenHands SaaS API agent_server_url: str = "" agent_server_api_key: str = "" + # Deployment profile for agent-mode runs; the server advertises the runtime. + agent_profile: str = "" + conversation_max_concurrent_runs: int = Field(default=2, ge=1) + # Optional override for the AGENT_SERVER_URL env var exported into the - # in-sandbox bash chain by LocalAgentServerBackend.build_env_vars. + # in-sandbox bash chain by ExistingAgentServerProvider.build_env_vars. # When empty, defaults to agent_server_url (the URL the backend itself # uses). Needed in container-split dev setups (e.g. agent-canvas # `dev:docker`) where the host-side backend reaches the agent-server at diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index e5b6cef6..783f227c 100644 --- a/openhands/automation/dispatcher.py +++ b/openhands/automation/dispatcher.py @@ -22,7 +22,7 @@ from typing import Any import httpx -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sqlalchemy.orm import selectinload @@ -131,6 +131,28 @@ async def _poll_pending_runs( Eagerly loads the ``automation`` relationship so that ``user_id``, ``org_id``, and tarball config are available for dispatch. """ + settings = get_config().service + active_conversation_automations = [] + conversation_capacity: int | None = None + if settings.is_local_mode: + active_conversation_automations = ( + ( + await session.execute( + select(AutomationRun.automation_id).where( + AutomationRun.status == AutomationRunStatus.RUNNING, + AutomationRun.execution_scope == "conversation", + ) + ) + ) + .scalars() + .all() + ) + conversation_capacity = settings.conversation_max_concurrent_runs - len( + active_conversation_automations + ) + if conversation_capacity > 0: + batch_size = min(batch_size, 1, conversation_capacity) + select_query = ( select(AutomationRun) .join(AutomationRun.automation) @@ -143,6 +165,16 @@ async def _poll_pending_runs( .order_by(AutomationRun.created_at.asc()) .limit(batch_size) ) + if conversation_capacity is not None and conversation_capacity <= 0: + # Run-scoped work remains runnable while conversations occupy + # every bounded runtime slot. + select_query = select_query.where( + AutomationRun.execution_scope != "conversation" + ) + elif active_conversation_automations: + select_query = select_query.where( + AutomationRun.automation_id.not_in(active_conversation_automations) + ) # Apply row locking for PostgreSQL only (SQLite doesn't support it) if not using_sqlite(): @@ -207,8 +239,9 @@ async def _execute_run( 4. Execute in context (upload tarball, start entrypoint) 5. Store sandbox_id for watchdog verification (if applicable) - The SDK inside the execution environment fires the completion callback on exit. - The watchdog will verify status if the callback is missed. + Legacy backends receive SDK completion callbacks, with watchdog fallback. + Profile-backed conversations use watchdog verification through the SDK + runtime so workers do not need the shared Automation callback credential. """ run_id = str(run.id) automation = run.automation @@ -305,7 +338,7 @@ async def _fail( return except Exception as exc: logger.exception("Failed to get execution context", extra=_log_ctx()) - source = "agent_server" if backend.is_local_mode else "sandbox_api" + source = "sandbox_api" if backend.provisions_agent_server else "agent_server" await _fail( "Failed to get execution context", status_detail=run_status_detail_from_exception( @@ -326,10 +359,17 @@ async def _fail( # 3. Build env vars (must be after get_execution_context for cloud mode API key) callback_url = f"{settings.resolved_base_url.rstrip('/')}/v1/runs/{run_id}/complete" env_vars = backend.build_env_vars() - env_vars["AUTOMATION_CALLBACK_URL"] = callback_url - env_vars["AUTOMATION_PHASE_URL"] = ( - f"{settings.resolved_base_url.rstrip('/')}/v1/runs/{run_id}/phase" - ) + # Callbacks are optional. A restricted backend without callback credentials + # uses the existing runtime watchdog instead of receiving the service key. + if ( + env_vars.get("AUTOMATION_CALLBACK_API_KEY") + or env_vars.get("OPENHANDS_API_KEY") + or not settings.local_api_key + ): + env_vars["AUTOMATION_CALLBACK_URL"] = callback_url + env_vars["AUTOMATION_PHASE_URL"] = ( + f"{settings.resolved_base_url.rstrip('/')}/v1/runs/{run_id}/phase" + ) env_vars["AUTOMATION_RUN_ID"] = run_id env_vars["AUTOMATION_USER_ID"] = str(automation.user_id) env_vars["AUTOMATION_ORG_ID"] = str(automation.org_id) @@ -435,7 +475,6 @@ async def _fail( work_dir = backend.get_work_dir(run_id) try: result = await execute_in_context( - client=client, agent_url=ctx.agent_url, session_key=ctx.session_key, entrypoint=automation.entrypoint, @@ -445,6 +484,7 @@ async def _fail( timeout=effective_timeout, run_id=run_id, sandbox_id=ctx.sandbox_id, + runtime_conversation_id=ctx.runtime_conversation_id, ) except PermanentDispatchError as exc: logger.error( @@ -473,7 +513,7 @@ async def _fail( "Background execution failed", extra=_log_ctx(sandbox_id=ctx.sandbox_id) ) await backend.release_context(client, ctx) - source = "agent_server" if backend.is_local_mode else "sandbox_api" + source = "sandbox_api" if backend.provisions_agent_server else "agent_server" await _fail( "Internal error", status_detail=run_status_detail_from_exception( @@ -487,6 +527,14 @@ async def _fail( # 6. Handle result if result.success: + if ctx.runtime_conversation_id is not None: + async with session_factory() as link_session: + await link_session.execute( + update(AutomationRun) + .where(AutomationRun.id == run.id) + .values(conversation_id=env_vars["AUTOMATION_CONVERSATION_ID"]) + ) + await link_session.commit() await update_run_current_phase(session_factory, run.id, "Starting automation") if ctx.sandbox_id: await update_sandbox_id(session_factory, run.id, ctx.sandbox_id) @@ -511,7 +559,7 @@ async def _fail( ), ) logger.info( - "Automation dispatched successfully, waiting for callback", + "Automation dispatched successfully, waiting for completion", extra=_log_ctx(sandbox_id=ctx.sandbox_id), ) return @@ -528,7 +576,7 @@ async def _fail( kind=RunStatusDetailKind.EXECUTION_ERROR, detail=error, transient=False, - source="agent_server" if backend.is_local_mode else "sandbox_api", + source="sandbox_api" if backend.provisions_agent_server else "agent_server", operation="execute_in_context", ), ) diff --git a/openhands/automation/execution.py b/openhands/automation/execution.py index 2c8e6d23..f01e9c1a 100644 --- a/openhands/automation/execution.py +++ b/openhands/automation/execution.py @@ -10,6 +10,7 @@ import re import tarfile from typing import Any +from uuid import UUID import httpx from pydantic.dataclasses import dataclass @@ -27,6 +28,7 @@ from openhands.automation.utils import log_extra from openhands.automation.utils.sandbox import delete_sandbox from openhands.automation.utils.timeout import resolve_automation_timeout_seconds +from openhands.sdk.workspace import AsyncRemoteWorkspace # Default working directory for cloud/container mode @@ -166,11 +168,11 @@ async def _create_and_wait( async def _upload( - client: httpx.AsyncClient, agent_url: str, session_key: str, data: bytes, dest: str, + runtime_conversation_id: UUID | None = None, ) -> None: """Upload bytes to the sandbox via the agent-server file API. @@ -178,59 +180,54 @@ async def _upload( with proxies that collapse double-slashes (e.g. //tmp -> /tmp). See: https://github.com/All-Hands-AI/OpenHands/commit/a14158e """ - # Use query param instead of path param to avoid double-slash normalization - from urllib.parse import urlencode - - params = urlencode({"path": dest}) - resp = await client.post( - f"{agent_url}/api/file/upload?{params}", - files={"file": ("upload", data)}, - headers={"X-Session-API-Key": session_key}, - ) - resp.raise_for_status() + async with AsyncRemoteWorkspace( + host=agent_url, + api_key=session_key, + working_dir="/", + runtime_conversation_id=runtime_conversation_id, + ) as workspace: + result = await workspace.file_upload(data, dest) + if not result.success: + raise RuntimeError(result.error or "Automation bundle upload failed") async def _bash( - client: httpx.AsyncClient, agent_url: str, session_key: str, command: str, timeout: int | None = None, + runtime_conversation_id: UUID | None = None, ) -> tuple[int | None, str, str]: """Run a bash command synchronously. Returns ``(exit_code, stdout, stderr)``.""" if timeout is None: timeout = resolve_automation_timeout_seconds(None) - resp = await client.post( - f"{agent_url}/api/bash/execute_bash_command", - json={"command": command, "timeout": timeout}, - headers={"X-Session-API-Key": session_key}, - timeout=httpx.Timeout(timeout + 30), - ) - resp.raise_for_status() - body = resp.json() - return body.get("exit_code"), body.get("stdout") or "", body.get("stderr") or "" + async with AsyncRemoteWorkspace( + host=agent_url, + api_key=session_key, + working_dir="/", + runtime_conversation_id=runtime_conversation_id, + ) as workspace: + result = await workspace.execute_command(command, timeout=timeout) + return result.exit_code, result.stdout, result.stderr async def _start_bash( - client: httpx.AsyncClient, agent_url: str, session_key: str, command: str, timeout: int | None = None, + runtime_conversation_id: UUID | None = None, ) -> str: """Start a bash command in the background. Returns the command ID.""" if timeout is None: timeout = resolve_automation_timeout_seconds(None) - http_timeout = get_config().http.http_timeout - resp = await client.post( - f"{agent_url}/api/bash/start_bash_command", - json={"command": command, "timeout": timeout}, - headers={"X-Session-API-Key": session_key}, - timeout=http_timeout, - ) - resp.raise_for_status() - body = resp.json() - return body.get("id") + async with AsyncRemoteWorkspace( + host=agent_url, + api_key=session_key, + working_dir="/", + runtime_conversation_id=runtime_conversation_id, + ) as workspace: + return await workspace.start_command(command, timeout=timeout) def _is_permanent_http_error(stderr: str) -> bool: @@ -252,13 +249,13 @@ def _is_permanent_http_error(stderr: str) -> bool: async def _download_in_sandbox( - client: httpx.AsyncClient, agent_url: str, session_key: str, tarball_url: str, dest: str, timeout: int | None = None, max_filesize: int | None = None, + runtime_conversation_id: UUID | None = None, ) -> None: """Download a tarball directly inside the sandbox using curl. @@ -292,7 +289,11 @@ async def _download_in_sandbox( ) exit_code, stdout, stderr = await _bash( - client, agent_url, session_key, cmd, timeout=timeout + 30 + agent_url, + session_key, + cmd, + timeout=timeout + 30, + runtime_conversation_id=runtime_conversation_id, ) if exit_code != 0: @@ -331,7 +332,6 @@ class DispatchResult: async def execute_in_context( - client: httpx.AsyncClient, agent_url: str, session_key: str, entrypoint: str, @@ -341,6 +341,7 @@ async def execute_in_context( timeout: int | None = None, run_id: str | None = None, sandbox_id: str | None = None, + runtime_conversation_id: UUID | None = None, ) -> DispatchResult: """Execute automation code in an existing execution context. @@ -383,28 +384,40 @@ def _log_ctx() -> dict[str, Any]: if run_id and "/" not in run_id else TARBALL_PATH ) + if runtime_conversation_id is not None: + tarball_path = f"{work_dir}/automation-{run_id or 'run'}.tar.gz" env_path: str | None = None try: # Get tarball into environment: upload bytes or download from URL if isinstance(tarball_source, bytes): logger.info("Uploading tarball", extra=_log_ctx()) - await _upload(client, agent_url, session_key, tarball_source, tarball_path) + await _upload( + agent_url, + session_key, + tarball_source, + tarball_path, + runtime_conversation_id=runtime_conversation_id, + ) else: logger.info("Downloading tarball from URL", extra=_log_ctx()) await _download_in_sandbox( - client, agent_url, session_key, tarball_source, tarball_path + agent_url, + session_key, + tarball_source, + tarball_path, + runtime_conversation_id=runtime_conversation_id, ) env_prefix = "" if env_vars: env_path = f"{tarball_path}.env" await _upload( - client, agent_url, session_key, _serialize_env_vars(env_vars), env_path, + runtime_conversation_id=runtime_conversation_id, ) env_prefix = _env_command_prefix(env_path) @@ -419,7 +432,11 @@ def _log_ctx() -> dict[str, Any]: logger.info("Starting entrypoint: %s", entrypoint, extra=_log_ctx()) command_id = await _start_bash( - client, agent_url, session_key, cmd, timeout=timeout + agent_url, + session_key, + cmd, + timeout=timeout, + runtime_conversation_id=runtime_conversation_id, ) env_path = None logger.info( @@ -444,10 +461,10 @@ def _log_ctx() -> dict[str, Any]: if env_path is not None: try: exit_code, _, stderr = await _bash( - client, agent_url, session_key, f"rm -f -- {_shell_quote(env_path)}", + runtime_conversation_id=runtime_conversation_id, timeout=int(get_config().http.http_timeout), ) if exit_code != 0: @@ -553,20 +570,17 @@ def _log_ctx() -> dict[str, Any]: # Get tarball into sandbox: upload bytes or download from URL if isinstance(tarball_source, bytes): logger.info("Uploading tarball to sandbox", extra=_log_ctx()) - await _upload( - client, agent_url, session_key, tarball_source, TARBALL_PATH - ) + await _upload(agent_url, session_key, tarball_source, TARBALL_PATH) else: logger.info("Downloading tarball in sandbox from URL", extra=_log_ctx()) await _download_in_sandbox( - client, agent_url, session_key, tarball_source, TARBALL_PATH + agent_url, session_key, tarball_source, TARBALL_PATH ) env_prefix = "" if env_vars: env_path = f"{TARBALL_PATH}.env" await _upload( - client, agent_url, session_key, _serialize_env_vars(env_vars), @@ -584,7 +598,10 @@ def _log_ctx() -> dict[str, Any]: logger.info("Executing entrypoint: %s", entrypoint, extra=_log_ctx()) exit_code, stdout, stderr = await _bash( - client, agent_url, session_key, cmd, timeout=timeout + agent_url, + session_key, + cmd, + timeout=timeout, ) success = exit_code == 0 diff --git a/openhands/automation/git_sync/loop.py b/openhands/automation/git_sync/loop.py index 5c3e363d..ab634140 100644 --- a/openhands/automation/git_sync/loop.py +++ b/openhands/automation/git_sync/loop.py @@ -501,12 +501,16 @@ async def _validate_and_resolve_fields( fields.get("setup_script_path"), "setup_script_path" ) timeout = validate_automation_timeout(fields.get("timeout")) + execution_scope = fields.get("execution_scope", "run") + if execution_scope not in {"run", "conversation"}: + raise ValueError("execution_scope must be 'run' or 'conversation'") tarball_path = await _resolve_tarball_path( session, fields, deserialized, slug, existing, pending_storage_deletes, owner ) return { "name": name, + "execution_scope": execution_scope, "model": fields.get("model"), "trigger": trigger.model_dump(), "entrypoint": entrypoint, diff --git a/openhands/automation/git_sync/serializer.py b/openhands/automation/git_sync/serializer.py index 6e035202..e10ce7c7 100644 --- a/openhands/automation/git_sync/serializer.py +++ b/openhands/automation/git_sync/serializer.py @@ -133,6 +133,7 @@ def _automation_yaml_fields( ) -> dict[str, Any]: fields: dict[str, Any] = { "name": automation.name, + "execution_scope": automation.execution_scope, "model": automation.model, "trigger": automation.trigger, "setup_script_path": automation.setup_script_path, diff --git a/openhands/automation/models.py b/openhands/automation/models.py index 1d250491..446e8ca1 100644 --- a/openhands/automation/models.py +++ b/openhands/automation/models.py @@ -48,6 +48,13 @@ class AutomationRunStatus(enum.Enum): SKIPPED = "SKIPPED" +class AutomationExecutionScope(enum.StrEnum): + """Object that owns an automation run's workspace and lifecycle.""" + + RUN = "run" + CONVERSATION = "conversation" + + class Automation(Base): """An automation definition: what to run and when to trigger it.""" @@ -73,6 +80,10 @@ class Automation(Base): # None is only used for legacy/local fallback. model: Mapped[str | None] = mapped_column(String(64), nullable=True) + execution_scope: Mapped[str] = mapped_column( + String(20), nullable=False, default=AutomationExecutionScope.RUN.value + ) + # Trigger config — for MVP, only cron is supported. # Uses generic JSON type for cross-database compatibility (PostgreSQL + SQLite) trigger: Mapped[dict] = mapped_column(JSON, nullable=False) @@ -168,6 +179,11 @@ class AutomationRun(Base): default=AutomationRunStatus.PENDING, ) + # Snapshot how the definition executes so later edits affect future runs only. + execution_scope: Mapped[str] = mapped_column( + String(20), nullable=False, default=AutomationExecutionScope.RUN.value + ) + # Error details if status is FAILED error_detail: Mapped[str | None] = mapped_column(Text, nullable=True) diff --git a/openhands/automation/router.py b/openhands/automation/router.py index 2cce3e7a..0f9f6109 100644 --- a/openhands/automation/router.py +++ b/openhands/automation/router.py @@ -164,6 +164,7 @@ async def create_automation( auto = Automation( user_id=user.user_id, org_id=user.org_id, + execution_scope=body.execution_scope, name=body.name, model=model, preset_metadata=preset_metadata, @@ -903,6 +904,18 @@ async def cancel_run( properties={"trigger_source": "manual"}, ) + if run.execution_scope == "conversation": + from openhands.automation.backends import get_backend + + # Release the transaction before waiting for Docker to stop. + await session.commit() + try: + await get_backend(run).cleanup_after_verification(str(run_id)) + except Exception: + logger.warning( + "Runtime cleanup failed for cancelled run %s", run_id, exc_info=True + ) + # Clean up sandbox for runs that were RUNNING. Cancelling is explicit, so # unlike `complete_run` the sandbox goes even when the run owns a subject # -- but the subject is released with it, or the next event would pick this diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index efb9996e..e462a943 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -423,6 +423,13 @@ def validate_config_size(cls, v: dict[str, Any] | None) -> dict[str, Any] | None class CreateAutomationRequest(BaseModel): model_config = ConfigDict(extra="forbid") + execution_scope: Literal["run", "conversation"] = Field( + default="run", + description=( + "Whether this run owns its workspace directly or executes within " + "a persistent agent conversation." + ), + ) name: str = Field(..., min_length=1, max_length=500) model: str | None = Field( default=None, @@ -523,6 +530,7 @@ class UpdateAutomationRequest(BaseModel): model_config = ConfigDict(extra="forbid") + execution_scope: Literal["run", "conversation"] | None = None name: str | None = Field(default=None, min_length=1, max_length=500) model: str | None = Field( default=None, @@ -862,6 +870,7 @@ class TelemetryConsentResponse(BaseModel): class AutomationResponse(BaseModel): + execution_scope: Literal["run", "conversation"] = "run" id: uuid.UUID user_id: uuid.UUID org_id: uuid.UUID @@ -939,6 +948,7 @@ def normalize_phase(cls, v: Any) -> Any: class AutomationRunResponse(BaseModel): """Response for a single automation run.""" + execution_scope: Literal["run", "conversation"] = "run" id: uuid.UUID automation_id: uuid.UUID status: RunStatus diff --git a/openhands/automation/utils/agent_server.py b/openhands/automation/utils/agent_server.py index 53960d33..d44bbe23 100644 --- a/openhands/automation/utils/agent_server.py +++ b/openhands/automation/utils/agent_server.py @@ -7,8 +7,8 @@ import logging from enum import StrEnum +from uuid import UUID -import httpx from pydantic.dataclasses import dataclass from openhands.automation.utils.log_context import log_extra @@ -16,6 +16,7 @@ TransientErrorInfo, classify_httpx_transient_error, ) +from openhands.sdk.workspace import AsyncRemoteWorkspace logger = logging.getLogger(__name__) @@ -34,10 +35,10 @@ class BashCommandResult: async def get_last_bash_command_result( - client: httpx.AsyncClient, agent_url: str, session_key: str, command_id: str | None = None, + runtime_conversation_id: UUID | None = None, ) -> BashCommandResult: """Query the agent server for a bash command's result. @@ -53,7 +54,6 @@ async def get_last_bash_command_result( output behavior. Callers that have a command id should always pass it. Args: - client: HTTP client agent_url: Agent server URL session_key: API key for the agent server command_id: Optional BashCommand id (hex) to filter by @@ -62,31 +62,16 @@ async def get_last_bash_command_result( BashCommandResult with found=True if command result was retrieved """ try: - # Search for the most recent BashOutput event, scoped to this run's - # bash command whenever we know which one it is. The agent-server's - # search endpoint accepts ``command_id__eq`` and only matches - # BashOutput files whose embedded command_id matches. - params: dict[str, str | int] = { - "kind__eq": "BashOutput", - "sort_order": "TIMESTAMP_DESC", - "limit": 1, - } - if command_id: - params["command_id__eq"] = command_id - resp = await client.get( - f"{agent_url}/api/bash/bash_events/search", - params=params, - headers={"X-Session-API-Key": session_key}, - timeout=30.0, - ) - resp.raise_for_status() - page = resp.json() - - items = page.get("items", []) - if not items: + async with AsyncRemoteWorkspace( + host=agent_url, + api_key=session_key, + working_dir="/", + runtime_conversation_id=runtime_conversation_id, + ) as workspace: + output = await workspace.get_command_output(command_id) + if output is None: return BashCommandResult(found=False, error="No bash output found") - output = items[0] exit_code = output.get("exit_code") # If exit_code is None, the command is still running @@ -207,6 +192,7 @@ async def verify_run_on_agent_server( session_key: str, run_id: str | None = None, bash_command_id: str | None = None, + runtime_conversation_id: UUID | None = None, ) -> VerificationResult: """Verify an automation run's status by querying an agent server directly. @@ -231,51 +217,53 @@ async def verify_run_on_agent_server( agent_url = agent_url.rstrip("/") extra = log_extra(run_id=run_id) - async with httpx.AsyncClient(timeout=60.0) as client: - # Get last bash command result, scoped to this run's command if known - bash_result = await get_last_bash_command_result( - client, agent_url, session_key, command_id=bash_command_id + # Get last bash command result, scoped to this run's command if known + bash_result = await get_last_bash_command_result( + agent_url, + session_key, + command_id=bash_command_id, + runtime_conversation_id=runtime_conversation_id, + ) + + if not bash_result.found: + logger.warning( + "Could not find bash command result: %s", + bash_result.error, + extra=extra, ) - - if not bash_result.found: - logger.warning( - "Could not find bash command result: %s", - bash_result.error, - extra=extra, - ) - if bash_result.error_info is not None: - return VerificationResult( - outcome=VerificationOutcome.TRANSIENT_ERROR, - detail=bash_result.error, - error_info=bash_result.error_info, - ) + if bash_result.error_info is not None: return VerificationResult( - outcome=VerificationOutcome.STILL_RUNNING - if bash_result.error == "No bash output found" - else VerificationOutcome.VERIFICATION_ERROR, + outcome=VerificationOutcome.TRANSIENT_ERROR, detail=bash_result.error, + error_info=bash_result.error_info, ) - - if bash_result.exit_code is None: - logger.info("Bash command still running", extra=extra) - return VerificationResult( - outcome=VerificationOutcome.STILL_RUNNING, - detail="Command still running", - ) - - success = bash_result.exit_code == 0 - logger.info( - "Verified run status: exit_code=%s, success=%s", - bash_result.exit_code, - success, - extra=extra, + return VerificationResult( + outcome=VerificationOutcome.STILL_RUNNING + if bash_result.error == "No bash output found" + else VerificationOutcome.VERIFICATION_ERROR, + detail=bash_result.error, ) + if bash_result.exit_code is None: + logger.info("Bash command still running", extra=extra) return VerificationResult( - outcome=VerificationOutcome.COMPLETED - if success - else VerificationOutcome.FAILED, - exit_code=bash_result.exit_code, - stdout=bash_result.stdout, - stderr=bash_result.stderr, + outcome=VerificationOutcome.STILL_RUNNING, + detail="Command still running", ) + + success = bash_result.exit_code == 0 + logger.info( + "Verified run status: exit_code=%s, success=%s", + bash_result.exit_code, + success, + extra=extra, + ) + + return VerificationResult( + outcome=VerificationOutcome.COMPLETED + if success + else VerificationOutcome.FAILED, + exit_code=bash_result.exit_code, + stdout=bash_result.stdout, + stderr=bash_result.stderr, + ) diff --git a/openhands/automation/utils/conversation_outcome.py b/openhands/automation/utils/conversation_outcome.py index 515c3604..fbd3bea7 100644 --- a/openhands/automation/utils/conversation_outcome.py +++ b/openhands/automation/utils/conversation_outcome.py @@ -2,21 +2,27 @@ from __future__ import annotations +import asyncio import json import logging from typing import Any +from uuid import UUID import httpx from openhands.automation.backends import get_backend +from openhands.automation.backends.providers.existing import ( + ExistingAgentServerProvider, +) from openhands.automation.config import get_config from openhands.automation.models import AutomationRun from openhands.automation.utils.sandbox import get_sandbox_agent_url +from openhands.sdk import RemoteConversation +from openhands.sdk.workspace import RemoteWorkspace logger = logging.getLogger(__name__) -ACTION_EVENT_KIND = "openhands.sdk.event.llm_convertible.action.ActionEvent" FINISH_TOOL_NAME = "finish" @@ -50,29 +56,39 @@ def latest_finish_tool_response_from_events( return None +def _fetch_latest_finish_tool_response( + agent_url: str, + session_key: str, + conversation_id: str, +) -> Any | None: + workspace = RemoteWorkspace(host=agent_url, api_key=session_key, working_dir="/") + conversation = None + try: + conversation = RemoteConversation.attach( + workspace=workspace, + conversation_id=UUID(conversation_id), + visualizer=None, + ) + events = [event.model_dump(mode="json") for event in conversation.state.events] + return latest_finish_tool_response_from_events(list(reversed(events))) + finally: + if conversation is not None: + conversation.close() + workspace.reset_client() + + async def fetch_latest_finish_tool_response( - client: httpx.AsyncClient, agent_url: str, session_key: str, conversation_id: str, ) -> Any | None: - """Fetch recent conversation actions and return the latest finish response.""" - response = await client.get( - f"{agent_url.rstrip('/')}/api/conversations/{conversation_id}/events/search", - params={ - "kind": ACTION_EVENT_KIND, - "sort_order": "TIMESTAMP_DESC", - "limit": 100, - }, - headers={"X-Session-API-Key": session_key}, - timeout=30.0, + """Read raw FinishTool arguments through the SDK's typed conversation state.""" + return await asyncio.to_thread( + _fetch_latest_finish_tool_response, + agent_url, + session_key, + conversation_id, ) - response.raise_for_status() - page = response.json() - items = page.get("items") if isinstance(page, dict) else None - if not isinstance(items, list): - return None - return latest_finish_tool_response_from_events(items) async def fetch_latest_finish_tool_response_for_run( @@ -82,20 +98,19 @@ async def fetch_latest_finish_tool_response_for_run( """Best-effort lookup of the latest raw FinishTool response for a run.""" try: backend = get_backend(run) + provider = backend.provider async with httpx.AsyncClient(timeout=60.0) as client: - if backend.is_local_mode: - ctx = await backend.get_execution_context(client) + if isinstance(provider, ExistingAgentServerProvider): return await fetch_latest_finish_tool_response( - client, - ctx.agent_url, - ctx.session_key, + provider.agent_server_url, + await provider.get_api_key(), conversation_id, ) if not run.sandbox_id: return None - api_key = await backend.get_api_key() + api_key = await provider.get_api_key() result = await get_sandbox_agent_url( client, get_config().service.openhands_api_base_url, @@ -106,7 +121,6 @@ async def fetch_latest_finish_tool_response_for_run( return None agent_url, session_key = result return await fetch_latest_finish_tool_response( - client, agent_url, session_key, conversation_id, diff --git a/openhands/automation/utils/conversation_turn.py b/openhands/automation/utils/conversation_turn.py index 2dbdbe3b..97413ca9 100644 --- a/openhands/automation/utils/conversation_turn.py +++ b/openhands/automation/utils/conversation_turn.py @@ -16,14 +16,20 @@ import logging import time from typing import Any, Final +from uuid import UUID import httpx from openhands.automation.backends import get_backend +from openhands.automation.backends.providers.existing import ( + ExistingAgentServerProvider, +) from openhands.automation.config import get_config from openhands.automation.models import AutomationRun from openhands.automation.utils.log_context import log_extra from openhands.automation.utils.sandbox import get_sandbox_agent_url, resume_sandbox +from openhands.sdk import RemoteConversation +from openhands.sdk.workspace import RemoteWorkspace logger = logging.getLogger("automation.conversation_turn") @@ -184,14 +190,14 @@ async def _resolve_agent_server( ) -> tuple[str, str] | None: """Find the agent server holding this run's conversation.""" backend = get_backend(run) - if backend.is_local_mode: - # Side-effect free here; the cloud backend's version creates a sandbox. - ctx = await backend.get_execution_context(client) - return ctx.agent_url, ctx.session_key + provider = backend.provider + if isinstance(provider, ExistingAgentServerProvider): + # Resolving an existing conversation must not provision another one. + return provider.agent_server_url, await provider.get_api_key() if not run.sandbox_id: return None - api_key = await backend.get_api_key() + api_key = await provider.get_api_key() api_url = get_config().service.openhands_api_base_url resolved = await get_sandbox_agent_url(client, api_url, api_key, run.sandbox_id) if resolved is not None: @@ -227,6 +233,26 @@ async def _resume_and_wait( await asyncio.sleep(RESUME_POLL_SECONDS) +def _send_turn( + agent_url: str, session_key: str, conversation_id: str, text: str, wake_agent: bool +) -> None: + workspace = RemoteWorkspace(host=agent_url, api_key=session_key, working_dir="/") + conversation = None + try: + conversation = RemoteConversation.attach( + workspace=workspace, + conversation_id=UUID(conversation_id), + visualizer=None, + ) + conversation.send_message(text) + if wake_agent: + conversation.run(blocking=False) + finally: + if conversation is not None: + conversation.close() + workspace.reset_client() + + async def send_conversation_turn( run: AutomationRun, conversation_id: str, @@ -257,27 +283,25 @@ async def send_conversation_turn( CONVERSATION_WAIT_SECONDS if run.completed_at is None else 0 ) while True: - response = await client.post( - f"{agent_url.rstrip('/')}/api/conversations/" - f"{conversation_id}/events", - json={ - "role": "user", - "content": [{"type": "text", "text": text}], - # False leaves the message in history unanswered. It - # does not stop a loop already running from reading it. - "run": wake_agent, - }, - headers={"X-Session-API-Key": session_key}, - ) - if response.status_code != 404 or time.monotonic() >= deadline: + try: + await asyncio.to_thread( + _send_turn, + agent_url, + session_key, + conversation_id, + text, + wake_agent, + ) break + except httpx.HTTPStatusError as exc: + if exc.response.status_code != 404 or time.monotonic() >= deadline: + raise logger.info( "Conversation %s is not open yet; waiting for it", conversation_id, extra=extra, ) await asyncio.sleep(CONVERSATION_POLL_SECONDS) - response.raise_for_status() except Exception as exc: logger.info( "Could not send a turn to conversation %s: %s", diff --git a/openhands/automation/utils/run.py b/openhands/automation/utils/run.py index 7fe41731..106ddd69 100644 --- a/openhands/automation/utils/run.py +++ b/openhands/automation/utils/run.py @@ -181,6 +181,7 @@ async def create_pending_run( now = utcnow() run = AutomationRun( + execution_scope=automation.execution_scope, id=uuid.uuid4(), automation_id=automation.id, status=AutomationRunStatus.PENDING, diff --git a/openhands/automation/utils/sandbox.py b/openhands/automation/utils/sandbox.py index 57e3c4c1..e80acee2 100644 --- a/openhands/automation/utils/sandbox.py +++ b/openhands/automation/utils/sandbox.py @@ -288,7 +288,7 @@ async def verify_run_status( # Get last bash command result, scoped to this run's command if known bash_result = await get_last_bash_command_result( - client, agent_url, session_key, command_id=bash_command_id + agent_url, session_key, command_id=bash_command_id ) if not bash_result.found: diff --git a/openhands/automation/utils/webhook.py b/openhands/automation/utils/webhook.py index 8826945d..91a5eb51 100644 --- a/openhands/automation/utils/webhook.py +++ b/openhands/automation/utils/webhook.py @@ -260,6 +260,7 @@ async def create_automation_run( The created AutomationRun instance """ run = AutomationRun( + execution_scope=automation.execution_scope, id=uuid.uuid4(), automation_id=automation.id, status=AutomationRunStatus.PENDING, diff --git a/openhands/automation/watchdog.py b/openhands/automation/watchdog.py index 0f344bd8..2a7d0c6e 100644 --- a/openhands/automation/watchdog.py +++ b/openhands/automation/watchdog.py @@ -1,10 +1,12 @@ """Staleness watchdog for stuck RUNNING automation runs. -Periodically scans for runs stuck in RUNNING state past their pre-computed -``timeout_at`` deadline. Before marking as FAILED, attempts to verify the -actual run status by querying the execution environment. A verification -result that means "the bash command may still be executing" defers the -deadline (bounded by a hard cap) instead of terminalizing the run. +Periodically verifies detached commands and scans for runs stuck in RUNNING +state past their pre-computed ``timeout_at`` deadline. Once a command ID is +known, every scan can observe its terminal result without waiting for the +deadline or relying on an optional completion callback. A verification result +that means "the bash command may still be executing" leaves its existing +deadline in place. Runs that never acquired a command ID are checked only after +that deadline. The ``timeout_at`` column is set to a provisioning-phase deadline when the dispatcher transitions a run to RUNNING (see ``mark_run_status``), then @@ -37,7 +39,7 @@ from sqlalchemy.orm import selectinload from openhands.automation.backends import ExecutionBackend, get_backend -from openhands.automation.backends.local import local_runs_root +from openhands.automation.backends.providers.existing import local_runs_root from openhands.automation.config import Settings, get_config from openhands.automation.models import ( Automation, @@ -166,15 +168,17 @@ def _loaded_automation(run: AutomationRun) -> Automation | None: return run.automation -def _should_cleanup_sandbox_after_terminal( +def _should_cleanup_runtime_after_terminal( run: AutomationRun, keep_alive: bool | None ) -> bool: - """Return whether watchdog should explicitly delete this run's sandbox. + """Return whether watchdog should release this run's execution runtime. - A `continue_conversation` automation is forced keep_alive at creation, so - the sandbox carrying a live conversation is already excluded here. + A kept conversation retains its runtime; other provisioned sandboxes and + conversation-scoped runtimes are released after terminal verification. """ - return bool(run.sandbox_id) and keep_alive is not True + return ( + bool(run.sandbox_id) or run.execution_scope == "conversation" + ) and keep_alive is not True async def _defer_sandbox_cleanup( @@ -399,8 +403,8 @@ async def _verify_and_mark_run( ) if result.rowcount > 0: keep_alive = await _get_automation_keep_alive(session, run) - if _should_cleanup_sandbox_after_terminal(run, keep_alive): - if settings.sandbox_cleanup_delay_seconds > 0: + if _should_cleanup_runtime_after_terminal(run, keep_alive): + if settings.sandbox_cleanup_delay_seconds > 0 and run.sandbox_id: await _defer_sandbox_cleanup(session, run, backend, settings, now) else: try: @@ -468,7 +472,7 @@ async def _verify_and_mark_run( # Clean up resources via backend only when the automation owns explicit # cleanup. Otherwise, leave cleanup to the runtime TTL reaper. keep_alive = await _get_automation_keep_alive(session, run) - if _should_cleanup_sandbox_after_terminal(run, keep_alive): + if _should_cleanup_runtime_after_terminal(run, keep_alive): if settings.sandbox_cleanup_delay_seconds > 0: await _defer_sandbox_cleanup(session, run, backend, settings, now) else: @@ -543,11 +547,11 @@ async def mark_stale_runs( session_factory: async_sessionmaker[AsyncSession], settings: Settings, ) -> int: - """Find and process stale RUNNING runs. + """Verify RUNNING commands and process stale runs. - A run is stale if ``timeout_at < now()``. Before marking as FAILED, - attempts to verify the actual status by querying the sandbox. Uses - optimistic locking so concurrent callbacks win. + Every detached command with a recorded Bash command ID is polled on each + scan. Runs that have not started a command are checked after their timeout. + Verification uses optimistic locking so a concurrent callback can win. Each run is processed in its own session so that row locks are released immediately after commit rather than held for the duration of the batch. @@ -559,18 +563,21 @@ async def mark_stale_runs( marked = 0 async with session_factory() as session: - # Fetch stale run IDs only — close this session before doing any + # Fetch eligible run IDs only — close this session before doing any # per-run work so we don't hold locks across slow verify calls. result = await session.execute( select(AutomationRun.id).where( AutomationRun.status == AutomationRunStatus.RUNNING, AutomationRun.timeout_at.isnot(None), - AutomationRun.timeout_at < now, + ( + AutomationRun.bash_command_id.isnot(None) + | (AutomationRun.timeout_at < now) + ), ) ) - stale_run_ids = list(result.scalars().all()) + eligible_run_ids = list(result.scalars().all()) - for run_id in stale_run_ids: + for run_id in eligible_run_ids: async with session_factory() as session: # Re-fetch with automation relationship inside a fresh session. result = await session.execute( diff --git a/pyproject.toml b/pyproject.toml index eea21177..4c239927 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,8 @@ dependencies = [ "google-cloud-storage>=2.18", "httpx>=0.27", "jmespath>=1.0", - "openhands-sdk==1.46.0", + # Temporary integration pin for software-agent-sdk#5010; replace with its release. + "openhands-sdk @ git+https://github.com/OpenHands/software-agent-sdk.git@e9392e7cf35fcb011acf2eca825a80f9a22774ea#subdirectory=openhands-sdk", "openhands-workspace==1.46.0", "pg8000>=1.31", "prometheus-client>=0.19", diff --git a/tests/conftest.py b/tests/conftest.py index 45fe1074..7e771d79 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -203,3 +203,28 @@ def mock_settings(): service_key="test-service-key", base_url="http://localhost:8000", ) + + +@pytest.fixture +def sdk_http_transport(monkeypatch): + """Intercept SDK-owned HTTP pools without borrowing a caller's client.""" + import httpx + + async_init = httpx.AsyncClient.__init__ + sync_init = httpx.Client.__init__ + + def install(handler): + transport = httpx.MockTransport(handler) + + def init_async(self, *args, **kwargs): + kwargs.setdefault("transport", transport) + async_init(self, *args, **kwargs) + + def init_sync(self, *args, **kwargs): + kwargs.setdefault("transport", transport) + sync_init(self, *args, **kwargs) + + monkeypatch.setattr(httpx.AsyncClient, "__init__", init_async) + monkeypatch.setattr(httpx.Client, "__init__", init_sync) + + return install diff --git a/tests/test_backends.py b/tests/test_backends.py index 8f48eb0e..3b840a6e 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -7,13 +7,14 @@ import pytest from openhands.automation.backends import ( - CloudSandboxBackend, + CloudSandboxAgentServerProvider, ExecutionContext, - LocalAgentServerBackend, + ExistingAgentServerProvider, + RunBackend, get_backend, ) -from openhands.automation.backends.cloud import _concurrency_limit_detail -from openhands.automation.backends.local import ( +from openhands.automation.backends.providers.cloud import _concurrency_limit_detail +from openhands.automation.backends.providers.existing import ( local_runs_root, resolve_local_workspace_base, ) @@ -47,8 +48,8 @@ def test_cloud_mode_fields(self): assert ctx.api_key == "api-key" -class TestLocalAgentServerBackend: - """Tests for LocalAgentServerBackend.""" +class TestExistingAgentServerProvider: + """Tests for ExistingAgentServerProvider.""" @pytest.fixture def mock_run(self): @@ -60,18 +61,18 @@ def mock_run(self): run.bash_command_id = None return run - def test_is_local_mode(self, mock_run): - """LocalAgentServerBackend reports local mode.""" - backend = LocalAgentServerBackend( + def test_provisions_agent_server(self, mock_run): + """An existing endpoint does not provision an Agent Server.""" + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="test-key", run=mock_run, ) - assert backend.is_local_mode is True + assert backend.provisions_agent_server is False def test_strips_trailing_slash(self, mock_run): """URL trailing slash is stripped.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000/", api_key="test-key", run=mock_run, @@ -81,7 +82,7 @@ def test_strips_trailing_slash(self, mock_run): @pytest.mark.asyncio async def test_get_execution_context_returns_context(self, mock_run): """get_execution_context() returns ExecutionContext with configured values.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="local-key", run=mock_run, @@ -95,7 +96,7 @@ async def test_get_execution_context_returns_context(self, mock_run): @pytest.mark.asyncio async def test_release_context_is_noop(self, mock_run): """release_context() is a no-op for local backend.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="local-key", run=mock_run, @@ -110,7 +111,7 @@ async def test_release_context_is_noop(self, mock_run): @pytest.mark.asyncio async def test_get_api_key_returns_config_key(self, mock_run): """get_api_key() returns the pre-configured API key.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="local-key", run=mock_run, @@ -120,7 +121,7 @@ async def test_get_api_key_returns_config_key(self, mock_run): def test_build_env_vars(self, mock_run): """build_env_vars() returns required env vars for local mode.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="agent-server-key", run=mock_run, @@ -142,7 +143,7 @@ def test_build_env_vars(self, mock_run): def test_build_env_vars_custom_workspace_base(self, mock_run): """build_env_vars() uses custom workspace_base when provided.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="agent-key", run=mock_run, @@ -161,7 +162,7 @@ def test_build_env_vars_custom_workspace_base(self, mock_run): def test_build_env_vars_no_callback_key(self, mock_run): """build_env_vars() omits callback key when callback_api_key is not set.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="agent-key", run=mock_run, @@ -176,7 +177,7 @@ def test_build_env_vars_no_callback_key(self, mock_run): def test_build_env_vars_sandbox_url_override(self, mock_run): """sandbox_agent_server_url overrides AGENT_SERVER_URL only in the sandbox export — the backend itself still uses agent_server_url.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:18000", api_key="agent-key", run=mock_run, @@ -191,7 +192,7 @@ def test_build_env_vars_sandbox_url_override(self, mock_run): def test_build_env_vars_sandbox_url_falls_back(self, mock_run): """When sandbox_agent_server_url is None or empty, the in-sandbox AGENT_SERVER_URL falls back to agent_server_url (current behaviour).""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="agent-key", run=mock_run, @@ -203,7 +204,7 @@ def test_build_env_vars_sandbox_url_falls_back(self, mock_run): def test_get_work_dir_default_workspace(self, mock_run): """get_work_dir() returns isolated directory with ~ expanded.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="test-key", run=mock_run, @@ -215,7 +216,7 @@ def test_get_work_dir_default_workspace(self, mock_run): def test_get_work_dir_custom_workspace(self, mock_run): """get_work_dir() uses custom workspace_base when provided.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="test-key", run=mock_run, @@ -238,7 +239,7 @@ def test_workspace_base_resolution_contract(self, mock_run, tmp_path, monkeypatc for configured, expected in cases: assert Path(resolve_local_workspace_base(configured)) == expected assert local_runs_root(configured) == expected / "automation-runs" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="test-key", run=mock_run, @@ -256,7 +257,7 @@ async def test_verify_run_calls_agent_server(self, mock_run): the most recent BashOutput on a shared agent server. """ mock_run.bash_command_id = "abc123def456" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="local-key", run=mock_run, @@ -264,7 +265,7 @@ async def test_verify_run_calls_agent_server(self, mock_run): mock_result = MagicMock(verified=True, exit_code=0) with patch( - "openhands.automation.backends.local.verify_run_on_agent_server", + "openhands.automation.backends.providers.existing.verify_run_on_agent_server", new_callable=AsyncMock, return_value=mock_result, ) as mock_verify: @@ -280,7 +281,7 @@ async def test_verify_run_calls_agent_server(self, mock_run): @pytest.mark.asyncio async def test_cleanup_after_verification_is_noop(self, mock_run): """cleanup_after_verification() is a no-op for local backend.""" - backend = LocalAgentServerBackend( + backend = ExistingAgentServerProvider( agent_server_url="http://localhost:3000", api_key="local-key", run=mock_run, @@ -289,8 +290,8 @@ async def test_cleanup_after_verification_is_noop(self, mock_run): await backend.cleanup_after_verification("run-123") -class TestCloudSandboxBackend: - """Tests for CloudSandboxBackend.""" +class TestCloudSandboxAgentServerProvider: + """Tests for CloudSandboxAgentServerProvider.""" @pytest.fixture def mock_run(self): @@ -301,14 +302,16 @@ def mock_run(self): run.bash_command_id = None return run - def test_is_local_mode(self, mock_run): - """CloudSandboxBackend reports cloud mode.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) - assert backend.is_local_mode is False + def test_provisions_agent_server(self, mock_run): + """A Cloud sandbox provisions an Agent Server.""" + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) + assert backend.provisions_agent_server is True def test_strips_trailing_slash(self, mock_run): """URL trailing slash is stripped.""" - backend = CloudSandboxBackend( + backend = CloudSandboxAgentServerProvider( api_url="https://app.all-hands.dev/", run=mock_run ) assert backend.api_url == "https://app.all-hands.dev" @@ -322,7 +325,7 @@ def test_find_agent_server_url_found(self): ], "session_api_key": "session-key", } - result = CloudSandboxBackend._find_agent_server_url(sandbox) + result = CloudSandboxAgentServerProvider._find_agent_server_url(sandbox) assert result == ("http://agent.example.com", "session-key") def test_find_agent_server_url_not_found(self): @@ -332,22 +335,24 @@ def test_find_agent_server_url_not_found(self): {"name": "OTHER", "url": "http://other.example.com"}, ], } - result = CloudSandboxBackend._find_agent_server_url(sandbox) + result = CloudSandboxAgentServerProvider._find_agent_server_url(sandbox) assert result is None def test_find_agent_server_url_empty(self): """_find_agent_server_url handles empty exposed_urls.""" sandbox = {"exposed_urls": None} - result = CloudSandboxBackend._find_agent_server_url(sandbox) + result = CloudSandboxAgentServerProvider._find_agent_server_url(sandbox) assert result is None @pytest.mark.asyncio async def test_get_api_key_mints_per_user_key(self, mock_run): """get_api_key() mints a per-user key via service key.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) with patch( - "openhands.automation.backends.cloud.get_api_key_for_automation_run", + "openhands.automation.backends.providers.cloud.get_api_key_for_automation_run", new_callable=AsyncMock, return_value="sk-user-minted", ) as mock_mint: @@ -358,10 +363,12 @@ async def test_get_api_key_mints_per_user_key(self, mock_run): @pytest.mark.asyncio async def test_build_env_vars(self, mock_run): """build_env_vars() includes Cloud API credentials after key is minted.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) with patch( - "openhands.automation.backends.cloud.get_api_key_for_automation_run", + "openhands.automation.backends.providers.cloud.get_api_key_for_automation_run", new_callable=AsyncMock, return_value="sk-user", ): @@ -376,7 +383,9 @@ async def test_build_env_vars(self, mock_run): def test_build_env_vars_raises_without_api_key(self, mock_run): """build_env_vars() raises if API key not initialized.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) with pytest.raises(RuntimeError, match="API key not initialized"): backend.build_env_vars() @@ -384,7 +393,9 @@ def test_build_env_vars_raises_without_api_key(self, mock_run): async def test_verify_run_without_sandbox_id(self, mock_run): """verify_run() returns error when sandbox_id is missing.""" mock_run.sandbox_id = None - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) result = await backend.verify_run("run-123") assert result.verified is False @@ -397,17 +408,19 @@ async def test_verify_run_calls_verify_run_status(self, mock_run): run's specific command. """ mock_run.bash_command_id = "deadbeefcafebabe" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) mock_result = MagicMock(verified=True, exit_code=0) with ( patch( - "openhands.automation.backends.cloud.get_api_key_for_automation_run", + "openhands.automation.backends.providers.cloud.get_api_key_for_automation_run", new_callable=AsyncMock, return_value="sk-user", ), patch( - "openhands.automation.backends.cloud.verify_run_status", + "openhands.automation.backends.providers.cloud.verify_run_status", new_callable=AsyncMock, return_value=mock_result, ) as mock_verify, @@ -425,16 +438,18 @@ async def test_verify_run_calls_verify_run_status(self, mock_run): @pytest.mark.asyncio async def test_cleanup_after_verification_deletes_sandbox(self, mock_run): """cleanup_after_verification() deletes sandbox when called.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) with ( patch( - "openhands.automation.backends.cloud.get_api_key_for_automation_run", + "openhands.automation.backends.providers.cloud.get_api_key_for_automation_run", new_callable=AsyncMock, return_value="sk-user", ), patch( - "openhands.automation.backends.cloud.cleanup_sandbox", + "openhands.automation.backends.providers.cloud.cleanup_sandbox", new_callable=AsyncMock, ) as mock_cleanup, ): @@ -455,10 +470,11 @@ def mock_run(self): """Create a mock AutomationRun.""" run = MagicMock() run.sandbox_id = "sandbox-123" + run.execution_scope = "run" return run def test_local_mode(self, monkeypatch, mock_run): - """get_backend returns LocalAgentServerBackend when configured.""" + """Run execution uses the configured Agent Server.""" monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://localhost:3000") monkeypatch.setenv("AUTOMATION_AGENT_SERVER_API_KEY", "local-key") @@ -468,12 +484,13 @@ def test_local_mode(self, monkeypatch, mock_run): clear_config_cache() backend = get_backend(mock_run) - assert isinstance(backend, LocalAgentServerBackend) - assert backend.agent_server_url == "http://localhost:3000" - assert backend.api_key == "local-key" + assert isinstance(backend, RunBackend) + assert isinstance(backend.provider, ExistingAgentServerProvider) + assert backend.provider.agent_server_url == "http://localhost:3000" + assert backend.provider.api_key == "local-key" def test_cloud_mode(self, monkeypatch, mock_run): - """get_backend returns CloudSandboxBackend when not in local mode.""" + """Run execution provisions an Agent Server in a Cloud sandbox.""" monkeypatch.delenv("AUTOMATION_AGENT_SERVER_URL", raising=False) monkeypatch.setenv( "AUTOMATION_OPENHANDS_API_BASE_URL", "https://app.all-hands.dev" @@ -485,8 +502,9 @@ def test_cloud_mode(self, monkeypatch, mock_run): clear_config_cache() backend = get_backend(mock_run) - assert isinstance(backend, CloudSandboxBackend) - assert backend.api_url == "https://app.all-hands.dev" + assert isinstance(backend, RunBackend) + assert isinstance(backend.provider, CloudSandboxAgentServerProvider) + assert backend.provider.api_url == "https://app.all-hands.dev" class TestConcurrencyLimitDetection: @@ -559,7 +577,9 @@ def mock_run(self): async def test_create_sandbox_raises_and_does_not_retry(self, mock_run): """A concurrency-limit 429 raises ConcurrencyLimitReachedError on the first attempt — retrying cannot free a slot, so it must not be retried.""" - backend = CloudSandboxBackend(api_url="https://app.all-hands.dev", run=mock_run) + backend = CloudSandboxAgentServerProvider( + api_url="https://app.all-hands.dev", run=mock_run + ) req = httpx.Request("POST", "https://app.all-hands.dev/api/v1/sandboxes") resp = httpx.Response( diff --git a/tests/test_cancel_run.py b/tests/test_cancel_run.py index ee54032e..347155b1 100644 --- a/tests/test_cancel_run.py +++ b/tests/test_cancel_run.py @@ -2,6 +2,8 @@ import uuid +import pytest + from openhands.automation.models import Automation, AutomationRun, AutomationRunStatus from openhands.automation.utils import utcnow @@ -156,3 +158,36 @@ async def test_cancel_same_org_other_users_run(async_client, async_session): resp = await async_client.post(f"/api/automation/v1/runs/{run.id}/cancel") assert resp.status_code == 200 assert resp.json()["status"] == "CANCELLED" + + +@pytest.mark.parametrize("cleanup_fails", [False, True]) +async def test_cancel_conversation_run_without_cloud_id( + async_client, async_session, monkeypatch, cleanup_fails +): + from unittest.mock import AsyncMock, Mock + + from openhands.automation import backends + from openhands.automation.config import clear_config_cache + + monkeypatch.setenv("AUTOMATION_AGENT_PROFILE", str(uuid.uuid4())) + clear_config_cache() + backend = Mock( + cleanup_after_verification=AsyncMock( + side_effect=RuntimeError("Runtime temporarily unreachable") + if cleanup_fails + else None + ) + ) + monkeypatch.setattr(backends, "get_backend", lambda run: backend) + try: + _, run = await _create_automation_with_run( + async_session, status=AutomationRunStatus.RUNNING + ) + run.execution_scope = "conversation" + run_id = str(run.id) + resp = await async_client.post(f"/api/automation/v1/runs/{run_id}/cancel") + assert resp.status_code == 200 + assert resp.json()["status"] == "CANCELLED" + backend.cleanup_after_verification.assert_awaited_once_with(run_id) + finally: + clear_config_cache() diff --git a/tests/test_conversation_backend.py b/tests/test_conversation_backend.py new file mode 100644 index 00000000..64eaf222 --- /dev/null +++ b/tests/test_conversation_backend.py @@ -0,0 +1,217 @@ +"""Identical bundle-facing contract across advertised workspace runtimes.""" + +import asyncio +import json +from uuid import uuid4 + +import httpx +import pytest + +from openhands.automation.backends.conversation import ConversationBackend +from openhands.automation.backends.providers.existing import ( + ExistingAgentServerProvider, +) +from openhands.automation.backends.run import RunBackend +from openhands.automation.execution import execute_in_context +from openhands.automation.models import Automation, AutomationRun +from openhands.automation.subjects import conversation_id_for +from openhands.sdk.conversation.request import StartConversationRequest + + +@pytest.mark.asyncio +@pytest.mark.parametrize("runtime", ["local", "docker"]) +@pytest.mark.parametrize("subject", [None, "org/repo/42"]) +async def test_same_bundle_contract_and_scoped_execution( + runtime, subject, tmp_path, sdk_http_transport, monkeypatch +): + automation = Automation( + id=uuid4(), + org_id=uuid4(), + name="portable workflow", + trigger={"type": "event", "source": "github-events"}, + ) + run = AutomationRun(id=uuid4(), automation=automation, subject_key=subject) + conversation_id = ( + conversation_id_for(automation.org_id, automation.id, "github-events", subject) + if subject + else str(run.id) + ) + provider = ExistingAgentServerProvider( + "http://server", + "host-key", + run, + workspace_base=str(tmp_path), + callback_api_key="callback-key", + ) + backend = ConversationBackend(provider, agent_profile_id=uuid4()) + requests = [] + from unittest.mock import MagicMock + + from openhands.sdk import LLM, Agent + + monkeypatch.setattr( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient", + MagicMock(), + ) + agent = Agent(llm=LLM(model="test-model", api_key="test-key")) + + def respond(request): + requests.append(request) + if request.url.path == "/server_info": + return httpx.Response(200, json={"conversation_runtime": runtime}) + if ( + request.url.path == f"/api/conversations/{conversation_id}" + and request.method == "GET" + ): + return httpx.Response(404) + if request.url.path == "/api/conversations": + StartConversationRequest.model_validate_json(request.content) + return httpx.Response( + 200, + json={ + "id": conversation_id, + "agent": agent.model_dump(mode="json"), + "max_iterations": 160, + }, + ) + return httpx.Response( + 200, json={"id": "command", "session_api_key": "inner-key", "items": []} + ) + + sdk_http_transport(respond) + async with httpx.AsyncClient() as client: + context = await backend.get_execution_context(client) + env = backend.build_env_vars() + assert set(env) == { + "AGENT_SERVER_URL", + "SESSION_API_KEY", + "AUTOMATION_CONVERSATION_ID", + "WORKSPACE_BASE", + } + assert env["AUTOMATION_CONVERSATION_ID"] == conversation_id + assert env["WORKSPACE_BASE"] == backend.get_work_dir(str(run.id)) + assert env["SESSION_API_KEY"] == ( + "inner-key" if runtime == "docker" else "host-key" + ) + if runtime == "docker": + assert "host-key" not in str(env) + creations = [r for r in requests if r.url.path == "/api/conversations"] + assert len(creations) == 1 + creation = creations[0] + assert creation.method == "POST" + assert not any( + r.method == "GET" and r.url.path == f"/api/conversations/{conversation_id}" + for r in requests + ) + payload = json.loads(creation.content) + assert payload["workspace"]["working_dir"] == env["WORKSPACE_BASE"] + assert payload["conversation_id"] == conversation_id + assert payload["agent_profile_id"] == str(backend.agent_profile_id) + assert payload["max_iterations"] == 160 + assert payload["tags"] == {"automationrun": str(run.id)} + result = await execute_in_context( + context.agent_url, + context.session_key, + "python3 main.py", + b"same-bundle", + env["WORKSPACE_BASE"], + env, + run_id=str(run.id), + runtime_conversation_id=context.runtime_conversation_id, + ) + assert result.success + assert result.bash_command_id == "command" + execution = [ + r for r in requests if "/file/" in r.url.path or "/bash/" in r.url.path + ] + assert execution + assert all( + r.url.path.startswith(f"/api/conversations/{conversation_id}/") + for r in execution + ) + before = len(requests) + await backend.release_context(client, context) + if runtime == "local": + assert len(requests) == before + else: + assert requests[-1].method == "DELETE" + assert ( + requests[-1].url.path == f"/api/conversations/{conversation_id}/runtime" + ) + + +@pytest.mark.parametrize( + ("execution_scope", "expected_type"), + [("run", RunBackend), ("conversation", ConversationBackend)], +) +def test_execution_scope_selects_backend(monkeypatch, execution_scope, expected_type): + from openhands.automation.backends import get_backend + from openhands.automation.config import clear_config_cache + + automation_id = uuid4() + profile_id = uuid4() + monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") + monkeypatch.setenv("AUTOMATION_AGENT_PROFILE", str(profile_id)) + clear_config_cache() + try: + backend = get_backend( + AutomationRun( + id=uuid4(), + automation_id=automation_id, + execution_scope=execution_scope, + ) + ) + assert type(backend) is expected_type + if isinstance(backend, ConversationBackend): + assert backend.agent_profile_id == profile_id + else: + assert type(backend.provider) is ExistingAgentServerProvider + finally: + clear_config_cache() + + +def test_conversation_execution_requires_profile(monkeypatch): + from openhands.automation.backends import get_backend + from openhands.automation.config import clear_config_cache + + monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") + monkeypatch.delenv("AUTOMATION_AGENT_PROFILE", raising=False) + clear_config_cache() + try: + with pytest.raises(ValueError, match="requires an agent profile"): + get_backend(AutomationRun(id=uuid4(), execution_scope="conversation")) + finally: + clear_config_cache() + + +@pytest.mark.asyncio +async def test_cleanup_uses_sdk_lifecycle_timeout(): + # A runtime release can exceed HTTPX's implicit five-second client timeout. + requests = [] + + async def release(reader, writer): + try: + requests.append(await reader.readuntil(b"\r\n\r\n")) + await asyncio.sleep(5.1) + writer.write(b"HTTP/1.1 204 No Content\r\nConnection: close\r\n\r\n") + await writer.drain() + finally: + writer.close() + await writer.wait_closed() + + server = await asyncio.start_server(release, "127.0.0.1", 0) + port = server.sockets[0].getsockname()[1] + run = AutomationRun(id=uuid4(), automation=Automation(name="slow cleanup")) + provider = ExistingAgentServerProvider( + f"http://127.0.0.1:{port}", + "key", + run, + ) + backend = ConversationBackend(provider, agent_profile_id=uuid4()) + backend._runtime_kind = "docker" + async with server: + await backend.cleanup_after_verification(str(run.id)) + assert len(requests) == 1 + assert requests[0].startswith( + f"DELETE /api/conversations/{run.id}/runtime HTTP/1.1".encode() + ) diff --git a/tests/test_conversation_outcome.py b/tests/test_conversation_outcome.py index 0d3f480f..5f10c8d1 100644 --- a/tests/test_conversation_outcome.py +++ b/tests/test_conversation_outcome.py @@ -1,14 +1,14 @@ import json from types import SimpleNamespace -from typing import cast +from typing import Any, cast -import httpx import pytest +from openhands.automation.backends.providers.existing import ExistingAgentServerProvider +from openhands.automation.backends.run import RunBackend from openhands.automation.models import AutomationRun from openhands.automation.utils import conversation_outcome as outcome_module from openhands.automation.utils.conversation_outcome import ( - ACTION_EVENT_KIND, fetch_latest_finish_tool_response, finish_tool_response_from_event, latest_finish_tool_response_from_events, @@ -101,36 +101,32 @@ def test_latest_finish_tool_response_does_not_fall_back_past_latest_finish(): @pytest.mark.asyncio -async def test_fetch_latest_finish_tool_response_queries_conversation_events(): - event = _finish_event( - { - "message": "Done", - "status": "success", - "outcome_summary": "Everything completed.", - } +async def test_fetch_latest_finish_tool_response_preserves_raw_sdk_events(monkeypatch): + from unittest.mock import MagicMock + + newest = {"message": "Done", "status": "success", "confidence": 0.9} + events = [MagicMock(), MagicMock()] + events[0].model_dump.return_value = _finish_event({"message": "older"}) + events[1].model_dump.return_value = _finish_event(newest) + conversation = MagicMock() + conversation.state.events = events + remote_conversation = MagicMock() + remote_conversation.attach.return_value = conversation + workspace = MagicMock() + monkeypatch.setattr(outcome_module, "RemoteConversation", remote_conversation) + monkeypatch.setattr(outcome_module, "RemoteWorkspace", lambda **kwargs: workspace) + + response = await fetch_latest_finish_tool_response( + "https://agent.example.com", + "session-key", + "e793aaea-50c7-4fd7-a686-2c76a6c2a80e", ) - async def handler(request: httpx.Request) -> httpx.Response: - assert request.url.path == "/api/conversations/conv-123/events/search" - assert request.url.params["kind"] == ACTION_EVENT_KIND - assert request.url.params["sort_order"] == "TIMESTAMP_DESC" - assert request.url.params["limit"] == "100" - assert request.headers["X-Session-API-Key"] == "session-key" - return httpx.Response(200, json={"items": [event]}) - - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - response = await fetch_latest_finish_tool_response( - client, - "https://agent.example.com", - "session-key", - "conv-123", - ) - - assert response == { - "message": "Done", - "status": "success", - "outcome_summary": "Everything completed.", - } + assert response == newest + remote_conversation.attach.assert_called_once() + assert "agent" not in remote_conversation.attach.call_args.kwargs + conversation.close.assert_called_once() + workspace.reset_client.assert_called_once() @pytest.mark.asyncio @@ -139,21 +135,25 @@ async def test_fetch_latest_finish_tool_response_for_run_uses_local_context( ): calls = {} - class FakeBackend: - is_local_mode = True + class FakeBackend(ExistingAgentServerProvider): + def __init__(self): + self.agent_server_url = "https://local-agent.example.com" + + async def get_api_key(self): + return "local-session-key" async def get_execution_context(self, client): - calls["context_client"] = client - return SimpleNamespace( - agent_url="https://local-agent.example.com", - session_key="local-session-key", - ) - - async def fake_fetch(client, agent_url, session_key, conversation_id): - calls["fetch"] = (client, agent_url, session_key, conversation_id) + pytest.fail("Reading an outcome must not provision a conversation") + + async def fake_fetch(agent_url, session_key, conversation_id): + calls["fetch"] = (agent_url, session_key, conversation_id) return {"status": "success"} - monkeypatch.setattr(outcome_module, "get_backend", lambda run: FakeBackend()) + monkeypatch.setattr( + outcome_module, + "get_backend", + lambda run: RunBackend(cast(Any, FakeBackend())), + ) monkeypatch.setattr(outcome_module, "fetch_latest_finish_tool_response", fake_fetch) run = cast(AutomationRun, SimpleNamespace(id="run-1", sandbox_id=None)) @@ -162,7 +162,6 @@ async def fake_fetch(client, agent_url, session_key, conversation_id): run, "conv-1" ) == {"status": "success"} assert calls["fetch"] == ( - calls["context_client"], "https://local-agent.example.com", "local-session-key", "conv-1", @@ -175,8 +174,8 @@ async def test_fetch_latest_finish_tool_response_for_run_uses_remote_sandbox( ): calls = {} - class FakeBackend: - is_local_mode = False + class FakeProvider: + provisions_agent_server = True async def get_api_key(self): calls["api_key_requested"] = True @@ -186,11 +185,15 @@ async def fake_get_sandbox_agent_url(client, api_url, api_key, sandbox_id): calls["sandbox_lookup"] = (client, api_url, api_key, sandbox_id) return "https://sandbox-agent.example.com", "sandbox-session-key" - async def fake_fetch(client, agent_url, session_key, conversation_id): - calls["fetch"] = (client, agent_url, session_key, conversation_id) + async def fake_fetch(agent_url, session_key, conversation_id): + calls["fetch"] = (agent_url, session_key, conversation_id) return {"status": "partial_success"} - monkeypatch.setattr(outcome_module, "get_backend", lambda run: FakeBackend()) + monkeypatch.setattr( + outcome_module, + "get_backend", + lambda run: RunBackend(cast(Any, FakeProvider())), + ) monkeypatch.setattr( outcome_module, "get_sandbox_agent_url", fake_get_sandbox_agent_url ) @@ -207,7 +210,6 @@ async def fake_fetch(client, agent_url, session_key, conversation_id): assert api_key == "sandbox-api-key" assert sandbox_id == "sandbox-123" assert calls["fetch"] == ( - lookup_client, "https://sandbox-agent.example.com", "sandbox-session-key", "conv-2", diff --git a/tests/test_conversation_turn.py b/tests/test_conversation_turn.py index 144c4e17..6af36112 100644 --- a/tests/test_conversation_turn.py +++ b/tests/test_conversation_turn.py @@ -1,16 +1,20 @@ """Tests for the request that continues a conversation. -`POST /api/conversations/{id}/events` with `run: true` is what -`continue_conversation` is built on; these tests pin its shape. +The SDK appends a message and separately triggers a nonblocking run. +These tests preserve buffering, resumed sandboxes, and bounded retries. """ import json from types import SimpleNamespace -from typing import cast +from typing import Any, cast +from uuid import uuid4 import httpx import pytest +from openhands.automation.backends.conversation import ConversationBackend +from openhands.automation.backends.providers.existing import ExistingAgentServerProvider +from openhands.automation.backends.run import RunBackend from openhands.automation.models import AutomationRun from openhands.automation.utils import conversation_turn as turn_module, utcnow from openhands.automation.utils.conversation_turn import ( @@ -19,33 +23,67 @@ ) -def fake_httpx(handler) -> SimpleNamespace: - """Stand in for the module's `httpx`, serving `handler` to every request.""" +CONVERSATION_ID = str(uuid4()) - def client(**kwargs): - return httpx.AsyncClient(transport=httpx.MockTransport(handler)) - return SimpleNamespace(AsyncClient=client) +@pytest.fixture +def conversation_transport(sdk_http_transport, monkeypatch): + from unittest.mock import MagicMock + from openhands.sdk import LLM, Agent -def local_backend(agent_url="https://local-agent.example.com"): - class FakeBackend: - is_local_mode = True - - async def get_execution_context(self, client): - return SimpleNamespace(agent_url=agent_url, session_key="local-key") - - return FakeBackend() + monkeypatch.setattr( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient", + MagicMock(), + ) + agent = Agent(llm=LLM(model="test-model", api_key="test-key")) + + def install(handler): + requests = [] + + def respond(request): + requests.append(request) + if request.url.path.endswith("/events/search"): + return httpx.Response(200, json={"items": []}) + if request.method == "GET": + return httpx.Response( + 200, + json={ + "id": CONVERSATION_ID, + "agent": agent.model_dump(mode="json"), + "max_iterations": 160, + }, + ) + if request.url.path.endswith("/run"): + return httpx.Response(200, json={}) + return handler(request) + + sdk_http_transport(respond) + return requests + + return install + + +def local_backend( + agent_url="https://local-agent.example.com", *, conversation_runtime=False +): + if conversation_runtime: + provider = ExistingAgentServerProvider(agent_url, "local-key", make_run()) + return ConversationBackend( + provider, + agent_profile_id=uuid4(), + ) + return RunBackend(ExistingAgentServerProvider(agent_url, "local-key", make_run())) def cloud_backend(): - class FakeBackend: - is_local_mode = False + class FakeProvider: + provisions_agent_server = True async def get_api_key(self): return "cloud-key" - return FakeBackend() + return RunBackend(cast(Any, FakeProvider())) def make_run(sandbox_id: str | None = None, *, finished: bool = False) -> AutomationRun: @@ -60,31 +98,45 @@ def make_run(sandbox_id: str | None = None, *, finished: bool = False) -> Automa @pytest.mark.asyncio -async def test_a_turn_is_a_user_message_that_starts_the_loop(monkeypatch): +@pytest.mark.parametrize("conversation_runtime", [False, True]) +async def test_a_turn_is_a_user_message_that_starts_the_loop( + monkeypatch, conversation_runtime, conversation_transport +): seen: dict = {} - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: seen["path"] = request.url.path seen["key"] = request.headers.get("X-Session-API-Key") seen["body"] = request.read() return httpx.Response(200, json={"success": True}) - monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + monkeypatch.setattr( + turn_module, + "get_backend", + lambda run: local_backend(conversation_runtime=conversation_runtime), + ) + requests = conversation_transport(handler) - assert await send_conversation_turn(make_run(), "conv-1", "another turn") is True + assert ( + await send_conversation_turn(make_run(), CONVERSATION_ID, "another turn") + is True + ) body = json.loads(seen["body"]) - assert seen["path"] == "/api/conversations/conv-1/events" + assert seen["path"] == f"/api/conversations/{CONVERSATION_ID}/events" assert seen["key"] == "local-key" - assert body["role"] == "user" - assert body["content"] == [{"type": "text", "text": "another turn"}] + assert body.get("role", "user") == "user" + assert [(c["type"], c["text"]) for c in body["content"]] == [ + ("text", "another turn") + ] # Without this the message lands in history unanswered. - assert body["run"] is True + assert any(r.url.path.endswith("/run") for r in requests) @pytest.mark.asyncio -async def test_wake_agent_false_appends_without_starting_the_loop(monkeypatch): +async def test_wake_agent_false_appends_without_starting_the_loop( + monkeypatch, conversation_transport +): """A trigger that buffers rather than interrupts. The turn still has to reach the agent server -- the conversation is what @@ -92,28 +144,30 @@ async def test_wake_agent_false_appends_without_starting_the_loop(monkeypatch): """ seen: dict = {} - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: seen["body"] = request.read() return httpx.Response(200, json={"success": True}) monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + requests = conversation_transport(handler) delivered = await send_conversation_turn( - make_run(), "conv-1", "for later", wake_agent=False + make_run(), CONVERSATION_ID, "for later", wake_agent=False ) assert delivered is True body = json.loads(seen["body"]) - assert body["run"] is False - assert body["content"] == [{"type": "text", "text": "for later"}] + assert not any(r.url.path.endswith("/run") for r in requests) + assert [(c["type"], c["text"]) for c in body["content"]] == [("text", "for later")] @pytest.mark.asyncio -async def test_a_cloud_run_is_reached_through_its_sandbox(monkeypatch): +async def test_a_cloud_run_is_reached_through_its_sandbox( + monkeypatch, conversation_transport +): seen: dict = {} - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: seen["host"] = request.url.host seen["key"] = request.headers.get("X-Session-API-Key") return httpx.Response(200, json={"success": True}) @@ -126,30 +180,36 @@ async def fake_get_sandbox_agent_url(client, api_url, api_key, sandbox_id): monkeypatch.setattr( turn_module, "get_sandbox_agent_url", fake_get_sandbox_agent_url ) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) - assert await send_conversation_turn(make_run("sbx-1"), "conv-1", "hi") is True + assert ( + await send_conversation_turn(make_run("sbx-1"), CONVERSATION_ID, "hi") is True + ) assert seen["sandbox_id"] == "sbx-1" assert seen["host"] == "sandbox.example.com" assert seen["key"] == "sandbox-key" @pytest.mark.asyncio -async def test_a_cloud_run_with_no_sandbox_never_sends(monkeypatch): - async def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover +async def test_a_cloud_run_with_no_sandbox_never_sends( + monkeypatch, conversation_transport +): + def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover raise AssertionError("should not have been called") monkeypatch.setattr(turn_module, "get_backend", lambda run: cloud_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) - assert await send_conversation_turn(make_run(None), "conv-1", "hi") is False + assert await send_conversation_turn(make_run(None), CONVERSATION_ID, "hi") is False @pytest.mark.asyncio -async def test_a_reaped_sandbox_is_a_false_not_an_exception(monkeypatch): +async def test_a_reaped_sandbox_is_a_false_not_an_exception( + monkeypatch, conversation_transport +): """The caller answers a False by starting a run.""" - async def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover + def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover raise AssertionError("should not have been called") async def fake_get_sandbox_agent_url(client, api_url, api_key, sandbox_id): @@ -164,13 +224,17 @@ async def fake_resume_sandbox(client, api_url, api_key, sandbox_id): turn_module, "get_sandbox_agent_url", fake_get_sandbox_agent_url ) monkeypatch.setattr(turn_module, "resume_sandbox", fake_resume_sandbox) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) - assert await send_conversation_turn(make_run("sbx-1"), "conv-1", "hi") is False + assert ( + await send_conversation_turn(make_run("sbx-1"), CONVERSATION_ID, "hi") is False + ) @pytest.mark.asyncio -async def test_a_paused_sandbox_is_resumed_rather_than_abandoned(monkeypatch): +async def test_a_paused_sandbox_is_resumed_rather_than_abandoned( + monkeypatch, conversation_transport +): """An idle sandbox is paused, not deleted -- the conversation survives it. Falling straight back to a run here would start a second conversation and @@ -178,7 +242,7 @@ async def test_a_paused_sandbox_is_resumed_rather_than_abandoned(monkeypatch): """ calls: list[str] = [] - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: calls.append(request.url.path) return httpx.Response(200, json={"success": True}) @@ -197,19 +261,23 @@ async def fake_resume_sandbox(client, api_url, api_key, sandbox_id): turn_module, "get_sandbox_agent_url", fake_get_sandbox_agent_url ) monkeypatch.setattr(turn_module, "resume_sandbox", fake_resume_sandbox) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) monkeypatch.setattr(turn_module, "RESUME_POLL_SECONDS", 0) - assert await send_conversation_turn(make_run("sbx-1"), "conv-1", "hi") is True + assert ( + await send_conversation_turn(make_run("sbx-1"), CONVERSATION_ID, "hi") is True + ) assert "resumed" in calls - assert "/api/conversations/conv-1/events" in calls + assert f"/api/conversations/{CONVERSATION_ID}/events" in calls @pytest.mark.asyncio -async def test_a_resume_that_never_comes_back_gives_up(monkeypatch): +async def test_a_resume_that_never_comes_back_gives_up( + monkeypatch, conversation_transport +): """Past the budget the caller starts a run, as it did before.""" - async def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover + def handler(request: httpx.Request) -> httpx.Response: # pragma: no cover raise AssertionError("should not have been called") async def fake_get_sandbox_agent_url(client, api_url, api_key, sandbox_id): @@ -223,33 +291,37 @@ async def fake_resume_sandbox(client, api_url, api_key, sandbox_id): turn_module, "get_sandbox_agent_url", fake_get_sandbox_agent_url ) monkeypatch.setattr(turn_module, "resume_sandbox", fake_resume_sandbox) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) monkeypatch.setattr(turn_module, "RESUME_POLL_SECONDS", 0) monkeypatch.setattr(turn_module, "RESUME_WAIT_SECONDS", 0) - assert await send_conversation_turn(make_run("sbx-1"), "conv-1", "hi") is False + assert ( + await send_conversation_turn(make_run("sbx-1"), CONVERSATION_ID, "hi") is False + ) @pytest.mark.asyncio -async def test_a_missing_conversation_is_a_false(monkeypatch): +async def test_a_missing_conversation_is_a_false(monkeypatch, conversation_transport): """A finished run's conversation is gone for good; do not wait on it.""" calls = 0 - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: nonlocal calls calls += 1 return httpx.Response(404, json={"detail": "Item not found"}) monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) run = make_run(finished=True) - assert await send_conversation_turn(run, "conv-gone", "hi") is False + assert await send_conversation_turn(run, CONVERSATION_ID, "hi") is False assert calls == 1 @pytest.mark.asyncio -async def test_a_conversation_still_opening_is_waited_for(monkeypatch): +async def test_a_conversation_still_opening_is_waited_for( + monkeypatch, conversation_transport +): """The sandbox answers before the script has opened the conversation. Giving up on that 404 is what forked a second run for an event arriving @@ -257,7 +329,7 @@ async def test_a_conversation_still_opening_is_waited_for(monkeypatch): """ calls = 0 - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: nonlocal calls calls += 1 if calls < 3: @@ -265,37 +337,39 @@ async def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(200, json={"ok": True}) monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) monkeypatch.setattr(turn_module, "CONVERSATION_POLL_SECONDS", 0) - assert await send_conversation_turn(make_run(), "conv-1", "hi") is True + assert await send_conversation_turn(make_run(), CONVERSATION_ID, "hi") is True assert calls == 3 @pytest.mark.asyncio -async def test_waiting_for_a_conversation_is_bounded(monkeypatch): +async def test_waiting_for_a_conversation_is_bounded( + monkeypatch, conversation_transport +): """A conversation that never opens still degrades to a run.""" - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(404, json={"detail": "Item not found"}) monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) monkeypatch.setattr(turn_module, "CONVERSATION_POLL_SECONDS", 0) monkeypatch.setattr(turn_module, "CONVERSATION_WAIT_SECONDS", 0) - assert await send_conversation_turn(make_run(), "conv-1", "hi") is False + assert await send_conversation_turn(make_run(), CONVERSATION_ID, "hi") is False @pytest.mark.asyncio -async def test_a_transport_failure_is_a_false(monkeypatch): - async def handler(request: httpx.Request) -> httpx.Response: +async def test_a_transport_failure_is_a_false(monkeypatch, conversation_transport): + def handler(request: httpx.Request) -> httpx.Response: raise httpx.ConnectError("no route to host") monkeypatch.setattr(turn_module, "get_backend", lambda run: local_backend()) - monkeypatch.setattr(turn_module, "httpx", fake_httpx(handler)) + conversation_transport(handler) - assert await send_conversation_turn(make_run(), "conv-1", "hi") is False + assert await send_conversation_turn(make_run(), CONVERSATION_ID, "hi") is False def test_a_specific_nested_path_beats_a_generic_top_level_one(): diff --git a/tests/test_conversations.py b/tests/test_conversations.py index 56f12c56..19d210f7 100644 --- a/tests/test_conversations.py +++ b/tests/test_conversations.py @@ -631,7 +631,7 @@ async def test_local_mode_continues_without_a_sandbox_id( ): """Local mode never sets `run.sandbox_id`, and must still continue. - `LocalAgentServerBackend.get_execution_context` returns `sandbox_id=None` + `ExistingAgentServerProvider.get_execution_context` returns `sandbox_id=None` and the dispatcher only records one when it is truthy, so gating on it would leave the whole feature dead on a local deployment. """ @@ -1008,7 +1008,11 @@ async def test_completing_an_ordinary_run_still_cleans_up( @pytest.mark.asyncio async def test_continue_conversation_loads_the_run_s_automation( - org_id, async_session_factory, mock_authenticated_user, monkeypatch + org_id, + async_session_factory, + mock_authenticated_user, + monkeypatch, + sdk_http_transport, ): """Minting a cloud API key reads `run.automation`. @@ -1053,19 +1057,33 @@ async def fake_sandbox_url(client, api_url, api_key, sandbox_id): fake_sandbox_url, ) + from unittest.mock import MagicMock + + from openhands.sdk import LLM, Agent + + monkeypatch.setattr( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient", + MagicMock(), + ) + agent = Agent(llm=LLM(model="test-model", api_key="test-key")) posted: list[str] = [] - async def handler(request: httpx.Request) -> httpx.Response: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/events/search"): + return httpx.Response(200, json={"items": []}) + if request.method == "GET": + return httpx.Response( + 200, + json={ + "id": request.url.path.rsplit("/", 1)[-1], + "agent": agent.model_dump(mode="json"), + "max_iterations": 160, + }, + ) posted.append(request.url.path) return httpx.Response(200, json={"success": True}) - def client_factory(**kwargs): - return httpx.AsyncClient(transport=httpx.MockTransport(handler)) - - monkeypatch.setattr( - "openhands.automation.utils.conversation_turn.httpx", - SimpleNamespace(AsyncClient=client_factory), - ) + sdk_http_transport(handler) subject_key = f"{TEAM}/C123/1.1" async with async_session_factory() as setup: @@ -1103,7 +1121,10 @@ def client_factory(**kwargs): assert result.conversation_id == derived assert result.coalesced is False assert key_urls, "the API key was never minted, so run.automation failed" - assert posted == [f"/api/conversations/{derived}/events"] + assert posted == [ + f"/api/conversations/{derived}/events", + f"/api/conversations/{derived}/run", + ] # --------------------------------------------------------------------------- diff --git a/tests/test_db.py b/tests/test_db.py index 8df9f9cc..488a2961 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -259,6 +259,10 @@ def test_migrations_run_on_sqlite(self, monkeypatch): column["name"] for column in inspector.get_columns("automation_runs") } assert "cost" in run_columns + assert "execution_scope" in run_columns + assert "execution_scope" in { + column["name"] for column in inspector.get_columns("automations") + } engine.dispose() finally: diff --git a/tests/test_disable_automation.py b/tests/test_disable_automation.py index 86798868..ea031d9a 100644 --- a/tests/test_disable_automation.py +++ b/tests/test_disable_automation.py @@ -32,7 +32,7 @@ def _create_mock_backend() -> MagicMock: mock_backend = MagicMock() mock_backend.get_api_key = AsyncMock(return_value="test-api-key") mock_backend.build_env_vars = MagicMock(return_value={}) - mock_backend.is_local_mode = False + mock_backend.provisions_agent_server = True # Mock execution context methods mock_ctx = ExecutionContext( agent_url="http://localhost:3000", @@ -467,7 +467,7 @@ async def test_does_not_disable_on_transient_error( def _create_mock_backend_with_api_key_check() -> MagicMock: """Create a mock backend that enforces API key initialization order. - This simulates CloudSandboxBackend behavior where build_env_vars() + This simulates CloudSandboxAgentServerProvider behavior where build_env_vars() requires get_execution_context() to be called first. """ from openhands.automation.backends.base import ExecutionContext @@ -539,7 +539,7 @@ async def test_build_env_vars_called_after_get_execution_context( This test catches the bug where build_env_vars() was called before get_execution_context(), causing 'API key not initialized' errors - in CloudSandboxBackend. + in CloudSandboxAgentServerProvider. """ from openhands.automation.dispatcher import _execute_run from openhands.automation.execution import DispatchResult diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index ec84ad84..da9e89e2 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -14,11 +14,13 @@ from sqlalchemy import select from sqlalchemy.orm import selectinload +from openhands.automation.backends.base import ExecutionContext from openhands.automation.config import get_config from openhands.automation.conversations import COALESCED_TURNS_KEY from openhands.automation.dispatcher import ( _build_event_payload, _execute_run, + _poll_pending_runs, dispatch_pending_runs, dispatcher_loop, ) @@ -73,6 +75,65 @@ def test_gs_url_is_not_http(self): assert is_http_url("gs://bucket/key.tar.gz") is False +async def test_run_dispatch_is_not_blocked_by_full_conversation_capacity( + async_session_factory, monkeypatch +): + """Bounded agent conversations must not starve host-side scanners.""" + from openhands.automation.config import clear_config_cache + + monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") + monkeypatch.setenv("AUTOMATION_CONVERSATION_MAX_CONCURRENT_RUNS", "1") + clear_config_cache() + try: + async with async_session_factory() as session: + agent = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Agent", + execution_scope="conversation", + trigger={"type": "cron", "schedule": "* * * * *"}, + tarball_path="s3://bucket/agent.tar.gz", + entrypoint="python agent.py", + ) + script = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Scanner", + execution_scope="run", + trigger={"type": "cron", "schedule": "* * * * *"}, + tarball_path="s3://bucket/scanner.tar.gz", + entrypoint="python scanner.py", + ) + session.add_all([agent, script]) + await session.flush() + session.add_all( + [ + AutomationRun( + automation_id=agent.id, + execution_scope="conversation", + status=AutomationRunStatus.RUNNING, + ), + AutomationRun( + automation_id=agent.id, + execution_scope="conversation", + status=AutomationRunStatus.PENDING, + ), + AutomationRun( + automation_id=script.id, + execution_scope="run", + status=AutomationRunStatus.PENDING, + ), + ] + ) + await session.commit() + + pending = await _poll_pending_runs(session, batch_size=10) + + assert [run.execution_scope for run in pending] == ["run"] + finally: + clear_config_cache() + + class TestMarkRunStatus: """Tests for mark_run_status function.""" @@ -871,7 +932,7 @@ async def test_successful_dispatch_resets_timeout_to_bash_start( ) backend = MagicMock() - ctx = MagicMock( + ctx = ExecutionContext( agent_url="http://agent.test", sandbox_id="sbx-1", session_key="sk-1" ) backend.get_execution_context = AsyncMock(return_value=ctx) @@ -897,7 +958,14 @@ class TestExecuteRunPhaseReporting: """Phase-reporting wiring in _execute_run.""" async def _run_successful_execution( - self, mock_execute, async_session_factory, mock_settings, mock_client + self, + mock_execute, + async_session_factory, + mock_settings, + mock_client, + *, + runtime_conversation_id=None, + conversation_id=None, ): """Drive _execute_run through a successful dispatch; returns run_id.""" async with async_session_factory() as session: @@ -917,6 +985,7 @@ async def _run_successful_execution( automation_id=automation.id, status=AutomationRunStatus.RUNNING, started_at=utcnow(), + conversation_id=conversation_id, ) session.add(run) await session.commit() @@ -936,11 +1005,20 @@ async def _run_successful_execution( ) backend = MagicMock() - ctx = MagicMock( - agent_url="http://agent.test", sandbox_id="sbx-1", session_key="sk-1" + ctx = ExecutionContext( + agent_url="http://agent.test", + sandbox_id="sbx-1", + session_key="sk-1", + runtime_conversation_id=( + uuid.UUID(runtime_conversation_id) if runtime_conversation_id else None + ), ) backend.get_execution_context = AsyncMock(return_value=ctx) - backend.build_env_vars = MagicMock(return_value={}) + backend.build_env_vars = MagicMock( + return_value={"AUTOMATION_CONVERSATION_ID": runtime_conversation_id} + if runtime_conversation_id + else {} + ) backend.get_work_dir = MagicMock(return_value="/workspace") mock_execute.return_value = MagicMock( success=True, bash_command_id="cmd-1", error=None @@ -951,6 +1029,33 @@ async def _run_successful_execution( return run_id + @pytest.mark.parametrize("scoped_runtime", [False, True]) + @pytest.mark.parametrize("conversation_id", [None, "callback-conversation"]) + @patch("openhands.automation.dispatcher.execute_in_context", new_callable=AsyncMock) + async def test_links_only_conversation_scoped_runtimes( + self, + mock_execute, + async_session_factory, + mock_settings, + mock_client, + scoped_runtime, + conversation_id, + ): + runtime_id = str(uuid.uuid4()) if scoped_runtime else None + run_id = await self._run_successful_execution( + mock_execute, + async_session_factory, + mock_settings, + mock_client, + runtime_conversation_id=runtime_id, + conversation_id=conversation_id, + ) + async with async_session_factory() as session: + updated = await session.get(AutomationRun, run_id) + assert updated.conversation_id == ( + runtime_id if scoped_runtime else conversation_id + ) + @patch("openhands.automation.dispatcher.execute_in_context", new_callable=AsyncMock) async def test_exposes_phase_url_to_sandbox( self, mock_execute, async_session_factory, mock_settings, mock_client @@ -963,6 +1068,19 @@ async def test_exposes_phase_url_to_sandbox( env_vars = mock_execute.await_args.kwargs["env_vars"] assert env_vars["AUTOMATION_PHASE_URL"].endswith(f"/v1/runs/{run_id}/phase") + @patch("openhands.automation.dispatcher.execute_in_context", new_callable=AsyncMock) + async def test_restricted_backend_uses_runtime_polling_without_service_credentials( + self, mock_execute, async_session_factory, mock_settings, mock_client + ): + mock_settings.local_api_key = "service-admin-key" + await self._run_successful_execution( + mock_execute, async_session_factory, mock_settings, mock_client + ) + env_vars = mock_execute.await_args.kwargs["env_vars"] + assert "AUTOMATION_CALLBACK_URL" not in env_vars + assert "AUTOMATION_PHASE_URL" not in env_vars + assert "service-admin-key" not in env_vars.values() + @patch("openhands.automation.dispatcher.execute_in_context", new_callable=AsyncMock) async def test_marks_starting_automation_phase_after_bash_dispatch( self, mock_execute, async_session_factory, mock_settings, mock_client @@ -1200,7 +1318,7 @@ async def test_concurrency_limit_marks_skipped_and_keeps_enabled( run, run_id, automation_id = await self._make_running_run(async_session_factory) backend = MagicMock() - backend.is_local_mode = False + backend.provisions_agent_server = True backend.get_execution_context = AsyncMock( side_effect=ConcurrencyLimitReachedError( "You have reached your limit of 3 concurrent conversations." @@ -1251,7 +1369,7 @@ async def test_generic_context_failure_still_marks_failed( run, run_id, _ = await self._make_running_run(async_session_factory) backend = MagicMock() - backend.is_local_mode = False + backend.provisions_agent_server = True backend.get_execution_context = AsyncMock(side_effect=RuntimeError("boom")) backend.release_context = AsyncMock() @@ -1331,11 +1449,27 @@ async def _dispatch( ) backend = MagicMock() - ctx = MagicMock( - agent_url="http://agent.test", sandbox_id="sbx-1", session_key="sk-1" + runtime_id = ( + uuid.UUID( + conversation_id_for( + org_id, automation_id, trigger["source"], subject_key + ) + ) + if subject_key + else None + ) + ctx = ExecutionContext( + agent_url="http://agent.test", + sandbox_id="sbx-1", + session_key="sk-1", + runtime_conversation_id=runtime_id, ) backend.get_execution_context = AsyncMock(return_value=ctx) - backend.build_env_vars = MagicMock(return_value={}) + backend.build_env_vars = MagicMock( + return_value={"AUTOMATION_CONVERSATION_ID": str(runtime_id)} + if runtime_id + else {} + ) backend.get_work_dir = MagicMock(return_value="/workspace") mock_execute.return_value = MagicMock( success=True, bash_command_id="cmd-1", error=None @@ -1344,6 +1478,10 @@ async def _dispatch( with patch("openhands.automation.dispatcher.get_backend", return_value=backend): await _execute_run(run, mock_settings, async_session_factory, mock_client) + if runtime_id: + async with async_session_factory() as session: + updated = await session.get(AutomationRun, run_id) + assert updated.conversation_id == str(runtime_id) return mock_execute.await_args.kwargs["env_vars"], org_id, automation_id @patch("openhands.automation.dispatcher.execute_in_context", new_callable=AsyncMock) diff --git a/tests/test_execution.py b/tests/test_execution.py index b65f21bf..d54c92a2 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -9,6 +9,7 @@ import tarfile from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest from openhands.automation.config import get_config @@ -160,59 +161,32 @@ class TestUploadUsesQueryParams: """ @pytest.mark.asyncio - async def test_upload_uses_query_param_for_path(self): - """_upload should use ?path= query param, not path in URL.""" - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - - mock_client = AsyncMock() - mock_client.post = AsyncMock(return_value=mock_response) - - await _upload( - client=mock_client, - agent_url="https://agent.example.com", - session_key="test-session-key", - data=b"test data", - dest="/tmp/automation.tar.gz", - ) - - # Verify post was called with query param, not path param - mock_client.post.assert_called_once() - call_args = mock_client.post.call_args - - url = call_args[0][0] - # URL should use query param format - assert "?path=" in url, f"Expected query param in URL, got: {url}" - assert "/tmp/automation.tar.gz" not in url.split("?")[0], ( - f"Path should not be in URL path segment: {url}" - ) - # Verify the path is properly encoded in query string - assert ( - "path=%2Ftmp%2Fautomation.tar.gz" in url - or "path=/tmp/automation.tar.gz" in url - ) - - @pytest.mark.asyncio - async def test_upload_preserves_absolute_path(self): - """_upload should preserve leading slash in path via query param.""" - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() + @pytest.mark.parametrize( + "destination", ["/tmp/automation.tar.gz", "/workspace/file.txt"] + ) + async def test_upload_preserves_path_in_query( + self, destination, sdk_http_transport + ): + """Exercise the SDK transport through the dispatcher's upload entrypoint.""" + requests = [] - mock_client = AsyncMock() - mock_client.post = AsyncMock(return_value=mock_response) + def respond(request): + requests.append(request) + return httpx.Response(200, json={}) + sdk_http_transport(respond) await _upload( - client=mock_client, agent_url="https://agent.example.com", session_key="test-session-key", data=b"test data", - dest="/workspace/file.txt", + dest=destination, ) - url = mock_client.post.call_args[0][0] - # The path in query param should preserve the leading slash - # (either URL-encoded as %2F or literal /) - assert "%2Fworkspace" in url or "/workspace" in url.split("?")[1] + assert len(requests) == 1 + assert requests[0].method == "POST" + assert requests[0].url.path == "/api/file/upload" + assert requests[0].url.params["path"] == destination + assert requests[0].headers["X-Session-API-Key"] == "test-session-key" class TestExecuteInContextErrors: @@ -226,10 +200,8 @@ async def test_reraises_permanent_error(self, mock_download_in_sandbox): "External tarball URL is not accessible" ) - mock_client = AsyncMock() with pytest.raises(TarballNotFoundError) as exc_info: await execute_in_context( - client=mock_client, agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", @@ -247,9 +219,7 @@ async def test_transient_error_returns_dispatch_result( """Non-permanent errors return DispatchResult with success=False.""" mock_download_in_sandbox.side_effect = RuntimeError("Connection timeout") - mock_client = AsyncMock() result = await execute_in_context( - client=mock_client, agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", @@ -270,7 +240,6 @@ async def test_custom_timeout_is_passed_to_bash(self, mock_upload, mock_start_ba mock_start_bash.return_value = "cmd-1" result = await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", @@ -289,10 +258,8 @@ async def test_permanent_error_with_bytes_tarball_reraises(self, mock_upload): """PermanentDispatchError during upload is also re-raised.""" mock_upload.side_effect = PermanentDispatchError("Upload permanently failed") - mock_client = AsyncMock() with pytest.raises(PermanentDispatchError) as exc_info: await execute_in_context( - client=mock_client, agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", @@ -310,9 +277,7 @@ async def test_success_returns_dispatch_result(self, mock_start_bash, mock_uploa mock_upload.return_value = None mock_start_bash.return_value = "cmd-123" - mock_client = AsyncMock() result = await execute_in_context( - client=mock_client, agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", @@ -400,7 +365,6 @@ async def test_execute_command_omits_env_values(self, mock_upload, mock_start_ba mock_start_bash.return_value = "cmd-123" result = await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="session-key", entrypoint="python main.py", @@ -413,10 +377,10 @@ async def test_execute_command_omits_env_values(self, mock_upload, mock_start_ba assert result.success is True assert mock_upload.await_count == 2 env_upload = mock_upload.await_args_list[1] - assert env_upload.args[4] == f"/tmp/automation-{run_id}.tar.gz.env" - assert secret.encode() in env_upload.args[3] + assert env_upload.args[3] == f"/tmp/automation-{run_id}.tar.gz.env" + assert secret.encode() in env_upload.args[2] - command = mock_start_bash.await_args.args[3] + command = mock_start_bash.await_args.args[2] assert secret not in command assert "it's $private" not in command assert "set +x" in command @@ -436,7 +400,6 @@ async def test_successful_start_leaves_env_cleanup_to_the_command( mock_start_bash.return_value = "cmd-123" result = await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="session-key", entrypoint="python main.py", @@ -459,14 +422,12 @@ async def test_start_failure_removes_uploaded_env( mock_start_bash, mock_bash, ): - client = AsyncMock() run_id = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" env_path = f"/tmp/automation-{run_id}.tar.gz.env" mock_start_bash.side_effect = RuntimeError("command startup failed") mock_bash.return_value = (0, "", "") result = await execute_in_context( - client=client, agent_url="https://agent.example.com", session_key="session-key", entrypoint="python main.py", @@ -480,10 +441,10 @@ async def test_start_failure_removes_uploaded_env( assert result.error == "command startup failed" assert mock_upload.await_count == 2 mock_bash.assert_awaited_once_with( - client, "https://agent.example.com", "session-key", f"rm -f -- '{env_path}'", + runtime_conversation_id=None, timeout=int(get_config().http.http_timeout), ) @@ -503,7 +464,6 @@ async def test_cleanup_failure_preserves_permanent_start_error( with pytest.raises(PermanentDispatchError) as exc_info: await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="session-key", entrypoint="python main.py", @@ -550,8 +510,8 @@ async def test_blocking_command_omits_session_key( assert result.success is True assert mock_upload.await_count == 2 - assert session_key.encode() in mock_upload.await_args_list[1].args[3] - command = mock_bash.await_args.args[3] + assert session_key.encode() in mock_upload.await_args_list[1].args[2] + command = mock_bash.await_args.args[2] assert session_key not in command assert f"{TARBALL_PATH}.env" in command @@ -579,7 +539,6 @@ async def test_bytes_upload_uses_per_run_path(self, mock_start_bash, mock_upload run_id = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -588,7 +547,7 @@ async def test_bytes_upload_uses_per_run_path(self, mock_start_bash, mock_upload run_id=run_id, ) - uploaded_dest = mock_upload.call_args.args[4] # (client, url, key, data, dest) + uploaded_dest = mock_upload.call_args.args[3] # (url, key, data, dest) assert uploaded_dest == f"/tmp/automation-{run_id}.tar.gz" assert uploaded_dest != TARBALL_PATH @@ -604,7 +563,6 @@ async def test_url_download_uses_per_run_path( run_id = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -613,7 +571,7 @@ async def test_url_download_uses_per_run_path( run_id=run_id, ) - download_dest = mock_download_in_sandbox.call_args.args[4] + download_dest = mock_download_in_sandbox.call_args.args[3] assert download_dest == f"/tmp/automation-{run_id}.tar.gz" assert download_dest != TARBALL_PATH @@ -630,7 +588,6 @@ async def test_bash_cmd_uses_per_run_path_and_cleans_up( expected_path = f"/tmp/automation-{run_id}.tar.gz" await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -639,7 +596,7 @@ async def test_bash_cmd_uses_per_run_path_and_cleans_up( run_id=run_id, ) - bash_cmd = mock_start_bash.call_args.args[3] # (client, url, key, command) + bash_cmd = mock_start_bash.call_args.args[2] # (url, key, command) assert f"tar xzf {expected_path}" in bash_cmd assert f"rm -f {expected_path}" in bash_cmd assert TARBALL_PATH not in bash_cmd @@ -655,7 +612,6 @@ async def test_no_run_id_falls_back_to_shared_constant( mock_start_bash.return_value = "cmd-abc" await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -664,9 +620,9 @@ async def test_no_run_id_falls_back_to_shared_constant( run_id=None, ) - uploaded_dest = mock_upload.call_args.args[4] + uploaded_dest = mock_upload.call_args.args[3] assert uploaded_dest == TARBALL_PATH - bash_cmd = mock_start_bash.call_args.args[3] + bash_cmd = mock_start_bash.call_args.args[2] assert f"tar xzf {TARBALL_PATH}" in bash_cmd @pytest.mark.asyncio @@ -691,7 +647,6 @@ async def test_concurrent_runs_use_distinct_paths( await asyncio.gather( execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -700,7 +655,6 @@ async def test_concurrent_runs_use_distinct_paths( run_id=run_id_a, ), execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -710,7 +664,7 @@ async def test_concurrent_runs_use_distinct_paths( ), ) - upload_dests = {c.args[4] for c in mock_upload.call_args_list} + upload_dests = {c.args[3] for c in mock_upload.call_args_list} assert f"/tmp/automation-{run_id_a}.tar.gz" in upload_dests assert f"/tmp/automation-{run_id_b}.tar.gz" in upload_dests assert len(upload_dests) == 2, "Each run must upload to its own unique path" @@ -726,7 +680,6 @@ async def test_run_id_with_slash_falls_back_to_shared_constant( mock_start_bash.return_value = "cmd-abc" await execute_in_context( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="key", entrypoint="python main.py", @@ -735,6 +688,63 @@ async def test_run_id_with_slash_falls_back_to_shared_constant( run_id="../../etc/passwd", ) - uploaded_dest = mock_upload.call_args.args[4] + uploaded_dest = mock_upload.call_args.args[3] assert uploaded_dest == TARBALL_PATH assert "etc/passwd" not in uploaded_dest + + +@pytest.mark.asyncio +async def test_sdk_upload_failure_never_starts_entrypoint(sdk_http_transport): + requests = [] + + def respond(request): + requests.append(request) + return httpx.Response(403, json={"detail": "Upload denied"}) + + sdk_http_transport(respond) + result = await execute_in_context( + agent_url="http://server", + session_key="key", + entrypoint="python main.py", + tarball_source=b"bundle", + work_dir="/workspace", + ) + assert result.success is False + assert result.error and "403" in result.error + assert all(request.url.path == "/api/file/upload" for request in requests) + + +@pytest.mark.asyncio +async def test_sdk_command_failure_never_starts_entrypoint(sdk_http_transport): + commands = [] + + def respond(request): + if request.method == "POST": + commands.append(request.content) + return httpx.Response(200, json={"id": "download-command"}) + return httpx.Response( + 200, + json={ + "items": [ + { + "kind": "BashOutput", + "exit_code": 7, + "stderr": "connection failed", + } + ] + }, + ) + + sdk_http_transport(respond) + result = await execute_in_context( + agent_url="http://server", + session_key="key", + entrypoint="python main.py", + tarball_source="https://bundles.example/automation.tar.gz", + work_dir="/workspace", + ) + assert result.success is False + assert result.error and "connection failed" in result.error + assert len(commands) == 1 + assert b"curl" in commands[0] + assert b"python main.py" not in commands[0] diff --git a/tests/test_local_mode.py b/tests/test_local_mode.py index 4fd7bc10..fd777abe 100644 --- a/tests/test_local_mode.py +++ b/tests/test_local_mode.py @@ -91,172 +91,73 @@ def test_verified_failure(self): class TestGetLastBashCommandResult: - """Tests for get_last_bash_command_result function.""" + """Exercise legacy verification through the public SDK's HTTP transport.""" @pytest.mark.asyncio - async def test_handles_http_error(self): - """Returns error result when HTTP request fails.""" - from unittest.mock import AsyncMock, MagicMock - + @pytest.mark.parametrize("status", [404, 429]) + async def test_handles_http_errors(self, status, sdk_http_transport): import httpx - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( - "Not found", request=MagicMock(), response=MagicMock(status_code=404) - ) - mock_client.get = AsyncMock(return_value=mock_response) - - result = await get_last_bash_command_result( - mock_client, "http://localhost:3000", "test-key" - ) - + sdk_http_transport(lambda _: httpx.Response(status)) + result = await get_last_bash_command_result("http://localhost:3000", "test-key") assert result.found is False - assert result.error is not None and "Not found" in result.error - - @pytest.mark.asyncio - async def test_handles_transient_rate_limit(self): - """Returns structured transient info for retryable agent-server errors.""" - from unittest.mock import AsyncMock, MagicMock - - import httpx - - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( - "Rate limited", request=MagicMock(), response=MagicMock(status_code=429) - ) - mock_client.get = AsyncMock(return_value=mock_response) - - result = await get_last_bash_command_result( - mock_client, "http://localhost:3000", "test-key" - ) - - assert result.found is False - assert result.error is not None and "HTTP 429" in result.error - assert result.error_info is not None - assert ( - result.error_info.fingerprint - == "agent_server:bash_events_search:rate_limited:429" - ) - - @pytest.mark.asyncio - async def test_handles_empty_response(self): - """Returns error result when no bash output found.""" - from unittest.mock import AsyncMock, MagicMock - - import httpx - - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - mock_response.json.return_value = {"items": []} - mock_client.get = AsyncMock(return_value=mock_response) - - result = await get_last_bash_command_result( - mock_client, "http://localhost:3000", "test-key" - ) - - assert result.found is False - assert result.error == "No bash output found" - - @pytest.mark.asyncio - async def test_handles_running_command(self): - """Returns running result when exit_code is None.""" - from unittest.mock import AsyncMock, MagicMock - - import httpx - - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - mock_response.json.return_value = { - "items": [{"exit_code": None, "stdout": "", "stderr": ""}] - } - mock_client.get = AsyncMock(return_value=mock_response) - - result = await get_last_bash_command_result( - mock_client, "http://localhost:3000", "test-key" - ) - - assert result.found is True - assert result.exit_code is None - assert result.error == "Command still running" + assert result.error and str(status) in result.error + if status == 429: + assert result.error_info is not None + assert ( + result.error_info.fingerprint + == "agent_server:bash_events_search:rate_limited:429" + ) + else: + assert result.error_info is None @pytest.mark.asyncio - async def test_handles_completed_command(self): - """Returns completed result with exit code and output.""" - from unittest.mock import AsyncMock, MagicMock - + @pytest.mark.parametrize( + "items,expected", + [ + ([], BashCommandResult(found=False, error="No bash output found")), + ( + [{"exit_code": None}], + BashCommandResult(found=True, error="Command still running"), + ), + ( + [{"exit_code": 0, "stdout": "Hello", "stderr": ""}], + BashCommandResult(found=True, exit_code=0, stdout="Hello"), + ), + ], + ) + async def test_command_result_states(self, items, expected, sdk_http_transport): import httpx - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - mock_response.json.return_value = { - "items": [{"exit_code": 0, "stdout": "Hello", "stderr": ""}] - } - mock_client.get = AsyncMock(return_value=mock_response) - - result = await get_last_bash_command_result( - mock_client, "http://localhost:3000", "test-key" - ) - - assert result.found is True - assert result.exit_code == 0 - assert result.stdout == "Hello" + sdk_http_transport(lambda _: httpx.Response(200, json={"items": items})) + result = await get_last_bash_command_result("http://localhost:3000", "test-key") + assert result == expected @pytest.mark.asyncio - async def test_adds_command_id_filter_when_provided(self): - """When command_id is provided, params include command_id__eq.""" - from unittest.mock import AsyncMock, MagicMock - + @pytest.mark.parametrize("command_id", [None, "abc123"]) + async def test_correlates_the_selected_command( + self, command_id, sdk_http_transport + ): import httpx - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - mock_response.json.return_value = {"items": []} - mock_client.get = AsyncMock(return_value=mock_response) - - await get_last_bash_command_result( - mock_client, - "http://localhost:3000", - "test-key", - command_id="abc123", - ) + requests = [] - # Verify the request was made with command_id__eq in params - mock_client.get.assert_called_once() - _, kwargs = mock_client.get.call_args - assert kwargs["params"]["command_id__eq"] == "abc123" - assert kwargs["params"]["kind__eq"] == "BashOutput" - assert kwargs["params"]["sort_order"] == "TIMESTAMP_DESC" - assert kwargs["params"]["limit"] == 1 - - @pytest.mark.asyncio - async def test_omits_command_id_filter_when_none(self): - """When command_id is None, params do NOT include command_id__eq.""" - from unittest.mock import AsyncMock, MagicMock - - import httpx - - mock_client = MagicMock(spec=httpx.AsyncClient) - mock_response = MagicMock() - mock_response.raise_for_status = MagicMock() - mock_response.json.return_value = {"items": []} - mock_client.get = AsyncMock(return_value=mock_response) + def respond(request): + requests.append(request) + return httpx.Response(200, json={"items": []}) + sdk_http_transport(respond) await get_last_bash_command_result( - mock_client, - "http://localhost:3000", - "test-key", + "http://localhost:3000", "test-key", command_id=command_id ) - - mock_client.get.assert_called_once() - _, kwargs = mock_client.get.call_args - assert "command_id__eq" not in kwargs["params"] - assert kwargs["params"]["kind__eq"] == "BashOutput" + assert len(requests) == 1 + request = requests[0] + assert request.url.path == "/api/bash/bash_events/search" + assert request.headers["X-Session-API-Key"] == "test-key" + assert request.url.params.get("command_id__eq") == command_id + assert request.url.params["kind__eq"] == "BashOutput" + assert request.url.params["sort_order"] == "TIMESTAMP_DESC" + assert request.url.params["limit"] == "1" class TestVerifyRunOnAgentServer: diff --git a/tests/test_watchdog.py b/tests/test_watchdog.py index 7955f833..1992c833 100644 --- a/tests/test_watchdog.py +++ b/tests/test_watchdog.py @@ -1,8 +1,4 @@ -"""Tests for the watchdog module. - -The watchdog processes stale runs (RUNNING but past timeout_at) and marks them -with appropriate status based on sandbox verification results. -""" +"""Tests for detached-command completion and stale-run recovery.""" import asyncio import uuid @@ -23,7 +19,7 @@ from openhands.automation.utils.agent_server import VerificationResult from openhands.automation.watchdog import ( PRUNE_BATCH_SIZE, - _should_cleanup_sandbox_after_terminal, + _should_cleanup_runtime_after_terminal, _verify_and_mark_run, cleanup_due_sandboxes, mark_stale_runs, @@ -674,6 +670,44 @@ def test_batch_size_is_bounded(self): class TestDeferredSandboxCleanup: """With a cleanup delay the watchdog pauses now and deletes later.""" + @pytest.mark.asyncio + async def test_conversation_runtime_without_sandbox_id_releases_immediately( + self, async_session_factory, automation_with_run, mock_settings + ): + run_id = automation_with_run["run_id"] + settings = mock_settings.model_copy( + update={"sandbox_cleanup_delay_seconds": 600} + ) + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + run.execution_scope = "conversation" + run.sandbox_id = None + await session.commit() + + mock_backend = _create_mock_backend( + VerificationResult( + verified=True, success=True, exit_code=0, stdout="ok", stderr="" + ) + ) + with ( + patch( + "openhands.automation.watchdog.get_backend", return_value=mock_backend + ), + patch( + "openhands.automation.watchdog.pause_sandbox", new_callable=AsyncMock + ) as mock_pause, + ): + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + assert await _verify_and_mark_run(session, run, settings) is True + await session.commit() + + mock_backend.cleanup_after_verification.assert_awaited_once_with(str(run_id)) + mock_pause.assert_not_awaited() + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + assert run.sandbox_cleanup_due_at is None + @pytest.mark.asyncio async def test_verified_exit_pauses_and_books_deletion_instead_of_deleting( self, async_session_factory, automation_with_run, mock_settings @@ -829,13 +863,13 @@ def test_the_helper_holds_a_kept_sandbox(self): run = MagicMock(spec=AutomationRun) run.sandbox_id = "sbx-1" run.subject_key = "T06P212QSEA/C123/1755000000.000100" - assert _should_cleanup_sandbox_after_terminal(run, keep_alive=True) is False + assert _should_cleanup_runtime_after_terminal(run, keep_alive=True) is False def test_an_ordinary_run_is_still_cleaned_up(self): run = MagicMock(spec=AutomationRun) run.sandbox_id = "sbx-1" run.subject_key = None - assert _should_cleanup_sandbox_after_terminal(run, keep_alive=False) is True + assert _should_cleanup_runtime_after_terminal(run, keep_alive=False) is True @pytest.mark.asyncio async def test_watchdog_does_not_delete_the_conversations_sandbox( @@ -874,6 +908,69 @@ async def test_watchdog_does_not_delete_the_conversations_sandbox( assert run.subject_released_at is None +@pytest.mark.asyncio +class TestDetachedCommandPolling: + async def test_run_scoped_command_completes_before_timeout( + self, async_session_factory, automation_with_run, mock_settings + ): + run_id = automation_with_run["run_id"] + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + run.execution_scope = "run" + run.bash_command_id = "command-123" + run.timeout_at = utcnow() + timedelta(minutes=30) + await session.commit() + + mock_backend = _create_mock_backend( + VerificationResult( + verified=True, + success=True, + exit_code=0, + stdout="done", + stderr="", + ) + ) + with patch( + "openhands.automation.watchdog.get_backend", return_value=mock_backend + ): + assert await mark_stale_runs(async_session_factory, mock_settings) == 1 + + mock_backend.verify_run.assert_awaited_once_with(str(run_id)) + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + assert run.status == AutomationRunStatus.COMPLETED + + async def test_pre_command_run_waits_for_timeout( + self, async_session_factory, automation_with_run, mock_settings + ): + run_id = automation_with_run["run_id"] + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + run.execution_scope = "run" + run.bash_command_id = None + run.timeout_at = utcnow() + timedelta(minutes=30) + await session.commit() + + mock_backend = _create_mock_backend( + VerificationResult( + verified=True, + success=True, + exit_code=0, + stdout="done", + stderr="", + ) + ) + with patch( + "openhands.automation.watchdog.get_backend", return_value=mock_backend + ): + assert await mark_stale_runs(async_session_factory, mock_settings) == 0 + + mock_backend.verify_run.assert_not_awaited() + async with async_session_factory() as session: + run = await session.get(AutomationRun, run_id) + assert run.status == AutomationRunStatus.RUNNING + + @pytest.mark.asyncio class TestMarkStaleRunsAutoDisable: """The watchdog must re-check auto-disable after it authors a failure. diff --git a/uv.lock b/uv.lock index e8a0c2d2..f3f87f14 100644 --- a/uv.lock +++ b/uv.lock @@ -2269,7 +2269,7 @@ requires-dist = [ { name = "google-cloud-storage", specifier = ">=2.18" }, { name = "httpx", specifier = ">=0.27" }, { name = "jmespath", specifier = ">=1.0" }, - { name = "openhands-sdk", specifier = "==1.46.0" }, + { name = "openhands-sdk", git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=e9392e7cf35fcb011acf2eca825a80f9a22774ea" }, { name = "openhands-workspace", specifier = "==1.46.0" }, { name = "pg8000", specifier = ">=1.31" }, { name = "prometheus-client", specifier = ">=0.19" }, @@ -2301,8 +2301,8 @@ dev = [ [[package]] name = "openhands-sdk" -version = "1.46.0" -source = { registry = "https://pypi.org/simple" } +version = "1.47.0" +source = { git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=e9392e7cf35fcb011acf2eca825a80f9a22774ea#e9392e7cf35fcb011acf2eca825a80f9a22774ea" } dependencies = [ { name = "agent-client-protocol" }, { name = "deprecation" }, @@ -2323,10 +2323,6 @@ dependencies = [ { name = "tree-sitter-bash" }, { name = "websockets" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/7c/09/ca00f55b3bca9198b257cc733067d13c01f07cfc400f9e03785545ffa2a5/openhands_sdk-1.46.0.tar.gz", hash = "sha256:4240c0d17d681d694b6b0d2c92462b023ff20c16d5233c6229ec57a5d4f34cbb", size = 714276, upload-time = "2026-09-09T00:56:41.775Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/63/63/199953bcb187562763052ef71db500ae22c84d355eb950ea260fd698830e/openhands_sdk-1.46.0-py3-none-any.whl", hash = "sha256:ef6fd74ac80208d67aa138a1da321479cf3b6a80aed2d68c53463c0551fee84d", size = 849925, upload-time = "2026-09-09T00:56:37.127Z" }, -] [[package]] name = "openhands-workspace" From 0b141386606ae14d74cbade5ffe2118e1013587b Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 14 Sep 2026 16:36:52 +0000 Subject: [PATCH 2/7] feat: select agent profiles for automation runs Co-authored-by: openhands --- README.md | 27 ++- migrations/versions/026_add_agent_profile.py | 24 +++ openhands/automation/backends/__init__.py | 8 +- openhands/automation/backends/conversation.py | 4 + openhands/automation/capabilities_router.py | 18 +- openhands/automation/config.py | 3 +- openhands/automation/git_sync/loop.py | 12 ++ openhands/automation/git_sync/serializer.py | 3 + openhands/automation/models.py | 6 + openhands/automation/preset_router.py | 66 ++++++- .../automation/presets/plugin/sdk_main.py | 176 ++++++++++-------- .../automation/presets/prompt/sdk_main.py | 174 +++++++++-------- openhands/automation/router.py | 35 +++- openhands/automation/schemas.py | 16 ++ openhands/automation/utils/model_profiles.py | 16 ++ openhands/automation/utils/run.py | 1 + tests/test_backends.py | 1 + tests/test_cancel_run.py | 25 +-- tests/test_conversation_backend.py | 48 ++++- tests/test_db.py | 4 + tests/test_git_sync.py | 39 ++++ tests/test_git_sync_serializer.py | 8 + tests/test_router.py | 81 ++++++++ 23 files changed, 589 insertions(+), 206 deletions(-) create mode 100644 migrations/versions/026_add_agent_profile.py diff --git a/README.md b/README.md index f4fc609c..1184f864 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,9 @@ The Automation Service owns automation definitions, cron scheduling, webhooks, r ### Conversation execution in local or Docker workspaces -Set `AUTOMATION_AGENT_SERVER_URL`, `AUTOMATION_AGENT_SERVER_API_KEY`, and -`AUTOMATION_AGENT_PROFILE` (a saved agent profile UUID). The backend reads the +Set `AUTOMATION_AGENT_SERVER_URL` and `AUTOMATION_AGENT_SERVER_API_KEY`, then +select a saved `agent_profile_id` on each conversation-scoped automation. The +selected profile is snapshotted when the run is queued. The backend reads the server's authoritative `conversation_runtime`. Definitions with `execution_scope: "conversation"` provision a conversation using the same API and profile in either workspace mode; definitions default to @@ -49,6 +50,17 @@ support runtime credential provisioning and release; bound container CPU, memory and PIDs in the server configuration. Completed Docker runtimes are released while history remains; the persistent local server and its history are retained. +The Agent Server resolves each selected profile at dispatch, including its +model, tools, and `secret_refs`. Missing profiles or secrets fail creation +instead of falling back to a more privileged agent. A separate `model` +selection cannot be combined with an agent profile. Run-scoped automations may +omit `agent_profile_id`. + +Profile selection is advertised by the `agentProfiles` capability when an +Agent Server is configured. Cloud dispatch without a configured Agent Server +retains its existing behavior and rejects explicit profile selections. Local +workspaces share the host security boundary; use Docker for process isolation. + ### Prerequisites - Python 3.12+ @@ -132,6 +144,17 @@ containers/ # Docker configuration This service is deployed via the [deploy repository](https://github.com/All-Hands-AI/deploy). Docker images are automatically built and pushed to `ghcr.io/openhands/automation` on every push to main and on tags. +Each automation chooses its saved Agent Server profile through the create or +patch API. Profile IDs are included in git sync and run history: + +```json +{"agent_profile_id": "11111111-1111-4111-8111-111111111111"} +``` + +The automation definition contains no token values or host-side permission +map. Manage credential availability through the profile's `secret_refs` and +the Agent Server's secret store. + ### SDK Integration Dependency The conversation backend and Agent Server execution helpers reuse `RemoteConversation` and `RemoteWorkspace` diff --git a/migrations/versions/026_add_agent_profile.py b/migrations/versions/026_add_agent_profile.py new file mode 100644 index 00000000..a9b33617 --- /dev/null +++ b/migrations/versions/026_add_agent_profile.py @@ -0,0 +1,24 @@ +"""Persist automation profile selection and snapshot it on queued runs. + +Revision ID: 026 +Revises: 025 +""" + +import sqlalchemy as sa +from alembic import op + + +revision = "026" +down_revision = "025" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + for table in ("automations", "automation_runs"): + op.add_column(table, sa.Column("agent_profile_id", sa.Uuid(), nullable=True)) + + +def downgrade() -> None: + for table in ("automation_runs", "automations"): + op.drop_column(table, "agent_profile_id") diff --git a/openhands/automation/backends/__init__.py b/openhands/automation/backends/__init__.py index f0e77915..68f9d376 100644 --- a/openhands/automation/backends/__init__.py +++ b/openhands/automation/backends/__init__.py @@ -8,7 +8,6 @@ from __future__ import annotations from typing import TYPE_CHECKING -from uuid import UUID from openhands.automation.backends.base import ExecutionBackend, ExecutionContext from openhands.automation.backends.conversation import ConversationBackend @@ -42,14 +41,17 @@ def get_backend(run: AutomationRun) -> ExecutionBackend: from openhands.automation.config import get_config settings = get_config().service + profile_id = run.agent_profile_id + if profile_id and not settings.is_local_mode: + raise ValueError("Agent profiles require an existing Agent Server") if run.execution_scope == "conversation": if not settings.is_local_mode: raise ValueError("Conversation execution requires an existing Agent Server") - if not settings.agent_profile: + if profile_id is None: raise ValueError("Conversation execution requires an agent profile") return ConversationBackend( _existing_agent_server(run), - agent_profile_id=UUID(settings.agent_profile), + agent_profile_id=profile_id, ) provider = ( diff --git a/openhands/automation/backends/conversation.py b/openhands/automation/backends/conversation.py index 230dac98..f9702cd3 100644 --- a/openhands/automation/backends/conversation.py +++ b/openhands/automation/backends/conversation.py @@ -90,6 +90,9 @@ def _create_conversation(self) -> None: workspace=LocalWorkspace(working_dir=workspace.working_dir), conversation_id=self.conversation_id, agent_profile_id=self.agent_profile_id, + plugins=(self.provider.run.automation.preset_metadata or {}).get( + "plugins" + ), max_iterations=160, tags={"automationrun": str(self.provider.run.id)}, ), @@ -141,6 +144,7 @@ def build_env_vars(self) -> dict[str, str]: ) ), "AUTOMATION_CONVERSATION_ID": str(self.conversation_id), + "AUTOMATION_AGENT_PROFILE_ID": str(self.agent_profile_id), "WORKSPACE_BASE": self.get_work_dir(str(self.provider.run.id)), "SESSION_API_KEY": self.runtime_api_key, } diff --git a/openhands/automation/capabilities_router.py b/openhands/automation/capabilities_router.py index fbb9037b..74d6588b 100644 --- a/openhands/automation/capabilities_router.py +++ b/openhands/automation/capabilities_router.py @@ -46,7 +46,10 @@ ) from openhands.automation.trigger_matcher import matches_trigger from openhands.automation.utils.cron import min_interval_seconds -from openhands.automation.utils.model_profiles import validate_model_profile_for_user +from openhands.automation.utils.model_profiles import ( + validate_agent_profile_selection, + validate_model_profile_for_user, +) from openhands.automation.utils.webhook import get_webhook_config @@ -119,6 +122,8 @@ async def get_capabilities( event_sources = sorted({*builtin, *await _custom_sources(user.org_id, session)}) features = [*_STATIC_FEATURES] + if config.service.is_local_mode: + features.append("agentProfiles") if event_sources: features.append("webhookDelivery") if config.kv.enabled: @@ -182,6 +187,17 @@ async def validate_draft( ) ) + try: + validate_agent_profile_selection(draft.agent_profile_id, draft.model) + except HTTPException as e: + errors.append( + DraftValidationError( + field="agent_profile_id", + code="invalid_agent_profile", + message=str(e.detail), + ) + ) + trigger = draft.trigger if isinstance(trigger, CronTrigger): errors.extend(_cron_errors(trigger)) diff --git a/openhands/automation/config.py b/openhands/automation/config.py index d8ad6fce..a9a661ca 100644 --- a/openhands/automation/config.py +++ b/openhands/automation/config.py @@ -581,8 +581,7 @@ class ServiceSettings(BaseSettings): # - Authenticates using local_api_key instead of OpenHands SaaS API agent_server_url: str = "" agent_server_api_key: str = "" - # Deployment profile for agent-mode runs; the server advertises the runtime. - agent_profile: str = "" + # Shared conversation execution; the server advertises its workspace runtime. conversation_max_concurrent_runs: int = Field(default=2, ge=1) # Optional override for the AGENT_SERVER_URL env var exported into the diff --git a/openhands/automation/git_sync/loop.py b/openhands/automation/git_sync/loop.py index ab634140..28bcf420 100644 --- a/openhands/automation/git_sync/loop.py +++ b/openhands/automation/git_sync/loop.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import Final, NamedTuple +from fastapi import HTTPException from pydantic import TypeAdapter, ValidationError from sqlalchemy import or_, select, update from sqlalchemy.engine import CursorResult @@ -64,6 +65,7 @@ from openhands.automation.schemas import Trigger, validate_command_string from openhands.automation.storage import ObjectNotFoundError, get_file_store from openhands.automation.utils import utcnow +from openhands.automation.utils.model_profiles import validate_agent_profile_selection from openhands.automation.utils.periodic_loop import run_periodic_loop from openhands.automation.utils.tarball_validation import ( build_internal_url, @@ -504,6 +506,15 @@ async def _validate_and_resolve_fields( execution_scope = fields.get("execution_scope", "run") if execution_scope not in {"run", "conversation"}: raise ValueError("execution_scope must be 'run' or 'conversation'") + agent_profile_id = ( + uuid.UUID(fields["agent_profile_id"]) + if fields.get("agent_profile_id") + else None + ) + try: + validate_agent_profile_selection(agent_profile_id, fields.get("model")) + except HTTPException as exc: + raise ValueError(exc.detail) from exc tarball_path = await _resolve_tarball_path( session, fields, deserialized, slug, existing, pending_storage_deletes, owner ) @@ -512,6 +523,7 @@ async def _validate_and_resolve_fields( "name": name, "execution_scope": execution_scope, "model": fields.get("model"), + "agent_profile_id": agent_profile_id, "trigger": trigger.model_dump(), "entrypoint": entrypoint, "setup_script_path": setup_script_path, diff --git a/openhands/automation/git_sync/serializer.py b/openhands/automation/git_sync/serializer.py index e10ce7c7..d5b74a9c 100644 --- a/openhands/automation/git_sync/serializer.py +++ b/openhands/automation/git_sync/serializer.py @@ -135,6 +135,9 @@ def _automation_yaml_fields( "name": automation.name, "execution_scope": automation.execution_scope, "model": automation.model, + "agent_profile_id": str(automation.agent_profile_id) + if automation.agent_profile_id + else None, "trigger": automation.trigger, "setup_script_path": automation.setup_script_path, "entrypoint": automation.entrypoint, diff --git a/openhands/automation/models.py b/openhands/automation/models.py index 446e8ca1..66310b57 100644 --- a/openhands/automation/models.py +++ b/openhands/automation/models.py @@ -80,6 +80,9 @@ class Automation(Base): # None is only used for legacy/local fallback. model: Mapped[str | None] = mapped_column(String(64), nullable=True) + # Profile IDs belong to the configured Agent Server, not to this database. + agent_profile_id: Mapped[uuid.UUID | None] = mapped_column(Uuid, nullable=True) + execution_scope: Mapped[str] = mapped_column( String(20), nullable=False, default=AutomationExecutionScope.RUN.value ) @@ -179,6 +182,9 @@ class AutomationRun(Base): default=AutomationRunStatus.PENDING, ) + # Snapshot the selected profile when queuing so edits affect future runs only. + agent_profile_id: Mapped[uuid.UUID | None] = mapped_column(Uuid, nullable=True) + # Snapshot how the definition executes so later edits affect future runs only. execution_scope: Mapped[str] = mapped_column( String(20), nullable=False, default=AutomationExecutionScope.RUN.value diff --git a/openhands/automation/preset_router.py b/openhands/automation/preset_router.py index bb66d049..52834af2 100644 --- a/openhands/automation/preset_router.py +++ b/openhands/automation/preset_router.py @@ -51,7 +51,10 @@ get_request_telemetry_context, ) from openhands.automation.utils import utcnow -from openhands.automation.utils.model_profiles import resolve_model_profile_for_user +from openhands.automation.utils.model_profiles import ( + resolve_model_profile_for_user, + validate_agent_profile_selection, +) from openhands.automation.utils.tarball_validation import ( build_internal_url, build_upload_storage_path, @@ -145,6 +148,8 @@ class CreatePromptAutomationRequest(BaseModel): model_config = ConfigDict(extra="forbid") + agent_profile_id: uuid.UUID | None = None + name: str = Field(..., min_length=1, max_length=500) prompt: str = Field( ..., @@ -276,27 +281,38 @@ def _generate_tarball(prompt: str, repos: list[RepoSource] | None = None) -> byt _build_storage_path = build_upload_storage_path -def _replace_prompt_in_tarball(tarball_bytes: bytes, new_prompt: str) -> bytes | None: +def _replace_prompt_in_tarball( + tarball_bytes: bytes, + new_prompt: str, + runner_files: dict[str, str] | None = None, +) -> bytes | None: """Return a copy of a preset tarball with ``prompt.txt`` swapped for ``new_prompt``. - Every other member (``main.py``, ``setup.sh``, ``plugins_config.json``, - ``repos_config.json``, ...) is copied through unchanged, so plugin and repo - configuration are preserved and the working template is untouched. + Optional ``runner_files`` upgrade the generated runner when its profile + changes. All other members, including plugin and repository configuration, + are preserved. Returns ``None`` if the archive has no ``prompt.txt`` member — i.e. it is not a regenerable preset tarball — so the caller can leave the tarball as-is. """ out_buffer = io.BytesIO() found = False + replacements = {"prompt.txt": new_prompt, **(runner_files or {})} + seen = set() with ( tarfile.open(fileobj=io.BytesIO(tarball_bytes), mode="r:gz") as src, tarfile.open(fileobj=out_buffer, mode="w:gz") as dst, ): for member in src.getmembers(): + seen.add(member.name) if member.name == "prompt.txt": found = True + if member.name in replacements: _add_file_to_tar( - dst, "prompt.txt", new_prompt, mode=member.mode or 0o644 + dst, + member.name, + replacements[member.name], + mode=member.mode or 0o644, ) continue if member.isfile(): @@ -309,6 +325,8 @@ def _replace_prompt_in_tarball(tarball_bytes: bytes, new_prompt: str) -> bytes | dst.addfile(info, io.BytesIO(data)) else: dst.addfile(member) + for name in replacements.keys() - seen: + _add_file_to_tar(dst, name, replacements[name]) if not found: return None @@ -334,6 +352,8 @@ async def regenerate_preset_prompt_tarball( new_prompt: str, session: AsyncSession, background_tasks: BackgroundTasks, + *, + refresh_runner: bool = False, ) -> str | None: """Rebuild a preset automation's tarball with an updated prompt. @@ -343,7 +363,8 @@ async def regenerate_preset_prompt_tarball( running the original prompt. Reads the automation's current internal-upload tarball, swaps in ``new_prompt`` - (leaving all other files untouched), uploads the result as a new internal upload, + and optionally refreshes the preset runner when its agent profile changes. + Repository and plugin configuration are preserved. Uploads a new internal upload, and returns its ``oh-internal://`` URL for the caller to store on ``tarball_path``. The superseded upload is soft-deleted in the current transaction; its storage object is removed via ``background_tasks`` only after the transaction commits. @@ -373,7 +394,14 @@ async def regenerate_preset_prompt_tarball( # leaving the old prompt baked into the tarball. return None - new_tarball = _replace_prompt_in_tarball(current_tarball, new_prompt) + runner_files = None + if refresh_runner: + kind = (automation.preset_metadata or {}).get("preset_type") + if kind == "prompt": + runner_files = _load_prompt_preset_files() + elif kind == "plugin": + runner_files = _load_plugin_preset_files() + new_tarball = _replace_prompt_in_tarball(current_tarball, new_prompt, runner_files) if new_tarball is None: return None @@ -481,7 +509,12 @@ async def create_automation_from_prompt( response.status_code = status.HTTP_200_OK return AutomationResponse.model_validate(existing) - model = resolve_model_profile_for_user(body.model, user) + validate_agent_profile_selection(body.agent_profile_id, body.model) + model = ( + None + if body.agent_profile_id + else resolve_model_profile_for_user(body.model, user) + ) # 1. Generate tarball with SDK code, prompt, and optional repos config tarball_content = _generate_tarball(body.prompt, repos=body.repos) @@ -545,6 +578,7 @@ async def create_automation_from_prompt( prompt=body.prompt, preset_metadata=preset_metadata, model=model, + agent_profile_id=body.agent_profile_id, trigger=body.trigger.model_dump(), tarball_path=tarball_path, setup_script_path="setup.sh", @@ -628,6 +662,8 @@ class CreatePluginAutomationRequest(BaseModel): model_config = ConfigDict(extra="forbid") + agent_profile_id: uuid.UUID | None = None + name: str = Field(..., min_length=1, max_length=500) plugins: list[PluginSource] | None = Field( default=None, @@ -891,7 +927,16 @@ async def create_automation_from_plugin( response.status_code = status.HTTP_200_OK return AutomationResponse.model_validate(existing) - model = resolve_model_profile_for_user(body.model, user) + validate_agent_profile_selection(body.agent_profile_id, body.model) + model = ( + None + if body.agent_profile_id + else resolve_model_profile_for_user(body.model, user) + ) + if body.agent_profile_id and body.variants: + raise HTTPException( + 422, "Agent profiles cannot be combined with model experiment variants" + ) variants = _resolve_experiment_variant_models( body.variants, user, default_model=model ) @@ -978,6 +1023,7 @@ async def create_automation_from_plugin( prompt=body.prompt, preset_metadata=preset_metadata, model=model, + agent_profile_id=body.agent_profile_id, trigger=body.trigger.model_dump(), tarball_path=tarball_path, setup_script_path="setup.sh", diff --git a/openhands/automation/presets/plugin/sdk_main.py b/openhands/automation/presets/plugin/sdk_main.py index ac2aaa2c..f9a9f32c 100644 --- a/openhands/automation/presets/plugin/sdk_main.py +++ b/openhands/automation/presets/plugin/sdk_main.py @@ -74,6 +74,8 @@ import uuid from datetime import datetime, timezone + + # Detect execution mode based on AGENT_SERVER_URL presence agent_server_url = os.environ.get("AGENT_SERVER_URL", "").rstrip("/") IS_LOCAL_MODE = bool(agent_server_url) @@ -184,10 +186,12 @@ def _phase_poster() -> None: # SDK imports (before workspace context so import errors are caught) -from openhands.sdk import Conversation, RemoteConversation from finish_tool_hook import finish_tool_required_hook_config + +from openhands.sdk import Conversation, RemoteConversation from openhands.tools.preset import TaskOutcome + try: from openhands.sdk.mcp.config import coerce_mcp_config as _coerce_mcp_config except ImportError: @@ -217,7 +221,6 @@ def _normalize_mcp_config(raw_mcp_config): return raw_mcp_config - def _build_conversation_title(event_context) -> str | None: """Build a descriptive conversation title from the automation event context. @@ -301,6 +304,7 @@ def _build_conversation_title(event_context) -> str | None: # -- All remaining setup happens inside the workspace context -- # This ensures failures trigger the __exit__ callback report_phase("Setting up workspace") + has_provisioned_conversation = bool(os.environ.get("AUTOMATION_AGENT_PROFILE_ID")) # Parse event payload if present (for event-triggered automations) event_context = None @@ -317,12 +321,19 @@ def _build_conversation_title(event_context) -> str | None: REPOS_CONFIG_FILE = os.path.join(SCRIPT_DIR, "repos_config.json") clone_result = None repo_dirs = [] + profile_repos_context = "" if os.path.exists(REPOS_CONFIG_FILE): print("\n=== CLONE REPOS ===") with open(REPOS_CONFIG_FILE) as f: repos_config = json.load(f) - if repos_config: + if repos_config and has_provisioned_conversation: + profile_repos_context = ( + "Check out these repositories in your workspace using only the credentials " + "available to your agent profile, then follow their repository guidance:\n" + + json.dumps(repos_config) + ) + elif repos_config: report_phase("Cloning repositories") clone_result = workspace.clone_repos(repos_config) print(f" cloned {clone_result.success_count}/{len(repos_config)} repos") @@ -335,13 +346,15 @@ def _build_conversation_title(event_context) -> str | None: # If repos were cloned, project skills are loaded from EACH cloned repo print("\n=== LOAD SKILLS ===") report_phase("Loading skills") - loaded_skills, agent_context = workspace.load_skills_from_agent_server( - project_dirs=repo_dirs if repo_dirs else None - ) + loaded_skills, agent_context = [], None + if not has_provisioned_conversation: + loaded_skills, agent_context = workspace.load_skills_from_agent_server( + project_dirs=repo_dirs if repo_dirs else None + ) print(f" loaded {len(loaded_skills)} skills") # Get repos context (mapping of URLs to local paths) - repos_context = "" + repos_context = profile_repos_context if clone_result and clone_result.repo_mappings: repos_context = workspace.get_repos_context(clone_result.repo_mappings) @@ -400,9 +413,7 @@ def _build_conversation_title(event_context) -> str | None: # the service could not deliver them as turns. They open the conversation # with this one instead of each starting a run of its own. if event_context and event_context.get("follow_up_turns"): - follow_ups = "\n\n".join( - str(turn) for turn in event_context["follow_up_turns"] - ) + follow_ups = "\n\n".join(str(turn) for turn in event_context["follow_up_turns"]) context_sections.append(f"""## Follow-up messages More activity arrived on the same subject while this run was queued: @@ -428,70 +439,76 @@ def _build_conversation_title(event_context) -> str | None: path_str = f" ({ps.repo_path})" if ps.repo_path else "" print(f" - {ps.source}{ref_str}{path_str}") - # Get LLM config via workspace/profile APIs - print("\n=== GET_LLM ===") - try: - llm = workspace.get_llm(profile_name=model_profile) - except FileNotFoundError: - if not model_profile: - raise - print( - f" profile {model_profile!r} not found; " - "falling back to active/default profile" + if has_provisioned_conversation: + # The server already resolved the model, tools, skills, MCP, and secrets. + # Attaching must not reload defaults or forward the host's secret store. + agent = None + secrets = {} + else: + # Get LLM config via workspace/profile APIs + print("\n=== GET_LLM ===") + try: + llm = workspace.get_llm(profile_name=model_profile) + except FileNotFoundError: + if not model_profile: + raise + print( + f" profile {model_profile!r} not found; " + "falling back to active/default profile" + ) + llm = workspace.get_llm() + print(f" profile: {model_profile or 'DEFAULT'}") + print(f" model: {llm.model}") + print(f" api_key present: {bool(llm.api_key)}") + + # Get secrets via workspace + print("\n=== GET_SECRETS ===") + secrets = {} + try: + secrets = workspace.get_secrets() + print(f" available: {list(secrets.keys()) or '(none)'}") + except Exception as e: + # Not a hard failure — user may not have secrets configured + print(f" get_secrets() failed (ok if no secrets): {e}") + + # Get MCP config via workspace + print("\n=== GET_MCP_CONFIG ===") + mcp_config = {} + try: + mcp_config = _normalize_mcp_config(workspace.get_mcp_config()) + if mcp_config: + print(f" servers: {list(mcp_config.keys())}") + else: + print(" no MCP servers configured") + except Exception as e: + # Not a hard failure — user may not have MCP configured + print(f" get_mcp_config() failed (ok if no MCP): {e}") + + # Get default agent with tools and condenser (CLI mode to disable browser) + print("\n=== AGENT ===") + report_phase("Configuring agent") + # Keep finish-tool schema wiring in sync with presets/prompt/sdk_main.py. + agent = get_default_agent( + llm=llm, + cli_mode=True, + finish_tool_response_schema=TaskOutcome, ) - llm = workspace.get_llm() - print(f" profile: {model_profile or 'DEFAULT'}") - print(f" model: {llm.model}") - print(f" api_key present: {bool(llm.api_key)}") - - # Get secrets via workspace - print("\n=== GET_SECRETS ===") - secrets = {} - try: - secrets = workspace.get_secrets() - print(f" available: {list(secrets.keys()) or '(none)'}") - except Exception as e: - # Not a hard failure — user may not have secrets configured - print(f" get_secrets() failed (ok if no secrets): {e}") - - # Get MCP config via workspace - print("\n=== GET_MCP_CONFIG ===") - mcp_config = {} - try: - mcp_config = _normalize_mcp_config(workspace.get_mcp_config()) - if mcp_config: - print(f" servers: {list(mcp_config.keys())}") - else: - print(" no MCP servers configured") - except Exception as e: - # Not a hard failure — user may not have MCP configured - print(f" get_mcp_config() failed (ok if no MCP): {e}") - - # Get default agent with tools and condenser (CLI mode to disable browser) - print("\n=== AGENT ===") - report_phase("Configuring agent") - # Keep finish-tool schema wiring in sync with presets/prompt/sdk_main.py. - agent = get_default_agent( - llm=llm, - cli_mode=True, - finish_tool_response_schema=TaskOutcome, - ) - # Add MCP config and agent_context using model_copy if configured - # (Plugin MCP configs will be merged when plugins are loaded) - agent_updates = {} - if mcp_config: - agent_updates["mcp_config"] = mcp_config - if agent_context: - agent_updates["agent_context"] = agent_context - if agent_updates: - agent = agent.model_copy(update=agent_updates) - - print(f" tools: {[t.name for t in agent.tools]}") - print(f" mcp_config: {'configured' if mcp_config else 'none'}") - print(f" skills: {len(loaded_skills) if loaded_skills else 0}") - condenser_name = type(agent.condenser).__name__ if agent.condenser else "none" - print(f" condenser: {condenser_name}") + # Add MCP config and agent_context using model_copy if configured + # (Plugin MCP configs will be merged when plugins are loaded) + agent_updates = {} + if mcp_config: + agent_updates["mcp_config"] = mcp_config + if agent_context: + agent_updates["agent_context"] = agent_context + if agent_updates: + agent = agent.model_copy(update=agent_updates) + + print(f" tools: {[t.name for t in agent.tools]}") + print(f" mcp_config: {'configured' if mcp_config else 'none'}") + print(f" skills: {len(loaded_skills) if loaded_skills else 0}") + condenser_name = type(agent.condenser).__name__ if agent.condenser else "none" + print(f" condenser: {condenser_name}") # Create conversation with plugins print("\n=== CONVERSATION ===") @@ -554,7 +571,14 @@ def event_callback(event) -> None: automation_conversation_id = os.environ.get("AUTOMATION_CONVERSATION_ID") if automation_conversation_id: conversation_kwargs["conversation_id"] = uuid.UUID(automation_conversation_id) - conversation = Conversation(**conversation_kwargs) + if has_provisioned_conversation: + conversation = RemoteConversation.attach( + workspace=workspace, + conversation_id=uuid.UUID(os.environ["AUTOMATION_CONVERSATION_ID"]), + callbacks=[event_callback], + ) + else: + conversation = Conversation(**conversation_kwargs) assert isinstance(conversation, RemoteConversation) print(f" conversation created: {type(conversation).__name__}") print(f" plugins loaded: {len(plugin_sources)}") @@ -567,11 +591,7 @@ def event_callback(event) -> None: conversation_title = _build_conversation_title(event_context) if conversation_title: try: - resp = workspace.client.patch( - f"/api/conversations/{conversation.id}", - json={"title": conversation_title}, - ) - resp.raise_for_status() + conversation.set_title(conversation_title) print(f" title: {conversation_title}") except Exception as e: # Not a hard failure — autotitle fallback still applies diff --git a/openhands/automation/presets/prompt/sdk_main.py b/openhands/automation/presets/prompt/sdk_main.py index 20ac68da..71f9d920 100644 --- a/openhands/automation/presets/prompt/sdk_main.py +++ b/openhands/automation/presets/prompt/sdk_main.py @@ -77,6 +77,8 @@ import uuid from datetime import datetime, timezone + + # Detect execution mode based on AGENT_SERVER_URL presence agent_server_url = os.environ.get("AGENT_SERVER_URL", "").rstrip("/") IS_LOCAL_MODE = bool(agent_server_url) @@ -188,10 +190,12 @@ def _phase_poster() -> None: # SDK imports (before workspace context so import errors are caught) -from openhands.sdk import Conversation, RemoteConversation from finish_tool_hook import finish_tool_required_hook_config + +from openhands.sdk import Conversation, RemoteConversation from openhands.tools.preset import TaskOutcome + try: from openhands.sdk.mcp.config import coerce_mcp_config as _coerce_mcp_config except ImportError: @@ -220,7 +224,6 @@ def _normalize_mcp_config(raw_mcp_config): return raw_mcp_config - def _build_conversation_title(event_context) -> str | None: """Build a descriptive conversation title from the automation event context. @@ -308,6 +311,7 @@ def _build_conversation_title(event_context) -> str | None: # -- All remaining setup happens inside the workspace context -- # This ensures failures trigger the __exit__ callback report_phase("Setting up workspace") + has_provisioned_conversation = bool(os.environ.get("AUTOMATION_AGENT_PROFILE_ID")) # Parse event payload if present (for event-triggered automations) event_context = None @@ -324,12 +328,19 @@ def _build_conversation_title(event_context) -> str | None: REPOS_CONFIG_FILE = os.path.join(SCRIPT_DIR, "repos_config.json") clone_result = None repo_dirs = [] + profile_repos_context = "" if os.path.exists(REPOS_CONFIG_FILE): print("\n=== CLONE REPOS ===") with open(REPOS_CONFIG_FILE) as f: repos_config = json.load(f) - if repos_config: + if repos_config and has_provisioned_conversation: + profile_repos_context = ( + "Check out these repositories in your workspace using only the credentials " + "available to your agent profile, then follow their repository guidance:\n" + + json.dumps(repos_config) + ) + elif repos_config: report_phase("Cloning repositories") clone_result = workspace.clone_repos(repos_config) print(f" cloned {clone_result.success_count}/{len(repos_config)} repos") @@ -342,13 +353,15 @@ def _build_conversation_title(event_context) -> str | None: # If repos were cloned, project skills are loaded from EACH cloned repo print("\n=== LOAD SKILLS ===") report_phase("Loading skills") - loaded_skills, agent_context = workspace.load_skills_from_agent_server( - project_dirs=repo_dirs if repo_dirs else None - ) + loaded_skills, agent_context = [], None + if not has_provisioned_conversation: + loaded_skills, agent_context = workspace.load_skills_from_agent_server( + project_dirs=repo_dirs if repo_dirs else None + ) print(f" loaded {len(loaded_skills)} skills") # Get repos context (mapping of URLs to local paths) - repos_context = "" + repos_context = profile_repos_context if clone_result and clone_result.repo_mappings: repos_context = workspace.get_repos_context(clone_result.repo_mappings) @@ -379,9 +392,7 @@ def _build_conversation_title(event_context) -> str | None: # the service could not deliver them as turns. They open the conversation # with this one instead of each starting a run of its own. if event_context and event_context.get("follow_up_turns"): - follow_ups = "\n\n".join( - str(turn) for turn in event_context["follow_up_turns"] - ) + follow_ups = "\n\n".join(str(turn) for turn in event_context["follow_up_turns"]) context_sections.append(f"""## Follow-up messages More activity arrived on the same subject while this run was queued: @@ -397,69 +408,75 @@ def _build_conversation_title(event_context) -> str | None: {USER_PROMPT}""" - # Get LLM config via workspace/profile APIs - print("\n=== GET_LLM ===") - try: - llm = workspace.get_llm(profile_name=model_profile) - except FileNotFoundError: - if not model_profile: - raise - print( - f" profile {model_profile!r} not found; " - "falling back to active/default profile" + if has_provisioned_conversation: + # The server already resolved the model, tools, skills, MCP, and secrets. + # Attaching must not reload defaults or forward the host's secret store. + agent = None + secrets = {} + else: + # Get LLM config via workspace/profile APIs + print("\n=== GET_LLM ===") + try: + llm = workspace.get_llm(profile_name=model_profile) + except FileNotFoundError: + if not model_profile: + raise + print( + f" profile {model_profile!r} not found; " + "falling back to active/default profile" + ) + llm = workspace.get_llm() + print(f" profile: {model_profile or 'DEFAULT'}") + print(f" model: {llm.model}") + print(f" api_key present: {bool(llm.api_key)}") + + # Get secrets via workspace + print("\n=== GET_SECRETS ===") + secrets = {} + try: + secrets = workspace.get_secrets() + print(f" available: {list(secrets.keys()) or '(none)'}") + except Exception as e: + # Not a hard failure — user may not have secrets configured + print(f" get_secrets() failed (ok if no secrets): {e}") + + # Get MCP config via workspace + print("\n=== GET_MCP_CONFIG ===") + mcp_config = {} + try: + mcp_config = _normalize_mcp_config(workspace.get_mcp_config()) + if mcp_config: + print(f" servers: {list(mcp_config.keys())}") + else: + print(" no MCP servers configured") + except Exception as e: + # Not a hard failure — user may not have MCP configured + print(f" get_mcp_config() failed (ok if no MCP): {e}") + + # Get default agent with tools and condenser (CLI mode to disable browser) + print("\n=== AGENT ===") + report_phase("Configuring agent") + # Keep finish-tool schema wiring in sync with presets/plugin/sdk_main.py. + agent = get_default_agent( + llm=llm, + cli_mode=True, + finish_tool_response_schema=TaskOutcome, ) - llm = workspace.get_llm() - print(f" profile: {model_profile or 'DEFAULT'}") - print(f" model: {llm.model}") - print(f" api_key present: {bool(llm.api_key)}") - - # Get secrets via workspace - print("\n=== GET_SECRETS ===") - secrets = {} - try: - secrets = workspace.get_secrets() - print(f" available: {list(secrets.keys()) or '(none)'}") - except Exception as e: - # Not a hard failure — user may not have secrets configured - print(f" get_secrets() failed (ok if no secrets): {e}") - - # Get MCP config via workspace - print("\n=== GET_MCP_CONFIG ===") - mcp_config = {} - try: - mcp_config = _normalize_mcp_config(workspace.get_mcp_config()) - if mcp_config: - print(f" servers: {list(mcp_config.keys())}") - else: - print(" no MCP servers configured") - except Exception as e: - # Not a hard failure — user may not have MCP configured - print(f" get_mcp_config() failed (ok if no MCP): {e}") - - # Get default agent with tools and condenser (CLI mode to disable browser) - print("\n=== AGENT ===") - report_phase("Configuring agent") - # Keep finish-tool schema wiring in sync with presets/plugin/sdk_main.py. - agent = get_default_agent( - llm=llm, - cli_mode=True, - finish_tool_response_schema=TaskOutcome, - ) - # Add MCP config and agent_context using model_copy if configured - agent_updates = {} - if mcp_config: - agent_updates["mcp_config"] = mcp_config - if agent_context: - agent_updates["agent_context"] = agent_context - if agent_updates: - agent = agent.model_copy(update=agent_updates) - - print(f" tools: {[t.name for t in agent.tools]}") - print(f" mcp_config: {'configured' if mcp_config else 'none'}") - print(f" skills: {len(loaded_skills) if loaded_skills else 0}") - condenser_name = type(agent.condenser).__name__ if agent.condenser else "none" - print(f" condenser: {condenser_name}") + # Add MCP config and agent_context using model_copy if configured + agent_updates = {} + if mcp_config: + agent_updates["mcp_config"] = mcp_config + if agent_context: + agent_updates["agent_context"] = agent_context + if agent_updates: + agent = agent.model_copy(update=agent_updates) + + print(f" tools: {[t.name for t in agent.tools]}") + print(f" mcp_config: {'configured' if mcp_config else 'none'}") + print(f" skills: {len(loaded_skills) if loaded_skills else 0}") + condenser_name = type(agent.condenser).__name__ if agent.condenser else "none" + print(f" condenser: {condenser_name}") # Create conversation print("\n=== CONVERSATION ===") @@ -507,7 +524,14 @@ def event_callback(event) -> None: automation_conversation_id = os.environ.get("AUTOMATION_CONVERSATION_ID") if automation_conversation_id: conversation_kwargs["conversation_id"] = uuid.UUID(automation_conversation_id) - conversation = Conversation(**conversation_kwargs) + if has_provisioned_conversation: + conversation = RemoteConversation.attach( + workspace=workspace, + conversation_id=uuid.UUID(os.environ["AUTOMATION_CONVERSATION_ID"]), + callbacks=[event_callback], + ) + else: + conversation = Conversation(**conversation_kwargs) assert isinstance(conversation, RemoteConversation) print(f" conversation created: {type(conversation).__name__}") @@ -517,11 +541,7 @@ def event_callback(event) -> None: conversation_title = _build_conversation_title(event_context) if conversation_title: try: - resp = workspace.client.patch( - f"/api/conversations/{conversation.id}", - json={"title": conversation_title}, - ) - resp.raise_for_status() + conversation.set_title(conversation_title) print(f" title: {conversation_title}") except Exception as e: # Not a hard failure — autotitle fallback still applies diff --git a/openhands/automation/router.py b/openhands/automation/router.py index 0f9f6109..fb956dff 100644 --- a/openhands/automation/router.py +++ b/openhands/automation/router.py @@ -58,7 +58,10 @@ from openhands.automation.utils.conversation_outcome import ( fetch_latest_finish_tool_response_for_run, ) -from openhands.automation.utils.model_profiles import resolve_model_profile_for_user +from openhands.automation.utils.model_profiles import ( + resolve_model_profile_for_user, + validate_agent_profile_selection, +) from openhands.automation.utils.run import ( create_pending_run, record_first_run_outcome, @@ -155,7 +158,12 @@ async def create_automation( org_id=user.org_id, session=session, ) - model = resolve_model_profile_for_user(body.model, user) + validate_agent_profile_selection(body.agent_profile_id, body.model) + model = ( + None + if body.agent_profile_id + else resolve_model_profile_for_user(body.model, user) + ) preset_metadata: dict[str, Any] | None = None if body.template is not None: @@ -167,6 +175,7 @@ async def create_automation( execution_scope=body.execution_scope, name=body.name, model=model, + agent_profile_id=body.agent_profile_id, preset_metadata=preset_metadata, trigger=body.trigger.model_dump(), tarball_path=body.tarball_path, @@ -312,10 +321,19 @@ async def update_automation( source="manual", ) - if "model" in update_data: - update_data["model"] = resolve_model_profile_for_user(body.model, user) + if "agent_profile_id" in update_data or "model" in update_data: + selected_profile = update_data.get("agent_profile_id", auto.agent_profile_id) + validate_agent_profile_selection(selected_profile, body.model) + if selected_profile: + update_data["model"] = None + elif "model" in update_data: + update_data["model"] = resolve_model_profile_for_user(body.model, user) original_prompt = auto.prompt + profile_changed = ( + "agent_profile_id" in update_data + and auto.agent_profile_id != update_data["agent_profile_id"] + ) for field, value in update_data.items(): setattr(auto, field, value) @@ -324,13 +342,12 @@ async def update_automation( # changes, rebuild the tarball so the next dispatch runs the new prompt # instead of the original baked one. Skipped when the value is unchanged (a # no-op edit), or for non-preset automations. - if ( - "prompt" in update_data - and isinstance(auto.prompt, str) - and auto.prompt != original_prompt + if isinstance(auto.prompt, str) and ( + ("prompt" in update_data and auto.prompt != original_prompt) + or (profile_changed and auto.preset_metadata) ): new_tarball_path = await regenerate_preset_prompt_tarball( - auto, auto.prompt, session, background_tasks + auto, auto.prompt, session, background_tasks, refresh_runner=profile_changed ) if new_tarball_path is not None: auto.tarball_path = new_tarball_path diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index e462a943..abe6f424 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -430,6 +430,13 @@ class CreateAutomationRequest(BaseModel): "a persistent agent conversation." ), ) + agent_profile_id: uuid.UUID | None = Field( + default=None, + description=( + "Agent profile selected for conversation-scoped execution. The " + "profile owns agent settings and secret selection." + ), + ) name: str = Field(..., min_length=1, max_length=500) model: str | None = Field( default=None, @@ -531,6 +538,13 @@ class UpdateAutomationRequest(BaseModel): model_config = ConfigDict(extra="forbid") execution_scope: Literal["run", "conversation"] | None = None + agent_profile_id: uuid.UUID | None = Field( + default=None, + description=( + "Agent profile selected for conversation-scoped execution. The " + "profile owns agent settings and secret selection." + ), + ) name: str | None = Field(default=None, min_length=1, max_length=500) model: str | None = Field( default=None, @@ -871,6 +885,7 @@ class TelemetryConsentResponse(BaseModel): class AutomationResponse(BaseModel): execution_scope: Literal["run", "conversation"] = "run" + agent_profile_id: uuid.UUID | None = None id: uuid.UUID user_id: uuid.UUID org_id: uuid.UUID @@ -949,6 +964,7 @@ class AutomationRunResponse(BaseModel): """Response for a single automation run.""" execution_scope: Literal["run", "conversation"] = "run" + agent_profile_id: uuid.UUID | None = None id: uuid.UUID automation_id: uuid.UUID status: RunStatus diff --git a/openhands/automation/utils/model_profiles.py b/openhands/automation/utils/model_profiles.py index 1b8e6365..afef5184 100644 --- a/openhands/automation/utils/model_profiles.py +++ b/openhands/automation/utils/model_profiles.py @@ -1,5 +1,7 @@ """Helpers for resolving and validating model profile selections.""" +import uuid + from fastapi import HTTPException, status from openhands.automation.auth import AuthenticatedUser @@ -37,3 +39,17 @@ def resolve_model_profile_for_user( model_profile = requested_profile or user.active_model_profile_name validate_model_profile_for_user(model_profile, user) return model_profile + + +def validate_agent_profile_selection( + agent_profile_id: uuid.UUID | None, model: str | None +) -> None: + """An agent profile owns its model and is resolved by the configured server.""" + if agent_profile_id is None: + return + from openhands.automation.config import get_config + + if not get_config().service.is_local_mode: + raise HTTPException(422, "Agent profiles require a configured Agent Server") + if model: + raise HTTPException(422, "An agent profile already specifies the model") diff --git a/openhands/automation/utils/run.py b/openhands/automation/utils/run.py index 106ddd69..b91d8d27 100644 --- a/openhands/automation/utils/run.py +++ b/openhands/automation/utils/run.py @@ -181,6 +181,7 @@ async def create_pending_run( now = utcnow() run = AutomationRun( + agent_profile_id=automation.agent_profile_id, execution_scope=automation.execution_scope, id=uuid.uuid4(), automation_id=automation.id, diff --git a/tests/test_backends.py b/tests/test_backends.py index 3b840a6e..43044849 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -471,6 +471,7 @@ def mock_run(self): run = MagicMock() run.sandbox_id = "sandbox-123" run.execution_scope = "run" + run.agent_profile_id = None return run def test_local_mode(self, monkeypatch, mock_run): diff --git a/tests/test_cancel_run.py b/tests/test_cancel_run.py index 347155b1..98e55fbe 100644 --- a/tests/test_cancel_run.py +++ b/tests/test_cancel_run.py @@ -167,10 +167,7 @@ async def test_cancel_conversation_run_without_cloud_id( from unittest.mock import AsyncMock, Mock from openhands.automation import backends - from openhands.automation.config import clear_config_cache - monkeypatch.setenv("AUTOMATION_AGENT_PROFILE", str(uuid.uuid4())) - clear_config_cache() backend = Mock( cleanup_after_verification=AsyncMock( side_effect=RuntimeError("Runtime temporarily unreachable") @@ -179,15 +176,13 @@ async def test_cancel_conversation_run_without_cloud_id( ) ) monkeypatch.setattr(backends, "get_backend", lambda run: backend) - try: - _, run = await _create_automation_with_run( - async_session, status=AutomationRunStatus.RUNNING - ) - run.execution_scope = "conversation" - run_id = str(run.id) - resp = await async_client.post(f"/api/automation/v1/runs/{run_id}/cancel") - assert resp.status_code == 200 - assert resp.json()["status"] == "CANCELLED" - backend.cleanup_after_verification.assert_awaited_once_with(run_id) - finally: - clear_config_cache() + _, run = await _create_automation_with_run( + async_session, status=AutomationRunStatus.RUNNING + ) + run.execution_scope = "conversation" + run.agent_profile_id = uuid.uuid4() + run_id = str(run.id) + resp = await async_client.post(f"/api/automation/v1/runs/{run_id}/cancel") + assert resp.status_code == 200 + assert resp.json()["status"] == "CANCELLED" + backend.cleanup_after_verification.assert_awaited_once_with(run_id) diff --git a/tests/test_conversation_backend.py b/tests/test_conversation_backend.py index 64eaf222..4f84a8b2 100644 --- a/tests/test_conversation_backend.py +++ b/tests/test_conversation_backend.py @@ -86,6 +86,7 @@ def respond(request): "AGENT_SERVER_URL", "SESSION_API_KEY", "AUTOMATION_CONVERSATION_ID", + "AUTOMATION_AGENT_PROFILE_ID", "WORKSPACE_BASE", } assert env["AUTOMATION_CONVERSATION_ID"] == conversation_id @@ -141,29 +142,33 @@ def respond(request): @pytest.mark.parametrize( - ("execution_scope", "expected_type"), - [("run", RunBackend), ("conversation", ConversationBackend)], + ("profile", "execution_scope", "expected_type"), + [ + (False, "run", RunBackend), + (True, "run", RunBackend), + (True, "conversation", ConversationBackend), + ], ) -def test_execution_scope_selects_backend(monkeypatch, execution_scope, expected_type): +def test_execution_scope_selects_backend( + monkeypatch, profile, execution_scope, expected_type +): from openhands.automation.backends import get_backend from openhands.automation.config import clear_config_cache - automation_id = uuid4() - profile_id = uuid4() + selected = uuid4() monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") - monkeypatch.setenv("AUTOMATION_AGENT_PROFILE", str(profile_id)) clear_config_cache() try: backend = get_backend( AutomationRun( id=uuid4(), - automation_id=automation_id, + agent_profile_id=selected if profile else None, execution_scope=execution_scope, ) ) assert type(backend) is expected_type if isinstance(backend, ConversationBackend): - assert backend.agent_profile_id == profile_id + assert backend.agent_profile_id == selected else: assert type(backend.provider) is ExistingAgentServerProvider finally: @@ -175,7 +180,6 @@ def test_conversation_execution_requires_profile(monkeypatch): from openhands.automation.config import clear_config_cache monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") - monkeypatch.delenv("AUTOMATION_AGENT_PROFILE", raising=False) clear_config_cache() try: with pytest.raises(ValueError, match="requires an agent profile"): @@ -184,6 +188,32 @@ def test_conversation_execution_requires_profile(monkeypatch): clear_config_cache() +@pytest.mark.asyncio +async def test_failed_credential_handoff_releases_runtime( + sdk_http_transport, monkeypatch +): + run = AutomationRun(id=uuid4(), automation=Automation(name="reviewer")) + provider = ExistingAgentServerProvider("http://server", "host-key", run) + backend = ConversationBackend(provider, agent_profile_id=uuid4()) + requests = [] + + def respond(request): + requests.append(request) + if request.url.path == "/server_info": + return httpx.Response(200, json={"conversation_runtime": "docker"}) + return httpx.Response(409 if request.url.path.endswith("credentials") else 200) + + monkeypatch.setattr(backend, "_create_conversation", lambda: None) + sdk_http_transport(respond) + async with httpx.AsyncClient() as client: + with pytest.raises(httpx.HTTPStatusError): + await backend.get_execution_context(client) + assert requests[-1].method == "DELETE" + assert requests[-1].url.path.endswith("/runtime") + with pytest.raises(RuntimeError, match="not been provisioned"): + backend.build_env_vars() + + @pytest.mark.asyncio async def test_cleanup_uses_sdk_lifecycle_timeout(): # A runtime release can exceed HTTPX's implicit five-second client timeout. diff --git a/tests/test_db.py b/tests/test_db.py index 488a2961..3215383b 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -263,6 +263,10 @@ def test_migrations_run_on_sqlite(self, monkeypatch): assert "execution_scope" in { column["name"] for column in inspector.get_columns("automations") } + assert "agent_profile_id" in run_columns + assert "agent_profile_id" in { + column["name"] for column in inspector.get_columns("automations") + } engine.dispose() finally: diff --git a/tests/test_git_sync.py b/tests/test_git_sync.py index 4b835121..57cb1582 100644 --- a/tests/test_git_sync.py +++ b/tests/test_git_sync.py @@ -1835,6 +1835,45 @@ async def test_yaml_only_edit_does_not_create_a_new_upload( uploads = (await session.execute(select(TarballUpload))).scalars().all() assert len(uploads) == upload_count_before + @pytest.mark.parametrize("model", [None, "explicit-model"]) + async def test_profile_import_uses_api_validation( + self, + sqlite_session_factory, + file_store, + git_settings, + service_settings, + origin, + model, + ): + automation_id = await _create_internal_automation( + sqlite_session_factory, file_store + ) + await run_sync_cycle( + sqlite_session_factory, LOCAL_ORG_ID, git_settings, service_settings + ) + selected = uuid.uuid4() + await self._push_yaml_edit( + origin, + "editor-profile", + "agent_profile_id: null", + f"agent_profile_id: {selected}", + ) + if model: + await self._push_yaml_edit( + origin, "editor-model", "model: null", f"model: {model}" + ) + + await run_sync_cycle( + sqlite_session_factory, LOCAL_ORG_ID, git_settings, service_settings + ) + async with sqlite_session_factory() as session: + automation = await session.get(Automation, automation_id) + assert automation.agent_profile_id == (None if model else selected) + assert automation.model is None + assert ( + len((await session.execute(select(TarballUpload))).scalars().all()) == 1 + ) + async def test_superseded_upload_is_soft_deleted_when_the_tarball_changes( self, sqlite_session_factory, file_store, git_settings, service_settings, origin ): diff --git a/tests/test_git_sync_serializer.py b/tests/test_git_sync_serializer.py index 9bcaab35..e62ad0f6 100644 --- a/tests/test_git_sync_serializer.py +++ b/tests/test_git_sync_serializer.py @@ -387,3 +387,11 @@ def test_repacking_is_stable_across_framings(self): {"b.py": (b"y", 0o755), "a.py": (b"x", 0o644)} ) assert canonical_tarball_bytes(first) == canonical_tarball_bytes(second) + + +def test_profile_reference_survives_git_round_trip(): + selected = uuid.uuid4() + automation = _make_automation(agent_profile_id=selected) + restored = deserialize_automation(serialize_automation(automation, None)) + assert restored is not None + assert restored.fields["agent_profile_id"] == str(selected) diff --git a/tests/test_router.py b/tests/test_router.py index c42f3fb4..f1e2bd60 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -312,6 +312,45 @@ async def test_disable_with_edits_as_non_creator_manager_returns_403( class TestCreateAutomation: """Tests for POST /v1 endpoint.""" + async def test_profile_round_trip_and_queued_run_snapshot( + self, async_client, async_session, local_mode + ): + from openhands.automation.utils.run import create_pending_run + + selected, replacement = uuid.uuid4(), uuid.uuid4() + response = await async_client.post( + "/api/automation/v1", + json={ + "name": "Independent reviewer", + "agent_profile_id": str(selected), + "trigger": {"type": "cron", "schedule": "*/5 * * * *"}, + "tarball_path": "s3://bucket/reviewer.tar.gz", + "entrypoint": "python3 main.py", + }, + ) + assert response.status_code == 201 + data = response.json() + assert data["agent_profile_id"] == str(selected) + path = "/api/automation/v1/" + data["id"] + automation = await async_session.get(Automation, uuid.UUID(data["id"])) + queued = await create_pending_run(async_session, automation) + await async_session.commit() + changed = await async_client.patch( + path, json={"agent_profile_id": str(replacement)} + ) + assert changed.status_code == 200 + assert changed.json()["agent_profile_id"] == str(replacement) + await async_session.refresh(queued) + assert queued.agent_profile_id == selected + cleared = await async_client.patch(path, json={"agent_profile_id": None}) + assert cleared.status_code == 200 + assert cleared.json()["agent_profile_id"] is None + assert (await async_client.get(path)).json()["agent_profile_id"] is None + invalid = await async_client.patch( + path, json={"agent_profile_id": "missing-profile"} + ) + assert invalid.status_code == 422 + async def test_create_automation_success( self, async_client, async_session, local_mode ): @@ -1200,6 +1239,48 @@ async def test_delete_automation_already_deleted(self, async_client, async_sessi class TestUpdateAutomation: + async def test_selecting_profile_refreshes_existing_preset_runner( + self, async_client, async_session, preset_store, local_mode + ): + automation = await _seed_prompt_preset_automation( + async_session, preset_store, "Keep this task" + ) + automation.preset_metadata = {"preset_type": "prompt"} + await async_session.commit() + upload_id = parse_internal_upload_id(automation.tarball_path) + assert upload_id is not None + old_path = _build_storage_path(TEST_ORG_ID, TEST_USER_ID, upload_id) + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as tar: + for name, data in { + "main.py": b"legacy runner", + "prompt.txt": b"Keep this task", + "repos_config.json": b"[]", + }.items(): + info = tarfile.TarInfo(name) + info.size = len(data) + tar.addfile(info, io.BytesIO(data)) + preset_store._storage[old_path] = buffer.getvalue() + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"agent_profile_id": str(uuid.uuid4())}, + ) + assert response.status_code == 200 + new_id = parse_internal_upload_id(response.json()["tarball_path"]) + assert new_id is not None and new_id != upload_id + new_path = _build_storage_path(TEST_ORG_ID, TEST_USER_ID, new_id) + with tarfile.open( + fileobj=io.BytesIO(preset_store._storage[new_path]), mode="r:gz" + ) as tar: + runner = tar.extractfile("main.py") + prompt = tar.extractfile("prompt.txt") + repos = tar.extractfile("repos_config.json") + assert runner and prompt and repos + assert b"conversation = RemoteConversation.attach(" in runner.read() + assert "agent_profile.py" not in tar.getnames() + assert prompt.read() == b"Keep this task" + assert repos.read() == b"[]" + """Tests for PATCH /v1/{id} endpoint.""" async def test_update_automation_name(self, async_client, async_session): From 6d36f7a10d41989d9bdbcb7b7678f826c7f7361d Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 14 Sep 2026 17:51:53 +0000 Subject: [PATCH 3/7] feat: separate profile scope from agent execution Co-authored-by: openhands --- README.md | 38 ++++++++++++++++----------- openhands/automation/dispatcher.py | 3 +++ openhands/automation/execution.py | 7 ++++- openhands/automation/preset_router.py | 2 ++ openhands/automation/schemas.py | 24 ++++++++++++----- openhands/automation/utils/webhook.py | 1 + pyproject.toml | 4 +-- tests/test_execution.py | 4 +++ uv.lock | 4 +-- 9 files changed, 60 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 1184f864..75d5f3ce 100644 --- a/README.md +++ b/README.md @@ -19,19 +19,21 @@ The Automation Service owns automation definitions, cron scheduling, webhooks, r ## Development -### Conversation execution in local or Docker workspaces +### Run and conversation execution Set `AUTOMATION_AGENT_SERVER_URL` and `AUTOMATION_AGENT_SERVER_API_KEY`, then -select a saved `agent_profile_id` on each conversation-scoped automation. The -selected profile is snapshotted when the run is queued. The backend reads the -server's authoritative `conversation_runtime`. Definitions with -`execution_scope: "conversation"` provision a conversation using the same API -and profile in either workspace mode; definitions default to -`execution_scope: "run"` and run their entrypoint directly. Profile -configuration never selects the execution scope. No bundle configuration or -workflow branch changes when switching workspace kind. - -Both workspace kinds supply `AUTOMATION_CONVERSATION_ID`, `AGENT_SERVER_URL`, +choose an `execution_scope` and optional saved `agent_profile_id` for each +automation. Both values are snapshotted when a run is queued. Scope controls +lifecycle: `run` executes the bundle without creating a conversation, while +`conversation` creates a persistent conversation and requires a profile. +Profile selection independently controls agent settings and credentials. A +profile on run-scoped work limits the saved secrets available to its command. + +Only conversation-scoped execution reads the server's authoritative +`conversation_runtime`. The service uses the same profile-backed conversation +API in local and Docker workspaces, so automation code remains independent of +workspace kind. Both workspace kinds supply `AUTOMATION_CONVERSATION_ID`, +`AGENT_SERVER_URL`, `SESSION_API_KEY`, and `WORKSPACE_BASE`, and use conversation-scoped upload, bash execution, and completion verification. Local workspaces live in per-run subdirectories of the configured workspace root. Docker workspaces use @@ -51,10 +53,9 @@ and PIDs in the server configuration. Completed Docker runtimes are released whi history remains; the persistent local server and its history are retained. The Agent Server resolves each selected profile at dispatch, including its -model, tools, and `secret_refs`. Missing profiles or secrets fail creation -instead of falling back to a more privileged agent. A separate `model` -selection cannot be combined with an agent profile. Run-scoped automations may -omit `agent_profile_id`. +model, tools, and `secret_refs`. Missing profiles or selected secrets fail +instead of falling back to a more privileged scope. A separate `model` +selection cannot be combined with an agent profile. Profile selection is advertised by the `agentProfiles` capability when an Agent Server is configured. Cloud dispatch without a configured Agent Server @@ -148,7 +149,10 @@ Each automation chooses its saved Agent Server profile through the create or patch API. Profile IDs are included in git sync and run history: ```json -{"agent_profile_id": "11111111-1111-4111-8111-111111111111"} +{ + "agent_profile_id": "11111111-1111-4111-8111-111111111111", + "execution_scope": "run" +} ``` The automation definition contains no token values or host-side permission @@ -159,6 +163,8 @@ the Agent Server's secret store. The conversation backend and Agent Server execution helpers reuse `RemoteConversation` and `RemoteWorkspace` from [software-agent-sdk #5010](https://github.com/OpenHands/software-agent-sdk/pull/5010). +Profile-scoped script commands use +[software-agent-sdk #5046](https://github.com/OpenHands/software-agent-sdk/pull/5046). This draft pins the SDK implementation by immutable Git commit so its tests and source installation are reproducible. Replace that integration pin with the SDK release before merging. The live factory additionally integrates the server diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index 783f227c..07df47af 100644 --- a/openhands/automation/dispatcher.py +++ b/openhands/automation/dispatcher.py @@ -485,6 +485,9 @@ async def _fail( run_id=run_id, sandbox_id=ctx.sandbox_id, runtime_conversation_id=ctx.runtime_conversation_id, + agent_profile_id=( + run.agent_profile_id if run.execution_scope == "run" else None + ), ) except PermanentDispatchError as exc: logger.error( diff --git a/openhands/automation/execution.py b/openhands/automation/execution.py index f01e9c1a..627e441b 100644 --- a/openhands/automation/execution.py +++ b/openhands/automation/execution.py @@ -217,6 +217,7 @@ async def _start_bash( command: str, timeout: int | None = None, runtime_conversation_id: UUID | None = None, + agent_profile_id: UUID | None = None, ) -> str: """Start a bash command in the background. Returns the command ID.""" if timeout is None: @@ -227,7 +228,9 @@ async def _start_bash( working_dir="/", runtime_conversation_id=runtime_conversation_id, ) as workspace: - return await workspace.start_command(command, timeout=timeout) + return await workspace.start_command( + command, timeout=timeout, agent_profile_id=agent_profile_id + ) def _is_permanent_http_error(stderr: str) -> bool: @@ -342,6 +345,7 @@ async def execute_in_context( run_id: str | None = None, sandbox_id: str | None = None, runtime_conversation_id: UUID | None = None, + agent_profile_id: UUID | None = None, ) -> DispatchResult: """Execute automation code in an existing execution context. @@ -437,6 +441,7 @@ def _log_ctx() -> dict[str, Any]: cmd, timeout=timeout, runtime_conversation_id=runtime_conversation_id, + agent_profile_id=agent_profile_id, ) env_path = None logger.info( diff --git a/openhands/automation/preset_router.py b/openhands/automation/preset_router.py index 52834af2..461bc2cb 100644 --- a/openhands/automation/preset_router.py +++ b/openhands/automation/preset_router.py @@ -579,6 +579,7 @@ async def create_automation_from_prompt( preset_metadata=preset_metadata, model=model, agent_profile_id=body.agent_profile_id, + execution_scope="conversation" if body.agent_profile_id else "run", trigger=body.trigger.model_dump(), tarball_path=tarball_path, setup_script_path="setup.sh", @@ -1024,6 +1025,7 @@ async def create_automation_from_plugin( preset_metadata=preset_metadata, model=model, agent_profile_id=body.agent_profile_id, + execution_scope="conversation" if body.agent_profile_id else "run", trigger=body.trigger.model_dump(), tarball_path=tarball_path, setup_script_path="setup.sh", diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index abe6f424..beb2462a 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -433,8 +433,8 @@ class CreateAutomationRequest(BaseModel): agent_profile_id: uuid.UUID | None = Field( default=None, description=( - "Agent profile selected for conversation-scoped execution. The " - "profile owns agent settings and secret selection." + "Selected agent profile. It scopes saved secrets for run-scoped " + "work and supplies settings and secrets for conversations." ), ) name: str = Field(..., min_length=1, max_length=500) @@ -541,8 +541,8 @@ class UpdateAutomationRequest(BaseModel): agent_profile_id: uuid.UUID | None = Field( default=None, description=( - "Agent profile selected for conversation-scoped execution. The " - "profile owns agent settings and secret selection." + "Selected agent profile. It scopes saved secrets for run-scoped " + "work and supplies settings and secrets for conversations." ), ) name: str | None = Field(default=None, min_length=1, max_length=500) @@ -885,7 +885,13 @@ class TelemetryConsentResponse(BaseModel): class AutomationResponse(BaseModel): execution_scope: Literal["run", "conversation"] = "run" - agent_profile_id: uuid.UUID | None = None + agent_profile_id: uuid.UUID | None = Field( + default=None, + description=( + "Selected agent profile. It scopes saved secrets for run-scoped " + "work and supplies settings and secrets for conversations." + ), + ) id: uuid.UUID user_id: uuid.UUID org_id: uuid.UUID @@ -964,7 +970,13 @@ class AutomationRunResponse(BaseModel): """Response for a single automation run.""" execution_scope: Literal["run", "conversation"] = "run" - agent_profile_id: uuid.UUID | None = None + agent_profile_id: uuid.UUID | None = Field( + default=None, + description=( + "Selected agent profile. It scopes saved secrets for run-scoped " + "work and supplies settings and secrets for conversations." + ), + ) id: uuid.UUID automation_id: uuid.UUID status: RunStatus diff --git a/openhands/automation/utils/webhook.py b/openhands/automation/utils/webhook.py index 91a5eb51..df3d52ec 100644 --- a/openhands/automation/utils/webhook.py +++ b/openhands/automation/utils/webhook.py @@ -266,6 +266,7 @@ async def create_automation_run( status=AutomationRunStatus.PENDING, event_payload=event_payload, telemetry_distinct_id=automation.telemetry_distinct_id, + agent_profile_id=automation.agent_profile_id, subject_key=subject_key, ) session.add(run) diff --git a/pyproject.toml b/pyproject.toml index 4c239927..6daba61c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,8 +25,8 @@ dependencies = [ "google-cloud-storage>=2.18", "httpx>=0.27", "jmespath>=1.0", - # Temporary integration pin for software-agent-sdk#5010; replace with its release. - "openhands-sdk @ git+https://github.com/OpenHands/software-agent-sdk.git@e9392e7cf35fcb011acf2eca825a80f9a22774ea#subdirectory=openhands-sdk", + # Temporary integration pin for software-agent-sdk#5046; replace with its release. + "openhands-sdk @ git+https://github.com/OpenHands/software-agent-sdk.git@91a259dcf62eb9ff688291bd01a355529488aaf3#subdirectory=openhands-sdk", "openhands-workspace==1.46.0", "pg8000>=1.31", "prometheus-client>=0.19", diff --git a/tests/test_execution.py b/tests/test_execution.py index d54c92a2..2b0227d4 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -7,6 +7,7 @@ import io import subprocess import tarfile +import uuid from unittest.mock import AsyncMock, MagicMock, patch import httpx @@ -238,6 +239,7 @@ async def test_transient_error_returns_dispatch_result( async def test_custom_timeout_is_passed_to_bash(self, mock_upload, mock_start_bash): """execute_in_context passes above-default custom timeouts to bash.""" mock_start_bash.return_value = "cmd-1" + profile_id = uuid.uuid4() result = await execute_in_context( agent_url="https://agent.example.com", @@ -246,11 +248,13 @@ async def test_custom_timeout_is_passed_to_bash(self, mock_upload, mock_start_ba tarball_source=b"test tarball", work_dir=DEFAULT_WORK_DIR, timeout=1200, + agent_profile_id=profile_id, ) assert result.success is True mock_upload.assert_awaited_once() assert mock_start_bash.await_args.kwargs["timeout"] == 1200 + assert mock_start_bash.await_args.kwargs["agent_profile_id"] == profile_id @pytest.mark.asyncio @patch("openhands.automation.execution._upload") diff --git a/uv.lock b/uv.lock index f3f87f14..de0b5acb 100644 --- a/uv.lock +++ b/uv.lock @@ -2269,7 +2269,7 @@ requires-dist = [ { name = "google-cloud-storage", specifier = ">=2.18" }, { name = "httpx", specifier = ">=0.27" }, { name = "jmespath", specifier = ">=1.0" }, - { name = "openhands-sdk", git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=e9392e7cf35fcb011acf2eca825a80f9a22774ea" }, + { name = "openhands-sdk", git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=91a259dcf62eb9ff688291bd01a355529488aaf3" }, { name = "openhands-workspace", specifier = "==1.46.0" }, { name = "pg8000", specifier = ">=1.31" }, { name = "prometheus-client", specifier = ">=0.19" }, @@ -2302,7 +2302,7 @@ dev = [ [[package]] name = "openhands-sdk" version = "1.47.0" -source = { git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=e9392e7cf35fcb011acf2eca825a80f9a22774ea#e9392e7cf35fcb011acf2eca825a80f9a22774ea" } +source = { git = "https://github.com/OpenHands/software-agent-sdk.git?subdirectory=openhands-sdk&rev=91a259dcf62eb9ff688291bd01a355529488aaf3#91a259dcf62eb9ff688291bd01a355529488aaf3" } dependencies = [ { name = "agent-client-protocol" }, { name = "deprecation" }, From 988bdee8fbd6549641e80ce6a8c491e279adee71 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 14 Sep 2026 18:03:49 +0000 Subject: [PATCH 4/7] feat: run scanner-selected conversation turns Co-authored-by: openhands --- .../027_add_conversation_turn_runs.py | 74 +++++++++++ openhands/automation/backends/conversation.py | 2 + openhands/automation/conversations.py | 13 +- openhands/automation/dispatcher.py | 117 ++++++++++++++---- openhands/automation/models.py | 11 ++ .../automation/utils/conversation_turn.py | 49 ++++++++ openhands/automation/utils/run.py | 30 +++++ openhands/automation/utils/webhook.py | 3 + tests/test_conversation_turn.py | 32 +++++ tests/test_conversations.py | 5 +- tests/test_db.py | 3 + tests/test_dispatcher.py | 115 +++++++++++++++++ 12 files changed, 424 insertions(+), 30 deletions(-) create mode 100644 migrations/versions/027_add_conversation_turn_runs.py diff --git a/migrations/versions/027_add_conversation_turn_runs.py b/migrations/versions/027_add_conversation_turn_runs.py new file mode 100644 index 00000000..911f3a26 --- /dev/null +++ b/migrations/versions/027_add_conversation_turn_runs.py @@ -0,0 +1,74 @@ +"""Add service-owned conversation-turn runs. + +Revision ID: 027 +Revises: 026 +""" + +import sqlalchemy as sa +from alembic import op + + +revision = "027" +down_revision = "026" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_index("ix_automation_runs_subject", table_name="automation_runs") + op.add_column( + "automation_runs", sa.Column("subject_source", sa.String(100), nullable=True) + ) + op.add_column( + "automation_runs", sa.Column("conversation_turn", sa.Text(), nullable=True) + ) + op.add_column( + "automation_runs", + sa.Column("conversation_wake_agent", sa.Boolean(), nullable=True), + ) + op.execute( + """ + UPDATE automation_runs + SET subject_source = ( + SELECT json_extract(automations.trigger, '$.source') + FROM automations + WHERE automations.id = automation_runs.automation_id + ) + WHERE subject_key IS NOT NULL + """ + if op.get_context().dialect.name == "sqlite" + else """ + UPDATE automation_runs AS runs + SET subject_source = automations.trigger ->> 'source' + FROM automations + WHERE automations.id = runs.automation_id + AND runs.subject_key IS NOT NULL + """ + ) + op.create_index( + "ix_automation_runs_subject", + "automation_runs", + ["automation_id", "subject_source", "subject_key", "created_at"], + unique=False, + postgresql_where=sa.text( + "subject_key IS NOT NULL AND subject_released_at IS NULL" + ), + sqlite_where=sa.text("subject_key IS NOT NULL AND subject_released_at IS NULL"), + ) + + +def downgrade() -> None: + op.drop_index("ix_automation_runs_subject", table_name="automation_runs") + op.drop_column("automation_runs", "conversation_turn") + op.drop_column("automation_runs", "conversation_wake_agent") + op.drop_column("automation_runs", "subject_source") + op.create_index( + "ix_automation_runs_subject", + "automation_runs", + ["automation_id", "subject_key", "created_at"], + unique=False, + postgresql_where=sa.text( + "subject_key IS NOT NULL AND subject_released_at IS NULL" + ), + sqlite_where=sa.text("subject_key IS NOT NULL AND subject_released_at IS NULL"), + ) diff --git a/openhands/automation/backends/conversation.py b/openhands/automation/backends/conversation.py index f9702cd3..2ac5964e 100644 --- a/openhands/automation/backends/conversation.py +++ b/openhands/automation/backends/conversation.py @@ -50,6 +50,8 @@ async def get_api_key(self) -> str: @property def conversation_id(self) -> UUID: + if self.provider.run.conversation_id: + return UUID(self.provider.run.conversation_id) automation = self.provider.run.automation source = (automation.trigger or {}).get("source") if self.provider.run.subject_key and source: diff --git a/openhands/automation/conversations.py b/openhands/automation/conversations.py index a07ee192..5c100a39 100644 --- a/openhands/automation/conversations.py +++ b/openhands/automation/conversations.py @@ -152,6 +152,7 @@ def _clean_key(value: str, origin: str) -> str | None: async def _take_subject_lock( session: AsyncSession, automation_id: uuid.UUID, + source: str, subject_key: str, ) -> None: """Serialise every event for one subject, including those finding no run. @@ -170,7 +171,7 @@ async def _take_subject_lock( # Hashed here, not with `hashtextextended`, so the key does not depend on # a server-side hash staying stable across versions. digest = hashlib.blake2b( - f"{automation_id}/{subject_key}".encode(), digest_size=8 + f"{automation_id}/{source}/{subject_key}".encode(), digest_size=8 ).digest() await session.execute( text("SELECT pg_advisory_xact_lock(:key)").bindparams( @@ -182,6 +183,7 @@ async def _take_subject_lock( async def _lock_subject_run( session: AsyncSession, automation_id: uuid.UUID, + source: str, subject_key: str, ) -> AutomationRun | None: """The most recent run holding this subject, started or not, locked. @@ -199,6 +201,7 @@ async def _lock_subject_run( select(AutomationRun) .where( AutomationRun.automation_id == automation_id, + AutomationRun.subject_source == source, AutomationRun.subject_key == subject_key, AutomationRun.subject_released_at.is_(None), ) @@ -258,13 +261,15 @@ async def continue_conversation( The id is known before the first run finishes, so an event arriving mid-run continues that conversation instead of racing a second run. """ - await _take_subject_lock(session, automation_id, subject_key) + await _take_subject_lock(session, automation_id, source, subject_key) - run = await _lock_subject_run(session, automation_id, subject_key) + run = await _lock_subject_run(session, automation_id, source, subject_key) if run is None: return ContinueResult() - conversation_id = conversation_id_for(org_id, automation_id, source, subject_key) + conversation_id = run.conversation_id or conversation_id_for( + org_id, automation_id, source, subject_key + ) turn = compose_turn(source, event_key, event_payload, override=turn_text) if run.started_at is None: diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index 07df47af..961212b7 100644 --- a/openhands/automation/dispatcher.py +++ b/openhands/automation/dispatcher.py @@ -22,7 +22,7 @@ from typing import Any import httpx -from sqlalchemy import select, update +from sqlalchemy import or_, select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from sqlalchemy.orm import selectinload @@ -46,6 +46,7 @@ from openhands.automation.telemetry import capture_automation_event from openhands.automation.utils import log_extra from openhands.automation.utils.api_key import APIKeyError +from openhands.automation.utils.conversation_turn import run_conversation_turn from openhands.automation.utils.kv import create_kv_token from openhands.automation.utils.run import ( disable_automation, @@ -132,26 +133,28 @@ async def _poll_pending_runs( ``org_id``, and tarball config are available for dispatch. """ settings = get_config().service - active_conversation_automations = [] + active: list[tuple[uuid.UUID, uuid.UUID, str]] = [] conversation_capacity: int | None = None if settings.is_local_mode: - active_conversation_automations = ( + active = list( ( await session.execute( - select(AutomationRun.automation_id).where( - AutomationRun.status == AutomationRunStatus.RUNNING, - AutomationRun.execution_scope == "conversation", - ) + select( + AutomationRun.id, + AutomationRun.automation_id, + AutomationRun.execution_scope, + ).where(AutomationRun.status == AutomationRunStatus.RUNNING) ) ) - .scalars() + .tuples() .all() ) + active_conversations = [row for row in active if row[2] == "conversation"] conversation_capacity = settings.conversation_max_concurrent_runs - len( - active_conversation_automations + active_conversations ) if conversation_capacity > 0: - batch_size = min(batch_size, 1, conversation_capacity) + batch_size = min(batch_size, conversation_capacity) select_query = ( select(AutomationRun) @@ -171,10 +174,21 @@ async def _poll_pending_runs( select_query = select_query.where( AutomationRun.execution_scope != "conversation" ) - elif active_conversation_automations: - select_query = select_query.where( - AutomationRun.automation_id.not_in(active_conversation_automations) - ) + if active: + active_run_automations = { + automation_id + for _, automation_id, execution_scope in active + if execution_scope == "run" + } + if active_run_automations: + # Do not overlap two run-scoped commands for one definition. + # Conversation-scoped work may fan out independently. + select_query = select_query.where( + or_( + AutomationRun.execution_scope == "conversation", + AutomationRun.automation_id.not_in(active_run_automations), + ) + ) # Apply row locking for PostgreSQL only (SQLite doesn't support it) if not using_sqlite(): @@ -356,6 +370,71 @@ async def _fail( extra=_log_ctx(sandbox_id=ctx.sandbox_id), ) + # Make the provisioned context discoverable before work starts so a + # follow-up event can find a conversation that is already running. + if ctx.runtime_conversation_id is not None: + run.conversation_id = str(ctx.runtime_conversation_id) + async with session_factory() as link_session: + await link_session.execute( + update(AutomationRun) + .where(AutomationRun.id == run.id) + .values(conversation_id=run.conversation_id) + ) + await link_session.commit() + if ctx.sandbox_id: + run.sandbox_id = ctx.sandbox_id + await update_sandbox_id(session_factory, run.id, ctx.sandbox_id) + + if run.conversation_turn is not None: + assert run.conversation_id is not None + await update_run_current_phase(session_factory, run.id, "Agent is working") + try: + await run_conversation_turn( + ctx, + run.conversation_id, + run.conversation_turn, + wake_agent=run.conversation_wake_agent is not False, + timeout=effective_timeout, + ) + except Exception as exc: + run.subject_released_at = utcnow() + async with session_factory() as release_session: + await release_session.execute( + update(AutomationRun) + .where(AutomationRun.id == run.id) + .values(subject_released_at=run.subject_released_at) + ) + await release_session.commit() + await backend.release_context(client, ctx) + await _fail( + "Subject conversation failed", + status_detail=run_status_detail_from_exception( + exc, + phase=RunStatusPhase.EXECUTION, + source="agent_server", + operation="run_subject_turn", + ), + ) + return + try: + await backend.release_context(client, ctx) + except Exception: + # Conversation history is persisted outside the runtime; runtime + # TTL cleanup remains the fallback. + logger.exception( + "Failed to release completed subject runtime", + extra=_log_ctx(sandbox_id=ctx.sandbox_id), + ) + await mark_run_terminal(session_factory, run, AutomationRunStatus.COMPLETED) + await capture_automation_event( + "automation_run_completed", + automation=automation, + run=run, + session_factory=session_factory, + properties={"trigger_source": "subject_turn"}, + ) + return + # 3. Build env vars (must be after get_execution_context for cloud mode API key) callback_url = f"{settings.resolved_base_url.rstrip('/')}/v1/runs/{run_id}/complete" env_vars = backend.build_env_vars() @@ -530,17 +609,7 @@ async def _fail( # 6. Handle result if result.success: - if ctx.runtime_conversation_id is not None: - async with session_factory() as link_session: - await link_session.execute( - update(AutomationRun) - .where(AutomationRun.id == run.id) - .values(conversation_id=env_vars["AUTOMATION_CONVERSATION_ID"]) - ) - await link_session.commit() await update_run_current_phase(session_factory, run.id, "Starting automation") - if ctx.sandbox_id: - await update_sandbox_id(session_factory, run.id, ctx.sandbox_id) if result.bash_command_id: # Persist the BashCommand id so the verifier can filter # BashOutput events by exactly this command (avoids diff --git a/openhands/automation/models.py b/openhands/automation/models.py index 66310b57..b1a91426 100644 --- a/openhands/automation/models.py +++ b/openhands/automation/models.py @@ -8,6 +8,7 @@ from sqlalchemy import ( JSON, BigInteger, + Boolean, DateTime, Enum, Float, @@ -226,6 +227,15 @@ class AutomationRun(Base): # sandbox holding the conversation; the conversation id itself is derived. subject_key: Mapped[str | None] = mapped_column(String(500), nullable=True) + # Namespace of the external subject (for example ``github``). Event runs + # inherit it from their trigger; programmatic subject turns supply it. + subject_source: Mapped[str | None] = mapped_column(String(100), nullable=True) + + # A service-owned conversation turn. The dispatcher runs this text + # in the provisioned conversation instead of starting the bundle entrypoint. + conversation_turn: Mapped[str | None] = mapped_column(Text, nullable=True) + conversation_wake_agent: Mapped[bool | None] = mapped_column(Boolean, nullable=True) + # When this run stopped being the subject's routing target -- its sandbox # was deleted, or a turn could not reach it. The key itself stays for the # historical record, so lookups filter on this instead of on its absence. @@ -293,6 +303,7 @@ class AutomationRun(Base): Index( "ix_automation_runs_subject", "automation_id", + "subject_source", "subject_key", "created_at", postgresql_where=(subject_key.isnot(None)) diff --git a/openhands/automation/utils/conversation_turn.py b/openhands/automation/utils/conversation_turn.py index 97413ca9..f2f44439 100644 --- a/openhands/automation/utils/conversation_turn.py +++ b/openhands/automation/utils/conversation_turn.py @@ -21,6 +21,7 @@ import httpx from openhands.automation.backends import get_backend +from openhands.automation.backends.base import ExecutionContext from openhands.automation.backends.providers.existing import ( ExistingAgentServerProvider, ) @@ -253,6 +254,54 @@ def _send_turn( workspace.reset_client() +def _run_turn( + context: ExecutionContext, + conversation_id: str, + text: str, + wake_agent: bool, + timeout: int, +) -> None: + workspace = RemoteWorkspace( + host=context.agent_url, + api_key=context.session_key, + working_dir="/", + runtime_conversation_id=context.runtime_conversation_id, + ) + conversation = None + try: + conversation = RemoteConversation.attach( + workspace=workspace, + conversation_id=UUID(conversation_id), + visualizer=None, + ) + conversation.send_message(text) + if wake_agent: + conversation.run(timeout=timeout) + finally: + if conversation is not None: + conversation.close() + workspace.reset_client() + + +async def run_conversation_turn( + context: ExecutionContext, + conversation_id: str, + text: str, + *, + wake_agent: bool, + timeout: int, +) -> None: + """Run one service-owned turn in an already provisioned conversation.""" + await asyncio.to_thread( + _run_turn, + context, + conversation_id, + text, + wake_agent, + timeout, + ) + + async def send_conversation_turn( run: AutomationRun, conversation_id: str, diff --git a/openhands/automation/utils/run.py b/openhands/automation/utils/run.py index b91d8d27..801b2f9d 100644 --- a/openhands/automation/utils/run.py +++ b/openhands/automation/utils/run.py @@ -15,6 +15,7 @@ AutomationRun, AutomationRunStatus, ) +from openhands.automation.subjects import conversation_id_for from openhands.automation.telemetry import capture_automation_event from openhands.automation.utils.time import utcnow from openhands.automation.utils.timeout import resolve_automation_timeout_seconds @@ -30,6 +31,35 @@ ) +def create_conversation_turn_run( + requester: AutomationRun, + *, + source: str, + subject_key: str, + turn: str, + wake_agent: bool, +) -> AutomationRun: + """Build a conversation-scoped run selected by a run-scoped automation.""" + if requester.agent_profile_id is None: + raise ValueError("Conversation turns require an agent profile") + automation = requester.automation + return AutomationRun( + id=uuid.uuid4(), + automation_id=requester.automation_id, + agent_profile_id=requester.agent_profile_id, + execution_scope="conversation", + telemetry_distinct_id=requester.telemetry_distinct_id, + status=AutomationRunStatus.PENDING, + subject_source=source, + subject_key=subject_key, + conversation_id=conversation_id_for( + automation.org_id, automation.id, source, subject_key + ), + conversation_turn=turn, + conversation_wake_agent=wake_agent, + ) + + async def disable_automation( session_factory: async_sessionmaker[AsyncSession], automation_id: uuid.UUID, diff --git a/openhands/automation/utils/webhook.py b/openhands/automation/utils/webhook.py index df3d52ec..ba5088ae 100644 --- a/openhands/automation/utils/webhook.py +++ b/openhands/automation/utils/webhook.py @@ -268,6 +268,9 @@ async def create_automation_run( telemetry_distinct_id=automation.telemetry_distinct_id, agent_profile_id=automation.agent_profile_id, subject_key=subject_key, + subject_source=( + (automation.trigger or {}).get("source") if subject_key else None + ), ) session.add(run) return run diff --git a/tests/test_conversation_turn.py b/tests/test_conversation_turn.py index 6af36112..2bcc9063 100644 --- a/tests/test_conversation_turn.py +++ b/tests/test_conversation_turn.py @@ -12,12 +12,14 @@ import httpx import pytest +from openhands.automation.backends.base import ExecutionContext from openhands.automation.backends.conversation import ConversationBackend from openhands.automation.backends.providers.existing import ExistingAgentServerProvider from openhands.automation.backends.run import RunBackend from openhands.automation.models import AutomationRun from openhands.automation.utils import conversation_turn as turn_module, utcnow from openhands.automation.utils.conversation_turn import ( + _run_turn, compose_turn, send_conversation_turn, ) @@ -26,6 +28,36 @@ CONVERSATION_ID = str(uuid4()) +def test_service_owned_turn_keeps_the_runtime_scope(monkeypatch): + """Agent tools stay in the conversation's selected local/Docker runtime.""" + from unittest.mock import MagicMock + + runtime_id = uuid4() + workspace = MagicMock() + remote_workspace = MagicMock(return_value=workspace) + conversation = MagicMock() + monkeypatch.setattr(turn_module, "RemoteWorkspace", remote_workspace) + monkeypatch.setattr( + turn_module.RemoteConversation, "attach", MagicMock(return_value=conversation) + ) + + _run_turn( + ExecutionContext( + agent_url="https://agent.example.com", + session_key="outer-key", + runtime_conversation_id=runtime_id, + ), + CONVERSATION_ID, + "work on this", + True, + 120, + ) + + assert remote_workspace.call_args.kwargs["runtime_conversation_id"] == runtime_id + conversation.send_message.assert_called_once_with("work on this") + conversation.run.assert_called_once_with(timeout=120) + + @pytest.fixture def conversation_transport(sdk_http_transport, monkeypatch): from unittest.mock import MagicMock diff --git a/tests/test_conversations.py b/tests/test_conversations.py index 19d210f7..2d405295 100644 --- a/tests/test_conversations.py +++ b/tests/test_conversations.py @@ -1098,6 +1098,7 @@ def handler(request: httpx.Request) -> httpx.Response: status=AutomationRunStatus.COMPLETED, started_at=utcnow(), sandbox_id="sbx-1", + subject_source="slack", subject_key=subject_key, ) ) @@ -1295,7 +1296,7 @@ async def test_the_subject_lock_orders_events_that_find_no_run( async def hold(name: str, work: float) -> None: async with async_session_factory() as session: - await _take_subject_lock(session, automation_id, "T1/C1/1.1") + await _take_subject_lock(session, automation_id, "slack", "T1/C1/1.1") order.append(f"{name} in") await asyncio.sleep(work) order.append(f"{name} out") @@ -1331,7 +1332,7 @@ async def test_a_different_subject_is_not_blocked_by_a_held_lock( async def hold(name: str, subject_key: str, work: float) -> None: async with async_session_factory() as session: - await _take_subject_lock(session, automation_id, subject_key) + await _take_subject_lock(session, automation_id, "slack", subject_key) order.append(f"{name} in") await asyncio.sleep(work) order.append(f"{name} out") diff --git a/tests/test_db.py b/tests/test_db.py index 3215383b..e47f6d60 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -267,6 +267,9 @@ def test_migrations_run_on_sqlite(self, monkeypatch): assert "agent_profile_id" in { column["name"] for column in inspector.get_columns("automations") } + assert "subject_source" in run_columns + assert "conversation_turn" in run_columns + assert "conversation_wake_agent" in run_columns engine.dispose() finally: diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index da9e89e2..e7450c2e 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -29,6 +29,7 @@ from openhands.automation.subjects import conversation_id_for from openhands.automation.utils import utcnow from openhands.automation.utils.run import ( + create_conversation_turn_run, mark_run_status, mark_run_terminal, update_run_current_phase, @@ -75,6 +76,56 @@ def test_gs_url_is_not_http(self): assert is_http_url("gs://bucket/key.tar.gz") is False +@pytest.mark.asyncio +async def test_local_poller_fans_out_conversations_from_one_automation( + async_session, monkeypatch +): + from openhands.automation.config import clear_config_cache + + monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://agent.test") + monkeypatch.setenv("AUTOMATION_CONVERSATION_MAX_CONCURRENT_RUNS", "4") + clear_config_cache() + try: + automation = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Developer scanner", + trigger={"type": "cron", "schedule": "* * * * *", "timezone": "UTC"}, + tarball_path="https://example.com/scanner.tar.gz", + entrypoint="python scanner.py", + ) + async_session.add(automation) + await async_session.flush() + async_session.add( + AutomationRun( + automation_id=automation.id, + execution_scope="conversation", + status=AutomationRunStatus.RUNNING, + conversation_turn="Implement issue 1", + ) + ) + for number in (2, 3, 4): + async_session.add( + AutomationRun( + automation_id=automation.id, + execution_scope="conversation", + status=AutomationRunStatus.PENDING, + conversation_turn=f"Implement issue {number}", + ) + ) + await async_session.commit() + + pending = await _poll_pending_runs(async_session, batch_size=10) + + assert {run.conversation_turn for run in pending} == { + "Implement issue 2", + "Implement issue 3", + "Implement issue 4", + } + finally: + clear_config_cache() + + async def test_run_dispatch_is_not_blocked_by_full_conversation_capacity( async_session_factory, monkeypatch ): @@ -134,6 +185,70 @@ async def test_run_dispatch_is_not_blocked_by_full_conversation_capacity( clear_config_cache() +@pytest.mark.asyncio +async def test_conversation_turn_run_skips_bundle_and_releases_runtime( + async_session_factory, mock_settings, mock_client +): + async with async_session_factory() as session: + automation = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Developer scanner", + trigger={"type": "cron", "schedule": "* * * * *", "timezone": "UTC"}, + tarball_path="https://example.com/scanner.tar.gz", + entrypoint="python scanner.py", + ) + requester = AutomationRun( + automation=automation, + status=AutomationRunStatus.RUNNING, + agent_profile_id=uuid.uuid4(), + ) + session.add(requester) + await session.flush() + run = create_conversation_turn_run( + requester, + source="github", + subject_key="repository-42/issue-9", + turn="Implement issue 9", + wake_agent=True, + ) + run.status = AutomationRunStatus.RUNNING + run.started_at = utcnow() + assert run.conversation_id is not None + conversation_id = uuid.UUID(run.conversation_id) + session.add(run) + await session.commit() + run_id = run.id + await session.refresh(run, attribute_names=["automation"]) + + backend = AsyncMock() + backend.get_execution_context.return_value = ExecutionContext( + agent_url="http://agent.test", + session_key="runtime-key", + runtime_conversation_id=conversation_id, + ) + with ( + patch("openhands.automation.dispatcher.get_backend", return_value=backend), + patch( + "openhands.automation.dispatcher.run_conversation_turn", + new_callable=AsyncMock, + ) as run_turn, + patch( + "openhands.automation.dispatcher.execute_in_context", + new_callable=AsyncMock, + ) as execute_bundle, + ): + await _execute_run(run, mock_settings, async_session_factory, mock_client) + + run_turn.assert_awaited_once() + backend.release_context.assert_awaited_once() + execute_bundle.assert_not_awaited() + async with async_session_factory() as session: + finished = await session.get(AutomationRun, run_id) + assert finished is not None + assert finished.status == AutomationRunStatus.COMPLETED + + class TestMarkRunStatus: """Tests for mark_run_status function.""" From db6694c316de01d58ab4a05521fa6e77fb343e5e Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 14 Sep 2026 18:07:10 +0000 Subject: [PATCH 5/7] feat: add scoped subject-turn submission Co-authored-by: openhands --- README.md | 7 + .../versions/028_add_subject_turn_requests.py | 54 +++++ openhands/automation/app.py | 2 + openhands/automation/conversations.py | 149 +++++++++++- openhands/automation/dispatcher.py | 21 ++ openhands/automation/models.py | 42 ++++ openhands/automation/schemas.py | 28 +++ openhands/automation/subject_router.py | 93 +++++++ openhands/automation/utils/run_token.py | 73 ++++++ tests/test_db.py | 1 + tests/test_subject_turns.py | 227 ++++++++++++++++++ 11 files changed, 695 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/028_add_subject_turn_requests.py create mode 100644 openhands/automation/subject_router.py create mode 100644 openhands/automation/utils/run_token.py create mode 100644 tests/test_subject_turns.py diff --git a/README.md b/README.md index 75d5f3ce..f86a04e7 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,13 @@ lifecycle: `run` executes the bundle without creating a conversation, while Profile selection independently controls agent settings and credentials. A profile on run-scoped work limits the saved secrets available to its command. +A running automation can submit work for an external subject to +`POST /v1/runs/{run_id}/subject-turns` with a source, stable subject key, +prompt, and idempotency key. The service creates or resumes that subject's +deterministic conversation. The scanner never receives runtime credentials or +attaches to the conversation itself, and one short run can fan out several +independent conversations within the configured concurrency limit. + Only conversation-scoped execution reads the server's authoritative `conversation_runtime`. The service uses the same profile-backed conversation API in local and Docker workspaces, so automation code remains independent of diff --git a/migrations/versions/028_add_subject_turn_requests.py b/migrations/versions/028_add_subject_turn_requests.py new file mode 100644 index 00000000..aa1cdf00 --- /dev/null +++ b/migrations/versions/028_add_subject_turn_requests.py @@ -0,0 +1,54 @@ +"""Add idempotent subject-turn requests. + +Revision ID: 028 +Revises: 027 +""" + +import sqlalchemy as sa +from alembic import op + + +revision = "028" +down_revision = "027" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "automation_subject_turns", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("automation_id", sa.Uuid(), nullable=False), + sa.Column("requester_run_id", sa.Uuid(), nullable=False), + sa.Column("subject_run_id", sa.Uuid(), nullable=False), + sa.Column("source", sa.String(100), nullable=False), + sa.Column("subject_key", sa.String(500), nullable=False), + sa.Column("idempotency_key", sa.String(500), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("CURRENT_TIMESTAMP"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["automation_id"], ["automations.id"], ondelete="CASCADE" + ), + sa.ForeignKeyConstraint( + ["requester_run_id"], ["automation_runs.id"], ondelete="CASCADE" + ), + sa.ForeignKeyConstraint( + ["subject_run_id"], ["automation_runs.id"], ondelete="CASCADE" + ), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "automation_id", + "source", + "subject_key", + "idempotency_key", + name="uq_automation_subject_turn_idempotency", + ), + ) + + +def downgrade() -> None: + op.drop_table("automation_subject_turns") diff --git a/openhands/automation/app.py b/openhands/automation/app.py index a25c94cd..9546defc 100644 --- a/openhands/automation/app.py +++ b/openhands/automation/app.py @@ -32,6 +32,7 @@ from openhands.automation.router import router from openhands.automation.scheduler import scheduler_loop from openhands.automation.streams import stream_supervisor_loop +from openhands.automation.subject_router import router as subject_router from openhands.automation.telemetry_router import router as telemetry_router from openhands.automation.uploads import router as uploads_router from openhands.automation.utils.version import get_sdk_version, get_server_version_info @@ -295,6 +296,7 @@ def _create_app() -> FastAPI: app.include_router(webhook_router, prefix=_base_path) app.include_router(telemetry_router, prefix=_base_path) app.include_router(git_sync_router, prefix=_base_path) +app.include_router(subject_router, prefix=_base_path) app.include_router(kv_router, prefix=_base_path) app.include_router(router, prefix=_base_path) diff --git a/openhands/automation/conversations.py b/openhands/automation/conversations.py index 5c100a39..60768e7f 100644 --- a/openhands/automation/conversations.py +++ b/openhands/automation/conversations.py @@ -9,7 +9,7 @@ import logging import uuid from dataclasses import dataclass -from typing import Any, Final +from typing import Any, Final, Literal from sqlalchemy import select, text from sqlalchemy.ext.asyncio import AsyncSession @@ -20,7 +20,11 @@ FilterEvaluationError, evaluate_expression, ) -from openhands.automation.models import AutomationRun, AutomationRunStatus +from openhands.automation.models import ( + AutomationRun, + AutomationRunStatus, + AutomationSubjectTurn, +) from openhands.automation.schemas import EventTrigger from openhands.automation.subjects import conversation_id_for from openhands.automation.utils import utcnow @@ -28,6 +32,7 @@ compose_turn, send_conversation_turn, ) +from openhands.automation.utils.run import create_conversation_turn_run logger = logging.getLogger("automation.conversations") @@ -70,6 +75,15 @@ def needs_run(self) -> bool: return self.conversation_id is None +@dataclass(frozen=True, slots=True) +class SubjectTurnResult: + """Result returned to a poller that submitted work for one subject.""" + + disposition: Literal["created", "queued", "delivered", "deduplicated"] + run_id: uuid.UUID + conversation_id: str + + def resolve_subject_key( trigger: EventTrigger, payload: dict[str, Any], @@ -149,6 +163,14 @@ def _clean_key(value: str, origin: str) -> str | None: return key +def clean_subject_key(value: str) -> str: + """Validate a caller-supplied subject key without changing its identity.""" + key = _clean_key(value, "subject turn") + if key is None: + raise ValueError("subject_key must be 1 to 500 non-whitespace characters") + return key + + async def _take_subject_lock( session: AsyncSession, automation_id: uuid.UUID, @@ -262,6 +284,32 @@ async def continue_conversation( mid-run continues that conversation instead of racing a second run. """ await _take_subject_lock(session, automation_id, source, subject_key) + return await _continue_conversation_locked( + session, + org_id=org_id, + source=source, + subject_key=subject_key, + automation_id=automation_id, + event_key=event_key, + event_payload=event_payload, + turn_text=turn_text, + wake_agent=wake_agent, + ) + + +async def _continue_conversation_locked( + session: AsyncSession, + *, + org_id: uuid.UUID, + source: str, + subject_key: str, + automation_id: uuid.UUID, + event_key: str, + event_payload: dict[str, Any] | None, + turn_text: str | None, + wake_agent: bool, +) -> ContinueResult: + """Continue one subject after the caller has acquired its transaction lock.""" run = await _lock_subject_run(session, automation_id, source, subject_key) if run is None: @@ -294,3 +342,100 @@ async def continue_conversation( return ContinueResult() return ContinueResult(conversation_id=conversation_id) + + +async def submit_subject_turn( + session: AsyncSession, + *, + requester: AutomationRun, + source: str, + subject_key: str, + turn: str, + idempotency_key: str, + wake_agent: bool, +) -> SubjectTurnResult: + """Create or continue conversation work selected by a running automation. + + The requester chooses an external identity and prompt. The service owns + conversation identity, profile selection, runtime attachment, and + serialization. The transaction-scoped subject lock also orders duplicate + idempotency checks, so one retry cannot enqueue two conversations. + """ + subject_key = clean_subject_key(subject_key) + await _take_subject_lock(session, requester.automation_id, source, subject_key) + + duplicate = ( + ( + await session.execute( + select(AutomationSubjectTurn).where( + AutomationSubjectTurn.automation_id == requester.automation_id, + AutomationSubjectTurn.source == source, + AutomationSubjectTurn.subject_key == subject_key, + AutomationSubjectTurn.idempotency_key == idempotency_key, + ) + ) + ) + .scalars() + .first() + ) + if duplicate is not None: + duplicate_run = await session.get(AutomationRun, duplicate.subject_run_id) + if duplicate_run is None or duplicate_run.conversation_id is None: + raise RuntimeError("Subject-turn idempotency record has no conversation") + return SubjectTurnResult( + disposition="deduplicated", + run_id=duplicate_run.id, + conversation_id=duplicate_run.conversation_id, + ) + + outcome = await _continue_conversation_locked( + session, + org_id=requester.automation.org_id, + source=source, + subject_key=subject_key, + automation_id=requester.automation_id, + event_key="subject.turn", + event_payload=None, + turn_text=turn, + wake_agent=wake_agent, + ) + + subject_run = await _lock_subject_run( + session, requester.automation_id, source, subject_key + ) + if outcome.needs_run: + if subject_run is not None and subject_run.status not in _FINISHED: + raise RuntimeError("The subject conversation is not reachable yet") + subject_run = create_conversation_turn_run( + requester, + source=source, + subject_key=subject_key, + turn=turn, + wake_agent=wake_agent, + ) + assert subject_run.conversation_id is not None + conversation_id = subject_run.conversation_id + session.add(subject_run) + disposition = "created" + else: + assert subject_run is not None + conversation_id = outcome.conversation_id + assert conversation_id is not None + disposition = "queued" if outcome.coalesced else "delivered" + + session.add( + AutomationSubjectTurn( + automation_id=requester.automation_id, + requester_run_id=requester.id, + subject_run_id=subject_run.id, + source=source, + subject_key=subject_key, + idempotency_key=idempotency_key, + ) + ) + await session.flush() + return SubjectTurnResult( + disposition=disposition, + run_id=subject_run.id, + conversation_id=conversation_id, + ) diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index 961212b7..691887a6 100644 --- a/openhands/automation/dispatcher.py +++ b/openhands/automation/dispatcher.py @@ -63,6 +63,12 @@ make_run_status_detail, run_status_detail_from_exception, ) +from openhands.automation.utils.run_token import ( + SUBMIT_SUBJECT_TURN, + RunTokenError, + create_run_token, + signing_secret, +) from openhands.automation.utils.tarball_validation import ( is_http_url, parse_internal_upload_id, @@ -474,6 +480,21 @@ async def _fail( env_vars["SANDBOX_ID"] = ctx.sandbox_id env_vars["SESSION_API_KEY"] = ctx.session_key + try: + run_secret = signing_secret(settings) + except RunTokenError: + pass + else: + env_vars["AUTOMATION_RUN_TOKEN"] = create_run_token( + secret=run_secret, + automation_id=automation.id, + run_id=run.id, + scopes=(SUBMIT_SUBJECT_TURN,), + ) + env_vars["AUTOMATION_SUBJECT_TURN_URL"] = ( + f"{settings.resolved_base_url.rstrip('/')}/v1/runs/{run_id}/subject-turns" + ) + # Inject a KV token whenever the service has a KV secret configured. # The KV store is always available to automations — there is no per- # automation toggle. If no secret is configured the feature is simply diff --git a/openhands/automation/models.py b/openhands/automation/models.py index b1a91426..3c5bcb03 100644 --- a/openhands/automation/models.py +++ b/openhands/automation/models.py @@ -17,6 +17,7 @@ Integer, String, Text, + UniqueConstraint, Uuid, text, ) @@ -321,6 +322,47 @@ class AutomationRun(Base): ) +class AutomationSubjectTurn(Base): + """Idempotency record for a programmatic subject turn.""" + + __tablename__ = "automation_subject_turns" + + id: Mapped[uuid.UUID] = mapped_column(Uuid, primary_key=True, default=uuid.uuid4) + automation_id: Mapped[uuid.UUID] = mapped_column( + Uuid, + ForeignKey("automations.id", ondelete="CASCADE"), + nullable=False, + ) + requester_run_id: Mapped[uuid.UUID] = mapped_column( + Uuid, + ForeignKey("automation_runs.id", ondelete="CASCADE"), + nullable=False, + ) + subject_run_id: Mapped[uuid.UUID] = mapped_column( + Uuid, + ForeignKey("automation_runs.id", ondelete="CASCADE"), + nullable=False, + ) + source: Mapped[str] = mapped_column(String(100), nullable=False) + subject_key: Mapped[str] = mapped_column(String(500), nullable=False) + idempotency_key: Mapped[str] = mapped_column(String(500), nullable=False) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + server_default=text("CURRENT_TIMESTAMP"), + nullable=False, + ) + + __table_args__ = ( + UniqueConstraint( + "automation_id", + "source", + "subject_key", + "idempotency_key", + name="uq_automation_subject_turn_idempotency", + ), + ) + + class AutomationDisableEvent(Base): """Historical record of an automation being disabled.""" diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index beb2462a..d4391eaf 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -966,6 +966,34 @@ def normalize_phase(cls, v: Any) -> Any: return " ".join(_PHASE_CONTROL_CHARS_RE.sub(" ", v).split()) +class SubjectTurnRequest(BaseModel): + """Conversation work selected by a running automation for an external subject.""" + + model_config = ConfigDict(extra="forbid") + + source: str = Field(..., min_length=1, max_length=100) + subject_key: str = Field(..., min_length=1, max_length=500) + turn: str = Field(..., min_length=1, max_length=50000) + idempotency_key: str = Field(..., min_length=1, max_length=500) + wake_agent: bool = True + + @field_validator("source", "subject_key", "turn", "idempotency_key") + @classmethod + def reject_whitespace_only(cls, value: str) -> str: + value = value.strip() + if not value: + raise ValueError("must contain non-whitespace characters") + return value + + +class SubjectTurnResponse(BaseModel): + """Accepted routing decision for a programmatic subject turn.""" + + disposition: Literal["created", "queued", "delivered", "deduplicated"] + run_id: uuid.UUID + conversation_id: uuid.UUID + + class AutomationRunResponse(BaseModel): """Response for a single automation run.""" diff --git a/openhands/automation/subject_router.py b/openhands/automation/subject_router.py new file mode 100644 index 00000000..b4b18d55 --- /dev/null +++ b/openhands/automation/subject_router.py @@ -0,0 +1,93 @@ +"""Scoped API for work selected by a running automation.""" + +import uuid +from typing import Annotated + +from fastapi import APIRouter, Depends, Header, HTTPException, status +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession +from sqlalchemy.orm import selectinload + +from openhands.automation.config import get_config +from openhands.automation.conversations import submit_subject_turn +from openhands.automation.db import get_session +from openhands.automation.models import AutomationRun, AutomationRunStatus +from openhands.automation.schemas import SubjectTurnRequest, SubjectTurnResponse +from openhands.automation.utils.run_token import ( + SUBMIT_SUBJECT_TURN, + RunTokenError, + signing_secret, + verify_run_token, +) + + +router = APIRouter(prefix="/v1/runs", tags=["Automation subject turns"]) + + +@router.post( + "/{run_id}/subject-turns", + response_model=SubjectTurnResponse, + status_code=status.HTTP_202_ACCEPTED, +) +async def create_subject_turn( + run_id: uuid.UUID, + body: SubjectTurnRequest, + authorization: Annotated[str | None, Header()] = None, + session: AsyncSession = Depends(get_session), +) -> SubjectTurnResponse: + """Create or continue a subject conversation for the caller's automation.""" + if not authorization or not authorization.startswith("Bearer "): + raise HTTPException(status.HTTP_401_UNAUTHORIZED, "Run token required") + try: + claims = verify_run_token( + signing_secret(get_config().service), + authorization.removeprefix("Bearer ").strip(), + SUBMIT_SUBJECT_TURN, + ) + except RunTokenError as exc: + raise HTTPException(status.HTTP_401_UNAUTHORIZED, str(exc)) from exc + if claims.run_id != run_id: + raise HTTPException(status.HTTP_403_FORBIDDEN, "Token belongs to another run") + + requester = ( + ( + await session.execute( + select(AutomationRun) + .where(AutomationRun.id == run_id) + .options(selectinload(AutomationRun.automation)) + ) + ) + .scalars() + .first() + ) + if requester is None: + raise HTTPException(status.HTTP_404_NOT_FOUND, "Run not found") + if requester.automation_id != claims.automation_id: + raise HTTPException( + status.HTTP_403_FORBIDDEN, "Token belongs to another automation" + ) + if requester.status != AutomationRunStatus.RUNNING: + raise HTTPException( + status.HTTP_409_CONFLICT, "Only a running automation can submit work" + ) + + try: + result = await submit_subject_turn( + session, + requester=requester, + source=body.source, + subject_key=body.subject_key, + turn=body.turn, + idempotency_key=body.idempotency_key, + wake_agent=body.wake_agent, + ) + except ValueError as exc: + raise HTTPException(status.HTTP_422_UNPROCESSABLE_ENTITY, str(exc)) from exc + except RuntimeError as exc: + raise HTTPException(status.HTTP_503_SERVICE_UNAVAILABLE, str(exc)) from exc + await session.commit() + return SubjectTurnResponse( + disposition=result.disposition, + run_id=result.run_id, + conversation_id=uuid.UUID(result.conversation_id), + ) diff --git a/openhands/automation/utils/run_token.py b/openhands/automation/utils/run_token.py new file mode 100644 index 00000000..10c22964 --- /dev/null +++ b/openhands/automation/utils/run_token.py @@ -0,0 +1,73 @@ +"""Short-lived credentials for capabilities granted to one automation run.""" + +import uuid +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from typing import Final + +import jwt + +from openhands.automation.config import ServiceSettings + + +SUBMIT_SUBJECT_TURN: Final[str] = "subject_turn:submit" +RUN_TOKEN_EXPIRATION_HOURS: Final[int] = 24 + + +class RunTokenError(Exception): + """A run token is absent, expired, malformed, or has the wrong scope.""" + + +@dataclass(frozen=True, slots=True) +class RunTokenClaims: + automation_id: uuid.UUID + run_id: uuid.UUID + scopes: frozenset[str] + + +def signing_secret(settings: ServiceSettings) -> str: + """Return the deployment secret used only to sign scoped run tokens.""" + secret = settings.service_key or settings.local_api_key + if not secret: + raise RunTokenError( + "AUTOMATION_SERVICE_KEY or AUTOMATION_LOCAL_API_KEY is required" + ) + return secret + + +def create_run_token( + *, + secret: str, + automation_id: uuid.UUID, + run_id: uuid.UUID, + scopes: tuple[str, ...], +) -> str: + now = datetime.now(UTC) + return jwt.encode( + { + "automation_id": str(automation_id), + "run_id": str(run_id), + "scopes": list(scopes), + "iat": now, + "exp": now + timedelta(hours=RUN_TOKEN_EXPIRATION_HOURS), + }, + secret, + algorithm="HS256", + ) + + +def verify_run_token(secret: str, token: str, required_scope: str) -> RunTokenClaims: + try: + payload = jwt.decode(token, secret, algorithms=["HS256"]) + scopes = frozenset(payload.get("scopes") or ()) + if required_scope not in scopes: + raise RunTokenError("Token does not grant the required scope") + return RunTokenClaims( + automation_id=uuid.UUID(payload["automation_id"]), + run_id=uuid.UUID(payload["run_id"]), + scopes=scopes, + ) + except RunTokenError: + raise + except (KeyError, TypeError, ValueError, jwt.PyJWTError) as exc: + raise RunTokenError("Invalid or expired run token") from exc diff --git a/tests/test_db.py b/tests/test_db.py index e47f6d60..57935038 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -245,6 +245,7 @@ def test_migrations_run_on_sqlite(self, monkeypatch): # Verify all expected tables exist assert "automations" in tables assert "automation_runs" in tables + assert "automation_subject_turns" in tables assert "tarball_uploads" in tables assert "custom_webhooks" in tables assert "alembic_version" in tables diff --git a/tests/test_subject_turns.py b/tests/test_subject_turns.py new file mode 100644 index 00000000..cdc56e29 --- /dev/null +++ b/tests/test_subject_turns.py @@ -0,0 +1,227 @@ +"""Programmatic subject work stays scoped, idempotent, and service-owned.""" + +import uuid +from unittest.mock import AsyncMock, patch + +import pytest +from sqlalchemy import func, select + +from openhands.automation.conversations import submit_subject_turn +from openhands.automation.models import ( + Automation, + AutomationRun, + AutomationRunStatus, + AutomationSubjectTurn, +) +from openhands.automation.subjects import conversation_id_for +from openhands.automation.utils.run_token import ( + SUBMIT_SUBJECT_TURN, + create_run_token, +) + + +async def _requester(session, user, *, profile: bool = True) -> AutomationRun: + automation = Automation( + user_id=user.user_id, + org_id=user.org_id, + name="GitHub selector", + trigger={"type": "cron", "schedule": "*/5 * * * *", "timezone": "UTC"}, + tarball_path="https://example.com/selector.tar.gz", + entrypoint="python3 worker.py", + agent_profile_id=uuid.uuid4() if profile else None, + ) + session.add(automation) + await session.flush() + run = AutomationRun( + automation=automation, + agent_profile_id=automation.agent_profile_id, + status=AutomationRunStatus.RUNNING, + ) + session.add(run) + await session.commit() + return run + + +@pytest.mark.asyncio +async def test_first_turn_creates_profile_backed_subject_run( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + + result = await submit_subject_turn( + async_session, + requester=requester, + source="github", + subject_key="repository-42/issue-7", + turn="Implement issue 7", + idempotency_key="issue-7-ready-v1", + wake_agent=True, + ) + await async_session.commit() + + child = await async_session.get(AutomationRun, result.run_id) + assert result.disposition == "created" + assert child is not None + assert child.agent_profile_id == requester.agent_profile_id + assert child.execution_scope == "conversation" + assert child.conversation_turn == "Implement issue 7" + assert child.conversation_id == conversation_id_for( + requester.automation.org_id, + requester.automation_id, + "github", + "repository-42/issue-7", + ) + + +@pytest.mark.asyncio +async def test_same_idempotency_key_returns_the_original_run( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + request = { + "requester": requester, + "source": "github", + "subject_key": "repository-42/pr-8", + "turn": "Review PR 8", + "idempotency_key": "head-deadbeef", + "wake_agent": True, + } + first = await submit_subject_turn(async_session, **request) + await async_session.commit() + second = await submit_subject_turn(async_session, **request) + await async_session.commit() + + count = await async_session.scalar(select(func.count(AutomationSubjectTurn.id))) + assert second.disposition == "deduplicated" + assert second.run_id == first.run_id + assert count == 1 + + +@pytest.mark.asyncio +async def test_source_is_part_of_subject_identity( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + first = await submit_subject_turn( + async_session, + requester=requester, + source="github", + subject_key="42", + turn="GitHub work", + idempotency_key="one", + wake_agent=True, + ) + await async_session.commit() + second = await submit_subject_turn( + async_session, + requester=requester, + source="linear", + subject_key="42", + turn="Linear work", + idempotency_key="one", + wake_agent=True, + ) + await async_session.commit() + + assert first.run_id != second.run_id + assert first.conversation_id != second.conversation_id + + +@pytest.mark.asyncio +async def test_endpoint_rejects_a_token_for_another_run( + async_client, async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + token = create_run_token( + secret="test-secret", + automation_id=requester.automation_id, + run_id=uuid.uuid4(), + scopes=(SUBMIT_SUBJECT_TURN,), + ) + with patch( + "openhands.automation.subject_router.signing_secret", + return_value="test-secret", + ): + response = await async_client.post( + f"/api/automation/v1/runs/{requester.id}/subject-turns", + headers={"Authorization": f"Bearer {token}"}, + json={ + "source": "github", + "subject_key": "repo/issue-1", + "turn": "Implement it", + "idempotency_key": "ready-v1", + }, + ) + + assert response.status_code == 403 + + +@pytest.mark.asyncio +async def test_endpoint_accepts_only_its_running_automation( + async_client, async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + token = create_run_token( + secret="test-secret", + automation_id=requester.automation_id, + run_id=requester.id, + scopes=(SUBMIT_SUBJECT_TURN,), + ) + with patch( + "openhands.automation.subject_router.signing_secret", + return_value="test-secret", + ): + response = await async_client.post( + f"/api/automation/v1/runs/{requester.id}/subject-turns", + headers={"Authorization": f"Bearer {token}"}, + json={ + "source": "github", + "subject_key": "repo/issue-1", + "turn": "Implement it", + "idempotency_key": "ready-v1", + }, + ) + + assert response.status_code == 202 + assert response.json()["disposition"] == "created" + + +@pytest.mark.asyncio +async def test_existing_subject_is_continued_without_a_second_run( + async_session, mock_authenticated_user, monkeypatch +): + requester = await _requester(async_session, mock_authenticated_user) + first = await submit_subject_turn( + async_session, + requester=requester, + source="github", + subject_key="repository-42/pr-8", + turn="Review head one", + idempotency_key="head-one", + wake_agent=True, + ) + await async_session.commit() + child = await async_session.get(AutomationRun, first.run_id) + assert child is not None + child.status = AutomationRunStatus.COMPLETED + child.started_at = requester.created_at + await async_session.commit() + delivered = AsyncMock(return_value=True) + monkeypatch.setattr( + "openhands.automation.conversations.send_conversation_turn", delivered + ) + + second = await submit_subject_turn( + async_session, + requester=requester, + source="github", + subject_key="repository-42/pr-8", + turn="Review head two", + idempotency_key="head-two", + wake_agent=True, + ) + await async_session.commit() + + assert second.disposition == "delivered" + assert second.run_id == first.run_id + delivered.assert_awaited_once() From d14aef4157923174c477d1220bb019ccab0a9677 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 15 Sep 2026 00:51:41 +0000 Subject: [PATCH 6/7] fix: retry unsuccessful subject turns Co-authored-by: openhands --- openhands/automation/conversations.py | 36 ++++++++--- openhands/automation/dispatcher.py | 34 ++++++++--- tests/test_dispatcher.py | 48 +++++++++++++++ tests/test_subject_turns.py | 88 +++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 15 deletions(-) diff --git a/openhands/automation/conversations.py b/openhands/automation/conversations.py index 60768e7f..12d11b73 100644 --- a/openhands/automation/conversations.py +++ b/openhands/automation/conversations.py @@ -46,6 +46,12 @@ AutomationRunStatus.SKIPPED, ) +_RETRYABLE = ( + AutomationRunStatus.FAILED, + AutomationRunStatus.CANCELLED, + AutomationRunStatus.SKIPPED, +) + # Matches AutomationRun.subject_key. Truncating would merge two subjects. MAX_SUBJECT_KEY_LENGTH: Final[int] = 500 @@ -378,15 +384,26 @@ async def submit_subject_turn( .scalars() .first() ) + retry_record: AutomationSubjectTurn | None = None if duplicate is not None: duplicate_run = await session.get(AutomationRun, duplicate.subject_run_id) if duplicate_run is None or duplicate_run.conversation_id is None: raise RuntimeError("Subject-turn idempotency record has no conversation") - return SubjectTurnResult( - disposition="deduplicated", - run_id=duplicate_run.id, - conversation_id=duplicate_run.conversation_id, + can_retry = duplicate_run.status in _RETRYABLE and ( + duplicate_run.started_at is None + or duplicate_run.subject_released_at is not None ) + if not can_retry: + return SubjectTurnResult( + disposition="deduplicated", + run_id=duplicate_run.id, + conversation_id=duplicate_run.conversation_id, + ) + # A run canceled before dispatch never owned a runtime, but excluding it + # from the subject lookup still requires the normal released marker. + if duplicate_run.started_at is None: + duplicate_run.subject_released_at = utcnow() + retry_record = duplicate outcome = await _continue_conversation_locked( session, @@ -423,8 +440,8 @@ async def submit_subject_turn( assert conversation_id is not None disposition = "queued" if outcome.coalesced else "delivered" - session.add( - AutomationSubjectTurn( + if retry_record is None: + retry_record = AutomationSubjectTurn( automation_id=requester.automation_id, requester_run_id=requester.id, subject_run_id=subject_run.id, @@ -432,7 +449,12 @@ async def submit_subject_turn( subject_key=subject_key, idempotency_key=idempotency_key, ) - ) + session.add(retry_record) + else: + # Keep the unique idempotency record and point it at the new attempt. + # The superseded run remains in run history for diagnosis. + retry_record.requester_run_id = requester.id + retry_record.subject_run_id = subject_run.id await session.flush() return SubjectTurnResult( disposition=disposition, diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index 691887a6..48484853 100644 --- a/openhands/automation/dispatcher.py +++ b/openhands/automation/dispatcher.py @@ -317,6 +317,18 @@ async def _fail( }, ) + async def _release_conversation_subject() -> None: + """Release a child subject after its runtime is absent or released.""" + assert run.conversation_turn is not None + run.subject_released_at = utcnow() + async with session_factory() as release_session: + await release_session.execute( + update(AutomationRun) + .where(AutomationRun.id == run.id) + .values(subject_released_at=run.subject_released_at) + ) + await release_session.commit() + # 1. Calculate effective timeout (doesn't depend on ctx). This same value # drives both the bash command timeout and the watchdog cleanup deadline. effective_timeout = resolve_automation_timeout_seconds(automation.timeout) @@ -339,6 +351,8 @@ async def _fail( source="sandbox_api", operation="get_execution_context", ) + if run.conversation_turn is not None: + await _release_conversation_subject() await mark_run_terminal( session_factory, run, @@ -359,6 +373,8 @@ async def _fail( except Exception as exc: logger.exception("Failed to get execution context", extra=_log_ctx()) source = "sandbox_api" if backend.provisions_agent_server else "agent_server" + if run.conversation_turn is not None: + await _release_conversation_subject() await _fail( "Failed to get execution context", status_detail=run_status_detail_from_exception( @@ -403,15 +419,17 @@ async def _fail( timeout=effective_timeout, ) except Exception as exc: - run.subject_released_at = utcnow() - async with session_factory() as release_session: - await release_session.execute( - update(AutomationRun) - .where(AutomationRun.id == run.id) - .values(subject_released_at=run.subject_released_at) + runtime_released = False + try: + await backend.release_context(client, ctx) + runtime_released = True + except Exception: + logger.exception( + "Failed to release failed subject runtime", + extra=_log_ctx(sandbox_id=ctx.sandbox_id), ) - await release_session.commit() - await backend.release_context(client, ctx) + if runtime_released: + await _release_conversation_subject() await _fail( "Subject conversation failed", status_detail=run_status_detail_from_exception( diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index e7450c2e..567826ba 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -247,6 +247,54 @@ async def test_conversation_turn_run_skips_bundle_and_releases_runtime( finished = await session.get(AutomationRun, run_id) assert finished is not None assert finished.status == AutomationRunStatus.COMPLETED + assert finished.subject_released_at is None + + +@pytest.mark.asyncio +async def test_conversation_turn_context_failure_releases_subject( + async_session_factory, mock_settings, mock_client +): + async with async_session_factory() as session: + automation = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Reviewer scanner", + trigger={"type": "cron", "schedule": "* * * * *", "timezone": "UTC"}, + tarball_path="https://example.com/scanner.tar.gz", + entrypoint="python scanner.py", + ) + requester = AutomationRun( + automation=automation, + status=AutomationRunStatus.RUNNING, + agent_profile_id=uuid.uuid4(), + ) + session.add(requester) + await session.flush() + run = create_conversation_turn_run( + requester, + source="github", + subject_key="repository-42/pr-9", + turn="Review PR 9", + wake_agent=True, + ) + run.status = AutomationRunStatus.RUNNING + run.started_at = utcnow() + session.add(run) + await session.commit() + run_id = run.id + await session.refresh(run, attribute_names=["automation"]) + + backend = MagicMock() + backend.provisions_agent_server = False + backend.get_execution_context = AsyncMock(side_effect=RuntimeError("unavailable")) + with patch("openhands.automation.dispatcher.get_backend", return_value=backend): + await _execute_run(run, mock_settings, async_session_factory, mock_client) + + async with async_session_factory() as session: + finished = await session.get(AutomationRun, run_id) + assert finished is not None + assert finished.status == AutomationRunStatus.FAILED + assert finished.subject_released_at is not None class TestMarkRunStatus: diff --git a/tests/test_subject_turns.py b/tests/test_subject_turns.py index cdc56e29..4194b390 100644 --- a/tests/test_subject_turns.py +++ b/tests/test_subject_turns.py @@ -14,6 +14,7 @@ AutomationSubjectTurn, ) from openhands.automation.subjects import conversation_id_for +from openhands.automation.utils import utcnow from openhands.automation.utils.run_token import ( SUBMIT_SUBJECT_TURN, create_run_token, @@ -42,6 +43,17 @@ async def _requester(session, user, *, profile: bool = True) -> AutomationRun: return run +def _turn_request(requester: AutomationRun) -> dict: + return { + "requester": requester, + "source": "github", + "subject_key": "repository-42/issue-7", + "turn": "Implement issue 7", + "idempotency_key": "issue-7-ready-v1", + "wake_agent": True, + } + + @pytest.mark.asyncio async def test_first_turn_creates_profile_backed_subject_run( async_session, mock_authenticated_user @@ -97,6 +109,82 @@ async def test_same_idempotency_key_returns_the_original_run( assert count == 1 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "failed_status", + [ + AutomationRunStatus.FAILED, + AutomationRunStatus.CANCELLED, + AutomationRunStatus.SKIPPED, + ], +) +async def test_same_idempotency_key_retries_an_unsuccessful_released_run( + async_session, mock_authenticated_user, failed_status +): + requester = await _requester(async_session, mock_authenticated_user) + request = _turn_request(requester) + first = await submit_subject_turn(async_session, **request) + await async_session.commit() + failed = await async_session.get(AutomationRun, first.run_id) + assert failed is not None + failed.status = failed_status + failed.subject_released_at = utcnow() + await async_session.commit() + + second = await submit_subject_turn(async_session, **request) + await async_session.commit() + + record = (await async_session.execute(select(AutomationSubjectTurn))).scalar_one() + assert second.disposition == "created" + assert second.run_id != first.run_id + assert second.conversation_id == first.conversation_id + assert record.subject_run_id == second.run_id + + +@pytest.mark.asyncio +async def test_same_idempotency_key_retries_a_skipped_run_that_never_started( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + request = _turn_request(requester) + first = await submit_subject_turn(async_session, **request) + await async_session.commit() + skipped = await async_session.get(AutomationRun, first.run_id) + assert skipped is not None + skipped.status = AutomationRunStatus.SKIPPED + skipped.started_at = None + await async_session.commit() + + second = await submit_subject_turn(async_session, **request) + await async_session.commit() + + assert second.disposition == "created" + assert second.run_id != first.run_id + assert second.conversation_id == first.conversation_id + assert skipped.subject_released_at is not None + + +@pytest.mark.asyncio +async def test_same_idempotency_key_waits_for_failed_run_release( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + request = _turn_request(requester) + first = await submit_subject_turn(async_session, **request) + await async_session.commit() + failed = await async_session.get(AutomationRun, first.run_id) + assert failed is not None + failed.status = AutomationRunStatus.FAILED + failed.started_at = utcnow() + await async_session.commit() + + second = await submit_subject_turn(async_session, **request) + await async_session.commit() + + assert second.disposition == "deduplicated" + assert second.run_id == first.run_id + + @pytest.mark.asyncio async def test_source_is_part_of_subject_identity( async_session, mock_authenticated_user From a95124ede12ce089a564d710ab734335376ba45e Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 15 Sep 2026 01:06:28 +0000 Subject: [PATCH 7/7] test: cover queued subject turns Co-authored-by: openhands --- tests/test_subject_turns.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_subject_turns.py b/tests/test_subject_turns.py index 4194b390..b06ae0e5 100644 --- a/tests/test_subject_turns.py +++ b/tests/test_subject_turns.py @@ -109,6 +109,33 @@ async def test_same_idempotency_key_returns_the_original_run( assert count == 1 +@pytest.mark.asyncio +async def test_new_turn_is_queued_on_the_subject_run_before_dispatch( + async_session, mock_authenticated_user +): + requester = await _requester(async_session, mock_authenticated_user) + first = await submit_subject_turn(async_session, **_turn_request(requester)) + await async_session.commit() + + second = await submit_subject_turn( + async_session, + **{ + **_turn_request(requester), + "turn": "Acceptance criteria changed", + "idempotency_key": "issue-7-ready-v2", + }, + ) + await async_session.commit() + + assert second.disposition == "queued" + assert second.run_id == first.run_id + run = await async_session.get(AutomationRun, first.run_id) + assert run is not None + assert run.event_payload == { + "_automation_follow_up_turns": ["Acceptance criteria changed"] + } + + @pytest.mark.asyncio @pytest.mark.parametrize( "failed_status",