Repository navigation
fix(executor): clean up task state after cancel to prevent 'Request interrupted' error - #10
Merged
Merged
Conversation
Change attachment download path from project_path to WORKSPACE_ROOT/task_id to ensure attachments are saved to the correct location.
Expected: /workspace/{task_id}/{task_id}:executor:attachments/{subtask_id}/
Previous: /workspace/{task_id}/{repo_name}/{task_id}:executor:attachments/{subtask_id}/
…nterrupted" error When a task is cancelled, TaskStateManager stores the CANCELLED state. If the state is not cleaned up, the next message sent by the user will check is_cancelled() which returns true, causing the agent to return immediately with "Request interrupted by user" error. This fix adds task state cleanup in send_cancel_callback_async() after the cancel callback is sent, ensuring subsequent messages can be processed normally. Changes: - Import TaskStateManager in agent_service.py - Call task_state_manager.cleanup(task_id) after cancel callback is sent - Add cleanup in error handler as well to ensure state is cleaned up
Contributor
📝 WalkthroughWalkthroughRefined AgentService behavior and logs: clarified agent creation vs reuse, safer bot_config parsing, stronger cancellation and cleanup with TaskStateManager, and standardized status/result logging. Claude code agent now stores attachments under WORKSPACE_ROOT/{task_id}. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AgentService
participant AgentStore
participant AgentSession
participant TaskStateManager
participant CallbackSink
Client->>AgentService: Start task/run request
AgentService->>AgentStore: find existing agent/session by id
alt agent exists
AgentStore-->>AgentService: AgentSession
AgentService->>AgentSession: update prompt / prepare
else agent missing
AgentService->>AgentStore: create AgentSession / Agent
AgentStore-->>AgentService: new AgentSession
end
AgentService->>AgentSession: initialize & run
AgentSession->>TaskStateManager: update state (running)
AgentSession->>CallbackSink: send progress/results
CallbackSink-->>AgentService: ack / error
AgentService->>TaskStateManager: cleanup state on completion/failure
AgentService-->>Client: final status/result
sequenceDiagram
autonumber
participant Client
participant AgentService
participant AgentSession
participant TaskStateManager
participant CallbackSink
Client->>AgentService: Cancel request
AgentService->>AgentSession: locate session / agent
alt agent supports cancel
AgentSession->>AgentService: cancel initiated
AgentService->>TaskStateManager: mark cancelled / cleanup
AgentService->>CallbackSink: send cancellation update
CallbackSink-->>AgentService: ack / error
AgentService-->>Client: cancellation confirmed
else cancel unsupported or fails
AgentService->>TaskStateManager: ensure cleanup fallback
AgentService-->>Client: cancellation failed / not supported
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @executor/services/agent_service.py:
- Around line 288-294: The cancel-callback currently does task_data =
getattr(agent, "task_data", {}) and then calls task_data.get(...), which will
crash if agent.task_data exists but is None or a non-dict; update the logic
around agent.task_data to ensure it's a dict before using .get: retrieve
agent.task_data, check isinstance(task_data, dict) (or coerce None/other types
to an empty dict), then read subtask_id, task_title, and subtask_title from that
safe dict so accesses in the cancel callback (references to agent.task_data,
subtask_id, task_title, subtask_title) never call .get on a non-dict.
🧹 Nitpick comments (1)
executor/services/agent_service.py (1)
118-120: Drop redundant{e}inlogger.exception(...)messages (keeps stack trace, reduces noise).
Ruff’sTRY401is right here:logger.exception(...)already records exception info.Proposed fix
- logger.exception( - f"[{_format_task_log(task_id, subtask_id)}] Exception during agent creation: {e}" - ) + logger.exception( + f"[{_format_task_log(task_id, subtask_id)}] Exception during agent creation" + ) @@ - logger.exception( - f"[{agent.get_name()}][{_format_task_log(agent.task_id, agent.subtask_id)}] Execution error: {e}" - ) + logger.exception( + f"[{agent.get_name()}][{_format_task_log(agent.task_id, agent.subtask_id)}] Execution error" + ) @@ - logger.exception( - f"[{_format_task_log(task_id, subtask_id)}] Task execution error: {e}" - ) + logger.exception( + f"[{_format_task_log(task_id, subtask_id)}] Task execution error" + ) @@ - logger.exception( - f"[{_format_task_log(task_id, -1)}] Error closing agent: {e}" - ) + logger.exception( + f"[{_format_task_log(task_id, -1)}] Error closing agent" + ) @@ - logger.exception(f"[{task_id}] Error sending cancel callback: {e}") + logger.exception(f"[{task_id}] Error sending cancel callback")Also applies to: 132-135, 162-165, 180-183, 328-339
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
executor/services/agent_service.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-18T02:09:09.776Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T02:09:09.776Z
Learning: Applies to executor/agents/**/*.py : Executor agents MUST support multiple types: ClaudeCode (local_engine), Agno (local_engine), Dify (external_api), Chat (direct_chat), ImageValidator (validator)
Applied to files:
executor/services/agent_service.py
📚 Learning: 2025-12-18T02:09:09.776Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T02:09:09.776Z
Learning: Applies to executor/agents/claude_code/**/*.py : ClaudeCode agent MUST support Claude Code SDK, Git clone, Skills, MCP servers, and custom instructions (.cursorrules, .windsurfrules)
Applied to files:
executor/services/agent_service.py
🪛 Ruff (0.14.10)
executor/services/agent_service.py
115-115: Consider moving this statement to an else block
(TRY300)
119-119: Redundant exception object included in logging.exception call
(TRY401)
133-133: Redundant exception object included in logging.exception call
(TRY401)
163-163: Redundant exception object included in logging.exception call
(TRY401)
181-181: Redundant exception object included in logging.exception call
(TRY401)
328-328: Redundant exception object included in logging.exception call
(TRY401)
336-336: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: E2E Tests (Shard 1/3)
- GitHub Check: E2E Tests (Shard 3/3)
- GitHub Check: E2E Tests (Shard 2/3)
- GitHub Check: Test Frontend
- GitHub Check: Test wegent CLI Integration
🔇 Additional comments (3)
executor/services/agent_service.py (3)
123-135: Behavior-preserving improvements look good (logging clarity + cancel callback status + structured returns).Also applies to: 146-152, 243-266, 295-317, 341-350, 398-399
319-339: Singleton pattern confirmed, but race condition concern is mitigated by agent-level cancellation.The
TaskStateManager()instances do share underlying state—it's a proper singleton using the double-checked locking pattern (lines 42–50 intask_state_manager.py). However, cleanup timing is not a blocker:
cancel_task()callsagent.cancel_run()synchronously before returning, which cancels execution at the agent level.is_cancelled()checks in streaming loops (dify_agent.py:356, 469, 587; response_processor.py:75) rely on agent-internal state during execution.- The cleanup in
send_cancel_callback_async()is defensive cleanup after the callback—not the primary cancellation mechanism.The async gap between callback dispatch and state cleanup is acceptable because agents stop processing based on their own cancellation flags, not just
TaskStateManagerstate.
49-56: Actual issue is opposite: Lines 63 and 143 incorrectly convert tostrwhen lookups should use rawintkeys.The review's diagnosis is reversed. When
json.loads()parsestask_data, numeric values become Pythonint, notstr. Storage at line 111 uses rawtask_id(anint), making it consistent withcancel_task(line 232) andsend_cancel_callback_async(line 280), which both correctly use rawtask_id: intkeys. However,create_agent(line 63) andexecute_task(line 143) incorrectly callself.get_agent(f"{task_id}"), converting tostrkeys that won't match the storedintkeys—causing session reuse to fail.The correct fix is to remove the string conversions at lines 63 and 143, not add them elsewhere. Additionally,
delete_session_asynctakes astrparameter (line 186) while the execute path usesint—this API inconsistency should also be resolved.Likely an incorrect or invalid review comment.
Comment on lines
+288
to
+294
| task_data = getattr(agent, "task_data", {}) | ||
|
|
||
| # Get task information | ||
| subtask_id = task_data.get("subtask_id", -1) | ||
| task_title = task_data.get("task_title", "") | ||
| subtask_title = task_data.get("subtask_title", "") | ||
|
|
||
| logger.info(f"[{_format_task_log(task_id, subtask_id)}] Sending cancel callback asynchronously") | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
Harden cancel-callback task_data access (can be None / non-dict).
getattr(agent, "task_data", {}) won’t protect you if task_data exists but is None (or another type), and .get(...) will raise.
Proposed fix
- task_data = getattr(agent, "task_data", {})
+ task_data = getattr(agent, "task_data", None) or {}
+ if not isinstance(task_data, dict):
+ task_data = {}🤖 Prompt for AI Agents
In @executor/services/agent_service.py around lines 288 - 294, The
cancel-callback currently does task_data = getattr(agent, "task_data", {}) and
then calls task_data.get(...), which will crash if agent.task_data exists but is
None or a non-dict; update the logic around agent.task_data to ensure it's a
dict before using .get: retrieve agent.task_data, check isinstance(task_data,
dict) (or coerce None/other types to an empty dict), then read subtask_id,
task_title, and subtask_title from that safe dict so accesses in the cancel
callback (references to agent.task_data, subtask_id, task_title, subtask_title)
never call .get on a non-dict.
…path fix(executor): use correct workspace path for attachment downloads
qdaxb
approved these changes
Jan 12, 2026
Merged
This was referenced Feb 5, 2026
Merged
qdaxb
pushed a commit
that referenced
this pull request
Jun 10, 2026
* feat(chat): add context status metrics and toolbar indicator * feat(chat): finalize Phase 1 chat status toolbar and reconnect recovery - Replace QuotaUsage with ChatToolbarStatus driven by chat:status_updated - Wire useChatStatusIndicator to derive a fallback snapshot from the latest persisted AI message so toolbar status survives cold reloads before any live socket event arrives - Type UnifiedMessage.result.context_metrics so persisted snapshots flow through the state machine without runtime casts Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chat_shell): add TruncationPolicy and GuardSource Protocol (T1) Introduce the shared types module for Phase 2 context governance: - TruncationPolicy: frozen dataclass with kind (bytes/tokens) and limit - GuardSource: runtime_checkable Protocol for guard source adapters - DEFAULT_EMERGENCY_RATIO = 0.3 and default_emergency_policy helper Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chat_shell): add ToolOutputGuardAdapter and compact tool string format (T2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(chat_shell): tighten compact footer detection and dedupe encoding (T2 review) * feat(chat_shell): add UnifiedContextGuard with source + compression pipeline (T3) Phase 2 unified context guard. Runs as LangGraph pre_model_hook before every model call (pre-turn and mid-turn alike) and applies: 1. Source-level pass: convert raw tool messages to compact form via the registered GuardSource adapters; mark `compacted=true` so subsequent passes treat them as already-handled. 2. Request-level compression: when live state exceeds the model trigger limit, run MessageCompressor and translate its diff into LangGraph state updates (RemoveMessage for dropped ids, fresh BaseMessage for synthesized summaries; compacted=true marker propagates). 3. Emergency re-truncation: stub for T7; logs a warning when compression alone is insufficient. Skips messages without a stable id since add_messages cannot upsert them without doubling content. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chat_shell): wire UnifiedContextGuard as pre_model_hook (T4) Phase 2 wiring task: - chat_service constructs UnifiedContextGuard with ToolOutputGuardAdapter as the only source. The guard is chained AFTER guidance_consumer's hook via chain_pre_model_hooks so guidance injection observes already-budgeted state and writes its llm_input_messages on top of the compacted view. - agent.build_messages no longer invokes MessageCompressor — budget enforcement now lives entirely in the pre_model_hook, which fires on pre-turn AND every mid-turn follow-up. Cache-breakpoint application remains in build_messages on the assembled layout. - Drops the now-unused MessageCompressor import from agent.py. - Adds guard/composition.py with chain_pre_model_hooks, supporting mixed sync/async hooks; messages updates concatenate, llm_input_messages from the last hook wins. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(chat_shell): finalize Phase 2 context governance (T7+T8+T9 + review) Completes Phase 2 by adding emergency re-truncation, removing legacy serialization-time truncation, refactoring the metrics tracker to emit-only, and addressing the review findings on the resulting code. T7 — Emergency re-truncation - UnifiedContextGuard stage 3 now re-renders source-owned messages under each source's emergency policy, biggest-first with early stop once usage drops below trigger. - ToolOutputGuardAdapter accepts an emergency_ratio (defaults to DEFAULT_EMERGENCY_RATIO); chat_service passes settings.EMERGENCY_TOOL_OUTPUT_RATIO. - Synthesized messages from stage 2 are excluded from the candidate set so the upsert-by-id contract holds. T8 — Delete legacy _truncate_tool_result_for_storage - graph_builder._serialize_messages_chain no longer chops tool results at serialization time; budget enforcement is owned by the guard. - Removed MAX_TOOL_RESULT_LENGTH from config and its tests. T9 — Tracker → emit-only wrapper - ContextMetricsTracker no longer keeps its own messages duplicate. Single API: capture(messages, phase). Snapshot computation is delegated to an injected metrics_fn so accounting flows from one source. - chat_service captures BUILD_MESSAGES at turn start and FINAL at end with messages + agent_builder._last_messages_chain. - tools/events drops the per-tool tracker calls. Review fixes - #1: extract_raw now parses already-compact strings via the new _split_compact helper, so stage 3 emergency re-render no longer produces nested headers / fabricated total_tokens counts. - #2: UnifiedContextGuard.__call__ is async and emits a metrics snapshot via the injected tracker on every invocation. Phase is PHASE_AFTER_COMPACTION when compaction happened, PHASE_AFTER_TOOL_END otherwise; bucket-throttling in should_emit_status_update suppresses redundant emits. Tests converted to async. - #3: ToolResultTruncationStrategy now skips messages flagged additional_kwargs.compacted=True. Without this, stage 2 compression would corrupt the guard's fixed-format compact strings, leaving a misleading total_tokens count and mixing two truncation notices. - #5: Documented the source-disjointness assumption in _apply_source_pass — registered GuardSources must have non-overlapping applies_to predicates. - #6: calculate_context_metrics accepts pre-built TokenCounter and ModelContextConfig; guard.metrics() reuses its instance state instead of constructing fresh ones on every snapshot. - #8: Kept default_emergency_policy as a documented public reference helper for future GuardSource impls. - #9: _dict_to_base_message forwards d.get('id') so the compressor's synthesized ids survive the round-trip. - #10: New test verifies cache_control markers in additional_kwargs survive guard compaction (Anthropic prompt-cache compatibility). Tests - 14 new tests across guard composition, tracker emit, compact-string roundtrip, id propagation, cache-breakpoint preservation, and compressor skip behavior. Total chat_shell suite: 549 passing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(chat-shell): finalize phase2 context guard * fix(chat-shell): tune tool guard controls and diagnostics * feat(knowledge): add listing pagination and guard fixes * fix(frontend): improve chat toolbar status sections --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
send_cancel_callback_async()方法中添加任务状态清理逻辑Problem
当用户取消 Claude 任务后，
TaskStateManager中的任务状态被设置为CANCELLED。但是由于没有清理该状态，当用户发送下一条消息时：_async_execute()方法检查task_state_manager.is_cancelled(task_id)返回TrueSolution
在
send_cancel_callback_async()方法中，取消回调发送后调用task_state_manager.cleanup(task_id)清理任务状态，确保后续消息可以正常处理。Changes
executor/services/agent_service.py中导入TaskStateManagertask_state_manager.cleanup(task_id)Test plan
Summary by CodeRabbit
Bug Fixes
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.