Persist the reclaimer's FX release so a reload cannot re-drive a spent record, and report the FX-restore drops it was swallowing

src/app/main.cpp

@@ -227,7 +227,7 @@ static void OnTimer()
     // the view model wholesale, and the applyMode above is what re-derives the queue
     // from it, so draining first would spend a tick against the outgoing model.
     if (reasampler::g_fxReclaimPending)
-        reasampler::fxReclaimTick(g_session.view(), nullptr);
+        reasampler::fxReclaimTick(g_session, nullptr);
 
     reasampler::bankPanelRefresh();  // cheap fingerprint compare; no-op when unchanged/closed
 }

src/shell/view/fx_reclaim.cpp

@@ -10,12 +10,14 @@
 #include <string>
 #include <vector>
 
+#include "shell/persist/session.h"  // saveViewStateOnly — the release's durable half
 #include "shell/view/view.h"        // modeApplyInProgress
 #include "shell/view/view_park.h"   // fxGuidString — the one FX-identity read
 #include "shell/view/view_tracks.h"
 
 #define REAPERAPI_MINIMAL
 #define REAPERAPI_WANT_EnumProjects
+#define REAPERAPI_WANT_ShowConsoleMsg
 #define REAPERAPI_WANT_TrackFX_GetCount
 #define REAPERAPI_WANT_TrackFX_GetOffline
 #define REAPERAPI_WANT_TrackFX_SetOffline
@@ -68,7 +70,7 @@ std::set<std::string> liveTrackGuids(ReaProject* proj) {
 // The live half of drainTick's Host contract (see fx_reclaim.h).
 struct ReclaimHost {
     ReaProject*        project = nullptr;
-    ViewModeModel* model   = nullptr;
+    ReaSamplerSession* session = nullptr;
 
     // Monotonic and sub-millisecond: the budget below discriminates at 30ms, which
     // GetTickCount's ~16ms granularity cannot do. No wraparound to guard against, so
@@ -111,10 +113,41 @@ struct ReclaimHost {
     }
 
     // Reached only after the drain has unwound and the queue has proved its generation
-    // did not move, so `model` is still the object the queue was derived against.
+    // did not move, so the session still holds the model the queue was derived against.
-    void releaseFx(const std::string& guid) { model->releaseSnapshotFx(guid); }
+    void releaseFx(const std::string& guid) { session->view().releaseSnapshotFx(guid); }
+
+    // saveViewStateOnly writes the ACTIVE project and takes none, so re-read that rather
+    // than trust the project this tick pinned: a write pumps, and a tab switch inside one
+    // would otherwise land this model in the wrong project's ext state. Skipping is
+    // self-healing — coming back to the tab reloads the model from the record still on
+    // disk, and the re-derived online writes nothing and releases again. Promptless,
+    // unlike design_view_actions' persistViewState, which can open a modal Save-As.
+    void persistModel() {
+        if (EnumProjects(-1, nullptr, 0) != project) return;
+        session->saveViewStateOnly();
+    }
 };
 
+// The drop set the last console report named, and the project it named it for. Same
+// one-per-changed-set memo as view_park's reportExposedTracks, which owns the reasoning;
+// here the repeat it swallows is an aborted tick's re-derivation resolving identically.
+// The pointer is COMPARE-ONLY and never dereferenced.
+ReaProject*               g_reportedDropOwner = nullptr;
+std::vector<TrackFxDrops> g_reportedDrops;
+
+void reportDrops(ReaProject* proj, const std::vector<TrackFxDrops>& drops) {
+    if (!shouldReportDrops(proj == g_reportedDropOwner, g_reportedDrops, drops)) return;
+    g_reportedDropOwner = proj;
+    g_reportedDrops     = drops;
+
+    FxRestoreDrops total;
+    for (const TrackFxDrops& d : drops) total.add(d.drops);
+    const std::string msg = describeFxRestoreDrops(total, static_cast<int>(drops.size()));
+    // "!SHOW:" so a background tick cannot force the console open
+    // (reaper_plugin_functions.h:6536).
+    if (!msg.empty()) ShowConsoleMsg(("!SHOW:" + msg).c_str());
+}
+
 // Every path that stops scheduling ticks leaves the same state behind: no queue, no pin,
 // nothing pending. The debounce clock is NOT part of it — see fx_reclaim.h.
 void goIdle() {
@@ -137,7 +170,7 @@ void invalidateFxReclaim() {
 
 bool fxReclaimDrainInProgress() { return g_draining; }
 
-void fxReclaimTick(ViewModeModel& model, ReaProject* proj) {
+void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
     if (g_draining) return;
     // The model is mid-rewrite and its records are being consumed out from under any
     // plan derived from it. The apply invalidates on its way out, so nothing is lost.
@@ -161,12 +194,14 @@ void fxReclaimTick(ViewModeModel& model, ReaProject* proj) {
     // invalidated, so a REAPER quit mid-drain simply re-derives and finishes on reopen.
     if (g_queue.stale()) {
         g_project = project;
+        const ViewModeModel& model = session.view();
         g_queue.adopt(makeQueue(
             view::desiredFxState(model, model.activeModeId(), liveTrackGuids(project))));
     }
 
-    ReclaimHost host{project, &model};
+    ReclaimHost host{project, &session};
     const ReclaimRunResult run = g_queue.run(host);
+    reportDrops(project, run.drops);
     if (!run.aborted && run.settled) goIdle();
 }
 

src/shell/view/fx_reclaim.h

@@ -16,6 +16,8 @@ class ReaProject;
 
 namespace reasampler {
 
+class ReaSamplerSession;
+
 // --- tuning ------------------------------------------------------------------
 
 // A track's OFFLINE work fires only after it has been continuously parked this long.
@@ -116,6 +118,25 @@ inline int nextEligible(const std::vector<ReclaimItem>& items, const ParkClock&
 
 // --- the drain ----------------------------------------------------------------
 
+// One ONLINE item's captured state the chain had no home for. An OFFLINE item can
+// never produce one: its targets are the live chain's own slots, so every op resolves.
+struct TrackFxDrops {
+    std::string    trackGuid;
+    FxRestoreDrops drops;
+
+    bool operator==(const TrackFxDrops& o) const {
+        return trackGuid == o.trackGuid && drops == o.drops;
+    }
+};
+
+// True when a drop set is worth printing: any change of owner or of the set itself.
+// The one-per-changed-set memo this gates is view_park's `shouldReport`/
+// `reportExposedTracks`, which states the reason; the shell holds the memo.
+inline bool shouldReportDrops(bool sameOwnerAsLast, const std::vector<TrackFxDrops>& last,
+                              const std::vector<TrackFxDrops>& now) {
+    return !(sameOwnerAsLast && last == now);
+}
+
 // What one tick did. `settled` means every item is complete and the caller may go idle
 // until the next invalidation. An item merely waiting out its debounce is NOT settled —
 // going idle there would strand its reclaim for the rest of the session.
@@ -128,6 +149,12 @@ struct ReclaimTickResult {
     // what a re-entered load can replace mid-drain, so only the owner may commit it —
     // see FxReclaimQueue::run.
     std::vector<std::string> released;
+
+    // Per track that lost any, what the tick could not place. Carried out for the shell
+    // to print through core/view/fx_offline's describeFxRestoreDrops — a plugin deleted
+    // while its track was parked is a degrade the user has to repair by hand, and
+    // swallowing it leaves no trace at all.
+    std::vector<TrackFxDrops> drops;
 };
 
 // One tick of the convergence loop. Templated on `Host` so the ordering, budget and
@@ -144,6 +171,9 @@ struct ReclaimTickResult {
 //                                                        called by FxReclaimQueue::run
 //                                                        after the drain has unwound,
 //                                                        never from inside the drain
+//   void   persistModel()                                make the releases just applied
+//                                                        durable; run calls it ONCE per
+//                                                        tick and only after a release
 //
 // `items` and `clock` must be the caller's OWN storage, not shared state a re-entered
 // call could reach: a write pumps the message loop. FxReclaimQueue::run is what makes
@@ -181,9 +211,14 @@ ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, H
             const FxRestoreResolution resolved = view::resolveFxWork(item.work, liveFxGuids);
             for (const FxOfflineWrite& w : resolved.writes)
                 if (host.writeOffline(item.work.trackGuid, w.fxIndex, w.offline)) ++result.writes;
+            if (resolved.drops.total() > 0)
+                result.drops.push_back(TrackFxDrops{item.work.trackGuid, resolved.drops});
 
-            // The record is spent only once the chain has been driven to every state it
+            // The record is spent once the chain has been driven to every captured state
-            // captured — releasing any earlier destroys the only account of which plugins
+            // that is still PLACEABLE — which is why the release fires with drops
+            // outstanding: a dropped entry names an FX the chain no longer has, so
+            // holding the record for it would replan work that can never resolve.
+            // Releasing any EARLIER is what destroys the only account of which plugins
             // the USER had offline before the park, and the next online blanket-enables
             // them. Reported rather than applied; see ReclaimTickResult::released.
             if (item.work.kind == view::FxWorkKind::Online)
@@ -208,6 +243,12 @@ struct ReclaimRunResult {
     // the releases are DISCARDED, `settled` is never true, and the caller must not go
     // idle — the next tick re-derives against whatever changed.
     bool aborted = false;
+
+    // Carried out on the abort path TOO, unlike the releases: a drop is a fact about the
+    // chain the tick just wrote, not a claim about the model a re-entered load may have
+    // replaced. The re-derivation that follows an abort resolves the very same set,
+    // which is what shouldReportDrops is there to swallow.
+    std::vector<TrackFxDrops> drops;
 };
 
 // Owns the derived queue and the debounce clock across a tick that can be re-entered:
@@ -256,14 +297,26 @@ public:
         // back whether or not the drain's own work survives.
         clock_.swap(clock);
 
-        if (generation_ != entered) return ReclaimRunResult{tick.writes, false, true};
+        if (generation_ != entered)
+            return ReclaimRunResult{tick.writes, false, true, tick.drops};
 
         items_.swap(work);
         // A release dropped by an abort is re-owed, not lost: the next derivation plans
         // the same Online item, its writes all find the chain already there, and the
         // release fires then.
+        if (!tick.released.empty()) {
             for (const std::string& guid : tick.released) host.releaseFx(guid);
-        return ReclaimRunResult{tick.writes, tick.settled, false};
+            // The spend has to be as durable as the record it spends. A reload — a
+            // reopen, or the undo/redo loadFromProject — rebuilds the model from the
+            // PERSISTED view state, and a record still held there re-derives this same
+            // Online work and re-drives the chain, silently reverting whatever per-FX
+            // offline the user set once the reclaim had finished. One persist per tick,
+            // after every release the tick earned, and still killable in between: die
+            // there and the record simply survives on disk, whereupon the re-derived
+            // item finds the chain already online, writes nothing, and releases again.
+            host.persistModel();
+        }
+        return ReclaimRunResult{tick.writes, tick.settled, false, tick.drops};
     }
 
 private:
@@ -293,9 +346,11 @@ void invalidateFxReclaim();
 bool fxReclaimDrainInProgress();
 
 // Drains one tick against `proj` (nullptr = current project), re-deriving the queue from
-// `model` first if it was invalidated. Returns without touching the project while an
+// the session's view model first if it was invalidated. Returns without touching the
-// applyMode is on the stack, while already draining, or when the active project is not
+// project while an applyMode is on the stack, while already draining, or when the active
-// the one the queue was derived against.
+// project is not the one the queue was derived against. Takes the SESSION rather than the
-void fxReclaimTick(ViewModeModel& model, ReaProject* proj);
+// model alone because completing an online spends the park record's FX half, and that
+// spend has to reach the persisted view state — see FxReclaimQueue::run.
+void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj);
 
 }  // namespace reasampler

tests/test_fx_reclaim.cpp

@@ -2,13 +2,15 @@
 // loop is templated on its host precisely so the properties that make it safe to run in
 // the background are assertable here: the debounce, the per-tick budget's between-track
 // yield, the queue being a cache of intent that a model change discards, the user's own
-// offline plugins surviving an online, and — via a host that invalidates from inside a
+// offline plugins surviving an online, the release reaching the persisted model, and —
-// write — that a re-entered invalidation reaches nothing the drain is using.
+// via a host that invalidates from inside a write — that a re-entered invalidation
+// reaches nothing the drain is using.
 
 #include "../src/shell/view/fx_reclaim.h"
 
 #include <cstdio>
 #include <map>
+#include <optional>
 #include <set>
 #include <string>
 #include <vector>
@@ -75,7 +77,23 @@ struct FakeHost {
         return true;
     }
 
-    void releaseFx(const std::string& guid) { released.insert(guid); }
+    // `model` is the live host's session model and `persisted` stands in for view_state
+    // on disk — the blob a reopen or an undo/redo loadFromProject rebuilds from. Set them
+    // and the release actually lands, so a persist ordered BEFORE it would serialize a
+    // record that is still held and fail the reload assertions.
+    ViewModeModel* model = nullptr;
+    std::string    persisted;
+    int            persists = 0;
+
+    void releaseFx(const std::string& guid) {
+        released.insert(guid);
+        if (model) model->releaseSnapshotFx(guid);
+    }
+
+    void persistModel() {
+        ++persists;
+        if (model) persisted = model->serialize();
+    }
 
     bool offlineAt(const std::string& guid, std::size_t slot) const {
         auto it = chains.find(guid);
@@ -491,6 +509,7 @@ static void testAnInvalidationDuringAnOnlineDropsThatTicksRelease() {
     CHECK(r.aborted);
     CHECK(r.writes == 2);          // the item still completed — a step is atomic
     CHECK(host.released.empty());  // but its record was NOT spent
+    CHECK(host.persists == 0);     // and nothing durable claims it was
 
     // Re-owed, not lost: the re-derived item finds the chain already where the record
     // wants it, writes nothing more, and releases then.
@@ -499,6 +518,119 @@ static void testAnInvalidationDuringAnOnlineDropsThatTicksRelease() {
     CHECK(q.run(host).settled);
     CHECK(host.writeLog.size() == 2);
     CHECK(host.released.count("{T}") == 1);
+    CHECK(host.persists == 1);
+}
+
+// -- 6. the release has to outlive the session ------------------------------------
+
+static void testACompletedOnlineIsPersistedSoAReloadPlansNoWork() {
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
+    vm.setActiveMode(kDesignModeId);
+    vm.releaseSnapshotFlags("{T}");
+
+    // The bug this closes, stated as the baseline: the model as it stands BEFORE the
+    // release re-derives the same Online work when reloaded, and re-driving it would
+    // revert whatever per-FX offline the user set after the reclaim had finished.
+    const std::optional<ViewModeModel> stale = ViewModeModel::deserialize(vm.serialize());
+    CHECK(stale.has_value());
+    CHECK(derive(*stale, {"{T}"}).size() == 1);
+
+    FakeHost host;
+    host.model = &vm;
+    host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"});
+
+    FxReclaimQueue q = queueOf(vm, {"{T}"});
+    drainToSettled(q, host);
+    CHECK(host.released.count("{T}") == 1);
+    CHECK(host.persists == 1);  // once per tick that released, not once per release
+
+    const std::optional<ViewModeModel> reloaded = ViewModeModel::deserialize(host.persisted);
+    CHECK(reloaded.has_value());
+    CHECK(reloaded->snapshot("{T}") == nullptr);  // both halves spent ⇒ record erased
+    CHECK(derive(*reloaded, {"{T}"}).empty());
+}
+
+static void testATickThatSpendsNoRecordPersistsNothing() {
+    // OFFLINE work spends no record, so there is nothing to make durable — and a
+    // background reclaim must not dirty the project for work it can simply redo.
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
+    FakeHost host;
+    host.model = &vm;
+    host.chains["{T}"] = onlineChain({"{FX-A}"});
+
+    FxReclaimQueue q = queueOf(vm, {"{T}"});
+    q.run(host);  // observes the hold at t=0
+    host.clock += kParkHoldMs;
+    drainToSettled(q, host);
+
+    CHECK(host.writeLog.size() == 1);  // it did move the plugin
+    CHECK(host.released.empty());
+    CHECK(host.persists == 0);
+    CHECK(host.persisted.empty());
+}
+
+// -- 7. captured FX state the online direction cannot place -------------------------
+
+static void testADropIsReportedOnceAcrossRepeatedRederivations() {
+    // {FX-B} was deleted while the track was parked, so its captured state has nowhere
+    // to land. Swallowing that leaves the user no trace of it at all.
+    ViewModeModel vm;
+    vm.membership().tag("{T}", kDesignModeId);
+    vm.setActiveMode(kDesignModeId);
+    vm.storeSnapshot("{T}", recordWithFx({{"{FX-A}", 0}, {"{FX-B}", 1}}));
+    vm.releaseSnapshotFlags("{T}");
+
+    FakeHost host;
+    host.model = &vm;
+    host.chains["{T}"] = offlineChain({"{FX-A}"});  // {FX-B} gone from the chain
+
+    FxReclaimQueue q = queueOf(vm, {"{T}"});
+    host.reentrant = &q;
+    host.invalidateAfterWrite = 1;  // aborts, so the identical item re-derives next tick
+
+    const ReclaimRunResult first = q.run(host);
+    CHECK(first.aborted);
+    CHECK(first.drops.size() == 1);
+    CHECK(first.drops[0].trackGuid == "{T}");
+    CHECK(first.drops[0].drops.missingIdentity == 1);
+    CHECK(first.drops[0].drops.total() == 1);
+    CHECK(host.released.empty());  // the abort still refuses to spend the record
+
+    host.reentrant = nullptr;
+    q.adopt(derive(vm, {"{T}"}));
+    const ReclaimRunResult second = q.run(host);
+    CHECK(second.settled);
+    CHECK(second.drops == first.drops);  // the re-derivation resolves the very same set
+
+    // The gate the shell's console report sits behind: first sighting prints, that
+    // identical re-derivation does not, a different set does.
+    const std::vector<TrackFxDrops> none;
+    CHECK(shouldReportDrops(/*sameOwnerAsLast=*/false, none, first.drops));
+    CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/true, first.drops, second.drops));
+    const std::vector<TrackFxDrops> elsewhere{TrackFxDrops{"{U}", FxRestoreDrops{1, 0, 0}}};
+    CHECK(shouldReportDrops(/*sameOwnerAsLast=*/true, first.drops, elsewhere));
+
+    // The record IS spent with a drop outstanding: holding it for an FX the chain no
+    // longer has would replan work that can never resolve.
+    CHECK(host.released.count("{T}") == 1);
+    CHECK(!host.offlineAt("{T}", 0));  // and the placeable half did land
+}
+
+static void testAnOfflineTickReportsNoDrops() {
+    // Offline targets are the live chain's own slots, so nothing can fail to resolve.
+    // Keeps the drop assertions above honest: they must catch a real degrade, not an
+    // accumulator that fills on every tick.
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
+    FakeHost host;
+    host.chains["{T}"] = onlineChain({"{FX-A}", "{FX-B}"});
+
+    FxReclaimQueue q = queueOf(vm, {"{T}"});
+    q.run(host);
+    host.clock += kParkHoldMs;
+    const ReclaimRunResult r = q.run(host);
+    CHECK(r.settled);
+    CHECK(r.writes == 2);
+    CHECK(r.drops.empty());
 }
 
 int main() {
@@ -519,6 +651,12 @@ int main() {
     testAnInvalidationDuringTheLastItemLeavesTheTickUnsettled();
     testAnInvalidationDuringAnOnlineDropsThatTicksRelease();
 
+    testACompletedOnlineIsPersistedSoAReloadPlansNoWork();
+    testATickThatSpendsNoRecordPersistsNothing();
+
+    testADropIsReportedOnceAcrossRepeatedRederivations();
+    testAnOfflineTickReportsNoDrops();
+
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;
 }