feat(delegate): expose redacted child tool history - #62011
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the existing child trace instead of adding a second tracker. The premise holds on current main: tools/delegate_tool.py:2061-2093 builds tool_trace, while the subagent_stop call at tools/delegate_tool.py:2717-2726 does not expose it.
Problems
tools/delegate_tool.py:336rebuilds a sanitized URL withparsed.netloc.urlsplit()includes URL userinfo innetloc, sohttps://user:password@example.test/pathwould forwarduser:passwordto the hook history. The shell bridge serializes extra hook kwargs unchanged atagent/shell_hooks.py:539-552, violating the stated credential-redaction boundary.
Suggested changes
- Reconstruct the URL from a credential-free host and permitted port, or reject URLs with userinfo; add coverage that verifies userinfo is absent from both history and the shell-hook payload.
Automated hermes-sweeper review.
4862926 to
7fe8ba8
Compare
|
Addressed in 7fe8ba8. URL targets are now rebuilt from the parsed hostname plus validated port, so |
|
Merged via PR #72403 — thank you! Both your commits (the feature and the URL-userinfo redaction follow-up) were cherry-picked onto current main with your authorship preserved in git history. |
…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.
* 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>
…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.
Summary
tool_call_historyon everysubagent_stophook payloadPrivacy and trust boundary
The hook payload does not contain raw prompts, shell commands, file contents, headers, credentials, tool arguments, or tool results.
tool_inputcontains only top-level argument names and a bounded allowlist of side-effect targets such as file paths, directories, working directories, and URLs. URL query strings, fragments, anduser:password@userinfo are removed. URL authorities are rebuilt from the parsed hostname and validated port before entering hook-visible history. The hook boundary revalidates that summary instead of trusting arbitrary fields added totool_trace.This lets governance hooks compare claims such as writing a specific artifact against actual tool targets without turning the lifecycle event into a raw data sink.
Current-main proof
Before this change, the new regression failed with
KeyError: tool_call_history:delegate_taskbuilttool_tracebut dropped it from thesubagent_stopinvocation.After the change, hooks receive the redacted ordered history, raw injected fields are excluded, and a mutating hook cannot change the child result returned to the parent.
Duplicate / sibling-path audit
Reviewed hook registration PRs #61315, #61823, and #39751 plus draft execution-receipts PR #49371. Those address hook availability or a separate opt-in durable receipt stream. None exposes the existing child tool trace in the
subagent_stoplifecycle payload.Verification
tests/agent/test_subagent_stop_hook.py+tests/tools/test_delegate.py: 166 passed after URL-userinfo hardeninggit diff --checkpassedNo live Hermes state, child processes, providers, or credentials were used.
Fixes #61974