Skip to content

feat: add event bus for cross-session coordination (Phase 4)#94

Merged
evansenter merged 5 commits intomainfrom
feat/acp-phase4
Jan 22, 2026
Merged

feat: add event bus for cross-session coordination (Phase 4)#94
evansenter merged 5 commits intomainfrom
feat/acp-phase4

Conversation

@evansenter
Copy link
Copy Markdown
Owner

@evansenter evansenter commented Jan 22, 2026

Summary

Implements Phase 4 of ACP integration (#90): a SQLite-backed event bus for cross-session coordination.

  • EventBus module (src/event_bus.rs): Core event bus implementation with SQLite persistence at ~/.clemini/event_bus.db
  • Event bus tools (src/tools/event_bus_tools.rs): 6 tools for AI-driven session coordination
  • Test coverage: 17 unit tests for event bus, 4 for tools (all pass)

Features

Feature Description
Session registration Register with name, machine, cwd; optional client_id for resumption
Heartbeat & expiration Sessions auto-expire after 5 minutes without activity
Channel pub-sub Channels: all, session:<id>, repo:<name>, machine:<name>
Cursor-based streaming Resume event consumption across restarts

Tools

  • event_bus_register - Register a session with the event bus
  • event_bus_list_sessions - List active sessions
  • event_bus_list_channels - List channels with subscriber counts
  • event_bus_publish - Publish events to channels
  • event_bus_get_events - Get events with filtering and pagination
  • event_bus_unregister - Unregister a session

Test plan

  • make clippy passes
  • make fmt passes
  • make test passes
  • Event bus module tests pass (17 tests)
  • Event bus tools tests pass (4 tests)

Part of #90

🤖 Generated with Claude Code

Implements SQLite-backed event bus for clemini sessions to communicate:

- EventBus module with SQLite persistence (~/.clemini/event_bus.db)
- Session registration with heartbeats and auto-expiration (5 min timeout)
- Channel-based pub-sub: all, session:<id>, repo:<name>, machine:<name>
- Cursor-based event streaming for resumable consumption
- Session resumption via machine + client_id matching

Tools added (7 new tools):
- event_bus_register: Register a session
- event_bus_list_sessions: List active sessions
- event_bus_list_channels: List channels with subscriber counts
- event_bus_publish: Publish events to channels
- event_bus_get_events: Get events with filtering/pagination
- event_bus_unregister: Unregister a session
- event_bus_notify: Send desktop notifications (Linux/macOS)

Test coverage: 17 unit tests for event bus module, 5 for tools

Part of #90 (ACP integration)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Jan 22, 2026

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

Adds a SQLite-backed event bus module (src/event_bus.rs) and 7 new tools for cross-session coordination, with 22 tests covering core functionality.

Issues Found

Critical

None

Important

  • src/tools/event_bus_tools.rs:1342 - The sound parameter in EventBusNotifyTool is parsed but never used (stored as _sound). Either implement sound support or remove the parameter from the tool declaration.

Suggestions

  • src/event_bus.rs:564-595 - Custom UUID v4 implementation could use the uuid crate instead for correctness guarantees and simpler code. The rand crate is already a dependency, and adding uuid = { version = "1", features = ["v4"] } would make this Uuid::new_v4().to_string().
  • src/event_bus.rs:335-376 - list_channels() holds the connection lock then calls self.list_sessions() which acquires its own lock. This works (separate lock acquisitions) but is a code smell. Consider refactoring to pass the connection or extract a shared helper.
  • docs/TOOLS.md - The new event bus tools (event_bus_register, event_bus_publish, etc.) should be documented in TOOLS.md per the project's documentation conventions.

Verdict

REQUEST_CHANGES - Important issue with unused sound parameter and a few suggestions for code quality improvements.


Automated review by Claude Code

Add logging to trace ACP prompt handling:
- Log prompt text extraction (length and preview)
- Log before/after run_interaction calls

Helps debug hangs during Toad integration.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Jan 22, 2026

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

Adds a SQLite-backed event bus module (src/event_bus.rs) and 7 new tools for cross-session coordination (Phase 4 of ACP integration #90), with comprehensive test coverage (17 unit tests for event bus, 5 for tools).

Issues Found

Critical

None

Important

  • src/tools/event_bus_tools.rs:1376 - The sound parameter in EventBusNotifyTool is parsed but stored as _sound (unused). Either implement sound support (macOS osascript supports sound name) or remove the parameter from the tool declaration (lines 1355-1358). This was flagged in the previous review and remains unaddressed.

Suggestions

  • src/event_bus.rs:599-628 - Custom UUID v4 implementation could use the uuid crate for correctness guarantees. The rand crate is already a dependency; adding uuid = { version = "1", features = ["v4"] } simplifies this to Uuid::new_v4().to_string().
  • src/event_bus.rs:369-409 - list_channels() acquires the connection lock, then calls self.list_sessions() which acquires its own lock. While this works (separate acquisitions), it's a potential deadlock pattern if the code evolves. Consider passing the connection or extracting a shared helper that takes &Connection.
  • docs/TOOLS.md - The 7 new event bus tools should be documented per project conventions. The tool reference in TOOLS.md helps users discover available functionality.

Verdict

REQUEST_CHANGES - Important issue with unused sound parameter needs to be addressed (either implement or remove). Suggestions are optional improvements.


Automated review by Claude Code

- Remove EventBusNotifyTool (notifications can use system tools)
- Remove temporary ACP debug logging
- Keep 6 core event bus tools: register, list_sessions, list_channels,
  publish, get_events, unregister

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Jan 22, 2026

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

Adds a SQLite-backed event bus module (src/event_bus.rs) and 6 new tools for cross-session coordination (Phase 4 of ACP integration #90), with comprehensive test coverage (17 unit tests for event bus, 4 for tools).

Issues Found

Critical

None

Important

  • PR description mismatch - The PR description claims 7 tools including event_bus_notify, but only 6 tools are implemented. Either add the notify tool or update the description to accurately reflect the implementation.

Suggestions

  • src/event_bus.rs:564-595 - Custom UUID v4 implementation could use the uuid crate for correctness guarantees. The rand crate is already a dependency; adding uuid = { version = "1", features = ["v4"] } simplifies this to Uuid::new_v4().to_string().
  • src/event_bus.rs:335-347 - list_channels() acquires the connection lock, then calls self.list_sessions() which acquires its own lock (separate acquisitions). While this works, it is a potential deadlock pattern if the code evolves. Consider passing the connection or extracting a shared helper that takes &Connection.
  • docs/TOOLS.md - The 6 new event bus tools should be documented per project conventions. The tool reference in TOOLS.md helps users discover available functionality.

Verdict

REQUEST_CHANGES - Important issue: PR description claims 7 tools but only 6 are implemented (missing event_bus_notify). Additionally, suggestions for code quality improvements.


Automated review by Claude Code

Addresses automated review feedback:

1. **Critical**: Fix deadlock in list_channels() - was calling
   list_sessions() while holding the Mutex lock. Now releases
   lock before calling list_sessions(). Added test to verify.

2. **Suggestion**: Replace custom UUID v4 implementation with
   uuid crate for correctness guarantees and simpler code.

3. **Suggestion**: Add documentation for 6 event bus tools to
   docs/TOOLS.md with parameters, returns, and examples.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@evansenter
Copy link
Copy Markdown
Owner Author

Feedback Addressed

Implemented

  • [Critical] Deadlock in list_channels() - Fixed by releasing the Mutex lock before calling list_sessions(). Added test_list_channels to verify the fix.
  • [Important] Unused sound parameter - Removed EventBusNotifyTool entirely (notifications not needed for MVP).
  • [Suggestion] Use uuid crate - Replaced custom UUID v4 implementation with uuid::Uuid::new_v4().
  • [Suggestion] Document tools in TOOLS.md - Added comprehensive documentation for all 6 event bus tools with parameters, return values, and examples.

@claude
Copy link
Copy Markdown

claude bot commented Jan 22, 2026

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

Implements Phase 4 of ACP integration (#90): a SQLite-backed event bus for cross-session coordination with 6 tools (event_bus_register, event_bus_list_sessions, event_bus_list_channels, event_bus_publish, event_bus_get_events, event_bus_unregister) and comprehensive test coverage.

Issues Found

Critical

None

Important

None

Suggestions

  • src/event_bus.rs:937-938 - Duplicate #[test] attribute on test_heartbeat function. This compiles but is likely a copy-paste error that could confuse future readers.

Previously Addressed (Filtered)

  • src/event_bus.rs - Deadlock in list_channels() (Implemented: lock released before calling list_sessions())
  • src/tools/event_bus_tools.rs - Unused sound parameter in EventBusNotifyTool (Implemented: tool removed entirely)
  • src/event_bus.rs - Custom UUID v4 implementation (Implemented: now uses uuid::Uuid::new_v4())
  • docs/TOOLS.md - Missing event bus tool documentation (Implemented: comprehensive docs added)

4 items from prior feedback rounds were not re-raised.

Verdict

REQUEST_CHANGES - Minor suggestion: duplicate #[test] attribute on line 937-938 should be cleaned up.


Automated review by Claude Code

Leftover from removing test_uuid_v4_format test.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Jan 22, 2026

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

Implements Phase 4 of ACP integration (#90): a SQLite-backed event bus for cross-session coordination with 6 tools (event_bus_register, event_bus_list_sessions, event_bus_list_channels, event_bus_publish, event_bus_get_events, event_bus_unregister) and comprehensive test coverage (17 unit tests for event bus, 4 for tools).

Issues Found

Critical

None

Important

None

Suggestions

  • PR description mismatch - The PR body claims "7 new tools" but only 6 are implemented (the event_bus_notify tool was intentionally removed per prior feedback). Consider updating the PR description to accurately reflect the current implementation (6 tools).

Previously Addressed (Filtered)

  • src/event_bus.rs:522-529 - Deadlock in list_channels() (Implemented: lock released before calling list_sessions())
  • src/tools/event_bus_tools.rs - Unused sound parameter in EventBusNotifyTool (Implemented: tool removed entirely)
  • src/event_bus.rs:751-753 - Custom UUID v4 implementation (Implemented: now uses uuid::Uuid::new_v4())
  • docs/TOOLS.md - Missing event bus tool documentation (Implemented: comprehensive docs added for all 6 tools)
  • src/event_bus.rs:937-938 - Duplicate #[test] attribute (Prior false positive: no duplicate exists in current code)

5 items from prior feedback rounds were not re-raised.

Verdict

REQUEST_CHANGES - Minor suggestion: PR description should be updated to reflect the actual 6 tools implemented (not 7).


Automated review by Claude Code

@evansenter
Copy link
Copy Markdown
Owner Author

Final Feedback Addressed

Implemented

  • [Suggestion] PR description mismatch - Updated to reflect 6 tools (removed reference to event_bus_notify)

All CI checks passing. Ready to merge.

@evansenter evansenter merged commit fcb1b4c into main Jan 22, 2026
9 checks passed
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