From 24ba7eca35e077eb8edb725503cee551ed8edb6d Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Tue, 20 Oct 2026 20:11:03 -0600 Subject: [PATCH 1/3] Make MIDI automatic: on by default, output connects on the first MOO message - preferences: midi.enabled defaults to true; STORAGE_VERSION 3 turns it on once for older saves, after which the user's choice sticks. - Client.Midi: before sending a MOO note/CC/program/system/raw message, connect an output if none is connected and the user did not disconnect on purpose: the saved output when it is in the device list, else the soundfont. One in-flight connect is shared by concurrent messages. The soundfont path does not initialize JZZ, whose webmidi engine calls navigator.requestMIDIAccess. Co-Authored-By: Claude --- agent-notes/feat-auto-midi.md | 26 ++++++++ src/gmcp/Client/Midi.test.ts | 95 ++++++++++++++++++++++++++--- src/gmcp/Client/Midi.ts | 43 ++++++++++--- src/stores/preferencesStore.test.ts | 17 +++++- src/stores/preferencesStore.ts | 9 ++- 5 files changed, 173 insertions(+), 17 deletions(-) create mode 100644 agent-notes/feat-auto-midi.md diff --git a/agent-notes/feat-auto-midi.md b/agent-notes/feat-auto-midi.md new file mode 100644 index 00000000..dc23a078 --- /dev/null +++ b/agent-notes/feat-auto-midi.md @@ -0,0 +1,26 @@ +# 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). +- `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). +- 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 is still initialized when the sidebar renders. Out of this task's scope; reported, not changed. +- 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 the PR's CI result. Open question for Q (not acted on): whether the MIDI tab should get `mountOnSelect` so the sidebar does not initialize JZZ at load. diff --git a/src/gmcp/Client/Midi.test.ts b/src/gmcp/Client/Midi.test.ts index 85f4dc41..024ef514 100644 --- a/src/gmcp/Client/Midi.test.ts +++ b/src/gmcp/Client/Midi.test.ts @@ -1,12 +1,25 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { midiService } = vi.hoisted(() => ({ - midiService: { - connectionStatus: { inputConnected: false, outputConnected: true }, - hasOutputDevice: true, - sendRawMessage: vi.fn(), - }, -})); +const { midiService, output } = vi.hoisted(() => { + const output = { connected: true, intentional: false, initialized: false }; + return { + output, + midiService: { + get connectionStatus() { return { inputConnected: false, outputConnected: output.connected }; }, + get intentionalDisconnectStatus() { return { input: false, output: output.intentional }; }, + get hasOutputDevice() { return output.connected; }, + get isInitialized() { return output.initialized; }, + isSupported: true, + initialize: vi.fn(async () => { output.initialized = true; return true; }), + attemptAutoReconnectInput: vi.fn(async () => false), + getOutputDevices: vi.fn(() => [{ id: 'soundfont-synth', name: 'Soundfont Synthesizer' }]), + connectOutputDevice: vi.fn(async () => { await Promise.resolve(); output.connected = true; return true; }), + sendNote: vi.fn(), + scheduleNoteOff: vi.fn(() => true), + sendRawMessage: vi.fn(), + }, + }; +}); vi.mock('../../MidiService', () => ({ midiService })); @@ -74,6 +87,74 @@ describe('GMCPClientMidi Song', () => { }); }); +describe('GMCPClientMidi automatic output', () => { + const previous = usePreferences.getState().midi; + let midi: GMCPClientMidi; + + beforeEach(() => { + vi.clearAllMocks(); + Object.assign(output, { connected: false, intentional: false, initialized: false }); + usePreferences.getState().setMidi({ ...previous, enabled: true, lastOutputDeviceId: undefined }); + midi = new GMCPClientMidi({ gmcp: { send: vi.fn() } } as never); + }); + + afterEach(() => { + Object.assign(output, { connected: true, intentional: false, initialized: false }); + usePreferences.getState().setMidi(previous); + }); + + const note = (n: number) => midi.receiveRegisteredMessage('Note', { note: n, velocity: 90, on: true }); + + it('connects the soundfont before the first MOO note, without initializing Web MIDI', async () => { + note(60); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalledWith({ note: 60, velocity: 90, on: true, channel: undefined })); + expect(midiService.connectOutputDevice).toHaveBeenCalledWith('soundfont-synth'); + expect(midiService.connectOutputDevice.mock.invocationCallOrder[0]) + .toBeLessThan(midiService.sendNote.mock.invocationCallOrder[0]); + expect(midiService.initialize).not.toHaveBeenCalled(); + }); + + it('connects once for notes that arrive while the output is connecting', async () => { + note(60); + note(64); + note(67); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalledTimes(3)); + expect(midiService.connectOutputDevice).toHaveBeenCalledOnce(); + }); + + it('does not connect after the user disconnected the output on purpose', async () => { + output.intentional = true; + note(60); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalled()); + expect(midiService.connectOutputDevice).not.toHaveBeenCalled(); + }); + + it('initializes and connects the saved output when it is in the device list', async () => { + usePreferences.getState().setMidi({ ...usePreferences.getState().midi, lastOutputDeviceId: 'hw-1' }); + midiService.getOutputDevices.mockReturnValue([ + { id: 'soundfont-synth', name: 'Soundfont Synthesizer' }, { id: 'hw-1', name: 'Port 1' }, + ]); + note(60); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalled()); + expect(midiService.initialize).toHaveBeenCalledOnce(); + expect(midiService.connectOutputDevice).toHaveBeenCalledWith('hw-1'); + }); + + it('falls back to the soundfont when the saved output is gone', async () => { + usePreferences.getState().setMidi({ ...usePreferences.getState().midi, lastOutputDeviceId: 'hw-gone' }); + midiService.getOutputDevices.mockReturnValue([{ id: 'soundfont-synth', name: 'Soundfont Synthesizer' }]); + note(60); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalled()); + expect(midiService.connectOutputDevice).toHaveBeenCalledWith('soundfont-synth'); + }); + + it('connects an output for a MOO ProgramChange before sending it', async () => { + midi.receiveRegisteredMessage('ProgramChange', { channel: 0, program: 5 }); + await vi.waitFor(() => expect(midiService.sendRawMessage).toHaveBeenCalledWith([0xc0, 5])); + expect(midiService.connectOutputDevice).toHaveBeenCalledWith('soundfont-synth'); + }); +}); + describe('GMCPClientMidi effects route', () => { it('routes browser synths through the media chain system without loading a synth', () => { const previous = usePreferences.getState().midi; diff --git a/src/gmcp/Client/Midi.ts b/src/gmcp/Client/Midi.ts index 36ceb594..69b21223 100644 --- a/src/gmcp/Client/Midi.ts +++ b/src/gmcp/Client/Midi.ts @@ -2,6 +2,7 @@ import type { MidiMessage, MidiNote } from "../../MidiService"; import { usePreferences } from "../../stores/preferencesStore"; import { duplex, inbound } from "../../protocol/messages"; import { decodeMidiRoute, type MidiRoute } from '../../audio/MidiAudioRoute'; +import { SOUNDFONT_OUTPUT } from "../../midi/soundfonts"; import { gmcpJsonMessage } from "../messages"; import { GMCPMessage, GMCPPackage } from "../package"; @@ -92,6 +93,7 @@ export class GMCPClientMidi extends GMCPClientMidiBase { private isAdvertised: boolean = false; private debugCallback?: (hex: string, type: string, gmcpMessage: string) => void; private midiServicePromise: Promise | null = null; + private outputConnecting: Promise | null = null; private serviceInitialized = false; private hasInputDeviceConnected = false; private hasOutputDeviceConnected = false; @@ -119,6 +121,33 @@ export class GMCPClientMidi extends GMCPClientMidiBase { return this.midiServicePromise; } + /** + * The service, with an output connected for a MOO message unless the user + * disconnected it on purpose. Concurrent callers share one connect. + */ + private async loadMidiOutput(): Promise { + const midiService = await this.loadMidiService(); + if (!midiService.connectionStatus.outputConnected && !midiService.intentionalDisconnectStatus.output) { + this.outputConnecting ??= this.connectAutomaticOutput(midiService) + .finally(() => { this.outputConnecting = null; }); + await this.outputConnecting; + } + return midiService; + } + + private async connectAutomaticOutput(midiService: MidiService): Promise { + const saved = usePreferences.getState().midi.lastOutputDeviceId; + // The soundfont needs no Web MIDI, so only a saved hardware or virtual + // output initializes JZZ (and its permission prompt). + if (saved && saved !== SOUNDFONT_OUTPUT) { + await this.ensureInitialized(); + if (midiService.connectionStatus.outputConnected) return; + } + const available = midiService.getOutputDevices().some((device) => device.id === saved); + await midiService.connectOutputDevice(saved && available ? saved : SOUNDFONT_OUTPUT); + this.syncConnectionState(midiService); + } + private syncConnectionState(midiService: MidiService): void { const status = midiService.connectionStatus; this.hasInputDeviceConnected = status.inputConnected; @@ -259,7 +288,7 @@ export class GMCPClientMidi extends GMCPClientMidiBase { } private async handleNoteAsync(data: GMCPMessageClientMidiNote): Promise { - const midiService = await this.loadMidiService(); + const midiService = await this.loadMidiOutput(); const note: MidiNote = { note: data.note, velocity: data.velocity, @@ -296,26 +325,26 @@ export class GMCPClientMidi extends GMCPClientMidiBase { handleControlChange(data: GMCPMessageClientMidiControlChange): void { if (!this.enabled) return; - void this.sendRawIfOutputConnected([0xB0 | data.channel, data.controller, data.value]); + void this.sendRawToOutput([0xB0 | data.channel, data.controller, data.value]); } handleProgramChange(data: GMCPMessageClientMidiProgramChange): void { if (!this.enabled) return; - void this.sendRawIfOutputConnected([0xC0 | data.channel, data.program]); + void this.sendRawToOutput([0xC0 | data.channel, data.program]); } handleSystemMessage(data: GMCPMessageClientMidiSystemMessage): void { if (!this.enabled) return; - void this.sendRawIfOutputConnected(data.data); + void this.sendRawToOutput(data.data); } handleRawMessage(data: GMCPMessageClientMidiRawMessage): void { if (!this.enabled) return; - void this.sendRawIfOutputConnected(data.data); + void this.sendRawToOutput(data.data); } - private async sendRawIfOutputConnected(data: number[]): Promise { - const midiService = await this.loadMidiService(); + private async sendRawToOutput(data: number[]): Promise { + const midiService = await this.loadMidiOutput(); this.syncConnectionState(midiService); if (midiService.hasOutputDevice) { midiService.sendRawMessage(data); diff --git a/src/stores/preferencesStore.test.ts b/src/stores/preferencesStore.test.ts index 21747ed7..d046aaad 100644 --- a/src/stores/preferencesStore.test.ts +++ b/src/stores/preferencesStore.test.ts @@ -61,7 +61,7 @@ describe("preferencesStore", () => { expect(setItem).toHaveBeenCalledWith("preferences", expect.any(String)); const lastCall = setItem.mock.calls.at(-1); const stored = JSON.parse(lastCall?.[1] as string); - expect(stored.version).toBe(2); + expect(stored.version).toBe(3); expect(stored.data.general.localEcho).toBe(true); expect(stored.data.general.syncTimezoneToServer).toBe(false); expect(stored.data.general.syncLocationToServer).toBe(true); @@ -247,6 +247,21 @@ describe("preferencesStore", () => { expect(unknown.midi.midiJsSoundfont).toBe('MusyngKite'); }); + it("turns MIDI on by default", async () => { + const fresh = await loadWith(null); + expect(fresh.midi.enabled).toBe(true); + }); + + it("turns MIDI on once for preferences saved before version 3", async () => { + const old = await loadWith({ version: 2, data: { midi: { enabled: false } } }); + expect(old.midi.enabled).toBe(true); + }); + + it("keeps MIDI off when the user turned it off at version 3", async () => { + const chosen = await loadWith({ version: 3, data: { midi: { enabled: false } } }); + expect(chosen.midi.enabled).toBe(false); + }); + it("keeps changed keys and starts with none", async () => { const empty = await loadWith({ version: 2, data: {} }); expect(empty.keystrokes).toEqual({}); diff --git a/src/stores/preferencesStore.ts b/src/stores/preferencesStore.ts index 3338400a..f85ef404 100644 --- a/src/stores/preferencesStore.ts +++ b/src/stores/preferencesStore.ts @@ -128,7 +128,7 @@ type PrefActions = { }; const STORAGE_KEY = "preferences"; -const STORAGE_VERSION = 2; +const STORAGE_VERSION = 3; const PERSIST_DEBOUNCE_MS = 250; function getInitialPreferences(): PrefState { @@ -152,7 +152,7 @@ function getInitialPreferences(): PrefState { }, editor: { autocompleteEnabled: true, accessibilityMode: true, lineComments: false }, keyboard: { navigationKeyScheme: "jkli" }, - midi: { enabled: false }, + midi: { enabled: true }, haptics: { enabled: false, intensityCap: 1.0, autoStopTimeout: 5 }, autologging: { enabled: false, maxBytes: 100 * 1024 * 1024 }, voiceInput: { enabled: false, sendImmediately: false, language: "" }, @@ -210,6 +210,11 @@ const preferencesSchema: LocalStorageSchema = { delete (parsed as Partial).channels; } + // Version 3 turned MIDI on by default; turn it on once for older saves. + if (storedVersion < 3) { + parsed.midi = { ...parsed.midi, enabled: true }; + } + return mergePreferences(getInitialPreferences(), parsed); }, }; From ea28d5a16649c414f3437cd6ce54e6a3e7aa2436 Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Tue, 20 Oct 2026 20:14:07 -0600 Subject: [PATCH 2/3] Mount the MIDI tab only when selected MidiStatus calls ensureInitialized() on mount (MidiStatus.tsx:219-223), which runs JZZ() and navigator.requestMIDIAccess. With MIDI on by default, the tab mounted with the sidebar. mountOnSelect defers that to the first time the user opens the MIDI tab, as the haptics tab already does. Co-Authored-By: Claude --- agent-notes/feat-auto-midi.md | 5 +++-- src/components/sidebar.test.tsx | 14 ++++++++++++-- src/components/sidebar.tsx | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/agent-notes/feat-auto-midi.md b/agent-notes/feat-auto-midi.md index dc23a078..ed56692e 100644 --- a/agent-notes/feat-auto-midi.md +++ b/agent-notes/feat-auto-midi.md @@ -11,6 +11,7 @@ MOO MIDI plays without the user setting anything up: - 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). - `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 @@ -19,8 +20,8 @@ MOO MIDI plays without the user setting anything up: ## 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 is still initialized when the sidebar renders. Out of this task's scope; reported, not changed. +- `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 the PR's CI result. Open question for Q (not acted on): whether the MIDI tab should get `mountOnSelect` so the sidebar does not initialize JZZ at load. +Read PR #202's CI result. diff --git a/src/components/sidebar.test.tsx b/src/components/sidebar.test.tsx index 7b6c0424..c21960c9 100644 --- a/src/components/sidebar.test.tsx +++ b/src/components/sidebar.test.tsx @@ -12,6 +12,9 @@ vi.mock('./ServerFeaturesPanel', () => ({ default: () => Server informatio vi.mock('./AudioDiagnostics', () => ({ default: () => Diagnostics })); vi.mock('./HelpPanel', () => ({ default: () => Help contents })); vi.mock('./MapPanel', () => ({ default: () => Map contents })); +vi.mock('./MidiStatus', () => ({ default: () => MIDI contents })); + +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(); - const client = { connected: false, gmcp: { handlers: {} }, media: { diagnostics: {} }, onFileTransferOffer: () => () => {} } as unknown as MudClient; render(<>); 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(); + 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(); const tabs = screen.getAllByRole('tab'); diff --git a/src/components/sidebar.tsx b/src/components/sidebar.tsx index 1f0229cf..2c3b9471 100644 --- a/src/components/sidebar.tsx +++ b/src/components/sidebar.tsx @@ -217,6 +217,7 @@ const Sidebar = React.forwardRef( ), + mountOnSelect: true, condition: midiPreferences.enabled, }, { From 859dd6845851f01bfc95f05e456fef40627642cf Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Tue, 20 Oct 2026 20:23:31 -0600 Subject: [PATCH 3/3] Load no MIDI code before the first MOO MIDI message or MIDI tab CI's lazy-audio check (check-audio-cache.mjs:108) saw VirtualMidiService and MidiService load at startup once MIDI defaulted on. - App.tsx: drop the effect that loaded VirtualMidiService and bound it to the client's audio whenever MIDI was enabled. - Client.Midi loadMidiService(): load both services together and do that binding before resolving, so it precedes every output connect. - client.ts: reset intentional-disconnect flags through the Client.Midi package, which does nothing until MidiService has loaded. - Client.Midi sendAllNotesOff(): only silence an already loaded service; it runs on every disconnect and was loading MidiService too. Co-Authored-By: Claude --- agent-notes/feat-auto-midi.md | 6 +++++ src/App.test.tsx | 14 ++++++++++ src/App.tsx | 37 -------------------------- src/client.test.ts | 17 ++++++++++++ src/client.ts | 16 ++---------- src/gmcp/Client/Midi.test.ts | 49 ++++++++++++++++++++++++++++++++--- src/gmcp/Client/Midi.ts | 21 ++++++++++++--- 7 files changed, 102 insertions(+), 58 deletions(-) diff --git a/agent-notes/feat-auto-midi.md b/agent-notes/feat-auto-midi.md index ed56692e..d8b281d7 100644 --- a/agent-notes/feat-auto-midi.md +++ b/agent-notes/feat-auto-midi.md @@ -12,10 +12,16 @@ MOO MIDI plays without the user setting anything up: - 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 diff --git a/src/App.test.tsx b/src/App.test.tsx index 64a83b30..fe073903 100644 --- a/src/App.test.tsx +++ b/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(); + 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(); diff --git a/src/App.tsx b/src/App.tsx index d238c3e8..1b289571 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -162,7 +162,6 @@ function App() { const clientInitialized = useRef(false); const hapticsRuntimeRef = useRef(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); diff --git a/src/client.test.ts b/src/client.test.ts index b99b9897..5a767ca1 100644 --- a/src/client.test.ts +++ b/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(); diff --git a/src/client.ts b/src/client.ts index b9db2e28..0c93b859 100644 --- a/src/client.ts +++ b/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(); } diff --git a/src/gmcp/Client/Midi.test.ts b/src/gmcp/Client/Midi.test.ts index 024ef514..001b14e2 100644 --- a/src/gmcp/Client/Midi.test.ts +++ b/src/gmcp/Client/Midi.test.ts @@ -17,11 +17,29 @@ const { midiService, output } = vi.hoisted(() => { sendNote: vi.fn(), scheduleNoteOff: vi.fn(() => true), sendRawMessage: vi.fn(), + sendAllNotesOff: vi.fn(), + resetIntentionalDisconnectFlags: vi.fn(), + }, + }; +}); + +const { virtualMidiService, audioOutput, media } = vi.hoisted(() => { + const audioOutput = { context: {}, destination: {} }; + return { + audioOutput, + media: { getMidiAudioOutput: () => audioOutput }, + virtualMidiService: { + setPlaybackSource: vi.fn(() => () => {}), + setAudioOutput: vi.fn(async () => {}), + initialize: vi.fn(async () => true), }, }; }); vi.mock('../../MidiService', () => ({ midiService })); +vi.mock('../../VirtualMidiService', () => ({ virtualMidiService })); + +const makeClient = () => ({ gmcp: { send: vi.fn() }, media }) as never; import { usePreferences } from '../../stores/preferencesStore'; import { GMCPClientMidi } from './Midi'; @@ -33,7 +51,7 @@ describe('GMCPClientMidi ControlChange', () => { beforeEach(() => { vi.clearAllMocks(); usePreferences.getState().setMidi({ ...previous, enabled: true }); - midi = new GMCPClientMidi({ gmcp: { send: vi.fn() } } as never); + midi = new GMCPClientMidi(makeClient()); }); afterEach(() => { @@ -66,7 +84,7 @@ describe('GMCPClientMidi Song', () => { }); it('plays each note of one Song message at its offset', async () => { - const midi = new GMCPClientMidi({ gmcp: { send: vi.fn() } } as never); + const midi = new GMCPClientMidi(makeClient()); const played: number[] = []; vi.spyOn(midi as never, 'handleNoteAsync').mockImplementation(((note: { note: number }) => { played.push(note.note); @@ -95,7 +113,7 @@ describe('GMCPClientMidi automatic output', () => { vi.clearAllMocks(); Object.assign(output, { connected: false, intentional: false, initialized: false }); usePreferences.getState().setMidi({ ...previous, enabled: true, lastOutputDeviceId: undefined }); - midi = new GMCPClientMidi({ gmcp: { send: vi.fn() } } as never); + midi = new GMCPClientMidi(makeClient()); }); afterEach(() => { @@ -148,6 +166,31 @@ describe('GMCPClientMidi automatic output', () => { expect(midiService.connectOutputDevice).toHaveBeenCalledWith('soundfont-synth'); }); + it('renders the synth into the client audio graph before the first output connects', async () => { + note(60); + await vi.waitFor(() => expect(midiService.connectOutputDevice).toHaveBeenCalled()); + expect(virtualMidiService.setPlaybackSource).toHaveBeenCalledWith(media); + expect(virtualMidiService.setAudioOutput).toHaveBeenCalledWith(audioOutput); + expect(virtualMidiService.setAudioOutput.mock.invocationCallOrder[0]) + .toBeLessThan(midiService.connectOutputDevice.mock.invocationCallOrder[0]); + }); + + it('loads no MIDI code when reset or reconnected before any MIDI message', async () => { + midi.reset(); + midi.resetIntentionalDisconnects(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(virtualMidiService.setPlaybackSource).not.toHaveBeenCalled(); + expect(midiService.sendAllNotesOff).not.toHaveBeenCalled(); + expect(midiService.resetIntentionalDisconnectFlags).not.toHaveBeenCalled(); + }); + + it('resets the intentional disconnect flags once MIDI is loaded', async () => { + note(60); + await vi.waitFor(() => expect(midiService.sendNote).toHaveBeenCalled()); + midi.resetIntentionalDisconnects(); + await vi.waitFor(() => expect(midiService.resetIntentionalDisconnectFlags).toHaveBeenCalledOnce()); + }); + it('connects an output for a MOO ProgramChange before sending it', async () => { midi.receiveRegisteredMessage('ProgramChange', { channel: 0, program: 5 }); await vi.waitFor(() => expect(midiService.sendRawMessage).toHaveBeenCalledWith([0xc0, 5])); diff --git a/src/gmcp/Client/Midi.ts b/src/gmcp/Client/Midi.ts index 69b21223..3d5db497 100644 --- a/src/gmcp/Client/Midi.ts +++ b/src/gmcp/Client/Midi.ts @@ -115,12 +115,24 @@ export class GMCPClientMidi extends GMCPClientMidiBase { } private loadMidiService(): Promise { - if (!this.midiServicePromise) { - this.midiServicePromise = import("../../MidiService").then(({ midiService }) => midiService); - } + this.midiServicePromise ??= Promise.all([import("../../MidiService"), import("../../VirtualMidiService")]) + .then(async ([{ midiService }, { virtualMidiService }]) => { + // Render the synth (and soundfonts, which use its output) into the + // client's master gain before any output connects, so it shares the + // client's clock, volume, mute, and autoplay unlock. + virtualMidiService.setPlaybackSource(this.client.media); + await virtualMidiService.setAudioOutput(this.client.media.getMidiAudioOutput()); + void virtualMidiService.initialize(); + return midiService; + }); return this.midiServicePromise; } + /** On server reconnect. Before MIDI loads, the flags are already false. */ + resetIntentionalDisconnects(): void { + void this.midiServicePromise?.then((midiService) => midiService.resetIntentionalDisconnectFlags()); + } + /** * The service, with an output connected for a MOO message unless the user * disconnected it on purpose. Concurrent callers share one connect. @@ -402,8 +414,9 @@ export class GMCPClientMidi extends GMCPClientMidiBase { } sendAllNotesOff(): void { + // Nothing can be sounding before MIDI has loaded, so don't load it here. if (this.enabled) { - void this.loadMidiService().then((midiService) => { + void this.midiServicePromise?.then((midiService) => { midiService.sendAllNotesOff(); }); }