forked from 1b-agent/opencadstudio-upstream
feat: SCRIPTCOMMANDS setting to turn script-driven commands off
A saved user preference (on by default) toggled by the SCRIPTCOMMANDS command gates HostApi::run_command; Cancel stays allowed so a waiting command is never stranded. The runner refuses the SCRIPTCOMMANDS command itself, so a script cannot change it. Documents why no helper-thread or stack-growing dependency was added: native builds link with at least 8 MiB, the nesting depth is fixed, and the deepest step found needs 2 to 4 MiB in a debug build. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
a25ae4f8c0
commit
2ed41586de
8 files changed
+120
-15
No files matched your search
@@ -9,7 +9,7 @@ Build a general Python document model over the OCS host API so a script can cont
|
|||||||
- **Entities:** explicit writes for all 43 canvas kinds (41 creatable; the legacy `Polyline` and `Body` are update-only). **31 kinds are `Complete`** (every C/R/E/D/U/I/W/V/P gate passes). The remaining kinds are integration-tested and held short of `Complete` by a named blocker in the ledger: seven by cadcodec DXF bugs (fixes submitted as [cadcodec#48](https://github.com/HakanSeven12/cadcodec/pull/48), not yet merged, so OCS still pins the unfixed revision), Polyline3D by the DWG format, Polyline2D by a kind change on DXF save, RasterImage by an image-reactor structure that cannot be verified, and Body by having no creation path.
|
- **Entities:** explicit writes for all 43 canvas kinds (41 creatable; the legacy `Polyline` and `Body` are update-only). **31 kinds are `Complete`** (every C/R/E/D/U/I/W/V/P gate passes). The remaining kinds are integration-tested and held short of `Complete` by a named blocker in the ledger: seven by cadcodec DXF bugs (fixes submitted as [cadcodec#48](https://github.com/HakanSeven12/cadcodec/pull/48), not yet merged, so OCS still pins the unfixed revision), Polyline3D by the DWG format, Polyline2D by a kind change on DXF save, RasterImage by an image-reactor structure that cannot be verified, and Body by having no creation path.
|
||||||
- **Drawing tables:** layers, text and dimension styles, blocks and their contents, linetypes and layouts, through the additive `TableOperation` request (see the log entries dated 2026-09-21).
|
- **Drawing tables:** layers, text and dimension styles, blocks and their contents, linetypes and layouts, through the additive `TableOperation` request (see the log entries dated 2026-09-21).
|
||||||
- **Commands:** `HostApi::run_command` drives the real OCS commands step by step; `doc.command`, `doc.start_command` and `doc.modify` build on it.
|
- **Commands:** `HostApi::run_command` drives the real OCS commands step by step; `doc.command`, `doc.start_command` and `doc.modify` build on it.
|
||||||
- **Known open items:** the unfiled cadcodec findings (`cadcodec-reader-gaps.md`), complex (text and shape) linetypes, plot devices and named page setups, PEDIT's remaining options, and an on/off setting for script-driven commands.
|
- **Known open items:** the unfiled cadcodec findings (`cadcodec-reader-gaps.md`), complex (text and shape) linetypes, plot devices and named page setups, and PEDIT's remaining options.
|
||||||
|
|
||||||
Repositories:
|
Repositories:
|
||||||
|
|
||||||
@@ -302,3 +302,21 @@ step needs between 2 and 4 MiB of stack in a debug build, more than a test threa
|
|||||||
application's main thread has 8 MiB, and a release build uses far less. Not wrapped: PEDIT fit, spline, decurve, linetype generation and
|
application's main thread has 8 MiB, and a release build uses far less. Not wrapped: PEDIT fit, spline, decurve, linetype generation and
|
||||||
vertex editing.
|
vertex editing.
|
||||||
|
|
||||||
|
### 2026-09-21: guardrails (`SCRIPTCOMMANDS`, stack)
|
||||||
|
|
||||||
|
**On/off setting.** A saved user preference, `UserSettings::script_commands` (on by default, so an existing config keeps
|
||||||
|
scripts allowed), toggled by the `SCRIPTCOMMANDS [0|1]` command, gates `HostApi::run_command`: while it is off every `Run`,
|
||||||
|
`Start` and input request is refused (`Cancel` stays allowed so a waiting command is never stranded) with a message that says how to turn it back on. The runner refuses the command
|
||||||
|
itself (it joins `QUIT`, `NEW`, `PY_*` and the rest on the denylist), so a script cannot switch it. It is a command and a
|
||||||
|
persisted flag only; there is no Options-page control, which would need locale and layout work in files outside this feature.
|
||||||
|
Evidence: `script_commands_setting_gates_the_runner` (default on; a script cannot change it; the user turns it off and nothing
|
||||||
|
runs, `Run` and `Start` both refused, no entity added; back on and it runs) and `script_commands_default_on_and_survive_a_round_trip`.
|
||||||
|
Checked that the test did not write the setting into the real user config.
|
||||||
|
|
||||||
|
**Stack.** The planned large-stack thread for command steps was not built, on the evidence: the native builds already link with
|
||||||
|
at least 8 MiB (`.cargo/config.toml` raises Windows to 16 MiB), the deepest step found (PEDIT converting a line to a polyline)
|
||||||
|
needs 2 to 4 MiB in a debug build, and the nesting depth is fixed because `PY_*` is refused, so it cannot recurse. Moving the step
|
||||||
|
to a helper thread would be unsound here (the editor state is not `Send` and some of it has thread affinity), and the alternative,
|
||||||
|
a stack-growing dependency such as `stacker`, adds a crate to H7's tree to guard a case that cannot occur. The 64 MiB thread in the
|
||||||
|
PEDIT audit stays, because a test thread gets only 2 MiB.
|
||||||
|
|
||||||
+13
-10
@@ -96,11 +96,14 @@ to arrive from outside; an earlier fire-and-forget replay could hang, this does
|
|||||||
|
|
||||||
Guards: refused while another command is active or off the active tab; refused for
|
Guards: refused while another command is active or off the active tab; refused for
|
||||||
commands that could end the session or re-enter Python (`QUIT`, `EXIT`, `CLOSE*`,
|
commands that could end the session or re-enter Python (`QUIT`, `EXIT`, `CLOSE*`,
|
||||||
`NEW`, `QNEW`, `OPEN`, `SAVE*`, `RECOVER`, `SCRIPT`, `RUNSCRIPT`, `PY_*`); an editor or
|
`NEW`, `QNEW`, `OPEN`, `SAVE*`, `RECOVER`, `SCRIPT`, `RUNSCRIPT`, `SCRIPTCOMMANDS`, `PY_*`); an editor or
|
||||||
dialog a command opens is reported and closed by `Cancel`. It does not consult the
|
dialog a command opens is reported and closed by `Cancel`. The user can turn the whole
|
||||||
automation on/off switch, which governs the external MCP and serve channels; a script
|
facility off with the saved `SCRIPTCOMMANDS` preference (on by default; `SCRIPTCOMMANDS 0`
|
||||||
is something the user chose to run. The Python layer cancels any step sequence that
|
at the command line): while it is off, every request except `Cancel` is refused (so a command a script left
|
||||||
leaves a command waiting.
|
waiting can still be closed). A script cannot change
|
||||||
|
it, because the runner refuses that command. The runner does not consult the automation
|
||||||
|
on/off switch, which governs the external MCP and serve channels. The Python layer cancels
|
||||||
|
any step sequence that leaves a command waiting.
|
||||||
|
|
||||||
## Python surface
|
## Python surface
|
||||||
|
|
||||||
@@ -126,8 +129,8 @@ as `{"type": "overflow", "dropped": n}`. Tokens are bound to the tab that asked.
|
|||||||
descriptions, a dimension style's text-style name) and mis-scales one angle; these are
|
descriptions, a dimension style's text-style name) and mis-scales one angle; these are
|
||||||
pinned by canaries and listed in `cadcodec-reader-gaps.md`.
|
pinned by canaries and listed in `cadcodec-reader-gaps.md`.
|
||||||
- Not available from a script: complex (text and shape) linetypes, plot devices and named
|
- Not available from a script: complex (text and shape) linetypes, plot devices and named
|
||||||
page setups, PEDIT's fit and spline options, interactive-only commands such as HATCH, and
|
page setups, PEDIT's fit and spline options, and interactive-only commands such as HATCH.
|
||||||
a setting to turn script-driven commands off.
|
- A command step nests inside the editor's message handling. In a debug build a deep step
|
||||||
- A command step nests inside the editor's message handling; in a debug build a deep step
|
(PEDIT converting a line to a polyline) needs between 2 and 4 MiB of stack; the native
|
||||||
(PEDIT converting a line to a polyline) needs more than 2 MiB of stack, which the
|
builds link with at least 8 MiB (16 MiB on Windows, `.cargo/config.toml`), the nesting depth
|
||||||
application's 8 MiB main thread provides and a release build needs far less of.
|
is fixed rather than recursive (`PY_*` is refused), and a release build uses far less.
|
||||||
@@ -312,9 +312,13 @@ points default to the middle of a line, arc or circle and the first vertex of a
|
|||||||
undone as the editor would undo it.
|
undone as the editor would undo it.
|
||||||
|
|
||||||
Guards: a command is refused while another is active (cancel it first) or when the
|
Guards: a command is refused while another is active (cancel it first) or when the
|
||||||
line names a command that could end the session or re-enter Python: `QUIT`,
|
line names a command that could end the session, re-enter Python or change this
|
||||||
`EXIT`, `CLOSE`, `CLOSEALL`, `NEW`, `QNEW`, `OPEN`, `SAVE`, `QSAVE`, `SAVEAS`, `SAVEALL`,
|
setting: `QUIT`, `EXIT`, `CLOSE`, `CLOSEALL`, `NEW`, `QNEW`, `OPEN`, `SAVE`, `QSAVE`,
|
||||||
`RECOVER`, `SCRIPT`, `RUNSCRIPT` and any `PY_*`. Commands run on the active
|
`SAVEAS`, `SAVEALL`, `RECOVER`, `SCRIPT`, `RUNSCRIPT`, `SCRIPTCOMMANDS` and any `PY_*`.
|
||||||
|
The user can switch the whole facility off with the `SCRIPTCOMMANDS 0` command (`1`
|
||||||
|
turns it back on, plain `SCRIPTCOMMANDS` shows it); it is a saved preference, on by
|
||||||
|
default, and while it is off every `doc.command`, `start_command` and `doc.modify` call
|
||||||
|
raises `RuntimeError`. Only the command line can change it, never a script. Commands run on the active
|
||||||
document only. Commands that need an interactive surface a script cannot fill
|
document only. Commands that need an interactive surface a script cannot fill
|
||||||
(the rich-text editor, a file dialog) report it in `blocked_by` and are cancelled.
|
(the rich-text editor, a file dialog) report it in `blocked_by` and are cancelled.
|
||||||
PEDIT's fit, spline, decurve and vertex-edit options are not wrapped (reach them with
|
PEDIT's fit, spline, decurve and vertex-edit options are not wrapped (reach them with
|
||||||
|
|||||||
@@ -2894,6 +2894,25 @@ impl OpenCADStudio {
|
|||||||
_ => self.command_line.push_error(crate::t!("Requires 0 or 1").as_ref()),
|
_ => self.command_line.push_error(crate::t!("Requires 0 or 1").as_ref()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
cmd if cmd == "SCRIPTCOMMANDS" || cmd.starts_with("SCRIPTCOMMANDS ") => {
|
||||||
|
// A user setting, deliberately outside the script's reach: it is refused by
|
||||||
|
// the script command runner, so only the command line can change it.
|
||||||
|
let arg = cmd.trim_start_matches("SCRIPTCOMMANDS").trim();
|
||||||
|
match arg {
|
||||||
|
"" => {}
|
||||||
|
"1" | "ON" | "YES" => self.script_commands = true,
|
||||||
|
"0" | "OFF" | "NO" => self.script_commands = false,
|
||||||
|
_ => {
|
||||||
|
self.command_line.push_error("SCRIPTCOMMANDS takes 1 (scripts may run commands) or 0 (they may not)");
|
||||||
|
return Some(Task::none());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !arg.is_empty() {
|
||||||
|
self.persist_settings_if_changed();
|
||||||
|
}
|
||||||
|
let state = if self.script_commands { 1 } else { 0 };
|
||||||
|
self.command_line.push_output(&format!("SCRIPTCOMMANDS = {state}"));
|
||||||
|
}
|
||||||
"SAVETIME" => {
|
"SAVETIME" => {
|
||||||
use crate::command::ValuePromptCommand;
|
use crate::command::ValuePromptCommand;
|
||||||
let c = ValuePromptCommand::new(
|
let c = ValuePromptCommand::new(
|
||||||
|
|||||||
@@ -616,6 +616,8 @@ pub(super) struct OpenCADStudio {
|
|||||||
/// Minutes between autosaves to a `.sv$` recovery file (SAVETIME command);
|
/// Minutes between autosaves to a `.sv$` recovery file (SAVETIME command);
|
||||||
/// 0 disables autosave.
|
/// 0 disables autosave.
|
||||||
pub savetime_min: i32,
|
pub savetime_min: i32,
|
||||||
|
/// SCRIPTCOMMANDS: scripts may run OCS commands (see `UserSettings`).
|
||||||
|
pub script_commands: bool,
|
||||||
/// Persisted default viewport background, restored from settings and applied
|
/// Persisted default viewport background, restored from settings and applied
|
||||||
/// to every drawing tab (new and opened) so a chosen background survives
|
/// to every drawing tab (new and opened) so a chosen background survives
|
||||||
/// restarts (#188). `None` = the built-in dark-grey / off-white defaults.
|
/// restarts (#188). `None` = the built-in dark-grey / off-white defaults.
|
||||||
@@ -3956,6 +3958,7 @@ impl OpenCADStudio {
|
|||||||
constraint_bar_display: 3,
|
constraint_bar_display: 3,
|
||||||
constraint_bar_mode: 4095,
|
constraint_bar_mode: 4095,
|
||||||
savetime_min: 10,
|
savetime_min: 10,
|
||||||
|
script_commands: true,
|
||||||
default_bg_color: None,
|
default_bg_color: None,
|
||||||
default_paper_bg_color: None,
|
default_paper_bg_color: None,
|
||||||
cliprompt_lines: 3,
|
cliprompt_lines: 3,
|
||||||
|
|||||||
+39
-1
@@ -1291,7 +1291,7 @@ impl<'a> HostSession<'a> {
|
|||||||
let first = line.split_whitespace().next()?.trim_start_matches(['\'', '_']).to_uppercase();
|
let first = line.split_whitespace().next()?.trim_start_matches(['\'', '_']).to_uppercase();
|
||||||
const DENIED: &[&str] = &[
|
const DENIED: &[&str] = &[
|
||||||
"QUIT", "EXIT", "CLOSE", "CLOSEALL", "NEW", "QNEW", "OPEN", "SAVE", "QSAVE", "SAVEAS",
|
"QUIT", "EXIT", "CLOSE", "CLOSEALL", "NEW", "QNEW", "OPEN", "SAVE", "QSAVE", "SAVEAS",
|
||||||
"SAVEALL", "RECOVER", "SCRIPT", "RUNSCRIPT",
|
"SAVEALL", "RECOVER", "SCRIPT", "RUNSCRIPT", "SCRIPTCOMMANDS",
|
||||||
];
|
];
|
||||||
if DENIED.contains(&first.as_str()) || first.starts_with("PY_") {
|
if DENIED.contains(&first.as_str()) || first.starts_with("PY_") {
|
||||||
Some(format!("the command {first} cannot be run from a script"))
|
Some(format!("the command {first} cannot be run from a script"))
|
||||||
@@ -1299,6 +1299,11 @@ impl<'a> HostSession<'a> {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
// Cancel stays allowed: if the user turns scripts off mid-run, a command a script left
|
||||||
|
// waiting must still be closable rather than stranded.
|
||||||
|
if !self.app.script_commands && !matches!(request, R::Cancel) {
|
||||||
|
return Err("running commands from a script is turned off (SCRIPTCOMMANDS = 0); turn it on at the command line with SCRIPTCOMMANDS 1".to_owned());
|
||||||
|
}
|
||||||
let active = self.app.tabs[tab].active_cmd.is_some();
|
let active = self.app.tabs[tab].active_cmd.is_some();
|
||||||
let before = self.document().entities().count() as i64;
|
let before = self.document().entities().count() as i64;
|
||||||
let error_revision = self.app.command_line.error_revision;
|
let error_revision = self.app.command_line.error_revision;
|
||||||
@@ -8265,6 +8270,39 @@ step('undo_move', lambda: M.move([u], (0, 0, 0), (7, 0, 0)))
|
|||||||
drop(process);
|
drop(process);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The user's SCRIPTCOMMANDS setting gates the command runner, and a script cannot change it.
|
||||||
|
#[test]
|
||||||
|
fn script_commands_setting_gates_the_runner() {
|
||||||
|
use ocs_plugin_api::host::CommandRequest as R;
|
||||||
|
let mut app = OpenCADStudio::new_for_test();
|
||||||
|
app.tabs[0].is_start = false;
|
||||||
|
assert!(app.script_commands, "scripts may run commands by default");
|
||||||
|
let run = |app: &mut OpenCADStudio, line: &str| HostSession::new(app, 0).run_command(R::Run { line: line.into() });
|
||||||
|
assert!(run(&mut app, "LINE 0,0 10,0").is_ok());
|
||||||
|
|
||||||
|
// A script cannot touch the setting, on or off.
|
||||||
|
let refused = run(&mut app, "SCRIPTCOMMANDS 0").unwrap_err();
|
||||||
|
assert!(refused.contains("cannot be run from a script"), "{refused}");
|
||||||
|
assert!(app.script_commands);
|
||||||
|
|
||||||
|
// The user turns it off at the command line.
|
||||||
|
let reply = app.automation_op(r#"{"op":"run","cmd":"SCRIPTCOMMANDS 0"}"#);
|
||||||
|
assert_eq!(reply["ok"], true, "{reply}");
|
||||||
|
assert!(!app.script_commands);
|
||||||
|
let refused = run(&mut app, "LINE 0,0 20,0").unwrap_err();
|
||||||
|
assert!(refused.contains("turned off"), "{refused}");
|
||||||
|
let refused = HostSession::new(&mut app, 0).run_command(R::Start { name: "LINE".into() }).unwrap_err();
|
||||||
|
assert!(refused.contains("turned off"), "{refused}");
|
||||||
|
assert_eq!(app.tabs[0].scene.document.entities().count(), 1, "nothing ran while it was off");
|
||||||
|
assert!(HostSession::new(&mut app, 0).run_command(R::Cancel).is_ok(), "Cancel is still allowed while it is off");
|
||||||
|
|
||||||
|
// And back on.
|
||||||
|
app.automation_op(r#"{"op":"run","cmd":"SCRIPTCOMMANDS 1"}"#);
|
||||||
|
assert!(app.script_commands);
|
||||||
|
assert!(run(&mut app, "LINE 0,0 20,0").is_ok());
|
||||||
|
assert_eq!(app.tabs[0].scene.document.entities().count(), 2);
|
||||||
|
}
|
||||||
|
|
||||||
/// Exploration harness (not a gate): prints how the real commands answer
|
/// Exploration harness (not a gate): prints how the real commands answer
|
||||||
/// each step so the wrappers are written against observed prompts.
|
/// each step so the wrappers are written against observed prompts.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -476,6 +476,10 @@ pub struct UserSettings {
|
|||||||
/// Minutes between autosaves to a `.sv$` recovery file (SAVETIME command).
|
/// Minutes between autosaves to a `.sv$` recovery file (SAVETIME command).
|
||||||
/// 0 disables autosave.
|
/// 0 disables autosave.
|
||||||
pub savetime_min: i32,
|
pub savetime_min: i32,
|
||||||
|
/// SCRIPTCOMMANDS: whether a script (the Python plugin) may run OCS commands
|
||||||
|
/// through the host. On by default; a user turns it off with the
|
||||||
|
/// `SCRIPTCOMMANDS 0` command, and a script cannot change it.
|
||||||
|
pub script_commands: bool,
|
||||||
/// File type and version used when a new/unsaved drawing is first saved.
|
/// File type and version used when a new/unsaved drawing is first saved.
|
||||||
/// Existing drawings keep their own type and version.
|
/// Existing drawings keep their own type and version.
|
||||||
pub default_save_format: String,
|
pub default_save_format: String,
|
||||||
@@ -676,6 +680,7 @@ impl Default for UserSettings {
|
|||||||
constraint_bar_display: 3,
|
constraint_bar_display: 3,
|
||||||
constraint_bar_mode: 4095,
|
constraint_bar_mode: 4095,
|
||||||
savetime_min: 10,
|
savetime_min: 10,
|
||||||
|
script_commands: true,
|
||||||
default_save_format: crate::io::DEFAULT_SAVE_FORMAT.to_string(),
|
default_save_format: crate::io::DEFAULT_SAVE_FORMAT.to_string(),
|
||||||
pick_add: true,
|
pick_add: true,
|
||||||
pick_drag_rect: false,
|
pick_drag_rect: false,
|
||||||
@@ -789,6 +794,19 @@ mod tests {
|
|||||||
assert_eq!(cfg.settings.savetime_min, 42);
|
assert_eq!(cfg.settings.savetime_min, 42);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn script_commands_default_on_and_survive_a_round_trip() {
|
||||||
|
let missing: crate::app::config::AppConfig =
|
||||||
|
serde_json::from_str(r#"{"settings": {"pick_add": false}}"#).unwrap();
|
||||||
|
assert!(missing.settings.script_commands, "an old config keeps scripts allowed");
|
||||||
|
let off: crate::app::config::AppConfig =
|
||||||
|
serde_json::from_str(r#"{"settings": {"script_commands": false}}"#).unwrap();
|
||||||
|
assert!(!off.settings.script_commands);
|
||||||
|
let text = serde_json::to_string(&off).unwrap();
|
||||||
|
let back: crate::app::config::AppConfig = serde_json::from_str(&text).unwrap();
|
||||||
|
assert!(!back.settings.script_commands);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn isometric_drafting_never_persists_or_loads() {
|
fn isometric_drafting_never_persists_or_loads() {
|
||||||
// A persisted On used to bring the isoplane crosshair (vertical +
|
// A persisted On used to bring the isoplane crosshair (vertical +
|
||||||
|
|||||||
@@ -619,6 +619,7 @@ impl OpenCADStudio {
|
|||||||
constraint_bar_display: self.constraint_bar_display,
|
constraint_bar_display: self.constraint_bar_display,
|
||||||
constraint_bar_mode: self.constraint_bar_mode,
|
constraint_bar_mode: self.constraint_bar_mode,
|
||||||
savetime_min: self.savetime_min,
|
savetime_min: self.savetime_min,
|
||||||
|
script_commands: self.script_commands,
|
||||||
default_save_format: self.default_save_format.clone(),
|
default_save_format: self.default_save_format.clone(),
|
||||||
pick_add: self.pick_add,
|
pick_add: self.pick_add,
|
||||||
pick_drag_rect: self.pick_drag_rect,
|
pick_drag_rect: self.pick_drag_rect,
|
||||||
@@ -723,6 +724,7 @@ impl OpenCADStudio {
|
|||||||
self.constraint_bar_display = s.constraint_bar_display.clamp(0, 3);
|
self.constraint_bar_display = s.constraint_bar_display.clamp(0, 3);
|
||||||
self.constraint_bar_mode = s.constraint_bar_mode.clamp(0, 4095);
|
self.constraint_bar_mode = s.constraint_bar_mode.clamp(0, 4095);
|
||||||
self.savetime_min = s.savetime_min;
|
self.savetime_min = s.savetime_min;
|
||||||
|
self.script_commands = s.script_commands;
|
||||||
self.default_save_format =
|
self.default_save_format =
|
||||||
crate::io::canonical_save_format(&s.default_save_format).to_string();
|
crate::io::canonical_save_format(&s.default_save_format).to_string();
|
||||||
self.pick_add = s.pick_add;
|
self.pick_add = s.pick_add;
|
||||||
|
|||||||
Reference in new issue
Block a user