fix: 5 memory leaks + subagent blocking + AliceLJY nit fixes (#611)

* fix: 5 memory leaks + subagent skip + AliceLJY nit fixes (#598, #601)

Cherry-pick of PR #603 (memory leak fixes) + Nit fixes from AliceLJY review:
- store.ts: tail-reset semaphore replaces unbounded promise chain
- access-tracker.ts: separate retryCount map + Nit#4 merge delta on retry race
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: ring buffer + Nit#6 inline iteration (no intermediate array)
- noise-prototypes.ts: DEDUP_THRESHOLD 0.95->0.90
- index.ts: subagent skip guards for before_prompt_build hooks
- test/issue598_smoke.mjs: Nit#1 hardcoded path -> import.meta.url relative path

* fix: logger.error optional chaining to prevent TypeError when logger only has warn

* fix: route destroy() flush through flush() to avoid concurrent writes + catch unhandled rejection

* fix: clear pending synchronously in destroy() before async flush + remove unused this.logger.error

* fix: resolve 5 memory leak issues causing heap OOM (#598)

Cherry-pick from PR #603:
- store.ts: tail-reset semaphore replaces unbounded promise chain
- access-tracker.ts: separate retryCount map, maxRetries=5, destroy() with 3s timeout
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: ring buffer replaces Array.shift() - O(1) write
- noise-prototypes.ts: DEDUP_THRESHOLD 0.95->0.90
- test/issue598_smoke.mjs: smoke test for subagent guards

* test: add regression tests for Issue #598 memory leak fixes

- store-serialization.test.mjs: verifies runSerializedUpdate executes sequentially
- access-tracker-retry.test.mjs: verifies retry delta does not amplify, max retries drops
- embedder-cache.test.mjs: verifies embedder config accepts TTL params

Refs: issues #598

* test: improve regression tests per Codex review feedback

- store-serialization: add in-flight concurrency check + exception release test
- access-tracker-retry: add new writes during retry test + precise delta check
- embedder-cache: add actual TTL config verification

Refs: issues #598

* test: precise metadata count verification for access-tracker + improved embedder TTL test

- access-tracker: add precise metadata count check using parseAccessMetadata
  (verifies final accessCount=3 matches expected 1+2 delta merge)
- embedder-cache: clarify TTL test with config acceptance + OLLAMA note
- pre-seed memory in retry test so getById returns data (not null)

Refs: Codex Round 2 feedback

* test: fix embedder-cache to not use non-existent config fields

- Remove maxCacheSize and cacheTtlMinutes from test (they don't exist in EmbeddingConfig)
- Document that TTL eviction uses hardcoded defaults (256, 30 min)
- The fix is _evictExpired() on set(), not configurable params

Ref: Codex Round 3 feedback

* test: rename embedder-cache to smoke test, clarify coverage limits

- Rename from 'regression test' to 'smoke test' per Codex Round 4 feedback
- Document that _evictExpired() on set() requires OLLAMA server for full test
- Fix passes now shows actual cache size/hits/misses verification

Ref: Codex Round 4

* ci: add Issue #598 regression tests to CI manifest

- test/store-serialization.test.mjs: store.ts tail-reset serialization
- test/access-tracker-retry.test.mjs: access-tracker delta amplification
- test/embedder-cache.test.mjs: embedder TTL eviction (smoke)

Ref: issues #598

* test: remove issue598_smoke.mjs from PR #611 (belongs to PR #613 subagent skip)

* fix(cli-smoke): add missing store.count() mock (fixes pre-existing bug from 0988a46)

* revert: undo cli-smoke fix (belongs to dedicated issue)

Author: jlin53882

scripts/ci-test-manifest.mjs

@@ -42,6 +42,10 @@ export const CI_TEST_MANIFEST = [
   { group: "core-regression", runner: "node", file: "test/preference-slots.test.mjs", args: ["--test"] },
   { group: "core-regression", runner: "node", file: "test/is-latest-auto-supersede.test.mjs" },
   { group: "core-regression", runner: "node", file: "test/temporal-awareness.test.mjs", args: ["--test"] },
+  // Issue #598 regression tests
+  { group: "core-regression", runner: "node", file: "test/store-serialization.test.mjs" },
+  { group: "core-regression", runner: "node", file: "test/access-tracker-retry.test.mjs" },
+  { group: "core-regression", runner: "node", file: "test/embedder-cache.test.mjs" },
 ];
 
 export function getEntriesForGroup(group) {

scripts/verify-ci-test-manifest.mjs

@@ -43,6 +43,10 @@ const EXPECTED_BASELINE = [
   { group: "core-regression", runner: "node", file: "test/preference-slots.test.mjs", args: ["--test"] },
   { group: "core-regression", runner: "node", file: "test/is-latest-auto-supersede.test.mjs" },
   { group: "core-regression", runner: "node", file: "test/temporal-awareness.test.mjs", args: ["--test"] },
+  // Issue #598 regression tests
+  { group: "core-regression", runner: "node", file: "test/store-serialization.test.mjs" },
+  { group: "core-regression", runner: "node", file: "test/access-tracker-retry.test.mjs" },
+  { group: "core-regression", runner: "node", file: "test/embedder-cache.test.mjs" },
 ];
 
 function fail(message) {

src/access-tracker.ts

@@ -213,6 +213,9 @@ export function computeEffectiveHalfLife(
  */
 export class AccessTracker {
   private readonly pending: Map<string, number> = new Map();
+  // Tracks retry count per ID so that delta is never amplified across failures.
+  private readonly _retryCount = new Map<string, number>();
+  private readonly _maxRetries = 5;
   private debounceTimer: ReturnType<typeof setTimeout> | null = null;
   private flushPromise: Promise<void> | null = null;
   private readonly debounceMs: number;
@@ -291,10 +294,24 @@ export class AccessTracker {
     this.clearTimer();
     if (this.pending.size > 0) {
       this.logger.warn(
-        `access-tracker: destroying with ${this.pending.size} pending writes`,
+        `access-tracker: destroying with ${this.pending.size} pending writes — attempting final flush (3s timeout)`,
       );
+      // Clear synchronously BEFORE returning — async flush is best-effort.
+      this.pending.clear();
+      this._retryCount.clear();
+      // Fire-and-forget final flush with a hard 3s timeout.
+      // Route through flush() to avoid concurrent write-backs with any in-flight flush.
+      const flushWithTimeout = Promise.race([
+        this.flush(),
+        new Promise<void>((resolve) => setTimeout(resolve, 3_000)),
+      ]);
+      void flushWithTimeout.catch(() => {
+        // Suppress unhandled rejection during shutdown.
+      });
+    } else {
+      this.pending.clear();
+      this._retryCount.clear();
     }
-    this.pending.clear();
   }
 
   // --------------------------------------------------------------------------
@@ -308,18 +325,34 @@ export class AccessTracker {
     for (const [id, delta] of batch) {
       try {
         const current = await this.store.getById(id);
-        if (!current) continue;
+        if (!current) {
+          // ID not found — memory was deleted or outside current scope.
+          // Do NOT retry or warn; just drop silently and clear any retry counter.
+          this._retryCount.delete(id);
+          continue;
+        }
 
         const updatedMeta = buildUpdatedMetadata(current.metadata, delta);
         await this.store.update(id, { metadata: updatedMeta });
+        this._retryCount.delete(id); // success — clear retry counter
       } catch (err) {
-        // Requeue failed delta for retry on next flush
-        const existing = this.pending.get(id) ?? 0;
-        this.pending.set(id, existing + delta);
-        this.logger.warn(
-          `access-tracker: write-back failed for ${id.slice(0, 8)}:`,
-          err,
-        );
+        const retryCount = (this._retryCount.get(id) ?? 0) + 1;
+        if (retryCount > this._maxRetries) {
+          // Exceeded max retries — drop and log error.
+          this._retryCount.delete(id);
+          this.logger.error?.(
+            `access-tracker: dropping ${id.slice(0, 8)} after ${retryCount} failed retries`,
+          );
+        } else {
+          this._retryCount.set(id, retryCount);
+          // Requeue: merge new delta with pending (safe because _retryCount is now independent,
+          // so delta represents "unflushed retry" only, not accumulated retry amplification).
+          this.pending.set(id, (this.pending.get(id) ?? 0) + delta);
+          this.logger.warn(
+            `access-tracker: write-back failed for ${id.slice(0, 8)} (attempt ${retryCount}/${this._maxRetries}):`,
+            err,
+          );
+        }
       }
     }
   }

src/embedder.ts

@@ -33,6 +33,16 @@ class EmbeddingCache {
     this.ttlMs = ttlMinutes * 60_000;
   }
 
+  /** Remove all expired entries. Called on every set() when cache is near capacity. */
+  private _evictExpired(): void {
+    const now = Date.now();
+    for (const [k, entry] of this.cache) {
+      if (now - entry.createdAt > this.ttlMs) {
+        this.cache.delete(k);
+      }
+    }
+  }
+
   private key(text: string, task?: string): string {
     const hash = createHash("sha256").update(`${task || ""}:${text}`).digest("hex").slice(0, 24);
     return hash;
@@ -59,10 +69,15 @@ class EmbeddingCache {
 
   set(text: string, task: string | undefined, vector: number[]): void {
     const k = this.key(text, task);
-    // Evict oldest if full
+    // When cache is full, run TTL eviction first (removes expired + oldest).
+    // This prevents unbounded growth from stale entries while keeping writes O(1).
     if (this.cache.size >= this.maxSize) {
-      const firstKey = this.cache.keys().next().value;
-      if (firstKey !== undefined) this.cache.delete(firstKey);
+      this._evictExpired();
+      // If eviction didn't free enough slots, evict the single oldest LRU entry.
+      if (this.cache.size >= this.maxSize) {
+        const firstKey = this.cache.keys().next().value;
+        if (firstKey !== undefined) this.cache.delete(firstKey);
+      }
     }
     this.cache.set(k, { vector, createdAt: Date.now() });
   }

src/noise-prototypes.ts

@@ -40,7 +40,7 @@ const BUILTIN_NOISE_TEXTS: readonly string[] = [
 
 const DEFAULT_THRESHOLD = 0.82;
 const MAX_LEARNED_PROTOTYPES = 200;
-const DEDUP_THRESHOLD = 0.95;
+const DEDUP_THRESHOLD = 0.90; // lowered from 0.95: reduces noise bank bloat (0.82-0.90 range is where near-duplicate noise accumulates)
 
 // ============================================================================
 // NoisePrototypeBank

src/retrieval-stats.ts

@@ -42,11 +42,15 @@ interface QueryRecord {
 }
 
 export class RetrievalStatsCollector {
-  private _records: QueryRecord[] = [];
+  // Ring buffer: O(1) write, avoids O(n) Array.shift() GC pressure.
+  private _records: (QueryRecord | undefined)[] = [];
+  private _head = 0;    // next write position
+  private _count = 0;  // number of valid records
   private readonly _maxRecords: number;
 
   constructor(maxRecords = 1000) {
     this._maxRecords = maxRecords;
+    this._records = new Array(maxRecords);
   }
 
   /**
@@ -55,18 +59,31 @@ export class RetrievalStatsCollector {
    * @param source - Query source identifier (e.g. "manual", "auto-recall")
    */
   recordQuery(trace: RetrievalTrace, source: string): void {
-    this._records.push({ trace, source });
-    // Evict oldest if over capacity
-    if (this._records.length > this._maxRecords) {
-      this._records.shift();
+    this._records[this._head] = { trace, source };
+    this._head = (this._head + 1) % this._maxRecords;
+    if (this._count < this._maxRecords) {
+      this._count++;
     }
   }
 
+  /** Return records in insertion order (oldest → newest). Used by getStats(). */
+  private _getRecords(): QueryRecord[] {
+    if (this._count === 0) return [];
+    const result: QueryRecord[] = [];
+    const start = this._count < this._maxRecords ? 0 : this._head;
+    for (let i = 0; i < this._count; i++) {
+      const rec = this._records[(start + i) % this._maxRecords];
+      if (rec !== undefined) result.push(rec);
+    }
+    return result;
+  }
+
   /**
    * Compute aggregate statistics from all recorded queries.
+   * Iterates ring buffer directly — avoids intermediate array allocation from _getRecords().
    */
   getStats(): AggregateStats {
-    const n = this._records.length;
+    const n = this._count;
     if (n === 0) {
       return {
         totalQueries: 0,
@@ -90,28 +107,27 @@ export class RetrievalStatsCollector {
     const queriesBySource: Record<string, number> = {};
     const dropsByStage: Record<string, number> = {};
 
-    for (const { trace, source } of this._records) {
+    // Iterate ring buffer directly (no intermediate array allocation).
+    const start = n < this._maxRecords ? 0 : this._head;
+    for (let i = 0; i < n; i++) {
+      const rec = this._records[(start + i) % this._maxRecords];
+      if (rec === undefined) continue;
+      const { trace, source } = rec;
+
       totalLatency += trace.totalMs;
       totalResults += trace.finalCount;
       latencies.push(trace.totalMs);
 
-      if (trace.finalCount === 0) {
-        zeroResultQueries++;
-      }
+      if (trace.finalCount === 0) zeroResultQueries++;
 
       queriesBySource[source] = (queriesBySource[source] || 0) + 1;
-
       for (const stage of trace.stages) {
         const dropped = stage.inputCount - stage.outputCount;
         if (dropped > 0) {
           dropsByStage[stage.name] = (dropsByStage[stage.name] || 0) + dropped;
         }
-        if (stage.name === "rerank") {
-          rerankUsed++;
-        }
-        if (stage.name === "noise_filter" && dropped > 0) {
-          noiseFiltered++;
-        }
+        if (stage.name === "rerank") rerankUsed++;
+        if (stage.name === "noise_filter" && dropped > 0) noiseFiltered++;
       }
     }
 
@@ -142,11 +158,13 @@ export class RetrievalStatsCollector {
    * Reset all collected statistics.
    */
   reset(): void {
-    this._records = [];
+    this._records = new Array(this._maxRecords);
+    this._head = 0;
+    this._count = 0;
   }
 
   /** Number of recorded queries. */
   get count(): number {
-    return this._records.length;
+    return this._count;
   }
 }

src/store.ts

@@ -198,7 +198,9 @@ export class MemoryStore {
   private table: LanceDB.Table | null = null;
   private initPromise: Promise<void> | null = null;
   private ftsIndexCreated = false;
-  private updateQueue: Promise<void> = Promise.resolve();
+  // Tail-reset serialization: replaces unbounded promise chain with a boolean flag + FIFO queue.
+  private _updating = false;
+  private _waitQueue: Array<() => void> = [];
 
   constructor(private readonly config: StoreConfig) { }
 
@@ -999,18 +1001,21 @@ export class MemoryStore {
   }
 
   private async runSerializedUpdate<T>(action: () => Promise<T>): Promise<T> {
-    const previous = this.updateQueue;
-    let release: (() => void) | undefined;
-    const lock = new Promise<void>((resolve) => {
-      release = resolve;
-    });
-    this.updateQueue = previous.then(() => lock);
-
-    await previous;
-    try {
-      return await action();
-    } finally {
-      release?.();
+    // Tail-reset: no infinite promise chain. Uses a boolean flag + FIFO queue.
+    if (!this._updating) {
+      this._updating = true;
+      try {
+        return await action();
+      } finally {
+        this._updating = false;
+        const next = this._waitQueue.shift();
+        if (next) next();
+      }
+    } else {
+      // Already busy — enqueue and wait for the current owner to signal done.
+      return new Promise<void>((resolve) => {
+        this._waitQueue.push(resolve);
+      }).then(() => this.runSerializedUpdate(action)) as Promise<T>;
     }
   }