diff --git a/v3/@claude-flow/cli/__tests__/ewc-fisher-ema-direction.test.ts b/v3/@claude-flow/cli/__tests__/ewc-fisher-ema-direction.test.ts index c772c49ede..d7ddbaed22 100644 --- a/v3/@claude-flow/cli/__tests__/ewc-fisher-ema-direction.test.ts +++ b/v3/@claude-flow/cli/__tests__/ewc-fisher-ema-direction.test.ts @@ -97,4 +97,91 @@ describe('#dream-2026-09-22 EWCConsolidator.updateFisherFromConfidences EMA dire // same decay rate and inputs whenever decay != 0.5. expect(statsA.avgFisherValue).toBeCloseTo(statsB.avgFisherValue, 10); }); + + // Post-review hardening (ruvnet, PR #3395): the EMA-direction fix above was + // confirmed correct, but the function had no defenses against malformed + // runtime input or a corrupted/incompatible persisted state file. Fixed + // fail-closed rather than adding a follow-up candidate. + it('drops a sample with non-finite embedding values instead of poisoning globalFisher', async () => { + const { EWCConsolidator } = await import('../src/memory/ewc-consolidation.js'); + const consolidator = new EWCConsolidator({ + dimensions: 4, + fisherDecayRate: 0.5, + storagePath: '/tmp/ewc-ema-hardening-test-a.json', + }); + + consolidator.updateFisherFromConfidences([ + { id: 'good', embedding: [3, 3, 3, 3], oldConf: 0.2, newConf: 0.8 }, + ]); + const beforeBad = consolidator.getConsolidationStats().avgFisherValue; + + consolidator.updateFisherFromConfidences([ + { id: 'nan-embedding', embedding: [NaN, 1, 1, 1], oldConf: 0.2, newConf: 0.8 }, + { id: 'infinite-embedding', embedding: [Infinity, 1, 1, 1], oldConf: 0.2, newConf: 0.8 }, + { id: 'nan-confidence', embedding: [1, 1, 1, 1], oldConf: NaN, newConf: 0.8 }, + { id: 'infinite-confidence', embedding: [1, 1, 1, 1], oldConf: 0.2, newConf: Infinity }, + ]); + const afterBad = consolidator.getConsolidationStats().avgFisherValue; + + // Every entry in the second batch was malformed, so none contributed a + // sample -- globalFisher (and avgFisherValue) must be byte-identical to + // before, and in particular must not have become NaN. + expect(afterBad).toBe(beforeBad); + expect(Number.isFinite(afterBad)).toBe(true); + }); + + it('leaves globalFisher and persisted state untouched when no sample contributes (all deltas zero)', async () => { + const { EWCConsolidator } = await import('../src/memory/ewc-consolidation.js'); + const storagePath = '/tmp/ewc-ema-hardening-test-b.json'; + const fs = await import('node:fs'); + if (fs.existsSync(storagePath)) fs.unlinkSync(storagePath); + + const consolidator = new EWCConsolidator({ + dimensions: 4, + fisherDecayRate: 0.5, + storagePath, + }); + + // No prior signal established, and this batch's only entry has a zero + // confidence delta -- sampleCount stays 0. + consolidator.updateFisherFromConfidences([ + { id: 'no-op', embedding: [5, 5, 5, 5], oldConf: 0.5, newConf: 0.5 }, + ]); + + expect(consolidator.getConsolidationStats().avgFisherValue).toBe(0); + // A no-contributing-sample update must not persist -- it has nothing new + // to record and decaying accumulated importance toward an empty batch + // would defeat the same anti-forgetting purpose the EMA fix restores. + expect(fs.existsSync(storagePath)).toBe(false); + }); + + it('quarantines a corrupted persisted globalFisher on load instead of resuming from it', async () => { + const { EWCConsolidator } = await import('../src/memory/ewc-consolidation.js'); + const fs = await import('node:fs'); + const storagePath = '/tmp/ewc-ema-hardening-test-c.json'; + + fs.writeFileSync( + storagePath, + JSON.stringify({ + version: '1.0.0', + config: { lambda: 0.4, dimensions: 4, fisherDecayRate: 0.01 }, + // Corrupted: a non-finite value survived a lossy JSON round-trip as + // `null`, and one entry is negative (impossible for a real Fisher + // value, which is always a sum of squares). + globalFisher: [1.5, null, -2, 3], + patterns: [], + consolidationHistory: [], + savedAt: Date.now(), + }), + ); + + const consolidator = new EWCConsolidator({ dimensions: 4, storagePath }); + await consolidator.initialize(); + + // The whole array is quarantined (not element-by-element patched) -- + // resuming with a zeroed Fisher is a known-safe state; silently keeping + // 3 of 4 "plausible-looking" values from a file already proven corrupt + // is not. + expect(consolidator.getConsolidationStats().avgFisherValue).toBe(0); + }); }); diff --git a/v3/@claude-flow/cli/src/memory/ewc-consolidation.ts b/v3/@claude-flow/cli/src/memory/ewc-consolidation.ts index fd33500d19..039500495b 100644 --- a/v3/@claude-flow/cli/src/memory/ewc-consolidation.ts +++ b/v3/@claude-flow/cli/src/memory/ewc-consolidation.ts @@ -548,29 +548,52 @@ export class EWCConsolidator { for (const change of confidenceChanges) { if (!change.embedding || change.embedding.length === 0) continue; + if (!Number.isFinite(change.oldConf) || !Number.isFinite(change.newConf)) continue; const confDelta = Math.abs(change.newConf - change.oldConf); if (confDelta === 0) continue; - sampleCount++; const len = Math.min(change.embedding.length, this.config.dimensions); - // Squared gradient proxy: embedding scaled by confidence change magnitude + // Squared gradient proxy: embedding scaled by confidence change magnitude. + // Validate the whole sample before folding any of it in -- a single + // non-finite embedding dimension (NaN/Infinity) must not silently poison + // the shared accumulator for every other pattern in this batch. + const sampleFisher = new Array(len); + let sampleValid = true; for (let i = 0; i < len; i++) { const grad = change.embedding[i] * confDelta; - currentFisher[i] += grad * grad; + const sq = grad * grad; + if (!Number.isFinite(sq)) { + sampleValid = false; + break; + } + sampleFisher[i] = sq; + } + if (!sampleValid) continue; + + sampleCount++; + for (let i = 0; i < len; i++) { + currentFisher[i] += sampleFisher[i]; } } - if (sampleCount > 0) { - for (let i = 0; i < this.config.dimensions; i++) { - currentFisher[i] /= sampleCount; - } + // No sample contributed usable signal (all filtered out, or every delta was + // zero): leave globalFisher and persisted state untouched rather than + // decaying accumulated importance toward an empty/no-op batch. + if (sampleCount === 0) return; + + for (let i = 0; i < this.config.dimensions; i++) { + currentFisher[i] /= sampleCount; } // Online EMA: F_new = (1 - decay) * F_old + decay * F_current for (let i = 0; i < this.config.dimensions; i++) { - this.globalFisher[i] = (1 - decay) * this.globalFisher[i] + decay * currentFisher[i]; + const next = (1 - decay) * this.globalFisher[i] + decay * currentFisher[i]; + // Defensive: never let a non-finite result (e.g. from a corrupted prior + // globalFisher[i]) propagate into the live signal -- keep the last known + // finite value instead of poisoning it further. + this.globalFisher[i] = Number.isFinite(next) ? next : this.globalFisher[i]; } this.saveToDisk(); @@ -620,6 +643,22 @@ export class EWCConsolidator { // Private Methods // ============================================================================ + /** + * Validate a loaded globalFisher candidate before trusting it: must be an + * array of exactly `dimensions` finite, non-negative numbers. Fisher + * values are sums of squares (see computeFisherMatrix/recordGradient/ + * updateFisherFromConfidences above), so a negative or non-finite entry + * means the persisted file is corrupted or from an incompatible layout, + * not a legitimate state to resume from. + */ + private isValidFisherArray(value: unknown): value is number[] { + return ( + Array.isArray(value) && + value.length === this.config.dimensions && + value.every((v) => typeof v === 'number' && Number.isFinite(v) && v >= 0) + ); + } + /** * Calculate importance score for a pattern based on usage */ @@ -731,8 +770,15 @@ export class EWCConsolidator { throw new Error(`Unsupported state version: ${state.version}`); } - // Restore state - this.globalFisher = state.globalFisher || new Array(this.config.dimensions).fill(0); + // Restore the Fisher matrix, quarantining anything malformed -- wrong + // length, non-numeric/non-finite entries (e.g. JSON's lossy NaN/Infinity + // -> null round-trip), or negative values (Fisher values are sums of + // squares and can never legitimately be negative) -- rather than + // silently absorbing corrupted or truncated state into live importance + // tracking on restart. + this.globalFisher = this.isValidFisherArray(state.globalFisher) + ? state.globalFisher + : new Array(this.config.dimensions).fill(0); // Restore patterns this.patterns.clear();