Skip to content

fix: Mark sessions as ended on cleanup to prevent stale registry entries#11

Open
greghughespdx wants to merge 2 commits into
dbenn8:mainfrom
greghughespdx:fix/session-cleanup-independent
Open

fix: Mark sessions as ended on cleanup to prevent stale registry entries#11
greghughespdx wants to merge 2 commits into
dbenn8:mainfrom
greghughespdx:fix/session-cleanup-independent

Conversation

@greghughespdx

@greghughespdx greghughespdx commented Jan 11, 2026

Copy link
Copy Markdown

Note: This PR is independent of #9 and can be merged on its own. If you prefer to merge #9 first, this will apply cleanly afterward with only a trivial import-line merge conflict.

Summary

When Claude sessions end, they leave stale "active" entries in the registry database. This can cause slack_listener to repeatedly attempt sending to non-existent sockets, resulting in BrokenPipeError spam in logs.

This PR ensures sessions are properly marked as status='ended' when the wrapper exits.

Problem

  1. Wrapper cleanup() removed socket files but never updated the registry database
  2. Sessions remained marked as "active" after the Claude process exited
  3. slack_listener continued trying to send messages to dead sockets
  4. Result: Log spam and confusing "active" sessions in claude slack output, as well as unreliable communications on active sessions

Solution

1. Add END_SESSION command to registry (session_registry.py)

New command handler that marks a session as ended without archiving the Slack thread (preserving thread for history).

2. Enhanced wrapper cleanup (claude_wrapper_hybrid.py)

  • Add end_session() method to RegistryClient
  • Modify cleanup() to mark session as ended BEFORE removing socket
  • Register SIGTERM/SIGINT handlers and atexit for graceful shutdown
  • Add idempotency guard to prevent double cleanup
  • Direct DB fallback: If registry socket is unavailable at cleanup time, access the database directly to ensure session is marked ended

3. Defense-in-depth in listener (slack_listener.py)

  • Add socket existence validation before attempting to send
  • Prevents BrokenPipeError when socket was removed but session not yet cleaned

Files Changed

File Changes
core/session_registry.py +10 lines: END_SESSION command handler
core/claude_wrapper_hybrid.py +98 lines: end_session(), cleanup improvements, signal handlers, DB fallback
core/slack_listener.py +10/-2 lines: Socket existence validation

Testing

  • Session cleanup via normal exit (Ctrl+D)
  • Session cleanup via SIGTERM (kill process)
  • Session cleanup via SIGINT (Ctrl+C)
  • DB fallback when registry socket unavailable
  • Verified claudes shows status='ended' after session exit
  • No more BrokenPipeError spam in logs
  • Slack thread preserved (not archived) after session ends

Breaking Changes

None - existing installations continue to work. Sessions that were previously left as "active" will now correctly show as "ended".

greghughespdx and others added 2 commits January 10, 2026 19:09
Problem:
- When Claude sessions ended, cleanup() removed socket files but never
  marked sessions as inactive in the registry database
- This left stale "active" entries that caused BrokenPipeError spam
  when slack_listener tried to send to non-existent sockets

Solution:
- Add END_SESSION command handler to session_registry.py
- Add end_session() method to RegistryClient in wrapper
- Modify cleanup() to mark session as ended BEFORE removing socket
- Add idempotency guard to prevent double cleanup
- Register SIGTERM/SIGINT handlers and atexit for graceful shutdown
- Add defense-in-depth socket existence check in slack_listener

Changes:
- core/session_registry.py: Add END_SESSION command (~10 lines)
- core/claude_wrapper_hybrid.py: Add end_session(), modify cleanup(),
  add signal handlers (~70 lines)
- core/slack_listener.py: Add socket existence validation (~8 lines)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Root cause: end_session() silently fails when registry socket doesn't exist at cleanup time, leaving sessions in 'active' state.

Changes:
1. RegistryClient.end_session():
   - Removed premature socket existence check
   - Let connect() fail naturally with proper error handling
   - Changed exception type to be more specific

2. HybridPTYWrapper.cleanup():
   - Try socket-based END_SESSION first
   - If that fails, use direct database access as fallback
   - Import RegistryDatabase and call db.end_session() directly
   - Make failures visible with stderr output and logging

Verification: Sessions should now correctly show status='ended' in database even when registry socket is unavailable during cleanup.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@greghughespdx
greghughespdx marked this pull request as ready for review January 11, 2026 03:21
@greghughespdx

Copy link
Copy Markdown
Author

Thanks again for the cool wrapper. Great stuff!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant