fix(audio): input-callback double registration, stale-format race, effects schema alignment (#86)

* fix(audio): prefer same-backend duplex routing

* fix(audio): centre mono input; limit duplex to same-endpoint devices

Two issues found while testing the USB-guitar-cable path on Windows:

1. Centre a mono input. SourceChain::processBlock fell into the
   pass-through branch for a 1-channel input, filling only
   min(inputChannels, outputChannels) = 1 output channel and zeroing the
   rest, so a mono USB guitar cable played out of the left speaker only.
   A single-channel input is now broadcast across every output channel.

2. Only attempt the combined (duplex) device when input and output are
   the SAME physical endpoint. Two different endpoints of the same
   backend (USB cable in + separate speakers out) are independent
   hardware clocks; routing them through one duplex device was unstable
   across the app lifecycle (no audio until an explicit Apply, then
   distortion / dropouts / silent-in-song on navigation). Different
   endpoints now use the split path, whose ring bridges the two clocks.
   Same-endpoint duplex (one interface for in and out) keeps the
   low-latency win. Low latency for the two-device case is a follow-up
   that needs the device-lifecycle work (startup restore + reconfigure
   on navigation).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu

* fix(audio): probe same-endpoint duplex the same way apply routes it

Startup auto-apply (renderer init) fail-closes on probeDeviceOptionsDual's
`compatible` verdict, but the probe still measured a COMBINED duplex device
for any same-backend pair while setAudioDevices now opens split for
different endpoints. That mismatch made the startup probe describe a config
that isn't the one applied — surfacing as "no audio until I press Apply" for
a USB cable + separate speakers. Gate the probe's duplex path on the same
sameEndpointIntent (same type AND same device) the apply path uses, so a
two-device pair is probed via the split path it will actually run on.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu

* fix(audio): never feed chain processors blocks larger than prepared size

WASAPI shared mode can deliver oversized blocks right after a device
start. The NAM core pre-allocates its conv ring/output buffers to the
Reset() maxBufferSize and only asserts (release no-op) on larger blocks;
one oversized block corrupts the conv ring state and garbles all
subsequent audio until the next Reset() — the 'first start heavily
distorted until tone reset / engine restart' bug.

- NAMProcessor::processBlock: process in slices of at most the prepared
  block size.
- SignalChain::process: slice oversized device blocks into prepared-size
  chunks before any slot (VST/NAM/IR) sees them.

See docs/audio-distortion-first-start-investigation.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(audio): close stale-format race when processors are added mid-reconfigure

addProcessor/replaceProcessor prepare the incoming processor off the
audio lock on N-API worker threads. A concurrent device reconfigure's
SignalChain::prepare() can't see that processor (not slotted yet), so a
slot could go live prepared at a stale sample rate / block size and stay
wrong until the next device restart — heard as pitch-shifted/garbled
monitoring when a chain loads while the device is being (re)opened
(widest window: WASAPI exclusive mode's slower open).

Re-check the chain's current format under the lock at insert/swap time
and re-prepare if it moved; log the transition to stderr so tester logs
show when the race fired. prepare() now publishes the format under the
lock so the check can't tear.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(effects): align executor plan schema with rebranded capability layer

The rebrand left a three-way schema split: rig_builder sent the old
'slopsmith.audio_effects.chain_plan.v1', the renderer capability layer
validated against the new 'feedBack.…' id (rejecting every plan), and
this executor still expected the old one. Result: every song chain load
fell back to legacy clearChain+loadPreset — a full multi-VST rebuild per
currentSong poll cycle, heard as continuous distortion during playback
(tester logs: 4-6 rebuilds/session, slot IDs into the 90s).

Executor now uses the rebranded id and accepts the legacy one as an
alias (matching the capability layer's new alias), so neither side of
the handoff can break on old plugin bundles.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(audio): guard input callback against double registration

Tester main-process log showed two consecutive 'startAudio: duplex=0'
lines: a transient audioDeviceStopped() (WASAPI exclusive opens fire one
mid-start) cleared audioRunning while the input callback stayed
attached, so the second startAudio() re-added it. JUCE then dispatched
the input callback twice per block: DSP ran twice and each block was
pushed into the split ring twice — every sample played twice (half
speed, one octave down, garbled). stopAudio()'s single
removeAudioCallback left the duplicate registration alive, wedging the
engine (restart no longer helped) and keeping the exclusive-mode device
open even after app close.

Mirror the existing outputCallbackRegistered guard for the input side.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(audio): diagnostic instrumentation for tester repro builds

Main-process stderr logging on every open lead, RT paths rate-limited
(first-25 per anomaly + ~5s heartbeats per callback clock):

- primary callback re-entrancy (duplicate registration detector)
- oversized blocks on primary/output callbacks, SignalChain slicing,
  NAM chunking (pre-fix corruption trigger visibility)
- ring fill + under/overflow counters (split-mode pacing)
- device lifecycle: aboutToStart/stopped on both managers with sr/bs and
  callback-registration flags; startAudio guard-skip; stopAudio state
- SourceChain.prepare format trace (stale-rate lead)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(audio): drop diagnostic heartbeats, keep anomaly detectors

The 5s ring/format heartbeats served the distortion hunt and are noise
now. Keep the cheap anomaly-only diagnostics (callback re-entrancy,
oversized-block detectors, chain slicing/chunking, stale-format
re-prepare, device lifecycle) — they log only on misbehavior and stay
relevant for the exclusive-mode playback work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: keep investigation notes out of the PR (local working notes)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(audio): gate lifecycle [diag] logs behind SLOPSMITH_SANDBOX_DEBUG

Review follow-up (#86): the six lifecycle diagnostics (stopAudio,
audioDeviceAboutToStart/Stopped, audioOutputAboutToStart/Stopped,
SourceChain::prepare) printed unconditionally while the PR body claimed
they were verbose-gated. Gate them behind the existing
slopsmith_vst_trace::isEnabled() runtime flag (SLOPSMITH_SANDBOX_DEBUG —
already flipped by the app's debug-logging switch, so tester debug runs
still capture them). The RT-path anomaly detectors (primary re-entry,
oversized-block) keep their firstN/anomaly bounds unchanged, as reviewed.

VSTTrace.h now defines NOMINMAX/WIN32_LEAN_AND_MEAN before windows.h so
including it from engine TUs doesn't clobber std::min.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

src/audio/AudioEngine.cpp

@@ -1,11 +1,24 @@
 #include "AudioEngine.h"
 #include "AudioSanitize.h"
+#include "VSTTrace.h"
 
 #include <algorithm>
 #include <chrono>
 #include <cmath>
 #include <thread>
 
+// ── Diagnostic instrumentation (tester builds) ────────────────────────────────
+// Every counter is a file-static atomic so the RT paths stay allocation-free.
+// First-N logging for anomalies (so a storm can't flood the log) + a periodic
+// stats heartbeat every ~5 s of processed audio on each callback clock.
+namespace audiodiag {
+static std::atomic<uint32_t> primaryReentry{0};      // concurrent primary callback bodies seen
+static std::atomic<uint32_t> oversizedBlocks{0};     // numSamples > inputBlockSize on primary
+static std::atomic<uint32_t> outputOversized{0};     // numSamples > scratch on output callback
+static constexpr uint32_t kFirstN = 25;              // per-anomaly log budget
+inline bool firstN(std::atomic<uint32_t>& c) { return c.fetch_add(1, std::memory_order_relaxed) < kFirstN; }
+}
+
 // Hard ceiling on backing playback speed. This drives input buffer sizing and runtime clamp.
 static constexpr double kMaxBackingSpeed = 4.0;
 // Transparent full-speed path — skip the stretcher when rate is effectively 1×.
@@ -1189,13 +1202,28 @@ void AudioEngine::startAudio()
 
     // Input first so it has time to prefill the ring before the output
     // callback pulls — otherwise split mode underflows once at start.
-    inputDeviceManager.addAudioCallback(this);
+    //
+    // Same double-registration guard as the output side below: audioRunning is
+    // cleared by audioDeviceStopped() on a transient stop (WASAPI exclusive
+    // opens routinely fire one mid-start) while this callback stays attached,
+    // so an unguarded re-add here registered the INPUT callback twice — every
+    // block then ran the DSP twice and pushed into the split ring twice,
+    // playing each sample twice (half speed, one octave down, garbled), and
+    // stopAudio()'s single removeAudioCallback left a live registration behind
+    // that kept the device (exclusive!) open after "stop" and after app close.
+    if (!inputCallbackRegistered)
+    {
+        inputDeviceManager.addAudioCallback(this);
+        inputCallbackRegistered = true;
+    }
+    else
+    {
+        // DIAG: this is exactly the path that used to double-register the
+        // input callback (half-speed garble + device held open). Now skipped —
+        // log it so tester logs prove the guard fired.
+        fprintf(stderr, "[diag] startAudio: input callback already registered — skipping re-add (guard active)\n");
+    }
 
-    // Guard against double-registration: audioRunning can be cleared by
-    // audioDeviceStopped() on a transient input unplug while the output
-    // callback intentionally stays registered (JUCE auto-restart relies on
-    // that). A later startAudio() would then add the same callback again
-    // and JUCE would dispatch it twice per block.
     if (!duplexMode.load(std::memory_order_relaxed) && !outputCallbackRegistered)
     {
         outputDeviceManager.addAudioCallback(&outputCallback);
@@ -1222,6 +1250,10 @@ void AudioEngine::startAudio()
 
 void AudioEngine::stopAudio()
 {
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] stopAudio: audioRunning=%d inputCbReg=%d outputCbReg=%d\n",
+            (int) audioRunning.load(std::memory_order_relaxed),
+            (int) inputCallbackRegistered, (int) outputCallbackRegistered);
     // Always attempt to detach both callbacks — removeAudioCallback is
     // idempotent. We don't gate on audioRunning here because that flag can
     // be cleared externally by audioDeviceStopped() (input device
@@ -1232,6 +1264,7 @@ void AudioEngine::stopAudio()
     outputDeviceManager.removeAudioCallback(&outputCallback);
     outputCallbackRegistered = false;
     inputDeviceManager.removeAudioCallback(this);
+    inputCallbackRegistered = false;
     // Extra input devices are opened independently of startAudio(); close them here
     // too so a stopped engine never leaves a second interface capturing, feeding
     // detectors, and holding the hardware open in the background. KEEP their
@@ -1769,6 +1802,11 @@ void AudioEngine::audioDeviceAboutToStart(juce::AudioIODevice* device)
     audioRunning.store(true, std::memory_order_relaxed);
     const double sr = device->getCurrentSampleRate();
     const int bs = device->getCurrentBufferSizeSamples();
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] audioDeviceAboutToStart: dev='%s' sr=%.0f bs=%d duplex=%d inputCbReg=%d outputCbReg=%d\n",
+            device->getName().toRawUTF8(), sr, bs,
+            (int) duplexMode.load(std::memory_order_relaxed),
+            (int) inputCallbackRegistered, (int) outputCallbackRegistered);
     currentSampleRate.store(sr, std::memory_order_relaxed);
     inputBlockSize.store(bs, std::memory_order_relaxed);
     if (duplexMode.load(std::memory_order_relaxed))
@@ -1841,6 +1879,8 @@ void AudioEngine::audioDeviceStopped()
     // sources: an EXTRA-device source is processed by that device's own callback,
     // which may still be running on its own thread — releasing it here would race.
     // Extra sources are released by extraInputStopped()/unbindInputDevice().
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] audioDeviceStopped (audioRunning cleared; callbacks stay attached for JUCE auto-restart)\n");
     audioRunning.store(false, std::memory_order_relaxed);
     {
         std::lock_guard<std::mutex> lock(sourcesMutex);
@@ -1873,6 +1913,9 @@ void AudioEngine::audioDeviceStopped()
 void AudioEngine::audioOutputAboutToStart(juce::AudioIODevice* device)
 {
     const int bs = device->getCurrentBufferSizeSamples();
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] audioOutputAboutToStart: dev='%s' sr=%.0f bs=%d\n",
+            device->getName().toRawUTF8(), device->getCurrentSampleRate(), bs);
     outputBlockSize.store(bs, std::memory_order_relaxed);
 
     if ((int) outputPullScratchL.size() < bs) outputPullScratchL.assign((size_t) bs, 0.0f);
@@ -1937,6 +1980,8 @@ void AudioEngine::audioOutputAboutToStart(juce::AudioIODevice* device)
 
 void AudioEngine::audioOutputStopped()
 {
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] audioOutputStopped\n");
     // No-op by design. The consumer's catch-up branch in audioOutputCallback
     // handles both (w - r) > cap (producer lapped during the stop) and
     // w < r (a future reset race) on the next output start, so we don't
@@ -2212,7 +2257,22 @@ void AudioEngine::audioDeviceIOCallbackWithContext(
     // Publish that the callback body is executing so removeSource() and deferred-
     // release reclamation know when no source is being processed (the body is
     // quiescent) and a removed source can be safely released. Index 0 = primary.
-    callbacksInFlight[0].fetch_add(1, std::memory_order_acq_rel);
+    const int inFlightBefore = callbacksInFlight[0].fetch_add(1, std::memory_order_acq_rel);
+
+    // DIAG: two primary callback bodies at once = the input callback is
+    // registered twice on the device manager (the half-speed/garble bug) or a
+    // second device is dispatching into the primary path. Should never fire.
+    if (inFlightBefore > 0 && audiodiag::firstN(audiodiag::primaryReentry))
+        fprintf(stderr, "[diag] PRIMARY CALLBACK RE-ENTERED (inFlight=%d, numSamples=%d) — duplicate registration?\n",
+                inFlightBefore + 1, numSamples);
+
+    // DIAG: block larger than the size everything was prepared with.
+    {
+        const int preparedBs = inputBlockSize.load(std::memory_order_relaxed);
+        if (preparedBs > 0 && numSamples > preparedBs && audiodiag::firstN(audiodiag::oversizedBlocks))
+            fprintf(stderr, "[diag] primary callback OVERSIZED block: numSamples=%d > prepared=%d\n",
+                    numSamples, preparedBs);
+    }
 
     const bool duplex = duplexMode.load(std::memory_order_relaxed);
 
@@ -2817,6 +2877,10 @@ void AudioEngine::audioOutputCallback(const float* const* /*inputData*/,
     constexpr uint64_t kMask = (uint64_t) kOutputRingFrames - 1;
     constexpr uint64_t kCap  = (uint64_t) kOutputRingFrames;
 
+    if ((int) outputPullScratchL.size() < numSamples && audiodiag::firstN(audiodiag::outputOversized))
+        fprintf(stderr, "[diag] output callback OVERSIZED block: numSamples=%d > scratch=%d\n",
+                numSamples, (int) outputPullScratchL.size());
+
     // Clamp the working size to the scratch capacity pre-allocated in
     // audioOutputAboutToStart() so the .assign() calls below never realloc
     // on the RT thread when a transient oversized block arrives (mirrors

src/audio/AudioEngine.h

@@ -566,6 +566,12 @@ private:
     std::vector<float> outputPullScratchR;
     juce::AudioBuffer<float> outputBackingBuffer;
     bool outputCallbackRegistered = false;
+    // Same guard for the primary INPUT callback (`this`): audioRunning can be
+    // cleared by a transient audioDeviceStopped() while the callback stays
+    // attached, and an unguarded startAudio() re-add would dispatch it twice
+    // per block (double DSP + double ring push → half-speed garbled audio) and
+    // leave a live registration behind after stopAudio()'s single remove.
+    bool inputCallbackRegistered = false;
 
     // ── Phase 2: additional input devices ────────────────────────────────────
     // Each ADDITIONAL physical input device (a 2nd/3rd USB interface, e.g. two

src/audio/NAMProcessor.cpp

@@ -104,6 +104,15 @@ void NAMProcessor::processBlock(juce::AudioBuffer<float>& buffer, juce::MidiBuff
     // and leaves the conv ring misaligned, garbling ALL subsequent audio until
     // the next Reset().
     const int maxChunk = currentBlockSize > 0 ? currentBlockSize : numSamples;
+    if (numSamples > maxChunk)
+    {
+        // DIAG (first 25): block bigger than the size the NAM core was Reset()
+        // with — the exact pre-fix corruption trigger, now chunked instead.
+        static std::atomic<uint32_t> chunkLogs{0};
+        if (chunkLogs.fetch_add(1, std::memory_order_relaxed) < 25)
+            fprintf(stderr, "[diag] NAMProcessor chunking oversized block: %d > prepared %d\n",
+                    numSamples, maxChunk);
+    }
     for (int offset = 0; offset < numSamples; offset += maxChunk)
     {
         const int chunk = juce::jmin(maxChunk, numSamples - offset);

src/audio/SignalChain.cpp

@@ -275,6 +275,16 @@ void SignalChain::process(juce::AudioBuffer<float>& buffer, juce::MidiBuffer& mi
         return;
     }
 
+    // DIAG (first 25): an oversized device block reached the chain and is
+    // being sliced — records how often the pre-fix corruption path would
+    // have fired and with what sizes.
+    {
+        static std::atomic<uint32_t> sliceLogs{0};
+        if (sliceLogs.fetch_add(1, std::memory_order_relaxed) < 25)
+            fprintf(stderr, "[diag] SignalChain slicing oversized block: %d > prepared %d\n",
+                    totalSamples, maxChunk);
+    }
+
     constexpr int kMaxSliceChannels = 8;
     const int numChannels = buffer.getNumChannels();
     if (numChannels > kMaxSliceChannels)

src/audio/SourceChain.cpp

@@ -1,4 +1,5 @@
 #include "SourceChain.h"
+#include "VSTTrace.h"
 #include "AudioSanitize.h"
 
 #include <cmath>
@@ -13,6 +14,8 @@
 
 void SourceChain::prepare(double sr, int blockSize)
 {
+    if (slopsmith_vst_trace::isEnabled())
+        fprintf(stderr, "[diag] SourceChain[%d].prepare: sr=%.0f bs=%d\n", sourceId, sr, blockSize);
     // Reset the input rings so a stop→start cycle delivers a clean zero-padded
     // cold-start frame instead of mixing in stale samples from the previous run.
     // The audio thread isn't running yet (device-start hook), so relaxed is fine.

src/audio/VSTTrace.h

@@ -21,6 +21,16 @@
 #include <mutex>
 
 #if defined(_WIN32)
+  // Keep windows.h from defining min/max macros that clobber std::min /
+  // std::max in any TU that includes this header (NOMINMAX only helps if
+  // nothing else included windows.h first — WIN32_LEAN_AND_MEAN keeps the
+  // surface small either way).
+  #ifndef NOMINMAX
+    #define NOMINMAX
+  #endif
+  #ifndef WIN32_LEAN_AND_MEAN
+    #define WIN32_LEAN_AND_MEAN
+  #endif
   #include <windows.h>
 #else
   #include <unistd.h>

src/main/audio-effects-executor.ts

@@ -1,7 +1,12 @@
 import * as fs from 'fs';
 import * as path from 'path';
 
-const PLAN_SCHEMA = 'slopsmith.audio_effects.chain_plan.v1';
+const PLAN_SCHEMA = 'feedBack.audio_effects.chain_plan.v1';
+// Pre-rebrand identifier. The renderer capability layer rewrites validated
+// plans to PLAN_SCHEMA, but older plugin bundles and direct callers may still
+// send the slopsmith-era id — accept it as an alias so the rebrand can't
+// silently break the executor handoff again.
+const LEGACY_PLAN_SCHEMA = 'slopsmith.audio_effects.chain_plan.v1';
 const DEFAULT_ROUTE_KEY = 'desktop-main';
 const MAX_STAGES = 24;
 const MAX_SEGMENTS = 80;
@@ -265,7 +270,7 @@ function validatePlan(request: unknown): { ok: true; plan: ValidPlan; presetJson
     }
 
     const schema = String(planInput.schema ?? '').trim();
-    if (schema !== PLAN_SCHEMA) errors.push('Unsupported audio-effects chain plan schema');
+    if (schema !== PLAN_SCHEMA && schema !== LEGACY_PLAN_SCHEMA) errors.push('Unsupported audio-effects chain plan schema');
 
     const routeKey = safeId(planInput.routeKey ?? planInput.route ?? DEFAULT_ROUTE_KEY, DEFAULT_ROUTE_KEY);
     const providerId = safeId(planInput.providerId, 'provider');