Skip to content

fix(a2a): orphan sweep honors A2A_REPLY_TIMEOUT - #106982

Closed
gaoanze888 wants to merge 6 commits into
NousResearch:mainfrom
gaoanze888:fix/a2a-orphan-sweep-reply-timeout
Closed

gaoanze888 wants to merge 6 commits into
NousResearch:mainfrom
gaoanze888:fix/a2a-orphan-sweep-reply-timeout

Conversation

@gaoanze888

Copy link
Copy Markdown

Summary

Fixes #106972. The A2A orphan-task sweep used a hardcoded _ORPHAN_TIMEOUT = 300 while the caller's reply deadline already honored A2A_REPLY_TIMEOUT via _reply_timeout(). A turn taking 350–600s was marked TASK_STATE_FAILED by the sweep before the configured reply deadline arrived, and once terminal the TaskStore.complete guard silently discarded the real (eventally-produced) reply — so raising A2A_REPLY_TIMEOUT had no effect, contradicting the docs that present it as the fix for "Replies time out on long tasks."

Root cause

  • plugins/platforms/a2a/adapter.py: _ORPHAN_TIMEOUT = 300 (hardcoded) fed self.tasks.fail_orphans(_ORPHAN_TIMEOUT) in _watchdog_loop. Separately, _reply_timeout() reads os.getenv("A2A_REPLY_TIMEOUT", "300") — but only for the active caller's own wait deadline, never for the orphan sweep.
  • plugins/platforms/a2a/protocol.py: fail_orphans marks any task older than timeout_seconds as STATE_FAILED; complete() then has if rec["state"] in TERMINAL_STATES: return None, so a later legitimate reply is silently dropped.

Fix

Extract the sweep into _sweep_orphans() and resolve the window via _reply_timeout() so the sweep window matches the configured reply deadline: a long-running turn survives long enough for its real reply to complete the task (the reply arrives → task goes STATE_COMPLETED → the sweep's not in TERMINAL_STATES guard skips it). _ORPHAN_TIMEOUT is removed; _WATCHDOG_INTERVAL is unchanged.

This is option (1) from @leyhux101-bit's report (make _ORPHAN_TIMEOUT respect A2A_REPLY_TIMEOUT). With it, the docs become accurate as written; no doc change is needed.

Tests

New TestOrphanSweepRespectsReplyTimeout in tests/plugins/test_a2a_plugin.py (4 tests):

Mutation-verified: reverting _sweep_orphans to the old orphan_timeout = 300 makes the regression test fail (400s-old task swept at 300 even with A2A_REPLY_TIMEOUT=600); restored, all 4 pass. Full a2a suite: 159 passed (4 new + 155 existing), 18 integration deselected. ruff check clean.

Note

Huge credit to @leyhux101-bit for the precise root-cause writeup (the split between _reply_timeout() and the hardcoded _ORPHAN_TIMEOUT is the whole story). The reporter offered to open this PR — I put it up to unblock the fix, but if you'd rather land your own implementation @leyhux101-bit I'm happy to close this in favor of yours.

The diff also carries unrelated .github/workflows changes — that's a fork/main ↔ origin/main sync artifact (my fork is behind on two workflow files and I lack a workflow-scope token to fully resync); the actual code change is only the two files listed above.

The watchdog orphan sweep used a hardcoded _ORPHAN_TIMEOUT = 300s while the
caller's reply deadline already honored A2A_REPLY_TIMEOUT via _reply_timeout().
A turn taking 350-600s was therefore marked TASK_STATE_FAILED by the sweep
before the configured reply deadline arrived, and once terminal the
TaskStore.complete guard silently discarded the real (eventually-produced)
reply — raising A2A_REPLY_TIMEOUT had no effect, contradicting the docs.

Extract the sweep into _sweep_orphans() and resolve the window via
_reply_timeout() so the sweep window matches the configured reply deadline:
a long-running turn survives long enough for its real reply to complete the
task. _ORPHAN_TIMEOUT is removed; _WATCHDOG_INTERVAL stays.

Fixes NousResearch#106972
@gaoanze888
gaoanze888 requested a review from a team September 9, 2026 23:56
@gaoanze888

Copy link
Copy Markdown
Author

Closing this as a duplicate — thanks to @alt-glitch's triage I see the orphan-sweep / reply-timeout mechanism already has three open PRs in flight:

Five PRs for a one-symbol fix is just noise for the maintainers, and #100312 is both first and more correct, so I'd rather not split reviewer attention. The four regression tests I added here (default-window sweep, the A2A_REPLY_TIMEOUT=600-preserves-a-400s-task regression, terminal-task skip, invalid-env fallback) are mutation-verified and available if @rtx9jckz2g2g-prog wants to lift them onto #100312 — but no obligation. Credit to @leyhux101-bit for the clean root-cause split between _reply_timeout() and the hardcoded _ORPHAN_TIMEOUT; that writeup is the whole story.

@gaoanze888 gaoanze888 closed this Sep 10, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation duplicate This issue or pull request already exists labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A2A: A2A_REPLY_TIMEOUT does not affect the orphan-task sweep (hardcoded 300s) — late replies get silently discarded

2 participants