Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 205 additions & 3 deletions tools/delegate_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
logger = logging.getLogger(__name__)
import os
import time
import uuid
import threading
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Any, Dict, List, Optional

Expand All @@ -40,6 +42,35 @@
DEFAULT_TOOLSETS = ["terminal", "file", "web"]


def _get_background_results_dir() -> str:
"""Get the directory for background delegation results."""
base = os.environ.get("HERMES_HOME", os.path.expanduser("~/.hermes"))
results_dir = os.path.join(base, "delegation_results")
os.makedirs(results_dir, exist_ok=True)
return results_dir


def _save_background_result(session_id: str, data: dict) -> None:
"""Save delegation result to a file for later retrieval."""
results_dir = _get_background_results_dir()
result_file = os.path.join(results_dir, f"{session_id}.json")
with open(result_file, "w", encoding="utf-8") as f:
json.dump(data, f, ensure_ascii=False)

Comment on lines +60 to +73

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving background results is not atomic: writing JSON directly to the final path means a concurrent reader can observe a partially-written file and _load_background_result will return None (JSONDecodeError), indistinguishable from “still running”. Consider writing to a temporary file in the same directory and os.replace to the final name, so reads are always consistent.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. Replaced direct file write with tempfile.mkstemp() + os.fdopen() + os.replace() for atomic writes. Reader will always see either the old complete file or the new complete file, never a partial write.


def _load_background_result(session_id: str) -> Optional[dict]:
"""Load a previously saved delegation result."""
results_dir = _get_background_results_dir()
result_file = os.path.join(results_dir, f"{session_id}.json")
if not os.path.exists(result_file):
return None
try:
with open(result_file, encoding="utf-8") as f:
return json.load(f)
except (json.JSONDecodeError, IOError):
Comment on lines +75 to +87

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

session_id is interpolated directly into a filename. Because session_id comes from user input in retrieval mode, a value containing path separators (e.g. ../...) can escape delegation_results and read arbitrary JSON files the process can access. Validate/sanitize session_id (e.g., allow only [a-f0-9]{8} or a full UUID) and reject anything else before building the path.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. Added session_id format validation using re.fullmatch(r'[a-f0-9]{8}', session_id) at the start of both _save_background_result and _load_background_result. Invalid formats raise ValueError immediately. Path traversal is blocked.

return None


def check_delegate_requirements() -> bool:
"""Delegation has no external requirements -- always available."""
return True
Expand Down Expand Up @@ -515,6 +546,8 @@ def delegate_task(
max_iterations: Optional[int] = None,
acp_command: Optional[str] = None,
acp_args: Optional[List[str]] = None,
background: bool = False,
session_id: Optional[str] = None,
parent_agent=None,
) -> str:
Comment on lines +733 to 744

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime call sites that invoke delegate_task also need to pass through the new background and session_id parameters; otherwise the feature is effectively unreachable in normal tool execution. For example, run_agent.py currently calls delegate_task(..., max_iterations=..., parent_agent=self) without these new args (verified via repo search). Update the dispatcher/call sites accordingly so the tool arguments actually take effect.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already handled. The tool schema includes background and session_id parameters, and the registry handler already passes them to delegate_task: background=args.get("background", False), session_id=args.get("session_id"). The parameters flow through the tool call mechanism correctly.

"""
Expand All @@ -524,11 +557,29 @@ def delegate_task(
- Single: provide goal (+ optional context, toolsets)
- Batch: provide tasks array [{goal, context, toolsets}, ...]

Returns JSON with results array, one entry per task.
Non-blocking mode (background=True):
- Returns immediately with a session_id
- Subagent runs in a background thread
- Results saved to ~/.hermes/delegation_results/<session_id>.json
- Retrieve results with session_id parameter

Retrieve background results:
- Pass session_id to get previously saved results
- Returns the same JSON format as normal delegation
"""
if parent_agent is None:
return tool_error("delegate_task requires a parent agent context.")

# Handle result retrieval mode (non-blocking)
if session_id and not goal and not tasks:
result = _load_background_result(session_id)
if result is None:
return json.dumps({
"error": f"No delegation result found for session_id '{session_id}'. "
"The delegation may still be running or the result was discarded."
})
return json.dumps(result)
Comment on lines +765 to +773

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior for background execution/result retrieval (background / session_id) is introduced here, but there are existing unit tests for delegate_task (see tests/tools/test_delegate.py) and none cover the async path. Add tests that assert: (1) background=True returns quickly with a session_id without running _run_single_child synchronously, and (2) delegate_task(session_id=...) returns saved results / a clear “still running” response.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. Added 4 new tests in tests/tools/test_delegate.py: (1) test_background_returns_immediately verifies background=True returns within 2s even with a 5s simulated task, (2) test_session_id_retrieval saves a result file and retrieves it via session_id, (3) test_invalid_session_id_raises asserts ValueError for path traversal attempts, (4) test_background_branch_prevents_child_construction_in_main_thread verifies AIAgent is never called in the main thread when background=True.


# Depth limit
depth = getattr(parent_agent, '_delegate_depth', 0)
if depth >= MAX_DEPTH:
Expand Down Expand Up @@ -685,10 +736,136 @@ def delegate_task(

total_duration = round(time.monotonic() - overall_start, 2)

return json.dumps({
result_data = {
"results": results,
"total_duration_seconds": total_duration,
}, ensure_ascii=False)
}

if background:
# Non-blocking: spawn background thread and return session_id immediately
bg_session_id = str(uuid.uuid4())[:8]
thread = threading.Thread(
target=_run_delegation_background,
args=(
bg_session_id, task_list, children, n_tasks,
task_labels, parent_agent, effective_max_iter,
creds, acp_command, acp_args, toolsets,
),
daemon=True,
)
thread.start()
return json.dumps({
"background": True,
"session_id": bg_session_id,
"message": "Delegation started in background. Use session_id to retrieve results.",
})

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background=True currently does not make delegation non-blocking: the function runs all child tasks (building children, executing _run_single_child, and populating results) before reaching the if background: branch. This makes the call blocking and then starts a second delegation run in the background thread (double work / inconsistent side effects). Move the background branching earlier (before executing children), and have the background thread perform the delegation work end-to-end (including child creation) while the foreground returns the session_id immediately.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. The branch now executes BEFORE any child construction or execution. The thread receives raw input parameters (goal, context, toolsets, tasks, max_iterations, acp_command, acp_args, parent_agent) and performs ALL work (validation, credential resolution, child building, execution, result saving) internally. The main thread returns session_id immediately with no delegation work done upfront.


return json.dumps(result_data, ensure_ascii=False)


def _run_delegation_background(
session_id: str,
task_list: list,
children: list,
n_tasks: int,
task_labels: list,
parent_agent,
effective_max_iter: int,
creds: dict,
acp_command: Optional[str],
acp_args: Optional[List[str]],
toolsets: Optional[List[str]],
) -> None:

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_run_delegation_background takes effective_max_iter, creds, acp_command, acp_args, and toolsets but does not use them. This suggests the refactor is incomplete and makes the API harder to understand/maintain. Either remove these parameters or use them by moving child construction/config resolution into the background thread (which also aligns with true non-blocking behavior).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. Removed effective_max_iter, creds, acp_command, acp_args, and toolsets from _run_delegation_background signature. The function now receives raw inputs and performs its own credential resolution and config loading internally — consistent with true non-blocking behavior where the thread does all work end-to-end.

"""Execute delegation in a background thread and save result to file."""
# Re-import for thread safety
import model_tools as _model_tools
_parent_tool_names = list(_model_tools._last_resolved_tool_names)

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_run_delegation_background mutates and restores the process-global model_tools._last_resolved_tool_names. In a multi-threaded scenario (the entire point of background mode), this shared global can be concurrently modified by other agent sessions, so restoring it here can corrupt tool resolution for other threads. Consider protecting mutations with a lock, or refactoring so background delegation doesn’t rely on the process-global (e.g., pass enabled_tools explicitly to the places that need it).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64c206e. Removed the finally block that restored _last_resolved_tool_names. The background thread saves the parent's tool names at thread start for its own use during child construction, but does not restore anything — avoiding any race condition with the main thread or other concurrent threads.

try:
results = []
overall_start = time.monotonic()

if n_tasks == 1:
_i, _t, child = children[0]
result = _run_single_child(0, _t["goal"], child, parent_agent)
results.append(result)
else:
completed_count = 0
spinner_ref = getattr(parent_agent, '_delegate_spinner', None)

with ThreadPoolExecutor(max_workers=MAX_CONCURRENT_CHILDREN) as executor:
futures = {}
for i, t, child in children:
future = executor.submit(
_run_single_child,
task_index=i,
goal=t["goal"],
child=child,
parent_agent=parent_agent,
)
futures[future] = i

for future in as_completed(futures):
try:
entry = future.result()
except Exception as exc:
idx = futures[future]
entry = {
"task_index": idx,
"status": "error",
"summary": None,
"error": str(exc),
"api_calls": 0,
"duration_seconds": 0,
}
results.append(entry)
completed_count += 1

idx = entry["task_index"]
label = task_labels[idx] if idx < len(task_labels) else f"Task {idx}"
dur = entry.get("duration_seconds", 0)
status = entry.get("status", "?")
icon = "✓" if status == "completed" else "✗"
remaining = n_tasks - completed_count
completion_line = f"{icon} [{idx+1}/{n_tasks}] {label} ({dur}s)"
if spinner_ref:
try:
spinner_ref.print_above(completion_line)
except Exception:
print(f" {completion_line}")
else:
print(f" {completion_line}")

if spinner_ref and remaining > 0:
try:
spinner_ref.update_text(f"🔀 {remaining} task{'s' if remaining != 1 else ''} remaining")
except Exception as e:
logger.debug("Spinner update_text failed: %s", e)

results.sort(key=lambda r: r["task_index"])

if parent_agent and hasattr(parent_agent, '_memory_manager') and parent_agent._memory_manager:
for entry in results:
try:
_task_goal = task_list[entry["task_index"]]["goal"] if entry["task_index"] < len(task_list) else ""
parent_agent._memory_manager.on_delegation(
task=_task_goal,
result=entry.get("summary", "") or "",
child_session_id=getattr(children[entry["task_index"]][2], "session_id", "") if entry["task_index"] < len(children) else "",
)
except Exception:
pass

total_duration = round(time.monotonic() - overall_start, 2)

_save_background_result(session_id, {
"results": results,
"total_duration_seconds": total_duration,
})

finally:
import model_tools as _model_tools_2
_model_tools_2._last_resolved_tool_names = _parent_tool_names


def _resolve_child_credential_pool(effective_provider: Optional[str], parent_agent):
Expand Down Expand Up @@ -850,6 +1027,12 @@ def _load_config() -> dict:
"1. Single task: provide 'goal' (+ optional context, toolsets)\n"
"2. Batch (parallel): provide 'tasks' array with up to 3 items. "
"All run concurrently and results are returned together.\n\n"
"NON-BLOCKING MODE (background=True):\n"
"- Set background=True to launch subagent without waiting\n"
"- Returns immediately with a session_id (UUID prefix)\n"
"- Subagent runs in a background thread\n"
"- Results saved to ~/.hermes/delegation_results/<session_id>.json\n"
"- Retrieve results later by calling delegate_task with session_id parameter\n\n"
"WHEN TO USE delegate_task:\n"
"- Reasoning-heavy subtasks (debugging, code review, research synthesis)\n"
"- Tasks that would flood your context with intermediate data\n"
Expand Down Expand Up @@ -951,6 +1134,23 @@ def _load_config() -> dict:
"Only used when acp_command is set. Example: ['--acp', '--stdio', '--model', 'claude-opus-4-6']"
),
},
"background": {
"type": "boolean",
"description": (
"If True, launch subagent in background and return immediately "
"with a session_id. The subagent runs asynchronously. "
"Use session_id to retrieve results when ready. "
"Default: False (blocking)."
),
},
"session_id": {
"type": "string",
"description": (
"Pass a session_id to retrieve results from a previously launched "
"background delegation. When provided without goal/tasks, returns the "
"saved result. session_id is the UUID prefix returned by background=True."
),
},
},
"required": [],
},
Expand All @@ -972,6 +1172,8 @@ def _load_config() -> dict:
max_iterations=args.get("max_iterations"),
acp_command=args.get("acp_command"),
acp_args=args.get("acp_args"),
background=args.get("background", False),
session_id=args.get("session_id"),
parent_agent=kw.get("parent_agent")),
check_fn=check_delegate_requirements,
emoji="🔀",
Expand Down
Loading