feat(skills): rebuild the skill system for the multi-user model
Nightly Build / build (push) Successful in 8m6s
Nightly Build / build (push) Successful in 8m6s
Per blueprint/skill-project.md: the old single-namespace, hand-maintained
index is gone, replaced by a read-only, two-scope tree whose index is a
runtime function of its content.
- skills/ index generated at runtime (crates/skald-core/src/skills/:
inventory, install, validate, watch), injected through the new
<!-- SKILLS_LIST --> placeholder in AGENT.md (agents/common/skills.md);
meta.json inject_skills flag removed. 11 chat/task agents carry the
include, the 4 system agents do not.
- Two trees, both read-only in both directions: skills/shared/{id} (the
group's) and skills/{username}/{id} (one member's own, on the stable
userid). The root is closed too: UserFs::SkillMounts + RouteError (alias
probe, plain-denied paths, no home fallback) and a per-user
.skills-root/{userid} container mount with the two scope mounts nested
inside, plus the fifth self-heal axis (skills_mounted).
- Agent verbs: skill_register/skill_delete (Config group, global scope
behind the new skill.manage capability), fetch_repo for public repos,
list_items(type="skills"); reads are plain read_file on the printed
path. Seeded @fs_read skills/* allow.
- Freshness: a digest-gated watcher on the two trees emits
SystemEvent::SkillsChanged, whose subscriber rebuilds the frozen prompt
prefix via Skald::invalidate_prompt_prefix; in-process writers invalidate
directly.
- The build ships no skills: the three bundled skills (ics2json,
mcp-builder, skill-creator) and skills/index.md are removed, skills/ is
instance data (gitignored, not packaged, no longer pruned by update.sh).
- Docs: skills.md, agents.md, shared-folders.md added; docs/index.md and
agents/README.md updated.
This commit is contained in:
@@ -118,6 +118,7 @@ Two rules keep the boundary real, and both are enforced by the compiler:
|
||||
| `crates/skald-core/src/transcribe/` | Transcription providers |
|
||||
| `crates/skald-core/src/image_generate/` | Image generation providers |
|
||||
| `crates/skald-core/src/memory/` | Agent memory tools |
|
||||
| `crates/skald-core/src/skills/` | The skills index: pure functions over the two read-only trees (enumerate → parse frontmatter → render → digest). No state, no watcher — see the skills paragraphs in Filesystem & containers |
|
||||
| `src/frontend/mod.rs` | `WebFrontend`: wires router_factory, starts plugins, runs Axum |
|
||||
| `src/frontend/server.rs` | Axum router, static file serving |
|
||||
| `src/frontend/api/` | HTTP + WebSocket handlers — `State<Arc<Skald>>` |
|
||||
@@ -144,7 +145,7 @@ The schema is split into two buckets (§5.1), and the split is the point:
|
||||
|
||||
**Memory injection into the prompt**: `AgentSystemContext::load_inject_memory` (`loop_adapters/system.rs`) routes each `meta.inject_memory` entry — `user-memory/…` → owner pool, `shared-memory/…` → the shared (`system.db`) pool, both via `memory_docs::get`; anything else (`data/…`, `$WD/…`) is a disk read. The shared pool is threaded `ChatSessionManager` → `UserLoopRuntime` → `AgentSystemContext`. `assistant` and `project-coordinator` inject `user-memory/index.md` + `shared-memory/index.md`.
|
||||
|
||||
**Prompt substitutions**: an `AGENT.md` may carry `<!-- KEY -->` placeholders; `agents::resolve_includes` turns each into a `__KEY__` sentinel, replaced at request time. Two are resolved by the system-context source itself (`loop_adapters/system.rs`) from the session owner (`user_id`) + registry (`shared_pool`), so every source (WS, mobile, cron, sub-agents) gets them with no caller plumbing: `__SHARED_FOLDERS__` (the user's shared-folders table) and `__USER_PROFILE__` (the owner's directory profile: `Name`, `Date of birth` with age computed at build time, `Sex`, `Preferred language`, admin `Notes` — unset values render as explicit `unknown` / `not specified`, the `Notes` line is omitted when empty). Any other key comes from the per-call `SendMessageOptions::system_substitutions` map.
|
||||
**Prompt substitutions**: an `AGENT.md` may carry `<!-- KEY -->` placeholders; `agents::resolve_includes` turns each into a `__KEY__` sentinel, replaced at request time. Several are resolved by the system-context source itself (`loop_adapters/system.rs`) from the session owner (`user_id`) + registry (`shared_pool`) + their `UserFs`, so every source (WS, mobile, cron, sub-agents) gets them with no caller plumbing: `__SKILLS_LIST__` (the generated skills index — see Filesystem & containers), `__SHARED_FOLDERS__` (the user's shared-folders table) and `__USER_PROFILE__` (the owner's directory profile: `Name`, `Date of birth` with age computed at build time, `Sex`, `Preferred language`, admin `Notes` — unset values render as explicit `unknown` / `not specified`, the `Notes` line is omitted when empty). Any other key comes from the per-call `SendMessageOptions::system_substitutions` map.
|
||||
|
||||
`system.db` still gets **both** bucket functions — but no longer because the migration is unstarted. It gets the owner schema because it *is* the owner of **shared** memory (`memory_docs`) plus, for now, the globally-scoped `secrets` (`SecretsStore` is built on the system pool and shared by reference into every `UserContext`; the global runtime's *config* now lives in the registry table `mcp_global_servers`, and per-user connector config in each user's owner `mcp_user_servers`). The global runtime no longer writes `mcp_events` there: notification persistence is an explicit `McpManager::new` argument (`EventLog::{Persist,Discard}`), `Discard` for the ownerless global runtime and `Persist` for each per-user one, because an event belongs to whoever it happened to and its only reader (event triage) is per-user. Every *other* owner table is created there but never written to anymore — the global owner-bound managers that would write them (chat/jobs/etc.) are inert (see "Current state"). Fully dropping `create_owner_tables` from `system.db` is blocked on the §4 scope decision for secrets, not on call-site migration.
|
||||
|
||||
@@ -162,6 +163,8 @@ The agent sees **one namespace**, routed on the first path component. The choke
|
||||
| `shared-memory/…` | SQLite `system.db` | `classify_memory` → `memory_docs` |
|
||||
| `shared/{X}/…` | host `{WD}/shared/{X}` (if a member) | `UserFs::host_base_and_tail` |
|
||||
| `projects/{O}/{S}/…` | host `{WD}/projects/{owner_userid}/{S}` (if a member) | `UserFs::host_base_and_tail` |
|
||||
| `skills/shared/{id}/…` | host `{WD}/skills/{id}` — **read-only** | `UserFs::host_base_and_tail` |
|
||||
| `skills/{username}/{id}/…` | host `{WD}/skills-users/{userid}/{id}` — **read-only** | `UserFs::host_base_and_tail` |
|
||||
| `~/…`, relative | host `{WD}/homes/{userid}` | `UserFs::host_base_and_tail` |
|
||||
| any other absolute path (`/tmp/…`, `/etc/…`) | the **container's own** filesystem | `resolve_target` → `container::exec_fs` |
|
||||
|
||||
@@ -173,6 +176,10 @@ Two views, **one storage**: for the mounted subtree the fs-tools run **host-side
|
||||
|
||||
**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.
|
||||
|
||||
**Skills are a read-only tree with two scopes, and the space *between* them is closed too.** `skills/shared/{id}` is the group's, `skills/{username}/{id}` is one member's own (`core-api`'s `SkillMounts`; agent path on the username like `projects/`, host path on the stable userid). Everything under `skills/` is read-only in **both** directions — `:ro` bind mounts and `can_write_to → false` — because these hold installed artefacts, not working files: a skill body is *read as instruction* by whoever it is visible to, so writing one is a decision that must pass a gate, not a file write — the one door is `skill_register` (with `skill_delete` and `list_items(type="skills")`), called from the chat and gated `require`; a public repo is fetched with `fetch_repo` and then registered. Two traps, both closed together and neither covering the other's half. **Host-side**, the `skills` arm of `can_write_to`/`host_base_and_tail` spans the **whole root**, not the two known scopes: the fallthrough answers `true`/home, so an invented scope segment (`skills/pippo/SKILL.md` — the *likely* guess, not the lucky one) would land in a physical directory under the home that no indexer ever reads. That is the memory-signpost failure exactly. **In-container**, the defect is structural rather than name-dependent: the scope mounts nest inside `container_home`, so `/root/skills` would be a real directory inside the *writable* home mount and `mkdir -p ~/skills/pippo` would succeed. Hence a third mount: `{WD}/.skills-root/{userid}` → `{container_home}/skills:ro`, holding the signpost README plus the two scope mountpoints. That root is **per-user and materialized whole** (`container::ensure_skills_root`) because Docker refuses to create a mountpoint inside a `:ro` mount — `shared/` and `{username}/` must already exist in the root's own source, and one of those names is the member's — which is also why the three host paths are one `SkillMounts` field rather than three `Option<PathBuf>`. `mounts()` emits them root-first; `skills_mounted` is the **fifth self-heal axis**, for the signposts' reason. A stale scope dir left by a rename is pruned at each `ensure`. The bare-id alias `skills/{id}` (the shortest spelling, so the one a model writes unprompted) resolves in `resolve_skill_alias` — **only** when the id is unique across the two trees, failing loudly with both full paths otherwise, since a personal skill silently shadowing a group one is a divergence nobody chose. `UserFs` stays pure: it returns `RouteError::SkillAlias` and skald-core does the probe.
|
||||
|
||||
**The skills index is generated, and the sentinel is the knob.** What reaches the model is not a file anyone maintains but a **function of the two trees** (`crates/skald-core/src/skills/`, pure functions in the shape of `LlmCommandManager`): each skill's `SKILL.md` **path** plus its frontmatter `description`, truncated to 200 chars, under an imperative header ("you MUST read its SKILL.md") — the countermeasure to the real failure mode, which is the model *under*-triggering. Printing the full path rather than an id plus a composition rule is what makes a read tool unnecessary: `read_file` on the printed path is one call, and there is no step left for the model to get wrong. Injection is the placeholder `<!-- SKILLS_LIST -->` (normally `<!-- INCLUDE: common/skills.md -->`, a fragment that holds **only** the sentinel), substituted in `AgentSystemContext::build_base` beside `__MCP_LIST__`; `resolve_includes` needs no branch, its generic `<!-- KEY -->` → `__KEY__` arm already covers it. There is **no `meta.json` flag** — the sentinel *is* the switch, so the four `type: system` agents opt out by not including the fragment (an imperative "read it with read_file" is exactly wrong in an unattended turn, and some of those run with `allow_tools: false`). All eleven `chat`/`task` agents carry the include, sub-agents included: in a delegation the one doing the work is the child. Three rendering rules are load-bearing and each closes a specific failure: a **stable order** (scope, then id) because the index sits inside the provider's cache key; a **deterministic tail cut** at an 8 KB budget, announced by a `[N more skills omitted]` line, because a silently truncated index has the model conclude in good faith that a skill does not exist; and **empty in, empty out** — every word of prose lives inside the render, so an instance with no skills spends zero tokens and leaves no orphan sentence (the MCP list is the counter-example: its prose sits *around* the placeholder, and the empty state once had the model inventing a discovery tool). A colliding id is marked `[name collision]` on **both** lines, never shadowed. A malformed skill is skipped with a `warn!`, never fatal — the index is built while assembling a prompt. Freshness has two doors, one per writer. The in-process tools invalidate directly (`Skald::invalidate_prompt_prefix`, called by `skill_register`/`skill_delete`); a hand edit on the box is caught by the **skills watcher** (`skills/watch.rs`, spawned from `spawn_background`): a recursive `notify` on `{WD}/skills` + `{WD}/skills-users`, debounced ~800 ms, that re-digests each touched tree (`skills::tree_digest` — the (id, description) pairs the index is made of) and emits `SystemEvent::SkillsChanged { scope }` only when the digest moved. The subscriber `spawn_skills_freshness` (next to `spawn_user_lifecycle`, same `Weak` shape) maps the scope and calls the same invalidate accessor. Editing a script leaves the digest byte-identical and announces nothing — which is exactly the §6 rule, so an invisible change costs nobody a cache miss. Two gotchas the code carries comments for: the watcher **canonicalizes `{WD}`** (FSEvents reports real paths, and `/var` is a symlink on macOS), and it creates the two trees if absent (a box before its first user has neither).
|
||||
|
||||
**Containment** (`resolve_host_path`) is unchanged and still guards **the host branch**: every path that lands on a mount is canonicalized (following symlinks) and prefix-checked against its mount base, **fail-closed**. That check is what it always was — the defence against a symlink planted from inside the container pointing at the **host's** `/etc`, which the host-side tool would otherwise follow off the box. Opening the container branch does not weaken it: that branch never touches the host filesystem, so there is no host to escape from, and the check keeps applying to everything mounted. `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.
|
||||
|
||||
Reference in New Issue
Block a user