Stop automation re-routing dialogs to file-writing handlers

`dialogs::confirm` picks a dialog's handler by marker fields (`__diskChanged`,
`__export`, `__web`, `__print`, ...). Only the app sets them now:

- `ui.dialog.set` and `ui.dialog.open`'s `fields` refuse to add a key any of
  confirm's or cancel's `owns()` predicates route on
  (`dialogs::is_route_marker`, which probes the predicates themselves). A
  dialog's own fields, private ones included, stay settable.
- `dialogs::confirm`, when driven by automation (a control request's gate or
  synthetic input), judges a handler that writes files itself as the file
  command it stands for: file.save, file.export.exportAs,
  file.export.saveForWebLegacy or file.print.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
James Chapman
2026-10-22 18:41:32 +00:00
committed by aiongg
co-authored by Claude Opus 5.5
parent a2fe5b2458
commit ebdddbe1f5
2 changed files with 146 additions and 0 deletions
+76
View File
@@ -902,6 +902,9 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest)
};
let mut fields = if kind == DialogKind::NewDocument { app.new_document_fields() } else { Default::default() };
if let Some(f) = p.get("fields").and_then(Value::as_object) {
if let Some(marker) = f.keys().find(|k| crate::dialogs::is_route_marker(k)) {
return err(format!("`{marker}` is set only by the dialog it belongs to"));
}
fields.extend(f.clone());
}
ok(json!({"dialog": app.ui.open_dialog(kind, fields)}))
@@ -955,6 +958,10 @@ fn dispatch(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest)
let (Some(id), Some(field)) = (u("dialog"), s("field")) else { return err("need `dialog` and `field`") };
let value = p.get("value").cloned().unwrap_or(Value::Null);
match app.ui.dialog_mut(id) {
// A marker picks the handler OK runs: a dialog keeps the ones it was opened with.
Some(d) if !d.fields.contains_key(field) && crate::dialogs::is_route_marker(field) => {
err(format!("`{field}` is set only by the dialog it belongs to"))
}
Some(d) => {
d.fields.insert(field.to_string(), value);
ok(Value::Null)
@@ -2661,6 +2668,75 @@ mod tests {
}
}
/// An app whose automation gates refuse file commands, with a document open, and the paths
/// its writer was asked to write.
fn gated_app_recording_writes() -> (PhotocraftApp, egui::Context, std::rc::Rc<std::cell::RefCell<Vec<String>>>, u64) {
let written: std::rc::Rc<std::cell::RefCell<Vec<String>>> = std::rc::Rc::default();
let w = written.clone();
let services = crate::Services {
automation_authorize: Some(deny_ambient_file),
automation_command: Some(Box::new(|id, params| deny_ambient_file(id, params).map_err(|e| e.to_string()))),
export: Some(Box::new(|_: &photocraft_doc::Document, _: &str, _: &crate::ExportSettings| Ok((b"out".to_vec(), Vec::new())))),
write: Some(Box::new(move |path: &str, _: &[u8]| {
w.borrow_mut().push(path.to_string());
Ok(())
})),
..Default::default()
};
let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), services);
app.background_jobs = false;
app.session.execute("file.new", json!({"width": 4, "height": 4})).unwrap();
let doc = app.session.active().unwrap().doc.id.0;
(app, egui::Context::default(), written, doc)
}
fn outside_target() -> String {
std::env::temp_dir().join("outside").join("x.png").to_string_lossy().into_owned()
}
/// `ui.dialog.set` and `ui.dialog.open` never add a marker that picks a dialog's OK handler:
/// a View › Zoom In dialog can't be made to confirm as the disk-change prompt's save.
#[test]
fn automation_cannot_add_a_marker_that_reroutes_a_dialog() {
let (mut app, ctx, written, doc) = gated_app_recording_writes();
let target = outside_target();
let r = call(&mut app, &ctx, "ui.dialog.open", json!({"kind": "command", "command": "view.zoomIn"}));
let dialog = r["result"]["dialog"].clone();
for (field, value) in [("document", json!(doc)), ("path", json!(target)), ("flat", json!(false))] {
let r = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": dialog, "field": field, "value": value}));
assert!(r.get("error").is_none(), "{r}");
}
let injected = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": dialog, "field": "__diskChanged", "value": true}));
call(&mut app, &ctx, "ui.dialog.confirm", json!({"dialog": dialog}));
assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow());
assert!(injected["error"].as_str().is_some_and(|e| e.contains("set only by the dialog")), "{injected}");
let r = call(&mut app, &ctx, "ui.dialog.open", json!({"kind": "about", "fields": {"__export": true}}));
assert!(r["error"].as_str().is_some_and(|e| e.contains("set only by the dialog")), "{r}");
}
/// A handler that writes files itself (the disk-change prompt's save) is judged as the file
/// command it stands for, through a control request or synthetic input alike.
#[test]
fn automation_confirming_a_file_writing_dialog_is_judged_as_its_file_command() {
let (mut app, ctx, written, doc) = gated_app_recording_writes();
let target = outside_target();
// A real disk-change prompt keeps its own fields settable, but its save is a file command.
let fields = json!({"__diskChanged": true, "__label": "Save", "message": "", "document": doc, "path": target, "flat": false});
let prompt = app.ui.open_dialog(DialogKind::Command, fields.as_object().cloned().unwrap());
let r = call(&mut app, &ctx, "ui.dialog.set", json!({"dialog": prompt, "field": "__diskChanged", "value": true}));
assert!(r.get("error").is_none(), "{r}");
let r = call(&mut app, &ctx, "ui.dialog.confirm", json!({"dialog": prompt}));
assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow());
assert!(r["error"].as_str().is_some_and(|e| e.contains("file.save")), "{r}");
// Synthetic input (Enter or OK) reaches the same handler without the control pre-check.
let synthetic = app.ui.open_dialog(DialogKind::Command, fields.as_object().cloned().unwrap());
app.automation_input = true;
let r = crate::dialogs::confirm(&mut app, synthetic);
app.automation_input = false;
assert!(written.borrow().is_empty(), "nothing is written: {:?}", written.borrow());
assert!(r.is_err_and(|e| e.contains("file.save")));
}
#[test]
fn app_open_opens_a_directory_pcraft_and_falls_back_for_the_zip_form() {
use photocraft_doc::Document;
+70
View File
@@ -688,11 +688,81 @@ pub fn title(d: &Dialog) -> String {
}
}
type Fields = serde_json::Map<String, Value>;
/// The markers [`confirm`] and [`cancel`] choose a dialog's handler by: keys the code opening a
/// dialog puts in its fields. Keep in step with the arms of `confirm`.
const ROUTES: &[fn(&Fields) -> bool] = &[
crate::guide_layout_ui::owns,
crate::brush_preset_dialog::owns,
crate::fill_ui::owns,
crate::pattern_fill_ui::owns,
crate::stroke_ui::owns,
crate::channel_options_ui::owns,
crate::channel_options_ui::owns_new_spot,
crate::shape_dialog::owns,
crate::delete_layer_prompt::owns,
crate::disk_change_prompt::owns,
crate::actions::owns_insert_stop,
crate::new_layer_ui::owns,
crate::mask_trash_prompt::owns,
crate::rasterize_prompt::owns,
crate::duplicate_layer_ui::owns,
crate::stack_dialog::owns,
crate::tool_reset::owns,
crate::crop_presets::owns,
crate::variables_ui::owns,
crate::file_ui::owns,
crate::new_guide_ui::owns,
crate::color_picker_ui::owns,
crate::color_range_ui::owns,
crate::prefs_ui::owns,
crate::open_prompt::owns,
crate::layer_style::color_picker::owns,
|f| f.contains_key("__export"),
|f| f.contains_key("__smartFilter"),
|f| f.contains_key(crate::actions::RESUME),
];
/// Whether `key` is a marker that picks a dialog's handler. Automation may change the values of
/// a dialog's fields, private ones included, but never add a marker: a dialog it opened would
/// otherwise confirm through another dialog's handler, one its command was never judged for.
pub fn is_route_marker(key: &str) -> bool {
let probe = Fields::from_iter([(key.to_string(), Value::Bool(true))]);
ROUTES.iter().any(|owns| owns(&probe))
}
/// The file command a dialog's handler stands for when it reads or writes files itself, outside
/// the engine (a disk-change save, Export As, Save for Web, Print), so automation is judged for
/// that command rather than for the dialog's `__command`.
fn file_command(fields: &Fields) -> Option<&'static str> {
if crate::disk_change_prompt::owns(fields) {
Some("file.save")
} else if fields.contains_key("__export") {
Some("file.export.exportAs")
} else if fields.contains_key("__web") {
Some("file.export.saveForWebLegacy")
} else if fields.contains_key("__print") {
Some("file.print")
} else {
None
}
}
/// Confirm a dialog: run its action and close it. Used by the OK button and by automation.
pub fn confirm(app: &mut PhotocraftApp, id: u64) -> Result<Value, String> {
if crate::layer_style::color_picker::has_child(app, id) {
return Err("confirm or cancel the Layer Style color picker first".into());
}
// A control request installs the session gate; synthetic input sets `automation_input`.
// Either way the handler's own file access is judged before it runs, as its command would be.
if (app.automation_input || app.session.authorize.is_some())
&& let Some(d) = app.ui.dialogs.iter().find(|d| d.id == id)
&& let Some(command) = file_command(&d.fields)
&& let Some(authorize) = app.services.automation_command.as_ref()
{
authorize(command, &Value::Object(d.fields.clone()))?;
}
if let Some(d) = app.ui.dialogs.iter().find(|d| d.id == id).filter(|d| crate::guide_layout_ui::owns(&d.fields)) {
if app.ui.dialogs.iter().any(|child| crate::color_picker_ui::owns(&child.fields) && child.fields.get("__dialog").and_then(Value::as_u64) == Some(id)) {
return Err("confirm or cancel the guide color picker first".into());