Prove every ext-state write by reading the key back, so no guard is constant-true

SetProjExtState's return describes the whole extname. The persist and the instrument's publish now re-read their key; both refusals can finally fire.
This commit is contained in:
2026-08-02 13:14:49 -04:00
parent 0afb885987
commit 25390d5253
14 changed files with 231 additions and 112 deletions
+1 -1
View File
@@ -103,7 +103,7 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h
## Modules
- `reaper_bridge` — READ-ONLY bank consumer: receives bank snapshots from the extension and exposes them as a read-only view. **Never writes to the extension's bank** — this is a load-bearing invariant; no mutation path exists in this module. It owns TWO prefix-guarded ext-state write entry points, `writeUsageExtState` (`rsusage_`) and `writeBakeExtState` (`rsbake_`), each refusing every other key; neither weakens the read-only-*bank* invariant, because neither payload is bank state and `banks`/`view`/`tail`/`assign` stay structurally unwritable. It also owns the bake crossing — `extensionActionAvailable` / `invokeExtensionAction` (`NamedCommandLookup` + `Main_OnCommandEx` with `getReaperParent(3)`, the instance's OWN project tab, as `proj` — a request, not a DAW-verified guarantee; see the header) and `projectTempoBpm`.
- `reaper_bridge` — READ-ONLY bank consumer: receives bank snapshots from the extension and exposes them as a read-only view. **Never writes to the extension's bank** — this is a load-bearing invariant; no mutation path exists in this module. It owns TWO prefix-guarded ext-state write entry points, `writeUsageExtState` (`rsusage_`) and `writeBakeExtState` (`rsbake_`), each refusing every other key; neither weakens the read-only-*bank* invariant, because neither payload is bank state and `banks`/`view`/`tail`/`assign` stay structurally unwritable. Both PROVE the write by reading the key back (`wire::extStateWriteLanded`) — `SetProjExtState`'s own return cannot speak for one key, so testing it was a guard that could never fire, and the bake's "could not publish" refusal was consequently unreachable. It also owns the bake crossing — `extensionActionAvailable` / `invokeExtensionAction` (`NamedCommandLookup` + `Main_OnCommandEx` with `getReaperParent(3)`, the instance's OWN project tab, as `proj` — a request, not a DAW-verified guarantee; see the header) and `projectTempoBpm`.
- `reasampler_processor` (`shell/instrument/`: `reasampler_processor.cpp` lifecycle + `process()`, `processor_state.cpp` component-state I/O + UI-thread parameter accessors, `processor_reload.cpp` the off-audio-thread `reloadInstrument`/publish family — Q-W2v, T4-12 split; `process()` and its per-block work stay ONE TU on purpose, no cross-TU call on the per-sample path) — VST3 `SingleComponentEffect` shell: declares event-input bus + **permanently stereo** output (GA fix: dynamic mono↔stereo bus renegotiation deleted; `ChannelMode` is now decode-only), marshals MIDI note-on/off into the VoiceEngine, renders audio; owns off-audio-thread `reloadInstrument` + atomic pointer swap so `process()` does no allocation, no file I/O, no bridge calls. The instance state is `{loaded capture id, one InstrumentParams}`, and `reloadInstrument` resolves + decodes exactly that one capture into the `SampleData` the engine plays. **Self-contained playback (pS):** `ComponentState` v10 adds a `SampleRefs` table — per referenced sample, a project-relative path + decode intrinsics (root, loop, channels, displayName); `reloadInstrument` decodes directly from `SampleRefs`, bank-free (plays with the extension absent). The bank/bridge is a browser source: loading a capture copies its reference in; the reopen-heal timer + poll-to-play apparatus are removed. `retireIdleDrain()` retires fully-idle drain snapshots on the UI-timer cadence. Voice-param edits (`setVoiceCount`/`setVoiceMode`/`setMonoTrigger`) rebuild the engine from the already-decoded `SampleData` via the drain-slot swap — no bank re-read, no WAV re-decode, no audible cut to ringing tails. **FB1:** applies the post-mixer `masterGainLinear` (from `ComponentState` v8) as a per-sample ramp over the summed output — no zipper noise. **GA v9:** `channelModeExplicit_` flag persisted; `channelModeFor()` auto-defaults the mode from the loaded capture's channel count when the flag is not set. **pS:** `ComponentState` bumped v9→v10 (`SampleRefs` table); pre-v10 blobs lift to empty refs and re-save self-contained. **pS-usage:** publishes instance usage (held `SampleRefs` paths) to `rsusage_<instanceGuid>` at the tail of `reloadInstrument` (off audio thread) via `reaper_bridge::writeUsageExtState`; `ComponentState` bumped v10→**v11** (`instanceGuid` field); pre-v11 blobs mint guid on first publish.
- `reasampler_editor` — VST3 `IPlugView` LICE editor shell: hosts a LICE-drawn child window; the Sample face is home and Browse is a modal picker over it. Split on the Sample face's BAND axis, mirroring the pure `sample_bands` allocator: `editor_session` (session/bridge state, caches, commit-and-reload), `editor_controls` (the ONE `faceLayout` band resolve every paint and hit-test path shares, the node-drag bounds, the value labels, and the per-instance controls the parameter set does not carry — the parameter-set binding itself is the pure `core/instrument/ui/deck_values` module this only adapts int ids onto), `editor_models` (the orthogonal half: which stored struct each transient editor selection names — the staged-envelope pack/unpack, the drawn contour, and the three velocity curves), then matching paint and input sets — `editor_paint`/`editor_input` (dispatch + drag router + hover dispatch), `_chrome`, `_waveform`, `_deck` — plus the two band-independent surfaces (`_browse` for the modal picker, `_curve` for the velocity-curve popup) and `editor_platform` (IPlugView/Win32 window plumbing). Shared internals in `editor_internal.h`, no TU of its own. Drop-onto-editor ingest is NOT shipped (deferred).
- `reasampler_embed` — implements `IReaperUIEmbedInterface` so the instrument draws inline in the TCP/MCP without a plugin-owned HWND; delegates layout to `embed_strip`. A read-only readout: the loaded capture across the keyboard span with its root marked, plus the activity level. It takes no mouse input (there is nothing on the strip to select).
+6 -1
View File
@@ -186,8 +186,13 @@ BakeChainResult runBake(ReaSamplerProcessor& processor) {
const std::string key = bakeKeyFor(instanceGuid);
RequestKeyGuard keyGuard(bridge, key);
// The bridge PROVES this by reading the key back, so a false here is real: the
// extension would find nothing to land. It does not say which of the three ways failed,
// so neither does this sentence.
if (!bridge.writeBakeExtState(key, wire::encodeBakeRequest(request)))
return fail("could not publish the bake request");
return fail("the bake request under " + key +
" could not be confirmed -- it was either never written or did not "
"read back as written, so the extension has nothing to land");
// Synchronous: the extension's landing runs to completion inside this call and writes
// its outcome back over the same key before returning.
+19 -9
View File
@@ -2,6 +2,8 @@
#include "shell/instrument/reaper_bridge.h"
#include <optional>
#include <string>
#include <vector>
#include "core/instrument/map/bridge_marshal.h"
@@ -112,7 +114,9 @@ std::optional<std::string> ReaperBridge::readReasamplerExtState(const std::strin
bool ReaperBridge::writeGuarded(const std::string& key, const std::string& value,
const char* requiredPrefix) {
if (!setProjExtState_ || !hostApp_) return false;
// getProjExtState_ is required too: without it the write could not be proven, and an
// unprovable write may not be reported as a landed one.
if (!setProjExtState_ || !getProjExtState_ || !hostApp_) return false;
// Read-only-BANK guard: this module writes the two sanctioned per-instance prefixes
// and nothing else. Any other key is refused rather than widening the instrument's
// write surface (banks/view/tail/assign stay extension-owned).
@@ -121,14 +125,20 @@ bool ReaperBridge::writeGuarded(const std::string& key, const std::string& value
auto* reaper = static_cast<Steinberg::IReaperHostApplication*>(hostApp_);
void* proj = reaper->getReaperParent(3); // null = current project (same as reads)
// SetProjExtState returns the size of the extname's state — after storing a
// non-empty value that's necessarily > 0, so <= 0 means the write did not land (the
// publish path retries next reload tick; a silent drop would leave holds unprotected).
// A deliberate CLEAR (empty value) shrinks the state and can legitimately return 0,
// so it is reported as landed.
const int rv =
setProjExtState_(proj, kProjExtNamespace(), key.c_str(), value.c_str());
return value.empty() ? true : rv > 0;
// The return is DELIBERATELY discarded: it describes the whole extname's state, not
// this key, so testing it was a guard that could not fire for the bake publish
// (wire::extStateWriteLanded owns the reasoning). The read-back below is the proof.
setProjExtState_(proj, kProjExtNamespace(), key.c_str(), value.c_str());
const auto back = wire::readProjExtStateGrowing([&](char* buf, int cap) {
return getProjExtState_(proj, kProjExtNamespace(), key.c_str(), buf, cap);
});
if (back.status == wire::GrowingExtStateRead::Status::Overflow)
return false; // could not check -> report unconfirmed, never a claimed success
return wire::extStateWriteLanded(
value, back.status == wire::GrowingExtStateRead::Status::Complete
? std::optional<std::string>(back.value)
: std::nullopt);
}
bool ReaperBridge::writeUsageExtState(const std::string& usageKey,
+8 -2
View File
@@ -47,10 +47,16 @@ public:
// The instrument's TWO sanctioned ext-state write surfaces, each accepting exactly one
// key prefix and refusing every other key. That structural refusal is what keeps the
// read-only-BANK invariant intact — banks/view/tail/assign stay unwritable from here —
// and neither payload is bank state. Both return true iff the write landed (the
// SetProjExtState return is checked) and neither is RT-safe: the call sites are the
// and neither payload is bank state. Neither is RT-safe: the call sites are the
// off-audio-thread reload path and the editor's UI tick.
//
// Both return true iff the key READ BACK as exactly the value written (an empty value
// is a clear, which lands as an absent-or-empty key). SetProjExtState's own return
// cannot answer that — wire::extStateWriteLanded owns why, and testing it here was a
// guard that could never fire. A `false` does not distinguish a write that was never
// issued (unconnected host, refused prefix) from one that did not take or could not be
// checked; a caller must not name one of the three.
//
// Neither marks the project dirty. A usage change always rides a component-state change
// that already does; a bake request is transient and is cleared in the same tick.