Fix drag-drop review findings: unify teardown, gate FX-add on outside-panel, trim comments

src/shell/actions/drag_out_win.h

@@ -26,13 +26,10 @@ namespace reasampler {
 // (DROPEFFECT_COPY); the return is advisory — a failed drag surfaces no error.
 bool initiateDragOut(HWND__* panelHwnd, const std::vector<std::string>& absolutePaths);
 
-// Cheap, side-effect-free readiness probe: true iff initiateDragOut would actually be able to
+// Side-effect-free readiness probe for initiateDragOut. Call BEFORE tearing down internal drag
-// START a drag for `absolutePaths` right now. Call this BEFORE tearing down internal drag
+// state — an OS that isn't ready (OLE unavailable, HDROP build failure) must not consume the
-// state (release capture, clear drag fields) — an OS that isn't ready (OLE unavailable, or the
+// gesture like an empty payload would. Windows re-runs the OLE-init + HDROP checks and frees the
-// path list can't build a CF_HDROP) must not consume the gesture the same way an empty payload
+// probe HGLOBAL immediately; SWELL exposes no probe, so macOS/Linux reduces to the non-empty check.
-// would. On Windows this re-runs the same OLE-init + HDROP-build checks initiateDragOut does,
-// freeing the probe HGLOBAL immediately; SWELL exposes no such probe, so macOS/Linux reduces to
-// the non-empty check alone.
 bool canInitiateDragOut(const std::vector<std::string>& absolutePaths);
 
 } // namespace reasampler

src/shell/panel/panel_drag.cpp

@@ -327,13 +327,7 @@ void onMouseMove(int x, int y) {
             // DoDragDrop runs its own modal loop and takes over mouse capture, so the internal
             // drag must be fully wound down first.
             if (GetCapture() == g_panel.hwnd) ReleaseCapture();
-            g_panel.dragArmed = false;
+            resetDragState();
-            g_panel.dragging = false;
-            g_panel.dropKind = DropKind::None;
-            g_panel.dropBankId.clear();
-            g_panel.cardGesture = CardGesture::None;
-            g_panel.dragTargetSlot = -1;
-            g_panel.dragPrimaryId.clear();
             invalidatePanel();
 
             initiateDragOut(g_panel.hwnd, paths);  // COPY-ONLY; blocking on Windows
@@ -424,7 +418,7 @@ void onLBtnUp(int x, int y) {
             const FxDropTarget fx = resolveFxDropTarget(sp.x, sp.y);
             if (fx.valid()) dropTrack = fx.track;
         }
-        if (dropTrack && singleCapture) {
+        if (!inside && dropTrack && singleCapture) {
             const std::string sampleId = g_panel.dragSampleIds.front();
             performInstrumentDrop(dropTrack, buildInstrumentDropPreset(sampleId));
             // Read-only over the bank + arrange: the only mutations are the new FX instance +
@@ -440,8 +434,6 @@ void onLBtnUp(int x, int y) {
             } else if (g == CardGesture::Replace) {
                 // Replace targets the OCCUPANT of the target slot with the single grabbed card.
                 const bool isBanks = g_panel.dragSourceRegion == Region::Banks;
-                RECT cr{};
-                GetClientRect(g_panel.hwnd, &cr);
                 const int w = cr.right - cr.left, h = cr.bottom - cr.top;
                 const RECT region = isBanks ? banksRegionRect(w, h) : poolRegionRect(w, h);
                 const RegionDisplay disp = regionDisplay(region, isBanks, g_panel.dragSourceRegion);

src/shell/panel/panel_state.h

@@ -320,10 +320,8 @@ struct PanelState {
     // when the pointer is not over an FX button.
     MediaTrack* instrumentDropTrack = nullptr;
 
-    // Latched once an OsDrag hand-off attempt for THIS gesture resolves to "cannot hand off"
+    // Latched once an OsDrag hand-off resolves "cannot hand off" for this gesture — skips
-    // (empty/unresolvable payload, or the OS isn't ready) — skips re-running
+    // re-running resolveDragPathsForOs (fs::exists per sample) each move. Cleared by resetDragState.
-    // resolveDragPathsForOs (an fs::exists per sample) and the readiness probe on every
-    // subsequent move while the drag stays alive. Cleared by resetDragState.
     bool dragOsHandoffBlocked = false;
 
     // Authoritative tail setting lives in ReaSamplerSession, not here; panel reads it for

tests/test_instrument_drop.cpp

@@ -234,18 +234,10 @@ static void testEmbedStripIsNotHotspot() {
     CHECK(!infoNamesFxHotspot("mcp.fxembed extra"));
 }
 
-// --- Drop surface parity: container vs FX button ------------------------------
+// Do not reintroduce a per-surface "capture carries" loop test: buildInstrumentDropPreset takes
-//
+// only sampleId (proven by testPresetRoundTripsThroughInstrumentReader), and per-surface hotspot
-// The container-drop regression (instance loads, capture does not): the two surfaces must be
+// coverage already exists (testTcpMcpFxFamilyIsHotspot / testFxWindowStillHotspot) — a loop with
-// one code path carrying one payload. buildInstrumentDropPreset takes only sampleId, so the
+// an identical body per surface string can't distinguish them.
-// payload side of that claim is already proven once by testPresetRoundTripsThroughInstrumentReader
-// and every "fx_"/"tcp.fx"/"mcp.fx" surface classifying as a hotspot is proven by
-// testTcpMcpFxFamilyIsHotspot / testFxWindowStillHotspot. A loop that reruns both against a
-// fixed sampleId per surface string can't distinguish the surfaces (the loop body is identical
-// every iteration) — it isn't a stronger test than those two, so there is no separate test here.
-// The one thing that DOES vary by surface — the shell's TrackFX_AddByName `instantiate` value
-// picked for a container drop vs. a bare FX-button drop — lives in instrument_drop_win.cpp and
-// is untestable without a live DAW (GetThingFromPoint/TrackFX_AddByName have no pure model).
 
 // --- All-or-nothing rollback --------------------------------------------------