mirror of
https://github.com/Alishahryar1/free-claude-code.git
synced 2026-07-10 00:14:16 +00:00
## Problem
Messaging `/clear` and `/stop` could return before cancelled node tasks
finished cleanup. Late cleanup could save stale conversation state after
`/clear` reset FCC state.
## Changes
| Before | After |
| --- | --- |
| Tree cancellation called `task.cancel()` and returned immediately. |
Tree cancellation awaits cancelled task cleanup outside tree locks with
a bounded timeout. |
| Node runners saved snapshots even after their tree was removed or
replaced. | Node runners save only when their node still belongs to the
active tree queue. |
| `/clear` deletion stopped at a batch failure and left tracking vague.
| `/clear` attempts each tracked delete independently and clears
FCC-owned tracking state. |
| Architecture docs did not state terminal cancellation ownership. |
Architecture docs assign terminal cancellation to `messaging/trees` and
guarded cleanup persistence to node runners. |
| Package metadata stayed at `3.4.0`. | Package metadata and lockfile
move to `3.4.1`. |
<!-- greptile_comment -->
<details open><summary><h3>Greptile Summary</h3></summary>
This PR makes messaging cancellation wait for node cleanup before
command cleanup continues. The main changes are:
- Drains cancelled tree tasks with a bounded timeout outside tree locks.
- Guards node-runner snapshot saves so removed or replaced trees are not
restored by late cleanup.
- Changes `/clear` deletion to try each tracked platform message
independently.
- Removes cleared branch message IDs from FCC-owned tracking state.
- Adds cancellation, stale-save, and clear-delete regression tests.
- Bumps package metadata and lockfile version to `3.4.1`.
</details>
<h3>Confidence Score: 5/5</h3>
Safe to merge with minimal risk.
The cancellation paths now drain outside tree locks with a bounded wait,
stale snapshot persistence is guarded, and `/clear` continues through
individual delete failures. Tests cover the key cancellation cleanup,
timeout, stale-save, and clear-delete cases. No blocking correctness or
security issues were found in the changed files.
No files require special attention.
<details><summary><h3><a href="https://www.greptile.com/trex"><img
alt="T-Rex"
src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg"
height="20" align="absmiddle"></a> T-Rex Logs</h3></summary>
**What T-Rex did**
- Ran the messaging cancellation test suite with verbose output to
capture the command, current working directory, timestamps, and verbose
test names.
- Verified the targeted cancellation tests passed, including
test\_cancel\_tree\_waits\_for\_current\_task\_cleanup,
test\_cancel\_node\_waits\_for\_current\_task\_cleanup,
test\_cancel\_branch\_waits\_for\_current\_task\_cleanup,
test\_cancel\_all\_waits\_for\_current\_task\_cleanup\_across\_trees,
and test\_cancel\_task\_drain\_timeout\_is\_bounded.
- Verified that the coverage tests for /clear handling and stale
persistence guard also passed, including
test\_handle\_message\_clear\_command\_stops\_deletes\_and\_wipes\_state
and
test\_cancelled\_node\_runner\_does\_not\_save\_after\_clear\_replaces\_queue.
<a
href="https://app.greptile.com/trex/runs/13359514/artifacts"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifactsDark.svg?v=4"><source
media="(prefers-color-scheme: light)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"><img
alt="View all artifacts"
src="https://greptile-static-assets.s3.amazonaws.com/badges/ViewAllArtifacts.svg?v=4"></picture></a>
<sub><a href="https://www.greptile.com/trex"><img alt="T-Rex"
src="https://greptile-static-assets.s3.amazonaws.com/trex/trex_green.svg"
height="14" align="absmiddle"></a> Ran code and verified through
T-Rex</sub>
</details>
<details open><summary><h3>Important Files Changed</h3></summary>
| Filename | Overview |
|----------|----------|
| messaging/trees/manager.py | Makes tree, node, branch, and all-tree
cancellation await cancelled task cleanup outside tree locks with a
bounded drain helper. |
| messaging/node_runner.py | Guards runner-owned snapshot saves by
confirming the node still belongs to the active tree queue. |
| messaging/commands.py | Updates `/clear` deletion to attempt message
deletes individually and forget branch-owned message IDs after branch
clears. |
| messaging/session/message_log.py | Adds targeted removal of tracked
message IDs while keeping the per-chat ID cache synchronized. |
| tests/messaging/test_tree_queue.py | Adds cancellation-drain tests for
tree, node, branch, all-tree, and timeout-bounded cleanup paths. |
| tests/messaging/test_handler.py | Adds coverage for resilient clear
deletion, branch message-log cleanup, and stale cancellation persistence
prevention. |
</details>
<details open><summary><h3>Sequence Diagram</h3></summary>
<a href="#gh-light-mode-only">
```mermaid
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant Commands as messaging/commands.py
participant Manager as TreeQueueManager
participant Tree as MessageTree
participant Runner as MessagingNodeRunner
participant Store as SessionStore
participant Outbound as OutboundMessenger
User->>Commands: /clear or /stop
Commands->>Manager: cancel_tree/cancel_branch/cancel_all
Manager->>Tree: cancel_current_task()
Tree-->>Manager: cancelled asyncio.Task
Manager->>Tree: mark queued/current nodes ERROR
Manager->>Runner: task cancellation propagates
Manager->>Manager: await _drain_cancelled_tasks(timeout)
Runner->>Runner: cancellation cleanup/update UI
Runner->>Manager: check active tree for node
alt node still belongs to active queue
Runner->>Store: save_tree_snapshot(snapshot)
else tree removed or queue replaced
Runner-->>Store: skip stale save
end
Commands->>Outbound: delete tracked messages individually
Commands->>Store: clear_all or forget_message_ids
```
</a>
<a href="#gh-dark-mode-only">
```mermaid
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant Commands as messaging/commands.py
participant Manager as TreeQueueManager
participant Tree as MessageTree
participant Runner as MessagingNodeRunner
participant Store as SessionStore
participant Outbound as OutboundMessenger
User->>Commands: /clear or /stop
Commands->>Manager: cancel_tree/cancel_branch/cancel_all
Manager->>Tree: cancel_current_task()
Tree-->>Manager: cancelled asyncio.Task
Manager->>Tree: mark queued/current nodes ERROR
Manager->>Runner: task cancellation propagates
Manager->>Manager: await _drain_cancelled_tasks(timeout)
Runner->>Runner: cancellation cleanup/update UI
Runner->>Manager: check active tree for node
alt node still belongs to active queue
Runner->>Store: save_tree_snapshot(snapshot)
else tree removed or queue replaced
Runner-->>Store: skip stale save
end
Commands->>Outbound: delete tracked messages individually
Commands->>Store: clear_all or forget_message_ids
```
</a>
</details>
<sub>Reviews (2): Last reviewed commit: ["Bound messaging cancellation
drain"](d63b5b43f5)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=41990665)</sub>
<!-- /greptile_comment -->
357 lines
13 KiB
Python
357 lines
13 KiB
Python
"""Run queued messaging nodes through a managed CLI session."""
|
|
|
|
import asyncio
|
|
from collections.abc import Callable
|
|
|
|
from loguru import logger
|
|
|
|
from core.anthropic import format_user_error_preview, get_user_facing_error_message
|
|
from core.trace import trace_event
|
|
|
|
from .event_parser import parse_cli_event
|
|
from .managed_protocols import ManagedClaudeSessionManagerProtocol
|
|
from .node_event_pipeline import handle_session_info_event, process_parsed_cli_event
|
|
from .platforms.ports import OutboundMessenger
|
|
from .safe_diagnostics import format_exception_for_log
|
|
from .session import SessionStore
|
|
from .transcript import RenderCtx, TranscriptBuffer
|
|
from .trees import MessageNode, MessageState, MessageTree, TreeQueueManager
|
|
from .ui_updates import ThrottledTranscriptEditor
|
|
|
|
|
|
class MessagingNodeRunner:
|
|
"""Owns the lifecycle of one queued messaging node."""
|
|
|
|
def __init__(
|
|
self,
|
|
*,
|
|
platform_name: str,
|
|
outbound: OutboundMessenger,
|
|
cli_manager: ManagedClaudeSessionManagerProtocol,
|
|
session_store: SessionStore,
|
|
get_tree_queue: Callable[[], TreeQueueManager],
|
|
format_status: Callable[[str, str, str | None], str],
|
|
get_parse_mode: Callable[[], str | None],
|
|
get_render_ctx: Callable[[], RenderCtx],
|
|
get_limit_chars: Callable[[], int],
|
|
debug_platform_edits: bool = False,
|
|
debug_subagent_stack: bool = False,
|
|
log_raw_cli_diagnostics: bool = False,
|
|
log_messaging_error_details: bool = False,
|
|
) -> None:
|
|
self.platform_name = platform_name
|
|
self.outbound = outbound
|
|
self.cli_manager = cli_manager
|
|
self.session_store = session_store
|
|
self._get_tree_queue = get_tree_queue
|
|
self._format_status = format_status
|
|
self._get_parse_mode = get_parse_mode
|
|
self._get_render_ctx = get_render_ctx
|
|
self._get_limit_chars = get_limit_chars
|
|
self._debug_platform_edits = debug_platform_edits
|
|
self._debug_subagent_stack = debug_subagent_stack
|
|
self._log_raw_cli_diagnostics = log_raw_cli_diagnostics
|
|
self._log_messaging_error_details = log_messaging_error_details
|
|
|
|
def _create_transcript_and_render_ctx(
|
|
self,
|
|
) -> tuple[TranscriptBuffer, RenderCtx]:
|
|
"""Create transcript buffer and render context for node processing."""
|
|
transcript = TranscriptBuffer(
|
|
show_tool_results=False,
|
|
debug_subagent_stack=self._debug_subagent_stack,
|
|
)
|
|
return transcript, self._get_render_ctx()
|
|
|
|
def _save_tree(self, tree: MessageTree | None, node_id: str) -> None:
|
|
"""Persist tree state after runner-owned mutations."""
|
|
if not tree:
|
|
return
|
|
active_tree = self._get_tree_queue().get_tree_for_node(node_id)
|
|
if active_tree is not tree:
|
|
logger.debug(
|
|
"Skipping stale tree save for node {} after cancellation/clear",
|
|
node_id,
|
|
)
|
|
return
|
|
self.session_store.save_tree_snapshot(tree.snapshot())
|
|
|
|
async def process_node(
|
|
self,
|
|
node_id: str,
|
|
node: MessageNode,
|
|
) -> None:
|
|
"""Core task processor for a single CLI interaction."""
|
|
incoming = node.incoming
|
|
status_msg_id = node.status_message_id
|
|
chat_id = incoming.chat_id
|
|
|
|
with logger.contextualize(node_id=node_id, chat_id=chat_id):
|
|
await self._process_node_impl(node_id, node, chat_id, status_msg_id)
|
|
|
|
async def _process_node_impl(
|
|
self,
|
|
node_id: str,
|
|
node: MessageNode,
|
|
chat_id: str,
|
|
status_msg_id: str,
|
|
) -> None:
|
|
"""Internal implementation of process_node with context bound."""
|
|
incoming = node.incoming
|
|
|
|
tree_queue = self._get_tree_queue()
|
|
tree = tree_queue.get_tree_for_node(node_id)
|
|
if tree:
|
|
await tree.update_state(node_id, MessageState.IN_PROGRESS)
|
|
|
|
transcript, render_ctx = self._create_transcript_and_render_ctx()
|
|
|
|
had_transcript_events = False
|
|
captured_session_id = None
|
|
temp_session_id = None
|
|
last_status: str | None = None
|
|
|
|
parent_session_id = None
|
|
platform_nm = self.platform_name
|
|
if tree and node.parent_id:
|
|
parent_session_id = tree.get_parent_session_id(node_id)
|
|
if parent_session_id:
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event="claude_cli.fork.from_parent_session",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
parent_session_id=parent_session_id,
|
|
)
|
|
|
|
editor = ThrottledTranscriptEditor(
|
|
outbound=self.outbound,
|
|
parse_mode=self._get_parse_mode(),
|
|
get_limit_chars=self._get_limit_chars,
|
|
transcript=transcript,
|
|
render_ctx=render_ctx,
|
|
node_id=node_id,
|
|
chat_id=chat_id,
|
|
status_msg_id=status_msg_id,
|
|
debug_platform_edits=self._debug_platform_edits,
|
|
log_messaging_error_details=self._log_messaging_error_details,
|
|
)
|
|
|
|
async def update_ui(status: str | None = None, force: bool = False) -> None:
|
|
await editor.update(status, force=force)
|
|
|
|
try:
|
|
try:
|
|
(
|
|
cli_session,
|
|
session_or_temp_id,
|
|
is_new,
|
|
) = await self.cli_manager.get_or_create_session(
|
|
session_id=parent_session_id
|
|
)
|
|
if is_new:
|
|
temp_session_id = session_or_temp_id
|
|
else:
|
|
captured_session_id = session_or_temp_id
|
|
|
|
sess_evt = (
|
|
"claude_cli.session.pending_created"
|
|
if is_new
|
|
else "claude_cli.session.reused"
|
|
)
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event=sess_evt,
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
status_message_id=status_msg_id,
|
|
session_handle=str(session_or_temp_id),
|
|
parent_resume_session_id=parent_session_id,
|
|
fork_requested=bool(parent_session_id),
|
|
)
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event="claude_cli.request.sent",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
prompt=incoming.text,
|
|
fork_session_arg=bool(parent_session_id),
|
|
resume_session_arg=parent_session_id,
|
|
)
|
|
except RuntimeError as e:
|
|
error_message = get_user_facing_error_message(e)
|
|
transcript.apply({"type": "error", "message": error_message})
|
|
await update_ui(
|
|
self._format_status("⏳", "Session limit reached", None),
|
|
force=True,
|
|
)
|
|
if tree:
|
|
await tree.update_state(
|
|
node_id,
|
|
MessageState.ERROR,
|
|
error_message=error_message,
|
|
)
|
|
self._save_tree(tree, node_id)
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event="claude_cli.session.limit_reached",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
)
|
|
return
|
|
|
|
async for event_data in cli_session.start_task(
|
|
incoming.text,
|
|
session_id=parent_session_id,
|
|
fork_session=bool(parent_session_id),
|
|
):
|
|
if not isinstance(event_data, dict):
|
|
logger.warning(
|
|
f"HANDLER: Non-dict event received: {type(event_data)}"
|
|
)
|
|
continue
|
|
|
|
(
|
|
captured_session_id,
|
|
temp_session_id,
|
|
) = await handle_session_info_event(
|
|
event_data,
|
|
tree,
|
|
node_id,
|
|
captured_session_id,
|
|
temp_session_id,
|
|
cli_manager=self.cli_manager,
|
|
session_store=self.session_store,
|
|
save_tree_snapshot=lambda updated_tree: self._save_tree(
|
|
updated_tree, node_id
|
|
),
|
|
)
|
|
if event_data.get("type") == "session_info":
|
|
continue
|
|
|
|
parsed_list = parse_cli_event(
|
|
event_data, log_raw_cli=self._log_raw_cli_diagnostics
|
|
)
|
|
|
|
for parsed in parsed_list:
|
|
(
|
|
last_status,
|
|
had_transcript_events,
|
|
) = await process_parsed_cli_event(
|
|
parsed,
|
|
transcript,
|
|
update_ui,
|
|
last_status,
|
|
had_transcript_events,
|
|
tree,
|
|
node_id,
|
|
captured_session_id,
|
|
session_store=self.session_store,
|
|
save_tree_snapshot=lambda updated_tree: self._save_tree(
|
|
updated_tree, node_id
|
|
),
|
|
format_status=self._format_status,
|
|
propagate_error_to_children=self.propagate_error_to_children,
|
|
log_messaging_error_details=self._log_messaging_error_details,
|
|
)
|
|
|
|
except asyncio.CancelledError:
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event="turn.processor.cancelled",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
)
|
|
logger.warning(f"HANDLER: Task cancelled for node {node_id}")
|
|
cancel_reason = None
|
|
if isinstance(node.context, dict):
|
|
cancel_reason = node.context.get("cancel_reason")
|
|
|
|
if cancel_reason == "stop":
|
|
await update_ui(self._format_status("⏹", "Stopped.", None), force=True)
|
|
else:
|
|
transcript.apply({"type": "error", "message": "Task was cancelled"})
|
|
await update_ui(
|
|
self._format_status("❌", "Cancelled", None), force=True
|
|
)
|
|
|
|
if tree:
|
|
await tree.update_state(
|
|
node_id, MessageState.ERROR, error_message="Cancelled by user"
|
|
)
|
|
self._save_tree(tree, node_id)
|
|
except Exception as e:
|
|
trace_event(
|
|
stage="claude_cli",
|
|
event="turn.processor.exception",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
exc_type=type(e).__name__,
|
|
)
|
|
logger.error(
|
|
"HANDLER: Task failed with exception: {}",
|
|
format_exception_for_log(
|
|
e, log_full_message=self._log_messaging_error_details
|
|
),
|
|
)
|
|
error_msg = format_user_error_preview(e)
|
|
transcript.apply({"type": "error", "message": error_msg})
|
|
await update_ui(self._format_status("💥", "Task Failed", None), force=True)
|
|
if tree:
|
|
await self.propagate_error_to_children(
|
|
node_id, error_msg, "Parent task failed"
|
|
)
|
|
finally:
|
|
trace_event(
|
|
stage="routing",
|
|
event="turn.processor.finished",
|
|
source=platform_nm,
|
|
chat_id=chat_id,
|
|
node_id=node_id,
|
|
claude_session_id=captured_session_id or temp_session_id,
|
|
)
|
|
try:
|
|
if captured_session_id:
|
|
await self.cli_manager.remove_session(captured_session_id)
|
|
elif temp_session_id:
|
|
await self.cli_manager.remove_session(temp_session_id)
|
|
except Exception as e:
|
|
logger.debug(
|
|
"Failed to remove session for node {}: {}",
|
|
node_id,
|
|
format_exception_for_log(
|
|
e, log_full_message=self._log_messaging_error_details
|
|
),
|
|
)
|
|
|
|
async def propagate_error_to_children(
|
|
self,
|
|
node_id: str,
|
|
error_msg: str,
|
|
child_status_text: str,
|
|
) -> None:
|
|
"""Mark node as error and propagate to pending children with UI updates."""
|
|
tree_queue = self._get_tree_queue()
|
|
affected = await tree_queue.mark_node_error(
|
|
node_id, error_msg, propagate_to_children=True
|
|
)
|
|
if affected:
|
|
self._save_tree(tree_queue.get_tree_for_node(node_id), node_id)
|
|
for child in affected[1:]:
|
|
self.outbound.fire_and_forget(
|
|
self.outbound.queue_edit_message(
|
|
child.incoming.chat_id,
|
|
child.status_message_id,
|
|
self._format_status("❌", "Cancelled:", child_status_text),
|
|
parse_mode=self._get_parse_mode(),
|
|
)
|
|
)
|
|
|
|
|
|
__all__ = ["MessagingNodeRunner"]
|