feat(plugins): add public subagent lifecycle API - #63359
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for proposing a typed plugin-facing lifecycle boundary. The API is additive, but its current execution path bypasses delegation invariants that main already centralizes.
Problems
agent/subagent_lifecycle.py:196calls_build_child_agent()without the parent tool-name save/restore required bytools/delegate_tool.py:2499-2541;_run_single_child()later restores the child’s saved value (tools/delegate_tool.py:2314-2320). This can leave process-global tool resolution on the child toolset.agent/subagent_lifecycle.py:230directly runs_run_single_child(), skipping the parent-side memory notification, serializedsubagent_stophooks, and cost rollup intools/delegate_tool.py:2687-2779.hermes_cli/plugins.py:375resolves only through_cli_ref, butwebsite/docs/developer-guide/plugins/index.md:859-862documents that_cli_refis absent in gateway, non-interactive, and kanban-worker contexts.
Suggested changes
- Route both public lifecycle calls and
delegate_taskthrough one host-owned lifecycle executor that preserves current construction, concurrency, cleanup, aggregation, and hook contracts. - Define a session-scoped resolver and lifecycle ownership model before exposing this surface across plugin contexts.
- Validate malformed deserialized handle fields before HMAC comparison; current tests cover a forged string capability but not malformed typed fields.
Automated hermes-sweeper review.
| from tools.delegate_tool import _build_child_agent, DEFAULT_MAX_ITERATIONS | ||
|
|
||
| child = _build_child_agent( | ||
| task_index=0, |
There was a problem hiding this comment.
_build_child_agent() mutates model_tools._last_resolved_tool_names. The existing delegate_task() path saves the parent value before construction, stores it on the child, and restores it afterward (tools/delegate_tool.py:2499-2541). Preserve that invariant here or route through a shared constructor/executor; otherwise child construction can corrupt the parent process's resolved tool set.
| if request.correlation_id: | ||
| _REGISTRY.correlations[correlation_key] = subagent_id | ||
| record.future = _EXECUTOR.submit(self._run, record, request.goal, parent) | ||
| return handle |
There was a problem hiding this comment.
Submitting _run_single_child() directly bypasses the parent-side aggregation in delegate_task() that fires subagent_stop, notifies the memory provider, and rolls child cost into the parent (tools/delegate_tool.py:2687-2779). Please use a shared host-owned lifecycle path rather than reproducing only the worker portion.
| ): | ||
| return None | ||
| if not hmac.compare_digest( | ||
| handle.capability, |
There was a problem hiding this comment.
This only verifies the dataclass type/version. SubagentHandle.from_dict() accepts arbitrary field types, so malformed capability, subagent_id, or created_at values can raise during HMAC computation/comparison instead of producing the documented UNKNOWN response. Validate all capability-input fields before calling _capability().
| from agent.subagent_lifecycle import SubagentLifecycleService | ||
| self._subagent_lifecycle = SubagentLifecycleService( | ||
| lambda: getattr(self._manager._cli_ref, "agent", None) | ||
| if self._manager._cli_ref is not None else None |
There was a problem hiding this comment.
This resolver is CLI-only: _cli_ref is populated by cli.py:13487-13489, while plugin docs explicitly say it is absent in gateway, non-interactive, and kanban-worker contexts (website/docs/developer-guide/plugins/index.md:859-862). Define a session-scoped resolver for those surfaces or document and enforce a deliberately CLI-only API.
|
Implemented the review fixes in
Validation:
I have left the review threads unresolved for reviewer confirmation. |
6cb4bb8 to
d3db019
Compare
|
Rebased onto latest upstream/main (0 behind). All 3 hermes-sweeper concerns were already addressed by the follow-up commit 6cb4bb80a:
Additional: malformed handle field validation (type checks on every 18/18 lifecycle contract tests pass on the rebased branch. Ready for human review. |
|
@asimons81 I'm evaluating #65447 for a concrete use case: a Luna parent selectively launching a full Sol child. The current #63359 head appears to provide the missing plugin lifecycle surface, and its CI is green. Is this still the intended salvage path? If useful, I can validate model selection and shared delegation limits against the current upstream base. I do not want to open a duplicate PR. |
|
@2001Y -- yes, this is still the intended path for #65447. The branch was rebased onto upstream/main 4 days ago and the review feedback was addressed in a follow-up commit. CI is green and the change is mergable cleanly. Your Luna/Sol use case is exactly the kind of real validation this needs. The shared delegation limits path (acceptance criterion 3 from #65447) is the obvious thing to validate -- both delegate_task and the plugin API route through the same host-owned lifecycle executor now, so the concurrency cap and depth limit should hit the same counters. Having someone verify that against a profile-parented launch would close the gap between contract tests and real behavior. Want me to give you push access to the branch, or would you rather work off the current head and open a sibling PR against #65447? Happy to coordinate either way. |
…mon pool Follow-ups on the salvaged #63359: - _finalize_child_results carries tool_call_history on subagent_stop (the #62011/#72403 field landed after the PR branched; the shared pipeline must emit it for both delegate_task and plugin-launched children). Lifecycle test updated for the new payload field. - The lifecycle executor uses DaemonThreadPoolExecutor — a wedged or abandoned child must never block interpreter exit at atexit-join time (same rationale as _run_single_child's timeout executor and the async-delegation pool). - delegate_task's batch path keeps live-transcript wiring while routing child construction through the shared _build_child_preserving_parent_tools helper.
|
Merged via PR #72501 — thank you for this substantial piece of work! Both your commits were cherry-picked onto current main with your authorship preserved in git history (1865fb5, f60abd6). During the salvage we resolved conflicts against the delegation work that landed since your branch (live transcripts, progress-based stall detection #72227, inline child API calls #72412) and added two integration follow-ups: the shared Plugins can now launch subagents via |
* perf(desktop): 60fps sash drag on real sessions — height-gate the RO pins
Driving HER real instance (real profile, real transcripts, streams live)
via CDP instead of synthetic tiles finally exposed the remaining stall.
The timeline on a real 60-frame sash drag:
style recalc 2736ms | script 1027ms | layout 89ms
top callsite: pin @ fallback.tsx — 927ms
Two pin-to-bottom ResizeObservers (the bounded tool window's and the
reasoning preview's) pinned on EVERY resize delivery. A sash drag changes
every message's WIDTH once per frame, so each frame ran scrollTop write ->
scrollHeight read across every tool group: a forced write-read reflow
cascade that the render counters could never see (zero React involvement).
Both pins are now height-gated off the RO entry (reflow-free): only
content GROWTH pins. Width-only deliveries return immediately.
Measured on the live app, same drag, before -> after:
fps 11.5 -> 59-60
p95 101ms -> 18ms
slow>33 60/60 -> 1/60
Also in this batch (each was verified live before the next was attempted):
- thread/list: split messageSignature into STRUCTURAL (ids/roles — keys
boundaries + row identity) and WEIGHT (part counts — budget only), and
memoize groups + row JSX. A streamed part-append re-rendered every
turn's boundary via its resetKey prop; explain() measured 540-865
wasted Block renders per drag/stream sample, now {}.
- message-render-boundary: document the structural-only resetKey contract.
- tool/fallback: memoize ToolFallback's part object + ToolEntry/ToolTitle/
ToolGlyph (151 renders each, 100% wasted, on real transcripts).
- use-message-stream: ADAPTIVE flush floor — next flush waits 3x the
measured cost of the last one (33ms floor, 250ms cap), so multi-stream
load degrades text update rate instead of input latency.
- tree-split: preview sash drags with inline flex on the two seam
wrappers, committing the store ONCE on release (fixed-zone sides get
flexBasis only, so a hidden sidebar can't leave a phantom gap).
- debug/: perf-live LoAF long-frame attribution, explain() cascade walker
with changed-hook indices, diag-real-loop/key-latency/switch-trace
probes that drive the real app over CDP.
Typing during 2 live streams: keystroke->paint p50 3.3ms, p95 18.4ms,
zero frames over 33ms. Session switch p50 ~35ms settled; the remaining
~1.3s outlier tail is streaming-session switches (React work-loop, not
style/layout) — next target.
* perf(desktop): don't backfill the transcript while its thread streams
Switching to a STREAMING session took ~1.4s to settle while an idle
session settled in ~50ms. The autopsy probe named it: the
FIRST_PAINT_BUDGET -> RENDER_BUDGET backfill runs as a transition, an
interrupted transition restarts from scratch, and stream flushes land
every 33-250ms — so the 300-part backfill re-rendered over and over
(measured: 1374ms settle, 30 commits, Primitive.div x2237 for one switch).
Gate the backfill on the thread being idle. The user lands on the live
tail immediately either way; older turns backfill the moment the run
ends, and 'Show earlier' remains the manual path meanwhile.
Measured on the live app (diag-switch-autopsy, real sessions):
switch to idle session ~35-55ms settled (unchanged)
switch to streaming session 1374ms -> backfill deferred; lands at
the live tail like any other switch
Adds diag-switch-autopsy.mjs (per-switch settle/commits/top-renders) and
live-drive.mjs (status/fps/drag one-liners against the running app).
* fix(cli): scope -c/--resume to the current workspace
`hermes -c`/`--resume` (continue last session) resolved the globally
most-recently-used session, then cd'd into *its* recorded cwd. So running
`hermes -c` from repo A could land you in repo B's session — the session
you last touched anywhere, not the last one *here*.
Now `_resolve_last_session` scopes to the current workspace first: the git
repo root when CWD is inside a repo (so all sessions across its
subdirs/worktrees group together), else the CWD itself — matching the
`workspace_key` identity `hermes sessions list --workspace` already groups
on. It falls back to the unscoped global MRU when no session matches the
current workspace, preserving the old behaviour for fresh directories.
Adds `workspace_key` param to `SessionDB.search_sessions` and a
`_workspace_key_clause` SQL helper that mirrors `workspace_key()`: a row
matches when its `git_repo_root` equals the key, or (legacy rows without
git metadata) when its `cwd` is at or under it.
* feat(plugins): add public subagent lifecycle API
* fix subagent lifecycle ownership invariants
* fix(delegation): integrate lifecycle refactor with tool-history + daemon pool
Follow-ups on the salvaged NousResearch#63359:
- _finalize_child_results carries tool_call_history on subagent_stop
(the NousResearch#62011/NousResearch#72403 field landed after the PR branched; the shared
pipeline must emit it for both delegate_task and plugin-launched
children). Lifecycle test updated for the new payload field.
- The lifecycle executor uses DaemonThreadPoolExecutor — a wedged or
abandoned child must never block interpreter exit at atexit-join time
(same rationale as _run_single_child's timeout executor and the
async-delegation pool).
- delegate_task's batch path keeps live-transcript wiring while routing
child construction through the shared
_build_child_preserving_parent_tools helper.
* Revert the streaming-backfill gate — it broke a real E2E invariant
Deferring the FIRST_PAINT_BUDGET -> RENDER_BUDGET backfill while a thread
streams cut a 1374ms streaming-session switch to instant, but it also
means a streaming transcript stays clipped to 60 parts for the duration
of the run. `large-session-resume` asserts the resumed transcript shows
every seeded reply exactly once, and that count is short while the budget
is held down — a genuine behavior change, not a flaky test.
The switch cost is real and still worth fixing, but the fix has to keep
the full transcript mounted (raise the budget in idle callbacks, or
virtualize) rather than withhold it. Session-switch work is happening in
a parallel effort; leaving the invariant intact for them.
Everything else in this branch is untouched: the reflow-gated RO pins
(11.5 -> 59fps drag), the structural/weight signature split, the adaptive
stream flush, the tree-split preview, and the tool-row memo boundaries.
* fix(sessions): verify fully reconstructed recovery
* test(desktop): wait for committed compress directive
* test(desktop): assert compress argument stage
* test(desktop): isolate compression from slash completion
* fix(desktop): keep pinned sidebar rows in user order
flattenSessionsWithBranches always re-sorted roots by last_active, so a
turn finishing floated background tasks over the hand-picked Pinned list
even though $pinnedSessionIds already stored drag order. preserveOrder
skips that sort for pins (and other non-date-grouped manual lists); default
recents stay recency-sorted for truthful date buckets.
* refactor(fallback): single owner for backend identity and failure-scoped skips
Every fallback/dedup/skip decision asks one question — 'is this candidate
the same backend as the one that failed, along the axis that failure
invalidated?' — but it was re-implemented inline at six sites across four
subsystems, each comparing whatever string was locally convenient. Each
incident fixed one site while the others kept the bug: NousResearch#22548, NousResearch#70893,
NousResearch#59561, NousResearch#72468, NousResearch#62984/NousResearch#54250/NousResearch#57584.
agent/backend_identity.py now owns the concept: BackendIdentity (provider /
model / base_url axes), FailureScope (MODEL / CREDENTIAL / ENDPOINT — each
failure class invalidates a different axis), and should_skip_candidate().
Unknown axes never manufacture a skip (over-skipping strands failover; a
wrong try costs one RTT).
Migrated sites:
- chat_completion_helpers.try_activate_fallback: replaces the provider+model
early-exit (the NousResearch#62984 bug: ignored base_url, stranding multi-endpoint
pools) AND _fallback_entry_is_same_backend_by_base_url (deleted)
- auxiliary_client._try_configured_fallback_chain +
_try_main_agent_model_fallback: replace label/model comparisons; auth and
payment map to CREDENTIAL scope, keeping the NousResearch#59561 carve-out
- hermes_cli/fallback_cmd add: primary-match + duplicate checks now identity-
aware (NousResearch#54250/NousResearch#57584): same provider+model on a different explicit
base_url is a pool entry, not a duplicate
_mark_provider_unhealthy stays label-keyed deliberately: its only triggers
are confirmed 402s, which ARE credential-scoped.
Owner-level tests pin each incident's semantics by number; sabotage-verified
(removing the base_url axis fails the NousResearch#62984 test).
* test(desktop): wait for backfill before the duplicate-count baseline
The large-session-resume E2E captured initialMockReplyCount immediately
after openSeededSession, which returns once the NEWEST turn is in the
viewport. With FIRST_PAINT_BUDGET=20 (lowered from 60 in this branch),
only the newest ~10 turns mount at first paint; the older turns
backfill in a rAF. The baseline was reading 10 instead of 27, so once
the backfill mounted the full 28 (27 seeded + 1 new), the test saw
"28 ≠ 11" and reported duplicates that were never there.
Wait for the oldest seeded turn to mount before taking the baseline.
This makes the count reflect the fully-mounted transcript regardless
of FIRST_PAINT_BUDGET, so the perf win (smaller first paint) and the
no-duplicate invariant both hold.
Refs NousResearch#72504
* perf(desktop): keep thread message component types stable across a session switch
* perf(desktop): bail the transcript out of router-driven re-renders on session switch
* fix(tui): paint the OSC-10 default foreground on quantizing terminals
A skin that authors a background paints both terminal defaults: OSC-11
for the backdrop, OSC-10 to re-base every default-fg token (markdown
body, borders, anything rendered without an explicit color) onto the
theme's text tone.
The OSC-10 half never fired on a limited-palette terminal.
`normalizeThemeForAnsiLightTerminal` rewrites the foreground tones to
`ansi256(N)`, and `setTerminalForeground` only accepts `#rrggbb` — so
the argument failed the hex test and the write was silently skipped.
The background moved to the skin while default-fg text stayed on the
host profile's foreground.
That split is the reported symptom: prose renders in the terminal's own
near-black while every themed token beside it renders the skin's gray,
so the base text color appears to change between adjacent words. A
resize repaints the affected cells from the screen buffer, which is why
the text "goes black" on resize and why the mix looks scattered rather
than uniform.
Resolve the tone through a new `themeToneHex` before handing it to
OSC-10: `ansi256(N)` maps through the xterm grayscale ramp and 6x6x6
cube, an authored hex passes through, and anything with no paintable
color yields '' (which correctly clears back to the terminal default).
Verified on Terminal.app + the `brooklyn` skin: `theme.color.text` is
`ansi256(238)`, previously dropped, now emitted as
`ESC]10;#444444 BEL` alongside the existing `ESC]11;#f6f9fd BEL`.
* fmt(js): `npm run fix` on merge (NousResearch#72522)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fmt(js): `npm run fix` on merge (NousResearch#72532)
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* ci: retrigger checks (GitHub Actions failed to resolve workflow file)
* chore: sync homelab branch with upstream main
---------
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
Co-authored-by: Tony Simons <asimons81@gmail.com>
Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
Co-authored-by: b <b@b>
Co-authored-by: hermes-seaeye[bot] <307254004+hermes-seaeye[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hermes Agent <hermes-agent@users.noreply.github.com>
…tration Follow-up on the salvaged #71610 commits: - acquire/release a scoped lock on relay_url+pubkey in connect/disconnect (IRC pattern) so two profiles can't drive one Buzz identity — duplicate replies and split de-dupe state; +2 tests - negative-cache _resolve_user_name failures so a profile-less pubkey doesn't re-hit 'users get' every poll sweep (flagged by @jethac on the PR) - register user-guide/messaging/buzz in website/sidebars.ts (page was unreachable — the #63359 trap)
…tration Follow-up on the salvaged #71610 commits: - acquire/release a scoped lock on relay_url+pubkey in connect/disconnect (IRC pattern) so two profiles can't drive one Buzz identity — duplicate replies and split de-dupe state; +2 tests - negative-cache _resolve_user_name failures so a profile-less pubkey doesn't re-hit 'users get' every poll sweep (flagged by @jethac on the PR) - register user-guide/messaging/buzz in website/sidebars.ts (page was unreachable — the #63359 trap)
…mon pool Follow-ups on the salvaged NousResearch#63359: - _finalize_child_results carries tool_call_history on subagent_stop (the NousResearch#62011/NousResearch#72403 field landed after the PR branched; the shared pipeline must emit it for both delegate_task and plugin-launched children). Lifecycle test updated for the new payload field. - The lifecycle executor uses DaemonThreadPoolExecutor — a wedged or abandoned child must never block interpreter exit at atexit-join time (same rationale as _run_single_child's timeout executor and the async-delegation pool). - delegate_task's batch path keeps live-transcript wiring while routing child construction through the shared _build_child_preserving_parent_tools helper.
…tration Follow-up on the salvaged NousResearch#71610 commits: - acquire/release a scoped lock on relay_url+pubkey in connect/disconnect (IRC pattern) so two profiles can't drive one Buzz identity — duplicate replies and split de-dupe state; +2 tests - negative-cache _resolve_user_name failures so a profile-less pubkey doesn't re-hit 'users get' every poll sweep (flagged by @jethac on the PR) - register user-guide/messaging/buzz in website/sidebars.ts (page was unreachable — the NousResearch#63359 trap)
Summary
Expose a typed, plugin-safe subagent lifecycle service through
PluginContext.subagent_lifecycle. Plugins can launch, monitor, cancel, and retrieve results from fresh Hermes child sessions without importing private delegation internals, gateway state, or TUI components.agent/subagent_lifecycle.py--SubagentLifecycleServicewith launch, status, bounded wait, cooperative cancellation, immutable terminal-result retrieval, and reconnect diagnosticsPluginContext.subagent_lifecycleproperty (hermes_cli/plugins.py, +17 lines) with lazy initialization and a parent-agent resolver lambdatests/agent/test_subagent_lifecycle.py-- 98 lines of contract/security testswebsite/docs/developer-guide/subagent-lifecycle-api.md-- public API documentationPublic API
The service is obtained from
PluginContext:SubagentHandleis fully serializable (to_dict()/from_dict()) and carries a versioned, opaque HMAC capability. Malformed or forged handles returnUNKNOWN/UNKNOWN_HANDLEand cannot access a child.Key request fields:
goalstrcontextOptional[str]metadataMapping[str, Any]allowed_toolsetsOptional[tuple[str, ...]]correlation_idOptional[str]Lifecycle semantics
States:
PENDING->STARTING->RUNNING->SUCCEEDED/FAILED/INTERRUPTED, withCANCEL_REQUESTEDas a transient during cooperative cancellation.UNKNOWNis returned for unrecognized or forged handles.cancel()is cooperative: it callsagent.interrupt()and returnsCANCEL_REQUESTED. The state only transitions toCANCELLEDafter terminal confirmation from the child loop.wait()blocks on the child'sFuturewith an optional timeout and returnsSubagentTerminalState.result()returns an immutableSubagentResultwith aresult_hash(SHA-256 of serialized payload). Repeated calls return the same instance.reconnect()returnsconnected=Truefor in-process handles that are still in the registry. After a process restart it returnsconnected=False, diagnostic="RECONNECT_UNAVAILABLE".Plugin usage
Compatibility
Existing
delegate_task, batch delegation, child tool restrictions, and TUI/gateway active-child display are unchanged. The service shares the internal child construction and execution path. It does not expose live agent objects, transcripts, or hidden reasoning.Security
SubagentLifecycleErrororUNKNOWNstate.UNKNOWNstate.parent_session_id.allowed_toolsetsto narrow a child.Validation
compileall)Known limitations
reconnect()returnsRECONNECT_UNAVAILABLEand never starts a replacement child.CANCEL_REQUESTEDuntil the child thread confirms the interrupt at its next safe boundary.tests/tools/test_search_hidden_dirs.py) invokes the Unix commandwhich rg.venv/bin/python.What reviewers should focus on
agent/subagent_lifecycle.py-- the entire new module. Check that the public contract is complete, the HMAC capability scheme is correct, the_validate_requestgates cover all injection surfaces, and the thread safety of_Registryis sound.hermes_cli/plugins.py-- thesubagent_lifecycleproperty. Confirm the lazy-init pattern and parent-agent resolver lambda are correct for the plugin lifecycle.tests/agent/test_subagent_lifecycle.py-- verify the contract tests cover launch, wait, result, cancel (cooperative), forged-handle rejection, cross-parent isolation, duplicate correlation IDs, toolset-broadening rejection, and in-process reconnect.website/docs/developer-guide/subagent-lifecycle-api.md-- confirm the documentation accurately reflects the implementation.