docs(plugin): fix inaccuracies and flag real gaps found building a plugin

Found while building an out-of-tree scripting plugin from scratch against
this doc and the actual ocs_plugin_api/host source. Two categories:

Fixed outright (docs/examples didn't match the real code):
- The worked CadModule example returned Vec<RibbonGroup> from
  ribbon_groups(); the real trait requires &[RibbonGroup] - the example
  wouldn't compile. Fixed with the OnceLock pattern the host's own
  benchmark code already uses.
- Reconciled a contradiction between "process isolation removes the need
  for a shared Rust ABI" and the exact-rustc_version requirement described
  a few paragraphs earlier in the same doc.
- The roadmap's "Next" list still had external automation as unstarted;
  --mcp/--serve are real and operational (src/cli.rs, src/mcp.rs).
- Added the missing plugin.toml to docs/plugin-template-v2 (its own
  DESIGN.md already claimed one could be installed).
- Documented that plugin-template's CI floats rust-toolchain@stable, which
  doesn't guarantee matching the host's exact rustc - added acadrust_source
  to both templates' plugin.toml plus the release.yml step that fills it
  from Cargo.lock's resolved acadrust source, alongside the existing
  rustc_version fill.

Flagging for your call, not fixed here (found the behavior, not sure it's
the intended one, prefer to ask than guess at a fix):
- acadrust_compatible()'s ABI gate only runs at all if plugin.toml declares
  acadrust_source - an undeclared field is silently treated as compatible,
  unlike rustc_version (undeclared = incompatible). The same shortcut
  appears in two places: ExternalPlugin::acadrust_compatible
  (src/plugin/external.rs) and the inline check inside fetch_release_info
  (src/plugin/marketplace.rs). If this is deliberate (e.g. back-compat with
  manifests from before the field existed), no action needed beyond a
  comment; if not, both call sites need the same fix.
- The command line still uppercases typed/pasted input even with the `>`
  literal-spaces prefix (Message::CommandInput, src/app/update/mod.rs) -
  `>` only stops a space from auto-submitting the line, it doesn't skip
  `s.to_uppercase()`. A plugin command needing a case-sensitive typed
  argument has no escape today short of automation.
- fetch_release_info (src/plugin/marketplace.rs) returns every release
  under a linked repo with no filter on the manifest's own id field (which
  it already parses to build ReleaseInfo) - a repo hosting more than one
  plugin under tag-namespaced releases surfaces all of them mixed
  together in one plugin's version list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Felix RiestraandClaude Sonnet 5 committed 2026-09-16 03:37:54 +02:00
1 parent 1450fdeedf
commit bd49bd66d4
5 files changed
+153 -14

No files matched your search

+97 -11
View File
@@ -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<RibbonGroup> {
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<Vec<RibbonGroup>> = 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=<short>#<full-40-char-commit>"
```
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
`"<command> <path>"` 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+...#<hash>"`) 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<ReleaseInfo>`, 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`.
---
+6 -2
View File
@@ -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.
---
+30
View File
@@ -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"]
+13 -1
View File
@@ -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: |
+7
View File
@@ -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"]