fix(image-generate): save generated images into the caller's workspace

image_generate wrote the file into the server's own data/images/ and handed
that host path to the model. It is a path in nobody's vocabulary: not the
caller's home, not their container. Telegram's send_attachment therefore
resolved it under the user's home and answered "file not found", and
read_file, execute_cmd and the viewer could not reach it either. The web URL
was the only surface that worked, which is why the failure only ever showed
on Telegram -- and why the model there, having no working way to hand the
file over, started inventing send_photo and send_media.

Placement moves to the tool, the one place holding a ToolContext:

- The manager returns bytes (generate_bytes) and no longer knows where an
  image goes. It has no UserFs and no session, so it never could have.

- run_with saves through uploads::save_to_home into uploads/{session}/. The
  returned path is agent vocabulary, so every consumer resolves it, and that
  is the one directory the media inliner is authorized to read from -- a
  vision model can be shown the image it just made. execute_async, the
  context-free path, now fails loudly rather than writing somewhere nobody
  can read; same shape as execute_cmd.

- The extension is sniffed rather than assumed png: it is what decides
  whether Telegram sends the picture inline or as an anonymous document, and
  providers return jpeg and webp too. The file is named after the prompt, so
  it reads as something in the explorer and in Telegram.

The result still carries a url, since the chat renders Markdown images and
![](url) beats naming a file the user then has to open. It points at
/api/file?path=..., which resolves through the caller's own UserFs. The old
/api/images/{id} route is removed: it had no writer left once placement
moved, and it addressed one instance-wide directory behind require_auth
alone, with no notion of who owned the image -- the same shape as the /data
static mount removed before it. That leaves data_root unused, so the manager
no longer knows about the server's filesystem at all.

Docs: the Telegram page explains send_attachment as the channel's equivalent
of show_file_to_user; the ComfyUI page says where a generated image lands and
which of the two handles to use where.

Also introduces CHANGELOG.md and the standing rule for it in CLAUDE.md.

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<SqlitePool>,
     registry:  Arc<ProviderRegistry>,
     state:     RwLock<ManagerState>,
-    data_root: PathBuf,
 }
 
 impl ImageGeneratorManager {
     pub async fn new(
-        pool:      Arc<SqlitePool>,
-        registry:  Arc<ProviderRegistry>,
-        data_root: impl Into<PathBuf>,
+        pool:     Arc<SqlitePool>,
+        registry: Arc<ProviderRegistry>,
     ) -> Result<Arc<Self>> {
         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<Vec<u8>> {
         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<()> {

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

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<Box<dyn std::future::Future<Output = Result<String>> + 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<dyn ToolExecution + 'a> {
+        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<Box<dyn std::future::Future<Output = Result<String>> + 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("-."));
+    }
+}