diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..5624de2 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,66 @@ +# Changelog + +All notable changes to Skald Circle are recorded here, newest first. + +The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versions +are the workspace `Cargo.toml` version — the one `ci/verify-version.sh` checks before a +release PR may merge — and a section is closed at the commit that bumps it. + +## [Unreleased] + +### Added + +- Several conversations per source: open extra chats with `+`, and the tab bar you left + open is restored at your next login, on any device. +- A background task now reports back into the chat that started it instead of only the + Inbox, and a chat shows the tasks still running under it. +- Skills reworked for the multi-user model: a shared tree plus a per-member one, with a + generated index injected into the agent's prompt. +- The agent is told what its sandbox can actually run, from a probe of its own container. +- Event triage can be tuned per person: a check interval that overrides the instance one, + and notification preferences read from `user-memory/notifications.md`. +- File viewer: syntax highlighting for code files and for code blocks in the chat, a + hover copy button on those blocks, and history browsing for a file under git. +- Project explorer: download a folder as a streaming ZIP. +- Collapsible icon-only sidebar on desktop. +- DeepInfra, as a declarative LLM provider. +- The project coordinator offers to keep a history of a project. +- An agent can ask which connectors it holds instead of guessing. + +### Changed + +- Runtime image `v4`: Debian 13 base, plus the shared libraries a headless Chromium needs. +- Unencrypted users are unlocked and their runtimes started at boot, so Telegram, cron and + the background agents work after a restart without anyone opening the web app first. +- PDFs render through pdf.js instead of an iframe. + +### Fixed + +- The server keeps running after you log out of the box; the install / update / uninstall + scripts were hardened alongside it. +- Skald survives a restart of the Docker daemon. +- A user database gets the owner schema re-applied when it is opened. +- An approval bypass applies to the tool it was granted for, not to its whole connector. +- Connectors: an admin can use the ones they implicitly hold, per-user ones appear in the + security-group picker, one whose process died is brought back, a global one's + dependencies are installed where they are needed, and the prompt's connector list is + rebuilt when the set changes. +- Telegram: pairing codes are no longer burned on the way out nor handed out unrecorded, + and `send_attachment` resolves paths in the user's own workspace. +- The notification home is stored in the owner's database instead of the registry, where + it silently dropped every batch it built. +- LLM calls send the provider's model id on the wire rather than the local alias, and + catalog capabilities resolve for reasoning-mode queries. +- `get_ast_outline` runs in the caller's workspace, gives a markdown heading a section + range instead of a single line, and shows a proper name and icon on its chat card. +- The re-login dialog no longer hijacks the login screen, the new-chat `+` menu is visible + and clickable, and the session-detail page stays live instead of freezing on a snapshot. +- A silently dead agent WebSocket is detected and redialled. +- A generated image lands in your own workspace instead of a server folder nobody could + reach, so the assistant can finally send it to you on Telegram, open it in the viewer, + or work on it with a command. It still shows inline in the web chat, its file is named + after the prompt, and it is now readable only by the person who asked for it. + +--- + +Releases up to and including `0.2.0` predate this file; `git log` is the record for them. diff --git a/CLAUDE.md b/CLAUDE.md index 44a143d..eb08023 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -444,6 +444,14 @@ Create `agents//meta.json` and `agents//AGENT.md`. The agent is discover `docs/` is **not developer documentation** — it's written for the in-app LLM, not for a human reading the repo, and is mounted read-only into every user's container at `~/docs/` (see the Filesystem & containers section: `docs_host` on `UserFs`, `DOCS_DIR` in `container/mod.rs`). It explains the software's UX (plugins, and eventually agents/connectors/memory/roles/…) in plain terms, in English, so the assistant can help a non-technical user configure things instead of guessing. `docs/index.md` is the entry point (general index of feature pages); `docs/plugins/.md` covers each built-in plugin. The three `type: chat` agents (`assistant`, `kid`, `project-coordinator`) are told in their `AGENT.md` to read `docs/index.md` when a user asks how the software works. **Standing rule: every change that impacts the UX must update `docs/` in the same change** — a new/renamed feature page plus the `docs/index.md` index entry. It goes stale like any other doc, except users actually see this one. +### The changelog + +`CHANGELOG.md` (repo root) is the release history, and it carries the **twin standing rule**: every change a user or an operator would notice must add a bullet under `## [Unreleased]` **in the same change** — a feature, a behaviour change, a bug fix, a new config key, an image-tag bump. Same reason as `docs/`: written after the fact it is written from the diff, which is exactly the version nobody can use. + +Format is [Keep a Changelog](https://keepachangelog.com): newest first, one `## [x.y.z] - YYYY-MM-DD` section per released version, bullets grouped under `Added` / `Changed` / `Fixed` / `Removed` / `Security`. The versions are the **workspace `Cargo.toml` version** — the same string `ci/verify-version.sh` gates a release PR on — so cutting a release is two edits in one commit: bump `version` in `Cargo.toml`, and rename `## [Unreleased]` to the version with today's date, leaving a fresh empty `Unreleased` above it. There are no git tags on this repo; the changelog *is* the record of what a given `v{version}` tarball contains. + +Entries are written **for the person reading the release, not for the person who wrote the code**: say what changed for them, not which module moved — the commit message and the diff already hold that. Which is also the test for whether a bullet is owed at all: a refactor with no observable effect gets none, however large. Keep one bullet per user-visible thing, not one per commit, and fold a fix-on-top-of-an-unreleased-feature into that feature's bullet rather than listing a bug that never shipped. History before `0.2.0` is not covered — git is the record for it. + ## Config Copy `default.config.yaml` → `config.yml`. Never commit `config.yml` (contains API keys). diff --git a/crates/skald-core/src/image_generate/manager.rs b/crates/skald-core/src/image_generate/manager.rs index f40853a..4070674 100644 --- a/crates/skald-core/src/image_generate/manager.rs +++ b/crates/skald-core/src/image_generate/manager.rs @@ -9,12 +9,10 @@ /// /// `get(id)` resolves by explicit id across both plugin and DB-backed providers. /// When called without an id, plugin providers take precedence over DB-backed ones. -use std::path::PathBuf; use std::sync::Arc; -use anyhow::{Context, Result, anyhow}; +use anyhow::{Result, anyhow}; use async_trait::async_trait; -use rand::RngExt; use sqlx::SqlitePool; use tokio::sync::RwLock; use tracing::{info, warn}; @@ -50,14 +48,12 @@ pub struct ImageGeneratorManager { pool: Arc, registry: Arc, state: RwLock, - data_root: PathBuf, } impl ImageGeneratorManager { pub async fn new( - pool: Arc, - registry: Arc, - data_root: impl Into, + pool: Arc, + registry: Arc, ) -> Result> { let mgr = Arc::new(Self { pool, @@ -66,7 +62,6 @@ impl ImageGeneratorManager { db_slots: Vec::new(), plugins: Vec::new(), }), - data_root: data_root.into(), }); mgr.reload().await?; Ok(mgr) @@ -192,32 +187,30 @@ impl ImageGeneratorManager { // ── Generation ──────────────────────────────────────────────────────────── - pub async fn generate( + /// Renders `prompt` with `provider_id` and hands the raw bytes back. + /// + /// **Placement is the caller's**, deliberately. This used to write the file + /// into the server's own `data/images/` and return that host path to + /// the model — a path in nobody's vocabulary: it is not the caller's home, + /// not their container, and every consumer downstream resolves agent paths + /// (§6). Telegram's `send_attachment` therefore looked for + /// `data/images/x.png` under the user's home and answered "file not found", + /// and `read_file`/`execute_cmd`/the viewer could not reach it either. The + /// manager has no `UserFs` and no session, so the one place that does — the + /// tool, through its `ToolContext` — owns where the image lands. + pub async fn generate_bytes( &self, provider_id: &str, prompt: &str, extra_params: Option<&serde_json::Value>, - ) -> Result<(PathBuf, String)> { + ) -> Result> { let provider = self.get(provider_id).await .ok_or_else(|| anyhow!("image provider '{}' not found", provider_id))?; - let images_dir = self.data_root.join("images"); - tokio::fs::create_dir_all(&images_dir).await?; - let bytes = provider.generate(prompt, extra_params).await?; + info!(provider_id, bytes = bytes.len(), "image generated"); - let file_id: String = rand::rng() - .sample_iter(rand::distr::Alphanumeric) - .take(32) - .map(char::from) - .collect(); - let path = images_dir.join(format!("{file_id}.png")); - tokio::fs::write(&path, &bytes).await?; - - let url = format!("/api/images/{file_id}"); - info!(provider_id, path = %path.display(), "image generated"); - - Ok((path, url)) + Ok(bytes) } // ── Tool injection ───────────────────────────────────────────────────────── @@ -236,10 +229,6 @@ impl ImageGeneratorManager { ] } - pub fn images_dir(&self) -> PathBuf { - self.data_root.join("images") - } - // ── Private ─────────────────────────────────────────────────────────────── async fn reload(&self) -> Result<()> { diff --git a/crates/skald-core/src/skald/bundles.rs b/crates/skald-core/src/skald/bundles.rs index 0b2b6ad..946eaf8 100644 --- a/crates/skald-core/src/skald/bundles.rs +++ b/crates/skald-core/src/skald/bundles.rs @@ -108,7 +108,6 @@ impl Media { let image_generator_manager = ImageGeneratorManager::new( Arc::clone(&rt.db), Arc::clone(&models.provider_registry), - "data", ).await?; // Evaluate the await outside the `info!` macro: leaving the temporary // `tracing::Value` from the field expression alive across the await diff --git a/crates/skald-core/src/tools/image_generate.rs b/crates/skald-core/src/tools/image_generate.rs index 038c06b..b681a95 100644 --- a/crates/skald-core/src/tools/image_generate.rs +++ b/crates/skald-core/src/tools/image_generate.rs @@ -4,7 +4,10 @@ use anyhow::Result; use serde_json::{Value, json}; use crate::image_generate::ImageGeneratorManager; -use crate::tools::{Tool, ToolCategory, ToolDescriptionLength, truncate_label, MAX_LABEL_SHORT, MAX_LABEL_FULL}; +use crate::tools::{ + SimpleExecution, Tool, ToolCategory, ToolContext, ToolDescriptionLength, ToolExecution, + ToolResult, truncate_label, MAX_LABEL_SHORT, MAX_LABEL_FULL, +}; // ── image_generate_providers_list ───────────────────────────────────────────── @@ -52,8 +55,12 @@ impl Tool for ImageGenerateTool { fn category(&self) -> ToolCategory { ToolCategory::Config } fn description(&self) -> &str { - "Generate an image from a text prompt. \ - Blocks until the image is ready, then returns the local path and a web URL." + "Generate an image from a text prompt. Blocks until the image is ready, then saves \ + it into your own workspace and returns `{path, url}`. `path` (under `uploads/…`, \ + relative to your home) is in your usual vocabulary — pass it to show_file_to_user, \ + send_attachment, read_file or execute_cmd. `url` renders the image inline in the \ + web and mobile chat if you embed it as a Markdown image, ![](url); it is a web \ + link, so on a channel without Markdown (Telegram) send the file itself instead." } fn parameters_schema(&self) -> Value { @@ -87,9 +94,27 @@ impl Tool for ImageGenerateTool { } } - fn execute_async<'a>(&'a self, args: Value) -> std::pin::Pin> + Send + 'a>> { - let mgr = Arc::clone(&self.mgr); - Box::pin(async move { + /// The generated image is written into the **caller's** workspace, so the path + /// this returns is the one agent vocabulary every consumer already speaks: + /// `send_attachment` (Telegram), `show_file_to_user` (web/mobile), the fs-tools + /// and `execute_cmd` all resolve it through the same [`UserFs`]. It previously + /// returned a path under the server's own data root, which none of them could + /// reach — the model got a file it could not hand to anyone. + /// + /// `uploads/{session}/` rather than a directory of its own: that is the single + /// placement seam (`uploads::save_to_home`, collision-safe naming included) and + /// the one directory the media inliner is authorized to read from, so a vision + /// model can be shown the image it just made. + /// + /// A `url` rides alongside the path because the chat renders Markdown images — + /// so `![](url)` in the answer shows the picture inline instead of naming a file + /// the user then has to open. + fn run_with<'a>(&'a self, ctx: &ToolContext, args: Value) -> Box { + let mgr = Arc::clone(&self.mgr); + let fs = Arc::clone(&ctx.fs); + let session_id = ctx.session_id; + + Box::new(SimpleExecution::new(Box::pin(async move { let provider_id = args["provider_id"].as_str() .ok_or_else(|| anyhow::anyhow!("missing provider_id"))? .to_string(); @@ -100,8 +125,129 @@ impl Tool for ImageGenerateTool { Value::Object(_) => Some(args["extra_params"].clone()), _ => None, }; - let (path, url) = mgr.generate(&provider_id, &prompt, extra_params.as_ref()).await?; - Ok(json!({ "path": path, "url": url }).to_string()) + + let bytes = mgr.generate_bytes(&provider_id, &prompt, extra_params.as_ref()).await?; + + // The extension is sniffed, never assumed: providers return png, jpeg or + // webp, and it is the extension that decides whether Telegram sends the + // file inline as a photo or as a nondescript document. + let mime = crate::session::handler::media::sniff_mime(&bytes[..bytes.len().min(16)]) + .unwrap_or("image/png"); + let name = file_name_for(&prompt, mime); + + let att = crate::uploads::save_to_home( + &fs, session_id, &name, Some(mime.to_string()), &bytes, + ).await?; + + let url = file_url(&att.path); + Ok(ToolResult::Text(json!({ "path": att.path, "url": url }).to_string())) + }))) + } + + /// Generation needs the caller's workspace to put the image in, and only + /// [`run_with`](Self::run_with) is handed one. Same shape as `execute_cmd`: + /// the context-free path fails loudly rather than writing somewhere nobody + /// can read. + fn execute_async<'a>(&'a self, _args: Value) -> std::pin::Pin> + Send + 'a>> { + Box::pin(async move { + Err(anyhow::anyhow!( + "image_generate needs a session context to save the image into your workspace" + )) }) } } + +/// Web URL for a file in the caller's workspace: the per-user file endpoint, the +/// same one the viewer and the explorer read through. +/// +/// Deliberately an **ownership-scoped** endpoint. Generated images used to be +/// served from one instance-wide directory by id, behind `require_auth` alone — +/// authenticated but with no notion of who owned the image, the same shape as the +/// `/data` static mount that was removed for exactly that. `/api/file` resolves +/// the path through the caller's own `UserFs`, so a link that leaks reveals +/// nothing to anyone not already entitled to it. (That id-based route is gone: it +/// had no writer left once placement moved into the workspace.) +fn file_url(agent_path: &str) -> String { + // Same idiom as `mcp::oauth`: a throwaway URL's query is a correctly + // percent-encoded `path=…`, without hand-rolling an encoder. + let query = reqwest::Url::parse_with_params("http://local/", &[("path", agent_path)]) + .ok() + .and_then(|u| u.query().map(str::to_owned)) + .unwrap_or_else(|| format!("path={agent_path}")); + + format!("/api/file?{query}") +} + +/// Builds a file name from the prompt, so the user sees `a-red-bicycle.png` in +/// their files and in Telegram rather than a random id. Collisions are the upload +/// seam's problem (it appends `_1`, `_2`, …), so this need not be unique. +fn file_name_for(prompt: &str, mime: &str) -> String { + let ext = match mime { + "image/jpeg" => "jpg", + "image/webp" => "webp", + "image/gif" => "gif", + _ => "png", + }; + + let mut slug = String::new(); + for ch in prompt.chars() { + if slug.len() >= 48 { break; } + if ch.is_ascii_alphanumeric() { + slug.extend(ch.to_lowercase()); + } else if !slug.ends_with('-') { + slug.push('-'); + } + } + let slug = slug.trim_matches('-'); + let stem = if slug.is_empty() { "image" } else { slug }; + + format!("{stem}.{ext}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn url_points_at_the_per_user_file_endpoint_and_encodes_the_path() { + assert_eq!( + file_url("uploads/7/a-red-bicycle.png"), + "/api/file?path=uploads%2F7%2Fa-red-bicycle.png", + ); + // A name the slug rules cannot produce, but that the endpoint must still + // receive intact rather than as two query params. + assert_eq!( + file_url("uploads/7/a&b c.png"), + "/api/file?path=uploads%2F7%2Fa%26b+c.png", + ); + } + + #[test] + fn file_name_slugs_the_prompt_and_keys_the_extension_on_the_mime() { + assert_eq!(file_name_for("A red bicycle", "image/png"), "a-red-bicycle.png"); + assert_eq!(file_name_for("A red bicycle", "image/jpeg"), "a-red-bicycle.jpg"); + assert_eq!(file_name_for("A red bicycle", "image/webp"), "a-red-bicycle.webp"); + // An unrecognized type still produces a usable image name. + assert_eq!(file_name_for("A red bicycle", "application/octet-stream"), "a-red-bicycle.png"); + } + + #[test] + fn file_name_never_carries_path_or_shell_syntax_out_of_the_prompt() { + // Everything non-alphanumeric collapses to a single dash, so a prompt can + // neither escape the uploads directory nor smuggle syntax into a later + // `execute_cmd` on the returned path. + assert_eq!(file_name_for("../../etc/passwd", "image/png"), "etc-passwd.png"); + assert_eq!(file_name_for("a $(whoami) cat", "image/png"), "a-whoami-cat.png"); + // A prompt with nothing usable still yields a name. + assert_eq!(file_name_for("...", "image/png"), "image.png"); + assert_eq!(file_name_for("", "image/png"), "image.png"); + } + + #[test] + fn file_name_stays_short_for_a_long_prompt() { + let name = file_name_for(&"word ".repeat(60), "image/png"); + assert!(name.len() <= 53, "{name}"); + assert!(name.ends_with(".png")); + assert!(!name.starts_with('-') && !name.contains("-.")); + } +} diff --git a/docs/plugins/comfyui.md b/docs/plugins/comfyui.md index 1465f9f..e71be0a 100644 --- a/docs/plugins/comfyui.md +++ b/docs/plugins/comfyui.md @@ -28,4 +28,5 @@ The plugin polls the ComfyUI server every 5 seconds. If it's offline, every mode - A single generation can take up to 5 minutes before the plugin times out and reports an error. - Image-to-image is supported when the workflow declares an `input_image_node` in `_personal_agent`. +- A generated image is saved into the requesting user's own workspace, under `uploads//`, and the tool returns both a path and a web URL. In the web or mobile chat, embedding the URL as a Markdown image shows the picture directly in the conversation; the path is for everything else — `show_file_to_user`, `send_attachment` on Telegram (which has no Markdown, so the file must be sent, not linked), and ordinary file work like reading it, moving it into a project or a shared folder, or processing it with a command. - If ComfyUI is unreachable, tell the user to start their ComfyUI server — there is nothing to fix in this app's configuration in that case. diff --git a/docs/plugins/telegram.md b/docs/plugins/telegram.md index 4a06c2b..597424b 100644 --- a/docs/plugins/telegram.md +++ b/docs/plugins/telegram.md @@ -34,3 +34,4 @@ That's the whole flow — no admin involvement needed for a normal pairing. (An - Output sent to Telegram is automatically constrained to Telegram-safe HTML formatting (bold, italic, code blocks, links, quotes) — no Markdown, no tables. This is handled automatically; nothing to configure. - Revoking a user's plugin access immediately stops that person's Telegram chat from working, without needing them to re-pair if access is restored later. +- You can send a file from your workspace into the Telegram chat with `send_attachment`, giving it a path in your usual vocabulary (`~/report.pdf`, `uploads/…`, `shared/…`, or an absolute path inside your sandbox). Images and videos arrive inline, anything else as a downloadable file. This is the Telegram equivalent of `show_file_to_user`, which only exists in the web and mobile apps — on Telegram a generated image or a document you produced has to be sent this way, or the user never sees it. diff --git a/src/frontend/api/images.rs b/src/frontend/api/images.rs deleted file mode 100644 index 329bcbc..0000000 --- a/src/frontend/api/images.rs +++ /dev/null @@ -1,37 +0,0 @@ -use axum::{ - extract::{Path, State}, - http::{HeaderValue, StatusCode, header}, - response::{IntoResponse, Response}, -}; -use tokio::fs; - -use std::sync::Arc; -use skald_core::skald::Skald; - -/// GET /api/images/:task_id -/// -/// Serves a generated image from `data/images/.png`. -pub async fn get_image( - State(skald): State>, - Path(task_id): Path, -) -> Response { - // Reject any path traversal attempts. - if task_id.contains('/') || task_id.contains('\\') || task_id.contains("..") { - return StatusCode::BAD_REQUEST.into_response(); - } - - let task_id = task_id.trim_end_matches(".png"); - let path = skald.image_generator_manager().images_dir().join(format!("{task_id}.png")); - - match fs::read(&path).await { - Ok(bytes) => { - let mut response = bytes.into_response(); - response.headers_mut().insert( - header::CONTENT_TYPE, - HeaderValue::from_static("image/png"), - ); - response - } - Err(_) => StatusCode::NOT_FOUND.into_response(), - } -} diff --git a/src/frontend/api/mod.rs b/src/frontend/api/mod.rs index a29557d..01e7a66 100644 --- a/src/frontend/api/mod.rs +++ b/src/frontend/api/mod.rs @@ -11,7 +11,6 @@ pub mod guard; pub mod stats; pub mod files; pub mod image_generate_models; -pub mod images; pub mod inbox; pub mod llm; pub mod marketplace; @@ -229,8 +228,6 @@ pub fn router() -> Router> { .route("/shared-folders/{id}", patch(shared_folders::update_description).delete(shared_folders::delete)) .route("/shared-folders/{id}/members", post(shared_folders::add_member)) .route("/shared-folders/{id}/members/{user_id}", delete(shared_folders::remove_member)) - // Images (generated by image_generate tool) - .route("/images/{task_id}", get(images::get_image)) // MCP tool-result media (images/audio/files returned by MCP servers) .route("/mcp-media/{file}", get(mcp_media::get_media)) // Files