diff --git a/docs/plugin-architecture.md b/docs/plugin-architecture.md index 19b1a0968..9e573253d 100644 --- a/docs/plugin-architecture.md +++ b/docs/plugin-architecture.md @@ -101,13 +101,27 @@ affect whether a prebuilt plugin can safely be loaded: A plugin's `plugin.toml` declares the same values under `[opencad]`. For API v4 and later, `rustc_version` is required and the host refuses to load the plugin -unless it matches exactly. This turns the current silent runner crash into a -one-line diagnosis such as: +unless it matches exactly — omitting it is treated as a mismatch, not skipped. +This turns the current silent runner crash into a one-line diagnosis such as: ``` Plugin built with rustc 1.96.0, host requires 1.98.0 - rebuild required ``` +**`acadrust_source` is checked differently, and this matters:** unlike +`rustc_version`, the check only runs if the plugin's `plugin.toml` declares +`acadrust_source` at all — the same `if !acadrust_declared { return true; }` +(or equivalent inline match) appears in **two** places, +`ExternalPlugin::acadrust_compatible` in `src/plugin/external.rs` (used when +loading an installed plugin) and the inline check inside +`fetch_release_info` in `src/plugin/marketplace.rs` (used when listing +releases in the Plugin Manager) — a manifest that omits the key entirely is treated +as compatible, not rejected. In practice this means the ABI gate is +opt-in per plugin rather than enforced across the board: a `plugin.toml` copied +from an older template that never added the field sails through unchecked. +**Always declare `acadrust_source`** (see the worked example below) so the gate +actually applies to your plugin; don't rely on the host to catch its absence. + ### `PluginManifest` ```rust @@ -202,11 +216,14 @@ struct ExampleModule; impl CadModule for ExampleModule { fn id(&self) -> &'static str { "example" } fn title(&self) -> &'static str { "Example" } - fn ribbon_groups(&self) -> Vec { - vec![RibbonGroup { title: "Demo", tools: vec![RibbonItem::LargeTool(ToolDef { + // Returns a slice, not an owned Vec — build once into a static and hand + // back a reference, rather than rebuilding (and dropping) it every call. + fn ribbon_groups(&self) -> &[RibbonGroup] { + static GROUPS: std::sync::OnceLock> = std::sync::OnceLock::new(); + GROUPS.get_or_init(|| vec![RibbonGroup { title: "Demo", tools: vec![RibbonItem::LargeTool(ToolDef { id: "EX_HELLO", label: "Hello", icon: IconKind::Glyph("◆"), event: ModuleEvent::Command("EX_HELLO".to_string()), - })]}] + })]}]}) } } @@ -235,8 +252,12 @@ api_version = 5 ribbon_order = 50 command_prefixes = ["EX_"] xdata_apps = [] -# Filled with the exact compiler output while staging the release asset. +# Both filled at release-build time, not by hand — see the CI step below. +# Declare both; omitting acadrust_source silently skips that half of the +# compatibility check instead of failing it (see "Build-time ABI metadata" +# above), which defeats the point of declaring either field. rustc_version = "rustc 1.98.0 (hash date)" +acadrust_source = "git+https://github.com/HakanSeven12/cadcodec.git?rev=#" ``` The full, buildable scaffold is in [`docs/plugin-template/`](plugin-template); @@ -255,6 +276,27 @@ tool fires `ModuleEvent::Command("EX_FOO")`, which round-trips to tool request a native file picker; on selection the host dispatches `" "` back with the path's original case preserved. +**Known gap: typed/pasted commands still get uppercased even when a leading +`>` is used to keep spaces literal.** The interactive command line +(`Message::CommandInput` in `src/app/update/mod.rs`) unconditionally calls +`s.to_uppercase()` on typed input unless a free-form text prompt is active +(`is_free_text_active()` — cell text, `TEXT`/`MTEXT` bodies). The `>` prefix +and `literal_spaces` mode (added since this was first found) correctly stop +a typed space from auto-submitting the line early — solving multi-word +arguments — but neither one skips the uppercase call, so a plugin command +needing a case-sensitive argument (a file path with mixed case, an embedded +expression, anything not meant to be all-caps) still gets mangled when +typed or pasted directly, even with `>`. Automation (`--serve`/`--mcp`) +bypasses this input path entirely and is unaffected — this is specifically +a typed/pasted-into-the-command-line issue. `ModuleEvent::PluginFileDialog` +sidesteps it for file paths (dispatches back with original case intact, as +noted above) but there's no equivalent for a plugin command that needs +case-sensitive *typed* text more generally. Fix direction: extend whatever +condition currently gates `literal_spaces`/`>` to also skip the uppercase +call, rather than treating "keep spaces literal" and "keep case literal" as +solved by the same escape when a plugin command may need one without the +other. + ### Interactive (click-to-place) commands For tools that collect points — placing a structure, drawing a pipe — call @@ -306,7 +348,18 @@ plugin.toml ``` A GitHub Actions matrix workflow (see the example repo / template) cross-builds -and uploads these on a `v*` tag. +and uploads these on a `v*` tag. It also fills `plugin.toml`'s `rustc_version` +and `acadrust_source` placeholders from the CI run's actual toolchain and +resolved dependency, rather than hand-editing them per release — see +`docs/plugin-template/.github/workflows/release.yml` for the exact `sed` +substitutions, including reading `acadrust_source` out of `Cargo.lock`'s +resolved `acadrust` package entry (`source = "git+...#"`) since that's +the same value the host embeds at its own build time. The template's CI pins +`rust-toolchain@stable`, which is a floating target — a real release pipeline +should pin the exact toolchain version the host was built with (check +`crates/ocs_plugin_api/src/version_info.rs`'s embedded value, or the host's +own CI config) rather than trusting "whatever's current stable today" to +keep matching it. --- @@ -383,6 +436,24 @@ dynamic libraries: - **Enable/disable** — toggles a loaded plugin's ribbon tab + dispatch without uninstalling. +**Known gap: one linked repo is treated as one plugin's release history, even +when it hosts several.** `fetch_releases`/`fetch_release_info` +(`src/plugin/marketplace.rs`) fetch and parse *every* release under a linked +`owner/repo` into one flat `Vec`, with no filter on the parsed +manifest's own `id` field — which is already available at that point, since +each release's `plugin.toml` is downloaded and parsed to build `ReleaseInfo` +in the first place. A repo that ships more than one plugin (tag-namespaced +releases like `pluginA-v1.0.0`, `pluginB-v1.0.0`) will surface every other +plugin's releases as if they belonged to whichever one the user is looking +at, and any future "is there a newer version" check that walks this same +list by date/tag would need the same fix. A user can still install correctly +by picking the right tagged release (each release's own bundled `plugin.toml` +is what actually gets installed), but the discovery/version list itself +doesn't distinguish plugins sharing a repo. Fix direction: filter +`fetch_release_info`'s result (or its caller) to releases whose manifest +`id` matches the plugin currently being viewed/installed, the same `id` +`ReleaseInfo` already carries. + --- ## Compatibility & ABI @@ -391,9 +462,16 @@ Plugins are loaded as `cdylib`s by a plugin-runner child process. The host spawn this child from its own executable (`--ocs-plugin-runner` mode), so the runner and host always share the same `ocs_plugin_api` build. The runner checks `ocs_plugin_api_version` before any plugin code runs. Each plugin runs in its -own OS process, so the host is protected from plugin crashes and memory -corruption. Process isolation removes the need for the host and plugin to share -a Rust toolchain ABI beyond the stable `ocs_plugin_api` contract. +own OS process, so a plugin crash or memory-corruption bug cannot take down the +host process itself — but process isolation alone does **not** mean a +toolchain mismatch is safe to ignore: the plugin and host still exchange data +across the shared `ocs_plugin_api` types over IPC, and a struct-layout or ABI +difference between toolchain versions could corrupt that exchange in ways +subtler than an immediate crash. That's why, independently of process +isolation, API v4+ *also* enforces an exact `rustc_version` match (and, +correctly declared, `acadrust_source` — see above) before loading a plugin at +all, rather than relying on isolation to make a mismatch merely "someone +else's problem." A future hardening step is a `#[repr(C)]` vtable (a true C ABI) so binaries built by any toolchain interoperate — required before trusting prebuilt binaries from @@ -414,6 +492,12 @@ Done: reinstall / uninstall, enable/disable. - [x] Interactive command round-trip over IPC (prompt, point/enter/object-pick). - [x] Spawn and per-call IPC timeouts so a stuck runner cannot freeze the host. +- [x] External automation (`--mcp` — local stdio MCP/JSON-RPC server for AI + clients; `--serve` — headless JSON automation server; both drive OCS + from a separate process without the GUI, per `src/cli.rs`/`src/mcp.rs`/ + `app::serve`). This closes the "drive OCS headless from a process" goal + of issue #29; this line previously and incorrectly listed it as not yet + started. Next: @@ -421,7 +505,9 @@ Next: - [ ] `#[repr(C)]` vtable / strict handshake for cross-toolchain binaries. - [ ] Trust: checksums / signatures before spawning plugin processes. - [ ] Interchange (LandXML / SWMM) and live `on_entity_committed` hooks. -- [ ] External automation API (drive OCS headless from a process) — issue #29. +- [ ] Close the `acadrust_source` enforcement gap noted above (see + "Build-time ABI metadata") — an omitted field is currently accepted as + compatible instead of rejected, unlike `rustc_version`. --- diff --git a/docs/plugin-template-v2/DESIGN.md b/docs/plugin-template-v2/DESIGN.md index 8a30078cf..502db63a0 100644 --- a/docs/plugin-template-v2/DESIGN.md +++ b/docs/plugin-template-v2/DESIGN.md @@ -64,8 +64,12 @@ From the workspace root: cargo build -p plugin-template-v2 ``` -The resulting cdylib plus `plugin.toml` can be installed into the host's -plugins folder (see `docs/plugin-architecture.md`). +The resulting cdylib plus `plugin.toml` (checked in alongside this file) can +be installed into the host's plugins folder (see `docs/plugin-architecture.md`). +`plugin.toml`'s `rustc_version`/`acadrust_source` placeholders are filled by +CI in `docs/plugin-template` (v1) — this template has no release workflow of +its own, so fill them by hand (`rustc --version`, and `Cargo.lock`'s resolved +`acadrust` package `source` line) before installing a real build. --- diff --git a/docs/plugin-template-v2/plugin.toml b/docs/plugin-template-v2/plugin.toml new file mode 100644 index 000000000..d63c12011 --- /dev/null +++ b/docs/plugin-template-v2/plugin.toml @@ -0,0 +1,30 @@ +[plugin] +id = "com.example.plugin-template-v2" +name = "Plugin Template v2" +version = "0.1.0" +description = "Demonstrates read/write object round-trips with the host." +author = "Your Name" +license = "GPL-3.0-only" +repository = "your-account/your-plugin-repo" + +[opencad] +api_version = 5 +# Both filled at release-build time from the actual compiler and Cargo.lock's +# resolved `acadrust` source — see docs/plugin-template's release.yml for the +# exact substitution. Do not fill these in by hand. Declare acadrust_source +# even though it's optional: the host's compatibility check only runs at all +# if this field is present, so an omitted field is silently treated as +# compatible rather than rejected (see plugin-architecture.md's "Build-time +# ABI metadata"). This template ships without its own CI workflow (see +# DESIGN.md) — copy docs/plugin-template's release.yml if you turn this into +# a real, released plugin. +rustc_version = "__RUSTC_VERSION__" +acadrust_source = "__ACADRUST_SOURCE__" +ribbon_order = 100 +command_prefixes = [ + "COUNT_SURVEY_POINTS", + "ADD_SURVEY_POINT", + "MARK_FIRST_SURVEY_POINT", + "COUNT_MARKED_SURVEY_POINTS", +] +xdata_apps = ["SURVEYMARK"] diff --git a/docs/plugin-template/.github/workflows/release.yml b/docs/plugin-template/.github/workflows/release.yml index 76588a1eb..3b3b1e45e 100644 --- a/docs/plugin-template/.github/workflows/release.yml +++ b/docs/plugin-template/.github/workflows/release.yml @@ -29,6 +29,14 @@ jobs: runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 + # NOTE: @stable floats to whatever's current when this job runs, which + # is NOT the same thing as "matches the host's rustc exactly" — the API + # v4+ load check compares plugin.toml's rustc_version to the host's + # build-time compiler byte-for-byte (see plugin-architecture.md's + # "Build-time ABI metadata"). Pin this to the host's actual toolchain + # version (check crates/ocs_plugin_api/src/version_info.rs's embedded + # value, or the host's own CI config) before a real release, or your + # binary may be refused at load with a version-mismatch error. - uses: dtolnay/rust-toolchain@stable - run: cargo build --release - name: Stage release asset @@ -36,7 +44,11 @@ jobs: run: | mkdir -p dist cp target/release/*.${{ matrix.ext }} "dist/${{ matrix.asset }}" - sed "s|__RUSTC_VERSION__|$(rustc --version)|" plugin.toml > dist/plugin.toml + acadrust_source=$(grep -A3 '^name = "acadrust"' Cargo.lock | grep '^source' | head -1 | sed 's/^source = "//;s/"$//') + sed \ + -e "s|__RUSTC_VERSION__|$(rustc --version)|" \ + -e "s|__ACADRUST_SOURCE__|$acadrust_source|" \ + plugin.toml > dist/plugin.toml - uses: softprops/action-gh-release@v2 with: files: | diff --git a/docs/plugin-template/plugin.toml b/docs/plugin-template/plugin.toml index d07f0b702..340ab99e1 100644 --- a/docs/plugin-template/plugin.toml +++ b/docs/plugin-template/plugin.toml @@ -10,6 +10,13 @@ repository = "your-account/your-plugin-repo" [opencad] api_version = 5 rustc_version = "__RUSTC_VERSION__" +# Both placeholders are filled by release.yml at build time (from `rustc +# --version` and Cargo.lock's resolved `acadrust` source respectively) — do +# not fill these in by hand. Declare acadrust_source even though it's +# optional: the host's compatibility check only runs at all if this field is +# present, so an omitted field is silently treated as compatible rather than +# rejected (see plugin-architecture.md's "Build-time ABI metadata"). +acadrust_source = "__ACADRUST_SOURCE__" ribbon_order = 60 command_prefixes = ["MP_"] xdata_apps = ["MYPLUGIN_RECORD"]