RAGE #131: principal-scoped conversation and memory isolation #148

Merged

lost-rob0t merged 31 commits from rage/131-principal-isolation into master

2026-08-22 23:26:20 +00:00

lost-rob0t commented

2026-08-22 19:30:06 +00:00

(Migrated from github.com)

RAGE implementation for #131 under daemon epic #127.

Immutable RAGE start: 71615bcd4cdece940db433cb79ea46fe9ad085e3.

Implemented and regression-proven:

principal-bound ConversationStore with owner columns and owner-scoped read/write/search paths;
foreign and missing conversation IDs use the same not-found behavior;
local-owner-only legacy row claim with concurrent SQLite migration coverage;
principal-bound MemoryManager, per-principal vector namespaces, owner metadata/filtering, and guest/ephemeral local-only state;
authenticated PrincipalContext is threaded through the default supervisor → backend → AgentManager path;
live transient ConversationManager context now carries that same canonical principal and remains instance-isolated per runtime;
private runtime event subscriptions remain per-principal;
no #51 ContextManager substitute and no second conversation persistence subsystem.

Latest TDD slice: test-only 46a8330f453def30f060af9dc4fb9bfa283dc684 produced the expected red (1 failed, 890 passed) because live conversation context had no principal. Minimal production wiring at 4880f2adb2b66265b70d860c64378c4f7d804d6e passed the complete repository/Nix/packaging gate in CI #371. The RAGE work log records all red/green evidence and the final exact-head gate requirement.

Closes #131.

RAGE implementation for #131 under daemon epic #127. Immutable RAGE start: `71615bcd4cdece940db433cb79ea46fe9ad085e3`. Implemented and regression-proven: - principal-bound `ConversationStore` with owner columns and owner-scoped read/write/search paths; - foreign and missing conversation IDs use the same not-found behavior; - local-owner-only legacy row claim with concurrent SQLite migration coverage; - principal-bound `MemoryManager`, per-principal vector namespaces, owner metadata/filtering, and guest/ephemeral local-only state; - authenticated `PrincipalContext` is threaded through the default supervisor → backend → `AgentManager` path; - live transient `ConversationManager` context now carries that same canonical principal and remains instance-isolated per runtime; - private runtime event subscriptions remain per-principal; - no #51 ContextManager substitute and no second conversation persistence subsystem. Latest TDD slice: test-only `46a8330f453def30f060af9dc4fb9bfa283dc684` produced the expected red (1 failed, 890 passed) because live conversation context had no principal. Minimal production wiring at `4880f2adb2b66265b70d860c64378c4f7d804d6e` passed the complete repository/Nix/packaging gate in CI #371. The RAGE work log records all red/green evidence and the final exact-head gate requirement. Closes #131.

lost-rob0t (Migrated from github.com) reviewed

2026-08-22 21:48:51 +00:00

lost-rob0t (Migrated from github.com) left a comment

RAGE review against #131 acceptance, while treating this branch as the architectural baseline for the next daemon/client work.

Blocking findings before #131 can be called complete:

The authenticated principal is not wired into the default daemon backend/memory path. RuntimeSupervisor._build_default_host(_principal, bus) currently discards _principal; RuntimeHost builds the normal AgentRuntimeBackend; AgentManager then calls build_memory_manager(memory_config) without a principal. The new principal-bound MemoryManager API is therefore proven in direct tests but not structurally enforced by the production daemon path. Please thread PrincipalContext through the runtime/backend construction and add a test that starts two real supervisor runtimes and proves their managers are owner-bound without test-only dependency injection.
The durable conversation boundary is not on the daemon agent path. #148 hardens ConversationStore, but the default AgentManager still owns ConversationManager, whose history is an in-memory list. Nothing in the current branch connects the authenticated principal/runtime to the owner-bound durable conversation store. #131 acceptance says daemon conversation persistence and private state are principal-scoped, so either wire the canonical persistent boundary into the runtime path in this issue or explicitly prove/reconcile the intended daemon conversation owner before closing #131. Direct store tests alone do not prove the service path.
Exact-head CI is red because the new runtime-isolation test violates the event contract. In test_principal_event_subscriptions_do_not_cross_runtime_buses, the fake PublishingHost publishes the submitted SubmitTurn RuntimeCommand directly to RuntimeEventBus; that bus correctly accepts only RuntimeEvent, producing the current TypeError. Make the fake emit a real provider-neutral runtime event (or observe commands separately) so the test proves cross-principal event isolation without weakening the bus type boundary. Current exact-head run #352 is 884 passed / 1 failed; platform jobs and the 91 protocol tests are green.

Hardening that should be covered while touching these seams:

make explicit-principal MemoryManager construction require/validate PrincipalContext rather than arbitrary Any-shaped objects, preserving only a deliberate compatibility default if needed;
define the transient-principal rule centrally (guest/ephemeral) rather than keying persistence behavior to one magic kind string;
add concurrent SQLite schema-upgrade coverage: two principal stores opening an old DB must not race on ALTER TABLE after both observe the pre-migration schema.

None of these findings argue against the owner-column / per-principal namespace design. They are integration and lifecycle gaps: #131 should prove the actual daemon construction path inherits those boundaries, not only the persistence classes in isolation.

RAGE review against #131 acceptance, while treating this branch as the architectural baseline for the next daemon/client work. Blocking findings before #131 can be called complete: 1. **The authenticated principal is not wired into the default daemon backend/memory path.** `RuntimeSupervisor._build_default_host(_principal, bus)` currently discards `_principal`; `RuntimeHost` builds the normal `AgentRuntimeBackend`; `AgentManager` then calls `build_memory_manager(memory_config)` without a principal. The new principal-bound `MemoryManager` API is therefore proven in direct tests but not structurally enforced by the production daemon path. Please thread `PrincipalContext` through the runtime/backend construction and add a test that starts two real supervisor runtimes and proves their managers are owner-bound without test-only dependency injection. 2. **The durable conversation boundary is not on the daemon agent path.** #148 hardens `ConversationStore`, but the default `AgentManager` still owns `ConversationManager`, whose history is an in-memory list. Nothing in the current branch connects the authenticated principal/runtime to the owner-bound durable conversation store. #131 acceptance says daemon conversation persistence and private state are principal-scoped, so either wire the canonical persistent boundary into the runtime path in this issue or explicitly prove/reconcile the intended daemon conversation owner before closing #131. Direct store tests alone do not prove the service path. 3. **Exact-head CI is red because the new runtime-isolation test violates the event contract.** In `test_principal_event_subscriptions_do_not_cross_runtime_buses`, the fake `PublishingHost` publishes the submitted `SubmitTurn` `RuntimeCommand` directly to `RuntimeEventBus`; that bus correctly accepts only `RuntimeEvent`, producing the current TypeError. Make the fake emit a real provider-neutral runtime event (or observe commands separately) so the test proves cross-principal event isolation without weakening the bus type boundary. Current exact-head run #352 is 884 passed / 1 failed; platform jobs and the 91 protocol tests are green. Hardening that should be covered while touching these seams: - make explicit-principal `MemoryManager` construction require/validate `PrincipalContext` rather than arbitrary `Any`-shaped objects, preserving only a deliberate compatibility default if needed; - define the transient-principal rule centrally (`guest`/ephemeral) rather than keying persistence behavior to one magic kind string; - add concurrent SQLite schema-upgrade coverage: two principal stores opening an old DB must not race on `ALTER TABLE` after both observe the pre-migration schema. None of these findings argue against the owner-column / per-principal namespace design. They are integration and lifecycle gaps: #131 should prove the actual daemon construction path inherits those boundaries, not only the persistence classes in isolation.