SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

refactor(cli,mcp,web): typed error enums for the densest stringly paths

Browse source 
CliError/SkillInstallError carry every op-cli failure (94 sites),
ProgramError types batch_program (14), WebCanvasError maps route
failures onto their existing HTTP statuses (23 -> 6 deliberate
boundary adapters). Display output is byte-identical to the old
strings so caller-visible text and tests are unchanged.
This commit is contained in:
Kayshen-X 2026-07-26 13:20:59 +08:00
parent 621f30e425
commit 0fc6753dd9
22 changed files with 1090 additions and 477 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

107
crates/op-cli/src/app_control_cli.rs
View file

@ -1,3 +1,4 @@
use crate::cli_error::CliError;
use op_process_io::{spawn_null, wait_for_child_or, wait_until_false, WaitOutcome};
use serde_json::json;
use std::env;
@ -47,7 +48,7 @@ pub(crate) fn run_start(
headless: bool,
web: bool,
host: Option<&str>,
) -> Result<String, String> {
) -> Result<String, CliError> {
if web {
return run_start_web(port, document_path, host);
}
@ -96,7 +97,7 @@ fn editor_will_open(path: &str) -> bool {
/// Launch the visible editor with `--live-mcp <port>` and wait for it to
/// publish the discovery port file. No `$TMPDIR` manager files: the
/// editor owns `~/.openpencil/.op-mcp-port` and removes it on exit.
fn run_start_live(port: u16, document_path: Option<&str>) -> Result<String, String> {
fn run_start_live(port: u16, document_path: Option<&str>) -> Result<String, CliError> {
let binary = find_desktop_binary()?;
let mut command = Command::new(&binary);
command.arg("--live-mcp").arg(port.to_string());
@ -106,7 +107,7 @@ fn run_start_live(port: u16, document_path: Option<&str>) -> Result<String, Stri
command.arg(path);
}
let mut child = spawn_null(&mut command)
.map_err(|e| format!("spawn {} --live-mcp: {e}", binary.display()))?;
.map_err(|e| CliError::Daemon(format!("spawn {} --live-mcp: {e}", binary.display())))?;
// Only report a `documentPath` the editor will ACTUALLY open: its
// `initial_file_from_argv` gate ignores a missing / unsupported path
@ -117,15 +118,15 @@ fn run_start_live(port: u16, document_path: Option<&str>) -> Result<String, Stri
match wait_for_child_or(&mut child, 150, Duration::from_millis(100), || {
reachable_live_port_file()
})
.map_err(|e| format!("wait for {} --live-mcp: {e}", binary.display()))?
.map_err(|e| CliError::Daemon(format!("wait for {} --live-mcp: {e}", binary.display())))?
{
WaitOutcome::Ready((live_port, live_pid)) => {
return Ok(start_json(live_pid, live_port, opened.map(Path::new)));
}
WaitOutcome::Exited(status) => {
return Err(format!(
return Err(CliError::Daemon(format!(
"OpenPencil editor exited before serving the live MCP server: {status}"
));
)));
}
WaitOutcome::TimedOut => {}
}
@ -133,10 +134,10 @@ fn run_start_live(port: u16, document_path: Option<&str>) -> Result<String, Stri
// rather than a fabricated success on the requested port — the editor
// process is left running and may still come up, but we cannot
// confirm the canvas is live yet.
Err(format!(
Err(CliError::Daemon(format!(
"OpenPencil editor did not publish a live MCP server within 15s \
(expected ~/.openpencil/{LIVE_PORT_FILE_NAME}); it may still be starting"
))
)))
}
/// `op start --web` — spawn the `--serve-web` daemon (headless: static wasm
@ -149,7 +150,7 @@ fn run_start_web(
port: u16,
document_path: Option<&str>,
host: Option<&str>,
) -> Result<String, String> {
) -> Result<String, CliError> {
// Reuse only an already-running WEB daemon (token-verified ping +
// `mode:"web-canvas"` health). A plain `--mcp-http` server answers the
// ping too but serves no editor, so reusing it would hand the user a
@ -160,10 +161,10 @@ fn run_start_web(
open_in_browser(&url);
return Ok(start_web_json(existing.pid, existing.port, None, host));
}
return Err(format!(
return Err(CliError::Daemon(format!(
"a non-web MCP server (pid {}) already owns port {}; run `op stop` first",
existing.pid, existing.port
));
)));
}
let binary = find_desktop_binary()?;
@ -188,7 +189,7 @@ fn run_start_web(
}
command.env(op_config_store::env_vars::MCP_TOKEN, &token);
let mut child = spawn_null(&mut command)
.map_err(|e| format!("spawn {} --serve-web: {e}", binary.display()))?;
.map_err(|e| CliError::Daemon(format!("spawn {} --serve-web: {e}", binary.display())))?;
let pid = child.id();
write_manager_files(pid, port, &token)?;
@ -199,7 +200,7 @@ fn run_start_web(
match wait_for_child_or(&mut child, 50, Duration::from_millis(100), || {
crate::mcp_http_cli::mcp_ping_headless(port, &token).then_some(())
})
.map_err(|e| format!("wait for {} --serve-web: {e}", binary.display()))?
.map_err(|e| CliError::Daemon(format!("wait for {} --serve-web: {e}", binary.display())))?
{
WaitOutcome::Ready(()) => {
let url = format!("http://127.0.0.1:{port}");
@ -208,15 +209,15 @@ fn run_start_web(
}
WaitOutcome::Exited(status) => {
remove_manager_files();
return Err(format!(
return Err(CliError::Daemon(format!(
"OpenPencil web daemon exited before accepting connections: {status}"
));
)));
}
WaitOutcome::TimedOut => {}
}
Err(format!(
Err(CliError::Daemon(format!(
"OpenPencil web daemon did not respond on 127.0.0.1:{port} within 5s"
))
)))
}
/// Best-effort default-browser launch, per OS: `open` (macOS),
@ -283,7 +284,7 @@ fn start_web_json(pid: u32, port: u16, document_path: Option<&Path>, host: Optio
/// Legacy windowless mode: spawn `--mcp-http` against a `.op` file and
/// track it via the `$TMPDIR` pid/port manager files.
fn run_start_headless(port: u16, document_path: Option<&str>) -> Result<String, String> {
fn run_start_headless(port: u16, document_path: Option<&str>) -> Result<String, CliError> {
let document = match document_path {
Some(path) => PathBuf::from(path),
None => default_document_path()?,
@ -302,7 +303,7 @@ fn run_start_headless(port: u16, document_path: Option<&str>) -> Result<String,
.arg(&document)
.env(op_config_store::env_vars::MCP_TOKEN, &token);
let mut child = spawn_null(&mut command)
.map_err(|e| format!("spawn {} --mcp-http: {e}", binary.display()))?;
.map_err(|e| CliError::Daemon(format!("spawn {} --mcp-http: {e}", binary.display())))?;
let pid = child.id();
write_manager_files(pid, port, &token)?;
@ -313,26 +314,26 @@ fn run_start_headless(port: u16, document_path: Option<&str>) -> Result<String,
match wait_for_child_or(&mut child, 30, Duration::from_millis(100), || {
crate::mcp_http_cli::mcp_ping_headless(port, &token).then_some(())
})
.map_err(|e| format!("wait for {} --mcp-http: {e}", binary.display()))?
.map_err(|e| CliError::Daemon(format!("wait for {} --mcp-http: {e}", binary.display())))?
{
WaitOutcome::Ready(()) => {
return Ok(start_json(pid, port, Some(&document)));
}
WaitOutcome::Exited(status) => {
remove_manager_files();
return Err(format!(
return Err(CliError::Daemon(format!(
"OpenPencil MCP server exited before accepting connections: {status}"
));
)));
}
WaitOutcome::TimedOut => {}
}
Err(format!(
Err(CliError::Daemon(format!(
"OpenPencil MCP server did not respond on 127.0.0.1:{port} within 3s"
))
)))
}
pub(crate) fn run_stop() -> Result<String, String> {
pub(crate) fn run_stop() -> Result<String, CliError> {
// `op stop` asks the server to quit ITSELF via a token-authed
// `openpencil/shutdown`. This NEVER signals a pid, so there is no
// recycled-pid / wrong-process race anywhere — and the live editor
@ -461,17 +462,22 @@ pub(crate) fn discover_running_port() -> Option<u16> {
reachable_headless_server().map(|info| info.port)
}
pub(crate) fn ensure_document_file(path: &Path) -> Result<(), String> {
pub(crate) fn ensure_document_file(path: &Path) -> Result<(), CliError> {
if path.exists() {
if path.is_file() {
return Ok(());
}
return Err(format!("{} exists but is not a file", path.display()));
return Err(CliError::Io(format!(
"{} exists but is not a file",
path.display()
)));
}
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).map_err(|e| format!("create {}: {e}", parent.display()))?;
fs::create_dir_all(parent)
.map_err(|e| CliError::Io(format!("create {}: {e}", parent.display())))?;
}
fs::write(path, MINIMAL_DOCUMENT).map_err(|e| format!("write {}: {e}", path.display()))
fs::write(path, MINIMAL_DOCUMENT)
.map_err(|e| CliError::Io(format!("write {}: {e}", path.display())))
}
/// Verify `path` holds a document the headless MCP server can actually load,
@ -485,26 +491,27 @@ pub(crate) fn ensure_document_file(path: &Path) -> Result<(), String> {
/// bare "connection refused" — with no hint that the file is the cause. The
/// check never mutates the file, so a corrupt-but-valuable document is
/// preserved rather than silently replaced.
fn preflight_document(path: &Path) -> Result<(), String> {
let bytes = fs::read(path).map_err(|e| format!("read {}: {e}", path.display()))?;
fn preflight_document(path: &Path) -> Result<(), CliError> {
let bytes =
fs::read(path).map_err(|e| CliError::Io(format!("read {}: {e}", path.display())))?;
// A `.op` document is UTF-8 JSON; a real `.fig` / `.pen` is a binary ZIP
// archive (invalid UTF-8). Reject the binary case here as a wrong file type
// rather than letting the server's `read_to_string` fail with an IO-flavored
// error that reads like the file is unreadable.
let src = std::str::from_utf8(&bytes).map_err(|_| {
format!(
CliError::Document(format!(
"{} is not a valid OpenPencil document: not UTF-8 text \
(looks like a binary archive). Only .op (JSON) documents open \
directly; legacy .pen / .fig files must be imported.",
path.display()
)
))
})?;
op_pen_loader::load_canonical(src).map(|_| ()).map_err(|e| {
format!(
CliError::Document(format!(
"{} is not a valid OpenPencil document: {e}\n\
Only .op (JSON) documents open directly; legacy .pen / .fig files must be imported.",
path.display()
)
))
})
}
@ -544,13 +551,14 @@ fn running_mcp_from_pid_file() -> Option<RunningMcp> {
Some(RunningMcp { pid, port, token })
}
fn write_manager_files(pid: u32, port: u16, token: &str) -> Result<(), String> {
fn write_manager_files(pid: u32, port: u16, token: &str) -> Result<(), CliError> {
let (pid_file, port_file, token_file) = manager_files();
fs::write(&pid_file, pid.to_string())
.map_err(|e| format!("write {}: {e}", pid_file.display()))?;
.map_err(|e| CliError::Io(format!("write {}: {e}", pid_file.display())))?;
fs::write(&port_file, port.to_string())
.map_err(|e| format!("write {}: {e}", port_file.display()))?;
fs::write(&token_file, token).map_err(|e| format!("write {}: {e}", token_file.display()))
.map_err(|e| CliError::Io(format!("write {}: {e}", port_file.display())))?;
fs::write(&token_file, token)
.map_err(|e| CliError::Io(format!("write {}: {e}", token_file.display())))
}
fn remove_manager_files() {
@ -581,29 +589,29 @@ fn make_token() -> String {
format!("{pid:x}-{nanos:x}")
}
fn default_document_path() -> Result<PathBuf, String> {
let store = op_config_store::ConfigStore::user().map_err(|e| e.to_string())?;
default_document_path_in(&store).map_err(|e| e.to_string())
fn default_document_path() -> Result<PathBuf, CliError> {
let store = op_config_store::ConfigStore::user().map_err(|e| CliError::Io(e.to_string()))?;
default_document_path_in(&store).map_err(|e| CliError::Io(e.to_string()))
}
fn default_document_path_in(store: &op_config_store::ConfigStore) -> std::io::Result<PathBuf> {
store.path(op_config_store::well_known::CLI_SESSION)
}
fn home_dir() -> Result<PathBuf, String> {
fn home_dir() -> Result<PathBuf, CliError> {
op_config_store::home_dir()
.map_err(|_| "home directory not available; pass --file <path.op>".to_string())
.map_err(|_| CliError::Io("home directory not available; pass --file <path.op>".into()))
}
fn find_desktop_binary() -> Result<PathBuf, String> {
fn find_desktop_binary() -> Result<PathBuf, CliError> {
if let Some(path) = env::var_os("OPENPENCIL_DESKTOP_BIN").map(PathBuf::from) {
if path.is_file() {
return Ok(path);
}
return Err(format!(
return Err(CliError::Daemon(format!(
"OPENPENCIL_DESKTOP_BIN points to a missing file: {}",
path.display()
));
)));
}
for path in desktop_binary_candidates() {
@ -611,7 +619,10 @@ fn find_desktop_binary() -> Result<PathBuf, String> {
return Ok(path);
}
}
Err("OpenPencil desktop binary not found; set OPENPENCIL_DESKTOP_BIN or build openpencil-desktop".into())
Err(CliError::Daemon(
"OpenPencil desktop binary not found; set OPENPENCIL_DESKTOP_BIN or build openpencil-desktop"
.into(),
))
}
fn desktop_binary_candidates() -> Vec<PathBuf> {

8
crates/op-cli/src/app_control_cli_tests.rs
View file

@ -17,7 +17,9 @@ fn preflight_rejects_malformed_documents_and_accepts_a_valid_one() {
// (which would exit(1) before binding → opaque "connection refused").
let archive = dir.join("archive.op");
fs::write(&archive, b"PK\x03\x04\xff\xfe\x00\x01binary\x80\x81").expect("write archive");
let err = preflight_document(&archive).expect_err("binary archive must be rejected");
let err = preflight_document(&archive)
.expect_err("binary archive must be rejected")
.to_string();
assert!(
err.contains("is not a valid OpenPencil document"),
"unexpected error text: {err}"
@ -27,7 +29,9 @@ fn preflight_rejects_malformed_documents_and_accepts_a_valid_one() {
// JSON parse-error path (distinct from the invalid-UTF-8 path above).
let garbage = dir.join("garbage.op");
fs::write(&garbage, b"this is not a .op document").expect("write garbage");
let err = preflight_document(&garbage).expect_err("non-document text must be rejected");
let err = preflight_document(&garbage)
.expect_err("non-document text must be rejected")
.to_string();
assert!(
err.contains("is not a valid OpenPencil document"),
"unexpected error text: {err}"

25
crates/op-cli/src/cli_conversion.rs
View file

@ -3,11 +3,12 @@
use std::fs;
use super::{flag_value, pair, tool_call, Command, Flags};
use crate::cli_error::CliError;
pub(crate) fn map_design_conversion(
positionals: &[String],
flags: &Flags,
) -> Result<Command, String> {
) -> Result<Command, CliError> {
match positionals[0].as_str() {
"design:upsert-vars" => map_upsert_variables(flags),
"design:upsert-component" => map_upsert_component(flags),
@ -18,7 +19,7 @@ pub(crate) fn map_design_conversion(
}
}
fn map_upsert_variables(flags: &Flags) -> Result<Command, String> {
fn map_upsert_variables(flags: &Flags) -> Result<Command, CliError> {
let key = required_flag(
flags,
"key",
@ -33,7 +34,7 @@ fn map_upsert_variables(flags: &Flags) -> Result<Command, String> {
tool_call("upsert_variables", args)
}
fn map_upsert_component(flags: &Flags) -> Result<Command, String> {
fn map_upsert_component(flags: &Flags) -> Result<Command, CliError> {
let key = required_flag(
flags,
"key",
@ -57,7 +58,7 @@ fn map_upsert_component(flags: &Flags) -> Result<Command, String> {
tool_call("upsert_component", args)
}
fn map_upsert_screen(flags: &Flags) -> Result<Command, String> {
fn map_upsert_screen(flags: &Flags) -> Result<Command, CliError> {
let key = required_flag(
flags,
"key",
@ -72,18 +73,20 @@ fn map_upsert_screen(flags: &Flags) -> Result<Command, String> {
tool_call("upsert_screen", args)
}
fn map_status(flags: &Flags) -> Result<Command, String> {
fn map_status(flags: &Flags) -> Result<Command, CliError> {
let mut args = Vec::new();
if let Some(kind) = flag_value(flags, "kind") {
if !matches!(kind.as_str(), "token" | "component" | "screen") {
return Err("--kind must be token, component, or screen".into());
return Err(CliError::usage(
"--kind must be token, component, or screen",
));
}
args.push(pair("kind", kind));
}
tool_call("conversion_status", args)
}
fn map_lint(flags: &Flags) -> Result<Command, String> {
fn map_lint(flags: &Flags) -> Result<Command, CliError> {
let mut args = Vec::new();
if let Some(node_id) = flag_value(flags, "node") {
args.push(pair("nodeId", node_id));
@ -91,15 +94,15 @@ fn map_lint(flags: &Flags) -> Result<Command, String> {
tool_call("lint_document", args)
}
fn required_flag(flags: &Flags, name: &str, usage: &str) -> Result<String, String> {
flag_value(flags, name).ok_or_else(|| usage.to_string())
fn required_flag(flags: &Flags, name: &str, usage: &str) -> Result<String, CliError> {
flag_value(flags, name).ok_or_else(|| CliError::usage(usage))
}
fn read_payload_file(flags: &Flags, usage: &str) -> Result<String, String> {
fn read_payload_file(flags: &Flags, usage: &str) -> Result<String, CliError> {
let path = required_flag(flags, "file", usage)?;
fs::read_to_string(&path)
.map(|contents| contents.trim().to_string())
.map_err(|e| format!("cannot read --file {path:?}: {e}"))
.map_err(|e| CliError::Io(format!("cannot read --file {path:?}: {e}")))
}
fn push_source_fields(args: &mut Vec<(String, String)>, flags: &Flags) {

71
crates/op-cli/src/cli_error.rs Normal file
View file

@ -0,0 +1,71 @@
//! The one error type every `op` subcommand fails with.
//!
//! Style mirrors `op_orchestrator::OrchestratorError`: a plain enum plus a
//! hand-written `Display` — no `thiserror`, no extra dependency. The
//! variants name the REAL failure kinds the CLI can hit (bad arguments, a
//! malformed payload, filesystem trouble, a daemon that will not come up, a
//! transport failure, a tool that reported an error, a document that will not
//! load). Each variant's payload is the exact user-facing sentence, so
//! `Display` is transparent: `main` still prints `op: {e}` byte-for-byte as
//! before and the message-shape tests keep passing.
//!
//! The classification is what's new and load-bearing — a caller (or a future
//! exit-code table) can now branch on the KIND instead of grepping the text.
use crate::skill_install_error::SkillInstallError;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum CliError {
/// Missing, malformed, or mutually exclusive command-line arguments.
/// The payload is the usage line or validation sentence shown to the user.
Usage(String),
/// A JSON payload (argument, file, or server reply) failed to parse,
/// failed to serialize, or had the wrong shape.
Payload(String),
/// A filesystem read / write / create failed.
Io(String),
/// Locating, spawning, or waiting for the OpenPencil desktop / daemon
/// process failed, or a conflicting server already owns the port.
Daemon(String),
/// Talking to a running MCP server over HTTP failed (connect, timeout,
/// or a non-2xx status).
Transport(String),
/// The remote MCP tool ran and reported its own failure. The payload is
/// the tool's verbatim text — never reworded, so agents parsing `op`
/// output see exactly what the server said.
Tool(String),
/// A document could not be read, validated, converted, or imported.
Document(String),
/// Installing or uninstalling the bundled agent skill failed.
Skill(SkillInstallError),
}
impl std::fmt::Display for CliError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
CliError::Usage(m)
| CliError::Payload(m)
| CliError::Io(m)
| CliError::Daemon(m)
| CliError::Transport(m)
| CliError::Tool(m)
| CliError::Document(m) => f.write_str(m),
CliError::Skill(e) => write!(f, "{e}"),
}
}
}
impl std::error::Error for CliError {}
impl From<SkillInstallError> for CliError {
fn from(error: SkillInstallError) -> Self {
CliError::Skill(error)
}
}
impl CliError {
/// Shorthand for the very common `--flag`-shaped usage complaint.
pub(crate) fn usage(message: impl Into<String>) -> Self {
CliError::Usage(message.into())
}
}

10
crates/op-cli/src/cli_export_tests.rs
View file

@ -98,7 +98,10 @@ fn export_rejects_conflicting_target_and_format_flags() {
"--output",
"/tmp/node.png",
]));
assert!(target.unwrap_err().contains("--item and --selection"));
assert!(target
.unwrap_err()
.to_string()
.contains("--item and --selection"));
let format = parse_args(&args(&[
"export",
@ -109,7 +112,10 @@ fn export_rejects_conflicting_target_and_format_flags() {
"--formats",
"jpeg",
]));
assert!(format.unwrap_err().contains("--format and --formats"));
assert!(format
.unwrap_err()
.to_string()
.contains("--format and --formats"));
}
#[test]

26
crates/op-cli/src/codegen_cli.rs
View file

@ -4,8 +4,9 @@ use super::{
flag_value, json_escape, pair, required_pos, resolve_arg, resolve_file_path_arg, tool_call,
Command, Flags,
};
use crate::cli_error::CliError;
pub(super) fn map_codegen(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(super) fn map_codegen(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
match positionals[0].as_str() {
"codegen:plan" => map_codegen_plan(positionals, flags),
"codegen:submit" => map_codegen_submit(positionals, flags),
@ -15,7 +16,7 @@ pub(super) fn map_codegen(positionals: &[String], flags: &Flags) -> Result<Comma
}
}
fn map_codegen_plan(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_codegen_plan(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let raw =
resolve_codegen_payload(positionals, 1, "Usage: op codegen:plan <plan-json|@file|->")?;
let plan = compact_json_object(&raw, "plan-json")?;
@ -31,7 +32,7 @@ fn map_codegen_plan(positionals: &[String], flags: &Flags) -> Result<Command, St
raw_json_tool_call("codegen_plan", fields)
}
fn map_codegen_submit(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_codegen_submit(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let plan_id = required_pos(
positionals,
1,
@ -53,7 +54,7 @@ fn map_codegen_submit(positionals: &[String], flags: &Flags) -> Result<Command,
raw_json_tool_call("codegen_submit_chunk", fields)
}
fn map_codegen_assemble(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_codegen_assemble(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let plan_id = required_pos(
positionals,
1,
@ -66,7 +67,7 @@ fn map_codegen_assemble(positionals: &[String], flags: &Flags) -> Result<Command
)
}
fn map_codegen_clean(positionals: &[String]) -> Result<Command, String> {
fn map_codegen_clean(positionals: &[String]) -> Result<Command, CliError> {
let plan_id = required_pos(positionals, 1, "Usage: op codegen:clean <planId>")?;
tool_call("codegen_clean", vec![pair("planId", plan_id)])
}
@ -75,28 +76,29 @@ fn resolve_codegen_payload(
positionals: &[String],
index: usize,
usage: &str,
) -> Result<String, String> {
) -> Result<String, CliError> {
let arg = positionals
.get(index)
.map(String::as_str)
.ok_or_else(|| usage.to_string())?;
.ok_or_else(|| CliError::usage(usage))?;
resolve_arg(Some(arg))
}
fn compact_json_object(raw: &str, label: &str) -> Result<String, String> {
fn compact_json_object(raw: &str, label: &str) -> Result<String, CliError> {
let value: Value = serde_json::from_str(raw.trim())
.map_err(|e| format!("invalid {label} JSON payload: {e}"))?;
.map_err(|e| CliError::Payload(format!("invalid {label} JSON payload: {e}")))?;
if !value.is_object() {
return Err(format!("{label} must be a JSON object"));
return Err(CliError::Payload(format!("{label} must be a JSON object")));
}
serde_json::to_string(&value).map_err(|e| format!("cannot serialize {label}: {e}"))
serde_json::to_string(&value)
.map_err(|e| CliError::Payload(format!("cannot serialize {label}: {e}")))
}
fn json_string_field(key: &str, value: &str) -> String {
format!(r#""{}":"{}""#, json_escape(key), json_escape(value))
}
fn raw_json_tool_call(tool: &str, fields: Vec<String>) -> Result<Command, String> {
fn raw_json_tool_call(tool: &str, fields: Vec<String>) -> Result<Command, CliError> {
Ok(Command::ToolCallJson {
tool: tool.to_string(),
args_json: format!("{{{}}}", fields.join(",")),

5
crates/op-cli/src/command_helpers.rs
View file

@ -1,3 +1,4 @@
use crate::cli_error::CliError;
use crate::path_args::resolve_file_path_arg;
use crate::{Command, Flags};
@ -15,13 +16,13 @@ pub(crate) fn pair(key: impl Into<String>, value: impl Into<String>) -> (String,
(key.into(), value.into())
}
pub(crate) fn tool_call_with_file(tool: &str, flags: &Flags) -> Result<Command, String> {
pub(crate) fn tool_call_with_file(tool: &str, flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
push_file_path(&mut pairs, flags);
tool_call(tool, pairs)
}
pub(crate) fn tool_call(tool: &str, args: Vec<(String, String)>) -> Result<Command, String> {
pub(crate) fn tool_call(tool: &str, args: Vec<(String, String)>) -> Result<Command, CliError> {
Ok(Command::ToolCall {
tool: tool.to_string(),
args,

45
crates/op-cli/src/export_cli.rs
View file

@ -3,33 +3,41 @@ use std::path::Path;
use base64::Engine as _;
use serde_json::Value;
use crate::cli_error::CliError;
use crate::command_helpers::flag_value;
use crate::mcp_http_cli::{post, tool_call_body};
use crate::{Command, Flags};
pub(crate) fn map_export(flags: &Flags) -> Result<Command, String> {
pub(crate) fn map_export(flags: &Flags) -> Result<Command, CliError> {
let item_id = flag_value(flags, "item");
let selection = flags.contains_key("selection");
if item_id.is_some() && selection {
return Err("--item and --selection cannot be used together".into());
return Err(CliError::usage(
"--item and --selection cannot be used together",
));
}
let format_flag = flag_value(flags, "format");
let formats_flag = flag_value(flags, "formats");
if let (Some(format), Some(formats)) = (&format_flag, &formats_flag) {
if format != formats {
return Err("--format and --formats cannot specify different values".into());
return Err(CliError::usage(
"--format and --formats cannot specify different values",
));
}
}
let format = format_flag.or(formats_flag).unwrap_or_else(|| "png".into());
if !matches!(format.as_str(), "png" | "jpeg" | "jpg" | "webp" | "pdf") {
return Err(format!("unsupported export format {format:?}"));
return Err(CliError::Usage(format!(
"unsupported export format {format:?}"
)));
}
let output = flag_value(flags, "output").ok_or("--output is required")?;
let output =
flag_value(flags, "output").ok_or_else(|| CliError::usage("--output is required"))?;
let scale = flag_value(flags, "scale");
if let Some(value) = &scale {
value
.parse::<f32>()
.map_err(|_| format!("--scale must be a number, got {value:?}"))?;
.map_err(|_| CliError::Usage(format!("--scale must be a number, got {value:?}")))?;
}
Ok(Command::Export {
item_id,
@ -46,7 +54,7 @@ pub(crate) fn run_export(
output: &str,
format: &str,
scale: Option<&str>,
) -> Result<String, String> {
) -> Result<String, CliError> {
let mut arguments = serde_json::Map::new();
if let Some(item_id) = item_id {
arguments.insert("itemId".into(), Value::String(item_id.into()));
@ -55,7 +63,7 @@ pub(crate) fn run_export(
if let Some(scale) = scale {
let scale = scale
.parse::<f64>()
.map_err(|_| format!("--scale must be a number, got {scale:?}"))?;
.map_err(|_| CliError::Usage(format!("--scale must be a number, got {scale:?}")))?;
arguments.insert("scale".into(), Value::from(scale));
}
let response = post(
@ -65,18 +73,25 @@ pub(crate) fn run_export(
write_export_response(&response, Path::new(output))
}
pub(crate) fn write_export_response(response: &str, output: &Path) -> Result<String, String> {
let value: Value = serde_json::from_str(response)
.map_err(|error| format!("export_item returned invalid JSON: {error}"))?;
pub(crate) fn write_export_response(response: &str, output: &Path) -> Result<String, CliError> {
let value: Value = serde_json::from_str(response).map_err(|error| {
CliError::Payload(format!("export_item returned invalid JSON: {error}"))
})?;
let encoded = value
.get("bytes_base64")
.and_then(Value::as_str)
.ok_or("export_item response is missing bytes_base64")?;
.ok_or_else(|| CliError::Payload("export_item response is missing bytes_base64".into()))?;
let bytes = base64::engine::general_purpose::STANDARD
.decode(encoded)
.map_err(|error| format!("export_item returned invalid Base64: {error}"))?;
std::fs::write(output, bytes)
.map_err(|error| format!("cannot write export to {}: {error}", output.display()))?;
.map_err(|error| {
CliError::Payload(format!("export_item returned invalid Base64: {error}"))
})?;
std::fs::write(output, bytes).map_err(|error| {
CliError::Io(format!(
"cannot write export to {}: {error}",
output.display()
))
})?;
Ok(serde_json::json!({
"output": output.to_string_lossy(),

14
crates/op-cli/src/figma_cli.rs
View file

@ -3,8 +3,9 @@ use std::path::Path;
use serde_json::json;
use super::{flag_value, required_pos, Command, Flags};
use crate::cli_error::CliError;
pub(super) fn map_import_figma(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(super) fn map_import_figma(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let fig_path = required_pos(
positionals,
1,
@ -21,22 +22,23 @@ pub(super) fn figma_default_out_path(fig_path: &str) -> String {
.unwrap_or_else(|| fig_path.to_string())
}
pub(super) fn run_import_figma(fig_path: &str, out_path: &str) -> Result<String, String> {
let bytes = std::fs::read(fig_path).map_err(|e| format!("read {fig_path:?}: {e}"))?;
pub(super) fn run_import_figma(fig_path: &str, out_path: &str) -> Result<String, CliError> {
let bytes =
std::fs::read(fig_path).map_err(|e| CliError::Io(format!("read {fig_path:?}: {e}")))?;
let file_name = Path::new(fig_path)
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("Figma Import");
let import = op_figma::parse_fig_binary(&bytes, file_name, op_figma::FigLayoutMode::OpenPencil)
.map_err(|e| format!("import {fig_path:?}: {e}"))?;
.map_err(|e| CliError::Document(format!("import {fig_path:?}: {e}")))?;
// Dedup shared image payloads into the `images` table — an
// image-heavy `.fig` references the same bitmap from many fills,
// and the inline form writes one full copy per reference.
let mut value = serde_json::to_value(&import.document)
.map_err(|e| format!("serialize {out_path:?}: {e}"))?;
.map_err(|e| CliError::Payload(format!("serialize {out_path:?}: {e}")))?;
jian_ops_schema::image_table::externalize_images(&mut value);
let raw = value.to_string();
std::fs::write(out_path, raw).map_err(|e| format!("write {out_path:?}: {e}"))?;
std::fs::write(out_path, raw).map_err(|e| CliError::Io(format!("write {out_path:?}: {e}")))?;
let page_count = import.document.pages.as_ref().map_or(1, Vec::len);
let node_count = import
.document

37
crates/op-cli/src/html_cli.rs
View file

@ -4,11 +4,12 @@ use jian_ops_schema::node::PenNode;
use serde_json::json;
use super::{flag_value, pair, push_file_path, required_pos, tool_call, Command, Flags};
use crate::cli_error::CliError;
/// The virtual origin `op-html` rebases project-local resources onto.
const LOCAL_RESOURCE_ORIGIN: &str = op_html::VIRTUAL_PROJECT_ORIGIN;
pub(super) fn map_import_svg(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(super) fn map_import_svg(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let path = required_pos(
positionals,
1,
@ -29,7 +30,7 @@ pub(super) fn map_import_svg(positionals: &[String], flags: &Flags) -> Result<Co
tool_call("import_svg", pairs)
}
pub(super) fn map_import_html(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(super) fn map_import_html(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let source = required_pos(
positionals,
1,
@ -38,7 +39,9 @@ pub(super) fn map_import_html(positionals: &[String], flags: &Flags) -> Result<C
let is_url = source.starts_with("http://") || source.starts_with("https://");
if let Some(out_path) = flag_value(flags, "out") {
if is_url {
return Err("--out requires a local file; URL import needs a running editor".into());
return Err(CliError::usage(
"--out requires a local file; URL import needs a running editor",
));
}
return Ok(Command::ImportHtml {
html_path: source,
@ -75,7 +78,7 @@ pub(super) fn map_import_html(positionals: &[String], flags: &Flags) -> Result<C
pub(super) fn map_import_snapshot(
positionals: &[String],
flags: &Flags,
) -> Result<Command, String> {
) -> Result<Command, CliError> {
let json_path = required_pos(
positionals,
1,
@ -103,9 +106,9 @@ pub(super) fn map_import_snapshot(
tool_call("import_web_snapshot", pairs)
}
pub(super) fn run_import_html(html_path: &str, out_path: &str) -> Result<String, String> {
let source_bytes =
std::fs::read(html_path).map_err(|error| format!("read {html_path:?}: {error}"))?;
pub(super) fn run_import_html(html_path: &str, out_path: &str) -> Result<String, CliError> {
let source_bytes = std::fs::read(html_path)
.map_err(|error| CliError::Io(format!("read {html_path:?}: {error}")))?;
let source = op_html::html_encoding::decode_html_bytes(&source_bytes);
let source_path = Path::new(html_path);
let resource_dir = source_path.parent().unwrap_or_else(|| Path::new("."));
@ -130,12 +133,14 @@ pub(super) fn run_import_html(html_path: &str, out_path: &str) -> Result<String,
.first()
.map(String::as_str)
.unwrap_or("input produced no nodes");
return Err(format!("no importable content: {detail}"));
return Err(CliError::Document(format!(
"no importable content: {detail}"
)));
}
let value = serde_json::to_value(&imported.document)
.map_err(|error| format!("serialize {out_path:?}: {error}"))?;
.map_err(|error| CliError::Payload(format!("serialize {out_path:?}: {error}")))?;
std::fs::write(out_path, value.to_string())
.map_err(|error| format!("write {out_path:?}: {error}"))?;
.map_err(|error| CliError::Io(format!("write {out_path:?}: {error}")))?;
Ok(json!({
"ok": true,
"filePath": out_path,
@ -145,9 +150,9 @@ pub(super) fn run_import_html(html_path: &str, out_path: &str) -> Result<String,
.to_string())
}
pub(super) fn run_import_snapshot(json_path: &str, out_path: &str) -> Result<String, String> {
pub(super) fn run_import_snapshot(json_path: &str, out_path: &str) -> Result<String, CliError> {
let source = std::fs::read_to_string(json_path)
.map_err(|error| format!("read {json_path:?}: {error}"))?;
.map_err(|error| CliError::Io(format!("read {json_path:?}: {error}")))?;
let imported =
op_html::import_snapshot_document(&source, &op_html::HtmlImportOptions::default());
if imported.document.children.is_empty() {
@ -156,12 +161,14 @@ pub(super) fn run_import_snapshot(json_path: &str, out_path: &str) -> Result<Str
.first()
.map(String::as_str)
.unwrap_or("input produced no nodes");
return Err(format!("no importable content: {detail}"));
return Err(CliError::Document(format!(
"no importable content: {detail}"
)));
}
let value = serde_json::to_value(&imported.document)
.map_err(|error| format!("serialize {out_path:?}: {error}"))?;
.map_err(|error| CliError::Payload(format!("serialize {out_path:?}: {error}")))?;
std::fs::write(out_path, value.to_string())
.map_err(|error| format!("write {out_path:?}: {error}"))?;
.map_err(|error| CliError::Io(format!("write {out_path:?}: {error}")))?;
Ok(json!({
"ok": true,
"filePath": out_path,

155
crates/op-cli/src/main.rs
View file

@ -8,6 +8,7 @@ use std::io::{self, Read};
mod app_control_cli;
mod cli_conversion;
mod cli_error;
mod codegen_cli;
mod command_helpers;
mod export_cli;
@ -18,6 +19,9 @@ mod page_theme_cli;
mod path_args;
mod skill_export_cli;
mod skill_install_cli;
mod skill_install_error;
use cli_error::CliError;
use command_helpers::{
flag_value, pair, push_file_path, tool_call, tool_call_with_file, version_json,
@ -52,7 +56,7 @@ fn main() {
}
/// Parse `args`, perform the request, return the text to print.
fn run(args: &[String]) -> Result<String, String> {
fn run(args: &[String]) -> Result<String, CliError> {
let Parsed {
port,
port_explicit,
@ -203,7 +207,7 @@ type Flags = BTreeMap<String, Option<String>>;
/// Parse command-line args. `--port`, `--pretty`, `--help`, and
/// `--version` are global; the rest are left for command aliases or
/// low-level MCP tool arguments.
fn parse_args(args: &[String]) -> Result<Parsed, String> {
fn parse_args(args: &[String]) -> Result<Parsed, CliError> {
let mut port = DEFAULT_PORT;
let mut port_explicit = false;
let mut pretty = false;
@ -237,16 +241,16 @@ fn parse_args(args: &[String]) -> Result<Parsed, String> {
let raw_port = match inline_value {
Some(v) => v,
None => {
let next = args
.get(i + 1)
.ok_or("--port needs a value (e.g. --port 3100)")?;
let next = args.get(i + 1).ok_or_else(|| {
CliError::usage("--port needs a value (e.g. --port 3100)")
})?;
i += 1;
next.clone()
}
};
port = raw_port
.parse::<u16>()
.map_err(|_| format!("--port must be a u16, got {raw_port:?}"))?;
port = raw_port.parse::<u16>().map_err(|_| {
CliError::Usage(format!("--port must be a u16, got {raw_port:?}"))
})?;
port_explicit = true;
}
"pretty" => pretty = true,
@ -291,7 +295,7 @@ fn parse_args(args: &[String]) -> Result<Parsed, String> {
})
}
fn command_from_positionals(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn command_from_positionals(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
match positionals[0].as_str() {
"help" | "-h" | "--help" => Ok(Command::Help),
"version" => Ok(Command::Version),
@ -301,13 +305,15 @@ fn command_from_positionals(positionals: &[String], flags: &Flags) -> Result<Com
let headless = flags.contains_key("headless");
let web = flags.contains_key("web");
if headless && web {
return Err("--headless and --web are mutually exclusive".into());
return Err(CliError::usage(
"--headless and --web are mutually exclusive",
));
}
let host = flag_value(flags, "host");
if host.is_some() && !web {
return Err(
"--host requires --web (only the web daemon binds non-loopback)".into(),
);
return Err(CliError::usage(
"--host requires --web (only the web daemon binds non-loopback)",
));
}
Ok(Command::StartMcp {
document_path: flag_value(flags, "file"),
@ -387,7 +393,7 @@ fn command_from_positionals(positionals: &[String], flags: &Flags) -> Result<Com
}
}
fn map_get(flags: &Flags) -> Result<Command, String> {
fn map_get(flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
if let Some(id) = flag_value(flags, "id") {
pairs.push(pair("nodeIds", format!(r#"["{}"]"#, json_escape(&id))));
@ -415,7 +421,7 @@ fn map_get(flags: &Flags) -> Result<Command, String> {
tool_call("batch_get", pairs)
}
fn map_selection(flags: &Flags) -> Result<Command, String> {
fn map_selection(flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
if let Some(depth) = flag_value(flags, "depth") {
pairs.push(pair("readDepth", depth));
@ -424,7 +430,7 @@ fn map_selection(flags: &Flags) -> Result<Command, String> {
tool_call("get_selection", pairs)
}
fn map_insert(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_insert(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let raw = resolve_arg(positionals.get(1).map(String::as_str))?;
let mut pairs = data_pairs(&raw)?;
if let Some(parent) = flag_value(flags, "parent") {
@ -440,7 +446,7 @@ fn map_insert(positionals: &[String], flags: &Flags) -> Result<Command, String>
tool_call("insert_node", pairs)
}
fn map_update(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_update(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let node_id = required_pos(positionals, 1, "Usage: op update <node-id> <json>")?;
let raw = resolve_arg(positionals.get(2).map(String::as_str))?;
let mut pairs = vec![pair("nodeId", node_id)];
@ -455,7 +461,7 @@ fn map_update(positionals: &[String], flags: &Flags) -> Result<Command, String>
tool_call("update_node", pairs)
}
fn map_replace(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_replace(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let node_id = required_pos(positionals, 1, "Usage: op replace <node-id> <json>")?;
let raw = resolve_arg(positionals.get(2).map(String::as_str))?;
let mut pairs = vec![pair("nodeId", node_id)];
@ -470,7 +476,7 @@ fn map_replace(positionals: &[String], flags: &Flags) -> Result<Command, String>
tool_call("replace_node", pairs)
}
fn map_delete(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_delete(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let id = required_pos(positionals, 1, "Usage: op delete <node-id>")?;
let mut pairs = vec![pair("nodeId", id)];
if let Some(page) = flag_value(flags, "page") {
@ -480,7 +486,7 @@ fn map_delete(positionals: &[String], flags: &Flags) -> Result<Command, String>
tool_call("delete_node", pairs)
}
fn map_read_nodes(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_read_nodes(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
if let Some(ids) = positionals.get(1) {
pairs.push(pair("nodeIds", ids.clone()));
@ -498,7 +504,7 @@ fn map_read_nodes(positionals: &[String], flags: &Flags) -> Result<Command, Stri
tool_call("read_nodes", pairs)
}
fn map_reparent(tool: &str, positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_reparent(tool: &str, positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let id = required_pos(
positionals,
1,
@ -528,7 +534,7 @@ fn map_design_like(
payload: Option<&String>,
flags: &Flags,
default_post_process: bool,
) -> Result<Command, String> {
) -> Result<Command, CliError> {
let raw = resolve_arg(payload.map(String::as_str))?;
let trimmed = raw.trim();
let script_requested = flags.contains_key("script")
@ -556,7 +562,7 @@ fn map_design_like(
tool_call(tool, pairs)
}
fn map_design_content(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_design_content(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let section_id = required_pos(
positionals,
1,
@ -564,21 +570,25 @@ fn map_design_content(positionals: &[String], flags: &Flags) -> Result<Command,
)?;
let raw = resolve_arg(positionals.get(2).map(String::as_str))?;
let payload: Value = serde_json::from_str(raw.trim())
.map_err(|e| format!("invalid design:content JSON payload: {e}"))?;
let children = payload
.get("children")
.ok_or("design:content JSON payload must contain a children array")?;
.map_err(|e| CliError::Payload(format!("invalid design:content JSON payload: {e}")))?;
let children = payload.get("children").ok_or_else(|| {
CliError::Payload("design:content JSON payload must contain a children array".into())
})?;
if !children.is_array() {
return Err("design:content children must be an array".into());
return Err(CliError::Payload(
"design:content children must be an array".into(),
));
}
let mut args = serde_json::Map::new();
args.insert("sectionId".into(), Value::String(section_id));
args.insert("children".into(), children.clone());
if let Some(canvas_width) = flag_value(flags, "canvas-width") {
let width = canvas_width
.parse::<i64>()
.map_err(|_| format!("--canvas-width must be an integer, got {canvas_width:?}"))?;
let width = canvas_width.parse::<i64>().map_err(|_| {
CliError::Usage(format!(
"--canvas-width must be an integer, got {canvas_width:?}"
))
})?;
args.insert("canvasWidth".into(), Value::Number(width.into()));
}
if let Some(page) = flag_value(flags, "page") {
@ -588,28 +598,32 @@ fn map_design_content(positionals: &[String], flags: &Flags) -> Result<Command,
args.insert("filePath".into(), resolve_file_path_value(&file));
}
let args_json = serde_json::to_string(&Value::Object(args))
.map_err(|e| format!("cannot serialize design:content args: {e}"))?;
.map_err(|e| CliError::Payload(format!("cannot serialize design:content args: {e}")))?;
Ok(Command::ToolCallJson {
tool: "design_content".into(),
args_json,
})
}
fn map_design_skeleton(positionals: &[String], flags: &Flags) -> Result<Command, String> {
fn map_design_skeleton(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let raw = resolve_arg(positionals.get(1).map(String::as_str))?;
let payload: Value = serde_json::from_str(raw.trim())
.map_err(|e| format!("invalid design:skeleton JSON payload: {e}"))?;
let root_frame = payload
.get("rootFrame")
.ok_or("design:skeleton JSON payload must contain rootFrame")?;
.map_err(|e| CliError::Payload(format!("invalid design:skeleton JSON payload: {e}")))?;
let root_frame = payload.get("rootFrame").ok_or_else(|| {
CliError::Payload("design:skeleton JSON payload must contain rootFrame".into())
})?;
if !root_frame.is_object() {
return Err("design:skeleton rootFrame must be an object".into());
return Err(CliError::Payload(
"design:skeleton rootFrame must be an object".into(),
));
}
let sections = payload
.get("sections")
.ok_or("design:skeleton JSON payload must contain sections")?;
let sections = payload.get("sections").ok_or_else(|| {
CliError::Payload("design:skeleton JSON payload must contain sections".into())
})?;
if !sections.is_array() {
return Err("design:skeleton sections must be an array".into());
return Err(CliError::Payload(
"design:skeleton sections must be an array".into(),
));
}
let mut args = serde_json::Map::new();
@ -617,14 +631,18 @@ fn map_design_skeleton(positionals: &[String], flags: &Flags) -> Result<Command,
args.insert("sections".into(), sections.clone());
if let Some(style_guide) = payload.get("styleGuide") {
if !style_guide.is_object() {
return Err("design:skeleton styleGuide must be an object".into());
return Err(CliError::Payload(
"design:skeleton styleGuide must be an object".into(),
));
}
args.insert("styleGuide".into(), style_guide.clone());
}
if let Some(canvas_width) = flag_value(flags, "canvas-width") {
let width = canvas_width
.parse::<i64>()
.map_err(|_| format!("--canvas-width must be an integer, got {canvas_width:?}"))?;
let width = canvas_width.parse::<i64>().map_err(|_| {
CliError::Usage(format!(
"--canvas-width must be an integer, got {canvas_width:?}"
))
})?;
args.insert("canvasWidth".into(), Value::Number(width.into()));
}
if let Some(page) = flag_value(flags, "page") {
@ -634,15 +652,16 @@ fn map_design_skeleton(positionals: &[String], flags: &Flags) -> Result<Command,
args.insert("filePath".into(), resolve_file_path_value(&file));
}
let args_json = serde_json::to_string(&Value::Object(args))
.map_err(|e| format!("cannot serialize design:skeleton args: {e}"))?;
.map_err(|e| CliError::Payload(format!("cannot serialize design:skeleton args: {e}")))?;
Ok(Command::ToolCallJson {
tool: "design_skeleton".into(),
args_json,
})
}
fn map_design_refine(flags: &Flags) -> Result<Command, String> {
let root_id = flag_value(flags, "root-id").ok_or("Usage: op design:refine --root-id <id>")?;
fn map_design_refine(flags: &Flags) -> Result<Command, CliError> {
let root_id = flag_value(flags, "root-id")
.ok_or_else(|| CliError::usage("Usage: op design:refine --root-id <id>"))?;
let mut pairs = vec![pair("rootId", root_id)];
if let Some(canvas_width) = flag_value(flags, "canvas-width") {
pairs.push(pair("canvasWidth", canvas_width));
@ -654,7 +673,7 @@ fn map_design_refine(flags: &Flags) -> Result<Command, String> {
tool_call("design_refine", pairs)
}
fn map_layout(flags: &Flags) -> Result<Command, String> {
fn map_layout(flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
if let Some(parent) = flag_value(flags, "parent") {
pairs.push(pair("parentId", parent));
@ -669,7 +688,7 @@ fn map_layout(flags: &Flags) -> Result<Command, String> {
tool_call("snapshot_layout", pairs)
}
fn map_find_space(flags: &Flags) -> Result<Command, String> {
fn map_find_space(flags: &Flags) -> Result<Command, CliError> {
let direction = flag_value(flags, "direction").unwrap_or_else(|| "right".into());
let width = flag_value(flags, "width").unwrap_or_else(|| "400".into());
let height = flag_value(flags, "height").unwrap_or_else(|| "300".into());
@ -694,14 +713,16 @@ fn map_find_space(flags: &Flags) -> Result<Command, String> {
tool_call("find_empty_space", pairs)
}
fn generic_tool_call(tool: &str, rest: &[String], flags: &Flags) -> Result<Command, String> {
fn generic_tool_call(tool: &str, rest: &[String], flags: &Flags) -> Result<Command, CliError> {
let mut pairs = Vec::new();
for kv in rest {
let (k, v) = kv
.split_once('=')
.ok_or_else(|| format!("argument must be key=value, got {kv:?}"))?;
.ok_or_else(|| CliError::Usage(format!("argument must be key=value, got {kv:?}")))?;
if k.is_empty() {
return Err(format!("argument has an empty key: {kv:?}"));
return Err(CliError::Usage(format!(
"argument has an empty key: {kv:?}"
)));
}
pairs.push(pair(k, v));
}
@ -719,29 +740,29 @@ fn is_global_compat_flag(key: &str) -> bool {
["file", "page", "post-process", "canvas-width", "depth"].contains(&key)
}
fn parse_json_object(raw: &str) -> Result<Value, String> {
let value: Value =
serde_json::from_str(raw.trim()).map_err(|e| format!("invalid JSON payload: {e}"))?;
fn parse_json_object(raw: &str) -> Result<Value, CliError> {
let value: Value = serde_json::from_str(raw.trim())
.map_err(|e| CliError::Payload(format!("invalid JSON payload: {e}")))?;
if !value.is_object() {
return Err("JSON payload must be an object".into());
return Err(CliError::Payload("JSON payload must be an object".into()));
}
Ok(value)
}
fn data_pairs(raw: &str) -> Result<Vec<(String, String)>, String> {
fn data_pairs(raw: &str) -> Result<Vec<(String, String)>, CliError> {
parse_json_object(raw)?;
Ok(vec![pair("data", raw.trim())])
}
fn resolve_arg(arg: Option<&str>) -> Result<String, String> {
fn resolve_arg(arg: Option<&str>) -> Result<String, CliError> {
match arg {
Some("-") => {
let mut input = String::new();
io::stdin()
.read_to_string(&mut input)
.map_err(|e| format!("stdin read failed: {e}"))?;
.map_err(|e| CliError::Io(format!("stdin read failed: {e}")))?;
if input.trim().is_empty() {
Err("No data received from stdin".into())
Err(CliError::usage("No data received from stdin"))
} else {
Ok(input.trim().to_string())
}
@ -750,18 +771,20 @@ fn resolve_arg(arg: Option<&str>) -> Result<String, String> {
let path = &path[1..];
fs::read_to_string(path)
.map(|s| s.trim().to_string())
.map_err(|e| format!("cannot read file {path:?}: {e}"))
.map_err(|e| CliError::Io(format!("cannot read file {path:?}: {e}")))
}
Some(value) => Ok(value.to_string()),
None => Err("No data provided. Pass as argument, @filepath, or '-' for stdin".into()),
None => Err(CliError::usage(
"No data provided. Pass as argument, @filepath, or '-' for stdin",
)),
}
}
fn required_pos(positionals: &[String], index: usize, usage: &str) -> Result<String, String> {
fn required_pos(positionals: &[String], index: usize, usage: &str) -> Result<String, CliError> {
positionals
.get(index)
.cloned()
.ok_or_else(|| usage.to_string())
.ok_or_else(|| CliError::usage(usage))
}
#[cfg(test)]

33
crates/op-cli/src/mcp_http_cli.rs
View file

@ -1,6 +1,7 @@
use serde_json::Value;
use std::time::{SystemTime, UNIX_EPOCH};
use crate::cli_error::CliError;
use op_rpc_transport::{JsonRpcRequest, TcpJsonRpc, PING_TIMEOUT, POST_TIMEOUT, SHUTDOWN_TIMEOUT};
/// OpenPencil MCP identity marker reported in the `ping` reply's `result`
@ -99,10 +100,18 @@ pub(crate) fn http_request(body: &str) -> String {
/// return `(http_status, body)`. `http_status` is 0 when no recognizable
/// status line was returned. The deadlines stop a stale port whose
/// service accepts but never replies from hanging the CLI indefinitely.
fn post_raw(port: u16, body: &str, timeout: std::time::Duration) -> Result<(u16, String), String> {
fn post_raw(
port: u16,
body: &str,
timeout: std::time::Duration,
) -> Result<(u16, String), CliError> {
// `op-rpc-transport` is a shared crate outside this conversion's scope and
// still reports failures as `String`; adapt it here so the stringly-typed
// error never escapes into the CLI's own call graph.
TcpJsonRpc::local_mcp(port)
.post_raw(body, timeout)
.map(|reply| (reply.status, reply.body))
.map_err(CliError::Transport)
}
/// POST `body` to the HTTP MCP server on `127.0.0.1:port` and return the
@ -111,12 +120,12 @@ fn post_raw(port: u16, body: &str, timeout: std::time::Duration) -> Result<(u16,
/// `tools/call` content envelope is unwrapped to the raw tool result, and an
/// `isError` tool result (or a JSON-RPC transport error) is surfaced as an
/// error — so `op` exits non-zero on a failed tool, like the TS CLI.
pub(crate) fn post(port: u16, body: &str) -> Result<String, String> {
pub(crate) fn post(port: u16, body: &str) -> Result<String, CliError> {
let (status, reply) = post_raw(port, body, POST_TIMEOUT)?;
if !(200..300).contains(&status) {
return Err(format!(
return Err(CliError::Transport(format!(
"MCP server on 127.0.0.1:{port} returned HTTP {status}: {reply}"
));
)));
}
unwrap_mcp_reply(&reply)
}
@ -127,7 +136,7 @@ pub(crate) fn post(port: u16, body: &str) -> Result<String, String> {
/// an `isError:true` result → `Err(text)`.
/// - a JSON-RPC transport `error` → `Err(message)`.
/// - anything else (e.g. a `tools/list` reply) → the raw reply unchanged.
fn unwrap_mcp_reply(reply: &str) -> Result<String, String> {
fn unwrap_mcp_reply(reply: &str) -> Result<String, CliError> {
let Ok(value) = serde_json::from_str::<Value>(reply) else {
return Ok(reply.to_string());
};
@ -136,7 +145,7 @@ fn unwrap_mcp_reply(reply: &str) -> Result<String, String> {
.and_then(|err| err.get("message"))
.and_then(Value::as_str)
{
return Err(message.to_string());
return Err(CliError::Tool(message.to_string()));
}
let Some(result) = value.get("result") else {
return Ok(reply.to_string());
@ -148,7 +157,7 @@ fn unwrap_mcp_reply(reply: &str) -> Result<String, String> {
.collect::<Vec<_>>()
.join("\n");
if result.get("isError").and_then(Value::as_bool) == Some(true) {
return Err(text);
return Err(CliError::Tool(text));
}
return Ok(text);
}
@ -161,10 +170,11 @@ fn http_get_raw(
port: u16,
path: &str,
timeout: std::time::Duration,
) -> Result<(u16, String), String> {
) -> Result<(u16, String), CliError> {
TcpJsonRpc::local_mcp(port)
.get_raw(path, timeout)
.map(|reply| (reply.status, reply.body))
.map_err(CliError::Transport)
}
/// True when `127.0.0.1:port` is the `--serve-web` web-canvas daemon —
@ -279,13 +289,16 @@ mod tests {
#[test]
fn unwrap_surfaces_iserror_result_as_err() {
let reply = r#"{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"Error: boom"}],"isError":true}}"#;
assert_eq!(unwrap_mcp_reply(reply), Err("Error: boom".to_string()));
assert_eq!(
unwrap_mcp_reply(reply).unwrap_err().to_string(),
"Error: boom"
);
}
#[test]
fn unwrap_surfaces_transport_error_as_err() {
let reply = r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"bad args"}}"#;
assert_eq!(unwrap_mcp_reply(reply), Err("bad args".to_string()));
assert_eq!(unwrap_mcp_reply(reply).unwrap_err().to_string(), "bad args");
}
#[test]

22
crates/op-cli/src/page_theme_cli.rs
View file

@ -2,14 +2,14 @@
//! mappers — split out of `main.rs` per the 800-line-cap convention. Pure
//! arg → `Command` mapping; the shared helpers stay in `main.rs`.
use crate::cli_error::CliError;
use crate::{flag_value, pair, push_file_path, required_pos, resolve_arg, tool_call};
use crate::{Command, Flags};
pub(crate) fn map_page(positionals: &[String], flags: &Flags) -> Result<Command, String> {
let sub = positionals
.get(1)
.map(String::as_str)
.ok_or("Usage: op page list|add|remove|rename|reorder|duplicate ...")?;
pub(crate) fn map_page(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let sub = positionals.get(1).map(String::as_str).ok_or_else(|| {
CliError::usage("Usage: op page list|add|remove|rename|reorder|duplicate ...")
})?;
match sub {
"list" => {
let mut pairs = Vec::new();
@ -54,11 +54,11 @@ pub(crate) fn map_page(positionals: &[String], flags: &Flags) -> Result<Command,
push_file_path(&mut pairs, flags);
tool_call("duplicate_page", pairs)
}
_ => Err(format!("unknown page subcommand {sub:?}")),
_ => Err(CliError::Usage(format!("unknown page subcommand {sub:?}"))),
}
}
pub(crate) fn map_vars_set(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(crate) fn map_vars_set(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let raw = resolve_arg(positionals.get(1).map(String::as_str))?;
let mut pairs = vec![pair("variables", raw)];
if flags.contains_key("replace") {
@ -68,7 +68,7 @@ pub(crate) fn map_vars_set(positionals: &[String], flags: &Flags) -> Result<Comm
tool_call("set_variables", pairs)
}
pub(crate) fn map_themes_set(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(crate) fn map_themes_set(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let raw = resolve_arg(positionals.get(1).map(String::as_str))?;
let mut pairs = vec![pair("themes", raw)];
if flags.contains_key("replace") {
@ -78,7 +78,7 @@ pub(crate) fn map_themes_set(positionals: &[String], flags: &Flags) -> Result<Co
tool_call("set_themes", pairs)
}
pub(crate) fn map_theme_save(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(crate) fn map_theme_save(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let preset_path = required_pos(positionals, 1, "Usage: op theme:save <file.optheme>")?;
let mut pairs = vec![pair("presetPath", preset_path)];
if let Some(name) = flag_value(flags, "name") {
@ -88,14 +88,14 @@ pub(crate) fn map_theme_save(positionals: &[String], flags: &Flags) -> Result<Co
tool_call("save_theme_preset", pairs)
}
pub(crate) fn map_theme_load(positionals: &[String], flags: &Flags) -> Result<Command, String> {
pub(crate) fn map_theme_load(positionals: &[String], flags: &Flags) -> Result<Command, CliError> {
let preset_path = required_pos(positionals, 1, "Usage: op theme:load <file.optheme>")?;
let mut pairs = vec![pair("presetPath", preset_path)];
push_file_path(&mut pairs, flags);
tool_call("load_theme_preset", pairs)
}
pub(crate) fn map_theme_list(positionals: &[String]) -> Result<Command, String> {
pub(crate) fn map_theme_list(positionals: &[String]) -> Result<Command, CliError> {
let directory = required_pos(positionals, 1, "Usage: op theme:list <directory>")?;
tool_call("list_theme_presets", vec![pair("directory", directory)])
}

14
crates/op-cli/src/skill_export_cli.rs
View file

@ -5,14 +5,17 @@ use std::path::Path;
use serde_json::json;
use crate::cli_error::CliError;
const DEFAULT_SKILL_OUT_DIR: &str = ".claude/skills";
pub(crate) fn run_export(name: &str, out_dir: Option<&str>) -> Result<String, String> {
let skill =
op_ai_skills::get_skill_by_name(name).ok_or_else(|| format!("unknown skill {name:?}"))?;
pub(crate) fn run_export(name: &str, out_dir: Option<&str>) -> Result<String, CliError> {
let skill = op_ai_skills::get_skill_by_name(name)
.ok_or_else(|| CliError::Usage(format!("unknown skill {name:?}")))?;
let root = out_dir.unwrap_or(DEFAULT_SKILL_OUT_DIR);
let dir = Path::new(root).join(&skill.meta.name);
fs::create_dir_all(&dir).map_err(|e| format!("cannot create {}: {e}", dir.display()))?;
fs::create_dir_all(&dir)
.map_err(|e| CliError::Io(format!("cannot create {}: {e}", dir.display())))?;
let path = dir.join("SKILL.md");
let contents = format!(
@ -21,7 +24,8 @@ pub(crate) fn run_export(name: &str, out_dir: Option<&str>) -> Result<String, St
skill.meta.description.trim(),
skill.content.trim()
);
fs::write(&path, contents).map_err(|e| format!("cannot write {}: {e}", path.display()))?;
fs::write(&path, contents)
.map_err(|e| CliError::Io(format!("cannot write {}: {e}", path.display())))?;
Ok(json!({
"ok": true,

156
crates/op-cli/src/skill_install_cli.rs
View file

@ -3,8 +3,12 @@ use std::env;
use std::fs;
use std::path::{Path, PathBuf};
use crate::skill_install_error::{BundleError, FsAction, SkillInstallError};
type Result<T> = std::result::Result<T, SkillInstallError>;
const BUNDLE_JSON: &str = include_str!("../assets/skill-bundle.json");
const VERSION_SENTINEL: &str = "__OPENPENCIL_VERSION__";
pub(crate) const VERSION_SENTINEL: &str = "__OPENPENCIL_VERSION__";
// Top-level bundle version plus four embedded plugin/package manifest versions.
const EXPECTED_VERSION_SENTINEL_COUNT: usize = 5;
const REPO: &str = "zseven-w/openpencil-skill";
@ -25,15 +29,13 @@ enum Target {
}
impl Target {
fn parse(raw: &str) -> Result<Self, String> {
fn parse(raw: &str) -> Result<Self> {
match raw.to_ascii_lowercase().as_str() {
"claude" | "claude-code" | "claudecode" => Ok(Target::Claude),
"codex" => Ok(Target::Codex),
"cursor" => Ok(Target::Cursor),
"opencode" | "open-code" => Ok(Target::OpenCode),
_ => Err(format!(
"unknown target {raw:?}; available: claude, codex, cursor, opencode"
)),
_ => Err(SkillInstallError::UnknownTarget(raw.to_string())),
}
}
@ -47,22 +49,22 @@ impl Target {
}
}
pub(crate) fn run_install(target: Option<&str>) -> Result<String, String> {
pub(crate) fn run_install(target: Option<&str>) -> Result<String> {
run_for_home(Action::Install, target, &home_dir()?)
}
pub(crate) fn run_uninstall(target: Option<&str>) -> Result<String, String> {
pub(crate) fn run_uninstall(target: Option<&str>) -> Result<String> {
run_for_home(Action::Uninstall, target, &home_dir()?)
}
#[cfg(test)]
pub(crate) fn install_target_at_home(target: &str, home: &Path) -> Result<(), String> {
pub(crate) fn install_target_at_home(target: &str, home: &Path) -> Result<()> {
let bundle = load_bundle()?;
install_target(Target::parse(target)?, home, &bundle)
}
#[cfg(test)]
pub(crate) fn uninstall_target_at_home(target: &str, home: &Path) -> Result<(), String> {
pub(crate) fn uninstall_target_at_home(target: &str, home: &Path) -> Result<()> {
uninstall_target(Target::parse(target)?, home)
}
@ -72,7 +74,7 @@ enum Action {
Uninstall,
}
fn run_for_home(action: Action, target: Option<&str>, home: &Path) -> Result<String, String> {
fn run_for_home(action: Action, target: Option<&str>, home: &Path) -> Result<String> {
let targets = resolve_targets(target, action, home)?;
let bundle = load_bundle()?;
let mut results = Vec::new();
@ -83,7 +85,9 @@ fn run_for_home(action: Action, target: Option<&str>, home: &Path) -> Result<Str
};
results.push(match result {
Ok(()) => json!({ "target": target.key(), "ok": true }),
Err(error) => json!({ "target": target.key(), "ok": false, "error": error }),
Err(error) => {
json!({ "target": target.key(), "ok": false, "error": error.to_string() })
}
});
}
Ok(json!({
@ -96,20 +100,13 @@ fn run_for_home(action: Action, target: Option<&str>, home: &Path) -> Result<Str
.to_string())
}
fn resolve_targets(
target: Option<&str>,
action: Action,
home: &Path,
) -> Result<Vec<Target>, String> {
fn resolve_targets(target: Option<&str>, action: Action, home: &Path) -> Result<Vec<Target>> {
if let Some(target) = target {
return Ok(vec![Target::parse(target)?]);
}
let detected = detect_targets(home);
if detected.is_empty() && matches!(action, Action::Install) {
return Err(
"no supported AI coding agents detected; pass --target claude|codex|cursor|opencode"
.into(),
);
return Err(SkillInstallError::NoTargetsDetected);
}
Ok(detected)
}
@ -141,7 +138,7 @@ fn command_exists(name: &str) -> bool {
})
}
fn install_target(target: Target, home: &Path, bundle: &SkillBundle) -> Result<(), String> {
fn install_target(target: Target, home: &Path, bundle: &SkillBundle) -> Result<()> {
match target {
Target::Claude => install_claude(home, bundle),
Target::Codex => install_codex(home, bundle),
@ -150,7 +147,7 @@ fn install_target(target: Target, home: &Path, bundle: &SkillBundle) -> Result<(
}
}
fn uninstall_target(target: Target, home: &Path) -> Result<(), String> {
fn uninstall_target(target: Target, home: &Path) -> Result<()> {
match target {
Target::Claude => uninstall_claude(home),
Target::Codex => uninstall_codex(home),
@ -159,7 +156,7 @@ fn uninstall_target(target: Target, home: &Path) -> Result<(), String> {
}
}
fn install_claude(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
fn install_claude(home: &Path, bundle: &SkillBundle) -> Result<()> {
let cache_dir = home
.join(".claude/plugins/cache")
.join(SKILL_NAME)
@ -197,7 +194,7 @@ fn install_claude(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
write_json_object(&marketplace_path, &marketplaces)
}
fn uninstall_claude(home: &Path) -> Result<(), String> {
fn uninstall_claude(home: &Path) -> Result<()> {
remove_path(&home.join(".claude/plugins/cache").join(SKILL_NAME))?;
let registry_path = home.join(".claude/plugins/installed_plugins.json");
if registry_path.exists() {
@ -210,12 +207,13 @@ fn uninstall_claude(home: &Path) -> Result<(), String> {
Ok(())
}
fn install_codex(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
fn install_codex(home: &Path, bundle: &SkillBundle) -> Result<()> {
let clone_dir = home.join(".codex").join(SKILL_NAME);
write_bundle_to(&clone_dir, bundle)?;
let skills_dir = home.join(".agents/skills");
fs::create_dir_all(&skills_dir).map_err(|e| format!("create {}: {e}", skills_dir.display()))?;
fs::create_dir_all(&skills_dir)
.map_err(|e| SkillInstallError::fs(FsAction::Create, &skills_dir, e))?;
let link_path = skills_dir.join(SKILL_NAME);
let link_target = clone_dir.join("skills");
if fs::symlink_metadata(&link_path).is_err() {
@ -224,12 +222,12 @@ fn install_codex(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
Ok(())
}
fn uninstall_codex(home: &Path) -> Result<(), String> {
fn uninstall_codex(home: &Path) -> Result<()> {
remove_path(&home.join(".agents/skills").join(SKILL_NAME))?;
remove_path(&home.join(".codex").join(SKILL_NAME))
}
fn install_opencode(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
fn install_opencode(home: &Path, bundle: &SkillBundle) -> Result<()> {
// opencode discovers skills by scanning its config directory for
// `{skill,skills}/**/SKILL.md` (packages/opencode/src/skill/index.ts).
// A `plugin` array entry does NOT work for skills: the npm/git package
@ -240,7 +238,8 @@ fn install_opencode(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
write_bundle_to(&bundle_dir, bundle)?;
let skills_dir = home.join(".config/opencode/skills");
fs::create_dir_all(&skills_dir).map_err(|e| format!("create {}: {e}", skills_dir.display()))?;
fs::create_dir_all(&skills_dir)
.map_err(|e| SkillInstallError::fs(FsAction::Create, &skills_dir, e))?;
let link_path = skills_dir.join(SKILL_NAME);
let link_target = bundle_dir.join("skills");
// The discovery entry is owned by this installer: recreate it on every
@ -253,7 +252,7 @@ fn install_opencode(home: &Path, bundle: &SkillBundle) -> Result<(), String> {
prune_opencode_plugin_entry(home)
}
fn uninstall_opencode(home: &Path) -> Result<(), String> {
fn uninstall_opencode(home: &Path) -> Result<()> {
remove_path(&home.join(".config/opencode/skills").join(SKILL_NAME))?;
remove_path(&home.join(".config/opencode").join(SKILL_NAME))?;
prune_opencode_plugin_entry(home)
@ -261,7 +260,7 @@ fn uninstall_opencode(home: &Path) -> Result<(), String> {
/// Remove the legacy `openpencil-skill@git+…` plugin entry (older installers
/// wrote it; opencode installs the package but never loads anything from it).
fn prune_opencode_plugin_entry(home: &Path) -> Result<(), String> {
fn prune_opencode_plugin_entry(home: &Path) -> Result<()> {
let config_path = home.join(".config/opencode/opencode.json");
if !config_path.exists() {
return Ok(());
@ -272,73 +271,75 @@ fn prune_opencode_plugin_entry(home: &Path) -> Result<(), String> {
write_json_object(&config_path, &config)
}
fn render_bundle_template(template: &str, version: &str) -> Result<String, String> {
fn render_bundle_template(
template: &str,
version: &str,
) -> std::result::Result<String, BundleError> {
let sentinel_count = template.matches(VERSION_SENTINEL).count();
if sentinel_count != EXPECTED_VERSION_SENTINEL_COUNT {
return Err(format!(
"embedded skill bundle template expected {EXPECTED_VERSION_SENTINEL_COUNT} version \
sentinels {VERSION_SENTINEL:?}, found {sentinel_count}"
));
return Err(BundleError::SentinelCount {
expected: EXPECTED_VERSION_SENTINEL_COUNT,
found: sentinel_count,
});
}
let rendered = template.replace(VERSION_SENTINEL, version);
if rendered.contains(VERSION_SENTINEL) {
return Err(
"embedded skill bundle still contains the version sentinel after rendering".into(),
);
return Err(BundleError::SentinelRemains);
}
Ok(rendered)
}
fn load_bundle() -> Result<SkillBundle, String> {
fn load_bundle() -> Result<SkillBundle> {
let rendered = render_bundle_template(BUNDLE_JSON, env!("CARGO_PKG_VERSION"))?;
let value: Value =
serde_json::from_str(&rendered).map_err(|e| format!("parse skill bundle: {e}"))?;
serde_json::from_str(&rendered).map_err(|e| BundleError::Parse(e.to_string()))?;
let version = value
.get("version")
.and_then(Value::as_str)
.ok_or("skill bundle missing version")?
.ok_or(BundleError::MissingField("version"))?
.to_string();
let files_obj = value
.get("files")
.and_then(Value::as_object)
.ok_or("skill bundle missing files")?;
.ok_or(BundleError::MissingField("files"))?;
if files_obj.is_empty() {
return Err("embedded skill bundle is empty".into());
return Err(BundleError::Empty.into());
}
let mut files = Vec::new();
for (path, content) in files_obj {
let content = content
.as_str()
.ok_or_else(|| format!("bundle file {path:?} is not a string"))?;
.ok_or_else(|| BundleError::FileNotString(path.clone()))?;
files.push((path.clone(), content.to_string()));
}
Ok(SkillBundle { version, files })
}
fn write_bundle_to(dest: &Path, bundle: &SkillBundle) -> Result<(), String> {
fs::create_dir_all(dest).map_err(|e| format!("create {}: {e}", dest.display()))?;
fn write_bundle_to(dest: &Path, bundle: &SkillBundle) -> Result<()> {
fs::create_dir_all(dest).map_err(|e| SkillInstallError::fs(FsAction::Create, dest, e))?;
for (relative, content) in &bundle.files {
let path = dest.join(relative);
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).map_err(|e| format!("create {}: {e}", parent.display()))?;
fs::create_dir_all(parent)
.map_err(|e| SkillInstallError::fs(FsAction::Create, parent, e))?;
}
fs::write(&path, content).map_err(|e| format!("write {}: {e}", path.display()))?;
fs::write(&path, content).map_err(|e| SkillInstallError::fs(FsAction::Write, &path, e))?;
}
Ok(())
}
fn link_or_copy_dir(target: &Path, link_path: &Path) -> Result<(), String> {
fn link_or_copy_dir(target: &Path, link_path: &Path) -> Result<()> {
#[cfg(unix)]
{
std::os::unix::fs::symlink(target, link_path)
.or_else(|_| copy_dir_recursive(target, link_path))
.map_err(|e| format!("link {} -> {}: {e}", link_path.display(), target.display()))
.map_err(|e| SkillInstallError::link(link_path, target, e))
}
#[cfg(windows)]
{
std::os::windows::fs::symlink_dir(target, link_path)
.or_else(|_| copy_dir_recursive(target, link_path))
.map_err(|e| format!("link {} -> {}: {e}", link_path.display(), target.display()))
.map_err(|e| SkillInstallError::link(link_path, target, e))
}
}
@ -357,7 +358,7 @@ fn copy_dir_recursive(src: &Path, dest: &Path) -> std::io::Result<()> {
Ok(())
}
fn remove_path(path: &Path) -> Result<(), String> {
fn remove_path(path: &Path) -> Result<()> {
// Only a missing entry is "nothing to remove"; any other metadata error
// (permissions, I/O) must propagate — treating it as absence would let a
// later create step fail with a misleading error, or silently keep a
@ -365,62 +366,64 @@ fn remove_path(path: &Path) -> Result<(), String> {
let metadata = match fs::symlink_metadata(path) {
Ok(metadata) => metadata,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
Err(e) => return Err(format!("inspect {}: {e}", path.display())),
Err(e) => return Err(SkillInstallError::fs(FsAction::Inspect, path, e)),
};
if metadata.file_type().is_symlink() {
remove_symlink_path(path)
} else if metadata.is_file() {
fs::remove_file(path).map_err(|e| format!("remove {}: {e}", path.display()))
fs::remove_file(path).map_err(|e| SkillInstallError::fs(FsAction::Remove, path, e))
} else {
fs::remove_dir_all(path).map_err(|e| format!("remove {}: {e}", path.display()))
fs::remove_dir_all(path).map_err(|e| SkillInstallError::fs(FsAction::Remove, path, e))
}
}
#[cfg(windows)]
fn remove_symlink_path(path: &Path) -> Result<(), String> {
fn remove_symlink_path(path: &Path) -> Result<()> {
// Windows removes directory symlinks via remove_dir and file symlinks via
// remove_file. `is_dir()` follows the target, so a DANGLING directory
// symlink reports false — try both forms instead of classifying.
fs::remove_dir(path)
.or_else(|_| fs::remove_file(path))
.map_err(|e| format!("remove {}: {e}", path.display()))
.map_err(|e| SkillInstallError::fs(FsAction::Remove, path, e))
}
#[cfg(not(windows))]
fn remove_symlink_path(path: &Path) -> Result<(), String> {
fs::remove_file(path).map_err(|e| format!("remove {}: {e}", path.display()))
fn remove_symlink_path(path: &Path) -> Result<()> {
fs::remove_file(path).map_err(|e| SkillInstallError::fs(FsAction::Remove, path, e))
}
fn read_json_object(path: &Path) -> Result<Map<String, Value>, String> {
fn read_json_object(path: &Path) -> Result<Map<String, Value>> {
let text = match fs::read_to_string(path) {
Ok(text) => text,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Map::new()),
Err(e) => return Err(format!("read {}: {e}", path.display())),
Err(e) => return Err(SkillInstallError::fs(FsAction::Read, path, e)),
};
if text.trim().is_empty() {
return Ok(Map::new());
}
let value: Value =
serde_json::from_str(&text).map_err(|e| format!("parse {}: {e}", path.display()))?;
serde_json::from_str(&text).map_err(|e| SkillInstallError::fs(FsAction::Parse, path, e))?;
value
.as_object()
.cloned()
.ok_or_else(|| format!("{} must contain a JSON object", path.display()))
.ok_or_else(|| SkillInstallError::NotAJsonObject(path.to_path_buf()))
}
fn write_json_object(path: &Path, root: &Map<String, Value>) -> Result<(), String> {
fn write_json_object(path: &Path, root: &Map<String, Value>) -> Result<()> {
if let Some(parent) = path.parent() {
fs::create_dir_all(parent).map_err(|e| format!("create {}: {e}", parent.display()))?;
fs::create_dir_all(parent)
.map_err(|e| SkillInstallError::fs(FsAction::Create, parent, e))?;
}
let text = serde_json::to_string_pretty(root)
.map_err(|e| format!("serialize {}: {e}", path.display()))?;
fs::write(path, format!("{text}\n")).map_err(|e| format!("write {}: {e}", path.display()))
.map_err(|e| SkillInstallError::fs(FsAction::Serialize, path, e))?;
fs::write(path, format!("{text}\n"))
.map_err(|e| SkillInstallError::fs(FsAction::Write, path, e))
}
fn object_entry<'a>(
root: &'a mut Map<String, Value>,
key: &str,
) -> Result<&'a mut Map<String, Value>, String> {
) -> Result<&'a mut Map<String, Value>> {
let entry = root
.entry(key.to_string())
.or_insert_with(|| Value::Object(Map::new()));
@ -429,7 +432,7 @@ fn object_entry<'a>(
}
entry
.as_object_mut()
.ok_or_else(|| format!("{key} is not an object"))
.ok_or_else(|| SkillInstallError::NotAnObject(key.to_string()))
}
fn array_entry<'a>(root: &'a mut Map<String, Value>, key: &str) -> &'a mut Vec<Value> {
@ -442,11 +445,11 @@ fn array_entry<'a>(root: &'a mut Map<String, Value>, key: &str) -> &'a mut Vec<V
entry.as_array_mut().expect("array value")
}
fn home_dir() -> Result<PathBuf, String> {
fn home_dir() -> Result<PathBuf> {
env::var_os("HOME")
.or_else(|| env::var_os("USERPROFILE"))
.map(PathBuf::from)
.ok_or_else(|| "home directory not available".to_string())
.ok_or(SkillInstallError::HomeUnavailable)
}
fn timestamp_string() -> String {
@ -522,7 +525,8 @@ mod tests {
#[test]
fn bundle_renderer_rejects_missing_version_sentinels() {
let error = render_bundle_template("{}", env!("CARGO_PKG_VERSION"))
.expect_err("template without version sentinels should fail");
.expect_err("template without version sentinels should fail")
.to_string();
let expected = format!("expected {EXPECTED_VERSION_SENTINEL_COUNT}");
assert!(error.contains(&expected), "unexpected error: {error}");
@ -533,7 +537,8 @@ mod tests {
fn bundle_renderer_rejects_partially_templated_versions() {
let partial_template = BUNDLE_JSON.replacen(VERSION_SENTINEL, env!("CARGO_PKG_VERSION"), 1);
let error = render_bundle_template(&partial_template, env!("CARGO_PKG_VERSION"))
.expect_err("partially rendered template should fail");
.expect_err("partially rendered template should fail")
.to_string();
let expected = format!("expected {EXPECTED_VERSION_SENTINEL_COUNT}");
let found = format!("found {}", EXPECTED_VERSION_SENTINEL_COUNT - 1);
@ -544,7 +549,8 @@ mod tests {
#[test]
fn gemini_cli_is_not_an_install_target() {
let error = super::Target::parse("gemini-cli")
.expect_err("retired Gemini CLI integration must stay unavailable");
.expect_err("retired Gemini CLI integration must stay unavailable")
.to_string();
assert!(
error.contains("unknown target"),

172
crates/op-cli/src/skill_install_error.rs Normal file
View file

@ -0,0 +1,172 @@
//! Typed failures for `op install` / `op uninstall` (see `skill_install_cli`).
//!
//! Same hand-rolled style as `op_orchestrator::OrchestratorError` — a plain
//! enum with a `Display` impl, no `thiserror`. Every variant reproduces the
//! exact sentence the stringly-typed version produced, so the JSON `error`
//! field each per-target result carries is unchanged.
use std::fmt;
use std::path::PathBuf;
/// The filesystem verb that failed. Doubles as the message prefix, which is
/// why the messages read `create <path>: <io error>` etc.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum FsAction {
Create,
Write,
Read,
Remove,
Inspect,
Parse,
Serialize,
}
impl FsAction {
fn label(self) -> &'static str {
match self {
FsAction::Create => "create",
FsAction::Write => "write",
FsAction::Read => "read",
FsAction::Remove => "remove",
FsAction::Inspect => "inspect",
FsAction::Parse => "parse",
FsAction::Serialize => "serialize",
}
}
}
/// Something is wrong with the skill bundle compiled into the binary — a
/// build-time packaging fault, never a user mistake.
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum BundleError {
/// The template did not carry the expected number of version sentinels.
SentinelCount { expected: usize, found: usize },
/// A sentinel survived rendering (so a shipped file would advertise the
/// placeholder as its version).
SentinelRemains,
/// The rendered bundle is not valid JSON.
Parse(String),
/// A required top-level field (`version` / `files`) is absent.
MissingField(&'static str),
/// The bundle carries no files at all.
Empty,
/// A bundle file entry is not a JSON string.
FileNotString(String),
}
impl fmt::Display for BundleError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
BundleError::SentinelCount { expected, found } => write!(
f,
"embedded skill bundle template expected {expected} version sentinels {:?}, \
found {found}",
crate::skill_install_cli::VERSION_SENTINEL
),
BundleError::SentinelRemains => f.write_str(
"embedded skill bundle still contains the version sentinel after rendering",
),
BundleError::Parse(e) => write!(f, "parse skill bundle: {e}"),
BundleError::MissingField(field) => write!(f, "skill bundle missing {field}"),
BundleError::Empty => f.write_str("embedded skill bundle is empty"),
BundleError::FileNotString(path) => write!(f, "bundle file {path:?} is not a string"),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum SkillInstallError {
/// `--target <name>` named an agent this build does not support.
UnknownTarget(String),
/// Nothing to install into and no `--target` to disambiguate.
NoTargetsDetected,
/// Neither `$HOME` nor `%USERPROFILE%` is set.
HomeUnavailable,
/// A plain filesystem / serde operation against `path` failed.
Fs {
action: FsAction,
path: PathBuf,
detail: String,
},
/// Neither symlinking nor copying could create the discovery entry.
Link {
link: PathBuf,
target: PathBuf,
detail: String,
},
/// An agent config file exists but its root is not a JSON object.
NotAJsonObject(PathBuf),
/// A config key that must hold an object holds something else.
NotAnObject(String),
/// The bundle compiled into this binary is unusable.
Bundle(BundleError),
}
impl SkillInstallError {
pub(crate) fn fs(
action: FsAction,
path: impl Into<PathBuf>,
detail: impl fmt::Display,
) -> Self {
SkillInstallError::Fs {
action,
path: path.into(),
detail: detail.to_string(),
}
}
pub(crate) fn link(
link: impl Into<PathBuf>,
target: impl Into<PathBuf>,
detail: impl fmt::Display,
) -> Self {
SkillInstallError::Link {
link: link.into(),
target: target.into(),
detail: detail.to_string(),
}
}
}
impl fmt::Display for SkillInstallError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
SkillInstallError::UnknownTarget(raw) => write!(
f,
"unknown target {raw:?}; available: claude, codex, cursor, opencode"
),
SkillInstallError::NoTargetsDetected => f.write_str(
"no supported AI coding agents detected; pass --target claude|codex|cursor|opencode",
),
SkillInstallError::HomeUnavailable => f.write_str("home directory not available"),
SkillInstallError::Fs {
action,
path,
detail,
} => write!(f, "{} {}: {detail}", action.label(), path.display()),
SkillInstallError::Link {
link,
target,
detail,
} => write!(
f,
"link {} -> {}: {detail}",
link.display(),
target.display()
),
SkillInstallError::NotAJsonObject(path) => {
write!(f, "{} must contain a JSON object", path.display())
}
SkillInstallError::NotAnObject(key) => write!(f, "{key} is not an object"),
SkillInstallError::Bundle(e) => write!(f, "{e}"),
}
}
}
impl std::error::Error for SkillInstallError {}
impl From<BundleError> for SkillInstallError {
fn from(error: BundleError) -> Self {
SkillInstallError::Bundle(error)
}
}

1
crates/op-host-services/src/lib.rs
View file

@ -75,6 +75,7 @@ pub mod settings_io;
pub mod validation_providers;
pub(crate) mod web_auth;
pub mod web_canvas_server;
mod web_canvas_server_error;
pub mod web_chat_standard;
pub mod web_credential_policy;
pub mod web_credentials;

318
crates/op-host-services/src/web_canvas_server.rs
View file

@ -28,8 +28,12 @@ use base64::Engine as _;
use op_editor_core::agent_settings::{AcpAgentConnectOutcome, ProviderConnectOutcome};
use op_editor_core::{AgentSettings, EditorState};
use crate::web_canvas_server_error::WebCanvasError;
use crate::web_credential_policy::WebCredentialPersistence;
/// Every fallible step of this module fails with [`WebCanvasError`].
type Result<T> = std::result::Result<T, WebCanvasError>;
/// Slow/stalled-peer bound — bodies can be large (whole documents with embedded
/// images), so a connection that opens and dribbles must not pin a thread.
const IO_TIMEOUT: Duration = Duration::from_secs(30);
@ -188,9 +192,10 @@ impl WebCanvasState {
/// Clear the transient web-sync document back to the same starter document a
/// fresh browser shell paints before the daemon applies updates.
pub(crate) fn reset_document(&mut self) -> Result<u64, String> {
pub(crate) fn reset_document(&mut self) -> Result<u64> {
if let Some(path) = self.current_path.clone() {
let mut next = crate::mcp_serve::load_editor_state(&path)?;
let mut next =
crate::mcp_serve::load_editor_state(&path).map_err(WebCanvasError::Document)?;
preserve_web_canvas_preferences(&self.editor, &mut next);
set_file_name_display(&mut next, &path);
self.editor = next;
@ -209,7 +214,7 @@ impl WebCanvasState {
/// retryable, and the document/version are left exactly as
/// `reset_document`'s own `?`-early-return already guarantees (nothing
/// touched before the fallible load succeeds).
pub(crate) fn reset_document_guarded(&mut self) -> Result<ResetOutcome, String> {
pub(crate) fn reset_document_guarded(&mut self) -> Result<ResetOutcome> {
if self.reset_consumed {
return Ok(ResetOutcome { skipped: true });
}
@ -233,8 +238,9 @@ impl WebCanvasState {
&mut self,
body: &str,
base_version_override: Option<u64>,
) -> Result<PushOutcome, String> {
let request = crate::mcp_serve::parse_document_sync_request(body)?;
) -> Result<PushOutcome> {
let request = crate::mcp_serve::parse_document_sync_request(body)
.map_err(WebCanvasError::BadRequest)?;
let base_version = base_version_override.or(request.base_version);
if let Some(expected) = base_version {
if expected != self.version {
@ -258,8 +264,8 @@ impl WebCanvasState {
}
// Load via the same proven path as desktop file-open. A load failure
// is a client fault → 400, like the TS validation 400s.
let loaded =
op_pen_loader::load_canonical(request.document_json).map_err(|e| e.to_string())?;
let loaded = op_pen_loader::load_canonical(request.document_json)
.map_err(|e| WebCanvasError::Document(e.to_string()))?;
for w in &loaded.warnings {
eprintln!("openpencil-desktop --serve-web: schema warning: {w:?}");
}
@ -306,7 +312,9 @@ fn persist_api_settings<F>(
save_credentials: F,
) -> WebReply
where
F: FnOnce(&EditorState) -> Result<(), String>,
// `settings_io::save_checked` is outside this conversion's scope and still
// reports `String`; only its success/failure is consulted here.
F: FnOnce(&EditorState) -> std::result::Result<(), String>,
{
if method == "POST" && path == "/api/settings/credentials" && reply.status == "200 OK" {
if save_credentials(&state.editor).is_err() {
@ -396,9 +404,9 @@ pub fn handle_web_canvas_request(
})
.to_string(),
},
Err(message) => WebReply {
status: "400 Bad Request",
body: crate::mcp_serve::rest_error_body(&message),
Err(error) => WebReply {
status: error.http_status(),
body: crate::mcp_serve::rest_error_body(&error.to_string()),
},
},
("GET", "/api/mcp/version") => WebReply {
@ -424,7 +432,7 @@ pub fn handle_web_canvas_request(
body: crate::mcp_serve::document_sync_ok(state.version),
},
Err(e) => WebReply {
status: "400 Bad Request",
status: e.http_status(),
body: crate::mcp_serve::rest_error_body(&format!("sync reset failed: {e}")),
},
},
@ -539,13 +547,13 @@ fn export_raster_download(body: &str, state: &WebCanvasState) -> WebReply {
.to_string(),
},
Err(e) => WebReply {
status: "400 Bad Request",
status: e.http_status(),
body: crate::mcp_serve::rest_error_body(&format!("export raster failed: {e}")),
},
}
}
fn build_raster_download(body: &str, fallback: &EditorState) -> Result<RasterDownload, String> {
fn build_raster_download(body: &str, fallback: &EditorState) -> Result<RasterDownload> {
let parsed = parse_export_body(body)?;
let editor = export_editor_from_value(parsed.as_ref(), fallback)?;
let (format, file_name, mime, ext) = raster_format_from_export_body(parsed.as_ref())?;
@ -564,9 +572,9 @@ fn build_raster_download(body: &str, fallback: &EditorState) -> Result<RasterDow
};
if let Err(e) = result {
let _ = std::fs::remove_file(&tmp);
return Err(e);
return Err(WebCanvasError::Export(e));
}
let bytes = std::fs::read(&tmp).map_err(|e| e.to_string())?;
let bytes = std::fs::read(&tmp).map_err(|e| WebCanvasError::Io(e.to_string()))?;
let _ = std::fs::remove_file(&tmp);
Ok(RasterDownload {
file_name,
@ -577,15 +585,12 @@ fn build_raster_download(body: &str, fallback: &EditorState) -> Result<RasterDow
fn raster_format_from_export_body(
body: Option<&serde_json::Value>,
) -> Result<
(
crate::export::RasterFormat,
&'static str,
&'static str,
&'static str,
),
String,
> {
) -> Result<(
crate::export::RasterFormat,
&'static str,
&'static str,
&'static str,
)> {
let format = body
.and_then(|body| body.get("format"))
.and_then(|format| format.as_str())
@ -609,7 +614,9 @@ fn raster_format_from_export_body(
"image/webp",
"webp",
)),
other => Err(format!("unsupported raster format: {other}")),
other => Err(WebCanvasError::BadRequest(format!(
"unsupported raster format: {other}"
))),
}
}
@ -643,23 +650,23 @@ fn export_pdf_download(body: &str, state: &WebCanvasState) -> WebReply {
.to_string(),
},
Err(e) => WebReply {
status: "400 Bad Request",
status: e.http_status(),
body: crate::mcp_serve::rest_error_body(&format!("export PDF failed: {e}")),
},
}
}
fn build_pdf_download(body: &str, fallback: &EditorState) -> Result<Vec<u8>, String> {
fn build_pdf_download(body: &str, fallback: &EditorState) -> Result<Vec<u8>> {
let editor = export_editor_from_body(body, fallback)?;
let scene = op_pen_loader::editor_state_to_layout_scene(&editor);
let tmp = tmp_export_path("pdf");
crate::export_pdf::export_pdf(&scene, &tmp)?;
let bytes = std::fs::read(&tmp).map_err(|e| e.to_string())?;
crate::export_pdf::export_pdf(&scene, &tmp).map_err(WebCanvasError::Export)?;
let bytes = std::fs::read(&tmp).map_err(|e| WebCanvasError::Io(e.to_string()))?;
let _ = std::fs::remove_file(&tmp);
Ok(bytes)
}
fn export_editor_from_body(body: &str, fallback: &EditorState) -> Result<EditorState, String> {
fn export_editor_from_body(body: &str, fallback: &EditorState) -> Result<EditorState> {
let parsed = parse_export_body(body)?;
export_editor_from_value(parsed.as_ref(), fallback)
}
@ -667,16 +674,19 @@ fn export_editor_from_body(body: &str, fallback: &EditorState) -> Result<EditorS
fn export_editor_from_value(
body: Option<&serde_json::Value>,
fallback: &EditorState,
) -> Result<EditorState, String> {
) -> Result<EditorState> {
let Some(doc) = body.and_then(|body| body.get("document")) else {
return Ok(fallback.clone());
};
if !doc.is_object() {
return Err("document must be an object".into());
return Err(WebCanvasError::BadRequest(
"document must be an object".into(),
));
}
let src = serde_json::to_string(doc).map_err(|e| e.to_string())?;
let src = serde_json::to_string(doc).map_err(|e| WebCanvasError::BadRequest(e.to_string()))?;
let editor_meta = op_pen_loader::extract_editor_meta(&src);
let loaded = op_pen_loader::load_canonical(&src).map_err(|e| e.to_string())?;
let loaded =
op_pen_loader::load_canonical(&src).map_err(|e| WebCanvasError::Document(e.to_string()))?;
let mut editor = EditorState::from_document(loaded.value);
if let Some(meta) = editor_meta {
op_pen_loader::apply_editor_meta(&mut editor, meta);
@ -698,13 +708,13 @@ fn export_editor_from_value(
Ok(editor)
}
fn parse_export_body(body: &str) -> Result<Option<serde_json::Value>, String> {
fn parse_export_body(body: &str) -> Result<Option<serde_json::Value>> {
if body.trim().is_empty() {
return Ok(None);
}
serde_json::from_str(body)
.map(Some)
.map_err(|e| format!("parse request body: {e}"))
.map_err(|e| WebCanvasError::BadRequest(format!("parse request body: {e}")))
}
fn tmp_export_path(ext: &str) -> PathBuf {
@ -741,7 +751,7 @@ fn save_current_file(body: &str, state: &mut WebCanvasState) -> WebReply {
}
}
Err(e) => WebReply {
status: "400 Bad Request",
status: e.http_status(),
body: crate::mcp_serve::rest_error_body(&format!("save failed: {e}")),
},
}
@ -751,7 +761,7 @@ fn save_editor_from_body(
body: &str,
previous: &EditorState,
path: &std::path::Path,
) -> Result<EditorState, String> {
) -> Result<EditorState> {
let (doc, active_page_index, editor_meta) = document_and_active_page_from_body(body)?;
let mut next = previous.clone();
next.replace_document(doc);
@ -769,30 +779,30 @@ fn save_editor_from_body(
next.ui.active_page_index = index.min(page_count - 1);
}
set_file_name_display(&mut next, path);
crate::doc_io::save_to_path(&next, path)?;
crate::doc_io::save_to_path(&next, path).map_err(WebCanvasError::Io)?;
Ok(next)
}
fn document_and_active_page_from_body(
body: &str,
) -> Result<
(
jian_ops_schema::PenDocument,
Option<usize>,
Option<op_pen_loader::EditorMeta>,
),
String,
> {
let parsed =
crate::mcp_serve::parse_borrowed_document_envelope(body).map_err(|e| e.to_string())?;
) -> Result<(
jian_ops_schema::PenDocument,
Option<usize>,
Option<op_pen_loader::EditorMeta>,
)> {
let parsed = crate::mcp_serve::parse_borrowed_document_envelope(body)
.map_err(|e| WebCanvasError::BadRequest(e.to_string()))?;
let Some(doc_json) = parsed.document_json else {
return Err("missing document".into());
return Err(WebCanvasError::BadRequest("missing document".into()));
};
if !doc_json.trim_start().starts_with('{') {
return Err("document must be an object".into());
return Err(WebCanvasError::BadRequest(
"document must be an object".into(),
));
}
let editor_meta = op_pen_loader::extract_editor_meta(doc_json);
let loaded = op_pen_loader::load_canonical(doc_json).map_err(|e| e.to_string())?;
let loaded = op_pen_loader::load_canonical(doc_json)
.map_err(|e| WebCanvasError::Document(e.to_string()))?;
for w in &loaded.warnings {
eprintln!("openpencil-desktop --serve-web: schema warning: {w:?}");
}
@ -1269,22 +1279,33 @@ pub struct ServeWebOptions {
/// The host defaults to loopback; `--host 0.0.0.0` is the LAN/Docker opt-in
/// (no TLS — deploy behind a proxy for anything beyond a trusted network).
pub fn parse_serve_web_args<I: Iterator<Item = String>>(
mut args: I,
) -> Result<ServeWebOptions, String> {
args: I,
) -> std::result::Result<ServeWebOptions, String> {
// Public entry point consumed by `cli_modes.rs` and the host binaries,
// which are outside this conversion's scope — keep the `String` contract
// and adapt the typed error here rather than rippling outward.
parse_serve_web_args_typed(args).map_err(|e| e.to_string())
}
fn parse_serve_web_args_typed<I: Iterator<Item = String>>(mut args: I) -> Result<ServeWebOptions> {
let Some(first) = args.next() else {
return Err("missing <port> arg".into());
return Err(WebCanvasError::Config("missing <port> arg".into()));
};
if first.starts_with("--") {
return parse_serve_web_args_managed(first, args);
}
let Ok(port) = first.parse::<u16>() else {
return Err(format!("<port> must be a u16, got {first:?}"));
return Err(WebCanvasError::Config(format!(
"<port> must be a u16, got {first:?}"
)));
};
let mut path: Option<PathBuf> = None;
let mut host = "127.0.0.1".to_string();
while let Some(arg) = args.next() {
if arg == "--host" {
host = args.next().ok_or("--host needs a value (e.g. 0.0.0.0)")?;
host = args.next().ok_or_else(|| {
WebCanvasError::Config("--host needs a value (e.g. 0.0.0.0)".into())
})?;
} else if let Some(value) = arg.strip_prefix("--host=") {
host = value.to_string();
} else if path.is_none() {
@ -1292,11 +1313,11 @@ pub fn parse_serve_web_args<I: Iterator<Item = String>>(
// from the same starter document the web shell paints locally.
path = Some(PathBuf::from(arg));
} else {
return Err(format!("unexpected arg {arg:?}"));
return Err(WebCanvasError::Config(format!("unexpected arg {arg:?}")));
}
}
if host.is_empty() {
return Err("--host must not be empty".into());
return Err(WebCanvasError::Config("--host must not be empty".into()));
}
Ok(ServeWebOptions {
port,
@ -1315,7 +1336,7 @@ pub fn parse_serve_web_args<I: Iterator<Item = String>>(
fn parse_serve_web_args_managed<I: Iterator<Item = String>>(
first_flag: String,
mut args: I,
) -> Result<ServeWebOptions, String> {
) -> Result<ServeWebOptions> {
let mut managed = false;
let mut port: Option<u16> = None;
let mut path: Option<PathBuf> = None;
@ -1326,30 +1347,36 @@ fn parse_serve_web_args_managed<I: Iterator<Item = String>>(
match arg.as_str() {
"--managed" => managed = true,
"--port" => {
let value = args.next().ok_or("--port needs a value")?;
port = Some(
value
.parse::<u16>()
.map_err(|_| format!("--port must be a u16, got {value:?}"))?,
);
let value = args
.next()
.ok_or_else(|| WebCanvasError::Config("--port needs a value".into()))?;
port = Some(value.parse::<u16>().map_err(|_| {
WebCanvasError::Config(format!("--port must be a u16, got {value:?}"))
})?);
}
"--file" => {
path = Some(PathBuf::from(args.next().ok_or("--file needs a value")?));
path = Some(PathBuf::from(args.next().ok_or_else(|| {
WebCanvasError::Config("--file needs a value".into())
})?));
}
"--host" => {
host = args.next().ok_or("--host needs a value (e.g. 0.0.0.0)")?;
host = args.next().ok_or_else(|| {
WebCanvasError::Config("--host needs a value (e.g. 0.0.0.0)".into())
})?;
}
"--allow-origin" => {
allow_origins.push(args.next().ok_or("--allow-origin needs a value")?);
allow_origins.push(args.next().ok_or_else(|| {
WebCanvasError::Config("--allow-origin needs a value".into())
})?);
}
other => return Err(format!("unexpected arg {other:?}")),
other => return Err(WebCanvasError::Config(format!("unexpected arg {other:?}"))),
}
}
let Some(port) = port else {
return Err("missing --port <n>".into());
return Err(WebCanvasError::Config("missing --port <n>".into()));
};
if host.is_empty() {
return Err("--host must not be empty".into());
return Err(WebCanvasError::Config("--host must not be empty".into()));
}
Ok(ServeWebOptions {
port,
@ -1397,10 +1424,11 @@ fn random_token() -> String {
fn startup_editor_from_base_for_web_canvas(
base: EditorState,
path: Option<PathBuf>,
) -> Result<EditorState, String> {
) -> Result<EditorState> {
match path {
Some(p) => {
let mut next = crate::mcp_serve::load_editor_state(&p)?;
let mut next =
crate::mcp_serve::load_editor_state(&p).map_err(WebCanvasError::Document)?;
preserve_web_canvas_preferences(&base, &mut next);
set_file_name_display(&mut next, &p);
next.editor_ui.touch_recent_file(
@ -1420,24 +1448,31 @@ fn startup_editor_for_web_canvas_with_loader<Checked>(
path: Option<PathBuf>,
_policy: WebCredentialPersistence,
checked_load: Checked,
) -> Result<EditorState, String>
) -> Result<EditorState>
where
Checked: FnOnce(&mut EditorState) -> Result<(), String>,
// `settings_io::load_checked` is outside this conversion's scope and
// still reports `String`; keep its shape and adapt at the call.
Checked: FnOnce(&mut EditorState) -> std::result::Result<(), String>,
{
let mut base = EditorState::starter();
checked_load(&mut base)?;
checked_load(&mut base).map_err(WebCanvasError::Config)?;
startup_editor_from_base_for_web_canvas(base, path)
}
fn startup_editor_for_web_canvas_with_policy(
path: Option<PathBuf>,
policy: WebCredentialPersistence,
) -> Result<EditorState, String> {
) -> Result<EditorState> {
startup_editor_for_web_canvas_with_loader(path, policy, crate::settings_io::load_checked)
}
pub fn startup_editor_for_web_canvas(path: Option<PathBuf>) -> Result<EditorState, String> {
/// Public entry point (host binaries) — keeps the `String` contract and
/// adapts the typed error at the boundary.
pub fn startup_editor_for_web_canvas(
path: Option<PathBuf>,
) -> std::result::Result<EditorState, String> {
startup_editor_for_web_canvas_with_policy(path, crate::web_credential_policy::from_env())
.map_err(|e| e.to_string())
}
/// Run the web-canvas daemon per `options` (host/port default `127.0.0.1`),
@ -1457,7 +1492,12 @@ pub fn startup_editor_for_web_canvas(path: Option<PathBuf>) -> Result<EditorStat
/// path uses, waking the accept loop by connecting back to the bound
/// address. Non-managed mode is untouched: no token, no handshake output, no
/// stdin thread.
pub fn run_web_canvas(options: ServeWebOptions) -> Result<(), String> {
pub fn run_web_canvas(options: ServeWebOptions) -> std::result::Result<(), String> {
// Public entry point (`cli_modes.rs`) — `String` contract preserved.
run_web_canvas_typed(options).map_err(|e| e.to_string())
}
fn run_web_canvas_typed(options: ServeWebOptions) -> Result<()> {
let ServeWebOptions {
port,
path,
@ -1479,9 +1519,11 @@ pub fn run_web_canvas(options: ServeWebOptions) -> Result<(), String> {
// to the daemon owner, not to whoever can reach the port.
let loopback_bind = matches!(host.as_str(), "127.0.0.1" | "localhost" | "::1");
crate::web_auth::init(&mut editor, managed || loopback_bind);
let listener =
TcpListener::bind((host.as_str(), port)).map_err(|e| format!("bind {host}:{port}: {e}"))?;
let local_addr = listener.local_addr().map_err(|e| e.to_string())?;
let listener = TcpListener::bind((host.as_str(), port))
.map_err(|e| WebCanvasError::Config(format!("bind {host}:{port}: {e}")))?;
let local_addr = listener
.local_addr()
.map_err(|e| WebCanvasError::Config(e.to_string()))?;
let bound = local_addr.port();
eprintln!("openpencil-desktop --serve-web: listening on {host}:{bound}");
match crate::web_static::resolve_bundle_dir() {
@ -1521,17 +1563,36 @@ pub fn run_web_canvas(options: ServeWebOptions) -> Result<(), String> {
let _ = out.flush();
drop(out);
let shutdown_stdin = Arc::clone(&shutdown);
std::thread::spawn(move || {
let mut sink = [0u8; 64];
let mut stdin = std::io::stdin();
while matches!(stdin.read(&mut sink), Ok(n) if n > 0) {}
shutdown_stdin.store(true, Ordering::Release);
// Wake the (possibly blocked) accept loop — reconnect to the
// bound address exactly (works for IPv6 / custom --host, unlike
// the loopback-only wake used by the token-authed shutdown
// path below).
let _ = std::net::TcpStream::connect(local_addr);
});
// Detached on purpose — there is NO portable way to cancel a thread
// parked in a blocking `Stdin::read`. A channel or flag can only be
// observed between reads, and putting fd 0 into non-blocking mode
// would need platform `fcntl`/`SetNamedPipeHandleState` calls (a new
// dependency or unsafe per-OS code) AND would change what "EOF" means
// for the parent-death lease, which is this thread's whole purpose.
// So the exit path is: (a) the parent closes stdin — the loop ends and
// raises `shutdown` itself, or (b) some other path raised `shutdown`
// first, in which case the checks below make this thread a no-op and
// the process exit reaps it. The flag check per iteration is what
// makes (b) prompt rather than "whenever the parent next writes".
let _ = std::thread::Builder::new()
.name("op-serve-web-stdin".into())
.spawn(move || {
let mut sink = [0u8; 64];
let mut stdin = std::io::stdin();
while !shutdown_stdin.load(Ordering::Acquire)
&& matches!(stdin.read(&mut sink), Ok(n) if n > 0)
{}
// Only raise + wake when nobody else already shut the daemon
// down; a redundant wake connect against an already-closed
// listener is harmless but pointlessly noisy.
if !shutdown_stdin.swap(true, Ordering::AcqRel) {
// Wake the (possibly blocked) accept loop — reconnect to
// the bound address exactly (works for IPv6 / custom
// --host, unlike the loopback-only wake used by the
// token-authed shutdown path below).
let _ = std::net::TcpStream::connect(local_addr);
}
});
}
// Stash the managed token + allow-origins on the shared state. `serve_one`
// reads them via `RequestAuth` gate (token) and `cors_origin_for` (CORS
@ -1596,16 +1657,20 @@ fn enforce_credential_persistence_policy<F>(
editor: &mut EditorState,
policy: WebCredentialPersistence,
save: F,
) -> Result<(), String>
) -> Result<()>
where
F: FnOnce(&EditorState) -> Result<(), String>,
// `settings_io::save_checked`'s `String` shape is preserved (unowned);
// only the outcome is retyped.
F: FnOnce(&EditorState) -> std::result::Result<(), String>,
{
if !policy.server_persistence()
&& crate::web_credentials::remove_browser_owned_credentials(editor)
{
save(editor).map_err(|_| {
"failed to remove browser-owned credentials while server persistence is disabled"
.to_string()
WebCanvasError::Config(
"failed to remove browser-owned credentials while server persistence is disabled"
.into(),
)
})?;
}
Ok(())
@ -1678,8 +1743,8 @@ fn serve_one<S: Read + Write>(
stream: &mut S,
state: &Mutex<WebCanvasState>,
hub: &SseHub,
) -> Result<bool, String> {
let req = crate::mcp_serve::read_http_request(stream)?;
) -> Result<bool> {
let req = crate::mcp_serve::read_http_request(stream).map_err(WebCanvasError::Transport)?;
let (auth, allow_origins) = {
let guard = state.lock().unwrap_or_else(|p| p.into_inner());
let auth = RequestAuth {
@ -1701,6 +1766,7 @@ fn serve_one<S: Read + Write>(
"",
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
if is_sensitive_browser_post(&req) && !credential_request_origin_allowed(&req) {
@ -1710,6 +1776,7 @@ fn serve_one<S: Read + Write>(
&crate::mcp_serve::rest_error_body("cross-origin sensitive request is forbidden"),
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Sensitive JSON routes refuse CORS "simple request" content types
@ -1724,6 +1791,7 @@ fn serve_one<S: Read + Write>(
),
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Static serving: the host page (`/`) and the wasm-bindgen bundle
@ -1734,6 +1802,7 @@ fn serve_one<S: Read + Write>(
crate::web_static::handle_static_request(&req.path, bundle_dir.as_deref())
{
return crate::web_static::write_static_response(stream, &reply, cors_origin)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
}
@ -1746,6 +1815,7 @@ fn serve_one<S: Read + Write>(
body: crate::web_auth::LOADING_PAGE_HTML.as_bytes().to_vec(),
};
return crate::web_static::write_static_response(stream, &reply, cors_origin)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Managed-mode token gate: everything below this point is a privileged
@ -1760,6 +1830,7 @@ fn serve_one<S: Read + Write>(
r#"{"ok":false,"error":"unauthorized"}"#,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Device-login begin: waits (per-connection thread, off the state
@ -1774,6 +1845,7 @@ fn serve_one<S: Read + Write>(
&reply.body,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// SSE live-update stream: the browser shell subscribes and re-syncs whenever
@ -1795,7 +1867,7 @@ fn serve_one<S: Read + Write>(
if req.method == "POST" && req.path == "/api/ai/stream" {
let Some(ai_req) = crate::ai_proxy::parse_ai_stream_body(&req.body) else {
return crate::ai_proxy::write_sse_error(stream, "invalid request body", cors_origin)
.map_err(|e| format!("ai stream error: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai stream error: {e}")))
.map(|()| false);
};
let provider = {
@ -1814,17 +1886,17 @@ fn serve_one<S: Read + Write>(
"no model configured",
cors_origin,
)
.map_err(|e| format!("ai stream error: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai stream error: {e}")))
.map(|()| false);
}
Err(message) => {
return crate::ai_proxy::write_sse_error(stream, &message, cors_origin)
.map_err(|e| format!("ai stream error: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai stream error: {e}")))
.map(|()| false);
}
};
return crate::ai_proxy::stream_ai_response(stream, ai_req, provider.as_ref(), cors_origin)
.map_err(|e| format!("ai stream: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai stream: {e}")))
.map(|()| false);
}
// Standard web chat/design turn: same external-CLI routing shape as
@ -1834,7 +1906,7 @@ fn serve_one<S: Read + Write>(
let Some(standard_req) = crate::web_chat_standard::parse_standard_turn_body(&req.body)
else {
return crate::ai_proxy::write_sse_error(stream, "invalid request body", cors_origin)
.map_err(|e| format!("ai standard error: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai standard error: {e}")))
.map(|()| false);
};
return crate::web_chat_standard::stream_standard_turn(
@ -1844,7 +1916,7 @@ fn serve_one<S: Read + Write>(
hub,
cors_origin,
)
.map_err(|e| format!("ai standard: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("ai standard: {e}")))
.map(|()| false);
}
// Image panel Search popover (desktop `image_panel_host` parity). Long
@ -1891,6 +1963,7 @@ fn serve_one<S: Read + Write>(
&body,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Image panel Generate popover (desktop `image_generate_host` parity).
@ -1927,6 +2000,7 @@ fn serve_one<S: Read + Write>(
&body,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Offline `.fig` -> `.op` convert for the VS Code plugin: it can't parse
@ -1949,6 +2023,7 @@ fn serve_one<S: Read + Write>(
&body,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// All `/api/mcp/*` REST paths go to the REST handler — including ones this
@ -1988,6 +2063,7 @@ fn serve_one<S: Read + Write>(
&reply.body,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// JSON-RPC tool dispatch is served ONLY as a POST to `/` or `/mcp`. An
@ -2001,6 +2077,7 @@ fn serve_one<S: Read + Write>(
r#"{"ok":false,"error":"Not found. Use /, /pkg/*, /api/mcp/document, /api/mcp/sync-reset, /api/mcp/server, /api/mcp/events, /api/file/save, /api/export/raster, /api/export/pdf, or /mcp."}"#,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
if req.method != "POST" {
@ -2010,6 +2087,7 @@ fn serve_one<S: Read + Write>(
r#"{"ok":false,"error":"Method not allowed. POST a JSON-RPC message to /mcp."}"#,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// Token-authed graceful shutdown (`op stop`): same contract as the
@ -2025,7 +2103,8 @@ fn serve_one<S: Read + Write>(
"200 OK",
&crate::mcp_serve::shutdown_ok_response(&id),
cors_origin,
)?;
)
.map_err(WebCanvasError::Transport)?;
return Ok(true);
}
// `debug_screenshot` for `--serve-web`: the browser shell mirrors this
@ -2051,6 +2130,7 @@ fn serve_one<S: Read + Write>(
&response,
cors_origin,
)
.map_err(WebCanvasError::Transport)
.map(|()| false);
}
// JSON-RPC `/mcp` dispatch against the in-memory document. A mutating apply
@ -2075,7 +2155,8 @@ fn serve_one<S: Read + Write>(
applied_any |= ok;
ok
},
)?
)
.map_err(WebCanvasError::BadRequest)?
.unwrap_or_default();
if applied_any {
guard.version += 1;
@ -2093,6 +2174,7 @@ fn serve_one<S: Read + Write>(
"200 OK"
};
crate::mcp_serve::write_mcp_http_response_with_origin(stream, status, &response, cors_origin)
.map_err(WebCanvasError::Transport)
.map(|()| false)
}
@ -2218,7 +2300,7 @@ fn serve_sse<S: Write>(
rx: Receiver<u64>,
current_version: u64,
cors_origin: Option<&str>,
) -> Result<(), String> {
) -> Result<()> {
let cors_line = cors_origin
.map(|origin| format!("Access-Control-Allow-Origin: {origin}\r\n"))
.unwrap_or_default();
@ -2231,7 +2313,7 @@ fn serve_sse<S: Write>(
);
stream
.write_all(headers.as_bytes())
.map_err(|e| format!("sse headers: {e}"))?;
.map_err(|e| WebCanvasError::Transport(format!("sse headers: {e}")))?;
write_sse_event(stream, current_version)?;
loop {
match rx.recv_timeout(SSE_HEARTBEAT) {
@ -2250,8 +2332,10 @@ fn serve_sse<S: Write>(
// write here is how we notice it disconnected.
stream
.write_all(b": ping\n\n")
.map_err(|e| format!("sse heartbeat: {e}"))?;
stream.flush().map_err(|e| format!("sse flush: {e}"))?;
.map_err(|e| WebCanvasError::Transport(format!("sse heartbeat: {e}")))?;
stream
.flush()
.map_err(|e| WebCanvasError::Transport(format!("sse flush: {e}")))?;
}
Err(RecvTimeoutError::Disconnected) => return Ok(()),
}
@ -2259,12 +2343,14 @@ fn serve_sse<S: Write>(
}
/// Format + write one SSE `data:` event carrying the document version.
fn write_sse_event<S: Write>(stream: &mut S, version: u64) -> Result<(), String> {
fn write_sse_event<S: Write>(stream: &mut S, version: u64) -> Result<()> {
let event = format!("data: {{\"version\":{version}}}\n\n");
stream
.write_all(event.as_bytes())
.map_err(|e| format!("sse write: {e}"))?;
stream.flush().map_err(|e| format!("sse flush: {e}"))
.map_err(|e| WebCanvasError::Transport(format!("sse write: {e}")))?;
stream
.flush()
.map_err(|e| WebCanvasError::Transport(format!("sse flush: {e}")))
}
#[cfg(test)]

79
crates/op-host-services/src/web_canvas_server_error.rs Normal file
View file

@ -0,0 +1,79 @@
//! Typed failures for the `--serve-web` web-canvas daemon
//! (`web_canvas_server.rs`).
//!
//! Style follows `op_orchestrator::OrchestratorError`: a plain enum plus a
//! hand-written `Display`, no `thiserror` and no new dependency. `Display` is
//! transparent — each variant carries the exact sentence the route already
//! embedded in its JSON error body, so the wire bytes are unchanged.
//!
//! What the enum adds is a route-independent classification plus
//! [`WebCanvasError::http_status`], which turns "which status does this
//! failure answer with" from a per-call-site literal into one table.
//!
//! Several dependencies of this module (`mcp_serve`, `doc_io`, `export`,
//! `settings_io`, `op_pen_loader`) still report `String`; they are outside
//! this conversion's scope, so their errors are adapted into a variant at the
//! call site instead of rippling the change into them. The daemon's three
//! public entry points (`parse_serve_web_args`, `startup_editor_for_web_canvas`,
//! `run_web_canvas`) likewise keep their `Result<_, String>` signatures — they
//! are consumed by `cli_modes.rs` and the host binaries — and convert at the
//! boundary.
use std::fmt;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum WebCanvasError {
/// The request body is malformed, has the wrong shape, or names an
/// unsupported parameter — a client fault.
BadRequest(String),
/// A document failed to load or validate through the canonical loader.
Document(String),
/// The raster / PDF export pipeline failed.
Export(String),
/// A filesystem operation on a request-scoped file (the save target, an
/// export temp file, the backing document) failed.
Io(String),
/// Start-up configuration failed: bad `--serve-web` argv, an unusable
/// settings file, or a socket that would not bind. Never becomes an HTTP
/// response — it aborts daemon start-up.
Config(String),
/// A connection-level read/write failed (request parse, response write,
/// SSE stream). Never becomes an HTTP response — the socket is already
/// unusable; the accept loop just logs it.
Transport(String),
}
impl WebCanvasError {
/// The HTTP status a route answers with when it surfaces this failure.
///
/// The four request-scoped kinds are all client faults — the daemon's
/// in-memory document authority is healthy; what failed is the request's
/// payload or the file it named — so they answer `400`, matching the
/// pre-conversion behaviour of every route in this module exactly.
/// `Config` / `Transport` never reach a response; they report the generic
/// `500` so a future caller that does surface one is not silently wrong.
pub(crate) fn http_status(&self) -> &'static str {
match self {
WebCanvasError::BadRequest(_)
| WebCanvasError::Document(_)
| WebCanvasError::Export(_)
| WebCanvasError::Io(_) => "400 Bad Request",
WebCanvasError::Config(_) | WebCanvasError::Transport(_) => "500 Internal Server Error",
}
}
}
impl fmt::Display for WebCanvasError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
WebCanvasError::BadRequest(m)
| WebCanvasError::Document(m)
| WebCanvasError::Export(m)
| WebCanvasError::Io(m)
| WebCanvasError::Config(m)
| WebCanvasError::Transport(m) => f.write_str(m),
}
}
}
impl std::error::Error for WebCanvasError {}

7
crates/op-host-services/src/web_canvas_server_tests.rs
View file

@ -230,7 +230,8 @@ fn browser_only_startup_propagates_credential_scrub_save_failure() {
crate::web_credential_policy::WebCredentialPersistence::BrowserOnly,
|_| Err("simulated disk failure".into()),
)
.expect_err("startup must fail when scrubbed settings cannot be saved");
.expect_err("startup must fail when scrubbed settings cannot be saved")
.to_string();
assert_eq!(
error,
@ -478,7 +479,9 @@ fn every_web_persistence_policy_propagates_strict_settings_load_failures() {
Err("invalid existing settings".into())
});
let error = result.expect_err("all web policies must fail closed on invalid settings");
let error = result
.expect_err("all web policies must fail closed on invalid settings")
.to_string();
assert_eq!(error, "invalid existing settings");
assert_eq!(checked_calls.get(), 1, "policy={policy:?}");
}

199
crates/op-mcp/src/batch_program.rs
View file

@ -67,8 +67,12 @@ use super::batch_design::{
};
use super::batch_direct_ops::{split_top_level_args, update_command_from_value};
use super::batch_page::optional_page_id;
use super::batch_program_error::ProgramError;
use super::{EditorCommand, ToolOutcome};
/// Every fallible step of the executor fails with [`ProgramError`].
type Result<T> = std::result::Result<T, ProgramError>;
/// Run a mixed multi-op DSL program against the document snapshot and
/// return the TS `handleBatchDesign` envelope:
/// `{ results, nodeCount, postProcessed?, errors? }`.
@ -120,7 +124,7 @@ pub(crate) fn run_batch_design_program(
for (line_index, line) in lines.into_iter().enumerate() {
ctx.current_line = line_index;
if let Err(error) = execute_line(&line, &mut ctx) {
errors.push(json!({ "line": line_preview(&line), "error": error }));
errors.push(json!({ "line": line_preview(&line), "error": error.to_string() }));
}
}
@ -189,9 +193,9 @@ struct ProgramCtx {
impl ProgramCtx {
/// Emit `cmd` AND apply it to the sim. The sim apply is the line's
/// final validation gate — the host will run the same code.
fn emit(&mut self, cmd: EditorCommand, failure: &str) -> Result<(), String> {
fn emit(&mut self, cmd: EditorCommand, failure: &str) -> Result<()> {
if !self.sim.apply(cmd.clone()) {
return Err(failure.to_string());
return Err(ProgramError::ApplyRejected(failure.to_string()));
}
self.commands.push(cmd);
Ok(())
@ -218,14 +222,14 @@ impl ProgramCtx {
/// Assign fresh sim-allocator ids to `nodes` (in place); returns
/// the (authored → final) map. Authored ids are recorded into the
/// alias table so slash paths keep resolving TS-style.
fn remap(&mut self, nodes: &mut [PenNode]) -> Result<Vec<(String, String)>, String> {
fn remap(&mut self, nodes: &mut [PenNode]) -> Result<Vec<(String, String)>> {
let mut seed = self
.sim
.next_node_id_seed()
.ok_or("node id space exhausted")?;
.ok_or(ProgramError::IdSpaceExhausted)?;
let mut taken = self.sim.collect_node_ids();
let map = remap_subtree_ids_mapping(nodes, &mut seed, &mut taken)
.ok_or("node id space exhausted")?;
.ok_or(ProgramError::IdSpaceExhausted)?;
for (old, new) in &map {
if !old.starts_with("__op_tmp_") {
self.alias.entry(old.clone()).or_insert_with(|| new.clone());
@ -236,7 +240,7 @@ impl ProgramCtx {
}
/// TS `executeLine` — one DSL operation.
fn execute_line(line: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
fn execute_line(line: &str, ctx: &mut ProgramCtx) -> Result<()> {
// TS line grammar (dotAll `s` flag — pretty-printed JSON bodies
// carry literal newlines inside the arg list):
// binding=OP(args) for I/C/K/R/M/G
@ -272,7 +276,7 @@ fn execute_line(line: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
_ => unreachable!(),
};
}
Err(format!("Cannot parse operation: {line}"))
Err(ProgramError::UnparsableLine(line.to_string()))
}
/// Return append-G line indexes that are robustly sized by later U() calls in
@ -368,7 +372,7 @@ fn positive_json_number(value: &Value) -> bool {
.is_some_and(|number| number.is_finite() && number > 0.0)
}
fn execute_assign(op: &str, binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
fn execute_assign(op: &str, binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
match op {
"I" => execute_insert(binding, args, ctx),
"C" => execute_copy(binding, args, ctx),
@ -385,8 +389,9 @@ fn execute_assign(op: &str, binding: &str, args: &str, ctx: &mut ProgramCtx) ->
}
/// `binding=I(parent, data)` — insert a (possibly nested) node.
fn execute_insert(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
let comma = find_top_level_char(args, ',').ok_or("Insert requires parent and node data")?;
fn execute_insert(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let comma = find_top_level_char(args, ',')
.ok_or_else(|| ProgramError::Syntax("Insert requires parent and node data".into()))?;
let parent_raw = args[..comma].trim();
let parent = resolve_parent_ref(parent_raw, &ctx.bindings);
let mut node = parse_node_json(&args[comma + 1..], ctx.post_process)?;
@ -419,7 +424,7 @@ fn execute_insert(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
let root_id = map
.first()
.map(|(_, new)| new.clone())
.ok_or("Insert produced no node")?;
.ok_or(ProgramError::ProducedNoNode("Insert"))?;
for (cmd, failure) in pre_commands {
ctx.emit(cmd, failure)?;
}
@ -461,8 +466,9 @@ fn execute_insert(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
}
/// `binding=C(sourceId, parent[, overrides])` — clone with fresh ids.
fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
let first = find_top_level_char(args, ',').ok_or("Copy requires sourceId, parent, and data")?;
fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let first = find_top_level_char(args, ',')
.ok_or_else(|| ProgramError::Syntax("Copy requires sourceId, parent, and data".into()))?;
let source_raw = args[..first].trim();
let rest = args[first + 1..].trim();
let (parent_raw, data_str) = match find_top_level_char(rest, ',') {
@ -473,15 +479,19 @@ fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), S
let Some(source) =
op_editor_core::walkers::find_node(ctx.sim.active_children(), &NodeId::new(&source_id))
else {
return Err(format!("Copy source not found: {source_id}"));
return Err(ProgramError::NotFound(format!(
"Copy source not found: {source_id}"
)));
};
let mut cloned_value =
serde_json::to_value(source).map_err(|e| format!("Copy source unserializable: {e}"))?;
let mut cloned_value = serde_json::to_value(source)
.map_err(|e| ProgramError::InvalidNode(format!("Copy source unserializable: {e}")))?;
// TS `Object.assign(cloned, data)` minus `id` (never overridden)
// and `descendants` (a TS no-op — see module docs).
let overrides = parse_json_arg(data_str)?;
let Some(overrides) = overrides.as_object() else {
return Err("C() overrides JSON must be an object".into());
return Err(ProgramError::Syntax(
"C() overrides JSON must be an object".into(),
));
};
if let Some(obj) = cloned_value.as_object_mut() {
for (key, value) in overrides {
@ -492,8 +502,9 @@ fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), S
}
}
normalize_node_shape(&mut cloned_value);
let mut node: PenNode = serde_json::from_value(cloned_value)
.map_err(|e| format!("C() overrides produce an invalid node: {e}"))?;
let mut node: PenNode = serde_json::from_value(cloned_value).map_err(|e| {
ProgramError::InvalidNode(format!("C() overrides produce an invalid node: {e}"))
})?;
if ctx.post_process {
let _ = op_editor_core::command_refine::refine_subtree(&mut node);
}
@ -508,7 +519,7 @@ fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), S
let clone_id = map
.first()
.map(|(_, new)| new.clone())
.ok_or("Copy produced no node")?;
.ok_or(ProgramError::ProducedNoNode("Copy"))?;
let parent = resolve_parent_ref(parent_raw, &ctx.bindings);
ctx.emit(
EditorCommand::InsertAuthoredSubtree {
@ -532,10 +543,12 @@ fn execute_copy(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), S
/// `shadcn-<short-id>` (for example `shadcn/btn-primary` →
/// `shadcn-ui` / `shadcn-btn-primary`). Exact `<kit-id>/<component-id>`
/// pairs are also accepted for imported/future kits.
fn execute_kit_instantiate(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
fn execute_kit_instantiate(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let parts = split_top_level_args(args);
if !(2..=3).contains(&parts.len()) {
return Err("K() requires kitComponentId, parent, and optional overrides".into());
return Err(ProgramError::Syntax(
"K() requires kitComponentId, parent, and optional overrides".into(),
));
}
let kit_component_id = parse_string_arg(parts[0].trim(), "K() kitComponentId")?;
let (kit_id, component_id) = resolve_kit_component_id(&kit_component_id, &ctx.sim)?;
@ -550,7 +563,9 @@ fn execute_kit_instantiate(binding: &str, args: &str, ctx: &mut ProgramCtx) -> R
Some(raw) => {
let value = parse_json_arg(raw)?;
if !value.is_object() {
return Err("K() overrides JSON must be an object".into());
return Err(ProgramError::Syntax(
"K() overrides JSON must be an object".into(),
));
}
Some(value.to_string())
}
@ -571,7 +586,9 @@ fn execute_kit_instantiate(binding: &str, args: &str, ctx: &mut ProgramCtx) -> R
if !node_id.is_real()
|| op_editor_core::walkers::find_node(ctx.sim.active_children(), &node_id).is_none()
{
return Err("K() did not produce a selected node".into());
return Err(ProgramError::Rejected(
"K() did not produce a selected node".into(),
));
}
ctx.bind(binding, node_id.as_str());
Ok(())
@ -579,12 +596,15 @@ fn execute_kit_instantiate(binding: &str, args: &str, ctx: &mut ProgramCtx) -> R
/// `binding=R(path, data)` — replace the node at `path` with a fresh
/// node built from `data` (same slot, fresh id, children dropped).
fn execute_replace(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
let comma = find_top_level_char(args, ',').ok_or("Replace requires path and node data")?;
fn execute_replace(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let comma = find_top_level_char(args, ',')
.ok_or_else(|| ProgramError::Syntax("Replace requires path and node data".into()))?;
let path_raw = args[..comma].trim();
let path = resolve_path_expr(path_raw, &ctx.bindings);
let Some(old) = find_node_by_path(ctx.sim.active_children(), &path, &ctx.alias) else {
return Err(format!("Replace target not found: {path}"));
return Err(ProgramError::NotFound(format!(
"Replace target not found: {path}"
)));
};
let old_id = old.id_str().to_string();
let mut node = parse_node_json(&args[comma + 1..], ctx.post_process)?;
@ -603,7 +623,7 @@ fn execute_replace(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()
let new_id = map
.first()
.map(|(_, new)| new.clone())
.ok_or("Replace produced no node")?;
.ok_or(ProgramError::ProducedNoNode("Replace"))?;
ctx.emit(
EditorCommand::ReplaceSubtree {
node_id: NodeId::new(&old_id),
@ -638,10 +658,10 @@ fn execute_replace(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()
/// hatch allows a new sibling only under a horizontal/vertical flow parent.
/// No fetcher at this layer — `src` stays empty (browser-caller parity);
/// the host's own image pipeline enriches later.
fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let parts = split_top_level_args(args);
if !matches!(parts.len(), 3 | 4) {
return Err(format!("Invalid G() syntax: {args}"));
return Err(ProgramError::Syntax(format!("Invalid G() syntax: {args}")));
}
let parent_raw = parts[0].trim();
let parent = if matches!(parent_raw, "null" | "undefined" | "0" | "\"\"" | "\"0\"") {
@ -650,21 +670,23 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
resolve_path_expr(parent_raw, &ctx.bindings)
};
let mode = serde_json::from_str::<String>(parts[1].trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?;
.map_err(|_| ProgramError::Syntax(format!("Invalid G() syntax: {args}")))?;
if !matches!(mode.as_str(), "search" | "generate") {
return Err(format!("G() mode must be search or generate: {mode}"));
return Err(ProgramError::Rejected(format!(
"G() mode must be search or generate: {mode}"
)));
}
let prompt = serde_json::from_str::<String>(parts[2].trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?;
.map_err(|_| ProgramError::Syntax(format!("Invalid G() syntax: {args}")))?;
let placement = match parts.get(3) {
None => "slot".to_string(),
Some(raw) => serde_json::from_str::<String>(raw.trim())
.map_err(|_| format!("Invalid G() syntax: {args}"))?,
.map_err(|_| ProgramError::Syntax(format!("Invalid G() syntax: {args}")))?,
};
if !matches!(placement.as_str(), "slot" | "append") {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() placement must be \"slot\" or \"append\", got {placement:?}"
));
)));
}
let name: String = prompt.chars().take(40).collect();
let mut value = json!({
@ -682,18 +704,21 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
value["imageSearchQuery"] = json!(prompt);
}
if parent.trim().is_empty() || parent.trim() == "0" {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() placement {placement:?} requires an explicit frame/rectangle target id; create the target first instead of using null"
));
)));
}
let target = find_node_by_path(ctx.sim.active_children(), &parent, &ctx.alias)
.ok_or_else(|| format!("G() parent not found or not a container: {parent}"))?;
let target =
find_node_by_path(ctx.sim.active_children(), &parent, &ctx.alias).ok_or_else(|| {
ProgramError::NotFound(format!("G() parent not found or not a container: {parent}"))
})?;
// `parent` may be a slash path or an authored id that `find_node_by_path`
// translated through `ctx.alias`. The emitted insert must target the live
// resolved node id, never the caller's path/alias spelling.
let target_id = target.id_str().to_string();
let container = node_container(target)
.ok_or_else(|| format!("G() parent not found or not a container: {parent}"))?;
let container = node_container(target).ok_or_else(|| {
ProgramError::NotFound(format!("G() parent not found or not a container: {parent}"))
})?;
// Placement is an explicit structural contract. Slot-fill accepts only an
// EMPTY target; append accepts only an explicitly-authored flow parent.
// Never recover intent from names, dimensions, child kinds, or position.
@ -706,29 +731,29 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
.map(PenNode::id_str)
.collect::<Vec<_>>();
if !child_ids.is_empty() {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() slot target {} must be empty, but it has children [{}]. Pass the exact empty frame/rectangle slot id; use \"append\" only for an intentional child of an explicit horizontal/vertical flow parent",
target.id_str(),
child_ids.join(", ")
));
)));
}
}
"append" => {
if explicit_flow_layout(container).is_none() {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() append target {} must declare layout \"horizontal\" or \"vertical\"; got {}. Append means a new flow sibling and is never an absolute overlay",
target.id_str(),
layout_label(container)
));
)));
}
if !ctx
.explicitly_sized_append_lines
.contains(&ctx.current_line)
{
return Err(
return Err(ProgramError::Rejected(
"G() append requires a result binding followed in the same batch by U(binding, {\"width\": <positive number>, \"height\": <positive number>}); refusing an unsized flow child with default fill_container width and height"
.into(),
);
));
}
}
_ => unreachable!("placement validated above"),
@ -741,14 +766,14 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
let width = target.width_px().or_else(|| resolved.map(|size| size.0));
let height = target.height_px().or_else(|| resolved.map(|size| size.1));
let (Some(width), Some(height)) = (width, height) else {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() target {target_id} uses layout none, so it needs declared width and height that resolve above zero before an image can fill it"
));
)));
};
if width <= 0.0 || height <= 0.0 {
return Err(format!(
return Err(ProgramError::Rejected(format!(
"G() target {target_id} uses layout none, so it needs declared width and height that resolve above zero before an image can fill it"
));
)));
}
value["x"] = json!(0);
value["y"] = json!(0);
@ -761,14 +786,14 @@ fn execute_image(binding: &str, args: &str, ctx: &mut ProgramCtx) -> Result<(),
value["width"] = json!("fill_container");
value["height"] = json!("fill_container");
}
let node: PenNode =
serde_json::from_value(value).map_err(|e| format!("invalid G() image node: {e}"))?;
let node: PenNode = serde_json::from_value(value)
.map_err(|e| ProgramError::InvalidNode(format!("invalid G() image node: {e}")))?;
let mut nodes = vec![node];
let map = ctx.remap(&mut nodes)?;
let image_id = map
.first()
.map(|(_, new)| new.clone())
.ok_or("G() produced no node")?;
.ok_or(ProgramError::ProducedNoNode("G()"))?;
ctx.emit(
EditorCommand::InsertAuthoredSubtree {
nodes,
@ -829,23 +854,29 @@ fn resolved_node_size(state: &EditorState, node_id: &str) -> Option<(f64, f64)>
/// `U(path, data)` — shallow-patch the node at `path`. No result entry
/// (TS call-form ops don't push results).
fn execute_update(args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
let comma = find_top_level_char(args, ',').ok_or("Update requires path and update data")?;
fn execute_update(args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let comma = find_top_level_char(args, ',')
.ok_or_else(|| ProgramError::Syntax("Update requires path and update data".into()))?;
let path = resolve_path_expr(args[..comma].trim(), &ctx.bindings);
let mut value = parse_json_arg(&args[comma + 1..])?;
normalize_node_shape(&mut value);
let Some(target) = find_node_by_path(ctx.sim.active_children(), &path, &ctx.alias) else {
return Err(format!("Update target not found: {path}"));
return Err(ProgramError::NotFound(format!(
"Update target not found: {path}"
)));
};
let node_id = NodeId::new(target.id_str());
let cmd = update_command_from_value(node_id, &value)?;
// `batch_direct_ops` is shared with the non-program write paths and is
// outside this conversion's scope; adapt its `String` at the boundary
// rather than rippling the change into it.
let cmd = update_command_from_value(node_id, &value).map_err(ProgramError::Rejected)?;
let cmd = with_page_id(cmd, ctx.page_id.clone());
ctx.emit(cmd, &format!("Update failed for: {path}"))
}
/// `D(ref)` — delete. TS `removeNodeFromTree` silently no-ops on an
/// unknown id: no error, no result.
fn execute_delete(args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
fn execute_delete(args: &str, ctx: &mut ProgramCtx) -> Result<()> {
let raw = strip_outer_quotes(args.trim());
let node_id = lookup_id(&resolve_ref(&raw, &ctx.bindings), &ctx.alias);
if op_editor_core::walkers::find_node(ctx.sim.active_children(), &NodeId::new(&node_id))
@ -864,16 +895,20 @@ fn execute_delete(args: &str, ctx: &mut ProgramCtx) -> Result<(), String> {
/// `M(nodeId, parent[, index])` (call or bound form) — reparent.
/// Returns the moved node's id so the bound form can record it.
fn execute_move(args: &str, ctx: &mut ProgramCtx) -> Result<String, String> {
fn execute_move(args: &str, ctx: &mut ProgramCtx) -> Result<String> {
let parts = split_top_level_args(args);
if parts.len() < 2 {
return Err("Move requires nodeId and parent".into());
return Err(ProgramError::Syntax(
"Move requires nodeId and parent".into(),
));
}
let node_id = lookup_id(&resolve_ref(parts[0].trim(), &ctx.bindings), &ctx.alias);
if op_editor_core::walkers::find_node(ctx.sim.active_children(), &NodeId::new(&node_id))
.is_none()
{
return Err(format!("Move target not found: {node_id}"));
return Err(ProgramError::NotFound(format!(
"Move target not found: {node_id}"
)));
}
let parent_raw = parts[1].trim();
let parent = resolve_parent_ref(parent_raw, &ctx.bindings);
@ -882,7 +917,11 @@ fn execute_move(args: &str, ctx: &mut ProgramCtx) -> Result<String, String> {
Some(raw) => Some(
strip_outer_quotes(raw.trim())
.parse::<usize>()
.map_err(|_| format!("M() index must be a non-negative integer, got {raw:?}"))?,
.map_err(|_| {
ProgramError::Syntax(format!(
"M() index must be a non-negative integer, got {raw:?}"
))
})?,
),
};
ctx.emit(
@ -963,16 +1002,16 @@ fn find_node_with_parent<'a>(
/// Parse + normalize an I()/R() node body into a `PenNode` with
/// authored ids filled in (the caller remaps them to final ids).
fn parse_node_json(raw: &str, post_process: bool) -> Result<PenNode, String> {
fn parse_node_json(raw: &str, post_process: bool) -> Result<PenNode> {
let mut value = parse_json_arg(raw)?;
if !value.is_object() {
return Err("node data must be a JSON object".into());
return Err(ProgramError::Json("node data must be a JSON object".into()));
}
normalize_node_shape(&mut value);
let mut tmp = 1usize;
ensure_node_ids(&mut value, &mut tmp);
let mut node: PenNode =
serde_json::from_value(value).map_err(|e| format!("invalid PenNode payload: {e}"))?;
let mut node: PenNode = serde_json::from_value(value)
.map_err(|e| ProgramError::InvalidNode(format!("invalid PenNode payload: {e}")))?;
if post_process {
// TS postProcess hooks (emoji strip, unique ids, layout-child
// position sanitize, screen-bounds clamp) — the deterministic
@ -985,7 +1024,7 @@ fn parse_node_json(raw: &str, post_process: bool) -> Result<PenNode, String> {
/// TS `parseJsonArg` — strict JSON first, then the lenient agent-typo
/// pipeline: quote unquoted keys, single→double quote delimiters,
/// strip empty-key artifacts and trailing commas.
fn parse_json_arg(raw: &str) -> Result<Value, String> {
fn parse_json_arg(raw: &str) -> Result<Value> {
let trimmed = raw.trim();
if let Ok(value) = serde_json::from_str::<Value>(trimmed) {
return Ok(value);
@ -1039,24 +1078,24 @@ fn parse_json_arg(raw: &str) -> Result<Value, String> {
.map_err(|e| {
let snippet: String = raw.chars().take(300).collect();
let ellipsis = if raw.chars().count() > 300 { "..." } else { "" };
format!("Failed to parse JSON ({e}): {snippet}{ellipsis}")
ProgramError::Json(format!("Failed to parse JSON ({e}): {snippet}{ellipsis}"))
})
}
fn parse_string_arg(raw: &str, label: &str) -> Result<String, String> {
fn parse_string_arg(raw: &str, label: &str) -> Result<String> {
let value = parse_json_arg(raw)?;
value
.as_str()
.map(str::to_string)
.ok_or_else(|| format!("{label} must be a JSON string"))
.ok_or_else(|| ProgramError::Syntax(format!("{label} must be a JSON string")))
}
fn resolve_kit_component_id(raw: &str, state: &EditorState) -> Result<(String, String), String> {
fn resolve_kit_component_id(raw: &str, state: &EditorState) -> Result<(String, String)> {
let Some((kit_part, component_part)) = raw.split_once('/') else {
return Err(
return Err(ProgramError::Syntax(
"K() kitComponentId must be starter/<id>, shadcn/<id>, or <kit-id>/<component-id>"
.into(),
);
));
};
let kit_id = match kit_part {
"starter" => "openpencil-starter".to_string(),
@ -1069,17 +1108,19 @@ fn resolve_kit_component_id(raw: &str, state: &EditorState) -> Result<(String, S
component_part.to_string()
};
let Some(kit) = state.ui_kits.iter().find(|kit| kit.id == kit_id) else {
return Err(format!("K() kit not found: {kit_part}"));
return Err(ProgramError::NotFound(format!(
"K() kit not found: {kit_part}"
)));
};
if !kit
.components
.iter()
.any(|component| component.id == component_id)
{
return Err(format!(
return Err(ProgramError::NotFound(format!(
"K() component not found: {raw} (resolved to {}/{})",
kit.id, component_id
));
)));
}
Ok((kit.id.clone(), component_id))
}

63
crates/op-mcp/src/batch_program_error.rs Normal file
View file

@ -0,0 +1,63 @@
//! Typed per-line failures for the `batch_design` DSL program executor
//! (`batch_program.rs`).
//!
//! Style follows `op_orchestrator::OrchestratorError`: a plain enum plus a
//! hand-written `Display`, no `thiserror` and no new dependency. Each
//! variant's `Display` reproduces the exact sentence the stringly-typed
//! executor produced, because those sentences ship verbatim to the model in
//! the envelope's `errors[]` array (and several tests assert on them).
//!
//! What the enum buys over `String` is the CLASSIFICATION: the executor —
//! and anything downstream, e.g. a future retry ladder in `program_gen.rs` —
//! can now tell a grammar mistake from a missing node from an id-space
//! exhaustion without pattern-matching prose.
use std::fmt;
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum ProgramError {
/// The line matched none of the DSL operation grammars.
UnparsableLine(String),
/// An operation's argument list is malformed, has the wrong arity, or a
/// scalar argument has the wrong JSON type.
Syntax(String),
/// A JSON argument could not be parsed even after the lenient
/// agent-typo repair pipeline, or parsed to the wrong JSON kind.
Json(String),
/// A body parsed as JSON but is not a valid `PenNode`.
InvalidNode(String),
/// A referenced node, path, kit, or component does not exist.
NotFound(String),
/// The operation parsed and resolved, but a structural / semantic rule
/// of the design protocol refuses it (placement contracts, sizing
/// requirements, layout preconditions).
Rejected(String),
/// The simulated apply refused the command — the host would refuse it
/// too, so the line cannot ship.
ApplyRejected(String),
/// An operation that must yield a node yielded none. The payload is the
/// operation label as it appears in the message (`Insert`, `Copy`,
/// `Replace`, `G()`).
ProducedNoNode(&'static str),
/// The document's node id space is exhausted, so no fresh id can be
/// minted for the remapped subtree.
IdSpaceExhausted,
}
impl fmt::Display for ProgramError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ProgramError::UnparsableLine(line) => write!(f, "Cannot parse operation: {line}"),
ProgramError::Syntax(m)
| ProgramError::Json(m)
| ProgramError::InvalidNode(m)
| ProgramError::NotFound(m)
| ProgramError::Rejected(m)
| ProgramError::ApplyRejected(m) => f.write_str(m),
ProgramError::ProducedNoNode(op) => write!(f, "{op} produced no node"),
ProgramError::IdSpaceExhausted => f.write_str("node id space exhausted"),
}
}
}
impl std::error::Error for ProgramError {}