Merge Ψ-W1-T3: publish the Media Explorer import into the Media Explorer action section as well as Main

CLAUDE.md

@@ -181,6 +181,7 @@ Comments carry *why*, and context where non-obvious — never *what* the code al
   2. `rec->Register("gaccel", &accel)` — puts the action in the Actions list.
   3. `rec->Register("hookcommand", ...)` — receives every action fired; claim only your own id, return `false` otherwise.
   4. On unload (`rec == nullptr`), mirror-unregister everything with the same strings prefixed by `'-'`.
+- **Non-main sections use a different mechanism.** `gaccel_register_t` carries no section field — `command_id` + `gaccel` can only ever produce a Main-section action. To publish into another section (Media Explorer = 32063, MIDI editor = 32060, MIDI event list = 32061, MIDI inline = 32062), register a `custom_action_register_t{uniqueSectionId, idStr, name, extra}` under `"custom_action"`; it returns the command id, or **0 on failure** (e.g. a duplicate `idStr`) — which the caller must tolerate rather than half-register. `idStr` must be unique **across all sections**, so an action published into both Main and a non-main section needs a SECOND id string; the FOREVER-STABLE contract binds it identically from the moment it ships. `custom_action_register_t` has no `ACCEL`, so a non-main entry ships no default keybinding. Dispatch for these ids arrives through `"hookcommand2"` (`bool(KbdSectionInfo*, int command, int val, int val2, int relmode, HWND)`) — `"hookcommand"` runs for the main section only. The two hooks must partition the ids between them; what happens when a command is claimed by both is unspecified by the SDK (`hookcommand2`'s doc says a `true` return prevents further hooks/actions from running, which is in tension with a clean double-fire either way), so nothing may rely on either outcome. On unload, mirror with `"-custom_action"` `[verify — DAW]` (the header confirms the `-` prefix for "most" registration types and spells out only `-pcmsrc` by name; `custom_action` itself is unconfirmed) and `"-hookcommand2"`.
 
 ## Product design docs
 

docs/PLAN.md

@@ -2412,7 +2412,10 @@ must not touch `main.cpp`; this track is why.
   (`custom_action` / `hookcommand2` / `-custom_action` mirror) — a deliverable of this
   track.
 - **DAW-verification obligation:** Daniel adds the action to the Media Explorer toolbar
-  and imports from it; also confirms the Main-section binding still fires.
+  and imports from it; also confirms the Main-section binding still fires. Also: unload,
+  reload, confirm no duplicate Media Explorer entry — the `-custom_action` unload mirror
+  is unconfirmed against the SDK header (root `CLAUDE.md` §"REAPER extension contract",
+  `[verify — DAW]`), and if unsupported the entry leaks across a reload.
 
 **Open questions.** `[verify]` re-verify `custom_action_register_t` and `hookcommand2`
 argument order/types against the SDK headers at implementation time (root `CLAUDE.md`

docs/TODO.md

@@ -445,3 +445,40 @@ today.
 bitmap and confirmed to place the stroke correctly, or it is redesigned to rasterize at
 physical rather than logical resolution once `IPlugViewContentScaleSupport` (or
 equivalent) makes scaling real.
+
+## `ingestHandleSectionCommand` has no unit test
+
+**Context (what shipped — Ψ-W1-T3, media-explorer-section).** The Media-Explorer
+import now dispatches through two hooks — `ingestHandleCommand` (Main,
+`"hookcommand"`) and `ingestHandleSectionCommand` (Media Explorer,
+`"hookcommand2"`). Both live in `ingest.cpp`, which compiles straight into the
+`reaper_reasampler` MODULE target.
+
+**The wart.** No `shell/` translation unit in this repo has a test target — every
+`<module>_tests` executable is a `core/` pure-module target. `ingestHandleSectionCommand`
+is a two-line command-id comparison; correctness here rests on code review, not CTest.
+Review verified this constraint is real and the deferral correct.
+
+**Intended fix.** Make `action_registry` a linkable library and give it the repo's
+first `shell/` test target, driven by a fake `reaper_plugin_info_t`. Its own header
+(`reaper_plugin.h:153-172`) shows `Register` is a plain member-function pointer on the
+struct, not a REAPER API pointer resolved through `REAPERAPI_LoadAPI` — a fake instance
+needs no live REAPER process to exercise `rec->Register(...)` calls. Once
+`action_registry` is test-covered, move the Media-Explorer section registration into it.
+
+**The constraint the fix MUST handle.** The extraction alone buys nothing:
+`action_registry` has no test target today either, so lifting `ingestHandleSectionCommand`
+into it without also standing up the test target just relocates the untested code. The
+same follow-up could collapse `ingest.cpp:466-472`'s hand-rolled `command_id`+`gaccel`
+pair onto `action_registry::registerAction`, which already does exactly that dance for
+the Q-W6 table.
+
+**Priority / risk.** Low / deferred. `ingestHandleSectionCommand` is a two-branch
+comparison, reviewed and correct at this scope; the gap is the missing test seam, not a
+known defect.
+
+**Done looks like.** `action_registry` is a linkable library with its own `shell/`-first
+CTest target driven by a fake `reaper_plugin_info_t`; the Media-Explorer section
+registration and `ingestHandleSectionCommand` move into it and gain unit coverage; and
+`ingest.cpp`'s own `command_id`+`gaccel` registration collapses onto
+`action_registry::registerAction` where the shapes match.

src/app/main.cpp

@@ -216,8 +216,8 @@ static project_config_extension_t g_projectConfig{
     nullptr,  // userData
 };
 
-// REAPER calls this for EVERY action fired anywhere; claim only our own id, return
+// REAPER calls this for every action fired in the MAIN section; claim only our own id,
-// false otherwise so REAPER keeps looking. This TU's own family dispatches through
+// return false otherwise so REAPER keeps looking. This TU's own family dispatches through
 // the registration table; the other families claim their own ids after it.
 static bool OnHookCommand(int command, int /*flag*/)
 {
@@ -229,6 +229,16 @@ static bool OnHookCommand(int command, int /*flag*/)
     return false;
 }
 
+// "hookcommand" covers the main section only, so actions we published into another
+// section arrive here instead. Partitioning contract: root `CLAUDE.md` §"REAPER
+// extension contract".
+static bool OnHookCommand2(KbdSectionInfo* /*sec*/, int command, int /*val*/, int /*val2*/,
+                           int /*relmode*/, HWND /*hwnd*/)
+{
+    if (command == 0) return false;
+    return reasampler::ingestHandleSectionCommand(command);
+}
+
 // REAPER polls this to render each of OUR actions' checked state in menus/toolbars.
 // Return 1 (on) / 0 (off) for ids we own, -1 for everything else (per the contract).
 static int OnToggleAction(int command)
@@ -255,6 +265,7 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT(
             g_rec->Register("-projectconfig", (void*)&g_projectConfig);
             g_rec->Register("-toggleaction", (void*)&OnToggleAction);
             g_rec->Register("-hookcommand", (void*)&OnHookCommand);
+            g_rec->Register("-hookcommand2", (void*)&OnHookCommand2);
             reasampler::designViewUnregisterActions(g_rec);
             reasampler::bankUnregisterActions(g_rec);
             reasampler::ingestUnregisterActions(g_rec);
@@ -307,6 +318,7 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT(
     reasampler::ingestRegisterActions(rec, &g_session);
 
     rec->Register("hookcommand", (void*)&OnHookCommand);
+    rec->Register("hookcommand2", (void*)&OnHookCommand2);
 
     // Drives project-load / Save-As detection: the timer polls the active project
     // each tick; on a project load it reloads the bank from ext state, on a Save-As

src/shell/actions/CLAUDE.md

@@ -34,7 +34,7 @@ is owned by other directories and only skinned here.
 - `shell/actions` (`action_registry` / `design_view_actions` / `bank_actions` / `prune_action`) — the bindable action families, all routed via the `command_id`/`gaccel`/`hookcommand` contract. `action_registry` owns the shared registration plumbing (interned channel-qualified id strings; register and mirror-unregister present the identical pointer) **and the Q-W6 registration TABLE**: `main.cpp`'s own family (capture scopes, panel toggle, insert, batch, realtime, recapture, version) is one `ActionTableRow` array — suffix, phrase, flat function-pointer handler — that registration, hookcommand dispatch, and the unload mirror-unregister all iterate, so adding an action touches the table only (OCP). Bank mutations flow through the promptless `shell/bank_ops` verbs (`bankOp*` + `persistBankOp`, taking `ReaSamplerSession&`), which the panel menus and `bank_actions` consume as thin UX skins. **Every bank index verb wraps its mutation in a batched REAPER undo point (`Undo_BeginBlock2`/`EndBlock2`, `UNDO_STATE_MISCCFG`) so one bank operation is one Ctrl-Z.** The prune action (`prune_action`, `BANK_PRUNE_FOLDER`) is **the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). `BANK_PRUNE_FOLDER` halts on `blockedByTracking` and prints each blocker that fired, with recovery instructions.
 - `drag_out_win` — OS drag-out shell: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`.
 - `instrument_drop_win` — FX-button drop shell: resolves a screen point to a track + FX-surface hotspot, then adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.**
-- `ingest` — ingest-through-the-bank shell on the EXTENSION side: three surfaces — (1) arrange capture→bank→assign (bindable action), (2) Media-Explorer import→bank→instrument on the selected track, (3) file drop onto the bank panel→bank only. Only surface (1) writes the `assignment_request` ext-state wire. **ingest NEVER inserts a timeline item.**
+- `ingest` — ingest-through-the-bank shell on the EXTENSION side: three surfaces — (1) arrange capture→bank→assign (bindable action), (2) Media-Explorer import→bank→instrument on the selected track, (3) file drop onto the bank panel→bank only. Only surface (1) writes the `assignment_request` ext-state wire. **ingest NEVER inserts a timeline item.** Surface (2)'s action is the one in this directory published into a NON-main action section (Media Explorer) as well as Main — two ids, one handler, two dispatch hooks; see root `CLAUDE.md` §"REAPER extension contract" for the mechanism.
 
 ## Gotchas
 

src/shell/actions/ingest.cpp

@@ -62,18 +62,22 @@ namespace {
 // Not owned here (main.cpp owns g_session).
 ReaSamplerSession* g_session = nullptr;
 
-// FOREVER-STABLE suffix — NEVER change after ship. Only the Media-Explorer import
+// Only the Media-Explorer import registers here — the arrange capture+assign action lives
-// registers here — the arrange capture+assign action lives in the capture family in
+// in the capture family in main.cpp, and the drop path is a panel callback
-// main.cpp, and the drop path is a panel callback (ingestDroppedFiles), not a
+// (ingestDroppedFiles), not a bindable action. Its two FOREVER-STABLE suffixes are in
-// bindable action.
+// ingest.h.
-constexpr const char* kIdImportMediaExplorer = "INGEST_IMPORT_MEDIA_EXPLORER";
+constexpr int kSectionMediaExplorer = 32063;
 
-int               g_cmdImportMediaExplorer = 0;
+int                       g_cmdImportMediaExplorer   = 0;
-gaccel_register_t g_accelImportMediaExplorer{};
+int                       g_cmdImportMediaExplorerMx = 0;
+gaccel_register_t         g_accelImportMediaExplorer{};
+custom_action_register_t  g_customImportMediaExplorer{};
 
 // c_str() pointers are handed to REAPER at register and re-presented at unregister,
-// so these strings must not be mutated after registration.
+// so these strings must not be mutated after registration. The label backs BOTH the
+// gaccel desc and the custom_action name.
 std::string g_idImportStr;
+std::string g_idImportMxStr;
 std::string g_labelImportStr;
 
 // Forward-slashed, no trailing slash. Empty for an unsaved/no-active project, which
@@ -457,14 +461,31 @@ void ingestDroppedFiles(const std::vector<std::string>& absolutePaths) {
 void ingestRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) {
     g_session = session;
 
-    g_idImportStr = channelCommandId(kIdImportMediaExplorer);
+    g_labelImportStr = channelActionName("import Media Explorer file into selected track");
+
+    g_idImportStr = channelCommandId(kIngestImportMediaExplorerId);
     g_cmdImportMediaExplorer = rec->Register("command_id", (void*)g_idImportStr.c_str());
     if (g_cmdImportMediaExplorer) {
-        g_labelImportStr = channelActionName("import Media Explorer file into selected track");
         g_accelImportMediaExplorer.accel.cmd = g_cmdImportMediaExplorer;
         g_accelImportMediaExplorer.desc      = g_labelImportStr.c_str();
         rec->Register("gaccel", (void*)&g_accelImportMediaExplorer);
     }
+
+    // Second publication of the SAME action, into the Media Explorer section, so it can
+    // be put on that window's toolbar. gaccel cannot express this — it registers into the
+    // main keyboard section only — so custom_action is the one mechanism. It carries no
+    // ACCEL, hence no default keybinding for this entry; toolbar reach is the point.
+    g_idImportMxStr = channelCommandId(kIngestImportMediaExplorerMxId);
+    g_customImportMediaExplorer.uniqueSectionId = kSectionMediaExplorer;
+    g_customImportMediaExplorer.idStr           = g_idImportMxStr.c_str();
+    g_customImportMediaExplorer.name            = g_labelImportStr.c_str();
+    g_customImportMediaExplorer.extra           = nullptr;
+    g_cmdImportMediaExplorerMx =
+        rec->Register("custom_action", (void*)&g_customImportMediaExplorer);
+    if (!g_cmdImportMediaExplorerMx)
+        ShowConsoleMsg("ReaSampler: could not publish the Media Explorer import action into "
+                       "the Media Explorer action section -- it is still available in the "
+                       "Main section.\n");
 }
 
 bool ingestHandleCommand(int command) {
@@ -473,10 +494,24 @@ bool ingestHandleCommand(int command) {
     return false;  // not ours — caller's hookcommand keeps looking
 }
 
+bool ingestHandleSectionCommand(int command) {
+    if (command == 0 || !g_session) return false;
+    // ONLY the Media Explorer id — partitioning contract: root `CLAUDE.md`
+    // §"REAPER extension contract".
+    if (command == g_cmdImportMediaExplorerMx) { doImportFromMediaExplorer(); return true; }
+    return false;
+}
+
 void ingestUnregisterActions(reaper_plugin_info_t* rec) {
+    // A 0 return from "custom_action" means REAPER holds no registration of ours (a dupe
+    // idStr is one documented cause) — mirroring it anyway could retire someone else's.
+    if (g_cmdImportMediaExplorerMx)
+        rec->Register("-custom_action", (void*)&g_customImportMediaExplorer);
     // '-command_id' re-presents the SAME interned id used at register (g_idImportStr).
     rec->Register("-gaccel", (void*)&g_accelImportMediaExplorer);
     rec->Register("-command_id", (void*)g_idImportStr.c_str());
+    g_cmdImportMediaExplorer   = 0;
+    g_cmdImportMediaExplorerMx = 0;
     g_session = nullptr;
 }
 

src/shell/actions/ingest.h

@@ -29,12 +29,25 @@ namespace reasampler {
 
 class ReaSamplerSession;
 
-// `session` is shared with the capture / bank / Design-View families; the single
+// FOREVER-STABLE command-id suffixes — NEVER change after ship; user keybindings key off
-// hookcommand in main.cpp routes fired ids here via ingestHandleCommand.
+// the composed ids. Two, because the Media-Explorer import publishes into two action
+// sections and a custom_action idStr must be unique across all of them. Exposed here so
+// the id-composition contract is assertable without linking this REAPER-facing TU.
+inline constexpr const char* kIngestImportMediaExplorerId   = "INGEST_IMPORT_MEDIA_EXPLORER";
+inline constexpr const char* kIngestImportMediaExplorerMxId = "INGEST_IMPORT_MEDIA_EXPLORER_MX";
+
+// `session` is shared with the capture / bank / Design-View families; main.cpp's two
+// dispatch hooks route fired ids back through the two handlers below. Both registration
+// mechanisms are specified in root CLAUDE.md §"REAPER extension contract".
 void ingestRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session);
 
+// Main-section dispatch ("hookcommand").
 bool ingestHandleCommand(int command);
 
+// Non-main-section dispatch ("hookcommand2"). Claims only ids this family published
+// outside the Main section, so the two hooks never both claim one command.
+bool ingestHandleSectionCommand(int command);
+
 void ingestUnregisterActions(reaper_plugin_info_t* rec);
 
 // "The active sampler instance should now play (bankId, sampleId)." Called by EVERY

tests/test_app_version.cpp

@@ -7,6 +7,10 @@
 
 #include "../src/core/version/app_version.h"
 
+// REAPER-free header; pulled in for the ingest action's FOREVER-STABLE id suffixes, so
+// the composition assertions below check the strings that actually ship.
+#include "../src/shell/actions/ingest.h"
+
 #include <cstdio>
 #include <string>
 
@@ -151,6 +155,27 @@ static void testChannelQualifiedIdAndNameComposition() {
     }
 }
 
+static void testMediaExplorerImportIdsAreDistinctAndChannelIsolated() {
+    // The Media-Explorer import publishes into TWO action sections, and a custom_action
+    // idStr must be unique across all sections — so the two entries carry two suffixes.
+    // Both are FOREVER-STABLE per channel. Composed from the SHIPPED constants and checked
+    // against spelled-out literals, so a suffix edit in ingest.h fails here.
+    const std::string mainId = channelCommandId(kIngestImportMediaExplorerId);
+    const std::string mxId   = channelCommandId(kIngestImportMediaExplorerMxId);
+
+    if (isBeta()) {
+        CHECK(mainId == "CEREBELLUM_REASAMPLER_BETA_INGEST_IMPORT_MEDIA_EXPLORER");
+        CHECK(mxId   == "CEREBELLUM_REASAMPLER_BETA_INGEST_IMPORT_MEDIA_EXPLORER_MX");
+        CHECK(channelActionName("import Media Explorer file into selected track") ==
+              "ReaSampler beta: import Media Explorer file into selected track");
+    } else {
+        CHECK(mainId == "CEREBELLUM_REASAMPLER_INGEST_IMPORT_MEDIA_EXPLORER");
+        CHECK(mxId   == "CEREBELLUM_REASAMPLER_INGEST_IMPORT_MEDIA_EXPLORER_MX");
+        CHECK(channelActionName("import Media Explorer file into selected track") ==
+              "ReaSampler: import Media Explorer file into selected track");
+    }
+}
+
 static void testStampClassifiesAsStampedOnOwnChannel() {
     // The V4 stamp-classifiability requirement: the value a channel WRITES (stampVersion())
     // must classify as Stamped when that same channel reads it back — on BOTH channels. A
@@ -277,6 +302,7 @@ int main() {
     testVstIdentityStringsForkByChannel();
     testVstIdentityAndDataNamespaceShareOneChannel();
     testChannelQualifiedIdAndNameComposition();
+    testMediaExplorerImportIdsAreDistinctAndChannelIsolated();
     testStampClassifiesAsStampedOnOwnChannel();
     testParseWellFormed();
     testParseRejectsMalformed();