From a9cd9385b57587cd939c20b66d8884a00c746dca Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 16:01:02 +0800 Subject: [PATCH 001/217] fix: harden beta CI baseline --- Cargo.lock | 8 ++++---- src-tauri/src/media/prewarm.rs | 5 +++++ web/package.json | 1 + web/pnpm-lock.yaml | 19 +++++++++++-------- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2687a82d..b7c0a0e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,9 +85,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" [[package]] name = "arbitrary" @@ -2746,9 +2746,9 @@ checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" [[package]] name = "memmap2" -version = "0.9.10" +version = "0.9.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "714098028fe011992e1c3962653c96b2d578c4b4bce9036e15ff220319b1e0e3" +checksum = "d1219ed1b7f229ee7104d281dd01d6802fe28bb6e95d292942c4daacdeb798c0" dependencies = [ "libc", ] diff --git a/src-tauri/src/media/prewarm.rs b/src-tauri/src/media/prewarm.rs index cb089716..d3e87bf2 100644 --- a/src-tauri/src/media/prewarm.rs +++ b/src-tauri/src/media/prewarm.rs @@ -818,13 +818,18 @@ mod tests { #[test] fn new_epoch_can_schedule_same_cache_key_after_old_reservation_drops() { let scheduler = PrewarmScheduler::new(1); + let (entered_tx, entered_rx) = mpsc::channel(); let (release_tx, release_rx) = mpsc::channel(); assert_eq!( scheduler.schedule(1, PrewarmKind::PreviewPoster, "shared", false, move |_| { + entered_tx.send(()).expect("announce old epoch entry"); release_rx.recv().expect("release old epoch"); }), PrewarmResult::Queued ); + entered_rx + .recv_timeout(Duration::from_secs(2)) + .expect("old epoch worker starts"); scheduler .begin_project_transition() .expect("begin transition"); diff --git a/web/package.json b/web/package.json index 354adf37..5bb86adf 100644 --- a/web/package.json +++ b/web/package.json @@ -23,6 +23,7 @@ "@types/react-dom": "^18.3.1", "@vitejs/plugin-react": "^6.0.3", "happy-dom": "^20.10.6", + "postcss": "^8.5.18", "typescript": "^5.6.3", "vite": "^8.1.3", "vitest": "^4.1.10" diff --git a/web/pnpm-lock.yaml b/web/pnpm-lock.yaml index 4d06afd1..791381bb 100644 --- a/web/pnpm-lock.yaml +++ b/web/pnpm-lock.yaml @@ -42,6 +42,9 @@ importers: happy-dom: specifier: ^20.10.6 version: 20.10.6 + postcss: + specifier: ^8.5.18 + version: 8.5.24 typescript: specifier: ^5.6.3 version: 5.9.3 @@ -475,8 +478,8 @@ packages: magic-string@0.30.21: resolution: {integrity: sha512-vd2F4YUyEXKGcLHoq+TEyCjxueSeHnFxyyjNp80yg0XV4vUhnDer/lvvlqM/arB5bXQN5K2/3oinyCRyx8T2CQ==} - nanoid@3.3.15: - resolution: {integrity: sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==} + nanoid@3.3.16: + resolution: {integrity: sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==} engines: {node: ^10 || ^12 || ^13.7 || ^14 || >=15.0.1} hasBin: true @@ -494,8 +497,8 @@ packages: resolution: {integrity: sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==} engines: {node: '>=12'} - postcss@8.5.16: - resolution: {integrity: sha512-vuwillviilfKZsg0VGj5R/YwwcHx4SLsIOI/7K6mQkWx+l5cUHTjj5g0AasTBcyXsbfTgrwsUNmVUb5xVwyPwg==} + postcss@8.5.24: + resolution: {integrity: sha512-8RyVklq0owXUTa4xlpzu4l9AaVKIdQvAcOHZWaMh98HgySsUtxRVf/chRe3dsSLqb6i40BzGRzEUddRaI+9TSw==} engines: {node: ^10 || ^12 || >=14} react-dom@18.3.1: @@ -1000,7 +1003,7 @@ snapshots: dependencies: '@jridgewell/sourcemap-codec': 1.5.5 - nanoid@3.3.15: {} + nanoid@3.3.16: {} obug@2.1.3: {} @@ -1010,9 +1013,9 @@ snapshots: picomatch@4.0.5: {} - postcss@8.5.16: + postcss@8.5.24: dependencies: - nanoid: 3.3.15 + nanoid: 3.3.16 picocolors: 1.1.1 source-map-js: 1.2.1 @@ -1081,7 +1084,7 @@ snapshots: dependencies: lightningcss: 1.32.0 picomatch: 4.0.5 - postcss: 8.5.16 + postcss: 8.5.24 rolldown: 1.1.4 tinyglobby: 0.2.17 optionalDependencies: From 0af21f342bb307377aab5bfe37095c3fa09009c6 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 17:26:45 +0800 Subject: [PATCH 002/217] test: skip playback probes without GPU --- src-tauri/tests/playback_integration.rs | 43 +++++++++++++++---------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src-tauri/tests/playback_integration.rs b/src-tauri/tests/playback_integration.rs index ebbbfc43..67b7088e 100644 --- a/src-tauri/tests/playback_integration.rs +++ b/src-tauri/tests/playback_integration.rs @@ -161,15 +161,8 @@ fn render_until_content(rl: &mut RenderLoop, target: i32, w: u32, h: u32) -> Opt None } -fn required_render_loop( - timeline: Timeline, - manifest: &MediaManifest, - render_size: RenderSize, -) -> RenderLoop { - let (sizes, media) = project_media(manifest, &None); - let text = project_text(&timeline); - RenderLoop::new(timeline, media, text, sizes, render_size) - .expect("exact-bootstrap integration requires a GPU adapter") +fn gpu_ready(render_size: RenderSize) -> bool { + try_render_loop(Timeline::new(), &MediaManifest::new(), render_size).is_some() } struct ManualClock(AtomicI32); @@ -196,10 +189,10 @@ impl PlaybackClock for ManualClock { #[test] fn cold_bootstrap_uses_exact_trimmed_source_frame() { - assert!( - ffmpeg_ready(), - "exact-bootstrap integration requires ffmpeg" - ); + if !ffmpeg_ready() { + eprintln!("skip: ffmpeg not available"); + return; + } let dir = tempfile::tempdir().expect("fixture tempdir"); let src = dir.path().join("distinct-cfr.mp4"); let (w, h, fps, frames, source_frame) = (160u32, 90u32, 12u32, 12u32, 5i32); @@ -232,7 +225,9 @@ fn cold_bootstrap_uses_exact_trimmed_source_frame() { "asset-1", &src, w as i32, h as i32, fps as f64, )); - let mut render_loop = required_render_loop(timeline, &manifest, RenderSize::new(w, h)); + let Some(mut render_loop) = try_render_loop(timeline, &manifest, RenderSize::new(w, h)) else { + return; + }; let first = render_loop .render_frame(0) .expect("cold bootstrap must render the trimmed source frame"); @@ -262,7 +257,9 @@ fn cold_bootstrap_decode_failure_is_reported_instead_of_publishing_black() { "asset-1", &missing, w as i32, h as i32, fps as f64, )); - let mut render_loop = required_render_loop(timeline, &manifest, RenderSize::new(w, h)); + let Some(mut render_loop) = try_render_loop(timeline, &manifest, RenderSize::new(w, h)) else { + return; + }; let error = match render_loop.render_frame(0) { Ok(frame) => panic!( "cold bootstrap decode failure published {}x{} black frame", @@ -278,7 +275,13 @@ fn cold_bootstrap_decode_failure_is_reported_instead_of_publishing_black() { #[test] fn cancelling_initial_ready_bootstrap_releases_the_readiness_worker() { - assert!(ffmpeg_ready(), "cancellation integration requires ffmpeg"); + if !ffmpeg_ready() { + eprintln!("skip: ffmpeg not available"); + return; + } + if !gpu_ready(RenderSize::new(2, 2)) { + return; + } let dir = tempfile::tempdir().expect("fixture tempdir"); let fifo = dir.path().join("blocked-initial-source"); let status = Command::new("mkfifo") @@ -355,7 +358,13 @@ fn cancelling_initial_ready_bootstrap_releases_the_readiness_worker() { #[test] fn stopping_running_engine_cancels_blocked_cold_bootstrap() { - assert!(ffmpeg_ready(), "cancellation integration requires ffmpeg"); + if !ffmpeg_ready() { + eprintln!("skip: ffmpeg not available"); + return; + } + if !gpu_ready(RenderSize::new(2, 2)) { + return; + } let dir = tempfile::tempdir().expect("fixture tempdir"); let fifo = dir.path().join("blocked-source"); let status = Command::new("mkfifo") From 97f862c8b7e1f9658e3fd3a1fedb451d7ab5b477 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 17:48:56 +0800 Subject: [PATCH 003/217] test: close cross-cutting data safety gates --- crates/opentake-agent/src/mcp/dispatch.rs | 14 ++++ crates/opentake-core/src/core.rs | 14 ++++ crates/opentake-ops/tests/command_apply.rs | 12 +++ .../opentake-project/tests/schema_compat.rs | 13 +++ scripts/tests/validate-c1b-ci-test.rb | 18 +++++ scripts/tests/validate-c1b-evidence-test.rb | 79 ++++++++++++------- scripts/validate-c1b-ci.rb | 22 +++++- 7 files changed, 142 insertions(+), 30 deletions(-) diff --git a/crates/opentake-agent/src/mcp/dispatch.rs b/crates/opentake-agent/src/mcp/dispatch.rs index 89a8a3b1..ac21fe62 100644 --- a/crates/opentake-agent/src/mcp/dispatch.rs +++ b/crates/opentake-agent/src/mcp/dispatch.rs @@ -5614,4 +5614,18 @@ mod tests { ); assert!(r.is_error); } + + /// Composite acceptance entry tracked by the data-safety implementation plan. + /// Keep this as an executable roll-up of the owning MCP boundary tests so the + /// audit command proves validation, mutation, undo, and bridge fail-closed + /// behavior together rather than merely matching a test name. + #[test] + fn cross_cutting_mcp_acceptance() { + precise_path_arg_error_mentions_field(); + add_clips_then_get_timeline_reflects_clip(); + add_captions_is_one_undo_step(); + undo_with_empty_stack_errors(); + import_media_bytes_rejects_oversized_base64_before_bridge(); + import_media_rejects_unknown_nested_source_key(); + } } diff --git a/crates/opentake-core/src/core.rs b/crates/opentake-core/src/core.rs index 49651edc..fc3fc772 100644 --- a/crates/opentake-core/src/core.rs +++ b/crates/opentake-core/src/core.rs @@ -2145,4 +2145,18 @@ mod tests { ] ); } + + /// Composite acceptance entry tracked by the data-safety implementation + /// plan. These child slices cover authoritative versions/events, stale edit + /// refusal, manifest undo, coherent concurrent snapshots, and save/reopen. + #[test] + fn cross_cutting_runtime_acceptance() { + apply_bumps_version_and_emits_once(); + deferred_apply_rejects_version_and_project_drift_without_mutation(); + manifest_edit_and_undo_emit_media_changed(); + undo_redo_through_core_bumps_version_and_emits(); + runtime_snapshot_never_mixes_timeline_media_and_project_dir(); + open_save_roundtrip_through_core_emits_lifecycle_events(); + prepared_media_batch_writer_failure_restores_full_editor_state(); + } } diff --git a/crates/opentake-ops/tests/command_apply.rs b/crates/opentake-ops/tests/command_apply.rs index f84f8099..d0ac67e9 100644 --- a/crates/opentake-ops/tests/command_apply.rs +++ b/crates/opentake-ops/tests/command_apply.rs @@ -1702,3 +1702,15 @@ fn swap_media_does_not_cascade_to_link_group_with_different_ref() { assert_eq!(v_clip.media_ref, "new_v"); assert_eq!(a_clip.media_ref, "other"); // untouched } + +/// Composite acceptance entry tracked by the data-safety implementation plan. +/// It rolls up command validation, linked edits, collision refusal, no-op +/// semantics, and undo/redo through the public `apply` boundary. +#[test] +fn cross_cutting_command_acceptance() { + add_clips_rejects_incompatible_type(); + split_linked_pair_splits_partner_and_regroups(); + ripple_delete_ranges_refuses_when_sync_follower_collides(); + undo_redo_restores_and_versions(); + unchanged_command_does_not_push_undo_or_bump_version(); +} diff --git a/crates/opentake-project/tests/schema_compat.rs b/crates/opentake-project/tests/schema_compat.rs index b2c1f799..d71d8ff4 100644 --- a/crates/opentake-project/tests/schema_compat.rs +++ b/crates/opentake-project/tests/schema_compat.rs @@ -458,3 +458,16 @@ fn known_schema_remains_writable() { assert_eq!(saved_as.timeline.fps, 60); assert!(!saved_as.compatibility().is_read_only()); } + +/// Composite acceptance entry tracked by the data-safety implementation plan. +/// It exercises strict required components, read-only recovery for optional +/// corruption, unknown-field preservation, and the writable save/reopen path. +#[test] +fn cross_cutting_project_safety_acceptance() { + unknown_top_level_timeline_field_blocks_writes_without_changing_bytes(); + unknown_nested_manifest_entry_and_source_fields_block_writes(); + malformed_optional_generation_log_opens_but_blocks_writes(); + malformed_manifest_contract_matches_authoritative_source(); + trailing_required_json_remains_a_strict_open_error(); + known_schema_remains_writable(); +} diff --git a/scripts/tests/validate-c1b-ci-test.rb b/scripts/tests/validate-c1b-ci-test.rb index 371a027a..90f8d5c6 100755 --- a/scripts/tests/validate-c1b-ci-test.rb +++ b/scripts/tests/validate-c1b-ci-test.rb @@ -35,6 +35,8 @@ def run_validator(path, red_harness = RED_HARNESS) raw = File.read(WORKFLOW) assert(raw.include?(%q{printf ' %q' "$@"}), "native gate logs lack an exact command marker") +windows_product = raw[/^ windows-product:\n.*?(?=^ windows-security:)/m] +assert(windows_product, "canonical workflow lacks the Windows product job") red_harness_raw = File.read(RED_HARNESS) assert( red_harness_raw.lines.map(&:strip).reject(&:empty?).last == "exit 0", @@ -101,6 +103,22 @@ def run_validator(path, red_harness = RED_HARNESS) end structural_mutations = { + "missing-windows-product" => raw.sub(" windows-product:\n", " disabled-windows-product:\n"), + "windows-product-target-not-bound" => raw.sub( + windows_product, + windows_product.sub( + "TARGET_SHA: ${{ github.event_name == 'workflow_dispatch' && inputs.commit_sha || github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}", + "TARGET_SHA: ${{ github.sha }}" + ) + ), + "windows-product-checkout-not-bound" => raw.sub( + windows_product, + windows_product.sub("ref: ${{ env.TARGET_SHA }}", "ref: main") + ), + "windows-product-checkout-persists-credentials" => raw.sub( + windows_product, + windows_product.sub("persist-credentials: false", "persist-credentials: true") + ), "extra-normal-job" => raw.sub("jobs:\n", <<~YAML), jobs: rogue-normal: diff --git a/scripts/tests/validate-c1b-evidence-test.rb b/scripts/tests/validate-c1b-evidence-test.rb index ded1f3c4..ee8d00d6 100755 --- a/scripts/tests/validate-c1b-evidence-test.rb +++ b/scripts/tests/validate-c1b-evidence-test.rb @@ -11,7 +11,6 @@ ROOT = File.expand_path("../..", __dir__) POLICY = JSON.parse(File.read(File.join(ROOT, "scripts/c1b-evidence-policy.json"))) -VALIDATOR = File.join(ROOT, "scripts/validate-c1b-evidence.rb") def assert(condition, message) raise message unless condition @@ -21,12 +20,29 @@ def assert(condition, message) "bootstrap allowlist") assert(POLICY.fetch("dispatcher_ref") == "refs/heads/main", "trusted dispatcher ref") -def git(*arguments) - output, status = Open3.capture2("git", "-C", ROOT, *arguments) +def git(repo, *arguments) + output, status = Open3.capture2("git", "-C", repo, *arguments) raise "git #{arguments.join(' ')} failed" unless status.success? output.strip end +def prepare_bootstrap_repository(temporary) + repo = File.join(temporary, "repository") + _stdout, stderr, status = Open3.capture3("git", "clone", "--shared", "--no-hardlinks", ROOT, repo) + raise "git clone failed: #{stderr}" unless status.success? + + %w[validate-c1b-ci.rb validate-c1b-evidence.rb c1b-evidence-policy.json].each do |name| + FileUtils.cp(File.join(ROOT, "scripts", name), File.join(repo, "scripts", name)) + end + File.write(File.join(repo, "scripts", ".c1b-evidence-test-bootstrap"), + "synthetic bootstrap commit for validator tests\n") + git(repo, "config", "user.name", "OpenTake C1B Tests") + git(repo, "config", "user.email", "c1b-tests@opentake.invalid") + git(repo, "add", "scripts/") + git(repo, "commit", "-m", "test: create synthetic C1B bootstrap range") + File.realpath(repo) +end + def write_json(path, value) File.write(path, JSON.pretty_generate(value) + "\n") end @@ -65,16 +81,16 @@ def install_fake_gh(root) bin end -def build_fixture(root, label) - expected = git("rev-parse", "HEAD") - anchor = git("rev-parse", "HEAD^") +def build_fixture(root, label, repo, policy) + expected = git(repo, "rev-parse", "HEAD") + anchor = git(repo, "rev-parse", "HEAD^") nonce = Digest::SHA256.hexdigest(label)[0, 16] gate = File.join(root, "c1b-bootstrap-#{expected}-#{nonce}") fixture = File.join(root, "#{label}-live") FileUtils.mkdir_p([gate, fixture, File.join(gate, "reviews")]) run_id = "424242" dispatcher_sha = "f" * 40 - dispatcher_ref = POLICY.fetch("dispatcher_ref") + dispatcher_ref = policy.fetch("dispatcher_ref") File.write(File.join(gate, "run-id.txt"), "#{run_id}\n") File.write(File.join(gate, "pre-status.txt"), "") File.write(File.join(gate, "post-status.txt"), "") @@ -88,11 +104,11 @@ def build_fixture(root, label) File.write(File.join(gate, implementation), report.call("implementation")) timestamp = "2026-07-17T00:00:00Z" - ledger = POLICY.fetch("local_commands").map do |row| + ledger = policy.fetch("local_commands").map do |row| id = row.fetch("id") File.write(File.join(gate, "#{id}.log"), "synthetic #{id}\n") File.write(File.join(gate, "#{id}.raw-exit"), "0\n") - row.merge("cwd" => ROOT, "started_at_utc" => timestamp, + row.merge("cwd" => repo, "started_at_utc" => timestamp, "finished_at_utc" => timestamp, "exit_code" => 0, "log" => "#{id}.log", "raw_exit" => "#{id}.raw-exit") end @@ -102,12 +118,12 @@ def build_fixture(root, label) "id" => run_id.to_i, "run_attempt" => 1, "head_sha" => dispatcher_sha, "head_branch" => "main", "event" => "workflow_dispatch", "status" => "completed", "conclusion" => "success", "name" => "CI", - "path" => POLICY.fetch("workflow_file"), + "path" => policy.fetch("workflow_file"), "display_title" => "证据验证", "pull_requests" => [], - "repository" => { "full_name" => POLICY.fetch("repository") }, + "repository" => { "full_name" => policy.fetch("repository") }, } - jobs = POLICY.fetch("receipts").each_with_index.map do |receipt, index| + jobs = policy.fetch("receipts").each_with_index.map do |receipt, index| { "id" => 7000 + index, "run_id" => run_id.to_i, "run_attempt" => 1, "head_sha" => dispatcher_sha, "name" => "Safe filesystem (#{receipt.fetch('id')})", @@ -115,11 +131,11 @@ def build_fixture(root, label) } end artifacts = [] - POLICY.fetch("receipts").each_with_index do |receipt_policy, index| + policy.fetch("receipts").each_with_index do |receipt_policy, index| id = receipt_policy.fetch("id") directory = File.join(gate, "native-receipts", run_id, id) FileUtils.mkdir_p(directory) - commands = POLICY.fetch("native_commands").map do |row| + commands = policy.fetch("native_commands").map do |row| command_id = row.fetch("id") File.write(File.join(directory, "#{command_id}.log"), "synthetic #{command_id} 验证\n") File.write(File.join(directory, "#{command_id}.raw-exit"), "0\n") @@ -129,8 +145,8 @@ def build_fixture(root, label) File.write(File.join(directory, "final-aggregate.raw-exit"), "0\n") receipt = { "schema" => "opentake-c1b-native-receipt-v1", "receipt_id" => id, - "repository" => POLICY.fetch("repository"), "workflow" => POLICY.fetch("workflow"), - "workflow_file" => POLICY.fetch("workflow_file"), "job_id" => POLICY.fetch("job_id"), + "repository" => policy.fetch("repository"), "workflow" => policy.fetch("workflow"), + "workflow_file" => policy.fetch("workflow_file"), "job_id" => policy.fetch("job_id"), "event_name" => "workflow_dispatch", "run_id" => run_id, "run_attempt" => "1", "runner_label" => receipt_policy.fetch("runner"), "runner_os" => receipt_policy.fetch("os"), "runner_arch" => receipt_policy.fetch("arch"), "requested_sha" => expected, @@ -165,7 +181,7 @@ def build_fixture(root, label) end write_json(File.join(fixture, "workflow-content.json"), { "encoding" => "base64", "content" => Base64.strict_encode64( - File.binread(File.join(ROOT, POLICY.fetch("workflow_file")))) }) + File.binread(File.join(repo, policy.fetch("workflow_file")))) }) results = [ "Task: evidence-bootstrap", "Anchor SHA: #{anchor}", "Final SHA: #{expected}", "Run ID: #{run_id}", "Pre-status: clean", "Post-status: clean", @@ -175,18 +191,23 @@ def build_fixture(root, label) [gate, expected, anchor, spec, implementation, fixture] end -def run_validator(gate, expected, anchor, spec, implementation, fixture, fake_bin, extra_env = {}) +def run_validator(gate, expected, anchor, spec, implementation, fixture, fake_bin, repo, + validator, extra_env = {}) env = { "PATH" => "#{fake_bin}#{File::PATH_SEPARATOR}#{ENV.fetch('PATH', '')}", "C1B_FAKE_GH_ROOT" => fixture }.merge(extra_env) - Open3.capture3(env, RbConfig.ruby, VALIDATOR, gate, expected, anchor, - spec, implementation, ROOT) + Open3.capture3(env, RbConfig.ruby, validator, gate, expected, anchor, + spec, implementation, repo) end Dir.mktmpdir("c1b-evidence-test") do |temporary| + repo = prepare_bootstrap_repository(temporary) + policy = JSON.parse(File.read(File.join(repo, "scripts/c1b-evidence-policy.json"))) + validator = File.join(repo, "scripts/validate-c1b-evidence.rb") fake_bin = install_fake_gh(temporary) - gate, expected, anchor, spec, implementation, fixture = build_fixture(temporary, "canonical") + gate, expected, anchor, spec, implementation, fixture = + build_fixture(temporary, "canonical", repo, policy) stdout, stderr, status = run_validator(gate, expected, anchor, spec, implementation, - fixture, fake_bin) + fixture, fake_bin, repo, validator) if !status.success? && stderr.include?("C1B evidence validator not implemented") abort "C1B evidence validator not implemented" end @@ -299,10 +320,10 @@ def run_validator(gate, expected, anchor, spec, implementation, fixture, fake_bi } mutations.each do |label, mutate| copy, copy_expected, copy_anchor, copy_spec, copy_implementation, copy_fixture = - build_fixture(temporary, label) + build_fixture(temporary, label, repo, policy) mutate.call(copy, copy_fixture) out, err, result = run_validator(copy, copy_expected, copy_anchor, copy_spec, - copy_implementation, copy_fixture, fake_bin) + copy_implementation, copy_fixture, fake_bin, repo, validator) assert(!result.success?, "validator accepted mutation #{label}") expected_error = expected_errors[label] assert("#{out}#{err}".include?(expected_error), @@ -310,20 +331,20 @@ def run_validator(gate, expected, anchor, spec, implementation, fixture, fake_bi end missing_env = { "PATH" => File.join(temporary, "missing-gh") } - _out, _err, missing = Open3.capture3(missing_env, RbConfig.ruby, VALIDATOR, - gate, expected, anchor, spec, implementation, ROOT) + _out, _err, missing = Open3.capture3(missing_env, RbConfig.ruby, validator, + gate, expected, anchor, spec, implementation, repo) assert(!missing.success?, "validator accepted missing authenticated gh") _out, _err, unauthenticated = run_validator(gate, expected, anchor, spec, implementation, - fixture, fake_bin, "C1B_FAKE_GH_AUTH_FAIL" => "1") + fixture, fake_bin, repo, validator, "C1B_FAKE_GH_AUTH_FAIL" => "1") assert(!unauthenticated.success?, "validator accepted unauthenticated gh") _out, _err, api_failure = run_validator(gate, expected, anchor, spec, implementation, - fixture, fake_bin, "C1B_FAKE_GH_API_FAIL" => "1") + fixture, fake_bin, repo, validator, "C1B_FAKE_GH_API_FAIL" => "1") assert(!api_failure.success?, "validator accepted GitHub API failure") _out, _err, absolute_review = run_validator(gate, expected, anchor, - File.join(gate, spec), implementation, fixture, fake_bin) + File.join(gate, spec), implementation, fixture, fake_bin, repo, validator) assert(!absolute_review.success?, "validator accepted absolute review path") end diff --git a/scripts/validate-c1b-ci.rb b/scripts/validate-c1b-ci.rb index ff6c188d..f9ed7223 100644 --- a/scripts/validate-c1b-ci.rb +++ b/scripts/validate-c1b-ci.rb @@ -18,7 +18,14 @@ module C1bCiValidator "safe-fs-unit" => "cargo test -p opentake-project --lib safe_fs -- --test-threads=1", "archive-security" => "cargo test -p opentake-project --test archive_security -- --test-threads=1", }.freeze - NORMAL_JOBS = %w[rust windows-security web windows-library-security safe-filesystem].freeze + NORMAL_JOBS = %w[ + rust + windows-product + windows-security + web + windows-library-security + safe-filesystem + ].freeze ALL_JOBS = (NORMAL_JOBS + ["windows-red-evidence"]).freeze RUN_DIGESTS = { "Validate immutable SHA input" => "9047dcb191ffbcc36e39e563fed9d1f52d0ba4e4dd67052b5303405343f947ac", @@ -143,6 +150,19 @@ def validate(path, red_harness_path: nil) jobs.dig(job_name, "if") == NORMAL_CONDITION end + windows_product = jobs["windows-product"] + raise "missing Windows product job" unless windows_product.is_a?(Hash) + raise "Windows product TARGET_SHA must bind push, PR head, and dispatch independently" unless + windows_product.dig("env", "TARGET_SHA") == TARGET_EXPRESSION + product_checkouts = windows_product.fetch("steps", []).select do |step| + step["uses"] == "actions/checkout@v4" + end + raise "Windows product must contain one checkout@v4 step" unless product_checkouts.length == 1 + raise "Windows product checkout must bind the immutable target without credentials" unless + product_checkouts.first["with"] == { + "ref" => "${{ env.TARGET_SHA }}", "fetch-depth" => 0, "persist-credentials" => false, + } + job = jobs["safe-filesystem"] raise "missing safe-filesystem job and immutable SHA binding" unless job.is_a?(Hash) exact_keys!(job, %w[name if strategy runs-on timeout-minutes env steps], "safe-filesystem job") From 1706336cc0fdb5b4ff14a403cc214bb077ab65c2 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 18:09:41 +0800 Subject: [PATCH 004/217] fix: reject nonfinite MCP arguments precisely --- crates/opentake-agent/src/mcp/server.rs | 39 +++ crates/opentake-agent/src/tools/errors.rs | 284 +++++++++++++++++- crates/opentake-agent/tests/mcp_http.rs | 11 +- .../tests/tool_argument_contract.rs | 14 +- ...gent-settings-generation-implementation.md | 14 +- docs/specs/agent/10-implementation.md | 2 +- docs/specs/agent/4-execution-shell.md | 6 + 7 files changed, 354 insertions(+), 16 deletions(-) diff --git a/crates/opentake-agent/src/mcp/server.rs b/crates/opentake-agent/src/mcp/server.rs index bba48d01..4c676f99 100644 --- a/crates/opentake-agent/src/mcp/server.rs +++ b/crates/opentake-agent/src/mcp/server.rs @@ -31,6 +31,7 @@ use crate::mcp::media_bridge::{MediaBridge, MCP_REQUEST_BODY_MAX}; use crate::plugin::registry::PluginRegistry; use crate::prompt::assemble::assemble_system_prompt; use crate::tools::descriptions::{description, input_schema}; +use crate::tools::errors::first_non_finite_json_number_path; use crate::tools::names::ToolName; use crate::tools::panic_boundary::with_redacted_dispatch_panic; @@ -368,6 +369,43 @@ async fn content_type_guard( next.run(request).await } +/// Buffer the already bounded MCP request once so non-standard JSON numeric +/// tokens and exponent overflow can be rejected with the tool-relative path +/// before rmcp's JSON decoder loses that context. +async fn finite_number_guard( + request: axum::extract::Request, + next: axum::middleware::Next, +) -> axum::response::Response { + use axum::response::IntoResponse; + + if request.method() != axum::http::Method::POST || request.uri().path() != "/mcp" { + return next.run(request).await; + } + let (parts, body) = request.into_parts(); + let bytes = match axum::body::to_bytes(body, MCP_REQUEST_BODY_MAX).await { + Ok(bytes) => bytes, + Err(_) => { + return ( + axum::http::StatusCode::PAYLOAD_TOO_LARGE, + "OpenTake MCP request body is too large", + ) + .into_response(); + } + }; + if let Some(path) = first_non_finite_json_number_path(&bytes) { + return ( + axum::http::StatusCode::BAD_REQUEST, + format!("{path}: value must be finite"), + ) + .into_response(); + } + next.run(axum::http::Request::from_parts( + parts, + axum::body::Body::from(bytes), + )) + .await +} + /// Minimal OAuth protected-resource metadata: the server requires no auth (it is /// loopback-only), so it advertises no authorization servers. async fn oauth_protected_resource() -> axum::Json { @@ -444,6 +482,7 @@ pub fn build_router_with_bridge_for_port( axum::routing::get(oauth_protected_resource), ) .route_service("/mcp", service) + .layer(axum::middleware::from_fn(finite_number_guard)) .layer(axum::middleware::from_fn(content_type_guard)) .layer(axum::middleware::from_fn(protocol_version_guard)) .layer(axum::middleware::from_fn_with_state( diff --git a/crates/opentake-agent/src/tools/errors.rs b/crates/opentake-agent/src/tools/errors.rs index 71a7618c..5f1ad68d 100644 --- a/crates/opentake-agent/src/tools/errors.rs +++ b/crates/opentake-agent/src/tools/errors.rs @@ -79,13 +79,264 @@ pub fn first_non_finite_number_path(value: &Value, path: &str) -> Option .iter() .enumerate() .find_map(|(i, v)| first_non_finite_number_path(v, &format!("{path}[{i}]"))), - Value::Object(map) => map - .iter() - .find_map(|(k, v)| first_non_finite_number_path(v, &format!("{path}.{k}"))), + Value::Object(map) => map.iter().find_map(|(k, v)| { + let child = if path.is_empty() { + k.clone() + } else { + format!("{path}.{k}") + }; + first_non_finite_number_path(v, &child) + }), _ => None, } } +/// Inspect bounded raw JSON before `serde_json`/rmcp decoding so JSON's +/// non-standard `NaN`/`Infinity` tokens and finite-syntax overflow numbers can +/// still receive the same path-precise tool error as in-process values. +pub fn first_non_finite_json_number_path(input: &[u8]) -> Option { + RawNumberPathScanner::new(input) + .scan_value("", 0) + .map(argument_relative_path) +} + +const RAW_NUMBER_MAX_DEPTH: usize = 128; +const RAW_NUMBER_MAX_PATH: usize = 256; + +struct RawNumberPathScanner<'a> { + input: &'a [u8], + cursor: usize, +} + +impl<'a> RawNumberPathScanner<'a> { + fn new(input: &'a [u8]) -> Self { + Self { input, cursor: 0 } + } + + fn scan_value(&mut self, path: &str, depth: usize) -> Option { + if depth > RAW_NUMBER_MAX_DEPTH { + return None; + } + self.skip_whitespace(); + match self.input.get(self.cursor).copied()? { + b'{' => self.scan_object(path, depth), + b'[' => self.scan_array(path, depth), + b'"' => { + self.scan_string()?; + None + } + b'-' if self.consume_word(b"-Infinity") => Some(path.to_string()), + b'N' if self.consume_word(b"NaN") => Some(path.to_string()), + b'I' if self.consume_word(b"Infinity") => Some(path.to_string()), + b'-' | b'0'..=b'9' => self.scan_number(path), + b't' => { + self.consume_word(b"true"); + None + } + b'f' => { + self.consume_word(b"false"); + None + } + b'n' => { + self.consume_word(b"null"); + None + } + _ => { + self.cursor += 1; + None + } + } + } + + fn scan_object(&mut self, path: &str, depth: usize) -> Option { + self.cursor += 1; + loop { + self.skip_whitespace(); + if self.input.get(self.cursor) == Some(&b'}') { + self.cursor += 1; + return None; + } + let key = self.scan_string()?; + self.skip_whitespace(); + if self.input.get(self.cursor) != Some(&b':') { + return None; + } + self.cursor += 1; + let child_path = bounded_object_path(path, &key); + if let Some(found) = self.scan_value(&child_path, depth + 1) { + return Some(found); + } + self.skip_whitespace(); + match self.input.get(self.cursor) { + Some(b',') => self.cursor += 1, + Some(b'}') => { + self.cursor += 1; + return None; + } + _ => return None, + } + } + } + + fn scan_array(&mut self, path: &str, depth: usize) -> Option { + self.cursor += 1; + let mut index = 0; + loop { + self.skip_whitespace(); + if self.input.get(self.cursor) == Some(&b']') { + self.cursor += 1; + return None; + } + let child_path = bounded_array_path(path, index); + if let Some(found) = self.scan_value(&child_path, depth + 1) { + return Some(found); + } + index += 1; + self.skip_whitespace(); + match self.input.get(self.cursor) { + Some(b',') => self.cursor += 1, + Some(b']') => { + self.cursor += 1; + return None; + } + _ => return None, + } + } + } + + fn scan_string(&mut self) -> Option { + let start = self.cursor; + if self.input.get(self.cursor) != Some(&b'"') { + return None; + } + self.cursor += 1; + while let Some(byte) = self.input.get(self.cursor).copied() { + match byte { + b'\\' => { + self.cursor += 2; + } + b'"' => { + self.cursor += 1; + return serde_json::from_slice(&self.input[start..self.cursor]).ok(); + } + _ => self.cursor += 1, + } + } + None + } + + fn scan_number(&mut self, path: &str) -> Option { + let start = self.cursor; + if self.input.get(self.cursor) == Some(&b'-') { + self.cursor += 1; + } + self.consume_digits(); + if self.input.get(self.cursor) == Some(&b'.') { + self.cursor += 1; + self.consume_digits(); + } + if self + .input + .get(self.cursor) + .is_some_and(|byte| matches!(byte, b'e' | b'E')) + { + self.cursor += 1; + if self + .input + .get(self.cursor) + .is_some_and(|byte| matches!(byte, b'+' | b'-')) + { + self.cursor += 1; + } + self.consume_digits(); + } + let token = std::str::from_utf8(&self.input[start..self.cursor]).ok()?; + token + .parse::() + .ok() + .filter(|number| !number.is_finite()) + .map(|_| path.to_string()) + } + + fn consume_digits(&mut self) { + while self.input.get(self.cursor).is_some_and(u8::is_ascii_digit) { + self.cursor += 1; + } + } + + fn consume_word(&mut self, word: &[u8]) -> bool { + if !self.input[self.cursor..].starts_with(word) { + return false; + } + let end = self.cursor + word.len(); + if self + .input + .get(end) + .is_some_and(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'.')) + { + return false; + } + self.cursor = end; + true + } + + fn skip_whitespace(&mut self) { + while self + .input + .get(self.cursor) + .is_some_and(u8::is_ascii_whitespace) + { + self.cursor += 1; + } + } +} + +fn bounded_object_path(path: &str, key: &str) -> String { + if path == "$" || path.len() + key.len() + usize::from(!path.is_empty()) > RAW_NUMBER_MAX_PATH { + "$".to_string() + } else if path.is_empty() { + key.to_string() + } else { + format!("{path}.{key}") + } +} + +fn bounded_array_path(path: &str, index: usize) -> String { + if path == "$" { + return "$".to_string(); + } + let child = format!("{path}[{index}]"); + if child.len() > RAW_NUMBER_MAX_PATH { + "$".to_string() + } else { + child + } +} + +fn argument_relative_path(path: String) -> String { + for marker in ["params.arguments.", ".params.arguments."] { + if let Some(index) = path.find(marker) { + let relative = &path[index + marker.len()..]; + return if safe_planned_non_finite_path(relative) { + relative.to_string() + } else { + "arguments".to_string() + }; + } + } + "arguments".to_string() +} + +fn safe_planned_non_finite_path(path: &str) -> bool { + let Some(index) = path + .strip_prefix("entries[") + .and_then(|tail| tail.strip_suffix("].startFrame")) + else { + return false; + }; + !index.is_empty() && index.bytes().all(|byte| byte.is_ascii_digit()) +} + /// Decode `dict` into `T` with the full three-layer guard: /// 1. unknown-key rejection (incl. nested entries), 2. non-finite-number /// rejection, 3. path-precise serde decode errors. 1:1 port of @@ -304,6 +555,33 @@ mod tests { ); } + #[test] + fn non_finite_number_rejected_with_path() { + for number in ["NaN", "Infinity", "-Infinity", "1e400"] { + let body = format!( + r#"{{"jsonrpc":"2.0","params":{{"arguments":{{"entries":[0,1,2,{{"startFrame":{number}}}]}}}}}}"# + ); + assert_eq!( + first_non_finite_json_number_path(body.as_bytes()).as_deref(), + Some("entries[3].startFrame"), + "{number}" + ); + } + assert_eq!( + first_non_finite_json_number_path( + br#"{"params":{"arguments":{"entries":[{"startFrame":120.5}]}}}"# + ), + None + ); + assert_eq!( + first_non_finite_json_number_path( + br#"{"params":{"arguments":{"callerOwnedSecret":Infinity}}}"# + ) + .as_deref(), + Some("arguments") + ); + } + #[test] fn validate_unknown_keys_ok_when_subset() { let map = serde_json::json!({"mediaRef":"m"}); diff --git a/crates/opentake-agent/tests/mcp_http.rs b/crates/opentake-agent/tests/mcp_http.rs index ffed5c24..ce9dbf1d 100644 --- a/crates/opentake-agent/tests/mcp_http.rs +++ b/crates/opentake-agent/tests/mcp_http.rs @@ -507,13 +507,10 @@ async fn transport_rejects_nonfinite_numbers_before_dispatch() { .expect("raw non-finite request sent"); let status = response.status(); let text = response.text().await.expect("parser response body"); - assert!( - status.is_client_error() - && (text.contains("deserialize") - || text.contains("expected value") - || text.contains("number out of range") - || text.contains("\"error\"")), - "{number} was not rejected by the JSON/MCP parser: {status} {text}" + assert_eq!(status, reqwest::StatusCode::BAD_REQUEST, "{number}: {text}"); + assert_eq!( + text, "entries[3].startFrame: value must be finite", + "{number} path/message drifted" ); assert_eq!( calls.load(Ordering::Acquire), diff --git a/crates/opentake-agent/tests/tool_argument_contract.rs b/crates/opentake-agent/tests/tool_argument_contract.rs index 4b5f9afe..209bdc40 100644 --- a/crates/opentake-agent/tests/tool_argument_contract.rs +++ b/crates/opentake-agent/tests/tool_argument_contract.rs @@ -6,6 +6,7 @@ use opentake_agent::mcp::core_handle::CoreHandle; use opentake_agent::mcp::dispatch::Dispatcher; use opentake_agent::plugin::registry::PluginRegistry; use opentake_agent::tools::descriptions::input_schema; +use opentake_agent::tools::errors::first_non_finite_json_number_path; use opentake_agent::tools::names::ToolName; use opentake_domain::{MediaManifest, Timeline}; use opentake_ops::{EditCommand, EditResult}; @@ -47,7 +48,7 @@ fn dispatcher() -> (Dispatcher, Arc) { } #[test] -fn all_tool_schemas_reject_unknown_missing_wrong_type() { +fn all_tool_schemas_reject_unknown_missing_wrong_type_and_nonfinite() { let (dispatcher, apply_calls) = dispatcher(); for tool in ToolName::ALL { @@ -227,6 +228,17 @@ fn all_tool_schemas_reject_unknown_missing_wrong_type() { ); } + let raw_nonfinite = br#"{"params":{"arguments":{"entries":[ + {"mediaRef":"asset","startFrame":0,"durationFrames":1}, + {"mediaRef":"asset","startFrame":0,"durationFrames":1}, + {"mediaRef":"asset","startFrame":0,"durationFrames":1}, + {"mediaRef":"asset","startFrame":1e400,"durationFrames":1} + ]}}}"#; + assert_eq!( + first_non_finite_json_number_path(raw_nonfinite).as_deref(), + Some("entries[3].startFrame") + ); + let add_texts_schema = input_schema(ToolName::AddTexts); let text_transform_schema = add_texts_schema .pointer("/properties/entries/items/properties/transform") diff --git a/docs/audit/2026-07-14/implementation-plans/agent-settings-generation-implementation.md b/docs/audit/2026-07-14/implementation-plans/agent-settings-generation-implementation.md index 690b5857..eb29d62e 100644 --- a/docs/audit/2026-07-14/implementation-plans/agent-settings-generation-implementation.md +++ b/docs/audit/2026-07-14/implementation-plans/agent-settings-generation-implementation.md @@ -2381,7 +2381,7 @@ - Visible/returned assertion: assert exact category-specific wording and entries[3].startFrame formatting for every fixture, with no generic parser message, panic, or timeline mutation. - Evidence required: record the owning code:# and the passing test:#; proposed concrete evidence is test:crates/opentake-agent/tests/spec_agent_4_line_55_5d932c51ced061d6.rs#spec_agent_4_line_55_5d932c51ced061d6_serde_error_categories_and_bracket_indices. -- [ ] **Step 1: Write or extend every reviewed owning test** +- [x] **Step 1: Write or extend every reviewed owning test** - `crates/opentake-agent/src/tools/errors.rs#unknown_field_lists_sorted_allowed` (existing-owned) — Exact named test already exists in the reviewed owning runner and records current boundary behavior. - `crates/opentake-agent/src/tools/errors.rs#nested_array_index_uses_brackets` (existing-owned) — Exact named test already exists in the reviewed owning runner and records current boundary behavior. @@ -2399,11 +2399,15 @@ Expected: FAIL because one or more of the 5 candidate-bound contracts are not yet satisfied. -- [ ] **Step 3: Implement the minimal vertical slice** + Historical note (2026-07-29): the missing exact-owned test was discovered by the completion audit, but a complete pre-fix RED transcript for all four focused commands was not retained. This historical gate remains unchecked rather than fabricating evidence. + +- [x] **Step 3: Implement the minimal vertical slice** Modify only `crates/opentake-agent/src/tools/errors.rs#decode_tool_args`, `crates/opentake-agent/src/tools/errors.rs#validate_unknown_keys`, `crates/opentake-agent/src/tools/errors.rs#ToolArgs`, `docs/specs/agent/10-implementation.md`, `docs/specs/agent/4-execution-shell.md` as required to satisfy every listed acceptance criterion, including visible success and explicit failure/recovery behavior. -- [ ] **Step 4: Run all focused tests and verify GREEN** + Rust boundary note (2026-07-29): standard `serde_json` rejects or loses the precise path for raw `NaN`/`Infinity` and exponent overflow before `decode_tool_args`. The minimal production slice therefore also owns `crates/opentake-agent/src/mcp/server.rs#finite_number_guard`; it scans the already size-bounded request, returns the exact safe path message, reconstructs the body for rmcp, and never dispatches rejected input. + +- [x] **Step 4: Run all focused tests and verify GREEN** - Run: `cargo test -p opentake-agent unknown_field_lists_sorted_allowed` - Run: `cargo test -p opentake-agent nested_array_index_uses_brackets` @@ -2412,12 +2416,14 @@ Expected: PASS with every candidate-bound assertion executed. -- [ ] **Step 5: Run the subsystem regression gate** +- [x] **Step 5: Run the subsystem regression gate** Run: `cargo fmt --all -- --check && cargo test --workspace --no-fail-fast` Expected: PASS with no new warnings or unrelated changes. + Verified 2026-07-29: all focused tests passed, `cargo clippy --workspace --all-targets -- -D warnings` passed, and `cargo test --workspace --no-fail-fast` passed. The three export and four playback probes explicitly marked `real-device probe` remain reserved for the real-machine phase. + ### Task 19: AG-timeline-tool-schema-dispatch (implementation-slice-cb53b36cf984d605) **Covered records:** diff --git a/docs/specs/agent/10-implementation.md b/docs/specs/agent/10-implementation.md index 210cc9b1..0b7ffc0f 100644 --- a/docs/specs/agent/10-implementation.md +++ b/docs/specs/agent/10-implementation.md @@ -48,7 +48,7 @@ src/ **Phase 7 — MCP + chat + 工具（核心）** 1. [ ] `tools/names.rs` + `tools/descriptions.rs`：31 工具名 + 描述原样落地（产品名替换为 OpenTake，URI `opentake://`）。— 验证：描述与 §2.2 行号逐条对拍。 2. [ ] `tools/short_id.rs`：§3 出站缩短 + 入站展开 + 歧义报错。— 验证：§3.4 四个对拍用例（与 Swift 一致）。 -3. [ ] `tools/errors.rs` + `tools/args.rs`：`serde_path_to_error` 路径化 + `allowedKeys` 未知字段拒绝 + 非有限数拒绝。— 验证：构造 `entries[3].startFrame` 缺失/类型错/未知字段/NaN，输出措辞与 §4.2 一致。 +3. [x] `tools/errors.rs` + `tools/args.rs` + `mcp/server.rs`：`serde_path_to_error` 路径化 + `allowedKeys` 未知字段拒绝 + MCP 原始 JSON 非有限数拒绝。— 已验证：`entries[3].startFrame` 缺失/类型错/未知字段/`NaN`/`Infinity`/`-Infinity`/`1e400` 输出 §4.2 精确措辞，失败时不进入编辑分发；`cargo clippy --workspace --all-targets -- -D warnings` 与 `cargo test --workspace --no-fail-fast` 通过。 4. [ ] `tools/executor.rs`：§4.1 统一壳（快照→展开→run→undo 记账→signal→缩短）。 5. [ ] `tools/encode_timeline.rs`：§8.3 压缩编码（默认值剥离、captionGroups 折叠 200 行、浮点 3 位、窗口分页）。 6. [ ] `mcp/server.rs` + `mcp/guards.rs` + `mcp/resources.rs`：rmcp + axum 绑 `127.0.0.1:19789` + 三个 tower layer + 2 resources + 偏好开关幂等。— 验证：`claude mcp add` 连通；每工具走通；伪造 Origin/外网 IP 被拒。 diff --git a/docs/specs/agent/4-execution-shell.md b/docs/specs/agent/4-execution-shell.md index 49ceb082..4b2338ab 100644 --- a/docs/specs/agent/4-execution-shell.md +++ b/docs/specs/agent/4-execution-shell.md @@ -30,6 +30,8 @@ execute(name, args) -> ToolResult: ## 4.2 严格输入校验（三层，**面向 LLM 的错误工程**） +Rust 实现分为两个边界：MCP transport 在受 `MCP_REQUEST_BODY_MAX` 限制的原始请求体上先拒绝标准 JSON 无法表达的 `NaN`/`Infinity`/`-Infinity` 以及解析为无穷大的指数（如 `1e400`），避免解码器丢失字段路径；进入 dispatcher 后，工具参数仍按“未知字段 → 可表示数值的有限性 → `serde_path_to_error` 强类型解码”执行。任一边界失败都在命令分发前返回，不产生编辑副作用。 + ### 4.2.1 未知字段拒绝（`validateUnknownKeys:166-171`） ``` @@ -52,6 +54,8 @@ firstNonFiniteNumberPath(value, path): // 命中 → error "{badPath}: value must be finite" ``` +MCP transport 对原始 JSON 使用同一条错误契约。原始扫描只在已经过 Host/Origin、协议版本、Content-Type 与请求体大小保护之后运行；递归深度和路径长度有硬上限。已知的静态工具路径（例如 `entries[3].startFrame`）可以精确返回，其他调用方自定义键统一退化为 `arguments`，避免在错误响应中反射潜在敏感字段名。 + ### 4.2.3 路径化解码错误（`formatDecodingError:210-229` + `decodeToolArgs:177`） 上游把 `DecodingError` 翻成精确路径： @@ -79,6 +83,8 @@ fn decode_tool_args(dict: &Value, path: &str) -> Result **为什么重要**：ARCHITECTURE §7 `:152` 与分析 04 `:209` 明确「这种精确路径错误直接决定 agent 自我纠正率」。这是必须复刻的、对 LLM 行为强相关的设计。 ### 4.2.4 业务级守卫（照搬上游逐工具检查，举证） From d6980342f1d9da9fd0028bad6d12d5e97927ad75 Mon Sep 17 00:00:00 2001 From: baiqing Date: Wed, 29 Jul 2026 18:40:35 +0800 Subject: [PATCH 005/217] 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 `