Skip to content

fix: cancel/time-bound in-flight gateway agent turns - #3472

Merged
MervinPraison merged 2 commits into
mainfrom
claude/issue-3467-20260728-0912
Jul 29, 2026
Merged

fix: cancel/time-bound in-flight gateway agent turns#3472
MervinPraison merged 2 commits into
mainfrom
claude/issue-3467-20260728-0912

Conversation

@praisonai-triage-agent

@praisonai-triage-agent praisonai-triage-agent Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #3467

Summary

The gateway had no way to stop an agent turn already running, and no per-turn timeout — a single slow tool call or doom-looping agent wedged the serial per-session queue indefinitely, with no recourse short of restarting the process. Core already supplied the contract (EventType.MESSAGE_ABORT) and the primitive (InterruptController); this wires them into the gateway.

Changes

  • Advertise abort in the hello capability handshake (methods: ["message","leave","abort"] + message_abort event), replacing the # abort not implemented placeholder.
  • Handle abort: new abort/message_abort WS frame branch (WRITE scope) and a portable /stop (or stop) chat command. Both call _abort_active_turn, which drives the existing InterruptController and cancels the driving asyncio.Task.
  • Per-turn timeout: each turn now runs via _drive_turn, registered in a new _active_turns registry and awaited under asyncio.wait_for(...) when gateway.per_turn_timeout > 0. Cancelled/timed-out turns are normalised to a typed terminal message instead of hanging or surfacing a raw traceback.
  • Config knob: added gateway.per_turn_timeout (float seconds, 0 = disabled) to core GatewayConfig — dataclass field, validation, from_dict, and to_dict.

Reuses agent.interrupt_controller (already checked by the agent run loop); no new agent params, keeping the surface lightweight.

Layer placement

  • Wrapper (praisonai_bot gateway server): abort handling, /stop, _active_turns, _drive_turn, timeout.
  • Core (praisonaiagents): only the per_turn_timeout config field — protocol/config surface, no heavy implementation.

Testing

  • All 70 core gateway/config tests pass (tests/**/test_gateway*).
  • Added inline functional verification of both the timeout path (auto-cancel after per_turn_timeout) and the abort path (/stop), confirming the _active_turns registry is cleaned up.

Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added configurable per-turn timeouts for agent responses, disabled by default.
    • Added cooperative cancellation for in-progress agent turns.
    • Users can stop active turns with /stop or supported abort messages.
    • The gateway now reports whether abort controls are available and returns clear cancellation outcomes.
  • Bug Fixes

    • Added validation to prevent negative timeout values.
    • Timed-out or cancelled turns now end with a clear status message.

Wire per-turn abort + timeout into the gateway so a runaway turn can no
longer wedge the serial per-session queue:

- Advertise the `abort` method (+ `message_abort` event) in the hello
  capability handshake instead of the "abort not implemented" placeholder.
- Handle `abort`/`message_abort` WS frames and a portable `/stop` chat
  command; both drive the existing InterruptController and cancel the
  driving asyncio task via a new `_active_turns` registry.
- Run each turn under an optional `gateway.per_turn_timeout` (YAML/config
  knob, 0 = disabled) via `_drive_turn`, normalising cancelled/timed-out
  turns to a typed terminal message instead of leaving them hanging.

Reuses core's InterruptController (already checked by the agent run loop)
and `agent.interrupt_controller`; no new agent params.

Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
@MervinPraison

Copy link
Copy Markdown
Owner

@coderabbitai review

@MervinPraison

Copy link
Copy Markdown
Owner

/review

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@MervinPraison MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:manual-review Blocked: requires manual review pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cc641a1c-bd0b-4b04-8113-12cb912db3bc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The gateway adds a configurable per-turn timeout, tracks active turns, advertises abort support, handles /stop and abort frames, and cancels turns through InterruptController and asyncio task cancellation.

Changes

Gateway turn control

Layer / File(s) Summary
Per-turn timeout configuration
src/praisonai-agents/praisonaiagents/gateway/config.py
Adds the per_turn_timeout field, validates non-negative values, serializes it, and loads it from gateway configuration data.
Interruptible turn execution
src/praisonai-bot/praisonai_bot/gateway/server.py
Tracks active turns, injects interruption controllers, and applies cooperative cancellation and per-turn timeout handling.
Abort protocol surface
src/praisonai-bot/praisonai_bot/gateway/server.py
Advertises abort capability and handles /stop, abort, and message_abort requests with aborted or no_active_turn outcomes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Gateway
  participant SessionQueue
  participant InterruptController
  participant AgentTurn
  Client->>Gateway: Send abort request
  Gateway->>InterruptController: Request interruption
  Gateway->>AgentTurn: Cancel driving task
  SessionQueue->>AgentTurn: Finalize cancelled turn
  AgentTurn-->>Gateway: Return terminal response
  Gateway-->>Client: Send aborted outcome
Loading

Possibly related PRs

Suggested labels: pipeline/reviews-pending

Suggested reviewers: mervinpraison

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: canceling and time-bounding in-flight gateway agent turns.
Linked Issues check ✅ Passed The changes align with #3467 by adding abort handling, /stop support, InterruptController cancellation, and per-turn timeouts.
Out of Scope Changes check ✅ Passed The changes stay focused on gateway turn cancellation/timeouts and the needed config surface; no unrelated scope is evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-3467-20260728-0912

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread src/praisonai-agents/praisonaiagents/gateway/config.py
Comment thread src/praisonai-bot/praisonai_bot/gateway/server.py Outdated
Comment thread src/praisonai-bot/praisonai_bot/gateway/server.py Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds cancellable and time-bounded gateway turns.

  • Advertises and handles abort frames and portable stop commands.
  • Tracks active turns and introduces cooperative cancellation with a bounded settlement period.
  • Adds and propagates the gateway.per_turn_timeout configuration setting.

Confidence Score: 3/5

The PR is not yet safe to merge because sync executor turns can outlive cancellation, and the compatibility fallback can duplicate a turn while restoring shared cancellation state.

A timed-out executor-backed chat leaves its worker running while the session queue advances, and an internal TypeError from an entry point causes the gateway to retry the full turn after assigning its controller to the shared agent.

Files Needing Attention: src/praisonai-bot/praisonai_bot/gateway/server.py

Important Files Changed

Filename Overview
src/praisonai-bot/praisonai_bot/gateway/server.py Adds abort and timeout orchestration, but unrestricted TypeError fallbacks can rerun turns and the executor cancellation path still advances before the worker stops.
src/praisonai-bot/praisonai_bot/bots/_config_schema.py Extends validated gateway YAML with a non-negative per-turn timeout field.
src/praisonai-agents/praisonaiagents/gateway/config.py Adds validated parsing and serialization for the per-turn timeout setting.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Gateway
  participant Queue
  participant Agent
  Client->>Gateway: message
  Gateway->>Queue: enqueue turn
  Queue->>Agent: run with cancel_token
  alt abort or timeout
    Gateway->>Agent: request interruption
    Gateway->>Queue: await bounded settlement
  end
  Agent-->>Queue: result or cancellation
  Queue-->>Client: terminal response
Loading

Reviews (2): Last reviewed commit: "fix: reach + isolate + settle gateway pe..." | Re-trigger Greptile

@MervinPraison

Copy link
Copy Markdown
Owner

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

  1. Protocol-driven: check heavy implementations vs core SDK
  2. Backward compatible: ensure zero feature regressions
  3. Performance: no hot-path regressions
  4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
  5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
  6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

@MervinPraison MervinPraison added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:cooldown Blocked: post-push or @claude cooldown and removed pipeline/final-claude-pending Reviews done; waiting for FINAL @claude pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet labels Jul 28, 2026
@praisonai-triage-agent

praisonai-triage-agent Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Claude finished @MervinPraison's task —— View job


Final Architecture Review — PR #3472 ✅ Fixes pushed

Reviewed against AGENTS.md and validated all three P1 issues raised by Greptile. All three were valid and are now fixed on this branch (aa37915).

Phase 1 — Architecture (per AGENTS.md)

  • Protocol-driven ✅ — Core praisonaiagents only gains the per_turn_timeout config field (dataclass/validation/from_dict/to_dict). All heavy behaviour (abort handling, _active_turns, _drive_turn, timeout) stays in the praisonai-bot wrapper. Correct layer split.
  • No Agent bloat ✅ — Reuses the existing cancel_token param on chat/achat and the existing InterruptController. Zero new Agent params.
  • Backward compatible ✅per_turn_timeout defaults to 0 (disabled); abort is additive. No regressions in the 70 core gateway/config tests.
  • SDK value ✅ — Genuinely closes a real hole (a runaway turn wedging the serial per-session queue with no recourse). Not scope creep.
  • Routing ✅ — Nothing misrouted to tools/plugins.

Phase 2 — Fixes implemented

# Issue (reviewer) Verdict Fix
1 Timeout config unreachable (Greptile P1) ✅ Valid Added per_turn_timeout to bot GatewayServerSchema (extra="forbid" was rejecting the YAML key) and stamped it onto self.config in start_with_config() — the constructor-built config never carried it, so the documented knob was silently disabled.
2 Shared controller breaks turn isolation (Greptile P1) ✅ Valid _dispatch_agent_turn now passes the InterruptController per-turn as cancel_token instead of mutating the shared agent.interrupt_controller. One Agent instance serving overlapping sessions no longer has one session's abort/timeout hit another. Legacy agents without a cancel_token kwarg fall back to the old shared-attribute stamp.
3 Executor turns survive cancellation (Greptile P1) ✅ Valid (bounded) Cancellation is now cooperative-first: the controller is request()ed and the turn is given a bounded grace window (_ABORT_GRACE_SECONDS) to actually unwind — including a sync agent.chat in a worker thread that Python cannot force-kill — before the serial queue advances. Hard task.cancel() remains only as a fallback for a genuinely stuck turn. This prevents a timed-out sync turn from mutating shared agent state concurrently with the next turn. (A worker thread still cannot be forcibly reclaimed — that's a Python limitation — but we no longer claim clean cancellation while racing the next turn.)

Verification

  • ✅ 70/70 core gateway + config tests pass.
  • ✅ Core config roundtrip + negative-value validation.
  • ✅ Bot schema accepts per_turn_timeout, rejects negatives, still forbids unknown keys.
  • ✅ Functional test: cancel_token is passed per-turn and the shared agent.interrupt_controller is not clobbered; legacy fallback path confirmed.

Files modified

  • src/praisonai-bot/praisonai_bot/bots/_config_schema.pyper_turn_timeout field.
  • src/praisonai-bot/praisonai_bot/gateway/server.py — schema propagation in start_with_config, per-turn cancel_token isolation in _dispatch_agent_turn, bounded cooperative settle in _drive_turn/_abort_active_turn (new _settle_cancelled_turn).

Skipped: no core-SDK changes needed beyond the existing config field; no praisonai-rust/praisonai-ts touched (out of scope).

Phase 3 — Verdict

Approve. All P1 blocking issues resolved; architecture is lightweight, protocol-driven, and backward-compatible. Note: I cannot submit a formal GitHub approval (permissions), so this comment is the review sign-off.
• Branch: claude/issue-3467-20260728-0912

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/praisonai-bot/praisonai_bot/gateway/server.py (1)

7519-7535: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Major: per_turn_timeout is never wired from gateway.yaml into self.config, so the feature is inert for the primary runtime path.

start_with_config builds self.config in __init__ as GatewayConfig(host=host, port=port) (default per_turn_timeout=0.0) and only patches specific keys from the parsed gw_cfg dict onto it here (max_buffered_bytes, max_queued_frames, notify_on_undelivered, undelivered_template, etc.). per_turn_timeout is not among them, even though _run_session_queue (line 3195) reads getattr(self.config, "per_turn_timeout", 0.0). MultiChannelGatewayConfig.from_dict in config.py correctly parses per_turn_timeout from YAML, but that code path is not used by start_with_config (which loads a raw dict via load_gateway_config, not MultiChannelGatewayConfig.from_dict). As a result, any operator setting gateway.per_turn_timeout in gateway.yaml will see it silently ignored — timeouts stay permanently disabled.

🐛 Suggested fix
         if "max_queued_frames" in gw_cfg:
             self.config.max_queued_frames = int(gw_cfg["max_queued_frames"])
+        if "per_turn_timeout" in gw_cfg:
+            self.config.per_turn_timeout = float(gw_cfg["per_turn_timeout"] or 0.0)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/praisonai-bot/praisonai_bot/gateway/server.py` around lines 7519 - 7535,
Wire the validated gateway configuration’s per_turn_timeout into self.config
within start_with_config, alongside the existing max_buffered_bytes and
max_queued_frames assignments. Read the value from gw_cfg, convert it to the
expected numeric type, and preserve the default behavior when the key is absent
so _run_session_queue receives the configured timeout.
🧹 Nitpick comments (1)
src/praisonai-bot/praisonai_bot/gateway/server.py (1)

3102-3124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Minor: silent exception swallowing in _abort_active_turn without logging.

Both try/except Exception: pass blocks (controller.request, task.cancel) swallow any failure silently, per Ruff S110/BLE001. If either fails (e.g. a buggy interrupt controller or a task in an unexpected state), an abort request would silently no-op with no trace to debug. Consider at least a logger.debug(...) on the except branches.

🩹 Suggested fix
         try:
             if controller is not None:
                 controller.request(reason)
-        except Exception:
-            pass
+        except Exception as e:
+            logger.debug("Failed to request interrupt for session %s: %s", session_id, e)
         try:
             if task is not None and not task.done():
                 task.cancel()
-        except Exception:
-            pass
+        except Exception as e:
+            logger.debug("Failed to cancel turn task for session %s: %s", session_id, e)
As per static analysis hints (Ruff S110/BLE001: "`try`-`except`-`pass` detected, consider logging the exception").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/praisonai-bot/praisonai_bot/gateway/server.py` around lines 3102 - 3124,
Update both exception handlers in _abort_active_turn to log the caught exception
at debug level, including whether controller.request or task.cancel failed and
the session context, while preserving the current best-effort abort behavior and
return value.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/praisonai-bot/praisonai_bot/gateway/server.py`:
- Around line 3126-3167: Update the asyncio.CancelledError handler in
_drive_turn to distinguish external cancellation from requested turn
interruption: if controller.is_set() is false, re-raise the cancellation;
otherwise preserve the existing _finalise_aborted_turn behavior using the
controller reason.
- Around line 3077-3100: Update _dispatch_agent_turn to avoid assigning the
per-turn interrupt controller to the shared agent.interrupt_controller. Use a
per-turn agent clone or explicit per-turn state so concurrent sessions retain
independent controllers, while preserving native async dispatch and the
synchronous agent.chat executor fallback.

---

Outside diff comments:
In `@src/praisonai-bot/praisonai_bot/gateway/server.py`:
- Around line 7519-7535: Wire the validated gateway configuration’s
per_turn_timeout into self.config within start_with_config, alongside the
existing max_buffered_bytes and max_queued_frames assignments. Read the value
from gw_cfg, convert it to the expected numeric type, and preserve the default
behavior when the key is absent so _run_session_queue receives the configured
timeout.

---

Nitpick comments:
In `@src/praisonai-bot/praisonai_bot/gateway/server.py`:
- Around line 3102-3124: Update both exception handlers in _abort_active_turn to
log the caught exception at debug level, including whether controller.request or
task.cancel failed and the session context, while preserving the current
best-effort abort behavior and return value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b1484562-170c-47a5-9b30-a9eea64805d0

📥 Commits

Reviewing files that changed from the base of the PR and between b43688a and 8cbc4f4.

📒 Files selected for processing (2)
  • src/praisonai-agents/praisonaiagents/gateway/config.py
  • src/praisonai-bot/praisonai_bot/gateway/server.py

Comment on lines 3077 to 3100
async def _dispatch_agent_turn(
agent: Any, content: str, interrupt: Any = None
) -> Any:
"""Execute a single agent turn.

Prefers the agent's native async entry point (``arun``/``achat``) so
turns run directly on the event loop, giving native asyncio concurrency,
cleaner cancellation/timeout and true async streaming. Falls back to
offloading the synchronous ``chat`` onto the default thread pool only
when no async entry point is available (sync-only agents).

Issue #3467: when ``interrupt`` is supplied it is stamped onto the agent
(``agent.interrupt_controller``) so the agent's run loop cooperatively
stops on the next checkpoint; the caller also cancels the driving task
for a hard stop when the agent does not yield promptly.
"""
if interrupt is not None and hasattr(agent, "interrupt_controller"):
agent.interrupt_controller = interrupt
for _name in ("arun", "achat"):
_fn = getattr(agent, _name, None)
if _fn is not None and asyncio.iscoroutinefunction(_fn):
return await _fn(content)
loop = asyncio.get_running_loop()
return await loop.run_in_executor(None, agent.chat, content)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

Does praisonaiagents Agent.arun/achat accept a cancel_token or interrupt parameter?

💡 Result:

No, the praisonaiagents Agent.arun and Agent.achat methods do not explicitly accept a cancel_token or interrupt parameter in their function signatures [1][2]. While the Agent class does maintain an internal interrupt_controller (an InterruptController instance) intended for cooperative cancellation [3], this controller is typically initialized at the agent level rather than being passed as an argument to individual execution methods like arun() or achat() [2][3]. The public method signatures for these functions are focused on prompt processing and task configuration (e.g., temperature, tools, output_json) rather than low-level interrupt or cancellation tokens [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate relevant files =="
git ls-files | rg '(^src/praisonai-agents/praisonaiagents/agent/.*\.py$|^src/praisonai-agents/.*/.*\.py$|^src/praisonai-bot/praisonai_bot/gateway/server.py$|^src/praisonai-agents/praisonaiagents/agent/chat_mixin\.py$)' || true

echo
echo "== server.py relevant sections =="
wc -l src/praisonai-bot/praisonai_bot/gateway/server.py
sed -n '3050,3125p' src/praisonai-bot/praisonai_bot/gateway/server.py
echo
sed -n '7485,7580p' src/praisonai-bot/praisonai_bot/gateway/server.py

echo
echo "== agent-related signatures/usages =="
rg -n "class Agent|def arun|def achat|def chat|interrupt_controller|cancel_token|clone_for_channel" src/praisonai-agents src/praisonai-bot/praisonai_bot/gateway/server.py | head -n 200

echo
echo "== relevant agent.py sections =="
# locate agent.py without assuming path
for f in $(git ls-files | rg '/praisonaiagents/agent/agent\.py$'); do
  echo "--- $f"
  wc -l "$f"
  sed -n '1,220p' "$f"
  sed -n '350,600p' "$f"
done

echo
echo "== chat_mixin.py relevant sections =="
f=$(git ls-files | rg '/chat_mixin\.py$' | head -n1 || true)
if [ -n "$f" ]; then
  echo "--- $f"
  wc -l "$f"
  sed -n '1,260p' "$f"
fi

Repository: MervinPraison/PraisonAI

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== server dispatch/agent resolution =="
wc -l src/praisonai-bot/praisonai_bot/gateway/server.py
sed -n '3000,3105p' src/praisonai-bot/praisonai_bot/gateway/server.py
rg -n "_process_agent_message|_dispatch_agent_turn|self\\._agents\\.get|agent\\.interrupt_controller|agent\\.clone_for_channel" src/praisonai-bot/praisonai_bot/gateway/server.py | head -n 80

echo
echo "== agent.py class/init/run/chat/signatures =="
wc -l src/praisonai-agents/praisonaiagents/agent/agent.py
rg -n "class Agent|def run\\(|def run_async\\(|def chat\\(|def arun\\(|def achat\\(|def clone\\(|def clone_for_channel\\(|interrupt_controller|cancel_token" src/praisonai-agents/praisonaiagents/agent/agent.py src/praisonai-agents/praisonaiagents/agent/interrupt.py src/praisonai-agents/praisonaiagents/agent/chat_mixin.py | head -n 160

echo
echo "== focused agent.py sections =="
sed -n '1,360p' src/praisonai-agents/praisonaiagents/agent/agent.py
sed -n '360,740p' src/src/praisonai-agents/praisonaiagents/agent/agent.py 2>/dev/null || sed -n '360,740p' src/praisonai-agents/praisonaiagents/agent/agent.py
sed -n '740,1140p' src/praisonai-agents/praisonaiagents/agent/agent.py

echo
echo "== focused mixin/interrupt sections =="
sed -n '1,320p' src/praisonai-agents/praisonaiagents/agent/chat_mixin.py
sed -n '1,220p' src/praisonai-agents/praisonaiagents/agent/interrupt.py

Repository: MervinPraison/PraisonAI

Length of output: 50381


Avoid mutating the shared agent’s interrupt_controller.

_dispatch_agent_turn stamps the per-turn InterruptController onto agent.interrupt_controller, but _process_agent_message reuses the same Agent object for every WebSocket session with the same agent_id. If concurrent turns share that agent, one turn can overwrite the controller another turn is checkpointing against, breaking cooperative cancellation for that agent while only the hard task cancellation remains reliable. Use per-turn cloning or pass the controller through explicit per-turn state instead of mutating shared agent state.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/praisonai-bot/praisonai_bot/gateway/server.py` around lines 3077 - 3100,
Update _dispatch_agent_turn to avoid assigning the per-turn interrupt controller
to the shared agent.interrupt_controller. Use a per-turn agent clone or explicit
per-turn state so concurrent sessions retain independent controllers, while
preserving native async dispatch and the synchronous agent.chat executor
fallback.

Source: Coding guidelines

Comment on lines +3126 to +3167
async def _drive_turn(
self,
session: GatewaySession,
agent: Any,
content: str,
controller: Any,
timeout: float,
) -> Any:
"""Run one agent turn cancellably and under an optional per-turn timeout.

Registers the driving task in ``_active_turns`` so ``_abort_active_turn``
can interrupt it, then awaits it with ``asyncio.wait_for`` when a
positive ``timeout`` is configured. A cancelled or timed-out turn is
normalised to a terminal string rather than left hanging or surfaced as
a raw traceback.
"""
sid = session.session_id
task = asyncio.ensure_future(
self._dispatch_agent_turn(agent, content, interrupt=controller)
)
self._active_turns[sid] = (task, controller)
try:
if timeout and timeout > 0:
try:
return await asyncio.wait_for(task, timeout=timeout)
except asyncio.TimeoutError:
controller.request("timeout")
task.cancel()
try:
await task
except (asyncio.CancelledError, Exception):
pass
return self._finalise_aborted_turn(controller, "timeout")
return await task
except asyncio.CancelledError:
reason = controller.reason or "user"
return self._finalise_aborted_turn(controller, reason)
finally:
existing = self._active_turns.get(sid)
if existing is not None and existing[0] is task:
self._active_turns.pop(sid, None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does InterruptController in praisonaiagents.agent.interrupt expose an is_set() method?

💡 Result:

Yes, the InterruptController in praisonaiagents.agent.interrupt utilizes an is_set method, typically accessed via its internal cancellation attributes [1]. According to official PraisonAI documentation, the InterruptController is used for cooperative cancellation [1]. When passed as a cancel_token to agent methods like chat, the agent checks for an interrupt signal at various checkpoints [1]. Documentation explicitly notes that when this token's is_set returns true, an InterruptedError is raised [1]. While the exact implementation details can vary across versions, common patterns in this framework involve the InterruptController managing an internal state (often utilizing threading.Event or similar mechanisms) where is_set is used to verify if an interruption has been requested [1][2][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
git ls-files | rg 'src/praisonai-bot/praisonai_bot/gateway/server.py|InterruptController|interrupt' | head -100

echo
echo "== server excerpt =="
sed -n '2980,3190p' src/praisonai-bot/praisonai_bot/gateway/server.py

echo
echo "== interrupt-related usages in server =="
rg -n "_drive_turn|_finalise_aborted_turn|_active_turns|_abort_active_turn|InterruptController|controller\.is_set|controller\.request|reason or \"user\"" src/praisonai-bot/praisonai_bot/gateway/server.py

echo
echo "== InterruptController definitions/usages in repo and installed package if available =="
rg -n "class InterruptController|def is_set|is_set\\(" . -g '*.py' | head -200
python3 - <<'PY'
try:
    import praisonaiagents.agent.interrupt as mod
    print("package imported ok")
    for name in dir(mod):
        if "Interrupt" in name:
            cls = getattr(mod, name)
            print(name, type(cls), hasattr(cls, "is_set"))
except Exception as e:
    print("import failed:", type(e).__name__, e)
PY

Repository: MervinPraison/PraisonAI

Length of output: 15971


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== server queue excerpt =="
sed -n '3188,3230p' src/praisonai-bot/praisonai_bot/gateway/server.py

echo
echo "== InterruptController implementation =="
sed -n '1,115p' src/praisonai-agents/praisonaiagents/agent/interrupt.py

echo
echo "== InterruptController tests =="
sed -n '1,135p' src/praisonai-agents/tests/unit/agent/test_interrupt.py

echo
echo "== asyncio cancellation behavior probe for await CancelledError catch =="
python3 - <<'PY'
import asyncio

async def cancels_await():
    for i in range(3):
        await a_task()
        print("loop", i)

async def a_task():
    raise asyncio.CancelledError("example external cancellation")

try:
    asyncio.get_event_loop().run_until_complete(cancels_await())
except asyncio.CancelledError as e:
    print("external cancellation propagated:", e.args)
else:
    print("external cancellation swallowed")
PY

Repository: MervinPraison/PraisonAI

Length of output: 10013


Propagate external cancellations in _drive_turn

At lines 3161-3162, the current except asyncio.CancelledError catches CancelledError from the whole _drive_turn coroutine and returns "Turn cancelled: user." instead of propagating an external cancellation. Since controller.is_set() is only true after _abort_active_turn/timeout explicitly requests interruption, re-raise CancelledError when the controller is not set so the enclosing queue task can actually stop as intended.

🧰 Tools
🪛 Ruff (0.15.21)

[error] 3156-3157: try-except-pass detected, consider logging the exception

(S110)


[warning] 3156-3156: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/praisonai-bot/praisonai_bot/gateway/server.py` around lines 3126 - 3167,
Update the asyncio.CancelledError handler in _drive_turn to distinguish external
cancellation from requested turn interruption: if controller.is_set() is false,
re-raise the cancellation; otherwise preserve the existing
_finalise_aborted_turn behavior using the controller reason.

Address three P1 gaps in the gateway cancellation/timeout work:

- Timeout was unreachable: add per_turn_timeout to the bot GatewayServerSchema
  (extra="forbid" was rejecting it) and stamp it onto self.config in
  start_with_config so the documented YAML knob actually takes effect.
- Turn isolation: pass the InterruptController per-turn as the entry point's
  cancel_token instead of mutating the shared agent.interrupt_controller, so
  overlapping sessions on one Agent instance no longer interrupt each other.
  Legacy agents without a cancel_token kwarg fall back to the shared attribute.
- Cancellation honesty: interrupt cooperatively first and wait a bounded grace
  window (_ABORT_GRACE_SECONDS) for the turn — including a sync agent.chat in a
  worker thread that cannot be force-killed — to unwind before the serial
  session queue advances; hard task.cancel() only as a fallback.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
@MervinPraison MervinPraison added pipeline/blocked:stale-final Blocked: FINAL stale after new commits and removed pipeline/blocked:stale-final Blocked: FINAL stale after new commits labels Jul 28, 2026
Comment on lines +3104 to +3113
try:
return await fn(content, **_kw)
except TypeError:
if not _kw:
raise
# Entry point predates cancel_token: fall back to the shared
# attribute for this turn (best-effort, non-isolated).
if hasattr(agent, "interrupt_controller"):
agent.interrupt_controller = interrupt
return await fn(content)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 TypeError fallback duplicates turns

When an agent entry point accepts cancel_token but raises TypeError after beginning provider or tool execution, this handler treats the runtime error as an unsupported argument, assigns the controller to the shared agent, and invokes the entire turn again. This can repeat tool side effects and restore cross-session cancellation interference.

Knowledge Base Used: praisonai-bot

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

Labels

pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:manual-review Blocked: requires manual review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway cannot cancel or time-bound an in-flight agent turn — a runaway turn blocks the whole session queue

1 participant