Skip to content

fix(process_registry): keep poll() read-only — do not mark completions consumed - #31750

Closed
someaka wants to merge 2 commits into
NousResearch:mainfrom
someaka:fix/poll-completion-consumed
Closed

someaka wants to merge 2 commits into
NousResearch:mainfrom
someaka:fix/poll-completion-consumed

Conversation

@someaka

@someaka someaka commented May 25, 2026

Copy link
Copy Markdown

Summary

Keeps process_registry.poll() read-only for exited background processes.

poll() should report status, not mark a notify_on_complete result as consumed. Otherwise a normal status check can race with the gateway watcher and permanently suppress the synthetic completion event before it is delivered back to the chat.

Root cause

PR #8228 (f53a5a7f) added _completion_consumed tracking to avoid duplicate completion notifications. That behavior is correct for explicit consumption paths like wait() and read_log(), but not for poll():

  • wait() = blocking consume ("give me the result, I'm done")
  • read_log() = explicit consumption ("I read the output, I'm done")
  • poll() = read-only status query ("what's the current state?")

Treating poll() as consumption can cause a race:

  1. Agent starts a background process with notify_on_complete=true
  2. Gateway creates a watcher asyncio task (polls every 5s)
  3. Process exits
  4. Agent calls process(action='poll') on the next turn
  5. poll() marks _completion_consumed.add(session_id) (the bug)
  6. Watcher wakes up → checks is_completion_consumed() → returns Trueskips notification → breaks (never retries)

Changes

  • tools/process_registry.py: removed self._completion_consumed.add(session_id) from poll()
  • tests/tools/test_process_registry.py: added test_poll_does_not_mark_completion_consumed regression guard

Test plan

python -m pytest tests/tools/test_process_registry.py -x -q
65 passed in 3.86s

Closes #10156

Radical Edward added 2 commits May 25, 2026 02:49
Regression guard for NousResearch#10156. Ensures that poll() (a read-only status
query) does not have the side effect of consuming completion notifications,
which would silently suppress notify_on_complete watcher delivery.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels May 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Duplicate of #10158 (same fix for #10156). Previous duplicates by same author: #26659 (closed), also #21903 (closed).

@someaka

someaka commented May 30, 2026

Copy link
Copy Markdown
Author

Closing — duplicate of #10158 per @alt-glitch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: poll() marking _completion_consumed silently suppresses watcher notification

2 participants