Conversation
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the outer-runner lifecycle gap. The premise remains valid on current main: tools/async_delegation.py:722-745 only finalizes after the detached batch runner returns, while tools/delegate_tool.py:1917-1956 applies child_timeout_seconds only to the inner child future.
Problems
- This branch predates current durable completion handling. Current
tools/async_delegation.py:565-584keeps recordsfinalizingthrough terminal persistence, andtools/async_delegation.py:634persists before queue publication. The watchdog must preserve that lifecycle rather than use the older direct terminal-state flow. - The new tests do not verify a timeout is durable and restart-recoverable, despite
tools/async_delegation.py:266-290restoring pending terminal events.
Suggested changes
- Conflict-resolve the watchdog into the current finalization/persistence path and retain exactly-once terminal delivery.
- Add an isolated durable-timeout/restart recovery test.
Automated hermes-sweeper review.
teknium1
added
sweeper:risk-session-state
Async background delegation can leave gateway sessions holding only a dispatched handle when the detached runner wedges before it can return and enqueue a completion. Enforce the configured child timeout in the async registry so the parent observes a terminal timeout event and the async slot is released. Constraint: Issue NousResearch#60203 reports long-lived gateway processes with background child delegates that never produce completion events despite child_timeout_seconds being configured. Rejected: Relying only on _run_single_child timeout handling | it cannot finalize the async registry when the outer runner thread itself never reaches normal completion. Confidence: high Scope-risk: narrow Directive: Keep background delegation completion owned by the async registry whenever detached workers can outlive the caller's immediate control. Tested: .venv/bin/python -m pytest tests/tools/test_async_delegation.py tests/tools/test_delegate_subagent_timeout_diagnostic.py tests/tools/test_delegate.py -q Tested: .venv/bin/python -m ruff check tools/async_delegation.py tools/delegate_tool.py tests/tools/test_async_delegation.py Tested: git diff --check Not-tested: Multi-day real gateway degradation; covered with deterministic stuck-runner registry tests.
556e08d to
5a3e153
Compare
|
Addressed in 5a3e153. |
|
Thanks for this fix @izumi0uu — the approach is exactly right. Cross-vendor review resultsI ran this through cross-vendor review (Gemini 3.6 Flash + GPT-OSS): Gemini Flash: All 7 review questions rated ACCEPTABLE. The atomic GPT-OSS: 6/7 ACCEPTABLE/NIT. One SHOULD-FIX about resource cleanup when the interrupted worker holds locks — however this is about pre-existing interrupt behavior in Merge conflictsThe original branch had conflicts with current If you'd prefer to rebase your own branch instead, the conflicts are trivial — both are additive parameter additions that need to coexist. Happy to close #71096 if you push a rebase. |
Thanks for taking this forward. I’m happy for #71096 to supersede this PR. Since it preserves the original authorship, resolves the conflicts against current main, and passes the targeted tests, there’s no need for me to duplicate the rebase work here. Please feel free to close #60234 as superseded once #71096 lands. |
…nners Replace the wall-clock timeout watchdog (from #60234) with progress-based staleness detection, on by default with zero config: - The async registry now accepts a progress_fn per dispatch; delegate_task wires a sampler over the batch's child agents (api_call_count + current_tool from get_activity_summary()). - A single monitor thread sweeps running delegations: a child whose progress token keeps advancing is never touched, no matter how long it runs. A frozen token past the stale threshold (450s idle / 1200s in-tool, mirroring the sync-path heartbeat monitor) marks the record 'stalling' and interrupts the child. - A stalling child that unwinds within the grace window (120s) finalizes through the NORMAL path, preserving its partial results. One that never returns is force-finalized with a terminal 'stalled' completion event so the owning session hears an outcome and the async slot frees. - Late runner returns after force-finalization are deduped by the begin/push/finish finalization split (kept from #60234). Why not a timeout: delegation.child_timeout_seconds defaults to 0 by deliberate design (DEFAULT_CHILD_TIMEOUT rationale) — a timeout-based watchdog never arms for default configs, leaving the reported silent- profile symptom (#60203) unfixed, and when armed it kills legitimately slow heavy subagents mid-task. Progress detection distinguishes 'wedged at first API call' from 'grinding through a 2h review'. Builds on izumi0uu's finalization-atomicity work from #60234.
|
Merged via PR #72227 — thank you for this fix! Your finalization-atomicity work (the atomic running→finalizing claim and late-return dedup) was cherry-picked onto current main with your authorship preserved in git history (commit 65420cd). During review we replaced the wall-clock timeout watchdog half with progress-based staleness detection: |
Emit and persist completed concurrent tool results while the rest of the batch is still running. This reduces head-of-line blocking where one wedged tool hides already-finished tool output from the TUI/session DB until timeout. Refs upstream tool-hang/session-state issues: - NousResearch#57765 - NousResearch#60234 - NousResearch#57180 - NousResearch#60183 - NousResearch#39737 (cherry picked from commit 13d60bd7f26d9ad0ceec79d5b2717ad08bae5ae1) (cherry picked from commit f62b365e9cd32f739ca18545f8285428003fbffd)
…nners Replace the wall-clock timeout watchdog (from NousResearch#60234) with progress-based staleness detection, on by default with zero config: - The async registry now accepts a progress_fn per dispatch; delegate_task wires a sampler over the batch's child agents (api_call_count + current_tool from get_activity_summary()). - A single monitor thread sweeps running delegations: a child whose progress token keeps advancing is never touched, no matter how long it runs. A frozen token past the stale threshold (450s idle / 1200s in-tool, mirroring the sync-path heartbeat monitor) marks the record 'stalling' and interrupts the child. - A stalling child that unwinds within the grace window (120s) finalizes through the NORMAL path, preserving its partial results. One that never returns is force-finalized with a terminal 'stalled' completion event so the owning session hears an outcome and the async slot frees. - Late runner returns after force-finalization are deduped by the begin/push/finish finalization split (kept from NousResearch#60234). Why not a timeout: delegation.child_timeout_seconds defaults to 0 by deliberate design (DEFAULT_CHILD_TIMEOUT rationale) — a timeout-based watchdog never arms for default configs, leaving the reported silent- profile symptom (NousResearch#60203) unfixed, and when armed it kills legitimately slow heavy subagents mid-task. Progress detection distinguishes 'wedged at first API call' from 'grinding through a 2h review'. Builds on izumi0uu's finalization-atomicity work from NousResearch#60234.
…nners Replace the wall-clock timeout watchdog (from NousResearch#60234) with progress-based staleness detection, on by default with zero config: - The async registry now accepts a progress_fn per dispatch; delegate_task wires a sampler over the batch's child agents (api_call_count + current_tool from get_activity_summary()). - A single monitor thread sweeps running delegations: a child whose progress token keeps advancing is never touched, no matter how long it runs. A frozen token past the stale threshold (450s idle / 1200s in-tool, mirroring the sync-path heartbeat monitor) marks the record 'stalling' and interrupts the child. - A stalling child that unwinds within the grace window (120s) finalizes through the NORMAL path, preserving its partial results. One that never returns is force-finalized with a terminal 'stalled' completion event so the owning session hears an outcome and the async slot frees. - Late runner returns after force-finalization are deduped by the begin/push/finish finalization split (kept from NousResearch#60234). Why not a timeout: delegation.child_timeout_seconds defaults to 0 by deliberate design (DEFAULT_CHILD_TIMEOUT rationale) — a timeout-based watchdog never arms for default configs, leaving the reported silent- profile symptom (NousResearch#60203) unfixed, and when armed it kills legitimately slow heavy subagents mid-task. Progress detection distinguishes 'wedged at first API call' from 'grinding through a 2h review'. Builds on izumi0uu's finalization-atomicity work from NousResearch#60234.
Emit and persist completed concurrent tool results while the rest of the batch is still running. This reduces head-of-line blocking where one wedged tool hides already-finished tool output from the TUI/session DB until timeout. Refs upstream tool-hang/session-state issues: - NousResearch#57765 - NousResearch#60234 - NousResearch#57180 - NousResearch#60183 - NousResearch#39737 (cherry picked from commit 13d60bd7f26d9ad0ceec79d5b2717ad08bae5ae1)
Emit and persist completed concurrent tool results while the rest of the batch is still running. This reduces head-of-line blocking where one wedged tool hides already-finished tool output from the TUI/session DB until timeout. Refs upstream tool-hang/session-state issues: - NousResearch#57765 - NousResearch#60234 - NousResearch#57180 - NousResearch#60183 - NousResearch#39737 (cherry picked from commit 13d60bd7f26d9ad0ceec79d5b2717ad08bae5ae1)
…nners Replace the wall-clock timeout watchdog (from NousResearch#60234) with progress-based staleness detection, on by default with zero config: - The async registry now accepts a progress_fn per dispatch; delegate_task wires a sampler over the batch's child agents (api_call_count + current_tool from get_activity_summary()). - A single monitor thread sweeps running delegations: a child whose progress token keeps advancing is never touched, no matter how long it runs. A frozen token past the stale threshold (450s idle / 1200s in-tool, mirroring the sync-path heartbeat monitor) marks the record 'stalling' and interrupts the child. - A stalling child that unwinds within the grace window (120s) finalizes through the NORMAL path, preserving its partial results. One that never returns is force-finalized with a terminal 'stalled' completion event so the owning session hears an outcome and the async slot frees. - Late runner returns after force-finalization are deduped by the begin/push/finish finalization split (kept from NousResearch#60234). Why not a timeout: delegation.child_timeout_seconds defaults to 0 by deliberate design (DEFAULT_CHILD_TIMEOUT rationale) — a timeout-based watchdog never arms for default configs, leaving the reported silent- profile symptom (NousResearch#60203) unfixed, and when armed it kills legitimately slow heavy subagents mid-task. Progress detection distinguishes 'wedged at first API call' from 'grinding through a 2h review'. Builds on izumi0uu's finalization-atomicity work from NousResearch#60234.
4 participants
What does this PR do?
Fixes a stuck lifecycle path for
delegate_task(background=true): if the detached async delegation runner wedges before it returns and enqueues a completion event, gateway sessions can keep seeing only the original dispatched handle while the async slot remains occupied.This PR makes the async delegation registry enforce the existing
delegation.child_timeout_secondssetting for detached runners. When a still-running async delegation exceeds that timeout, the registry now emits one terminaltimeoutcompletion event, releases the async slot, drops the interrupt closure, and best-effort interrupts the detached child. Late worker returns are ignored so the parent cannot receive duplicate completion events.Related Issue
Fixes #60203
Type of Change
Changes Made
tools/async_delegation.py: add per-dispatch timeout watchdogs for single and batch async delegations.tools/async_delegation.py: make normal finalizers no-op after timeout/interruption so a late runner cannot enqueue a second completion event.tools/delegate_tool.py: pass the configured child timeout into the background async registry path.tests/tools/test_async_delegation.py: cover stuck single runners, stuck batch runners, and the delegate-tool timeout handoff.How to Test
.venv/bin/python -m pytest tests/tools/test_async_delegation.py tests/tools/test_delegate_subagent_timeout_diagnostic.py tests/tools/test_delegate.py -q.venv/bin/python -m ruff check tools/async_delegation.py tools/delegate_tool.py tests/tools/test_async_delegation.pygit diff --check upstream/main...HEADChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Targeted validation: