Add task cancellation feature for Web UI stop button #84
Conversation
This commit implements the ability to cancel running tasks via the Web UI stop button.
The cancellation stops task execution without deleting Docker containers or closing Agent connections,
allowing users to continue conversations in the same session.
Backend changes:
- Add cancel_task API endpoint (POST /api/tasks/{task_id}/cancel) in tasks.py
- Implement cancel_task business logic in TaskKindsService with optimistic locking
- Add stop_executor_task methods (sync/async) in ExecutorKindsService
- Add EXECUTOR_STOP_TASK_URL configuration in config.py
Executor Manager changes:
- Add stop_task abstract method in Executor base class
- Implement stop_task in DockerExecutor to send HTTP stop signal to containers
- Add /executor-manager/executor/stop route in routers.py
Key features:
- Updates task and subtask statuses to CANCELLED
- Sends stop signals to running executors without deleting containers
- Maintains session continuity for user to continue conversations
- Uses optimistic locking to prevent race conditions
WalkthroughA task cancellation feature is implemented across the full stack: backend API endpoint, service logic to update task status and notify executors, executor stop capability, and frontend UI with translated stop button and success/error messaging. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend UI
participant TaskContext
participant Backend API
participant TaskService
participant ExecutorService
participant ExecutorManager
participant Container
User->>Frontend UI: Click Stop Button
Frontend UI->>TaskContext: cancelTask(taskId)
TaskContext->>Backend API: POST /tasks/{id}/cancel
Backend API->>TaskService: cancel_task(db, task_id, user_id)
TaskService->>TaskService: Update task status to CANCELLED
TaskService->>TaskService: Cancel all subtasks
TaskService->>TaskService: Collect unique running executors
loop For each executor
TaskService->>ExecutorService: stop_executor_task_sync(executor)
ExecutorService->>ExecutorManager: POST /executor-manager/executor/stop
ExecutorManager->>ExecutorManager: Dispatch to executor.stop_task()
ExecutorManager->>Container: POST /stop
Container-->>ExecutorManager: Stop signal received
ExecutorManager-->>ExecutorService: Success response
end
TaskService-->>Backend API: Return updated task (CANCELLED)
Backend API-->>TaskContext: TaskInDB response
TaskContext->>TaskContext: Update state + show success message
TaskContext-->>Frontend UI: Refresh task details
Frontend UI-->>User: Display cancelled task state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Implemented stop button in Web UI to cancel running tasks. The button is dynamically displayed based on task status (RUNNING/PENDING) and switches back to send button when task is completed or cancelled. Frontend changes: - Add cancelTask API method in tasks.ts - Add cancelTask method in TaskContext with isCancelling state - Add stop button in ChatArea component with conditional rendering - Add internationalization for stop button and cancel messages (en/zh-CN) UI Features: - Stop button only shows when task is RUNNING or PENDING - Shows loading state while cancelling - Displays success/error messages using antd message component - Automatically refreshes task details after cancellation - Uses StopIcon from heroicons User Experience: - Seamless transition between send and stop buttons - Clear visual feedback during cancellation - Maintains session continuity after stopping - Users can continue conversation in same session
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
executor_manager/executors/base.py (1)
32-47: Align stop interface with routing model (executor_namespace mismatch)The new
stop_task(self, executor_name: str)keeps the Executor interface simple, but the router’sStopExecutorRequestalready accepts anexecutor_namespacethat never gets propagated. That creates a small API mismatch and may constrain non-Docker implementations later.Consider either:
- Extending the interface now (e.g.
stop_task(self, executor_name: str, executor_namespace: Optional[str] = None)) and threading it through, or- Dropping
executor_namespacefrom the stop request until it is actually needed.executor_manager/routers/routers.py (1)
140-173: Stop executor route works, but executor_namespace is unused and error handling can be tightenedThe stop endpoint wiring is functionally correct for the Docker executor (fetch dispatcher, call
stop_task, return its result). Two minor points:
Unused
executor_namespace
StopExecutorRequestacceptsexecutor_namespace, but the handler ignores it and only passesrequest.executor_nameintoexecutor.stop_task(...).- If namespace is not needed for any current executor implementation, consider removing the field from the request model; if it is intended for Kubernetes or future backends, it should be threaded into the executor layer (and likely the
Executor.stop_taskinterface) rather than silently dropped.Exception handling style (non-blocking)
- The broad
except Exceptionwithlogger.error(...)andHTTPException(500, ...)matches the existing delete route, but it’s less than ideal:
- A narrower exception set or an
elseblock for the happy path would satisfy Ruff’s TRY300 suggestion.- Using
logger.exception(...)here would preserve the traceback as Ruff suggests (TRY400).- If you do keep the broad catch, consider
raise HTTPException(...) from eto distinguish handler failures from downstream issues (TRY400/B904).None of these are blockers, but addressing them would make the stop route more explicit and future-proof.
executor_manager/executors/docker/executor.py (1)
330-405: Docker stop_task behavior matches non-destructive “best-effort” designThe implementation correctly:
- Verifies ownership before acting.
- Uses
get_container_portsand handles both failure and “no ports” cases.- Sends a POST to the container’s
/stopendpoint with a bounded timeout and treats timeouts as acceptable, which aligns with the best‑effort semantics described in the PR.- Returns a structured
{status, message|error_msg}result consistent with the rest of the executor layer.Only optional polish items:
- Consider
logger.exception(...)in the outerexceptto capture tracebacks, and ideally narrow theexcept Exceptionif you want to satisfy Ruff’s BLE001/TRY400 suggestions.- If a consumer later needs to distinguish “partial” vs “failed” more formally, you might centralize the status strings as constants, but that’s not necessary for this PR.
backend/app/services/adapters/executor_kinds.py (2)
669-700: Consider adding exception chaining for better error traceability.The exception handling catches
RequestExceptionbut doesn't chain it when raising the newHTTPException. This loses the original traceback, making debugging harder.Apply this diff to preserve the exception chain:
except requests.exceptions.RequestException as e: raise HTTPException( status_code=500, detail=f"Error stopping executor task: {str(e)}" - ) + ) from e
702-732: Consider adding exception chaining for better error traceability.Similar to the sync version, preserve the original exception chain when raising
HTTPException.Apply this diff:
except httpx.HTTPError as e: raise HTTPException( status_code=500, detail=f"Error stopping executor task: {str(e)}" - ) + ) from efrontend/src/features/tasks/contexts/taskContext.tsx (1)
226-248: Consider simplifying the refresh logic.The function updates the tasks list manually (line 233), then conditionally refreshes the selected task detail (lines 236-238), and finally refreshes the entire tasks list (line 241). This creates redundancy:
- The manual
setTasksupdate (line 233) will be immediately overwritten byrefreshTasks()(line 241)- If the selected task is being cancelled,
refreshTasks()will fetch the updated task anywayConsider simplifying to:
const cancelTask = async (taskId: number) => { setIsCancelling(true); try { - const updatedTask = await taskApis.cancelTask(taskId); - - // Update task in list - setTasks(prev => prev.map(t => (t.id === taskId ? updatedTask : t))); + await taskApis.cancelTask(taskId); // Update selected task detail if it's the same task if (selectedTaskDetail?.id === taskId) { await refreshSelectedTaskDetail(false); } // Refresh tasks to ensure consistency await refreshTasks(); } catch (error) { console.error('Failed to cancel task:', error); throw error; } finally { setIsCancelling(false); } };backend/app/services/adapters/task_kinds.py (2)
635-635: Minor style improvement: Simplify boolean comparison.Per Python best practices, checking
is_active == Truecan be simplified to justis_active.Apply this diff:
task = db.query(Kind).filter( Kind.id == task_id, Kind.user_id == user_id, Kind.kind == "Task", - Kind.is_active == True + Kind.is_active ).first()
712-718: Improve exception handling and logging.The exception handler could be improved in two ways:
- Use
logging.exception()instead oflogging.error()to automatically include the traceback- Chain the exception when raising HTTPException to preserve the error context
Apply this diff:
except Exception as e: db.rollback() - logger.error(f"Error cancelling task {task_id}: {str(e)}") + logger.exception(f"Error cancelling task {task_id}: {str(e)}") raise HTTPException( status_code=500, detail=f"Error cancelling task: {str(e)}" - ) + ) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
backend/app/api/endpoints/adapter/tasks.py(1 hunks)backend/app/core/config.py(1 hunks)backend/app/services/adapters/executor_kinds.py(2 hunks)backend/app/services/adapters/task_kinds.py(3 hunks)executor_manager/executors/base.py(1 hunks)executor_manager/executors/docker/executor.py(1 hunks)executor_manager/routers/routers.py(2 hunks)frontend/src/apis/tasks.ts(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(7 hunks)frontend/src/features/tasks/contexts/taskContext.tsx(4 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
frontend/src/apis/tasks.ts (2)
frontend/src/types/api.ts (1)
Task(192-213)frontend/src/apis/client.ts (1)
apiClient(105-105)
executor_manager/executors/base.py (1)
executor_manager/executors/docker/executor.py (1)
stop_task(330-405)
backend/app/api/endpoints/adapter/tasks.py (3)
backend/app/schemas/task.py (1)
TaskInDB(81-91)backend/app/services/adapters/task_kinds.py (1)
cancel_task(621-718)backend/app/core/security.py (1)
get_current_user(26-49)
executor_manager/executors/docker/executor.py (2)
executor_manager/executors/base.py (1)
stop_task(37-47)executor_manager/executors/docker/utils.py (2)
check_container_ownership(147-178)get_container_ports(343-408)
backend/app/services/adapters/task_kinds.py (3)
backend/app/schemas/subtask.py (1)
SubtaskStatus(21-27)backend/app/api/endpoints/adapter/tasks.py (1)
cancel_task(117-123)backend/app/services/adapters/executor_kinds.py (1)
stop_executor_task_sync(669-700)
executor_manager/routers/routers.py (2)
executor_manager/executors/base.py (1)
stop_task(37-47)executor_manager/executors/docker/executor.py (1)
stop_task(330-405)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
frontend/src/apis/tasks.ts (1)
taskApis(85-159)
🪛 Ruff (0.14.5)
backend/app/services/adapters/executor_kinds.py
697-700: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
699-699: Use explicit conversion flag
Replace with conversion flag
(RUF010)
729-732: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
731-731: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/api/endpoints/adapter/tasks.py
119-119: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
120-120: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
executor_manager/executors/docker/executor.py
397-397: Use explicit conversion flag
Replace with conversion flag
(RUF010)
400-400: Do not catch blind exception: Exception
(BLE001)
401-401: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
404-404: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/adapters/task_kinds.py
635-635: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
665-668: Abstract raise to an inner function
(TRY301)
699-699: Do not catch blind exception: Exception
(BLE001)
701-701: Use explicit conversion flag
Replace with conversion flag
(RUF010)
712-712: Do not catch blind exception: Exception
(BLE001)
714-714: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
714-714: Use explicit conversion flag
Replace with conversion flag
(RUF010)
715-718: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
717-717: Use explicit conversion flag
Replace with conversion flag
(RUF010)
executor_manager/routers/routers.py
169-169: Consider moving this statement to an else block
(TRY300)
170-170: Do not catch blind exception: Exception
(BLE001)
171-171: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
172-172: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (8)
backend/app/core/config.py (1)
18-21: Stop-task URL wiring looks correct
EXECUTOR_STOP_TASK_URLmatches the executor-manager stop endpoint and mirrors the existing delete URL pattern; configuration is clear and environment-overridable.frontend/src/i18n/locales/zh-CN/common.json (1)
250-254: New task cancellation i18n keys look good (zh-CN)
task.stop,task.cancel_success, andtask.cancel_failedare clear and consistent with the intended UX.frontend/src/apis/tasks.ts (1)
145-147: cancelTask API is correctly wired to backend cancel endpointThe
cancelTaskhelper cleanly maps toPOST /tasks/{id}/canceland aligns with the existingtaskApisstyle and expectedTaskreturn type.frontend/src/i18n/locales/en/common.json (1)
250-254: English task cancellation i18n strings look goodThe new
tasknamespace (stop,cancel_success,cancel_failed) is clear and matches the intended cancellation UX, and is consistent with the zh-CN locale.backend/app/api/endpoints/adapter/tasks.py (2)
106-115: Good improvement: explicit return message.Adding an explicit success message for the delete endpoint improves API consistency and provides clearer feedback to clients.
116-123: Endpoint implementation looks correct.The cancel endpoint follows the established patterns and properly delegates to the service layer. However, note that there's a critical bug in the service layer's
cancel_taskmethod (see comment on task_kinds.py) that needs to be fixed.frontend/src/features/tasks/components/ChatArea.tsx (2)
211-223: Well-implemented task cancellation handler.The function properly guards against missing task ID, handles success/error cases, displays appropriate user feedback, and refreshes the task detail. Error handling is robust with logging and user notification.
390-416: Conditional button rendering is well-implemented.The logic correctly shows:
- Stop button when a task is actively RUNNING or PENDING
- Send button otherwise (new task or completed task)
The button states (loading/disabled) are properly managed, and the UI provides clear visual feedback to users.
| # Update subtasks status to CANCELLED | ||
| for subtask in task_subtasks: | ||
| if subtask.status in [SubtaskStatus.RUNNING, SubtaskStatus.PENDING]: | ||
| subtask.status = SubtaskStatus.CANCELLED | ||
| subtask.error_message = "User cancelled" | ||
| subtask.completed_at = datetime.now() | ||
| subtask.updated_at = datetime.now() | ||
|
|
||
| # Collect unique executor keys to notify (namespace + name) | ||
| unique_executor_keys = set() | ||
| for subtask in task_subtasks: | ||
| if subtask.executor_name and subtask.status == SubtaskStatus.RUNNING: | ||
| unique_executor_keys.add((subtask.executor_namespace, subtask.executor_name)) |
There was a problem hiding this comment.
Critical logic error: Executor stop notifications will never be sent.
The code collects executor keys (lines 688-691) by checking subtask.status == SubtaskStatus.RUNNING, but this happens after all RUNNING subtasks have been updated to CANCELLED (lines 680-685). This means unique_executor_keys will always be empty, and no stop notifications will be sent to executors.
Apply this diff to fix the ordering:
+ # Collect unique executor keys to notify BEFORE updating status (namespace + name)
+ unique_executor_keys = set()
+ for subtask in task_subtasks:
+ if subtask.executor_name and subtask.status == SubtaskStatus.RUNNING:
+ unique_executor_keys.add((subtask.executor_namespace, subtask.executor_name))
+
# Update subtasks status to CANCELLED
for subtask in task_subtasks:
if subtask.status in [SubtaskStatus.RUNNING, SubtaskStatus.PENDING]:
subtask.status = SubtaskStatus.CANCELLED
subtask.error_message = "User cancelled"
subtask.completed_at = datetime.now()
subtask.updated_at = datetime.now()
- # Collect unique executor keys to notify (namespace + name)
- unique_executor_keys = set()
- for subtask in task_subtasks:
- if subtask.executor_name and subtask.status == SubtaskStatus.RUNNING:
- unique_executor_keys.add((subtask.executor_namespace, subtask.executor_name))
-
# Notify executors to stop task (best effort, don't delete containers)🤖 Prompt for AI Agents
In backend/app/services/adapters/task_kinds.py around lines 679 to 691, the code
collects executor keys after mutating all RUNNING subtasks to CANCELLED so the
set is always empty; to fix, first iterate task_subtasks and collect unique
executor keys for subtasks that have executor_name and are currently RUNNING,
then in a separate loop update those same subtasks' status/error/completion
timestamps to CANCELLED; ensure you use the pre-mutation status check when
building unique_executor_keys so stop notifications will be sent.
Summary
Implements task cancellation functionality allowing users to stop running tasks via the Web UI without deleting Docker containers or closing Agent connections. This enables users to continue conversations in the same session after canceling a task.
Changes
Backend API Layer
POST /api/tasks/{task_id}/cancelendpointService Layer
task_kinds.py: Implemented
cancel_task()methodexecutor_kinds.py: Added stop executor methods
stop_executor_task_sync(): Synchronous versionstop_executor_task_async(): Asynchronous versionConfiguration
EXECUTOR_STOP_TASK_URLconfigurationhttp://localhost:8001/executor-manager/executor/stopExecutor Manager Layer
POST /executor-manager/executor/stoproutestop_task()methodExecutor Implementation
base.py: Added
stop_task()abstract method in Executor base classdocker/executor.py: Implemented
stop_task()for Docker executor/stopendpointKey Features
Testing Notes
Summary by CodeRabbit
Release Notes