diff --git a/app/src-tauri/capabilities/default.json b/app/src-tauri/capabilities/default.json index c19e0aa3b1..b922e72899 100644 --- a/app/src-tauri/capabilities/default.json +++ b/app/src-tauri/capabilities/default.json @@ -31,6 +31,7 @@ "updater:default", "allow-core-process", "allow-workspace-files", + "allow-artifact-download", "allow-app-update", "allow-loopback-oauth" ] diff --git a/app/src-tauri/permissions/allow-artifact-download.toml b/app/src-tauri/permissions/allow-artifact-download.toml new file mode 100644 index 0000000000..d74afdc60a --- /dev/null +++ b/app/src-tauri/permissions/allow-artifact-download.toml @@ -0,0 +1,11 @@ +[[permission]] +identifier = "allow-artifact-download" +description = "Allow copying agent-generated artifacts from the workspace artifacts/ tree to the user's Downloads folder (#2779)" + +[permission.commands] + +allow = [ + "download_artifact_to_downloads", +] + +deny = [] diff --git a/app/src-tauri/src/artifact_commands.rs b/app/src-tauri/src/artifact_commands.rs new file mode 100644 index 0000000000..d11dbfd893 --- /dev/null +++ b/app/src-tauri/src/artifact_commands.rs @@ -0,0 +1,243 @@ +//! Tauri command for downloading agent-generated artifacts (#2779). +//! +//! 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: +//! +//! 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). +//! +//! 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. + +use std::path::{Path, PathBuf}; + +/// 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. +const MAX_COLLISION_SUFFIX: u32 = 1000; + +#[tauri::command] +pub async fn download_artifact_to_downloads( + source_path: String, + filename: String, +) -> Result { + if source_path.trim().is_empty() { + return Err("source_path must not be empty".to_string()); + } + 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() + .and_then(|u| u.download_dir().map(|p| p.to_path_buf())) + .ok_or_else(|| "OS Downloads directory not resolvable".to_string())?; + tokio::fs::create_dir_all(&downloads) + .await + .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))?; + + log::info!( + "[artifact_commands] download_artifact_to_downloads bytes={bytes} dest={}", + dest.display() + ); + Ok(dest.display().to_string()) +} + +/// 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. +fn sanitize_filename(name: &str) -> Result { + let trimmed = name.trim(); + if trimmed.is_empty() { + return Err("filename must not be empty after trim".to_string()); + } + if trimmed.contains('/') || trimmed.contains('\\') { + return Err(format!( + "filename must not contain path separators: {trimmed:?}" + )); + } + if trimmed.contains('\0') { + return Err(format!("filename must not contain NUL bytes: {trimmed:?}")); + } + if trimmed == "." || trimmed == ".." { + return Err(format!("filename must not be '.' or '..': {trimmed:?}")); + } + Ok(trimmed.to_string()) +} + +/// Pick a destination path under `dir` that does not exist yet. +/// Inserts ` (N)` between the stem and the extension. Falls back to +/// a UUID suffix after [`MAX_COLLISION_SUFFIX`] tries. +fn pick_unique_path(dir: &Path, filename: &str) -> PathBuf { + let candidate = dir.join(filename); + if !candidate.exists() { + return candidate; + } + let (stem, ext) = split_stem_ext(filename); + for n in 1..=MAX_COLLISION_SUFFIX { + let nth = if ext.is_empty() { + format!("{stem} ({n})") + } else { + format!("{stem} ({n}).{ext}") + }; + let path = dir.join(&nth); + if !path.exists() { + return path; + } + } + // 1000 collisions is implausible in practice; if we hit it, fall + // back to a monotonic nanosecond suffix so the copy still succeeds + // without overwriting anything. Reaches for the OS clock instead of + // pulling in `uuid` as a Tauri-shell dep just for this corner. + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let with_uniq = if ext.is_empty() { + format!("{stem}-{nanos}") + } else { + format!("{stem}-{nanos}.{ext}") + }; + dir.join(with_uniq) +} + +fn split_stem_ext(filename: &str) -> (String, String) { + if let Some(idx) = filename.rfind('.') { + // Reject leading-dot files (`.hidden`) — treat as having no extension. + if idx > 0 && idx < filename.len() - 1 { + return (filename[..idx].to_string(), filename[idx + 1..].to_string()); + } + } + (filename.to_string(), String::new()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sanitize_rejects_path_separators() { + assert!(sanitize_filename("../etc/passwd").is_err()); + assert!(sanitize_filename("a\\b.pptx").is_err()); + assert!(sanitize_filename("a/b.pptx").is_err()); + assert!(sanitize_filename("").is_err()); + assert!(sanitize_filename(".").is_err()); + assert!(sanitize_filename("..").is_err()); + assert!(sanitize_filename("ok.pptx\0").is_err()); + } + + #[test] + fn sanitize_accepts_plain_names() { + assert_eq!( + sanitize_filename("Quarterly Update.pptx").unwrap(), + "Quarterly Update.pptx" + ); + assert_eq!(sanitize_filename(" trim me ").unwrap(), "trim me"); + } + + #[test] + fn split_stem_ext_pairs() { + assert_eq!( + split_stem_ext("file.pptx"), + ("file".to_string(), "pptx".to_string()) + ); + assert_eq!( + split_stem_ext("noext"), + ("noext".to_string(), String::new()) + ); + assert_eq!( + split_stem_ext(".hidden"), + (".hidden".to_string(), String::new()) + ); + assert_eq!( + split_stem_ext("trailing."), + ("trailing.".to_string(), String::new()) + ); + assert_eq!( + split_stem_ext("a.b.c"), + ("a.b".to_string(), "c".to_string()) + ); + } + + #[test] + fn pick_unique_inserts_collision_suffix() { + let temp = tempfile::tempdir().unwrap(); + let dir = temp.path(); + let first = pick_unique_path(dir, "deck.pptx"); + assert_eq!(first, dir.join("deck.pptx")); + + std::fs::write(&first, b"").unwrap(); + let second = pick_unique_path(dir, "deck.pptx"); + assert_eq!(second, dir.join("deck (1).pptx")); + + std::fs::write(&second, b"").unwrap(); + let third = pick_unique_path(dir, "deck.pptx"); + assert_eq!(third, dir.join("deck (2).pptx")); + } + + #[test] + fn pick_unique_handles_no_extension() { + let temp = tempfile::tempdir().unwrap(); + let dir = temp.path(); + let first = pick_unique_path(dir, "noext"); + assert_eq!(first, dir.join("noext")); + std::fs::write(&first, b"").unwrap(); + let second = pick_unique_path(dir, "noext"); + assert_eq!(second, dir.join("noext (1)")); + } + + #[tokio::test] + async fn download_rejects_invalid_inputs() { + assert!( + download_artifact_to_downloads(String::new(), "x.pptx".to_string()) + .await + .is_err() + ); + assert!( + download_artifact_to_downloads("/tmp/x".to_string(), String::new()) + .await + .is_err() + ); + assert!( + download_artifact_to_downloads("relative".to_string(), "x.pptx".to_string()) + .await + .is_err() + ); + assert!( + download_artifact_to_downloads("/nope".to_string(), "../escape.pptx".to_string()) + .await + .is_err() + ); + } +} diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 66833576eb..d25b105f32 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -3,8 +3,9 @@ #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))] compile_error!("src-tauri host supports desktop (Windows/macOS/Linux) only. Mobile lives in app/src-tauri-mobile."); -mod cdp; #[cfg(any(target_os = "macos", target_os = "linux"))] +mod artifact_commands; +mod cdp; mod cef_preflight; mod cef_profile; mod companion_commands; @@ -3081,6 +3082,12 @@ pub fn run() { core_rpc_token, 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_commands::download_artifact_to_downloads, check_core_update, apply_core_update, check_app_update, diff --git a/app/src/components/chat/ArtifactCard.test.tsx b/app/src/components/chat/ArtifactCard.test.tsx new file mode 100644 index 0000000000..61a21ebef2 --- /dev/null +++ b/app/src/components/chat/ArtifactCard.test.tsx @@ -0,0 +1,281 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { ArtifactSnapshot } from '../../store/chatRuntimeSlice'; +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 revealArtifactInFileManagerMock = vi.fn(); +vi.mock('../../services/artifactDownloadService', () => ({ + downloadArtifact: (...args: unknown[]) => downloadArtifactMock(...args), + revealArtifactInFileManager: (...args: unknown[]) => revealArtifactInFileManagerMock(...args), +})); + +function snapshot(overrides: Partial = {}): ArtifactSnapshot { + return { + artifactId: 'a-1', + kind: 'presentation', + title: 'Quarterly Deck', + status: 'ready', + sizeBytes: 4096, + path: 'a-1/deck.pptx', + updatedAt: 0, + ...overrides, + }; +} + +beforeEach(() => { + downloadArtifactMock.mockReset(); + revealArtifactInFileManagerMock.mockReset(); +}); + +describe(' — in_progress state', () => { + it('renders the title with the generating sub-label and no buttons', () => { + render( + + ); + + expect(screen.getByText('Quarterly Deck')).toBeInTheDocument(); + expect(screen.getByText(/Generating presentation/i)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /download/i })).not.toBeInTheDocument(); + }); + + it('reflects the artifact kind in the generating label', () => { + render( + + ); + + expect(screen.getByText(/Generating image/i)).toBeInTheDocument(); + }); +}); + +describe(' — ready state', () => { + it('renders the Download button with the formatted size', () => { + render(); + + expect(screen.getByRole('button', { name: 'Download' })).toBeEnabled(); + // formatFileSize on 4096 should land in the "4 KB"-ish range; we + // just assert the size cell exists rather than asserting an exact + // formatter output to avoid coupling to its implementation detail. + const subLabel = screen.getByText(/Ready ·/); + expect(subLabel.textContent ?? '').toMatch(/[KMG]?B/); + }); + + it('omits the size suffix when sizeBytes is null', () => { + render(); + + // Status text should be empty when sizeBytes is missing on a ready + // artifact — the conditional collapses to ''. + const dl = screen.getByRole('button', { name: 'Download' }); + expect(dl).toBeInTheDocument(); + }); + + it('drives the download → done flow and reveals on click', async () => { + downloadArtifactMock.mockResolvedValueOnce({ + ok: true, + path: '/Users/me/Downloads/Quarterly Deck.pptx', + }); + revealArtifactInFileManagerMock.mockResolvedValueOnce(true); + + render(); + + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + + // 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'); + + // Saved-to row appears with the reveal button. + await screen.findByText( + (_, el) => el?.textContent === 'Saved to /Users/me/Downloads/Quarterly Deck.pptx' + ); + + fireEvent.click(screen.getByRole('button', { name: 'Show in folder' })); + await waitFor(() => expect(revealArtifactInFileManagerMock).toHaveBeenCalledTimes(1)); + expect(revealArtifactInFileManagerMock).toHaveBeenCalledWith( + '/Users/me/Downloads/Quarterly Deck.pptx' + ); + + // Download button is gone now that state === 'done'. + expect(screen.queryByRole('button', { name: 'Download' })).not.toBeInTheDocument(); + }); + + it('falls back to the kind-based extension when title has no dot', async () => { + downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/Doc.pdf' }); + render( + + ); + + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1)); + expect(downloadArtifactMock).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' }); + render( + + ); + + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1)); + expect(downloadArtifactMock).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' }); + render( + + ); + + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1)); + expect(downloadArtifactMock).toHaveBeenCalledWith('a-4', 'Blob', 'bin'); + }); + + it('uses the existing extension on the title when present', async () => { + downloadArtifactMock.mockResolvedValueOnce({ ok: true, path: '/tmp/notes.txt' }); + render( + + ); + + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + await waitFor(() => expect(downloadArtifactMock).toHaveBeenCalledTimes(1)); + expect(downloadArtifactMock).toHaveBeenCalledWith('a-5', 'notes.txt', 'txt'); + }); + + it('surfaces the service error message when download fails', async () => { + downloadArtifactMock.mockResolvedValueOnce({ ok: false, error: 'disk full' }); + + render(); + fireEvent.click(screen.getByRole('button', { name: 'Download' })); + + await screen.findByText((_, el) => el?.textContent === 'Download failed: disk full'); + // Download button stays so the user can retry. + expect(screen.getByRole('button', { name: 'Download' })).toBeEnabled(); + }); + + it('does not call reveal when download.path is missing', async () => { + // Synthetic edge: the success path always carries a path, but if the + // user clicks reveal before a download ever happened the handler is + // guarded. + render(); + // No reveal button is rendered until state === 'done'. + expect(screen.queryByRole('button', { name: 'Show in folder' })).not.toBeInTheDocument(); + expect(revealArtifactInFileManagerMock).not.toHaveBeenCalled(); + }); +}); + +describe(' — failed state', () => { + it('shows the producer error verbatim under the preview cap', () => { + render( + + ); + + expect(screen.getByText('producer crashed')).toBeInTheDocument(); + expect(screen.getByText(/Generation failed/i)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /show more/i })).not.toBeInTheDocument(); + }); + + it('truncates long errors with a Show more toggle', () => { + const huge = 'x'.repeat(400); + render( + + ); + + // Truncated body ends with the ellipsis suffix. + const truncated = screen.getByText(/x…$/); + expect(truncated).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Show more' })); + // After expand, the full string is rendered without the ellipsis. + expect(screen.getByText(huge)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Show less' })); + // Re-collapses back to truncated form. + expect(screen.getByText(/x…$/)).toBeInTheDocument(); + }); + + it('renders the Retry button when onRetry is provided', () => { + const onRetry = vi.fn(); + render( + + ); + + const retry = screen.getByRole('button', { name: 'Retry' }); + fireEvent.click(retry); + expect(onRetry).toHaveBeenCalledWith('a-1'); + }); + + it('omits the Retry button when onRetry is not provided', () => { + render( + + ); + expect(screen.queryByRole('button', { name: 'Retry' })).not.toBeInTheDocument(); + }); + + it('omits the error block entirely when no error string is set', () => { + render( + + ); + expect(screen.getByText(/Generation failed/i)).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Show more' })).not.toBeInTheDocument(); + }); +}); + +describe(' — aria label', () => { + it('exposes an aria-label with the artifact title', () => { + render(); + expect(screen.getByRole('group', { name: /Q3 Plan/ })).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/chat/ArtifactCard.tsx b/app/src/components/chat/ArtifactCard.tsx new file mode 100644 index 0000000000..1d62217b19 --- /dev/null +++ b/app/src/components/chat/ArtifactCard.tsx @@ -0,0 +1,272 @@ +import { useState } from 'react'; + +import { formatFileSize } from '../../lib/attachments'; +import { useT } from '../../lib/i18n/I18nContext'; +import { + downloadArtifact, + revealArtifactInFileManager, +} from '../../services/artifactDownloadService'; +import type { ArtifactSnapshot } from '../../store/chatRuntimeSlice'; + +/** + * Inline chat card surfacing a single agent-generated artifact (#2779). + * + * Renders three visual states keyed off `artifact.status`: + * + * - `in_progress` — pulsing dot + title + "Generating …" label. + * Derived state: a `ChatToolCallEvent` for an artifact-producing + * tool was seen but no `artifact_ready` / `artifact_failed` has + * landed yet. + * - `ready` — kind icon + title + human-readable size + Download + * button. Click → `downloadArtifact()` → "Saved to …" w/ a + * "Show in folder" link. + * - `failed` — error icon + title + producer-supplied reason + + * optional Retry button (only when `onRetry` is provided). + * + * Visual style mirrors `ApprovalRequestCard` / `AttachmentPreview`: + * rounded card, dark/light Tailwind variants, mono accents on + * numeric values, inline SVG icons. No new icon dependency. + */ +export interface ArtifactCardProps { + artifact: ArtifactSnapshot; + /** When provided, render a Retry button on the `failed` state. */ + onRetry?: (artifactId: string) => void; +} + +/** + * Extension hint for the Tauri download command. Falls back to a + * lowercased kind slug when the title doesn't carry an explicit + * extension (defensive — `create_artifact` sanitises the title + + * extension separately, but a malformed title shouldn't crash the + * card). + */ +function extensionFor(kind: ArtifactSnapshot['kind'], title: string): string { + const dot = title.lastIndexOf('.'); + if (dot > 0 && dot < title.length - 1) { + return title.slice(dot + 1).toLowerCase(); + } + switch (kind) { + case 'presentation': + return 'pptx'; + case 'document': + return 'pdf'; + case 'image': + return 'png'; + default: + return 'bin'; + } +} + +function KindIcon({ kind }: { kind: ArtifactSnapshot['kind'] }) { + const stroke = 'currentColor'; + switch (kind) { + case 'presentation': + return ( + + ); + case 'document': + return ( + + ); + case 'image': + return ( + + ); + default: + return ( + + ); + } +} + +function Spinner() { + return ( + + ); +} + +function FailedIcon() { + return ( + + ); +} + +/** + * Cap the visible failure reason at ~280 chars. Producer-side errors + * can be enormous (e.g. a multi-KB pip stderr from a failed venv + * setup — observed at 13K chars in dev:app on 2026-05-30) and + * dumping that raw into a flex card breaks layout + can freeze the + * scrolling page. We collapse by default and let the user expand + * via "Show more" if they actually want to read it. + */ +const ERROR_REASON_PREVIEW_CHARS = 280; + +export default function ArtifactCard({ artifact, onRetry }: ArtifactCardProps) { + const { t } = useT(); + const [download, setDownload] = useState<{ + state: 'idle' | 'downloading' | 'done' | 'error'; + path?: string; + error?: string; + }>({ state: 'idle' }); + const [errorExpanded, setErrorExpanded] = useState(false); + + const handleDownload = async () => { + setDownload({ state: 'downloading' }); + const ext = extensionFor(artifact.kind, artifact.title); + const outcome = await downloadArtifact(artifact.artifactId, artifact.title, ext); + if (outcome.ok) { + setDownload({ state: 'done', path: outcome.path }); + } else { + setDownload({ state: 'error', error: outcome.error }); + } + }; + + const handleReveal = async () => { + if (download.path) { + await revealArtifactInFileManager(download.path); + } + }; + + return ( +
+
+ {artifact.status === 'in_progress' ? ( + + ) : artifact.status === 'failed' ? ( + + ) : ( + + )} +
+ {artifact.title} + + {artifact.status === 'in_progress' + ? t('chat.artifact.generating').replace('{kind}', artifact.kind) + : artifact.status === 'ready' && artifact.sizeBytes != null + ? `${t('chat.artifact.ready')} · ${formatFileSize(artifact.sizeBytes)}` + : artifact.status === 'failed' + ? t('chat.artifact.failed') + : ''} + +
+ {artifact.status === 'ready' && download.state !== 'done' && ( + + )} + {artifact.status === 'failed' && onRetry && ( + + )} +
+ {artifact.status === 'failed' && artifact.error && ( +
+

+ {errorExpanded || artifact.error.length <= ERROR_REASON_PREVIEW_CHARS + ? artifact.error + : `${artifact.error.slice(0, ERROR_REASON_PREVIEW_CHARS)}…`} +

+ {artifact.error.length > ERROR_REASON_PREVIEW_CHARS && ( + + )} +
+ )} + {download.state === 'done' && download.path && ( +
+ + {t('chat.artifact.downloaded').replace('{path}', download.path)} + + +
+ )} + {download.state === 'error' && download.error && ( +

+ {t('chat.artifact.download_failed').replace('{reason}', download.error)} +

+ )} +
+ ); +} diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 7a198be695..b330a6c6e3 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -4385,6 +4385,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'رفض التخزين المحلي', 'pages.settings.account.security': 'الأمان', 'pages.settings.account.securityDesc': 'وضع تخزين الأسرار وحالة سلسلة المفاتيح', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'الملف: {title}', + 'chat.artifact.generating': 'جارٍ إنشاء {kind}…', + 'chat.artifact.ready': 'جاهز', + 'chat.artifact.failed': 'فشل الإنشاء', + 'chat.artifact.download': 'تنزيل', + 'chat.artifact.downloading': 'جارٍ التنزيل…', + 'chat.artifact.downloaded': 'تم الحفظ في {path}', + 'chat.artifact.download_failed': 'فشل التنزيل: {reason}', + 'chat.artifact.retry': 'إعادة المحاولة', + 'chat.artifact.reveal': 'عرض في المجلد', + 'chat.artifact.show_more': 'عرض المزيد', + 'chat.artifact.show_less': 'عرض أقل', // Chat composer toolbar 'composer.attachFile': 'إرفاق ملف', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index dad8d14fe3..bbfbd2cac2 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -4463,6 +4463,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'স্থানীয় সঞ্চয়স্থান প্রত্যাখ্যান করুন', 'pages.settings.account.security': 'নিরাপত্তা', 'pages.settings.account.securityDesc': 'গোপনীয়তা সঞ্চয়স্থান মোড এবং কিচেন অবস্থা', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'আর্টিফ্যাক্ট: {title}', + 'chat.artifact.generating': '{kind} তৈরি হচ্ছে…', + 'chat.artifact.ready': 'প্রস্তুত', + 'chat.artifact.failed': 'তৈরি ব্যর্থ হয়েছে', + 'chat.artifact.download': 'ডাউনলোড', + 'chat.artifact.downloading': 'ডাউনলোড হচ্ছে…', + 'chat.artifact.downloaded': '{path} এ সংরক্ষিত', + 'chat.artifact.download_failed': 'ডাউনলোড ব্যর্থ: {reason}', + 'chat.artifact.retry': 'পুনরায় চেষ্টা', + 'chat.artifact.reveal': 'ফোল্ডারে দেখান', + 'chat.artifact.show_more': 'আরও দেখুন', + 'chat.artifact.show_less': 'কম দেখুন', // Chat composer toolbar 'composer.attachFile': 'ফাইল সংযুক্ত করুন', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 031e879419..fa5dea0dc6 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -4580,6 +4580,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Lokalen Speicher ablehnen', 'pages.settings.account.security': 'Sicherheit', 'pages.settings.account.securityDesc': 'Geheimnisspeicher-Modus und Schlüsselbund-Status', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefakt: {title}', + 'chat.artifact.generating': 'Erstelle {kind}…', + 'chat.artifact.ready': 'Bereit', + 'chat.artifact.failed': 'Erstellung fehlgeschlagen', + 'chat.artifact.download': 'Herunterladen', + 'chat.artifact.downloading': 'Wird heruntergeladen…', + 'chat.artifact.downloaded': 'Gespeichert unter {path}', + 'chat.artifact.download_failed': 'Download fehlgeschlagen: {reason}', + 'chat.artifact.retry': 'Erneut versuchen', + 'chat.artifact.reveal': 'Im Ordner anzeigen', + 'chat.artifact.show_more': 'Mehr anzeigen', + 'chat.artifact.show_less': 'Weniger anzeigen', // Chat composer toolbar 'composer.attachFile': 'Datei anhängen', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 1bd2e4e2cb..557b984404 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -4788,6 +4788,19 @@ const en: TranslationMap = { 'pages.settings.account.security': 'Security', 'pages.settings.account.securityDesc': 'Secret storage mode and keychain status', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artifact: {title}', + 'chat.artifact.generating': 'Generating {kind}…', + 'chat.artifact.ready': 'Ready', + 'chat.artifact.failed': 'Generation failed', + 'chat.artifact.download': 'Download', + 'chat.artifact.downloading': 'Downloading…', + 'chat.artifact.downloaded': 'Saved to {path}', + 'chat.artifact.download_failed': 'Download failed: {reason}', + 'chat.artifact.retry': 'Retry', + 'chat.artifact.reveal': 'Show in folder', + 'chat.artifact.show_more': 'Show more', + 'chat.artifact.show_less': 'Show less', // Chat composer toolbar 'composer.attachFile': 'Attach file', 'composer.modelSelector': 'Model', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index 7fe1be1e7b..6782368679 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -4546,6 +4546,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Rechazar almacenamiento local', 'pages.settings.account.security': 'Seguridad', 'pages.settings.account.securityDesc': 'Modo de almacenamiento de secretos y estado del llavero', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefacto: {title}', + 'chat.artifact.generating': 'Generando {kind}…', + 'chat.artifact.ready': 'Listo', + 'chat.artifact.failed': 'Error al generar', + 'chat.artifact.download': 'Descargar', + 'chat.artifact.downloading': 'Descargando…', + 'chat.artifact.downloaded': 'Guardado en {path}', + 'chat.artifact.download_failed': 'Error al descargar: {reason}', + 'chat.artifact.retry': 'Reintentar', + 'chat.artifact.reveal': 'Mostrar en la carpeta', + 'chat.artifact.show_more': 'Ver más', + 'chat.artifact.show_less': 'Ver menos', // Chat composer toolbar 'composer.attachFile': 'Adjuntar archivo', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index 47879577dd..fac5846359 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -4562,6 +4562,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Refuser le stockage local', 'pages.settings.account.security': 'Sécurité', 'pages.settings.account.securityDesc': 'Mode de stockage des secrets et état du trousseau', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefact : {title}', + 'chat.artifact.generating': 'Génération de {kind}…', + 'chat.artifact.ready': 'Prêt', + 'chat.artifact.failed': 'Échec de la génération', + 'chat.artifact.download': 'Télécharger', + 'chat.artifact.downloading': 'Téléchargement…', + 'chat.artifact.downloaded': 'Enregistré dans {path}', + 'chat.artifact.download_failed': 'Échec du téléchargement : {reason}', + 'chat.artifact.retry': 'Réessayer', + 'chat.artifact.reveal': 'Afficher dans le dossier', + 'chat.artifact.show_more': 'Voir plus', + 'chat.artifact.show_less': 'Voir moins', // Chat composer toolbar 'composer.attachFile': 'Joindre un fichier', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 5b96f8439c..3826e52ae2 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -4470,6 +4470,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'स्थानीय भंडारण अस्वीकार करें', 'pages.settings.account.security': 'सुरक्षा', 'pages.settings.account.securityDesc': 'रहस्य भंडारण मोड और कीचेन स्थिति', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'आर्टिफैक्ट: {title}', + 'chat.artifact.generating': '{kind} बना रहा है…', + 'chat.artifact.ready': 'तैयार', + 'chat.artifact.failed': 'निर्माण विफल', + 'chat.artifact.download': 'डाउनलोड', + 'chat.artifact.downloading': 'डाउनलोड हो रहा है…', + 'chat.artifact.downloaded': '{path} में सहेजा गया', + 'chat.artifact.download_failed': 'डाउनलोड विफल: {reason}', + 'chat.artifact.retry': 'पुनः प्रयास', + 'chat.artifact.reveal': 'फ़ोल्डर में दिखाएं', + 'chat.artifact.show_more': 'और दिखाएं', + 'chat.artifact.show_less': 'कम दिखाएं', // Chat composer toolbar 'composer.attachFile': 'फ़ाइल संलग्न करें', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 5ac00f7680..3d90ffed0d 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -4480,6 +4480,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Tolak penyimpanan lokal', 'pages.settings.account.security': 'Keamanan', 'pages.settings.account.securityDesc': 'Mode penyimpanan rahasia dan status keychain', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefak: {title}', + 'chat.artifact.generating': 'Membuat {kind}…', + 'chat.artifact.ready': 'Siap', + 'chat.artifact.failed': 'Gagal dibuat', + 'chat.artifact.download': 'Unduh', + 'chat.artifact.downloading': 'Mengunduh…', + 'chat.artifact.downloaded': 'Disimpan ke {path}', + 'chat.artifact.download_failed': 'Unduhan gagal: {reason}', + 'chat.artifact.retry': 'Coba lagi', + 'chat.artifact.reveal': 'Tampilkan di folder', + 'chat.artifact.show_more': 'Tampilkan selengkapnya', + 'chat.artifact.show_less': 'Tampilkan lebih sedikit', // Chat composer toolbar 'composer.attachFile': 'Lampirkan file', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index c06f666e2d..01f4ea6281 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -4538,6 +4538,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Rifiuta archiviazione locale', 'pages.settings.account.security': 'Sicurezza', 'pages.settings.account.securityDesc': 'Modalità archiviazione segreti e stato del portachiavi', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefatto: {title}', + 'chat.artifact.generating': 'Generazione {kind}…', + 'chat.artifact.ready': 'Pronto', + 'chat.artifact.failed': 'Generazione fallita', + 'chat.artifact.download': 'Scarica', + 'chat.artifact.downloading': 'Scaricamento…', + 'chat.artifact.downloaded': 'Salvato in {path}', + 'chat.artifact.download_failed': 'Download fallito: {reason}', + 'chat.artifact.retry': 'Riprova', + 'chat.artifact.reveal': 'Mostra nella cartella', + 'chat.artifact.show_more': 'Mostra altro', + 'chat.artifact.show_less': 'Mostra meno', // Chat composer toolbar 'composer.attachFile': 'Allega file', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index f3575331f3..8ecb667f11 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -4427,6 +4427,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': '로컬 저장소 거부', 'pages.settings.account.security': '보안', 'pages.settings.account.securityDesc': '비밀 저장 모드 및 키체인 상태', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': '아티팩트: {title}', + 'chat.artifact.generating': '{kind} 생성 중…', + 'chat.artifact.ready': '준비됨', + 'chat.artifact.failed': '생성 실패', + 'chat.artifact.download': '다운로드', + 'chat.artifact.downloading': '다운로드 중…', + 'chat.artifact.downloaded': '{path}에 저장됨', + 'chat.artifact.download_failed': '다운로드 실패: {reason}', + 'chat.artifact.retry': '다시 시도', + 'chat.artifact.reveal': '폴더에서 보기', + 'chat.artifact.show_more': '더 보기', + 'chat.artifact.show_less': '간단히 보기', // Chat composer toolbar 'composer.attachFile': '파일 첨부', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index 1ebfa1600a..431e48fdf2 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -4537,6 +4537,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Odmów lokalnego przechowywania', 'pages.settings.account.security': 'Bezpieczeństwo', 'pages.settings.account.securityDesc': 'Tryb przechowywania sekretów i stan pęku kluczy', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefakt: {title}', + 'chat.artifact.generating': 'Tworzenie {kind}…', + 'chat.artifact.ready': 'Gotowe', + 'chat.artifact.failed': 'Tworzenie nie powiodło się', + 'chat.artifact.download': 'Pobierz', + 'chat.artifact.downloading': 'Pobieranie…', + 'chat.artifact.downloaded': 'Zapisano w {path}', + 'chat.artifact.download_failed': 'Pobieranie nie powiodło się: {reason}', + 'chat.artifact.retry': 'Spróbuj ponownie', + 'chat.artifact.reveal': 'Pokaż w folderze', + 'chat.artifact.show_more': 'Pokaż więcej', + 'chat.artifact.show_less': 'Pokaż mniej', // Chat composer toolbar 'composer.attachFile': 'Dołącz plik', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 854223a7fd..7caadee9dc 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -4536,6 +4536,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Recusar armazenamento local', 'pages.settings.account.security': 'Segurança', 'pages.settings.account.securityDesc': 'Modo de armazenamento de segredos e status do chaveiro', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Artefato: {title}', + 'chat.artifact.generating': 'Gerando {kind}…', + 'chat.artifact.ready': 'Pronto', + 'chat.artifact.failed': 'Falha ao gerar', + 'chat.artifact.download': 'Baixar', + 'chat.artifact.downloading': 'Baixando…', + 'chat.artifact.downloaded': 'Salvo em {path}', + 'chat.artifact.download_failed': 'Falha no download: {reason}', + 'chat.artifact.retry': 'Tentar novamente', + 'chat.artifact.reveal': 'Mostrar na pasta', + 'chat.artifact.show_more': 'Mostrar mais', + 'chat.artifact.show_less': 'Mostrar menos', // Chat composer toolbar 'composer.attachFile': 'Anexar arquivo', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 83a43f6029..2f6d9508a4 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -4506,6 +4506,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': 'Отклонить локальное хранилище', 'pages.settings.account.security': 'Безопасность', 'pages.settings.account.securityDesc': 'Режим хранения секретов и статус связки ключей', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': 'Артефакт: {title}', + 'chat.artifact.generating': 'Создание {kind}…', + 'chat.artifact.ready': 'Готово', + 'chat.artifact.failed': 'Сбой генерации', + 'chat.artifact.download': 'Скачать', + 'chat.artifact.downloading': 'Скачивание…', + 'chat.artifact.downloaded': 'Сохранено в {path}', + 'chat.artifact.download_failed': 'Сбой скачивания: {reason}', + 'chat.artifact.retry': 'Повторить', + 'chat.artifact.reveal': 'Показать в папке', + 'chat.artifact.show_more': 'Показать больше', + 'chat.artifact.show_less': 'Свернуть', // Chat composer toolbar 'composer.attachFile': 'Прикрепить файл', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index cb47796e8f..a314c70dec 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -4248,6 +4248,19 @@ const messages: TranslationMap = { 'keyring.settings.revokeConsent': '拒绝本地存储', 'pages.settings.account.security': '安全', 'pages.settings.account.securityDesc': '密钥存储模式和密钥链状态', + // Chat — agent-generated artifacts (#2779) + 'chat.artifact.aria': '工件:{title}', + 'chat.artifact.generating': '正在生成{kind}…', + 'chat.artifact.ready': '已就绪', + 'chat.artifact.failed': '生成失败', + 'chat.artifact.download': '下载', + 'chat.artifact.downloading': '下载中…', + 'chat.artifact.downloaded': '已保存到 {path}', + 'chat.artifact.download_failed': '下载失败:{reason}', + 'chat.artifact.retry': '重试', + 'chat.artifact.reveal': '在文件夹中显示', + 'chat.artifact.show_more': '显示更多', + 'chat.artifact.show_less': '收起', // Chat composer toolbar 'composer.attachFile': '附加文件', diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index 770bcc1545..479e45a78a 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -6,6 +6,7 @@ import { useLocation, useNavigate } from 'react-router-dom'; import { type ChatSendError, chatSendError } from '../chat/chatSendError'; import { checkPromptInjection, promptGuardMessage } from '../chat/promptInjectionGuard'; import ApprovalRequestCard from '../components/chat/ApprovalRequestCard'; +import ArtifactCard from '../components/chat/ArtifactCard'; import ChatComposer from '../components/chat/ChatComposer'; import { ConfirmationModal } from '../components/intelligence/ConfirmationModal'; import PillTabBar from '../components/PillTabBar'; @@ -230,6 +231,7 @@ const Conversations = ({ const inferenceStatusByThread = useAppSelector( state => state.chatRuntime.inferenceStatusByThread ); + const artifactsByThread = useAppSelector(state => state.chatRuntime.artifactsByThread); const pendingApprovalByThread = useAppSelector( state => state.chatRuntime.pendingApprovalByThread ); @@ -2092,6 +2094,43 @@ const Conversations = ({ ) : null; })()} + {(() => { + // Surface artifact cards for the shown thread above the composer + // (#2779). Mirrors the approval-card placement so the user sees + // the just-generated deck without scrolling. Cards stay visible + // across turns until the thread is cleared. ArtifactCard handles + // its own download lifecycle (dialog → copy → "Saved to …"). + const artifactThreadId = selectedThreadId ?? activeThreadId; + const artifacts = artifactThreadId ? (artifactsByThread[artifactThreadId] ?? []) : []; + if (artifacts.length === 0) return null; + return ( +
+ {artifacts.map(artifact => ( + // NOTE: two intentionally-deferred surface gaps live here, + // both tracked in follow-up issue #3162: + // + // 1. `onRetry` is intentionally omitted — `ArtifactCard` + // declares the prop as optional and renders a Retry + // button only when it's wired. Real retry (either + // `removeArtifact(thread, id)` to let the user + // re-prompt, or full re-dispatch of the producing + // tool call) is out of scope for #2779. The + // failed-card UI still surfaces the truncated error + // reason; the button just stays hidden until #3162. + // + // 2. The card's in-progress / "generating…" state is + // unreachable from this call site today — we only + // push an `ArtifactSnapshot` into `artifactsByThread` + // on `ArtifactReady` / `ArtifactFailed`, not on the + // earlier `ChatToolCallEvent` that fires when the + // agent dispatches `generate_presentation`. Wiring + // that event through is the other half of #3162. + + ))} +
+ ); + })()} + {composer === 'mic-cloud' ? (
{ } }); }, + onArtifactReady: event => { + rtLog('artifact_ready', { + thread: event.thread_id, + artifact_id: event.artifact_id, + kind: event.kind, + size_bytes: event.size_bytes, + }); + dispatch( + upsertArtifactReadyForThread({ + threadId: event.thread_id, + artifactId: event.artifact_id, + kind: event.kind, + title: event.title, + path: event.path, + sizeBytes: event.size_bytes, + }) + ); + }, + onArtifactFailed: event => { + // Defence-in-depth: producer is expected to pre-truncate the + // reason, but cap again here so a leaky producer cannot dump + // unbounded provider stderr into client telemetry. + rtLog('artifact_failed', { + thread: event.thread_id, + artifact_id: event.artifact_id, + kind: event.kind, + error: event.error.slice(0, 80), + }); + dispatch( + upsertArtifactFailedForThread({ + threadId: event.thread_id, + artifactId: event.artifact_id, + kind: event.kind, + title: event.title, + error: event.error, + }) + ); + }, onApprovalRequest: (event: ChatApprovalRequestEvent) => { rtLog('approval_request', { thread: event.thread_id, diff --git a/app/src/providers/__tests__/ChatRuntimeProvider.artifacts.test.tsx b/app/src/providers/__tests__/ChatRuntimeProvider.artifacts.test.tsx new file mode 100644 index 0000000000..dace21e7c6 --- /dev/null +++ b/app/src/providers/__tests__/ChatRuntimeProvider.artifacts.test.tsx @@ -0,0 +1,202 @@ +import { render } from '@testing-library/react'; +import { act } from 'react'; +import { Provider } from 'react-redux'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import * as chatService from '../../services/chatService'; +import { threadApi } from '../../services/api/threadApi'; +import { store } from '../../store'; +import { clearAllChatRuntime } from '../../store/chatRuntimeSlice'; +import { setStatusForUser } from '../../store/socketSlice'; +import { clearAllThreads } from '../../store/threadSlice'; +import ChatRuntimeProvider from '../ChatRuntimeProvider'; + +vi.mock('../../services/chatService', async () => { + const actual = await vi.importActual('../../services/chatService'); + return { ...actual, subscribeChatEvents: vi.fn() }; +}); + +vi.mock('../../services/api/threadApi', () => ({ + threadApi: { + createNewThread: vi.fn(), + getThreads: vi.fn(), + getThreadMessages: vi.fn(), + appendMessage: vi.fn(), + generateTitleIfNeeded: vi.fn(), + updateMessage: vi.fn(), + deleteThread: vi.fn(), + purge: vi.fn(), + getTaskBoard: vi.fn(), + putTaskBoard: vi.fn(), + }, +})); + +vi.mock('../../hooks/usageRefresh', () => ({ requestUsageRefresh: vi.fn() })); + +const mockRefetchSnapshot = vi.fn(); +vi.mock('../../hooks/useRefetchSnapshotOnTurnEnd', () => ({ + useRefetchSnapshotOnTurnEnd: () => ({ refetch: mockRefetchSnapshot }), +})); + +function renderProvider(): chatService.ChatEventListeners { + let captured: chatService.ChatEventListeners = {}; + vi.mocked(chatService.subscribeChatEvents).mockImplementation(listeners => { + captured = listeners; + return () => {}; + }); + + store.dispatch(setStatusForUser({ userId: '__pending__', status: 'connected' })); + + render( + + +
+ + + ); + + return captured; +} + +function resetRuntimeState() { + store.dispatch(clearAllThreads()); + store.dispatch(clearAllChatRuntime()); + store.dispatch(setStatusForUser({ userId: '__pending__', status: 'disconnected' })); +} + +describe('ChatRuntimeProvider — artifact event dispatch (#2779)', () => { + beforeEach(() => { + vi.clearAllMocks(); + resetRuntimeState(); + vi.mocked(threadApi.getThreads).mockResolvedValue({ threads: [], count: 0 }); + }); + + it('onArtifactReady upserts a ready snapshot keyed on the artifact id', () => { + const listeners = renderProvider(); + + act(() => { + listeners.onArtifactReady?.({ + thread_id: 'thread-1', + artifact_id: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + size_bytes: 4096, + }); + }); + + const bucket = store.getState().chatRuntime.artifactsByThread['thread-1']; + expect(bucket).toHaveLength(1); + expect(bucket?.[0]).toMatchObject({ + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + sizeBytes: 4096, + status: 'ready', + }); + }); + + it('onArtifactFailed records the producer-supplied error', () => { + const listeners = renderProvider(); + + act(() => { + listeners.onArtifactFailed?.({ + thread_id: 'thread-2', + artifact_id: 'a-2', + kind: 'document', + title: 'Notes', + error: 'producer crashed', + }); + }); + + const bucket = store.getState().chatRuntime.artifactsByThread['thread-2']; + expect(bucket).toHaveLength(1); + expect(bucket?.[0]).toMatchObject({ + artifactId: 'a-2', + kind: 'document', + title: 'Notes', + status: 'failed', + error: 'producer crashed', + }); + }); + + it('onArtifactFailed dispatches the FULL error to the store even when huge', () => { + // The provider's rtLog truncates to 80 chars for telemetry, but the + // dispatched payload must keep the full reason so the ArtifactCard + // can offer "Show more". + const listeners = renderProvider(); + const huge = 'p'.repeat(500); + + act(() => { + listeners.onArtifactFailed?.({ + thread_id: 'thread-3', + artifact_id: 'a-3', + kind: 'presentation', + title: 'Deck', + error: huge, + }); + }); + + const entry = store.getState().chatRuntime.artifactsByThread['thread-3']?.[0]; + expect(entry?.error).toHaveLength(500); + expect(entry?.error).toBe(huge); + }); + + it('an artifact_failed → artifact_ready sequence promotes the snapshot in place', () => { + const listeners = renderProvider(); + + act(() => { + listeners.onArtifactFailed?.({ + thread_id: 'thread-4', + artifact_id: 'a-4', + kind: 'presentation', + title: 'Deck', + error: 'boom', + }); + }); + act(() => { + listeners.onArtifactReady?.({ + thread_id: 'thread-4', + artifact_id: 'a-4', + kind: 'presentation', + title: 'Deck', + path: 'a-4/deck.pptx', + size_bytes: 1024, + }); + }); + + const bucket = store.getState().chatRuntime.artifactsByThread['thread-4']; + expect(bucket).toHaveLength(1); + expect(bucket?.[0].status).toBe('ready'); + expect(bucket?.[0].error).toBeUndefined(); + }); + + it('keeps artifact buckets per-thread', () => { + const listeners = renderProvider(); + + act(() => { + listeners.onArtifactReady?.({ + thread_id: 'thread-a', + artifact_id: 'a-1', + kind: 'image', + title: 'Pic', + path: 'a-1/pic.png', + size_bytes: 1, + }); + listeners.onArtifactFailed?.({ + thread_id: 'thread-b', + artifact_id: 'a-2', + kind: 'document', + title: 'Doc', + error: 'denied', + }); + }); + + const state = store.getState().chatRuntime.artifactsByThread; + expect(state['thread-a']?.[0].status).toBe('ready'); + expect(state['thread-b']?.[0].status).toBe('failed'); + expect(state['thread-a']?.[0].artifactId).toBe('a-1'); + expect(state['thread-b']?.[0].artifactId).toBe('a-2'); + }); +}); diff --git a/app/src/services/__tests__/artifactDownloadService.test.ts b/app/src/services/__tests__/artifactDownloadService.test.ts new file mode 100644 index 0000000000..6eb46c56aa --- /dev/null +++ b/app/src/services/__tests__/artifactDownloadService.test.ts @@ -0,0 +1,203 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Importing AFTER mocks are registered so the service binds to the stubs. +import { downloadArtifact, revealArtifactInFileManager } from '../artifactDownloadService'; + +// Mock the Tauri common shim (safeInvoke + isTauri) so the service +// runs entirely in JS — no real Tauri IPC reach. +const invokeMock = vi.fn(); +const isTauriMock = vi.fn(() => true); +vi.mock('../../utils/tauriCommands/common', () => ({ + safeInvoke: (...args: unknown[]) => invokeMock(...args), + isTauri: () => isTauriMock(), +})); + +// Mock the core RPC client — the service only uses one method. +const callCoreRpcMock = vi.fn(); +vi.mock('../coreRpcClient', () => ({ + callCoreRpc: (...args: unknown[]) => callCoreRpcMock(...args), +})); + +// Mock the opener plugin's reveal binding so we can assert it is +// invoked with the canonical absolute-path argument. +const revealMock = vi.fn(); +vi.mock('@tauri-apps/plugin-opener', () => ({ + revealItemInDir: (...args: unknown[]) => revealMock(...args), +})); + +beforeEach(() => { + invokeMock.mockReset(); + callCoreRpcMock.mockReset(); + revealMock.mockReset(); + isTauriMock.mockReset(); + isTauriMock.mockReturnValue(true); +}); + +describe('downloadArtifact', () => { + it('refuses outside Tauri with a user-facing message', async () => { + isTauriMock.mockReturnValueOnce(false); + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'Downloads are only available in the desktop app' }); + expect(callCoreRpcMock).not.toHaveBeenCalled(); + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it('refuses an empty artifact id', async () => { + const out = await downloadArtifact(' ', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'artifact id missing' }); + expect(callCoreRpcMock).not.toHaveBeenCalled(); + }); + + it('surfaces a friendly error when ai_get_artifact rejects (Error)', async () => { + callCoreRpcMock.mockRejectedValueOnce(new Error('rpc dropped')); + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out.ok).toBe(false); + expect(out.error).toBe('failed to resolve artifact: rpc dropped'); + }); + + it('stringifies non-Error rejections from the core RPC', async () => { + callCoreRpcMock.mockRejectedValueOnce('boom'); + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out.ok).toBe(false); + expect(out.error).toBe('failed to resolve artifact: boom'); + }); + + it('returns a clear error when the core response lacks absolute_path', async () => { + callCoreRpcMock.mockResolvedValueOnce({ meta: { title: 'Deck' } }); + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'artifact path missing from core response' }); + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it('treats a null core response as missing path', async () => { + callCoreRpcMock.mockResolvedValueOnce(null); + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'artifact path missing from core response' }); + }); + + it('prefers the persisted title over the caller fallback', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/deck.pptx', + meta: { title: 'Real Title' }, + }); + invokeMock.mockResolvedValueOnce('/Users/me/Downloads/Real Title.pptx'); + + const out = await downloadArtifact('a-1', 'Caller Fallback', 'pptx'); + + expect(out).toEqual({ ok: true, path: '/Users/me/Downloads/Real Title.pptx' }); + expect(callCoreRpcMock).toHaveBeenCalledWith({ + method: 'openhuman.ai_get_artifact', + params: { artifact_id: 'a-1' }, + }); + expect(invokeMock).toHaveBeenCalledWith('download_artifact_to_downloads', { + sourcePath: '/workspace/artifacts/a-1/deck.pptx', + filename: 'Real Title.pptx', + }); + }); + + it('falls back to the caller-supplied title when meta.title is blank', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/deck.pptx', + meta: { title: ' ' }, + }); + invokeMock.mockResolvedValueOnce('/Users/me/Downloads/Deck.pptx'); + + await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(invokeMock).toHaveBeenCalledWith('download_artifact_to_downloads', { + sourcePath: '/workspace/artifacts/a-1/deck.pptx', + filename: 'Deck.pptx', + }); + }); + + it('uses the "artifact" placeholder when both title sources are empty', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/x.bin', + meta: {}, + }); + invokeMock.mockResolvedValueOnce('/Users/me/Downloads/artifact.bin'); + + await downloadArtifact('a-1', ' ', 'bin'); + expect(invokeMock).toHaveBeenCalledWith('download_artifact_to_downloads', { + sourcePath: '/workspace/artifacts/a-1/x.bin', + filename: 'artifact.bin', + }); + }); + + it('strips leading dots from the extension before appending', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/deck.pptx', + meta: { title: 'Deck' }, + }); + invokeMock.mockResolvedValueOnce('/Users/me/Downloads/Deck.pptx'); + + await downloadArtifact('a-1', 'Deck', '..pptx'); + expect(invokeMock).toHaveBeenCalledWith('download_artifact_to_downloads', { + sourcePath: '/workspace/artifacts/a-1/deck.pptx', + filename: 'Deck.pptx', + }); + }); + + it('omits the dot when extension is empty', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/raw', + meta: { title: 'Raw' }, + }); + invokeMock.mockResolvedValueOnce('/Users/me/Downloads/Raw'); + + await downloadArtifact('a-1', 'Raw', ' '); + expect(invokeMock).toHaveBeenCalledWith('download_artifact_to_downloads', { + sourcePath: '/workspace/artifacts/a-1/raw', + filename: 'Raw', + }); + }); + + it('returns the Tauri error when the copy command rejects', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/deck.pptx', + meta: { title: 'Deck' }, + }); + invokeMock.mockRejectedValueOnce(new Error('disk full')); + + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'disk full' }); + }); + + it('stringifies a non-Error Tauri rejection', async () => { + callCoreRpcMock.mockResolvedValueOnce({ + absolute_path: '/workspace/artifacts/a-1/deck.pptx', + meta: { title: 'Deck' }, + }); + invokeMock.mockRejectedValueOnce('nope'); + + const out = await downloadArtifact('a-1', 'Deck', 'pptx'); + expect(out).toEqual({ ok: false, error: 'nope' }); + }); +}); + +describe('revealArtifactInFileManager', () => { + it('no-ops outside Tauri', async () => { + isTauriMock.mockReturnValueOnce(false); + const ok = await revealArtifactInFileManager('/Users/me/Downloads/Deck.pptx'); + expect(ok).toBe(false); + expect(revealMock).not.toHaveBeenCalled(); + }); + + it('no-ops on an empty absolute path', async () => { + const ok = await revealArtifactInFileManager(' '); + expect(ok).toBe(false); + expect(revealMock).not.toHaveBeenCalled(); + }); + + it('routes through the typed plugin binding', async () => { + revealMock.mockResolvedValueOnce(undefined); + const ok = await revealArtifactInFileManager('/Users/me/Downloads/Deck.pptx'); + expect(ok).toBe(true); + expect(revealMock).toHaveBeenCalledWith('/Users/me/Downloads/Deck.pptx'); + }); + + it('swallows reveal failures and returns false', async () => { + revealMock.mockRejectedValueOnce(new Error('opener missing')); + const ok = await revealArtifactInFileManager('/Users/me/Downloads/Deck.pptx'); + expect(ok).toBe(false); + }); +}); diff --git a/app/src/services/__tests__/chatService.artifactEvents.test.ts b/app/src/services/__tests__/chatService.artifactEvents.test.ts new file mode 100644 index 0000000000..09a8a4642c --- /dev/null +++ b/app/src/services/__tests__/chatService.artifactEvents.test.ts @@ -0,0 +1,252 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { subscribeChatEvents } from '../chatService'; +import { socketService } from '../socketService'; + +vi.mock('../socketService', () => ({ socketService: { getSocket: vi.fn() } })); +vi.mock('../coreRpcClient', () => ({ callCoreRpc: vi.fn() })); + +type Handler = (...args: unknown[]) => void; + +function createMockSocket() { + const handlers = new Map(); + const on = vi.fn((event: string, cb: Handler) => { + const existing = handlers.get(event) ?? []; + existing.push(cb); + handlers.set(event, existing); + }); + const off = vi.fn((event: string, cb: Handler) => { + const existing = handlers.get(event) ?? []; + handlers.set( + event, + existing.filter(handler => handler !== cb) + ); + }); + const emit = (event: string, payload: unknown) => { + for (const handler of handlers.get(event) ?? []) { + handler(payload); + } + }; + return { id: 'socket-1', on, off, emit }; +} + +describe('chatService — artifact_ready / artifact_failed handlers (#2779)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('subscribes to artifact events under canonical snake_case names', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + + subscribeChatEvents({ onArtifactReady: () => {}, onArtifactFailed: () => {} }); + + const events = socket.on.mock.calls.map(call => call[0]); + expect(events).toEqual(['artifact_ready', 'artifact_failed']); + }); + + it('flattens the wire envelope into a typed ArtifactReadyEvent', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactReady = vi.fn(); + + subscribeChatEvents({ onArtifactReady }); + + socket.emit('artifact_ready', { + thread_id: 'thread-1', + client_id: 'web-x', + args: { + artifact_id: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + size_bytes: 4096, + }, + }); + + expect(onArtifactReady).toHaveBeenCalledTimes(1); + expect(onArtifactReady).toHaveBeenCalledWith({ + thread_id: 'thread-1', + client_id: 'web-x', + artifact_id: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + size_bytes: 4096, + }); + }); + + it('drops an artifact_ready payload missing required fields', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactReady = vi.fn(); + + subscribeChatEvents({ onArtifactReady }); + + // No args at all → skipped. + socket.emit('artifact_ready', { thread_id: 'thread-1' }); + // Missing path → skipped. + socket.emit('artifact_ready', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'document', title: 'Doc', size_bytes: 1 }, + }); + // Missing size_bytes → skipped. + socket.emit('artifact_ready', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'document', title: 'Doc', path: 'a-1/doc.pdf' }, + }); + // Missing artifact_id → skipped. + socket.emit('artifact_ready', { + thread_id: 'thread-1', + args: { kind: 'document', title: 'Doc', path: 'a-1/doc.pdf', size_bytes: 1 }, + }); + + expect(onArtifactReady).not.toHaveBeenCalled(); + }); + + it('rejects artifact_ready with an unknown kind (not in the allowlist)', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactReady = vi.fn(); + + subscribeChatEvents({ onArtifactReady }); + + socket.emit('artifact_ready', { + thread_id: 'thread-1', + args: { + artifact_id: 'a-1', + // not in {presentation, document, image, other} + kind: 'spreadsheet', + title: 'Deck', + path: 'a-1/deck.pptx', + size_bytes: 4096, + }, + }); + + expect(onArtifactReady).not.toHaveBeenCalled(); + }); + + it('accepts every allowlisted kind for artifact_ready', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactReady = vi.fn(); + + subscribeChatEvents({ onArtifactReady }); + + for (const kind of ['presentation', 'document', 'image', 'other'] as const) { + socket.emit('artifact_ready', { + thread_id: 'thread-1', + args: { artifact_id: `a-${kind}`, kind, title: 'X', path: `a-${kind}/file`, size_bytes: 1 }, + }); + } + + expect(onArtifactReady).toHaveBeenCalledTimes(4); + expect(onArtifactReady.mock.calls.map(c => (c[0] as { kind: string }).kind)).toEqual([ + 'presentation', + 'document', + 'image', + 'other', + ]); + }); + + it('flattens artifact_failed into a typed ArtifactFailedEvent', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactFailed = vi.fn(); + + subscribeChatEvents({ onArtifactFailed }); + + socket.emit('artifact_failed', { + thread_id: 'thread-1', + client_id: 'web-y', + args: { + artifact_id: 'a-1', + kind: 'presentation', + title: 'Deck', + error: 'pip install failed', + }, + }); + + expect(onArtifactFailed).toHaveBeenCalledTimes(1); + expect(onArtifactFailed).toHaveBeenCalledWith({ + thread_id: 'thread-1', + client_id: 'web-y', + artifact_id: 'a-1', + kind: 'presentation', + title: 'Deck', + error: 'pip install failed', + }); + }); + + it('drops an artifact_failed payload missing required fields', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactFailed = vi.fn(); + + subscribeChatEvents({ onArtifactFailed }); + + // Missing error + socket.emit('artifact_failed', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'presentation', title: 'Deck' }, + }); + // Missing artifact_id + socket.emit('artifact_failed', { + thread_id: 'thread-1', + args: { kind: 'presentation', title: 'Deck', error: 'x' }, + }); + // Missing title + socket.emit('artifact_failed', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'presentation', error: 'x' }, + }); + + expect(onArtifactFailed).not.toHaveBeenCalled(); + }); + + it('rejects artifact_failed with an unknown kind', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactFailed = vi.fn(); + + subscribeChatEvents({ onArtifactFailed }); + + socket.emit('artifact_failed', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'video', title: 'X', error: 'boom' }, + }); + + expect(onArtifactFailed).not.toHaveBeenCalled(); + }); + + it('preserves the full error string on the dispatched event even when huge', () => { + // The handler caps only the LOG line, not the dispatched payload. + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + const onArtifactFailed = vi.fn(); + + subscribeChatEvents({ onArtifactFailed }); + + const huge = 'x'.repeat(500); + socket.emit('artifact_failed', { + thread_id: 'thread-1', + args: { artifact_id: 'a-1', kind: 'presentation', title: 'Deck', error: huge }, + }); + + expect(onArtifactFailed).toHaveBeenCalledTimes(1); + const event = onArtifactFailed.mock.calls[0]![0] as { error: string }; + expect(event.error).toHaveLength(500); + expect(event.error).toBe(huge); + }); + + it('removes both artifact handlers on cleanup', () => { + const socket = createMockSocket(); + vi.mocked(socketService.getSocket).mockReturnValue(socket as never); + + const cleanup = subscribeChatEvents({ onArtifactReady: () => {}, onArtifactFailed: () => {} }); + cleanup(); + + const offEvents = socket.off.mock.calls.map(call => call[0]); + expect(offEvents).toEqual(['artifact_ready', 'artifact_failed']); + }); +}); diff --git a/app/src/services/artifactDownloadService.ts b/app/src/services/artifactDownloadService.ts new file mode 100644 index 0000000000..a3aabb18ce --- /dev/null +++ b/app/src/services/artifactDownloadService.ts @@ -0,0 +1,117 @@ +/** + * Artifact download service (#2779). + * + * Flow: + * + * 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). + * + * No-ops outside Tauri (browser dev preview) — the download flow only + * makes sense in the desktop shell. + */ +import { revealItemInDir } from '@tauri-apps/plugin-opener'; + +import { safeInvoke as invoke, isTauri } from '../utils/tauriCommands/common'; +import { callCoreRpc } from './coreRpcClient'; + +/** Outcome surfaced to the UI for a single download attempt. */ +export interface DownloadArtifactOutcome { + ok: boolean; + /** Absolute destination path when `ok === true`. */ + path?: string; + /** Short, user-facing error string when `ok === false`. */ + error?: string; +} + +/** + * Shape of the `data` field returned by the + * `openhuman.ai_get_artifact` JSON-RPC method. We pull only the + * fields we need; extra fields are tolerated. + */ +interface AiGetArtifactData { + absolute_path?: string; + /** Full ArtifactMeta nested under this key on the core RPC response. */ + meta?: { id?: string; title?: string; path?: string; kind?: string; status?: 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. + */ +export async function downloadArtifact( + artifactId: string, + fallbackTitle: string, + extension: string +): Promise { + if (!isTauri()) { + return { ok: false, error: 'Downloads are only available in the desktop app' }; + } + if (!artifactId.trim()) { + return { ok: false, error: 'artifact id missing' }; + } + + let resolved: AiGetArtifactData; + try { + const raw = await callCoreRpc({ + method: 'openhuman.ai_get_artifact', + params: { artifact_id: artifactId }, + }); + resolved = raw ?? {}; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + return { ok: false, error: `failed to resolve artifact: ${reason}` }; + } + + const sourcePath = resolved.absolute_path; + if (!sourcePath) { + return { ok: false, error: 'artifact path missing from core response' }; + } + + // Prefer the persisted title (came from create_artifact's + // sanitized stem) but fall back to the caller-supplied hint. + const title = resolved.meta?.title?.trim() || fallbackTitle.trim() || 'artifact'; + const ext = extension.trim().replace(/^\.+/, ''); + const filename = ext ? `${title}.${ext}` : title; + + try { + const dest = await invoke('download_artifact_to_downloads', { sourcePath, filename }); + return { ok: true, path: dest }; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + return { ok: false, error: reason }; + } +} + +/** + * Open the user's file manager pointed at the just-downloaded file. + * Uses the existing `opener:allow-reveal-item-in-dir` capability — + * no new permission needed. Returns `false` when not in Tauri or the + * invoke fails (caller usually ignores the result). + */ +export async function revealArtifactInFileManager(absolutePath: string): Promise { + if (!isTauri()) return false; + if (!absolutePath.trim()) return false; + try { + // Use the plugin's typed binding — the raw `invoke('plugin:opener| + // reveal_item_in_dir', { path })` shape silently no-ops because the + // plugin expects `{ paths: [absolutePath] }` (array). The binding + // handles the wrap. + await revealItemInDir(absolutePath); + return true; + } catch (err) { + // Swallow — reveal is best-effort, the file is already saved. + // eslint-disable-next-line no-console + console.warn('[artifact] revealItemInDir failed:', err); + return false; + } +} diff --git a/app/src/services/chatService.ts b/app/src/services/chatService.ts index a60b0fcf1c..ddce6c5e15 100644 --- a/app/src/services/chatService.ts +++ b/app/src/services/chatService.ts @@ -136,6 +136,48 @@ export interface ChatApprovalRequestEvent { args?: Record; } +/** + * Lowercase variant of the Rust `ArtifactKind` enum surfaced on + * artifact lifecycle socket events. Mirrors the slugs produced by + * `ArtifactKind::as_str()` in `src/openhuman/artifacts/types.rs`. + */ +export type ArtifactKind = 'presentation' | 'document' | 'image' | 'other'; + +/** + * Emitted when the core `artifacts::store::finalize_artifact` flips an + * artifact's status to `Ready`. The chat runtime upserts the snapshot + * keyed on `artifact_id` so the `ArtifactCard` can render in the + * message timeline with a download button (#2779). + */ +export interface ArtifactReadyEvent { + thread_id: string; + client_id?: string; + /** UUID of the artifact record. Use with `ai_get_artifact`. */ + artifact_id: string; + kind: ArtifactKind; + /** Human-readable title; also the on-disk filename stem. */ + title: string; + /** Relative path under `/artifacts/`, e.g. `/deck.pptx`. */ + path: string; + /** Final on-disk size in bytes. */ + size_bytes: number; +} + +/** + * Emitted when `artifacts::store::fail_artifact` flips an artifact to + * `Failed` after the producer surfaced a reason. The frontend swaps + * the in-flight card for a retry-hint view. + */ +export interface ArtifactFailedEvent { + thread_id: string; + client_id?: string; + artifact_id: string; + kind: ArtifactKind; + title: string; + /** Producer-supplied failure reason, already truncated. */ + error: string; +} + /** Emitted when the agent turn begins (before the first LLM call). */ export interface ChatInferenceStartEvent { thread_id: string; @@ -354,6 +396,8 @@ export interface ChatEventListeners { onTaskBoardUpdated?: (event: ChatTaskBoardUpdatedEvent) => void; onProactiveMessage?: (event: ProactiveMessageEvent) => void; onApprovalRequest?: (event: ChatApprovalRequestEvent) => void; + onArtifactReady?: (event: ArtifactReadyEvent) => void; + onArtifactFailed?: (event: ArtifactFailedEvent) => void; onDone?: (event: ChatDoneEvent) => void; onError?: (event: ChatErrorEvent) => void; } @@ -387,6 +431,8 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void { taskBoardUpdated: 'task_board_updated', proactiveMessage: 'proactive_message', approvalRequest: 'approval_request', + artifactReady: 'artifact_ready', + artifactFailed: 'artifact_failed', done: 'chat_done', error: 'chat_error', } as const; @@ -706,6 +752,113 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void { handlers.push([EVENTS.approvalRequest, cb]); } + // Artifact lifecycle events (#2779). The Rust subscriber in + // `channels/providers/web::ArtifactSurfaceSubscriber` packs the + // artifact payload into the generic `args` field of the wire + // envelope (kept the WebChannelEvent struct shape stable to avoid + // touching ~10 existing call sites with `..Default::default()`). + // Flatten back into the typed `ArtifactReadyEvent` / + // `ArtifactFailedEvent` shape so listeners get a clean contract. + const validArtifactKinds: ReadonlySet = new Set([ + 'presentation', + 'document', + 'image', + 'other', + ]); + const isValidArtifactKind = (k: unknown): k is ArtifactKind => + typeof k === 'string' && validArtifactKinds.has(k as ArtifactKind); + if (listeners.onArtifactReady) { + const cb = (payload: unknown) => { + const raw = payload as { + thread_id: string; + client_id?: string; + args?: { + artifact_id?: string; + kind?: ArtifactKind; + title?: string; + path?: string; + size_bytes?: number; + }; + }; + const args = raw.args ?? {}; + if ( + !args.artifact_id || + !isValidArtifactKind(args.kind) || + !args.title || + !args.path || + args.size_bytes == null + ) { + chatLog( + '%s thread_id=%s — skipping malformed payload (missing args)', + EVENTS.artifactReady, + raw.thread_id + ); + return; + } + const event: ArtifactReadyEvent = { + thread_id: raw.thread_id, + client_id: raw.client_id, + artifact_id: args.artifact_id, + kind: args.kind, + title: args.title, + path: args.path, + size_bytes: args.size_bytes, + }; + chatLog( + '%s thread_id=%s artifact_id=%s kind=%s size=%d', + EVENTS.artifactReady, + event.thread_id, + event.artifact_id, + event.kind, + event.size_bytes + ); + listeners.onArtifactReady?.(event); + }; + socket.on(EVENTS.artifactReady, cb); + handlers.push([EVENTS.artifactReady, cb]); + } + + if (listeners.onArtifactFailed) { + const cb = (payload: unknown) => { + const raw = payload as { + thread_id: string; + client_id?: string; + args?: { artifact_id?: string; kind?: ArtifactKind; title?: string; error?: string }; + }; + const args = raw.args ?? {}; + if (!args.artifact_id || !isValidArtifactKind(args.kind) || !args.title || !args.error) { + chatLog( + '%s thread_id=%s — skipping malformed payload (missing args)', + EVENTS.artifactFailed, + raw.thread_id + ); + return; + } + const event: ArtifactFailedEvent = { + thread_id: raw.thread_id, + client_id: raw.client_id, + artifact_id: args.artifact_id, + kind: args.kind, + title: args.title, + error: args.error, + }; + // Defence-in-depth: producer is expected to pre-truncate, but + // cap the log preview again so a leaky producer cannot blast + // unbounded provider stderr into client telemetry. + chatLog( + '%s thread_id=%s artifact_id=%s kind=%s err=%s', + EVENTS.artifactFailed, + event.thread_id, + event.artifact_id, + event.kind, + event.error.slice(0, 80) + ); + listeners.onArtifactFailed?.(event); + }; + socket.on(EVENTS.artifactFailed, cb); + handlers.push([EVENTS.artifactFailed, cb]); + } + if (listeners.onTaskBoardUpdated) { const cb = (payload: unknown) => { const e = payload as ChatTaskBoardUpdatedEvent; diff --git a/app/src/store/__tests__/chatRuntimeSlice.artifacts.test.ts b/app/src/store/__tests__/chatRuntimeSlice.artifacts.test.ts new file mode 100644 index 0000000000..7fe8366567 --- /dev/null +++ b/app/src/store/__tests__/chatRuntimeSlice.artifacts.test.ts @@ -0,0 +1,236 @@ +import { describe, expect, it } from 'vitest'; + +import reducer, { + clearAllChatRuntime, + clearArtifactsForThread, + clearRuntimeForThread, + upsertArtifactFailedForThread, + upsertArtifactInProgressForThread, + upsertArtifactReadyForThread, +} from '../chatRuntimeSlice'; + +describe('chatRuntimeSlice — artifact lifecycle (#2779)', () => { + it('upserts a new in_progress snapshot for a thread', () => { + const next = reducer( + undefined, + upsertArtifactInProgressForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + }) + ); + + const list = next.artifactsByThread['t-1']; + expect(list).toHaveLength(1); + expect(list[0]).toMatchObject({ + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + status: 'in_progress', + }); + expect(typeof list[0].updatedAt).toBe('number'); + }); + + it('promotes the in_progress entry to ready in place, preserving order', () => { + const inProgress = reducer( + undefined, + upsertArtifactInProgressForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + }) + ); + const withSibling = reducer( + inProgress, + upsertArtifactInProgressForThread({ + threadId: 't-1', + artifactId: 'a-2', + kind: 'document', + title: 'Notes', + }) + ); + + const ready = reducer( + withSibling, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + sizeBytes: 4096, + }) + ); + + const list = ready.artifactsByThread['t-1']; + expect(list).toHaveLength(2); + // first slot stays first — replaced in place, not appended. + expect(list[0]).toMatchObject({ + artifactId: 'a-1', + status: 'ready', + path: 'a-1/deck.pptx', + sizeBytes: 4096, + }); + expect(list[1].artifactId).toBe('a-2'); + }); + + it('records a failed artifact with the producer reason', () => { + const next = reducer( + undefined, + upsertArtifactFailedForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + error: 'pip install failed', + }) + ); + + expect(next.artifactsByThread['t-1'][0]).toMatchObject({ + artifactId: 'a-1', + status: 'failed', + error: 'pip install failed', + }); + }); + + it('appends new artifactIds in insertion order', () => { + const a = reducer( + undefined, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'document', + title: 'Doc', + path: 'a-1/doc.pdf', + sizeBytes: 100, + }) + ); + const b = reducer( + a, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-2', + kind: 'image', + title: 'Pic', + path: 'a-2/pic.png', + sizeBytes: 200, + }) + ); + + const ids = b.artifactsByThread['t-1'].map(entry => entry.artifactId); + expect(ids).toEqual(['a-1', 'a-2']); + }); + + it('keeps thread buckets isolated', () => { + const a = reducer( + undefined, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'document', + title: 'Doc', + path: 'a-1/doc.pdf', + sizeBytes: 100, + }) + ); + const b = reducer( + a, + upsertArtifactReadyForThread({ + threadId: 't-2', + artifactId: 'a-2', + kind: 'image', + title: 'Pic', + path: 'a-2/pic.png', + sizeBytes: 200, + }) + ); + + expect(b.artifactsByThread['t-1']).toHaveLength(1); + expect(b.artifactsByThread['t-2']).toHaveLength(1); + expect(b.artifactsByThread['t-1'][0].artifactId).toBe('a-1'); + expect(b.artifactsByThread['t-2'][0].artifactId).toBe('a-2'); + }); + + it('clearArtifactsForThread drops just that thread bucket', () => { + const populated = reducer( + undefined, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'image', + title: 'P', + path: 'a-1/p.png', + sizeBytes: 1, + }) + ); + const cleared = reducer(populated, clearArtifactsForThread({ threadId: 't-1' })); + expect(cleared.artifactsByThread['t-1']).toBeUndefined(); + }); + + it('clearRuntimeForThread does NOT wipe artifact history', () => { + // ArtifactCard renders inline in the message timeline so the snapshot + // must survive turn boundaries — historic artifacts stay visible. + const populated = reducer( + undefined, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'image', + title: 'P', + path: 'a-1/p.png', + sizeBytes: 1, + }) + ); + const cleared = reducer(populated, clearRuntimeForThread({ threadId: 't-1' })); + expect(cleared.artifactsByThread['t-1']).toHaveLength(1); + }); + + it('clearAllChatRuntime drops all artifact buckets', () => { + const populated = reducer( + undefined, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'image', + title: 'P', + path: 'a-1/p.png', + sizeBytes: 1, + }) + ); + const cleared = reducer(populated, clearAllChatRuntime()); + expect(cleared.artifactsByThread).toEqual({}); + }); + + it('failed -> ready replays in place without duplicating', () => { + // Useful if a producer retries: failed snapshot should be replaced + // by ready, not stacked. + const failed = reducer( + undefined, + upsertArtifactFailedForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + error: 'boom', + }) + ); + const ready = reducer( + failed, + upsertArtifactReadyForThread({ + threadId: 't-1', + artifactId: 'a-1', + kind: 'presentation', + title: 'Deck', + path: 'a-1/deck.pptx', + sizeBytes: 1024, + }) + ); + + const list = ready.artifactsByThread['t-1']; + expect(list).toHaveLength(1); + expect(list[0].status).toBe('ready'); + expect(list[0].error).toBeUndefined(); + }); +}); diff --git a/app/src/store/chatRuntimeSlice.ts b/app/src/store/chatRuntimeSlice.ts index 960a34c344..fdcfc5c156 100644 --- a/app/src/store/chatRuntimeSlice.ts +++ b/app/src/store/chatRuntimeSlice.ts @@ -187,6 +187,41 @@ export interface PendingApproval { command?: string; } +/** + * Lifecycle status of a single agent-generated artifact, as projected + * onto the chat runtime per thread. + * + * - `in_progress` — derived: the producing tool call is in flight; we + * have not yet seen a ready/failed event. UI shows a spinner. + * - `ready` — `artifact_ready` socket event received. UI shows a + * download button. + * - `failed` — `artifact_failed` socket event received. UI shows the + * reason + a retry hint. + */ +export type ArtifactStatus = 'in_progress' | 'ready' | 'failed'; + +/** + * Per-thread snapshot of a single artifact's state. Upserted from + * artifact lifecycle socket events; consumed by `ArtifactCard` for + * inline message rendering (#2779). + */ +export interface ArtifactSnapshot { + artifactId: string; + /** Kind slug from the Rust `ArtifactKind` enum. */ + kind: 'presentation' | 'document' | 'image' | 'other'; + /** Human-readable title; also the on-disk filename stem. */ + title: string; + status: ArtifactStatus; + /** Final on-disk size. Only set when `status === 'ready'`. */ + sizeBytes?: number; + /** Relative path under `/artifacts/`. Only set when `status === 'ready'`. */ + path?: string; + /** Producer-supplied reason. Only set when `status === 'failed'`. */ + error?: string; + /** When the snapshot was last updated, milliseconds since epoch. */ + updatedAt: number; +} + /** * Per-thread UI state for an in-flight agent turn (socket events while the user * may navigate away from Conversations). The thread slice keeps `activeThreadId` @@ -200,6 +235,13 @@ interface ChatRuntimeState { taskBoardByThread: Record; inferenceTurnLifecycleByThread: Record; pendingApprovalByThread: Record; + /** + * Per-thread artifact ledger. Snapshots are upserted on + * `artifact_ready` / `artifact_failed` socket events keyed on + * `artifactId`. `ArtifactCard` reads this slice to render inline + * download / retry affordances (#2779). + */ + artifactsByThread: Record; sessionTokenUsage: SessionTokenUsage; } @@ -210,9 +252,30 @@ const initialState: ChatRuntimeState = { taskBoardByThread: {}, inferenceTurnLifecycleByThread: {}, pendingApprovalByThread: {}, + artifactsByThread: {}, sessionTokenUsage: { inputTokens: 0, outputTokens: 0, turns: 0, lastUpdated: 0 }, }; +/** + * Upsert a single artifact snapshot for a thread. New entries append + * in insertion order (matches the timeline ordering the UI expects); + * existing entries are replaced in place so the inline card flips + * status without remounting. + */ +function upsertArtifact( + bucket: ArtifactSnapshot[] | undefined, + snapshot: ArtifactSnapshot +): ArtifactSnapshot[] { + const list = bucket ?? []; + const idx = list.findIndex(entry => entry.artifactId === snapshot.artifactId); + if (idx === -1) { + return [...list, snapshot]; + } + const next = list.slice(); + next[idx] = snapshot; + return next; +} + function subagentToolCallFromPersisted(call: PersistedSubagentToolCall): SubagentToolCallEntry { return { callId: call.callId, @@ -406,6 +469,99 @@ const chatRuntimeSlice = createSlice({ clearPendingApprovalForThread: (state, action: PayloadAction<{ threadId: string }>) => { delete state.pendingApprovalByThread[action.payload.threadId]; }, + /** + * Mark a producer-tool call as in-flight so the `ArtifactCard` can + * render a spinner before any ready/failed event arrives. Caller + * usually fires this off the corresponding `ChatToolCallEvent` + * when the tool is in the known artifact-producing allowlist + * (e.g. `generate_presentation`). Re-firing for the same + * `artifactId` is a no-op (idempotent upsert). + */ + upsertArtifactInProgressForThread: ( + state, + action: PayloadAction<{ + threadId: string; + artifactId: string; + kind: ArtifactSnapshot['kind']; + title: string; + }> + ) => { + const { threadId, artifactId, kind, title } = action.payload; + const snapshot: ArtifactSnapshot = { + artifactId, + kind, + title, + status: 'in_progress', + updatedAt: Date.now(), + }; + state.artifactsByThread[threadId] = upsertArtifact( + state.artifactsByThread[threadId], + snapshot + ); + }, + /** + * Mark an artifact as ready (download-able). Triggered by the + * `artifact_ready` socket event. Promotes status off `in_progress` + * and fills in `path` / `sizeBytes` for the download flow. + */ + upsertArtifactReadyForThread: ( + state, + action: PayloadAction<{ + threadId: string; + artifactId: string; + kind: ArtifactSnapshot['kind']; + title: string; + path: string; + sizeBytes: number; + }> + ) => { + const { threadId, artifactId, kind, title, path, sizeBytes } = action.payload; + const snapshot: ArtifactSnapshot = { + artifactId, + kind, + title, + status: 'ready', + path, + sizeBytes, + updatedAt: Date.now(), + }; + state.artifactsByThread[threadId] = upsertArtifact( + state.artifactsByThread[threadId], + snapshot + ); + }, + /** + * Mark an artifact as failed. Triggered by the `artifact_failed` + * socket event. Promotes status off `in_progress` and persists the + * producer-supplied `error` so the card can show a retry hint. + */ + upsertArtifactFailedForThread: ( + state, + action: PayloadAction<{ + threadId: string; + artifactId: string; + kind: ArtifactSnapshot['kind']; + title: string; + error: string; + }> + ) => { + const { threadId, artifactId, kind, title, error } = action.payload; + const snapshot: ArtifactSnapshot = { + artifactId, + kind, + title, + status: 'failed', + error, + updatedAt: Date.now(), + }; + state.artifactsByThread[threadId] = upsertArtifact( + state.artifactsByThread[threadId], + snapshot + ); + }, + clearArtifactsForThread: (state, action: PayloadAction<{ threadId: string }>) => { + delete state.artifactsByThread[action.payload.threadId]; + }, beginInferenceTurn: (state, action: PayloadAction<{ threadId: string }>) => { state.inferenceTurnLifecycleByThread[action.payload.threadId] = 'started'; }, @@ -424,6 +580,11 @@ const chatRuntimeSlice = createSlice({ delete state.taskBoardByThread[action.payload.threadId]; delete state.inferenceTurnLifecycleByThread[action.payload.threadId]; delete state.pendingApprovalByThread[action.payload.threadId]; + // Note: artifactsByThread intentionally NOT cleared here. The + // ArtifactCard renders inline in the message timeline, so the + // snapshot needs to survive turn boundaries — historic artifacts + // stay visible alongside the messages that produced them. Use + // `clearArtifactsForThread` if a hard reset is desired. }, clearAllChatRuntime: state => { state.inferenceStatusByThread = {}; @@ -432,6 +593,7 @@ const chatRuntimeSlice = createSlice({ state.taskBoardByThread = {}; state.inferenceTurnLifecycleByThread = {}; state.pendingApprovalByThread = {}; + state.artifactsByThread = {}; }, recordChatTurnUsage: ( state, @@ -521,6 +683,10 @@ export const { clearTaskBoardForThread, setPendingApprovalForThread, clearPendingApprovalForThread, + upsertArtifactInProgressForThread, + upsertArtifactReadyForThread, + upsertArtifactFailedForThread, + clearArtifactsForThread, beginInferenceTurn, markInferenceTurnStreaming, endInferenceTurn, diff --git a/app/test/playwright/specs/chat-tool-error-recovery.spec.ts b/app/test/playwright/specs/chat-tool-error-recovery.spec.ts index 151c245afb..41d1c7cc76 100644 --- a/app/test/playwright/specs/chat-tool-error-recovery.spec.ts +++ b/app/test/playwright/specs/chat-tool-error-recovery.spec.ts @@ -119,7 +119,13 @@ test.describe('Chat Tool Error Recovery', () => { const threadId = await createNewThread(page); await sendMessage(page, 'Tell me something important.'); - await expect(page.getByText('Starting to answer')).toBeVisible({ timeout: 20_000 }); + // Two elements briefly carry the streamed text: the live-streaming + // preview block (`font-mono` console-style render in Conversations.tsx) + // and the persisted message bubble that takes over once the segment + // commits. Playwright strict-mode trips when both are simultaneously + // visible during the transition. `.first()` keeps the assertion robust + // — we just care that the streamed substring rendered somewhere. + await expect(page.getByText('Starting to answer').first()).toBeVisible({ timeout: 20_000 }); await expect .poll(async () => { diff --git a/app/test/playwright/specs/insights-dashboard.spec.ts b/app/test/playwright/specs/insights-dashboard.spec.ts index 69f1ae0051..58a024c212 100644 --- a/app/test/playwright/specs/insights-dashboard.spec.ts +++ b/app/test/playwright/specs/insights-dashboard.spec.ts @@ -1,17 +1,25 @@ import { expect, test } from '@playwright/test'; -import { bootAuthenticatedPage, waitForAppReady } from '../helpers/core-rpc'; +import { + bootAuthenticatedPage, + dismissWalkthroughIfPresent, + waitForAppReady, +} from '../helpers/core-rpc'; test.describe('Insights Dashboard', () => { test('renders the memory workspace and actions toolbar', async ({ page }) => { await bootAuthenticatedPage(page, 'pw-insights-user', '/intelligence'); await waitForAppReady(page); - - // The Intelligence page now defaults to the "Agent Tasks" tab (#2998), so - // select the Memory tab before asserting its workspace renders. + // /intelligence boots on the Tasks tab (default in Intelligence.tsx since + // #2998). The walkthrough portal can sit on top of the pill bar and the + // memory-workspace testid only mounts when tab=memory, so we dismiss the + // overlay and click the Memory pill before asserting the panel chrome. + await dismissWalkthroughIfPresent(page); await page.getByRole('tab', { name: 'Memory', exact: true }).click(); - await expect(page.getByRole('heading', { name: 'Memory', exact: true })).toBeVisible(); + await expect(page.getByRole('heading', { name: 'Memory', exact: true })).toBeVisible({ + timeout: 15_000, + }); await expect(page.locator('[data-testid="memory-workspace"]')).toBeVisible(); await expect(page.locator('[data-testid="memory-actions"]')).toBeVisible(); }); diff --git a/src/core/event_bus/events.rs b/src/core/event_bus/events.rs index b4216c11bd..f46b73cbcd 100644 --- a/src/core/event_bus/events.rs +++ b/src/core/event_bus/events.rs @@ -352,6 +352,52 @@ pub enum DomainEvent { decision: String, }, + // ── Artifacts ─────────────────────────────────────────────────────── + /// An artifact transitioned to [`ArtifactStatus::Ready`] — file + /// is on disk and ready to be downloaded. Published by + /// [`crate::openhuman::artifacts::store::finalize_artifact`]. + /// Bridged to the web channel as an `artifact_ready` socket event + /// when the publishing turn carries an `APPROVAL_CHAT_CONTEXT` + /// (see [`crate::openhuman::approval::ApprovalChatContext`]). + /// Sub-task #2779 of #1535. + ArtifactReady { + /// UUID of the artifact record. + artifact_id: String, + /// Lowercase variant of `ArtifactKind` (`presentation`, + /// `document`, `image`, `other`). + kind: String, + /// Human-readable title (also the on-disk filename stem). + title: String, + /// Relative path under `/artifacts/`, e.g. + /// `"/deck.pptx"`. The absolute path is reachable via + /// `ai_get_artifact` so the renderer never needs the + /// workspace root. + path: String, + /// Final on-disk file size in bytes. + size_bytes: u64, + /// Chat thread the artifact belongs to, when the producing + /// turn carried an `APPROVAL_CHAT_CONTEXT`. `None` for CLI / + /// cron / sub-agent paths — no client to fan out to. + thread_id: Option, + /// Socket.IO client id (room) to surface the card to, when + /// known. `None` for non-chat callers. + client_id: Option, + }, + /// An artifact transitioned to [`ArtifactStatus::Failed`] — the + /// producer surfaced a reason and the UI should render a + /// retry-hint card instead of a download. Bridged the same way + /// as [`Self::ArtifactReady`]. Sub-task #2779 of #1535. + ArtifactFailed { + artifact_id: String, + kind: String, + title: String, + /// Producer-supplied failure reason. Already truncated by the + /// producer (e.g. `PresentationError::truncate_stderr`). + error: String, + thread_id: Option, + client_id: Option, + }, + // ── Webhooks ──────────────────────────────────────────────────────── /// An incoming webhook request from the transport layer, ready for routing. WebhookIncomingRequest { @@ -846,6 +892,8 @@ impl DomainEvent { Self::ApprovalRequested { .. } | Self::ApprovalDecided { .. } => "approval", + Self::ArtifactReady { .. } | Self::ArtifactFailed { .. } => "artifact", + Self::McpServerInstalled { .. } | Self::McpServerConnected { .. } | Self::McpServerDisconnected { .. } @@ -934,6 +982,8 @@ impl DomainEvent { Self::SessionExpired { .. } => "SessionExpired", Self::ApprovalRequested { .. } => "ApprovalRequested", Self::ApprovalDecided { .. } => "ApprovalDecided", + Self::ArtifactReady { .. } => "ArtifactReady", + Self::ArtifactFailed { .. } => "ArtifactFailed", Self::McpServerInstalled { .. } => "McpServerInstalled", Self::McpServerConnected { .. } => "McpServerConnected", Self::McpServerDisconnected { .. } => "McpServerDisconnected", diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 945ac56ae7..9a681f2dbc 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -2081,6 +2081,13 @@ pub async fn bootstrap_core_runtime(embedded_core: bool) { Prompt-class external-effect tool calls run unprompted" ); } + // Artifact surface bridges DomainEvent::ArtifactReady/Failed onto the web + // channel ("Files in this chat" panel + ArtifactCard updates). This is + // independent of the approval-gate config — keep it outside the + // `if approval_gate` block so artifact events still publish when the user + // sets OPENHUMAN_APPROVAL_GATE=0 (CR #3328947323 on PR #3026). Idempotent + // (OnceLock-guarded inside register_artifact_surface_subscriber). + crate::openhuman::channels::providers::web::register_artifact_surface_subscriber(); // --- Workspace migrations -------------------------------------------- crate::openhuman::startup::run_workspace_migrations(&workspace_dir); diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index 595e8bf0b7..b5dcebf12d 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -160,7 +160,6 @@ named = [ # Both are routed through the same security/approval gate as `shell`. "workflow_load", "workflow_phase", - # Presentation generation (#2778). Synthesises a .pptx from a # structured slide spec via a native Rust engine (`ppt-rs`) running # in-process — no Python subprocess, no managed venv. Output lands # in the workspace artifacts directory and the tool returns the diff --git a/src/openhuman/artifacts/store.rs b/src/openhuman/artifacts/store.rs index 3d21ca0ef5..2a070d5904 100644 --- a/src/openhuman/artifacts/store.rs +++ b/src/openhuman/artifacts/store.rs @@ -348,13 +348,27 @@ pub async fn create_artifact( /// Flip a pending artifact to [`ArtifactStatus::Ready`] and persist /// the final size. Idempotent on already-ready artifacts (no-op + log). /// Returns the updated metadata. +/// +/// On a real transition (Pending → Ready), publishes +/// [`DomainEvent::ArtifactReady`] on the global bus so the web +/// channel can surface a download card to the originating thread. +/// When the calling task carries no +/// [`ApprovalChatContext`](crate::openhuman::approval::ApprovalChatContext) +/// (CLI / cron / sub-agent paths), the event is still published but +/// `thread_id` / `client_id` are `None` so the socket bridge silently +/// drops it. Idempotent calls (already-Ready) skip the publish so we +/// don't flap the UI. pub async fn finalize_artifact( workspace_dir: &Path, artifact_id: &str, size_bytes: u64, ) -> Result { let mut meta = get_artifact(workspace_dir, artifact_id).await?; - if matches!(meta.status, ArtifactStatus::Ready) && meta.size_bytes == size_bytes { + if matches!(meta.status, ArtifactStatus::Ready) { + // Idempotent on status alone: a second finalize with a different + // size_bytes shouldn't re-emit ArtifactReady and flap the UI card + // — once an artifact is Ready, callers should not be redefining + // its size. Per graycyrus on PR #3017. log::debug!("[artifacts] finalize_artifact: id={artifact_id} already Ready, no-op"); return Ok(meta); } @@ -363,6 +377,17 @@ pub async fn finalize_artifact( meta.error = None; save_artifact_meta(workspace_dir, &meta).await?; log::debug!("[artifacts] finalize_artifact: id={artifact_id} -> Ready size={size_bytes}"); + + let (thread_id, client_id) = current_chat_context(); + crate::core::event_bus::publish_global(crate::core::event_bus::DomainEvent::ArtifactReady { + artifact_id: meta.id.clone(), + kind: meta.kind.as_str().to_string(), + title: meta.title.clone(), + path: meta.path.clone(), + size_bytes: meta.size_bytes, + thread_id, + client_id, + }); Ok(meta) } @@ -370,6 +395,10 @@ pub async fn finalize_artifact( /// failure reason. The producer should call this when generation /// fails so the UI / RPC consumer can surface a useful message /// instead of an indefinite spinner. Returns the updated metadata. +/// +/// Publishes [`DomainEvent::ArtifactFailed`] so the chat surface +/// flips the in-flight card to a retry-hint state. Same chat-context +/// rules as [`finalize_artifact`]. pub async fn fail_artifact( workspace_dir: &Path, artifact_id: &str, @@ -379,10 +408,39 @@ pub async fn fail_artifact( meta.status = ArtifactStatus::Failed; meta.error = Some(reason.to_string()); save_artifact_meta(workspace_dir, &meta).await?; - log::warn!("[artifacts] fail_artifact: id={artifact_id} -> Failed reason={reason:?}"); + // Log only the size of the reason — it can carry provider stderr + // / user-derived content, which we don't want flushed verbatim + // into structured logs. The full payload is still persisted on + // `meta.error` for the UI surface and the chat event below. + log::warn!( + "[artifacts] fail_artifact: id={artifact_id} -> Failed reason_len={}", + reason.len() + ); + + let (thread_id, client_id) = current_chat_context(); + crate::core::event_bus::publish_global(crate::core::event_bus::DomainEvent::ArtifactFailed { + artifact_id: meta.id.clone(), + kind: meta.kind.as_str().to_string(), + title: meta.title.clone(), + error: reason.to_string(), + thread_id, + client_id, + }); Ok(meta) } +/// Read the active [`ApprovalChatContext`] task-local (set by +/// `channels::providers::web` around each chat turn) and return its +/// thread + client ids. Returns `(None, None)` for non-chat callers +/// (CLI, cron, sub-agent runners) so artifact emit hooks degrade +/// gracefully — the event is still published but the web subscriber +/// drops it for lack of a routing target. +fn current_chat_context() -> (Option, Option) { + crate::openhuman::approval::APPROVAL_CHAT_CONTEXT + .try_with(|ctx| (Some(ctx.thread_id.clone()), Some(ctx.client_id.clone()))) + .unwrap_or((None, None)) +} + #[cfg(test)] #[path = "store_tests.rs"] mod tests; diff --git a/src/openhuman/channels/providers/web.rs b/src/openhuman/channels/providers/web.rs index c14a76f8b6..972fde8645 100644 --- a/src/openhuman/channels/providers/web.rs +++ b/src/openhuman/channels/providers/web.rs @@ -62,6 +62,120 @@ pub fn register_approval_surface_subscriber() { } } +/// Handle for the artifact-surface subscriber. Set once on +/// [`register_artifact_surface_subscriber`]; subsequent calls no-op. +static ARTIFACT_SURFACE_HANDLE: OnceLock = OnceLock::new(); + +/// Bridge artifact lifecycle events onto the web channel. +/// `DomainEvent::ArtifactReady` / `ArtifactFailed` (published by +/// `artifacts::store::{finalize,fail}_artifact` from #2778's producer +/// surface) carry the thread_id + client_id when the producing turn +/// ran under an `APPROVAL_CHAT_CONTEXT`. When present, fan out as an +/// `artifact_ready` / `artifact_failed` socket event so the frontend +/// `chatRuntimeSlice` can upsert the snapshot and the `ArtifactCard` +/// can render in the message timeline. Sub-task #2779 of #1535. +/// Idempotent. No-op for non-chat events (thread/client id absent). +pub fn register_artifact_surface_subscriber() { + if ARTIFACT_SURFACE_HANDLE.get().is_some() { + return; + } + match crate::core::event_bus::subscribe_global(Arc::new(ArtifactSurfaceSubscriber)) { + Some(handle) => { + let _ = ARTIFACT_SURFACE_HANDLE.set(handle); + log::info!( + "[web-channel] artifact-surface subscriber registered (domain=artifact) — will bridge ArtifactReady/Failed → artifact_ready/artifact_failed socket events" + ); + } + None => { + log::warn!( + "[web-channel] failed to register artifact-surface subscriber — bus not initialized" + ); + } + } +} + +struct ArtifactSurfaceSubscriber; + +#[async_trait] +impl EventHandler for ArtifactSurfaceSubscriber { + fn name(&self) -> &str { + "channels::web::artifact_surface" + } + + fn domains(&self) -> Option<&[&str]> { + Some(&["artifact"]) + } + + async fn handle(&self, event: &DomainEvent) { + match event { + DomainEvent::ArtifactReady { + artifact_id, + kind, + title, + path, + size_bytes, + thread_id, + client_id, + } => { + let (Some(thread_id), Some(client_id)) = (thread_id, client_id) else { + log::debug!( + "[web-channel] artifact-surface skip ArtifactReady id={artifact_id}: no chat context" + ); + return; + }; + log::info!( + "[web-channel] artifact-surface emitting artifact_ready id={artifact_id} kind={kind} thread_id={thread_id} client_id={client_id}" + ); + publish_web_channel_event(WebChannelEvent { + event: "artifact_ready".to_string(), + client_id: client_id.clone(), + thread_id: thread_id.clone(), + args: Some(serde_json::json!({ + "artifact_id": artifact_id, + "kind": kind, + "title": title, + "path": path, + "size_bytes": size_bytes, + })), + ..Default::default() + }); + } + DomainEvent::ArtifactFailed { + artifact_id, + kind, + title, + error, + thread_id, + client_id, + } => { + let (Some(thread_id), Some(client_id)) = (thread_id, client_id) else { + log::debug!( + "[web-channel] artifact-surface skip ArtifactFailed id={artifact_id}: no chat context" + ); + return; + }; + log::warn!( + "[web-channel] artifact-surface emitting artifact_failed id={artifact_id} kind={kind} thread_id={thread_id} client_id={client_id} error_len={}", + error.len() + ); + publish_web_channel_event(WebChannelEvent { + event: "artifact_failed".to_string(), + client_id: client_id.clone(), + thread_id: thread_id.clone(), + args: Some(serde_json::json!({ + "artifact_id": artifact_id, + "kind": kind, + "title": title, + "error": error, + })), + ..Default::default() + }); + } + _ => {} + } + } +} + struct ApprovalSurfaceSubscriber; #[async_trait] diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index 5000032ee9..c5561228ac 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -56,6 +56,10 @@ pub async fn start_channels(mut config: Config) -> Result<()> { // Surface parked ApprovalGate requests as chat messages so the user can // answer yes/no in the thread (chat-native approval, issue #1339). crate::openhuman::channels::providers::web::register_approval_surface_subscriber(); + // Surface generated-artifact lifecycle events (ArtifactReady / + // ArtifactFailed) as `artifact_ready` / `artifact_failed` web-channel + // events so the frontend ArtifactCard can render in chat (#2779). + crate::openhuman::channels::providers::web::register_artifact_surface_subscriber(); // Spawn the per-toolkit provider periodic sync scheduler. This is // a thin tokio task that ticks every minute and dispatches into // any provider whose `sync_interval_secs` has elapsed for an