[WIP] Add collaboration and memory functions for sessions#6
[WIP] Add collaboration and memory functions for sessions#6
Conversation
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
Co-authored-by: blackboxprogramming <118287761+blackboxprogramming@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a full session management subsystem for the BlackRoad Bridge, including a registry for active sessions, a collaboration hub for inter-session messaging, a shared memory store, a CLI and demo, and MCP server tools to expose these capabilities, plus some runtime JSON artifacts under .sessions/.
Changes:
- Add core session management library (
sessionspackage) withSessionRegistry,CollaborationHub, andSharedMemoryimplementations, plus CLI, demo, and tests. - Integrate session management with the MCP server via new tools for registering/listing/pinging sessions, sending collaboration messages, and storing/querying shared memory.
- Add a
.STATUSupdater script and commit current.sessionsruntime data (active sessions, messages, and shared memory snapshots).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
prototypes/sessions/update_status.py |
Script to collect stats from the session registry/collab hub/shared memory and update a .STATUS beacon with active session info. |
prototypes/sessions/test_sessions.py |
Ad-hoc test suite covering SessionRegistry, CollaborationHub, SharedMemory, and an integration workflow. |
prototypes/sessions/setup.py |
Packaging metadata for the blackroad-sessions Python package and blackroad-sessions console entrypoint. |
prototypes/sessions/sessions/registry.py |
Implements Session and SessionRegistry with JSON-backed storage and basic querying and statistics. |
prototypes/sessions/sessions/memory.py |
Implements MemoryEntry and SharedMemory (JSON-backed shared key–value store with tagging, TTL, and search). |
prototypes/sessions/sessions/collaboration.py |
Implements Message and CollaborationHub for inter-session messaging, broadcast, pinging, and conversation threading. |
prototypes/sessions/sessions/cli.py |
Provides CLI commands for registering/listing/pinging/updating sessions, sending/broadcasting messages, querying memory, and displaying stats. |
prototypes/sessions/sessions/__main__.py |
Enables python -m sessions to invoke the CLI main entrypoint. |
prototypes/sessions/sessions/__init__.py |
Public package exports and version for the sessions package. |
prototypes/sessions/demo.py |
Interactive demo script showcasing session discovery, collaboration, shared memory, and a full multi-agent workflow. |
prototypes/sessions/README.md |
High-level documentation, quickstart, API examples, architecture, and CLI usage for the session management system. |
prototypes/mcp-server/blackroad_mcp/server.py |
Extends the MCP server with session/collaboration/memory tools and lazy-loaded accessors to the new session subsystems. |
.sessions/active_sessions.json |
JSON snapshot of current sessions as persisted by SessionRegistry. |
.sessions/messages/recent_messages.json |
JSON log of recent collaboration messages. |
.sessions/shared_memory/memory.json |
JSON snapshot of shared memory entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| import sys | ||
| from pathlib import Path | ||
| from datetime import datetime |
There was a problem hiding this comment.
Import of 'datetime' is not used.
| from datetime import datetime |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bed7181b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
prototypes/sessions/update_status.py
Outdated
| # Get bridge root | ||
| bridge_root = Path(__file__).parent.parent | ||
| status_file = bridge_root / ".STATUS" |
There was a problem hiding this comment.
This script says it updates the bridge .STATUS beacon, but bridge_root is set to Path(__file__).parent.parent, which resolves to .../prototypes for prototypes/sessions/update_status.py. As a result it writes prototypes/.STATUS instead of the real repo-root .STATUS, leaving the actual beacon stale and creating a second status file. Running the script in its intended context won’t update the file other tools read.
Useful? React with 👍 / 👎.
| data = { | ||
| 'entries': { | ||
| eid: entry.to_dict() | ||
| for eid, entry in self._entries.items() | ||
| }, |
There was a problem hiding this comment.
Shared memory is supposed to be cross-session, but _save serializes self._entries to disk without reloading or merging, so any concurrent writer with a stale in‑memory snapshot will overwrite new entries from other sessions (“last writer wins”). If two sessions call memory.set(...) in separate processes, the second write drops the first session’s entry. Consider reloading/merging or using a file lock/append strategy before overwriting memory.json.
Useful? React with 👍 / 👎.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Workflow Changes DetectedThis PR modifies GitHub Actions workflows. Please ensure:
Automated workflow review |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 25 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def find_replies(msg_id: str): | ||
| replies = [m for m in self._messages if m.in_reply_to == msg_id] | ||
| for reply in replies: | ||
| thread.append(reply) | ||
| find_replies(reply.message_id) | ||
|
|
||
| find_replies(thread[-1].message_id) |
There was a problem hiding this comment.
The recursive find_replies() function has no depth limit. If there's a circular reference in in_reply_to (due to a bug or corrupted data), this will cause infinite recursion and a stack overflow. Even without circular references, a very long conversation thread could hit Python's recursion limit.
Consider implementing this iteratively with a visited set to detect cycles, or adding a maximum depth parameter.
| def find_replies(msg_id: str): | |
| replies = [m for m in self._messages if m.in_reply_to == msg_id] | |
| for reply in replies: | |
| thread.append(reply) | |
| find_replies(reply.message_id) | |
| find_replies(thread[-1].message_id) | |
| def find_replies_iter(start_msg_id: str): | |
| """ | |
| Iteratively find all replies starting from the given message id. | |
| Uses a stack and a visited set to avoid recursion depth issues | |
| and to prevent infinite loops in case of cycles. | |
| """ | |
| visited = set() | |
| stack = [start_msg_id] | |
| while stack: | |
| current_id = stack.pop() | |
| if current_id in visited: | |
| continue | |
| visited.add(current_id) | |
| replies = [m for m in self._messages if m.in_reply_to == current_id] | |
| for reply in replies: | |
| thread.append(reply) | |
| stack.append(reply.message_id) | |
| # Preserve existing behavior: start from the last message in the thread | |
| find_replies_iter(thread[-1].message_id) |
| { | ||
| "entries": { | ||
| "c1e9fd53-77ac-4139-9d9b-1e9cca8c322f": { | ||
| "entry_id": "c1e9fd53-77ac-4139-9d9b-1e9cca8c322f", | ||
| "type": "state", | ||
| "key": "project_plan", | ||
| "value": { | ||
| "phase": "design", | ||
| "tasks": [ | ||
| "api-design", | ||
| "database-schema", | ||
| "frontend-mockups" | ||
| ], | ||
| "deadline": "2026-02-01" | ||
| }, | ||
| "session_id": "cece-001", | ||
| "timestamp": "2026-01-27T21:11:11.492758", | ||
| "ttl": null, | ||
| "tags": [ | ||
| "project", | ||
| "active", | ||
| "design" | ||
| ], | ||
| "metadata": {} | ||
| }, | ||
| "f628f9f0-2488-412d-8080-2142d48aae5a": { | ||
| "entry_id": "f628f9f0-2488-412d-8080-2142d48aae5a", | ||
| "type": "task", | ||
| "key": "task_api-design", | ||
| "value": { | ||
| "status": "completed", | ||
| "owner": "agent-002", | ||
| "completed_at": "2026-01-27" | ||
| }, | ||
| "session_id": "agent-002", | ||
| "timestamp": "2026-01-27T21:11:11.492905", | ||
| "ttl": null, | ||
| "tags": [ | ||
| "task", | ||
| "completed" | ||
| ], | ||
| "metadata": {} | ||
| }, | ||
| "9bedbbff-8385-4324-89ce-8a3dbd89772a": { | ||
| "entry_id": "9bedbbff-8385-4324-89ce-8a3dbd89772a", | ||
| "type": "state", | ||
| "key": "implementation_plan", | ||
| "value": { | ||
| "feature": "user-auth", | ||
| "steps": [ | ||
| "design", | ||
| "implement", | ||
| "test", | ||
| "review" | ||
| ] | ||
| }, | ||
| "session_id": "planner-001", | ||
| "timestamp": "2026-01-27T21:11:11.494364", | ||
| "ttl": null, | ||
| "tags": [ | ||
| "plan", | ||
| "auth" | ||
| ], | ||
| "metadata": {} | ||
| }, | ||
| "baac04e5-3913-459d-851d-fe03887f5605": { | ||
| "entry_id": "baac04e5-3913-459d-851d-fe03887f5605", | ||
| "type": "state", | ||
| "key": "code_user-auth", | ||
| "value": { | ||
| "file": "auth.py", | ||
| "lines": 250, | ||
| "tests": "passing" | ||
| }, | ||
| "session_id": "developer-001", | ||
| "timestamp": "2026-01-27T21:11:11.495052", | ||
| "ttl": null, | ||
| "tags": [ | ||
| "code", | ||
| "ready-for-review" | ||
| ], | ||
| "metadata": {} | ||
| }, | ||
| "5f0e106f-385c-4fdb-be39-ff23112215f3": { | ||
| "entry_id": "5f0e106f-385c-4fdb-be39-ff23112215f3", | ||
| "type": "decision", | ||
| "key": "review_user-auth", | ||
| "value": { | ||
| "status": "approved", | ||
| "issues": 0, | ||
| "suggestions": 2 | ||
| }, | ||
| "session_id": "reviewer-001", | ||
| "timestamp": "2026-01-27T21:11:11.496007", | ||
| "ttl": null, | ||
| "tags": [ | ||
| "review", | ||
| "approved" | ||
| ], | ||
| "metadata": {} | ||
| } | ||
| }, | ||
| "updated_at": "2026-01-27T21:11:11.496065" | ||
| } No newline at end of file |
There was a problem hiding this comment.
Runtime-generated data files should not be committed to version control. Add .sessions/ to .gitignore.
| { | |
| "entries": { | |
| "c1e9fd53-77ac-4139-9d9b-1e9cca8c322f": { | |
| "entry_id": "c1e9fd53-77ac-4139-9d9b-1e9cca8c322f", | |
| "type": "state", | |
| "key": "project_plan", | |
| "value": { | |
| "phase": "design", | |
| "tasks": [ | |
| "api-design", | |
| "database-schema", | |
| "frontend-mockups" | |
| ], | |
| "deadline": "2026-02-01" | |
| }, | |
| "session_id": "cece-001", | |
| "timestamp": "2026-01-27T21:11:11.492758", | |
| "ttl": null, | |
| "tags": [ | |
| "project", | |
| "active", | |
| "design" | |
| ], | |
| "metadata": {} | |
| }, | |
| "f628f9f0-2488-412d-8080-2142d48aae5a": { | |
| "entry_id": "f628f9f0-2488-412d-8080-2142d48aae5a", | |
| "type": "task", | |
| "key": "task_api-design", | |
| "value": { | |
| "status": "completed", | |
| "owner": "agent-002", | |
| "completed_at": "2026-01-27" | |
| }, | |
| "session_id": "agent-002", | |
| "timestamp": "2026-01-27T21:11:11.492905", | |
| "ttl": null, | |
| "tags": [ | |
| "task", | |
| "completed" | |
| ], | |
| "metadata": {} | |
| }, | |
| "9bedbbff-8385-4324-89ce-8a3dbd89772a": { | |
| "entry_id": "9bedbbff-8385-4324-89ce-8a3dbd89772a", | |
| "type": "state", | |
| "key": "implementation_plan", | |
| "value": { | |
| "feature": "user-auth", | |
| "steps": [ | |
| "design", | |
| "implement", | |
| "test", | |
| "review" | |
| ] | |
| }, | |
| "session_id": "planner-001", | |
| "timestamp": "2026-01-27T21:11:11.494364", | |
| "ttl": null, | |
| "tags": [ | |
| "plan", | |
| "auth" | |
| ], | |
| "metadata": {} | |
| }, | |
| "baac04e5-3913-459d-851d-fe03887f5605": { | |
| "entry_id": "baac04e5-3913-459d-851d-fe03887f5605", | |
| "type": "state", | |
| "key": "code_user-auth", | |
| "value": { | |
| "file": "auth.py", | |
| "lines": 250, | |
| "tests": "passing" | |
| }, | |
| "session_id": "developer-001", | |
| "timestamp": "2026-01-27T21:11:11.495052", | |
| "ttl": null, | |
| "tags": [ | |
| "code", | |
| "ready-for-review" | |
| ], | |
| "metadata": {} | |
| }, | |
| "5f0e106f-385c-4fdb-be39-ff23112215f3": { | |
| "entry_id": "5f0e106f-385c-4fdb-be39-ff23112215f3", | |
| "type": "decision", | |
| "key": "review_user-auth", | |
| "value": { | |
| "status": "approved", | |
| "issues": 0, | |
| "suggestions": 2 | |
| }, | |
| "session_id": "reviewer-001", | |
| "timestamp": "2026-01-27T21:11:11.496007", | |
| "ttl": null, | |
| "tags": [ | |
| "review", | |
| "approved" | |
| ], | |
| "metadata": {} | |
| } | |
| }, | |
| "updated_at": "2026-01-27T21:11:11.496065" | |
| } | |
| {} |
| def _rebuild_indices(self): | ||
| """Rebuild all indices.""" | ||
| self._index_by_key.clear() | ||
| self._index_by_session.clear() | ||
| self._index_by_tag.clear() | ||
|
|
||
| for entry_id, entry in self._entries.items(): | ||
| # Index by key | ||
| if entry.key not in self._index_by_key: | ||
| self._index_by_key[entry.key] = [] | ||
| self._index_by_key[entry.key].append(entry_id) | ||
|
|
||
| # Index by session | ||
| if entry.session_id not in self._index_by_session: | ||
| self._index_by_session[entry.session_id] = [] | ||
| self._index_by_session[entry.session_id].append(entry_id) | ||
|
|
||
| # Index by tags | ||
| for tag in entry.tags: | ||
| if tag not in self._index_by_tag: | ||
| self._index_by_tag[tag] = [] | ||
| self._index_by_tag[tag].append(entry_id) |
There was a problem hiding this comment.
The index rebuilding in _rebuild_indices() has O(n*m) complexity where n is the number of entries and m is the average number of tags per entry. For large memory stores, this could be slow. The method is called on every delete and after loading, potentially causing performance issues.
Consider incremental index updates rather than full rebuilds, or using a more efficient data structure for indexing.
| value={ | ||
| "phase": "design", | ||
| "tasks": ["api-design", "database-schema", "frontend-mockups"], | ||
| "deadline": "2026-02-01" |
There was a problem hiding this comment.
The deadline date "2026-02-01" in the demo data is in the past relative to the PR timestamp of "2026-01-27". While this is just demo data, it creates a confusing example where a project deadline has already passed at the time the code is being reviewed.
Consider using a future date or a relative date format in examples to avoid this confusion.
| "deadline": "2026-02-01" | |
| "deadline": "2026-12-31" |
| last_ping_dt = datetime.fromisoformat(self.last_ping.replace('Z', '+00:00')) | ||
| return datetime.utcnow() - last_ping_dt > timedelta(seconds=timeout_seconds) |
There was a problem hiding this comment.
The timezone handling in is_stale() is inconsistent. The replace('Z', '+00:00') approach assumes the timestamp ends with 'Z', but datetime.utcnow().isoformat() doesn't add a 'Z' suffix. This could cause a ValueError when parsing timestamps. Additionally, comparing a naive datetime (from datetime.utcnow()) with a timezone-aware datetime will raise a TypeError.
Consider making all datetime objects timezone-aware consistently, or use naive datetimes throughout.
| "Programming Language :: Python :: 3.8", | ||
| "Programming Language :: Python :: 3.9", | ||
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.11", |
There was a problem hiding this comment.
The package setup specifies Python 3.8+ compatibility but doesn't declare support for Python 3.12 or later versions. Given that the PR is dated 2026 and Python 3.12 was released in 2023, the classifiers should be updated to include newer Python versions that are likely being used.
Consider adding classifiers for Python 3.12 and possibly 3.13 if they're supported.
| "Programming Language :: Python :: 3.11", | |
| "Programming Language :: Python :: 3.11", | |
| "Programming Language :: Python :: 3.12", | |
| "Programming Language :: Python :: 3.13", |
| def _cleanup_stale(self, timeout_seconds: int = 300): | ||
| """Remove stale sessions.""" | ||
| stale = [ | ||
| sid for sid, session in self._sessions.items() | ||
| if session.is_stale(timeout_seconds) | ||
| ] | ||
|
|
||
| if not stale: | ||
| return | ||
|
|
||
| for sid in stale: | ||
| self._sessions[sid].status = SessionStatus.OFFLINE | ||
| # Don't delete, just mark offline for historical tracking | ||
|
|
||
| # Persist updated statuses so offline state is available across processes |
There was a problem hiding this comment.
The _cleanup_stale() method modifies session statuses to OFFLINE but doesn't remove them from the registry. Over time, this will cause the registry to accumulate offline sessions, potentially leading to memory bloat and performance degradation. While clear_offline() exists to manually clean these up, there's no automatic cleanup mechanism.
Consider implementing a maximum retention policy for offline sessions or automatically removing sessions that have been offline for an extended period.
| def _cleanup_stale(self, timeout_seconds: int = 300): | |
| """Remove stale sessions.""" | |
| stale = [ | |
| sid for sid, session in self._sessions.items() | |
| if session.is_stale(timeout_seconds) | |
| ] | |
| if not stale: | |
| return | |
| for sid in stale: | |
| self._sessions[sid].status = SessionStatus.OFFLINE | |
| # Don't delete, just mark offline for historical tracking | |
| # Persist updated statuses so offline state is available across processes | |
| def _cleanup_stale( | |
| self, | |
| timeout_seconds: int = 300, | |
| offline_retention_seconds: int = 3600, | |
| ): | |
| """ | |
| Clean up stale sessions. | |
| - Sessions that have been inactive for longer than `timeout_seconds` | |
| are marked as OFFLINE. | |
| - Sessions that are already OFFLINE and have remained stale for longer | |
| than `offline_retention_seconds` are removed from the registry. | |
| """ | |
| # Sessions that have gone stale but are not yet marked offline | |
| stale_to_offline = [ | |
| sid | |
| for sid, session in self._sessions.items() | |
| if session.is_stale(timeout_seconds) | |
| and session.status is not SessionStatus.OFFLINE | |
| ] | |
| # Sessions that have been offline for longer than the retention period | |
| offline_to_delete = [ | |
| sid | |
| for sid, session in self._sessions.items() | |
| if session.status is SessionStatus.OFFLINE | |
| and session.is_stale(offline_retention_seconds) | |
| ] | |
| if not stale_to_offline and not offline_to_delete: | |
| return | |
| # Mark newly stale sessions as offline | |
| for sid in stale_to_offline: | |
| self._sessions[sid].status = SessionStatus.OFFLINE | |
| # Remove long-offline sessions to prevent unbounded growth | |
| for sid in offline_to_delete: | |
| del self._sessions[sid] | |
| # Persist updated registry so status and removals are visible across processes |
| def _save_messages(self): | ||
| """Save recent messages to disk.""" | ||
| messages_file = self.messages_path / "recent_messages.json" | ||
|
|
||
| # Keep only recent messages (last 100) | ||
| recent = self._messages[-100:] |
There was a problem hiding this comment.
The message storage keeps only the last 100 messages in _save_messages(), but the in-memory _messages list can grow unbounded during runtime. If many messages are exchanged before the process restarts, this could lead to memory issues.
Consider also limiting the in-memory list size, or implementing a circular buffer to prevent unbounded growth.
| def set( | ||
| self, | ||
| session_id: str, | ||
| key: str, | ||
| value: Any, | ||
| type: MemoryType = MemoryType.STATE, | ||
| ttl: Optional[int] = None, | ||
| tags: Optional[List[str]] = None, | ||
| metadata: Optional[Dict[str, Any]] = None, | ||
| ) -> MemoryEntry: | ||
| """ | ||
| Store a value in shared memory. | ||
|
|
||
| Args: | ||
| session_id: Session storing the value | ||
| key: Memory key | ||
| value: Value to store | ||
| type: Type of memory entry | ||
| ttl: Time to live in seconds (None = forever) | ||
| tags: Tags for searching | ||
| metadata: Additional metadata | ||
|
|
||
| Returns: | ||
| The created MemoryEntry | ||
| """ | ||
| import uuid | ||
|
|
||
| entry = MemoryEntry( | ||
| entry_id=str(uuid.uuid4()), | ||
| type=type, | ||
| key=key, | ||
| value=value, | ||
| session_id=session_id, | ||
| timestamp=datetime.utcnow().isoformat(), | ||
| ttl=ttl, | ||
| tags=tags or [], | ||
| metadata=metadata or {}, | ||
| ) | ||
|
|
||
| self._entries[entry.entry_id] = entry | ||
|
|
||
| # Update indices | ||
| if key not in self._index_by_key: | ||
| self._index_by_key[key] = [] | ||
| self._index_by_key[key].append(entry.entry_id) | ||
|
|
||
| if session_id not in self._index_by_session: | ||
| self._index_by_session[session_id] = [] | ||
| self._index_by_session[session_id].append(entry.entry_id) | ||
|
|
||
| for tag in entry.tags: | ||
| if tag not in self._index_by_tag: | ||
| self._index_by_tag[tag] = [] | ||
| self._index_by_tag[tag].append(entry.entry_id) | ||
|
|
||
| self._save() | ||
|
|
||
| return entry |
There was a problem hiding this comment.
The shared memory system allows any session to store arbitrary data under any key without validation or sanitization. This creates several security concerns:
- No input validation on the
valueparameter (could store malicious code/data) - No access control - any session can overwrite memory set by any other session
- No size limits on values - could lead to disk exhaustion
- No validation of memory types or tags
Consider adding validation, access controls, and size limits to prevent abuse.
| except Exception as e: | ||
| print(f"Warning: Could not save session registry: {e}") |
There was a problem hiding this comment.
Catching broad exceptions and printing warnings can mask serious issues. If saving fails due to disk full, permission errors, or other problems, the system continues as if everything is fine. This means:
- Sessions might think they're registered but aren't persisted
- Messages could be lost silently
- Memory updates might not be saved
Consider being more specific about which exceptions to catch, and potentially raise critical errors (like disk full) rather than silently continuing. Also add proper logging.
Implementation Plan: Session Collaboration & Memory Functions
Create session registry system to track active sessions
Implement [COLLABORATION] function
Implement [MEMORY] function
Integration with existing systems
Testing and validation
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.