mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
This commit is contained in:
oxoxDev
Generated
+145
@@ -30,6 +30,7 @@ dependencies = [
|
||||
"rand 0.9.4",
|
||||
"reqwest 0.12.28",
|
||||
"resvg",
|
||||
"rfd",
|
||||
"rusqlite",
|
||||
"rustls",
|
||||
"sentry",
|
||||
@@ -262,6 +263,27 @@ version = "0.7.6"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50"
|
||||
|
||||
[[package]]
|
||||
name = "ashpd"
|
||||
version = "0.11.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "d2f3f79755c74fd155000314eb349864caa787c6592eace6c6882dad873d9c39"
|
||||
dependencies = [
|
||||
"enumflags2",
|
||||
"futures-channel",
|
||||
"futures-util",
|
||||
"rand 0.9.4",
|
||||
"raw-window-handle",
|
||||
"serde",
|
||||
"serde_repr",
|
||||
"tokio",
|
||||
"url",
|
||||
"wayland-backend",
|
||||
"wayland-client",
|
||||
"wayland-protocols",
|
||||
"zbus",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "async-broadcast"
|
||||
version = "0.7.2"
|
||||
@@ -2071,6 +2093,15 @@ dependencies = [
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "dlib"
|
||||
version = "0.5.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ab8ecd87370524b461f8557c119c405552c396ed91fc0a8eec68679eab26f94a"
|
||||
dependencies = [
|
||||
"libloading 0.8.9",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "dlopen2"
|
||||
version = "0.8.2"
|
||||
@@ -2133,6 +2164,12 @@ version = "0.15.7"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b"
|
||||
|
||||
[[package]]
|
||||
name = "downcast-rs"
|
||||
version = "1.2.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2"
|
||||
|
||||
[[package]]
|
||||
name = "download-cef"
|
||||
version = "2.3.2"
|
||||
@@ -6126,6 +6163,12 @@ dependencies = [
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pollster"
|
||||
version = "0.4.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2f3a9f18d041e6d0e102a0a46750538147e5e8992d3b4873aaafee2520b00ce3"
|
||||
|
||||
[[package]]
|
||||
name = "poly1305"
|
||||
version = "0.8.0"
|
||||
@@ -6436,6 +6479,15 @@ dependencies = [
|
||||
"memchr",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "quick-xml"
|
||||
version = "0.39.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "cdcc8dd4e2f670d309a5f0e83fe36dfdc05af317008fea29144da1a2ac858e5e"
|
||||
dependencies = [
|
||||
"memchr",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "quinn"
|
||||
version = "0.11.9"
|
||||
@@ -6885,6 +6937,30 @@ dependencies = [
|
||||
"subtle",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "rfd"
|
||||
version = "0.15.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ef2bee61e6cffa4635c72d7d81a84294e28f0930db0ddcb0f66d10244674ebed"
|
||||
dependencies = [
|
||||
"ashpd",
|
||||
"block2 0.6.2",
|
||||
"dispatch2",
|
||||
"js-sys",
|
||||
"log",
|
||||
"objc2 0.6.4",
|
||||
"objc2-app-kit 0.3.2",
|
||||
"objc2-core-foundation",
|
||||
"objc2-foundation 0.3.2",
|
||||
"pollster",
|
||||
"raw-window-handle",
|
||||
"urlencoding",
|
||||
"wasm-bindgen",
|
||||
"wasm-bindgen-futures",
|
||||
"web-sys",
|
||||
"windows-sys 0.59.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "rgb"
|
||||
version = "0.8.53"
|
||||
@@ -7206,6 +7282,12 @@ dependencies = [
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "scoped-tls"
|
||||
version = "1.0.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294"
|
||||
|
||||
[[package]]
|
||||
name = "scopeguard"
|
||||
version = "1.2.0"
|
||||
@@ -8877,6 +8959,7 @@ dependencies = [
|
||||
"signal-hook-registry",
|
||||
"socket2",
|
||||
"tokio-macros",
|
||||
"tracing",
|
||||
"windows-sys 0.61.2",
|
||||
]
|
||||
|
||||
@@ -9899,6 +9982,66 @@ dependencies = [
|
||||
"semver",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "wayland-backend"
|
||||
version = "0.3.15"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2857dd20b54e916ec7253b3d6b4d5c4d7d4ca2c33c2e11c6c76a99bd8744755d"
|
||||
dependencies = [
|
||||
"cc",
|
||||
"downcast-rs",
|
||||
"rustix",
|
||||
"scoped-tls",
|
||||
"smallvec",
|
||||
"wayland-sys",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "wayland-client"
|
||||
version = "0.31.14"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "645c7c96bb74690c3189b5c9cb4ca1627062bb23693a4fad9d8c3de958260144"
|
||||
dependencies = [
|
||||
"bitflags 2.11.1",
|
||||
"rustix",
|
||||
"wayland-backend",
|
||||
"wayland-scanner",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "wayland-protocols"
|
||||
version = "0.32.13"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "23d0c813de3daa2ed6520af85a3bd49b0e722a3078506899aa9686fea58dc4b6"
|
||||
dependencies = [
|
||||
"bitflags 2.11.1",
|
||||
"wayland-backend",
|
||||
"wayland-client",
|
||||
"wayland-scanner",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "wayland-scanner"
|
||||
version = "0.31.10"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9c324a910fd86ebdc364a3e61ec1f11737d3b1d6c273c0239ee8ff4bc0d24b4a"
|
||||
dependencies = [
|
||||
"proc-macro2",
|
||||
"quick-xml 0.39.4",
|
||||
"quote",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "wayland-sys"
|
||||
version = "0.31.11"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "d8eab23fefc9e41f8e841df4a9c707e8a8c4ed26e944ef69297184de2785e3be"
|
||||
dependencies = [
|
||||
"dlib",
|
||||
"log",
|
||||
"pkg-config",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "web-sys"
|
||||
version = "0.3.98"
|
||||
@@ -11108,6 +11251,7 @@ dependencies = [
|
||||
"rustix",
|
||||
"serde",
|
||||
"serde_repr",
|
||||
"tokio",
|
||||
"tracing",
|
||||
"uds_windows",
|
||||
"uuid 1.23.1",
|
||||
@@ -11358,6 +11502,7 @@ dependencies = [
|
||||
"endi",
|
||||
"enumflags2",
|
||||
"serde",
|
||||
"url",
|
||||
"winnow 1.0.2",
|
||||
"zvariant_derive",
|
||||
"zvariant_utils",
|
||||
|
||||
@@ -63,6 +63,10 @@ serde = { version = "1", features = ["derive"] }
|
||||
serde_json = "1"
|
||||
toml = "0.8"
|
||||
directories = "5"
|
||||
# Native Save-As file dialog for artifact export (#3162). default-features
|
||||
# off + xdg-portal keeps Linux off GTK (the Downloads-copy fallback covers
|
||||
# headless CI where no portal is available); tokio drives the async backend.
|
||||
rfd = { version = "0.15", default-features = false, features = ["xdg-portal", "tokio"] }
|
||||
# Used by gmail/cdp_fetch for decoding binary IO.read chunks. Base64 is
|
||||
# only emitted by CDP IO.read when the stream contains non-UTF-8 bytes,
|
||||
# but we opt into the feature to stay robust against unexpected responses.
|
||||
|
||||
@@ -32,6 +32,7 @@
|
||||
"allow-core-process",
|
||||
"allow-workspace-files",
|
||||
"allow-artifact-download",
|
||||
"allow-artifact-save",
|
||||
"allow-app-update",
|
||||
"allow-loopback-oauth"
|
||||
]
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
[[permission]]
|
||||
identifier = "allow-artifact-save"
|
||||
description = "Allow exporting an agent-generated artifact to a user-chosen path via the native Save-As dialog (#3162)"
|
||||
|
||||
[permission.commands]
|
||||
|
||||
allow = [
|
||||
"save_artifact_via_dialog",
|
||||
]
|
||||
|
||||
deny = []
|
||||
@@ -1,30 +1,129 @@
|
||||
//! Tauri command for downloading agent-generated artifacts (#2779).
|
||||
//! Tauri commands for exporting agent-generated artifacts (#2779, #3162).
|
||||
//!
|
||||
//! Contract: the frontend resolves the artifact's absolute source
|
||||
//! path via the existing `openhuman.ai_get_artifact` core RPC, then
|
||||
//! invokes [`download_artifact_to_downloads`] with that source path
|
||||
//! plus a filename hint. The command:
|
||||
//! Two export paths, both fed by the frontend resolving an artifact's
|
||||
//! absolute source path via the `openhuman.ai_get_artifact` core RPC:
|
||||
//!
|
||||
//! 1. Validates both inputs (no path traversal in the filename, source
|
||||
//! must be absolute + on disk).
|
||||
//! 2. Resolves the user's Downloads directory via the `dirs` crate.
|
||||
//! 3. Picks a non-colliding destination filename — `name.pptx`,
|
||||
//! `name (1).pptx`, `name (2).pptx`, …
|
||||
//! 4. Copies source → dest with `tokio::fs::copy`.
|
||||
//! 5. Returns the absolute dest path so the frontend can show a
|
||||
//! "Saved to …" toast with a "Reveal in Finder" button (the
|
||||
//! `opener:allow-reveal-item-in-dir` capability is already wired).
|
||||
//! 1. [`save_artifact_via_dialog`] (#3162) — opens a native Save-As
|
||||
//! dialog (macOS / Windows / Linux) pre-filled with the artifact's
|
||||
//! filename and copies the bytes to the user-chosen destination.
|
||||
//! Returns `Ok(None)` when the user cancels. Backed by the `rfd`
|
||||
//! crate, which talks to the OS dialog APIs directly and does NOT
|
||||
//! pull `tauri-plugin-fs` (whose `schemars` version conflict was the
|
||||
//! reason the original #2779 work shipped the Downloads fallback
|
||||
//! below instead of a dialog).
|
||||
//! 2. [`download_artifact_to_downloads`] (#2779) — copies the artifact
|
||||
//! into the user's Downloads directory with a non-colliding name and
|
||||
//! returns the dest path so the UI can offer "Reveal in Finder".
|
||||
//! Retained as the fallback the frontend uses when the dialog is
|
||||
//! unavailable (e.g. no portal on headless Linux) or the user cancels.
|
||||
//! Cross-platform — previously macOS/Linux-only, un-gated so the
|
||||
//! Save-As fallback works on Windows too.
|
||||
//!
|
||||
//! Why Downloads instead of a native save-file dialog: the
|
||||
//! `tauri-plugin-dialog` crate pulls `tauri-plugin-fs` transitively,
|
||||
//! which currently breaks the openhuman build with a `schemars`
|
||||
//! version conflict. The Downloads + reveal pattern satisfies the
|
||||
//! "user-chosen destination" intent of issue #2779 AC#3 without
|
||||
//! widening the Tauri allow-list, and matches what most desktop chat
|
||||
//! apps do for downloaded attachments.
|
||||
//! Both validate that the source is an existing file inside the
|
||||
//! OpenHuman data dir's `artifacts/` tree, and sanitize the filename
|
||||
//! hint, so the renderer can never copy an arbitrary local file out nor
|
||||
//! write outside the chosen directory.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// Open a native Save-As dialog pre-filled with `suggested_filename` and
|
||||
/// copy the artifact at `source_path` to the chosen destination (#3162).
|
||||
///
|
||||
/// Returns:
|
||||
/// - `Ok(Some(dest))` — the absolute path the user saved to.
|
||||
/// - `Ok(None)` — the user dismissed the dialog (not an error; the
|
||||
/// frontend simply stops).
|
||||
/// - `Err(_)` — bad inputs or a copy failure; the frontend falls back to
|
||||
/// [`download_artifact_to_downloads`] where available.
|
||||
#[tauri::command]
|
||||
pub async fn save_artifact_via_dialog(
|
||||
source_path: String,
|
||||
suggested_filename: String,
|
||||
) -> Result<Option<String>, String> {
|
||||
let source = validate_source(&source_path)?;
|
||||
let sanitized = sanitize_filename(&suggested_filename)?;
|
||||
|
||||
// `rfd` drives the OS-native dialog. On Linux this is the xdg-desktop
|
||||
// portal (no GTK link); on macOS/Windows the system panel. The await
|
||||
// resolves when the user picks a path or cancels.
|
||||
let handle = rfd::AsyncFileDialog::new()
|
||||
.set_file_name(&sanitized)
|
||||
.save_file()
|
||||
.await;
|
||||
|
||||
let Some(file) = handle else {
|
||||
log::info!("[artifact_commands] save_artifact_via_dialog cancelled by user");
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let dest = file.path().to_path_buf();
|
||||
let bytes = copy_to_path(&source, &dest).await?;
|
||||
log::info!(
|
||||
"[artifact_commands] save_artifact_via_dialog bytes={bytes} dest={}",
|
||||
dest.display()
|
||||
);
|
||||
Ok(Some(dest.display().to_string()))
|
||||
}
|
||||
|
||||
/// Validate a renderer-supplied source path: must be a non-empty,
|
||||
/// absolute path that exists on disk AND resolve inside the OpenHuman
|
||||
/// data directory's `artifacts/` tree. The path always originates from
|
||||
/// the core `ai_get_artifact` RPC's `absolute_path`, but the command is
|
||||
/// reachable by the renderer directly, so we re-validate the trust
|
||||
/// boundary here — without the artifacts-root check a compromised
|
||||
/// renderer could copy any readable local file out through the Save-As
|
||||
/// dialog under an artifact-looking name (Codex P2).
|
||||
fn validate_source(source_path: &str) -> Result<PathBuf, String> {
|
||||
if source_path.trim().is_empty() {
|
||||
return Err("source_path must not be empty".to_string());
|
||||
}
|
||||
let source = PathBuf::from(source_path);
|
||||
if !source.is_absolute() {
|
||||
return Err(format!(
|
||||
"source_path must be absolute (came from ai_get_artifact): {source_path:?}"
|
||||
));
|
||||
}
|
||||
if !source.is_file() {
|
||||
return Err(format!(
|
||||
"artifact source not present on disk: {source_path}"
|
||||
));
|
||||
}
|
||||
let root = crate::cef_profile::default_root_openhuman_dir()?;
|
||||
assert_artifact_source(&source, &root)?;
|
||||
Ok(source)
|
||||
}
|
||||
|
||||
/// Confirm `source` resolves inside `root` (the OpenHuman data dir) and
|
||||
/// carries an `artifacts` path component — i.e. it is a workspace
|
||||
/// artifact, not an arbitrary local file. Canonicalizes both sides so
|
||||
/// symlink trickery can't escape the root. Isolated for unit testing
|
||||
/// without touching the real home directory.
|
||||
fn assert_artifact_source(source: &Path, root: &Path) -> Result<(), String> {
|
||||
let canon_source = source
|
||||
.canonicalize()
|
||||
.map_err(|e| format!("cannot resolve source path: {e}"))?;
|
||||
let canon_root = root.canonicalize().unwrap_or_else(|_| root.to_path_buf());
|
||||
if !canon_source.starts_with(&canon_root) {
|
||||
return Err("source must be inside the OpenHuman data directory".to_string());
|
||||
}
|
||||
if !canon_source
|
||||
.components()
|
||||
.any(|c| c.as_os_str() == "artifacts")
|
||||
{
|
||||
return Err("source must be a workspace artifact file".to_string());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Copy `source` to `dest`, returning the byte count. Shared by the
|
||||
/// Save-As dialog flow; isolated so it is unit-testable without driving
|
||||
/// a real OS dialog.
|
||||
async fn copy_to_path(source: &Path, dest: &Path) -> Result<u64, String> {
|
||||
tokio::fs::copy(source, dest)
|
||||
.await
|
||||
.map_err(|e| format!("failed to copy artifact to {:?}: {e}", dest))
|
||||
}
|
||||
|
||||
/// Maximum number of `(N)` suffixes we'll append when picking a
|
||||
/// non-colliding filename. After 1000 we give up and append a UUID
|
||||
/// suffix instead so the download never silently overwrites.
|
||||
@@ -35,23 +134,10 @@ pub async fn download_artifact_to_downloads(
|
||||
source_path: String,
|
||||
filename: String,
|
||||
) -> Result<String, String> {
|
||||
if source_path.trim().is_empty() {
|
||||
return Err("source_path must not be empty".to_string());
|
||||
}
|
||||
let source = validate_source(&source_path)?;
|
||||
if filename.trim().is_empty() {
|
||||
return Err("filename must not be empty".to_string());
|
||||
}
|
||||
let source = PathBuf::from(&source_path);
|
||||
if !source.is_absolute() {
|
||||
return Err(format!(
|
||||
"source_path must be absolute (came from ai_get_artifact): {source_path:?}"
|
||||
));
|
||||
}
|
||||
if !source.is_file() {
|
||||
return Err(format!(
|
||||
"artifact source not present on disk: {source_path}"
|
||||
));
|
||||
}
|
||||
let sanitized = sanitize_filename(&filename)?;
|
||||
|
||||
let downloads = directories::UserDirs::new()
|
||||
@@ -62,9 +148,7 @@ pub async fn download_artifact_to_downloads(
|
||||
.map_err(|e| format!("failed to ensure Downloads dir {:?}: {e}", downloads))?;
|
||||
|
||||
let dest = pick_unique_path(&downloads, &sanitized);
|
||||
let bytes = tokio::fs::copy(&source, &dest)
|
||||
.await
|
||||
.map_err(|e| format!("failed to copy artifact to {:?}: {e}", dest))?;
|
||||
let bytes = copy_to_path(&source, &dest).await?;
|
||||
|
||||
log::info!(
|
||||
"[artifact_commands] download_artifact_to_downloads bytes={bytes} dest={}",
|
||||
@@ -76,7 +160,7 @@ pub async fn download_artifact_to_downloads(
|
||||
/// Strip path-traversal characters from a filename hint. The
|
||||
/// renderer is expected to pass something like `"My Deck.pptx"`;
|
||||
/// reject anything that contains a separator or null byte so a
|
||||
/// malicious `ai_get_artifact` response can never escape Downloads.
|
||||
/// malicious `ai_get_artifact` response can never escape the chosen dir.
|
||||
fn sanitize_filename(name: &str) -> Result<String, String> {
|
||||
let trimmed = name.trim();
|
||||
if trimmed.is_empty() {
|
||||
@@ -166,6 +250,74 @@ mod tests {
|
||||
assert_eq!(sanitize_filename(" trim me ").unwrap(), "trim me");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_source_rejects_relative_and_empty() {
|
||||
assert!(validate_source("").is_err());
|
||||
assert!(validate_source("relative/path.pptx").is_err());
|
||||
assert!(validate_source("/definitely/not/here.pptx").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assert_artifact_source_accepts_file_under_artifacts_root() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let root = temp.path();
|
||||
let art = root.join("users/u1/workspace/artifacts/a-1");
|
||||
std::fs::create_dir_all(&art).unwrap();
|
||||
let file = art.join("deck.pptx");
|
||||
std::fs::write(&file, b"x").unwrap();
|
||||
assert!(assert_artifact_source(&file, root).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assert_artifact_source_rejects_file_without_artifacts_component() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let root = temp.path();
|
||||
let other = root.join("users/u1/secrets");
|
||||
std::fs::create_dir_all(&other).unwrap();
|
||||
let file = other.join("token.txt");
|
||||
std::fs::write(&file, b"x").unwrap();
|
||||
assert!(assert_artifact_source(&file, root).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn assert_artifact_source_rejects_file_outside_root() {
|
||||
let root_dir = tempfile::tempdir().unwrap();
|
||||
let outside_dir = tempfile::tempdir().unwrap();
|
||||
// Even with an `artifacts` segment, a path outside the root is denied.
|
||||
let art = outside_dir.path().join("artifacts");
|
||||
std::fs::create_dir_all(&art).unwrap();
|
||||
let file = art.join("evil.pptx");
|
||||
std::fs::write(&file, b"x").unwrap();
|
||||
assert!(assert_artifact_source(&file, root_dir.path()).is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn copy_to_path_copies_bytes() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let src = temp.path().join("src.pptx");
|
||||
let dst = temp.path().join("dst.pptx");
|
||||
std::fs::write(&src, b"deck-bytes").unwrap();
|
||||
let n = copy_to_path(&src, &dst).await.unwrap();
|
||||
assert_eq!(n, b"deck-bytes".len() as u64);
|
||||
assert_eq!(std::fs::read(&dst).unwrap(), b"deck-bytes");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn save_via_dialog_rejects_bad_source() {
|
||||
// Validation runs before any dialog is shown, so these resolve
|
||||
// without user interaction.
|
||||
assert!(
|
||||
save_artifact_via_dialog(String::new(), "x.pptx".to_string())
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
assert!(
|
||||
save_artifact_via_dialog("relative".to_string(), "x.pptx".to_string())
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_stem_ext_pairs() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -4,7 +4,8 @@
|
||||
compile_error!("src-tauri host supports desktop (Windows/macOS/Linux) only. Mobile lives in app/src-tauri-mobile.");
|
||||
|
||||
mod app_update;
|
||||
#[cfg(any(target_os = "macos", target_os = "linux"))]
|
||||
// Artifact export commands (#2779, #3162) — both cross-platform
|
||||
// (macOS/Windows/Linux): native Save-As dialog (rfd) + Downloads copy.
|
||||
mod artifact_commands;
|
||||
mod cdp;
|
||||
// macOS/Linux only: depends on the `nix` crate (a `cfg(unix)` dependency) and
|
||||
@@ -3611,11 +3612,11 @@ pub fn run() {
|
||||
core_rpc::relay_http_rpc,
|
||||
overlay_parent_rpc_url,
|
||||
process_diagnostics_list_owned,
|
||||
// `mod artifact_commands;` is `#[cfg(any(target_os = "macos", target_os = "linux"))]`
|
||||
// (Downloads-dir + `tokio::fs::copy` flow is non-Windows-only today).
|
||||
// The handler entry MUST carry the same gate or Windows builds fail
|
||||
// with "function not found in scope" (CR #3328947313 on PR #3026).
|
||||
#[cfg(any(target_os = "macos", target_os = "linux"))]
|
||||
// Artifact export commands — both cross-platform (#3162). The
|
||||
// Downloads command was previously macOS/Linux-gated, but the
|
||||
// `directories` + `tokio::fs::copy` flow compiles on Windows too,
|
||||
// and the Save-As fallback needs it there (CodeRabbit on #4127).
|
||||
artifact_commands::save_artifact_via_dialog,
|
||||
artifact_commands::download_artifact_to_downloads,
|
||||
check_core_update,
|
||||
apply_core_update,
|
||||
|
||||
@@ -6,10 +6,10 @@ import ArtifactCard from './ArtifactCard';
|
||||
|
||||
// Mock the artifact download service — the card only consumes the
|
||||
// two public functions, so a per-test override is enough.
|
||||
const downloadArtifactMock = vi.fn();
|
||||
const saveArtifactViaDialogMock = vi.fn();
|
||||
const revealArtifactInFileManagerMock = vi.fn();
|
||||
vi.mock('../../services/artifactDownloadService', () => ({
|
||||
downloadArtifact: (...args: unknown[]) => downloadArtifactMock(...args),
|
||||
saveArtifactViaDialog: (...args: unknown[]) => saveArtifactViaDialogMock(...args),
|
||||
revealArtifactInFileManager: (...args: unknown[]) => revealArtifactInFileManagerMock(...args),
|
||||
}));
|
||||
|
||||
@@ -27,7 +27,7 @@ function snapshot(overrides: Partial<ArtifactSnapshot> = {}): ArtifactSnapshot {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
downloadArtifactMock.mockReset();
|
||||
saveArtifactViaDialogMock.mockReset();
|
||||
revealArtifactInFileManagerMock.mockReset();
|
||||
});
|
||||
|
||||
@@ -82,7 +82,7 @@ describe('<ArtifactCard /> — ready state', () => {
|
||||
});
|
||||
|
||||
it('drives the download → done flow and reveals on click', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
path: '/Users/me/Downloads/Quarterly Deck.pptx',
|
||||
});
|
||||
@@ -94,8 +94,8 @@ describe('<ArtifactCard /> — ready state', () => {
|
||||
|
||||
// Service called with (id, title, ext) where ext comes from the
|
||||
// title's extension when present, or kind fallback otherwise.
|
||||
await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1));
|
||||
expect(downloadArtifactMock).toHaveBeenCalledWith('a-1', 'Quarterly Deck', 'pptx');
|
||||
await waitFor(() => expect(saveArtifactViaDialogMock).toHaveBeenCalledTimes(1));
|
||||
expect(saveArtifactViaDialogMock).toHaveBeenCalledWith('a-1', 'Quarterly Deck', 'pptx');
|
||||
|
||||
// Saved-to row appears with the reveal button.
|
||||
await screen.findByText(
|
||||
@@ -113,40 +113,40 @@ describe('<ArtifactCard /> — ready state', () => {
|
||||
});
|
||||
|
||||
it('falls back to the kind-based extension when title has no dot', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Doc.pdf' });
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Doc.pdf' });
|
||||
render(
|
||||
<ArtifactCard artifact={snapshot({ artifactId: 'a-2', kind: 'document', title: 'Doc' })} />
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1));
|
||||
expect(downloadArtifactMock).toHaveBeenCalledWith('a-2', 'Doc', 'pdf');
|
||||
await waitFor(() => expect(saveArtifactViaDialogMock).toHaveBeenCalledTimes(1));
|
||||
expect(saveArtifactViaDialogMock).toHaveBeenCalledWith('a-2', 'Doc', 'pdf');
|
||||
});
|
||||
|
||||
it('falls back to png for image kind without an extension', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Pic.png' });
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Pic.png' });
|
||||
render(
|
||||
<ArtifactCard artifact={snapshot({ artifactId: 'a-3', kind: 'image', title: 'Pic' })} />
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1));
|
||||
expect(downloadArtifactMock).toHaveBeenCalledWith('a-3', 'Pic', 'png');
|
||||
await waitFor(() => expect(saveArtifactViaDialogMock).toHaveBeenCalledTimes(1));
|
||||
expect(saveArtifactViaDialogMock).toHaveBeenCalledWith('a-3', 'Pic', 'png');
|
||||
});
|
||||
|
||||
it('falls back to bin for the "other" kind without an extension', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Blob.bin' });
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Blob.bin' });
|
||||
render(
|
||||
<ArtifactCard artifact={snapshot({ artifactId: 'a-4', kind: 'other', title: 'Blob' })} />
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1));
|
||||
expect(downloadArtifactMock).toHaveBeenCalledWith('a-4', 'Blob', 'bin');
|
||||
await waitFor(() => expect(saveArtifactViaDialogMock).toHaveBeenCalledTimes(1));
|
||||
expect(saveArtifactViaDialogMock).toHaveBeenCalledWith('a-4', 'Blob', 'bin');
|
||||
});
|
||||
|
||||
it('uses the existing extension on the title when present', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/notes.txt' });
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({ ok: true, path: '/tmp/notes.txt' });
|
||||
render(
|
||||
<ArtifactCard
|
||||
artifact={snapshot({ artifactId: 'a-5', kind: 'document', title: 'notes.txt' })}
|
||||
@@ -154,12 +154,12 @@ describe('<ArtifactCard /> — ready state', () => {
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1));
|
||||
expect(downloadArtifactMock).toHaveBeenCalledWith('a-5', 'notes.txt', 'txt');
|
||||
await waitFor(() => expect(saveArtifactViaDialogMock).toHaveBeenCalledTimes(1));
|
||||
expect(saveArtifactViaDialogMock).toHaveBeenCalledWith('a-5', 'notes.txt', 'txt');
|
||||
});
|
||||
|
||||
it('surfaces the service error message when download fails', async () => {
|
||||
downloadArtifactMock.mockResolvedValueOnce({ ok: false, error: 'disk full' });
|
||||
saveArtifactViaDialogMock.mockResolvedValueOnce({ ok: false, error: 'disk full' });
|
||||
|
||||
render(<ArtifactCard artifact={snapshot()} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
|
||||
@@ -3,8 +3,8 @@ import { useState } from 'react';
|
||||
import { formatFileSize } from '../../lib/attachments';
|
||||
import { useT } from '../../lib/i18n/I18nContext';
|
||||
import {
|
||||
downloadArtifact,
|
||||
revealArtifactInFileManager,
|
||||
saveArtifactViaDialog,
|
||||
} from '../../services/artifactDownloadService';
|
||||
import type { ArtifactSnapshot } from '../../store/chatRuntimeSlice';
|
||||
|
||||
@@ -170,9 +170,12 @@ export default function ArtifactCard({ artifact, onRetry }: ArtifactCardProps) {
|
||||
const handleDownload = async () => {
|
||||
setDownload({ state: 'downloading' });
|
||||
const ext = extensionFor(artifact.kind, artifact.title);
|
||||
const outcome = await downloadArtifact(artifact.artifactId, artifact.title, ext);
|
||||
const outcome = await saveArtifactViaDialog(artifact.artifactId, artifact.title, ext);
|
||||
if (outcome.ok) {
|
||||
setDownload({ state: 'done', path: outcome.path });
|
||||
} else if (outcome.code === 'CANCELLED') {
|
||||
// User dismissed the Save-As dialog — quietly return to idle.
|
||||
setDownload({ state: 'idle' });
|
||||
} else {
|
||||
setDownload({ state: 'error', error: outcome.error });
|
||||
}
|
||||
|
||||
@@ -63,6 +63,10 @@ function localizeErrorCode(
|
||||
return t('chat.files.error.download_failed');
|
||||
case 'DELETE_FAILED':
|
||||
return t('chat.files.error.delete_failed');
|
||||
case 'CANCELLED':
|
||||
// User-initiated dialog dismissal — not a real error. Callers treat
|
||||
// it as a no-op; surface nothing (fall back to raw text if passed).
|
||||
return fallback ?? '';
|
||||
default: {
|
||||
// Exhaustive guard: a new code added to ArtifactErrorCode without a
|
||||
// matching arm here will fail to type-check.
|
||||
|
||||
@@ -2,14 +2,14 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import {
|
||||
downloadArtifact,
|
||||
revealArtifactInFileManager,
|
||||
saveArtifactViaDialog,
|
||||
} from '../../../services/artifactDownloadService';
|
||||
import type { ArtifactSnapshot } from '../../../store/chatRuntimeSlice';
|
||||
import ArtifactCard from '../ArtifactCard';
|
||||
|
||||
vi.mock('../../../services/artifactDownloadService', () => ({
|
||||
downloadArtifact: vi.fn(),
|
||||
saveArtifactViaDialog: vi.fn(),
|
||||
revealArtifactInFileManager: vi.fn(),
|
||||
}));
|
||||
|
||||
@@ -75,15 +75,15 @@ describe('ArtifactCard', () => {
|
||||
expect(screen.getByRole('button', { name: 'Download' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('on Download click → calls downloadArtifact with title-derived extension on success', async () => {
|
||||
vi.mocked(downloadArtifact).mockResolvedValueOnce({
|
||||
it('on Download click → calls saveArtifactViaDialog with title-derived extension on success', async () => {
|
||||
vi.mocked(saveArtifactViaDialog).mockResolvedValueOnce({
|
||||
ok: true,
|
||||
path: '/Users/me/Downloads/Climate Deck.pptx',
|
||||
});
|
||||
render(<ArtifactCard artifact={ready({ title: 'climate-deck.pptx' })} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => {
|
||||
expect(downloadArtifact).toHaveBeenCalledWith('art-1', 'climate-deck.pptx', 'pptx');
|
||||
expect(saveArtifactViaDialog).toHaveBeenCalledWith('art-1', 'climate-deck.pptx', 'pptx');
|
||||
});
|
||||
// Saved-to label appears with the resolved path
|
||||
await waitFor(() => {
|
||||
@@ -95,7 +95,7 @@ describe('ArtifactCard', () => {
|
||||
});
|
||||
|
||||
it('on Reveal click → calls revealArtifactInFileManager with the saved path', async () => {
|
||||
vi.mocked(downloadArtifact).mockResolvedValueOnce({
|
||||
vi.mocked(saveArtifactViaDialog).mockResolvedValueOnce({
|
||||
ok: true,
|
||||
path: '/Users/me/Downloads/Climate Deck.pptx',
|
||||
});
|
||||
@@ -113,7 +113,7 @@ describe('ArtifactCard', () => {
|
||||
});
|
||||
|
||||
it('on Download failure → surfaces the error reason and leaves the Download button in place', async () => {
|
||||
vi.mocked(downloadArtifact).mockResolvedValueOnce({
|
||||
vi.mocked(saveArtifactViaDialog).mockResolvedValueOnce({
|
||||
ok: false,
|
||||
code: 'NOT_DESKTOP',
|
||||
error: 'Downloads are only available in the desktop app',
|
||||
@@ -137,27 +137,27 @@ describe('ArtifactCard', () => {
|
||||
])(
|
||||
'falls back to per-kind extension when title lacks one (kind=%s → ext=%s)',
|
||||
async (kind, expectedExt) => {
|
||||
vi.mocked(downloadArtifact).mockResolvedValueOnce({ ok: true, path: '/d/x' });
|
||||
vi.mocked(saveArtifactViaDialog).mockResolvedValueOnce({ ok: true, path: '/d/x' });
|
||||
render(<ArtifactCard artifact={ready({ kind, title: 'no-extension' })} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => {
|
||||
expect(downloadArtifact).toHaveBeenCalledWith('art-1', 'no-extension', expectedExt);
|
||||
expect(saveArtifactViaDialog).toHaveBeenCalledWith('art-1', 'no-extension', expectedExt);
|
||||
});
|
||||
}
|
||||
);
|
||||
|
||||
it('treats a trailing-dot title as having no extension (falls through to kind default)', async () => {
|
||||
vi.mocked(downloadArtifact).mockResolvedValueOnce({ ok: true, path: '/d/x' });
|
||||
vi.mocked(saveArtifactViaDialog).mockResolvedValueOnce({ ok: true, path: '/d/x' });
|
||||
render(<ArtifactCard artifact={ready({ kind: 'presentation', title: 'trailing.' })} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Download' }));
|
||||
await waitFor(() => {
|
||||
expect(downloadArtifact).toHaveBeenCalledWith('art-1', 'trailing.', 'pptx');
|
||||
expect(saveArtifactViaDialog).toHaveBeenCalledWith('art-1', 'trailing.', 'pptx');
|
||||
});
|
||||
});
|
||||
|
||||
it('Download button is disabled while a download is in flight', async () => {
|
||||
let resolveDownload: (v: { ok: true; path: string }) => void = () => {};
|
||||
vi.mocked(downloadArtifact).mockImplementationOnce(
|
||||
vi.mocked(saveArtifactViaDialog).mockImplementationOnce(
|
||||
() =>
|
||||
new Promise(r => {
|
||||
resolveDownload = r;
|
||||
|
||||
@@ -36,7 +36,13 @@ import { trackEvent } from '../services/analytics';
|
||||
import { applyOpenRouterFreeModels } from '../services/api/openrouterFreeModels';
|
||||
import { subagentApi } from '../services/api/subagentApi';
|
||||
import { threadApi } from '../services/api/threadApi';
|
||||
import { chatCancel, chatClearQueue, chatSend, useRustChat } from '../services/chatService';
|
||||
import {
|
||||
aiRegenerate,
|
||||
chatCancel,
|
||||
chatClearQueue,
|
||||
chatSend,
|
||||
useRustChat,
|
||||
} from '../services/chatService';
|
||||
import { callCoreRpc } from '../services/coreRpcClient';
|
||||
import {
|
||||
loadAgentProfiles,
|
||||
@@ -2670,12 +2676,11 @@ const Conversations = ({
|
||||
// chat scroll area isn't permanently occupied — restored decks
|
||||
// are listable from the chip on demand.
|
||||
//
|
||||
// NOTE: `onRetry` is intentionally omitted on `ArtifactCard`
|
||||
// below — real retry (either `removeArtifact(thread, id)` to
|
||||
// let the user re-prompt, or full re-dispatch of the producing
|
||||
// tool call) is tracked in follow-up issue #3162. The
|
||||
// failed-card UI still surfaces the truncated error reason;
|
||||
// the button just stays hidden until #3162 lands.
|
||||
// The failed-card Retry button re-dispatches the producing tool
|
||||
// via `ai_regenerate` (#3162): the core reloads the persisted
|
||||
// creation args and re-runs generation under the original
|
||||
// artifact id, so the card swaps back to a spinner in place and
|
||||
// then to ready/failed via the socket events.
|
||||
const artifactThreadId = selectedThreadId ?? firstActiveThreadId;
|
||||
const all = artifactThreadId ? (artifactsByThread[artifactThreadId] ?? []) : [];
|
||||
const live = all.filter(a => a.status !== 'ready');
|
||||
@@ -2683,7 +2688,19 @@ const Conversations = ({
|
||||
return (
|
||||
<div className="mb-2 flex flex-col gap-2">
|
||||
{live.map(artifact => (
|
||||
<ArtifactCard key={artifact.artifactId} artifact={artifact} />
|
||||
<ArtifactCard
|
||||
key={artifact.artifactId}
|
||||
artifact={artifact}
|
||||
onRetry={
|
||||
artifactThreadId
|
||||
? id => {
|
||||
void aiRegenerate(id, artifactThreadId).catch(err => {
|
||||
console.warn('[artifact] regenerate failed:', err);
|
||||
});
|
||||
}
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -48,6 +48,7 @@ import {
|
||||
type ToolTimelineEntry,
|
||||
type ToolTimelineEntryStatus,
|
||||
upsertArtifactFailedForThread,
|
||||
upsertArtifactInProgressForThread,
|
||||
upsertArtifactReadyForThread,
|
||||
} from '../store/chatRuntimeSlice';
|
||||
import { useAppDispatch, useAppSelector } from '../store/hooks';
|
||||
@@ -964,6 +965,21 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
|
||||
}
|
||||
});
|
||||
},
|
||||
onArtifactPending: event => {
|
||||
rtLog('artifact_pending', {
|
||||
thread: event.thread_id,
|
||||
artifact_id: event.artifact_id,
|
||||
kind: event.kind,
|
||||
});
|
||||
dispatch(
|
||||
upsertArtifactInProgressForThread({
|
||||
threadId: event.thread_id,
|
||||
artifactId: event.artifact_id,
|
||||
kind: event.kind,
|
||||
title: event.title,
|
||||
})
|
||||
);
|
||||
},
|
||||
onArtifactReady: event => {
|
||||
rtLog('artifact_ready', {
|
||||
thread: event.thread_id,
|
||||
|
||||
@@ -98,6 +98,30 @@ describe('ChatRuntimeProvider — artifact event dispatch (#2779)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('onArtifactPending upserts an in_progress snapshot keyed on the artifact id (#3162)', () => {
|
||||
const listeners = renderProvider();
|
||||
|
||||
act(() => {
|
||||
listeners.onArtifactPending?.({
|
||||
thread_id: 'thread-1',
|
||||
artifact_id: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
workspace_dir: '/workspace',
|
||||
path: 'a-1/deck.pptx',
|
||||
});
|
||||
});
|
||||
|
||||
const bucket = store.getState().chatRuntime.artifactsByThread['thread-1'];
|
||||
expect(bucket).toHaveLength(1);
|
||||
expect(bucket?.[0]).toMatchObject({
|
||||
artifactId: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
status: 'in_progress',
|
||||
});
|
||||
});
|
||||
|
||||
it('onArtifactFailed records the producer-supplied error', () => {
|
||||
const listeners = renderProvider();
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
deleteArtifact,
|
||||
downloadArtifact,
|
||||
revealArtifactInFileManager,
|
||||
saveArtifactViaDialog,
|
||||
} from '../artifactDownloadService';
|
||||
import { callCoreRpc } from '../coreRpcClient';
|
||||
|
||||
@@ -297,3 +298,75 @@ describe('revealArtifactInFileManager', () => {
|
||||
warn.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveArtifactViaDialog (#3162)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
hoisted.isTauri.mockReturnValue(true);
|
||||
});
|
||||
|
||||
const resolveOk = () =>
|
||||
vi
|
||||
.mocked(callCoreRpc)
|
||||
.mockResolvedValueOnce({
|
||||
absolute_path: '/ws/artifacts/a-1/deck.pptx',
|
||||
meta: { id: 'a-1', title: 'Deck' },
|
||||
} as never);
|
||||
|
||||
it('returns NOT_DESKTOP outside Tauri', async () => {
|
||||
hoisted.isTauri.mockReturnValueOnce(false);
|
||||
const outcome = await saveArtifactViaDialog('a-1', 'Deck', 'pptx');
|
||||
expect(outcome).toEqual({
|
||||
ok: false,
|
||||
code: 'NOT_DESKTOP',
|
||||
error: expect.stringContaining('desktop'),
|
||||
});
|
||||
expect(callCoreRpc).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('saves to the user-chosen path and returns it', async () => {
|
||||
resolveOk();
|
||||
hoisted.invoke.mockResolvedValueOnce('/Users/me/Desktop/Deck.pptx');
|
||||
const outcome = await saveArtifactViaDialog('a-1', 'Deck', 'pptx');
|
||||
expect(outcome).toEqual({ ok: true, path: '/Users/me/Desktop/Deck.pptx' });
|
||||
expect(hoisted.invoke).toHaveBeenCalledWith('save_artifact_via_dialog', {
|
||||
sourcePath: '/ws/artifacts/a-1/deck.pptx',
|
||||
suggestedFilename: 'Deck.pptx',
|
||||
});
|
||||
});
|
||||
|
||||
it('treats a null result (dialog dismissed) as CANCELLED, not an error', async () => {
|
||||
resolveOk();
|
||||
hoisted.invoke.mockResolvedValueOnce(null);
|
||||
const outcome = await saveArtifactViaDialog('a-1', 'Deck', 'pptx');
|
||||
expect(outcome).toEqual({ ok: false, code: 'CANCELLED', error: expect.any(String) });
|
||||
});
|
||||
|
||||
it('falls back to the Downloads copy when the dialog is unavailable', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
// First resolve (dialog path) → invoke throws; fallback re-resolves
|
||||
// then invokes the Downloads command successfully.
|
||||
resolveOk();
|
||||
hoisted.invoke.mockRejectedValueOnce(new Error('no portal'));
|
||||
vi.mocked(callCoreRpc).mockResolvedValueOnce({
|
||||
absolute_path: '/ws/artifacts/a-1/deck.pptx',
|
||||
meta: { id: 'a-1', title: 'Deck' },
|
||||
} as never);
|
||||
hoisted.invoke.mockResolvedValueOnce('/Users/me/Downloads/Deck.pptx');
|
||||
|
||||
const outcome = await saveArtifactViaDialog('a-1', 'Deck', 'pptx');
|
||||
expect(outcome).toEqual({ ok: true, path: '/Users/me/Downloads/Deck.pptx' });
|
||||
expect(hoisted.invoke).toHaveBeenNthCalledWith(2, 'download_artifact_to_downloads', {
|
||||
sourcePath: '/ws/artifacts/a-1/deck.pptx',
|
||||
filename: 'Deck.pptx',
|
||||
});
|
||||
warn.mockRestore();
|
||||
});
|
||||
|
||||
it('propagates resolve failures without showing a dialog', async () => {
|
||||
vi.mocked(callCoreRpc).mockRejectedValueOnce(new Error('rpc down'));
|
||||
const outcome = await saveArtifactViaDialog('a-1', 'Deck', 'pptx');
|
||||
expect(outcome).toEqual({ ok: false, code: 'RESOLVE_FAILED', error: 'rpc down' });
|
||||
expect(hoisted.invoke).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { subscribeChatEvents } from '../chatService';
|
||||
import { aiRegenerate, subscribeChatEvents } from '../chatService';
|
||||
import { callCoreRpc } from '../coreRpcClient';
|
||||
import { socketService } from '../socketService';
|
||||
|
||||
vi.mock('../socketService', () => ({ socketService: { getSocket: vi.fn() } }));
|
||||
@@ -267,3 +268,91 @@ describe('chatService — artifact_ready / artifact_failed handlers (#2779)', ()
|
||||
expect(offEvents).toEqual(['artifact_ready', 'artifact_failed']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('chatService — artifact_pending handler (#3162)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('subscribes to artifact_pending under its canonical snake_case name', () => {
|
||||
const socket = createMockSocket();
|
||||
vi.mocked(socketService.getSocket).mockReturnValue(socket as never);
|
||||
|
||||
subscribeChatEvents({ onArtifactPending: () => {} });
|
||||
|
||||
const events = socket.on.mock.calls.map(call => call[0]);
|
||||
expect(events).toEqual(['artifact_pending']);
|
||||
});
|
||||
|
||||
it('flattens the wire envelope into a typed ArtifactPendingEvent', () => {
|
||||
const socket = createMockSocket();
|
||||
vi.mocked(socketService.getSocket).mockReturnValue(socket as never);
|
||||
const onArtifactPending = vi.fn();
|
||||
|
||||
subscribeChatEvents({ onArtifactPending });
|
||||
|
||||
socket.emit('artifact_pending', {
|
||||
thread_id: 'thread-1',
|
||||
client_id: 'web-x',
|
||||
args: {
|
||||
artifact_id: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
workspace_dir: '/workspace',
|
||||
path: 'a-1/deck.pptx',
|
||||
},
|
||||
});
|
||||
|
||||
expect(onArtifactPending).toHaveBeenCalledTimes(1);
|
||||
expect(onArtifactPending.mock.calls[0]![0]).toEqual({
|
||||
thread_id: 'thread-1',
|
||||
client_id: 'web-x',
|
||||
artifact_id: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
workspace_dir: '/workspace',
|
||||
path: 'a-1/deck.pptx',
|
||||
});
|
||||
});
|
||||
|
||||
it('drops a pending payload missing required args', () => {
|
||||
const socket = createMockSocket();
|
||||
vi.mocked(socketService.getSocket).mockReturnValue(socket as never);
|
||||
const onArtifactPending = vi.fn();
|
||||
|
||||
subscribeChatEvents({ onArtifactPending });
|
||||
|
||||
// Missing `path` → malformed, skipped.
|
||||
socket.emit('artifact_pending', {
|
||||
thread_id: 'thread-1',
|
||||
args: { artifact_id: 'a-1', kind: 'presentation', title: 'Deck', workspace_dir: '/ws' },
|
||||
});
|
||||
|
||||
expect(onArtifactPending).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('aiRegenerate (#3162)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('calls ai_regenerate with the socket client_id for routing', async () => {
|
||||
vi.mocked(socketService.getSocket).mockReturnValue({ id: 'socket-9' } as never);
|
||||
vi.mocked(callCoreRpc).mockResolvedValueOnce({} as never);
|
||||
|
||||
const ok = await aiRegenerate('a-1', 'thread-1');
|
||||
|
||||
expect(ok).toBe(true);
|
||||
expect(callCoreRpc).toHaveBeenCalledWith({
|
||||
method: 'openhuman.ai_regenerate',
|
||||
params: { artifact_id: 'a-1', thread_id: 'thread-1', client_id: 'socket-9' },
|
||||
});
|
||||
});
|
||||
|
||||
it('throws when the socket is not connected', async () => {
|
||||
vi.mocked(socketService.getSocket).mockReturnValue(undefined as never);
|
||||
await expect(aiRegenerate('a-1', 'thread-1')).rejects.toThrow('Socket not connected');
|
||||
expect(callCoreRpc).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,20 +1,20 @@
|
||||
/**
|
||||
* Artifact download service (#2779).
|
||||
* Artifact export service (#2779, #3162).
|
||||
*
|
||||
* Flow:
|
||||
* All paths first resolve the artifact's absolute on-disk path + meta
|
||||
* via the `openhuman.ai_get_artifact` core RPC, then hand a source path
|
||||
* + filename hint to a Tauri command:
|
||||
*
|
||||
* 1. Call `openhuman.ai_get_artifact` via the existing core RPC client
|
||||
* to resolve the artifact's absolute on-disk path + meta.
|
||||
* 2. Invoke the Tauri `download_artifact_to_downloads` command with
|
||||
* the source path + a filename hint built from the artifact's
|
||||
* title. The command picks a non-colliding name under the user's
|
||||
* Downloads directory and copies the file.
|
||||
* 3. Return the resolved dest path so the UI can show a "Saved to …"
|
||||
* toast with a "Reveal in Finder" button (the `opener` plugin's
|
||||
* `reveal-item-in-dir` capability is already wired).
|
||||
* - {@link saveArtifactViaDialog} (#3162) — native Save-As dialog
|
||||
* pre-filled with the filename; user picks the destination. Cancel is
|
||||
* reported as `{ ok: false, code: 'CANCELLED' }`. Falls back to the
|
||||
* Downloads copy if the dialog itself is unavailable.
|
||||
* - {@link downloadArtifact} (#2779) — copies into the user's Downloads
|
||||
* directory with a non-colliding name and returns the dest path so the
|
||||
* UI can offer "Reveal in Finder".
|
||||
*
|
||||
* No-ops outside Tauri (browser dev preview) — the download flow only
|
||||
* makes sense in the desktop shell.
|
||||
* No-ops outside Tauri (browser dev preview) — export only makes sense in
|
||||
* the desktop shell.
|
||||
*/
|
||||
import { revealItemInDir } from '@tauri-apps/plugin-opener';
|
||||
|
||||
@@ -35,6 +35,7 @@ export type ArtifactErrorCode =
|
||||
| 'MISSING_ARTIFACT_PATH'
|
||||
| 'RESOLVE_FAILED'
|
||||
| 'DOWNLOAD_FAILED'
|
||||
| 'CANCELLED'
|
||||
| 'DELETE_FAILED';
|
||||
|
||||
/** Outcome surfaced to the UI for a single download attempt. */
|
||||
@@ -80,25 +81,24 @@ interface AiGetArtifactData {
|
||||
meta?: { id?: string; title?: string; path?: string; kind?: string; status?: string };
|
||||
}
|
||||
|
||||
/** Resolved source path + filename hint for an artifact export. */
|
||||
interface ResolvedExport {
|
||||
ok: true;
|
||||
sourcePath: string;
|
||||
filename: string;
|
||||
}
|
||||
type ResolveExportResult = ResolvedExport | { ok: false; code: ArtifactErrorCode; error: string };
|
||||
|
||||
/**
|
||||
* Resolve the source path + filename hint, then copy to Downloads.
|
||||
*
|
||||
* `extension` is the file extension WITHOUT the leading dot
|
||||
* (`"pptx"`, `"pdf"`, …). Used to build the Downloads filename when
|
||||
* the title doesn't already carry one.
|
||||
* Resolve an artifact's absolute on-disk path (via `ai_get_artifact`)
|
||||
* and build the suggested filename. Shared by the Save-As and Downloads
|
||||
* export paths so both apply identical title/extension handling.
|
||||
*/
|
||||
export async function downloadArtifact(
|
||||
async function resolveArtifactForExport(
|
||||
artifactId: string,
|
||||
fallbackTitle: string,
|
||||
extension: string
|
||||
): Promise<DownloadArtifactOutcome> {
|
||||
if (!isTauri()) {
|
||||
return {
|
||||
ok: false,
|
||||
code: 'NOT_DESKTOP',
|
||||
error: 'Downloads are only available in the desktop app',
|
||||
};
|
||||
}
|
||||
): Promise<ResolveExportResult> {
|
||||
if (!artifactId.trim()) {
|
||||
return { ok: false, code: 'MISSING_ARTIFACT_ID', error: 'artifact id missing' };
|
||||
}
|
||||
@@ -136,8 +136,32 @@ export async function downloadArtifact(
|
||||
const titleHasSameExt = ext.length > 0 && title.toLowerCase().endsWith(`.${ext.toLowerCase()}`);
|
||||
const filename = ext && !titleHasExtension && !titleHasSameExt ? `${title}.${ext}` : title;
|
||||
|
||||
return { ok: true, sourcePath, filename };
|
||||
}
|
||||
|
||||
export async function downloadArtifact(
|
||||
artifactId: string,
|
||||
fallbackTitle: string,
|
||||
extension: string
|
||||
): Promise<DownloadArtifactOutcome> {
|
||||
if (!isTauri()) {
|
||||
return {
|
||||
ok: false,
|
||||
code: 'NOT_DESKTOP',
|
||||
error: 'Downloads are only available in the desktop app',
|
||||
};
|
||||
}
|
||||
|
||||
const resolved = await resolveArtifactForExport(artifactId, fallbackTitle, extension);
|
||||
if (!resolved.ok) {
|
||||
return { ok: false, code: resolved.code, error: resolved.error };
|
||||
}
|
||||
|
||||
try {
|
||||
const dest = await invoke<string>('download_artifact_to_downloads', { sourcePath, filename });
|
||||
const dest = await invoke<string>('download_artifact_to_downloads', {
|
||||
sourcePath: resolved.sourcePath,
|
||||
filename: resolved.filename,
|
||||
});
|
||||
return { ok: true, path: dest };
|
||||
} catch (err) {
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
@@ -145,6 +169,47 @@ export async function downloadArtifact(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Export an artifact via the native Save-As dialog (#3162), pre-filled
|
||||
* with the artifact's filename. On the user dismissing the dialog,
|
||||
* returns `{ ok: false, code: 'CANCELLED' }` (the caller should treat
|
||||
* this as a no-op, not an error). If the dialog itself is unavailable —
|
||||
* e.g. headless Linux with no xdg-desktop portal — falls back to the
|
||||
* Downloads copy so the artifact is still recoverable.
|
||||
*/
|
||||
export async function saveArtifactViaDialog(
|
||||
artifactId: string,
|
||||
fallbackTitle: string,
|
||||
extension: string
|
||||
): Promise<DownloadArtifactOutcome> {
|
||||
if (!isTauri()) {
|
||||
return { ok: false, code: 'NOT_DESKTOP', error: 'Saving is only available in the desktop app' };
|
||||
}
|
||||
|
||||
const resolved = await resolveArtifactForExport(artifactId, fallbackTitle, extension);
|
||||
if (!resolved.ok) {
|
||||
return { ok: false, code: resolved.code, error: resolved.error };
|
||||
}
|
||||
|
||||
try {
|
||||
// Command returns the saved path, or `null` when the user cancelled.
|
||||
const dest = await invoke<string | null>('save_artifact_via_dialog', {
|
||||
sourcePath: resolved.sourcePath,
|
||||
suggestedFilename: resolved.filename,
|
||||
});
|
||||
if (dest == null) {
|
||||
return { ok: false, code: 'CANCELLED', error: 'save cancelled by user' };
|
||||
}
|
||||
return { ok: true, path: dest };
|
||||
} catch (err) {
|
||||
// Dialog unavailable (no portal / unsupported) — recover the artifact
|
||||
// via the Downloads copy rather than stranding the user.
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
console.warn('[artifact] save dialog failed, falling back to Downloads:', reason);
|
||||
return downloadArtifact(artifactId, fallbackTitle, extension);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the user's file manager pointed at the just-downloaded file.
|
||||
* Uses the existing `opener:allow-reveal-item-in-dir` capability —
|
||||
|
||||
@@ -215,6 +215,26 @@ export interface ArtifactFailedEvent {
|
||||
error: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Emitted when `artifacts::store::create_artifact` reserves an artifact
|
||||
* row (status `Pending`), before the producer has written any bytes
|
||||
* (#3162). The chat runtime upserts an `in_progress` snapshot keyed on
|
||||
* `artifact_id` so the `ArtifactCard` shows a spinner the moment the
|
||||
* tool starts, then swaps in place when the matching `artifact_ready`
|
||||
* / `artifact_failed` arrives. Backend half shipped in #3277.
|
||||
*/
|
||||
export interface ArtifactPendingEvent {
|
||||
thread_id: string;
|
||||
client_id?: string;
|
||||
artifact_id: string;
|
||||
kind: ArtifactKind;
|
||||
title: string;
|
||||
/** Absolute workspace root — see {@link ArtifactReadyEvent.workspace_dir}. */
|
||||
workspace_dir: string;
|
||||
/** Relative path under `<workspace>/artifacts/`, e.g. `<uuid>/deck.pptx`. */
|
||||
path: string;
|
||||
}
|
||||
|
||||
/** Emitted when the agent turn begins (before the first LLM call). */
|
||||
export interface ChatInferenceStartEvent {
|
||||
thread_id: string;
|
||||
@@ -458,6 +478,7 @@ export interface ChatEventListeners {
|
||||
onProactiveMessage?: (event: ProactiveMessageEvent) => void;
|
||||
onApprovalRequest?: (event: ChatApprovalRequestEvent) => void;
|
||||
onPlanReviewRequest?: (event: ChatPlanReviewRequestEvent) => void;
|
||||
onArtifactPending?: (event: ArtifactPendingEvent) => void;
|
||||
onArtifactReady?: (event: ArtifactReadyEvent) => void;
|
||||
onArtifactFailed?: (event: ArtifactFailedEvent) => void;
|
||||
onDone?: (event: ChatDoneEvent) => void;
|
||||
@@ -494,6 +515,7 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void {
|
||||
proactiveMessage: 'proactive_message',
|
||||
approvalRequest: 'approval_request',
|
||||
planReviewRequest: 'plan_review_request',
|
||||
artifactPending: 'artifact_pending',
|
||||
artifactReady: 'artifact_ready',
|
||||
artifactFailed: 'artifact_failed',
|
||||
done: 'chat_done',
|
||||
@@ -861,6 +883,51 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void {
|
||||
return { thread_id: env.thread_id, client_id, args };
|
||||
};
|
||||
|
||||
if (listeners.onArtifactPending) {
|
||||
const cb = (payload: unknown) => {
|
||||
const env = readEnvelope(payload);
|
||||
if (!env) {
|
||||
chatLog('%s — skipping malformed payload (bad envelope)', EVENTS.artifactPending);
|
||||
return;
|
||||
}
|
||||
const { args } = env;
|
||||
// Pending carries no size/path-on-disk yet — only identity + title.
|
||||
if (
|
||||
!isNonEmptyString(args.artifact_id) ||
|
||||
!isValidArtifactKind(args.kind) ||
|
||||
!isNonEmptyString(args.title) ||
|
||||
!isNonEmptyString(args.workspace_dir) ||
|
||||
!isNonEmptyString(args.path)
|
||||
) {
|
||||
chatLog(
|
||||
'%s thread_id=%s — skipping malformed payload (bad args)',
|
||||
EVENTS.artifactPending,
|
||||
env.thread_id
|
||||
);
|
||||
return;
|
||||
}
|
||||
const event: ArtifactPendingEvent = {
|
||||
thread_id: env.thread_id,
|
||||
client_id: env.client_id,
|
||||
artifact_id: args.artifact_id,
|
||||
kind: args.kind,
|
||||
title: args.title,
|
||||
workspace_dir: args.workspace_dir,
|
||||
path: args.path,
|
||||
};
|
||||
chatLog(
|
||||
'%s thread_id=%s artifact_id=%s kind=%s',
|
||||
EVENTS.artifactPending,
|
||||
event.thread_id,
|
||||
event.artifact_id,
|
||||
event.kind
|
||||
);
|
||||
listeners.onArtifactPending?.(event);
|
||||
};
|
||||
socket.on(EVENTS.artifactPending, cb);
|
||||
handlers.push([EVENTS.artifactPending, cb]);
|
||||
}
|
||||
|
||||
if (listeners.onArtifactReady) {
|
||||
const cb = (payload: unknown) => {
|
||||
const env = readEnvelope(payload);
|
||||
@@ -1121,6 +1188,27 @@ export async function chatClearQueue(threadId: string): Promise<number | null> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-dispatch the producing tool for a failed artifact, reusing the same
|
||||
* artifact id so the card swaps in place (#3162). Drives the failed-card
|
||||
* Retry button: the core reloads the persisted creation args and re-runs
|
||||
* the producer, routing the fresh pending/ready/failed events back to
|
||||
* this thread + socket. Returns `true` when the RPC was accepted; the
|
||||
* card's live state is then driven by the socket events, not this call.
|
||||
*/
|
||||
export async function aiRegenerate(artifactId: string, threadId: string): Promise<boolean> {
|
||||
const socket = socketService.getSocket();
|
||||
const clientId = socket?.id;
|
||||
if (!clientId) {
|
||||
throw new Error('Socket not connected — no client ID for event routing');
|
||||
}
|
||||
await callCoreRpc({
|
||||
method: 'openhuman.ai_regenerate',
|
||||
params: { artifact_id: artifactId, thread_id: threadId, client_id: clientId },
|
||||
});
|
||||
return true;
|
||||
}
|
||||
|
||||
export function useRustChat(): boolean {
|
||||
// Legacy name kept for compatibility with existing call sites.
|
||||
return true;
|
||||
|
||||
@@ -234,3 +234,62 @@ describe('chatRuntimeSlice — artifact lifecycle (#2779)', () => {
|
||||
expect(list[0].error).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('chatRuntimeSlice — in_progress no-downgrade guard (#3162)', () => {
|
||||
it('does NOT regress a ready artifact back to in_progress', () => {
|
||||
let state = reducer(
|
||||
undefined,
|
||||
upsertArtifactReadyForThread({
|
||||
threadId: 't-1',
|
||||
artifactId: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
path: 'a-1/deck.pptx',
|
||||
sizeBytes: 4096,
|
||||
})
|
||||
);
|
||||
|
||||
// A late / duplicate artifact_pending must not wipe the ready state.
|
||||
state = reducer(
|
||||
state,
|
||||
upsertArtifactInProgressForThread({
|
||||
threadId: 't-1',
|
||||
artifactId: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
})
|
||||
);
|
||||
|
||||
const list = state.artifactsByThread['t-1'];
|
||||
expect(list).toHaveLength(1);
|
||||
expect(list[0]).toMatchObject({ status: 'ready', sizeBytes: 4096 });
|
||||
});
|
||||
|
||||
it('allows failed -> in_progress (an explicit retry re-shows the spinner)', () => {
|
||||
let state = reducer(
|
||||
undefined,
|
||||
upsertArtifactFailedForThread({
|
||||
threadId: 't-1',
|
||||
artifactId: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
error: 'boom',
|
||||
})
|
||||
);
|
||||
|
||||
state = reducer(
|
||||
state,
|
||||
upsertArtifactInProgressForThread({
|
||||
threadId: 't-1',
|
||||
artifactId: 'a-1',
|
||||
kind: 'presentation',
|
||||
title: 'Deck',
|
||||
})
|
||||
);
|
||||
|
||||
const list = state.artifactsByThread['t-1'];
|
||||
expect(list).toHaveLength(1);
|
||||
expect(list[0].status).toBe('in_progress');
|
||||
expect(list[0].error).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -920,6 +920,20 @@ const chatRuntimeSlice = createSlice({
|
||||
}>
|
||||
) => {
|
||||
const { threadId, artifactId, kind, title } = action.payload;
|
||||
// No-downgrade guard: a late `artifact_pending` (re-delivery, or a
|
||||
// socket race) must never regress an artifact that already reached
|
||||
// `ready` / `failed` back to a spinner. Only the regenerate flow
|
||||
// (#3162) legitimately re-enters `in_progress`, and that reuses the
|
||||
// id via a fresh pending event AFTER the failed state — which is
|
||||
// allowed because the previous terminal state was `failed`, and a
|
||||
// retry SHOULD show the spinner again. So: block downgrade only from
|
||||
// `ready`; allow `failed -> in_progress` (an explicit retry).
|
||||
const existing = (state.artifactsByThread[threadId] ?? []).find(
|
||||
entry => entry.artifactId === artifactId
|
||||
);
|
||||
if (existing && existing.status === 'ready') {
|
||||
return;
|
||||
}
|
||||
const snapshot: ArtifactSnapshot = {
|
||||
artifactId,
|
||||
kind,
|
||||
|
||||
@@ -1,9 +1,14 @@
|
||||
use serde_json::{json, Value};
|
||||
|
||||
use crate::openhuman::approval::{ApprovalChatContext, APPROVAL_CHAT_CONTEXT};
|
||||
use crate::openhuman::config::Config;
|
||||
use crate::openhuman::security::SecurityPolicy;
|
||||
use crate::openhuman::tools::traits::Tool;
|
||||
use crate::openhuman::tools::PresentationTool;
|
||||
use crate::rpc::RpcOutcome;
|
||||
|
||||
use super::store;
|
||||
use super::types::ArtifactKind;
|
||||
|
||||
/// Default page size for `ai_list_artifacts`.
|
||||
const DEFAULT_LIMIT: usize = 50;
|
||||
@@ -123,6 +128,101 @@ pub async fn ai_delete_artifact(
|
||||
Ok(RpcOutcome::new(value, vec![]))
|
||||
}
|
||||
|
||||
/// Re-dispatch the producing tool for a failed (or any) artifact using
|
||||
/// the args persisted at create-time, reusing the original `artifact_id`
|
||||
/// so the in-chat card swaps in place (#3162).
|
||||
///
|
||||
/// Drives the failed-card Retry affordance. The flow:
|
||||
///
|
||||
/// 1. Load the artifact's `meta.json`; only `presentation` artifacts are
|
||||
/// regenerable today (the single producing tool that persists args).
|
||||
/// 2. Load the verbatim `args.json` sidecar — absent for artifacts made
|
||||
/// before #3162 or by a non-persisting producer, which surfaces as a
|
||||
/// "not regenerable" error rather than a silent no-op.
|
||||
/// 3. Rebuild the producer tool + a fresh [`SecurityPolicy`] from the
|
||||
/// live config and run it inside both:
|
||||
/// - `REGENERATE_TARGET_ID.scope(artifact_id, …)` so `create_artifact`
|
||||
/// reuses the id + directory instead of minting a new one, and
|
||||
/// - `APPROVAL_CHAT_CONTEXT.scope({thread_id, client_id}, …)` so the
|
||||
/// Pending/Ready/Failed events route back to the originating chat
|
||||
/// surface (the RPC carries no ambient chat context of its own).
|
||||
///
|
||||
/// The returned value is a thin ack — the card's live state is driven by
|
||||
/// the socket events the re-run publishes, not by this RPC's result.
|
||||
pub async fn ai_regenerate(
|
||||
config: &Config,
|
||||
artifact_id: &str,
|
||||
thread_id: &str,
|
||||
client_id: &str,
|
||||
) -> Result<RpcOutcome<Value>, String> {
|
||||
log::info!(
|
||||
"[artifacts] ai_regenerate: id={artifact_id} thread_id={thread_id} client_id={client_id} workspace={:?}",
|
||||
config.workspace_dir
|
||||
);
|
||||
|
||||
if artifact_id.is_empty() {
|
||||
return Err("[artifacts] artifact_id must not be empty".to_string());
|
||||
}
|
||||
if thread_id.is_empty() || client_id.is_empty() {
|
||||
return Err(
|
||||
"[artifacts] regenerate requires thread_id + client_id for event routing".to_string(),
|
||||
);
|
||||
}
|
||||
|
||||
let meta = store::get_artifact(&config.workspace_dir, artifact_id).await?;
|
||||
if meta.kind != ArtifactKind::Presentation {
|
||||
return Err(format!(
|
||||
"[artifacts] regenerate is only supported for presentations (artifact id={artifact_id} is {})",
|
||||
meta.kind.as_str()
|
||||
));
|
||||
}
|
||||
|
||||
let args = store::read_artifact_args(&config.workspace_dir, artifact_id).await?;
|
||||
|
||||
// A fresh policy from the live config — cheap, sync, and mirrors how
|
||||
// the agent harness builds the same tool (see `tools::ops`).
|
||||
let security = std::sync::Arc::new(SecurityPolicy::from_config(
|
||||
&config.autonomy,
|
||||
&config.workspace_dir,
|
||||
&config.action_dir,
|
||||
));
|
||||
let tool = PresentationTool::new(config.workspace_dir.clone(), security);
|
||||
|
||||
let chat_ctx = ApprovalChatContext {
|
||||
thread_id: thread_id.to_string(),
|
||||
client_id: client_id.to_string(),
|
||||
};
|
||||
|
||||
let result = store::REGENERATE_TARGET_ID
|
||||
.scope(
|
||||
artifact_id.to_string(),
|
||||
APPROVAL_CHAT_CONTEXT.scope(chat_ctx, async move { tool.execute(args).await }),
|
||||
)
|
||||
.await;
|
||||
|
||||
match result {
|
||||
Ok(tool_result) => {
|
||||
// Even when the engine fails, `execute` returns `Ok(error)` and
|
||||
// has already published `ArtifactFailed` for the reused id, so
|
||||
// the card reflects the outcome via the socket. Report the flag
|
||||
// back so the caller can log it.
|
||||
log::info!(
|
||||
"[artifacts] ai_regenerate: id={artifact_id} re-dispatched (is_error={})",
|
||||
tool_result.is_error
|
||||
);
|
||||
let value = json!({
|
||||
"artifact_id": artifact_id,
|
||||
"regenerated": true,
|
||||
"is_error": tool_result.is_error,
|
||||
});
|
||||
Ok(RpcOutcome::new(value, vec![]))
|
||||
}
|
||||
Err(err) => Err(format!(
|
||||
"[artifacts] regenerate execution error for id={artifact_id}: {err}"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "ops_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -171,3 +171,105 @@ async fn delete_missing_id_error() {
|
||||
let err = ai_delete_artifact(&config, "").await.unwrap_err();
|
||||
assert!(err.contains("must not be empty"), "unexpected error: {err}");
|
||||
}
|
||||
|
||||
// ── ai_regenerate (#3162) ──────────────────────────────────────────────────
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_rejects_empty_id() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp);
|
||||
assert!(ai_regenerate(&config, "", "t", "c").await.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_requires_thread_and_client() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp);
|
||||
assert!(ai_regenerate(&config, "id", "", "c").await.is_err());
|
||||
assert!(ai_regenerate(&config, "id", "t", "").await.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_rejects_non_presentation_kind() {
|
||||
use crate::openhuman::artifacts::store::{save_artifact_args, save_artifact_meta};
|
||||
use crate::openhuman::artifacts::types::{ArtifactKind, ArtifactMeta, ArtifactStatus};
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp);
|
||||
|
||||
save_artifact_meta(
|
||||
tmp.path(),
|
||||
&ArtifactMeta {
|
||||
id: "doc-1".to_string(),
|
||||
kind: ArtifactKind::Document,
|
||||
title: "notes".to_string(),
|
||||
path: "doc-1/notes.txt".to_string(),
|
||||
size_bytes: 0,
|
||||
status: ArtifactStatus::Failed,
|
||||
created_at: chrono::Utc::now(),
|
||||
error: Some("boom".to_string()),
|
||||
thread_id: Some("t".to_string()),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
save_artifact_args(tmp.path(), "doc-1", &serde_json::json!({}))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let err = ai_regenerate(&config, "doc-1", "t", "c").await.unwrap_err();
|
||||
assert!(
|
||||
err.contains("only supported for presentations"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_errors_when_args_missing() {
|
||||
use crate::openhuman::artifacts::store::create_artifact;
|
||||
use crate::openhuman::artifacts::types::ArtifactKind;
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp);
|
||||
|
||||
// A presentation artifact with no persisted args.json (e.g. created
|
||||
// before #3162) cannot be regenerated.
|
||||
let (meta, _) = create_artifact(tmp.path(), ArtifactKind::Presentation, "Old Deck", "pptx")
|
||||
.await
|
||||
.unwrap();
|
||||
let err = ai_regenerate(&config, &meta.id, "t", "c")
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(err.contains("not regenerable"), "unexpected error: {err}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_reruns_producer_and_reuses_id() {
|
||||
use crate::openhuman::artifacts::store::{create_artifact, get_artifact, save_artifact_args};
|
||||
use crate::openhuman::artifacts::types::{ArtifactKind, ArtifactStatus};
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp);
|
||||
|
||||
// Seed a presentation artifact + its persisted creation args.
|
||||
let (meta, _) = create_artifact(tmp.path(), ArtifactKind::Presentation, "Q3 Deck", "pptx")
|
||||
.await
|
||||
.unwrap();
|
||||
let args = serde_json::json!({
|
||||
"title": "Q3 Deck",
|
||||
"slides": [{ "title": "Intro", "bullets": ["alpha", "beta"] }],
|
||||
});
|
||||
save_artifact_args(tmp.path(), &meta.id, &args)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let outcome = ai_regenerate(&config, &meta.id, "thread-1", "client-1")
|
||||
.await
|
||||
.unwrap();
|
||||
let value = outcome.into_cli_compatible_json().unwrap();
|
||||
assert_eq!(value["artifact_id"], meta.id);
|
||||
assert_eq!(value["regenerated"], true);
|
||||
assert_eq!(value["is_error"], false);
|
||||
|
||||
// Same id reused in place; the re-run drove it to Ready.
|
||||
let got = get_artifact(tmp.path(), &meta.id).await.unwrap();
|
||||
assert_eq!(got.id, meta.id);
|
||||
assert_eq!(got.status, ArtifactStatus::Ready);
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ pub fn all_controller_schemas() -> Vec<ControllerSchema> {
|
||||
schemas("list_artifacts"),
|
||||
schemas("get_artifact"),
|
||||
schemas("delete_artifact"),
|
||||
schemas("regenerate"),
|
||||
]
|
||||
}
|
||||
|
||||
@@ -28,6 +29,10 @@ pub fn all_registered_controllers() -> Vec<RegisteredController> {
|
||||
schema: schemas("delete_artifact"),
|
||||
handler: handle_delete_artifact,
|
||||
},
|
||||
RegisteredController {
|
||||
schema: schemas("regenerate"),
|
||||
handler: handle_regenerate,
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
@@ -171,6 +176,53 @@ pub fn schemas(function: &str) -> ControllerSchema {
|
||||
required: true,
|
||||
}],
|
||||
},
|
||||
"regenerate" => ControllerSchema {
|
||||
namespace: "ai",
|
||||
function: "regenerate",
|
||||
description: "Re-run the producing tool for an existing artifact using its persisted creation args, reusing the same artifact id so the chat card swaps in place. Drives the failed-card Retry affordance (#3162).",
|
||||
inputs: vec![
|
||||
artifact_id_input("Identifier of the artifact to regenerate."),
|
||||
FieldSchema {
|
||||
name: "thread_id",
|
||||
ty: TypeSchema::String,
|
||||
comment: "Chat thread to route the regenerated artifact's pending/ready/failed events to.",
|
||||
required: true,
|
||||
},
|
||||
FieldSchema {
|
||||
name: "client_id",
|
||||
ty: TypeSchema::String,
|
||||
comment: "Socket client id (web channel) to address the regenerated artifact's events to.",
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
outputs: vec![FieldSchema {
|
||||
name: "result",
|
||||
ty: TypeSchema::Object {
|
||||
fields: vec![
|
||||
FieldSchema {
|
||||
name: "artifact_id",
|
||||
ty: TypeSchema::String,
|
||||
comment: "Identifier that was regenerated (unchanged — reused in place).",
|
||||
required: true,
|
||||
},
|
||||
FieldSchema {
|
||||
name: "regenerated",
|
||||
ty: TypeSchema::Bool,
|
||||
comment: "True when the producing tool was re-dispatched.",
|
||||
required: true,
|
||||
},
|
||||
FieldSchema {
|
||||
name: "is_error",
|
||||
ty: TypeSchema::Bool,
|
||||
comment: "True when the re-dispatched generation itself reported an error (the card is flipped to failed via socket regardless).",
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
comment: "Regeneration ack payload.",
|
||||
required: true,
|
||||
}],
|
||||
},
|
||||
_other => ControllerSchema {
|
||||
namespace: "ai",
|
||||
function: "unknown",
|
||||
@@ -245,6 +297,24 @@ fn handle_delete_artifact(params: Map<String, Value>) -> ControllerFuture {
|
||||
})
|
||||
}
|
||||
|
||||
fn handle_regenerate(params: Map<String, Value>) -> ControllerFuture {
|
||||
Box::pin(async move {
|
||||
let config = config_rpc::load_config_with_timeout().await?;
|
||||
let artifact_id = read_required::<String>(¶ms, "artifact_id")?;
|
||||
let thread_id = read_required::<String>(¶ms, "thread_id")?;
|
||||
let client_id = read_required::<String>(¶ms, "client_id")?;
|
||||
to_json(
|
||||
crate::openhuman::artifacts::ops::ai_regenerate(
|
||||
&config,
|
||||
artifact_id.trim(),
|
||||
thread_id.trim(),
|
||||
client_id.trim(),
|
||||
)
|
||||
.await?,
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn read_required<T: DeserializeOwned>(params: &Map<String, Value>, key: &str) -> Result<T, String> {
|
||||
let value = params
|
||||
.get(key)
|
||||
@@ -382,21 +452,48 @@ mod tests {
|
||||
.collect();
|
||||
assert_eq!(
|
||||
names,
|
||||
vec!["list_artifacts", "get_artifact", "delete_artifact"]
|
||||
vec![
|
||||
"list_artifacts",
|
||||
"get_artifact",
|
||||
"delete_artifact",
|
||||
"regenerate"
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn all_registered_controllers_has_handler_per_schema() {
|
||||
let controllers = all_registered_controllers();
|
||||
assert_eq!(controllers.len(), 3);
|
||||
assert_eq!(controllers.len(), 4);
|
||||
let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect();
|
||||
assert_eq!(
|
||||
names,
|
||||
vec!["list_artifacts", "get_artifact", "delete_artifact"]
|
||||
vec![
|
||||
"list_artifacts",
|
||||
"get_artifact",
|
||||
"delete_artifact",
|
||||
"regenerate"
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schemas_regenerate_requires_artifact_id_thread_and_client() {
|
||||
let s = schemas("regenerate");
|
||||
assert_eq!(s.function, "regenerate");
|
||||
let input_names: Vec<_> = s.inputs.iter().map(|f| f.name).collect();
|
||||
assert_eq!(input_names, vec!["artifact_id", "thread_id", "client_id"]);
|
||||
assert!(s.inputs.iter().all(|f| f.required));
|
||||
if let TypeSchema::Object { fields } = &s.outputs[0].ty {
|
||||
let names: Vec<_> = fields.iter().map(|f| f.name).collect();
|
||||
assert!(names.contains(&"artifact_id"));
|
||||
assert!(names.contains(&"regenerated"));
|
||||
assert!(names.contains(&"is_error"));
|
||||
} else {
|
||||
panic!("expected object output type");
|
||||
}
|
||||
}
|
||||
|
||||
// ── read_required ───────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -4,6 +4,22 @@ use super::types::{ArtifactMeta, ArtifactStatus};
|
||||
|
||||
const ARTIFACTS_SUBDIR: &str = "artifacts";
|
||||
const META_FILENAME: &str = "meta.json";
|
||||
/// Sidecar file holding the verbatim producer-tool arguments that
|
||||
/// generated the artifact, persisted next to `meta.json` so a failed
|
||||
/// card's Retry button can re-dispatch the exact same generation
|
||||
/// deterministically without round-tripping the args back through the
|
||||
/// LLM (#3162). Written by the producing tool right after
|
||||
/// [`create_artifact`]; read by `ops::ai_regenerate`.
|
||||
const ARGS_FILENAME: &str = "args.json";
|
||||
|
||||
tokio::task_local! {
|
||||
/// When set (by `ops::ai_regenerate`), [`create_artifact`] reuses
|
||||
/// this id + its existing directory instead of minting a fresh
|
||||
/// UUID — so a Retry swaps the failed card in place rather than
|
||||
/// appending a second card (#3162). Unset for all normal
|
||||
/// generation paths, in which case a fresh UUID is minted.
|
||||
pub static REGENERATE_TARGET_ID: String;
|
||||
}
|
||||
|
||||
/// Returns the artifacts root directory, creating it if it doesn't exist.
|
||||
///
|
||||
@@ -231,6 +247,67 @@ pub(crate) async fn get_artifact(
|
||||
Ok(meta)
|
||||
}
|
||||
|
||||
/// Persist the verbatim producer-tool arguments alongside an artifact's
|
||||
/// `meta.json` as `<workspace>/artifacts/<id>/args.json` (#3162).
|
||||
///
|
||||
/// Stored so a later [`ops::ai_regenerate`](super::ops::ai_regenerate)
|
||||
/// can reload the exact spec and re-run generation deterministically —
|
||||
/// the Retry affordance on a failed card re-dispatches the *same*
|
||||
/// request rather than asking the LLM to reconstruct it. Best-effort
|
||||
/// from the producer's perspective: a write failure here does not fail
|
||||
/// the generation, it only forfeits the ability to regenerate that
|
||||
/// artifact.
|
||||
pub(crate) async fn save_artifact_args(
|
||||
workspace_dir: &Path,
|
||||
artifact_id: &str,
|
||||
args: &serde_json::Value,
|
||||
) -> Result<(), String> {
|
||||
log::debug!("[artifacts] save_artifact_args: id={artifact_id}");
|
||||
validate_artifact_id(artifact_id)?;
|
||||
let root = artifacts_root(workspace_dir).await?;
|
||||
let artifact_dir = root.join(artifact_id);
|
||||
assert_within_root(&root, &artifact_dir)?;
|
||||
tokio::fs::create_dir_all(&artifact_dir)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
format!(
|
||||
"[artifacts] failed to create artifact dir {:?}: {e}",
|
||||
artifact_dir
|
||||
)
|
||||
})?;
|
||||
let args_path = artifact_dir.join(ARGS_FILENAME);
|
||||
let json = serde_json::to_string_pretty(args)
|
||||
.map_err(|e| format!("[artifacts] failed to serialize args for id={artifact_id}: {e}"))?;
|
||||
tokio::fs::write(&args_path, json)
|
||||
.await
|
||||
.map_err(|e| format!("[artifacts] failed to write args.json for id={artifact_id}: {e}"))?;
|
||||
log::debug!("[artifacts] saved args.json for id={artifact_id}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Load the persisted producer-tool arguments for an artifact (#3162).
|
||||
///
|
||||
/// Returns an `Err` when no `args.json` exists — the common case for
|
||||
/// artifacts created before this sidecar was introduced, or by a
|
||||
/// producer that never persisted args — so the caller can surface a
|
||||
/// "cannot regenerate" message instead of silently doing nothing.
|
||||
pub(crate) async fn read_artifact_args(
|
||||
workspace_dir: &Path,
|
||||
artifact_id: &str,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
log::debug!("[artifacts] read_artifact_args: id={artifact_id}");
|
||||
validate_artifact_id(artifact_id)?;
|
||||
let root = artifacts_root(workspace_dir).await?;
|
||||
let artifact_dir = root.join(artifact_id);
|
||||
assert_within_root(&root, &artifact_dir)?;
|
||||
let args_path = artifact_dir.join(ARGS_FILENAME);
|
||||
let contents = tokio::fs::read_to_string(&args_path).await.map_err(|e| {
|
||||
format!("[artifacts] no persisted args for id={artifact_id} (not regenerable): {e}")
|
||||
})?;
|
||||
serde_json::from_str(&contents)
|
||||
.map_err(|e| format!("[artifacts] corrupt args.json for id={artifact_id}: {e}"))
|
||||
}
|
||||
|
||||
/// Read the raw bytes of a finalized artifact's output file.
|
||||
///
|
||||
/// Single source of truth for resolving an artifact id → on-disk bytes:
|
||||
@@ -369,7 +446,18 @@ pub async fn create_artifact(
|
||||
));
|
||||
}
|
||||
|
||||
let id = uuid::Uuid::new_v4().to_string();
|
||||
// Normal path mints a fresh UUID. A regenerate (#3162) runs inside
|
||||
// `REGENERATE_TARGET_ID.scope(...)` and reuses the original id so the
|
||||
// Pending/Ready/Failed events that follow carry the same artifact_id
|
||||
// and the card swaps in place. The reused dir already exists; the
|
||||
// `create_dir_all` below is idempotent and the meta/file are
|
||||
// overwritten with the fresh generation.
|
||||
let reused_id = REGENERATE_TARGET_ID
|
||||
.try_with(|target| target.clone())
|
||||
.ok()
|
||||
.filter(|target| !target.trim().is_empty());
|
||||
let is_regenerate = reused_id.is_some();
|
||||
let id = reused_id.unwrap_or_else(|| uuid::Uuid::new_v4().to_string());
|
||||
let filename = format!("{}.{trimmed_ext}", sanitize_filename_stem(trimmed_title));
|
||||
let relative_path = format!("{id}/{filename}");
|
||||
|
||||
@@ -393,6 +481,19 @@ pub async fn create_artifact(
|
||||
// routing target survives a process restart.
|
||||
let (thread_id, _) = current_chat_context();
|
||||
|
||||
// On a regenerate the id is reused in place, so preserve the original
|
||||
// `created_at` — bumping it to now would reorder the artifact to the
|
||||
// top of the `created_at`-sorted list/panel even though it is the same
|
||||
// logical artifact (#3162, CodeRabbit). New artifacts always stamp now.
|
||||
let created_at = if is_regenerate {
|
||||
match get_artifact(workspace_dir, &id).await {
|
||||
Ok(prev) => prev.created_at,
|
||||
Err(_) => chrono::Utc::now(),
|
||||
}
|
||||
} else {
|
||||
chrono::Utc::now()
|
||||
};
|
||||
|
||||
let meta = ArtifactMeta {
|
||||
id: id.clone(),
|
||||
kind,
|
||||
@@ -400,7 +501,7 @@ pub async fn create_artifact(
|
||||
path: relative_path,
|
||||
size_bytes: 0,
|
||||
status: ArtifactStatus::Pending,
|
||||
created_at: chrono::Utc::now(),
|
||||
created_at,
|
||||
error: None,
|
||||
thread_id,
|
||||
};
|
||||
|
||||
@@ -327,3 +327,84 @@ async fn create_artifact_publishes_artifact_pending_event() {
|
||||
assert!(thread_id.is_none(), "thread_id leaked, got {thread_id:?}");
|
||||
assert!(client_id.is_none(), "client_id leaked, got {client_id:?}");
|
||||
}
|
||||
|
||||
// ── args sidecar + regenerate id reuse (#3162) ────────────────────────────
|
||||
|
||||
#[tokio::test]
|
||||
async fn save_and_read_args_roundtrip() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let args = serde_json::json!({
|
||||
"title": "Q3 Deck",
|
||||
"slides": [{ "heading": "Intro", "bullets": ["a", "b"] }],
|
||||
});
|
||||
save_artifact_args(tmp.path(), "deck-1", &args)
|
||||
.await
|
||||
.unwrap();
|
||||
let got = read_artifact_args(tmp.path(), "deck-1").await.unwrap();
|
||||
assert_eq!(got, args);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_args_errors_when_absent() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
// No args.json written → not regenerable, surfaces an Err rather than
|
||||
// a silent empty value.
|
||||
let err = read_artifact_args(tmp.path(), "missing").await.unwrap_err();
|
||||
assert!(err.contains("not regenerable"), "unexpected error: {err}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_artifact_mints_fresh_id_without_scope() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let (meta, _path) = create_artifact(tmp.path(), ArtifactKind::Presentation, "Q3 Deck", "pptx")
|
||||
.await
|
||||
.unwrap();
|
||||
// A normal (non-regenerate) create mints a UUID, never an empty id.
|
||||
assert!(!meta.id.is_empty());
|
||||
assert_eq!(meta.status, ArtifactStatus::Pending);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_artifact_reuses_id_inside_regenerate_scope() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let workspace = tmp.path().to_path_buf();
|
||||
let (meta, _path) = REGENERATE_TARGET_ID
|
||||
.scope("reused-id".to_string(), async move {
|
||||
create_artifact(&workspace, ArtifactKind::Presentation, "Q3 Deck", "pptx").await
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
// The scoped target id is reused verbatim so the card swaps in place.
|
||||
assert_eq!(meta.id, "reused-id");
|
||||
// And the meta is actually persisted under that id.
|
||||
let got = get_artifact(tmp.path(), "reused-id").await.unwrap();
|
||||
assert_eq!(got.id, "reused-id");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn regenerate_preserves_original_created_at() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let workspace = tmp.path().to_path_buf();
|
||||
|
||||
// First create stamps `created_at = now`.
|
||||
let (first, _) = create_artifact(tmp.path(), ArtifactKind::Presentation, "Deck", "pptx")
|
||||
.await
|
||||
.unwrap();
|
||||
let original_created = first.created_at;
|
||||
|
||||
// Regenerate reuses the id; created_at must NOT be bumped, otherwise the
|
||||
// artifact jumps to the top of the created_at-sorted list (#3162).
|
||||
let id = first.id.clone();
|
||||
let ws = workspace.clone();
|
||||
let (second, _) = REGENERATE_TARGET_ID
|
||||
.scope(id.clone(), async move {
|
||||
create_artifact(&ws, ArtifactKind::Presentation, "Deck", "pptx").await
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(second.id, id);
|
||||
assert_eq!(
|
||||
second.created_at, original_created,
|
||||
"regenerate must preserve created_at, not bump it"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -244,6 +244,25 @@ impl Tool for PresentationTool {
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
|
||||
// Persist the verbatim args next to meta.json so a failed card's
|
||||
// Retry can re-dispatch this exact spec deterministically (#3162).
|
||||
// Best-effort: a write failure only forfeits future regeneration,
|
||||
// it must not abort an otherwise-successful generation.
|
||||
if let Err(err) = crate::openhuman::artifacts::store::save_artifact_args(
|
||||
&self.workspace_dir,
|
||||
&meta.id,
|
||||
&args,
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(
|
||||
target: "presentation",
|
||||
err = %err,
|
||||
artifact_id = %meta.id,
|
||||
"[presentation] failed to persist args.json; artifact will not be regenerable"
|
||||
);
|
||||
}
|
||||
|
||||
let bytes = match engine::generate(&input, &resolved_images, GENERATION_TIMEOUT).await {
|
||||
Ok(bytes) => bytes,
|
||||
Err(err) => {
|
||||
|
||||
Reference in New Issue
Block a user