[dotCooCoo]

What and why

PLUGIN_CHANGES keeps installs and removals in one process in order, but two processes that use the same Plug-ins folder (the app and vectorcraft-cli, or two app instances) wrote it without any lock. Two faults followed:

• After plugin.install committed a module, it wrote the module, then grants.json, then the disabled list, unlocked. A second process installing the same id between those writes could leave one install's module next to the other install's grant, so the folder held a grant for bytes it was never given for, or skipped an approved module at the next start.
• store_grant read grants.json, changed one entry and replaced the whole file. Two processes granting different ids could both read the old list, and the later write dropped the earlier grant while both calls reported success. Plug-in settings and cache files had the same read-change-write pattern, guarded only by the in-process PLUGIN_DATA_LOCK.

This change makes every window and process lock one file per folder, the existing disabled.lock, for the whole of each change. These are VectorCraft's own rules: a stored module keeps its own grant, and one write does not undo another.

• crates/engine/src/store.rs: lock_disabled becomes lock_store, which opens disabled.lock for reading and writing without truncating it and takes an exclusive File::lock (stable since Rust 1.89; the workspace minimum is 1.95). The held lock is a StoreTurn, and the store writers (module, put-back, removal, grant, disabled list, moving kept settings back, removing plug-in storage) take it as an argument and write nothing without it, so no caller takes the lock twice. Plug-in settings and cache changes take the folder lock before PLUGIN_DATA_LOCK. Loading the folder at startup takes it once across reading the grants and modules and removing stale grants. On wasm32 there is no file lock, and the web build attaches no folder, as before.
• crates/engine/src/cmd/plugin.rs: plugin.install and plugin.remove take the lock right after PLUGIN_CHANGES, before reading the stored approval, and hold it through the module, grant, put-back and disabled-list writes. plugin.setEnabled takes it before changing the list in memory.
• The lock order is PLUGIN_CHANGES, then the folder lock, then the registry locks, then PLUGIN_DATA_LOCK.
• When disabled.lock can't be locked, plugin.install and plugin.remove now fail with an error and change nothing; before, they wrote the folder without a lock. plugin.setEnabled keeps answering saved: false in that case, and loading the folder writes nothing to it.
• The params text of plugin.install and plugin.remove, docs/plugins.md, docs/mcp.md and one line in docs/target-app-parity.md describe the lock.

How it was tested

New tests in crates/engine/src/tests/plugins.rs. The two install and grant races use a second session with its own plugin_dir and no load, so it does not take PLUGIN_CHANGES; File::lock locks per handle, so the threads exercise the file lock. The install and grant tests wait on channels and a test hook that reports when a writer waits for the lock. The settings test waits on lines that the child process prints. None of the tests sleeps.

• installs_from_two_processes_into_one_store_keep_each_module_with_its_grant: one install stops between writing its module and its grant while a second process writes another module and its grant for the same id; the grant stored for the id matches the hash of the module stored for it.
• grants_written_by_two_processes_into_one_store_keep_both: one grant write stops after reading grants.json while another id is granted; both grants are kept.
• settings_changed_by_two_processes_keep_both: a child test process changes one setting of a plug-in while the parent changes another; both are kept.
• a_store_that_cant_be_locked_refuses_installs_and_removals: with disabled.lock replaced by a folder, plugin.install and plugin.remove return an error and leave the registry and the folder as they were.

The three race tests fail without the change (with lock_store returning before it locks: the stored grant's hash differs from the module's, one grant is lost, one setting is lost), and the last one fails when the install ignores the lock error. The store_files helper in the same file no longer counts disabled.lock, and hostile_storage_arguments_never_crash in crates/engine/tests/plugin_fuzz.rs allows the lock file in the folder.

Focused checks on Linux, on the final tree, on main at 1e4e3534f5 (the branch merges cleanly with it):

• cargo test -p vectorcraft-engine --lib tests::plugins::: 130 passed, 0 failed.
• cargo test -p vectorcraft-engine --test plugin_safe_mode: 17 passed; --test plugin_clear_cache: 1 passed.
• PROPTEST_CASES=1 cargo test -p vectorcraft-engine --test plugin_fuzz hostile_storage_arguments_never_crash: 1 passed.
• cargo clippy -p vectorcraft-engine --all-targets -- -D warnings: no warnings.
• cargo check --workspace --tests and cargo check -p vectorcraft-engine --target wasm32-unknown-unknown: no errors.
• cargo xtask brands, layers (30 crates) and assets (337 files): OK.
• cargo fmt --all -- --check reports only crates/svg/src/lib.rs:491, which this branch does not change; main at 1e4e3534f5 has the same diff.
• cargo xtask ci runs in this pull request's CI.

No Windows-only code changed. On Windows, File::lock uses LockFileEx. This pull request's CI runs the tests on Linux only, so it does not exercise the lock on Windows.

Checklist

• cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
• No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
  unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
  messages. Errors are returned as Result and shown to the user
  (why and how).
• Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
• Every new asset has a row in ASSETS.md.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 49 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: storytold/vectorcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: a47d7ee
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 73dfa796-5aa8-4d36-ba8b-d6c0363ab020

Commits

Reviewing files that changed from the base of the PR and between a47d7ee and baf9905.

Files selected for processing: 7

• crates/engine/src/cmd/plugin.rs
• crates/engine/src/store.rs
• crates/engine/src/tests/plugins.rs
• crates/engine/tests/plugin_fuzz.rs
• docs/mcp.md
• docs/plugins.md
• docs/target-app-parity.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[bflatastic]

Merged, thanks @dotCooCoo! It passed its CI checks.