diff --git a/.mcp.json b/.mcp.json --- a/.mcp.json +++ b/.mcp.json @@ -2,7 +2,7 @@ "mcpServers": { "filmcraft": { "command": "target/release/filmcraft-cli", - "args": ["mcp", "--bridge", "127.0.0.1:9876"] + "args": ["mcp", "--bridge", "127.0.0.1:9886"] }, "filmcraft-headless": { "command": "target/release/filmcraft-cli", diff --git a/CLAUDE.md b/CLAUDE.md --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,7 @@ FilmCraft is a clean-room, open-source, pure-Rust non-linear video editor target - **Commits:** one task id per commit (`M2.4: CABAC residual decoding`). Only green states. End messages with the attribution line required by the environment. ## Running and looking at the app -- `cargo run -p filmcraft -- --control 9876` opens the desktop app with the JSON-lines control server (see `docs/control-protocol.md`). +- `cargo run -p filmcraft -- --control 9886` opens the desktop app with the JSON-lines control server (see `docs/control-protocol.md`). - For UI work, **look at the result**: drive via the control channel and take `ui.screenshot`, compare with `plan/premiere/screenshots/` (maintainer-local; see [`docs/agents.md`](docs/agents.md) §3). - Parallel agents: separate git worktrees and `CARGO_TARGET_DIR=target/agent-`; keep every `Cargo.toml` valid at all times (the `crates/*` glob means one broken manifest breaks everyone). - Test fixtures: generate with ffmpeg into `target/fixtures/` (never commit media). See [`docs/testing.md`](docs/testing.md). diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -257,7 +257,7 @@ The trailer and the grades in these screenshots were built exactly this way, by ```sh cargo run --release -p filmcraft # the desktop app, with a demo project -cargo run --release -p filmcraft -- --control 9876 # plus the JSON-lines control server +cargo run --release -p filmcraft -- --control 9886 # plus the JSON-lines control server cargo run --release -p filmcraft-cli -- commands # list every engine command cargo run --release -p filmcraft-cli -- mcp # MCP server (headless) ``` diff --git a/apps/filmcraft-cli/src/args.rs b/apps/filmcraft-cli/src/args.rs --- a/apps/filmcraft-cli/src/args.rs +++ b/apps/filmcraft-cli/src/args.rs @@ -134,8 +134,8 @@ mod tests { assert_eq!(a.opt("--project"), Some("p.fcproj")); assert!(a.flag("--save") && a.flag("--demo") && a.flag("--compact")); assert!(!a.flag("--bridge")); - let b = args("--bridge=127.0.0.1:9876 inspect sequence"); - assert_eq!(b.opt("--bridge"), Some("127.0.0.1:9876")); + let b = args("--bridge=127.0.0.1:9886 inspect sequence"); + assert_eq!(b.opt("--bridge"), Some("127.0.0.1:9886")); assert_eq!(b.pos(1), Some("sequence")); } diff --git a/apps/filmcraft-cli/tests/it/cli.rs b/apps/filmcraft-cli/tests/it/cli.rs --- a/apps/filmcraft-cli/tests/it/cli.rs +++ b/apps/filmcraft-cli/tests/it/cli.rs @@ -23,8 +23,8 @@ fn help_and_usage_errors() { assert_eq!(cli(&["frobnicate"]).status.code(), Some(2)); assert_eq!(cli(&["exec", "file.newBin", "notkv"]).status.code(), Some(2)); // Invalid --bridge is a usage error (exit 2) for both exec and mcp (#921). - assert_eq!(cli(&["--bridge", "0.0.0.0:9876", "exec", "inspect.project"]).status.code(), Some(2)); - assert_eq!(cli(&["mcp", "--bridge", "0.0.0.0:9876"]).status.code(), Some(2)); + assert_eq!(cli(&["--bridge", "0.0.0.0:9886", "exec", "inspect.project"]).status.code(), Some(2)); + assert_eq!(cli(&["mcp", "--bridge", "0.0.0.0:9886"]).status.code(), Some(2)); } #[test] diff --git a/apps/filmcraft/src/args.rs b/apps/filmcraft/src/args.rs --- a/apps/filmcraft/src/args.rs +++ b/apps/filmcraft/src/args.rs @@ -204,7 +204,7 @@ mod tests { /// A bad `FILMCRAFT_CONTROL_PORT` used to start the app without the control server, silently. #[test] fn a_bad_port_in_the_environment_is_an_error_like_a_bad_control_flag() { - for bad in ["abc", "70000", "-1", "98 76", "9876x", "\u{fffd}"] { + for bad in ["abc", "70000", "-1", "98 76", "9886x", "\u{fffd}"] { let e = parse_env(Some(bad), &[]).unwrap_err(); assert!(e.contains("FILMCRAFT_CONTROL_PORT") && e.contains(bad), "{bad}: {e}"); assert!(parse_env(Some(bad), &["--empty", "a.mp4"]).is_err(), "{bad}"); @@ -238,8 +238,8 @@ mod tests { graphics: None } ); - let l = launch(&["--control", "9876", "--empty", "--no-recover", "--data-dir", "/tmp/fc", "cut.fcproj", "a.mp4"]); - assert_eq!(l.control_port, Some(9876)); + let l = launch(&["--control", "9886", "--empty", "--no-recover", "--data-dir", "/tmp/fc", "cut.fcproj", "a.mp4"]); + assert_eq!(l.control_port, Some(9886)); assert_eq!((l.demo, l.startup_flag, l.recover), (false, true, Some(false))); assert_eq!(l.data_dir, Some(std::path::PathBuf::from("/tmp/fc"))); assert_eq!(l.files, ["cut.fcproj", "a.mp4"]); @@ -297,10 +297,10 @@ mod tests { #[test] fn isolated_input_is_explicit_and_requires_a_control_server() { - assert!(!launch(&["--control", "9876"]).control_input_only); + assert!(!launch(&["--control", "9886"]).control_input_only); assert!(parse_args(&["--control-input-only"]).is_err()); - assert!(launch(&["--control-input-only", "--control", "9876"]).control_input_only); - assert!(matches!(parse_env(Some("9876"), &["--control-input-only"]), Ok(Cli::Run(Launch { control_input_only: true, control_port: Some(9876), .. })))); + assert!(launch(&["--control-input-only", "--control", "9886"]).control_input_only); + assert!(matches!(parse_env(Some("9886"), &["--control-input-only"]), Ok(Cli::Run(Launch { control_input_only: true, control_port: Some(9886), .. })))); assert_eq!(parse_args(&["--control-input-only", "--help"]), Ok(Cli::Help)); assert!(USAGE.contains("--control-input-only")); } diff --git a/apps/filmcraft/src/control_server.rs b/apps/filmcraft/src/control_server.rs --- a/apps/filmcraft/src/control_server.rs +++ b/apps/filmcraft/src/control_server.rs @@ -27,28 +27,41 @@ pub const MAX_CONNECTIONS: usize = 16; /// A peer that stops sending or reading must eventually give back its connection slot. const CONNECTION_TIMEOUT: Duration = Duration::from_secs(120); -pub fn start(port: u16, ctx: egui::Context) -> Receiver { +/// The request channel plus the startup failure, when the server is not listening. The channel +/// is returned either way so the app can wire it once; on a failure it never yields a request. +/// The reason comes back so the UI can show it (the status bar, `ui.inspect` → `control`): +/// stderr reaches nobody when the app is started from a desktop menu or by an agent (#3313). +pub fn start(port: u16, ctx: egui::Context) -> (Receiver, Option) { start_with_token(port, ctx, std::env::var("FILMCRAFT_CONTROL_TOKEN").ok()) } -fn start_with_token(port: u16, ctx: egui::Context, token: Option) -> Receiver { +fn start_with_token(port: u16, ctx: egui::Context, token: Option) -> (Receiver, Option) { let (tx, rx) = channel::(); // An inherited environment variable shares the credential with the owner's bridge process // without exposing it in command-line arguments, logs, or a shared discovery file. let Some(token) = token.filter(|t| t.len() == 64 && t.bytes().all(|b| b.is_ascii_hexdigit())) else { - eprintln!("filmcraft: control server disabled: FILMCRAFT_CONTROL_TOKEN must contain 64 random hex characters (see docs/control-protocol.md)"); - return rx; + return fail(rx, "control server disabled: FILMCRAFT_CONTROL_TOKEN must contain 64 random hex characters (see docs/control-protocol.md)".into()); }; let listener = match TcpListener::bind(("127.0.0.1", port)) { Ok(l) => l, + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { + return fail( + rx, + format!("control server can't listen on 127.0.0.1:{port}: the port is already in use (another program is bound to it; pick a free port)"), + ); + } Err(e) => { - eprintln!("filmcraft: control server failed to bind 127.0.0.1:{port}: {e}"); - return rx; + return fail(rx, format!("control server failed to bind 127.0.0.1:{port}: {e}")); } }; - eprintln!("filmcraft: control server listening on 127.0.0.1:{port}"); + log::info!("control server listening on 127.0.0.1:{port}"); std::thread::spawn(move || accept_loop(listener, tx, ctx, token)); - rx + (rx, None) +} + +fn fail(rx: Receiver, reason: String) -> (Receiver, Option) { + log::error!("{reason}"); + (rx, Some(reason)) } fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Context, token: String) { @@ -70,7 +83,7 @@ fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Con serve(stream, &tx, &ctx, &token); }); if let Err(e) = spawned { - eprintln!("filmcraft: control server could not start a connection thread: {e}"); + log::error!("control server could not start a connection thread: {e}"); } } } @@ -86,7 +99,7 @@ impl Drop for Slot { fn serve(stream: TcpStream, tx: &Sender, ctx: &egui::Context, token: &str) { if let Err(e) = serve_with_timeout(stream, tx, ctx, token, CONNECTION_TIMEOUT) { - eprintln!("filmcraft: could not configure control connection: {e}"); + log::error!("could not configure control connection: {e}"); } } diff --git a/apps/filmcraft/src/control_server/tests.rs b/apps/filmcraft/src/control_server/tests.rs --- a/apps/filmcraft/src/control_server/tests.rs +++ b/apps/filmcraft/src/control_server/tests.rs @@ -30,7 +30,7 @@ fn requests_are_answered_in_order_with_ids() { fn http_request_closes_before_the_body_runs() { // A browser's cross-origin `fetch` POST (issue #119): the request line is rejected and the // connection closed, so the JSON body is never executed. - let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1:9876\r\nOrigin: https://attacker.example\r\nContent-Type: text/plain\r\n\r\n\ + let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1:9886\r\nOrigin: https://attacker.example\r\nContent-Type: text/plain\r\n\r\n\ {\"id\":7,\"method\":\"engine.execute\",\"params\":{\"command\":\"file.openDemoProject\",\"params\":{}}}\n"; let (r, calls) = run(http); assert!(calls.is_empty(), "nothing may run: {calls:?}"); @@ -235,12 +235,29 @@ fn missing_or_invalid_token_leaves_the_listener_closed() { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); let port = listener.local_addr().unwrap().port(); drop(listener); - let rx = start_with_token(port, egui::Context::default(), token); + let (rx, error) = start_with_token(port, egui::Context::default(), token); + assert!(error.as_deref().is_some_and(|e| e.contains("FILMCRAFT_CONTROL_TOKEN")), "{error:?}"); assert!(matches!(rx.try_recv(), Err(std::sync::mpsc::TryRecvError::Disconnected))); let _still_available = TcpListener::bind(("127.0.0.1", port)).unwrap(); } } +/// A port that is already bound used to be reported only on stderr: the app then ran without +/// the control server a script was waiting for (#3313). +#[test] +fn a_port_in_use_is_reported_as_a_startup_error() { + let held = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let port = held.local_addr().unwrap().port(); + let (rx, error) = start_with_token(port, egui::Context::default(), Some(TOKEN.into())); + let error = error.expect("a bound port must be reported"); + assert!(error.contains(&port.to_string()) && error.contains("in use"), "{error}"); + assert!(matches!(rx.try_recv(), Err(std::sync::mpsc::TryRecvError::Disconnected))); + drop(held); + let (rx, error) = start_with_token(port, egui::Context::default(), Some(TOKEN.into())); + assert_eq!(error, None); + drop(rx); +} + #[test] fn unauthenticated_save_as_closes_before_any_command_runs() { for token in [Value::Null, json!(false), json!(42), json!([]), json!({}), json!(""), json!("a".repeat(64)), json!(&TOKEN[..63]), json!(format!("{TOKEN}0"))] diff --git a/apps/filmcraft/src/main.rs b/apps/filmcraft/src/main.rs --- a/apps/filmcraft/src/main.rs +++ b/apps/filmcraft/src/main.rs @@ -347,7 +347,13 @@ fn app_creator(launch: Launch, graphics: graphics::GraphicsStart, created: Arc) -> Result { let addr = addr.into(); @@ -90,13 +90,44 @@ impl BridgeClient { let id = self.next_id.fetch_add(1, Ordering::Relaxed); let line = format!("{}\n", json!({"id": id, "token": self.token, "method": method, "params": params})); let res: Result = async { - conn.1.write_all(line.as_bytes()).await.map_err(|e| AutomationError::Bridge(e.to_string()))?; + let gone = |e: &std::io::Error| { + matches!(e.kind(), std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::NotConnected) + }; + conn.1.write_all(line.as_bytes()).await.map_err(|e| { + if gone(&e) { + AutomationError::Bridge(format!("{} closed the connection without a reply", self.addr)) + } else { + AutomationError::Bridge(e.to_string()) + } + })?; let mut buf = String::new(); - tokio::time::timeout(Duration::from_secs(90), conn.0.read_line(&mut buf)) + let n = tokio::time::timeout(Duration::from_secs(90), conn.0.read_line(&mut buf)) .await .map_err(|_| AutomationError::Bridge("timeout".into()))? - .map_err(|e| AutomationError::Bridge(e.to_string()))?; - serde_json::from_str(&buf).map_err(|e| AutomationError::Bridge(format!("bad reply: {e}"))) + .map_err(|e| { + if gone(&e) { + AutomationError::Bridge(format!("{} closed the connection without a reply", self.addr)) + } else { + AutomationError::Bridge(e.to_string()) + } + })?; + if n == 0 { + return Err(AutomationError::Bridge(format!("{} closed the connection without a reply", self.addr))); + } + let v: Value = serde_json::from_str(&buf).map_err(|_| AutomationError::Bridge(Self::wrong_server(&self.addr)))?; + // A reply the app never sends (another program on the port, e.g. a different + // tool's server) used to surface as a bare parse or `ok` error (#3313). The app + // echoes the request's `id` on success; its own `ok: false` rejects (bad token, + // junk line) carry none. + let known = match v.get("ok") { + Some(Value::Bool(true)) => v.get("id") == Some(&json!(id)), + Some(Value::Bool(false)) => true, + _ => false, + }; + if !known { + return Err(AutomationError::Bridge(Self::wrong_server(&self.addr))); + } + Ok(v) } .await; match res { @@ -119,6 +150,13 @@ impl BridgeClient { Err(AutomationError::Bridge("unreachable".into())) } + /// The peer answered but not as the control server does (`{"ok": …}` per line): another + /// program owns the port. The classic case is a different tool's server, which used to + /// surface as a bare JSON parse error (#3313). + fn wrong_server(addr: &str) -> String { + format!("{addr} is not a FilmCraft control server: something else is listening on this port") + } + async fn connect(&self) -> Result { let s = tokio::time::timeout(Duration::from_secs(5), TcpStream::connect(&self.addr)) .await @@ -274,6 +312,79 @@ mod tests { assert_eq!(b.execute("fast", json!({})).await.unwrap(), json!("fast")); } + /// A stand-in for another program's server (the Blender MCP add-on that also defaults to a + /// nearby port, #3313): it answers every line, just never as the control server would. + async fn foreign_app(reply: &str) -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + let reply = reply.to_string(); + tokio::spawn(async move { + while let Ok((s, _)) = listener.accept().await { + let reply = reply.clone(); + tokio::spawn(async move { + let (r, mut w) = s.into_split(); + let mut lines = BufReader::new(r).lines(); + while let Ok(Some(_)) = lines.next_line().await { + let _ = w.write_all(format!("{reply}\n").as_bytes()).await; + } + }); + } + }); + addr + } + + /// A non-FilmCraft reply used to surface as `bad reply: …` (a JSON parse error) or a bare + /// `app: error`; the port's real owner is named instead (#3313). + #[tokio::test] + async fn a_reply_from_another_program_is_named() { + for reply in [ + "{\"status\": \"error\", \"message\": \"unknown command\"}", + "{\"jsonrpc\": \"2.0\", \"id\": 1, \"result\": {}}", + "not json at all", + "{\"ok\": true}", + "{\"ok\": true, \"id\": 9999}", + ] { + let addr = foreign_app(reply).await; + let b = BridgeClient::new(addr.clone()).unwrap(); + let e = b.call("ui.inspect", json!({})).await.unwrap_err(); + let e = e.to_string(); + assert!(e.contains("not a FilmCraft control server") && e.contains(&addr), "{reply}: {e}"); + } + } + + /// The app's own unsolicited rejects (`{"ok": false, "error": …}`) carry no `id` (bad token, + /// junk line): they must reach the caller as the app's error, not as a foreign server. + #[tokio::test] + async fn an_error_reply_without_an_id_is_the_apps_own_error() { + let addr = foreign_app("{\"ok\": false, \"error\": \"control authentication failed; closing the connection\"}").await; + let b = BridgeClient::new(addr).unwrap(); + let e = b.call("ui.inspect", json!({})).await.unwrap_err(); + assert_eq!(e.to_string(), "app: control authentication failed; closing the connection"); + } + + /// A peer that hangs up silently is named too, not "EOF while parsing a value". Closing + /// after reading the request sends a clean FIN; closing without reading sends a reset. + #[tokio::test] + async fn a_connection_closed_without_reply_is_named() { + for read_first in [true, false] { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + tokio::spawn(async move { + while let Ok((s, _)) = listener.accept().await { + tokio::spawn(async move { + if read_first { + let (r, _w) = s.into_split(); + let _ = BufReader::new(r).lines().next_line().await; + } + }); + } + }); + let b = BridgeClient::new(addr).unwrap(); + let e = b.call("ui.inspect", json!({})).await.unwrap_err(); + assert!(e.to_string().contains("closed the connection without a reply"), "read_first={read_first}: {e}"); + } + } + /// Everything else is forwarded unchanged. #[tokio::test] async fn other_commands_are_forwarded() { diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -64,6 +64,7 @@ impl FilmcraftApp { textures: HashMap::new(), thumb_used: HashMap::new(), control_rx: None, + control_status: None, pending_panel_raise: None, control_input_only: false, native_window_focused: None, diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -65,6 +65,16 @@ impl ControlRequest { } } +/// The control listener the app was asked to start (`--control`, `FILMCRAFT_CONTROL_PORT`): +/// `error` is why it is not listening (a bad `FILMCRAFT_CONTROL_TOKEN`, or a port another +/// program already bound). A bind failure is otherwise invisible to an agent (it sees only a +/// port that never answers), so `ui.inspect` reports it under `control` (#3313). +#[derive(Clone, Debug)] +pub struct ControlStatus { + pub port: u16, + pub error: Option, +} + pub enum Outcome { Done(Value), /// Try again on a later frame (e.g. the element is not on screen yet). @@ -595,6 +605,7 @@ pub fn inspect(app: &FilmcraftApp, ctx: &egui::Context) -> Value { "keyboardFocus": keyboard_focus, "input": {"mode": if app.control_input_only { "controlOnly" } else { "interactive" }, "nativeWindowFocused": app.native_window_focused, "effectiveWindowFocused": ctx.input(|input| input.focused)}, + "control": app.control_status.as_ref().map(|c| json!({"port": c.port, "listening": c.error.is_none(), "error": c.error})), "ui": app.ui, "sourcePlayback": {"playing": app.source_playback.clock.playing, "audioClock": app.source_playback.clock.audio_clock, "playhead": app.session.state.source_playhead.0}, "playback": {"playing": app.playback.playing, "speed": app.playback.speed, "loop": app.playback.looping, "audioClock": app.playback.audio_clock, "dropped": app.playback.meter.counts().1, "shown": app.playback.meter.counts().0, "preroll": app.playback.preroll.is_some()}, @@ -760,6 +771,18 @@ mod input_bounds_tests { .expect("hostile control parameters must not panic") } + /// `ui.inspect` reports the control listener's state, so an agent sees a bind failure the + /// status bar showed only in passing (#3313). + #[test] + fn inspect_reports_the_control_listener() { + let mut a = app(); + assert_eq!(inspect(&a, &egui::Context::default())["control"], Value::Null); + a.control_status = Some(ControlStatus { port: 9886, error: None }); + assert_eq!(inspect(&a, &egui::Context::default())["control"], json!({"port": 9886, "listening": true, "error": null})); + a.control_status = Some(ControlStatus { port: 9886, error: Some("port in use".into()) }); + assert_eq!(inspect(&a, &egui::Context::default())["control"], json!({"port": 9886, "listening": false, "error": "port in use"})); + } + /// Require an immediate error naming `field`, with all previously queued input unchanged. fn assert_rejected(app: &mut FilmcraftApp, method: &str, params: Value, field: &str) { let before = app.synthetic.clone(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -76,7 +76,7 @@ use filmcraft_engine::{Services, Session}; use filmcraft_time::Tick; use serde_json::{Value, json}; -pub use control::ControlRequest; +pub use control::{ControlRequest, ControlStatus}; use dock::{LayoutExt as _, PanelKind}; use frames::{FrameKey, FrameServer, Target}; use native_dialogs::FileDialog; @@ -212,6 +212,9 @@ pub struct FilmcraftApp { /// [`MAX_THUMB_TEXTURES`] are freed, least recently used first (#1682). thumb_used: HashMap, control_rx: Option>, + /// How the `--control` listener came up (set by the host): `ui.inspect` reports it under + /// `control`, so a port that failed to bind is visible to agents, not just stderr (#3313). + pub control_status: Option, /// One explicit panel reveal; consumed by the custom renderer, never repeated per frame. pub(crate) pending_panel_raise: Option, /// Explicit automation mode: native widget input and native menu commands are excluded. diff --git a/docs/agents.md b/docs/agents.md --- a/docs/agents.md +++ b/docs/agents.md @@ -12,7 +12,7 @@ FilmCraft from an agent. Part 2 covers developing FilmCraft as an agent. | Mode | Command | What it drives | |---|---|---| | Headless | `filmcraft-cli mcp --demo` or `--project p.fcproj` (neither = empty project) | an in-process engine session, no window | -| Bridge | `filmcraft-cli mcp --bridge 127.0.0.1:9876` | the running desktop app started with `filmcraft --control 9876` | +| Bridge | `filmcraft-cli mcp --bridge 127.0.0.1:9886` | the running desktop app started with `filmcraft --control 9886` | Headless mode also takes `--data-dir DIR` (user presets). Any other option is rejected (exit 2): there are no folder-confinement flags such as `--automation-read-root`, so the server can read @@ -24,7 +24,7 @@ and write wherever the user can. ```sh cargo build --release -p filmcraft-cli -p filmcraft export FILMCRAFT_CONTROL_TOKEN="$(python3 -c 'import secrets; print(secrets.token_hex(32))')" -./target/release/filmcraft --control 9876 & # only for the bridge server +./target/release/filmcraft --control 9886 & # only for the bridge server claude # approve the project MCP servers when asked ``` @@ -35,7 +35,7 @@ claude mcp add filmcraft-headless -- /abs/path/filmcraft/target/release/filmcraf ``` ```json -{ "mcpServers": { "filmcraft": { "command": "/abs/path/filmcraft-cli", "args": ["mcp", "--bridge", "127.0.0.1:9876"] } } } +{ "mcpServers": { "filmcraft": { "command": "/abs/path/filmcraft-cli", "args": ["mcp", "--bridge", "127.0.0.1:9886"] } } } ``` ### Effect parameter replies @@ -144,10 +144,12 @@ progress and stop it: ## 2. Control channel -`filmcraft --control 9876` (or `FILMCRAFT_CONTROL_PORT=9876`) listens on `127.0.0.1` only. Send one +`filmcraft --control 9886` (or `FILMCRAFT_CONTROL_PORT=9886`) listens on `127.0.0.1` only. Send one JSON request per line and get one JSON reply per line. Set a fresh `FILMCRAFT_CONTROL_TOKEN` for the desktop session as above; the CLI / MCP bridge must inherit the same variable. Raw requests must -include it as `token` on every line (replace the placeholders below). Full setup and method table: +include it as `token` on every line (replace the placeholders below). If the listener could not +start (port already bound, missing token), `ui.inspect` reports it under `control`; a bridge that +reaches another program's server names that instead of a parse error. Full setup and method table: [control-protocol.md](control-protocol.md). ```jsonc @@ -167,7 +169,7 @@ Minimal client: ```python import json, os, socket -s = socket.create_connection(("127.0.0.1", 9876)); f = s.makefile("rw") +s = socket.create_connection(("127.0.0.1", 9886)); f = s.makefile("rw") def call(method, **params): f.write(json.dumps({"id": 1, "token": os.environ["FILMCRAFT_CONTROL_TOKEN"], "method": method, "params": params}) + "\n"); f.flush() return json.loads(f.readline()) @@ -226,7 +228,7 @@ filmcraft-cli --project p.fcproj export out --preset "YouTube 1080p Full HD" --s filmcraft-cli export --list-presets prores # built-in + user presets (--data-dir for another library) filmcraft-cli --project p.fcproj exec export.queue.add preset="Apple ProRes 422 HQ" path=renders/ start=true wait=true echo '{"id":"file.newBin","params":{"name":"Selects"}}' | filmcraft-cli --project p.fcproj --save run - -filmcraft-cli --bridge 127.0.0.1:9876 exec window.workspace.color # the running app +filmcraft-cli --bridge 127.0.0.1:9886 exec window.workspace.color # the running app ``` `key=value` values are parsed as JSON when they can be (`3.5`, `true`, `[1,2]`), otherwise taken as @@ -263,7 +265,7 @@ failed write reports an error. The menu opens the normal save dialog; cancelling For any UI change, look at the result: -1. `cargo run --release -p filmcraft -- --control 9876` +1. `cargo run --release -p filmcraft -- --control 9886` 2. Drive the feature through the control channel or the MCP bridge: commands, then clicks and drags by automation id. 3. Assert on `ui.inspect`, `ui.elements` and `sequence.inspect`. diff --git a/docs/contributing.md b/docs/contributing.md --- a/docs/contributing.md +++ b/docs/contributing.md @@ -28,7 +28,7 @@ macOS. ```sh cargo run --release -p filmcraft # desktop app with the demo project cargo run --release -p filmcraft -- --empty # start without the demo project -cargo run --release -p filmcraft -- --control 9876 # plus the JSON-lines control server +cargo run --release -p filmcraft -- --control 9886 # plus the JSON-lines control server cargo run --release -p filmcraft -- a.mp4 b.wav # import media (or open a .fcproj) cargo run --release -p filmcraft-cli -- commands # list engine commands cargo run --release -p filmcraft-cli -- render --demo --seconds 3 --out frame.png diff --git a/docs/control-protocol.md b/docs/control-protocol.md --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -1,15 +1,15 @@ # Control protocol & MCP ## Desktop control channel -`filmcraft --control 9876` (or `FILMCRAFT_CONTROL_PORT`) listens on `127.0.0.1:` (loopback only). +`filmcraft --control 9886` (or `FILMCRAFT_CONTROL_PORT`) listens on `127.0.0.1:` (loopback only). The listener requires `FILMCRAFT_CONTROL_TOKEN`: 64 random hex characters shared with authorized clients through their inherited environment. Generate a fresh token for each desktop session, then start the desktop and bridge from that environment: ```sh export FILMCRAFT_CONTROL_TOKEN="$(python3 -c 'import secrets; print(secrets.token_hex(32))')" -filmcraft --control 9876 & -filmcraft-cli mcp --bridge 127.0.0.1:9876 +filmcraft --control 9886 & +filmcraft-cli mcp --bridge 127.0.0.1:9886 ``` The CLI / MCP bridge reads this variable automatically. Keep it private: don't put the value in @@ -18,6 +18,14 @@ the desktop leaves the listener closed. Every raw JSON request must include the `token` string; missing or incorrect credentials get an error and close the connection before any request is dispatched. Loopback alone does not restrict clients to the desktop owner's OS account. +Port 9886 is the documented example; the other Craft apps use their own (PhotoCraft 7878, +GridCraft 7979, WordCraft 7981, EffectCraft 9877). Pick a port nothing else listens on: Blender's +MCP add-ons default to 9876, a frequent neighbour of a video editor. If the port can't be bound +(already in use) or the token is missing, the app still starts but says so on stderr, in the log +file and in the status bar, and `ui.inspect` reports it under `control` (`{port, listening, +error}`). A bridge that reaches a server which is not FilmCraft reports +"not a FilmCraft control server: something else is listening on this port". + Socket reads and writes time out after two minutes without progress, releasing the connection slot. Expiration closes the connection without an unsolicited reply; reconnect before the next request (the CLI / MCP bridge does this automatically). Waiting for a UI reply has its separate 60-second @@ -48,7 +56,7 @@ Methods (handlers in `crates/ui-egui/src/control.rs`): |---|---|---| | `engine.execute` / `ui.menu.invoke` | `{command, params}` | any engine or UI command id | | `engine.commands` / `ui.menu.list` | – | command registry / menu tree | -| `ui.inspect` | – | UI state (tool, workspace, dock, timeline view, playback, fps…) | +| `ui.inspect` | – | UI state (tool, workspace, dock, timeline view, playback, fps…); `control` reports the control listener `{port, listening, error}` (`null` when neither `--control` nor a non-empty `FILMCRAFT_CONTROL_PORT` was given) | | `ui.docking` | – | the panel layout (craft-ui `Layout`), the focused and maximized panel, and `hitRects`: each docking gesture source drawn last frame (`{hit, rect}`) | | `ui.elements` | `{prefix?}` | every on-screen interactive element: id, label, rect; `window` (the `n` of `panel.float.`) marks an element drawn in a floating panel's own OS window, whose rect is in that window's coordinates | | `ui.set` | `{tool, workspace, mode, theme, focused, playbackRes, timeline:{pps,scroll,fit}, program:{…}, source:{…}, clipDialog:{param: value}, menuDialog:{param: value}, panels:{scopes, timecode, reference, events, progress}, export:{…}, keyframeSelection:[…]}` | `keyframeSelection` replaces the Effect Controls keyframe selection (`[{clip, effect, param, mask?, time}]`, `time` in media ticks; read back as `ui.inspect` → `ui.keyframe_selection`; a click on a lane keyframe selects just that one); `program` / `source` merge fields into the monitor view state (`res`, `paused_res`, `high_quality`, `display`, `zoom`, `pan`, `show_rulers`, `show_guides`, `lock_guides`, `snap`, `guides`, `compare_ref`, …; see [monitors.md](monitors.md)); `clipDialog` sets fields of the open Edit / Clip / File dialog (Paste Attributes, Make Subclip, Frame Hold Options, …); `menuDialog` those of the M3.11 dialogs (Find, Create Search Bin, Project Settings, Scene Edit Detection, Simplify Sequence, Automate to Sequence…; see `panels::menu_dialogs`); `panels` merges into the panel settings (`ui.inspect` → `ui.panels`; see `panels::panel_state`): Lumetri Scopes `{shown: ["vectorscopeYuv"|"vectorscopeHls"|"histogram"|"parade"|"waveform"], waveformType, paradeType, colorSpace, brightness, scale, clamp, targets}`, Timecode `{rows: [{source, mode, display}], showName}`, Reference Monitor `{ganged, time, display: "composite"|"scopes", scopes}`, Events `{level, selected}`, Progress `{showFinished}` `export` deep-merges into Export mode state (`preset` — applied first —, `settings` (ExportSettings JSON), `fileName`, `location`, `range`, `customStart`, `customEnd`, `openSections`, `manager` (Preset Manager: `query`, `favoritesOnly`, `selected`, `saveName`), `quickOpen`, `quickPath`, `quickPreset`); `program` / `source` merge fields into the monitor view state (`res`, `paused_res`, `high_quality`, `display`, `zoom`, `pan`, `show_rulers`, `show_guides`, `lock_guides`, `snap`, `guides`, `compare_ref`, …; see [monitors.md](monitors.md)); `clipDialog` sets fields of the open Edit / Clip / File dialog (Paste Attributes, Make Subclip, Frame Hold Options, …); `menuDialog` those of the M3.11 dialogs (Find, Create Search Bin, Project Settings, Scene Edit Detection, Simplify Sequence, Automate to Sequence…; see `panels::menu_dialogs`); `projectPanel` merges into the Project panel's view state (`bin` shown in place, `tabs` (bins in a tab or window: `{bin, floating, view, iconSize, nav}`), `activeTab`, `selectedBin`, `rename: {item|bin, text}`, `dialog` (`{"metadataDisplay": {columns, filter}}`, `{"savePresetAs": {name}}`, `{"managePresets": {selected, name}}`, `{"freeformOptions": {options}}`, `{"saveArrangement": {name, bin}}`)); `mediaBrowser` into the Media Browser's (`expanded`, `pathEdit`, `editColumns`, `treeWidth`). View settings, columns and presets are engine commands (`project.view.set`, `project.columns.set`, `project.viewPreset.*`, `mediaBrowser.settings`) | @@ -192,7 +200,7 @@ Ctrl), `Ctrl` (macOS ⌃), `Alt`, `Shift`. The dialog (Edit ▸ Keyboard Shortcu ## MCP `filmcraft-cli mcp` serves MCP on stdio: headless (in-process session; `--demo` / `--project p.fcproj`) -or `--bridge 127.0.0.1:9876` to drive the running app. Tools: `command_list`, `command_run`, +or `--bridge 127.0.0.1:9886` to drive the running app. Tools: `command_list`, `command_run`, `command_batch`, `doc_inspect`, `render_preview`, `project_inspect`, `sequence_inspect`, `media_import`, `render_frame`, `ui_inspect`, `ui_elements`, `ui_click`, `ui_drag`, `ui_key`, `ui_type`, `ui_screenshot`, `ui_control`; resources `filmcraft://document` and `filmcraft://commands`. `.mcp.json` registers both. diff --git a/docs/egui-037-migration-plan.md b/docs/egui-037-migration-plan.md --- a/docs/egui-037-migration-plan.md +++ b/docs/egui-037-migration-plan.md @@ -1226,7 +1226,7 @@ python3 /home/codex/work/maintenance/task.py run \ --task filmcraft-egui037-migration \ --source /home/codex/work/CraftApps/egui037-migration-20261022/filmcraft-implementation \ --evidence /home/codex/work/ssd2/task-evidence/egui037-migration-20261022/filmcraft \ - -- cargo run --release -p filmcraft -- --control 9876 + -- cargo run --release -p filmcraft -- --control 9886 python3 /home/codex/work/maintenance/task.py status filmcraft-egui037-linux-ui-01 --tail-bytes 8192 # after the UI workflow, evidence capture and review: python3 /home/codex/work/maintenance/task.py finish filmcraft-egui037-linux-ui-01 \ diff --git a/docs/testing.md b/docs/testing.md --- a/docs/testing.md +++ b/docs/testing.md @@ -253,7 +253,7 @@ project's sequences through a simulated 48 kHz device and counts dropouts, mixin For visual work, drive the real app: ```sh -cargo run --release -p filmcraft -- --control 9876 +cargo run --release -p filmcraft -- --control 9886 ``` Then script it over the control channel or MCP: run commands, click by automation id,