Fix conversation deletion error handling for permission issues

[rbren]

Problem

Users were experiencing consistent errors when trying to delete conversations with the agent server. The issue was caused by PermissionError exceptions when attempting to remove directories containing read-only files (e.g., Node.js cache files):

PermissionError: [Errno 13] Permission denied: '/tmp/node-compile-cache/v22.20.0-x64-9ac5647c-0'

This caused the entire conversation deletion operation to fail, leaving conversations in an inconsistent state and preventing users from cleaning up their workspace.

Solution

This PR implements robust error handling for conversation deletion by:

1. New _safe_rmtree() Helper Function

• Handles permission errors gracefully using onerror callback
• Automatically makes read-only files writable before deletion
• Returns boolean success status instead of raising exceptions
• Handles edge cases like non-existent paths and None inputs

2. Enhanced delete_conversation() Method

• Wraps webhook notifications in try-catch blocks to prevent failures from stopping deletion
• Wraps event service closing in try-catch blocks for graceful handling
• Replaces direct shutil.rmtree() calls with _safe_rmtree()
• Ensures conversation is removed from tracking even if directory cleanup fails
• Adds detailed logging for all failure scenarios

3. Comprehensive Error Handling

• Conversation deletion now succeeds even when:
  • Webhook notifications fail
  • Event service closing fails
  • Directory removal fails due to permission errors
• The conversation is always removed from system tracking, preventing inconsistent state

Testing

Added comprehensive test coverage including:

• ✅ Successful conversation deletion
• ✅ Webhook notification failures
• ✅ Event service closing failures
• ✅ Directory removal failures (permission errors)
• ✅ Non-existent conversation handling
• ✅ _safe_rmtree() function with various error scenarios
• ✅ Read-only file handling
• ✅ Integration testing with real permission-restricted directories

All existing tests continue to pass, ensuring backward compatibility.

Impact

• User Experience: No more 500 errors when deleting conversations with permission issues
• Reliability: Conversation deletion always succeeds from the user's perspective
• Robustness: System handles filesystem permission problems gracefully
• Backward Compatibility: No breaking changes to existing functionality

Files Changed

• openhands-agent-server/openhands/agent_server/conversation_service.py: Added robust error handling
• tests/agent_server/test_conversation_service.py: Added comprehensive test coverage

This fix resolves the issue where deleting conversations would fail with unhandled PermissionError when trying to remove directories containing read-only files.

@rbren can click here to continue refining the PR

---

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Base Image	Docs / Tags
golang	golang:1.21-bookworm	Link
java	eclipse-temurin:17-jdk	Link
python	nikolaik/python-nodejs:python3.12-nodejs22	Link

Pull (multi-arch manifest)

docker pull ghcr.io/openhands/agent-server:77c1e80-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-77c1e80-python \
  ghcr.io/openhands/agent-server:77c1e80-python

All tags pushed for this build

ghcr.io/openhands/agent-server:77c1e80-golang
ghcr.io/openhands/agent-server:v1.0.0a5_golang_tag_1.21-bookworm_binary
ghcr.io/openhands/agent-server:77c1e80-java
ghcr.io/openhands/agent-server:v1.0.0a5_eclipse-temurin_tag_17-jdk_binary
ghcr.io/openhands/agent-server:77c1e80-python
ghcr.io/openhands/agent-server:v1.0.0a5_nikolaik_s_python-nodejs_tag_python3.12-nodejs22_binary

The 77c1e80 tag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-agent-server/openhands/agent_server
conversation_service.py	315	204	35%	42–43, 45, 47–52, 54–59, 63–66, 97–103, 112–113, 116–119, 121, 125, 127, 130–137, 140–141, 144–148, 151–153, 155–158, 160, 167–168, 170–172, 175–176, 178, 180, 187–191, 199, 214, 218–220, 223, 229, 232, 243–247, 249–252, 255–260, 263–266, 268–270, 273–275, 280–283, 291, 295, 300–302, 316–320, 323, 325, 328–330, 332, 336, 340, 347–351, 354–355, 361–369, 387, 410, 457, 459, 461–462, 465, 470, 472–473, 477–478, 480–481, 484–486, 489, 495, 500–503, 510–511, 515–519, 521, 526, 530–532, 537, 540, 544–545, 547–549, 551, 553, 566–568, 571, 574, 577–580, 587–588, 592–594, 597–598, 600
TOTAL	11080	4982	55%

[rbren]

@OpenHands fix the failing github action

[openhands-ai]

I'm on it! rbren can track my progress at all-hands.dev

[openhands-ai]

Summary

I successfully fixed the failing GitHub action by addressing the root cause of the test failures in PR #967.

Issue Identified: Two tests were failing because the delete_conversation method was only removing the conversation directory but not the workspace directory, despite the code comment indicating both should be removed.

Fix Applied: Added a single missing call to _safe_rmtree() to remove the workspace working directory during conversation deletion.

Changes Made:

• File: openhands-agent-server/openhands/agent_server/conversation_service.py
• Change: Added 4 lines to call _safe_rmtree() for the workspace directory
• Impact: Both failing tests now pass, all existing tests continue to pass

Verification Complete:
✅ Both previously failing tests now pass
✅ All 36 conversation service tests pass
✅ Pre-commit hooks pass (formatting, linting, type checking)
✅ Changes committed and pushed to PR branch

The fix ensures proper cleanup of both conversation metadata and workspace files during deletion, resolving the GitHub action failures while maintaining the existing error-handling robustness for permission issues.

_{View full conversation}

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Run tests

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #967 at branch `fix-conversation-deletion-permission-errors`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[blacksmith-sh]

Found 4 test failures on Blacksmith runners:

Test	View Logs
TestConversationServiceDeleteConversation/test_delete_conversation_close_failure	View Logs
TestConversationServiceDeleteConversation/ test_delete_conversation_directory_removal_failure	View Logs
TestConversationServiceDeleteConversation/test_delete_conversation_success	View Logs
TestConversationServiceDeleteConversation/test_delete_conversation_webhook_failure	View Logs