mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-11 02:14:09 +00:00
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>
This commit is contained in:
co-authored by
Claude Fable 5
parent
dd40b2f227
commit
7b72907503
@@ -29,6 +29,7 @@ type AudioEffectsNativeAudio = {
|
||||
savePreset?: () => unknown;
|
||||
clearChain?: () => Promise<unknown> | unknown;
|
||||
getChainState?: () => unknown;
|
||||
getChainGeneration?: () => unknown;
|
||||
setBypass?: (slotId: number, bypassed: boolean) => unknown;
|
||||
setMultiBypass?: (changes: Array<{ slotId: number; bypassed: boolean }>) => unknown;
|
||||
setParameter?: (slotId: number, paramIndex: number, value: number) => unknown;
|
||||
@@ -89,6 +90,11 @@ type RouteState = {
|
||||
state: string;
|
||||
activeSegmentId: string;
|
||||
stageSlots: Map<string, number>;
|
||||
// Native chainGeneration this route's stageSlots map was built against
|
||||
// (phase 7a). A foreign writer (legacy loadPreset / clearChain) bumps the
|
||||
// native counter, invalidating the slot ids; stage operations detect the
|
||||
// divergence and report stale-route instead of mutating wrong slots.
|
||||
chainGeneration: number;
|
||||
stageKinds: Map<string, string>;
|
||||
segments: ValidSegment[];
|
||||
loadedAt: string;
|
||||
@@ -373,16 +379,24 @@ function validatePlan(request: unknown): { ok: true; plan: ValidPlan; presetJson
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeLoadResult(value: unknown): { success: boolean; slotsLoaded: number; error: string } {
|
||||
function normalizeLoadResult(value: unknown): { success: boolean; slotsLoaded: number; error: string; chainGeneration: number } {
|
||||
const record = asRecord(value);
|
||||
if (!record) return { success: false, slotsLoaded: 0, error: 'Native load returned an unsupported result' };
|
||||
if (!record) return { success: false, slotsLoaded: 0, error: 'Native load returned an unsupported result', chainGeneration: -1 };
|
||||
return {
|
||||
success: record.success === true,
|
||||
slotsLoaded: safeNumber(record.slotsLoaded, 0),
|
||||
error: bounded(record.error ?? ''),
|
||||
// -1 = addon predates the counter; staleness checks then no-op.
|
||||
chainGeneration: safeNumber(record.chainGeneration, -1),
|
||||
};
|
||||
}
|
||||
|
||||
// Current native chainGeneration, or -1 when the addon doesn't expose it.
|
||||
function currentChainGeneration(nativeAudio: AudioEffectsNativeAudio | null): number {
|
||||
if (!nativeAudio || typeof nativeAudio.getChainGeneration !== 'function') return -1;
|
||||
try { return safeNumber(nativeAudio.getChainGeneration(), -1); } catch { return -1; }
|
||||
}
|
||||
|
||||
function chainSlots(nativeAudio: AudioEffectsNativeAudio | null): Dict[] {
|
||||
if (!nativeAudio || typeof nativeAudio.getChainState !== 'function') return [];
|
||||
const state = nativeAudio.getChainState();
|
||||
@@ -481,7 +495,7 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
await trySetGain(nativeAudio, 'chain', 0);
|
||||
await trySetMonitorMute(nativeAudio, options.preloadMute.dryDuringLoad ? false : true);
|
||||
}
|
||||
let result: { success: boolean; slotsLoaded: number; error: string };
|
||||
let result: { success: boolean; slotsLoaded: number; error: string; chainGeneration: number };
|
||||
try {
|
||||
result = normalizeLoadResult(await nativeAudio.loadPreset(validation.presetJson));
|
||||
} catch (error) {
|
||||
@@ -556,6 +570,23 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
});
|
||||
}
|
||||
|
||||
// Detect a foreign write between our loadPreset and the getChainState
|
||||
// slot mapping above: the mapped ids would describe someone else's
|
||||
// chain. Roll back rather than store a poisoned route.
|
||||
const generationNow = currentChainGeneration(nativeAudio);
|
||||
if (result.chainGeneration >= 0 && generationNow >= 0 && generationNow !== result.chainGeneration) {
|
||||
const rollbackApplied = await restorePreset(nativeAudio, rollbackPreset);
|
||||
if (options.preloadMute?.enabled) schedulePreloadRestore(nativeAudio, previousMonitorMute, options.gains.chain ?? options.preloadMute.targetGain, 0, () => restoreVersion === preloadRestoreVersion);
|
||||
return safeOutcome('degraded', 'Native chain was modified by another writer during plan load', {
|
||||
routeKey: validation.plan.routeKey,
|
||||
providerId: validation.plan.providerId,
|
||||
planId: validation.plan.planId,
|
||||
expectedGeneration: result.chainGeneration,
|
||||
currentGeneration: generationNow,
|
||||
rollbackApplied,
|
||||
});
|
||||
}
|
||||
|
||||
const route: RouteState = {
|
||||
routeKey: validation.plan.routeKey,
|
||||
providerId: validation.plan.providerId,
|
||||
@@ -563,6 +594,7 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
state: result.slotsLoaded >= nativeStages.length ? 'loaded' : 'degraded',
|
||||
activeSegmentId: '',
|
||||
stageSlots,
|
||||
chainGeneration: result.chainGeneration,
|
||||
stageKinds,
|
||||
segments: validation.plan.segments,
|
||||
loadedAt: now(),
|
||||
@@ -634,6 +666,23 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
return updateOutcome(route, safeOutcome('handled', 'Audio-effects route gain applied', { route: safeRoute(route), gains }));
|
||||
}
|
||||
|
||||
// Stage operations act on the stageSlots map built at load time; a foreign
|
||||
// chain write since then (legacy loadPreset / clearChain — the documented
|
||||
// three-writer fight) makes those slot ids describe someone else's chain.
|
||||
// Detect via chainGeneration and report a stale route (the provider should
|
||||
// re-load its plan) instead of flipping bypass/params on wrong slots.
|
||||
function staleRouteOutcome(route: RouteState, nativeAudio: AudioEffectsNativeAudio | null, extra: Dict): SafeOutcome | null {
|
||||
if (route.chainGeneration < 0) return null; // addon predates the counter
|
||||
const generationNow = currentChainGeneration(nativeAudio);
|
||||
if (generationNow < 0 || generationNow === route.chainGeneration) return null;
|
||||
route.state = 'stale';
|
||||
return safeOutcome('no-target', 'Native chain was modified by another writer since this route loaded — re-load the plan', {
|
||||
...extra,
|
||||
expectedGeneration: route.chainGeneration,
|
||||
currentGeneration: generationNow,
|
||||
});
|
||||
}
|
||||
|
||||
async function setStageBypass(request: unknown): Promise<SafeOutcome> {
|
||||
const input = asRecord(request) || {};
|
||||
const routeKey = safeId(input.routeKey ?? DEFAULT_ROUTE_KEY, DEFAULT_ROUTE_KEY);
|
||||
@@ -644,6 +693,8 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
if (slotId == null) return updateOutcome(route, safeOutcome('no-target', 'Audio-effects stage is not mapped to a native slot', { routeKey, stageId }));
|
||||
const nativeAudio = getAudio();
|
||||
if (!nativeAudio || typeof nativeAudio.setBypass !== 'function') return updateOutcome(route, safeOutcome('unavailable', 'Native stage bypass is unavailable', { routeKey, stageId }));
|
||||
const stale = staleRouteOutcome(route, nativeAudio, { routeKey, stageId });
|
||||
if (stale) return updateOutcome(route, stale);
|
||||
try {
|
||||
const result = await nativeAudio.setBypass(slotId, safeBool(input.bypassed, false));
|
||||
if (nativeFailure(result)) return updateOutcome(route, safeOutcome('failed', 'Native stage bypass returned failure', { routeKey, stageId }));
|
||||
@@ -668,6 +719,8 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
}
|
||||
const nativeAudio = getAudio();
|
||||
if (!nativeAudio || typeof nativeAudio.setParameter !== 'function') return updateOutcome(route, safeOutcome('unavailable', 'Native stage parameter control is unavailable', { routeKey, stageId }));
|
||||
const stale = staleRouteOutcome(route, nativeAudio, { routeKey, stageId });
|
||||
if (stale) return updateOutcome(route, stale);
|
||||
try {
|
||||
const result = await nativeAudio.setParameter(slotId, paramIndex, value);
|
||||
if (nativeFailure(result)) return updateOutcome(route, safeOutcome('failed', 'Native stage parameter returned failure', { routeKey, stageId, paramIndex }));
|
||||
@@ -687,6 +740,8 @@ export function createAudioEffectsExecutor(getAudio: NativeAudioGetter) {
|
||||
if (!segment) return updateOutcome(route, safeOutcome('no-target', 'Audio-effects segment is not present in the loaded plan', { routeKey, segmentId }));
|
||||
const nativeAudio = getAudio();
|
||||
if (!nativeAudio || typeof nativeAudio.setMultiBypass !== 'function') return updateOutcome(route, safeOutcome('unavailable', 'Native multi-bypass is unavailable', { routeKey, segmentId }));
|
||||
const stale = staleRouteOutcome(route, nativeAudio, { routeKey, segmentId });
|
||||
if (stale) return updateOutcome(route, stale);
|
||||
const active = new Set(segment.stageIds);
|
||||
const changes = Array.from(route.stageSlots.entries()).map(([stageId, slotId]) => ({
|
||||
slotId,
|
||||
|
||||
@@ -390,3 +390,51 @@ test('preload exposes the trusted audio-effects executor surface', () => {
|
||||
assert.equal(bridge.includes('vstSlotPaths.clear();\n return await audioEffects.loadChainPlan(request);'), true);
|
||||
assert.equal(bridge.includes('if (normalizedPayload.inputType !== normalizedPayload.outputType)'), true);
|
||||
});
|
||||
|
||||
test('audio-effects executor detects a foreign chain write via chainGeneration and reports a stale route', async () => {
|
||||
const { createAudioEffectsExecutor } = loadExecutorModule();
|
||||
// Native stub with the phase-7a generation counter: our load lands at
|
||||
// generation 5; a foreign writer (legacy loadPreset / clearChain) later
|
||||
// bumps it to 6, invalidating the route's stageSlots map.
|
||||
let generation = 5;
|
||||
const bypassCalls = [];
|
||||
const native = {
|
||||
loadPreset: async presetJson => ({ success: true, slotsLoaded: JSON.parse(presetJson).chain.length, chainGeneration: generation }),
|
||||
getChainState: () => [{ id: 10 }, { id: 11 }],
|
||||
getChainGeneration: () => generation,
|
||||
setBypass: (slotId, bypassed) => { bypassCalls.push([slotId, bypassed]); return true; },
|
||||
setMultiBypass: changes => { bypassCalls.push(['multi', changes]); return true; },
|
||||
setParameter: () => true,
|
||||
};
|
||||
const executor = createAudioEffectsExecutor(() => native);
|
||||
const loaded = await executor.loadChainPlan({
|
||||
authorization: 'playback-session',
|
||||
plan: plan(),
|
||||
assets: {
|
||||
'asset:pre': { kind: 'nam', path: tempAsset('.nam'), safeName: 'pre' },
|
||||
'asset:cab': { kind: 'ir', path: tempAsset('.wav'), safeName: 'cab' },
|
||||
},
|
||||
});
|
||||
assert.equal(loaded.outcome, 'handled');
|
||||
|
||||
// Generation unchanged: stage ops flow normally.
|
||||
const fresh = await executor.setStageBypass({ routeKey: 'desktop-main', stageId: 'pre', bypassed: true });
|
||||
assert.equal(fresh.outcome, 'handled');
|
||||
assert.equal(bypassCalls.length, 1);
|
||||
|
||||
// Foreign write bumps the native counter.
|
||||
generation = 6;
|
||||
const stale = await executor.setStageBypass({ routeKey: 'desktop-main', stageId: 'pre', bypassed: false });
|
||||
assert.equal(stale.outcome, 'no-target');
|
||||
assert.match(stale.reason, /modified by another writer/);
|
||||
assert.equal(stale.payload.expectedGeneration, 5);
|
||||
assert.equal(stale.payload.currentGeneration, 6);
|
||||
assert.equal(bypassCalls.length, 1, 'stale route must NOT touch native slots');
|
||||
|
||||
// Segment activation and parameters are equally guarded.
|
||||
const seg = await executor.activateSegment({ routeKey: 'desktop-main', segmentId: 'lead' });
|
||||
assert.equal(seg.outcome, 'no-target');
|
||||
const param = await executor.setStageParameter({ routeKey: 'desktop-main', stageId: 'pre', paramIndex: 0, value: 0.5 });
|
||||
assert.equal(param.outcome, 'no-target');
|
||||
assert.equal(bypassCalls.length, 1);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user