diff --git a/CLAUDE.md b/CLAUDE.md index 920a82c..ebbe8a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,7 +130,7 @@ Two rules keep the boundary real, and both are enforced by the compiler: The schema is split into two buckets (§5.1), and the split is the point: - **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `plugin_access` + `plugin_user_configs`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `oauth_providers`, `role_capabilities`, `shared_folders` + `shared_folder_members`, `projects` + `project_members`, `supervision`, `system_agent_coverage`. The MCP tables back the Connectors model (§7/§14/§15 — see its own section); `oauth_providers` (accessor `db/oauth_providers.rs`) holds one row per identity provider (Google…) — endpoints + `client_id`/`client_secret` + `redirect_uri`, admin-owned household secrets (§4/§15b), never a per-user token. The last two pairs are junction-backed membership: `shared_folder_members` (accessor `db/shared_folders.rs`) for the on-disk shared folders (§6), `project_members` (accessor `db/project_members.rs`) for projects (see the Projects section) — both let a member be read-only (`can_write`) and both drive the container mount topology + the fs routing. Their FKs are registry→registry (same file), which is allowed — unlike an owner→registry key. -- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `system_agent_runs`, `system_agent_state`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`), `reports`. `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. (`projects`/`project_tickets` were owner tables in the single-user past: projects are shareable now, so `projects` + `project_members` are registry tables and `project_tickets` is gone.) +- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `system_agent_runs`, `system_agent_state`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `user_config`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`), `reports`. `user_config` is the per-user twin of the registry `config` table and deliberately does **not** share its name: the two hold different namespaces (instance settings the admin owns vs. one member's own preferences, the notification home being the first), and a same-named table in both files would turn a wrong-pool call into a silent read of the other scope — instead of the "no such table: config" that revealed `/sethome` writing owner state through `db::config` against a `{userid}.db`, which also had the notification consumer dropping every batch it ever built. `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. (`projects`/`project_tickets` were owner tables in the single-user past: projects are shareable now, so `projects` + `project_members` are registry tables and `project_tickets` is gone.) **The schema is no longer greenfield** (see the production note at the top): a full recreate is not an option anymore. `db::ensure_column` — `ALTER TABLE … ADD COLUMN` swallowing the "duplicate column" error, a no-op on a fresh DB where the `CREATE TABLE` already carries it — is therefore not a convenience for dev boxes anymore but the **only** change shape that is currently safe, and additive-with-a-default is the shape to design towards. Used for the OAuth columns on `mcp_catalog` / `mcp_user_servers`. Anything destructive waits for real versioning. diff --git a/crates/skald-core/src/chat_hub/mod.rs b/crates/skald-core/src/chat_hub/mod.rs index e4cf308..883e337 100644 --- a/crates/skald-core/src/chat_hub/mod.rs +++ b/crates/skald-core/src/chat_hub/mod.rs @@ -15,7 +15,7 @@ use inbox::{ConversationInbox, QueuedMessage, build_unit, drain_leading_user}; use crate::approval::ApprovalManager; use crate::cron::TaskManager; -use crate::db::{chat_history, chat_llm_tools, chat_sessions, chat_sessions_stack, config, sources}; +use crate::db::{chat_history, chat_llm_tools, chat_sessions, chat_sessions_stack, sources, user_config}; use crate::events::{GlobalEvent, ServerEvent}; use crate::notification::Notification; use crate::session::handler::{ @@ -434,15 +434,23 @@ impl ChatHub { } /// Set which source is the "home" for background agent notifications. + /// + /// The hub is owner-bound, so `self.db` is that person's own database and the + /// home is theirs: one member choosing Telegram cannot move anybody else's + /// notifications. That is why the key lives in the owner table `user_config` + /// and not in the registry `config` one — which this used to write, against a + /// `{userid}.db` that has no such table, so `/sethome` only ever answered + /// "no such table: config" and every notification batch was dropped by the + /// consumer below. pub async fn set_home(&self, source_id: &str) -> anyhow::Result<()> { - config::set(&self.db, HOME_SOURCE_KEY, source_id).await?; + user_config::set(&self.db, HOME_SOURCE_KEY, source_id).await?; info!(source_id, "ChatHub: home source set"); Ok(()) } /// Returns the current home source id, falling back to `web` if not configured. pub async fn home_source(&self) -> anyhow::Result { - Ok(config::get(&self.db, HOME_SOURCE_KEY) + Ok(user_config::get(&self.db, HOME_SOURCE_KEY) .await? .unwrap_or_else(|| DEFAULT_HOME_SOURCE.to_string())) } @@ -978,9 +986,18 @@ impl ChatHub { None => break, // ChatHub dropped }; + // A batch that got this far is data nobody can recreate, and the + // destination is the one thing here with a sane default — so a failed + // read degrades to it instead of discarding the notifications (which + // is precisely what a missing `config` table did, silently, to every + // `notify` and every cron completion on the box). let home = match hub.home_source().await { Ok(h) => h, - Err(e) => { error!(error = %e, "notification consumer: home_source failed"); continue; } + Err(e) => { + error!(error = %e, fallback = DEFAULT_HOME_SOURCE, + "notification consumer: home_source failed"); + DEFAULT_HOME_SOURCE.to_string() + } }; let count = notes.len(); diff --git a/crates/skald-core/src/db/mod.rs b/crates/skald-core/src/db/mod.rs index 2cd32c3..111bcf0 100644 --- a/crates/skald-core/src/db/mod.rs +++ b/crates/skald-core/src/db/mod.rs @@ -36,6 +36,7 @@ pub mod system_agent_coverage; pub mod system_agent_runs; pub mod system_agent_state; pub mod tool_permission_groups; +pub mod user_config; pub mod users; use std::path::{Path, PathBuf}; @@ -1144,6 +1145,23 @@ pub async fn create_owner_tables(pool: &SqlitePool) -> Result<()> { .execute(pool) .await?; + // One owner's own preferences — the per-user twin of the registry `config` + // table, deliberately **not** sharing its name. The two hold different + // namespaces (`ui_locale` and `compaction_model` are the admin's, the home + // source is the member's), and a same-named table in both files would turn + // every wrong-pool call into a silent read of the other scope instead of the + // loud "no such table" that caught `/sethome` writing a per-user setting + // through `db::config` against a `{userid}.db`. + sqlx::query( + "CREATE TABLE IF NOT EXISTS user_config ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL, + updated_at TEXT NOT NULL DEFAULT (datetime('now')) + )", + ) + .execute(pool) + .await?; + // NOTE: `projects` + `project_members` are **registry** tables (see // `create_registry_tables`) — shareable, not encrypted. The old owner-bucket // `projects`/`project_tickets` tables (single-user Skald leftover) were removed @@ -1337,6 +1355,7 @@ mod tests { one("INSERT INTO mcp_events (source, method, payload) VALUES ('s', 'm', '{}')").await.unwrap(); one("INSERT INTO sources (id, active_session_id) VALUES ('web', 1)").await.unwrap(); one("INSERT INTO secrets (key, value) VALUES ('k', 'v')").await.unwrap(); + one("INSERT INTO user_config (key, value) VALUES ('source_home', 'telegram')").await.unwrap(); one("INSERT INTO llm_request_payloads (request_id, request_json) VALUES ('r1', '{}')").await.unwrap(); // Fires the AFTER INSERT trigger into the external-content FTS5 table. one("INSERT INTO memory_docs (path, content) VALUES ('notes/x.md', 'hello world')").await.unwrap(); diff --git a/crates/skald-core/src/db/user_config.rs b/crates/skald-core/src/db/user_config.rs new file mode 100644 index 0000000..aa24738 --- /dev/null +++ b/crates/skald-core/src/db/user_config.rs @@ -0,0 +1,46 @@ +//! One owner's own key/value preferences, in their own database. +//! +//! The per-user twin of [`super::config`]: same shape, different file and a +//! different name on purpose (see the table comment in +//! [`super::create_owner_tables`]). Anything scoped to a person — the surface +//! their notifications go to, say — belongs here; instance-wide settings the +//! admin owns stay in the registry `config` table. + +use sqlx::SqlitePool; + +/// Get a value by key from this owner's database. +pub async fn get(pool: &SqlitePool, key: &str) -> anyhow::Result> { + let row = sqlx::query_as::<_, (String,)>( + "SELECT value FROM user_config WHERE key = ?", + ) + .bind(key) + .fetch_optional(pool) + .await?; + + Ok(row.map(|(v,)| v)) +} + +/// Upsert a key/value pair in this owner's database. +pub async fn set(pool: &SqlitePool, key: &str, value: &str) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO user_config (key, value, updated_at) + VALUES (?, ?, datetime('now')) + ON CONFLICT(key) DO UPDATE SET + value = excluded.value, + updated_at = excluded.updated_at", + ) + .bind(key) + .bind(value) + .execute(pool) + .await?; + Ok(()) +} + +/// Delete an entry. +pub async fn delete(pool: &SqlitePool, key: &str) -> anyhow::Result<()> { + sqlx::query("DELETE FROM user_config WHERE key = ?") + .bind(key) + .execute(pool) + .await?; + Ok(()) +}