feat: allow delegator to spawn pre-defined subagents #1223

simonrosenberg · 2025-11-21T16:02:33Z

PR Summary: Allow user to specify their own agent at runtime

Overview

Users can specify their own agent factory methods in agent delegation.

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	`eclipse-temurin:17-jdk`	Link
python	amd64, arm64	`nikolaik/python-nodejs:python3.12-nodejs22`	Link
golang	amd64, arm64	`golang:1.21-bookworm`	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:9cd58ec-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-9cd58ec-python \
  ghcr.io/openhands/agent-server:9cd58ec-python

All tags pushed for this build

ghcr.io/openhands/agent-server:9cd58ec-golang-amd64
ghcr.io/openhands/agent-server:9cd58ec-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:9cd58ec-golang-arm64
ghcr.io/openhands/agent-server:9cd58ec-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:9cd58ec-java-amd64
ghcr.io/openhands/agent-server:9cd58ec-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:9cd58ec-java-arm64
ghcr.io/openhands/agent-server:9cd58ec-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:9cd58ec-python-amd64
ghcr.io/openhands/agent-server:9cd58ec-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-amd64
ghcr.io/openhands/agent-server:9cd58ec-python-arm64
ghcr.io/openhands/agent-server:9cd58ec-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-arm64
ghcr.io/openhands/agent-server:9cd58ec-golang
ghcr.io/openhands/agent-server:9cd58ec-java
ghcr.io/openhands/agent-server:9cd58ec-python

About Multi-Architecture Support

Each variant tag (e.g., 9cd58ec-python) is a multi-arch manifest supporting both amd64 and arm64
Docker automatically pulls the correct architecture for your platform
Individual architecture tags (e.g., 9cd58ec-python-amd64) are also available if needed

simonrosenberg · 2025-11-21T16:05:07Z

@OpenHands /codereview-roasted Read the diff and write a code review. Specifically

how the agent_type "default" logic can be simplified
how the agent registration pattern could perhaps be closer to the tools registration pattern in the repo
Point out any other code issue

openhands-ai · 2025-11-21T16:05:20Z

I'm on it! simonrosenberg can track my progress at all-hands.dev

openhands-ai · 2025-11-21T16:08:21Z

Code Review Summary

I've completed a comprehensive code review of PR #1223 focusing on the specific issues you requested. Here are my findings:

🔍 Requested Analysis

1. Agent Type "default" Logic Simplification

Problem: The current logic in impl.py (lines 109-113) is unnecessarily complex with three levels of default handling:

agent_type = "default"
if action.agent_types is not None and i < len(action.agent_types):
    agent_type = action.agent_types[i] or "default"

Recommendation: Simplify to a single expression or extract to a helper function that clearly handles the default case without nested checks.

2. Agent Registration vs Tool Registration Pattern Mismatch

Key differences found:

Feature	Tool Registry	Agent Registry
Thread Safety	✅ Uses RLock	❌ None
Registration Types	3 types (instance, subclass, callable)	1 type (callable only)
Pattern	Resolver-based with ConversationState	Simple dict lookup
Default Handling	Raises errors	Pre-registers at import time

Recommendation: Either align agent registration with the robust tool registry pattern, or simplify tool registry if the complexity isn't needed. Inconsistent patterns create cognitive friction.

🔴 Critical Issues

Module-level side effects (registration.py:113-117): The "default" agent is registered at import time, creating a special case that's treated both as a sentinel value AND a real agent type
No thread safety: Unlike tool registry, agent registry lacks RLock protection
Poor data structure choice: "default" creates edge cases throughout - it's checked in multiple places, creating unnecessary complexity

🟡 Other Issues Found

Error handling: get_agent_factory() raises errors instead of sensible defaults
Verbose message building: Lines 137-145 in impl.py could be simplified with helper functions
Type ambiguity: agent_types: list[str | None] | None allows None in the list unnecessarily
Missing tests: No tests for the new registration system
Overly complex example: The example tries to demonstrate too much at once

💡 Key Insight

The fundamental problem is treating "default" as both a sentinel value and a registered agent type. This creates special cases everywhere. Better design: make default behavior implicit when no agent_type is specified, rather than requiring "default" to be passed around and registered.

Full detailed review: /workspace/project/CODE_REVIEW.md

The review follows the "Linus Torvalds roasted" style you requested - direct, technically rigorous, focusing on fundamental engineering principles over style preferences.

_{View full conversation}

simonrosenberg · 2025-11-21T16:39:58Z

@OpenHands /codereview-roasted Read the diff and write a code review

openhands-ai · 2025-11-21T16:40:07Z

I'm on it! simonrosenberg can track my progress at all-hands.dev

openhands-ai · 2025-11-21T16:43:21Z

I've completed a comprehensive code review of PR #1223 "feat: allow delegator to spawn pre-defined subagents". Since you requested a code review (not code changes), I have not modified any code - only provided detailed feedback.

Summary of Code Review Findings

I analyzed the diff and provided a Linus-style "roasted" code review focusing on your specific questions:

1. Agent Type "default" Logic Simplification

Critical Issue Found: The current _resolve_agent_type() method (lines 77-85 in impl.py) has unnecessary complexity with three levels of None-checking. The root problem is treating "default" as a special None value instead of a normal string, which creates ripple effects throughout the codebase with special-case handling.

Recommendation: Change _resolve_agent_type() to always return a string (including "default"), eliminating the None/empty string special cases entirely. This would simplify the code from ~9 lines with 3 conditionals to ~5 lines with clearer logic.

2. Agent Registration Pattern vs Tools Registration Pattern

Key Gap Identified: The agent registration is much simpler than the tool registration pattern, but in an awkward way:

Feature	Tool Registry	Agent Registry
Multiple registration types	✅ (instance, class, callable)	❌ (only callable)
Parameter support	✅ Full params dict	❌ Only LLM
ConversationState access	✅ Passed to factory	❌ Not available
Type validation	✅ Validates returns	❌ No validation

Recommendation: Either keep it simple but make it consistent (remove special cases, add validation), or elevate agent registration to SDK-level like tools. Currently it's in an uncomfortable middle ground.

3. Other Code Issues Found

No type safety: Factory functions aren't validated to return Agent instances
Global mutable state: Registry lacks clear lifecycle management
Dynamic description generation: Tool description rebuilt on every create() call
Missing test coverage: Only happy-path tests, no error cases
Unnecessary indirection: create_default_agent() wrapper adds no value

Overall Assessment

🟡 Acceptable - Core functionality is sound and solves a real user need, but the implementation has unnecessary complexity from special-case handling. The code would benefit from:

HIGH Priority: Eliminate None handling for default - use "default" string everywhere
HIGH Priority: Add validation that factories return Agent instances
MEDIUM Priority: Add negative test cases for error conditions

The review is available above with detailed line-by-line analysis, code examples, and specific recommendations. No code was modified per the code review instructions.

_{View full conversation}

github-actions · 2025-11-21T16:43:31Z

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-tools/openhands/tools/delegate
definition.py	25	7	72%	80–81, 84, 87–88, 97, 100
impl.py	120	100	16%	30, 32–33, 42–43, 47, 53–54, 57–60, 62, 74–75, 79–81, 85–86, 93–95, 104–105, 115–119, 121, 125–127, 129–130, 136, 138, 144, 147, 152, 157, 161, 166–168, 185–186, 193–195, 204, 206–208, 211–216, 218, 225–226, 228–229, 232–235, 237–238, 242–245, 248–250, 255–256, 259–260, 263, 265–269, 271, 274–276, 278–279, 282, 284, 289–291
registration.py	40	25	37%	70–72, 74, 93–94, 96–98, 100–102, 107, 112–113, 115–116, 120–121, 124, 126–127, 129, 134–135
TOTAL	12444	5778	53%

….com:OpenHands/software-agent-sdk into allow-delegator-to-load-predefined-subagents

openhands-tools/openhands/tools/delegate/definition.py

openhands-ai · 2025-11-21T17:06:35Z

Looks like there are a few issues preventing this PR from being merged!

GitHub Actions are failing:
- Check Documented Examples

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #1223 at branch `allow-delegator-to-load-predefined-subagents`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

Move the delegate tool description from an inline f-string to a separate Jinja template file for better maintainability and separation of concerns. Changes: - Create delegate/templates/delegate_tool_description.j2 - Update definition.py to use render_template() - Add .j2 file inclusion to MANIFEST.in for openhands-tools Co-authored-by: openhands <openhands@all-hands.dev>

enyst

Love this PR, thank you! I think people are asking for this 😄

simonrosenberg · 2025-11-25T11:07:11Z

@OpenHands solve the conflicts with the PR

openhands-ai · 2025-11-25T11:07:23Z

I'm on it! simonrosenberg can track my progress at all-hands.dev

Resolved conflict in 25_agent_delegation.py by integrating cost reporting from main branch into both the simple delegation and user-defined agents examples. Co-authored-by: openhands <openhands@all-hands.dev>

openhands-ai · 2025-11-25T11:11:00Z

✅ Conflicts Resolved Successfully

I've successfully resolved the merge conflicts in PR #1223 and pushed the changes to update the branch.

Summary of Changes

Conflict Location: examples/01_standalone_sdk/25_agent_delegation.py

Resolution:

The conflict was between:
- Main branch: Added cost reporting (EXAMPLE_COST) at the end of the simple delegation example
- Current branch: Extended the example with a second part demonstrating user-defined agent types
I merged both changes by:
1. Added cost reporting for the simple delegation example (line 83-85) with label "EXAMPLE_COST (simple delegation)"
2. Kept all the extended functionality for user-defined agent types (lines 90-196)
3. Added cost reporting for the user-defined agent types example (line 192-194) with label "EXAMPLE_COST (user-defined agents)"

Additional Merged Changes from Main

The merge also brought in several other updates from main:

Workflow improvements for eval and examples testing
New test infrastructure for running examples via pytest
Bug fix for file download API endpoint
Dependency updates

All pre-commit checks passed, and the changes have been committed and pushed to allow-delegator-to-load-predefined-subagents.

The PR is now up to date with main and ready for review!

_{View full conversation}

allow delegator to spawn pre-defined subagents

c738fe1

simonrosenberg self-assigned this Nov 21, 2025

simonrosenberg added 2 commits November 21, 2025 17:37

apply roasted code review

cfa9d5d

add follow-up message

85b3bb5

simonrosenberg marked this pull request as ready for review November 21, 2025 16:40

Merge branch 'main' into allow-delegator-to-load-predefined-subagents

ff821f7

simonrosenberg added 3 commits November 21, 2025 17:47

make 2 delegation files

07a4e9c

Merge branch 'allow-delegator-to-load-predefined-subagents' of github…

2275280

….com:OpenHands/software-agent-sdk into allow-delegator-to-load-predefined-subagents

apply review comments

b862a4d

enyst reviewed Nov 21, 2025

View reviewed changes

openhands-tools/openhands/tools/delegate/definition.py Outdated Show resolved Hide resolved

simonrosenberg and others added 2 commits November 21, 2025 18:19

move examples to same file

44a2d8b

simonrosenberg requested a review from enyst November 24, 2025 15:38

Merge branch 'main' into allow-delegator-to-load-predefined-subagents

02e4e33

enyst approved these changes Nov 25, 2025

View reviewed changes

Merge branch 'main' into allow-delegator-to-load-predefined-subagents

59cc39d

Resolved conflict in 25_agent_delegation.py by integrating cost reporting from main branch into both the simple delegation and user-defined agents examples. Co-authored-by: openhands <openhands@all-hands.dev>

Merge branch 'main' into allow-delegator-to-load-predefined-subagents

77e8536

simonrosenberg merged commit 3ec7bb2 into main Nov 25, 2025
22 checks passed

simonrosenberg deleted the allow-delegator-to-load-predefined-subagents branch November 25, 2025 15:37

jpshackelford mentioned this pull request Dec 3, 2025

Delegation - allow delegator to spawn sub-agent from a pool of pre-configured agents #1120

Closed

feat: allow delegator to spawn pre-defined subagents #1223

feat: allow delegator to spawn pre-defined subagents #1223

Uh oh!

Conversation

simonrosenberg commented Nov 21, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary: Allow user to specify their own agent at runtime

Overview

Uh oh!

simonrosenberg commented Nov 21, 2025

Uh oh!

openhands-ai bot commented Nov 21, 2025

Uh oh!

openhands-ai bot commented Nov 21, 2025

Code Review Summary

🔍 Requested Analysis

1. Agent Type "default" Logic Simplification

2. Agent Registration vs Tool Registration Pattern Mismatch

🔴 Critical Issues

🟡 Other Issues Found

💡 Key Insight

Uh oh!

simonrosenberg commented Nov 21, 2025

Uh oh!

openhands-ai bot commented Nov 21, 2025

Uh oh!

openhands-ai bot commented Nov 21, 2025

Summary of Code Review Findings

1. Agent Type "default" Logic Simplification

2. Agent Registration Pattern vs Tools Registration Pattern

3. Other Code Issues Found

Overall Assessment

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

openhands-ai bot commented Nov 21, 2025

Uh oh!

enyst left a comment

Choose a reason for hiding this comment

Uh oh!

simonrosenberg commented Nov 25, 2025

Uh oh!

openhands-ai bot commented Nov 25, 2025

Uh oh!

openhands-ai bot commented Nov 25, 2025

✅ Conflicts Resolved Successfully

Summary of Changes

Additional Merged Changes from Main

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

simonrosenberg commented Nov 21, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Nov 21, 2025 •

edited

Loading