feat: select agent profiles on automation definitions

[neubig]

Why

Different automations need different model, tool, MCP, egress, and secret policy. A definition therefore needs to select a saved Agent Server profile. Profile selection is policy; it is independent of the execution scope and Agent Server provisioning interfaces introduced by #449.

Summary

• Persist agent_profile_id on definitions and queued-run snapshots.
• Carry the selected profile through CRUD, presets, Git sync, history, and capability discovery.
• Resolve profiles through the existing Agent Server profile store and SDK APIs; Automation adds no profile loader or secret map.
• Supply the selected profile to conversation-scoped execution. Run-scoped credential injection follows in feat: scope run commands to selected agent profiles #466.
• Reject conflicting model/profile settings and unsupported cloud-profile combinations.

The migration is revision 026, after #449's execution-scope revision 025.

Issue Number

Implements #273. Canvas selector: OpenHands/OpenHands#17396.

How to Test

• Full assembled stack: 1,805 passed, 7 skipped.
• Focused profile persistence, validation, preset, Git-sync, backend, conversation, and migration tests pass.
• Formatting, Ruff, pycodestyle, and Pyright pass.

Live Agent Canvas evidence

Animated queued-profile demonstration · scenario, revisions, and limits. It demonstrates selected-profile conversation creation and a queued run retaining its snapshotted profile in local and Docker workspaces.

Dependencies and review order

Native stack #454: #449 → #453. Merged software-agent-sdk #5010 supplies typed conversation creation and attachment. Replace the temporary SDK source pin with a released SDK before merge.

Profile-selected secret enforcement is completed by merged software-agent-sdk #4931 and open #5017; selecting a profile alone is not the secret boundary.

Known pre-existing limitation: profile-created preset conversations do not yet receive the preset FinishTool enforcement hook. #457 tracks that separate correction.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: c6ebc5a1d9cb99363a8be16d3cdff1ec4760783f
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/1f50d614-7942-4d4e-bc07-745aaa3b6797

This comment was posted by an AI agent (OpenHands).

[github-actions]

Warning

Your comment is too long (maximum is 65536 characters), so the coverage report was not added. See the job log for how to reduce it.

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Review Summary

ð�¡ Acceptable â�� Core design is sound, but one critical bug will break Cloud and profile-less local dispatch.

Critical Issue

backend.api_prefix raises AttributeError on non-conversation backends (openhands/automation/dispatcher.py, line 515)

The old code checked get_config().service.run_agent_profile â�� a config string that was truthy when a profile was configured. The replacement backend.api_prefix is a property defined only on ConversationAgentServerBackend. Neither CloudSandboxBackend nor LocalAgentServerBackend defines it, so accessing it raises AttributeError.

This line is reached after a successful execution (if result.success:). The exception propagates to _execute_run_safe's broad except Exception, which marks the run FAILED â�� despite the actual execution having succeeded. The conversation_id link is also skipped.

Impact: Every successful Cloud dispatch and every successful local-mode-without-profile dispatch will be incorrectly marked FAILED.

Fix: Use isinstance(backend, ConversationAgentServerBackend) instead, or add an api_prefix property to the ExecutionBackend base class returning "" by default.

Risk Assessment

• Overall PR: ð�¡ MEDIUM â�� The AttributeError bug is a regression that breaks the primary dispatch path for Cloud and non-profile local runs. Once fixed, the rest of the change is well-structured: migration is correct, run snapshot logic is sound, validation properly rejects conflicting model+profile selections, git sync round-trip is tested, and the Docker compatibility backend removal is clean.

Verdict

â�� Needs rework â�� The backend.api_prefix AttributeError must be fixed before merge. All other changes look good.

Key Insight: The api_prefix property is conversation-backend-specific, but the guard that uses it runs for every backend type.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with � or � to give feedback.

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR persists agent_profile_id on automation definitions and snapshots it on queued runs, replacing the previous host-controlled deployment-level profile override map with per-automation profile selection. The CRUD API, preset endpoints, git sync, run history, and capability discovery all carry the selection. The unreleased Docker-only compatibility backend and profile override settings are removed.

Analysis

Data model & migration: The migration (023) adds a nullable agent_profile_id Uuid column to both automations and automation_runs, using generic SQLAlchemy types for cross-database compatibility. The snapshot-on-queue pattern in create_pending_run correctly ensures that editing a definition affects only future runs, not already-queued ones. The fallback to the deployment default (AUTOMATION_AGENT_PROFILE) when the automation has no explicit profile is handled correctly.

Validation: validate_agent_profile_selection consistently rejects (a) profiles in non-local mode and (b) a conflicting model field. This is applied uniformly across the CRUD create, CRUD update, prompt preset, plugin preset, and validate-draft endpoints. The update path correctly uses the pending update_data value (or the existing value when only model is being changed) for validation.

Dispatcher concurrency: Changing the gate from run_agent_profile to is_local_mode means all local-mode deployments now enforce conversation_max_concurrent_runs, not just those with a profile configured. This is correct — LocalAgentServerBackend also dispatches to the agent server, which has resource constraints. Since the removed settings were unreleased, no existing deployment loses behavior it was relying on.

Preset runner changes: The load_provisioned_agent helper fetches the conversation from the agent server, validates the launched profile matches AUTOMATION_AGENT_PROFILE_ID, and returns the already-configured agent. When a provisioned agent is present, the runner skips get_llm, get_secrets, get_mcp_config, and get_default_agent — meaning the host secret store is never forwarded to the sandbox. This is a genuine security improvement.

Profile-change tarball refresh: When an existing preset automation's profile is changed, the tarball is rebuilt with the current runner files (refresh_runner=True) while preserving prompt and plugin/repo configuration. The _replace_prompt_in_tarball function handles this correctly by tracking seen members and appending any replacement files that weren't already present.

Watchdog: The cleanup and staleness checks correctly shifted from deployment-level (run_agent_profile) to per-run (run.agent_profile_id or settings.agent_profile) and per-mode (is_local_mode) checks, which is more precise.

Risk Assessment

🟡 MEDIUM — The PR removes unreleased configuration options (AUTOMATION_DOCKER_AGENT_PROFILE, AUTOMATION_AGENT_PROFILE_OVERRIDES, docker_max_concurrent_runs) and the DockerAgentServerBackend compatibility class. Since these were unreleased, this is safe. The dispatcher concurrency behavior change for local-mode deployments without profiles is a behavioral shift but is correct. The SDK dependency bump (79021c6 → 85b8bc7) is a first-party package from the same organization and is excluded from the 7-day waiting rule, but the PR description notes a live factory rollout is in progress, which serves as real-world validation.

Verdict

✅ Worth merging — The design is sound, the data model is clean, validation is consistent across all entry points, and the preset runner changes improve security by avoiding host secret forwarding. Test coverage is thorough for the new behavior. No material issues found.

---

Improve this review? If any feedback above seems incorrect or irrelevant for this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

Automated review used the wrong decision (APPROVED instead of COMMENT) and is dismissed. Findings are reposted as a comment.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 84c744e662f201c4175722d1ce76c519993d6916
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/c1828ae6-17fd-4db0-b64b-01d651bb571c

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: 27bcf9e19963e778ca3db397eff8da2e4992707e
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/e0b7e464-79f8-4f8b-b69f-7893e3104b42

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR adds agent_profile_id to automation definitions and queued-run snapshots, carrying it through CRUD, presets, git sync, capabilities, and the watchdog. The design is clean: validation is centralized in validate_agent_profile_selection, the snapshot-on-queue pattern in create_pending_run is correct, and the preset runner scripts have a well-structured dual path (provisioned-conversation attach vs. legacy create).

Findings

No material issues found. A few observations:

1. Dispatcher/watchdog scope change is correct: The switch from settings.agent_profile to settings.is_local_mode in dispatcher.py and watchdog.py broadens the concurrency-limiting and staleness-scan logic to cover all local-mode runs, not just profile-based ones. This is the right call — local mode always uses the conversation backend pattern, so the concurrency limit and broader scan condition apply regardless of whether an individual run has a profile.

2. PATCH clearing without model restoration: In update_automation, when agent_profile_id is explicitly set to null without also providing a model, the existing model=None (set when the profile was attached) stays as-is rather than being resolved to the user's active default. This is a minor design choice, not a bug — model=None means "use deployment default," which is functionally equivalent in most cases. The test suite confirms the profile-clearing path works.

3. Git sync import consistency: The git import path in _validate_and_resolve_fields reuses validate_agent_profile_selection before resolving the tarball, which correctly rejects conflicting model + agent_profile_id combinations without side effects. The model value from YAML is stored directly without resolve_model_profile_for_user — this is pre-existing behavior, not a regression.

4. Security posture: The validation rejects profiles in cloud mode (preventing privilege escalation through a more privileged agent), and the README documents that missing profiles fail rather than falling back. The preset runner scripts correctly skip loading secrets/LLM/MCP when attaching to a provisioned conversation, avoiding secret-scope leakage.

5. Migration: Simple nullable sa.Uuid() column addition to both automations and automation_runs tables. Cross-database compatible.

Risk Assessment

🟢 LOW — The change is well-contained. Profile selection is validated consistently across all entry points. The snapshot pattern ensures queued runs are unaffected by definition edits. Tests cover round-trip creation, queued-run snapshots, preset runner refresh, git import validation, and credential handoff failure.

Verdict

✅ Worth merging — Core logic is sound, validation is consistent across all paths, and the security model is correct.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch with the /codereview trigger and context the reviewer is missing.
2. Re-request a review — the reviewer reads guidelines from the PR branch, so changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review.

Was this review helpful? React with 👍 or 👎 to give feedback.

[neubig]

@all-hands-bot please review the current head after restacking on the SDK pin refresh and explicitly approve it if there are no material findings.

[all-hands-bot]

Code Review â�� PR #453 (current head e17c670)

Verdict: � APPROVED � � LOW risk, no material findings.

I reviewed the full diff from d37f97b (main) to e17c670 (head), including the SDK pin restack commits (9b4ede3 + e17c670), which only refresh the SDK git pin from de5bc17 â�� e9392e7 in pyproject.toml and uv.lock â�� no code changes.

Key findings

Data model â�� Clean additive design. Nullable agent_profile_id columns on both Automation and AutomationRun, snapshotted at queue time in create_pending_run. Migration 024 correctly revises 023 (the current main head), so no reconciliation is needed.

Backend selection â�� get_backend() correctly routes profile-backed runs to ConversationAgentServerBackend, non-profile local runs to LocalAgentServerBackend, and cloud runs to CloudSandboxBackend. Profiles in non-local mode are rejected at both validation (validate_agent_profile_selection) and dispatch.

Security â�� The callback URL suppression in the dispatcher correctly prevents profile-backed workers from receiving the service-admin callback key. ConversationAgentServerBackend.build_env_vars() exposes only the runtime-scoped SESSION_API_KEY, never the callback credential. Watchdog polling replaces callbacks as the completion path for profile-backed runs.

isinstance fix â�� _resolve_agent_server and fetch_latest_finish_tool_response_for_run correctly use isinstance(backend, LocalAgentServerBackend) instead of backend.is_local_mode, preventing ConversationAgentServerBackend (a subclass) from provisioning new conversations during follow-up turns or outcome retrieval.

Conversation ID â�� Deterministic uuid5 derivation from (org_id, automation_id, source, subject_key) is consistent between the backend property and the dispatcher's computation. Falls back to run.id when no subject is present.

Dispatcher concurrency â�� New conversation_max_concurrent_runs setting (default 2) limits concurrent profile-backed runs. The batch logic correctly excludes already-active automations and caps new dispatches at 1 per poll cycle.

Watchdog â�� Broadened to poll profile-backed runs on every scan in local mode (not just after timeout), with cleanup correctly handling Docker runtime release via release_context().

Preset scripts â�� Both prompt and plugin presets correctly skip loading their own LLM/secrets/MCP when AUTOMATION_AGENT_PROFILE_ID is set, using RemoteConversation.attach() instead of the Conversation constructor.

SDK migration � All direct httpx calls to the agent server have been replaced with SDK workspace/conversation methods. The SDK pin is a temporary git reference, acknowledged in the PR description as needing replacement with a release before merge.

Tests

8 conversation backend tests and 36 other non-Docker tests pass locally. Docker-requiring tests (testcontainers/Postgres) could not run in this environment but are structurally sound from code inspection.

Acknowledged non-blocking items

• Temporary SDK git pin (software-agent-sdk e9392e7) â�� documented, must be replaced with a release before merge.
• FinishTool hook limitation â�� tracked separately in Apply preset FinishTool hooks to profile-created conversations #457.
• is_local_mode naming is slightly misleading (means "agent server configured") â�� pre-existing, not introduced by this PR.

Generated by OpenHands AI on behalf of the user.

[all-hands-bot]

Reviewed head e17c670 (post SDK pin restack). No material findings. See the detailed review comment for the full assessment. Verdict: APPROVED, risk LOW.

Generated by OpenHands AI on behalf of the user.

[all-hands-bot]

👋 This PR needs a couple of things fixed before OpenHands can review it:

• the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.