diff --git a/CLAUDE.md b/CLAUDE.md index 282c4df..fe8d66a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,11 +28,13 @@ Three global broadcast buses — **never add a fourth without checking these fir | Bus | Cap | Events | File | |-----|-----|--------|------| | `ChatEventBus` | 256 | user message, assistant response, compaction done | `core-api/src/bus.rs` | -| `SystemEventBus` | 64 | provider (un)registered, config key updated, job completed, session cancelled | `core-api/src/system_bus.rs` | +| `SystemEventBus` | 64 | provider (un)registered, config key updated, job completed, session cancelled, **user created/deleted/mounts-changed** | `core-api/src/system_bus.rs` | | `GlobalEvent` (per-user) | 512 | all `ServerEvent` variants → WS clients + inbox lifecycle | `core-api/src/events.rs` | Plus internal `mpsc` queues: per-source `SourceInbox` (message serialization) and a central `notify` queue (background agents → user). +**The user-lifecycle reconciler** is the worked example of the rule. Creating a user, deleting one, or changing a shared-folder/project membership all need Docker work (provision, tear down, recreate with new bind mounts). None of the endpoints that make those changes touches `ContainerManager`: each announces `SystemEvent::User{Created,Deleted,MountsChanged}` **after** its DB write, and one subscriber — `skald::wiring::spawn_user_lifecycle`, spawned post-construction because it reacts through `Skald`'s own accessors, holding only a `Weak` — does the reacting, sequentially and best-effort. The payoff is that a *future* endpoint granting membership cannot forget to remount, because remounting was never its job. Reactions never block the HTTP response, and a failure settles at the user's next login or at boot reconciliation. + **Before you add a direct function call or a new import between two components, stop and ask:** is one component producing data another needs? If yes, add a variant to an existing bus and spawn a subscriber. Don't call `some_manager.log_thing(...)` from the producer — emit a `ThingHappened` event on `SystemEventBus` and let the manager subscribe. **A new `mpsc::channel` or `broadcast::channel` is a code-review flag.** Nine times out of ten you want one of the three buses above. If you truly need a new one, be ready to explain why none of the existing three fits. @@ -158,11 +160,11 @@ Two views, **one storage**: the fs-tools run **host-side** in the Skald process **Containment** (`resolve_host_path`): every physical fs-tool op canonicalizes the resolved path (following symlinks) and prefix-checks it against its mount base, **fail-closed**. Since the same tree is writable from inside the container, a symlink planted there that points outside the home/shared root is caught here — the host-side tool never escapes the user's workspace. `grep_files` stays disk-only (regex ≠ FTS; memory → `memory_search`) but resolves its root the same way. `execute_cmd`'s `workdir` is an agent path mapped to its container path via `UserFs::to_container`. -The threading: `UserContext.fs` (built by `container::build_user_fs` at login, snapshotting shared memberships) → `ChatSessionManager` → `ChatSessionHandler.fs` → `ToolContext.fs`. **Admin CRUD is wired** (`src/frontend/api/shared_folders.rs` — `GET/POST /api/shared-folders`, `PATCH/DELETE /api/shared-folders/{id}`, `POST`/`DELETE .../members[/{user_id}]`; UI `shared-folders.js`): a create/describe/delete + per-member `can_write` surface, and each mutation calls a best-effort `remount(user)` that rebuilds the affected user's fs + container mounts **in place** — so a membership change lands without a re-login (blueprint §6's "admin CRUD" + "membership refresh without re-login" TODOs, now closed; it still settles at next login/boot if the live remount fails). `execute_cmd` /stop is robust: the command runs under `setsid -w` in its own process-group (leader pid recorded in a container pidfile), and a `KillReaper` drop-guard reaps that group on /stop **or** timeout via a detached `docker exec` that walks `/proc` and kills members by **positive pid** (the container's dash mishandles `kill -`); the pidfile is passed positionally (`$1`), and the container's `--init` (tini) reaps the killed processes so no zombies accumulate. **Per-user MCP connectors now run inside this container** (§7) — the container infra enabled it; see the MCP connectors section. +The threading: `UserContext.fs` (built by `container::build_user_fs` at login, snapshotting shared memberships) → `ChatSessionManager` → `ChatSessionHandler.fs` → `ToolContext.fs`. **Admin CRUD is wired** (`src/frontend/api/shared_folders.rs` — `GET/POST /api/shared-folders`, `PATCH/DELETE /api/shared-folders/{id}`, `POST`/`DELETE .../members[/{user_id}]`; UI `shared-folders.js`): a create/describe/delete + per-member `can_write` surface, and each mutation emits `SystemEvent::UserMountsChanged`, on which the lifecycle reconciler runs `Skald::refresh_user_mounts` — rebuilding the affected user's fs + container mounts **in place**, so a membership change lands without a re-login (blueprint §6's "admin CRUD" + "membership refresh without re-login" TODOs, now closed; it still settles at next login/boot if the live remount fails). `execute_cmd` /stop is robust: the command runs under `setsid -w` in its own process-group (leader pid recorded in a container pidfile), and a `KillReaper` drop-guard reaps that group on /stop **or** timeout via a detached `docker exec` that walks `/proc` and kills members by **positive pid** (the container's dash mishandles `kill -`); the pidfile is passed positionally (`$1`), and the container's `--init` (tini) reaps the killed processes so no zombies accumulate. **Per-user MCP connectors now run inside this container** (§7) — the container infra enabled it; see the MCP connectors section. ## Projects -A **project** is a shareable, self-service workspace: a folder at `{WD}/projects/{owner_userid}/{slug}` plus membership in the registry. `projects` (accessor `db/projects.rs` — slug is immutable, `UNIQUE(owner_user_id, slug)`) + `project_members` (junction with `can_write`; the owner is always a write-member, so a private project = one member). Sharing is **not** admin-gated: the owner and any write-member can add/remove/re-grant members and edit metadata; only the owner can delete. Each membership mutation remounts the affected users' containers in place (`Skald::refresh_user_mounts`). The mount appears in the agent namespace as `projects/{owner_username}/{slug}` (host keys on the stable userid, agent path on the username) — read-only members get a read-only bind mount in the container. +A **project** is a shareable, self-service workspace: a folder at `{WD}/projects/{owner_userid}/{slug}` plus membership in the registry. `projects` (accessor `db/projects.rs` — slug is immutable, `UNIQUE(owner_user_id, slug)`) + `project_members` (junction with `can_write`; the owner is always a write-member, so a private project = one member). Sharing is **not** admin-gated: the owner and any write-member can add/remove/re-grant members and edit metadata; only the owner can delete. Each membership mutation emits `SystemEvent::UserMountsChanged` for the affected user; the lifecycle reconciler remounts their container in place (`Skald::refresh_user_mounts`), so the folder is browsable at once (the explorer reads host-side) and reachable from `execute_cmd` a moment later. The mount appears in the agent namespace as `projects/{owner_username}/{slug}` (host keys on the stable userid, agent path on the username) — read-only members get a read-only bind mount in the container. **API** (`src/frontend/api/projects.rs`): `GET/POST /api/projects`, `GET/PUT/DELETE /api/projects/{id}`, `POST /api/projects/{id}/members`, `DELETE .../members/{user_id}`, `POST /api/projects/{id}/session`. `ProjectDetail` carries `root_path` — the agent path of the folder, computed server-side (owner username ≠ `owner_name`, which may be a display name) — the explorer's root. A `project-{id}` chat source provisions the `project-coordinator` agent with a project `RunContext` (`provisioning_for_source` → `skald_core::projects::build_project_run_context`: `project_root` + a system block with name/description/folder/members); every member keeps their **own private** `project-{id}` session — only the folder is shared. diff --git a/crates/core-api/src/system_bus.rs b/crates/core-api/src/system_bus.rs index 032d7f7..ed7979b 100644 --- a/crates/core-api/src/system_bus.rs +++ b/crates/core-api/src/system_bus.rs @@ -54,6 +54,27 @@ pub enum SystemEvent { SessionCancelled { session_id: i64, }, + + // ── User lifecycle (blueprint §6) ───────────────────────────────────────── + // Announced by whoever changed the row; the reaction — provisioning, tearing + // down or remounting a Docker container — belongs to the lifecycle reconciler + // in `skald-core`, never to the endpoint that made the change. + /// A user was created, by any creator (the Users admin page, the first-run + /// setup wizard). Their execution sandbox has to be provisioned. + UserCreated { + user_id: String, + }, + /// A user was deleted. Their sandbox has to be torn down. + UserDeleted { + user_id: String, + }, + /// A user's **mount topology** changed — a shared-folder or project membership + /// was granted, revoked or re-graded (RO ⇄ RW). Their container must be + /// recreated against the new mount set, and a live session's filesystem view + /// refreshed with it. + UserMountsChanged { + user_id: String, + }, } // ── Bus ─────────────────────────────────────────────────────────────────────── diff --git a/crates/skald-core/src/skald/mod.rs b/crates/skald-core/src/skald/mod.rs index dec7c7f..f30d0b5 100644 --- a/crates/skald-core/src/skald/mod.rs +++ b/crates/skald-core/src/skald/mod.rs @@ -31,7 +31,7 @@ use bundles::{Conversation, Infra, Integrations, Interaction, Media, Models, Tas use runtime::Runtime; use user_context::{UserContextFactory, UserContextRegistry}; pub use user_context::UserContext; -use wiring::{spawn_background, wire}; +use wiring::{spawn_background, spawn_user_lifecycle, wire}; pub struct Skald { rt: Runtime, @@ -107,6 +107,10 @@ impl Skald { // from WebFrontend::start, once the router factory is wired. skald.plugin_manager().set_skald(Arc::clone(&skald)); + // Same reason: the reconciler reacts through `Skald`'s own accessors, so it + // can only be spawned once the instance exists (blueprint §6). + spawn_user_lifecycle(&skald); + Ok(skald) } @@ -131,8 +135,9 @@ impl Skald { self.rt.users.lock_all().await; } - /// The container manager, so the API layer can provision (on user create) or - /// remove (on user delete) a user's container. + /// The container manager. The API layer no longer calls it: user provisioning + /// and teardown are driven by the lifecycle reconciler reacting to + /// `SystemEvent::User*` (see `wiring::spawn_user_lifecycle`). pub fn container(&self) -> ContainerManager { self.container.clone() } diff --git a/crates/skald-core/src/skald/wiring.rs b/crates/skald-core/src/skald/wiring.rs index 572a68f..9ae7cbd 100644 --- a/crates/skald-core/src/skald/wiring.rs +++ b/crates/skald-core/src/skald/wiring.rs @@ -4,12 +4,14 @@ //! //! Owner-bound background loops (cron, session-cancel, ticket-listener, tic) have //! moved per-user into `UserContextFactory::build`. What remains here are the -//! instance-wide tasks: LLM-log cleanup on the registry pool, and MCP server -//! initialization. +//! instance-wide tasks: LLM-log cleanup on the registry pool, MCP server +//! initialization, and the user-lifecycle reconciler (which needs the finished +//! `Arc` and is therefore spawned separately, after construction). use std::sync::Arc; -use tracing::info; +use core_api::system_bus::{RecvError, SystemEvent}; +use tracing::{info, warn}; use crate::config::CoreConfig; use crate::elicitation::ElicitationBridge; @@ -77,3 +79,70 @@ pub(super) fn spawn_background( }); } } + +/// Spawns the **user-lifecycle reconciler** — the single subscriber that turns +/// user and membership events into container work (blueprint §6). +/// +/// Producers (the Users admin page, the setup wizard, the shared-folder and +/// project membership endpoints) only announce *what changed*; none of them +/// reaches into [`ContainerManager`](crate::container::ContainerManager). That +/// is the point of routing this through the bus rather than calling the manager +/// from each handler: a future endpoint that grants membership cannot forget to +/// remount, because remounting was never its job. +/// +/// Every reaction is **best-effort by contract**: the row is already committed +/// when the event fires, so a Docker hiccup is logged, never surfaced to the +/// caller — the state settles at the user's next login or at boot +/// reconciliation. Events are handled **sequentially**, which also serialises +/// concurrent `docker` operations on the same container. +/// +/// Spawned after `Skald` is fully built (like `set_skald`) because +/// [`Skald::refresh_user_mounts`] is an accessor on the finished instance. The +/// back-reference is [`std::sync::Weak`], so this task never keeps `Skald` alive. +pub(super) fn spawn_user_lifecycle(skald: &Arc) { + let weak = Arc::downgrade(skald); + let shutdown = skald.rt.shutdown_token.clone(); + let mut rx = skald.rt.system_bus.subscribe(); + + skald.rt.supervisor.spawn("user-lifecycle", async move { + loop { + let event = tokio::select! { + _ = shutdown.cancelled() => break, + event = rx.recv() => match event { + Ok(e) => e, + // A dropped event costs a stale container until the user's next + // login/boot — never a lost row, since the DB write came first. + Err(RecvError::Lagged(n)) => { + warn!(n, "user-lifecycle: system_bus lagged; container state may be stale until next login/boot"); + continue; + } + Err(RecvError::Closed) => break, + }, + }; + + let Some(skald) = weak.upgrade() else { break }; + match event { + SystemEvent::UserCreated { user_id } => { + if let Err(e) = skald.container().ensure(&user_id).await { + warn!(user = %user_id, error = %e, + "user-lifecycle: failed to provision container (retried at next boot)"); + } + } + SystemEvent::UserDeleted { user_id } => { + if let Err(e) = skald.container().remove(&user_id).await { + warn!(user = %user_id, error = %e, + "user-lifecycle: failed to remove container"); + } + } + SystemEvent::UserMountsChanged { user_id } => { + if let Err(e) = skald.refresh_user_mounts(&user_id).await { + warn!(user = %user_id, error = %e, + "user-lifecycle: remount failed (settles at next login/boot)"); + } + } + _ => {} + } + } + info!("user-lifecycle: reconciler stopped"); + }); +} diff --git a/src/frontend/api/projects.rs b/src/frontend/api/projects.rs index 8227836..18eab37 100644 --- a/src/frontend/api/projects.rs +++ b/src/frontend/api/projects.rs @@ -16,6 +16,7 @@ use axum::{ extract::{Path, State}, http::StatusCode, }; +use core_api::system_bus::SystemEvent; use serde::{Deserialize, Serialize}; use skald_core::db::project_members::{ProjectAccess, ProjectMember}; @@ -143,13 +144,14 @@ fn project_dir(owner_user_id: &str, slug: &str) -> Result { .join(slug)) } -/// Recreate a user's container with the new mount set (best-effort — settles at their -/// next login/boot on failure). See [`Skald::refresh_user_mounts`]. -async fn remount(skald: &Skald, user_id: &str) { - if let Err(e) = skald.refresh_user_mounts(user_id).await { - tracing::warn!(user = %user_id, error = %e, - "project remount failed (settles at next login/boot)"); - } +/// Announces that a user's mount topology changed; the lifecycle reconciler +/// recreates their container with the new mount set (best-effort — settles at their +/// next login/boot on failure). See `Skald::refresh_user_mounts`, its subscriber. +/// +/// Emitted **after** the membership row is committed, since the reconciler reads +/// the current rows. +fn remount(skald: &Skald, user_id: &str) { + skald.system_bus().send(SystemEvent::UserMountsChanged { user_id: user_id.to_string() }); } async fn detail(skald: &Skald, project: Project, caller: &str, can_write: bool) -> Result { @@ -189,8 +191,9 @@ pub async fn list( } /// POST /api/projects — create a project owned by the caller (a private project = one -/// member). The folder is created and the caller's container remounted so the agent -/// can reach it immediately. +/// member). The folder is created synchronously (the explorer reads it host-side, so +/// it is browsable at once); the container remount that makes it reachable from +/// `execute_cmd` is announced on the bus and lands shortly after. pub async fn create( State(skald): State>, Extension(auth): Extension, @@ -217,7 +220,7 @@ pub async fn create( // Create the bind-mount source, then remount so the container sees it. std::fs::create_dir_all(project_dir(&auth.user_id, &slug)?) .map_err(|e| ApiError::bad_request(format!("failed to create project directory: {e}")))?; - remount(&skald, &auth.user_id).await; + remount(&skald, &auth.user_id); let d = detail(&skald, project, &auth.user_id, true).await?; Ok((StatusCode::CREATED, Json(d))) @@ -278,7 +281,7 @@ pub async fn delete( // Best-effort: drop the folder; the DB row is already gone. let _ = std::fs::remove_dir_all(project_dir(&project.owner_user_id, &project.slug)?); for m in members { - remount(&skald, &m.user_id).await; + remount(&skald, &m.user_id); } Ok(StatusCode::NO_CONTENT) } @@ -300,7 +303,7 @@ pub async fn add_member( } project_members::add_member(skald.db(), p.id, &body.user_id, body.can_write).await?; projects::touch(skald.db(), p.id).await?; - remount(&skald, &body.user_id).await; + remount(&skald, &body.user_id); let members = project_members::members(skald.db(), p.id).await?; Ok(Json(members.into_iter().map(Into::into).collect())) @@ -320,7 +323,7 @@ pub async fn remove_member( } project_members::remove_member(skald.db(), mp.id, &mp.user_id).await?; projects::touch(skald.db(), mp.id).await?; - remount(&skald, &mp.user_id).await; + remount(&skald, &mp.user_id); let members = project_members::members(skald.db(), mp.id).await?; Ok(Json(members.into_iter().map(Into::into).collect())) diff --git a/src/frontend/api/setup.rs b/src/frontend/api/setup.rs index 49c641a..e5438b7 100644 --- a/src/frontend/api/setup.rs +++ b/src/frontend/api/setup.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use axum::{Json, extract::State}; +use core_api::system_bus::SystemEvent; use serde::{Deserialize, Serialize}; use skald_core::skald::Skald; @@ -110,5 +111,12 @@ pub async fn create_user( ) .await?; + // The web wizard runs against a *live* server, where boot reconciliation has + // already happened — so without this the first admin had no container until the + // next restart. One announcement, and the reconciler provisions it like any + // other user (blueprint §6). The console shell needs no equivalent: it runs + // before the server, and `reconcile_all()` picks the admin up at boot. + skald.system_bus().send(SystemEvent::UserCreated { user_id: id.clone() }); + Ok(Json(CreateUserResult { user_id: id })) } diff --git a/src/frontend/api/shared_folders.rs b/src/frontend/api/shared_folders.rs index d290173..b284323 100644 --- a/src/frontend/api/shared_folders.rs +++ b/src/frontend/api/shared_folders.rs @@ -9,13 +9,15 @@ //! The folder rows + membership live in the registry (`system.db`); the physical //! directory `{WD}/shared/{name}` is created here so the bind-mount has a source //! and the admin can drop files in immediately. Propagating a membership change -//! into a *running* container (recreate) and into a logged-in user's fs view + -//! system prompt is the follow-on step — see blueprint §6. +//! into a *running* container and into a logged-in user's fs view is **not** done +//! here: this module only announces `UserMountsChanged` on the system bus, and the +//! lifecycle reconciler (`skald::wiring`) reacts (blueprint §6). use std::sync::Arc; use axum::extract::{Extension, Path, State}; use axum::Json; +use core_api::system_bus::SystemEvent; use serde::{Deserialize, Serialize}; use skald_core::db::{role_capabilities, shared_folders, users}; @@ -52,16 +54,16 @@ fn create_shared_dir(name: &str) -> Result<(), ApiError> { Ok(()) } -/// Applies a membership change to a user's live environment — recreate their -/// container with the new mounts and, if they are logged in, refresh their fs view -/// + per-user MCP in place (blueprint §6 remount). Best-effort: the membership row -/// is already committed, so a Docker hiccup is logged, not surfaced — it settles at +/// Announces that a user's mount topology changed. The lifecycle reconciler +/// recreates their container with the new mounts and, if they are logged in, +/// refreshes their fs view + per-user MCP in place (blueprint §6 remount). +/// +/// Emitted **after** the membership row is committed, since the reconciler reads +/// the current rows. Fire-and-forget by contract: a Docker hiccup is the +/// reconciler's to log, never this endpoint's to surface — the state settles at /// the user's next login/boot. -async fn remount(skald: &Skald, user_id: &str) { - if let Err(e) = skald.refresh_user_mounts(user_id).await { - tracing::warn!(user = %user_id, error = %e, - "shared-folder remount failed (settles at next login/boot)"); - } +fn remount(skald: &Skald, user_id: &str) { + skald.system_bus().send(SystemEvent::UserMountsChanged { user_id: user_id.to_string() }); } // ── response / request types ────────────────────────────────────────────────── @@ -183,7 +185,7 @@ pub async fn delete( // The on-disk directory is deliberately left in place: unsharing a folder must // not destroy the files inside it. The admin removes them by hand if intended. for m in &members { - remount(&skald, &m.user_id).await; + remount(&skald, &m.user_id); } Ok(Json(serde_json::json!({ "ok": true }))) } @@ -214,7 +216,7 @@ pub async fn add_member( } shared_folders::add_member(skald.db(), id, &body.user_id, body.can_write).await?; // Mount the folder into (or re-grant RO/RW inside) the member's environment. - remount(&skald, &body.user_id).await; + remount(&skald, &body.user_id); Ok(Json(serde_json::json!({ "ok": true }))) } @@ -228,6 +230,6 @@ pub async fn remove_member( require_manage(&skald, &auth.user_id).await?; shared_folders::remove_member(skald.db(), id, &user_id).await?; // Unmount the folder from the (former) member's environment. - remount(&skald, &user_id).await; + remount(&skald, &user_id); Ok(Json(serde_json::json!({ "ok": true }))) } diff --git a/src/frontend/api/users_mgmt.rs b/src/frontend/api/users_mgmt.rs index 7952839..2f0282d 100644 --- a/src/frontend/api/users_mgmt.rs +++ b/src/frontend/api/users_mgmt.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use axum::{Json, extract::{Path, State}}; +use core_api::system_bus::SystemEvent; use serde::{Deserialize, Serialize}; use skald_core::db::users::UserSummary; @@ -102,11 +103,10 @@ pub async fn create( .await?; } - // Provision the user's container now (blueprint §6). Best-effort: a failure here - // is not fatal to user creation — boot reconciliation will retry. - if let Err(e) = skald.container().ensure(&id).await { - tracing::warn!(user = %id, error = %e, "failed to provision user container (will retry at next boot)"); - } + // Announce the new user. Provisioning their container (blueprint §6) is the + // lifecycle reconciler's job, not this endpoint's — and creation does not wait + // on Docker, which was already best-effort here (boot reconciliation retries). + skald.system_bus().send(SystemEvent::UserCreated { user_id: id.clone() }); Ok(Json(CreatedUser { id })) } @@ -174,10 +174,8 @@ pub async fn delete( Path(id): Path, ) -> Result, ApiError> { skald.users().delete_user(&id).await?; - // Tear down the user's container (best-effort; a missing one is fine). - if let Err(e) = skald.container().remove(&id).await { - tracing::warn!(user = %id, error = %e, "failed to remove user container"); - } + // The row is gone; the reconciler tears the container down (a missing one is fine). + skald.system_bus().send(SystemEvent::UserDeleted { user_id: id }); Ok(Json(serde_json::json!({ "ok": true }))) }