Make MIDI automatic: on by default, output connects on the first MOO message

agent-notes/feat-auto-midi.md

@@ -0,0 +1,33 @@
+# feat/auto-midi: make MIDI automatic
+
+## Goal
+MOO MIDI plays without the user setting anything up:
+1. `midi.enabled` defaults to true; a storage migration turns it on once for old saved prefs, then the user's choice sticks.
+2. When a MIDI message arrives from the MOO (`src/gmcp/Client/Midi.ts`), connect an output if none is connected and the user did not disconnect on purpose: saved `lastOutputDeviceId` if it is in the device list, otherwise `SOUNDFONT_OUTPUT`. One in-flight connect shared by concurrent notes.
+3. Report whether `JZZ()` calls `navigator.requestMIDIAccess`; avoid initializing JZZ on the soundfont path.
+
+## State
+- Worktree `C:\Users\Q\code\react-client-auto-midi`, branch `feat/auto-midi` off origin/master b26b4bd4.
+- Worktree had no node_modules; ran `npm ci` (lockfile identical to main checkout's).
+- Done: tests written first and seen failing (Midi.test.ts 5 failed; preferencesStore.test.ts 3 failed), then implemented; both files pass (29 tests). Full suite/typecheck left to PR CI.
+- `preferencesStore.ts`: STORAGE_VERSION 3 (131), default `midi.enabled: true` (155), migration `storedVersion < 3` sets `midi.enabled = true` (214-216).
+- PR #202 open. Second commit: midi-tab gets `mountOnSelect: true` (sidebar.tsx:220), like haptics (228). Test in sidebar.test.tsx shows MidiStatus is not mounted until the MIDI tab is clicked (failed before the change, passes after).
+- CI run 39531908947 (head ea28d5a1) failed "Verify lazy audio caching": `scripts/check-audio-cache.mjs:108` saw `VirtualMidiService-*.js` and `MidiService-*.js` load at startup, because MIDI now defaults on. Third commit fixes it in code (script unchanged):
+  - App.tsx: removed the `midiEnabled` effect that imported VirtualMidiService, bound `setPlaybackSource`/`setAudioOutput(client.media.getMidiAudioOutput())` and called `initialize()` (was App.tsx:415-449), and the `midiEnabled` selector (was 165).
+  - Midi.ts `loadMidiService()` (117-129) now loads MidiService and VirtualMidiService together and does that wiring before resolving, so it runs before any output connects (every connect path goes through `loadMidiService`: automatic connect, `ensureInitialized`, `connectOutputDevice`).
+  - client.ts: removed `resetMidiIntentionalDisconnectFlags` (was 41-51, imported MidiService on every connect). onopen (244) and the worker path (446) call `this.gmcp.handlers["Client.Midi"]?.resetIntentionalDisconnects()`, which (Midi.ts:132-134) resets only if MidiService already loaded.
+  - Midi.ts `sendAllNotesOff` (419) uses `midiServicePromise?.then` instead of loading MidiService: `reset()` runs on every disconnect (session.ts registers `registerDisconnectReset`), so with MIDI on it would also have loaded the chunk at startup when the socket closes.
+- `Midi.ts`: `loadMidiOutput()` (128-136) shares one `outputConnecting` promise (96); `connectAutomaticOutput()` (138-149) picks saved output or `SOUNDFONT_OUTPUT`. Used by `handleNoteAsync` (291) and `sendRawToOutput` (346, renamed from `sendRawIfOutputConnected`).
+
+## Decisions and why
+- Soundfont path skips `ensureInitialized()`: `connectOutputDevice(SOUNDFONT_OUTPUT)` (MidiService.ts:293-319) never touches `this.jzz`, and `JZZ()` reaches `navigator.requestMIDIAccess` (see Surprises). JZZ is only initialized when the saved output is something other than the soundfont (a hardware port needs JZZ to appear in the device list).
+- Wiring moved into `GMCPClientMidi` rather than gated in App: the package already owns the one lazy load and has `this.client.media`. Lost from App's effect: releasing the playback source when MIDI is turned off or the App's client changes; the binding now lasts for the package's lifetime. `virtualMidiService.initialize()` is not awaited, because it waits while playback is paused (VirtualMidiService.ts:106-110) and would hold up notes.
+- The in-flight promise is cleared when it settles, so a failed connect is retried by the next message rather than never.
+
+## Surprises
+- JZZ 1.9.6 (`node_modules/jzz/javascript/JZZ.js`): `JZZ()` (1342-1345) runs `_initJZZ` (812-818), which tries engines `node, extension, plugin, webmidi` (790); the webmidi engine `_tryWebMIDI` (697-714) calls `_requestMIDIAccess.call(_navigator, {})` (709), i.e. `navigator.requestMIDIAccess` (688-690).
+- `MidiStatus` (the MIDI sidebar tab) calls `midiPackage.ensureInitialized()` on mount when MIDI is enabled (MidiStatus.tsx:219-223), and the midi tab has no `mountOnSelect` (sidebar.tsx:212-221, tabs.tsx:139), so with MIDI on by default JZZ was still initialized when the sidebar rendered. The coordinator ruled to add `mountOnSelect: true`; done in the second commit. Selecting the MIDI tab still initializes JZZ, which is expected (it lists hardware ports).
+- Saved `virtual-synth` only appears in `getOutputDevices()` once `virtualMidiService.initialized` (MidiService.ts:172); otherwise the auto-connect falls back to the soundfont, per the plan's "in the device list" rule.
+
+## Next step
+Read PR #202's CI result.

src/App.test.tsx

@@ -51,6 +51,13 @@ const {
   };
 });
 
+const { virtualMidiLoaded } = vi.hoisted(() => ({ virtualMidiLoaded: vi.fn() }));
+
+vi.mock('./VirtualMidiService', () => {
+  virtualMidiLoaded();
+  return { virtualMidiService: { setPlaybackSource: vi.fn(() => () => {}), setAudioOutput: vi.fn(async () => {}), initialize: vi.fn(async () => true) } };
+});
+
 vi.mock('react-beforeunload', () => ({
   useBeforeunload: vi.fn(),
 }));
@@ -183,6 +190,13 @@ describe('App haptics backend lifecycle', () => {
     cleanup();
   });
 
+  it('does not load the MIDI synth at startup when MIDI is on', async () => {
+    mockPreferences.midi.enabled = true;
+    render(<App />);
+    await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); });
+    expect(virtualMidiLoaded).not.toHaveBeenCalled();
+  });
+
   it('loads haptics only after enabling and disposes its runtime on cleanup', async () => {
     const view = render(<App />);
 

src/App.tsx

@@ -162,7 +162,6 @@ function App() {
 
   const clientInitialized = useRef(false);
   const hapticsRuntimeRef = useRef<HapticsRuntime | null>(null);
-  const midiEnabled = usePreferences((state) => state.midi.enabled);
   const hapticsEnabled = usePreferences((state) => state.haptics.enabled);
   const connected = useConnectionStore((state) => state.connected);
   useFileTransferNotifications(client);
@@ -412,42 +411,6 @@ function App() {
     };
   }, [handleAppKeyDown]);
 
-  useEffect(() => {
-    if (!midiEnabled) return;
-
-    let cancelled = false;
-    let releasePlayback: (() => void) | undefined;
-    import("./VirtualMidiService")
-      .then(async ({ virtualMidiService }) => {
-        if (cancelled) return false;
-        releasePlayback = virtualMidiService.setPlaybackSource(client?.media ?? null);
-        if (client) {
-          // Render into the client's master gain so the synth shares its
-          // clock, volume, mute, and autoplay unlock.
-          await virtualMidiService.setAudioOutput(client.media.getMidiAudioOutput());
-        }
-        if (cancelled) return false;
-        return virtualMidiService.initialize();
-      })
-      .then((success) => {
-        if (cancelled) return;
-        if (success) {
-          console.log("Virtual MIDI synthesizer initialized");
-        } else {
-          console.log("Failed to initialize virtual MIDI synthesizer");
-        }
-      })
-      .catch((error) => {
-        if (cancelled) return;
-        console.error("Error initializing virtual MIDI synthesizer:", error);
-      });
-
-    return () => {
-      cancelled = true;
-      releasePlayback?.();
-    };
-  }, [midiEnabled, client]);
-
   // Window subtitle tracks the current room from the room store. On disconnect
   // the client resets the store, which clears roomInfo and so clears the subtitle.
   const roomInfo = useRoomStore((state) => state.roomInfo);

src/client.test.ts

@@ -229,6 +229,13 @@ class MockAutoLoginPackage {
   shutdown = vi.fn();
 }
 
+class MockClientMidiPackage {
+  packageName = 'Client.Midi';
+  resetIntentionalDisconnects = vi.fn();
+  reset = vi.fn();
+  shutdown = vi.fn();
+}
+
 class MockClientMediaPackage {
   packageName = 'Client.Media';
   packageVersion = 1;
@@ -686,6 +693,16 @@ describe('MudClient lifecycle cleanup', () => {
     client.close();
   });
 
+  it('resets MIDI intentional disconnects through the MIDI package when the socket opens', () => {
+    const client = new MudClient('example.test', 443);
+    const midi = client.gmcp.register(MockClientMidiPackage as never) as unknown as MockClientMidiPackage;
+    client.connect();
+    expect(midi.resetIntentionalDisconnects).not.toHaveBeenCalled();
+    mockWebSocketInstances[0].onopen?.(new Event('open'));
+    expect(midi.resetIntentionalDisconnects).toHaveBeenCalledOnce();
+    client.close();
+  });
+
   it('sends through a connected open WebSocket', () => {
     const client = new MudClient('example.test', 443);
     client.connect();

src/client.ts

@@ -38,18 +38,6 @@ import { clientSpeechSuspended } from './clientSpeechSuspension';
 // treated as dead.
 const LIVENESS_TIMEOUT_MS = 4000;
 
-function resetMidiIntentionalDisconnectFlags(): void {
-  if (!usePreferences.getState().midi.enabled) return;
-
-  import("./MidiService")
-    .then(({ midiService }) => {
-      midiService.resetIntentionalDisconnectFlags();
-    })
-    .catch((error) => {
-      console.error("Failed to reset MIDI disconnect flags:", error);
-    });
-}
-
 class MudClient {
   private ws!: WebSocket;
   private decoder = new TextDecoder("utf8");
@@ -253,7 +241,7 @@ class MudClient {
       this.reconnectAttempt = 0;
 
       // Reset MIDI intentional disconnect flags when successfully reconnecting to server
-      resetMidiIntentionalDisconnectFlags();
+      this.gmcp.handlers["Client.Midi"]?.resetIntentionalDisconnects();
 
       useConnectionStore.getState().setConnected(true);
     };
@@ -455,7 +443,7 @@ class MudClient {
     // Mark as connected immediately — the worker will send the "connected"
     // message once the virtual connection is created.
     this._connected = true;
-    resetMidiIntentionalDisconnectFlags();
+    this.gmcp.handlers["Client.Midi"]?.resetIntentionalDisconnects();
     useConnectionStore.getState().setConnected(true);
     this.gmcp.markReady();
   }

src/components/sidebar.test.tsx

@@ -12,6 +12,9 @@ vi.mock('./ServerFeaturesPanel', () => ({ default: () => <span>Server informatio
 vi.mock('./AudioDiagnostics', () => ({ default: () => <span>Diagnostics</span> }));
 vi.mock('./HelpPanel', () => ({ default: () => <span>Help contents</span> }));
 vi.mock('./MapPanel', () => ({ default: () => <span>Map contents</span> }));
+vi.mock('./MidiStatus', () => ({ default: () => <span>MIDI contents</span> }));
+
+const client = { connected: false, gmcp: { handlers: {} }, media: { diagnostics: {} }, onFileTransferOffer: () => () => {} } as unknown as MudClient;
 
 beforeEach(() => {
   useRoomStore.getState().reset();
@@ -21,7 +24,6 @@ beforeEach(() => {
 
 it('reaches active destinations repeatedly and keeps shortcut positions stable as tabs arrive', () => {
   const ref = React.createRef<SidebarRef>();
-  const client = { connected: false, gmcp: { handlers: {} }, media: { diagnostics: {} }, onFileTransferOffer: () => () => {} } as unknown as MudClient;
   render(<><button type="button">Elsewhere</button><Sidebar ref={ref} client={client} collapsed={false} onToggleCollapse={vi.fn()} /></>);
   expect(screen.getByRole('tablist', { name: 'Sidebar' })).toBeTruthy();
   const outside = screen.getByRole('button', { name: 'Elsewhere' });
@@ -39,8 +41,16 @@ it('reaches active destinations repeatedly and keeps shortcut positions stable a
   expect(screen.getByRole('tab', { name: 'Users' })).toHaveAttribute('aria-keyshortcuts', 'Control+Shift+3');
 });
 
+it('does not mount the MIDI panel, which initializes Web MIDI, until its tab is selected', async () => {
+  usePreferences.setState({ midi: { enabled: true } });
+  render(<Sidebar client={client} collapsed={false} onToggleCollapse={vi.fn()} />);
+  await act(async () => { await import('./MidiStatus'); });
+  expect(screen.queryByText('MIDI contents')).not.toBeInTheDocument();
+  fireEvent.click(screen.getByRole('tab', { name: 'MIDI' }));
+  expect(await screen.findByText('MIDI contents')).toBeInTheDocument();
+});
+
 it('puts Help last, after Map, with no number shortcut of its own', async () => {
-  const client = { connected: false, gmcp: { handlers: {} }, media: { diagnostics: {} }, onFileTransferOffer: () => () => {} } as unknown as MudClient;
   act(() => useRoomStore.getState().setRoomInfo({ num: 1, name: 'Room', exits: {} }));
   render(<Sidebar client={client} collapsed={false} onToggleCollapse={vi.fn()} />);
   const tabs = screen.getAllByRole('tab');

src/components/sidebar.tsx

@@ -217,6 +217,7 @@ const Sidebar = React.forwardRef<SidebarRef, SidebarProps>(
             <MidiStatus client={client} />
           </Suspense>
         ),
+        mountOnSelect: true,
         condition: midiPreferences.enabled,
       },
       {