diff --git a/README.md b/README.md index 52c4404d..f86a04e7 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,56 @@ The Automation Service owns automation definitions, cron scheduling, webhooks, r ## Development +### Run and conversation execution + +Set `AUTOMATION_AGENT_SERVER_URL` and `AUTOMATION_AGENT_SERVER_API_KEY`, then +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. + +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 +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. + +The Agent Server resolves each selected profile at dispatch, including its +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 +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+ @@ -101,3 +151,30 @@ 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. + +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", + "execution_scope": "run" +} +``` + +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` +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 +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/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/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/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/backends/__init__.py b/openhands/automation/backends/__init__.py index d0e23d08..68f9d376 100644 --- a/openhands/automation/backends/__init__.py +++ b/openhands/automation/backends/__init__.py @@ -1,19 +1,8 @@ -"""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 @@ -21,48 +10,67 @@ from typing import TYPE_CHECKING 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 + 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 profile_id is None: + raise ValueError("Conversation execution requires an agent profile") + return ConversationBackend( + _existing_agent_server(run), + agent_profile_id=profile_id, ) - 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..2ac5964e --- /dev/null +++ b/openhands/automation/backends/conversation.py @@ -0,0 +1,197 @@ +"""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: + 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: + 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, + plugins=(self.provider.run.automation.preset_metadata or {}).get( + "plugins" + ), + 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), + "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, + } + + 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/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 6e7acfb6..a9a661ca 100644 --- a/openhands/automation/config.py +++ b/openhands/automation/config.py @@ -581,8 +581,11 @@ class ServiceSettings(BaseSettings): # - Authenticates using local_api_key instead of OpenHands SaaS API agent_server_url: str = "" agent_server_api_key: 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 - # 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/conversations.py b/openhands/automation/conversations.py index a07ee192..12d11b73 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") @@ -41,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 @@ -70,6 +81,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,9 +169,18 @@ 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, + source: str, subject_key: str, ) -> None: """Serialise every event for one subject, including those finding no run. @@ -170,7 +199,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 +211,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 +229,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 +289,41 @@ 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) + 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, 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: @@ -289,3 +348,116 @@ 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() + ) + 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") + 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, + 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" + + if retry_record is None: + retry_record = 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, + ) + 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, + run_id=subject_run.id, + conversation_id=conversation_id, + ) diff --git a/openhands/automation/dispatcher.py b/openhands/automation/dispatcher.py index e5b6cef6..48484853 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 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, @@ -62,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, @@ -131,6 +138,30 @@ 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: list[tuple[uuid.UUID, uuid.UUID, str]] = [] + conversation_capacity: int | None = None + if settings.is_local_mode: + active = list( + ( + await session.execute( + select( + AutomationRun.id, + AutomationRun.automation_id, + AutomationRun.execution_scope, + ).where(AutomationRun.status == AutomationRunStatus.RUNNING) + ) + ) + .tuples() + .all() + ) + active_conversations = [row for row in active if row[2] == "conversation"] + conversation_capacity = settings.conversation_max_concurrent_runs - len( + active_conversations + ) + if conversation_capacity > 0: + batch_size = min(batch_size, conversation_capacity) + select_query = ( select(AutomationRun) .join(AutomationRun.automation) @@ -143,6 +174,27 @@ 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" + ) + 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(): @@ -207,8 +259,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 @@ -264,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) @@ -286,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, @@ -305,7 +372,9 @@ 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" + 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( @@ -323,13 +392,87 @@ 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: + 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), + ) + if runtime_released: + await _release_conversation_subject() + 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() - 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) @@ -355,6 +498,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 @@ -435,7 +593,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 +602,10 @@ async def _fail( timeout=effective_timeout, 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( @@ -473,7 +634,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( @@ -488,8 +649,6 @@ async def _fail( # 6. Handle result if result.success: 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 @@ -511,7 +670,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 +687,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..627e441b 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,57 @@ 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, + agent_profile_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, agent_profile_id=agent_profile_id + ) def _is_permanent_http_error(stderr: str) -> bool: @@ -252,13 +252,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 +292,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 +335,6 @@ class DispatchResult: async def execute_in_context( - client: httpx.AsyncClient, agent_url: str, session_key: str, entrypoint: str, @@ -341,6 +344,8 @@ 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, + agent_profile_id: UUID | None = None, ) -> DispatchResult: """Execute automation code in an existing execution context. @@ -383,28 +388,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 +436,12 @@ 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, + agent_profile_id=agent_profile_id, ) env_path = None logger.info( @@ -444,10 +466,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 +575,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 +603,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..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, @@ -501,13 +503,27 @@ 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'") + 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 ) return { "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 6e035202..d5b74a9c 100644 --- a/openhands/automation/git_sync/serializer.py +++ b/openhands/automation/git_sync/serializer.py @@ -133,7 +133,11 @@ def _automation_yaml_fields( ) -> dict[str, Any]: fields: dict[str, Any] = { "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 1d250491..3c5bcb03 100644 --- a/openhands/automation/models.py +++ b/openhands/automation/models.py @@ -8,6 +8,7 @@ from sqlalchemy import ( JSON, BigInteger, + Boolean, DateTime, Enum, Float, @@ -16,6 +17,7 @@ Integer, String, Text, + UniqueConstraint, Uuid, text, ) @@ -48,6 +50,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 +82,13 @@ 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 + ) + # 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 +184,14 @@ 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 + ) + # Error details if status is FAILED error_detail: Mapped[str | None] = mapped_column(Text, nullable=True) @@ -204,6 +228,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. @@ -271,6 +304,7 @@ class AutomationRun(Base): Index( "ix_automation_runs_subject", "automation_id", + "subject_source", "subject_key", "created_at", postgresql_where=(subject_key.isnot(None)) @@ -288,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/preset_router.py b/openhands/automation/preset_router.py index bb66d049..461bc2cb 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,8 @@ async def create_automation_from_prompt( prompt=body.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", @@ -628,6 +663,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 +928,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 +1024,8 @@ async def create_automation_from_plugin( prompt=body.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", 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 2cce3e7a..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: @@ -164,8 +172,10 @@ 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, + agent_profile_id=body.agent_profile_id, preset_metadata=preset_metadata, trigger=body.trigger.model_dump(), tarball_path=body.tarball_path, @@ -311,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) @@ -323,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 @@ -903,6 +921,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..d4391eaf 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -423,6 +423,20 @@ 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." + ), + ) + 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." + ), + ) name: str = Field(..., min_length=1, max_length=500) model: str | None = Field( default=None, @@ -523,6 +537,14 @@ 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=( + "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) model: str | None = Field( default=None, @@ -862,6 +884,14 @@ class TelemetryConsentResponse(BaseModel): class AutomationResponse(BaseModel): + execution_scope: Literal["run", "conversation"] = "run" + 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 @@ -936,9 +966,45 @@ 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.""" + execution_scope: Literal["run", "conversation"] = "run" + 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/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/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..f2f44439 100644 --- a/openhands/automation/utils/conversation_turn.py +++ b/openhands/automation/utils/conversation_turn.py @@ -16,14 +16,21 @@ 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.base import ExecutionContext +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 +191,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 +234,74 @@ 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() + + +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, @@ -257,27 +332,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/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 7fe41731..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, @@ -181,6 +211,8 @@ 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, status=AutomationRunStatus.PENDING, 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/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..ba5088ae 100644 --- a/openhands/automation/utils/webhook.py +++ b/openhands/automation/utils/webhook.py @@ -260,12 +260,17 @@ 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, event_payload=event_payload, 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/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..6daba61c 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#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/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..43044849 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,12 @@ def mock_run(self): """Create a mock AutomationRun.""" 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): - """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 +485,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 +503,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 +578,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..98e55fbe 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,31 @@ 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 + + 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) + _, 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 new file mode 100644 index 00000000..4f84a8b2 --- /dev/null +++ b/tests/test_conversation_backend.py @@ -0,0 +1,247 @@ +"""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", + "AUTOMATION_AGENT_PROFILE_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( + ("profile", "execution_scope", "expected_type"), + [ + (False, "run", RunBackend), + (True, "run", RunBackend), + (True, "conversation", ConversationBackend), + ], +) +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 + + selected = uuid4() + monkeypatch.setenv("AUTOMATION_AGENT_SERVER_URL", "http://server") + clear_config_cache() + try: + backend = get_backend( + AutomationRun( + id=uuid4(), + 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 == selected + 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") + 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_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. + 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..2bcc9063 100644 --- a/tests/test_conversation_turn.py +++ b/tests/test_conversation_turn.py @@ -1,51 +1,121 @@ """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.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, ) -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) +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, + ) -def local_backend(agent_url="https://local-agent.example.com"): - class FakeBackend: - is_local_mode = True + 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) - async def get_execution_context(self, client): - return SimpleNamespace(agent_url=agent_url, session_key="local-key") - return FakeBackend() +@pytest.fixture +def conversation_transport(sdk_http_transport, monkeypatch): + 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 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 +130,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 +176,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 +212,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 +256,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 +274,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 +293,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 +323,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 +361,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 +369,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..2d405295 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: @@ -1080,6 +1098,7 @@ def client_factory(**kwargs): status=AutomationRunStatus.COMPLETED, started_at=utcnow(), sandbox_id="sbx-1", + subject_source="slack", subject_key=subject_key, ) ) @@ -1103,7 +1122,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", + ] # --------------------------------------------------------------------------- @@ -1274,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") @@ -1310,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 8df9f9cc..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 @@ -259,6 +260,17 @@ 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") + } + assert "agent_profile_id" in run_columns + 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_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..567826ba 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, ) @@ -27,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, @@ -73,6 +76,227 @@ 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 +): + """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() + + +@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 + 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: """Tests for mark_run_status function.""" @@ -871,7 +1095,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 +1121,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 +1148,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 +1168,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 +1192,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 +1231,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 +1481,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 +1532,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 +1612,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 +1641,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..2b0227d4 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -7,8 +7,10 @@ import io import subprocess import tarfile +import uuid from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest from openhands.automation.config import get_config @@ -160,59 +162,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 +201,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 +220,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", @@ -268,20 +239,22 @@ 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( - client=AsyncMock(), agent_url="https://agent.example.com", session_key="test-session-key", entrypoint="python main.py", 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") @@ -289,10 +262,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 +281,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 +369,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 +381,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 +404,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 +426,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 +445,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 +468,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 +514,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 +543,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 +551,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 +567,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 +575,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 +592,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 +600,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 +616,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 +624,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 +651,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 +659,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 +668,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 +684,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 +692,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_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_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_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): diff --git a/tests/test_subject_turns.py b/tests/test_subject_turns.py new file mode 100644 index 00000000..b06ae0e5 --- /dev/null +++ b/tests/test_subject_turns.py @@ -0,0 +1,342 @@ +"""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 import utcnow +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 + + +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 +): + 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_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", + [ + 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 +): + 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() 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..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", specifier = "==1.46.0" }, + { 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" }, @@ -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=91a259dcf62eb9ff688291bd01a355529488aaf3#91a259dcf62eb9ff688291bd01a355529488aaf3" } 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"