Conversation
…ousResearch#73728) The backend emits terminal statuses including 'timeout' and 'error' in subagent.complete payloads, but asStatus() only recognised 'completed', 'failed', 'interrupted', and 'queued'. Unrecognised values fell through to 'running', making timed-out subagents immortal in the active status stack. Fix: map timeout/error to 'failed', cancelled/canceled to 'interrupted'. Nonterminal unknown statuses still default to 'running' for forward compatibility. Fixes NousResearch#73728
68c6985 to
276779a
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks — this addresses the verified current mismatch: tools/delegate_tool.py:2243-2251 emits subagent.complete with timeout or error, while current Desktop normalization in apps/desktop/src/store/subagents.ts:58-59 turns both into running.
Problems
apps/desktop/src/store/subagents.ts:62still maps every unknown status torunningwithout considering the event type. An unknown terminal value onsubagent.completewould therefore remain active and survive the running/queued-only prune atapps/desktop/src/store/subagents.ts:212. This is the future-terminal-status case called out in #73728.
Suggested changes
- Make normalization event-aware: map unknown values on
subagent.completetofailed, while retainingrunningfor unknown nonterminal events. Add the corresponding prune regression test.
Automated hermes-sweeper review.
| if (v === 'completed' || v === 'failed' || v === 'interrupted' || v === 'queued') return v | ||
| if (v === 'timeout' || v === 'error') return 'failed' | ||
| if (v === 'cancelled' || v === 'canceled') return 'interrupted' | ||
| return 'running' |
There was a problem hiding this comment.
asStatus() cannot distinguish a progress update from subagent.complete. Please map an unknown value on a completion event to failed (and test pruning it), while keeping unknown nonterminal events as running; otherwise a future terminal backend status recreates the stuck-active-row bug.
SummaryOne PR addresses issue #73728. #73859 fixes the reproduced Related pull requests
Suggested consolidationKeep #73859 open with a salvage path: retain its targeted terminal-status mappings and pruning regression coverage, then make normalization event-aware so an unknown status on Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I73728(["issue #73728 (open)"])
P73859["PR #73859 (open)"]
P73859 -->|best fix| I73728
class I73728 open
class P73859 open
class P73859 best
class P73859 target
click I73728 "https://github.com/NousResearch/hermes-agent/issues/73728"
click P73859 "https://github.com/NousResearch/hermes-agent/pull/73859"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 3 kB of PR diffs, 5 kB of issue/PR text, 3 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
… surface timeout reason Follow-up to the NousResearch#73728 normalization fix (supersedes the event-agnostic fallback the maintainers flagged as incomplete): - subagent.complete is terminal by definition — an unrecognized status on it now renders as 'failed' instead of falling through to 'running', which would recreate the immortal false-active row for any future backend status (the keep_open request on NousResearch#73859). - Live events keep the lenient 'running' fallback. - Synthesize a 'Timed out after Xs' summary from duration_seconds when the backend completes with status 'timeout' and no summary, so the failed row explains itself. - Tests: timeout reason synthesis + pruning, event-aware fail-closed vs lenient live fallback (13 total).
|
Thanks for the fix — it correctly identifies the normalization gap and maps the known statuses. To help move this forward following the maintainer review (which requested event-aware handling of unknown completion statuses), I've opened #85493 building on top of your work: your commit is cherry-picked in with authorship preserved, and it adds the event-aware fail-closed fallback, timeout summary synthesis, and tests. If you'd prefer to push updates directly to this branch instead, let me know and I'm glad to defer to your PR. |
… surface timeout reason Follow-up to the #73728 normalization fix (supersedes the event-agnostic fallback the maintainers flagged as incomplete): - subagent.complete is terminal by definition — an unrecognized status on it now renders as 'failed' instead of falling through to 'running', which would recreate the immortal false-active row for any future backend status (the keep_open request on #73859). - Live events keep the lenient 'running' fallback. - Synthesize a 'Timed out after Xs' summary from duration_seconds when the backend completes with status 'timeout' and no summary, so the failed row explains itself. - Tests: timeout reason synthesis + pruning, event-aware fail-closed vs lenient live fallback (13 total).
… surface timeout reason Follow-up to the NousResearch#73728 normalization fix (supersedes the event-agnostic fallback the maintainers flagged as incomplete): - subagent.complete is terminal by definition — an unrecognized status on it now renders as 'failed' instead of falling through to 'running', which would recreate the immortal false-active row for any future backend status (the keep_open request on NousResearch#73859). - Live events keep the lenient 'running' fallback. - Synthesize a 'Timed out after Xs' summary from duration_seconds when the backend completes with status 'timeout' and no summary, so the failed row explains itself. - Tests: timeout reason synthesis + pruning, event-aware fail-closed vs lenient live fallback (13 total).
… surface timeout reason Follow-up to the NousResearch#73728 normalization fix (supersedes the event-agnostic fallback the maintainers flagged as incomplete): - subagent.complete is terminal by definition — an unrecognized status on it now renders as 'failed' instead of falling through to 'running', which would recreate the immortal false-active row for any future backend status (the keep_open request on NousResearch#73859). - Live events keep the lenient 'running' fallback. - Synthesize a 'Timed out after Xs' summary from duration_seconds when the backend completes with status 'timeout' and no summary, so the failed row explains itself. - Tests: timeout reason synthesis + pruning, event-aware fail-closed vs lenient live fallback (13 total).
|
Landed on |
Summary
Fixes #73728. The backend emits terminal statuses including
timeoutanderrorinsubagent.completepayloads, butasStatus()only recognisedcompleted,failed,interrupted, andqueued. Unrecognised values fell through torunning, making timed-out subagents immortal in the active status stack.Changes
apps/desktop/src/store/subagents.ts:asStatus()now mapstimeout/error→failedandcancelled/canceled→interrupted. Nonterminal unknown statuses still default torunningfor forward compatibility.apps/desktop/src/store/subagents.test.ts: Added regression test covering all four backend terminal status strings (timeout,error,cancelled,canceled) with verification that they are recognised as terminal and pruned bypruneFinishedSessionSubagents().Root cause
The backend's
delegate_tool.pyusestimeoutas a terminal status, and error paths useerror. Both were normalised torunning, sopruneFinishedSessionSubagents()kept them forever and they rendered as active in the composer status stack.Test plan