From d6980342f1d9da9fd0028bad6d12d5e97927ad75 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 18:40:35 +0800 Subject: [PATCH] feat: close advertised tool reachability gap --- CLAUDE.md | 4 +- crates/opentake-agent/src/chat/loop.rs | 9 +- crates/opentake-agent/src/mcp/convert.rs | 28 + crates/opentake-agent/src/mcp/dispatch.rs | 586 +++++++++++++++++- crates/opentake-agent/src/mcp/media_bridge.rs | 111 +++- crates/opentake-agent/src/mcp/server.rs | 2 +- crates/opentake-agent/src/prompt/base.rs | 67 +- crates/opentake-agent/src/tools/names.rs | 68 +- crates/opentake-agent/src/tools/result.rs | 14 + .../tests/advertised_tool_acceptance.rs | 91 +++ .../tests/tool_argument_contract.rs | 2 +- docs/architecture/BUGS.md | 6 +- docs/architecture/FULL_PROJECT_SCAN_REPORT.md | 10 +- docs/architecture/HANDOFF-2026-07.md | 6 +- docs/architecture/ROADMAP.md | 2 +- ...gent-settings-generation-implementation.md | 8 +- docs/modules/opentake-agent/OVERVIEW.md | 11 +- docs/modules/opentake-agent/dispatch-tools.md | 16 +- docs/modules/opentake-agent/mcp-server.md | 2 +- docs/specs/agent/2-tools.md | 2 + src-tauri/src/mcp.rs | 375 ++++++++++- 21 files changed, 1332 insertions(+), 88 deletions(-) create mode 100644 crates/opentake-agent/tests/advertised_tool_acceptance.rs diff --git a/CLAUDE.md b/CLAUDE.md index 94290982..0820c5de 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,14 +76,14 @@ ## 5. 🟦 可认领/未完成(供同事,注意文件区避免冲突) - **🔴 #53 [#47-C] 时间线播放引擎**(连续解码 + cpal 音频 + A/V 同步 + MJPEG 回环传输)。子项 #63(cpal)/#64(MJPEG 传输)/#65(Lottie 烘焙)。最大未完成项,需专门会话 + 真机视觉验证。 - **#48 片段编辑收尾**:Delete/切割/片段右键菜单/Inspector 三段式/Toolbar 接线。 -- **剩余 MCP 工具 stub**:媒体读取(inspect_media/get_transcript/search_media)+ import_media 需**拓宽 CoreHandle 接 MediaEngine**(注意:CoreHandle 现仅持 AppCore,MediaEngine 在 MediaState,需架构扩展);`generate_*`/upscale 需异步 GenClient + BYOK;add_captions 需端上 whisper。 +- **剩余隐藏能力**:`inspect_media/get_transcript/search_media/inspect_timeline/import_media/add_captions` 已接真实路径；生成/超分/Motion 六个线名在生产后端完成前不进入发现面。`inspect_media` 尚缺 Lottie；`generate_*`/upscale 仍需异步 GenClient + BYOK，Motion 仍需确定性渲染/导入事务。 - **#49 项目内文件夹导入 + 嵌套文件夹浏览(剪映式)**:文件夹图标/双击进入/面包屑/拖出;DTO 加 folderId+folders;import_folder 镜像目录树。用户很想要。 - **#37 全局可复用素材库 + 收藏**(跨项目/分类/音效库/全库可见):**后端已并入 main** —— 存储层 `crates/opentake-media/src/library.rs`(#37-A/#54,PR #104,copy-on-favorite + SHA-256 内容寻址去重 + JSON manifest 原子写)+ Tauri 命令层 `src-tauri/src/library.rs`(#37-B/#55,PR #106,7 命令 list/favorite/unfavorite/categorize/rename/delete/import_to_project)。**前端 #37-C/#56 已并入 main**(PR #115:独立 `LibraryView` 全屏视图 + `libraryStore`/`libraryApi`,分类树/网格/搜索/排序/跨视图聚合/音效库;Home/TitleBar 入口;前端↔后端 7 命令契约已核实)。**#37 epic 收口**(后端 #104/#106 + 前端 #115)。剩:库→时间线拖拽(现用「导入当前项目」按钮)、媒体面板「星标→library_favorite」接线、收藏从 localStorage 迁后端。follow-up:`library.rs:322` remove() 静默吞 remove_file 错误,建议补 `tracing::warn!`;`library_delete` 与 `library_unfavorite` 现为纯别名,建议语义区分。 - **#39 提取音频星标 · #40 设置多分页+主页 1:1 · #34 motion dispatch · #27–30 进阶 B/C/D/E · #22–25 #12 follow-up · #35 bundle id 改名**。 - 冲突注意:我(#47/#48)动 opentake-render/opentake-media(decode/FrameProvider)/src-tauri(composite_frame、autosave)/web Preview+timeline;#36 动 agent+src-tauri(server 段);#37/#49 动 opentake-media(library/folders)+web media。**src-tauri/lib.rs、opentake-media 是多方交汇点,合并按 issue 顺序、各自小段、勤 rebase。** ## 6. MCP 配置(#36 落地后) -Streamable-HTTP `http://127.0.0.1:19789/mcp`(loopback+Origin 校验)。`claude mcp add --transport http opentake http://127.0.0.1:19789/mcp`;Cursor/Codex/Claude Desktop 同址。40 工具,返回附 context_signal。 +Streamable-HTTP `http://127.0.0.1:19789/mcp`(loopback+Origin 校验)。`claude mcp add --transport http opentake http://127.0.0.1:19789/mcp`;Cursor/Codex/Claude Desktop 同址。当前发布 38 个真实路径工具，返回附 context_signal。 ## 7. 压缩后立即执行 1. 读本文件 + `docs/architecture/PORT-1TO1-GAP.md`。2. `git -C OpenTake pull`(main)。3. 盘点 `gh issue list`,挑最高价值且可完整交付的:**首选 🔴 #53 播放引擎**(大,需专门会话),或 #48 片段编辑收尾、#49/#37 库与文件夹、剩余 MCP 工具 stub。4. 每项走 分支→写→自审→`cargo fmt`+clippy+test→真机/确定性验证→`gh run watch` 双绿→`--admin` 合并。5. 新依赖先读 `~/.cargo/registry/src` 真实源码核实 API(cosmic-text/rmcp 都这么做的),别照猜测写。 diff --git a/crates/opentake-agent/src/chat/loop.rs b/crates/opentake-agent/src/chat/loop.rs index 3566d107..8a494ec7 100644 --- a/crates/opentake-agent/src/chat/loop.rs +++ b/crates/opentake-agent/src/chat/loop.rs @@ -251,7 +251,8 @@ impl ChatLoop { } /// The tool catalog in the OpenAI function-calling shape. Built fresh per - /// turn (cheap; ~44 tools) so the model always sees the current schema. + /// turn (cheap; currently 38 live tools) so the model always sees the + /// current fail-closed catalog. /// /// When the dispatcher lacks a media bridge, hide the bridge-dependent /// tools instead of advertising tools that would only fail at runtime. @@ -261,7 +262,10 @@ impl ChatLoop { .copied() .filter(|tool| { self.dispatcher.has_media_bridge() - || !matches!(tool, ToolName::InspectTimeline | ToolName::ImportMedia) + || !matches!( + tool, + ToolName::InspectMedia | ToolName::InspectTimeline | ToolName::ImportMedia + ) }) .map(|tool| ToolSchema { name: tool.as_str().to_string(), @@ -603,6 +607,7 @@ mod tests { let loop_ = build_loop(talking_head_timeline(), Arc::new(MemoryKeyStore::new())); let tools = loop_.tool_catalog(); assert!(tools.iter().any(|t| t.name == "tighten_silences")); + assert!(!tools.iter().any(|t| t.name == "inspect_media")); assert!(!tools.iter().any(|t| t.name == "inspect_timeline")); assert!(!tools.iter().any(|t| t.name == "import_media")); } diff --git a/crates/opentake-agent/src/mcp/convert.rs b/crates/opentake-agent/src/mcp/convert.rs index 900a3289..a8a63e91 100644 --- a/crates/opentake-agent/src/mcp/convert.rs +++ b/crates/opentake-agent/src/mcp/convert.rs @@ -136,6 +136,14 @@ fn safe_public_detail(kind: PublicErrorKind, private_detail: &str) -> Option { Some(safe_invalid_argument_detail(tool, private_detail)) } + PublicErrorKind::ResourceNotFound(tool) => Some(format!( + "{} could not resolve the referenced project resource.", + tool.as_str() + )), + PublicErrorKind::CapabilityUnavailable(tool) => Some(format!( + "{} cannot inspect this source in the current build or source state.", + tool.as_str() + )), } } @@ -314,6 +322,26 @@ mod tests { assert!(!wire.contains("expected i32")); } + #[test] + fn typed_unavailable_error_exposes_only_fixed_recovery_contract() { + let private = "inspect_media: /Users/alice/private.mov is offline"; + let result = ToolResult::public_error( + PublicErrorKind::CapabilityUnavailable(ToolName::InspectMedia), + private, + ); + let value = safe_tool_result_for_llm(&result); + assert_eq!(value["code"], "MCP_CAPABILITY_UNAVAILABLE"); + assert_eq!( + value["message"], + "This capability is unavailable for the referenced media." + ); + assert_eq!( + value["details"], + "inspect_media cannot inspect this source in the current build or source state." + ); + assert!(!value.to_string().contains("/Users/alice")); + } + #[test] fn explicitly_public_marker_cannot_bypass_detail_guard() { let private = "/Users/alice/private.mp4"; diff --git a/crates/opentake-agent/src/mcp/dispatch.rs b/crates/opentake-agent/src/mcp/dispatch.rs index ac21fe62..9dd34b06 100644 --- a/crates/opentake-agent/src/mcp/dispatch.rs +++ b/crates/opentake-agent/src/mcp/dispatch.rs @@ -12,9 +12,10 @@ //! 7. shorten outbound ids in the result, //! 8. return the [`ToolResult`]. //! -//! Sync throughout: every wired (EXISTS-mapped) tool is synchronous. The async -//! generation / media tools are stubs in this phase and return an honest -//! "not yet implemented" so the tool table is complete. +//! Sync throughout: every advertised (EXISTS-mapped) tool has a synchronous +//! dispatch path. Future async generation and Motion tools retain known wire +//! names for compatibility but stay out of discovery until their backends are +//! production-ready. use std::collections::BTreeMap; use std::sync::{Arc, Mutex, RwLock}; @@ -37,7 +38,8 @@ use serde_json::Value; use crate::mcp::core_handle::CoreHandle; use crate::mcp::gen_catalog; use crate::mcp::media_bridge::{ - frame_to_block, ImportSource, InspectResult, MediaBridge, SearchCandidate, TranscriptSource, + frame_to_block, media_frame_to_block, BridgeErrorKind, ImportSource, InspectMediaRequest, + InspectMediaResult, InspectResult, MediaBridge, SearchCandidate, TranscriptSource, IMPORT_BYTES_BASE64_MAX, }; use crate::plugin::registry::PluginRegistry; @@ -56,6 +58,10 @@ use crate::tools::short_id; const INSPECT_TIMELINE_DEFAULT_FRAMES: i32 = 6; const INSPECT_TIMELINE_MAX_FRAMES: i32 = 12; const INSPECT_TIMELINE_MAX_DIMENSION: u32 = 512; +const INSPECT_MEDIA_DEFAULT_FRAMES: usize = 6; +const INSPECT_MEDIA_MAX_FRAMES: usize = 12; +const INSPECT_MEDIA_MAX_SEGMENTS: usize = 400; +const INSPECT_MEDIA_MAX_WORDS: usize = 10_000; /// The in-process tool dispatcher. Holds the [`CoreHandle`] boundary, the plugin /// registry (read-locked for the active plugin), and a per-dispatcher agent-undo @@ -130,16 +136,21 @@ impl Dispatcher { ); }; - // Validate the complete wire shape before snapshots, side effects, or a - // not-yet-implemented stub can run. `run_body` still decodes the typed - // value it consumes after short-id expansion; this preflight is the - // fail-closed contract shared by every one of ToolName::ALL. + // Validate the complete wire shape before snapshots or side effects. + // Known-but-hidden compatibility names keep their strict schema + // contract, but a valid invocation is rejected below as unavailable. if let Err(error) = validate_tool_args(tool, &args) { return ToolResult::public_error( PublicErrorKind::InvalidArguments(tool), error.message, ); } + if !ToolName::ALL.contains(&tool) { + return ToolResult::public_error( + PublicErrorKind::UnknownTool, + format!("Tool is not advertised: {}", tool.as_str()), + ); + } // 2. Snapshot the pre-run state. let before = self.handle.timeline(); @@ -216,6 +227,7 @@ impl Dispatcher { Ok(ToolResult::ok(json.to_string())) } ToolName::ListModels => self.list_models_catalog(args), + ToolName::InspectMedia => self.inspect_media(args, before, manifest), // --- Editing (wired to EditCommand) --- ToolName::AddClips => self.add_clips(args, manifest, op), @@ -259,19 +271,17 @@ impl Dispatcher { ToolName::AddCaptions => self.add_captions(args, before, manifest), ToolName::SearchMedia => self.search_media(args, manifest), - // --- Not yet implementable in this phase (honest stubs) --- - // inspect_media still needs the analysis backend; generation/upscale - // need the async GenClient + BYOK auth. Motion graphics (#34) now - // routes through the planned Motion Canvas plugin: render mp4 -> - // import media -> place clip. - ToolName::InspectMedia - | ToolName::GenerateVideo + // --- Known but deliberately absent from discovery --- + // Generation/upscale need the async GenClient + BYOK auth. Motion + // graphics (#34) need the planned deterministic Motion Canvas path: + // render mp4 -> import media -> place clip. + ToolName::GenerateVideo | ToolName::GenerateImage | ToolName::GenerateAudio | ToolName::UpscaleMedia | ToolName::AddMotionGraphic | ToolName::EditMotionGraphic => Ok(ToolResult::error(format!( - "{}: not yet implemented", + "{}: capability is not advertised", tool.as_str() ))), } @@ -293,6 +303,92 @@ impl Dispatcher { // MARK: - Render + import tool bodies (backed by the MediaBridge) + /// Inspect one raw source asset with real decoded frames and optional local + /// transcription. Manifest/clip/range validation stays in the dispatcher; + /// retained source resolution and IO stay behind [`MediaBridge`]. + fn inspect_media( + &self, + args: &Value, + timeline: &Timeline, + manifest: &MediaManifest, + ) -> Result { + let a: InspectMediaArgs = decode_tool_args(args, "")?; + let Some(entry) = manifest + .entries + .iter() + .find(|entry| entry.id == a.media_ref) + else { + return Ok(ToolResult::public_error( + PublicErrorKind::ResourceNotFound(ToolName::InspectMedia), + format!("Media not found: {}", a.media_ref), + )); + }; + if entry.kind == opentake_domain::ClipType::Text { + return Ok(ToolResult::public_error( + PublicErrorKind::CapabilityUnavailable(ToolName::InspectMedia), + "Text clips are not stored as media assets.", + )); + } + + let mapping = if let Some(clip_id) = a.clip_id.as_deref() { + let clip = find_clip(timeline, clip_id) + .ok_or_else(|| ToolError::new(format!("Clip not found: {clip_id}")))?; + if clip.media_ref != entry.id { + return Err(ToolError::new(format!( + "Clip {clip_id} does not reference mediaRef {} (it references {})", + entry.id, clip.media_ref + ))); + } + Some(clip) + } else { + None + }; + + let duration = entry.duration.max(0.0); + let range = inspect_media_range(a.start_seconds, a.end_seconds, duration)?; + let max_frames = a + .max_frames + .unwrap_or(INSPECT_MEDIA_DEFAULT_FRAMES as i32) + .clamp(1, INSPECT_MEDIA_MAX_FRAMES as i32) as usize; + let Some(bridge) = self.bridge.as_ref() else { + return Ok(ToolResult::public_error( + PublicErrorKind::CapabilityUnavailable(ToolName::InspectMedia), + "inspect_media: source inspection is not available in this build", + )); + }; + let request = InspectMediaRequest { + media_ref: entry.id.clone(), + kind: entry.kind, + start_seconds: range.map(|value| value.0), + end_seconds: range.map(|value| value.1), + max_frames, + overview: a.overview.unwrap_or(false), + }; + let inspected = match bridge.inspect_media(&request) { + Ok(inspected) => inspected, + Err(error) => { + let kind = match error.kind { + BridgeErrorKind::Private => return Err(ToolError::new(error.message)), + BridgeErrorKind::NotFound => { + PublicErrorKind::ResourceNotFound(ToolName::InspectMedia) + } + BridgeErrorKind::Unavailable => { + PublicErrorKind::CapabilityUnavailable(ToolName::InspectMedia) + } + }; + return Ok(ToolResult::public_error(kind, error.message)); + } + }; + inspect_media_result( + entry, + timeline.fps, + mapping, + &request, + inspected, + a.word_timestamps, + ) + } + /// `inspect_timeline`: composite one project frame, or `maxFrames` frames /// evenly sampled across `[startFrame, endFrame)`, downscaled for tokens. /// 1:1 port of upstream `ToolExecutor+InspectTimeline.inspectTimeline` @@ -2885,6 +2981,283 @@ fn parse_interpolation(s: &str) -> Option { } } +fn inspect_media_range( + start: Option, + end: Option, + duration: f64, +) -> Result, ToolError> { + if start.is_none() && end.is_none() { + return Ok(None); + } + let start = start.unwrap_or(0.0).max(0.0); + let end = end.unwrap_or(duration).min(duration); + if start >= end { + return Err(ToolError::new(format!( + "Invalid time range [{start}, {end}] for media of duration {duration}s" + ))); + } + Ok(Some((start, end))) +} + +fn inspect_media_result( + entry: &opentake_domain::MediaManifestEntry, + timeline_fps: i32, + mapping: Option<&opentake_domain::Clip>, + request: &InspectMediaRequest, + inspected: InspectMediaResult, + include_words: Option, +) -> Result { + if entry.kind.is_visual() && inspected.frames.is_empty() { + return Err(ToolError::new(format!( + "Failed to extract frames from {}", + entry.name + ))); + } + + let mut blocks: Vec = inspected.frames.iter().map(media_frame_to_block).collect(); + let mut meta = serde_json::Map::new(); + meta.insert("id".into(), Value::String(entry.id.clone())); + meta.insert("name".into(), Value::String(entry.name.clone())); + meta.insert( + "type".into(), + serde_json::to_value(entry.kind).unwrap_or(Value::Null), + ); + meta.insert( + "duration".into(), + json_number(inspected.duration_seconds, 3), + ); + meta.insert("generationStatus".into(), Value::String("none".into())); + meta.insert("byteSize".into(), Value::from(inspected.byte_size)); + if let Some(file_name) = manifest_file_name(entry) { + meta.insert("fileName".into(), Value::String(file_name)); + } + if let Some(width) = inspected.width { + meta.insert("sourceWidth".into(), Value::from(width)); + } + if let Some(height) = inspected.height { + meta.insert("sourceHeight".into(), Value::from(height)); + } + if let Some(fps) = inspected.fps { + meta.insert("sourceFPS".into(), json_number(fps, 3)); + } + if let Some(input) = &entry.generation_input { + if let Ok(input) = serde_json::to_value(input) { + meta.insert("generationInput".into(), input); + } + } + if let (Some(start), Some(end)) = (request.start_seconds, request.end_seconds) { + meta.insert( + "timeRange".into(), + Value::Array(vec![json_number(start, 3), json_number(end, 3)]), + ); + } + + let frame_timestamps: Vec = inspected + .frames + .iter() + .map(|frame| json_number(frame.timestamp_seconds, 3)) + .collect(); + if request.overview { + let timestamps = inspected + .overview_timestamps + .iter() + .map(|timestamp| json_number(*timestamp, 3)) + .collect::>(); + meta.insert( + "overview".into(), + serde_json::json!({"tileTimestamps": timestamps}), + ); + } else if !frame_timestamps.is_empty() { + meta.insert("frameTimestamps".into(), Value::Array(frame_timestamps)); + } + + if entry.kind == opentake_domain::ClipType::Image { + if let Some(frame) = inspected.frames.first() { + meta.insert("mimeType".into(), Value::String(frame.media_type.clone())); + meta.insert("encodedByteSize".into(), Value::from(frame.bytes.len())); + } + if let (Some(width), Some(height)) = (inspected.width, inspected.height) { + meta.insert( + "imageProperties".into(), + serde_json::json!({"pixelWidth": width, "pixelHeight": height}), + ); + } + } + if entry.kind == opentake_domain::ClipType::Video { + meta.insert("hasAudio".into(), Value::Bool(inspected.has_audio)); + } + + if let Some(transcript) = inspected.transcript.as_ref() { + let transcript = transcription_meta( + transcript, + mapping, + timeline_fps, + include_words.unwrap_or(false), + ); + if entry.kind == opentake_domain::ClipType::Audio { + meta.extend(transcript); + } else { + meta.insert("transcription".into(), Value::Object(transcript)); + } + } else if inspected.transcription_unavailable { + meta.insert( + "transcriptionError".into(), + Value::String("On-device transcription is unavailable.".into()), + ); + } + if let Some(clip) = mapping { + meta.insert( + "timelineMapping".into(), + serde_json::json!({ + "clipId": clip.id, + "clipStartFrame": clip.start_frame, + "clipEndFrame": clip.end_frame(), + "fps": timeline_fps, + "note": "transcription segments/words are project frames for this clip; out-of-range entries are dropped." + }), + ); + } + + blocks.push(Block::text( + round_floats_3dp(Value::Object(meta)).to_string(), + )); + Ok(ToolResult::blocks(blocks)) +} + +fn transcription_meta( + transcript: &opentake_media::TranscriptionResult, + mapping: Option<&opentake_domain::Clip>, + timeline_fps: i32, + include_words: bool, +) -> serde_json::Map { + let mut out = serde_json::Map::new(); + out.insert( + "timing".into(), + Value::String(if mapping.is_some() { + "projectFrames".into() + } else { + "sourceSeconds".into() + }), + ); + if let Some(language) = &transcript.language { + out.insert("language".into(), Value::String(language.clone())); + } + + let segment_rows: Vec<(Value, f64)> = transcript + .segments + .iter() + .filter_map(|segment| { + let row = if let Some(clip) = mapping { + let (start, end) = opentake_media::transcribe::timeline::span_frames( + segment.start, + segment.end, + clip, + timeline_fps, + )?; + serde_json::json!([segment.text, start, end]) + } else { + serde_json::json!([ + segment.text, + json_number(segment.start, 2), + json_number(segment.end, 2) + ]) + }; + Some((row, segment.end)) + }) + .collect(); + out.insert( + "segments".into(), + Value::Array( + segment_rows + .iter() + .take(INSPECT_MEDIA_MAX_SEGMENTS) + .map(|(row, _)| row.clone()) + .collect(), + ), + ); + if segment_rows.len() > INSPECT_MEDIA_MAX_SEGMENTS { + out.insert("totalSegments".into(), Value::from(segment_rows.len())); + if let Some((_, end)) = segment_rows.get(INSPECT_MEDIA_MAX_SEGMENTS - 1) { + out.insert("nextStartSeconds".into(), json_number(*end, 2)); + } + out.insert( + "segmentsNote".into(), + Value::String(format!( + "First {} of {} segments. Continue with startSeconds = nextStartSeconds.", + INSPECT_MEDIA_MAX_SEGMENTS, + segment_rows.len() + )), + ); + } + + if include_words { + let words: Vec = transcript + .words + .iter() + .filter_map(|word| { + let (Some(start), Some(end)) = (word.start, word.end) else { + return None; + }; + if let Some(clip) = mapping { + let (start, end) = opentake_media::transcribe::timeline::span_frames( + start, + end, + clip, + timeline_fps, + )?; + Some(serde_json::json!([word.text, start, end])) + } else { + Some(serde_json::json!([ + word.text, + json_number(start, 2), + json_number(end, 2) + ])) + } + }) + .collect(); + out.insert( + "words".into(), + Value::Array( + words + .iter() + .take(INSPECT_MEDIA_MAX_WORDS) + .cloned() + .collect(), + ), + ); + if words.len() > INSPECT_MEDIA_MAX_WORDS { + out.insert("totalWords".into(), Value::from(words.len())); + out.insert( + "wordsNote".into(), + Value::String(format!( + "First {} of {} words. Narrow with startSeconds/endSeconds.", + INSPECT_MEDIA_MAX_WORDS, + words.len() + )), + ); + } + } + out +} + +fn manifest_file_name(entry: &opentake_domain::MediaManifestEntry) -> Option { + let path = match &entry.source { + opentake_domain::MediaSource::External { absolute_path } => absolute_path, + opentake_domain::MediaSource::Project { relative_path } => relative_path, + }; + std::path::Path::new(path) + .file_name() + .and_then(|name| name.to_str()) + .map(str::to_string) +} + +fn json_number(value: f64, places: i32) -> Value { + let factor = 10_f64.powi(places); + serde_json::Number::from_f64((value * factor).round() / factor) + .map(Value::Number) + .unwrap_or(Value::Null) +} + /// Round every float in a JSON tree to 3 decimal places (mirrors the encoder's /// `round3`), so `get_media` floats match the rest of the agent surface. fn round_floats_3dp(value: Value) -> Value { @@ -3084,16 +3457,12 @@ mod tests { } #[test] - fn stub_tool_reports_not_implemented() { + fn hidden_tool_is_rejected_as_unadvertised() { let d = dispatcher_with(Arc::new(TestHandle::new())); let r = d.dispatch("generate_video", serde_json::json!({"prompt": "x"})); assert!(r.is_error); - assert!( - r.text_joined() - .contains("generate_video: not yet implemented"), - "{}", - r.text_joined() - ); + assert_eq!(r.public_error_kind(), Some(PublicErrorKind::UnknownTool)); + assert!(r.text_joined().contains("not advertised")); } #[test] @@ -4270,8 +4639,9 @@ mod tests { // MARK: - MediaBridge tools (inspect_timeline / import_media) use crate::mcp::media_bridge::{ - BridgeError, ImportOutcome, ImportSource, InspectResult, InspectedFrame, MediaBridge, - TranscriptSource, TranscriptSourceResult, + BridgeError, ImportOutcome, ImportSource, InspectMediaRequest, InspectMediaResult, + InspectResult, InspectedFrame, InspectedMediaFrame, MediaBridge, TranscriptSource, + TranscriptSourceResult, }; use crate::tools::result::Block; use opentake_media::{TranscriptionResult, TranscriptionSegment, TranscriptionWord}; @@ -4289,6 +4659,7 @@ mod tests { #[derive(Default)] struct FakeBridge { inspect_calls: Mutex, u32)>>, + media_inspect_calls: Mutex>, import_calls: Mutex>, /// Canned transcripts keyed by media_ref (source-seconds timings). transcripts: Mutex>, @@ -4320,6 +4691,43 @@ mod tests { } impl MediaBridge for FakeBridge { + fn inspect_media( + &self, + request: &InspectMediaRequest, + ) -> Result { + self.media_inspect_calls + .lock() + .unwrap() + .push(request.clone()); + let transcript = self + .transcripts + .lock() + .unwrap() + .get(&request.media_ref) + .cloned(); + let frames = if request.kind.is_visual() { + vec![InspectedMediaFrame { + timestamp_seconds: request.start_seconds.unwrap_or(0.25), + bytes: vec![0xff, 0xd8, 0xff, 0xe0], + media_type: "image/jpeg".into(), + }] + } else { + Vec::new() + }; + Ok(InspectMediaResult { + frames, + overview_timestamps: Vec::new(), + duration_seconds: 1.0, + width: request.kind.is_visual().then_some(640), + height: request.kind.is_visual().then_some(360), + fps: (request.kind == ClipType::Video).then_some(30.0), + has_audio: request.kind == ClipType::Video, + byte_size: 4096, + transcript, + transcription_unavailable: false, + }) + } + fn transcribe_sources( &self, sources: &[TranscriptSource], @@ -4443,6 +4851,134 @@ mod tests { (d, bridge) } + fn inspected_transcript() -> TranscriptionResult { + TranscriptionResult { + text: "hello world".into(), + language: Some("en".into()), + segments: vec![TranscriptionSegment { + text: "hello world".into(), + start: 0.0, + end: 1.0, + }], + words: vec![TranscriptionWord { + text: "hello".into(), + start: Some(0.0), + end: Some(0.5), + }], + } + } + + #[test] + fn inspect_media_returns_real_blocks_metadata_and_transcript() { + let (d, bridge) = dispatcher_with_fake_bridge(); + bridge + .transcripts + .lock() + .unwrap() + .insert("asset-1".into(), inspected_transcript()); + + let result = d.dispatch( + "inspect_media", + serde_json::json!({ + "mediaRef": "asset-1", + "startSeconds": 0.1, + "endSeconds": 0.9, + "maxFrames": 99, + "wordTimestamps": true + }), + ); + assert!(!result.is_error, "{}", result.text_joined()); + assert!(matches!(result.content.first(), Some(Block::Image { .. }))); + let text = result + .content + .iter() + .find_map(|block| match block { + Block::Text { text } if text.starts_with('{') => Some(text), + _ => None, + }) + .expect("inspection metadata block"); + let metadata: Value = serde_json::from_str(text).unwrap(); + assert_eq!(metadata["id"], "asset-1"); + assert_eq!(metadata["type"], "video"); + assert_eq!(metadata["timeRange"], serde_json::json!([0.1, 0.9])); + assert_eq!(metadata["transcription"]["timing"], "sourceSeconds"); + assert_eq!( + metadata["transcription"]["segments"][0], + serde_json::json!(["hello world", 0.0, 1.0]) + ); + assert_eq!( + metadata["transcription"]["words"][0], + serde_json::json!(["hello", 0.0, 0.5]) + ); + + let calls = bridge.media_inspect_calls.lock().unwrap(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].max_frames, INSPECT_MEDIA_MAX_FRAMES); + assert_eq!(calls[0].start_seconds, Some(0.1)); + assert_eq!(calls[0].end_seconds, Some(0.9)); + } + + #[test] + fn inspect_media_clip_mapping_uses_project_frames() { + let (d, bridge) = dispatcher_with_fake_bridge(); + bridge + .transcripts + .lock() + .unwrap() + .insert("asset-1".into(), inspected_transcript()); + + let result = d.dispatch( + "inspect_media", + serde_json::json!({ + "mediaRef": "asset-1", + "clipId": "clip-1", + "wordTimestamps": true + }), + ); + assert!(!result.is_error, "{}", result.text_joined()); + let metadata: Value = serde_json::from_str( + result + .content + .iter() + .find_map(|block| match block { + Block::Text { text } if text.starts_with('{') => Some(text.as_str()), + _ => None, + }) + .unwrap(), + ) + .unwrap(); + assert_eq!(metadata["transcription"]["timing"], "projectFrames"); + assert_eq!( + metadata["transcription"]["segments"][0], + serde_json::json!(["hello world", 0, 30]) + ); + assert_eq!(metadata["timelineMapping"]["clipId"], "clip-1"); + } + + #[test] + fn inspect_media_rejects_missing_asset_and_invalid_range_before_io() { + let (d, bridge) = dispatcher_with_fake_bridge(); + let missing = d.dispatch("inspect_media", serde_json::json!({"mediaRef": "ghost"})); + assert!(missing.is_error); + assert!(missing.text_joined().contains("Media not found: ghost")); + assert_eq!( + missing.public_error_kind(), + Some(PublicErrorKind::ResourceNotFound(ToolName::InspectMedia)) + ); + + let invalid = d.dispatch( + "inspect_media", + serde_json::json!({ + "mediaRef": "asset-1", + "startSeconds": 0.9, + "endSeconds": 0.1 + }), + ); + assert!(invalid.is_error); + assert!(invalid.text_joined().contains("Invalid time range")); + assert!(bridge.media_inspect_calls.lock().unwrap().is_empty()); + } + #[test] fn inspect_timeline_without_bridge_reports_unavailable() { // The seeded TestHandle timeline is empty, so first assert the empty guard, diff --git a/crates/opentake-agent/src/mcp/media_bridge.rs b/crates/opentake-agent/src/mcp/media_bridge.rs index 2bb0a993..3cf7e2ba 100644 --- a/crates/opentake-agent/src/mcp/media_bridge.rs +++ b/crates/opentake-agent/src/mcp/media_bridge.rs @@ -23,6 +23,7 @@ //! Both methods default to `Err("unsupported")` so a hand-rolled bridge (or the //! absence of one) never breaks the build. +use opentake_domain::ClipType; use opentake_media::{MediaCancelToken, TranscriptionResult}; use crate::tools::result::Block; @@ -66,6 +67,50 @@ pub struct InspectResult { pub height: u32, } +/// One raw-source frame produced for `inspect_media`. +#[derive(Debug, Clone)] +pub struct InspectedMediaFrame { + /// Actual source timestamp decoded for this image. + pub timestamp_seconds: f64, + /// Encoded image bytes (JPEG in the desktop bridge). + pub bytes: Vec, + /// MIME type of `bytes`. + pub media_type: String, +} + +/// Validated source-inspection request. The dispatcher owns tool arguments and +/// manifest/clip validation; the desktop bridge owns retained source resolution, +/// probing, decoding, and transcription. +#[derive(Debug, Clone)] +pub struct InspectMediaRequest { + pub media_ref: String, + pub kind: ClipType, + pub start_seconds: Option, + pub end_seconds: Option, + pub max_frames: usize, + pub overview: bool, +} + +/// Backend facts and content returned for `inspect_media`. The dispatcher turns +/// this neutral result into image blocks plus the compact upstream JSON shape. +#[derive(Debug, Clone)] +pub struct InspectMediaResult { + pub frames: Vec, + /// Source timestamps represented by a single overview storyboard image. + /// Empty for ordinary per-frame inspection. + pub overview_timestamps: Vec, + pub duration_seconds: f64, + pub width: Option, + pub height: Option, + pub fps: Option, + pub has_audio: bool, + pub byte_size: u64, + pub transcript: Option, + /// True when visual inspection succeeded but local ASR was unavailable or + /// failed. Private backend diagnostics never cross this trait boundary. + pub transcription_unavailable: bool, +} + /// The outcome of an `import_media` call, mirroring upstream's `.ok("…")` string /// results. The dispatcher wraps `message` in a [`crate::tools::result::ToolResult`]. #[derive(Debug, Clone)] @@ -104,6 +149,16 @@ pub enum ImportSource { pub struct BridgeError { /// Private diagnostic text; never expose it directly to a model. pub message: String, + /// Fixed classification used to expose a safe recovery contract without + /// forwarding the private diagnostic. + pub kind: BridgeErrorKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BridgeErrorKind { + Private, + NotFound, + Unavailable, } impl BridgeError { @@ -111,6 +166,21 @@ impl BridgeError { pub fn new(message: impl Into) -> Self { BridgeError { message: message.into(), + kind: BridgeErrorKind::Private, + } + } + + pub fn not_found(message: impl Into) -> Self { + BridgeError { + message: message.into(), + kind: BridgeErrorKind::NotFound, + } + } + + pub fn unavailable(message: impl Into) -> Self { + BridgeError { + message: message.into(), + kind: BridgeErrorKind::Unavailable, } } } @@ -255,6 +325,18 @@ pub struct SearchMediaResult { /// so the [`Dispatcher`](super::dispatch::Dispatcher) can hold `Arc` across threads (matching [`CoreHandle`](super::core_handle)). pub trait MediaBridge: Send + Sync { + /// Inspect one source asset with real decoded frames and optional on-device + /// transcription. The default is explicitly unavailable so non-desktop + /// embedders do not advertise a fake success. + fn inspect_media( + &self, + _request: &InspectMediaRequest, + ) -> Result { + Err(BridgeError::new( + "inspect_media: source inspection is not available in this build", + )) + } + /// Transcribe each unique source for `get_transcript`, caching so a /// re-transcribe is instant. Per-source errors are returned inline (never /// fatal), matching upstream's skip-don't-fail loop. The default reports @@ -340,9 +422,18 @@ pub trait MediaBridge: Send + Sync { /// bytes (rmcp image content is base64). Kept here so the dispatcher stays free of /// encoding concerns. pub fn frame_to_block(frame: &InspectedFrame) -> Block { + encoded_image_to_block(&frame.bytes, &frame.media_type) +} + +/// Convert a raw-source inspection frame into an MCP image block. +pub fn media_frame_to_block(frame: &InspectedMediaFrame) -> Block { + encoded_image_to_block(&frame.bytes, &frame.media_type) +} + +fn encoded_image_to_block(bytes: &[u8], media_type: &str) -> Block { use base64::Engine as _; - let b64 = base64::engine::general_purpose::STANDARD.encode(&frame.bytes); - Block::image(b64, frame.media_type.clone()) + let b64 = base64::engine::general_purpose::STANDARD.encode(bytes); + Block::image(b64, media_type) } #[cfg(test)] @@ -361,6 +452,22 @@ mod tests { assert!(err.message.contains("not available"), "{}", err.message); } + #[test] + fn default_inspect_media_is_unsupported() { + let b = NoopBridge; + let err = b + .inspect_media(&InspectMediaRequest { + media_ref: "asset".into(), + kind: ClipType::Video, + start_seconds: None, + end_seconds: None, + max_frames: 6, + overview: false, + }) + .unwrap_err(); + assert!(err.message.contains("not available"), "{}", err.message); + } + #[test] fn default_import_media_is_unsupported() { let b = NoopBridge; diff --git a/crates/opentake-agent/src/mcp/server.rs b/crates/opentake-agent/src/mcp/server.rs index 4c676f99..d95af475 100644 --- a/crates/opentake-agent/src/mcp/server.rs +++ b/crates/opentake-agent/src/mcp/server.rs @@ -566,7 +566,7 @@ mod tests { } #[test] - fn lists_all_44_tools() { + fn lists_every_advertised_tool() { assert_eq!(McpServer::tools().len(), ToolName::ALL.len()); // Names round-trip to the wire names. let names: Vec = McpServer::tools() diff --git a/crates/opentake-agent/src/prompt/base.rs b/crates/opentake-agent/src/prompt/base.rs index a7df9837..58d2ee2b 100644 --- a/crates/opentake-agent/src/prompt/base.rs +++ b/crates/opentake-agent/src/prompt/base.rs @@ -5,6 +5,8 @@ //! sentences (frame math, the short-id "pass back verbatim" rule, the //! transcript-driven warning, the calm HIG voice) are kept VERBATIM. +use crate::tools::names::ToolName; + /// Section: who you are + the timeline model. Keeps the short-id contract /// sentence verbatim — without it the short-id system (`tools::short_id`) breaks. pub const CORE_MODEL: &str = "You are a creative AI assistant connected to OpenTake, an AI-native video editor. Help the user build and edit their project by calling the tools this server exposes.\n\n# Core model\n- The timeline has a fixed fps and resolution. All timing is in FRAMES, not seconds: frame = seconds × fps.\n- Tracks are ordered and typed (video or audio). Video clips, images, and text overlays all live on video tracks.\n- A clip references a media asset and occupies [startFrame, startFrame + durationFrames) on its track.\n- Clips have trimStartFrame / trimEndFrame (source-media offsets, not timeline offsets), speed, volume, and opacity.\n- Media assets live in a project library and are referenced by ID. They may be user-imported or AI-generated.\n- IDs (clipId, mediaRef, folderId, captionGroupId) are returned as short prefixes. Pass them back exactly as given — never pad, complete, or guess a longer form."; @@ -12,6 +14,11 @@ pub const CORE_MODEL: &str = "You are a creative AI assistant connected to OpenT /// Section: the always-do checklist (read-before-edit, model gating). pub const ALWAYS_DO: &str = "# Always do\n- Call get_timeline once per session (or after an out-of-band change) for fps, tracks, and existing clip frames. Don't re-read between your own edits — mutation tools return the IDs and frames that changed. Re-read only after a failure that suggests your model is stale. Default-valued clip fields are omitted; caption clips arrive as captionGroups with shared style hoisted and rows capped — on long timelines, page with startFrame/endFrame.\n- Call get_media before referencing any asset — every mediaRef comes from there.\n- Call list_models before generate_video, generate_image, generate_audio, or upscale_media so the model you pick supports the duration, aspect ratio, references, voice, or asset type you need.\n- get_timeline returns canGenerate. If false, every generation and upscale tool will fail — tell the user to sign in to OpenTake and subscribe before proposing them. (inspect_media transcription runs on-device and is unaffected.)\n- Before describing any user-supplied asset (referenceMediaRefs, startFrameMediaRef, etc.), call inspect_media and describe what you actually see — never paraphrase the filename. On long media, work coarse to fine: overview=true for a storyboard image, read the transcript segments, then zoom into a window with startSeconds/endSeconds for full frames. Plan splits, trims, and captions from segment timestamps; wordTimestamps=true on a narrow window for exact word boundaries.\n- To find a moment across the library (\"the sunset shot\", \"where she mentions the budget\"), call search_media before inspecting files one by one — describe what's on screen or quote the words said. Hits are source-second ranges ready to convert into add_clips trims."; +/// Always-do checklist when paid generation is deliberately absent from the +/// advertised tool catalog. The prompt must never teach the model to call a +/// capability that discovery does not expose. +pub const ALWAYS_DO_WITHOUT_GENERATION: &str = "# Always do\n- Call get_timeline once per session (or after an out-of-band change) for fps, tracks, and existing clip frames. Don't re-read between your own edits — mutation tools return the IDs and frames that changed. Re-read only after a failure that suggests your model is stale. Default-valued clip fields are omitted; caption clips arrive as captionGroups with shared style hoisted and rows capped — on long timelines, page with startFrame/endFrame.\n- Call get_media before referencing any asset — every mediaRef comes from there.\n- Before describing any user-supplied asset, call inspect_media and describe what you actually see — never paraphrase the filename. On long media, work coarse to fine: overview=true for a storyboard image, read the transcript segments, then zoom into a window with startSeconds/endSeconds for full frames. Plan splits, trims, and captions from segment timestamps; wordTimestamps=true on a narrow window for exact word boundaries.\n- To find a moment across the library (\"the sunset shot\", \"where she mentions the budget\"), call search_media before inspecting files one by one — describe what's on screen or quote the words said. Hits are source-second ranges ready to convert into add_clips trims."; + /// Section: editing surface + the transcript-driven warning (kept verbatim). pub const EDITING: &str = "# Editing\n- Placements must match track type: video on video tracks, audio on audio tracks.\n- The clip-editing surface mirrors human gestures — one tool per gesture, applied to a selection:\n • move_clips: change track and/or startFrame. Linked partners follow the frame delta; track changes don't propagate.\n • set_clip_properties: apply the same values (durationFrames, trim, speed, volume, opacity, transform, reversed, or text-style fields) to one or more clipIds. For per-clip differences, make separate calls. Setting volume or opacity here clears any existing keyframes on that property.\n • set_keyframes: replace the keyframe track for one (clipId, property) pair. Empty array clears. Frames are clip-relative.\n • split_clip: atFrame must be strictly inside the clip.\n- speed 1.0 is normal; <1.0 stretches the clip longer on the timeline; >1.0 shortens it. trim* values are source offsets, not timeline offsets. reversed=true plays a video clip backward through the same trimmed source window.\n- Edits are undoable and effectively free. Don't ask permission for individual edits — just explain what you changed.\n- Transcript-driven cuts (filler, dead air, duplicate/retake removal): read the WORD-level get_transcript end-to-end as prose at least once before deduping. The segments view and the ripple_delete diff are lossy — they hide reworded retakes (\"in one state\" vs \"in one place\") and sub-frame seam fragments (a word whose start == end rounds to zero frames). Verify a suspected dangling fragment against the words, not the summary."; @@ -33,24 +40,34 @@ pub const COMMUNICATION: &str = "# Communication\n- Default to one or two senten /// The model-strategy placeholder token replaced at assembly time. pub const MODEL_STRATEGY_TOKEN: &str = "{MODEL_STRATEGY}"; +fn generation_tools_are_advertised() -> bool { + [ + ToolName::GenerateVideo, + ToolName::GenerateImage, + ToolName::GenerateAudio, + ToolName::UpscaleMedia, + ] + .iter() + .all(|tool| ToolName::ALL.contains(tool)) +} + /// All sections in order, joined into the base prompt. `model_strategy` fills /// the generation placeholder (empty string drops the token cleanly). pub fn base_prompt(model_strategy: &str) -> String { - let generation = if model_strategy.is_empty() { - GENERATION.replace(MODEL_STRATEGY_TOKEN, "") + let mut sections = vec![CORE_MODEL.to_owned()]; + if generation_tools_are_advertised() { + sections.push(ALWAYS_DO.to_owned()); + let generation = GENERATION.replace(MODEL_STRATEGY_TOKEN, model_strategy); + sections.push(EDITING.to_owned()); + sections.push(generation); + sections.push(AUDIO_GENERATION.to_owned()); + sections.push(PROMPT_CRAFT.to_owned()); } else { - GENERATION.replace(MODEL_STRATEGY_TOKEN, model_strategy) - }; - [ - CORE_MODEL, - ALWAYS_DO, - EDITING, - &generation, - AUDIO_GENERATION, - PROMPT_CRAFT, - COMMUNICATION, - ] - .join("\n\n") + sections.push(ALWAYS_DO_WITHOUT_GENERATION.to_owned()); + sections.push(EDITING.to_owned()); + } + sections.push(COMMUNICATION.to_owned()); + sections.join("\n\n") } #[cfg(test)] @@ -92,14 +109,32 @@ mod tests { } #[test] - fn model_strategy_token_replaced() { + fn model_strategy_is_ignored_while_generation_is_hidden() { let with = base_prompt("Use Model X for video."); - assert!(with.contains("Use Model X for video.")); + assert!(!with.contains("Use Model X for video.")); assert!(!with.contains(MODEL_STRATEGY_TOKEN)); let without = base_prompt(""); assert!(!without.contains(MODEL_STRATEGY_TOKEN)); } + #[test] + fn prompt_does_not_teach_hidden_tools() { + let prompt = base_prompt("default"); + for hidden in [ + "generate_video", + "generate_image", + "generate_audio", + "upscale_media", + "add_motion_graphic", + "edit_motion_graphic", + ] { + assert!( + !prompt.contains(hidden), + "prompt advertises hidden {hidden}" + ); + } + } + #[test] fn signin_uses_opentake() { assert!(ALWAYS_DO.contains("sign in to OpenTake and subscribe")); diff --git a/crates/opentake-agent/src/tools/names.rs b/crates/opentake-agent/src/tools/names.rs index 1f19d1ba..5fee8fbf 100644 --- a/crates/opentake-agent/src/tools/names.rs +++ b/crates/opentake-agent/src/tools/names.rs @@ -113,8 +113,55 @@ impl ToolName { } } - /// All tools in registration order. - pub const ALL: [ToolName; 44] = [ + /// Tools advertised to MCP and in-app Chat in registration order. Provider- + /// backed generation and Motion Canvas tools remain known wire names, but + /// stay out of discovery until their production backends are connected. + pub const ALL: [ToolName; 38] = [ + ToolName::GetTimeline, + ToolName::GetMedia, + ToolName::InspectMedia, + ToolName::GetTranscript, + ToolName::InspectTimeline, + ToolName::SearchMedia, + ToolName::ListModels, + ToolName::AddClips, + ToolName::InsertClips, + ToolName::RemoveClips, + ToolName::RemoveTracks, + ToolName::MoveClips, + ToolName::SetClipProperties, + ToolName::SetKeyframes, + ToolName::SplitClip, + ToolName::RippleDeleteRanges, + ToolName::Undo, + ToolName::AddTexts, + ToolName::AddCaptions, + ToolName::DetectBeats, + ToolName::AutoCutToBeats, + ToolName::SmartReframe, + ToolName::TightenSilences, + ToolName::ImportMedia, + ToolName::ListFolders, + ToolName::CreateFolder, + ToolName::MoveToFolder, + ToolName::RenameMedia, + ToolName::RenameFolder, + ToolName::DeleteMedia, + ToolName::DeleteFolder, + ToolName::ActivateWorkflow, + ToolName::ListWorkflows, + ToolName::DeactivateWorkflow, + ToolName::SetColorGrade, + ToolName::ChromaKey, + ToolName::SetMask, + ToolName::ApplyEffect, + ]; + + /// Every recognized schema/wire name, including capabilities deliberately + /// hidden from discovery until a real backend exists. Keeping this set lets + /// strict argument validation and compatibility tests cover future tools + /// without advertising placeholder behavior to models. + pub const KNOWN: [ToolName; 44] = [ ToolName::GetTimeline, ToolName::GetMedia, ToolName::InspectMedia, @@ -200,7 +247,7 @@ impl ToolName { impl FromStr for ToolName { type Err = (); fn from_str(s: &str) -> Result { - ToolName::ALL + ToolName::KNOWN .iter() .copied() .find(|t| t.as_str() == s) @@ -218,8 +265,12 @@ mod tests { } #[test] - fn all_set_is_44() { - assert_eq!(ToolName::ALL.len(), 44); + fn advertised_set_is_38_and_known_set_is_44() { + assert_eq!(ToolName::ALL.len(), 38); + assert_eq!(ToolName::KNOWN.len(), 44); + assert!(ToolName::ALL + .iter() + .all(|tool| ToolName::KNOWN.contains(tool))); } #[test] @@ -247,14 +298,17 @@ mod tests { for t in [ToolName::AddMotionGraphic, ToolName::EditMotionGraphic] { assert_eq!(ToolName::from_str(t.as_str()), Ok(t)); } - // They are present in ALL exactly once each. + // They remain known for schema compatibility, but are not advertised + // until the production Motion Canvas renderer is wired. assert_eq!( - ToolName::ALL + ToolName::KNOWN .iter() .filter(|t| matches!(t, ToolName::AddMotionGraphic | ToolName::EditMotionGraphic)) .count(), 2 ); + assert!(!ToolName::ALL.contains(&ToolName::AddMotionGraphic)); + assert!(!ToolName::ALL.contains(&ToolName::EditMotionGraphic)); // ...and are NOT part of the 31 upstream tools. assert!(!ToolName::UPSTREAM.contains(&ToolName::AddMotionGraphic)); assert!(!ToolName::UPSTREAM.contains(&ToolName::EditMotionGraphic)); diff --git a/crates/opentake-agent/src/tools/result.rs b/crates/opentake-agent/src/tools/result.rs index 738ace36..c5d647ee 100644 --- a/crates/opentake-agent/src/tools/result.rs +++ b/crates/opentake-agent/src/tools/result.rs @@ -51,6 +51,8 @@ pub struct ToolResult { pub(crate) enum PublicErrorKind { UnknownTool, InvalidArguments(ToolName), + ResourceNotFound(ToolName), + CapabilityUnavailable(ToolName), } impl PublicErrorKind { @@ -58,6 +60,8 @@ impl PublicErrorKind { match self { Self::UnknownTool => "MCP_UNKNOWN_TOOL", Self::InvalidArguments(_) => "MCP_INVALID_ARGUMENTS", + Self::ResourceNotFound(_) => "MCP_RESOURCE_NOT_FOUND", + Self::CapabilityUnavailable(_) => "MCP_CAPABILITY_UNAVAILABLE", } } @@ -65,6 +69,10 @@ impl PublicErrorKind { match self { Self::UnknownTool => "The requested tool is not available.", Self::InvalidArguments(_) => "The tool request has invalid arguments.", + Self::ResourceNotFound(_) => "The referenced project resource was not found.", + Self::CapabilityUnavailable(_) => { + "This capability is unavailable for the referenced media." + } } } @@ -72,6 +80,12 @@ impl PublicErrorKind { match self { Self::UnknownTool => "Choose a tool returned by the current tool catalog, then retry.", Self::InvalidArguments(_) => "Correct the reported arguments, then retry.", + Self::ResourceNotFound(_) => { + "Refresh project state, choose an existing resource ID, then retry." + } + Self::CapabilityUnavailable(_) => { + "Use a supported source type or restore the source media, then retry." + } } } } diff --git a/crates/opentake-agent/tests/advertised_tool_acceptance.rs b/crates/opentake-agent/tests/advertised_tool_acceptance.rs new file mode 100644 index 00000000..19526565 --- /dev/null +++ b/crates/opentake-agent/tests/advertised_tool_acceptance.rs @@ -0,0 +1,91 @@ +use std::path::PathBuf; +use std::sync::{Arc, RwLock}; + +use opentake_agent::mcp::core_handle::CoreHandle; +use opentake_agent::mcp::dispatch::Dispatcher; +use opentake_agent::plugin::registry::PluginRegistry; +use opentake_agent::tools::names::ToolName; +use opentake_domain::{MediaManifest, Timeline}; +use opentake_ops::{EditCommand, EditResult}; + +struct ReadOnlyHandle; + +impl CoreHandle for ReadOnlyHandle { + fn timeline(&self) -> Timeline { + Timeline::new() + } + + fn media(&self) -> MediaManifest { + MediaManifest::new() + } + + fn apply(&self, _cmd: EditCommand) -> anyhow::Result { + anyhow::bail!("advertised-tool fixture is read-only") + } + + fn project_dir(&self) -> Option { + None + } +} + +#[test] +fn every_advertised_tool_is_live_or_absent() { + let dispatcher = Dispatcher::new( + Arc::new(ReadOnlyHandle), + Arc::new(RwLock::new(PluginRegistry::new())), + ); + let cases = [ + ( + ToolName::InspectMedia, + serde_json::json!({"mediaRef": "asset"}), + ), + ( + ToolName::GenerateVideo, + serde_json::json!({"prompt": "clip"}), + ), + ( + ToolName::GenerateImage, + serde_json::json!({"prompt": "still"}), + ), + ( + ToolName::GenerateAudio, + serde_json::json!({"prompt": "music"}), + ), + ( + ToolName::UpscaleMedia, + serde_json::json!({"mediaRef": "asset"}), + ), + ( + ToolName::AddMotionGraphic, + serde_json::json!({ + "source": {"code": "export default {}"}, + "startFrame": 0, + "durationFrames": 30 + }), + ), + ( + ToolName::EditMotionGraphic, + serde_json::json!({"clipId": "clip", "code": "export default {}"}), + ), + ]; + + for (tool, args) in cases { + if !ToolName::ALL.contains(&tool) { + let result = dispatcher.dispatch(tool.as_str(), args); + assert!( + result.text_joined().contains("not advertised"), + "{} is hidden from discovery but direct dispatch did not fail closed: {}", + tool.as_str(), + result.text_joined() + ); + continue; + } + let result = dispatcher.dispatch(tool.as_str(), args); + assert!( + !result.text_joined().contains("not yet implemented"), + "{} is advertised but still reaches a placeholder stub: {}", + tool.as_str(), + result.text_joined() + ); + } +} diff --git a/crates/opentake-agent/tests/tool_argument_contract.rs b/crates/opentake-agent/tests/tool_argument_contract.rs index 209bdc40..96c54b61 100644 --- a/crates/opentake-agent/tests/tool_argument_contract.rs +++ b/crates/opentake-agent/tests/tool_argument_contract.rs @@ -307,7 +307,7 @@ fn all_tool_schemas_reject_unknown_missing_wrong_type_and_nonfinite() { }), ); assert!( - motion.text_joined().contains("not yet implemented"), + motion.text_joined().contains("not advertised"), "dynamic motion params must remain open: {}", motion.text_joined() ); diff --git a/docs/architecture/BUGS.md b/docs/architecture/BUGS.md index 173671f5..83c6d339 100644 --- a/docs/architecture/BUGS.md +++ b/docs/architecture/BUGS.md @@ -59,12 +59,12 @@ | **描述** | GPU 合成的 infrastructure 已就绪（`composite_frame` Tauri 命令、`useTimelineFrame` hook 均存在），但 `Preview.tsx` 仍然使用 DOM `