feat: server-client separation for remote monitoring by xukai92 · Pull Request #189 · ai-companion/betty

xukai92 · 2026-04-08T03:07:54Z

Summary

Extract session discovery/state into standalone HTTP server (betty server) with REST API endpoints for health, sessions, and alerts
Add RemoteStore client that polls the server and exposes the same interface as EventStore, so the TUI works unchanged
betty --server http://host:port connects to a remote server; falls back to local file scanning when no server is reachable
35 new tests covering JSON serialization round-trips, server endpoints, client polling, and fallback behavior

Closes #187

Test plan

betty with no server running uses local scanning (unchanged behavior)
betty server starts HTTP API on port 5557
betty --server http://localhost:5557 connects to running server
betty --server http://bad:9999 falls back to local with warning
All 35 new tests pass (pytest tests/test_server_client.py)
All existing tests unaffected (218 pass; 15 pre-existing failures from read-only FS)

🤖 Generated with Claude Code

Extract session discovery and state management into a standalone HTTP server (`betty server`) and refactor the TUI to optionally fetch from it (`betty --server URL`). Falls back to local file scanning when no server is reachable, preserving backward compatibility. - server.py: built-in http.server REST API (health, sessions, alerts) - client.py: RemoteStore with polling-based EventStore-compatible interface - cli.py: `betty server` subcommand + `--server` flag on main command - 35 new tests covering serialization, endpoints, and client behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR introduces a server/client split so Betty’s session discovery/state can run as a standalone HTTP service (betty server), while the existing TUI can optionally act as a remote client (betty --server ...) without directly scanning local session files.

Changes:

Added an HTTP REST API server exposing health, sessions, session detail, alerts, summarization, and annotations endpoints.
Added a polling RemoteStore client intended to mirror the EventStore interface for the TUI.
Extended the CLI with betty server and a --server option, plus a new test suite validating server/client behavior and serialization.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 14 comments.

File	Description
`src/betty/server.py`	New REST API server (http.server-based) plus JSON serializers for turns/sessions/tasks.
`src/betty/client.py`	New polling `RemoteStore` client + JSON deserializers + server reachability check.
`src/betty/cli.py`	Adds `--server` remote-connect option and a `server` subcommand for starting the API process.
`tests/test_server_client.py`	New tests covering serialization round-trips, server endpoints, polling client behavior, and fallback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Remove unused imports (urllib.error, threading, datetime, patch, DEFAULT_PORT) - Fix first-poll listener semantics: initialize turn count baseline without firing listeners (matches EventStore behavior) - Thread-safe listener access: add _listener_lock, snapshot before iterating - Use old_ids to prune _turn_counts for removed sessions - Use urlparse for path routing (handles query strings correctly) - Switch HTTPServer → ThreadingHTTPServer for concurrent client support - Validate Content-Length defensively with _MAX_BODY_SIZE cap - Add security warning when binding to non-loopback host - Add server.server_close() to all test teardowns to avoid socket leaks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Check stop event before each HTTP request and reduce timeouts from 5s to 2s so the poll loop exits promptly when stop() is called. Catch RuntimeError on thread join to suppress exceptions during interpreter shutdown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T04:11:49Z

+def _dict_to_session(d: dict[str, Any], include_turns: bool = False) -> Session:
+    """Reconstruct a Session from a server JSON dict."""
+    session = Session(
+        session_id=d["session_id"],
+        project_path=d.get("project_path", ""),
+        model=d.get("model", "unknown"),
+        started_at=_parse_datetime(d.get("started_at")),
+        active=d.get("active", True),
+        branch=d.get("branch"),
+    )
+    # Set pr_info if present
+    pr = d.get("pr_info")
+    if pr:
+        from .github import PRInfo
+        session.pr_info = PRInfo(
+            number=pr["number"],
+            title=pr["title"],
+            url=pr["url"],
+            state=pr["state"],
+        )
+    # Set plan
+    session.plan_content = d.get("plan_content")
+    session.plan_file_path = d.get("plan_file_path")
+    # Set tasks
+    tasks_raw = d.get("tasks", {})
+    session.tasks = {tid: _dict_to_task(td) for tid, td in tasks_raw.items()}
+    # Set turns
+    if include_turns and "turns" in d:
+        session.turns = [_dict_to_turn(t) for t in d["turns"]]
+    # Cache display_name from server so we don't need local filesystem
+    if "display_name" in d:
+        session._display_name_from_path = d["display_name"]
+    return session


_dict_to_session() drops the aggregate fields the server already provides (last_activity, turn_count, token/tool/word totals, etc.). Because Session.last_activity, total_tool_calls, and len(session.turns) are derived from session.turns, sessions coming from /api/sessions will appear idle with 0 turns/tools and will sort/group incorrectly in the manager view until they become the active session. Consider either (1) fetching session detail (with turns) for each session returned by /api/sessions, or (2) updating the client + models/UI to consume server-provided aggregates instead of recomputing from turns when turns aren’t included.

Copilot · 2026-04-08T04:11:50Z

+    # Set plan
+    session.plan_content = d.get("plan_content")
+    session.plan_file_path = d.get("plan_file_path")
+    # Set tasks
+    tasks_raw = d.get("tasks", {})
+    session.tasks = {tid: _dict_to_task(td) for tid, td in tasks_raw.items()}
+    # Set turns
+    if include_turns and "turns" in d:
+        session.turns = [_dict_to_turn(t) for t in d["turns"]]
+    # Cache display_name from server so we don't need local filesystem
+    if "display_name" in d:
+        session._display_name_from_path = d["display_name"]
+    return session
+


The server serializes plan_content/plan_file_path but not plan_updated_at, and the client doesn’t populate Session.plan_updated_at. The Textual UI’s refresh fingerprint relies on plan_updated_at, so plan changes on the server won’t trigger UI refreshes unless a new turn arrives. Include plan_updated_at in the session JSON (and parse it client-side) to preserve existing plan-refresh behavior.

Copilot · 2026-04-08T04:11:50Z

+
+import json
+import logging
+from http.server import HTTPServer, ThreadingHTTPServer, BaseHTTPRequestHandler


HTTPServer is imported but unused (the implementation uses ThreadingHTTPServer). If the repo runs with unused-import enforcement in CI, this will fail; even without it, keeping imports minimal avoids confusion. Remove the unused HTTPServer import.

Suggested change

from http.server import HTTPServer, ThreadingHTTPServer, BaseHTTPRequestHandler

from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler

Copilot · 2026-04-08T04:11:50Z

+    d: dict[str, Any] = {
+        "session_id": session.session_id,
+        "project_path": session.project_path,
+        "model": session.model,
+        "started_at": session.started_at.isoformat(),
+        "display_name": session.display_name,
+        "last_activity": session.last_activity.isoformat(),
+        "branch": session.branch,
+        "pr_info": pr,
+        "active": session.active,
+        "total_input_tokens": session.total_input_tokens,
+        "total_output_tokens": session.total_output_tokens,
+        "total_cache_creation_tokens": session.total_cache_creation_tokens,
+        "total_cache_read_tokens": session.total_cache_read_tokens,
+        "total_tool_calls": session.total_tool_calls,
+        "total_input_words": session.total_input_words,
+        "total_output_words": session.total_output_words,
+        "has_token_data": session.has_token_data,
+        "estimated_cost": session.estimated_cost,
+        "plan_content": session.plan_content,
+        "plan_file_path": session.plan_file_path,
+        "turn_count": len(session.turns),
+        "tasks": {tid: _task_to_dict(t) for tid, t in session.tasks.items()},
+    }
+    if include_turns:
+        d["turns"] = [_turn_to_dict(t) for t in session.turns]
+    return d


_session_to_dict() includes full plan_content (and full tasks) in the /api/sessions list response. Since RemoteStore polls this endpoint frequently, repeatedly sending potentially large plan markdown for every session can create unnecessary bandwidth/CPU overhead. Consider omitting large fields from the list endpoint (or gating them behind an include_plan/include_tasks flag) and only returning them from the per-session detail endpoint.

Copilot · 2026-04-08T04:11:50Z

+        "has_token_data": session.has_token_data,
+        "estimated_cost": session.estimated_cost,
+        "plan_content": session.plan_content,
+        "plan_file_path": session.plan_file_path,


The API currently doesn’t serialize Session.plan_updated_at even though the local store populates it and the UI uses it to show “Updated: …” and to decide when to refresh. Add plan_updated_at (ISO string or null) to the session dict and have the client restore it, otherwise remote plan timestamps/refresh will be incorrect.

Suggested change

"plan_file_path": session.plan_file_path,

"plan_file_path": session.plan_file_path,

"plan_updated_at": session.plan_updated_at.isoformat() if session.plan_updated_at else None,

Copilot · 2026-04-08T04:11:51Z

+                return b""
+            if length <= 0:
+                return b""
+            if length > _MAX_BODY_SIZE:


When Content-Length exceeds _MAX_BODY_SIZE, _read_body() returns b"" without draining the socket or sending an explicit error. The caller then responds with a generic 400 “Invalid JSON”, and the unread bytes may interfere with subsequent requests on the same connection. Consider returning a 413 Payload Too Large (and/or setting close_connection = True) when the body is oversized, and ensure the request body is consumed or the connection is closed.

Suggested change

if length > _MAX_BODY_SIZE:

if length > _MAX_BODY_SIZE:

# Do not leave oversized request bytes unread on a connection

# that might otherwise be reused for subsequent requests.

self.close_connection = True

Copilot · 2026-04-08T04:11:51Z

+        time.sleep(0.3)  # Let at least one poll happen
+        sessions = self.client.get_sessions()
+        assert len(sessions) == 1
+        self.client.stop()
+
+


test_polling_lifecycle relies on time.sleep(0.3) to “let at least one poll happen”. On slower/loaded CI runners this can be flaky (thread scheduling + server startup). Prefer waiting with a short retry loop until sessions are observed (with an overall timeout) rather than a fixed sleep.

Suggested change

time.sleep(0.3) # Let at least one poll happen

sessions = self.client.get_sessions()

assert len(sessions) == 1

self.client.stop()

try:

deadline = time.monotonic() + 2.0

sessions = []

while time.monotonic() < deadline:

sessions = self.client.get_sessions()

if len(sessions) == 1:

break

time.sleep(0.05)

assert len(sessions) == 1

finally:

self.client.stop()

The join(timeout=2.0) was blocking the main thread when an HTTP request was in-flight, causing betty to hang on quit. Since the polling thread is already a daemon thread, Python won't wait for it on exit — just signal the stop event and return immediately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

All watcher threads (ProjectWatcher, TranscriptWatcher) and their watchdog observers are daemon threads, so joining them on shutdown just blocks the main thread for up to 11+ seconds (6s + 2s + 3s per session) waiting for threads that will exit on their own. - watcher.py: remove thread.join(3s) and observer.join(1s) - project_watcher.py: remove thread.join(6s) and observer.join(2s) - agent.py: add cancel_futures=True to executor shutdown store.stop() now signals all threads to stop and returns immediately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T04:39:20Z

+def _dict_to_session(d: dict[str, Any], include_turns: bool = False) -> Session:
+    """Reconstruct a Session from a server JSON dict."""
+    session = Session(
+        session_id=d["session_id"],
+        project_path=d.get("project_path", ""),
+        model=d.get("model", "unknown"),
+        started_at=_parse_datetime(d.get("started_at")),
+        active=d.get("active", True),
+        branch=d.get("branch"),
+    )


RemoteStore reconstructs Session objects from /api/sessions, but _dict_to_session() currently ignores server-provided aggregate fields like last_activity, turn_count, total_tool_calls, and token/word totals. Since the TUI computes things like session.last_activity, len(session.turns), and session.total_tool_calls from session.turns, non-active sessions fetched from the list endpoint will appear to have 0 turns/tools and “stale” activity in manager view. Consider either (1) persisting these aggregate values onto the Session object (e.g., via dedicated fields or by priming the cached totals) and updating the UI to use them, or (2) including enough turn data in the list endpoint to keep these properties accurate without fetching full transcripts for every session.

Copilot · 2026-04-08T04:39:20Z

+import json
+import logging
+import threading
+import urllib.request
+from datetime import datetime
+from typing import Any, Callable
+
+from .alerts import Alert, AlertLevel
+from .models import Session, TaskState, Turn
+
+logger = logging.getLogger(__name__)
+
+# How often the background thread polls the server (seconds).


logger = logging.getLogger(__name__) is currently unused in this module. If the repo runs linting (ruff/flake8), this can fail CI; either use it for debug logging in the exception handlers or remove it.

Copilot · 2026-04-08T04:39:21Z

+
+import json
+import logging
+from http.server import HTTPServer, ThreadingHTTPServer, BaseHTTPRequestHandler


HTTPServer is imported but not used (the module uses ThreadingHTTPServer). Please remove the unused import to avoid lint failures and keep imports minimal.

Suggested change

from http.server import HTTPServer, ThreadingHTTPServer, BaseHTTPRequestHandler

from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler

Copilot · 2026-04-08T04:39:21Z

+    except KeyboardInterrupt:
+        pass
+    finally:
+        server.shutdown()


run_server() calls server.shutdown() but never calls server.server_close(). shutdown() stops the serve loop, but server_close() is what closes the listening socket; without it, the port/file descriptor can remain open longer than expected (especially relevant in tests or when restarting the server in-process). Consider adding server.server_close() in the finally block.

Suggested change

server.shutdown()

server.shutdown()

server.server_close()

Copilot · 2026-04-08T04:39:21Z

+        def _read_body(self) -> bytes:
+            raw = self.headers.get("Content-Length", "0")
+            try:
+                length = int(raw)
+            except (ValueError, TypeError):
+                return b""
+            if length <= 0:
+                return b""
+            if length > _MAX_BODY_SIZE:
+                return b""
+            return self.rfile.read(length)


_read_body() silently returns b"" when Content-Length exceeds _MAX_BODY_SIZE, which then surfaces as a generic 400 “Invalid JSON”. Consider sending an explicit 413 Payload Too Large (or at least a clear 4xx error) so clients can distinguish oversized bodies from malformed JSON, and to avoid reading/processing ambiguous empty bodies.

- Remove unused imports (urllib.error, threading, datetime, patch, DEFAULT_PORT) - Fix first-poll listener semantics: initialize turn count baseline without firing listeners (matches EventStore behavior) - Thread-safe listener access: add _listener_lock, snapshot before iterating - Use old_ids to prune _turn_counts for removed sessions - Use urlparse for path routing (handles query strings correctly) - Switch HTTPServer → ThreadingHTTPServer for concurrent client support - Validate Content-Length defensively with _MAX_BODY_SIZE cap - Add security warning when binding to non-loopback host - Add server.server_close() to all test teardowns to avoid socket leaks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 8, 2026 03:07

Copilot started reviewing on behalf of xukai92 April 8, 2026 03:08 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

xukai92 and others added 2 commits April 8, 2026 03:21

Copilot AI review requested due to automatic review settings April 8, 2026 04:05

Copilot started reviewing on behalf of xukai92 April 8, 2026 04:06 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

xukai92 and others added 2 commits April 8, 2026 04:14

Copilot AI review requested due to automatic review settings April 8, 2026 04:33

Copilot started reviewing on behalf of xukai92 April 8, 2026 04:34 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

xukai92 merged commit f72f8ae into main Apr 8, 2026
7 checks passed

xukai92 deleted the server-client-separation branch April 8, 2026 13:37

	from http.server import HTTPServer, ThreadingHTTPServer, BaseHTTPRequestHandler
	from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler

	"plan_file_path": session.plan_file_path,
	"plan_file_path": session.plan_file_path,
	"plan_updated_at": session.plan_updated_at.isoformat() if session.plan_updated_at else None,

-            if length > _MAX_BODY_SIZE:
+            if length > _MAX_BODY_SIZE:
+                # Do not leave oversized request bytes unread on a connection
+                # that might otherwise be reused for subsequent requests.
+                self.close_connection = True

Uh oh!

Conversation

xukai92 commented Apr 8, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants