remove agent-callable restart tool (blast radius in multi-user model)
Nightly Build / build (push) Has been cancelled

This commit is contained in:
2026-07-22 09:22:11 +01:00
parent 5d5c3ff2ff
commit ff3e6942cc
12 changed files with 14 additions and 59 deletions
+8 -8
View File
@@ -55,7 +55,7 @@ The application core is the `skald-core` crate; the binaries are **shells** arou
Two rules keep the boundary real, and both are enforced by the compiler:
- **The core never names a plugin.** A plugin contributes tools through `Plugin::tools(self: Arc<Self>)` — the sibling of `http_router()` — so nothing in the core has to downcast to a concrete type. Naming one would drag every plugin in the tree into the core, including a C build via `plugin-transcribe-whisper-local`.
- **The core never learns about the process shell.** The `restart` tool defaults to the supervisor protocol (`exit(-1)`); a shell with different needs (e.g. one with no supervisor) can install its own `tools::restart::set_restart_handler` at startup. The default server shell installs none and relies on `run.sh`. The seam stays even though nothing installs a handler today.
- **The core never learns about the process shell.** There is no in-core restart hook — the former `restart` tool and its `tools::restart::set_restart_handler` seam were removed. The only coupling to the supervisor is now the `run.sh` exit-code protocol (exit `255` ⇒ re-exec the same binary by path), a seam no code currently triggers (kept for a future admin-driven restart). The live expression of this principle is `skald_core::boot`, which emits startup lines each shell renders (`src/boot_format.rs` here).
**Plugin visibility & per-user config.** The admin surface is split in two: `#plugin-catalog` (`plugin-catalog.js`) is a status board — one card per plugin with an enable toggle + health dot + a Configure button — and `#plugin-detail?id=<id>` (`plugin-detail.js`) holds the instance-config form + per-user access checklist for one plugin (the plugin counterpart of `connector-detail.js`). The user-facing half is `#plugins` (`plugins-page.js`): granted plugins + their per-user config forms. Enable/disable + instance config + access grants are gated by the `plugin.manage` capability (admin-only by construction). Visibility is **opt-in**: a row in `plugin_access(plugin_id, user_id)` grants a user sight of an enabled plugin (`plugin_id` is bare TEXT, never a FK — a `plugins` row exists only after the first toggle). A plugin with a non-empty `Plugin::user_config_schema()` exposes per-user settings, stored in `plugin_user_configs` (**admin-readable system.db — never secrets**) and applied through the `Plugin::update_user_config` hook, whose default just stores the blob via the `PluginUserConfigApi` on `PluginContext.user_config`. Telegram is the reference impl: the user pastes the bot's pairing code in their Plugins page, the override turns it into a `chat_id → user_id` binding (same write path as the `telegram_pairing` tool) and stores a `{linked, chat_id}` status blob for the UI. Endpoints: admin `GET/PUT /api/plugins[/{id}]` + `GET/PUT /api/plugins/{id}/access`; user `GET /api/plugins/mine` + `PUT /api/plugins/{id}/my-config`.
@@ -74,7 +74,7 @@ Two rules keep the boundary real, and both are enforced by the compiler:
| `crates/skald-core/src/chat_hub/` | `ChatHub`: broadcast events to all connected WS clients |
| `crates/skald-core/src/chat_event_bus.rs` | Global async bus for cross-session events |
| `crates/skald-core/src/agents.rs` | Discovers agents from `agents/*/`, loads meta + system prompt |
| `crates/skald-core/src/tools/` | Built-in tools: `exec` (**runs inside the caller's per-user Docker container** via `docker exec`, as the non-root host uid — `sudo` for system installs — with a robust /stop that reaps the command's process-group; see `container/`; the only live path is `run_with` (needs `ToolContext`) — the context-free `Tool::execute`/`execute_async` now **error** (`HOST_PATH_ERROR`) instead of the old host `sh -c`, so nothing can run a command outside the sandbox), `restart`, `list_agents`, `fs/*` (route `user-memory/`/`shared-memory/` to `memory_docs`, and every other **physical** path through `ctx.fs` to the caller's per-user host workspace — see DB tables + container), `notify`, `ast_outline`, `image_generate`, MCP tools, plugin tools, cron tools |
| `crates/skald-core/src/tools/` | Built-in tools: `exec` (**runs inside the caller's per-user Docker container** via `docker exec`, as the non-root host uid — `sudo` for system installs — with a robust /stop that reaps the command's process-group; see `container/`; the only live path is `run_with` (needs `ToolContext`) — the context-free `Tool::execute`/`execute_async` now **error** (`HOST_PATH_ERROR`) instead of the old host `sh -c`, so nothing can run a command outside the sandbox), `list_agents`, `fs/*` (route `user-memory/`/`shared-memory/` to `memory_docs`, and every other **physical** path through `ctx.fs` to the caller's per-user host workspace — see DB tables + container), `notify`, `ast_outline`, `image_generate`, MCP tools, plugin tools, cron tools |
| `crates/skald-core/src/container/` | `ContainerManager` (§6): per-user Docker containers (the execution sandbox). Docker is a **hard requirement**`check_docker()` fails `Skald::new` (→ shell exits) if the daemon is unreachable. Builds our own `skald-runtime` image (python+node+**sudo**; tag is **versioned** `skald-runtime:v2` so a `Dockerfile` change forces a rebuild) once from the embedded `Dockerfile`, then `reconcile_all()` at boot ensures one running container `skald-{userid}` per active user. Each container runs as the **host `uid:gid`** (`--user`, §6 UID coherence) with `--init` (tini reaps zombies); `ensure()` **self-heals** a container whose `--user` is stale (e.g. an old root one) by recreating it, and injects a passwd/shadow entry post-create so `sudo` (NOPASSWD, in the image) resolves the arbitrary uid. `build_user_fs()` assembles a user's `UserFs` (home `{WD}/homes/{userid}``/root`, plus each `shared/{name}` they belong to). Shells the `docker` CLI (no client crate) |
| `crates/skald-core/src/tool_catalog.rs` | `ToolCatalog`: unified tool listing façade (wraps ToolRegistry + McpManager) |
| `crates/skald-core/src/events.rs` | `ServerEvent` enum streamed over WebSocket to the frontend |
@@ -207,9 +207,9 @@ At context-build time (`MessageBuilder`), attachments of the **current turn** (t
## Approval gate
The rule engine `ApprovalManager::check` returns `Allow`/`Deny`/`Require` per tool call (default rules seeded on first boot; the catch-all `* require @999999` gates anything not explicitly allowed — e.g. `execute_cmd`, `restart`, `execute_task`, writes outside whitelisted paths). A `Require` registers a `oneshot` in the in-memory `pending` map keyed by `request_id` and emits an approval event over WS.
The rule engine `ApprovalManager::check` returns `Allow`/`Deny`/`Require` per tool call (default rules seeded on first boot; the catch-all `* require @999999` gates anything not explicitly allowed — e.g. `execute_cmd`, `execute_task`, writes outside whitelisted paths). A `Require` registers a `oneshot` in the in-memory `pending` map keyed by `request_id` and emits an approval event over WS.
Resolution is **source-agnostic**: the WS + Inbox paths resolve by `request_id`; the inline chat card resolves by the durable `tool_call_id` via `POST /api/tools/:tool_call_id/resolve` (`resolve_tool` in `src/frontend/api/sessions.rs`), which derives the owning session from the tool call's own stack row — never a hardcoded source. Live pending cards fire the `oneshot`; post-restart a simple tool runs directly on the owning session via `ChatSessionHandler::execute_tool`, which now goes through the **same canonical path as the live loop**`build_execution` (owner pool + per-user container `ToolContext`) driven by `drive_execution` — so a resolved `write_file`/`execute_cmd` acts on the user's workspace/container, never the server cwd/host (was a §6 escape; sub-agent + `restart` tools are still handled by their own branches earlier in `resolve_tool`). See `docs/approval/`.
Resolution is **source-agnostic**: the WS + Inbox paths resolve by `request_id`; the inline chat card resolves by the durable `tool_call_id` via `POST /api/tools/:tool_call_id/resolve` (`resolve_tool` in `src/frontend/api/sessions.rs`), which derives the owning session from the tool call's own stack row — never a hardcoded source. Live pending cards fire the `oneshot`; post-restart a simple tool runs directly on the owning session via `ChatSessionHandler::execute_tool`, which now goes through the **same canonical path as the live loop**`build_execution` (owner pool + per-user container `ToolContext`) driven by `drive_execution` — so a resolved `write_file`/`execute_cmd` acts on the user's workspace/container, never the server cwd/host (was a §6 escape; sub-agent tools are still handled by their own branch earlier in `resolve_tool`). See `docs/approval/`.
The **diff preview** in a `PendingWrite` event (`handler/approval.rs::read_current_content`) routes exactly like the fs-tools: `user-memory/`/`shared-memory/``memory_docs` on the right pool, every other agent path → the caller's host workspace via `resolve_host_path(&self.fs, …)`. It must never use the cwd-relative `fs::resolve` — that showed a bogus "new file" on overwrites (or the diff of a same-named cwd file), so the user would approve the wrong diff.
@@ -217,11 +217,11 @@ The **diff preview** in a `PendingWrite` event (`handler/approval.rs::read_curre
## Restart
`restart` **no longer rebuilds anything** — it does not compile.
There is **no in-app restart** anymore. The agent-callable `restart` tool and its `set_restart_handler` seam were removed (blast radius = the whole box: it dropped every user's session and in-RAM DEK from one user's chat — a power-user leftover, out of place in the multi-user model). Nothing in the process now calls `libc::_exit(-1)`.
No restart handler is installed, so `restart` calls `libc::_exit(-1)` (= exit code 255); `run.sh` re-executes the same binary *by path*. (The `set_restart_handler` seam stays for a hypothetical shell without a supervisor, but nothing installs a handler today.)
The supervisor protocol survives but is currently **unreachable in-app**: `run.sh` still re-executes the binary *by path* when it exits `255`, but no code produces that exit code. Restarting is therefore a manual/admin operation.
Use it to pick up `config.yml` / `providers.yaml` / database changes, which are only read at startup. To load new **code**: `./build.sh`, then restart — the supervisor picks up the new binary on the next loop, since `build.sh` installs it with an atomic rename.
To pick up `config.yml` / `providers.yaml` / database changes (read only at startup), or to load new **code** (`./build.sh` installs the new binary via atomic rename): stop the server and let `run.sh` loop, or re-run `./run.sh`. A future admin-only restart action (endpoint/button gated by an admin capability) would re-use the `255 ⇒ re-exec` seam — it is intentionally kept for that.
> `run.bat` is still stale (`cargo run`) and must be fixed.
@@ -253,7 +253,7 @@ The `docs/` directory is **ignored** for now — do not read it, reference it, o
Copy `default.config.yaml``config.yml`. Never commit `config.yml` (contains API keys).
`providers.yaml` (repo root, cwd-relative like `config.yml`) declares the **OpenAI-compatible LLM provider types** — endpoints, UI metadata, per-model JSON field mapping, id-glob enrichment rules, reasoning knobs. Loaded at boot by `llm::providers::declared`; edit + `restart`, no rebuild. An invalid entry is logged and skipped, never fatal; an `id` colliding with a native provider is skipped. Adding a new OpenAI-compatible provider is a YAML edit, not a Rust file. The shipped file is validated by a unit test (`declared::tests::shipped_providers_yaml_is_valid`).
`providers.yaml` (repo root, cwd-relative like `config.yml`) declares the **OpenAI-compatible LLM provider types** — endpoints, UI metadata, per-model JSON field mapping, id-glob enrichment rules, reasoning knobs. Loaded at boot by `llm::providers::declared`; edit + restart the process, no rebuild. An invalid entry is logged and skipped, never fatal; an `id` colliding with a native provider is skipped. Adding a new OpenAI-compatible provider is a YAML edit, not a Rust file. The shipped file is validated by a unit test (`declared::tests::shipped_providers_yaml_is_valid`).
## Python environment