fix: give the notification home a place to live in the owner's database
/sethome answered "no such table: config" from every surface. ChatHub is
owner-bound, so its pool is a {userid}.db, and `config` is a registry
table that only exists in system.db — the write had no table to land in.
The visible half was the lesser one. The notification consumer resolves
the home source before it delivers anything, and on an error it dropped
the batch: every `notify` from a background agent and every cron-job
completion has been discarded, silently, for as long as the hub has been
per-user. That error path now degrades to the default home instead — a
batch that got that far is data nobody can recreate, and the destination
is the one thing there with a sane fallback.
Where the setting belongs was never in doubt: one member choosing
Telegram must not move anybody else's notifications, so it is owner
state and it goes in their own file. The new owner table is `user_config`
and it deliberately does not reuse the registry name. The two hold
different namespaces — instance settings the admin owns versus one
person's own preferences — and a table called `config` in both files
would have turned this exact mistake into a silent read of the other
scope, which is strictly worse than the loud failure that revealed it.
Additive, so no migration: open_user_pool re-applies the owner schema on
every unlock, and the table appears at each user's next login.
This commit is contained in:
@@ -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:
|
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_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.
|
**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.
|
||||||
|
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ use inbox::{ConversationInbox, QueuedMessage, build_unit, drain_leading_user};
|
|||||||
|
|
||||||
use crate::approval::ApprovalManager;
|
use crate::approval::ApprovalManager;
|
||||||
use crate::cron::TaskManager;
|
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::events::{GlobalEvent, ServerEvent};
|
||||||
use crate::notification::Notification;
|
use crate::notification::Notification;
|
||||||
use crate::session::handler::{
|
use crate::session::handler::{
|
||||||
@@ -434,15 +434,23 @@ impl ChatHub {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Set which source is the "home" for background agent notifications.
|
/// 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<()> {
|
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");
|
info!(source_id, "ChatHub: home source set");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns the current home source id, falling back to `web` if not configured.
|
/// Returns the current home source id, falling back to `web` if not configured.
|
||||||
pub async fn home_source(&self) -> anyhow::Result<String> {
|
pub async fn home_source(&self) -> anyhow::Result<String> {
|
||||||
Ok(config::get(&self.db, HOME_SOURCE_KEY)
|
Ok(user_config::get(&self.db, HOME_SOURCE_KEY)
|
||||||
.await?
|
.await?
|
||||||
.unwrap_or_else(|| DEFAULT_HOME_SOURCE.to_string()))
|
.unwrap_or_else(|| DEFAULT_HOME_SOURCE.to_string()))
|
||||||
}
|
}
|
||||||
@@ -978,9 +986,18 @@ impl ChatHub {
|
|||||||
None => break, // ChatHub dropped
|
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 {
|
let home = match hub.home_source().await {
|
||||||
Ok(h) => h,
|
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();
|
let count = notes.len();
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ pub mod system_agent_coverage;
|
|||||||
pub mod system_agent_runs;
|
pub mod system_agent_runs;
|
||||||
pub mod system_agent_state;
|
pub mod system_agent_state;
|
||||||
pub mod tool_permission_groups;
|
pub mod tool_permission_groups;
|
||||||
|
pub mod user_config;
|
||||||
pub mod users;
|
pub mod users;
|
||||||
|
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
@@ -1144,6 +1145,23 @@ pub async fn create_owner_tables(pool: &SqlitePool) -> Result<()> {
|
|||||||
.execute(pool)
|
.execute(pool)
|
||||||
.await?;
|
.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
|
// NOTE: `projects` + `project_members` are **registry** tables (see
|
||||||
// `create_registry_tables`) — shareable, not encrypted. The old owner-bucket
|
// `create_registry_tables`) — shareable, not encrypted. The old owner-bucket
|
||||||
// `projects`/`project_tickets` tables (single-user Skald leftover) were removed
|
// `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 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 sources (id, active_session_id) VALUES ('web', 1)").await.unwrap();
|
||||||
one("INSERT INTO secrets (key, value) VALUES ('k', 'v')").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();
|
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.
|
// 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();
|
one("INSERT INTO memory_docs (path, content) VALUES ('notes/x.md', 'hello world')").await.unwrap();
|
||||||
|
|||||||
@@ -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<Option<String>> {
|
||||||
|
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(())
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user