fix(keys_highway_3d): stop auto-connect clobbering the global MIDI device (#825)

Opening the keys highway could silently switch the user's configured MIDI
device. Two coupled defects in the plugin's MIDI selection:

1. _midiAutoConnect only consulted the plugin's own localStorage pick
   (keys3d_midi_pick); with none saved it fell straight through to "first
   non-loopback device", ignoring the core midi-input domain's global
   selection (Settings -> Input Setup, window.slopsmith.midiInput.getSelected()).

2. _midiConnect unconditionally persisted every connect to BOTH the local
   pick and the shared domain selection (mi.select). So the first-device
   guess got frozen locally and overwrote the global default that other
   consumers (drums, Input Setup) rely on.

Make the domain-wide selection the source of truth: _pickMidiTarget now
resolves global -> legacy local pick (fallback + name-recovery for stale
ids) -> first device, and gates the "don't grab a random device" recovery
guard on any configured preference. Gate persistence behind an explicit
`persist` flag so only a deliberate device selection writes the local pick
and the shared global; auto-connect and programmatic (audio-input) opens
open the resolved device for the session without touching either store.
mi.select() is not needed to open (open takes the logicalSourceKey directly),
so dropping it from the auto path costs nothing.

Interim step toward instrument-scoped selection in the midi-input domain
itself (the input_setup wizard is already per-instrument, but the domain
stores a single selection); tracked as a separate core follow-up.

Pure decision logic extracted to _pickMidiTarget and covered by unit tests
in data_layer.test.js.

Signed-off-by: gionnibgud <gionnibgud@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

plugins/keys_highway_3d/screen.js

@@ -813,20 +813,36 @@
         _writeStore(STORE_KEYS.midiPick, JSON.stringify({ id: id || '', name: name || '', key: key || '' }));
     }
 
-    function _midiAutoConnect(allowFallback) {
+    // Pure decision logic (exported via __test): pick which device to
-        // Recovery (sources-changed after unplug) passes false: never switch to a
+    // auto-connect to from the current source list, the domain-wide selection
-        // fallback input, because _midiConnect persists the pick and that would
+    // (`globalKey`, from Settings → Input Setup), and this plugin's own legacy
-        // overwrite the user's saved device on a transient multi-device unplug
+    // saved pick. Returns null for "connect to nothing" (explicit None opt-out,
-        // (the original returns on replug and reconnects then).
+    // or the configured device currently absent during hotplug recovery).
-        if (allowFallback === undefined) allowFallback = true;
+    //
-        const inputs = _midiSources();
+    // The domain-wide selection is the SOURCE OF TRUTH (checked first): a device
-        if (!inputs.length) return;
+    // configured globally must never be overridden by a stale plugin-local pick
-        const saved = _readSavedPick();
+    // or an arbitrary first-device fallback — that override was the bug. The
-        // Explicit "None" opt-out.
+    // local pick is retained only as a fallback BELOW the global (and for
-        if (saved && saved.id === '' && saved.name === '') return;
+    // name-recovery when the global's logicalSourceKey went stale, e.g. a
-        // Prefer the globally-unique logicalSourceKey, then the legacy bare
+    // browser that regenerates MIDI port ids across reloads). Auto-connect no
-        // sourceId, then case-insensitive name (Chrome on Linux regenerates ids
+    // longer writes the local pick, so it only ever holds a value an explicit
-        // per page load), then first non-loopback.
+    // selection put there (or a stale one from a pre-fix build — the global
+    // still wins over it).
+    function _pickMidiTarget(inputs, saved, globalKey, allowFallback) {
+        if (!inputs.length) return null;
+        const notBlocked = (i) => !!i && !_MIDI_BLOCKLIST_RE.test(i.name || '');
+        // Explicit "None" opt-out (set only via the device-select API).
+        if (saved && saved.id === '' && saved.name === '') return null;
+
+        // 1. Domain-wide selection (Settings → Input Setup) — source of truth.
+        if (globalKey) {
+            const g = inputs.find(i => i.key === globalKey);
+            if (notBlocked(g)) return g;
+        }
+
+        // 2. Legacy plugin-local pick, as a fallback below the global. Prefer the
+        // globally-unique logicalSourceKey, then the legacy bare sourceId, then
+        // case-insensitive name (Chrome on Linux regenerates ids per page load).
         let target = null;
         if (saved && saved.key) target = inputs.find(i => i.key === saved.key) || null;
         if (!target && saved && saved.id) target = inputs.find(i => i.id === saved.id) || null;
@@ -834,23 +850,50 @@
             const n = saved.name.toLowerCase();
             target = inputs.find(i => (i.name || '').toLowerCase() === n) || null;
         }
-        // Never honour a saved pick that's a loopback / "Midi Through" port — it
+        // Never honour a saved pick that resolves to a loopback / "Midi Through"
-        // carries no device input, so a stale pick silently eats every note. The
+        // port — it carries no device input, so it silently eats every note.
-        // saved-pick lookups above bypass the block-list; re-apply it here.
+        if (target && !notBlocked(target)) target = null;
-        if (target && _MIDI_BLOCKLIST_RE.test(target.name || '')) target = null;
+        if (target) return target;
-        if (!target) {
+
-            // Skip the substitute ONLY when a saved pick exists but is currently
+        // 3. Nothing configured resolved to a present device. In recovery
-            // absent (recovery: preserve it, don't clobber on a transient unplug).
+        // (allowFallback=false) with a configured preference — a global pick or a
-            // With no saved pick at all, a fallback is the intended first-hotplug
+        // saved pick — that's currently absent, preserve it rather than switching
-            // auto-connect — allow it even in recovery.
+        // to an arbitrary device on a transient multi-device unplug. With no
-            const hasSavedPick = !!(saved && (saved.key || saved.id || saved.name));
+        // preference at all, a first-device grab is the intended first-hotplug
-            if (!allowFallback && hasSavedPick) return;
+        // auto-connect, allowed even in recovery.
-            target = inputs.find(i => !_MIDI_BLOCKLIST_RE.test(i.name || '')) || inputs[0];
+        const hasPreference = !!(globalKey || (saved && (saved.key || saved.id || saved.name)));
-        }
+        if (!allowFallback && hasPreference) return null;
+        // Connect to nothing rather than a loopback: if every present device is
+        // blocklisted, a first-device grab would attach to a "Midi Through"/IAC
+        // port that carries no input and silently eats every note.
+        return inputs.find(notBlocked) || null;
+    }
+
+    function _midiAutoConnect(allowFallback) {
+        // Recovery (sources-changed after unplug) passes false: never switch to a
+        // fallback input on a transient multi-device unplug (the configured
+        // device returns on replug and reconnects then). Auto-connect is
+        // non-persisting (persist omitted → false): it opens the resolved device
+        // for this session WITHOUT writing the plugin-local pick or the shared
+        // domain selection, so opening this highway can't clobber the user's
+        // globally-configured device.
+        if (allowFallback === undefined) allowFallback = true;
+        const inputs = _midiSources();
+        const saved = _readSavedPick();
+        const mi = _mi();
+        const globalKey = mi && typeof mi.getSelected === 'function' ? mi.getSelected() : null;
+        const target = _pickMidiTarget(inputs, saved, globalKey, allowFallback);
+        if (!target) return;
         _midiConnect(target.id, target.name, target.key);
     }
 
-    async function _midiConnect(id, name, key) {
+    // `persist` gates the two preference writes. Only an EXPLICIT device
+    // selection (the device-select API) persists: it writes the plugin-local
+    // pick AND the shared domain selection (`mi.select`, so the user's choice
+    // becomes the global default). Auto-connect and programmatic opens pass
+    // falsy — they open the resolved device for this session only, never
+    // touching either store, so they can't clobber a globally-configured device.
+    async function _midiConnect(id, name, key, persist) {
         // Capture our generation AFTER _midiDetach()'s own bump, so a later
         // detach (device removal / new connect / opt-out) reliably supersedes us.
         _midiDetach();
@@ -861,7 +904,7 @@
         for (const inst of _instances) {
             if (inst && typeof inst._releaseAllHeld === 'function') inst._releaseAllHeld();
         }
-        _writeSavedPick(id || '', name || '', key || '');
+        if (persist) _writeSavedPick(id || '', name || '', key || '');
         const mi = _mi();
         if ((id || key) && mi) {
             // Prefer the globally-unique logicalSourceKey so two providers that
@@ -874,13 +917,19 @@
                 const lkey = src.key || ('web-midi::' + src.id);
                 _midiInput = { id: src.id, name: src.name, key: lkey };
                 _midiJustConnected = true;
+                // Only an explicit selection writes the shared global default;
+                // open takes the logicalSourceKey directly, so select() is not
+                // needed to open — it exists purely to set the global. Persist it
+                // BEFORE the no-instance early return so a settings-panel pick with
+                // no live renderer still updates the shared default (best-effort:
+                // a select hiccup must not abort the connect).
+                if (persist) { try { await mi.select(lkey); } catch (_) { /* best-effort */ } }
                 // No live renderer to consume OR release a session — don't hold one
                 // open (settings-only ensure-init, or the last instance was torn
-                // down during async discovery). The pick is saved; a later renderer
+                // down during async discovery). A later renderer mount re-runs
-                // mount re-runs auto-connect and opens for real, releasing on destroy.
+                // auto-connect and opens for real, releasing on destroy.
                 if (_instances.size === 0) { _midiNotifyDeviceListChanged(); return; }
                 try {
-                    await mi.select(lkey);
                     const res = await mi.open({ requester: PLUGIN_ID, logicalSourceKey: lkey });
                     // A newer _midiConnect (device switch / None / replug) ran while
                     // we awaited open — discard this stale session so we don't wire a
@@ -1039,10 +1088,11 @@
     window.keysH3dGetMidiInputId = function () { return _midiInput ? _midiInput.id : ''; };
     window.keysH3dSetMidiInput = function (id) {
         // `id` may be a logicalSourceKey (new host calls) or a legacy sourceId.
+        // Explicit user selection → persist (local pick + shared global default).
         const src = id
             ? (_midiSources().find(s => s.key === id) || _midiSources().find(s => s.id === id))
             : null;
-        _midiConnect(src ? src.id : (id || ''), src ? src.name : '', src ? src.key : '');
+        _midiConnect(src ? src.id : (id || ''), src ? src.name : '', src ? src.key : '', true);
         return true;
     };
     window.keysH3dGetMidiChannel = function () { return _cfg.midiChannel; };
@@ -1547,6 +1597,8 @@
 
     function _aiOpen(req) {
         // Opening a MIDI source connects the corresponding Web MIDI input.
+        // Programmatic open (audio-input source.open) — non-persisting: it must
+        // not rewrite the user's saved pick or the shared global default.
         const idx = _aiIndexFor(req && (req.sourceId || req.logicalSourceKey));
         const inputs = _midiSources();   // carries .key (logicalSourceKey), unlike _midiListInputs()
         if (idx == null || idx >= inputs.length) {
@@ -4068,6 +4120,7 @@
         FX_DEFAULTS,
         FX_RANGES,
         _classifyTiming,
+        _pickMidiTarget,
     };
 
     // Headless verification hook: lets Playwright drive synthetic note-ons

plugins/keys_highway_3d/tests/data_layer.test.js

@@ -188,3 +188,99 @@ test('measureMarkers extracts idx/t pairs', () => {
         [{ idx: 1, t: 0 }, { idx: 2, t: 2.5 }],
     );
 });
+
+test('_pickMidiTarget: no plugin-local pick defers to the domain-wide selection, not "first device"', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [
+        { id: 'a', name: 'Device A', key: 'web-midi::a' },
+        { id: 'b', name: 'Device B', key: 'web-midi::b' },
+    ];
+    // Fresh install / never picked here — must use the Input Setup global,
+    // NOT fall through to inputs[0].
+    const target = _pickMidiTarget(inputs, null, 'web-midi::b', true);
+    assert.equal(target.id, 'b');
+});
+
+test('_pickMidiTarget: the domain-wide selection is the source of truth — it wins over a stale plugin-local pick', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [
+        { id: 'a', name: 'Device A', key: 'web-midi::a' },
+        { id: 'b', name: 'Device B', key: 'web-midi::b' },
+    ];
+    // A stale local pick (e.g. left by a pre-fix build's auto-connect) must
+    // NOT override the device the user configured in Settings → Input Setup.
+    const target = _pickMidiTarget(inputs, { id: 'a', name: 'Device A', key: 'web-midi::a' }, 'web-midi::b', true);
+    assert.equal(target.id, 'b');
+});
+
+test('_pickMidiTarget: local pick is used as a fallback when no global is configured', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [
+        { id: 'a', name: 'Device A', key: 'web-midi::a' },
+        { id: 'b', name: 'Device B', key: 'web-midi::b' },
+    ];
+    const target = _pickMidiTarget(inputs, { id: 'a', name: 'Device A', key: 'web-midi::a' }, null, true);
+    assert.equal(target.id, 'a');
+});
+
+test('_pickMidiTarget: local pick name-recovers when its logicalSourceKey went stale (id regeneration)', () => {
+    const { _pickMidiTarget } = load();
+    // Same physical device, new id/key across a reload; the saved key/id miss
+    // but the name still matches.
+    const inputs = [{ id: 'a2', name: 'Device A', key: 'web-midi::a2' }];
+    const target = _pickMidiTarget(inputs, { id: 'a1', name: 'Device A', key: 'web-midi::a1' }, null, true);
+    assert.equal(target.id, 'a2');
+});
+
+test('_pickMidiTarget: domain-wide selection is ignored if it names a blocklisted loopback port', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [
+        { id: 'thru', name: 'IAC Driver Bus 1', key: 'web-midi::thru' },
+        { id: 'b', name: 'Device B', key: 'web-midi::b' },
+    ];
+    const target = _pickMidiTarget(inputs, null, 'web-midi::thru', true);
+    assert.equal(target.id, 'b'); // falls through to the first non-loopback device
+});
+
+test('_pickMidiTarget: when every present device is a loopback, connect to nothing (never a dead port)', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [
+        { id: 'thru', name: 'MIDI Through Port-0', key: 'web-midi::thru' },
+        { id: 'iac', name: 'IAC Driver Bus 1', key: 'web-midi::iac' },
+    ];
+    // No non-loopback device exists — must NOT fall back to inputs[0] (a port
+    // that carries no input and would silently eat every note).
+    const target = _pickMidiTarget(inputs, null, null, true);
+    assert.equal(target, null);
+});
+
+test('_pickMidiTarget: explicit "None" opt-out still wins over any global default', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [{ id: 'a', name: 'Device A', key: 'web-midi::a' }];
+    const target = _pickMidiTarget(inputs, { id: '', name: '' }, 'web-midi::a', true);
+    assert.equal(target, null);
+});
+
+test('_pickMidiTarget: a present global wins even during hotplug recovery', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [{ id: 'b', name: 'Device B', key: 'web-midi::b' }];
+    // The configured global device is present — reconnect to it, don't bail.
+    const target = _pickMidiTarget(inputs, null, 'web-midi::b', false);
+    assert.equal(target.id, 'b');
+});
+
+test('_pickMidiTarget: recovery (allowFallback=false) preserves an absent configured device instead of grabbing a random one', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [{ id: 'b', name: 'Device B', key: 'web-midi::b' }];
+    // The configured device ('x', global) is currently unplugged; a transient
+    // recovery must NOT switch to the unrelated device that is present.
+    const target = _pickMidiTarget(inputs, null, 'web-midi::x', false);
+    assert.equal(target, null);
+});
+
+test('_pickMidiTarget: recovery with no preference at all still allows a first-hotplug grab', () => {
+    const { _pickMidiTarget } = load();
+    const inputs = [{ id: 'b', name: 'Device B', key: 'web-midi::b' }];
+    const target = _pickMidiTarget(inputs, null, null, false);
+    assert.equal(target.id, 'b');
+});