Close the collapse's two dropped guarantees: latch applyMode against re-entrancy, pin its project once; correct three overclaiming doc lines

src/shell/view/CLAUDE.md

@@ -157,6 +157,11 @@ applies the resulting lane state to live tracks.
 - The pure mode model (`ViewModeModel`, membership, `reconcile(liveGuids)`, the
   snapshot-based park/restore planner) lives in `core/view` — reference it, do not
   duplicate its spec here.
+- **`TrackPlan::fxOffline` is read on the RESTORE side only.** `planToggle` builds
+  every park plan with `fxCount=0`, and this shell's park loop passes `tp.flags`
+  alone to `parkTrack`, which expands the real writes itself from
+  `TrackFX_GetCount` — so the park-side field is populated only by tests. A shell
+  change that starts reading it on the park path is reading an empty vector.
 - The Two-canvas sub-phase (Phase D2/E)'s settled DAW-application rules
   (fixed-lane mechanics, mode-aware capture placement, hidden-AND-silenced) are
   reflected in Invariants above; the membership/lane-ownership model concepts
@@ -169,10 +174,14 @@ applies the resulting lane state to live tracks.
   the solo cache inherits it rather than introducing it. Not fixed here.
 - **Which routes reach the no-snapshot-plus-parked-chain pair, re-derived for the
   one-point design.** Three of the routes that used to reach it were artifacts of
-  the retired deferral window and are GONE BY CONSTRUCTION: there is no longer a
-  moment in which a track's flags have been written but its FX have not, so no
-  save, no reload and no discard can land between the two halves, and there is no
-  queued intent for an undo to drop. What survives:
+  the retired deferral window, and are now NARROWED TO THE RE-ENTRANCY WINDOW
+  rather than gone: the flags-written-FX-not moment still exists inside
+  `parkTrack`, but nothing can observe it except code re-entered through the
+  pump premise below, and `applyMode` fails closed when re-entered
+  (`modeApplyInProgress`). The window that remains is what a call REACHABLE FROM
+  a pumped message loop but not routed through `applyMode` could see — no save,
+  no reload and no discard reaches it, and there is no queued intent for an undo
+  to drop. What survives:
   - A `view_state` that PARSED but carries no snapshot for the track — a snapshot
     `reconcile` pruned while its track was out of the live enumeration, then
     undo/redo-restored with the track but not the snapshot. (Asserted by the
@@ -184,21 +193,39 @@ applies the resulting lane state to live tracks.
   - A track the USER keeps hidden from both panels, out of the mix and
     FX-bypassed. It reads identically to both of the above and is refused too —
     no lost state there at all, which is why the report asserts no cause.
+  - A REDO of a mode switch, hypothetically — the mechanism and its two
+    unverified links are in the `view_state` gotcha below, which owns that
+    question; do not re-derive it here.
 
-  An undo of a mode switch (Ctrl-Z) is NOT a route, and no longer needs an
+  An UNDO of a mode switch (Ctrl-Z) is NOT a route, and no longer needs an
   ordering premise to say so: flags and FX moved inside ONE block, so they roll
-  back together and no intermediate state exists for a park to misread.
+  back together and no intermediate state exists for a park to misread. The redo
+  direction is not symmetric with it — see below.
 - **The `view_state` write is NOT inside `applyMode`'s block.** `persistViewState`
   (`design_view_actions.cpp`) runs after `applyMode` has returned, so
   `SetProjExtState` lands past `Undo_EndBlock2`. The UNDO direction still comes
   out right — the point closed over the ext state as it stood BEFORE the switch,
   which is what a Ctrl-Z should restore — but by position, not by design. The
-  REDO direction is what this leaves open: the point never saw the new
-  `view_state`, so a redo may re-park the tracks while restoring the model that
-  describes the mode the user undid back to. `[verify — DAW]`; if it does bite,
-  the result is the pre-existing model-vs-undo split the solo-cache gotcha above
-  already describes, not a new refusal route. Do not "fix" it by moving
-  `persistViewState` inside the block — it can open a modal Save-As.
+  REDO direction is what this leaves open, and the cost is worse than the
+  model-vs-undo split the solo-cache gotcha describes: **a redo can plausibly
+  produce a NEW refusal route.** As a HYPOTHESIS — the redo restores the parked
+  flags AND the ext state as the point closed over it, i.e. the PRE-switch
+  `view_state`, carrying no snapshot for those tracks (`UNDO_STATE_MISCCFG`
+  covers extension state, `reaper_plugin.h:1544`). The `projectconfig` reload
+  then rebuilds the model from that stale value, and the reapply it triggers
+  plans NEITHER a park (the track is a member of the restored active mode) nor a
+  restore (no snapshot to restore from) — verified in `planToggle`, which pushes
+  nothing at all for an active leaf without a snapshot. The tracks stay parked
+  with no snapshot, so the next real switch parks them and `decidePark` refuses.
+  TWO links are unverified and the route dies if either fails: (1) that the
+  point's stored state is captured at `Undo_EndBlock2` and therefore excludes the
+  `persistViewState` write that follows it — the SDK documents no capture-time
+  semantics for the state mask; (2) that a redo fires `BeginLoadProjectState`
+  with `isUndo == true` so the session really does reload the stale ext state
+  (asserted in `main.cpp`, never observed). If (2) is false the in-memory model
+  keeps its snapshots and the redo is consistent. `[verify — DAW]`. Do not "fix"
+  it by moving `persistViewState` inside the block — it can open a modal Save-As;
+  whether to split the persist is a separate decision.
 - A DIFFERENT strand entirely, which the refusal report does NOT cover: an
   absent or malformed `view_state` (`loadViewModel`, `ext_state_io.cpp`) is not
   a parked-chain reading at all — it falls back to a DEFAULT model (active mode
@@ -212,15 +239,24 @@ applies the resulting lane state to live tracks.
   on REAPER coalescing nested blocks, which the SDK header does not document.
   The inverse is also open: a plugin unloading is ASSUMED to pump the message
   loop, so an action fired mid-apply could open its own block inside this one.
-  Nothing guards against that (the retired queue's `g_draining` never did either
-  — it only stopped a second drain). Both are `[verify — DAW]`.
-  That same premise leaves a STATED RESIDUAL on track handles. The park and
-  restore loops re-`ValidatePtr2` per track (`trackStillLive`), because they are
-  the ones interleaved with the FX writes. The passes that run AFTER them — lane
-  ops, parent visibility, the solo replay — still use handles resolved before the
-  first FX write, so a track deleted mid-apply would reach them stale. Not closed:
-  the premise it depends on is itself unobserved, and a partial guard covering
-  three of those four passes would read as complete.
+  `applyMode` itself cannot be that action — it fails closed when re-entered
+  (`modeApplyInProgress`, `view.cpp`'s `ApplyLatch`) — but nothing else is
+  latched, `mintManagedLanes` included, and it opens a block of its own. Both are
+  `[verify — DAW]`.
+  That same premise put a residual on track handles, now CLOSED by construction
+  rather than by a partial guard. The park and restore loops re-`ValidatePtr2`
+  per track (`trackStillLive`) because they are the ones interleaved with the FX
+  writes; the whole enumeration is then re-validated ONCE where those loops end,
+  so every pass after them — lane ops, parent visibility, the solo replay — sees
+  only handles REAPER still recognizes. A dead track's handle is nulled in place
+  rather than erased from the vector, which is what `view_solo`'s writers already
+  skip on.
+  The latch's own cost, stated: a mode switch or a membership reapply fired while
+  another apply is on the stack is DROPPED, not queued. The one caller with
+  nothing to fall back on is `main.cpp`'s load glue — it spends a one-shot signal
+  — so it tests `modeApplyInProgress` before consuming and retries next tick. The
+  action callers just no-op, silently (`reportModeSwitchRefused` speaks only for
+  the transport gate); the user re-fires.
 - `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
   attached to its plugin across a chain mutation while parked. That is
   `[verify — DAW]` (see `fxGuidString` in `view_fx_park.cpp`) and SWS issue #802 is a

src/shell/view/view.cpp

@@ -26,6 +26,7 @@
 
 #define REAPERAPI_MINIMAL
 #define REAPERAPI_WANT_CountTracks
+#define REAPERAPI_WANT_EnumProjects
 #define REAPERAPI_WANT_GetPlayStateEx
 #define REAPERAPI_WANT_GetTrack
 #define REAPERAPI_WANT_GetMediaTrackInfo_Value
@@ -37,6 +38,7 @@
 #define REAPERAPI_WANT_TrackList_AdjustWindows
 #define REAPERAPI_WANT_UpdateArrange
 #define REAPERAPI_WANT_UpdateTimeline
+#define REAPERAPI_WANT_ValidatePtr2
 // Lane minting: item-side lane reads/writes assigning each item to its mode's lane.
 #define REAPERAPI_WANT_CountTrackMediaItems
 #define REAPERAPI_WANT_GetTrackMediaItem
@@ -85,6 +87,19 @@ struct UiRefreshHold {
     UiRefreshHold& operator=(const UiRefreshHold&) = delete;
 };
 
+// applyMode is reachable from the load tick, the actions and render_in_place, and its
+// TrackFX_SetOffline calls are assumed to pump the message loop — so any of them can
+// re-enter it mid-apply, planning against the same model the outer loop is iterating.
+// An inner restoreTrack consuming a snapshot the outer sits mid-park over leaves that
+// track flags-restored, chain-offline and snapshotless: refused forever. Fail closed.
+bool g_applying = false;
+struct ApplyLatch {
+    ApplyLatch() { g_applying = true; }
+    ~ApplyLatch() { g_applying = false; }
+    ApplyLatch(const ApplyLatch&) = delete;
+    ApplyLatch& operator=(const ApplyLatch&) = delete;
+};
+
 // C_LANESCOLLAPSED=2: render a tool-split track like a normal single-lane
 // track showing only the playing lane (SDK: 1=collapsed, 2=hidden-lanes-exist
 // but displays as non-fixed-lane).
@@ -147,11 +162,6 @@ MediaTrack* resolve(const TrackByGuid& byGuid, const std::string& guid) {
     return it == byGuid.end() ? nullptr : it->second;  // stale/deleted GUID — pruned by being skipped
 }
 
-// Managed-lane application: LanePlayOps are keyed by the lane's DURABLE name but
-// C_LANEPLAYS:N by current ordinal, which renumbers on reorder — so every write
-// re-resolves durable key -> ordinal first, and a lane whose name lacks the
-// managed prefix never enters the map and so can never be driven.
-
 // Lane `laneIdx`'s durable name (P_LANENAME:n) on `tr`, or empty if unnamed /
 // unavailable (non-fixed-lane track).
 std::string laneName(MediaTrack* tr, int laneIdx) {
@@ -231,9 +241,6 @@ LaneApplyResult applyLaneOps(const TrackByGuid& handleByGuid,
     return out;
 }
 
-// Managed-lane minting: the DECISION (which tracks split, which lanes, which item
-// goes where) is planLaneMinting's; this shell only reads, calls and applies.
-
 // One pass, so the assign loop needs no per-item re-scan.
 std::map<std::string, MediaItem*> itemHandlesByGuid(MediaTrack* tr) {
     std::map<std::string, MediaItem*> byGuid;
@@ -367,11 +374,22 @@ bool transportBlocksModeSwitch(ReaProject* proj) {
     return (GetPlayStateEx(proj) & kTransportMoving) != 0;
 }
 
+bool modeApplyInProgress() { return g_applying; }
+
 bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) {
+    if (g_applying) return false;  // see ApplyLatch; same fail-closed shape as the guards below
+    ApplyLatch latch;
+
     if (!model.modes().contains(targetModeId)) {
         return false;  // reject before touching the project — no partial apply
     }
 
+    // Pinned ONCE: begin and end sit a whole apply apart with the pump premise between
+    // them, so resolving "current project" independently at each would let a tab switch
+    // mid-apply open the block on one project and close it on another.
+    ReaProject* const project = proj ? proj : EnumProjects(-1, nullptr, 0);
+    if (!ValidatePtr2(nullptr, project, "ReaProject*")) return false;
+
     // The discriminator behind both halves of the contract in view.h. A gate placed
     // unconditionally here would break tagging and the project-load reapply during
     // playback; a solo round on every reapply would flicker the user's solo on every
@@ -379,13 +397,13 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
     const std::string outgoingModeId = model.activeModeId();
     const bool realSwitch = targetModeId != outgoingModeId;
 
-    if (realSwitch && transportBlocksModeSwitch(proj)) {
+    if (realSwitch && transportBlocksModeSwitch(project)) {
         return false;  // same fail-closed shape as the mode-exists guard above
     }
 
     TrackHandles handleByGuid;
-    std::vector<TrackFolderEntry> entries = readFolderEntries(proj, handleByGuid);
-    const TrackByGuid trackByGuid = indexHandles(handleByGuid);
+    std::vector<TrackFolderEntry> entries = readFolderEntries(project, handleByGuid);
+    TrackByGuid trackByGuid = indexHandles(handleByGuid);
     FolderTree tree = buildFolderTree(entries);
 
     // Prune snapshots for tracks no longer in the live enumeration before
@@ -417,7 +435,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
     FxRestoreDrops fxDrops;
     int fxDropTracks = 0;
 
-    Undo_BeginBlock2(proj);
+    Undo_BeginBlock2(project);
     {
         UiRefreshHold uiHold;  // every write below lands with the TCP/MCP frozen
 
@@ -438,7 +456,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
             // Re-validated per track, not once at resolve: offlining a plugin is
             // ASSUMED to pump the message loop, so a track deleted during an
             // earlier iteration's FX writes would leave this handle dangling.
-            if (!tr || !trackStillLive(proj, tr)) continue;  // stale/deleted GUID — prune
+            if (!tr || !trackStillLive(project, tr)) continue;  // stale/deleted GUID — prune
 
             const TrackApplyResult r = parkTrack(model, guid, tr, tp.flags);
             if (r.refused) { refusedParkNames.push_back(trackDisplayName(tr)); continue; }
@@ -450,7 +468,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
             if (tp.flags.empty()) continue;
             const std::string& guid = tp.flags.front().guid;
             MediaTrack* tr = resolve(trackByGuid, guid);
-            if (!tr || !trackStillLive(proj, tr)) continue;  // stale/deleted GUID — prune
+            if (!tr || !trackStillLive(project, tr)) continue;  // stale/deleted GUID — prune
 
             const TrackApplyResult r = restoreTrack(model, guid, tr, tp.fxOffline, tp.flags);
             if (r.wrote) wrote = true;
@@ -458,6 +476,16 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
             if (r.drops.total() > 0) { fxDrops.add(r.drops); ++fxDropTracks; }
         }
 
+        // The FX writes above are the only thing here assumed to pump the message
+        // loop, so this is the one point a handle taken before them can have died.
+        // Re-validated for the whole enumeration rather than per consumer: lane ops,
+        // parent visibility and the solo replay all write through pre-FX handles.
+        for (auto& kv : handleByGuid) {
+            if (trackStillLive(project, kv.second)) continue;
+            trackByGuid.erase(kv.first);
+            kv.second = nullptr;  // view_solo's writers already skip a null handle
+        }
+
         // MANAGED LANES: the active mode's lane plays+shows, every other managed
         // lane is silenced+hidden. Empty on a D1-only project — byte-identical there.
         const LaneApplyResult lanes = applyLaneOps(trackByGuid, plan.lanes);
@@ -502,11 +530,11 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
     if (laneModeChanged) UpdateTimeline();
 
     const bool mint = applyMintsUndoPoint(realSwitch, wrote);
-    Undo_EndBlock2(proj, mint ? undoLabel.c_str() : "", mint ? undoMask : 0);
+    Undo_EndBlock2(project, mint ? undoLabel.c_str() : "", mint ? undoMask : 0);
 
     // After the undo block and the UI hold: a report is not a project write, and
     // it must not join what a Ctrl-Z rolls back.
-    reportRefusedParks(proj, refusedParkNames);
+    reportRefusedParks(project, refusedParkNames);
     reportFxRestoreDrops(fxDrops, fxDropTracks);
     return true;
 }
@@ -524,6 +552,8 @@ bool mintManagedLanes(ViewModeModel& model, ReaProject* proj) {
 
     const TrackByGuid trackByGuid = indexHandles(handleByGuid);
 
+    // `proj` deliberately NOT pinned the way applyMode pins it: nothing between this
+    // begin and its end pumps the message loop, so no tab switch can land between them.
     Undo_BeginBlock2(proj);
     const bool changed = applyMintPlan(model, plan, trackByGuid);
 

src/shell/view/view.h

@@ -40,15 +40,22 @@ inline bool applyMintsUndoPoint(bool realSwitch, bool wroteAnything) {
     return realSwitch || wroteAnything;
 }
 
+// True while an applyMode call is on the stack. applyMode fails closed when re-entered
+// (its FX writes are assumed to pump the message loop, so a timer tick or an action can
+// land inside one), and this lets a caller with no fallback test BEFORE it spends a
+// one-shot signal on a call that would be refused — see main.cpp's load glue.
+bool modeApplyInProgress();
+
 // Snapshots each about-to-park track's flags into `model`, caches/clears the
 // outgoing mode's solo state and replays the incoming mode's, runs planToggle,
 // applies park/restore writes — flags AND per-FX offline, synchronously — plus
 // parent visibility flags, then sets the active mode. ONE switch is ONE undo
 // point; everything it moved rolls back in a single Ctrl-Z. Returns false (no
-// mutation) if `targetModeId` isn't registered, or if this is a real switch
-// (target != active) while the transport is running. A reapply (target ==
+// mutation) if `targetModeId` isn't registered, if this is a real switch
+// (target != active) while the transport is running, if it is re-entered
+// (modeApplyInProgress), or if the project does not validate. A reapply (target ==
 // active) is never gated and never touches solo. `proj` == nullptr means the
-// current project.
+// current project, resolved ONCE and used for every call this apply makes.
 bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj);
 
 // Splits any track visible in more than one mode while carrying its own media

src/shell/view/view_fx_park.cpp

@@ -206,10 +206,11 @@ TrackApplyResult restoreTrack(ViewModeModel& model, const std::string& guid, Med
 
     if (applyFlags(tr, flags, out.fxMoved)) out.wrote = true;
 
-    // Unconditional, even when every write above was elided: the snapshot is
-    // gone, so this apply is not repeatable and must not close on the discard
-    // form. Model state, not project state — whether an undo actually puts the
-    // snapshot back is the ext-state question in src/shell/view/CLAUDE.md.
+    // Unconditional, even when every write above was elided: the snapshot is consumed
+    // here, so this apply changed state and is not repeatable — and `wrote`, whose one
+    // reader asks exactly that, would be a lie at false. NOT a claim about what the
+    // undo record holds: the block never carries the snapshot either way, that being
+    // model state persisted after Undo_EndBlock2 (src/shell/view/CLAUDE.md).
     model.clearSnapshot(guid);
     out.wrote = true;
     return out;