feat: signpost the virtual memory roots inside the container, instead of leaving them absent
Nightly Build / build (push) Successful in 7m38s
Nightly Build / build (push) Successful in 7m38s
`user-memory/` and `shared-memory/` live in SQLite, so nothing of them existed on disk — and that nothing was worse than it looks. `cat user-memory/x.md` returned a bare ENOENT, which a model reads as "the note is missing" rather than "wrong door"; and `mkdir -p user-memory && echo … > user-memory/x.md` *succeeded*, writing a real file into the home that no reader ever visits (every reader goes to `memory_docs`) and that the next `ls` then confirms as if it had worked. Each root now gets a read-only bind mount holding a README that names the tools to use instead. Read-only as a mount rather than as a mode: the container user has passwordless sudo, so a chmod would be a suggestion, while `:ro` holds — remounting needs CAP_SYS_ADMIN. Verified in a scratch container: write, sudo write, sudo chmod, sudo mount -o remount,rw and sudo rm all fail. And a README rather than an empty directory, because "Permission denied" is an error, not an instruction — models answer it by reaching for sudo; the README puts the correction in the same directory the failing command just named. The mounts are deliberately not part of `UserFs`: they back no agent path and the host-side fs-tools must never resolve into them. They reach existing containers as a fourth self-heal axis in `reusable()`, not as an IMAGE_TAG bump — the image is unchanged, and a bump would make every installation rebuild it to fix a mount. The matching half is in `classify_memory`, which now strips the home spellings (`./`, `~/`, `/root/`) before matching the root. Without it `~/user-memory/x.md` missed the match and fell through to the disk router — becoming exactly the invisible physical file the signpost exists to prevent. `agents/common/memory.md` says the rule outright: the stores are reachable only through the file tools and `memory_search`, never through `execute_cmd`.
This commit is contained in:
@@ -9,6 +9,9 @@ blueprint/
|
||||
/database/
|
||||
# Per-user container home dirs ({WD}/homes/{userid}) — instance data, not source
|
||||
/homes/
|
||||
# Read-only memory signposts mounted into every container; regenerated at boot
|
||||
# from the consts in crates/skald-core/src/container/mod.rs
|
||||
/.memory-signpost/
|
||||
# SQLite WAL-mode sidecar files (journal_mode=WAL)
|
||||
*.db-wal
|
||||
*.db-shm
|
||||
|
||||
@@ -166,6 +166,8 @@ The agent sees **one namespace**, routed on the first path component. The choke
|
||||
|
||||
Two views, **one storage**: the fs-tools run **host-side** in the Skald process on `{WD}/homes/{userid}` + `{WD}/shared/{X}`; `execute_cmd` runs **inside the container** (`docker exec -w <container-path> skald-{userid} sh -c …`, via `ExecuteCmd::run_with`) on the same paths bind-mounted (`homes/{userid}`→`/root`, `shared/{X}`→`/root/shared/{X}`, read-only when `can_write=0`). A file written in the container appears to the host fs-tools and vice versa.
|
||||
|
||||
**The memory roots are signposted inside the container, not merely absent.** `user-memory/`/`shared-memory/` are virtual, so nothing of them existed on disk — and the nothing was worse than it sounds: `cat user-memory/x.md` returned a bare ENOENT (which reads as *the note is missing*, not *wrong door*), while `mkdir -p user-memory && echo … > user-memory/x.md` **succeeded**, writing a real file into the home that no reader ever visits and that the next `ls` then confirms as if it had worked. Each root is therefore a **read-only bind mount** (`{WD}/.memory-signpost/{root}` → `{container_home}/{root}:ro`, gitignored, rewritten from consts on every `ensure`) holding a README that names the tools. Read-only *as a mount*, not as a mode: the container user has passwordless `sudo`, so a `chmod` would be a suggestion, whereas `:ro` holds — remounting needs `CAP_SYS_ADMIN` (verified: write, `sudo` write, `sudo chmod`, `sudo mount -o remount,rw` and `sudo rm` all fail). A README rather than an empty dir because `Permission denied` is an error, not an instruction — models answer it by reaching for `sudo`; the README puts the correction in the directory the failing command just named. These mounts are deliberately **not** in `UserFs`: they back no agent path and the host-side fs-tools must never resolve into them. They are the **fourth self-heal axis** in `reusable()` (`signposts_mounted`) rather than an `IMAGE_TAG` bump, since the image is unchanged and a bump would make every box rebuild it to fix a mount. The matching half is in `classify_memory`, which now strips the home spellings (`./`, `~/`, `/root/`) before matching the root — without it `~/user-memory/x.md` missed the match, fell through to the disk router, and became exactly the invisible physical file the signpost exists to prevent.
|
||||
|
||||
**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 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 -<pgid>`); 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.
|
||||
|
||||
@@ -7,6 +7,12 @@ You have two persistent note stores, kept as Markdown and searchable. **Sessions
|
||||
|
||||
When unsure where something belongs, prefer `user-memory/`.
|
||||
|
||||
## They are not folders on disk
|
||||
|
||||
Both stores are **virtual**: they live in the database, not in the filesystem. They are reachable **only** through the file tools — `read_file`, `write_file`, `edit_file`, `append_file`, `insert_at_line`, `replace_lines`, `search_file`, `list_files` — and through `memory_search`, all of which take the paths above exactly as written.
|
||||
|
||||
Never go through `execute_cmd`. A shell command cannot read a note (`cat user-memory/x.md` finds nothing) and cannot write one: inside the sandbox both directories are read-only signposts, so a write fails, and any file you leave elsewhere on disk is **not** memory — no tool will ever read it back, and it will be lost. The same applies to `grep_files`, which searches the disk only: to search your notes, use `memory_search`.
|
||||
|
||||
## The indexes
|
||||
|
||||
Each store has an `index.md` — one line per note with a brief summary — and **both are injected into your context automatically** at the start of each session (look for them below):
|
||||
|
||||
@@ -5,7 +5,9 @@
|
||||
//! user is created and started at application boot; `execute_cmd` and — later —
|
||||
//! the user's stateful MCP servers run inside it, against the user's bind-mounted
|
||||
//! home (`{WD}/homes/{userid}` → `/root`) plus the shared folders they belong to,
|
||||
//! plus the read-only `{WD}/docs` bundle mounted at `/root/docs` for every user.
|
||||
//! plus the read-only `{WD}/docs` bundle mounted at `/root/docs` for every user and
|
||||
//! the read-only memory **signposts** at `/root/{user,shared}-memory` (see
|
||||
//! [`signpost_mounts`]).
|
||||
//!
|
||||
//! Docker is a **hard requirement**: [`ContainerManager::check_docker`] fails
|
||||
//! construction if the daemon is unreachable, and the shell exits at boot.
|
||||
@@ -16,7 +18,7 @@
|
||||
//! a container can be recreated from the image at any time; boot reconciliation
|
||||
//! relies on that.
|
||||
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Stdio;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -27,6 +29,7 @@ use sqlx::SqlitePool;
|
||||
use core_api::user_fs::{ProjectMount, SharedMount, UserFs};
|
||||
|
||||
use crate::db;
|
||||
use crate::tools::fs as fs_tools;
|
||||
|
||||
/// Our runtime image tag. Built once from the embedded [`Dockerfile`]. The version
|
||||
/// suffix is the image cache-buster: [`ContainerManager::ensure_image`] rebuilds only
|
||||
@@ -50,6 +53,9 @@ pub const PROJECTS_DIR: &str = "projects";
|
||||
/// Subdirectory of the working directory holding the docs bundle, mounted
|
||||
/// read-only into every user's container at `{container_home}/docs`.
|
||||
pub const DOCS_DIR: &str = "docs";
|
||||
/// Subdirectory of the working directory holding the memory **signposts** — see
|
||||
/// [`signpost_mounts`]. Dot-prefixed: it is internal plumbing, not a user folder.
|
||||
pub const SIGNPOST_DIR: &str = ".memory-signpost";
|
||||
/// Home mount point inside the container.
|
||||
pub const CONTAINER_HOME: &str = "/root";
|
||||
/// Grace window `docker stop` gives in-container processes (SIGTERM → SIGKILL)
|
||||
@@ -126,6 +132,111 @@ pub async fn build_user_fs(system: &SqlitePool, user_id: &str) -> Result<UserFs>
|
||||
Ok(UserFs::new(user_id, home_host, container_name(user_id), container_home, shared, projects, docs_host))
|
||||
}
|
||||
|
||||
// ── Memory signposts ──────────────────────────────────────────────────────────
|
||||
//
|
||||
// `user-memory/` and `shared-memory/` are **virtual**: the fs-tools classify those
|
||||
// prefixes and route them to SQLite (`memory_docs`), so nothing of them exists on
|
||||
// disk. Inside the container that used to mean bash saw nothing at all — and the
|
||||
// nothing was worse than it sounds. `cat user-memory/x.md` returned a bare ENOENT,
|
||||
// which tells a model that the note is missing rather than that it used the wrong
|
||||
// door; and `mkdir -p user-memory && echo … > user-memory/x.md` *succeeded*,
|
||||
// writing a real file into the home that no reader ever visits (every reader —
|
||||
// `read_file`, `list_files`, `memory_search`, the lints, the viewer — goes to
|
||||
// `memory_docs`), which the next `ls` then confirms as if it had worked.
|
||||
//
|
||||
// So each root gets a **read-only bind mount** carrying a README that names the
|
||||
// tools to use instead. Two deliberate choices:
|
||||
//
|
||||
// - *Read-only as a mount, not as a mode.* The container user holds passwordless
|
||||
// `sudo`, so a `chmod 0555` would be a suggestion; a `:ro` bind mount holds,
|
||||
// because remounting it needs `CAP_SYS_ADMIN` and the container has none. Writes
|
||||
// fail with EROFS.
|
||||
// - *A README rather than an empty directory.* `Permission denied` is an error, not
|
||||
// an instruction — models answer it by reaching for `sudo`. The README puts the
|
||||
// correction in the same directory the failing command just named, which is the
|
||||
// one feedback channel that lands in the turn where the mistake happened.
|
||||
//
|
||||
// These mounts are **not** part of [`UserFs`]: they back no agent path (the agent
|
||||
// path `user-memory/…` is the note store) and the host-side fs-tools must never
|
||||
// resolve into them. They exist only inside the sandbox, which is the only place
|
||||
// the confusion happens.
|
||||
|
||||
const SIGNPOST_README: &str = "README.md";
|
||||
|
||||
/// The signpost text for `user-memory/`. Addressed to the agent, in the vocabulary
|
||||
/// its tools use.
|
||||
const USER_MEMORY_SIGNPOST: &str = "\
|
||||
# This is not a folder
|
||||
|
||||
`user-memory/` is a **virtual note store**, kept in the database, not on disk. This
|
||||
directory is a signpost and is read-only: shell commands cannot read or write your
|
||||
memory, and anything you manage to write near here is lost.
|
||||
|
||||
Use the tools instead — they take the same paths:
|
||||
|
||||
read_file path=\"user-memory/notes/x.md\"
|
||||
write_file path=\"user-memory/notes/x.md\" content=\"…\"
|
||||
edit_file path=\"user-memory/notes/x.md\" …
|
||||
list_files path=\"user-memory/\"
|
||||
memory_search query=\"<keywords>\"
|
||||
|
||||
`grep_files` does not reach the store either — use `memory_search`.
|
||||
";
|
||||
|
||||
/// The signpost text for `shared-memory/`. Same rule; the extra line is the one
|
||||
/// thing that differs about the shared store.
|
||||
const SHARED_MEMORY_SIGNPOST: &str = "\
|
||||
# This is not a folder
|
||||
|
||||
`shared-memory/` is a **virtual note store** shared with the whole group, kept in the
|
||||
database, not on disk. This directory is a signpost and is read-only: shell commands
|
||||
cannot read or write it, and anything you manage to write near here is lost.
|
||||
|
||||
Use the tools instead — they take the same paths:
|
||||
|
||||
read_file path=\"shared-memory/x.md\"
|
||||
write_file path=\"shared-memory/x.md\" content=\"…\"
|
||||
edit_file path=\"shared-memory/x.md\" …
|
||||
list_files path=\"shared-memory/\"
|
||||
memory_search query=\"<keywords>\"
|
||||
|
||||
Writing here asks the user to confirm first — that is expected, not an error.
|
||||
`grep_files` does not reach the store either — use `memory_search`.
|
||||
";
|
||||
|
||||
/// Where the two signposts live on the host and where they mount, read-only, in the
|
||||
/// container. One pair of host directories for the whole instance: the content is
|
||||
/// identical for every user, and the mount is a sign, not a workspace.
|
||||
fn signpost_mounts(wd: &Path, container_home: &Path) -> [(PathBuf, PathBuf); 2] {
|
||||
let root = wd.join(SIGNPOST_DIR);
|
||||
[
|
||||
(
|
||||
root.join(fs_tools::USER_MEMORY_ROOT),
|
||||
container_home.join(fs_tools::USER_MEMORY_ROOT),
|
||||
),
|
||||
(
|
||||
root.join(fs_tools::SHARED_MEMORY_ROOT),
|
||||
container_home.join(fs_tools::SHARED_MEMORY_ROOT),
|
||||
),
|
||||
]
|
||||
}
|
||||
|
||||
/// Creates the signpost directories and (re)writes their READMEs. The write is
|
||||
/// unconditional so an edited text reaches existing installations at the next
|
||||
/// container `ensure`, with no migration step — it is a few hundred bytes.
|
||||
fn ensure_signposts(wd: &Path) -> Result<()> {
|
||||
for ((host, _), body) in signpost_mounts(wd, Path::new(CONTAINER_HOME))
|
||||
.iter()
|
||||
.zip([USER_MEMORY_SIGNPOST, SHARED_MEMORY_SIGNPOST])
|
||||
{
|
||||
std::fs::create_dir_all(host)
|
||||
.with_context(|| format!("failed to create signpost dir {}", host.display()))?;
|
||||
std::fs::write(host.join(SIGNPOST_README), body)
|
||||
.with_context(|| format!("failed to write signpost in {}", host.display()))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Owns the container lifecycle: the docker availability check, the runtime image,
|
||||
/// and per-user create/start/stop/remove. Cheap to clone (holds an `Arc` pool).
|
||||
#[derive(Clone)]
|
||||
@@ -202,6 +313,7 @@ impl ContainerManager {
|
||||
/// container is already running.
|
||||
pub async fn ensure(&self, user_id: &str) -> Result<()> {
|
||||
let fs = build_user_fs(&self.system, user_id).await?;
|
||||
let wd = std::env::current_dir().context("failed to read working directory")?;
|
||||
|
||||
// Host directories must exist before the mount, or Docker creates them
|
||||
// root-owned with surprising modes. Created by the host process, so they are
|
||||
@@ -210,6 +322,7 @@ impl ContainerManager {
|
||||
std::fs::create_dir_all(&host)
|
||||
.with_context(|| format!("failed to create host dir {}", host.display()))?;
|
||||
}
|
||||
ensure_signposts(&wd)?;
|
||||
|
||||
let name = &fs.container_name;
|
||||
let want_user = host_uid_gid().map(|(uid, gid)| format!("{uid}:{gid}"));
|
||||
@@ -262,6 +375,12 @@ impl ContainerManager {
|
||||
args.push("-v".into());
|
||||
args.push(spec);
|
||||
}
|
||||
// The virtual memory roots, read-only, nested inside the home mount (Docker
|
||||
// orders mounts by destination depth, as it already does for `shared/`).
|
||||
for (host, container) in signpost_mounts(&wd, &fs.container_home) {
|
||||
args.push("-v".into());
|
||||
args.push(format!("{}:{}:ro", host.display(), container.display()));
|
||||
}
|
||||
args.push(IMAGE_TAG.into());
|
||||
// Long-lived idle process; nothing runs until `docker exec` drives it.
|
||||
args.extend(["sleep".into(), "infinity".into()]);
|
||||
@@ -406,11 +525,32 @@ async fn image_matches(name: &str) -> bool {
|
||||
.unwrap_or(true)
|
||||
}
|
||||
|
||||
/// Whether a container carries the memory signpost mounts (see [`signpost_mounts`]).
|
||||
/// Mounts are fixed at `docker create` time, so a container predating them keeps the
|
||||
/// old, confusing view — bash silently writing into a `user-memory/` directory nobody
|
||||
/// reads — until it is recreated. This is the fourth self-heal axis, and it is worth
|
||||
/// its own check rather than an [`IMAGE_TAG`] bump: the image itself is unchanged, and
|
||||
/// a bump would make every installation rebuild it to fix a mount. Unreadable inspect
|
||||
/// ⇒ `true`, so a docker hiccup never churns a working container.
|
||||
async fn signposts_mounted(name: &str) -> bool {
|
||||
let Ok(out) = docker(&["inspect", "-f", "{{range .Mounts}}{{println .Destination}}{{end}}", name]).await
|
||||
else {
|
||||
return true;
|
||||
};
|
||||
let dests: Vec<&str> = out.lines().map(str::trim).collect();
|
||||
signpost_mounts(Path::new(""), Path::new(CONTAINER_HOME))
|
||||
.iter()
|
||||
.all(|(_, container)| dests.iter().any(|d| Path::new(d) == container))
|
||||
}
|
||||
|
||||
/// Whether an existing container can be reused as-is: right `--user` (§6 UID coherence),
|
||||
/// `--init` (fast, clean `docker stop`) **and** the current image. A mismatch on any of
|
||||
/// the three recreates it.
|
||||
/// `--init` (fast, clean `docker stop`), the current image **and** the memory signpost
|
||||
/// mounts. A mismatch on any of the four recreates it.
|
||||
async fn reusable(name: &str, want_user: &Option<String>) -> bool {
|
||||
user_matches(name, want_user).await && init_matches(name).await && image_matches(name).await
|
||||
user_matches(name, want_user).await
|
||||
&& init_matches(name).await
|
||||
&& image_matches(name).await
|
||||
&& signposts_mounted(name).await
|
||||
}
|
||||
|
||||
/// Gives the container's runtime `uid`/`gid` a passwd + shadow (+ group) entry, so
|
||||
|
||||
@@ -68,17 +68,37 @@ pub struct MemRef {
|
||||
pub rel: String,
|
||||
}
|
||||
|
||||
/// Strips the ways an agent spells "in my home" — `./`, `~/`, the container-absolute
|
||||
/// `{CONTAINER_HOME}/` — so the memory roots are recognised whichever spelling the
|
||||
/// model reaches for.
|
||||
///
|
||||
/// Without this, `~/user-memory/x.md` misses the match below and falls through to the
|
||||
/// **disk** router, which resolves it against the caller's home: the note lands in a
|
||||
/// physical `user-memory/` directory that no tool ever reads back, since every reader
|
||||
/// (`read_file`, `list_files`, `memory_search`, the lints, the viewer) goes to
|
||||
/// `memory_docs`. Silent data loss, and the kind an agent then re-confirms by `ls`.
|
||||
fn strip_home_spelling(user_path: &str) -> &str {
|
||||
let p = user_path.trim_start_matches("./");
|
||||
if let Some(rest) = p.strip_prefix("~/") {
|
||||
return rest;
|
||||
}
|
||||
// `/root/user-memory/…`, but not `/rootless/…` — the separator is required.
|
||||
p.strip_prefix(crate::container::CONTAINER_HOME)
|
||||
.and_then(|rest| rest.strip_prefix('/'))
|
||||
.unwrap_or(p)
|
||||
}
|
||||
|
||||
/// Classifies a user-supplied path. Returns `Some` when it lands under one of the
|
||||
/// virtual memory roots — to be routed to SQLite — and `None` for an ordinary
|
||||
/// disk path.
|
||||
///
|
||||
/// The **first** component decides the store, taken raw *before* normalization, so
|
||||
/// a `..` in the tail can never drop the memory root and silently fall back to a
|
||||
/// disk path. The tail is then normalized (resolving `.`/`..`) and clamped at the
|
||||
/// store root, so a memory path stays within its store and an absolute path is
|
||||
/// always disk.
|
||||
/// The **first** component decides the store, taken raw *before* normalization (bar
|
||||
/// the home spelling, see [`strip_home_spelling`]), so a `..` in the tail can never
|
||||
/// drop the memory root and silently fall back to a disk path. The tail is then
|
||||
/// normalized (resolving `.`/`..`) and clamped at the store root, so a memory path
|
||||
/// stays within its store.
|
||||
pub fn classify_memory(user_path: &str) -> Option<MemRef> {
|
||||
let mut parts = user_path.trim_start_matches("./").splitn(2, ['/', '\\']);
|
||||
let mut parts = strip_home_spelling(user_path).splitn(2, ['/', '\\']);
|
||||
let scope = match parts.next()? {
|
||||
USER_MEMORY_ROOT => MemScope::User,
|
||||
SHARED_MEMORY_ROOT => MemScope::Shared,
|
||||
@@ -417,6 +437,26 @@ mod tests {
|
||||
assert!(classify_memory("user-memoryish/x").is_none());
|
||||
}
|
||||
|
||||
/// A memory path spelled as if it lived in the home must still reach the note
|
||||
/// store — otherwise it would be written to a *physical* `user-memory/` directory
|
||||
/// no reader ever looks at.
|
||||
#[test]
|
||||
fn classify_memory_accepts_home_spellings() {
|
||||
for p in ["~/user-memory/x.md", "/root/user-memory/x.md", "./user-memory/x.md"] {
|
||||
let m = classify_memory(p).unwrap_or_else(|| panic!("{p} must classify as memory"));
|
||||
assert!(matches!(m.scope, MemScope::User));
|
||||
assert_eq!(m.rel, "x.md", "{p}");
|
||||
}
|
||||
assert!(matches!(
|
||||
classify_memory("~/shared-memory/casa.md").unwrap().scope,
|
||||
MemScope::Shared
|
||||
));
|
||||
|
||||
// the container-home strip needs a real separator, and stops at the home
|
||||
assert!(classify_memory("/rootless/user-memory/x.md").is_none());
|
||||
assert!(classify_memory("/root/notes/user-memory/x.md").is_none());
|
||||
}
|
||||
|
||||
/// A throwaway owner-schema pool (as `Arc`, ready for a `ToolContext`), plus its
|
||||
/// dir for cleanup. `tag` + a counter keep parallel tests off the same file.
|
||||
async fn store(tag: &str) -> (Arc<SqlitePool>, PathBuf) {
|
||||
|
||||
Reference in New Issue
Block a user