Merge pull request #121 from teng-lin/fix/process-local-session-state-scaling

Harden session-state topology signaling and add lease coordination seam

Author: teng-lin

README.md

@@ -164,6 +164,19 @@ Then open `http://localhost:5174`.
 - **Rate limiting**: Token bucket per consumer (configurable)
 - **Circuit breaker**: Sliding window prevents CLI restart cascades
 
+## Deployment Topology
+
+BeamCode currently runs as a **single-node runtime** for session coordination:
+
+- Live session state is **process-local** (in-memory runtime objects)
+- Persistent storage supports restart recovery, but **does not provide distributed coordination**
+- Running multiple BeamCode instances (especially with different `--data-dir`) creates isolated session islands, not a shared cluster
+
+The `/health` endpoint exposes this explicitly under `deployment`:
+- `topology: "single-node"`
+- `session_state_scope: "process-local"`
+- `horizontal_scaling: "unsupported"`
+
 See [SECURITY.md](./SECURITY.md) for the full threat model and cryptographic details.
 
 ## Documentation

docs/architecture.md

@@ -619,6 +619,8 @@ Policy services follow the **observe and advise** pattern: they subscribe to dom
 
 The SessionRepository owns the in-memory session map (`Map<string, Session>`), creates live `Session` objects, provides session/query helpers, and delegates persistence operations to `SessionStorage`.
 
+> **Topology constraint:** live session coordination is process-local. Persistence enables restart recovery, but does not make multi-instance BeamCode nodes share runtime state. Current topology is single-node. A lease-coordination seam (`SessionLeaseCoordinator`) now exists for future distributed ownership.
+
 **Responsibilities:**
 - **Own live sessions:** `getOrCreate()`, `get()`, `has()`, `keys()`, and `remove()` over live `Session` objects
 - **Expose query snapshots:** `getSnapshot()` and `getAllStates()` for read models

docs/review/architecture-review-2026-02-22.md

@@ -0,0 +1,133 @@
+# Comprehensive Architecture Review
+
+Date: 2026-02-22
+Scope: Repository-wide architecture review (boundaries, runtime flow, resilience, security, operability, and testability)
+
+## Findings (Ordered by Severity)
+
+### High
+
+1. Single long-lived token reused across API and WebSocket auth
+- Impact: Token leakage grants broad control until process restart.
+- Evidence: `src/bin/beamcode.ts:322`, `src/http/server.ts:13`
+- Detail: A single `consumerToken` is injected into HTML and reused for `/api/*` plus WebSocket `?token` auth.
+- Recommendation: Split token scopes (API vs WS), add rotation/revocation, and enforce short lifetime.
+
+2. Process-local session state limits resilience and horizontal scaling
+- Impact: Restart/failover can drop in-flight session metadata and pending control state.
+- Evidence: `src/core/session/session-repository.ts:120`, `src/core/session-coordinator.ts:108`
+- Detail: Session state is held in-process (`Map`) with local snapshoting, with no cluster-safe coordination.
+- Recommendation: Introduce shared session backing (e.g., Redis/DB) or explicitly document single-node constraints.
+
+### Medium
+
+1. HTTP rename path bypasses domain/policy command flow
+- Impact: Audit/RBAC/policy hooks can miss state mutations.
+- Evidence: `src/http/api-sessions.ts:158`
+- Detail: Rename mutates registry and broadcasts directly rather than going through coordinator/domain pipeline.
+- Recommendation: Route through coordinator/bridge commands so all mutations emit uniform domain events.
+
+2. Lifecycle transition invariants are not enforced
+- Impact: Runtime can enter invalid states, causing misleading lifecycle-driven behavior.
+- Evidence: `src/core/session/session-runtime.ts:336`, `src/core/session/session-lifecycle.ts:17`
+- Detail: Invalid transitions are logged but still applied.
+- Recommendation: Reject invalid transitions (no mutation) and optionally emit explicit error events.
+
+3. Queued message state is not durably persisted
+- Impact: Queued work can disappear after restart/crash.
+- Evidence: `src/core/session/message-queue-handler.ts:80`, `src/core/session/session-runtime.ts:150`
+- Detail: Queue slot updates stay in memory without corresponding persistence.
+- Recommendation: Persist queue-slot state on change and restore it during startup.
+
+4. Root redirect can point to stale/deleted session
+- Impact: `/` may redirect users to dead session IDs.
+- Evidence: `src/bin/beamcode.ts:393`, `src/http/server.ts:60`
+- Detail: `activeSessionId` is set during startup and not consistently updated during session churn.
+- Recommendation: Sync active session ID on create/delete/close events from coordinator.
+
+5. Entrypoint behavior has limited default test coverage
+- Impact: CLI flag/shutdown/bootstrap regressions can ship unnoticed.
+- Evidence: `vitest.config.ts:5`, `src/bin/beamcode.ts:57`
+- Detail: Default test configuration excludes `src/bin/**`.
+- Recommendation: Add targeted unit tests for arg parsing, lifecycle wiring, and shutdown ordering.
+
+6. Default observability is weak without optional flags
+- Impact: Degraded visibility into session health and failure trends.
+- Evidence: `src/bin/beamcode.ts:300`, `src/adapters/console-metrics-collector.ts:60`, `src/http/health.ts:4`
+- Detail: Console metrics are mostly debug-level and minimal by default.
+- Recommendation: Promote critical signals to default output and expose key counters in health/metrics.
+
+### Low
+
+1. Tunnel restart/failure signals are not strongly surfaced
+- Impact: Public connectivity degradation may go unnoticed.
+- Evidence: `src/relay/cloudflared-manager.ts:183`
+- Detail: Restart logic exists, but health/metrics surfacing is limited.
+- Recommendation: Emit explicit tunnel health metrics/events and include them in health checks.
+
+2. Message tracing summary is global, not session-scoped
+- Impact: Harder to diagnose per-session issues in multi-session runs.
+- Evidence: `src/core/messaging/message-tracer.ts:353`
+- Detail: Summary aggregates process-wide sets rather than per-session views.
+- Recommendation: Track summary by session ID and surface session-tagged diagnostics.
+
+3. Consumer-plane composition is tightly coupled to runtime internals
+- Impact: Higher refactor cost and weaker transport/domain separation.
+- Evidence: `src/core/session-bridge/compose-consumer-plane.ts:56`
+- Detail: Consumer composition reaches deeply into runtime state and helpers.
+- Recommendation: Narrow interfaces so transport interacts via explicit domain/service contracts.
+
+## Strengths
+
+- The bounded-context architecture is clear and mostly reflected in implementation (`docs/architecture.md:51`).
+- `SessionCoordinator` and `SessionBridge` centralize lifecycle flow and context composition (`src/core/session-coordinator.ts:108`, `src/core/session-bridge.ts:24`).
+- Runtime ownership is concentrated in session runtime/repository abstractions, which provides a solid foundation for future hardening (`src/core/session/session-runtime.ts:1`, `src/core/session/session-repository.ts:1`).
+
+## Open Questions
+
+1. Is single-node operation an intentional product constraint, or should active-active/multi-instance support be a target?
+2. Is shared API/WS token behavior acceptable only for local trust mode, or intended for tunneled/remote usage?
+3. Should queued message durability be a guaranteed contract across restarts?
+
+## Recommended Next Steps
+
+1. Implement token scope separation and rotation/revocation.
+2. Enforce lifecycle transition validity in runtime state machine.
+3. Persist queued-message state and restore on startup.
+4. Sync root redirect target with live session lifecycle events.
+5. Expand test coverage for CLI entrypoint and flag interactions.
+6. Improve default observability and health surfacing for critical failures.
+
+## Remediation Plan: Process-Local Session State
+
+### Phase 1 (implemented in this branch)
+
+1. Explicitly codify single-node constraints in runtime and docs.
+2. Expose deployment topology in `/health` so operators and automation can detect unsupported horizontal scaling.
+3. Emit startup warning to reduce accidental multi-instance assumptions.
+
+Delivered changes:
+- `/health` now includes:
+  - `deployment.topology = "single-node"`
+  - `deployment.session_state_scope = "process-local"`
+  - `deployment.horizontal_scaling = "unsupported"`
+- Startup now logs an explicit process-local session-state warning.
+- CLI "already running" guidance now clarifies that separate `--data-dir` instances are isolated, not clustered.
+- Documentation updated in `README.md` and `docs/architecture.md`.
+
+### Phase 2 (in progress)
+
+1. Introduce a shared coordination contract for live session ownership/leases.
+2. Add pluggable distributed backend (Redis/DB) behind the contract.
+3. Gate session mutation on lease ownership to prevent split-brain writes.
+
+Delivered in this branch:
+- Added a `SessionLeaseCoordinator` contract with in-memory default implementation.
+- Added lease ownership checks at central runtime mutation ingress (`RuntimeApi`) and in mutating `SessionRuntime` methods.
+- Added lifecycle lease semantics: `getOrCreateSession` now acquires/validates lease ownership; `removeSession`/`closeSession` release leases.
+- Routed consumer/backend mutation paths through lease-aware runtime APIs where possible.
+
+### Phase 3 (next)
+
+1. Add multi-instance integration tests (failover, reconnect, queue durability).
+2. Add metrics for lease contention/failover and propagate into health/readiness checks.

src/bin/beamcode.ts

@@ -351,6 +351,9 @@ export async function runBeamcode(argv: string[] = process.argv): Promise<void>
     if (err instanceof Error && err.message.includes("already running")) {
       console.error(`Error: ${err.message}`);
       console.error("Stop the other instance first, or use a different --data-dir.");
+      console.error(
+        "Note: separate --data-dir instances do not share session state and are not horizontally coordinated.",
+      );
       process.exit(1);
     }
     throw err;
@@ -454,12 +457,24 @@ export async function runBeamcode(argv: string[] = process.argv): Promise<void>
   });
 
   let activeSessionId = "";
+  logger.warn(
+    "Session state is process-local to this BeamCode instance; horizontal scaling requires external coordination that is not enabled in this runtime.",
+    { component: "startup", topology: "single-node", sessionStateScope: "process-local" },
+  );
 
   const httpServer = createBeamcodeServer({
     sessionCoordinator,
     activeSessionId,
     apiKey: consumerToken,
-    healthContext: { version, metrics },
+    healthContext: {
+      version,
+      metrics,
+      deployment: {
+        topology: "single-node",
+        sessionStateScope: "process-local",
+        horizontalScaling: "unsupported",
+      },
+    },
     prometheusCollector,
   });
 
@@ -538,6 +553,7 @@ export async function runBeamcode(argv: string[] = process.argv): Promise<void>
   Local:   ${localUrl}${tunnelSessionUrl ? `\n  Tunnel:  ${tunnelSessionUrl}` : ""}
 ${activeSessionId ? `\n  Session: ${activeSessionId}` : ""}
   Adapter: ${adapter.name}${config.noAutoLaunch ? " (no auto-launch)" : ""}
+  Topology: single-node (process-local session state)
   CWD:     ${config.cwd}
   API Key: ${consumerToken}
 

src/core/bridge/runtime-api.test.ts

@@ -1,6 +1,11 @@
 import { describe, expect, it, vi } from "vitest";
 import type { Logger } from "../../interfaces/logger.js";
+import type { WebSocketLike } from "../../interfaces/transport.js";
 import type { PolicyCommand } from "../interfaces/runtime-commands.js";
+import {
+  InMemorySessionLeaseCoordinator,
+  type SessionLeaseCoordinator,
+} from "../session/session-lease-coordinator.js";
 import type { Session, SessionRepository } from "../session/session-repository.js";
 import { RuntimeApi } from "./runtime-api.js";
 import type { RuntimeManager } from "./runtime-manager.js";
@@ -22,10 +27,16 @@ function createRuntimeStub() {
     executeSlashCommand: vi.fn().mockResolvedValue({ content: "ok", source: "emulated" }),
     handlePolicyCommand: vi.fn(),
     sendToBackend: vi.fn(),
+    handleInboundCommand: vi.fn(),
+    handleBackendMessage: vi.fn(),
+    handleSignal: vi.fn(),
   };
 }
 
-function createApi() {
+function createApi(options?: {
+  leaseCoordinator?: SessionLeaseCoordinator;
+  leaseOwnerId?: string;
+}) {
   const sessions = new Map<string, Session>();
   const store = {
     get: vi.fn((sessionId: string) => sessions.get(sessionId)),
@@ -42,7 +53,9 @@ function createApi() {
     error: vi.fn(),
   };
 
-  const api = new RuntimeApi({ store, runtimeManager, logger });
+  const leaseCoordinator = options?.leaseCoordinator ?? new InMemorySessionLeaseCoordinator();
+  const leaseOwnerId = options?.leaseOwnerId ?? "owner-1";
+  const api = new RuntimeApi({ store, runtimeManager, logger, leaseCoordinator, leaseOwnerId });
   return { api, sessions, runtime, runtimeManager, logger };
 }
 
@@ -144,4 +157,38 @@ describe("RuntimeApi", () => {
       message: "ok",
     });
   });
+
+  it("delegates inbound/backend handlers when session exists", () => {
+    const { api, sessions, runtime } = createApi();
+    sessions.set("s1", stubSession("s1"));
+    const ws = {} as WebSocketLike;
+    const inbound = { type: "interrupt" } as any;
+    const backend = { type: "result", metadata: {} } as any;
+
+    api.handleInboundCommand("s1", inbound, ws);
+    api.handleBackendMessage("s1", backend);
+    api.handleLifecycleSignal("s1", "backend:connected");
+
+    expect(runtime.handleInboundCommand).toHaveBeenCalledWith(inbound, ws);
+    expect(runtime.handleBackendMessage).toHaveBeenCalledWith(backend);
+    expect(runtime.handleSignal).toHaveBeenCalledWith("backend:connected");
+  });
+
+  it("blocks mutation when lease is held by another owner", () => {
+    const leaseCoordinator = new InMemorySessionLeaseCoordinator();
+    leaseCoordinator.ensureLease("s1", "owner-other");
+    const { api, sessions, runtime, logger } = createApi({
+      leaseCoordinator,
+      leaseOwnerId: "owner-1",
+    });
+    sessions.set("s1", stubSession("s1"));
+
+    api.sendInterrupt("s1");
+
+    expect(runtime.sendInterrupt).not.toHaveBeenCalled();
+    expect(logger.warn).toHaveBeenCalledWith(
+      "Session mutation blocked: lease not owned by this runtime",
+      expect.objectContaining({ sessionId: "s1", operation: "sendInterrupt" }),
+    );
+  });
 });