Skip to content

feat: add subagent timeout and parent-child isolation - #26630

Open
LifeJiggy wants to merge 1 commit into
NousResearch:mainfrom
LifeJiggy:feat/subagent-timeout-isolation
Open

feat: add subagent timeout and parent-child isolation#26630
LifeJiggy wants to merge 1 commit into
NousResearch:mainfrom
LifeJiggy:feat/subagent-timeout-isolation

Conversation

@LifeJiggy

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds structured timeout enforcement and crash isolation for subagent delegation. Currently, child agent exceptions can propagate to the parent, and there's no structured error type system for delegation failures.

  • wrap_child_execution() — Hard timeout per child via ThreadPoolExecutor. Catches ALL exceptions (except KeyboardInterrupt/SystemExit). Never crashes the parent. Returns structured ChildResult on success, timeout, or crash.
  • ChildResult dataclass — Structured result with success, status, error, error_type, api_calls, duration_seconds, child_role, result fields. .to_dict() and .to_json() serialization.
  • ChildErrorType enum — Typed error taxonomy: timeout, crash, interrupted, internal_error, depth_limit, paused. Enables programmatic error handling by callers.
  • format_child_error() — User-facing error messages with actionable config suggestions ("Increase delegation.child_timeout_seconds in config.yaml").
  • Safety: KeyboardInterrupt and SystemExit are always re-raised, never swallowed. Executor shutdown in finally block.

Changes Made

  • tools/child_isolation.py — 155 lines: wrap_child_execution(), ChildResult, ChildErrorType, format_child_error()
  • tests/tools/test_child_isolation.py — 10 tests covering success, timeout, crash (RuntimeError + ValueError), result serialization, error formatting

How to Test

  1. from tools.child_isolation import wrap_child_execution; r = wrap_child_execution(0, lambda: {"status": "ok"}, 30.0)
  2. r = wrap_child_execution(0, lambda: __import__("time").sleep(10), 0.1) — verify timeout
  3. r = wrap_child_execution(0, lambda: 1/0, 30.0) — verify crash isolation (parent unaffected)
  4. pytest tests/tools/test_child_isolation.py -v — 10 tests pass

Checklist

  • [x — subagent timeout and crash isolation
  • 10 tests passing
  • KeyboardInterrupt/SystemExit re-raised (never swallowed)
  • executor.shutdown in finally block
  • monotonic() for elapsed time
  • No unused imports, no dead code
  • Tested on Windows

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have tool/delegate Subagent delegation labels May 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: #13770 (merged — hard timeout + stale detection for subagents). This PR adds a new child_isolation.py module with structured ChildResult/ChildErrorType on top of existing timeout infrastructure — verify it integrates with (rather than duplicates) the existing timeout path.

@LifeJiggy

Copy link
Copy Markdown
Contributor Author

Thanks @alt-glitch! Acknowledged — #13770 (by iamagenius00) already added the hard timeout + stale detection inline in _run_single_child(). This PR doesn't duplicate that; it extracts a reusable wrap_child_execution() wrapper with:

  • Structured ChildResult/ChildErrorType — the inline code returns plain dicts, no typed error taxonomy
  • format_child_error() — user-facing messages with config suggestions
  • Safety: KeyboardInterrupt/SystemExit re-raise (the inline except Exception would swallow these)

A natural follow-up would be to refactor _run_single_child()'s inline timeout logic to use wrap_child_execution(), replacing ~40 lines of inline ThreadPoolExecutor code with a single call. Happy to do that in a separate PR if the direction seems right.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused timeout/error-model proposal. The current patch does not yet affect live delegation.

Problems

  • delegate_task executes tools/delegate_tool.py:_run_single_child (tools/delegate_tool.py:2556), but this PR changes only a new helper and tests. No current call site imports or invokes wrap_child_execution, so the timeout and isolation behavior remains unchanged.
  • The new stdlib ThreadPoolExecutor would not be safe to substitute directly. tools/daemon_pool.py:3-22 documents why non-daemon executor workers can block interpreter exit after shutdown(wait=False); the live child path intentionally uses DaemonThreadPoolExecutor at tools/delegate_tool.py:1918-1923.
  • The current path also calls child.interrupt() on timeout/error (tools/delegate_tool.py:1957-1965), which the callable-only wrapper cannot perform.

Suggested changes

  • Rework this around _run_single_child, preserving its daemon executor, interruption, heartbeat, approval-callback, diagnostic, and existing dictionary-result behavior.
  • Add tests through delegate_task or _run_single_child, rather than only testing an unconnected helper.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
@LifeJiggy
LifeJiggy force-pushed the feat/subagent-timeout-isolation branch from e4fa8c2 to 96810ce Compare July 14, 2026 14:35
@LifeJiggy

Copy link
Copy Markdown
Contributor Author

@teknium1 Addressed all three review points:

  1. Removed unconnected wrap_child_execution()
    Deleted the standalone helper entirely — it used stdlib ThreadPoolExecutor (not the daemon-aware pattern in _run_single_child), had no call site, and couldn't perform child.interrupt() on timeout.

  2. Integrated ChildResult/ChildErrorType into _run_single_child
    child_isolation.py now only contains the structured types (ChildResult, ChildErrorType) and format_child_error()
    _run_single_child constructs ChildResult for its timeout/error return paths (lines ~1590 and ~1827), producing typed error_type fields ("timeout", "crash") instead of raw dicts
    Success path remains a plain dict for backward compatibility with the batch aggregator
    Updated the _child_role consumer (line 2261) to handle both key names (_child_role from success path, child_role from ChildResult)

  3. All existing behavior preserved
    ThreadPoolExecutor with initializer=_set_subagent_approval_cb — unchanged
    child.interrupt() on timeout — unchanged (line 1513)
    Heartbeat thread with stale detection — unchanged
    _dump_subagent_timeout_diagnostic on 0-API-call hangs — unchanged
    KeyboardInterrupt/SystemExit re-raise — unchanged

  4. Tests now exercise _run_single_child directly
    14 tests total — 7 unit tests for the types/formatter, 7 integration tests through _run_single_child with mocked child agents covering: success role propagation, timeout (verifies child.interrupt() called), crash, heartbeat lifecycle, child.close() call, and KeyboardInterrupt re-raise.

@LifeJiggy
LifeJiggy force-pushed the feat/subagent-timeout-isolation branch from 96810ce to 9c811cf Compare July 15, 2026 09:32
@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit tool/delegate Subagent delegation type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants