Commit Graph

165 Commits

Author SHA1 Message Date
Matthew Harris Glover
2a4396b7b7
Linux AppImage self-update on the nightly channel (#119)
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
* feat(update): Linux AppImage self-update on the nightly channel

Adds a self-update engine for the Linux AppImage build. There's no
Velopack pipeline for Linux (Windows/macOS use it, Linux doesn't), so
this is a small, purpose-built GitHub-releases checker instead:

- On the nightly channel, compares the commit baked into the running
  build (dist/main/build-info.json, written at build time) against the
  published nightly's target_commitish. A mismatch means the running
  build is behind, so it's offered as an update — this sidesteps the
  fact that the AppImage's filename and app.getVersion() never change
  between nightly builds, so semver comparison can't detect a new one.
- The check returns immediately and the ~1.5GB download runs in the
  background with live progress (a new update:progress IPC event), so
  the UI never blocks or freezes waiting on it.
- The download streams straight to disk (no buffering the whole file in
  memory) and is swapped in with an atomic rename next to the running
  AppImage. The stale-generation check (a channel switch or new check
  invalidating an in-flight download) runs before that swap, and a
  failed or superseded download always cleans up its temp file.
- Applying the update spawns the (already-swapped-in) AppImage as a
  detached process and waits for a real 'spawn' confirmation before
  quitting this one, rather than assuming success — child_process.spawn
  can fail asynchronously, and quitting on an unconfirmed relaunch could
  leave the user with nothing running.
- The pure idle/staged/download decision is split into
  linux-update-decision.ts with a small truth-table test, and every
  main-process decision point (and the equivalent renderer-side
  actions, in the companion feedBack PR) is traced through a new
  update:diag IPC event that lands in the app's existing "Export
  Diagnostics" console-capture bundle — this is how the handful of real
  bugs below were actually root-caused, from real device captures
  rather than guesswork.

Also removes a forgotten, dead second implementation of the
update-channel UI (src/renderer/screen.js's
setupUpdateChannelControls() + its markup in settings.html), left over
from before this work discovered the real, visible System-tab update
UI lives in the feedBack repo. It was still wired up in the
audio_engine plugin's own settings panel and silently called
setChannel() with a stale channel value every time that panel
rendered — invisibly corrupting the real UI's state. This was the
actual root cause of several rounds of flaky, hard-to-reproduce
on-device behavior (a stuck "unsupported" warning, downloads starting
without an explicit check, etc.) chased down via the diagnostic
tracing above; once found, no other logic needed to change.

Dev tooling only, not used by CI: forces --platform linux/amd64 in the
local Docker build wrapper (the Linux target is x86_64-only end to
end — needed on Apple Silicon, where Rosetta chokes on a foreign-arch
binary inside an otherwise-native container) and adds a SLOPSMITH_REPO
override so a contributor without push access to the core repo can
bundle a fork branch for a local test build.

Verified end-to-end on a Steam Deck across many build/deploy rounds:
fresh launch, channel selection, check, background download with live
progress, atomic swap, and relaunch onto the new build — confirmed via
a real Export Diagnostics capture showing a clean, fully-accounted-for
trace with zero orphaned state transitions.

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

* fix(update): fail safe when nightly release isn't pinned to a commit SHA

GitHub sets a release's target_commitish to whatever it was published
against — a 40-char SHA only if pinned, otherwise a branch name like
"main". The Linux update decision compares it SHA-vs-SHA, so a branch
name would never match the baked SHA and would re-download the ~1.5GB
AppImage on every check forever, never reaching idle. Add isCommitSha()
(pure, unit-tested) and have checkNowLinux() surface an error instead of
entering that loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>

---------

Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
2026-07-19 12:15:55 +02:00
K. O. A.
242cbf23bc
Merge pull request #121 from got-feedBack/chore/bundle-stem-splitter
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Sandbox IPC tests / sandbox-tests (macos-14) (push) Waiting to run
Sandbox IPC tests / sandbox-tests (ubuntu-22.04) (push) Waiting to run
Sandbox IPC tests / sandbox-e2e (macos-14) (push) Waiting to run
Sandbox IPC tests / sandbox-e2e (ubuntu-22.04) (push) Waiting to run
Ship CI / CI (push) Waiting to run
build: bundle feedBack-plugin-stem-splitter
2026-07-18 21:27:24 -04:00
topkoa
7212ce70a8 build: bundle feedBack-plugin-stem-splitter
Adds the Stem Splitter plugin to clone_slopsmith()'s bundled-plugin
list so packaged builds ship it. The repo uses the feedBack-plugin-*
casing (capital B), so the lowercase prefix strip can't derive the
dirname — explicit :stem_splitter, matching the plugin id, same as
rig_builder.

Signed-off-by: topkoa <topkoa@gmail.com>
2026-07-18 21:26:30 -04:00
OmikronApex
49f6963100
Merge pull request #120 from got-feedBack/fix/lifecycle-executor-p0
fix(audio): P0 lifecycle executor — serialized ops, generation guard, module pinning
2026-07-19 01:21:55 +02:00
Jorge Fritis
f82e9aad3e
Merge pull request #111 from got-feedBack/fix/steamdeck-external-urls
Open external links via Steam overlay browser on Steam Deck
2026-07-18 17:55:46 -04:00
Jorge Fritis
7a75328079
Merge pull request #110 from got-feedBack/reverse-vst-param-sync
Expose sandboxed VST parameter values via kListParameters
2026-07-18 17:55:22 -04:00
OmikronApex
4ac1b60766 fix(audio): header-only module pin; exception-safe ops; CI link fix
CI: slopsmith-vst-host (sandbox-e2e's own CMake project) and macOS's
slopsmith-vst-scan compile VSTHost.cpp without LifecycleExecutor.cpp and
failed to link pinPluginModuleForever. Move the pin to a header-only
PluginModulePin.h so every target that compiles VSTHost.cpp gets it with
no extra source-list entry; drop the now-unneeded LifecycleExecutor.cpp
from VST_HOST_SOURCES.

CodeRabbit review:
- Pin by full module path, not base name: enumerate loaded modules
  (PSAPI_VERSION=2 K32 exports, no psapi.lib) and pin every one whose
  path starts with the plugin identifier — two plugins sharing a DLL base
  name can no longer pin the wrong module, and the bundle-dir identifier
  now matches the inner Contents/<arch>/ file it actually loaded.
- Ops are exception-safe: a throwing op logs and still signals its
  waiter instead of leaving an unbounded caller blocked forever.
- Documented why the message-thread inline path is intentional (nested
  submissions are part of the outer op; dispatch-and-wait on the message
  thread would self-deadlock; FIFO is the contract between off-thread
  submitters).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-18 00:01:52 +02:00
OmikronApex
9e5ccdbd90
Merge pull request #117 from vo90/agent/library-path-live-refresh
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
fix(library): apply saved path without restart
2026-07-17 23:36:26 +02:00
OmikronApex
503f665865 build(windows): dereference plugin-tree symlinks when bundling
rig_builder now ships vst/src/{pedals,racks}/_shared -> ../_shared; Git
Bash cp -R fails to recreate symlinks without SeCreateSymbolicLink. The
bundle wants real files anyway — copy with -L.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 23:20:46 +02:00
OmikronApex
1359c7c24d fix(audio): P0 lifecycle executor — serialized ops, generation guard, module pinning
Three identical field crash dumps (2026-07-17, CFG fail-fast 0xc0000409
param 0xa = GUARD_ICALL_CHECK_FAILURE on the JUCE message thread, plugin
WndProc frame on the stack) traced to the dispatch timeout desync: every
lifecycle call site could give up after 15 s while its op was still queued,
leaving two owners mutating engine/plugin lifecycle state with no ordering
authority.

- New LifecycleExecutor (guide §12 P0): named lifecycle ops, strictly FIFO
  on the message thread, engine-generation stamped at submit and no-oped
  when stale. The wait is unbounded with a 15 s watchdog that reports and
  keeps waiting; false now always means "verifiably did not run" — the
  "false but it may still run later" state is gone.
- initialize/doShutdown bump the generation and run as executor ops;
  shutdown is the one bounded caller (60 s, then leak the pump — beats
  hanging process exit behind a wedged driver call).
- runDeviceLifecycleOp and closeAllPluginEditorWindows route through the
  executor; the editor-teardown 15 s give-up (delayed #56 UAF) is removed.
- pinPluginModuleForever: plugin modules are pinned on load so JUCE's
  refcount can never unload them mid-session — a queued window/timer
  message into an unloaded module is the CFG fail-fast in the dumps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 23:20:46 +02:00
OmikronApex
6eeec8e2a6 fix(library): keep the resolved env fallback on failure statuses
The backend has no built-in library default — _get_dlc_dir() returns None
when DLC_DIR is unset and config.json is unusable — so deleting DLC_DIR on
invalid-config/write-failed stranded the user with an empty library and
only a console warn (corrupt config.json, or a saved dlc_dir pointing at
an unplugged drive, previously still produced a working library via the
env fallback).

invalid-config and write-failed now export the resolved fallback as
DLC_DIR, restoring pre-#117 behaviour exactly and only where config.json
cannot own the path. Config-owned dynamic refresh is unchanged for the
configured/bootstrapped paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 23:16:55 +02:00
OmikronApex
62dbf2c50e
Merge pull request #114 from vo90/agent/asio-close-before-reconfigure
fix(audio): close Windows ASIO before reconfiguration
2026-07-17 23:03:55 +02:00
OmikronApex
16686389fd fix(audio): gate strict buffer-size verification to ASIO; review cleanups
Post-open verification previously failed closed on any buffer-size delta on
every backend. The wedge this guards against (a driver accepting a request
without changing its buffer, then blocking the next in-place request) is
ASIO behaviour; ALSA rounds requests to period constraints and CoreAudio
can clamp, and both previously worked by storing the driver-adjusted
actuals. Keep exact buffer equality for ASIO only; other backends log and
accept the adjusted size. Rate and channel-mask verification stay strict
everywhere.

Also from review: print the real `options.compatible` in the live-probe log
instead of a hard-coded 1, and document that the live-endpoint reuse in
probeDual is safe only under runDeviceLifecycleOp's message-thread
serialisation (PR #113).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 22:20:03 +02:00
Viktor Olausson
aa388b87d5 fix(library): validate saved library paths 2026-07-17 18:42:43 +02:00
Viktor Olausson
2dfa414e64 fix(library): validate DLC_DIR overrides 2026-07-17 16:24:39 +02:00
Viktor Olausson
9d40432700 fix(library): apply saved path without restart 2026-07-17 16:04:28 +02:00
Viktor Olausson
a558873c43 fix(audio): reuse live ASIO device capabilities 2026-07-17 15:18:33 +02:00
OmikronApex
cd93159751 fix(audio): clear duplexMode on duplex setup failure; review cleanups
Address PR #114 review:
- failClosed now stores duplexMode=false so a failed reconfigure cannot
  leave the engine reporting duplex-active on a closed device.
- Drop the dead BigInteger initializers in the verify block.
- Move the applyDuplex locate-guard ahead of the source slice in the
  lifecycle test so marker drift fails with a clear message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 14:03:56 +02:00
Viktor Olausson
78a3979a3a test(audio): keep rate-match checks active in release 2026-07-17 13:22:48 +02:00
Viktor Olausson
f1c2ebc28d fix(audio): close Windows ASIO before reconfiguration 2026-07-17 12:25:12 +02:00
OmikronApex
dee918e705
Merge pull request #113 from got-feedBack/fix/asio-device-lifecycle-message-thread
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
fix(audio): run device lifecycle ops on the JUCE message thread (Windows ASIO UAF)
2026-07-17 00:00:23 +02:00
OmikronApex
895d0af969 fix(audio): route device probing through the message thread; document timeout desync
Review follow-up (PR #113, finding 1): ProbeDeviceOptions constructs and
destroys short-lived ASIO device objects (DeviceSetup::probeDual /
rateSupportedBy createDevice) on the Node thread - the same
create/destroy-off-the-message-thread pattern as the lifecycle ops, and a
probe ASIOAudioIODevice owns the same reset-timer machinery. Wrap it in
runDeviceLifecycleOp for consistency.

Finding 2 (timeout desync - op reported failed may still complete late)
is documented as a known limitation on the helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-16 23:45:29 +02:00
OmikronApex
aa0bb9d173 fix(audio): fail runDeviceLifecycleOp when the MessageManager is gone
A null MessageManager (pre-init or mid-shutdown) previously fell through
to inline execution on the caller's thread while reporting success -
exactly the unserialised device teardown this helper prevents. Return
false instead, per the documented unavailable/timeout contract.

Addresses CodeRabbit review on #113.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-16 22:54:22 +02:00
OmikronApex
354052bc7c fix(audio): run device lifecycle ops on the JUCE message thread (Windows)
Tester crash (dump 2026-07-15, Focusrite USB ASIO): the driver's deferred
kAsioResetRequest fires a juce::Timer on the addon's JUCE message thread
(ASIOAudioIODevice::timerCallback -> reloadChannelNames) while the Node
thread concurrently destroys the device inside setAudioDevices/stopAudio —
use-after-free, ~5 minutes after every launch.

New runDeviceLifecycleOp() marshals every binding that can create or
destroy a juce::AudioIODevice (setDevice, device-type switches, start/stop,
stream output open/close, extra-input bind/unbind, add/removeSource) onto
the message thread on Windows, serialising them with those timers. Inline
on macOS (dispatch already inline) and Linux (ALSA main-thread contract
unchanged), and inline when already on the message thread to avoid
self-deadlock. Closures capture by value and return through shared_ptr so
a timed-out dispatch that runs late can't touch the caller's dead stack.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-16 22:34:36 +02:00
OmikronApex
10bb631ebb
Merge pull request #112 from got-feedBack/fix/sync-selected-input-on-device-change
fix(audio): stop stale input selection from clobbering the engine on song start
2026-07-16 22:32:36 +02:00
byrongamatos
633ace2052 fix(audio): don't let "Default" wipe the capability input selection
Review of #112: syncSelectedInputSource() treated an empty device name as
"nameless device — invalidate rather than guess" and called removeItem()
on the persisted selection. But the input dropdown's first option is
literally `<option value="">Default</option>`, so "" is the ordinary
"use the OS default" choice, not a nameless device.

init()'s auto-apply calls this on every startup, so a user sitting on
Default had their capability selection deleted at each launch — and that
selection is made in a DIFFERENT ui (the input_setup / tuner picker), so
this silently discarded a device they explicitly chose. Because
audioInputOpenHandler deliberately refuses to guess a device, a cleared
selection leaves plugins with no input at all: the same dead-guitar
symptom the PR set out to fix. Confirmed against the pre-fix build — a
sync with the Default value emits removeItem on the stored key.

"" now means "no opinion": leave the selection to the picker that owns it.

Also extract inputSourceNameKey() as the single place a named input's key
is built. Registration and selection were formatting the same template
independently and had already drifted on the nameless branch (registration
falls back to a positional key; sync emitted none). Tests pin the format,
the Default behaviour, and a round-trip through the open handler's own
parser regex.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-16 14:59:39 +02:00
Jafz2001
9388eb248f Open external links via Steam overlay browser on Steam Deck
Steam Deck / Big Picture (gamepad UI) has no registered default web browser,
so shell.openExternal(httpUrl) -> xdg-open falls back to the KDE Discover store
(a Firefox-install prompt) instead of opening the page. This broke 'Connect with
tone3000' and every external link on the Deck.

When RUNNING_UNDER_STEAM_GAMEPAD_UI (linux + SteamGamepadUI/SteamDeck env),
route web links through Steam's overlay browser via steam://openurl/, with a
fallback to the normal OS opener. Desktop platforms are unaffected. The tone3000
OAuth callback redirects to 127.0.0.1 on the device, so auth must complete in an
on-device browser — the Steam overlay browser reaches localhost.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-15 14:26:17 -04:00
Jafz2001
23047291e3 Expose sandboxed VST parameter values via kListParameters
A sandboxed (out-of-process) VST3 exposes no host-side JUCE parameter proxies,
so SignalChain::getParameters returned an empty list for it. That left the
renderer with no way to read a plugin's live values — the plugin's own editor
window could not be mirrored back into the in-app UI, and "Capture state" was a
no-op for sandboxed gear.

- vst-host/main.cpp: implement the (already-declared) op::kListParameters
  handler, replying with each parameter's index/name/value(0..1)/label/text
  from the child's live AudioProcessor.
- SandboxedProcessor::listSandboxedParameters(): blocking control-pipe
  round-trip returning the reply's params array.
- SignalChain::getParameters: resolve the slot under the audio lock, then do
  the sandboxed IPC AFTER releasing it (a blocking round-trip must not stall
  processBlock) — same discipline getStateInformation() already uses.
  In-process plugins still read their JUCE proxies directly under the lock.

Verified end-to-end against a bundled VST3: getParameters now returns live
values and setParameter -> getParameters round-trips.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-15 10:37:35 -04:00
OmikronApex
d791552261 fix(audio): skip redundant device rebind when opening the selected input
audioInputOpenHandler unconditionally called setDevice, so opening the
selected input (tuner/note_detect on song start) restarted the engine
even when the requested device was already bound — an audible dropout
and the 'Audio paused unexpectedly' seen in tester logs. Now the
handler reads the engine's actual binding (isAudioRunning +
getCurrentDevice) and returns the bound identity without touching the
device when input name and type already match; any read failure falls
through to the normal rebind path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-15 14:28:02 +02:00
OmikronApex
854a53e8f7 fix(audio): sync capability input selection when the user applies a device
A stale feedBack.audioInput.selectedLogicalSourceKey (persisted by the
audio-session capability) survived device changes made in the audio
settings screen. The next plugin to open the 'selected input' (tuner /
note_detect on song start) re-applied the stale device via
audioInputOpenHandler, clobbering the engine's input mid-session —
seen in the field as guitar input going dead when starting a song
(vorrin logs: ASIO M-Audio input replaced by 'Microphone (WO Mic
Device)' ~0.5s after playSong, input level 0.0005).

Fix: after a successful device apply (Apply button and init auto-
apply), select the matching source in the audio-session capability so
its in-memory selection and persisted key follow the user's choice;
fall back to rewriting/removing the localStorage key directly when the
capability or source isn't available yet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-15 14:10:08 +02:00
Byron Gamatos
4dc68a2a28
fix(main): a failing GPU degrades to software, it does not kill the app (#109)
Some checks failed
Addon CI / addon (arm64, macos-14, mac) (push) Has been cancelled
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Has been cancelled
Addon CI / addon (x64, windows-latest, win) (push) Has been cancelled
Ship CI / CI (push) Has been cancelled
When the GPU process can't launch or keeps crashing, Chromium's default is to
give up and FATAL-abort the whole browser process — the user sees the app vanish
("GPU process isn't usable. Goodbye."). We hit this repeatedly: a machine with a
flaky GPU stack took the app down mid-song, and it is the likely reason behind
the "gig always defaults to the classic 2D highway" reports — those machines are
one GPU hiccup away from a crash, not just a fallback.

--disable-gpu-process-crash-limit tells Chromium to keep the browser alive and
fall back to software rendering instead of aborting. Software rasterization
stays enabled (we never pass --disable-software-rasterizer), so there is a path
to land on. The 3D highway then degrades to 2D / runs slow under SwiftShader —
far better than the whole app dying. Cross-platform, set before whenReady.

Validated against a build that reliably FATAL-crashed within seconds on GPU
process launch failure: with the switch it stayed alive 40s+ and never hit the
fatal path — Chromium fell back instead of aborting.

Also adds child-process-gone / render-process-gone log handlers: now that the
app SURVIVES a dead GPU, that log is the only remaining signal it happened,
which is exactly what a "highway is 2D / app was crashing" report needs to be
diagnosable. Log-only.

typecheck + lint clean.
2026-07-15 12:59:47 +02:00
Byron Gamatos
337ee27c3b
Merge pull request #107 from got-feedBack/refactor/audio-engine-tlc
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
Audio engine TLC: decompose the AudioEngine/NodeAddon monoliths + deep-read fixes
2026-07-14 16:15:07 +02:00
byrongamatos
8354194722 fix(audio): LoadVST's macOS branch deadlocked on the chain mutex
The same finding as the clearChain/remove/moveProcessor fix, in a path I
missed on the first pass. CodeRabbit pointed at this line for the wrong reason
(it claimed the mutation was unguarded — it is guarded); the real defect is
WHERE the guard is taken.

LoadVST's macOS branch took a blocking lock_guard on chainMutationMutex on the
Node/main thread. On macOS that thread is also JUCE's message thread, and it
has no pump — its queue is drained by a libuv timer that only runs when the
thread is idle.

Meanwhile a LoadPresetWorker holding that mutex on a libuv thread calls JUCE's
*synchronous* createPluginInstance, which — when called off the message thread,
which is exactly where a worker calls it — posts an AsyncCreateMessage to the
message thread and blocks until it runs (juce_AudioPluginFormat.cpp,
createInstanceFromDescription). So: main thread blocks on the mutex → the
message queue stops draining → the worker's load never completes → the mutex is
never released → the app hangs permanently. Adding a VST while a preset load is
in flight was enough to trigger it.

The in-code comment asserted this was deadlock-safe because "loadVstSandboxAware's
JUCE_MAC branch is a synchronous load on the worker itself" — but JUCE's sync
load is not synchronous off the message thread, which is what makes the cycle.

The plugin INSTANTIATION has to stay on the main thread (JUCE requires it for
VST/AU on macOS), so only the mutation moves: it now goes through
queueChainSlotMutation(), a slot-id-returning sibling of queueChainMutation().
While the worker waits for the mutex the main thread stays free to drain the
queue, so the in-flight load completes and releases it.

The branch is portable C++, so it was compile-checked on Linux by forcing the
#if; the macOS addon lane is the real gate.
2026-07-14 15:13:09 +02:00
byrongamatos
e29312f446 fix(renderer): roll back the mute-suppression latch when the IPC fails
CodeRabbit caught a real bug in the previous commit's fix. The latch mirrors
the NATIVE refcount, but it was flipped before the invoke resolved: a rejected
release left it reading "released" while the engine still held the
suppression, so every later release short-circuited and monitor mute stayed
suppressed for good — the same stuck-suppression bug the latch exists to
prevent, just one level up.

The latch now only stays flipped if the call actually landed, and rolls back
otherwise (guarded so a newer call can't be clobbered by a stale rejection). A
downlevel addon with no arbiter leaves the latch untouched instead of
recording a hold it never acquired.

Pins the whole contract with a vm-extracted unit test on the real screen.js
function: unpaired acquires hold at most one native suppression, cycles stay
balanced across 25 song loads, a rejected release retries, and both the
downlevel and sync-throw paths are clean. Fails 3/5 against the original
branch (the refcount leak) and 2/5 against the pre-rollback version.
2026-07-14 14:52:34 +02:00
byrongamatos
a332c35c9b fix(audio): close the PR #107 review findings
Seven fixes on top of the audio-engine TLC branch, each with the gate that
catches its regression.

Blocking:

- Monitor-mute suppression leaked its refcount. setMonitorMuteSuppressed()
  became a refcounted acquire/release, but screen.js's callers are
  deliberately unpaired: resolveChainRebuildGuard() leaves the suppression on
  when a rebuild yields an empty chain, and returns early without releasing
  while a provider route is still resolving. Harmless against the old latched
  bool, a permanent +1 each against a refcount — after a failed tone rebuild
  the count never returned to zero and monitor mute was silently dead for the
  rest of the session. The renderer now holds at most one suppression.

- Slot ids are monotonic HANDLES (nextSlotId, never reset by clear()), not
  bounded indices, so argSlotId's 4096 ceiling meant that once a session
  created its 4096th processor EVERY guarded binding — setBypass,
  setParameter, remove/moveProcessor, open/closePluginEditor — silently
  no-opped for the rest of the run. Ceiling removed (same for
  SetMultiBypass's hardcoded 4096); unknown ids are still rejected by
  SignalChain::findSlotIndex.

- clearChain / removeProcessor / moveProcessor took chainMutationMutex with a
  blocking lock_guard on the N-API thread — Electron's main thread, and on
  macOS also the JUCE message thread. LoadPreset/LoadVST hold that mutex
  across an unbounded plugin init (done->wait() has no timeout by design), so
  a slow plugin froze the whole main process, every IPC channel with it. They
  now queue on a libuv worker via queueChainMutation() and resolve a promise;
  the bridge awaits them so callers still observe the mutation applied.

Also:

- getChainState() dereferenced raw ProcessorSlot* returned by getAllSlots()
  after the lock was dropped — a concurrent clear() frees them under the
  reader. Replaced with SignalChain::getSlotSummaries(), which copies under
  the lock. getAllSlots() is gone (it had one caller).
- The device-settings migration removed the localStorage copy even when the
  file-store save failed or was unavailable, losing the user's settings.
- SetSlotState and GetParameters kept the raw Int32Value() path: IsNumber()
  is true for NaN, so setSlotState(NaN) wrote onto slot 0 — the same
  coercion class the rest of the branch fixed.
- RendererBus flushed to the LIVE writeIndex, so a disable→re-enable with no
  pull in between discarded the freshly pushed audio along with the stale
  tail. It now snapshots the flush target at disable time.
- LoadPreset's rebuild barrier is now released by a scope guard, so a throw
  between arming it and Queue() can't block editor opens forever.

Gates: new renderer-bus case (fails on the old flush), new slot-id-handle
case (fails on the old ceiling). ctest 9/9, npm test 79 pass / 0 fail,
chain-mutation storm green, addon export contract unchanged.
2026-07-14 14:29:03 +02:00
OmikronApex
1ba9b59e8a fix(audio): validate LoadPreset arg before arming the rebuild barrier
PR #107 round-2 review: beginChainRebuild() ran before
info[0].As<Napi::String>(), so a non-string argument threw between begin
and the worker taking ownership — leaking the barrier and blocking editor
opens permanently. Validate + read the argument first; the barrier is now
armed only on a path where every exit releases it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 12:16:22 +02:00
OmikronApex
ea8c6a9ccd fix(audio): address PR #107 review — close serializer gaps, editor lifetime races, dispatch failures
All 8 CodeRabbit findings verified against the code and fixed:

- ChainOps: macOS LoadVST routes its addProcessor through chainMutationMutex
  (macOS is a first-class platform; deadlock-safe — a worker holding the
  mutex never waits on the Node/main thread there). All four single-slot
  workers (LoadVST/NAM/IR/ReplaceIR) now bump chainGeneration so the
  executor's foreign-write detection sees direct loads, not just presets.
- Rebuild barrier (beginChainRebuild/endChainRebuild): LoadPreset and
  ClearChain arm it before editor teardown; OpenPluginEditor refuses to
  open while a teardown+clear/rebuild is pending (#56 window between
  closeAllPluginEditorWindows returning and the worker taking the mutex).
- EditorWindows: all slot/processor resolution in editor lambdas runs under
  a try_lock of chainMutationMutex (try_lock, never blocking — workers
  holding the mutex block-wait on the message thread). Sandbox promotion
  bumps chainGeneration. editorWindows map is now message-thread-only
  (duplicate-window check and close-erase moved into the queued lambdas).
  Null slot->processor recheck after a faulted promotion capture.
- closeAllPluginEditorWindows returns false on refused post / 15s timeout;
  ClearChain skips the clear and LoadPreset resolves {success:false}
  instead of freeing processors under a live editor.
- AddonContext: dispatchOnMessageThread reports refused-post/timeout;
  doShutdown leaves the message thread running when teardown didn't
  complete instead of unloading mid-destruction.
- RendererBus::push rejects NaN/Inf/non-positive rates and a step that
  underflows to zero; new testRejectsUnusableRates unit case.

Verified: addon builds clean, all 78 JS tests pass (storm, contracts,
executor, N-API fuzz), all 5 engine_units native tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 11:56:08 +02:00
OmikronApex
d887c68014 docs: commit the audio-engine TLC findings + plan this branch implements
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:24:46 +02:00
OmikronApex
48d7e68a91 test: fix two Windows-environment-dependent failures (suite now fully green)
Both tests pre-dated this branch and failed only on Windows checkouts — the
product code was correct in both cases:

- audio-effects-executor 'preload exposes the trusted surface': asserted a
  byte-exact two-line bridge snippet with \n, which never matches a
  core.autocrlf (CRLF) working tree. Line endings are now normalized before
  the includes checks.

- config-paths 'SAFETY: ... ONLY in optInExtras': rebuilt the expected ML
  cache paths with host-native path.join, producing backslash paths that
  never equal the forward-slash simulated envs — failing the mlCaches
  equality and, worse, making the protected-root child checks vacuously
  pass on Windows (a silent coverage gap in the safety assertions). The
  test now uses the envs' resolved torchHome/hfHome fields, exactly what
  production returns, with '/' as the child separator.

npm test: 78/78 passing (1 quarantined storm gate, green under CHAIN_STORM=1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:18:33 +02:00
OmikronApex
9d0963d6d5 feat(audio): getLatencyBreakdown — one owner for every latency term
Deep-read §5: latency had three unreconciled truths — getLatencyMs' static
half-capacity ring guess (42.7 ms), the verifier's input-delta-only offset,
and the renderer bus adding prime+fill+resample that no figure surfaced.

New engine API + export: per-term breakdown (deviceBufferMs, input/output
driver latency, MEASURED split-ring residency, monitor total) plus the
renderer-bus song-audio delay (measured bus fill) as its own term. On the
user's split exclusive setup the measured ring sits at ~10 ms — the legacy
figure overstated monitor latency by ~33 ms (102.7 reported vs ~70 real).

getLatencyMs is unchanged for compatibility; UI adoption of the breakdown
is renderer follow-up. Snapshots regenerated (104 exports).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:05:19 +02:00
OmikronApex
f0340ed425 fix(audio-engine): single persistence store for device settings (TLC Part II §4)
The device config was persisted in TWO stores — the main process's
file-backed settings AND localStorage['slopsmith-audio-device'] — merged on
load by newest-savedAt. A main-side migration/reset left stale localStorage
that could win the timestamp race and resurrect wiped settings, and a device
re-save from either path re-persisted mute flags captured at that moment,
interleaving with the (now-arbitrated) runtime mute writers.

The file store is now the only write target. localStorage is treated as a
one-time migration source: a strictly-newer browser copy is imported into
the file store, then the key is deleted either way — after the first load
the file is the single source of truth.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:02:18 +02:00
OmikronApex
88f881dd1e fix(audio): refcounted monitor-mute arbiter (TLC Part II §2)
The old single monitorMuted atomic had five writers fighting
last-writer-wins: the settings checkbox, startup restore, the executor's
preload read-force-restore, releaseRoute's unconditional setMonitorMute(true)
(which clobbered the user's persisted preference), and the renderer's
song-load suppression (un-refcounted — overlapping windows un-suppressed
each other early).

Native arbiter on SourceChain: userMonitorMute (the preference — checkbox +
restore only), refcounted monitorMuteHolds (force-mute overrides), and
refcounted suppressions (setMonitorMuteSuppressed keeps its bool surface;
true=acquire, false=release, clamped at 0). Effective dry-mute =
(holds || pref) && chain empty && no suppression — the suppressed-beats-muted
precedence is unchanged. New exports: acquire/releaseMonitorMuteHold,
getMonitorMuteState (diag); snapshots regenerated.

Executor rewrite: acquires a suppression (dry-during-load, the default) or a
hold, and releases exactly what it acquired via a single-fire closure that
runs UNCONDITIONALLY (each load owns its acquisition — the stale-snapshot
race against a mid-hold user toggle is structurally gone). releaseRoute no
longer touches mute state at all. The ownership test now pins: preference
API never called, acquire/release balanced.

Renderer callers are unchanged: the checkbox writes the preference as
before, and the song-load suppression sites now compose instead of racing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:01:09 +02:00
OmikronApex
2906d2814b refactor(audio): split N-API bindings into grouped files (phase 7b complete)
Splits the remaining 88 handlers out of NodeAddon.cpp, grouped to match the
preload API sections (plan §3.5): DeviceBindings (enumeration/selection/
audio-control/stream sink), ControlBindings (gain/metering/MIDI/debug
logging), DetectionBindings (pitch/chart/verdict/source-indexed, owns the
shared getValidatedSource), ChainBindings (slot/state/preset), and
BackingBindings. Declarations live in addon/Bindings.h; NodeAddon.cpp keeps
Init/Shutdown and the exports table — which now doubles as the API index the
old 3699-line file lacked — at 459 lines.

This completes the Part IV decomposition: AudioEngine.{h,cpp} 819+3223 →
509+1284 across seven engine/ units; NodeAddon.cpp 3699 → 459 across seven
addon/ units. All gates green (contract-check, storm, arg-fuzz, full suite).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 02:33:24 +02:00
OmikronApex
f473aad920 refactor(audio): move chain workers into ChainOps.cpp (phase 7b)
Moves the five chain-mutating async workers (LoadPreset/LoadVST/LoadNAM/
LoadIR/ReplaceIR), their N-API handlers, loadVstSandboxAware, and the shared
load helpers (decodeStateBlob, loadSafeSampleRate/BlockSize) verbatim into
src/audio/addon/ChainOps.cpp — joining the phase-7a serialization primitives
in their planned home (§3.3). NodeAddon keeps using-declarations; the export
table is unchanged. Storm and arg-fuzz gates stay green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 02:29:16 +02:00
OmikronApex
95b32ba160 refactor(audio): extract EditorWindows (phase 7b)
Moves the in-process plugin editor cluster — PluginEditorWindow, the
slotId→window map, the message-thread teardown pair (closeAll / destroyAll,
the #56 use-after-free guards), OpenPluginEditor with the full Windows
sandbox-promotion flow, and ClosePluginEditor — verbatim into
src/audio/addon/EditorWindows.{h,cpp}. NodeAddon keeps using-declarations;
the export table and ClearChain/LoadPreset teardown calls are unchanged.

The two bindings pick up NapiHelpers slot-id validation while moving (the
same deep-read §2 fix the other bindings got in phase 6 — a NaN slot id
used to coerce to slot 0 and open/close the wrong editor).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 02:23:09 +02:00
OmikronApex
7b72907503 fix(audio-effects): executor detects foreign chain writes via chainGeneration
The JS half of the phase-7a serializer (TLC Part II §1, executor-state
hazard): the executor's stageSlots map (stageId → native slotId) is built at
load time, but any direct loadPreset/clearChain from the audio_engine bundle
or rig_builder's legacy path silently invalidated it — subsequent
setStageBypass/setStageParameter/activateSegment flipped bypass/params on
the WRONG slots or returned no-target with nothing detecting the divergence.

Now: the route records the chainGeneration its load returned; every stage
operation compares it against getChainGeneration() first and reports a
stale-route no-target ('re-load the plan', with expected/current generations)
instead of mutating someone else's chain. loadChainPlan also verifies the
generation didn't move between its loadPreset and the getChainState slot
mapping, rolling back if a foreign write landed in that window. Old addons
without the counter degrade gracefully (checks no-op).

Pinned by a new executor test: fresh route flows, foreign bump → all three
stage ops refuse without touching native slots.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 02:19:42 +02:00
OmikronApex
dd40b2f227 fix(audio): renderer-bus flush flag + reconfigure reads user intent (phase 8)
Two deep-read fixes now homed in their phase-1/2 units:

RendererBus (§4): setEnabled(false) no longer writes readIndex from the
control thread — the ring's designated consumer-side writer is pull(). The
drop-on-disable is now a flushRequested atomic the consumer honors at its
next pull, closing the last SPSC-discipline hole (a concurrent pull
mid-drain could overwrite the control thread's store and replay a stale
tail after re-enable). New unit test pins flush-then-fresh-audio.

setAudioDevices (§3): the restart decision reads state.userWantsAudio
(intent, written only by start/stopAudio) instead of the racy device-state
flag that transient audioDeviceStopped() fires clear — a reconfigure landing
inside a transient-stop window no longer leaves the engine configured but
stopped ('no audio until Start/Apply is pressed again').

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:47:32 +02:00
OmikronApex
db337eaf29 fix(audio): serialize chain mutations + chainGeneration (phase 7a)
The single highest-value fix of the TLC pass (deep-read §1): one native
chain-mutation mutex (addon/ChainOps) held across the FULL Execute() of
every chain worker (LoadPreset/LoadVST/LoadNAM/LoadIR/ReplaceIR) and the
synchronous mutators (clearChain/removeProcessor/moveProcessor). Two
overlapping loadPreset calls can no longer interleave clear()/addProcessor()
into a merged-garbage chain — the plugin-vs-plugin fight becomes
last-writer-wins.

chainGeneration (monotonic, bumped under the mutex) is returned in loadPreset
results and exposed as getChainGeneration (new export, snapshot regenerated),
so the audio-effects executor can detect a foreign write invalidated its
stageSlots map and re-sync instead of flipping bypass/params on wrong slots —
the prerequisite for the single-chain-owner ownership track.

Also rides here: LoadPresetWorker's slot-state restore goes through
setSlotState() instead of const_cast (deep-read §9).

The phase-0 storm test flips from expected-fail to a hard gate: 50 iterations
of concurrent loadPreset now always end with exactly one caller's chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:45:01 +02:00
OmikronApex
d4e0bfc272 refactor(audio): extract AddonContext + NapiHelpers, guard raw N-API args (phase 6)
AddonContext (src/audio/addon/): engine/vstHost lifetime + snapshot rule,
the JUCE message thread with the macOS no-pump fork quarantined into ONE
file, the shutdown latch (exposed as isShuttingDown), doShutdown with a UI
teardown hook (NodeAddon points it at the editor-window nuke, #56), and the
pending-async-load registry. NodeAddon keeps using-declarations so the
binding bodies are unchanged. Also fixes SetBackingSpeed's bare `engine`
dereference — the one binding that dodged the file's own snapshot rule.

NapiHelpers (typed extractors argInt/argSlotId/argFiniteFloat/argBool/
argMidiChannel/argMidiByte) + rewrites of the unguarded bindings — the
deep-read §2 fix, done once: SetParameter/SetBypass/RemoveProcessor/
MoveProcessor/SetMultiBypass/SendMidiToSlot/SetGain plus the St-1 routing
quartet (SetPan/SetPostGain/SetBranch/SetBranchSrc). NaN slot ids no longer
coerce to slot 0; MIDI channel/program are range-checked before JUCE.

New gate: tests/napi-arg-fuzz.test.js — table-driven garbage (NaN/Inf/
negative/string/missing/object) against the real addon; chain state must be
byte-identical after the storm and a valid call must still apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:41:07 +02:00
OmikronApex
6b0bfb7be3 refactor(audio): extract ExtraInputs (phase 5b)
Moves the additional-input-device registry — InputDeviceSlot (manager,
callback, ring, scratches, latency delta, desired-name intent,
permanent-unbind flag), bind/unbind/closeSlot/reopenDesired, the bindable
enumeration, and the per-slot device-callback trio — verbatim into
src/audio/engine/ExtraInputs.{h,cpp}. Sources are prepared/released through
the bound SourcePool (same locking as before); the primary manager reference
serves the duplicate-binding check, latency delta, and enumeration. The
slots array stays public so the split output callback's ring-drain loop is
unchanged; addSource resolves per-slot readiness via resolveForSource().

The (typeName, name) device-identity limitation moves with its honest
comment — its fix lands here later without touching the engine again
(plan §2.3). Completes phase 5; live 28-stage split-mode probe on real
devices behaves identically to pre-move.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:29:16 +02:00